| Message ID | 20220711094549.3445566-2-martin.kepplinger@puri.sm (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/3] power: domain: handle power supplies that need irq | expand |
On 11/07/2022 11:45, Martin Kepplinger wrote: > Add the power-supply-needs-irq board description property for power domains. Where is a board description here? I think you just meant "power-supply-needs-irq property"? > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > --- > .../devicetree/bindings/power/power-domain.yaml | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/power/power-domain.yaml b/Documentation/devicetree/bindings/power/power-domain.yaml > index 889091b9814f..e82c2f7ccb97 100644 > --- a/Documentation/devicetree/bindings/power/power-domain.yaml > +++ b/Documentation/devicetree/bindings/power/power-domain.yaml > @@ -70,6 +70,16 @@ properties: > by the given provider should be subdomains of the domain specified > by this binding. > > + power-supply: true This is a new property not described in the commit msg. > + > + power-supply-needs-irq: > + type: boolean > + description: > + A power-supply can link for example to a regulator controlled via > + i2c or otherwise needing interrupts enabled to be able to enable and > + disable. Not really a property of power domain. How the regulator supply works is entirely up to regulator. Otherwise such property should appear for every device. > This property makes various callbacks usually run in the > + noirq phase, being run when interrupts are available. Last sentence does not fit - you embed Linux implementation into DT bindings. noirq phase is Linux specific. > + > required: > - "#power-domain-cells" > Best regards, Krzysztof
Am Montag, dem 11.07.2022 um 12:38 +0200 schrieb Krzysztof Kozlowski: > On 11/07/2022 11:45, Martin Kepplinger wrote: > > Add the power-supply-needs-irq board description property for power > > domains. > > Where is a board description here? I think you just meant > "power-supply-needs-irq property"? > > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > --- > > .../devicetree/bindings/power/power-domain.yaml | 10 > > ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/power/power- > > domain.yaml b/Documentation/devicetree/bindings/power/power- > > domain.yaml > > index 889091b9814f..e82c2f7ccb97 100644 > > --- a/Documentation/devicetree/bindings/power/power-domain.yaml > > +++ b/Documentation/devicetree/bindings/power/power-domain.yaml > > @@ -70,6 +70,16 @@ properties: > > by the given provider should be subdomains of the domain > > specified > > by this binding. > > > > + power-supply: true > > This is a new property not described in the commit msg. true, I think it's missing and could be added as a separate patch. > > > + > > + power-supply-needs-irq: > > + type: boolean > > + description: > > + A power-supply can link for example to a regulator > > controlled via > > + i2c or otherwise needing interrupts enabled to be able to > > enable and > > + disable. > > Not really a property of power domain. How the regulator supply works > is > entirely up to regulator. Otherwise such property should appear for > every device. you're right. The power-domain driver could read the power-supply regulator node directly. Still, I think then a new regulator property is needed instead, or is it? > > > This property makes various callbacks usually run in the > > + noirq phase, being run when interrupts are available. > > Last sentence does not fit - you embed Linux implementation into DT > bindings. noirq phase is Linux specific. oh I keep making this mistake. thanks for the fast review! > > > + > > required: > > - "#power-domain-cells" > > > > > Best regards, > Krzysztof
On 11/07/2022 15:17, Martin Kepplinger wrote: > Am Montag, dem 11.07.2022 um 12:38 +0200 schrieb Krzysztof Kozlowski: >> On 11/07/2022 11:45, Martin Kepplinger wrote: >>> Add the power-supply-needs-irq board description property for power >>> domains. >> >> Where is a board description here? I think you just meant >> "power-supply-needs-irq property"? >>> >>> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> >>> --- >>> .../devicetree/bindings/power/power-domain.yaml | 10 >>> ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/power/power- >>> domain.yaml b/Documentation/devicetree/bindings/power/power- >>> domain.yaml >>> index 889091b9814f..e82c2f7ccb97 100644 >>> --- a/Documentation/devicetree/bindings/power/power-domain.yaml >>> +++ b/Documentation/devicetree/bindings/power/power-domain.yaml >>> @@ -70,6 +70,16 @@ properties: >>> by the given provider should be subdomains of the domain >>> specified >>> by this binding. >>> >>> + power-supply: true >> >> This is a new property not described in the commit msg. > > true, I think it's missing and could be added as a separate patch. > >> >>> + >>> + power-supply-needs-irq: >>> + type: boolean >>> + description: >>> + A power-supply can link for example to a regulator >>> controlled via >>> + i2c or otherwise needing interrupts enabled to be able to >>> enable and >>> + disable. >> >> Not really a property of power domain. How the regulator supply works >> is >> entirely up to regulator. Otherwise such property should appear for >> every device. > > you're right. The power-domain driver could read the power-supply > regulator node directly. Still, I think then a new regulator property > is needed instead, or is it? In case of regulator, I am not so sure it needs a dedicated property of DT. If it is I2C regulator - the parent node is I2C bus and regulator device is some child of I2C controller (could be via a MFD device), so no need for dedicated property. If it uses interrupts, then: 1. The presence of interrupts is already known - "interrupts" property. 2. The actual use of interrupts is DT independent and only driver knows it. Best regards, Krzysztof
Am Dienstag, dem 12.07.2022 um 09:01 +0200 schrieb Krzysztof Kozlowski: > On 11/07/2022 15:17, Martin Kepplinger wrote: > > Am Montag, dem 11.07.2022 um 12:38 +0200 schrieb Krzysztof > > Kozlowski: > > > On 11/07/2022 11:45, Martin Kepplinger wrote: > > > > Add the power-supply-needs-irq board description property for > > > > power > > > > domains. > > > > > > Where is a board description here? I think you just meant > > > "power-supply-needs-irq property"? > > > > > > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > > > --- > > > > .../devicetree/bindings/power/power-domain.yaml | 10 > > > > ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/power/power- > > > > domain.yaml b/Documentation/devicetree/bindings/power/power- > > > > domain.yaml > > > > index 889091b9814f..e82c2f7ccb97 100644 > > > > --- a/Documentation/devicetree/bindings/power/power-domain.yaml > > > > +++ b/Documentation/devicetree/bindings/power/power-domain.yaml > > > > @@ -70,6 +70,16 @@ properties: > > > > by the given provider should be subdomains of the domain > > > > specified > > > > by this binding. > > > > > > > > + power-supply: true > > > > > > This is a new property not described in the commit msg. > > > > true, I think it's missing and could be added as a separate patch. > > > > > > > > > + > > > > + power-supply-needs-irq: > > > > + type: boolean > > > > + description: > > > > + A power-supply can link for example to a regulator > > > > controlled via > > > > + i2c or otherwise needing interrupts enabled to be able > > > > to > > > > enable and > > > > + disable. > > > > > > Not really a property of power domain. How the regulator supply > > > works > > > is > > > entirely up to regulator. Otherwise such property should appear > > > for > > > every device. > > > > you're right. The power-domain driver could read the power-supply > > regulator node directly. Still, I think then a new regulator > > property > > is needed instead, or is it? > > In case of regulator, I am not so sure it needs a dedicated property > of > DT. If it is I2C regulator - the parent node is I2C bus and regulator > device is some child of I2C controller (could be via a MFD device), > so > no need for dedicated property. > > If it uses interrupts, then: > 1. The presence of interrupts is already known - "interrupts" > property. > 2. The actual use of interrupts is DT independent and only driver > knows it. thanks for this great suggestion! for the imx8mq devices this (1.) indeed is the case for exactly the 3 regulators I manually describe here. v2 of this patch looks very elegant (and I guess I could have removed the DT people from the email, I forgot). here it is: https://lore.kernel.org/linux-arm-kernel/20220712121832.3659769-1-martin.kepplinger@puri.sm/T/#u > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/power/power-domain.yaml b/Documentation/devicetree/bindings/power/power-domain.yaml index 889091b9814f..e82c2f7ccb97 100644 --- a/Documentation/devicetree/bindings/power/power-domain.yaml +++ b/Documentation/devicetree/bindings/power/power-domain.yaml @@ -70,6 +70,16 @@ properties: by the given provider should be subdomains of the domain specified by this binding. + power-supply: true + + power-supply-needs-irq: + type: boolean + description: + A power-supply can link for example to a regulator controlled via + i2c or otherwise needing interrupts enabled to be able to enable and + disable. This property makes various callbacks usually run in the + noirq phase, being run when interrupts are available. + required: - "#power-domain-cells"
Add the power-supply-needs-irq board description property for power domains. Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> --- .../devicetree/bindings/power/power-domain.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)