Re: [PATCH 06/23] ARM: OMAP: add OMAP5 DSI muxing

[Tony Lindgren <tony@atomide.com>]

* Tomi Valkeinen <tomi.valkeinen@ti.com> [140429 23:14]:
> On 29/04/14 20:38, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [140429 09:33]:
> >> On 29/04/14 19:19, Tomi Valkeinen wrote:
> >>> On 29/04/14 18:05, Tony Lindgren wrote:
> >>>
> >>>>> omap4_padconf_global is a syscon node, not pinctrl. As syscon just gives
> >>>>> a raw regmap to its memory area, the driver needs to know about the OMAP
> >>>>> control registers to use it.
> >>>>
> >>>> That would be probably best set up the same way we have already set up
> >>>> for example omap4_padconf_global: tisyscon@4a1005a0. Then drivers can
> >>>> access it using regmap, see how drivers/regulator/pbias-regulator.c
> >>>> sets up the pbias regulator with regmap for MMC.
> >>>
> >>> Right, but it means that the driver will contain platform specific code
> >>> for all the omap revisions it supports. Isn't that wrong?
> >>>
> >>> I already have a patch for DSI that uses the syscon-method, and it works
> >>> fine. But it's quite ugly, imo, to fiddle with the OMAP control
> >>> registers in a driver.
> > 
> > Anything using the system control module registers should be a separate
> > driver. And it should ideally be implemeting some Linux generic framework
> > that the consumer driver can then use. That leaves out the need to export
> > custom functions.
> 
> Ok.
> 
> > I guess we don't have a PHY framework for displays though, so how about
> > just a separate minimal driver under drivers/video/omap2 that uses the
> > syscon?
> 
> Well, this one is not really about DSI PHY. The CONTROL_DSIPHY register
> is not in the DSI PHY block, but in the control module, and it is used
> to enable/disable the pins (for omap4/5) and to set pull up/down for the
> pins (only for omap4). Oddly, for omap5, there's also a normal padconfig
> register for the DSI pins, but not so for omap4.
> 
> To me it looks like a pad config register. I don't know why there's a
> separate odd register for it and it's not using the normal padconfig system.
> 
> I'd like to use the pinctrl framework for it, but the pinctrl-single
> cannot handle such a funny register. So, if "Anything using the system
> control module registers should be a separate driver", then I guess I
> need to write a pinctrl driver for this single register?

Have you checked out pinctrl-single,bits binding? That should allow
you to map random bits in a single register to a pinctrl driver
instance.
 
> >> Oh, also, if I do that, I need to know both the SoC version and the DSS
> >> version in the driver.
> > 
> > Don't you get all you need in the compatible string? Something like
> > compatible ti,dss-phy-omap5?
> 
> We do use different compatible strings for different major versions of
> the DSS blocks, like ti,omap5-dsi. But that exactly same DSS block could
> be used on some other SoC, with different control stuff.
> 
> We could use separate compatible string for each SoC that uses DSS, but
> then we're really encoding the SoC version into the compatible string,
> not DSS version.
> 
> Of course, if there will be a separate driver managing the
> CONTROL_DSIPHY register, then that one should use compatible string
> specific to the SoC, as it's not a DSS driver at all.

Yeah probably using pinctrl-single,bits, or a separate driver with syscon
makes most sense here.

Regards,

Tony