Message ID | 20210210210809.30125-8-michael@walle.cc (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | net: phy: icplus: cleanups and new features | expand |
Context | Check | Description |
---|---|---|
netdev/cover_letter | success | Link |
netdev/fixes_present | success | Link |
netdev/patch_count | success | Link |
netdev/tree_selection | success | Clearly marked for net-next |
netdev/subject_prefix | success | Link |
netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
netdev/source_inline | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Link |
netdev/module_param | success | Was 0 now: 0 |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/verify_fixes | success | Link |
netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 131 lines checked |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/header_inline | success | Link |
netdev/stable | success | Stable not CCed |
Hi Michael,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Michael-Walle/net-phy-icplus-cleanups-and-new-features/20210211-051702
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git de1db4a6ed6241e34cab0e5059d4b56f6bae39b9
config: powerpc64-randconfig-r004-20210211 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c9439ca36342fb6013187d0a69aef92736951476)
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 powerpc64 cross compiling tool for clang build
# apt-get install binutils-powerpc64-linux-gnu
# https://github.com/0day-ci/linux/commit/07b7c444040f9baff7b28415b4f26be7e7a71e2e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Michael-Walle/net-phy-icplus-cleanups-and-new-features/20210211-051702
git checkout 07b7c444040f9baff7b28415b4f26be7e7a71e2e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64
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 >>):
__do_insb
^
arch/powerpc/include/asm/io.h:556:56: note: expanded from macro '__do_insb'
#define __do_insb(p, b, n) readsb((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/net/phy/icplus.c:11:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:10:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:619:
arch/powerpc/include/asm/io-defs.h:45:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(insw, (unsigned long p, void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:139:1: note: expanded from here
__do_insw
^
arch/powerpc/include/asm/io.h:557:56: note: expanded from macro '__do_insw'
#define __do_insw(p, b, n) readsw((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/net/phy/icplus.c:11:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:10:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:619:
arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:141:1: note: expanded from here
__do_insl
^
arch/powerpc/include/asm/io.h:558:56: note: expanded from macro '__do_insl'
#define __do_insl(p, b, n) readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/net/phy/icplus.c:11:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:10:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:619:
arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:143:1: note: expanded from here
__do_outsb
^
arch/powerpc/include/asm/io.h:559:58: note: expanded from macro '__do_outsb'
#define __do_outsb(p, b, n) writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/net/phy/icplus.c:11:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:10:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:619:
arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:145:1: note: expanded from here
__do_outsw
^
arch/powerpc/include/asm/io.h:560:58: note: expanded from macro '__do_outsw'
#define __do_outsw(p, b, n) writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
~~~~~~~~~~~~~~~~~~~~~^
In file included from drivers/net/phy/icplus.c:11:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:10:
In file included from arch/powerpc/include/asm/hardirq.h:6:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/powerpc/include/asm/io.h:619:
arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c),
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
__do_##name al; \
^~~~~~~~~~~~~~
<scratch space>:147:1: note: expanded from here
__do_outsl
^
arch/powerpc/include/asm/io.h:561:58: note: expanded from macro '__do_outsl'
#define __do_outsl(p, b, n) writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
~~~~~~~~~~~~~~~~~~~~~^
>> drivers/net/phy/icplus.c:237:2: warning: variable 'err' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
default:
^~~~~~~
drivers/net/phy/icplus.c:249:43: note: uninitialized use occurs here
return phy_restore_page(phydev, oldpage, err);
^~~
drivers/net/phy/icplus.c:216:18: note: initialize the variable 'err' to silence this warning
int oldpage, err;
^
= 0
7 warnings generated.
vim +/err +237 drivers/net/phy/icplus.c
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 212
56ff94ca1f47d1 Michael Walle 2021-02-10 213 static int ip101a_g_config_intr_pin(struct phy_device *phydev)
034289b2d7cf29 Martin Blumenstingl 2018-11-18 214 {
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 215 struct ip101a_g_phy_priv *priv = phydev->priv;
07b7c444040f9b Michael Walle 2021-02-10 216 int oldpage, err;
07b7c444040f9b Michael Walle 2021-02-10 217
07b7c444040f9b Michael Walle 2021-02-10 218 oldpage = phy_select_page(phydev, IP101G_DEFAULT_PAGE);
034289b2d7cf29 Martin Blumenstingl 2018-11-18 219
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 220 /* configure the RXER/INTR_32 pin of the 32-pin IP101GR if needed: */
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 221 switch (priv->sel_intr32) {
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 222 case IP101GR_SEL_INTR32_RXER:
07b7c444040f9b Michael Walle 2021-02-10 223 err = __phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL,
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 224 IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, 0);
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 225 if (err < 0)
07b7c444040f9b Michael Walle 2021-02-10 226 goto out;
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 227 break;
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 228
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 229 case IP101GR_SEL_INTR32_INTR:
07b7c444040f9b Michael Walle 2021-02-10 230 err = __phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL,
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 231 IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32,
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 232 IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32);
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 233 if (err < 0)
07b7c444040f9b Michael Walle 2021-02-10 234 goto out;
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 235 break;
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 236
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 @237 default:
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 238 /* Don't touch IP101G_DIGITAL_IO_SPEC_CTRL because it's not
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 239 * documented on IP101A and it's not clear whether this would
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 240 * cause problems.
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 241 * For the 32-pin IP101GR we simply keep the SEL_INTR32
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 242 * configuration as set by the bootloader when not configured
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 243 * to one of the special functions.
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 244 */
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 245 break;
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 246 }
f2f1a847e74f61 Martin Blumenstingl 2018-11-18 247
07b7c444040f9b Michael Walle 2021-02-10 248 out:
07b7c444040f9b Michael Walle 2021-02-10 249 return phy_restore_page(phydev, oldpage, err);
56ff94ca1f47d1 Michael Walle 2021-02-10 250 }
56ff94ca1f47d1 Michael Walle 2021-02-10 251
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Am 2021-02-10 22:08, schrieb Michael Walle: > Registers >= 16 are paged. Be sure to set the page. It seems this was > working for now, because the default is correct for the registers used > in the driver at the moment. But this will also assume, nobody will > change the page select register before linux is started. The page > select > register is _not_ reset with a soft reset of the PHY. > > To ease the function reuse between the non-paged register space of the > IP101A and the IP101G, add noop read_page()/write_page() callbacks so > the IP101G functions can also be used for the IP101A. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > Changes since v2: > - none > > Changes since v1: > - introduce a noop read/write_page() for the IP101A > - also use phy_*_paged() for the interrupt status register > > Andrew, I've dropped your Reviewed-by because of this. > > drivers/net/phy/icplus.c | 65 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 52 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c > index bc2b58061507..7e0ef05b1cae 100644 > --- a/drivers/net/phy/icplus.c > +++ b/drivers/net/phy/icplus.c > @@ -49,6 +49,8 @@ MODULE_LICENSE("GPL"); > #define IP101G_DIGITAL_IO_SPEC_CTRL 0x1d > #define IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32 BIT(2) > > +#define IP101G_DEFAULT_PAGE 16 > + > #define IP175C_PHY_ID 0x02430d80 > #define IP1001_PHY_ID 0x02430d90 > #define IP101A_PHY_ID 0x02430c54 > @@ -211,23 +213,25 @@ static int ip101a_g_probe(struct phy_device > *phydev) > static int ip101a_g_config_intr_pin(struct phy_device *phydev) > { > struct ip101a_g_phy_priv *priv = phydev->priv; > - int err; > + int oldpage, err; besides this being uninitialized > + > + oldpage = phy_select_page(phydev, IP101G_DEFAULT_PAGE); this is also missing a check for negative return values will be fixed in v4 -michael > > /* configure the RXER/INTR_32 pin of the 32-pin IP101GR if needed: */ > switch (priv->sel_intr32) { > case IP101GR_SEL_INTR32_RXER: > - err = phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL, > - IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, 0); > + err = __phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL, > + IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, 0); > if (err < 0) > - return err; > + goto out; > break; > > case IP101GR_SEL_INTR32_INTR: > - err = phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL, > - IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, > - IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32); > + err = __phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL, > + IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, > + IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32); > if (err < 0) > - return err; > + goto out; > break; > > default: > @@ -241,7 +245,8 @@ static int ip101a_g_config_intr_pin(struct > phy_device *phydev) > break; > } > > - return 0; > +out: > + return phy_restore_page(phydev, oldpage, err); > } > > static int ip101a_config_init(struct phy_device *phydev) > @@ -263,8 +268,10 @@ static int ip101g_config_init(struct phy_device > *phydev) > > static int ip101a_g_ack_interrupt(struct phy_device *phydev) > { > - int err = phy_read(phydev, IP101A_G_IRQ_CONF_STATUS); > + int err; > > + err = phy_read_paged(phydev, IP101G_DEFAULT_PAGE, > + IP101A_G_IRQ_CONF_STATUS); > if (err < 0) > return err; > > @@ -283,10 +290,12 @@ static int ip101a_g_config_intr(struct phy_device > *phydev) > > /* INTR pin used: Speed/link/duplex will cause an interrupt */ > val = IP101A_G_IRQ_PIN_USED; > - err = phy_write(phydev, IP101A_G_IRQ_CONF_STATUS, val); > + err = phy_write_paged(phydev, IP101G_DEFAULT_PAGE, > + IP101A_G_IRQ_CONF_STATUS, val); > } else { > val = IP101A_G_IRQ_ALL_MASK; > - err = phy_write(phydev, IP101A_G_IRQ_CONF_STATUS, val); > + err = phy_write_paged(phydev, IP101G_DEFAULT_PAGE, > + IP101A_G_IRQ_CONF_STATUS, val); > if (err) > return err; > > @@ -300,7 +309,8 @@ static irqreturn_t > ip101a_g_handle_interrupt(struct phy_device *phydev) > { > int irq_status; > > - irq_status = phy_read(phydev, IP101A_G_IRQ_CONF_STATUS); > + irq_status = phy_read_paged(phydev, IP101G_DEFAULT_PAGE, > + IP101A_G_IRQ_CONF_STATUS); > if (irq_status < 0) { > phy_error(phydev); > return IRQ_NONE; > @@ -316,6 +326,31 @@ static irqreturn_t > ip101a_g_handle_interrupt(struct phy_device *phydev) > return IRQ_HANDLED; > } > > +/* The IP101A doesn't really have a page register. We just pretend to > have one > + * so we can use the paged versions of the callbacks of the IP101G. > + */ > +static int ip101a_read_page(struct phy_device *phydev) > +{ > + return IP101G_DEFAULT_PAGE; > +} > + > +static int ip101a_write_page(struct phy_device *phydev, int page) > +{ > + WARN_ONCE(page != IP101G_DEFAULT_PAGE, "wrong page selected\n"); > + > + return 0; > +} > + > +static int ip101g_read_page(struct phy_device *phydev) > +{ > + return __phy_read(phydev, IP101G_PAGE_CONTROL); > +} > + > +static int ip101g_write_page(struct phy_device *phydev, int page) > +{ > + return __phy_write(phydev, IP101G_PAGE_CONTROL, page); > +} > + > static int ip101a_g_has_page_register(struct phy_device *phydev) > { > int oldval, val, ret; > @@ -390,6 +425,8 @@ static struct phy_driver icplus_driver[] = { > .name = "ICPlus IP101A", > .match_phy_device = ip101a_match_phy_device, > .probe = ip101a_g_probe, > + .read_page = ip101a_read_page, > + .write_page = ip101a_write_page, > .config_intr = ip101a_g_config_intr, > .handle_interrupt = ip101a_g_handle_interrupt, > .config_init = ip101a_config_init, > @@ -400,6 +437,8 @@ static struct phy_driver icplus_driver[] = { > .name = "ICPlus IP101G", > .match_phy_device = ip101g_match_phy_device, > .probe = ip101a_g_probe, > + .read_page = ip101g_read_page, > + .write_page = ip101g_write_page, > .config_intr = ip101a_g_config_intr, > .handle_interrupt = ip101a_g_handle_interrupt, > .config_init = ip101g_config_init,
diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c index bc2b58061507..7e0ef05b1cae 100644 --- a/drivers/net/phy/icplus.c +++ b/drivers/net/phy/icplus.c @@ -49,6 +49,8 @@ MODULE_LICENSE("GPL"); #define IP101G_DIGITAL_IO_SPEC_CTRL 0x1d #define IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32 BIT(2) +#define IP101G_DEFAULT_PAGE 16 + #define IP175C_PHY_ID 0x02430d80 #define IP1001_PHY_ID 0x02430d90 #define IP101A_PHY_ID 0x02430c54 @@ -211,23 +213,25 @@ static int ip101a_g_probe(struct phy_device *phydev) static int ip101a_g_config_intr_pin(struct phy_device *phydev) { struct ip101a_g_phy_priv *priv = phydev->priv; - int err; + int oldpage, err; + + oldpage = phy_select_page(phydev, IP101G_DEFAULT_PAGE); /* configure the RXER/INTR_32 pin of the 32-pin IP101GR if needed: */ switch (priv->sel_intr32) { case IP101GR_SEL_INTR32_RXER: - err = phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL, - IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, 0); + err = __phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL, + IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, 0); if (err < 0) - return err; + goto out; break; case IP101GR_SEL_INTR32_INTR: - err = phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL, - IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, - IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32); + err = __phy_modify(phydev, IP101G_DIGITAL_IO_SPEC_CTRL, + IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32, + IP101G_DIGITAL_IO_SPEC_CTRL_SEL_INTR32); if (err < 0) - return err; + goto out; break; default: @@ -241,7 +245,8 @@ static int ip101a_g_config_intr_pin(struct phy_device *phydev) break; } - return 0; +out: + return phy_restore_page(phydev, oldpage, err); } static int ip101a_config_init(struct phy_device *phydev) @@ -263,8 +268,10 @@ static int ip101g_config_init(struct phy_device *phydev) static int ip101a_g_ack_interrupt(struct phy_device *phydev) { - int err = phy_read(phydev, IP101A_G_IRQ_CONF_STATUS); + int err; + err = phy_read_paged(phydev, IP101G_DEFAULT_PAGE, + IP101A_G_IRQ_CONF_STATUS); if (err < 0) return err; @@ -283,10 +290,12 @@ static int ip101a_g_config_intr(struct phy_device *phydev) /* INTR pin used: Speed/link/duplex will cause an interrupt */ val = IP101A_G_IRQ_PIN_USED; - err = phy_write(phydev, IP101A_G_IRQ_CONF_STATUS, val); + err = phy_write_paged(phydev, IP101G_DEFAULT_PAGE, + IP101A_G_IRQ_CONF_STATUS, val); } else { val = IP101A_G_IRQ_ALL_MASK; - err = phy_write(phydev, IP101A_G_IRQ_CONF_STATUS, val); + err = phy_write_paged(phydev, IP101G_DEFAULT_PAGE, + IP101A_G_IRQ_CONF_STATUS, val); if (err) return err; @@ -300,7 +309,8 @@ static irqreturn_t ip101a_g_handle_interrupt(struct phy_device *phydev) { int irq_status; - irq_status = phy_read(phydev, IP101A_G_IRQ_CONF_STATUS); + irq_status = phy_read_paged(phydev, IP101G_DEFAULT_PAGE, + IP101A_G_IRQ_CONF_STATUS); if (irq_status < 0) { phy_error(phydev); return IRQ_NONE; @@ -316,6 +326,31 @@ static irqreturn_t ip101a_g_handle_interrupt(struct phy_device *phydev) return IRQ_HANDLED; } +/* The IP101A doesn't really have a page register. We just pretend to have one + * so we can use the paged versions of the callbacks of the IP101G. + */ +static int ip101a_read_page(struct phy_device *phydev) +{ + return IP101G_DEFAULT_PAGE; +} + +static int ip101a_write_page(struct phy_device *phydev, int page) +{ + WARN_ONCE(page != IP101G_DEFAULT_PAGE, "wrong page selected\n"); + + return 0; +} + +static int ip101g_read_page(struct phy_device *phydev) +{ + return __phy_read(phydev, IP101G_PAGE_CONTROL); +} + +static int ip101g_write_page(struct phy_device *phydev, int page) +{ + return __phy_write(phydev, IP101G_PAGE_CONTROL, page); +} + static int ip101a_g_has_page_register(struct phy_device *phydev) { int oldval, val, ret; @@ -390,6 +425,8 @@ static struct phy_driver icplus_driver[] = { .name = "ICPlus IP101A", .match_phy_device = ip101a_match_phy_device, .probe = ip101a_g_probe, + .read_page = ip101a_read_page, + .write_page = ip101a_write_page, .config_intr = ip101a_g_config_intr, .handle_interrupt = ip101a_g_handle_interrupt, .config_init = ip101a_config_init, @@ -400,6 +437,8 @@ static struct phy_driver icplus_driver[] = { .name = "ICPlus IP101G", .match_phy_device = ip101g_match_phy_device, .probe = ip101a_g_probe, + .read_page = ip101g_read_page, + .write_page = ip101g_write_page, .config_intr = ip101a_g_config_intr, .handle_interrupt = ip101a_g_handle_interrupt, .config_init = ip101g_config_init,
Registers >= 16 are paged. Be sure to set the page. It seems this was working for now, because the default is correct for the registers used in the driver at the moment. But this will also assume, nobody will change the page select register before linux is started. The page select register is _not_ reset with a soft reset of the PHY. To ease the function reuse between the non-paged register space of the IP101A and the IP101G, add noop read_page()/write_page() callbacks so the IP101G functions can also be used for the IP101A. Signed-off-by: Michael Walle <michael@walle.cc> --- Changes since v2: - none Changes since v1: - introduce a noop read/write_page() for the IP101A - also use phy_*_paged() for the interrupt status register Andrew, I've dropped your Reviewed-by because of this. drivers/net/phy/icplus.c | 65 ++++++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 13 deletions(-)