All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
@ 2016-09-24  3:54 Axel Lin
  2016-09-26 23:21 ` Li, Aubrey
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Axel Lin @ 2016-09-24  3:54 UTC (permalink / raw)
  To: Darren Hart; +Cc: Zha Qipeng, Aubrey Li, platform-driver-x86, Axel Lin

Use platform_device_register_full() instead of open-coded.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/platform/x86/intel_pmc_ipc.c | 110 +++++++++++------------------------
 1 file changed, 33 insertions(+), 77 deletions(-)

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index b86e1bc..665a9a1 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -522,48 +522,36 @@ static struct resource telemetry_res[] = {
 static int ipc_create_punit_device(void)
 {
 	struct platform_device *pdev;
-	int ret;
-
-	pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
-	if (!pdev) {
-		dev_err(ipcdev.dev, "Failed to alloc punit platform device\n");
-		return -ENOMEM;
-	}
-
-	pdev->dev.parent = ipcdev.dev;
-	ret = platform_device_add_resources(pdev, punit_res_array,
-					    ARRAY_SIZE(punit_res_array));
-	if (ret) {
-		dev_err(ipcdev.dev, "Failed to add platform punit resources\n");
-		goto err;
-	}
+	const struct platform_device_info pdevinfo = {
+		.parent = ipcdev.dev,
+		.name = PUNIT_DEVICE_NAME,
+		.id = -1,
+		.res = punit_res_array,
+		.num_res = ARRAY_SIZE(punit_res_array),
+		};
+
+	pdev = platform_device_register_full(&pdevinfo);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
 
-	ret = platform_device_add(pdev);
-	if (ret) {
-		dev_err(ipcdev.dev, "Failed to add punit platform device\n");
-		goto err;
-	}
 	ipcdev.punit_dev = pdev;
 
 	return 0;
-err:
-	platform_device_put(pdev);
-	return ret;
 }
 
 static int ipc_create_tco_device(void)
 {
 	struct platform_device *pdev;
 	struct resource *res;
-	int ret;
-
-	pdev = platform_device_alloc(TCO_DEVICE_NAME, -1);
-	if (!pdev) {
-		dev_err(ipcdev.dev, "Failed to alloc tco platform device\n");
-		return -ENOMEM;
-	}
-
-	pdev->dev.parent = ipcdev.dev;
+	const struct platform_device_info pdevinfo = {
+		.parent = ipcdev.dev,
+		.name = TCO_DEVICE_NAME,
+		.id = -1,
+		.res = tco_res,
+		.num_res = ARRAY_SIZE(tco_res),
+		.data = &tco_info,
+		.size_data = sizeof(tco_info),
+		};
 
 	res = tco_res + TCO_RESOURCE_ACPI_IO;
 	res->start = ipcdev.acpi_io_base + TCO_BASE_OFFSET;
@@ -577,45 +565,26 @@ static int ipc_create_tco_device(void)
 	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
 	res->end = res->start + TCO_PMC_SIZE - 1;
 
-	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
-	if (ret) {
-		dev_err(ipcdev.dev, "Failed to add tco platform resources\n");
-		goto err;
-	}
-
-	ret = platform_device_add_data(pdev, &tco_info, sizeof(tco_info));
-	if (ret) {
-		dev_err(ipcdev.dev, "Failed to add tco platform data\n");
-		goto err;
-	}
+	pdev = platform_device_register_full(&pdevinfo);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
 
-	ret = platform_device_add(pdev);
-	if (ret) {
-		dev_err(ipcdev.dev, "Failed to add tco platform device\n");
-		goto err;
-	}
 	ipcdev.tco_dev = pdev;
 
 	return 0;
-err:
-	platform_device_put(pdev);
-	return ret;
 }
 
 static int ipc_create_telemetry_device(void)
 {
 	struct platform_device *pdev;
 	struct resource *res;
-	int ret;
-
-	pdev = platform_device_alloc(TELEMETRY_DEVICE_NAME, -1);
-	if (!pdev) {
-		dev_err(ipcdev.dev,
-			"Failed to allocate telemetry platform device\n");
-		return -ENOMEM;
-	}
-
-	pdev->dev.parent = ipcdev.dev;
+	const struct platform_device_info pdevinfo = {
+		.parent = ipcdev.dev,
+		.name = TELEMETRY_DEVICE_NAME,
+		.id = -1,
+		.res = telemetry_res,
+		.num_res = ARRAY_SIZE(telemetry_res),
+		};
 
 	res = telemetry_res + TELEMETRY_RESOURCE_PUNIT_SSRAM;
 	res->start = ipcdev.telem_punit_ssram_base;
@@ -625,26 +594,13 @@ static int ipc_create_telemetry_device(void)
 	res->start = ipcdev.telem_pmc_ssram_base;
 	res->end = res->start + ipcdev.telem_pmc_ssram_size - 1;
 
-	ret = platform_device_add_resources(pdev, telemetry_res,
-					    ARRAY_SIZE(telemetry_res));
-	if (ret) {
-		dev_err(ipcdev.dev,
-			"Failed to add telemetry platform resources\n");
-		goto err;
-	}
+	pdev = platform_device_register_full(&pdevinfo);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
 
-	ret = platform_device_add(pdev);
-	if (ret) {
-		dev_err(ipcdev.dev,
-			"Failed to add telemetry platform device\n");
-		goto err;
-	}
 	ipcdev.telemetry_dev = pdev;
 
 	return 0;
-err:
-	platform_device_put(pdev);
-	return ret;
 }
 
 static int ipc_create_pmc_devices(void)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
  2016-09-24  3:54 [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full Axel Lin
@ 2016-09-26 23:21 ` Li, Aubrey
  2016-09-27  0:24   ` Axel Lin
  2016-09-28 23:35 ` Darren Hart
  2016-09-28 23:36 ` Darren Hart
  2 siblings, 1 reply; 9+ messages in thread
From: Li, Aubrey @ 2016-09-26 23:21 UTC (permalink / raw)
  To: Axel Lin, Darren Hart; +Cc: Zha Qipeng, platform-driver-x86

On 2016/9/24 11:54, Axel Lin wrote:
> Use platform_device_register_full() instead of open-coded.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 110 +++++++++++------------------------
>  1 file changed, 33 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index b86e1bc..665a9a1 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -522,48 +522,36 @@ static struct resource telemetry_res[] = {
>  static int ipc_create_punit_device(void)
>  {
>  	struct platform_device *pdev;
> -	int ret;
> -
> -	pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev, "Failed to alloc punit platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> -	ret = platform_device_add_resources(pdev, punit_res_array,
> -					    ARRAY_SIZE(punit_res_array));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add platform punit resources\n");
> -		goto err;
> -	}
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = PUNIT_DEVICE_NAME,
> +		.id = -1,
> +		.res = punit_res_array,
> +		.num_res = ARRAY_SIZE(punit_res_array),
> +		};
> +

Is it better to move the structure out of the routine?

Thanks,
-Aubrey



> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add punit platform device\n");
> -		goto err;
> -	}
>  	ipcdev.punit_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_tco_device(void)
>  {
>  	struct platform_device *pdev;
>  	struct resource *res;
> -	int ret;
> -
> -	pdev = platform_device_alloc(TCO_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev, "Failed to alloc tco platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = TCO_DEVICE_NAME,
> +		.id = -1,
> +		.res = tco_res,
> +		.num_res = ARRAY_SIZE(tco_res),
> +		.data = &tco_info,
> +		.size_data = sizeof(tco_info),
> +		};
>  
>  	res = tco_res + TCO_RESOURCE_ACPI_IO;
>  	res->start = ipcdev.acpi_io_base + TCO_BASE_OFFSET;
> @@ -577,45 +565,26 @@ static int ipc_create_tco_device(void)
>  	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
>  	res->end = res->start + TCO_PMC_SIZE - 1;
>  
> -	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform resources\n");
> -		goto err;
> -	}
> -
> -	ret = platform_device_add_data(pdev, &tco_info, sizeof(tco_info));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform data\n");
> -		goto err;
> -	}
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform device\n");
> -		goto err;
> -	}
>  	ipcdev.tco_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_telemetry_device(void)
>  {
>  	struct platform_device *pdev;
>  	struct resource *res;
> -	int ret;
> -
> -	pdev = platform_device_alloc(TELEMETRY_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev,
> -			"Failed to allocate telemetry platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = TELEMETRY_DEVICE_NAME,
> +		.id = -1,
> +		.res = telemetry_res,
> +		.num_res = ARRAY_SIZE(telemetry_res),
> +		};
>  
>  	res = telemetry_res + TELEMETRY_RESOURCE_PUNIT_SSRAM;
>  	res->start = ipcdev.telem_punit_ssram_base;
> @@ -625,26 +594,13 @@ static int ipc_create_telemetry_device(void)
>  	res->start = ipcdev.telem_pmc_ssram_base;
>  	res->end = res->start + ipcdev.telem_pmc_ssram_size - 1;
>  
> -	ret = platform_device_add_resources(pdev, telemetry_res,
> -					    ARRAY_SIZE(telemetry_res));
> -	if (ret) {
> -		dev_err(ipcdev.dev,
> -			"Failed to add telemetry platform resources\n");
> -		goto err;
> -	}
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev,
> -			"Failed to add telemetry platform device\n");
> -		goto err;
> -	}
>  	ipcdev.telemetry_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_pmc_devices(void)
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
  2016-09-26 23:21 ` Li, Aubrey
