Corrected Peter Chen address - should be peter.chen@xxxxxxxxxx >>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 <christophe.jaillet@xxxxxxxxxx> > >Acked-by: Pawel Laszczak <pawell@xxxxxxxxxxx> > >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 > >>--- >>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