diff mbox

[v2,3/3] drivers core: allow id match override when manually binding driver

Message ID 15ff382f699387e2d8f23779db851d0de7e9291e.1467053363.git.hramrach@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michal Suchanek June 27, 2016, 7:02 p.m. UTC
The spi bus has no autodetection whatsoever. The 'detection' of the
device that's suposed to be on the other side completely relies on user
supplied information coming from devicetree on many platforms. It is
completely reasonable then to allow the user to supply the information
at runtime by doing echo 'somedevice' >
/sys/bus/spi/drivers/somedriver/bind
This fails if somedriver does not have in its id table compatible of
somedevice so just skip this check for manual driver binding.

This allows binding spidev on any slave device by hand using sysfs
without adding superfluous compatibles or any other needless
complication.

Note that any slave driver that requires configuration will fail to
probe anyway if the configuration is not provided in the devicetree node.
A driver like spidev that requires no configuration can bind to any slave.

Signed-off-by: Michal Suchanek <hramrach@gmail.com>
---

 drivers/base/bus.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Comments

Greg KH June 27, 2016, 7:09 p.m. UTC | #1
On Mon, Jun 27, 2016 at 09:02:32PM +0200, Michal Suchanek wrote:
> The spi bus has no autodetection whatsoever. The 'detection' of the
> device that's suposed to be on the other side completely relies on user
> supplied information coming from devicetree on many platforms. It is
> completely reasonable then to allow the user to supply the information
> at runtime by doing echo 'somedevice' >
> /sys/bus/spi/drivers/somedriver/bind
> This fails if somedriver does not have in its id table compatible of
> somedevice so just skip this check for manual driver binding.

That's what the new_id file is for, right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Suchanek June 27, 2016, 7:40 p.m. UTC | #2
On 27 June 2016 at 21:09, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 27, 2016 at 09:02:32PM +0200, Michal Suchanek wrote:
>> The spi bus has no autodetection whatsoever. The 'detection' of the
>> device that's suposed to be on the other side completely relies on user
>> supplied information coming from devicetree on many platforms. It is
>> completely reasonable then to allow the user to supply the information
>> at runtime by doing echo 'somedevice' >
>> /sys/bus/spi/drivers/somedriver/bind
>> This fails if somedriver does not have in its id table compatible of
>> somedevice so just skip this check for manual driver binding.
>
> That's what the new_id file is for, right?
>

No. It's for buses that have some inherent identification. It's not for

1) generate random compatible and stick it in device tree
2) reboot with new devicetree or load overlay
3) write the random compatible you just generated to new_id file so
you can bind drivers to your device

You could have saved yourself a lot of hassle just ignoring the ID completely.

Do you have to go through that to connect a different modem to your
serial port? Or even a new i2c device to i2c bus?

also AFAIK new_id is not automagic and not all buses have it.

So it would have to be implemented on SPI. How? On PCI new_id is a PCI
id. What is it on SPI? ACPI PnP id? DT compatible? How do you tell?
And why when the bus does not even have IDs?

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown June 27, 2016, 8:32 p.m. UTC | #3
On Mon, Jun 27, 2016 at 09:40:38PM +0200, Michal Suchanek wrote:

> No. It's for buses that have some inherent identification. It's not for

> 1) generate random compatible and stick it in device tree

Don't generate a random compatible, generate one that accurately
describes your hardware.

> also AFAIK new_id is not automagic and not all buses have it.

Yes, this is the bit I've been trying to prompt you to implement rather
than going off doing something broken.

> So it would have to be implemented on SPI. How? On PCI new_id is a PCI
> id. What is it on SPI? ACPI PnP id? DT compatible? How do you tell?

Those sound like sensible ideas.

> And why when the bus does not even have IDs?

Identifiers are just a useful way of describing what the hardware is,
the fact that some of them can be read back from hardware isn't terribly
important here.
Greg KH June 27, 2016, 10:12 p.m. UTC | #4
On Mon, Jun 27, 2016 at 09:40:38PM +0200, Michal Suchanek wrote:
> On 27 June 2016 at 21:09, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > On Mon, Jun 27, 2016 at 09:02:32PM +0200, Michal Suchanek wrote:
> >> The spi bus has no autodetection whatsoever. The 'detection' of the
> >> device that's suposed to be on the other side completely relies on user
> >> supplied information coming from devicetree on many platforms. It is
> >> completely reasonable then to allow the user to supply the information
> >> at runtime by doing echo 'somedevice' >
> >> /sys/bus/spi/drivers/somedriver/bind
> >> This fails if somedriver does not have in its id table compatible of
> >> somedevice so just skip this check for manual driver binding.
> >
> > That's what the new_id file is for, right?
> >
> 
> No. It's for buses that have some inherent identification. It's not for
> 
> 1) generate random compatible and stick it in device tree
> 2) reboot with new devicetree or load overlay
> 3) write the random compatible you just generated to new_id file so
> you can bind drivers to your device
> 
> You could have saved yourself a lot of hassle just ignoring the ID completely.

Um, all devices have to have an "id" the driver core enforces this.

Now your bus can ignore them, or do whatever it wants, but really, you
can't just try to wave them away.

> Do you have to go through that to connect a different modem to your
> serial port?

As has been said numerous times, there is no serial port bus, but people
are working on it.

And no, you don't, and serial _devices_ work just fine with the driver
model.

