Message ID | 20231005133059.917577-1-naresh.solanki@9elements.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2016:b0:403:3b70:6f57 with SMTP id fe22csp335973vqb; Thu, 5 Oct 2023 07:27:03 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFOvVWkAZHQGcpRVC0PIWkRML60BMgtFZygY1UQwJgdpG+yn7WINjeGOw9HO268ZQEUjWS9 X-Received: by 2002:a05:6a20:dda6:b0:135:1af6:9a01 with SMTP id kw38-20020a056a20dda600b001351af69a01mr4339228pzb.8.1696516023510; Thu, 05 Oct 2023 07:27:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696516023; cv=none; d=google.com; s=arc-20160816; b=rrpkgEnXRCuOHsbOV2M2ot5X1O/6b6WXQnbkpsK4w8kWq0VepJXkBgMf0zYMMd3cv9 H5L/v4xVxO6QtqOkqx12cL/rhNaC2MH1blU3SqwHc1pCPvk06uVszCl0l4HLji9Diqzy EX16vmJZ2fas96RWoxEjNv8UULLwajHFsye7rqU3FIBdCxoj/mBcd9G/gI0ednf1E9pg E/Sjc5mV4BkswmFtXgg0U/CAEBza6N+jBqh/DnCjaBRm0UEBM7CLQoMWTlIfz145Qrgh j6j5CU2TnILdxsE3zBxoTRpPZmN+7JJ1FhC4qQneNQ/hzDkSaVFn8C/60HHOqZVVTvDF DVGg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=IiSMw0DH0YnEshhxsB4lPXfkEW4kd1IPdZ6vz5cekDQ=; fh=kwXqe5GQ/gU+HO0MA3lrzE4RDizuitADRzTm3ignGQE=; b=v3Kv/1uzvkh7RGzj937mdwRwqgFgsD0UyESY9y1NoZSc6m6fzQO7lHW+7D6rduL4WQ 0OfGoNUPrI79O0HbNb3r3O7fdn21AyJYo5hy0DpX209kCHbtHofrXp63LX2K7jU+PUFn uInoQu2Lix7hSYqL1pDaNYrs6oNtGvhwdqN6ueyqEjSBTk5l2fSveQ/Ox3DLE0dD/W5K fxkA0ffoU/dwHPxk4VQ1mY6RYRNM/UuLICbpBOgrOJj8bJvOF+bbY5iiLptEltvvyJA5 v1cuKvYBA8pFiXc/CdA1cA14dHZTQjrM2lWhuBA99ENT15ihLFNyZP9T/6Eu0cN8/ym7 wdAw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@9elements.com header.s=google header.b=NYiTR2Xl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=9elements.com Received: from howler.vger.email (howler.vger.email. [23.128.96.34]) by mx.google.com with ESMTPS id q15-20020a056a00088f00b0068fac3509a9si1490501pfj.350.2023.10.05.07.27.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Oct 2023 07:27:03 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34; Authentication-Results: mx.google.com; dkim=pass header.i=@9elements.com header.s=google header.b=NYiTR2Xl; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=9elements.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by howler.vger.email (Postfix) with ESMTP id BB31F80A857B; Thu, 5 Oct 2023 07:25:43 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233665AbjJEOZB (ORCPT <rfc822;ezelljr.billy@gmail.com> + 19 others); Thu, 5 Oct 2023 10:25:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36308 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233792AbjJEOWk (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 5 Oct 2023 10:22:40 -0400 Received: from mail-ej1-x629.google.com (mail-ej1-x629.google.com [IPv6:2a00:1450:4864:20::629]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6289327B01 for <linux-kernel@vger.kernel.org>; Thu, 5 Oct 2023 06:31:07 -0700 (PDT) Received: by mail-ej1-x629.google.com with SMTP id a640c23a62f3a-9b9ad5760b9so172374766b.3 for <linux-kernel@vger.kernel.org>; Thu, 05 Oct 2023 06:31:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=9elements.com; s=google; t=1696512666; x=1697117466; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=IiSMw0DH0YnEshhxsB4lPXfkEW4kd1IPdZ6vz5cekDQ=; b=NYiTR2Xlv2MbjTyZuRmw3riOMm8mWQg96PO9TFNpseIVFUW6eTnlOof/EXtXARzEIz B1cy7EM7M1gLX56GvnEVfGHLLgxV6kcJnaWjuDnwqBpiB/F/E0uCeq9XUESNl/dpQ1rV LB+02q5tG2AD3BsnbhbILTokeIJtjZpSraQ4C/IcchSoyNHt4KRUJx/aXEG0CIm7mEya kyCDcV/cYiFANpIlwhafAFRKGNoqvNU1BElVoc5ThcaVUXRh4tmahARJrKlo/c1ksZeR ohnPmoo/OEIZR0bVPR/ViZWy5EMSS2oll1G0AaHczULSGYaIKHwHV6mImFg0got9mUZF pjGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696512666; x=1697117466; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=IiSMw0DH0YnEshhxsB4lPXfkEW4kd1IPdZ6vz5cekDQ=; b=ayYMdAUZ2SQ0WTZ360K+Uv6SxIooAKlBWGPbr04QUqb/XLB3ohDQHqyDE3egXE5RwQ rP9koSA+xB8ecaXAnznZUi1k3ObZQRoEVS8s6D4vXdzWPwa+BUHggcO4ToypGQ2PRkl8 zM0O4a/pL4WQ67LJi1ZapYisDaRnpdnSGiuEU7zyzFo15Mt+hXFpDJ0m52g1NHU4ZoGF aYPOlC65A99ySlEfOjEMd4uMy/Du7AJnSMmGVY+9nu/HcJLPItERJ6lRq6oBWVksXPZ4 fyWjr9Exdu3foWLsLdw5Y7XX2DGVAxUI7ZH7+SES2ZiGxIvaF5r1/L1EL4I/S1b2v1pE Zxrw== X-Gm-Message-State: AOJu0YxsWeFfljo3ixoRNESMrdFlEjpmCHdBZPieErXvNTR9059CB72A IadN+Ky2lalkx+DFcc+r3aumuA== X-Received: by 2002:a17:906:18a1:b0:9b6:aac1:6fa5 with SMTP id c1-20020a17090618a100b009b6aac16fa5mr4541714ejf.55.1696512665832; Thu, 05 Oct 2023 06:31:05 -0700 (PDT) Received: from stroh80.sec.9e.network (ip-078-094-000-051.um19.pools.vodafone-ip.de. [78.94.0.51]) by smtp.gmail.com with ESMTPSA id b13-20020a170906d10d00b009930c80b87csm1229561ejz.142.2023.10.05.06.31.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Oct 2023 06:31:05 -0700 (PDT) From: Naresh Solanki <naresh.solanki@9elements.com> To: broonie@kernel.org, zev@bewilderbeest.net, Liam Girdwood <lgirdwood@gmail.com> Cc: Patrick Rudolph <patrick.rudolph@9elements.com>, Naresh Solanki <naresh.solanki@9elements.com>, linux-kernel@vger.kernel.org Subject: [PATCH] drivers/regulator: Notify sysfs about status changes Date: Thu, 5 Oct 2023 15:30:58 +0200 Message-ID: <20231005133059.917577-1-naresh.solanki@9elements.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Thu, 05 Oct 2023 07:25:43 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778925986102258688 X-GMAIL-MSGID: 1778925986102258688 |
Series |
drivers/regulator: Notify sysfs about status changes
|
|
Commit Message
Naresh Solanki
Oct. 5, 2023, 1:30 p.m. UTC
From: Patrick Rudolph <patrick.rudolph@9elements.com> Notify sysfs for state, status & microvolts. Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> --- drivers/regulator/core.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) base-commit: f9a1d31874c383f58bb4f89bfe79b764682cd026
Comments
On Thu, Oct 05, 2023 at 03:30:58PM +0200, Naresh Solanki wrote: > static int _notifier_call_chain(struct regulator_dev *rdev, > unsigned long event, void *data) > { > + const char *name; > + int ret; > + > /* call rdev chain first */ > - return blocking_notifier_call_chain(&rdev->notifier, event, data); > + ret = blocking_notifier_call_chain(&rdev->notifier, event, data); > + > + if (event & REGULATOR_EVENT_VOLTAGE_CHANGE) { > + name = dev_attr_microvolts.attr.name; > + sysfs_notify(&rdev->dev.kobj, NULL, name); > + } else { > + name = dev_attr_status.attr.name; > + sysfs_notify(&rdev->dev.kobj, NULL, name); > + } We probably should filter the events more, there's events for pre and post voltage change for example which aren't status changes so would be spurious. It ought not to break anything but we should still avoid unneeded work.
Hi Mark, On Thu, 5 Oct 2023 at 22:30, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Oct 05, 2023 at 03:30:58PM +0200, Naresh Solanki wrote: > > > static int _notifier_call_chain(struct regulator_dev *rdev, > > unsigned long event, void *data) > > { > > + const char *name; > > + int ret; > > + > > /* call rdev chain first */ > > - return blocking_notifier_call_chain(&rdev->notifier, event, data); > > + ret = blocking_notifier_call_chain(&rdev->notifier, event, data); > > + > > + if (event & REGULATOR_EVENT_VOLTAGE_CHANGE) { > > + name = dev_attr_microvolts.attr.name; > > + sysfs_notify(&rdev->dev.kobj, NULL, name); > > + } else { > > + name = dev_attr_status.attr.name; > > + sysfs_notify(&rdev->dev.kobj, NULL, name); > > + } > > We probably should filter the events more, there's events for pre and > post voltage change for example which aren't status changes so would be > spurious. It ought not to break anything but we should still avoid > unneeded work. Can you please provide me inputs on the additional filtering needed for this. Like some list of events for notify on status?
On Thu, Nov 02, 2023 at 05:35:42PM +0530, Naresh Solanki wrote: > On Thu, 5 Oct 2023 at 22:30, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Oct 05, 2023 at 03:30:58PM +0200, Naresh Solanki wrote: > > We probably should filter the events more, there's events for pre and > > post voltage change for example which aren't status changes so would be > > spurious. It ought not to break anything but we should still avoid > > unneeded work. > Can you please provide me inputs on the additional filtering needed for this. > Like some list of events for notify on status? I think I'd start off with just reporting things that are obviously errors and not things that should ever go off during normal operation.
Hi Mark, On Thu, 2 Nov 2023 at 18:36, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Nov 02, 2023 at 05:35:42PM +0530, Naresh Solanki wrote: > > On Thu, 5 Oct 2023 at 22:30, Mark Brown <broonie@kernel.org> wrote: > > > On Thu, Oct 05, 2023 at 03:30:58PM +0200, Naresh Solanki wrote: > > > > We probably should filter the events more, there's events for pre and > > > post voltage change for example which aren't status changes so would be > > > spurious. It ought not to break anything but we should still avoid > > > unneeded work. > > > Can you please provide me inputs on the additional filtering needed for this. > > Like some list of events for notify on status? > > I think I'd start off with just reporting things that are obviously > errors and not things that should ever go off during normal operation. This is what I could come up with: if (event & REGULATOR_EVENT_VOLTAGE_CHANGE) { name = dev_attr_microvolts.attr.name; sysfs_notify(&rdev->dev.kobj, NULL, name); } else if (event & (REGULATOR_EVENT_DISABLE | REGULATOR_EVENT_ENABLE)){ name = dev_attr_status.attr.name; sysfs_notify(&rdev->dev.kobj, NULL, name); } Regards, Naresh
On Thu, Nov 02, 2023 at 08:17:35PM +0530, Naresh Solanki wrote: > On Thu, 2 Nov 2023 at 18:36, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Nov 02, 2023 at 05:35:42PM +0530, Naresh Solanki wrote: > > > > We probably should filter the events more, there's events for pre and > > > > post voltage change for example which aren't status changes so would be > > > > spurious. It ought not to break anything but we should still avoid > > > > unneeded work. > > > Can you please provide me inputs on the additional filtering needed for this. > > > Like some list of events for notify on status? > > I think I'd start off with just reporting things that are obviously > > errors and not things that should ever go off during normal operation. > This is what I could come up with: > if (event & REGULATOR_EVENT_VOLTAGE_CHANGE) { > name = dev_attr_microvolts.attr.name; > sysfs_notify(&rdev->dev.kobj, NULL, name); > } else if (event & (REGULATOR_EVENT_DISABLE | REGULATOR_EVENT_ENABLE)){ > name = dev_attr_status.attr.name; > sysfs_notify(&rdev->dev.kobj, NULL, name); > } That's the opposite sense to what I was thinking of - we're reporting voltage changes and enables to userspace rather than just errors. My concern here is that this could generate an awful lot of notificaitons for normal operation on systems that don't use the uevents, I was expecting this to be used for errors. Could you remind me what the use case is here, I think I might've got myself confused sorry?
Hi Mark, On Thu, 2 Nov 2023 at 20:31, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Nov 02, 2023 at 08:17:35PM +0530, Naresh Solanki wrote: > > On Thu, 2 Nov 2023 at 18:36, Mark Brown <broonie@kernel.org> wrote: > > > On Thu, Nov 02, 2023 at 05:35:42PM +0530, Naresh Solanki wrote: > > > > > > We probably should filter the events more, there's events for pre and > > > > > post voltage change for example which aren't status changes so would be > > > > > spurious. It ought not to break anything but we should still avoid > > > > > unneeded work. > > > > > Can you please provide me inputs on the additional filtering needed for this. > > > > Like some list of events for notify on status? > > > > I think I'd start off with just reporting things that are obviously > > > errors and not things that should ever go off during normal operation. > > > This is what I could come up with: > > if (event & REGULATOR_EVENT_VOLTAGE_CHANGE) { > > name = dev_attr_microvolts.attr.name; > > sysfs_notify(&rdev->dev.kobj, NULL, name); > > } else if (event & (REGULATOR_EVENT_DISABLE | REGULATOR_EVENT_ENABLE)){ > > name = dev_attr_status.attr.name; > > sysfs_notify(&rdev->dev.kobj, NULL, name); > > } > > That's the opposite sense to what I was thinking of - we're reporting > voltage changes and enables to userspace rather than just errors. My > concern here is that this could generate an awful lot of notificaitons > for normal operation on systems that don't use the uevents, I was > expecting this to be used for errors. Could you remind me what the use > case is here, I think I might've got myself confused sorry? Sorry for confusion caused because I should first described my application requirements. Currently my application is interested in know regulator status i.e., ENABLE, DISABLE or ERROR. Also events are needed specifically to get them logged like UNDER_VOLTAGE, OVER_CURRENT, REGULATION_OUT, OVER_TEMP. Regards, Naresh
On Thu, Nov 02, 2023 at 09:03:40PM +0530, Naresh Solanki wrote: > On Thu, 2 Nov 2023 at 20:31, Mark Brown <broonie@kernel.org> wrote: > > That's the opposite sense to what I was thinking of - we're reporting > > voltage changes and enables to userspace rather than just errors. My > > concern here is that this could generate an awful lot of notificaitons > > for normal operation on systems that don't use the uevents, I was > > expecting this to be used for errors. Could you remind me what the use > > case is here, I think I might've got myself confused sorry? > Sorry for confusion caused because I should first described my application > requirements. > Currently my application is interested in know regulator status i.e., > ENABLE, DISABLE or ERROR. > Also events are needed specifically to get them logged like > UNDER_VOLTAGE, OVER_CURRENT, REGULATION_OUT, > OVER_TEMP. Ah, right. Everything except for the enable and disable there looks like it should be OK since they should normally just not happen but the enables and disables might get a bit frequent with runtime PM - not *super* frequent like voltage scaling but enough that people could have an issue with it. Netlink feels like it might be a better fit? Not really looked at the kernel side of implementing that and how sensible that ends up looking.
Hi Mark, On Thu, 2 Nov 2023 at 22:20, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Nov 02, 2023 at 09:03:40PM +0530, Naresh Solanki wrote: > > On Thu, 2 Nov 2023 at 20:31, Mark Brown <broonie@kernel.org> wrote: > > > > That's the opposite sense to what I was thinking of - we're reporting > > > voltage changes and enables to userspace rather than just errors. My > > > concern here is that this could generate an awful lot of notificaitons > > > for normal operation on systems that don't use the uevents, I was > > > expecting this to be used for errors. Could you remind me what the use > > > case is here, I think I might've got myself confused sorry? > > > Sorry for confusion caused because I should first described my application > > requirements. > > Currently my application is interested in know regulator status i.e., > > ENABLE, DISABLE or ERROR. > > Also events are needed specifically to get them logged like > > UNDER_VOLTAGE, OVER_CURRENT, REGULATION_OUT, > > OVER_TEMP. > > Ah, right. Everything except for the enable and disable there looks > like it should be OK since they should normally just not happen but the > enables and disables might get a bit frequent with runtime PM - not > *super* frequent like voltage scaling but enough that people could have > an issue with it. I aim for a straightforward implementation. Using attributes such as status or state seems ideal for receiving notifications. In my case, the application focuses on status changes—whether it's on, off, or encountering an error. These changes are driven by events originating from the regulator. So below change is what I see fit well. status_events = REGULATOR_EVENT_DISABLE; status_events |= REGULATOR_EVENT_ENABLE; status_events |= REGULATOR_EVENT_FAIL; status_events |= REGULATOR_EVENT_FORCE_DISABLE; status_events |= REGULATOR_EVENT_ABORT_DISABLE; if (event & status_events) { name = dev_attr_status.attr.name; sysfs_notify(&rdev->dev.kobj, NULL, name); } Let me know if this can be further tuned to be better.. Regards, Naresh > > Netlink feels like it might be a better fit? Not really looked at the > kernel side of implementing that and how sensible that ends up looking.
On Thu, Nov 09, 2023 at 04:08:06PM +0530, Naresh Solanki wrote: > On Thu, 2 Nov 2023 at 22:20, Mark Brown <broonie@kernel.org> wrote: > > Ah, right. Everything except for the enable and disable there looks > > like it should be OK since they should normally just not happen but the > > enables and disables might get a bit frequent with runtime PM - not > > *super* frequent like voltage scaling but enough that people could have > > an issue with it. > I aim for a straightforward implementation. > Using attributes such as status or state seems ideal for receiving > notifications. > In my case, the application focuses on status changes—whether it's on, off, > or encountering an error. > These changes are driven by events originating from the regulator. > So below change is what I see fit well. > > status_events = REGULATOR_EVENT_DISABLE; > status_events |= REGULATOR_EVENT_ENABLE; > status_events |= REGULATOR_EVENT_FAIL; > status_events |= REGULATOR_EVENT_FORCE_DISABLE; > status_events |= REGULATOR_EVENT_ABORT_DISABLE; In terms of the implementation for delivering uevents this looks fine, my concern here is that for some systems the enable and disable events might happen more often than is really a good fir for delivering via uevents, if a device like say a SD card is getting powered up and down via runtime PM that might happen relatively often and then the system would get a lot of uevents which it would most likely end up ignoring. That could have a noticable impact on power or performance.
Hi Mark, On Thu, 9 Nov 2023 at 17:01, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Nov 09, 2023 at 04:08:06PM +0530, Naresh Solanki wrote: > > On Thu, 2 Nov 2023 at 22:20, Mark Brown <broonie@kernel.org> wrote: > > > > Ah, right. Everything except for the enable and disable there looks > > > like it should be OK since they should normally just not happen but the > > > enables and disables might get a bit frequent with runtime PM - not > > > *super* frequent like voltage scaling but enough that people could have > > > an issue with it. > > > I aim for a straightforward implementation. > > Using attributes such as status or state seems ideal for receiving > > notifications. > > In my case, the application focuses on status changes—whether it's on, off, > > or encountering an error. > > These changes are driven by events originating from the regulator. > > So below change is what I see fit well. > > > > status_events = REGULATOR_EVENT_DISABLE; > > status_events |= REGULATOR_EVENT_ENABLE; > > status_events |= REGULATOR_EVENT_FAIL; > > status_events |= REGULATOR_EVENT_FORCE_DISABLE; > > status_events |= REGULATOR_EVENT_ABORT_DISABLE; > > In terms of the implementation for delivering uevents this looks fine, > my concern here is that for some systems the enable and disable events > might happen more often than is really a good fir for delivering via > uevents, if a device like say a SD card is getting powered up and down > via runtime PM that might happen relatively often and then the system > would get a lot of uevents which it would most likely end up ignoring. > That could have a noticable impact on power or performance. May be in that case should we consider adding a kernel parameter or some property in sysfs attribute to allow getting events ? Regards, Naresh
Hi Mark, On Thu, 9 Nov 2023 at 20:41, Naresh Solanki <naresh.solanki@9elements.com> wrote: > > Hi Mark, > > > On Thu, 9 Nov 2023 at 17:01, Mark Brown <broonie@kernel.org> wrote: > > > > On Thu, Nov 09, 2023 at 04:08:06PM +0530, Naresh Solanki wrote: > > > On Thu, 2 Nov 2023 at 22:20, Mark Brown <broonie@kernel.org> wrote: > > > > > > Ah, right. Everything except for the enable and disable there looks > > > > like it should be OK since they should normally just not happen but the > > > > enables and disables might get a bit frequent with runtime PM - not > > > > *super* frequent like voltage scaling but enough that people could have > > > > an issue with it. > > > > > I aim for a straightforward implementation. > > > Using attributes such as status or state seems ideal for receiving > > > notifications. > > > In my case, the application focuses on status changes—whether it's on, off, > > > or encountering an error. > > > These changes are driven by events originating from the regulator. > > > So below change is what I see fit well. > > > > > > status_events = REGULATOR_EVENT_DISABLE; > > > status_events |= REGULATOR_EVENT_ENABLE; > > > status_events |= REGULATOR_EVENT_FAIL; > > > status_events |= REGULATOR_EVENT_FORCE_DISABLE; > > > status_events |= REGULATOR_EVENT_ABORT_DISABLE; > > > > In terms of the implementation for delivering uevents this looks fine, > > my concern here is that for some systems the enable and disable events > > might happen more often than is really a good fir for delivering via > > uevents, if a device like say a SD card is getting powered up and down > > via runtime PM that might happen relatively often and then the system > > would get a lot of uevents which it would most likely end up ignoring. > > That could have a noticable impact on power or performance. > > May be in that case should we consider adding a kernel parameter or > some property in sysfs attribute to allow getting events ? Or may be even Kconfig option ? > > Regards, > Naresh
On Thu, Nov 09, 2023 at 08:51:40PM +0530, Naresh Solanki wrote: > On Thu, 9 Nov 2023 at 20:41, Naresh Solanki > > May be in that case should we consider adding a kernel parameter or > > some property in sysfs attribute to allow getting events ? > Or may be even Kconfig option ? It's not great but a sysfs attribute *might* work, possibly sysfs files per event like for tracing. However it really does feel like a fit for netlink - is there an issue you see with using that?
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 3137e40fcd3e..ef5fa70ae2f1 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -98,6 +98,10 @@ static struct regulator *create_regulator(struct regulator_dev *rdev, static void destroy_regulator(struct regulator *regulator); static void _regulator_put(struct regulator *regulator); +static struct device_attribute dev_attr_status; +static struct device_attribute dev_attr_microvolts; +static struct device_attribute dev_attr_state; + const char *rdev_get_name(struct regulator_dev *rdev) { if (rdev->constraints && rdev->constraints->name) @@ -2798,6 +2802,8 @@ static int _regulator_do_enable(struct regulator_dev *rdev) _regulator_delay_helper(delay); } + sysfs_notify(&rdev->dev.kobj, NULL, dev_attr_state.attr.name); + trace_regulator_enable_complete(rdev_get_name(rdev)); return 0; @@ -2980,6 +2986,8 @@ static int _regulator_do_disable(struct regulator_dev *rdev) if (rdev->desc->off_on_delay) rdev->last_off = ktime_get_boottime(); + sysfs_notify(&rdev->dev.kobj, NULL, dev_attr_state.attr.name); + trace_regulator_disable_complete(rdev_get_name(rdev)); return 0; @@ -4848,8 +4856,21 @@ EXPORT_SYMBOL_GPL(regulator_unregister_notifier); static int _notifier_call_chain(struct regulator_dev *rdev, unsigned long event, void *data) { + const char *name; + int ret; + /* call rdev chain first */ - return blocking_notifier_call_chain(&rdev->notifier, event, data); + ret = blocking_notifier_call_chain(&rdev->notifier, event, data); + + if (event & REGULATOR_EVENT_VOLTAGE_CHANGE) { + name = dev_attr_microvolts.attr.name; + sysfs_notify(&rdev->dev.kobj, NULL, name); + } else { + name = dev_attr_status.attr.name; + sysfs_notify(&rdev->dev.kobj, NULL, name); + } + + return ret; } int _regulator_bulk_get(struct device *dev, int num_consumers,