pull request: bluetooth-next 2012-03-01

pull request: bluetooth-next 2012-03-01

[Gustavo Padovan]

[-- Attachment #1: Type: text/plain, Size: 19411 bytes --]

Hi Dave,

Here is the Bluetooth pull request for 3.4, but first there is some things I
need to explain:
 - I'm pulling directly to you because due to the coding style issues that
   happened some days ago. We decided, after talk to John, to pull directly to
   you, so you can take a look before pull. I personally looked to the whole
   diff of this pull request and couldn't spot anything that doesn't seem
   specified in CodingStyle.
 - This is a big pull request. It happened that this is the busiest Bluetooth
   release cycle I have ever seen and I had to be away from development and
   maintainance for about one month. Putting these two facts together it
   happened that we have now more than 200 patches queued in the Bluetooth
   subsystem tree.

About the patches:
 - almost half of the patches is in the new Bluetooth Management interface, it
 finally reached a stable state regarding its API and we now enable it by
 default.
 - lots of Bluetooth LE patches. This implementation is still unstable but
   disabled by default.
 - some L2CAP layer refactoring to make our core code more generic and
   reusable.
 - New HCI monitor interface to monitor Bluetooth flow.
 - Lot of fixes and clean ups all over the Bluetooth subsystem.

	Gustavo


The following changes since commit b4017c5368f992fb8fb3a2545a0977082c6664e4:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2012-03-01 17:57:40 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-next master

Andre Guedes (24):
      Bluetooth: Add 'eir_len' param to mgmt_device_found()
      Bluetooth: Report LE devices
      Bluetooth: Use GFP_KERNEL in hci_conn_add()
      Bluetooth: Use GFP_KERNEL in hci_chan_create()
      Bluetooth: Fix potential deadlock
      Bluetooth: Remove unneeded locking
      Bluetooth: Use GFP_KERNEL in hci_add_adv_entry()
      Bluetooth: LE scan should send Discovering events
      Bluetooth: Minor code refactoring
      Bluetooth: Add hci_do_le_scan()
      Bluetooth: Add hci_le_scan()
      Bluetooth: MGMT start discovery LE-Only support
      Bluetooth: Fix indentation
      Bluetooth: Add BT_DBG to mgmt_discovering()
      Bluetooth: Fix discovery state machine
      Bluetooth: Fix event sending with DISCOVERY_STOPPED state
      Bluetooth: Prepare start_discovery
      Bluetooth: Track discovery type
      Bluetooth: Merge INQUIRY and LE_SCAN discovery states
      Bluetooth: Interleaved discovery support
      Bluetooth: Set DISCOVERY_STOPPED if controller resets
      Bluetooth: Change interleaved discovery behavior
      Bluetooth: Fix Kconfig help description
      Bluetooth: Check capabilities in BR/EDR and LE-Only discovery

Andrei Emeltchenko (30):
      Bluetooth: Process num completed data blocks event
      Bluetooth: Remove magic number from ACL TO
      Bluetooth: Use chan instead of sk
      Bluetooth: Change sk to l2cap_chan
      Bluetooth: trivial: space correction
      Bluetooth: Add alloc_skb chan operator
      Bluetooth: Use list _safe deleting from conn_hash_list
      Bluetooth: Use list _safe deleting from conn chan_list
      Bluetooth: Recalculate sched HCI blk/pkt flow ctrl
      Bluetooth: Helper removes duplicated code
      Bluetooth: Change chan_ready param from sk to chan
      Bluetooth: Clean up l2cap_chan_add
      Bluetooth: Remove unneeded sk variable
      Bluetooth: Do not dereference zero sk
      Bluetooth: Move scope of state_to_string
      Bluetooth: Use symbolic names for state in debug
      Bluetooth: Prefix hex numbers with object name
      Bluetooth: trivial: Fix long line
      Bluetooth: Revert to mutexes from RCU list
      Bluetooth: Add l2cap_chan_lock
      Bluetooth: Add locked and unlocked state_change
      Bluetooth: Add socket error function
      Bluetooth: Fix coding style issues in mgmt code
      Bluetooth: Add unlocked __l2cap_chan_add function
      Bluetooth: Change sk lock to chan lock in L2CAP core
      Bluetooth: Remove socket lock check
      Bluetooth: Fix init request completion with AMP controllers
      Bluetooth: Fix double locking in LE and conless chan
      Bluetooth: Remove duplicated code in l2cap conn req
      Bluetooth: Save remote L2CAP fixed channel mask

Andrzej Kaczmarek (2):
      Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket
      Bluetooth: l2cap_set_timer needs jiffies as timeout value

Dan Carpenter (2):
      Bluetooth: use kfree_skb() instead of kfree()
      Bluetooth: change min_t() cast in hci_reassembly()

Daniel Wagner (1):
      Bluetooth: Don't mark non xfer isoc endpoint URBs with URB_ISO_ASAP

David Herrmann (28):
      Bluetooth: hci-uart-ll: Use GFP_ATOMIC in open()
      Bluetooth: hci-uart-h4: Use GFP_ATOMIC in open()
      Bluetooth: hci-uart-bcsp: Use GFP_ATOMIC in open()
      Bluetooth: hci-uart-ath: Use GFP_ATOMIC in open()
      Bluetooth: dtl1: Fix memleak in probe()
      Bluetooth: Make hci-destruct callback optional
      Bluetooth: bluecard-cs: Remove empty destruct cb
      Bluetooth: bt3c-cs: Remove empty destruct cb
      Bluetooth: btmrvl: Remove empty destruct cb
      Bluetooth: btuart-cs: Remove empty destruct cb
      Bluetooth: btwilink: Remove empty destruct cb
      Bluetooth: dtl1-cs: Remove empty destruct cb
      Bluetooth: vhci: Free driver_data on file release
      Bluetooth: bfusb: Free driver_data on USB shutdown
      Bluetooth: btusb: Free driver data on USB shutdown
      Bluetooth: bpa10x: Free private driver data on usb shutdown
      Bluetooth: btsdio: Free driver data on SDIO shutdown
      Bluetooth: uart-ldisc: Fix memory leak and remove destruct cb
      Bluetooth: Remove unused hci-destruct cb
      Bluetooth: Correctly acquire module ref
      Bluetooth: Remove HCI-owner field
      Bluetooth: Correctly take hci_dev->dev refcount
      Bluetooth: Remove __hci_dev_put/hold
      Bluetooth: Introduce to_hci_dev()
      Bluetooth: Remove hci_dev->driver_data
      Bluetooth: Introduce to_hci_conn
      Bluetooth: Use proper datatypes in release-callbacks
      Bluetooth: btusb: Remove device lock on release

Fabio Estevam (1):
      Bluetooth: Fix 'enable_hs' type

Gustavo F. Padovan (1):
      Bluetooth: Fix coding style with breaking lines

Hemant Gupta (2):
      Bluetooth: Send correct response to IO Capability Request
      Bluetooth: Fix clearing of debug and linkkey flags

Joe Perches (1):
      Bluetooth: Add logging functions bt_info and bt_err

Johan Hedberg (119):
      Bluetooth: Convert inquiry cache to use standard list types
      Bluetooth: Move Extended Inquiry Response defines to hci.h
      Bluetooth: Add initial mgmt_confirm_name support
      Bluetooth: Return updated name state with hci_inquiry_cache_update
      Bluetooth: Flush inquiry cache when starting mgmt triggered inquiry
      Bluetooth: Rename hdev->inq_cache to hdev->discovery
      Bluetooth: Add discovery state tracking
      Bluetooth: Add name resolving support for mgmt based discovery
      Bluetooth: Remove bogus inline declaration from l2cap_chan_connect
      Bluetooth: Move mgmt related flags from hdev->flags to hdev->dev_flags
      Bluetooth: Fix resetting HCI_MGMT flag
      Bluetooth: Sort to-be-resolved devices by RSSI during discovery
      Bluetooth: Fix clearing persistent flags
      Bluetooth: Rename mgmt connected events to match user space
      Bluetooth: Add eir_len parameter to mgmt_ev_device_found
      Bluetooth: Rename eir_has_complete_name to eir_has_data_type
      Bluetooth: Add missing EIR defines to hci.h
      Bluetooth: Move eir_has_data_field to hci_core.h
      Bluetooth: Merge device class into the EIR data in mgmt_ev_device_found
      Bluetooth: Rename conn->pend to conn->flags
      Bluetooth: Convert hdev->out to a bool type
      Bluetooth: Update device_connected and device_found events to latest API
      Bluetooth: Merge boolean members of struct hci_conn into flags
      Bluetooth: Convert hdev->ssp_mode to a flag
      Bluetooth: Add a convenience function to check for SSP enabled
      Bluetooth: Update mgmt.h to match latest API spec
      Bluetooth: mgmt: Implement Cancel Pair Device command
      Bluetooth: Add missing QUIRK_NO_RESET test to hci_dev_do_close
      Bluetooth: Fix device_found event length for remote name resolving
      Bluetooth: Update and rename mgmt_remove_keys to mgmt_unpair_device
      Bluetooth: Update mgmt_disconnect to match latest API
      Bluetooth: Add address type to user_confirm and user_passkey messages
      Bluetooth: Add address type to Out Of Band mgmt messages
      Bluetooth: Add address type to mgmt blacklist messages
      Bluetooth: Add address type to mgmt_ev_auth_failed
      Bluetooth: Fix mgmt_unpair_device command status
      Bluetooth: Add Device Unpaired mgmt event
      Bluetooth: Implement Read Supported Commands commands for mgmt
      Merge branch 'master' of git://git.kernel.org/.../linville/wireless-next.git
      Bluetooth: Remove unused member from cmd_lookup struct
      Bluetooth: mgmt: Use more consistent error variable names
      Bluetooth: mgmt: Add support for Set Link Security command
      Bluetooth: mgmt: Add support for Set SSP command
      Bluetooth: mgmt: Add address type to link key messages
      Bluetooth: mgmt: Add address type to PIN code messages
      Bluetooth: mgmt: Add address type to confirm name command
      Bluetooth: Add Intel copyright to mgmt files
      Bluetooth: mgmt: Change ordering of cmd_status paramters
      Bluetooth: mgmt: Move status parameters into the cmd_complete header
      Bluetooth: mgmt: Fix Pair Device response status values
      Bluetooth: mgmt: Fix Start Discovery return parameters
      Bluetooth: mgmt: Fix (Un)Block Device return parameters
      Bluetooth: mgmt: Fix OOB command response parameters
      Bluetooth: mgmt: Bump mgmt version
      Bluetooth: Fix hci_connect error return values
      Bluetooth: mgmt: Add address type parameter to Stop Discovery command
      Bluetooth: mgmt: Add address type parameter to Discovering event
      Bluetooth: mgmt: Add basic support for Set High Speed command
      Bluetooth: mgmt: Fix Set SSP check for supported feature
      Bluetooth: mgmt: Clear EIR data when disabling SSP
      Bluetooth: mgmt: Fix powered checks for commands
      Bluetooth: mgmt: Fix set_local_name and set_dev_class powered checks
      Bluetooth: mgmt: Fix set_fast_connectable error return
      Bluetooth: mgmt: Fix pairable setting upon initialization
      Bluetooth: mgmt: Allow connectable/discoverable changes in off state
      Bluetooth: mgmt: Fix Removing discoverable timeout in set_connectable
      Bluetooth: mgmt: Fix current settings values when powered off
      Bluetooth: mgmt: Add convenience function for sending New Settings
      Bluetooth: mgmt: Fix New Settings event for connectable/discoverable
      Bluetooth: Fix clearing of persistent dev_flags
      Bluetooth: mgmt: Fix connectable/discoverable response values
      Bluetooth: mgmt: Make Set Link Security callable while powered off
      Bluetooth: Remove unneeded hci_cc_read_ssp_mode function
      Bluetooth: mgmt: Make Set SSP command callable while powered off
      Bluetooth: mgmt: Fix EIR toggling with SSP
      Bluetooth: mgmt: Fix clearing of hdev->eir
      Bluetooth: Explicitly clear EIR data upon hci_dev setup
      Bluetooth: mgmt: Fix Set SSP supported check
      Bluetooth: mgmt: Implement Set LE command
      Bluetooth: Fix EIR data clearing when powering off
      Bluetooth: mgmt: Fix updating EIR when updating the name
      Bluetooth: Add hdev->short_name for EIR generation
      Bluetooth: Fix read_name updating when HCI_SETUP is not set
      Bluetooth: mgmt: Allow local name changes while powered off
      Bluetooth: mgmt: Fix name_changed event for short name changes
      Bluetooth: mgmt: Fix missing short_name in read_info
      Bluetooth: Fix clearing of dev_class when powering down
      Bluetooth: mgmt: Fix return value for set_class
      Bluetooth: mgmt: Check for HCI_UP in update_eir() and update_class()
      Bluetooth: mgmt: Allow class of device changes while powered off
      Bluetooth: mgmt: Add missing powered checks to commands
      Bluetooth: mgmt: Fix unpair_device responses
      Bluetooth: mgmt: Fix device_found parameters
      Bluetooth: mgmt: Add legacy pairing info to dev_found events
      Bluetooth: mgmt: Fix count parameter in get_connections reply
      Bluetooth: mgmt: Fix update_eir/class with HCI_AUTO_OFF flag set
      Bluetooth: mgmt: Fix return value of add/remove_uuid
      Bluetooth: mgmt: Move service cache setting to a more sensible place
      Bluetooth: mgmt: Fix clear UUIDs response
      Bluetooth: mgmt: Add flags parameter to device_connected
      Bluetooth: mgmt: Track pending class changes
      Bluetooth: mgmt: Fix dev_class related command response timing
      Bluetooth: mgmt: Fix clear_uuids response
      Bluetooth: Fix init request completion with old controllers
      Bluetooth: Use kernel int types instead of ones from stdint.h
      Bluetooth: Don't send unnecessary write_le_enable command
      Bluetooth: Remove redundant read_host_features commands
      Bluetooth: Add missing host features definitions
      Bluetooth: Use LMP_HOST_SSP define instead of magic values
      Bluetooth: mgmt: Add missing hci_dev locking to set_le()
      Bluetooth: Fix init sequence for some CSR based controllers
      Bluetooth: mgmt: Refactor hci_dev lookup for commands
      Bluetooth: mgmt: Initialize HCI_MGMT flag for any command
      Bluetooth: mgmt: Move command handlers into a table
      Bluetooth: mgmt: Add defines for command sizes
      Bluetooth: mgmt: Centralize message length checks
      Bluetooth: Fix clearing of HCI_PENDING_CLASS flag
      Bluetooth: mgmt: Fix command status error code values
      Bluetooth: mgmt: Add new error code for invalid index

Keng-Yu Lin (1):
      Bluetooth: Add AR30XX device ID on Asus laptops

Manoj Iyer (1):
      Bluetooth: btusb: Add vendor specific ID (0a5c 21f3) for BCM20702A0

Marcel Holtmann (25):
      Bluetooth: Split sending for HCI raw and control sockets
      Bluetooth: Remove unneeded bt_cb(skb)->channel variable
      Bluetooth: Limit HCI raw socket options to actual raw sockets
      Bluetooth: Lock socket when reading HCI socket options
      Bluetooth: Add HCI CMSG details only to raw sockets
      Bluetooth: Simplify HCI socket bind handling
      Bluetooth: Fix issue with shared SKB between HCI raw socket and driver
      Bluetooth: Remove HCI notifier handling
      Bluetooth: Add support for HCI monitor channel
      Bluetooth: Restrict access to management interface
      Bluetooth: Set supported settings based on enabled HS and/or LE
      Bluetooth: Always enable management interface
      Bluetooth: Fix parameter list for setting local name
      Bluetooth: Only keep controller up after init if powered on
      Bluetooth: Don't send New Settings event during setup power down
      Bluetooth: Fix two minor style issues in management code
      Bluetooth: Fix two minor style issues in HCI code
      Bluetooth: Enable timestamps for control channel
      Bluetooth: Disabling discoverable with timeout is invalid
      Bluetooth: Fix handling of discoverable setting with timeout
      Bluetooth: Send management event for class of device changes
      Bluetooth: Allow HCI UART reset parameter via flags ioctl
      Bluetooth: Add support for creating HCI UART based AMP controllers
      Bluetooth: Update L2CAP timeout constants to use msecs_to_jiffies
      Bluetooth: Update MGMT and SMP timeout constants to use msecs_to_jiffies

Octavian Purdila (2):
      Bluetooth: silence lockdep warning
      Bluetooth: Fix RFCOMM session reference counting issue

Peter Hurley (1):
      Bluetooth: Fix l2cap conn failures for ssp devices

Szymon Janc (9):
      Bluetooth: Make l2cap_clear_timer return if timer was running or not
      Bluetooth: Set P-bit for SREJ frame only if there are I-frames to ack
      Bluetooth: Clear ack_timer when sending ack
      Bluetooth: Don't send RNR immediately when entering local busy
      Bluetooth: Drop L2CAP chan reference if ERTM ack_timer fired
      Bluetooth: Make l2cap_ertm_data_rcv static
      Bluetooth: Fix possible missing I-Frame acknowledgement
      Bluetooth: Fix double acking I-Frames when sending pending I-Frames
      Bluetooth: Use NULL instead of integer for mgmt_device_connected param

Ulisses Furquim (2):
      Bluetooth: Remove usage of __cancel_delayed_work()
      Bluetooth: Fix possible use after free in delete path

Vinicius Costa Gomes (11):
      Bluetooth: Fix using an absolute timeout on hci_conn_put()
      Bluetooth: Add structures for the new LTK exchange messages
      Bluetooth: Rename smp_key_size to enc_key_size
      Bluetooth: Fix invalid memory access when there's no SMP channel
      Bluetooth: Fix doing some useless casts when receiving MGMT commands
      Bluetooth: Add new structures for handling SMP Long Term Keys
      Bluetooth: Use the updated key structures for handling LTKs
      Bluetooth: Add MGMT handlers for dealing with SMP LTK's
      Bluetooth: Add support for removing LTK's when pairing is removed
      Bluetooth: Clean up structures left unused
      Bluetooth: Add support for notifying userspace of new LTK's

 drivers/bluetooth/ath3k.c          |    1 +
 drivers/bluetooth/bfusb.c          |   23 +-
 drivers/bluetooth/bluecard_cs.c    |   20 +-
 drivers/bluetooth/bpa10x.c         |   35 +-
 drivers/bluetooth/bt3c_cs.c        |   14 +-
 drivers/bluetooth/btmrvl_debugfs.c |   27 +-
 drivers/bluetooth/btmrvl_main.c    |   17 +-
 drivers/bluetooth/btsdio.c         |   23 +-
 drivers/bluetooth/btuart_cs.c      |   14 +-
 drivers/bluetooth/btusb.c          |   47 +-
 drivers/bluetooth/btwilink.c       |   18 +-
 drivers/bluetooth/dtl1_cs.c        |   34 +-
 drivers/bluetooth/hci_ath.c        |    2 +-
 drivers/bluetooth/hci_bcsp.c       |    2 +-
 drivers/bluetooth/hci_h4.c         |    2 +-
 drivers/bluetooth/hci_ldisc.c      |   34 +-
 drivers/bluetooth/hci_ll.c         |    2 +-
 drivers/bluetooth/hci_uart.h       |    2 +
 drivers/bluetooth/hci_vhci.c       |   17 +-
 include/net/bluetooth/bluetooth.h  |   40 +-
 include/net/bluetooth/hci.h        |   72 +-
 include/net/bluetooth/hci_core.h   |  284 +++--
 include/net/bluetooth/hci_mon.h    |   51 +
 include/net/bluetooth/l2cap.h      |   37 +-
 include/net/bluetooth/mgmt.h       |  231 +++--
 include/net/bluetooth/smp.h        |    2 +-
 net/bluetooth/Kconfig              |    1 -
 net/bluetooth/bnep/sock.c          |    6 +-
 net/bluetooth/cmtp/sock.c          |    6 +-
 net/bluetooth/hci_conn.c           |   73 +-
 net/bluetooth/hci_core.c           |  627 +++++++--
 net/bluetooth/hci_event.c          |  607 ++++++---
 net/bluetooth/hci_sock.c           |  470 ++++++--
 net/bluetooth/hci_sysfs.c          |   53 +-
 net/bluetooth/hidp/sock.c          |    6 +-
 net/bluetooth/l2cap_core.c         |  638 +++++----
 net/bluetooth/l2cap_sock.c         |   53 +-
 net/bluetooth/lib.c                |   27 +-
 net/bluetooth/mgmt.c               | 2590 +++++++++++++++++++++++-------------
 net/bluetooth/smp.c                |   92 +-
 40 files changed, 4157 insertions(+), 2143 deletions(-)
 create mode 100644 include/net/bluetooth/hci_mon.h


[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: pull request: bluetooth-next 2012-03-01

[David Miller]

From: Gustavo Padovan <gustavo@padovan.org>
Date: Thu, 1 Mar 2012 23:55:55 -0300

>  - I'm pulling directly to you because due to the coding style issues that
>    happened some days ago. We decided, after talk to John, to pull directly to
>    you, so you can take a look before pull. I personally looked to the whole
>    diff of this pull request and couldn't spot anything that doesn't seem
>    specified in CodingStyle.

For the members of "struct inquiry_cache" the tabbing has been all
messed up by commit 561aafbcb2e3f8fee11d3781f866c7b4c4f93a28.

Instead of all of the member names being nicely aligned using tabs it now
looks like (after all the commits it's now called "struct discovery_state"):

	int type;
	enum {
		DISCOVERY_STOPPED,
		DISCOVERY_STARTING,
		DISCOVERY_FINDING,
		DISCOVERY_RESOLVING,
		DISCOVERY_STOPPING,
	} state;
	struct list_head all;		/* All devices found during inquiry */
	struct list_head unknown;	/* Name state not known */
	struct list_head resolve;	/* Name needs to be resolved */
	__u32		timestamp;

instead of something more reasonable like:

	int			type;
	enum {
		DISCOVERY_STOPPED,
		DISCOVERY_STARTING,
		DISCOVERY_FINDING,
		DISCOVERY_RESOLVING,
		DISCOVERY_STOPPING,
	} state;
	struct list_head	all;		/* All devices found during inquiry */
	struct list_head 	unknown;	/* Name state not known */
	struct list_head 	resolve;	/* Name needs to be resolved */
	__u32			timestamp;

The person editing this had to go out of their way to make things like this
way.

This was only three commits into your tree, therefore I'm not very
optimistic to be honest with you.

Re: pull request: bluetooth-next 2012-03-01

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 01 Mar 2012 22:16:43 -0500 (EST)

> This was only three commits into your tree, therefore I'm not very
> optimistic to be honest with you.

In the very next commit, 3175405b906a85ed2bad21e09c444266e4a05a8e we end
up with:

		mgmt_device_found(hdev, &info->bdaddr, ACL_LINK, 0x00,
					info->dev_class, 0, 1, NULL);

which after all the rest of the commits still looks like:

		mgmt_device_found(hdev, &info->bdaddr, ACL_LINK, 0x00,
					info->dev_class, 0, !name_known, ssp,
					NULL, 0);

You have to make those arguments line up properly:

		mgmt_device_found(hdev, &info->bdaddr, ACL_LINK, 0x00,
				  info->dev_class, 0, !name_known, ssp,
				  NULL, 0);

Re: pull request: bluetooth-next 2012-03-01

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 01 Mar 2012 22:23:16 -0500 (EST)

> From: David Miller <davem@davemloft.net>
> Date: Thu, 01 Mar 2012 22:16:43 -0500 (EST)
> 
>> This was only three commits into your tree, therefore I'm not very
>> optimistic to be honest with you.
> 
> In the very next commit, 3175405b906a85ed2bad21e09c444266e4a05a8e we end
> up with:

And then 2 commits later in ff9ef5787046c3fd20cf9f7ca1cd70260c1eedb9:

+	if (hdev->discovery.state != DISCOVERY_STOPPED) {
+		err = cmd_status(sk, index, MGMT_OP_START_DISCOVERY,
+						MGMT_STATUS_BUSY);
+		goto failed;
+	}

it must be isntead:

		err = cmd_status(sk, index, MGMT_OP_START_DISCOVERY,
				 MGMT_STATUS_BUSY);

And that's pretty much as far as I'm willing to go.

You know what really pisses me off?  This is the one issue I made a huge
stink about last week, arguments to function calls and prototypes lining
up properl.  And it's in ever 2nd or 3rd commit in this pull request.

This means you really didn't check or you really don't care.

Come back when you've audited this whole thing properly and honestly.

Re: pull request: bluetooth-next 2012-03-01

[Joe Perches]

On Thu, 2012-03-01 at 22:26 -0500, David Miller wrote:
> You know what really pisses me off?  This is the one issue I made a huge
> stink about last week, arguments to function calls and prototypes lining
> up properl.  And it's in ever 2nd or 3rd commit in this pull request.
[]
> Come back when you've audited this whole thing properly and honestly.

I submitted a patch to checkpatch for a --strict rule for
this argument alignment.  https://lkml.org/lkml/2012/2/29/644

Are there any other tests you think should be added with
--strict?

Some I think possible are:

o no space after cast
	struct foo *bar = (struct foo *) baz;
sb:
	struct foo *bar = (struct foo *)baz;

o don't start comments with /*$
  (perhaps only if a blank line proceeds the comment)
	/*
	 * multiline comment
	 * ...
	 */
sb:
	/* multiline comment
	 * ...
	 */

o coalesce formats
	pr_<level>("format part 1 "
		   "format part 2\n", ...)
sb:
	pr_<level>("format part 1 format part 2\n",
		   ...);

o symmetric brace standardization
	if (foo) {
		bar;
		baz;
	} else
		bar;
sb:
	if (foo) {
		bar;
		baz;
	} else {
		bar;
	}

Any others?

[PATCH] checkpatch: Add --strict tests for braces, comments and casts

[Joe Perches]

Add some more subjective --strict tests.

Add a test for block comments that start with a blank
line followed only by a line with just the comment block
initiator.  Prefer a blank line followed by /* comment...

Add a test for unnecessary spaces after a cast.

Add a test for symmetric uses of braces in if/else blocks.
If one branch needs braces, then all branches should use braces.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |   40 ++++++++++++++++++++++++++++++++--------
 1 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 308e401..581a14c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1849,6 +1849,17 @@ sub process {
 			}
 		}
 
+		if ($line =~ /^\+.*\*[ \t]*\)[ \t]+/) {
+			CHK("SPACING",
+			    "No space is necessary after a cast\n" . $hereprev);
+		}
+
+		if ($rawline =~ /^\+[ \t]*\/\*[ \t]*$/ &&
+		    $prevrawline =~ /^\+[ \t]*$/) {
+			CHK("BLOCK_COMMENT_STYLE",
+			    "Don't begin block comments with only a /* line, use /* comment...\n" . $hereprev);
+		}
+
 # check for spaces at the beginning of a line.
 # Exceptions:
 #  1) within comments
@@ -2954,7 +2965,8 @@ sub process {
 			#print "chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
 			#print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n";
 			if ($#chunks > 0 && $level == 0) {
-				my $allowed = 0;
+				my @allowed = ();
+				my $allow = 0;
 				my $seen = 0;
 				my $herectx = $here . "\n";
 				my $ln = $linenr - 1;
@@ -2965,6 +2977,7 @@ sub process {
 					my ($whitespace) = ($cond =~ /^((?:\s*\n[+-])*\s*)/s);
 					my $offset = statement_rawlines($whitespace) - 1;
 
+					$allowed[$allow] = 0;
 					#print "COND<$cond> whitespace<$whitespace> offset<$offset>\n";
 
 					# We have looked at and allowed this specific line.
@@ -2977,23 +2990,34 @@ sub process {
 
 					$seen++ if ($block =~ /^\s*{/);
 
-					#print "cond<$cond> block<$block> allowed<$allowed>\n";
+					#print "cond<$cond> block<$block> allowed<$allowed[$allow]>\n";
 					if (statement_lines($cond) > 1) {
 						#print "APW: ALLOWED: cond<$cond>\n";
-						$allowed = 1;
+						$allowed[$allow] = 1;
 					}
 					if ($block =~/\b(?:if|for|while)\b/) {
 						#print "APW: ALLOWED: block<$block>\n";
-						$allowed = 1;
+						$allowed[$allow] = 1;
 					}
 					if (statement_block_size($block) > 1) {
 						#print "APW: ALLOWED: lines block<$block>\n";
-						$allowed = 1;
+						$allowed[$allow] = 1;
 					}
+					$allow++;
 				}
-				if ($seen && !$allowed) {
-					WARN("BRACES",
-					     "braces {} are not necessary for any arm of this statement\n" . $herectx);
+				if ($seen) {
+					my $sum_allowed = 0;
+					foreach (@allowed) {
+						$sum_allowed += $_;
+					}
+					if ($sum_allowed == 0) {
+						WARN("BRACES",
+						     "braces {} are not necessary for any arm of this statement\n" . $herectx);
+					} elsif ($sum_allowed != $allow &&
+						 $seen != $allow) {
+						CHK("BRACES",
+						    "braces {} should be used on all arms of this statement\n" . $herectx);
+					}
 				}
 			}
 		}
-- 
1.7.8.111.gad25c.dirty

[PATCH] checkpatch: Add --strict test for strings split across multiple lines

[Joe Perches]

Strings split across multiple lines are commonly used
as formats.  These uncoalesced formats are hard to grep
and are relatively error prone.

Suggest coalescing split strings when using --strict.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 581a14c..5d0b46c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1860,6 +1860,12 @@ sub process {
 			    "Don't begin block comments with only a /* line, use /* comment...\n" . $hereprev);
 		}
 
+		if ($prevline =~ /^\+.*"[ \t]*$/ &&
+		    $line =~ /^\+[ \t]*"/) {
+			CHK("COALESCE_STRING",
+			    "Coalesced strings are easier to grep and less error prone\n" . $hereprev);
+		}
+
 # check for spaces at the beginning of a line.
 # Exceptions:
 #  1) within comments
-- 
1.7.8.111.gad25c.dirty

[PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Joe Perches]

Suggest the shorter pr_<level> instead of printk(KERN_<LEVEL>.

Prefer to use pr_<level> over bare printks.
Prefer to use pr_warn over pr_warning.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 04eb9eb..ac2bbea 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2379,6 +2379,19 @@ sub process {
 			}
 		}
 
+		if ($line =~ /\bprintk\s*\(\s*KERN_([A-Z]+)/) {
+			my $orig = $1;
+			my $level = lc($orig);
+			$level = "warn" if ($level eq "warning");
+			WARN("PREFER_PR_LEVEL",
+			     "Prefer pr_$level(... to printk(KERN_$1, ...\n" . $herecurr);
+		}
+
+		if ($line =~ /\bpr_warning\s*\(/) {
+			WARN("PREFER_PR_LEVEL",
+			     "Prefer pr_warn(... to pr_warning(...\n" . $herecurr);
+		}
+
 # function brace can't be on same line, except for #defines of do while,
 # or if closed on same line
 		if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Mon, Mar 12, 2012 at 11:23:03PM -0700, Joe Perches wrote:
> Suggest the shorter pr_<level> instead of printk(KERN_<LEVEL>.
> 
> Prefer to use pr_<level> over bare printks.
> Prefer to use pr_warn over pr_warning.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Is this even worth a warning?   I don't think so....

   	     	     		  	- Ted

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Andrew Morton]

On Tue, 13 Mar 2012 08:05:14 -0400
"Ted Ts'o" <tytso@mit.edu> wrote:

> On Mon, Mar 12, 2012 at 11:23:03PM -0700, Joe Perches wrote:
> > Suggest the shorter pr_<level> instead of printk(KERN_<LEVEL>.
> > 
> > Prefer to use pr_<level> over bare printks.
> > Prefer to use pr_warn over pr_warning.
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> Is this even worth a warning?   I don't think so....

mm...  probably.  It's not a thing I ever bother mentioning in review,
but I guess pr_foo() is a bit denser, and doing the same thing in two
different ways is always an irritant.

I'll put the patch in my tree for a while and see how irritating I find
it ;)

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Tue, Mar 13, 2012 at 02:55:17PM -0700, Andrew Morton wrote:
> 
> mm...  probably.  It's not a thing I ever bother mentioning in review,
> but I guess pr_foo() is a bit denser, and doing the same thing in two
> different ways is always an irritant.

Sure but if a particular kernel file or subsystem is _not_ using
pr_foo(), having a checkpatch which tries to force everyone to use
pr_foo() is going to be really annoying to me as a maintainer...

	    	     	       		- Ted

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Andrew Morton]

On Tue, 13 Mar 2012 18:01:44 -0400
"Ted Ts'o" <tytso@mit.edu> wrote:

> On Tue, Mar 13, 2012 at 02:55:17PM -0700, Andrew Morton wrote:
> > 
> > mm...  probably.  It's not a thing I ever bother mentioning in review,
> > but I guess pr_foo() is a bit denser, and doing the same thing in two
> > different ways is always an irritant.
> 
> Sure but if a particular kernel file or subsystem is _not_ using
> pr_foo(), having a checkpatch which tries to force everyone to use
> pr_foo() is going to be really annoying to me as a maintainer...
> 

Yes, that's what I fear.  That's why I'm testing it...

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Tue, Mar 13, 2012 at 03:03:16PM -0700, Andrew Morton wrote:
> > Sure but if a particular kernel file or subsystem is _not_ using
> > pr_foo(), having a checkpatch which tries to force everyone to use
> > pr_foo() is going to be really annoying to me as a maintainer...
> 
> Yes, that's what I fear.  That's why I'm testing it...

Perhaps the answer is a ".checkpatch_ignore" file that can be
maintained by maintainers in each directory to suppress specific
checkpatch warnings that the maintainers don't agree with.  That way
we don't have to worry about checkpatch maintainers try to impose
their pet programming style preferences on maintainers who don't want
to get random trivial style-fixing patches from well-meaning kernel
newbies...

						- Ted

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Joe Perches]

On Tue, 2012-03-13 at 20:31 -0400, Ted Ts'o wrote:
> On Tue, Mar 13, 2012 at 03:03:16PM -0700, Andrew Morton wrote:
> > > Sure but if a particular kernel file or subsystem is _not_ using
> > > pr_foo(), having a checkpatch which tries to force everyone to use
> > > pr_foo() is going to be really annoying to me as a maintainer...
> > 
> > Yes, that's what I fear.  That's why I'm testing it...
> 
> Perhaps the answer is a ".checkpatch_ignore" file that can be
> maintained by maintainers in each directory to suppress specific
> checkpatch warnings that the maintainers don't agree with.  That way
> we don't have to worry about checkpatch maintainers try to impose
> their pet programming style preferences on maintainers who don't want
> to get random trivial style-fixing patches from well-meaning kernel
> newbies...

Or add an I: line to MAINTAINERS

though perhaps it's better to agree on a style.

I did send a few fixes and a style consolidation patch
for ext4 with no reply awhile ago:

https://lkml.org/lkml/2011/8/2/41

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Tue, Mar 13, 2012 at 05:47:06PM -0700, Joe Perches wrote:
> Or add an I: line to MAINTAINERS
> 
> though perhaps it's better to agree on a style.
> 
> I did send a few fixes and a style consolidation patch
> for ext4 with no reply awhile ago:
> 
> https://lkml.org/lkml/2011/8/2/41

You combined a huge number of changes into a single patch, and as far
as I was concerned the value it added Just Wasn't Worth It.  It adds
noise which causes other patches, which add real value, not to apply
cleanly.

I routinely ignore such patches because I have a limited amount of
time, and as far as I'm concerned they mainly make my life harder.

Looking more closely, there are a few changes in there that I'd
accept, but it was burried amongst so much other junk that if it's all
or nothing, it would be nothing.  Funny that someone who is an expert
in style things neglected something **far** more important ---
segregate logical changes in separate commits; don't collapse
everything into a single patch, which makes it hard to review.

      	     	    		  - Ted

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Joe Perches]

On Tue, 2012-03-13 at 21:07 -0400, Ted Ts'o wrote:
> On Tue, Mar 13, 2012 at 05:47:06PM -0700, Joe Perches wrote:
> > Or add an I: line to MAINTAINERS
> > 
> > though perhaps it's better to agree on a style.
> > 
> > I did send a few fixes and a style consolidation patch
> > for ext4 with no reply awhile ago:
> > 
> > https://lkml.org/lkml/2011/8/2/41
> 
> You combined a huge number of changes into a single patch, and as far
> as I was concerned the value it added Just Wasn't Worth It.  It adds
> noise which causes other patches, which add real value, not to apply
> cleanly.
> 
> I routinely ignore such patches because I have a limited amount of
> time, and as far as I'm concerned they mainly make my life harder.
> 
> Looking more closely, there are a few changes in there that I'd
> accept, but it was burried amongst so much other junk that if it's all
> or nothing, it would be nothing.  Funny that someone who is an expert
> in style things neglected something **far** more important ---
> segregate logical changes in separate commits; don't collapse
> everything into a single patch, which makes it hard to review.

The patch was all apiece, every bit associated to logging output.
It was bundled to make it easier to apply.

You call it junk, I call it cleanups.

Consistent style in a largish project has advantages.
You can ignore them if you choose.

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Tue, Mar 13, 2012 at 06:17:11PM -0700, Joe Perches wrote:
> The patch was all apiece, every bit associated to logging output.
> It was bundled to make it easier to apply.
> 
> You call it junk, I call it cleanups.

Changing stuff to pr_foo is ***NOISE***.  It adds no value, and it
makes it my life much, MUCH harder.

Andrew, please, can we NACK this pr_foo checkpatch.pl change?

							- Ted

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Joe Perches]

On Tue, 2012-03-13 at 22:19 -0400, Ted Ts'o wrote:
> On Tue, Mar 13, 2012 at 06:17:11PM -0700, Joe Perches wrote:
> > The patch was all apiece, every bit associated to logging output.
> > It was bundled to make it easier to apply.
> > 
> > You call it junk, I call it cleanups.
> 
> Changing stuff to pr_foo is ***NOISE***.  It adds no value, and it
> makes it my life much, MUCH harder.

I dispute that it add no value.

pr_<foo> adds value to the dmesg output because
it can be consistently prefixed via pr_fmt.

Right now many fs ext4 messages are somewhat opaque
without any reference to what kernel subsystem produced
the message.

For instance:

fs/ext4/ialloc.c:		printk(KERN_DEBUG "group %lu: stored = %d, counted = %lu\n",

This is a somewhat senseless output in dmesg without
any linkage to ext4.

Using pr_fmt and pr_debug as I sent a patch to do
instead emits in dmesg:

EXT4-fs: group: etc...

Using subsystem prefixes makes it easy and consistent to
grep dmesg.

cheers, Joe

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Tue, Mar 13, 2012 at 07:31:51PM -0700, Joe Perches wrote:
> Right now many fs ext4 messages are somewhat opaque
> without any reference to what kernel subsystem produced
> the message.
> 
> For instance:
> 
> fs/ext4/ialloc.c:		printk(KERN_DEBUG "group %lu: stored = %d, counted = %lu\n",
> 
> This is a somewhat senseless output in dmesg without
> any linkage to ext4.
> 
> Using pr_fmt and pr_debug as I sent a patch to do
> instead emits in dmesg:
> 
> EXT4-fs: group: etc...
> 
> Using subsystem prefixes makes it easy and consistent to
> grep dmesg.

That's a debug message which is never by anyone other than ext4
developers.  Your patch also hacked the Makefile to enable it by
default, which also enabled some performance degrading code paths
(again, only enabled by developers who manually drop the #define in a
header file when they are trying to figure out some obscure failure
during the development process).  This is why I don't like people who
are wanking around in code they don't understand just to fix style
fixes, in the mistaken belief that it adds value.

						- Ted

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Joe Perches]

On Tue, 2012-03-13 at 22:41 -0400, Ted Ts'o wrote:
> On Tue, Mar 13, 2012 at 07:31:51PM -0700, Joe Perches wrote:
> > Right now many fs ext4 messages are somewhat opaque
> > without any reference to what kernel subsystem produced
> > the message.
> > 
> > For instance:
> > 
> > fs/ext4/ialloc.c:		printk(KERN_DEBUG "group %lu: stored = %d, counted = %lu\n",
> > 
> > This is a somewhat senseless output in dmesg without
> > any linkage to ext4.
> > 
> > Using pr_fmt and pr_debug as I sent a patch to do
> > instead emits in dmesg:
> > 
> > EXT4-fs: group: etc...
> > 
> > Using subsystem prefixes makes it easy and consistent to
> > grep dmesg.
> 
> That's a debug message which is never by anyone other than ext4
> developers.  Your patch also hacked the Makefile to enable it by
> default,

It's just an example and no it didn't.
That output is still in an #ifdef EXT4FS_DEBUG
block and is unchanged.

What I did was #define DEBUG so pr_debug
(and so dynamic_debug if desired as well)
emits output to dmesg.

+ccflags-$(CONFIG_EXT4_FS) := -DDEBUG

ext4 doesn't currently use any #ifdef DEBUG blocks.

> which also enabled some performance degrading code paths
> (again, only enabled by developers who manually drop the #define in a
> header file when they are trying to figure out some obscure failure
> during the development process).  This is why I don't like people who
> are wanking around in code they don't understand just to fix style
> fixes, in the mistaken belief that it adds value.

cheers, Joe

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Tue, Mar 13, 2012 at 08:01:14PM -0700, Joe Perches wrote:
> > That's a debug message which is never by anyone other than ext4
> > developers.  Your patch also hacked the Makefile to enable it by
> > default,
> 
> It's just an example and no it didn't.
> That output is still in an #ifdef EXT4FS_DEBUG
> block and is unchanged.

I looked at your patch, and nearly all of them were in debug code.  I
know, because in practice the messages that come up with any kind of
regularity are all properly prefixed.

Look, the reason why I care about patch noise is because I have a huge
patch backlog.  Take a look at this:

      http://patchwork.ozlabs.org/project/linux-ext4/list/

Very few other people review patches, and even patches that survive
review, I've found problems that could potentially lead to data loss
or system instability.  This is not like your average device driver,
where if the machine panics once a week, "oh well", and you reboot.
Linus would get very cranky if he lost data as a result of a bad patch
slipping through.  Hence, patches don't go in until after significant
review and testing.

As a result, #1, patches that are don't add value, and are large,
simply won't get applied.  Period.  Avoiding the downside of lots of
people losing data is ****far**** more than your OCD wanting me to use
pr_warn(...) instead of printk(KERN_WARN, ...).

If you want your style patches to go in, break them into smaller
chunks, or I *will* ignore them.

#2, patches that don't add substantial value, and make it harder for
me to review and apply patches from my very substantial patch
backlog, I consider as adding ***substantial*** negative value.

That being said, I use checkpatch.pl as an initial screen, but it's
already been the case that there are warnings that I consider pure
noise, and I really, REALLY, rather not add more noise to
checkpatch.pl.  There is no group consensus about things like pr_foo
--- not as far as I've seen --- and to impose it from on high by some
patch wankers making changes to checkpatch.pl very much offends me.

      	      	     	     		      	   - Ted

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Joe Perches]

On Wed, 2012-03-14 at 08:34 -0400, Ted Ts'o wrote:
> On Tue, Mar 13, 2012 at 08:01:14PM -0700, Joe Perches wrote:
> > > That's a debug message which is never by anyone other than ext4
> > > developers.  Your patch also hacked the Makefile to enable it by
> > > default,
> > 
> > It's just an example and no it didn't.
> > That output is still in an #ifdef EXT4FS_DEBUG
> > block and is unchanged.
> 
> I looked at your patch, and nearly all of them were in debug code.  I
> know, because in practice the messages that come up with any kind of
> regularity are all properly prefixed.

Not really, some are prefixed with EXT4-fs, others EXT4,
some with colons, some without, some with no prefix,
some with function names only.

The idea is to be consistent and allow a mechanical
comprehensive dmesg grep with "EXT4-fs:" or some
other appropriate subsystem name.

$ grep -rP --include=*.[ch] "\bprintk\s*\(\s*KERN_[A-Z]+\s*\"[^\":]*" fs/ext4/

>       http://patchwork.ozlabs.org/project/linux-ext4/list/

Patchwork queues are pretty useless when patches entered
do not have their status updated for long periods.

The patch I sent in August 2011 shows "new" rather than
have an appropriate status.

There are patches in that queue from 2008 marked as "new"
that will never be applied or looked at again.

If you actually use patchwork, though it seems you don't,
I think you should just mark every patch that's new as
rejected and start over.

> Very few other people review patches, and even patches that survive
> review, I've found problems that could potentially lead to data loss
> or system instability.  This is not like your average device driver,
> where if the machine panics once a week, "oh well", and you reboot.
> Linus would get very cranky if he lost data as a result of a bad patch
> slipping through.  Hence, patches don't go in until after significant
> review and testing.
> 
> As a result, #1, patches that are don't add value, and are large,
> simply won't get applied.  Period.  Avoiding the downside of lots of
> people losing data is ****far**** more than your OCD wanting me to use
> pr_warn(...) instead of printk(KERN_WARN, ...).

I believe it's less OCD than you do.

Using a facility to prefix dmesg output consistently
per subsystem adds value in my opinion.

> If you want your style patches to go in, break them into smaller
> chunks, or I *will* ignore them.

OK, I'll resubmit it as micropatches.

cheers, Joe

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Ted Ts'o]

On Wed, Mar 14, 2012 at 06:05:36AM -0700, Joe Perches wrote:
> Patchwork queues are pretty useless when patches entered
> do not have their status updated for long periods.
> 
> The patch I sent in August 2011 shows "new" rather than
> have an appropriate status.
> 
> If you actually use patchwork, though it seems you don't,
> I think you should just mark every patch that's new as
> rejected and start over.

I use it, but not in the way you think I should be using it.  Your not
getting to your will on other kernel developers is what this thread is
all all about, ultimately.

I don't get to work on ext4 full time, and so every minute I put on it
has to not a be a waste of time.  This includes updating status
messages for patches that aren't obviously not applicable, or
superceded, but rather something that I might get to look at later.

	    	       		      	      - Ted

Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>

[Joe Perches]

On Wed, 2012-03-14 at 08:34 -0400, Ted Ts'o wrote:
> Look, the reason why I care about patch noise is because I have a huge
> patch backlog.  Take a look at this:
>       http://patchwork.ozlabs.org/project/linux-ext4/list/

On Wed, 2012-03-14 at 09:45 -0400, Ted Ts'o wrote:
> On Wed, Mar 14, 2012 at 06:05:36AM -0700, Joe Perches wrote:
> > If you actually use patchwork, though it seems you don't,
> > I think you should just mark every patch that's new as
> > rejected and start over.
> I use it, but not in the way you think I should be using it.

Given the information in the link you sent,
it's not possible for anyone but you to
assess your patch backlog.

cheers, Joe

Re: pull request: bluetooth-next 2012-03-01

[Gustavo Padovan]

Hi Dave,

* David Miller <davem@davemloft.net> [2012-03-01 22:26:04 -0500]:

> From: David Miller <davem@davemloft.net>
> Date: Thu, 01 Mar 2012 22:23:16 -0500 (EST)
> 
> > From: David Miller <davem@davemloft.net>
> > Date: Thu, 01 Mar 2012 22:16:43 -0500 (EST)
> > 
> >> This was only three commits into your tree, therefore I'm not very
> >> optimistic to be honest with you.
> > 
> > In the very next commit, 3175405b906a85ed2bad21e09c444266e4a05a8e we end
> > up with:
> 
> And then 2 commits later in ff9ef5787046c3fd20cf9f7ca1cd70260c1eedb9:
> 
> +	if (hdev->discovery.state != DISCOVERY_STOPPED) {
> +		err = cmd_status(sk, index, MGMT_OP_START_DISCOVERY,
> +						MGMT_STATUS_BUSY);
> +		goto failed;
> +	}
> 
> it must be isntead:
> 
> 		err = cmd_status(sk, index, MGMT_OP_START_DISCOVERY,
> 				 MGMT_STATUS_BUSY);
> 
> And that's pretty much as far as I'm willing to go.
> 
> You know what really pisses me off?  This is the one issue I made a huge
> stink about last week, arguments to function calls and prototypes lining
> up properl.  And it's in ever 2nd or 3rd commit in this pull request.

The styles in these patches is the same style we've been using in Bluetooth
code for more than 10 years and no one ever complained to us and we are the
only subsystem under Net doing something like this. I could find some examples
over the net code in a quick look.

I can see only two options here, leave this as is since this is coding style
we've been using since beginning or change the whole Bluetooth subsystem to
work like you said. In other words, are you telling me to make a patch to fix
this issue in the whole Bluetooth subsystem and not only in this pull request
diff? Otherwise we are putting the Bluetooth code in a inconsistent state with
two different coding styles. Is that what you want, an inconsistent state?

I'd be ok with changing the whole subsystem's style if you want.

	Gustavo

Re: pull request: bluetooth-next 2012-03-01

[David Miller]

From: Gustavo Padovan <padovan@profusion.mobi>
Date: Fri, 2 Mar 2012 13:37:16 -0300

> are you telling me to make a patch to fix this issue in the whole
> Bluetooth subsystem

I'm saying get it right in new code changes.

Re: pull request: bluetooth-next 2012-03-01

[Gustavo Padovan]

Hi Dave,

* David Miller <davem@davemloft.net> [2012-03-02 16:15:01 -0500]:

> From: Gustavo Padovan <padovan@profusion.mobi>
> Date: Fri, 2 Mar 2012 13:37:16 -0300
> 
> > are you telling me to make a patch to fix this issue in the whole
> > Bluetooth subsystem
> 
> I'm saying get it right in new code changes.

This is something we can't do. Changing only the new code will put the
Bluetooth subsystem in a inconsistent coding style with different styles
through the subsystem.

Also I talked with Marcel and Johan about this, we think the most important
here is to have this code in the 3.4 release and taking in account that this
coding style is the same we've been using for the last 11 years we think this
style discussion is not really important compared to the need to have our
changes in 3.4.

So our current idea here is keep this pull request and ask you to pull it as
is. If it happens that this doesn't get pulled into mainline for 3.4 then we
will have to deal with a worst problem that is keep our tree out of mainline.

	Gustavo


The following changes since commit b4017c5368f992fb8fb3a2545a0977082c6664e4:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2012-03-01 17:57:40 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-next.git master

Andre Guedes (24):
      Bluetooth: Add 'eir_len' param to mgmt_device_found()
      Bluetooth: Report LE devices
      Bluetooth: Use GFP_KERNEL in hci_conn_add()
      Bluetooth: Use GFP_KERNEL in hci_chan_create()
      Bluetooth: Fix potential deadlock
      Bluetooth: Remove unneeded locking
      Bluetooth: Use GFP_KERNEL in hci_add_adv_entry()
      Bluetooth: LE scan should send Discovering events
      Bluetooth: Minor code refactoring
      Bluetooth: Add hci_do_le_scan()
      Bluetooth: Add hci_le_scan()
      Bluetooth: MGMT start discovery LE-Only support
      Bluetooth: Fix indentation
      Bluetooth: Add BT_DBG to mgmt_discovering()
      Bluetooth: Fix discovery state machine
      Bluetooth: Fix event sending with DISCOVERY_STOPPED state
      Bluetooth: Prepare start_discovery
      Bluetooth: Track discovery type
      Bluetooth: Merge INQUIRY and LE_SCAN discovery states
      Bluetooth: Interleaved discovery support
      Bluetooth: Set DISCOVERY_STOPPED if controller resets
      Bluetooth: Change interleaved discovery behavior
      Bluetooth: Fix Kconfig help description
      Bluetooth: Check capabilities in BR/EDR and LE-Only discovery

Andrei Emeltchenko (30):
      Bluetooth: Process num completed data blocks event
      Bluetooth: Remove magic number from ACL TO
      Bluetooth: Use chan instead of sk
      Bluetooth: Change sk to l2cap_chan
      Bluetooth: trivial: space correction
      Bluetooth: Add alloc_skb chan operator
      Bluetooth: Use list _safe deleting from conn_hash_list
      Bluetooth: Use list _safe deleting from conn chan_list
      Bluetooth: Recalculate sched HCI blk/pkt flow ctrl
      Bluetooth: Helper removes duplicated code
      Bluetooth: Change chan_ready param from sk to chan
      Bluetooth: Clean up l2cap_chan_add
      Bluetooth: Remove unneeded sk variable
      Bluetooth: Do not dereference zero sk
      Bluetooth: Move scope of state_to_string
      Bluetooth: Use symbolic names for state in debug
      Bluetooth: Prefix hex numbers with object name
      Bluetooth: trivial: Fix long line
      Bluetooth: Revert to mutexes from RCU list
      Bluetooth: Add l2cap_chan_lock
      Bluetooth: Add locked and unlocked state_change
      Bluetooth: Add socket error function
      Bluetooth: Fix coding style issues in mgmt code
      Bluetooth: Add unlocked __l2cap_chan_add function
      Bluetooth: Change sk lock to chan lock in L2CAP core
      Bluetooth: Remove socket lock check
      Bluetooth: Fix init request completion with AMP controllers
      Bluetooth: Fix double locking in LE and conless chan
      Bluetooth: Remove duplicated code in l2cap conn req
      Bluetooth: Save remote L2CAP fixed channel mask

Andrzej Kaczmarek (2):
      Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket
      Bluetooth: l2cap_set_timer needs jiffies as timeout value

Dan Carpenter (2):
      Bluetooth: use kfree_skb() instead of kfree()
      Bluetooth: change min_t() cast in hci_reassembly()

Daniel Wagner (1):
      Bluetooth: Don't mark non xfer isoc endpoint URBs with URB_ISO_ASAP

David Herrmann (28):
      Bluetooth: hci-uart-ll: Use GFP_ATOMIC in open()
      Bluetooth: hci-uart-h4: Use GFP_ATOMIC in open()
      Bluetooth: hci-uart-bcsp: Use GFP_ATOMIC in open()
      Bluetooth: hci-uart-ath: Use GFP_ATOMIC in open()
      Bluetooth: dtl1: Fix memleak in probe()
      Bluetooth: Make hci-destruct callback optional
      Bluetooth: bluecard-cs: Remove empty destruct cb
      Bluetooth: bt3c-cs: Remove empty destruct cb
      Bluetooth: btmrvl: Remove empty destruct cb
      Bluetooth: btuart-cs: Remove empty destruct cb
      Bluetooth: btwilink: Remove empty destruct cb
      Bluetooth: dtl1-cs: Remove empty destruct cb
      Bluetooth: vhci: Free driver_data on file release
      Bluetooth: bfusb: Free driver_data on USB shutdown
      Bluetooth: btusb: Free driver data on USB shutdown
      Bluetooth: bpa10x: Free private driver data on usb shutdown
      Bluetooth: btsdio: Free driver data on SDIO shutdown
      Bluetooth: uart-ldisc: Fix memory leak and remove destruct cb
      Bluetooth: Remove unused hci-destruct cb
      Bluetooth: Correctly acquire module ref
      Bluetooth: Remove HCI-owner field
      Bluetooth: Correctly take hci_dev->dev refcount
      Bluetooth: Remove __hci_dev_put/hold
      Bluetooth: Introduce to_hci_dev()
      Bluetooth: Remove hci_dev->driver_data
      Bluetooth: Introduce to_hci_conn
      Bluetooth: Use proper datatypes in release-callbacks
      Bluetooth: btusb: Remove device lock on release

Fabio Estevam (1):
      Bluetooth: Fix 'enable_hs' type

Gustavo F. Padovan (1):
      Bluetooth: Fix coding style with breaking lines

Hemant Gupta (2):
      Bluetooth: Send correct response to IO Capability Request
      Bluetooth: Fix clearing of debug and linkkey flags

Joe Perches (1):
      Bluetooth: Add logging functions bt_info and bt_err

Johan Hedberg (119):
      Bluetooth: Convert inquiry cache to use standard list types
      Bluetooth: Move Extended Inquiry Response defines to hci.h
      Bluetooth: Add initial mgmt_confirm_name support
      Bluetooth: Return updated name state with hci_inquiry_cache_update
      Bluetooth: Flush inquiry cache when starting mgmt triggered inquiry
      Bluetooth: Rename hdev->inq_cache to hdev->discovery
      Bluetooth: Add discovery state tracking
      Bluetooth: Add name resolving support for mgmt based discovery
      Bluetooth: Remove bogus inline declaration from l2cap_chan_connect
      Bluetooth: Move mgmt related flags from hdev->flags to hdev->dev_flags
      Bluetooth: Fix resetting HCI_MGMT flag
      Bluetooth: Sort to-be-resolved devices by RSSI during discovery
      Bluetooth: Fix clearing persistent flags
      Bluetooth: Rename mgmt connected events to match user space
      Bluetooth: Add eir_len parameter to mgmt_ev_device_found
      Bluetooth: Rename eir_has_complete_name to eir_has_data_type
      Bluetooth: Add missing EIR defines to hci.h
      Bluetooth: Move eir_has_data_field to hci_core.h
      Bluetooth: Merge device class into the EIR data in mgmt_ev_device_found
      Bluetooth: Rename conn->pend to conn->flags
      Bluetooth: Convert hdev->out to a bool type
      Bluetooth: Update device_connected and device_found events to latest API
      Bluetooth: Merge boolean members of struct hci_conn into flags
      Bluetooth: Convert hdev->ssp_mode to a flag
      Bluetooth: Add a convenience function to check for SSP enabled
      Bluetooth: Update mgmt.h to match latest API spec
      Bluetooth: mgmt: Implement Cancel Pair Device command
      Bluetooth: Add missing QUIRK_NO_RESET test to hci_dev_do_close
      Bluetooth: Fix device_found event length for remote name resolving
      Bluetooth: Update and rename mgmt_remove_keys to mgmt_unpair_device
      Bluetooth: Update mgmt_disconnect to match latest API
      Bluetooth: Add address type to user_confirm and user_passkey messages
      Bluetooth: Add address type to Out Of Band mgmt messages
      Bluetooth: Add address type to mgmt blacklist messages
      Bluetooth: Add address type to mgmt_ev_auth_failed
      Bluetooth: Fix mgmt_unpair_device command status
      Bluetooth: Add Device Unpaired mgmt event
      Bluetooth: Implement Read Supported Commands commands for mgmt
      Merge branch 'master' of git://git.kernel.org/.../linville/wireless-next.git
      Bluetooth: Remove unused member from cmd_lookup struct
      Bluetooth: mgmt: Use more consistent error variable names
      Bluetooth: mgmt: Add support for Set Link Security command
      Bluetooth: mgmt: Add support for Set SSP command
      Bluetooth: mgmt: Add address type to link key messages
      Bluetooth: mgmt: Add address type to PIN code messages
      Bluetooth: mgmt: Add address type to confirm name command
      Bluetooth: Add Intel copyright to mgmt files
      Bluetooth: mgmt: Change ordering of cmd_status paramters
      Bluetooth: mgmt: Move status parameters into the cmd_complete header
      Bluetooth: mgmt: Fix Pair Device response status values
      Bluetooth: mgmt: Fix Start Discovery return parameters
      Bluetooth: mgmt: Fix (Un)Block Device return parameters
      Bluetooth: mgmt: Fix OOB command response parameters
      Bluetooth: mgmt: Bump mgmt version
      Bluetooth: Fix hci_connect error return values
      Bluetooth: mgmt: Add address type parameter to Stop Discovery command
      Bluetooth: mgmt: Add address type parameter to Discovering event
      Bluetooth: mgmt: Add basic support for Set High Speed command
      Bluetooth: mgmt: Fix Set SSP check for supported feature
      Bluetooth: mgmt: Clear EIR data when disabling SSP
      Bluetooth: mgmt: Fix powered checks for commands
      Bluetooth: mgmt: Fix set_local_name and set_dev_class powered checks
      Bluetooth: mgmt: Fix set_fast_connectable error return
      Bluetooth: mgmt: Fix pairable setting upon initialization
      Bluetooth: mgmt: Allow connectable/discoverable changes in off state
      Bluetooth: mgmt: Fix Removing discoverable timeout in set_connectable
      Bluetooth: mgmt: Fix current settings values when powered off
      Bluetooth: mgmt: Add convenience function for sending New Settings
      Bluetooth: mgmt: Fix New Settings event for connectable/discoverable
      Bluetooth: Fix clearing of persistent dev_flags
      Bluetooth: mgmt: Fix connectable/discoverable response values
      Bluetooth: mgmt: Make Set Link Security callable while powered off
      Bluetooth: Remove unneeded hci_cc_read_ssp_mode function
      Bluetooth: mgmt: Make Set SSP command callable while powered off
      Bluetooth: mgmt: Fix EIR toggling with SSP
      Bluetooth: mgmt: Fix clearing of hdev->eir
      Bluetooth: Explicitly clear EIR data upon hci_dev setup
      Bluetooth: mgmt: Fix Set SSP supported check
      Bluetooth: mgmt: Implement Set LE command
      Bluetooth: Fix EIR data clearing when powering off
      Bluetooth: mgmt: Fix updating EIR when updating the name
      Bluetooth: Add hdev->short_name for EIR generation
      Bluetooth: Fix read_name updating when HCI_SETUP is not set
      Bluetooth: mgmt: Allow local name changes while powered off
      Bluetooth: mgmt: Fix name_changed event for short name changes
      Bluetooth: mgmt: Fix missing short_name in read_info
      Bluetooth: Fix clearing of dev_class when powering down
      Bluetooth: mgmt: Fix return value for set_class
      Bluetooth: mgmt: Check for HCI_UP in update_eir() and update_class()
      Bluetooth: mgmt: Allow class of device changes while powered off
      Bluetooth: mgmt: Add missing powered checks to commands
      Bluetooth: mgmt: Fix unpair_device responses
      Bluetooth: mgmt: Fix device_found parameters
      Bluetooth: mgmt: Add legacy pairing info to dev_found events
      Bluetooth: mgmt: Fix count parameter in get_connections reply
      Bluetooth: mgmt: Fix update_eir/class with HCI_AUTO_OFF flag set
      Bluetooth: mgmt: Fix return value of add/remove_uuid
      Bluetooth: mgmt: Move service cache setting to a more sensible place
      Bluetooth: mgmt: Fix clear UUIDs response
      Bluetooth: mgmt: Add flags parameter to device_connected
      Bluetooth: mgmt: Track pending class changes
      Bluetooth: mgmt: Fix dev_class related command response timing
      Bluetooth: mgmt: Fix clear_uuids response
      Bluetooth: Fix init request completion with old controllers
      Bluetooth: Use kernel int types instead of ones from stdint.h
      Bluetooth: Don't send unnecessary write_le_enable command
      Bluetooth: Remove redundant read_host_features commands
      Bluetooth: Add missing host features definitions
      Bluetooth: Use LMP_HOST_SSP define instead of magic values
      Bluetooth: mgmt: Add missing hci_dev locking to set_le()
      Bluetooth: Fix init sequence for some CSR based controllers
      Bluetooth: mgmt: Refactor hci_dev lookup for commands
      Bluetooth: mgmt: Initialize HCI_MGMT flag for any command
      Bluetooth: mgmt: Move command handlers into a table
      Bluetooth: mgmt: Add defines for command sizes
      Bluetooth: mgmt: Centralize message length checks
      Bluetooth: Fix clearing of HCI_PENDING_CLASS flag
      Bluetooth: mgmt: Fix command status error code values
      Bluetooth: mgmt: Add new error code for invalid index

Keng-Yu Lin (1):
      Bluetooth: Add AR30XX device ID on Asus laptops

Manoj Iyer (1):
      Bluetooth: btusb: Add vendor specific ID (0a5c 21f3) for BCM20702A0

Marcel Holtmann (25):
      Bluetooth: Split sending for HCI raw and control sockets
      Bluetooth: Remove unneeded bt_cb(skb)->channel variable
      Bluetooth: Limit HCI raw socket options to actual raw sockets
      Bluetooth: Lock socket when reading HCI socket options
      Bluetooth: Add HCI CMSG details only to raw sockets
      Bluetooth: Simplify HCI socket bind handling
      Bluetooth: Fix issue with shared SKB between HCI raw socket and driver
      Bluetooth: Remove HCI notifier handling
      Bluetooth: Add support for HCI monitor channel
      Bluetooth: Restrict access to management interface
      Bluetooth: Set supported settings based on enabled HS and/or LE
      Bluetooth: Always enable management interface
      Bluetooth: Fix parameter list for setting local name
      Bluetooth: Only keep controller up after init if powered on
      Bluetooth: Don't send New Settings event during setup power down
      Bluetooth: Fix two minor style issues in management code
      Bluetooth: Fix two minor style issues in HCI code
      Bluetooth: Enable timestamps for control channel
      Bluetooth: Disabling discoverable with timeout is invalid
      Bluetooth: Fix handling of discoverable setting with timeout
      Bluetooth: Send management event for class of device changes
      Bluetooth: Allow HCI UART reset parameter via flags ioctl
      Bluetooth: Add support for creating HCI UART based AMP controllers
      Bluetooth: Update L2CAP timeout constants to use msecs_to_jiffies
      Bluetooth: Update MGMT and SMP timeout constants to use msecs_to_jiffies

Octavian Purdila (2):
      Bluetooth: silence lockdep warning
      Bluetooth: Fix RFCOMM session reference counting issue

Peter Hurley (1):
      Bluetooth: Fix l2cap conn failures for ssp devices

Szymon Janc (9):
      Bluetooth: Make l2cap_clear_timer return if timer was running or not
      Bluetooth: Set P-bit for SREJ frame only if there are I-frames to ack
      Bluetooth: Clear ack_timer when sending ack
      Bluetooth: Don't send RNR immediately when entering local busy
      Bluetooth: Drop L2CAP chan reference if ERTM ack_timer fired
      Bluetooth: Make l2cap_ertm_data_rcv static
      Bluetooth: Fix possible missing I-Frame acknowledgement
      Bluetooth: Fix double acking I-Frames when sending pending I-Frames
      Bluetooth: Use NULL instead of integer for mgmt_device_connected param

Ulisses Furquim (2):
      Bluetooth: Remove usage of __cancel_delayed_work()
      Bluetooth: Fix possible use after free in delete path

Vinicius Costa Gomes (11):
      Bluetooth: Fix using an absolute timeout on hci_conn_put()
      Bluetooth: Add structures for the new LTK exchange messages
      Bluetooth: Rename smp_key_size to enc_key_size
      Bluetooth: Fix invalid memory access when there's no SMP channel
      Bluetooth: Fix doing some useless casts when receiving MGMT commands
      Bluetooth: Add new structures for handling SMP Long Term Keys
      Bluetooth: Use the updated key structures for handling LTKs
      Bluetooth: Add MGMT handlers for dealing with SMP LTK's
      Bluetooth: Add support for removing LTK's when pairing is removed
      Bluetooth: Clean up structures left unused
      Bluetooth: Add support for notifying userspace of new LTK's

 drivers/bluetooth/ath3k.c          |    1 +
 drivers/bluetooth/bfusb.c          |   23 +-
 drivers/bluetooth/bluecard_cs.c    |   20 +-
 drivers/bluetooth/bpa10x.c         |   35 +-
 drivers/bluetooth/bt3c_cs.c        |   14 +-
 drivers/bluetooth/btmrvl_debugfs.c |   27 +-
 drivers/bluetooth/btmrvl_main.c    |   17 +-
 drivers/bluetooth/btsdio.c         |   23 +-
 drivers/bluetooth/btuart_cs.c      |   14 +-
 drivers/bluetooth/btusb.c          |   47 +-
 drivers/bluetooth/btwilink.c       |   18 +-
 drivers/bluetooth/dtl1_cs.c        |   34 +-
 drivers/bluetooth/hci_ath.c        |    2 +-
 drivers/bluetooth/hci_bcsp.c       |    2 +-
 drivers/bluetooth/hci_h4.c         |    2 +-
 drivers/bluetooth/hci_ldisc.c      |   34 +-
 drivers/bluetooth/hci_ll.c         |    2 +-
 drivers/bluetooth/hci_uart.h       |    2 +
 drivers/bluetooth/hci_vhci.c       |   17 +-
 include/net/bluetooth/bluetooth.h  |   40 +-
 include/net/bluetooth/hci.h        |   72 +-
 include/net/bluetooth/hci_core.h   |  284 +++--
 include/net/bluetooth/hci_mon.h    |   51 +
 include/net/bluetooth/l2cap.h      |   37 +-
 include/net/bluetooth/mgmt.h       |  231 +++--
 include/net/bluetooth/smp.h        |    2 +-
 net/bluetooth/Kconfig              |    1 -
 net/bluetooth/bnep/sock.c          |    6 +-
 net/bluetooth/cmtp/sock.c          |    6 +-
 net/bluetooth/hci_conn.c           |   73 +-
 net/bluetooth/hci_core.c           |  627 +++++++--
 net/bluetooth/hci_event.c          |  607 ++++++---
 net/bluetooth/hci_sock.c           |  470 ++++++--
 net/bluetooth/hci_sysfs.c          |   53 +-
 net/bluetooth/hidp/sock.c          |    6 +-
 net/bluetooth/l2cap_core.c         |  638 +++++----
 net/bluetooth/l2cap_sock.c         |   53 +-
 net/bluetooth/lib.c                |   27 +-
 net/bluetooth/mgmt.c               | 2590 +++++++++++++++++++++++-------------
 net/bluetooth/smp.c                |   92 +-
 40 files changed, 4157 insertions(+), 2143 deletions(-)
 create mode 100644 include/net/bluetooth/hci_mon.h

Re: pull request: bluetooth-next 2012-03-01

[David Miller]

From: Gustavo Padovan <padovan@profusion.mobi>
Date: Sat, 3 Mar 2012 13:46:55 -0300

> This is something we can't do.

Then I cannot pull from you.

I gave you guys a one-off free pass last time around when I took
you stuff in via John's last wireless pull request.

That was your opportunity to start doing things correct yet not
be inconveniences that one time.

But if I just keep pulling from you, that sends absolutely the
message.  I am serious and you must start making your code fit
my requirements for suitability.

Re: pull request: bluetooth-next 2012-03-01

[David Miller]

From: Gustavo Padovan <padovan@profusion.mobi>
Date: Sat, 3 Mar 2012 13:46:55 -0300

> Changing only the new code will put the Bluetooth subsystem in a
> inconsistent coding style with different styles through the
> subsystem.

Which btw would be perfectly fine, this is how we gradually fix
coding style in other areas of the tree too.  So don't use crap
like this as an excuse for not doing the right thing.

Requiring a big "fix all the coding style" patch before starting to do
things properly in small increments first is completely bogus.

Re: pull request: bluetooth-next 2012-03-01

[Joe Perches]

On Sat, 2012-03-03 at 14:47 -0500, David Miller wrote:
> From: Gustavo Padovan <padovan@profusion.mobi>
> Date: Sat, 3 Mar 2012 13:46:55 -0300
> 
> > Changing only the new code will put the Bluetooth subsystem in a
> > inconsistent coding style with different styles through the
> > subsystem.
> 
> Which btw would be perfectly fine, this is how we gradually fix
> coding style in other areas of the tree too.  So don't use crap
> like this as an excuse for not doing the right thing.
> 
> Requiring a big "fix all the coding style" patch before starting to do
> things properly in small increments first is completely bogus.

Style conformity is important to people for lots of
different reasons.

It's your choice but I personally think you should
give the bluetooth folk a chance to change their style
in a single largish whitespace commit immediately post
3.4 akin to the recent isdn one you just pulled.

If the bluetooth folk want help, I do have scripts
that would do a pretty decent job and make all
the git blame -w changes transparent.

Re: pull request: bluetooth-next 2012-03-01

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Sat, 03 Mar 2012 12:04:20 -0800

> It's your choice but I personally think you should
> give the bluetooth folk a chance to change their style
> in a single largish whitespace commit immediately post
> 3.4 akin to the recent isdn one you just pulled.

They didn't just continue to use existing coding style, they also
screwed up things that were done correctly.

The struct member tabbing thing is just one example.

I already gave them a free one-time pull even though I disagreed with
what was in their tree.  I'm not going to continually review new code
that isn't styled properly.