diff mbox

spi: Force the registration of the spidev devices

Message ID 1431462804-30467-1-git-send-email-maxime.ripard@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maxime Ripard May 12, 2015, 8:33 p.m. UTC
spidev device registration has always been a controversial subject since the
move to DT.

Obviously, a spidev node has nothing to do in the DT, and the position so far
has been to add the compatible of the devices to drive through spidev to the
list of the compatibles spidev can handle.

While this is nicer than the DT solution because of its accurate hardware
representation, it's still not perfect because you might not have access to the
DT, or you might be driving a completely generic device (such as a
microcontroller) that might be used for something else in a different
context/board.

Solve this by registering automatically spidev devices for all the unused chip
selects when a master registers itself against the spi core.

This also adds an i2cdev-like feeling, where you get all the spidev devices all
the time, without any modification.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/spi/spi.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

Comments

Mark Brown May 13, 2015, 11:26 a.m. UTC | #1
On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:

> While this is nicer than the DT solution because of its accurate hardware
> representation, it's still not perfect because you might not have access to the
> DT, or you might be driving a completely generic device (such as a
> microcontroller) that might be used for something else in a different
> context/board.

Greg, you're copied on this because this seems to be a generic problem
that should perhaps be solved at a driver model level - having a way to
bind userspace access to devices that we don't otherwise have a driver
for.  The subsystem could specify the UIO driver to use when no other
driver is available.

> Solve this by registering automatically spidev devices for all the unused chip
> selects when a master registers itself against the spi core.

So, aside from the concern about this being generic the other thing here
is that we often have devices offering more chip selects than can
physically be used in a system but not doing anything to ensure that the
invalid ones can't be used.  It's unclear to me if that's OK or not, it
might be since it's root only I think but I need to think it through a
bit.

> This also adds an i2cdev-like feeling, where you get all the spidev devices all
> the time, without any modification.

I2C is a bit safer here here since it's a shared bus so you can't do
anything to devices not connected to the bus by mistake.

> +		/*
> +		 * This is far from perfect since an addition might be
> +		 * done between here and the call to spi_add_device,
> +		 * but we can't hold the lock and call spi_add_device
> +		 * either, as it would trigger a deadlock.
> +		 *
> +		 * If such a race occurs, spi_add_device will still
> +		 * catch it though, as it also checks for devices
> +		 * being registered several times on the same chip
> +		 * select.
> +		*/
> +		status = bus_for_each_dev(&spi_bus_type, NULL, spi,
> +					  spi_dev_check);
> +		if (status) {
> +			dev_dbg(&master->dev, "Chipselect already in use.. Skipping.");
> +			spi_dev_put(spi);
> +			continue;
> +		}

This still leaves us in the situation where if we do know the device
that is connected we have to explicitly bind it in spidev which is
apparently unreasonably difficult for people.  I'm also concerned about
the interactions with DT overlays here - what happens if a DT overlay
or other dynamic hardware instantiation comes along later and does bind
something to this chip select?  It seems like we should be able to
combine the two models, and the fact that we only create these devices
with a Kconfig option is a bit of an interesting thing here.
Michal Suchanek May 13, 2015, 12:35 p.m. UTC | #2
On 13 May 2015 at 13:26, Mark Brown <broonie@kernel.org> wrote:
> On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
>
>> While this is nicer than the DT solution because of its accurate hardware
>> representation, it's still not perfect because you might not have access to the
>> DT, or you might be driving a completely generic device (such as a
>> microcontroller) that might be used for something else in a different
>> context/board.
>
> Greg, you're copied on this because this seems to be a generic problem
> that should perhaps be solved at a driver model level - having a way to
> bind userspace access to devices that we don't otherwise have a driver
> for.  The subsystem could specify the UIO driver to use when no other
> driver is available.
>
>> Solve this by registering automatically spidev devices for all the unused chip
>> selects when a master registers itself against the spi core.
>
> So, aside from the concern about this being generic the other thing here
> is that we often have devices offering more chip selects than can
> physically be used in a system but not doing anything to ensure that the
> invalid ones can't be used.  It's unclear to me if that's OK or not, it
> might be since it's root only I think but I need to think it through a
> bit.

Presumably you could add dt nodes for the chipselects and specify they
are disabled but it does not work for me currently.

This is mostly a cosmetic issue. The chipselects were always there but
now they are visible. Unlike the case when you explicitly bind spidev
using dt node the unusable chipselects are also bound.

>
>> This also adds an i2cdev-like feeling, where you get all the spidev devices all
>> the time, without any modification.
>
> I2C is a bit safer here here since it's a shared bus so you can't do
> anything to devices not connected to the bus by mistake.

This is quite safe too. Unless the device chipselect is activated the
device should ignore whatever you write on the bus - unless you get
the chipselect polarity wrong.

But you can get i2c address and lots of other things wrong too.

>
>> +             /*
>> +              * This is far from perfect since an addition might be
>> +              * done between here and the call to spi_add_device,
>> +              * but we can't hold the lock and call spi_add_device
>> +              * either, as it would trigger a deadlock.
>> +              *
>> +              * If such a race occurs, spi_add_device will still
>> +              * catch it though, as it also checks for devices
>> +              * being registered several times on the same chip
>> +              * select.
>> +             */
>> +             status = bus_for_each_dev(&spi_bus_type, NULL, spi,
>> +                                       spi_dev_check);
>> +             if (status) {
>> +                     dev_dbg(&master->dev, "Chipselect already in use.. Skipping.");
>> +                     spi_dev_put(spi);
>> +                     continue;
>> +             }
>
> This still leaves us in the situation where if we do know the device
> that is connected we have to explicitly bind it in spidev which is
> apparently unreasonably difficult for people.  I'm also concerned about
> the interactions with DT overlays here - what happens if a DT overlay
> or other dynamic hardware instantiation comes along later and does bind
> something to this chip select?  It seems like we should be able to
> combine the two models, and the fact that we only create these devices
> with a Kconfig option is a bit of an interesting thing here.

