2020-07-02 14:22:33

by Frieder Schrempf

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

From: Frieder Schrempf <[email protected]>

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

Signed-off-by: Frieder Schrempf <[email protected]>
---
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


2020-07-02 14:27:55

by Mark Brown

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

On Thu, Jul 02, 2020 at 04:18:46PM +0200, Schrempf Frieder wrote:
> From: Frieder Schrempf <[email protected]>
>
> 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?


Attachments:
(No filename) (283.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-02 14:47:07

by Frieder Schrempf

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

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 <[email protected]>
>>
>> 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.

2020-07-02 14:57:59

by Geert Uytterhoeven

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

Hi Frieder,

On Thu, Jul 2, 2020 at 4:46 PM Frieder Schrempf
<[email protected]> 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 <[email protected]>
> >>
> >> 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 -- [email protected]

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

2020-07-02 15:10:24

by Mark Brown

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

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.


Attachments:
(No filename) (654.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-02 16:04:46

by Frieder Schrempf

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

Hi Geert,

On 02.07.20 16:57, Geert Uytterhoeven wrote:
> Hi Frieder,
>
> On Thu, Jul 2, 2020 at 4:46 PM Frieder Schrempf
> <[email protected]> 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 <[email protected]>
>>>>
>>>> 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

2020-07-02 16:25:36

by Frieder Schrempf

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

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 ;)

2020-07-13 13:22:46

by Frieder Schrempf

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

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?

2020-07-13 15:12:02

by Mark Brown

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

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.


Attachments:
(No filename) (824.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-14 08:56:47

by Frieder Schrempf

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

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?

2020-07-14 19:30:06

by Mark Brown

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

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?


Attachments:
(No filename) (656.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-15 07:27:36

by Frieder Schrempf

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

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 ;)

2020-07-15 12:14:31

by Mark Brown

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

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!


Attachments:
(No filename) (971.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-15 12:17:54

by Frieder Schrempf

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

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!

2020-07-15 13:13:29

by Mark Brown

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

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.


Attachments:
(No filename) (711.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-15 15:08:49

by Mark Brown

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

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.


Attachments:
(No filename) (839.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-15 15:08:49

by Frieder Schrempf

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

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.

2020-07-15 19:11:29

by Geert Uytterhoeven

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

On Mon, Jul 13, 2020 at 5:11 PM Mark Brown <[email protected]> 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 -- [email protected]

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

2020-07-16 07:56:48

by Frieder Schrempf

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

On 15.07.20 21:06, Geert Uytterhoeven wrote:
> On Mon, Jul 13, 2020 at 5:11 PM Mark Brown <[email protected]> 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.