On Tue, 14 Jun 2022, Satya Priya wrote: > Use i2c_new_dummy_device() to register pm8008-regulator > client present at a different address space, instead of > defining a separate DT node. This avoids calling the probe > twice for the same chip, once for each client pm8008-infra > and pm8008-regulator. > > As a part of this define pm8008_regmap_init() to do regmap > init for both the clients and define pm8008_get_regmap() to > pass the regmap to the regulator driver. > > Signed-off-by: Satya Priya <quic_c_skakit@xxxxxxxxxxx> > Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx> > --- > Changes in V15: > - None. > > Changes in V14: > - None. > > Changes in V13: > - None. > > drivers/mfd/qcom-pm8008.c | 34 ++++++++++++++++++++++++++++++++-- > include/linux/mfd/qcom_pm8008.h | 9 +++++++++ > 2 files changed, 41 insertions(+), 2 deletions(-) > create mode 100644 include/linux/mfd/qcom_pm8008.h > > diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c > index 569ffd50..55e2a8e 100644 > --- a/drivers/mfd/qcom-pm8008.c > +++ b/drivers/mfd/qcom-pm8008.c > @@ -9,6 +9,7 @@ > #include <linux/interrupt.h> > #include <linux/irq.h> > #include <linux/irqdomain.h> > +#include <linux/mfd/qcom_pm8008.h> > #include <linux/module.h> > #include <linux/of_device.h> > #include <linux/of_platform.h> > @@ -57,6 +58,7 @@ enum { > > struct pm8008_data { > struct device *dev; > + struct regmap *regulators_regmap; > int irq; > struct regmap_irq_chip_data *irq_data; > }; > @@ -150,6 +152,12 @@ static struct regmap_config qcom_mfd_regmap_cfg = { > .max_register = 0xFFFF, > }; > > +struct regmap *pm8008_get_regmap(const struct pm8008_data *chip) > +{ > + return chip->regulators_regmap; > +} > +EXPORT_SYMBOL_GPL(pm8008_get_regmap); Seems like abstraction for the sake of abstraction. Why not do the dereference inside the regulator driver? > static int pm8008_init(struct regmap *regmap) > { > int rc; > @@ -217,11 +225,25 @@ static int pm8008_probe_irq_peripherals(struct pm8008_data *chip, > return 0; > } > > +static struct regmap *pm8008_regmap_init(struct i2c_client *client, > + struct pm8008_data *chip) > +{ > + struct regmap *regmap; > + > + regmap = devm_regmap_init_i2c(client, &qcom_mfd_regmap_cfg); > + if (!regmap) > + return NULL; > + > + i2c_set_clientdata(client, chip); > + return regmap; > +} This function seems superfluous. It's only called once and it contains a single call. Just pop the call directly into probe. > static int pm8008_probe(struct i2c_client *client) > { > int rc; > struct pm8008_data *chip; > struct gpio_desc *reset_gpio; > + struct i2c_client *regulators_client; > struct regmap *regmap; > > chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL); > @@ -229,11 +251,19 @@ static int pm8008_probe(struct i2c_client *client) > return -ENOMEM; > > chip->dev = &client->dev; > - regmap = devm_regmap_init_i2c(client, &qcom_mfd_regmap_cfg); > + regmap = pm8008_regmap_init(client, chip); > if (!regmap) > return -ENODEV; > > - i2c_set_clientdata(client, chip); > + regulators_client = i2c_new_dummy_device(client->adapter, client->addr + 1); > + if (IS_ERR(regulators_client)) { > + dev_err(&client->dev, "can't attach client\n"); > + return PTR_ERR(regulators_client); > + } > + > + chip->regulators_regmap = pm8008_regmap_init(regulators_client, chip); > + if (!chip->regulators_regmap) > + return -ENODEV; > > reset_gpio = devm_gpiod_get(chip->dev, "reset", GPIOD_OUT_LOW); > if (IS_ERR(reset_gpio)) > diff --git a/include/linux/mfd/qcom_pm8008.h b/include/linux/mfd/qcom_pm8008.h > new file mode 100644 > index 0000000..3814bff > --- /dev/null > +++ b/include/linux/mfd/qcom_pm8008.h > @@ -0,0 +1,9 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +// Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > +#ifndef __QCOM_PM8008_H__ > +#define __QCOM_PM8008_H__ > + > +struct pm8008_data; > +struct regmap *pm8008_get_regmap(const struct pm8008_data *chip); > + > +#endif -- Lee Jones [李琼斯] Principal Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog