2022-04-06 08:47:06

by Satya Priya

[permalink] [raw]
Subject: [PATCH V9 3/6] mfd: pm8008: Add mfd cell struct to register LDOs

Add mfd cell struct to match with the "qcom,pm8008-regulator"
driver and a separate probe to add pm8008_regulator_devs.
This separate probe is required to ensure the regulators are
registered only with the mfd device which contains regulators.

Add the reset-gpio toggling in the pm8008_probe() to bring
pm8008 chip out of reset instead of doing it in DT node using
"output-high" property.

Signed-off-by: Satya Priya <[email protected]>
---
Changes in V6:
- Changed the mfd_cell struct to have only name of the regulator driver.
- Using device_get_match_data() instead of of_match_node() to match data.
- Fixed few nits.

Changes in V7:
- Fixed minor errors.

Changes in V8:
- Split the probe for infra and regulator devices
- Add the reset-gpio toggling in the infra driver probe

Changes in V9:
- Fixed nits.

drivers/mfd/qcom-pm8008.c | 80 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/qcom-pm8008.c b/drivers/mfd/qcom-pm8008.c
index c472d7f..c7781ab 100644
--- a/drivers/mfd/qcom-pm8008.c
+++ b/drivers/mfd/qcom-pm8008.c
@@ -4,10 +4,12 @@
*/

#include <linux/bitops.h>
+#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
+#include <linux/mfd/core.h>
#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/of_platform.h>
@@ -27,6 +29,10 @@
#define INT_EN_CLR_OFFSET 0x16
#define INT_LATCHED_STS_OFFSET 0x18

+static const struct mfd_cell pm8008_regulator_devs[] = {
+ MFD_CELL_NAME("qcom,pm8008-regulators"),
+};
+
enum {
PM8008_MISC,
PM8008_TEMP_ALARM,
@@ -57,6 +63,7 @@ enum {
struct pm8008_data {
struct device *dev;
struct regmap *regmap;
+ struct gpio_desc *reset_gpio;
int irq;
struct regmap_irq_chip_data *irq_data;
};
@@ -239,13 +246,44 @@ static int pm8008_probe(struct i2c_client *client)
dev_err(chip->dev, "Failed to probe irq periphs: %d\n", rc);
}

+ chip->reset_gpio = devm_gpiod_get(chip->dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(chip->reset_gpio)) {
+ dev_err(chip->dev, "failed to acquire reset gpio\n");
+ return PTR_ERR(chip->reset_gpio);
+ }
+ gpiod_set_value(chip->reset_gpio, 1);
+
return devm_of_platform_populate(chip->dev);
}

+static int pm8008_probe_regulators(struct i2c_client *client)
+{
+
+ int rc;
+ struct pm8008_data *chip;
+
+ 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;
+
+ rc = devm_mfd_add_devices(chip->dev, 0, pm8008_regulator_devs,
+ ARRAY_SIZE(pm8008_regulator_devs), NULL, 0, NULL);
+ if (rc)
+ dev_err(chip->dev, "Failed to add children: %d\n", rc);
+
+ return rc;
+}
+
static const struct of_device_id pm8008_match[] = {
{ .compatible = "qcom,pm8008", },
- { },
+ { }
};
+MODULE_DEVICE_TABLE(of, pm8008_match);

static struct i2c_driver pm8008_mfd_driver = {
.driver = {
@@ -254,7 +292,45 @@ static struct i2c_driver pm8008_mfd_driver = {
},
.probe_new = pm8008_probe,
};
-module_i2c_driver(pm8008_mfd_driver);
+
+static const struct of_device_id pm8008_regulators_match[] = {
+ { .compatible = "qcom,pm8008-regulators", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, pm8008_regulators_match);
+
+static struct i2c_driver pm8008_regulators_driver = {
+ .driver = {
+ .name = "pm8008-regulators",
+ .of_match_table = pm8008_regulators_match,
+ },
+ .probe_new = pm8008_probe_regulators,
+};
+
+static int __init pm8008_i2c_init(void)
+{
+ int ret;
+
+ ret = i2c_add_driver(&pm8008_mfd_driver);
+ if (ret) {
+ pr_err("Failed to register driver for pm8008_infra: %d\n", ret);
+ return ret;
+ }
+
+ ret = i2c_add_driver(&pm8008_regulators_driver);
+ if (ret)
+ pr_err("Failed to register driver for pm8008_regulators: %d\n", ret);
+
+ return ret;
+}
+module_init(pm8008_i2c_init);
+
+static void __exit pm8008_i2c_exit(void)
+{
+ i2c_del_driver(&pm8008_mfd_driver);
+ i2c_del_driver(&pm8008_regulators_driver);
+}
+module_exit(pm8008_i2c_exit);

MODULE_LICENSE("GPL v2");
MODULE_ALIAS("i2c:qcom-pm8008");
--
2.7.4


2022-04-06 12:27:59

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH V9 3/6] mfd: pm8008: Add mfd cell struct to register LDOs

Quoting Satya Priya (2022-04-05 06:50:30)
> Add mfd cell struct to match with the "qcom,pm8008-regulator"
> driver and a separate probe to add pm8008_regulator_devs.
> This separate probe is required to ensure the regulators are
> registered only with the mfd device which contains regulators.

I don't get it. Shouldn't the pm8008-regulators compatible string be a
different i2c driver in drivers/regulator/ that matches the compatible
string for qcom,pm8008-regulators?

>
> Add the reset-gpio toggling in the pm8008_probe() to bring
> pm8008 chip out of reset instead of doing it in DT node using
> "output-high" property.

Please split this part off to a different patch.

2022-04-06 15:44:17

by Satya Priya

[permalink] [raw]
Subject: Re: [PATCH V9 3/6] mfd: pm8008: Add mfd cell struct to register LDOs


On 4/6/2022 12:38 AM, Stephen Boyd wrote:
> Quoting Satya Priya (2022-04-05 06:50:30)
>> Add mfd cell struct to match with the "qcom,pm8008-regulator"
>> driver and a separate probe to add pm8008_regulator_devs.
>> This separate probe is required to ensure the regulators are
>> registered only with the mfd device which contains regulators.
> I don't get it. Shouldn't the pm8008-regulators compatible string be a
> different i2c driver in drivers/regulator/ that matches the compatible
> string for qcom,pm8008-regulators?


Initially we had it that way but as per Mark's suggestion here [1] I've
made these changes to register regulators directly through mfd driver.

[1]
https://patchwork.kernel.org/project/linux-arm-msm/patch/[email protected]/#24618011


>> Add the reset-gpio toggling in the pm8008_probe() to bring
>> pm8008 chip out of reset instead of doing it in DT node using
>> "output-high" property.
> Please split this part off to a different patch.


Okay.