It does not bind anything because the chiselect is busy. That's why I
use a patch which disregards spidev when determining if a chiselect is
busy. That has the problem that you can access devices in use by
kernel using spidev then. Ideally spidev (as checked by module alias
or whatever) would be unbound when other driver requests the
chipselect and rebound when the driver quits.

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
Maxime Ripard May 13, 2015, 12:51 p.m. UTC | #3
On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
> > Solve this by registering automatically spidev devices for all the unused chip
> > selects when a master registers itself against the spi core.
> 
> So, aside from the concern about this being generic the other thing here
> is that we often have devices offering more chip selects than can
> physically be used in a system but not doing anything to ensure that the
> invalid ones can't be used.  It's unclear to me if that's OK or not, it
> might be since it's root only I think but I need to think it through a
> bit.

I might plead guilty here... :)

I'd say we're also ok because if we delegate the device driving logic
to userspace, we should expect it to know what it does to first drive
the device properly, but also to open the right device for this.

What's the worst that could happen in such a case? The data are output
without any chipselect line being driven by the controller? Isn't that
supposed to be ignored by the devices?

> > This also adds an i2cdev-like feeling, where you get all the
> > spidev devices all the time, without any modification.
> 
> I2C is a bit safer here since it's a shared bus so you can't do
> anything to devices not connected to the bus by mistake.

I'm not sure to understand what you mean here. How is SPI different
from that aspect?

> > +		/*
> > +		 * This is far from perfect since an addition might be
> > +		 * done between here and the call to spi_add_device,
> > +		 * but we can't hold the lock and call spi_add_device
> > +		 * either, as it would trigger a deadlock.
> > +		 *
> > +		 * If such a race occurs, spi_add_device will still
> > +		 * catch it though, as it also checks for devices
> > +		 * being registered several times on the same chip
> > +		 * select.
> > +		*/
> > +		status = bus_for_each_dev(&spi_bus_type, NULL, spi,
> > +					  spi_dev_check);
> > +		if (status) {
> > +			dev_dbg(&master->dev, "Chipselect already in use.. Skipping.");
> > +			spi_dev_put(spi);
> > +			continue;
> > +		}
> 
> This still leaves us in the situation where if we do know the device
> that is connected we have to explicitly bind it in spidev which is
> apparently unreasonably difficult for people.

You can still do that, but the point is that you don't have to.

If you know what device you have, and want to use spidev on that
device, it will still work, since it will create an spidev device when
we will parse the DT.

That device will then be skipped by that logic, which is ok.

However, if you don't specify anything in your DT, and have no device
created because you don't really know what's on the other end, this
logic will create the spidev device so that you'll still get an spidev
node exported to the userspace.

> I'm also concerned about the interactions with DT overlays here -
> what happens if a DT overlay or other dynamic hardware instantiation
> comes along later and does bind something to this chip select?  It
> seems like we should be able to combine the two models, and the fact
> that we only create these devices with a Kconfig option is a bit of
> an interesting thing here.

I think the safe approach would be, just like I told in this thread,
to just check whether the modalias is spidev. If it is, destroy the
previous (spidev) device, create a new device as specified by the DT,
you're done.

Maxime
Mark Brown May 13, 2015, 2:36 p.m. UTC | #4
On Wed, May 13, 2015 at 02:51:02PM +0200, Maxime Ripard wrote:

> I'd say we're also ok because if we delegate the device driving logic
> to userspace, we should expect it to know what it does to first drive
> the device properly, but also to open the right device for this.

> What's the worst that could happen in such a case? The data are output
> without any chipselect line being driven by the controller? Isn't that
> supposed to be ignored by the devices?

I'm more worried about the chip select line being connected to the
"make the board catch fire" signal or whatever (more realistically
causing us to drive against some other external component) if the extra
chip selects weren't pinmuxed away.

> > > This also adds an i2cdev-like feeling, where you get all the
> > > spidev devices all the time, without any modification.

> > I2C is a bit safer here since it's a shared bus so you can't do
> > anything to devices not connected to the bus by mistake.

> I'm not sure to understand what you mean here. How is SPI different
> from that aspect?

Chip select signals.

> > This still leaves us in the situation where if we do know the device
> > that is connected we have to explicitly bind it in spidev which is
> > apparently unreasonably difficult for people.

> You can still do that, but the point is that you don't have to.

Right, but that's not what I'd expect to happen (and seems to make it
easier for people to not list things in the DT at all which doesn't seem
great).  If we're going to make it available by default I'd expect to be
able to use a userspace driver with anything that doesn't have a driver
bound rather than with devices that explicitly don't have any
identification.

> > I'm also concerned about the interactions with DT overlays here -
> > what happens if a DT overlay or other dynamic hardware instantiation
> > comes along later and does bind something to this chip select?  It
> > seems like we should be able to combine the two models, and the fact
> > that we only create these devices with a Kconfig option is a bit of
> > an interesting thing here.

> I think the safe approach would be, just like I told in this thread,
> to just check whether the modalias is spidev. If it is, destroy the
> previous (spidev) device, create a new device as specified by the DT,
> you're done.

Sure, but I don't see code for that here.
Michal Suchanek May 13, 2015, 3:31 p.m. UTC | #5
On 13 May 2015 at 16:36, Mark Brown <broonie@kernel.org> wrote:
> On Wed, May 13, 2015 at 02:51:02PM +0200, Maxime Ripard wrote:
>
>> I'd say we're also ok because if we delegate the device driving logic
>> to userspace, we should expect it to know what it does to first drive
>> the device properly, but also to open the right device for this.
>
>> What's the worst that could happen in such a case? The data are output
>> without any chipselect line being driven by the controller? Isn't that
>> supposed to be ignored by the devices?
>
> I'm more worried about the chip select line being connected to the
> "make the board catch fire" signal or whatever (more realistically
> causing us to drive against some other external component) if the extra
> chip selects weren't pinmuxed away.

They would not be pinmuxed away if

1) they are unused by anything else and cs happens to be the default
2) they are used by a device not in DT

right?

For the latter case we might want some way to disable unused
chipselects as well as for the cosmetic issue of multitude of useless
devices popping up.

But you know, unused i2c bus can be also connected to "make the board
catch fire" trace and nobody would notice until somebody has the great
idea to probe it. Incidentally, both i2c and spi cs are active-low
iirc.

>> > This still leaves us in the situation where if we do know the device
>> > that is connected we have to explicitly bind it in spidev which is
>> > apparently unreasonably difficult for people.
>
>> You can still do that, but the point is that you don't have to.
>
> Right, but that's not what I'd expect to happen (and seems to make it
> easier for people to not list things in the DT at all which doesn't seem
> great).  If we're going to make it available by default I'd expect to be
> able to use a userspace driver with anything that doesn't have a driver
> bound rather than with devices that explicitly don't have any
> identification.

Remember that spidev can and is used for boards which have spi
*CONNECTOR* to which you can connect some random gadget and use a
userspace program to communicate with it.

It is cool that you can load a dt overlay if the gadget happens to
have a kernel driver.

However, what identification do you write in the dt when there is no
need to use a kernel driver at all?

The option to create a node with 'spidev' compatible was rejected as broken.

The option to add new compatible to the spidev driver every time you
want to connect a new device was rejected as absurdly complex for end
users who have a board with system already preinstalled because it
requires adding the compatible in the spidev source and rebuilding the
kernel.

So this patch implements another option which is to bind spidev
*always*. The configuration of the port is then not recorded in the DT
and it's up to the user to visually check the port or apply other
relevant heuristic and run the correct userspace application.

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
Greg Kroah-Hartman May 13, 2015, 3:37 p.m. UTC | #6
On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
> On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
> 
> > While this is nicer than the DT solution because of its accurate hardware
> > representation, it's still not perfect because you might not have access to the
> > DT, or you might be driving a completely generic device (such as a
> > microcontroller) that might be used for something else in a different
> > context/board.
> 
> Greg, you're copied on this because this seems to be a generic problem
> that should perhaps be solved at a driver model level - having a way to
> bind userspace access to devices that we don't otherwise have a driver
> for.  The subsystem could specify the UIO driver to use when no other
> driver is available.

That doesn't really work.  I've been talking to the ACPI people about
this, and the problem is "don't otherwise have a driver for" is an
impossible thing to prove, as you never know when a driver is going to
be loaded from userspace.

You can easily bind drivers to devices today from userspace, why not
just use the built-in functionality you have today if you "know" that
there is no driver for this hardware.

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 May 13, 2015, 3:52 p.m. UTC | #7
On 13 May 2015 at 17:37, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
>> On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
>>
>> > While this is nicer than the DT solution because of its accurate hardware
>> > representation, it's still not perfect because you might not have access to the
>> > DT, or you might be driving a completely generic device (such as a
>> > microcontroller) that might be used for something else in a different
>> > context/board.
>>
>> Greg, you're copied on this because this seems to be a generic problem
>> that should perhaps be solved at a driver model level - having a way to
>> bind userspace access to devices that we don't otherwise have a driver
>> for.  The subsystem could specify the UIO driver to use when no other
>> driver is available.
>
> That doesn't really work.  I've been talking to the ACPI people about
> this, and the problem is "don't otherwise have a driver for" is an
> impossible thing to prove, as you never know when a driver is going to
> be loaded from userspace.

Yes, exactly. That's why binding spidev in addition to regular drivers
or have spidev bind always and get unbound when another driver tries
to bind the CS is preferable. The latter is pretty much specifying an
UIO driver to use when no other driver is available in the light of
the fact that 'is available' might change over time. You can prove
that a driver is not available right now.

>
> You can easily bind drivers to devices today from userspace, why not
> just use the built-in functionality you have today if you "know" that
> there is no driver for this hardware.
>

And what exactly do you bind the driver to when there is no device
created with the built-in functionality you have today?

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 May 13, 2015, 5:13 p.m. UTC | #8
On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:

> You can easily bind drivers to devices today from userspace, why not
> just use the built-in functionality you have today if you "know" that
> there is no driver for this hardware.

So, that was my original suggestion too but people were complaining that
this wasn't a generally supported feature and requires specific support
of some kind in the bus (which nobody posted a patch for).  I have to
confess I didn't investigate too deeply myself.
Greg Kroah-Hartman May 13, 2015, 5:20 p.m. UTC | #9
On Wed, May 13, 2015 at 06:13:00PM +0100, Mark Brown wrote:
> On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:
> 
> > You can easily bind drivers to devices today from userspace, why not
> > just use the built-in functionality you have today if you "know" that
> > there is no driver for this hardware.
> 
> So, that was my original suggestion too but people were complaining that
> this wasn't a generally supported feature and requires specific support
> of some kind in the bus (which nobody posted a patch for).

I don't understand the complaint, it's a very "generally supported
feature" and has been there for a very long time for any bus that wants
to use it.

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
Mark Brown May 13, 2015, 5:39 p.m. UTC | #10
On Wed, May 13, 2015 at 10:20:28AM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 13, 2015 at 06:13:00PM +0100, Mark Brown wrote:

