2012-10-31 17:25:01

by Pantelis Antoniou

[permalink] [raw]
Subject: [PATCH 0/3] capebus moving omap_devices to mach-omap2

It is painless to move the adapter DT devices to arch/arm/mach-omap2

However I got bit by the __init at omap_build_device family functions.
If you don't remove it, crashes every time you instantiate a device
at runtime, or you load the cape driver as a module.

Pantelis Antoniou (3):
omap-device: Remove __init from omap_device_build family functions
da8xx-dt: Create da8xx DT adapter device
ti-tscadc-dt: Create ti-tscadc-dt DT adapter device

arch/arm/mach-omap2/Makefile | 4 +
arch/arm/mach-omap2/da8xx-dt.c | 197 +++++++++++++++++++++++++++++++++++++
arch/arm/mach-omap2/ti-tscadc-dt.c | 155 +++++++++++++++++++++++++++++
arch/arm/plat-omap/omap_device.c | 4 +-
4 files changed, 358 insertions(+), 2 deletions(-)
create mode 100644 arch/arm/mach-omap2/da8xx-dt.c
create mode 100644 arch/arm/mach-omap2/ti-tscadc-dt.c

--
1.7.12


2012-10-31 17:52:24

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

* Pantelis Antoniou <[email protected]> [121031 10:26]:
> It is painless to move the adapter DT devices to arch/arm/mach-omap2
>
> However I got bit by the __init at omap_build_device family functions.
> If you don't remove it, crashes every time you instantiate a device
> at runtime, or you load the cape driver as a module.

Hmm I think you misunderstood me. You only need to create the
platform_device under arch/arm/mach-omap2. The device creation
happens only at __init, so omap_build_device can stay as __init.
The driver itself should be under drivers.

But is this bus on non-device-tree omaps? If not, just make it
device tree only.

Regards,

Tony

2012-10-31 18:04:20

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Tony,

On Oct 31, 2012, at 7:52 PM, Tony Lindgren wrote:

> * Pantelis Antoniou <[email protected]> [121031 10:26]:
>> It is painless to move the adapter DT devices to arch/arm/mach-omap2
>>
>> However I got bit by the __init at omap_build_device family functions.
>> If you don't remove it, crashes every time you instantiate a device
>> at runtime, or you load the cape driver as a module.
>
> Hmm I think you misunderstood me. You only need to create the
> platform_device under arch/arm/mach-omap2. The device creation
> happens only at __init, so omap_build_device can stay as __init.
> The driver itself should be under drivers.
>
> But is this bus on non-device-tree omaps? If not, just make it
> device tree only.
>

I'm afraid that's not the case. The whole notion of capebus is that
instantiation of the devices doesn't just happen early at the boot
sequence.

It is perfectly valid for a cape to be instantiated via loading
a module, or by making an override by writing a sysfs file.

When having the __init there, the function has been long removed
and you get a crash by calling into the weeds.

So the sequence is:

<early boot> Register the adapter driver.

<user> insmod bone-geiger-cape
<cape-is-matched> call omap_build_device

Please look into the capebus patches for the details.

> Regards,
>
> Tony

Regards

-- Pantelis

2012-10-31 18:09:41

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

* Pantelis Antoniou <[email protected]> [121031 11:05]:
> Hi Tony,
>
> On Oct 31, 2012, at 7:52 PM, Tony Lindgren wrote:
>
> > * Pantelis Antoniou <[email protected]> [121031 10:26]:
> >> It is painless to move the adapter DT devices to arch/arm/mach-omap2
> >>
> >> However I got bit by the __init at omap_build_device family functions.
> >> If you don't remove it, crashes every time you instantiate a device
> >> at runtime, or you load the cape driver as a module.
> >
> > Hmm I think you misunderstood me. You only need to create the
> > platform_device under arch/arm/mach-omap2. The device creation
> > happens only at __init, so omap_build_device can stay as __init.
> > The driver itself should be under drivers.
> >
> > But is this bus on non-device-tree omaps? If not, just make it
> > device tree only.
> >
>
> I'm afraid that's not the case. The whole notion of capebus is that
> instantiation of the devices doesn't just happen early at the boot
> sequence.
>
> It is perfectly valid for a cape to be instantiated via loading
> a module, or by making an override by writing a sysfs file.
>
> When having the __init there, the function has been long removed
> and you get a crash by calling into the weeds.
>
> So the sequence is:
>
> <early boot> Register the adapter driver.

OK this is always there for the hardware, and done during
the __init and this one should have an omap_device..

> <user> insmod bone-geiger-cape
> <cape-is-matched> call omap_build_device
>
> Please look into the capebus patches for the details.

..but it seems that the devices connected to capebus should
not have anything to do with omap_device and hwmod?

They should only need to use capebus, just like USB devices
use the USB bus and are registered as USB devices.

Anyways, please sort it out with Benoit, Kevin and Paul,
I've added them to cc. Maybe I'm missing something here.

Regards,

Tony

2012-10-31 18:25:03

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2


On Oct 31, 2012, at 8:09 PM, Tony Lindgren wrote:

> * Pantelis Antoniou <[email protected]> [121031 11:05]:
>> Hi Tony,
>>
>> On Oct 31, 2012, at 7:52 PM, Tony Lindgren wrote:
>>
>>> * Pantelis Antoniou <[email protected]> [121031 10:26]:
>>>> It is painless to move the adapter DT devices to arch/arm/mach-omap2
>>>>
>>>> However I got bit by the __init at omap_build_device family functions.
>>>> If you don't remove it, crashes every time you instantiate a device
>>>> at runtime, or you load the cape driver as a module.
>>>
>>> Hmm I think you misunderstood me. You only need to create the
>>> platform_device under arch/arm/mach-omap2. The device creation
>>> happens only at __init, so omap_build_device can stay as __init.
>>> The driver itself should be under drivers.
>>>
>>> But is this bus on non-device-tree omaps? If not, just make it
>>> device tree only.
>>>
>>
>> I'm afraid that's not the case. The whole notion of capebus is that
>> instantiation of the devices doesn't just happen early at the boot
>> sequence.
>>
>> It is perfectly valid for a cape to be instantiated via loading
>> a module, or by making an override by writing a sysfs file.
>>
>> When having the __init there, the function has been long removed
>> and you get a crash by calling into the weeds.
>>
>> So the sequence is:
>>
>> <early boot> Register the adapter driver.
>
> OK this is always there for the hardware, and done during
> the __init and this one should have an omap_device..
>

It is there at this point in time. The __init section
will go away after the boot is complete.

So you have this (as an example da8xx-dt):

1. During the boot sequence (postcore_initcall) da8xx_dt_init()
is called. It registers the platform device driver (da8xx_dt_driver).
The probe method is _not_ called yet since a matching device is
_not_ yet in the DT. The matching device is found in a number of
cape definitions but none have been activated.

2. capebus probes the capes and enumerates using the EEPROM header
(for the beaglebone case). But a matching cape driver is not yet
been loaded.

3. At this point booting ends, and the __init sections are discarded.

4. User space starts and insmod's the cape driver.

5. Capebus now has match and calls the cape driver's probe method.

6. Cape driver, locates matching da8xx_dt in the cape DT definition,
and eventuall will call device_add. device_add will eventually
call da8xx-dt's probe method.

7. da8xx-dt probe method will retrieve the hwmod (all OK) and it
will call omap_device_build (which it is in an __init section)
and is now freed.

8. Crash.


>> <user> insmod bone-geiger-cape
>> <cape-is-matched> call omap_build_device
>>
>> Please look into the capebus patches for the details.
>
> ..but it seems that the devices connected to capebus should
> not have anything to do with omap_device and hwmod?
>
> They should only need to use capebus, just like USB devices
> use the USB bus and are registered as USB devices.
>
> Anyways, please sort it out with Benoit, Kevin and Paul,
> I've added them to cc. Maybe I'm missing something here.
>
> Regards,
>
> Tony


Regards

-- Pantelis

2012-10-31 19:55:27

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Panto,

On 10/31/2012 07:09 PM, Tony Lindgren wrote:
> * Pantelis Antoniou <[email protected]> [121031 11:05]:
>> Hi Tony,
>>
>> On Oct 31, 2012, at 7:52 PM, Tony Lindgren wrote:
>>
>>> * Pantelis Antoniou <[email protected]> [121031 10:26]:
>>>> It is painless to move the adapter DT devices to arch/arm/mach-omap2
>>>>
>>>> However I got bit by the __init at omap_build_device family functions.
>>>> If you don't remove it, crashes every time you instantiate a device
>>>> at runtime, or you load the cape driver as a module.
>>>
>>> Hmm I think you misunderstood me. You only need to create the
>>> platform_device under arch/arm/mach-omap2. The device creation
>>> happens only at __init, so omap_build_device can stay as __init.
>>> The driver itself should be under drivers.
>>>
>>> But is this bus on non-device-tree omaps? If not, just make it
>>> device tree only.
>>>
>>
>> I'm afraid that's not the case. The whole notion of capebus is that
>> instantiation of the devices doesn't just happen early at the boot
>> sequence.
>>
>> It is perfectly valid for a cape to be instantiated via loading
>> a module, or by making an override by writing a sysfs file.
>>
>> When having the __init there, the function has been long removed
>> and you get a crash by calling into the weeds.
>>
>> So the sequence is:
>>
>> <early boot> Register the adapter driver.
>
> OK this is always there for the hardware, and done during
> the __init and this one should have an omap_device..
>
>> <user> insmod bone-geiger-cape
>> <cape-is-matched> call omap_build_device
>>
>> Please look into the capebus patches for the details.
>
> ..but it seems that the devices connected to capebus should
> not have anything to do with omap_device and hwmod?


Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
could have an hwmod and thus must be handled by an omap_device.

Any devices that is created later cannot be omap_device. The DT core
will create regular platform_device for them.

Since cape is an external board, it should have nothing to do with
omap_device.

Looking at your patch:
da8xx-dt: Create da8xx DT adapter device

