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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YgPNLq6tdj5/UpZE@sirena.org.uk>
Date:   Wed, 9 Feb 2022 14:18:22 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Satya Priya <quic_c_skakit@...cinc.com>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Lee Jones <lee.jones@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Das Srinagesh <gurus@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, swboyd@...omium.org,
        quic_collinsd@...cinc.com, quic_subbaram@...cinc.com,
        quic_jprakash@...cinc.com
Subject: Re: [PATCH V5 4/6] regulator: Add a regulator driver for the PM8008
 PMIC

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
> 

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)