On Thu, Dec 14, 2023 at 10:03:51AM +0000, Sherry Sun wrote: > > > > -----Original Message----- > > From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > Sent: 2023年12月14日 13:13 > > To: Sherry Sun <sherry.sun@xxxxxxx> > > Cc: Hongxing Zhu <hongxing.zhu@xxxxxxx>; l.stach@xxxxxxxxxxxxxx; > > lpieralisi@xxxxxxxxxx; kw@xxxxxxxxx; robh@xxxxxxxxxx; > > bhelgaas@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; > > conor+dt@xxxxxxxxxx; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; > > kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-imx <linux- > > imx@xxxxxxx>; linux-pci@xxxxxxxxxxxxxxx; linux-arm- > > kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux- > > kernel@xxxxxxxxxxxxxxx > > 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 > > > > > > > > > > -- > > மணிவண்ணன் சதாசிவம் -- மணிவண்ணன் சதாசிவம்
