> -----Original Message----- > From: Mark Brown [mailto:broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx] > Sent: 2012年5月4日 0:51 > To: Neil Zhang > Cc: haojian.zhuang@xxxxxxxxx; eric.y.miao@xxxxxxxxx; Chao Xie; linux- > usb@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] regulator: add VPMIC driver for NFC test > > 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...). This patch is submitted by mistake, please ignore it. Anyway, thanks for your comments. Best Regards, Neil Zhang ?韬{.n?????%??檩??w?{.n???{炳???骅w*jg????????G??⒏⒎?:+v????????????"??????