Re: [Letux-kernel] [PATCH RFC] bluetooth: add uart h4 devices via serdev/devicetree

From: Andreas Kemnade
Date: Wed Nov 14 2018 - 06:13:59 EST


On Wed, 14 Nov 2018 08:51:17 +0100
Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:

> Hi Andreas,
>
> >>>>> Am 12.11.2018 um 21:59 schrieb Andreas Kemnade <andreas@xxxxxxxxxxxx>:
> >>>>> On Sun, 11 Nov 2018 03:46:48 +0100
> >>>>> Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> wrote:
> >>>>>> On Sun, Nov 11, 2018 at 12:20:34AM +0100, Andreas Kemnade wrote:
> >>>>>>> This is a first try to be able to use h4 devices specified in
> >>>>>>> the devicetree, so you do not need to call hciattach and
> >>>>>>> it can be automatically probed.
> >>>>>>>
> >>>>>>> Of course, proper devicetree bindings documentation is
> >>>>>>> missing. And also you would extend that by regulator/
> >>>>>>> enable gpio settings.
> >>>>>>>
> >>>>>>> But before proceeding further it should be checked if the
> >>>>>>> general way of doing things is right.
> >>>>>>>
> >>>>>>> Signed-off-by: Andreas Kemnade <andreas@xxxxxxxxxxxx>
> >>>>>>> ---
> >>>>>>
> >>>>>> Patch looks good to me, just one note
> >>>>>>
> >>>>> I found one thing myself:
> >>>>> Shouldn't we have a generic compatible string like "generic-h4".
> >>>>> ehci-platform.c has for example:
> >>>>> { .compatible = "generic-ehci", },
> >>>>
> >>>> There might be differences in h4 compatible devices (e.g. default
> >>>> baud rate) so that I would not bet there a "generic-h4" suffices
> >>>> in the long run.
> >>
> >> It will not because that doesn't define clocks, resets, gpios,
> >> supplies, etc. and the interactions of all those.
> >>
> > well, we need a simple supply being on when the device is on.
> > Nothing more.
> >
> >>> My suggestion is to use this in DT:
> >>>
> >>> compatible = "wi2wi,w2cbw003-bluetooth", "<something generic>";
> >>>
> > That would be my slight preference here.
> >
> >>> The driver can start with supporting just the generic compatible
> >>> string. If somebody finds some incompatible differences, the driver
> >>> can add a custom handler for the wi2wi chip without breaking DT
> >>> ABI.
> >>
> >> Any idea how many H4 devices there are? Somehow I doubt there are that
> >> many to be unmanageable.
> >>
> > Well, many devices are h4 devices with some more or less important
> > vendor-specific commands. Well, "hciattach any" uses simple h4 protocol.
> >
> > those firmware download commands and they have their own drivers.
> > Most devices I had used bluetooth uart from the command line with, were
> > simple enough. The other question is whether those devices will run a
> > modern kernel.
> >
> > No strong opinion here.
>
> doing the firmware load from user space via some magic tool is no longer going to work smoothly. It will be actually almost impossible with serdev. However I did post btuart.c driver which is pretty much just plain H:4. You would still somehow define the default baudraute since just picking 115200 is not always going to work.
>
Just to avoid some misunderstandings here. With this RFC patch I have only
devices in mind where we do not know any vendor specific commands and
do not need to do any firmware download.

> Btw. I see nothing standing in the way of merging btuart.c driver and then go from there. Either I dig this out and submit or someone else does.
>
Seems to be a good idea. So in the end I should not continue
that rtc patch?

And that thing has than a devicetree string of
"h4-generic" or a list of generic h4 devices?

Regards,
Andreas

Attachment: pgpDBubRLZsMe.pgp
Description: OpenPGP digital signature