[PATCH v2 0/3] serial: sc16is7xx: cosmetic cleanup

[PATCH v2 0/3] serial: sc16is7xx: cosmetic cleanup

[Lech Perczak]

When submitting previous, functional fixes, Tomasz Moń omitted those
two cosmetic patches, that kept lurking in our company tree - likely
by oversight. Let's submit them.
Also convert bitmask definitions to use BIT macro.

Signed-off-by: Lech Perczak <lech.perczak@camlingroup.com>
---
v2:
- Converted bitmask definitions to use BIT macro
  (thanks Jiri Slaby for the idea).
- Removed redundant comments in patch 2 altogether
- Fixed commit messages (thanks Andy Shevchenko for
  thorough review)

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: Andy Shevchenko <andy@kernel.org>

Lech Perczak (3):
  serial: sc16is7xx: remove SC16IS7XX_MSR_DELTA_MASK
  serial: sc16is7xx: fix copy-paste errors in EFR_SWFLOWx_BIT macros
  serial: sc16is7xx: convert bitmask definitions to use BIT macro

 drivers/tty/serial/sc16is7xx.c | 144 ++++++++++++++++-----------------
 1 file changed, 72 insertions(+), 72 deletions(-)


base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd
-- 
2.34.1

[PATCH v2 1/3] serial: sc16is7xx: remove SC16IS7XX_MSR_DELTA_MASK

[Lech Perczak]

This definition isn't used anywhere anymore, let's delete it.

Signed-off-by: Lech Perczak <lech.perczak@camlingroup.com>
---
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: Andy Shevchenko <andy@kernel.org>
---
 drivers/tty/serial/sc16is7xx.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index bf0065d1c8e9..8a2020f9930e 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -207,7 +207,6 @@
 #define SC16IS7XX_MSR_CD_BIT		(1 << 7) /* CD (IO6)
 						  * - only on 75x/76x
 						  */
-#define SC16IS7XX_MSR_DELTA_MASK	0x0F     /* Any of the delta bits! */
 
 /*
  * TCR register bits
-- 
2.34.1

Re: [PATCH v2 1/3] serial: sc16is7xx: remove SC16IS7XX_MSR_DELTA_MASK

[Andy Shevchenko]

On Mon, Aug 12, 2024 at 06:54:50PM +0200, Lech Perczak wrote:
> This definition isn't used anywhere anymore, let's delete it.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/3] serial: sc16is7xx: fix copy-paste errors in EFR_SWFLOWx_BIT macros

[Lech Perczak]

Comments attached to bits 0 and 1 incorrectly referenced bits 2 and 3,
which don't match the datasheet - fix them.
At the same time remove comments for individual macros, as they add
nothing to the definitions themselves.

Signed-off-by: Lech Perczak <lech.perczak@camlingroup.com>
---
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: Andy Shevchenko <andy@kernel.org>
---
 drivers/tty/serial/sc16is7xx.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 8a2020f9930e..36b7c682ae94 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -266,9 +266,9 @@
 						  * and writing to IER[7:4],
 						  * FCR[5:4], MCR[7:5]
 						  */
-#define SC16IS7XX_EFR_SWFLOW3_BIT	(1 << 3) /* SWFLOW bit 3 */
-#define SC16IS7XX_EFR_SWFLOW2_BIT	(1 << 2) /* SWFLOW bit 2
-						  *
+#define SC16IS7XX_EFR_SWFLOW3_BIT	(1 << 3)
+#define SC16IS7XX_EFR_SWFLOW2_BIT	(1 << 2)
+						 /*
 						  * SWFLOW bits 3 & 2 table:
 						  * 00 -> no transmitter flow
 						  *       control
@@ -280,10 +280,10 @@
 						  *       XON1, XON2, XOFF1 and
 						  *       XOFF2
 						  */
