All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Kees Cook <kees@kernel.org>, Mika Westerberg <westeri@kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Manivannan Sadhasivam <mani@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Saravana Kannan <saravanak@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andy Shevchenko <andy@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Srinivas Kandagatla <srini@kernel.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-sound@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Subject: Re: [PATCH v2 04/10] gpio: shared-proxy: implement the shared GPIO proxy driver
Date: Wed, 22 Oct 2025 21:04:21 +0300	[thread overview]
Message-ID: <aPkcpTWfTb0HOF51@smile.fi.intel.com> (raw)
In-Reply-To: <20251022-gpio-shared-v2-4-d34aa1fbdf06@linaro.org>

On Wed, Oct 22, 2025 at 03:10:43PM +0200, Bartosz Golaszewski wrote:
> 
> Add a virtual GPIO proxy driver which arbitrates access to a single
> shared GPIO by multiple users. It works together with the core shared
> GPIO support from GPIOLIB and functions by acquiring a reference to a
> shared GPIO descriptor exposed by gpiolib-shared and making sure that
> the state of the GPIO stays consistent.
> 
> In general: if there's only one user at the moment: allow it to do
> anything as if this was a normal GPIO (in essence: just propagate calls
> to the underlying real hardware driver). If there are more users: don't
> allow to change the direction set by the initial user, allow to change
> configuration options but warn about possible conflicts and finally:
> treat the output-high value as a reference counted, logical "GPIO
> enabled" setting, meaning: the GPIO value is set to high when the first
> user requests it to be high and back to low once the last user stops
> "voting" for high.

I have two Q:s about the design:
1) why can't the value be counted on the struct gpio_desc level?
2) can gpio-aggregator facilities be reused (to some extent)?

...

> +#include <linux/auxiliary_bus.h>
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/module.h>

+ types.h

> +#include "gpiolib-shared.h"

...

> +out:
> +	if (shared_desc->highcnt)
> +		dev_dbg(proxy->dev,
> +			"Voted for value '%s', effective value is 'high', number of votes for 'high': %u\n",
> +			value ? "high" : "low", shared_desc->highcnt);
> +	else
> +		dev_dbg(proxy->dev, "Voted for value 'low', effective value is 'low'\n");

You can unify and maybe save a few bytes here and there by doing something like
this:

	const char *tmp; // name is a placeholder

	tmp = str_high_low(shared_desc->highcnt);
	dev_dbg(proxy->dev,
		"Voted for value '%s', effective value is '%s', number of votes for '%s': %u\n",
		str_high_low(value), tmp, tmp, shared_desc->highcnt);

...

> +		dev_dbg(proxy->dev,
> +			"Only one user of this shared GPIO, allowing to set direction to output with value '%s'\n",
> +			value ? "high" : "low");

str_high_low() ?

> +		ret = gpiod_direction_output(desc, value);
> +		if (ret)
> +			return ret;
> +
> +		if (value) {
> +			proxy->voted_high = true;
> +			shared_desc->highcnt = 1;
> +		} else {
> +			proxy->voted_high = false;
> +			shared_desc->highcnt = 0;
> +		}
> +
> +		return 0;
> +	}

-- 
With Best Regards,
Andy Shevchenko




  reply	other threads:[~2025-10-22 18:04 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-22 13:10 [PATCH v2 00/10] gpio: improve support for shared GPIOs Bartosz Golaszewski
2025-10-22 13:10 ` [PATCH v2 01/10] string: provide strends() Bartosz Golaszewski
2025-10-22 13:33   ` Andy Shevchenko
2025-10-22 13:40     ` Bartosz Golaszewski
2025-10-22 15:23       ` Andy Shevchenko
2025-10-22 15:25   ` Andy Shevchenko
2025-10-22 15:36     ` Bartosz Golaszewski
2025-10-22 17:12       ` Andy Shevchenko
2025-10-23 18:43         ` Bartosz Golaszewski
2025-10-22 13:10 ` [PATCH v2 02/10] gpiolib: define GPIOD_FLAG_SHARED Bartosz Golaszewski
2025-10-22 13:10 ` [PATCH v2 03/10] gpiolib: implement low-level, shared GPIO support Bartosz Golaszewski
2025-10-22 17:34   ` Andy Shevchenko
2025-10-23 18:55     ` Bartosz Golaszewski
2025-10-24  7:09       ` Andy Shevchenko
2025-10-27 18:02         ` Bartosz Golaszewski
2025-10-22 13:10 ` [PATCH v2 04/10] gpio: shared-proxy: implement the shared GPIO proxy driver Bartosz Golaszewski
2025-10-22 18:04   ` Andy Shevchenko [this message]
2025-10-24  7:03     ` Bartosz Golaszewski
2025-10-22 13:10 ` [PATCH v2 05/10] gpiolib: support shared GPIOs in core subsystem code Bartosz Golaszewski
2025-10-22 13:10 ` [PATCH v2 06/10] gpio: provide gpiod_is_shared() Bartosz Golaszewski
2025-11-11 10:44   ` Linus Walleij
2025-11-12  8:06     ` Bartosz Golaszewski
2025-11-18 22:58       ` Linus Walleij
2025-10-22 13:10 ` [PATCH v2 07/10] arm64: select HAVE_SHARED_GPIOS for ARCH_QCOM Bartosz Golaszewski
2025-10-22 13:10 ` [PATCH v2 08/10] ASoC: wsa881x: drop GPIOD_FLAGS_BIT_NONEXCLUSIVE flag from GPIO lookup Bartosz Golaszewski
2025-10-24 15:46   ` Mark Brown
2025-10-24 23:32   ` Alexey Klimov
2025-10-22 13:10 ` [PATCH v2 09/10] ASoC: wsa883x: " Bartosz Golaszewski
2025-10-24 15:46   ` Mark Brown
2025-10-22 13:10 ` [PATCH v2 10/10] regulator: make the subsystem aware of shared GPIOs Bartosz Golaszewski
2025-10-24 15:57   ` Mark Brown
2025-10-24  7:17 ` [PATCH v2 00/10] gpio: improve support for " Péter Ujfalusi
2025-10-24  7:20   ` Bartosz Golaszewski
2025-10-24  7:32     ` Péter Ujfalusi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPkcpTWfTb0HOF51@smile.fi.intel.com \
    --to=andriy.shevchenko@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=andy@kernel.org \
    --cc=bartosz.golaszewski@linaro.org \
    --cc=brgl@bgdev.pl \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=conor+dt@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kees@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=perex@perex.cz \
    --cc=robh@kernel.org \
    --cc=saravanak@google.com \
    --cc=srini@kernel.org \
    --cc=tiwai@suse.com \
    --cc=westeri@kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.