On Thu, May 03, 2012 at 02:04:26PM +0800, Neil Zhang wrote: > Add a virtual PMIC in order to supply the regulators that pn544 needed. *Always* CC relevant maintainers and mailing lists on patches. you've sent this to some Marvell maintainers and the USB list... > Change-Id: I28ea959d760c8f264ce968df77b087d554140181 Don't include stuff like this in mainline submissions, it means nothing outside of your private gerrit install. > Signed-off-by: Neil Zhang <zhangwm@xxxxxxxxxxx> > +++ b/drivers/regulator/vpmic.c > @@ -0,0 +1,135 @@ > +/* > + * Virtual Regulators driver for PN544 I'm having a hard time understanding what the goal of this driver is. If this is virtual why is it specific to PN544? How does it differ from any of the existing virtual regulators and when would one wish to use it? > +static struct regulator_ops vpmic_regulator_ops = { > + .enable = vpmic_enable, > + .disable = vpmic_disable, > +}; It seems very odd to simulate enable and disable without is_enabled(). Though I'd *really* expect the consumer to be able to cope without being able to enable and disable if this is useful at all. > +static struct vpmic_regulator_info vpmic_info[] = { > + VPMIC_DVC(Vdd_IO), > + VPMIC_DVC(VBat), > + VPMIC_DVC(VSim), > +}; So, you've got a specific set of regulators here... why not just register fixed voltage regulators - that's the idiomatic thing to do if you've got uncontrollable regulaors (I'd assume the supplies actually exist in hardware...). -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html