[v2,2/3,media] atmel-isi: convert the pdata from pointer to structure

Message ID 1395744087-5753-3-git-send-email-josh.wu@atmel.com (mailing list archive)
State Accepted, archived
Delegated to: Guennadi Liakhovetski
Headers

Commit Message

Josh Wu March 25, 2014, 10:41 a.m. UTC
  Now the platform data is initialized by allocation of isi
structure. In the future, we use pdata to store the dt parameters.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
v1 --> v2:
 no change.

 drivers/media/platform/soc_camera/atmel-isi.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)
  

Comments

Guennadi Liakhovetski May 18, 2014, 8:59 p.m. UTC | #1
Hi Josh,

I'm still waiting for an update of Ben's patches to then also apply yours, 
but I decided to have a look at yours now to see if I find anything, that 
might be worth changing. A small note to this one below.

On Tue, 25 Mar 2014, Josh Wu wrote:

> Now the platform data is initialized by allocation of isi
> structure. In the future, we use pdata to store the dt parameters.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> ---
> v1 --> v2:
>  no change.
> 
>  drivers/media/platform/soc_camera/atmel-isi.c |   22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
> index 9d977c5..f4add0a 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c

[snip]

> @@ -912,7 +912,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
>  	if (IS_ERR(isi->pclk))
>  		return PTR_ERR(isi->pclk);
>  
> -	isi->pdata = pdata;
> +	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));

I think it'd be better to use

+	memcpy(&isi->pdata, pdata, sizeof(isi->pdata));

This way if the type of the pdata changes at any time in the future this 
line will not have to be changed. If you don't mind I can make this change 
myself, so you don't have to make a new version just for this.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Josh Wu May 23, 2014, 3:15 a.m. UTC | #2
Hi, Guennadi

On 5/19/2014 4:59 AM, Guennadi Liakhovetski wrote:
> Hi Josh,
>
> I'm still waiting for an update of Ben's patches to then also apply yours,
> but I decided to have a look at yours now to see if I find anything, that
> might be worth changing. A small note to this one below.
>
> On Tue, 25 Mar 2014, Josh Wu wrote:
>
>> Now the platform data is initialized by allocation of isi
>> structure. In the future, we use pdata to store the dt parameters.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> ---
>> v1 --> v2:
>>   no change.
>>
>>   drivers/media/platform/soc_camera/atmel-isi.c |   22 +++++++++++-----------
>>   1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
>> index 9d977c5..f4add0a 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> [snip]
>
>> @@ -912,7 +912,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
>>   	if (IS_ERR(isi->pclk))
>>   		return PTR_ERR(isi->pclk);
>>   
>> -	isi->pdata = pdata;
>> +	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
> I think it'd be better to use
>
> +	memcpy(&isi->pdata, pdata, sizeof(isi->pdata));
>
> This way if the type of the pdata changes at any time in the future this
> line will not have to be changed. If you don't mind I can make this change
> myself, so you don't have to make a new version just for this.

Thanks for pointing it out.  I think I will sent out a new version of 
patch (include bus-width parsing) then I will included with this fix.

Best Regards,
Josh Wu

>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 9d977c5..f4add0a 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -84,7 +84,7 @@  struct atmel_isi {
 	struct clk			*mck;
 	unsigned int			irq;
 
-	struct isi_platform_data	*pdata;
+	struct isi_platform_data	pdata;
 	u16				width_flags;	/* max 12 bits */
 
 	struct list_head		video_buffer_list;
@@ -350,7 +350,7 @@  static void start_dma(struct atmel_isi *isi, struct frame_buffer *buffer)
 
 	cfg1 &= ~ISI_CFG1_FRATE_DIV_MASK;
 	/* Enable linked list */
-	cfg1 |= isi->pdata->frate | ISI_CFG1_DISCR;
+	cfg1 |= isi->pdata.frate | ISI_CFG1_DISCR;
 
 	/* Enable codec path and ISI */
 	ctrl = ISI_CTRL_CDC | ISI_CTRL_EN;
@@ -797,7 +797,7 @@  static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 	/* Make choises, based on platform preferences */
 	if ((common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) &&
 	    (common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)) {
-		if (isi->pdata->hsync_act_low)
+		if (isi->pdata.hsync_act_low)
 			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_HIGH;
 		else
 			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_LOW;
@@ -805,7 +805,7 @@  static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 
 	if ((common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) &&
 	    (common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) {
-		if (isi->pdata->vsync_act_low)
+		if (isi->pdata.vsync_act_low)
 			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_HIGH;
 		else
 			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_LOW;
@@ -813,7 +813,7 @@  static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 
 	if ((common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING) &&
 	    (common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)) {
-		if (isi->pdata->pclk_act_falling)
+		if (isi->pdata.pclk_act_falling)
 			common_flags &= ~V4L2_MBUS_PCLK_SAMPLE_RISING;
 		else
 			common_flags &= ~V4L2_MBUS_PCLK_SAMPLE_FALLING;
@@ -835,9 +835,9 @@  static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 	if (common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
 		cfg1 |= ISI_CFG1_PIXCLK_POL_ACTIVE_FALLING;
 
-	if (isi->pdata->has_emb_sync)
+	if (isi->pdata.has_emb_sync)
 		cfg1 |= ISI_CFG1_EMB_SYNC;
-	if (isi->pdata->full_mode)
+	if (isi->pdata.full_mode)
 		cfg1 |= ISI_CFG1_FULL_MODE;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -912,7 +912,7 @@  static int atmel_isi_probe(struct platform_device *pdev)
 	if (IS_ERR(isi->pclk))
 		return PTR_ERR(isi->pclk);
 
-	isi->pdata = pdata;
+	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
 	isi->active = NULL;
 	spin_lock_init(&isi->lock);
 	INIT_LIST_HEAD(&isi->video_buffer_list);
@@ -928,7 +928,7 @@  static int atmel_isi_probe(struct platform_device *pdev)
 		/* Set ISI_MCK's frequency, it should be faster than pixel
 		 * clock.
 		 */
-		ret = clk_set_rate(isi->mck, pdata->mck_hz);
+		ret = clk_set_rate(isi->mck, isi->pdata.mck_hz);
 		if (ret < 0)
 			return ret;
 	}
@@ -962,9 +962,9 @@  static int atmel_isi_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
-	if (pdata->data_width_flags & ISI_DATAWIDTH_8)
+	if (isi->pdata.data_width_flags & ISI_DATAWIDTH_8)
 		isi->width_flags = 1 << 7;
-	if (pdata->data_width_flags & ISI_DATAWIDTH_10)
+	if (isi->pdata.data_width_flags & ISI_DATAWIDTH_10)
 		isi->width_flags |= 1 << 9;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);