Re: [PATCH V5 4/6] regulator: Add a regulator driver for the PM8008 PMIC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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), &reg, 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.

> +}
> +
> +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, &reg_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
> 

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux