Re: [PATCH] i2c-mxs: detect No Slave Ack on SELECT in PIO mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




W dniu 2014-09-19 04:45, Marek Vasut pisze:
On Wednesday, September 10, 2014 at 05:18:06 PM, Janusz Uzycki wrote:
Reported problem:
i2cdetect scanned i2c bus very slow if address was not occupied by any
device.

Solution:
The patch adds to mxs_i2c_pio_wait_xfer_end() function
NO_SLAVE_ACK_IRQ bit polling during wait loop (until timeout).
If the bit is set the function immediately returns ENXIO error
in order to break the loop and not reset I2C block (it is in idle state
then). The function is called by mxs_i2c_pio_setup_xfer() to wait for
complete xfer after sent SELECT, READ or WRITE command.
If SELECT command is sent and selected slave address is unused by any
device on the bus I2C block sets NO_SLAVE_ACK_IRQ flag and doesn't
deassert CTRL0_RUN. Therefore we need to break the timeout loop when the
flag is set,
otherwise the loop continues until long timeout (1000ms).
The change does not affect READ command because slave does not ack
any byte then (only the master does ack / or not for the last read byte).
According to i.MX28 reference manual (quoted below) it is not clear
if the patch affects WRITE command. However when no acked bytes
on WRITE command followed after address byte (SELECT command)
STAT_GOT_A_NAK flag is set rather than NO_SLAVE_ACK_IRQ (no tested).
Therefore clock stretching shouldn't be affected too.
It has confirmation in FSL BSP 2.6.35 i2c implementation which
completes xfer after NO_SLAVE_ACK_IRQ interrupt and scheduled work.
Registers on NO_SLAVE_ACK_IRQ in PIO mode:
* STAT: 0xd0000e00
	MASTER_PRESENT
	SLAVE_PRESENT
	GOT_A_NAK !
	BUS_BUSY
	CLK_GEN_BUSY
	DATA_ENGINE_BUSY
* CTRL0: 0x20230000
	RUN !
	RETAIN_CLOCK
	MASTER_MODE
	DIRECTION
* CTRL1: 0x688600a0
	RD_QUEUE_IRQ
	WR_QUEUE_IRQ
	ACK_MODE
	SLAVE_ADDRESS_BYTE=0b10000110
	BUS_FREE_IRQ
	NO_SLAVE_ACK_IRQ !

NO_SLAVE_ACK_IRQ (CTRL1):
When a start condition is transmitted in master mode, the next byte
contains an address for a targeted slave. If the targeted slave does not
acknowledge the address byte, then this interrupt is set, no further I2C
protocol is processed, and the I2C bus returns to the idle state.
This bit is set to indicate that an interrupt is requested
by the I2C controller because the slave addressed
by a master transfer did not respond with an acknowledge.

Signed-off-by: Janusz Uzycki <j.uzycki@xxxxxxxxxxxxxx>
OK, uh, can the commit message not be shortened to like 5-10 lines ? I think you
really need to find your balance when it comes to documenting changes, but don't
worry, this will happen sooner rather than later ;-)

It would be sufficient to say that you had problem with slow i2cdetect and that
was because the i2c controller driver ignored the NO_SLAVE_ACK bit. By
leveraging NO_SLAVE_ACK bit, the speedup happens. And this change is correct and
doesn't break anything because <a few lines here>.

Do you know what I mean ?


Yes, I know. It was explanation in details rather for comments than final patch.
Is it ok?:
i2cdetect scanned i2c bus slow because the i2c-mxs driver ignored the NO_SLAVE_ACK bit
during busy-waiting loop. Thanks to the patch, the speedup happens.
The change doesn't break anything else because:
- on SELECT: NO_SLAVE_ACK bit checking is just welcome
- on READ: master (the i2c controller, no slave device) generates ACK/NAK bit
- on WRITE: NO_SLAVE_ACK can be treated as NAK (the same effect)
  so even the i2c controller sets NO_SLAVE_ACK on NAK (not confirmed)
  the WRITE is not effected
- on clock stretching: SCL wire is involved, it has no influence
  on the ACK bit value on SDA wire

kind regards
Janusz

--
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




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux