On Mon, Sep 09, 2013 at 02:50:22PM +0100, Mark Brown wrote: > On Mon, Sep 09, 2013 at 04:34:43AM -0700, Guenter Roeck wrote: > > On 09/09/2013 04:12 AM, Mark Brown wrote: > > >On Mon, Sep 09, 2013 at 06:29:11PM +0800, Wei Ni wrote: > > > >This doesn't look good, it is going to ignore actual errors - I *really* > > >doubt that vcc is optional, it looks like it's the main power supply for > > >the device. You should use normal regulator_get(), _optional() is for > > >supplies which could physically not be provided in a system (eg, if the > > >device can generate them internally if required). > > > Then he'll have to make sure that all devicetree files in the system > > contain references to this regulator. > > Or get the patches applied on top of the code that'll be going in this > cycle implementing get_optional() properly - when that's done the > default will be to provide a dummy supply for regulator_get(). If you > ack the patch I'd be happy to carry it. > Jean will have to ack it. > > >Also do you really need 25ms after power on? > > > I had not noticed, but I recommend to reject the patch because of it. > > If we add 25ms delay to each driver, booting the system will take as > > long as booting windows. If enabling the regulator needs time, the > > regulator subsystem should do it. > > And indeed it does this (well, it does whatever the driver says in terms > of delay). However it is possible that the lm90 needs this time for > itself - if it's doing some sort of initialisation or callibration > sequence then that'll happen after the supplies come up. 25ms did seem > rather long, especially for such simple devices, but it's not beyond the > bounds of possibility. Even then it would be unreasonable to enforce such a delay for every instance of this driver, even if the regulator is a dummy one and/or has been enabled already. Many if not almost all users of the driver work just fine as-is, without additional enforced delay (and, for that matter, without regulator ;). If there is a delay, it would have to be optional: Only wait if the regulator was really turned on by the call and not already active. I don't know if the regulator subsystem has this capability; if not, maybe it should. On a higher level, I wonder if such functionality should be added in the i2c subsystem and not in i2c client drivers. Has anyone thought about this ? Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
