| Message ID | 1591775865-26872-1-git-send-email-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Support i.MX8 SoCs pinctrl drivers built as module | expand |
Hi Anson, Patch series mostly looks good to me. I have a comment about adding the MODULE_LICENSE. This is a pretty important change. Can you please add this change in a separate patch with a proper explanation of why it is needed. Most likely it is because it was forgotten in the previous patches. thanks, daniel. On 10.06.2020 10:57, Anson Huang wrote: > There are more and mroe requirements that SoC specific modules should be built > as module in order to support generic kernel image, such as Android GKI concept. > > This patch series supports i.MX8 SoCs pinctrl drivers to be built as module, > including i.MX8MQ/MM/MN/MP/QXP/QM/DXL SoCs, and it also supports building > i.MX common pinctrl driver and i.MX SCU common pinctrl driver as module. > > Compared to V3, the changes are as below: > - change the config dependency back to original; > - use function callbacks for SCU related functions, and all drivers > using SCU pinctrl driver need to initialize the function callbacks, > pinctrl-imx.c will check the SCU function callback and call it when > it is valid, then no build issue when PINCTRL_IMX is built in and > PINCTRL_IMX_SCU is built as module. > > Anson Huang (9): > pinctrl: imx: Support building SCU pinctrl driver as module > pinctrl: imx: Support building i.MX pinctrl driver as module > pinctrl: imx8mm: Support building as module > pinctrl: imx8mn: Support building as module > pinctrl: imx8mq: Support building as module > pinctrl: imx8mp: Support building as module > pinctrl: imx8qxp: Support building as module > pinctrl: imx8qm: Support building as module > pinctrl: imx8dxl: Support building as module > > drivers/pinctrl/freescale/Kconfig | 19 +++++----- > drivers/pinctrl/freescale/pinctrl-imx.c | 22 ++++++----- > drivers/pinctrl/freescale/pinctrl-imx.h | 57 ++++++++++++----------------- > drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 12 +++--- > drivers/pinctrl/freescale/pinctrl-imx8mm.c | 10 ++--- > drivers/pinctrl/freescale/pinctrl-imx8mn.c | 10 ++--- > drivers/pinctrl/freescale/pinctrl-imx8mp.c | 10 ++--- > drivers/pinctrl/freescale/pinctrl-imx8mq.c | 9 ++--- > drivers/pinctrl/freescale/pinctrl-imx8qm.c | 12 +++--- > drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 12 +++--- > drivers/pinctrl/freescale/pinctrl-scu.c | 6 +++ > 11 files changed, 86 insertions(+), 93 deletions(-) >
Hi, Daniel > Subject: Re: [PATCH V4 0/9] Support i.MX8 SoCs pinctrl drivers built as module > > Hi Anson, > > Patch series mostly looks good to me. I have a comment about adding > > the MODULE_LICENSE. This is a pretty important change. > > > Can you please add this change in a separate patch with a proper explanation > > of why it is needed. > > > Most likely it is because it was forgotten in the previous patches. Yes, it is obviously missed in the previous patches, as previously these pinctrl drivers do NOT support module build at all. And MODULE_LICENSE is a MUST when drivers supporting module build, build will report failure if module license missed, so I think it is also part of the module build support patch, do you mean it is better to add a separate patch to add the MODULE_LICENSE to all pinctrl drivers missing it? Maybe we can get more opinion from maintainer, I am NOT very sure whether it is better to separate the module license as a single patch.... thanks, Anson
On 11.06.2020 11:44, Anson Huang wrote: > Hi, Daniel > >> Subject: Re: [PATCH V4 0/9] Support i.MX8 SoCs pinctrl drivers built as module >> >> Hi Anson, >> >> Patch series mostly looks good to me. I have a comment about adding >> >> the MODULE_LICENSE. This is a pretty important change. >> >> >> Can you please add this change in a separate patch with a proper explanation >> >> of why it is needed. >> >> >> Most likely it is because it was forgotten in the previous patches. > Yes, it is obviously missed in the previous patches, as previously these pinctrl drivers > do NOT support module build at all. > > And MODULE_LICENSE is a MUST when drivers supporting module build, build will report failure > if module license missed, so I think it is also part of the module build support patch, > do you mean it is better to add a separate patch to add the MODULE_LICENSE to all pinctrl drivers missing it? > Maybe we can get more opinion from maintainer, I am NOT very sure whether it is better to separate > the module license as a single patch.... Hi Anson, This is my feeling. That the first patch in the series should add the MODULE_LICENSE to all of files. This keeps things simple, explains our intention with next patches. Daniel.