[02/16] media: ti-vpe: cal: use runtime_resume for errata handling

Message ID 20200313114121.32182-2-tomi.valkeinen@ti.com (mailing list archive)
State Changes Requested, archived
Delegated to: Hans Verkuil
Headers
Series [01/16] media: ti-vpe: cal: fix use of wrong macro |

Commit Message

Tomi Valkeinen March 13, 2020, 11:41 a.m. UTC
  We need to do errata handling every time CAL is being enabled. The code
is currently in cal_runtime_get(), which is not the correct place for
it.

Move the code to cal_runtime_resume, which is called every time CAL is
enabled.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/media/platform/ti-vpe/cal.c | 36 ++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 14 deletions(-)
  

Comments

Laurent Pinchart March 16, 2020, 12:28 p.m. UTC | #1
Hi Tomi,

Thank you for the patch.

On Fri, Mar 13, 2020 at 01:41:07PM +0200, Tomi Valkeinen wrote:
> We need to do errata handling every time CAL is being enabled. The code
> is currently in cal_runtime_get(), which is not the correct place for
> it.
> 
> Move the code to cal_runtime_resume, which is called every time CAL is
> enabled.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/media/platform/ti-vpe/cal.c | 36 ++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index 4b584c419e98..b4a9f4d16ce4 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -653,20 +653,7 @@ static void i913_errata(struct cal_dev *dev, unsigned int port)
>  
>  static int cal_runtime_get(struct cal_dev *dev)
>  {
> -	int r;
> -
> -	r = pm_runtime_get_sync(&dev->pdev->dev);
> -
> -	if (dev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
> -		/*
> -		 * Apply errata on both port eveytime we (re-)enable
> -		 * the clock
> -		 */
> -		i913_errata(dev, 0);
> -		i913_errata(dev, 1);
> -	}
> -
> -	return r;
> +	return pm_runtime_get_sync(&dev->pdev->dev);
>  }
>  
>  static inline void cal_runtime_put(struct cal_dev *dev)
> @@ -2397,11 +2384,32 @@ static const struct of_device_id cal_of_match[] = {
>  MODULE_DEVICE_TABLE(of, cal_of_match);
>  #endif
>  
> +static int cal_runtime_resume(struct device *dev)
> +{
> +	struct cal_dev *caldev = dev_get_drvdata(dev);
> +
> +	if (caldev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
> +		/*
> +		 * Apply errata on both port eveytime we (re-)enable

s/eveytime/everytime/

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Now that cal_runtime_get() and cal_runtime_put() are just wrappers
around pm_runtime_get_sync() and pm_runtime_put_sync(), how about
dropping the wrappers ?

> +		 * the clock
> +		 */
> +		i913_errata(caldev, 0);
> +		i913_errata(caldev, 1);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops cal_pm_ops = {
> +	.runtime_resume = cal_runtime_resume,
> +};
> +
>  static struct platform_driver cal_pdrv = {
>  	.probe		= cal_probe,
>  	.remove		= cal_remove,
>  	.driver		= {
>  		.name	= CAL_MODULE_NAME,
> +		.pm	= &cal_pm_ops,
>  		.of_match_table = of_match_ptr(cal_of_match),
>  	},
>  };
  
Tomi Valkeinen March 17, 2020, 8:54 a.m. UTC | #2
On 16/03/2020 14:28, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Fri, Mar 13, 2020 at 01:41:07PM +0200, Tomi Valkeinen wrote:
>> We need to do errata handling every time CAL is being enabled. The code
>> is currently in cal_runtime_get(), which is not the correct place for
>> it.
>>
>> Move the code to cal_runtime_resume, which is called every time CAL is
>> enabled.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>   drivers/media/platform/ti-vpe/cal.c | 36 ++++++++++++++++++-----------
>>   1 file changed, 22 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
>> index 4b584c419e98..b4a9f4d16ce4 100644
>> --- a/drivers/media/platform/ti-vpe/cal.c
>> +++ b/drivers/media/platform/ti-vpe/cal.c
>> @@ -653,20 +653,7 @@ static void i913_errata(struct cal_dev *dev, unsigned int port)
>>   
>>   static int cal_runtime_get(struct cal_dev *dev)
>>   {
>> -	int r;
>> -
>> -	r = pm_runtime_get_sync(&dev->pdev->dev);
>> -
>> -	if (dev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
>> -		/*
>> -		 * Apply errata on both port eveytime we (re-)enable
>> -		 * the clock
>> -		 */
>> -		i913_errata(dev, 0);
>> -		i913_errata(dev, 1);
>> -	}
>> -
>> -	return r;
>> +	return pm_runtime_get_sync(&dev->pdev->dev);
>>   }
>>   
>>   static inline void cal_runtime_put(struct cal_dev *dev)
>> @@ -2397,11 +2384,32 @@ static const struct of_device_id cal_of_match[] = {
>>   MODULE_DEVICE_TABLE(of, cal_of_match);
>>   #endif
>>   
>> +static int cal_runtime_resume(struct device *dev)
>> +{
>> +	struct cal_dev *caldev = dev_get_drvdata(dev);
>> +
>> +	if (caldev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
>> +		/*
>> +		 * Apply errata on both port eveytime we (re-)enable
> 
> s/eveytime/everytime/
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Now that cal_runtime_get() and cal_runtime_put() are just wrappers
> around pm_runtime_get_sync() and pm_runtime_put_sync(), how about
> dropping the wrappers ?

Yep, I'll add a patch for that.

  Tomi
  

Patch

diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 4b584c419e98..b4a9f4d16ce4 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -653,20 +653,7 @@  static void i913_errata(struct cal_dev *dev, unsigned int port)
 
 static int cal_runtime_get(struct cal_dev *dev)
 {
-	int r;
-
-	r = pm_runtime_get_sync(&dev->pdev->dev);
-
-	if (dev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
-		/*
-		 * Apply errata on both port eveytime we (re-)enable
-		 * the clock
-		 */
-		i913_errata(dev, 0);
-		i913_errata(dev, 1);
-	}
-
-	return r;
+	return pm_runtime_get_sync(&dev->pdev->dev);
 }
 
 static inline void cal_runtime_put(struct cal_dev *dev)
@@ -2397,11 +2384,32 @@  static const struct of_device_id cal_of_match[] = {
 MODULE_DEVICE_TABLE(of, cal_of_match);
 #endif
 
+static int cal_runtime_resume(struct device *dev)
+{
+	struct cal_dev *caldev = dev_get_drvdata(dev);
+
+	if (caldev->flags & DRA72_CAL_PRE_ES2_LDO_DISABLE) {
+		/*
+		 * Apply errata on both port eveytime we (re-)enable
+		 * the clock
+		 */
+		i913_errata(caldev, 0);
+		i913_errata(caldev, 1);
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops cal_pm_ops = {
+	.runtime_resume = cal_runtime_resume,
+};
+
 static struct platform_driver cal_pdrv = {
 	.probe		= cal_probe,
 	.remove		= cal_remove,
 	.driver		= {
 		.name	= CAL_MODULE_NAME,
+		.pm	= &cal_pm_ops,
 		.of_match_table = of_match_ptr(cal_of_match),
 	},
 };