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(+)
--
2.34.1
Add pci host wakeup feature for imx platforms.
Example of configuring the corresponding dts property under the PCI
node:
wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
Signed-off-by: Sherry Sun <[email protected]>
Reviewed-by: Richard Zhu <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 60 +++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 74703362aeec..0cf7f21adff8 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -72,6 +72,7 @@ struct imx6_pcie_drvdata {
struct imx6_pcie {
struct dw_pcie *pci;
int reset_gpio;
+ int host_wake_irq;
bool gpio_active_high;
bool link_is_up;
struct clk *pcie_bus;
@@ -1237,11 +1238,44 @@ static int imx6_pcie_resume_noirq(struct device *dev)
return 0;
}
+static int imx6_pcie_suspend(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+ if (imx6_pcie->host_wake_irq >= 0)
+ enable_irq_wake(imx6_pcie->host_wake_irq);
+
+ return 0;
+}
+
+static int imx6_pcie_resume(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+ if (imx6_pcie->host_wake_irq >= 0)
+ disable_irq_wake(imx6_pcie->host_wake_irq);
+
+ return 0;
+}
+
static const struct dev_pm_ops imx6_pcie_pm_ops = {
NOIRQ_SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend_noirq,
imx6_pcie_resume_noirq)
+ SYSTEM_SLEEP_PM_OPS(imx6_pcie_suspend, imx6_pcie_resume)
};
+irqreturn_t host_wake_irq_handler(int irq, void *priv)
+{
+ struct imx6_pcie *imx6_pcie = priv;
+ struct device *dev = imx6_pcie->pci->dev;
+
+ /* Notify PM core we are wakeup source */
+ pm_wakeup_event(dev, 0);
+ pm_system_wakeup();
+
+ return IRQ_HANDLED;
+}
+
static int imx6_pcie_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1250,6 +1284,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
struct device_node *np;
struct resource *dbi_base;
struct device_node *node = dev->of_node;
+ struct gpio_desc *host_wake_gpio;
int ret;
u16 val;
@@ -1457,6 +1492,26 @@ static int imx6_pcie_probe(struct platform_device *pdev)
val |= PCI_MSI_FLAGS_ENABLE;
dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
}
+
+ /* host wakeup support */
+ imx6_pcie->host_wake_irq = -1;
+ host_wake_gpio = devm_gpiod_get_optional(dev, "wake", GPIOD_IN);
+ if (IS_ERR(host_wake_gpio))
+ return PTR_ERR(host_wake_gpio);
+
+ if (host_wake_gpio != NULL) {
+ imx6_pcie->host_wake_irq = gpiod_to_irq(host_wake_gpio);
+ ret = devm_request_threaded_irq(dev, imx6_pcie->host_wake_irq, NULL,
+ host_wake_irq_handler,
+ IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+ "host_wake", imx6_pcie);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to request host_wake_irq\n");
+
+ ret = device_init_wakeup(dev, true);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to init host_wake_irq\n");
+ }
}
return 0;
@@ -1466,6 +1521,11 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
{
struct imx6_pcie *imx6_pcie = platform_get_drvdata(pdev);
+ if (imx6_pcie->host_wake_irq >= 0) {
+ device_init_wakeup(&pdev->dev, false);
+ disable_irq(imx6_pcie->host_wake_irq);
+ }
+
/* bring down link, so bootloader gets clean state in case of reboot */
imx6_pcie_assert_core_reset(imx6_pcie);
}
--
2.34.1
Add wake-gpios property that can be used to wakeup the host
processor.
Signed-off-by: Sherry Sun <[email protected]>
Reviewed-by: Richard Zhu <[email protected]>
---
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 81bbb8728f0f..fba757d937e1 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -72,6 +72,12 @@ properties:
L=operation state) (optional required).
type: boolean
+ wake-gpios:
+ description: If present this property specifies WAKE# sideband signaling
+ to implement wakeup functionality. This is an input GPIO pin for the Root
+ Port mode here. Host drivers will wakeup the host using the IRQ
+ corresponding to the passed GPIO.
+
required:
- compatible
- reg
--
2.34.1
The host wake pin is a standard feature in the PCIe bus specification,
so we add this property under PCI dts node to enable the host wake
function.
Signed-off-by: Sherry Sun <[email protected]>
Reviewed-by: Richard Zhu <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index f87fa5a948cc..4378b9c1308c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -542,6 +542,7 @@ &pcie {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pcie0>;
reset-gpio = <&gpio2 7 GPIO_ACTIVE_LOW>;
+ wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
vpcie-supply = <®_pcie0>;
status = "okay";
};
@@ -772,6 +773,7 @@ pinctrl_pcie0: pcie0grp {
fsl,pins = <
MX8MP_IOMUXC_I2C4_SCL__PCIE_CLKREQ_B 0x60 /* open drain, pull up */
MX8MP_IOMUXC_SD1_DATA5__GPIO2_IO07 0x40
+ MX8MP_IOMUXC_I2C4_SDA__GPIO5_IO21 0x1c4
>;
};
--
2.34.1
The host wake pin is a standard feature in the PCIe bus specification,
so we add this property under PCI dts node to enable the host wake
function.
Signed-off-by: Sherry Sun <[email protected]>
Reviewed-by: Richard Zhu <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index 7507548cdb16..b8463ef230c5 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -367,6 +367,7 @@ &pcie1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_pcie1>;
reset-gpio = <&gpio5 12 GPIO_ACTIVE_LOW>;
+ wake-gpios = <&gpio5 11 GPIO_ACTIVE_LOW>;
clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
<&pcie0_refclk>,
<&clk IMX8MQ_CLK_PCIE2_PHY>,
@@ -545,6 +546,7 @@ pinctrl_pcie1: pcie1grp {
fsl,pins = <
MX8MQ_IOMUXC_I2C4_SDA_PCIE2_CLKREQ_B 0x76
MX8MQ_IOMUXC_ECSPI2_MISO_GPIO5_IO12 0x16
+ MX8MQ_IOMUXC_ECSPI2_MOSI_GPIO5_IO11 0x41
>;
};
--
2.34.1
Drop period at the end of subject line. It only adds the possibility
of unnecessary line wrapping in git log.
On Wed, Dec 13, 2023 at 05:28:47PM +0800, Sherry Sun wrote:
> Add pci host wakeup feature for imx platforms.
s/pci/PCI/
s/imx/i.MX/ (based on how nxp.com web pages refer to it)
> Example of configuring the corresponding dts property under the PCI
> node:
> wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
Add newline between paragraphs or wrap into a single paragraph.
> + /* host wakeup support */
> + imx6_pcie->host_wake_irq = -1;
AFAIK, 0 is an invalid IRQ value. So why not drop this assignment
since imx6_pcie->host_wake_irq is 0 by default since it was allocated
with devm_kzalloc(), and test like this elsewhere:
if (imx6_pcie->host_wake_irq) {
enable_irq_wake(imx6_pcie->host_wake_irq)
> + host_wake_gpio = devm_gpiod_get_optional(dev, "wake", GPIOD_IN);
> + if (IS_ERR(host_wake_gpio))
> + return PTR_ERR(host_wake_gpio);
> +
> + if (host_wake_gpio != NULL) {
if (host_wake_gpio)
Bjorn
Hi Sherry,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus shawnguo/for-next robh/for-next linus/master v6.7-rc5 next-20231213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sherry-Sun/PCI-imx6-Add-pci-host-wakeup-support-on-imx-platforms/20231213-173031
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20231213092850.1706042-2-sherry.sun%40nxp.com
patch subject: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20231214/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/pci/controller/dwc/pci-imx6.c:1267:13: warning: no previous prototype for 'host_wake_irq_handler' [-Wmissing-prototypes]
1267 | irqreturn_t host_wake_irq_handler(int irq, void *priv)
| ^~~~~~~~~~~~~~~~~~~~~
vim +/host_wake_irq_handler +1267 drivers/pci/controller/dwc/pci-imx6.c
1266
> 1267 irqreturn_t host_wake_irq_handler(int irq, void *priv)
1268 {
1269 struct imx6_pcie *imx6_pcie = priv;
1270 struct device *dev = imx6_pcie->pci->dev;
1271
1272 /* Notify PM core we are wakeup source */
1273 pm_wakeup_event(dev, 0);
1274 pm_system_wakeup();
1275
1276 return IRQ_HANDLED;
1277 }
1278
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
> -----Original Message-----
> From: Bjorn Helgaas <[email protected]>
> Sent: 2023??12??14?? 3:58
> To: Sherry Sun <[email protected]>
> Cc: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx <linux-
> [email protected]>; [email protected]; linux-arm-
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx
> platforms.
>
> Drop period at the end of subject line. It only adds the possibility of
> unnecessary line wrapping in git log.
Hi Bjorn, thanks for the comments, will do in V3.
>
> On Wed, Dec 13, 2023 at 05:28:47PM +0800, Sherry Sun wrote:
> > Add pci host wakeup feature for imx platforms.
>
> s/pci/PCI/
> s/imx/i.MX/ (based on how nxp.com web pages refer to it)
>
Will do.
> > Example of configuring the corresponding dts property under the PCI
> > node:
> > wake-gpios = <&gpio5 21 GPIO_ACTIVE_LOW>;
>
> Add newline between paragraphs or wrap into a single paragraph.
Will do.
>
> > + /* host wakeup support */
> > + imx6_pcie->host_wake_irq = -1;
>
> AFAIK, 0 is an invalid IRQ value. So why not drop this assignment since
> imx6_pcie->host_wake_irq is 0 by default since it was allocated with
> devm_kzalloc(), and test like this elsewhere:
>
> if (imx6_pcie->host_wake_irq) {
> enable_irq_wake(imx6_pcie->host_wake_irq)
I plan to change the host_wake_irq to unsigned int type, and add following codes, then "if (imx6_pcie->host_wake_irq)" condition seems more reasonable, let me know if you have any further suggestions. thanks!
- imx6_pcie->host_wake_irq = gpiod_to_irq(host_wake_gpio);
+ ret = gpiod_to_irq(host_wake_gpio);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to get IRQ for WAKE gpio\n");
+
+ imx6_pcie->host_wake_irq = (unsigned int)ret;
>
> > + host_wake_gpio = devm_gpiod_get_optional(dev, "wake",
> GPIOD_IN);
> > + if (IS_ERR(host_wake_gpio))
> > + return PTR_ERR(host_wake_gpio);
> > +
> > + if (host_wake_gpio != NULL) {
>
> if (host_wake_gpio)
Will do.
Best Regards
Sherry
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/[email protected]/
- 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
>
>
--
மணிவண்ணன் சதாசிவம்
Hi Sherry,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus shawnguo/for-next robh/for-next linus/master v6.7-rc5 next-20231214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sherry-Sun/PCI-imx6-Add-pci-host-wakeup-support-on-imx-platforms/20231213-173031
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20231213092850.1706042-2-sherry.sun%40nxp.com
patch subject: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
config: alpha-randconfig-r112-20231214 (https://download.01.org/0day-ci/archive/20231214/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231214/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
sparse warnings: (new ones prefixed by >>)
>> drivers/pci/controller/dwc/pci-imx6.c:1267:13: sparse: sparse: symbol 'host_wake_irq_handler' was not declared. Should it be static?
drivers/pci/controller/dwc/pci-imx6.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/umh.h, include/linux/kmod.h, ...):
include/linux/page-flags.h:242:46: sparse: sparse: self-comparison always evaluates to false
vim +/host_wake_irq_handler +1267 drivers/pci/controller/dwc/pci-imx6.c
1266
> 1267 irqreturn_t host_wake_irq_handler(int irq, void *priv)
1268 {
1269 struct imx6_pcie *imx6_pcie = priv;
1270 struct device *dev = imx6_pcie->pci->dev;
1271
1272 /* Notify PM core we are wakeup source */
1273 pm_wakeup_event(dev, 0);
1274 pm_system_wakeup();
1275
1276 return IRQ_HANDLED;
1277 }
1278
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
> -----Original Message-----
> From: Manivannan Sadhasivam <[email protected]>
> Sent: 2023年12月14日 13:13
> To: Sherry Sun <[email protected]>
> Cc: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx <linux-
> [email protected]>; [email protected]; linux-arm-
> [email protected]; [email protected]; linux-
> [email protected]
> 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
> >
> >
>
> --
> மணிவண்ணன் சதாசிவம்
Hi Sherry,
kernel test robot noticed the following build warnings:
[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus shawnguo/for-next robh/for-next linus/master v6.7-rc5 next-20231214]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sherry-Sun/PCI-imx6-Add-pci-host-wakeup-support-on-imx-platforms/20231213-173031
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20231213092850.1706042-2-sherry.sun%40nxp.com
patch subject: [PATCH V2 1/4] PCI: imx6: Add pci host wakeup support on imx platforms.
config: arm-randconfig-004-20231213 (https://download.01.org/0day-ci/archive/20231214/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231214/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/pci/controller/dwc/pci-imx6.c:1267:13: warning: no previous prototype for function 'host_wake_irq_handler' [-Wmissing-prototypes]
1267 | irqreturn_t host_wake_irq_handler(int irq, void *priv)
| ^
drivers/pci/controller/dwc/pci-imx6.c:1267:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
1267 | irqreturn_t host_wake_irq_handler(int irq, void *priv)
| ^
| static
1 warning generated.
vim +/host_wake_irq_handler +1267 drivers/pci/controller/dwc/pci-imx6.c
1266
> 1267 irqreturn_t host_wake_irq_handler(int irq, void *priv)
1268 {
1269 struct imx6_pcie *imx6_pcie = priv;
1270 struct device *dev = imx6_pcie->pci->dev;
1271
1272 /* Notify PM core we are wakeup source */
1273 pm_wakeup_event(dev, 0);
1274 pm_system_wakeup();
1275
1276 return IRQ_HANDLED;
1277 }
1278
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, Dec 14, 2023 at 10:03:51AM +0000, Sherry Sun wrote:
>
>
> > -----Original Message-----
> > From: Manivannan Sadhasivam <[email protected]>
> > Sent: 2023年12月14日 13:13
> > To: Sherry Sun <[email protected]>
> > Cc: Hongxing Zhu <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; dl-linux-imx <linux-
> > [email protected]>; [email protected]; linux-arm-
> > [email protected]; [email protected]; linux-
> > [email protected]
> > 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
> > >
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
--
மணிவண்ணன் சதாசிவம்