* [PATCH v1 1/5] monitor/rfcomm.c: Add support for MSC frame decoding
2014-12-03 12:01 [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type Gowtham Anandha Babu
@ 2014-12-03 12:01 ` Gowtham Anandha Babu
2014-12-03 12:01 ` [PATCH v1 2/5] monitor/rfcomm: Add support for RPN " Gowtham Anandha Babu
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-03 12:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: d.kasatkin, bharat.panda, cpgs, Gowtham Anandha Babu
Changes made to decode MSC frame in RFCOMM for btmon.
RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
Address: 0x01 cr 0 dlci 0x00
Control: 0xef poll/final 0
Length: 4
FCS: 0xaa
MCC Message type: Modem Status Command CMD(0x38)
Length: 2
dlci 32
fc 0 rtc 1 rtr 1 ic 0 dv 1
---
monitor/rfcomm.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 59 insertions(+), 7 deletions(-)
diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index b3db98b..cd50c8c 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -49,11 +49,19 @@ static char *cr_str[] = {
"CMD"
};
-#define CR_STR(type) cr_str[GET_CR(type)]
-#define GET_LEN8(length) ((length & 0xfe) >> 1)
-#define GET_LEN16(length) ((length & 0xfffe) >> 1)
-#define GET_CR(type) ((type & 0x02) >> 1)
-#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
+/* RFCOMM frame parsing macros */
+#define CR_STR(type) cr_str[GET_CR(type)]
+#define GET_LEN8(length) ((length & 0xfe) >> 1)
+#define GET_LEN16(length) ((length & 0xfffe) >> 1)
+#define GET_CR(type) ((type & 0x02) >> 1)
+#define GET_PF(ctr) (((ctr) >> 4) & 0x1)
+
+/* MSC macros */
+#define GET_V24_FC(sigs) ((sigs & 0x02) >> 1)
+#define GET_V24_RTC(sigs) ((sigs & 0x04) >> 2)
+#define GET_V24_RTR(sigs) ((sigs & 0x08) >> 3)
+#define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
+#define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
struct rfcomm_lhdr {
uint8_t address;
@@ -63,6 +71,12 @@ struct rfcomm_lhdr {
uint8_t credits; /* only for UIH frame */
} __attribute__((packed));
+struct rfcomm_lmsc {
+ uint8_t dlci;
+ uint8_t v24_sig;
+ uint8_t break_sig;
+} __attribute__((packed));
+
struct rfcomm_lmcc {
uint8_t type;
uint16_t length;
@@ -92,6 +106,38 @@ static void print_rfcomm_hdr(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
print_field("%*cFCS: 0x%2.2x", indent, ' ', hdr.fcs);
}
+static inline bool mcc_msc(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+ struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+ struct rfcomm_lmsc msc;
+
+ if (!l2cap_frame_get_u8(frame, &msc.dlci))
+ return false;
+
+ print_field("%*cdlci %d ", indent, ' ', RFCOMM_GET_DLCI(msc.dlci));
+
+ if (!l2cap_frame_get_u8(frame, &msc.v24_sig))
+ return false;
+
+ /* v24 control signals */
+ print_field("%*cfc %d rtc %d rtr %d ic %d dv %d", indent, ' ',
+ GET_V24_FC(msc.v24_sig), GET_V24_RTC(msc.v24_sig),
+ GET_V24_RTR(msc.v24_sig), GET_V24_IC(msc.v24_sig),
+ GET_V24_DV(msc.v24_sig));
+
+ if (frame->size < 2)
+ goto done;
+
+ /*
+ * TODO: Implement the break signals decoding.
+ */
+
+ packet_hexdump(frame->data, frame->size);
+
+done:
+ return true;
+}
+
struct mcc_data {
uint8_t type;
const char *str;
@@ -151,7 +197,13 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
print_field("%*cLength: %d", indent+2, ' ', mcc.length);
rfcomm_frame->mcc = mcc;
- packet_hexdump(frame->data, frame->size);
+
+ switch (type) {
+ case RFCOMM_MSC:
+ return mcc_msc(rfcomm_frame, indent+2);
+ default:
+ packet_hexdump(frame->data, frame->size);
+ }
return true;
}
@@ -225,7 +277,7 @@ void rfcomm_packet(const struct l2cap_frame *frame)
l2cap_frame_pull(&tmp_frame, l2cap_frame, l2cap_frame->size-1);
- if(!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
+ if (!l2cap_frame_get_u8(&tmp_frame, &hdr.fcs))
goto fail;
/* Decoding frame type */
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-03 12:01 [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type Gowtham Anandha Babu
2014-12-03 12:01 ` [PATCH v1 1/5] monitor/rfcomm.c: Add support for MSC frame decoding Gowtham Anandha Babu
@ 2014-12-03 12:01 ` Gowtham Anandha Babu
2014-12-04 15:18 ` Szymon Janc
2014-12-03 12:01 ` [PATCH v1 3/5] monitor/rfcomm: Add support for RLS " Gowtham Anandha Babu
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-03 12:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: d.kasatkin, bharat.panda, cpgs, Gowtham Anandha Babu
Changes made to decode RPN frame in RFCOMM for btmon.
RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
Address: 0x03 cr 1 dlci 0x00
Control: 0xef poll/final 0
Length: 10
FCS: 0x70
MCC Message type: Remote Port Negotiation Command CMD(0x24)
Length: 8
dlci 32
br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
pm 0x3f7f
---
monitor/rfcomm.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index cd50c8c..62b52cc 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -63,6 +63,18 @@ static char *cr_str[] = {
#define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
#define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
+/* RPN macros */
+#define GET_RPN_DB(parity) (parity & 0x03)
+#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
+#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3)
+#define GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
+#define GET_RPN_XIN(io) (io & 0x01)
+#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
+#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
+#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
+#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
+#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
+
struct rfcomm_lhdr {
uint8_t address;
uint8_t control;
@@ -77,6 +89,16 @@ struct rfcomm_lmsc {
uint8_t break_sig;
} __attribute__((packed));
+struct rfcomm_rpn {
+ uint8_t dlci;
+ uint8_t bit_rate;
+ uint8_t parity;
+ uint8_t io;
+ uint8_t xon;
+ uint8_t xoff;
+ uint16_t pm;
+} __attribute__ ((packed));
+
struct rfcomm_lmcc {
uint8_t type;
uint16_t length;
@@ -138,6 +160,55 @@ done:
return true;
}
+static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+ struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+ struct rfcomm_rpn rpn;
+
+ if (!l2cap_frame_get_u8(frame, &rpn.dlci))
+ return false;
+
+ print_field("%*cdlci %d", indent, ' ', RFCOMM_GET_DLCI(rpn.dlci));
+
+ if (frame->size < 7)
+ goto done;
+
+ /* port value octets (optional) */
+
+ if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
+ return false;
+
+ if (!l2cap_frame_get_u8(frame, &rpn.parity))
+ return false;
+
+ if (!l2cap_frame_get_u8(frame, &rpn.io))
+ return false;
+
+ print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo %d", indent, ' ',
+ rpn.bit_rate, GET_RPN_DB(rpn.parity), GET_RPN_SB(rpn.parity),
+ GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
+ GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
+
+ if (!l2cap_frame_get_u8(frame, &rpn.xon))
+ return false;
+
+ if (!l2cap_frame_get_u8(frame, &rpn.xoff))
+ return false;
+
+ print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
+ indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
+ GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
+ rpn.xoff);
+
+ if (!l2cap_frame_get_be16(frame, &rpn.pm))
+ return false;
+
+ print_field("%*cpm 0x%04x", indent, ' ', __bswap_16(rpn.pm));
+
+done:
+ return true;
+}
+
struct mcc_data {
uint8_t type;
const char *str;
@@ -201,6 +272,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
switch (type) {
case RFCOMM_MSC:
return mcc_msc(rfcomm_frame, indent+2);
+ case RFCOMM_RPN:
+ return mcc_rpn(rfcomm_frame, indent+2);
default:
packet_hexdump(frame->data, frame->size);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-03 12:01 ` [PATCH v1 2/5] monitor/rfcomm: Add support for RPN " Gowtham Anandha Babu
@ 2014-12-04 15:18 ` Szymon Janc
2014-12-05 10:44 ` Gowtham Anandha Babu
0 siblings, 1 reply; 18+ messages in thread
From: Szymon Janc @ 2014-12-04 15:18 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: linux-bluetooth@vger.kernel.org, d.kasatkin, bharat.panda, cpgs
Hi Gowtham,
On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Changes made to decode RPN frame in RFCOMM for btmon.
>
> RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> Address: 0x03 cr 1 dlci 0x00
> Control: 0xef poll/final 0
> Length: 10
> FCS: 0x70
> MCC Message type: Remote Port Negotiation Command CMD(0x24)
> Length: 8
> dlci 32
> br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> pm 0x3f7f
> ---
> monitor/rfcomm.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
>
> diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
> index cd50c8c..62b52cc 100644
> --- a/monitor/rfcomm.c
> +++ b/monitor/rfcomm.c
> @@ -63,6 +63,18 @@ static char *cr_str[] = {
> #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
>
> +/* RPN macros */
> +#define GET_RPN_DB(parity) (parity & 0x03)
> +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3)
> +#define GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> +#define GET_RPN_XIN(io) (io & 0x01)
> +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> +
> struct rfcomm_lhdr {
> uint8_t address;
> uint8_t control;
> @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> uint8_t break_sig;
> } __attribute__((packed));
>
> +struct rfcomm_rpn {
> + uint8_t dlci;
> + uint8_t bit_rate;
> + uint8_t parity;
> + uint8_t io;
> + uint8_t xon;
> + uint8_t xoff;
> + uint16_t pm;
> +} __attribute__ ((packed));
> +
> struct rfcomm_lmcc {
> uint8_t type;
> uint16_t length;
> @@ -138,6 +160,55 @@ done:
> return true;
> }
>
> +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
> +{
> + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> + struct rfcomm_rpn rpn;
> +
> + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> + return false;
> +
> + print_field("%*cdlci %d", indent, ' ', RFCOMM_GET_DLCI(rpn.dlci));
> +
> + if (frame->size < 7)
> + goto done;
> +
> + /* port value octets (optional) */
> +
> + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> + return false;
> +
> + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> + return false;
> +
> + if (!l2cap_frame_get_u8(frame, &rpn.io))
> + return false;
> +
> + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo %d", indent, ' ',
> + rpn.bit_rate, GET_RPN_DB(rpn.parity), GET_RPN_SB(rpn.parity),
> + GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
> + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> +
> + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> + return false;
> +
> + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> + return false;
> +
> + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
> + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
> + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
> + rpn.xoff);
> +
> + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> + return false;
> +
> + print_field("%*cpm 0x%04x", indent, ' ', __bswap_16(rpn.pm));
This __bswap_16 is invalid since rpn.pm was already converted to host
order by l2cap_frame_get_be16(). (It wouldn't work correctly on BE
arch anyway).
Also this was causing build problems under Android. If you are able to
build-test under Android please do so. If not then at least point this
in cover letter or patch comment that code was tested only under
Linux).
I've fixed this myself but please pay attention to this with any
future submission.
> +
> +done:
> + return true;
> +}
> +
> struct mcc_data {
> uint8_t type;
> const char *str;
> @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
> switch (type) {
> case RFCOMM_MSC:
> return mcc_msc(rfcomm_frame, indent+2);
> + case RFCOMM_RPN:
> + return mcc_rpn(rfcomm_frame, indent+2);
> default:
> packet_hexdump(frame->data, frame->size);
> }
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
pozdrawiam
Szymon K. Janc
szymon.janc@gmail.com
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-04 15:18 ` Szymon Janc
@ 2014-12-05 10:44 ` Gowtham Anandha Babu
2014-12-05 11:03 ` Szymon Janc
0 siblings, 1 reply; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-05 10:44 UTC (permalink / raw)
To: 'Szymon Janc'; +Cc: linux-bluetooth, d.kasatkin, bharat.panda, cpgs
Hi Szymon,
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Szymon Janc
> Sent: Thursday, December 04, 2014 8:48 PM
> To: Gowtham Anandha Babu
> Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> bharat.panda@samsung.com; cpgs@samsung.com
> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> decoding
>
> Hi Gowtham,
>
> On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Changes made to decode RPN frame in RFCOMM for btmon.
> >
> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > Address: 0x03 cr 1 dlci 0x00
> > Control: 0xef poll/final 0
> > Length: 10
> > FCS: 0x70
> > MCC Message type: Remote Port Negotiation Command CMD(0x24)
> > Length: 8
> > dlci 32
> > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> > pm 0x3f7f
> > ---
> > monitor/rfcomm.c | 73
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 73 insertions(+)
> >
> > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > cd50c8c..62b52cc 100644
> > --- a/monitor/rfcomm.c
> > +++ b/monitor/rfcomm.c
> > @@ -63,6 +63,18 @@ static char *cr_str[] = {
> > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
> >
> > +/* RPN macros */
> > +#define GET_RPN_DB(parity) (parity & 0x03)
> > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
> > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> > +#define GET_RPN_XIN(io) (io & 0x01)
> > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> > +
> > struct rfcomm_lhdr {
> > uint8_t address;
> > uint8_t control;
> > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> > uint8_t break_sig;
> > } __attribute__((packed));
> >
> > +struct rfcomm_rpn {
> > + uint8_t dlci;
> > + uint8_t bit_rate;
> > + uint8_t parity;
> > + uint8_t io;
> > + uint8_t xon;
> > + uint8_t xoff;
> > + uint16_t pm;
> > +} __attribute__ ((packed));
> > +
> > struct rfcomm_lmcc {
> > uint8_t type;
> > uint16_t length;
> > @@ -138,6 +160,55 @@ done:
> > return true;
> > }
> >
> > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame, uint8_t
> > +indent) {
> > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> > + struct rfcomm_rpn rpn;
> > +
> > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> > + return false;
> > +
> > + print_field("%*cdlci %d", indent, ' ',
> > + RFCOMM_GET_DLCI(rpn.dlci));
> > +
> > + if (frame->size < 7)
> > + goto done;
> > +
> > + /* port value octets (optional) */
> > +
> > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> > + return false;
> > +
> > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> > + return false;
> > +
> > + if (!l2cap_frame_get_u8(frame, &rpn.io))
> > + return false;
> > +
> > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo %d",
indent, '
> ',
> > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
GET_RPN_SB(rpn.parity),
> > + GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
> > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> > +
> > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> > + return false;
> > +
> > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> > + return false;
> > +
> > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
> > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
> > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
> > + rpn.xoff);
> > +
> > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> > + return false;
> > +
> > + print_field("%*cpm 0x%04x", indent, ' ', __bswap_16(rpn.pm));
>
> This __bswap_16 is invalid since rpn.pm was already converted to host
order
> by l2cap_frame_get_be16(). (It wouldn't work correctly on BE arch anyway).
>
> Also this was causing build problems under Android. If you are able to
build-
> test under Android please do so. If not then at least point this in cover
letter
> or patch comment that code was tested only under Linux).
>
> I've fixed this myself but please pay attention to this with any future
> submission.
>
Sure I will mention the tested platform details for future submission.
Btw without __bswap_16(), rpn.pm and pn.mtu will be different from the
hcidump logs.
This is captured by removing __bswap_16().
Btmon log:
RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
Address: 0x03 cr 1 dlci 0x00
Control: 0xef poll/final 0
Length: 10
FCS: 0x70
MCC Message type: DLC Parameter Negotiation CMD(0x20)
Length: 8
dlci 6 frame_type 0 credit_flow 15 pri 3
ack_timer 0 frame_size 32256 max_retrans 0 credits 1
Hcidump log:
RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
frame_size 126 max_retrans 0 credits 1
The rpn.pm is fine. That may be printed without doing __bswap_16() also.
But in the above PN frame, frame_size is different from actual hcidump
trace.
I cross checked with PTS logs, it was same as the hcidump trace.
So, if it is causing build problems under Android, use
l2cap_frame_get_le16()
Instead of l2cap_frame_get_be16() for pn.mtu.
If we are consistent with hcidump, then replace the be16() by le16() for
both pn.mtu and rpn.pm.
After replacing the le16() function:
Btmon logs looks like:
RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
Address: 0x03 cr 1 dlci 0x00
Control: 0xef poll/final 0
Length: 10
FCS: 0x70
MCC Message type: DLC Parameter Negotiation CMD(0x20)
Length: 8
dlci 6 frame_type 0 credit_flow 15 pri 3
ack_timer 0 frame_size 126 max_retrans 0 credits 1
What do you think ?
> > +
> > +done:
> > + return true;
> > +}
> > +
> > struct mcc_data {
> > uint8_t type;
> > const char *str;
> > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct rfcomm_frame
> *rfcomm_frame, uint8_t indent)
> > switch (type) {
> > case RFCOMM_MSC:
> > return mcc_msc(rfcomm_frame, indent+2);
> > + case RFCOMM_RPN:
> > + return mcc_rpn(rfcomm_frame, indent+2);
> > default:
> > packet_hexdump(frame->data, frame->size);
> > }
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> pozdrawiam
> Szymon K. Janc
> szymon.janc@gmail.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
in
> the body of a message to majordomo@vger.kernel.org More majordomo
> info at http://vger.kernel.org/majordomo-info.html
Regards,
Gowtham Anandha Babu
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 10:44 ` Gowtham Anandha Babu
@ 2014-12-05 11:03 ` Szymon Janc
2014-12-05 11:10 ` Gowtham Anandha Babu
0 siblings, 1 reply; 18+ messages in thread
From: Szymon Janc @ 2014-12-05 11:03 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: 'Szymon Janc', linux-bluetooth, d.kasatkin, bharat.panda,
cpgs
Hi Gowtham,
On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu wrote:
> Hi Szymon,
>
> > -----Original Message-----
> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > owner@vger.kernel.org] On Behalf Of Szymon Janc
> > Sent: Thursday, December 04, 2014 8:48 PM
> > To: Gowtham Anandha Babu
> > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> > bharat.panda@samsung.com; cpgs@samsung.com
> > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> > decoding
> >
> > Hi Gowtham,
> >
> > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
> > <gowtham.ab@samsung.com> wrote:
> > > Changes made to decode RPN frame in RFCOMM for btmon.
> > >
> > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > Address: 0x03 cr 1 dlci 0x00
> > > Control: 0xef poll/final 0
> > > Length: 10
> > > FCS: 0x70
> > > MCC Message type: Remote Port Negotiation Command CMD(0x24)
> > > Length: 8
> > > dlci 32
> > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> > > pm 0x3f7f
> > > ---
> > > monitor/rfcomm.c | 73
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 73 insertions(+)
> > >
> > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > > cd50c8c..62b52cc 100644
> > > --- a/monitor/rfcomm.c
> > > +++ b/monitor/rfcomm.c
> > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
> > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
> > >
> > > +/* RPN macros */
> > > +#define GET_RPN_DB(parity) (parity & 0x03)
> > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
> > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> > > +#define GET_RPN_XIN(io) (io & 0x01)
> > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> > > +
> > > struct rfcomm_lhdr {
> > > uint8_t address;
> > > uint8_t control;
> > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> > > uint8_t break_sig;
> > > } __attribute__((packed));
> > >
> > > +struct rfcomm_rpn {
> > > + uint8_t dlci;
> > > + uint8_t bit_rate;
> > > + uint8_t parity;
> > > + uint8_t io;
> > > + uint8_t xon;
> > > + uint8_t xoff;
> > > + uint16_t pm;
> > > +} __attribute__ ((packed));
> > > +
> > > struct rfcomm_lmcc {
> > > uint8_t type;
> > > uint16_t length;
> > > @@ -138,6 +160,55 @@ done:
> > > return true;
> > > }
> > >
> > > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame, uint8_t
> > > +indent) {
> > > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> > > + struct rfcomm_rpn rpn;
> > > +
> > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> > > + return false;
> > > +
> > > + print_field("%*cdlci %d", indent, ' ',
> > > + RFCOMM_GET_DLCI(rpn.dlci));
> > > +
> > > + if (frame->size < 7)
> > > + goto done;
> > > +
> > > + /* port value octets (optional) */
> > > +
> > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> > > + return false;
> > > +
> > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> > > + return false;
> > > +
> > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
> > > + return false;
> > > +
> > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo %d",
> indent, '
> > ',
> > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
> GET_RPN_SB(rpn.parity),
> > > + GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
> > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> > > +
> > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> > > + return false;
> > > +
> > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> > > + return false;
> > > +
> > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
> > > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
> > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
> > > + rpn.xoff);
> > > +
> > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> > > + return false;
> > > +
> > > + print_field("%*cpm 0x%04x", indent, ' ', __bswap_16(rpn.pm));
> >
> > This __bswap_16 is invalid since rpn.pm was already converted to host
> order
> > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE arch anyway).
> >
> > Also this was causing build problems under Android. If you are able to
> build-
> > test under Android please do so. If not then at least point this in cover
> letter
> > or patch comment that code was tested only under Linux).
> >
> > I've fixed this myself but please pay attention to this with any future
> > submission.
> >
>
> Sure I will mention the tested platform details for future submission.
>
> Btw without __bswap_16(), rpn.pm and pn.mtu will be different from the
> hcidump logs.
>
> This is captured by removing __bswap_16().
>
> Btmon log:
> RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> Address: 0x03 cr 1 dlci 0x00
> Control: 0xef poll/final 0
> Length: 10
> FCS: 0x70
> MCC Message type: DLC Parameter Negotiation CMD(0x20)
> Length: 8
> dlci 6 frame_type 0 credit_flow 15 pri 3
> ack_timer 0 frame_size 32256 max_retrans 0 credits 1
>
> Hcidump log:
> RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
> dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
> frame_size 126 max_retrans 0 credits 1
>
> The rpn.pm is fine. That may be printed without doing __bswap_16() also.
>
> But in the above PN frame, frame_size is different from actual hcidump
> trace.
> I cross checked with PTS logs, it was same as the hcidump trace.
> So, if it is causing build problems under Android, use
> l2cap_frame_get_le16()
> Instead of l2cap_frame_get_be16() for pn.mtu.
>
> If we are consistent with hcidump, then replace the be16() by le16() for
> both pn.mtu and rpn.pm.
>
> After replacing the le16() function:
> Btmon logs looks like:
>
> RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> Address: 0x03 cr 1 dlci 0x00
> Control: 0xef poll/final 0
> Length: 10
> FCS: 0x70
> MCC Message type: DLC Parameter Negotiation CMD(0x20)
> Length: 8
> dlci 6 frame_type 0 credit_flow 15 pri 3
> ack_timer 0 frame_size 126 max_retrans 0 credits 1
>
> What do you think ?
If value is LE then l2cap_frame_get_le16() should be used.
(Same issue affects rpn.pm mcc_rpn(), right?)
>
> > > +
> > > +done:
> > > + return true;
> > > +}
> > > +
> > > struct mcc_data {
> > > uint8_t type;
> > > const char *str;
> > > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct rfcomm_frame
> > *rfcomm_frame, uint8_t indent)
> > > switch (type) {
> > > case RFCOMM_MSC:
> > > return mcc_msc(rfcomm_frame, indent+2);
> > > + case RFCOMM_RPN:
> > > + return mcc_rpn(rfcomm_frame, indent+2);
> > > default:
> > > packet_hexdump(frame->data, frame->size);
> > > }
> > > --
> > > 1.9.1
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> > > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >
> >
> > --
> > pozdrawiam
> > Szymon K. Janc
> > szymon.janc@gmail.com
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in
> > the body of a message to majordomo@vger.kernel.org More majordomo
> > info at http://vger.kernel.org/majordomo-info.html
>
>
> Regards,
> Gowtham Anandha Babu
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best regards,
Szymon Janc
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 11:03 ` Szymon Janc
@ 2014-12-05 11:10 ` Gowtham Anandha Babu
2014-12-05 11:26 ` Szymon Janc
2014-12-05 11:47 ` Luiz Augusto von Dentz
0 siblings, 2 replies; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-05 11:10 UTC (permalink / raw)
To: 'Szymon Janc'; +Cc: linux-bluetooth, d.kasatkin, bharat.panda, cpgs
Hi Szymon,
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Szymon Janc
> Sent: Friday, December 05, 2014 4:33 PM
> To: Gowtham Anandha Babu
> Cc: 'Szymon Janc'; linux-bluetooth@vger.kernel.org;
> d.kasatkin@samsung.com; bharat.panda@samsung.com;
> cpgs@samsung.com
> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> decoding
>
> Hi Gowtham,
>
> On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu wrote:
> > Hi Szymon,
> >
> > > -----Original Message-----
> > > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > > owner@vger.kernel.org] On Behalf Of Szymon Janc
> > > Sent: Thursday, December 04, 2014 8:48 PM
> > > To: Gowtham Anandha Babu
> > > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> > > bharat.panda@samsung.com; cpgs@samsung.com
> > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
> > > frame decoding
> > >
> > > Hi Gowtham,
> > >
> > > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
> > > <gowtham.ab@samsung.com> wrote:
> > > > Changes made to decode RPN frame in RFCOMM for btmon.
> > > >
> > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > > Address: 0x03 cr 1 dlci 0x00
> > > > Control: 0xef poll/final 0
> > > > Length: 10
> > > > FCS: 0x70
> > > > MCC Message type: Remote Port Negotiation Command
> CMD(0x24)
> > > > Length: 8
> > > > dlci 32
> > > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> > > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> > > > pm 0x3f7f
> > > > ---
> > > > monitor/rfcomm.c | 73
> > > >
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > 1 file changed, 73 insertions(+)
> > > >
> > > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > > > cd50c8c..62b52cc 100644
> > > > --- a/monitor/rfcomm.c
> > > > +++ b/monitor/rfcomm.c
> > > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
> > > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> > > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
> > > >
> > > > +/* RPN macros */
> > > > +#define GET_RPN_DB(parity) (parity & 0x03)
> > > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> > > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
> > > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> > > > +#define GET_RPN_XIN(io) (io & 0x01)
> > > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> > > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> > > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> > > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> > > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> > > > +
> > > > struct rfcomm_lhdr {
> > > > uint8_t address;
> > > > uint8_t control;
> > > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> > > > uint8_t break_sig;
> > > > } __attribute__((packed));
> > > >
> > > > +struct rfcomm_rpn {
> > > > + uint8_t dlci;
> > > > + uint8_t bit_rate;
> > > > + uint8_t parity;
> > > > + uint8_t io;
> > > > + uint8_t xon;
> > > > + uint8_t xoff;
> > > > + uint16_t pm;
> > > > +} __attribute__ ((packed));
> > > > +
> > > > struct rfcomm_lmcc {
> > > > uint8_t type;
> > > > uint16_t length;
> > > > @@ -138,6 +160,55 @@ done:
> > > > return true;
> > > > }
> > > >
> > > > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame,
> > > > +uint8_t
> > > > +indent) {
> > > > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> > > > + struct rfcomm_rpn rpn;
> > > > +
> > > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> > > > + return false;
> > > > +
> > > > + print_field("%*cdlci %d", indent, ' ',
> > > > + RFCOMM_GET_DLCI(rpn.dlci));
> > > > +
> > > > + if (frame->size < 7)
> > > > + goto done;
> > > > +
> > > > + /* port value octets (optional) */
> > > > +
> > > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> > > > + return false;
> > > > +
> > > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> > > > + return false;
> > > > +
> > > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
> > > > + return false;
> > > > +
> > > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo %d",
> > indent, '
> > > ',
> > > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
> > GET_RPN_SB(rpn.parity),
> > > > + GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
> > > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> > > > +
> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> > > > + return false;
> > > > +
> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> > > > + return false;
> > > > +
> > > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
> > > > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
> > > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
> > > > + rpn.xoff);
> > > > +
> > > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> > > > + return false;
> > > > +
> > > > + print_field("%*cpm 0x%04x", indent, ' ',
> > > > + __bswap_16(rpn.pm));
> > >
> > > This __bswap_16 is invalid since rpn.pm was already converted to
> > > host
> > order
> > > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE arch
> anyway).
> > >
> > > Also this was causing build problems under Android. If you are able
> > > to
> > build-
> > > test under Android please do so. If not then at least point this in
> > > cover
> > letter
> > > or patch comment that code was tested only under Linux).
> > >
> > > I've fixed this myself but please pay attention to this with any
> > > future submission.
> > >
> >
> > Sure I will mention the tested platform details for future submission.
> >
> > Btw without __bswap_16(), rpn.pm and pn.mtu will be different from the
> > hcidump logs.
> >
> > This is captured by removing __bswap_16().
> >
> > Btmon log:
> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > Address: 0x03 cr 1 dlci 0x00
> > Control: 0xef poll/final 0
> > Length: 10
> > FCS: 0x70
> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> > Length: 8
> > dlci 6 frame_type 0 credit_flow 15 pri 3
> > ack_timer 0 frame_size 32256 max_retrans 0 credits 1
> >
> > Hcidump log:
> > RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
> > dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
> > frame_size 126 max_retrans 0 credits 1
> >
> > The rpn.pm is fine. That may be printed without doing __bswap_16() also.
> >
> > But in the above PN frame, frame_size is different from actual hcidump
> > trace.
> > I cross checked with PTS logs, it was same as the hcidump trace.
> > So, if it is causing build problems under Android, use
> > l2cap_frame_get_le16()
> > Instead of l2cap_frame_get_be16() for pn.mtu.
> >
> > If we are consistent with hcidump, then replace the be16() by le16()
> > for both pn.mtu and rpn.pm.
> >
> > After replacing the le16() function:
> > Btmon logs looks like:
> >
> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > Address: 0x03 cr 1 dlci 0x00
> > Control: 0xef poll/final 0
> > Length: 10
> > FCS: 0x70
> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> > Length: 8
> > dlci 6 frame_type 0 credit_flow 15 pri 3
> > ack_timer 0 frame_size 126 max_retrans 0 credits 1
> >
> > What do you think ?
>
> If value is LE then l2cap_frame_get_le16() should be used.
> (Same issue affects rpn.pm mcc_rpn(), right?)
>
Yes you are right. We have to use l2cap_frame_get_le16() for fetching both rpn.pm and pn.mtu.
I hope it will not cause any Android build problem.
> >
> > > > +
> > > > +done:
> > > > + return true;
> > > > +}
> > > > +
> > > > struct mcc_data {
> > > > uint8_t type;
> > > > const char *str;
> > > > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct
> > > > rfcomm_frame
> > > *rfcomm_frame, uint8_t indent)
> > > > switch (type) {
> > > > case RFCOMM_MSC:
> > > > return mcc_msc(rfcomm_frame, indent+2);
> > > > + case RFCOMM_RPN:
> > > > + return mcc_rpn(rfcomm_frame, indent+2);
> > > > default:
> > > > packet_hexdump(frame->data, frame->size);
> > > > }
> > > > --
> > > > 1.9.1
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe
> > > > linux-bluetooth" in the body of a message to
> > > > majordomo@vger.kernel.org More majordomo info at
> > > > http://vger.kernel.org/majordomo-info.html
> > >
> > >
> > >
> > > --
> > > pozdrawiam
> > > Szymon K. Janc
> > > szymon.janc@gmail.com
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> > in
> > > the body of a message to majordomo@vger.kernel.org More majordomo
> > > info at http://vger.kernel.org/majordomo-info.html
> >
> >
> > Regards,
> > Gowtham Anandha Babu
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Best regards,
> Szymon Janc
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org More majordomo
> info at http://vger.kernel.org/majordomo-info.html
Regards,
Gowtham Anandha Babu
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 11:10 ` Gowtham Anandha Babu
@ 2014-12-05 11:26 ` Szymon Janc
2014-12-05 13:02 ` Gowtham Anandha Babu
2014-12-05 11:47 ` Luiz Augusto von Dentz
1 sibling, 1 reply; 18+ messages in thread
From: Szymon Janc @ 2014-12-05 11:26 UTC (permalink / raw)
To: Gowtham Anandha Babu; +Cc: linux-bluetooth, d.kasatkin, bharat.panda, cpgs
Hi Gowtham,
On Friday 05 of December 2014 16:40:52 Gowtham Anandha Babu wrote:
> Hi Szymon,
>
> > -----Original Message-----
> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > owner@vger.kernel.org] On Behalf Of Szymon Janc
> > Sent: Friday, December 05, 2014 4:33 PM
> > To: Gowtham Anandha Babu
> > Cc: 'Szymon Janc'; linux-bluetooth@vger.kernel.org;
> > d.kasatkin@samsung.com; bharat.panda@samsung.com;
> > cpgs@samsung.com
> > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> > decoding
> >
> > Hi Gowtham,
> >
> > On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu wrote:
> > > Hi Szymon,
> > >
> > > > -----Original Message-----
> > > > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > > > owner@vger.kernel.org] On Behalf Of Szymon Janc
> > > > Sent: Thursday, December 04, 2014 8:48 PM
> > > > To: Gowtham Anandha Babu
> > > > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> > > > bharat.panda@samsung.com; cpgs@samsung.com
> > > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
> > > > frame decoding
> > > >
> > > > Hi Gowtham,
> > > >
> > > > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
> > > > <gowtham.ab@samsung.com> wrote:
> > > > > Changes made to decode RPN frame in RFCOMM for btmon.
> > > > >
> > > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > > > Address: 0x03 cr 1 dlci 0x00
> > > > > Control: 0xef poll/final 0
> > > > > Length: 10
> > > > > FCS: 0x70
> > > > > MCC Message type: Remote Port Negotiation Command
> > CMD(0x24)
> > > > > Length: 8
> > > > > dlci 32
> > > > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> > > > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> > > > > pm 0x3f7f
> > > > > ---
> > > > > monitor/rfcomm.c | 73
> > > > >
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > 1 file changed, 73 insertions(+)
> > > > >
> > > > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > > > > cd50c8c..62b52cc 100644
> > > > > --- a/monitor/rfcomm.c
> > > > > +++ b/monitor/rfcomm.c
> > > > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
> > > > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> > > > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
> > > > >
> > > > > +/* RPN macros */
> > > > > +#define GET_RPN_DB(parity) (parity & 0x03)
> > > > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> > > > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
> > > > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> > > > > +#define GET_RPN_XIN(io) (io & 0x01)
> > > > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> > > > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> > > > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> > > > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> > > > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> > > > > +
> > > > > struct rfcomm_lhdr {
> > > > > uint8_t address;
> > > > > uint8_t control;
> > > > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> > > > > uint8_t break_sig;
> > > > > } __attribute__((packed));
> > > > >
> > > > > +struct rfcomm_rpn {
> > > > > + uint8_t dlci;
> > > > > + uint8_t bit_rate;
> > > > > + uint8_t parity;
> > > > > + uint8_t io;
> > > > > + uint8_t xon;
> > > > > + uint8_t xoff;
> > > > > + uint16_t pm;
> > > > > +} __attribute__ ((packed));
> > > > > +
> > > > > struct rfcomm_lmcc {
> > > > > uint8_t type;
> > > > > uint16_t length;
> > > > > @@ -138,6 +160,55 @@ done:
> > > > > return true;
> > > > > }
> > > > >
> > > > > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame,
> > > > > +uint8_t
> > > > > +indent) {
> > > > > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> > > > > + struct rfcomm_rpn rpn;
> > > > > +
> > > > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> > > > > + return false;
> > > > > +
> > > > > + print_field("%*cdlci %d", indent, ' ',
> > > > > + RFCOMM_GET_DLCI(rpn.dlci));
> > > > > +
> > > > > + if (frame->size < 7)
> > > > > + goto done;
> > > > > +
> > > > > + /* port value octets (optional) */
> > > > > +
> > > > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> > > > > + return false;
> > > > > +
> > > > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> > > > > + return false;
> > > > > +
> > > > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
> > > > > + return false;
> > > > > +
> > > > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo %d",
> > > indent, '
> > > > ',
> > > > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
> > > GET_RPN_SB(rpn.parity),
> > > > > + GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
> > > > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> > > > > +
> > > > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> > > > > + return false;
> > > > > +
> > > > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> > > > > + return false;
> > > > > +
> > > > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
> > > > > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
> > > > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
> > > > > + rpn.xoff);
> > > > > +
> > > > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> > > > > + return false;
> > > > > +
> > > > > + print_field("%*cpm 0x%04x", indent, ' ',
> > > > > + __bswap_16(rpn.pm));
> > > >
> > > > This __bswap_16 is invalid since rpn.pm was already converted to
> > > > host
> > > order
> > > > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE arch
> > anyway).
> > > >
> > > > Also this was causing build problems under Android. If you are able
> > > > to
> > > build-
> > > > test under Android please do so. If not then at least point this in
> > > > cover
> > > letter
> > > > or patch comment that code was tested only under Linux).
> > > >
> > > > I've fixed this myself but please pay attention to this with any
> > > > future submission.
> > > >
> > >
> > > Sure I will mention the tested platform details for future submission.
> > >
> > > Btw without __bswap_16(), rpn.pm and pn.mtu will be different from the
> > > hcidump logs.
> > >
> > > This is captured by removing __bswap_16().
> > >
> > > Btmon log:
> > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > Address: 0x03 cr 1 dlci 0x00
> > > Control: 0xef poll/final 0
> > > Length: 10
> > > FCS: 0x70
> > > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> > > Length: 8
> > > dlci 6 frame_type 0 credit_flow 15 pri 3
> > > ack_timer 0 frame_size 32256 max_retrans 0 credits 1
> > >
> > > Hcidump log:
> > > RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
> > > dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
> > > frame_size 126 max_retrans 0 credits 1
> > >
> > > The rpn.pm is fine. That may be printed without doing __bswap_16() also.
> > >
> > > But in the above PN frame, frame_size is different from actual hcidump
> > > trace.
> > > I cross checked with PTS logs, it was same as the hcidump trace.
> > > So, if it is causing build problems under Android, use
> > > l2cap_frame_get_le16()
> > > Instead of l2cap_frame_get_be16() for pn.mtu.
> > >
> > > If we are consistent with hcidump, then replace the be16() by le16()
> > > for both pn.mtu and rpn.pm.
> > >
> > > After replacing the le16() function:
> > > Btmon logs looks like:
> > >
> > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > Address: 0x03 cr 1 dlci 0x00
> > > Control: 0xef poll/final 0
> > > Length: 10
> > > FCS: 0x70
> > > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> > > Length: 8
> > > dlci 6 frame_type 0 credit_flow 15 pri 3
> > > ack_timer 0 frame_size 126 max_retrans 0 credits 1
> > >
> > > What do you think ?
> >
> > If value is LE then l2cap_frame_get_le16() should be used.
> > (Same issue affects rpn.pm mcc_rpn(), right?)
> >
>
> Yes you are right. We have to use l2cap_frame_get_le16() for fetching both rpn.pm and pn.mtu.
> I hope it will not cause any Android build problem.
Should be fine, I'll try to build-test monitor patches on Android before they
get applied anyway.
>
> > >
> > > > > +
> > > > > +done:
> > > > > + return true;
> > > > > +}
> > > > > +
> > > > > struct mcc_data {
> > > > > uint8_t type;
> > > > > const char *str;
> > > > > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct
> > > > > rfcomm_frame
> > > > *rfcomm_frame, uint8_t indent)
> > > > > switch (type) {
> > > > > case RFCOMM_MSC:
> > > > > return mcc_msc(rfcomm_frame, indent+2);
> > > > > + case RFCOMM_RPN:
> > > > > + return mcc_rpn(rfcomm_frame, indent+2);
> > > > > default:
> > > > > packet_hexdump(frame->data, frame->size);
> > > > > }
> > > > > --
> > > > > 1.9.1
> > > > >
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe
> > > > > linux-bluetooth" in the body of a message to
> > > > > majordomo@vger.kernel.org More majordomo info at
> > > > > http://vger.kernel.org/majordomo-info.html
> > > >
> > > >
> > > >
> > > > --
> > > > pozdrawiam
> > > > Szymon K. Janc
> > > > szymon.janc@gmail.com
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> > > in
> > > > the body of a message to majordomo@vger.kernel.org More majordomo
> > > > info at http://vger.kernel.org/majordomo-info.html
> > >
> > >
> > > Regards,
> > > Gowtham Anandha Babu
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> > > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > --
> > Best regards,
> > Szymon Janc
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> > the body of a message to majordomo@vger.kernel.org More majordomo
> > info at http://vger.kernel.org/majordomo-info.html
>
> Regards,
> Gowtham Anandha Babu
>
--
Best regards,
Szymon Janc
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 11:26 ` Szymon Janc
@ 2014-12-05 13:02 ` Gowtham Anandha Babu
2014-12-05 13:09 ` Szymon Janc
0 siblings, 1 reply; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-05 13:02 UTC (permalink / raw)
To: 'Szymon Janc'; +Cc: linux-bluetooth, d.kasatkin, bharat.panda, cpgs
Hi Szymon,
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Szymon Janc
> Sent: Friday, December 05, 2014 4:57 PM
> To: Gowtham Anandha Babu
> Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> bharat.panda@samsung.com; cpgs@samsung.com
> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> decoding
>
> Hi Gowtham,
>
> On Friday 05 of December 2014 16:40:52 Gowtham Anandha Babu wrote:
> > Hi Szymon,
> >
> > > -----Original Message-----
> > > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > > owner@vger.kernel.org] On Behalf Of Szymon Janc
> > > Sent: Friday, December 05, 2014 4:33 PM
> > > To: Gowtham Anandha Babu
> > > Cc: 'Szymon Janc'; linux-bluetooth@vger.kernel.org;
> > > d.kasatkin@samsung.com; bharat.panda@samsung.com;
> cpgs@samsung.com
> > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
> > > frame decoding
> > >
> > > Hi Gowtham,
> > >
> > > On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu wrote:
> > > > Hi Szymon,
> > > >
> > > > > -----Original Message-----
> > > > > From: linux-bluetooth-owner@vger.kernel.org
> > > > > [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of
> > > > > Szymon Janc
> > > > > Sent: Thursday, December 04, 2014 8:48 PM
> > > > > To: Gowtham Anandha Babu
> > > > > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> > > > > bharat.panda@samsung.com; cpgs@samsung.com
> > > > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
> > > > > frame decoding
> > > > >
> > > > > Hi Gowtham,
> > > > >
> > > > > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
> > > > > <gowtham.ab@samsung.com> wrote:
> > > > > > Changes made to decode RPN frame in RFCOMM for btmon.
> > > > > >
> > > > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > > > > Address: 0x03 cr 1 dlci 0x00
> > > > > > Control: 0xef poll/final 0
> > > > > > Length: 10
> > > > > > FCS: 0x70
> > > > > > MCC Message type: Remote Port Negotiation Command
> > > CMD(0x24)
> > > > > > Length: 8
> > > > > > dlci 32
> > > > > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> > > > > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> > > > > > pm 0x3f7f
> > > > > > ---
> > > > > > monitor/rfcomm.c | 73
> > > > > >
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 73 insertions(+)
> > > > > >
> > > > > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > > > > > cd50c8c..62b52cc 100644
> > > > > > --- a/monitor/rfcomm.c
> > > > > > +++ b/monitor/rfcomm.c
> > > > > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
> > > > > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> > > > > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
> > > > > >
> > > > > > +/* RPN macros */
> > > > > > +#define GET_RPN_DB(parity) (parity & 0x03)
> > > > > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> > > > > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
> > > > > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> > > > > > +#define GET_RPN_XIN(io) (io & 0x01)
> > > > > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> > > > > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> > > > > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> > > > > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> > > > > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> > > > > > +
> > > > > > struct rfcomm_lhdr {
> > > > > > uint8_t address;
> > > > > > uint8_t control;
> > > > > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> > > > > > uint8_t break_sig;
> > > > > > } __attribute__((packed));
> > > > > >
> > > > > > +struct rfcomm_rpn {
> > > > > > + uint8_t dlci;
> > > > > > + uint8_t bit_rate;
> > > > > > + uint8_t parity;
> > > > > > + uint8_t io;
> > > > > > + uint8_t xon;
> > > > > > + uint8_t xoff;
> > > > > > + uint16_t pm;
> > > > > > +} __attribute__ ((packed));
> > > > > > +
> > > > > > struct rfcomm_lmcc {
> > > > > > uint8_t type;
> > > > > > uint16_t length;
> > > > > > @@ -138,6 +160,55 @@ done:
> > > > > > return true;
> > > > > > }
> > > > > >
> > > > > > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame,
> > > > > > +uint8_t
> > > > > > +indent) {
> > > > > > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> > > > > > + struct rfcomm_rpn rpn;
> > > > > > +
> > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> > > > > > + return false;
> > > > > > +
> > > > > > + print_field("%*cdlci %d", indent, ' ',
> > > > > > + RFCOMM_GET_DLCI(rpn.dlci));
> > > > > > +
> > > > > > + if (frame->size < 7)
> > > > > > + goto done;
> > > > > > +
> > > > > > + /* port value octets (optional) */
> > > > > > +
> > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> > > > > > + return false;
> > > > > > +
> > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> > > > > > + return false;
> > > > > > +
> > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
> > > > > > + return false;
> > > > > > +
> > > > > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo
> > > > > > + %d",
> > > > indent, '
> > > > > ',
> > > > > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
> > > > GET_RPN_SB(rpn.parity),
> > > > > > + GET_RPN_PARITY(rpn.parity),
> GET_RPN_PTYPE(rpn.parity),
> > > > > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> > > > > > +
> > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> > > > > > + return false;
> > > > > > +
> > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> > > > > > + return false;
> > > > > > +
> > > > > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff
> %d",
> > > > > > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
> > > > > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
> > > > > > + rpn.xoff);
> > > > > > +
> > > > > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> > > > > > + return false;
> > > > > > +
> > > > > > + print_field("%*cpm 0x%04x", indent, ' ',
> > > > > > + __bswap_16(rpn.pm));
> > > > >
> > > > > This __bswap_16 is invalid since rpn.pm was already converted to
> > > > > host
> > > > order
> > > > > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE
> > > > > arch
> > > anyway).
> > > > >
> > > > > Also this was causing build problems under Android. If you are
> > > > > able to
> > > > build-
> > > > > test under Android please do so. If not then at least point this
> > > > > in cover
> > > > letter
> > > > > or patch comment that code was tested only under Linux).
> > > > >
> > > > > I've fixed this myself but please pay attention to this with any
> > > > > future submission.
> > > > >
> > > >
> > > > Sure I will mention the tested platform details for future submission.
> > > >
> > > > Btw without __bswap_16(), rpn.pm and pn.mtu will be different from
> > > > the hcidump logs.
> > > >
> > > > This is captured by removing __bswap_16().
> > > >
> > > > Btmon log:
> > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > > Address: 0x03 cr 1 dlci 0x00
> > > > Control: 0xef poll/final 0
> > > > Length: 10
> > > > FCS: 0x70
> > > > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> > > > Length: 8
> > > > dlci 6 frame_type 0 credit_flow 15 pri 3
> > > > ack_timer 0 frame_size 32256 max_retrans 0 credits 1
> > > >
> > > > Hcidump log:
> > > > RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
> > > > dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
> > > > frame_size 126 max_retrans 0 credits 1
> > > >
> > > > The rpn.pm is fine. That may be printed without doing __bswap_16()
> also.
> > > >
> > > > But in the above PN frame, frame_size is different from actual
> > > > hcidump trace.
> > > > I cross checked with PTS logs, it was same as the hcidump trace.
> > > > So, if it is causing build problems under Android, use
> > > > l2cap_frame_get_le16()
> > > > Instead of l2cap_frame_get_be16() for pn.mtu.
> > > >
> > > > If we are consistent with hcidump, then replace the be16() by
> > > > le16() for both pn.mtu and rpn.pm.
> > > >
> > > > After replacing the le16() function:
> > > > Btmon logs looks like:
> > > >
> > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > > Address: 0x03 cr 1 dlci 0x00
> > > > Control: 0xef poll/final 0
> > > > Length: 10
> > > > FCS: 0x70
> > > > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> > > > Length: 8
> > > > dlci 6 frame_type 0 credit_flow 15 pri 3
> > > > ack_timer 0 frame_size 126 max_retrans 0 credits 1
> > > >
> > > > What do you think ?
> > >
> > > If value is LE then l2cap_frame_get_le16() should be used.
> > > (Same issue affects rpn.pm mcc_rpn(), right?)
> > >
> >
> > Yes you are right. We have to use l2cap_frame_get_le16() for fetching both
> rpn.pm and pn.mtu.
> > I hope it will not cause any Android build problem.
>
> Should be fine, I'll try to build-test monitor patches on Android before they
> get applied anyway.
>
Thanks for testing it in android.
As Luiz mentioned, the correct byte ordering for RFCOMM protocol is little endian.
Btw shall we submit patches on android? Right now we don’t have the setup for testing.
Probably someone should test it after submitting. Is that ok?
> >
> > > >
> > > > > > +
> > > > > > +done:
> > > > > > + return true;
> > > > > > +}
> > > > > > +
> > > > > > struct mcc_data {
> > > > > > uint8_t type;
> > > > > > const char *str;
> > > > > > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct
> > > > > > rfcomm_frame
> > > > > *rfcomm_frame, uint8_t indent)
> > > > > > switch (type) {
> > > > > > case RFCOMM_MSC:
> > > > > > return mcc_msc(rfcomm_frame, indent+2);
> > > > > > + case RFCOMM_RPN:
> > > > > > + return mcc_rpn(rfcomm_frame, indent+2);
> > > > > > default:
> > > > > > packet_hexdump(frame->data, frame->size);
> > > > > > }
> > > > > > --
> > > > > > 1.9.1
> > > > > >
> > > > > > --
> > > > > > To unsubscribe from this list: send the line "unsubscribe
> > > > > > linux-bluetooth" in the body of a message to
> > > > > > majordomo@vger.kernel.org More majordomo info at
> > > > > > http://vger.kernel.org/majordomo-info.html
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > pozdrawiam
> > > > > Szymon K. Janc
> > > > > szymon.janc@gmail.com
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe linux-
> bluetooth"
> > > > in
> > > > > the body of a message to majordomo@vger.kernel.org More
> > > > > majordomo info at http://vger.kernel.org/majordomo-info.html
> > > >
> > > >
> > > > Regards,
> > > > Gowtham Anandha Babu
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe
> > > > linux-bluetooth" in the body of a message to
> > > > majordomo@vger.kernel.org More majordomo info at
> > > > http://vger.kernel.org/majordomo-info.html
> > >
> > > --
> > > Best regards,
> > > Szymon Janc
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe
> > > linux-bluetooth" in the body of a message to
> > > majordomo@vger.kernel.org More majordomo info at
> > > http://vger.kernel.org/majordomo-info.html
> >
> > Regards,
> > Gowtham Anandha Babu
> >
>
> --
> Best regards,
> Szymon Janc
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org More majordomo
> info at http://vger.kernel.org/majordomo-info.html
Regards,
Gowtham Anandha Babu
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 13:02 ` Gowtham Anandha Babu
@ 2014-12-05 13:09 ` Szymon Janc
0 siblings, 0 replies; 18+ messages in thread
From: Szymon Janc @ 2014-12-05 13:09 UTC (permalink / raw)
To: Gowtham Anandha Babu; +Cc: linux-bluetooth, d.kasatkin, bharat.panda, cpgs
Hi Gowtham,
On Friday 05 of December 2014 18:32:51 Gowtham Anandha Babu wrote:
> Hi Szymon,
>
> > -----Original Message-----
> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > owner@vger.kernel.org] On Behalf Of Szymon Janc
> > Sent: Friday, December 05, 2014 4:57 PM
> > To: Gowtham Anandha Babu
> > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> > bharat.panda@samsung.com; cpgs@samsung.com
> > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> > decoding
> >
> > Hi Gowtham,
> >
> > On Friday 05 of December 2014 16:40:52 Gowtham Anandha Babu wrote:
> > > Hi Szymon,
> > >
> > > > -----Original Message-----
> > > > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > > > owner@vger.kernel.org] On Behalf Of Szymon Janc
> > > > Sent: Friday, December 05, 2014 4:33 PM
> > > > To: Gowtham Anandha Babu
> > > > Cc: 'Szymon Janc'; linux-bluetooth@vger.kernel.org;
> > > > d.kasatkin@samsung.com; bharat.panda@samsung.com;
> > cpgs@samsung.com
> > > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
> > > > frame decoding
> > > >
> > > > Hi Gowtham,
> > > >
> > > > On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu wrote:
> > > > > Hi Szymon,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: linux-bluetooth-owner@vger.kernel.org
> > > > > > [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of
> > > > > > Szymon Janc
> > > > > > Sent: Thursday, December 04, 2014 8:48 PM
> > > > > > To: Gowtham Anandha Babu
> > > > > > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> > > > > > bharat.panda@samsung.com; cpgs@samsung.com
> > > > > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
> > > > > > frame decoding
> > > > > >
> > > > > > Hi Gowtham,
> > > > > >
> > > > > > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
> > > > > > <gowtham.ab@samsung.com> wrote:
> > > > > > > Changes made to decode RPN frame in RFCOMM for btmon.
> > > > > > >
> > > > > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > > > > > Address: 0x03 cr 1 dlci 0x00
> > > > > > > Control: 0xef poll/final 0
> > > > > > > Length: 10
> > > > > > > FCS: 0x70
> > > > > > > MCC Message type: Remote Port Negotiation Command
> > > > CMD(0x24)
> > > > > > > Length: 8
> > > > > > > dlci 32
> > > > > > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> > > > > > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> > > > > > > pm 0x3f7f
> > > > > > > ---
> > > > > > > monitor/rfcomm.c | 73
> > > > > > >
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > > 1 file changed, 73 insertions(+)
> > > > > > >
> > > > > > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> > > > > > > cd50c8c..62b52cc 100644
> > > > > > > --- a/monitor/rfcomm.c
> > > > > > > +++ b/monitor/rfcomm.c
> > > > > > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
> > > > > > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> > > > > > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
> > > > > > >
> > > > > > > +/* RPN macros */
> > > > > > > +#define GET_RPN_DB(parity) (parity & 0x03)
> > > > > > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> > > > > > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
> > > > > > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> > > > > > > +#define GET_RPN_XIN(io) (io & 0x01)
> > > > > > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> > > > > > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> > > > > > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> > > > > > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> > > > > > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> > > > > > > +
> > > > > > > struct rfcomm_lhdr {
> > > > > > > uint8_t address;
> > > > > > > uint8_t control;
> > > > > > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> > > > > > > uint8_t break_sig;
> > > > > > > } __attribute__((packed));
> > > > > > >
> > > > > > > +struct rfcomm_rpn {
> > > > > > > + uint8_t dlci;
> > > > > > > + uint8_t bit_rate;
> > > > > > > + uint8_t parity;
> > > > > > > + uint8_t io;
> > > > > > > + uint8_t xon;
> > > > > > > + uint8_t xoff;
> > > > > > > + uint16_t pm;
> > > > > > > +} __attribute__ ((packed));
> > > > > > > +
> > > > > > > struct rfcomm_lmcc {
> > > > > > > uint8_t type;
> > > > > > > uint16_t length;
> > > > > > > @@ -138,6 +160,55 @@ done:
> > > > > > > return true;
> > > > > > > }
> > > > > > >
> > > > > > > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame,
> > > > > > > +uint8_t
> > > > > > > +indent) {
> > > > > > > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> > > > > > > + struct rfcomm_rpn rpn;
> > > > > > > +
> > > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + print_field("%*cdlci %d", indent, ' ',
> > > > > > > + RFCOMM_GET_DLCI(rpn.dlci));
> > > > > > > +
> > > > > > > + if (frame->size < 7)
> > > > > > > + goto done;
> > > > > > > +
> > > > > > > + /* port value octets (optional) */
> > > > > > > +
> > > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo
> > > > > > > + %d",
> > > > > indent, '
> > > > > > ',
> > > > > > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
> > > > > GET_RPN_SB(rpn.parity),
> > > > > > > + GET_RPN_PARITY(rpn.parity),
> > GET_RPN_PTYPE(rpn.parity),
> > > > > > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> > > > > > > +
> > > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff
> > %d",
> > > > > > > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
> > > > > > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
> > > > > > > + rpn.xoff);
> > > > > > > +
> > > > > > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> > > > > > > + return false;
> > > > > > > +
> > > > > > > + print_field("%*cpm 0x%04x", indent, ' ',
> > > > > > > + __bswap_16(rpn.pm));
> > > > > >
> > > > > > This __bswap_16 is invalid since rpn.pm was already converted to
> > > > > > host
> > > > > order
> > > > > > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE
> > > > > > arch
> > > > anyway).
> > > > > >
> > > > > > Also this was causing build problems under Android. If you are
> > > > > > able to
> > > > > build-
> > > > > > test under Android please do so. If not then at least point this
> > > > > > in cover
> > > > > letter
> > > > > > or patch comment that code was tested only under Linux).
> > > > > >
> > > > > > I've fixed this myself but please pay attention to this with any
> > > > > > future submission.
> > > > > >
> > > > >
> > > > > Sure I will mention the tested platform details for future submission.
> > > > >
> > > > > Btw without __bswap_16(), rpn.pm and pn.mtu will be different from
> > > > > the hcidump logs.
> > > > >
> > > > > This is captured by removing __bswap_16().
> > > > >
> > > > > Btmon log:
> > > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > > > Address: 0x03 cr 1 dlci 0x00
> > > > > Control: 0xef poll/final 0
> > > > > Length: 10
> > > > > FCS: 0x70
> > > > > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> > > > > Length: 8
> > > > > dlci 6 frame_type 0 credit_flow 15 pri 3
> > > > > ack_timer 0 frame_size 32256 max_retrans 0 credits 1
> > > > >
> > > > > Hcidump log:
> > > > > RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
> > > > > dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
> > > > > frame_size 126 max_retrans 0 credits 1
> > > > >
> > > > > The rpn.pm is fine. That may be printed without doing __bswap_16()
> > also.
> > > > >
> > > > > But in the above PN frame, frame_size is different from actual
> > > > > hcidump trace.
> > > > > I cross checked with PTS logs, it was same as the hcidump trace.
> > > > > So, if it is causing build problems under Android, use
> > > > > l2cap_frame_get_le16()
> > > > > Instead of l2cap_frame_get_be16() for pn.mtu.
> > > > >
> > > > > If we are consistent with hcidump, then replace the be16() by
> > > > > le16() for both pn.mtu and rpn.pm.
> > > > >
> > > > > After replacing the le16() function:
> > > > > Btmon logs looks like:
> > > > >
> > > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> > > > > Address: 0x03 cr 1 dlci 0x00
> > > > > Control: 0xef poll/final 0
> > > > > Length: 10
> > > > > FCS: 0x70
> > > > > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> > > > > Length: 8
> > > > > dlci 6 frame_type 0 credit_flow 15 pri 3
> > > > > ack_timer 0 frame_size 126 max_retrans 0 credits 1
> > > > >
> > > > > What do you think ?
> > > >
> > > > If value is LE then l2cap_frame_get_le16() should be used.
> > > > (Same issue affects rpn.pm mcc_rpn(), right?)
> > > >
> > >
> > > Yes you are right. We have to use l2cap_frame_get_le16() for fetching both
> > rpn.pm and pn.mtu.
> > > I hope it will not cause any Android build problem.
> >
> > Should be fine, I'll try to build-test monitor patches on Android before they
> > get applied anyway.
> >
>
> Thanks for testing it in android.
> As Luiz mentioned, the correct byte ordering for RFCOMM protocol is little endian.
> Btw shall we submit patches on android? Right now we don’t have the setup for testing.
> Probably someone should test it after submitting. Is that ok?
We have automated build system for this so no worries.
--
Best regards,
Szymon Janc
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 11:10 ` Gowtham Anandha Babu
2014-12-05 11:26 ` Szymon Janc
@ 2014-12-05 11:47 ` Luiz Augusto von Dentz
2014-12-05 13:04 ` Gowtham Anandha Babu
1 sibling, 1 reply; 18+ messages in thread
From: Luiz Augusto von Dentz @ 2014-12-05 11:47 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: Szymon Janc, linux-bluetooth@vger.kernel.org, Dmitry Kasatkin,
Bharat Panda, cpgs
Hi Gowtham,
On Fri, Dec 5, 2014 at 1:10 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Hi Szymon,
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> owner@vger.kernel.org] On Behalf Of Szymon Janc
>> Sent: Friday, December 05, 2014 4:33 PM
>> To: Gowtham Anandha Babu
>> Cc: 'Szymon Janc'; linux-bluetooth@vger.kernel.org;
>> d.kasatkin@samsung.com; bharat.panda@samsung.com;
>> cpgs@samsung.com
>> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
>> decoding
>>
>> Hi Gowtham,
>>
>> On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu wrote:
>> > Hi Szymon,
>> >
>> > > -----Original Message-----
>> > > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> > > owner@vger.kernel.org] On Behalf Of Szymon Janc
>> > > Sent: Thursday, December 04, 2014 8:48 PM
>> > > To: Gowtham Anandha Babu
>> > > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
>> > > bharat.panda@samsung.com; cpgs@samsung.com
>> > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
>> > > frame decoding
>> > >
>> > > Hi Gowtham,
>> > >
>> > > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
>> > > <gowtham.ab@samsung.com> wrote:
>> > > > Changes made to decode RPN frame in RFCOMM for btmon.
>> > > >
>> > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> > > > Address: 0x03 cr 1 dlci 0x00
>> > > > Control: 0xef poll/final 0
>> > > > Length: 10
>> > > > FCS: 0x70
>> > > > MCC Message type: Remote Port Negotiation Command
>> CMD(0x24)
>> > > > Length: 8
>> > > > dlci 32
>> > > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
>> > > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
>> > > > pm 0x3f7f
>> > > > ---
>> > > > monitor/rfcomm.c | 73
>> > > >
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> > > > 1 file changed, 73 insertions(+)
>> > > >
>> > > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
>> > > > cd50c8c..62b52cc 100644
>> > > > --- a/monitor/rfcomm.c
>> > > > +++ b/monitor/rfcomm.c
>> > > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
>> > > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
>> > > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
>> > > >
>> > > > +/* RPN macros */
>> > > > +#define GET_RPN_DB(parity) (parity & 0x03)
>> > > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
>> > > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
>> > > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
>> > > > +#define GET_RPN_XIN(io) (io & 0x01)
>> > > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
>> > > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
>> > > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
>> > > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
>> > > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
>> > > > +
>> > > > struct rfcomm_lhdr {
>> > > > uint8_t address;
>> > > > uint8_t control;
>> > > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
>> > > > uint8_t break_sig;
>> > > > } __attribute__((packed));
>> > > >
>> > > > +struct rfcomm_rpn {
>> > > > + uint8_t dlci;
>> > > > + uint8_t bit_rate;
>> > > > + uint8_t parity;
>> > > > + uint8_t io;
>> > > > + uint8_t xon;
>> > > > + uint8_t xoff;
>> > > > + uint16_t pm;
>> > > > +} __attribute__ ((packed));
>> > > > +
>> > > > struct rfcomm_lmcc {
>> > > > uint8_t type;
>> > > > uint16_t length;
>> > > > @@ -138,6 +160,55 @@ done:
>> > > > return true;
>> > > > }
>> > > >
>> > > > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame,
>> > > > +uint8_t
>> > > > +indent) {
>> > > > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
>> > > > + struct rfcomm_rpn rpn;
>> > > > +
>> > > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
>> > > > + return false;
>> > > > +
>> > > > + print_field("%*cdlci %d", indent, ' ',
>> > > > + RFCOMM_GET_DLCI(rpn.dlci));
>> > > > +
>> > > > + if (frame->size < 7)
>> > > > + goto done;
>> > > > +
>> > > > + /* port value octets (optional) */
>> > > > +
>> > > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
>> > > > + return false;
>> > > > +
>> > > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
>> > > > + return false;
>> > > > +
>> > > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
>> > > > + return false;
>> > > > +
>> > > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo %d",
>> > indent, '
>> > > ',
>> > > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
>> > GET_RPN_SB(rpn.parity),
>> > > > + GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
>> > > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
>> > > > +
>> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
>> > > > + return false;
>> > > > +
>> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
>> > > > + return false;
>> > > > +
>> > > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
>> > > > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
>> > > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
>> > > > + rpn.xoff);
>> > > > +
>> > > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
>> > > > + return false;
>> > > > +
>> > > > + print_field("%*cpm 0x%04x", indent, ' ',
>> > > > + __bswap_16(rpn.pm));
>> > >
>> > > This __bswap_16 is invalid since rpn.pm was already converted to
>> > > host
>> > order
>> > > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE arch
>> anyway).
>> > >
>> > > Also this was causing build problems under Android. If you are able
>> > > to
>> > build-
>> > > test under Android please do so. If not then at least point this in
>> > > cover
>> > letter
>> > > or patch comment that code was tested only under Linux).
>> > >
>> > > I've fixed this myself but please pay attention to this with any
>> > > future submission.
>> > >
>> >
>> > Sure I will mention the tested platform details for future submission.
>> >
>> > Btw without __bswap_16(), rpn.pm and pn.mtu will be different from the
>> > hcidump logs.
>> >
>> > This is captured by removing __bswap_16().
>> >
>> > Btmon log:
>> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> > Address: 0x03 cr 1 dlci 0x00
>> > Control: 0xef poll/final 0
>> > Length: 10
>> > FCS: 0x70
>> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
>> > Length: 8
>> > dlci 6 frame_type 0 credit_flow 15 pri 3
>> > ack_timer 0 frame_size 32256 max_retrans 0 credits 1
>> >
>> > Hcidump log:
>> > RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
>> > dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
>> > frame_size 126 max_retrans 0 credits 1
>> >
>> > The rpn.pm is fine. That may be printed without doing __bswap_16() also.
>> >
>> > But in the above PN frame, frame_size is different from actual hcidump
>> > trace.
>> > I cross checked with PTS logs, it was same as the hcidump trace.
>> > So, if it is causing build problems under Android, use
>> > l2cap_frame_get_le16()
>> > Instead of l2cap_frame_get_be16() for pn.mtu.
>> >
>> > If we are consistent with hcidump, then replace the be16() by le16()
>> > for both pn.mtu and rpn.pm.
>> >
>> > After replacing the le16() function:
>> > Btmon logs looks like:
>> >
>> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> > Address: 0x03 cr 1 dlci 0x00
>> > Control: 0xef poll/final 0
>> > Length: 10
>> > FCS: 0x70
>> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
>> > Length: 8
>> > dlci 6 frame_type 0 credit_flow 15 pri 3
>> > ack_timer 0 frame_size 126 max_retrans 0 credits 1
>> >
>> > What do you think ?
>>
>> If value is LE then l2cap_frame_get_le16() should be used.
>> (Same issue affects rpn.pm mcc_rpn(), right?)
>>
>
> Yes you are right. We have to use l2cap_frame_get_le16() for fetching both rpn.pm and pn.mtu.
> I hope it will not cause any Android build problem.
Actually you need to check what endianess for the protocol, the kernel
seems to treat RFCOMM as little endian which I suppose is the correct
byte order otherwise we would not be able to qualify or even
interoperate with other devices.
>> >
>> > > > +
>> > > > +done:
>> > > > + return true;
>> > > > +}
>> > > > +
>> > > > struct mcc_data {
>> > > > uint8_t type;
>> > > > const char *str;
>> > > > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct
>> > > > rfcomm_frame
>> > > *rfcomm_frame, uint8_t indent)
>> > > > switch (type) {
>> > > > case RFCOMM_MSC:
>> > > > return mcc_msc(rfcomm_frame, indent+2);
>> > > > + case RFCOMM_RPN:
>> > > > + return mcc_rpn(rfcomm_frame, indent+2);
>> > > > default:
>> > > > packet_hexdump(frame->data, frame->size);
>> > > > }
>> > > > --
>> > > > 1.9.1
>> > > >
>> > > > --
>> > > > To unsubscribe from this list: send the line "unsubscribe
>> > > > linux-bluetooth" in the body of a message to
>> > > > majordomo@vger.kernel.org More majordomo info at
>> > > > http://vger.kernel.org/majordomo-info.html
>> > >
>> > >
>> > >
>> > > --
>> > > pozdrawiam
>> > > Szymon K. Janc
>> > > szymon.janc@gmail.com
>> > > --
>> > > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
>> > in
>> > > the body of a message to majordomo@vger.kernel.org More majordomo
>> > > info at http://vger.kernel.org/majordomo-info.html
>> >
>> >
>> > Regards,
>> > Gowtham Anandha Babu
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe
>> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> --
>> Best regards,
>> Szymon Janc
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org More majordomo
>> info at http://vger.kernel.org/majordomo-info.html
>
> Regards,
> Gowtham Anandha Babu
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 11:47 ` Luiz Augusto von Dentz
@ 2014-12-05 13:04 ` Gowtham Anandha Babu
2014-12-05 13:07 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-05 13:04 UTC (permalink / raw)
To: 'Luiz Augusto von Dentz', 'Szymon Janc'
Cc: linux-bluetooth, 'Dmitry Kasatkin',
'Bharat Panda', cpgs
Hi Luiz,
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> Sent: Friday, December 05, 2014 5:18 PM
> To: Gowtham Anandha Babu
> Cc: Szymon Janc; linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat
> Panda; cpgs@samsung.com
> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> decoding
>
> Hi Gowtham,
>
> On Fri, Dec 5, 2014 at 1:10 PM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Hi Szymon,
> >
> >> -----Original Message-----
> >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> >> owner@vger.kernel.org] On Behalf Of Szymon Janc
> >> Sent: Friday, December 05, 2014 4:33 PM
> >> To: Gowtham Anandha Babu
> >> Cc: 'Szymon Janc'; linux-bluetooth@vger.kernel.org;
> >> d.kasatkin@samsung.com; bharat.panda@samsung.com;
> cpgs@samsung.com
> >> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> >> decoding
> >>
> >> Hi Gowtham,
> >>
> >> On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu wrote:
> >> > Hi Szymon,
> >> >
> >> > > -----Original Message-----
> >> > > From: linux-bluetooth-owner@vger.kernel.org
> >> > > [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of
> >> > > Szymon Janc
> >> > > Sent: Thursday, December 04, 2014 8:48 PM
> >> > > To: Gowtham Anandha Babu
> >> > > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> >> > > bharat.panda@samsung.com; cpgs@samsung.com
> >> > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
> >> > > frame decoding
> >> > >
> >> > > Hi Gowtham,
> >> > >
> >> > > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
> >> > > <gowtham.ab@samsung.com> wrote:
> >> > > > Changes made to decode RPN frame in RFCOMM for btmon.
> >> > > >
> >> > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >> > > > Address: 0x03 cr 1 dlci 0x00
> >> > > > Control: 0xef poll/final 0
> >> > > > Length: 10
> >> > > > FCS: 0x70
> >> > > > MCC Message type: Remote Port Negotiation Command
> >> CMD(0x24)
> >> > > > Length: 8
> >> > > > dlci 32
> >> > > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> >> > > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> >> > > > pm 0x3f7f
> >> > > > ---
> >> > > > monitor/rfcomm.c | 73
> >> > > >
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> > > > 1 file changed, 73 insertions(+)
> >> > > >
> >> > > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> >> > > > cd50c8c..62b52cc 100644
> >> > > > --- a/monitor/rfcomm.c
> >> > > > +++ b/monitor/rfcomm.c
> >> > > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
> >> > > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> >> > > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
> >> > > >
> >> > > > +/* RPN macros */
> >> > > > +#define GET_RPN_DB(parity) (parity & 0x03)
> >> > > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> >> > > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
> >> > > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> >> > > > +#define GET_RPN_XIN(io) (io & 0x01)
> >> > > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> >> > > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> >> > > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> >> > > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> >> > > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> >> > > > +
> >> > > > struct rfcomm_lhdr {
> >> > > > uint8_t address;
> >> > > > uint8_t control;
> >> > > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> >> > > > uint8_t break_sig;
> >> > > > } __attribute__((packed));
> >> > > >
> >> > > > +struct rfcomm_rpn {
> >> > > > + uint8_t dlci;
> >> > > > + uint8_t bit_rate;
> >> > > > + uint8_t parity;
> >> > > > + uint8_t io;
> >> > > > + uint8_t xon;
> >> > > > + uint8_t xoff;
> >> > > > + uint16_t pm;
> >> > > > +} __attribute__ ((packed));
> >> > > > +
> >> > > > struct rfcomm_lmcc {
> >> > > > uint8_t type;
> >> > > > uint16_t length;
> >> > > > @@ -138,6 +160,55 @@ done:
> >> > > > return true;
> >> > > > }
> >> > > >
> >> > > > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame,
> >> > > > +uint8_t
> >> > > > +indent) {
> >> > > > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
> >> > > > + struct rfcomm_rpn rpn;
> >> > > > +
> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> >> > > > + return false;
> >> > > > +
> >> > > > + print_field("%*cdlci %d", indent, ' ',
> >> > > > + RFCOMM_GET_DLCI(rpn.dlci));
> >> > > > +
> >> > > > + if (frame->size < 7)
> >> > > > + goto done;
> >> > > > +
> >> > > > + /* port value octets (optional) */
> >> > > > +
> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> >> > > > + return false;
> >> > > > +
> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> >> > > > + return false;
> >> > > > +
> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
> >> > > > + return false;
> >> > > > +
> >> > > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo
> >> > > > + %d",
> >> > indent, '
> >> > > ',
> >> > > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
> >> > GET_RPN_SB(rpn.parity),
> >> > > > + GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
> >> > > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> >> > > > +
> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> >> > > > + return false;
> >> > > > +
> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> >> > > > + return false;
> >> > > > +
> >> > > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
> >> > > > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
> >> > > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
> >> > > > + rpn.xoff);
> >> > > > +
> >> > > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> >> > > > + return false;
> >> > > > +
> >> > > > + print_field("%*cpm 0x%04x", indent, ' ',
> >> > > > + __bswap_16(rpn.pm));
> >> > >
> >> > > This __bswap_16 is invalid since rpn.pm was already converted to
> >> > > host
> >> > order
> >> > > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE arch
> >> anyway).
> >> > >
> >> > > Also this was causing build problems under Android. If you are
> >> > > able to
> >> > build-
> >> > > test under Android please do so. If not then at least point this
> >> > > in cover
> >> > letter
> >> > > or patch comment that code was tested only under Linux).
> >> > >
> >> > > I've fixed this myself but please pay attention to this with any
> >> > > future submission.
> >> > >
> >> >
> >> > Sure I will mention the tested platform details for future submission.
> >> >
> >> > Btw without __bswap_16(), rpn.pm and pn.mtu will be different from
> >> > the hcidump logs.
> >> >
> >> > This is captured by removing __bswap_16().
> >> >
> >> > Btmon log:
> >> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >> > Address: 0x03 cr 1 dlci 0x00
> >> > Control: 0xef poll/final 0
> >> > Length: 10
> >> > FCS: 0x70
> >> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> >> > Length: 8
> >> > dlci 6 frame_type 0 credit_flow 15 pri 3
> >> > ack_timer 0 frame_size 32256 max_retrans 0 credits 1
> >> >
> >> > Hcidump log:
> >> > RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
> >> > dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
> >> > frame_size 126 max_retrans 0 credits 1
> >> >
> >> > The rpn.pm is fine. That may be printed without doing __bswap_16()
> also.
> >> >
> >> > But in the above PN frame, frame_size is different from actual
> >> > hcidump trace.
> >> > I cross checked with PTS logs, it was same as the hcidump trace.
> >> > So, if it is causing build problems under Android, use
> >> > l2cap_frame_get_le16()
> >> > Instead of l2cap_frame_get_be16() for pn.mtu.
> >> >
> >> > If we are consistent with hcidump, then replace the be16() by
> >> > le16() for both pn.mtu and rpn.pm.
> >> >
> >> > After replacing the le16() function:
> >> > Btmon logs looks like:
> >> >
> >> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >> > Address: 0x03 cr 1 dlci 0x00
> >> > Control: 0xef poll/final 0
> >> > Length: 10
> >> > FCS: 0x70
> >> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> >> > Length: 8
> >> > dlci 6 frame_type 0 credit_flow 15 pri 3
> >> > ack_timer 0 frame_size 126 max_retrans 0 credits 1
> >> >
> >> > What do you think ?
> >>
> >> If value is LE then l2cap_frame_get_le16() should be used.
> >> (Same issue affects rpn.pm mcc_rpn(), right?)
> >>
> >
> > Yes you are right. We have to use l2cap_frame_get_le16() for fetching both
> rpn.pm and pn.mtu.
> > I hope it will not cause any Android build problem.
>
> Actually you need to check what endianess for the protocol, the kernel
> seems to treat RFCOMM as little endian which I suppose is the correct byte
> order otherwise we would not be able to qualify or even interoperate with
> other devices.
>
I checked it, the correct byte ordering for RFCOMM protocol is LE only.
> >> >
> >> > > > +
> >> > > > +done:
> >> > > > + return true;
> >> > > > +}
> >> > > > +
> >> > > > struct mcc_data {
> >> > > > uint8_t type;
> >> > > > const char *str;
> >> > > > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct
> >> > > > rfcomm_frame
> >> > > *rfcomm_frame, uint8_t indent)
> >> > > > switch (type) {
> >> > > > case RFCOMM_MSC:
> >> > > > return mcc_msc(rfcomm_frame, indent+2);
> >> > > > + case RFCOMM_RPN:
> >> > > > + return mcc_rpn(rfcomm_frame, indent+2);
> >> > > > default:
> >> > > > packet_hexdump(frame->data, frame->size);
> >> > > > }
> >> > > > --
> >> > > > 1.9.1
> >> > > >
> >> > > > --
> >> > > > To unsubscribe from this list: send the line "unsubscribe
> >> > > > linux-bluetooth" in the body of a message to
> >> > > > majordomo@vger.kernel.org More majordomo info at
> >> > > > http://vger.kernel.org/majordomo-info.html
> >> > >
> >> > >
> >> > >
> >> > > --
> >> > > pozdrawiam
> >> > > Szymon K. Janc
> >> > > szymon.janc@gmail.com
> >> > > --
> >> > > To unsubscribe from this list: send the line "unsubscribe linux-
> bluetooth"
> >> > in
> >> > > the body of a message to majordomo@vger.kernel.org More
> majordomo
> >> > > info at http://vger.kernel.org/majordomo-info.html
> >> >
> >> >
> >> > Regards,
> >> > Gowtham Anandha Babu
> >> >
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe
> >> > linux-bluetooth" in the body of a message to
> >> > majordomo@vger.kernel.org More majordomo info at
> >> > http://vger.kernel.org/majordomo-info.html
> >>
> >> --
> >> Best regards,
> >> Szymon Janc
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe
> >> linux-bluetooth" in the body of a message to
> >> majordomo@vger.kernel.org More majordomo info at
> >> http://vger.kernel.org/majordomo-info.html
> >
> > Regards,
> > Gowtham Anandha Babu
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org More majordomo
> info at http://vger.kernel.org/majordomo-info.html
Regards,
Gowtham Anandha Babu
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 13:04 ` Gowtham Anandha Babu
@ 2014-12-05 13:07 ` Luiz Augusto von Dentz
2014-12-05 13:48 ` Gowtham Anandha Babu
0 siblings, 1 reply; 18+ messages in thread
From: Luiz Augusto von Dentz @ 2014-12-05 13:07 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: Szymon Janc, linux-bluetooth@vger.kernel.org, Dmitry Kasatkin,
Bharat Panda, cpgs
Hi Gowtham,
On Fri, Dec 5, 2014 at 3:04 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> Hi Luiz,
>
>> -----Original Message-----
>> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
>> Sent: Friday, December 05, 2014 5:18 PM
>> To: Gowtham Anandha Babu
>> Cc: Szymon Janc; linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat
>> Panda; cpgs@samsung.com
>> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
>> decoding
>>
>> Hi Gowtham,
>>
>> On Fri, Dec 5, 2014 at 1:10 PM, Gowtham Anandha Babu
>> <gowtham.ab@samsung.com> wrote:
>> > Hi Szymon,
>> >
>> >> -----Original Message-----
>> >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
>> >> owner@vger.kernel.org] On Behalf Of Szymon Janc
>> >> Sent: Friday, December 05, 2014 4:33 PM
>> >> To: Gowtham Anandha Babu
>> >> Cc: 'Szymon Janc'; linux-bluetooth@vger.kernel.org;
>> >> d.kasatkin@samsung.com; bharat.panda@samsung.com;
>> cpgs@samsung.com
>> >> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
>> >> decoding
>> >>
>> >> Hi Gowtham,
>> >>
>> >> On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu wrote:
>> >> > Hi Szymon,
>> >> >
>> >> > > -----Original Message-----
>> >> > > From: linux-bluetooth-owner@vger.kernel.org
>> >> > > [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of
>> >> > > Szymon Janc
>> >> > > Sent: Thursday, December 04, 2014 8:48 PM
>> >> > > To: Gowtham Anandha Babu
>> >> > > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
>> >> > > bharat.panda@samsung.com; cpgs@samsung.com
>> >> > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
>> >> > > frame decoding
>> >> > >
>> >> > > Hi Gowtham,
>> >> > >
>> >> > > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
>> >> > > <gowtham.ab@samsung.com> wrote:
>> >> > > > Changes made to decode RPN frame in RFCOMM for btmon.
>> >> > > >
>> >> > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> >> > > > Address: 0x03 cr 1 dlci 0x00
>> >> > > > Control: 0xef poll/final 0
>> >> > > > Length: 10
>> >> > > > FCS: 0x70
>> >> > > > MCC Message type: Remote Port Negotiation Command
>> >> CMD(0x24)
>> >> > > > Length: 8
>> >> > > > dlci 32
>> >> > > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
>> >> > > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
>> >> > > > pm 0x3f7f
>> >> > > > ---
>> >> > > > monitor/rfcomm.c | 73
>> >> > > >
>> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> > > > 1 file changed, 73 insertions(+)
>> >> > > >
>> >> > > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
>> >> > > > cd50c8c..62b52cc 100644
>> >> > > > --- a/monitor/rfcomm.c
>> >> > > > +++ b/monitor/rfcomm.c
>> >> > > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
>> >> > > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
>> >> > > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
>> >> > > >
>> >> > > > +/* RPN macros */
>> >> > > > +#define GET_RPN_DB(parity) (parity & 0x03)
>> >> > > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
>> >> > > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3) #define
>> >> > > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
>> >> > > > +#define GET_RPN_XIN(io) (io & 0x01)
>> >> > > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
>> >> > > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
>> >> > > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
>> >> > > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
>> >> > > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
>> >> > > > +
>> >> > > > struct rfcomm_lhdr {
>> >> > > > uint8_t address;
>> >> > > > uint8_t control;
>> >> > > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
>> >> > > > uint8_t break_sig;
>> >> > > > } __attribute__((packed));
>> >> > > >
>> >> > > > +struct rfcomm_rpn {
>> >> > > > + uint8_t dlci;
>> >> > > > + uint8_t bit_rate;
>> >> > > > + uint8_t parity;
>> >> > > > + uint8_t io;
>> >> > > > + uint8_t xon;
>> >> > > > + uint8_t xoff;
>> >> > > > + uint16_t pm;
>> >> > > > +} __attribute__ ((packed));
>> >> > > > +
>> >> > > > struct rfcomm_lmcc {
>> >> > > > uint8_t type;
>> >> > > > uint16_t length;
>> >> > > > @@ -138,6 +160,55 @@ done:
>> >> > > > return true;
>> >> > > > }
>> >> > > >
>> >> > > > +static inline bool mcc_rpn(struct rfcomm_frame *rfcomm_frame,
>> >> > > > +uint8_t
>> >> > > > +indent) {
>> >> > > > + struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
>> >> > > > + struct rfcomm_rpn rpn;
>> >> > > > +
>> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
>> >> > > > + return false;
>> >> > > > +
>> >> > > > + print_field("%*cdlci %d", indent, ' ',
>> >> > > > + RFCOMM_GET_DLCI(rpn.dlci));
>> >> > > > +
>> >> > > > + if (frame->size < 7)
>> >> > > > + goto done;
>> >> > > > +
>> >> > > > + /* port value octets (optional) */
>> >> > > > +
>> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
>> >> > > > + return false;
>> >> > > > +
>> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
>> >> > > > + return false;
>> >> > > > +
>> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
>> >> > > > + return false;
>> >> > > > +
>> >> > > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d xo
>> >> > > > + %d",
>> >> > indent, '
>> >> > > ',
>> >> > > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
>> >> > GET_RPN_SB(rpn.parity),
>> >> > > > + GET_RPN_PARITY(rpn.parity), GET_RPN_PTYPE(rpn.parity),
>> >> > > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
>> >> > > > +
>> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
>> >> > > > + return false;
>> >> > > > +
>> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
>> >> > > > + return false;
>> >> > > > +
>> >> > > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff %d",
>> >> > > > + indent, ' ', GET_RPN_RTRI(rpn.io), GET_RPN_RTRO(rpn.io),
>> >> > > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io), rpn.xon,
>> >> > > > + rpn.xoff);
>> >> > > > +
>> >> > > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
>> >> > > > + return false;
>> >> > > > +
>> >> > > > + print_field("%*cpm 0x%04x", indent, ' ',
>> >> > > > + __bswap_16(rpn.pm));
>> >> > >
>> >> > > This __bswap_16 is invalid since rpn.pm was already converted to
>> >> > > host
>> >> > order
>> >> > > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE arch
>> >> anyway).
>> >> > >
>> >> > > Also this was causing build problems under Android. If you are
>> >> > > able to
>> >> > build-
>> >> > > test under Android please do so. If not then at least point this
>> >> > > in cover
>> >> > letter
>> >> > > or patch comment that code was tested only under Linux).
>> >> > >
>> >> > > I've fixed this myself but please pay attention to this with any
>> >> > > future submission.
>> >> > >
>> >> >
>> >> > Sure I will mention the tested platform details for future submission.
>> >> >
>> >> > Btw without __bswap_16(), rpn.pm and pn.mtu will be different from
>> >> > the hcidump logs.
>> >> >
>> >> > This is captured by removing __bswap_16().
>> >> >
>> >> > Btmon log:
>> >> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> >> > Address: 0x03 cr 1 dlci 0x00
>> >> > Control: 0xef poll/final 0
>> >> > Length: 10
>> >> > FCS: 0x70
>> >> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
>> >> > Length: 8
>> >> > dlci 6 frame_type 0 credit_flow 15 pri 3
>> >> > ack_timer 0 frame_size 32256 max_retrans 0 credits 1
>> >> >
>> >> > Hcidump log:
>> >> > RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
>> >> > dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
>> >> > frame_size 126 max_retrans 0 credits 1
>> >> >
>> >> > The rpn.pm is fine. That may be printed without doing __bswap_16()
>> also.
>> >> >
>> >> > But in the above PN frame, frame_size is different from actual
>> >> > hcidump trace.
>> >> > I cross checked with PTS logs, it was same as the hcidump trace.
>> >> > So, if it is causing build problems under Android, use
>> >> > l2cap_frame_get_le16()
>> >> > Instead of l2cap_frame_get_be16() for pn.mtu.
>> >> >
>> >> > If we are consistent with hcidump, then replace the be16() by
>> >> > le16() for both pn.mtu and rpn.pm.
>> >> >
>> >> > After replacing the le16() function:
>> >> > Btmon logs looks like:
>> >> >
>> >> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
>> >> > Address: 0x03 cr 1 dlci 0x00
>> >> > Control: 0xef poll/final 0
>> >> > Length: 10
>> >> > FCS: 0x70
>> >> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
>> >> > Length: 8
>> >> > dlci 6 frame_type 0 credit_flow 15 pri 3
>> >> > ack_timer 0 frame_size 126 max_retrans 0 credits 1
>> >> >
>> >> > What do you think ?
>> >>
>> >> If value is LE then l2cap_frame_get_le16() should be used.
>> >> (Same issue affects rpn.pm mcc_rpn(), right?)
>> >>
>> >
>> > Yes you are right. We have to use l2cap_frame_get_le16() for fetching both
>> rpn.pm and pn.mtu.
>> > I hope it will not cause any Android build problem.
>>
>> Actually you need to check what endianess for the protocol, the kernel
>> seems to treat RFCOMM as little endian which I suppose is the correct byte
>> order otherwise we would not be able to qualify or even interoperate with
>> other devices.
>>
>
> I checked it, the correct byte ordering for RFCOMM protocol is LE only.
Then we need to replace every l2cap_frame_get_be* with l2cap_frame_get_le*.
>
>> >> >
>> >> > > > +
>> >> > > > +done:
>> >> > > > + return true;
>> >> > > > +}
>> >> > > > +
>> >> > > > struct mcc_data {
>> >> > > > uint8_t type;
>> >> > > > const char *str;
>> >> > > > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct
>> >> > > > rfcomm_frame
>> >> > > *rfcomm_frame, uint8_t indent)
>> >> > > > switch (type) {
>> >> > > > case RFCOMM_MSC:
>> >> > > > return mcc_msc(rfcomm_frame, indent+2);
>> >> > > > + case RFCOMM_RPN:
>> >> > > > + return mcc_rpn(rfcomm_frame, indent+2);
>> >> > > > default:
>> >> > > > packet_hexdump(frame->data, frame->size);
>> >> > > > }
>> >> > > > --
>> >> > > > 1.9.1
>> >> > > >
>> >> > > > --
>> >> > > > To unsubscribe from this list: send the line "unsubscribe
>> >> > > > linux-bluetooth" in the body of a message to
>> >> > > > majordomo@vger.kernel.org More majordomo info at
>> >> > > > http://vger.kernel.org/majordomo-info.html
>> >> > >
>> >> > >
>> >> > >
>> >> > > --
>> >> > > pozdrawiam
>> >> > > Szymon K. Janc
>> >> > > szymon.janc@gmail.com
>> >> > > --
>> >> > > To unsubscribe from this list: send the line "unsubscribe linux-
>> bluetooth"
>> >> > in
>> >> > > the body of a message to majordomo@vger.kernel.org More
>> majordomo
>> >> > > info at http://vger.kernel.org/majordomo-info.html
>> >> >
>> >> >
>> >> > Regards,
>> >> > Gowtham Anandha Babu
>> >> >
>> >> > --
>> >> > To unsubscribe from this list: send the line "unsubscribe
>> >> > linux-bluetooth" in the body of a message to
>> >> > majordomo@vger.kernel.org More majordomo info at
>> >> > http://vger.kernel.org/majordomo-info.html
>> >>
>> >> --
>> >> Best regards,
>> >> Szymon Janc
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe
>> >> linux-bluetooth" in the body of a message to
>> >> majordomo@vger.kernel.org More majordomo info at
>> >> http://vger.kernel.org/majordomo-info.html
>> >
>> > Regards,
>> > Gowtham Anandha Babu
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe
>> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Luiz Augusto von Dentz
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org More majordomo
>> info at http://vger.kernel.org/majordomo-info.html
>
>
> Regards,
> Gowtham Anandha Babu
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 18+ messages in thread* RE: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame decoding
2014-12-05 13:07 ` Luiz Augusto von Dentz
@ 2014-12-05 13:48 ` Gowtham Anandha Babu
0 siblings, 0 replies; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-05 13:48 UTC (permalink / raw)
To: 'Luiz Augusto von Dentz'
Cc: linux-bluetooth, 'Dmitry Kasatkin',
'Bharat Panda', cpgs
Hi Luiz,
> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> Sent: Friday, December 05, 2014 6:38 PM
> To: Gowtham Anandha Babu
> Cc: Szymon Janc; linux-bluetooth@vger.kernel.org; Dmitry Kasatkin; Bharat
> Panda; cpgs@samsung.com
> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> decoding
>
> Hi Gowtham,
>
> On Fri, Dec 5, 2014 at 3:04 PM, Gowtham Anandha Babu
> <gowtham.ab@samsung.com> wrote:
> > Hi Luiz,
> >
> >> -----Original Message-----
> >> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> >> owner@vger.kernel.org] On Behalf Of Luiz Augusto von Dentz
> >> Sent: Friday, December 05, 2014 5:18 PM
> >> To: Gowtham Anandha Babu
> >> Cc: Szymon Janc; linux-bluetooth@vger.kernel.org; Dmitry Kasatkin;
> >> Bharat Panda; cpgs@samsung.com
> >> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN frame
> >> decoding
> >>
> >> Hi Gowtham,
> >>
> >> On Fri, Dec 5, 2014 at 1:10 PM, Gowtham Anandha Babu
> >> <gowtham.ab@samsung.com> wrote:
> >> > Hi Szymon,
> >> >
> >> >> -----Original Message-----
> >> >> From: linux-bluetooth-owner@vger.kernel.org
> >> >> [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of
> >> >> Szymon Janc
> >> >> Sent: Friday, December 05, 2014 4:33 PM
> >> >> To: Gowtham Anandha Babu
> >> >> Cc: 'Szymon Janc'; linux-bluetooth@vger.kernel.org;
> >> >> d.kasatkin@samsung.com; bharat.panda@samsung.com;
> >> cpgs@samsung.com
> >> >> Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for RPN
> >> >> frame decoding
> >> >>
> >> >> Hi Gowtham,
> >> >>
> >> >> On Friday 05 of December 2014 16:14:20 Gowtham Anandha Babu
> wrote:
> >> >> > Hi Szymon,
> >> >> >
> >> >> > > -----Original Message-----
> >> >> > > From: linux-bluetooth-owner@vger.kernel.org
> >> >> > > [mailto:linux-bluetooth- owner@vger.kernel.org] On Behalf Of
> >> >> > > Szymon Janc
> >> >> > > Sent: Thursday, December 04, 2014 8:48 PM
> >> >> > > To: Gowtham Anandha Babu
> >> >> > > Cc: linux-bluetooth@vger.kernel.org; d.kasatkin@samsung.com;
> >> >> > > bharat.panda@samsung.com; cpgs@samsung.com
> >> >> > > Subject: Re: [PATCH v1 2/5] monitor/rfcomm: Add support for
> >> >> > > RPN frame decoding
> >> >> > >
> >> >> > > Hi Gowtham,
> >> >> > >
> >> >> > > On Wed, Dec 3, 2014 at 1:01 PM, Gowtham Anandha Babu
> >> >> > > <gowtham.ab@samsung.com> wrote:
> >> >> > > > Changes made to decode RPN frame in RFCOMM for btmon.
> >> >> > > >
> >> >> > > > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >> >> > > > Address: 0x03 cr 1 dlci 0x00
> >> >> > > > Control: 0xef poll/final 0
> >> >> > > > Length: 10
> >> >> > > > FCS: 0x70
> >> >> > > > MCC Message type: Remote Port Negotiation Command
> >> >> CMD(0x24)
> >> >> > > > Length: 8
> >> >> > > > dlci 32
> >> >> > > > br 3 db 2 sb 0 p 0 pt 0 xi 0 xo 0
> >> >> > > > rtri 0 rtro 0 rtci 0 rtco 0 xon 17 xoff 19
> >> >> > > > pm 0x3f7f
> >> >> > > > ---
> >> >> > > > monitor/rfcomm.c | 73
> >> >> > > >
> >> >>
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >> > > > 1 file changed, 73 insertions(+)
> >> >> > > >
> >> >> > > > diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c index
> >> >> > > > cd50c8c..62b52cc 100644
> >> >> > > > --- a/monitor/rfcomm.c
> >> >> > > > +++ b/monitor/rfcomm.c
> >> >> > > > @@ -63,6 +63,18 @@ static char *cr_str[] = {
> >> >> > > > #define GET_V24_IC(sigs) ((sigs & 0x40) >> 6)
> >> >> > > > #define GET_V24_DV(sigs) ((sigs & 0x80) >> 7)
> >> >> > > >
> >> >> > > > +/* RPN macros */
> >> >> > > > +#define GET_RPN_DB(parity) (parity & 0x03)
> >> >> > > > +#define GET_RPN_SB(parity) ((parity & 0x04) >> 2)
> >> >> > > > +#define GET_RPN_PARITY(parity) ((parity & 0x08) >> 3)
> >> >> > > > +#define
> >> >> > > > +GET_RPN_PTYPE(parity) ((parity & 0x03) >> 3)
> >> >> > > > +#define GET_RPN_XIN(io) (io & 0x01)
> >> >> > > > +#define GET_RPN_XOUT(io) ((io & 0x02) >> 1)
> >> >> > > > +#define GET_RPN_RTRI(io) ((io & 0x04) >> 2)
> >> >> > > > +#define GET_RPN_RTRO(io) ((io & 0x08) >> 3)
> >> >> > > > +#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
> >> >> > > > +#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
> >> >> > > > +
> >> >> > > > struct rfcomm_lhdr {
> >> >> > > > uint8_t address;
> >> >> > > > uint8_t control;
> >> >> > > > @@ -77,6 +89,16 @@ struct rfcomm_lmsc {
> >> >> > > > uint8_t break_sig;
> >> >> > > > } __attribute__((packed));
> >> >> > > >
> >> >> > > > +struct rfcomm_rpn {
> >> >> > > > + uint8_t dlci;
> >> >> > > > + uint8_t bit_rate;
> >> >> > > > + uint8_t parity;
> >> >> > > > + uint8_t io;
> >> >> > > > + uint8_t xon;
> >> >> > > > + uint8_t xoff;
> >> >> > > > + uint16_t pm;
> >> >> > > > +} __attribute__ ((packed));
> >> >> > > > +
> >> >> > > > struct rfcomm_lmcc {
> >> >> > > > uint8_t type;
> >> >> > > > uint16_t length;
> >> >> > > > @@ -138,6 +160,55 @@ done:
> >> >> > > > return true;
> >> >> > > > }
> >> >> > > >
> >> >> > > > +static inline bool mcc_rpn(struct rfcomm_frame
> >> >> > > > +*rfcomm_frame, uint8_t
> >> >> > > > +indent) {
> >> >> > > > + struct l2cap_frame *frame = &rfcomm_frame-
> >l2cap_frame;
> >> >> > > > + struct rfcomm_rpn rpn;
> >> >> > > > +
> >> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.dlci))
> >> >> > > > + return false;
> >> >> > > > +
> >> >> > > > + print_field("%*cdlci %d", indent, ' ',
> >> >> > > > + RFCOMM_GET_DLCI(rpn.dlci));
> >> >> > > > +
> >> >> > > > + if (frame->size < 7)
> >> >> > > > + goto done;
> >> >> > > > +
> >> >> > > > + /* port value octets (optional) */
> >> >> > > > +
> >> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.bit_rate))
> >> >> > > > + return false;
> >> >> > > > +
> >> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.parity))
> >> >> > > > + return false;
> >> >> > > > +
> >> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.io))
> >> >> > > > + return false;
> >> >> > > > +
> >> >> > > > + print_field("%*cbr %d db %d sb %d p %d pt %d xi %d
> >> >> > > > + xo %d",
> >> >> > indent, '
> >> >> > > ',
> >> >> > > > + rpn.bit_rate, GET_RPN_DB(rpn.parity),
> >> >> > GET_RPN_SB(rpn.parity),
> >> >> > > > + GET_RPN_PARITY(rpn.parity),
> GET_RPN_PTYPE(rpn.parity),
> >> >> > > > + GET_RPN_XIN(rpn.io), GET_RPN_XOUT(rpn.io));
> >> >> > > > +
> >> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xon))
> >> >> > > > + return false;
> >> >> > > > +
> >> >> > > > + if (!l2cap_frame_get_u8(frame, &rpn.xoff))
> >> >> > > > + return false;
> >> >> > > > +
> >> >> > > > + print_field("%*crtri %d rtro %d rtci %d rtco %d xon %d xoff
> %d",
> >> >> > > > + indent, ' ', GET_RPN_RTRI(rpn.io),
> GET_RPN_RTRO(rpn.io),
> >> >> > > > + GET_RPN_RTCI(rpn.io), GET_RPN_RTCO(rpn.io),
> rpn.xon,
> >> >> > > > + rpn.xoff);
> >> >> > > > +
> >> >> > > > + if (!l2cap_frame_get_be16(frame, &rpn.pm))
> >> >> > > > + return false;
> >> >> > > > +
> >> >> > > > + print_field("%*cpm 0x%04x", indent, ' ',
> >> >> > > > + __bswap_16(rpn.pm));
> >> >> > >
> >> >> > > This __bswap_16 is invalid since rpn.pm was already converted
> >> >> > > to host
> >> >> > order
> >> >> > > by l2cap_frame_get_be16(). (It wouldn't work correctly on BE
> >> >> > > arch
> >> >> anyway).
> >> >> > >
> >> >> > > Also this was causing build problems under Android. If you are
> >> >> > > able to
> >> >> > build-
> >> >> > > test under Android please do so. If not then at least point
> >> >> > > this in cover
> >> >> > letter
> >> >> > > or patch comment that code was tested only under Linux).
> >> >> > >
> >> >> > > I've fixed this myself but please pay attention to this with
> >> >> > > any future submission.
> >> >> > >
> >> >> >
> >> >> > Sure I will mention the tested platform details for future submission.
> >> >> >
> >> >> > Btw without __bswap_16(), rpn.pm and pn.mtu will be different
> >> >> > from the hcidump logs.
> >> >> >
> >> >> > This is captured by removing __bswap_16().
> >> >> >
> >> >> > Btmon log:
> >> >> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >> >> > Address: 0x03 cr 1 dlci 0x00
> >> >> > Control: 0xef poll/final 0
> >> >> > Length: 10
> >> >> > FCS: 0x70
> >> >> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> >> >> > Length: 8
> >> >> > dlci 6 frame_type 0 credit_flow 15 pri 3
> >> >> > ack_timer 0 frame_size 32256 max_retrans 0 credits 1
> >> >> >
> >> >> > Hcidump log:
> >> >> > RFCOMM(s): PN CMD: cr 1 dlci 0 pf 0 ilen 10 fcs 0x70 mcc_len 8
> >> >> > dlci 6 frame_type 0 credit_flow 15 pri 3 ack_timer 0
> >> >> > frame_size 126 max_retrans 0 credits 1
> >> >> >
> >> >> > The rpn.pm is fine. That may be printed without doing
> >> >> > __bswap_16()
> >> also.
> >> >> >
> >> >> > But in the above PN frame, frame_size is different from actual
> >> >> > hcidump trace.
> >> >> > I cross checked with PTS logs, it was same as the hcidump trace.
> >> >> > So, if it is causing build problems under Android, use
> >> >> > l2cap_frame_get_le16()
> >> >> > Instead of l2cap_frame_get_be16() for pn.mtu.
> >> >> >
> >> >> > If we are consistent with hcidump, then replace the be16() by
> >> >> > le16() for both pn.mtu and rpn.pm.
> >> >> >
> >> >> > After replacing the le16() function:
> >> >> > Btmon logs looks like:
> >> >> >
> >> >> > RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
> >> >> > Address: 0x03 cr 1 dlci 0x00
> >> >> > Control: 0xef poll/final 0
> >> >> > Length: 10
> >> >> > FCS: 0x70
> >> >> > MCC Message type: DLC Parameter Negotiation CMD(0x20)
> >> >> > Length: 8
> >> >> > dlci 6 frame_type 0 credit_flow 15 pri 3
> >> >> > ack_timer 0 frame_size 126 max_retrans 0 credits 1
> >> >> >
> >> >> > What do you think ?
> >> >>
> >> >> If value is LE then l2cap_frame_get_le16() should be used.
> >> >> (Same issue affects rpn.pm mcc_rpn(), right?)
> >> >>
> >> >
> >> > Yes you are right. We have to use l2cap_frame_get_le16() for
> >> > fetching both
> >> rpn.pm and pn.mtu.
> >> > I hope it will not cause any Android build problem.
> >>
> >> Actually you need to check what endianess for the protocol, the
> >> kernel seems to treat RFCOMM as little endian which I suppose is the
> >> correct byte order otherwise we would not be able to qualify or even
> >> interoperate with other devices.
> >>
> >
> > I checked it, the correct byte ordering for RFCOMM protocol is LE only.
>
> Then we need to replace every l2cap_frame_get_be* with
> l2cap_frame_get_le*.
>
Submitted the same as patch. Please have a look at it.
> >
> >> >> >
> >> >> > > > +
> >> >> > > > +done:
> >> >> > > > + return true;
> >> >> > > > +}
> >> >> > > > +
> >> >> > > > struct mcc_data {
> >> >> > > > uint8_t type;
> >> >> > > > const char *str;
> >> >> > > > @@ -201,6 +272,8 @@ static inline bool mcc_frame(struct
> >> >> > > > rfcomm_frame
> >> >> > > *rfcomm_frame, uint8_t indent)
> >> >> > > > switch (type) {
> >> >> > > > case RFCOMM_MSC:
> >> >> > > > return mcc_msc(rfcomm_frame, indent+2);
> >> >> > > > + case RFCOMM_RPN:
> >> >> > > > + return mcc_rpn(rfcomm_frame, indent+2);
> >> >> > > > default:
> >> >> > > > packet_hexdump(frame->data, frame->size);
> >> >> > > > }
> >> >> > > > --
> >> >> > > > 1.9.1
> >> >> > > >
> >> >> > > > --
> >> >> > > > To unsubscribe from this list: send the line "unsubscribe
> >> >> > > > linux-bluetooth" in the body of a message to
> >> >> > > > majordomo@vger.kernel.org More majordomo info at
> >> >> > > > http://vger.kernel.org/majordomo-info.html
> >> >> > >
> >> >> > >
> >> >> > >
> >> >> > > --
> >> >> > > pozdrawiam
> >> >> > > Szymon K. Janc
> >> >> > > szymon.janc@gmail.com
> >> >> > > --
> >> >> > > To unsubscribe from this list: send the line "unsubscribe
> >> >> > > linux-
> >> bluetooth"
> >> >> > in
> >> >> > > the body of a message to majordomo@vger.kernel.org More
> >> majordomo
> >> >> > > info at http://vger.kernel.org/majordomo-info.html
> >> >> >
> >> >> >
> >> >> > Regards,
> >> >> > Gowtham Anandha Babu
> >> >> >
> >> >> > --
> >> >> > To unsubscribe from this list: send the line "unsubscribe
> >> >> > linux-bluetooth" in the body of a message to
> >> >> > majordomo@vger.kernel.org More majordomo info at
> >> >> > http://vger.kernel.org/majordomo-info.html
> >> >>
> >> >> --
> >> >> Best regards,
> >> >> Szymon Janc
> >> >> --
> >> >> To unsubscribe from this list: send the line "unsubscribe
> >> >> linux-bluetooth" in the body of a message to
> >> >> majordomo@vger.kernel.org More majordomo info at
> >> >> http://vger.kernel.org/majordomo-info.html
> >> >
> >> > Regards,
> >> > Gowtham Anandha Babu
> >> >
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe
> >> > linux-bluetooth" in the body of a message to
> >> > majordomo@vger.kernel.org More majordomo info at
> >> > http://vger.kernel.org/majordomo-info.html
> >>
> >>
> >>
> >> --
> >> Luiz Augusto von Dentz
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe
> >> linux-bluetooth" in the body of a message to
> >> majordomo@vger.kernel.org More majordomo info at
> >> http://vger.kernel.org/majordomo-info.html
> >
> >
> > Regards,
> > Gowtham Anandha Babu
> >
>
>
>
> --
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org More majordomo
> info at http://vger.kernel.org/majordomo-info.html
Regards,
Gowtham Anandha Babu
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v1 3/5] monitor/rfcomm: Add support for RLS frame decoding
2014-12-03 12:01 [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type Gowtham Anandha Babu
2014-12-03 12:01 ` [PATCH v1 1/5] monitor/rfcomm.c: Add support for MSC frame decoding Gowtham Anandha Babu
2014-12-03 12:01 ` [PATCH v1 2/5] monitor/rfcomm: Add support for RPN " Gowtham Anandha Babu
@ 2014-12-03 12:01 ` Gowtham Anandha Babu
2014-12-03 12:01 ` [PATCH v1 4/5] monitor/rfcomm: Add support for PN " Gowtham Anandha Babu
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-03 12:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: d.kasatkin, bharat.panda, cpgs, Gowtham Anandha Babu
Changes made to decode RLS frame in RFCOMM for btmon.
RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
Address: 0x03 cr 1 dlci 0x00
Control: 0xef poll/final 0
Length: 4
FCS: 0x70
MCC Message type: Remote Line Status CMD(0x14)
Length: 2
dlci 32 error: 5
---
monitor/rfcomm.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index 62b52cc..848bad7 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -75,6 +75,9 @@ static char *cr_str[] = {
#define GET_RPN_RTCI(io) ((io & 0x10) >> 4)
#define GET_RPN_RTCO(io) ((io & 0x20) >> 5)
+/* RLS macro */
+#define GET_ERROR(err) (err & 0x0f)
+
struct rfcomm_lhdr {
uint8_t address;
uint8_t control;
@@ -99,6 +102,11 @@ struct rfcomm_rpn {
uint16_t pm;
} __attribute__ ((packed));
+struct rfcomm_rls {
+ uint8_t dlci;
+ uint8_t error;
+} __attribute__((packed));
+
struct rfcomm_lmcc {
uint8_t type;
uint16_t length;
@@ -209,6 +217,23 @@ done:
return true;
}
+static inline bool mcc_rls(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+ struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+ struct rfcomm_rls rls;
+
+ if (!l2cap_frame_get_u8(frame, &rls.dlci))
+ return false;
+
+ if (!l2cap_frame_get_u8(frame, &rls.error))
+ return false;
+
+ print_field("%*cdlci %d error: %d", indent, ' ',
+ RFCOMM_GET_DLCI(rls.dlci), GET_ERROR(rls.error));
+
+ return true;
+}
+
struct mcc_data {
uint8_t type;
const char *str;
@@ -274,6 +299,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
return mcc_msc(rfcomm_frame, indent+2);
case RFCOMM_RPN:
return mcc_rpn(rfcomm_frame, indent+2);
+ case RFCOMM_RLS:
+ return mcc_rls(rfcomm_frame, indent+2);
default:
packet_hexdump(frame->data, frame->size);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v1 4/5] monitor/rfcomm: Add support for PN frame decoding
2014-12-03 12:01 [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type Gowtham Anandha Babu
` (2 preceding siblings ...)
2014-12-03 12:01 ` [PATCH v1 3/5] monitor/rfcomm: Add support for RLS " Gowtham Anandha Babu
@ 2014-12-03 12:01 ` Gowtham Anandha Babu
2014-12-03 12:01 ` [PATCH v1 5/5] monitor/rfcomm: Add support for NSC " Gowtham Anandha Babu
2014-12-04 12:17 ` [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type Luiz Augusto von Dentz
5 siblings, 0 replies; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-03 12:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: d.kasatkin, bharat.panda, cpgs, Gowtham Anandha Babu
Changes made to decode PN frame in RFCOMM for btmon.
RFCOMM: Unnumbered Info with Header Check (UIH)(0xef)
Address: 0x01 cr 0 dlci 0x00
Control: 0xef poll/final 0
Length: 10
FCS: 0xaa
MCC Message type: DLC Parameter Negotiation RSP(0x20)
Length: 8
dlci 32 frame_type 0 credit_flow 14 pri 39
ack_timer 0 frame_size 666 max_retrans 0 credits 7
---
monitor/rfcomm.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index 848bad7..2a82001 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -78,6 +78,12 @@ static char *cr_str[] = {
/* RLS macro */
#define GET_ERROR(err) (err & 0x0f)
+/* PN macros */
+#define GET_FRM_TYPE(ctrl) ((ctrl & 0x0f))
+#define GET_CRT_FLOW(ctrl) ((ctrl & 0xf0) >> 4)
+#define GET_PRIORITY(prio) ((prio & 0x3f))
+#define GET_PN_DLCI(dlci) ((dlci & 0x3f))
+
struct rfcomm_lhdr {
uint8_t address;
uint8_t control;
@@ -234,6 +240,45 @@ static inline bool mcc_rls(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
return true;
}
+static inline bool mcc_pn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+ struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+ struct rfcomm_pn pn;
+
+ /* rfcomm_pn struct is defined in rfcomm.h */
+
+ if (!l2cap_frame_get_u8(frame, &pn.dlci))
+ return false;
+
+ if (!l2cap_frame_get_u8(frame, &pn.flow_ctrl))
+ return false;
+
+ if (!l2cap_frame_get_u8(frame, &pn.priority))
+ return false;
+
+ print_field("%*cdlci %d frame_type %d credit_flow %d pri %d", indent,
+ ' ', GET_PN_DLCI(pn.dlci), GET_FRM_TYPE(pn.flow_ctrl),
+ GET_CRT_FLOW(pn.flow_ctrl), GET_PRIORITY(pn.priority));
+
+ if (!l2cap_frame_get_u8(frame, &pn.ack_timer))
+ return false;
+
+ if (!l2cap_frame_get_be16(frame, &pn.mtu))
+ return false;
+
+ if (!l2cap_frame_get_u8(frame, &pn.max_retrans))
+ return false;
+
+ if (!l2cap_frame_get_u8(frame, &pn.credits))
+ return false;
+
+ print_field("%*cack_timer %d frame_size %d max_retrans %d credits %d",
+ indent, ' ', pn.ack_timer, __bswap_16(pn.mtu),
+ pn.max_retrans, pn.credits);
+
+ return true;
+}
+
struct mcc_data {
uint8_t type;
const char *str;
@@ -301,6 +346,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
return mcc_rpn(rfcomm_frame, indent+2);
case RFCOMM_RLS:
return mcc_rls(rfcomm_frame, indent+2);
+ case RFCOMM_PN:
+ return mcc_pn(rfcomm_frame, indent+2);
default:
packet_hexdump(frame->data, frame->size);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v1 5/5] monitor/rfcomm: Add support for NSC frame decoding
2014-12-03 12:01 [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type Gowtham Anandha Babu
` (3 preceding siblings ...)
2014-12-03 12:01 ` [PATCH v1 4/5] monitor/rfcomm: Add support for PN " Gowtham Anandha Babu
@ 2014-12-03 12:01 ` Gowtham Anandha Babu
2014-12-04 12:17 ` [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type Luiz Augusto von Dentz
5 siblings, 0 replies; 18+ messages in thread
From: Gowtham Anandha Babu @ 2014-12-03 12:01 UTC (permalink / raw)
To: linux-bluetooth; +Cc: d.kasatkin, bharat.panda, cpgs, Gowtham Anandha Babu
Changes made to decode NSC frame in RFCOMM for btmon.
Not able capture the output.
---
monitor/rfcomm.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/monitor/rfcomm.c b/monitor/rfcomm.c
index 2a82001..e90fc1c 100644
--- a/monitor/rfcomm.c
+++ b/monitor/rfcomm.c
@@ -113,6 +113,10 @@ struct rfcomm_rls {
uint8_t error;
} __attribute__((packed));
+struct rfcomm_nsc {
+ uint8_t cmd_type;
+} __attribute__((packed));
+
struct rfcomm_lmcc {
uint8_t type;
uint16_t length;
@@ -279,6 +283,20 @@ static inline bool mcc_pn(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
return true;
}
+static inline bool mcc_nsc(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
+{
+ struct l2cap_frame *frame = &rfcomm_frame->l2cap_frame;
+ struct rfcomm_nsc nsc;
+
+ if (!l2cap_frame_get_u8(frame, &nsc.cmd_type))
+ return false;
+
+ print_field("%*ccr %d, mcc_cmd_type %x", indent, ' ',
+ GET_CR(nsc.cmd_type), RFCOMM_GET_MCC_TYPE(nsc.cmd_type));
+
+ return true;
+}
+
struct mcc_data {
uint8_t type;
const char *str;
@@ -348,6 +366,8 @@ static inline bool mcc_frame(struct rfcomm_frame *rfcomm_frame, uint8_t indent)
return mcc_rls(rfcomm_frame, indent+2);
case RFCOMM_PN:
return mcc_pn(rfcomm_frame, indent+2);
+ case RFCOMM_NSC:
+ return mcc_nsc(rfcomm_frame, indent+2);
default:
packet_hexdump(frame->data, frame->size);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type
2014-12-03 12:01 [PATCH v1 0/5] Add support for decoding RFCOMM MCC Message Type Gowtham Anandha Babu
` (4 preceding siblings ...)
2014-12-03 12:01 ` [PATCH v1 5/5] monitor/rfcomm: Add support for NSC " Gowtham Anandha Babu
@ 2014-12-04 12:17 ` Luiz Augusto von Dentz
5 siblings, 0 replies; 18+ messages in thread
From: Luiz Augusto von Dentz @ 2014-12-04 12:17 UTC (permalink / raw)
To: Gowtham Anandha Babu
Cc: linux-bluetooth@vger.kernel.org, Dmitry Kasatkin, Bharat Panda,
cpgs
Hi Gowtham,
On Wed, Dec 3, 2014 at 2:01 PM, Gowtham Anandha Babu
<gowtham.ab@samsung.com> wrote:
> *v1: Removed the break signal decoding implementation for now.
>
> This patch set implements the handlers for decoding
> MSC, RPN, RLS, PN, NSC frames for RFCOMM in bluetooth
> monitor.
>
> Gowtham Anandha Babu (5):
> monitor/rfcomm.c: Add support for MSC frame decoding
> monitor/rfcomm: Add support for RPN frame decoding
> monitor/rfcomm: Add support for RLS frame decoding
> monitor/rfcomm: Add support for PN frame decoding
> monitor/rfcomm: Add support for NSC frame decoding
>
> monitor/rfcomm.c | 233 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 226 insertions(+), 7 deletions(-)
>
> --
> 1.9.1
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 18+ messages in thread