[PATCH 0/3] Add spidev nodes for SPI controllers

[PATCH 0/3] Add spidev nodes for SPI controllers

[Vishwaroop A]

The patchset contains spidev node support for SPI Controllers, adds
DT schema and "tegra-spidev" name string in the spidev driver.

Vishwaroop A (3):
  arm64: tegra: Add spidev nodes for SPI controllers
  dt-bindings: spi: Add DT schema for Tegra SPIDEV controller
  spi: spidev: add "tegra-spidev" name string

 .../devicetree/bindings/trivial-devices.yaml  |  2 ++
 .../dts/nvidia/tegra234-p3737-0000+p3701.dtsi | 28 +++++++++++++++++++
 drivers/spi/spidev.c                          |  2 ++
 3 files changed, 32 insertions(+)

-- 
2.17.1

[PATCH 1/3] arm64: tegra: Add spidev nodes for SPI controllers

[Vishwaroop A]

Add spidev nodes to two existing SPI controllers on the
NVIDIA Tegra234 SoC. These additions enable user-space
SPI device support for testing and development purposes.
Specifies a maximum frequency of 81MHz for each spidev node.

The 81MHz maximum frequency is defined to ensure stable operation
while allowing for high-speed SPI communication. This frequency
can be adjusted based on specific hardware capabilities and
requirements.

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 .../dts/nvidia/tegra234-p3737-0000+p3701.dtsi | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
index f6cad29355e6..17de3d0feb45 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
@@ -84,6 +84,34 @@
 			};
 		};
 
+		spi@3210000 {
+			status = "okay";
+			spi@0 {
+				compatible = "nvidia,tegra-spidev";
+				reg = <0x0>;
+				spi-max-frequency = <81000000>;
+			};
+			spi@1 {
+				compatible = "nvidia,tegra-spidev";
+				reg = <0x1>;
+				spi-max-frequency = <81000000>;
+			};
+		};
+
+		spi@3230000 {
+			status = "okay";
+			spi@0 {
+				compatible = "nvidia,tegra-spidev";
+				reg = <0x0>;
+				spi-max-frequency = <81000000>;
+			};
+			spi@1 {
+				compatible = "nvidia,tegra-spidev";
+				reg = <0x1>;
+				spi-max-frequency = <81000000>;
+			};
+		};
+
 		pwm@3280000 {
 			status = "okay";
 		};
-- 
2.17.1

[PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Vishwaroop A]

Add DeviceTree schema for NVIDIA'S SPIDEV controller.
The schema supports both master and slave modes for data transfer
purposes. Specifies required properties: "compatible", "reg", and
"spi-max-frequency".

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 88abb5c174f3..c8b39a513fc5 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -293,6 +293,8 @@ properties:
           - national,lm85
             # I2C ±0.33°C Accurate, 12-Bit + Sign Temperature Sensor and Thermal Window Comparator
           - national,lm92
+            # NVIDIA Tegra SPIDEV Controller device
+          - nvidia,tegra-spidev
             # Nuvoton Temperature Sensor
           - nuvoton,w83773g
             # OKI ML86V7667 video decoder
-- 
2.17.1

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Tue, Nov 26, 2024 at 01:45:28PM +0000, Vishwaroop A wrote:

> +            # NVIDIA Tegra SPIDEV Controller device
> +          - nvidia,tegra-spidev

All the issues with having spidev nodes directly in the DT also apply if
you add a prefix onto it.  Whatever is attached to the SPI controller
needs to be described, not just a placeholder like this.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Jon Hunter]

Hi Mark, Krzysztof,

On 26/11/2024 13:56, Mark Brown wrote:
> On Tue, Nov 26, 2024 at 01:45:28PM +0000, Vishwaroop A wrote:
> 
>> +            # NVIDIA Tegra SPIDEV Controller device
>> +          - nvidia,tegra-spidev
> 
> All the issues with having spidev nodes directly in the DT also apply if
> you add a prefix onto it.  Whatever is attached to the SPI controller
> needs to be described, not just a placeholder like this.


