Re: [PATCH 2/5] capemgr: Add beaglebone's cape driver bindings

2013-03-27 Thread Pantelis Antoniou
Hi Grant

On Mar 26, 2013, at 7:36 PM, Grant Likely wrote:

> On Mon,  7 Jan 2013 20:51:03 +0200, Pantelis Antoniou 
>  wrote:
>> Document the beaglebone's cape driver bindings.
>> 
>> Signed-off-by: Pantelis Antoniou 
> 
> Hi Pantelis,
> 
> I'll go back and review the driver in a minute, but I'll start here
> since this is the data model that we'll be using for a long time.
> Overall this looks pretty sane. It's pretty well contained too which I
> like. Long term I do want to try and create some common patterns for
> overlay connections, but it's really hard to do that when this is the
> first serious attempt at implementing one.  :-) This is nicely contained
> to only the beaglebone use case which makes it easy to try out without
> forcing this exact data model on all users. If it works in other places,
> then fantastic! we've got our generic model. If not, then we can refine
> the interface for new boards without breaking beaglebone.
> 

Glad you liked it. I really tried hard to keep things nicely separated since
we're definitely on to uncharted waters.

We will definitely adopt this for the beagleboard too (maybe some other boards 
too)
so some rearrangement will take place for the v2 of the patches.

We're under the gun for the new beaglebone that's coming out shortly, so the 
new patches
will be out in a couple of weeks.

> Comments below...
> 
>> ---
>> .../devicetree/bindings/misc/capes-beaglebone.txt  | 110 
>> +
>> 1 file changed, 110 insertions(+)
>> create mode 100644 
>> Documentation/devicetree/bindings/misc/capes-beaglebone.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/misc/capes-beaglebone.txt 
>> b/Documentation/devicetree/bindings/misc/capes-beaglebone.txt
>> new file mode 100644
>> index 000..f73cab5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/capes-beaglebone.txt
>> @@ -0,0 +1,110 @@
>> +* TI Beaglebone DT Overlay Cape Driver
>> +
>> +Required properties:
>> +
>> +- compatible: "ti,bone-capemgr"
> 
> "capemgr" sounds like a software construct. Can you rename this to
> something that sounds more like describing concrete hardware? From that
> perspective, "ti,bone-capebus" would be appropriate here, regardless of
> where the driver actually lives in the kernel tree.
> 

Hum, well I guess so, that would work. Perhaps same as with 
ti,beagle-capebus.

But I thought using the work bus was out :)

> It would be better to be more specific here with the compatible
> property also. Put the actual board name into the compatible string.
> Following the lead of the board level compatible property, call it
> 'ti,am335x-bone-capebus". Newer boards can claim compatibilty with the
> older where appropriate.
> 

Let's not put am335x there, there's nothing SoC specific. The bone is
a am335x, but the beagleboard is not, etc.

There's also some crazy talk about compatibility between different board
families; we can keep it aside for now, but let's not assign anything
SoC specific. 

>> +
>> +- eeprom: Contains the phandle beaglebone's baseboard i2c eeprom.
>> +
>> +- baseboardmaps - node containing a list of supported
>> +beaglebone revisions; each node in should have the
>> +following properties:
>> +- board-name: The board name stored in the baseboard
>> +eeprom.
> 
> If it is stored in the baseboard eeprom, then why does it need to appear
> in the .dtb?

It needs to because there are slightly different revisions of the bone.
Defining the revisions it allows us to auto-load the dtbos containing the
differences for the baseboard revision.

In this way we keep the same kernel/software images working on all the
bone revisions, significantly reducing the support burden.

> 
>> +- compatible-name: The name which will be used for
>> +matching compatible capes.
> 
> What is the matching logic for this compatible capes? How does it get
> used?
> 

See below.

>> +
>> +- slots: node containing a list of slot nodes (which in the beaglebone
>> +case correspond to I2C addresses for dynamically probed capes,
>> +or an override slot definition for hardcoded capes.
>> +- eeprom: Contains the phandle beaglebone's cape i2c eeprom.
>> +
>> +It is possible to define override slots that will be activated
>> +when the baseboard matches, and/or if supplied on the kernel command
>> +line and/or when dynamically requested on runtime.
>> +In that case the slot is marked with
>> +- ti,cape-override: Marks a slot override.
>> +- compatible: any of the "runtime", "kernel", or any compatible-name
>> +  on a matching baseboardmap node.
>> +- Any of the eeprom-format-revision, board-name, version, manufacturer,
>> +  part-number, number-of-pins, serial-number, pin-usage, vdd-3v3exp,
>> +  vdd-5v, sys-5v, dc-supplied properties which fill in the simulated
>> +  cape's EEPROM fields. The part-number field is required, the rest
>> +  are optional taking into default values.
> 
> I co

Re: [PATCH 2/5] capemgr: Add beaglebone's cape driver bindings

2013-03-26 Thread Grant Likely
On Mon,  7 Jan 2013 20:51:03 +0200, Pantelis Antoniou 
 wrote:
> Document the beaglebone's cape driver bindings.
> 
> Signed-off-by: Pantelis Antoniou 

Hi Pantelis,

I'll go back and review the driver in a minute, but I'll start here
since this is the data model that we'll be using for a long time.
Overall this looks pretty sane. It's pretty well contained too which I
like. Long term I do want to try and create some common patterns for
overlay connections, but it's really hard to do that when this is the
first serious attempt at implementing one.  :-) This is nicely contained
to only the beaglebone use case which makes it easy to try out without
forcing this exact data model on all users. If it works in other places,
then fantastic! we've got our generic model. If not, then we can refine
the interface for new boards without breaking beaglebone.

Comments below...

> ---
>  .../devicetree/bindings/misc/capes-beaglebone.txt  | 110 
> +
>  1 file changed, 110 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/misc/capes-beaglebone.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/capes-beaglebone.txt 
> b/Documentation/devicetree/bindings/misc/capes-beaglebone.txt
> new file mode 100644
> index 000..f73cab5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/capes-beaglebone.txt
> @@ -0,0 +1,110 @@
> +* TI Beaglebone DT Overlay Cape Driver
> +
> +Required properties:
> +
> +- compatible: "ti,bone-capemgr"

"capemgr" sounds like a software construct. Can you rename this to
something that sounds more like describing concrete hardware? From that
perspective, "ti,bone-capebus" would be appropriate here, regardless of
where the driver actually lives in the kernel tree.

It would be better to be more specific here with the compatible
property also. Put the actual board name into the compatible string.
Following the lead of the board level compatible property, call it
'ti,am335x-bone-capebus". Newer boards can claim compatibilty with the
older where appropriate.

> +
> +- eeprom: Contains the phandle beaglebone's baseboard i2c eeprom.
> +
> +- baseboardmaps - node containing a list of supported
> + beaglebone revisions; each node in should have the
> + following properties:
> + - board-name: The board name stored in the baseboard
> + eeprom.

If it is stored in the baseboard eeprom, then why does it need to appear
in the .dtb?

> + - compatible-name: The name which will be used for
> + matching compatible capes.

What is the matching logic for this compatible capes? How does it get
used?

> +
> +- slots: node containing a list of slot nodes (which in the beaglebone
> + case correspond to I2C addresses for dynamically probed capes,
> + or an override slot definition for hardcoded capes.
> + - eeprom: Contains the phandle beaglebone's cape i2c eeprom.
> +
> + It is possible to define override slots that will be activated
> + when the baseboard matches, and/or if supplied on the kernel command
> + line and/or when dynamically requested on runtime.
> + In that case the slot is marked with
> + - ti,cape-override: Marks a slot override.
> + - compatible: any of the "runtime", "kernel", or any compatible-name
> +   on a matching baseboardmap node.
> + - Any of the eeprom-format-revision, board-name, version, manufacturer,
> +   part-number, number-of-pins, serial-number, pin-usage, vdd-3v3exp,
> +   vdd-5v, sys-5v, dc-supplied properties which fill in the simulated
> +   cape's EEPROM fields. The part-number field is required, the rest
> +   are optional taking into default values.

I could use some help understanding the use-case for the override slots.
It isn't clear to me how the override is intended to work. Can you
describe exactly what happens when an override slot is defined? Do
override slots replace detected slots, or are they separate?

> +
> +- capemaps: node contains list of cape mappings, which allow converting
> + from a part-number & version tuple to the filename of the dtbo file.
> + - part-number: part number contained in the EEPROM
> + - version node containing a
> + - version: specific version to map to
> + - dtbo: name of the dtbo file 

I think you'd be better off here defining a direct 1:1 mapping from
board name + revision to dtb filename. Maintaining a list of mappings in
the dtb file means it needs to be updated when new capes are created. It
would be nicer to drop the new overlay in the root filesystem (or initrd
if that is more convenient) and have the kernel know when to pick it up.

> +
> +Example:
> +bone_capemgr {
> + compatible = "ti,bone-capemgr";
> + status = "okay";
> +
> + eeprom = <&baseboard_eeprom>;
> +
> + baseboardmaps {
> + baseboard_beaglebone: board@0 {
> + board-name = "A335BONE";
> + compatible-name = "ti,beag

[PATCH 2/5] capemgr: Add beaglebone's cape driver bindings

2013-01-07 Thread Pantelis Antoniou
Document the beaglebone's cape driver bindings.

Signed-off-by: Pantelis Antoniou 
---
 .../devicetree/bindings/misc/capes-beaglebone.txt  | 110 +
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/capes-beaglebone.txt

diff --git a/Documentation/devicetree/bindings/misc/capes-beaglebone.txt 
b/Documentation/devicetree/bindings/misc/capes-beaglebone.txt
new file mode 100644
index 000..f73cab5
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/capes-beaglebone.txt
@@ -0,0 +1,110 @@
+* TI Beaglebone DT Overlay Cape Driver
+
+Required properties:
+
+- compatible: "ti,bone-capemgr"
+
+- eeprom: Contains the phandle beaglebone's baseboard i2c eeprom.
+
+- baseboardmaps - node containing a list of supported
+   beaglebone revisions; each node in should have the
+   following properties:
+   - board-name: The board name stored in the baseboard
+   eeprom.
+   - compatible-name: The name which will be used for
+   matching compatible capes.
+
+- slots: node containing a list of slot nodes (which in the beaglebone
+   case correspond to I2C addresses for dynamically probed capes,
+   or an override slot definition for hardcoded capes.
+   - eeprom: Contains the phandle beaglebone's cape i2c eeprom.
+
+   It is possible to define override slots that will be activated
+   when the baseboard matches, and/or if supplied on the kernel command
+   line and/or when dynamically requested on runtime.
+   In that case the slot is marked with
+   - ti,cape-override: Marks a slot override.
+   - compatible: any of the "runtime", "kernel", or any compatible-name
+ on a matching baseboardmap node.
+   - Any of the eeprom-format-revision, board-name, version, manufacturer,
+ part-number, number-of-pins, serial-number, pin-usage, vdd-3v3exp,
+ vdd-5v, sys-5v, dc-supplied properties which fill in the simulated
+ cape's EEPROM fields. The part-number field is required, the rest
+ are optional taking into default values.
+
+- capemaps: node contains list of cape mappings, which allow converting
+   from a part-number & version tuple to the filename of the dtbo file.
+   - part-number: part number contained in the EEPROM
+   - version node containing a
+   - version: specific version to map to
+   - dtbo: name of the dtbo file 
+
+Example:
+bone_capemgr {
+   compatible = "ti,bone-capemgr";
+   status = "okay";
+
+   eeprom = <&baseboard_eeprom>;
+
+   baseboardmaps {
+   baseboard_beaglebone: board@0 {
+   board-name = "A335BONE";
+   compatible-name = "ti,beaglebone";
+   };
+   };
+
+   slots {
+   slot@0 {
+   eeprom = <&cape_eeprom0>;
+   };
+
+   slot@1 {
+   eeprom = <&cape_eeprom1>;
+   };
+
+   slot@2 {
+   eeprom = <&cape_eeprom2>;
+   };
+
+   slot@3 {
+   eeprom = <&cape_eeprom3>;
+   };
+   };
+
+   /* mapping between board names and dtb objects */
+   capemaps {
+   /* Weather cape */
+   cape@0 {
+   part-number = "BB-BONE-WTHR-01";
+   version@00A0 {
+   version = "00A0";
+   dtbo = "cape-bone-weather-00A0.dtbo";
+   };
+   };
+   };
+};
+
+Example of the override syntax when used on a bone compatible foo board.
+
+{
+   ...
+
+   baseboardmaps {
+   ...
+   baseboard_beaglebone: board@0 {
+   board-name = "A335FOO";
+   compatible-name = "ti,foo";
+   };
+
+   slot@6 {
+   ti,cape-override;
+   compatible = "ti,foo";
+   board-name = "FOO-hardcoded";
+   version = "00A0";
+   manufacturer = "Texas Instruments";
+   part-number = "BB-BONE-FOO-01";
+   };
+   };
+
+};
+   
-- 
1.7.12

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