-#define SC16IS7XX_EFR_SWFLOW1_BIT	(1 << 1) /* SWFLOW bit 2 */
-#define SC16IS7XX_EFR_SWFLOW0_BIT	(1 << 0) /* SWFLOW bit 3
-						  *
-						  * SWFLOW bits 3 & 2 table:
+#define SC16IS7XX_EFR_SWFLOW1_BIT	(1 << 1)
+#define SC16IS7XX_EFR_SWFLOW0_BIT	(1 << 0)
+						 /*
+						  * SWFLOW bits 1 & 0 table:
 						  * 00 -> no received flow
 						  *       control
 						  * 01 -> receiver compares
-- 
2.34.1

Re: [PATCH v2 2/3] serial: sc16is7xx: fix copy-paste errors in EFR_SWFLOWx_BIT macros

[Andy Shevchenko]

On Mon, Aug 12, 2024 at 06:56:02PM +0200, Lech Perczak wrote:
> Comments attached to bits 0 and 1 incorrectly referenced bits 2 and 3,
> which don't match the datasheet - fix them.
> At the same time remove comments for individual macros, as they add
> nothing to the definitions themselves.

Strictly speaking we use term "definitions" or "constants" for the defined
constants instead of "macros" (see Subject).

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 3/3] serial: sc16is7xx: convert bitmask definitions to use BIT macro

[Lech Perczak]

Now that bit definition comments were cleaned up, convert bitmask
definitions to use BIT macro for clarity.

Signed-off-by: Lech Perczak <lech.perczak@camlingroup.com>
---
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: Andy Shevchenko <andy@kernel.org>
---
 drivers/tty/serial/sc16is7xx.c | 137 +++++++++++++++++----------------
 1 file changed, 69 insertions(+), 68 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 36b7c682ae94..a3c2ae112090 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -10,6 +10,7 @@
 #undef DEFAULT_SYMBOL_NAMESPACE
 #define DEFAULT_SYMBOL_NAMESPACE SERIAL_NXP_SC16IS7XX
 
+#include <linux/bits.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -77,33 +78,33 @@
 #define SC16IS7XX_XOFF2_REG		(0x07) /* Xoff2 word */
 
 /* IER register bits */
-#define SC16IS7XX_IER_RDI_BIT		(1 << 0) /* Enable RX data interrupt */
-#define SC16IS7XX_IER_THRI_BIT		(1 << 1) /* Enable TX holding register
+#define SC16IS7XX_IER_RDI_BIT		BIT(0)   /* Enable RX data interrupt */
+#define SC16IS7XX_IER_THRI_BIT		BIT(1)   /* Enable TX holding register
 						  * interrupt */
-#define SC16IS7XX_IER_RLSI_BIT		(1 << 2) /* Enable RX line status
+#define SC16IS7XX_IER_RLSI_BIT		BIT(2)   /* Enable RX line status
 						  * interrupt */
-#define SC16IS7XX_IER_MSI_BIT		(1 << 3) /* Enable Modem status
+#define SC16IS7XX_IER_MSI_BIT		BIT(3)   /* Enable Modem status
 						  * interrupt */
 
 /* IER register bits - write only if (EFR[4] == 1) */
-#define SC16IS7XX_IER_SLEEP_BIT		(1 << 4) /* Enable Sleep mode */
-#define SC16IS7XX_IER_XOFFI_BIT		(1 << 5) /* Enable Xoff interrupt */
-#define SC16IS7XX_IER_RTSI_BIT		(1 << 6) /* Enable nRTS interrupt */
-#define SC16IS7XX_IER_CTSI_BIT		(1 << 7) /* Enable nCTS interrupt */
+#define SC16IS7XX_IER_SLEEP_BIT		BIT(4)   /* Enable Sleep mode */
+#define SC16IS7XX_IER_XOFFI_BIT		BIT(5)   /* Enable Xoff interrupt */
+#define SC16IS7XX_IER_RTSI_BIT		BIT(6)   /* Enable nRTS interrupt */
+#define SC16IS7XX_IER_CTSI_BIT		BIT(7)   /* Enable nCTS interrupt */
 
 /* FCR register bits */
-#define SC16IS7XX_FCR_FIFO_BIT		(1 << 0) /* Enable FIFO */
-#define SC16IS7XX_FCR_RXRESET_BIT	(1 << 1) /* Reset RX FIFO */
-#define SC16IS7XX_FCR_TXRESET_BIT	(1 << 2) /* Reset TX FIFO */
-#define SC16IS7XX_FCR_RXLVLL_BIT	(1 << 6) /* RX Trigger level LSB */
-#define SC16IS7XX_FCR_RXLVLH_BIT	(1 << 7) /* RX Trigger level MSB */
+#define SC16IS7XX_FCR_FIFO_BIT		BIT(0)   /* Enable FIFO */
+#define SC16IS7XX_FCR_RXRESET_BIT	BIT(1)   /* Reset RX FIFO */
+#define SC16IS7XX_FCR_TXRESET_BIT	BIT(2)   /* Reset TX FIFO */
+#define SC16IS7XX_FCR_RXLVLL_BIT	BIT(6)   /* RX Trigger level LSB */
+#define SC16IS7XX_FCR_RXLVLH_BIT	BIT(7)   /* RX Trigger level MSB */
 
 /* FCR register bits - write only if (EFR[4] == 1) */
-#define SC16IS7XX_FCR_TXLVLL_BIT	(1 << 4) /* TX Trigger level LSB */
-#define SC16IS7XX_FCR_TXLVLH_BIT	(1 << 5) /* TX Trigger level MSB */
+#define SC16IS7XX_FCR_TXLVLL_BIT	BIT(4)   /* TX Trigger level LSB */
+#define SC16IS7XX_FCR_TXLVLH_BIT	BIT(5)   /* TX Trigger level MSB */
 
 /* IIR register bits */
-#define SC16IS7XX_IIR_NO_INT_BIT	(1 << 0) /* No interrupts pending */
+#define SC16IS7XX_IIR_NO_INT_BIT	BIT(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 */
@@ -121,8 +122,8 @@
 						  * to inactive (HIGH)
 						  */
 /* LCR register bits */
-#define SC16IS7XX_LCR_LENGTH0_BIT	(1 << 0) /* Word length bit 0 */
-#define SC16IS7XX_LCR_LENGTH1_BIT	(1 << 1) /* Word length bit 1
+#define SC16IS7XX_LCR_LENGTH0_BIT	BIT(0)   /* Word length bit 0 */
+#define SC16IS7XX_LCR_LENGTH1_BIT	BIT(1)   /* Word length bit 1
 						  *
 						  * Word length bits table:
 						  * 00 -> 5 bit words
@@ -130,7 +131,7 @@
 						  * 10 -> 7 bit words
 						  * 11 -> 8 bit words
 						  */
-#define SC16IS7XX_LCR_STOPLEN_BIT	(1 << 2) /* STOP length bit
+#define SC16IS7XX_LCR_STOPLEN_BIT	BIT(2)   /* STOP length bit
 						  *
 						  * STOP length bit table:
 						  * 0 -> 1 stop bit
@@ -138,11 +139,11 @@
 						  *      word length is 5,
 						  *      2 stop bits otherwise
 						  */
-#define SC16IS7XX_LCR_PARITY_BIT	(1 << 3) /* Parity bit enable */
-#define SC16IS7XX_LCR_EVENPARITY_BIT	(1 << 4) /* Even parity bit enable */
-#define SC16IS7XX_LCR_FORCEPARITY_BIT	(1 << 5) /* 9-bit multidrop parity */
-#define SC16IS7XX_LCR_TXBREAK_BIT	(1 << 6) /* TX break enable */
-#define SC16IS7XX_LCR_DLAB_BIT		(1 << 7) /* Divisor Latch enable */
+#define SC16IS7XX_LCR_PARITY_BIT	BIT(3)   /* Parity bit enable */
+#define SC16IS7XX_LCR_EVENPARITY_BIT	BIT(4)   /* Even parity bit enable */
+#define SC16IS7XX_LCR_FORCEPARITY_BIT	BIT(5)   /* 9-bit multidrop parity */
+#define SC16IS7XX_LCR_TXBREAK_BIT	BIT(6)   /* TX break enable */
+#define SC16IS7XX_LCR_DLAB_BIT		BIT(7)   /* Divisor Latch enable */
 #define SC16IS7XX_LCR_WORD_LEN_5	(0x00)
 #define SC16IS7XX_LCR_WORD_LEN_6	(0x01)
 #define SC16IS7XX_LCR_WORD_LEN_7	(0x02)
@@ -153,58 +154,58 @@
 								* reg set */
 
 /* MCR register bits */
-#define SC16IS7XX_MCR_DTR_BIT		(1 << 0) /* DTR complement
+#define SC16IS7XX_MCR_DTR_BIT		BIT(0)   /* DTR complement
 						  * - only on 75x/76x
 						  */
-#define SC16IS7XX_MCR_RTS_BIT		(1 << 1) /* RTS complement */
-#define SC16IS7XX_MCR_TCRTLR_BIT	(1 << 2) /* TCR/TLR register enable */
-#define SC16IS7XX_MCR_LOOP_BIT		(1 << 4) /* Enable loopback test mode */
-#define SC16IS7XX_MCR_XONANY_BIT	(1 << 5) /* Enable Xon Any
+#define SC16IS7XX_MCR_RTS_BIT		BIT(1)   /* RTS complement */
+#define SC16IS7XX_MCR_TCRTLR_BIT	BIT(2)   /* TCR/TLR register enable */
+#define SC16IS7XX_MCR_LOOP_BIT		BIT(4)   /* Enable loopback test mode */
+#define SC16IS7XX_MCR_XONANY_BIT	BIT(5)   /* Enable Xon Any
 						  * - write enabled
 						  * if (EFR[4] == 1)
 						  */
-#define SC16IS7XX_MCR_IRDA_BIT		(1 << 6) /* Enable IrDA mode
+#define SC16IS7XX_MCR_IRDA_BIT		BIT(6)   /* Enable IrDA mode
 						  * - write enabled
 						  * if (EFR[4] == 1)
 						  */
-#define SC16IS7XX_MCR_CLKSEL_BIT	(1 << 7) /* Divide clock by 4
+#define SC16IS7XX_MCR_CLKSEL_BIT	BIT(7)   /* Divide clock by 4
 						  * - write enabled
 						  * if (EFR[4] == 1)
 						  */
 
 /* LSR register bits */
-#define SC16IS7XX_LSR_DR_BIT		(1 << 0) /* Receiver data ready */
-#define SC16IS7XX_LSR_OE_BIT		(1 << 1) /* Overrun Error */
-#define SC16IS7XX_LSR_PE_BIT		(1 << 2) /* Parity Error */
-#define SC16IS7XX_LSR_FE_BIT		(1 << 3) /* Frame Error */
-#define SC16IS7XX_LSR_BI_BIT		(1 << 4) /* Break Interrupt */
+#define SC16IS7XX_LSR_DR_BIT		BIT(0)   /* Receiver data ready */
+#define SC16IS7XX_LSR_OE_BIT		BIT(1)   /* Overrun Error */
+#define SC16IS7XX_LSR_PE_BIT		BIT(2)   /* Parity Error */
+#define SC16IS7XX_LSR_FE_BIT		BIT(3)   /* Frame Error */
+#define SC16IS7XX_LSR_BI_BIT		BIT(4)   /* Break Interrupt */
 #define SC16IS7XX_LSR_BRK_ERROR_MASK	0x1E     /* BI, FE, PE, OE bits */
