[PATCH V2] spi: Update speed/mode on change

[Da Xue]

On Fri, Jul 2, 2021 at 3:06 PM Marek Vasut <marex at denx.de> wrote:

> On 7/2/21 8:28 PM, Da Xue wrote:
> > On Fri, Jul 2, 2021 at 2:10 PM Marek Vasut <marex at denx.de> wrote:
> >
> >> On 7/2/21 8:03 PM, Da Xue wrote:
> >>> SF: Detected gd25lq128 with page size 256 Bytes, erase size 64 KiB,
> total
> >>> 16 MiB
> >>> SPI mode: 3
> >>> meson_spifc spi at 8c80: Cannot set mode (err=-19)
> >>> Failed to initialize SPI flash at 0:0 (error -19)
> >>>
> >>> I added spi-rx-bus-width = <1> and spi-tx-bus-width = <1>
> >>>
> >>> On Fri, Jul 2, 2021 at 1:44 PM Marek Vasut <marex at denx.de> wrote:
> >>>
> >>>> On 7/2/21 7:24 PM, Da Xue wrote:
> >>>>> Hi Marek,
> >>>>>
> >>>>> This patch breaks meson_spifc:
> >>>>>
> >>>>> SF: Detected gd25lq128 with page size 256 Bytes, erase size 64 KiB,
> >> total
> >>>>> 16 MiB
> >>>>> meson_spifc spi at 8c80: Cannot set mode (err=-19)
> >>>>> Failed to initialize SPI flash at 0:0 (error -19)
> >>>>>
> >>>>> Best,
> >>>>>
> >>>>> Da
> >>>>>
> >>>>> On Wed, Jun 30, 2021 at 12:49 PM Tom Rini <trini at konsulko.com>
> wrote:
> >>>>>
> >>>>>> On Thu, Jun 10, 2021 at 02:00:00PM +0200, Marek Vasut wrote:
> >>>>>>
> >>>>>>> The spi_get_bus_and_cs() may be called on the same bus and
> chipselect
> >>>>>>> with different frequency or mode. This is valid usecase, but the
> code
> >>>>>>> fails to notify the controller of such a configuration change. Call
> >>>>>>> spi_set_speed_mode() in case bus frequency or bus mode changed to
> let
> >>>>>>> the controller update the configuration.
> >>>>>>>
> >>>>>>> The problem can easily be triggered using the sspi command:
> >>>>>>> => sspi 0:0 at 1000
> >>>>>>> => sspi 0:0 at 2000
> >>>>>>> Without this patch, both transfers happen at 1000 Hz. With this
> >> patch,
> >>>>>>> the later transfer happens correctly at 2000 Hz.
> >>>>>>>
> >>>>>>> Signed-off-by: Marek Vasut <marex at denx.de>
> >>>>>>> Cc: Jagan Teki <jagan at amarulasolutions.com>
> >>>>>>> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> >>>>>>
> >>>>>> Applied to u-boot/master, thanks!
> >>>>>>
> >>>>>> --
> >>>>>> Tom
> >>>>
> >>>> Seems like you're hitting this code in drivers/spi/meson_spifc.c
> >>>>
> >>>> 250 static int meson_spifc_set_mode(struct udevice *dev, uint mode)
> >>>> 251 {
> >>>> 252         struct meson_spifc_priv *spifc = dev_get_priv(dev);
> >>>> 253
> >>>> 254         if (mode & (SPI_CPHA | SPI_RX_QUAD | SPI_RX_DUAL |
> >>>> 255                     SPI_TX_QUAD | SPI_TX_DUAL))
> >>>> 256                 return -ENODEV;
> >>>>
> >>>> (the -ENODEV code doesn't look right, it should be some -EOPNOTSUP or
> >> so)
> >>>>
> >>>> Can you check which of the mode bits is set and triggers the
> condition ?
> >>>>
> >>>> I think you might be missing something like
> >>>> spi-rx-bus-width = <1>;
> >>>> spi-tx-bus-width = <1>;
> >>>> in your DT, but that's a guess.
> >>
> >> Can you check which of the mode bits is set and triggers the condition ?
> >>
> >> Also, where in the DT did you add spi-rx-bus-width = <1> and
> >> spi-tx-bus-width = <1> ?
> >>
> >> Finally, please do not top-post and keep the list on CC.
> >>
> >
> > My apologies about the top-posting.
> >
> > --- a/arch/arm/dts/meson-gxl-s805x-libretech-ac.dts
> > +++ b/arch/arm/dts/meson-gxl-s805x-libretech-ac.dts
> > @@ -304,6 +304,8 @@
> >                  compatible = "jedec,spi-nor";
> >                  reg = <0>;
> >                  spi-max-frequency = <80000000>;
> > +               spi-rx-bus-width = <1>;
> > +               spi-tx-bus-width = <1>;
> >          };
> >   };
>
> That should do the trick. Can you check which of the mode bits is set in
> meson_spifc_set_mode() and triggers the ENODEV condition ?
>

SPI_CPHA seems to be  the culprit. I tried adding spi-cpha = <0> to no
avail.