> Or even a new i2c device to i2c bus?

If you have an unknown i2c device, yes you do.

> also AFAIK new_id is not automagic and not all buses have it.

That's up to the bus to enable that option.  Just like you are creating
a new option.

> So it would have to be implemented on SPI. How? On PCI new_id is a PCI
> id. What is it on SPI? ACPI PnP id? DT compatible? How do you tell?
> And why when the bus does not even have IDs?

Then fake the id out, like all other dumb busses that don't have an id.

Again, I'm not going to take this driver core change, sorry.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Suchanek June 28, 2016, 12:40 p.m. UTC | #5
On 27 June 2016 at 22:32, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Jun 27, 2016 at 09:40:38PM +0200, Michal Suchanek wrote:
>
>> No. It's for buses that have some inherent identification. It's not for
>
>> 1) generate random compatible and stick it in device tree
>
> Don't generate a random compatible, generate one that accurately
> describes your hardware.

Ok, what compatible do you generate to connect a serial modem or use a
serial console?

I just want to use the SPI bus and not a compatible.

There is no hardware other than pin header. You are preparing a
factory image for a devboard with a pin header that can be used for
connecting SPI devices. Some devices have fine kernel drivers and for
these you prepare fine overlays. Some devices have fine userspace
drivers and you do NOT want kernel driver for them even if it is
available later on. What compatible do you put in the factory image so
that the user can just connect an external device and run a
corresponding application to use it?

>
>> also AFAIK new_id is not automagic and not all buses have it.
>
> Yes, this is the bit I've been trying to prompt you to implement rather
> than going off doing something broken.

The new_id part is also broken. How do you new_id a port that can be
used to connect a serial modem, serial console, SLIP line, or pretty
much anything else the user pleases? I'm sure users will be happy if
you enforce to new_id  any use of a serial port once serial bus is
done.

>
>> So it would have to be implemented on SPI. How? On PCI new_id is a PCI
>> id. What is it on SPI? ACPI PnP id? DT compatible? How do you tell?
>
> Those sound like sensible ideas.

There is slight problem that SPI bus can have *both* ACPI PnP IDs and
DT compatibles. so you cannot tell which one it is. Even if you put in
all the infra to tell if the particular bus uses one or the other it
still does not solve the basic problem that SPI is generic
communication bus and users want to just send and receive data on the
bus.

>
>> And why when the bus does not even have IDs?
>
> Identifiers are just a useful way of describing what the hardware is,
> the fact that some of them can be read back from hardware isn't terribly
> important here.

Maybe the fact that some buses are useful for just sending random data
that the kernel does not understand and has no business meddling with
is important then.



On 28 June 2016 at 00:12, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 27, 2016 at 09:40:38PM +0200, Michal Suchanek wrote:
>> On 27 June 2016 at 21:09, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>> > On Mon, Jun 27, 2016 at 09:02:32PM +0200, Michal Suchanek wrote:
>> >> The spi bus has no autodetection whatsoever. The 'detection' of the
>> >> device that's suposed to be on the other side completely relies on user
>> >> supplied information coming from devicetree on many platforms. It is
>> >> completely reasonable then to allow the user to supply the information
>> >> at runtime by doing echo 'somedevice' >
>> >> /sys/bus/spi/drivers/somedriver/bind
>> >> This fails if somedriver does not have in its id table compatible of
>> >> somedevice so just skip this check for manual driver binding.
>> >
>> > That's what the new_id file is for, right?
>> >
>>
>> No. It's for buses that have some inherent identification. It's not for
>>
>> 1) generate random compatible and stick it in device tree
>> 2) reboot with new devicetree or load overlay
>> 3) write the random compatible you just generated to new_id file so
>> you can bind drivers to your device
>>
>> You could have saved yourself a lot of hassle just ignoring the ID completely.
>
> Um, all devices have to have an "id" the driver core enforces this.
>
> Now your bus can ignore them, or do whatever it wants, but really, you
> can't just try to wave them away.

What id has a serial modem or a serial console?

None as far as I can tell. You just connect a serial cable (or not )
and write random data to the port. The protocol is defined by
userspace application and the kernel has no business telling it what
it should be like.

>
>> Do you have to go through that to connect a different modem to your
>> serial port?
>
> As has been said numerous times, there is no serial port bus, but people
> are working on it.
>
> And no, you don't, and serial _devices_ work just fine with the driver
> model.

Then how does the serial bus work so that you do not have to new_id a
modem or serial console and it just works as if there was no bus?

>
>> Or even a new i2c device to i2c bus?
>
> If you have an unknown i2c device, yes you do.

No, you don't. You can just talk to any devices you like from
userspace so long as the address is not used by a kernel driver.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown June 28, 2016, 3:51 p.m. UTC | #6
On Tue, Jun 28, 2016 at 02:40:41PM +0200, Michal Suchanek wrote:

> There is no hardware other than pin header. You are preparing a
> factory image for a devboard with a pin header that can be used for
> connecting SPI devices. Some devices have fine kernel drivers and for
> these you prepare fine overlays. Some devices have fine userspace
> drivers and you do NOT want kernel driver for them even if it is
> available later on. What compatible do you put in the factory image so
> that the user can just connect an external device and run a
> corresponding application to use it?

