Quoting Satya Priya (2022-04-14 05:30:16)
> diff --git a/drivers/regulator/qcom-pm8008-regulator.c b/drivers/regulator/qcom-pm8008-regulator.c
> new file mode 100644
> index 0000000..4375ac5
> --- /dev/null
> +++ b/drivers/regulator/qcom-pm8008-regulator.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2022, The Linux Foundation. All rights reserved. */
> +
[..]
> +
> +static int pm8008_regulator_probe(struct platform_device *pdev)
> +{
> + struct pm8008_data *chip = dev_get_drvdata(pdev->dev.parent);
> + struct device *dev = &pdev->dev;
> + struct regulator_dev *rdev;
> + struct pm8008_regulator *pm8008_reg;
> + struct regmap *regmap;
> + struct regulator_config reg_config = {};
> + int rc, i;
> + unsigned int reg;
> +
> + regmap = pm8008_get_regmap(chip, PM8008_REGULATORS_SID);
Can we use dev_get_regmap(pdev->dev.parent, "regulators") here
instead? The benefit to that is we don't tightly couple this driver to
the pm8008_data structure or header file.
> + if (!regmap) {
> + dev_err(dev, "parent regmap is missing\n");
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(reg_data); i++) {
> + pm8008_reg = devm_kzalloc(dev, sizeof(*pm8008_reg), GFP_KERNEL);
> + if (!pm8008_reg)
> + return -ENOMEM;
> +
> + pm8008_reg->regmap = regmap;
> + pm8008_reg->dev = dev;
> + pm8008_reg->base = reg_data[i].base;
> +
> + /* get slew rate */
> + rc = regmap_bulk_read(pm8008_reg->regmap,
> + LDO_STEPPER_CTL_REG(pm8008_reg->base), ®, 1);
> + if (rc < 0) {
> + dev_err(dev, "failed to read step rate configuration rc=%d\n", rc);
> + return rc;
> + }
> + reg &= STEP_RATE_MASK;
> + pm8008_reg->step_rate = DEFAULT_VOLTAGE_STEPPER_RATE >> reg;
> +
> + pm8008_reg->rdesc.type = REGULATOR_VOLTAGE;
> + pm8008_reg->rdesc.ops = &pm8008_regulator_ops;
> + pm8008_reg->rdesc.name = reg_data[i].name;
> + pm8008_reg->rdesc.supply_name = reg_data[i].supply_name;
> + pm8008_reg->rdesc.of_match = reg_data[i].name;
> + pm8008_reg->rdesc.uV_step = VSET_STEP_UV;
> + pm8008_reg->rdesc.min_uV = reg_data[i].min_uv;
> + pm8008_reg->rdesc.n_voltages
> + = ((reg_data[i].max_uv - reg_data[i].min_uv)
> + / pm8008_reg->rdesc.uV_step) + 1;
> + pm8008_reg->rdesc.linear_ranges = reg_data[i].voltage_range;
> + pm8008_reg->rdesc.n_linear_ranges = 1;
> + pm8008_reg->rdesc.enable_reg = LDO_ENABLE_REG(pm8008_reg->base);
> + pm8008_reg->rdesc.enable_mask = ENABLE_BIT;
> + pm8008_reg->rdesc.min_dropout_uV = reg_data[i].min_dropout_uv;
> + pm8008_reg->voltage_selector = -ENOTRECOVERABLE;
> + pm8008_reg->rdesc.regulators_node = of_match_ptr("regulators");
> +
> + reg_config.dev = dev->parent;
> + reg_config.driver_data = pm8008_reg;
> +
> + rdev = devm_regulator_register(dev, &pm8008_reg->rdesc, ®_config);
> + if (IS_ERR(rdev)) {
> + rc = PTR_ERR(rdev);
> + dev_err(dev, "%s: failed to register regulator rc=%d\n",
> + reg_data[i].name, rc);
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static struct platform_driver pm8008_regulator_driver = {
> + .driver = {
> + .name = "qcom-pm8008-regulator",
I'd prefer to use an of_device_id table here. That would let us populate
a "qcom,pm8008-regulators" node that had the ldo nodes as children and
avoid mfd cells.
