diff mbox series

[v3,1/2] dt-bindings: PCI: mediatek-gen3: Add property to disable dvfsrc voltage request

Message ID 20210630024934.18903-2-jianjun.wang@mediatek.com
State New
Headers show
Series PCI: mediatek-gen3: Add support for disable dvfsrc | expand

Commit Message

Jianjun Wang (王建军) June 30, 2021, 2:49 a.m. UTC
Add property to disable dvfsrc voltage request, if this property
is presented, we assume that the requested voltage is always
higher enough to keep the PCIe controller active.

Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
 .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml       | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

qizhong cheng June 30, 2021, 3:40 a.m. UTC | #1
Reviewed-by: Qizhong Cheng <qizhong.cheng@mediatek.com>
Tested-by: Qizhong Cheng <qizhong.cheng@mediatek.com>

On Wed, 2021-06-30 at 10:49 +0800, Jianjun Wang wrote:
> Add property to disable dvfsrc voltage request, if this property
> is presented, we assume that the requested voltage is always
> higher enough to keep the PCIe controller active.
> 
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
>  .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml       | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> index e7b1f9892da4..3e26c032cea9 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> @@ -96,6 +96,12 @@ properties:
>    phys:
>      maxItems: 1
>  
> +  disable-dvfsrc-vlt-req:
> +    description: Disable dvfsrc voltage request, if this property is presented,
> +      we assume that the requested voltage is always higher enough to keep
> +      the PCIe controller active.
> +    type: boolean
> +
>    '#interrupt-cells':
>      const: 1
>  
> @@ -166,6 +172,8 @@ examples:
>                       <&infracfg_rst 3>;
>              reset-names = "phy", "mac";
>  
> +            disable-dvfsrc-vlt-req;
> +
>              #interrupt-cells = <1>;
>              interrupt-map-mask = <0 0 0 0x7>;
>              interrupt-map = <0 0 0 1 &pcie_intc 0>,
Jianjun Wang (王建军) July 14, 2021, 11:20 a.m. UTC | #2
Hi,

Just gentle ping for this patch set, please kindly let me know your
comments about this patch set.

Thanks.

On Wed, 2021-06-30 at 11:40 +0800, Qizhong Cheng wrote:
> Reviewed-by: Qizhong Cheng <qizhong.cheng@mediatek.com>
> Tested-by: Qizhong Cheng <qizhong.cheng@mediatek.com>
> 
> On Wed, 2021-06-30 at 10:49 +0800, Jianjun Wang wrote:
> > Add property to disable dvfsrc voltage request, if this property
> > is presented, we assume that the requested voltage is always
> > higher enough to keep the PCIe controller active.
> > 
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> >  .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml       | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > index e7b1f9892da4..3e26c032cea9 100644
> > --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > @@ -96,6 +96,12 @@ properties:
> >    phys:
> >      maxItems: 1
> >  
> > +  disable-dvfsrc-vlt-req:
> > +    description: Disable dvfsrc voltage request, if this property is presented,
> > +      we assume that the requested voltage is always higher enough to keep
> > +      the PCIe controller active.
> > +    type: boolean
> > +
> >    '#interrupt-cells':
> >      const: 1
> >  
> > @@ -166,6 +172,8 @@ examples:
> >                       <&infracfg_rst 3>;
> >              reset-names = "phy", "mac";
> >  
> > +            disable-dvfsrc-vlt-req;
> > +
> >              #interrupt-cells = <1>;
> >              interrupt-map-mask = <0 0 0 0x7>;
> >              interrupt-map = <0 0 0 1 &pcie_intc 0>,
> 
>
Rob Herring (Arm) July 16, 2021, 5:33 p.m. UTC | #3
On Wed, Jun 30, 2021 at 10:49:33AM +0800, Jianjun Wang wrote:
> Add property to disable dvfsrc voltage request, if this property
> is presented, we assume that the requested voltage is always
> higher enough to keep the PCIe controller active.
> 
> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
>  .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml       | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> index e7b1f9892da4..3e26c032cea9 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> @@ -96,6 +96,12 @@ properties:
>    phys:
>      maxItems: 1
>  
> +  disable-dvfsrc-vlt-req:
> +    description: Disable dvfsrc voltage request, if this property is presented,
> +      we assume that the requested voltage is always higher enough to keep
> +      the PCIe controller active.
> +    type: boolean

What determines setting this property? Can it be implied by the 
compatible (which should be SoC specific).

Is this property specific to PCIe controller? 

Wouldn't the request be harmless to make the voltage request even if not 
needed?

I think this probably should be addressed in a common way as part of 
other QoS, devfreq, etc. requirements for devices.

Rob
Jianjun Wang (王建军) Aug. 19, 2021, 3:40 a.m. UTC | #4
On Fri, 2021-07-16 at 11:33 -0600, Rob Herring wrote:
> On Wed, Jun 30, 2021 at 10:49:33AM +0800, Jianjun Wang wrote:
> > Add property to disable dvfsrc voltage request, if this property
> > is presented, we assume that the requested voltage is always
> > higher enough to keep the PCIe controller active.
> > 
> > Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> > ---
> >  .../devicetree/bindings/pci/mediatek-pcie-gen3.yaml       | 8
> > ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-
> > gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-
> > gen3.yaml
> > index e7b1f9892da4..3e26c032cea9 100644
> > --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > @@ -96,6 +96,12 @@ properties:
> >    phys:
> >      maxItems: 1
> >  
> > +  disable-dvfsrc-vlt-req:
> > +    description: Disable dvfsrc voltage request, if this property
> > is presented,
> > +      we assume that the requested voltage is always higher enough
> > to keep
> > +      the PCIe controller active.
> > +    type: boolean
> What determines setting this property? Can it be implied by the 
> compatible (which should be SoC specific).
> 
> Is this property specific to PCIe controller? 
> 
> Wouldn't the request be harmless to make the voltage request even if
> not 
> needed?
> 
> I think this probably should be addressed in a common way as part of 
> other QoS, devfreq, etc. requirements for devices.
> 
> Rob

Hi Rob,

Thanks for your review and sorry for the late response.

We have internal discussion and we agree with that this feature should
not be specific to the PCIe controller, we need to find a common way to
do this. 

But as the driver of dvfsrc is not finished the upstream, we don't have
a better solution for now, so we would like to pull back this patch and
will send another patch to disable dvfsrc by default until we find a
common solution to enable it.

Thanks.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
index e7b1f9892da4..3e26c032cea9 100644
--- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
@@ -96,6 +96,12 @@  properties:
   phys:
     maxItems: 1
 
+  disable-dvfsrc-vlt-req:
+    description: Disable dvfsrc voltage request, if this property is presented,
+      we assume that the requested voltage is always higher enough to keep
+      the PCIe controller active.
+    type: boolean
+
   '#interrupt-cells':
     const: 1
 
@@ -166,6 +172,8 @@  examples:
                      <&infracfg_rst 3>;
             reset-names = "phy", "mac";
 
+            disable-dvfsrc-vlt-req;
+
             #interrupt-cells = <1>;
             interrupt-map-mask = <0 0 0 0x7>;
             interrupt-map = <0 0 0 1 &pcie_intc 0>,