RE: [PATCH v4] pwm: add sysfs interface

[H Hartley Sweeten]

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.

I'll change the PWM_SYSFS to a hidden option and repost the patch to see if
that is acceptable. Like Ryan mentioned, most test configs have CONFIG_SYSFS
enabled so as long as CONFIG_PWM is enabled we also get the sysfs interface.

Regards,
Hartley


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/