All of lore.kernel.org
 help / color / mirror / Atom feed
From: dtor_core@ameritech.net (Dmitry Torokhov)
To: sensors@Stimpy.netroedge.com, LKML <linux-kernel@vger.kernel.org>,
	Greg KH <gregkh@suse.de>, Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Subject: [RFC/PATCH 0/22] W1: sysfs, lifetime and other fixes
Date: Thu, 19 May 2005 06:25:53 +0000	[thread overview]
Message-ID: <200504210207.02421.dtor_core@ameritech.net> (raw)

Hi,

I happened to take a look into drivers/w1 and found there bunch of thigs
that IMO should be changed:

- custom-made refcounting is racy
- lifetime rules need to be better enforced
- family framework is insufficient for many advanced w1 devices
- custom-made hotplug notification over netlink should be removed in favor
  of standard hotplug notification
- sysfs attributes have unnecessary prefixes (like w1_master) or not needed
  at all (w1_master_pointer)

Please consider series of patches below. Unfortunately I do not have any W1
equipment so it was only compile-tested. Please also note that lifetime and
locking rules were changed on object-by-object base so mid-series some stuff
may appear broken but as far as I can see the end result shoudl work pretty
well.

w1-whitespace.patch
   Whitespace fixes.

w1-formatting.patch
   Some formatting changes to bring the code in line with CodingStyle
   guidelines.

w1-master-attr-group.patch
   Use attribute_group to create master device attributes to guarantee
   proper cleanup in case of failure. Also, hide most of attribute define
   ugliness in macros.

w1-slave-attr-group.patc
   Add 2 default attributes "family" and "serial" to slave devices, every
   1-Wire slave has them. Use attribute_group to handle. The rest of slave
   attributes are left as is - will be dealt with later.
   
w1-lists-cleanup.patch
   List handling cleanup. Most of the list_for_each_safe users don't need
   *_safe variant, *_entry variant is better suited in most places. Also,
   checking retrieved list element for null is a bit pointless...

w1-drop-owner.patch
   Drop owner field from w1_master and w1_slave structures. Just having it
   there does not magically fixes lifetime rules.

w1-bus-ops.patch
   Cleanup bus operations code:
   - have bus operatiions accept w1_master instead of unsigned long and
     drop data field from w1_bus_master so the structure can be statically
     allocated by driver implementing it;
   - rename w1_bus_master to w1_bus_ops to avoid confusion with w1_master;
   - separate master registering and allocation so drivers can setup proper
     link between private data and master and set useable master's name.

w1-fold-w1-int.patch
   Fold w1_int.c into w1.c - there is no point in artificially separating
   code for master devices between 2 files.

w1-drop-netlink.patch
   Drop custom-made hotplug over netlink notification from w1 core.
   Standard hotplug mechanism should work just fine (patch will follow).

w1-drop-control-thread.patch
   Drop control thread from w1 core, whatever it does can also be done in
   the context of w1_remove_master_device. Also, pin the module when
   registering new master device to make sure that w1 core is not unloaded
   until last device is gone. This simplifies logic a lot.

w1-move-search-to-io.patch
   Move w1_search function to w1_io.c to be with the rest of IO code.

w1-master-drop-attrs.patch
   Get rid of unneeded master device attributes:
   - 'pointer' and 'attempts' are meaningless for userspace;
   - information provided by 'slaves' and 'slave_count' can be gathered
     from other sysfs bits;
   - w1_slave_found has to be rearranged now that slave_count field is gone.

w1-master-attr-cleanup.patch
   Clean-up master attribute implementation:
   - drop unnecessary "w1_master" prefix from attribute names;
   - do not acquire master->mutex when accessing attributes;
   - move attribute code "closer" to the rest of master code.

w1-master-scan-interval.patch
   More master attributes changes:
   - rename timeout parameter/attribute to scan_interval to better
     reflect its purpose;
   - make scan_timeout be a per-device attribute and allow changing
     it from userspace via sysfs;
   - allow changing max_slave_count it from userspace as well.

w1-master-add-ttl-attr.patch
   Add slave_ttl attribute to w1 masters.

w1-master-cleanup.patch
   Clean-up master device implementation:
   - get rid of separate refcount, rely on driver model to enforce
     lifetime rules;
   - use atomic to generate unique master IDs;
   - drop unused fields.

