On 11/6/2017 9:54 PM, Mark Brown wrote:
> On Fri, Nov 03, 2017 at 04:35:45PM -0400, Alex Deucher wrote:
>
>> Minimum time required between power On of codec and read
>> of RT5645_VENDOR_ID2 is 400msec. We should wait and attempt
>> before erroring out.
>
> So the description says we have to wait 400ms before attempting a
> read...
Yes, that's true.
>
>> BUG=b:66978383
>
> What does this mean?
A bug ID. Removing it in V2.
>
>> @@ -3786,6 +3789,15 @@ static int rt5645_i2c_probe(struct i2c_client *i2c,
>> }
>> regmap_read(regmap, RT5645_VENDOR_ID2, &val);
>>
>> + /*
>> + * Read for 400msec, as it is the interval required between
>> + * read and power On.
>> + */
>> + while (val != RT5645_DEVICE_ID && val != RT5650_DEVICE_ID && --timeout) {
>> + msleep(1);
>> + regmap_read(regmap, RT5645_VENDOR_ID2, &val);
>> + }
>> +
>
> ...but what we actually do is try to read up to 400 times starting well
> before that 400ms is up. This directly contradicts what the commit
> message said we needed, may take a lot longer if the chip misbehaves on
> the I2C bus while it's not ready (which wouldn't be that much of a
> surprise), might lead to us reporting before the chip is really stable
> (if the read happens to work while the chip isn't yet stable) and could
> cause lots of noise on the console if the I2C controller gets upset.
> What are we actually waiting for here?
>
In my understanding if we get RT5645 or RT5650 DEVICE ID from register
RT5645_VENDOR_ID2 then that means the chip is stable.
> If we really need 400ms of dead reckoning time (which is a lot for a
> modern chip, that feels more like a VMID ramp) then it's going to be
> safer to just do that.
>
Agreed, will just sleep for 400ms before reading the register value.