@ 2016-09-27  0:24   ` Axel Lin
  0 siblings, 0 replies; 9+ messages in thread
From: Axel Lin @ 2016-09-27  0:24 UTC (permalink / raw)
  To: Li, Aubrey; +Cc: Darren Hart, Zha Qipeng, platform-driver-x86

2016-09-27 7:21 GMT+08:00 Li, Aubrey <aubrey.li@linux.intel.com>:
> On 2016/9/24 11:54, Axel Lin wrote:
>> Use platform_device_register_full() instead of open-coded.
>>
>> Signed-off-by: Axel Lin <axel.lin@ingics.com>
>> ---
>>  drivers/platform/x86/intel_pmc_ipc.c | 110 +++++++++++------------------------
>>  1 file changed, 33 insertions(+), 77 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
>> index b86e1bc..665a9a1 100644
>> --- a/drivers/platform/x86/intel_pmc_ipc.c
>> +++ b/drivers/platform/x86/intel_pmc_ipc.c
>> @@ -522,48 +522,36 @@ static struct resource telemetry_res[] = {
>>  static int ipc_create_punit_device(void)
>>  {
>>       struct platform_device *pdev;
>> -     int ret;
>> -
>> -     pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
>> -     if (!pdev) {
>> -             dev_err(ipcdev.dev, "Failed to alloc punit platform device\n");
>> -             return -ENOMEM;
>> -     }
>> -
>> -     pdev->dev.parent = ipcdev.dev;
>> -     ret = platform_device_add_resources(pdev, punit_res_array,
>> -                                         ARRAY_SIZE(punit_res_array));
>> -     if (ret) {
>> -             dev_err(ipcdev.dev, "Failed to add platform punit resources\n");
>> -             goto err;
>> -     }
>> +     const struct platform_device_info pdevinfo = {
>> +             .parent = ipcdev.dev,
>> +             .name = PUNIT_DEVICE_NAME,
>> +             .id = -1,
>> +             .res = punit_res_array,
>> +             .num_res = ARRAY_SIZE(punit_res_array),
>> +             };
>> +
>
> Is it better to move the structure out of the routine?

Either is fine.

The struct platform_device_info is not very big as most of it's fields are
pointers. And this variable is for setup only so it's fine to use
local variable,
we don't need it once the function is done.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
  2016-09-24  3:54 [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full Axel Lin
  2016-09-26 23:21 ` Li, Aubrey
