On Fri, Nov 14, 2014 at 1:58 AM, Tony Lindgren [off-list ref] wrote:

* Paul Walmsley [off-list ref] [141113 15:01]:

quoted

Hi

On Thu, 13 Nov 2014, Tony Lindgren wrote:

quoted

* Tomi Valkeinen [off-list ref] [141113 03:33]:

quoted

On 12/11/14 17:02, Tony Lindgren wrote:

quoted

quoted

And, with a quick grep, I see CONTROL_DEVCONF1 touched in multiple
places in the kernel. I wonder if adding a pinmux entry for it could
cause some rather odd problems.

They can all use pinctrl-single no problem.

Can, but don't. That's my worry. If we touch the DEVCONF1 via pinmux,
and we have code in mach-omap2 that also touch DEVCONF1, without any
knowledge (and locking) between those...

Hmm yeah the McBSP clock mux could be racy as the mux register for
McBSP is treated as a clock. This register muxes the clock between
external pin and internal clock. Considering that this should be
selectable at board level as the external clock probably needs to be
used if level shifters are being used, it should be really handled by
pinctrl-single.

The other use for hsmmc.c and pdata-quirks.c for the one time mux for
MMC clock from the MMC clock pin. That can be done with pinctrl-single
from the MMC driver too for DT based booting.

Then we just have the save and restore of the registers for
off-idle.

quoted

So _maybe_ that's not an issue, as the pinmux config we have here is
fixed, and done once at boot time, and maybe the code in mach-omap2 that
touch DEVCONF1 is also ran just once and not at the same time as the
pinmux. But I don't know if that's so.

It seems we could just do a read-only check for McBSP in the clock
code for the mux register, or even completely drop that code from
cclock3xxx_data.c and start using the pinctrl for that mux.

Paul & Tero, got any comments here?

It's best to move all of the SCM register reads/writes to an SCM IP block
driver.  This driver would be the only entity that would touch the SCM IP
block registers - no other code on the system would touch it (perhaps
aside from anything needed for early init).  The SCM driver would enforce
mutual exclusion via a spinlock, so concurrent SCM register modifications
wouldn't flake out.  Then the SCM driver would register clocks with the
CCF, register pins with the pinctrl subsystem, etc. etc.

We actually do have that with pinctrl-single + syscon. We certainly
need to implement more Linux framework drivers for the SCM registers.
Things like regulators, clocks, and PHYs, but they should use
pinctrl-single + syscon. See the the pbias-regulator.c for example.

Looking at the McBSP clock handling, threre's yet more handling of
the same DEVCONF1 mux register in omap2_mcbsp_set_clks_src that gets
alled from omap_mcbsp_dai_set_dai_sysclk.

To me it seems that if we handle the DEVCONF with pinctrl-single, we
don't need most of the McBSP fck code or the omap2_mcbsp_set_clks_src.
Having the mux register as the clock enable register is not nice..
Who knows what the clock coming from the external pin might be :)

How will audio do dynamic muxing without that code?
The pin must be remuxed back to internal clock when audio stops, or
else PM breaks.


Gražvydas