pwm: Declare waveform stubs for when PWM is not reachable

[PATCH] pwm: Declare waveform stubs for when PWM is not reachable

Posted by Marcelo Schmitt 4 months ago

Previously, the PWM waveform consumer API would not be declared if
CONFIG_PWM was not reachable. That caused kernel builds to fail if a
consumer driver was enabled but PWM disabled. Add stubs for PWM waveform
functions so client drivers that use, but don't depend on PWM, can build if
PWM is disabled.

Fixes: 6c5126c6406d ("pwm: Provide new consumer API functions for waveforms")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202509272028.0zLNiR5w-lkp@intel.com/
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Cc: Jonathan Cameron <jic23@kernel.org>
Hi Uwe,

This is a fix based on a report from 0-day bot [1].
We need this for a sophisticated IIO device that makes direct use of a PWM
waveform (in addition to indirect use of PWM through SPI_OFFLOAD_TRIGGER_PWM). 
I'm not very familiar with the details of how it works for series of
patches that update multiple subsystems. Documentation says such sets may go
through the -mm tree. Though, this is a small change and the consumer driver set
depends on it. Would it be okay if this gets picked up through Jonathan's IIO tree?

[1]: https://lore.kernel.org/linux-iio/202509272028.0zLNiR5w-lkp@intel.com/

Thanks,
Marcelo

 include/linux/pwm.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 549ac4aaad59..a20ddc40a32a 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -504,6 +504,25 @@ struct pwm_device *devm_fwnode_pwm_get(struct device *dev,
 				       struct fwnode_handle *fwnode,
 				       const char *con_id);
 #else
