mbox series

[net-next,v6,0/4] RGMII Internal delay common property

Message ID 20200604111410.17918-1-dmurphy@ti.com
Headers show
Series RGMII Internal delay common property | expand

Message

Dan Murphy June 4, 2020, 11:14 a.m. UTC
Hello

The RGMII internal delay is a common setting found in most RGMII capable PHY
devices.  It was found that many vendor specific device tree properties exist
to do the same function. This creates a common property to be used for PHY's
that have tunable internal delays for the Rx and Tx paths.

Dan Murphy (4):
  dt-bindings: net: Add tx and rx internal delays
  net: phy: Add a helper to return the index for of the internal delay
  dt-bindings: net: Add RGMII internal delay for DP83869
  net: dp83869: Add RGMII internal delay configuration

 .../devicetree/bindings/net/ethernet-phy.yaml | 13 ++++
 .../devicetree/bindings/net/ti,dp83869.yaml   | 16 ++++-
 drivers/net/phy/dp83869.c                     | 53 ++++++++++++++-
 drivers/net/phy/phy_device.c                  | 68 +++++++++++++++++++
 include/linux/phy.h                           |  4 ++
 5 files changed, 150 insertions(+), 4 deletions(-)

Comments

Jakub Kicinski June 4, 2020, 4:25 p.m. UTC | #1
On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:
> Add RGMII internal delay configuration for Rx and Tx.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Hi Dan, please make sure W=1 C=1 build is clean:

drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
  103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
      |                  ^~~~~~~~~~~~~~~~~~~~~~

Also net-next is closed right now, you can post RFCs but normal patches
should be deferred until after net-next reopens.
Dan Murphy June 4, 2020, 4:38 p.m. UTC | #2
Jakub

On 6/4/20 11:25 AM, Jakub Kicinski wrote:
> On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:
>> Add RGMII internal delay configuration for Rx and Tx.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> Hi Dan, please make sure W=1 C=1 build is clean:
>
> drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
>    103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
>        |                  ^~~~~~~~~~~~~~~~~~~~~~

I built with W=1 and C=1 and did not see this warning.

What defconfig are you using?

Can you check if CONFIG_OF_MDIO is set or not?  That would be the only 
way that warning would come up.

> Also net-next is closed right now, you can post RFCs but normal patches
> should be deferred until after net-next reopens.

I know net-next is closed.

I pinged David M when it was open about what is meant by "new" patches 
in the net-dev FAQ.  So I figured I would send the patches to see what 
the response was.

To me these are not new they are in process patches.  My understand is 
New is v1 patchesets.

But now I have the answer.

Dan
Jakub Kicinski June 4, 2020, 4:48 p.m. UTC | #3
On Thu, 4 Jun 2020 11:38:14 -0500 Dan Murphy wrote:
> Jakub
> 
> On 6/4/20 11:25 AM, Jakub Kicinski wrote:
> > On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:  
> >> Add RGMII internal delay configuration for Rx and Tx.
> >>
> >> Signed-off-by: Dan Murphy <dmurphy@ti.com>  
> > Hi Dan, please make sure W=1 C=1 build is clean:
> >
> > drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
> >    103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
> >        |                  ^~~~~~~~~~~~~~~~~~~~~~  
> 
> I built with W=1 and C=1 and did not see this warning.
> 
> What defconfig are you using?

allmodconfig with gcc-10

> Can you check if CONFIG_OF_MDIO is set or not?  That would be the only 
> way that warning would come up.

Hm. I don't have the config from this particular build but just running
allmodconfig makes it CONFIG_OF_MDIO=m

> > Also net-next is closed right now, you can post RFCs but normal patches
> > should be deferred until after net-next reopens.  
> 
> I know net-next is closed.
> 
> I pinged David M when it was open about what is meant by "new" patches 
> in the net-dev FAQ.  So I figured I would send the patches to see what 
> the response was.
> 
> To me these are not new they are in process patches.  My understand is 
> New is v1 patchesets.
> 
> But now I have the answer.

Oh sorry, I may be wrong in this case, I haven't tracked this series.
Dan Murphy June 4, 2020, 8:27 p.m. UTC | #4
Jakub

On 6/4/20 11:48 AM, Jakub Kicinski wrote:
> On Thu, 4 Jun 2020 11:38:14 -0500 Dan Murphy wrote:
>> Jakub
>>
>> On 6/4/20 11:25 AM, Jakub Kicinski wrote:
>>> On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:
>>>> Add RGMII internal delay configuration for Rx and Tx.
>>>>
>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> Hi Dan, please make sure W=1 C=1 build is clean:
>>>
>>> drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
>>>     103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
>>>         |                  ^~~~~~~~~~~~~~~~~~~~~~
>> I built with W=1 and C=1 and did not see this warning.
>>
>> What defconfig are you using?
> allmodconfig with gcc-10
>
>> Can you check if CONFIG_OF_MDIO is set or not?  That would be the only
>> way that warning would come up.
> Hm. I don't have the config from this particular build but just running
> allmodconfig makes it CONFIG_OF_MDIO=m

OK that makes sense then.  That is an existing bug that shows up because 
of this.

#ifdef CONFIG_OF_MDIO

So the addition of the array exposed an existing issue.

That bug fix can go to net then.
>>> Also net-next is closed right now, you can post RFCs but normal patches
>>> should be deferred until after net-next reopens.
>> I know net-next is closed.
>>
>> I pinged David M when it was open about what is meant by "new" patches
>> in the net-dev FAQ.  So I figured I would send the patches to see what
>> the response was.
>>
>> To me these are not new they are in process patches.  My understand is
>> New is v1 patchesets.
>>
>> But now I have the answer.
> Oh sorry, I may be wrong in this case, I haven't tracked this series.
>
It says v6 in $subject.

But still you may be correct I don't know

Dan