Messages in this thread | | | From | Danny Huang <> | Date | Fri, 5 Aug 2011 19:51:15 +0800 | Subject | RE: [PATCH] regulator: tps6586x: add SMx slew rate setting |
| |
Thanks for the feedback. The reason for using driver_data is that I can't find a proper field in regulator_init_data for the slew rate setting. I'll do some correction based on the feedback and try to add a new field for the slew rate setting.
-----Original Message----- From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com] Sent: Thursday, August 04, 2011 20:13 To: Danny Huang Cc: lrg@ti.com; mike@compulab.co.il; sameo@linux.intel.com; Xin Xie; gking@nvidia.com; linux-kernel@vger.kernel.org Subject: Re: [PATCH] regulator: tps6586x: add SMx slew rate setting
On Thu, Aug 04, 2011 at 07:34:22PM +0800, dahuang@nvidia.com wrote: > From: Danny Huang <dahuang@nvidia.com> > > Add output vlotage slew rate setting for SM0/SM1 > > From: Xin Xie <xxie@nvidia.com>
Looks like you messed up here, I rather suspect Xin Xie rather than you should be the author?
> +static int tps6586x_regulator_set_slew_rate(struct platform_device > +*pdev) { > + struct device *parent = pdev->dev.parent; > + struct regulator_init_data *p = pdev->dev.platform_data; > + struct tps6586x_settings *setting = p->driver_data;
If this is system configured data (which is what one would expect for this) it should be coming in as platform data not driver data - what's happened here?
> + default: > + dev_err(&pdev->dev, "invalid regulator ID\n"); > + return -EINVAL; > + }
Should say what data is invalid here, otherwise it's not going to be at all obvious what's invalid.
> +enum { > + TPS6586x_SLEW_RATE_INSTANTLY, > + TPS6586x_SLEW_RATE_110UV, > + TPS6586x_SLEW_RATE_220UV, > + TPS6586x_SLEW_RATE_440UV, > + TPS6586x_SLEW_RATE_880UV, > + TPS6586x_SLEW_RATE_1760UV, > + TPS6586x_SLEW_RATE_3520UV, > + TPS6586x_SLEW_RATE_7040UV, > +};
If the values are being written directly to the chip you should probably explicitly specify the values that are being set.
| |