Subject: RE: [PATCH V5 1/9] pinctrl: imx: Support building SCU pinctrl driver as
module

[...]

quoted

quoted

quoted

quoted

quoted

- * @dev: a pointer back to containing device
- * @base: the offset to the controller in virtual memory
- */
-struct imx_pinctrl {
-	struct device *dev;
-	struct pinctrl_dev *pctl;
-	void __iomem *base;
-	void __iomem *input_sel_base;
-	const struct imx_pinctrl_soc_info *info;
-	struct imx_pin_reg *pin_regs;
-	unsigned int group_index;
-	struct mutex mutex;
+	int (*imx_pinconf_get)(struct pinctrl_dev *pctldev, unsigned
+int

pin_id,

quoted

quoted

quoted

+			       unsigned long *config);
+	int (*imx_pinconf_set)(struct pinctrl_dev *pctldev, unsigned
+int

pin_id,

quoted

quoted

quoted

+			       unsigned long *configs, unsigned int

num_configs);

quoted

quoted

quoted

quoted

quoted

+	void (*imx_pinctrl_parse_pin)(struct imx_pinctrl *ipctl,
+				      unsigned int *pin_id, struct imx_pin *pin,
+				      const __be32 **list_p);

Compared with V4, this new implementation seems a bit complicated.
I guess we don't have to support PINCTRL_IMX=y &&
PINCTRL_IMX_SCU=m case.
Will that make the support a bit easier?

I am NOT sure if such scenario meets requirement, the fact is
other non-i.MX SoC also selects the PINCTRL_IMX which will make
PINCTRL_IMX=y, so in that case, even all i.MX PINCTRL drivers are
set to module, it will still have PINCTRL_IMX=y and
PINCTRL_IMX_SCU=m, then build will fail. And I believe the auto
build test may also cover such case and build error will be
reported, that is why this change is needed and with this change,
function is NOT impacted,

Is it possible to add some constrainst to make sure PINCTRL_IMX_SCU
value is the same as PINCTRL_IMX? Or combine them into one?
If we can do that, it may ease the implementation a lot and make the
code still clean.

Combine PINCTRL_IMX_SCU and PINCTRL_IMX is NOT making sense, since

for

quoted

non-SCU platforms, PINCTRL_IMX_SCU is NOT necessary, to make
PINCTRL_IMX_SCU same value as PINCTRL_IMX, unless make "select
PINCTRL_IMX_SCU" in PINCTRL_IMX, but that is also NOT making sense,
because, PINCTRL_IMX does NOT depends on PINCTRL_IMX_SCU at all.

PINCTRL_IMX_SCU could be conditionally compiled.
Something like follows:
obj-$(CONFIG_PINCTRL_IMX) += pinctrl-imx-core.o pinctrl-imx-core-y :=
pinctrl-imx.o
pinctrl-imx-core-$(CONFIG_PINCTRL_IMX_SCU) += pinctrl-scu.o

Can you try if this way could work?

It does NOW work, when PINCTRL_IMX=y and PINCTRL_IMX_SCU=m, config as below,
build will failed because some SCU functions NOT implemented, this is the exact reason
why have to use function callback. Currently, when PINCTRL_IMX=y, PINCTRL_IMX_SCU
MUST be =y, but that is NOT making enough sense for i.MX8M SoCs:

 CONFIG_PINCTRL_IMX=y
 CONFIG_PINCTRL_IMX_SCU=m
 CONFIG_PINCTRL_IMX8MM=y
 CONFIG_PINCTRL_IMX8MN=m
 CONFIG_PINCTRL_IMX8MP=m
 CONFIG_PINCTRL_IMX8MQ=m

aarch64-poky-linux-ld: drivers/pinctrl/freescale/pinctrl-imx.o: in function `imx_pinconf_dbg_show':
/home/b20788/workspace/stash/linux-next/drivers/pinctrl/freescale/pinctrl-imx.c:444: undefined reference to `imx_pinconf_get_scu'
aarch64-poky-linux-ld: drivers/pinctrl/freescale/pinctrl-imx.o: in function `imx_pinconf_get':
/home/b20788/workspace/stash/linux-next/drivers/pinctrl/freescale/pinctrl-imx.c:377: undefined reference to `imx_pinconf_get_scu'
aarch64-poky-linux-ld: drivers/pinctrl/freescale/pinctrl-imx.o: in function `imx_pinconf_set':
/home/b20788/workspace/stash/linux-next/drivers/pinctrl/freescale/pinctrl-imx.c:427: undefined reference to `imx_pinconf_set_scu'
aarch64-poky-linux-ld: drivers/pinctrl/freescale/pinctrl-imx.o: in function `imx_pinctrl_parse_groups':
/home/b20788/workspace/stash/linux-next/drivers/pinctrl/freescale/pinctrl-imx.c:633: undefined reference to `imx_pinctrl_parse_pin_scu'

Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel