Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754175AbbDMNMI (ORCPT ); Mon, 13 Apr 2015 09:12:08 -0400 Received: from mail-pd0-f170.google.com ([209.85.192.170]:33333 "EHLO mail-pd0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752889AbbDMNME (ORCPT ); Mon, 13 Apr 2015 09:12:04 -0400 Message-ID: <552BC09D.9040502@samsung.com> Date: Mon, 13 Apr 2015 22:11:57 +0900 From: Krzysztof Kozlowski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Laurentiu Palcu , Sebastian Reichel , Krzysztof Kozlowski , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Dmitry Eremin-Solenikov , David Woodhouse CC: devicetree@vger.kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] power_supply: Add TI BQ24257 charger driver References: <1428671897-12232-1-git-send-email-laurentiu.palcu@intel.com> In-Reply-To: <1428671897-12232-1-git-send-email-laurentiu.palcu@intel.com> Content-Type: text/plain; charset=iso-8859-2; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4540 Lines: 149 W dniu 10.04.2015 o 22:18, Laurentiu Palcu pisze: > Based on the datasheet found here: > http://www.ti.com/lit/ds/symlink/bq24257.pdf > > Signed-off-by: Laurentiu Palcu (...) > diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c > new file mode 100644 > index 0000000..58b9af1 > --- /dev/null > +++ b/drivers/power/bq24257_charger.c > @@ -0,0 +1,868 @@ > +/* > + * TI BQ24257 charger driver > + * > + * Copyright (C) 2015 Intel Corporation > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > + > +#define BQ24257_REG_1 0x00 > +#define BQ24257_REG_2 0x01 > +#define BQ24257_REG_3 0x02 > +#define BQ24257_REG_4 0x03 > +#define BQ24257_REG_5 0x04 > +#define BQ24257_REG_6 0x05 > +#define BQ24257_REG_7 0x06 > + > +#define BQ24257_MANUFACTURER "Texas Instruments" > +#define BQ24257_STAT_IRQ "stat" > +#define BQ24257_PG_GPIO "pg" > + > +#define BQ24257_ILIM_SET_DELAY 1000 /* msec */ > + > +enum bq24257_fields { > + F_WD_FAULT, F_WD_EN, F_STAT, F_FAULT, /* REG 1 */ > + F_RESET, F_IILIMIT, F_EN_STAT, F_EN_TERM, F_CE, F_HZ_MODE, /* REG 2 */ > + F_VBAT, F_USB_DET, /* REG 3 */ > + F_ICHG, F_ITERM, /* REG 4 */ > + F_LOOP_STATUS, F_LOW_CHG, F_DPDM_EN, F_CE_STATUS, F_VINDPM, /* REG 5 */ > + F_X2_TMR_EN, F_TMR, F_SYSOFF, F_TS_STAT, /* REG 6 */ > + F_VOVP, F_CLR_VDP, F_FORCE_BATDET, F_FORCE_PTM, /* REG 7 */ > + > + F_MAX_FIELDS > +}; > + > +/* initial field values, converted from uV/uA */ > +struct bq24257_init_data { > + u8 ichg; /* charge current */ > + u8 vbat; /* regulation voltage */ > + u8 iterm; /* termination current */ > +}; > + > +struct bq24257_state { > + u8 status; > + u8 fault; > + bool power_good; > +}; > + > +struct bq24257_device { > + struct i2c_client *client; > + struct device *dev; > + struct power_supply *charger; > + struct power_supply_desc charger_desc; > + > + struct regmap *rmap; > + struct regmap_field *rmap_fields[F_MAX_FIELDS]; > + > + struct gpio_desc *pg; > + > + struct delayed_work iilimit_setup_work; > + > + struct bq24257_init_data init_data; > + struct bq24257_state state; > + > + struct mutex lock; /* protect state data */ > +}; > + > +static bool bq24257_is_writeable_reg(struct device *dev, unsigned int reg) > +{ > + return true; What is the benefit of such function always returning true? (...) > + > +static enum power_supply_property bq24257_power_supply_props[] = { > + POWER_SUPPLY_PROP_MANUFACTURER, > + POWER_SUPPLY_PROP_STATUS, > + POWER_SUPPLY_PROP_ONLINE, > + POWER_SUPPLY_PROP_HEALTH, > + POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT, > + POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX, > + POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE, > + POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX, > + POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT, > +}; > + > +static char *bq24257_charger_supplied_to[] = { > + "main-battery", > +}; > + > +static int bq24257_power_supply_init(struct bq24257_device *bq) > +{ > + struct power_supply_config psy_cfg = { .drv_data = bq, }; > + struct power_supply_desc *psy_desc = &bq->charger_desc; > + > + psy_desc->name = "bq24257-charger"; > + psy_desc->type = POWER_SUPPLY_TYPE_USB; > + psy_desc->properties = bq24257_power_supply_props; > + psy_desc->num_properties = ARRAY_SIZE(bq24257_power_supply_props); > + psy_desc->get_property = bq24257_power_supply_get_property; You are not modifying the power_supply_desc so it can be made a static const variable (file scope). Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/