The answer is still the same here: if you've got a plug in module you
need to load an overlay for that plug in module (see Pantelis' work for
mechanisms to do that).  That overlay should describe the hardware in
the standard DT fashion.  If the current way to support a given bit of
hardware is with a userspace driver then you should add the compatible
string for that hardware to spidev or join up the dots to allow that to
be done at runtime and then do it at runtime.

Repeating yourself over and over again is not going to help here, it's
just going to make people more annoyed.  Please stop this.

> >> So it would have to be implemented on SPI. How? On PCI new_id is a PCI
> >> id. What is it on SPI? ACPI PnP id? DT compatible? How do you tell?

> > Those sound like sensible ideas.

> There is slight problem that SPI bus can have *both* ACPI PnP IDs and
> DT compatibles. so you cannot tell which one it is. Even if you put in
> all the infra to tell if the particular bus uses one or the other it

The formats of compatible strings and ACPI IDs are both well enough
defined and incompatible with each other, distinguishing them is not
hard.

> still does not solve the basic problem that SPI is generic
> communication bus and users want to just send and receive data on the
> bus.

If there is no hardware connected we can massively optimize this by just
not doing anything to the bus in software.

> > Identifiers are just a useful way of describing what the hardware is,
> > the fact that some of them can be read back from hardware isn't terribly
> > important here.

> Maybe the fact that some buses are useful for just sending random data
> that the kernel does not understand and has no business meddling with
> is important then.

This is not helpful.  None of the discussion here is about what is done
with devices once drivers are bound, it's all about how we bind drivers.
Michal Suchanek June 28, 2016, 4:24 p.m. UTC | #7
On 28 June 2016 at 17:51, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 28, 2016 at 02:40:41PM +0200, Michal Suchanek wrote:
>
>> There is no hardware other than pin header. You are preparing a
>> factory image for a devboard with a pin header that can be used for
>> connecting SPI devices. Some devices have fine kernel drivers and for
>> these you prepare fine overlays. Some devices have fine userspace
>> drivers and you do NOT want kernel driver for them even if it is
>> available later on. What compatible do you put in the factory image so
>> that the user can just connect an external device and run a
>> corresponding application to use it?
>
> The answer is still the same here: if you've got a plug in module you
> need to load an overlay for that plug in module (see Pantelis' work for
> mechanisms to do that).  That overlay should describe the hardware in
> the standard DT fashion.  If the current way to support a given bit of
> hardware is with a userspace driver then you should add the compatible
> string for that hardware to spidev or join up the dots to allow that to
> be done at runtime and then do it at runtime.

No, no, no!

This is NOT about loading an overlay. This is about talking on the bus
without loading an overlay.

And this is NOT about adding a compatible for the device. Adding
compatibles for 10 different devices just to use an userspace driver
for them is ridiculous and nobody will do that. If that really turns
out necessary anyone will add the compatible for the fist device so
used and use it for any other device. Which gets us back to generating
random compatible. Which is even better than registering a compatible
that makes sense because then it cannot happen that somebody writes a
kernel driver for your device and you have to unbind it first.

>
> Repeating yourself over and over again is not going to help here, it's
> just going to make people more annoyed.  Please stop this.

Then please add something constructive to the debate rather than
repeating that users have to add compatible for devices used with
userspace driver. That's ridiculous from usability point of view and
so will never happen. At best workarounds for kernel obtuseness will
be used.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown June 28, 2016, 6:38 p.m. UTC | #8
On Tue, Jun 28, 2016 at 06:24:58PM +0200, Michal Suchanek wrote:

> No, no, no!

> This is NOT about loading an overlay. This is about talking on the bus
> without loading an overlay.

This is the solution you want to adopt because you've decided that it's
what you want.  You've said this quite a number of times now.

> > Repeating yourself over and over again is not going to help here, it's
> > just going to make people more annoyed.  Please stop this.

> Then please add something constructive to the debate rather than
> repeating that users have to add compatible for devices used with
> userspace driver. That's ridiculous from usability point of view and
> so will never happen. At best workarounds for kernel obtuseness will
> be used.

It would take a lot less effort for you to just describe your hardware
in DT than it would for you to continue repeating yourself, this is a
trivial task.  If the tooling around this is too hard to use then that
seems like a useful thing to work on, having simple ways for people to
describe modules they build for maker type systems (ideally integrated
with schematic capture software) would be great for example.

Both Greg and myself have provided you with feedback which you appear to
be ignoring entirely.  What you are demanding is not idiomatic for DT or
for the device model, this needs to be taken on board rather than just
shouting at us.  Putting a quick hack in that happens to work for your
very specific use case isn't going to give us a consistent, maintainable
and well designed system if it doesn't fit with anything else we're
doing.
Michal Suchanek June 28, 2016, 8:02 p.m. UTC | #9
On 28 June 2016 at 20:38, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 28, 2016 at 06:24:58PM +0200, Michal Suchanek wrote:
>
>> No, no, no!
>
>> This is NOT about loading an overlay. This is about talking on the bus
>> without loading an overlay.
>
> This is the solution you want to adopt because you've decided that it's
> what you want.  You've said this quite a number of times now.
>
>> > Repeating yourself over and over again is not going to help here, it's
>> > just going to make people more annoyed.  Please stop this.
>
>> Then please add something constructive to the debate rather than
>> repeating that users have to add compatible for devices used with
>> userspace driver. That's ridiculous from usability point of view and
>> so will never happen. At best workarounds for kernel obtuseness will
>> be used.
>
> It would take a lot less effort for you to just describe your hardware
> in DT than it would for you to continue repeating yourself, this is a
> trivial task.

