[PATCH v3 0/2] PCI: mediatek-gen3: Support controlling power supplies

[PATCH v3 0/2] PCI: mediatek-gen3: Support controlling power supplies

[Jian Yang]

These series patches is based on linux-next, tag: next-20230921.
Add support for controlling power supplies and reset GPIO of a downstream
component in Mediatek's PCIe GEN3 controller driver.

Changes in v3:
1. Modify description of power supply properties in DT binding.
2. Remove unused header files.
3. Use 'device_wakeup_path' to determine whether the downstream component
needs to skip the reset process in system suspend scenarios.

jian.yang (2):
  dt-bindings: PCI: mediatek-gen3: Add support for controlling power and
    reset
  PCI: mediatek-gen3: Add power and reset control feature for downstream
    component

 .../bindings/pci/mediatek-pcie-gen3.yaml      | 25 +++++
 drivers/pci/controller/pcie-mediatek-gen3.c   | 93 ++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletion(-)

-- 
2.18.0

Re: [PATCH v3 1/2] dt-bindings: PCI: mediatek-gen3: Add support for controlling power and reset

[Rob Herring]

On Thu, 28 Sep 2023 18:58:20 +0800, Jian Yang wrote:
> From: "jian.yang" <jian.yang@mediatek.com>
> 
> Add new properties to support control power supplies and reset pin of
> a downstream component.
> 
> Signed-off-by: jian.yang <jian.yang@mediatek.com>
> ---
>  .../bindings/pci/mediatek-pcie-gen3.yaml      | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dts:72.24-75.19: Warning (unit_address_vs_reg): /example-0/bus/pcie@11230000/pcie@0: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dts:72.24-75.19: Warning (pci_bridge): /example-0/bus/pcie@11230000/pcie@0: missing ranges for PCI bridge (or not a bridge)
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dts:72.24-75.19: Warning (pci_bridge): /example-0/bus/pcie@11230000/pcie@0: incorrect #address-cells for PCI bridge
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dts:72.24-75.19: Warning (pci_bridge): /example-0/bus/pcie@11230000/pcie@0: incorrect #size-cells for PCI bridge
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: Warning (unit_address_format): Failed prerequisite 'pci_bridge'
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: Warning (pci_device_reg): Failed prerequisite 'pci_bridge'
Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: pcie@11230000: pcie@0: 'reg' is a required property
	from schema $id: http://devicetree.org/schemas/pci/mediatek-pcie-gen3.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: pcie@11230000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'bus-range', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'pcie@0' were unexpected)
	from schema $id: http://devicetree.org/schemas/pci/mediatek-pcie-gen3.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: pcie@11230000: pcie@0: 'reg' is a required property
	from schema $id: http://devicetree.org/schemas/pci/pci-bus.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: pcie@0: 'ranges' is a required property
	from schema $id: http://devicetree.org/schemas/pci/pci-bus.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: pcie@0: 'reg' is a required property
	from schema $id: http://devicetree.org/schemas/pci/pci-bus.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: pcie@0: '#address-cells' is a required property
	from schema $id: http://devicetree.org/schemas/pci/pci-bus.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.example.dtb: pcie@0: '#size-cells' is a required property
	from schema $id: http://devicetree.org/schemas/pci/pci-bus.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230928105819.5161-2-jian.yang@mediatek.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 1/2] dt-bindings: PCI: mediatek-gen3: Add support for controlling power and reset

[Rob Herring]

On Thu, Sep 28, 2023 at 06:58:20PM +0800, Jian Yang wrote:
> From: "jian.yang" <jian.yang@mediatek.com>
> 
> Add new properties to support control power supplies and reset pin of
> a downstream component.
> 
> Signed-off-by: jian.yang <jian.yang@mediatek.com>
> ---
>  .../bindings/pci/mediatek-pcie-gen3.yaml      | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> index 7e8c7a2a5f9b..32031362db58 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> @@ -84,6 +84,26 @@ properties:
>      items:
>        enum: [ phy, mac ]
>  
> +  pcie1v8-supply:
> +    description:
> +      The regulator phandle that provides 1.8V power from root port to a
> +      downstream component.
> +
> +  pcie3v3-supply:
> +    description:
> +      The regulator phandle that provides 3.3V power from root port to a
> +      downstream component.
> +
> +  pcie12v-supply:
> +    description:
> +      The regulator phandle that provides 12V power from root port to a
> +      downstream component.
> +
> +  dsc-reset-gpios:

