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

Uwe Kleine-König
2025-10-13T09:12:18Z

[-- Attachment #1: Type: text/plain, Size: 3705 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]