2015-04-28 12:53:40

by Michal Suchanek

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On 28 April 2015 at 14:15, Eric D. <[email protected]> wrote:
> Hi,
>
> I'am a mainline linux user of A20 (bananapi). I'am currently running a
> debian jessie with latest mainline kernel (4.0.0+).
> I have a project of home automation, based on nrfl04+ spi driven wireless
> chip.
> I was just seeking a way to make spidev device appear under mainline kernel
> and found this thread.
> Could someone explain the right way to do this ?

There is no approved RightWay(tm).

However, you can use this patch:

https://lkml.org/lkml/2014/4/28/612

which is probably the least controversial way for now.

The example at the start of the thread also works but is
CertainlyNotApprovedWay(tm).

HTH

Michal


2015-04-28 14:16:50

by Mark Brown

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Tue, Apr 28, 2015 at 02:52:54PM +0200, Michal Suchanek wrote:
> On 28 April 2015 at 14:15, Eric D. <[email protected]> wrote:

> > I was just seeking a way to make spidev device appear under mainline kernel
> > and found this thread.
> > Could someone explain the right way to do this ?

> There is no approved RightWay(tm).

That is not the case as you well know. As has been said several times
the compatible for the device should be added to the match table in
spidev.c.


Attachments:
(No filename) (484.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-28 14:23:09

by Michal Suchanek

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On 28 April 2015 at 16:16, Mark Brown <[email protected]> wrote:
> On Tue, Apr 28, 2015 at 02:52:54PM +0200, Michal Suchanek wrote:
>> On 28 April 2015 at 14:15, Eric D. <[email protected]> wrote:
>
>> > I was just seeking a way to make spidev device appear under mainline kernel
>> > and found this thread.
>> > Could someone explain the right way to do this ?
>
>> There is no approved RightWay(tm).
>
> That is not the case as you well know. As has been said several times
> the compatible for the device should be added to the match table in
> spidev.c.

That's a way unusable for people who actually want to use spidev so it
might be TheApprovedWay(tm) but not RightWay(tm).

Thanks

Michal

2015-04-28 17:17:59

by Mark Brown

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Tue, Apr 28, 2015 at 04:22:24PM +0200, Michal Suchanek wrote:
> On 28 April 2015 at 16:16, Mark Brown <[email protected]> wrote:

> > That is not the case as you well know. As has been said several times
> > the compatible for the device should be added to the match table in
> > spidev.c.

> That's a way unusable for people who actually want to use spidev so it
> might be TheApprovedWay(tm) but not RightWay(tm).

I'm pretty sure that someone who's able to edit one file can edit two.
I know you have a viewpoint on this but engaging in this way is not
helping anyone.


Attachments:
(No filename) (577.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-28 20:44:21

by Michal Suchanek

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On 28 April 2015 at 19:17, Mark Brown <[email protected]> wrote:
> On Tue, Apr 28, 2015 at 04:22:24PM +0200, Michal Suchanek wrote:
>> On 28 April 2015 at 16:16, Mark Brown <[email protected]> wrote:
>
>> > That is not the case as you well know. As has been said several times
>> > the compatible for the device should be added to the match table in
>> > spidev.c.
>
>> That's a way unusable for people who actually want to use spidev so it
>> might be TheApprovedWay(tm) but not RightWay(tm).
>
> I'm pretty sure that someone who's able to edit one file can edit two.
> I know you have a viewpoint on this but engaging in this way is not
> helping anyone.

The point is that patching the kernel to use spidev is totally useless
complication which is brought on spidev users by obtuse kernel
maintainers.

This patch applied to a kernel which is distributed with the board
solves the problem for all spidev users of the board which
TheApprovedWay(tm) does not.

Thanks

Michal

2015-04-29 17:41:18

by Mark Brown

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Tue, Apr 28, 2015 at 10:43:37PM +0200, Michal Suchanek wrote:

> > I know you have a viewpoint on this but engaging in this way is not
> > helping anyone.

> The point is that patching the kernel to use spidev is totally useless
> complication which is brought on spidev users by obtuse kernel
> maintainers.

> This patch applied to a kernel which is distributed with the board
> solves the problem for all spidev users of the board which
> TheApprovedWay(tm) does not.

Please stop this, it is not helpful.


Attachments:
(No filename) (512.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-29 17:45:44

by Michal Suchanek

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On 29 April 2015 at 19:40, Mark Brown <[email protected]> wrote:
> On Tue, Apr 28, 2015 at 10:43:37PM +0200, Michal Suchanek wrote:
>
>> > I know you have a viewpoint on this but engaging in this way is not
>> > helping anyone.
>
>> The point is that patching the kernel to use spidev is totally useless
>> complication which is brought on spidev users by obtuse kernel
>> maintainers.
>
>> This patch applied to a kernel which is distributed with the board
>> solves the problem for all spidev users of the board which
>> TheApprovedWay(tm) does not.
>
> Please stop this, it is not helpful.

Then please make one of the useful ways of instantiating spidev nodes
approved or suggest another that when implemented can be mainlined.

Thanks

Michal

2015-04-29 18:07:17

by Mark Brown

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Wed, Apr 29, 2015 at 07:44:59PM +0200, Michal Suchanek wrote:
> On 29 April 2015 at 19:40, Mark Brown <[email protected]> wrote:

> > Please stop this, it is not helpful.

> Then please make one of the useful ways of instantiating spidev nodes
> approved or suggest another that when implemented can be mainlined.

I think the rest of the thread had that covered - there's both adding
the device IDs and Maxime's patch.


Attachments:
(No filename) (424.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-04-29 18:38:09

by Michal Suchanek

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On 29 April 2015 at 20:06, Mark Brown <[email protected]> wrote:
> On Wed, Apr 29, 2015 at 07:44:59PM +0200, Michal Suchanek wrote:
>> On 29 April 2015 at 19:40, Mark Brown <[email protected]> wrote:
>
>> > Please stop this, it is not helpful.
>
>> Then please make one of the useful ways of instantiating spidev nodes
>> approved or suggest another that when implemented can be mainlined.
>
> I think the rest of the thread had that covered - there's both adding
> the device IDs and Maxime's patch.

And adding device IDs is unacceptable for users of devboards while
Maxime's patch is not accepted into the kernel.

I am using a version of Maxime's patch myself right now. It does not
seem it's going to be include in the kernel any time soon, however.

FWIW I added the ability to open any CS, even those claimed by kernel
drivers. This addresses any potential race of spidev binding before
the actual driver but has the potential to introduce some subtle bugs
when you open and reconfigure a CS used by a kernel driver or send
some commands that upset the device.

Thanks

Michal

2015-04-29 18:56:30

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Wed, Apr 29, 2015 at 8:37 PM, Michal Suchanek <[email protected]> wrote:
> I am using a version of Maxime's patch myself right now. It does not
> seem it's going to be include in the kernel any time soon, however.
>
> FWIW I added the ability to open any CS, even those claimed by kernel
> drivers. This addresses any potential race of spidev binding before
> the actual driver but has the potential to introduce some subtle bugs
> when you open and reconfigure a CS used by a kernel driver or send
> some commands that upset the device.

Uh, that sounds dangerous.

Perhaps you can add some safety net, before user space can access
them, cfr. /sys/class/gpio/export?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2015-04-29 19:25:42

by Michal Suchanek

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On 29 April 2015 at 20:56, Geert Uytterhoeven <[email protected]> wrote:
> On Wed, Apr 29, 2015 at 8:37 PM, Michal Suchanek <[email protected]> wrote:
>> I am using a version of Maxime's patch myself right now. It does not
>> seem it's going to be include in the kernel any time soon, however.
>>
>> FWIW I added the ability to open any CS, even those claimed by kernel
>> drivers. This addresses any potential race of spidev binding before
>> the actual driver but has the potential to introduce some subtle bugs
>> when you open and reconfigure a CS used by a kernel driver or send
>> some commands that upset the device.
>
> Uh, that sounds dangerous.
>
> Perhaps you can add some safety net, before user space can access
> them, cfr. /sys/class/gpio/export?
>

That's what accessing random devices from userspace is. I can issue
the identify commead to my SPI flash allright since it is idle.

I am not sure of its protocol details but I am quite sure that some
displays have data transfers that allow pauses so if I sent a command
during a screen update it would likely get inserted into the
framebuffer bitstream. And changing CS polarity or something like that
will certainly have interesting results.

Still not binding spidev to busy CS is just one test that can be
compiled in as an option. If things stay this simple I don't see much
problem with that.

Thanks

Michal

2015-04-30 19:58:49

by Mark Brown

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Wed, Apr 29, 2015 at 08:37:24PM +0200, Michal Suchanek wrote:
> On 29 April 2015 at 20:06, Mark Brown <[email protected]> wrote:

> > I think the rest of the thread had that covered - there's both adding
> > the device IDs and Maxime's patch.

> And adding device IDs is unacceptable for users of devboards while
> Maxime's patch is not accepted into the kernel.

> I am using a version of Maxime's patch myself right now. It does not
> seem it's going to be include in the kernel any time soon, however.

A big reason for that is that it's not in my inbox for me to review,
these messages I flagged as unhelpful aren't going to help with that if
only because I don't want to create the impression that such behaviour
achieves results.

> FWIW I added the ability to open any CS, even those claimed by kernel
> drivers. This addresses any potential race of spidev binding before
> the actual driver but has the potential to introduce some subtle bugs
> when you open and reconfigure a CS used by a kernel driver or send
> some commands that upset the device.

This doesn't seem like an obviously good idea - having userspace be able
to interact with a device without a running kernel driver knowing about
it doesn't seem like something that will end well.


Attachments:
(No filename) (1.23 kB)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-05-03 09:01:20

by Martin Sperl

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.


> On 30.04.2015, at 21:58, Mark Brown <[email protected]> wrote:
>
> A big reason for that is that it's not in my inbox for me to review,
> these messages I flagged as unhelpful aren't going to help with that if
> only because I don't want to create the impression that such behaviour
> achieves results.
>

What about implementing it like this:
echo -n “spi32761.4” > /sys/bus/spi/drivers/spidev/bind

Would this be an acceptable solution?

This is actually mentioned in Documentation/spi/spidev as a
possible option for the future - quote:

> (Sysfs also supports userspace driven binding/unbinding of drivers to
> devices. That mechanism might be supported here in the future.)

Not sure why it does not work right now (but it works for “real”
device-drivers), but I guess it has to do with compatibility checks.

Martin

2015-05-03 09:59:51

by Mark Brown

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Sun, May 03, 2015 at 11:01:05AM +0200, Martin Sperl wrote:

> What about implementing it like this:
> echo -n “spi32761.4” > /sys/bus/spi/drivers/spidev/bind

> Would this be an acceptable solution?

> This is actually mentioned in Documentation/spi/spidev as a
> possible option for the future - quote:

> > (Sysfs also supports userspace driven binding/unbinding of drivers to
> > devices. That mechanism might be supported here in the future.)

> Not sure why it does not work right now (but it works for “real”
> device-drivers), but I guess it has to do with compatibility checks.

Hrm, yes - that should work. I'd ask Greg, that's not something the bus
implements.


Attachments:
(No filename) (684.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-05-03 10:02:33

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Sun, May 3, 2015 at 11:01 AM, Martin Sperl <[email protected]> wrote:
>> On 30.04.2015, at 21:58, Mark Brown <[email protected]> wrote:
>> A big reason for that is that it's not in my inbox for me to review,
>> these messages I flagged as unhelpful aren't going to help with that if
>> only because I don't want to create the impression that such behaviour
>> achieves results.
>
> What about implementing it like this:
> echo -n “spi32761.4” > /sys/bus/spi/drivers/spidev/bind
>
> Would this be an acceptable solution?
>
> This is actually mentioned in Documentation/spi/spidev as a
> possible option for the future - quote:
>
>> (Sysfs also supports userspace driven binding/unbinding of drivers to
>> devices. That mechanism might be supported here in the future.)
>
> Not sure why it does not work right now (but it works for “real”
> device-drivers), but I guess it has to do with compatibility checks.

IIRC, PCI drivers support adding more compatible entries through /sysfs.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2015-05-03 21:00:51

by Martin Sperl

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.


> On 03.05.2015, at 11:59, Mark Brown <[email protected]> wrote:
> Hrm, yes - that should work. I'd ask Greg, that's not something the bus
> implements.

It is still slightly more “complicated” from a distribution perspective,
but if that is what makes it a “clean” solution, then that is the way to
go forward.

I will investigate the fine details, but I fear we may need some
“compatibility” magic similar to “new_id” in USB to make it work,
because it seems as if you can ONLY force a driver to bind if it
_is_ compatible...

See also here: https://lwn.net/Articles/143397/

But from what I can tell this functionality (mentioned in this article
by Greg) has not been moved into driver-core and bus, so we would need
to run our own version of it.-

2015-05-04 08:37:22

by Michal Suchanek

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

Hello,

On 3 May 2015 at 23:00, Martin Sperl <[email protected]> wrote:
>
>> On 03.05.2015, at 11:59, Mark Brown <[email protected]> wrote:
>> Hrm, yes - that should work. I'd ask Greg, that's not something the bus
>> implements.
>
> It is still slightly more “complicated” from a distribution perspective,
> but if that is what makes it a “clean” solution, then that is the way to
> go forward.
>
> I will investigate the fine details, but I fear we may need some
> “compatibility” magic similar to “new_id” in USB to make it work,
> because it seems as if you can ONLY force a driver to bind if it
> _is_ compatible...
>
> See also here: https://lwn.net/Articles/143397/
>
> But from what I can tell this functionality (mentioned in this article
> by Greg) has not been moved into driver-core and bus, so we would need
> to run our own version of it.

Maybe you could make the spidev driver magically bind to any CS to
which no other driver is bound? This would also probably solve the
problem with the device going away when the driver is unbound if that
still happens.

Thanks

Michal

2015-05-04 10:12:39

by Mark Brown

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Sun, May 03, 2015 at 11:00:40PM +0200, Martin Sperl wrote:

> I will investigate the fine details, but I fear we may need some
> “compatibility” magic similar to “new_id” in USB to make it work,
> because it seems as if you can ONLY force a driver to bind if it
> _is_ compatible...

I'm confused. What would the point of the functionality be if not to
override the existing data, otherwise we'd already have bound the
driver?

> But from what I can tell this functionality (mentioned in this article
> by Greg) has not been moved into driver-core and bus, so we would need
> to run our own version of it.

That seems like things are being done at the wrong level.


Attachments:
(No filename) (677.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments

2015-05-04 10:49:54

by Michal Suchanek

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On 4 May 2015 at 12:12, Mark Brown <[email protected]> wrote:
> On Sun, May 03, 2015 at 11:00:40PM +0200, Martin Sperl wrote:
>
>> I will investigate the fine details, but I fear we may need some
>> “compatibility” magic similar to “new_id” in USB to make it work,
>> because it seems as if you can ONLY force a driver to bind if it
>> _is_ compatible...
>
> I'm confused. What would the point of the functionality be if not to
> override the existing data, otherwise we'd already have bound the
> driver?

Presumably you can swap different versions of a driver this way.

Many devices have two drivers in Linux (old and new) which are
obviously both compatible.

Loading driver which is not compatible is something which you probably
do not want to be done easily as much as sending random junk to SPI
devices controlled by a kernel driver.

I am, of course, enjoying the ability to send some ID command to a
flash memory which is technically controlled by a kernel driver when I
physically replace the chip in the socket or the chip was not seated
well to start with and I want to check that it's working without
rebooting the board.

Thanks

Michal

2015-05-04 13:18:12

by Mark Brown

[permalink] [raw]
Subject: Re: [linux-sunxi] [PATCH 2/3] spidev: Add DT binding example.

On Mon, May 04, 2015 at 12:42:03PM +0200, Michal Suchanek wrote:
> On 4 May 2015 at 12:12, Mark Brown <[email protected]> wrote:

> > I'm confused. What would the point of the functionality be if not to
> > override the existing data, otherwise we'd already have bound the
> > driver?

> Presumably you can swap different versions of a driver this way.

> Many devices have two drivers in Linux (old and new) which are
> obviously both compatible.

Many? We have a couple but if we ever have many then that would be a
bit of a disaster.


Attachments:
(No filename) (540.00 B)
signature.asc (473.00 B)
Digital signature
Download all attachments