Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230710042812.2007928-1-huqiang.qin@amlogic.com> <20230710042812.2007928-3-huqiang.qin@amlogic.com>
In-Reply-To: <20230710042812.2007928-3-huqiang.qin@amlogic.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Mon, 10 Jul 2023 10:33:46 +0300
Message-ID: <CAHp75VevmTiv-OYVTn3bF_8UT9SKNU2oTrzS-AM1zoWefeJ=3w@mail.gmail.com>
Subject: Re: [PATCH V2 2/2] pinctrl: Add driver support for Amlogic C3 SoCs
To:     Huqiang Qin <huqiang.qin@amlogic.com>
Cc:     linus.walleij@linaro.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        neil.armstrong@linaro.org, khilman@baylibre.com,
        jbrunet@baylibre.com, martin.blumenstingl@googlemail.com,
        brgl@bgdev.pl, andy@kernel.org, linux-gpio@vger.kernel.org,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jul 10, 2023 at 7:28=E2=80=AFAM Huqiang Qin <huqiang.qin@amlogic.co=
m> 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[] =3D {
> +       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.

> +};

--=20
With Best Regards,
Andy Shevchenko