Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero

Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero

[Alexey Kardashevskiy]

On 29/11/16 21:19, Icenowy Zheng wrote:
> 
> 2016年11月29日 15:16于 Alexey Kardashevskiy <aik@ozlabs.ru>写道：
>>
>>
>>
>> On Wed, Nov 23, 2016 at 6:59 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>>> > There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>>> > Orange Pi Zero, which used a dedicated regulator to power.
>>> >
>>> > Add the device tree node of the regulator, the enable gpio (with
>>> > mmc-pwrseq) and the sdio controller.
>>> >
>>> > There's a out-of-tree driver tested to work with this device tree.
>>
>>
>> btw could you please give a pointer where to find a XR819 driver for
> relatively recent kernel (4.8 may be, just not 3.4)? Thanks.
> 
> https://github.com/Icenowy/xradio

Thanks!

I tried, cannot make it work though.

This compiles:

CONFIG_XRADIO=y
CONFIG_XRADIO_SDIO=y
CONFIG_XRADIO_NON_POWER_OF_TWO_BLOCKSIZES=y
# CONFIG_XRADIO_USE_GPIO_IRQ is not set
# CONFIG_XRADIO_5GHZ_SUPPORT is not set
# CONFIG_XRADIO_WAPI_SUPPORT is not set
CONFIG_XRADIO_USE_EXTENSIONS=y

But produces:

[    0.964793] [XRADIO] Driver Label:L34M.01.08.0002
[    0.969659] [XRADIO] Allocated hw_priv @ df684dc0
[    3.038167] [SBUS_ERR] sdio probe timeout!
[    3.046283] [XRADIO_ERR] sbus_sdio_init failed


If I enable CONFIG_XRADIO_USE_GPIO_IRQ (which Kconfig suggest to enable -
do I really need it?), it does not compile:

drivers/built-in.o: In function `sdio_irq_subscribe':
sunxi_sid.c:(.text+0x12dfe8): undefined reference to `xradio_request_gpio_irq'
sunxi_sid.c:(.text+0x12e06c): undefined reference to `xradio_free_gpio_irq'
sunxi_sid.c:(.text+0x12e080): undefined reference to `sunxi_mci_check_r1_ready'
drivers/built-in.o: In function `sdio_irq_unsubscribe':
sunxi_sid.c:(.text+0x12e3b0): undefined reference to `xradio_free_gpio_irq'
Makefile:962: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1


I am using recent upstream kernel and
https://patchwork.kernel.org/patch/9439759/
https://patchwork.kernel.org/patch/9439761/

and I added missing emac.

Basically, here is my current tree with all sun8xi support
https://github.com/aik/linux/commits/pizero

and I just did
git -C drivers/net/ethernet clone https://github.com/Icenowy/xradio

and compiled it into vmlinux (no modules).

What am I missing? Sorry I am a newbie with ARM. Thanks.


-- 
Alexey

Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero

[Andre Przywara]

Hi,

On 29/11/16 10:19, Icenowy Zheng wrote:
> 
> 2016年11月29日 15:16于 Alexey Kardashevskiy <aik@ozlabs.ru>写道：
>>
>>
>>
>> On Wed, Nov 23, 2016 at 6:59 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>>> > There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>>> > Orange Pi Zero, which used a dedicated regulator to power.
>>> >
>>> > Add the device tree node of the regulator, the enable gpio (with
>>> > mmc-pwrseq) and the sdio controller.
>>> >
>>> > There's a out-of-tree driver tested to work with this device tree.
>>
>>
>> btw could you please give a pointer where to find a XR819 driver for
> relatively recent kernel (4.8 may be, just not 3.4)? Thanks.
> 
> https://github.com/Icenowy/xradio

I was just curious, so pulled your tree and tried to just compile it. It
still threw warnings at me for ARM, and even more so for arm64.
I fixed all of them and put that on my github[1]. Feel free to just pick
them from there or wait till I manage to clean them up and send you a
pull request.
And also just a a test, I quickly put it in drivers/net/wireless/xradio,
where it compiled fine after registering it with the upper level Kconfig
and Makefile.

And while looking at it: This looks like typical AW code, not even
remotely upstreameable and probably far too complicated. Enabling some
Kconfig options made it complain about missing functions.
Has anyone checked if this is close to an existing WiFi chip? That
wouldn't be a first ;-)

Cheers,
Andre.

