Hi Mat, > > On Wed, Feb 8, 2012 at 7:32 AM, Luiz Augusto von Dentz > > <luiz.dentz@xxxxxxxxx> wrote: > >> Hi Andrei, > >> > >> On Wed, Feb 8, 2012 at 11:09 AM, Andrei Emeltchenko > >> <andrei.emeltchenko.news@xxxxxxxxx> wrote: > >>> > >>> I think this would be good to send as patch series so that people can > >>> comment. What comes to my mind is that the patch might be reduced if it > >>> does not change order of functions and defines like: > >>> > > ... <snip> ... > > >>> those magic number does not look nice IMO and the code is not looking any > >>> better. > >> > >> In this aspect perhaps, but we don't need to take the code as it is, > >> but the point here is following the states defined by the spec and > >> that is IMO much better. > > Right - the code as it stands now was ported quickly, and does not > take in to account many of Andrei's upstream improvements. I don't > want to move backwards in terms of magic numbers, and there is some > extra noise in there due to symbol names. I need to address these > issues before merging. > > I'm also looking for ideas on how to do this as a patch series. Since > it takes a very different approach from the original code, it's hard > to make meaningful, small patches that don't break functionality. At > some point there has to be a major switchover but there is a lot of > room to split this up. > > One approach is to add inactive code until everything is there, then > have one commit that calls in to the new code instead of the old code. > Then the old code can be removed. I talked to Gustavo about that a > while ago, and he preferred finding a different way. Maybe an > intermediate step is to put the state machine code in there, but call > the existing frame handling functions in every state. I am fine with this approach. We must make sure that we have a complete series and be merging it at once. Otherwise dealing with compiler warnings in -next trees is just creating too much noise. And to make this crystal clear to everybody. I want _ONE_ upstream L2CAP ERTM implementation and no future fragmentation. I am dealing with this issue once and after that is has to be upstream first. No exceptions. > > I've taken a quick look last week at their code and indeed it looks > > better regarding following the states. In particular they track > > rx_state and tx_state and then decide what to do based on that and > > what happened. I like it because it's explicit about that instead of > > demanding us to reason a lot on what state we are and what we should > > do. I'm all for changing our ERTM to that so it'll be more > > maintainable. > > That was the goal of the design. It has worked well during in-house > testing, and has been through several UPFs. > > > > Marcel mentioned the separation of L2CAP channel and socket. That is a > > work in progress by Andrei and judging by what you said, Marcel, you > > want that merged before we change ERTM, is that it? > > Andrei and Marcel, let's figure out this order now. It doesn't make > sense for one of us to have a bunch of changes staged, only to have > major merge/rebase conflicts when another far-reaching patch set gets > merged. This is between you and Andrei. I want to see the L2CAP channel vs socket split happening. I do not care in which order. You guys agree on a way forward and I will just go along with it. Regards Marcel -- 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
