Messages in this thread | | | Date | Thu, 24 Oct 2019 10:52:13 +0200 | From | Wolfram Sang <> | Subject | Re: [PATCH v4] mfd: mt6360: add pmic mt6360 driver |
| |
> > + for (i = 0; i < MT6360_SLAVE_MAX; i++) { > > + if (mt6360_slave_addr[i] == client->addr) { > > + mpd->i2c[i] = client; > > + continue; > > + }
Not knowing the DT bindings, I wonder if we can let the for-loop start at 1 and do beforehand: mpd->i2c[0] = client;
That would save the above if block. However, this is a minor nit.
> > + mpd->i2c[i] = i2c_new_dummy(client->adapter, > > + mt6360_slave_addr[i]);
Please use the new API i2c_new_dummy_device here...
> > + if (!mpd->i2c[i]) {
... and IS_ERR() here.
> > + dev_err(&client->dev, "new i2c dev [%d] fail\n", i); > > + ret = -ENODEV;
Then you can also return a proper errno value.
You can probably also use devm_i2c_new_dummy_device()...
> > + goto out; > > + } > > + i2c_set_clientdata(mpd->i2c[i], mpd); > > + } > > + > > + ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_AUTO, > > + mt6360_devs, ARRAY_SIZE(mt6360_devs), NULL, > > + 0, regmap_irq_get_domain(mpd->irq_data)); > > + if (ret < 0) { > > + dev_err(&client->dev, "mfd add cells fail\n"); > > + goto out; > > + } > > + > > + return 0; > > +out: > > + while (--i >= 0) { > > + if (mpd->i2c[i]->addr == client->addr) > > + continue; > > + i2c_unregister_device(mpd->i2c[i]);
... to save this ...
> > + } > > + > > + return ret; > > +} > > + > > +static int mt6360_pmu_remove(struct i2c_client *client) > > +{ > > + struct mt6360_pmu_data *mpd = i2c_get_clientdata(client); > > + int i; > > + > > + for (i = 0; i < MT6360_SLAVE_MAX; i++) { > > + if (mpd->i2c[i]->addr == client->addr) > > + continue; > > + i2c_unregister_device(mpd->i2c[i]); > > + }
... and this. But please double check.
[unhandled content-type:application/pgp-signature]
| |