| Message ID | 20200604111410.17918-5-dmurphy@ti.com |
|---|---|
| State | Deferred |
| Delegated to: | David Miller |
| Headers | show |
| Series | RGMII Internal delay common property | expand |
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.
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
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.
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
Hi Dan,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on robh/for-next sparc-next/master net/master linus/master v5.7 next-20200604]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Dan-Murphy/RGMII-Internal-delay-common-property/20200605-003113
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git cb8e59cc87201af93dfbb6c3dccc8fcad72a09c2
config: x86_64-allmodconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ac47588bc4ff5927a01ed6fcd269ce86aba52a7c)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/net/phy/dp83869.c:103:18: warning: unused variable 'dp83869_internal_delay' [-Wunused-const-variable]
static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
^
1 warning generated.
vim +/dp83869_internal_delay +103 drivers/net/phy/dp83869.c
102
> 103 static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
104 1750, 2000, 2250, 2500, 2750, 3000,
105 3250, 3500, 3750, 4000};
106
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c index cfb22a21a2e6..801341edbe31 100644 --- a/drivers/net/phy/dp83869.c +++ b/drivers/net/phy/dp83869.c @@ -64,6 +64,10 @@ #define DP83869_RGMII_TX_CLK_DELAY_EN BIT(1) #define DP83869_RGMII_RX_CLK_DELAY_EN BIT(0) +/* RGMIIDCTL */ +#define DP83869_RGMII_CLK_DELAY_SHIFT 4 +#define DP83869_CLK_DELAY_DEF 7 + /* STRAP_STS1 bits */ #define DP83869_STRAP_OP_MODE_MASK GENMASK(2, 0) #define DP83869_STRAP_STS1_RESERVED BIT(11) @@ -78,9 +82,6 @@ #define DP83869_PHYCR_FIFO_DEPTH_MASK GENMASK(15, 12) #define DP83869_PHYCR_RESERVED_MASK BIT(11) -/* RGMIIDCTL bits */ -#define DP83869_RGMII_TX_CLK_DELAY_SHIFT 4 - /* IO_MUX_CFG bits */ #define DP83869_IO_MUX_CFG_IO_IMPEDANCE_CTRL 0x1f @@ -99,6 +100,10 @@ #define DP83869_OP_MODE_MII BIT(5) #define DP83869_SGMII_RGMII_BRIDGE BIT(6) +static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500, + 1750, 2000, 2250, 2500, 2750, 3000, + 3250, 3500, 3750, 4000}; + enum { DP83869_PORT_MIRRORING_KEEP, DP83869_PORT_MIRRORING_EN, @@ -108,6 +113,8 @@ enum { struct dp83869_private { int tx_fifo_depth; int rx_fifo_depth; + s32 rx_id_delay; + s32 tx_id_delay; int io_impedance; int port_mirroring; bool rxctrl_strap_quirk; @@ -182,6 +189,7 @@ static int dp83869_of_init(struct phy_device *phydev) struct dp83869_private *dp83869 = phydev->priv; struct device *dev = &phydev->mdio.dev; struct device_node *of_node = dev->of_node; + int delay_size = ARRAY_SIZE(dp83869_internal_delay); int ret; if (!of_node) @@ -232,6 +240,20 @@ static int dp83869_of_init(struct phy_device *phydev) &dp83869->tx_fifo_depth)) dp83869->tx_fifo_depth = DP83869_PHYCR_FIFO_DEPTH_4_B_NIB; + dp83869->rx_id_delay = phy_get_internal_delay(phydev, dev, + &dp83869_internal_delay[0], + delay_size, true); + if (dp83869->rx_id_delay < 0) + dp83869->rx_id_delay = + dp83869_internal_delay[DP83869_CLK_DELAY_DEF]; + + dp83869->tx_id_delay = phy_get_internal_delay(phydev, dev, + &dp83869_internal_delay[0], + delay_size, false); + if (dp83869->tx_id_delay < 0) + dp83869->tx_id_delay = + dp83869_internal_delay[DP83869_CLK_DELAY_DEF]; + return ret; } #else @@ -394,6 +416,31 @@ static int dp83869_config_init(struct phy_device *phydev) dp83869->clk_output_sel << DP83869_IO_MUX_CFG_CLK_O_SEL_SHIFT); + if (phy_interface_is_rgmii(phydev)) { + ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIIDCTL, + dp83869->rx_id_delay | + dp83869->tx_id_delay << DP83869_RGMII_CLK_DELAY_SHIFT); + if (ret) + return ret; + + val = phy_read_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL); + val &= ~(DP83869_RGMII_TX_CLK_DELAY_EN | + DP83869_RGMII_RX_CLK_DELAY_EN); + + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) + val |= (DP83869_RGMII_TX_CLK_DELAY_EN | + DP83869_RGMII_RX_CLK_DELAY_EN); + + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) + val |= DP83869_RGMII_TX_CLK_DELAY_EN; + + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) + val |= DP83869_RGMII_RX_CLK_DELAY_EN; + + ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_RGMIICTL, + val); + } + return ret; }
Add RGMII internal delay configuration for Rx and Tx. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- drivers/net/phy/dp83869.c | 53 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 50 insertions(+), 3 deletions(-)