All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
@ 2016-10-03  8:56 Pavel Machek
  2016-10-05 11:14 ` Marcel Holtmann
  2016-10-05 20:51 ` [PATCHv2] " Pavel Machek
  0 siblings, 2 replies; 21+ messages in thread
From: Pavel Machek @ 2016-10-03  8:56 UTC (permalink / raw)
  To: trivial, marcel, gustavo, johan.hedberg, davem, linux-bluetooth,
	netdev, linux-kernel

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


bluetooth.h is not part of user API, so __ variants are not neccessary
here.

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index bfd1590..aea0371 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -60,8 +60,8 @@
 
 #define BT_SECURITY	4
 struct bt_security {
-	__u8 level;
-	__u8 key_size;
+	u8 level;
+	u8 key_size;
 };
 #define BT_SECURITY_SDP		0
 #define BT_SECURITY_LOW		1
@@ -78,7 +78,7 @@ struct bt_security {
 
 #define BT_POWER	9
 struct bt_power {
-	__u8 force_active;
+	u8 force_active;
 };
 #define BT_POWER_FORCE_ACTIVE_OFF 0
 #define BT_POWER_FORCE_ACTIVE_ON  1
@@ -112,7 +112,7 @@ struct bt_power {
 
 #define BT_VOICE		11
 struct bt_voice {
-	__u16 setting;
+	u16 setting;
 };
 
 #define BT_VOICE_TRANSPARENT			0x0003
@@ -188,7 +188,7 @@ static inline const char *state_to_string(int state)
 
 /* BD Address */
 typedef struct {
-	__u8 b[6];
+	u8 b[6];
 } __packed bdaddr_t;
 
 /* BD Address type */
@@ -196,7 +196,7 @@ typedef struct {
 #define BDADDR_LE_PUBLIC	0x01
 #define BDADDR_LE_RANDOM	0x02
 
-static inline bool bdaddr_type_is_valid(__u8 type)
+static inline bool bdaddr_type_is_valid(u8 type)
 {
 	switch (type) {
 	case BDADDR_BREDR:
@@ -208,7 +208,7 @@ static inline bool bdaddr_type_is_valid(__u8 type)
 	return false;
 }
 
-static inline bool bdaddr_type_is_le(__u8 type)
+static inline bool bdaddr_type_is_le(u8 type)
 {
 	switch (type) {
 	case BDADDR_LE_PUBLIC:
@@ -278,15 +278,16 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);
 
 /* Skb helpers */
 struct l2cap_ctrl {
-	__u8	sframe:1,
+	u8	sframe:1,
 		poll:1,
 		final:1,
 		fcs:1,
 		sar:2,
 		super:2;
-	__u16	reqseq;
-	__u16	txseq;
-	__u8	retries;
+
+	u16	reqseq;
+	u16	txseq;
+	u8	retries;
 	__le16  psm;
 	bdaddr_t bdaddr;
 	struct l2cap_chan *chan;
@@ -302,7 +303,7 @@ typedef void (*hci_req_complete_skb_t)(struct hci_dev *hdev, u8 status,
 #define HCI_REQ_SKB	BIT(1)
 
 struct hci_ctrl {
-	__u16 opcode;
+	u16 opcode;
 	u8 req_flags;
 	u8 req_event;
 	union {
@@ -312,10 +313,10 @@ struct hci_ctrl {
 };
 
 struct bt_skb_cb {
-	__u8 pkt_type;
-	__u8 force_active;
-	__u16 expect;
-	__u8 incoming:1;
+	u8 pkt_type;
+	u8 force_active;
+	u16 expect;
+	u8 incoming:1;
 	union {
 		struct l2cap_ctrl l2cap;
 		struct hci_ctrl hci;
@@ -365,7 +366,7 @@ out:
 	return NULL;
 }
 
-int bt_to_errno(__u16 code);
+int bt_to_errno(u16 code);
 
 void hci_sock_set_flag(struct sock *sk, int nr);
 void hci_sock_clear_flag(struct sock *sk, int nr);

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-03  8:56 [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel Pavel Machek
@ 2016-10-05 11:14 ` Marcel Holtmann
  2016-10-05 17:53   ` Joe Perches
  2016-10-05 20:51 ` [PATCHv2] " Pavel Machek
  1 sibling, 1 reply; 21+ messages in thread
From: Marcel Holtmann @ 2016-10-05 11:14 UTC (permalink / raw)
  To: Pavel Machek
  Cc: trivial, Gustavo F. Padovan, Johan Hedberg, David S. Miller,
	linux-bluetooth, netdev, linux-kernel

Hi Pavel,

> bluetooth.h is not part of user API, so __ variants are not neccessary
> here.
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index bfd1590..aea0371 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -60,8 +60,8 @@
> 
> #define BT_SECURITY	4
> struct bt_security {
> -	__u8 level;
> -	__u8 key_size;
> +	u8 level;
> +	u8 key_size;
> };
> #define BT_SECURITY_SDP		0
> #define BT_SECURITY_LOW		1
> @@ -78,7 +78,7 @@ struct bt_security {
> 
> #define BT_POWER	9
> struct bt_power {
> -	__u8 force_active;
> +	u8 force_active;
> };
> #define BT_POWER_FORCE_ACTIVE_OFF 0
> #define BT_POWER_FORCE_ACTIVE_ON  1
> @@ -112,7 +112,7 @@ struct bt_power {
> 
> #define BT_VOICE		11
> struct bt_voice {
> -	__u16 setting;
> +	u16 setting;
> };
> 
> #define BT_VOICE_TRANSPARENT			0x0003
> @@ -188,7 +188,7 @@ static inline const char *state_to_string(int state)
> 
> /* BD Address */
> typedef struct {
> -	__u8 b[6];
> +	u8 b[6];
> } __packed bdaddr_t;
> 

can you leave these out please. They are meant to become UAPI eventually.

> /* BD Address type */
> @@ -196,7 +196,7 @@ typedef struct {
> #define BDADDR_LE_PUBLIC	0x01
> #define BDADDR_LE_RANDOM	0x02
> 
> -static inline bool bdaddr_type_is_valid(__u8 type)
> +static inline bool bdaddr_type_is_valid(u8 type)
> {
> 	switch (type) {
> 	case BDADDR_BREDR:
> @@ -208,7 +208,7 @@ static inline bool bdaddr_type_is_valid(__u8 type)
> 	return false;
> }
> 
> -static inline bool bdaddr_type_is_le(__u8 type)
> +static inline bool bdaddr_type_is_le(u8 type)
> {
> 	switch (type) {
> 	case BDADDR_LE_PUBLIC:
> @@ -278,15 +278,16 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);
> 
> /* Skb helpers */
> struct l2cap_ctrl {
> -	__u8	sframe:1,
> +	u8	sframe:1,
> 		poll:1,
> 		final:1,
> 		fcs:1,
> 		sar:2,
> 		super:2;
> -	__u16	reqseq;
> -	__u16	txseq;
> -	__u8	retries;
> +
> +	u16	reqseq;
> +	u16	txseq;
> +	u8	retries;
> 	__le16  psm;
> 	bdaddr_t bdaddr;
> 	struct l2cap_chan *chan;
> @@ -302,7 +303,7 @@ typedef void (*hci_req_complete_skb_t)(struct hci_dev *hdev, u8 status,
> #define HCI_REQ_SKB	BIT(1)
> 
> struct hci_ctrl {
> -	__u16 opcode;
> +	u16 opcode;
> 	u8 req_flags;
> 	u8 req_event;
> 	union {
> @@ -312,10 +313,10 @@ struct hci_ctrl {
> };
> 
> struct bt_skb_cb {
> -	__u8 pkt_type;
> -	__u8 force_active;
> -	__u16 expect;
> -	__u8 incoming:1;
> +	u8 pkt_type;
> +	u8 force_active;
> +	u16 expect;
> +	u8 incoming:1;
> 	union {
> 		struct l2cap_ctrl l2cap;
> 		struct hci_ctrl hci;
> @@ -365,7 +366,7 @@ out:
> 	return NULL;
> }
> 
> -int bt_to_errno(__u16 code);
> +int bt_to_errno(u16 code);

The rest looks good to me.

Regards

Marcel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-05 11:14 ` Marcel Holtmann
@ 2016-10-05 17:53   ` Joe Perches
  2016-10-05 19:11       ` Pavel Machek
  0 siblings, 1 reply; 21+ messages in thread
From: Joe Perches @ 2016-10-05 17:53 UTC (permalink / raw)
  To: Marcel Holtmann, Pavel Machek
  Cc: trivial, Gustavo F. Padovan, Johan Hedberg, David S. Miller,
	linux-bluetooth, netdev, linux-kernel

On Wed, 2016-10-05 at 13:14 +0200, Marcel Holtmann wrote:
> Hi Pavel,
> 
> > bluetooth.h is not part of user API, so __ variants are not neccessary
> > here.
> > 
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
[]
> > struct bt_skb_cb {
> > -	__u8 pkt_type;
> > -	__u8 force_active;
> > -	__u16 expect;
> > -	__u8 incoming:1;
> > +	u8 pkt_type;
> > +	u8 force_active;
> > +	u16 expect;
> > +	u8 incoming:1;
> > 	union {
> > 		struct l2cap_ctrl l2cap;
> > 		struct hci_ctrl hci;

trivia:

It's generally faster to use bool instead of u8 foo:1;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
@ 2016-10-05 19:11       ` Pavel Machek
  0 siblings, 0 replies; 21+ messages in thread
From: Pavel Machek @ 2016-10-05 19:11 UTC (permalink / raw)
  To: Joe Perches
  Cc: Marcel Holtmann, trivial, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

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

On Wed 2016-10-05 10:53:16, Joe Perches wrote:
> On Wed, 2016-10-05 at 13:14 +0200, Marcel Holtmann wrote:
> > Hi Pavel,
> > 
> > > bluetooth.h is not part of user API, so __ variants are not neccessary
> > > here.
> > > 
> > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > 
> > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> []
> > > struct bt_skb_cb {
> > > -	__u8 pkt_type;
> > > -	__u8 force_active;
> > > -	__u16 expect;
> > > -	__u8 incoming:1;
> > > +	u8 pkt_type;
> > > +	u8 force_active;
> > > +	u16 expect;
> > > +	u8 incoming:1;
> > > 	union {
> > > 		struct l2cap_ctrl l2cap;
> > > 		struct hci_ctrl hci;
> 
> trivia:
> 
> It's generally faster to use bool instead of u8 foo:1;

Ok, but I'm not changing that in this patch.

(And actually, bool will take a lot more memory, right?)
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
@ 2016-10-05 19:11       ` Pavel Machek
  0 siblings, 0 replies; 21+ messages in thread
From: Pavel Machek @ 2016-10-05 19:11 UTC (permalink / raw)
  To: Joe Perches
  Cc: Marcel Holtmann, trivial-DgEjT+Ai2ygdnm+yROfE0A,
	Gustavo F. Padovan, Johan Hedberg, David S. Miller,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

On Wed 2016-10-05 10:53:16, Joe Perches wrote:
> On Wed, 2016-10-05 at 13:14 +0200, Marcel Holtmann wrote:
> > Hi Pavel,
> > 
> > > bluetooth.h is not part of user API, so __ variants are not neccessary
> > > here.
> > > 
> > > Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > > 
> > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> []
> > > struct bt_skb_cb {
> > > -	__u8 pkt_type;
> > > -	__u8 force_active;
> > > -	__u16 expect;
> > > -	__u8 incoming:1;
> > > +	u8 pkt_type;
> > > +	u8 force_active;
> > > +	u16 expect;
> > > +	u8 incoming:1;
> > > 	union {
> > > 		struct l2cap_ctrl l2cap;
> > > 		struct hci_ctrl hci;
> 
> trivia:
> 
> It's generally faster to use bool instead of u8 foo:1;

Ok, but I'm not changing that in this patch.

(And actually, bool will take a lot more memory, right?)
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-05 19:11       ` Pavel Machek
@ 2016-10-05 19:15         ` Joe Perches
  -1 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2016-10-05 19:15 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Marcel Holtmann, trivial, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

On Wed, 2016-10-05 at 21:11 +0200, Pavel Machek wrote:
> On Wed 2016-10-05 10:53:16, Joe Perches wrote:
> > On Wed, 2016-10-05 at 13:14 +0200, Marcel Holtmann wrote:
> > > Hi Pavel,
> > > 
> > > > bluetooth.h is not part of user API, so __ variants are not neccessary
> > > > here.
> > > > 
> > > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > > 
> > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > 
> > []
> > > > struct bt_skb_cb {
> > > > -	__u8 pkt_type;
> > > > -	__u8 force_active;
> > > > -	__u16 expect;
> > > > -	__u8 incoming:1;
> > > > +	u8 pkt_type;
> > > > +	u8 force_active;
> > > > +	u16 expect;
> > > > +	u8 incoming:1;
> > > > 	union {
> > > > 		struct l2cap_ctrl l2cap;
> > > > 		struct hci_ctrl hci;
> > 
> > 
> > trivia:
> > 
> > It's generally faster to use bool instead of u8 foo:1;
> 
> Ok, but I'm not changing that in this patch.
> (And actually, bool will take a lot more memory, right?)

No worries, and bool is the same size as u8.
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
@ 2016-10-05 19:15         ` Joe Perches
  0 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2016-10-05 19:15 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Marcel Holtmann, trivial, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

On Wed, 2016-10-05 at 21:11 +0200, Pavel Machek wrote:
> On Wed 2016-10-05 10:53:16, Joe Perches wrote:
> > On Wed, 2016-10-05 at 13:14 +0200, Marcel Holtmann wrote:
> > > Hi Pavel,
> > > 
> > > > bluetooth.h is not part of user API, so __ variants are not neccessary
> > > > here.
> > > > 
> > > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > > 
> > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > 
> > []
> > > > struct bt_skb_cb {
> > > > -	__u8 pkt_type;
> > > > -	__u8 force_active;
> > > > -	__u16 expect;
> > > > -	__u8 incoming:1;
> > > > +	u8 pkt_type;
> > > > +	u8 force_active;
> > > > +	u16 expect;
> > > > +	u8 incoming:1;
> > > > 	union {
> > > > 		struct l2cap_ctrl l2cap;
> > > > 		struct hci_ctrl hci;
> > 
> > 
> > trivia:
> > 
> > It's generally faster to use bool instead of u8 foo:1;
> 
> Ok, but I'm not changing that in this patch.
> (And actually, bool will take a lot more memory, right?)

No worries, and bool is the same size as u8.
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCHv2] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-03  8:56 [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel Pavel Machek
  2016-10-05 11:14 ` Marcel Holtmann
@ 2016-10-05 20:51 ` Pavel Machek
  2016-10-05 21:54   ` Marcel Holtmann
  1 sibling, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2016-10-05 20:51 UTC (permalink / raw)
  To: trivial, marcel, gustavo, johan.hedberg, davem, linux-bluetooth,
	netdev, linux-kernel

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


bluetooth.h is not part of user API, so __ variants are not neccessary
here.

Signed-off-by: Pavel Machek <pavel@ucw.cz>

---
v2: not touching stuff that Marcel does not want touched, as it will
become API later.

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index bfd1590..aea0371 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -196,7 +196,7 @@ typedef struct {
 #define BDADDR_LE_PUBLIC	0x01
 #define BDADDR_LE_RANDOM	0x02
 
-static inline bool bdaddr_type_is_valid(__u8 type)
+static inline bool bdaddr_type_is_valid(u8 type)
 {
 	switch (type) {
 	case BDADDR_BREDR:
@@ -208,7 +208,7 @@ static inline bool bdaddr_type_is_valid(__u8 type)
 	return false;
 }
 
-static inline bool bdaddr_type_is_le(__u8 type)
+static inline bool bdaddr_type_is_le(u8 type)
 {
 	switch (type) {
 	case BDADDR_LE_PUBLIC:
@@ -278,15 +278,16 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);
 
 /* Skb helpers */
 struct l2cap_ctrl {
-	__u8	sframe:1,
+	u8	sframe:1,
 		poll:1,
 		final:1,
 		fcs:1,
 		sar:2,
 		super:2;
-	__u16	reqseq;
-	__u16	txseq;
-	__u8	retries;
+
+	u16	reqseq;
+	u16	txseq;
+	u8	retries;
 	__le16  psm;
 	bdaddr_t bdaddr;
 	struct l2cap_chan *chan;
@@ -302,7 +303,7 @@ typedef void (*hci_req_complete_skb_t)(struct hci_dev *hdev, u8 status,
 #define HCI_REQ_SKB	BIT(1)
 
 struct hci_ctrl {
-	__u16 opcode;
+	u16 opcode;
 	u8 req_flags;
 	u8 req_event;
 	union {
@@ -312,10 +313,10 @@ struct hci_ctrl {
 };
 
 struct bt_skb_cb {
-	__u8 pkt_type;
-	__u8 force_active;
-	__u16 expect;
-	__u8 incoming:1;
+	u8 pkt_type;
+	u8 force_active;
+	u16 expect;
+	u8 incoming:1;
 	union {
 		struct l2cap_ctrl l2cap;
 		struct hci_ctrl hci;
@@ -365,7 +366,7 @@ out:
 	return NULL;
 }
 
-int bt_to_errno(__u16 code);
+int bt_to_errno(u16 code);
 
 void hci_sock_set_flag(struct sock *sk, int nr);
 void hci_sock_clear_flag(struct sock *sk, int nr);



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCHv2] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-05 20:51 ` [PATCHv2] " Pavel Machek
@ 2016-10-05 21:54   ` Marcel Holtmann
  0 siblings, 0 replies; 21+ messages in thread
From: Marcel Holtmann @ 2016-10-05 21:54 UTC (permalink / raw)
  To: Pavel Machek
  Cc: trivial, Gustavo F. Padovan, Johan Hedberg, David S. Miller,
	open list:BLUETOOTH DRIVERS, netdev, linux-kernel

Hi Pavel,

> bluetooth.h is not part of user API, so __ variants are not neccessary
> here.
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> ---
> v2: not touching stuff that Marcel does not want touched, as it will
> become API later.

patch has been applied to bluetooth-next tree.

Regards

Marcel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-05 19:15         ` Joe Perches
  (?)
@ 2016-10-05 22:13         ` Pavel Machek
  2016-10-05 22:28             ` Joe Perches
  -1 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2016-10-05 22:13 UTC (permalink / raw)
  To: Joe Perches
  Cc: Marcel Holtmann, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

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

On Wed 2016-10-05 12:15:34, Joe Perches wrote:
> On Wed, 2016-10-05 at 21:11 +0200, Pavel Machek wrote:
> > On Wed 2016-10-05 10:53:16, Joe Perches wrote:
> > > On Wed, 2016-10-05 at 13:14 +0200, Marcel Holtmann wrote:
> > > > Hi Pavel,
> > > > 
> > > > > bluetooth.h is not part of user API, so __ variants are not neccessary
> > > > > here.
> > > > > 
> > > > > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > > > > 
> > > > > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > > 
> > > []
> > > > > struct bt_skb_cb {
> > > > > -	__u8 pkt_type;
> > > > > -	__u8 force_active;
> > > > > -	__u16 expect;
> > > > > -	__u8 incoming:1;
> > > > > +	u8 pkt_type;
> > > > > +	u8 force_active;
> > > > > +	u16 expect;
> > > > > +	u8 incoming:1;
> > > > > 	union {
> > > > > 		struct l2cap_ctrl l2cap;
> > > > > 		struct hci_ctrl hci;
> > > 
> > > 
> > > trivia:
> > > 
> > > It's generally faster to use bool instead of u8 foo:1;
> > 
> > Ok, but I'm not changing that in this patch.
> > (And actually, bool will take a lot more memory, right?)
> 
> No worries, and bool is the same size as u8.

Exactly what I'm talking about :-). One byte vs. one bit, right?

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-05 22:13         ` Pavel Machek
@ 2016-10-05 22:28             ` Joe Perches
  0 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2016-10-05 22:28 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Marcel Holtmann, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

On Thu, 2016-10-06 at 00:13 +0200, Pavel Machek wrote:
> On Wed 2016-10-05 12:15:34, Joe Perches wrote:
> > On Wed, 2016-10-05 at 21:11 +0200, Pavel Machek wrote:
> > > On Wed 2016-10-05 10:53:16, Joe Perches wrote:
[]
> > > > trivia:
> > > > It's generally faster to use bool instead of u8 foo:1;
> > > Ok, but I'm not changing that in this patch.
> > > (And actually, bool will take a lot more memory, right?)
> > No worries, and bool is the same size as u8.
> Exactly what I'm talking about :-). One byte vs. one bit, right?

Memory isn't bit addressable.
So it's the same byte, it just doesn't use a read/modify/write
operation to update a value.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
@ 2016-10-05 22:28             ` Joe Perches
  0 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2016-10-05 22:28 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Marcel Holtmann, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

On Thu, 2016-10-06 at 00:13 +0200, Pavel Machek wrote:
> On Wed 2016-10-05 12:15:34, Joe Perches wrote:
> > On Wed, 2016-10-05 at 21:11 +0200, Pavel Machek wrote:
> > > On Wed 2016-10-05 10:53:16, Joe Perches wrote:
[]
> > > > trivia:
> > > > It's generally faster to use bool instead of u8 foo:1;
> > > Ok, but I'm not changing that in this patch.
> > > (And actually, bool will take a lot more memory, right?)
> > No worries, and bool is the same size as u8.
> Exactly what I'm talking about :-). One byte vs. one bit, right?

Memory isn't bit addressable.
So it's the same byte, it just doesn't use a read/modify/write
operation to update a value.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-05 22:28             ` Joe Perches
  (?)
@ 2016-10-06  7:02             ` Pavel Machek
  2016-10-06  7:07                 ` Joe Perches
  -1 siblings, 1 reply; 21+ messages in thread
From: Pavel Machek @ 2016-10-06  7:02 UTC (permalink / raw)
  To: Joe Perches
  Cc: Marcel Holtmann, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

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

On Wed 2016-10-05 15:28:51, Joe Perches wrote:
> On Thu, 2016-10-06 at 00:13 +0200, Pavel Machek wrote:
> > On Wed 2016-10-05 12:15:34, Joe Perches wrote:
> > > On Wed, 2016-10-05 at 21:11 +0200, Pavel Machek wrote:
> > > > On Wed 2016-10-05 10:53:16, Joe Perches wrote:
> []
> > > > > trivia:
> > > > > It's generally faster to use bool instead of u8 foo:1;
> > > > Ok, but I'm not changing that in this patch.
> > > > (And actually, bool will take a lot more memory, right?)
> > > No worries, and bool is the same size as u8.
> > Exactly what I'm talking about :-). One byte vs. one bit, right?
> 
> Memory isn't bit addressable.
> So it's the same byte, it just doesn't use a read/modify/write
> operation to update a value.

I believe you are wrong. bit addressability does not matter, cpu can
definitely get the bit values.

u8 foo:1;
u8 bar:1;
u8 baz:1;

should take 1 byte, where

bool foo, bar, baz;

will take more like 3.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-06  7:02             ` Pavel Machek
@ 2016-10-06  7:07                 ` Joe Perches
  0 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2016-10-06  7:07 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Marcel Holtmann, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

On Thu, 2016-10-06 at 09:02 +0200, Pavel Machek wrote:
> I believe you are wrong. bit addressability does not matter, cpu can
> definitely get the bit values.
> 
> u8 foo:1;
> u8 bar:1;
> u8 baz:1;
> 
> should take 1 byte, where
> 
> bool foo, bar, baz;
> 
> will take more like 3.

Definitely true.

There is only one single bitfield foo here though
so what you wrote doesn't apply.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
@ 2016-10-06  7:07                 ` Joe Perches
  0 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2016-10-06  7:07 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Marcel Holtmann, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

On Thu, 2016-10-06 at 09:02 +0200, Pavel Machek wrote:
> I believe you are wrong. bit addressability does not matter, cpu can
> definitely get the bit values.
> 
> u8 foo:1;
> u8 bar:1;
> u8 baz:1;
> 
> should take 1 byte, where
> 
> bool foo, bar, baz;
> 
> will take more like 3.

Definitely true.

There is only one single bitfield foo here though
so what you wrote doesn't apply.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-06  7:07                 ` Joe Perches
  (?)
@ 2016-10-06  8:38                 ` Johan Hedberg
  -1 siblings, 0 replies; 21+ messages in thread
From: Johan Hedberg @ 2016-10-06  8:38 UTC (permalink / raw)
  To: Joe Perches
  Cc: Pavel Machek, Marcel Holtmann, Gustavo F. Padovan,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

Hi,

On Thu, Oct 06, 2016, Joe Perches wrote:
> On Thu, 2016-10-06 at 09:02 +0200, Pavel Machek wrote:
> > I believe you are wrong. bit addressability does not matter, cpu can
> > definitely get the bit values.
> > 
> > u8 foo:1;
> > u8 bar:1;
> > u8 baz:1;
> > 
> > should take 1 byte, where
> > 
> > bool foo, bar, baz;
> > 
> > will take more like 3.
> 
> Definitely true.
> 
> There is only one single bitfield foo here though
> so what you wrote doesn't apply.

What's in the tree is a left-over from times when there were multiple
bit fields in this struct. By the time others were removed and there was
only one left no-one has apparently bothered to update it to a bool or
single u8.

Johan

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-05 19:15         ` Joe Perches
  (?)
  (?)
@ 2016-10-06  9:41         ` David Laight
  2016-10-06 11:38           ` Joe Perches
  -1 siblings, 1 reply; 21+ messages in thread
From: David Laight @ 2016-10-06  9:41 UTC (permalink / raw)
  To: 'Joe Perches', Pavel Machek
  Cc: Marcel Holtmann, trivial, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

From: Of Joe Perches
...
> No worries, and bool is the same size as u8.

That is not guaranteed at all.
One of the ARM ABI defined bool to be the size of int.

	David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-06  9:41         ` David Laight
@ 2016-10-06 11:38           ` Joe Perches
  2016-10-06 13:00               ` David Laight
  0 siblings, 1 reply; 21+ messages in thread
From: Joe Perches @ 2016-10-06 11:38 UTC (permalink / raw)
  To: David Laight, Pavel Machek
  Cc: Marcel Holtmann, trivial, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

On Thu, 2016-10-06 at 09:41 +0000, David Laight wrote:
> From: Joe Perches
> > No worries, and bool is the same ,size as u8.
> That is not guaranteed at all.
> One of the ARM ABI defined bool to be the size of int.

Really?  What kernel has sizeof(_Bool) != 1 ?

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-06 11:38           ` Joe Perches
@ 2016-10-06 13:00               ` David Laight
  0 siblings, 0 replies; 21+ messages in thread
From: David Laight @ 2016-10-06 13:00 UTC (permalink / raw)
  To: 'Joe Perches', Pavel Machek
  Cc: Marcel Holtmann, trivial, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

From: Joe Perches
> Sent: 06 October 2016 12:39
> On Thu, 2016-10-06 at 09:41 +0000, David Laight wrote:
> > From: Joe Perches
> > > No worries, and bool is the same ,size as u8.
> > That is not guaranteed at all.
> > One of the ARM ABI defined bool to be the size of int.
> 
> Really?  What kernel has sizeof(_Bool) != 1 ?

Probably none, but I know systems have used larger bool.
I found this:
> with egcs-2.90.29 980515 (egcs-1.0.3 release) on alphaev56-dec-osf4.0d

>  bool  = 8
>  short = 2
>  int   = 4 
>  long  = 8

I'm pretty sure something newer than an old alpha ABI used 4 byte bool.

	David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
@ 2016-10-06 13:00               ` David Laight
  0 siblings, 0 replies; 21+ messages in thread
From: David Laight @ 2016-10-06 13:00 UTC (permalink / raw)
  To: 'Joe Perches', Pavel Machek
  Cc: Marcel Holtmann, trivial, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

From: Joe Perches
> Sent: 06 October 2016 12:39
> On Thu, 2016-10-06 at 09:41 +0000, David Laight wrote:
> > From: Joe Perches
> > > No worries, and bool is the same ,size as u8.
> > That is not guaranteed at all.
> > One of the ARM ABI defined bool to be the size of int.
>=20
> Really?  What kernel has sizeof(_Bool) !=3D 1 ?

Probably none, but I know systems have used larger bool.
I found this:
> with egcs-2.90.29 980515 (egcs-1.0.3 release) on alphaev56-dec-osf4.0d

>  bool  =3D 8
>  short =3D 2
>  int   =3D 4=20
>  long  =3D 8

I'm pretty sure something newer than an old alpha ABI used 4 byte bool.

	David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel
  2016-10-06 13:00               ` David Laight
  (?)
@ 2016-10-06 15:41               ` Joe Perches
  -1 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2016-10-06 15:41 UTC (permalink / raw)
  To: David Laight, Pavel Machek
  Cc: Marcel Holtmann, trivial, Gustavo F. Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth, netdev, linux-kernel

On Thu, 2016-10-06 at 13:00 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 06 October 2016 12:39
> > On Thu, 2016-10-06 at 09:41 +0000, David Laight wrote:
> > > From: Joe Perches
> > > > No worries, and bool is the same ,size as u8.
> > > That is not guaranteed at all.
> > > One of the ARM ABI defined bool to be the size of int.
> > Really?  What kernel has sizeof(_Bool) != 1 ?
> Probably none, but I know systems have used larger bool.
> I found this: 
> > with egcs-2.90.29 980515 (egcs-1.0.3 release) on alphaev56-dec-osf4.0d
> >  bool  = 8
> >  short = 2
> >  int   = 4 
> >  long  = 8

It's likely there are probably DSPs and old TOPS-20/CDC-6400
systems where sizeof(u16) isn't 2 as well.

I think linux isn't likely to be ported successfully to
those platforms.

No matter.  If bool isn't desired because some future
expansion to this is likely and memory needs to be conserved,
fine, use a bitfield.

It can be slower than bool because it can be RMW.

cheers, Joe

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2016-10-06 15:41 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-03  8:56 [PATCH] bluetooth.h: __ variants of u8 and friends are not neccessary inside kernel Pavel Machek
2016-10-05 11:14 ` Marcel Holtmann
2016-10-05 17:53   ` Joe Perches
2016-10-05 19:11     ` Pavel Machek
2016-10-05 19:11       ` Pavel Machek
2016-10-05 19:15       ` Joe Perches
2016-10-05 19:15         ` Joe Perches
2016-10-05 22:13         ` Pavel Machek
2016-10-05 22:28           ` Joe Perches
2016-10-05 22:28             ` Joe Perches
2016-10-06  7:02             ` Pavel Machek
2016-10-06  7:07               ` Joe Perches
2016-10-06  7:07                 ` Joe Perches
2016-10-06  8:38                 ` Johan Hedberg
2016-10-06  9:41         ` David Laight
2016-10-06 11:38           ` Joe Perches
2016-10-06 13:00             ` David Laight
2016-10-06 13:00               ` David Laight
2016-10-06 15:41               ` Joe Perches
2016-10-05 20:51 ` [PATCHv2] " Pavel Machek
2016-10-05 21:54   ` Marcel Holtmann

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.