Hello Yuvaraj,
On 08/22/2014 08:01 AM, Yuvaraj Cd wrote:
>> +
>> +static int max77802_pmic_probe(struct platform_device *pdev)
>> +{
>> + struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
>> + struct max77686_platform_data *pdata = dev_get_platdata(iodev->dev);
>> + struct max77802_regulator_prv *max77802;
>> + int i, ret = 0, val;
>> + struct regulator_config config = { };
>> +
>> + /* This is allocated by the MFD driver */
>> + if (!pdata) {
>> + dev_err(&pdev->dev, "no platform data found for regulator\n");
>> + return -ENODEV;
>> + }
>> +
>> + max77802 = devm_kzalloc(&pdev->dev,
>> + sizeof(struct max77802_regulator_prv),
>> + GFP_KERNEL);
>> + if (!max77802)
>> + return -ENOMEM;
>> +
>> + if (iodev->dev->of_node) {
>> + ret = max77802_pmic_dt_parse_pdata(pdev, pdata);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + config.dev = iodev->dev;
>> + config.regmap = iodev->regmap;
>> + config.driver_data = max77802;
>> + platform_set_drvdata(pdev, max77802);
>> +
>> + for (i = 0; i < MAX77802_REG_MAX; i++) {
>> + struct regulator_dev *rdev;
>> + int id = pdata->regulators[i].id;
>> + int shift = max77802_get_opmode_shift(id);
>> +
>> + config.init_data = pdata->regulators[i].initdata;
>> + config.of_node = pdata->regulators[i].of_node;
>> +
>> + ret = regmap_read(iodev->regmap, regulators[i].enable_reg, &val);
>> + max77802->opmode[id] = val >> shift & MAX77802_OPMODE_MASK;
>
> I have been using this patch series for adding UHS support for dw_mmc
> driver. During reboot testing I came across an issue where card
> detection fails due to vqmmc regulator not getting enabled. On
> debugging further, I found that PMIC driver is reading the operating
> mode during probe and reusing it in the enable function. With the UHS
> patches vqmmc regulator gets disabled during POWER_OFF and if we do
> warm reboot, an incorrect operating mode(OFF) is read. This leads to
> the vqmmc regulator staying disabled. I have referred to 77686 driver
> and observed that they are handling this a little differently. With
> the following change in the driver above issue is resolved:
>
> - ret = regmap_read(iodev->regmap,
> regulators[i].enable_reg, &val);
> - max77802->opmode[id] = val >> shift & MAX77802_OPMODE_MASK;
> + max77802->opmode[i] = regulators[i].enable_mask >> shift;
>
I don't think this change is correct it its current form since
.enable_mask is initialized to MAX77802_OPMODE_MASK << shift so this is
actually setting the opmode to MAX77802_OPMODE_MASK.
Now, MAX77802_OPMODE_MASK has the same value than MAX77802_OPMODE_NORMAL
so what you are really doing here is initializing the opmode to
MAX77802_OPMODE_NORMAL.
> Please have a look and let me know, if there is any better way of handling this.
>
The first versions of this driver did in fact set the opmode to
MAX77802_OPMODE_NORMAL on probe but Mark asked me to read it from the
hardware instead [0]. I was indeed worried at the time that something like
this could happen on a warm reset [1].
Mark, any opinions on how this should be solved will be highly appreciated.
Best regards,
Javier
[0]: https://lkml.org/lkml/2014/6/16/576
[1]: https://lkml.org/lkml/2014/6/17/174
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
