| 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: <20241217142028.GA1441221-robh@kernel.org>
Date: Tue, 17 Dec 2024 08:20:28 -0600
From: Rob Herring <robh@...nel.org>
To: 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>,
Neil Armstrong <neil.armstrong@...aro.org>,
Kevin Hilman <khilman@...libre.com>,
Jerome Brunet <jbrunet@...libre.com>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
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 RFC 1/3] dt-bindings: pinctrl: Add support for Amlogic
SoCs
On Wed, Dec 11, 2024 at 02:47:49PM +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 and
> alternative func value of all Amlogic subsequent SoCs.
>
> Signed-off-by: Xianwei Zhao <xianwei.zhao@...ogic.com>
> ---
> .../bindings/pinctrl/amlogic,pinctrl.yaml | 150 +++++++++++++++++++++
> include/dt-bindings/pinctrl/amlogic,pinctrl.h | 68 ++++++++++
> 2 files changed, 218 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl.yaml
> new file mode 100644
> index 000000000000..b0c2ae585d7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl.yaml
> @@ -0,0 +1,150 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/amlogic,pinctrl.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
Only one Amlogic pinctrl block? Pretty sure there's more than 1 SoC and
pinctrl is unlikely identical for all. This must be SoC specific.
> +
> + reg:
> + minItems: 2
> +
> + reg-names:
> + items:
> + - const: mux
> + - const: gpio
> +
> + "#address-cells":
> + const: 2
> +
> + "#size-cells":
> + const: 2
Why do you need these? You don't have any child nodes with addresses.
But maybe you should?
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - "#address-cells"
> + - "#size-cells"
> +
> +patternProperties:
> + "^gpio[a-z]":
This allows "gpioanythingyouwant" for example. If there is no
unit-address, then it should be '^gpio-[0-9]+$'.
> + type: object
> +
> + properties:
> + gpio-controller: true
> +
> + "#gpio-cells":
> + const: 2
> +
> + bank-name:
> + $ref: /schemas/types.yaml#/definitions/string
You must define values, but why do you need this? No other GPIO
controller needs something like this.
> +
> + npins:
We have a standard property for this: ngpios
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1
> + maximum: 32
> +
> + bank-index:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 29
Consider if 'bank' should be part of the gpio cells rather than
separate controller nodes. That works better if each bank is not its own
separate h/w block. If banks share a register, then that's not separate
blocks.
> +
> + reg-mux-offset:
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + bit-mux-offset:
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> + reg-gpio-offset:
> + $ref: /schemas/types.yaml#/definitions/uint32
Put these into 'reg'. We even have a way to deal with bit offsets. See
leds/register-bit-led.yaml for example.
> +
> + required:
> + - gpio-controller
> + - "#gpio-cells"
> + - npins
> + - bank-index
> + - reg-mux-offset
> + - reg-gpio-offset
> + - bank-name
> +
> + additionalProperties: false
> + unevaluatedProperties: false
Don't need both. 'additionalProperties' is more restrictive, so drop
unevaluatedProperties.
> +
> + "^func-[0-9a-z]":
"^func-[0-9a-z-]+$"
> + type: object
> + additionalProperties:
> + type: object
> + allOf:
> + - $ref: pincfg-node.yaml#
> + - $ref: pinmux-node.yaml#
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + apb {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + periphs_pinctrl: pinctrl@...00 {
> + compatible = "amlogic,pinctrl";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + reg = <0x0 0x8e700 0x0 0x04>,
> + <0x0 0x8e704 0x0 0x60>;
> + reg-names = "mux", "gpio";
> +
> + gpiob {
> + gpio-controller;
> + #gpio-cells = <2>;
> + npins = <10>;
> + bank-index = <1>;
> + reg-mux-offset = <0x14>;
> + bit-mux-offset = <0x14>;
> + reg-gpio-offset = <0x30>;
> + bank-name = "GPIOB";
> + };
> +
> + gpioe {
> + gpio-controller;
> + #gpio-cells = <2>;
> + npins = <10>;
> + bank-index = <5>;
> + reg-mux-offset = <0x14>;
> + reg-gpio-offset = <0x30>;
> + bank-name = "GPIOE";
> + };
> +
> + func-uart-b {
> + uart-b-default{
> + pinmux = <3>;
> + bias-pull-up;
> + drive-strength-microamp = <4000>;
> + };
> +
> + uart-c-default{
> + pinmux = <4>;
> + bias-pull-up;
> + drive-strength-microamp = <4000>;
> + };
> + };
> +
> + func-uart-c {
> + uart-c-default{
> + pinmux = <3>;
> + 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..03db0a730e8b
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
> @@ -0,0 +1,68 @@
> +/* 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
> +
> +/* define PIN modes */
> +#define AF0 0x0
> +#define AF1 0x1
> +#define AF2 0x2
> +#define AF3 0x3
> +#define AF4 0x4
> +#define AF5 0x5
> +#define AF6 0x6
> +#define AF7 0x7
> +#define AF8 0x8
> +#define AF9 0x9
> +#define AF10 0xa
> +#define AF11 0xb
> +#define AF12 0xc
> +#define AF13 0xd
> +#define AF14 0xe
> +#define AF15 0xf
> +
> +#define AML_PIN_ALT_FUNC_MASK 0xf
> +
> +/* 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 AML_PINMUX(bank, offset, mode) (((((bank) << 8) + (offset)) << 8) | (mode))
> +#define AML_PINMUX_TO_BANK(pinmux) (((pinmux) >> 16) & 0xff)
> +#define AML_PINMUX_TO_OFFSET(pinmux) (((pinmux) >> 8) & 0xff)
> +
> +#endif /* _DT_BINDINGS_AMLOGIC_PINCTRL_H */
>
> --
> 2.37.1
>
Powered by blists - more mailing lists