[lkml]   [2013]   [Sep]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [RFCv2 3/3] ARM: dts: N900: Add SSI information
On 09/15/2013 02:44 PM, Sebastian Reichel wrote:
> Add SSI device tree data for OMAP34xx and Nokia N900.

What is an "SSI" device, ...

> diff --git a/Documentation/devicetree/bindings/hsi/omap_ssi.txt b/Documentation/devicetree/bindings/hsi/omap_ssi.txt

... and what is the "HSI" subsystem?

> +OMAP SSI controller bindings
> +
> +Required properties:
> +- compatible: Should be set to the following value
> + ti,omap3-ssi (applicable to OMAP34xx devices)

I think that'd be better phrased as:

Should include "ti,omap3-ssi".

The binding should not preclude other compatibel values being present
(e.g. a SoC-specific compatible value, to allow SoC-specific quirks to
be enabled later).

> +- ti,hwmods: Name of the hwmod associated to the controller, which
> + is "ssi".

I don't think we should add any more of that, for new bindings.

> +- reg: Contains SSI register address range (base address and
> + length).
> +- reg-names: Contains the names of the address ranges. It's
> + expected, that "sys" and "gdd" address ranges are
> + provided.

Why two entries in reg-names but only one in reg?

I think it'd be better to write:

reg-names: Contains the values "sys" and "gdd".
reg: Contains a register specifier for each entry in

A similar re-ordering/-wording would apply to interrupts/interrupt-names.

> +- ranges Required as an empty node


Why must ranges be empty? As long as the content correctly represents
the bus setup, why does the content matter at all. How about:

ranges Represents the bus address mapping between the main
controller node and the child nodes below.

> +Each port is represented as a sub-node of the ti,omap3-ssi device.
> +
> +Required Port sub-node properties:
> +- compatible: Should be set to the following value
> + ti,omap3-ssi-port (applicable to OMAP34xx devices)

Hmm. Is it really the case that there is 1 controller with n ports? Are
the ports really dependent upon some shared resource? Couldn't the ports
be represented as separate top-level SSI controllers?

> +- interrupts: Contains the interrupt information for the port.
> +- interrupt-names: Contains the names of the interrupts. It's expected,
> + that "mpu_irq0" and "mpu_irq1" are provided.

What exactly are those interrupts? "MPU" sounds like an external

> +- ti,ssi-cawake-gpio: Defines which GPIO pin is used to signify CAWAKE
> + events for the port. This is an optional board-specific
> + property. If it's missing the port will not be
> + enabled.

That also sounds like something that's a higher-level protocol, rather
than whatever low-level transport "SSI" implements. Should this be part
of a child node that represents the device attached to the SSI controller?

Does the SSI controller (or its ports) not need any clocks, resets,
regulators, ...?

 \ /
  Last update: 2013-09-23 23:01    [W:0.150 / U:0.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site