[U-Boot] [PATCH] drivers/net/phy/fixed: do not overwrite addr

[Hannes Schmelzer]

On 06/06/2017 02:57 PM, Christian Gmeiner wrote:
> 2017-06-06 14:51 GMT+02:00 Hannes Schmelzer
> <Hannes.Schmelzer at br-automation.com>:
>> "U-Boot" <u-boot-bounces at lists.denx.de> schrieb am 06.06.2017 14:35:29:
>>
>>> Von: Christian Gmeiner <christian.gmeiner at gmail.com>
>>> An: u-boot at lists.denx.de,
>>> Kopie: joe.hershberger at ni.com, oe5hpm at oevsv.at
>>> Datum: 06.06.2017 14:35
>>> Betreff: [U-Boot] [PATCH] drivers/net/phy/fixed: do not overwrite addr
>>> Gesendet von: "U-Boot" <u-boot-bounces at lists.denx.de>
>>>
>>> phy_device_create(..) sets the addr of phy_device with a sane value.
>>> There is no need overwrite it.
>> Hi Christian,
>>
>> The addr=0 was not an accident.
>> Since there is no real phy in this case outside, i did set the address to
>> zero (no real phy has address #0),
>> later on in the board_phy_config(...) i look at the phydev->addr for
>> detecting that i deal with the "fixed-link" driver.
>>
>> But for sure there are better ways to handle this, any suggestion ?
>>
> I have a similar use case where I have one variant of a device with
> fixed-link and an
> other variant with a 'real' phy.
>
> Here is how I handle that case:
>
> -------8<------
>
> int get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id)
> {
>      int phy_reg;
>
>      /* MR variant has a fixed phy at address 5 */
>      if (addr == 5) {
>          *phy_id = PHY_FIXED_ID;
>          return 0;
>      }
>
>      /* Grab the bits from PHYIR1, and put them
>       * in the upper half */
>      phy_reg = bus->read(bus, addr, devad, MII_PHYSID1);
>
>      if (phy_reg < 0)
>          return -EIO;
>
>      *phy_id = (phy_reg & 0xffff) << 16;
>
>      /* Grab the bits from PHYIR2, and put them in the lower half */
>      phy_reg = bus->read(bus, addr, devad, MII_PHYSID2);
>
>      if (phy_reg < 0)
>          return -EIO;
>
>      *phy_id |= (phy_reg & 0xffff);
>
>      return 0;
> }
>
> int board_eth_init(bd_t *bis)
> {
>      uint32_t base = IMX_FEC_BASE;
>      struct mii_dev *bus = NULL;
>      struct phy_device *phydev = NULL;
>      int ret;
>
>      setup_iomux_enet();
>
>      bus = fec_get_miibus(base, -1);
>      if (!bus)
>          return -EINVAL;
>
>      /* scan phy 0 and 5 */
>      phydev = phy_find_by_mask(bus, 0x21, PHY_INTERFACE_MODE_RGMII);
>      if (!phydev) {
>          ret = -EINVAL;
>          goto free_bus;
>      }
>
>      /* depending on the phy address we can detect our board version */
>      if (phydev->addr == 0)
>          setenv("boardver", "");
>      else
>          setenv("boardver", "mr");
>
>      ret = fec_probe(bis, -1, base, bus, phydev);
>      if (ret)
>          goto free_phydev;
>
>      return 0;
>
> free_phydev:
>      free(phydev);
> free_bus:
>      free(bus);
>      return ret;
> }
>
> -------8<------
>
> As you can see I use phydev->addr to detect my board variant and with this patch
> everything works as expected.
Hi Christian,

yes doing that stuff using the phy-id is much more elegant.
I will adapt my board-code doing so.

Reviewed-by: Hannes Schmelzer <oe5hpm at oevsv.at>
Tested-by: Hannes Schmelzer <oe5hpm at oevsv.at>
> greets
> --
> Christian Gmeiner, MSc
>
> https://www.youtube.com/user/AloryOFFICIAL
> https://soundcloud.com/christian-gmeiner
cheers,
Hannes