Quoting Satya Priya Kakitapalli (Temp) (2022-04-18 08:08:33) > > On 4/15/2022 5:51 AM, Stephen Boyd wrote: > > Quoting Satya Priya (2022-04-14 05:30:14) > >> Use i2c_new_dummy_device() to register clients along with > >> the main mfd device. > > Why? > > > As discussed on V9 of this series, I've done these changes. > > By using this API we can register other clients at different address > space without having separate DT node. > > This avoids calling the probe twice for the same chip, once for each > address space 0x8 and 0x9. I'll add this description in commit text. > Perfect, thanks. > > >> @@ -221,19 +236,38 @@ static int pm8008_probe_irq_peripherals(struct pm8008_data *chip, > >> > >> static int pm8008_probe(struct i2c_client *client) > >> { > >> - int rc; > >> + int rc, i; > >> struct pm8008_data *chip; > >> + struct device_node *node = client->dev.of_node; > >> > >> chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL); > >> if (!chip) > >> return -ENOMEM; > >> > >> chip->dev = &client->dev; > >> - chip->regmap = devm_regmap_init_i2c(client, &qcom_mfd_regmap_cfg); > >> - if (!chip->regmap) > >> - return -ENODEV; > >> > >> - i2c_set_clientdata(client, chip); > >> + for (i = 0; i < PM8008_NUM_CLIENTS; i++) { > > This is 2. Why do we have a loop? Just register the i2c client for > > pm8008_infra first and then make a dummy for the second address without > > the loop and the indentation. Are there going to be more i2c clients? > > > There wont be more than 2 clients, I can remove the loop, but then we > will have repetitive code.. something like below Repetitive code can be refactored into a subroutine. > > chip->dev = &client->dev; > i2c_set_clientdata(client, chip); > > regmap = devm_regmap_init_i2c(client, &qcom_mfd_regmap_cfg[0]); > if (!regmap) > return -ENODEV; > > > pm8008_client = i2c_new_dummy_device(client->adapter, > client->addr + 1); > if (IS_ERR(pm8008_client)) { > dev_err(&pm8008_client->dev, "can't attach client\n"); > return PTR_ERR(pm8008_client); > } > pm8008_client->dev.of_node = of_node_get(client->dev.of_node); > i2c_set_clientdata(pm8008_client, chip); > > regulators_regmap = devm_regmap_init_i2c(pm8008_client, > &qcom_mfd_regmap_cfg[1]); > if (!regmap) > return -ENODEV; > > >> diff --git a/include/linux/mfd/qcom_pm8008.h b/include/linux/mfd/qcom_pm8008.h > >> new file mode 100644 > >> index 0000000..bc64f01 > >> --- /dev/null > >> +++ b/include/linux/mfd/qcom_pm8008.h > >> @@ -0,0 +1,13 @@ > >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> +#ifndef __QCOM_PM8008_H__ > >> +#define __QCOM_PM8008_H__ > >> + > >> +#define PM8008_INFRA_SID 0 > >> +#define PM8008_REGULATORS_SID 1 > >> + > >> +#define PM8008_NUM_CLIENTS 2 > >> + > >> +struct pm8008_data; > >> +struct regmap *pm8008_get_regmap(struct pm8008_data *chip, u8 sid); > > Could this be avoided if the regulator driver used > > dev_get_regmap(&pdev->dev.parent, "regulator") to find the regmap named > > "regulator" of the parent device, i.e. pm8008-infra. > > I gave it a try, it didn't work. I could not get the regmap for > regulators using pm8008-infra i.e., 0x8 device pointer. Did it not work because the regmap config for the regulator config didn't have a name?