On 26/07/2023 10:16, like@xxxxxxxxxx wrote: > From: Alec Li <like@xxxxxxxxxx> > > Add regulator driver for the device AWINIC AW37503 which is single > inductor - dual output power supply device. AW37503 device is > designed to support general positive/negative driven applications > like TFT display panels. > Thank you for your patch. There is something to discuss/improve. > + > +static int aw37503_probe(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + struct aw37503_regulator *chip; > + struct regulator_dev *rdev; > + struct regmap *regmap; > + struct regulator_config config = { }; > + int id; > + int ret; > + > + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL); > + if (!chip) > + return -ENOMEM; > + > + regmap = devm_regmap_init_i2c(client, &aw37503_regmap_config); > + if (IS_ERR(regmap)) { > + ret = PTR_ERR(regmap); > + dev_err(dev, "regmap init failed: %d\n", ret); > + return ret; return dev_err_probe > + } > + > + i2c_set_clientdata(client, chip); > + chip->dev = dev; > + > + for (id = 0; id < AW37503_MAX_REGULATORS; ++id) { > + config.regmap = regmap; > + config.dev = dev; > + config.driver_data = chip; > + > + rdev = devm_regulator_register(dev, &aw_regs_desc[id], > + &config); > + if (IS_ERR(rdev)) { > + ret = PTR_ERR(rdev); > + dev_err(dev, "regulator %s register failed: %d\n", > + aw_regs_desc[id].name, ret); > + return ret; return dev_err_probe will be easier > + } > + } > + return 0; > +} > + > +static const struct i2c_device_id aw37503_id[] = { > + {.name = "aw37503",}, > + {}, > +}; > +MODULE_DEVICE_TABLE(i2c, aw37503_id); > + > +static const struct of_device_id aw37503_dt_ids[] = { > + {.compatible = "awinic,aw37503",}, > + { /* Sentinel */ }, > +}; > + > +MODULE_DEVICE_TABLE(of, aw37503_dt_ids); > + > +static struct i2c_driver aw37503_i2c_driver = { > + .driver = { > + .name = "aw37503", > + .of_match_table = of_match_ptr(aw37503_dt_ids), Drop of_match_ptr() Best regards, Krzysztof