Messages in this thread | | | Date | Wed, 15 Jan 2014 14:20:57 +0400 | From | Vladimir Barinov <> | Subject | Re: [PATCH 1/3] power_supply: modelgauge_battery: Maxim ModelGauge ICs gauge |
| |
Hello, Krzysztof,
Thank you for the review.
On 01/14/2014 08:31 PM, Krzysztof Kozlowski wrote: > On 01/09/2014 05:49 PM, Vladimir Barinov wrote: >> Add Maxim ModelGauge ICs gauge driver for >> MAX17040/41/43/44/48/49/58/59 chips >> >> Signed-off-by: Vladimir Barinov >> <vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> >> >> --- >> drivers/power/Kconfig | 8 >> drivers/power/Makefile | 1 >> drivers/power/modelgauge_battery.c | 875 >> +++++++++++++++++++++++ >> include/linux/platform_data/battery-modelgauge.h | 44 + >> 4 files changed, 928 insertions(+) >> > > [...] > >> +static int modelgauge_read_reg(struct i2c_client *client, u8 reg, >> u16 *value) >> +{ >> + int ret = i2c_smbus_read_word_data(client, reg); >> + >> + if (ret < 0) { >> + dev_err(&client->dev, "%s: err %d\n", __func__, ret); >> + return ret; >> + } >> + >> + *value = be16_to_cpu(ret); >> + return 0; >> +} > > Have you considered using regmap for accessing registers? I know that > other max17* drivers don't use it but it may be this could be the time > to switch to regmap API? > > [...] Sure, I will rework for the next try. Thx for pointing to this. > >> +static int modelgauge_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) >> +{ >> + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); >> + struct modelgauge_priv *priv; >> + int ret; >> + >> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) >> + return -EIO; >> + >> + priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + if (client->dev.of_node) >> + priv->pdata = modelgauge_parse_dt(&client->dev); >> + else >> + priv->pdata = client->dev.platform_data; >> + >> + priv->client = client; >> + priv->chip = id->driver_data; >> + >> + i2c_set_clientdata(client, priv); >> + >> + priv->battery.name = "modelgauge_battery"; >> + priv->battery.type = POWER_SUPPLY_TYPE_BATTERY; >> + priv->battery.get_property = modelgauge_get_property; >> + priv->battery.properties = modelgauge_battery_props; >> + priv->battery.num_properties = >> ARRAY_SIZE(modelgauge_battery_props); >> + >> + INIT_WORK(&priv->load_work, modelgauge_load_model_work); >> + INIT_DELAYED_WORK(&priv->rcomp_work, modelgauge_update_rcomp_work); >> + >> + ret = modelgauge_init(priv); >> + if (ret) >> + return ret; >> + >> + ret = power_supply_register(&client->dev, &priv->battery); >> + if (ret) { >> + dev_err(&client->dev, "failed: power supply register\n"); >> + goto err_supply; >> + } >> + >> + if (client->irq) { >> + switch (priv->chip) { >> + case ID_MAX17040: >> + case ID_MAX17041: >> + dev_err(&client->dev, "alert line is not supported\n"); >> + ret = -EINVAL; >> + goto err_irq; >> + default: >> + ret = request_threaded_irq(client->irq, NULL, >> + modelgauge_irq_handler, >> + IRQF_TRIGGER_FALLING, >> + priv->battery.name, priv); > > I think you may use devm_request_threaded_irq() here. Ok, thx, > > Best regards, > Krzysztof
Regards, Vladimir
| |