It would be less work having a mainline solution but whatever.

Of course I could add "my-shiny-new-board" compatible to the device
tree and spidev. Then when I build my-shiny-new-board2 and it happens
to also use userspace driver I will not bother to update the
compatible nor will anyone else doing this. So it's equivalent to
having "foobar123" in there from the start. Your proposed solution is
so complex that it will never be used as intended even if changing the
compatible on the node and driver could be both accomplished by an
echo to sysfs.

>  If the tooling around this is too hard to use then that
> seems like a useful thing to work on, having simple ways for people to
> describe modules they build for maker type systems (ideally integrated
> with schematic capture software) would be great for example.

Again, what simple to use tooling do you use to describe the modem you
connect to a serial port in the kernel (ideally integrated with
schematic capture software)? I use none. I just connect the modem. I
want to do that with all SPI devices that I do not intend to use with
an in-kernel driver above the spi master driver that provides the
communication channel as is the case with the serial modem.

>
> Both Greg and myself have provided you with feedback which you appear to
> be ignoring entirely.  What you are demanding is not idiomatic for DT or
> for the device model, this needs to be taken on board rather than just
> shouting at us.  Putting a quick hack in that happens to work for your
> very specific use case isn't going to give us a consistent, maintainable
> and well designed system if it doesn't fit with anything else we're
> doing.

It is not my very specific use case. It's common use case for
devboards which have spi connectors.

If this particular way to solve the problem does not fit with the
driver model does any of the previously listed ones fit better,
perhaps with some changes?

Or since you are familiar with the driver model perhaps you are away
of a solution that was not mentioned so far that would accomplish this
and fits with the driver model?

Or if this use case does not fit into the driver model at all then
perhaps the driver model needs updating to catch up with real world?

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown June 28, 2016, 8:57 p.m. UTC | #10
On Tue, Jun 28, 2016 at 10:02:59PM +0200, Michal Suchanek wrote:

> Of course I could add "my-shiny-new-board" compatible to the device
> tree and spidev. Then when I build my-shiny-new-board2 and it happens
> to also use userspace driver I will not bother to update the
> compatible nor will anyone else doing this. So it's equivalent to
> having "foobar123" in there from the start. Your proposed solution is
> so complex that it will never be used as intended even if changing the
> compatible on the node and driver could be both accomplished by an
> echo to sysfs.

If you are describing the connected SPI device as the board rather than
the actual device you're doing things wrong, and if you have the same
thing on two boards then /of course/ they should list the same
compatible string so the existing software stack should work.  If people
just use some random board description that happens to work right now
even if it doesn't match their actual hardware then they're no worse off
than with your hacks but we've been pushing people to do something long
term supportable so a bunch of other people who paid attention will be
better off.

> >  If the tooling around this is too hard to use then that
> > seems like a useful thing to work on, having simple ways for people to
> > describe modules they build for maker type systems (ideally integrated
> > with schematic capture software) would be great for example.

> Again, what simple to use tooling do you use to describe the modem you
> connect to a serial port in the kernel (ideally integrated with
> schematic capture software)? I use none. I just connect the modem. I
> want to do that with all SPI devices that I do not intend to use with
> an in-kernel driver above the spi master driver that provides the
> communication channel as is the case with the serial modem.

This has been gone through repeatedly.  There's obvious differences here
- quite apart from anything else dedicated SPI connectors that are
completely fixed function and also not part of a bigger expansion
connector are rather rare and are a clear subset of the more general
plugin module case.  The way we handle serial ports is currently not
ideal anyway, and the general software stacks sitting on top of serial
ports are very different to those for SPI.

> It is not my very specific use case. It's common use case for
> devboards which have spi connectors.

That's really not that common - it *is* common to have expansion
connectors, but single function ones not so much and certainly not in
sufficient numbers that it's worth doing something that only works for
them alone (and then only in cases where spidev happens to be the best
way to control the device which is a further subset).  There's also the
fact that the limited connectors are more commonly found on the more
expensive eval boards which of course tend to have users who are more
technically experienced than the maker boards and should really not
struggle with basic tasks.

> If this particular way to solve the problem does not fit with the
> driver model does any of the previously listed ones fit better,
> perhaps with some changes?

> Or since you are familiar with the driver model perhaps you are away
> of a solution that was not mentioned so far that would accomplish this
> and fits with the driver model?

The answer remains the same, describe your module with a DT overlay and
load the overlay.  Or modify your DT directly if you like given that the
board is probably not enumerable anyway, especially if it's a flying
wire job.  If you want to bind devices at runtime you'll need new_id.

> Or if this use case does not fit into the driver model at all then
> perhaps the driver model needs updating to catch up with real world?

