Return-Path: MIME-Version: 1.0 In-Reply-To: <201011080624.49224.arnd@arndb.de> References: <20101031120429.1ca38140@pyx> <201011080624.49224.arnd@arndb.de> Date: Thu, 11 Nov 2010 15:28:51 +0100 Message-ID: Subject: Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900. From: Par-Gunnar Hjalmdahl To: Arnd Bergmann Cc: Alan Cox , linus.walleij@stericsson.com, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, Lukasz.Rymanowski@tieto.com Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: 2010/11/8 Arnd Bergmann : > On Friday 05 November 2010, Par-Gunnar Hjalmdahl wrote: >> 2010/10/31, Alan Cox : >> >> It's about the ldisc number. Both bluetooth and cg2900 register themselves >> >> to ldisc 15 (N_HCI). A user doing TIOCSETD can only get one of the two. >> > >> > Ah I see - I had assumed any actual final merge would be assigning it a >> > new LDISC code as is required. >> >> Sorry for not answering quicker. We in my department have been >> discussing new design as well as trying out some solutions. >> >> As an answer to previous comments, both from Arnd and Alan, we would >> like to do the following: >> ?- Introduce a new ldisc called N_H4_COMBO with a ldisc driver >> accordingly that will be placed under drivers/char > > I'm not convinced, although that's what Alan was suggesting. I'd like > to hear from the bluetooth people what they think about this. > > Could you summarize why you think that cg2900 is different enough from > an HCI to require a different line discipline? From your previous > explanation it sounded like it was mostly added functionality, > not something entirely different. > >> We were thinking about if we could re-use the existing hci_ldisc >> driver. As stated before the big problem here is however that >> hci_ldisc directly registers to the Bluetooth stack. We could separate >> the data for FM and GPS in the protocol driver, but it is pretty ugly >> to have two separate data paths within the same driver. > > One of us must be misreading the code. As far as I can tell, hci_ldisc > does not register to the bluetooth stack at all, it just provides > the basic multiplex for multiple HCI protocols, while hci_h4.c > communicates to the bluetooth stack. > > If I read it correctly, that means that you can still use hci_ldisc, > but need to add another protocol next to hci_h4 and hci_bcsp for > your cg2900. > If you look in function hci_ldisc.c/hci_uart_register_dev(), it here registers as a driver to the Bluetooth stack. This means that received Bluetooth packets would go ldisc->protocol->ldisc->bluetooth, while FM and GPS would go ldisc->protocol->(FM/GPS)stack. I think it's quite bad to have two different data paths like this. The new ldisc we're creating looks a lot like hci_ldisc, but it does not register itself to an overlaying stack directly. One option would of course be to modify the existing hci_ldisc.c but I feel that be rather dangerous and which could create a lot of problems. /P-G