Hi Szymon, > -----Original Message----- > From: linux-bluetooth-owner@xxxxxxxxxxxxxxx [mailto:linux-bluetooth- > owner@xxxxxxxxxxxxxxx] On Behalf Of Szymon Janc > Sent: Friday, December 05, 2014 4:57 PM > To: Gowtham Anandha Babu > Cc: linux-bluetooth@xxxxxxxxxxxxxxx; d.kasatkin@xxxxxxxxxxx; > bharat.panda@xxxxxxxxxxx; cpgs@xxxxxxxxxxx > 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@xxxxxxxxxxxxxxx [mailto:linux-bluetooth- > > > owner@xxxxxxxxxxxxxxx] On Behalf Of Szymon Janc > > > Sent: Friday, December 05, 2014 4:33 PM > > > To: Gowtham Anandha Babu > > > Cc: 'Szymon Janc'; linux-bluetooth@xxxxxxxxxxxxxxx; > > > d.kasatkin@xxxxxxxxxxx; bharat.panda@xxxxxxxxxxx; > cpgs@xxxxxxxxxxx > > > 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@xxxxxxxxxxxxxxx > > > > > [mailto:linux-bluetooth- owner@xxxxxxxxxxxxxxx] On Behalf Of > > > > > Szymon Janc > > > > > Sent: Thursday, December 04, 2014 8:48 PM > > > > > To: Gowtham Anandha Babu > > > > > Cc: linux-bluetooth@xxxxxxxxxxxxxxx; d.kasatkin@xxxxxxxxxxx; > > > > > bharat.panda@xxxxxxxxxxx; cpgs@xxxxxxxxxxx > > > > > 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@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxx More majordomo info at > > > > > > http://vger.kernel.org/majordomo-info.html > > > > > > > > > > > > > > > > > > > > -- > > > > > pozdrawiam > > > > > Szymon K. Janc > > > > > szymon.janc@xxxxxxxxx > > > > > -- > > > > > To unsubscribe from this list: send the line "unsubscribe linux- > bluetooth" > > > > in > > > > > the body of a message to majordomo@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
