Re: [PATCH] pinctrl/meson: enable building as modules

[Kevin Hilman <khilman@baylibre.com>]

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic