| Message ID | 20211008031953.339461-3-zhang.lyra@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add Unisoc's SC2730 regulator support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success | |
| robh/dtbs-check | success |
On Fri, Oct 08, 2021 at 11:19:53AM +0800, Chunyan Zhang wrote: > +properties: > + compatible: > + const: sprd,sc2730-regulator I still don't understand why this MFD subfunction for a specific device is a separate binding with a separate compatible string, the issues I mentioned previously with this just encoding current Linux internals into the DT rather than describing the device still apply.
Hi Mark, Sorry for the late response. [1] is the v1 on which we had some discussion. I hope that can help recall the issue below. On Fri, 12 Nov 2021 at 21:46, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Oct 08, 2021 at 11:19:53AM +0800, Chunyan Zhang wrote: > > > +properties: > > + compatible: > > + const: sprd,sc2730-regulator > > I still don't understand why this MFD subfunction for a specific device > is a separate binding with a separate compatible string, the issues I > mentioned previously with this just encoding current Linux internals > into the DT rather than describing the device still apply. I understand your point. But like I described previously [1], if we still use the current solution (i.e. use devm_of_platform_populate() to register MFD subdevices), a compatible string is required. I'm open to switching to other solutions, do you have some suggestions? Thanks, Chunyan [1] https://lkml.org/lkml/2021/9/29/1166
On Thu, 22 Sep 2022, Chunyan Zhang wrote: > Hi Mark, > > Sorry for the late response. > [1] is the v1 on which we had some discussion. I hope that can help > recall the issue below. > > On Fri, 12 Nov 2021 at 21:46, Mark Brown <broonie@kernel.org> wrote: > > > > On Fri, Oct 08, 2021 at 11:19:53AM +0800, Chunyan Zhang wrote: > > > > > +properties: > > > + compatible: > > > + const: sprd,sc2730-regulator > > > > I still don't understand why this MFD subfunction for a specific device > > is a separate binding with a separate compatible string, the issues I > > mentioned previously with this just encoding current Linux internals > > into the DT rather than describing the device still apply. > > I understand your point. But like I described previously [1], if we > still use the current solution (i.e. use devm_of_platform_populate() > to register MFD subdevices), a compatible string is required. I'm open > to switching to other solutions, do you have some suggestions? Many IPs encompassing multiple functions are described that way in DT. I don't have the details for *this* device to hand, so my comments here aren't specific to this use-case, but describing each function individually does describe the H/W accurately, which is all DT calls for. Can you imagine describing an SoC, which can be considered as a huge MFD, with only a single node? Does the regulator functionality have it's own bank of registers?
On Thu, Sep 22, 2022 at 11:19:08AM +0100, Lee Jones wrote: > On Thu, 22 Sep 2022, Chunyan Zhang wrote: > > I understand your point. But like I described previously [1], if we > > still use the current solution (i.e. use devm_of_platform_populate() > > to register MFD subdevices), a compatible string is required. I'm open > > to switching to other solutions, do you have some suggestions? > > Many IPs encompassing multiple functions are described that way in > DT. I don't have the details for *this* device to hand, so my > comments here aren't specific to this use-case, but describing each > function individually does describe the H/W accurately, which is all > DT calls for. If people want to describe the individual regulators that'd be less of an issue, it's mainly when you're nesting what's effectively another MFD within a parent MFD that it's just noise that's being added to the DT. > Can you imagine describing an SoC, which can be considered as a huge > MFD, with only a single node? Honestly we should be arranging things so they're more like that, at least using overlays for the internals of the SoC so you don't have to rebuild the whole DT for updates to the SoC internals.
On Thu, 22 Sep 2022, Mark Brown wrote: > On Thu, Sep 22, 2022 at 11:19:08AM +0100, Lee Jones wrote: > > On Thu, 22 Sep 2022, Chunyan Zhang wrote: > > > > I understand your point. But like I described previously [1], if we > > > still use the current solution (i.e. use devm_of_platform_populate() > > > to register MFD subdevices), a compatible string is required. I'm open > > > to switching to other solutions, do you have some suggestions? > > > > Many IPs encompassing multiple functions are described that way in > > DT. I don't have the details for *this* device to hand, so my > > comments here aren't specific to this use-case, but describing each > > function individually does describe the H/W accurately, which is all > > DT calls for. > > If people want to describe the individual regulators that'd be > less of an issue, it's mainly when you're nesting what's > effectively another MFD within a parent MFD that it's just noise > that's being added to the DT. As I say, I haven't studied this use-case. These comments were designed to be more generic. What do you mean by nested MFDs? > > Can you imagine describing an SoC, which can be considered as a huge > > MFD, with only a single node? > > Honestly we should be arranging things so they're more like that, > at least using overlays for the internals of the SoC so you don't > have to rebuild the whole DT for updates to the SoC internals. Right, there would be one device root node. However each function; clock providers, regulator controllers, PWMs, GPIOs, networking (various), reset, watchdog, etc would have their own nodes. Rather than attempting to describe everything in the parent's node.
On Mon, Sep 26, 2022 at 07:59:08AM +0100, Lee Jones wrote: > On Thu, 22 Sep 2022, Mark Brown wrote: > > If people want to describe the individual regulators that'd be > > less of an issue, it's mainly when you're nesting what's > > effectively another MFD within a parent MFD that it's just noise > > that's being added to the DT. > As I say, I haven't studied this use-case. > These comments were designed to be more generic. > What do you mean by nested MFDs? Given that individual regulators tend to be separate physical IPs in the chip if you create a single device tree node that lumps them together you still need to also represent the individual regulators as well so that collection is functioning like a MFD does except not on a chip boundary. > > > Can you imagine describing an SoC, which can be considered as a huge > > > MFD, with only a single node? > > Honestly we should be arranging things so they're more like that, > > at least using overlays for the internals of the SoC so you don't > > have to rebuild the whole DT for updates to the SoC internals. > Right, there would be one device root node. However each function; > clock providers, regulator controllers, PWMs, GPIOs, networking > (various), reset, watchdog, etc would have their own nodes. Rather > than attempting to describe everything in the parent's node. We don't split things up by function, we split them up by IP - we don't just allocate a compatible for all the networking related functionality simply because there's a networking subsystem in Linux for example.
diff --git a/Documentation/devicetree/bindings/regulator/sprd,sc2730-regulator.yaml b/Documentation/devicetree/bindings/regulator/sprd,sc2730-regulator.yaml new file mode 100644 index 000000000000..6d1bec03eb52 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/sprd,sc2730-regulator.yaml @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright 2019-2021 Unisoc Inc. +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/regulator/sprd,sc2730-regulator.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Unisoc SC2730 Voltage Regulators Device Tree Bindings + +maintainers: + - Orson Zhai <orsonzhai@gmail.com> + - Baolin Wang <baolin.wang7@gmail.com> + - Chunyan Zhang <zhang.lyra@gmail.com> + +properties: + compatible: + const: sprd,sc2730-regulator + + reg: + maxItems: 1 + +patternProperties: + "^(DCDC|BUCK|LDO)_.+": + # Child node + type: object + description: dcdc/buck/ldo regulator nodes(s). + $ref: "regulator.yaml#" + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + pmic { + #address-cells = <1>; + #size-cells = <0>; + + regulators@1800 { + compatible = "sprd,sc2730-regulator"; + reg = <0x1800>; + + DCDC_CPU { + regulator-name = "vddcpu"; + regulator-min-microvolt = <200000>; + regulator-max-microvolt = <1596875>; + regulator-ramp-delay = <25000>; + regulator-always-on; + }; + + LDO_VDDWCN { + regulator-name = "vddwcn"; + regulator-min-microvolt = <900000>; + regulator-max-microvolt = <1845000>; + regulator-ramp-delay = <25000>; + }; + }; + }; +...