| 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
| ||
|
Message-ID: <aMGb8RzMGMKf/0hH@lizhi-Precision-Tower-5810>
Date: Wed, 10 Sep 2025 11:40:33 -0400
From: Frank Li <Frank.li@....com>
To: Dario Binacchi <dario.binacchi@...rulasolutions.com>
Cc: linux-kernel@...r.kernel.org, linux-amarula@...rulasolutions.com,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Fabio Estevam <festevam@...il.com>,
Michael Trimarchi <michael@...rulasolutions.com>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Sascha Hauer <s.hauer@...gutronix.de>,
Shawn Guo <shawnguo@...nel.org>, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-input@...r.kernel.org
Subject: Re: [RESEND PATCH 4/4] Input: imx6ul_tsc - use BIT, FIELD_{GET,PREP}
and GENMASK macros
On Wed, Sep 10, 2025 at 03:58:38PM +0200, Dario Binacchi wrote:
> Replace opencoded masking and shifting, with BIT(), GENMASK(),
> FIELD_GET() and FIELD_PREP() macros.
Add "No functional change." to help filter out if need bisect.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@...rulasolutions.com>
>
> ---
This cleanup patch should follow patch1, which is typo fix.
Reviewed-by: Frank Li <Frank.Li@....com>
>
> drivers/input/touchscreen/imx6ul_tsc.c | 88 ++++++++++++++------------
> 1 file changed, 48 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
> index a6066643bd48..e74999c5e22f 100644
> --- a/drivers/input/touchscreen/imx6ul_tsc.c
> +++ b/drivers/input/touchscreen/imx6ul_tsc.c
> @@ -21,25 +21,23 @@
> #include <linux/log2.h>
>
> /* ADC configuration registers field define */
> -#define ADC_AIEN (0x1 << 7)
> +#define ADC_AIEN BIT(7)
> +#define ADC_ADCH_MASK GENMASK(4, 0)
> #define ADC_CONV_DISABLE 0x1F
> -#define ADC_AVGE (0x1 << 5)
> -#define ADC_CAL (0x1 << 7)
> -#define ADC_CALF 0x2
> -#define ADC_12BIT_MODE (0x2 << 2)
> -#define ADC_CONV_MODE_MASK (0x3 << 2)
> +#define ADC_AVGE BIT(5)
> +#define ADC_CAL BIT(7)
> +#define ADC_CALF BIT(1)
> +#define ADC_CONV_MODE_MASK GENMASK(3, 2)
> +#define ADC_12BIT_MODE 0x2
> #define ADC_IPG_CLK 0x00
> -#define ADC_INPUT_CLK_MASK 0x3
> -#define ADC_CLK_DIV_8 (0x03 << 5)
> -#define ADC_CLK_DIV_MASK (0x3 << 5)
> -#define ADC_SHORT_SAMPLE_MODE (0x0 << 4)
> -#define ADC_SAMPLE_MODE_MASK (0x1 << 4)
> -#define ADC_HARDWARE_TRIGGER (0x1 << 13)
> -#define ADC_AVGS_SHIFT 14
> -#define ADC_AVGS_MASK (0x3 << 14)
> +#define ADC_INPUT_CLK_MASK GENMASK(1, 0)
> +#define ADC_CLK_DIV_8 0x03
> +#define ADC_CLK_DIV_MASK GENMASK(6, 5)
> +#define ADC_SAMPLE_MODE BIT(4)
> +#define ADC_HARDWARE_TRIGGER BIT(13)
> +#define ADC_AVGS_MASK GENMASK(15, 14)
> #define SELECT_CHANNEL_4 0x04
> #define SELECT_CHANNEL_1 0x01
> -#define DISABLE_CONVERSION_INT (0x0 << 7)
>
> /* ADC registers */
> #define REG_ADC_HC0 0x00
> @@ -66,20 +64,26 @@
> #define REG_TSC_DEBUG_MODE 0x70
> #define REG_TSC_DEBUG_MODE2 0x80
>
> +/* TSC_MEASURE_VALUE register field define */
> +#define X_VALUE_MASK GENMASK(27, 16)
> +#define Y_VALUE_MASK GENMASK(11, 0)
> +
> /* TSC configuration registers field define */
> -#define DETECT_4_WIRE_MODE (0x0 << 4)
> -#define AUTO_MEASURE 0x1
> -#define MEASURE_SIGNAL 0x1
> -#define DETECT_SIGNAL (0x1 << 4)
> -#define VALID_SIGNAL (0x1 << 8)
> -#define MEASURE_INT_EN 0x1
> -#define MEASURE_SIG_EN 0x1
> -#define VALID_SIG_EN (0x1 << 8)
> +#define MEASURE_DELAY_TIME_MASK GENMASK(31, 8)
> +#define DETECT_5_WIRE_MODE BIT(4)
> +#define AUTO_MEASURE BIT(0)
> +#define MEASURE_SIGNAL BIT(0)
> +#define DETECT_SIGNAL BIT(4)
> +#define VALID_SIGNAL BIT(8)
> +#define MEASURE_INT_EN BIT(0)
> +#define MEASURE_SIG_EN BIT(0)
> +#define VALID_SIG_EN BIT(8)
> #define DE_GLITCH_MASK GENMASK(30, 29)
> #define DE_GLITCH_DEF 0x02
> -#define START_SENSE (0x1 << 12)
> -#define TSC_DISABLE (0x1 << 16)
> +#define START_SENSE BIT(12)
> +#define TSC_DISABLE BIT(16)
> #define DETECT_MODE 0x2
> +#define STATE_MACHINE_MASK GENMASK(22, 20)
>
> struct imx6ul_tsc {
> struct device *dev;
> @@ -115,19 +119,20 @@ static int imx6ul_adc_init(struct imx6ul_tsc *tsc)
>
> adc_cfg = readl(tsc->adc_regs + REG_ADC_CFG);
> adc_cfg &= ~(ADC_CONV_MODE_MASK | ADC_INPUT_CLK_MASK);
> - adc_cfg |= ADC_12BIT_MODE | ADC_IPG_CLK;
> - adc_cfg &= ~(ADC_CLK_DIV_MASK | ADC_SAMPLE_MODE_MASK);
> - adc_cfg |= ADC_CLK_DIV_8 | ADC_SHORT_SAMPLE_MODE;
> + adc_cfg |= FIELD_PREP(ADC_CONV_MODE_MASK, ADC_12BIT_MODE) |
> + FIELD_PREP(ADC_INPUT_CLK_MASK, ADC_IPG_CLK);
> + adc_cfg &= ~(ADC_CLK_DIV_MASK | ADC_SAMPLE_MODE);
> + adc_cfg |= FIELD_PREP(ADC_CLK_DIV_MASK, ADC_CLK_DIV_8);
> if (tsc->average_enable) {
> adc_cfg &= ~ADC_AVGS_MASK;
> - adc_cfg |= (tsc->average_select) << ADC_AVGS_SHIFT;
> + adc_cfg |= FIELD_PREP(ADC_AVGS_MASK, tsc->average_select);
> }
> adc_cfg &= ~ADC_HARDWARE_TRIGGER;
> writel(adc_cfg, tsc->adc_regs + REG_ADC_CFG);
>
> /* enable calibration interrupt */
> adc_hc |= ADC_AIEN;
> - adc_hc |= ADC_CONV_DISABLE;
> + adc_hc |= FIELD_PREP(ADC_ADCH_MASK, ADC_CONV_DISABLE);
> writel(adc_hc, tsc->adc_regs + REG_ADC_HC0);
>
> /* start ADC calibration */
> @@ -167,19 +172,21 @@ static void imx6ul_tsc_channel_config(struct imx6ul_tsc *tsc)
> {
> u32 adc_hc0, adc_hc1, adc_hc2, adc_hc3, adc_hc4;
>
> - adc_hc0 = DISABLE_CONVERSION_INT;
> + adc_hc0 = FIELD_PREP(ADC_AIEN, 0);
> writel(adc_hc0, tsc->adc_regs + REG_ADC_HC0);
>
> - adc_hc1 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_4;
> + adc_hc1 = FIELD_PREP(ADC_AIEN, 0) |
> + FIELD_PREP(ADC_ADCH_MASK, SELECT_CHANNEL_4);
> writel(adc_hc1, tsc->adc_regs + REG_ADC_HC1);
>
> - adc_hc2 = DISABLE_CONVERSION_INT;
> + adc_hc2 = FIELD_PREP(ADC_AIEN, 0);
> writel(adc_hc2, tsc->adc_regs + REG_ADC_HC2);
>
> - adc_hc3 = DISABLE_CONVERSION_INT | SELECT_CHANNEL_1;
> + adc_hc3 = FIELD_PREP(ADC_AIEN, 0) |
> + FIELD_PREP(ADC_ADCH_MASK, SELECT_CHANNEL_1);
> writel(adc_hc3, tsc->adc_regs + REG_ADC_HC3);
>
> - adc_hc4 = DISABLE_CONVERSION_INT;
> + adc_hc4 = FIELD_PREP(ADC_AIEN, 0);
> writel(adc_hc4, tsc->adc_regs + REG_ADC_HC4);
> }
>
> @@ -194,8 +201,9 @@ static void imx6ul_tsc_set(struct imx6ul_tsc *tsc)
> u32 debug_mode2;
> u32 start;
>
> - basic_setting |= tsc->measure_delay_time << 8;
> - basic_setting |= DETECT_4_WIRE_MODE | AUTO_MEASURE;
> + basic_setting |= FIELD_PREP(MEASURE_DELAY_TIME_MASK,
> + tsc->measure_delay_time);
> + basic_setting |= AUTO_MEASURE;
> writel(basic_setting, tsc->tsc_regs + REG_TSC_BASIC_SETTING);
>
> debug_mode2 = FIELD_PREP(DE_GLITCH_MASK, tsc->de_glitch);
> @@ -255,7 +263,7 @@ static bool tsc_wait_detect_mode(struct imx6ul_tsc *tsc)
>
> usleep_range(200, 400);
> debug_mode2 = readl(tsc->tsc_regs + REG_TSC_DEBUG_MODE2);
> - state_machine = (debug_mode2 >> 20) & 0x7;
> + state_machine = FIELD_GET(STATE_MACHINE_MASK, debug_mode2);
> } while (state_machine != DETECT_MODE);
>
> usleep_range(200, 400);
> @@ -283,8 +291,8 @@ static irqreturn_t tsc_irq_fn(int irq, void *dev_id)
>
> if (status & MEASURE_SIGNAL) {
> value = readl(tsc->tsc_regs + REG_TSC_MEASURE_VALUE);
> - x = (value >> 16) & 0x0fff;
> - y = value & 0x0fff;
> + x = FIELD_GET(X_VALUE_MASK, value);
> + y = FIELD_GET(Y_VALUE_MASK, value);
>
> /*
> * In detect mode, we can get the xnur gpio value,
> --
> 2.43.0
>
Powered by blists - more mailing lists