lkml.org 
[lkml]   [2019]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 3/3] power: supply: bq2515x: Introduce the bq2515x family
    From
    Date
    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

    \
     
     \ /
      Last update: 2019-10-23 21:03    [W:5.169 / U:0.652 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site