-#define SC16IS7XX_LSR_THRE_BIT		(1 << 5) /* TX holding register empty */
-#define SC16IS7XX_LSR_TEMT_BIT		(1 << 6) /* Transmitter empty */
-#define SC16IS7XX_LSR_FIFOE_BIT		(1 << 7) /* Fifo Error */
+#define SC16IS7XX_LSR_THRE_BIT		BIT(5)   /* TX holding register empty */
+#define SC16IS7XX_LSR_TEMT_BIT		BIT(6)   /* Transmitter empty */
+#define SC16IS7XX_LSR_FIFOE_BIT		BIT(7)   /* Fifo Error */
 
 /* MSR register bits */
-#define SC16IS7XX_MSR_DCTS_BIT		(1 << 0) /* Delta CTS Clear To Send */
-#define SC16IS7XX_MSR_DDSR_BIT		(1 << 1) /* Delta DSR Data Set Ready
+#define SC16IS7XX_MSR_DCTS_BIT		BIT(0)   /* Delta CTS Clear To Send */
+#define SC16IS7XX_MSR_DDSR_BIT		BIT(1)   /* Delta DSR Data Set Ready
 						  * or (IO4)
 						  * - only on 75x/76x
 						  */
-#define SC16IS7XX_MSR_DRI_BIT		(1 << 2) /* Delta RI Ring Indicator
+#define SC16IS7XX_MSR_DRI_BIT		BIT(2)   /* Delta RI Ring Indicator
 						  * or (IO7)
 						  * - only on 75x/76x
 						  */
-#define SC16IS7XX_MSR_DCD_BIT		(1 << 3) /* Delta CD Carrier Detect
+#define SC16IS7XX_MSR_DCD_BIT		BIT(3)   /* Delta CD Carrier Detect
 						  * or (IO6)
 						  * - only on 75x/76x
 						  */
-#define SC16IS7XX_MSR_CTS_BIT		(1 << 4) /* CTS */
-#define SC16IS7XX_MSR_DSR_BIT		(1 << 5) /* DSR (IO4)
+#define SC16IS7XX_MSR_CTS_BIT		BIT(4)   /* CTS */
+#define SC16IS7XX_MSR_DSR_BIT		BIT(5)   /* DSR (IO4)
 						  * - only on 75x/76x
 						  */
-#define SC16IS7XX_MSR_RI_BIT		(1 << 6) /* RI (IO7)
+#define SC16IS7XX_MSR_RI_BIT		BIT(6)   /* RI (IO7)
 						  * - only on 75x/76x
 						  */
-#define SC16IS7XX_MSR_CD_BIT		(1 << 7) /* CD (IO6)
+#define SC16IS7XX_MSR_CD_BIT		BIT(7)   /* CD (IO6)
 						  * - only on 75x/76x
 						  */
 
@@ -239,19 +240,19 @@
 #define SC16IS7XX_TLR_RX_TRIGGER(words)	((((words) / 4) & 0x0f) << 4)
 
 /* IOControl register bits (Only 75x/76x) */
-#define SC16IS7XX_IOCONTROL_LATCH_BIT	(1 << 0) /* Enable input latching */
-#define SC16IS7XX_IOCONTROL_MODEM_A_BIT	(1 << 1) /* Enable GPIO[7:4] as modem A pins */
-#define SC16IS7XX_IOCONTROL_MODEM_B_BIT	(1 << 2) /* Enable GPIO[3:0] as modem B pins */
-#define SC16IS7XX_IOCONTROL_SRESET_BIT	(1 << 3) /* Software Reset */
+#define SC16IS7XX_IOCONTROL_LATCH_BIT	BIT(0)   /* Enable input latching */
+#define SC16IS7XX_IOCONTROL_MODEM_A_BIT	BIT(1)   /* Enable GPIO[7:4] as modem A pins */
+#define SC16IS7XX_IOCONTROL_MODEM_B_BIT	BIT(2)   /* Enable GPIO[3:0] as modem B pins */
+#define SC16IS7XX_IOCONTROL_SRESET_BIT	BIT(3)   /* Software Reset */
 
 /* EFCR register bits */
-#define SC16IS7XX_EFCR_9BIT_MODE_BIT	(1 << 0) /* Enable 9-bit or Multidrop
+#define SC16IS7XX_EFCR_9BIT_MODE_BIT	BIT(0)   /* Enable 9-bit or Multidrop
 						  * mode (RS485) */
-#define SC16IS7XX_EFCR_RXDISABLE_BIT	(1 << 1) /* Disable receiver */
-#define SC16IS7XX_EFCR_TXDISABLE_BIT	(1 << 2) /* Disable transmitter */
-#define SC16IS7XX_EFCR_AUTO_RS485_BIT	(1 << 4) /* Auto RS485 RTS direction */
-#define SC16IS7XX_EFCR_RTS_INVERT_BIT	(1 << 5) /* RTS output inversion */
-#define SC16IS7XX_EFCR_IRDA_MODE_BIT	(1 << 7) /* IrDA mode
+#define SC16IS7XX_EFCR_RXDISABLE_BIT	BIT(1)   /* Disable receiver */
+#define SC16IS7XX_EFCR_TXDISABLE_BIT	BIT(2)   /* Disable transmitter */
+#define SC16IS7XX_EFCR_AUTO_RS485_BIT	BIT(4)   /* Auto RS485 RTS direction */
+#define SC16IS7XX_EFCR_RTS_INVERT_BIT	BIT(5)   /* RTS output inversion */
+#define SC16IS7XX_EFCR_IRDA_MODE_BIT	BIT(7)   /* IrDA mode
 						  * 0 = rate upto 115.2 kbit/s
 						  *   - Only 75x/76x
 						  * 1 = rate upto 1.152 Mbit/s
@@ -259,15 +260,15 @@
 						  */
 
 /* EFR register bits */
-#define SC16IS7XX_EFR_AUTORTS_BIT	(1 << 6) /* Auto RTS flow ctrl enable */
-#define SC16IS7XX_EFR_AUTOCTS_BIT	(1 << 7) /* Auto CTS flow ctrl enable */
-#define SC16IS7XX_EFR_XOFF2_DETECT_BIT	(1 << 5) /* Enable Xoff2 detection */
-#define SC16IS7XX_EFR_ENABLE_BIT	(1 << 4) /* Enable enhanced functions
+#define SC16IS7XX_EFR_AUTORTS_BIT	BIT(6)   /* Auto RTS flow ctrl enable */
+#define SC16IS7XX_EFR_AUTOCTS_BIT	BIT(7)   /* Auto CTS flow ctrl enable */
+#define SC16IS7XX_EFR_XOFF2_DETECT_BIT	BIT(5)   /* Enable Xoff2 detection */
+#define SC16IS7XX_EFR_ENABLE_BIT	BIT(4)   /* Enable enhanced functions
 						  * and writing to IER[7:4],
 						  * FCR[5:4], MCR[7:5]
 						  */
-#define SC16IS7XX_EFR_SWFLOW3_BIT	(1 << 3)
-#define SC16IS7XX_EFR_SWFLOW2_BIT	(1 << 2)
+#define SC16IS7XX_EFR_SWFLOW3_BIT	BIT(3)
+#define SC16IS7XX_EFR_SWFLOW2_BIT	BIT(2)
 						 /*
 						  * SWFLOW bits 3 & 2 table:
 						  * 00 -> no transmitter flow
@@ -280,8 +281,8 @@
 						  *       XON1, XON2, XOFF1 and
 						  *       XOFF2
 						  */
-#define SC16IS7XX_EFR_SWFLOW1_BIT	(1 << 1)
-#define SC16IS7XX_EFR_SWFLOW0_BIT	(1 << 0)
+#define SC16IS7XX_EFR_SWFLOW1_BIT	BIT(1)
+#define SC16IS7XX_EFR_SWFLOW0_BIT	BIT(0)
 						 /*
 						  * SWFLOW bits 1 & 0 table:
 						  * 00 -> no received flow
@@ -307,9 +308,9 @@
 #define SC16IS7XX_FIFO_SIZE		(64)
 #define SC16IS7XX_GPIOS_PER_BANK	4
 
-#define SC16IS7XX_RECONF_MD		(1 << 0)
-#define SC16IS7XX_RECONF_IER		(1 << 1)
-#define SC16IS7XX_RECONF_RS485		(1 << 2)
+#define SC16IS7XX_RECONF_MD		BIT(0)
+#define SC16IS7XX_RECONF_IER		BIT(1)
+#define SC16IS7XX_RECONF_RS485		BIT(2)
 
 struct sc16is7xx_one_config {
 	unsigned int			flags;
-- 
2.34.1

Re: [PATCH v2 3/3] serial: sc16is7xx: convert bitmask definitions to use BIT macro

[Andy Shevchenko]

On Mon, Aug 12, 2024 at 06:56:53PM +0200, Lech Perczak wrote:
> Now that bit definition comments were cleaned up, convert bitmask
> definitions to use BIT macro for clarity.

In the Subject and here: BIT()

...

>  #define SC16IS7XX_IIR_ID_MASK		0x3e     /* Mask for the interrupt ID */

While at it, why not convert the MASK(s) to use GENMASK()?
It's logically coupled change, no need to have a separate patch.

...

>  #define SC16IS7XX_LSR_BRK_ERROR_MASK	0x1E     /* BI, FE, PE, OE bits */

Ditto.

(maybe more that are not visible in the context of this patch)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/3] serial: sc16is7xx: convert bitmask definitions to use BIT macro

[Ilpo Järvinen]

On Mon, 12 Aug 2024, Andy Shevchenko wrote:

> On Mon, Aug 12, 2024 at 06:56:53PM +0200, Lech Perczak wrote:
> > Now that bit definition comments were cleaned up, convert bitmask
> > definitions to use BIT macro for clarity.
> 
> In the Subject and here: BIT()
> 
> ...
> 
> >  #define SC16IS7XX_IIR_ID_MASK		0x3e     /* Mask for the interrupt ID */
> 
> While at it, why not convert the MASK(s) to use GENMASK()?
> It's logically coupled change, no need to have a separate patch.
> 
> ...
> 
> >  #define SC16IS7XX_LSR_BRK_ERROR_MASK	0x1E     /* BI, FE, PE, OE bits */
> 
> Ditto.

No. This is not GENMASK() but combination of other bits. This mask define 
should be a combination those bit defines and the comment dropped because 
it adds no value after using the bit defines directly.

> (maybe more that are not visible in the context of this patch)


-- 
 i.

Re: [PATCH v2 3/3] serial: sc16is7xx: convert bitmask definitions to use BIT macro

[Andy Shevchenko]

On Tue, Aug 13, 2024 at 11:30:57AM +0300, Ilpo Järvinen wrote:
> On Mon, 12 Aug 2024, Andy Shevchenko wrote:
> > On Mon, Aug 12, 2024 at 06:56:53PM +0200, Lech Perczak wrote:

...

> > >  #define SC16IS7XX_IIR_ID_MASK		0x3e     /* Mask for the interrupt ID */
> > 
> > While at it, why not convert the MASK(s) to use GENMASK()?
> > It's logically coupled change, no need to have a separate patch.

...

> > >  #define SC16IS7XX_LSR_BRK_ERROR_MASK	0x1E     /* BI, FE, PE, OE bits */
> > 
> > Ditto.
> 
> No. This is not GENMASK() but combination of other bits. This mask define 
> should be a combination those bit defines and the comment dropped because 
> it adds no value after using the bit defines directly.

I agree with you on this. I'm pretty fine with either approach.

> > (maybe more that are not visible in the context of this patch)

-- 
With Best Regards,
Andy Shevchenko