| Message ID | 20230725063132.42132-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] regulator: dt-bindings: dlg,slg51000: Convert to DT schema | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 140 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Tue, Jul 25, 2023 at 08:31:32AM +0200, Krzysztof Kozlowski wrote:
> 1. Mention that supplies are required, if the regulator is enabled.
This is just adding an OS specific note in the description, it's not
actually marking the properties as required.
On 25/07/2023 12:43, Mark Brown wrote: > On Tue, Jul 25, 2023 at 08:31:32AM +0200, Krzysztof Kozlowski wrote: > >> 1. Mention that supplies are required, if the regulator is enabled. > > This is just adding an OS specific note in the description, it's not > actually marking the properties as required. They cannot be required, because it depends whether the regulator is used or not. IOW, they are not required for unused regulators, which is not possible to encode in the schema. Best regards, Krzysztof
On Wed, Jul 26, 2023 at 09:32:17AM +0200, Krzysztof Kozlowski wrote: > On 25/07/2023 12:43, Mark Brown wrote: > > This is just adding an OS specific note in the description, it's not > > actually marking the properties as required. > They cannot be required, because it depends whether the regulator is > used or not. IOW, they are not required for unused regulators, which is > not possible to encode in the schema. Oh, you mean if the regulator is in use in the system rather than if it's enabled! I suspect that there's a requirement that either at least one of the supplies be provided so that the chip I/O works, or there's some other currently undocumented supply that is required for that reason. BTW there's also a formatting error: + vin3-supply: + description: + Input supply for ldo3, required if regulatoris enabled missing space before is.
On 26/07/2023 13:55, Mark Brown wrote: > On Wed, Jul 26, 2023 at 09:32:17AM +0200, Krzysztof Kozlowski wrote: >> On 25/07/2023 12:43, Mark Brown wrote: > >>> This is just adding an OS specific note in the description, it's not >>> actually marking the properties as required. > >> They cannot be required, because it depends whether the regulator is >> used or not. IOW, they are not required for unused regulators, which is >> not possible to encode in the schema. > > Oh, you mean if the regulator is in use in the system rather than if > it's enabled! Enabled as "always-on" or "boot-on" could be encoded in the schema with multiple if::then:. But it is not enough, because regulators can be enabled on demand by drivers. So that's what I meant by "used". > I suspect that there's a requirement that either at least > one of the supplies be provided so that the chip I/O works, or there's > some other currently undocumented supply that is required for that > reason. I can add requirement of at least one supply. I don't think it changes much, but sure. > > BTW there's also a formatting error: > > + vin3-supply: > + description: > + Input supply for ldo3, required if regulatoris enabled > > missing space before is. I'll fix with above at least one supply required. Best regards, Krzysztof
On Wed, Jul 26, 2023 at 02:18:43PM +0200, Krzysztof Kozlowski wrote: > On 26/07/2023 13:55, Mark Brown wrote: > > Oh, you mean if the regulator is in use in the system rather than if > > it's enabled! > Enabled as "always-on" or "boot-on" could be encoded in the schema with > multiple if::then:. But it is not enough, because regulators can be > enabled on demand by drivers. So that's what I meant by "used". Right, you said "enabled" in the changelog bit though. > > I suspect that there's a requirement that either at least > > one of the supplies be provided so that the chip I/O works, or there's > > some other currently undocumented supply that is required for that > > reason. > I can add requirement of at least one supply. I don't think it changes > much, but sure. I wouldn't, I suspect it's the latter case and there's actually another undocumented change.
On 26/07/2023 14:30, Mark Brown wrote: > On Wed, Jul 26, 2023 at 02:18:43PM +0200, Krzysztof Kozlowski wrote: >> On 26/07/2023 13:55, Mark Brown wrote: > >>> Oh, you mean if the regulator is in use in the system rather than if >>> it's enabled! > >> Enabled as "always-on" or "boot-on" could be encoded in the schema with >> multiple if::then:. But it is not enough, because regulators can be >> enabled on demand by drivers. So that's what I meant by "used". > > Right, you said "enabled" in the changelog bit though. Regulator can be enabled in different ways, including by drivers. The text in description was accurate. Best regards, Krzysztof
On Tue, 25 Jul 2023 08:31:32 +0200, Krzysztof Kozlowski wrote: > Convert the bindings for Dialog Semiconductor SLG51000 Voltage Regulator > to DT schema. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next Thanks! [1/1] regulator: dt-bindings: dlg,slg51000: Convert to DT schema commit: cfef69cbe3726c095f55769bd0e7c72f32bf5060 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On Wed, Jul 26, 2023 at 06:06:24PM +0200, Krzysztof Kozlowski wrote: > On 26/07/2023 14:30, Mark Brown wrote: > > Right, you said "enabled" in the changelog bit though. > Regulator can be enabled in different ways, including by drivers. The > text in description was accurate. No, really. The requirement here is orthogonal to the regulator ever being powered on.
diff --git a/Documentation/devicetree/bindings/regulator/dlg,slg51000.yaml b/Documentation/devicetree/bindings/regulator/dlg,slg51000.yaml new file mode 100644 index 000000000000..0a901345ce33 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/dlg,slg51000.yaml @@ -0,0 +1,132 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/regulator/dlg,slg51000.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Dialog Semiconductor SLG51000 Voltage Regulator + +maintainers: + - Eric Jeong <eric.jeong.opensource@diasemi.com> + - Support Opensource <support.opensource@diasemi.com> + +properties: + compatible: + const: dlg,slg51000 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + dlg,cs-gpios: + maxItems: 1 + description: + GPIO for chip select + + vin3-supply: + description: + Input supply for ldo3, required if regulatoris enabled + + vin4-supply: + description: + Input supply for ldo4, required if regulatoris enabled + + vin5-supply: + description: + Input supply for ldo5, required if regulatoris enabled + + vin6-supply: + description: + Input supply for ldo6, required if regulatoris enabled + + vin7-supply: + description: + Input supply for ldo7, required if regulatoris enabled + + regulators: + type: object + additionalProperties: false + + patternProperties: + "^ldo[1-7]$": + type: object + $ref: /schemas/regulator/regulator.yaml# + unevaluatedProperties: false + + properties: + enable-gpios: + maxItems: 1 + + required: + - regulator-name + +required: + - compatible + - reg + - regulators + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/regulator/dlg,da9121-regulator.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + + pmic@75 { + compatible = "dlg,slg51000"; + reg = <0x75>; + dlg,cs-gpios = <&tlmm 69 GPIO_ACTIVE_HIGH>; + vin5-supply = <&vreg_s1f_1p2>; + vin6-supply = <&vreg_s1f_1p2>; + + regulators { + ldo1 { + regulator-name = "slg51000_b_ldo1"; + regulator-min-microvolt = <2400000>; + regulator-max-microvolt = <3300000>; + }; + + ldo2 { + regulator-name = "slg51000_b_ldo2"; + regulator-min-microvolt = <2400000>; + regulator-max-microvolt = <3300000>; + }; + + ldo3 { + regulator-name = "slg51000_b_ldo3"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3750000>; + }; + + ldo4 { + regulator-name = "slg51000_b_ldo4"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3750000>; + }; + + ldo5 { + regulator-name = "slg51000_b_ldo5"; + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <1200000>; + }; + + ldo6 { + regulator-name = "slg51000_b_ldo6"; + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <1200000>; + }; + + ldo7 { + regulator-name = "slg51000_b_ldo7"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3750000>; + }; + }; + }; + }; diff --git a/Documentation/devicetree/bindings/regulator/slg51000.txt b/Documentation/devicetree/bindings/regulator/slg51000.txt deleted file mode 100644 index aa0733e49b90..000000000000 --- a/Documentation/devicetree/bindings/regulator/slg51000.txt +++ /dev/null @@ -1,88 +0,0 @@ -* Dialog Semiconductor SLG51000 Voltage Regulator - -Required properties: -- compatible : Should be "dlg,slg51000" for SLG51000 -- reg : Specifies the I2C slave address. -- xxx-supply: Input voltage supply regulator for ldo3 to ldo7. - These entries are required if regulators are enabled for a device. - An absence of these properties can cause the regulator registration to fail. - If some of input supply is powered through battery or always-on supply then - also it is required to have these parameters with proper node handle of always - on power supply. - vin3-supply: Input supply for ldo3 - vin4-supply: Input supply for ldo4 - vin5-supply: Input supply for ldo5 - vin6-supply: Input supply for ldo6 - vin7-supply: Input supply for ldo7 - -Optional properties: -- interrupt-parent : Specifies the reference to the interrupt controller. -- interrupts : IRQ line information. -- dlg,cs-gpios : Specify a valid GPIO for chip select - -Sub-nodes: -- regulators : This node defines the settings for the regulators. - The content of the sub-node is defined by the standard binding - for regulators; see regulator.txt. - - The SLG51000 regulators are bound using their names listed below: - ldo1 - ldo2 - ldo3 - ldo4 - ldo5 - ldo6 - ldo7 - -Optional properties for regulators: -- enable-gpios : Specify a valid GPIO for platform control of the regulator. - -Example: - pmic: slg51000@75 { - compatible = "dlg,slg51000"; - reg = <0x75>; - - regulators { - ldo1 { - regulator-name = "ldo1"; - regulator-min-microvolt = <2400000>; - regulator-max-microvolt = <3300000>; - }; - - ldo2 { - regulator-name = "ldo2"; - regulator-min-microvolt = <2400000>; - regulator-max-microvolt = <3300000>; - }; - - ldo3 { - regulator-name = "ldo3"; - regulator-min-microvolt = <1200000>; - regulator-max-microvolt = <3750000>; - }; - - ldo4 { - regulator-name = "ldo4"; - regulator-min-microvolt = <1200000>; - regulator-max-microvolt = <3750000>; - }; - - ldo5 { - regulator-name = "ldo5"; - regulator-min-microvolt = <500000>; - regulator-max-microvolt = <1200000>; - }; - - ldo6 { - regulator-name = "ldo6"; - regulator-min-microvolt = <500000>; - regulator-max-microvolt = <1200000>; - }; - - ldo7 { - regulator-name = "ldo7"; - regulator-min-microvolt = <1200000>; - regulator-max-microvolt = <3750000>; - }; - }; - }; diff --git a/MAINTAINERS b/MAINTAINERS index 1f2645d07b59..8955cf44fc81 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6039,7 +6039,7 @@ F: Documentation/devicetree/bindings/mfd/da90*.txt F: Documentation/devicetree/bindings/mfd/dlg,da90*.yaml F: Documentation/devicetree/bindings/regulator/da92*.txt F: Documentation/devicetree/bindings/regulator/dlg,da9*.yaml -F: Documentation/devicetree/bindings/regulator/slg51000.txt +F: Documentation/devicetree/bindings/regulator/dlg,slg51000.yaml F: Documentation/devicetree/bindings/sound/da[79]*.txt F: Documentation/devicetree/bindings/thermal/da90??-thermal.txt F: Documentation/devicetree/bindings/watchdog/da90??-wdt.txt
Convert the bindings for Dialog Semiconductor SLG51000 Voltage Regulator to DT schema. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Changes in v2: 1. Mention that supplies are required, if the regulator is enabled. --- .../bindings/regulator/dlg,slg51000.yaml | 132 ++++++++++++++++++ .../bindings/regulator/slg51000.txt | 88 ------------ MAINTAINERS | 2 +- 3 files changed, 133 insertions(+), 89 deletions(-) create mode 100644 Documentation/devicetree/bindings/regulator/dlg,slg51000.yaml delete mode 100644 Documentation/devicetree/bindings/regulator/slg51000.txt