Re: [PATCH 4/5] Input: aw86927 - handle the AW8693X play and boost register layout

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Input: aw86927 - 1-bit AW86938_PLAYCFG1_BST_MODE_MASK in aw86927_play_mode()
--

commit c20411cd705818dd58facc17d48437366131a42a
Author: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

Input: aw86927 - handle the AW8693X play and boost register layout

This commit updates the aw86927 driver to handle the play, boost, VBAT,
and two-step stop register layout differences on AW86937 and AW86938
devices.

> diff --git a/drivers/input/misc/aw86927.c b/drivers/input/misc/aw86927.c
> index 42eb531c7e3b3..e8fd880ac1ded 100644
> --- a/drivers/input/misc/aw86927.c
> +++ b/drivers/input/misc/aw86927.c

[ ... ]

> @@ -291,9 +299,14 @@ static int aw86927_play_mode(struct aw86927_data *haptics, u8 play_mode)
>  
>  		err = regmap_update_bits(haptics->regmap,
>  					 AW86927_PLAYCFG1_REG,
> -					 AW86927_PLAYCFG1_BST_MODE_MASK,
> +					 haptics->model == AW86927 ?
> +					 AW86927_PLAYCFG1_BST_MODE_MASK :
> +					 AW86938_PLAYCFG1_BST_MODE_MASK,
> +					 haptics->model == AW86927 ?
>  					 FIELD_PREP(AW86927_PLAYCFG1_BST_MODE_MASK,
> -						    AW86927_PLAYCFG1_BST_MODE_BYPASS));
> +						    AW86927_PLAYCFG1_BST_MODE_BYPASS) :
> +					 FIELD_PREP(AW86938_PLAYCFG1_BST_MODE_MASK,
> +						    AW86938_PLAYCFG1_BST_MODE_BYPASS));
>  		if (err)
>  			return err;

[Severity: Medium]
The commit description notes that on AW86937 and AW86938 "the boost mode in
PLAYCFG1 is a two-bit field at bit 5 instead of bit 7", but
AW86938_PLAYCFG1_BST_MODE_MASK in drivers/input/misc/aw86927.c is defined as
a 1-bit mask:

#define AW86938_PLAYCFG1_REG			0x06
#define AW86938_PLAYCFG1_BST_MODE_MASK		GENMASK(5, 5)
#define AW86938_PLAYCFG1_BST_MODE_BYPASS	0
#define AW86938_PLAYCFG1_BST_VOUT_VREFSET_MASK	GENMASK(4, 0)

When aw86927_play_sine() -> aw86927_play_mode(haptics, AW86927_RAM_MODE) runs
on AW86937 or AW86938, regmap_update_bits() only updates bit 5 of
AW86927_PLAYCFG1_REG and leaves bit 6 untouched.

Should AW86938_PLAYCFG1_BST_MODE_MASK be defined as GENMASK(6, 5) so both
bits of the boost mode field are masked when configuring bypass mode?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027132929.242158-1-xiaqinghuan@mail.ustc.edu.cn?part=4