This should be in the downstream component if it is something extra. So 
not the root port node, but the next one down. 

> +    description:
> +      The extra reset pin other than PERST# required by a downstream component.
> +    maxItems: 1
> +
>    clocks:
>      minItems: 4
>      maxItems: 6
> @@ -238,5 +258,10 @@ examples:
>                        #interrupt-cells = <1>;
>                        interrupt-controller;
>              };
> +
> +            pcie@0 {

Missing 'reg'.

> +              device_type = "pci";
> +              pcie-3v3-supply = <&pcie3v3_regulator>;

This is in the root port (which is good), but you've defined the schema 
to put them in the host bridge node. IOW, these need to go in a PCI 
root-port or P2P bridge schema which doesn't yet exist. I have an 
inprogress branch for dtschema to split up pci-bus.yaml for that 
purpose. Will try to finish it up soon.

Rob

[PATCH v3 0/2] PCI: mediatek-gen3: Support controlling power supplies

[Jian Yang]

These series patches is based on linux-next, tag: next-20231009.
Add support for controlling power supplies and reset GPIO of a downstream
component in Mediatek's PCIe GEN3 controller driver.

Changes in v3:
1. Modify description of power supply properties in DT binding.
2. Remove unused header files.
3. Use 'device_wakeup_path' to determine whether the downstream component
needs to skip the reset process in system suspend scenarios.

jian.yang (2):
  dt-bindings: PCI: mediatek-gen3: Add support for controlling power and
    reset
  PCI: mediatek-gen3: Add power and reset control feature for downstream
    component

 .../bindings/pci/mediatek-pcie-gen3.yaml      | 30 ++++++
 drivers/pci/controller/pcie-mediatek-gen3.c   | 93 ++++++++++++++++++-
 2 files changed, 122 insertions(+), 1 deletion(-)

-- 
2.18.0

Re: [PATCH v3 1/2] dt-bindings: PCI: mediatek-gen3: Add support for controlling power and reset

[AngeloGioacchino Del Regno]

Il 09/10/23 10:49, Jian Yang ha scritto:
> From: "jian.yang" <jian.yang@mediatek.com>
> 
> Add new properties to support control power supplies and reset pin of
> a downstream component.
> 
> Signed-off-by: jian.yang <jian.yang@mediatek.com>
> ---
>   .../bindings/pci/mediatek-pcie-gen3.yaml      | 30 +++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> index 7e8c7a2a5f9b..eb4ad98549d1 100644
> --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> @@ -84,6 +84,26 @@ properties:
>       items:
>         enum: [ phy, mac ]
>   
> +  pcie1v8-supply:

There are another two controllers having such regulators and they all have the
same name for those supplies; Can you please change the names to be consistent
with the other controllers?

vpcie1v8
vpcie3v3
vpcie12v

Regards,
Angelo

Re: [PATCH v3 1/2] dt-bindings: PCI: mediatek-gen3: Add support for controlling power and reset

[Jian Yang (杨戬)]

Hi Angelo,

Thanks for your comment.

On Tue, 2023-10-10 at 11:45 +0200, AngeloGioacchino Del Regno wrote:
> Il 09/10/23 10:49, Jian Yang ha scritto:
> > From: "jian.yang" <jian.yang@mediatek.com>
> > 
> > Add new properties to support control power supplies and reset pin
> > of
> > a downstream component.
> > 
> > Signed-off-by: jian.yang <jian.yang@mediatek.com>
> > ---
> >   .../bindings/pci/mediatek-pcie-gen3.yaml      | 30
> > +++++++++++++++++++
> >   1 file changed, 30 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-
> > gen3.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-
> > gen3.yaml
> > index 7e8c7a2a5f9b..eb4ad98549d1 100644
> > --- a/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > +++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-gen3.yaml
> > @@ -84,6 +84,26 @@ properties:
> >       items:
> >         enum: [ phy, mac ]
> >   
> > +  pcie1v8-supply:
> 
> There are another two controllers having such regulators and they all
> have the
> same name for those supplies; Can you please change the names to be
> consistent
> with the other controllers?
> 
> vpcie1v8
> vpcie3v3
> vpcie12v
> 
I will rename them in next patch version.

> Regards,
> Angelo
> 
> 
Best regards,
Jian Yang