Re: [PATCH] mfd: add bq2415x charger driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux