[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bluetooth-dev] Alignement fix



I have tried to carrefully commit with your recomendations and :

cvs commit: Examining linux/drivers/char/bluetooth
cvs [server aborted]: "commit" requires write access to the repository
cvs commit: saving log message in /tmp/cvsy50Tzg                              

Do I have write access to the repository ? (I logged in with my name dlibault 
and password)

David.
  
Le Jeudi 01 Mars 2001 18:07, Peter Kjellerstedt a écrit :
> Ok, I have looked through the patch and it looks ok.
>
> There are a few minor details. Your local changes in
> btconfig.h and btdebug.h should probably not be checked in.
> Neither should the MTU change in rfcomm.c. Also it seems
> that you do not have the very latest version of
> apps/bluetooth/userstack (cvs would alert you about this
> if you had tried to commit, so it is no big deal).
>
> A question: why have you added -I /usr/include to the
> linux/drivers/char/bluetooth/Makefile?

If not, the compiler (on my PC) complains about headers not compiling 
properly. I don't know why I need to do that... I use Mandrake 7.2.

>
> It seems like the change in hci.h (ocf + ogf -> opcode)
> and in l2cap.h are related to endianess. However, I
> fail to see how hci_put_opcode() would change anything
> regarding endianess compared to using th bitfields.
> If it had used cpu_to_le16() I would have understood it.
> Another solution would have been to change the definition
> of the struct to:
>
> typedef struct cmd_pkt {
> 	u8 type;
> #ifdef __LITTLE_ENDIAN_BITFIELD
> 	u16 ocf:10;
> 	u16 ogf:6;
> #elif __BIG_ENDIAN_BITFIELD
> 	u16 ogf:6;
> 	u16 ocf:10;
> #else
> #error "You need to define either __LITTLE_ENDIAN_BITFIELD or
> __BIG_ENDIAN_BITFIELD" #endif
> 	u8 len;
>
> 	u8 data[256];
> };
>
> (The above code is of course totally untested.)
> A solution like the one above would probably be needed to
> fix all the bitfields used in rfcomm.c, sdp.c and tcs.c.
>

1 - this was in the Gordon's patch (that is why I did it, and believe me, 
changing all the hci commands was not a nice work to do...)
2 - I think this is the first step to remove the bitfields in the stack which 
are not a nice C feature to use (a lot of portability issues). Moreover, it 
doesn't improve the performance because, at the end, the processor will still 
have to do bit masks... so I would prefer to just use | and &, it will save 
everybody time and energy...

>
> I would recommend that you commit your changes in the
> linux directory separately from the changes in apps as
> they do not seem related.
>
> Start by temporarily reverting you change to the MTu in
> rfcomm.c The the following commands should commit your
> changes in the linux directory. After running each cvs
> command you will be asked for a changelog comment.
> Describe what you have done (try to keep your lines
> shorter than 75 characters).
>
> # First update the code to be sure you have the very latest
> cvs up linux
> # Then proceed to commmit your changes
> cvs commit linux/drivers/char/bluetooth
> cd linux/include/bluetooth
> cvs commit btcommon.h hci.h hci_internal.h l2cap.h rfcomm.h
>
> //Peter
>
> > -----Original Message-----
> > From: david LIBAULT [mailto:david.libault@xxxxxxx.fr]
> > Sent: 01 March 2001 16:06
> > To: Peter Kjellerstedt
> > Cc: bluetooth-dev@xxxxxxx.com
> > Subject: Re: [bluetooth-dev] Alignement fix
> >
> > Le Jeudi 01 Mars 2001 15:48, Peter Kjellerstedt a écrit :
> > > > -----Original Message-----
> > > > From: david LIBAULT [mailto:david.libault@xxxxxxx.fr]
> > > > Sent: 01 March 2001 15:11
> > > > To: bluetooth-dev@xxxxxxx.com
> > > > Subject: [bluetooth-dev] Alignement fix
> > > >
> > > > So,
> > > >
> > > > I have finished with the alignement fixes. I have tested the
> > > > stack with my ARM system, and I could open an RFCOMM channel...
> > > > The same code also works on a PC (same test).
> > > >
> > > > I can not test the module on the ARM because the cvs version
> > > > I have doesn't have the 2.4 fixes (and my ARM system runs
> > > > kernel 2.4 only...).
> > > >
> > > > What do I do now ? Should I update the CVS ? How do I do that
> > > > (I wouldn't like to skrew up anything...) ?
> > > >
> > > > David.
> > >
> > > Since this is the first time you are using CVS (if I am not
> > > mistaken) you could start by sending a patch file with your
> > > changes to the list and we could look at it. After we have
> > > had a look at it, and there are no problems, you (or someone
> > > else) can commit it to the repository.
> > >
> > > To create the patch file, you can do the following:
> > >
> > > cvs diff -ud linux apps > bt.patch
> > >
> > > //Peter
> >
> > There you go...
> > Note that the long UIH packet problem has not been fixed yet...
> >
> > David.
-
To unsubscribe from this list: send the line "unsubscribe bluetooth-dev" in
the body of a message to majordomo@xxxxxxx.com