> > So, that was my original suggestion too but people were complaining that
> > this wasn't a generally supported feature and requires specific support
> > of some kind in the bus (which nobody posted a patch for).

> I don't understand the complaint, it's a very "generally supported
> feature" and has been there for a very long time for any bus that wants
> to use it.

So it's not something that just works for all buses and someone would
need to write a patch (hint, hint - that's one of the big blockers
here)?  It is a bit surprising to me that this would be something that
the buses would need to cope with individually since for most things the
matching mechanics are taken care of in the driver core and we're
explicitly overriding them here.
Mark Brown May 13, 2015, 5:43 p.m. UTC | #11
On Wed, May 13, 2015 at 05:31:05PM +0200, Michal Suchanek wrote:

> But you know, unused i2c bus can be also connected to "make the board
> catch fire" trace and nobody would notice until somebody has the great
> idea to probe it. Incidentally, both i2c and spi cs are active-low
> iirc.

Someone would need to go and explicitly mark an I2C controller as
enabled on a board for that to happen.
Maxime Ripard May 13, 2015, 5:50 p.m. UTC | #12
Hi Greg,

On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
> > On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
> > 
> > > While this is nicer than the DT solution because of its accurate hardware
> > > representation, it's still not perfect because you might not have access to the
> > > DT, or you might be driving a completely generic device (such as a
> > > microcontroller) that might be used for something else in a different
> > > context/board.
> > 
> > Greg, you're copied on this because this seems to be a generic problem
> > that should perhaps be solved at a driver model level - having a way to
> > bind userspace access to devices that we don't otherwise have a driver
> > for.  The subsystem could specify the UIO driver to use when no other
> > driver is available.
> 
> That doesn't really work.  I've been talking to the ACPI people about
> this, and the problem is "don't otherwise have a driver for" is an
> impossible thing to prove, as you never know when a driver is going to
> be loaded from userspace.
> 
> You can easily bind drivers to devices today from userspace, why not
> just use the built-in functionality you have today if you "know" that
> there is no driver for this hardware.

What we're really after here is that we want to have an spidev
instance when we don't even have a device.

And since SPI isn't really doing any kind of hotplug, the only
situation that might be problematic is if we have the DT overlays
registering new devices.

Maxime
Mark Brown May 13, 2015, 6:12 p.m. UTC | #13
On Wed, May 13, 2015 at 07:50:34PM +0200, Maxime Ripard wrote:
> On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:

> > That doesn't really work.  I've been talking to the ACPI people about
> > this, and the problem is "don't otherwise have a driver for" is an
> > impossible thing to prove, as you never know when a driver is going to
> > be loaded from userspace.

> > You can easily bind drivers to devices today from userspace, why not
> > just use the built-in functionality you have today if you "know" that
> > there is no driver for this hardware.

> What we're really after here is that we want to have an spidev
> instance when we don't even have a device.

We do?

> And since SPI isn't really doing any kind of hotplug, the only
> situation that might be problematic is if we have the DT overlays
> registering new devices.

Well, we still have the driver loaded from userspace in the future
situation as well - the hardware might be static but the software isn't.
Greg Kroah-Hartman May 13, 2015, 6:16 p.m. UTC | #14
On Wed, May 13, 2015 at 06:39:22PM +0100, Mark Brown wrote:
> On Wed, May 13, 2015 at 10:20:28AM -0700, Greg Kroah-Hartman wrote:
> > On Wed, May 13, 2015 at 06:13:00PM +0100, Mark Brown wrote:
> 
> > > So, that was my original suggestion too but people were complaining that
> > > this wasn't a generally supported feature and requires specific support
> > > of some kind in the bus (which nobody posted a patch for).
> 
> > I don't understand the complaint, it's a very "generally supported
> > feature" and has been there for a very long time for any bus that wants
> > to use it.
> 
> So it's not something that just works for all buses and someone would
> need to write a patch (hint, hint - that's one of the big blockers
> here)?  It is a bit surprising to me that this would be something that
> the buses would need to cope with individually since for most things the
> matching mechanics are taken care of in the driver core and we're
> explicitly overriding them here.

It should "just work" for all busses, but if you want to add a "new_id"
sysfs file, you need to add the logic for that to your bus.  It's the
bind/unbind files in the driver directories.

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
Greg Kroah-Hartman May 13, 2015, 6:17 p.m. UTC | #15
On Wed, May 13, 2015 at 07:50:34PM +0200, Maxime Ripard wrote:
> Hi Greg,
> 
> On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:
> > On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
> > > On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
> > > 
> > > > While this is nicer than the DT solution because of its accurate hardware
> > > > representation, it's still not perfect because you might not have access to the
> > > > DT, or you might be driving a completely generic device (such as a
> > > > microcontroller) that might be used for something else in a different
> > > > context/board.
> > > 
> > > Greg, you're copied on this because this seems to be a generic problem
> > > that should perhaps be solved at a driver model level - having a way to
> > > bind userspace access to devices that we don't otherwise have a driver
> > > for.  The subsystem could specify the UIO driver to use when no other
> > > driver is available.
> > 
> > That doesn't really work.  I've been talking to the ACPI people about
> > this, and the problem is "don't otherwise have a driver for" is an
> > impossible thing to prove, as you never know when a driver is going to
> > be loaded from userspace.
> > 
> > You can easily bind drivers to devices today from userspace, why not
> > just use the built-in functionality you have today if you "know" that
> > there is no driver for this hardware.
> 
> What we're really after here is that we want to have an spidev
> instance when we don't even have a device.

That's crazy, just create a device, things do not work without one.

