Message ID | 20221101215804.16262-2-afd@ti.com |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | TI-Nspire cleanups | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success | |
robh/patch-applied | success | |
robh/dtbs-check | warning | build log |
robh/dt-meta-schema | success |
On Tue, Nov 01, 2022 at 04:57:56PM -0500, Andrew Davis wrote: > The TI Nspire devices contain a set of registers with a seemingly > miscellaneous set of functionality. This area is known simply as the > "misc" region. > > Signed-off-by: Andrew Davis <afd@ti.com> > --- > .../bindings/mfd/ti,nspire-misc.yaml | 55 +++++++++++++++++++ > 1 file changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml > > diff --git a/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml > new file mode 100644 > index 0000000000000..d409eae7537bd > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml > @@ -0,0 +1,55 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/ti,nspire-misc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: TI Nspire MISC hardware block > + > +maintainers: > + - Andrew Davis <afd@ti.com> > + > +description: | > + System controller node represents a register region containing a set > + of miscellaneous registers. The registers are not cohesive enough to > + represent as any specific type of device. The typical use-case is > + for some other node's driver, or platform-specific code, to acquire > + a reference to the syscon node (e.g. by phandle, node path, or > + search using a specific compatible value), interrogate the node (or > + associated OS driver) to determine the location of the registers, > + and access the registers directly. Looks like you copied the generic description? Describe what MISC contains. > + > +properties: > + compatible: > + items: > + - enum: > + - ti,nspire-misc > + - const: syscon > + - const: simple-mfd > + > + reg: > + maxItems: 1 > + > + reboot: > + $ref: "../power/reset/syscon-reboot.yaml" /schemas/power/... And no quotes needed. > + > +required: > + - compatible > + - reg > + - reboot > + > +additionalProperties: false > + > +examples: > + - | > + misc: misc@900a0000 { > + compatible = "ti,nspire-misc", "syscon", "simple-mfd"; > + reg = <0x900a0000 0x1000>; > + > + reboot { > + compatible = "syscon-reboot"; > + offset = <0x08>; > + value = <0x02>; > + }; > + }; > -- > 2.37.3 > >
On 11/2/22 12:35 PM, Rob Herring wrote: > On Tue, Nov 01, 2022 at 04:57:56PM -0500, Andrew Davis wrote: >> The TI Nspire devices contain a set of registers with a seemingly >> miscellaneous set of functionality. This area is known simply as the >> "misc" region. >> >> Signed-off-by: Andrew Davis <afd@ti.com> >> --- >> .../bindings/mfd/ti,nspire-misc.yaml | 55 +++++++++++++++++++ >> 1 file changed, 55 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml >> >> diff --git a/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml >> new file mode 100644 >> index 0000000000000..d409eae7537bd >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml >> @@ -0,0 +1,55 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +# Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/mfd/ti,nspire-misc.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: TI Nspire MISC hardware block >> + >> +maintainers: >> + - Andrew Davis <afd@ti.com> >> + >> +description: | >> + System controller node represents a register region containing a set >> + of miscellaneous registers. The registers are not cohesive enough to >> + represent as any specific type of device. The typical use-case is >> + for some other node's driver, or platform-specific code, to acquire >> + a reference to the syscon node (e.g. by phandle, node path, or >> + search using a specific compatible value), interrogate the node (or >> + associated OS driver) to determine the location of the registers, >> + and access the registers directly. > > Looks like you copied the generic description? Describe what MISC > contains. > I don't know what all MISC contains (or maybe I do, but it is not publicly available so I'm not going to add anything that hasn't already been found by clean-room reverse engineering [0]). This is the point I was trying to make in that thread on v3. The node's content *is* the hardware description. Every time a new register is found it could have just been added to the DT. But now we also have to go back here and add the exact same information to the binding, every time. We don't require that for simple-bus, should simple-mfd be given the same flexibility? [0] https://hackspire.org/index.php?title=Memory-mapped_I/O_ports_on_Classic#900A0000_-_Miscellaneous >> + >> +properties: >> + compatible: >> + items: >> + - enum: >> + - ti,nspire-misc >> + - const: syscon >> + - const: simple-mfd >> + >> + reg: >> + maxItems: 1 >> + >> + reboot: >> + $ref: "../power/reset/syscon-reboot.yaml" > > /schemas/power/... > > And no quotes needed. > Will fix. Andrew >> + >> +required: >> + - compatible >> + - reg >> + - reboot >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + misc: misc@900a0000 { >> + compatible = "ti,nspire-misc", "syscon", "simple-mfd"; >> + reg = <0x900a0000 0x1000>; >> + >> + reboot { >> + compatible = "syscon-reboot"; >> + offset = <0x08>; >> + value = <0x02>; >> + }; >> + }; >> -- >> 2.37.3 >> >>
On Wed, Nov 02, 2022 at 02:05:28PM -0500, Andrew Davis wrote: > On 11/2/22 12:35 PM, Rob Herring wrote: > > On Tue, Nov 01, 2022 at 04:57:56PM -0500, Andrew Davis wrote: > > > The TI Nspire devices contain a set of registers with a seemingly > > > miscellaneous set of functionality. This area is known simply as the > > > "misc" region. > > > > > > Signed-off-by: Andrew Davis <afd@ti.com> > > > --- > > > .../bindings/mfd/ti,nspire-misc.yaml | 55 +++++++++++++++++++ > > > 1 file changed, 55 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml > > > new file mode 100644 > > > index 0000000000000..d409eae7537bd > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml > > > @@ -0,0 +1,55 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +# Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/mfd/ti,nspire-misc.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: TI Nspire MISC hardware block > > > + > > > +maintainers: > > > + - Andrew Davis <afd@ti.com> > > > + > > > +description: | > > > + System controller node represents a register region containing a set > > > + of miscellaneous registers. The registers are not cohesive enough to > > > + represent as any specific type of device. The typical use-case is > > > + for some other node's driver, or platform-specific code, to acquire > > > + a reference to the syscon node (e.g. by phandle, node path, or > > > + search using a specific compatible value), interrogate the node (or > > > + associated OS driver) to determine the location of the registers, > > > + and access the registers directly. > > > > Looks like you copied the generic description? Describe what MISC > > contains. > > > > I don't know what all MISC contains (or maybe I do, but it is not > publicly available so I'm not going to add anything that hasn't > already been found by clean-room reverse engineering [0]). Put whatever you are comfortable with, but not a duplicate generic description. You know it at least it has reboot registers... > This is the point I was trying to make in that thread on v3. The > node's content *is* the hardware description. Every time a new > register is found it could have just been added to the DT. But now > we also have to go back here and add the exact same information > to the binding, every time. We don't require that for simple-bus, > should simple-mfd be given the same flexibility? The thing with any MFD is it makes up a device even if it's 'simple', so it's important to get a full picture of the device. It could evolve to not being 'simple'. For example, if you came along and wanted to make it a clock provider, you'd probably create a clock child node that's just a compatible and '#clock-cells'. The feedback would be just add '#clock-cells' to the 'ti,nspire-misc' node. So yes, I want to see additions. Rob
On 11/2/22 4:26 PM, Rob Herring wrote: > On Wed, Nov 02, 2022 at 02:05:28PM -0500, Andrew Davis wrote: >> On 11/2/22 12:35 PM, Rob Herring wrote: >>> On Tue, Nov 01, 2022 at 04:57:56PM -0500, Andrew Davis wrote: >>>> The TI Nspire devices contain a set of registers with a seemingly >>>> miscellaneous set of functionality. This area is known simply as the >>>> "misc" region. >>>> >>>> Signed-off-by: Andrew Davis <afd@ti.com> >>>> --- >>>> .../bindings/mfd/ti,nspire-misc.yaml | 55 +++++++++++++++++++ >>>> 1 file changed, 55 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml >>>> new file mode 100644 >>>> index 0000000000000..d409eae7537bd >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml >>>> @@ -0,0 +1,55 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +# Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/mfd/ti,nspire-misc.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: TI Nspire MISC hardware block >>>> + >>>> +maintainers: >>>> + - Andrew Davis <afd@ti.com> >>>> + >>>> +description: | >>>> + System controller node represents a register region containing a set >>>> + of miscellaneous registers. The registers are not cohesive enough to >>>> + represent as any specific type of device. The typical use-case is >>>> + for some other node's driver, or platform-specific code, to acquire >>>> + a reference to the syscon node (e.g. by phandle, node path, or >>>> + search using a specific compatible value), interrogate the node (or >>>> + associated OS driver) to determine the location of the registers, >>>> + and access the registers directly. >>> >>> Looks like you copied the generic description? Describe what MISC >>> contains. >>> >> >> I don't know what all MISC contains (or maybe I do, but it is not >> publicly available so I'm not going to add anything that hasn't >> already been found by clean-room reverse engineering [0]). > > Put whatever you are comfortable with, but not a duplicate generic > description. You know it at least it has reboot registers... > >> This is the point I was trying to make in that thread on v3. The >> node's content *is* the hardware description. Every time a new >> register is found it could have just been added to the DT. But now >> we also have to go back here and add the exact same information >> to the binding, every time. We don't require that for simple-bus, >> should simple-mfd be given the same flexibility? > > The thing with any MFD is it makes up a device even if it's 'simple', so > it's important to get a full picture of the device. It could evolve to > not being 'simple'. > It would not evolve as a generic simple-mfd binding would not allow any extra non-node properties. '^.*$': type: object additionalProperties: false Maybe I'm thinking about this group of registers wrong, maybe it is a simple-bus not a simple-mfd. I mean from software what else is a bus but a space of addressable registers.. The issue would be each child node of a simple-bus needs a unit-address and 'reg'. Which we *could* do here if not for all this syscon weirdness :( > For example, if you came along and wanted to make it a clock provider, > you'd probably create a clock child node that's just a compatible and > '#clock-cells'. The feedback would be just add '#clock-cells' to the > 'ti,nspire-misc' node. So yes, I want to see additions. > And then that would still fail the dtbs_check, since the simple-mfd binding would only allow for child nodes, nothing else, that's what makes them "simple". Each child node would need a compatible and therefor a binding. You would still see everything and could prevent malformed nodes just the same. Andrew
diff --git a/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml new file mode 100644 index 0000000000000..d409eae7537bd --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/ +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/ti,nspire-misc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: TI Nspire MISC hardware block + +maintainers: + - Andrew Davis <afd@ti.com> + +description: | + System controller node represents a register region containing a set + of miscellaneous registers. The registers are not cohesive enough to + represent as any specific type of device. The typical use-case is + for some other node's driver, or platform-specific code, to acquire + a reference to the syscon node (e.g. by phandle, node path, or + search using a specific compatible value), interrogate the node (or + associated OS driver) to determine the location of the registers, + and access the registers directly. + +properties: + compatible: + items: + - enum: + - ti,nspire-misc + - const: syscon + - const: simple-mfd + + reg: + maxItems: 1 + + reboot: + $ref: "../power/reset/syscon-reboot.yaml" + +required: + - compatible + - reg + - reboot + +additionalProperties: false + +examples: + - | + misc: misc@900a0000 { + compatible = "ti,nspire-misc", "syscon", "simple-mfd"; + reg = <0x900a0000 0x1000>; + + reboot { + compatible = "syscon-reboot"; + offset = <0x08>; + value = <0x02>; + }; + };
The TI Nspire devices contain a set of registers with a seemingly miscellaneous set of functionality. This area is known simply as the "misc" region. Signed-off-by: Andrew Davis <afd@ti.com> --- .../bindings/mfd/ti,nspire-misc.yaml | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/ti,nspire-misc.yaml