[1] https://github.com/apritzel/xradio/commits/quickfixes

Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Wed, Nov 23, 2016 at 10:25:57PM +0800, Chen-Yu Tsai wrote:
> >>  &r_pio {
> >> @@ -111,6 +148,11 @@
> >>               pins = "PL10";
> >>               function = "gpio_out";
> >>       };
> >> +
> >> +     wifi_pwrseq_pin_opi0: wifi_pwrseq_pin@0 {
> >> +             allwinner,pins = "PL7";
> >> +             allwinner,function = "gpio_out";
> >
> > And same thing here.
> 
> Might we do away with the pinmux for gpio pins tradition?
> Recent patches I've sent all omit them.

Oh, yes, that's true.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero

[Hans de Goede]

Hi,

On 23-11-16 15:25, Chen-Yu Tsai wrote:
> On Wed, Nov 23, 2016 at 3:59 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> Hi,
>>
>> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>>> There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>>> Orange Pi Zero, which used a dedicated regulator to power.
>>>
>>> Add the device tree node of the regulator, the enable gpio (with
>>> mmc-pwrseq) and the sdio controller.
>>>
>>> There's a out-of-tree driver tested to work with this device tree.
>>>
>>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>>> ---
>>> New patch in the patchset, since a out-of-tree working xradio driver is done.
>>>
>>> If there is any problem in this patch, it can be omitted.
>>
>> No particular problem with this one, however it can and should be
>> merged with the previous one.
>>
>> Minor comments below though.
>>
>>>
>>>  arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts | 42 ++++++++++++++++++++++++
>>>  1 file changed, 42 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> index b428e47..39cac26 100644
>>> --- a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> +++ b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>> @@ -79,6 +79,24 @@
>>>                       gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
>>>               };
>>>       };
>>> +
>>> +     reg_vcc_wifi: reg_vcc_wifi {
>>> +             compatible = "regulator-fixed";
>>> +             pinctrl-names = "default";
>>> +             pinctrl-0 = <&vcc_wifi_pin_opi0>;
>>> +             regulator-min-microvolt = <3300000>;
>>> +             regulator-max-microvolt = <3300000>;
>>> +             regulator-name = "vcc-wifi";
>>> +             enable-active-high;
>>> +             gpio = <&pio 0 20 GPIO_ACTIVE_HIGH>;
>>> +     };
>>> +
>>> +     wifi_pwrseq: wifi_pwrseq {
>>> +             compatible = "mmc-pwrseq-simple";
>>> +             pinctrl-names = "default";
>>> +             pinctrl-0 = <&wifi_pwrseq_pin_opi0>;
>>> +             reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
>>> +     };
>>>  };
>>>
>>>  &ehci1 {
>>> @@ -95,6 +113,20 @@
>>>       status = "okay";
>>>  };
>>>
>>> +&mmc1 {
>>> +     pinctrl-names = "default";
>>> +     pinctrl-0 = <&mmc1_pins_a>;
>>> +     vmmc-supply = <&reg_vcc_wifi>;
>>> +     mmc-pwrseq = <&wifi_pwrseq>;
>>> +     bus-width = <4>;
>>> +     non-removable;
>>> +     status = "okay";
>>> +};
>>> +
>>> +&mmc1_pins_a {
>>> +     allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>>
>> This should be bias-pull-up.
>
> IIRC I already added this for _all_ existing mmc pinmux settings?
>
>>
>>> +};
>>> +
>>>  &ohci1 {
>>>       status = "okay";
>>>  };
>>> @@ -104,6 +136,11 @@
>>>               pins = "PA17";
>>>               function = "gpio_out";
>>>       };
>>> +
>>> +     vcc_wifi_pin_opi0: vcc_wifi_pin@0 {
>>> +             allwinner,pins = "PA20";
>>
>> This should be pins
>>
>>> +             allwinner,function = "gpio_out";
>>
>> This should be function
>>
>>> +     };
>>>  };
>>>
>>>  &r_pio {
>>> @@ -111,6 +148,11 @@
>>>               pins = "PL10";
>>>               function = "gpio_out";
>>>       };
>>> +
>>> +     wifi_pwrseq_pin_opi0: wifi_pwrseq_pin@0 {
>>> +             allwinner,pins = "PL7";
>>> +             allwinner,function = "gpio_out";
>>
>> And same thing here.
>
> Might we do away with the pinmux for gpio pins tradition?
> Recent patches I've sent all omit them.

I'm in favor of doing away with them, except there were
we need to configure bias / strength.

Regards,

Hans

Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero

[Chen-Yu Tsai]

On Wed, Nov 23, 2016 at 3:59 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>> There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>> Orange Pi Zero, which used a dedicated regulator to power.
>>
>> Add the device tree node of the regulator, the enable gpio (with
>> mmc-pwrseq) and the sdio controller.
>>
>> There's a out-of-tree driver tested to work with this device tree.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> ---
>> New patch in the patchset, since a out-of-tree working xradio driver is done.
>>
>> If there is any problem in this patch, it can be omitted.
>
> No particular problem with this one, however it can and should be
> merged with the previous one.
>
> Minor comments below though.
>
>>
>>  arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts | 42 ++++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>> index b428e47..39cac26 100644
>> --- a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>> +++ b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>> @@ -79,6 +79,24 @@
>>                       gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
>>               };
>>       };
>> +
>> +     reg_vcc_wifi: reg_vcc_wifi {
>> +             compatible = "regulator-fixed";
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&vcc_wifi_pin_opi0>;
>> +             regulator-min-microvolt = <3300000>;
>> +             regulator-max-microvolt = <3300000>;
>> +             regulator-name = "vcc-wifi";
>> +             enable-active-high;
>> +             gpio = <&pio 0 20 GPIO_ACTIVE_HIGH>;
>> +     };
>> +
>> +     wifi_pwrseq: wifi_pwrseq {
>> +             compatible = "mmc-pwrseq-simple";
>> +             pinctrl-names = "default";
>> +             pinctrl-0 = <&wifi_pwrseq_pin_opi0>;
>> +             reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
>> +     };
>>  };
>>
>>  &ehci1 {
>> @@ -95,6 +113,20 @@
>>       status = "okay";
>>  };
>>
>> +&mmc1 {
>> +     pinctrl-names = "default";
>> +     pinctrl-0 = <&mmc1_pins_a>;
>> +     vmmc-supply = <&reg_vcc_wifi>;
>> +     mmc-pwrseq = <&wifi_pwrseq>;
>> +     bus-width = <4>;
>> +     non-removable;
>> +     status = "okay";
>> +};
>> +
>> +&mmc1_pins_a {
>> +     allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
>
> This should be bias-pull-up.

IIRC I already added this for _all_ existing mmc pinmux settings?

>
>> +};
>> +
>>  &ohci1 {
>>       status = "okay";
>>  };
>> @@ -104,6 +136,11 @@
>>               pins = "PA17";
>>               function = "gpio_out";
>>       };
>> +
>> +     vcc_wifi_pin_opi0: vcc_wifi_pin@0 {
>> +             allwinner,pins = "PA20";
>
> This should be pins
>
>> +             allwinner,function = "gpio_out";
>
> This should be function
>
>> +     };
>>  };
>>
>>  &r_pio {
>> @@ -111,6 +148,11 @@
>>               pins = "PL10";
>>               function = "gpio_out";
>>       };
>> +
>> +     wifi_pwrseq_pin_opi0: wifi_pwrseq_pin@0 {
>> +             allwinner,pins = "PL7";
>> +             allwinner,function = "gpio_out";
>
> And same thing here.

Might we do away with the pinmux for gpio pins tradition?
Recent patches I've sent all omit them.

ChenYu

Re: [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2865 bytes --]

Hi,

On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
> There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
> Orange Pi Zero, which used a dedicated regulator to power.
> 
> Add the device tree node of the regulator, the enable gpio (with
> mmc-pwrseq) and the sdio controller.
> 
> There's a out-of-tree driver tested to work with this device tree.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> ---
> New patch in the patchset, since a out-of-tree working xradio driver is done.
> 
> If there is any problem in this patch, it can be omitted.

No particular problem with this one, however it can and should be
merged with the previous one.

Minor comments below though.

> 
>  arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts | 42 ++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
> index b428e47..39cac26 100644
> --- a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
> +++ b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
> @@ -79,6 +79,24 @@
>  			gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
>  		};
>  	};
> +
> +	reg_vcc_wifi: reg_vcc_wifi {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&vcc_wifi_pin_opi0>;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-name = "vcc-wifi";
> +		enable-active-high;
> +		gpio = <&pio 0 20 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	wifi_pwrseq: wifi_pwrseq {
> +		compatible = "mmc-pwrseq-simple";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wifi_pwrseq_pin_opi0>;
> +		reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
> +	};
>  };
>  
>  &ehci1 {
> @@ -95,6 +113,20 @@
>  	status = "okay";
>  };
>  
> +&mmc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc1_pins_a>;
> +	vmmc-supply = <&reg_vcc_wifi>;
> +	mmc-pwrseq = <&wifi_pwrseq>;
> +	bus-width = <4>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&mmc1_pins_a {
> +	allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;

This should be bias-pull-up.

> +};
> +
>  &ohci1 {
>  	status = "okay";
>  };
> @@ -104,6 +136,11 @@
>  		pins = "PA17";
>  		function = "gpio_out";
>  	};
> +
> +	vcc_wifi_pin_opi0: vcc_wifi_pin@0 {
> +		allwinner,pins = "PA20";

This should be pins

> +		allwinner,function = "gpio_out";

This should be function

> +	};
>  };
>  
>  &r_pio {
> @@ -111,6 +148,11 @@
>  		pins = "PL10";
>  		function = "gpio_out";
>  	};
> +
> +	wifi_pwrseq_pin_opi0: wifi_pwrseq_pin@0 {
> +		allwinner,pins = "PL7";
> +		allwinner,function = "gpio_out";

And same thing here.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]