Hi Jiri,
Thanks for the feedback.
On Thu, 13 Apr, 2023 17:07:00 +0200 Jiri Kosina <jikos@xxxxxxxxxx> 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@xxxxxxxxxx>
>
> 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
