Hi Paul,
On 2011/06/24 14:34, Paul Mundt wrote:
> On Wed, Jun 22, 2011 at 04:49:48PM +0900, Damian Hobson-Garcia wrote:
>> +static int sh_mobile_meram_pm_get_sync(struct sh_mobile_meram_info *pdata)
>> +{
>> + if (!pdata || !pdata->pdev)
>> + return -EINVAL;
>> +
>> + dev_dbg(&pdata->pdev->dev, "Enabling sh_mobile_meram clock.");
>> + pm_runtime_get_sync(&pdata->pdev->dev);
>> + return 0;
>> +}
>> +
>
> On Wed, Jun 22, 2011 at 04:49:49PM +0900, Damian Hobson-Garcia wrote:
>> @@ -259,6 +259,11 @@ static void sh_mobile_lcdc_clk_on(struct sh_mobile_lcdc_priv *priv)
>> pm_runtime_get_sync(priv->dev);
>> if (priv->dot_clk)
>> clk_enable(priv->dot_clk);
>> + if (priv->meram_dev && priv->meram_dev->ops) {
>> + struct sh_mobile_meram_info *mdev;
>> + mdev = priv->meram_dev;
>> + mdev->ops->meram_pm_get_sync(mdev);
>> + }
>> }
>> }
>>
> I'm not sure that I really see the point in the callbacks. The callbacks
> would make sense in the case where you're dealing with opaque types that
> you don't wish to have knowledge of in the other drivers, but when all
> you're doing is fetching the pointer and wrapping verbatim in to the
> runtime pm calls, it just seems like a pointless layer of indirection.
>
> You could easily just do this as:
>
> if (priv->meram_dev)
> pm_runtime_get_sync(&priv->meram_dev->pdev->dev);
>
> and be done with it.
>
> This will also save you from having to add additional callbacks should
> you decide that you suddenly require async behaviour or so in other
> cases, too.
Thanks for your comment.
You raise a good point here. I'll get rid of the useless call.
Damian
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
