| Message ID | 20220728131852.41518-1-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v1,1/1] net: dsa: microchip: KSZ9893: do not write to not supported Output Clock Control Register | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/apply | fail | Patch does not apply to net |
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
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.
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.
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.
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
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;
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(+)