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

2014-03-14 Thread Mark Rutland
On Thu, Mar 13, 2014 at 06:22:54PM +, Tomi Valkeinen wrote:
 On 13/03/14 19:46, Mark Rutland wrote:
  On Thu, Mar 13, 2014 at 08:58:29AM +, 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 sath...@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.

Ok. Then simple-bus is not appropriate, as the dss node cannot be
ignored for the children to function.

 
 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.

The simple-bus compatible string is intended for busses which are
transparent (bar some address remapping expressed via ranges), and is
not intended as an annotation to get Linux to probe child nodes.

Any node with a simple-bus entry in the compatible list should either
be handled as a transparent bus, or optionally as the more specific bus
it claims to be (where some hardware configuration may be required
before children can be probed). Unfortunately Linux probes chidlren
regardless, which is arguable a Linux bug.

There's no reason to leak this issue into dts files. Please remove the
simple-bus string, and get the dss driver to probe children as
required -- as described above the dss node never makes sense as a
simple-bus.

 
 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.

And those should be fixed.

Cheers,
Mark.
--
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


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

2014-03-14 Thread Tomi Valkeinen
On 14/03/14 11:10, Mark Rutland wrote:

 The simple-bus compatible string is intended for busses which are
 transparent (bar some address remapping expressed via ranges), and is
 not intended as an annotation to get Linux to probe child nodes.
 
 Any node with a simple-bus entry in the compatible list should either
 be handled as a transparent bus, or optionally as the more specific bus
 it claims to be (where some hardware configuration may be required
 before children can be probed). Unfortunately Linux probes chidlren
 regardless, which is arguable a Linux bug.
 
 There's no reason to leak this issue into dts files. Please remove the
 simple-bus string, and get the dss driver to probe children as
 required -- as described above the dss node never makes sense as a
 simple-bus.

Ok. I'll remove the simple-bus, and make the dss_core register the
devices. I presume of_platform_populate() is fine for this? Seems to
work fine for registration, but I haven't figured out yet how to
unregister the devices (I get a crash in platform_device_del() if I just
call platform_device_unregister for the submodules).

 Tomi




signature.asc
Description: OpenPGP digital signature


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

2014-03-14 Thread Mark Rutland
On Fri, Mar 14, 2014 at 09:42:26AM +, Tomi Valkeinen wrote:
 On 14/03/14 11:10, Mark Rutland wrote:
 
  The simple-bus compatible string is intended for busses which are
  transparent (bar some address remapping expressed via ranges), and is
  not intended as an annotation to get Linux to probe child nodes.
  
  Any node with a simple-bus entry in the compatible list should either
  be handled as a transparent bus, or optionally as the more specific bus
  it claims to be (where some hardware configuration may be required
  before children can be probed). Unfortunately Linux probes chidlren
  regardless, which is arguable a Linux bug.
  
  There's no reason to leak this issue into dts files. Please remove the
  simple-bus string, and get the dss driver to probe children as
  required -- as described above the dss node never makes sense as a
  simple-bus.
 
 Ok. I'll remove the simple-bus, and make the dss_core register the
 devices. I presume of_platform_populate() is fine for this? Seems to
 work fine for registration, but I haven't figured out yet how to
 unregister the devices (I get a crash in platform_device_del() if I just
 call platform_device_unregister for the submodules).

I think of_platform_populate should be ok. It's not fantastic -- all
child nodes will be probed, regardless of whether you expect them to
exist, but it's not as broken as using simple-bus.

I'm unfortunately not familiar with how unregistration works.

I can't see anything obviously wrong in platform_device_del. Do you have
a backtrace?

Thanks,
Mark.
--
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


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

2014-03-14 Thread Tomi Valkeinen
On 14/03/14 12:14, Mark Rutland wrote:

 I can't see anything obviously wrong in platform_device_del. Do you have
 a backtrace?

Yes, below.

I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same thing
I do, so maybe I've got something wrong with the omapdss driver.

 Tomi

[   62.987335] Unable to handle kernel NULL pointer dereference at virtual 
address 0018
[   62.995910] pgd = eb6b8000
[   62.998779] [0018] *pgd=aa127831, *pte=, *ppte=
[   63.005462] Internal error: Oops: 17 [#1] SMP ARM
[   63.005462] Modules linked in: omapdss(-) [last unloaded: encoder_tfp410]
[   63.011779] CPU: 1 PID: 1021 Comm: rmmod Not tainted 
3.14.0-rc2-00057-gacd3401a1fea-dirty #69
[   63.025909] task: eb17a040 ti: ea14e000 task.ti: ea14e000
[   63.032287] PC is at release_resource+0x1c/0x84
[   63.032287] LR is at _raw_write_lock+0x50/0x58
[   63.041748] pc : [c004adc8]lr : [c05bbd5c]psr: 6113
[   63.041748] sp : ea14fde0  ip : ea14fdb8  fp : ea14fdf4
[   63.053833] r10:   r9 : ea14e000  r8 : c000f704
[   63.053833] r7 : 0081  r6 : bf0004fc  r5 : eb714400  r4 : eb6a9600
[   63.061798] r3 :   r2 : 0001  r1 : 0011  r0 : c08cec3c
[   63.071746] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[   63.071746] Control: 10c53c7d  Table: ab6b804a  DAC: 0015
[   63.081787] Process rmmod (pid: 1021, stack limit = 0xea14e248)
[   63.091735] Stack: (0xea14fde0 to 0xea15)
[   63.093109] fde0: 0001 eb714400 ea14fe0c ea14fdf8 c03953dc c004adb8 
eb714400 
[   63.101806] fe00: ea14fe24 ea14fe10 c0395808 c0395374 eb17a040 eb714400 
ea14fe3c ea14fe28
[   63.114715] fe20: bf000530 c0395800 eb714410  ea14fe64 ea14fe40 
c038fa5c bf000508
[   63.121795] fe40: eb214b80 eb6a9d30 ea14fe7c eb204410 bf032d98 eb20 
ea14fe7c ea14fe68
[   63.123352] fe60: bf022574 c038fa2c bf02254c eb204410 ea14fe8c ea14fe80 
c0395080 bf022558
[   63.140136] fe80: ea14fea4 ea14fe90 c0393494 c039506c eb204410 bf032d98 
ea14fec4 ea14fea8
[   63.149200] fea0: c0393efc c0393428  bf032d98 bf034438 bf0342b8 
ea14fedc ea14fec8
[   63.154327] fec0: c0393244 c0393e4c eb6fa300 bf032d98 ea14fef4 ea14fee0 
c0394570 c03931ec
[   63.166442] fee0: bf022800 0005 ea14ff04 ea14fef8 c03951d0 c039454c 
ea14ff14 ea14ff08
[   63.175048] ff00: bf0010e8 c03951c8 ea14ff34 ea14ff18 bf022534 bf0010e0 
bf0224f4 
[   63.183685] ff20: bf0342d0 0880 ea14ffa4 ea14ff38 c00b7f50 bf022500 
c000f564 
[   63.183685] ff40: bf0342d0 0880 ea14ff3c 70616d6f 00737364 00088ec9 
ea14ff84 ea14ff68
[   63.200927] ff60: c008fff0 c008fdd8 0001cec8 70616d6f 00737364 0081 
ea14ff94 ea14ff88
[   63.209533] ff80: c00900dc 0008ff08  0001cec8 70616d6f 00737364 
 ea14ffa8
[   63.218170] ffa0: c000f540 c00b7e0c 0001cec8 70616d6f bea28b10 0880 
bea28b10 0880
[   63.226776] ffc0: 0001cec8 70616d6f 00737364 0081 000acc00 0042 
00088ec9 
[   63.234619] ffe0: bea28b08 bea28af8 0001cda4 b6ed3390 6110 bea28b10 
 
[   63.234619] Backtrace: 
[   63.246612] [c004adac] (release_resource) from [c03953dc] 
(platform_device_del+0x74/0xa4)
[   63.246612]  r5:eb714400 r4:0001
[   63.246612] [c0395368] (platform_device_del) from [c0395808] 
(platform_device_unregister+0x14/0x20)
[   63.267150]  r5: r4:eb714400
[   63.273223] [c03957f4] (platform_device_unregister) from [bf000530] 
(dss_uninit_submodule_dev+0x34/0x40 [omapdss])
[   63.281799]  r4:eb714400 r3:eb17a040
[   63.281799] [bf0004fc] (dss_uninit_submodule_dev [omapdss]) from 
[c038fa5c] (device_for_each_child+0x3c/0x7c)
[   63.291839]  r4: r3:eb714410
[   63.303070] [c038fa20] (device_for_each_child) from [bf022574] 
(omap_dsshw_remove+0x28/0x70 [omapdss])
[   63.312255]  r6:eb20 r5:bf032d98 r4:eb204410
[   63.318237] [bf02254c] (omap_dsshw_remove [omapdss]) from [c0395080] 
(platform_drv_remove+0x20/0x24)
[   63.321807]  r4:eb204410 r3:bf02254c
[   63.331848] [c0395060] (platform_drv_remove) from [c0393494] 
(__device_release_driver+0x78/0xd0)
[   63.341644] [c039341c] (__device_release_driver) from [c0393efc] 
(driver_detach+0xbc/0xc0)
[   63.350708]  r5:bf032d98 r4:eb204410
[   63.352874] [c0393e40] (driver_detach) from [c0393244] 
(bus_remove_driver+0x64/0xcc)
[   63.352874]  r6:bf0342b8 r5:bf034438 r4:bf032d98 r3:
[   63.369018] [c03931e0] (bus_remove_driver) from [c0394570] 
(driver_unregister+0x30/0x50)
[   63.372894]  r4:bf032d98 r3:eb6fa300
[   63.381713] [c0394540] (driver_unregister) from [c03951d0] 
(platform_driver_unregister+0x14/0x18)
[   63.391418]  r4:0005 r3:bf022800
[   63.391845] [c03951bc] (platform_driver_unregister) from [bf0010e8] 
(dss_uninit_platform_driver+0x14/0x1c [omapdss])
[   63.401794] [bf0010d4] (dss_uninit_platform_driver [omapdss]) from 
[bf022534] (omap_dss_exit+0x40/0x58 [omapdss])
[   63.406951] [bf0224f4] (omap_dss_exit [omapdss]) from [c00b7f50] 
(SyS_delete_module+0x150/0x1e0)
[   

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

2014-03-14 Thread Tomi Valkeinen
On 14/03/14 12:19, Tomi Valkeinen wrote:
 On 14/03/14 12:14, Mark Rutland wrote:
 
 I can't see anything obviously wrong in platform_device_del. Do you have
 a backtrace?
 
 Yes, below.
 
 I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same thing
 I do, so maybe I've got something wrong with the omapdss driver.

Looks to me that the devices created by of_platform_populate() are not
unregisterable in all cases. The address resource created via
of_platform_populate() had NULL res-parent, which causes
release_resource to crash.

I can as well call of_platform_populate() in the platform init code at
boot time, and create the devices there for now.

 Tomi




signature.asc
Description: OpenPGP digital signature


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

2014-03-14 Thread Mark Rutland
On Fri, Mar 14, 2014 at 11:07:05AM +, Tomi Valkeinen wrote:
 On 14/03/14 12:19, Tomi Valkeinen wrote:
  On 14/03/14 12:14, Mark Rutland wrote:
  
  I can't see anything obviously wrong in platform_device_del. Do you have
  a backtrace?
  
  Yes, below.
  
  I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same thing
  I do, so maybe I've got something wrong with the omapdss driver.
 
 Looks to me that the devices created by of_platform_populate() are not
 unregisterable in all cases. The address resource created via
 of_platform_populate() had NULL res-parent, which causes
 release_resource to crash.

Hmm. I can't see that unregistering such devices ever works as you say,
given that __release_resource expects a non-NULL parent pointer. Either
we should be setting the parent pointer when initialising devices from
dt or we should teach __release_resource to not care. I'll have a go at
fixing that.

It looks like drivers/usb/dwc3/dwc3-exynos.c only unregisters the
top-level device, not children. This top-level device has no
IORESOURCE_{IO,MEM} resources judging by
arch/arm/boot/dts/exynos5250.dtsi, which would explain why that driver
isn't exploding: __release_resource will never get called.

Anton, Felipe: 

Does unregistering the parent ensure the children get cleaned up, or
does it leave them dangling in the dwc3-exynos driver?

Cheers,
Mark.
--
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


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

2014-03-14 Thread Felipe Balbi
On Fri, Mar 14, 2014 at 02:07:45PM +, Mark Rutland wrote:
 On Fri, Mar 14, 2014 at 11:07:05AM +, Tomi Valkeinen wrote:
  On 14/03/14 12:19, Tomi Valkeinen wrote:
   On 14/03/14 12:14, Mark Rutland wrote:
   
   I can't see anything obviously wrong in platform_device_del. Do you have
   a backtrace?
   
   Yes, below.
   
   I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same 
   thing
   I do, so maybe I've got something wrong with the omapdss driver.
  
  Looks to me that the devices created by of_platform_populate() are not
  unregisterable in all cases. The address resource created via
  of_platform_populate() had NULL res-parent, which causes
  release_resource to crash.
 
 Hmm. I can't see that unregistering such devices ever works as you say,
 given that __release_resource expects a non-NULL parent pointer. Either
 we should be setting the parent pointer when initialising devices from
 dt or we should teach __release_resource to not care. I'll have a go at
 fixing that.
 
 It looks like drivers/usb/dwc3/dwc3-exynos.c only unregisters the
 top-level device, not children. This top-level device has no
 IORESOURCE_{IO,MEM} resources judging by
 arch/arm/boot/dts/exynos5250.dtsi, which would explain why that driver
 isn't exploding: __release_resource will never get called.
 
 Anton, Felipe: 
 
 Does unregistering the parent ensure the children get cleaned up, or
 does it leave them dangling in the dwc3-exynos driver?

you should platform_device_unregister() for each children and
dwc3-exynos does that just fine:

167 static int dwc3_exynos_remove(struct platform_device *pdev)
168 {
169 struct dwc3_exynos  *exynos = platform_get_drvdata(pdev);
170 
171 device_for_each_child(pdev-dev, NULL, dwc3_exynos_remove_child);
^^

172 platform_device_unregister(exynos-usb2_phy);
173 platform_device_unregister(exynos-usb3_phy);
174 
175 clk_disable_unprepare(exynos-clk);
176 
177 return 0;
178 }

for each child of this device, we call dwc3_exynos_remove_child(), which
will:


94 static int dwc3_exynos_remove_child(struct device *dev, void *unused)
95 {
96 struct platform_device *pdev = to_platform_device(dev);
97 
98 platform_device_unregister(pdev);
99 
100return 0;
101 }

-- 
balbi


signature.asc
Description: Digital signature


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

2014-03-14 Thread Tomi Valkeinen
On 14/03/14 18:04, Felipe Balbi wrote:
 On Fri, Mar 14, 2014 at 02:07:45PM +, Mark Rutland wrote:
 On Fri, Mar 14, 2014 at 11:07:05AM +, Tomi Valkeinen wrote:
 On 14/03/14 12:19, Tomi Valkeinen wrote:
 On 14/03/14 12:14, Mark Rutland wrote:

 I can't see anything obviously wrong in platform_device_del. Do you have
 a backtrace?

 Yes, below.

 I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same 
 thing
 I do, so maybe I've got something wrong with the omapdss driver.

 Looks to me that the devices created by of_platform_populate() are not
 unregisterable in all cases. The address resource created via
 of_platform_populate() had NULL res-parent, which causes
 release_resource to crash.

 Hmm. I can't see that unregistering such devices ever works as you say,
 given that __release_resource expects a non-NULL parent pointer. Either
 we should be setting the parent pointer when initialising devices from
 dt or we should teach __release_resource to not care. I'll have a go at
 fixing that.

 It looks like drivers/usb/dwc3/dwc3-exynos.c only unregisters the
 top-level device, not children. This top-level device has no
 IORESOURCE_{IO,MEM} resources judging by
 arch/arm/boot/dts/exynos5250.dtsi, which would explain why that driver
 isn't exploding: __release_resource will never get called.

 Anton, Felipe: 

 Does unregistering the parent ensure the children get cleaned up, or
 does it leave them dangling in the dwc3-exynos driver?
 
 you should platform_device_unregister() for each children and
 dwc3-exynos does that just fine:

Yes, that's what I do also, and it crashes. What Mark said above about
unregistering never working for such devices is correct, but I don't
know why he said dwc3-exynos only unregisters the top level device.

such devices above meaning devices with a 'reg' defined in the DT
data, if I'm not mistaken.

So at the moment, I think of_platform_populate() and
platform_device_unregister() combination in a driver is a bit risky.
Work fine for certain cases, not for some other.

 Tomi




signature.asc
Description: OpenPGP digital signature


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

2014-03-14 Thread Mark Rutland
On Fri, Mar 14, 2014 at 04:34:19PM +, Tomi Valkeinen wrote:
 On 14/03/14 18:04, Felipe Balbi wrote:
  On Fri, Mar 14, 2014 at 02:07:45PM +, Mark Rutland wrote:
  On Fri, Mar 14, 2014 at 11:07:05AM +, Tomi Valkeinen wrote:
  On 14/03/14 12:19, Tomi Valkeinen wrote:
  On 14/03/14 12:14, Mark Rutland wrote:
 
  I can't see anything obviously wrong in platform_device_del. Do you have
  a backtrace?
 
  Yes, below.
 
  I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same 
  thing
  I do, so maybe I've got something wrong with the omapdss driver.
 
  Looks to me that the devices created by of_platform_populate() are not
  unregisterable in all cases. The address resource created via
  of_platform_populate() had NULL res-parent, which causes
  release_resource to crash.
 
  Hmm. I can't see that unregistering such devices ever works as you say,
  given that __release_resource expects a non-NULL parent pointer. Either
  we should be setting the parent pointer when initialising devices from
  dt or we should teach __release_resource to not care. I'll have a go at
  fixing that.
 
  It looks like drivers/usb/dwc3/dwc3-exynos.c only unregisters the
  top-level device, not children. This top-level device has no
  IORESOURCE_{IO,MEM} resources judging by
  arch/arm/boot/dts/exynos5250.dtsi, which would explain why that driver
  isn't exploding: __release_resource will never get called.
 
  Anton, Felipe: 
 
  Does unregistering the parent ensure the children get cleaned up, or
  does it leave them dangling in the dwc3-exynos driver?
  
  you should platform_device_unregister() for each children and
  dwc3-exynos does that just fine:
 
 Yes, that's what I do also, and it crashes. What Mark said above about
 unregistering never working for such devices is correct, but I don't
 know why he said dwc3-exynos only unregisters the top level device.

Because I'd failed to read the code correctly. It does indeed unregister
each of the children via platform_device_unregister.

Apologies for the confusion there.

 such devices above meaning devices with a 'reg' defined in the DT
 data, if I'm not mistaken.

Yes. As far as I can see, IORESOURCE_MEM resources instantiated from reg
entries in dt do not have their parent pointer initialised, and will
cause __release_resource to blow up. A quick inspection of resources on
my TC2 shows this to be the case -- the parent, sibling, and child
pointers are all NULL.

I'm at a loss to explain how the dwc3-exynos driver can call
platform_device_unregister on devices with such resource and not blow
up. Is anyone able to test dwc3_exynos_remove?

 So at the moment, I think of_platform_populate() and
 platform_device_unregister() combination in a driver is a bit risky.
 Work fine for certain cases, not for some other.

It certainly looks to be broken at the moment, but it should be fixable.

Thanks,
Mark.
--
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


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

2014-03-13 Thread Sathya Prakash M R
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 sath...@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;
+   reg = 0x4832A000 0x200;
+   ti,hwmods = dss_core;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges;
+
+   dispc@4832A400 {
+   compatible = ti,omap3-dispc;
+   reg = 0x4832A400 0x400;
+   interrupts = GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH;
+   ti,hwmods = dss_dispc;
+   };
+
+   dpi: encoder@0 {
+   compatible = ti,omap3-dpi;
+   };
+
+   rfbi: rfbi@4832A800 {
+   compatible = ti,omap3-rfbi;
+   reg = 0x4832A800 0x100;
+   ti,hwmods = dss_rfbi;
+   };
+
+   };
+
};
 
