| Message ID | 20240218212557.22836-1-marvin24@gmx.de |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [1/2] staging/nvec: update comment regarding udelay in the isr | expand |
On Sun Feb 18, 2024 at 10:25 PM CET, Marc Dietrich wrote: > Update the comment before the udelay on how to replace it. > > Signed-off-by: Marc Dietrich <marvin24@gmx.de> > --- > drivers/staging/nvec/nvec.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c > index b3f114cb00dc..ec081d81a308 100644 > --- a/drivers/staging/nvec/nvec.c > +++ b/drivers/staging/nvec/nvec.c > @@ -709,10 +709,11 @@ static irqreturn_t nvec_interrupt(int irq, void *dev) > status & RNW ? " RNW" : ""); > > /* > - * TODO: A correct fix needs to be found for this. > + * TODO: replace the udelay with a read back after each writel above > + * in order to work around a hardware issue, see i2c-tegra.c i2c-tegra.c is almost 2000 lines, so it'd be good to be a little more specific. Are you referring to the work-around in i2c_writel()? If so, perhaps mention that function so that people can find it. > * > - * We experience less incomplete messages with this delay than without > - * it, but we don't know why. Help is appreciated. > + * Unfortunately, this change causes an intialisation issue with the > + * touchpad, which needs to be fixed first. Perhaps something like: "... this change would cause an initialisation issue... " to make it a little more clear what you are referencing. Is there any information about the touchpad issue? Any idea what's going wrong during the initialization? Seems like this issue has been around a very long time already, and the further we kick this can down the road the less likely it will be that we'll ever fix it. Thierry > */ > udelay(100); > > -- > 2.40.1
Hi Thierry, sorry to reply late, but I somehow missed your mail. On Mon, 19 Feb 2024, Thierry Reding wrote: > On Sun Feb 18, 2024 at 10:25 PM CET, Marc Dietrich wrote: >> Update the comment before the udelay on how to replace it. >> >> Signed-off-by: Marc Dietrich <marvin24@gmx.de> >> --- >> drivers/staging/nvec/nvec.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c >> index b3f114cb00dc..ec081d81a308 100644 >> --- a/drivers/staging/nvec/nvec.c >> +++ b/drivers/staging/nvec/nvec.c >> @@ -709,10 +709,11 @@ static irqreturn_t nvec_interrupt(int irq, void *dev) >> status & RNW ? " RNW" : ""); >> >> /* >> - * TODO: A correct fix needs to be found for this. >> + * TODO: replace the udelay with a read back after each writel above >> + * in order to work around a hardware issue, see i2c-tegra.c > > i2c-tegra.c is almost 2000 lines, so it'd be good to be a little more > specific. Are you referring to the work-around in i2c_writel()? If so, > perhaps mention that function so that people can find it. yes, I thought this should have been clear as there is (nearly) no write without a readback in that driver. >> * >> - * We experience less incomplete messages with this delay than without >> - * it, but we don't know why. Help is appreciated. >> + * Unfortunately, this change causes an intialisation issue with the >> + * touchpad, which needs to be fixed first. > > Perhaps something like: "... this change would cause an initialisation > issue... " to make it a little more clear what you are referencing. Is > there any information about the touchpad issue? Any idea what's going > wrong during the initialization? > > Seems like this issue has been around a very long time already, and the > further we kick this can down the road the less likely it will be that > we'll ever fix it. I must confess that I wasn't motivated to fix it, because it works most of the time. The driver needs more love than just fixing this issue, e.g. implementing a reliable sync write which may be one of the underlying problems. On the other hand, there is a downstream i2c client implementation [1], which uses the kernel i2c infrastructure and may lay a better foundation for the current driver. I'm also not happy with the current situation, but the only alternative is to remove the driver completly. I'll try to find some time to improve the keyboard / touchpad initialisation in the next weeks. Marc [1] https://nv-tegra.nvidia.com/r/plugins/gitiles/linux-4.9/+/refs/heads/l4t/l4t-r31.0.1/drivers/i2c/busses/i2c-tegra-slave.c
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c index b3f114cb00dc..ec081d81a308 100644 --- a/drivers/staging/nvec/nvec.c +++ b/drivers/staging/nvec/nvec.c @@ -709,10 +709,11 @@ static irqreturn_t nvec_interrupt(int irq, void *dev) status & RNW ? " RNW" : ""); /* - * TODO: A correct fix needs to be found for this. + * TODO: replace the udelay with a read back after each writel above + * in order to work around a hardware issue, see i2c-tegra.c * - * We experience less incomplete messages with this delay than without - * it, but we don't know why. Help is appreciated. + * Unfortunately, this change causes an intialisation issue with the + * touchpad, which needs to be fixed first. */ udelay(100);
Update the comment before the udelay on how to replace it. Signed-off-by: Marc Dietrich <marvin24@gmx.de> --- drivers/staging/nvec/nvec.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) -- 2.40.1