RE: [PATCH] usb: cdnsp: Fix the IMAN_IE_SET and IMAN_IE_CLEAR macro.
From: Pawel Laszczak <pawell@cadence.com>
Date: 2021-06-25 04:23:24
Also in:
kernel-janitors, lkml
IMAN_IE is BIT(1), so these macro are respectively equivalent to BIT(1)
and 0, whatever the value of 'p'.
The purpose was to set and reset a single bit in 'p'.
Fix these macros to do that correctly.
Fixes: e93e58d27402 ("usb: cdnsp: Device side header file for CDNSP driver")
Signed-off-by: Christophe JAILLET <redacted>Acked-by: Pawel Laszczak <pawell@cadence.com> Currently the bit 1(RW) and bit 0 (W1toClr) are implemented. All other bits are not used and are reserved for future RW implementations. I was a bit afraid about bit 0 which is "write 1 to clear" but I haven't found any issue with this patch during testing so we can simplify these macros Thanks Christophe
quoted hunk ↗ jump to hunk
--- This patch is speculative. It is not tested. Please, review with care. Actually, the usage of these macros is: readl(somewhere) set or reset the IMAN_IE bit write(somewhere) So it is likely that we want to preserve the other bits read. Otherwise, the code could be much simpler. --- drivers/usb/cdns3/cdnsp-gadget.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h index 783ca8ffde00..f740fa6089d8 100644 --- a/drivers/usb/cdns3/cdnsp-gadget.h +++ b/drivers/usb/cdns3/cdnsp-gadget.h@@ -383,8 +383,8 @@ struct cdnsp_intr_reg {#define IMAN_IE BIT(1) #define IMAN_IP BIT(0) /* bits 2:31 need to be preserved */ -#define IMAN_IE_SET(p) (((p) & IMAN_IE) | 0x2) -#define IMAN_IE_CLEAR(p) (((p) & IMAN_IE) & ~(0x2)) +#define IMAN_IE_SET(p) ((p) | IMAN_IE) +#define IMAN_IE_CLEAR(p) ((p) & ~IMAN_IE) /* IMOD - Interrupter Moderation Register - irq_control bitmasks. */ /* -- 2.30.2
-- Regards, Pawel Laszczak