[RFC PATCH 2/3] net: phy: Add helper routines to set and clear bits

Michal Simek michal.simek at xilinx.com
Tue Apr 21 13:52:27 CEST 2020


On 21. 04. 20 13:43, Dan Murphy wrote:
> Michal
> 
> Thanks for the review
> 
> On 4/21/20 3:04 AM, Michal Simek wrote:
>> On 20. 04. 20 20:53, Dan Murphy wrote:
>>> Add phy_set/clear_bit helper routines so that ported drivers from the
>>> kernel can use these functions.
>>>
>>> Signed-off-by: Dan Murphy <dmurphy at ti.com>
>>> ---
>>>   include/phy.h | 38 ++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 38 insertions(+)
>>>
>>> diff --git a/include/phy.h b/include/phy.h
>>> index b5de14cbfc29..050c989fa537 100644
>>> --- a/include/phy.h
>>> +++ b/include/phy.h
>>> @@ -257,6 +257,44 @@ static inline int phy_write_mmd(struct
>>> phy_device *phydev, int devad,
>>>       return phy_write(phydev, MDIO_DEVAD_NONE, MII_MMD_DATA, val);
>>>   }
>>>   
>> kernel-doc description would be good.
> 
> Just following the files coding practice.  No other APIs have kernel-doc.
> 
> Not that I won't add them but it is worth pointing this out.

Not sure if this straight copy from Linux but from phy_init() down all
functions used that.

>>
>>> +static inline int phy_set_bits_mmd(struct phy_device *phydev, int
>>> devad,
>>> +                   u32 regnum, u16 val)
>>> +{
>>> +    int value;
>>> +    int ret;
>> nit: on one line should be better.
> 
> I was under the impression that when code is side ported to uBoot it
> should be as close to the origin as possible for bug fixes to be easily
> understood and applied.
> 
> Please correct my understanding if I had the wrong impression.
> 
> Not that this specific nit is going to make a difference but it applies
> to other comments made in the other patches.

then would be good to fix it in origin source. :-) Even this fix is
quite trivial.

Thanks,
Michal




More information about the U-Boot mailing list