[regression] jetson-tk1: spi do not probe anymore

[regression] jetson-tk1: spi do not probe anymore

[Corentin Labbe]

Hello

On my jetson-tk1, SPI do not probe anymore:
[    1.330681] spi spi1.0: Invalid delay unit 2, should be SPI_DELAY_UNIT_SCK
[    1.335185] spi-tegra114 7000da00.spi: can't setup spi1.0, status -22
[    1.341643] spi_master spi1: spi_device register error /spi@7000da00/flash@0
[    1.348637] spi_master spi1: Failed to create SPI device for /spi@7000da00/flash@0
I tested 6.14.7
The SPI probed perfectly in 4.17.14

I tried to debug a bit, and the driver requires units to be SPI_DELAY_UNIT_SCK, but it seems there is no way to set it.
Removing the "return -EINVAL" in tegra_spi_set_hw_cs_timing() lead to a successfull probe and the flash device appear.
But I agree, it is not a correct fix:)

Since only the test made it fail, I think the driver is bad since commit 810593668468 ("spi: tegra114: change format for `spi_set_cs_timing()` function")

Regards

Re: [regression] jetson-tk1: spi do not probe anymore

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

On Sun, May 04, 2025 at 01:23:29PM +0200, Corentin Labbe wrote:

> On my jetson-tk1, SPI do not probe anymore:
> [    1.330681] spi spi1.0: Invalid delay unit 2, should be SPI_DELAY_UNIT_SCK
> [    1.335185] spi-tegra114 7000da00.spi: can't setup spi1.0, status -22
> [    1.341643] spi_master spi1: spi_device register error /spi@7000da00/flash@0
> [    1.348637] spi_master spi1: Failed to create SPI device for /spi@7000da00/flash@0
> I tested 6.14.7
> The SPI probed perfectly in 4.17.14

That's a pretty big jump in versions...

> I tried to debug a bit, and the driver requires units to be SPI_DELAY_UNIT_SCK, but it seems there is no way to set it.
> Removing the "return -EINVAL" in tegra_spi_set_hw_cs_timing() lead to a successfull probe and the flash device appear.
> But I agree, it is not a correct fix:)
> 
> Since only the test made it fail, I think the driver is bad since commit 810593668468 ("spi: tegra114: change format for `spi_set_cs_timing()` function")

Adding Alexandru who wrote that commit.  Assuming the delays came from
DT (I'm not actually finding them, I didn't look too hard though) and
are therefore in units of wall clock time I think we need to add a
conversion helper for the units which the driver can use to convert to
whatever units it actually wants to use, we do need to accept units of
wall clock time given the generic binding there.  That gets a bit
annoying if the bus speed changes, though that's quite infrequent in
practice.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [regression] jetson-tk1: spi do not probe anymore

[Aaron Kling]

On Sun, May 4, 2025 at 5:37 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Sun, May 04, 2025 at 01:23:29PM +0200, Corentin Labbe wrote:
>
> > On my jetson-tk1, SPI do not probe anymore:
> > [    1.330681] spi spi1.0: Invalid delay unit 2, should be SPI_DELAY_UNIT_SCK
> > [    1.335185] spi-tegra114 7000da00.spi: can't setup spi1.0, status -22
> > [    1.341643] spi_master spi1: spi_device register error /spi@7000da00/flash@0
> > [    1.348637] spi_master spi1: Failed to create SPI device for /spi@7000da00/flash@0
> > I tested 6.14.7
> > The SPI probed perfectly in 4.17.14
>
> That's a pretty big jump in versions...
>
> > I tried to debug a bit, and the driver requires units to be SPI_DELAY_UNIT_SCK, but it seems there is no way to set it.
> > Removing the "return -EINVAL" in tegra_spi_set_hw_cs_timing() lead to a successfull probe and the flash device appear.
> > But I agree, it is not a correct fix:)
> >
> > Since only the test made it fail, I think the driver is bad since commit 810593668468 ("spi: tegra114: change format for `spi_set_cs_timing()` function")
>
> Adding Alexandru who wrote that commit.  Assuming the delays came from
> DT (I'm not actually finding them, I didn't look too hard though) and
> are therefore in units of wall clock time I think we need to add a
> conversion helper for the units which the driver can use to convert to
> whatever units it actually wants to use, we do need to accept units of
> wall clock time given the generic binding there.  That gets a bit
> annoying if the bus speed changes, though that's quite infrequent in
> practice.

This should be fixed by a patch [0] which was recently picked up. I
saw the same issue on a tegra 210 device and submitted that to fix it.
Can you verify if it fixes this case too?

Sincerely,
Aaron

[0] https://lore.kernel.org/all/20250423-spi-tegra114-v1-1-2d608bcc12f9@gmail.com/

Re: [regression] jetson-tk1: spi do not probe anymore

[Corentin Labbe]

Le Sun, May 04, 2025 at 09:10:59PM -0500, Aaron Kling a écrit :
> On Sun, May 4, 2025 at 5:37 PM Mark Brown <broonie@kernel.org> wrote:
> >
> > On Sun, May 04, 2025 at 01:23:29PM +0200, Corentin Labbe wrote:
> >
> > > On my jetson-tk1, SPI do not probe anymore:
> > > [    1.330681] spi spi1.0: Invalid delay unit 2, should be SPI_DELAY_UNIT_SCK
> > > [    1.335185] spi-tegra114 7000da00.spi: can't setup spi1.0, status -22
> > > [    1.341643] spi_master spi1: spi_device register error /spi@7000da00/flash@0
> > > [    1.348637] spi_master spi1: Failed to create SPI device for /spi@7000da00/flash@0
> > > I tested 6.14.7
> > > The SPI probed perfectly in 4.17.14
> >
> > That's a pretty big jump in versions...
> >
> > > I tried to debug a bit, and the driver requires units to be SPI_DELAY_UNIT_SCK, but it seems there is no way to set it.
> > > Removing the "return -EINVAL" in tegra_spi_set_hw_cs_timing() lead to a successfull probe and the flash device appear.
> > > But I agree, it is not a correct fix:)
> > >
> > > Since only the test made it fail, I think the driver is bad since commit 810593668468 ("spi: tegra114: change format for `spi_set_cs_timing()` function")
> >
> > Adding Alexandru who wrote that commit.  Assuming the delays came from
> > DT (I'm not actually finding them, I didn't look too hard though) and
> > are therefore in units of wall clock time I think we need to add a
> > conversion helper for the units which the driver can use to convert to
> > whatever units it actually wants to use, we do need to accept units of
> > wall clock time given the generic binding there.  That gets a bit
> > annoying if the bus speed changes, though that's quite infrequent in
> > practice.
> 
> This should be fixed by a patch [0] which was recently picked up. I
> saw the same issue on a tegra 210 device and submitted that to fix it.
> Can you verify if it fixes this case too?
> 
> Sincerely,
> Aaron
> 
> [0] https://lore.kernel.org/all/20250423-spi-tegra114-v1-1-2d608bcc12f9@gmail.com/

Yes it fixes my case

Thanks
Regards

Re: [regression] jetson-tk1: spi do not probe anymore

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On Sun, May 04, 2025 at 09:10:59PM -0500, Aaron Kling wrote:

> This should be fixed by a patch [0] which was recently picked up. I
> saw the same issue on a tegra 210 device and submitted that to fix it.
> Can you verify if it fixes this case too?

For those playing at home that's "spi: tegra114: Don't fail
set_cs_timing when delays are zero" - that explains why I couldn't find
where the delays were set.

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]