Chatting with Vishwaroop, what he is trying to accomplish with this 
patchset it to have a pseudo SPI device that we can control from userspace.

On the Tegra Jetson boards we have a 40-pin expansion header similar to 
what is found on boards like Raspberry Pi and allows users to connect 
various cards to. By having a pseudo device we can interact with 
different SPI devices.

Yes by default nothing is connected and so there is nothing to talk to. 
However, this does enable us to do SPI loopback testing for example.

So I am wondering if it would be acceptable to having some generic dummy 
device-tree compatible string for this? I guess it does not need to be 
Tegra specific.

Thanks
Jon

-- 
nvpublic

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Wed, Nov 27, 2024 at 03:54:52PM +0000, Jon Hunter wrote:

> On the Tegra Jetson boards we have a 40-pin expansion header similar to what
> is found on boards like Raspberry Pi and allows users to connect various
> cards to. By having a pseudo device we can interact with different SPI
> devices.

> Yes by default nothing is connected and so there is nothing to talk to.
> However, this does enable us to do SPI loopback testing for example.

> So I am wondering if it would be acceptable to having some generic dummy
> device-tree compatible string for this? I guess it does not need to be Tegra
> specific.

I understand what he's trying to accomplish, it's the same thing as
what everyone who wants to put a raw spidev compatible in their DT is
trying to do.  The way to do this would be something like a DT overlay
that describes whatever is actually connected, or just customise the DT
locally.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Jon Hunter]

On 27/11/2024 16:09, Mark Brown wrote:
> On Wed, Nov 27, 2024 at 03:54:52PM +0000, Jon Hunter wrote:
> 
>> On the Tegra Jetson boards we have a 40-pin expansion header similar to what
>> is found on boards like Raspberry Pi and allows users to connect various
>> cards to. By having a pseudo device we can interact with different SPI
>> devices.
> 
>> Yes by default nothing is connected and so there is nothing to talk to.
>> However, this does enable us to do SPI loopback testing for example.
> 
>> So I am wondering if it would be acceptable to having some generic dummy
>> device-tree compatible string for this? I guess it does not need to be Tegra
>> specific.
> 
> I understand what he's trying to accomplish, it's the same thing as
> what everyone who wants to put a raw spidev compatible in their DT is
> trying to do.  The way to do this would be something like a DT overlay
> that describes whatever is actually connected, or just customise the DT
> locally.

We could certainly use an overlay, but how do we handle the kernel side? 
My understanding is that per patch 3/3 we need to reference a compatible 
string the kernel is aware of. I guess we could use an existing one, but 
feels like a massive hack. It would be nice if there is something 
generic we can use for this like 'linux,spidev'.

I see that ACPI has something and it does print a warning that this 
should not be used in production systems.

Jon

-- 
nvpublic

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Wed, Nov 27, 2024 at 05:24:01PM +0000, Jon Hunter wrote:
> On 27/11/2024 16:09, Mark Brown wrote:

> > I understand what he's trying to accomplish, it's the same thing as
> > what everyone who wants to put a raw spidev compatible in their DT is
> > trying to do.  The way to do this would be something like a DT overlay
> > that describes whatever is actually connected, or just customise the DT
> > locally.

> We could certainly use an overlay, but how do we handle the kernel side? My
> understanding is that per patch 3/3 we need to reference a compatible string
> the kernel is aware of. I guess we could use an existing one, but feels like
> a massive hack. It would be nice if there is something generic we can use
> for this like 'linux,spidev'.

> I see that ACPI has something and it does print a warning that this should
> not be used in production systems.

You can put 'spidev' in as the compatible and get the warning, we don't
require specific compatibles if the Linux device ID is good enough.  If
you genuinely just have bare wires you're probably able to cope with the
warning.  If something is actually connected you should use the
compatible for whatever that is, if spidev makes sense for it then
that'd be OK to add to spidev.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Jon Hunter]

Hi Mark,

On 27/11/2024 17:31, Mark Brown wrote:
> On Wed, Nov 27, 2024 at 05:24:01PM +0000, Jon Hunter wrote:
>> On 27/11/2024 16:09, Mark Brown wrote:
> 
>>> I understand what he's trying to accomplish, it's the same thing as
>>> what everyone who wants to put a raw spidev compatible in their DT is
>>> trying to do.  The way to do this would be something like a DT overlay
>>> that describes whatever is actually connected, or just customise the DT
>>> locally.
> 
>> We could certainly use an overlay, but how do we handle the kernel side? My
>> understanding is that per patch 3/3 we need to reference a compatible string
>> the kernel is aware of. I guess we could use an existing one, but feels like
>> a massive hack. It would be nice if there is something generic we can use
>> for this like 'linux,spidev'.
> 
>> I see that ACPI has something and it does print a warning that this should
>> not be used in production systems.
> 
> You can put 'spidev' in as the compatible and get the warning, we don't
> require specific compatibles if the Linux device ID is good enough.  If
> you genuinely just have bare wires you're probably able to cope with the
> warning.  If something is actually connected you should use the
> compatible for whatever that is, if spidev makes sense for it then
> that'd be OK to add to spidev.


We finally got back to this. Looks like just having 'spidev' as the 
compatible does not work. Apparently, it use to work and yes you would 
get the warning, but that no longer seems to be the case. I see a few 
others have been doing similar things and hacking their device-trees in 
different ways [0].

I completely agree that ideally we would have a proper compatible string 
for this because after all device-tree describes the hardware. One 
use-case that we use is external loop back for verifying SPI by simply 
connecting MOSI to the MISO. Would it be acceptable to have a compatible 
string for external loopback connections?

Thanks
Jon

[0] 
https://stackoverflow.com/questions/53634892/linux-spidev-why-it-shouldnt-be-directly-in-devicetree

-- 
nvpublic

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Tue, Mar 25, 2025 at 10:36:29AM +0000, Jon Hunter wrote:
> On 27/11/2024 17:31, Mark Brown wrote:

> > You can put 'spidev' in as the compatible and get the warning, we don't
> > require specific compatibles if the Linux device ID is good enough.  If
> > you genuinely just have bare wires you're probably able to cope with the
> > warning.  If something is actually connected you should use the
> > compatible for whatever that is, if spidev makes sense for it then
> > that'd be OK to add to spidev.

> We finally got back to this. Looks like just having 'spidev' as the
> compatible does not work. Apparently, it use to work and yes you would get
> the warning, but that no longer seems to be the case. I see a few others
> have been doing similar things and hacking their device-trees in different
> ways [0].

Huh, OK.  I don't recall any deliberate SPI change for that.

> I completely agree that ideally we would have a proper compatible string for
> this because after all device-tree describes the hardware. One use-case that
> we use is external loop back for verifying SPI by simply connecting MOSI to
> the MISO. Would it be acceptable to have a compatible string for external
> loopback connections?

That sounds fine.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Thierry Reding]

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

On Tue, Mar 25, 2025 at 12:10:19PM +0000, Mark Brown wrote:
> On Tue, Mar 25, 2025 at 10:36:29AM +0000, Jon Hunter wrote:
> > On 27/11/2024 17:31, Mark Brown wrote:
> 
> > > You can put 'spidev' in as the compatible and get the warning, we don't
> > > require specific compatibles if the Linux device ID is good enough.  If
> > > you genuinely just have bare wires you're probably able to cope with the
> > > warning.  If something is actually connected you should use the
> > > compatible for whatever that is, if spidev makes sense for it then
> > > that'd be OK to add to spidev.
> 
> > We finally got back to this. Looks like just having 'spidev' as the
> > compatible does not work. Apparently, it use to work and yes you would get
> > the warning, but that no longer seems to be the case. I see a few others
> > have been doing similar things and hacking their device-trees in different
> > ways [0].
> 
> Huh, OK.  I don't recall any deliberate SPI change for that.

People in the discussion that Jon linked to indicated that it was this
patch that caused the "regression":

commit 6840615f85f6046039ebc4989870ddb12892b7fc
Author: Mark Brown <broonie@kernel.org>
Date:   Thu Sep 23 18:00:23 2021 +0100
    spi: spidev: Add SPI ID table
    
    Currently autoloading for SPI devices does not use the DT ID table, it uses
    SPI modalises. Supporting OF modalises is going to be difficult if not
    impractical, an attempt was made but has been reverted, so ensure that
    module autoloading works for this driver by adding an id_table listing the
    SPI IDs for everything.
    
    Fixes: 96c8395e2166 ("spi: Revert modalias changes")
    Signed-off-by: Mark Brown <broonie@kernel.org>
    Link: https://lore.kernel.org/r/20210923170023.1683-1-broonie@kernel.org
    Signed-off-by: Mark Brown <broonie@kernel.org>

If you say that the regression wasn't deliberate, maybe we should look
at fixing this so that people don't have to work around stuff?

Thierry

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Thierry Reding]

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

On Tue, Mar 25, 2025 at 01:45:26PM +0100, Thierry Reding wrote:
> On Tue, Mar 25, 2025 at 12:10:19PM +0000, Mark Brown wrote:
> > On Tue, Mar 25, 2025 at 10:36:29AM +0000, Jon Hunter wrote:
> > > On 27/11/2024 17:31, Mark Brown wrote:
> > 
> > > > You can put 'spidev' in as the compatible and get the warning, we don't
> > > > require specific compatibles if the Linux device ID is good enough.  If
> > > > you genuinely just have bare wires you're probably able to cope with the
> > > > warning.  If something is actually connected you should use the
> > > > compatible for whatever that is, if spidev makes sense for it then
> > > > that'd be OK to add to spidev.
> > 
> > > We finally got back to this. Looks like just having 'spidev' as the
> > > compatible does not work. Apparently, it use to work and yes you would get
> > > the warning, but that no longer seems to be the case. I see a few others
> > > have been doing similar things and hacking their device-trees in different
> > > ways [0].
> > 
> > Huh, OK.  I don't recall any deliberate SPI change for that.
> 
> People in the discussion that Jon linked to indicated that it was this
> patch that caused the "regression":
> 
> commit 6840615f85f6046039ebc4989870ddb12892b7fc
> Author: Mark Brown <broonie@kernel.org>
> Date:   Thu Sep 23 18:00:23 2021 +0100
>     spi: spidev: Add SPI ID table
>     
>     Currently autoloading for SPI devices does not use the DT ID table, it uses
>     SPI modalises. Supporting OF modalises is going to be difficult if not
>     impractical, an attempt was made but has been reverted, so ensure that
>     module autoloading works for this driver by adding an id_table listing the
>     SPI IDs for everything.
>     
>     Fixes: 96c8395e2166 ("spi: Revert modalias changes")
>     Signed-off-by: Mark Brown <broonie@kernel.org>
>     Link: https://lore.kernel.org/r/20210923170023.1683-1-broonie@kernel.org
>     Signed-off-by: Mark Brown <broonie@kernel.org>
> 
> If you say that the regression wasn't deliberate, maybe we should look
> at fixing this so that people don't have to work around stuff?

Hm... there's also this:

commit f6f6a6320eeeb3e80e1393f727f898f8ca976bfd
Author: Javier Martinez Canillas <javierm@redhat.com>
Date:   Fri Nov 19 13:11:39 2021 +0100
    spi: docs: improve the SPI userspace API documentation

    This doc is fairly outdated and only uses legacy device instantiation
    terminology. Let us update it and also mention the OF and ACPI device
    tables, to make easier for users to figure out how should be defined.

    Also, mention that devices bind could be done in user-space now using
    the "driver_override" sysfs entry.

    Suggested-by: Ralph Siemsen <ralph.siemsen@linaro.org>
    Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
    Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
    Link: https://lore.kernel.org/r/20211119121139.2412761-1-javierm@redhat.com
    Signed-off-by: Mark Brown <broonie@kernel.org>

