Hi,
On 12/11/2024 11:26, Xianwei Zhao via B4 Relay wrote:
From: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
Add a new pinctrl driver for Amlogic A4 SoCs which share
the same register layout as the previous Amlogic S4.
Signed-off-by: Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>
---
drivers/pinctrl/meson/Kconfig | 6 +
drivers/pinctrl/meson/Makefile | 1 +
drivers/pinctrl/meson/pinctrl-amlogic-a4.c | 1335 ++++++++++++++++++++++++++++
3 files changed, 1342 insertions(+)
<snip>
+
+static int a4_of_gpio_xlate(struct gpio_chip *gc,
+ const struct of_phandle_args *gpiospec,
+ u32 *flags)
+{
+ int gpio_num;
+
+ u32 bank = gpiospec->args[0];
+ u32 offset = gpiospec->args[1];
+
+ if (gc->of_gpio_n_cells < 3) {
+ WARN_ON(1);
+ return -EINVAL;
+ }
This check is unnecessary, drop it, it's already done by of_xlate_and_get_gpiod_flags(),
and if you _really_ want to keep it, move it before accessing gpiospec->args array.
+
+ if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
+ return -EINVAL;
This one aswell, drop it.
+
+ switch (bank) {
Just use: switch (gpiospec->args[0]) {
You can even simplify further by dropping offset and using:
int gpio_num = gpiospec->args[1];
and then:
+ case AMLOGIC_GPIO_B:
+ if (offset >= GPIOB_NUM)
if (gpio_num >= GPIOB_NUM)
+ return -EINVAL;
+ gpio_num = GPIOB_0 + offset;
gpio_num += GPIOB_0;
+ break;
+
+ case AMLOGIC_GPIO_D:
+ if (offset >= GPIOD_NUM)
+ return -EINVAL;
+ gpio_num = GPIOD_0 + offset;
+ break;
+
+ case AMLOGIC_GPIO_E:
+ if (offset >= GPIOE_NUM)
+ return -EINVAL;
+ gpio_num = GPIOE_0 + offset;
+ break;
+
+ case AMLOGIC_GPIO_X:
+ if (offset >= GPIOX_NUM)
+ return -EINVAL;
+ gpio_num = GPIOX_0 + offset;
+ break;
+
+ case AMLOGIC_GPIO_T:
+ if (offset >= GPIOT_NUM)
+ return -EINVAL;
+ gpio_num = GPIOT_0 + offset;
+ break;
+
+ case AMLOGIC_GPIO_TEST_N:
+ if (offset != 0)
+ return -EINVAL;
+ gpio_num = GPIO_TEST_N;
+ break;
+
+ case AMLOGIC_GPIO_AO:
+ if (offset >= GPIOAO_NUM)
+ return -EINVAL;
+ gpio_num = GPIOAO_0 + offset;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ if (flags)
+ *flags = gpiospec->args[2];
+
+ return gpio_num;
+}
+
+static const struct meson_pinctrl_data a4_periphs_pinctrl_data = {
+ .name = "periphs-banks",
+ .pins = a4_periphs_pins,
+ .groups = a4_periphs_groups,
+ .funcs = a4_periphs_functions,
+ .banks = a4_periphs_banks,
+ .num_pins = ARRAY_SIZE(a4_periphs_pins),
+ .num_groups = ARRAY_SIZE(a4_periphs_groups),
+ .num_funcs = ARRAY_SIZE(a4_periphs_functions),
+ .num_banks = ARRAY_SIZE(a4_periphs_banks),
+ .pmx_ops = &meson_axg_pmx_ops,
+ .pmx_data = &a4_periphs_pmx_banks_data,
+ .parse_dt = &meson_a1_parse_dt_extra,
+ .of_gpio_n_cells = 3,
+ .of_xlate = &a4_of_gpio_xlate,
+};
+
+static const struct meson_pinctrl_data a4_aobus_pinctrl_data = {
+ .name = "aobus-banks",
+ .pins = a4_aobus_pins,
+ .groups = a4_aobus_groups,
+ .funcs = a4_aobus_functions,
+ .banks = a4_aobus_banks,
+ .num_pins = ARRAY_SIZE(a4_aobus_pins),
+ .num_groups = ARRAY_SIZE(a4_aobus_groups),
+ .num_funcs = ARRAY_SIZE(a4_aobus_functions),
+ .num_banks = ARRAY_SIZE(a4_aobus_banks),
+ .pmx_ops = &meson_axg_pmx_ops,
+ .pmx_data = &a4_aobus_pmx_banks_data,
+ .parse_dt = &meson_a1_parse_dt_extra,
+ .of_gpio_n_cells = 3,
+ .of_xlate = &a4_of_gpio_xlate,
+};
+
+static const struct of_device_id a4_pinctrl_dt_match[] = {
+ {
+ .compatible = "amlogic,a4-periphs-pinctrl",
+ .data = &a4_periphs_pinctrl_data,
+ },
+ {
+ .compatible = "amlogic,a4-aobus-pinctrl",
+ .data = &a4_aobus_pinctrl_data,
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, a4_pinctrl_dt_match);
+
+static struct platform_driver a4_pinctrl_driver = {
+ .probe = meson_pinctrl_probe,
+ .driver = {
+ .name = "amlogic-a4-pinctrl",
+ .of_match_table = a4_pinctrl_dt_match,
+ },
+};
+
+module_platform_driver(a4_pinctrl_driver);
+
+MODULE_AUTHOR("Xianwei Zhao <xianwei.zhao@xxxxxxxxxxx>");
+MODULE_DESCRIPTION("Pin controller and GPIO driver for Amlogic A4 SoC");
+MODULE_LICENSE("Dual BSD/GPL");
Thanks,
Neil
