[PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Schrempf Frieder]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

Allow external SPI ports on Kontron boards to use the spidev driver.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/spi/spidev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 59e07675ef86..058b08a3767d 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -677,6 +677,7 @@ static const struct of_device_id spidev_dt_ids[] = {
 	{ .compatible = "lwn,bk4" },
 	{ .compatible = "dh,dhcom-board" },
 	{ .compatible = "menlo,m53cpld" },
+	{ .compatible = "kontron,user-spi" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, spidev_dt_ids);
-- 
2.17.1

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Mark Brown]

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

On Thu, Jul 02, 2020 at 04:18:46PM +0200, Schrempf Frieder wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Allow external SPI ports on Kontron boards to use the spidev driver.

I'd have expected this to require loading a DT overlay for whatever's
attached?

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

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

On 02.07.20 16:25, Mark Brown wrote:
> On Thu, Jul 02, 2020 at 04:18:46PM +0200, Schrempf Frieder wrote:
>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>
>> Allow external SPI ports on Kontron boards to use the spidev driver.
> 
> I'd have expected this to require loading a DT overlay for whatever's
> attached?

My intention is to use the spidev driver in the default board DT for an 
interface that is routed to an extension connector and has no dedicated 
slave device attached onboard. So users can attach sensors, etc. with
userspace drivers without touching the kernel or DT.

See https://patchwork.kernel.org/patch/11639075/ for the boards DT.

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Geert Uytterhoeven]

Hi Frieder,

On Thu, Jul 2, 2020 at 4:46 PM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
> On 02.07.20 16:25, Mark Brown wrote:
> > On Thu, Jul 02, 2020 at 04:18:46PM +0200, Schrempf Frieder wrote:
> >> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> >>
> >> Allow external SPI ports on Kontron boards to use the spidev driver.
> >
> > I'd have expected this to require loading a DT overlay for whatever's
> > attached?
>
> My intention is to use the spidev driver in the default board DT for an
> interface that is routed to an extension connector and has no dedicated
> slave device attached onboard. So users can attach sensors, etc. with
> userspace drivers without touching the kernel or DT.
>
> See https://patchwork.kernel.org/patch/11639075/ for the boards DT.

You can bind "kontron,user-spi" devices to spidev from userspace:
[PATCH v2 0/3] device tree spidev solution - driver_override for SPI
https://spinics.net/lists/linux-spi/msg13951.html

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Mark Brown]

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

On Thu, Jul 02, 2020 at 04:46:09PM +0200, Frieder Schrempf wrote:

> My intention is to use the spidev driver in the default board DT for an
> interface that is routed to an extension connector and has no dedicated
> slave device attached onboard. So users can attach sensors, etc. with
> userspace drivers without touching the kernel or DT.

The expected way of doing this is to describe whatever was attached via
DT when it's attached - the device is what has the compatible, not some
connector in the middle of the connection.  The way you've got things
set up if the device has a driver then they won't be able to instantiate
the driver.

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

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

Hi Geert,

On 02.07.20 16:57, Geert Uytterhoeven wrote:
> Hi Frieder,
> 
> On Thu, Jul 2, 2020 at 4:46 PM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
>> On 02.07.20 16:25, Mark Brown wrote:
>>> On Thu, Jul 02, 2020 at 04:18:46PM +0200, Schrempf Frieder wrote:
>>>> From: Frieder Schrempf <frieder.schrempf@kontron.de>
>>>>
>>>> Allow external SPI ports on Kontron boards to use the spidev driver.
>>>
>>> I'd have expected this to require loading a DT overlay for whatever's
>>> attached?
>>
>> My intention is to use the spidev driver in the default board DT for an
>> interface that is routed to an extension connector and has no dedicated
>> slave device attached onboard. So users can attach sensors, etc. with
>> userspace drivers without touching the kernel or DT.
>>
>> See https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F11639075%2F&amp;data=02%7C01%7Cfrieder.schrempf%40kontron.de%7C5ca9f0ba0ccb4ab0329f08d81e983d4e%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637292987071583819&amp;sdata=M8y9HYICQLocSgRmak6uYsx9Y%2FoukaqgmK2D0F%2FTV98%3D&amp;reserved=0 for the boards DT.
> 
> You can bind "kontron,user-spi" devices to spidev from userspace:
> [PATCH v2 0/3] device tree spidev solution - driver_override for SPI
> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fspinics.net%2Flists%2Flinux-spi%2Fmsg13951.html&amp;data=02%7C01%7Cfrieder.schrempf%40kontron.de%7C5ca9f0ba0ccb4ab0329f08d81e983d4e%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637292987071583819&amp;sdata=D9pum1oTXWSt%2BY8Egb1J4DOgSa5KH3RwxsSmUl7LgUk%3D&amp;reserved=0

Thanks for pointing that out. I didn't know that this is possible.
I just tried like below it and it works like a charm:

 > echo "spidev" > 
/sys/devices/platform/soc@0/30800000.bus/30840000.spi/spi_master/spi2/spi2.0/driver_override
 > echo "spi2.0" > /sys/bus/spi/drivers/spidev/bind

Thanks,
Frieder

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

On 02.07.20 17:07, Mark Brown wrote:
> On Thu, Jul 02, 2020 at 04:46:09PM +0200, Frieder Schrempf wrote:
> 
>> My intention is to use the spidev driver in the default board DT for an
>> interface that is routed to an extension connector and has no dedicated
>> slave device attached onboard. So users can attach sensors, etc. with
>> userspace drivers without touching the kernel or DT.
> 
> The expected way of doing this is to describe whatever was attached via
> DT when it's attached - the device is what has the compatible, not some
> connector in the middle of the connection.  The way you've got things
> set up if the device has a driver then they won't be able to instantiate
> the driver.

Ok thanks, got it. I will remove the spi device from the board DT and 
use an overlay if required. Now I got a reason to learn writing DT 
overlays ;)

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

On 02.07.20 18:24, Frieder Schrempf wrote:
> On 02.07.20 17:07, Mark Brown wrote:
>> On Thu, Jul 02, 2020 at 04:46:09PM +0200, Frieder Schrempf wrote:
>>
>>> My intention is to use the spidev driver in the default board DT for an
>>> interface that is routed to an extension connector and has no dedicated
>>> slave device attached onboard. So users can attach sensors, etc. with
>>> userspace drivers without touching the kernel or DT.
>>
>> The expected way of doing this is to describe whatever was attached via
>> DT when it's attached - the device is what has the compatible, not some
>> connector in the middle of the connection.  The way you've got things
>> set up if the device has a driver then they won't be able to instantiate
>> the driver.
> 
> Ok thanks, got it. I will remove the spi device from the board DT and 
> use an overlay if required. Now I got a reason to learn writing DT 
> overlays ;)

I need to come back to this for another question. I would very much like 
to do it the way Mark described it above: Use a DT overlay to describe 
the device and load this overlay when the device is attached.

But if I get this correctly it requires me to write a specific driver 
that handles the loading of the overlay, which is way too much overhead 
for the simple issue I'm trying to solve.

I would have expected that there is some kind of existing userspace API 
to load an overlay manually, but it seems like there isn't!?

So what's the reasoning behind this? How can I solve this in a 
mainline-compliant way, meaning without either keeping downstream 
patches to bind spidev to my device or writing and maintaining code that 
does the overlay loading?

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Mark Brown]

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

On Mon, Jul 13, 2020 at 03:19:52PM +0200, Frieder Schrempf wrote:

> I would have expected that there is some kind of existing userspace API to
> load an overlay manually, but it seems like there isn't!?

> So what's the reasoning behind this? How can I solve this in a
> mainline-compliant way, meaning without either keeping downstream patches to
> bind spidev to my device or writing and maintaining code that does the
> overlay loading?

Basically the reasoning is that nobody's done it rather than any grand
design not to do it.  There's some issues for more complex connectors
present on multiple boards with mapping the same connector onto multiple
boards where a resource on the connector might be provided by different
things on the base board so it's not quite as trivial for them as it
should be.

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

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

On 13.07.20 17:11, Mark Brown wrote:
> On Mon, Jul 13, 2020 at 03:19:52PM +0200, Frieder Schrempf wrote:
> 
>> I would have expected that there is some kind of existing userspace API to
>> load an overlay manually, but it seems like there isn't!?
> 
>> So what's the reasoning behind this? How can I solve this in a
>> mainline-compliant way, meaning without either keeping downstream patches to
>> bind spidev to my device or writing and maintaining code that does the
>> overlay loading?
> 
> Basically the reasoning is that nobody's done it rather than any grand
> design not to do it.  There's some issues for more complex connectors
> present on multiple boards with mapping the same connector onto multiple
> boards where a resource on the connector might be provided by different
> things on the base board so it's not quite as trivial for them as it
> should be.

Ok, thanks. I'm afraid I'm currently not in the position to work on any 
generic solution to leverage the loading of DT overlays from userspace.

It would still be quite nice to benefit from the flexibility of DT 
overlays not only for the SPI use case. But before I come up with any 
custom solution, for now I will rather have the device in the DT statically.

I just wonder if I need to keep the DT node for the device in a separate 
patch in our own tree, or if a node with a custom compatible string like 
for example "kontron,user-spi" would be accepted upstream, without a 
matching driver?

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Mark Brown]

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

