[PATCH net v1 1/1] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register

[PATCH net v1 1/1] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register

[Oleksij Rempel]

KSZ9893 compatible chips do not have "Output Clock Control Register 0x0103".
So, avoid writing to it.

Fixes: 462d525018f0 ("net: dsa: microchip: move ksz_chip_data to ksz_common")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/dsa/microchip/ksz9477.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 5dff6c3279bb..c73bb6d383ad 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -198,6 +198,10 @@ int ksz9477_reset_switch(struct ksz_device *dev)
 	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
 	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
 
+	/* KSZ9893 compatible chips do not support refclk configuration */
+	if (dev->chip_id == KSZ9893_CHIP_ID)
+		return 0;
+
 	data8 = SW_ENABLE_REFCLKO;
 	if (dev->synclko_disable)
 		data8 = 0;
-- 
2.30.2

Re: [PATCH net v1 1/1] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register

[Andrew Lunn]

On Thu, Jul 28, 2022 at 03:18:52PM +0200, Oleksij Rempel wrote:
> KSZ9893 compatible chips do not have "Output Clock Control Register 0x0103".
> So, avoid writing to it.
> 
> Fixes: 462d525018f0 ("net: dsa: microchip: move ksz_chip_data to ksz_common")
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/dsa/microchip/ksz9477.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
> index 5dff6c3279bb..c73bb6d383ad 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -198,6 +198,10 @@ int ksz9477_reset_switch(struct ksz_device *dev)
>  	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
>  	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
>  
> +	/* KSZ9893 compatible chips do not support refclk configuration */
> +	if (dev->chip_id == KSZ9893_CHIP_ID)
> +		return 0;
> +

Do you actually want to return -EINVAL? I assume this is being driven
by a DT property? And that property is not valid for this chip. So we
want to let the DT writer know. Question is, is there a backwards
compatibility issue? If this has always been silently ignored, and
there are DT with this property, do we want to break them.

      Andrew

Re: [PATCH net v1 1/1] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register

[Jakub Kicinski]

On Thu, 28 Jul 2022 15:18:52 +0200 Oleksij Rempel wrote:
> KSZ9893 compatible chips do not have "Output Clock Control Register 0x0103".
> So, avoid writing to it.

Respin will be needed regardless of the answer to Andrew - patch does
not apply.

Re: [PATCH net v1 1/1] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register

[Oleksij Rempel]

On Thu, Jul 28, 2022 at 10:23:16PM -0700, Jakub Kicinski wrote:
> On Thu, 28 Jul 2022 15:18:52 +0200 Oleksij Rempel wrote:
> > KSZ9893 compatible chips do not have "Output Clock Control Register 0x0103".
> > So, avoid writing to it.
> 
> Respin will be needed regardless of the answer to Andrew - patch does
> not apply.

Hm, this driver was hardly refactored in net-next. I'll better send it
against net-next otherwise things will break.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net v1 1/1] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register

[Jakub Kicinski]

On Fri, 29 Jul 2022 11:48:40 +0200 Oleksij Rempel wrote:
> On Thu, Jul 28, 2022 at 10:23:16PM -0700, Jakub Kicinski wrote:
> > On Thu, 28 Jul 2022 15:18:52 +0200 Oleksij Rempel wrote:  
> > > KSZ9893 compatible chips do not have "Output Clock Control Register 0x0103".
> > > So, avoid writing to it.  
> > 
> > Respin will be needed regardless of the answer to Andrew - patch does
> > not apply.  
> 
> Hm, this driver was hardly refactored in net-next. I'll better send it
> against net-next otherwise things will break.

Probably fine either way, with the net-next patch (i.e. this patch) 
on the list we shouldn't have problems resolving the conflict correctly.

The real question is whether it's okay for 5.19 to not have this patch.
It'd be good to have the user-visible impact specified in the commit
message.

Re: [PATCH net v1 1/1] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register

[Oleksij Rempel]

On Fri, Jul 29, 2022 at 08:41:49AM -0700, Jakub Kicinski wrote:
> On Fri, 29 Jul 2022 11:48:40 +0200 Oleksij Rempel wrote:
> > On Thu, Jul 28, 2022 at 10:23:16PM -0700, Jakub Kicinski wrote:
> > > On Thu, 28 Jul 2022 15:18:52 +0200 Oleksij Rempel wrote:  
> > > > KSZ9893 compatible chips do not have "Output Clock Control Register 0x0103".
> > > > So, avoid writing to it.  
> > > 
> > > Respin will be needed regardless of the answer to Andrew - patch does
> > > not apply.  
> > 
> > Hm, this driver was hardly refactored in net-next. I'll better send it
> > against net-next otherwise things will break.
> 
> Probably fine either way, with the net-next patch (i.e. this patch) 
> on the list we shouldn't have problems resolving the conflict correctly.
> 
> The real question is whether it's okay for 5.19 to not have this patch.
> It'd be good to have the user-visible impact specified in the commit
> message.

So far I'm not aware about issues related to this, only warning in the data
sheet:
"RESERVED address space must not be written under any circumstances. Failure
to heed this warning may result in untoward operation and unexpected results."

I have send this patch as part of register access validation patch set.
There are move fixes any way.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |