On Wed, 2019-08-21 at 19:42 +0200, Guido Günther wrote:
Hi, On Tue, Aug 13, 2019 at 01:07:52PM +0200, Arnd Bergmann wrote:
On Tue, Aug 13, 2019 at 12:10 PM Guido Günther agx@sigxcpu.org wrote:
On Tue, Aug 13, 2019 at 10:08:44AM +0200, Arnd Bergmann wrote:
On Fri, Aug 9, 2019 at 6:24 PM Guido Günther agx@sigxcpu.org wrote:
This adds all the gpr registers and the define needed for selecting the input source in the imx-nwl drm bridge.
Signed-off-by: Guido Günther agx@sigxcpu.org
+#define IOMUXC_GPR0 0x00 +#define IOMUXC_GPR1 0x04 +#define IOMUXC_GPR2 0x08 +#define IOMUXC_GPR3 0x0c +#define IOMUXC_GPR4 0x10 +#define IOMUXC_GPR5 0x14 +#define IOMUXC_GPR6 0x18 +#define IOMUXC_GPR7 0x1c
(more of the same)
huh?
These are the names from the imx8MQ reference manual (general purpose registers, they lump together all sorts of things), it's the same on imx6/imx7):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/mfd/syscon/imx7-iomuxc-gpr.h+/* i.MX8Mq iomux gpr register field defines */ +#define IMX8MQ_GPR13_MIPI_MUX_SEL BIT(2)
I think this define should probably be local to the pinctrl driver, to ensure that no other drivers fiddle with the registers manually.
The purpose of these bits is for a driver to fiddle with them to select the input source. Similar on imx7 it's already used for e.g. the phy refclk in the pci controller:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/dwc/pci-imx6.c#n638That one should likely use either the clk interface or the phy interface instead.
The GPRs are not about pad configuration but gather all sorts of things (section 8.2.4 of the imx8mq reference manual): pcie setup, dsi related bits so I don't think this should be done via a pinctrl driver. Should we handle that differently than on imx6/7?
It would be nice to fix the existing code as well, but for the moment, I only think we should not add more of that.
Generally speaking, we can use syscon to do random things that don't have a subsystem of their own, but we should not use it to do things that have an existing driver framework like pinctrl, clock, reset, phy etc.
Since it's not an external pin i opted to use MUX_MMIO instead which seems like a good fit here. Does that make sense?
Yes, I agree. The i.MX6 display subsystem predates the mux framework, otherwise I would have used it for the LVDS and HDMI muxes in the first place. We should probably switch imx-drm over as well, in a backwards compatible fashion. The &mux definitions are already there in arch/arm/boot/dts/imx6q.dtsi.
regards Philipp