Hi Gustavo, [PATCH 1/3] is adding AMP device hci layer support, even it modify some code, but it doesn't affect BR/EDR original logic, so we can consider it as totally new. [PATCH 2/3] is adding AMP manager/A2MP protocol support, it is totally new. So in my thought these two patches can be considered as atomic patches. [PATCH 3/3] modify a lot of ERTM/Streaming mode code, but it's hard to split up, since they depends each other, so any comments for how to split is welcome. Best Regards Haijun 86-21-61820900 Ext. 6331 Work Haijun.Liu@xxxxxxxxxxx Skype: liu.haijun -----Original Message----- From: Gustavo F. Padovan [mailto:gfpadovan@xxxxxxxxx] On Behalf Of Gustavo F. Padovan Sent: Friday, July 16, 2010 3:43 AM To: Luis Rodriguez Cc: Dan Tian; linux-bluetooth@xxxxxxxxxxxxxxx; Haijun Liu; Luis Rodriguez Subject: Re: [PATCH 1/3] Add BT3 AMP device support, by Atheros Linux BT3 team. Hi Luis, * Luis R. Rodriguez <lrodriguez@xxxxxxxxxxx> [2010-07-15 11:43:08 -0700]: > On Thu, Jul 15, 2010 at 08:09:21AM -0700, Gustavo F. Padovan wrote: > > Hi Dan, > > > > * Dan Tian <Dan.Tian@xxxxxxxxxxx> [2010-07-15 15:37:32 +0800]: > > > > > From 40cbfbf5c1bc9109364970956d9b6f74f5846c70 Mon Sep 17 00:00:00 2001 > > > From: Haijun.Liu <Haijun.Liu@xxxxxxxxxxx> > > > Date: Wed, 14 Jul 2010 22:50:56 +0800 > > > Subject: [PATCH 1/3] Add BT3 AMP device support, by Atheros Linux BT3 team. > > > > > > > > > Signed-off-by: Haijun.Liu <Haijun.Liu@xxxxxxxxxxx> > > > --- > > > include/net/bluetooth/hci.h | 342 +++++++++++++++++++- > > > include/net/bluetooth/hci_core.h | 264 ++++++++++++++- > > > net/bluetooth/cmtp/core.c | 1 + > > > net/bluetooth/hci_conn.c | 201 +++++++++++- > > > net/bluetooth/hci_core.c | 349 ++++++++++++++++++-- > > > net/bluetooth/hci_event.c | 698 +++++++++++++++++++++++++++++++++++++- > > > 6 files changed, 1822 insertions(+), 33 deletions(-) > > > > > > Could you split all your patches in little chunks? +1882 is very hard to > > review. And please be more verbose on the commit message, it can help us > > figure out what's happening. > > This used to be 1 large patch, it was recently split up to 3, I havent't > had a chance yet to review further but my understanding is this this was > split up as much as possible in the last try. If you can provide suggestions > how to split it up more that might be useful, but agreed, if possible more > splitup would help. But it is still too big. I looked to patch 3/3, it does a lot different things in the same patch that make the patch impossible to be tracked. We need atomic patches for at least each change in the ERTM code, each new state added in the L2CAP state machine, each new HS feature, etc, i.e., we need proper patches with proper commit messages explaining what's happening. The same should apply to 1/3 and 2/3, I didn't looked to them. > > Dan, can you also please drop the "by Atheros Linux BT3 team." on the subject > of the patches. This is already implied by the From and the Signed-off-by. Dan, your patches don't apply cleanly upstream, please rebase them against Marcel's bluetooth-next tree and do the proper split of the patches. -- Gustavo F. Padovan http://padovan.org -- 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
