linux-kernel - Re: [PATCH v3 3/3] serial: sc16is7xx: convert bitmask definitions to use BIT() macro

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Zsx9SqoWIkAyo7cD@smile.fi.intel.com>
Date: Mon, 26 Aug 2024 16:04:10 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Lech Perczak <lech.perczak@...lingroup.com>
Cc: linux-serial@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>,
	Hugo Villeneuve <hvilleneuve@...onoff.com>,
	Krzysztof Drobiński <k.drobinski@...lintechnologies.com>,
	Paweł Lenkow <pawel.lenkow@...lingroup.com>,
	Kirill Yatsenko <kirill.yatsenko@...lingroup.com>
Subject: Re: [PATCH v3 3/3] serial: sc16is7xx: convert bitmask definitions to
 use BIT() macro

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@...lingroup.com> 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