w1-slave-cleanup.patch
   Clean-up slave device implementation:
   - get rid of separate refcount, rely on driver model to enforce
     lifetime rules;
   - pin w1 module until slave device is registered with sysfs to make
     sure W1 core stays loaded.
   - drop 'name' attribute as we already have it in bus_id.

w1-family-cleanup.patch
   Clean-up family implementation:
   - get rid of w1_family_ops and template attributes in w1_slave
     structure and have family drivers create necessary attributes
     themselves. There are too many different devices using 1-Wire
     interface and it is impossible to fit them all into single
     attribute model. If interface unification is needed it can be
     done by building cross-bus class hierarchy.
   - rename w1_smem to w1_sernum because devices are called Silicon
     serial numbers, they have address (ID) but don't have memory
     in regular sense.
   - rename w1_therm to w1_thermal.

w1-family-is-driver.patch
   Convert family into proper device-model drivers:
   - embed driver structure into w1_family and register with the
     driver core;
   - do not try to manually bind slaves to familes, leave it to
     the driver core;
   - fold w1_family.c into w1.c

w1-device-id.patch
   Support for automatic family drivers loading via hotplug:
   - allow family drivers support list of families;
   - export supported families through MODULE_DEVICE_TABLE.

w1-hotplug.patch
   Implement W1 bus hotplug handler. Slave devices will define
   FID=%02x (family ID) end SN=%024llX environment variables.

w1-module-attrs.patch
   Allow changing w1 module parameters through sysfs, add parameter
   descriptions and document them in Documentation/kernel-parameters.txt

Thanks!

-- 
Dmitry

WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Torokhov <dtor_core@ameritech.net>
To: sensors@Stimpy.netroedge.com, LKML <linux-kernel@vger.kernel.org>,
	Greg KH <gregkh@suse.de>, Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Subject: [RFC/PATCH 0/22] W1: sysfs, lifetime and other fixes
Date: Thu, 21 Apr 2005 02:07:02 -0500	[thread overview]
Message-ID: <200504210207.02421.dtor_core@ameritech.net> (raw)

Hi,

I happened to take a look into drivers/w1 and found there bunch of thigs
that IMO should be changed:

- custom-made refcounting is racy
- lifetime rules need to be better enforced
- family framework is insufficient for many advanced w1 devices
- custom-made hotplug notification over netlink should be removed in favor
  of standard hotplug notification
- sysfs attributes have unnecessary prefixes (like w1_master) or not needed
  at all (w1_master_pointer)

Please consider series of patches below. Unfortunately I do not have any W1
equipment so it was only compile-tested. Please also note that lifetime and
locking rules were changed on object-by-object base so mid-series some stuff
may appear broken but as far as I can see the end result shoudl work pretty
well.

w1-whitespace.patch
   Whitespace fixes.

w1-formatting.patch
   Some formatting changes to bring the code in line with CodingStyle
   guidelines.

w1-master-attr-group.patch
   Use attribute_group to create master device attributes to guarantee
   proper cleanup in case of failure. Also, hide most of attribute define
   ugliness in macros.

w1-slave-attr-group.patc
   Add 2 default attributes "family" and "serial" to slave devices, every
   1-Wire slave has them. Use attribute_group to handle. The rest of slave
   attributes are left as is - will be dealt with later.
   
w1-lists-cleanup.patch
   List handling cleanup. Most of the list_for_each_safe users don't need
   *_safe variant, *_entry variant is better suited in most places. Also,
   checking retrieved list element for null is a bit pointless...

w1-drop-owner.patch
   Drop owner field from w1_master and w1_slave structures. Just having it
   there does not magically fixes lifetime rules.

w1-bus-ops.patch
   Cleanup bus operations code:
   - have bus operatiions accept w1_master instead of unsigned long and
     drop data field from w1_bus_master so the structure can be statically
     allocated by driver implementing it;
   - rename w1_bus_master to w1_bus_ops to avoid confusion with w1_master;
   - separate master registering and allocation so drivers can setup proper
     link between private data and master and set useable master's name.

w1-fold-w1-int.patch
   Fold w1_int.c into w1.c - there is no point in artificially separating
   code for master devices between 2 files.

w1-drop-netlink.patch
   Drop custom-made hotplug over netlink notification from w1 core.
   Standard hotplug mechanism should work just fine (patch will follow).

