* pull request: bluetooth-next 2012-03-01
@ 2012-03-02 2:55 Gustavo Padovan
2012-03-02 3:16 ` David Miller
0 siblings, 1 reply; 31+ messages in thread
From: Gustavo Padovan @ 2012-03-02 2:55 UTC (permalink / raw)
To: davem; +Cc: johan.hedberg, linville, netdev
[-- 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 --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-02 2:55 pull request: bluetooth-next 2012-03-01 Gustavo Padovan
@ 2012-03-02 3:16 ` David Miller
2012-03-02 3:23 ` David Miller
0 siblings, 1 reply; 31+ messages in thread
From: David Miller @ 2012-03-02 3:16 UTC (permalink / raw)
To: gustavo; +Cc: johan.hedberg, linville, netdev
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-02 3:16 ` David Miller
@ 2012-03-02 3:23 ` David Miller
2012-03-02 3:26 ` David Miller
0 siblings, 1 reply; 31+ messages in thread
From: David Miller @ 2012-03-02 3:23 UTC (permalink / raw)
To: gustavo; +Cc: johan.hedberg, linville, netdev
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);
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-02 3:23 ` David Miller
@ 2012-03-02 3:26 ` David Miller
2012-03-02 4:13 ` Joe Perches
2012-03-02 16:37 ` pull request: bluetooth-next 2012-03-01 Gustavo Padovan
0 siblings, 2 replies; 31+ messages in thread
From: David Miller @ 2012-03-02 3:26 UTC (permalink / raw)
To: gustavo; +Cc: johan.hedberg, linville, netdev
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-02 3:26 ` David Miller
@ 2012-03-02 4:13 ` Joe Perches
2012-03-02 5:35 ` [PATCH] checkpatch: Add --strict tests for braces, comments and casts Joe Perches
2012-03-02 16:37 ` pull request: bluetooth-next 2012-03-01 Gustavo Padovan
1 sibling, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-02 4:13 UTC (permalink / raw)
To: David Miller; +Cc: gustavo, johan.hedberg, linville, netdev
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?
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] checkpatch: Add --strict tests for braces, comments and casts
2012-03-02 4:13 ` Joe Perches
@ 2012-03-02 5:35 ` Joe Perches
2012-03-02 5:54 ` [PATCH] checkpatch: Add --strict test for strings split across multiple lines Joe Perches
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-02 5:35 UTC (permalink / raw)
To: David Miller, Andrew Morton, Andy Whitcroft
Cc: Dan Carpenter, gustavo, johan.hedberg, linville, netdev, LKML
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH] checkpatch: Add --strict test for strings split across multiple lines
2012-03-02 5:35 ` [PATCH] checkpatch: Add --strict tests for braces, comments and casts Joe Perches
@ 2012-03-02 5:54 ` Joe Perches
2012-03-13 6:23 ` [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL> Joe Perches
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-02 5:54 UTC (permalink / raw)
To: David Miller, Andy Whitcroft, Andrew Morton
Cc: Dan Carpenter, gustavo, johan.hedberg, linville, netdev, LKML
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-02 3:26 ` David Miller
2012-03-02 4:13 ` Joe Perches
@ 2012-03-02 16:37 ` Gustavo Padovan
2012-03-02 21:15 ` David Miller
1 sibling, 1 reply; 31+ messages in thread
From: Gustavo Padovan @ 2012-03-02 16:37 UTC (permalink / raw)
To: David Miller; +Cc: gustavo, johan.hedberg, linville, netdev
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-02 16:37 ` pull request: bluetooth-next 2012-03-01 Gustavo Padovan
@ 2012-03-02 21:15 ` David Miller
2012-03-03 16:46 ` Gustavo Padovan
0 siblings, 1 reply; 31+ messages in thread
From: David Miller @ 2012-03-02 21:15 UTC (permalink / raw)
To: padovan; +Cc: gustavo, johan.hedberg, linville, netdev
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-02 21:15 ` David Miller
@ 2012-03-03 16:46 ` Gustavo Padovan
2012-03-03 19:46 ` David Miller
2012-03-03 19:47 ` David Miller
0 siblings, 2 replies; 31+ messages in thread
From: Gustavo Padovan @ 2012-03-03 16:46 UTC (permalink / raw)
To: David Miller; +Cc: gustavo, johan.hedberg, linville, marcel, netdev
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-03 16:46 ` Gustavo Padovan
@ 2012-03-03 19:46 ` David Miller
2012-03-03 19:47 ` David Miller
1 sibling, 0 replies; 31+ messages in thread
From: David Miller @ 2012-03-03 19:46 UTC (permalink / raw)
To: padovan; +Cc: gustavo, johan.hedberg, linville, marcel, netdev
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-03 16:46 ` Gustavo Padovan
2012-03-03 19:46 ` David Miller
@ 2012-03-03 19:47 ` David Miller
2012-03-03 20:04 ` Joe Perches
1 sibling, 1 reply; 31+ messages in thread
From: David Miller @ 2012-03-03 19:47 UTC (permalink / raw)
To: padovan; +Cc: gustavo, johan.hedberg, linville, marcel, netdev
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-03 19:47 ` David Miller
@ 2012-03-03 20:04 ` Joe Perches
2012-03-03 20:06 ` David Miller
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-03 20:04 UTC (permalink / raw)
To: David Miller; +Cc: padovan, gustavo, johan.hedberg, linville, marcel, netdev
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: pull request: bluetooth-next 2012-03-01
2012-03-03 20:04 ` Joe Perches
@ 2012-03-03 20:06 ` David Miller
0 siblings, 0 replies; 31+ messages in thread
From: David Miller @ 2012-03-03 20:06 UTC (permalink / raw)
To: joe; +Cc: padovan, gustavo, johan.hedberg, linville, marcel, netdev
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-02 5:54 ` [PATCH] checkpatch: Add --strict test for strings split across multiple lines Joe Perches
@ 2012-03-13 6:23 ` Joe Perches
2012-03-13 12:05 ` Ted Ts'o
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-13 6:23 UTC (permalink / raw)
To: Andrew Morton; +Cc: Andy Whitcroft, LKML
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
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-13 6:23 ` [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL> Joe Perches
@ 2012-03-13 12:05 ` Ted Ts'o
2012-03-13 21:55 ` Andrew Morton
0 siblings, 1 reply; 31+ messages in thread
From: Ted Ts'o @ 2012-03-13 12:05 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-13 12:05 ` Ted Ts'o
@ 2012-03-13 21:55 ` Andrew Morton
2012-03-13 22:01 ` Ted Ts'o
0 siblings, 1 reply; 31+ messages in thread
From: Andrew Morton @ 2012-03-13 21:55 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Joe Perches, Andy Whitcroft, LKML
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 ;)
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-13 21:55 ` Andrew Morton
@ 2012-03-13 22:01 ` Ted Ts'o
2012-03-13 22:03 ` Andrew Morton
0 siblings, 1 reply; 31+ messages in thread
From: Ted Ts'o @ 2012-03-13 22:01 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-13 22:01 ` Ted Ts'o
@ 2012-03-13 22:03 ` Andrew Morton
2012-03-14 0:31 ` Ted Ts'o
0 siblings, 1 reply; 31+ messages in thread
From: Andrew Morton @ 2012-03-13 22:03 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Joe Perches, Andy Whitcroft, LKML
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...
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-13 22:03 ` Andrew Morton
@ 2012-03-14 0:31 ` Ted Ts'o
2012-03-14 0:47 ` Joe Perches
0 siblings, 1 reply; 31+ messages in thread
From: Ted Ts'o @ 2012-03-14 0:31 UTC (permalink / raw)
To: Andrew Morton; +Cc: Joe Perches, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 0:31 ` Ted Ts'o
@ 2012-03-14 0:47 ` Joe Perches
2012-03-14 1:07 ` Ted Ts'o
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-14 0:47 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 0:47 ` Joe Perches
@ 2012-03-14 1:07 ` Ted Ts'o
2012-03-14 1:17 ` Joe Perches
0 siblings, 1 reply; 31+ messages in thread
From: Ted Ts'o @ 2012-03-14 1:07 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 1:07 ` Ted Ts'o
@ 2012-03-14 1:17 ` Joe Perches
2012-03-14 2:19 ` Ted Ts'o
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-14 1:17 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Andrew Morton, Andy Whitcroft, LKML
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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 1:17 ` Joe Perches
@ 2012-03-14 2:19 ` Ted Ts'o
2012-03-14 2:31 ` Joe Perches
0 siblings, 1 reply; 31+ messages in thread
From: Ted Ts'o @ 2012-03-14 2:19 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 2:19 ` Ted Ts'o
@ 2012-03-14 2:31 ` Joe Perches
2012-03-14 2:41 ` Ted Ts'o
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-14 2:31 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 2:31 ` Joe Perches
@ 2012-03-14 2:41 ` Ted Ts'o
2012-03-14 3:01 ` Joe Perches
0 siblings, 1 reply; 31+ messages in thread
From: Ted Ts'o @ 2012-03-14 2:41 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 2:41 ` Ted Ts'o
@ 2012-03-14 3:01 ` Joe Perches
2012-03-14 12:34 ` Ted Ts'o
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-14 3:01 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 3:01 ` Joe Perches
@ 2012-03-14 12:34 ` Ted Ts'o
2012-03-14 13:05 ` Joe Perches
0 siblings, 1 reply; 31+ messages in thread
From: Ted Ts'o @ 2012-03-14 12:34 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 12:34 ` Ted Ts'o
@ 2012-03-14 13:05 ` Joe Perches
2012-03-14 13:45 ` Ted Ts'o
0 siblings, 1 reply; 31+ messages in thread
From: Joe Perches @ 2012-03-14 13:05 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 13:05 ` Joe Perches
@ 2012-03-14 13:45 ` Ted Ts'o
2012-03-14 14:06 ` Joe Perches
0 siblings, 1 reply; 31+ messages in thread
From: Ted Ts'o @ 2012-03-14 13:45 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL>
2012-03-14 13:45 ` Ted Ts'o
@ 2012-03-14 14:06 ` Joe Perches
0 siblings, 0 replies; 31+ messages in thread
From: Joe Perches @ 2012-03-14 14:06 UTC (permalink / raw)
To: Ted Ts'o; +Cc: Andrew Morton, Andy Whitcroft, LKML
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
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2012-03-14 14:06 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-02 2:55 pull request: bluetooth-next 2012-03-01 Gustavo Padovan
2012-03-02 3:16 ` David Miller
2012-03-02 3:23 ` David Miller
2012-03-02 3:26 ` David Miller
2012-03-02 4:13 ` Joe Perches
2012-03-02 5:35 ` [PATCH] checkpatch: Add --strict tests for braces, comments and casts Joe Perches
2012-03-02 5:54 ` [PATCH] checkpatch: Add --strict test for strings split across multiple lines Joe Perches
2012-03-13 6:23 ` [PATCH] checkpatch: Suggest pr_<level> over printk(KERN_<LEVEL> Joe Perches
2012-03-13 12:05 ` Ted Ts'o
2012-03-13 21:55 ` Andrew Morton
2012-03-13 22:01 ` Ted Ts'o
2012-03-13 22:03 ` Andrew Morton
2012-03-14 0:31 ` Ted Ts'o
2012-03-14 0:47 ` Joe Perches
2012-03-14 1:07 ` Ted Ts'o
2012-03-14 1:17 ` Joe Perches
2012-03-14 2:19 ` Ted Ts'o
2012-03-14 2:31 ` Joe Perches
2012-03-14 2:41 ` Ted Ts'o
2012-03-14 3:01 ` Joe Perches
2012-03-14 12:34 ` Ted Ts'o
2012-03-14 13:05 ` Joe Perches
2012-03-14 13:45 ` Ted Ts'o
2012-03-14 14:06 ` Joe Perches
2012-03-02 16:37 ` pull request: bluetooth-next 2012-03-01 Gustavo Padovan
2012-03-02 21:15 ` David Miller
2012-03-03 16:46 ` Gustavo Padovan
2012-03-03 19:46 ` David Miller
2012-03-03 19:47 ` David Miller
2012-03-03 20:04 ` Joe Perches
2012-03-03 20:06 ` David Miller
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.