The problem isn't that the driver model can't cope, the problem is that
you are flat out refusing to do anything that involves describing the
hardware.
Michal Suchanek June 29, 2016, 3:32 a.m. UTC | #11
On 28 June 2016 at 22:57, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Jun 28, 2016 at 10:02:59PM +0200, Michal Suchanek wrote:
>
>> Of course I could add "my-shiny-new-board" compatible to the device
>> tree and spidev. Then when I build my-shiny-new-board2 and it happens
>> to also use userspace driver I will not bother to update the
>> compatible nor will anyone else doing this. So it's equivalent to
>> having "foobar123" in there from the start. Your proposed solution is
>> so complex that it will never be used as intended even if changing the
>> compatible on the node and driver could be both accomplished by an
>> echo to sysfs.
>
> If you are describing the connected SPI device as the board rather than
> the actual device you're doing things wrong, and if you have the same
> thing on two boards then /of course/ they should list the same
> compatible string so the existing software stack should work.  If people
> just use some random board description that happens to work right now
> even if it doesn't match their actual hardware then they're no worse off
> than with your hacks but we've been pushing people to do something long
> term supportable so a bunch of other people who paid attention will be
> better off.

ok, so my-shiny-new-spi-expansion-board and my-dhiny-new-expansion-board-2

It's unsupportable to pretend people will change the hardware
descriptio, either. It won't happen.

>
>> >  If the tooling around this is too hard to use then that
>> > seems like a useful thing to work on, having simple ways for people to
>> > describe modules they build for maker type systems (ideally integrated
>> > with schematic capture software) would be great for example.
>
>> Again, what simple to use tooling do you use to describe the modem you
>> connect to a serial port in the kernel (ideally integrated with
>> schematic capture software)? I use none. I just connect the modem. I
>> want to do that with all SPI devices that I do not intend to use with
>> an in-kernel driver above the spi master driver that provides the
>> communication channel as is the case with the serial modem.
>
> This has been gone through repeatedly.  There's obvious differences here
> - quite apart from anything else dedicated SPI connectors that are
> completely fixed function and also not part of a bigger expansion
> connector are rather rare and are a clear subset of the more general
> plugin module case.

The thing is that there are two common use cases. One is to get an
expansion board designed to connect to your big expansion connector.

The other is to get a generic expansion board and jumper wires. Of
course, you will not use all pins of your expansion connector this
way. On the other hand using the remaining pins becomes challenging
because of the jumper wires you just added. You can use some of the
remaining pins as GPIO or add more jumper wires and use some of the
remaining functions of the connector for another expansion board.

> The way we handle serial ports is currently not
> ideal anyway, and the general software stacks sitting on top of serial
> ports are very different to those for SPI.

Of course they are different. It's different communication channel
with different set of supported devices on the other end.

>
>> It is not my very specific use case. It's common use case for
>> devboards which have spi connectors.
>
> That's really not that common - it *is* common to have expansion
> connectors, but single function ones not so much and certainly not in
> sufficient numbers that it's worth doing something that only works for
> them alone (and then only in cases where spidev happens to be the best
> way to control the device which is a further subset).

It's not about single function connector. It's about using the
particular bus available on the connector. Devices using multiple
buses are very rare so even if you have specially designed expansion
board it rarely uses more than one function of the connector.

> There's also the
> fact that the limited connectors are more commonly found on the more
> expensive eval boards which of course tend to have users who are more
> technically experienced than the maker boards and should really not
> struggle with basic tasks.

On the other hand, expansion boards that designed for a particular
multi-function connector are much more expensive than generic boards
that just have a device on them and make the signals required to use
the device available on pin header which can be wired to any board.
Also the range of devices available on generic expansion board is much
larger than that available for a particular connector - be it Arduino
compatible, Pi compatible, Beagleboard compatible or anything else.

So it's very common to get a shiny new generic expansion board, look
at the pin signal description, wire these, look at the chip printing
and

1) it reads something like ds1301 and I see kernel driver for this -
mod the DT or write an overlay
2) it reads something like rc522 or pn533 and there is no kernel
driver or I want to use userspace driver although there is kernel
driver available - do something to talk to this from userspace

I don't see how case 2) differs from using a serial modem
significantly. For serial attached expansion boards or i2c ones all
that is required is to find which serial port or i2c bus to use. You
can talk to the device right away. For spi you have to do something to
load spidev.

The 'do something to load spidev' should be something that

 - is the same for any board used in 2). If it isn't people will us it
interchangeably anyway because from the kernel point of view there is
no change. You may pretend you are pushing for accurate hardware
description but that won't happen.
 - is something that can be prepared beforehand for users of a
devboard by the devboard distributor in their pre-built linux image

>
>> If this particular way to solve the problem does not fit with the
>> driver model does any of the previously listed ones fit better,
>> perhaps with some changes?
>
>> Or since you are familiar with the driver model perhaps you are away
>> of a solution that was not mentioned so far that would accomplish this
>> and fits with the driver model?
>
> The answer remains the same, describe your module with a DT overlay and
> load the overlay.  Or modify your DT directly if you like given that the
> board is probably not enumerable anyway, especially if it's a flying
> wire job.  If you want to bind devices at runtime you'll need new_id.

And the answer is always the same. That's not gonna happen. It's too
complex solution for a task that boils down to the same for every spi
expansion board used in 2) - load spidev. You are repeating yourself
uselessly.

>
>> Or if this use case does not fit into the driver model at all then
>> perhaps the driver model needs updating to catch up with real world?
>
> The problem isn't that the driver model can't cope, the problem is that
> you are flat out refusing to do anything that involves describing the
> hardware.

Because it does not make sense to describe the hardware for this use case.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown June 29, 2016, 6:02 p.m. UTC | #12
On Wed, Jun 29, 2016 at 05:32:48AM +0200, Michal Suchanek wrote:

> The other is to get a generic expansion board and jumper wires. Of
> course, you will not use all pins of your expansion connector this
> way. On the other hand using the remaining pins becomes challenging
> because of the jumper wires you just added. You can use some of the
> remaining pins as GPIO or add more jumper wires and use some of the
> remaining functions of the connector for another expansion board.

I'm more than familiar with flying wire systems.  They just aren't a big
deal here, this is a very technical audience making systems that aren't
going to have their software meaningfully distributed.  Users making
flying wire systems really ought to be capable of making the trivial DT
changes required for them, and if they just hack something not great up
locally that's not really a problem so long as it works for them.
Michal Suchanek June 30, 2016, 7:47 a.m. UTC | #13
On 29 June 2016 at 20:02, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Jun 29, 2016 at 05:32:48AM +0200, Michal Suchanek wrote:
>
>> The other is to get a generic expansion board and jumper wires. Of
>> course, you will not use all pins of your expansion connector this
>> way. On the other hand using the remaining pins becomes challenging
>> because of the jumper wires you just added. You can use some of the
>> remaining pins as GPIO or add more jumper wires and use some of the
>> remaining functions of the connector for another expansion board.
>
> I'm more than familiar with flying wire systems.  They just aren't a big
> deal here, this is a very technical audience making systems that aren't
> going to have their software meaningfully distributed.  Users making
> flying wire systems really ought to be capable of making the trivial DT
> changes required for them, and if they just hack something not great up
> locally that's not really a problem so long as it works for them.

No. Arduino tutorials made flying wire systems accessible to
non-technical audience. There are similar tutorials for boards running
vendor kernels.

Of course somebody with technical skill has to write the tutorial but
following it requires only understanding of written text - which is
becoming increasingly rare but is still not uncommon skill.

So to write such tutorial you have to give some steps to follow which
are reasonably likely to succeed on wide variety of boards and require
smallest possible changes to the board software.

For mainline this would currently entail

a) loading an overlay that lists spidev directly as compatible and
joke about obtuseness of the kernel developers that force you to read
the log spam

b) failing a) decompile your board devicetree, add the spidev
compatible - apply the overlay manually offline, recompile, and reboot

Adding new_id to spi bus does not simplify the thing at all. You still
have to modify your devicetree. It will at most remove the log spam.

I can even imagine people listing *all* their devices as compatible on
single dt node when the board does not ship with kernel patched for
loading overlays.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan O'Donovan June 30, 2016, 9:03 a.m. UTC | #14
On 06/30/2016 08:47 AM, Michal Suchanek wrote:
> On 29 June 2016 at 20:02, Mark Brown <broonie@kernel.org> wrote:
>> On Wed, Jun 29, 2016 at 05:32:48AM +0200, Michal Suchanek wrote:
>>
>>> The other is to get a generic expansion board and jumper wires. Of
>>> course, you will not use all pins of your expansion connector this
>>> way. On the other hand using the remaining pins becomes challenging
>>> because of the jumper wires you just added. You can use some of the
>>> remaining pins as GPIO or add more jumper wires and use some of the
>>> remaining functions of the connector for another expansion board.
>> I'm more than familiar with flying wire systems.  They just aren't a big
>> deal here, this is a very technical audience making systems that aren't
>> going to have their software meaningfully distributed.  Users making
>> flying wire systems really ought to be capable of making the trivial DT
>> changes required for them, and if they just hack something not great up
>> locally that's not really a problem so long as it works for them.
> No. Arduino tutorials made flying wire systems accessible to
> non-technical audience. There are similar tutorials for boards running
> vendor kernels.
In case its relevant, I'd like add to this point by emphasising that there is an increasing number of "maker" boards available, such as UP, Minnowboard, Intel Galileo, Intel Edison, Raspberry Pi, and more, which expose an open SPI bus interface on a pin header with the intention that a primary use of that SPI interface is decided at application level.  'spidev' is the adopted method for making that SPI interface accessible to those applications, and those boards do typically ship with Linux kernel that either (i) use spi_register_board_info in some board-specific platform driver to enable spidev by default for those interfaces, or (ii) include some hack in a custom kernel (such as adding a "spidev" compatible string) to allow it to be enabled with a DT or ACPI overlay, neither of which seem ideal.  Sorry if I'm just re-stating what's already been said.  Anyway, I would love to see a solution integrated for this, whatever the appropriate solution may be.
>
> Of course somebody with technical skill has to write the tutorial but
> following it requires only understanding of written text - which is
> becoming increasingly rare but is still not uncommon skill.
>
> So to write such tutorial you have to give some steps to follow which
> are reasonably likely to succeed on wide variety of boards and require
> smallest possible changes to the board software.
>
> For mainline this would currently entail
>
> a) loading an overlay that lists spidev directly as compatible and
> joke about obtuseness of the kernel developers that force you to read
> the log spam
>
> b) failing a) decompile your board devicetree, add the spidev
> compatible - apply the overlay manually offline, recompile, and reboot
>
> Adding new_id to spi bus does not simplify the thing at all. You still
> have to modify your devicetree. It will at most remove the log spam.
>
> I can even imagine people listing *all* their devices as compatible on
> single dt node when the board does not ship with kernel patched for
> loading overlays.
>
> Thanks
>
> Michal
> --
> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown July 1, 2016, 8:25 a.m. UTC | #15
On Thu, Jun 30, 2016 at 09:47:32AM +0200, Michal Suchanek wrote:

> So to write such tutorial you have to give some steps to follow which
> are reasonably likely to succeed on wide variety of boards and require
> smallest possible changes to the board software.

> For mainline this would currently entail

It's been repeatedly suggested to you that the tooling for this stuff
could use some work.  Please go and put some effort into that rather
than continuing this thread which is accomplishing nothing.
Michal Suchanek July 1, 2016, 8:58 a.m. UTC | #16
On 1 July 2016 at 10:25, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Jun 30, 2016 at 09:47:32AM +0200, Michal Suchanek wrote:
>
>> So to write such tutorial you have to give some steps to follow which
>> are reasonably likely to succeed on wide variety of boards and require
>> smallest possible changes to the board software.
>
>> For mainline this would currently entail
>
> It's been repeatedly suggested to you that the tooling for this stuff
> could use some work.  Please go and put some effort into that rather
> than continuing this thread which is accomplishing nothing.

You completely miss the point. No tooling will make people reconfigure
the kernel when the configuration in fact stays the same.

Sure the tooling does need work. And it would help getting the cases
when the tooling is NOT needed out of the way.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown July 1, 2016, 9:36 a.m. UTC | #17
On Thu, Jun 30, 2016 at 10:03:57AM +0100, Dan O'Donovan wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> In case its relevant, I'd like add to this point by emphasising that
> there is an increasing number of "maker" boards available, such as UP,

We know.

> which expose an open SPI bus interface on a pin header with the
> intention that a primary use of that SPI interface is decided at
> application level.  'spidev' is the adopted method for making that SPI
> interface accessible to those applications, and those boards do
> typically ship with Linux kernel that either (i) use

It's not the case that these boards (even when used with flying wires)
universally want or use spidev, people will want to use existing drivers
for devices that have them, and equally there are a bunch of non-maker
applications for spidev.  It's not like there is a special kind of
silicon that only makers use here.

> Anyway, I would love to see a solution integrated for this, whatever
> the appropriate solution may be.

There's a bunch of work going on to make overlays easier to use with
connectors which will hopefully help a lot here.  The other big bit that
seems to be missing is automating the process of going from schematic
capture for the modules to DTs so that people can design something and
get the bits needed for the software as trivially as possible.
Michal Suchanek July 1, 2016, 10:11 a.m. UTC | #18
On 1 July 2016 at 11:36, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Jun 30, 2016 at 10:03:57AM +0100, Dan O'Donovan wrote:
>
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns.  Doing this makes your messages much
> easier to read and reply to.

Sorry about that.
To get a reasonably portable access to e-mail I use web e-mail client
and it only knows about auto-wrapping.

>
>> In case its relevant, I'd like add to this point by emphasising that
>> there is an increasing number of "maker" boards available, such as UP,
>
> We know.
>
>> which expose an open SPI bus interface on a pin header with the
>> intention that a primary use of that SPI interface is decided at
>> application level.  'spidev' is the adopted method for making that SPI
>> interface accessible to those applications, and those boards do
>> typically ship with Linux kernel that either (i) use
>
> It's not the case that these boards (even when used with flying wires)
> universally want or use spidev, people will want to use existing drivers
> for devices that have them, and equally there are a bunch of non-maker
> applications for spidev.  It's not like there is a special kind of
> silicon that only makers use here.

Sure. The thing is that like quarter or so of the external expansion
SPI boards are used with userspace driver.
Either there is no kernel driver or it is unsuitable for the particular
application.

You say that people should describe the hardware to the kernel
regardless.

That's not going to happen. The hardware description in kernel
has no meaning when all the kernel does is provide a communication
channel for use by userspace application.

So people using several devices with spidev want to change the
application but not the kernel configuration. Changing the kernel
configuration only makes sense when you want a different service
from the kernel as a result. Otherwise it's needless obstruction
that people can and naturally will avoid.

And people here does not mean one person connecting random
things to an EVB to design a STB or tablet.
With maker boards people can mean all the people using a particular
board with the software that was preinstalled on it.

>
>> Anyway, I would love to see a solution integrated for this, whatever
>> the appropriate solution may be.
>
> There's a bunch of work going on to make overlays easier to use with
> connectors which will hopefully help a lot here.  The other big bit that
> seems to be missing is automating the process of going from schematic
> capture for the modules to DTs so that people can design something and
> get the bits needed for the software as trivially as possible.

That's nice for the cases when you do want to use a kernel driver.
I think it's easy enough to write overlays even without automatic conversion
from schematic when the application calls for it.

That's not what this is about, however.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown July 1, 2016, 3 p.m. UTC | #19
On Fri, Jul 01, 2016 at 10:58:34AM +0200, Michal Suchanek wrote:
> On 1 July 2016 at 10:25, Mark Brown <broonie@kernel.org> wrote:

> > It's been repeatedly suggested to you that the tooling for this stuff
> > could use some work.  Please go and put some effort into that rather
> > than continuing this thread which is accomplishing nothing.

> You completely miss the point. No tooling will make people reconfigure
> the kernel when the configuration in fact stays the same.

> Sure the tooling does need work. And it would help getting the cases
> when the tooling is NOT needed out of the way.

