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

[Christian Gmeiner]

2017-06-07 6:33 GMT+02:00 Hannes Schmelzer <hannes at schmelzer.or.at>:
> 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
>

gentle ping

greets
--
Christian Gmeiner, MSc

https://www.youtube.com/user/AloryOFFICIAL
https://soundcloud.com/christian-gmeiner