RE: [NEW DRIVER V6 1/7] drivers/mfd: DA9058 MFD core driver

[Opensource [Anthony Olech]]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@xxxxxxxxxx]
> Sent: 22 April 2013 14:34
> To: Opensource [Anthony Olech]
> Cc: Samuel Ortiz; Arnd Bergmann; Mauro Carvalho Chehab; Steven Toth;
> Michael Krufky; LKML; David Dajun Chen
> Subject: Re: [NEW DRIVER V6 1/7] drivers/mfd: DA9058 MFD core driver
> 
> On Fri, Apr 19, 2013 at 05:56:24PM +0100, Anthony Olech wrote:
> 
> > +static struct da9058_regulator_pdata buck1_pdata = {
> > +	.init.constraints.name = "DA9058_BUCK1",
> > +	.regulator_id = DA9058_BUCK_1,
> > +	.init.constraints.min_uV = DA9058_BUCK12_VOLT_LOWER * 1000,
> > +	.init.constraints.max_uV = DA9058_BUCK12_VOLT_UPPER * 1000,
> > +	.control_voltage_step = DA9058_BUCK_VOLT_STEP * 1000,
> > +	.control_register = DA9058_BUCK1_REG,
> > +	.control_step_mask = DA9058_MAX_VSEL,
> > +	.control_enable_mask = DA9058_BUCK_LDO_EN,
> > +	.ramp_register = DA9058_SUPPLY_REG,
> > +	.ramp_enable_mask = DA9058_SUPPLY_VBUCK1GO,
> > +	.init.constraints.valid_ops_mask = REGULATOR_CHANGE_VOLTAGE |
> > +			REGULATOR_CHANGE_MODE |
> REGULATOR_CHANGE_STATUS,
> > +	.init.constraints.valid_modes_mask = REGULATOR_MODE_NORMAL,
> > +	.init.constraints.boot_on = 1,
> 
> No, you're totally missing the point here.  Your regulator driver needs
> to be generic and work on any machine using the device.  This means you
> can't specify any constraints, the constraints need to come from the
> system integration.
> 
[...]

thanks Mark for your helpful comments.

As far as I can see there are two types of constraints:-
1) the absolute max/min that the PMIC itself is designed for.
2) the max/min that the board is designed for, ie system integration.

Thus constraints type (1) must come from the driver and constraints type (2)
must come from the machine driver, which for my testing is the mach-smdk6410.

Also, I believe that two further principles apply:
A) the machine driver should only specify a constraint if it restricts a value from (1) above.
B) sensible defaults should be specified in the driver so that a system should have some
   chance of operating in the case that the machine driver provides no constraints/overrides.

Thus, in the case of this PMIC, the big BUCK is designed to provide power to the processor,
so the only possible sensible default can be "boot_on == 1". It then makes sense that if a
PMIC is to be used in the situation it is designed for, then the number of system integration 
constraints should be almost zero.

>From your comment it seems that I have missed a point, but what?

Tony Olech

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/