diff mbox series

[RFC/RFT,net-next,2/4] net: dsa: microchip: partial conversion to regfields API for KSZ8795 (WIP)

Message ID 20230316161250.3286055-3-vladimir.oltean@nxp.com
State New
Headers
Series KSZ DSA driver: xMII speed adjustment and partial reg_fields conversion |

Commit Message

Vladimir Oltean March 16, 2023, 4:12 p.m. UTC 
  During review, Russell King has pointed out that the current way in
which the KSZ common driver performs register access is error-prone.

Based on the KSZ9477 software model where we have the XMII Port Control
0 Register (at offset 0xN300) and XMII Port Control 1 Register
(at offset 0xN301), this driver also holds P_XMII_CTRL_0 and P_XMII_CTRL_1.

However, on some switches, fields accessed through P_XMII_CTRL_0 (for
example P_MII_100MBIT_M or P_MII_DUPLEX_M) and fields accessed through
P_XMII_CTRL_1 (for example P_MII_SEL_M) may end up accessing the same
hardware register. Or at least, that was certainly the impression after
commit
https://patchwork.kernel.org/project/netdevbpf/patch/20230315231916.2998480-1-vladimir.oltean@nxp.com/
(sorry, can't reference the sha1sum of an unmerged commit), because for
ksz8795_regs[], P_XMII_CTRL_0 and P_XMII_CTRL_1 now have the same value.

But the reality is far more interesting. Opening a KSZ8795 datasheet, I
see that out of the register fields accessed via P_XMII_CTRL_0:
- what the driver names P_MII_SEL_M *is* actually "GMII/MII Mode Select"
  (bit 2) of the Port 5 Interface Control 6, address 0x56 (all good here)
- what the driver names P_MII_100MBIT_M is actually "Switch SW5-MII/RMII
  Speed" (bit 4) of the Global Control 4 register, address 0x06.

That is a huge problem, because the driver cannot access this register
for KSZ8795 in its current form, even if that register exists. This
creates an even stronger motivation to try to do something to normalize
the way in which this driver abstracts away register field movement from
one switch family to another.

As I had proposed in that thread, reg_fields from regmap propose to
solve exactly this problem. This patch contains a COMPLETELY UNTESTED
rework of the driver, so that accesses done through the following
registers (for demonstration purposes):
- REG_IND_BYTE - a global register
- REG_IND_CTRL_0 - another global register
- P_LOCAL_CTRL - a port register
- P_FORCE_CTRL - another port register
- P_XMII_CTRL_0 and P_XMII_CTRL_1 - either port register, or global
  registers, depending on which manual you read!

are converted to the regfields API.

!! WARNING !! I only attempted to add a ksz_reg_fields structure for
KSZ8795. The other switch families will currently crash!

For easier partial migration, I have renamed the "REG_" or "P_" prefixes
of the enum ksz_regs values into a common "RF_" (for reg field) prefix
for a new enum type: ksz_rf. Eventually, enum ksz_regs, as well as the
masks, should disappear completely, being replaced by reg fields.

Link: https://lore.kernel.org/netdev/Y%2FYPfxg8Ackb8zmW@shell.armlinux.org.uk/
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/microchip/ksz8795.c     |  37 ++---
 drivers/net/dsa/microchip/ksz8863_smi.c |   9 +
 drivers/net/dsa/microchip/ksz9477_i2c.c |   9 +
 drivers/net/dsa/microchip/ksz_common.c  | 209 ++++++++++++++++--------
 drivers/net/dsa/microchip/ksz_common.h  |  49 ++++++
 drivers/net/dsa/microchip/ksz_spi.c     |   4 +
 6 files changed, 227 insertions(+), 90 deletions(-)

Comments

Vladimir Oltean March 17, 2023, 9:46 a.m. UTC | #1 
On Thu, Mar 16, 2023 at 06:12:48PM +0200, Vladimir Oltean wrote:
> During review, Russell King has pointed out that the current way in
> which the KSZ common driver performs register access is error-prone.
> 
> Based on the KSZ9477 software model where we have the XMII Port Control
> 0 Register (at offset 0xN300) and XMII Port Control 1 Register
> (at offset 0xN301), this driver also holds P_XMII_CTRL_0 and P_XMII_CTRL_1.
> 
> However, on some switches, fields accessed through P_XMII_CTRL_0 (for
> example P_MII_100MBIT_M or P_MII_DUPLEX_M) and fields accessed through
> P_XMII_CTRL_1 (for example P_MII_SEL_M) may end up accessing the same
> hardware register. Or at least, that was certainly the impression after
> commit
> https://patchwork.kernel.org/project/netdevbpf/patch/20230315231916.2998480-1-vladimir.oltean@nxp.com/
> (sorry, can't reference the sha1sum of an unmerged commit), because for
> ksz8795_regs[], P_XMII_CTRL_0 and P_XMII_CTRL_1 now have the same value.
> 
> But the reality is far more interesting. Opening a KSZ8795 datasheet, I
> see that out of the register fields accessed via P_XMII_CTRL_0:
> - what the driver names P_MII_SEL_M *is* actually "GMII/MII Mode Select"
>   (bit 2) of the Port 5 Interface Control 6, address 0x56 (all good here)
> - what the driver names P_MII_100MBIT_M is actually "Switch SW5-MII/RMII
>   Speed" (bit 4) of the Global Control 4 register, address 0x06.
> 
> That is a huge problem, because the driver cannot access this register
> for KSZ8795 in its current form, even if that register exists. This
> creates an even stronger motivation to try to do something to normalize
> the way in which this driver abstracts away register field movement from
> one switch family to another.
> 
> As I had proposed in that thread, reg_fields from regmap propose to
> solve exactly this problem. This patch contains a COMPLETELY UNTESTED
> rework of the driver, so that accesses done through the following
> registers (for demonstration purposes):
> - REG_IND_BYTE - a global register
> - REG_IND_CTRL_0 - another global register
> - P_LOCAL_CTRL - a port register
> - P_FORCE_CTRL - another port register
> - P_XMII_CTRL_0 and P_XMII_CTRL_1 - either port register, or global
>   registers, depending on which manual you read!
> 
> are converted to the regfields API.
> 
> !! WARNING !! I only attempted to add a ksz_reg_fields structure for
> KSZ8795. The other switch families will currently crash!
> 
> For easier partial migration, I have renamed the "REG_" or "P_" prefixes
> of the enum ksz_regs values into a common "RF_" (for reg field) prefix
> for a new enum type: ksz_rf. Eventually, enum ksz_regs, as well as the
> masks, should disappear completely, being replaced by reg fields.
> 
> Link: https://lore.kernel.org/netdev/Y%2FYPfxg8Ackb8zmW@shell.armlinux.org.uk/
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c
> index 5871f27451cb..f9d22a444146 100644
> --- a/drivers/net/dsa/microchip/ksz8863_smi.c
> +++ b/drivers/net/dsa/microchip/ksz8863_smi.c
> @@ -136,11 +136,16 @@ static const struct regmap_config ksz8863_regmap_config[] = {
>  
>  static int ksz8863_smi_probe(struct mdio_device *mdiodev)
>  {
> +	const struct ksz_chip_data *chip;
>  	struct regmap_config rc;
>  	struct ksz_device *dev;
>  	int ret;
>  	int i;
>  
> +	chip = device_get_match_data(ddev);

s/ddev/&mdiodev->dev/

> +	if (!chip)
> +		return -EINVAL;
> +
>  	dev = ksz_switch_alloc(&mdiodev->dev, mdiodev);
>  	if (!dev)
>  		return -ENOMEM;
> @@ -159,6 +164,10 @@ static int ksz8863_smi_probe(struct mdio_device *mdiodev)
>  		}
>  	}
>  
> +	ret = ksz_regfields_init(dev, chip);
> +	if (ret)
> +		return ret;
> +
>  	if (mdiodev->dev.platform_data)
>  		dev->pdata = mdiodev->dev.platform_data;
>  
> diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c
> index 497be833f707..2cbd76aed974 100644
> --- a/drivers/net/dsa/microchip/ksz9477_i2c.c
> +++ b/drivers/net/dsa/microchip/ksz9477_i2c.c
> @@ -16,10 +16,15 @@ KSZ_REGMAP_TABLE(ksz9477, not_used, 16, 0, 0);
>  
>  static int ksz9477_i2c_probe(struct i2c_client *i2c)
>  {
> +	const struct ksz_chip_data *chip;
>  	struct regmap_config rc;
>  	struct ksz_device *dev;
>  	int i, ret;
>  
> +	chip = device_get_match_data(ddev);

s/ddev/&i2c->dev/

> +	if (!chip)
> +		return -EINVAL;
> +
>  	dev = ksz_switch_alloc(&i2c->dev, i2c);
>  	if (!dev)
>  		return -ENOMEM;
> @@ -35,6 +40,10 @@ static int ksz9477_i2c_probe(struct i2c_client *i2c)
>  		}
>  	}
>  
> +	ret = ksz_regfields_init(dev, chip);
> +	if (ret)
> +		return ret;
> +
>  	if (i2c->dev.platform_data)
>  		dev->pdata = i2c->dev.platform_data;
>
Oleksij Rempel March 17, 2023, 11:46 a.m. UTC | #2 
On Fri, Mar 17, 2023 at 11:46:29AM +0200, Vladimir Oltean wrote:
> > +++ b/drivers/net/dsa/microchip/ksz8863_smi.c
> > @@ -136,11 +136,16 @@ static const struct regmap_config ksz8863_regmap_config[] = {
> >  
> >  static int ksz8863_smi_probe(struct mdio_device *mdiodev)
> >  {
> > +	const struct ksz_chip_data *chip;
> >  	struct regmap_config rc;
> >  	struct ksz_device *dev;
> >  	int ret;
> >  	int i;
> >  
> > +	chip = device_get_match_data(ddev);
> 
> s/ddev/&mdiodev->dev/

It fails on ksz8873 switch with following trace:

[    2.490822] 8<--- cut here ---
[    2.493907] Unable to handle kernel NULL pointer dereference at virtual address 00000004 when read
[    2.502924] [00000004] *pgd=00000000
[    2.506536] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[    2.511864] Modules linked in:
[    2.514935] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.3.0-rc1-00519-gd11a10757686-dirty #263
[    2.523562] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[    2.530100] PC is at ksz_regfields_init+0x44/0xa0
[    2.534833] LR is at _raw_spin_unlock_irqrestore+0x44/0x68
[    2.540336] pc : [<8075384c>]    lr : [<80ca2ca0>]    psr: a0000013
[    2.546614] sp : a0835c30  ip : a0835bd0  fp : a0835c5c
[    2.551848] r10: 80dacf48  r9 : 80dacdc0  r8 : 821d82ac
[    2.557083] r7 : 80dad00c  r6 : 00000000  r5 : 821d8240  r4 : 00000000
[    2.563622] r3 : 00000000  r2 : 00000000  r1 : 1ed17000  r0 : 821dac40
...
[    2.936367] Backtrace:                                                                    
[    2.938826]  ksz_regfields_init from ksz8863_smi_probe+0xfc/0x134
[    2.944962]  r6:00000003 r5:820bc800 r4:821d8240
[    2.949588]  ksz8863_smi_probe from mdio_probe+0x38/0x5c
[    2.954948]  r10:811495b8 r9:821da338 r8:00000000 r7:814dbbb8 r6:81468944 r5:820bc800
[    2.962786]  r4:81468944
[    2.965327]  mdio_probe from really_probe+0x1ac/0x3c8
[    2.970414]  r5:00000000 r4:820bc800                                                      
[    2.973997]  really_probe from __driver_probe_device+0x1d0/0x204
[    2.980036]  r8:814cc580 r7:814dbbb8 r6:820bc800 r5:81468944 r4:820bc800
[    2.986744]  __driver_probe_device from driver_probe_device+0x4c/0xc8
[    2.993217]  r9:821da338 r8:814cc580 r7:00000000 r6:820bc800 r5:81468944 r4:8154f4d4
[    3.000967]  driver_probe_device from __driver_attach+0x158/0x17c
[    3.007092]  r7:8067f830 r6:81468944 r5:81468944 r4:820bc800
[    3.012758]  __driver_attach from bus_for_each_dev+0x88/0xc8
[    3.018449]  r7:8067f830 r6:81468944 r5:81b25400 r4:821d0634
[    3.024116]  bus_for_each_dev from driver_attach+0x28/0x30
[    3.029631]  r7:00000000 r6:81b25400 r5:821da300 r4:81468944
[    3.035296]  driver_attach from bus_add_driver+0xdc/0x1f8
[    3.040719]  bus_add_driver from driver_register+0xc8/0x110
[    3.046326]  r9:814cc580 r8:814cc580 r7:00000000 r6:00000006 r5:81235290 r4:81468944
[    3.054076]  driver_register from mdio_driver_register+0x60/0xa0
[    3.060117]  r5:81235290 r4:81468944
[    3.063699]  mdio_driver_register from mdio_module_init+0x1c/0x24
[    3.069827]  r5:81235290 r4:81968940
[    3.073409]  mdio_module_init from do_one_initcall+0xac/0x214
[    3.079183]  do_one_initcall from kernel_init_freeable+0x1f8/0x244
[    3.085399]  r8:8124b858 r7:8124b834 r6:00000006 r5:00000075 r4:8199d580
[    3.092108]  kernel_init_freeable from kernel_init+0x24/0x140
[    3.097888]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:80c9d3a8
[    3.105726]  r4:81304f00
[    3.108268]  kernel_init from ret_from_fork+0x14/0x2c
[    3.113344] Exception stack(0xa0835fb0 to 0xa0835ff8)
...

There reason is that ksz8795_regfields[] is assigned only to KSZ8795.
KSZ8794, KSZ8765 and KSZ8830 (KSZ8863/KSZ8873) do not have needed regfields.

Please note, ksz8795_regfields[] is not compatible with KSZ8830 (KSZ8863/KSZ8873)
series.
Vladimir Oltean March 17, 2023, 12:50 p.m. UTC | #3 
On Fri, Mar 17, 2023 at 12:46:46PM +0100, Oleksij Rempel wrote:
> There reason is that ksz8795_regfields[] is assigned only to KSZ8795.
> KSZ8794, KSZ8765 and KSZ8830 (KSZ8863/KSZ8873) do not have needed regfields.
> 
> Please note, ksz8795_regfields[] is not compatible with KSZ8830 (KSZ8863/KSZ8873)
> series.

Right... well, it's kind of in the title and in the commit description:

| !! WARNING !! I only attempted to add a ksz_reg_fields structure for
| KSZ8795. The other switch families will currently crash!

If the only device you can test on is KSZ8873, that isn't going to help
me very much at the moment, because it doesn't have an xMII port, but
rather, either MII or RMII depending on part number. AFAIU, ksz_is_ksz88x3()
returns true for your device, and this means that neither phylink_mac_link_up()
nor phylink_mac_config() do nothing for your device. Also, above all,
ksz8863_regs[] does not have either P_XMII_CTRL_0 nor P_XMII_CTRL_1
defined, which are some of the registers I had converted to reg_fields,
in order to see whether it's possible to access a global register via a
port regfield call.

I'm going to let this patch set simmer for a few more days. If no one
volunteers to test on a KSZ8795, IMO the exercise is slightly pointless,
as that's where the problems were, and more and more blind reasoning
about what could be a problem isn't going to get us very far. I'd rather
not spend more time on this problem at this stage. I've copied some more
people who contributed patches to this switch family in the past few
years, in the hope that maybe someone can help.

For context, the cover letter is here:
https://patchwork.kernel.org/project/netdevbpf/cover/20230316161250.3286055-1-vladimir.oltean@nxp.com/
Oleksij Rempel March 17, 2023, 1:21 p.m. UTC | #4 
On Fri, Mar 17, 2023 at 02:50:22PM +0200, Vladimir Oltean wrote:
> On Fri, Mar 17, 2023 at 12:46:46PM +0100, Oleksij Rempel wrote:
> > There reason is that ksz8795_regfields[] is assigned only to KSZ8795.
> > KSZ8794, KSZ8765 and KSZ8830 (KSZ8863/KSZ8873) do not have needed regfields.
> > 
> > Please note, ksz8795_regfields[] is not compatible with KSZ8830 (KSZ8863/KSZ8873)
> > series.
> 
> Right... well, it's kind of in the title and in the commit description:
> 
> | !! WARNING !! I only attempted to add a ksz_reg_fields structure for
> | KSZ8795. The other switch families will currently crash!
> 
> If the only device you can test on is KSZ8873, that isn't going to help
> me very much at the moment, because it doesn't have an xMII port, but
> rather, either MII or RMII depending on part number. AFAIU, ksz_is_ksz88x3()
> returns true for your device, and this means that neither phylink_mac_link_up()
> nor phylink_mac_config() do nothing for your device. Also, above all,
> ksz8863_regs[] does not have either P_XMII_CTRL_0 nor P_XMII_CTRL_1
> defined, which are some of the registers I had converted to reg_fields,
> in order to see whether it's possible to access a global register via a
> port regfield call.
> 
> I'm going to let this patch set simmer for a few more days. If no one
> volunteers to test on a KSZ8795, IMO the exercise is slightly pointless,
> as that's where the problems were, and more and more blind reasoning
> about what could be a problem isn't going to get us very far. I'd rather
> not spend more time on this problem at this stage. I've copied some more
> people who contributed patches to this switch family in the past few
> years, in the hope that maybe someone can help.
> 
> For context, the cover letter is here:
> https://patchwork.kernel.org/project/netdevbpf/cover/20230316161250.3286055-1-vladimir.oltean@nxp.com/

If you'll give up, may be i'll be able to take it over.

Thanks!
Oleksij
Vladimir Oltean March 17, 2023, 2:07 p.m. UTC | #5 
On Fri, Mar 17, 2023 at 02:21:40PM +0100, Oleksij Rempel wrote:
> If you'll give up, may be i'll be able to take it over.

I have not given up yet, I just need someone to run a few tests on KSZ8795.
Vladimir Oltean March 25, 2023, 12:16 p.m. UTC | #6 
On Fri, Mar 17, 2023 at 04:07:22PM +0200, Vladimir Oltean wrote:
> On Fri, Mar 17, 2023 at 02:21:40PM +0100, Oleksij Rempel wrote:
> > If you'll give up, may be i'll be able to take it over.
> 
> I have not given up yet, I just need someone to run a few tests on KSZ8795.

Seeing that the first line of people (able to test on KSZ8795) has no
opinion, maybe it's time to turn to the second line of people.

Russell, you've expressed that this driver is "vile" for permitting
access to the same hardware register through 2 different ksz8795_regs[]
elements. Does the approach used in this patch set address your concerns,
or in other words, do you believe is it worth extending the conversion
to the KSZ switches that other people can test, such that the entire
driver ultimately benefits from the conversion?
Russell King (Oracle) March 25, 2023, 2:26 p.m. UTC | #7 
On Sat, Mar 25, 2023 at 02:16:55PM +0200, Vladimir Oltean wrote:
> Russell, you've expressed that this driver is "vile" for permitting
> access to the same hardware register through 2 different ksz8795_regs[]
> elements. Does the approach used in this patch set address your concerns,
> or in other words, do you believe is it worth extending the conversion
> to the KSZ switches that other people can test, such that the entire
> driver ultimately benefits from the conversion?

Thanks for pointing it out - I'll look at it next week.
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 7f9ab6d13952..1abdc45278ad 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -40,18 +40,15 @@  static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
 
 static int ksz8_ind_write8(struct ksz_device *dev, u8 table, u16 addr, u8 data)
 {
-	const u16 *regs;
 	u16 ctrl_addr;
 	int ret = 0;
 
-	regs = dev->info->regs;
-
 	mutex_lock(&dev->alu_mutex);
 
 	ctrl_addr = IND_ACC_TABLE(table) | addr;
-	ret = ksz_write8(dev, regs[REG_IND_BYTE], data);
+	ret = ksz_regfield_write(dev, RF_IND_BYTE, data);
 	if (!ret)
-		ret = ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+		ret = ksz_regfield_write(dev, RF_IND_CTRL_0, ctrl_addr);
 
 	mutex_unlock(&dev->alu_mutex);
 
@@ -176,7 +173,7 @@  void ksz8_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 *cnt)
 	ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
 
 	mutex_lock(&dev->alu_mutex);
-	ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+	ksz_regfield_write(dev, RF_IND_CTRL_0, ctrl_addr);
 
 	/* It is almost guaranteed to always read the valid bit because of
 	 * slow SPI speed.
@@ -214,7 +211,7 @@  static void ksz8795_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
 	ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
 
 	mutex_lock(&dev->alu_mutex);
-	ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+	ksz_regfield_write(dev, RF_IND_CTRL_0, ctrl_addr);
 
 	/* It is almost guaranteed to always read the valid bit because of
 	 * slow SPI speed.
@@ -265,7 +262,7 @@  static void ksz8863_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
 	ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
 
 	mutex_lock(&dev->alu_mutex);
-	ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+	ksz_regfield_write(dev, RF_IND_CTRL_0, ctrl_addr);
 	ksz_read32(dev, regs[REG_IND_DATA_LO], &data);
 	mutex_unlock(&dev->alu_mutex);
 
@@ -346,7 +343,7 @@  static void ksz8_r_table(struct ksz_device *dev, int table, u16 addr, u64 *data)
 	ctrl_addr = IND_ACC_TABLE(table | TABLE_READ) | addr;
 
 	mutex_lock(&dev->alu_mutex);
-	ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+	ksz_regfield_write(dev, RF_IND_CTRL_0, ctrl_addr);
 	ksz_read64(dev, regs[REG_IND_DATA_HI], data);
 	mutex_unlock(&dev->alu_mutex);
 }
@@ -362,7 +359,7 @@  static void ksz8_w_table(struct ksz_device *dev, int table, u16 addr, u64 data)
 
 	mutex_lock(&dev->alu_mutex);
 	ksz_write64(dev, regs[REG_IND_DATA_HI], data);
-	ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+	ksz_regfield_write(dev, RF_IND_CTRL_0, ctrl_addr);
 	mutex_unlock(&dev->alu_mutex);
 }
 
@@ -412,7 +409,7 @@  int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr, u8 *mac_addr,
 	ctrl_addr = IND_ACC_TABLE(TABLE_DYNAMIC_MAC | TABLE_READ) | addr;
 
 	mutex_lock(&dev->alu_mutex);
-	ksz_write16(dev, regs[REG_IND_CTRL_0], ctrl_addr);
+	ksz_regfield_write(dev, RF_IND_CTRL_0, ctrl_addr);
 
 	rc = ksz8_valid_dyn_entry(dev, &data);
 	if (rc == -EAGAIN) {
@@ -605,8 +602,9 @@  static void ksz8_w_vlan_table(struct ksz_device *dev, u16 vid, u16 vlan)
 
 int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
 {
-	u8 restart, speed, ctrl, link;
+	u8 restart, speed, link;
 	int processed = true;
+	unsigned int ctrl;
 	const u16 *regs;
 	u8 val1, val2;
 	u16 data = 0;
@@ -625,7 +623,7 @@  int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
 		if (ret)
 			return ret;
 
-		ret = ksz_pread8(dev, p, regs[P_FORCE_CTRL], &ctrl);
+		ret = ksz_regfields_read(dev, p, RF_FORCE_CTRL, &ctrl);
 		if (ret)
 			return ret;
 
@@ -682,7 +680,7 @@  int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
 			data = KSZ8795_ID_LO;
 		break;
 	case MII_ADVERTISE:
-		ret = ksz_pread8(dev, p, regs[P_LOCAL_CTRL], &ctrl);
+		ret = ksz_regfields_read(dev, p, RF_LOCAL_CTRL, &ctrl);
 		if (ret)
 			return ret;
 
@@ -759,7 +757,8 @@  int ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
 
 int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
 {
-	u8 restart, speed, ctrl, data;
+	u8 restart, speed, data;
+	unsigned int ctrl;
 	const u16 *regs;
 	u8 p = phy;
 	int ret;
@@ -788,7 +787,7 @@  int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
 				return ret;
 		}
 
-		ret = ksz_pread8(dev, p, regs[P_FORCE_CTRL], &ctrl);
+		ret = ksz_regfields_read(dev, p, RF_FORCE_CTRL, &ctrl);
 		if (ret)
 			return ret;
 
@@ -819,7 +818,7 @@  int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
 			data &= ~PORT_FORCE_FULL_DUPLEX;
 
 		if (data != ctrl) {
-			ret = ksz_pwrite8(dev, p, regs[P_FORCE_CTRL], data);
+			ret = ksz_regfields_write(dev, p, RF_FORCE_CTRL, data);
 			if (ret)
 				return ret;
 		}
@@ -866,7 +865,7 @@  int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
 		}
 		break;
 	case MII_ADVERTISE:
-		ret = ksz_pread8(dev, p, regs[P_LOCAL_CTRL], &ctrl);
+		ret = ksz_regfields_read(dev, p, RF_LOCAL_CTRL, &ctrl);
 		if (ret)
 			return ret;
 
@@ -888,7 +887,7 @@  int ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
 			data |= PORT_AUTO_NEG_10BT;
 
 		if (data != ctrl) {
-			ret = ksz_pwrite8(dev, p, regs[P_LOCAL_CTRL], data);
+			ret = ksz_regfields_write(dev, p, RF_LOCAL_CTRL, data);
 			if (ret)
 				return ret;
 		}
diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c
index 5871f27451cb..f9d22a444146 100644
--- a/drivers/net/dsa/microchip/ksz8863_smi.c
+++ b/drivers/net/dsa/microchip/ksz8863_smi.c
@@ -136,11 +136,16 @@  static const struct regmap_config ksz8863_regmap_config[] = {
 
 static int ksz8863_smi_probe(struct mdio_device *mdiodev)
 {
+	const struct ksz_chip_data *chip;
 	struct regmap_config rc;
 	struct ksz_device *dev;
 	int ret;
 	int i;
 
+	chip = device_get_match_data(ddev);
+	if (!chip)
+		return -EINVAL;
+
 	dev = ksz_switch_alloc(&mdiodev->dev, mdiodev);
 	if (!dev)
 		return -ENOMEM;
@@ -159,6 +164,10 @@  static int ksz8863_smi_probe(struct mdio_device *mdiodev)
 		}
 	}
 
+	ret = ksz_regfields_init(dev, chip);
+	if (ret)
+		return ret;
+
 	if (mdiodev->dev.platform_data)
 		dev->pdata = mdiodev->dev.platform_data;
 
diff --git a/drivers/net/dsa/microchip/ksz9477_i2c.c b/drivers/net/dsa/microchip/ksz9477_i2c.c
index 497be833f707..2cbd76aed974 100644
--- a/drivers/net/dsa/microchip/ksz9477_i2c.c
+++ b/drivers/net/dsa/microchip/ksz9477_i2c.c
@@ -16,10 +16,15 @@  KSZ_REGMAP_TABLE(ksz9477, not_used, 16, 0, 0);
 
 static int ksz9477_i2c_probe(struct i2c_client *i2c)
 {
+	const struct ksz_chip_data *chip;
 	struct regmap_config rc;
 	struct ksz_device *dev;
 	int i, ret;
 
+	chip = device_get_match_data(ddev);
+	if (!chip)
+		return -EINVAL;
+
 	dev = ksz_switch_alloc(&i2c->dev, i2c);
 	if (!dev)
 		return -ENOMEM;
@@ -35,6 +40,10 @@  static int ksz9477_i2c_probe(struct i2c_client *i2c)
 		}
 	}
 
+	ret = ksz_regfields_init(dev, chip);
+	if (ret)
+		return ret;
+
 	if (i2c->dev.platform_data)
 		dev->pdata = i2c->dev.platform_data;
 
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 8617aeaa0248..5bcbea8d9151 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -295,6 +295,65 @@  static const struct ksz_dev_ops lan937x_dev_ops = {
 	.exit = lan937x_switch_exit,
 };
 
+#define KSZ8_PORT_BASE		0x10
+#define KSZ8_PORT_SPACING	0x10
+#define KSZ9477_PORT_BASE	0x1000
+#define KSZ9477_PORT_SPACING	0x1000
+
+#define KSZ_REG_FIELD_8(_reg, _lsb, _msb) \
+	{ .width = KSZ_REGMAP_8, .regfield = REG_FIELD(_reg, _lsb, _msb) }
+#define KSZ_REG_FIELD_16(_reg, _lsb, _msb) \
+	{ .width = KSZ_REGMAP_16, .regfield = REG_FIELD(_reg, _lsb, _msb) }
+#define KSZ_REG_FIELD_32(_reg, _lsb, _msb) \
+	{ .width = KSZ_REGMAP_32, .regfield = REG_FIELD(_reg, _lsb, _msb) }
+#define KSZ8_PORT_REG_FIELD_8(_reg, _lsb, _msb) \
+	{ \
+		.width = KSZ_REGMAP_8, \
+		.regfield = REG_FIELD_ID(KSZ8_PORT_BASE + (_reg), \
+					 _lsb, _msb, KSZ_MAX_NUM_PORTS, \
+					 KSZ8_PORT_SPACING) \
+	}
+#define KSZ8_PORT_REG_FIELD_16(_reg, _lsb, _msb) \
+	{ \
+		.width = KSZ_REGMAP_16, \
+		.regfield = REG_FIELD_ID(KSZ8_PORT_BASE + (_reg), \
+					 _lsb, _msb, KSZ_MAX_NUM_PORTS, \
+					 KSZ8_PORT_SPACING) \
+	}
+#define KSZ8_PORT_REG_FIELD_32(_reg, _lsb, _msb) \
+	{ \
+		.width = KSZ_REGMAP_32, \
+		.regfield = REG_FIELD_ID(KSZ8_PORT_BASE + (_reg), \
+					 _lsb, _msb, KSZ_MAX_NUM_PORTS, \
+					 KSZ8_PORT_SPACING) \
+	}
+/* Some registers are wacky, in the sense that they should be per port (and are
+ * accessed using per-port regfields by the driver), but on some hardware, they
+ * are defined in the global address space, so they should be accessed via the
+ * global regfield API. We hack these up by using a REG_FIELD_ID() definition
+ * with a spacing of 0, so that accesses to any port access the same (global)
+ * register.
+ */
+#define KSZ_WACKY_REG_FIELD_8(_reg, _lsb, _msb) \
+	{ \
+		.width = KSZ_REGMAP_8, \
+		.regfield = REG_FIELD_ID(_reg, _lsb, _msb, KSZ_MAX_NUM_PORTS, 0) \
+	}
+
+static const struct ksz_reg_field ksz8795_regfields[__KSZ_NUM_REGFIELDS] = {
+	[RF_IND_CTRL_0] = KSZ_REG_FIELD_16(0x6E, 0, 15),
+	[RF_IND_BYTE] = KSZ_REG_FIELD_8(0xA0, 0, 7),
+	[RF_FORCE_CTRL] = KSZ8_PORT_REG_FIELD_8(0x0C, 0, 7),
+	[RF_LOCAL_CTRL] = KSZ8_PORT_REG_FIELD_8(0x07, 0, 7),
+	[RF_GMII_1GBIT] = KSZ8_PORT_REG_FIELD_8(0x06, 6, 6),
+	[RF_MII_SEL] = KSZ8_PORT_REG_FIELD_8(0x06, 2, 2),
+	[RF_RGMII_ID_IG_ENABLE] = KSZ8_PORT_REG_FIELD_8(0x06, 4, 4),
+	[RF_RGMII_ID_EG_ENABLE] = KSZ8_PORT_REG_FIELD_8(0x06, 3, 3),
+	[RF_MII_DUPLEX] = KSZ_WACKY_REG_FIELD_8(0x06, 6, 6), // Global Control 4
+	[RF_MII_TX_FLOW_CTRL] = KSZ_WACKY_REG_FIELD_8(0x06, 5, 5), // Global Control 4
+	[RF_MII_100MBIT] = KSZ_WACKY_REG_FIELD_8(0x06, 4, 4), // Global Control 4
+};
+
 static const u16 ksz8795_regs[] = {
 	[REG_IND_CTRL_0]		= 0x6E,
 	[REG_IND_DATA_8]		= 0x70,
@@ -1121,6 +1180,7 @@  const struct ksz_chip_data ksz_switch_chips[] = {
 		.regs = ksz8795_regs,
 		.masks = ksz8795_masks,
 		.shifts = ksz8795_shifts,
+		.regfields = ksz8795_regfields,
 		.xmii_ctrl0 = ksz8795_xmii_ctrl0,
 		.xmii_ctrl1 = ksz8795_xmii_ctrl1,
 		.supports_mii = {false, false, false, false, true},
@@ -2747,34 +2807,28 @@  static void ksz_set_xmii(struct ksz_device *dev, int port,
 {
 	const u8 *bitval = dev->info->xmii_ctrl1;
 	struct ksz_port *p = &dev->ports[port];
-	const u16 *regs = dev->info->regs;
-	u8 data8;
-
-	ksz_pread8(dev, port, regs[P_XMII_CTRL_1], &data8);
-
-	data8 &= ~(P_MII_SEL_M | P_RGMII_ID_IG_ENABLE |
-		   P_RGMII_ID_EG_ENABLE);
+	unsigned int mii_sel;
 
 	switch (interface) {
 	case PHY_INTERFACE_MODE_MII:
-		data8 |= bitval[P_MII_SEL];
+		mii_sel = bitval[P_MII_SEL];
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-		data8 |= bitval[P_RMII_SEL];
+		mii_sel = bitval[P_RMII_SEL];
 		break;
 	case PHY_INTERFACE_MODE_GMII:
-		data8 |= bitval[P_GMII_SEL];
+		mii_sel = bitval[P_GMII_SEL];
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
-		data8 |= bitval[P_RGMII_SEL];
+		mii_sel = bitval[P_RGMII_SEL];
 		/* On KSZ9893, disable RGMII in-band status support */
 		if (dev->chip_id == KSZ9893_CHIP_ID ||
 		    dev->chip_id == KSZ8563_CHIP_ID ||
 		    dev->chip_id == KSZ9563_CHIP_ID)
-			data8 &= ~P_MII_MAC_MODE;
+			ksz_regfields_write(dev, port, RF_MII_MAC_MODE, 0);
 		break;
 	default:
 		dev_err(dev->dev, "Unsupported interface '%s' for port %d\n",
@@ -2782,42 +2836,46 @@  static void ksz_set_xmii(struct ksz_device *dev, int port,
 		return;
 	}
 
-	if (p->rgmii_tx_val)
-		data8 |= P_RGMII_ID_EG_ENABLE;
-
-	if (p->rgmii_rx_val)
-		data8 |= P_RGMII_ID_IG_ENABLE;
-
-	/* Write the updated value */
-	ksz_pwrite8(dev, port, regs[P_XMII_CTRL_1], data8);
+	ksz_regfields_write(dev, port, RF_MII_SEL, mii_sel);
+	ksz_regfields_write(dev, port, RF_RGMII_ID_EG_ENABLE,
+			    !!p->rgmii_tx_val);
+	ksz_regfields_write(dev, port, RF_RGMII_ID_IG_ENABLE,
+			    !!p->rgmii_rx_val);
 }
 
 phy_interface_t ksz_get_xmii(struct ksz_device *dev, int port, bool gbit)
 {
 	const u8 *bitval = dev->info->xmii_ctrl1;
-	const u16 *regs = dev->info->regs;
+	unsigned int mii_sel, ig, eg;
 	phy_interface_t interface;
-	u8 data8;
-	u8 val;
-
-	ksz_pread8(dev, port, regs[P_XMII_CTRL_1], &data8);
+	int ret;
 
-	val = FIELD_GET(P_MII_SEL_M, data8);
+	ret = ksz_regfields_read(dev, port, RF_MII_SEL, &mii_sel);
+	if (WARN_ON(ret))
+		return PHY_INTERFACE_MODE_NA;
 
-	if (val == bitval[P_MII_SEL]) {
+	if (mii_sel == bitval[P_MII_SEL]) {
 		if (gbit)
 			interface = PHY_INTERFACE_MODE_GMII;
 		else
 			interface = PHY_INTERFACE_MODE_MII;
-	} else if (val == bitval[P_RMII_SEL]) {
+	} else if (mii_sel == bitval[P_RMII_SEL]) {
 		interface = PHY_INTERFACE_MODE_RGMII;
 	} else {
+		ret = ksz_regfields_read(dev, port, RF_RGMII_ID_IG_ENABLE, &ig);
+		if (WARN_ON(ret))
+			return PHY_INTERFACE_MODE_NA;
+
+		ret = ksz_regfields_read(dev, port, RF_RGMII_ID_IG_ENABLE, &eg);
+		if (WARN_ON(ret))
+			return PHY_INTERFACE_MODE_NA;
+
 		interface = PHY_INTERFACE_MODE_RGMII;
-		if (data8 & P_RGMII_ID_EG_ENABLE)
+		if (eg)
 			interface = PHY_INTERFACE_MODE_RGMII_TXID;
-		if (data8 & P_RGMII_ID_IG_ENABLE) {
+		if (ig) {
 			interface = PHY_INTERFACE_MODE_RGMII_RXID;
-			if (data8 & P_RGMII_ID_EG_ENABLE)
+			if (eg)
 				interface = PHY_INTERFACE_MODE_RGMII_ID;
 		}
 	}
@@ -2855,14 +2913,13 @@  static void ksz_phylink_mac_config(struct dsa_switch *ds, int port,
 bool ksz_get_gbit(struct ksz_device *dev, int port)
 {
 	const u8 *bitval = dev->info->xmii_ctrl1;
-	const u16 *regs = dev->info->regs;
 	bool gbit = false;
-	u8 data8;
-	bool val;
-
-	ksz_pread8(dev, port, regs[P_XMII_CTRL_1], &data8);
+	unsigned int val;
+	int ret;
 
-	val = FIELD_GET(P_GMII_1GBIT_M, data8);
+	ret = ksz_regfields_read(dev, port, RF_GMII_1GBIT, &val);
+	if (WARN_ON(ret))
+		return false;
 
 	if (val == bitval[P_GMII_1GBIT])
 		gbit = true;
@@ -2873,39 +2930,27 @@  bool ksz_get_gbit(struct ksz_device *dev, int port)
 static void ksz_set_gbit(struct ksz_device *dev, int port, bool gbit)
 {
 	const u8 *bitval = dev->info->xmii_ctrl1;
-	const u16 *regs = dev->info->regs;
-	u8 data8;
-
-	ksz_pread8(dev, port, regs[P_XMII_CTRL_1], &data8);
-
-	data8 &= ~P_GMII_1GBIT_M;
+	unsigned int val;
 
 	if (gbit)
-		data8 |= FIELD_PREP(P_GMII_1GBIT_M, bitval[P_GMII_1GBIT]);
+		val = bitval[P_GMII_1GBIT];
 	else
-		data8 |= FIELD_PREP(P_GMII_1GBIT_M, bitval[P_GMII_NOT_1GBIT]);
+		val = bitval[P_GMII_NOT_1GBIT];
 
-	/* Write the updated value */
-	ksz_pwrite8(dev, port, regs[P_XMII_CTRL_1], data8);
+	ksz_regfields_write(dev, port, RF_GMII_1GBIT, val);
 }
 
 static void ksz_set_100_10mbit(struct ksz_device *dev, int port, int speed)
 {
 	const u8 *bitval = dev->info->xmii_ctrl0;
-	const u16 *regs = dev->info->regs;
-	u8 data8;
-
-	ksz_pread8(dev, port, regs[P_XMII_CTRL_0], &data8);
-
-	data8 &= ~P_MII_100MBIT_M;
+	unsigned int val;
 
 	if (speed == SPEED_100)
-		data8 |= FIELD_PREP(P_MII_100MBIT_M, bitval[P_MII_100MBIT]);
+		val = bitval[P_MII_100MBIT];
 	else
-		data8 |= FIELD_PREP(P_MII_100MBIT_M, bitval[P_MII_10MBIT]);
+		val = bitval[P_MII_10MBIT];
 
-	/* Write the updated value */
-	ksz_pwrite8(dev, port, regs[P_XMII_CTRL_0], data8);
+	ksz_regfields_write(dev, port, RF_MII_100MBIT, val);
 }
 
 static void ksz_port_set_xmii_speed(struct ksz_device *dev, int port, int speed)
@@ -2923,26 +2968,19 @@  static void ksz_duplex_flowctrl(struct ksz_device *dev, int port, int duplex,
 				bool tx_pause, bool rx_pause)
 {
 	const u8 *bitval = dev->info->xmii_ctrl0;
-	const u32 *masks = dev->info->masks;
-	const u16 *regs = dev->info->regs;
-	u8 mask;
-	u8 val;
-
-	mask = P_MII_DUPLEX_M | masks[P_MII_TX_FLOW_CTRL] |
-	       masks[P_MII_RX_FLOW_CTRL];
+	unsigned int val;
 
 	if (duplex == DUPLEX_FULL)
-		val = FIELD_PREP(P_MII_DUPLEX_M, bitval[P_MII_FULL_DUPLEX]);
+		val = bitval[P_MII_FULL_DUPLEX];
 	else
-		val = FIELD_PREP(P_MII_DUPLEX_M, bitval[P_MII_HALF_DUPLEX]);
+		val = bitval[P_MII_HALF_DUPLEX];
 
-	if (tx_pause)
-		val |= masks[P_MII_TX_FLOW_CTRL];
+	ksz_regfields_write(dev, port, RF_MII_DUPLEX, val);
 
-	if (rx_pause)
-		val |= masks[P_MII_RX_FLOW_CTRL];
+	ksz_regfields_write(dev, port, RF_MII_TX_FLOW_CTRL, !!tx_pause);
 
-	ksz_prmw8(dev, port, regs[P_XMII_CTRL_0], mask, val);
+	if (dev->regfields[RF_MII_RX_FLOW_CTRL])
+		ksz_regfields_write(dev, port, RF_MII_RX_FLOW_CTRL, !!rx_pause);
 }
 
 static void ksz9477_phylink_mac_link_up(struct ksz_device *dev, int port,
@@ -3420,6 +3458,35 @@  static const struct dsa_switch_ops ksz_switch_ops = {
 	.set_mac_eee		= ksz_set_mac_eee,
 };
 
+int ksz_regfields_init(struct ksz_device *dev, const struct ksz_chip_data *chip)
+{
+	const struct ksz_reg_field *regfields = chip->regfields;
+	struct regmap_field *rf;
+	struct regmap *regmap;
+	enum ksz_rf i;
+
+	dev->regfields = devm_kcalloc(dev->dev, __KSZ_NUM_REGFIELDS,
+				      sizeof(*dev->regfields), GFP_KERNEL);
+	if (!dev->regfields)
+		return -ENOMEM;
+
+	for (i = 0; i < __KSZ_NUM_REGFIELDS; i++) {
+		if (!regfields[i].regfield.reg)
+			continue;
+
+		regmap = dev->regmap[regfields[i].width];
+
+		rf = devm_regmap_field_alloc(dev->dev, regmap,
+					     regfields[i].regfield);
+		if (IS_ERR(rf))
+			return PTR_ERR(rf);
+
+		dev->regfields[i] = rf;
+	}
+
+	return 0;
+}
+
 struct ksz_device *ksz_switch_alloc(struct device *base, void *priv)
 {
 	struct dsa_switch *ds;
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index ef1643c357a4..a92ebf5417b4 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -47,6 +47,11 @@  struct ksz_mib_names {
 	char string[ETH_GSTRING_LEN];
 };
 
+struct ksz_reg_field {
+	enum ksz_regmap_width width;
+	struct reg_field regfield;
+};
+
 struct ksz_chip_data {
 	u32 chip_id;
 	const char *dev_name;
@@ -68,6 +73,7 @@  struct ksz_chip_data {
 	const u16 *regs;
 	const u32 *masks;
 	const u8 *shifts;
+	const struct ksz_reg_field *regfields;
 	const u8 *xmii_ctrl0;
 	const u8 *xmii_ctrl1;
 	int stp_ctrl_reg;
@@ -145,6 +151,7 @@  struct ksz_device {
 
 	struct device *dev;
 	struct regmap *regmap[__KSZ_NUM_REGMAPS];
+	struct regmap_field **regfields;
 
 	void *priv;
 	int irq;
@@ -235,6 +242,23 @@  enum ksz_regs {
 	P_XMII_CTRL_1,
 };
 
+enum ksz_rf {
+	RF_IND_CTRL_0,
+	RF_IND_BYTE,
+	RF_FORCE_CTRL,
+	RF_LOCAL_CTRL,
+	RF_GMII_1GBIT,
+	RF_MII_100MBIT,
+	RF_MII_SEL,
+	RF_MII_DUPLEX,
+	RF_MII_TX_FLOW_CTRL,
+	RF_MII_RX_FLOW_CTRL,
+	RF_RGMII_ID_IG_ENABLE,
+	RF_RGMII_ID_EG_ENABLE,
+	RF_MII_MAC_MODE,
+	__KSZ_NUM_REGFIELDS,
+};
+
 enum ksz_masks {
 	PORT_802_1P_REMAPPING,
 	SW_TAIL_TAG_ENABLE,
@@ -371,6 +395,7 @@  struct ksz_dev_ops {
 	void (*exit)(struct ksz_device *dev);
 };
 
+int ksz_regfields_init(struct ksz_device *dev, const struct ksz_chip_data *chip);
 struct ksz_device *ksz_switch_alloc(struct device *base, void *priv);
 int ksz_switch_register(struct ksz_device *dev);
 void ksz_switch_remove(struct ksz_device *dev);
@@ -578,6 +603,30 @@  static inline void ksz_prmw8(struct ksz_device *dev, int port, int offset,
 			   mask, val);
 }
 
+static inline int ksz_regfield_read(struct ksz_device *dev, enum ksz_rf rf,
+				    unsigned int *val)
+{
+	return regmap_field_read(dev->regfields[rf], val);
+}
+
+static inline int ksz_regfield_write(struct ksz_device *dev, enum ksz_rf rf,
+				     unsigned int val)
+{
+	return regmap_field_write(dev->regfields[rf], val);
+}
+
+static inline int ksz_regfields_read(struct ksz_device *dev, enum ksz_rf rf,
+				     unsigned int port, unsigned int *val)
+{
+	return regmap_fields_read(dev->regfields[rf], port, val);
+}
+
+static inline int ksz_regfields_write(struct ksz_device *dev, enum ksz_rf rf,
+				      unsigned int port, unsigned int val)
+{
+	return regmap_fields_write(dev->regfields[rf], port, val);
+}
+
 static inline void ksz_regmap_lock(void *__mtx)
 {
 	struct mutex *mtx = __mtx;
diff --git a/drivers/net/dsa/microchip/ksz_spi.c b/drivers/net/dsa/microchip/ksz_spi.c
index 279338451621..a64d2c71ef68 100644
--- a/drivers/net/dsa/microchip/ksz_spi.c
+++ b/drivers/net/dsa/microchip/ksz_spi.c
@@ -77,6 +77,10 @@  static int ksz_spi_probe(struct spi_device *spi)
 		}
 	}
 
+	ret = ksz_regfields_init(dev, chip);
+	if (ret)
+		return ret;
+
 	if (spi->dev.platform_data)
 		dev->pdata = spi->dev.platform_data;