Re: [PATCH 2/2] i2c-mxs: fixed PIO NACK error instead of timeout

[Marek Vasut <marex-ynQEQJNshbs@public.gmane.org>]

On Tuesday, September 09, 2014 at 03:10:25 PM, Janusz Użycki wrote:
> W dniu 2014-09-09 14:48, Marek Vasut pisze:
> >>> Shouldn't this check be used only after the 'SELECT' command ?
> >> 
> >> It looks |||mxs_i2c_isr()| for DMA transfer does not differentiate
> >> commands also
> >> and does not mask irqs for each command.
> >> 
> >> |STAT_GOT_A_NAK|  is a separate bit.|CTRL1_NO_SLAVE_ACK_IRQ can be set
> >> 
> >> both after SELECT and WRITE command. Should we differentiate?
> > 
> > What about writes that take long time, will checking this bit not break
> > them ? (like programming a slow eeprom or such)
> 
> No, master clock speed (SCL) decides here.
> If slave does not confirm I2C address  (SELECT) using ACK
> any timeout doesn't help.

The SELECT case is OK in that aspect. But I was talking about WRITEs.

> RUN bit is not deasserted.
> The same thing is for WRITE cmd. Each byte has to be acked
> otherwise STAT_GOT_A_NAK is set.

OK

> If it is too fast a slave has possibility to inform by setting SCL low.

Do you mean clock stretching ?

> However I haven't seen any driver which supports the method.
> 
> >> |Checking CTRL1_NO_SLAVE_ACK_IRQ |bit for SELECT command will increase
> >> 
> >> code size only
> >> without special profit. Current PIO implementation also gathers all
> >> errors together and reads them on the end by
> >> mxs_i2c_pio_check_error_state(). Probably
> >> mxs_i2c_pio_check_error_state() call or
> >> enabling interrupt masks for PIO could be better than
> >> direct |CTRL1_NO_SLAVE_ACK_IRQ |bit checking for clear code.
> >> It also could support multimaster for PIO (MASTER_LOSS).
> > 
> > Actually, the PIO is explicitly IRQ-less and is used only for
> > transferring very short amounts of data.
> 
> Yes but error service could be more common probably.

Feel free to prepare a patch please!

Best regards,
Marek Vasut