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

[Marek Vasut <marex@denx.de>]

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.