Messages in this thread | | | Subject | Re: [PATCH V5 4/6] regulator: Add a regulator driver for the PM8008 PMIC | From | "Satya Priya Kakitapalli (Temp)" <> | Date | Fri, 11 Feb 2022 15:30:48 +0530 |
| |
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 >>
| |