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

[Nuno Sá]

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