Messages in this thread Patch in this message | | | Subject | Re: [PATCH 1/2] pinctrl: imx: Support building SCU pinctrl driver as module | From | Daniel Baluta <> | Date | Fri, 17 Jul 2020 11:39:53 +0300 |
| |
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@nxp.com>"); +MODULE_DESCRIPTION("NXP i.MX SCU common pinctrl driver"); +MODULE_LICENSE("GPL v2");
This can be in a separate patch.
| |