On Thu, Jan 13, 2011 at 4:48 PM, Premi, Sanjeev <premi@xxxxxx> wrote:
>> -----Original Message-----
>> From: G, Manjunath Kondaiah
>> Sent: Thursday, January 13, 2011 2:58 PM
>> To: Premi, Sanjeev
>> Cc: linux-omap@xxxxxxxxxxxxxxx
>> Subject: Re: [PATCHv2] omap3: Add basic support for 720MHz part
>>
>
> [....]
>
>> >> goto err;
>> >> }
>> >>
>> >> Do you see any issues?
>> >
>> > Though it is not considered in this patch, but in the devices that
>> > don't contain iva, the code related to enabling the iva freq can
>> > be easily excluded in current flow.
>>
>> In this case, the function returns error if we have device with no
>> iva. The API should
>> enable only mpu opp and return sane value for devices with no iva.
>>
>
> [sp] Don't want to get into details of next patch here, but when there
> is no IVA, the "if" condition for for iva won't/shouldn't be entered.
exactly
>
> Values returned should just be the status opp_enable. OR am I missing
> something in your comment.
> e.g.
> pdev = oh_mpu->od->pdev;
> r = opp_enable(&pdev->dev, 720000000);
> if (r < 0) {
> dev_err(&pdev->dev, "Unable to enable OPP for mpu.\n");
> goto err;
> }
>
> if omap3_has_iva()) {
> oh_iva = omap_hwmod_lookup("iva"); //move from current impl.
This looks fine and it can translate into:
if (!oh_mpu || !oh_mpu->od)
goto err_nodevice;
pdev = oh_mpu->od->pdev;
r = opp_enable(&pdev->dev, 720000000);
if (r < 0)
goto err_oppenable;
if (!omap3_has_iva())
goto err_nodevice;
r = -ENODEV;
if(!oh_iva || !oh_iva->od)
goto err_nodevice;
pdev = oh_iva->od->pdev;
r = opp_enable(&pdev->dev, 520000000);
if (r < 0)
goto err_oppenable;
return r;
err_oppenable:
dev_err(&pdev->dev, "Unable to enable OPP\n");
err_nodevice:
return r;
-Manjunath
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
