Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Thu, 14 Dec 2023 15:45:15 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To:     Sherry Sun <sherry.sun@nxp.com>
Cc:     Hongxing Zhu <hongxing.zhu@nxp.com>,
        "l.stach@pengutronix.de" <l.stach@pengutronix.de>,
        "lpieralisi@kernel.org" <lpieralisi@kernel.org>,
        "kw@linux.com" <kw@linux.com>, "robh@kernel.org" <robh@kernel.org>,
        "bhelgaas@google.com" <bhelgaas@google.com>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "conor+dt@kernel.org" <conor+dt@kernel.org>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>,
        "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
        "kernel@pengutronix.de" <kernel@pengutronix.de>,
        "festevam@gmail.com" <festevam@gmail.com>,
        dl-linux-imx <linux-imx@nxp.com>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V2 0/4] PCI: imx6: Add pci host wakeup support
Message-ID: <20231214101515.GJ2938@thinkpad>
References: <20231213092850.1706042-1-sherry.sun@nxp.com>
 <20231214051328.GD2938@thinkpad>
 <AS8PR04MB84046143C236AEA322BBBAD3928CA@AS8PR04MB8404.eurprd04.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <AS8PR04MB84046143C236AEA322BBBAD3928CA@AS8PR04MB8404.eurprd04.prod.outlook.com>
Precedence: bulk

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
> > >
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்

-- 
மணிவண்ணன் சதாசிவம்