[linux-sunxi] Re: [PATCH 7/7] ARM: sun7i: cubietruck: enable bluetooth module

[Chen-Yu Tsai]

On Tue, Apr 15, 2014 at 10:42 PM, Maxime Ripard
<maxime.ripard at free-electrons.com> wrote:
> On Tue, Apr 15, 2014 at 02:41:41PM +0800, Chen-Yu Tsai wrote:
>> The CubieTruck has an AMPAK AP6210 WiFi+Bluetooth module. The Bluetooth
>> part is a BCM20710 device connected to UART2 in the A20 SoC.
>>
>> The IC requires a 32.768 KHz low power clock input for proper
>> auto-detection of the main clock, and an enable signal via GPIO.
>>
>> Signed-off-by: Chen-Yu Tsai <wens at csie.org>
>> ---
>>  arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 25 +++++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>> index cb25d3c..767c8e1 100644
>> --- a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>> +++ b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
>> @@ -61,6 +61,13 @@
>>                               allwinner,drive = <0>;
>>                               allwinner,pull = <0>;
>>                       };
>> +
>> +                     bt_pwr_pin_cubietruck: bt_pwr_pin at 0 {
>> +                             allwinner,pins = "PH18";
>> +                             allwinner,function = "gpio_out";
>> +                             allwinner,drive = <0>;
>> +                             allwinner,pull = <0>;
>> +                     };
>>               };
>>
>>               uart0: serial at 01c28000 {
>> @@ -69,6 +76,12 @@
>>                       status = "okay";
>>               };
>>
>> +             uart2: serial at 01c28800 {
>> +                     pinctrl-names = "default";
>> +                     pinctrl-0 = <&uart2_pins_a>;
>> +                     status = "okay";
>> +             };
>> +
>
> Please make this a separate patch.

Will do.

>>               i2c0: i2c at 01c2ac00 {
>>                       pinctrl-names = "default";
>>                       pinctrl-0 = <&i2c0_pins_a>;
>> @@ -139,4 +152,16 @@
>>       reg_usb2_vbus: usb2-vbus {
>>               status = "okay";
>>       };
>> +
>> +     rfkill_bt {
>> +             compatible = "rfkill-gpio";
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&bt_pwr_pin_cubietruck>, <&clk_out_a_pins_a>;
>> +             clocks = <&clk_out_a>;
>> +             clock-frequency = <32768>;
>> +             gpios = <&pio 7 18 0>; /* PH18 */
>> +             gpio-names = "reset";
>> +             rfkill-name = "bt";
>> +             rfkill-type = <2>;
>> +     };
>
> Hmmm, I don't think that's actually right.
>
> If you have such a device, then I'd expect it to be represented as a
> full device in the DT, probably with one part for the WiFi, one part
> for the Bluetooth, and here the definition of the rfkill device that
> controls it.

The AP6210 is not one device, but 2 separate chips in one module. Each
chip has its own controls and interface. They just so happen to share
the same enclosure. Even 2-in-1 chips by Broadcom have separate controls
and interfaces. The WiFi side is most likely connected via SDIO, while
the Bluetooth side is connected to a UART, and optionally I2S for sound.

> But tying parts of the device to the rfkill that controls it, such as
> the clocks, or the frequency it runs at seems just wrong.

I understand where you're coming from. For devices on buses that require
drivers (such as USB, SDIO) these properties probably should be tied to
the device node.

For our use case here, which is a bluetooth chip connected on the UART,
there is no in kernel representation or driver to tie them to. Same goes
for UART based GPS chips. They just so happen to require toggling a GPIO,
and maybe enabling a specific clock, to get it running. Afterwards,
accessing it is done solely from userspace. For our Broadcom chips, the
user has to upload its firmware first, then designate the tty as a Bluetooth
HCI using hciattach.

We are using the rfkill device as a on-off switch.

Hope this explains the situation.


Cheers
ChenYu