Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Rahul Rameshbabu <rrameshbabu@nvidia.com>
To:     Jiri Kosina <jikos@kernel.org>
Cc:     Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/1] HID: shield: Initial driver implementation with
 Thunderstrike support
In-Reply-To: <nycvar.YFH.7.76.2304131705350.29760@cbobk.fhfr.pm> (Jiri
        Kosina's message of "Thu, 13 Apr 2023 17:07:00 +0200 (CEST)")
References: <20230410170840.16119-1-rrameshbabu@nvidia.com>
        <20230410170840.16119-2-rrameshbabu@nvidia.com>
        <nycvar.YFH.7.76.2304131705350.29760@cbobk.fhfr.pm>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Date:   Fri, 14 Apr 2023 00:09:32 -0700
Message-ID: <87a5zbhrdf.fsf@nvidia.com>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?b/76MLpCpPu3MpPaPqUICCB1Mg+NhGL5QDyeQ8zzXQmo/RYnO2MvmrHzUoPT?=
 =?us-ascii?Q?T3jRevv/J7/LCbmbWXtUDVC6sihOZZO0pzwSv1xIEcXmHCBCfrjOtgvdMLBA?=
 =?us-ascii?Q?/zChVce1B0B5O1iuxV43ooBZezblWzyzksQBmfPMaq2Gb47mZTRrx5kn1ebB?=
 =?us-ascii?Q?ZwfCAT181Oh07+yRPM5hC0UAUnQNa6Z0avH1b4WXKiG0Ka9QGcmFcS2QxpuA?=
 =?us-ascii?Q?QdWjS0Bf2PMIFpmaegDl9EqIckUYLO+mm8bN+Zm1tiTY4TrDE2wjy+EPlk9K?=
 =?us-ascii?Q?ozARL8b60c8j2MTdJ5nQ5MyBqUsihBKqTiyuQuI4SX8XrxqtutP3Z51Za8cZ?=
 =?us-ascii?Q?65bH9zqsORqXFieTpHJz4a2kh8Brs2O0V+kwEFc7+mRomEyco6z0dnN8nTqh?=
 =?us-ascii?Q?ZPObq9STCSYp9wWZXMAQWTu5WRecTdKGx16FczKczAddlXAWrF4Ah5GwLQ2Y?=
 =?us-ascii?Q?4Pnn0IRt4AljO+QA7fzQCyjo4NeQcNGzn+waazA63UGnu7h27Z320qz1A+Id?=
 =?us-ascii?Q?vlK1dbBJlbNkblICsmDG0GAixpizx77Rx3vOZ3Uc0nwy+OnCKXCZOhWyreTF?=
 =?us-ascii?Q?KEtXLZ3JEIw1eMCeenOEfkrva5kZ/oqjZorKjCXP8vaR/qAOhZ+Q9RtTBFP6?=
 =?us-ascii?Q?6GXTjBE7aVRrGDLy+Hazy8q2RNLX7BdjngrSzA82/MaIDo2nHGcUVUbO6C5Z?=
 =?us-ascii?Q?hDn/UaLE9ieHFkdT13mwJ/kddq7M3Yr0SgjbLgcur4T7jJa7vPv5Iu9lllGu?=
 =?us-ascii?Q?XHNSPkg/82zEV5VpHlvnudCcqCrIugho70TsBSLy7brSa3qS84HIJQOa9kkL?=
 =?us-ascii?Q?hAHZWIJ86Zwh4m1K0iCwG22u+roE4e6oaFb2tWr4tijP4fHUQjbK/S8NK+g4?=
 =?us-ascii?Q?twzKk52pI0Rv/dN7Lp5xFsaIQbmIypb8+ci0PL6sYDdI6y41q01svkqSDR39?=
 =?us-ascii?Q?SNvvP2MYRz6jGWJ3VAOggJ5wxIxFNB/zIwn9bDpxabPhyrau+qHWRjhm4hyM?=
 =?us-ascii?Q?h/z02QOtTrhqHvSLgLaFnM4h3p24fbD9XMMg4VSoZjYlv0uOaROXbKe4SiIi?=
 =?us-ascii?Q?cBspkGx9BmkKpzLSf5bZVL/eN9qL8fbQmvQPAQuUVcTLFF0KMxulJYbCUP8g?=
 =?us-ascii?Q?KFzUF4uOFIC8EBXdmJMfsGXXSucDzr+JUH4CAIc55bfwCTfiHCkHvR3df0m6?=
 =?us-ascii?Q?84ZWlhKEjmn74WUtD0lW1YGQavs2195IBY894GrXqJffMIg0eK4UR2gMIeFW?=
 =?us-ascii?Q?S+XiEwkmi0UdyMuDE/gYfSMkuzOpnyHyYzw0Xt52NJ/sXwuxjhgvxFdZGiAo?=
 =?us-ascii?Q?9YTyDSIE8gR72fzuXdUT/7FBg/7yJgBv7BVCxAbxBKYM7Dlns66VtZXFx+YY?=
 =?us-ascii?Q?7Oh3J3oRlirHI8o/bCQYxUNIy4aH/JgcsIVRrCD9EnbKoLR+Vd6IXHBJIjgm?=
 =?us-ascii?Q?wsFUBuXLATmfUT6EgW1Fy5FBMGL8FTnhi3FK8njfLa/Uerfb8zRFzwL+4yem?=
 =?us-ascii?Q?lNbR8F6/q8Fn2JHwZNF2+387I8deWdqvxP0lZbytDArqFL+obfa3JYmbLoPh?=
 =?us-ascii?Q?GC48nxbjmrhdBpsxUyqC2EtZlKfQ+9YHWwEE0M7ieL0oW7Nd7DmYpEGCpZKY?=
 =?us-ascii?Q?fA=3D=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a32cfe84-6791-41c1-0c3c-08db3cb73924
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB2743.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Apr 2023 07:09:44.0586
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cBSYvFYWWs5hJ0BNcJiDoSjuJbxC/PPmLOY5ecPVjOO95T61lCa0hTs3+sUMJSADq+XfgJdCuYzUSukFMfzDUw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB7599
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jiri,

Thanks for the feedback.

On Thu, 13 Apr, 2023 17:07:00 +0200 Jiri Kosina <jikos@kernel.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@nvidia.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