diff mbox series

pinctrl/meson: enable building as modules

Message ID 20201019230914.26300-1-khilman@baylibre.com (mailing list archive)
State New, archived
Headers show
Series pinctrl/meson: enable building as modules | expand

Commit Message

Kevin Hilman Oct. 19, 2020, 11:09 p.m. UTC
Enable pinctrl drivers for 64-bit Amlogic SoCs to be built as modules.

The default is still built-in, this only adds the option of building
as module.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
Tested on meson-sm1-khadas-vim3.

 drivers/pinctrl/meson/Kconfig                 | 17 +++++++++--------
 drivers/pinctrl/meson/pinctrl-meson-a1.c      |  4 +++-
 drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c |  3 +++
 drivers/pinctrl/meson/pinctrl-meson-axg.c     |  4 +++-
 drivers/pinctrl/meson/pinctrl-meson-g12a.c    |  4 +++-
 drivers/pinctrl/meson/pinctrl-meson-gxbb.c    |  4 +++-
 drivers/pinctrl/meson/pinctrl-meson-gxl.c     |  4 +++-
 drivers/pinctrl/meson/pinctrl-meson.c         |  9 +++++++++
 drivers/pinctrl/meson/pinctrl-meson.h         |  1 +
 drivers/pinctrl/meson/pinctrl-meson8-pmx.c    |  2 ++
 10 files changed, 39 insertions(+), 13 deletions(-)

Comments

Martin Blumenstingl Oct. 20, 2020, 7:56 p.m. UTC | #1
Hi Kevin,

On Tue, Oct 20, 2020 at 1:09 AM Kevin Hilman <khilman@baylibre.com> wrote:
[...]
> The default is still built-in, this only adds the option of building
> as module.
(as of today) the majority of the SoC specific pin controller drivers
use bool instead of tristate
my understanding is that tristate can be "dangerous" because if you
compile a driver as module then your kernel may not boot (I haven't
tried, but in that case the reset controller driver probably has to be
added to the initramfs).
is there something that we need to do about this? my thoughts: these
options default to "y" so it shouldn't be a problem unless someone
tinkers with their kernel configuration

