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: <66830ee2-5846-8298-3371-359c7d1999db@quicinc.com>
Date:   Fri, 11 Feb 2022 15:30:48 +0530
From:   "Satya Priya Kakitapalli (Temp)" <quic_c_skakit@...cinc.com>
To:     Mark Brown <broonie@...nel.org>
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 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), &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.


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, &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
>>