RE: [PATCH net] net: dsa: microchip: Select SPI_MODE 0 for KSZ8463

[Tristram.Ha]

> On 9/11/25 12:10 AM, Tristram.Ha@xxxxxxxxxxxxx wrote:
> >> KSZ8463 expects the SPI clock to be low on idle and samples data on
> >> rising edges. This fits SPI mode 0 (CPOL = 0 / CPHA = 0) but the SPI
> >> mode is set to 3 for all the switches supported by the driver. This
> >> can lead to invalid read/write on the SPI bus.
> >>
> >> Set SPI mode to 0 for the KSZ8463.
> >> Leave SPI mode 3 as default for the other switches.
> >>
> >> Signed-off-by: Bastien Curutchet (Schneider Electric)
> >> <bastien.curutchet@xxxxxxxxxxx>
> >> Fixes: 84c47bfc5b3b ("net: dsa: microchip: Add KSZ8463 switch support to KSZ DSA
> >> driver")
> >> ---
> >>   drivers/net/dsa/microchip/ksz_spi.c | 7 +++++--
> >>   1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/dsa/microchip/ksz_spi.c
> >> b/drivers/net/dsa/microchip/ksz_spi.c
> >> index
> >>
> d8001734b05741446fa78a1e88c2f82e894835ce..dcc0dbddf7b9d70fbfb31d4b260b80
> >> ca78a65975 100644
> >> --- a/drivers/net/dsa/microchip/ksz_spi.c
> >> +++ b/drivers/net/dsa/microchip/ksz_spi.c
> >> @@ -139,6 +139,7 @@ static int ksz_spi_probe(struct spi_device *spi)
> >>          const struct regmap_config *regmap_config;
> >>          const struct ksz_chip_data *chip;
> >>          struct device *ddev = &spi->dev;
> >> +       u32 spi_mode = SPI_MODE_3;
> >>          struct regmap_config rc;
> >>          struct ksz_device *dev;
> >>          int i, ret = 0;
> >> @@ -155,8 +156,10 @@ static int ksz_spi_probe(struct spi_device *spi)
> >>          dev->chip_id = chip->chip_id;
> >>          if (chip->chip_id == KSZ88X3_CHIP_ID)
> >>                  regmap_config = ksz8863_regmap_config;
> >> -       else if (chip->chip_id == KSZ8463_CHIP_ID)
> >> +       else if (chip->chip_id == KSZ8463_CHIP_ID) {
> >>                  regmap_config = ksz8463_regmap_config;
> >> +               spi_mode = SPI_MODE_0;
> >> +       }
> >>          else if (chip->chip_id == KSZ8795_CHIP_ID ||
> >>                   chip->chip_id == KSZ8794_CHIP_ID ||
> >>                   chip->chip_id == KSZ8765_CHIP_ID)
> >> @@ -185,7 +188,7 @@ static int ksz_spi_probe(struct spi_device *spi)
> >>                  dev->pdata = spi->dev.platform_data;
> >>
> >>          /* setup spi */
> >> -       spi->mode = SPI_MODE_3;
> >> +       spi->mode = spi_mode;
> >>          ret = spi_setup(spi);
> >>          if (ret)
> >>                  return ret;
> >>
> >> ---
> >> base-commit: c65e2aee8971eb9d4bc2b8edc3a3a62dc98f0410
> >> change-id: 20250910-fix-omap-spi-d7c64f2416df
> >
> > Actually it is best to completely remove the code.  The SPI mode should
> > be dictated by spi-cpol and spi-cpha settings in the device tree.  I do
> > not know why that code was there from the beginning.
> >
> 
> Ok, I didn't know these settings were available on the device-tree, I
> can remove the spi->mode setting in a new patch.
> 
> > All KSZ switches can use SPI mode 0 and 3, and 3 is recommended for high
> > SPI frequency.  Sometimes a bug/quirk in the SPI bus driver prevents the
> > very first SPI transfer to be successful in mode 3 because of a missed
> > rising edge clock signal, so it is forced to use mode 0.  (The Atmel SPI
> > bus driver has this issue in some old kernel versions.)
> >
> > As for KSZ8463 I have always used mode 3 and do not know of any issue of
> > using that mode.
> >
> 
> I have issues on the first transfer with the AM335x's spi-omap2-mcspi
> driver. I first tried to fix this driver but since the KSZ8463's
> datasheet explicitly mentions that it expects the CLK to be low at idle,
> I thought this was the right fix.
> 
> But I'll fix the SPI driver then, thanks.

It seems you may have the same issue I faced before.  A workaround in the
SPI bus driver may avoid the problem, but the change happens in spi.c, so
it may not be a good solution.

Your best bet is to remove the spi_setup call and use mode 0 in the
device tree.

Another quick fix is to do a dummy write in the DSA driver before reading
and checking the chip id.