[02/16] media: ti-vpe: cal: use runtime_resume for errata handling
Commit Message
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
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),
> },
> };
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
@@ -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),
},
};