From patchwork Tue Dec 25 20:49:56 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Christian Lamparter X-Patchwork-Id: 10742623 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3DB3A746 for ; Tue, 25 Dec 2018 20:53:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 225F628824 for ; Tue, 25 Dec 2018 20:53:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 159822889C; Tue, 25 Dec 2018 20:53:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 626AD28824 for ; Tue, 25 Dec 2018 20:53:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725860AbeLYUuB (ORCPT ); Tue, 25 Dec 2018 15:50:01 -0500 Received: from mail-wm1-f68.google.com ([209.85.128.68]:52963 "EHLO mail-wm1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725852AbeLYUuB (ORCPT ); Tue, 25 Dec 2018 15:50:01 -0500 Received: by mail-wm1-f68.google.com with SMTP id m1so13408688wml.2; Tue, 25 Dec 2018 12:49:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=hMdchFEAs+bItmwgfAPgZvWNnj0S3j+7S9GmX6n/7OA=; b=XGbxOclkldrwJO+18wn4jpX7IGjtr14WQhgT3RCYSdFxmShyyQ8D6+kVV86txlFgT3 1AIh4osOlbSLvDMz74cAf6udgbYT4jYsb9ca7o2wCF5RLE6T/7Z7+rTZbTnagS5Dd+Ua X5PGajMsZCoicnkZH0zjlTLE8HnAiE+pKNTo9PiD+3uR+pbOazRVKFfP7Emq5vLRFLcK YodGXAhrZwobY+TaiaVITWBpRHnFd/Fcon1vdjjvpG6KUaylr/VGjZ7ulymX/zb7PHbC IiMpCbio1j0KqFFgu2yMUoU6OONNytleEvTNIJ/yYzIVyhfIW2e7LZ3wXUU2HM1/AXAL 8pHw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=hMdchFEAs+bItmwgfAPgZvWNnj0S3j+7S9GmX6n/7OA=; b=VXB60ga5O7q5vSorEO5z2PqlAEiwx5+URYpqI7dJroG+5oq0V52SIaoo5gOuNLtF8Q Uwwqp03JBQuTtM00kYxLCoDF0s6uLPXhXYtZt4MVqReikd2BGCoj9cumQjSkYdpkm5Ab VuYqT+gPcgpys5hCZLCPoDGlQExmSg1iuuo0+exA7kIYI/1NGgnNNnvIDCau371/ZPOU a5/g5HAykfFGRNCUDdY2IWGo4igBDWNa9TIjyqldatrMWDRKQpaCZ9OhKEIXj7qsgySq WSA04y2ffnSM4BdwGZAEPhNcLXdKltEUhavbhia/dPEeqjPp8z6qox9jh6LAVxJropgr K/kg== X-Gm-Message-State: AA+aEWa3T2iA9Qv96WvxyouEI2dI8pS3rP/XQ8o5PMMthCrLErW/eLWZ rSabh4IZwlj59NMs5Btz9sRbguTv X-Google-Smtp-Source: AFSGD/UUxStukNK84UY9ahy0uGvKbu8jcLGQL36jKRjplkP0Nu9YGqN+WYLxKcMm0MxKdHQRxb5O8g== X-Received: by 2002:a1c:c10f:: with SMTP id r15mr16263289wmf.27.1545770998394; Tue, 25 Dec 2018 12:49:58 -0800 (PST) Received: from debian64.daheim (p5B0D7432.dip0.t-ipconnect.de. [91.13.116.50]) by smtp.gmail.com with ESMTPSA id o8sm20846419wrx.15.2018.12.25.12.49.57 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 25 Dec 2018 12:49:57 -0800 (PST) Received: from chuck by debian64.daheim with local (Exim 4.91) (envelope-from ) id 1gbteG-0004fP-MZ; Tue, 25 Dec 2018 21:49:56 +0100 From: Christian Lamparter To: linux-usb@vger.kernel.org, linux-leds@vger.kernel.org Cc: =?utf-8?b?UmFmYcWCIE1pxYJlY2tp?= , =?utf-8?q?Uwe_Klein?= =?utf-8?q?e-K=C3=B6nig?= , Jacek Anaszewski , Greg Kroah-Hartman Subject: [RFC PATCH v1] leds: fix regression in usbport led trigger Date: Tue, 25 Dec 2018 21:49:56 +0100 Message-Id: <20181225204956.17897-1-chunkeey@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The patch "usb: simplify usbport trigger" together with "leds: triggers: add device attribute support" caused an regression for the usbport trigger. it will no longer enumerate any active usb hub ports under the "ports" directory in the sysfs class directory, if the usb host drivers are fully initialized before the usbport trigger was loaded. The reason is that the usbport driver tries to register the sysfs entries during the activate() callback. And this will fail with -2 / ENOENT because the patch "leds: triggers: add device attribute support" made it so that the sysfs "ports" group was only being added after the activate() callback succeeded. This version of the patch moves the device_add_groups() in front of the call to the trigger's activate() function in order to solve the problem. Cc: Greg Kroah-Hartman Cc: Uwe Kleine-König Cc: Rafał Miłecki Fixes: 6f7b0bad8839 ("usb: simplify usbport trigger") Signed-off-by: Christian Lamparter Acked-by: Jacek Anaszewski --- This version of the patch ... of which will be many more?! yeah, device_add_groups() in front of activate() works for the usbport driver since it dynamically registers the entries in "ports". However, if a trigger has a static list of entities this can get more complicated, since I think the sysfs entries will now be available before the activate() call even started and this can end up badly. So, is there any better approach? Introduce a "post_activate()" callback? Or use the event system and make usbport trigger on the KOBJ_CHANGE? use a (delayed) work_struct in usbport to register the ports at a slightly later time? etc... (Note: I'm hitting this on 4.19 too, so whatever the real fix will look like it should be backported) --- drivers/leds/led-triggers.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c index 17d73db1456e..08e7c724a9dc 100644 --- a/drivers/leds/led-triggers.c +++ b/drivers/leds/led-triggers.c @@ -134,6 +134,12 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig) led_set_brightness(led_cdev, LED_OFF); } if (trig) { + ret = device_add_groups(led_cdev->dev, trig->groups); + if (ret) { + dev_err(led_cdev->dev, "Failed to add trigger attributes\n"); + goto err_add_groups; + } + write_lock_irqsave(&trig->leddev_list_lock, flags); list_add_tail(&led_cdev->trig_list, &trig->led_cdevs); write_unlock_irqrestore(&trig->leddev_list_lock, flags); @@ -146,12 +152,6 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig) if (ret) goto err_activate; - - ret = device_add_groups(led_cdev->dev, trig->groups); - if (ret) { - dev_err(led_cdev->dev, "Failed to add trigger attributes\n"); - goto err_add_groups; - } } if (event) { @@ -165,17 +165,18 @@ int led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig) return 0; -err_add_groups: - +err_activate: + device_remove_groups(led_cdev->dev, trig->groups); if (trig->deactivate) trig->deactivate(led_cdev); -err_activate: led_cdev->trigger = NULL; led_cdev->trigger_data = NULL; write_lock_irqsave(&led_cdev->trigger->leddev_list_lock, flags); list_del(&led_cdev->trig_list); write_unlock_irqrestore(&led_cdev->trigger->leddev_list_lock, flags); + +err_add_groups: led_set_brightness(led_cdev, LED_OFF); return ret;