Re: [RFC v2 7/9] bluetooth: btrtl: load the config blob from devicetree when available

[Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>]

On Wed, Jan 3, 2018 at 8:50 PM, Martin Blumenstingl
<martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> Hi Carlo,

Hi Martin,

>> As Marcel suggested we can assume that the information in the DSDT is
>> correct so that we can get rid of the config blob also for x86
>> platforms (assuming that the only useful information in the config
>> blobs is the UART configuration).
> in my tests I tried to send only the firmware without the config to my
> RTL8723BS. unfortunately the last firmware chunk (sent to the
> controller) times out in that case (even if I set a proper baudrate
> before or if I specify no baudrate at all and keep the serdev at
> 115200)

What's in the config blobs besides UART configuration?

It's odd because reading into hciattach_rtk.c it seems that the config
file is actually only used by the userspace tools (hciattach) to
retrieve the UART configuration and nothing else, whereas in the
kernel driver the config blob is appended to the firmware.

> have you tested this (= uploading the firmware without the config
> blob) on your x86 board before?

No, on the cherry-trail I have I'm using hciattach to upload the
firmware and AFAICT only the firmware is uploaded and (as written
before) the config blob is only used to get the UART configuration.

> so far the following solutions for the config blob were discussed:
> 1) put the config blob in userspace (/lib/firmware/...) -> not good
> because it makes the rootfs board-specific
> 2) auto-generate the config blob -> didn't work for me, last firmware
> chunk times out (just as if I had no config at all)
> 3) putting the config blob in DT -> possible but not very nice to read
>
> I had another idea:
> what if we mix solution 1) and 2)?
> the idea: load the config blob from userspace (/lib/firmware/...) and
> update it's settings with the values we got from devicetree-properties
> / DSDT

If you are shipping already the config blob in userspace I don't see
why you would do the update since you have already all the info you
need.
I would rather check why (2) didn't work fine. Have you any
documentation how the config blobs are generated? The code in
hciattach_rtk.c seems a good starting point.

> I have not tested this yet, but I just want to hear everyone's (at
> least Marcel, Rob and Carlo) opinion on that
> (this would also allow us to fully auto-generate the config blob in
> the future once we figure out how to do that)
>
>> Adding the ACPI support on top of your patches is (hopefully) trivial,
>> just follow what was done for hci_bcm.c, basically adding a new _HID
>> and reading the configuration for GPIOs and UART, all the rest should
>> be transparent for serdev.
> thanks for the reference to hci_bcm.c - I will have a look at this for
> the next version
> one question: "_HID" would be OBDA8723 in our case?

Yes

>> I'll test your patches on the hardware I have.
> great, thank you!

Cheers!

-- 
Carlo Caione  |  +44.7384.69.16.04  |  Endless