| Message ID | 20231208091355.1417292-3-sherry.sun@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
> -----Original Message----- > From: Lucas Stach <l.stach@pengutronix.de> > Sent: 2023年12月8日 18:00 > To: Sherry Sun <sherry.sun@nxp.com>; Hongxing Zhu > <hongxing.zhu@nxp.com>; lpieralisi@kernel.org; kw@linux.com; > robh@kernel.org; bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org; > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de; > kernel@pengutronix.de; festevam@gmail.com > Cc: dl-linux-imx <linux-imx@nxp.com>; linux-pci@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH 2/4] dt-bindings: imx6q-pcie: Add host-wake-gpio > property > > Hi Sherry, > > Am Freitag, dem 08.12.2023 um 17:13 +0800 schrieb Sherry Sun: > > Add host-wake-gpio property that can be used to wakeup the host > > processor. > > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com> > > Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com> > > --- > > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > index 81bbb8728f0f..944f0f961809 100644 > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > @@ -72,6 +72,10 @@ properties: > > L=operation state) (optional required). > > type: boolean > > > > + host-wake-gpio: > > There is only one wake signal in PCIe and it has a defined direction, so there > is no point in specifying that it is a host wakeup. Also GPIO handles without a > traling 's' are deprecated. So this should be > > wake-gpios Hi Lucas, thanks for the comment, will change it in V2. > > > + description: Should specify the GPIO for controlling the PCI bus device > > + wake signal, used to wakeup the host processor. Default to active-low. > > The description is wrong. For the RC complex case (which is the binding you > are modifying here) the controller does not control the wake signal, but > instead uses it as a input. The description should reflect that. > > The default is also quite useless, as your implementation does not allow to > change it. Please translate the GPIO active flags from the DT to the proper > IRQ flags and drop this default here. The DT should simply carry the proper > polarity. > Will improve the description in V2, thanks. Best Regards Sherry
> -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: 2023年12月9日 4:56 > To: Lucas Stach <l.stach@pengutronix.de> > Cc: Sherry Sun <sherry.sun@nxp.com>; Hongxing Zhu > <hongxing.zhu@nxp.com>; lpieralisi@kernel.org; kw@linux.com; > bhelgaas@google.com; krzysztof.kozlowski+dt@linaro.org; > conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de; > kernel@pengutronix.de; festevam@gmail.com; dl-linux-imx <linux- > imx@nxp.com>; linux-pci@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH 2/4] dt-bindings: imx6q-pcie: Add host-wake-gpio > property > > On Fri, Dec 08, 2023 at 11:00:19AM +0100, Lucas Stach wrote: > > Hi Sherry, > > > > Am Freitag, dem 08.12.2023 um 17:13 +0800 schrieb Sherry Sun: > > > Add host-wake-gpio property that can be used to wakeup the host > > > processor. > > > > > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com> > > > Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com> > > > --- > > > Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > > index 81bbb8728f0f..944f0f961809 100644 > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > > @@ -72,6 +72,10 @@ properties: > > > L=operation state) (optional required). > > > type: boolean > > > > > > + host-wake-gpio: > > > > There is only one wake signal in PCIe and it has a defined direction, > > so there is no point in specifying that it is a host wakeup. Also GPIO > > handles without a traling 's' are deprecated. So this should be > > > > wake-gpios > > Any standard PCI slot signals need to be documented in common PCI schema. > And they should start going into root port nodes rather than the host bridge > node because it's the root ports that correspond to slots rather than the host > bridge. We've just taken shortcuts because many host bridges only have 1 > root port. > > Note that I'm in the middle of splitting pci-bus.yaml into host bridge, PCI-PCI > bridge (and RP), and common device schemas. > Hi Rob, thanks for your comment, I am new to PCIe, can you please provide more details on which common PCI schema the WAKE# property should be added in (maybe snps,dw-pcie.yaml or something else)? Best Regards Sherry
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml index 81bbb8728f0f..944f0f961809 100644 --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml @@ -72,6 +72,10 @@ properties: L=operation state) (optional required). type: boolean + host-wake-gpio: + description: Should specify the GPIO for controlling the PCI bus device + wake signal, used to wakeup the host processor. Default to active-low. + required: - compatible - reg