> -----Original Message----- > From: G, Manjunath Kondaiah [mailto:manjugk@xxxxxx] > Sent: Thursday, January 13, 2011 5:16 PM > To: Premi, Sanjeev > Cc: linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [PATCHv2] omap3: Add basic support for 720MHz part > > 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; Not having an IVA is not error! and goto's really aren't saving anything. > > 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