That explicitly says that the linux,spidev or spidev compatible strings
are no longer supported, but also mentions the sysfs interface to bind
spidev to a given device.

Jon, I take it that the current sysfs is not enough for our use-case
because it only works if a device is predefined (i.e. the sysfs doesn't
allow specifying the chipselect)? Maybe we could enhance sysfs to allow
adding a generic spidev client, similar to how other busses (USB, PCI, I
think) allow using configfs to create these devices at runtime.

I2C has a different mechanism that could perhaps be used to achieve
something similar, except that it exposes each controller as a device
with a set of defined IOCTLs. That would be another option, but given
that there's already a spidev interface that doesn't seem like a good
fit for SPI.

Mark, would another alternative be to add something like a sysfs export
attribute? Something that you'd write a controller/chipselect pair to in
order to create a spidev device? That has the benefit of removing this
entirely from device tree where it doesn't belong, but still makes this
option available to users that would otherwise have to resort to hacks.

Thierry

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Tue, Mar 25, 2025 at 02:05:01PM +0100, Thierry Reding wrote:

> Mark, would another alternative be to add something like a sysfs export
> attribute? Something that you'd write a controller/chipselect pair to in
> order to create a spidev device? That has the benefit of removing this
> entirely from device tree where it doesn't belong, but still makes this
> option available to users that would otherwise have to resort to hacks.

Possibly?  I think I've lost track of what the use case is here, usually
for the spidev stuff DT overlays seem like they're the right thing but
perhaps this is different?  If we are doing this at runtime sysfs seems
like a reasonable way to trigger it, though you'd still need the DT to
describe the controller and the chipselects that are available.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Thierry Reding]

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

On Tue, Mar 25, 2025 at 03:55:02PM +0000, Mark Brown wrote:
> On Tue, Mar 25, 2025 at 02:05:01PM +0100, Thierry Reding wrote:
> 
> > Mark, would another alternative be to add something like a sysfs export
> > attribute? Something that you'd write a controller/chipselect pair to in
> > order to create a spidev device? That has the benefit of removing this
> > entirely from device tree where it doesn't belong, but still makes this
> > option available to users that would otherwise have to resort to hacks.
> 
> Possibly?  I think I've lost track of what the use case is here, usually
> for the spidev stuff DT overlays seem like they're the right thing but
> perhaps this is different?  If we are doing this at runtime sysfs seems
> like a reasonable way to trigger it, though you'd still need the DT to
> describe the controller and the chipselects that are available.

Heh... it's exactly the opposite for me. I feel like I don't understand
the need for spidev with a specific compatible string. If you've got a
compatible string (or in other words you have a device with a very
specific SPI chip connected), then why would you want to access it from
userspace? Isn't a proper kernel driver the better option in most cases?
That usually allows for better abstraction via some other subsystem. I
suppose there are cases where there may not be a subsystem, or for other
reasons it's more convenient to have direct access from userspace to
avoid the roundtrip. Or maybe users could be concerned about safety?

I think the more common use-case is for things like these semi-standard
expansion headers you see on embedded devices that expose things like
GPIOs, I2C and SPI pins. In some cases users might end up installing
off-the-shelf expansion boards and they will be happy to add device tree
overlays to add whatever is available on that board. Again, for these I
suspect a kernel driver matching on that compatible string is more
appropriate.

In other cases users may just want to connect something completely
custom or just have a way to poke whatever they connect. There may not
be a proper driver for it. Or it could perhaps even be used temporarily
as a way to write a userspace driver conveniently before porting it to
Linux.

The way I imagine it, exporting would involve writing a chip-select to a
specific SPI controller's "export" sysfs attribute to have a SPI device
created for that particular chip-select and bind it to spidev.

Thierry

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Tue, Mar 25, 2025 at 05:38:57PM +0100, Thierry Reding wrote:
> On Tue, Mar 25, 2025 at 03:55:02PM +0000, Mark Brown wrote:

> > Possibly?  I think I've lost track of what the use case is here, usually
> > for the spidev stuff DT overlays seem like they're the right thing but
> > perhaps this is different?  If we are doing this at runtime sysfs seems
> > like a reasonable way to trigger it, though you'd still need the DT to
> > describe the controller and the chipselects that are available.

> Heh... it's exactly the opposite for me. I feel like I don't understand
> the need for spidev with a specific compatible string. If you've got a
> compatible string (or in other words you have a device with a very
> specific SPI chip connected), then why would you want to access it from
> userspace? Isn't a proper kernel driver the better option in most cases?
> That usually allows for better abstraction via some other subsystem. I
> suppose there are cases where there may not be a subsystem, or for other
> reasons it's more convenient to have direct access from userspace to
> avoid the roundtrip. Or maybe users could be concerned about safety?

A lot of things would be happier with a driver, yes.  One of the use
cases that did make sense to me longer term was DSP/coprocessor type
things with flexible functions where distributing the firmware and
application that talks to it together makes sense.

> In other cases users may just want to connect something completely
> custom or just have a way to poke whatever they connect. There may not
> be a proper driver for it. Or it could perhaps even be used temporarily
> as a way to write a userspace driver conveniently before porting it to
> Linux.

> The way I imagine it, exporting would involve writing a chip-select to a
> specific SPI controller's "export" sysfs attribute to have a SPI device
> created for that particular chip-select and bind it to spidev.

My general feeling with those is that if you're building for them you're
probably either already modifiying your kernel or easily able to cope
with doing so.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Jon Hunter]

On 25/03/2025 17:05, Mark Brown wrote:

...

>> The way I imagine it, exporting would involve writing a chip-select to a
>> specific SPI controller's "export" sysfs attribute to have a SPI device
>> created for that particular chip-select and bind it to spidev.
> 
> My general feeling with those is that if you're building for them you're
> probably either already modifiying your kernel or easily able to cope
> with doing so.


That's definitely what we do today, modify the kernel directly to 
achieve what we need. I am trying to avoid carrying too many out of tree 
patches for stuff like this and have something in the kernel that works 
by default. This is even more important for 3rd party Linux distros that 
will not accept non-upstream code.

Our devkits, very much like Raspberry PI, allow users to connect various 
hardware for development and so having an easy way to connect a SPI 
device is useful. For any production systems, users will definitely want 
a proper device and device-tree bindings. So I am just trying to explore 
what would be acceptable. If it is acceptable to have a sysfs interface 
for creating a SPI device at runtime, then we can look into that.

Cheers!
Jon

-- 
nvpublic

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Wed, Mar 26, 2025 at 12:16:53PM +0000, Jon Hunter wrote:
> On 25/03/2025 17:05, Mark Brown wrote:

> > > The way I imagine it, exporting would involve writing a chip-select to a
> > > specific SPI controller's "export" sysfs attribute to have a SPI device
> > > created for that particular chip-select and bind it to spidev.

> > My general feeling with those is that if you're building for them you're
> > probably either already modifiying your kernel or easily able to cope
> > with doing so.

> That's definitely what we do today, modify the kernel directly to achieve
> what we need. I am trying to avoid carrying too many out of tree patches for
> stuff like this and have something in the kernel that works by default. This
> is even more important for 3rd party Linux distros that will not accept
> non-upstream code.

Overlays should work well for that case too!

> Our devkits, very much like Raspberry PI, allow users to connect various
> hardware for development and so having an easy way to connect a SPI device
> is useful. For any production systems, users will definitely want a proper
> device and device-tree bindings. So I am just trying to explore what would
> be acceptable. If it is acceptable to have a sysfs interface for creating a
> SPI device at runtime, then we can look into that.

The main issue I see with the sysfs thing is that you have to describe
the presence of the device somehow which currently needs a device of
some kind there, it's not like I2C where you can just use the address.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Jon Hunter]

