linux-kernel - Re: [PATCH v6 4/5] pinctrl: meson: Add driver support for Amlogic A4 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]

Message-ID: <1764b1b4-336d-4ca5-ab21-8213691a9622@linaro.org>
Date: Thu, 14 Nov 2024 10:21:42 +0100
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Rob Herring <robh@...nel.org>, Xianwei Zhao <xianwei.zhao@...ogic.com>
Cc: Linus Walleij <linus.walleij@...aro.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.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 v6 4/5] pinctrl: meson: Add driver support for Amlogic A4
 SoCs

On 13/11/2024 19:04, Rob Herring wrote:
> On Wed, Nov 13, 2024 at 03:29:42PM +0800, Xianwei Zhao wrote:
>> 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 | 1324 ++++++++++++++++++++++++++++
>>   3 files changed, 1331 insertions(+)
>>
>> diff --git a/drivers/pinctrl/meson/Kconfig b/drivers/pinctrl/meson/Kconfig
>> index cc397896762c..3e90bb5ec442 100644
>> --- a/drivers/pinctrl/meson/Kconfig
>> +++ b/drivers/pinctrl/meson/Kconfig
>> @@ -67,6 +67,12 @@ config PINCTRL_MESON_S4
>>   	select PINCTRL_MESON_AXG_PMX
>>   	default y
>>   
>> +config PINCTRL_AMLOGIC_A4
>> +	tristate "Amlogic A4 SoC pinctrl driver"
>> +	depends on ARM64
>> +	select PINCTRL_MESON_AXG_PMX
>> +	default y
>> +
>>   config PINCTRL_AMLOGIC_C3
>>   	tristate "Amlogic C3 SoC pinctrl driver"
>>   	depends on ARM64
>> diff --git a/drivers/pinctrl/meson/Makefile b/drivers/pinctrl/meson/Makefile
>> index 9e538b9ffb9b..c92a65a83344 100644
>> --- a/drivers/pinctrl/meson/Makefile
>> +++ b/drivers/pinctrl/meson/Makefile
>> @@ -10,5 +10,6 @@ obj-$(CONFIG_PINCTRL_MESON_AXG) += pinctrl-meson-axg.o
>>   obj-$(CONFIG_PINCTRL_MESON_G12A) += pinctrl-meson-g12a.o
>>   obj-$(CONFIG_PINCTRL_MESON_A1) += pinctrl-meson-a1.o
>>   obj-$(CONFIG_PINCTRL_MESON_S4) += pinctrl-meson-s4.o
>> +obj-$(CONFIG_PINCTRL_AMLOGIC_A4) += pinctrl-amlogic-a4.o
>>   obj-$(CONFIG_PINCTRL_AMLOGIC_C3) += pinctrl-amlogic-c3.o
>>   obj-$(CONFIG_PINCTRL_AMLOGIC_T7) += pinctrl-amlogic-t7.o
>> diff --git a/drivers/pinctrl/meson/pinctrl-amlogic-a4.c b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
>> new file mode 100644
>> index 000000000000..edc5f2ba2c8a
>> --- /dev/null
>> +++ b/drivers/pinctrl/meson/pinctrl-amlogic-a4.c
>> @@ -0,0 +1,1324 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>> +/*
>> + * Pin controller and GPIO driver for Amlogic A4 SoC.
>> + *
>> + * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
>> + * Author: Xianwei Zhao <xianwei.zhao@...ogic.com>
>> + *         Huqiang Qin <huqiang.qin@...ogic.com>
>> + */
>> +
>> +#include "pinctrl-meson.h"
>> +#include "pinctrl-meson-axg-pmx.h"
>> +#include <dt-bindings/gpio/amlogic-gpio.h>
>> +
>> +/* Standard port */
>> +
>> +#define GPIOE_0				0
>> +#define GPIOE_1				1
>> +
>> +#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
> 
> The conversion from bank+index to a single index space seems less than
> ideal, and looks like a work-around to fit into the existing driver from
> a brief look at it.

Not really, it simply adds a custom xlate per SoC, nothing particulary hacky.

I was relunctant at first, but since Xianwei added the plumbing for a per-SoC
xlate, then it was easy to add 3-cells support.

> 
> If there's not really banks of GPIOs here, then DT shouldn't have them
> either. The question is does anything need to know the bank number
> and/or index? If it's only for human readability (and matching to
> datasheet), then just something like this can be done:
> 
> #define GPIOD(n) (2 + (n))

There's no linear mapping possible, each set of gpios is grouped into logical
"banks" per group of functions, and this grouping is also in the gpio controller
register space.

So it makes sense to split this in 2 with banks and offset, it maps the architecture
of the SoC, and with this scheme we only need to add the bindings for the
"banks" once since all the SoCs shares the same names.

It simplifies bindings, has minimal cost due to the 3 cells, and only requires slightly
more larger xlate function per SoC family.

Neil

> 
> Rob