[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87a5zbhrdf.fsf@nvidia.com>
Date: Fri, 14 Apr 2023 00:09:32 -0700
From: Rahul Rameshbabu <rrameshbabu@...dia.com>
To: Jiri Kosina <jikos@...nel.org>
Cc: Benjamin Tissoires <benjamin.tissoires@...hat.com>,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] HID: shield: Initial driver implementation with
Thunderstrike support
Hi Jiri,
Thanks for the feedback.
On Thu, 13 Apr, 2023 17:07:00 +0200 Jiri Kosina <jikos@...nel.org> wrote:
> On Mon, 10 Apr 2023, Rahul Rameshbabu wrote:
>
>> Supports the Thunderstrike (SHIELD 2017) controller. Implements support for
>> the Thunderstrike HOSTCMD firmware interface. Adds sysfs attributes about a
>> SHIELD device and introduces haptics support for controllers.
>>
>> Signed-off-by: Rahul Rameshbabu <rrameshbabu@...dia.com>
>
> Thanks a lot for submitting the driver. I have one minor question:
>
> [ ... snip ... ]
>> +thunderstrike_update_haptics(struct thunderstrike *ts,
>> + struct thunderstrike_hostcmd_haptics *motors)
>> +{
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&ts->haptics_update_lock, flags);
>> + ts->haptics_val = *motors;
>> + spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
>
> Do we really have to disable interrupts when taking haptics_update_lock?
> Is it ever being taken from interrupt context?
The reason why I disable interrupts is because I call
thunderstrike_update_haptics from the play_effect callback I pass to
input_ff_create_memless. From there, it is used in ml_effect_timer
drivers/input/ff-memless.c, which is a timer interrupt context so we
should disable interrupts when taking haptics_update_lock.
static void ml_effect_timer(struct timer_list *t)
{
struct ml_device *ml = from_timer(ml, t, timer);
struct input_dev *dev = ml->dev;
unsigned long flags;
pr_debug("timer: updating effects\n");
spin_lock_irqsave(&dev->event_lock, flags);
ml_play_effects(ml);
spin_unlock_irqrestore(&dev->event_lock, flags);
}
Let me know if this seems off.
>
> Thanks,
-- Rahul Rameshbabu
Powered by blists - more mailing lists