| 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
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. */
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> 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 ``` dev_err(&spi->dev, "ksz8795_spi_probe:%d: ret of ksz8795_switch_register=%d\n", __LINE__, ret); ``` reveals. Without 8c4599f49841dd663402ec52325dc2233add1d32: ``` [ 1.809914] ksz8795-switch spi0.1: ksz8795_spi_probe:60: ret of ksz8795_switch_register=-517 … [ 2.069770] ksz8795-switch spi0.1: ksz8795_spi_probe:60: ret of ksz8795_switch_register=-517 [ 2.209715] ksz8795-switch spi0.1: ksz8795_spi_probe:60: ret of ksz8795_switch_register=-517 [ 2.223625] davinci_emac 1e20000.ethernet: incompatible machine/device type for reading mac address [ 3.110937] ksz8795-switch spi0.1 lan-x1 (uninitialized): PHY [dsa-0.0:00] driver [Generic PHY] (irq=POLL) [ 3.125766] ksz8795-switch spi0.1 lan-x2 (uninitialized): PHY [dsa-0.0:01] driver [Generic PHY] (irq=POLL) [ 3.140779] ksz8795-switch spi0.1 lan-x3 (uninitialized): PHY [dsa-0.0:02] driver [Generic PHY] (irq=POLL) [ 3.155026] ksz8795-switch spi0.1 lan-bp (uninitialized): PHY [dsa-0.0:03] driver [Generic PHY] (irq=POLL) [ 3.168579] ksz8795-switch spi0.1: configuring for fixed/rmii link mode [ 3.175324] eth0: mtu greater than device maximum [ 3.180273] davinci_emac 1e20000.ethernet eth0: error -22 setting MTU to include DSA overhead [ 3.189075] DSA: tree 0 setup [ 3.192155] ksz8795-switch spi0.1: ksz8795_spi_probe:60: ret of ksz8795_switch_register=0 [ 3.204819] device eth0 entered promiscuous mode [ 3.209962] ksz8795-switch spi0.1: Link is Up - 100Mbps/Full - flow control rx/tx [ 3.221219] davinci_emac 1e20000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off [ 3.232894] ksz8795-switch spi0.1 lan-x1: configuring for phy/gmii link mode [ 6.409818] ksz8795-switch spi0.1 lan-x1: Link is Up - 100Mbps/Full - flow control rx/tx ``` With 8c4599f49841dd663402ec52325dc2233add1d32: ``` [ 1.808910] ksz8795-switch spi0.1: ksz8795_spi_probe:60: ret of ksz8795_switch_register=-22 [ 1.817612] ksz8795-switch: probe of spi0.1 failed with error -22 [ 1.823992] spi_davinci 1f0e000.spi: Controller at 0x(ptrval) … [ 1.952350] davinci_emac 1e20000.ethernet: incompatible machine/device type for reading mac address ``` So, I inserted these lines and it works. ``` ret = ksz8_switch_register(dev); + if (ret == -EINVAL || ret == -ENODEV) + ret = -EPROBE_DEFER; ``` Is this the correct solution or should it be done something different? Something like this is still missing in ksz_spi.c:88-100. Thanks in advance. Kind regards, Jörg