| Message ID | 20231213092850.1706042-1-sherry.sun@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | PCI: imx6: Add pci host wakeup support | expand |
On Wed, Dec 13, 2023 at 05:28:46PM +0800, Sherry Sun wrote: > Add pci host wakeup feature for imx platforms. The host wake pin is a > standard feature in the PCIe bus specification, so we can add this > property under PCI dts node to support the host gpio wakeup feature. > > Example of configuring the corresponding dts property under the PCI node: > wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>; > As you mentioned, WAKE# is a standard sideband signal defined in the PCI spec. So the support for handling it has to be in the PCI core layer, not in the host controller drivers. There is already a series floating to add support for WAKE# in PCI core. Please take a look: https://lore.kernel.org/linux-pci/20230208111645.3863534-1-mmaddireddy@nvidia.com/ - Mani > --- > changes in V2: > 1. Rename host-wake-gpio property to wake-gpios. > 2. Improve the wake-gpios property description in the dt-binding doc to avoid > confusion. > 3. Remove unnecessary debugging info in host_wake_irq_handler(). > 4. Remove unnecessary imx6_pcie->host_wake_irq = -1 resetting in error paths. > 5. Use dev_err_probe() to simplify error path code. > --- > > Sherry Sun (4): > PCI: imx6: Add pci host wakeup support on imx platforms. > dt-bindings: imx6q-pcie: Add wake-gpios property > arm64: dts: imx8mp-evk: add wake-gpios property for pci bus > arm64: dts: imx8mq-evk: add wake-gpios property for pci bus > > .../bindings/pci/fsl,imx6q-pcie.yaml | 6 ++ > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 + > arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 2 + > drivers/pci/controller/dwc/pci-imx6.c | 60 +++++++++++++++++++ > 4 files changed, 70 insertions(+) > > -- > 2.34.1 > >
> -----Original Message----- > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Sent: 2023年12月14日 13:13 > To: Sherry Sun <sherry.sun@nxp.com> > Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de; > 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; 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 V2 0/4] PCI: imx6: Add pci host wakeup support > > On Wed, Dec 13, 2023 at 05:28:46PM +0800, Sherry Sun wrote: > > Add pci host wakeup feature for imx platforms. The host wake pin is a > > standard feature in the PCIe bus specification, so we can add this > > property under PCI dts node to support the host gpio wakeup feature. > > > > Example of configuring the corresponding dts property under the PCI node: > > wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>; > > > > As you mentioned, WAKE# is a standard sideband signal defined in the PCI > spec. > So the support for handling it has to be in the PCI core layer, not in the host > controller drivers. > > There is already a series floating to add support for WAKE# in PCI core. > Please take a look: > > https://lore.k/ > ernel.org%2Flinux-pci%2F20230208111645.3863534-1- > mmaddireddy%40nvidia.com%2F&data=05%7C02%7Csherry.sun%40nxp.co > m%7C0254c001df61498c09d408dbfc636f5c%7C686ea1d3bc2b4c6fa92cd99c5 > c301635%7C0%7C0%7C638381276239824912%7CUnknown%7CTWFpbGZsb3 > d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0% > 3D%7C3000%7C%7C%7C&sdata=IoBAwTy0qeb0J6JrK0WRhI8A4ThUfkVx6mri > ve%2BK5xs%3D&reserved=0 Hi Manivannan, I checked the patch set, the implementation of host wake gpio is different from mine, I referred to the mmc bus cd(card detect) pin implementation and I think it is simpler and clearer. Regarding whether the WAKE# support should be moved to PCI core layer, we may need more research and discussion. Thanks for your suggestions. Best Regards Sherry > > - Mani > > > --- > > changes in V2: > > 1. Rename host-wake-gpio property to wake-gpios. > > 2. Improve the wake-gpios property description in the dt-binding doc > > to avoid confusion. > > 3. Remove unnecessary debugging info in host_wake_irq_handler(). > > 4. Remove unnecessary imx6_pcie->host_wake_irq = -1 resetting in error > paths. > > 5. Use dev_err_probe() to simplify error path code. > > --- > > > > Sherry Sun (4): > > PCI: imx6: Add pci host wakeup support on imx platforms. > > dt-bindings: imx6q-pcie: Add wake-gpios property > > arm64: dts: imx8mp-evk: add wake-gpios property for pci bus > > arm64: dts: imx8mq-evk: add wake-gpios property for pci bus > > > > .../bindings/pci/fsl,imx6q-pcie.yaml | 6 ++ > > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 + > > arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 2 + > > drivers/pci/controller/dwc/pci-imx6.c | 60 +++++++++++++++++++ > > 4 files changed, 70 insertions(+) > > > > -- > > 2.34.1 > > > > > > -- > மணிவண்ணன் சதாசிவம்
On Thu, Dec 14, 2023 at 10:03:51AM +0000, Sherry Sun wrote: > > > > -----Original Message----- > > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Sent: 2023年12月14日 13:13 > > To: Sherry Sun <sherry.sun@nxp.com> > > Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de; > > 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; 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 V2 0/4] PCI: imx6: Add pci host wakeup support > > > > On Wed, Dec 13, 2023 at 05:28:46PM +0800, Sherry Sun wrote: > > > Add pci host wakeup feature for imx platforms. The host wake pin is a > > > standard feature in the PCIe bus specification, so we can add this > > > property under PCI dts node to support the host gpio wakeup feature. > > > > > > Example of configuring the corresponding dts property under the PCI node: > > > wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>; > > > > > > > As you mentioned, WAKE# is a standard sideband signal defined in the PCI > > spec. > > So the support for handling it has to be in the PCI core layer, not in the host > > controller drivers. > > > > There is already a series floating to add support for WAKE# in PCI core. > > Please take a look: > > > > https://lore.k/ > > ernel.org%2Flinux-pci%2F20230208111645.3863534-1- > > mmaddireddy%40nvidia.com%2F&data=05%7C02%7Csherry.sun%40nxp.co > > m%7C0254c001df61498c09d408dbfc636f5c%7C686ea1d3bc2b4c6fa92cd99c5 > > c301635%7C0%7C0%7C638381276239824912%7CUnknown%7CTWFpbGZsb3 > > d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0% > > 3D%7C3000%7C%7C%7C&sdata=IoBAwTy0qeb0J6JrK0WRhI8A4ThUfkVx6mri > > ve%2BK5xs%3D&reserved=0 > > Hi Manivannan, > I checked the patch set, the implementation of host wake gpio is different from mine, I referred to the mmc bus cd(card detect) pin implementation and I think it is simpler and clearer. It's not just about simple and clear code, but about scalability. See below. > Regarding whether the WAKE# support should be moved to PCI core layer, we may need more research and discussion. Thanks for your suggestions. > We can research and come up with a better solution, but the implementation has to be done in the PCI core layer. Otherwise, host controllers supporting WAKE# has to duplicate the code which is common. - Mani > Best Regards > Sherry > > > > > > - Mani > > > > > --- > > > changes in V2: > > > 1. Rename host-wake-gpio property to wake-gpios. > > > 2. Improve the wake-gpios property description in the dt-binding doc > > > to avoid confusion. > > > 3. Remove unnecessary debugging info in host_wake_irq_handler(). > > > 4. Remove unnecessary imx6_pcie->host_wake_irq = -1 resetting in error > > paths. > > > 5. Use dev_err_probe() to simplify error path code. > > > --- > > > > > > Sherry Sun (4): > > > PCI: imx6: Add pci host wakeup support on imx platforms. > > > dt-bindings: imx6q-pcie: Add wake-gpios property > > > arm64: dts: imx8mp-evk: add wake-gpios property for pci bus > > > arm64: dts: imx8mq-evk: add wake-gpios property for pci bus > > > > > > .../bindings/pci/fsl,imx6q-pcie.yaml | 6 ++ > > > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 + > > > arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 2 + > > > drivers/pci/controller/dwc/pci-imx6.c | 60 +++++++++++++++++++ > > > 4 files changed, 70 insertions(+) > > > > > > -- > > > 2.34.1 > > > > > > > > > > -- > > மணிவண்ணன் சதாசிவம்
Add pci host wakeup feature for imx platforms. The host wake pin is a standard feature in the PCIe bus specification, so we can add this property under PCI dts node to support the host gpio wakeup feature. Example of configuring the corresponding dts property under the PCI node: wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>; --- changes in V2: 1. Rename host-wake-gpio property to wake-gpios. 2. Improve the wake-gpios property description in the dt-binding doc to avoid confusion. 3. Remove unnecessary debugging info in host_wake_irq_handler(). 4. Remove unnecessary imx6_pcie->host_wake_irq = -1 resetting in error paths. 5. Use dev_err_probe() to simplify error path code. --- Sherry Sun (4): PCI: imx6: Add pci host wakeup support on imx platforms. dt-bindings: imx6q-pcie: Add wake-gpios property arm64: dts: imx8mp-evk: add wake-gpios property for pci bus arm64: dts: imx8mq-evk: add wake-gpios property for pci bus .../bindings/pci/fsl,imx6q-pcie.yaml | 6 ++ arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 + arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 2 + drivers/pci/controller/dwc/pci-imx6.c | 60 +++++++++++++++++++ 4 files changed, 70 insertions(+)