Re: [PATCH v2 1/2] pinctrl: pinctrl-imx: add support for set bits for general purpose registers

On Sat, Jul 14, 2012 at 10:43:03PM +0200, Linus Walleij wrote:

On Thu, Jul 12, 2012 at 11:07 AM, Dong Aisheng [off-list ref] wrote:

Hm, hm. This makes be ever more hesitant to have this in pinctrl.

Remeber again that nothing stops you from remapping the same register
range in another driver.

quoted

+#define IMX6Q_GPR0_CLOCK_8_MUX_SEL_MASK                (0x3 << 30)

This belongs in drivers/clk/*

Why funnel these register writes through pinctrl? Just remap that address
offset in the clk driver.

quoted

+#define IMX6Q_GPR0_DMAREQ_MUX_SEL7_MASK                BIT(7)

This belongs in drivers/dma/*

Same comments.

quoted

+#define IMX6Q_GPR1_PCIE_REQ_MASK               (0x3 << 30)

Looks like it belongs in some PCI driver or glue layer.

quoted

+#define IMX6Q_GPR1_MIPI_COLOR_SW               BIT(25)
+#define IMX6Q_GPR1_DPI_OFF                     BIT(24)

Looks related to some test or something...

And so on.

If you really wants a "funnel driver" doing all these diverse things,
I'd put it in drivers/mfd.

It's like driver drivers/mfd/anatop-mfd.c. They both store misc bits.
We may need a generic driver to simply provide register accessor.

Thanks
Richard

This whole issue appears in other systems so you're not alone
on this, for example the ux500 PRCMU driver has exactly these
properties.

I'm also contemplating drivers/syscon again, but
I have a hard time seeing it would be much more than another
drivers/mfd-similar construct.

I would really like input from Arnd and Samuel and other clever
people on the placement of drivers like this one :-/

But close address range proximity to the pin controller is not a
reason to have it in pinctrl.

Yours,
Linus Walleij