Re: [PATCH V2] spi: Update speed/mode on change
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@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@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@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@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@1000 => sspi 0:0@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@denx.de Cc: Jagan Teki jagan@amarulasolutions.com Cc: Patrick Delaunay patrick.delaunay@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.
On Fri, Jul 2, 2021 at 2:10 PM Marek Vasut marex@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@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@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@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@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@1000 => sspi 0:0@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@denx.de Cc: Jagan Teki jagan@amarulasolutions.com Cc: Patrick Delaunay patrick.delaunay@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>; }; };
On 7/2/21 8:28 PM, Da Xue wrote:
On Fri, Jul 2, 2021 at 2:10 PM Marek Vasut marex@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@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@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@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@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@1000 > => sspi 0:0@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@denx.de > Cc: Jagan Teki jagan@amarulasolutions.com > Cc: Patrick Delaunay patrick.delaunay@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 ?
On Fri, Jul 2, 2021 at 3:06 PM Marek Vasut marex@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@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@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@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@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@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@1000 >> => sspi 0:0@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@denx.de >> Cc: Jagan Teki jagan@amarulasolutions.com >> Cc: Patrick Delaunay patrick.delaunay@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.
On 7/2/21 9:35 PM, Da Xue wrote:
[...]
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.
Can you find out what is setting the SPI_CPHA in the first place on your machine ?
On Fri, Jul 2, 2021 at 3:40 PM Marek Vasut marex@denx.de wrote:
On 7/2/21 9:35 PM, Da Xue wrote:
[...]
> 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.
Can you find out what is setting the SPI_CPHA in the first place on your machine ?
The Kconfig was setting it to 3. I manually added the default mode (0) to my board's config and it worked. Thanks Marek.
On 7/2/21 10:10 PM, Da Xue wrote: [...]
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.
Can you find out what is setting the SPI_CPHA in the first place on your machine ?
The Kconfig was setting it to 3. I manually added the default mode (0) to my board's config and it worked. Thanks Marek.
Sure, good thing this was fixed. Please send a patch for the board, so it can be added to this release.
participants (2)
-
Da Xue -
Marek Vasut