From patchwork Thu Jul 27 17:16:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 707077 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EBF33C001E0 for ; Thu, 27 Jul 2023 17:18:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232601AbjG0RSr (ORCPT ); Thu, 27 Jul 2023 13:18:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50974 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231602AbjG0RSh (ORCPT ); Thu, 27 Jul 2023 13:18:37 -0400 Received: from mail-pf1-x42e.google.com (mail-pf1-x42e.google.com [IPv6:2607:f8b0:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7185D128 for ; Thu, 27 Jul 2023 10:18:35 -0700 (PDT) Received: by mail-pf1-x42e.google.com with SMTP id d2e1a72fcca58-686f38692b3so869294b3a.2 for ; Thu, 27 Jul 2023 10:18:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1690478315; x=1691083115; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=KQZ8Cw06bentZdl+4zG5W3NqtbC6pKPyy9qdL8J5He4=; b=Wtom9MHIMsOHg/Z5VkFflz+bbEfQkzNKAwvEaF2r5kP67rXgwP4lABgDusT6irOTWp i+kJfgTKgHTJio8COvmtKItEZCkajdrmWCUQnGqX8I0XsIE2KGhYFYiXLYUuu3PivQ+e nssRUDSVXyRXCS49j6EwevNZvfsHtD/m1nATk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690478315; x=1691083115; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KQZ8Cw06bentZdl+4zG5W3NqtbC6pKPyy9qdL8J5He4=; b=G3NCYIo6onyuF8zyZOf1Ro4ZHWQrrKXmpx2KrVlI+756V9E6lCKzj0tbpULdrM+HF/ GBgle3/YnrJBIKSIF+q85nX1fYGQPii18Hpz0De4ZFyITHSSvK3qlPamtX+thic5DLpq WV6eUVtG55z5bcY6VEz1lD37eclSBGw5sj4K6L+WtmrzQQlK7vYrhl8OpPxMghIIhJq7 fZO4+hI+VMRc+mX9hPS2L2HWYS4LNxFDZEcu+PiMpAKO5EYJ0or4xpaYGooJWVzjLpIb qXxDz6Pb4QsNIycm9X+FcnW4Lvi+5F9RsuFBf3aJedjvcpL3Lu4QU0sWK3aPTQ87KIAZ vQxw== X-Gm-Message-State: ABy/qLbmXGl7Q2H8NQQ+LfhZt6hQkDwVWMcKVqnJXqSEQ0Bcioch66GO kJeMJGa5ZOVxLqrC7lFzUuqIzA== X-Google-Smtp-Source: APBJJlGmZC9borubx3T6e0kzMzxMxJUj+Df+XCqWavB4O4v7bS4lCkgxWXjA17w0bSDBXf5fn5KDug== X-Received: by 2002:a05:6a20:6a04:b0:133:21c3:115e with SMTP id p4-20020a056a206a0400b0013321c3115emr7569088pzk.48.1690478314913; Thu, 27 Jul 2023 10:18:34 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:2339:954b:b98f:611a]) by smtp.gmail.com with ESMTPSA id 17-20020aa79111000000b0064f76992905sm1702524pfh.202.2023.07.27.10.18.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jul 2023 10:18:34 -0700 (PDT) From: Douglas Anderson To: Jiri Kosina , Benjamin Tissoires , Bjorn Andersson , Konrad Dybcio , Rob Herring , Frank Rowand , Krzysztof Kozlowski , Conor Dooley , Neil Armstrong , Sam Ravnborg , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann Cc: linux-arm-msm@vger.kernel.org, yangcong5@huaqin.corp-partner.google.com, devicetree@vger.kernel.org, Daniel Vetter , hsinyi@google.com, Chris Morgan , linux-input@vger.kernel.org, cros-qcom-dts-watchers@chromium.org, Dmitry Torokhov , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Douglas Anderson Subject: [PATCH v4 02/11] drm/panel: Check for already prepared/enabled in drm_panel Date: Thu, 27 Jul 2023 10:16:29 -0700 Message-ID: <20230727101636.v4.2.I59b417d4c29151cc2eff053369ec4822b606f375@changeid> X-Mailer: git-send-email 2.41.0.487.g6d72f3e995-goog In-Reply-To: <20230727171750.633410-1-dianders@chromium.org> References: <20230727171750.633410-1-dianders@chromium.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org In a whole pile of panel drivers, we have code to make the prepare/unprepare/enable/disable callbacks behave as no-ops if they've already been called. It's silly to have this code duplicated everywhere. Add it to the core instead so that we can eventually delete it from all the drivers. Note: to get some idea of the duplicated code, try: git grep 'if.*>prepared' -- drivers/gpu/drm/panel git grep 'if.*>enabled' -- drivers/gpu/drm/panel NOTE: arguably, the right thing to do here is actually to skip this patch and simply remove all the extra checks from the individual drivers. Perhaps the checks were needed at some point in time in the past but maybe they no longer are? Certainly as we continue transitioning over to "panel_bridge" then we expect there to be much less variety in how these calls are made. When we're called as part of the bridge chain, things should be pretty simple. In fact, there was some discussion in the past about these checks [1], including a discussion about whether the checks were needed and whether the calls ought to be refcounted. At the time, I decided not to mess with it because it felt too risky. Looking closer at it now, I'm fairly certain that nothing in the existing codebase is expecting these calls to be refcounted. The only real question is whether someone is already doing something to ensure prepare()/unprepare() match and enabled()/disable() match. I would say that, even if there is something else ensuring that things match, there's enough complexity that adding an extra bool and an extra double-check here is a good idea. Let's add a drm_warn() to let people know that it's considered a minor error to take advantage of drm_panel's double-checking but we'll still make things work fine. We'll also add an entry to the official DRM todo list to remove the now pointless check from the panels after this patch lands and, eventually, fixup anyone who is triggering the new warning. [1] https://lore.kernel.org/r/20210416153909.v4.27.I502f2a92ddd36c3d28d014dd75e170c2d405a0a5@changeid Acked-by: Neil Armstrong Reviewed-by: Maxime Ripard Signed-off-by: Douglas Anderson --- This has Neil's Ack and Maxime's review and I could commit it to drm-misc-next, but for now I'm holding off to land with the rest of this series since the second drm patch in my series depends on this one. Once this lands somewhere, I'll take the action item to post patches to delete the now pointless checks in the individual panels. Changes in v4: - Document further cleanup in the official DRM todo list. Documentation/gpu/todo.rst | 24 ++++++++++++++++++ drivers/gpu/drm/drm_panel.c | 49 ++++++++++++++++++++++++++++++++----- include/drm/drm_panel.h | 14 +++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 68bdafa0284f..e3b272c97758 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -452,6 +452,30 @@ Contact: Thomas Zimmermann Level: Starter +Clean up checks for already prepared/enabled in panels +------------------------------------------------------ + +In a whole pile of panel drivers, we have code to make the +prepare/unprepare/enable/disable callbacks behave as no-ops if they've already +been called. To get some idea of the duplicated code, try: + git grep 'if.*>prepared' -- drivers/gpu/drm/panel + git grep 'if.*>enabled' -- drivers/gpu/drm/panel + +In the patch ("drm/panel: Check for already prepared/enabled in drm_panel") +we've moved this check to the core. Now we can most definitely remove the +check from the individual panels and save a pile of code. + +In adition to removing the check from the individual panels, it is believed +that even the core shouldn't need this check and that should be considered +an error if other code ever relies on this check. The check in the core +currently prints a warning whenever something is relying on this check with +dev_warn(). After a little while, we likely want to promote this to a +WARN(1) to help encourage folks not to rely on this behavior. + +Contact: Douglas Anderson + +Level: Starter/Intermediate + Core refactorings ================= diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c index f634371c717a..4e1c4e42575b 100644 --- a/drivers/gpu/drm/drm_panel.c +++ b/drivers/gpu/drm/drm_panel.c @@ -105,11 +105,22 @@ EXPORT_SYMBOL(drm_panel_remove); */ int drm_panel_prepare(struct drm_panel *panel) { + int ret; + if (!panel) return -EINVAL; - if (panel->funcs && panel->funcs->prepare) - return panel->funcs->prepare(panel); + if (panel->prepared) { + dev_warn(panel->dev, "Skipping prepare of already prepared panel\n"); + return 0; + } + + if (panel->funcs && panel->funcs->prepare) { + ret = panel->funcs->prepare(panel); + if (ret < 0) + return ret; + } + panel->prepared = true; return 0; } @@ -128,11 +139,22 @@ EXPORT_SYMBOL(drm_panel_prepare); */ int drm_panel_unprepare(struct drm_panel *panel) { + int ret; + if (!panel) return -EINVAL; - if (panel->funcs && panel->funcs->unprepare) - return panel->funcs->unprepare(panel); + if (!panel->prepared) { + dev_warn(panel->dev, "Skipping unprepare of already unprepared panel\n"); + return 0; + } + + if (panel->funcs && panel->funcs->unprepare) { + ret = panel->funcs->unprepare(panel); + if (ret < 0) + return ret; + } + panel->prepared = false; return 0; } @@ -155,11 +177,17 @@ int drm_panel_enable(struct drm_panel *panel) if (!panel) return -EINVAL; + if (panel->enabled) { + dev_warn(panel->dev, "Skipping enable of already enabled panel\n"); + return 0; + } + if (panel->funcs && panel->funcs->enable) { ret = panel->funcs->enable(panel); if (ret < 0) return ret; } + panel->enabled = true; ret = backlight_enable(panel->backlight); if (ret < 0) @@ -187,13 +215,22 @@ int drm_panel_disable(struct drm_panel *panel) if (!panel) return -EINVAL; + if (!panel->enabled) { + dev_warn(panel->dev, "Skipping disable of already disabled panel\n"); + return 0; + } + ret = backlight_disable(panel->backlight); if (ret < 0) DRM_DEV_INFO(panel->dev, "failed to disable backlight: %d\n", ret); - if (panel->funcs && panel->funcs->disable) - return panel->funcs->disable(panel); + if (panel->funcs && panel->funcs->disable) { + ret = panel->funcs->disable(panel); + if (ret < 0) + return ret; + } + panel->enabled = false; return 0; } diff --git a/include/drm/drm_panel.h b/include/drm/drm_panel.h index 432fab2347eb..c6cf75909389 100644 --- a/include/drm/drm_panel.h +++ b/include/drm/drm_panel.h @@ -198,6 +198,20 @@ struct drm_panel { * the panel is powered up. */ bool prepare_prev_first; + + /** + * @prepared: + * + * If true then the panel has been prepared. + */ + bool prepared; + + /** + * @enabled: + * + * If true then the panel has been enabled. + */ + bool enabled; }; void drm_panel_init(struct drm_panel *panel, struct device *dev, From patchwork Thu Jul 27 17:16:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 707076 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 529DAC001E0 for ; Thu, 27 Jul 2023 17:19:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232747AbjG0RTB (ORCPT ); Thu, 27 Jul 2023 13:19:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51024 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232412AbjG0RSs (ORCPT ); Thu, 27 Jul 2023 13:18:48 -0400 Received: from mail-pf1-x42f.google.com (mail-pf1-x42f.google.com [IPv6:2607:f8b0:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 02BCE30F9 for ; Thu, 27 Jul 2023 10:18:40 -0700 (PDT) Received: by mail-pf1-x42f.google.com with SMTP id d2e1a72fcca58-686f0d66652so1078277b3a.2 for ; Thu, 27 Jul 2023 10:18:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1690478319; x=1691083119; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=B+sJqO38e2gNSysz3Tl3WNQ34X1K+Pprq+XQpM/wPpA=; b=XR79+iJg38TdqRqVksssPvqJCfRVppUDJ0SCoIzYZz9E7npJ3QbCrYBR0YtCH1Iwpz bh8ddXSBmExnRlwhNEaqyewsrL8WmSIwwp8P2pEYNXLOnclbvGHTfebTxUYiViXnkb3Q cJhn2pL+oUoG+Carw6pl4TGtM19+hj0XsDhTo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690478319; x=1691083119; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=B+sJqO38e2gNSysz3Tl3WNQ34X1K+Pprq+XQpM/wPpA=; b=BoS8jCbq8/edp2AVT0Y0awasth6KDywP4K1G/TRsPadwgA7GMZgXj9IZ88ZzNR7IeG djO5+s0NOL+VZbovyAhXQDmuQZW+ZqCQLz2Ip6Q3kZ+4E94KIMWq870Ktm1yU+nqW1oy yTUVlUDrhW57B+mxMSrm2oQOoId6r56PFgbKEnVwU0sAYNC5ZN2WAKv9/e+BGy11S18I CL3Tgn+KVjS05xKEAiCIpAZ6A8JaHjrQ3tOZjRsE0qkiWk9L2L+5cJhiHi06jSx8SAu0 QNO8YADLBC0yapGHX2Ezj2gpY0Fr8GPbrifM5zSBNDA4MfnIZPe9EPF2LkLoOLWs6y7n i6MA== X-Gm-Message-State: ABy/qLZAdyG696UCJL0d7gnlyKuesdViPHK9cdTl25VtQ5dEwIJODTzB 2NaphppG7+vCHBFqbb8pe/L6jg== X-Google-Smtp-Source: APBJJlEtdcPHvIy7yXOPJnhlhZ0fZ1vrgQ0KPFYytG6arm/TebgKO/M3EPsrsa35yfXcgeHGVYzcKw== X-Received: by 2002:a05:6a21:6d88:b0:134:4f86:7966 with SMTP id wl8-20020a056a216d8800b001344f867966mr7749482pzb.9.1690478319489; Thu, 27 Jul 2023 10:18:39 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:2339:954b:b98f:611a]) by smtp.gmail.com with ESMTPSA id 17-20020aa79111000000b0064f76992905sm1702524pfh.202.2023.07.27.10.18.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jul 2023 10:18:38 -0700 (PDT) From: Douglas Anderson To: Jiri Kosina , Benjamin Tissoires , Bjorn Andersson , Konrad Dybcio , Rob Herring , Frank Rowand , Krzysztof Kozlowski , Conor Dooley , Neil Armstrong , Sam Ravnborg , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann Cc: linux-arm-msm@vger.kernel.org, yangcong5@huaqin.corp-partner.google.com, devicetree@vger.kernel.org, Daniel Vetter , hsinyi@google.com, Chris Morgan , linux-input@vger.kernel.org, cros-qcom-dts-watchers@chromium.org, Dmitry Torokhov , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Douglas Anderson , Rob Herring Subject: [PATCH v4 04/11] of: property: fw_devlink: Add a devlink for panel followers Date: Thu, 27 Jul 2023 10:16:31 -0700 Message-ID: <20230727101636.v4.4.Ibf8e1342b5b7906279db2365aca45e6253857bb3@changeid> X-Mailer: git-send-email 2.41.0.487.g6d72f3e995-goog In-Reply-To: <20230727171750.633410-1-dianders@chromium.org> References: <20230727171750.633410-1-dianders@chromium.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Inform fw_devlink of the fact that a panel follower (like a touchscreen) is effectively a consumer of the panel from the purposes of fw_devlink. NOTE: this patch isn't required for correctness but instead optimizes probe order / helps avoid deferrals. Acked-by: Rob Herring Reviewed-by: Maxime Ripard Signed-off-by: Douglas Anderson --- Since this is so small, I'd presume it's OK for it to go through a DRM tree with the proper Ack. That being said, this patch is just an optimization and thus it could land completely separately from the rest and they could all meet up in mainline. (no changes since v2) Changes in v2: - ("Add a devlink for panel followers") new for v2. drivers/of/property.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/of/property.c b/drivers/of/property.c index ddc75cd50825..cf8dacf3e3b8 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -1266,6 +1266,7 @@ DEFINE_SIMPLE_PROP(pwms, "pwms", "#pwm-cells") DEFINE_SIMPLE_PROP(resets, "resets", "#reset-cells") DEFINE_SIMPLE_PROP(leds, "leds", NULL) DEFINE_SIMPLE_PROP(backlight, "backlight", NULL) +DEFINE_SIMPLE_PROP(panel, "panel", NULL) DEFINE_SUFFIX_PROP(regulators, "-supply", NULL) DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells") @@ -1354,6 +1355,7 @@ static const struct supplier_bindings of_supplier_bindings[] = { { .parse_prop = parse_resets, }, { .parse_prop = parse_leds, }, { .parse_prop = parse_backlight, }, + { .parse_prop = parse_panel, }, { .parse_prop = parse_gpio_compat, }, { .parse_prop = parse_interrupts, }, { .parse_prop = parse_regulators, }, From patchwork Thu Jul 27 17:16:33 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 707075 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0B582C001E0 for ; Thu, 27 Jul 2023 17:19:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230484AbjG0RTV (ORCPT ); Thu, 27 Jul 2023 13:19:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51010 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232798AbjG0RTG (ORCPT ); Thu, 27 Jul 2023 13:19:06 -0400 Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AF7AC3A80 for ; Thu, 27 Jul 2023 10:18:45 -0700 (PDT) Received: by mail-pf1-x42d.google.com with SMTP id d2e1a72fcca58-687087d8ddaso262562b3a.1 for ; Thu, 27 Jul 2023 10:18:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1690478324; x=1691083124; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ktpWtMwT9dpeC33OUaPHH6/Z5MIjVDj91Q86MN6Mk68=; b=e6DcIHEEnnX3iIOWGLqJ7y9zqMALlfXD5b2RWvBK62PzvT1+HUn4sLyb1zwNNYmEv0 FcT63N8JPGM1eTXDVZt2Hr8pIf9AGYh7ZwpqJOGANz8WWDL8DzJFqgNsAarfmhvMb/xp 2ytjgoAe177xfFghUbeJWiyjBs/6yiKjoWV+k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690478324; x=1691083124; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ktpWtMwT9dpeC33OUaPHH6/Z5MIjVDj91Q86MN6Mk68=; b=gQNEJtjG4wf+20wGZXdDVGWL1abT2oxLLZxJrNC7VBHVVnYZ8zagNHjq7gNawuIrF+ GNVOKa3AqVnzExJXb+uS2iv6tY4oKTPZtk59/7sl4E5n02yZzM1oXDHYcqJzOzKimCcc iXzvnJJu3jHiOX7YeWEXHqiNM606CLH8bz1SlZF9KySWvXEBT2FxbIer9SzCnes2IhxG V2yMAHSA7JY/kS7G+LRSI+vKqa2UM8e/jfX9LWQ9tCOeRVvdonNj/H/CjjkOFxkHhF5n JD+SvZY35w43SWVYIi5U0w3YV9CEIX8wyKlQb/FzTXP0ipNNPRJnXGXUSHlHetlhVx6y 5qlA== X-Gm-Message-State: ABy/qLYhvSsE4NtVrbC1eHEpYJyzLlqLz0/SWD074TT2GoHgoE0NSrqf d+3HJn3Oej0u0h81gQzd4D3u3Q== X-Google-Smtp-Source: APBJJlFI4ulREcnYhiYEqiypn/CcfSaWl2pU5xhKDO414/TYYGi809LpHmTqvUM3kKTBTycyzjKtEA== X-Received: by 2002:a05:6a20:410:b0:12c:c614:f55 with SMTP id a16-20020a056a20041000b0012cc6140f55mr4946150pza.43.1690478324616; Thu, 27 Jul 2023 10:18:44 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:2339:954b:b98f:611a]) by smtp.gmail.com with ESMTPSA id 17-20020aa79111000000b0064f76992905sm1702524pfh.202.2023.07.27.10.18.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jul 2023 10:18:44 -0700 (PDT) From: Douglas Anderson To: Jiri Kosina , Benjamin Tissoires , Bjorn Andersson , Konrad Dybcio , Rob Herring , Frank Rowand , Krzysztof Kozlowski , Conor Dooley , Neil Armstrong , Sam Ravnborg , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann Cc: linux-arm-msm@vger.kernel.org, yangcong5@huaqin.corp-partner.google.com, devicetree@vger.kernel.org, Daniel Vetter , hsinyi@google.com, Chris Morgan , linux-input@vger.kernel.org, cros-qcom-dts-watchers@chromium.org, Dmitry Torokhov , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Douglas Anderson Subject: [PATCH v4 06/11] HID: i2c-hid: Rearrange probe() to power things up later Date: Thu, 27 Jul 2023 10:16:33 -0700 Message-ID: <20230727101636.v4.6.Ifcc9b0a44895d164788966f9b9511fe094ca8cf9@changeid> X-Mailer: git-send-email 2.41.0.487.g6d72f3e995-goog In-Reply-To: <20230727171750.633410-1-dianders@chromium.org> References: <20230727171750.633410-1-dianders@chromium.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org In a future patch, we want to change i2c-hid not to necessarily power up the touchscreen during probe. In preparation for that, rearrange the probe function so that we put as much stuff _before_ powering up the device as possible. This change is expected to have no functional effect. Reviewed-by: Maxime Ripard Signed-off-by: Douglas Anderson --- (no changes since v2) Changes in v2: - i2c_hid_core_initial_power_up() is now static. drivers/hid/i2c-hid/i2c-hid-core.c | 124 ++++++++++++++++++----------- 1 file changed, 77 insertions(+), 47 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 19d985c20a5c..d29e6421ecba 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -855,7 +855,8 @@ static int i2c_hid_init_irq(struct i2c_client *client) irqflags = IRQF_TRIGGER_LOW; ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq, - irqflags | IRQF_ONESHOT, client->name, ihid); + irqflags | IRQF_ONESHOT | IRQF_NO_AUTOEN, + client->name, ihid); if (ret < 0) { dev_warn(&client->dev, "Could not register for %s interrupt, irq = %d," @@ -940,6 +941,72 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid) ihid->ops->shutdown_tail(ihid->ops); } +/** + * i2c_hid_core_initial_power_up() - First time power up of the i2c-hid device. + * @ihid: The ihid object created during probe. + * + * This function is called at probe time. + * + * The initial power on is where we do some basic validation that the device + * exists, where we fetch the HID descriptor, and where we create the actual + * HID devices. + * + * Return: 0 or error code. + */ +static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid) +{ + struct i2c_client *client = ihid->client; + struct hid_device *hid = ihid->hid; + int ret; + + ret = i2c_hid_core_power_up(ihid); + if (ret) + return ret; + + /* Make sure there is something at this address */ + ret = i2c_smbus_read_byte(client); + if (ret < 0) { + i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret); + ret = -ENXIO; + goto err; + } + + ret = i2c_hid_fetch_hid_descriptor(ihid); + if (ret < 0) { + dev_err(&client->dev, + "Failed to fetch the HID Descriptor\n"); + goto err; + } + + enable_irq(client->irq); + + hid->version = le16_to_cpu(ihid->hdesc.bcdVersion); + hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID); + hid->product = le16_to_cpu(ihid->hdesc.wProductID); + + hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor, + hid->product); + + snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X", + client->name, (u16)hid->vendor, (u16)hid->product); + strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys)); + + ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product); + + ret = hid_add_device(hid); + if (ret) { + if (ret != -ENODEV) + hid_err(client, "can't add hid device: %d\n", ret); + goto err; + } + + return 0; + +err: + i2c_hid_core_power_down(ihid); + return ret; +} + int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops, u16 hid_descriptor_address, u32 quirks) { @@ -966,16 +1033,10 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops, if (!ihid) return -ENOMEM; - ihid->ops = ops; - - ret = i2c_hid_core_power_up(ihid); - if (ret) - return ret; - i2c_set_clientdata(client, ihid); + ihid->ops = ops; ihid->client = client; - ihid->wHIDDescRegister = cpu_to_le16(hid_descriptor_address); init_waitqueue_head(&ihid->wait); @@ -986,28 +1047,12 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops, * real computation later. */ ret = i2c_hid_alloc_buffers(ihid, HID_MIN_BUFFER_SIZE); if (ret < 0) - goto err_powered; - + return ret; device_enable_async_suspend(&client->dev); - /* Make sure there is something at this address */ - ret = i2c_smbus_read_byte(client); - if (ret < 0) { - i2c_hid_dbg(ihid, "nothing at this address: %d\n", ret); - ret = -ENXIO; - goto err_powered; - } - - ret = i2c_hid_fetch_hid_descriptor(ihid); - if (ret < 0) { - dev_err(&client->dev, - "Failed to fetch the HID Descriptor\n"); - goto err_powered; - } - ret = i2c_hid_init_irq(client); if (ret < 0) - goto err_powered; + goto err_buffers_allocated; hid = hid_allocate_device(); if (IS_ERR(hid)) { @@ -1021,26 +1066,11 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops, hid->ll_driver = &i2c_hid_ll_driver; hid->dev.parent = &client->dev; hid->bus = BUS_I2C; - hid->version = le16_to_cpu(ihid->hdesc.bcdVersion); - hid->vendor = le16_to_cpu(ihid->hdesc.wVendorID); - hid->product = le16_to_cpu(ihid->hdesc.wProductID); - hid->initial_quirks = quirks; - hid->initial_quirks |= i2c_hid_get_dmi_quirks(hid->vendor, - hid->product); - - snprintf(hid->name, sizeof(hid->name), "%s %04X:%04X", - client->name, (u16)hid->vendor, (u16)hid->product); - strscpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys)); - - ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product); - ret = hid_add_device(hid); - if (ret) { - if (ret != -ENODEV) - hid_err(client, "can't add hid device: %d\n", ret); + ret = i2c_hid_core_initial_power_up(ihid); + if (ret) goto err_mem_free; - } return 0; @@ -1050,9 +1080,9 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops, err_irq: free_irq(client->irq, ihid); -err_powered: - i2c_hid_core_power_down(ihid); +err_buffers_allocated: i2c_hid_free_buffers(ihid); + return ret; } EXPORT_SYMBOL_GPL(i2c_hid_core_probe); @@ -1062,6 +1092,8 @@ void i2c_hid_core_remove(struct i2c_client *client) struct i2c_hid *ihid = i2c_get_clientdata(client); struct hid_device *hid; + i2c_hid_core_power_down(ihid); + hid = ihid->hid; hid_destroy_device(hid); @@ -1069,8 +1101,6 @@ void i2c_hid_core_remove(struct i2c_client *client) if (ihid->bufsize) i2c_hid_free_buffers(ihid); - - i2c_hid_core_power_down(ihid); } EXPORT_SYMBOL_GPL(i2c_hid_core_remove); From patchwork Thu Jul 27 17:16:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 707074 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2A025C0015E for ; Thu, 27 Jul 2023 17:19:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233095AbjG0RT0 (ORCPT ); Thu, 27 Jul 2023 13:19:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51968 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233125AbjG0RTR (ORCPT ); Thu, 27 Jul 2023 13:19:17 -0400 Received: from mail-pf1-x435.google.com (mail-pf1-x435.google.com [IPv6:2607:f8b0:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1AE403AA2 for ; Thu, 27 Jul 2023 10:18:49 -0700 (PDT) Received: by mail-pf1-x435.google.com with SMTP id d2e1a72fcca58-686daaa5f1fso940174b3a.3 for ; Thu, 27 Jul 2023 10:18:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1690478329; x=1691083129; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=JE0/UXh96ihgYVclX63uFx20OU8KLkSmKPmtU0UMZZk=; b=jFm7Y3pVztqxRpdquGXn2GCDPzKNRJczGTzTOaX187e/SfwDrJBxSzzonbM4Bjz+P9 uEOhwdf/+JdRdT+MJHMzWK1WoaFhvxbd3DqXDhNFE9w32cSkuiw7MFJ6oFNQT3Vcy2p+ zzRxkIHv7qezN3fHVjV8gV3aAnNQYniV7Lb+8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690478329; x=1691083129; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JE0/UXh96ihgYVclX63uFx20OU8KLkSmKPmtU0UMZZk=; b=OgRYt7J4+shCv4XOkjvyzrDW27ChHhM32x6h/fojMPkhMj7rg5Hp8aCynB7q5FYunL K0Xjhn3mBw/SN1mZan2oMc6Z4qiY89IbY8c3laaBe5yHHNnP2Nw1Q6WkWViZeEux1c+f hRGszSt0yJc25eQvPie3JTB6gXKiHIXdJFp/sGDgF91Fr9gUTZWP0mr1EOkoTQmM99A4 EyHB8AP0DiZ7MC3CU70MvRH4zGzjvsu6o3/zAltPuzzP5uT4mwAvYQjLCP56pcOG7zD7 T+NGMbEFEgZwA13UD4aznhLPa8TJiqDE5B/IFXEjphAtprTiB0viC0qiduHgNU6VqCjv aEyA== X-Gm-Message-State: ABy/qLZek/1CN81arY5Tm+GeqQhIW2doyz4XMOkkArx6pZ8EFSoC3Dnh J9FZuNhy4XF6TSg3GIhiz5fhPw== X-Google-Smtp-Source: APBJJlG5PO4jOR+JrrecJ0XN7BS9lBiF+A7oJWjyuw0srUfej6DlO1pGmADkWGz3mc8LeneWVnX6nQ== X-Received: by 2002:a05:6a00:15cb:b0:686:be77:4311 with SMTP id o11-20020a056a0015cb00b00686be774311mr5711310pfu.4.1690478329127; Thu, 27 Jul 2023 10:18:49 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:2339:954b:b98f:611a]) by smtp.gmail.com with ESMTPSA id 17-20020aa79111000000b0064f76992905sm1702524pfh.202.2023.07.27.10.18.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jul 2023 10:18:48 -0700 (PDT) From: Douglas Anderson To: Jiri Kosina , Benjamin Tissoires , Bjorn Andersson , Konrad Dybcio , Rob Herring , Frank Rowand , Krzysztof Kozlowski , Conor Dooley , Neil Armstrong , Sam Ravnborg , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann Cc: linux-arm-msm@vger.kernel.org, yangcong5@huaqin.corp-partner.google.com, devicetree@vger.kernel.org, Daniel Vetter , hsinyi@google.com, Chris Morgan , linux-input@vger.kernel.org, cros-qcom-dts-watchers@chromium.org, Dmitry Torokhov , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Douglas Anderson Subject: [PATCH v4 08/11] HID: i2c-hid: Suspend i2c-hid devices in remove Date: Thu, 27 Jul 2023 10:16:35 -0700 Message-ID: <20230727101636.v4.8.Ic3ecad4a825905f4e4ce2a772b17f3c9cb2d60a2@changeid> X-Mailer: git-send-email 2.41.0.487.g6d72f3e995-goog In-Reply-To: <20230727171750.633410-1-dianders@chromium.org> References: <20230727171750.633410-1-dianders@chromium.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org In the i2c-hid remove() function we currently try to power off, depopulate our child device, and free our resources. That's OK, but... * If the i2c-hid device is on a power rail that can't turn off (either an always-on or a shared power rail) we won't try to put the device in a low power state during remove(). This probably doesn't matter for very many devices but it could be nice in some instances. * If the i2c-hid device somehow manages to generate an interrupt after we tried to power off it is conceivable that the interrupt could arrive during or after the call to hid_destroy_device() but before the call to free_irq(). That could cause a crash since our IRQ handler isn't expecting it. One could imagine this happening in the case where we couldn't turn off (see the previous bullet) or, possibly, if the interrupt line could glitch shortly after the device powered off. Let's call the suspend code during remove to avoid these issues. That will put the device into a low power state and also disable interrupts. Technically, one could consider this a "fix" of commit 4a200c3b9a40 ("HID: i2c-hid: introduce HID over i2c specification implementation"). However, since the above bullet points are more theoretical than problems seen on real systems and since the remove() of an i2c-hid touchscreen isn't terribly likely to be called in production, it's probably not worth the bother of trying to backport it. Signed-off-by: Douglas Anderson --- Changes in v4: - ("Suspend i2c-hid devices in remove") new for v4. drivers/hid/i2c-hid/i2c-hid-core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index fa8a1ca43d7f..46658ed6380f 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -941,7 +941,7 @@ static void i2c_hid_core_shutdown_tail(struct i2c_hid *ihid) ihid->ops->shutdown_tail(ihid->ops); } -static int i2c_hid_core_suspend(struct i2c_hid *ihid) +static int i2c_hid_core_suspend(struct i2c_hid *ihid, bool force_poweroff) { struct i2c_client *client = ihid->client; struct hid_device *hid = ihid->hid; @@ -956,7 +956,7 @@ static int i2c_hid_core_suspend(struct i2c_hid *ihid) disable_irq(client->irq); - if (!device_may_wakeup(&client->dev)) + if (force_poweroff || !device_may_wakeup(&client->dev)) i2c_hid_core_power_down(ihid); return 0; @@ -1143,7 +1143,7 @@ void i2c_hid_core_remove(struct i2c_client *client) struct i2c_hid *ihid = i2c_get_clientdata(client); struct hid_device *hid; - i2c_hid_core_power_down(ihid); + i2c_hid_core_suspend(ihid, true); hid = ihid->hid; hid_destroy_device(hid); @@ -1171,7 +1171,7 @@ static int i2c_hid_core_pm_suspend(struct device *dev) struct i2c_client *client = to_i2c_client(dev); struct i2c_hid *ihid = i2c_get_clientdata(client); - return i2c_hid_core_suspend(ihid); + return i2c_hid_core_suspend(ihid, false); } static int i2c_hid_core_pm_resume(struct device *dev) From patchwork Thu Jul 27 17:16:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 707073 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 19553C04A94 for ; Thu, 27 Jul 2023 17:19:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233003AbjG0RTs (ORCPT ); Thu, 27 Jul 2023 13:19:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51882 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233004AbjG0RTV (ORCPT ); Thu, 27 Jul 2023 13:19:21 -0400 Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D40833C3B for ; Thu, 27 Jul 2023 10:18:54 -0700 (PDT) Received: by mail-pf1-x42d.google.com with SMTP id d2e1a72fcca58-686ba29ccb1so756306b3a.1 for ; Thu, 27 Jul 2023 10:18:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1690478333; x=1691083133; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=oQ1jk/KOTlP01sIFni7CwDm+I437xIRRpg2DbYMJz0w=; b=aAswUTeKFFukfWQJx4v7uNalIbnFpfwWYhVmE+KGA04UV5rjshOUja9o6SMWNzn/WG xt5tAr4aVqPTnzu+ScC6lFGwkbYXq07byVeGdfgjq5whQyVo7TppEx0AIiuMr/LNN/lr ExFuTKo34K51MTFeVz3TwHBDvJJOp/wgjqxho= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690478333; x=1691083133; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=oQ1jk/KOTlP01sIFni7CwDm+I437xIRRpg2DbYMJz0w=; b=lT/xlx2jtDDIf9I8X2/LRQf3itGGRxAW7olO5JFJGw70w9bfPzNC9/ttUKx33gF4EC XmbhR7ih90SyuReu/w4+j0ilZMFO+IK8gLiRSfJBiVpHyjqxTSjpdUOy+xE1pP09IaLB sCVsMTTjJ4PaQ4+sTiHl3XWjBTyy9DCHDCbKQhQTjpXpmfpWsaVcC3MOX1uPxRIb05UF JXPM2GsQPnxc6ynk+pxtYqfN57INS8Y6eZXADUlVtSddSnuGUVKZiZLKwY2R7x8HmF0x o7IgVWaPds7y7W53N/4BRM+vNMv8jDzgwaUj12yGwTCERKBi48bOhixfRPBy1hSvjP3M dI3Q== X-Gm-Message-State: ABy/qLZ+Sdr1FlfFQS8KYQuSdcBi9RGmmBbBnZTlCZFg00Rh/SDnLdGt 2Xp5hZl37xtfeEEWfSf4u32fsg== X-Google-Smtp-Source: APBJJlFoDfQb+6/rj0nXnjfpM4aIPCqGSEFrI23kykKq4DzWXZqpYTvtPVU5dUZSdSKTjXpKGEFQAg== X-Received: by 2002:a05:6a20:3d02:b0:132:c1fd:aaab with SMTP id y2-20020a056a203d0200b00132c1fdaaabmr4137849pzi.30.1690478333644; Thu, 27 Jul 2023 10:18:53 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:2339:954b:b98f:611a]) by smtp.gmail.com with ESMTPSA id 17-20020aa79111000000b0064f76992905sm1702524pfh.202.2023.07.27.10.18.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jul 2023 10:18:53 -0700 (PDT) From: Douglas Anderson To: Jiri Kosina , Benjamin Tissoires , Bjorn Andersson , Konrad Dybcio , Rob Herring , Frank Rowand , Krzysztof Kozlowski , Conor Dooley , Neil Armstrong , Sam Ravnborg , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann Cc: linux-arm-msm@vger.kernel.org, yangcong5@huaqin.corp-partner.google.com, devicetree@vger.kernel.org, Daniel Vetter , hsinyi@google.com, Chris Morgan , linux-input@vger.kernel.org, cros-qcom-dts-watchers@chromium.org, Dmitry Torokhov , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Douglas Anderson Subject: [PATCH v4 10/11] HID: i2c-hid: Do panel follower work on the system_wq Date: Thu, 27 Jul 2023 10:16:37 -0700 Message-ID: <20230727101636.v4.10.I962bb462ede779005341c49320740ed95810021d@changeid> X-Mailer: git-send-email 2.41.0.487.g6d72f3e995-goog In-Reply-To: <20230727171750.633410-1-dianders@chromium.org> References: <20230727171750.633410-1-dianders@chromium.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Turning on an i2c-hid device can be a slow process. This is why i2c-hid devices use PROBE_PREFER_ASYNCHRONOUS. Unfortunately, when we're a panel follower the i2c-hid power up sequence now blocks the power on of the panel. Let's fix that by scheduling the work on the system_wq. Reviewed-by: Maxime Ripard Signed-off-by: Douglas Anderson --- (no changes since v2) Changes in v2: - ihid_core_panel_prepare_work() is now static. - Improve documentation for smp_wmb(). drivers/hid/i2c-hid/i2c-hid-core.c | 50 +++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index fc3087a983f5..9601c0605fd9 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -110,7 +110,9 @@ struct i2c_hid { struct i2chid_ops *ops; struct drm_panel_follower panel_follower; + struct work_struct panel_follower_prepare_work; bool is_panel_follower; + bool prepare_work_finished; }; static const struct i2c_hid_quirks { @@ -1062,10 +1064,12 @@ static int __do_i2c_hid_core_initial_power_up(struct i2c_hid *ihid) return ret; } -static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower) +static void ihid_core_panel_prepare_work(struct work_struct *work) { - struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower); + struct i2c_hid *ihid = container_of(work, struct i2c_hid, + panel_follower_prepare_work); struct hid_device *hid = ihid->hid; + int ret; /* * hid->version is set on the first power up. If it's still zero then @@ -1073,15 +1077,52 @@ static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower) * steps. */ if (!hid->version) - return __do_i2c_hid_core_initial_power_up(ihid); + ret = __do_i2c_hid_core_initial_power_up(ihid); + else + ret = i2c_hid_core_resume(ihid); - return i2c_hid_core_resume(ihid); + if (ret) + dev_warn(&ihid->client->dev, "Power on failed: %d\n", ret); + else + WRITE_ONCE(ihid->prepare_work_finished, true); + + /* + * The work APIs provide a number of memory ordering guarantees + * including one that says that memory writes before schedule_work() + * are always visible to the work function, but they don't appear to + * guarantee that a write that happened in the work is visible after + * cancel_work_sync(). We'll add a write memory barrier here to match + * with i2c_hid_core_panel_unpreparing() to ensure that our write to + * prepare_work_finished is visible there. + */ + smp_wmb(); +} + +static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower) +{ + struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower); + + /* + * Powering on a touchscreen can be a slow process. Queue the work to + * the system workqueue so we don't block the panel's power up. + */ + WRITE_ONCE(ihid->prepare_work_finished, false); + schedule_work(&ihid->panel_follower_prepare_work); + + return 0; } static int i2c_hid_core_panel_unpreparing(struct drm_panel_follower *follower) { struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower); + cancel_work_sync(&ihid->panel_follower_prepare_work); + + /* Match with ihid_core_panel_prepare_work() */ + smp_rmb(); + if (!READ_ONCE(ihid->prepare_work_finished)) + return 0; + return i2c_hid_core_suspend(ihid, true); } @@ -1173,6 +1214,7 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops, init_waitqueue_head(&ihid->wait); mutex_init(&ihid->reset_lock); + INIT_WORK(&ihid->panel_follower_prepare_work, ihid_core_panel_prepare_work); /* we need to allocate the command buffer without knowing the maximum * size of the reports. Let's use HID_MIN_BUFFER_SIZE, then we do the