netdev - Re: [PATCH] dt-bindings: arm: Convert Marvell CP110 System Controller to DT schema

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] [day] [month] [year] [list]

Message-ID: <87ldl3rpp7.fsf@bootlin.com>
Date: Wed, 22 Oct 2025 16:11:48 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: "Rob Herring (Arm)" <robh@...nel.org>
Cc: Andrew Lunn <andrew@...n.ch>,  Gregory Clement
 <gregory.clement@...tlin.com>,  Sebastian Hesselbarth
 <sebastian.hesselbarth@...il.com>,  Krzysztof Kozlowski
 <krzk+dt@...nel.org>,  Conor Dooley <conor+dt@...nel.org>,  Michael
 Turquette <mturquette@...libre.com>,  Stephen Boyd <sboyd@...nel.org>,
  Linus Walleij <linus.walleij@...aro.org>,  Richard Cochran
 <richardcochran@...il.com>,  linux-arm-kernel@...ts.infradead.org,
  devicetree@...r.kernel.org,  linux-kernel@...r.kernel.org,
  linux-clk@...r.kernel.org,  linux-gpio@...r.kernel.org,
  netdev@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: arm: Convert Marvell CP110 System
 Controller to DT schema

Hi Rob,

Thanks for the conversion!

On 14/10/2025 at 10:30:19 -05, "Rob Herring (Arm)" <robh@...nel.org> wrote:

> Convert the Marvell CP110 System Controller binding to DT schema
> format.
>
> There's not any specific compatible for the whole block which is a
> separate problem, so just the child nodes are documented. Only the
> pinctrl and clock child nodes need to be converted as the GPIO node
> already has a schema.
>
> Signed-off-by: Rob Herring (Arm) <robh@...nel.org>
> ---

...

> -Those clocks can be referenced by other Device Tree nodes using two
> -cells:
> - - The first cell must be 0 or 1. 0 for the core clocks and 1 for the
> -   gateable clocks.
> - - The second cell identifies the particular core clock or gateable
> -   clocks.
> -
> -The following clocks are available:
> - - Core clocks
> -   - 0 0	APLL
> -   - 0 1	PPv2 core
> -   - 0 2	EIP
> -   - 0 3	Core
> -   - 0 4	NAND core
> -   - 0 5	SDIO core
> - - Gateable clocks
> -   - 1 0	Audio
> -   - 1 1	Comm Unit
> -   - 1 2	NAND
> -   - 1 3	PPv2
> -   - 1 4	SDIO
...

Why do you want to drop this information? Telling

    #clock-cells = <2>

is not enough IMO, we must tell people what is expected in these
cells. At the very least the cell values can be constrained to [0-1] for the first
one and [0-5] or [0-26] respectively for the second one.

But giving their meaning I think makes sense. I agree, these should have been
defined inside a shared header, that would have been a better way to
keep track of their meaning, but if we don't have that, I would propose
to keep the information here?

[...]

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-7k-pinctrl.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/marvell,armada-7k-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Armada 7K/8K pin controller

...

> +        enum: [
> +          au, dev, ge, ge0, ge1, gpio, i2c0, i2c1, io, led, link,
> mii,

I don't think "io" has ever been a valid value, it probably comes from a
typo while sorting out all the possibilities ;-) (probably a left over
of a gpio, mdio or sdio string).

> +          mss_gpio0, mss_gpio1, mss_gpio2, mss_gpio3, mss_gpio4, mss_gpio5,
> +          mss_gpio6, mss_gpio7, mss_i2c, mss_spi, mss_uart, nf, pcie, pcie0,
> +          pcie1, pcie2, ptp, rei, sata0, sata1, sdio, sdio_cd, sdio_wp, sei,
> +          spi0, spi1, synce1, synce2, tdm, uart0, uart1, uart2, uart3, wakeup,
> +          xg

Rest LGTM otherwise.

Thanks,
Miquèl