Hello Robert! On Mon, 2023-11-27 at 14:18 -0600, Rob Herring wrote: > On Wed, Nov 22, 2023 at 11:59:44AM +0300, Nikita Shubin wrote: > > Add device tree bindings for the Cirrus Logic EP93xx SoC. > > > > Signed-off-by: Nikita Shubin <nikita.shubin@xxxxxxxxxxx> > > --- > > .../bindings/arm/cirrus/cirrus,ep9301.yaml | 38 ++++++++++ > > > .../bindings/soc/cirrus/cirrus,ep9301-syscon.yaml | 83 > > ++++++++++++++++++++++ > > include/dt-bindings/soc/cirrus,ep9301-syscon.h | 46 > > ++++++++++++ > > This looks like it is mostly clocks. I'd put it in bindings/clock/ > and > include/dt-bindings/clock/ instead. > > > 3 files changed, 167 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/arm/cirrus/cirrus,ep9301.yaml > > b/Documentation/devicetree/bindings/arm/cirrus/cirrus,ep9301.yaml > > new file mode 100644 > > index 000000000000..97dd8b6aefa9 > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/arm/cirrus/cirrus,ep9301.yaml > > @@ -0,0 +1,38 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/arm/cirrus/cirrus,ep9301.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Cirrus Logic EP93xx platforms > > + > > +description: > > + The EP93xx SoC is a ARMv4T-based with 200 MHz ARM9 CPU. > > + > > +maintainers: > > + - Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx> > > + - Nikita Shubin <nikita.shubin@xxxxxxxxxxx> > > + > > +properties: > > + $nodename: > > + const: '/' > > + compatible: > > + oneOf: > > + - description: The TS-7250 is a compact, full-featured > > Single Board Computer (SBC) > > + based upon the Cirrus EP9302 ARM9 CPU > > Wrap lines at 80 unless there's some advantage to going to 100. Here > there is not. > > > + items: > > + - const: technologic,ts7250 > > + - const: cirrus,ep9301 > > + > > + - description: The Liebherr BK3 is a derivate from ts7250 > > board > > + items: > > + - const: liebherr,bk3 > > + - const: cirrus,ep9301 > > + > > + - description: EDB302 is an evaluation board by Cirrus > > Logic, > > + based on a Cirrus Logic EP9302 CPU > > + items: > > + - const: cirrus,edb9302 > > + - const: cirrus,ep9301 > > + > > +additionalProperties: true > > diff --git > > a/Documentation/devicetree/bindings/soc/cirrus/cirrus,ep9301- > > syscon.yaml > > b/Documentation/devicetree/bindings/soc/cirrus/cirrus,ep9301- > > syscon.yaml > > new file mode 100644 > > index 000000000000..283cf2386d95 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/soc/cirrus/cirrus,ep9301- > > syscon.yaml > > @@ -0,0 +1,83 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: > > http://devicetree.org/schemas/soc/cirrus/cirrus,ep9301-syscon.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Cirrus Logic EP93xx Platforms System Controller > > + > > +maintainers: > > + - Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx> > > + - Nikita Shubin <nikita.shubin@xxxxxxxxxxx> > > + > > +properties: > > + compatible: > > + oneOf: > > + - items: > > + - enum: > > + - cirrus,ep9302-syscon > > + - cirrus,ep9307-syscon > > + - cirrus,ep9312-syscon > > + - cirrus,ep9315-syscon > > The registers of these are all the same as (or a superset of) the > EP9301? That's what you are claiming. Perhaps already discussed, but > there's no blurb here to explain the relationship. > > 'syscon' is a Linux term. Is this block really called this? Citing documentation: "The System Controller (Syscon) provides: • Clock control • Power management • System configuration management These central resources are controlled by a set of software-locked registers, which can be used to prevent accidental accesses. Syscon generates the various bus and peripheral clocks and controls the system startup configuration." I'll put the following into description: "Central resources are controlled by a set of software-locked registers, which can be used to prevent accidental accesses. Syscon generates the various bus and peripheral clocks and controls the system startup configuration. The System Controller (Syscon) provides: - Clock control - Power management - System configuration management Syscon registers are common for all EP93xx SoC's, through some actual peripheral may be missing depending on actual SoC model." > > > + - const: cirrus,ep9301-syscon > > + - const: syscon > > + - const: simple-mfd > > + - items: > > + - const: cirrus,ep9301-syscon > > + - const: syscon > > + - const: simple-mfd > > Doesn't look like an simple-mfd to me. You don't have independent > child > blocks which don't depend on the parent node. > > > + > > + reg: > > + maxItems: 1 > > + > > + "#clock-cells": > > + const: 1 > > + > > + clocks: > > + items: > > + - description: reference clock > > + > > +patternProperties: > > + '^pins-': > > + type: object > > + description: pin node > > + $ref: /schemas/pinctrl/pinmux-node.yaml > > + > > + properties: > > + function: > > + enum: [ spi, ac97, i2s, pwm, keypad, pata, lcd, gpio ] > > + > > + groups: > > + enum: [ ssp, ac97, i2s_on_ssp, i2s_on_ac97, pwm1, > > gpio1agrp, > > + gpio2agrp, gpio3agrp, gpio4agrp, gpio6agrp, > > gpio7agrp, > > + rasteronsdram0grp, rasteronsdram3grp, keypadgrp, > > idegrp ] > > + > > + required: > > + - function > > + - groups > > + > > + unevaluatedProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - "#clock-cells" > > + - clocks > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + syscon@80930000 { > > + compatible = "cirrus,ep9301-syscon", > > + "syscon", "simple-mfd"; > > + reg = <0x80930000 0x1000>; > > + > > + #clock-cells = <1>; > > + clocks = <&xtali>; > > + > > + spi_default_pins: pins-spi { > > + function = "spi"; > > + groups = "ssp"; > > + }; > > + }; > > diff --git a/include/dt-bindings/soc/cirrus,ep9301-syscon.h > > b/include/dt-bindings/soc/cirrus,ep9301-syscon.h > > new file mode 100644 > > index 000000000000..6bb8f532e7d0 > > --- /dev/null > > +++ b/include/dt-bindings/soc/cirrus,ep9301-syscon.h > > @@ -0,0 +1,46 @@ > > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */ > > +#ifndef DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H > > +#define DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H > > + > > +#define EP93XX_CLK_PLL1 0 > > +#define EP93XX_CLK_PLL2 1 > > + > > +#define EP93XX_CLK_FCLK 2 > > +#define EP93XX_CLK_HCLK 3 > > +#define EP93XX_CLK_PCLK 4 > > + > > +#define EP93XX_CLK_UART 5 > > +#define EP93XX_CLK_SPI 6 > > +#define EP93XX_CLK_PWM 7 > > +#define EP93XX_CLK_USB 8 > > + > > +#define EP93XX_CLK_M2M0 9 > > +#define EP93XX_CLK_M2M1 10 > > + > > +#define EP93XX_CLK_M2P0 11 > > +#define EP93XX_CLK_M2P1 12 > > +#define EP93XX_CLK_M2P2 13 > > +#define EP93XX_CLK_M2P3 14 > > +#define EP93XX_CLK_M2P4 15 > > +#define EP93XX_CLK_M2P5 16 > > +#define EP93XX_CLK_M2P6 17 > > +#define EP93XX_CLK_M2P7 18 > > +#define EP93XX_CLK_M2P8 19 > > +#define EP93XX_CLK_M2P9 20 > > + > > +#define EP93XX_CLK_UART1 21 > > +#define EP93XX_CLK_UART2 22 > > +#define EP93XX_CLK_UART3 23 > > + > > +#define EP93XX_CLK_ADC 24 > > +#define EP93XX_CLK_ADC_EN 25 > > + > > +#define EP93XX_CLK_KEYPAD 26 > > + > > +#define EP93XX_CLK_VIDEO 27 > > + > > +#define EP93XX_CLK_I2S_MCLK 28 > > +#define EP93XX_CLK_I2S_SCLK 29 > > +#define EP93XX_CLK_I2S_LRCLK 30 > > + > > +#endif /* DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H */ > > > > -- > > 2.41.0 > >
