Messages in this thread | | | Date | Wed, 18 Feb 2009 20:14:19 +0100 | Subject | Re: [PATCH] Export device_add_attributes() so drivers can use it. | From | Kay Sievers <> |
| |
On Wed, Feb 18, 2009 at 19:39, Greg KH <gregkh@suse.de> wrote: > On Wed, Feb 18, 2009 at 07:32:19PM +0100, Kay Sievers wrote: >> On Wed, Feb 18, 2009 at 18:51, Grant Likely <grant.likely@secretlab.ca> wrote: >> > On Wed, Feb 18, 2009 at 9:49 AM, Kay Sievers <kay.sievers@vrfy.org> wrote: >> >> On Wed, Feb 18, 2009 at 17:34, Grant Likely <grant.likely@secretlab.ca> wrote: >> >>> On Wed, Feb 18, 2009 at 8:53 AM, Kay Sievers <kay.sievers@vrfy.org> wrote: >> >>>> On Wed, Feb 18, 2009 at 16:48, Grant Likely <grant.likely@secretlab.ca> wrote: >> >>>>> On Wed, Feb 18, 2009 at 8:45 AM, Kay Sievers <kay.sievers@vrfy.org> wrote: >> >>>>>> On Wed, Feb 18, 2009 at 16:29, Greg KH <gregkh@suse.de> wrote: >> >>>>>>> On Wed, Feb 18, 2009 at 08:11:34AM -0700, Grant Likely wrote: >> >>>>>>>> From: Grant Likely <grant.likely@secretlab.ca> >> >>>>>>>> >> >>>>>>>> I find myself using the pattern of device_add_attributes() and >> >>>>>>>> device_remove_attributes() frequently in my drivers. Rather than >> >>>>>>>> reinventing the wheel every time, I'm floating this patch to export >> >>>>>>>> the symbols to see how it is received. If this looks okay then I'll >> >>>>>>>> rework my drivers and post additional patches to use these functions. >> >>>>>>> >> >>>>>>> No objection from me, as long as the symbols are EXPORT_SYMBOL_GPL(), >> >>>>>>> like the rest of the driver core. Is that ok with you? >> >>>>>> >> >>>>>> These functions used outside the core create attributes after the >> >>>>>> uevent is sent, and userspace will not see these files at event time. >> >>>>>> This is in most cases a pretty broken behavior. Is that the expected >> >>>>>> behavior in your drivers? >> >>>>> >> >>>>> ??? I don't follow what you mean. >> >>>>> >> >>>>> I'm using these functions to allow the driver to add device attribs; >> >>>>> primarily for debugging knobs and controls. Userspace will see the >> >>>>> files after the driver is bound to the device. The uevent doesn't >> >>>>> really come into play. >> >>>> >> >>>> Sure, they do. Many things expect all files which are visible at the >> >>>> device to be readable also at event time. That's the whole way udev >> >>>> and device property matching works. There are only a few exceptions >> >>>> where creating files at a device later, after it is registered with >> >>>> the core, is not a bug. >> >>> >> >>> Let me make sure I understand you... >> >>> >> >>> Is it a bug for a device driver to call >> >>> device_create_file()/device_remove_file() at probe time? For example, >> >>> if I have a data capture device which is probed via the platform bus, >> >>> is it okay for the .probe() function for the driver to use >> >>> device_create_file() to add a 'rate_statistics' file which dumps out >> >>> some data rate statistics in ASCII form? >> >>> >> >>> I was under the impression that >> >>> device_create_file()/device_remove_file() were okay to use at probe >> >>> time. device_add_attributes()/device_remove_attributes() are only >> >>> wrappers around device_create_file()/device_remove_file() with error >> >>> checking and unwinding when things go wrong. >> >>> >> >>> Am I incorrect here? >> >> >> >> You are probing an existing "struct device", and then create >> >> attributes at this device when the probe succeeds? >> > >> > Yes >> > >> >> If yes, why don't >> >> you create a new child "struct device" with your functionality and add >> >> the attributes there? >> > >> > Mostly because I have no need another full struct device, and none of >> > the files that I'm adding have any bearing on udev. They are debug >> > and statistical files for an embedded system that are used by the >> > developer. I don't want application code depending on them and I'm >> > not interested in having them advertised. >> >> That's what debugfs is for. > > Kay is right, use debugfs for stuff like this. Not sysfs. > > So sorry, I'm not going to apply the patch, and you should change your > code to not open-code this as well.
You can probably use an attribute group instead, if you want to create a bunch of attributes with error handling. If the group has a name, the attributes end in a subdir, without a name, they will be created directly.
Kay
| |