[U-Boot] [PATCH 07/51] phy: Allow forcing clause 45 access

Mario Six mario.six at gdsys.cc
Tue Jul 25 07:30:30 UTC 2017


Hi Simon,

On Tue, Jul 18, 2017 at 4:01 PM, Simon Glass <sjg at chromium.org> wrote:
> On 14 July 2017 at 05:54, Mario Six <mario.six at gdsys.cc> wrote:
>> From: Dirk Eibach <dirk.eibach at gdsys.cc>
>>
>> get_phy_device_by_mask() assumes that a clause 45 phy does not respond
>> to clause 22 requests. That is not true at least for Marvell 88X2242.
>> So allow forcing clause 45 access to prevent reading bogus device ids.
>>
>> Signed-off-by: Dirk Eibach <dirk.eibach at gdsys.cc>
>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>> ---
>>
>>  drivers/net/phy/phy.c | 12 +++++++-----
>>  include/phy.h         | 12 ++++++++++--
>>  2 files changed, 17 insertions(+), 7 deletions(-)
>
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> Please see below.
>
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 97e0bc022b..08ec2f2ad6 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -707,7 +707,7 @@ static struct phy_device *search_for_existing_phy(struct mii_dev *bus,
>>  }
>>
>>  static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus,
>> -               unsigned phy_mask, phy_interface_t interface)
>> +               unsigned phy_mask, phy_interface_t interface, bool force_c45)
>>  {
>>         int i;
>>         struct phy_device *phydev;
>> @@ -718,6 +718,8 @@ static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus,
>>         /* Try Standard (ie Clause 22) access */
>>         /* Otherwise we have to try Clause 45 */
>>         for (i = 0; i < 5; i++) {
>> +               if (!i && force_c45)
>> +                       continue;
>>                 phydev = create_phy_by_mask(bus, phy_mask,
>>                                 i ? i : MDIO_DEVAD_NONE, interface);
>>                 if (IS_ERR(phydev))
>> @@ -748,7 +750,7 @@ static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus,
>>  static struct phy_device *get_phy_device(struct mii_dev *bus, int addr,
>>                                          phy_interface_t interface)
>>  {
>> -       return get_phy_device_by_mask(bus, 1 << addr, interface);
>> +       return get_phy_device_by_mask(bus, 1 << addr, interface, false);
>>  }
>>
>>  int phy_reset(struct phy_device *phydev)
>> @@ -817,8 +819,8 @@ int miiphy_reset(const char *devname, unsigned char addr)
>>         return phy_reset(phydev);
>>  }
>>
>> -struct phy_device *phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask,
>> -               phy_interface_t interface)
>> +struct phy_device *__phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask,
>> +               phy_interface_t interface, bool force_c45)
>>  {
>>         /* Reset the bus */
>>         if (bus->reset) {
>> @@ -828,7 +830,7 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask,
>>                 udelay(15000);
>>         }
>>
>> -       return get_phy_device_by_mask(bus, phy_mask, interface);
>> +       return get_phy_device_by_mask(bus, phy_mask, interface, force_c45);
>>  }
>>
>>  #ifdef CONFIG_DM_ETH
>> diff --git a/include/phy.h b/include/phy.h
>> index 4f2094bdf0..75a9ae3314 100644
>> --- a/include/phy.h
>> +++ b/include/phy.h
>> @@ -227,8 +227,16 @@ static inline int is_10g_interface(phy_interface_t interface)
>>
>>  int phy_init(void);
>>  int phy_reset(struct phy_device *phydev);
>> -struct phy_device *phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask,
>> -               phy_interface_t interface);
>> +struct phy_device *__phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask,
>> +               phy_interface_t interface, bool force_c45);
>
> Can you please add a function comment?
>

I'll add a comment in v2.

>> +static inline struct phy_device *phy_find_by_mask(struct mii_dev *bus,
>> +               unsigned phy_mask, phy_interface_t interface) {
>> +       return __phy_find_by_mask(bus, phy_mask, interface, 0);
>> +}
>> +static inline struct phy_device *phy_find_by_mask_c45(struct mii_dev *bus,
>> +               unsigned phy_mask, phy_interface_t interface) {
>> +       return __phy_find_by_mask(bus, phy_mask, interface, 1);
>> +}
>>  #ifdef CONFIG_DM_ETH
>>  void phy_connect_dev(struct phy_device *phydev, struct udevice *dev);
>>  struct phy_device *phy_connect(struct mii_dev *bus, int addr,
>> --
>> 2.11.0
>>

Best regards,

Mario


More information about the U-Boot mailing list