On 7/17/20 2:44 AM, Anson Huang wrote:
Hi, Daniel
Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as
module
On 7/16/20 6:21 PM, Anson Huang wrote:
Hi, Daniel
Subject: Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl
driver as module
Hi Anson,
Few comments inline:
On 7/16/20 6:06 PM, Anson Huang wrote:
To support building i.MX SCU pinctrl driver as module, below things
need to
be changed:
- Export SCU related functions and use "IS_ENABLED" instead of
"ifdef" to support SCU pinctrl driver user and itself to be
built as module;
- Use function callbacks for SCU related functions in pinctrl-imx.c
in order to support the scenario of PINCTRL_IMX is built in
while PINCTRL_IMX_SCU is built as module;
- All drivers using SCU pinctrl driver need to initialize the
SCU related function callback;
- Change PINCTR_IMX_SCU to tristate;
- Add module author, description and license.
With above changes, i.MX SCU pinctrl driver can be built as module.
There are a lot of changes here. I think it would be better to try to
split them
per functionality. One functional change per patch.
Actually, I ever tried to split them, but the function will be broken.
All the changes are just to support the module build. If split them,
the bisect will have pinctrl build or function broken.
Hi Anson,
I see your point and I know that this is a very hard task to get it right from
the first patches.
But let me suggest at least that:
- changes in drivers/pinctrl/freescale/pinctrl-imx.c (include file and
MODULE_ macros should go to a separate patch).
You meant in patch #2, the changes in Kconfig and the changes in .c file should
be split to 2 patches?
No. I mean look at path #1.
The section above should be placed in a separate patch.
static const struct of_device_id imx8qxp_pinctrl_of_match[] = {
diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
index 9df45d3..59b5f8a 100644
--- a/drivers/pinctrl/freescale/pinctrl-scu.c
+++ b/drivers/pinctrl/freescale/pinctrl-scu.c
@@ -7,6 +7,7 @@
#include <linux/err.h>
#include <linux/firmware/imx/sci.h>
+#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/platform_device.h>
@@ -123,3 +124,7 @@ void imx_pinctrl_parse_pin_scu(struct imx_pinctrl *ipctl,
pin_scu->mux_mode, pin_scu->config);
}
EXPORT_SYMBOL_GPL(imx_pinctrl_parse_pin_scu);
+
+MODULE_AUTHOR("Dong Aisheng<aisheng.dong@xxxxxxx>");
+MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver");
+MODULE_LICENSE("GPL v2");
This can be in a separate patch.
