Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.

[Par-Gunnar Hjalmdahl]

2010/11/8 Arnd Bergmann <arnd@xxxxxxxx>:
> On Friday 05 November 2010, Par-Gunnar Hjalmdahl wrote:
>> 2010/10/31, Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>:
>> >> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/