I understand why you do that, but in fact that patch does not make sense
to me :-(

Why do you have to create an omap_device from the driver probe?


Regards,
Benoit

2012-10-31 20:12:52

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Benoit,

On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:

> Hi Panto,
>
> On 10/31/2012 07:09 PM, Tony Lindgren wrote:
>> * Pantelis Antoniou <[email protected]> [121031 11:05]:
>>> Hi Tony,
>>>
>>> On Oct 31, 2012, at 7:52 PM, Tony Lindgren wrote:
>>>
>>>> * Pantelis Antoniou <[email protected]> [121031 10:26]:
>>>>> It is painless to move the adapter DT devices to arch/arm/mach-omap2
>>>>>
>>>>> However I got bit by the __init at omap_build_device family functions.
>>>>> If you don't remove it, crashes every time you instantiate a device
>>>>> at runtime, or you load the cape driver as a module.
>>>>
>>>> Hmm I think you misunderstood me. You only need to create the
>>>> platform_device under arch/arm/mach-omap2. The device creation
>>>> happens only at __init, so omap_build_device can stay as __init.
>>>> The driver itself should be under drivers.
>>>>
>>>> But is this bus on non-device-tree omaps? If not, just make it
>>>> device tree only.
>>>>
>>>
>>> I'm afraid that's not the case. The whole notion of capebus is that
>>> instantiation of the devices doesn't just happen early at the boot
>>> sequence.
>>>
>>> It is perfectly valid for a cape to be instantiated via loading
>>> a module, or by making an override by writing a sysfs file.
>>>
>>> When having the __init there, the function has been long removed
>>> and you get a crash by calling into the weeds.
>>>
>>> So the sequence is:
>>>
>>> <early boot> Register the adapter driver.
>>
>> OK this is always there for the hardware, and done during
>> the __init and this one should have an omap_device..
>>
>>> <user> insmod bone-geiger-cape
>>> <cape-is-matched> call omap_build_device
>>>
>>> Please look into the capebus patches for the details.
>>
>> ..but it seems that the devices connected to capebus should
>> not have anything to do with omap_device and hwmod?
>
>
> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
> could have an hwmod and thus must be handled by an omap_device.
>
> Any devices that is created later cannot be omap_device. The DT core
> will create regular platform_device for them.
>
> Since cape is an external board, it should have nothing to do with
> omap_device.
>
> Looking at your patch:
> da8xx-dt: Create da8xx DT adapter device
>
> I understand why you do that, but in fact that patch does not make sense
> to me :-(
>
> Why do you have to create an omap_device from the driver probe?
>

The problem is that capes are not external boards in the normal sense
as a PCI board is. In the PCI case the hardware that implements the
desired functionality is on the PCI board, while in the cape case the
hardware module is in the SoC. The cape most of the times is quite
simple and contains passive components, leds or some kind of I2C/SPI devices.

In the da8xx case it contains only the minimum hardware required for
interfacing to the required display type, be it an LCD or a DVI or something
other.

You can't instantiate the omap_device early in the boot process like it was done up to
now in the board file. You can only do that later in the boot process (for built-in
cape drivers), or even after user-space has booted and the matching cape driver module
has been loaded.

So this is a use case that hasn't been encountered yet I'm afraid, but it is a valid
use case.

I don't see what the big deal is about to be honest. The omap device is safely created
in the da8xx-dt adapter's probe method, and everything seems to work.

If there are any gotcha's or problems with this approach I think can be overcome.

>
> Regards,
> Benoit
>

Regards

-- Pantelis

2012-10-31 21:26:44

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

* Pantelis Antoniou <[email protected]> [121031 13:14]:
> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
> >
> > Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
> > could have an hwmod and thus must be handled by an omap_device.
> >
> > Any devices that is created later cannot be omap_device. The DT core
> > will create regular platform_device for them.
> >
> > Since cape is an external board, it should have nothing to do with
> > omap_device.
> >
> > Looking at your patch:
> > da8xx-dt: Create da8xx DT adapter device
> >
> > I understand why you do that, but in fact that patch does not make sense
> > to me :-(
> >
> > Why do you have to create an omap_device from the driver probe?
> >
>
> The problem is that capes are not external boards in the normal sense
> as a PCI board is. In the PCI case the hardware that implements the
> desired functionality is on the PCI board, while in the cape case the
> hardware module is in the SoC. The cape most of the times is quite
> simple and contains passive components, leds or some kind of I2C/SPI devices.

Ah now I see, you're talking about the beaglebone extension
boards :)

The way to deal with those properly is to just edit the
board .dts entry to include omap-beaglebone-cape-xyz.dtsi
whatever.

> You can't instantiate the omap_device early in the boot process like it was done up to
> now in the board file. You can only do that later in the boot process (for built-in
> cape drivers), or even after user-space has booted and the matching cape driver module
> has been loaded.

Yes you can, just edit the .dts file for the extension
board you have connected. This stuff should be DT
only for sure, let's not even start adding platform_data
entries for that.

The omap_device entries for the omap internal devices
will be created automatically during startup based on the
.dts. Note that you can set status = "disabled" for the
omap internal devices in the .dts file, the devices are
there in the hardware.

If you are sure the extension boards are safely hot
pluggable, then all you need is some interface to enable
and disable devices after booting. But that should be
done in Linux generic way.

So we should not export any omap_hwmod or omap_device
things, and want to keep it __init only, and local to
arch/arm/mach-omap2.

Regards,

Tony

2012-10-31 21:36:33

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Tony,

On Oct 31, 2012, at 11:26 PM, Tony Lindgren wrote:

> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>
>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>> could have an hwmod and thus must be handled by an omap_device.
>>>
>>> Any devices that is created later cannot be omap_device. The DT core
>>> will create regular platform_device for them.
>>>
>>> Since cape is an external board, it should have nothing to do with
>>> omap_device.
>>>
>>> Looking at your patch:
>>> da8xx-dt: Create da8xx DT adapter device
>>>
>>> I understand why you do that, but in fact that patch does not make sense
>>> to me :-(
>>>
>>> Why do you have to create an omap_device from the driver probe?
>>>
>>
>> The problem is that capes are not external boards in the normal sense
>> as a PCI board is. In the PCI case the hardware that implements the
>> desired functionality is on the PCI board, while in the cape case the
>> hardware module is in the SoC. The cape most of the times is quite
>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>
> Ah now I see, you're talking about the beaglebone extension
> boards :)
>
> The way to deal with those properly is to just edit the
> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
> whatever.
>

That is what is being done...
This is the fallout.

>> You can't instantiate the omap_device early in the boot process like it was done up to
>> now in the board file. You can only do that later in the boot process (for built-in
>> cape drivers), or even after user-space has booted and the matching cape driver module
>> has been loaded.
>
> Yes you can, just edit the .dts file for the extension
> board you have connected. This stuff should be DT
> only for sure, let's not even start adding platform_data
> entries for that.
>
> The omap_device entries for the omap internal devices
> will be created automatically during startup based on the
> .dts. Note that you can set status = "disabled" for the
> omap internal devices in the .dts file, the devices are
> there in the hardware.
>
> If you are sure the extension boards are safely hot
> pluggable, then all you need is some interface to enable
> and disable devices after booting. But that should be
> done in Linux generic way.
>
> So we should not export any omap_hwmod or omap_device
> things, and want to keep it __init only, and local to
> arch/arm/mach-omap2.
>

I disagree. This is not something that is handled by just
editing the dts. The way it is handled, is in the Linux
generic way, we have a proper bus, and the drivers as compiled
as standalone file.

We're hitting a use case that wasn't there in omap until now.

There a a whole bunch of conflicting capes. There's no
way to instantiate them together. They must be instantiated
only after their EEPROMs are read and they are matched
to their corresponding cape drivers.

We don't have a board file anymore where to do i. This is all on
board-generic, getting ready for the one kernel fits all for ARM.

Everything is local to arch/arm/mach-omap2, the only
problem is that __init modifier that's keeping us from
loading the cape drivers as modules.

If you take some time and scan the full patchset you will
understand why this is the way it is.

> Regards,
>
> Tony

Regards

-- Pantelis

2012-10-31 21:43:17

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

* Pantelis Antoniou <[email protected]> [121031 14:38]:
>
> There a a whole bunch of conflicting capes. There's no
> way to instantiate them together. They must be instantiated
> only after their EEPROMs are read and they are matched
> to their corresponding cape drivers.

You don't need to instantiate the capes during __init,
you need to just instantiate the omap internal devices
which are always there in the hardware. These internal
devices just need to be set to state = "disabled" until
they are used.

The capes themselves should not have anything to do
with omap_hwmod or omap_device, they just contain
external connectors, regulators, LCD panels etc.

To your capebus the omap internal devices should look
like just regular struct device entries.

Regards,

Tony

2012-10-31 22:00:53

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Tony,

On Oct 31, 2012, at 11:43 PM, Tony Lindgren wrote:

> * Pantelis Antoniou <[email protected]> [121031 14:38]:
>>
>> There a a whole bunch of conflicting capes. There's no
>> way to instantiate them together. They must be instantiated
>> only after their EEPROMs are read and they are matched
>> to their corresponding cape drivers.
>
> You don't need to instantiate the capes during __init,
> you need to just instantiate the omap internal devices
> which are always there in the hardware. These internal
> devices just need to be set to state = "disabled" until
> they are used.
>

If you set the device's node state to disable the device
doesn't get registered; i.e. the device's probe method will not be
called.

The devices are instantiated by the call to

of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);

in omap_generic_init() (arch/arm/mach-omap2/board-generic.c)

Which for each child of the root node will call of_platform_bus_create
which in turn will call of_platform_device_create_pdata.

First check in there is

if (!of_device_is_available(np))
return NULL;

of_device_is_available() will return 0 in any case status is neither
"okay" or "ok".

So when device's node is 'disabled' of_platform_device_create_pdata()
will not create the device.

Now, of course it is possible to re-trigger the platform's probe method
to be called, and in fact I do so in the capebus patches.

Since the bus drives of things like i2c & spi must be disabled if not
used by any cape, they are "disabled" by default, and are activated when
referenced by a cape.

In the file drivers/capebus/capebus-driver.c, there are two functions.

The first, capebus_of_platform_device_enable() will check if the referenced
device node is disabled, and after enabling it (with capebus_of_device_node_enable())
will call of_platform_device_create(), but now this time the instantiation
will be successful.

The problem is that now, we're way past the point in the boot sequence
that omap_build_device has been freed; calling the probe method will now lead
to a crash.

> The capes themselves should not have anything to do
> with omap_hwmod or omap_device, they just contain
> external connectors, regulators, LCD panels etc.
>
> To your capebus the omap internal devices should look
> like just regular struct device entries.
>
> Regards,
>
> Tony

Regards

-- Pantelis

2012-10-31 22:16:15

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

* Pantelis Antoniou <[email protected]> [121031 15:02]:
>
> So when device's node is 'disabled' of_platform_device_create_pdata()
> will not create the device.
>
> Now, of course it is possible to re-trigger the platform's probe method
> to be called, and in fact I do so in the capebus patches.

You should fix this in generic way then rather than working
around it in capebus. The same problem exists changing
between different functionality for the shared pins,
let's say between USB pins and UART pins if you want a
serial debug console on some phone.

Regards,

Tony

2012-10-31 22:20:04

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi,

On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
> > * Pantelis Antoniou <[email protected]> [121031 13:14]:
> >> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
> >>>
> >>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
> >>> could have an hwmod and thus must be handled by an omap_device.
> >>>
> >>> Any devices that is created later cannot be omap_device. The DT core
> >>> will create regular platform_device for them.
> >>>
> >>> Since cape is an external board, it should have nothing to do with
> >>> omap_device.
> >>>
> >>> Looking at your patch:
> >>> da8xx-dt: Create da8xx DT adapter device
> >>>
> >>> I understand why you do that, but in fact that patch does not make sense
> >>> to me :-(
> >>>
> >>> Why do you have to create an omap_device from the driver probe?
> >>>
> >>
> >> The problem is that capes are not external boards in the normal sense
> >> as a PCI board is. In the PCI case the hardware that implements the
> >> desired functionality is on the PCI board, while in the cape case the
> >> hardware module is in the SoC. The cape most of the times is quite
> >> simple and contains passive components, leds or some kind of I2C/SPI devices.
> >
> > Ah now I see, you're talking about the beaglebone extension
> > boards :)
> >
> > The way to deal with those properly is to just edit the
> > board .dts entry to include omap-beaglebone-cape-xyz.dtsi
> > whatever.
> >
>
> That is what is being done...
> This is the fallout.
>
> >> You can't instantiate the omap_device early in the boot process like it was done up to
> >> now in the board file. You can only do that later in the boot process (for built-in
> >> cape drivers), or even after user-space has booted and the matching cape driver module
> >> has been loaded.
> >
> > Yes you can, just edit the .dts file for the extension
> > board you have connected. This stuff should be DT
> > only for sure, let's not even start adding platform_data
> > entries for that.
> >
> > The omap_device entries for the omap internal devices
> > will be created automatically during startup based on the
> > .dts. Note that you can set status = "disabled" for the
> > omap internal devices in the .dts file, the devices are
> > there in the hardware.
> >
> > If you are sure the extension boards are safely hot
> > pluggable, then all you need is some interface to enable
> > and disable devices after booting. But that should be
> > done in Linux generic way.
> >
> > So we should not export any omap_hwmod or omap_device
> > things, and want to keep it __init only, and local to
> > arch/arm/mach-omap2.
> >
>
> I disagree. This is not something that is handled by just
> editing the dts. The way it is handled, is in the Linux
> generic way, we have a proper bus, and the drivers as compiled
> as standalone file.

when you have an actual bus, that'd be correct.

> We're hitting a use case that wasn't there in omap until now.
>
> There a a whole bunch of conflicting capes. There's no
> way to instantiate them together. They must be instantiated
> only after their EEPROMs are read and they are matched
> to their corresponding cape drivers.

why this requirement of instantiating them only after reading EEPROMs ?

It's unnecessary to add that requirement, just do what Tony said
(include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
should be created automatically.

The thing is that there is no such thing as a cape-device. The cape is
just a collection of I2C, 1-wire and SPI devices anyway. What we should
instantiate is bmp085, tls2550, sht21, instead of instantiating
beagle-bone-weather-cape.

What's the problem in just instantiating all of those from bone.dts ?
Expose the EEPROMs to userland so whatever SW you guys have running on
the bone can figure out what features to expose to the SDK which the
user sees, but the kernel doesn't need to know that there is a
weather-cape attached to the bone.

--
balbi


Attachments:
(No filename) (3.91 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-10-31 17:25:04

by Pantelis Antoniou

[permalink] [raw]
Subject: [PATCH 1/3] omap-device: Remove __init from omap_device_build family functions

Marking omap_device_build && omap_device_build_ss as __init is
really bad when you want to instantiate a device later in the
boot sequence, or from a module.

Removing them makes the crashes go away.

Signed-off-by: Pantelis Antoniou <[email protected]>
---
arch/arm/plat-omap/omap_device.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index 7a7d1f2..cf9a18d 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -663,7 +663,7 @@ void omap_device_delete(struct omap_device *od)
* information. Returns ERR_PTR(-EINVAL) if @oh is NULL; otherwise,
* passes along the return value of omap_device_build_ss().
*/
-struct platform_device __init *omap_device_build(const char *pdev_name, int pdev_id,
+struct platform_device *omap_device_build(const char *pdev_name, int pdev_id,
struct omap_hwmod *oh, void *pdata,
int pdata_len,
struct omap_device_pm_latency *pm_lats,
@@ -696,7 +696,7 @@ struct platform_device __init *omap_device_build(const char *pdev_name, int pdev
* platform_device record. Returns an ERR_PTR() on error, or passes
* along the return value of omap_device_register().
*/
-struct platform_device __init *omap_device_build_ss(const char *pdev_name, int pdev_id,
+struct platform_device *omap_device_build_ss(const char *pdev_name, int pdev_id,
struct omap_hwmod **ohs, int oh_cnt,
void *pdata, int pdata_len,
struct omap_device_pm_latency *pm_lats,
--
1.7.12

2012-10-31 17:25:08

by Pantelis Antoniou

[permalink] [raw]
Subject: [PATCH 3/3] ti-tscadc-dt: Create ti-tscadc-dt DT adapter device

omap_device is going private.

Move the ti-tscadc-dt adapter device to arch/arm/mach-omap2.

Signed-off-by: Pantelis Antoniou <[email protected]>
---
arch/arm/mach-omap2/Makefile | 1 +
arch/arm/mach-omap2/ti-tscadc-dt.c | 155 +++++++++++++++++++++++++++++++++++++
2 files changed, 156 insertions(+)
create mode 100644 arch/arm/mach-omap2/ti-tscadc-dt.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 3a9a0ff..1eab37b 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -202,6 +202,7 @@ endif

# AM3XX Device Tree adapters for legacy drivers
obj-$(CONFIG_SOC_AM33XX) += da8xx-dt.o
+obj-$(CONFIG_SOC_AM33XX) += ti-tscadc-dt.o

# Specific board support
obj-$(CONFIG_MACH_OMAP_GENERIC) += board-generic.o
diff --git a/arch/arm/mach-omap2/ti-tscadc-dt.c b/arch/arm/mach-omap2/ti-tscadc-dt.c
new file mode 100644
index 0000000..0a1a17a
--- /dev/null
+++ b/arch/arm/mach-omap2/ti-tscadc-dt.c
@@ -0,0 +1,155 @@
+/*
+ * TI-TSCADC-DT: Device tree adapter using the legacy driver
+ *
+ * Copyright (C) 2012 Pantelis Antoniou <[email protected]>
+ * Copyright (C) 2012 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/clk.h>
+#include <linux/input/ti_am335x_tsc.h>
+#include <linux/platform_data/ti_am335x_adc.h>
+#include <linux/mfd/ti_am335x_tscadc.h>
+#include <plat/clock.h>
+#include <plat/omap_device.h>
+
+struct ti_tscadc_priv {
+ struct omap_hwmod *tsc_oh;
+ struct tsc_data tsc_data;
+ struct adc_data adc_data;
+ struct mfd_tscadc_board tscadc_data;
+ struct platform_device *tscadc_pdev;
+};
+
+static const struct of_device_id of_ti_tscadc_dt_match[] = {
+ { .compatible = "ti-tscadc-dt", },
+ {},
+};
+
+static int __devinit ti_tscadc_dt_probe(struct platform_device *pdev)
+{
+ struct ti_tscadc_priv *priv;
+ struct pinctrl *pinctrl;
+ u32 val;
+ int ret;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (priv == NULL) {
+ dev_err(&pdev->dev, "Failed to allocate priv\n");
+ return -ENOMEM;
+ }
+
+ pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+ if (IS_ERR(pinctrl))
+ dev_warn(&pdev->dev,
+ "pins are not configured from the driver\n");
+
+ ret = of_property_read_u32(pdev->dev.of_node, "tsc-wires", &val);
+ if (ret != 0) {
+ dev_info(&pdev->dev, "no tsc-wires property; disabling TSC\n");
+ val = 0;
+ }
+ priv->tsc_data.wires = val;
+
+ if (priv->tsc_data.wires > 0) {
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "tsc-x-plate-resistance", &val);
+ if (ret != 0) {
+ dev_err(&pdev->dev, "Failed to read "
+ "tsc-x-plate-resistance property\n");
+ return ret;
+ }
+ priv->tsc_data.x_plate_resistance = val;
+
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "tsc-steps", &val);
+ if (ret != 0) {
+ dev_err(&pdev->dev, "Failed to read "
+ "tsc-steps property\n");
+ return ret;
+ }
+ priv->tsc_data.steps_to_configure = val;
+ }
+
+ ret = of_property_read_u32(pdev->dev.of_node, "adc-channels", &val);
+ if (ret != 0) {
+ dev_info(&pdev->dev, "No adc-channels property; "
+ "disabling adc\n");
+ val = 0;
+ }
+ priv->adc_data.adc_channels = val;
+
+ priv->tscadc_data.tsc_init = &priv->tsc_data;
+ priv->tscadc_data.adc_init = &priv->adc_data;
+
+ priv->tsc_oh = omap_hwmod_lookup("adc_tsc");
+ if (priv->tsc_oh == NULL) {
+ dev_err(&pdev->dev, "Could not lookup HWMOD %s\n", "adc_tsc");
+ return -ENODEV;
+ }
+
+ priv->tscadc_pdev = omap_device_build("ti_tscadc", -1, priv->tsc_oh,
+ &priv->tscadc_data, sizeof(priv->tscadc_data),
+ NULL, 0, 0);
+ if (priv->tscadc_pdev == NULL) {
+ dev_err(&pdev->dev, "Could not create tsc_adc device\n");
+ return -ENODEV;
+ }
+
+ dev_info(&pdev->dev, "TI tscadc pdev created OK\n");
+
+ platform_set_drvdata(pdev, priv);
+
+ return 0;
+}
+
+static int __devexit ti_tscadc_dt_remove(struct platform_device *pdev)
+{
+ return -EINVAL; /* not supporting removal yet */
+}
+
+static struct platform_driver ti_tscadc_dt_driver = {
+ .probe = ti_tscadc_dt_probe,
+ .remove = __devexit_p(ti_tscadc_dt_remove),
+ .driver = {
+ .name = "ti_tscadc-dt",
+ .owner = THIS_MODULE,
+ .of_match_table = of_ti_tscadc_dt_match,
+ },
+};
+
+static __init int ti_tscadc_dt_init(void)
+{
+ return platform_driver_register(&ti_tscadc_dt_driver);
+}
+
+static __exit void ti_tscadc_dt_exit(void)
+{
+ platform_driver_unregister(&ti_tscadc_dt_driver);
+}
+
+postcore_initcall(ti_tscadc_dt_init);
+module_exit(ti_tscadc_dt_exit);
--
1.7.12

2012-10-31 17:25:55

by Pantelis Antoniou

[permalink] [raw]
Subject: [PATCH 2/3] da8xx-dt: Create da8xx DT adapter device

omap_device is going private.

Move the da8xx-dt adapter device to arch/arm/mach-omap2.

Signed-off-by: Pantelis Antoniou <[email protected]>
---
arch/arm/mach-omap2/Makefile | 3 +
arch/arm/mach-omap2/da8xx-dt.c | 197 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 200 insertions(+)
create mode 100644 arch/arm/mach-omap2/da8xx-dt.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index fe40d9e..3a9a0ff 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -200,6 +200,9 @@ ifneq ($(CONFIG_DRM_OMAP),)
obj-y += drm.o
endif

+# AM3XX Device Tree adapters for legacy drivers
+obj-$(CONFIG_SOC_AM33XX) += da8xx-dt.o
+
# Specific board support
obj-$(CONFIG_MACH_OMAP_GENERIC) += board-generic.o
obj-$(CONFIG_MACH_OMAP_H4) += board-h4.o
diff --git a/arch/arm/mach-omap2/da8xx-dt.c b/arch/arm/mach-omap2/da8xx-dt.c
new file mode 100644
index 0000000..d3c6f48
--- /dev/null
+++ b/arch/arm/mach-omap2/da8xx-dt.c
@@ -0,0 +1,197 @@
+/*
+ * DA8XX-DT: Device tree adapter using the legacy driver
+ *
+ * Copyright (C) 2012 Pantelis Antoniou <[email protected]>
+ * Copyright (C) 2012 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <video/da8xx-fb.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/clk.h>
+#include <plat/clock.h>
+#include <plat/omap_device.h>
+
+struct da8xx_priv {
+ struct da8xx_lcdc_platform_data lcd_pdata;
+ struct lcd_ctrl_config lcd_cfg;
+ struct display_panel lcd_panel;
+ struct platform_device *lcdc_pdev;
+ struct omap_hwmod *lcdc_oh;
+ struct resource lcdc_res[1];
+ int power_dn_gpio;
+};
+
+static const struct of_device_id of_da8xx_dt_match[] = {
+ { .compatible = "da8xx-dt", },
+ {},
+};
+
+static int __devinit da8xx_dt_probe(struct platform_device *pdev)
+{
+ struct da8xx_priv *priv;
+ struct clk *disp_pll;
+ struct pinctrl *pinctrl;
+ u32 disp_pll_val;
+ const char *panel_type;
+ int ret = -EINVAL;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (priv == NULL) {
+ dev_err(&pdev->dev, "Failed to allocate priv\n");
+ return -ENOMEM;
+ }
+ priv->power_dn_gpio = -1;
+
+ pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+ if (IS_ERR(pinctrl))
+ dev_warn(&pdev->dev,
+ "pins are not configured from the driver\n");
+
+ ret = of_property_read_u32(pdev->dev.of_node, "disp-pll",
+ &disp_pll_val);
+ if (ret != 0) {
+ dev_err(&pdev->dev, "Failed to read disp-pll property\n");
+ return ret;
+ }
+
+ ret = of_property_read_string(pdev->dev.of_node, "panel-type",
+ &panel_type);
+ if (ret != 0) {
+ dev_err(&pdev->dev, "Failed to read panel-type property\n");
+ return ret;
+ }
+
+ /* conf_disp_pll(disp_pll); */
+ disp_pll = clk_get(NULL, "dpll_disp_ck");
+ if (IS_ERR(disp_pll)) {
+ dev_err(&pdev->dev, "Cannot clk_get disp_pll\n");
+ return PTR_ERR(disp_pll);
+ }
+ ret = clk_set_rate(disp_pll, disp_pll_val);
+ clk_put(disp_pll);
+ if (ret != 0) {
+ dev_err(&pdev->dev, "Failed to set disp_pll\n");
+ return ret;
+ }
+
+ ret = of_get_named_gpio_flags(pdev->dev.of_node, "powerdn-gpio",
+ 0, NULL);
+ if (IS_ERR_VALUE(ret)) {
+ dev_info(&pdev->dev, "No power down GPIO\n");
+ } else {
+ priv->power_dn_gpio = ret;
+
+ ret = devm_gpio_request(&pdev->dev, priv->power_dn_gpio,
+ "da8xx-dt:PDN");
+ if (ret != 0) {
+ dev_err(&pdev->dev, "Failed to gpio_request\n");
+ return ret;
+ }
+
+ ret = gpio_direction_output(priv->power_dn_gpio, 1);
+ if (ret != 0) {
+ dev_err(&pdev->dev, "Failed to set powerdn to 1\n");
+ return ret;
+ }
+ }
+
+ /* display_panel */
+ priv->lcd_panel.panel_type = QVGA;
+ priv->lcd_panel.max_bpp = 16;
+ priv->lcd_panel.min_bpp = 16;
+ priv->lcd_panel.panel_shade = COLOR_ACTIVE;
+
+ /* lcd_ctrl_config */
+ priv->lcd_cfg.p_disp_panel = &priv->lcd_panel;
+ priv->lcd_cfg.ac_bias = 255;
+ priv->lcd_cfg.ac_bias_intrpt = 0;
+ priv->lcd_cfg.dma_burst_sz = 16;
+ priv->lcd_cfg.bpp = 16;
+ priv->lcd_cfg.fdd = 0x80;
+ priv->lcd_cfg.tft_alt_mode = 0;
+ priv->lcd_cfg.stn_565_mode = 0;
+ priv->lcd_cfg.mono_8bit_mode = 0;
+ priv->lcd_cfg.invert_line_clock = 1;
+ priv->lcd_cfg.invert_frm_clock = 1;
+ priv->lcd_cfg.sync_edge = 0;
+ priv->lcd_cfg.sync_ctrl = 1;
+ priv->lcd_cfg.raster_order = 0;
+
+ /* da8xx_lcdc_platform_data */
+ strcpy(priv->lcd_pdata.manu_name, "BBToys");
+ priv->lcd_pdata.controller_data = &priv->lcd_cfg;
+ strcpy(priv->lcd_pdata.type, panel_type);
+
+ priv->lcdc_oh = omap_hwmod_lookup("lcdc");
+ if (priv->lcdc_oh == NULL) {
+ dev_err(&pdev->dev, "Failed to lookup omap_hwmod lcdc\n");
+ return -ENODEV;
+ }
+
+ priv->lcdc_pdev = omap_device_build("da8xx_lcdc", 0, priv->lcdc_oh,
+ &priv->lcd_pdata,
+ sizeof(struct da8xx_lcdc_platform_data),
+ NULL, 0, 0);
+ if (priv->lcdc_pdev == NULL) {
+ dev_err(&pdev->dev, "Failed to build LCDC device\n");
+ return -ENODEV;
+ }
+
+ dev_info(&pdev->dev, "Registered bone LCDC OK.\n");
+
+ platform_set_drvdata(pdev, priv);
+
+ return 0;
+}
+
+static int __devexit da8xx_dt_remove(struct platform_device *pdev)
+{
+ return -EINVAL; /* not supporting removal yet */
+}
+
+static struct platform_driver da8xx_dt_driver = {
+ .probe = da8xx_dt_probe,
+ .remove = __devexit_p(da8xx_dt_remove),
+ .driver = {
+ .name = "da8xx-dt",
+ .owner = THIS_MODULE,
+ .of_match_table = of_da8xx_dt_match,
+ },
+};
+
+static __init int da8xx_dt_init(void)
+{
+ return platform_driver_register(&da8xx_dt_driver);
+}
+
+static __exit void da8xx_dt_exit(void)
+{
+ platform_driver_unregister(&da8xx_dt_driver);
+}
+
+postcore_initcall(da8xx_dt_init);
+module_exit(da8xx_dt_exit);
--
1.7.12

2012-11-01 07:02:25

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Felibe,

On Nov 1, 2012, at 12:14 AM, Felipe Balbi wrote:

> Hi,
>
> On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
>>> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>>>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>>>
>>>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>>>> could have an hwmod and thus must be handled by an omap_device.
>>>>>
>>>>> Any devices that is created later cannot be omap_device. The DT core
>>>>> will create regular platform_device for them.
>>>>>
>>>>> Since cape is an external board, it should have nothing to do with
>>>>> omap_device.
>>>>>
>>>>> Looking at your patch:
>>>>> da8xx-dt: Create da8xx DT adapter device
>>>>>
>>>>> I understand why you do that, but in fact that patch does not make sense
>>>>> to me :-(
>>>>>
>>>>> Why do you have to create an omap_device from the driver probe?
>>>>>
>>>>
>>>> The problem is that capes are not external boards in the normal sense
>>>> as a PCI board is. In the PCI case the hardware that implements the
>>>> desired functionality is on the PCI board, while in the cape case the
>>>> hardware module is in the SoC. The cape most of the times is quite
>>>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>>>
>>> Ah now I see, you're talking about the beaglebone extension
>>> boards :)
>>>
>>> The way to deal with those properly is to just edit the
>>> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
>>> whatever.
>>>
>>
>> That is what is being done...
>> This is the fallout.
>>
>>>> You can't instantiate the omap_device early in the boot process like it was done up to
>>>> now in the board file. You can only do that later in the boot process (for built-in
>>>> cape drivers), or even after user-space has booted and the matching cape driver module
>>>> has been loaded.
>>>
>>> Yes you can, just edit the .dts file for the extension
>>> board you have connected. This stuff should be DT
>>> only for sure, let's not even start adding platform_data
>>> entries for that.
>>>
>>> The omap_device entries for the omap internal devices
>>> will be created automatically during startup based on the
>>> .dts. Note that you can set status = "disabled" for the
>>> omap internal devices in the .dts file, the devices are
>>> there in the hardware.
>>>
>>> If you are sure the extension boards are safely hot
>>> pluggable, then all you need is some interface to enable
>>> and disable devices after booting. But that should be
>>> done in Linux generic way.
>>>
>>> So we should not export any omap_hwmod or omap_device
>>> things, and want to keep it __init only, and local to
>>> arch/arm/mach-omap2.
>>>
>>
>> I disagree. This is not something that is handled by just
>> editing the dts. The way it is handled, is in the Linux
>> generic way, we have a proper bus, and the drivers as compiled
>> as standalone file.
>
> when you have an actual bus, that'd be correct.

What do you think capebus is? It is an actual bus that allows you
to do so.

>
>> We're hitting a use case that wasn't there in omap until now.
>>
>> There a a whole bunch of conflicting capes. There's no
>> way to instantiate them together. They must be instantiated
>> only after their EEPROMs are read and they are matched
>> to their corresponding cape drivers.
>
> why this requirement of instantiating them only after reading EEPROMs ?
>
> It's unnecessary to add that requirement, just do what Tony said
> (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
> should be created automatically.
>
> The thing is that there is no such thing as a cape-device. The cape is
> just a collection of I2C, 1-wire and SPI devices anyway. What we should
> instantiate is bmp085, tls2550, sht21, instead of instantiating
> beagle-bone-weather-cape.
>
> What's the problem in just instantiating all of those from bone.dts ?
> Expose the EEPROMs to userland so whatever SW you guys have running on
> the bone can figure out what features to expose to the SDK which the
> user sees, but the kernel doesn't need to know that there is a
> weather-cape attached to the bone.
>

The I2C/SPI devices are not a problem at all.

Let me try to explain what the problem is, and why it all this
is needed.

First of all, capes may be relatively simple boards, which may function
as a generic device - like the generic capes. They might not necessarily
be simple. Some capes, for example the geiger cape have a number of
components that perform a specific task; in this instance the driving
circuit of the geiger tube and the event detector input. Other capes
that are being designed now are even more complex, i.e. there's a
radar cape, an fpga cape and so on. So for these kind of boards
you will need a specific driver. That driver will probably use some
generic-cape bits (like gpios, pwms, keys what-ever).
But it will put them to use in the custom in-kernel driver in it's own
way. You can't put that task to user-space, if it's ever slightly complex.

So for really simple capes, after considerable pain you might, just
might, dump the problem to user-space and try to make it work.
People have tried that and it's a total pain. There is no way that this will
work with anything more complex than a generic cape.

Then there's the out of the box experience problem.

What a customer that buys one of these boards along with a number of
capes wants is to plug them, and have them work. You cannot have
the DTS file activating all the drivers for each possible cape since
they conflict; the pins that one SPI bus that one cape uses might be a
PWM output for another, and so on.

Asking the customer to edit the distro supplied kernel's dts and recompiling,
while figuring out how to solve conflicts is not going to fly.

We have the versioning problem. All the standard capes go through
a very fast evolution processes. So we now have 3 versions of a DVI cape,
2 of a small LCD cape and so on. We don't want to abandon the capes that
are already sold and are out on the field, but we don't want to spend
too much time hacking the driver to support all the different version.
The version capability of capebus handles that; taking as an example the
DVI cape in which the power down GPIO moved around:

> version@00A0 {
> version = "00A0";
> dvi {
> compatible = "da8xx-dt";
> pinctrl-names = "default";
> pinctrl-0 = <&bone_dvi_cape_dvi_00A0_pins>;
> ti,hwmods = "lcdc";
>
> disp-pll = <560000000>;
> panel-type = "1024x768@60";
> powerdn-gpio = <&gpio2 7 0>;
> };
> };
>
> version@00A1 {
> version = "00A1", "01";
> dvi {
> compatible = "da8xx-dt";
> pinctrl-names = "default";
> pinctrl-0 = <&bone_dvi_cape_dvi_00A1_pins>;
> ti,hwmods = "lcdc";
>
> disp-pll = <560000000>;
> panel-type = "1024x768@60";
> powerdn-gpio = <&gpio2 31 0>;
> };
> };
>
>

Finally, we have the target user-base problem. The users are all going to
be hobbyists, persons that buy beaglebones as a step up from Arduino. They
will want to hack their own kind of hardware; they might not even have
an EEPROM in their custom boards. They need some kind of runtime selection
of the cape they have defined, these is handled by the case of overrides.

For example, the Adafruit 1.8 Display is pretty popular, but it's not a cape.
You have to wire it yourself and connect it to the expansion header. Then
there's two different kinds of the display. It also has a PWM input for
backlight control.

Making use of it is simple with capebus:

# cd /sys/devices/capebus.10
# echo "0:Adafruit 1.8 Cape" >slots

And that's it. The disable spi & pwm device the display needs will
be turned on, the pinmux it needs will be set, and the spi device
that the display uses will be created.

I have no clue how this can be simpler using any kind of user-space
method. And no, you can't have this active in the DTS of the common
kernel for all beaglebones.

> &bone_adafruit_cape {
> board-name = "Adafruit 1.8 Cape";
>
> backlight {
> compatible = "pwm-backlight";
> pinctrl-names = "default";
> pinctrl-0 = <&pwm_bl_pins>;
>
> pwms = <&ehrpwm1 1 500000 0>;
> pwm-names = "st7735fb";
> brightness-levels = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36
> default-brightness-level = <50>; /* index to the array above */
> };
>
> spi1-devices {
> compatible = "spi-dt";
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> parent = <&spi1>;
>
> lcd@0 {
> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
> spi-max-frequency = <8000000>;
> reg = <0>;
> spi-cpol;
> spi-cpha;
> pinctrl-names = "default";
> pinctrl-0 = <&lcd_pins>;
> st7735-rst = <&gpio4 19 0>;
> st7735-dc = <&gpio4 21 0>;
> };
>
> };
> };
>


> --
> balbi

Regards

-- Pantelis

2012-11-01 10:23:40

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On 11/1/2012 8:02 AM, Pantelis Antoniou wrote:
> Hi Felibe,
>
> On Nov 1, 2012, at 12:14 AM, Felipe Balbi wrote:
>
>> Hi,
>>
>> On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
>>>> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>>>>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>>>>
>>>>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>>>>> could have an hwmod and thus must be handled by an omap_device.
>>>>>>
>>>>>> Any devices that is created later cannot be omap_device. The DT core
>>>>>> will create regular platform_device for them.
>>>>>>
>>>>>> Since cape is an external board, it should have nothing to do with
>>>>>> omap_device.
>>>>>>
>>>>>> Looking at your patch:
>>>>>> da8xx-dt: Create da8xx DT adapter device
>>>>>>
>>>>>> I understand why you do that, but in fact that patch does not make sense
>>>>>> to me :-(
>>>>>>
>>>>>> Why do you have to create an omap_device from the driver probe?
>>>>>>
>>>>>
>>>>> The problem is that capes are not external boards in the normal sense
>>>>> as a PCI board is. In the PCI case the hardware that implements the
>>>>> desired functionality is on the PCI board, while in the cape case the
>>>>> hardware module is in the SoC. The cape most of the times is quite
>>>>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>>>>
>>>> Ah now I see, you're talking about the beaglebone extension
>>>> boards :)
>>>>
>>>> The way to deal with those properly is to just edit the
>>>> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
>>>> whatever.
>>>>
>>>
>>> That is what is being done...
>>> This is the fallout.
>>>
>>>>> You can't instantiate the omap_device early in the boot process like it was done up to
>>>>> now in the board file. You can only do that later in the boot process (for built-in
>>>>> cape drivers), or even after user-space has booted and the matching cape driver module
>>>>> has been loaded.
>>>>
>>>> Yes you can, just edit the .dts file for the extension
>>>> board you have connected. This stuff should be DT
>>>> only for sure, let's not even start adding platform_data
>>>> entries for that.
>>>>
>>>> The omap_device entries for the omap internal devices
>>>> will be created automatically during startup based on the
>>>> .dts. Note that you can set status = "disabled" for the
>>>> omap internal devices in the .dts file, the devices are
>>>> there in the hardware.
>>>>
>>>> If you are sure the extension boards are safely hot
>>>> pluggable, then all you need is some interface to enable
>>>> and disable devices after booting. But that should be
>>>> done in Linux generic way.
>>>>
>>>> So we should not export any omap_hwmod or omap_device
>>>> things, and want to keep it __init only, and local to
>>>> arch/arm/mach-omap2.
>>>>
>>>
>>> I disagree. This is not something that is handled by just
>>> editing the dts. The way it is handled, is in the Linux
>>> generic way, we have a proper bus, and the drivers as compiled
>>> as standalone file.
>>
>> when you have an actual bus, that'd be correct.
>
> What do you think capebus is? It is an actual bus that allows you
> to do so.
>
>>
>>> We're hitting a use case that wasn't there in omap until now.
>>>
>>> There a a whole bunch of conflicting capes. There's no
>>> way to instantiate them together. They must be instantiated
>>> only after their EEPROMs are read and they are matched
>>> to their corresponding cape drivers.
>>
>> why this requirement of instantiating them only after reading EEPROMs ?
>>
>> It's unnecessary to add that requirement, just do what Tony said
>> (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
>> should be created automatically.
>>
>> The thing is that there is no such thing as a cape-device. The cape is
>> just a collection of I2C, 1-wire and SPI devices anyway. What we should
>> instantiate is bmp085, tls2550, sht21, instead of instantiating
>> beagle-bone-weather-cape.
>>
>> What's the problem in just instantiating all of those from bone.dts ?
>> Expose the EEPROMs to userland so whatever SW you guys have running on
>> the bone can figure out what features to expose to the SDK which the
>> user sees, but the kernel doesn't need to know that there is a
>> weather-cape attached to the bone.
>>
>
> The I2C/SPI devices are not a problem at all.
>
> Let me try to explain what the problem is, and why it all this
> is needed.
>
> First of all, capes may be relatively simple boards, which may function
> as a generic device - like the generic capes. They might not necessarily
> be simple. Some capes, for example the geiger cape have a number of
> components that perform a specific task; in this instance the driving
> circuit of the geiger tube and the event detector input. Other capes
> that are being designed now are even more complex, i.e. there's a
> radar cape, an fpga cape and so on. So for these kind of boards
> you will need a specific driver. That driver will probably use some
> generic-cape bits (like gpios, pwms, keys what-ever).
> But it will put them to use in the custom in-kernel driver in it's own
> way. You can't put that task to user-space, if it's ever slightly complex.
>
> So for really simple capes, after considerable pain you might, just
> might, dump the problem to user-space and try to make it work.
> People have tried that and it's a total pain. There is no way that this will
> work with anything more complex than a generic cape.
>
> Then there's the out of the box experience problem.
>
> What a customer that buys one of these boards along with a number of
> capes wants is to plug them, and have them work. You cannot have
> the DTS file activating all the drivers for each possible cape since
> they conflict; the pins that one SPI bus that one cape uses might be a
> PWM output for another, and so on.
>
> Asking the customer to edit the distro supplied kernel's dts and recompiling,
> while figuring out how to solve conflicts is not going to fly.

Well you do not have to rebuild the kernel if you just want to update
the DTS.
The way DT is done, the best you can do is to rely on the bootloader in
your case to select the proper DTB. You should try to merge dynamically
AMxx + beaglebone + capeXXX based on some EEPROM id.

FWIW, we do have a similar, but simpler, problem with the beagle /
beagle-xm and panda / panda-es. But for the moment we are just using a
different DTS for each board.

> We have the versioning problem. All the standard capes go through
> a very fast evolution processes. So we now have 3 versions of a DVI cape,
> 2 of a small LCD cape and so on. We don't want to abandon the capes that
> are already sold and are out on the field, but we don't want to spend
> too much time hacking the driver to support all the different version.
> The version capability of capebus handles that; taking as an example the
> DVI cape in which the power down GPIO moved around:
>
>> version@00A0 {
>> version = "00A0";
>> dvi {
>> compatible = "da8xx-dt";
>> pinctrl-names = "default";
>> pinctrl-0 = <&bone_dvi_cape_dvi_00A0_pins>;
>> ti,hwmods = "lcdc";

That part is still weird to me. Assuming the lcdc is the Display
controller, the DVI should be a child of the lcdc not the opposite.

>> disp-pll = <560000000>;
>> panel-type = "1024x768@60";
>> powerdn-gpio = <&gpio2 7 0>;
>> };
>> };
>>
>> version@00A1 {
>> version = "00A1", "01";
>> dvi {
>> compatible = "da8xx-dt";
>> pinctrl-names = "default";
>> pinctrl-0 = <&bone_dvi_cape_dvi_00A1_pins>;
>> ti,hwmods = "lcdc";
>>
>> disp-pll = <560000000>;
>> panel-type = "1024x768@60";
>> powerdn-gpio = <&gpio2 31 0>;
>> };
>> };
>>
>>
>
> Finally, we have the target user-base problem. The users are all going to
> be hobbyists, persons that buy beaglebones as a step up from Arduino. They
> will want to hack their own kind of hardware; they might not even have
> an EEPROM in their custom boards. They need some kind of runtime selection
> of the cape they have defined, these is handled by the case of overrides.
>
> For example, the Adafruit 1.8 Display is pretty popular, but it's not a cape.
> You have to wire it yourself and connect it to the expansion header. Then
> there's two different kinds of the display. It also has a PWM input for
> backlight control.
>
> Making use of it is simple with capebus:
>
> # cd /sys/devices/capebus.10
> # echo "0:Adafruit 1.8 Cape" >slots
>
> And that's it. The disable spi & pwm device the display needs will
> be turned on, the pinmux it needs will be set, and the spi device
> that the display uses will be created.
>
> I have no clue how this can be simpler using any kind of user-space
> method. And no, you can't have this active in the DTS of the common
> kernel for all beaglebones.
>
>> &bone_adafruit_cape {
>> board-name = "Adafruit 1.8 Cape";
>>
>> backlight {
>> compatible = "pwm-backlight";
>> pinctrl-names = "default";
>> pinctrl-0 = <&pwm_bl_pins>;
>>
>> pwms = <&ehrpwm1 1 500000 0>;
>> pwm-names = "st7735fb";
>> brightness-levels = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36
>> default-brightness-level = <50>; /* index to the array above */
>> };
>>
>> spi1-devices {
>> compatible = "spi-dt";
>>
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> parent = <&spi1>;

Why do you need that? You seems to completely break the whole DT
semantic with tons of customs bindings.

From the driver code, it looks like the cape bus is not a real bus at
all. If this is just a connector, then the I2C devices will be children
of the AM33xx I2C controller, but there is no intermediate HW in the middle.

>>
>> lcd@0 {
>> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
>> spi-max-frequency = <8000000>;
>> reg = <0>;
>> spi-cpol;
>> spi-cpha;
>> pinctrl-names = "default";
>> pinctrl-0 = <&lcd_pins>;
>> st7735-rst = <&gpio4 19 0>;
>> st7735-dc = <&gpio4 21 0>;
>> };
>>
>> };
>> };
>>

I guess there is no easy solution for that, but it looks to me that what
you have to do is to make the DT creation dynamic in your case. Assuming
you do not want to do that in the bootloader, you must do that pretty
early during the boot process to end up with a full description of your
DT tree before creating the devices.

Each cape will have their own DTS and based on some board id you will
fix the DT dynamically.

My point is that the issue you are facing is a real limitation of DT, so
you should fix the DT core and not workaround it by creating artificial
bindings / drivers.

Regards,
Benoit

2012-11-01 10:39:38

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Benoit,

On Nov 1, 2012, at 12:23 PM, Cousson, Benoit wrote:

> On 11/1/2012 8:02 AM, Pantelis Antoniou wrote:
>> Hi Felibe,
>>
>> On Nov 1, 2012, at 12:14 AM, Felipe Balbi wrote:
>>
>>> Hi,
>>>
>>> On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
>>>>> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>>>>>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>>>>>
>>>>>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>>>>>> could have an hwmod and thus must be handled by an omap_device.
>>>>>>>
>>>>>>> Any devices that is created later cannot be omap_device. The DT core
>>>>>>> will create regular platform_device for them.
>>>>>>>
>>>>>>> Since cape is an external board, it should have nothing to do with
>>>>>>> omap_device.
>>>>>>>
>>>>>>> Looking at your patch:
>>>>>>> da8xx-dt: Create da8xx DT adapter device
>>>>>>>
>>>>>>> I understand why you do that, but in fact that patch does not make sense
>>>>>>> to me :-(
>>>>>>>
>>>>>>> Why do you have to create an omap_device from the driver probe?
>>>>>>>
>>>>>>
>>>>>> The problem is that capes are not external boards in the normal sense
>>>>>> as a PCI board is. In the PCI case the hardware that implements the
>>>>>> desired functionality is on the PCI board, while in the cape case the
>>>>>> hardware module is in the SoC. The cape most of the times is quite
>>>>>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>>>>>
>>>>> Ah now I see, you're talking about the beaglebone extension
>>>>> boards :)
>>>>>
>>>>> The way to deal with those properly is to just edit the
>>>>> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
>>>>> whatever.
>>>>>
>>>>
>>>> That is what is being done...
>>>> This is the fallout.
>>>>
>>>>>> You can't instantiate the omap_device early in the boot process like it was done up to
>>>>>> now in the board file. You can only do that later in the boot process (for built-in
>>>>>> cape drivers), or even after user-space has booted and the matching cape driver module
>>>>>> has been loaded.
>>>>>
>>>>> Yes you can, just edit the .dts file for the extension
>>>>> board you have connected. This stuff should be DT
>>>>> only for sure, let's not even start adding platform_data
>>>>> entries for that.
>>>>>
>>>>> The omap_device entries for the omap internal devices
>>>>> will be created automatically during startup based on the
>>>>> .dts. Note that you can set status = "disabled" for the
>>>>> omap internal devices in the .dts file, the devices are
>>>>> there in the hardware.
>>>>>
>>>>> If you are sure the extension boards are safely hot
>>>>> pluggable, then all you need is some interface to enable
>>>>> and disable devices after booting. But that should be
>>>>> done in Linux generic way.
>>>>>
>>>>> So we should not export any omap_hwmod or omap_device
>>>>> things, and want to keep it __init only, and local to
>>>>> arch/arm/mach-omap2.
>>>>>
>>>>
>>>> I disagree. This is not something that is handled by just
>>>> editing the dts. The way it is handled, is in the Linux
>>>> generic way, we have a proper bus, and the drivers as compiled
>>>> as standalone file.
>>>
>>> when you have an actual bus, that'd be correct.
>>
>> What do you think capebus is? It is an actual bus that allows you
>> to do so.
>>
>>>
>>>> We're hitting a use case that wasn't there in omap until now.
>>>>
>>>> There a a whole bunch of conflicting capes. There's no
>>>> way to instantiate them together. They must be instantiated
>>>> only after their EEPROMs are read and they are matched
>>>> to their corresponding cape drivers.
>>>
>>> why this requirement of instantiating them only after reading EEPROMs ?
>>>
>>> It's unnecessary to add that requirement, just do what Tony said
>>> (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
>>> should be created automatically.
>>>
>>> The thing is that there is no such thing as a cape-device. The cape is
>>> just a collection of I2C, 1-wire and SPI devices anyway. What we should
>>> instantiate is bmp085, tls2550, sht21, instead of instantiating
>>> beagle-bone-weather-cape.
>>>
>>> What's the problem in just instantiating all of those from bone.dts ?
>>> Expose the EEPROMs to userland so whatever SW you guys have running on
>>> the bone can figure out what features to expose to the SDK which the
>>> user sees, but the kernel doesn't need to know that there is a
>>> weather-cape attached to the bone.
>>>
>>
>> The I2C/SPI devices are not a problem at all.
>>
>> Let me try to explain what the problem is, and why it all this
>> is needed.
>>
>> First of all, capes may be relatively simple boards, which may function
>> as a generic device - like the generic capes. They might not necessarily
>> be simple. Some capes, for example the geiger cape have a number of
>> components that perform a specific task; in this instance the driving
>> circuit of the geiger tube and the event detector input. Other capes
>> that are being designed now are even more complex, i.e. there's a
>> radar cape, an fpga cape and so on. So for these kind of boards
>> you will need a specific driver. That driver will probably use some
>> generic-cape bits (like gpios, pwms, keys what-ever).
>> But it will put them to use in the custom in-kernel driver in it's own
>> way. You can't put that task to user-space, if it's ever slightly complex.
>>
>> So for really simple capes, after considerable pain you might, just
>> might, dump the problem to user-space and try to make it work.
>> People have tried that and it's a total pain. There is no way that this will
>> work with anything more complex than a generic cape.
>>
>> Then there's the out of the box experience problem.
>>
>> What a customer that buys one of these boards along with a number of
>> capes wants is to plug them, and have them work. You cannot have
>> the DTS file activating all the drivers for each possible cape since
>> they conflict; the pins that one SPI bus that one cape uses might be a
>> PWM output for another, and so on.
>>
>> Asking the customer to edit the distro supplied kernel's dts and recompiling,
>> while figuring out how to solve conflicts is not going to fly.
>
> Well you do not have to rebuild the kernel if you just want to update the DTS.
> The way DT is done, the best you can do is to rely on the bootloader in your case to select the proper DTB. You should try to merge dynamically AMxx + beaglebone + capeXXX based on some EEPROM id.
>

That is what the capebus does. It inserts

> FWIW, we do have a similar, but simpler, problem with the beagle / beagle-xm and panda / panda-es. But for the moment we are just using a different DTS for each board.

A different DTS for each board combination... There alot more capes for the bone that they are for the beagle & the panda.
And more are going to come, but not necessarily from people that have any connection to TI or CCO.

You still haven't described how I could solve the issue of conflicting capes using a single DTS.


>
>> We have the versioning problem. All the standard capes go through
>> a very fast evolution processes. So we now have 3 versions of a DVI cape,
>> 2 of a small LCD cape and so on. We don't want to abandon the capes that
>> are already sold and are out on the field, but we don't want to spend
>> too much time hacking the driver to support all the different version.
>> The version capability of capebus handles that; taking as an example the
>> DVI cape in which the power down GPIO moved around:
>>
>>> version@00A0 {
>>> version = "00A0";
>>> dvi {
>>> compatible = "da8xx-dt";
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&bone_dvi_cape_dvi_00A0_pins>;
>>> ti,hwmods = "lcdc";
>
> That part is still weird to me. Assuming the lcdc is the Display controller, the DVI should be a child of the lcdc not the opposite.
>

At the time of this there are no DT bindings yet for the da8xx driver.

>>> disp-pll = <560000000>;
>>> panel-type = "1024x768@60";
>>> powerdn-gpio = <&gpio2 7 0>;
>>> };
>>> };
>>>
>>> version@00A1 {
>>> version = "00A1", "01";
>>> dvi {
>>> compatible = "da8xx-dt";
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&bone_dvi_cape_dvi_00A1_pins>;
>>> ti,hwmods = "lcdc";
>>>
>>> disp-pll = <560000000>;
>>> panel-type = "1024x768@60";
>>> powerdn-gpio = <&gpio2 31 0>;
>>> };
>>> };
>>>
>>>
>>
>> Finally, we have the target user-base problem. The users are all going to
>> be hobbyists, persons that buy beaglebones as a step up from Arduino. They
>> will want to hack their own kind of hardware; they might not even have
>> an EEPROM in their custom boards. They need some kind of runtime selection
>> of the cape they have defined, these is handled by the case of overrides.
>>
>> For example, the Adafruit 1.8 Display is pretty popular, but it's not a cape.
>> You have to wire it yourself and connect it to the expansion header. Then
>> there's two different kinds of the display. It also has a PWM input for
>> backlight control.
>>
>> Making use of it is simple with capebus:
>>
>> # cd /sys/devices/capebus.10
>> # echo "0:Adafruit 1.8 Cape" >slots
>>
>> And that's it. The disable spi & pwm device the display needs will
>> be turned on, the pinmux it needs will be set, and the spi device
>> that the display uses will be created.
>>
>> I have no clue how this can be simpler using any kind of user-space
>> method. And no, you can't have this active in the DTS of the common
>> kernel for all beaglebones.
>>
>>> &bone_adafruit_cape {
>>> board-name = "Adafruit 1.8 Cape";
>>>
>>> backlight {
>>> compatible = "pwm-backlight";
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&pwm_bl_pins>;
>>>
>>> pwms = <&ehrpwm1 1 500000 0>;
>>> pwm-names = "st7735fb";
>>> brightness-levels = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36
>>> default-brightness-level = <50>; /* index to the array above */
>>> };
>>>
>>> spi1-devices {
>>> compatible = "spi-dt";
>>>
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> parent = <&spi1>;
>
> Why do you need that? You seems to completely break the whole DT semantic with tons of customs bindings.
>
> From the driver code, it looks like the cape bus is not a real bus at all. If this is just a connector, then the I2C devices will be children of the AM33xx I2C controller, but there is no intermediate HW in the middle.

No it is not that way. One set of capes can be just simple connector capes. Other capes are full blown devices with their
own drivers. What do you propose as a fix is for the complex kind of capes?

>
>>>
>>> lcd@0 {
>>> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
>>> spi-max-frequency = <8000000>;
>>> reg = <0>;
>>> spi-cpol;
>>> spi-cpha;
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&lcd_pins>;
>>> st7735-rst = <&gpio4 19 0>;
>>> st7735-dc = <&gpio4 21 0>;
>>> };
>>>
>>> };
>>> };
>>>
>
> I guess there is no easy solution for that, but it looks to me that what you have to do is to make the DT creation dynamic in your case. Assuming you do not want to do that in the bootloader, you must do that pretty early during the boot process to end up with a full description of your DT tree before creating the devices.
>

Do it pretty early in the boot processes ended up with the am335xevm board file in the PSP tree.

The whole set of possible cape designs cannot be controlled, nor do we want to.
We want to empower users to come up with their own designs without having to do any kernel/boot loader
hacking.

> Each cape will have their own DTS and based on some board id you will fix the DT dynamically.
>
> My point is that the issue you are facing is a real limitation of DT, so you should fix the DT core and not workaround it by creating artificial bindings / drivers.
>

You still haven't described any mechanism to deal with all the use cases I described.

DT can't and will not deal with the complexity that we're facing right now.

This is a relatively clean way to fix it; we can actually ship a mainline kernel that works.


> Regards,
> Benoit
>
>

Regards

-- Pantelis

2012-11-01 11:10:19

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi,

On Thu, Nov 01, 2012 at 12:39:30PM +0200, Pantelis Antoniou wrote:
> >>> lcd@0 {
> >>> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
> >>> spi-max-frequency = <8000000>;
> >>> reg = <0>;
> >>> spi-cpol;
> >>> spi-cpha;
> >>> pinctrl-names = "default";
> >>> pinctrl-0 = <&lcd_pins>;
> >>> st7735-rst = <&gpio4 19 0>;
> >>> st7735-dc = <&gpio4 21 0>;
> >>> };
> >>>
> >>> };
> >>> };
> >>>
> >
> > I guess there is no easy solution for that, but it looks to me that
> > what you have to do is to make the DT creation dynamic in your case.
> > Assuming you do not want to do that in the bootloader, you must do
> > that pretty early during the boot process to end up with a full
> > description of your DT tree before creating the devices.
> >
>
> Do it pretty early in the boot processes ended up with the am335xevm board file in the PSP tree.
>
> The whole set of possible cape designs cannot be controlled, nor do we want to.
> We want to empower users to come up with their own designs without having to do any kernel/boot loader
> hacking.

that's impossible since you will have to provide the capebus driver
anyway.

> > Each cape will have their own DTS and based on some board id you
> > will fix the DT dynamically.
> >
> > My point is that the issue you are facing is a real limitation of
> > DT, so you should fix the DT core and not workaround it by creating
> > artificial bindings / drivers.
> >
>
> You still haven't described any mechanism to deal with all the use
> cases I described.
>
> DT can't and will not deal with the complexity that we're facing right
> now.

and DT-itself shouldn't. I agree with Benoit that this should be built
at bootloader level, perhaps. Whatever you're doing in capebus, you
could do at kernel space, build your DT bindings in runtime, and pass
that DT blob to kernel.

One question though, what do you mean by "some capes are full blown
devices with their own drivers" ? Do you mean you have capes running
some other (RT)OS and communicating with linux somehow ? How does it
communicate to the bone ?

cheers

--
balbi


Attachments:
(No filename) (2.30 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-11-01 11:26:09

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Panto,

On 11/1/2012 11:39 AM, Pantelis Antoniou wrote:
> Hi Benoit,
>
> On Nov 1, 2012, at 12:23 PM, Cousson, Benoit wrote:
>
>> On 11/1/2012 8:02 AM, Pantelis Antoniou wrote:
>>> Hi Felibe,
>>>
>>> On Nov 1, 2012, at 12:14 AM, Felipe Balbi wrote:
>>>
>>>> Hi,
>>>>
>>>> On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
>>>>>> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>>>>>>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>>>>>>
>>>>>>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>>>>>>> could have an hwmod and thus must be handled by an omap_device.
>>>>>>>>
>>>>>>>> Any devices that is created later cannot be omap_device. The DT core
>>>>>>>> will create regular platform_device for them.
>>>>>>>>
>>>>>>>> Since cape is an external board, it should have nothing to do with
>>>>>>>> omap_device.
>>>>>>>>
>>>>>>>> Looking at your patch:
>>>>>>>> da8xx-dt: Create da8xx DT adapter device
>>>>>>>>
>>>>>>>> I understand why you do that, but in fact that patch does not make sense
>>>>>>>> to me :-(
>>>>>>>>
>>>>>>>> Why do you have to create an omap_device from the driver probe?
>>>>>>>>
>>>>>>>
>>>>>>> The problem is that capes are not external boards in the normal sense
>>>>>>> as a PCI board is. In the PCI case the hardware that implements the
>>>>>>> desired functionality is on the PCI board, while in the cape case the
>>>>>>> hardware module is in the SoC. The cape most of the times is quite
>>>>>>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>>>>>>
>>>>>> Ah now I see, you're talking about the beaglebone extension
>>>>>> boards :)
>>>>>>
>>>>>> The way to deal with those properly is to just edit the
>>>>>> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
>>>>>> whatever.
>>>>>>
>>>>>
>>>>> That is what is being done...
>>>>> This is the fallout.
>>>>>
>>>>>>> You can't instantiate the omap_device early in the boot process like it was done up to
>>>>>>> now in the board file. You can only do that later in the boot process (for built-in
>>>>>>> cape drivers), or even after user-space has booted and the matching cape driver module
>>>>>>> has been loaded.
>>>>>>
>>>>>> Yes you can, just edit the .dts file for the extension
>>>>>> board you have connected. This stuff should be DT
>>>>>> only for sure, let's not even start adding platform_data
>>>>>> entries for that.
>>>>>>
>>>>>> The omap_device entries for the omap internal devices
>>>>>> will be created automatically during startup based on the
>>>>>> .dts. Note that you can set status = "disabled" for the
>>>>>> omap internal devices in the .dts file, the devices are
>>>>>> there in the hardware.
>>>>>>
>>>>>> If you are sure the extension boards are safely hot
>>>>>> pluggable, then all you need is some interface to enable
>>>>>> and disable devices after booting. But that should be
>>>>>> done in Linux generic way.
>>>>>>
>>>>>> So we should not export any omap_hwmod or omap_device
>>>>>> things, and want to keep it __init only, and local to
>>>>>> arch/arm/mach-omap2.
>>>>>>
>>>>>
>>>>> I disagree. This is not something that is handled by just
>>>>> editing the dts. The way it is handled, is in the Linux
>>>>> generic way, we have a proper bus, and the drivers as compiled
>>>>> as standalone file.
>>>>
>>>> when you have an actual bus, that'd be correct.
>>>
>>> What do you think capebus is? It is an actual bus that allows you
>>> to do so.
>>>
>>>>
>>>>> We're hitting a use case that wasn't there in omap until now.
>>>>>
>>>>> There a a whole bunch of conflicting capes. There's no
>>>>> way to instantiate them together. They must be instantiated
>>>>> only after their EEPROMs are read and they are matched
>>>>> to their corresponding cape drivers.
>>>>
>>>> why this requirement of instantiating them only after reading EEPROMs ?
>>>>
>>>> It's unnecessary to add that requirement, just do what Tony said
>>>> (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
>>>> should be created automatically.
>>>>
>>>> The thing is that there is no such thing as a cape-device. The cape is
>>>> just a collection of I2C, 1-wire and SPI devices anyway. What we should
>>>> instantiate is bmp085, tls2550, sht21, instead of instantiating
>>>> beagle-bone-weather-cape.
>>>>
>>>> What's the problem in just instantiating all of those from bone.dts ?
>>>> Expose the EEPROMs to userland so whatever SW you guys have running on
>>>> the bone can figure out what features to expose to the SDK which the
>>>> user sees, but the kernel doesn't need to know that there is a
>>>> weather-cape attached to the bone.
>>>>
>>>
>>> The I2C/SPI devices are not a problem at all.
>>>
>>> Let me try to explain what the problem is, and why it all this
>>> is needed.
>>>
>>> First of all, capes may be relatively simple boards, which may function
>>> as a generic device - like the generic capes. They might not necessarily
>>> be simple. Some capes, for example the geiger cape have a number of
>>> components that perform a specific task; in this instance the driving
>>> circuit of the geiger tube and the event detector input. Other capes
>>> that are being designed now are even more complex, i.e. there's a
>>> radar cape, an fpga cape and so on. So for these kind of boards
>>> you will need a specific driver. That driver will probably use some
>>> generic-cape bits (like gpios, pwms, keys what-ever).
>>> But it will put them to use in the custom in-kernel driver in it's own
>>> way. You can't put that task to user-space, if it's ever slightly complex.
>>>
>>> So for really simple capes, after considerable pain you might, just
>>> might, dump the problem to user-space and try to make it work.
>>> People have tried that and it's a total pain. There is no way that this will
>>> work with anything more complex than a generic cape.
>>>
>>> Then there's the out of the box experience problem.
>>>
>>> What a customer that buys one of these boards along with a number of
>>> capes wants is to plug them, and have them work. You cannot have
>>> the DTS file activating all the drivers for each possible cape since
>>> they conflict; the pins that one SPI bus that one cape uses might be a
>>> PWM output for another, and so on.
>>>
>>> Asking the customer to edit the distro supplied kernel's dts and recompiling,
>>> while figuring out how to solve conflicts is not going to fly.
>>
>> Well you do not have to rebuild the kernel if you just want to update the DTS.
>> The way DT is done, the best you can do is to rely on the bootloader in your case to select the proper DTB. You should try to merge dynamically AMxx + beaglebone + capeXXX based on some EEPROM id.
>>
>
> That is what the capebus does. It inserts

It inserts what?

>> FWIW, we do have a similar, but simpler, problem with the beagle / beagle-xm and panda / panda-es. But for the moment we are just using a different DTS for each board.
>
> A different DTS for each board combination... There alot more capes for the bone that they are for the beagle & the panda.
> And more are going to come, but not necessarily from people that have any connection to TI or CCO.

Sure, but my point is that your solution will not solve our problem :-)
This is a generic problem that you address with a very custom / specific
approach.

> You still haven't described how I could solve the issue of conflicting capes using a single DTS.

Well I don't have the solution otherwise I will have done it already :-)
My point is that the solution has to be in the DT core if not in the
bootloader.
We can add some new binding to handle revision. And then we should be
able during DT tree device creation to remove nodes that does not match
the version criteria.

In the case of panda/beagle, some GPIO lines are used to identify the
board version.

So we should be able to define some generic way to export revision and
use that to select the proper nodes.

This is not necessarily different to what your are doing with your
capebus. The point is to build a generic way / binding to do that properly.

>>> We have the versioning problem. All the standard capes go through
>>> a very fast evolution processes. So we now have 3 versions of a DVI cape,
>>> 2 of a small LCD cape and so on. We don't want to abandon the capes that
>>> are already sold and are out on the field, but we don't want to spend
>>> too much time hacking the driver to support all the different version.
>>> The version capability of capebus handles that; taking as an example the
>>> DVI cape in which the power down GPIO moved around:
>>>
>>>> version@00A0 {
>>>> version = "00A0";
>>>> dvi {
>>>> compatible = "da8xx-dt";
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&bone_dvi_cape_dvi_00A0_pins>;
>>>> ti,hwmods = "lcdc";
>>
>> That part is still weird to me. Assuming the lcdc is the Display controller, the DVI should be a child of the lcdc not the opposite.
>>
>
> At the time of this there are no DT bindings yet for the da8xx driver.

OK, but that does not answer my question...

I don't know this HW, but the LCD controller should be the parent in
that case.

lcdc@48000000 {
ti,hwmods = "lcdc";
reg = <0x48000000>;

dvi {
disp-pll = <>;
panel-type = "1024x560000000768@60";
powerdn-gpio = <&gpio2 7 0>;
};
} ;

FWIW, there is some discussion about a generic panel fmwk / DT binding,
so I'm not even sure it will look like that at the end.

>>>> disp-pll = <>;
>>>> panel-type = "1024x560000000768@60";
>>>> powerdn-gpio = <&gpio2 7 0>;
>>>> };
>>>> };
>>>>
>>>> version@00A1 {
>>>> version = "00A1", "01";
>>>> dvi {
>>>> compatible = "da8xx-dt";
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&bone_dvi_cape_dvi_00A1_pins>;
>>>> ti,hwmods = "lcdc";
>>>>
>>>> disp-pll = <560000000>;
>>>> panel-type = "1024x768@60";
>>>> powerdn-gpio = <&gpio2 31 0>;
>>>> };
>>>> };
>>>>
>>>>
>>>
>>> Finally, we have the target user-base problem. The users are all going to
>>> be hobbyists, persons that buy beaglebones as a step up from Arduino. They
>>> will want to hack their own kind of hardware; they might not even have
>>> an EEPROM in their custom boards. They need some kind of runtime selection
>>> of the cape they have defined, these is handled by the case of overrides.
>>>
>>> For example, the Adafruit 1.8 Display is pretty popular, but it's not a cape.
>>> You have to wire it yourself and connect it to the expansion header. Then
>>> there's two different kinds of the display. It also has a PWM input for
>>> backlight control.
>>>
>>> Making use of it is simple with capebus:
>>>
>>> # cd /sys/devices/capebus.10
>>> # echo "0:Adafruit 1.8 Cape" >slots
>>>
>>> And that's it. The disable spi & pwm device the display needs will
>>> be turned on, the pinmux it needs will be set, and the spi device
>>> that the display uses will be created.
>>>
>>> I have no clue how this can be simpler using any kind of user-space
>>> method. And no, you can't have this active in the DTS of the common
>>> kernel for all beaglebones.
>>>
>>>> &bone_adafruit_cape {
>>>> board-name = "Adafruit 1.8 Cape";
>>>>
>>>> backlight {
>>>> compatible = "pwm-backlight";
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&pwm_bl_pins>;
>>>>
>>>> pwms = <&ehrpwm1 1 500000 0>;
>>>> pwm-names = "st7735fb";
>>>> brightness-levels = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36
>>>> default-brightness-level = <50>; /* index to the array above */
>>>> };
>>>>
>>>> spi1-devices {
>>>> compatible = "spi-dt";
>>>>
>>>> #address-cells = <1>;
>>>> #size-cells = <0>;
>>>>
>>>> parent = <&spi1>;
>>
>> Why do you need that? You seems to completely break the whole DT semantic with tons of customs bindings.
>>
>> From the driver code, it looks like the cape bus is not a real bus at all. If this is just a connector, then the I2C devices will be children of the AM33xx I2C controller, but there is no intermediate HW in the middle.
>
> No it is not that way. One set of capes can be just simple connector capes. Other capes are full blown devices with their
> own drivers. What do you propose as a fix is for the complex kind of capes?

What does that mean?

If the HW is like that:

beaglebone -- AM33XX -- I2C bus -- connector -- I2C devices in the cape

The connector does not have do be represented in the DT since it is a
dumb interface without any device / driver. At the end this is just an
board with more devices.

What is a complex cape?

>>>> lcd@0 {
>>>> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
>>>> spi-max-frequency = <8000000>;
>>>> reg = <0>;
>>>> spi-cpol;
>>>> spi-cpha;
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&lcd_pins>;
>>>> st7735-rst = <&gpio4 19 0>;
>>>> st7735-dc = <&gpio4 21 0>;
>>>> };
>>>>
>>>> };
>>>> };
>>>>
>>
>> I guess there is no easy solution for that, but it looks to me that what you have to do is to make the DT creation dynamic in your case. Assuming you do not want to do that in the bootloader, you must do that pretty early during the boot process to end up with a full description of your DT tree before creating the devices.
>>
>
> Do it pretty early in the boot processes ended up with the am335xevm board file in the PSP tree.

Well, you have to put somewhere the data that will represent the HW. And
you seem to do that anyway in am335x-bone-common.dtsi. So I'm not sure
to get your point.

My point was to potentially use some smaller DTB: am335x-bone.dtb,
geiger.dtb, capeXXX.dtb and create a mechanism to merge these DTB based
on configuration in bootloader/ early kernel boot.

> The whole set of possible cape designs cannot be controlled, nor do we want to.
> We want to empower users to come up with their own designs without having to do any kernel/boot loader
> hacking.

Mmm, how can that be possible? They do have at least to write a driver
for their cape and add some more DTS data, isn't it?

>> Each cape will have their own DTS and based on some board id you will fix the DT dynamically.
>>
>> My point is that the issue you are facing is a real limitation of DT, so you should fix the DT core and not workaround it by creating artificial bindings / drivers.
>>
>
> You still haven't described any mechanism to deal with all the use cases I described.
>
> DT can't and will not deal with the complexity that we're facing right now.
>
> This is a relatively clean way to fix it; we can actually ship a mainline kernel that works.

Sure, but that does not mean it cannot be done in a more generic /
cleaner way.

Regards,
Benoit


2012-11-01 11:26:18

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi

On Nov 1, 2012, at 1:04 PM, Felipe Balbi wrote:

> Hi,
>
> On Thu, Nov 01, 2012 at 12:39:30PM +0200, Pantelis Antoniou wrote:
>>>>> lcd@0 {
>>>>> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
>>>>> spi-max-frequency = <8000000>;
>>>>> reg = <0>;
>>>>> spi-cpol;
>>>>> spi-cpha;
>>>>> pinctrl-names = "default";
>>>>> pinctrl-0 = <&lcd_pins>;
>>>>> st7735-rst = <&gpio4 19 0>;
>>>>> st7735-dc = <&gpio4 21 0>;
>>>>> };
>>>>>
>>>>> };
>>>>> };
>>>>>
>>>
>>> I guess there is no easy solution for that, but it looks to me that
>>> what you have to do is to make the DT creation dynamic in your case.
>>> Assuming you do not want to do that in the bootloader, you must do
>>> that pretty early during the boot process to end up with a full
>>> description of your DT tree before creating the devices.
>>>
>>
>> Do it pretty early in the boot processes ended up with the am335xevm board file in the PSP tree.
>>
>> The whole set of possible cape designs cannot be controlled, nor do we want to.
>> We want to empower users to come up with their own designs without having to do any kernel/boot loader
>> hacking.
>
> that's impossible since you will have to provide the capebus driver
> anyway.
>

The generic cape bus driver can handle the easy stuff. More complex
stuff can be supported with per-cape drivers.

>>> Each cape will have their own DTS and based on some board id you
>>> will fix the DT dynamically.
>>>
>>> My point is that the issue you are facing is a real limitation of
>>> DT, so you should fix the DT core and not workaround it by creating
>>> artificial bindings / drivers.
>>>
>>
>> You still haven't described any mechanism to deal with all the use
>> cases I described.
>>
>> DT can't and will not deal with the complexity that we're facing right
>> now.
>
> and DT-itself shouldn't. I agree with Benoit that this should be built
> at bootloader level, perhaps. Whatever you're doing in capebus, you
> could do at kernel space, build your DT bindings in runtime, and pass
> that DT blob to kernel.

We're just passing the buck someplace else. We're not fixing the problem.
What makes you think that dealing with this in the bootloader is going
to be simpler?

>
> One question though, what do you mean by "some capes are full blown
> devices with their own drivers" ? Do you mean you have capes running
> some other (RT)OS and communicating with linux somehow ? How does it
> communicate to the bone ?

Some have FPGAs.
https://specialcomp.com/beaglebone/BeagleBone_FPGA.html

Some capes have their own MCUs.
A recent one has an 6502 communicating with uio_pruss.
https://github.com/ohporter/b6502

What I'm saying is that there are simple capes, that are just DT devices
which can be handled via cape-generic. The complex ones need their own
drivers which need a place to live.

Capebus handles both.

>
> cheers
>
> --
> balbi

Regards

-- Pantelis

2012-11-01 11:39:49

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Benoit,

On Nov 1, 2012, at 1:26 PM, Cousson, Benoit wrote:

> Hi Panto,
>
> On 11/1/2012 11:39 AM, Pantelis Antoniou wrote:
>> Hi Benoit,
>>
>> On Nov 1, 2012, at 12:23 PM, Cousson, Benoit wrote:
>>
>>> On 11/1/2012 8:02 AM, Pantelis Antoniou wrote:
>>>> Hi Felibe,
>>>>
>>>> On Nov 1, 2012, at 12:14 AM, Felipe Balbi wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
>>>>>>> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>>>>>>>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>>>>>>>
>>>>>>>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>>>>>>>> could have an hwmod and thus must be handled by an omap_device.
>>>>>>>>>
>>>>>>>>> Any devices that is created later cannot be omap_device. The DT core
>>>>>>>>> will create regular platform_device for them.
>>>>>>>>>
>>>>>>>>> Since cape is an external board, it should have nothing to do with
>>>>>>>>> omap_device.
>>>>>>>>>
>>>>>>>>> Looking at your patch:
>>>>>>>>> da8xx-dt: Create da8xx DT adapter device
>>>>>>>>>
>>>>>>>>> I understand why you do that, but in fact that patch does not make sense
>>>>>>>>> to me :-(
>>>>>>>>>
>>>>>>>>> Why do you have to create an omap_device from the driver probe?
>>>>>>>>>
>>>>>>>>
>>>>>>>> The problem is that capes are not external boards in the normal sense
>>>>>>>> as a PCI board is. In the PCI case the hardware that implements the
>>>>>>>> desired functionality is on the PCI board, while in the cape case the
>>>>>>>> hardware module is in the SoC. The cape most of the times is quite
>>>>>>>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>>>>>>>
>>>>>>> Ah now I see, you're talking about the beaglebone extension
>>>>>>> boards :)
>>>>>>>
>>>>>>> The way to deal with those properly is to just edit the
>>>>>>> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
>>>>>>> whatever.
>>>>>>>
>>>>>>
>>>>>> That is what is being done...
>>>>>> This is the fallout.
>>>>>>
>>>>>>>> You can't instantiate the omap_device early in the boot process like it was done up to
>>>>>>>> now in the board file. You can only do that later in the boot process (for built-in
>>>>>>>> cape drivers), or even after user-space has booted and the matching cape driver module
>>>>>>>> has been loaded.
>>>>>>>
>>>>>>> Yes you can, just edit the .dts file for the extension
>>>>>>> board you have connected. This stuff should be DT
>>>>>>> only for sure, let's not even start adding platform_data
>>>>>>> entries for that.
>>>>>>>
>>>>>>> The omap_device entries for the omap internal devices
>>>>>>> will be created automatically during startup based on the
>>>>>>> .dts. Note that you can set status = "disabled" for the
>>>>>>> omap internal devices in the .dts file, the devices are
>>>>>>> there in the hardware.
>>>>>>>
>>>>>>> If you are sure the extension boards are safely hot
>>>>>>> pluggable, then all you need is some interface to enable
>>>>>>> and disable devices after booting. But that should be
>>>>>>> done in Linux generic way.
>>>>>>>
>>>>>>> So we should not export any omap_hwmod or omap_device
>>>>>>> things, and want to keep it __init only, and local to
>>>>>>> arch/arm/mach-omap2.
>>>>>>>
>>>>>>
>>>>>> I disagree. This is not something that is handled by just
>>>>>> editing the dts. The way it is handled, is in the Linux
>>>>>> generic way, we have a proper bus, and the drivers as compiled
>>>>>> as standalone file.
>>>>>
>>>>> when you have an actual bus, that'd be correct.
>>>>
>>>> What do you think capebus is? It is an actual bus that allows you
>>>> to do so.
>>>>
>>>>>
>>>>>> We're hitting a use case that wasn't there in omap until now.
>>>>>>
>>>>>> There a a whole bunch of conflicting capes. There's no
>>>>>> way to instantiate them together. They must be instantiated
>>>>>> only after their EEPROMs are read and they are matched
>>>>>> to their corresponding cape drivers.
>>>>>
>>>>> why this requirement of instantiating them only after reading EEPROMs ?
>>>>>
>>>>> It's unnecessary to add that requirement, just do what Tony said
>>>>> (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
>>>>> should be created automatically.
>>>>>
>>>>> The thing is that there is no such thing as a cape-device. The cape is
>>>>> just a collection of I2C, 1-wire and SPI devices anyway. What we should
>>>>> instantiate is bmp085, tls2550, sht21, instead of instantiating
>>>>> beagle-bone-weather-cape.
>>>>>
>>>>> What's the problem in just instantiating all of those from bone.dts ?
>>>>> Expose the EEPROMs to userland so whatever SW you guys have running on
>>>>> the bone can figure out what features to expose to the SDK which the
>>>>> user sees, but the kernel doesn't need to know that there is a
>>>>> weather-cape attached to the bone.
>>>>>
>>>>
>>>> The I2C/SPI devices are not a problem at all.
>>>>
>>>> Let me try to explain what the problem is, and why it all this
>>>> is needed.
>>>>
>>>> First of all, capes may be relatively simple boards, which may function
>>>> as a generic device - like the generic capes. They might not necessarily
>>>> be simple. Some capes, for example the geiger cape have a number of
>>>> components that perform a specific task; in this instance the driving
>>>> circuit of the geiger tube and the event detector input. Other capes
>>>> that are being designed now are even more complex, i.e. there's a
>>>> radar cape, an fpga cape and so on. So for these kind of boards
>>>> you will need a specific driver. That driver will probably use some
>>>> generic-cape bits (like gpios, pwms, keys what-ever).
>>>> But it will put them to use in the custom in-kernel driver in it's own
>>>> way. You can't put that task to user-space, if it's ever slightly complex.
>>>>
>>>> So for really simple capes, after considerable pain you might, just
>>>> might, dump the problem to user-space and try to make it work.
>>>> People have tried that and it's a total pain. There is no way that this will
>>>> work with anything more complex than a generic cape.
>>>>
>>>> Then there's the out of the box experience problem.
>>>>
>>>> What a customer that buys one of these boards along with a number of
>>>> capes wants is to plug them, and have them work. You cannot have
>>>> the DTS file activating all the drivers for each possible cape since
>>>> they conflict; the pins that one SPI bus that one cape uses might be a
>>>> PWM output for another, and so on.
>>>>
>>>> Asking the customer to edit the distro supplied kernel's dts and recompiling,
>>>> while figuring out how to solve conflicts is not going to fly.
>>>
>>> Well you do not have to rebuild the kernel if you just want to update the DTS.
>>> The way DT is done, the best you can do is to rely on the bootloader in your case to select the proper DTB. You should try to merge dynamically AMxx + beaglebone + capeXXX based on some EEPROM id.
>>>
>>
>> That is what the capebus does. It inserts
>
> It inserts what?

Type, inserts device nodes - from the matching capebus node to the live DT tree and triggers instantiation.

>
>>> FWIW, we do have a similar, but simpler, problem with the beagle / beagle-xm and panda / panda-es. But for the moment we are just using a different DTS for each board.
>>
>> A different DTS for each board combination... There alot more capes for the bone that they are for the beagle & the panda.
>> And more are going to come, but not necessarily from people that have any connection to TI or CCO.
>
> Sure, but my point is that your solution will not solve our problem :-)
> This is a generic problem that you address with a very custom / specific approach.
>

The capebus is pretty generic. There is surprisingly few bone specific parts.

>> You still haven't described how I could solve the issue of conflicting capes using a single DTS.
>
> Well I don't have the solution otherwise I will have done it already :-)
> My point is that the solution has to be in the DT core if not in the bootloader.
> We can add some new binding to handle revision. And then we should be able during DT tree device creation to remove nodes that does not match the version criteria.
>
> In the case of panda/beagle, some GPIO lines are used to identify the board version.
>
> So we should be able to define some generic way to export revision and use that to select the proper nodes.
>
> This is not necessarily different to what your are doing with your capebus. The point is to build a generic way / binding to do that properly.
>

I do have beagleboards and pandaboards, but I don't have any capes for them yet.
I will pick up a few at ELC in barcelona in a few days.

I will then proceed and add support for both the beagleboard and the pandaboard
in capebus.

It is going to be pretty easy, and it will demonstrate where I'm going at.

Please don't think this is just a beaglebone hack.

>>>> We have the versioning problem. All the standard capes go through
>>>> a very fast evolution processes. So we now have 3 versions of a DVI cape,
>>>> 2 of a small LCD cape and so on. We don't want to abandon the capes that
>>>> are already sold and are out on the field, but we don't want to spend
>>>> too much time hacking the driver to support all the different version.
>>>> The version capability of capebus handles that; taking as an example the
>>>> DVI cape in which the power down GPIO moved around:
>>>>
>>>>> version@00A0 {
>>>>> version = "00A0";
>>>>> dvi {
>>>>> compatible = "da8xx-dt";
>>>>> pinctrl-names = "default";
>>>>> pinctrl-0 = <&bone_dvi_cape_dvi_00A0_pins>;
>>>>> ti,hwmods = "lcdc";
>>>
>>> That part is still weird to me. Assuming the lcdc is the Display controller, the DVI should be a child of the lcdc not the opposite.
>>>
>>
>> At the time of this there are no DT bindings yet for the da8xx driver.
>
> OK, but that does not answer my question...
>
> I don't know this HW, but the LCD controller should be the parent in that case.
>
> lcdc@48000000 {
> ti,hwmods = "lcdc";
> reg = <0x48000000>;
>
> dvi {
> disp-pll = <>;
> panel-type = "1024x560000000768@60";
> powerdn-gpio = <&gpio2 7 0>;
> };
> } ;
>
> FWIW, there is some discussion about a generic panel fmwk / DT binding, so I'm not even sure it will look like that at the end.
>

I am aware of the discussion, and apparently there are some new patches that add DT bindings for the da8xx.
I intend to integrate them and get rid of da8xx-dt completely. The da8xx-dt is just an adapter device with limited
bindings.

>>>>> disp-pll = <>;
>>>>> panel-type = "1024x560000000768@60";
>>>>> powerdn-gpio = <&gpio2 7 0>;
>>>>> };
>>>>> };
>>>>>
>>>>> version@00A1 {
>>>>> version = "00A1", "01";
>>>>> dvi {
>>>>> compatible = "da8xx-dt";
>>>>> pinctrl-names = "default";
>>>>> pinctrl-0 = <&bone_dvi_cape_dvi_00A1_pins>;
>>>>> ti,hwmods = "lcdc";
>>>>>
>>>>> disp-pll = <560000000>;
>>>>> panel-type = "1024x768@60";
>>>>> powerdn-gpio = <&gpio2 31 0>;
>>>>> };
>>>>> };
>>>>>
>>>>>
>>>>
>>>> Finally, we have the target user-base problem. The users are all going to
>>>> be hobbyists, persons that buy beaglebones as a step up from Arduino. They
>>>> will want to hack their own kind of hardware; they might not even have
>>>> an EEPROM in their custom boards. They need some kind of runtime selection
>>>> of the cape they have defined, these is handled by the case of overrides.
>>>>
>>>> For example, the Adafruit 1.8 Display is pretty popular, but it's not a cape.
>>>> You have to wire it yourself and connect it to the expansion header. Then
>>>> there's two different kinds of the display. It also has a PWM input for
>>>> backlight control.
>>>>
>>>> Making use of it is simple with capebus:
>>>>
>>>> # cd /sys/devices/capebus.10
>>>> # echo "0:Adafruit 1.8 Cape" >slots
>>>>
>>>> And that's it. The disable spi & pwm device the display needs will
>>>> be turned on, the pinmux it needs will be set, and the spi device
>>>> that the display uses will be created.
>>>>
>>>> I have no clue how this can be simpler using any kind of user-space
>>>> method. And no, you can't have this active in the DTS of the common
>>>> kernel for all beaglebones.
>>>>
>>>>> &bone_adafruit_cape {
>>>>> board-name = "Adafruit 1.8 Cape";
>>>>>
>>>>> backlight {
>>>>> compatible = "pwm-backlight";
>>>>> pinctrl-names = "default";
>>>>> pinctrl-0 = <&pwm_bl_pins>;
>>>>>
>>>>> pwms = <&ehrpwm1 1 500000 0>;
>>>>> pwm-names = "st7735fb";
>>>>> brightness-levels = <0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36
>>>>> default-brightness-level = <50>; /* index to the array above */
>>>>> };
>>>>>
>>>>> spi1-devices {
>>>>> compatible = "spi-dt";
>>>>>
>>>>> #address-cells = <1>;
>>>>> #size-cells = <0>;
>>>>>
>>>>> parent = <&spi1>;
>>>
>>> Why do you need that? You seems to completely break the whole DT semantic with tons of customs bindings.
>>>
>>> From the driver code, it looks like the cape bus is not a real bus at all. If this is just a connector, then the I2C devices will be children of the AM33xx I2C controller, but there is no intermediate HW in the middle.
>>
>> No it is not that way. One set of capes can be just simple connector capes. Other capes are full blown devices with their
>> own drivers. What do you propose as a fix is for the complex kind of capes?
>
> What does that mean?
>
> If the HW is like that:
>
> beaglebone -- AM33XX -- I2C bus -- connector -- I2C devices in the cape
>
> The connector does not have do be represented in the DT since it is a dumb interface without any device / driver. At the end this is just an board with more devices.
>
> What is a complex cape?

What you are describing are simple generic capes. The complex capes arrange some generic functionality (like I2C devices, or PWMs, or GPIOs)
and add custom hardware. One (medium complexity board) example is the geiger cape. More complex capes have FPGAs or even MCUs with their
own set of peripherals.

>
>>>>> lcd@0 {
>>>>> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
>>>>> spi-max-frequency = <8000000>;
>>>>> reg = <0>;
>>>>> spi-cpol;
>>>>> spi-cpha;
>>>>> pinctrl-names = "default";
>>>>> pinctrl-0 = <&lcd_pins>;
>>>>> st7735-rst = <&gpio4 19 0>;
>>>>> st7735-dc = <&gpio4 21 0>;
>>>>> };
>>>>>
>>>>> };
>>>>> };
>>>>>
>>>
>>> I guess there is no easy solution for that, but it looks to me that what you have to do is to make the DT creation dynamic in your case. Assuming you do not want to do that in the bootloader, you must do that pretty early during the boot process to end up with a full description of your DT tree before creating the devices.
>>>
>>
>> Do it pretty early in the boot processes ended up with the am335xevm board file in the PSP tree.
>
> Well, you have to put somewhere the data that will represent the HW. And you seem to do that anyway in am335x-bone-common.dtsi. So I'm not sure to get your point.
>
> My point was to potentially use some smaller DTB: am335x-bone.dtb, geiger.dtb, capeXXX.dtb and create a mechanism to merge these DTB based on configuration in bootloader/ early kernel boot.
>

We're just shuffling the responsibility of building the DT around. It not trivial to merge DT fragments.
The DT fragments have references to the core DTB; to deal with them we would need a full blown DT object format
capable of having a list of aliases to be resolved at runtime, and a DT loader linker to do it.

>> The whole set of possible cape designs cannot be controlled, nor do we want to.
>> We want to empower users to come up with their own designs without having to do any kernel/boot loader
>> hacking.
>
> Mmm, how can that be possible? They do have at least to write a driver for their cape and add some more DTS data, isn't it?
>

No they don't have to write a driver for a generic cape; they do have to tweak the DTS.

Take a look at the am335x-bone-common.dtsi patch fragment. There you will see whole bunch of capes
that are supported by the bone-generic-cape driver, like bone_lcd3_cape, bone_lcd7_cape, bone_dvi_cape,
bone_weather_cape, etc.


>>> Each cape will have their own DTS and based on some board id you will fix the DT dynamically.
>>>
>>> My point is that the issue you are facing is a real limitation of DT, so you should fix the DT core and not workaround it by creating artificial bindings / drivers.
>>>
>>
>> You still haven't described any mechanism to deal with all the use cases I described.
>>
>> DT can't and will not deal with the complexity that we're facing right now.
>>
>> This is a relatively clean way to fix it; we can actually ship a mainline kernel that works.
>
> Sure, but that does not mean it cannot be done in a more generic / cleaner way.
>

Most of the stuff that the beaglebone board controller uses is generic to capebus.
It is trivial to add beagleboard & pandaboard support, and in fact I plan to do so.

> Regards,
> Benoit
>
>
>

Regards

-- Pantelis

2012-11-01 12:00:25

by Koen Kooi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

tl;dr: please suggest an actual solution that allows plug&play when plugging in multiple capes and applying power after that. Preferably one that doesn't pass the buck to u-boot.

Op 1 nov. 2012, om 12:26 heeft "Cousson, Benoit" <[email protected]> het volgende geschreven:

> Hi Panto,
>
> On 11/1/2012 11:39 AM, Pantelis Antoniou wrote:
>> Hi Benoit,
>>
>> On Nov 1, 2012, at 12:23 PM, Cousson, Benoit wrote:
>>
>>> On 11/1/2012 8:02 AM, Pantelis Antoniou wrote:
>>>> Hi Felibe,
>>>>
>>>> On Nov 1, 2012, at 12:14 AM, Felipe Balbi wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
>>>>>>> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>>>>>>>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>>>>>>>
>>>>>>>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>>>>>>>> could have an hwmod and thus must be handled by an omap_device.
>>>>>>>>>
>>>>>>>>> Any devices that is created later cannot be omap_device. The DT core
>>>>>>>>> will create regular platform_device for them.
>>>>>>>>>
>>>>>>>>> Since cape is an external board, it should have nothing to do with
>>>>>>>>> omap_device.
>>>>>>>>>
>>>>>>>>> Looking at your patch:
>>>>>>>>> da8xx-dt: Create da8xx DT adapter device
>>>>>>>>>
>>>>>>>>> I understand why you do that, but in fact that patch does not make sense
>>>>>>>>> to me :-(
>>>>>>>>>
>>>>>>>>> Why do you have to create an omap_device from the driver probe?
>>>>>>>>>
>>>>>>>>
>>>>>>>> The problem is that capes are not external boards in the normal sense
>>>>>>>> as a PCI board is. In the PCI case the hardware that implements the
>>>>>>>> desired functionality is on the PCI board, while in the cape case the
>>>>>>>> hardware module is in the SoC. The cape most of the times is quite
>>>>>>>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>>>>>>>
>>>>>>> Ah now I see, you're talking about the beaglebone extension
>>>>>>> boards :)
>>>>>>>
>>>>>>> The way to deal with those properly is to just edit the
>>>>>>> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
>>>>>>> whatever.
>>>>>>>
>>>>>>
>>>>>> That is what is being done...
>>>>>> This is the fallout.
>>>>>>
>>>>>>>> You can't instantiate the omap_device early in the boot process like it was done up to
>>>>>>>> now in the board file. You can only do that later in the boot process (for built-in
>>>>>>>> cape drivers), or even after user-space has booted and the matching cape driver module
>>>>>>>> has been loaded.
>>>>>>>
>>>>>>> Yes you can, just edit the .dts file for the extension
>>>>>>> board you have connected. This stuff should be DT
>>>>>>> only for sure, let's not even start adding platform_data
>>>>>>> entries for that.
>>>>>>>
>>>>>>> The omap_device entries for the omap internal devices
>>>>>>> will be created automatically during startup based on the
>>>>>>> .dts. Note that you can set status = "disabled" for the
>>>>>>> omap internal devices in the .dts file, the devices are
>>>>>>> there in the hardware.
>>>>>>>
>>>>>>> If you are sure the extension boards are safely hot
>>>>>>> pluggable, then all you need is some interface to enable
>>>>>>> and disable devices after booting. But that should be
>>>>>>> done in Linux generic way.
>>>>>>>
>>>>>>> So we should not export any omap_hwmod or omap_device
>>>>>>> things, and want to keep it __init only, and local to
>>>>>>> arch/arm/mach-omap2.
>>>>>>>
>>>>>>
>>>>>> I disagree. This is not something that is handled by just
>>>>>> editing the dts. The way it is handled, is in the Linux
>>>>>> generic way, we have a proper bus, and the drivers as compiled
>>>>>> as standalone file.
>>>>>
>>>>> when you have an actual bus, that'd be correct.
>>>>
>>>> What do you think capebus is? It is an actual bus that allows you
>>>> to do so.
>>>>
>>>>>
>>>>>> We're hitting a use case that wasn't there in omap until now.
>>>>>>
>>>>>> There a a whole bunch of conflicting capes. There's no
>>>>>> way to instantiate them together. They must be instantiated
>>>>>> only after their EEPROMs are read and they are matched
>>>>>> to their corresponding cape drivers.
>>>>>
>>>>> why this requirement of instantiating them only after reading EEPROMs ?
>>>>>
>>>>> It's unnecessary to add that requirement, just do what Tony said
>>>>> (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
>>>>> should be created automatically.
>>>>>
>>>>> The thing is that there is no such thing as a cape-device. The cape is
>>>>> just a collection of I2C, 1-wire and SPI devices anyway. What we should
>>>>> instantiate is bmp085, tls2550, sht21, instead of instantiating
>>>>> beagle-bone-weather-cape.
>>>>>
>>>>> What's the problem in just instantiating all of those from bone.dts ?
>>>>> Expose the EEPROMs to userland so whatever SW you guys have running on
>>>>> the bone can figure out what features to expose to the SDK which the
>>>>> user sees, but the kernel doesn't need to know that there is a
>>>>> weather-cape attached to the bone.
>>>>>
>>>>
>>>> The I2C/SPI devices are not a problem at all.
>>>>
>>>> Let me try to explain what the problem is, and why it all this
>>>> is needed.
>>>>
>>>> First of all, capes may be relatively simple boards, which may function
>>>> as a generic device - like the generic capes. They might not necessarily
>>>> be simple. Some capes, for example the geiger cape have a number of
>>>> components that perform a specific task; in this instance the driving
>>>> circuit of the geiger tube and the event detector input. Other capes
>>>> that are being designed now are even more complex, i.e. there's a
>>>> radar cape, an fpga cape and so on. So for these kind of boards
>>>> you will need a specific driver. That driver will probably use some
>>>> generic-cape bits (like gpios, pwms, keys what-ever).
>>>> But it will put them to use in the custom in-kernel driver in it's own
>>>> way. You can't put that task to user-space, if it's ever slightly complex.
>>>>
>>>> So for really simple capes, after considerable pain you might, just
>>>> might, dump the problem to user-space and try to make it work.
>>>> People have tried that and it's a total pain. There is no way that this will
>>>> work with anything more complex than a generic cape.
>>>>
>>>> Then there's the out of the box experience problem.
>>>>
>>>> What a customer that buys one of these boards along with a number of
>>>> capes wants is to plug them, and have them work. You cannot have
>>>> the DTS file activating all the drivers for each possible cape since
>>>> they conflict; the pins that one SPI bus that one cape uses might be a
>>>> PWM output for another, and so on.
>>>>
>>>> Asking the customer to edit the distro supplied kernel's dts and recompiling,
>>>> while figuring out how to solve conflicts is not going to fly.
>>>
>>> Well you do not have to rebuild the kernel if you just want to update the DTS.
>>> The way DT is done, the best you can do is to rely on the bootloader in your case to select the proper DTB. You should try to merge dynamically AMxx + beaglebone + capeXXX based on some EEPROM id.
>>>
>>
>> That is what the capebus does. It inserts
>
> It inserts what?
>
>>> FWIW, we do have a similar, but simpler, problem with the beagle / beagle-xm and panda / panda-es. But for the moment we are just using a different DTS for each board.
>>
>> A different DTS for each board combination... There alot more capes for the bone that they are for the beagle & the panda.
>> And more are going to come, but not necessarily from people that have any connection to TI or CCO.
>
> Sure, but my point is that your solution will not solve our problem :-)
> This is a generic problem that you address with a very custom / specific approach.
>
>> You still haven't described how I could solve the issue of conflicting capes using a single DTS.
>
> Well I don't have the solution otherwise I will have done it already :-)
> My point is that the solution has to be in the DT core if not in the bootloader.

<snarky comment>So when we at beagleboard.org handled the beagleboard and beagleboard xM expansionboards in the bootloader (detection, muxing, etc) we were told it was wrong and we should handle it in the kernel and if we waited a bit, DT would solve everything. And now that we actually handle it in the kernel you are saying that it is wrong and we should handle it in the bootloader and that DT won't solve everything. I guess someone will now tell us that uEFI will fix everything.</snarky comment>

Apart from that, you and others still fail to tell us how you want to handle a user (or customer) that buys a beaglebone, an LCD cape, a weatherstation cape and a geiger counter cape and plugs those together and powers them on. With the evil TI vendor tree and extra patches the boardfile reads the eeproms and tries its best to instantiate all the platform data. One of the capes won't have working LEDs since appending to the leds-gpio struct is pretty much impossible in this situation. But it gets a picture on the screen, blinks LEDs and does largely what the user expects.

With capebus we get:

1) da8xx-fb which lacks DT bindingsp[1] receives plaftorm data to match the LCD
2) the i2c sensors on the weathercape are registered
3) the one-wire bus on the weathercape gets registered
4) the LEDs on the lcd cape get registered5
5) the LED on the geigercape gets registered and adds a custom trigger
6) the ADC, which again, lacks DT bindings[2], receives plaftorm data and a custom IIO binding
7) pinctrl sets the pinmuxes for the above

In other words: plug & play, even for devices with drivers that are still lacking DT support.

Now please explain to me why you think it's such an awesome idea that the users will need to find the right dtsi files (multiple revisions of the lcd cape exist), include them in the dts, run dtc, add a few missing semicolons, run dtc again, copy it over to /boot and reboot to have a change of making it work?
I know you can't escape that for new or custom capes, but I do want all the capes my company assembles work out of the box. (Cross)compiling a kernel is a bridge too far for 95% of the intended audience.

With capebus most capes can be supported by editing the DTS, your bootloader proposal involves updating u-boot for every new cape as well. That is downright scary. The RMA department will get flooded.

More importantly: capebus is generic enough to work on beagleboard, beagleboard xm, panda, panda es and even raspberry-pi. Basically on any DT capable platform.

Koen

[1] Patches for DT bindings were sent for that last week
[2] No DT bindings, contacting the author of the ADC support: *crickets*-

2012-11-01 12:46:24

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi,

On Thu, Nov 01, 2012 at 01:26:10PM +0200, Pantelis Antoniou wrote:
> Hi
>
> On Nov 1, 2012, at 1:04 PM, Felipe Balbi wrote:
>
> > Hi,
> >
> > On Thu, Nov 01, 2012 at 12:39:30PM +0200, Pantelis Antoniou wrote:
> >>>>> lcd@0 {
> >>>>> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
> >>>>> spi-max-frequency = <8000000>;
> >>>>> reg = <0>;
> >>>>> spi-cpol;
> >>>>> spi-cpha;
> >>>>> pinctrl-names = "default";
> >>>>> pinctrl-0 = <&lcd_pins>;
> >>>>> st7735-rst = <&gpio4 19 0>;
> >>>>> st7735-dc = <&gpio4 21 0>;
> >>>>> };
> >>>>>
> >>>>> };
> >>>>> };
> >>>>>
> >>>
> >>> I guess there is no easy solution for that, but it looks to me that
> >>> what you have to do is to make the DT creation dynamic in your case.
> >>> Assuming you do not want to do that in the bootloader, you must do
> >>> that pretty early during the boot process to end up with a full
> >>> description of your DT tree before creating the devices.
> >>>
> >>
> >> Do it pretty early in the boot processes ended up with the am335xevm board file in the PSP tree.
> >>
> >> The whole set of possible cape designs cannot be controlled, nor do we want to.
> >> We want to empower users to come up with their own designs without having to do any kernel/boot loader
> >> hacking.
> >
> > that's impossible since you will have to provide the capebus driver
> > anyway.
> >
>
> The generic cape bus driver can handle the easy stuff. More complex
> stuff can be supported with per-cape drivers.

likewise:

the generic stuff can be handled by FDT easily, the more complex stuff
can be supported by dynamically changing FDT blob, no ?

> >>> Each cape will have their own DTS and based on some board id you
> >>> will fix the DT dynamically.
> >>>
> >>> My point is that the issue you are facing is a real limitation of
> >>> DT, so you should fix the DT core and not workaround it by creating
> >>> artificial bindings / drivers.
> >>>
> >>
> >> You still haven't described any mechanism to deal with all the use
> >> cases I described.
> >>
> >> DT can't and will not deal with the complexity that we're facing right
> >> now.
> >
> > and DT-itself shouldn't. I agree with Benoit that this should be built
> > at bootloader level, perhaps. Whatever you're doing in capebus, you
> > could do at kernel space, build your DT bindings in runtime, and pass
> > that DT blob to kernel.
>
> We're just passing the buck someplace else. We're not fixing the problem.
> What makes you think that dealing with this in the bootloader is going
> to be simpler?

I never said it was supposed to be simpler, it just doesn't sound like
something the kernel should care about. Kernel cares about the

>
> >
> > One question though, what do you mean by "some capes are full blown
> > devices with their own drivers" ? Do you mean you have capes running
> > some other (RT)OS and communicating with linux somehow ? How does it
> > communicate to the bone ?
>
> Some have FPGAs.
> https://specialcomp.com/beaglebone/BeagleBone_FPGA.html

how does linux communicate with those ? What they are matters very
little ;-) All we need is an interface to load binaries to the FPGA.

> Some capes have their own MCUs.
> A recent one has an 6502 communicating with uio_pruss.
> https://github.com/ohporter/b6502

that uses remoteproc, so I assume it's using OMAP's mailbox ?

Again I say that this is not a 'capebus', it's just another device
which we use another interface to talk to.

i2c devices will be children of the omap i2c controller, spi devices
will be children of the omap mcspi, GPMC devices will need the GPMC
controller and so on, but none of this looks like argument to introduce
a fake bus into the kernel.

--
balbi


Attachments:
(No filename) (3.86 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-11-01 12:57:34

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi,

On Nov 1, 2012, at 2:40 PM, Felipe Balbi wrote:

> Hi,
>
> On Thu, Nov 01, 2012 at 01:26:10PM +0200, Pantelis Antoniou wrote:
>> Hi
>>
>> On Nov 1, 2012, at 1:04 PM, Felipe Balbi wrote:
>>
>>> Hi,
>>>
>>> On Thu, Nov 01, 2012 at 12:39:30PM +0200, Pantelis Antoniou wrote:
>>>>>>> lcd@0 {
>>>>>>> compatible = "adafruit,tft-lcd-1.8-red", "sitronix,st7735";
>>>>>>> spi-max-frequency = <8000000>;
>>>>>>> reg = <0>;
>>>>>>> spi-cpol;
>>>>>>> spi-cpha;
>>>>>>> pinctrl-names = "default";
>>>>>>> pinctrl-0 = <&lcd_pins>;
>>>>>>> st7735-rst = <&gpio4 19 0>;
>>>>>>> st7735-dc = <&gpio4 21 0>;
>>>>>>> };
>>>>>>>
>>>>>>> };
>>>>>>> };
>>>>>>>
>>>>>
>>>>> I guess there is no easy solution for that, but it looks to me that
>>>>> what you have to do is to make the DT creation dynamic in your case.
>>>>> Assuming you do not want to do that in the bootloader, you must do
>>>>> that pretty early during the boot process to end up with a full
>>>>> description of your DT tree before creating the devices.
>>>>>
>>>>
>>>> Do it pretty early in the boot processes ended up with the am335xevm board file in the PSP tree.
>>>>
>>>> The whole set of possible cape designs cannot be controlled, nor do we want to.
>>>> We want to empower users to come up with their own designs without having to do any kernel/boot loader
>>>> hacking.
>>>
>>> that's impossible since you will have to provide the capebus driver
>>> anyway.
>>>
>>
>> The generic cape bus driver can handle the easy stuff. More complex
>> stuff can be supported with per-cape drivers.
>
> likewise:
>
> the generic stuff can be handled by FDT easily, the more complex stuff
> can be supported by dynamically changing FDT blob, no ?
>

No. You're missing the point. Up until we read the EEPROM we don't know
what to dynamically change in the device tree (not the blob). What you
will end up is something like capebus again. Only you're going to reinvent it
with a different name.

>>>>> Each cape will have their own DTS and based on some board id you
>>>>> will fix the DT dynamically.
>>>>>
>>>>> My point is that the issue you are facing is a real limitation of
>>>>> DT, so you should fix the DT core and not workaround it by creating
>>>>> artificial bindings / drivers.
>>>>>
>>>>
>>>> You still haven't described any mechanism to deal with all the use
>>>> cases I described.
>>>>
>>>> DT can't and will not deal with the complexity that we're facing right
>>>> now.
>>>
>>> and DT-itself shouldn't. I agree with Benoit that this should be built
>>> at bootloader level, perhaps. Whatever you're doing in capebus, you
>>> could do at kernel space, build your DT bindings in runtime, and pass
>>> that DT blob to kernel.
>>
>> We're just passing the buck someplace else. We're not fixing the problem.
>> What makes you think that dealing with this in the bootloader is going
>> to be simpler?
>
> I never said it was supposed to be simpler, it just doesn't sound like
> something the kernel should care about. Kernel cares about the

I just disagree here. The kernel should provide services that make the life
of users easier, not the lives of the kernel developers easier.

>
>>
>>>
>>> One question though, what do you mean by "some capes are full blown
>>> devices with their own drivers" ? Do you mean you have capes running
>>> some other (RT)OS and communicating with linux somehow ? How does it
>>> communicate to the bone ?
>>
>> Some have FPGAs.
>> https://specialcomp.com/beaglebone/BeagleBone_FPGA.html
>
> how does linux communicate with those ? What they are matters very
> little ;-) All we need is an interface to load binaries to the FPGA.
>

We can not know what people will come up with. It might have a few GPIOs
to load the bitfile to the FPGA, but after that, no-one can tell.
I might not interface to Linux at all; it might interface via I2C, or RS232.

Chances are, it won't fit in any kind of known drivers of linux.
Some guy is using an FPGA for SDR, another is making a radar cape.

These guys don't give a damn frankly about Linux. What they do care about
is having a cheap/easy to develop platform for their own little widget.
If you are going to ask from the to hunt down their own dts and assemble
from various dtsi's they'll just move to something else.

Which is a shame, cause we do have a nice platform here.

>> Some capes have their own MCUs.
>> A recent one has an 6502 communicating with uio_pruss.
>> https://github.com/ohporter/b6502
>
> that uses remoteproc, so I assume it's using OMAP's mailbox ?
>
> Again I say that this is not a 'capebus', it's just another device
> which we use another interface to talk to.
>
> i2c devices will be children of the omap i2c controller, spi devices
> will be children of the omap mcspi, GPMC devices will need the GPMC
> controller and so on, but none of this looks like argument to introduce
> a fake bus into the kernel.
>

I'll let the users of said bus to do the talking. You're just flat out
wrong IMO.

> --
> balbi

Regards

-- Pantelis

2012-11-01 13:12:37

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi,

On Thu, Nov 01, 2012 at 01:00:21PM +0100, Koen Kooi wrote:
> tl;dr: please suggest an actual solution that allows plug&play when
> plugging in multiple capes and applying power after that. Preferably
> one that doesn't pass the buck to u-boot.

I can think of a few ways:

1) ship your distribution with all necessary drivers and let udev handle
driver probing.

clearly this will require constant updates for the distribution
as new capes are developed. But IIUC, that's the same with
Arduino where new "libraries" are added to the Arduino OS.

2) ship with drivers for EEPROMs only and setup a repository of drivers

this would require every driver to be packaged separately, then
you create a setup where bone's userland will use EEPROMs data
to figure out which drivers it needs, pass that information to
SDK via USB, then SDK downloads all necessary/missing packages,
sends them to bone via USB and all packages are installed on the
bone.

Note that since packages would be 'installed', this would be a
one-time-only thing.

3) realize that if your user can build an FPGA cape, s/he can most
likely write code and adding/recompiling kernel shouldn't be the
biggest of his/her worries

(no comments to this one, I understand it's not feasible)

in any case, capebus sounds like something which is hugely unnecessary.

ps: at least for the I2C subsystem, i2c-core can detect for you if your
driver provides ->detect() method.

> Op 1 nov. 2012, om 12:26 heeft "Cousson, Benoit" <[email protected]> het volgende geschreven:
> >>> FWIW, we do have a similar, but simpler, problem with the beagle /
> >>> beagle-xm and panda / panda-es. But for the moment we are just
> >>> using a different DTS for each board.
> >>
> >> A different DTS for each board combination... There alot more capes
> >> for the bone that they are for the beagle & the panda. And more
> >> are going to come, but not necessarily from people that have any
> >> connection to TI or CCO.
> >
> > Sure, but my point is that your solution will not solve our problem
> > :-)
> > This is a generic problem that you address with a very custom /
> > specific approach.
> >
> >> You still haven't described how I could solve the issue of
> >> conflicting capes using a single DTS.
> >
> > Well I don't have the solution otherwise I will have done it already
> > :-)
> > My point is that the solution has to be in the DT core if not in the
> > bootloader.
>
> <snarky comment>So when we at beagleboard.org handled the beagleboard
> and beagleboard xM expansionboards in the bootloader (detection,
> muxing, etc) we were told it was wrong and we should handle it in the
> kernel and if we waited a bit, DT would solve everything. And now that
> we actually handle it in the kernel you are saying that it is wrong
> and we should handle it in the bootloader and that DT won't solve
> everything. I guess someone will now tell us that uEFI will fix
> everything.</snarky comment>
>
> Apart from that, you and others still fail to tell us how you want to
> handle a user (or customer) that buys a beaglebone, an LCD cape, a
> weatherstation cape and a geiger counter cape and plugs those together
> and powers them on. With the evil TI vendor tree and extra patches the
> boardfile reads the eeproms and tries its best to instantiate all the
> platform data. One of the capes won't have working LEDs since
> appending to the leds-gpio struct is pretty much impossible in this

couldn't you just instantiate multiple leds-gpio device ?

> situation. But it gets a picture on the screen, blinks LEDs and does
> largely what the user expects.
>
> With capebus we get:
>
> 1) da8xx-fb which lacks DT bindingsp[1] receives plaftorm data to
> match the LCD

this is something which could be fixed at the driver level, right ? :-)

> 2) the i2c sensors on the weathercape are registered

that will work with or without capebus.

> 3) the one-wire bus on the weathercape gets registered

also should work with or without capebus.

> 4) the LEDs on the lcd cape get registered5

also works with or without capebus.

> 5) the LED on the geigercape gets registered and adds a custom trigger

also works with or without capebus.

> 6) the ADC, which again, lacks DT bindings[2], receives plaftorm data
> and a custom IIO binding

driver problem.

> 7) pinctrl sets the pinmuxes for the above

also works with or without capebus.

> In other words: plug & play, even for devices with drivers that are
> still lacking DT support.

I _do_ agree with you that we could have a "grace period" where DT and
non-DT would boot together, but apparently that's not something which
isn't trivial to do.

I guess Benoit might also be concerned that if we add such an
infrastructure than conversion to DT will never finish heh.

> Now please explain to me why you think it's such an awesome idea that
> the users will need to find the right dtsi files (multiple revisions
> of the lcd cape exist), include them in the dts, run dtc, add a few
> missing semicolons, run dtc again, copy it over to /boot and reboot to
> have a change of making it work?

that shouldn't be necessary. At least for all the I2C devices, you can
just implement ->detect() and it will just work. Maybe similar tricks
can be done for 1-wire and SPI, I haven't looked into the details of
those buses to be sure, though.

> I know you can't escape that for new or custom capes, but I do want
> all the capes my company assembles work out of the box.

then push all drivers to mainline and start shipping your boards with
those drivers enabled.

> (Cross)compiling a kernel is a bridge too far for 95% of the intended
> audience.

Agreed, but that doesn't prevent you from either shipping distribution
with drivers enabled or providing pre-compiled modules.

> With capebus most capes can be supported by editing the DTS, your
> bootloader proposal involves updating u-boot for every new cape as
> well. That is downright scary. The RMA department will get flooded.

that's not true at all. If capebus can do all that from within kernel,
it surely can do the same (with a few changes here and there) from
within bootloader.

> More importantly: capebus is generic enough to work on beagleboard,
> beagleboard xm, panda, panda es and even raspberry-pi. Basically on
> any DT capable platform.

that doesn't matter much I guess. MTP is so cool that works on Exynos,
OMAP, Tegra, x86, Cris, AVR, etc etc etc and we still don't have an MTP
stack inside the kernel (ok a bit sarcastic... but you get the drift,
hopefully).

--
balbi


Attachments:
(No filename) (6.41 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-11-01 13:22:09

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi,

On Thu, Nov 01, 2012 at 02:57:26PM +0200, Pantelis Antoniou wrote:
> >>>>> Each cape will have their own DTS and based on some board id you
> >>>>> will fix the DT dynamically.
> >>>>>
> >>>>> My point is that the issue you are facing is a real limitation of
> >>>>> DT, so you should fix the DT core and not workaround it by creating
> >>>>> artificial bindings / drivers.
> >>>>>
> >>>>
> >>>> You still haven't described any mechanism to deal with all the use
> >>>> cases I described.
> >>>>
> >>>> DT can't and will not deal with the complexity that we're facing right
> >>>> now.
> >>>
> >>> and DT-itself shouldn't. I agree with Benoit that this should be built
> >>> at bootloader level, perhaps. Whatever you're doing in capebus, you
> >>> could do at kernel space, build your DT bindings in runtime, and pass
> >>> that DT blob to kernel.
> >>
> >> We're just passing the buck someplace else. We're not fixing the problem.
> >> What makes you think that dealing with this in the bootloader is going
> >> to be simpler?
> >
> > I never said it was supposed to be simpler, it just doesn't sound like
> > something the kernel should care about. Kernel cares about the
>
> I just disagree here. The kernel should provide services that make the life
> of users easier, not the lives of the kernel developers easier.

and it's already doing that isn't it ? we have i2c framework for i2c
clients. From a userland point of view, you have input layer, iio,
hwmon, chardev and a whole bunch of other interfaces which standardize
device accesses.

Look at your sentence again: "kernel should make life of users easier,
not that of kernel developers"; IMHO capebus is just making it easier
for the kernel developers who have to maintain a bunch of drivers for
different devices on different capes ;-)

At the end of the day, capebus will just be creating devices which will
be handled by the same I2C framework, iio, input, hwmon, and so on. So
what was the great benefit from capebus other than decreasing the
amount of changes to .dts files ?

> >>> One question though, what do you mean by "some capes are full blown
> >>> devices with their own drivers" ? Do you mean you have capes running
> >>> some other (RT)OS and communicating with linux somehow ? How does it
> >>> communicate to the bone ?
> >>
> >> Some have FPGAs.
> >> https://specialcomp.com/beaglebone/BeagleBone_FPGA.html
> >
> > how does linux communicate with those ? What they are matters very
> > little ;-) All we need is an interface to load binaries to the FPGA.
> >
>
> We can not know what people will come up with. It might have a few GPIOs
> to load the bitfile to the FPGA, but after that, no-one can tell.
> I might not interface to Linux at all; it might interface via I2C, or RS232.

which means that whoever writes RTL for the FPGA needs to do so with
bone's I/O choices in mind.

Let's assume the use UART to send bitfile to FPGA and bitfile is a model
of an I2C Sensor, they'll have to use /dev/ttyOn to pass bitfile to FPGA
and later an i2c-client driver (possibly using iio, since it's a sensor)
will be loaded. Right ?

> Chances are, it won't fit in any kind of known drivers of linux.
> Some guy is using an FPGA for SDR, another is making a radar cape.

awesome. That means we need to take care of those :-) Even with capebus,
they will still have to write those drivers won't they ? So instead of
writing some capebus driver, why can't the guy write a driver for his
radar instead ? That way, if he ever turns that into an ASIC and decides
to sell it as a chip, he doesn't have to write another driver just to
strip the capebus away.

> These guys don't give a damn frankly about Linux. What they do care
> about is having a cheap/easy to develop platform for their own little
> widget. If you are going to ask from the to hunt down their own dts
> and assemble from various dtsi's they'll just move to something else.

I never asked that :-) What I'm claiming is that capebus doesn't sound
like the best solution for the problem exposed.

> Which is a shame, cause we do have a nice platform here.

I agree with you, the bone is quite awesome ;-)

> >> Some capes have their own MCUs.
> >> A recent one has an 6502 communicating with uio_pruss.
> >> https://github.com/ohporter/b6502
> >
> > that uses remoteproc, so I assume it's using OMAP's mailbox ?
> >
> > Again I say that this is not a 'capebus', it's just another device
> > which we use another interface to talk to.
> >
> > i2c devices will be children of the omap i2c controller, spi devices
> > will be children of the omap mcspi, GPMC devices will need the GPMC
> > controller and so on, but none of this looks like argument to introduce
> > a fake bus into the kernel.
> >
>
> I'll let the users of said bus to do the talking. You're just flat out
> wrong IMO.

fair enough, I feel the same way about your capebus ;-)

--
balbi


Attachments:
(No filename) (4.79 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-11-01 13:33:31

by Koen Kooi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2


Op 1 nov. 2012, om 14:06 heeft Felipe Balbi <[email protected]> het volgende geschreven:

> Hi,
>
> On Thu, Nov 01, 2012 at 01:00:21PM +0100, Koen Kooi wrote:
>> tl;dr: please suggest an actual solution that allows plug&play when
>> plugging in multiple capes and applying power after that. Preferably
>> one that doesn't pass the buck to u-boot.
>
> I can think of a few ways:
>
> 1) ship your distribution with all necessary drivers and let udev handle
> driver probing.
>
> clearly this will require constant updates for the distribution
> as new capes are developed. But IIUC, that's the same with
> Arduino where new "libraries" are added to the Arduino OS.

And how are you going to handle pinmuxing and conflict resolution? You're basically saying that people should just use /dev/mem to write drivers, since you don't consider their use case valid.

>
> 2) ship with drivers for EEPROMs only and setup a repository of drivers
>
> this would require every driver to be packaged separately, then
> you create a setup where bone's userland will use EEPROMs data
> to figure out which drivers it needs, pass that information to
> SDK via USB, then SDK downloads all necessary/missing packages,
> sends them to bone via USB and all packages are installed on the
> bone.
>
> Note that since packages would be 'installed', this would be a
> one-time-only thing.

I lack the words to describe my emotions after reading this. Let's leave it at that.

> 3) realize that if your user can build an FPGA cape, s/he can most
> likely write code and adding/recompiling kernel shouldn't be the
> biggest of his/her worries
>
> (no comments to this one, I understand it's not feasible)

You'd be surprised.

> in any case, capebus sounds like something which is hugely unnecessary.

On tablets and phones, yes. But TI chips are use for different use-cases, bone + capes being one of those.

> ps: at least for the I2C subsystem, i2c-core can detect for you if your
> driver provides ->detect() method.

So I just need to patch all i2c drivers and force people to use the "standard" address for the i2c chip when designing a board.

>> Op 1 nov. 2012, om 12:26 heeft "Cousson, Benoit" <[email protected]> het volgende geschreven:
>>>>> FWIW, we do have a similar, but simpler, problem with the beagle /
>>>>> beagle-xm and panda / panda-es. But for the moment we are just
>>>>> using a different DTS for each board.
>>>>
>>>> A different DTS for each board combination... There alot more capes
>>>> for the bone that they are for the beagle & the panda. And more
>>>> are going to come, but not necessarily from people that have any
>>>> connection to TI or CCO.
>>>
>>> Sure, but my point is that your solution will not solve our problem
>>> :-)
>>> This is a generic problem that you address with a very custom /
>>> specific approach.
>>>
>>>> You still haven't described how I could solve the issue of
>>>> conflicting capes using a single DTS.
>>>
>>> Well I don't have the solution otherwise I will have done it already
>>> :-)
>>> My point is that the solution has to be in the DT core if not in the
>>> bootloader.
>>
>> <snarky comment>So when we at beagleboard.org handled the beagleboard
>> and beagleboard xM expansionboards in the bootloader (detection,
>> muxing, etc) we were told it was wrong and we should handle it in the
>> kernel and if we waited a bit, DT would solve everything. And now that
>> we actually handle it in the kernel you are saying that it is wrong
>> and we should handle it in the bootloader and that DT won't solve
>> everything. I guess someone will now tell us that uEFI will fix
>> everything.</snarky comment>
>>
>> Apart from that, you and others still fail to tell us how you want to
>> handle a user (or customer) that buys a beaglebone, an LCD cape, a
>> weatherstation cape and a geiger counter cape and plugs those together
>> and powers them on. With the evil TI vendor tree and extra patches the
>> boardfile reads the eeproms and tries its best to instantiate all the
>> platform data. One of the capes won't have working LEDs since
>> appending to the leds-gpio struct is pretty much impossible in this
>
> couldn't you just instantiate multiple leds-gpio device ?

No, and that is a problem in the driver core, see earlier discussions about similar problems.

>> situation. But it gets a picture on the screen, blinks LEDs and does
>> largely what the user expects.
>>
>> With capebus we get:
>>
>> 1) da8xx-fb which lacks DT bindingsp[1] receives plaftorm data to
>> match the LCD
>
> this is something which could be fixed at the driver level, right ? :-)

You'd have to tell your coworkers that.

>> 2) the i2c sensors on the weathercape are registered
>
> that will work with or without capebus.

Not in a plug and play way.

>> 3) the one-wire bus on the weathercape gets registered
>
> also should work with or without capebus.

Not in a plug and play way.

>> 4) the LEDs on the lcd cape get registered5
>
> also works with or without capebus.

Not in a plug and play way.

>> 5) the LED on the geigercape gets registered and adds a custom trigger
>
> also works with or without capebus.

Not in a plug and play way.

>
>> 6) the ADC, which again, lacks DT bindings[2], receives plaftorm data
>> and a custom IIO binding
>
> driver problem.

Yes, beat up your coworkers, I can't.

>
>> 7) pinctrl sets the pinmuxes for the above
>
> also works with or without capebus.

Not in a plug and play way.

>
>> In other words: plug & play, even for devices with drivers that are
>> still lacking DT support.
>
> I _do_ agree with you that we could have a "grace period" where DT and
> non-DT would boot together, but apparently that's not something which
> isn't trivial to do.
>
> I guess Benoit might also be concerned that if we add such an
> infrastructure than conversion to DT will never finish heh.

Like I said, take that up with your coworkers. I want DT support for all TI IP blocks, apparently TI disagrees.

>> Now please explain to me why you think it's such an awesome idea that
>> the users will need to find the right dtsi files (multiple revisions
>> of the lcd cape exist), include them in the dts, run dtc, add a few
>> missing semicolons, run dtc again, copy it over to /boot and reboot to
>> have a change of making it work?
>
> that shouldn't be necessary. At least for all the I2C devices, you can
> just implement ->detect() and it will just work. Maybe similar tricks
> can be done for 1-wire and SPI, I haven't looked into the details of
> those buses to be sure, though.
>
>> I know you can't escape that for new or custom capes, but I do want
>> all the capes my company assembles work out of the box.
>
> then push all drivers to mainline and start shipping your boards with
> those drivers enabled.

So how do you solve conflicts? The PRU pins are mixed with the LCD pins. So how can I enable both drivers? Same deal with mcasp and SPI.

>
>> (Cross)compiling a kernel is a bridge too far for 95% of the intended
>> audience.
>
> Agreed, but that doesn't prevent you from either shipping distribution
> with drivers enabled or providing pre-compiled modules.
>
>> With capebus most capes can be supported by editing the DTS, your
>> bootloader proposal involves updating u-boot for every new cape as
>> well. That is downright scary. The RMA department will get flooded.
>
> that's not true at all. If capebus can do all that from within kernel,
> it surely can do the same (with a few changes here and there) from
> within bootloader.

Yes, and then I have 2 places to add support for capes instead of one. And I seriously question why anyone thinks that having users replace their bootloader everytime they add a new i2c device or LED to their board is a good idea.

>
>> More importantly: capebus is generic enough to work on beagleboard,
>> beagleboard xm, panda, panda es and even raspberry-pi. Basically on
>> any DT capable platform.
>
> that doesn't matter much I guess. MTP is so cool that works on Exynos,
> OMAP, Tegra, x86, Cris, AVR, etc etc etc and we still don't have an MTP
> stack inside the kernel (ok a bit sarcastic... but you get the drift,
> hopefully).

Yes, I agree, we still don't have a working MUSB driver in the kernel either. But that shouldn't stop capebus from being considered.-

2012-11-01 13:36:06

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi

On Nov 1, 2012, at 3:16 PM, Felipe Balbi wrote:

> Hi,
>
> On Thu, Nov 01, 2012 at 02:57:26PM +0200, Pantelis Antoniou wrote:
>>>>>>> Each cape will have their own DTS and based on some board id you
>>>>>>> will fix the DT dynamically.
>>>>>>>
>>>>>>> My point is that the issue you are facing is a real limitation of
>>>>>>> DT, so you should fix the DT core and not workaround it by creating
>>>>>>> artificial bindings / drivers.
>>>>>>>
>>>>>>
>>>>>> You still haven't described any mechanism to deal with all the use
>>>>>> cases I described.
>>>>>>
>>>>>> DT can't and will not deal with the complexity that we're facing right
>>>>>> now.
>>>>>
>>>>> and DT-itself shouldn't. I agree with Benoit that this should be built
>>>>> at bootloader level, perhaps. Whatever you're doing in capebus, you
>>>>> could do at kernel space, build your DT bindings in runtime, and pass
>>>>> that DT blob to kernel.
>>>>
>>>> We're just passing the buck someplace else. We're not fixing the problem.
>>>> What makes you think that dealing with this in the bootloader is going
>>>> to be simpler?
>>>
>>> I never said it was supposed to be simpler, it just doesn't sound like
>>> something the kernel should care about. Kernel cares about the
>>
>> I just disagree here. The kernel should provide services that make the life
>> of users easier, not the lives of the kernel developers easier.
>
> and it's already doing that isn't it ? we have i2c framework for i2c
> clients. From a userland point of view, you have input layer, iio,
> hwmon, chardev and a whole bunch of other interfaces which standardize
> device accesses.
>
> Look at your sentence again: "kernel should make life of users easier,
> not that of kernel developers"; IMHO capebus is just making it easier
> for the kernel developers who have to maintain a bunch of drivers for
> different devices on different capes ;-)
>
> At the end of the day, capebus will just be creating devices which will
> be handled by the same I2C framework, iio, input, hwmon, and so on. So
> what was the great benefit from capebus other than decreasing the
> amount of changes to .dts files ?
>

How about true plug and play, conflict resolution & real easy development of
new cape designs.

I fail to see how your convoluted method(s) fixes any of the above. And no
passing the buck to someone else (be it u-boot, u-dev, user-space) won't cut
it.

What you are proposing will just stop dead all cape development besides the
ones that dedicated teams at TI do. In fact the whole project, of a cheap
Linux based, hackable community board will fail.


>>>>> One question though, what do you mean by "some capes are full blown
>>>>> devices with their own drivers" ? Do you mean you have capes running
>>>>> some other (RT)OS and communicating with linux somehow ? How does it
>>>>> communicate to the bone ?
>>>>
>>>> Some have FPGAs.
>>>> https://specialcomp.com/beaglebone/BeagleBone_FPGA.html
>>>
>>> how does linux communicate with those ? What they are matters very
>>> little ;-) All we need is an interface to load binaries to the FPGA.
>>>
>>
>> We can not know what people will come up with. It might have a few GPIOs
>> to load the bitfile to the FPGA, but after that, no-one can tell.
>> I might not interface to Linux at all; it might interface via I2C, or RS232.
>
> which means that whoever writes RTL for the FPGA needs to do so with
> bone's I/O choices in mind.
>
> Let's assume the use UART to send bitfile to FPGA and bitfile is a model
> of an I2C Sensor, they'll have to use /dev/ttyOn to pass bitfile to FPGA
> and later an i2c-client driver (possibly using iio, since it's a sensor)
> will be loaded. Right ?
>

No. I don't know details about what the actual cape design is, but they
certainly don't want to deal with the piece-meal method, of loading one
driver, and then creating a bunch of I2C devices, and then activating an
RS232 tty, and then using that to burn the bitfile and so on.

What they want, and what every user wants, is I plug this board in, and
the driver make sure everything is loaded and ready. No, the end users
don't want to see any of the implementation details of how the bitfile
is transported; the driver can handle it.

>> Chances are, it won't fit in any kind of known drivers of linux.
>> Some guy is using an FPGA for SDR, another is making a radar cape.
>
> awesome. That means we need to take care of those :-) Even with capebus,
> they will still have to write those drivers won't they ? So instead of
> writing some capebus driver, why can't the guy write a driver for his
> radar instead ? That way, if he ever turns that into an ASIC and decides
> to sell it as a chip, he doesn't have to write another driver just to
> strip the capebus away.

You have to build something working first, before you move on to make a custom
board based to it. FWIW, after a design gets to work as a cape, chances
are the hardware redesign will just end up with the same h/w connections
as a cape, and just use an override to avoid putting the EEPROM in there.

>
>> These guys don't give a damn frankly about Linux. What they do care
>> about is having a cheap/easy to develop platform for their own little
>> widget. If you are going to ask from the to hunt down their own dts
>> and assemble from various dtsi's they'll just move to something else.
>
> I never asked that :-) What I'm claiming is that capebus doesn't sound
> like the best solution for the problem exposed.
>
>> Which is a shame, cause we do have a nice platform here.
>
> I agree with you, the bone is quite awesome ;-)

Not without any working capes, it isn't.

>
>>>> Some capes have their own MCUs.
>>>> A recent one has an 6502 communicating with uio_pruss.
>>>> https://github.com/ohporter/b6502
>>>
>>> that uses remoteproc, so I assume it's using OMAP's mailbox ?
>>>
>>> Again I say that this is not a 'capebus', it's just another device
>>> which we use another interface to talk to.
>>>
>>> i2c devices will be children of the omap i2c controller, spi devices
>>> will be children of the omap mcspi, GPMC devices will need the GPMC
>>> controller and so on, but none of this looks like argument to introduce
>>> a fake bus into the kernel.
>>>
>>
>> I'll let the users of said bus to do the talking. You're just flat out
>> wrong IMO.
>
> fair enough, I feel the same way about your capebus ;-)

Let's agree to disagree then :)

>
> --
> balbi

Regards

-- Pantelis

2012-11-01 13:46:57

by Alan

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

> What they want, and what every user wants, is I plug this board in, and
> the driver make sure everything is loaded and ready. No, the end users
> don't want to see any of the implementation details of how the bitfile
> is transported; the driver can handle it.

That doesn't necessarily make it a bus merely some kind of hotplug
enumeration of devices. That should all work properly both for devices
and busses with spi and i²c as the final bits needed for it got fixed
some time ago.

In an ideal world you don't want to be writing custom drivers for stuff.
If your cape routes an i²c serial device to the existing system i²c
busses then you want to just create an instance of any existing driver on
the existing i²c bus not create a whole new layer of goop.

It does need to do the plumbing and resource management for the plumbing
but thats not the same as being a bus.

Alan

2012-11-01 13:59:59

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Alan,

On Nov 1, 2012, at 3:51 PM, Alan Cox wrote:

>> What they want, and what every user wants, is I plug this board in, and
>> the driver make sure everything is loaded and ready. No, the end users
>> don't want to see any of the implementation details of how the bitfile
>> is transported; the driver can handle it.
>
> That doesn't necessarily make it a bus merely some kind of hotplug
> enumeration of devices. That should all work properly both for devices
> and busses with spi and i?c as the final bits needed for it got fixed
> some time ago.
>
> In an ideal world you don't want to be writing custom drivers for stuff.
> If your cape routes an i?c serial device to the existing system i?c
> busses then you want to just create an instance of any existing driver on
> the existing i?c bus not create a whole new layer of goop.
>
> It does need to do the plumbing and resource management for the plumbing
> but thats not the same as being a bus.
>
> Alan


Fair enough. But there's no such thing a 'hotplug enumeration construct' in Linux
yet, and a bus is the closest thing to it. It does take advantage of the nice
way device code matches drivers and devices though.

I'm afraid that having the I2C/SPI drivers doing the detection won't work.
The capes can have arbitrary I2C/SPI devices (and even more weird components).
There is no way to assure for example that the I2C device responding to address 'foo'
in cape A is the same I2C device responding to the same address in cape B.

In a nutshell, no, I'm not writing any custom drivers for the stuff that exists
already. There is no extra layer after the device is created and operates as it was
located in the proper place in the DTS.

The custom drivers are only needed for complex capes where you can't
have the device simply work by creating the devices.

Regards

-- Pantelis



2012-11-01 14:36:15

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH 2/3] da8xx-dt: Create da8xx DT adapter device

On 2012-11-01 17:18, Pantelis Antoniou wrote:
> omap_device is going private.
>
> Move the da8xx-dt adapter device to arch/arm/mach-omap2.
>
> Signed-off-by: Pantelis Antoniou <[email protected]>


> +
> + /* display_panel */
> + priv->lcd_panel.panel_type = QVGA;
> + priv->lcd_panel.max_bpp = 16;
> + priv->lcd_panel.min_bpp = 16;
> + priv->lcd_panel.panel_shade = COLOR_ACTIVE;
> +
> + /* lcd_ctrl_config */
> + priv->lcd_cfg.p_disp_panel = &priv->lcd_panel;
> + priv->lcd_cfg.ac_bias = 255;
> + priv->lcd_cfg.ac_bias_intrpt = 0;
> + priv->lcd_cfg.dma_burst_sz = 16;
> + priv->lcd_cfg.bpp = 16;
> + priv->lcd_cfg.fdd = 0x80;
> + priv->lcd_cfg.tft_alt_mode = 0;
> + priv->lcd_cfg.stn_565_mode = 0;
> + priv->lcd_cfg.mono_8bit_mode = 0;
> + priv->lcd_cfg.invert_line_clock = 1;
> + priv->lcd_cfg.invert_frm_clock = 1;
> + priv->lcd_cfg.sync_edge = 0;
> + priv->lcd_cfg.sync_ctrl = 1;
> + priv->lcd_cfg.raster_order = 0;
> +
> + /* da8xx_lcdc_platform_data */
> + strcpy(priv->lcd_pdata.manu_name, "BBToys");
> + priv->lcd_pdata.controller_data = &priv->lcd_cfg;
> + strcpy(priv->lcd_pdata.type, panel_type);
> +
> + priv->lcdc_oh = omap_hwmod_lookup("lcdc");
> + if (priv->lcdc_oh == NULL) {
> + dev_err(&pdev->dev, "Failed to lookup omap_hwmod lcdc\n");
> + return -ENODEV;
> + }
> +
> + priv->lcdc_pdev = omap_device_build("da8xx_lcdc", 0, priv->lcdc_oh,
> + &priv->lcd_pdata,
> + sizeof(struct da8xx_lcdc_platform_data),
> + NULL, 0, 0);
> + if (priv->lcdc_pdev == NULL) {
> + dev_err(&pdev->dev, "Failed to build LCDC device\n");
> + return -ENODEV;
> + }

I know nothing about BeagleBone, da8xx_lcdc, or the capes, but here are
my thoughts... =)

If I understood this cape-thing correctly, to handle the LCDs properly
the da8xx_lcdc driver should be changed to be more dynamic.

The da8xx_lcdc device should be always added by the standard SoC code.
If the LCD panel is fixed for the board in question, the lcdc could be
passed the LCD data as done now (I guess).

But in beaglebone's case the platform data for lcdc should be empty, and
the lcdc driver should do just minimal setup, like memmapping its
registers. No pinmuxing nor other such things.

When the cape is identified and the cape's driver is loaded, the driver
would configure lcdc depending on the cape, including pinmuxing. After
that the LCD should be functional.

Tomi

2012-11-01 14:38:11

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 2/3] da8xx-dt: Create da8xx DT adapter device

Hi Tomi,

On Nov 1, 2012, at 4:36 PM, Tomi Valkeinen wrote:

> On 2012-11-01 17:18, Pantelis Antoniou wrote:
>> omap_device is going private.
>>
>> Move the da8xx-dt adapter device to arch/arm/mach-omap2.
>>
>> Signed-off-by: Pantelis Antoniou <[email protected]>
>
>
>> +
>> + /* display_panel */
>> + priv->lcd_panel.panel_type = QVGA;
>> + priv->lcd_panel.max_bpp = 16;
>> + priv->lcd_panel.min_bpp = 16;
>> + priv->lcd_panel.panel_shade = COLOR_ACTIVE;
>> +
>> + /* lcd_ctrl_config */
>> + priv->lcd_cfg.p_disp_panel = &priv->lcd_panel;
>> + priv->lcd_cfg.ac_bias = 255;
>> + priv->lcd_cfg.ac_bias_intrpt = 0;
>> + priv->lcd_cfg.dma_burst_sz = 16;
>> + priv->lcd_cfg.bpp = 16;
>> + priv->lcd_cfg.fdd = 0x80;
>> + priv->lcd_cfg.tft_alt_mode = 0;
>> + priv->lcd_cfg.stn_565_mode = 0;
>> + priv->lcd_cfg.mono_8bit_mode = 0;
>> + priv->lcd_cfg.invert_line_clock = 1;
>> + priv->lcd_cfg.invert_frm_clock = 1;
>> + priv->lcd_cfg.sync_edge = 0;
>> + priv->lcd_cfg.sync_ctrl = 1;
>> + priv->lcd_cfg.raster_order = 0;
>> +
>> + /* da8xx_lcdc_platform_data */
>> + strcpy(priv->lcd_pdata.manu_name, "BBToys");
>> + priv->lcd_pdata.controller_data = &priv->lcd_cfg;
>> + strcpy(priv->lcd_pdata.type, panel_type);
>> +
>> + priv->lcdc_oh = omap_hwmod_lookup("lcdc");
>> + if (priv->lcdc_oh == NULL) {
>> + dev_err(&pdev->dev, "Failed to lookup omap_hwmod lcdc\n");
>> + return -ENODEV;
>> + }
>> +
>> + priv->lcdc_pdev = omap_device_build("da8xx_lcdc", 0, priv->lcdc_oh,
>> + &priv->lcd_pdata,
>> + sizeof(struct da8xx_lcdc_platform_data),
>> + NULL, 0, 0);
>> + if (priv->lcdc_pdev == NULL) {
>> + dev_err(&pdev->dev, "Failed to build LCDC device\n");
>> + return -ENODEV;
>> + }
>
> I know nothing about BeagleBone, da8xx_lcdc, or the capes, but here are
> my thoughts... =)
>
> If I understood this cape-thing correctly, to handle the LCDs properly
> the da8xx_lcdc driver should be changed to be more dynamic.
>
> The da8xx_lcdc device should be always added by the standard SoC code.
> If the LCD panel is fixed for the board in question, the lcdc could be
> passed the LCD data as done now (I guess).
>
> But in beaglebone's case the platform data for lcdc should be empty, and
> the lcdc driver should do just minimal setup, like memmapping its
> registers. No pinmuxing nor other such things.
>
> When the cape is identified and the cape's driver is loaded, the driver
> would configure lcdc depending on the cape, including pinmuxing. After
> that the LCD should be functional.
>
> Tomi
>

You are absolutely correct. That's the proper way to go about it,
but you go to war with the drivers you have, and not the drivers you wish
you had :)

Meaning, it is getting fixed by the da8xx maintainers, but it's not ready yet.

The da8xx-dt can die a short and merciful death then.

Regards

-- Pantelis-

2012-11-01 18:50:23

by Jason Kridner

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

My apologies for starting a new thread, but I don't have this thread
in my Inbox.

http://www.spinics.net/lists/linux-omap/msg81034.html

Tony Lindgren wrote:

>* Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx> [121031 15:02]:
>>
>> So when device's node is 'disabled' of_platform_device_create_pdata()
>> will not create the device.
>>
>> Now, of course it is possible to re-trigger the platform's probe method
>> to be called, and in fact I do so in the capebus patches.
>
>You should fix this in generic way then rather than working
>around it in capebus. The same problem exists changing
>between different functionality for the shared pins,
>let's say between USB pins and UART pins if you want a
>serial debug console on some phone.

The current capebus solution goes a long way to fixing a huge issue
for BeagleBone users and I don't understand what seems to be a
push-back on principle. On BeagleBone capes, these conflicts cannot be
resolved early.

Do you have suggestions on some more generic method? It seems to me
the proposed capebus approach strikes a good balance.

2012-11-01 19:07:40

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

* Jason Kridner <[email protected]> [121101 11:52]:
> My apologies for starting a new thread, but I don't have this thread
> in my Inbox.
>
> http://www.spinics.net/lists/linux-omap/msg81034.html
>
> Tony Lindgren wrote:
>
> >* Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx> [121031 15:02]:
> >>
> >> So when device's node is 'disabled' of_platform_device_create_pdata()
> >> will not create the device.
> >>
> >> Now, of course it is possible to re-trigger the platform's probe method
> >> to be called, and in fact I do so in the capebus patches.
> >
> >You should fix this in generic way then rather than working
> >around it in capebus. The same problem exists changing
> >between different functionality for the shared pins,
> >let's say between USB pins and UART pins if you want a
> >serial debug console on some phone.
>
> The current capebus solution goes a long way to fixing a huge issue
> for BeagleBone users and I don't understand what seems to be a
> push-back on principle. On BeagleBone capes, these conflicts cannot be
> resolved early.
>
> Do you have suggestions on some more generic method? It seems to me
> the proposed capebus approach strikes a good balance.

Having it all behave like a hotpluggable bus makes sense.

But the way to sort it out is to have all the omap internal
devices defined in a .dts file and have them set initially
with status = "disabled" in the .dts.

Then you just need a function dynamically change the kernel
internal device tree to enable and disable devices dynamically
so the dev entries get created and driver probe can happen.

Sure that means that some hwmod and omap_device functions
can't be __init any longer, but there should not be any
need to call hwmod and omap_device functions directly from
capebus.

If you want to take it one step further, you can also add
new capes to the kernel internal device tree dynamically
as you may have different pinmux and omap internal device
configurations on the capes.

Regards,

Tony

2012-11-01 22:11:28

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

HI,

On Thu, Nov 01, 2012 at 03:59:50PM +0200, Pantelis Antoniou wrote:
> Hi Alan,
>
> On Nov 1, 2012, at 3:51 PM, Alan Cox wrote:
>
> >> What they want, and what every user wants, is I plug this board in, and
> >> the driver make sure everything is loaded and ready. No, the end users
> >> don't want to see any of the implementation details of how the bitfile
> >> is transported; the driver can handle it.
> >
> > That doesn't necessarily make it a bus merely some kind of hotplug
> > enumeration of devices. That should all work properly both for devices
> > and busses with spi and i?c as the final bits needed for it got fixed
> > some time ago.
> >
> > In an ideal world you don't want to be writing custom drivers for stuff.
> > If your cape routes an i?c serial device to the existing system i?c
> > busses then you want to just create an instance of any existing driver on
> > the existing i?c bus not create a whole new layer of goop.
> >
> > It does need to do the plumbing and resource management for the plumbing
> > but thats not the same as being a bus.
> >
> > Alan
>
>
> Fair enough. But there's no such thing a 'hotplug enumeration
> construct' in Linux yet, and a bus is the closest thing to it. It does
> take advantage of the nice way device code matches drivers and devices
> though.
>
> I'm afraid that having the I2C/SPI drivers doing the detection won't
> work. The capes can have arbitrary I2C/SPI devices (and even more
> weird components). There is no way to assure for example that the I2C
> device responding to address 'foo' in cape A is the same I2C device
> responding to the same address in cape B.

your ->detect() method should take care of that.

--
balbi


Attachments:
(No filename) (1.67 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-11-01 23:49:27

by Russ Dill

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Thu, Nov 1, 2012 at 3:05 PM, Felipe Balbi <[email protected]> wrote:
> HI,
>
> On Thu, Nov 01, 2012 at 03:59:50PM +0200, Pantelis Antoniou wrote:
>> Hi Alan,
>>
>> On Nov 1, 2012, at 3:51 PM, Alan Cox wrote:
>>
>> >> What they want, and what every user wants, is I plug this board in, and
>> >> the driver make sure everything is loaded and ready. No, the end users
>> >> don't want to see any of the implementation details of how the bitfile
>> >> is transported; the driver can handle it.
>> >
>> > That doesn't necessarily make it a bus merely some kind of hotplug
>> > enumeration of devices. That should all work properly both for devices
>> > and busses with spi and i²c as the final bits needed for it got fixed
>> > some time ago.
>> >
>> > In an ideal world you don't want to be writing custom drivers for stuff.
>> > If your cape routes an i²c serial device to the existing system i²c
>> > busses then you want to just create an instance of any existing driver on
>> > the existing i²c bus not create a whole new layer of goop.
>> >
>> > It does need to do the plumbing and resource management for the plumbing
>> > but thats not the same as being a bus.
>> >
>> > Alan
>>
>>
>> Fair enough. But there's no such thing a 'hotplug enumeration
>> construct' in Linux yet, and a bus is the closest thing to it. It does
>> take advantage of the nice way device code matches drivers and devices
>> though.
>>
>> I'm afraid that having the I2C/SPI drivers doing the detection won't
>> work. The capes can have arbitrary I2C/SPI devices (and even more
>> weird components). There is no way to assure for example that the I2C
>> device responding to address 'foo' in cape A is the same I2C device
>> responding to the same address in cape B.
>
> your ->detect() method should take care of that.

There isn't some magical serial number in I²C devices that a
->detect() method can read and the implementation of I²C is somewhat
flexible. One devices read may be another devices write. A detect
method that only performs reads could easily toggle a gpio that resets
the board, rewrite and eeprom, or set the printer on fire. If you
browse through various detect functions, yes, some of them key off an
ID, but a lot of them just check various registers to see if certain
bits are zero, or certain bits are one. A lot of I²C devices I've
dealt with have no good way of probing them, especially GPIO chips
(you'll notice none of the I²C GPIO expanders have detect functions)

On top of all this the detect routine does not tell you if the alert
pin is connected to some IRQ, or in the case of a GPIO expander, what
those GPIOs are connected to, etc, etc.

2012-11-02 08:16:41

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On 11/1/2012 1:00 PM, Koen Kooi wrote:
> tl;dr: please suggest an actual solution that allows plug&play when plugging in multiple capes and applying power after that. Preferably one that doesn't pass the buck to u-boot.
>
> Op 1 nov. 2012, om 12:26 heeft "Cousson, Benoit" <[email protected]> het volgende geschreven:
>
>> Hi Panto,
>>
>> On 11/1/2012 11:39 AM, Pantelis Antoniou wrote:
>>> Hi Benoit,
>>>
>>> On Nov 1, 2012, at 12:23 PM, Cousson, Benoit wrote:
>>>
>>>> On 11/1/2012 8:02 AM, Pantelis Antoniou wrote:
>>>>> Hi Felibe,
>>>>>
>>>>> On Nov 1, 2012, at 12:14 AM, Felipe Balbi wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
>>>>>>>> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>>>>>>>>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>>>>>>>>
>>>>>>>>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>>>>>>>>> could have an hwmod and thus must be handled by an omap_device.
>>>>>>>>>>
>>>>>>>>>> Any devices that is created later cannot be omap_device. The DT core
>>>>>>>>>> will create regular platform_device for them.
>>>>>>>>>>
>>>>>>>>>> Since cape is an external board, it should have nothing to do with
>>>>>>>>>> omap_device.
>>>>>>>>>>
>>>>>>>>>> Looking at your patch:
>>>>>>>>>> da8xx-dt: Create da8xx DT adapter device
>>>>>>>>>>
>>>>>>>>>> I understand why you do that, but in fact that patch does not make sense
>>>>>>>>>> to me :-(
>>>>>>>>>>
>>>>>>>>>> Why do you have to create an omap_device from the driver probe?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The problem is that capes are not external boards in the normal sense
>>>>>>>>> as a PCI board is. In the PCI case the hardware that implements the
>>>>>>>>> desired functionality is on the PCI board, while in the cape case the
>>>>>>>>> hardware module is in the SoC. The cape most of the times is quite
>>>>>>>>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>>>>>>>>
>>>>>>>> Ah now I see, you're talking about the beaglebone extension
>>>>>>>> boards :)
>>>>>>>>
>>>>>>>> The way to deal with those properly is to just edit the
>>>>>>>> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
>>>>>>>> whatever.
>>>>>>>>
>>>>>>>
>>>>>>> That is what is being done...
>>>>>>> This is the fallout.
>>>>>>>
>>>>>>>>> You can't instantiate the omap_device early in the boot process like it was done up to
>>>>>>>>> now in the board file. You can only do that later in the boot process (for built-in
>>>>>>>>> cape drivers), or even after user-space has booted and the matching cape driver module
>>>>>>>>> has been loaded.
>>>>>>>>
>>>>>>>> Yes you can, just edit the .dts file for the extension
>>>>>>>> board you have connected. This stuff should be DT
>>>>>>>> only for sure, let's not even start adding platform_data
>>>>>>>> entries for that.
>>>>>>>>
>>>>>>>> The omap_device entries for the omap internal devices
>>>>>>>> will be created automatically during startup based on the
>>>>>>>> .dts. Note that you can set status = "disabled" for the
>>>>>>>> omap internal devices in the .dts file, the devices are
>>>>>>>> there in the hardware.
>>>>>>>>
>>>>>>>> If you are sure the extension boards are safely hot
>>>>>>>> pluggable, then all you need is some interface to enable
>>>>>>>> and disable devices after booting. But that should be
>>>>>>>> done in Linux generic way.
>>>>>>>>
>>>>>>>> So we should not export any omap_hwmod or omap_device
>>>>>>>> things, and want to keep it __init only, and local to
>>>>>>>> arch/arm/mach-omap2.
>>>>>>>>
>>>>>>>
>>>>>>> I disagree. This is not something that is handled by just
>>>>>>> editing the dts. The way it is handled, is in the Linux
>>>>>>> generic way, we have a proper bus, and the drivers as compiled
>>>>>>> as standalone file.
>>>>>>
>>>>>> when you have an actual bus, that'd be correct.
>>>>>
>>>>> What do you think capebus is? It is an actual bus that allows you
>>>>> to do so.
>>>>>
>>>>>>
>>>>>>> We're hitting a use case that wasn't there in omap until now.
>>>>>>>
>>>>>>> There a a whole bunch of conflicting capes. There's no
>>>>>>> way to instantiate them together. They must be instantiated
>>>>>>> only after their EEPROMs are read and they are matched
>>>>>>> to their corresponding cape drivers.
>>>>>>
>>>>>> why this requirement of instantiating them only after reading EEPROMs ?
>>>>>>
>>>>>> It's unnecessary to add that requirement, just do what Tony said
>>>>>> (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
>>>>>> should be created automatically.
>>>>>>
>>>>>> The thing is that there is no such thing as a cape-device. The cape is
>>>>>> just a collection of I2C, 1-wire and SPI devices anyway. What we should
>>>>>> instantiate is bmp085, tls2550, sht21, instead of instantiating
>>>>>> beagle-bone-weather-cape.
>>>>>>
>>>>>> What's the problem in just instantiating all of those from bone.dts ?
>>>>>> Expose the EEPROMs to userland so whatever SW you guys have running on
>>>>>> the bone can figure out what features to expose to the SDK which the
>>>>>> user sees, but the kernel doesn't need to know that there is a
>>>>>> weather-cape attached to the bone.
>>>>>>
>>>>>
>>>>> The I2C/SPI devices are not a problem at all.
>>>>>
>>>>> Let me try to explain what the problem is, and why it all this
>>>>> is needed.
>>>>>
>>>>> First of all, capes may be relatively simple boards, which may function
>>>>> as a generic device - like the generic capes. They might not necessarily
>>>>> be simple. Some capes, for example the geiger cape have a number of
>>>>> components that perform a specific task; in this instance the driving
>>>>> circuit of the geiger tube and the event detector input. Other capes
>>>>> that are being designed now are even more complex, i.e. there's a
>>>>> radar cape, an fpga cape and so on. So for these kind of boards
>>>>> you will need a specific driver. That driver will probably use some
>>>>> generic-cape bits (like gpios, pwms, keys what-ever).
>>>>> But it will put them to use in the custom in-kernel driver in it's own
>>>>> way. You can't put that task to user-space, if it's ever slightly complex.
>>>>>
>>>>> So for really simple capes, after considerable pain you might, just
>>>>> might, dump the problem to user-space and try to make it work.
>>>>> People have tried that and it's a total pain. There is no way that this will
>>>>> work with anything more complex than a generic cape.
>>>>>
>>>>> Then there's the out of the box experience problem.
>>>>>
>>>>> What a customer that buys one of these boards along with a number of
>>>>> capes wants is to plug them, and have them work. You cannot have
>>>>> the DTS file activating all the drivers for each possible cape since
>>>>> they conflict; the pins that one SPI bus that one cape uses might be a
>>>>> PWM output for another, and so on.
>>>>>
>>>>> Asking the customer to edit the distro supplied kernel's dts and recompiling,
>>>>> while figuring out how to solve conflicts is not going to fly.
>>>>
>>>> Well you do not have to rebuild the kernel if you just want to update the DTS.
>>>> The way DT is done, the best you can do is to rely on the bootloader in your case to select the proper DTB. You should try to merge dynamically AMxx + beaglebone + capeXXX based on some EEPROM id.
>>>>
>>>
>>> That is what the capebus does. It inserts
>>
>> It inserts what?
>>
>>>> FWIW, we do have a similar, but simpler, problem with the beagle / beagle-xm and panda / panda-es. But for the moment we are just using a different DTS for each board.
>>>
>>> A different DTS for each board combination... There alot more capes for the bone that they are for the beagle & the panda.
>>> And more are going to come, but not necessarily from people that have any connection to TI or CCO.
>>
>> Sure, but my point is that your solution will not solve our problem :-)
>> This is a generic problem that you address with a very custom / specific approach.
>>
>>> You still haven't described how I could solve the issue of conflicting capes using a single DTS.
>>
>> Well I don't have the solution otherwise I will have done it already :-)
>> My point is that the solution has to be in the DT core if not in the bootloader.
>
> <snarky comment>So when we at beagleboard.org handled the beagleboard and beagleboard xM expansionboards in the bootloader (detection, muxing, etc) we were told it was wrong and we should handle it in the kernel and if we waited a bit, DT would solve everything. And now that we actually handle it in the kernel you are saying that it is wrong and we should handle it in the bootloader and that DT won't solve everything. I guess someone will now tell us that uEFI will fix everything.</snarky comment>

Well, at that time I guess people were not that aware of the DT limitation.
In fact at that time, this kind of stuff were done by bootloader.

But I'm quite sure uEFI will fix that :-)

The point being, there is indeed no proper solution as of today beside
the bootloader hack.

> Apart from that, you and others still fail to tell us how you want to handle a user (or customer) that buys a beaglebone, an LCD cape, a weatherstation cape and a geiger counter cape and plugs those together and powers them on. With the evil TI vendor tree and extra patches the boardfile reads the eeproms and tries its best to instantiate all the platform data. One of the capes won't have working LEDs since appending to the leds-gpio struct is pretty much impossible in this situation. But it gets a picture on the screen, blinks LEDs and does largely what the user expects.

As I told Panto, it is not like it is a straightforward problem. My
point is still valid, this is a generic issue that should not be fixed
using some capebus hack.

> With capebus we get:
>
> 1) da8xx-fb which lacks DT bindingsp[1] receives plaftorm data to match the LCD
> 2) the i2c sensors on the weathercape are registered
> 3) the one-wire bus on the weathercape gets registered
> 4) the LEDs on the lcd cape get registered5
> 5) the LED on the geigercape gets registered and adds a custom trigger
> 6) the ADC, which again, lacks DT bindings[2], receives plaftorm data and a custom IIO binding
> 7) pinctrl sets the pinmuxes for the above
>
> In other words: plug & play, even for devices with drivers that are still lacking DT support.
>
> Now please explain to me why you think it's such an awesome idea that the users will need to find the right dtsi files (multiple revisions of the lcd cape exist), include them in the dts, run dtc, add a few missing semicolons, run dtc again, copy it over to /boot and reboot to have a change of making it work?

Because, anyway, any new complex capes will require a new DTS to handle
it if it requires some parameters.
The whole issue is the lack of discoverable bus, and there is nothing
you can do to fix that.

> I know you can't escape that for new or custom capes, but I do want all the capes my company assembles work out of the box. (Cross)compiling a kernel is a bridge too far for 95% of the intended audience.

The whole point of DTS is that you don't have to recompile to describe a
new board. So why do you want to compile a kernel?

> With capebus most capes can be supported by editing the DTS, your bootloader proposal involves updating u-boot for every new cape as well. That is downright scary. The RMA department will get flooded.

Fair enough, hacking boot loader sucks.

> More importantly: capebus is generic enough to work on beagleboard, beagleboard xm, panda, panda es and even raspberry-pi. Basically on any DT capable platform.

Then make it part of DT core.

If you do want a pure dynamic stuff that minimize rebuild kernel,
potentially loadable module and loadable DTB sub tree makes more sense
than the capebus infrastructure.

The whole point here is to be able to build a DT dynamically.

We can boot with the default one.

beaglebone.dts:
/{
i2c@0 {
status = "disabled";
};

spi@0 {
status = "disabled";
};

gmpc {
status = "disabled";
};
}


And then later load that one using a bone-cape driver that can detect
revision.

super-cape.dts:
{
i2c@0 {
toaster@42 {
blabla...
};
};

gmpc {
mega-fpga@42 {
blabla...
};
};
}

And then use the standard DT support to create later the platform_device
that does represent the new super-cape devices.

Regards,
Benoit

2012-11-02 08:43:45

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Benoit,

On Nov 2, 2012, at 10:15 AM, Cousson, Benoit wrote:

> On 11/1/2012 1:00 PM, Koen Kooi wrote:
>> tl;dr: please suggest an actual solution that allows plug&play when plugging in multiple capes and applying power after that. Preferably one that doesn't pass the buck to u-boot.
>>
>> Op 1 nov. 2012, om 12:26 heeft "Cousson, Benoit" <[email protected]> het volgende geschreven:
>>
>>> Hi Panto,
>>>
>>> On 11/1/2012 11:39 AM, Pantelis Antoniou wrote:
>>>> Hi Benoit,
>>>>
>>>> On Nov 1, 2012, at 12:23 PM, Cousson, Benoit wrote:
>>>>
>>>>> On 11/1/2012 8:02 AM, Pantelis Antoniou wrote:
>>>>>> Hi Felibe,
>>>>>>
>>>>>> On Nov 1, 2012, at 12:14 AM, Felipe Balbi wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Wed, Oct 31, 2012 at 11:36:25PM +0200, Pantelis Antoniou wrote:
>>>>>>>>> * Pantelis Antoniou <[email protected]> [121031 13:14]:
>>>>>>>>>> On Oct 31, 2012, at 9:55 PM, Benoit Cousson wrote:
>>>>>>>>>>>
>>>>>>>>>>> Yeah, I do agree. I'm confused as well. Only OMAP IPs under PRCM control
>>>>>>>>>>> could have an hwmod and thus must be handled by an omap_device.
>>>>>>>>>>>
>>>>>>>>>>> Any devices that is created later cannot be omap_device. The DT core
>>>>>>>>>>> will create regular platform_device for them.
>>>>>>>>>>>
>>>>>>>>>>> Since cape is an external board, it should have nothing to do with
>>>>>>>>>>> omap_device.
>>>>>>>>>>>
>>>>>>>>>>> Looking at your patch:
>>>>>>>>>>> da8xx-dt: Create da8xx DT adapter device
>>>>>>>>>>>
>>>>>>>>>>> I understand why you do that, but in fact that patch does not make sense
>>>>>>>>>>> to me :-(
>>>>>>>>>>>
>>>>>>>>>>> Why do you have to create an omap_device from the driver probe?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The problem is that capes are not external boards in the normal sense
>>>>>>>>>> as a PCI board is. In the PCI case the hardware that implements the
>>>>>>>>>> desired functionality is on the PCI board, while in the cape case the
>>>>>>>>>> hardware module is in the SoC. The cape most of the times is quite
>>>>>>>>>> simple and contains passive components, leds or some kind of I2C/SPI devices.
>>>>>>>>>
>>>>>>>>> Ah now I see, you're talking about the beaglebone extension
>>>>>>>>> boards :)
>>>>>>>>>
>>>>>>>>> The way to deal with those properly is to just edit the
>>>>>>>>> board .dts entry to include omap-beaglebone-cape-xyz.dtsi
>>>>>>>>> whatever.
>>>>>>>>>
>>>>>>>>
>>>>>>>> That is what is being done...
>>>>>>>> This is the fallout.
>>>>>>>>
>>>>>>>>>> You can't instantiate the omap_device early in the boot process like it was done up to
>>>>>>>>>> now in the board file. You can only do that later in the boot process (for built-in
>>>>>>>>>> cape drivers), or even after user-space has booted and the matching cape driver module
>>>>>>>>>> has been loaded.
>>>>>>>>>
>>>>>>>>> Yes you can, just edit the .dts file for the extension
>>>>>>>>> board you have connected. This stuff should be DT
>>>>>>>>> only for sure, let's not even start adding platform_data
>>>>>>>>> entries for that.
>>>>>>>>>
>>>>>>>>> The omap_device entries for the omap internal devices
>>>>>>>>> will be created automatically during startup based on the
>>>>>>>>> .dts. Note that you can set status = "disabled" for the
>>>>>>>>> omap internal devices in the .dts file, the devices are
>>>>>>>>> there in the hardware.
>>>>>>>>>
>>>>>>>>> If you are sure the extension boards are safely hot
>>>>>>>>> pluggable, then all you need is some interface to enable
>>>>>>>>> and disable devices after booting. But that should be
>>>>>>>>> done in Linux generic way.
>>>>>>>>>
>>>>>>>>> So we should not export any omap_hwmod or omap_device
>>>>>>>>> things, and want to keep it __init only, and local to
>>>>>>>>> arch/arm/mach-omap2.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I disagree. This is not something that is handled by just
>>>>>>>> editing the dts. The way it is handled, is in the Linux
>>>>>>>> generic way, we have a proper bus, and the drivers as compiled
>>>>>>>> as standalone file.
>>>>>>>
>>>>>>> when you have an actual bus, that'd be correct.
>>>>>>
>>>>>> What do you think capebus is? It is an actual bus that allows you
>>>>>> to do so.
>>>>>>
>>>>>>>
>>>>>>>> We're hitting a use case that wasn't there in omap until now.
>>>>>>>>
>>>>>>>> There a a whole bunch of conflicting capes. There's no
>>>>>>>> way to instantiate them together. They must be instantiated
>>>>>>>> only after their EEPROMs are read and they are matched
>>>>>>>> to their corresponding cape drivers.
>>>>>>>
>>>>>>> why this requirement of instantiating them only after reading EEPROMs ?
>>>>>>>
>>>>>>> It's unnecessary to add that requirement, just do what Tony said
>>>>>>> (include my-awesome-cape.dtsi to bone.dts) and all i2c/spi devices
>>>>>>> should be created automatically.
>>>>>>>
>>>>>>> The thing is that there is no such thing as a cape-device. The cape is
>>>>>>> just a collection of I2C, 1-wire and SPI devices anyway. What we should
>>>>>>> instantiate is bmp085, tls2550, sht21, instead of instantiating
>>>>>>> beagle-bone-weather-cape.
>>>>>>>
>>>>>>> What's the problem in just instantiating all of those from bone.dts ?
>>>>>>> Expose the EEPROMs to userland so whatever SW you guys have running on
>>>>>>> the bone can figure out what features to expose to the SDK which the
>>>>>>> user sees, but the kernel doesn't need to know that there is a
>>>>>>> weather-cape attached to the bone.
>>>>>>>
>>>>>>
>>>>>> The I2C/SPI devices are not a problem at all.
>>>>>>
>>>>>> Let me try to explain what the problem is, and why it all this
>>>>>> is needed.
>>>>>>
>>>>>> First of all, capes may be relatively simple boards, which may function
>>>>>> as a generic device - like the generic capes. They might not necessarily
>>>>>> be simple. Some capes, for example the geiger cape have a number of
>>>>>> components that perform a specific task; in this instance the driving
>>>>>> circuit of the geiger tube and the event detector input. Other capes
>>>>>> that are being designed now are even more complex, i.e. there's a
>>>>>> radar cape, an fpga cape and so on. So for these kind of boards
>>>>>> you will need a specific driver. That driver will probably use some
>>>>>> generic-cape bits (like gpios, pwms, keys what-ever).
>>>>>> But it will put them to use in the custom in-kernel driver in it's own
>>>>>> way. You can't put that task to user-space, if it's ever slightly complex.
>>>>>>
>>>>>> So for really simple capes, after considerable pain you might, just
>>>>>> might, dump the problem to user-space and try to make it work.
>>>>>> People have tried that and it's a total pain. There is no way that this will
>>>>>> work with anything more complex than a generic cape.
>>>>>>
>>>>>> Then there's the out of the box experience problem.
>>>>>>
>>>>>> What a customer that buys one of these boards along with a number of
>>>>>> capes wants is to plug them, and have them work. You cannot have
>>>>>> the DTS file activating all the drivers for each possible cape since
>>>>>> they conflict; the pins that one SPI bus that one cape uses might be a
>>>>>> PWM output for another, and so on.
>>>>>>
>>>>>> Asking the customer to edit the distro supplied kernel's dts and recompiling,
>>>>>> while figuring out how to solve conflicts is not going to fly.
>>>>>
>>>>> Well you do not have to rebuild the kernel if you just want to update the DTS.
>>>>> The way DT is done, the best you can do is to rely on the bootloader in your case to select the proper DTB. You should try to merge dynamically AMxx + beaglebone + capeXXX based on some EEPROM id.
>>>>>
>>>>
>>>> That is what the capebus does. It inserts
>>>
>>> It inserts what?
>>>
>>>>> FWIW, we do have a similar, but simpler, problem with the beagle / beagle-xm and panda / panda-es. But for the moment we are just using a different DTS for each board.
>>>>
>>>> A different DTS for each board combination... There alot more capes for the bone that they are for the beagle & the panda.
>>>> And more are going to come, but not necessarily from people that have any connection to TI or CCO.
>>>
>>> Sure, but my point is that your solution will not solve our problem :-)
>>> This is a generic problem that you address with a very custom / specific approach.
>>>
>>>> You still haven't described how I could solve the issue of conflicting capes using a single DTS.
>>>
>>> Well I don't have the solution otherwise I will have done it already :-)
>>> My point is that the solution has to be in the DT core if not in the bootloader.
>>
>> <snarky comment>So when we at beagleboard.org handled the beagleboard and beagleboard xM expansionboards in the bootloader (detection, muxing, etc) we were told it was wrong and we should handle it in the kernel and if we waited a bit, DT would solve everything. And now that we actually handle it in the kernel you are saying that it is wrong and we should handle it in the bootloader and that DT won't solve everything. I guess someone will now tell us that uEFI will fix everything.</snarky comment>
>
> Well, at that time I guess people were not that aware of the DT limitation.
> In fact at that time, this kind of stuff were done by bootloader.
>
> But I'm quite sure uEFI will fix that :-)
>
> The point being, there is indeed no proper solution as of today beside the bootloader hack.
>
>> Apart from that, you and others still fail to tell us how you want to handle a user (or customer) that buys a beaglebone, an LCD cape, a weatherstation cape and a geiger counter cape and plugs those together and powers them on. With the evil TI vendor tree and extra patches the boardfile reads the eeproms and tries its best to instantiate all the platform data. One of the capes won't have working LEDs since appending to the leds-gpio struct is pretty much impossible in this situation. But it gets a picture on the screen, blinks LEDs and does largely what the user expects.
>
> As I told Panto, it is not like it is a straightforward problem. My point is still valid, this is a generic issue that should not be fixed using some capebus hack.
>
>> With capebus we get:
>>
>> 1) da8xx-fb which lacks DT bindingsp[1] receives plaftorm data to match the LCD
>> 2) the i2c sensors on the weathercape are registered
>> 3) the one-wire bus on the weathercape gets registered
>> 4) the LEDs on the lcd cape get registered5
>> 5) the LED on the geigercape gets registered and adds a custom trigger
>> 6) the ADC, which again, lacks DT bindings[2], receives plaftorm data and a custom IIO binding
>> 7) pinctrl sets the pinmuxes for the above
>>
>> In other words: plug & play, even for devices with drivers that are still lacking DT support.
>>
>> Now please explain to me why you think it's such an awesome idea that the users will need to find the right dtsi files (multiple revisions of the lcd cape exist), include them in the dts, run dtc, add a few missing semicolons, run dtc again, copy it over to /boot and reboot to have a change of making it work?
>
> Because, anyway, any new complex capes will require a new DTS to handle it if it requires some parameters.
> The whole issue is the lack of discoverable bus, and there is nothing you can do to fix that.
>
>> I know you can't escape that for new or custom capes, but I do want all the capes my company assembles work out of the box. (Cross)compiling a kernel is a bridge too far for 95% of the intended audience.
>
> The whole point of DTS is that you don't have to recompile to describe a new board. So why do you want to compile a kernel?
>
>> With capebus most capes can be supported by editing the DTS, your bootloader proposal involves updating u-boot for every new cape as well. That is downright scary. The RMA department will get flooded.
>
> Fair enough, hacking boot loader sucks.
>
>> More importantly: capebus is generic enough to work on beagleboard, beagleboard xm, panda, panda es and even raspberry-pi. Basically on any DT capable platform.
>
> Then make it part of DT core.
>
> If you do want a pure dynamic stuff that minimize rebuild kernel, potentially loadable module and loadable DTB sub tree makes more sense than the capebus infrastructure.
>
> The whole point here is to be able to build a DT dynamically.
>
> We can boot with the default one.
>
> beaglebone.dts:
> /{
> i2c@0 {
> status = "disabled";
> };
>
> spi@0 {
> status = "disabled";
> };
>
> gmpc {
> status = "disabled";
> };
> }
>
>
> And then later load that one using a bone-cape driver that can detect revision.
>
> super-cape.dts:
> {
> i2c@0 {
> toaster@42 {
> blabla...
> };
> };
>
> gmpc {
> mega-fpga@42 {
> blabla...
> };
> };
> }
>
> And then use the standard DT support to create later the platform_device that does represent the new super-cape devices.
>

We know this is the ideal case. In fact that's the long term goal and we had internal discussions about it.

DT is nowhere near being able to do it.

That would require the introduction of a DT object file format with aliases being capable to be
resolved dynamically. We're talking about major changes here in the way DT files are being compiled and
used in practice.

So yes, in an ideal world that would be great. We're nowhere near close today unfortunately.

Assuming that we do work on a DT object format, and that the runtime resolution mechanism is approved,
then I agree that this part of the capebus patches can be dropped and the functionality assumed by generic
DT core.

The question is that this will take time, with no guarantees that this would be acceptable from
the device tree maintainers. So I am putting them in the CC list, to see what they think about it.

> Regards,
> Benoit
>

Regards

-- Pantelis

2012-11-02 09:03:31

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi,

On Thu, Nov 01, 2012 at 04:49:23PM -0700, Russ Dill wrote:
> On Thu, Nov 1, 2012 at 3:05 PM, Felipe Balbi <[email protected]> wrote:
> > HI,
> >
> > On Thu, Nov 01, 2012 at 03:59:50PM +0200, Pantelis Antoniou wrote:
> >> Hi Alan,
> >>
> >> On Nov 1, 2012, at 3:51 PM, Alan Cox wrote:
> >>
> >> >> What they want, and what every user wants, is I plug this board in, and
> >> >> the driver make sure everything is loaded and ready. No, the end users
> >> >> don't want to see any of the implementation details of how the bitfile
> >> >> is transported; the driver can handle it.
> >> >
> >> > That doesn't necessarily make it a bus merely some kind of hotplug
> >> > enumeration of devices. That should all work properly both for devices
> >> > and busses with spi and i?c as the final bits needed for it got fixed
> >> > some time ago.
> >> >
> >> > In an ideal world you don't want to be writing custom drivers for stuff.
> >> > If your cape routes an i?c serial device to the existing system i?c
> >> > busses then you want to just create an instance of any existing driver on
> >> > the existing i?c bus not create a whole new layer of goop.
> >> >
> >> > It does need to do the plumbing and resource management for the plumbing
> >> > but thats not the same as being a bus.
> >> >
> >> > Alan
> >>
> >>
> >> Fair enough. But there's no such thing a 'hotplug enumeration
> >> construct' in Linux yet, and a bus is the closest thing to it. It does
> >> take advantage of the nice way device code matches drivers and devices
> >> though.
> >>
> >> I'm afraid that having the I2C/SPI drivers doing the detection won't
> >> work. The capes can have arbitrary I2C/SPI devices (and even more
> >> weird components). There is no way to assure for example that the I2C
> >> device responding to address 'foo' in cape A is the same I2C device
> >> responding to the same address in cape B.
> >
> > your ->detect() method should take care of that.
>
> There isn't some magical serial number in I?C devices that a
> ->detect() method can read and the implementation of I?C is somewhat
> flexible. One devices read may be another devices write. A detect

look at what other drivers do. You can read a revision register, you can
write a command and see if the device responds as expected, it doesn't
matter.

> method that only performs reads could easily toggle a gpio that resets
> the board, rewrite and eeprom, or set the printer on fire. If you

how ? It's just a read.

> browse through various detect functions, yes, some of them key off an
> ID, but a lot of them just check various registers to see if certain
> bits are zero, or certain bits are one. A lot of I?C devices I've
> dealt with have no good way of probing them, especially GPIO chips
> (you'll notice none of the I?C GPIO expanders have detect functions)

it doesn't mean it can't be done.

> On top of all this the detect routine does not tell you if the alert
> pin is connected to some IRQ, or in the case of a GPIO expander, what
> those GPIOs are connected to, etc, etc.

so what ? All you want to do with detect is figure out if the far end is
who you think it is, not what it's doing.

--
balbi


Attachments:
(No filename) (3.09 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-11-02 09:26:42

by Benoit Cousson

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Jason,

On 11/1/2012 7:50 PM, Jason Kridner wrote:
> My apologies for starting a new thread, but I don't have this thread
> in my Inbox.
>
> http://www.spinics.net/lists/linux-omap/msg81034.html
>
> Tony Lindgren wrote:
>
>> * Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx> [121031 15:02]:
>>>
>>> So when device's node is 'disabled' of_platform_device_create_pdata()
>>> will not create the device.
>>>
>>> Now, of course it is possible to re-trigger the platform's probe method
>>> to be called, and in fact I do so in the capebus patches.
>>
>> You should fix this in generic way then rather than working
>> around it in capebus. The same problem exists changing
>> between different functionality for the shared pins,
>> let's say between USB pins and UART pins if you want a
>> serial debug console on some phone.
>
> The current capebus solution goes a long way to fixing a huge issue
> for BeagleBone users and I don't understand what seems to be a
> push-back on principle. On BeagleBone capes, these conflicts cannot be
> resolved early.

I don't think there is any push-back on the principle. It is a very
valid problem that does not have any solution today.

The comments are more on the implementation.

> Do you have suggestions on some more generic method? It seems to me
> the proposed capebus approach strikes a good balance.

Well, yeah, that's a generic DT issue, not a beagle-cape issue.
We should not necessarily handle it by introducing some fake bus and
some new binding like spi-dt / i2c-dt that does not mean anything in
term of HW.

DT is about pure HW representation. Introducing some fake hierarchy to
make SW life easier is not necessarily the good approach.

Adding the version information in the nodes is potentially a good idea,
but should clearly be well thought and part of the DT core.

Regards,
Benoit

2012-11-02 09:42:55

by Russ Dill

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Fri, Nov 2, 2012 at 1:57 AM, Felipe Balbi <[email protected]> wrote:
> Hi,
>
> On Thu, Nov 01, 2012 at 04:49:23PM -0700, Russ Dill wrote:
>> On Thu, Nov 1, 2012 at 3:05 PM, Felipe Balbi <[email protected]> wrote:
>> > HI,
>> >
>> > On Thu, Nov 01, 2012 at 03:59:50PM +0200, Pantelis Antoniou wrote:
>> >> Hi Alan,
>> >>
>> >> On Nov 1, 2012, at 3:51 PM, Alan Cox wrote:
>> >>
>> >> >> What they want, and what every user wants, is I plug this board in, and
>> >> >> the driver make sure everything is loaded and ready. No, the end users
>> >> >> don't want to see any of the implementation details of how the bitfile
>> >> >> is transported; the driver can handle it.
>> >> >
>> >> > That doesn't necessarily make it a bus merely some kind of hotplug
>> >> > enumeration of devices. That should all work properly both for devices
>> >> > and busses with spi and i²c as the final bits needed for it got fixed
>> >> > some time ago.
>> >> >
>> >> > In an ideal world you don't want to be writing custom drivers for stuff.
>> >> > If your cape routes an i²c serial device to the existing system i²c
>> >> > busses then you want to just create an instance of any existing driver on
>> >> > the existing i²c bus not create a whole new layer of goop.
>> >> >
>> >> > It does need to do the plumbing and resource management for the plumbing
>> >> > but thats not the same as being a bus.
>> >> >
>> >> > Alan
>> >>
>> >>
>> >> Fair enough. But there's no such thing a 'hotplug enumeration
>> >> construct' in Linux yet, and a bus is the closest thing to it. It does
>> >> take advantage of the nice way device code matches drivers and devices
>> >> though.
>> >>
>> >> I'm afraid that having the I2C/SPI drivers doing the detection won't
>> >> work. The capes can have arbitrary I2C/SPI devices (and even more
>> >> weird components). There is no way to assure for example that the I2C
>> >> device responding to address 'foo' in cape A is the same I2C device
>> >> responding to the same address in cape B.
>> >
>> > your ->detect() method should take care of that.
>>
>> There isn't some magical serial number in I²C devices that a
>> ->detect() method can read and the implementation of I²C is somewhat
>> flexible. One devices read may be another devices write. A detect
>
> look at what other drivers do. You can read a revision register, you can
> write a command and see if the device responds as expected, it doesn't
> matter.

Since a "revision" register isn't required by the I²C spec, it isn't
implemented on a huge number of chips. Also, having a few dozen probe
routines come though and write to random address of every single I²C
device can a) take a really long time, and b) have quite a few
unintended side effects.

>> method that only performs reads could easily toggle a gpio that resets
>> the board, rewrite and eeprom, or set the printer on fire. If you
>
> how ? It's just a read.

Because the I²C spec is incredibly flexible. For a lot of devices,
reading from a register is done by writing the register address, and
then reading the contents. For devices that don't implement registers
in that way (such as many eeproms), this is just a write.

>> browse through various detect functions, yes, some of them key off an
>> ID, but a lot of them just check various registers to see if certain
>> bits are zero, or certain bits are one. A lot of I²C devices I've
>> dealt with have no good way of probing them, especially GPIO chips
>> (you'll notice none of the I²C GPIO expanders have detect functions)
>
> it doesn't mean it can't be done.

Really? Please, do tell how you would write a detect function for a
PCA9534. It has 4 registers, an input port registers, an output port
register, a polarity inversion register, and a configuration register.
And don't forget, since we are probing, every detect routine for every
I²C driver will have to run with every I²C address on every bus,
possibly with both address formats.

>> On top of all this the detect routine does not tell you if the alert
>> pin is connected to some IRQ, or in the case of a GPIO expander, what
>> those GPIOs are connected to, etc, etc.
>
> so what ? All you want to do with detect is figure out if the far end is
> who you think it is, not what it's doing.

If we already knew who was there, we wouldn't need a detect routine.

2012-11-02 10:19:11

by Koen Kooi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2


Op 2 nov. 2012, om 10:26 heeft "Cousson, Benoit" <[email protected]> het volgende geschreven:

> Hi Jason,
>
> On 11/1/2012 7:50 PM, Jason Kridner wrote:
>> My apologies for starting a new thread, but I don't have this thread
>> in my Inbox.
>>
>> http://www.spinics.net/lists/linux-omap/msg81034.html
>>
>> Tony Lindgren wrote:
>>
>>> * Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx> [121031 15:02]:
>>>>
>>>> So when device's node is 'disabled' of_platform_device_create_pdata()
>>>> will not create the device.
>>>>
>>>> Now, of course it is possible to re-trigger the platform's probe method
>>>> to be called, and in fact I do so in the capebus patches.
>>>
>>> You should fix this in generic way then rather than working
>>> around it in capebus. The same problem exists changing
>>> between different functionality for the shared pins,
>>> let's say between USB pins and UART pins if you want a
>>> serial debug console on some phone.
>>
>> The current capebus solution goes a long way to fixing a huge issue
>> for BeagleBone users and I don't understand what seems to be a
>> push-back on principle. On BeagleBone capes, these conflicts cannot be
>> resolved early.
>
> I don't think there is any push-back on the principle. It is a very valid problem that does not have any solution today.
>
> The comments are more on the implementation.
>
>> Do you have suggestions on some more generic method? It seems to me
>> the proposed capebus approach strikes a good balance.
>
> Well, yeah, that's a generic DT issue, not a beagle-cape issue.
> We should not necessarily handle it by introducing some fake bus and some new binding like spi-dt / i2c-dt that does not mean anything in term of HW.
>
> DT is about pure HW representation. Introducing some fake hierarchy to make SW life easier is not necessarily the good approach.

I see, pure HW. Let's look at this:

gpio_keys {
compatible = "gpio-keys";
pinctrl-names = "default";
pinctrl-0 = <&bone_lcd3_cape_keys_00A0_pins>;

#address-cells = <1>;
#size-cells = <0>;

button@1 {
debounce_interval = <50>;
linux,code = <105>;
label = "left";
gpios = <&gpio2 16 0x0>;
gpio-key,wakeup;
autorepeat;
};

Is the "linux,code" pure hardware or have there already been exceptions to that rule?

regards,

Koen-

2012-11-02 10:39:40

by Koen Kooi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2


Op 2 nov. 2012, om 10:42 heeft Russ Dill <[email protected]> het volgende geschreven:

> On Fri, Nov 2, 2012 at 1:57 AM, Felipe Balbi <[email protected]> wrote:
>> Hi,
>>
>> On Thu, Nov 01, 2012 at 04:49:23PM -0700, Russ Dill wrote:
>>> On Thu, Nov 1, 2012 at 3:05 PM, Felipe Balbi <[email protected]> wrote:
>>>> HI,
>>>>
>>>> On Thu, Nov 01, 2012 at 03:59:50PM +0200, Pantelis Antoniou wrote:
>>>>> Hi Alan,
>>>>>
>>>>> On Nov 1, 2012, at 3:51 PM, Alan Cox wrote:
>>>>>
>>>>>>> What they want, and what every user wants, is I plug this board in, and
>>>>>>> the driver make sure everything is loaded and ready. No, the end users
>>>>>>> don't want to see any of the implementation details of how the bitfile
>>>>>>> is transported; the driver can handle it.
>>>>>>
>>>>>> That doesn't necessarily make it a bus merely some kind of hotplug
>>>>>> enumeration of devices. That should all work properly both for devices
>>>>>> and busses with spi and i?c as the final bits needed for it got fixed
>>>>>> some time ago.
>>>>>>
>>>>>> In an ideal world you don't want to be writing custom drivers for stuff.
>>>>>> If your cape routes an i?c serial device to the existing system i?c
>>>>>> busses then you want to just create an instance of any existing driver on
>>>>>> the existing i?c bus not create a whole new layer of goop.
>>>>>>
>>>>>> It does need to do the plumbing and resource management for the plumbing
>>>>>> but thats not the same as being a bus.
>>>>>>
>>>>>> Alan
>>>>>
>>>>>
>>>>> Fair enough. But there's no such thing a 'hotplug enumeration
>>>>> construct' in Linux yet, and a bus is the closest thing to it. It does
>>>>> take advantage of the nice way device code matches drivers and devices
>>>>> though.
>>>>>
>>>>> I'm afraid that having the I2C/SPI drivers doing the detection won't
>>>>> work. The capes can have arbitrary I2C/SPI devices (and even more
>>>>> weird components). There is no way to assure for example that the I2C
>>>>> device responding to address 'foo' in cape A is the same I2C device
>>>>> responding to the same address in cape B.
>>>>
>>>> your ->detect() method should take care of that.
>>>
>>> There isn't some magical serial number in I?C devices that a
>>> ->detect() method can read and the implementation of I?C is somewhat
>>> flexible. One devices read may be another devices write. A detect
>>
>> look at what other drivers do. You can read a revision register, you can
>> write a command and see if the device responds as expected, it doesn't
>> matter.
>
> Since a "revision" register isn't required by the I?C spec, it isn't
> implemented on a huge number of chips. Also, having a few dozen probe
> routines come though and write to random address of every single I?C
> device can a) take a really long time, and b) have quite a few
> unintended side effects.
>
>>> method that only performs reads could easily toggle a gpio that resets
>>> the board, rewrite and eeprom, or set the printer on fire. If you
>>
>> how ? It's just a read.
>
> Because the I?C spec is incredibly flexible. For a lot of devices,
> reading from a register is done by writing the register address, and
> then reading the contents. For devices that don't implement registers
> in that way (such as many eeproms), this is just a write.
>
>>> browse through various detect functions, yes, some of them key off an
>>> ID, but a lot of them just check various registers to see if certain
>>> bits are zero, or certain bits are one. A lot of I?C devices I've
>>> dealt with have no good way of probing them, especially GPIO chips
>>> (you'll notice none of the I?C GPIO expanders have detect functions)
>>
>> it doesn't mean it can't be done.
>
> Really? Please, do tell how you would write a detect function for a
> PCA9534. It has 4 registers, an input port registers, an output port
> register, a polarity inversion register, and a configuration register.
> And don't forget, since we are probing, every detect routine for every
> I?C driver will have to run with every I?C address on every bus,
> possibly with both address formats.

Worse, things like early revisions of the picoDLP projector will erase their firmware if you do a linear scan through all addresses.

regards,

Koen-

2012-11-02 11:06:47

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi,

On Fri, Nov 02, 2012 at 02:42:51AM -0700, Russ Dill wrote:
> >> browse through various detect functions, yes, some of them key off an
> >> ID, but a lot of them just check various registers to see if certain
> >> bits are zero, or certain bits are one. A lot of I?C devices I've
> >> dealt with have no good way of probing them, especially GPIO chips
> >> (you'll notice none of the I?C GPIO expanders have detect functions)
> >
> > it doesn't mean it can't be done.
>
> Really? Please, do tell how you would write a detect function for a
> PCA9534. It has 4 registers, an input port registers, an output port
> register, a polarity inversion register, and a configuration register.

read them and match to their reset values, perhaps ?

> And don't forget, since we are probing, every detect routine for every
> I?C driver will have to run with every I?C address on every bus,
> possibly with both address formats.

not *every* I2C address. What you say is wrong, a ->detect() method will
only run for those addresses which the device can actually assume.

> >> On top of all this the detect routine does not tell you if the alert
> >> pin is connected to some IRQ, or in the case of a GPIO expander, what
> >> those GPIOs are connected to, etc, etc.
> >
> > so what ? All you want to do with detect is figure out if the far end is
> > who you think it is, not what it's doing.
>
> If we already knew who was there, we wouldn't need a detect routine.

of course not :-) But the whole discussion has been about not knowing
which capes (and thus which devices) are attached to the bone.

--
balbi


Attachments:
(No filename) (1.57 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2012-11-02 11:16:13

by Alan

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

> >> Fair enough. But there's no such thing a 'hotplug enumeration
> >> construct' in Linux yet, and a bus is the closest thing to it. It does
> >> take advantage of the nice way device code matches drivers and devices
> >> though.

A bus is the wrong construct. You need something to add devices onto the
busses. You can do that. The Intel SFI layer on phones for example
enumerates devices through a firmware table set and creates them on the
right actual physical bus not on their own fake one.

It's not hotplug in the sense that the phone happens be a fixed
configuration but it does support hotplug behaviour because the order of
the drivers and enumeration is undefined (and both orders work).

> >>
> >> I'm afraid that having the I2C/SPI drivers doing the detection won't
> >> work. The capes can have arbitrary I2C/SPI devices (and even more
> >> weird components). There is no way to assure for example that the I2C
> >> device responding to address 'foo' in cape A is the same I2C device
> >> responding to the same address in cape B.
> >
> > your ->detect() method should take care of that.
>
> There isn't some magical serial number in I²C devices that a
> ->detect() method can read and the implementation of I²C is somewhat

It doesn't matter.

What you are basically talking about is


cape layer
- wtf is this
- how do I plumb it

- create device nodes with correct name for
binding, address etc on the right bus


i2c layer
- ooh a new i2c device
- probe as indicated by device name
- attach correct driver


Architecturally its possible you want to make some caps MFDs if they have
their own bus heirarchies etc but generally I doubt it.


Take a look at arch/x86/platform/mrst/mrst.c. It's a specific example of
a platform which parses tables and attaches devices to the right physical
bus in a manner they can be reliably probed even when the device has no
sane autodetect.

Alan

2012-11-02 12:33:01

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Alan,

On Nov 2, 2012, at 1:21 PM, Alan Cox wrote:

>>>> Fair enough. But there's no such thing a 'hotplug enumeration
>>>> construct' in Linux yet, and a bus is the closest thing to it. It does
>>>> take advantage of the nice way device code matches drivers and devices
>>>> though.
>
> A bus is the wrong construct. You need something to add devices onto the
> busses. You can do that. The Intel SFI layer on phones for example
> enumerates devices through a firmware table set and creates them on the
> right actual physical bus not on their own fake one.
>
> It's not hotplug in the sense that the phone happens be a fixed
> configuration but it does support hotplug behaviour because the order of
> the drivers and enumeration is undefined (and both orders work).

I see. The firmware table format is similar to our case where we use DT.
We don't have to come up with another firmware table format, like SFI here.
The problem is that we have to inject DT fragments to the kernel's runtime
device tree, at some arbitrary point in time, after the cape probing is complete.
Not only that, the DT fragment for each device is not self contained; it contains
references to other nodes which have to be resolved in runtime.

Let me demonstrate what the problem is - take for example the very simple
example of a (non-versioned) cape, with a single I2C device.

------8<-----------8<--------------
Kernel's boot DT:

i2c2: i2c@4819c000 {
compatible = "ti,omap4-i2c";
#address-cells = <1>;
#size-cells = <0>;
ti,hwmods = "i2c3"; /* TODO: Fix hwmod */
reg = <0x4819c000 0x1000>;
interrupt-parent = <&intc>;
interrupts = <30>;

status = "on-demand"; /* there's no on-demand status property defined yet */
/* means that it will be set to "okay" when a i2c */
/* device node turn it to "okay" */
/* we can't enable it at boot because of possible */
/* conflicts with other peripherals */
pinctrl-names = "default";
pinctrl-0 = <&i2c2_pins>;

clock-frequency = <100000>;

};
------8<-----------8<--------------

A cape DT fragment (probable syntax) :

i2c2-devices {
parent = <&i2c2>;

/* Ambient light sensor */
tsl2550@39 {
compatible = "tsl,tsl2550";
reg = <0x39>;
};
};

------8<-----------8<--------------

The i2c2 alias cannot be resolved at compile time; there has to be

a) A DT object format where unresolved aliases (symbols) are tracked
b) A runtime DT linker that will resolve the alias, and will insert the
i2c2-devices child nodes as children in the i2c2 node.
c) A method to trigger platform device creation for the child nodes just
inserted.

DT core changes aren't bound to be easily accepted, so without having a clear
signal from the DT maintainers that they would consider such a method people
are just hesitant to go down this road.


>
>>>>
>>>> I'm afraid that having the I2C/SPI drivers doing the detection won't
>>>> work. The capes can have arbitrary I2C/SPI devices (and even more
>>>> weird components). There is no way to assure for example that the I2C
>>>> device responding to address 'foo' in cape A is the same I2C device
>>>> responding to the same address in cape B.
>>>
>>> your ->detect() method should take care of that.
>>
>> There isn't some magical serial number in I?C devices that a
>> ->detect() method can read and the implementation of I?C is somewhat
>
> It doesn't matter.
>
> What you are basically talking about is
>
>
> cape layer
> - wtf is this
> - how do I plumb it
>
> - create device nodes with correct name for
> binding, address etc on the right bus
>
>
> i2c layer
> - ooh a new i2c device
> - probe as indicated by device name
> - attach correct driver
>
>
> Architecturally its possible you want to make some caps MFDs if they have
> their own bus heirarchies etc but generally I doubt it.
>
>
> Take a look at arch/x86/platform/mrst/mrst.c. It's a specific example of
> a platform which parses tables and attaches devices to the right physical
> bus in a manner they can be reliably probed even when the device has no
> sane autodetect.
>
> Alan

Regards

-- Pantelis

2012-11-02 16:07:15

by Jason Kridner

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Fri, Nov 2, 2012 at 7:21 AM, Alan Cox <[email protected]> wrote:
>> >> Fair enough. But there's no such thing a 'hotplug enumeration
>> >> construct' in Linux yet, and a bus is the closest thing to it. It does
>> >> take advantage of the nice way device code matches drivers and devices
>> >> though.
>
> A bus is the wrong construct. You need something to add devices onto the
> busses. You can do that. The Intel SFI layer on phones for example
> enumerates devices through a firmware table set and creates them on the
> right actual physical bus not on their own fake one.

Physically, it is a bus, though it is made up of several other busses.
While I could certainly see people using the mechanism for enumerating
soldered-down devices over the fixed busses, there is a physical
connector that deserves some abstraction/identification.

Further, it is critical to enable hardware vendors to avoid writing
any code for which there are existing drivers.

>
> It's not hotplug in the sense that the phone happens be a fixed
> configuration but it does support hotplug behaviour because the order of
> the drivers and enumeration is undefined (and both orders work).

I think SFI is interesting, but certainly lacks all of the interfaces.
It seems reasonable to me to extend the specification to add the
missing interfaces, but what doesn't seem to map is the fact that the
SFI structures are initially processed in the bootloader and passed
statically to the kernel, rather than enabling run-time operation.
Users may make run-time trade-offs and also need mechanisms for
performing initial debug on products like "proto capes".

Capebus seems to me to provide this solution fairly well. I don't
believe the SFI approach covers the most critical aspects of hotplug
behaviour.

>
>> >>
>> >> I'm afraid that having the I2C/SPI drivers doing the detection won't
>> >> work. The capes can have arbitrary I2C/SPI devices (and even more
>> >> weird components). There is no way to assure for example that the I2C
>> >> device responding to address 'foo' in cape A is the same I2C device
>> >> responding to the same address in cape B.
>> >
>> > your ->detect() method should take care of that.
>>
>> There isn't some magical serial number in I?C devices that a
>> ->detect() method can read and the implementation of I?C is somewhat
>
> It doesn't matter.
>
> What you are basically talking about is
>
>
> cape layer
> - wtf is this
> - how do I plumb it
>
> - create device nodes with correct name for
> binding, address etc on the right bus
>
>
> i2c layer
> - ooh a new i2c device
> - probe as indicated by device name
> - attach correct driver

Many of the devices cannot be probed.

>
>
> Architecturally its possible you want to make some caps MFDs if they have
> their own bus heirarchies etc but generally I doubt it.
>
>
> Take a look at arch/x86/platform/mrst/mrst.c. It's a specific example of
> a platform which parses tables and attaches devices to the right physical
> bus in a manner they can be reliably probed even when the device has no
> sane autodetect.

I know I *am* the slow person in the room, but doesn't this approach
require the code to be compiled into the kernel to support the devices
ahead of time? While I think it might be reasonable to have hardware
developers provide DT fragments in their EEPROMs, there's no way to
get them to submit code patches in order to get their hardware to
work. They need to ship hardware that works with pre-existing
software, since there will be hundreds of boards created by people
with little to no previous Linux experience (akin to Arduino). I seem
to be missing how that approach would get us there.

>
> Alan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2012-11-02 16:23:36

by Alan

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

> Further, it is critical to enable hardware vendors to avoid writing
> any code for which there are existing drivers.

Which is why you don't want to create a new bus type for it.

> Capebus seems to me to provide this solution fairly well. I don't
> believe the SFI approach covers the most critical aspects of hotplug
> behaviour.

I think you missed the point - it just an example of doing this not one
I'd suggest using directly.

> >> > your ->detect() method should take care of that.
> >>
> >> There isn't some magical serial number in I²C devices that a
> >> ->detect() method can read and the implementation of I²C is somewhat
> >
> > It doesn't matter.
> >
> > What you are basically talking about is
> >
> >
> > cape layer
> > - wtf is this
> > - how do I plumb it
> >
> > - create device nodes with correct name for
> > binding, address etc on the right bus
> >
> >
> > i2c layer
> > - ooh a new i2c device
> > - probe as indicated by device name
> > - attach correct driver
>
> Many of the devices cannot be probed.

Look more closely at the code I pointed you at. I wonder if have a
differing understanding of the word "probe" in this situation. In the
Linux space the driver bindings call the matching probe function based
upon the device structure.

In Linux the probe method does not mean "scan the bus and enumerate/detect
the devices"

If an i²c bus is thrown a device which has an address and a matching name
the only thing it will attempt to do is to call the probe method of the
device driver matching that name on the given i²c address. In other words
its a "probe" in the sense of "I've been told there is one of your
widgets here, please take a look" not a "probe" in the sense of "scan 255
i²c addresses and poke randomly at them"

SFI for example creates entries for things like

"type foo pressure sensor at 0x68 on bus 3"

and the core kernel i²c code will only call the "foo" drivers probe method
and only on bus 3 and only for address 0x68.

In your case you want to use your DT fragments or any other descriptor
format to do exactly the same. Not create a new bus but add a bunch of
devices to the existing busses.

It's like hot plugging a PCI card - you don't create a new PCI bus, you
add a device to the existing bus. In the PCI case the device has
properties that uniquely identify it from hardware level. In the i²c case
the properties come from your DT fragment or similar. The rest is the
same.

> I know I *am* the slow person in the room, but doesn't this approach
> require the code to be compiled into the kernel to support the devices
> ahead of time? While I think it might be reasonable to have hardware

The specific implementation in SFI does but thats a property of SFI. I'm
not trying to push SFI itself anywhere except over the edge of a very tall
cliff.

The point is that you can take a description of things like i²c devices
and have then properly identified on the existing busses using the
existing bus architecture just fine.

> developers provide DT fragments in their EEPROMs, there's no way to
> get them to submit code patches in order to get their hardware to
> work. They need to ship hardware that works with pre-existing
> software, since there will be hundreds of boards created by people
> with little to no previous Linux experience (akin to Arduino). I seem
> to be missing how that approach would get us there.

That is what I assume and what I believe can be provided without
inventing imaginary bus types.

Alan

2012-11-02 16:44:35

by Russ Dill

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Fri, Nov 2, 2012 at 4:00 AM, Felipe Balbi <[email protected]> wrote:
> Hi,
>
> On Fri, Nov 02, 2012 at 02:42:51AM -0700, Russ Dill wrote:
>> >> browse through various detect functions, yes, some of them key off an
>> >> ID, but a lot of them just check various registers to see if certain
>> >> bits are zero, or certain bits are one. A lot of I²C devices I've
>> >> dealt with have no good way of probing them, especially GPIO chips
>> >> (you'll notice none of the I²C GPIO expanders have detect functions)
>> >
>> > it doesn't mean it can't be done.
>>
>> Really? Please, do tell how you would write a detect function for a
>> PCA9534. It has 4 registers, an input port registers, an output port
>> register, a polarity inversion register, and a configuration register.
>
> read them and match to their reset values, perhaps ?

So its ok for it to not work on warm reset? Also, I'm pretty sure [
random, 0xff, 0x00, 0xff ] describes quite a few chips.

>> And don't forget, since we are probing, every detect routine for every
>> I²C driver will have to run with every I²C address on every bus,
>> possibly with both address formats.
>
> not *every* I2C address. What you say is wrong, a ->detect() method will
> only run for those addresses which the device can actually assume.

OK, that's still a potentially large number of addresses.

>> >> On top of all this the detect routine does not tell you if the alert
>> >> pin is connected to some IRQ, or in the case of a GPIO expander, what
>> >> those GPIOs are connected to, etc, etc.
>> >
>> > so what ? All you want to do with detect is figure out if the far end is
>> > who you think it is, not what it's doing.
>>
>> If we already knew who was there, we wouldn't need a detect routine.
>
> of course not :-) But the whole discussion has been about not knowing
> which capes (and thus which devices) are attached to the bone.

Eh? Finding out which bone is connected is pretty easy, they all have
an EEPROM with identifying information. That isn't the problem that
capebus is solving, capebus is solving the problem of enumerating that
hardware.

2012-11-03 11:50:45

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On 11/02/2012 09:43 AM, Pantelis Antoniou wrote:
[...]

>>
>> And then use the standard DT support to create later the platform_device that does represent the new super-cape devices.
>>
>
> We know this is the ideal case. In fact that's the long term goal and we had internal discussions about it.

Since you already had internal discussions about this, it would have
been a great help in avoiding lots of this discussion if you would've
summarized this ideal case from the beginning, then describe the
weaknesses and limitations of DT for handling hotplug/dynamic devices
and thus the reasoning behind creating capebus. Now it's taken this
long thread for others to try to convince you about something you
already knew to be the ideal case. Seems a bit wasteful.

Kernel development typically works by building/extending infrastructure
that is already there. Only when it's obvious that the current
infrastructure cannot be extended for a new kind of usage do we build
new infrastruture.

In this case, DT is the obvious infrastructure that needs extending. At
least we can all agree on that, for starters.

> DT is nowhere near being able to do it.
>
> That would require the introduction of a DT object file format with aliases being capable to be
> resolved dynamically. We're talking about major changes here in the way DT files are being compiled and
> used in practice.
>
> So yes, in an ideal world that would be great. We're nowhere near close today unfortunately.
>
> Assuming that we do work on a DT object format, and that the runtime resolution mechanism is approved,
> then I agree that this part of the capebus patches can be dropped and the functionality assumed by generic
> DT core.
>
> The question is that this will take time, with no guarantees that this would be acceptable from
> the device tree maintainers. So I am putting them in the CC list, to see what they think about it.

Since this thread has already ventured into the weeds a few times, I
would suggest that you summarize the DT limitations (focusing on they
dynamic/hotplug needs) and start a thread on devicetree-discuss, so that
the DT maintainers can be helpful without having to follow this thread.

IMO, the path forward is clear (though probably longer than you would
like): Let's first try and extend DT infrastructure. If that is
obviously going nowhere, and DT maintainers are against it. Then, let's
revisit capebus.

Kevin

2012-11-05 00:22:55

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Fri, Nov 2, 2012 at 8:43 AM, Pantelis Antoniou
<[email protected]> wrote:
> Assuming that we do work on a DT object format, and that the runtime resolution mechanism is approved,
> then I agree that this part of the capebus patches can be dropped and the functionality assumed by generic
> DT core.
>
> The question is that this will take time, with no guarantees that this would be acceptable from
> the device tree maintainers. So I am putting them in the CC list, to see what they think about it.

This is actually exactly the direction I want to go with DT, which the
ability to load supplemental DT data blobs from either a kernel module
or userspace using the firmware loading infrastructure.

g.

2012-11-05 00:33:13

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Fri, Nov 2, 2012 at 10:19 AM, Koen Kooi <[email protected]> wrote:
> button@1 {
> debounce_interval = <50>;
> linux,code = <105>;
> label = "left";
> gpios = <&gpio2 16 0x0>;
> gpio-key,wakeup;
> autorepeat;
> };
>
> Is the "linux,code" pure hardware or have there already been exceptions to that rule?

I generally push back on "linux,blah" bindings since they have the
tendency to change over time when someone changes the driver. However,
since the keycodes are part of the Linux userspace ABI, and are
therefore pretty much set it stone, I didn't object to the
linux,keycode binding. There are already drivers using it.

g.

2012-11-05 00:37:49

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Fri, Nov 2, 2012 at 12:32 PM, Pantelis Antoniou
<[email protected]> wrote:
> The i2c2 alias cannot be resolved at compile time; there has to be
>
> a) A DT object format where unresolved aliases (symbols) are tracked
> b) A runtime DT linker that will resolve the alias, and will insert the
> i2c2-devices child nodes as children in the i2c2 node.
> c) A method to trigger platform device creation for the child nodes just
> inserted.
>
> DT core changes aren't bound to be easily accepted, so without having a clear
> signal from the DT maintainers that they would consider such a method people
> are just hesitant to go down this road.

I do agree and will accept such a method.

g.

2012-11-05 01:06:06

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Fri, Nov 2, 2012 at 4:07 PM, Jason Kridner <[email protected]> wrote:
>> Take a look at arch/x86/platform/mrst/mrst.c. It's a specific example of
>> a platform which parses tables and attaches devices to the right physical
>> bus in a manner they can be reliably probed even when the device has no
>> sane autodetect.
>
> I know I *am* the slow person in the room, but doesn't this approach
> require the code to be compiled into the kernel to support the devices
> ahead of time? While I think it might be reasonable to have hardware
> developers provide DT fragments in their EEPROMs, there's no way to
> get them to submit code patches in order to get their hardware to
> work. They need to ship hardware that works with pre-existing
> software, since there will be hundreds of boards created by people
> with little to no previous Linux experience (akin to Arduino). I seem
> to be missing how that approach would get us there.

If it is truly new hardware, then there really is no way around them
either a) submitting new kernel drivers or b) driving them from
userspace.

If it is devices with existing drivers populated onto new custom cape
boards, then the DT fragment approach should be sufficient for
populating them into the Linux driver model. (assuming of course those
drivers are already compiled into the kernel)

g.

2012-11-05 13:26:38

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2


On Nov 5, 2012, at 1:22 AM, Grant Likely wrote:

> On Fri, Nov 2, 2012 at 8:43 AM, Pantelis Antoniou
> <[email protected]> wrote:
>> Assuming that we do work on a DT object format, and that the runtime resolution mechanism is approved,
>> then I agree that this part of the capebus patches can be dropped and the functionality assumed by generic
>> DT core.
>>
>> The question is that this will take time, with no guarantees that this would be acceptable from
>> the device tree maintainers. So I am putting them in the CC list, to see what they think about it.
>
> This is actually exactly the direction I want to go with DT, which the
> ability to load supplemental DT data blobs from either a kernel module
> or userspace using the firmware loading infrastructure.
>
> g.

Hi Grant,

That's pretty much our use case.

Regards

-- Pantelis

2012-11-05 14:34:24

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Mon, Nov 5, 2012 at 1:25 PM, Pantelis Antoniou
<[email protected]> wrote:
>
> On Nov 5, 2012, at 1:22 AM, Grant Likely wrote:
>
>> On Fri, Nov 2, 2012 at 8:43 AM, Pantelis Antoniou
>> <[email protected]> wrote:
>>> Assuming that we do work on a DT object format, and that the runtime resolution mechanism is approved,
>>> then I agree that this part of the capebus patches can be dropped and the functionality assumed by generic
>>> DT core.
>>>
>>> The question is that this will take time, with no guarantees that this would be acceptable from
>>> the device tree maintainers. So I am putting them in the CC list, to see what they think about it.
>>
>> This is actually exactly the direction I want to go with DT, which the
>> ability to load supplemental DT data blobs from either a kernel module
>> or userspace using the firmware loading infrastructure.
>>
>> g.
>
> Hi Grant,
>
> That's pretty much our use case.
>
> Regards

Good. I'm about 80% though putting together a project plan of what is
required to implement this. I'll post it for RFC shortly. I would
appreciate feedback and help on flushing out the design.

g.

2012-11-05 15:34:54

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

* Grant Likely <[email protected]> [121105 06:36]:
> On Mon, Nov 5, 2012 at 1:25 PM, Pantelis Antoniou
> <[email protected]> wrote:
> >
> > On Nov 5, 2012, at 1:22 AM, Grant Likely wrote:
> >
> >> On Fri, Nov 2, 2012 at 8:43 AM, Pantelis Antoniou
> >> <[email protected]> wrote:
> >>> Assuming that we do work on a DT object format, and that the runtime resolution mechanism is approved,
> >>> then I agree that this part of the capebus patches can be dropped and the functionality assumed by generic
> >>> DT core.
> >>>
> >>> The question is that this will take time, with no guarantees that this would be acceptable from
> >>> the device tree maintainers. So I am putting them in the CC list, to see what they think about it.
> >>
> >> This is actually exactly the direction I want to go with DT, which the
> >> ability to load supplemental DT data blobs from either a kernel module
> >> or userspace using the firmware loading infrastructure.
> >>
> >> g.
> >
> > Hi Grant,
> >
> > That's pretty much our use case.
> >
> > Regards
>
> Good. I'm about 80% though putting together a project plan of what is
> required to implement this. I'll post it for RFC shortly. I would
> appreciate feedback and help on flushing out the design.

Great, sounds like almost-a-plan then :)

Regards,

Tony

2012-11-05 15:37:29

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Grant,

On Nov 5, 2012, at 1:37 AM, Grant Likely wrote:

> On Fri, Nov 2, 2012 at 12:32 PM, Pantelis Antoniou
> <[email protected]> wrote:
>> The i2c2 alias cannot be resolved at compile time; there has to be
>>
>> a) A DT object format where unresolved aliases (symbols) are tracked
>> b) A runtime DT linker that will resolve the alias, and will insert the
>> i2c2-devices child nodes as children in the i2c2 node.
>> c) A method to trigger platform device creation for the child nodes just
>> inserted.
>>
>> DT core changes aren't bound to be easily accepted, so without having a clear
>> signal from the DT maintainers that they would consider such a method people
>> are just hesitant to go down this road.
>
> I do agree and will accept such a method.
>
> g.

Understood.

I'll think about the issues and come up with a design for the format, but
let's talk about this in the open for a while.

I don't want to modify the DTB format, in order to avoid impacting any other
DT users.

So would something like this work for you?

-----8<--------8<--------

/* booting DT (all symbols are resolved) */
/* this is a valid DTB today */

/ {

i2c2: i2c@4819c000 {
compatible = "ti,omap4-i2c";
#address-cells = <1>;
#size-cells = <0>;
ti,hwmods = "i2c3"; /* TODO: Fix hwmod */
reg = <0x4819c000 0x1000>;
interrupt-parent = <&intc>;
interrupts = <30>;
status = "on-demand";
};

/* .... others nodes .... */

/* the symbols node can be generated by the DTC */
symbols {
compatible = "dt,symbols";

symbol@0 {
name = "i2c2";
phandle = <&i2c2>;
};

...
};
};

-----8<--------8<--------

/* fragment DT (not all symbols are resolved) */

/ {
i2c2-append {
compatible = "dt,append";
#address-cells = <1>;
#size-cells = <0>;

append-at = <&i2c2>; /* offset F00, either -1 (end of list) */
/* or next offset in the list of i2c2 refs. */

/* Ambient light sensor */
tsl2550@39 {
compatible = "tsl,tsl2550";
reg = <0x39>;
};
};
};

-----8<--------8<--------

The <&i2c2> value can't be resolved at compile time.

We can use a simple linked list method for keeping track of the
locations in the fragment which we need to resolve, terminating with -1.

The compiler can add the following nodes to mark the entries requiring
resolution.

For example i2c2...

dt-resolution {
i2c2 = <0xf00>; /* first offset in the dtb fragment requiring */
/* resolution of i2c2 */
....
};

The dt-resolution node, can be appended to the dt fragment contents and
can be generated by the compiler.

Upon injection of the DT fragment the in-kernel loader can perform
symbol resolution and replace the phandles with the in-kernel values.

I know there's an aliases node, but I don't know if it's wise to modify it.

What do you think?

Regards

-- Pantelis

2012-11-05 15:56:34

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On 11/05/2012 08:34 AM, Grant Likely wrote:
> On Mon, Nov 5, 2012 at 1:25 PM, Pantelis Antoniou
> <[email protected]> wrote:
>>
>> On Nov 5, 2012, at 1:22 AM, Grant Likely wrote:
>>
>>> On Fri, Nov 2, 2012 at 8:43 AM, Pantelis Antoniou
>>> <[email protected]> wrote:
>>>> Assuming that we do work on a DT object format, and that the runtime resolution mechanism is approved,
>>>> then I agree that this part of the capebus patches can be dropped and the functionality assumed by generic
>>>> DT core.
>>>>
>>>> The question is that this will take time, with no guarantees that this would be acceptable from
>>>> the device tree maintainers. So I am putting them in the CC list, to see what they think about it.
>>>
>>> This is actually exactly the direction I want to go with DT, which the
>>> ability to load supplemental DT data blobs from either a kernel module
>>> or userspace using the firmware loading infrastructure.
>>>
>>> g.
>>
>> Hi Grant,
>>
>> That's pretty much our use case.
>>
>> Regards
>
> Good. I'm about 80% though putting together a project plan of what is
> required to implement this. I'll post it for RFC shortly. I would
> appreciate feedback and help on flushing out the design.

The FPGA folks are also interested in dynamic DT configuration. There's
been some discussion and postings on the DT list in the last few months
you may have missed. It's maybe not completely the same use case, but
there is some overlap here.

Rob

2012-11-05 19:10:50

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Mon, Nov 5, 2012 at 3:37 PM, Pantelis Antoniou
<[email protected]> wrote:
> Hi Grant,
>
> On Nov 5, 2012, at 1:37 AM, Grant Likely wrote:
>
>> On Fri, Nov 2, 2012 at 12:32 PM, Pantelis Antoniou
>> <[email protected]> wrote:
>>> The i2c2 alias cannot be resolved at compile time; there has to be
>>>
>>> a) A DT object format where unresolved aliases (symbols) are tracked
>>> b) A runtime DT linker that will resolve the alias, and will insert the
>>> i2c2-devices child nodes as children in the i2c2 node.
>>> c) A method to trigger platform device creation for the child nodes just
>>> inserted.
>>>
>>> DT core changes aren't bound to be easily accepted, so without having a clear
>>> signal from the DT maintainers that they would consider such a method people
>>> are just hesitant to go down this road.
>>
>> I do agree and will accept such a method.
>>
>> g.
>
> Understood.
>
> I'll think about the issues and come up with a design for the format, but
> let's talk about this in the open for a while.

Agreed. I'm planning to post my first draft/RFC tonight.

> I don't want to modify the DTB format, in order to avoid impacting any other
> DT users.

Ditto. It should be a direct extension.

> So would something like this work for you?
[...]
> The dt-resolution node, can be appended to the dt fragment contents and
> can be generated by the compiler.
>
> Upon injection of the DT fragment the in-kernel loader can perform
> symbol resolution and replace the phandles with the in-kernel values.
>
> I know there's an aliases node, but I don't know if it's wise to modify it.
>
> What do you think?

If I'm reading your intent correctly, your primary worry is the
problem of matching up phandles from the base dt with (potentially
different) phandle values in the overlay. This solution solves it by
encoding all of the phandle locations as offsets from the start of the
file into the dt-resolution node. Correct? My concern with this
approach is that it is fragile. Any changes to the fragment file, such
as to add extra properties or nodes, or even to repack the tree will
break all the offsets; probably silently.

It would be less fragile if each property containing phandles had some
kind of .<prop>-phandle-offsets companion property that listed the
phandles that need to be fixed up as an offset to the beginning of
only that properties' data. Better, in that modifying the tree won't
break the links, but I still worry that it is fragile and possibly too
complex.

However, the problem is based on the assumption that phandles are
effectively random and could change ever time the tree is recompiled.
Well, what if they weren't? What if dtc generated phandles using a
hash of the node full name so that the value changed rarely? Also,
what if the format was oriented around detecting if the phandles don't
match instead of fixing things up? The solution becomes a lot simpler
if the parser only has to verify that the referenced phandles already
exist at the right path in the tree.

Something like this:

/include/ "base-file.dts" /* include might not be the right syntax here */
&i2c0 { /* i2c0 resolved by label */
touchpad@10 {
compatible = "acme,touchpad";
reg = <0x10>;
interrupt-parent = <&intc>;
interrupts = <100>;
};
};

And the generated overlay dtb may look like this:

/ {
.readonly;
interrupt-controller@0x10005000 {
.readonly;
phandle = <0x1234>;
};

peripheral-bus {
.readonly;
i2c@20001000 {
touchpad@10 {
compatible = "acme,touchpad";
reg = <0x10>;
interrupt-parent = <0x1234>;
interrupts = <100>;
};
};
};
};

Which is obviously missing a bunch of information for the rest of the
system, but contains enough data to ensure that the path to the touchpad
node is valid and that the phandle has not changed.

This handles many of the use cases, but it assumes that an overlay is
board specific. If it ever is required to support multiple base boards
with a single overlay file then there is a problem. The .dtb overlays
generated in this manor cannot handle different phandles or nodes that
are in a different place. On the other hand, the overlay source files
should have no problem being compiled for multiple targets, so maybe
it isn't an issue. Plus if dtc is installed on the target, then the
live tree from /proc can be used as the reference when compiling the
overlay.

g.

2012-11-05 19:40:45

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Mon, Nov 5, 2012 at 3:56 PM, Rob Herring <[email protected]> wrote:
> On 11/05/2012 08:34 AM, Grant Likely wrote:
>> Good. I'm about 80% though putting together a project plan of what is
>> required to implement this. I'll post it for RFC shortly. I would
>> appreciate feedback and help on flushing out the design.
>
> The FPGA folks are also interested in dynamic DT configuration. There's
> been some discussion and postings on the DT list in the last few months
> you may have missed. It's maybe not completely the same use case, but
> there is some overlap here.

It should be very similar. I'll see if I can dig up those conversations.

g.

2012-11-05 19:54:36

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Grant,

On Nov 5, 2012, at 8:10 PM, Grant Likely wrote:

> On Mon, Nov 5, 2012 at 3:37 PM, Pantelis Antoniou
> <[email protected]> wrote:
>> Hi Grant,
>>
>> On Nov 5, 2012, at 1:37 AM, Grant Likely wrote:
>>
>>> On Fri, Nov 2, 2012 at 12:32 PM, Pantelis Antoniou
>>> <[email protected]> wrote:
>>>> The i2c2 alias cannot be resolved at compile time; there has to be
>>>>
>>>> a) A DT object format where unresolved aliases (symbols) are tracked
>>>> b) A runtime DT linker that will resolve the alias, and will insert the
>>>> i2c2-devices child nodes as children in the i2c2 node.
>>>> c) A method to trigger platform device creation for the child nodes just
>>>> inserted.
>>>>
>>>> DT core changes aren't bound to be easily accepted, so without having a clear
>>>> signal from the DT maintainers that they would consider such a method people
>>>> are just hesitant to go down this road.
>>>
>>> I do agree and will accept such a method.
>>>
>>> g.
>>
>> Understood.
>>
>> I'll think about the issues and come up with a design for the format, but
>> let's talk about this in the open for a while.
>
> Agreed. I'm planning to post my first draft/RFC tonight.
>
>> I don't want to modify the DTB format, in order to avoid impacting any other
>> DT users.
>
> Ditto. It should be a direct extension.
>

Avoiding mobs of angry users with broken bootloader/kernel combos is a good
thing...

>> So would something like this work for you?
> [...]
>> The dt-resolution node, can be appended to the dt fragment contents and
>> can be generated by the compiler.
>>
>> Upon injection of the DT fragment the in-kernel loader can perform
>> symbol resolution and replace the phandles with the in-kernel values.
>>
>> I know there's an aliases node, but I don't know if it's wise to modify it.
>>
>> What do you think?
>
> If I'm reading your intent correctly, your primary worry is the
> problem of matching up phandles from the base dt with (potentially
> different) phandle values in the overlay. This solution solves it by
> encoding all of the phandle locations as offsets from the start of the
> file into the dt-resolution node. Correct? My concern with this
> approach is that it is fragile. Any changes to the fragment file, such
> as to add extra properties or nodes, or even to repack the tree will
> break all the offsets; probably silently.
>

Yes, this will not survive modification of the fragment file.
For the use case I'm targeting the DT fragment is going to be quite
minimal, a few tens of nodes at the most. Modification of a single
fragment file is not expected.

Modifications to the base tree, those would work just fine. The base
tree by definition will not have external references.

> It would be less fragile if each property containing phandles had some
> kind of .<prop>-phandle-offsets companion property that listed the
> phandles that need to be fixed up as an offset to the beginning of
> only that properties' data. Better, in that modifying the tree won't
> break the links, but I still worry that it is fragile and possibly too
> complex.

I believe this is considerably more complex, without fixing the
fragility problem.

>
> However, the problem is based on the assumption that phandles are
> effectively random and could change ever time the tree is recompiled.
> Well, what if they weren't? What if dtc generated phandles using a
> hash of the node full name so that the value changed rarely? Also,
> what if the format was oriented around detecting if the phandles don't
> match instead of fixing things up? The solution becomes a lot simpler
> if the parser only has to verify that the referenced phandles already
> exist at the right path in the tree.
>
> Something like this:
>
> /include/ "base-file.dts" /* include might not be the right syntax here */
> &i2c0 { /* i2c0 resolved by label */
> touchpad@10 {
> compatible = "acme,touchpad";
> reg = <0x10>;
> interrupt-parent = <&intc>;
> interrupts = <100>;
> };
> };
>
> And the generated overlay dtb may look like this:
>
> / {
> .readonly;
> interrupt-controller@0x10005000 {
> .readonly;
> phandle = <0x1234>;
> };
>
> peripheral-bus {
> .readonly;
> i2c@20001000 {
> touchpad@10 {
> compatible = "acme,touchpad";
> reg = <0x10>;
> interrupt-parent = <0x1234>;
> interrupts = <100>;
> };
> };
> };
> };
>
> Which is obviously missing a bunch of information for the rest of the
> system, but contains enough data to ensure that the path to the touchpad
> node is valid and that the phandle has not changed.

I see what you mean. It will work, provided you 'link' (I don't know what
the proper terminology is) against the correct board dts file.
>
> This handles many of the use cases, but it assumes that an overlay is
> board specific. If it ever is required to support multiple base boards
> with a single overlay file then there is a problem. The .dtb overlays
> generated in this manor cannot handle different phandles or nodes that
> are in a different place. On the other hand, the overlay source files
> should have no problem being compiled for multiple targets, so maybe
> it isn't an issue. Plus if dtc is installed on the target, then the
> live tree from /proc can be used as the reference when compiling the
> overlay.

My worry is that this format is dependent on linking against the board
DTS file. One of the ideas thrown around here was that it might make
sense to store the DTB fragment in the EEPROM of the device.

In that case you have a OS independent hardware description, which can
be even used even by the bootloader to access devices it knows not about
at compile time.

Other than that, I have no other objections.

>
> g.

Regards

-- Pantelis

2012-11-05 20:14:37

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Mon, Nov 5, 2012 at 7:54 PM, Pantelis Antoniou
<[email protected]> wrote:
>> This handles many of the use cases, but it assumes that an overlay is
>> board specific. If it ever is required to support multiple base boards
>> with a single overlay file then there is a problem. The .dtb overlays
>> generated in this manor cannot handle different phandles or nodes that
>> are in a different place. On the other hand, the overlay source files
>> should have no problem being compiled for multiple targets, so maybe
>> it isn't an issue. Plus if dtc is installed on the target, then the
>> live tree from /proc can be used as the reference when compiling the
>> overlay.
>
> My worry is that this format is dependent on linking against the board
> DTS file. One of the ideas thrown around here was that it might make
> sense to store the DTB fragment in the EEPROM of the device.

Right, that wouldn't work well if the base DT changed, or if a
BeagleBone2 is released that has the same header configuration, but
different backing devices. It would be nice to have a solution for
that.

> In that case you have a OS independent hardware description, which can
> be even used even by the bootloader to access devices it knows not about
> at compile time.
>
> Other than that, I have no other objections.

I'm open to suggestions if anyone has any. I have not objections to a
fixup approach, but I'm not comfortable with anything that is fragile
to modifications to the fragment.

g.

2012-11-05 22:59:59

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Grant,

On Mon, Nov 5, 2012 at 2:14 PM, Grant Likely <[email protected]> wrote:
> On Mon, Nov 5, 2012 at 7:54 PM, Pantelis Antoniou
> <[email protected]> wrote:
>>> This handles many of the use cases, but it assumes that an overlay is
>>> board specific. If it ever is required to support multiple base boards
>>> with a single overlay file then there is a problem. The .dtb overlays
>>> generated in this manor cannot handle different phandles or nodes that
>>> are in a different place. On the other hand, the overlay source files
>>> should have no problem being compiled for multiple targets, so maybe
>>> it isn't an issue. Plus if dtc is installed on the target, then the
>>> live tree from /proc can be used as the reference when compiling the
>>> overlay.
>>
>> My worry is that this format is dependent on linking against the board
>> DTS file. One of the ideas thrown around here was that it might make
>> sense to store the DTB fragment in the EEPROM of the device.
>
> Right, that wouldn't work well if the base DT changed, or if a
> BeagleBone2 is released that has the same header configuration, but
> different backing devices. It would be nice to have a solution for
> that.
>
>> In that case you have a OS independent hardware description, which can
>> be even used even by the bootloader to access devices it knows not about
>> at compile time.
>>
>> Other than that, I have no other objections.
>
> I'm open to suggestions if anyone has any. I have not objections to a
> fixup approach, but I'm not comfortable with anything that is fragile
> to modifications to the fragment.

I am fairly new to the DT world so please bear with me, but how about
a method that resolves symbols the same way that the linux dynamic
linker does when shared libraries are loaded?

A separate table (similar to .PLT/GOT sections in the ELF object
format) could be created when the fragment is loaded, and the phandle
references could be fixed to point to the table offsets during compile
time. This table would be a second level indirection and the kernel
would populate this table with the in-kernel values of the phandles
that the dt fragment refers to.

This might involve changes to the DT core, but as such, this method
wouldn't suffer from the fragility problem of either base or fragment
DT trees being modified.

The table itself could be added to the tree by the compiler, and the
phandles could point to it (fixed). such phandles could be marked for
special handling to facilitate the 1-level indirection.

What do you think about this?

Regards,
Joel

2012-11-05 23:58:38

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2



Joel A Fernandes <[email protected]> wrote:

>Hi Grant,
>
>On Mon, Nov 5, 2012 at 2:14 PM, Grant Likely
><[email protected]> wrote:
>> I'm open to suggestions if anyone has any. I have not objections to a
>> fixup approach, but I'm not comfortable with anything that is fragile
>> to modifications to the fragment.
>
>I am fairly new to the DT world so please bear with me, but how about
>a method that resolves symbols the same way that the linux dynamic
>linker does when shared libraries are loaded?
>
>A separate table (similar to .PLT/GOT sections in the ELF object
>format) could be created when the fragment is loaded, and the phandle
>references could be fixed to point to the table offsets during compile
>time. This table would be a second level indirection and the kernel
>would populate this table with the in-kernel values of the phandles
>that the dt fragment refers to.

That's an interesting idea that is worth exploring. That would make it possible to avoid a fixup stage, but it also means that any parsing code must know how to handle the got-like table. It won't be backwards compatible with existing tools. It also wouldn't easily support the case of firmware applying the overlay and passing the resulting tree to the kernel. Hmmm.... Not being backwards compatible at the data level is a big problem. I really want a method that can resolve back to the current data format or is a compatible extension of it.

>
>This might involve changes to the DT core, but as such, this method
>wouldn't suffer from the fragility problem of either base or fragment
>DT trees being modified.
>
>The table itself could be added to the tree by the compiler, and the
>phandles could point to it (fixed). such phandles could be marked for
>special handling to facilitate the 1-level indirection.

That's part of the problem. Property values are essentially anaonymous data. There is no mechanism currently for marking data such as indicate which data values are phandles. If there were then it would be a simple matter to find all the phandles and fix them up.

We could possibly add data type suppplementary properties to the tree to solve that problem. They would have to be optional for the base tree to retain backwards compatibility, but could be required on overlays.

g.

--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

2012-11-06 03:06:45

by Joel A Fernandes

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Grant,

On Mon, Nov 5, 2012 at 5:58 PM, Grant Likely <[email protected]> wrote:
>
>
> Joel A Fernandes <[email protected]> wrote:
>
>>Hi Grant,
>>
>>On Mon, Nov 5, 2012 at 2:14 PM, Grant Likely
>><[email protected]> wrote:
>>> I'm open to suggestions if anyone has any. I have not objections to a
>>> fixup approach, but I'm not comfortable with anything that is fragile
>>> to modifications to the fragment.
>>
>>I am fairly new to the DT world so please bear with me, but how about
>>a method that resolves symbols the same way that the linux dynamic
>>linker does when shared libraries are loaded?
>>
>>A separate table (similar to .PLT/GOT sections in the ELF object
>>format) could be created when the fragment is loaded, and the phandle
>>references could be fixed to point to the table offsets during compile
>>time. This table would be a second level indirection and the kernel
>>would populate this table with the in-kernel values of the phandles
>>that the dt fragment refers to.
>
> That's an interesting idea that is worth exploring. That would make
> it possible to avoid a fixup stage, but it also means that any parsing
> code must know how to handle the got-like table. It won't be
> backwards compatible with existing tools. It also wouldn't easily
> support the case of firmware applying the overlay and passing the
> resulting tree to the kernel. Hmmm.... Not being backwards compatible
> at the data level is a big problem. I really want a method that can
> resolve back to the current data format or is a compatible extension
> of it.
>

Is it meaningful or feasible to make the table a part of the tree
structure itself? the table would initially be empty. If I'm right,
this is how the GOT tables in ELF objects that refer to unresolved
symbols in shared libraries are implemented as well, they are a part
of the object and are loaded into memory and fixed up. If the table is
a part of the DT data, the tools would then be able to parse such a
tree. We could enforce that the got-like tree would strictly follow a
predefined format.

Something along these lines would also avoid the need for a tree fix
up. Do you think this reduces the difficulty of backward compatibility
with the parsing tools and firmware loading?

>>
>>This might involve changes to the DT core, but as such, this method
>>wouldn't suffer from the fragility problem of either base or fragment
>>DT trees being modified.
>>
>>The table itself could be added to the tree by the compiler, and the
>>phandles could point to it (fixed). such phandles could be marked for
>>special handling to facilitate the 1-level indirection.
>
> That's part of the problem. Property values are essentially
> anaonymous data. There is no mechanism currently for marking data
> such as indicate which data values are phandles. If there were then
> it would be a simple matter to find all the phandles and fix them up.
>
> We could possibly add data type suppplementary properties to the tree
> to solve that problem. They would have to be optional for the base
> tree to retain backwards compatibility, but could be required on
> overlays.
>

Sure, so if we add data type supplementary properties to the tree to
indicate the data type as "indirect phandle", then kernel could refer
to the index in the got-like table to fetch the actual phandle address
(1-level of indirection), instead of directly using the address in the
data field.


Thanks and Regards,

Joel

2012-11-06 08:15:03

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

Hi Joel,

On Nov 6, 2012, at 4:06 AM, Joel A Fernandes wrote:

> Hi Grant,
>
> On Mon, Nov 5, 2012 at 5:58 PM, Grant Likely <[email protected]> wrote:
>>
>>
>> Joel A Fernandes <[email protected]> wrote:
>>
>>> Hi Grant,
>>>
>>> On Mon, Nov 5, 2012 at 2:14 PM, Grant Likely
>>> <[email protected]> wrote:
>>>> I'm open to suggestions if anyone has any. I have not objections to a
>>>> fixup approach, but I'm not comfortable with anything that is fragile
>>>> to modifications to the fragment.
>>>
>>> I am fairly new to the DT world so please bear with me, but how about
>>> a method that resolves symbols the same way that the linux dynamic
>>> linker does when shared libraries are loaded?
>>>
>>> A separate table (similar to .PLT/GOT sections in the ELF object
>>> format) could be created when the fragment is loaded, and the phandle
>>> references could be fixed to point to the table offsets during compile
>>> time. This table would be a second level indirection and the kernel
>>> would populate this table with the in-kernel values of the phandles
>>> that the dt fragment refers to.
>>
>> That's an interesting idea that is worth exploring. That would make
>> it possible to avoid a fixup stage, but it also means that any parsing
>> code must know how to handle the got-like table. It won't be
>> backwards compatible with existing tools. It also wouldn't easily
>> support the case of firmware applying the overlay and passing the
>> resulting tree to the kernel. Hmmm.... Not being backwards compatible
>> at the data level is a big problem. I really want a method that can
>> resolve back to the current data format or is a compatible extension
>> of it.
>>
>
> Is it meaningful or feasible to make the table a part of the tree
> structure itself? the table would initially be empty. If I'm right,
> this is how the GOT tables in ELF objects that refer to unresolved
> symbols in shared libraries are implemented as well, they are a part
> of the object and are loaded into memory and fixed up. If the table is
> a part of the DT data, the tools would then be able to parse such a
> tree. We could enforce that the got-like tree would strictly follow a
> predefined format.
>
> Something along these lines would also avoid the need for a tree fix
> up. Do you think this reduces the difficulty of backward compatibility
> with the parsing tools and firmware loading?

To be honest here, we are discussing a new object format.
There are a few twists IMO.

a) We definitely, absolutely, don't want to introduce anything that will
risk compatibility with devices already out there. The binary format
for device trees that don't need the dynamic resolution features we're
talking about here, should be be usable for those older devices.

b) Device tree is flexible enough to store the additional data in it's own
node format. So we shouldn't have any kind of binary data tacked on; this
ties with a) - make sure that the binary format doesn't change.

c) There is no need (at least AFAIKT) of having any other resolution type
than a phandle of a node.

d) Finally, for some use-cases the problem is simplified by not having
all the features of a true dynamic linker. For example for the capebus
case the 'base' DTS won't have any references to any fragments. It is only
the fragments that have unresolved references and only to the 'base' DTS.

>
>>>
>>> This might involve changes to the DT core, but as such, this method
>>> wouldn't suffer from the fragility problem of either base or fragment
>>> DT trees being modified.
>>>
>>> The table itself could be added to the tree by the compiler, and the
>>> phandles could point to it (fixed). such phandles could be marked for
>>> special handling to facilitate the 1-level indirection.
>>
>> That's part of the problem. Property values are essentially
>> anaonymous data. There is no mechanism currently for marking data
>> such as indicate which data values are phandles. If there were then
>> it would be a simple matter to find all the phandles and fix them up.
>>
>> We could possibly add data type suppplementary properties to the tree
>> to solve that problem. They would have to be optional for the base
>> tree to retain backwards compatibility, but could be required on
>> overlays.
>>
>
> Sure, so if we add data type supplementary properties to the tree to
> indicate the data type as "indirect phandle", then kernel could refer
> to the index in the got-like table to fetch the actual phandle address
> (1-level of indirection), instead of directly using the address in the
> data field.
>

I'm fine with this.

>
> Thanks and Regards,
>
> Joel

Regards

-- Pantelis

2012-11-06 11:17:21

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

On Tue, Nov 6, 2012 at 8:14 AM, Pantelis Antoniou
<[email protected]> wrote:
> On Nov 6, 2012, at 4:06 AM, Joel A Fernandes wrote:
>> Sure, so if we add data type supplementary properties to the tree to
>> indicate the data type as "indirect phandle", then kernel could refer
>> to the index in the got-like table to fetch the actual phandle address
>> (1-level of indirection), instead of directly using the address in the
>> data field.
>>
>
> I'm fine with this.

But if the data about phandles is already in the tree, then the need
for a GOT simply goes away. The phandles can be fixed up directly and
it is so much better because it works with existing parsing code after
the merge is applied.

g.

2012-11-06 13:55:12

by Pantelis Antoniou

[permalink] [raw]
Subject: Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2



Από το iPhone μου

6 Νοε 2012, 12:16, ο/η Grant Likely <[email protected]> έγραψε:

> On Tue, Nov 6, 2012 at 8:14 AM, Pantelis Antoniou
> <[email protected]> wrote:
>> On Nov 6, 2012, at 4:06 AM, Joel A Fernandes wrote:
>>> Sure, so if we add data type supplementary properties to the tree to
>>> indicate the data type as "indirect phandle", then kernel could refer
>>> to the index in the got-like table to fetch the actual phandle address
>>> (1-level of indirection), instead of directly using the address in the
>>> data field.
>>
>> I'm fine with this.
>
> But if the data about phandles is already in the tree, then the need
> for a GOT simply goes away. The phandles can be fixed up directly and
> it is so much better because it works with existing parsing code after
> the merge is applied.
>

Either way works. It is your call after all.
I agree that your method is simpler.
> g.

Regards

-- Pantelis-