Re: [PATCH 1/2] intel_pmc_ipc: Fix GCR register base address and length

Re: [PATCH 1/2] intel_pmc_ipc: Fix GCR register base address and length

[Andy Shevchenko]

On Thu, 2016-02-18 at 02:03 +0800, Qipeng Zha wrote:
> GCR register (pmc_cfg register) is at offset 0x1008, and
> remapping of 0x4 bytes is enough.
> 

Darren, both patches are fine by me.

> Signed-off-by: Francois-Nicolas Muller <francois-
> nicolas.muller@intel.com>
> Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>
> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c
> b/drivers/platform/x86/intel_pmc_ipc.c
> index ca69135..abdf67c 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -67,7 +67,8 @@
>  /* exported resources from IFWI */
>  #define PLAT_RESOURCE_IPC_INDEX		0
>  #define PLAT_RESOURCE_IPC_SIZE		0x1000
> -#define PLAT_RESOURCE_GCR_SIZE		0x1000
> +#define PLAT_RESOURCE_GCR_OFFSET	0x1008
> +#define PLAT_RESOURCE_GCR_SIZE		0x4
>  #define PLAT_RESOURCE_BIOS_DATA_INDEX	1
>  #define PLAT_RESOURCE_BIOS_IFACE_INDEX	2
>  #define PLAT_RESOURCE_ISP_DATA_INDEX	4
> @@ -688,7 +689,7 @@ static int ipc_plat_get_res(struct
> platform_device *pdev)
>  	}
>  	ipcdev.ipc_base = addr;
>  
> -	ipcdev.gcr_base = res->start + size;
> +	ipcdev.gcr_base = res->start + PLAT_RESOURCE_GCR_OFFSET;
>  	ipcdev.gcr_size = PLAT_RESOURCE_GCR_SIZE;
>  	dev_info(&pdev->dev, "ipc res: %pR\n", res);
>  

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[PATCH 1/2] intel_pmc_ipc: Fix GCR register base address and length

[Qipeng Zha]

GCR register (pmc_cfg register) is at offset 0x1008, and
remapping of 0x4 bytes is enough.

Signed-off-by: Francois-Nicolas Muller <francois-nicolas.muller@intel.com>
Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>
---
 drivers/platform/x86/intel_pmc_ipc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index ca69135..abdf67c 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -67,7 +67,8 @@
 /* exported resources from IFWI */
 #define PLAT_RESOURCE_IPC_INDEX		0
 #define PLAT_RESOURCE_IPC_SIZE		0x1000
-#define PLAT_RESOURCE_GCR_SIZE		0x1000
+#define PLAT_RESOURCE_GCR_OFFSET	0x1008
+#define PLAT_RESOURCE_GCR_SIZE		0x4
 #define PLAT_RESOURCE_BIOS_DATA_INDEX	1
 #define PLAT_RESOURCE_BIOS_IFACE_INDEX	2
 #define PLAT_RESOURCE_ISP_DATA_INDEX	4
@@ -688,7 +689,7 @@ static int ipc_plat_get_res(struct platform_device *pdev)
 	}
 	ipcdev.ipc_base = addr;
 
-	ipcdev.gcr_base = res->start + size;
+	ipcdev.gcr_base = res->start + PLAT_RESOURCE_GCR_OFFSET;
 	ipcdev.gcr_size = PLAT_RESOURCE_GCR_SIZE;
 	dev_info(&pdev->dev, "ipc res: %pR\n", res);
 
-- 
1.8.3.2

[PATCH 2/2] intel_pmc_ipc: avoid pending IPC1 command during legacy suspend

[Qipeng Zha]

During legacy suspend flow, IPC1 commands are being requested
from opregion driver. But the PMC_IPC1 command will timeout as example:

[  281.444600] ipc_debug##: ipc_send_command: cmd=0x201ff,
[  281.444648] wbuf[0]=0x4ea6
[  281.444668] wbuf[1]=0x0
[  281.444674] wbuf[2]=0x0
[  281.444676] wbuf[3]=0x0
[  284.446467] pmc-ipc-plat INT34D2:00: IPC timed out, TS=0x4, CMD=0x200ff

This is because before the opregion driver could send IPC1 commands,
the PMC_IPC irq is already suspended. Which makes the IPC command to
timeout.

Solution: register pmc_ipc irq as IRQF_NO_SUSPEND

Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>
---
 drivers/platform/x86/intel_pmc_ipc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index abdf67c..140aa77 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -731,7 +731,8 @@ static int ipc_plat_probe(struct platform_device *pdev)
 		goto err_device;
 	}
 
-	if (request_irq(ipcdev.irq, ioc, 0, "intel_pmc_ipc", &ipcdev)) {
+	if (request_irq(ipcdev.irq, ioc, IRQF_NO_SUSPEND,
+			"intel_pmc_ipc", &ipcdev)) {
 		dev_err(&pdev->dev, "Failed to request irq\n");
 		ret = -EBUSY;
 		goto err_irq;
-- 
1.8.3.2

Re: [PATCH 2/2] intel_pmc_ipc: avoid pending IPC1 command during legacy suspend

[Darren Hart]

On Thu, Feb 18, 2016 at 02:03:38AM +0800, Qipeng Zha wrote:
> During legacy suspend flow, IPC1 commands are being requested
> from opregion driver. But the PMC_IPC1 command will timeout as example:
> 
> [  281.444600] ipc_debug##: ipc_send_command: cmd=0x201ff,
> [  281.444648] wbuf[0]=0x4ea6
> [  281.444668] wbuf[1]=0x0
> [  281.444674] wbuf[2]=0x0
> [  281.444676] wbuf[3]=0x0
> [  284.446467] pmc-ipc-plat INT34D2:00: IPC timed out, TS=0x4, CMD=0x200ff
> 
> This is because before the opregion driver could send IPC1 commands,
> the PMC_IPC irq is already suspended. Which makes the IPC command to
> timeout.
> 
> Solution: register pmc_ipc irq as IRQF_NO_SUSPEND
> 
> Signed-off-by: Ananth Krishna R <ananth.krishna.r@intel.com>
> Signed-off-by: Bharath K Veera <bharath.k.veera@intel.com>
> Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>

Qipeng, who wrote this patch? This is the one I need a Signed-off-by from. Feel
free to credit the others with concept, debug, testing in the commit message,
but the Signed-off-by indicates authorship and the path the patch took to
mainline.

> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index abdf67c..140aa77 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -731,7 +731,8 @@ static int ipc_plat_probe(struct platform_device *pdev)
>  		goto err_device;
>  	}
>  
> -	if (request_irq(ipcdev.irq, ioc, 0, "intel_pmc_ipc", &ipcdev)) {
> +	if (request_irq(ipcdev.irq, ioc, IRQF_NO_SUSPEND,
> +			"intel_pmc_ipc", &ipcdev)) {
>  		dev_err(&pdev->dev, "Failed to request irq\n");
>  		ret = -EBUSY;
>  		goto err_irq;
> -- 
> 1.8.3.2
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/2] intel_pmc_ipc: Fix GCR register base address and length

[Darren Hart]

On Thu, Feb 18, 2016 at 02:03:37AM +0800, Qipeng Zha wrote:
> GCR register (pmc_cfg register) is at offset 0x1008, and
> remapping of 0x4 bytes is enough.
> 
> Signed-off-by: Francois-Nicolas Muller <francois-nicolas.muller@intel.com>
> Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>

Who wrote the patch?

> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index ca69135..abdf67c 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -67,7 +67,8 @@
>  /* exported resources from IFWI */
>  #define PLAT_RESOURCE_IPC_INDEX		0
>  #define PLAT_RESOURCE_IPC_SIZE		0x1000
> -#define PLAT_RESOURCE_GCR_SIZE		0x1000
> +#define PLAT_RESOURCE_GCR_OFFSET	0x1008
> +#define PLAT_RESOURCE_GCR_SIZE		0x4
>  #define PLAT_RESOURCE_BIOS_DATA_INDEX	1
>  #define PLAT_RESOURCE_BIOS_IFACE_INDEX	2
>  #define PLAT_RESOURCE_ISP_DATA_INDEX	4
> @@ -688,7 +689,7 @@ static int ipc_plat_get_res(struct platform_device *pdev)
>  	}
>  	ipcdev.ipc_base = addr;
>  
> -	ipcdev.gcr_base = res->start + size;
> +	ipcdev.gcr_base = res->start + PLAT_RESOURCE_GCR_OFFSET;
>  	ipcdev.gcr_size = PLAT_RESOURCE_GCR_SIZE;
>  	dev_info(&pdev->dev, "ipc res: %pR\n", res);
>  
> -- 
> 1.8.3.2
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

RE: [PATCH 1/2] intel_pmc_ipc: Fix GCR register base address and length

[Zha, Qipeng]

> 
> Signed-off-by: Francois-Nicolas Muller 
> <francois-nicolas.muller@intel.com>
> Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>

> Who wrote the patch?
Darren, Francois-Nicolas wrote this patch whicch merged in internal kernel.


> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c 
> b/drivers/platform/x86/intel_pmc_ipc.c
> index ca69135..abdf67c 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -67,7 +67,8 @@
>  /* exported resources from IFWI */
>  #define PLAT_RESOURCE_IPC_INDEX		0
>  #define PLAT_RESOURCE_IPC_SIZE		0x1000
> -#define PLAT_RESOURCE_GCR_SIZE		0x1000
> +#define PLAT_RESOURCE_GCR_OFFSET	0x1008
> +#define PLAT_RESOURCE_GCR_SIZE		0x4
>  #define PLAT_RESOURCE_BIOS_DATA_INDEX	1
>  #define PLAT_RESOURCE_BIOS_IFACE_INDEX	2
>  #define PLAT_RESOURCE_ISP_DATA_INDEX	4
> @@ -688,7 +689,7 @@ static int ipc_plat_get_res(struct platform_device *pdev)
>  	}
>  	ipcdev.ipc_base = addr;
>  
> -	ipcdev.gcr_base = res->start + size;
> +	ipcdev.gcr_base = res->start + PLAT_RESOURCE_GCR_OFFSET;
>  	ipcdev.gcr_size = PLAT_RESOURCE_GCR_SIZE;
>  	dev_info(&pdev->dev, "ipc res: %pR\n", res);
>  
> --
> 1.8.3.2
> 
> 

--
Darren Hart
Intel Open Source Technology Center

Re: [PATCH 1/2] intel_pmc_ipc: Fix GCR register base address and length

[Darren Hart]

On Thu, Feb 18, 2016 at 06:19:35AM +0000, Zha, Qipeng wrote:
> > 
> > Signed-off-by: Francois-Nicolas Muller 
> > <francois-nicolas.muller@intel.com>
> > Signed-off-by: Qipeng Zha <qipeng.zha@intel.com>
> 
> > Who wrote the patch?
> Darren, Francois-Nicolas wrote this patch whicch merged in internal kernel.

Thanks Qipeng. If the author isn't submitting, please give me a head's up in the
cover letter (PATCH 0/X) in future submissions.

-- 
Darren Hart
Intel Open Source Technology Center