Re: [PATCH v4] pwm: add sysfs interface

[Thierry Reding]

On Tue, Jun 11, 2013 at 11:47:23AM -0500, H Hartley Sweeten wrote:
> On Tuesday, June 11, 2013 9:09 AM, H Hartley Sweeten wrote:
> > On Tuesday, June 11, 2013 4:29 AM, Ryan Mallon wrote:
> >> On 11/06/13 20:14, Thierry Reding wrote:
> >>> On Mon, Jun 10, 2013 at 04:12:07PM -0700, H Hartley Sweeten wrote:
> >>>> +config PWM_SYSFS
> >>>> +	bool "/sys/class/pwm/... (sysfs interface)"
> >>>> +	depends on SYSFS
> >>>> +	help
> >>>> +	  Say Y here to provide a sysfs interface to control PWMs.
> >>>> +
> >>>> +	  For every instance of a PWM device there is a pwmchipN directory
> >>>> +	  created in /sys/class/pwm. Use the export attribute to request
> >>>> +	  a PWM to be accessible from userspace and the unexport attribute
> >>>> +	  to return the PWM to the kernel. Each exported PWM will have a
> >>>> +	  pwmX directory in the pwmchipN it is associated with.
> >>> 
> >>> I have a small quibble with this. Introducing options like this make it
> >>> increasingly difficult to compile-test all the various combinations, so
> >>> I'd like to see this converted to a form that will play well with the
> >>> IS_ENABLED() macro. We already have the same issue with DEBUG_FS, only
> >>> to a lesser degree because it doesn't have an additional PWM-specific
> >>> Kconfig option.
> >
> > How about removing the Kconfig option and just doing:
> >
> > obj-$(CONFIG_SYSFS)			+= sysfs.o
> >
> > This way the PWM sysfs interface is always compiled and included in the build
> > as long as CONFIG_SYSFS is enabled. The check in the header would change to
> 
> That didn't work. As Ryan pointed out we get undefined references due to
> sysfs.c being compiled but not core.c when CONFIG_PWM is not enabled.

Why not add dummies for the missing functions? It was my impression that
we had dummies for all of them already, but if not they should certainly
be added to match what other subsystems do.

Thierry

Attachment: pgp00000.pgp
Description: PGP signature