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@xxxxxxxxxxxxxxxx>
---
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
--
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
