On Wed, Dec 18, 2013 at 04:24:52PM +0000, Mark Brown wrote:
> On Wed, Dec 18, 2013 at 03:50:07PM +0100, Markus Pargmann wrote:
> 
> > @@ -771,7 +794,7 @@ static struct regulator_ops tps65910_ops = {
> >     .get_voltage_sel        = tps65910_get_voltage_sel,
> >     .set_voltage_sel        = tps65910_set_voltage_sel,
> >     .list_voltage           = regulator_list_voltage_table,
> > -   .map_voltage            = regulator_map_voltage_ascend,
> > +   .map_voltage            = regulator_map_voltage_iterate,
> >  };
> 
> You should make separate ops for this rather than make all the other
> regulators take the performance hit.

Fixed.

> 
> >  static struct regulator_ops tps65911_ops = {
> > @@ -944,6 +967,7 @@ static struct of_regulator_match tps65910_matches[] = {
> >     { .name = "vaux2",      .driver_data = (void *) &tps65910_regs[10] },
> >     { .name = "vaux33",     .driver_data = (void *) &tps65910_regs[11] },
> >     { .name = "vmmc",       .driver_data = (void *) &tps65910_regs[12] },
> > +   { .name = "vbb",        .driver_data = (void *) &tps65910_regs[13] },
> >  };
> 
> Ugh, these numbered tables aren't good.  Not a problem from this patch
> though.
> 
> > -           pmic->desc[i].enable_mask = TPS65910_SUPPLY_STATE_ENABLED;
> > +           if (tps65910_chip_id(tps65910) == TPS65910 &&
> > +                           i == TPS65910_REG_VBB)
> > +                   pmic->desc[i].enable_mask = BBCH_BBCHEN_MASK;
> > +           else
> > +                   pmic->desc[i].enable_mask = 
> > TPS65910_SUPPLY_STATE_ENABLED;
> 
> switch statements please - it means if additional things need
> customising they can drop right in.

Fixed.


Thank you,

Markus


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to