> And since SPI isn't really doing any kind of hotplug, the only
> situation that might be problematic is if we have the DT overlays
> registering new devices.

I have no idea what this all is, sorry, patches would be good if you
want to get your idea across.  Maybe it was sent early in this thread,
but I can't seem to find one...

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
Mark Brown May 13, 2015, 6:32 p.m. UTC | #16
On Wed, May 13, 2015 at 11:16:31AM -0700, Greg Kroah-Hartman wrote:

> It should "just work" for all busses, but if you want to add a "new_id"
> sysfs file, you need to add the logic for that to your bus.  It's the
> bind/unbind files in the driver directories.

Oh, right.  For this to be useful here we'd need to implement a new_id
file, bind and unbind don't do anything helpful here.  I think I'd have
expected this to have a default implementation that non-enumerable buses
could pick up.
Greg Kroah-Hartman May 13, 2015, 6:36 p.m. UTC | #17
On Wed, May 13, 2015 at 07:32:11PM +0100, Mark Brown wrote:
> On Wed, May 13, 2015 at 11:16:31AM -0700, Greg Kroah-Hartman wrote:
> 
> > It should "just work" for all busses, but if you want to add a "new_id"
> > sysfs file, you need to add the logic for that to your bus.  It's the
> > bind/unbind files in the driver directories.
> 
> Oh, right.  For this to be useful here we'd need to implement a new_id
> file, bind and unbind don't do anything helpful here.  I think I'd have
> expected this to have a default implementation that non-enumerable buses
> could pick up.

No one has ever asked for that, so feel free to make a patch that adds
it :)

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
Mark Brown May 13, 2015, 6:51 p.m. UTC | #18
On Wed, May 13, 2015 at 11:36:53AM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 13, 2015 at 07:32:11PM +0100, Mark Brown wrote:

> > Oh, right.  For this to be useful here we'd need to implement a new_id
> > file, bind and unbind don't do anything helpful here.  I think I'd have
> > expected this to have a default implementation that non-enumerable buses
> > could pick up.

> No one has ever asked for that, so feel free to make a patch that adds
> it :)

Maxime?  :P
Maxime Ripard May 13, 2015, 7:09 p.m. UTC | #19
On Wed, May 13, 2015 at 03:36:10PM +0100, Mark Brown wrote:
> On Wed, May 13, 2015 at 02:51:02PM +0200, Maxime Ripard wrote:
> 
> > I'd say we're also ok because if we delegate the device driving logic
> > to userspace, we should expect it to know what it does to first drive
> > the device properly, but also to open the right device for this.
> 
> > What's the worst that could happen in such a case? The data are output
> > without any chipselect line being driven by the controller? Isn't that
> > supposed to be ignored by the devices?
> 
> I'm more worried about the chip select line being connected to the
> "make the board catch fire" signal or whatever (more realistically
> causing us to drive against some other external component) if the extra
> chip selects weren't pinmuxed away.

It seems we've had this discussion at lot lately ;)

That indeed might be problematic....

> > > > This also adds an i2cdev-like feeling, where you get all the
> > > > spidev devices all the time, without any modification.
> 
> > > I2C is a bit safer here since it's a shared bus so you can't do
> > > anything to devices not connected to the bus by mistake.
> 
> > I'm not sure to understand what you mean here. How is SPI different
> > from that aspect?
> 
> Chip select signals.

Well, if it's not connected to the bus, it probably won't be connected
to the chip select either, will it?

> > > This still leaves us in the situation where if we do know the device
> > > that is connected we have to explicitly bind it in spidev which is
> > > apparently unreasonably difficult for people.
> 
> > You can still do that, but the point is that you don't have to.
> 
> Right, but that's not what I'd expect to happen (and seems to make it
> easier for people to not list things in the DT at all which doesn't seem
> great).  If we're going to make it available by default I'd expect to be
> able to use a userspace driver with anything that doesn't have a driver
> bound rather than with devices that explicitly don't have any
> identification.

The point is that if we don't have anything declared in the DT, we
won't even have a device. So we can't really expect that the device
will not be bound to a driver, because it won't even be there in the
first place.

> > > I'm also concerned about the interactions with DT overlays here -
> > > what happens if a DT overlay or other dynamic hardware instantiation
> > > comes along later and does bind something to this chip select?  It
> > > seems like we should be able to combine the two models, and the fact
> > > that we only create these devices with a Kconfig option is a bit of
> > > an interesting thing here.
> 
> > I think the safe approach would be, just like I told in this thread,
> > to just check whether the modalias is spidev. If it is, destroy the
> > previous (spidev) device, create a new device as specified by the DT,
> > you're done.
> 
> Sure, but I don't see code for that here.

No, of course. Remember that this code was written before the overlays
were posted.

Maxime
Geert Uytterhoeven May 13, 2015, 7:10 p.m. UTC | #20
On Wed, May 13, 2015 at 2:51 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
>> > This also adds an i2cdev-like feeling, where you get all the
>> > spidev devices all the time, without any modification.
>>
>> I2C is a bit safer here since it's a shared bus so you can't do
>> anything to devices not connected to the bus by mistake.
>
> I'm not sure to understand what you mean here. How is SPI different
> from that aspect?

If you talk to a nonexistent i2c device, nothing happens, as it just sends
a message with a nonexistent address on the shared bus.

If you talk to a nonexistent spi device, hell may break loose if e.g. some
"smart" hardware engineer used the "unused" CS as a pull-up for the
_RESET line on an external device... It's a bit like banging random
"unused" GPIOs.

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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
Maxime Ripard May 13, 2015, 7:17 p.m. UTC | #21
On Wed, May 13, 2015 at 07:51:49PM +0100, Mark Brown wrote:
> On Wed, May 13, 2015 at 11:36:53AM -0700, Greg Kroah-Hartman wrote:
> > On Wed, May 13, 2015 at 07:32:11PM +0100, Mark Brown wrote:
> 
> > > Oh, right.  For this to be useful here we'd need to implement a new_id
> > > file, bind and unbind don't do anything helpful here.  I think I'd have
> > > expected this to have a default implementation that non-enumerable buses
> > > could pick up.
> 
> > No one has ever asked for that, so feel free to make a patch that adds
> > it :)
> 
> Maxime?  :P

I can try to give it a shot.

Maxime
Geert Uytterhoeven May 13, 2015, 7:23 p.m. UTC | #22
On Wed, May 13, 2015 at 8:17 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, May 13, 2015 at 07:50:34PM +0200, Maxime Ripard wrote:
>> On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:
>> > On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
>> > > On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
>> > >
>> > > > While this is nicer than the DT solution because of its accurate hardware
>> > > > representation, it's still not perfect because you might not have access to the
>> > > > DT, or you might be driving a completely generic device (such as a
>> > > > microcontroller) that might be used for something else in a different
>> > > > context/board.
>> > >
>> > > Greg, you're copied on this because this seems to be a generic problem
>> > > that should perhaps be solved at a driver model level - having a way to
>> > > bind userspace access to devices that we don't otherwise have a driver
>> > > for.  The subsystem could specify the UIO driver to use when no other
>> > > driver is available.
>> >
>> > That doesn't really work.  I've been talking to the ACPI people about
>> > this, and the problem is "don't otherwise have a driver for" is an
>> > impossible thing to prove, as you never know when a driver is going to
>> > be loaded from userspace.
>> >
>> > You can easily bind drivers to devices today from userspace, why not
>> > just use the built-in functionality you have today if you "know" that
>> > there is no driver for this hardware.
>>
>> What we're really after here is that we want to have an spidev
>> instance when we don't even have a device.
>
> That's crazy, just create a device, things do not work without one.

One simple way that works today is to create a device tree overlay with a
device that's compatible with "spidev". That does violate DT policy, as it
doesn't describe the hardware. But it works. This can be done from userspace,
so we (the naggers about incorrect description of the hardware) will never
see the abusive dtso.

Alternatively, you can write some kernel code that does more or less the same
thing, i.e. create a device for "spidev", but without any DT involved.
If you add remove support, and make this controllable through sysfs, I think
this could be a viable solution. It would be similar to gpio export.
A DT overlay with the real device can still take over, if you instruct the
removal of the "spidev" device through sysfs first.

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
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
Maxime Ripard May 13, 2015, 7:26 p.m. UTC | #23
On Wed, May 13, 2015 at 11:17:36AM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 13, 2015 at 07:50:34PM +0200, Maxime Ripard wrote:
> > Hi Greg,
> > 
> > On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:
> > > On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
> > > > On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
> > > > 
> > > > > While this is nicer than the DT solution because of its accurate hardware
> > > > > representation, it's still not perfect because you might not have access to the
> > > > > DT, or you might be driving a completely generic device (such as a
> > > > > microcontroller) that might be used for something else in a different
> > > > > context/board.
> > > > 
> > > > Greg, you're copied on this because this seems to be a generic problem
> > > > that should perhaps be solved at a driver model level - having a way to
> > > > bind userspace access to devices that we don't otherwise have a driver
> > > > for.  The subsystem could specify the UIO driver to use when no other
> > > > driver is available.
> > > 
> > > That doesn't really work.  I've been talking to the ACPI people about
> > > this, and the problem is "don't otherwise have a driver for" is an
> > > impossible thing to prove, as you never know when a driver is going to
> > > be loaded from userspace.
> > > 
> > > You can easily bind drivers to devices today from userspace, why not
> > > just use the built-in functionality you have today if you "know" that
> > > there is no driver for this hardware.
> > 
> > What we're really after here is that we want to have an spidev
> > instance when we don't even have a device.
> 
> That's crazy, just create a device, things do not work without one.

Our use case is this one: we want to export spidev files so that "dev
boards" with a header that allows to plug virtually anything on it
(Raspberry Pi, Cubieboards, Xplained, and all the likes) without
having to change the kernel and / or device tree.

That would mean that if we plug something to that port, no device will
be created because the DT itself won't have that device declared in
the first place.

This patch is actually doing this: creating a new device for all the
chipselects that are not in use that will be bound to the spidev
driver.

Maxime
Maxime Ripard May 13, 2015, 7:41 p.m. UTC | #24
On Wed, May 13, 2015 at 09:10:48PM +0200, Geert Uytterhoeven wrote:
> On Wed, May 13, 2015 at 2:51 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> >> > This also adds an i2cdev-like feeling, where you get all the
> >> > spidev devices all the time, without any modification.
> >>
> >> I2C is a bit safer here since it's a shared bus so you can't do
> >> anything to devices not connected to the bus by mistake.
> >
> > I'm not sure to understand what you mean here. How is SPI different
> > from that aspect?
> 
> If you talk to a nonexistent i2c device, nothing happens, as it just sends
> a message with a nonexistent address on the shared bus.
> 
> If you talk to a nonexistent spi device, hell may break loose if e.g. some
> "smart" hardware engineer used the "unused" CS as a pull-up for the
> _RESET line on an external device... It's a bit like banging random
> "unused" GPIOs.

Ah, right. I'm always surprised by how creative the hardware engineers
actually are :)

Maxime
Greg Kroah-Hartman May 13, 2015, 10:33 p.m. UTC | #25
On Wed, May 13, 2015 at 09:26:40PM +0200, Maxime Ripard wrote:
> On Wed, May 13, 2015 at 11:17:36AM -0700, Greg Kroah-Hartman wrote:
> > On Wed, May 13, 2015 at 07:50:34PM +0200, Maxime Ripard wrote:
> > > Hi Greg,
> > > 
> > > On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:
> > > > On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
> > > > > On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
> > > > > 
> > > > > > While this is nicer than the DT solution because of its accurate hardware
> > > > > > representation, it's still not perfect because you might not have access to the
> > > > > > DT, or you might be driving a completely generic device (such as a
> > > > > > microcontroller) that might be used for something else in a different
> > > > > > context/board.
> > > > > 
> > > > > Greg, you're copied on this because this seems to be a generic problem
> > > > > that should perhaps be solved at a driver model level - having a way to
> > > > > bind userspace access to devices that we don't otherwise have a driver
> > > > > for.  The subsystem could specify the UIO driver to use when no other
> > > > > driver is available.
> > > > 
> > > > That doesn't really work.  I've been talking to the ACPI people about
> > > > this, and the problem is "don't otherwise have a driver for" is an
> > > > impossible thing to prove, as you never know when a driver is going to
> > > > be loaded from userspace.
> > > > 
> > > > You can easily bind drivers to devices today from userspace, why not
> > > > just use the built-in functionality you have today if you "know" that
> > > > there is no driver for this hardware.
> > > 
> > > What we're really after here is that we want to have an spidev
> > > instance when we don't even have a device.
> > 
> > That's crazy, just create a device, things do not work without one.
> 
> Our use case is this one: we want to export spidev files so that "dev
> boards" with a header that allows to plug virtually anything on it
> (Raspberry Pi, Cubieboards, Xplained, and all the likes) without
> having to change the kernel and / or device tree.

You want to do that on a bus that is not self-describing or dynamic?
I too want a pony.  Please go kick the hardware engineer who designed
such a mess, we solved this problem 20+ years ago with "real" busses.

> That would mean that if we plug something to that port, no device will
> be created because the DT itself won't have that device declared in
> the first place.

Because you can't dynamically determine that something was plugged in,
of course.

> This patch is actually doing this: creating a new device for all the
> chipselects that are not in use that will be bound to the spidev
> driver.

I have yet to see a patch...
--
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 May 14, 2015, 2:34 p.m. UTC | #26
On Wed, May 13, 2015 at 03:33:31PM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 13, 2015 at 09:26:40PM +0200, Maxime Ripard wrote:

> > Our use case is this one: we want to export spidev files so that "dev
> > boards" with a header that allows to plug virtually anything on it
> > (Raspberry Pi, Cubieboards, Xplained, and all the likes) without
> > having to change the kernel and / or device tree.

> You want to do that on a bus that is not self-describing or dynamic?
> I too want a pony.  Please go kick the hardware engineer who designed
> such a mess, we solved this problem 20+ years ago with "real" busses.

The hardware engineers for some of these things did build enumeration in
but none of them with any sort of community have been able to make it
stick - the community just ends up ignoring the ID allocation.

> > This patch is actually doing this: creating a new device for all the
> > chipselects that are not in use that will be bound to the spidev
> > driver.

> I have yet to see a patch...

That was the first message in the thread.
Maxime Ripard May 15, 2015, 8:09 a.m. UTC | #27
On Wed, May 13, 2015 at 03:33:31PM -0700, Greg Kroah-Hartman wrote:
> On Wed, May 13, 2015 at 09:26:40PM +0200, Maxime Ripard wrote:
> > On Wed, May 13, 2015 at 11:17:36AM -0700, Greg Kroah-Hartman wrote:
> > > On Wed, May 13, 2015 at 07:50:34PM +0200, Maxime Ripard wrote:
> > > > Hi Greg,
> > > > 
> > > > On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:
> > > > > On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
> > > > > > On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
> > > > > > 
> > > > > > > While this is nicer than the DT solution because of its accurate hardware
> > > > > > > representation, it's still not perfect because you might not have access to the
> > > > > > > DT, or you might be driving a completely generic device (such as a
> > > > > > > microcontroller) that might be used for something else in a different
> > > > > > > context/board.
> > > > > > 
> > > > > > Greg, you're copied on this because this seems to be a generic problem
> > > > > > that should perhaps be solved at a driver model level - having a way to
> > > > > > bind userspace access to devices that we don't otherwise have a driver
> > > > > > for.  The subsystem could specify the UIO driver to use when no other
> > > > > > driver is available.
> > > > > 
> > > > > That doesn't really work.  I've been talking to the ACPI people about
> > > > > this, and the problem is "don't otherwise have a driver for" is an
> > > > > impossible thing to prove, as you never know when a driver is going to
> > > > > be loaded from userspace.
> > > > > 
> > > > > You can easily bind drivers to devices today from userspace, why not
> > > > > just use the built-in functionality you have today if you "know" that
> > > > > there is no driver for this hardware.
> > > > 
> > > > What we're really after here is that we want to have an spidev
> > > > instance when we don't even have a device.
> > > 
> > > That's crazy, just create a device, things do not work without one.
> > 
> > Our use case is this one: we want to export spidev files so that "dev
> > boards" with a header that allows to plug virtually anything on it
> > (Raspberry Pi, Cubieboards, Xplained, and all the likes) without
> > having to change the kernel and / or device tree.
> 
> You want to do that on a bus that is not self-describing or dynamic?
> I too want a pony.  Please go kick the hardware engineer who designed
> such a mess, we solved this problem 20+ years ago with "real" busses.

Well, we do have such ponies on some bus that don't have any kind of
enumeration. i2cdev allows to do just that already. That would seem
logical to have a similar behaviour for SPI.

> > That would mean that if we plug something to that port, no device will
> > be created because the DT itself won't have that device declared in
> > the first place.
> 
> Because you can't dynamically determine that something was plugged in,
> of course.

Well.. Yeah.

> 
> > This patch is actually doing this: creating a new device for all the
> > chipselects that are not in use that will be bound to the spidev
> > driver.
> 
> I have yet to see a patch...

You were in Cc of that patch, which is the first message in this thread.

Maxime
Lucas De Marchi July 15, 2015, 6:27 a.m. UTC | #28
On Wed, May 13, 2015 at 7:33 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, May 13, 2015 at 09:26:40PM +0200, Maxime Ripard wrote:
>> On Wed, May 13, 2015 at 11:17:36AM -0700, Greg Kroah-Hartman wrote:
>> > On Wed, May 13, 2015 at 07:50:34PM +0200, Maxime Ripard wrote:
>> > > Hi Greg,
>> > >
>> > > On Wed, May 13, 2015 at 08:37:40AM -0700, Greg Kroah-Hartman wrote:
>> > > > On Wed, May 13, 2015 at 12:26:04PM +0100, Mark Brown wrote:
>> > > > > On Tue, May 12, 2015 at 10:33:24PM +0200, Maxime Ripard wrote:
>> > > > >
>> > > > > > While this is nicer than the DT solution because of its accurate hardware
>> > > > > > representation, it's still not perfect because you might not have access to the
>> > > > > > DT, or you might be driving a completely generic device (such as a
>> > > > > > microcontroller) that might be used for something else in a different
>> > > > > > context/board.
>> > > > >
>> > > > > Greg, you're copied on this because this seems to be a generic problem
>> > > > > that should perhaps be solved at a driver model level - having a way to
>> > > > > bind userspace access to devices that we don't otherwise have a driver
>> > > > > for.  The subsystem could specify the UIO driver to use when no other
>> > > > > driver is available.
>> > > >
>> > > > That doesn't really work.  I've been talking to the ACPI people about
>> > > > this, and the problem is "don't otherwise have a driver for" is an
>> > > > impossible thing to prove, as you never know when a driver is going to
>> > > > be loaded from userspace.
>> > > >
>> > > > You can easily bind drivers to devices today from userspace, why not
>> > > > just use the built-in functionality you have today if you "know" that
>> > > > there is no driver for this hardware.
>> > >
>> > > What we're really after here is that we want to have an spidev
>> > > instance when we don't even have a device.
>> >
>> > That's crazy, just create a device, things do not work without one.
>>
>> Our use case is this one: we want to export spidev files so that "dev
>> boards" with a header that allows to plug virtually anything on it
>> (Raspberry Pi, Cubieboards, Xplained, and all the likes) without
>> having to change the kernel and / or device tree.
>
> You want to do that on a bus that is not self-describing or dynamic?
> I too want a pony.  Please go kick the hardware engineer who designed
> such a mess, we solved this problem 20+ years ago with "real" busses.

Mind you we are talking about buses created more than 20+ years ago.
(Unfortunately) They are still used today for all kind of sensors.
Boards like RPi, beaglebone, minnowboard expose the pins so we can
actually talk to those sensors, plugging in anyone we'd like to.  For
some of them for example there are IIO drivers that we could use the
driver model to allow binding them. But spidev/i2c-dev allow userspace
to talk directly to them. And you don't know what *others* will plug
into that bus... might even be their own microntrollers with no
identification at all.

Without something like the patch in the first message, people need to
create DT overlays for platforms that support that. ACPI doesn't
support overlays (yet) so we need to keep awful external platform
drivers[1] just to make spidev to work.
diff mbox

Patch

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index d5d7d2235163..e6ca46e1e0fc 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1384,6 +1384,52 @@  static void acpi_register_spi_devices(struct spi_master *master)
 static inline void acpi_register_spi_devices(struct spi_master *master) {}
 #endif /* CONFIG_ACPI */
 
+#ifdef CONFIG_SPI_SPIDEV
+static void spidev_register_devices(struct spi_master *master)
+{
+	struct spi_device *spi;
+	int i, status;
+
+	for (i = 0; i < master->num_chipselect; i++) {
+		spi = spi_alloc_device(master);
+		if (!spi) {
+			dev_err(&master->dev, "Couldn't allocate spidev device\n");
+			continue;
+		}
+
+		spi->chip_select = i;
+		strlcpy(spi->modalias, "spidev", sizeof(spi->modalias));
+
+		/*
+		 * This is far from perfect since an addition might be
+		 * done between here and the call to spi_add_device,
+		 * but we can't hold the lock and call spi_add_device
+		 * either, as it would trigger a deadlock.
+		 *
+		 * If such a race occurs, spi_add_device will still
+		 * catch it though, as it also checks for devices
+		 * being registered several times on the same chip
+		 * select.
+		*/
+		status = bus_for_each_dev(&spi_bus_type, NULL, spi,
+					  spi_dev_check);
+		if (status) {
+			dev_dbg(&master->dev, "Chipselect already in use.. Skipping.");
+			spi_dev_put(spi);
+			continue;
+		}
+
+		if (spi_add_device(spi)) {
+			dev_err(&master->dev, "Couldn't add spidev device\n");
+			spi_dev_put(spi);
+		}
+	}
+
+}
+#else
+static inline void spidev_register_devices(struct spi_master *master) {}
+#endif /* CONFIG_SPI_SPIDEV */
+
 static void spi_master_release(struct device *dev)
 {
 	struct spi_master *master;
@@ -1575,6 +1621,7 @@  int spi_register_master(struct spi_master *master)
 	/* Register devices from the device tree and ACPI */
 	of_register_spi_devices(master);
 	acpi_register_spi_devices(master);
+	spidev_register_devices(master);
 done:
 	return status;
 }