[...]
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
> index 20683cd072bb..b467ab49539a 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
> @@ -152,6 +152,7 @@ int meson_pmx_get_funcs_count(struct pinctrl_dev *pcdev)
>
>         return pc->data->num_funcs;
>  }
> +EXPORT_SYMBOL_GPL(meson_pmx_get_funcs_count);
>
>  const char *meson_pmx_get_func_name(struct pinctrl_dev *pcdev,
>                                     unsigned selector)
> @@ -160,6 +161,7 @@ const char *meson_pmx_get_func_name(struct pinctrl_dev *pcdev,
>
>         return pc->data->funcs[selector].name;
>  }
> +EXPORT_SYMBOL_GPL(meson_pmx_get_func_name);
>
>  int meson_pmx_get_groups(struct pinctrl_dev *pcdev, unsigned selector,
>                          const char * const **groups,
> @@ -172,6 +174,7 @@ int meson_pmx_get_groups(struct pinctrl_dev *pcdev, unsigned selector,
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(meson_pmx_get_groups);
>
>  static int meson_pinconf_set_gpio_bit(struct meson_pinctrl *pc,
>                                       unsigned int pin,
> @@ -723,6 +726,7 @@ int meson8_aobus_parse_dt_extra(struct meson_pinctrl *pc)
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(meson8_aobus_parse_dt_extra);
>
>  int meson_a1_parse_dt_extra(struct meson_pinctrl *pc)
>  {
> @@ -732,6 +736,7 @@ int meson_a1_parse_dt_extra(struct meson_pinctrl *pc)
>
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(meson_a1_parse_dt_extra);
>
>  int meson_pinctrl_probe(struct platform_device *pdev)
>  {
> @@ -766,3 +771,7 @@ int meson_pinctrl_probe(struct platform_device *pdev)
>
>         return meson_gpiolib_register(pc);
>  }
> +EXPORT_SYMBOL_GPL(meson_pinctrl_probe);
> +
> +MODULE_LICENSE("GPL v2");
> +
please remove this blank line at EOF because there is already one in
the existing file (with this one there's now two of them)


Best regards,
Martin
Kevin Hilman Oct. 20, 2020, 9:13 p.m. UTC | #2
Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:

> Hi Kevin,
>
> On Tue, Oct 20, 2020 at 1:09 AM Kevin Hilman <khilman@baylibre.com> wrote:
> [...]
>> The default is still built-in, this only adds the option of building
>> as module.
>
> (as of today) the majority of the SoC specific pin controller drivers
> use bool instead of tristate
> my understanding is that tristate can be "dangerous" because if you
> compile a driver as module then your kernel may not boot (I haven't
> tried, but in that case the reset controller driver probably has to be
> added to the initramfs).
> is there something that we need to do about this?

Yes.  We need to test it.

> my thoughts: these
> options default to "y" so it shouldn't be a problem unless someone
> tinkers with their kernel configuration

Correct. I want to keep a working default upstream defconfig, but the
goal of this is to enable the tinkering. :)

I've been testing this, and meson pinctrl driver works fine as a module
(so far only tested in G12A and SM1).  Obviously, it depends on the
bootloader to have some sane default, and in general depending on the
bootloader for this is not a great idea.

In fact, I currently have a kernel booting on meson-sm1-khadas-vim3l
that has everything (including clocks, pinctrl, GPIO IRQs and PM
domains) built as modules.  The clock stuff requires a bit more work,
but there's a handful of patches to enable the rest of the drivers as
modules from Neil and myself over the last few days. 

Kevin

> [...]
>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
>> index 20683cd072bb..b467ab49539a 100644
>> --- a/drivers/pinctrl/meson/pinctrl-meson.c
>> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
>> @@ -152,6 +152,7 @@ int meson_pmx_get_funcs_count(struct pinctrl_dev *pcdev)
>>
>>         return pc->data->num_funcs;
>>  }
>> +EXPORT_SYMBOL_GPL(meson_pmx_get_funcs_count);
>>
>>  const char *meson_pmx_get_func_name(struct pinctrl_dev *pcdev,
>>                                     unsigned selector)
>> @@ -160,6 +161,7 @@ const char *meson_pmx_get_func_name(struct pinctrl_dev *pcdev,
>>
>>         return pc->data->funcs[selector].name;
>>  }
>> +EXPORT_SYMBOL_GPL(meson_pmx_get_func_name);
>>
>>  int meson_pmx_get_groups(struct pinctrl_dev *pcdev, unsigned selector,
>>                          const char * const **groups,
>> @@ -172,6 +174,7 @@ int meson_pmx_get_groups(struct pinctrl_dev *pcdev, unsigned selector,
>>
>>         return 0;
>>  }
>> +EXPORT_SYMBOL_GPL(meson_pmx_get_groups);
>>
>>  static int meson_pinconf_set_gpio_bit(struct meson_pinctrl *pc,
>>                                       unsigned int pin,
>> @@ -723,6 +726,7 @@ int meson8_aobus_parse_dt_extra(struct meson_pinctrl *pc)
>>
>>         return 0;
>>  }
>> +EXPORT_SYMBOL_GPL(meson8_aobus_parse_dt_extra);
>>
>>  int meson_a1_parse_dt_extra(struct meson_pinctrl *pc)
>>  {
>> @@ -732,6 +736,7 @@ int meson_a1_parse_dt_extra(struct meson_pinctrl *pc)
>>
>>         return 0;
>>  }
>> +EXPORT_SYMBOL_GPL(meson_a1_parse_dt_extra);
>>
>>  int meson_pinctrl_probe(struct platform_device *pdev)
>>  {
>> @@ -766,3 +771,7 @@ int meson_pinctrl_probe(struct platform_device *pdev)
>>
>>         return meson_gpiolib_register(pc);
>>  }
>> +EXPORT_SYMBOL_GPL(meson_pinctrl_probe);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +
> please remove this blank line at EOF because there is already one in
> the existing file (with this one there's now two of them)
>
>
> Best regards,
> Martin
diff mbox series

Patch

diff --git a/drivers/pinctrl/meson/Kconfig b/drivers/pinctrl/meson/Kconfig
index 3cb119105ddb..b2855e341a75 100644
--- a/drivers/pinctrl/meson/Kconfig
+++ b/drivers/pinctrl/meson/Kconfig
@@ -1,8 +1,9 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 menuconfig PINCTRL_MESON
-	bool "Amlogic SoC pinctrl drivers"
+	tristate "Amlogic SoC pinctrl drivers"
 	depends on ARCH_MESON
 	depends on OF
+	default y
 	select PINMUX
 	select PINCONF
 	select GENERIC_PINCONF
@@ -25,37 +26,37 @@  config PINCTRL_MESON8B
 	default y
 
 config PINCTRL_MESON_GXBB
-	bool "Meson gxbb SoC pinctrl driver"
+	tristate "Meson gxbb SoC pinctrl driver"
 	depends on ARM64
 	select PINCTRL_MESON8_PMX
 	default y
 
 config PINCTRL_MESON_GXL
-	bool "Meson gxl SoC pinctrl driver"
+	tristate "Meson gxl SoC pinctrl driver"
 	depends on ARM64
 	select PINCTRL_MESON8_PMX
 	default y
 
 config PINCTRL_MESON8_PMX
-	bool
+	tristate
 
 config PINCTRL_MESON_AXG
-	bool "Meson axg Soc pinctrl driver"
+	tristate "Meson axg Soc pinctrl driver"
 	depends on ARM64
 	select PINCTRL_MESON_AXG_PMX
 	default y
 
 config PINCTRL_MESON_AXG_PMX
-	bool
+	tristate
 
 config PINCTRL_MESON_G12A
-	bool "Meson g12a Soc pinctrl driver"
+	tristate "Meson g12a Soc pinctrl driver"
 	depends on ARM64
 	select PINCTRL_MESON_AXG_PMX
 	default y
 
 config PINCTRL_MESON_A1
-	bool "Meson a1 Soc pinctrl driver"
+	tristate "Meson a1 Soc pinctrl driver"
 	depends on ARM64
 	select PINCTRL_MESON_AXG_PMX
 	default y
diff --git a/drivers/pinctrl/meson/pinctrl-meson-a1.c b/drivers/pinctrl/meson/pinctrl-meson-a1.c
index 8abf750eac7e..79f5d753d7e1 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-a1.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-a1.c
@@ -925,6 +925,7 @@  static const struct of_device_id meson_a1_pinctrl_dt_match[] = {
 	},
 	{ },
 };
+MODULE_DEVICE_TABLE(of, meson_a1_pinctrl_dt_match);
 
 static struct platform_driver meson_a1_pinctrl_driver = {
 	.probe  = meson_pinctrl_probe,
@@ -934,4 +935,5 @@  static struct platform_driver meson_a1_pinctrl_driver = {
 	},
 };
 
-builtin_platform_driver(meson_a1_pinctrl_driver);
+module_platform_driver(meson_a1_pinctrl_driver);
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c b/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c
index e8931d9cf863..80c43683c789 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c
@@ -116,3 +116,6 @@  const struct pinmux_ops meson_axg_pmx_ops = {
 	.get_function_groups = meson_pmx_get_groups,
 	.gpio_request_enable = meson_axg_pmx_request_gpio,
 };
+EXPORT_SYMBOL_GPL(meson_axg_pmx_ops);
+
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c b/drivers/pinctrl/meson/pinctrl-meson-axg.c
index 072765db93d7..7bfecdfba177 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-axg.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c
@@ -1080,6 +1080,7 @@  static const struct of_device_id meson_axg_pinctrl_dt_match[] = {
 	},
 	{ },
 };
+MODULE_DEVICE_TABLE(of, meson_axg_pinctrl_dt_match);
 
 static struct platform_driver meson_axg_pinctrl_driver = {
 	.probe		= meson_pinctrl_probe,
@@ -1089,4 +1090,5 @@  static struct platform_driver meson_axg_pinctrl_driver = {
 	},
 };
 
-builtin_platform_driver(meson_axg_pinctrl_driver);
+module_platform_driver(meson_axg_pinctrl_driver);
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
index 41850e3c0091..cd9656b13836 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-g12a.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c
@@ -1410,6 +1410,7 @@  static const struct of_device_id meson_g12a_pinctrl_dt_match[] = {
 	},
 	{ },
 };
+MODULE_DEVICE_TABLE(of, meson_g12a_pinctrl_dt_match);
 
 static struct platform_driver meson_g12a_pinctrl_driver = {
 	.probe  = meson_pinctrl_probe,
@@ -1419,4 +1420,5 @@  static struct platform_driver meson_g12a_pinctrl_driver = {
 	},
 };
 
-builtin_platform_driver(meson_g12a_pinctrl_driver);
+module_platform_driver(meson_g12a_pinctrl_driver);
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
index d130c635f74b..d9f5c4caf78d 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
@@ -900,6 +900,7 @@  static const struct of_device_id meson_gxbb_pinctrl_dt_match[] = {
 	},
 	{ },
 };
+MODULE_DEVICE_TABLE(of, meson_gxbb_pinctrl_dt_match);
 
 static struct platform_driver meson_gxbb_pinctrl_driver = {
 	.probe		= meson_pinctrl_probe,
@@ -908,4 +909,5 @@  static struct platform_driver meson_gxbb_pinctrl_driver = {
 		.of_match_table = meson_gxbb_pinctrl_dt_match,
 	},
 };
-builtin_platform_driver(meson_gxbb_pinctrl_driver);
+module_platform_driver(meson_gxbb_pinctrl_driver);
+MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>");
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxl.c b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
index 32552d795bb2..51408996255b 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxl.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
@@ -861,6 +861,7 @@  static const struct of_device_id meson_gxl_pinctrl_dt_match[] = {
 	},
 	{ },
 };
