| Message ID | 1438344034-20211-7-git-send-email-rabel@cit-ec.uni-bielefeld.de |
|---|---|
| State | New |
| Headers | show |
On 07/31/2015 02:00 PM, Robert ABEL wrote: > CONFIG_I2C_XILINX_ERRATA makes resetting XIIC after every > Master Transmit error configurable. > Also mention proper module name for XIIC kernel module. Datasheet? version. > > Signed-off-by: Robert ABEL <rabel@cit-ec.uni-bielefeld.de> > --- > drivers/i2c/busses/Kconfig | 9 ++++++++- > drivers/i2c/busses/i2c-xiic.c | 9 +++++++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 46d5488..3255e89 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -886,7 +886,14 @@ config I2C_XILINX > Xilinx I2C controller. > > This driver can also be built as a module. If so, the module > - will be called xilinx_i2c. > + will be called i2c-xiic. > + > +config I2C_XILINX_ERRATA > + bool "Reset on " > + depends on I2C_XILINX > + help > + By enabling this option, the Xilinx I2C Controller will be reset > + after Master Transmit Errors. > > config I2C_XLR > tristate "XLR I2C support" > diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c > index 5c9897e..6a834bc 100644 > --- a/drivers/i2c/busses/i2c-xiic.c > +++ b/drivers/i2c/busses/i2c-xiic.c > @@ -509,6 +509,15 @@ static irqreturn_t xiic_process(int irq, void *dev_id) > break; > } > > +#if defined(CONFIG_I2C_XILINX_ERRATA) > + if (!(msg->flags & I2C_M_RD)) { > + /* dynamic mode seem to suffer from problems if we just flush > + * fifos and the next message is a TX with len 0 (only addr) > + * reset the IP instead of just flushing fifos > + */ > + xiic_reinit(i2c); > + } > +#endif > } > > /* Receive FIFO is full */ > Make it DT configurable. Thanks, Michal -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Michal, On Mon, Aug 3, 2015 at 7:34 AM, Michal Simek <michal.simek@xilinx.com> wrote: > Datasheet? version. > > Make it DT configurable. I'm not sure what you're asking here. This errata isn't covered by any datasheets and I don't have the time to confirm whether it actually exists. I'm thinking it might have been an issue with the old ISR and not the IP itself (it works fine in simulation when I cause a TX error, then transmit only SLA afterwards). I cannot test exhaustively on hardware right now, as I'm not able to monitor the bus directly on my development platform. Because of the above 'hunch', I didn't make this DT configurable, because I feel either the bug is present and thus the code should always be run, or the bug doesn't exist. I don't think it depends on the hardware device, so hence no DT binding. Regards Robert -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/03/2015 09:50 AM, Robert Abel wrote: > Hi Michal, > > On Mon, Aug 3, 2015 at 7:34 AM, Michal Simek <michal.simek@xilinx.com> wrote: >> Datasheet? version. >> >> Make it DT configurable. > > I'm not sure what you're asking here. This errata isn't covered by any > datasheets and I don't have the time to confirm whether it actually > exists. > I'm thinking it might have been an issue with the old ISR and not the > IP itself (it works fine in simulation when I cause a TX error, then > transmit only SLA afterwards). > I cannot test exhaustively on hardware right now, as I'm not able to > monitor the bus directly on my development platform. Every patch like this has to fix something. If some certain IP versions are affected then it should be listed at least one version. > Because of the above 'hunch', I didn't make this DT configurable, > because I feel either the bug is present and thus the code should > always be run, or the bug doesn't exist. > I don't think it depends on the hardware device, so hence no DT binding. Huh. Don't get what you are saying here. Based on your description it is HW bug and it is completely fine to have dt property to avoid this bug. Having another config property is just not acceptable solution now. Thanks, Michal -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Michal, On Thu, Aug 6, 2015 at 11:18 AM, Michal Simek <michal.simek@xilinx.com> wrote: > Every patch like this has to fix something. If some certain IP versions > are affected then it should be listed at least one version. Please re-read my email. I said: - I do NOT know of any bug. - all information about a possible bug come from that three lines of commentary which are in the _original_ mainline driver. - there is no errata mentioned in any of the datasheets I read. > Huh. Don't get what you are saying here. Based on your description it is > HW bug and it is completely fine to have dt property to avoid this bug. > Having another config property is just not acceptable solution now. You misunderstood. To recapitulate: - I have not seen this bug in action.- - I have not observed this bug in simulation. - I don't think it exists in current IP. - I personally think any odd behavior the original committers might have observed came down to bugs in their ISR. If any errata had existed in previous IP, I would expect the datasheets to mention this-- they don't. I have looked at datasheets axi_iic_ds756 v1.01a, axi_iic_ds756 v1.01b, axi_iic_ds756 v1.02a and xps_iic_ds606 v2.03a. I have simulated axi_iic v1.02a and successfully use axi_iic v1.02a in hardware with my patched driver without the CONFIG_I2C_XILINX_ERRATA configuration option configured, i.e. # CONFIG_I2C_XILINX_ERRATA is not set. On the off-chance this bug _does_ exist in previous or current IP, I provided this compile-time configurable option to enable the _old_ mainline behavior of resetting the IP after every TX error so as to not break somebody's platform. And that's why it isn't a DT property. It can always be promoted to a DT property when the existence of any errata were actually verified, but I have neither time nor inclination to do so. Therefore, I _don't know_ which version of the IP _if any_ is affected. If you require more information, I suggest you inquire with Richard Röjfors or possibly Ben Dooks about it. Richard Röjfors is to git blame for the reset-after-Tx-error functionality and the comment mentioning a possible errata. Regards, Robert -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 46d5488..3255e89 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -886,7 +886,14 @@ config I2C_XILINX Xilinx I2C controller. This driver can also be built as a module. If so, the module - will be called xilinx_i2c. + will be called i2c-xiic. + +config I2C_XILINX_ERRATA + bool "Reset on " + depends on I2C_XILINX + help + By enabling this option, the Xilinx I2C Controller will be reset + after Master Transmit Errors. config I2C_XLR tristate "XLR I2C support" diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c index 5c9897e..6a834bc 100644 --- a/drivers/i2c/busses/i2c-xiic.c +++ b/drivers/i2c/busses/i2c-xiic.c @@ -509,6 +509,15 @@ static irqreturn_t xiic_process(int irq, void *dev_id) break; } +#if defined(CONFIG_I2C_XILINX_ERRATA) + if (!(msg->flags & I2C_M_RD)) { + /* dynamic mode seem to suffer from problems if we just flush + * fifos and the next message is a TX with len 0 (only addr) + * reset the IP instead of just flushing fifos + */ + xiic_reinit(i2c); + } +#endif } /* Receive FIFO is full */
CONFIG_I2C_XILINX_ERRATA makes resetting XIIC after every Master Transmit error configurable. Also mention proper module name for XIIC kernel module. Signed-off-by: Robert ABEL <rabel@cit-ec.uni-bielefeld.de> --- drivers/i2c/busses/Kconfig | 9 ++++++++- drivers/i2c/busses/i2c-xiic.c | 9 +++++++++ 2 files changed, 17 insertions(+), 1 deletion(-)