linux-kernel - RE: [PATCH v4] pwm: add sysfs interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 11 Jun 2013 13:45:37 -0500
From:	H Hartley Sweeten <hartleys@...ionengravers.com>
To:	Thierry Reding <thierry.reding@...il.com>
CC:	Ryan Mallon <rmallon@...il.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	"linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"poeschel@...onage.de" <poeschel@...onage.de>,
	"rob@...dley.net" <rob@...dley.net>
Subject: RE: [PATCH v4] pwm: add sysfs interface

On Tuesday, June 11, 2013 11:35 AM, Thierry Reding wrote:
> 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.

I just looked it over and the undefined reference was for pwm_set_polarity().
The dummy for that function is missing.

The other problem with this approach is the sysfs.c file gets compiled whenever
CONFIG_SYSFS is enabled even if CONFIG_PWM is not enabled.

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