On Mon, Aug 26, 2024 at 02:35:37PM +0200, Lech Perczak wrote: > W dniu 23.08.2024 o 19:58, Andy Shevchenko pisze: > > On Fri, Aug 23, 2024 at 7:55 PM Lech Perczak > > <lech.perczak@xxxxxxxxxxxxxxx> wrote: ... > >> /* IIR register bits */ > >> -#define SC16IS7XX_IIR_NO_INT_BIT (1 << 0) /* No interrupts pending */ > >> -#define SC16IS7XX_IIR_ID_MASK 0x3e /* Mask for the interrupt ID */ > >> -#define SC16IS7XX_IIR_THRI_SRC 0x02 /* TX holding register empty */ > >> -#define SC16IS7XX_IIR_RDI_SRC 0x04 /* RX data interrupt */ > >> -#define SC16IS7XX_IIR_RLSE_SRC 0x06 /* RX line status error */ > >> -#define SC16IS7XX_IIR_RTOI_SRC 0x0c /* RX time-out interrupt */ > >> -#define SC16IS7XX_IIR_MSI_SRC 0x00 /* Modem status interrupt > >> - * - only on 75x/76x > >> - */ > >> -#define SC16IS7XX_IIR_INPIN_SRC 0x30 /* Input pin change of state > >> - * - only on 75x/76x > >> - */ > >> -#define SC16IS7XX_IIR_XOFFI_SRC 0x10 /* Received Xoff */ > >> -#define SC16IS7XX_IIR_CTSRTS_SRC 0x20 /* nCTS,nRTS change of state > >> - * from active (LOW) > >> - * to inactive (HIGH) > >> - */ > >> +#define SC16IS7XX_IIR_NO_INT_BIT BIT(0) /* No interrupts pending */ > >> +#define SC16IS7XX_IIR_ID_MASK GENMASK(5,1) /* Mask for the interrupt ID */ This is the only change (one definition / line) makes sense in this block. > > This is okay, but the rest of the bit combinations are better to have > > to be plain numbers as usually they are listed in this way in the > > datasheets. Note as well that 0x00 is a valid value which you can't > > express using BIT() or GENMASK() (and this is usually the main point > > to *not* convert them to these macros). > > > >> +#define SC16IS7XX_IIR_THRI_SRC BIT(1) /* TX holding register empty */ > >> +#define SC16IS7XX_IIR_RDI_SRC BIT(2) /* RX data interrupt */ > >> +#define SC16IS7XX_IIR_RLSE_SRC GENMASK(2,1) /* RX line status error */ > >> +#define SC16IS7XX_IIR_RTOI_SRC GENMASK(3,2) /* RX time-out interrupt */ > >> +#define SC16IS7XX_IIR_MSI_SRC 0x00 /* Modem status interrupt > >> + * - only on 75x/76x > >> + */ > >> +#define SC16IS7XX_IIR_INPIN_SRC GENMASK(5,4) /* Input pin change of state > >> + * - only on 75x/76x > >> + */ > >> +#define SC16IS7XX_IIR_XOFFI_SRC BIT(4) /* Received Xoff */ > >> +#define SC16IS7XX_IIR_CTSRTS_SRC BIT(5) /* nCTS,nRTS change of state > >> + * from active (LOW) > >> + * to inactive (HIGH) > >> + */ > > > Before I send out v4, do I get it right, that I should convert back SC16IS7XX_*_SRC > (i.e. interrupt source constants), and leave the rest as in v3? See above. I.o.w. change only _MASK and leave the rest as is. -- With Best Regards, Andy Shevchenko