On Tue, Jul 14, 2020 at 10:54:15AM +0200, Frieder Schrempf wrote:

> It would still be quite nice to benefit from the flexibility of DT overlays
> not only for the SPI use case. But before I come up with any custom
> solution, for now I will rather have the device in the DT statically.

> I just wonder if I need to keep the DT node for the device in a separate
> patch in our own tree, or if a node with a custom compatible string like for
> example "kontron,user-spi" would be accepted upstream, without a matching
> driver?

I'm having a hard time getting enthusiastic about it TBH - can you not
just use spidev and live with the warning?

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

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

On 14.07.20 21:29, Mark Brown wrote:
> On Tue, Jul 14, 2020 at 10:54:15AM +0200, Frieder Schrempf wrote:
> 
>> It would still be quite nice to benefit from the flexibility of DT overlays
>> not only for the SPI use case. But before I come up with any custom
>> solution, for now I will rather have the device in the DT statically.
> 
>> I just wonder if I need to keep the DT node for the device in a separate
>> patch in our own tree, or if a node with a custom compatible string like for
>> example "kontron,user-spi" would be accepted upstream, without a matching
>> driver?
> 
> I'm having a hard time getting enthusiastic about it TBH - can you not
> just use spidev and live with the warning?

Ok, I can do that, but when I resend my patches and add "compatible = 
'spidev'" to my DT I expect someone to complain again as my DT does not 
describe the hardware.

Seeing that there are quite a few DTs that still do it like this, I 
probably will try it still and also keep a patch in our tree to remove 
the warning so customers won't be getting worried.

But for obvious reasons this can't be considered a good solution and it 
seems somewhat disturbing that the maintainer needs to propose it 
because of lack of proper solutions ;)

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Mark Brown]

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

On Wed, Jul 15, 2020 at 09:26:29AM +0200, Frieder Schrempf wrote:
> On 14.07.20 21:29, Mark Brown wrote:
> > On Tue, Jul 14, 2020 at 10:54:15AM +0200, Frieder Schrempf wrote:

> > > patch in our own tree, or if a node with a custom compatible string like for
> > > example "kontron,user-spi" would be accepted upstream, without a matching
> > > driver?

> > I'm having a hard time getting enthusiastic about it TBH - can you not
> > just use spidev and live with the warning?

> Ok, I can do that, but when I resend my patches and add "compatible =
> 'spidev'" to my DT I expect someone to complain again as my DT does not
> describe the hardware.

That's the issue with kontron,user-spi too though :/

> But for obvious reasons this can't be considered a good solution and it
> seems somewhat disturbing that the maintainer needs to propose it because of
> lack of proper solutions ;)

Hey, I proposed other solutions you didn't want to implement!

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

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

On 15.07.20 13:36, Mark Brown wrote:
> On Wed, Jul 15, 2020 at 09:26:29AM +0200, Frieder Schrempf wrote:
>> On 14.07.20 21:29, Mark Brown wrote:
>>> On Tue, Jul 14, 2020 at 10:54:15AM +0200, Frieder Schrempf wrote:
> 
>>>> patch in our own tree, or if a node with a custom compatible string like for
>>>> example "kontron,user-spi" would be accepted upstream, without a matching
>>>> driver?
> 
>>> I'm having a hard time getting enthusiastic about it TBH - can you not
>>> just use spidev and live with the warning?
> 
>> Ok, I can do that, but when I resend my patches and add "compatible =
>> 'spidev'" to my DT I expect someone to complain again as my DT does not
>> describe the hardware.
> 
> That's the issue with kontron,user-spi too though :/

Yes, true.

> 
>> But for obvious reasons this can't be considered a good solution and it
>> seems somewhat disturbing that the maintainer needs to propose it because of
>> lack of proper solutions ;)
> 
> Hey, I proposed other solutions you didn't want to implement!

Right, but you have to admit that the other solutions turned out to be 
rather out of scope for someone like me who merely wants to use the 
spidev driver.

But I don't blame you. I'm now having a better idea of how things are 
(or aren't) supposed to look like. So thanks for your patience!

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Mark Brown]

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

On Wed, Jul 15, 2020 at 01:45:51PM +0200, Frieder Schrempf wrote:
> On 15.07.20 13:36, Mark Brown wrote:

> > Hey, I proposed other solutions you didn't want to implement!

> Right, but you have to admit that the other solutions turned out to be
> rather out of scope for someone like me who merely wants to use the spidev
> driver.

> But I don't blame you. I'm now having a better idea of how things are (or
> aren't) supposed to look like. So thanks for your patience!

I'm not sure platforms like this are a great fit for DT TBH - the
trouble with DT is that it turns things into ABIs regardless of if it's
really a finished thing, platform data based systems were a lot more
flexible here.

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

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

On 15.07.20 15:10, Mark Brown wrote:
> On Wed, Jul 15, 2020 at 01:45:51PM +0200, Frieder Schrempf wrote:
>> On 15.07.20 13:36, Mark Brown wrote:
> 
>>> Hey, I proposed other solutions you didn't want to implement!
> 
>> Right, but you have to admit that the other solutions turned out to be
>> rather out of scope for someone like me who merely wants to use the spidev
>> driver.
> 
>> But I don't blame you. I'm now having a better idea of how things are (or
>> aren't) supposed to look like. So thanks for your patience!
> 
> I'm not sure platforms like this are a great fit for DT TBH - the
> trouble with DT is that it turns things into ABIs regardless of if it's
> really a finished thing, platform data based systems were a lot more
> flexible here.

I see your point. Still it seems like the overlays would provide a 
sufficient alternative in matters of flexibility. Raspberry Pi uses it 
for years, which made me assume that some generic loading API is 
available. But it seems like everything is RPi-specific and downstream.

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Mark Brown]

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

On Wed, Jul 15, 2020 at 03:48:37PM +0200, Frieder Schrempf wrote:
> On 15.07.20 15:10, Mark Brown wrote:

> > I'm not sure platforms like this are a great fit for DT TBH - the
> > trouble with DT is that it turns things into ABIs regardless of if it's
> > really a finished thing, platform data based systems were a lot more
> > flexible here.

> I see your point. Still it seems like the overlays would provide a
> sufficient alternative in matters of flexibility. Raspberry Pi uses it for
> years, which made me assume that some generic loading API is available. But
> it seems like everything is RPi-specific and downstream.

Yes, overlays would be fine if there were a way to use them for
connectors like this.  The connectors are a real problem, it's not just
RPi that has their own loaders/infrastructure downstream.

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

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Geert Uytterhoeven]

On Mon, Jul 13, 2020 at 5:11 PM Mark Brown <broonie@kernel.org> wrote:
> On Mon, Jul 13, 2020 at 03:19:52PM +0200, Frieder Schrempf wrote:
> > I would have expected that there is some kind of existing userspace API to
> > load an overlay manually, but it seems like there isn't!?
>
> > So what's the reasoning behind this? How can I solve this in a
> > mainline-compliant way, meaning without either keeping downstream patches to
> > bind spidev to my device or writing and maintaining code that does the
> > overlay loading?
>
> Basically the reasoning is that nobody's done it rather than any grand

Nah, it's been done, but a bit unsafe, if you don't know what you're doing
("with great power come great responsibilities").

Please check out https://elinux.org/R-Car/DT-Overlays
I do my best to keep topic/overlays branch up-to-date, and working.

> design not to do it.  There's some issues for more complex connectors
> present on multiple boards with mapping the same connector onto multiple
> boards where a resource on the connector might be provided by different
> things on the base board so it's not quite as trivial for them as it
> should be.

There's a big list of issues at
https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts
In other words: more work to be done, to polish it, and make it safe(r).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] spi: spidev: Add compatible for external SPI ports on Kontron boards

[Frieder Schrempf]

On 15.07.20 21:06, Geert Uytterhoeven wrote:
> On Mon, Jul 13, 2020 at 5:11 PM Mark Brown <broonie@kernel.org> wrote:
>> On Mon, Jul 13, 2020 at 03:19:52PM +0200, Frieder Schrempf wrote:
>>> I would have expected that there is some kind of existing userspace API to
>>> load an overlay manually, but it seems like there isn't!?
>>
>>> So what's the reasoning behind this? How can I solve this in a
>>> mainline-compliant way, meaning without either keeping downstream patches to
>>> bind spidev to my device or writing and maintaining code that does the
>>> overlay loading?
>>
>> Basically the reasoning is that nobody's done it rather than any grand
> 
> Nah, it's been done, but a bit unsafe, if you don't know what you're doing
> ("with great power come great responsibilities").
> 
> Please check out https://elinux.org/R-Car/DT-Overlays
> I do my best to keep topic/overlays branch up-to-date, and working.

Thanks! That looks like what I was searching for, but couldn't find it. 
So there's some work being done. That's good to know. We probably can't 
use it in this state, but that's still a lot better than nothing.

> 
>> design not to do it.  There's some issues for more complex connectors
>> present on multiple boards with mapping the same connector onto multiple
>> boards where a resource on the connector might be provided by different
>> things on the base board so it's not quite as trivial for them as it
>> should be.
> 
> There's a big list of issues at
> https://elinux.org/Frank%27s_Evolving_Overlay_Thoughts
> In other words: more work to be done, to polish it, and make it safe(r).

I'm now getting a rough idea about the efforts and issues behind this, 
thanks.