linux-kernel - Re: [PATCH V2 2/2] pinctrl: Add driver support for Amlogic C3 SoCs

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Jul 2023 10:33:46 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Huqiang Qin <huqiang.qin@...ogic.com>
Cc:     linus.walleij@...aro.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        neil.armstrong@...aro.org, khilman@...libre.com,
        jbrunet@...libre.com, martin.blumenstingl@...glemail.com,
        brgl@...ev.pl, andy@...nel.org, 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 V2 2/2] pinctrl: Add driver support for Amlogic C3 SoCs

On Mon, Jul 10, 2023 at 7:28 AM Huqiang Qin <huqiang.qin@...ogic.com> wrote:
>
> Add a new pinctrl driver for Amlogic C3 SoCs which share
> the same register layout as the previous Amloigc S4.

...

> +#include <dt-bindings/gpio/amlogic-c3-gpio.h>

Seems some headers are missing. The rule of thumb is to include
headers the code uses directly.
Moreover, using a "proxy" header is not the best idea.

kernel.h // ARRAY_SIZE()
mod_devicetable.h // of_device_id
module.h
platform_device.h

pinctrl/pinctrl.h

> +#include "pinctrl-meson.h"
> +#include "pinctrl-meson-axg-pmx.h"

With the above, it might be that existing inclusions become unused, so
drop them in such a case.

...

> +static struct meson_pmx_func c3_periphs_functions[] = {
> +       FUNCTION(gpio_periphs),
> +       FUNCTION(uart_a),
> +       FUNCTION(uart_b),
> +       FUNCTION(uart_c),
> +       FUNCTION(uart_d),
> +       FUNCTION(uart_e),
> +       FUNCTION(i2c0),
> +       FUNCTION(i2c1),
> +       FUNCTION(i2c2),
> +       FUNCTION(i2c3),
> +       FUNCTION(i2c_slave),
> +       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(pwm_i),
> +       FUNCTION(pwm_j),
> +       FUNCTION(pwm_k),
> +       FUNCTION(pwm_l),
> +       FUNCTION(pwm_m),
> +       FUNCTION(pwm_n),
> +       FUNCTION(pwm_c_hiz),
> +       FUNCTION(ir_out),
> +       FUNCTION(ir_in),
> +       FUNCTION(jtag_a),
> +       FUNCTION(jtag_b),
> +       FUNCTION(gen_clk),
> +       FUNCTION(clk12_24),
> +       FUNCTION(clk_32k_in),
> +       FUNCTION(emmc),
> +       FUNCTION(nand),
> +       FUNCTION(spif),
> +       FUNCTION(spi_a),
> +       FUNCTION(spi_b),
> +       FUNCTION(sdcard),
> +       FUNCTION(sdio),
> +       FUNCTION(pdm),
> +       FUNCTION(eth),
> +       FUNCTION(mclk_0),
> +       FUNCTION(mclk_1),
> +       FUNCTION(tdm),
> +       FUNCTION(lcd)

+ trailing comma. The rule of thumb is to add a comma when it's not a
terminator entry.

> +};

-- 
With Best Regards,
Andy Shevchenko