On Fri, Mar 4, 2016 at 2:10 AM, Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote:
> On Thu, Mar 03, 2016 at 11:20:40AM +0530, Shubhrajyoti Datta wrote:
>
>> Implement save restore for i2c module.
>> Since we have only a couple of registers
>> an unconditional restore is done.
>
> But you only save one register instead of a couple? Also, the subject
> could be more descriptive IMO.
That is because the timeout is written a fixed value always so just restore
is done.
>
>> zynq-mp has the capability of going off.
>> the current kernel does not hit off however some day it will.
>> since the overhead of having the support is not much may be it is better to have it in the kernel.
>
> This sounds like the patch is not tested?
to simulate the suspend behavior I did a reset of the module by writing to
slcr register.
And then did a transfer.
>
>> +static void cdns_i2c_init(struct cdns_i2c *id)
>> +{
>> + cdns_i2c_writereg(id->ctrl_reg, CDNS_I2C_CR_OFFSET);
>> + /*
>> + * Cadence I2C controller has a bug wherein it generates
>> + * invalid read transaction after HW timeout in master receiver mode.
>> + * HW timeout is not used by this driver and the interrupt is disabled.
>> + * But the feature itself cannot be disabled. Hence maximum value
>> + * is written to this register to reduce the chances of error.
>> + */
>> + cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
>> +}
>
> This...
Writes the values to the register.
>
>> +
>> +/**
>> * cdns_i2c_runtime_resume - Runtime resume
>> * @dev: Address of the platform_device structure
>> *
>> @@ -853,6 +874,7 @@ static int __maybe_unused cdns_i2c_runtime_resume(struct device *dev)
>> dev_err(dev, "Cannot enable clock.\n");
>> return ret;
>> }
>> + cdns_i2c_init(xi2c);
>
> and this...
At resume .
(Actually this could be conditional however since we have 2 registers
so can be unconditioal)
>
>> -
>> - /*
>> - * Cadence I2C controller has a bug wherein it generates
>> - * invalid read transaction after HW timeout in master receiver mode.
>> - * HW timeout is not used by this driver and the interrupt is disabled.
>> - * But the feature itself cannot be disabled. Hence maximum value
>> - * is written to this register to reduce the chances of error.
>> - */
>> - cdns_i2c_writereg(CDNS_I2C_TIMEOUT_MAX, CDNS_I2C_TIME_OUT_OFFSET);
>> + cdns_i2c_init(id);
And at init.
>
> ... and this look like a unrelated change to me?
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
