On Thu, Jul 06, 2023 at 07:45:22PM +0800, Huqiang Qin wrote:
> Add new pinctrl driver for Amlogic C3 SoCs which share the
a new
> same register layout as the previous Amloigc S4
Missing period at the end.
...
> +config PINCTRL_AMLOGIC_C3
> + tristate "Amlogic C3 SoC pinctrl driver"
> + depends on ARM64
> + select PINCTRL_MESON_AXG_PMX
> + default y
This default seems a bad cargo cult. Why ARM64 kernel should have all them be
opt-out, instead of opt-in? Shouldn't this be a distro problem?
...
> + MESON_PIN(GPIO_TEST_N)
Is it real one?
> +};
...
> + /* Bank A func6 */
> + GROUP(spi_a_mosi_a, 6),
> + GROUP(gen_clk_a4, 6),
> + GROUP(clk12_24_a, 6)
Leave trailing comma here as it's not a terminator.
...
> +static const char * const i2c2_groups[] = {
> + "i2c2_sda", "i2c2_scl"
Ditto.
> +};
> +
> +static const char * const i2c3_groups[] = {
> + "i2c3_sda_c", "i2c3_scl_c",
> + "i2c3_sda_x", "i2c3_scl_x",
> + "i2c3_sda_d", "i2c3_scl_d"
Ditto.
> +};
...
> +#ifdef CONFIG_OF
Drop this ugly ifdeffery.
> +static const struct of_device_id c3_pinctrl_dt_match[] = {
> + {
> + .compatible = "amlogic,c3-periphs-pinctrl",
> + .data = &c3_periphs_pinctrl_data,
> + },
> + { },
No comma for the terminator.
> +};
> +MODULE_DEVICE_TABLE(of, c3_pinctrl_dt_match);
> +#endif /* CONFIG_OF */
> +
> +static struct platform_driver c3_pinctrl_driver = {
> + .probe = meson_pinctrl_probe,
> + .driver = {
> + .name = "amlogic-c3-pinctrl",
> + .of_match_table = of_match_ptr(c3_pinctrl_dt_match),
Drop the rather problematic of_match_ptr().
> + },
> +};
--
With Best Regards,
Andy Shevchenko
