| 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: <0f0c0d16-f736-419e-9ffc-c3dc507b815c@linaro.org>
Date: Tue, 19 Dec 2023 08:34:51 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Théo Lebrun <theo.lebrun@...tlin.com>,
Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>,
Gregory CLEMENT <gregory.clement@...tlin.com>,
Linus Walleij <linus.walleij@...aro.org>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>
Cc: linux-mips@...r.kernel.org, linux-gpio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Tawfik Bayouk <tawfik.bayouk@...ileye.com>
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: mobileye,eyeq5-pinctrl: add
bindings
On 18/12/2023 18:19, Théo Lebrun wrote:
> Add dt-schema type bindings for the Mobileye EyeQ5 pin controller.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@...tlin.com>
> ---
> .../bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml | 125 +++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 126 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
> new file mode 100644
> index 000000000000..5faddebe2413
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mobileye EyeQ5 pinctrl (pinmux & pinconf) controller
pinctrl means pin controller, so you basically wrote:
pin controller pinmux and pin configuration controller
Just "pin controller"
> +
> +description:
> + The EyeQ5 pin controller handles a pin bank. It is custom to this platform,
Can part of SoC be not custom to given platform? I mean... describe the
hardware, not write essay.
> + its registers live in a shared region called OLB.
> + There are two pin banks on the platform, each having a specific compatible.
Instead of repeating something obvious - visible from the binding -
explain why. Say something different than the binding is saying.
> + Pins and groups are bijective.
> +
> +maintainers:
> + - Grégory Clement <gregory.clement@...tlin.com>
> + - Théo Lebrun <theo.lebrun@...tlin.com>
> + - Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>
> +
> +properties:
> + $nodename:
> + pattern: "^pinctrl([0-9]+)?$"
> + description:
> + We have no unique address, we rely on OLB; we therefore can't keep the
> + standard pattern and cannot inherit from pinctrl.yaml.
No, instead fix pinctrl.yaml
> +
> + compatible:
> + enum:
> + - mobileye,eyeq5-a-pinctrl
> + - mobileye,eyeq5-b-pinctrl
Why two compatibles? Description provided no rationale for this.
> +
> + "#pinctrl-cells":
> + const: 1
> +
> + mobileye,olb:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + A phandle to the OLB syscon. This is a fallback to using the parent as
> + syscon node.
So here is the explanation for missing unit address. If all registers,
as you claim in description, belong to OLB, then this should be part of
OLB. Drop the phandle.
> +
> +required:
> + - compatible
> + - "#pinctrl-cells"
So now please test your code without olb phandle...
> +
> +patternProperties:
patternProperties go after properties
> + "-pins?$":
> + type: object
> + description: Pin muxing configuration.
> + $ref: pinmux-node.yaml#
> + additionalProperties: false
Why not unevaluatedProperties?
> + properties:
> + pins: true
> + function: true
> + bias-disable: true
> + bias-pull-down: true
> + bias-pull-up: true
> + drive-strength: true
> + required:
> + - pins
> + - function
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mobileye,eyeq5-a-pinctrl
> + then:
> + patternProperties:
> + "-pins?$":
> + properties:
> + function:
> + enum: [gpio, timer0, timer1, timer2, timer5, uart0, uart1, can0,
> + can1, spi0, spi1, refclk0]
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mobileye,eyeq5-b-pinctrl
> + then:
> + patternProperties:
> + "-pins?$":
> + properties:
> + function:
> + enum: [gpio, timer3, timer4, timer6, uart2, can2, spi2, spi3,
> + mclk0]
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + olb@...000 {
> + compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
Drop, not erlated.
> + reg = <0xe00000 0x400>;
> + reg-io-width = <4>;
> +
> + pinctrl0 {
Suffixes are always after -
> + compatible = "mobileye,eyeq5-a-pinctrl";
> + #pinctrl-cells = <1>;
Where is the phandle?
> + };
> +
> + pinctrl1 {
> + compatible = "mobileye,eyeq5-b-pinctrl";
> + #pinctrl-cells = <1>;
> + };
> + };
> + - |
> + olb: olb@...000 {
> + compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
> + reg = <0xe00000 0x400>;
> + reg-io-width = <4>;
> + };
> +
> + pinctrl0 {
> + compatible = "mobileye,eyeq5-a-pinctrl";
> + #pinctrl-cells = <1>;
> + mobileye,olb = <&olb>;
Really, why? This is just confusing. There is no explanation for
supporting both. Hardware is either this or that, not both!
Best regards,
Krzysztof
Powered by blists - more mailing lists