Re: [PATCH] dt-bindings: arm: Convert Marvell CP110 System Controller to DT schema
From: Miquel Raynal <miquel.raynal@bootlin.com>
Date: 2025-10-22 14:11:55
Also in:
linux-arm-kernel, linux-clk, linux-devicetree, linux-gpio, lkml
Hi Rob, Thanks for the conversion! On 14/10/2025 at 10:30:19 -05, "Rob Herring (Arm)" [off-list ref] 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@kernel.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?
[...]
quoted hunk
--- /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