w1-drop-control-thread.patch
   Drop control thread from w1 core, whatever it does can also be done in
   the context of w1_remove_master_device. Also, pin the module when
   registering new master device to make sure that w1 core is not unloaded
   until last device is gone. This simplifies logic a lot.

w1-move-search-to-io.patch
   Move w1_search function to w1_io.c to be with the rest of IO code.

w1-master-drop-attrs.patch
   Get rid of unneeded master device attributes:
   - 'pointer' and 'attempts' are meaningless for userspace;
   - information provided by 'slaves' and 'slave_count' can be gathered
     from other sysfs bits;
   - w1_slave_found has to be rearranged now that slave_count field is gone.

w1-master-attr-cleanup.patch
   Clean-up master attribute implementation:
   - drop unnecessary "w1_master" prefix from attribute names;
   - do not acquire master->mutex when accessing attributes;
   - move attribute code "closer" to the rest of master code.

w1-master-scan-interval.patch
   More master attributes changes:
   - rename timeout parameter/attribute to scan_interval to better
     reflect its purpose;
   - make scan_timeout be a per-device attribute and allow changing
     it from userspace via sysfs;
   - allow changing max_slave_count it from userspace as well.

w1-master-add-ttl-attr.patch
   Add slave_ttl attribute to w1 masters.

w1-master-cleanup.patch
   Clean-up master device implementation:
   - get rid of separate refcount, rely on driver model to enforce
     lifetime rules;
   - use atomic to generate unique master IDs;
   - drop unused fields.

w1-slave-cleanup.patch
   Clean-up slave device implementation:
   - get rid of separate refcount, rely on driver model to enforce
     lifetime rules;
   - pin w1 module until slave device is registered with sysfs to make
     sure W1 core stays loaded.
   - drop 'name' attribute as we already have it in bus_id.

w1-family-cleanup.patch
   Clean-up family implementation:
   - get rid of w1_family_ops and template attributes in w1_slave
     structure and have family drivers create necessary attributes
     themselves. There are too many different devices using 1-Wire
     interface and it is impossible to fit them all into single
     attribute model. If interface unification is needed it can be
     done by building cross-bus class hierarchy.
   - rename w1_smem to w1_sernum because devices are called Silicon
     serial numbers, they have address (ID) but don't have memory
     in regular sense.
   - rename w1_therm to w1_thermal.

w1-family-is-driver.patch
   Convert family into proper device-model drivers:
   - embed driver structure into w1_family and register with the
     driver core;
   - do not try to manually bind slaves to familes, leave it to
     the driver core;
   - fold w1_family.c into w1.c

w1-device-id.patch
   Support for automatic family drivers loading via hotplug:
   - allow family drivers support list of families;
   - export supported families through MODULE_DEVICE_TABLE.

w1-hotplug.patch
   Implement W1 bus hotplug handler. Slave devices will define
   FID=%02x (family ID) end SN=%024llX environment variables.

w1-module-attrs.patch
   Allow changing w1 module parameters through sysfs, add parameter
   descriptions and document them in Documentation/kernel-parameters.txt

Thanks!

-- 
Dmitry

             reply	other threads:[~2005-05-19  6:25 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-04-21  7:07 Dmitry Torokhov [this message]
2005-05-19  6:25 ` [RFC/PATCH 0/22] W1: sysfs, lifetime and other fixes Dmitry Torokhov
2005-04-21  7:08 ` [RFC/PATCH 1/22] W1: whitespace fixes Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:08 ` [RFC/PATCH 2/22] W1: formatting fixes Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:09 ` [RFC/PATCH 3/22] W1: use attribute group for master's attributes Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:10 ` [RFC/PATCH 4/22] W1: use attribute group for slave's attributes Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:11 ` [RFC/PATCH 5/22] W1: list handling cleanup Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:13 ` [RFC/PATCH 6/22] W1: drop owner field from master and slave structures Dmitry Torokhov
2005-05-19  6:25   ` [RFC/PATCH 6/22] W1: drop owner field from master and slave Dmitry Torokhov
2005-04-21  7:13 ` [RFC/PATCH 7/22] W1: bus operations cleanup Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:15 ` [RFC/PATCH 8/22] W1: merge master code into one file Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:16 ` [RFC/PATCH 9/22] W1: drop custom hotplug over netlink notification Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:17 ` [RFC/PATCH 10/22] W1: drop main control thread Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:18 ` [RFC/PATCH 11/22] W1: move w1_search to the rest of IO code Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:19 ` [RFC/PATCH 12/22] W1: drop unneeded master attributes Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:20 ` [RFC/PATCH 13/22] W1: cleanup master attributes handling Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:21 ` [RFC/PATCH 14/22] W1: rename timeout to scan_interval Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:22 ` [RFC/PATCH 15/22] W1: add slave_ttl master attribute Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:23 ` [RFC/PATCH 16/22] W1: cleanup masters refcounting & more Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:23 ` [RFC/PATCH 17/22] W1: cleanup slave " Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:25 ` [RFC/PATCH 18/22] W1: cleanup family implementation Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:26 ` [RFC/PATCH 19/22] W1: convert families to be proper sysfs rivers Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:27 ` [RFC/PATCH 20/22] W1: add w1_device_id/MODULE_DEVICE_TABLE for automatic driver loading Dmitry Torokhov
2005-05-19  6:25   ` [RFC/PATCH 20/22] W1: add w1_device_id/MODULE_DEVICE_TABLE for Dmitry Torokhov
2005-04-21  7:36 ` [RFC/PATCH 21/22] W1: implement standard hotplug handler Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21  7:38 ` [RFC/PATCH 22/22] W1: expose module parameters in sysfs Dmitry Torokhov
2005-05-19  6:25   ` Dmitry Torokhov
2005-04-21 13:18 ` [RFC/PATCH 0/22] W1: sysfs, lifetime and other fixes Evgeniy Polyakov
2005-05-19  6:25   ` Evgeniy Polyakov
2005-04-21 14:31   ` Dmitry Torokhov
2005-05-19  6:25     ` Dmitry Torokhov
2005-04-25  9:08     ` Evgeniy Polyakov
2005-05-19  6:25       ` Evgeniy Polyakov
2005-04-25 16:32       ` Dmitry Torokhov
2005-05-19  6:25         ` Dmitry Torokhov
2005-04-25 19:26         ` Evgeniy Polyakov
2005-05-19  6:25           ` Evgeniy Polyakov
2005-04-25 21:32           ` Dmitry Torokhov
2005-05-19  6:25             ` Dmitry Torokhov
2005-04-26  7:19             ` Evgeniy Polyakov
2005-05-19  6:25               ` Evgeniy Polyakov
2005-04-25 20:15         ` Evgeniy Polyakov
2005-05-19  6:25           ` Evgeniy Polyakov
2005-04-25 20:22           ` Dmitry Torokhov
2005-05-19  6:25             ` Dmitry Torokhov
2005-04-26  6:43             ` Evgeniy Polyakov
2005-05-19  6:25               ` Evgeniy Polyakov
2005-04-26  6:50               ` Dmitry Torokhov
2005-05-19  6:25                 ` Dmitry Torokhov
2005-04-26  7:06                 ` Evgeniy Polyakov
2005-05-19  6:25                   ` Evgeniy Polyakov
2005-04-26  7:16                   ` Dmitry Torokhov
2005-05-19  6:25                     ` Dmitry Torokhov
2005-04-26  7:35                     ` Evgeniy Polyakov
2005-05-19  6:25                       ` Evgeniy Polyakov
2005-04-26  7:00               ` Greg KH
2005-05-19  6:25                 ` Greg KH
2005-04-26  7:17                 ` Evgeniy Polyakov
2005-05-19  6:25                   ` Evgeniy Polyakov
2005-04-26  6:58         ` Greg KH
2005-05-19  6:25           ` Greg KH
2005-04-21 16:09   ` Dmitry Torokhov
2005-05-19  6:25     ` Dmitry Torokhov
2005-04-25  9:11     ` Evgeniy Polyakov
2005-05-19  6:25       ` Evgeniy Polyakov
2005-04-25 16:36       ` Dmitry Torokhov
2005-05-19  6:25         ` Dmitry Torokhov
2005-04-25 19:32         ` Evgeniy Polyakov
2005-05-19  6:25           ` Evgeniy Polyakov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200504210207.02421.dtor_core@ameritech.net \
    --to=dtor_core@ameritech.net \
    --cc=gregkh@suse.de \
    --cc=johnpol@2ka.mipt.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sensors@Stimpy.netroedge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.