Message ID | ojegz5rmcjavsi7rnpkhunyu2mgikibugaffvj24vomvan3jqx@5v6fyz32wqoz (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | KSZ8795 not detected at start to boot from NFS | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Not a local patch |
On Sat, Dec 07, 2024 at 08:53:38AM +0100, Jörg Sommer wrote: > Hi, > > I'm trying to load the root filesystem via NFS with a > NET_DSA_MICROCHIP_KSZ8795_SPI attached to an TI_DAVINCI_EMAC. With 5.10 it > works, but with later versions the kernel fails to detect the switch. It is > since > > commit 8c4599f49841dd663402ec52325dc2233add1d32 > Author: Christian Eggers <ceggers@arri.de> > Date: Fri Nov 20 12:21:07 2020 +0100 > > net: dsa: microchip: ksz8795: setup SPI mode > > This should be done in the device driver instead of the device tree. > > Signed-off-by: Christian Eggers <ceggers@arri.de> > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > > diff --git drivers/net/dsa/microchip/ksz8795_spi.c drivers/net/dsa/microchip/ksz8795_spi.c > index 8b00f8e6c02f..f98432a3e2b5 100644 > --- drivers/net/dsa/microchip/ksz8795_spi.c > +++ drivers/net/dsa/microchip/ksz8795_spi.c > @@ -49,6 +49,12 @@ static int ksz8795_spi_probe(struct spi_device *spi) > if (spi->dev.platform_data) > dev->pdata = spi->dev.platform_data; > > + /* setup spi */ > + spi->mode = SPI_MODE_3; > + ret = spi_setup(spi); > + if (ret) > + return ret; > + > ret = ksz8795_switch_register(dev); > > /* Main DSA driver may not be started yet. */ > > The kernel reports > > [ 1.912756] ksz8795-switch: probe of spi0.1 failed with error -22 > … > [ 2.048054] davinci_emac 1e20000.ethernet: incompatible machine/device type for reading mac address > [ 2.062352] davinci_emac davinci_emac.1: failed to get EMAC clock > [ 2.068632] davinci_emac: probe of davinci_emac.1 failed with error -16 > > It tries to initialize the switch before the ethernet of the SoC is ready. > > Before this commit the kernel returned EPROBE_DEFER instead of EINVAL (or > ENODEV) as a quick This is often true for DSA switches, that the first probe fails with EPROBE_DEFER, because the conduit ethernet is not ready. What i don't understand from your description is why: > + /* setup spi */ > + spi->mode = SPI_MODE_3; > + ret = spi_setup(spi); > + if (ret) > + return ret; > + is causing this issue. Is spi_setup() failing? Andrew
Andrew Lunn schrieb am Sa 07. Dez, 21:47 (+0100): > On Sat, Dec 07, 2024 at 08:53:38AM +0100, Jörg Sommer wrote: > > It tries to initialize the switch before the ethernet of the SoC is ready. > > > > Before this commit the kernel returned EPROBE_DEFER instead of EINVAL (or > > ENODEV) as a quick > > This is often true for DSA switches, that the first probe fails with > EPROBE_DEFER, because the conduit ethernet is not ready. > > What i don't understand from your description is why: > > > + /* setup spi */ > > + spi->mode = SPI_MODE_3; > > + ret = spi_setup(spi); > > + if (ret) > > + return ret; > > + > > is causing this issue. Is spi_setup() failing? I've added another dev_err() after the spi_setup: [ 1.680516] ksz8795-switch spi0.1: ksz8795_spi_probe:55: ret of spi_setup=0 [ 1.819194] ksz8795-switch spi0.1: ksz8795_spi_probe:61: ret=-22 [ 1.825611] spi_davinci 1f0e000.spi: Controller at 0x(ptrval) … [ 1.949668] ksz8795-switch spi0.1: ksz8795_spi_probe:55: ret of spi_setup=0 [ 2.090136] ksz8795-switch spi0.1: ksz8795_spi_probe:61: ret=-517 [ 2.097438] ksz8795-switch spi0.1: ksz8795_spi_probe:55: ret of spi_setup=0 [ 2.230043] ksz8795-switch spi0.1: ksz8795_spi_probe:61: ret=-517 [ 2.241625] davinci_emac 1e20000.ethernet: incompatible machine/device type for reading mac address [ 2.255218] ksz8795-switch spi0.1: ksz8795_spi_probe:55: ret of spi_setup=0 … [ 3.121263] libphy: dsa slave smi: probed [ 3.127206] ksz8795-switch spi0.1 lan-x1 (uninitialized): PHY [dsa-0.0:00] driver [Generic PHY] (irq=POLL) [ 3.142775] ksz8795-switch spi0.1 lan-x2 (uninitialized): PHY [dsa-0.0:01] driver [Generic PHY] (irq=POLL)
Hi Jörg, hi Andrew, On Saturday, 7 December 2024, 21:47:31 CET, Andrew Lunn wrote: > What i don't understand from your description is why: > > > + /* setup spi */ > > + spi->mode = SPI_MODE_3; > > + ret = spi_setup(spi); > > + if (ret) > > + return ret; > > + > > is causing this issue. Is spi_setup() failing? On Saturday, 7 December 2024, 22:07:23 CET, Jörg Sommer wrote: > I've added another dev_err() after the spi_setup: > > [ 1.680516] ksz8795-switch spi0.1: ksz8795_spi_probe:55: ret of spi_setup=0 > [ 1.819194] ksz8795-switch spi0.1: ksz8795_spi_probe:61: ret=-22# It doesn't look so. @Jörg. You didn't explicitly mention which kernel version you are trying to run. But from the line numbers in you log I guess at could be 5.11 or similar. I guess that the 2nd return code (-22) originates from ksz8795_switch_register() which in turn calls ksz_switch_register() [ksz_common.c]. Maybe the -EINVAL comes from line 414: if (dev->dev_ops->detect(dev)) return -EINVAL; But this is only a guess. Can you please add further debug messages in ksz_switch_register() in order to track down were the -EINVAL actually comes from? Maybe it's one of the error returns from ksz8795_register_switch(). My original intention for the mentioned patch presumably was, that always SPI mode 3 should be configured for this switch (as stated in the data sheet). But maybe this isn't true for your setup (do you have an inverter in your SPI clock line)? > Andrew Lunn schrieb am Sa 07. Dez, 21:47 (+0100): > > > > What i don't understand from your description is why: > > > > > + /* setup spi */ > > > + spi->mode = SPI_MODE_3; > > > + ret = spi_setup(spi); > > > + if (ret) > > > + return ret; > > > + > > > > is causing this issue. Is spi_setup() failing? Maybe that the configured SPI mode does somehow not work for Jörg's setup. Perhaps SPI mode 3 on his controller is not the same as for my one (NXP i.MX6). This could then cause a mismatch when reading the chip id in ksz8795_switch_detect(). @Jörg: Can you please check this? If possible, a measurement of the SPI lines (with an oscilloscope or logic analyzer) would be interesting. regards, Christian
Hi Christian, Christian Eggers schrieb am Sa 07. Dez, 23:44 (+0100): > On Saturday, 7 December 2024, 21:47:31 CET, Andrew Lunn wrote: > > What i don't understand from your description is why: > > > > > + /* setup spi */ > > > + spi->mode = SPI_MODE_3; > > > + ret = spi_setup(spi); > > > + if (ret) > > > + return ret; > > > + > > > > is causing this issue. Is spi_setup() failing? > > On Saturday, 7 December 2024, 22:07:23 CET, Jörg Sommer wrote: > > > I've added another dev_err() after the spi_setup: > > > > [ 1.680516] ksz8795-switch spi0.1: ksz8795_spi_probe:55: ret of spi_setup=0 > > [ 1.819194] ksz8795-switch spi0.1: ksz8795_spi_probe:61: ret=-22# > > It doesn't look so. > > @Jörg. You didn't explicitly mention which kernel version you are trying to run. I've checked out 8c4599f49841dd663402ec52325dc2233add1d32. But I also have to apply the commits 1aa4ee0ec7fe929bd46ae20d9457f0a242115643 ba6e5af621ab2fb4cd4acb37d4914c832991689c f19d8dfad67b641af274a9a317a12f31c430e254, because 5.10 doesn't work without them, too, and they where added somewhere in 5.10.223. From 5.10 with these additionally changes I started and found 8c4599. > Maybe the -EINVAL comes from line 414: > > if (dev->dev_ops->detect(dev)) > return -EINVAL; > > But this is only a guess. You're right. [ 1.678236] ksz8795-switch spi0.1: ksz8795_spi_probe:53: spi->mode=0 before SPI_MODE_3=3 [ 1.686754] ksz8795-switch spi0.1: ksz8795_spi_probe:56: spi_setup()=0 [ 1.817017] ksz8795-switch spi0.1: ksz8795_switch_detect:1151: id1=0 <> FAMILY_ID=135, id2=0 <> CHIP_ID_94=96 && CHIP_ID_95=144 [ 1.828812] ksz8795-switch spi0.1: ksz_switch_register:415: dev->dev_ops->detect()c037e47c=-19 [ 1.837609] ksz8795-switch spi0.1: ksz8795_spi_probe:62: ksz8795_switch_register()=-22 [ 1.845909] spi_davinci 1f0e000.spi: Controller at 0x(ptrval) … [ 1.969734] ksz8795-switch spi0.1: ksz8795_spi_probe:53: spi->mode=3 before SPI_MODE_3=3 [ 1.978309] ksz8795-switch spi0.1: ksz8795_spi_probe:56: spi_setup()=0 [ 2.117906] ksz8795-switch spi0.1: ksz8795_spi_probe:62: ksz8795_switch_register()=-517 [ 2.127235] ksz8795-switch spi0.1: ksz8795_spi_probe:53: spi->mode=3 before SPI_MODE_3=3 [ 2.135427] ksz8795-switch spi0.1: ksz8795_spi_probe:56: spi_setup()=0 [ 2.267825] ksz8795-switch spi0.1: ksz8795_spi_probe:62: ksz8795_switch_register()=-517 [ 2.281336] davinci_emac 1e20000.ethernet: incompatible machine/device type for reading mac address [ 2.294928] ksz8795-switch spi0.1: ksz8795_spi_probe:53: spi->mode=3 before SPI_MODE_3=3 [ 2.305365] ksz8795-switch spi0.1: ksz8795_spi_probe:56: spi_setup()=0 [ 2.954286] random: crng init done [ 3.174408] libphy: dsa slave smi: probed [ 3.180666] ksz8795-switch spi0.1 lan-x1 (uninitialized): PHY [dsa-0.0:00] driver [Generic PHY] (irq=POLL) This is my setting: git reset --hard 8c4599f49841dd663402ec52325dc2233add1d32 git cherry-pick -n \ 1aa4ee0ec7fe929bd46ae20d9457f0a242115643 \ f365d53c868725c472d515fa1ce4f57d0eaff5ae \ ba6e5af621ab2fb4cd4acb37d4914c832991689c \ f19d8dfad67b641af274a9a317a12f31c430e254 git apply -p0 --ignore-whitespace <<__EOF diff --git drivers/net/dsa/microchip/ksz8795.c drivers/net/dsa/microchip/ksz8795.c index 1e101ab56cea..4c9f27a061c1 100644 --- drivers/net/dsa/microchip/ksz8795.c +++ drivers/net/dsa/microchip/ksz8795.c @@ -1147,8 +1147,11 @@ static int ksz8795_switch_detect(struct ksz_device *dev) id1 = id16 >> 8; id2 = id16 & SW_CHIP_ID_M; if (id1 != FAMILY_ID || - (id2 != CHIP_ID_94 && id2 != CHIP_ID_95)) + (id2 != CHIP_ID_94 && id2 != CHIP_ID_95)) { +dev_err(dev->dev, "%s:%d: id1=%d <> FAMILY_ID=%d, id2=%d <> CHIP_ID_94=%d && CHIP_ID_95=%d \n", + __func__, __LINE__, id1, FAMILY_ID, id2, CHIP_ID_94, CHIP_ID_95); return -ENODEV; + } dev->mib_port_cnt = TOTAL_PORT_NUM; dev->phy_port_cnt = SWITCH_PORT_NUM; diff --git drivers/net/dsa/microchip/ksz8795_spi.c drivers/net/dsa/microchip/ksz8795_spi.c index 5dab5d36c675..cb812538ec5b 100644 --- drivers/net/dsa/microchip/ksz8795_spi.c +++ drivers/net/dsa/microchip/ksz8795_spi.c @@ -50,13 +50,19 @@ static int ksz8795_spi_probe(struct spi_device *spi) dev->pdata = spi->dev.platform_data; /* setup spi */ + dev_err(&spi->dev, "%s:%d: spi->mode=%d before SPI_MODE_3=%d\n", __func__, __LINE__, spi->mode, SPI_MODE_3); spi->mode = SPI_MODE_3; ret = spi_setup(spi); + dev_err(&spi->dev, "%s:%d: spi_setup()=%d\n", __func__, __LINE__, ret); if (ret) return ret; ret = ksz8795_switch_register(dev); + dev_err(&spi->dev, "%s:%d: ksz8795_switch_register()=%d\n", __func__, __LINE__, ret); + if (ret == -EINVAL || ret == -ENODEV) + ret = -EPROBE_DEFER; + /* Main DSA driver may not be started yet. */ if (ret) return ret; diff --git drivers/net/dsa/microchip/ksz_common.c drivers/net/dsa/microchip/ksz_common.c index 32836450d62c..44b66951c633 100644 --- drivers/net/dsa/microchip/ksz_common.c +++ drivers/net/dsa/microchip/ksz_common.c @@ -410,8 +410,11 @@ int ksz_switch_register(struct ksz_device *dev, dev->dev_ops = ops; - if (dev->dev_ops->detect(dev)) +ret = dev->dev_ops->detect(dev); + if (ret) { +dev_err(dev->dev, "%s:%d: dev->dev_ops->detect()%lx=%d\n", __func__, __LINE__, dev->dev_ops->detect, ret); return -EINVAL; + } ret = dev->dev_ops->init(dev); if (ret) > > Andrew Lunn schrieb am Sa 07. Dez, 21:47 (+0100): > > > > > > What i don't understand from your description is why: > > > > > > > + /* setup spi */ > > > > + spi->mode = SPI_MODE_3; > > > > + ret = spi_setup(spi); > > > > + if (ret) > > > > + return ret; > > > > + > > > > > > is causing this issue. Is spi_setup() failing? > > Maybe that the configured SPI mode does somehow not work for Jörg's setup. > Perhaps SPI mode 3 on his controller is not the same as for my one (NXP i.MX6). > This could then cause a mismatch when reading the chip id in ksz8795_switch_detect(). Or am I missing something in my devicetree to set the SPI to mode 3? > @Jörg: Can you please check this? If possible, a measurement of the SPI > lines (with an oscilloscope or logic analyzer) would be interesting. I can check this in the next days. I only have remote access to the device. Thanks for helping me. Jörg
Jörg Sommer schrieb am So 08. Dez, 17:44 (+0100): > Christian Eggers schrieb am Sa 07. Dez, 23:44 (+0100): > > On Saturday, 7 December 2024, 21:47:31 CET, Andrew Lunn wrote: > > > What i don't understand from your description is why: > > > > > > > + /* setup spi */ > > > > + spi->mode = SPI_MODE_3; > > > > + ret = spi_setup(spi); > > > > + if (ret) > > > > + return ret; > > > > + > > > > > > is causing this issue. Is spi_setup() failing? > > > > On Saturday, 7 December 2024, 22:07:23 CET, Jörg Sommer wrote: > > > > > I've added another dev_err() after the spi_setup: > > > > > > [ 1.680516] ksz8795-switch spi0.1: ksz8795_spi_probe:55: ret of spi_setup=0 > > > [ 1.819194] ksz8795-switch spi0.1: ksz8795_spi_probe:61: ret=-22# Hi Christian, I've changed the code for debugging to this: { u16 id16; ret = ksz_read16(dev, 0x00, &id16); dev_err(&spi->dev, "%s:%d: ksz_read16(REG_CHIP_ID0, %d) = %d\n", __func__, __LINE__, id16, ret); } /* setup spi */ dev_err(&spi->dev, "Switching SPI mode from %d to spi-cpha,spi-cpol\n", spi->mode); spi->mode = SPI_MODE_3; ret = spi_setup(spi); if (ret) return ret; { u16 id16; ret = ksz_read16(dev, 0x00, &id16); dev_err(&spi->dev, "%s:%d: ksz_read16(REG_CHIP_ID0, %d) = %d\n", __func__, __LINE__, id16, ret); } ret = ksz8_switch_register(dev); dev_err(&spi->dev, "ksz8795_spi_probe:%d: ret=%d\n", __LINE__, ret); if (ret == -EINVAL || ret == -ENODEV) ret = -EPROBE_DEFER; With a devicetree without spi-cpha, spi-cpol I get this: [ 1.751347] ksz8795-switch spi0.1: ksz8795_spi_probe:76: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 1.751445] ksz8795-switch spi0.1: Switching SPI mode from 0 to spi-cpha,spi-cpol [ 1.752226] ksz8795-switch spi0.1: ksz8795_spi_probe:91: ksz_read16(REG_CHIP_ID0, 0) = 0 [ 1.889370] ksz8795-switch spi0.1: ksz8795_spi_probe:97: ret=-517 [ 1.891431] ksz8795-switch spi0.1: ksz8795_spi_probe:76: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 1.891525] ksz8795-switch spi0.1: Switching SPI mode from 3 to spi-cpha,spi-cpol [ 1.892282] ksz8795-switch spi0.1: ksz8795_spi_probe:91: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 2.034994] ksz8795-switch spi0.1: ksz8795_spi_probe:97: ret=-517 [ 2.043130] ksz8795-switch spi0.1: ksz8795_spi_probe:76: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 2.043217] ksz8795-switch spi0.1: Switching SPI mode from 3 to spi-cpha,spi-cpol [ 2.043957] ksz8795-switch spi0.1: ksz8795_spi_probe:91: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 2.187793] ksz8795-switch spi0.1: ksz8795_spi_probe:97: ret=-517 [ 2.195915] davinci_emac 1e20000.ethernet: incompatible machine/device type for reading mac address [ 2.207703] ksz8795-switch spi0.1: ksz8795_spi_probe:76: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 2.207795] ksz8795-switch spi0.1: Switching SPI mode from 3 to spi-cpha,spi-cpol [ 2.208554] ksz8795-switch spi0.1: ksz8795_spi_probe:91: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 4.212102] ksz8795-switch spi0.1: configuring for fixed/rmii link mode [ 4.219662] ksz8795-switch spi0.1: Link is Up - 100Mbps/Full - flow control rx/tx The immediate read after the spi_setup reports `0` for the chip ID. If I comment the second block, this brings the -22 which stops further probing (if not mangled by by EINVAL → EPROBE_DEFER). [ 1.712445] ksz8795-switch spi0.1: ksz8795_spi_probe:76: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 1.712545] ksz8795-switch spi0.1: Switching SPI mode from 0 to spi-cpha,spi-cpol [ 1.851109] ksz8795-switch spi0.1: invalid family id: 0 [ 1.851192] ksz8795-switch spi0.1: ksz8795_spi_probe:97: ret=-22 [ 1.853241] ksz8795-switch spi0.1: ksz8795_spi_probe:76: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 1.853336] ksz8795-switch spi0.1: Switching SPI mode from 3 to spi-cpha,spi-cpol [ 1.992243] ksz8795-switch spi0.1: ksz8795_spi_probe:97: ret=-517 [ 1.994767] ksz8795-switch spi0.1: ksz8795_spi_probe:76: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 1.994860] ksz8795-switch spi0.1: Switching SPI mode from 3 to spi-cpha,spi-cpol [ 2.128827] ksz8795-switch spi0.1: ksz8795_spi_probe:97: ret=-517 [ 2.140263] davinci_emac 1e20000.ethernet: incompatible machine/device type for reading mac address [ 2.148743] ksz8795-switch spi0.1: ksz8795_spi_probe:76: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 2.148836] ksz8795-switch spi0.1: Switching SPI mode from 3 to spi-cpha,spi-cpol [ 4.145963] ksz8795-switch spi0.1: configuring for fixed/rmii link mode [ 4.146161] ksz8795-switch spi0.1: Link is Up - 100Mbps/Full - flow control rx/tx So I think it's a timing problem: the ksz8795 isn't ready after the SPI reset, when the chip ID gets read, and this causes the probing to stop. Why is SPI_MODE_3 required? At me, the chip works fine with SPI_MODE_0. Regards, Jörg
> So I think it's a timing problem: the ksz8795 isn't ready after the SPI > reset, when the chip ID gets read, and this causes the probing to stop. Is there anything in the datasheet about reset timing? Andrew
Andrew Lunn schrieb am Di 10. Dez, 18:41 (+0100): > > So I think it's a timing problem: the ksz8795 isn't ready after the SPI > > reset, when the chip ID gets read, and this causes the probing to stop. > > Is there anything in the datasheet about reset timing? Not exactly. On page 127 is a diagram about resetting the chip. https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8795CLX-Data-Sheet-DS00002112.pdf Here's a commit that contains a calculation and determines 100ms. commit 1aa4ee0ec7fe929bd46ae20d9457f0a242115643 Author: Marek Vasut <marex@denx.de> Date: Wed Jan 20 04:05:02 2021 +0100 net: dsa: microchip: Adjust reset release timing to match reference reset circuit commit 1c45ba93d34cd6af75228f34d0675200c81738b5 upstream. KSZ8794CNX datasheet section 8.0 RESET CIRCUIT describes recommended circuit for interfacing with CPU/FPGA reset consisting of 10k pullup resistor and 10uF capacitor to ground. This circuit takes ~100 ms to rise enough to release the reset. For maximum supply voltage VDDIO=3.3V VIH=2.0V R=10kR C=10uF that is VDDIO - VIH t = R * C * -ln( ------------- ) = 10000*0.00001*-(-0.93)=0.093 s VDDIO so we need ~95 ms for the reset to really de-assert, and then the original 100us for the switch itself to come out of reset. Simply msleep() for 100 ms which fits the constraint with a bit of extra space. Jörg
Hi Jörg, On Tuesday, 10 December 2024, 17:43:01 CET, Jörg Sommer wrote: > > So I think it's a timing problem: the ksz8795 isn't ready after the SPI > reset, when the chip ID gets read, and this causes the probing to stop. > > Why is SPI_MODE_3 required? At me, the chip works fine with SPI_MODE_0. I tried to reconstruct why I actually did this change (sorry, I am over 40): 1. I was working on the PTP patches for KSZ956x. 2. It was necessary to convert the devicetree bindings to Yaml. 3. There where objections against keeping "spi-cpha" and "spi-cpol" in the example code: https://lore.kernel.org/netdev/20201119134801.GB3149565@bogus/ On Thursday, 19 November 2020 07:48:01 -0600, Rob Hering wrote: > On Wed, Nov 18, 2020 at 09:30:02PM +0100, Christian Eggers wrote: ... > > + ksz9477: switch@0 { > > + compatible = "microchip,ksz9477"; > > + reg = <0>; > > + reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; > > + > > + spi-max-frequency = <44000000>; > > + spi-cpha; > > + spi-cpol; > > Are these 2 optional or required? Being optional is rare as most > devices support 1 mode, but not unheard of. In general, you shouldn't > need them as the driver should know how to configure the mode if the h/w > is fixed. ... It seems that I considered the h/w as "fixed". The pre-existing device tree bindings and the diagrams on page 53 suggested that SPI mode 3 is the only valid option. Particularly the idle state of the "SCL" signal is high here: https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf But the text description on page 52 says something different: > SCL is expected to stay low when SPI operation is idle. Especially the timing diagrams on page 206 look more like SPI mode 0. So it is possible that my patch was wrong (due to inconsistent description on the data sheet / pre existing device tree binding). As I already mentioned, I did this only due to the DT conversion, I actually don't use SPI on such devices myself. N.B. Which KSZ device do you actually use (I didn't find this in you previous mails)? On Sunday, 8 December 2024, 17:44:49 CET, Jörg Sommer wrote: > Or am I missing something in my devicetree to set the SPI to mode 3? The idea was that for "fixed" hardware the SPI mode should be setup in the drivers probe() routine rather than in the device tree. regards, Christian
Christian Eggers schrieb am Mi 11. Dez, 11:18 (+0100): > Hi Jörg, > > On Tuesday, 10 December 2024, 17:43:01 CET, Jörg Sommer wrote: > > > > So I think it's a timing problem: the ksz8795 isn't ready after the SPI > > reset, when the chip ID gets read, and this causes the probing to stop. > > > > Why is SPI_MODE_3 required? At me, the chip works fine with SPI_MODE_0. > > I tried to reconstruct why I actually did this change (sorry, I am over 40): Don't worry. Me, too. :) > 1. I was working on the PTP patches for KSZ956x. > 2. It was necessary to convert the devicetree bindings to Yaml. > 3. There where objections against keeping "spi-cpha" and "spi-cpol" > in the example code: > https://lore.kernel.org/netdev/20201119134801.GB3149565@bogus/ I think for 8795 these are optional. At me, it works with 0 and 3. I'm not an expert. So, please, double check this: the spec [1] says on page 53, table 4-3, register 11, bit 0 “Trigger on the rising edge of SPI clock (for higher speed SPI)”. According to [2] the rising edge is cpol=0 and mode 0. So, “higher speed SPI” (I think this is the 25MHz) should use mode 0. [1] https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8795CLX-Data-Sheet-DS00002112.pdf [2] https://electronics.stackexchange.com/a/455564 > On Thursday, 19 November 2020 07:48:01 -0600, Rob Hering wrote: > > On Wed, Nov 18, 2020 at 09:30:02PM +0100, Christian Eggers wrote: > ... > > > + ksz9477: switch@0 { > > > + compatible = "microchip,ksz9477"; > > > + reg = <0>; > > > + reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; > > > + > > > + spi-max-frequency = <44000000>; > > > + spi-cpha; > > > + spi-cpol; > > > > Are these 2 optional or required? Being optional is rare as most > > devices support 1 mode, but not unheard of. In general, you shouldn't > > need them as the driver should know how to configure the mode if the h/w > > is fixed. > ... > > It seems that I considered the h/w as "fixed". The pre-existing device tree > bindings and the diagrams on page 53 suggested that SPI mode 3 is the only > valid option. Particularly the idle state of the "SCL" signal is high here: > > https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf > > But the text description on page 52 says something different: > > SCL is expected to stay low when SPI operation is idle. > > Especially the timing diagrams on page 206 look more like SPI mode 0. > > So it is possible that my patch was wrong (due to inconsistent description > on the data sheet / pre existing device tree binding). As I already mentioned, > I did this only due to the DT conversion, I actually don't use SPI on such > devices myself. > > N.B. Which KSZ device do you actually use (I didn't find this in you previous > mails)? I'm using KSZ8795. Jörg.
Hi Jörg, On Wednesday, 11 December 2024, 13:23:38 CET, Jörg Sommer wrote: > Christian Eggers schrieb am Mi 11. Dez, 11:18 (+0100): > I think for 8795 these are optional. At me, it works with 0 and 3. Hmm, I understood that setting SPI mode to 3 (by my patch) is the root of your problem? If you revert my patch and set spi-cpol + spi-cpha in you device tree, the result should be more or less the same. If you think that your problem is related to the reset timing, feel free to increase the sleep time after asserting/deasserting the reset line. Beside the hardware reset there's usually also a software reset. But this type of reset normally doesn't affect consecutive register accesses. > I'm not an expert. So, please, double check this: the spec [1] says on > page 53, table 4-3, register 11, bit 0 “Trigger on the rising edge of SPI > clock (for higher speed SPI)”. According to [2] the rising edge is cpol=0 > and mode 0. So, “higher speed SPI” (I think this is the 25MHz) should use > mode 0. > > [1] https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8795CLX-Data-Sheet-DS00002112.pdf > [2] https://electronics.stackexchange.com/a/455564 I hate SPI because of its poorly written specifications! When I read the corresponding sections of the KSZ9563 DS [3], I come to the conclusion that the register bit you mentioned above affects the SPI *output* signal (SPIQ a.k.a MISO). This would also make more sense, as you usually cannot change the behavior of the SPI input lines. [3] https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf Page 68, on the bottom: > *SPI Data Out Edge Select* > 0 = SDO data is clocked by the falling edge of SCL > 1 = SDO data is clocked by the rising edge of SCL So the bit 0 is intended to adjust the phase of the SPIQ/SDO/MOSI output signal, in order to avoid that this signal is switched at the same clock edge where your uC samples the MISO input. Also for the KSZ8795CLX there seems to be a mismatch regarding the SPI clock polarity in the datasheet. Page 28 (functional description) implies CPOL=1 whilst page 123 (timing diagram) shows CPOL=0. I would trust the latter in this case. > > > > On Thursday, 19 November 2020 07:48:01 -0600, Rob Hering wrote: > > > On Wed, Nov 18, 2020 at 09:30:02PM +0100, Christian Eggers wrote: > > ... > > > > + ksz9477: switch@0 { > > > > + compatible = "microchip,ksz9477"; > > > > + reg = <0>; > > > > + reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; > > > > + > > > > + spi-max-frequency = <44000000>; > > > > + spi-cpha; > > > > + spi-cpol; > > > > > > Are these 2 optional or required? Being optional is rare as most > > > devices support 1 mode, but not unheard of. In general, you shouldn't > > > need them as the driver should know how to configure the mode if the h/w > > > is fixed. > > ... > > > > It seems that I considered the h/w as "fixed". The pre-existing device tree > > bindings and the diagrams on page 53 suggested that SPI mode 3 is the only > > valid option. Particularly the idle state of the "SCL" signal is high here: > > > > https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf > > > > But the text description on page 52 says something different: > > > SCL is expected to stay low when SPI operation is idle. > > > > Especially the timing diagrams on page 206 look more like SPI mode 0. > > > > So it is possible that my patch was wrong (due to inconsistent description > > on the data sheet / pre existing device tree binding). As I already mentioned, > > I did this only due to the DT conversion, I actually don't use SPI on such > > devices myself. > > > > N.B. Which KSZ device do you actually use (I didn't find this in you previous > > mails)? > > I'm using KSZ8795. I should better read the subject line ... Summary: - The timing diagrams of KSZ8795CLX and KSZ9563 implies that SPI mode 0 is correct - The functional descriptions in the datasheets look more like SPI mode 3, but this is not authoritative. - Maybe that the KSZ devices can work with both modes. regards, Christian
> https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf > > But the text description on page 52 says something different: > > SCL is expected to stay low when SPI operation is idle. > > Especially the timing diagrams on page 206 look more like SPI mode 0. You probably want to Cc: some SPI people. netdev people don't normally get involved in low level SPI stuff, so are probably not going to give useful comments. Andrew
Hi Christian, hi SPI people, we are debating the SPI mode setting for the microchip ksz8795 and ksz9477 and possibly others. Since the commit 8c4599f49841dd663402ec52325dc2233add1d32 the SPI mode gets fixed to mode 3 in the code. But at least my ksz8795 works also with mode 0 and shows better initialization behaviour with mode 0. The big question is: can both chips work with both modes? Should this setting stay in code or moved to the device tree? https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8795CLX-Data-Sheet-DS00002112.pdf Christian Eggers schrieb am Mi 11. Dez, 14:04 (+0100): > On Wednesday, 11 December 2024, 13:23:38 CET, Jörg Sommer wrote: > > Christian Eggers schrieb am Mi 11. Dez, 11:18 (+0100): > > > I think for 8795 these are optional. At me, it works with 0 and 3. > Hmm, I understood that setting SPI mode to 3 (by my patch) is the > root of your problem? If you revert my patch and set spi-cpol + spi-cpha > in you device tree, the result should be more or less the same. Yes, that's right. When I set spi-cpol + spi-cpha in the DT (or with your patch) I get the message “ksz8795-switch spi0.1: invalid family id: 0”. Without it, I don't get this message and the switch works fine. > If you think that your problem is related to the reset timing, Not really. My impression is more that “the first read after mode switch always fails.” From my other mail: [ 1.712545] ksz8795-switch spi0.1: Switching SPI mode from 0 to spi-cpha,spi-cpol [ 1.851109] ksz8795-switch spi0.1: invalid family id: 0 a gap of 140ms. With two reads immediately after the spi_setup: [ 1.569835] ksz8795-switch spi0.1: Switching SPI mode from 0 to spi-cpha,spi-cpol [ 1.570641] ksz8795-switch spi0.1: ksz8795_spi_probe:84: ksz_read16(REG_CHIP_ID0, 0) = 0 [ 1.571420] ksz8795-switch spi0.1: ksz8795_spi_probe:90: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 1.701375] ksz8795-switch spi0.1: Switching SPI mode from 3 to spi-cpha,spi-cpol [ 1.702191] ksz8795-switch spi0.1: ksz8795_spi_probe:84: ksz_read16(REG_CHIP_ID0, 34705) = 0 [ 1.702928] ksz8795-switch spi0.1: ksz8795_spi_probe:90: ksz_read16(REG_CHIP_ID0, 34705) = 0 Maybe the chip needs this read to detect the SPI mode. And if this first read is the chip detection the initialization fails. > > > On Thursday, 19 November 2020 07:48:01 -0600, Rob Hering wrote: > > > > On Wed, Nov 18, 2020 at 09:30:02PM +0100, Christian Eggers wrote: > > > ... > > > > > + ksz9477: switch@0 { > > > > > + compatible = "microchip,ksz9477"; > > > > > + reg = <0>; > > > > > + reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; > > > > > + > > > > > + spi-max-frequency = <44000000>; > > > > > + spi-cpha; > > > > > + spi-cpol; > > > > > > > > Are these 2 optional or required? Being optional is rare as most > > > > devices support 1 mode, but not unheard of. In general, you shouldn't > > > > need them as the driver should know how to configure the mode if the h/w > > > > is fixed. > > > ... > > > > > > It seems that I considered the h/w as "fixed". The pre-existing device tree > > > bindings and the diagrams on page 53 suggested that SPI mode 3 is the only > > > valid option. Particularly the idle state of the "SCL" signal is high here: > > > > > > https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf > > > > > > But the text description on page 52 says something different: > > > > SCL is expected to stay low when SPI operation is idle. > > > > > > Especially the timing diagrams on page 206 look more like SPI mode 0. > > > > > > So it is possible that my patch was wrong (due to inconsistent description > > > on the data sheet / pre existing device tree binding). As I already mentioned, > > > I did this only due to the DT conversion, I actually don't use SPI on such > > > devices myself. > > > > > > N.B. Which KSZ device do you actually use (I didn't find this in you previous > > > mails)? > > > > I'm using KSZ8795. > > I should better read the subject line ... > > Summary: > - The timing diagrams of KSZ8795CLX and KSZ9563 implies that SPI mode 0 is correct > - The functional descriptions in the datasheets look more like SPI mode 3, but this > is not authoritative. > - Maybe that the KSZ devices can work with both modes. Or maybe not all ksz8 behave the same? Should we revert the behaviour and leave it up to the device tree to decide? Jörg
Hi everyone, I've added you to the list of recipients, because you where somehow involved in changes of the KSZ-SPI switch code. We are debating the SPI mode setting for the microchip ksz8795 and ksz9477 and possibly others. Since the commit 8c4599f49841dd663402ec52325dc2233add1d32 the SPI mode gets fixed to mode 3 in the code. But at least my ksz8795 works also with mode 0 and shows better initialization behaviour with mode 0. The big question is: can both (or all ksz) chips work with both modes? Should this setting stay in code or moved to the device tree? The specs are here, but I found no evidence about the supported/recommended SPI modes: https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8795CLX-Data-Sheet-DS00002112.pdf The commit 8c4599f498 was a result of the conversion of device trees where already the question about the SPI mode was raised: https://lore.kernel.org/netdev/20201119134801.GB3149565@bogus/ > > + spi-cpha; > > + spi-cpol; > > Are these 2 optional or required? Being optional is rare as most > devices support 1 mode, but not unheard of. In general, you shouldn't > need them as the driver should know how to configure the mode if the h/w > is fixed. My observation is that the ksz8795 supports mode 0 and mode 3, but it seems the first read with mode 3 always fails (read value for e.g. chip ID is 0) which causes the initialization of the chip to fail. Removing the code in ksz_spi.c:88,+5 or modifying it to use mode 0 gives me a working ksz chip and I can happily mount the NFS root filesystem. Can someone at microchip acknowledge that both SPI modes are supported by the ksz8795 or all chips? Who would be able to test patches? Thanks in advance. Here is the last part of the discussion started at https://lore.kernel.org/netdev/cxe42bethnzs7f46xxyvj6ok6ve7itssdxyh2vuftnfws4aa3z@2o4njdkw3r5i/T/ Jörg Sommer schrieb am Mi 11. Dez, 15:46 (+0100): > Christian Eggers schrieb am Mi 11. Dez, 14:04 (+0100): > > On Wednesday, 11 December 2024, 13:23:38 CET, Jörg Sommer wrote: > > > Christian Eggers schrieb am Mi 11. Dez, 11:18 (+0100): > > > > > I think for 8795 these are optional. At me, it works with 0 and 3. > > Hmm, I understood that setting SPI mode to 3 (by my patch) is the > > root of your problem? If you revert my patch and set spi-cpol + spi-cpha > > in you device tree, the result should be more or less the same. > > Yes, that's right. When I set spi-cpol + spi-cpha in the DT (or with your > patch) I get the message “ksz8795-switch spi0.1: invalid family id: 0”. > Without it, I don't get this message and the switch works fine. > > > If you think that your problem is related to the reset timing, > > Not really. My impression is more that “the first read after mode switch > always fails.” > > From my other mail: > > [ 1.712545] ksz8795-switch spi0.1: Switching SPI mode from 0 to spi-cpha,spi-cpol > [ 1.851109] ksz8795-switch spi0.1: invalid family id: 0 > > a gap of 140ms. > > With two reads immediately after the spi_setup: > > [ 1.569835] ksz8795-switch spi0.1: Switching SPI mode from 0 to spi-cpha,spi-cpol > [ 1.570641] ksz8795-switch spi0.1: ksz8795_spi_probe:84: ksz_read16(REG_CHIP_ID0, 0) = 0 > [ 1.571420] ksz8795-switch spi0.1: ksz8795_spi_probe:90: ksz_read16(REG_CHIP_ID0, 34705) = 0 > [ 1.701375] ksz8795-switch spi0.1: Switching SPI mode from 3 to spi-cpha,spi-cpol > [ 1.702191] ksz8795-switch spi0.1: ksz8795_spi_probe:84: ksz_read16(REG_CHIP_ID0, 34705) = 0 > [ 1.702928] ksz8795-switch spi0.1: ksz8795_spi_probe:90: ksz_read16(REG_CHIP_ID0, 34705) = 0 > > Maybe the chip needs this read to detect the SPI mode. And if this first > read is the chip detection the initialization fails. > > > > > On Thursday, 19 November 2020 07:48:01 -0600, Rob Hering wrote: > > > > > On Wed, Nov 18, 2020 at 09:30:02PM +0100, Christian Eggers wrote: > > > > ... > > > > > > + ksz9477: switch@0 { > > > > > > + compatible = "microchip,ksz9477"; > > > > > > + reg = <0>; > > > > > > + reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; > > > > > > + > > > > > > + spi-max-frequency = <44000000>; > > > > > > + spi-cpha; > > > > > > + spi-cpol; > > > > > > > > > > Are these 2 optional or required? Being optional is rare as most > > > > > devices support 1 mode, but not unheard of. In general, you shouldn't > > > > > need them as the driver should know how to configure the mode if the h/w > > > > > is fixed. > > > > ... > > > > > > > > It seems that I considered the h/w as "fixed". The pre-existing device tree > > > > bindings and the diagrams on page 53 suggested that SPI mode 3 is the only > > > > valid option. Particularly the idle state of the "SCL" signal is high here: > > > > > > > > https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf > > > > > > > > But the text description on page 52 says something different: > > > > > SCL is expected to stay low when SPI operation is idle. > > > > > > > > Especially the timing diagrams on page 206 look more like SPI mode 0. > > > > > > > > So it is possible that my patch was wrong (due to inconsistent description > > > > on the data sheet / pre existing device tree binding). As I already mentioned, > > > > I did this only due to the DT conversion, I actually don't use SPI on such > > > > devices myself. > > > > > > > > N.B. Which KSZ device do you actually use (I didn't find this in you previous > > > > mails)? > > > > > > I'm using KSZ8795. > > > > I should better read the subject line ... > > > > Summary: > > - The timing diagrams of KSZ8795CLX and KSZ9563 implies that SPI mode 0 is correct > > - The functional descriptions in the datasheets look more like SPI mode 3, but this > > is not authoritative. > > - Maybe that the KSZ devices can work with both modes. > > Or maybe not all ksz8 behave the same? Should we revert the behaviour and > leave it up to the device tree to decide?
On Sun, Jan 05, 2025 at 05:33:38PM +0100, Jörg Sommer wrote: > Hi everyone, > > I've added you to the list of recipients, because you where somehow involved > in changes of the KSZ-SPI switch code. > > We are debating the SPI mode setting for the microchip ksz8795 and ksz9477 > and possibly others. Since the commit > 8c4599f49841dd663402ec52325dc2233add1d32 the SPI mode gets fixed to mode 3 > in the code. But at least my ksz8795 works also with mode 0 and shows better > initialization behaviour with mode 0. > > The big question is: can both (or all ksz) chips work with both modes? > Should this setting stay in code or moved to the device tree? > > The specs are here, but I found no evidence about the supported/recommended > SPI modes: > > https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf Don't trust what i say, i'm not an SPI expert, but i can use grep. https://www.kernel.org/doc/Documentation/spi/spi-summary says: I'm confused. What are these four SPI "clock modes"? ----------------------------------------------------- It's easy to be confused here, and the vendor documentation you'll find isn't necessarily helpful. The four modes combine two mode bits: - CPOL indicates the initial clock polarity. CPOL=0 means the clock starts low, so the first (leading) edge is rising, and the second (trailing) edge is falling. CPOL=1 means the clock starts high, so the first (leading) edge is falling. - CPHA indicates the clock phase used to sample data; CPHA=0 says sample on the leading edge, CPHA=1 means the trailing edge. Since the signal needs to stablize before it's sampled, CPHA=0 implies that its data is written half a clock before the first clock edge. The chipselect may have made it become available. Chip specs won't always say "uses SPI mode X" in as many words, but their timing diagrams will make the CPOL and CPHA modes clear. In the SPI mode number, CPOL is the high order bit and CPHA is the low order bit. So when a chip's timing diagram shows the clock starting low (CPOL=0) and data stabilized for sampling during the trailing clock edge (CPHA=1), that's SPI mode 1. And in the datasheet it says: SCL is expected to stay low when SPI operation is idle. Input data on SDI is latched on the rising edge of serial clock SCL. Output data on SDO is clocked on the falling edge of SCL. My interpretation of this is that the initial clock priority is low, so CPOL=0. The rising edge will be the leading edge, so CPHA=0. So that makes the mode = 0. Can you hard code this in the driver? I guess that depends on if you want to support a PCB that puts in a line driver which adds a NOT gate to the clock? Does that ever happen? I don't know. The real SPI experts should answer that. Andrew
Hi Jörg, all, On 05/01/2025 18:08, Andrew Lunn wrote: > On Sun, Jan 05, 2025 at 05:33:38PM +0100, Jörg Sommer wrote: >> Hi everyone, >> >> I've added you to the list of recipients, because you where somehow involved >> in changes of the KSZ-SPI switch code. >> >> We are debating the SPI mode setting for the microchip ksz8795 and ksz9477 >> and possibly others. Since the commit >> 8c4599f49841dd663402ec52325dc2233add1d32 the SPI mode gets fixed to mode 3 >> in the code. But at least my ksz8795 works also with mode 0 and shows better >> initialization behaviour with mode 0. >> >> The big question is: can both (or all ksz) chips work with both modes? >> Should this setting stay in code or moved to the device tree? >> >> The specs are here, but I found no evidence about the supported/recommended >> SPI modes: >> >> https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf From this KSZ9563 datasheet it is quite clear from Figure 6-9 that it requires mode 0, for KSZ8794 (which I have and can test) Figure 7-8 [1] also indicates mode 0. Note however that older KSZ8794 datasheets (revision DS00002134A from 2016, can upload if needed) rather indicate a mode 3, which is a hint to me that indeed both modes were once supported. Appendix A from [1] states that in 2021 the SPI Timing images and parameters have been updated. No further information there but your experience and the datasheet update seem to indicate mode 0 has better support. My SPI peripheral (and KSZ8 driver) is configured for mode 3 I see but the frequency is set to 10 MHz while the max is 50 MHz. From experience with other SPI devices I know that with higher frequencies the timing parameters (setup and hold times, mode) become more important, what frequency do you have configured Jörg? I'm asking because I don't experience the issue you have. [1] https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8794CNX-Data-Sheet-DS00002134.pdf > > My interpretation of this is that the initial clock priority is low, > so CPOL=0. The rising edge will be the leading edge, so CPHA=0. So > that makes the mode = 0. > Cheers, Pieter
Pieter Van Trappen schrieb am Mo 06. Jan, 14:38 (+0100): > On 05/01/2025 18:08, Andrew Lunn wrote: > > On Sun, Jan 05, 2025 at 05:33:38PM +0100, Jörg Sommer wrote: > > > Hi everyone, > > > > > > I've added you to the list of recipients, because you where somehow involved > > > in changes of the KSZ-SPI switch code. > > > > > > We are debating the SPI mode setting for the microchip ksz8795 and ksz9477 > > > and possibly others. Since the commit > > > 8c4599f49841dd663402ec52325dc2233add1d32 the SPI mode gets fixed to mode 3 > > > in the code. But at least my ksz8795 works also with mode 0 and shows better > > > initialization behaviour with mode 0. > > > > > > The big question is: can both (or all ksz) chips work with both modes? > > > Should this setting stay in code or moved to the device tree? > > > > > > The specs are here, but I found no evidence about the supported/recommended > > > SPI modes: > > > > > > https://ww1.microchip.com/downloads/en/DeviceDoc/KSZ9563R-Data-Sheet-DS00002419D.pdf > > From this KSZ9563 datasheet it is quite clear from Figure 6-9 that it > requires mode 0, for KSZ8794 (which I have and can test) Figure 7-8 [1] also > indicates mode 0. Note however that older KSZ8794 datasheets (revision > DS00002134A from 2016, can upload if needed) rather indicate a mode 3, which > is a hint to me that indeed both modes were once supported. Appendix A from > [1] states that in 2021 the SPI Timing images and parameters have been > updated. No further information there but your experience and the datasheet > update seem to indicate mode 0 has better support. The ksz8795 spec [2] says on page 53, table 4-3, register 11, bit 0 “Trigger on the rising edge of SPI clock (for higher speed SPI)”. The rising edge should mean SPI mode 0. So mode 0 should be recommended for higher speed. [2] https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8795CLX-Data-Sheet-DS00002112.pdf > My SPI peripheral (and KSZ8 driver) is configured for mode 3 I see but the > frequency is set to 10 MHz while the max is 50 MHz. From experience with > other SPI devices I know that with higher frequencies the timing parameters > (setup and hold times, mode) become more important, what frequency do you > have configured Jörg? I'm asking because I don't experience the issue you > have. > > [1] > https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ8794CNX-Data-Sheet-DS00002134.pdf I am using 25MHz which is the maximum for the ksz8795; see [2] page 11, table 2-1, pin 67; page 27, sec "3.5.1.1 SPI Client Serial Bus Configuration" I've tried 10MHz with the code in place that switches to SPI mode 3, but I still get: [ 2.078732] ksz8795-switch spi0.1: invalid family id: 0 [ 2.078808] ksz8795-switch: probe of spi0.1 failed with error -22 I think the main problem is that the chip needs a read access to detect/adjust itself to the new mode. If this is the read for the chip id, this fails. I'm in favour of removing the code, and leave it up to the devicetree to make the decision. Maybe add a note to micrel-ks8995.txt that spi-cpha and spi-cpol are possible. diff --git drivers/net/dsa/microchip/ksz_spi.c drivers/net/dsa/microchip/ksz_spi.c index 108a958dc356..046f2a7d1e08 100644 --- drivers/net/dsa/microchip/ksz_spi.c +++ drivers/net/dsa/microchip/ksz_spi.c @@ -85,12 +85,6 @@ static int ksz_spi_probe(struct spi_device *spi) if (spi->dev.platform_data) dev->pdata = spi->dev.platform_data; - /* setup spi */ - spi->mode = SPI_MODE_3; - ret = spi_setup(spi); - if (ret) - return ret; - dev->irq = spi->irq; ret = ksz_switch_register(dev); Regards, Jörg
> From: Jörg Sommer <joerg@jo-so.de> > Sent: Tuesday, January 7, 2025 2:14 PM > To: Pieter Van Trappen <pieter.van.trappen@cern.ch> > Cc: Christian Eggers <ceggers@arri.de>; Jakub Kicinski <kuba@kernel.org>; Tristram > Ha - C24268 <Tristram.Ha@microchip.com>; Woojung Huh - C21699 > <Woojung.Huh@microchip.com>; netdev@vger.kernel.org; linux-spi@vger.kernel.org; > Andrew Lunn <andrew@lunn.ch> > Subject: Re: KSZ8795 not detected at start to boot from NFS > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content > is safe It is perfectly okay to remove the code as KSZ switches are not limited to SPI mode 3. Generally they work for SPI modes 0, 2, and 3. This SPI access problem is caused by the SPI bus driver. In certain kernel versions the Atmel SPI bus driver has this problem. There is a workaround for that but the fix occurs in the main SPI bus driver, not the Atmel sub-driver. This problem does not occur in latest kernels and I do not know which part of SPI driver fixed the behavior. The problem is the very first SPI access after kernel boots is not recognized by the chip and so it does not return any value. This is probably caused by the SPI chip select signal not setting properly because of polarity. A workaround is to read the chip again or just use SPI mode 0, which starts with low level instead of high level.
diff --git drivers/net/dsa/microchip/ksz8795_spi.c drivers/net/dsa/microchip/ksz8795_spi.c index 8b00f8e6c02f..f98432a3e2b5 100644 --- drivers/net/dsa/microchip/ksz8795_spi.c +++ drivers/net/dsa/microchip/ksz8795_spi.c @@ -49,6 +49,12 @@ static int ksz8795_spi_probe(struct spi_device *spi) if (spi->dev.platform_data) dev->pdata = spi->dev.platform_data; + /* setup spi */ + spi->mode = SPI_MODE_3; + ret = spi_setup(spi); + if (ret) + return ret; + ret = ksz8795_switch_register(dev); /* Main DSA driver may not be started yet. */