linux-kernel - Re: [PATCH] pwm: Declare waveform stubs for when PWM is not reachable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <zmag5qgkvk2mnkiiyxzdeft6edttolvlmulmtajw4q46setaie@4febizheayqz>
Date: Mon, 13 Oct 2025 11:12:15 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Marcelo Schmitt <marcelo.schmitt1@...il.com>, 
	Marcelo Schmitt <marcelo.schmitt@...log.com>, linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	jic23@...nel.org, kernel test robot <lkp@...el.com>, 
	Trevor Gamblin <tgamblin@...libre.com>, Axel Haslam <ahaslam@...libre.com>
Subject: Re: [PATCH] pwm: Declare waveform stubs for when PWM is not reachable

On Fri, Oct 10, 2025 at 07:51:09PM +0100, Nuno Sá wrote:
> On Fri, 2025-10-10 at 16:01 -0300, Marcelo Schmitt wrote:
> > On 10/10, Nuno Sá wrote:
> > > On Thu, 2025-10-09 at 18:53 +0200, Uwe Kleine-König wrote:
> > ...
> > > > > 
> > > > > Fixes: 6c5126c6406d ("pwm: Provide new consumer API functions for
> > > > > waveforms")
> > > > 
> > > > At the time 6c5126c6406d was applied, there was no user of the API that
> > > > doesn't depend on CONFIG_PWM, so I object adding this Fixes line.
> > 
> > Fair. Looks like the stubs are not the preferred solution so probably going to
> > drop this patch. 
> > 
> > > > 
> > ...
> > > > 
> > > > 	WARNING: unmet direct dependencies detected for
> > > > SPI_OFFLOAD_TRIGGER_PWM
> > > > 	  Depends on [n]: SPI [=y] && SPI_OFFLOAD [=y] && PWM [=n]
> > > > 	  Selected by [y]:
> > > > 	  - AD4030 [=y] && IIO [=y] && SPI [=y] && GPIOLIB [=y]
> > > > 
> > > > This is the thing that needs fixing, i.e. don't select a symbol with
> > > > dependencies that the selecting symbol doesn't assert to be fulfilled.
> > > > 
> > > 
> > > Agreed. Seems to be one of those cases where we select a symbol that depends on
> > > something.
> > > 
> > > However, this driver can indeed work without spi offload and hence PWM and
> > > SPI_OFFLOAD (AFAIR) are optional so I wonder what's the right approach. Looking
> > > at the new series I already see:
> > > 
> > > select SPI_OFFLOAD_TRIGGER_PWM if (SPI_OFFLOAD && PWM)
> > > 
> > > Which makes more sense but I guess we still need the stubs with the above. But 
> > > I would also expect stubs to be needed for spi/offload/consumer.h. Maybe I'm
> > > missing something though...
> > 
> > I have successfully tested it with imply SPI_OFFLOAD_TRIGGER_PWM in Kconfig and
> > MODULE_IMPORT_NS("SPI_OFFLOAD") in the ADC driver.
> > Both the PWM offload trigger and ADC driver were built as modules.
> > I'm not sure the import ns is needed, but ended up breaking the remote test
> > setup so will only be able to tell it next week.
> > 
> > > 
> > > I did not tested but I also wonder if 'imply SPI_OFFLOAD_TRIGGER_PWM' is not
> > > similar to the above.
> > 
> > It works, and I'll update the IIO patch to have
> > 	select SPI_OFFLOAD
> > 	imply PWM
> > 	imply SPI_OFFLOAD_TRIGGER_PWM
> > in Kconfig. The PWM imply is because I think SPI offload support meets the 
> > "highly desirable feature" criterion mentioned by kbuild doc [1].
> 
> With imply we then need to take care either using stubs (which seems not to be an
> option) or with preprocessor conditions in your driver. As discussed in the other

Note that if there is a better reason than broken dependencies, I'm not
a fan of those stubs, but would accept them.

What I don't like about the similar discussion to make GPIOs optional
is: If a hardware might or might not have a reset line connected to a
GPIO, the driver typically does:

	gpiod = gpiod_get_optional(...);
	if (IS_ERR(gpiod))
		return PTR_ERR(gpiod);

. With gpiod_get_optional() being a stub without GPIO support enabled,
you get a working driver if the hardware doesn't have a GPIO. If however
the hardware has a GPIO, IMHO the driver should fail to bind. But
instead it continues to operate and probably fails at some point down
the road, in the worst case with a floating reset pin only working on
Tuesdays or when the fridge door is closed.

So the misconception is: gpiod_get_optional() means: "Give me a GPIO if
the hardware has one *and* GPIO support is enabled". However most driver
authors expect the semantic to be "Give me a GPIO if the hardware has
one." I want to prevent that for PWM.

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)