On 2/9/2022 7:48 PM, Mark Brown wrote:
On Tue, Feb 08, 2022 at 08:22:18PM +0530, Satya Priya wrote:+static int pm8008_regulator_of_parse(struct device_node *node, + const struct regulator_desc *desc, + struct regulator_config *config) +{ + struct pm8008_regulator *pm8008_reg = config->driver_data; + int rc; + unsigned int reg; + + /* get slew rate */ + rc = regmap_bulk_read(pm8008_reg->regmap, + LDO_STEPPER_CTL_REG(pm8008_reg->base), ®, 1); + if (rc < 0) { + dev_err(pm8008_reg->dev, + "%s: failed to read step rate configuration rc=%d\n", + pm8008_reg->rdesc.name, rc); + return rc; + } + reg &= STEP_RATE_MASK; + pm8008_reg->step_rate = DEFAULT_VOLTAGE_STEPPER_RATE >> reg; + + return 0;This is not doing any parsing of any DT properties at all, it is just reading a default value back from the hardware. This shouldn't be in the of_parse() callback, it should be done on probe() or something so that if someone adds ACPI support or whatever there's no surprise breakage, and so that we've got this configured even if there's no DT bindings for the specific regulator.
Okay, I'll move it to probe.
+} + +static int pm8008_regulator_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + int id = pdev->id % PM8008_NUM_LDOS; + struct regulator_dev *rdev; + struct pm8008_regulator *pm8008_reg; + struct regmap *regmap; + struct regulator_config reg_config = {}; + int rc; + + dev_dbg(dev, "DEBUG: Probing LDO%d\n", id + 1); + + regmap = dev_get_regmap(dev->parent, NULL); + if (!regmap) { + dev_err(dev, "parent regmap is missing\n"); + return -EINVAL; + } + + 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[id].base; + + pm8008_reg->rdesc.type = REGULATOR_VOLTAGE; + pm8008_reg->rdesc.regulators_node = of_match_ptr("regulators"); + pm8008_reg->rdesc.ops = &pm8008_regulator_ops; + pm8008_reg->rdesc.name = reg_data[id].name; + pm8008_reg->rdesc.supply_name = reg_data[id].supply_name; + pm8008_reg->rdesc.of_match = reg_data[id].name; + pm8008_reg->rdesc.of_parse_cb = pm8008_regulator_of_parse; + pm8008_reg->rdesc.uV_step = VSET_STEP_UV; + pm8008_reg->rdesc.min_uV = reg_data[id].min_uv; + pm8008_reg->rdesc.n_voltages + = ((reg_data[id].max_uv - reg_data[id].min_uv) + / pm8008_reg->rdesc.uV_step) + 1; + pm8008_reg->rdesc.linear_ranges = reg_data[id].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[id].min_dropout_uv; + + 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[id].name, rc); + return rc; + } + + return 0; +} + +static struct platform_driver pm8008_regulator_driver = { + .driver = { + .name = "qcom,pm8008-regulators", + }, + .probe = pm8008_regulator_probe, +}; + +module_platform_driver(pm8008_regulator_driver); + +MODULE_DESCRIPTION("Qualcomm PM8008 PMIC Regulator Driver"); +MODULE_LICENSE("GPL"); -- 2.7.4
