On Tue, Dec 6, 2011 at 1:31 PM, Mark Brown
<broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, Dec 06, 2011 at 12:35:41AM +0200, Felipe Contreras wrote:
>
>> --- a/arch/arm/mach-omap2/board-rx51-peripherals.c
>> +++ b/arch/arm/mach-omap2/board-rx51-peripherals.c
>> @@ -925,6 +925,9 @@ static struct i2c_board_info __initdata rx51_peripherals_i2c_board_info_2[] = {
>> I2C_BOARD_INFO("bq27200", 0x55),
>> },
>> #endif
>> + {
>> + I2C_BOARD_INFO("bq24150", 0x6b),
>> + },
>
> Clearly this is orthogonal.
Yes, I added it just to demonstrate how to activate it.
>> +static inline int bq2415x_i2c_read(struct i2c_client *cli, u8 reg)
>> +{
>> + return i2c_smbus_read_byte_data(cli, reg);
>> +}
>> +
>> +static inline int bq2415x_i2c_write(struct i2c_client *cli, u8 reg, u8 val)
>> +{
>> + return i2c_smbus_write_byte_data(cli, reg, val);
>> +}
>
> regmap might be useful here (it's got an update bits operation and
> cache).
Will take a look.
>> +static int bq2415x_set_current_limit(struct i2c_client *cli,
>> + int min_uA, int max_uA)
>> +{
>> + int res;
>> +
>> + res = bq2415x_i2c_read(cli, BQ2415X_GEN_CTL);
>> + if (res < 0)
>> + return res;
>> +
>> + res &= ~BQ2415X_IIN_LIMIT_UNLIM_MASK;
>> +
>> + if (min_uA >= BQ2415X_IIN_LIMIT_100 && max_uA < BQ2415X_IIN_LIMIT_500)
>> + ;
>> + else if (min_uA >= BQ2415X_IIN_LIMIT_500 && max_uA < BQ2415X_IIN_LIMIT_800)
>> + res |= BQ2415X_IIN_LIMIT_500_MASK;
>> + else if (min_uA >= BQ2415X_IIN_LIMIT_800 && max_uA < BQ2415X_IIN_LIMIT_UNLIM)
>> + res |= BQ2415X_IIN_LIMIT_800_MASK;
>> + else if (min_uA >= BQ2415X_IIN_LIMIT_UNLIM)
>> + res |= BQ2415X_IIN_LIMIT_UNLIM_MASK;
>> + else
>> + return -EINVAL;
>> +
>> + return bq2415x_i2c_write(cli, BQ2415X_GEN_CTL, res);
>> +}
>
> This is the sort of stuff that people were pushing through the regulator
> API (and you have cloned the interface...) in order to allow a separate
> bit of code to pick the current limits. At the minute it looks like
> you're hard coding the settings, the regulator API would at least let
> you punt those to machines with fixed configurations and provides a hook
> for anything which does want to play with the configuration at runtime.
> Don't know if there's a better API, but it does seem like this is a
> general thing for chargers and should therefore go through a generic
> API.
Yes, for this particular case the regulator API might be useful, but I
don't see how external code will use this. Will they have to search
for the name of this regulator, and then try to change the
current_limit?
Anyway, there are other levels I'm not sure the regulator interface is
good for, like a "weak battery" voltage threshold, that most likely
would only make sense to handle internally in the driver.
> On the other hand if you just set limits and let the charger get on with
> its thing and run autonomously starting, stopping and fast charging by
> itself then a power supply driver seems like a good fit - just provide
> the upper limits as platform data or something and watch it go.
It would have to change its behavior depending on external events,
like charger plugged/unplugged, different types of chargers, and so
on. I'm thinking the rx51 board code could join some hooks from
isp1704 (which detects the events) into this driver.
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
