Messages in this thread | | | Date | Fri, 22 Aug 2014 14:15:46 +0200 | From | Javier Martinez Canillas <> | Subject | Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators |
| |
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
| |