From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5AADCC433FF for ; Wed, 31 Jul 2019 23:45:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 25F59208E4 for ; Wed, 31 Jul 2019 23:45:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="ftYDBFAu" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730952AbfGaXpe (ORCPT ); Wed, 31 Jul 2019 19:45:34 -0400 Received: from mail-pg1-f194.google.com ([209.85.215.194]:32861 "EHLO mail-pg1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728083AbfGaXpd (ORCPT ); Wed, 31 Jul 2019 19:45:33 -0400 Received: by mail-pg1-f194.google.com with SMTP id n190so1417075pgn.0 for ; Wed, 31 Jul 2019 16:45:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=message-id:mime-version:content-transfer-encoding:in-reply-to :references:cc:from:to:subject:user-agent:date; bh=2aTqXCxfS0wES19jPnEhvSwuE9w59s+Ba6S/ZSuGNNY=; b=ftYDBFAuvCukNdwLQxFOFw0D47LFOVpE9titYeQErJZGve6bds0/csNvRpFmz4WSSf VU9D98vtfozObHI2w10LxC1cOKtSNqfUMerIIaIiDDsBuKyzZOzf5VQUJbMjIXc6n43W aLdxS5baeyz+kNSrC5bOO13Fz8t9Fk9NROzjM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:mime-version :content-transfer-encoding:in-reply-to:references:cc:from:to:subject :user-agent:date; bh=2aTqXCxfS0wES19jPnEhvSwuE9w59s+Ba6S/ZSuGNNY=; b=brVbwIrZHJkl10WNAuiEEc+Nh99bulkjAkmYNivwcL/8V+inWwjQPk/el5PyS0D83n m+qTHsSR/1bsCG6ki+5sPEfBEscTijFfYJBk6PZQRT/hTrkd9XCE8GcEbBdmC7SRCuEy pNBLJuAPG/B8E5B5L4fnAjD7MSBwVNbd02gts9ar3a/qdyqotkt5O/POnuQkgnuxj+FS qeSkjc3IdfEY1iZMAZFJpKw1aX9zYQ2+tU0NwoOGzp3AXSC2Q+j+P3WvLyZlGL6Fi17N xtRWBd0F4Z4rXUEGOVrUDVY43geHQxS7gnZPQ8+zieDoY4XOwagS+txqVdkAHEuHIUXe h1mA== X-Gm-Message-State: APjAAAVVq8HsgWN5P1sCdqjoWhO/tvKlV7PkKeAXnSrNBNGRQRQCLm5k HBlUqWUyEjnhTrKDQXMEUMwt8g== X-Google-Smtp-Source: APXvYqzpR+yF2tlL0G8/rKxh8Cfu9rEUYcaYuyAEnU2+suNL/2Oh/9t4urfyQDt9WpQjQE8iR5ILsg== X-Received: by 2002:a65:4507:: with SMTP id n7mr8434326pgq.86.1564616732856; Wed, 31 Jul 2019 16:45:32 -0700 (PDT) Received: from chromium.org ([2620:15c:202:1:fa53:7765:582b:82b9]) by smtp.gmail.com with ESMTPSA id i7sm2433751pjk.24.2019.07.31.16.45.32 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Wed, 31 Jul 2019 16:45:32 -0700 (PDT) Message-ID: <5d42281c.1c69fb81.bcda1.71f5@mx.google.com> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: References: <20190731215514.212215-1-trong@android.com> <32598586.Mjd66ZhNnG@kreacher> <6987393.M0uybTKmdI@kreacher> Cc: "Rafael J. Wysocki" , Greg Kroah-Hartman , Viresh Kumar , "Rafael J. Wysocki" , Hridya Valsaraju , Sandeep Patil , Kalesh Singh , Ravi Chandra Sadineni , LKML , Linux PM , "Cc: Android Kernel" From: Stephen Boyd To: "Rafael J. Wysocki" , Tri Vo Subject: Re: [PATCH v6] PM / wakeup: show wakeup sources stats in sysfs User-Agent: alot/0.8.1 Date: Wed, 31 Jul 2019 16:45:31 -0700 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Quoting Rafael J. Wysocki (2019-07-31 16:10:38) > On Thu, Aug 1, 2019 at 12:59 AM Tri Vo wrote: > > > > On Wed, Jul 31, 2019 at 3:42 PM Rafael J. Wysocki w= rote: > > > > > > That's not my point (see below). > > > > > > > > > > + if (id < 0) > > > > > > > + return id; > > > > > > > + ws->id =3D id; > > > > > > > + > > > > > > > + dev =3D device_create_with_groups(wakeup_class, paren= t, MKDEV(0, 0), ws, > > > > > > > + wakeup_source_groups,= "ws%d", > > > > > > > > > > > > I thought the name was going to still be 'wakeupN'? > > > > > > > > > > So can't we prefix the wakeup source name with something like "wa= keup:" or similar here? > > > > > > > > "ws%d" here is the name in the sysfs path rather than the name of t= he > > > > wakeup source. Wakeup source name is not altered in this patch. > > > > > > > > > > So why wouldn't something like this suffice: > > > > > > dev =3D device_create_with_groups(wakeup_class, parent, MKDEV(0, 0), = ws, > > > wakeup_source_groups, "wakeup:%s", ws= ->name); > > > > > > ? > > > > ws->name is inherited from the device name. IIUC device names are not > > guaranteed to be unique. So if different devices with the same name > > register wakeup sources, there is an error. >=20 > OK >=20 > So I guess the names are retained for backwards compatibility with > existing user space that may be using them? >=20 > That's kind of fair enough, but having two different identification > schemes for wakeup sources will end up confusing. I understand your concern about the IDA now. Thanks for clarifying. How about we name the devices 'wakeupN' with the IDA when they're registered with a non-NULL device pointer and then name them whatever the name argument is when the device pointer is NULL. If we have this, we should be able to drop the name attribute in sysfs and figure out the name either by looking at the device name in /sys/class/wakeup/ if it isn't 'wakeupN', or follow the symlink to the device in /sys/devices/ and look at the parent device name there. The only problem I see is the alarmtimer code where it might register a second wakeup source for the same rtc device. In this case, we probably want to use whatever name is passed in ("alarmtimer") instead of the IDA. This approach also nicely detects duplicate wakeup source names in the case that the string passed in to wakeup_source_register() is already used on the virtual bus. ---8<---- diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index 79668b45eae6..1c98f83c576e 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -201,7 +201,7 @@ EXPORT_SYMBOL_GPL(wakeup_source_remove); /** * wakeup_source_register - Create wakeup source and add it to the list. * @dev: Device this wakeup source is associated with (or NULL if virtual). - * @name: Name of the wakeup source to register. + * @name: Name of the wakeup source to register (or NULL if device wakeup). */ struct wakeup_source *wakeup_source_register(struct device *dev, const char *name) @@ -209,6 +209,9 @@ struct wakeup_source *wakeup_source_register(struct dev= ice *dev, struct wakeup_source *ws; int ret; =20 + if (!name) + name =3D dev_name(dev); + ws =3D wakeup_source_create(name); if (ws) { ret =3D wakeup_source_sysfs_add(dev, ws); @@ -275,7 +278,7 @@ int device_wakeup_enable(struct device *dev) if (pm_suspend_target_state !=3D PM_SUSPEND_ON) dev_dbg(dev, "Suspicious %s() during system transition!\n", __func__); =20 - ws =3D wakeup_source_register(dev, dev_name(dev)); + ws =3D wakeup_source_register(dev, NULL); if (!ws) return -ENOMEM; =20 diff --git a/drivers/base/power/wakeup_stats.c b/drivers/base/power/wakeup_= stats.c index a26f019faca9..11e2906dca4c 100644 --- a/drivers/base/power/wakeup_stats.c +++ b/drivers/base/power/wakeup_stats.c @@ -132,16 +132,22 @@ int wakeup_source_sysfs_add(struct device *parent, st= ruct wakeup_source *ws) struct device *dev; int id; =20 - id =3D ida_alloc(&wakeup_ida, GFP_KERNEL); - if (id < 0) - return id; - ws->id =3D id; + if (parent) { + id =3D ida_alloc(&wakeup_ida, GFP_KERNEL); + if (id < 0) + return id; + ws->id =3D id; + } else { + ws->id =3D -1; + } =20 dev =3D device_create_with_groups(wakeup_class, parent, MKDEV(0, 0), ws, - wakeup_source_groups, "ws%d", - ws->id); + wakeup_source_groups, + ws->id >=3D 0 ? "wakeup%d" : "%s", + ws->id >=3D 0 ? ws->id : ws->name); if (IS_ERR(dev)) { - ida_free(&wakeup_ida, ws->id); + if (ws->id >=3D 0) + ida_free(&wakeup_ida, ws->id); return PTR_ERR(dev); } =20