On Mon, Nov 26, 2012 at 02:19:07PM +0100, Terje Bergstrom wrote: > + > +struct nvhost_chip_support *nvhost_chip_ops; should be static? > +static int __devinit nvhost_alloc_resources(struct nvhost_master *host) > +{ > + int err; > + > + err = nvhost_init_chip_support(host); > + if (err) > + return err; > + > + return 0; nit: why not just return err - the 'if(err)' is unnecessary)? > + > + nvhost = host; I think this should be delayed until the init is complete as this variable is not cleared if there is a failure during init. Also I feel that the name nvhost is a bit short for an exported variable. > +static void to_state_running_locked(struct platform_device *dev) > +{ > + struct nvhost_device_data *pdata = platform_get_drvdata(dev); > + int prev_state = pdata->powerstate; > + > + if (pdata->powerstate == NVHOST_POWER_STATE_POWERGATED) > + to_state_clockgated_locked(dev); > + > + if (pdata->powerstate == NVHOST_POWER_STATE_CLOCKGATED) { > + int i; > + > + if (dev->dev.parent) > + nvhost_module_busy(to_platform_device(dev->dev.parent)); > + > + for (i = 0; i < pdata->num_clks; i++) { > + int err = clk_prepare_enable(pdata->clk[i]); > + if (err) { > + dev_err(&dev->dev, "Cannot turn on clock %s", > + pdata->clocks[i].name); > + return; In case of an error, returning here leaves some clocks turned on. Sivaram _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel