Shubhrajyoti D <shubhrajy...@ti.com> writes:

> For OMAP4 Interrupt enable register is a legacy register.

I don't see anything in the docs mentioning this is legacy.  In fact,
that register is used extensivly throughout the driver, even for OMAP4.

I think the CLR/SET registers were added to aid atomically
setting/clearing specific interrupts, but when disabling all, I don't
see why I2C_IE cannot be used.

For that reason, any reason why the 4430-specific check cannot simply be
removed to fix this interrupt clearing bug?

> To clear the interrupts we were writing 0 to it. 

This patch still writes 0 to it, so I'm not seeing the point of this comment.

> However on OMAP4 we were writing 1 to IRQENABLE_CLR which clears only
> the arbitration lost interrupt. The patch intends to fix the same by
> writing 1 to all the bits.

This is the bug, and should come first in the changelog so readers know
what the problem is.

> Signed-off-by: Shubhrajyoti D <shubhrajy...@ti.com>

I believe this patch was originally from a fix by Vikram Pandita.  Even
if you've now changed the implementation, please credit the original
author (and Cc them) in the changelog.  It's common practice (and common
courtesy) to say something like "Based on an a patch originally from
Author <email>".  Thanks.

Also, this patch doesn't apply to mainline or linux-omap master.  Can
you please update?

Thanks,

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to