2015-04-26 10:33:12

by Mark Brown

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

On Tue, Mar 24, 2015 at 11:50:53AM +0100, Michal Suchanek wrote:

> +A spidev example for devicetree binding in a board dts file
> +&spi2 {

No, this is broken - nothing should ever bind to spidev as spidev. The
fact that we have a binding document at all is a bug.


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

2015-04-26 10:55:21

by Michal Suchanek

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

On 26 April 2015 at 12:32, Mark Brown <[email protected]> wrote:
> On Tue, Mar 24, 2015 at 11:50:53AM +0100, Michal Suchanek wrote:
>
>> +A spidev example for devicetree binding in a board dts file
>> +&spi2 {
>
> No, this is broken - nothing should ever bind to spidev as spidev. The
> fact that we have a binding document at all is a bug.

And how do you get spidev if nothing binds to it?

When you boot without the binding no device is created.

Thanks

Michal

2015-04-26 11:01:55

by Mark Brown

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

On Sun, Apr 26, 2015 at 12:54:36PM +0200, Michal Suchanek wrote:
> On 26 April 2015 at 12:32, Mark Brown <[email protected]> wrote:

> > No, this is broken - nothing should ever bind to spidev as spidev. The
> > fact that we have a binding document at all is a bug.

> And how do you get spidev if nothing binds to it?

> When you boot without the binding no device is created.

Describe your actual hardware in the DT then add the relevant device ID
to spidev.


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

2015-04-26 11:23:44

by Hans de Goede

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

Hi,

On 26-04-15 13:01, Mark Brown wrote:
> On Sun, Apr 26, 2015 at 12:54:36PM +0200, Michal Suchanek wrote:
>> On 26 April 2015 at 12:32, Mark Brown <[email protected]> wrote:
>
>>> No, this is broken - nothing should ever bind to spidev as spidev. The
>>> fact that we have a binding document at all is a bug.
>
>> And how do you get spidev if nothing binds to it?
>
>> When you boot without the binding no device is created.
>
> Describe your actual hardware in the DT then add the relevant device ID
> to spidev.

I think there is actual a use for just binding spidev as spidev,
think e.g. the spi pins on the raspberry pi.

How do you deal we suggest with such a situation ?

Regards,

Hans

2015-04-26 11:26:53

by Michal Suchanek

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

On 26 April 2015 at 13:01, Mark Brown <[email protected]> wrote:
> On Sun, Apr 26, 2015 at 12:54:36PM +0200, Michal Suchanek wrote:
>> On 26 April 2015 at 12:32, Mark Brown <[email protected]> wrote:
>
>> > No, this is broken - nothing should ever bind to spidev as spidev. The
>> > fact that we have a binding document at all is a bug.
>
>> And how do you get spidev if nothing binds to it?
>
>> When you boot without the binding no device is created.
>
> Describe your actual hardware in the DT then add the relevant device ID
> to spidev.

My hardware is a sunxi SPI controller and it is described in the DT. I
have pins on the board which I want to use for connecting arbitrary
devices. How do I add those to spidev other than creating a subnode of
the spi controller and binding that to spidev?

Thanks

Michal

2015-04-26 11:56:19

by Martin Sperl

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


> On 26.04.2015, at 13:23, Hans de Goede <[email protected]> wrote:
> I think there is actual a use for just binding spidev as spidev,
> think e.g. the spi pins on the raspberry pi.
>
> How do you deal we suggest with such a situation ?

I actually asked the same question a few days ago on the spi list
(in thread: "spi: spidev: Warn loudly if instantiated from DT as “spidev”)
and the summary was:

You can still do as before, but you have to accept that long
irritating warning.

Or you patch spidev.c to include your pattern of choice for compatiblity

Or you implement the following proposal (which needs a volunteer):
> On 23.04.2015, at 09:42, Geert Uytterhoeven <[email protected]> wrote:
>
> So what you need is a way to handover from generic spidev to a device-specific
> driver, cfr. what graphics drivers do when the device has been bound to by
> vesafb or simplefb.
>
> Could this be implemented in a generic way in the spi or DT code?

...
> On 23.04.2015, at 12:36, Mark Brown <[email protected]> wrote:
> On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote:
>
>> I guess this has been suggested before: the spi core could provide spidev
>> access to all spi client devices which are not bound by a driver?
>
> I don't know if it's been suggested before, certainly nobody did the
> work to make it happen. I don't think I have a massive objection in
> principal.


Martin


2015-04-26 12:39:04

by Michal Suchanek

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

On 26 April 2015 at 13:56, Martin Sperl <[email protected]> wrote:
>
>> On 26.04.2015, at 13:23, Hans de Goede <[email protected]> wrote:
>> I think there is actual a use for just binding spidev as spidev,
>> think e.g. the spi pins on the raspberry pi.
>>
>> How do you deal we suggest with such a situation ?
>
> I actually asked the same question a few days ago on the spi list
> (in thread: "spi: spidev: Warn loudly if instantiated from DT as “spidev”)
> and the summary was:
>
> You can still do as before, but you have to accept that long
> irritating warning.
>
> Or you patch spidev.c to include your pattern of choice for compatiblity

So the suggestion is to add a compatible string like olimex,uext-slot
to spidev and use that compatible in the DT?

That can certainly be done but adding a new compatible for every board
that has some random pins looks like a needless nuisance to me.
Especially compared to i2c where you can just open the bus so long as
ti is enabled.

>
> Or you implement the following proposal (which needs a volunteer):
>> On 23.04.2015, at 09:42, Geert Uytterhoeven <[email protected]> wrote:
>>
>> So what you need is a way to handover from generic spidev to a device-specific
>> driver, cfr. what graphics drivers do when the device has been bound to by
>> vesafb or simplefb.
>>
>> Could this be implemented in a generic way in the spi or DT code?
>
> ...
>> On 23.04.2015, at 12:36, Mark Brown <[email protected]> wrote:
>> On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote:
>>
>>> I guess this has been suggested before: the spi core could provide spidev
>>> access to all spi client devices which are not bound by a driver?
>>
>> I don't know if it's been suggested before, certainly nobody did the
>> work to make it happen. I don't think I have a massive objection in
>> principal.

But how do you know there is a device?

Devices on i2c can be probed. On spi you just transfer random data and
hope it does something useful. Some devices have readable registers
and can be probed in a device-specific way but others are write-only.

So binding spidev is in my view just saying that you are going to
transfer random data from userspace on this bus.

Thanks

Michal

2015-04-26 12:55:07

by Maxime Ripard

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

On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote:
> On 26 April 2015 at 13:56, Martin Sperl <[email protected]> wrote:
> >
> >> On 26.04.2015, at 13:23, Hans de Goede <[email protected]> wrote:
> >> I think there is actual a use for just binding spidev as spidev,
> >> think e.g. the spi pins on the raspberry pi.
> >>
> >> How do you deal we suggest with such a situation ?
> >
> > I actually asked the same question a few days ago on the spi list
> > (in thread: "spi: spidev: Warn loudly if instantiated from DT as “spidev”)
> > and the summary was:
> >
> > You can still do as before, but you have to accept that long
> > irritating warning.
> >
> > Or you patch spidev.c to include your pattern of choice for compatiblity
>
> So the suggestion is to add a compatible string like olimex,uext-slot
> to spidev and use that compatible in the DT?

No, you add a compatible for the device that is connected to the bus
through that slot.

> That can certainly be done but adding a new compatible for every board
> that has some random pins looks like a needless nuisance to me.
> Especially compared to i2c where you can just open the bus so long as
> ti is enabled.
>
> >
> > Or you implement the following proposal (which needs a volunteer):
> >> On 23.04.2015, at 09:42, Geert Uytterhoeven <[email protected]> wrote:
> >>
> >> So what you need is a way to handover from generic spidev to a device-specific
> >> driver, cfr. what graphics drivers do when the device has been bound to by
> >> vesafb or simplefb.
> >>
> >> Could this be implemented in a generic way in the spi or DT code?
> >
> > ...
> >> On 23.04.2015, at 12:36, Mark Brown <[email protected]> wrote:
> >> On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote:
> >>
> >>> I guess this has been suggested before: the spi core could provide spidev
> >>> access to all spi client devices which are not bound by a driver?
> >>
> >> I don't know if it's been suggested before, certainly nobody did the
> >> work to make it happen. I don't think I have a massive objection in
> >> principal.

Actually, I did it a year ago, and it looked at the time that it
wasn't what should be done either.

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

> But how do you know there is a device?
>
> Devices on i2c can be probed. On spi you just transfer random data and
> hope it does something useful. Some devices have readable registers
> and can be probed in a device-specific way but others are write-only.

Well, what's the point of communicating with a non-existent device in
the first place?

> So binding spidev is in my view just saying that you are going to
> transfer random data from userspace on this bus.

Yes, to a device connected on that bus.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (2.80 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-04-26 14:15:20

by Michal Suchanek

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

On 26 April 2015 at 14:51, Maxime Ripard
<[email protected]> wrote:
> On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote:
>> On 26 April 2015 at 13:56, Martin Sperl <[email protected]> wrote:
>> >
>> >> On 26.04.2015, at 13:23, Hans de Goede <[email protected]> wrote:
>> >> I think there is actual a use for just binding spidev as spidev,
>> >> think e.g. the spi pins on the raspberry pi.
>> >>
>> >> How do you deal we suggest with such a situation ?
>> >
>> > I actually asked the same question a few days ago on the spi list
>> > (in thread: "spi: spidev: Warn loudly if instantiated from DT as “spidev”)
>> > and the summary was:
>> >
>> > You can still do as before, but you have to accept that long
>> > irritating warning.
>> >
>> > Or you patch spidev.c to include your pattern of choice for compatiblity
>>
>> So the suggestion is to add a compatible string like olimex,uext-slot
>> to spidev and use that compatible in the DT?
>
> No, you add a compatible for the device that is connected to the bus
> through that slot.

There is no device connected in the slot by design. The slot is there
for connecting random stuff you find in your mailbox or other drawers
and boxes.

>
>> That can certainly be done but adding a new compatible for every board
>> that has some random pins looks like a needless nuisance to me.
>> Especially compared to i2c where you can just open the bus so long as
>> ti is enabled.
>>
>> >
>> > Or you implement the following proposal (which needs a volunteer):
>> >> On 23.04.2015, at 09:42, Geert Uytterhoeven <[email protected]> wrote:
>> >>
>> >> So what you need is a way to handover from generic spidev to a device-specific
>> >> driver, cfr. what graphics drivers do when the device has been bound to by
>> >> vesafb or simplefb.
>> >>
>> >> Could this be implemented in a generic way in the spi or DT code?
>> >
>> > ...
>> >> On 23.04.2015, at 12:36, Mark Brown <[email protected]> wrote:
>> >> On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote:
>> >>
>> >>> I guess this has been suggested before: the spi core could provide spidev
>> >>> access to all spi client devices which are not bound by a driver?
>> >>
>> >> I don't know if it's been suggested before, certainly nobody did the
>> >> work to make it happen. I don't think I have a massive objection in
>> >> principal.
>
> Actually, I did it a year ago, and it looked at the time that it
> wasn't what should be done either.

There is nothing like unclaimed device. Either there is a device and
driver for it may in principle be loaded later as a module or the chip
select is reserved for use from userspace.

Userspace driver is valid option and should have the ability to have
the chip select reserved.

>
> https://lkml.org/lkml/2014/4/28/612
>
>> But how do you know there is a device?
>>
>> Devices on i2c can be probed. On spi you just transfer random data and
>> hope it does something useful. Some devices have readable registers
>> and can be probed in a device-specific way but others are write-only.
>
> Well, what's the point of communicating with a non-existent device in
> the first place?

I have multitude of SPI devices which are not part of the board and
hence its DT and can be connected to the board with jumper wires.

Most of them don't have a linux driver or compatible to bind with.

>
>> So binding spidev is in my view just saying that you are going to
>> transfer random data from userspace on this bus.
>
> Yes, to a device connected on that bus.

Yes, so I have this red rectangular PCB, blue PCB, and red square PCB
and blue very thin rectangular PCB.

Please enlighten me how to add DT bindings for these and the PCB which
is in the mail and I did not pick up at the post office yet.

Of course, I have some hope that the chips on the PCBs are at least
somewhat compatible with what I ordered but I cannot be sure until I
test that.

Thanks

Michal

2015-04-26 14:35:08

by Maxime Ripard

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

On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:
> On 26 April 2015 at 14:51, Maxime Ripard
> <[email protected]> wrote:
> > On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote:
> >> On 26 April 2015 at 13:56, Martin Sperl <[email protected]> wrote:
> >> >
> >> >> On 26.04.2015, at 13:23, Hans de Goede <[email protected]> wrote:
> >> >> I think there is actual a use for just binding spidev as spidev,
> >> >> think e.g. the spi pins on the raspberry pi.
> >> >>
> >> >> How do you deal we suggest with such a situation ?
> >> >
> >> > I actually asked the same question a few days ago on the spi list
> >> > (in thread: "spi: spidev: Warn loudly if instantiated from DT as “spidev”)
> >> > and the summary was:
> >> >
> >> > You can still do as before, but you have to accept that long
> >> > irritating warning.
> >> >
> >> > Or you patch spidev.c to include your pattern of choice for compatiblity
> >>
> >> So the suggestion is to add a compatible string like olimex,uext-slot
> >> to spidev and use that compatible in the DT?
> >
> > No, you add a compatible for the device that is connected to the bus
> > through that slot.
>
> There is no device connected in the slot by design. The slot is there
> for connecting random stuff you find in your mailbox or other drawers
> and boxes.

I know. Our point is add a compatible for that random device you find
in your mailbox.

> >> That can certainly be done but adding a new compatible for every board
> >> that has some random pins looks like a needless nuisance to me.
> >> Especially compared to i2c where you can just open the bus so long as
> >> ti is enabled.
> >>
> >> >
> >> > Or you implement the following proposal (which needs a volunteer):
> >> >> On 23.04.2015, at 09:42, Geert Uytterhoeven <[email protected]> wrote:
> >> >>
> >> >> So what you need is a way to handover from generic spidev to a device-specific
> >> >> driver, cfr. what graphics drivers do when the device has been bound to by
> >> >> vesafb or simplefb.
> >> >>
> >> >> Could this be implemented in a generic way in the spi or DT code?
> >> >
> >> > ...
> >> >> On 23.04.2015, at 12:36, Mark Brown <[email protected]> wrote:
> >> >> On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote:
> >> >>
> >> >>> I guess this has been suggested before: the spi core could provide spidev
> >> >>> access to all spi client devices which are not bound by a driver?
> >> >>
> >> >> I don't know if it's been suggested before, certainly nobody did the
> >> >> work to make it happen. I don't think I have a massive objection in
> >> >> principal.
> >
> > Actually, I did it a year ago, and it looked at the time that it
> > wasn't what should be done either.
>
> There is nothing like unclaimed device. Either there is a device and
> driver for it may in principle be loaded later as a module or the chip
> select is reserved for use from userspace.

I never said it was perfect.

> Userspace driver is valid option and should have the ability to have
> the chip select reserved.

Whether an userspace driver is a valid option can spawn a whole debate
by its own, but it's true that we should be able to have it exported
to the userspace.

However, having a spidev compatible is not the solution to that
problem.

> > https://lkml.org/lkml/2014/4/28/612
> >
> >> But how do you know there is a device?
> >>
> >> Devices on i2c can be probed. On spi you just transfer random data and
> >> hope it does something useful. Some devices have readable registers
> >> and can be probed in a device-specific way but others are write-only.
> >
> > Well, what's the point of communicating with a non-existent device in
> > the first place?
>
> I have multitude of SPI devices which are not part of the board and
> hence its DT and can be connected to the board with jumper wires.
>
> Most of them don't have a linux driver or compatible to bind with.

Then create such a compatible...

> >> So binding spidev is in my view just saying that you are going to
> >> transfer random data from userspace on this bus.
> >
> > Yes, to a device connected on that bus.
>
> Yes, so I have this red rectangular PCB, blue PCB, and red square PCB
> and blue very thin rectangular PCB.
>
> Please enlighten me how to add DT bindings for these and the PCB which
> is in the mail and I did not pick up at the post office yet.
>
> Of course, I have some hope that the chips on the PCBs are at least
> somewhat compatible with what I ordered but I cannot be sure until I
> test that.

Come on, this is pure bad faith. If you don't even know what that
device is, how do you even know how to interact with it?

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (4.69 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-04-26 14:41:21

by Hans de Goede

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

Hi,

I've a feeling everyone in this thread is ignoring the
raspberry pi use-case. Where the board is specifically
designed for educational purposes and used with lots of
peripherals which are usually programmed from userspace
using e.g. python bindings for i2c-dev or spidev, for
such a setup we really want spidev to be loaded on the
spibus by default and we really do not have a proper
compatible for a child device.

And no having to use per device devicetree overlays
for this is not the answer, this needs to be really
really easy. With pre device-tree kernels this just
works, we should be able to match that ease of use
with devicetree.

Regards,

Hans

2015-04-26 15:34:20

by Michal Suchanek

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

On 26 April 2015 at 16:33, Maxime Ripard
<[email protected]> wrote:
> On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:
>> On 26 April 2015 at 14:51, Maxime Ripard
>> <[email protected]> wrote:
>> > On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote:
>> >> On 26 April 2015 at 13:56, Martin Sperl <[email protected]> wrote:
>> >> >
>> >> >> On 26.04.2015, at 13:23, Hans de Goede <[email protected]> wrote:
>> >> >> I think there is actual a use for just binding spidev as spidev,
>> >> >> think e.g. the spi pins on the raspberry pi.
>> >> >>
>> >> >> How do you deal we suggest with such a situation ?
>> >> >
>> >> > I actually asked the same question a few days ago on the spi list
>> >> > (in thread: "spi: spidev: Warn loudly if instantiated from DT as “spidev”)
>> >> > and the summary was:
>> >> >
>> >> > You can still do as before, but you have to accept that long
>> >> > irritating warning.
>> >> >
>> >> > Or you patch spidev.c to include your pattern of choice for compatiblity
>> >>
>> >> So the suggestion is to add a compatible string like olimex,uext-slot
>> >> to spidev and use that compatible in the DT?
>> >
>> > No, you add a compatible for the device that is connected to the bus
>> > through that slot.
>>
>> There is no device connected in the slot by design. The slot is there
>> for connecting random stuff you find in your mailbox or other drawers
>> and boxes.
>
> I know. Our point is add a compatible for that random device you find
> in your mailbox.

That would be mailbox,device-tbd I suppose?

>
>> >> That can certainly be done but adding a new compatible for every board
>> >> that has some random pins looks like a needless nuisance to me.
>> >> Especially compared to i2c where you can just open the bus so long as
>> >> ti is enabled.
>> >>
>> >> >
>> >> > Or you implement the following proposal (which needs a volunteer):
>> >> >> On 23.04.2015, at 09:42, Geert Uytterhoeven <[email protected]> wrote:
>> >> >>
>> >> >> So what you need is a way to handover from generic spidev to a device-specific
>> >> >> driver, cfr. what graphics drivers do when the device has been bound to by
>> >> >> vesafb or simplefb.
>> >> >>
>> >> >> Could this be implemented in a generic way in the spi or DT code?
>> >> >
>> >> > ...
>> >> >> On 23.04.2015, at 12:36, Mark Brown <[email protected]> wrote:
>> >> >> On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote:
>> >> >>
>> >> >>> I guess this has been suggested before: the spi core could provide spidev
>> >> >>> access to all spi client devices which are not bound by a driver?
>> >> >>
>> >> >> I don't know if it's been suggested before, certainly nobody did the
>> >> >> work to make it happen. I don't think I have a massive objection in
>> >> >> principal.
>> >
>> > Actually, I did it a year ago, and it looked at the time that it
>> > wasn't what should be done either.
>>
>> There is nothing like unclaimed device. Either there is a device and
>> driver for it may in principle be loaded later as a module or the chip
>> select is reserved for use from userspace.
>
> I never said it was perfect.
>
>> Userspace driver is valid option and should have the ability to have
>> the chip select reserved.
>
> Whether an userspace driver is a valid option can spawn a whole debate
> by its own, but it's true that we should be able to have it exported
> to the userspace.
>
> However, having a spidev compatible is not the solution to that
> problem.

Having to patch the kernel to use an unknown device with userspace
driver is not the answer either.

Devices which are not performance critical and don't use existing
kernel frameworks don't have any use for kernel driver.

In fact, writing a kernel driver for them is counter-productive
because you will have to write from scratch when you connect the
device to a box running another OS due to interface *and* license
difference.

Also for driver prototyping you need a compatible which makes the
device accessible.

If no spidev general compatible is available people will just use
comaptible for some random device which happens to bind to spidev and
will send many letters of thanks to the DT maintainers when the device
used for this purpose suddenly grows a Linux driver.

>
>> > https://lkml.org/lkml/2014/4/28/612
>> >
>> >> But how do you know there is a device?
>> >>
>> >> Devices on i2c can be probed. On spi you just transfer random data and
>> >> hope it does something useful. Some devices have readable registers
>> >> and can be probed in a device-specific way but others are write-only.
>> >
>> > Well, what's the point of communicating with a non-existent device in
>> > the first place?
>>
>> I have multitude of SPI devices which are not part of the board and
>> hence its DT and can be connected to the board with jumper wires.
>>
>> Most of them don't have a linux driver or compatible to bind with.
>
> Then create such a compatible...

I will if and when the device is usable.

>
>> >> So binding spidev is in my view just saying that you are going to
>> >> transfer random data from userspace on this bus.
>> >
>> > Yes, to a device connected on that bus.
>>
>> Yes, so I have this red rectangular PCB, blue PCB, and red square PCB
>> and blue very thin rectangular PCB.
>>
>> Please enlighten me how to add DT bindings for these and the PCB which
>> is in the mail and I did not pick up at the post office yet.
>>
>> Of course, I have some hope that the chips on the PCBs are at least
>> somewhat compatible with what I ordered but I cannot be sure until I
>> test that.
>
> Come on, this is pure bad faith. If you don't even know what that
> device is, how do you even know how to interact with it?
>

It either works as described or not. How do you even find out without
without talking to the device?

Thanks

Michal

2015-04-26 15:50:07

by Maxime Ripard

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

On Sun, Apr 26, 2015 at 04:40:50PM +0200, Hans de Goede wrote:
> Hi,
>
> I've a feeling everyone in this thread is ignoring the
> raspberry pi use-case. Where the board is specifically
> designed for educational purposes and used with lots of
> peripherals which are usually programmed from userspace
> using e.g. python bindings for i2c-dev or spidev, for
> such a setup we really want spidev to be loaded on the
> spibus by default and we really do not have a proper
> compatible for a child device.

I'm not sure we're ignoring it, it just is the exact same use case
than the whole spidev use case: people want to write SPI userspace
drivers, the rpi really is not special here, except maybe for its user
space code base, but it really boils down to the same issue.

> And no having to use per device devicetree overlays
> for this is not the answer, this needs to be really
> really easy. With pre device-tree kernels this just
> works, we should be able to match that ease of use
> with devicetree.

We do agree on that. We repeatedly told that the DT was not a good
solution, overlays or not, and this is exactly one of the reasons.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


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

2015-04-26 15:55:07

by Maxime Ripard

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

On Sun, Apr 26, 2015 at 05:33:36PM +0200, Michal Suchanek wrote:
> On 26 April 2015 at 16:33, Maxime Ripard
> <[email protected]> wrote:
> > On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:
> >> On 26 April 2015 at 14:51, Maxime Ripard
> >> <[email protected]> wrote:
> >> > On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote:
> >> >> On 26 April 2015 at 13:56, Martin Sperl <[email protected]> wrote:
> >> >> >
> >> >> >> On 26.04.2015, at 13:23, Hans de Goede <[email protected]> wrote:
> >> >> >> I think there is actual a use for just binding spidev as spidev,
> >> >> >> think e.g. the spi pins on the raspberry pi.
> >> >> >>
> >> >> >> How do you deal we suggest with such a situation ?
> >> >> >
> >> >> > I actually asked the same question a few days ago on the spi list
> >> >> > (in thread: "spi: spidev: Warn loudly if instantiated from DT as “spidev”)
> >> >> > and the summary was:
> >> >> >
> >> >> > You can still do as before, but you have to accept that long
> >> >> > irritating warning.
> >> >> >
> >> >> > Or you patch spidev.c to include your pattern of choice for compatiblity
> >> >>
> >> >> So the suggestion is to add a compatible string like olimex,uext-slot
> >> >> to spidev and use that compatible in the DT?
> >> >
> >> > No, you add a compatible for the device that is connected to the bus
> >> > through that slot.
> >>
> >> There is no device connected in the slot by design. The slot is there
> >> for connecting random stuff you find in your mailbox or other drawers
> >> and boxes.
> >
> > I know. Our point is add a compatible for that random device you find
> > in your mailbox.
>
> That would be mailbox,device-tbd I suppose?

If you can find a programming model and a matching datasheet for that
device, I suppose, yes.

> >> >> That can certainly be done but adding a new compatible for every board
> >> >> that has some random pins looks like a needless nuisance to me.
> >> >> Especially compared to i2c where you can just open the bus so long as
> >> >> ti is enabled.
> >> >>
> >> >> >
> >> >> > Or you implement the following proposal (which needs a volunteer):
> >> >> >> On 23.04.2015, at 09:42, Geert Uytterhoeven <[email protected]> wrote:
> >> >> >>
> >> >> >> So what you need is a way to handover from generic spidev to a device-specific
> >> >> >> driver, cfr. what graphics drivers do when the device has been bound to by
> >> >> >> vesafb or simplefb.
> >> >> >>
> >> >> >> Could this be implemented in a generic way in the spi or DT code?
> >> >> >
> >> >> > ...
> >> >> >> On 23.04.2015, at 12:36, Mark Brown <[email protected]> wrote:
> >> >> >> On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote:
> >> >> >>
> >> >> >>> I guess this has been suggested before: the spi core could provide spidev
> >> >> >>> access to all spi client devices which are not bound by a driver?
> >> >> >>
> >> >> >> I don't know if it's been suggested before, certainly nobody did the
> >> >> >> work to make it happen. I don't think I have a massive objection in
> >> >> >> principal.
> >> >
> >> > Actually, I did it a year ago, and it looked at the time that it
> >> > wasn't what should be done either.
> >>
> >> There is nothing like unclaimed device. Either there is a device and
> >> driver for it may in principle be loaded later as a module or the chip
> >> select is reserved for use from userspace.
> >
> > I never said it was perfect.
> >
> >> Userspace driver is valid option and should have the ability to have
> >> the chip select reserved.
> >
> > Whether an userspace driver is a valid option can spawn a whole debate
> > by its own, but it's true that we should be able to have it exported
> > to the userspace.
> >
> > However, having a spidev compatible is not the solution to that
> > problem.
>
> Having to patch the kernel to use an unknown device with userspace
> driver is not the answer either.
>
> Devices which are not performance critical and don't use existing
> kernel frameworks don't have any use for kernel driver.
>
> In fact, writing a kernel driver for them is counter-productive
> because you will have to write from scratch when you connect the
> device to a box running another OS due to interface *and* license
> difference.

And here is the debate...

> Also for driver prototyping you need a compatible which makes the
> device accessible.
>
> If no spidev general compatible is available people will just use
> comaptible for some random device which happens to bind to spidev and
> will send many letters of thanks to the DT maintainers when the device
> used for this purpose suddenly grows a Linux driver.

If people do dumb things, they should expect it to backfire.

> >> > https://lkml.org/lkml/2014/4/28/612
> >> >
> >> >> But how do you know there is a device?
> >> >>
> >> >> Devices on i2c can be probed. On spi you just transfer random data and
> >> >> hope it does something useful. Some devices have readable registers
> >> >> and can be probed in a device-specific way but others are write-only.
> >> >
> >> > Well, what's the point of communicating with a non-existent device in
> >> > the first place?
> >>
> >> I have multitude of SPI devices which are not part of the board and
> >> hence its DT and can be connected to the board with jumper wires.
> >>
> >> Most of them don't have a linux driver or compatible to bind with.
> >
> > Then create such a compatible...
>
> I will if and when the device is usable.

That's backward. The fact that your "driver" works really doesn't
depend on what the device actually is.

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (5.59 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-04-26 18:54:04

by Michal Suchanek

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

On 26 April 2015 at 17:54, Maxime Ripard
<[email protected]> wrote:
> On Sun, Apr 26, 2015 at 05:33:36PM +0200, Michal Suchanek wrote:
>> On 26 April 2015 at 16:33, Maxime Ripard
>> <[email protected]> wrote:
>> > On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:
>> >> On 26 April 2015 at 14:51, Maxime Ripard
>> >> <[email protected]> wrote:
>> >> > On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote:
>> >> >> On 26 April 2015 at 13:56, Martin Sperl <[email protected]> wrote:
>> >> >> >
>> >> >> >> On 26.04.2015, at 13:23, Hans de Goede <[email protected]> wrote:
>> >> >> >> I think there is actual a use for just binding spidev as spidev,
>> >> >> >> think e.g. the spi pins on the raspberry pi.
>> >> >> >>
>> >> >> >> How do you deal we suggest with such a situation ?
>> >> >> >
>> >> >> > I actually asked the same question a few days ago on the spi list
>> >> >> > (in thread: "spi: spidev: Warn loudly if instantiated from DT as “spidev”)
>> >> >> > and the summary was:
>> >> >> >
>> >> >> > You can still do as before, but you have to accept that long
>> >> >> > irritating warning.
>> >> >> >
>> >> >> > Or you patch spidev.c to include your pattern of choice for compatiblity
>> >> >>
>> >> >> So the suggestion is to add a compatible string like olimex,uext-slot
>> >> >> to spidev and use that compatible in the DT?
>> >> >
>> >> > No, you add a compatible for the device that is connected to the bus
>> >> > through that slot.
>> >>
>> >> There is no device connected in the slot by design. The slot is there
>> >> for connecting random stuff you find in your mailbox or other drawers
>> >> and boxes.
>> >
>> > I know. Our point is add a compatible for that random device you find
>> > in your mailbox.
>>
>> That would be mailbox,device-tbd I suppose?
>
> If you can find a programming model and a matching datasheet for that
> device, I suppose, yes.
>
>> >> >> That can certainly be done but adding a new compatible for every board
>> >> >> that has some random pins looks like a needless nuisance to me.
>> >> >> Especially compared to i2c where you can just open the bus so long as
>> >> >> ti is enabled.
>> >> >>
>> >> >> >
>> >> >> > Or you implement the following proposal (which needs a volunteer):
>> >> >> >> On 23.04.2015, at 09:42, Geert Uytterhoeven <[email protected]> wrote:
>> >> >> >>
>> >> >> >> So what you need is a way to handover from generic spidev to a device-specific
>> >> >> >> driver, cfr. what graphics drivers do when the device has been bound to by
>> >> >> >> vesafb or simplefb.
>> >> >> >>
>> >> >> >> Could this be implemented in a generic way in the spi or DT code?
>> >> >> >
>> >> >> > ...
>> >> >> >> On 23.04.2015, at 12:36, Mark Brown <[email protected]> wrote:
>> >> >> >> On Thu, Apr 23, 2015 at 09:45:16AM +0200, Geert Uytterhoeven wrote:
>> >> >> >>
>> >> >> >>> I guess this has been suggested before: the spi core could provide spidev
>> >> >> >>> access to all spi client devices which are not bound by a driver?
>> >> >> >>
>> >> >> >> I don't know if it's been suggested before, certainly nobody did the
>> >> >> >> work to make it happen. I don't think I have a massive objection in
>> >> >> >> principal.
>> >> >
>> >> > Actually, I did it a year ago, and it looked at the time that it
>> >> > wasn't what should be done either.
>> >>
>> >> There is nothing like unclaimed device. Either there is a device and
>> >> driver for it may in principle be loaded later as a module or the chip
>> >> select is reserved for use from userspace.
>> >
>> > I never said it was perfect.
>> >
>> >> Userspace driver is valid option and should have the ability to have
>> >> the chip select reserved.
>> >
>> > Whether an userspace driver is a valid option can spawn a whole debate
>> > by its own, but it's true that we should be able to have it exported
>> > to the userspace.
>> >
>> > However, having a spidev compatible is not the solution to that
>> > problem.
>>
>> Having to patch the kernel to use an unknown device with userspace
>> driver is not the answer either.
>>
>> Devices which are not performance critical and don't use existing
>> kernel frameworks don't have any use for kernel driver.
>>
>> In fact, writing a kernel driver for them is counter-productive
>> because you will have to write from scratch when you connect the
>> device to a box running another OS due to interface *and* license
>> difference.
>
> And here is the debate...
>
>> Also for driver prototyping you need a compatible which makes the
>> device accessible.
>>
>> If no spidev general compatible is available people will just use
>> compatible for some random device which happens to bind to spidev and
>> will send many letters of thanks to the DT maintainers when the device
>> used for this purpose suddenly grows a Linux driver.
>
> If people do dumb things, they should expect it to backfire.

Yes, dumb things like not allowing people to say in the DT that the
board actually has pins on it connected to a SPI bus. Which is the
actual hardware which should be described in the DT.

Do you have to describe a modem or terminal emulator in DT to connect
it to your serial port? You just describe the port. So here you have a
SPI port and it should be described in the DT as faithfully as the
serial port.

>
>> >> > https://lkml.org/lkml/2014/4/28/612
>> >> >
>> >> >> But how do you know there is a device?
>> >> >>
>> >> >> Devices on i2c can be probed. On spi you just transfer random data and
>> >> >> hope it does something useful. Some devices have readable registers
>> >> >> and can be probed in a device-specific way but others are write-only.
>> >> >
>> >> > Well, what's the point of communicating with a non-existent device in
>> >> > the first place?
>> >>
>> >> I have multitude of SPI devices which are not part of the board and
>> >> hence its DT and can be connected to the board with jumper wires.
>> >>
>> >> Most of them don't have a linux driver or compatible to bind with.
>> >
>> > Then create such a compatible...
>>
>> I will if and when the device is usable.
>
> That's backward. The fact that your "driver" works really doesn't
> depend on what the device actually is.

Indeed.

However, for the device to have a compatible the compatible must be
specified in a driver and then I need a driver for the device to
record the compatible in.

Or do you suggest that I patch the compatible into spidev, write a
driver for it, and then back out the compatible from spidev and check
in the compatible again with the driver?

Now that is backwards.

Thanks

Michal

2015-04-27 06:51:58

by Michal Suchanek

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

On 26 April 2015 at 17:47, Maxime Ripard
<[email protected]> wrote:
> On Sun, Apr 26, 2015 at 04:40:50PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> I've a feeling everyone in this thread is ignoring the
>> raspberry pi use-case. Where the board is specifically
>> designed for educational purposes and used with lots of
>> peripherals which are usually programmed from userspace
>> using e.g. python bindings for i2c-dev or spidev, for
>> such a setup we really want spidev to be loaded on the
>> spibus by default and we really do not have a proper
>> compatible for a child device.
>
> I'm not sure we're ignoring it, it just is the exact same use case
> than the whole spidev use case: people want to write SPI userspace
> drivers, the rpi really is not special here, except maybe for its user
> space code base, but it really boils down to the same issue.
>
>> And no having to use per device devicetree overlays
>> for this is not the answer, this needs to be really
>> really easy. With pre device-tree kernels this just
>> works, we should be able to match that ease of use
>> with devicetree.
>
> We do agree on that. We repeatedly told that the DT was not a good
> solution, overlays or not, and this is exactly one of the reasons.
>

Ok, so how about skipping the bindings altogether.

Just instantiate a spidev for each SPI bus and each CS the SPI core
knows of once spidev is loaded.

Thanks

Michal

2015-04-27 09:36:52

by Mark Brown

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

On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:
> On 26 April 2015 at 14:51, Maxime Ripard

> > No, you add a compatible for the device that is connected to the bus
> > through that slot.

> There is no device connected in the slot by design. The slot is there
> for connecting random stuff you find in your mailbox or other drawers
> and boxes.

You should be using device tree overlays to describe what actually ended
up getting connected there.


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

2015-04-27 09:40:34

by Michal Suchanek

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

On 27 April 2015 at 11:36, Mark Brown <[email protected]> wrote:
> On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:
>> On 26 April 2015 at 14:51, Maxime Ripard
>
>> > No, you add a compatible for the device that is connected to the bus
>> > through that slot.
>
>> There is no device connected in the slot by design. The slot is there
>> for connecting random stuff you find in your mailbox or other drawers
>> and boxes.
>
> You should be using device tree overlays to describe what actually ended
> up getting connected there.

NO

2015-04-27 10:04:44

by Hans de Goede

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

Hi Mark,

On 27-04-15 11:36, Mark Brown wrote:
> On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:
>> On 26 April 2015 at 14:51, Maxime Ripard
>
>>> No, you add a compatible for the device that is connected to the bus
>>> through that slot.
>
>> There is no device connected in the slot by design. The slot is there
>> for connecting random stuff you find in your mailbox or other drawers
>> and boxes.
>
> You should be using device tree overlays to describe what actually ended
> up getting connected there.

Have you seen my mail about the raspberry pi use-case? Using dt-overlays
simply is not an acceptable answer there. There are legitimate use-cases
for a "generic spi bus" concept with the bus only being accessible via
spidev.

Blocking this use-case because you do not believe it is a valid use-case
is not going to help, this will just lead to the custom distros these
boards are shipping doing some ugly hack, which is not what we want
IMHO.

I hope that with the raspberry pi 2 the raspberry pi-s will eventually
go fully devicetree / multi-platform kernel so that they can be supported
ootb by distros which only want to ship a single multi-platform kernel
like Debian, but that does require us to solve problems like this one.

Regards,

Hans

2015-04-27 10:05:11

by Maxime Ripard

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

On Sun, Apr 26, 2015 at 08:53:16PM +0200, Michal Suchanek wrote:
> >> Also for driver prototyping you need a compatible which makes the
> >> device accessible.
> >>
> >> If no spidev general compatible is available people will just use
> >> compatible for some random device which happens to bind to spidev and
> >> will send many letters of thanks to the DT maintainers when the device
> >> used for this purpose suddenly grows a Linux driver.
> >
> > If people do dumb things, they should expect it to backfire.
>
> Yes, dumb things like not allowing people to say in the DT that the
> board actually has pins on it connected to a SPI bus. Which is the
> actual hardware which should be described in the DT.

It's not connected to an SPI bus. It's connected to a device using an
SPI bus. If you just had floating SPI lines, I'm pretty sure you
wouldn't care about spidev at all.

> Do you have to describe a modem or terminal emulator in DT to connect
> it to your serial port? You just describe the port. So here you have a
> SPI port and it should be described in the DT as faithfully as the
> serial port.

Except that in the serial port, you have a representation of a bus,
while spidev represents a *device* connected on an SPI bus. So these
are two different things, really.

> >> >> > https://lkml.org/lkml/2014/4/28/612
> >> >> >
> >> >> >> But how do you know there is a device?
> >> >> >>
> >> >> >> Devices on i2c can be probed. On spi you just transfer random data and
> >> >> >> hope it does something useful. Some devices have readable registers
> >> >> >> and can be probed in a device-specific way but others are write-only.
> >> >> >
> >> >> > Well, what's the point of communicating with a non-existent device in
> >> >> > the first place?
> >> >>
> >> >> I have multitude of SPI devices which are not part of the board and
> >> >> hence its DT and can be connected to the board with jumper wires.
> >> >>
> >> >> Most of them don't have a linux driver or compatible to bind with.
> >> >
> >> > Then create such a compatible...
> >>
> >> I will if and when the device is usable.
> >
> > That's backward. The fact that your "driver" works really doesn't
> > depend on what the device actually is.
>
> Indeed.
>
> However, for the device to have a compatible the compatible must be
> specified in a driver and then I need a driver for the device to
> record the compatible in.
>
> Or do you suggest that I patch the compatible into spidev, write a
> driver for it, and then back out the compatible from spidev and check
> in the compatible again with the driver?
>
> Now that is backwards.

What Mark was suggesting was that you add a compatible to the spidev
driver, and then you have access to spidev from userspace, period.

If later on, you introduce a real driver for that, then yes, you would
have to remove the compatible from spidev, and have that matching
compatible in that new driver.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (2.96 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-04-27 10:10:08

by Maxime Ripard

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

On Mon, Apr 27, 2015 at 08:51:12AM +0200, Michal Suchanek wrote:
> On 26 April 2015 at 17:47, Maxime Ripard
> <[email protected]> wrote:
> > On Sun, Apr 26, 2015 at 04:40:50PM +0200, Hans de Goede wrote:
> >> Hi,
> >>
> >> I've a feeling everyone in this thread is ignoring the
> >> raspberry pi use-case. Where the board is specifically
> >> designed for educational purposes and used with lots of
> >> peripherals which are usually programmed from userspace
> >> using e.g. python bindings for i2c-dev or spidev, for
> >> such a setup we really want spidev to be loaded on the
> >> spibus by default and we really do not have a proper
> >> compatible for a child device.
> >
> > I'm not sure we're ignoring it, it just is the exact same use case
> > than the whole spidev use case: people want to write SPI userspace
> > drivers, the rpi really is not special here, except maybe for its user
> > space code base, but it really boils down to the same issue.
> >
> >> And no having to use per device devicetree overlays
> >> for this is not the answer, this needs to be really
> >> really easy. With pre device-tree kernels this just
> >> works, we should be able to match that ease of use
> >> with devicetree.
> >
> > We do agree on that. We repeatedly told that the DT was not a good
> > solution, overlays or not, and this is exactly one of the reasons.
> >
>
> Ok, so how about skipping the bindings altogether.
>
> Just instantiate a spidev for each SPI bus and each CS the SPI core
> knows of once spidev is loaded.

Which is exactly what my patch did but didn't seem like good enough
for you at the time...

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.71 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-04-27 10:10:10

by Hans de Goede

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

Hi,

On 27-04-15 12:04, Hans de Goede wrote:
> Hi Mark,
>
> On 27-04-15 11:36, Mark Brown wrote:
>> On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:
>>> On 26 April 2015 at 14:51, Maxime Ripard
>>
>>>> No, you add a compatible for the device that is connected to the bus
>>>> through that slot.
>>
>>> There is no device connected in the slot by design. The slot is there
>>> for connecting random stuff you find in your mailbox or other drawers
>>> and boxes.
>>
>> You should be using device tree overlays to describe what actually ended
>> up getting connected there.
>
> Have you seen my mail about the raspberry pi use-case? Using dt-overlays
> simply is not an acceptable answer there. There are legitimate use-cases
> for a "generic spi bus" concept with the bus only being accessible via
> spidev.
>
> Blocking this use-case because you do not believe it is a valid use-case
> is not going to help, this will just lead to the custom distros these
> boards are shipping doing some ugly hack, which is not what we want
> IMHO.
>
> I hope that with the raspberry pi 2 the raspberry pi-s will eventually
> go fully devicetree / multi-platform kernel so that they can be supported
> ootb by distros which only want to ship a single multi-platform kernel
> like Debian, but that does require us to solve problems like this one.

To expand a bit on this, as you know there is this whole maker movement
thing going on, these are people using arduinos and raspberry pi-s and
they expect things to be easy and just work. This is also a huge pool
of potential new FOSS / Linux contributors. I believe that it is
important that we cater to these people.

One day one of them may have a kernel related itch and decide to scratch
it, do we work that person to then work on some weird (and often ugly)
fork of the kernel, or do we want such boards to be running a pristine
upstream kernel and have potential new contributors work on the upstream
kernel and hopefully also engage with the upstream kernel community,
where we can teach them our best practices which are often lacking in
the various "fork" communities ?

Regards,

Hans

2015-04-27 10:10:36

by Mark Brown

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

On Sun, Apr 26, 2015 at 04:40:50PM +0200, Hans de Goede wrote:

> Hi,

Please always provide context in your replies so people know what you're
talking about.

> I've a feeling everyone in this thread is ignoring the
> raspberry pi use-case. Where the board is specifically
> designed for educational purposes and used with lots of
> peripherals which are usually programmed from userspace
> using e.g. python bindings for i2c-dev or spidev, for
> such a setup we really want spidev to be loaded on the
> spibus by default and we really do not have a proper
> compatible for a child device.

No, that's a different problem - the context you describe just happens
to be your use case but it's in no way universal, there are plenty of
expansion boards out there that have devices that use real drivers
connected to them normally (and some of those could even be used in the
educational contexts you describe). I think the underlying need here is
either for a better way of registering things or better tooling around
device tree overlays to address the usability issues.


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

2015-04-27 10:16:27

by Mark Brown

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

On Sun, Apr 26, 2015 at 02:51:13PM +0200, Maxime Ripard wrote:
> On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote:

> > >> I don't know if it's been suggested before, certainly nobody did the
> > >> work to make it happen. I don't think I have a massive objection in
> > >> principal.

> Actually, I did it a year ago, and it looked at the time that it
> wasn't what should be done either.

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

lkml.org is being terrible as usual so I can't see half the thread (or
at least got fed up trying to get it to load) but I think the discussion
there petered out more than anything else. Or was it the suggestion to
make this something that the driver core supported so that we didn't
have to open code it for every bus?


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

2015-04-27 10:46:24

by Mark Brown

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

On Sun, Apr 26, 2015 at 08:53:16PM +0200, Michal Suchanek wrote:

> Do you have to describe a modem or terminal emulator in DT to connect
> it to your serial port? You just describe the port. So here you have a
> SPI port and it should be described in the DT as faithfully as the
> serial port.

Serial ports have runtime mechanisms defined for connecting devices to
them (opening the device or attaching a line discipline).

> Or do you suggest that I patch the compatible into spidev, write a
> driver for it, and then back out the compatible from spidev and check
> in the compatible again with the driver?

Until someone provides a way of binding spidev to otherwise unbound
devices that's exactly what is being suggested; if your main purpose in
this is to write a kernel driver for the device then that would be a
local modification to add the compatible (or you could just ignore the
warning) until you get something in kernel space, no need to upstream.


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

2015-04-27 10:59:49

by Mark Brown

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

On Mon, Apr 27, 2015 at 11:39:50AM +0200, Michal Suchanek wrote:
> On 27 April 2015 at 11:36, Mark Brown <[email protected]> wrote:
> > On Sun, Apr 26, 2015 at 04:14:33PM +0200, Michal Suchanek wrote:

> >> There is no device connected in the slot by design. The slot is there
> >> for connecting random stuff you find in your mailbox or other drawers
> >> and boxes.

> > You should be using device tree overlays to describe what actually ended
> > up getting connected there.

> NO

Please try to discuss things in a more appropriate fashion, responses
like the above are not constructive.


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

2015-04-27 11:19:05

by Michal Suchanek

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

On 27 April 2015 at 12:04, Maxime Ripard
<[email protected]> wrote:
> On Sun, Apr 26, 2015 at 08:53:16PM +0200, Michal Suchanek wrote:
>> >> Also for driver prototyping you need a compatible which makes the
>> >> device accessible.
>> >>
>> >> If no spidev general compatible is available people will just use
>> >> compatible for some random device which happens to bind to spidev and
>> >> will send many letters of thanks to the DT maintainers when the device
>> >> used for this purpose suddenly grows a Linux driver.
>> >
>> > If people do dumb things, they should expect it to backfire.
>>
>> Yes, dumb things like not allowing people to say in the DT that the
>> board actually has pins on it connected to a SPI bus. Which is the
>> actual hardware which should be described in the DT.
>
> It's not connected to an SPI bus. It's connected to a device using an
> SPI bus. If you just had floating SPI lines, I'm pretty sure you
> wouldn't care about spidev at all.
>
>> Do you have to describe a modem or terminal emulator in DT to connect
>> it to your serial port? You just describe the port. So here you have a
>> SPI port and it should be described in the DT as faithfully as the
>> serial port.
>
> Except that in the serial port, you have a representation of a bus,
> while spidev represents a *device* connected on an SPI bus. So these
> are two different things, really.

No it's the same thing, really.

With serial you just have serial lines which you expose on a connector.

With SPI you have CS so you can technically have several connectors
for the same SPI bus selected by different CS.

So yes, making a spidev entry for the connector under SPI bus is the
equivalent of making an UART entry to specify that there is a
connector.

Thanks

Michal

2015-04-27 11:26:12

by Mark Brown

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

On Mon, Apr 27, 2015 at 12:04:12PM +0200, Hans de Goede wrote:

> Have you seen my mail about the raspberry pi use-case? Using dt-overlays
> simply is not an acceptable answer there. There are legitimate use-cases
> for a "generic spi bus" concept with the bus only being accessible via
> spidev.

> Blocking this use-case because you do not believe it is a valid use-case
> is not going to help, this will just lead to the custom distros these
> boards are shipping doing some ugly hack, which is not what we want
> IMHO.

I don't think you've fully considered your use case here. As I said in
my reply to your earlier e-mail I think what you're looking for here is
something like better UI around overlays. Registering a SPI bus without
knowing what's connected to it doesn't allow generic maker style usage
of the board, it's just as likely to hinder a user as help them. For
example, if someone wants to use the SPI pins for another function such
as GPIO they'll have to handle that (and may have problems with pin
conflicts causing electrical issues if they do load up the DT with
spidev in it). If someone has a SPI device they want to bind an in
kernel driver to they'll have to handle that, if they want to use a GPIO
to provide an additional chip select they'll have to handle that too.

Note that the discussion here isn't about userspace drivers, it's about
how the hardware is described.


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

2015-04-27 14:14:41

by Martin Sperl

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

On 2015-04-27 13:25, Mark Brown wrote:
> On Mon, Apr 27, 2015 at 12:04:12PM +0200, Hans de Goede wrote:
>
>> Have you seen my mail about the raspberry pi use-case? Using dt-overlays
>> simply is not an acceptable answer there. There are legitimate use-cases
>> for a "generic spi bus" concept with the bus only being accessible via
>> spidev.
>
>> Blocking this use-case because you do not believe it is a valid use-case
>> is not going to help, this will just lead to the custom distros these
>> boards are shipping doing some ugly hack, which is not what we want
>> IMHO.
>
> I don't think you've fully considered your use case here. As I said in
> my reply to your earlier e-mail I think what you're looking for here is
> something like better UI around overlays. Registering a SPI bus without
> knowing what's connected to it doesn't allow generic maker style usage
> of the board, it's just as likely to hinder a user as help them. For
> example, if someone wants to use the SPI pins for another function such
> as GPIO they'll have to handle that (and may have problems with pin
> conflicts causing electrical issues if they do load up the DT with
> spidev in it). If someone has a SPI device they want to bind an in
> kernel driver to they'll have to handle that, if they want to use a GPIO
> to provide an additional chip select they'll have to handle that too.
>
> Note that the discussion here isn't about userspace drivers, it's about
> how the hardware is described.

Mark, maybe you are missing something of how this can get done on the
raspberry pi with devicetree (and overlays).

So here how the raspberry-foundation describes the devices in their
device-tree for spi:

dtsi:
spi0: spi@7e204000 {
compatible = "brcm,bcm2708-spi";
reg = <0x7e204000 0x1000>;
interrupts = <2 22>;
clocks = <&clk_spi>;
#address-cells = <1>;
#size-cells = <0>;
status = "disabled";
};

dts:
&spi0 {
pinctrl-names = "default";
pinctrl-0 = <&spi0_pins>;

spidev@0{
compatible = "spidev";
reg = <0>; /* CE0 */
#address-cells = <1>;
#size-cells = <0>;
spi-max-frequency = <500000>;
};

spidev@1{
compatible = "spidev";
reg = <1>; /* CE1 */
#address-cells = <1>;
#size-cells = <0>;
spi-max-frequency = <500000>;
};
};
/ {
__overrides__ {
spi = <&spi0>,"status";
};
}

So you see that spi is disabled by default - the pins are
free to use for anything.

The firmware then integrates the overrrides into the device-tree
before loading the kernel.

So to enable spi in general you add the following to /boot/config.txt:
dtparam=spi=on

That gives you spi plus two "generic" spidev devices.

If you want to include a mcp2515 you add also the following:
dtoverlay=mcp2515-can0-overlay
and that loads also the overlay for the mcp2515 can module.

The corresponding mcp2515 overlay looks like this:
/ {
/* the spi config of the can-controller itself */
fragment@1 {
target = <&spi0>;
__overlay__ {
/* needed to avoid dtc warning */
#address-cells = <1>;
#size-cells = <0>;
can0: mcp2515@0 {
reg = <0>;
compatible = "microchip,mcp2515";
pinctrl-names = "default";
pinctrl-0 = <&can0_pins>;
spi-max-frequency = <10000000>;
interrupt-parent = <&gpio>;
interrupts = <25 0x2>;
clocks = <&can0_osc>;
};
};
};
};

(left out the unimportant stuff like clocks,
interrupt GPIOs,...)

All this implements:
* easy means to enable spi if requested by user
* by default includes spidev as the default device
* but this can get just as easily get overridden by another
devicetree to get specific devices onboarded using the
in kernel drivers - there are now something like 25+
overlays provided by the foundation that follow this
pattern...

This is really about describing the hardware in the best possible
ways and keeping the interface with the users simple by having
him only to edit /boot/config.txt.

Adding your own overlays is just as simple and also quite well
defined.

So coming from this perspective I believe that there is some
concern in the raspberry pi community, because the description
they provide is now specific to the HW and their intent and so
the loud "croaking" of spidev will irritate people even when
they have done everything the best they can.

OK, I admit, the spi-devices could be separate overlays if
one really wants to have them, but they can get disabled just
as easily (by a specific overlay) if only a single device is
needed.

The only thing that could possibly be better would be that
the user would define the "real" name of the device in the
device tree and spidev would bind to it if there is no driver
available (but that would require this "fallback" binding by
spidev in case of no driver).

Martin

P.s: note that by default the foundation still relies on the
very old spi driver, but there is an overlay to load
spi-bcm2835 instead - just add:
dtoverlay=spi-bcm2835-overlay

Which basically contains:
/ {
compatible = "brcm,bcm2835", "brcm,bcm2836", "brcm,bcm2708",
"brcm,bcm2709";
/* setting up compatiblity to allow loading the spi-bcm2835 driver */
fragment@0 {
target = <&spi0>;
__overlay__ {
status = "okay";
compatible = "brcm,bcm2835-spi";
};
};
};

2015-04-27 14:29:22

by Michal Suchanek

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

On 27 April 2015 at 12:10, Mark Brown <[email protected]> wrote:
> On Sun, Apr 26, 2015 at 04:40:50PM +0200, Hans de Goede wrote:
>
>> Hi,
>
> Please always provide context in your replies so people know what you're
> talking about.
>
>> I've a feeling everyone in this thread is ignoring the
>> raspberry pi use-case. Where the board is specifically
>> designed for educational purposes and used with lots of
>> peripherals which are usually programmed from userspace
>> using e.g. python bindings for i2c-dev or spidev, for
>> such a setup we really want spidev to be loaded on the
>> spibus by default and we really do not have a proper
>> compatible for a child device.
>
> No, that's a different problem - the context you describe just happens
> to be your use case but it's in no way universal, there are plenty of
> expansion boards out there that have devices that use real drivers
> connected to them normally (and some of those could even be used in the
> educational contexts you describe). I think the underlying need here is
> either for a better way of registering things or better tooling around
> device tree overlays to address the usability issues.

No.

When you have a serial port and just connect serial device to it with
no special requirement you just specify the serial port in DT and talk
to the device directly without any DT foo.

When there is a BT module with reset lines and a kernel driver you
write in DT that you have such and such BT module with such and such
reset lines on the uart so the driver picks it up.

Same for USB attached on-board WiFi - that some USB device needs
special handling does not mean you have to specify your keyboard in DT
to connect it to an USB port.

What you are mandating here, basically, is equivalent of requiring a
DT overlay to connect an USB keyboard or mouse because it is a device
connected in hardware to the USB port and DT is supposed to describe
the hardware.

Thanks

Michal

2015-04-27 15:13:58

by Geert Uytterhoeven

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

On Mon, Apr 27, 2015 at 4:28 PM, Michal Suchanek <[email protected]> wrote:
> When you have a serial port and just connect serial device to it with
> no special requirement you just specify the serial port in DT and talk
> to the device directly without any DT foo.
>
> When there is a BT module with reset lines and a kernel driver you
> write in DT that you have such and such BT module with such and such
> reset lines on the uart so the driver picks it up.
>
> Same for USB attached on-board WiFi - that some USB device needs
> special handling does not mean you have to specify your keyboard in DT
> to connect it to an USB port.
>
> What you are mandating here, basically, is equivalent of requiring a
> DT overlay to connect an USB keyboard or mouse because it is a device
> connected in hardware to the USB port and DT is supposed to describe
> the hardware.

Unlike the spi bus, the USB bus is a discoverable bus. So you can probe
for connected devices. No need to put them in DT (but you can, if the
firmware does the probing and updates the DTB. E.g. Open Firmware did
that for PCI (which was not hot-pluggable)).

Unlike the spi bus, you can (more or less) find out if a device is attached
to a uart, by listening to the bus, and waiting for a start bit.

I do agree that it would be nice to put the other end of the uart link in DT,
as you can't always know what you're talking to. Is it a modem? Is it a
printer? Is it a BT module? Is it Super Grover? Is it Mega Mindy?

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-27 15:27:30

by Mark Brown

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

On Mon, Apr 27, 2015 at 04:14:31PM +0200, Martin Sperl wrote:
> On 2015-04-27 13:25, Mark Brown wrote:

> >I don't think you've fully considered your use case here. As I said in
> >my reply to your earlier e-mail I think what you're looking for here is
> >something like better UI around overlays. Registering a SPI bus without
> >knowing what's connected to it doesn't allow generic maker style usage
> >of the board, it's just as likely to hinder a user as help them. For

> So you see that spi is disabled by default - the pins are
> free to use for anything.

> The firmware then integrates the overrrides into the device-tree
> before loading the kernel.

> So to enable spi in general you add the following to /boot/config.txt:
> dtparam=spi=on

> That gives you spi plus two "generic" spidev devices.

OK, so that is just a default overlay which is abusing the fact that we
will bind to spidev without a DT compatible and when the binding is
undocumented (which also applies to other devices and buses sadly).

Unfortunately nobody ever mentioned this upstream and the feedback
upstream that listing spidev in a DT is a bad idea has been ignored.
The whole reason we're doing this in the first place is that we got sick
of telling people that using spidev in DTs like this was a bad idea.

> If you want to include a mcp2515 you add also the following:
> dtoverlay=mcp2515-can0-overlay
> and that loads also the overlay for the mcp2515 can module.

And this is just a user specified overlay which is completely fine
already.

> So coming from this perspective I believe that there is some
> concern in the raspberry pi community, because the description
> they provide is now specific to the HW and their intent and so
> the loud "croaking" of spidev will irritate people even when
> they have done everything the best they can.

It does sound like the people maintianing the u-boot fork for the Pi
need to talk to both u-boot upstream (nothing here is specific to the
Pi that I can see) and the kernel community a bit more. I'd be a bit
worried that they may be relying on other things that just happen to
work without being intentional (and are therefore more vulnerable to
issues) and it's a bit depressing to see things like this stuck in a
fork where only a limited community can make use of them.

> The only thing that could possibly be better would be that
> the user would define the "real" name of the device in the
> device tree and spidev would bind to it if there is no driver
> available (but that would require this "fallback" binding by
> spidev in case of no driver).

Yes, that is exactly the solution I'd suggest - change the UI to provide
a DT compatible to be used for the new device. That would also have the
benefit of meaning that users who have connected some device that does
have a driver that works with a simple binding wouldn't need to write an
overlay which seems like it should be useful.


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

2015-04-27 15:45:18

by Michal Suchanek

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

On 27 April 2015 at 17:13, Geert Uytterhoeven <[email protected]> wrote:
> On Mon, Apr 27, 2015 at 4:28 PM, Michal Suchanek <[email protected]> wrote:
>> When you have a serial port and just connect serial device to it with
>> no special requirement you just specify the serial port in DT and talk
>> to the device directly without any DT foo.
>>
>> When there is a BT module with reset lines and a kernel driver you
>> write in DT that you have such and such BT module with such and such
>> reset lines on the uart so the driver picks it up.
>>
>> Same for USB attached on-board WiFi - that some USB device needs
>> special handling does not mean you have to specify your keyboard in DT
>> to connect it to an USB port.
>>
>> What you are mandating here, basically, is equivalent of requiring a
>> DT overlay to connect an USB keyboard or mouse because it is a device
>> connected in hardware to the USB port and DT is supposed to describe
>> the hardware.
>
> Unlike the spi bus, the USB bus is a discoverable bus. So you can probe
> for connected devices. No need to put them in DT (but you can, if the
> firmware does the probing and updates the DTB. E.g. Open Firmware did
> that for PCI (which was not hot-pluggable)).
>
> Unlike the spi bus, you can (more or less) find out if a device is attached
> to a uart, by listening to the bus, and waiting for a start bit.
>
> I do agree that it would be nice to put the other end of the uart link in DT,
> as you can't always know what you're talking to. Is it a modem? Is it a
> printer? Is it a BT module? Is it Super Grover? Is it Mega Mindy?

And the point is that it can be any of those and you can swap them. So
the DT cannot and will not describe the other end of the bus. There is
some point where the static part of the hardware configuration ends
and pluggable devices start and DT has to deal with that. Sticking
your head into the sand and pretending there is no connector on the
device will only give you a fork.

When the DT is unusable for me and it is unusable for other people
then either a change gets upstream which would be nice or it's bound
to be floating around the net as a patchset which would be annoying
but has happened in the past and will again.

And we are not really discussing some fundamental change to the kernel
that might have unexpected effects on unrelated subsystems. We are
just having a bikeshedding debate if and how a SPI connector is
represented in DT.

Thanks

Michal

2015-04-27 16:25:32

by Martin Sperl

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


> On 27.04.2015, at 17:27, Mark Brown <[email protected]> wrote:
>
>
> OK, so that is just a default overlay which is abusing the fact that we
> will bind to spidev without a DT compatible and when the binding is
> undocumented (which also applies to other devices and buses sadly).
>
> Unfortunately nobody ever mentioned this upstream and the feedback
> upstream that listing spidev in a DT is a bad idea has been ignored.

Maybe it should also have been documented as such in
Documentation/spi/spidev or in Documentation/devicetree/bindings/spi/

> The whole reason we're doing this in the first place is that we got sick
> of telling people that using spidev in DTs like this was a bad idea.


The only reference I found in my history of the spi-list is this email:
> On 08.10.2014, at 22:05, Mark Brown <[email protected]> wrote:
>
> On Wed, Oct 08, 2014 at 02:27:08PM -0500, [email protected] wrote:
>
>> + spidev@0 {
>> + compatible = "spidev";
>> + reg = <0>; /* chip select */
>> + spi-max-frequency = <100000000>;
>> + };
>
> No, if you're putting spidev into the DT that's broken - describe the
> hardware, not the software you're using to control it.


And google found this patch from a little earlier:
http://comments.gmane.org/gmane.linux.ports.arm.kernel/231050

So finding this piece of information on the “use-policy” is quite hard
- google finds lots of links where it is described as working that way.

> It does sound like the people maintianing the u-boot fork for the Pi
> need to talk to both u-boot upstream (nothing here is specific to the
> Pi that I can see) and the kernel community a bit more. I'd be a bit
> worried that they may be relying on other things that just happen to
> work without being intentional (and are therefore more vulnerable to
> issues) and it's a bit depressing to see things like this stuck in a
> fork where only a limited community can make use of them.
Actually this functionality is not in u-boot, but in the firmware
boot-loader itself, which can load the kernel (and the devicetree)
without u-boot, but which can also load u-boot as an additional
intermediate boot-stage.

>> The only thing that could possibly be better would be that
>> the user would define the "real" name of the device in the
>> device tree and spidev would bind to it if there is no driver
>> available (but that would require this "fallback" binding by
>> spidev in case of no driver).
>
> Yes, that is exactly the solution I'd suggest - change the UI to provide
> a DT compatible to be used for the new device. That would also have the
> benefit of meaning that users who have connected some device that does
> have a driver that works with a simple binding wouldn't need to write an
> overlay which seems like it should be useful.

Well then why did you just make the system complain loudly and bringing
problems to people instead of solving it in a usable manner that does not
require people to maintain an out of tree patch to work around that warning?

We still have the one-line warning about using the depreciated
spi_master.transfer interface, but it is not such loud warning as this one.

I guess the time spent discussing this could have been better spent
implementing that solution instead.

All we need is a volunteer to get that implemented.

2015-04-27 17:35:20

by Maxime Ripard

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

On Mon, Apr 27, 2015 at 11:16:01AM +0100, Mark Brown wrote:
> On Sun, Apr 26, 2015 at 02:51:13PM +0200, Maxime Ripard wrote:
> > On Sun, Apr 26, 2015 at 02:38:18PM +0200, Michal Suchanek wrote:
>
> > > >> I don't know if it's been suggested before, certainly nobody did the
> > > >> work to make it happen. I don't think I have a massive objection in
> > > >> principal.
>
> > Actually, I did it a year ago, and it looked at the time that it
> > wasn't what should be done either.
>
> > https://lkml.org/lkml/2014/4/28/612
>
> lkml.org is being terrible as usual so I can't see half the thread (or
> at least got fed up trying to get it to load)

A part of it is also here:
http://lkml.iu.edu/hypermail/linux/kernel/1405.0/00484.html

> but I think the discussion there petered out more than anything
> else.

Maybe it did :)

> Or was it the suggestion to make this something that the driver core
> supported so that we didn't have to open code it for every bus?

Probably. That's something I really haven't took the time to look at,
and don't really plan on doing so.

I guess a good way forward would be to revive this patch, and wait for
that generic way to happen.

What do you think of this?

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.29 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-04-27 18:00:25

by Mark Brown

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

On Mon, Apr 27, 2015 at 06:25:26PM +0200, Martin Sperl wrote:
> > On 27.04.2015, at 17:27, Mark Brown <[email protected]> wrote:

> > OK, so that is just a default overlay which is abusing the fact that we
> > will bind to spidev without a DT compatible and when the binding is
> > undocumented (which also applies to other devices and buses sadly).

> > Unfortunately nobody ever mentioned this upstream and the feedback
> > upstream that listing spidev in a DT is a bad idea has been ignored.

> Maybe it should also have been documented as such in
> Documentation/spi/spidev or in Documentation/devicetree/bindings/spi/

It was documented in the DT bindings in that there was no documented
binding; people are in general expected to know that anything they're
using should be documented. For the main spidev document I guess it's
something similar, though if you can think of something there by all
means send a patch.

> > The whole reason we're doing this in the first place is that we got sick
> > of telling people that using spidev in DTs like this was a bad idea.

> The only reference I found in my history of the spi-list is this email:

It mostly comes up in review of DT files for boards so won't be on the
SPI list.

> > It does sound like the people maintianing the u-boot fork for the Pi
> > need to talk to both u-boot upstream (nothing here is specific to the
> > Pi that I can see) and the kernel community a bit more. I'd be a bit
> > worried that they may be relying on other things that just happen to
> > work without being intentional (and are therefore more vulnerable to
> > issues) and it's a bit depressing to see things like this stuck in a
> > fork where only a limited community can make use of them.

> Actually this functionality is not in u-boot, but in the firmware
> boot-loader itself, which can load the kernel (and the devicetree)
> without u-boot, but which can also load u-boot as an additional
> intermediate boot-stage.

Ugh, right. The thing about talking to the kernel community does still
stand though.

> >> The only thing that could possibly be better would be that
> >> the user would define the "real" name of the device in the
> >> device tree and spidev would bind to it if there is no driver
> >> available (but that would require this "fallback" binding by
> >> spidev in case of no driver).

> > Yes, that is exactly the solution I'd suggest - change the UI to provide
> > a DT compatible to be used for the new device. That would also have the
> > benefit of meaning that users who have connected some device that does
> > have a driver that works with a simple binding wouldn't need to write an
> > overlay which seems like it should be useful.

> Well then why did you just make the system complain loudly and bringing
> problems to people instead of solving it in a usable manner that does not
> require people to maintain an out of tree patch to work around that warning?

This is quite honestly the first time I've heard of this bootloader
interface that's been implemented for the Pi. All the other users I've
been aware of have been writing DT files or overlays in tree and
therefore it's not a substantial effort (and indeed the misuse is
basically just people being lazy) - if you can use the shipped binaries
it's a very different tradeoff.

> We still have the one-line warning about using the depreciated
> spi_master.transfer interface, but it is not such loud warning as this one.

Right, that is a purely in kernel interface visible only to people
directly working on implementing SPI controller drivers that isn't
especially open to misuse and therefore both less serious and more
likely to be noticed.

> I guess the time spent discussing this could have been better spent
> implementing that solution instead.

> All we need is a volunteer to get that implemented.

Yes.


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

2015-04-27 18:07:49

by Mark Brown

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

On Mon, Apr 27, 2015 at 07:30:36PM +0200, Maxime Ripard wrote:
> On Mon, Apr 27, 2015 at 11:16:01AM +0100, Mark Brown wrote:

> > lkml.org is being terrible as usual so I can't see half the thread (or
> > at least got fed up trying to get it to load)

> A part of it is also here:
> http://lkml.iu.edu/hypermail/linux/kernel/1405.0/00484.html

OK, thanks.

> > but I think the discussion there petered out more than anything
> > else.

> Maybe it did :)

I think so looking at the archives.

> > Or was it the suggestion to make this something that the driver core
> > supported so that we didn't have to open code it for every bus?

> Probably. That's something I really haven't took the time to look at,
> and don't really plan on doing so.

> I guess a good way forward would be to revive this patch, and wait for
> that generic way to happen.

> What do you think of this?

Probably best, the Pi bootloader does make it a bit more important.
Might also be worth speaking to Greg though.


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

2015-05-12 14:30:09

by Maxime Ripard

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

On Mon, Apr 27, 2015 at 07:07:28PM +0100, Mark Brown wrote:
> On Mon, Apr 27, 2015 at 07:30:36PM +0200, Maxime Ripard wrote:
> > On Mon, Apr 27, 2015 at 11:16:01AM +0100, Mark Brown wrote:
>
> > > lkml.org is being terrible as usual so I can't see half the thread (or
> > > at least got fed up trying to get it to load)
>
> > A part of it is also here:
> > http://lkml.iu.edu/hypermail/linux/kernel/1405.0/00484.html
>
> OK, thanks.
>
> > > but I think the discussion there petered out more than anything
> > > else.
>
> > Maybe it did :)
>
> I think so looking at the archives.
>
> > > Or was it the suggestion to make this something that the driver core
> > > supported so that we didn't have to open code it for every bus?
>
> > Probably. That's something I really haven't took the time to look at,
> > and don't really plan on doing so.
>
> > I guess a good way forward would be to revive this patch, and wait for
> > that generic way to happen.
>
> > What do you think of this?
>
> Probably best, the Pi bootloader does make it a bit more important.
> Might also be worth speaking to Greg though.

So, do you want me to resend that patch and discuss this directly
there (with Greg in Cc) ?

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


Attachments:
(No filename) (1.29 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-05-12 14:52:45

by Michal Suchanek

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

Hello,

On 12 May 2015 at 16:27, Maxime Ripard <[email protected]> wrote:
> On Mon, Apr 27, 2015 at 07:07:28PM +0100, Mark Brown wrote:
>> On Mon, Apr 27, 2015 at 07:30:36PM +0200, Maxime Ripard wrote:
>> > On Mon, Apr 27, 2015 at 11:16:01AM +0100, Mark Brown wrote:
>>
>> > > lkml.org is being terrible as usual so I can't see half the thread (or
>> > > at least got fed up trying to get it to load)
>>
>> > A part of it is also here:
>> > http://lkml.iu.edu/hypermail/linux/kernel/1405.0/00484.html
>>
>> OK, thanks.
>>
>> > > but I think the discussion there petered out more than anything
>> > > else.
>>
>> > Maybe it did :)
>>
>> I think so looking at the archives.
>>
>> > > Or was it the suggestion to make this something that the driver core
>> > > supported so that we didn't have to open code it for every bus?
>>
>> > Probably. That's something I really haven't took the time to look at,
>> > and don't really plan on doing so.
>>
>> > I guess a good way forward would be to revive this patch, and wait for
>> > that generic way to happen.
>>
>> > What do you think of this?
>>
>> Probably best, the Pi bootloader does make it a bit more important.
>> Might also be worth speaking to Greg though.
>
> So, do you want me to resend that patch and discuss this directly
> there (with Greg in Cc) ?

My general idea is to get overlay loading to work and then make spidev
bind to all CS which are not taken by anything and unbind when an
overlay tries to take over the CS. Since there are some overlay
loading patches available that part can be considered solved but I did
not get to looking at the dynamic spidev binding.

For now I use your patch with additional patch that marks the spidev
devices with a flag so they are not checked when it is determined if
the CS is in use.

Thanks

Michal

2015-05-12 16:06:44

by Mark Brown

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

On Tue, May 12, 2015 at 04:27:59PM +0200, Maxime Ripard wrote:
> On Mon, Apr 27, 2015 at 07:07:28PM +0100, Mark Brown wrote:

> > Probably best, the Pi bootloader does make it a bit more important.
> > Might also be worth speaking to Greg though.

> So, do you want me to resend that patch and discuss this directly
> there (with Greg in Cc) ?

Sounds like a good first step.


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