| 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: <b12d31b3-9d0d-480c-9adb-baa3aa7aa437@amlogic.com>
Date: Fri, 7 Feb 2025 14:59:15 +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-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-amlogic@...ts.infradead.org
Subject: Re: [PATCH v4 1/5] dt-bindings: pinctrl: Add support for Amlogic SoCs
Hi Rob,
Thanks for your reply.
On 2025/1/28 02:59, Rob Herring wrote:
> [ EXTERNAL EMAIL ]
>
> On Wed, Jan 22, 2025 at 11:25:59AM +0800, Xianwei Zhao wrote:
>> Add the dt-bindings for Amlogic pin controller, and add a new
>> dt-binding header file which document the GPIO bank names of
>> all Amlogic subsequent SoCs.
>
> Did you mean 'all subsequent Amlogic SoCs'? How can you know that?
> Future SoCs may be different.
> > Update the subject to say this is for A4 SoC.
>
Will do.
>>
>> Signed-off-by: Xianwei Zhao <xianwei.zhao@...ogic.com>
>> ---
>> .../bindings/pinctrl/amlogic,pinctrl-a4.yaml | 127 +++++++++++++++++++++
>> include/dt-bindings/pinctrl/amlogic,pinctrl.h | 46 ++++++++
>> 2 files changed, 173 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
>> new file mode 100644
>> index 000000000000..c4fdcd69bbf5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
>> @@ -0,0 +1,127 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pinctrl/amlogic,pinctrl-a4.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Amlogic pinmux controller
>> +
>> +maintainers:
>> + - Xianwei Zhao <xianwei.zhao@...ogic.com>
>> +
>> +allOf:
>> + - $ref: pinctrl.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: amlogic,pinctrl-a4
>> +
>> + "#address-cells":
>> + const: 2
>> +
>> + "#size-cells":
>> + const: 2
>
> Do you really need 2^64 address space to define the GPIO nodes?
>
I discussed it with Krzysztof earlier in the V3.
32-bit is enough, but since APB bus is defined as 64-bit, here is following.
https://lore.kernel.org/all/d08136ac-e08f-4e5d-b562-78529f34d86f@kernel.org/
>> +
>> + ranges: true
>> +
>> +patternProperties:
>> + "^gpio@[0-9a-f]+$":
>> + type: object
>> +
>> + properties:
>> + reg:
>> + minItems: 1
>> + items:
>> + - description: pin config register
>> + - description: pin mux setting register (some special pin fixed function)
>> + - description: pin drive strength register (optional)
>> +
>> + reg-names:
>> + minItems: 1
>> + items:
>> + - const: gpio
>> + - const: mux
>> + - const: ds
>> +
>> + gpio-controller: true
>> +
>> + "#gpio-cells":
>> + const: 2
>> +
>> + gpio-ranges:
>> + maxItems: 1
>> +
>> + required:
>> + - reg
>> + - reg-names
>> + - gpio-controller
>> + - "#gpio-cells"
>> + - gpio-ranges
>> +
>> + additionalProperties: false
>
> Move this above 'properties'
>
Will do.
>> +
>> + "^func-[0-9a-z-]+$":
>> + type: object
>> + patternProperties:
>> + "^group-[0-9a-z-]+$":
>> + type: object
>> + allOf:
>> + - $ref: /schemas/pinctrl/pincfg-node.yaml
>> + - $ref: /schemas/pinctrl/pinmux-node.yaml
>> +
>> + required:
>> + - pinmux
>> + additionalProperties: false
>
> Move this above 'patternProperties'
>
Will do.
>> +
>> +required:
>> + - compatible
>> + - "#address-cells"
>> + - "#size-cells"
>> + - ranges
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/pinctrl/amlogic,pinctrl.h>
>> + apb {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + periphs_pinctrl: pinctrl {
>> + compatible = "amlogic,pinctrl-a4";
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>> +
>> + gpio@14 {
>> + reg = <0 0x14 0 0x10>, <0 0x14 0 0x10>;
>
> Both registers at the same address? I hope that's a typo.
>
Will fix it.
>> + reg-names = "gpio", "mux";
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + gpio-ranges = <&periphs_pinctrl 0 8 10>;
>> + };
>> +
>> + func-uart-b {
>> + group-default {
>> + pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 1, 4)>;
>> + bias-pull-up;
>> + drive-strength-microamp = <4000>;
>> + };
>> +
>> + group-pins1 {
>> + pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 5, 2)>;
>> + bias-pull-up;
>> + drive-strength-microamp = <4000>;
>> + };
>> + };
>> +
>> + func-uart-c {
>> + group-default {
>> + pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 3, 1)>,
>> + <AML_PINMUX(AMLOGIC_GPIO_B, 2, 1)>;
>> + bias-pull-up;
>> + drive-strength-microamp = <4000>;
>> + };
>> + };
>> + };
>> + };
>> diff --git a/include/dt-bindings/pinctrl/amlogic,pinctrl.h b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
>> new file mode 100644
>> index 000000000000..7d40aecc7147
>> --- /dev/null
>> +++ b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
>> @@ -0,0 +1,46 @@
>> +/* 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_PINCTRL_H
>> +#define _DT_BINDINGS_AMLOGIC_PINCTRL_H
>> +/* Normal PIN bank */
>> +#define AMLOGIC_GPIO_A 0
>> +#define AMLOGIC_GPIO_B 1
>> +#define AMLOGIC_GPIO_C 2
>> +#define AMLOGIC_GPIO_D 3
>> +#define AMLOGIC_GPIO_E 4
>> +#define AMLOGIC_GPIO_F 5
>> +#define AMLOGIC_GPIO_G 6
>> +#define AMLOGIC_GPIO_H 7
>> +#define AMLOGIC_GPIO_I 8
>> +#define AMLOGIC_GPIO_J 9
>> +#define AMLOGIC_GPIO_K 10
>> +#define AMLOGIC_GPIO_L 11
>> +#define AMLOGIC_GPIO_M 12
>> +#define AMLOGIC_GPIO_N 13
>> +#define AMLOGIC_GPIO_O 14
>> +#define AMLOGIC_GPIO_P 15
>> +#define AMLOGIC_GPIO_Q 16
>> +#define AMLOGIC_GPIO_R 17
>> +#define AMLOGIC_GPIO_S 18
>> +#define AMLOGIC_GPIO_T 19
>> +#define AMLOGIC_GPIO_U 20
>> +#define AMLOGIC_GPIO_V 21
>> +#define AMLOGIC_GPIO_W 22
>> +#define AMLOGIC_GPIO_X 23
>> +#define AMLOGIC_GPIO_Y 24
>> +#define AMLOGIC_GPIO_Z 25
>> +
>> +/* Special PIN bank */
>> +#define AMLOGIC_GPIO_DV 26
>> +#define AMLOGIC_GPIO_AO 27
>> +#define AMLOGIC_GPIO_CC 28
>> +#define AMLOGIC_GPIO_TEST_N 29
>> +#define AMLOGIC_GPIO_ANALOG 30
>> +
>> +#define AML_PINMUX(bank, offset, mode) (((((bank) << 8) + (offset)) << 8) | (mode))
>> +
>> +#endif /* _DT_BINDINGS_AMLOGIC_PINCTRL_H */
>>
>> --
>> 2.37.1
>>
Powered by blists - more mailing lists