| Message ID | 20200702141846.7752-1-frieder.schrempf@kontron.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: spidev: Add compatible for external SPI ports on Kontron boards | expand |
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?
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.
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
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.
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&data=02%7C01%7Cfrieder.schrempf%40kontron.de%7C5ca9f0ba0ccb4ab0329f08d81e983d4e%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637292987071583819&sdata=M8y9HYICQLocSgRmak6uYsx9Y%2FoukaqgmK2D0F%2FTV98%3D&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&data=02%7C01%7Cfrieder.schrempf%40kontron.de%7C5ca9f0ba0ccb4ab0329f08d81e983d4e%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637292987071583819&sdata=D9pum1oTXWSt%2BY8Egb1J4DOgSa5KH3RwxsSmUl7LgUk%3D&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
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 ;)
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?
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.
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?
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?
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 ;)
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!
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!
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.
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.
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.
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
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.
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);