On Thu, Jan 5, 2017 at 6:34 PM, Matt Ranostay <matt@ranostay.consulting> wrote:
> On Thu, Jan 5, 2017 at 5:53 PM, Sebastian Reichel <sre@xxxxxxxxxx> wrote:
>> On Wed, Jan 04, 2017 at 06:10:07PM -0800, Matt Ranostay wrote:
>>> Initial support for access and modification of the non-volatile regions
>>> of the bq27425 fuel gauge DesignEnergy, DesignCapacity, and
>>> TerminateVoltage settings.
>>>
>>> This is intended for fine tuning the fuel gauge state machine for the
>>> respective battery specifications.
>>>
>>> Cc: Sebastian Reichel <sre@xxxxxxxxxx>
>>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>>> ---
>>> drivers/power/supply/bq27xxx_battery_i2c.c | 335 +++++++++++++++++++++++++++++
>>> include/linux/power/bq27xxx_battery.h | 4 +
>>> 2 files changed, 339 insertions(+)
>>
>> I only skipped over this one, as the changed DT binding requires
>> quite some changes in this patch anyways. Here are a couple of
>> comments how I would like to see this implemented:
>>
>> Add a patch for the power-supply core, which implements a
>> structure for the battery info. Something like this:
>>
>> struct power_supply_battery_info {
>> uint32 energy; /* µWh */
>> uint32 power; /* µAh */
>> uint32 nominal_voltage; /* µV */
>> /* ... */
>> };
>>
>> And a function in the core framework, which gets the information
>> from DT. The function itself should *not* be DT specific, so that
>> ACPI/platformdata/whatever support can be added later without
>> modifying every single driver.
>>
>> static struct power_supply_battery_info*
>> power_supply_get_battery_info(struct power_supply *psy) {
>> if (psy->dt) {
>> /* get battery phandle or return -ENXIO */
>> /* fill and return struct */
>> }
>>
>> return -ENOTSUP;
>> }
>>
>> Then call power_supply_get_battery_info() during bq27xxx probe and
>> use the struct to initialize the registers. Also I expect that
>> functionality in bq27xxx_battery.c instead of bq27xxx_battery_i2c.c,
>> since it's not I²C specific.
>
> They may not be i2c specific but they are only used by the i2c path of
> the code currently. Do you think that platform data would ever have a
> struct to pass with the respective data?
>
> Also it would have to be in bq27xxx_battery_setup() and not
> bq27xxx_battery_platform_probe() since the former is only thing called
> by both code paths.
Also there is no bq27xxx_battery_platform_write to even write the
configuration data.
>
>
>>
>> -- Sebastian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