+MODULE_DEVICE_TABLE(of, meson_gxl_pinctrl_dt_match);
 
 static struct platform_driver meson_gxl_pinctrl_driver = {
 	.probe		= meson_pinctrl_probe,
@@ -869,4 +870,5 @@  static struct platform_driver meson_gxl_pinctrl_driver = {
 		.of_match_table = meson_gxl_pinctrl_dt_match,
 	},
 };
-builtin_platform_driver(meson_gxl_pinctrl_driver);
+module_platform_driver(meson_gxl_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 20683cd072bb..b467ab49539a 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -152,6 +152,7 @@  int meson_pmx_get_funcs_count(struct pinctrl_dev *pcdev)
 
 	return pc->data->num_funcs;
 }
+EXPORT_SYMBOL_GPL(meson_pmx_get_funcs_count);
 
 const char *meson_pmx_get_func_name(struct pinctrl_dev *pcdev,
 				    unsigned selector)
@@ -160,6 +161,7 @@  const char *meson_pmx_get_func_name(struct pinctrl_dev *pcdev,
 
 	return pc->data->funcs[selector].name;
 }
+EXPORT_SYMBOL_GPL(meson_pmx_get_func_name);
 
 int meson_pmx_get_groups(struct pinctrl_dev *pcdev, unsigned selector,
 			 const char * const **groups,
@@ -172,6 +174,7 @@  int meson_pmx_get_groups(struct pinctrl_dev *pcdev, unsigned selector,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(meson_pmx_get_groups);
 
 static int meson_pinconf_set_gpio_bit(struct meson_pinctrl *pc,
 				      unsigned int pin,
@@ -723,6 +726,7 @@  int meson8_aobus_parse_dt_extra(struct meson_pinctrl *pc)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(meson8_aobus_parse_dt_extra);
 
 int meson_a1_parse_dt_extra(struct meson_pinctrl *pc)
 {
@@ -732,6 +736,7 @@  int meson_a1_parse_dt_extra(struct meson_pinctrl *pc)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(meson_a1_parse_dt_extra);
 
 int meson_pinctrl_probe(struct platform_device *pdev)
 {
@@ -766,3 +771,7 @@  int meson_pinctrl_probe(struct platform_device *pdev)
 
 	return meson_gpiolib_register(pc);
 }
+EXPORT_SYMBOL_GPL(meson_pinctrl_probe);
+
+MODULE_LICENSE("GPL v2");
+
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index f8b0ff9d419a..ff5372e0a475 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -10,6 +10,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/types.h>
+#include <linux/module.h>
 
 struct meson_pinctrl;
 
diff --git a/drivers/pinctrl/meson/pinctrl-meson8-pmx.c b/drivers/pinctrl/meson/pinctrl-meson8-pmx.c
index 66a908f9f13d..f767b6923f9f 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8-pmx.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8-pmx.c
@@ -100,3 +100,5 @@  const struct pinmux_ops meson8_pmx_ops = {
 	.get_function_groups = meson_pmx_get_groups,
 	.gpio_request_enable = meson8_pmx_request_gpio,
 };
+EXPORT_SYMBOL_GPL(meson8_pmx_ops);
+MODULE_LICENSE("GPL v2");