Re: [RESEND PATCH v6 5/6] i2c: designware: add SLAVE mode functions

From: Luis Oliveira
Date: Fri Mar 17 2017 - 07:26:58 EST


On 02-Mar-17 15:17, Luis Oliveira wrote:
> On 02-Mar-17 14:33, Jarkko Nikula wrote:
>> On 03/01/17 17:59, Luis Oliveira wrote:
>>> - Changes in Kconfig to enable I2C_DESIGNWARE_SLAVE support
>>> - Slave functions added to core library file
>>> - Slave abort sources added to common source file
>>> - New driver: i2c-designware-slave added
>>> - Changes in the Makefile to compile the I2C_DESIGNWARE_SLAVE module
>>> when supported by the architecture.
>>>
>>> All the SLAVE flow is added but it is not enabled via platform
>>> driver.
>>>
>>> Signed-off-by: Luis Oliveira <lolivei@xxxxxxxxxxxx>
>>> ---
>>> V5->V6
>>> - Enable Slave mode using a different option (kbuild reported an error)
>>> - Identation fixes needed
>>> - Changed debug prints to dev_vdbg
>>> - Fixed slave enumeration
>>>
>>> drivers/i2c/busses/Kconfig | 14 +-
>>> drivers/i2c/busses/Makefile | 1 +
>>> drivers/i2c/busses/i2c-designware-common.c | 7 +
>>> drivers/i2c/busses/i2c-designware-core.h | 3 +
>>> drivers/i2c/busses/i2c-designware-slave.c | 403 +++++++++++++++++++++++++++++
>>> 5 files changed, 427 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/i2c/busses/i2c-designware-slave.c
>>>
>> ...
>>> diff --git a/drivers/i2c/busses/i2c-designware-common.c
>>> b/drivers/i2c/busses/i2c-designware-common.c
>>> index 6357c7c78f6d..24641635aa20 100644
>>> --- a/drivers/i2c/busses/i2c-designware-common.c
>>> +++ b/drivers/i2c/busses/i2c-designware-common.c
>>> @@ -56,6 +56,13 @@ static char *abort_sources[] = {
>>> "trying to use disabled adapter",
>>> [ARB_LOST] =
>>> "lost arbitration",
>>> + [ABRT_SLAVE_FLUSH_TXFIFO] =
>>> + "read command so flush old data in the TX FIFO",
>>> + [ABRT_SLAVE_ARBLOST] =
>>> + "slave lost the bus while transmitting data to a remote master",
>>> + [ABRT_SLAVE_RD_INTX] =
>>> + "slave request for data to be transmitted and there is a 1 in "
>>> + "bit 8 of IC_DATA_CMD",
>>> };
>>
>> I know ABRT_SLAVE_RD_INTX case is from the specification but I would prefer here
>> one-line error text so it will be easier to grep from sources.
>>
>> I would like it to be also be more understandable. I don't know did I understood
>> it correctly but does it occur when attempting to read controller while
>> transmitting or something like that?
>
> It happens in the "slave-transmitter" mode, when the processor side responds to
> a request for data to be transmitted and at the same time someone writes 1 to
> the bit 8 of the IC_DATA_CMD. This bit in "slave-transmitter" mode has to be "0"
> to indicate the data is to be transmitted.
>
> In "slave-receiver" mode it has no purpose since writes to this register are not
> required.

If you agree for the v7 I can change it to "incorrect slave-transmitter mode
configuration", it's not as clear but it gives a clue of where the error comes from.

>
>>
>>> diff --git a/drivers/i2c/busses/i2c-designware-core.h
>>> b/drivers/i2c/busses/i2c-designware-core.h
>>> index b620d76ffc8d..6c70945cccea 100644
>>> --- a/drivers/i2c/busses/i2c-designware-core.h
>>> +++ b/drivers/i2c/busses/i2c-designware-core.h
>>> @@ -277,6 +277,7 @@ struct dw_i2c_dev {
>>> void (*disable)(struct dw_i2c_dev *dev);
>>> void (*disable_int)(struct dw_i2c_dev *dev);
>>> int (*init)(struct dw_i2c_dev *dev);
>>> + bool mode;
>>> };
>>>
>>
>> As I mentioned add here comment for "mode" not in patch 4/6.
>>
> Ok, I will.
> Thanks.
>