Sebastian
On 10/20/19 7:15 AM, Sebastian Reichel wrote:
Hi Dan,
On Mon, Sep 30, 2019 at 09:31:37AM -0500, Dan Murphy wrote:
[...]
+
+static int bq2515x_power_supply_register(struct bq2515x_device *bq2515x)
+{
+ struct power_supply_config psy_cfg = { .drv_data = bq2515x, };
+ int ret = -EINVAL;
+
+ bq2515x->mains = devm_power_supply_register(bq2515x->dev,
+ &bq2515x_mains_desc,
+ &psy_cfg);
+ if (IS_ERR(bq2515x->mains))
+ return ret;
+
+ bq2515x->battery = devm_power_supply_register(bq2515x->dev,
+ &bq2515x_battery_desc,
+ &psy_cfg);
+ if (IS_ERR(bq2515x->battery)) {
+ power_supply_unregister(bq2515x->mains);
you registered the mains power-supply with devm_ prefix, so it
will be removed automatically.
Ack
+ return ret;
+ }
+
+ return 0;
+}
+
+static int bq2515x_hw_init(struct bq2515x_device *bq2515x)
+{
+ int ret = 0;
+
+ if (bq2515x->init_data.ichg)
+ ret = bq2515x_set_ilim_lvl(bq2515x, bq2515x->init_data.ichg);
+
+ if (bq2515x->init_data.vreg)
+ ret = bq2515x_set_batt_reg(bq2515x, bq2515x->init_data.vreg);
This throws away potential error code from bq2515x_set_ilim_lvl().
Ack
+ return ret;
+}
+
+static int bq2515x_read_properties(struct bq2515x_device *bq2515x)
+{
+ int ret;
+
+ ret = device_property_read_u8(bq2515x->dev, "ti,charge-current",
+ &bq2515x->init_data.ichg);
+ if (ret)
+ goto fail;
+
+ ret = device_property_read_u8(bq2515x->dev,
+ "ti,battery-regulation-voltage",
+ &bq2515x->init_data.vreg);
+ if (ret)
+ goto fail;
The above properties are marked as optional in the DT bindings
document.
ACK. Will just set them to the data sheet default values
+ bq2515x->pg_gpio = devm_gpiod_get_optional(bq2515x->dev,
+ "pg", GPIOD_IN);
+ if (IS_ERR(bq2515x->pg_gpio))
+ dev_info(bq2515x->dev, "PG GPIO not defined");
+
+ bq2515x->reset_gpio = devm_gpiod_get_optional(bq2515x->dev,
+ "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(bq2515x->reset_gpio))
+ dev_info(bq2515x->dev, "reset GPIO not defined");
+
+ bq2515x->lp_gpio = devm_gpiod_get_optional(bq2515x->dev, "low-power",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(bq2515x->lp_gpio))
+ dev_info(bq2515x->dev, "LP GPIO not defined");
+
+ bq2515x->ce_gpio = devm_gpiod_get_optional(bq2515x->dev,
+ "charge-enable",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(bq2515x->ce_gpio))
+ dev_info(bq2515x->dev, "Charge enable GPIO not defined");
The GPIO errors should not be ignored, especially since you
might get a -EPROBE_DEFER.
ACK
+fail:
+ return ret;
+}
+
+static const struct regmap_config bq2515x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = BQ2515X_DEVICE_ID,
+ .reg_defaults = bq2515x_reg_defs,
+ .num_reg_defaults = ARRAY_SIZE(bq2515x_reg_defs),
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int bq2515x_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct bq2515x_device *bq;
+ int ret;
+
+ bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
+ if (!bq)
+ return -ENOMEM;
+
+ bq->client = client;
+ bq->dev = dev;
+
+ mutex_init(&bq->lock);
+
+ bq->regmap = devm_regmap_init_i2c(client, &bq2515x_regmap_config);
+ if (IS_ERR(bq->regmap)) {
+ dev_err(dev, "failed to allocate register map\n");
+ return PTR_ERR(bq->regmap);
+ }
+
+ strncpy(bq->model_name, id->name, I2C_NAME_SIZE);
+
+ i2c_set_clientdata(client, bq);
+
+ ret = bq2515x_read_properties(bq);
+ if (ret < 0) {
+ dev_err(dev, "Failed to register power supply\n");
+ return ret;
+ }
+
+ ret = bq2515x_hw_init(bq);
+ if (ret < 0) {
+ dev_err(dev, "Cannot initialize the chip.\n");
+ return ret;
+ }
+
+ return bq2515x_power_supply_register(bq);
+}
+
+static const struct i2c_device_id bq2515x_i2c_ids[] = {
+ { "bq25150", 0 },
+ { "bq25155", 1 },
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, bq2515x_i2c_ids);
+
+static const struct of_device_id bq2515x_of_match[] = {
+ { .compatible = "ti,bq25150", },
+ { .compatible = "ti,bq25155", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, bq2515x_of_match);
+
+static struct i2c_driver bq2515x_driver = {
+ .driver = {
+ .name = "bq2515x-charger",
+ .of_match_table = of_match_ptr(bq2515x_of_match),
You need to do one of those:
* remove of_match_ptr and specify bq2515x_of_match directly
* put the OF table in #ifdef CONFIG_OF
* mark the OF table as __maybe_unused
Probably just do the first one.
Dan
