Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 0/2] map Fn + R key on newer Yogas and Legions
From: Gergo Koteles @ 2024-02-20  3:39 UTC (permalink / raw)
  To: Ike Panhc, Hans de Goede, Ilpo Järvinen, Dmitry Torokhov,
	Philipp Jungkamp
  Cc: platform-driver-x86, linux-kernel, linux-input, Gergo Koteles

Hi All,

This patch series adds a new KEY_FN_R input event code and map the
Fn + R key to it in the ideapad-laptop driver.

It affects two WMI keycodes and I couldn't try the 0x0a, but I couldn't 
find any indication that the refresh rate toggle should not be Fn + R.

Regards,
Gergo

Gergo Koteles (2):
  Input: allocate keycode for Fn + R
  platform/x86: ideapad-laptop: map Fn + R key to KEY_FN_R

 drivers/platform/x86/ideapad-laptop.c  | 4 ++--
 include/uapi/linux/input-event-codes.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)


base-commit: b401b621758e46812da61fa58a67c3fd8d91de0d
prerequisite-patch-id: 66a1ec71f181c6468a8226430af7ee917f40138a
-- 
2.43.2


^ permalink raw reply

* [PATCH 1/2] Input: allocate keycode for Fn + R
From: Gergo Koteles @ 2024-02-20  3:39 UTC (permalink / raw)
  To: Ike Panhc, Hans de Goede, Ilpo Järvinen, Dmitry Torokhov,
	Philipp Jungkamp
  Cc: platform-driver-x86, linux-kernel, linux-input, Gergo Koteles
In-Reply-To: <cover.1708399689.git.soyer@irl.hu>

Newer Lenovo Yogas and Legions with 60Hz/90Hz displays send a wmi event
when Fn + R is pressed. This is intended for use to adjust refresh rate.

But there is no silkscreen print on the R key to indicate this,
so add a KEY_FN_R keycode define to use for this key-combo.

Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 include/uapi/linux/input-event-codes.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 022a520e31fc..ba40af25943a 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -547,6 +547,7 @@
 #define KEY_FN_S		0x1e3
 #define KEY_FN_B		0x1e4
 #define KEY_FN_RIGHT_SHIFT	0x1e5
+#define KEY_FN_R		0x1e6
 
 #define KEY_BRL_DOT1		0x1f1
 #define KEY_BRL_DOT2		0x1f2
-- 
2.43.2


^ permalink raw reply related

* [PATCH 2/2] platform/x86: ideapad-laptop: map Fn + R key to KEY_FN_R
From: Gergo Koteles @ 2024-02-20  3:39 UTC (permalink / raw)
  To: Ike Panhc, Hans de Goede, Ilpo Järvinen, Dmitry Torokhov,
	Philipp Jungkamp
  Cc: platform-driver-x86, linux-kernel, linux-input, Gergo Koteles
In-Reply-To: <cover.1708399689.git.soyer@irl.hu>

Newer Lenovo Yogas (e.g. Yoga 7 14ARB7) and Legions with 60Hz/90Hz
displays send a wmi event when Fn + R is pressed. This is intended for
use to adjust refresh rate, but there is no silkscreen print on the R
key to indicate this.

The Fn + R key is incorrectly assigned to KEY_DISPLAYTOGGLE, as the
latter is used to toggle the display on and off.

Map the Fn + R key to a more generic KEY_FN_R event code, since the
refresh rate toggle feature is not common and is only used by a few
Lenovo laptops.

This commit depends on "platform/x86: ideapad-laptop: support Fn+R
dual-function key"

Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 drivers/platform/x86/ideapad-laptop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 4c130957f80d..c31c96228912 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1092,7 +1092,7 @@ static const struct key_entry ideapad_keymap[] = {
 	{ KE_KEY,	0x0e | IDEAPAD_WMI_KEY, { KEY_PICKUP_PHONE } },
 	{ KE_KEY,	0x0f | IDEAPAD_WMI_KEY, { KEY_HANGUP_PHONE } },
 	/* Refresh Rate Toggle (Fn+R) */
-	{ KE_KEY,	0x10 | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
+	{ KE_KEY,	0x10 | IDEAPAD_WMI_KEY, { KEY_FN_R } },
 	/* Dark mode toggle */
 	{ KE_KEY,	0x13 | IDEAPAD_WMI_KEY, { KEY_PROG1 } },
 	/* Sound profile switch */
@@ -1102,7 +1102,7 @@ static const struct key_entry ideapad_keymap[] = {
 	/* Lenovo Support */
 	{ KE_KEY,	0x27 | IDEAPAD_WMI_KEY, { KEY_HELP } },
 	/* Refresh Rate Toggle */
-	{ KE_KEY,	0x0a | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
+	{ KE_KEY,	0x0a | IDEAPAD_WMI_KEY, { KEY_FN_R } },
 
 	{ KE_END },
 };
-- 
2.43.2


^ permalink raw reply related

* Re: [PATCH 01/10] backlight: Match backlight device against struct fb_info.bl_dev
From: Javier Martinez Canillas @ 2024-02-20  9:17 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann
In-Reply-To: <20240212162645.5661-2-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Framebuffer drivers for devices with dedicated backlight are supposed
> to set struct fb_info.bl_dev to the backlight's respective device. Use
> the value to match backlight and framebuffer in the backlight core code.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/video/backlight/backlight.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 86e1cdc8e3697..48844a4f28ad3 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self,
>  {
>  	struct backlight_device *bd;
>  	struct fb_event *evdata = data;
> -	int node = evdata->info->node;
> +	struct fb_info *info = evdata->info;
> +	int node = info->node;
>  	int fb_blank = 0;
>  
>  	/* If we aren't interested in this event, skip it immediately ... */
> @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self,
>  
>  	if (!bd->ops)
>  		goto out;
> -	if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
> +	else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info))
>  		goto out;
> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
> +	else if (info->bl_dev && info->bl_dev != bd)

If the driver doesn't provide a struct backlight_ops .check_fb callback, I
think that having an info->bl_dev should be mandatory ? Or at least maybe
there should be a warning if info->bl_dev isn't set ?

The would be a driver bug, right ?

Regardless, the change makes sense to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 02/10] auxdisplay/ht16k33: Remove struct backlight_ops.check_fb
From: Javier Martinez Canillas @ 2024-02-20  9:18 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann,
	Robin van der Gracht
In-Reply-To: <20240212162645.5661-3-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> The driver sets struct fb_info.bl_dev to the correct backlight
> device. Thus rely on the backlight core code to match backlight
> and framebuffer devices, and remove the extra check_fb function
> from struct backlight_ops.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Robin van der Gracht <robin@protonic.nl>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 03/10] hid/hid-picolcd: Fix initialization order
From: Javier Martinez Canillas @ 2024-02-20  9:19 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann,
	Bruno Prémont
In-Reply-To: <20240212162645.5661-4-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> For drivers that support backlight, LCD and fbdev devices, fbdev has
> to be initialized last. See documentation for struct fbinfo.bl_dev.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 04/10] hid/hid-picolcd: Remove struct backlight_ops.check_fb
From: Javier Martinez Canillas @ 2024-02-20  9:24 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann,
	Bruno Prémont
In-Reply-To: <20240212162645.5661-5-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> The driver sets struct fb_info.bl_dev to the correct backlight
> device. Thus rely on the backlight core code to match backlight
> and framebuffer devices, and remove the extra check_fb function
> from struct backlight_ops.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
> ---

[...]

> +#ifdef CONFIG_HID_PICOLCD_BACKLIGHT
> +	info->bl_dev = data->backlight;
> +#endif
> +

The robot complains about this, I think that you also need to guard
against CONFIG_FB_BACKLIGHT being defined. Alternatively, you could
include a preparatory patch to fix the HID_PICOLCD_BACKLIGHT config
symbol dependencies.

Other than that,

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 05/10] backlight/aat2870-backlight: Remove struct backlight.check_fb
From: Javier Martinez Canillas @ 2024-02-20  9:26 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann
In-Reply-To: <20240212162645.5661-6-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> The driver's implementation of check_fb always returns true, which
> is the default if no implementation has been set. So remove the code
> from the driver.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 06/10] backlight/pwm-backlight: Remove struct backlight_ops.check_fb
From: Javier Martinez Canillas @ 2024-02-20  9:27 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann,
	Uwe Kleine-König
In-Reply-To: <20240212162645.5661-7-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> The internal check_fb callback from struct pwm_bl_data is never
> implemented. thus the driver's implementation of check_fb always
> returns true, which is the backlight core's default if no
> implementation has been set. So remove the code from the driver.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 07/10] fbdev/sh_mobile_lcdc_fb: Remove struct backlight_ops.check_fb
From: Javier Martinez Canillas @ 2024-02-20  9:27 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann
In-Reply-To: <20240212162645.5661-8-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> The driver sets struct fb_info.bl_dev to the correct backlight
> device. Thus rely on the backlight core code to match backlight
> and framebuffer devices, and remove the extra check_fb function
> from struct backlight_ops.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 08/10] fbdev/ssd1307fb: Init backlight before registering framebuffer
From: Javier Martinez Canillas @ 2024-02-20  9:32 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann
In-Reply-To: <20240212162645.5661-9-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> For drivers that support backlight, LCD and fbdev devices, fbdev has
> to be initialized last. See documentation for struct fbinfo.bl_dev.
>
> The backlight name's index number comes from register_framebuffer(),
> which now happens after initializing the backlight device. So like
> in all other backlight drivers, we now give the backlight device a
> generic name without the fbdev index.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 09/10] fbdev/ssd1307fb: Remove struct backlight_ops.check_fb
From: Javier Martinez Canillas @ 2024-02-20  9:32 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann
In-Reply-To: <20240212162645.5661-10-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> The driver sets struct fb_info.bl_dev to the correct backlight
> device. Thus rely on the backlight core code to match backlight
> and framebuffer devices, and remove the extra check_fb function
> from struct backlight_ops.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 10/10] backlight: Add controls_device callback to struct backlight_ops
From: Javier Martinez Canillas @ 2024-02-20  9:35 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm, Thomas Zimmermann
In-Reply-To: <20240212162645.5661-11-tzimmermann@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Replace check_fb with controls_device in struct backlight_ops. The
> new callback interface takes a Linux device instead of a framebuffer.
> Resolves one of the dependencies of backlight.h on fb.h.
>
> The few drivers that had custom implementations of check_fb can easily
> use the framebuffer's Linux device instead. Update them accordingly.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

Personally, I don't like the "control_device" callback name that much. I
think that check_device or probe_device would be a better one and easier
to understand what it does.

But not strong opinion on that,

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: [PATCH 01/10] backlight: Match backlight device against struct fb_info.bl_dev
From: Thomas Zimmermann @ 2024-02-20  9:37 UTC (permalink / raw)
  To: Javier Martinez Canillas, lee, daniel.thompson, jingoohan1,
	deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm
In-Reply-To: <87bk8bjxzo.fsf@minerva.mail-host-address-is-not-set>

Hi

Am 20.02.24 um 10:17 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
>
> Hello Thomas,
>
>> Framebuffer drivers for devices with dedicated backlight are supposed
>> to set struct fb_info.bl_dev to the backlight's respective device. Use
>> the value to match backlight and framebuffer in the backlight core code.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/video/backlight/backlight.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
>> index 86e1cdc8e3697..48844a4f28ad3 100644
>> --- a/drivers/video/backlight/backlight.c
>> +++ b/drivers/video/backlight/backlight.c
>> @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self,
>>   {
>>   	struct backlight_device *bd;
>>   	struct fb_event *evdata = data;
>> -	int node = evdata->info->node;
>> +	struct fb_info *info = evdata->info;
>> +	int node = info->node;
>>   	int fb_blank = 0;
>>   
>>   	/* If we aren't interested in this event, skip it immediately ... */
>> @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self,
>>   
>>   	if (!bd->ops)
>>   		goto out;
>> -	if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
>> +	else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info))
>>   		goto out;
>> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
>> +	else if (info->bl_dev && info->bl_dev != bd)
> If the driver doesn't provide a struct backlight_ops .check_fb callback, I
> think that having an info->bl_dev should be mandatory ? Or at least maybe
> there should be a warning if info->bl_dev isn't set ?

bl_dev can only be used for display drivers that set an explicit 
backlight device; otherwise it's NULL. There seem to be systems where 
backlight and display are distinct. And the docs for check_fb say that 
by default the backlight matches against any display. I tried to keep 
this semantics by silently succeeding if neither check_fb nor bl_dev 
have bene set.

>
> The would be a driver bug, right ?

I assume that some systems create the backlight instance from platform 
data or DT and the display driver has no means of knowing about it.

Best regards
Thomas

>
> Regardless, the change makes sense to me.
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)


^ permalink raw reply

* Re: [PATCH 01/10] backlight: Match backlight device against struct fb_info.bl_dev
From: Javier Martinez Canillas @ 2024-02-20  9:46 UTC (permalink / raw)
  To: Thomas Zimmermann, lee, daniel.thompson, jingoohan1, deller
  Cc: dri-devel, linux-fbdev, linux-input, linux-pwm
In-Reply-To: <a5b9a2d8-f305-4b68-9086-76d5f1b985c7@suse.de>

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Hi
>
> Am 20.02.24 um 10:17 schrieb Javier Martinez Canillas:
>> Thomas Zimmermann <tzimmermann@suse.de> writes:
>>
>> Hello Thomas,
>>
>>> Framebuffer drivers for devices with dedicated backlight are supposed
>>> to set struct fb_info.bl_dev to the backlight's respective device. Use
>>> the value to match backlight and framebuffer in the backlight core code.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> ---
>>>   drivers/video/backlight/backlight.c | 9 +++++++--
>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
>>> index 86e1cdc8e3697..48844a4f28ad3 100644
>>> --- a/drivers/video/backlight/backlight.c
>>> +++ b/drivers/video/backlight/backlight.c
>>> @@ -98,7 +98,8 @@ static int fb_notifier_callback(struct notifier_block *self,
>>>   {
>>>   	struct backlight_device *bd;
>>>   	struct fb_event *evdata = data;
>>> -	int node = evdata->info->node;
>>> +	struct fb_info *info = evdata->info;
>>> +	int node = info->node;
>>>   	int fb_blank = 0;
>>>   
>>>   	/* If we aren't interested in this event, skip it immediately ... */
>>> @@ -110,8 +111,12 @@ static int fb_notifier_callback(struct notifier_block *self,
>>>   
>>>   	if (!bd->ops)
>>>   		goto out;
>>> -	if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
>>> +	else if (bd->ops->check_fb && !bd->ops->check_fb(bd, info))
>>>   		goto out;
>>> +#if IS_ENABLED(CONFIG_FB_BACKLIGHT)
>>> +	else if (info->bl_dev && info->bl_dev != bd)
>> If the driver doesn't provide a struct backlight_ops .check_fb callback, I
>> think that having an info->bl_dev should be mandatory ? Or at least maybe
>> there should be a warning if info->bl_dev isn't set ?
>
> bl_dev can only be used for display drivers that set an explicit 
> backlight device; otherwise it's NULL. There seem to be systems where 
> backlight and display are distinct. And the docs for check_fb say that 
> by default the backlight matches against any display. I tried to keep 
> this semantics by silently succeeding if neither check_fb nor bl_dev 
> have bene set.
>
>>
>> The would be a driver bug, right ?
>
> I assume that some systems create the backlight instance from platform 
> data or DT and the display driver has no means of knowing about it.
>

Ok. I thought that in that case a (platform specific) .check_fb callback
would have to be provided then. But if the semantic is that none could be
missing, then I guess is OK to silently succeeding.

I wonder if at least a debug printout is worth it. But maybe a follow-up.

> Best regards
> Thomas
>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply

* Re: Regression with Lenovo ThinkPad Compact USB Keyboard
From: Thorsten Leemhuis @ 2024-02-20 11:35 UTC (permalink / raw)
  To: Raphaël Halimi, Linux Stable Mailing List
  Cc: Linux Regressions Mailing List, Linux Input Mailing List,
	Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires,
	Mikhail Khvainitski, Linux kernel regressions list
In-Reply-To: <a29d56d2-c440-4a26-a9ac-014595d2ae8c@gmail.com>

[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]

Hi, Thorsten here, the Linux kernel's regression tracker.

On 16.02.24 12:51, Raphaël Halimi wrote:
> 
> (sorry for the long CC list, it looks quite long to me, but I tried to
> follow the issue reporting guide as closely as possible)
> 
> Since patches [1], [2] and [3] were applied to the kernel, there is a
> regression with Lenovo ThinkPad Compact USB Keyboard (old model, not II).
> 
> [1]
> https://github.com/torvalds/linux/commit/46a0a2c96f0f47628190f122c2e3d879e590bcbe
> [2]
> https://github.com/torvalds/linux/commit/2f2bd7cbd1d1548137b351040dc4e037d18cdfdc
> [3]
> https://github.com/torvalds/linux/commit/43527a0094c10dfbf0d5a2e7979395a38de3ff65
> 
> The regression is that a middle click is performed when releasing middle
> button after wheel emulation.

How did you identify these three commits? Or do you just suspect that
it's one of them?

And did you try to check which of the three is the actual culprit?
Either by reverting them on top of master or by checking the parent for
each of the commits (git show '2f2bd7cbd1d^' shows the parent for
2f2bd7cbd1d).

> The bug appears randomly, it can be after 5 minutes or 1 hour of
> keyboard usage, and can only be worked around by unplugging/re-plugging
> the keyboard. (I ended up resorting to simulate an unplug/replug, with a
> script which echoes 0 then 1 to /sys/bus/usb/devices/<id>/authorized,
> since I was afraid to damage the Micro-USB outlet by physically
> unplugging/re-plugging too much).
> 
> Those spurious clicks are very annoying, since they can open links in
> new tabs when scrolling in Firefox, or pasting text when scrolling in
> terminals, or other unwanted stuff.
> 
> I witnessed it with latest kernels (Debian unstable) as well as stable
> kernels (Debian 12 Bookworm, stable).
> 
> On Debian Stable, the last working kernel was 5.10.127, the regression
> appeared in 5.10.136 (i read all changelogs on kernel.org between those
> two releases but couldn't find anything about hid-lenovo, so I can't
> tell exactly in which release the regression appeared, Debian upgraded
> directly from .127 to .136).

Why not bisect between .127 and .136 then?

> I reported it in Debian [4], and apparently I'm not the only person
> suffering from it [5].
> 
> [4] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1058758#32
> [5] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1058758#42
> 
> I would understand that such bugs would end up in a development kernel
> like the ones provided by Debian Unstable, but not with stable kernels
> like the ones provided by Debian Stable.

A bug report like yours can do the trick sometimes, as it might be
enough to ring a bell for one of the developers. But given that nobody
replied yet it looks like that is not the case. Then you most likely
will need to perform a bisection to identify the exact commit that broke
things.

FWIW, I'm currently working on a new document describing the bisection,
maybe it's of help for you:
https://www.leemhuis.info/files/misc/How%20to%20bisect%20a%20Linux%20kernel%20regression%20%e2%80%94%20The%20Linux%20Kernel%20documentation.html

Ciao, Thorsten

P.S.: To be sure the issue doesn't fall through the cracks unnoticed,
I'm adding it to regzbot, the Linux kernel regression tracking bot:

#regzbot ^introduced v5.10.127..v5.10.136
#regzbot title HID: lenovo: Lenovo ThinkPad Compact USB Keyboard
sometimes sends middle-click
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

^ permalink raw reply

* [syzbot] [input?] [usb?] INFO: rcu detected stall in sys_syslog (2)
From: syzbot @ 2024-02-20 16:45 UTC (permalink / raw)
  To: bp, dave.hansen, hpa, linux-input, linux-kernel, linux-usb, mingo,
	syzkaller-bugs, tglx, x86

Hello,

syzbot found the following issue on:

HEAD commit:    4f5e5092fdbf Merge tag 'net-6.8-rc5' of git://git.kernel.o..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12cff3a4180000
kernel config:  https://syzkaller.appspot.com/x/.config?x=1d7c92dd8d5c7a1e
dashboard link: https://syzkaller.appspot.com/bug?extid=269f9ad9bc32451d5fb5
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15f4a68a180000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/d430539932db/disk-4f5e5092.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/6369586e33b7/vmlinux-4f5e5092.xz
kernel image: https://storage.googleapis.com/syzbot-assets/9c1e38f80254/bzImage-4f5e5092.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+269f9ad9bc32451d5fb5@syzkaller.appspotmail.com

rcu: INFO: rcu_preempt detected expedited stalls on CPUs/tasks: {
 0-...D
 } 2649 jiffies s: 5869 root: 0x1/.
rcu: blocking rcu_node structures (internal RCU debug):

Sending NMI from CPU 1 to CPUs 0:
 kthread+0x2ef/0x390 kernel/kthread.c:388
NMI backtrace for cpu 0
CPU: 0 PID: 5441 Comm: syz-executor.0 Not tainted 6.8.0-rc4-syzkaller-00180-g4f5e5092fdbf #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
RIP: 0010:check_kcov_mode kernel/kcov.c:173 [inline]
RIP: 0010:__sanitizer_cov_trace_pc+0x2e/0x70 kernel/kcov.c:207
Code: 48 8b 04 24 65 48 8b 0d b0 52 70 7e 65 8b 15 b1 52 70 7e f7 c2 00 01 ff 00 74 11 f7 c2 00 01 00 00 74 35 83 b9 fc 15 00 00 00 <74> 2c 8b 91 d8 15 00 00 83 fa 02 75 21 48 8b 91 e0 15 00 00 48 8b
RSP: 0018:ffffc90000006128 EFLAGS: 00000046
RAX: ffffffff8b5fbfa4 RBX: 0000000000000131 RCX: ffff88805db05940
RDX: 0000000000000103 RSI: 0000000000000131 RDI: ffffc900000061e0
RBP: ffffc90000006270 R08: ffffffff8b60127a R09: 0000000000000000
R10: ffffc900000061e0 R11: fffff52000000c3f R12: 00000000ffff0a00
R13: dffffc0000000000 R14: ffffc900000061e0 R15: ffffc900000065c1
FS:  0000555555c97480(0000) GS:ffff8880b9400000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fe746ba8008 CR3: 000000002f932000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <NMI>
 </NMI>
 <IRQ>
 put_dec+0x14/0xe0 lib/vsprintf.c:291
 number+0x337/0xf90 lib/vsprintf.c:511
 vsnprintf+0x1542/0x1da0 lib/vsprintf.c:2890
 sprintf+0xda/0x120 lib/vsprintf.c:3028
 print_time kernel/printk/printk.c:1324 [inline]
 info_print_prefix+0x16b/0x310 kernel/printk/printk.c:1350
 record_print_text kernel/printk/printk.c:1399 [inline]
 printk_get_next_message+0x408/0xce0 kernel/printk/printk.c:2828
 console_emit_next_record kernel/printk/printk.c:2868 [inline]
 console_flush_all+0x42d/0xec0 kernel/printk/printk.c:2967
 console_unlock+0x13b/0x4d0 kernel/printk/printk.c:3036
 vprintk_emit+0x508/0x720 kernel/printk/printk.c:2303
 _printk+0xd5/0x120 kernel/printk/printk.c:2328
 printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
 show_trace_log_lvl+0x438/0x520 arch/x86/kernel/dumpstack.c:285
 sched_show_task+0x50c/0x6d0 kernel/sched/core.c:9171
 show_state_filter+0x19e/0x270 kernel/sched/core.c:9216
 kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
 kbd_event+0x30fa/0x4910 drivers/tty/vt/keyboard.c:1543
 input_to_handler drivers/input/input.c:132 [inline]
 input_pass_values+0x945/0x1200 drivers/input/input.c:161
 input_event_dispose+0x402/0x650 drivers/input/input.c:389
 input_handle_event+0xa71/0xbe0 drivers/input/input.c:406
 input_event+0xa4/0xd0 drivers/input/input.c:435
 hidinput_hid_event+0x138b/0x1c90 drivers/hid/hid-input.c:1731
 hid_process_event+0x439/0x590 drivers/hid/hid-core.c:1541
 hid_input_array_field+0x3fb/0x5c0 drivers/hid/hid-core.c:1653
 hid_process_report drivers/hid/hid-core.c:1695 [inline]
 hid_report_raw_event+0xf4f/0x18a0 drivers/hid/hid-core.c:2016
 hid_input_report+0x416/0x500 drivers/hid/hid-core.c:2090
 hid_irq_in+0x4a0/0x6d0 drivers/hid/usbhid/hid-core.c:284
 __usb_hcd_giveback_urb+0x373/0x530 drivers/usb/core/hcd.c:1650
 dummy_timer+0x8aa/0x3220 drivers/usb/gadget/udc/dummy_hcd.c:1987
 call_timer_fn+0x17e/0x600 kernel/time/timer.c:1700
 expire_timers kernel/time/timer.c:1751 [inline]
 __run_timers+0x621/0x830 kernel/time/timer.c:2038
 run_timer_softirq+0x67/0xf0 kernel/time/timer.c:2051
 __do_softirq+0x2bb/0x942 kernel/softirq.c:553
 invoke_softirq kernel/softirq.c:427 [inline]
 __irq_exit_rcu+0xf1/0x1c0 kernel/softirq.c:632
 irq_exit_rcu+0x9/0x30 kernel/softirq.c:644
 sysvec_apic_timer_interrupt+0x97/0xb0 arch/x86/kernel/apic/apic.c:1076
 </IRQ>
 <TASK>
 asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:649
RIP: 0010:__asan_memcpy+0x3b/0x70 mm/kasan/shadow.c:106
Code: 49 89 ff 48 8b 5c 24 20 48 89 f7 48 89 d6 31 d2 48 89 d9 e8 a7 e3 ff ff 84 c0 74 2c 4c 89 ff 4c 89 e6 ba 01 00 00 00 48 89 d9 <e8> 90 e3 ff ff 84 c0 74 15 4c 89 ff 4c 89 f6 4c 89 e2 5b 41 5c 41
RSP: 0018:ffffc90004e875f8 EFLAGS: 00000202
RAX: 0000000000000001 RBX: ffffffff8b5fca58 RCX: ffffffff8b5fca58
RDX: 0000000000000001 RSI: 0000000000000001 RDI: ffffc90004e879c8
RBP: ffffc90004e87710 R08: 0000000000000004 R09: fffffbfff1756eb8
R10: dffffc0000000000 R11: fffffbfff1756eb8 R12: 0000000000000001
R13: dffffc0000000000 R14: ffffffff8bab75c4 R15: ffffc90004e879c8
 vsnprintf+0x948/0x1da0 lib/vsprintf.c:2786
 sprintf+0xda/0x120 lib/vsprintf.c:3028
 print_caller kernel/printk/printk.c:1335 [inline]
 info_print_prefix+0x204/0x310 kernel/printk/printk.c:1352
 get_record_print_text_size kernel/printk/printk.c:1487 [inline]
 find_first_fitting_seq+0x1b2/0x580 kernel/printk/printk.c:1516
 syslog_print_all+0x1ae/0x7e0 kernel/printk/printk.c:1664
 do_syslog+0x38d/0x890 kernel/printk/printk.c:1742
 __do_sys_syslog kernel/printk/printk.c:1820 [inline]
 __se_sys_syslog kernel/printk/printk.c:1818 [inline]
 __x64_sys_syslog+0x7c/0x90 kernel/printk/printk.c:1818
 do_syscall_64+0xf9/0x240
 entry_SYSCALL_64_after_hwframe+0x6f/0x77
RIP: 0033:0x7f92d367dda9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffec8726828 EFLAGS: 00000246 ORIG_RAX: 0000000000000067
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f92d367dda9
RDX: 000000000000003f RSI: 00007ffec8726900 RDI: 0000000000000003
RBP: 00007ffec8726900 R08: 00007ffec87e60b0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffec8726ee8
R13: 00007ffec8726ee8 R14: 0000000000000001 R15: 00000000fffffff1
 </TASK>
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>
task:kworker/R-bat_e state:I stack:28848 pid:2748  tgid:2748  ppid:2      flags:0x00004000
Call Trace:
 <TASK>
 context_switch kernel/sched/core.c:5400 [inline]
 __schedule+0x17d1/0x49f0 kernel/sched/core.c:6727
 __schedule_loop kernel/sched/core.c:6802 [inline]
 schedule+0x149/0x260 kernel/sched/core.c:6817
 rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
 kthread+0x2ef/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>
task:kworker/u4:10   state:I stack:25808 pid:2799  tgid:2799  ppid:2      flags:0x00004000
Workqueue:  0x0 (events_unbound)
Call Trace:
 <TASK>
 context_switch kernel/sched/core.c:5400 [inline]
 __schedule+0x17d1/0x49f0 kernel/sched/core.c:6727
 __schedule_loop kernel/sched/core.c:6802 [inline]
 schedule+0x149/0x260 kernel/sched/core.c:6817
 worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
 kthread+0x2ef/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>
task:kworker/u4:11   state:I stack:24248 pid:2829  tgid:2829  ppid:2      flags:0x00004000
Workqueue:  0x0 (events_unbound)
Call Trace:
 <TASK>
 context_switch kernel/sched/core.c:5400 [inline]
 __schedule+0x17d1/0x49f0 kernel/sched/core.c:6727
 __schedule_loop kernel/sched/core.c:6802 [inline]
 schedule+0x149/0x260 kernel/sched/core.c:6817
 worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
 kthread+0x2ef/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>
task:kworker/u4:12   state:I stack:23024 pid:2849  tgid:2849  ppid:2      flags:0x00004000
Workqueue:  0x0 (events_unbound)
Call Trace:
 <TASK>
 context_switch kernel/sched/core.c:5400 [inline]
 __schedule+0x17d1/0x49f0 kernel/sched/core.c:6727
 __schedule_loop kernel/sched/core.c:6802 [inline]
 schedule+0x149/0x260 kernel/sched/core.c:6817
 worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
 kthread+0x2ef/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>
task:kworker/u4:13   state:I stack:26800 pid:2888  tgid:2888  ppid:2      flags:0x00004000
Workqueue:  0x0 (events_unbound)
Call Trace:
 <TASK>
 context_switch kernel/sched/core.c:5400 [inline]
 __schedule+0x17d1/0x49f0 kernel/sched/core.c:6727
 __schedule_loop kernel/sched/core.c:6802 [inline]
 schedule+0x149/0x260 kernel/sched/core.c:6817
 worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
 kthread+0x2ef/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>
task:kworker/u4:14   state:I stack:29392 pid:2906  tgid:2906  ppid:2      flags:0x00004000
Call Trace:
 <TASK>
 context_switch kernel/sched/core.c:5400 [inline]
 __schedule+0x17d1/0x49f0 kernel/sched/core.c:6727
 __schedule_loop kernel/sched/core.c:6802 [inline]
 schedule+0x149/0x260 kernel/sched/core.c:6817
 worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
 kthread+0x2ef/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>
task:kworker/R-zswap state:I stack:29744 pid:4464  tgid:4464  ppid:2      flags:0x00004000
Call Trace:
 <TASK>
 context_switch kernel/sched/core.c:5400 [inline]
 __schedule+0x17d1/0x49f0 kernel/sched/core.c:6727
 __schedule_loop kernel/sched/core.c:6802 [inline]
 schedule+0x149/0x260 kernel/sched/core.c:6817
 rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
 kthread+0x2ef/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply

* Re: [PATCH v1] HID: nintendo: Don't fail on setting baud rate
From: Max Staudt @ 2024-02-20 17:20 UTC (permalink / raw)
  To: Daniel J . Ogorchock, Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel
In-Reply-To: <20240115145000.12777-1-max@enpas.org>

Ping :)

Do you think this patch could be upstreamed to widen the controller support?


Thank you for looking at this!

Max




On 1/15/24 23:50, Max Staudt wrote:
> Some third-party controllers can't change the baud rate.
> 
> We can still use the gamepad as-is, so let's do that.
> 
> Signed-off-by: Max Staudt <max@enpas.org>
> ---
>   drivers/hid/hid-nintendo.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 997c3a1adaca..8cba0612c3ae 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -2164,8 +2164,11 @@ static int joycon_init(struct hid_device *hdev)
>   		/* set baudrate for improved latency */
>   		ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
>   		if (ret) {
> +			/*
> +			 * We can function with the default baudrate.
> +			 * Provide a warning, and continue on.
> +			 */
>   			hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> -			goto out_unlock;
>   		}
>   		/* handshake */
>   		ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);


^ permalink raw reply

* Re: Regression with Lenovo ThinkPad Compact USB Keyboard
From: Raphaël Halimi @ 2024-02-20 18:12 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Linux Regressions Mailing List, Linux Input Mailing List,
	Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires,
	Mikhail Khvainitski, Linux Stable Mailing List
In-Reply-To: <21370dc5-94a3-442c-ae04-76f9f94b1b96@leemhuis.info>

Le 20/02/2024 à 12:35, Thorsten Leemhuis a écrit :
> [CCing the regression list, as it should be in the loop for regressions:
> https://docs.kernel.org/admin-guide/reporting-regressions.html]
> 
> Hi, Thorsten here, the Linux kernel's regression tracker.

Hi, thanks for replying (even if I find your tone a bit harsh, but I 
don't blame you - and since English is not my native language, maybe I'm 
mistaking).

>> [1]
>> https://github.com/torvalds/linux/commit/46a0a2c96f0f47628190f122c2e3d879e590bcbe
>> [2]
>> https://github.com/torvalds/linux/commit/2f2bd7cbd1d1548137b351040dc4e037d18cdfdc
>> [3]
>> https://github.com/torvalds/linux/commit/43527a0094c10dfbf0d5a2e7979395a38de3ff65
>>
>> The regression is that a middle click is performed when releasing middle
>> button after wheel emulation.
> 
> How did you identify these three commits? Or do you just suspect that
> it's one of them?

No, I didn't "just suspect" that it was one of them. I may not be a 
kernel developer but I'm an experienced sysadmin (25+ years). So please 
stop taking users for idiots.

First, I compared the three machines I used which have a keyboard with a 
TrackPoint: my desktop at home (external "Lenovo ThinkPad Compact 
Keyboard with TrackPoint" (not II, not Bluetooth), Debian unstable (I'm 
a DM), my desktop at work (same keyboard, Debian Stable) and my personal 
laptop (ThinkPad X270, internal keyboard, Debian Stable but with backports).

The machine at work had a 5.10 kernel at the time, and the other ones 
had a 6.6, but only the machines with an external keyboard exhibited the 
spurious middle-clicks. So I compared the loaded HID drivers, and 
noticed that both of them had hid_lenovo loaded, whereas the laptop did not.

Confident that I probably pinpointed the faulty driver, I simply looked 
at the file history on Github, and saw that those three commits were 
dated from after the time when the bug appeared ; moreover, the comments 
did mention stuff related to wheel emulation and spurious middle-clicks.

So, no, I didn't "just suspected" that they were responsible, but I hope 
you'll admit my method was sound, and that my conclusion is a pretty 
strong (to not say "almost certain") probability.

> And did you try to check which of the three is the actual culprit?
> Either by reverting them on top of master or by checking the parent for
> each of the commits (git show '2f2bd7cbd1d^' shows the parent for
> 2f2bd7cbd1d).

I admit I didn't. I didn't compile my own kernels for ages. I used to do 
it in the past, but I came to trust Debian's kernels and rely on the 
maintainers' work. But read below.

>> On Debian Stable, the last working kernel was 5.10.127, the regression
>> appeared in 5.10.136 (i read all changelogs on kernel.org between those
>> two releases but couldn't find anything about hid-lenovo, so I can't
>> tell exactly in which release the regression appeared, Debian upgraded
>> directly from .127 to .136).
> 
> Why not bisect between .127 and .136 then?

I heard of that term before (and I understand the mathematical meaning 
of it), but I never did it with a Git tree. I read the guide you 
mentioned below, but it seems much too complicated and too long to me 
for just verifying if those three commits are indeed the cause of the 
regression (which I'm almost sure of, as stated above).

So in the meantime, I decided to follow my hunch and recompile only the 
hid_lenovo module (following the guide at [6], updating it slightly by 
manually removing kernel signing options in .config, since I obviously 
don't have Debian's signing keys, and replacing "make 
SUBDIRS=drivers/..." with "make M=..." as suggested by make), after 
un-applying those three patches in reverse order.

[6] https://askubuntu.com/a/338403/387067

The HID modules built successfully, and after copying my modified 
hid-lenovo.ko to /usr/lib/modules/6.6.15-amd64/updates/ and running 
'depmod -a', the module loaded fine with Debian's kernel (I don't use 
Secure Boot on this machine).

I'll let a few days pass (remember, the bug doesn't happen immediately 
but only after a varying amount of time) and I'll report here if the 
spurious middle-clicks happened again or not.

Notes:

1/ Thank you for (indirectly) giving me this idea. Maybe this relatively 
simple procedure should be made available somewhere on Debian's wiki 
(instead of an outdated, but still useful, answer on AskUbuntu).

2/ Please note that I did it only for unstable kernel; unfortunately, I 
can't do the same for the stable kernel, since I don't have access to my 
  machine at work anymore (my freelance contract ended one week ago) and 
I don't have any other machine at home exhibiting this bug. So I won't 
be able to test it on a stable kernel.

>> I reported it in Debian [4], and apparently I'm not the only person
>> suffering from it [5].
>>
>> [4] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1058758#32
>> [5] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1058758#42
>>
>> I would understand that such bugs would end up in a development kernel
>> like the ones provided by Debian Unstable, but not with stable kernels
>> like the ones provided by Debian Stable.
> 
> A bug report like yours can do the trick sometimes, as it might be
> enough to ring a bell for one of the developers. But given that nobody
> replied yet it looks like that is not the case. Then you most likely
> will need to perform a bisection to identify the exact commit that broke
> things.

Nobody amongst the developers, yes, I'll give you that. But the comment 
I linked from the Debian BTS, plus another bug report I found in the 
Input mailing list [7], show that I'm not the only user complaining from 
the recent regressions.

[7] 
https://lore.kernel.org/linux-input/CACSVgagaEHO2zoYQ8zDBrMT9OvT8R5B_h3dxfZuLQFAUBtKMmQ@mail.gmail.com

Regards,

-- 
Raphaël Halimi

^ permalink raw reply

* Re: Regression with Lenovo ThinkPad Compact USB Keyboard
From: Raphaël Halimi @ 2024-02-20 19:05 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Linux Regressions Mailing List, Linux Input Mailing List,
	Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires,
	Mikhail Khvainitski, Linux Stable Mailing List
In-Reply-To: <c8986411-2bf7-4b7d-8ac1-f702dc7c725a@gmail.com>

Le 20/02/2024 à 19:12, Raphaël Halimi a écrit :
> Confident that I probably pinpointed the faulty driver, I simply looked 
> at the file history on Github, and saw that those three commits were 
> dated from after the time when the bug appeared ; moreover, the comments 
> did mention stuff related to wheel emulation and spurious middle-clicks.

s/after/just before/

Regards,

-- 
Raphaël Halimi


^ permalink raw reply

* [PATCH v8 0/5] Add support for CS40L50
From: James Ogletree @ 2024-02-21  0:36 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree

Changes in v8:
- set_sysclk() -> set_bclk_ratio()
- Added ID table to codec driver
- Style improvements
- Fixed ordering of new write sequence operations

Changes in v7:
- Fixed sparse warning
- Moved write sequences to private data structure
- Logical and style improvements in write sequence interface

Changes in v6:
- Updated write sequencer interface to be control-name based
- Fixed a race condition and non-handling of repeats in playback callback
- Stylistic and logical improvements all around

Changes in v5:
- Added a codec sub-device to support I2S streaming
- Moved write sequencer code from cirrus_haptics to cs_dsp
- Reverted cirrus_haptics library; future Cirrus input
  drivers will export and utilize cs40l50_vibra functions
- Added more comments
- Many small stylistic and logical improvements

Changes in v4:
- Moved from Input to MFD
- Moved common Cirrus haptic functions to a library
- Incorporated runtime PM framework
- Many style improvements

Changes in v3:
- YAML formatting corrections
- Fixed typo in MAINTAINERS
- Used generic node name "haptic-driver"
- Fixed probe error code paths
- Switched to "sizeof(*)"
- Removed tree reference in MAINTAINERS

Changes in v2:
- Fixed checkpatch warnings

James Ogletree (5):
  firmware: cs_dsp: Add write sequencer interface
  dt-bindings: input: cirrus,cs40l50: Add initial DT binding
  mfd: cs40l50: Add support for CS40L50 core driver
  Input: cs40l50 - Add support for the CS40L50 haptic driver
  ASoC: cs40l50: Support I2S streaming to CS40L50

 .../bindings/input/cirrus,cs40l50.yaml        |  70 +++
 MAINTAINERS                                   |  12 +
 drivers/firmware/cirrus/cs_dsp.c              | 265 ++++++++
 drivers/input/misc/Kconfig                    |  10 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/cs40l50-vibra.c            | 575 ++++++++++++++++++
 drivers/mfd/Kconfig                           |  30 +
 drivers/mfd/Makefile                          |   4 +
 drivers/mfd/cs40l50-core.c                    | 531 ++++++++++++++++
 drivers/mfd/cs40l50-i2c.c                     |  69 +++
 drivers/mfd/cs40l50-spi.c                     |  69 +++
 include/linux/firmware/cirrus/cs_dsp.h        |  28 +
 include/linux/mfd/cs40l50.h                   | 142 +++++
 sound/soc/codecs/Kconfig                      |  11 +
 sound/soc/codecs/Makefile                     |   2 +
 sound/soc/codecs/cs40l50-codec.c              | 307 ++++++++++
 16 files changed, 2126 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
 create mode 100644 drivers/input/misc/cs40l50-vibra.c
 create mode 100644 drivers/mfd/cs40l50-core.c
 create mode 100644 drivers/mfd/cs40l50-i2c.c
 create mode 100644 drivers/mfd/cs40l50-spi.c
 create mode 100644 include/linux/mfd/cs40l50.h
 create mode 100644 sound/soc/codecs/cs40l50-codec.c

-- 
2.25.1


^ permalink raw reply

* [PATCH v8 1/5] firmware: cs_dsp: Add write sequencer interface
From: James Ogletree @ 2024-02-21  0:36 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree
In-Reply-To: <20240221003630.2535938-1-jogletre@opensource.cirrus.com>

A write sequencer is a sequence of register addresses
and values executed by some Cirrus DSPs following
power state transitions.

Add support for Cirrus drivers to update or add to a
write sequencer present in firmware.

Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
 drivers/firmware/cirrus/cs_dsp.c       | 265 +++++++++++++++++++++++++
 include/linux/firmware/cirrus/cs_dsp.h |  28 +++
 2 files changed, 293 insertions(+)

diff --git a/drivers/firmware/cirrus/cs_dsp.c b/drivers/firmware/cirrus/cs_dsp.c
index 79d4254d1f9b..9a597c5695d9 100644
--- a/drivers/firmware/cirrus/cs_dsp.c
+++ b/drivers/firmware/cirrus/cs_dsp.c
@@ -275,6 +275,12 @@
 #define HALO_MPU_VIO_ERR_SRC_MASK           0x00007fff
 #define HALO_MPU_VIO_ERR_SRC_SHIFT                   0
 
+/*
+ * Write Sequence
+ */
+#define WSEQ_OP_MAX_WORDS	3
+#define WSEQ_END_OF_SCRIPT	0xFFFFFF
+
 struct cs_dsp_ops {
 	bool (*validate_version)(struct cs_dsp *dsp, unsigned int version);
 	unsigned int (*parse_sizes)(struct cs_dsp *dsp,
@@ -3339,6 +3345,265 @@ int cs_dsp_chunk_read(struct cs_dsp_chunk *ch, int nbits)
 }
 EXPORT_SYMBOL_NS_GPL(cs_dsp_chunk_read, FW_CS_DSP);
 
+
+struct cs_dsp_wseq_op {
+	struct list_head list;
+	u32 address;
+	u32 data;
+	u16 offset;
+	u8 operation;
+};
+
+static int cs_dsp_populate_wseq(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq)
+{
+	struct cs_dsp_wseq_op *op = NULL;
+	struct cs_dsp_chunk ch;
+	u8 *words;
+	int ret;
+
+	if (!wseq->ctl) {
+		cs_dsp_err(dsp, "No control for write sequence\n");
+		return -EINVAL;
+	}
+
+	words = kzalloc(wseq->ctl->len, GFP_KERNEL);
+	if (!words)
+		return -ENOMEM;
+
+	ret = cs_dsp_coeff_read_ctrl(wseq->ctl, 0, words, wseq->ctl->len);
+	if (ret) {
+		cs_dsp_err(dsp, "Failed to read %s: %d\n", wseq->ctl->subname, ret);
+		goto err_free;
+	}
+
+	INIT_LIST_HEAD(&wseq->ops);
+
+	ch = cs_dsp_chunk(words, wseq->ctl->len);
+
+	while (!cs_dsp_chunk_end(&ch)) {
+		op = devm_kzalloc(dsp->dev, sizeof(*op), GFP_KERNEL);
+		if (!op) {
+			ret = -ENOMEM;
+			goto err_free;
+		}
+
+		op->offset = cs_dsp_chunk_bytes(&ch);
+		op->operation = cs_dsp_chunk_read(&ch, 8);
+
+		switch (op->operation) {
+		case CS_DSP_WSEQ_END:
+			op->data = WSEQ_END_OF_SCRIPT;
+			break;
+		case CS_DSP_WSEQ_UNLOCK:
+			op->data = cs_dsp_chunk_read(&ch, 16);
+			break;
+		case CS_DSP_WSEQ_ADDR8:
+			op->address = cs_dsp_chunk_read(&ch, 8);
+			op->data = cs_dsp_chunk_read(&ch, 32);
+			break;
+		case CS_DSP_WSEQ_H16:
+		case CS_DSP_WSEQ_L16:
+			op->address = cs_dsp_chunk_read(&ch, 24);
+			op->data = cs_dsp_chunk_read(&ch, 16);
+			break;
+		case CS_DSP_WSEQ_FULL:
+			op->address = cs_dsp_chunk_read(&ch, 32);
+			op->data = cs_dsp_chunk_read(&ch, 32);
+			break;
+		default:
+			ret = -EINVAL;
+			cs_dsp_err(dsp, "Unsupported op: %X\n", op->operation);
+			goto err_free;
+		}
+
+		list_add_tail(&op->list, &wseq->ops);
+
+		if (op->operation == CS_DSP_WSEQ_END)
+			break;
+	}
+
+	if (op && op->operation != CS_DSP_WSEQ_END) {
+		cs_dsp_err(dsp, "Write sequence missing end terminator\n");
+		ret = -ENOENT;
+	}
+
+err_free:
+	kfree(words);
+
+	return ret;
+}
+
+/**
+ * cs_dsp_wseq_init() - Initialize write sequences contained within the loaded DSP firmware
+ * @dsp: Pointer to DSP structure
+ * @wseqs: List of write sequences to initialize
+ * @num_wseqs: Number of write sequences to initialize
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs)
+{
+	int i, ret = 0;
+
+	mutex_lock(&dsp->pwr_lock);
+
+	for (i = 0; i < num_wseqs; i++) {
+		ret = cs_dsp_populate_wseq(dsp, &wseqs[i]);
+		if (ret)
+			break;
+	}
+
+	mutex_unlock(&dsp->pwr_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_init, FW_CS_DSP);
+
+static struct cs_dsp_wseq_op *cs_dsp_wseq_find_op(u32 addr, u8 op_code,
+						  struct list_head *wseq_ops)
+{
+	struct cs_dsp_wseq_op *op;
+
+	list_for_each_entry(op, wseq_ops, list) {
+		if (op->operation == op_code && op->address == addr)
+			return op;
+	}
+
+	return NULL;
+}
+
+/**
+ * cs_dsp_wseq_write() - Add or update an entry in a write sequence
+ * @dsp: Pointer to a DSP structure
+ * @wseq: Write sequence to write to
+ * @addr: Address of the register to be written to
+ * @data: Data to be written
+ * @op_code: The type of operation of the new entry
+ * @update: If true, searches for the first entry in the write sequence with
+ * the same address and op_code, and replaces it. If false, creates a new entry
+ * at the tail.
+ *
+ * This function formats register address and value pairs into the format
+ * required for write sequence entries, and either updates or adds the
+ * new entry into the write sequence.
+ *
+ * If update is set to true and no matching entry is found, it will add a new entry.
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+		      u32 addr, u32 data, u8 op_code, bool update)
+{
+	struct cs_dsp_wseq_op *op_end, *op_new = NULL;
+	u32 words[WSEQ_OP_MAX_WORDS];
+	struct cs_dsp_chunk ch;
+	int new_op_size, ret;
+
+	if (update)
+		op_new = cs_dsp_wseq_find_op(addr, op_code, &wseq->ops);
+
+	/* If entry to update is not found, treat it as a new operation */
+	if (!op_new) {
+		op_end = cs_dsp_wseq_find_op(0, CS_DSP_WSEQ_END, &wseq->ops);
+		if (!op_end) {
+			cs_dsp_err(dsp, "Missing write sequence list terminator\n");
+			return -EINVAL;
+		}
+
+		op_new = devm_kzalloc(dsp->dev, sizeof(*op_new), GFP_KERNEL);
+		if (!op_new)
+			return -ENOMEM;
+
+		op_new->operation = op_code;
+		op_new->address = addr;
+		op_new->offset = op_end->offset;
+		update = false;
+	}
+
+	op_new->data = data;
+
+	ch = cs_dsp_chunk(words, sizeof(words));
+	cs_dsp_chunk_write(&ch, 8, op_new->operation);
+	switch (op_code) {
+	case CS_DSP_WSEQ_FULL:
+		cs_dsp_chunk_write(&ch, 32, op_new->address);
+		cs_dsp_chunk_write(&ch, 32, op_new->data);
+		break;
+	case CS_DSP_WSEQ_L16:
+	case CS_DSP_WSEQ_H16:
+		cs_dsp_chunk_write(&ch, 24, op_new->address);
+		cs_dsp_chunk_write(&ch, 16, op_new->data);
+		break;
+	default:
+		ret = -EINVAL;
+		cs_dsp_err(dsp, "Op code not supported: %X\n", op_code);
+		goto op_new_free;
+	}
+
+	new_op_size = cs_dsp_chunk_bytes(&ch);
+
+	if (!update) {
+		if (wseq->ctl->len - op_end->offset < new_op_size) {
+			cs_dsp_err(dsp, "Not enough memory in write sequence for entry\n");
+			ret = -ENOMEM;
+			goto op_new_free;
+		}
+
+		op_end->offset += new_op_size;
+
+		ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_end->offset / sizeof(u32),
+					      &op_end->data, sizeof(u32));
+		if (ret)
+			goto op_new_free;
+
+		list_add_tail(&op_new->list, &op_end->list);
+	}
+
+	ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_new->offset / sizeof(u32),
+				      words, new_op_size);
+	if (ret)
+		goto op_new_free;
+
+	return 0;
+
+op_new_free:
+	devm_kfree(dsp->dev, op_new);
+
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_write, FW_CS_DSP);
+
+/**
+ * cs_dsp_wseq_multi_write() - Add or update multiple entries in the write sequence
+ * @dsp: Pointer to a DSP structure
+ * @wseq: Write sequence to write to
+ * @reg_seq: List of address-data pairs
+ * @num_regs: Number of address-data pairs
+ * @op_code: The types of operations of the new entries
+ * @update: If true, searches for the first entry in the write sequence with the same
+ * address and op code, and replaces it. If false, creates a new entry at the tail.
+ *
+ * This function calls cs_dsp_wseq_write() for multiple address-data pairs.
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+			    const struct reg_sequence *reg_seq, int num_regs,
+			    u8 op_code, bool update)
+{
+	int ret, i;
+
+	for (i = 0; i < num_regs; i++) {
+		ret = cs_dsp_wseq_write(dsp, wseq, reg_seq[i].reg,
+					reg_seq[i].def, update, op_code);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_multi_write, FW_CS_DSP);
+
 MODULE_DESCRIPTION("Cirrus Logic DSP Support");
 MODULE_AUTHOR("Simon Trimmer <simont@opensource.cirrus.com>");
 MODULE_LICENSE("GPL v2");
diff --git a/include/linux/firmware/cirrus/cs_dsp.h b/include/linux/firmware/cirrus/cs_dsp.h
index 29cd11d5a3cf..cfeab75772f6 100644
--- a/include/linux/firmware/cirrus/cs_dsp.h
+++ b/include/linux/firmware/cirrus/cs_dsp.h
@@ -42,6 +42,16 @@
 #define CS_DSP_ACKED_CTL_MIN_VALUE           0
 #define CS_DSP_ACKED_CTL_MAX_VALUE           0xFFFFFF
 
+/*
+ * Write sequence operation codes
+ */
+#define CS_DSP_WSEQ_FULL	0x00
+#define CS_DSP_WSEQ_ADDR8	0x02
+#define CS_DSP_WSEQ_L16		0x04
+#define CS_DSP_WSEQ_H16		0x05
+#define CS_DSP_WSEQ_UNLOCK	0xFD
+#define CS_DSP_WSEQ_END		0xFF
+
 /**
  * struct cs_dsp_region - Describes a logical memory region in DSP address space
  * @type:	Memory region type
@@ -255,6 +265,24 @@ struct cs_dsp_alg_region *cs_dsp_find_alg_region(struct cs_dsp *dsp,
 
 const char *cs_dsp_mem_region_name(unsigned int type);
 
+/**
+ * struct cs_dsp_wseq - Describes a write sequence
+ * @name:	Name of cs_dsp control
+ * @ctl:	Write sequence cs_dsp control
+ * @ops:	Operations contained within this write sequence
+ */
+struct cs_dsp_wseq {
+	struct cs_dsp_coeff_ctl *ctl;
+	struct list_head ops;
+};
+
+int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs);
+int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq, u32 addr, u32 data,
+		      u8 op_code, bool update);
+int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+			    const struct reg_sequence *reg_seq, int num_regs,
+			    u8 op_code, bool update);
+
 /**
  * struct cs_dsp_chunk - Describes a buffer holding data formatted for the DSP
  * @data:	Pointer to underlying buffer memory
-- 
2.25.1


^ permalink raw reply related

* [PATCH v8 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
From: James Ogletree @ 2024-02-21  0:36 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree
In-Reply-To: <20240221003630.2535938-1-jogletre@opensource.cirrus.com>

Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.

The ASoC driver enables I2S streaming to the device.

Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
 MAINTAINERS                      |   1 +
 sound/soc/codecs/Kconfig         |  11 ++
 sound/soc/codecs/Makefile        |   2 +
 sound/soc/codecs/cs40l50-codec.c | 307 +++++++++++++++++++++++++++++++
 4 files changed, 321 insertions(+)
 create mode 100644 sound/soc/codecs/cs40l50-codec.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 24cfb4f017bb..fca2454a7a38 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4943,6 +4943,7 @@ F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
 F:	drivers/input/misc/cs40l*
 F:	drivers/mfd/cs40l*
 F:	include/linux/mfd/cs40l*
+F:	sound/soc/codecs/cs40l*
 
 CIRRUS LOGIC DSP FIRMWARE DRIVER
 M:	Simon Trimmer <simont@opensource.cirrus.com>
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index f1e1dbc509f6..1a81bedfdbe3 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -73,6 +73,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_CS35L56_I2C
 	imply SND_SOC_CS35L56_SPI
 	imply SND_SOC_CS35L56_SDW
+	imply SND_SOC_CS40L50
 	imply SND_SOC_CS42L42
 	imply SND_SOC_CS42L42_SDW
 	imply SND_SOC_CS42L43
@@ -800,6 +801,16 @@ config SND_SOC_CS35L56_SDW
 	help
 	  Enable support for Cirrus Logic CS35L56 boosted amplifier with SoundWire control
 
+config SND_SOC_CS40L50
+	tristate "Cirrus Logic CS40L50 CODEC"
+	depends on MFD_CS40L50_CORE
+	help
+	  This option enables support for I2S streaming to Cirrus Logic CS40L50.
+
+	  CS40L50 is a haptic driver with waveform memory, an integrated
+	  DSP, and closed-loop algorithms. If built as a module, it will be
+	  called snd-soc-cs40l50.
+
 config SND_SOC_CS42L42_CORE
 	tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index a87e56938ce5..7e31f000774a 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -74,6 +74,7 @@ snd-soc-cs35l56-shared-objs := cs35l56-shared.o
 snd-soc-cs35l56-i2c-objs := cs35l56-i2c.o
 snd-soc-cs35l56-spi-objs := cs35l56-spi.o
 snd-soc-cs35l56-sdw-objs := cs35l56-sdw.o
+snd-soc-cs40l50-objs := cs40l50-codec.o
 snd-soc-cs42l42-objs := cs42l42.o
 snd-soc-cs42l42-i2c-objs := cs42l42-i2c.o
 snd-soc-cs42l42-sdw-objs := cs42l42-sdw.o
@@ -460,6 +461,7 @@ obj-$(CONFIG_SND_SOC_CS35L56_SHARED)	+= snd-soc-cs35l56-shared.o
 obj-$(CONFIG_SND_SOC_CS35L56_I2C)	+= snd-soc-cs35l56-i2c.o
 obj-$(CONFIG_SND_SOC_CS35L56_SPI)	+= snd-soc-cs35l56-spi.o
 obj-$(CONFIG_SND_SOC_CS35L56_SDW)	+= snd-soc-cs35l56-sdw.o
+obj-$(CONFIG_SND_SOC_CS40L50)		+= snd-soc-cs40l50.o
 obj-$(CONFIG_SND_SOC_CS42L42_CORE)	+= snd-soc-cs42l42.o
 obj-$(CONFIG_SND_SOC_CS42L42)	+= snd-soc-cs42l42-i2c.o
 obj-$(CONFIG_SND_SOC_CS42L42_SDW)	+= snd-soc-cs42l42-sdw.o
diff --git a/sound/soc/codecs/cs40l50-codec.c b/sound/soc/codecs/cs40l50-codec.c
new file mode 100644
index 000000000000..8906a05da14e
--- /dev/null
+++ b/sound/soc/codecs/cs40l50-codec.c
@@ -0,0 +1,307 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// CS40L50 Advanced Haptic Driver with waveform memory,
+// integrated DSP, and closed-loop algorithms
+//
+// Copyright 2024 Cirrus Logic, Inc.
+//
+// Author: James Ogletree <james.ogletree@cirrus.com>
+
+#include <linux/bitfield.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/pm_runtime.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#define CS40L50_REFCLK_INPUT		0x2C04
+#define CS40L50_ASP_CONTROL2		0x4808
+#define CS40L50_ASP_DATA_CONTROL5	0x4840
+
+/* PLL Config */
+#define CS40L50_PLL_REFCLK_BCLK		0x0
+#define CS40L50_PLL_REFCLK_MCLK		0x5
+#define CS40L50_PLL_REFCLK_LOOP_MASK	BIT(11)
+#define CS40L50_PLL_REFCLK_OPEN_LOOP	1
+#define CS40L50_PLL_REFCLK_CLOSED_LOOP	0
+#define CS40L50_PLL_REFCLK_LOOP_SHIFT	11
+#define CS40L50_PLL_REFCLK_FREQ_MASK	GENMASK(10, 5)
+#define CS40L50_PLL_REFCLK_FREQ_SHIFT	5
+#define CS40L50_PLL_REFCLK_SEL_MASK	GENMASK(2, 0)
+#define CS40L50_BCLK_RATIO_DEFAULT	32
+
+/* ASP Config */
+#define CS40L50_ASP_RX_WIDTH_SHIFT	24
+#define CS40L50_ASP_RX_WIDTH_MASK	GENMASK(31, 24)
+#define CS40L50_ASP_RX_WL_MASK		GENMASK(5, 0)
+#define CS40L50_ASP_FSYNC_INV_MASK	BIT(2)
+#define CS40L50_ASP_BCLK_INV_MASK	BIT(6)
+#define CS40L50_ASP_FMT_MASK		GENMASK(10, 8)
+#define CS40L50_ASP_FMT_I2S		0x2
+
+struct cs40l50_pll_config {
+	unsigned int freq;
+	unsigned int cfg;
+};
+
+struct cs40l50_codec {
+	struct device *dev;
+	struct regmap *regmap;
+	unsigned int daifmt;
+	unsigned int bclk_ratio;
+	unsigned int rate;
+};
+
+static const struct cs40l50_pll_config cs40l50_pll_cfg[] = {
+	{32768, 0x00},
+	{1536000, 0x1B},
+	{3072000, 0x21},
+	{6144000, 0x28},
+	{9600000, 0x30},
+	{12288000, 0x33},
+};
+
+static int cs40l50_get_clk_config(unsigned int freq, unsigned int *cfg)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cs40l50_pll_cfg); i++) {
+		if (cs40l50_pll_cfg[i].freq == freq) {
+			*cfg = cs40l50_pll_cfg[i].cfg;
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int cs40l50_swap_ext_clk(struct cs40l50_codec *codec, unsigned int clk_src)
+{
+	unsigned int cfg;
+	int ret;
+
+	switch (clk_src) {
+	case CS40L50_PLL_REFCLK_BCLK:
+		ret = cs40l50_get_clk_config(codec->bclk_ratio * codec->rate, &cfg);
+		if (ret)
+			return ret;
+		break;
+	case CS40L50_PLL_REFCLK_MCLK:
+		cfg = 0x00;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+				 CS40L50_PLL_REFCLK_LOOP_MASK,
+				 CS40L50_PLL_REFCLK_OPEN_LOOP <<
+				 CS40L50_PLL_REFCLK_LOOP_SHIFT);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+				 CS40L50_PLL_REFCLK_FREQ_MASK |
+				 CS40L50_PLL_REFCLK_SEL_MASK,
+				 (cfg << CS40L50_PLL_REFCLK_FREQ_SHIFT) | clk_src);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+				  CS40L50_PLL_REFCLK_LOOP_MASK,
+				  CS40L50_PLL_REFCLK_CLOSED_LOOP <<
+				  CS40L50_PLL_REFCLK_LOOP_SHIFT);
+}
+
+static int cs40l50_clk_en(struct snd_soc_dapm_widget *w,
+			  struct snd_kcontrol *kcontrol,
+			  int event)
+{
+	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(comp);
+	int ret;
+
+	switch (event) {
+	case SND_SOC_DAPM_POST_PMU:
+		ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_STOP_PLAYBACK);
+		if (ret)
+			return ret;
+
+		ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_START_I2S);
+		if (ret)
+			return ret;
+
+		ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_BCLK);
+		if (ret)
+			return ret;
+		break;
+	case SND_SOC_DAPM_PRE_PMD:
+		ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_MCLK);
+		if (ret)
+			return ret;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget cs40l50_dapm_widgets[] = {
+	SND_SOC_DAPM_SUPPLY_S("ASP PLL", 0, SND_SOC_NOPM, 0, 0, cs40l50_clk_en,
+		SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
+	SND_SOC_DAPM_AIF_IN("ASPRX1", NULL, 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("ASPRX2", NULL, 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_OUTPUT("OUT"),
+};
+
+static const struct snd_soc_dapm_route cs40l50_dapm_routes[] = {
+	{ "ASP Playback", NULL, "ASP PLL" },
+	{ "ASPRX1", NULL, "ASP Playback" },
+	{ "ASPRX2", NULL, "ASP Playback" },
+
+	{ "OUT", NULL, "ASPRX1" },
+	{ "OUT", NULL, "ASPRX2" },
+};
+
+static int cs40l50_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
+{
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(codec_dai->component);
+
+	if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBC_CFC)
+		return -EINVAL;
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		codec->daifmt = 0;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		codec->daifmt = CS40L50_ASP_BCLK_INV_MASK;
+		break;
+	case SND_SOC_DAIFMT_IB_IF:
+		codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK | CS40L50_ASP_BCLK_INV_MASK;
+		break;
+	default:
+		dev_err(codec->dev, "Invalid clock invert\n");
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		codec->daifmt |= FIELD_PREP(CS40L50_ASP_FMT_MASK, CS40L50_ASP_FMT_I2S);
+		break;
+	default:
+		dev_err(codec->dev, "Unsupported DAI format\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int cs40l50_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
+	unsigned int asp_rx_wl = params_width(params);
+	int ret;
+
+	codec->rate = params_rate(params);
+
+	ret = regmap_update_bits(codec->regmap, CS40L50_ASP_DATA_CONTROL5,
+				 CS40L50_ASP_RX_WL_MASK, asp_rx_wl);
+	if (ret)
+		return ret;
+
+	codec->daifmt |= (asp_rx_wl << CS40L50_ASP_RX_WIDTH_SHIFT);
+
+	return regmap_update_bits(codec->regmap, CS40L50_ASP_CONTROL2,
+				  CS40L50_ASP_FSYNC_INV_MASK |
+				  CS40L50_ASP_BCLK_INV_MASK |
+				  CS40L50_ASP_FMT_MASK |
+				  CS40L50_ASP_RX_WIDTH_MASK, codec->daifmt);
+}
+
+static int cs40l50_set_dai_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio)
+{
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
+
+	codec->bclk_ratio = ratio;
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops cs40l50_dai_ops = {
+	.set_fmt = cs40l50_set_dai_fmt,
+	.set_bclk_ratio = cs40l50_set_dai_bclk_ratio,
+	.hw_params = cs40l50_hw_params,
+};
+
+static struct snd_soc_dai_driver cs40l50_dai[] = {
+	{
+		.name = "cs40l50-pcm",
+		.id = 0,
+		.playback = {
+			.stream_name = "ASP Playback",
+			.channels_min = 1,
+			.channels_max = 2,
+			.rates = SNDRV_PCM_RATE_48000,
+			.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+		},
+		.ops = &cs40l50_dai_ops,
+	},
+};
+
+static int cs40l50_codec_probe(struct snd_soc_component *component)
+{
+	struct cs40l50_codec *codec = snd_soc_component_get_drvdata(component);
+
+	codec->bclk_ratio = CS40L50_BCLK_RATIO_DEFAULT;
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver soc_codec_dev_cs40l50 = {
+	.probe = cs40l50_codec_probe,
+	.dapm_widgets = cs40l50_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(cs40l50_dapm_widgets),
+	.dapm_routes = cs40l50_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(cs40l50_dapm_routes),
+};
+
+static int cs40l50_codec_driver_probe(struct platform_device *pdev)
+{
+	struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
+	struct cs40l50_codec *codec;
+
+	codec = devm_kzalloc(&pdev->dev, sizeof(*codec), GFP_KERNEL);
+	if (!codec)
+		return -ENOMEM;
+
+	codec->regmap = cs40l50->regmap;
+	codec->dev = &pdev->dev;
+
+	return devm_snd_soc_register_component(&pdev->dev, &soc_codec_dev_cs40l50,
+					       cs40l50_dai, ARRAY_SIZE(cs40l50_dai));
+}
+
+static const struct platform_device_id cs40l50_id[] = {
+	{ "cs40l50-codec", },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, cs40l50_id);
+
+static struct platform_driver cs40l50_codec_driver = {
+	.probe = cs40l50_codec_driver_probe,
+	.id_table = cs40l50_id,
+	.driver = {
+		.name = "cs40l50-codec",
+	},
+};
+module_platform_driver(cs40l50_codec_driver);
+
+MODULE_DESCRIPTION("ASoC CS40L50 driver");
+MODULE_AUTHOR("James Ogletree <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* [PATCH v8 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
From: James Ogletree @ 2024-02-21  0:36 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree
In-Reply-To: <20240221003630.2535938-1-jogletre@opensource.cirrus.com>

Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.

The input driver provides the interface for control of
haptic effects through the device.

Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
v7: v8:
Please advise if playback stop is still misused with respect to not
specifying an effect ID. The device can only play one effect at a
time, but setting max effects for the EVIOCGEFFECTS ioctl to 1 would
restrict the number of uploads to 1 as well. Is there a workaround?
Please correct any wrong assumptions here.

 MAINTAINERS                        |   1 +
 drivers/input/misc/Kconfig         |  10 +
 drivers/input/misc/Makefile        |   1 +
 drivers/input/misc/cs40l50-vibra.c | 575 +++++++++++++++++++++++++++++
 4 files changed, 587 insertions(+)
 create mode 100644 drivers/input/misc/cs40l50-vibra.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 69a9e0a3b968..24cfb4f017bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4940,6 +4940,7 @@ M:	Ben Bright <ben.bright@cirrus.com>
 L:	patches@opensource.cirrus.com
 S:	Supported
 F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
+F:	drivers/input/misc/cs40l*
 F:	drivers/mfd/cs40l*
 F:	include/linux/mfd/cs40l*
 
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6ba984d7f0b1..ee45dbb0636e 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -140,6 +140,16 @@ config INPUT_BMA150
 	  To compile this driver as a module, choose M here: the
 	  module will be called bma150.
 
+config INPUT_CS40L50_VIBRA
+	tristate "CS40L50 Haptic Driver support"
+	depends on MFD_CS40L50_CORE
+	help
+	  Say Y here to enable support for Cirrus Logic's CS40L50
+	  haptic driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cs40l50-vibra.
+
 config INPUT_E3X0_BUTTON
 	tristate "NI Ettus Research USRP E3xx Button support."
 	default n
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 04296a4abe8e..88279de6d3d5 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_INPUT_CMA3000)		+= cma3000_d0x.o
 obj-$(CONFIG_INPUT_CMA3000_I2C)		+= cma3000_d0x_i2c.o
 obj-$(CONFIG_INPUT_COBALT_BTNS)		+= cobalt_btns.o
 obj-$(CONFIG_INPUT_CPCAP_PWRBUTTON)	+= cpcap-pwrbutton.o
+obj-$(CONFIG_INPUT_CS40L50_VIBRA)	+= cs40l50-vibra.o
 obj-$(CONFIG_INPUT_DA7280_HAPTICS)	+= da7280.o
 obj-$(CONFIG_INPUT_DA9052_ONKEY)	+= da9052_onkey.o
 obj-$(CONFIG_INPUT_DA9055_ONKEY)	+= da9055_onkey.o
diff --git a/drivers/input/misc/cs40l50-vibra.c b/drivers/input/misc/cs40l50-vibra.c
new file mode 100644
index 000000000000..dce2a71fbb20
--- /dev/null
+++ b/drivers/input/misc/cs40l50-vibra.c
@@ -0,0 +1,575 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/input.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+/* Wavetables */
+#define CS40L50_RAM_INDEX_START		0x1000000
+#define CS40L50_RAM_INDEX_END		0x100007F
+#define CS40L50_RTH_INDEX_START		0x1400000
+#define CS40L50_RTH_INDEX_END		0x1400001
+#define CS40L50_ROM_INDEX_START		0x1800000
+#define CS40L50_ROM_INDEX_END		0x180001A
+#define CS40L50_TYPE_PCM		8
+#define CS40L50_TYPE_PWLE		12
+#define CS40L50_PCM_ID			0x0
+#define CS40L50_OWT_CUSTOM_DATA_SIZE	2
+
+/* DSP */
+#define CS40L50_GPIO_BASE		0x2804140
+#define CS40L50_OWT_BASE		0x2805C34
+#define CS40L50_OWT_SIZE		0x2805C38
+#define CS40L50_OWT_NEXT		0x2805C3C
+
+/* GPIO */
+#define CS40L50_GPIO_NUM_MASK		GENMASK(14, 12)
+#define CS40L50_GPIO_EDGE_MASK		BIT(15)
+#define CS40L50_GPIO_MAPPING_NONE	0
+#define CS40L50_GPIO_DISABLE		0x1FF
+
+enum vibra_bank_type {
+	WVFRM_BANK_RAM,
+	WVFRM_BANK_ROM,
+	WVFRM_BANK_OWT,
+	WVFRM_BANK_NUM,
+};
+
+/* Describes an area in DSP memory populated by effects */
+struct vibra_bank {
+	enum vibra_bank_type bank;
+	u32 base_index;
+	u32 max_index;
+};
+
+struct vibra_effect {
+	enum vibra_bank_type bank;
+	struct list_head list;
+	u32 gpio_reg;
+	u32 index;
+	int id;
+};
+
+/* Describes haptic interface of loaded DSP firmware */
+struct vibra_dsp {
+	struct vibra_bank *banks;
+	u32 gpio_base_reg;
+	u32 owt_offset_reg;
+	u32 owt_size_reg;
+	u32 owt_base_reg;
+	int (*write)(struct device *dev, struct regmap *regmap, u32 val);
+	u32 push_owt_cmd;
+	u32 delete_owt_cmd;
+	u32 stop_cmd;
+};
+
+/* Describes configuration and state of haptic operations */
+struct vibra_info {
+	struct device *dev;
+	struct regmap *regmap;
+	struct input_dev *input;
+	struct mutex lock;
+	struct workqueue_struct *vibe_wq;
+	struct list_head effect_head;
+	struct vibra_dsp dsp;
+};
+
+struct vibra_work {
+	struct vibra_info *info;
+	struct ff_effect *effect;
+	struct work_struct work;
+	s16 *custom_data;
+	int custom_len;
+	int count;
+	int error;
+};
+
+static struct vibra_bank cs40l50_banks[] = {
+	{
+		.bank =		WVFRM_BANK_RAM,
+		.base_index =	CS40L50_RAM_INDEX_START,
+		.max_index =	CS40L50_RAM_INDEX_END,
+	},
+	{
+		.bank =		WVFRM_BANK_ROM,
+		.base_index =	CS40L50_ROM_INDEX_START,
+		.max_index =	CS40L50_ROM_INDEX_END,
+	},
+	{
+		.bank =		WVFRM_BANK_OWT,
+		.base_index =	CS40L50_RTH_INDEX_START,
+		.max_index =	CS40L50_RTH_INDEX_END,
+	},
+};
+
+static struct vibra_dsp cs40l50_dsp = {
+	.banks =		cs40l50_banks,
+	.gpio_base_reg =	CS40L50_GPIO_BASE,
+	.owt_base_reg =		CS40L50_OWT_BASE,
+	.owt_offset_reg =	CS40L50_OWT_NEXT,
+	.owt_size_reg =		CS40L50_OWT_SIZE,
+
+	.push_owt_cmd =		CS40L50_OWT_PUSH,
+	.delete_owt_cmd =	CS40L50_OWT_DELETE,
+	.stop_cmd =		CS40L50_STOP_PLAYBACK,
+
+	.write =		cs40l50_dsp_write,
+};
+
+static struct vibra_effect *vibra_find_effect(int id, struct list_head *effect_head)
+{
+	struct vibra_effect *effect;
+
+	list_for_each_entry(effect, effect_head, list)
+		if (effect->id == id)
+			return effect;
+
+	return NULL;
+}
+
+static int vibra_effect_bank_set(struct vibra_work *work_data,
+				 struct vibra_effect *effect)
+{
+	s16 bank = work_data->custom_data[0] & 0xffffu;
+
+	if (bank >= WVFRM_BANK_NUM) {
+		dev_err(work_data->info->dev, "Invalid waveform bank: %d\n", bank);
+		return -EINVAL;
+	}
+
+	if (work_data->custom_len > CS40L50_OWT_CUSTOM_DATA_SIZE)
+		effect->bank = WVFRM_BANK_OWT;
+	else
+		effect->bank = bank;
+
+	return 0;
+}
+
+static int vibra_effect_gpio_mapping_set(struct vibra_work *work_data,
+				    struct vibra_effect *effect)
+{
+	u16 button = work_data->effect->trigger.button;
+	struct vibra_info *info = work_data->info;
+	u32 gpio_num, gpio_edge;
+
+	if (button) {
+		gpio_num = FIELD_GET(CS40L50_GPIO_NUM_MASK, button);
+		gpio_edge = FIELD_GET(CS40L50_GPIO_EDGE_MASK, button);
+		effect->gpio_reg = info->dsp.gpio_base_reg + (gpio_num * 8) - gpio_edge;
+
+		return regmap_write(info->regmap, effect->gpio_reg, button);
+	}
+
+	effect->gpio_reg = CS40L50_GPIO_MAPPING_NONE;
+
+	return 0;
+}
+
+static int vibra_effect_index_set(struct vibra_work *work_data,
+				  struct vibra_effect *effect)
+{
+	struct vibra_info *info = work_data->info;
+	struct vibra_effect *owt_effect;
+	u32 base_index, max_index;
+
+	base_index = info->dsp.banks[effect->bank].base_index;
+	max_index = info->dsp.banks[effect->bank].max_index;
+
+	effect->index = base_index;
+
+	switch (effect->bank) {
+	case WVFRM_BANK_OWT:
+		list_for_each_entry(owt_effect, &info->effect_head, list)
+			if (owt_effect->bank == WVFRM_BANK_OWT)
+				effect->index++;
+		break;
+	case WVFRM_BANK_ROM:
+	case WVFRM_BANK_RAM:
+		effect->index += work_data->custom_data[1] & 0xffffu;
+		break;
+	default:
+		dev_err(info->dev, "Bank not supported: %d\n", effect->bank);
+		return -EINVAL;
+	}
+
+	if (effect->index > max_index || effect->index < base_index) {
+		dev_err(info->dev, "Index out of bounds: %u\n", effect->index);
+		return -ENOSPC;
+	}
+
+	return 0;
+}
+
+/* Describes a header for an OWT effect */
+struct owt_header {
+	u32 type;
+	u32 data_words;
+	u32 offset;
+} __packed;
+
+static int vibra_upload_owt(struct vibra_work *work_data)
+{
+	u32 len = 2 * work_data->custom_len, wt_offset, wt_size;
+	struct vibra_info *info = work_data->info;
+	struct owt_header header;
+	u8 *out_data;
+	int error;
+
+	error = regmap_read(info->regmap, info->dsp.owt_size_reg, &wt_size);
+	if (error)
+		return error;
+
+	if ((wt_size * sizeof(u32)) < sizeof(header) + len) {
+		dev_err(info->dev, "No space in OWT bank for effect\n");
+		return -ENOSPC;
+	}
+
+	out_data = kzalloc(sizeof(header) + len, GFP_KERNEL);
+	if (!out_data)
+		return -ENOMEM;
+
+	header.type = work_data->custom_data[0] == CS40L50_PCM_ID ? CS40L50_TYPE_PCM :
+								    CS40L50_TYPE_PWLE;
+	header.offset = sizeof(header) / sizeof(u32);
+	header.data_words = len / sizeof(u32);
+
+	memcpy(out_data, &header, sizeof(header));
+	memcpy(out_data + sizeof(header), work_data->custom_data, len);
+
+	error = regmap_read(info->regmap, info->dsp.owt_offset_reg, &wt_offset);
+	if (error)
+		return error;
+
+	error = regmap_bulk_write(info->regmap, info->dsp.owt_base_reg +
+				  (wt_offset * sizeof(u32)), out_data,
+				  sizeof(header) + len);
+	if (error)
+		goto err_free;
+
+	error = info->dsp.write(info->dev, info->regmap, info->dsp.push_owt_cmd);
+err_free:
+	kfree(out_data);
+
+	return error;
+}
+
+static void vibra_add_worker(struct work_struct *work)
+{
+	struct vibra_work *work_data = container_of(work, struct vibra_work, work);
+	struct vibra_info *info = work_data->info;
+	struct vibra_effect *effect;
+	bool is_new = false;
+	int error;
+
+	error = pm_runtime_resume_and_get(info->dev);
+	if (error < 0)
+		goto exit;
+
+	mutex_lock(&info->lock);
+
+	effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
+	if (!effect) {
+		effect = kzalloc(sizeof(*effect), GFP_KERNEL);
+		if (!effect) {
+			error = -ENOMEM;
+			goto err_mutex;
+		}
+
+		effect->id = work_data->effect->id;
+		is_new = true;
+	}
+
+	error = vibra_effect_bank_set(work_data, effect);
+	if (error)
+		goto err_free;
+
+	error = vibra_effect_index_set(work_data, effect);
+	if (error)
+		goto err_free;
+
+	error = vibra_effect_gpio_mapping_set(work_data, effect);
+	if (error)
+		goto err_free;
+
+	if (effect->bank == WVFRM_BANK_OWT)
+		error = vibra_upload_owt(work_data);
+err_free:
+	if (is_new) {
+		if (error)
+			kfree(effect);
+		else
+			list_add(&effect->list, &info->effect_head);
+	}
+err_mutex:
+	mutex_unlock(&info->lock);
+
+	pm_runtime_mark_last_busy(info->dev);
+	pm_runtime_put_autosuspend(info->dev);
+exit:
+	work_data->error = error;
+}
+
+static int vibra_add(struct input_dev *dev, struct ff_effect *effect,
+		     struct ff_effect *old)
+{
+	struct ff_periodic_effect *periodic = &effect->u.periodic;
+	struct vibra_info *info = input_get_drvdata(dev);
+	struct vibra_work work_data = { info, effect, };
+	u32 len = effect->u.periodic.custom_len;
+
+	if (effect->type != FF_PERIODIC || periodic->waveform != FF_CUSTOM) {
+		dev_err(info->dev, "Type (%#X) or waveform (%#X) unsupported\n",
+			effect->type, periodic->waveform);
+		return -EINVAL;
+	}
+
+	work_data.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
+	if (!work_data.custom_data)
+		return -ENOMEM;
+
+	if (copy_from_user(work_data.custom_data, effect->u.periodic.custom_data,
+			   sizeof(s16) * len)) {
+		work_data.error = -EFAULT;
+		goto out_free;
+	}
+
+	work_data.custom_len = len;
+
+	INIT_WORK(&work_data.work, vibra_add_worker);
+
+	/* Push to the workqueue to serialize with playbacks */
+	queue_work(info->vibe_wq, &work_data.work);
+	flush_work(&work_data.work);
+out_free:
+	kfree(work_data.custom_data);
+
+	return work_data.error;
+}
+
+static void vibra_start_worker(struct work_struct *work)
+{
+	struct vibra_work *work_data = container_of(work, struct vibra_work, work);
+	struct vibra_info *info = work_data->info;
+	struct vibra_effect *start_effect;
+
+	if (pm_runtime_resume_and_get(info->dev) < 0)
+		goto error;
+
+	mutex_lock(&info->lock);
+
+	start_effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
+	if (start_effect) {
+		while (--work_data->count >= 0) {
+			info->dsp.write(info->dev, info->regmap, start_effect->index);
+			usleep_range(work_data->effect->replay.length,
+				     work_data->effect->replay.length + 100);
+		}
+	}
+
+	mutex_unlock(&info->lock);
+
+	if (!start_effect)
+		dev_err(info->dev, "Effect to play not found\n");
+
+	pm_runtime_mark_last_busy(info->dev);
+	pm_runtime_put_autosuspend(info->dev);
+error:
+	kfree(work_data);
+}
+
+static void vibra_stop_worker(struct work_struct *work)
+{
+	struct vibra_work *work_data = container_of(work, struct vibra_work, work);
+	struct vibra_info *info = work_data->info;
+
+	if (pm_runtime_resume_and_get(info->dev) < 0)
+		return;
+
+	mutex_lock(&info->lock);
+
+	info->dsp.write(info->dev, info->regmap, info->dsp.stop_cmd);
+
+	mutex_unlock(&info->lock);
+
+	pm_runtime_mark_last_busy(info->dev);
+	pm_runtime_put_autosuspend(info->dev);
+
+	kfree(work_data);
+}
+
+static int vibra_playback(struct input_dev *dev, int effect_id, int val)
+{
+	struct vibra_info *info = input_get_drvdata(dev);
+	struct vibra_work *work_data;
+
+	work_data = kzalloc(sizeof(*work_data), GFP_ATOMIC);
+	if (!work_data)
+		return -ENOMEM;
+
+	work_data->info = info;
+
+	if (val > 0) {
+		work_data->effect = &dev->ff->effects[effect_id];
+		work_data->count = val;
+		INIT_WORK(&work_data->work, vibra_start_worker);
+	} else {
+		/* Stop the amplifier as device can only drive one effect */
+		INIT_WORK(&work_data->work, vibra_stop_worker);
+	}
+
+	queue_work(info->vibe_wq, &work_data->work);
+
+	return 0;
+}
+
+static void vibra_erase_worker(struct work_struct *work)
+{
+	struct vibra_work *work_data = container_of(work, struct vibra_work, work);
+	struct vibra_effect *owt_effect, *erase_effect;
+	struct vibra_info *info = work_data->info;
+	int error;
+
+	error = pm_runtime_resume_and_get(info->dev);
+	if (error < 0)
+		goto exit;
+
+	mutex_lock(&info->lock);
+
+	erase_effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
+	if (!erase_effect) {
+		dev_err(info->dev, "Effect to erase not found\n");
+		error = -EINVAL;
+		goto error;
+	}
+
+	if (erase_effect->gpio_reg != CS40L50_GPIO_MAPPING_NONE) {
+		error = regmap_write(info->regmap, erase_effect->gpio_reg,
+				     CS40L50_GPIO_DISABLE);
+		if (error)
+			goto error;
+	}
+
+	if (erase_effect->bank == WVFRM_BANK_OWT) {
+		error = info->dsp.write(info->dev, info->regmap,
+					info->dsp.delete_owt_cmd |
+					erase_effect->index);
+		if (error)
+			goto error;
+
+		list_for_each_entry(owt_effect, &info->effect_head, list)
+			if (owt_effect->bank == WVFRM_BANK_OWT &&
+			    owt_effect->index > erase_effect->index)
+				owt_effect->index--;
+	}
+
+	list_del(&erase_effect->list);
+	kfree(erase_effect);
+error:
+	mutex_unlock(&info->lock);
+
+	pm_runtime_mark_last_busy(info->dev);
+	pm_runtime_put_autosuspend(info->dev);
+exit:
+	work_data->error = error;
+}
+
+static int vibra_erase(struct input_dev *dev, int effect_id)
+{
+	struct vibra_info *info = input_get_drvdata(dev);
+	struct vibra_work work_data = { info, &dev->ff->effects[effect_id], };
+
+	INIT_WORK(&work_data.work, vibra_erase_worker);
+
+	/* Push to workqueue to serialize with playbacks */
+	queue_work(info->vibe_wq, &work_data.work);
+	flush_work(&work_data.work);
+
+	return work_data.error;
+}
+
+static void vibra_remove_wq(void *data)
+{
+	struct vibra_info *info = data;
+
+	flush_workqueue(info->vibe_wq);
+	destroy_workqueue(info->vibe_wq);
+}
+
+static int cs40l50_vibra_probe(struct platform_device *pdev)
+{
+	struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
+	struct vibra_info *info;
+	int error;
+
+	info = devm_kzalloc(pdev->dev.parent, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	mutex_init(&info->lock);
+
+	info->dev = cs40l50->dev;
+	info->regmap = cs40l50->regmap;
+	info->dsp = cs40l50_dsp;
+
+	info->input = devm_input_allocate_device(info->dev);
+	if (!info->input)
+		return -ENOMEM;
+	info->input->id.product = cs40l50->devid & 0xFFFF;
+	info->input->id.version = cs40l50->revid;
+	info->input->name = "cs40l50_vibra";
+
+	input_set_drvdata(info->input, info);
+	input_set_capability(info->input, EV_FF, FF_PERIODIC);
+	input_set_capability(info->input, EV_FF, FF_CUSTOM);
+
+	error = input_ff_create(info->input, FF_MAX_EFFECTS);
+	if (error) {
+		dev_err(info->dev, "Failed to create input device\n");
+		return error;
+	}
+
+	info->input->ff->upload = vibra_add;
+	info->input->ff->playback = vibra_playback;
+	info->input->ff->erase = vibra_erase;
+
+	INIT_LIST_HEAD(&info->effect_head);
+
+	info->vibe_wq = alloc_ordered_workqueue("vibe_wq", WQ_HIGHPRI);
+	if (!info->vibe_wq)
+		return -ENOMEM;
+
+	error = devm_add_action_or_reset(info->dev, vibra_remove_wq, info);
+	if (error)
+		return error;
+
+	return input_register_device(info->input);
+}
+
+static const struct platform_device_id vibra_id_match[] = {
+	{ "cs40l50-vibra", },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, vibra_id_match);
+
+static struct platform_driver cs40l50_vibra_driver = {
+	.probe		= cs40l50_vibra_probe,
+	.id_table	= vibra_id_match,
+	.driver		= {
+		.name	= "cs40l50-vibra",
+	},
+};
+module_platform_driver(cs40l50_vibra_driver);
+
+MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* [PATCH v8 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
From: James Ogletree @ 2024-02-21  0:36 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
	broonie, jeff
  Cc: patches, linux-sound, linux-input, devicetree, James Ogletree,
	Krzysztof Kozlowski
In-Reply-To: <20240221003630.2535938-1-jogletre@opensource.cirrus.com>

The CS40L50 is a haptic driver with waveform memory,
integrated DSP, and closed-loop algorithms.

Add a YAML DT binding document for this device.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
 .../bindings/input/cirrus,cs40l50.yaml        | 70 +++++++++++++++++++
 MAINTAINERS                                   |  8 +++
 2 files changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml

diff --git a/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
new file mode 100644
index 000000000000..6a5bdafed56b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cirrus,cs40l50.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS40L50 Advanced Haptic Driver
+
+maintainers:
+  - James Ogletree <james.ogletree@cirrus.com>
+
+description:
+  CS40L50 is a haptic driver with waveform memory,
+  integrated DSP, and closed-loop algorithms.
+
+properties:
+  compatible:
+    enum:
+      - cirrus,cs40l50
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  va-supply:
+    description: Power supply for internal analog circuits.
+
+  vp-supply:
+    description: Power supply for always-on circuits.
+
+  vio-supply:
+    description: Power supply for digital input/output.
+
+  vamp-supply:
+    description: Power supply for the Class D amplifier.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - reset-gpios
+  - vp-supply
+  - vio-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      haptic-driver@34 {
+        compatible = "cirrus,cs40l50";
+        reg = <0x34>;
+        interrupt-parent = <&gpio>;
+        interrupts = <113 IRQ_TYPE_LEVEL_LOW>;
+        reset-gpios = <&gpio 112 GPIO_ACTIVE_LOW>;
+        vp-supply = <&vreg>;
+        vio-supply = <&vreg>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index dd5de540ec0b..b71017a187f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4933,6 +4933,14 @@ F:	sound/pci/hda/cs*
 F:	sound/pci/hda/hda_cs_dsp_ctl.*
 F:	sound/soc/codecs/cs*
 
+CIRRUS LOGIC HAPTIC DRIVERS
+M:	James Ogletree <james.ogletree@cirrus.com>
+M:	Fred Treven <fred.treven@cirrus.com>
+M:	Ben Bright <ben.bright@cirrus.com>
+L:	patches@opensource.cirrus.com
+S:	Supported
+F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
+
 CIRRUS LOGIC DSP FIRMWARE DRIVER
 M:	Simon Trimmer <simont@opensource.cirrus.com>
 M:	Charles Keepax <ckeepax@opensource.cirrus.com>
-- 
2.25.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox