| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3e1b23e9-d5a3-4b3d-973c-546b994e3ae2@wanadoo.fr>
Date: Fri, 18 Oct 2024 17:51:44 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: xianwei.zhao@...ogic.com, Linus Walleij <linus.walleij@...aro.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Neil Armstrong <neil.armstrong@...aro.org>,
Kevin Hilman <khilman@...libre.com>, Jerome Brunet <jbrunet@...libre.com>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
Bartosz Golaszewski <brgl@...ev.pl>
Cc: linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-amlogic@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] pinctrl: meson: Add driver support for Amlogic A4
SoCs
Le 18/10/2024 à 10:10, Xianwei Zhao via B4 Relay a écrit :
> From: Xianwei Zhao <xianwei.zhao@...ogic.com>
>
> 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@...ogic.com>
> ---
> drivers/pinctrl/meson/Kconfig | 6 +
> drivers/pinctrl/meson/Makefile | 1 +
> drivers/pinctrl/meson/pinctrl-amlogic-a4.c | 1253 ++++++++++++++++++++++++++++
> 3 files changed, 1260 insertions(+)
Hi,
a few nitpicks below.
...
> +
> +static struct meson_pmx_group a4_periphs_groups[] = {
I think that struct meson_pmx_group could be const.
(same for a4_aobus_groups above)
> + /* func0 as GPIO */
> + GPIO_GROUP(GPIOE_0),
> + GPIO_GROUP(GPIOE_1),
> +
...
> +static struct meson_pmx_func a4_periphs_functions[] = {
I think that struct meson_pmx_func could be const.
(a4_aobus_functions above as well)
> + FUNCTION(gpio_periphs),
> + FUNCTION(uart_a),
> + FUNCTION(uart_b),
> + FUNCTION(uart_d),
> + FUNCTION(uart_e),
> + FUNCTION(i2c0),
> + FUNCTION(i2c1),
> + FUNCTION(i2c2),
> + FUNCTION(i2c3),
> + FUNCTION(pwm_a),
> + FUNCTION(pwm_b),
> + FUNCTION(pwm_c),
> + FUNCTION(pwm_d),
> + FUNCTION(pwm_e),
> + FUNCTION(pwm_f),
> + FUNCTION(pwm_g),
> + FUNCTION(pwm_h),
> + FUNCTION(remote_out),
> + FUNCTION(remote_in),
> + FUNCTION(dcon_led),
> + FUNCTION(spinf),
> + FUNCTION(lcd),
> + FUNCTION(jtag_1),
> + FUNCTION(gen_clk),
> + FUNCTION(clk12_24),
> + FUNCTION(emmc),
> + FUNCTION(nand),
> + FUNCTION(spi_a),
> + FUNCTION(spi_b),
> + FUNCTION(pdm),
> + FUNCTION(sdio),
> + FUNCTION(eth),
> + FUNCTION(mic_mute),
> + FUNCTION(mclk),
> + FUNCTION(tdm),
> + FUNCTION(spdif_in),
> + FUNCTION(spdif_out)
> +};
> +
> +static struct meson_bank a4_periphs_banks[] = {
I think that both struct meson_bank could be const.
> + /* name first last irq pullen pull dir out in */
> + BANK_DS("E", GPIOE_0, GPIOE_1, 14, 15,
> + 0x43, 0, 0x44, 0, 0x42, 0, 0x41, 0, 0x40, 0, 0x47, 0),
> + BANK_DS("D", GPIOD_0, GPIOD_15, 16, 31,
> + 0x33, 0, 0x34, 0, 0x32, 0, 0x31, 0, 0x30, 0, 0x37, 0),
> + BANK_DS("B", GPIOB_0, GPIOB_13, 0, 13,
> + 0x63, 0, 0x64, 0, 0x62, 0, 0x61, 0, 0x60, 0, 0x67, 0),
> + BANK_DS("X", GPIOX_0, GPIOX_17, 55, 72,
> + 0x13, 0, 0x14, 0, 0x12, 0, 0x11, 0, 0x10, 0, 0x17, 0),
> + BANK_DS("T", GPIOT_0, GPIOT_22, 32, 54,
> + 0x23, 0, 0x24, 0, 0x22, 0, 0x21, 0, 0x20, 0, 0x27, 0),
> +};
> +
> +static struct meson_bank a4_aobus_banks[] = {
> + BANK_DS("AO", GPIOAO_0, GPIOAO_6, 0, 6,
> + 0x3, 0, 0x4, 0, 0x2, 0, 0x1, 0, 0x0, 0, 0x7, 0),
> + BANK_DS("TEST_N", GPIO_TEST_N, GPIO_TEST_N, 7, 7,
> + 0x13, 0, 0x14, 0, 0x12, 0, 0x11, 0, 0x10, 0, 0x17, 0),
> +};
> +
> +static struct meson_pmx_bank a4_periphs_pmx_banks[] = {
I think that both struct meson_pmx_bank could be const.
> + /* name first lask reg offset */
> + BANK_PMX("E", GPIOE_0, GPIOE_1, 0x12, 0),
> + BANK_PMX("D", GPIOD_0, GPIOD_15, 0x10, 0),
> + BANK_PMX("B", GPIOB_0, GPIOB_13, 0x00, 0),
> + BANK_PMX("X", GPIOX_0, GPIOX_17, 0x03, 0),
> + BANK_PMX("T", GPIOT_0, GPIOT_22, 0x0b, 0),
> +};
> +
> +static struct meson_pmx_bank a4_aobus_pmx_banks[] = {
> + BANK_PMX("AO", GPIOAO_0, GPIOAO_6, 0x00, 0),
> + BANK_PMX("TEST_N", GPIO_TEST_N, GPIO_TEST_N, 0x0, 28),
> +};
> +
> +static struct meson_axg_pmx_data a4_periphs_pmx_banks_data = {
I think that both struct meson_axg_pmx_data could be const.
> + .pmx_banks = a4_periphs_pmx_banks,
> + .num_pmx_banks = ARRAY_SIZE(a4_periphs_pmx_banks),
> +};
> +
> +static struct meson_axg_pmx_data a4_aobus_pmx_banks_data = {
> + .pmx_banks = a4_aobus_pmx_banks,
> + .num_pmx_banks = ARRAY_SIZE(a4_aobus_pmx_banks),
> +};
> +
> +static struct meson_pinctrl_data a4_periphs_pinctrl_data = {
I think that both struct meson_pinctrl_data could be const.
> + .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,
> +};
> +
> +static 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,
> +};
> +
> +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,
> + },
> + { },
Usually, there is no extra "," after a terinator item.
> +};
> +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,
> + },
> +};
...
CJ
Powered by blists - more mailing lists