@ 2016-09-28 23:35 ` Darren Hart
  2016-09-28 23:36 ` Darren Hart
  2 siblings, 0 replies; 9+ messages in thread
From: Darren Hart @ 2016-09-28 23:35 UTC (permalink / raw)
  To: Axel Lin; +Cc: Zha Qipeng, Aubrey Li, platform-driver-x86

On Sat, Sep 24, 2016 at 11:54:08AM +0800, Axel Lin wrote:
> Use platform_device_register_full() instead of open-coded.
> 

Thanks Axel. This looks good to me. My only comment would be that the change
eliminates all the dev_err calls which will reduce the debugability some. I've
queued this to testing, but Qipeng, please speak up if you have any objections.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
  2016-09-24  3:54 [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full Axel Lin
  2016-09-26 23:21 ` Li, Aubrey
  2016-09-28 23:35 ` Darren Hart
@ 2016-09-28 23:36 ` Darren Hart
  2016-09-29  0:59   ` Axel Lin
  2016-09-29  1:51     ` Zha, Qipeng
  2 siblings, 2 replies; 9+ messages in thread
From: Darren Hart @ 2016-09-28 23:36 UTC (permalink / raw)
  To: Axel Lin
  Cc: Zha Qipeng, Aubrey Li, platform-driver-x86, Linux Kernel Mailing List

+LKML

Axel, please always include LKML on any patch.

Qipeng, any concerns?

On Sat, Sep 24, 2016 at 11:54:08AM +0800, Axel Lin wrote:
> Use platform_device_register_full() instead of open-coded.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 110 +++++++++++------------------------
>  1 file changed, 33 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
> index b86e1bc..665a9a1 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -522,48 +522,36 @@ static struct resource telemetry_res[] = {
>  static int ipc_create_punit_device(void)
>  {
>  	struct platform_device *pdev;
> -	int ret;
> -
> -	pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev, "Failed to alloc punit platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> -	ret = platform_device_add_resources(pdev, punit_res_array,
> -					    ARRAY_SIZE(punit_res_array));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add platform punit resources\n");
> -		goto err;
> -	}
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = PUNIT_DEVICE_NAME,
> +		.id = -1,
> +		.res = punit_res_array,
> +		.num_res = ARRAY_SIZE(punit_res_array),
> +		};
> +
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add punit platform device\n");
> -		goto err;
> -	}
>  	ipcdev.punit_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_tco_device(void)
>  {
>  	struct platform_device *pdev;
>  	struct resource *res;
> -	int ret;
> -
> -	pdev = platform_device_alloc(TCO_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev, "Failed to alloc tco platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = TCO_DEVICE_NAME,
> +		.id = -1,
> +		.res = tco_res,
> +		.num_res = ARRAY_SIZE(tco_res),
> +		.data = &tco_info,
> +		.size_data = sizeof(tco_info),
> +		};
>  
>  	res = tco_res + TCO_RESOURCE_ACPI_IO;
>  	res->start = ipcdev.acpi_io_base + TCO_BASE_OFFSET;
> @@ -577,45 +565,26 @@ static int ipc_create_tco_device(void)
>  	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
>  	res->end = res->start + TCO_PMC_SIZE - 1;
>  
> -	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform resources\n");
> -		goto err;
> -	}
> -
> -	ret = platform_device_add_data(pdev, &tco_info, sizeof(tco_info));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform data\n");
> -		goto err;
> -	}
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform device\n");
> -		goto err;
> -	}
>  	ipcdev.tco_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_telemetry_device(void)
>  {
>  	struct platform_device *pdev;
>  	struct resource *res;
> -	int ret;
> -
> -	pdev = platform_device_alloc(TELEMETRY_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev,
> -			"Failed to allocate telemetry platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = TELEMETRY_DEVICE_NAME,
> +		.id = -1,
> +		.res = telemetry_res,
> +		.num_res = ARRAY_SIZE(telemetry_res),
> +		};
>  
>  	res = telemetry_res + TELEMETRY_RESOURCE_PUNIT_SSRAM;
>  	res->start = ipcdev.telem_punit_ssram_base;
> @@ -625,26 +594,13 @@ static int ipc_create_telemetry_device(void)
>  	res->start = ipcdev.telem_pmc_ssram_base;
>  	res->end = res->start + ipcdev.telem_pmc_ssram_size - 1;
>  
> -	ret = platform_device_add_resources(pdev, telemetry_res,
> -					    ARRAY_SIZE(telemetry_res));
> -	if (ret) {
> -		dev_err(ipcdev.dev,
> -			"Failed to add telemetry platform resources\n");
> -		goto err;
> -	}
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev,
> -			"Failed to add telemetry platform device\n");
> -		goto err;
> -	}
>  	ipcdev.telemetry_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_pmc_devices(void)
> -- 
> 2.7.4
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
  2016-09-28 23:36 ` Darren Hart
@ 2016-09-29  0:59   ` Axel Lin
  2016-09-29  1:15     ` Darren Hart
  2016-09-29  1:51     ` Zha, Qipeng
  1 sibling, 1 reply; 9+ messages in thread
From: Axel Lin @ 2016-09-29  0:59 UTC (permalink / raw)
  To: Darren Hart
  Cc: Zha Qipeng, Aubrey Li, platform-driver-x86, Linux Kernel Mailing List

2016-09-29 7:36 GMT+08:00 Darren Hart <dvhart@infradead.org>:
> +LKML
>
> Axel, please always include LKML on any patch.

I thought  +LKML is optional if you already have a dedicated
platform-driver-x86 maillist.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
  2016-09-29  0:59   ` Axel Lin
@ 2016-09-29  1:15     ` Darren Hart
  0 siblings, 0 replies; 9+ messages in thread
From: Darren Hart @ 2016-09-29  1:15 UTC (permalink / raw)
  To: Axel Lin
  Cc: Zha Qipeng, Aubrey Li, platform-driver-x86, Linux Kernel Mailing List

On Thu, Sep 29, 2016 at 08:59:55AM +0800, Axel Lin wrote:
> 2016-09-29 7:36 GMT+08:00 Darren Hart <dvhart@infradead.org>:
> > +LKML
> >
> > Axel, please always include LKML on any patch.
> 
> I thought  +LKML is optional if you already have a dedicated
> platform-driver-x86 maillist.

Nope, LKML is never optional. All patches to the Linux Kernel must include LKML.


-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
  2016-09-28 23:36 ` Darren Hart
@ 2016-09-29  1:51     ` Zha, Qipeng
  2016-09-29  1:51     ` Zha, Qipeng
  1 sibling, 0 replies; 9+ messages in thread
From: Zha, Qipeng @ 2016-09-29  1:51 UTC (permalink / raw)
  To: Darren Hart, Axel Lin
  Cc: Aubrey Li, platform-driver-x86, Linux Kernel Mailing List

It's good to me, thanks.

>+LKML
>
>Axel, please always include LKML on any patch.
>
>Qipeng, any concerns?

On Sat, Sep 24, 2016 at 11:54:08AM +0800, Axel Lin wrote:
> Use platform_device_register_full() instead of open-coded.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 110 
> +++++++++++------------------------
>  1 file changed, 33 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c 
> b/drivers/platform/x86/intel_pmc_ipc.c
> index b86e1bc..665a9a1 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -522,48 +522,36 @@ static struct resource telemetry_res[] = {  
> static int ipc_create_punit_device(void)  {
>  	struct platform_device *pdev;
> -	int ret;
> -
> -	pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev, "Failed to alloc punit platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> -	ret = platform_device_add_resources(pdev, punit_res_array,
> -					    ARRAY_SIZE(punit_res_array));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add platform punit resources\n");
> -		goto err;
> -	}
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = PUNIT_DEVICE_NAME,
> +		.id = -1,
> +		.res = punit_res_array,
> +		.num_res = ARRAY_SIZE(punit_res_array),
> +		};
> +
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add punit platform device\n");
> -		goto err;
> -	}
>  	ipcdev.punit_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_tco_device(void)  {
>  	struct platform_device *pdev;
>  	struct resource *res;
> -	int ret;
> -
> -	pdev = platform_device_alloc(TCO_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev, "Failed to alloc tco platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = TCO_DEVICE_NAME,
> +		.id = -1,
> +		.res = tco_res,
> +		.num_res = ARRAY_SIZE(tco_res),
> +		.data = &tco_info,
> +		.size_data = sizeof(tco_info),
> +		};
>  
>  	res = tco_res + TCO_RESOURCE_ACPI_IO;
>  	res->start = ipcdev.acpi_io_base + TCO_BASE_OFFSET; @@ -577,45 
> +565,26 @@ static int ipc_create_tco_device(void)
>  	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
>  	res->end = res->start + TCO_PMC_SIZE - 1;
>  
> -	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform resources\n");
> -		goto err;
> -	}
> -
> -	ret = platform_device_add_data(pdev, &tco_info, sizeof(tco_info));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform data\n");
> -		goto err;
> -	}
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform device\n");
> -		goto err;
> -	}
>  	ipcdev.tco_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_telemetry_device(void)  {
>  	struct platform_device *pdev;
>  	struct resource *res;
> -	int ret;
> -
> -	pdev = platform_device_alloc(TELEMETRY_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev,
> -			"Failed to allocate telemetry platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = TELEMETRY_DEVICE_NAME,
> +		.id = -1,
> +		.res = telemetry_res,
> +		.num_res = ARRAY_SIZE(telemetry_res),
> +		};
>  
>  	res = telemetry_res + TELEMETRY_RESOURCE_PUNIT_SSRAM;
>  	res->start = ipcdev.telem_punit_ssram_base; @@ -625,26 +594,13 @@ 
> static int ipc_create_telemetry_device(void)
>  	res->start = ipcdev.telem_pmc_ssram_base;
>  	res->end = res->start + ipcdev.telem_pmc_ssram_size - 1;
>  
> -	ret = platform_device_add_resources(pdev, telemetry_res,
> -					    ARRAY_SIZE(telemetry_res));
> -	if (ret) {
> -		dev_err(ipcdev.dev,
> -			"Failed to add telemetry platform resources\n");
> -		goto err;
> -	}
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev,
> -			"Failed to add telemetry platform device\n");
> -		goto err;
> -	}
>  	ipcdev.telemetry_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_pmc_devices(void)
> --
> 2.7.4
> 
> 

--
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full
@ 2016-09-29  1:51     ` Zha, Qipeng
  0 siblings, 0 replies; 9+ messages in thread
From: Zha, Qipeng @ 2016-09-29  1:51 UTC (permalink / raw)
  To: Darren Hart, Axel Lin
  Cc: Aubrey Li, platform-driver-x86, Linux Kernel Mailing List

It's good to me, thanks.

>+LKML
>
>Axel, please always include LKML on any patch.
>
>Qipeng, any concerns?

On Sat, Sep 24, 2016 at 11:54:08AM +0800, Axel Lin wrote:
> Use platform_device_register_full() instead of open-coded.
> 
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
>  drivers/platform/x86/intel_pmc_ipc.c | 110 
> +++++++++++------------------------
>  1 file changed, 33 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel_pmc_ipc.c 
> b/drivers/platform/x86/intel_pmc_ipc.c
> index b86e1bc..665a9a1 100644
> --- a/drivers/platform/x86/intel_pmc_ipc.c
> +++ b/drivers/platform/x86/intel_pmc_ipc.c
> @@ -522,48 +522,36 @@ static struct resource telemetry_res[] = {  
> static int ipc_create_punit_device(void)  {
>  	struct platform_device *pdev;
> -	int ret;
> -
> -	pdev = platform_device_alloc(PUNIT_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev, "Failed to alloc punit platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> -	ret = platform_device_add_resources(pdev, punit_res_array,
> -					    ARRAY_SIZE(punit_res_array));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add platform punit resources\n");
> -		goto err;
> -	}
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = PUNIT_DEVICE_NAME,
> +		.id = -1,
> +		.res = punit_res_array,
> +		.num_res = ARRAY_SIZE(punit_res_array),
> +		};
> +
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add punit platform device\n");
> -		goto err;
> -	}
>  	ipcdev.punit_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_tco_device(void)  {
>  	struct platform_device *pdev;
>  	struct resource *res;
> -	int ret;
> -
> -	pdev = platform_device_alloc(TCO_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev, "Failed to alloc tco platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = TCO_DEVICE_NAME,
> +		.id = -1,
> +		.res = tco_res,
> +		.num_res = ARRAY_SIZE(tco_res),
> +		.data = &tco_info,
> +		.size_data = sizeof(tco_info),
> +		};
>  
>  	res = tco_res + TCO_RESOURCE_ACPI_IO;
>  	res->start = ipcdev.acpi_io_base + TCO_BASE_OFFSET; @@ -577,45 
> +565,26 @@ static int ipc_create_tco_device(void)
>  	res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
>  	res->end = res->start + TCO_PMC_SIZE - 1;
>  
> -	ret = platform_device_add_resources(pdev, tco_res, ARRAY_SIZE(tco_res));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform resources\n");
> -		goto err;
> -	}
> -
> -	ret = platform_device_add_data(pdev, &tco_info, sizeof(tco_info));
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform data\n");
> -		goto err;
> -	}
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev, "Failed to add tco platform device\n");
> -		goto err;
> -	}
>  	ipcdev.tco_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_telemetry_device(void)  {
>  	struct platform_device *pdev;
>  	struct resource *res;
> -	int ret;
> -
> -	pdev = platform_device_alloc(TELEMETRY_DEVICE_NAME, -1);
> -	if (!pdev) {
> -		dev_err(ipcdev.dev,
> -			"Failed to allocate telemetry platform device\n");
> -		return -ENOMEM;
> -	}
> -
> -	pdev->dev.parent = ipcdev.dev;
> +	const struct platform_device_info pdevinfo = {
> +		.parent = ipcdev.dev,
> +		.name = TELEMETRY_DEVICE_NAME,
> +		.id = -1,
> +		.res = telemetry_res,
> +		.num_res = ARRAY_SIZE(telemetry_res),
> +		};
>  
>  	res = telemetry_res + TELEMETRY_RESOURCE_PUNIT_SSRAM;
>  	res->start = ipcdev.telem_punit_ssram_base; @@ -625,26 +594,13 @@ 
> static int ipc_create_telemetry_device(void)
>  	res->start = ipcdev.telem_pmc_ssram_base;
>  	res->end = res->start + ipcdev.telem_pmc_ssram_size - 1;
>  
> -	ret = platform_device_add_resources(pdev, telemetry_res,
> -					    ARRAY_SIZE(telemetry_res));
> -	if (ret) {
> -		dev_err(ipcdev.dev,
> -			"Failed to add telemetry platform resources\n");
> -		goto err;
> -	}
> +	pdev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
>  
> -	ret = platform_device_add(pdev);
> -	if (ret) {
> -		dev_err(ipcdev.dev,
> -			"Failed to add telemetry platform device\n");
> -		goto err;
> -	}
>  	ipcdev.telemetry_dev = pdev;
>  
>  	return 0;
> -err:
> -	platform_device_put(pdev);
> -	return ret;
>  }
>  
>  static int ipc_create_pmc_devices(void)
> --
> 2.7.4
> 
> 

--
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-09-29  1:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-24  3:54 [PATCH RFT] platform/x86: intel_pmc_ipc: Convert to use platform_device_register_full Axel Lin
2016-09-26 23:21 ` Li, Aubrey
2016-09-27  0:24   ` Axel Lin
2016-09-28 23:35 ` Darren Hart
2016-09-28 23:36 ` Darren Hart
2016-09-29  0:59   ` Axel Lin
2016-09-29  1:15     ` Darren Hart
2016-09-29  1:51   ` Zha, Qipeng
2016-09-29  1:51     ` Zha, Qipeng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.