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

["Nuno Sá" <noname.nuno@gmail.com>]

On Fri, 2025-10-10 at 17:37 -0300, Marcelo Schmitt wrote:
> ...
> > > > 
> > > > 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)
> ?
> 

With imply I don't think you need those if (...). However, the key point is that
with it, I believe you'll need the stubs (so you need some convincing) because
those configs can be disabled which means your driver should not compile. While
I feel sympathetic with that "depend on optional code", the above does not look
pretty :). For the IIO_BUFFER* stuff I would likely not care about it and for
PWM and SPI_OFFLOAD either depend on we need the stubs.

But if you really feel strong about this, one possible solution would be to try
and factor out all of the spi_offload related code into an extra source file
like ad4030-offload.c (that would have it's own kconfig knob) and that would
need to depend on PWM and then you would also be "free" to have the ad4030-*
stubs in your header file so that it does not fail to compile in case PWM=n.

- Nuno Sá

> 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