clocks {
diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts 
b/arch/arm/boot/dts/am437x-gp-evm.dts
index 2e79bda..a178e8d 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -24,6 +24,33 @@
brightness-levels = 0 51 53 56 62 75 101 152 255;
default-brightness-level = 8;
};
+
+   aliases {
+   display0 = lcd0;
+   };
+
+   lcd0: display {
+   compatible = osddisplays,osd057T0559-34ts, panel-dpi;
+   label = lcd;
+   panel-timing {
+   clock-frequency = 3300;
+   hactive = 800;
+   vactive = 480;
+   hfront-porch = 210;
+   hback-porch = 16;
+   hsync-len = 30;
+   vback-porch = 10;
+   vfront-porch = 22;
+   vsync-len = 13;
+   hsync-active = 0;
+   vsync-active = 0;
+   de-active = 1;
+   pixelclk-active = 1;
+   };
+   lcd_in: endpoint {
+   remote-endpoint = dpi_out;
+   };
+   };
 };
 
 am43xx_pinmux {
@@ -46,6 +73,40 @@
0x164 MUX_MODE0   /* 
eCAP0_in_PWM0_out.eCAP0_in_PWM0_out MODE0 */
;
};
+
+   dss_pinctrl: dss_pinctrl {
+   pinctrl-single,pins = 
+   0x020 (PIN_OUTPUT_PULLUP | MUX_MODE1) /*gpmc ad 8 - 
DSS DATA 23 */
+   0x024 (PIN_OUTPUT_PULLUP | MUX_MODE1)
+   0x028 (PIN_OUTPUT_PULLUP | MUX_MODE1)
+   0x02C (PIN_OUTPUT_PULLUP | MUX_MODE1)
+   0x030 (PIN_OUTPUT_PULLUP | MUX_MODE1)
+   0x034 (PIN_OUTPUT_PULLUP | MUX_MODE1)
+   0x038 (PIN_OUTPUT_PULLUP | MUX_MODE1)
+   0x03C (PIN_OUTPUT_PULLUP | MUX_MODE1) /*gpmc ad 15 - 
DSS DATA 16 */
+   0x0A0 (PIN_OUTPUT_PULLUP | MUX_MODE0) /* DSS DATA 0 */
+   0x0A4 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0A8 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0AC (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0B0 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0B4 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0B8 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0BC (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0C0 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0C4 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0C8 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0CC (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0D0 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0D4 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0D8 (PIN_OUTPUT_PULLUP | MUX_MODE0)
+   0x0DC 

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

2014-03-13 Thread Tomi Valkeinen
On 13/03/14 10:58, 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 sath...@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;
 + reg = 0x4832A000 0x200;
 + ti,hwmods = dss_core;
 + #address-cells = 1;
 + #size-cells = 1;
 + ranges;
 +
 + dispc@4832A400 {
 + compatible = ti,omap3-dispc;
 + reg = 0x4832A400 0x400;
 + interrupts = GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH;
 + ti,hwmods = dss_dispc;
 + };
 +
 + dpi: encoder@0 {
 + compatible = ti,omap3-dpi;
 + };
 +
 + rfbi: rfbi@4832A800 {
 + compatible = ti,omap3-rfbi;
 + reg = 0x4832A800 0x100;
 + ti,hwmods = dss_rfbi;
 + };
 +
 + };
 +

This seems to be based on old version of the DSS DT. Please rebase this
on top of the latest one:

git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git work/dss-dt

You also need to split this into smaller pieces: the dts clock fix, the
am4372.dtsi, and the board changes.

 Tomi




signature.asc
Description: OpenPGP digital signature


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

2014-03-13 Thread Sathya Prakash

On Thursday 13 March 2014 03:51 PM, Tomi Valkeinen wrote:

On 13/03/14 10:58, 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 sath...@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;
+   reg = 0x4832A000 0x200;
+   ti,hwmods = dss_core;
+   #address-cells = 1;
+   #size-cells = 1;
+   ranges;
+
+   dispc@4832A400 {
+   compatible = ti,omap3-dispc;
+   reg = 0x4832A400 0x400;
+   interrupts = GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH;
+   ti,hwmods = dss_dispc;
+   };
+
+   dpi: encoder@0 {
+   compatible = ti,omap3-dpi;
+   };
+
+   rfbi: rfbi@4832A800 {
+   compatible = ti,omap3-rfbi;
+   reg = 0x4832A800 0x100;
+   ti,hwmods = dss_rfbi;
+   };
+
+   };
+

This seems to be based on old version of the DSS DT. Please rebase this
on top of the latest one:

git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git work/dss-dt

Thanks. maybe i missed couple of changes. Will do that.


You also need to split this into smaller pieces: the dts clock fix, the
am4372.dtsi, and the board changes.

ok sure.


  Tomi




--
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


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

2014-03-13 Thread Mark Rutland
On Thu, Mar 13, 2014 at 08:58:29AM +, 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 sath...@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?

Thanks,
Mark.

 + reg = 0x4832A000 0x200;
 + ti,hwmods = dss_core;
 + #address-cells = 1;
 + #size-cells = 1;
 + ranges;
 +
 + dispc@4832A400 {
 + compatible = ti,omap3-dispc;
 + reg = 0x4832A400 0x400;
 + interrupts = GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH;
 + ti,hwmods = dss_dispc;
 + };
 +
 + dpi: encoder@0 {
 + compatible = ti,omap3-dpi;
 + };
 +
 + rfbi: rfbi@4832A800 {
 + compatible = ti,omap3-rfbi;
 + reg = 0x4832A800 0x100;
 + ti,hwmods = dss_rfbi;
 + };
 +
 + };
 +
   };
--
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


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

2014-03-13 Thread Tomi Valkeinen
On 13/03/14 19:46, Mark Rutland wrote:
 On Thu, Mar 13, 2014 at 08:58:29AM +, 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 sath...@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




signature.asc
Description: OpenPGP digital signature