| 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: <42752f1c-fa79-450c-bbf8-f55464db14ae@amlogic.com>
Date: Fri, 14 Jun 2024 16:51:01 +0800
From: Xianwei Zhao <xianwei.zhao@...ogic.com>
To: Rob Herring <robh@...nel.org>
Cc: Linus Walleij <linus.walleij@...aro.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>, 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 1/3] dt-bindings: pinctrl: Add support for Amlogic A4 SoCs
Hi Rob,
Thanks for your review.
On 2024/6/14 01:08, Rob Herring wrote:
> [ EXTERNAL EMAIL ]
>
> On Tue, Jun 11, 2024 at 01:10:57PM +0800, Xianwei Zhao wrote:
>> Add the new compatible name for Amlogic A4 pin controller, and add
>> a new dt-binding header file which document the detail pin names.
>>
>> Signed-off-by: Xianwei Zhao <xianwei.zhao@...ogic.com>
>> ---
>> .../bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml | 2 +
>> .../dt-bindings/gpio/amlogic,a4-aobus-pinctrl.h | 21 +++++
>> .../dt-bindings/gpio/amlogic,a4-periphs-pinctrl.h | 93 ++++++++++++++++++++++
>> 3 files changed, 116 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml
>> index d9e0b2c48e84..f5eefa0fab5b 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/amlogic,meson-pinctrl-a1.yaml
>> @@ -15,6 +15,8 @@ allOf:
>> properties:
>> compatible:
>> enum:
>> + - amlogic,a4-aobus-pinctrl
>> + - amlogic,a4-periphs-pinctrl
>> - amlogic,c3-periphs-pinctrl
>> - amlogic,t7-periphs-pinctrl
>> - amlogic,meson-a1-periphs-pinctrl
>> diff --git a/include/dt-bindings/gpio/amlogic,a4-aobus-pinctrl.h b/include/dt-bindings/gpio/amlogic,a4-aobus-pinctrl.h
>> new file mode 100644
>> index 000000000000..7c7e746baed5
>> --- /dev/null
>> +++ b/include/dt-bindings/gpio/amlogic,a4-aobus-pinctrl.h
>> @@ -0,0 +1,21 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
>> +/*
>> + * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
>> + * Author: Xianwei Zhao <xianwei.zhao@...ogic.com>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_AMLOGIC_A4_AOBUS_PINCTRL_H
>> +#define _DT_BINDINGS_AMLOGIC_A4_AOBUS_PINCTRL_H
>> +
>> +/* GPIOAO */
>> +#define GPIOAO_0 0
>> +#define GPIOAO_1 1
>> +#define GPIOAO_2 2
>> +#define GPIOAO_3 3
>> +#define GPIOAO_4 4
>> +#define GPIOAO_5 5
>> +#define GPIOAO_6 6
>
> I find defines with the value of the define in the name pretty
> pointless.
>
In the driver, this macro definition not only uses its value, but also
uses this character, for example as following,
MESON_PIN(GPIOE_0),
#define MESON_PIN(x) PINCTRL_PIN(x, #x)
GPIO_GROUP(GPIOE_0),
#define GPIO_GROUP(gpio) \
{ \
.name = #gpio, \
.pins = (const unsigned int[]){ gpio }, \
.num_pins = 1, \
.data = (const struct meson_pmx_axg_data[]){ \
PMX_DATA(0), \
}, \
}
>> +
>> +#define GPIO_TEST_N 7
>> +
>> +#endif
>> diff --git a/include/dt-bindings/gpio/amlogic,a4-periphs-pinctrl.h b/include/dt-bindings/gpio/amlogic,a4-periphs-pinctrl.h
>> new file mode 100644
>> index 000000000000..dfabca4b4790
>> --- /dev/null
>> +++ b/include/dt-bindings/gpio/amlogic,a4-periphs-pinctrl.h
>> @@ -0,0 +1,93 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
>> +/*
>> + * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
>> + * Author: Xianwei Zhao <xianwei.zhao@...ogic.com>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_AMLOGIC_A4_PERIPHS_PINCTRL_H
>> +#define _DT_BINDINGS_AMLOGIC_A4_PERIPHS_PINCTRL_H
>> +
>> +/* GPIOE */
>> +#define GPIOE_0 0
>> +#define GPIOE_1 1
>> +
>> +/* GPIOD */
>> +#define GPIOD_0 2
>> +#define GPIOD_1 3
>> +#define GPIOD_2 4
>> +#define GPIOD_3 5
>> +#define GPIOD_4 6
>> +#define GPIOD_5 7
>> +#define GPIOD_6 8
>> +#define GPIOD_7 9
>> +#define GPIOD_8 10
>> +#define GPIOD_9 11
>> +#define GPIOD_10 12
>> +#define GPIOD_11 13
>> +#define GPIOD_12 14
>> +#define GPIOD_13 15
>> +#define GPIOD_14 16
>> +#define GPIOD_15 17
>
> I'm not really much of a fan of using defines for GPIOs, but if you do,
> wouldn't be better to split banks and lines up rather than a global
> number space. See ASPEED_GPIO() or tegra header.
> For the same reasons described above.
I want to keep the same style as the previous drive.
> Rob
Powered by blists - more mailing lists