Re: [PATCH v2 4/4] ARM: DTS: AM43x: Add DSS node

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2953 bytes --]

On 13/03/14 19:46, Mark Rutland wrote:
> On Thu, Mar 13, 2014 at 08:58:29AM +0000, Sathya Prakash M R wrote:
>> Add device node for DSS module for AM4372. Both the
>> AM437x-Gp evm and Am43x-Epos evm use the same LCD panel.
>> The lcd timings are added in respective dts files.
>> Adds display pinctrl and enables required gpio.
>> Also set the right parent clock to the DSS clock.
>>
>> Signed-off-by: Sathya Prakash M R <sathyap@ti.com>
>> ---
>>  arch/arm/boot/dts/am4372.dtsi        |   28 +++++++++++++
>>  arch/arm/boot/dts/am437x-gp-evm.dts  |   77 ++++++++++++++++++++++++++++++++++
>>  arch/arm/boot/dts/am43x-epos-evm.dts |   73 ++++++++++++++++++++++++++++++++
>>  arch/arm/boot/dts/am43xx-clocks.dtsi |    2 +
>>  4 files changed, 180 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
>> index ea55a4e..b72a7df 100644
>> --- a/arch/arm/boot/dts/am4372.dtsi
>> +++ b/arch/arm/boot/dts/am4372.dtsi
>> @@ -684,6 +684,34 @@
>>  			num-cs = <4>;
>>                          status = "disabled";
>>                  };
>> +
>> +		dss: dss@4832A000 {
>> +			compatible = "ti,omap3-dss", "simple-bus";
> 
> This doesn't look right to me. I'm not sure it makes sense for
> "simple-bus" to be in the compatible list.
> 
> Are the child nodes usable in isolation, or are they dependent on the
> "ti,omap3-dss" node? What exactly does the "ti,omap3-dss" node
> represent?

The child nodes are dependent on the dss node.

The "ti,omap3-dss" represents the dss_core block of the OMAP display
subsystem. The dss_core is a small IP, a wrapper for the submodules,
handling things like clock and video path routing between the submodules
and the OMAP's other components (like the PRCM where we get clocks). It
also handles reset, so when dss_core is reset, all the submodules are reset.

The HW design is a bit odd, in my opinion, as the submodules are proper
IP blocks, and as far as I see, they could be designed to be independent
of each others. But they have not been designed so.

Having dss_core as the parent node for the submodules gives us automatic
runtime-pm handling, so when one submodule is enabled, it forces
dss_core to be enabled first. This makes the reset work right (i.e. we
don't accidentally reset dss_core when one of the submodules is in use),
and, as the dss_core is always needed to setup the clock and video path
routing, it gets properly initialized before any of the submodules will
use it.

What "simple-bus" mostly gives us here is automatic creation of the
platform devices for the submodules. We could also create the devices
for submodules in the driver of the dss_core. I did have that at some
point, but the "simple-bus" does identical job, and it seemed to make
sense to me.

Note that the same method is used for omap2/3/4 also, in the patches
that have been going around for some time in the lists.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]