[bionic][PATCH 00/10] Add drivers for RTL8821C WiFi and BT

[Jesse Sung]

Hi Seth,

On Tue, Mar 27, 2018 at 10:10 PM, Seth Forshee
<seth.forshee at canonical.com> wrote:
> On Sat, Mar 24, 2018 at 12:46:57AM +0800, Wen-chien Jesse Sung wrote:
>> BugLink: https://launchpad.net/bugs/1740231
>> BugLink: https://launchpad.net/bugs/1742613
>>
>> These are based on the drivers provided by Realtek for 4.13.
>>
>> Since btusb will bind to this bluetooth device, it must be blacklisted
>> in btusb to make sure that the correct driver is used. Also the table in
>> ubuntu/rtl8821c-bt is modified so that the driver works only for this
>> device only.
>>
>> Risk should be low since both drivers have no impact for systems without
>> these devices.
>
> This is a driver with almost 600,000 lines of code, submitted at the
> last minute without any explanation of why we should be adding it to our
> kernel. Please explain why this is needed.

This device was enabled on some HP laptops. Right now we're supporting
it with linux-oem 4.13 in Xenial. In order to keep this device working after
upcoming linux-oem upgrade/migration, the best way is to integrate this into
bionic kernel.

>
> Some of my objections/questions from just a quick skim:
>
>  - Being built with our kernel, the driver will be signed and thus
>    eligible to be loaded during secure boot. There's no chance that we
>    can review it to ensure that it doesn't expose any unsafe interfaces
>    to userspace. Have you done such a review?

For platforms w/o this device, if one can load a module to expose something,
he can always load anything harmful directly anyway. But for platforms w/ the
device, I'd admit that I have the same doubt as you.

>
>  - Why does this need to hook into our debian build scripts rather than
>    being built like a normal module?

I tried to keep the modification to the driver as minimal as possible, in case
that there's an update (a new tarball) from Realtek. But yes, I can modify
the Makefile to make it look like a normal module if it's preferred.

>
>  - You blacklist usb device 0bda:b00a for all architectures but only
>    build the replacement driver for x86. Is there any chance someone
>    might want to use the device with another architecture? If so you'll
>    be breaking them.

Since the btusb doesn't work with this device, this may not be a problem.

Thanks,
Jesse

>
>  - Presumably we'll have to keep maintaining this huge pile of code for
>    years into the future, up to and including the next LTS I expect.
>
> Thanks,
> Seth