I understand the problem perfectly, no amount of repeating yourself is
going to change the problems that the bodge you are trying to force in
creates for other users and the maintainability of the system.
Michal Suchanek July 1, 2016, 3:37 p.m. UTC | #20
On 1 July 2016 at 17:00, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 01, 2016 at 10:58:34AM +0200, Michal Suchanek wrote:
>> On 1 July 2016 at 10:25, Mark Brown <broonie@kernel.org> wrote:
>
>> > It's been repeatedly suggested to you that the tooling for this stuff
>> > could use some work.  Please go and put some effort into that rather
>> > than continuing this thread which is accomplishing nothing.
>
>> You completely miss the point. No tooling will make people reconfigure
>> the kernel when the configuration in fact stays the same.
>
>> Sure the tooling does need work. And it would help getting the cases
>> when the tooling is NOT needed out of the way.
>
> I understand the problem perfectly, no amount of repeating yourself is
> going to change the problems that the bodge you are trying to force in
> creates for other users and the maintainability of the system.

Can you, please, specify what problems this patch creates for other users
and maintainability?

Without stating the problems of this solution clearly or proposing alternate
workable solution there is not much that can be done.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown July 1, 2016, 4:22 p.m. UTC | #21
On Fri, Jul 01, 2016 at 05:37:53PM +0200, Michal Suchanek wrote:

> Can you, please, specify what problems this patch creates for other users
> and maintainability?

To repeat yet again a major design goal for DT is to describe the
hardware rather than implementation details of the software you plan to
run on the system.  Not doing this breaks upgrades or changes in our
ideas of how we control a given bit of hardware.

> Without stating the problems of this solution clearly or proposing alternate
> workable solution there is not much that can be done.

You are rejecting out of hand any suggestion that is not the one
solution you are demanding.

I'm done with this thread, please come back with new code that fits in
with the device model and device tree designs.
Michal Suchanek July 1, 2016, 6:56 p.m. UTC | #22
On 1 July 2016 at 18:22, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 01, 2016 at 05:37:53PM +0200, Michal Suchanek wrote:
>
>> Can you, please, specify what problems this patch creates for other users
>> and maintainability?
>
> To repeat yet again a major design goal for DT is to describe the
> hardware rather than implementation details of the software you plan to
> run on the system.  Not doing this breaks upgrades or changes in our
> ideas of how we control a given bit of hardware.
>
>> Without stating the problems of this solution clearly or proposing alternate
>> workable solution there is not much that can be done.
>
> You are rejecting out of hand any suggestion that is not the one
> solution you are demanding.

Your only suggestion is that hardware that is not driven by kernel is
described to the kernel. I have written in great detail why this does
not make sense.

So to me it seems that you reject out of hand any suggestion that
is not the one solution you are demanding. You have not raised any
concrete reason against the proposed solution other than it does not
describe the hardware. The hardware cannot be described for practical
usability reasons.

>
> I'm done with this thread, please come back with new code that fits in
> with the device model and device tree designs.

Is there any solution that fits the driver model and allows using spidev with
arbitrary device without describing said device in devicetree?

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Suchanek July 1, 2016, 7:36 p.m. UTC | #23
On 1 July 2016 at 20:56, Michal Suchanek <hramrach@gmail.com> wrote:
> On 1 July 2016 at 18:22, Mark Brown <broonie@kernel.org> wrote:
>> On Fri, Jul 01, 2016 at 05:37:53PM +0200, Michal Suchanek wrote:
>>
>>> Can you, please, specify what problems this patch creates for other users
>>> and maintainability?
>>
>> To repeat yet again a major design goal for DT is to describe the
>> hardware rather than implementation details of the software you plan to
>> run on the system.  Not doing this breaks upgrades or changes in our
>> ideas of how we control a given bit of hardware.

Oh, and can you point out where in this patchset are implementation details
of the software I intend to run put in the devicetree?

I don't see any such thing here.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 6470eb8..a896aed 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -199,6 +199,28 @@  static ssize_t unbind_store(struct device_driver *drv, const char *buf,
 }
 static DRIVER_ATTR_WO(unbind);
 
+static const char *driver_override_buses[] = {
+	"spi",
+	NULL
+};
+
+static inline bool driver_match_override(struct device_driver *drv,
+					 struct device *dev)
+{
+	struct bus_type *bus = bus_get(drv->bus);
+	int i;
+
+	for (i = 0; driver_override_buses[i]; i++) {
+		if (!strcmp(bus->name, driver_override_buses[i])) {
+			pr_notice("Overriding id match on manual driver binding:\n bus: %s  driver: %s  device: %s\n",
+				  bus->name, drv->name, dev_name(dev));
+			return true;
+		}
+	}
+
+	return false;
+}
+
 /*
  * Manually attach a device to a driver.
  * Note: the driver must want to bind to the device,
@@ -212,7 +234,8 @@  static ssize_t bind_store(struct device_driver *drv, const char *buf,
 	int err = -ENODEV;
 
 	dev = bus_find_device_by_name(bus, NULL, buf);
-	if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
+	if (dev && dev->driver == NULL && (driver_match_device(drv, dev)
+	    || driver_match_override(drv, dev))) {
 		if (dev->parent)	/* Needed for USB */
 			device_lock(dev->parent);
 		device_lock(dev);