On 02/26/2014 11:28 AM, Peter Ujfalusi wrote: > On 02/26/2014 11:53 AM, Florian Vaussard wrote: >> On 02/26/2014 08:26 AM, Peter Ujfalusi wrote: >>> On 02/25/2014 05:41 PM, Florian Vaussard wrote: >>>> If the power was not enabled at all, I would be unable to read the >>>> revision register, no? And delaying the probe by one millisecond would >>>> be of no help in this case IMHO. >>> >>> One thing which might cause this is that if the audpwron GPIO is set high >>> before the twl6040 module is probing. When I request the GPIO I ask it to be >>> set to low. If the line was high before this means we initiate the power off >>> sequence. >>> Can you try something like this: >>> >> >> I statistically checked that the sleep should be placed after the GPIO >> request, so indeed this seems to be the problem, and your explanation is >> plausible. Can you send a proper patch? >> >> Now, related to this, I managed to found a part of the datasheet on the >> Great Internet. Looking at the "Power-Up Sequence" section, it is written: >> >> - NRESPWRON goes high -> plug detect and GPO are available >> - V2V1 goes high -> hook-detect available by I2C programming (sleep mode) >> - AUDPWRON goes high && READYINT -> ready to communicate through I2C and PDM >> >> So, although there seems to be some contradictions on when it is >> possible to access the I2C, shouldn't we enable the AUDPWRON GPIO >> _before_ making any I2C access? >> >> For the twl6040_probe, the following path would seem more correct to me: >> >> 1) enable regulators >> 2) request AUDPWRON >> 3) twl6040_power(ON) && regcache_cache_only(false) >> 4) wait for READYINT (or sleep if deterministic) >> 5) perform all required I2C accesses (read revision, etc.) >> 6) twl6040_power(OFF) && regcache_cache_only(true) >> >> What do you think? > > Even when the AUDPWRON signal is low we can access to registers in VIO domain, > plug detect and GPO functions. So there's no need to power on the codec just > to power it down later. > Ok, if it is safe to do so, I am fine with the current probe. I let you post a patch to add a delay after requesting the GPIO. Regards, Florian > >> Thanks, >> Florian >> >>> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c >>> index 75316fb33448..d2a0bd1539ae 100644 >>> --- a/drivers/mfd/twl6040.c >>> +++ b/drivers/mfd/twl6040.c >>> @@ -674,6 +674,9 @@ static int twl6040_probe(struct i2c_client *client, >>> GPIOF_OUT_INIT_LOW, "audpwron"); >>> if (ret) >>> goto gpio_err; >>> + >>> + /* power-down sequence latency */ >>> + usleep_range(500, 700); >>> } >>> >>> ret = regmap_add_irq_chip(twl6040->regmap, twl6040->irq, IRQF_ONESHOT, >>> >>> > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