+static inline int pwm_round_waveform_might_sleep(struct pwm_device *pwm, struct pwm_waveform *wf)
+{
+	might_sleep();
+	return -EOPNOTSUPP;
+}
+
+static inline int pwm_get_waveform_might_sleep(struct pwm_device *pwm, struct pwm_waveform *wf)
+{
+	might_sleep();
+	return -EOPNOTSUPP;
+}
+
+static inline int pwm_set_waveform_might_sleep(struct pwm_device *pwm,
+					       const struct pwm_waveform *wf, bool exact)
+{
+	might_sleep();
+	return -EOPNOTSUPP;
+}
+
 static inline bool pwm_might_sleep(struct pwm_device *pwm)
 {
 	return true;

base-commit: 8f2689f194b8d1bff41150ae316abdfccf191309
-- 
2.39.2

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

Posted by Uwe Kleine-König 4 months ago

Hello Marcelo,

On Tue, Oct 07, 2025 at 07:19:38PM -0300, Marcelo Schmitt wrote:
> Previously, the PWM waveform consumer API would not be declared if
> CONFIG_PWM was not reachable. That caused kernel builds to fail if a
> consumer driver was enabled but PWM disabled. Add stubs for PWM waveform
> functions so client drivers that use, but don't depend on PWM, can build if
> PWM is disabled.
> 
> 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.

> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202509272028.0zLNiR5w-lkp@intel.com/
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Cc: Jonathan Cameron <jic23@kernel.org>
> Hi Uwe,
> 
> This is a fix based on a report from 0-day bot [1].
> We need this for a sophisticated IIO device that makes direct use of a PWM
> waveform (in addition to indirect use of PWM through SPI_OFFLOAD_TRIGGER_PWM). 

Does the driver work in some configuration with the pwm stubs? If not,
the right thing to do is to let it depend on PWM. (Note the inverse
isn't necessarily a good idea.)

And I wonder how you could even compile your driver without PWM support
given that it selects SPI_OFFLOAD_TRIGGER_PWM which depends on PWM.

... some time later ...

OK, I tried to reproduce the problem that the kernel build bot run into.
There is a warning:

	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.

Best regards
Uwe

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

Posted by Nuno Sá 4 months ago

On Thu, 2025-10-09 at 18:53 +0200, Uwe Kleine-König wrote:
> Hello Marcelo,
> 
> On Tue, Oct 07, 2025 at 07:19:38PM -0300, Marcelo Schmitt wrote:
> > Previously, the PWM waveform consumer API would not be declared if
> > CONFIG_PWM was not reachable. That caused kernel builds to fail if a
> > consumer driver was enabled but PWM disabled. Add stubs for PWM waveform
> > functions so client drivers that use, but don't depend on PWM, can build if
> > PWM is disabled.
> > 
> > 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.
> 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes:
> > https://lore.kernel.org/oe-kbuild-all/202509272028.0zLNiR5w-lkp@intel.com/
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Hi Uwe,
> > 
> > This is a fix based on a report from 0-day bot [1].
> > We need this for a sophisticated IIO device that makes direct use of a PWM
> > waveform (in addition to indirect use of PWM through
> > SPI_OFFLOAD_TRIGGER_PWM). 
> 
> Does the driver work in some configuration with the pwm stubs? If not,
> the right thing to do is to let it depend on PWM. (Note the inverse
> isn't necessarily a good idea.)
> 
> And I wonder how you could even compile your driver without PWM support
> given that it selects SPI_OFFLOAD_TRIGGER_PWM which depends on PWM.
> 
> ... some time later ...
> 
> OK, I tried to reproduce the problem that the kernel build bot run into.
> There is a warning:
> 
> 	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 did not tested but I also wonder if 'imply SPI_OFFLOAD_TRIGGER_PWM' is not
similar to the above.

- Nuno Sá

> Best regards
> Uwe

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

Posted by Marcelo Schmitt 4 months ago

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].

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/kconfig-language.rst?h=v6.17#n197

One alternative to this patch is to have `#if IS_REACHABLE(CONFIG_PWM)` in the
ADC driver as David suggested in the other thread. I'll probably do that and
drop the changes to PWM.

I first thought of using `#ifdef CONFIG_PWM`, but couldn't convince myself about
that from the relatively small number of ifdef use-cases in IIO.

Thanks,
Marcelo

> 
> - Nuno Sá
> 
> > Best regards
> > Uwe

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

Posted by Nuno Sá 4 months ago

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
thread I would just select SPI_OFFLOAD. Basically I would:

	select SPI_OFFLOAD
	select SPI_OFFLOAD_TRIGGER_PWM
	depends on PWM

- Nuno Sá
	
> 
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/kconfig-language.rst?h=v6.17#n197
> 
> One alternative to this patch is to have `#if IS_REACHABLE(CONFIG_PWM)` in the
> ADC driver as David suggested in the other thread. I'll probably do that and
> drop the changes to PWM.
> 
> I first thought of using `#ifdef CONFIG_PWM`, but couldn't convince myself about
> that from the relatively small number of ifdef use-cases in IIO.
> 
> Thanks,
> Marcelo
> 
> > 
> > - Nuno Sá
> > 
> > > Best regards
> > > Uwe

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

Posted by Uwe Kleine-König 3 months, 4 weeks ago

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

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

Posted by Marcelo Schmitt 4 months ago

...
> > > 
> > > 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
> thread I would just select SPI_OFFLOAD. Basically I would:
> 
> 	select SPI_OFFLOAD
> 	select SPI_OFFLOAD_TRIGGER_PWM
> 	depends on PWM

Yeah, depending on PWM is what I was trying to avoid because the ADC can be used
without PWM. Doing the above is the easiest solution - depend on everything,
select everything. Though, I guess I'm technically not keeping backwards
compatibility if I add a new dependency to the driver.

IIO_BUFFER_DMA and IIO_BUFFER_DMAENGINE are part of IIO subsystem so okay to
select them? Otherwise, yeah, they should be optional too (would either imply
them or select if SPI_OFFLOAD).

I'm currently leaning towards
 	imply PWM
 	imply SPI_OFFLOAD_TRIGGER_PWM //(SPI_OFFLOAD_TRIGGER_PWM depends on SPI_OFFLOAD)
but not really sure.

It's sort of a feature bundle we want to enable to provide SPI offloading.

if SPI_OFFLOAD && PWM
	select SPI_OFFLOAD_TRIGGER_PWM
	select IIO_BUFFER_DMA
	select IIO_BUFFER_DMAENGINE

we can have
	imply IIO_BUFFER_DMA
	imply IIO_BUFFER_DMAENGINE
 	imply PWM
 	imply SPI_OFFLOAD_TRIGGER_PWM

but we could then have IIO_BUFFER_DMA=y and PWM=n and still be unable to SPI offload?

Maybe
	imply IIO_BUFFER_DMA if (SPI_OFFLOAD && PWM)
	imply IIO_BUFFER_DMAENGINE if (SPI_OFFLOAD && PWM)
 	imply PWM
 	imply SPI_OFFLOAD_TRIGGER_PWM if (SPI_OFFLOAD && PWM)
?

Forgot to add David to CC list on previous reply so doing it now.

> 
> - Nuno Sá
> 	
> > 
> > [1]:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/kconfig-language.rst?h=v6.17#n197
> > 
> > One alternative to this patch is to have `#if IS_REACHABLE(CONFIG_PWM)` in the
> > ADC driver as David suggested in the other thread. I'll probably do that and
> > drop the changes to PWM.
> > 
> > I first thought of using `#ifdef CONFIG_PWM`, but couldn't convince myself about
> > that from the relatively small number of ifdef use-cases in IIO.
> > 
> > Thanks,
> > Marcelo
> > 
> > > 
> > > - Nuno Sá
> > > 
> > > > Best regards
> > > > Uwe