On 27/03/2025 15:33, Mark Brown wrote:
> On Wed, Mar 26, 2025 at 12:16:53PM +0000, Jon Hunter wrote:
>> On 25/03/2025 17:05, Mark Brown wrote:
> 
>>>> The way I imagine it, exporting would involve writing a chip-select to a
>>>> specific SPI controller's "export" sysfs attribute to have a SPI device
>>>> created for that particular chip-select and bind it to spidev.
> 
>>> My general feeling with those is that if you're building for them you're
>>> probably either already modifiying your kernel or easily able to cope
>>> with doing so.
> 
>> That's definitely what we do today, modify the kernel directly to achieve
>> what we need. I am trying to avoid carrying too many out of tree patches for
>> stuff like this and have something in the kernel that works by default. This
>> is even more important for 3rd party Linux distros that will not accept
>> non-upstream code.
> 
> Overlays should work well for that case too!


If you mean device-tree overlays, I don't see how that will work today. 
Unless we are to use one of the existing compatible strings, but that 
feels wrong because we are not using any of those devices and like I 
mentioned, just using 'spidev' alone in device-tree does not work with 
the latest kernels.

Jon

-- 
nvpublic

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Mon, Mar 31, 2025 at 01:34:21PM +0100, Jon Hunter wrote:
> On 27/03/2025 15:33, Mark Brown wrote:
> > On Wed, Mar 26, 2025 at 12:16:53PM +0000, Jon Hunter wrote:

> > > That's definitely what we do today, modify the kernel directly to achieve
> > > what we need. I am trying to avoid carrying too many out of tree patches for
> > > stuff like this and have something in the kernel that works by default. This
> > > is even more important for 3rd party Linux distros that will not accept
> > > non-upstream code.

> > Overlays should work well for that case too!

> If you mean device-tree overlays, I don't see how that will work today.
> Unless we are to use one of the existing compatible strings, but that feels
> wrong because we are not using any of those devices and like I mentioned,
> just using 'spidev' alone in device-tree does not work with the latest
> kernels.

Why would you need to use a compatible string for anything?  I'd expect
the overlay to add the entire new device, compatible and all.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Jon Hunter]

On 31/03/2025 13:44, Mark Brown wrote:
> On Mon, Mar 31, 2025 at 01:34:21PM +0100, Jon Hunter wrote:
>> On 27/03/2025 15:33, Mark Brown wrote:
>>> On Wed, Mar 26, 2025 at 12:16:53PM +0000, Jon Hunter wrote:
> 
>>>> That's definitely what we do today, modify the kernel directly to achieve
>>>> what we need. I am trying to avoid carrying too many out of tree patches for
>>>> stuff like this and have something in the kernel that works by default. This
>>>> is even more important for 3rd party Linux distros that will not accept
>>>> non-upstream code.
> 
>>> Overlays should work well for that case too!
> 
>> If you mean device-tree overlays, I don't see how that will work today.
>> Unless we are to use one of the existing compatible strings, but that feels
>> wrong because we are not using any of those devices and like I mentioned,
>> just using 'spidev' alone in device-tree does not work with the latest
>> kernels.
> 
> Why would you need to use a compatible string for anything?  I'd expect
> the overlay to add the entire new device, compatible and all.


We need a compatible string for the SPI device in device-tree. Sorry, 
but I am not sure how we can add a SPI device without one.

Can you confirm what you mean by 'overlay'? To me an overlay, is purely 
a device-tree overlay and even if we have a device-tree overlay, we are 
still missing the kernel driver part that matches the compatible string.

I could be completely missing your point here, but it is not obvious to 
me what you are suggesting?

Jon

-- 
nvpublic

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Mon, Mar 31, 2025 at 02:11:35PM +0100, Jon Hunter wrote:
> On 31/03/2025 13:44, Mark Brown wrote:

> > Why would you need to use a compatible string for anything?  I'd expect
> > the overlay to add the entire new device, compatible and all.

> We need a compatible string for the SPI device in device-tree. Sorry, but I
> am not sure how we can add a SPI device without one.

> Can you confirm what you mean by 'overlay'? To me an overlay, is purely a
> device-tree overlay and even if we have a device-tree overlay, we are still
> missing the kernel driver part that matches the compatible string.

I would expect the overlay to be a device tree overlay and to add the
entire definition of the device, not just patch the compatible in an
existing half constructed device.  I would expect the driver portion of
supporting the device to happen as normal, if spidev makes sense for
whatever devices these are then then that could be upstreamed without
adding them to any board.

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

Re: [PATCH 2/3] dt-bindings: spi: Add DT schema for Tegra SPIDEV controller

[Mark Brown]

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

On Tue, Mar 25, 2025 at 01:45:26PM +0100, Thierry Reding wrote:
> On Tue, Mar 25, 2025 at 12:10:19PM +0000, Mark Brown wrote:
> > On Tue, Mar 25, 2025 at 10:36:29AM +0000, Jon Hunter wrote:

> > > We finally got back to this. Looks like just having 'spidev' as the
> > > compatible does not work. Apparently, it use to work and yes you would get
> > > the warning, but that no longer seems to be the case. I see a few others
> > > have been doing similar things and hacking their device-trees in different
> > > ways [0].

> > Huh, OK.  I don't recall any deliberate SPI change for that.

> People in the discussion that Jon linked to indicated that it was this
> patch that caused the "regression":

...

> If you say that the regression wasn't deliberate, maybe we should look
> at fixing this so that people don't have to work around stuff?

I mean, we don't want people to be doing this which is why it generates
the warning - it's more tolerated than supported.  I don't super mind
keeping it so long as it's not getting in the way of all the fragility
with the enumeration stuff.

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

[PATCH 3/3] spi: spidev: add "tegra-spidev" name string

[Vishwaroop A]

Add "tegra-spidev" name string to load spidev driver
which allows user-space programs to access and communicate
with SPI devices connected to the system.

Signed-off-by: Vishwaroop A <va@nvidia.com>
---
 drivers/spi/spidev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 653f82984216..1d2c01bd76a9 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -711,6 +711,7 @@ static const struct spi_device_id spidev_spi_ids[] = {
 	{ .name = "spi-authenta" },
 	{ .name = "em3581" },
 	{ .name = "si3210" },
+	{ .name = "tegra-spidev" },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, spidev_spi_ids);
@@ -741,6 +742,7 @@ static const struct of_device_id spidev_dt_ids[] = {
 	{ .compatible = "semtech,sx1301", .data = &spidev_of_check },
 	{ .compatible = "silabs,em3581", .data = &spidev_of_check },
 	{ .compatible = "silabs,si3210", .data = &spidev_of_check },
+	{ .compatible = "nvidia,tegra-spidev", .data = &spidev_of_check },
 	{},
 };
 MODULE_DEVICE_TABLE(of, spidev_dt_ids);
-- 
2.17.1

Re: [PATCH 3/3] spi: spidev: add "tegra-spidev" name string

[Krzysztof Kozlowski]

On 26/11/2024 14:45, Vishwaroop A wrote:
> Add "tegra-spidev" name string to load spidev driver
> which allows user-space programs to access and communicate
> with SPI devices connected to the system.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: Vishwaroop A <va@nvidia.com>
> ---
>  drivers/spi/spidev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index 653f82984216..1d2c01bd76a9 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -711,6 +711,7 @@ static const struct spi_device_id spidev_spi_ids[] = {
>  	{ .name = "spi-authenta" },
>  	{ .name = "em3581" },
>  	{ .name = "si3210" },
> +	{ .name = "tegra-spidev" },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(spi, spidev_spi_ids);
> @@ -741,6 +742,7 @@ static const struct of_device_id spidev_dt_ids[] = {
>  	{ .compatible = "semtech,sx1301", .data = &spidev_of_check },
>  	{ .compatible = "silabs,em3581", .data = &spidev_of_check },
>  	{ .compatible = "silabs,si3210", .data = &spidev_of_check },
> +	{ .compatible = "nvidia,tegra-spidev", .data = &spidev_of_check },

That's too generic. You need to come with compatibles matching exactly
the device connected.

Above comment applies to all your patches here - DTS and bindings as well.

Best regards,
Krzysztof