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

Dan Murphy dmurphy at ti.com
Tue Apr 21 13:54:06 CEST 2020


Michal

On 4/21/20 6:52 AM, Michal Simek wrote:
> 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.

Yes they do but the inline functions don't.

I will actually go one better and add the k-doc to the other in-line 
functions above for consistency.


>>>> +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.

I am currently working on bug fixing the upstream DP83869 driver so I 
can add some of the fixes.  I will throw your Reported-by in the commit 
message if you want.

Dan



More information about the U-Boot mailing list