arm64: dts: ulcb-kf: Add support for TI WL1837
diff mbox series

Message ID 20190411124102.22442-1-spapageorgiou@de.adit-jv.com
State Accepted
Delegated to: Simon Horman
Headers show
Series
  • arm64: dts: ulcb-kf: Add support for TI WL1837
Related show

Commit Message

Spyridon Papageorgiou April 11, 2019, 12:41 p.m. UTC
This patch adds description of TI WL1837 and links interfaces
to communicate with the IC, namely the SDIO interface to WLAN.

Signed-off-by: Spyridon Papageorgiou <spapageorgiou@de.adit-jv.com>
---
 arch/arm64/boot/dts/renesas/ulcb-kf.dtsi | 49 ++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Comments

Eugeniu Rosca April 25, 2019, 11:12 a.m. UTC | #1
Hi Simon,

Do we have any chance getting this upstream? If so, would you kindly
list the acceptance criteria we have to conform to?

Many thanks.

On Thu, Apr 11, 2019 at 02:41:03PM +0200, Spyridon Papageorgiou wrote:
> This patch adds description of TI WL1837 and links interfaces
> to communicate with the IC, namely the SDIO interface to WLAN.
> 
> Signed-off-by: Spyridon Papageorgiou <spapageorgiou@de.adit-jv.com>
> ---
>  arch/arm64/boot/dts/renesas/ulcb-kf.dtsi | 49 ++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> index 7a09576..27851a7 100644
> --- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> +++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> @@ -38,6 +38,18 @@
>  		regulator-min-microvolt = <5000000>;
>  		regulator-max-microvolt = <5000000>;
>  	};
> +
> +	wlan_en: regulator-wlan_en {
> +		compatible = "regulator-fixed";
> +		regulator-name = "wlan-en-regulator";
> +
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +
> +		gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
> +		startup-delay-us = <70000>;
> +		enable-active-high;
> +	};
>  };
>  
>  &can0 {
> @@ -88,6 +100,13 @@
>  			line-name = "Audio_Out_OFF";
>  		};
>  
> +		sd-wifi-mux {
> +			gpio-hog;
> +			gpios = <5 GPIO_ACTIVE_HIGH>;
> +			output-low;	/* Connect WL1837 */
> +			line-name = "SD WiFi mux";
> +		};
> +
>  		hub_pwen {
>  			gpio-hog;
>  			gpios = <6 GPIO_ACTIVE_HIGH>;
> @@ -254,6 +273,12 @@
>  		function = "scif1";
>  	};
>  
> +	sdhi3_pins: sdhi3 {
> +		groups = "sdhi3_data4", "sdhi3_ctrl";
> +		function = "sdhi3";
> +		power-source = <3300>;
> +	};
> +
>  	usb0_pins: usb0 {
>  		groups = "usb0";
>  		function = "usb0";
> @@ -273,6 +298,30 @@
>  	status = "okay";
>  };
>  
> +&sdhi3 {
> +	pinctrl-0 = <&sdhi3_pins>;
> +	pinctrl-names = "default";
> +
> +	vmmc-supply = <&wlan_en>;
> +	vqmmc-supply = <&wlan_en>;
> +	bus-width = <4>;
> +	no-1-8-v;
> +	non-removable;
> +	cap-power-off-card;
> +	keep-power-in-suspend;
> +	max-frequency = <26000000>;
> +	status = "okay";
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	wlcore: wlcore@2 {
> +		compatible = "ti,wl1837";
> +		reg = <2>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> +	};
> +};
> +
>  &usb2_phy0 {
>  	pinctrl-0 = <&usb0_pins>;
>  	pinctrl-names = "default";
> -- 
> 2.7.4
>
Simon Horman April 26, 2019, 9:50 a.m. UTC | #2
Hi,

from my point of view what is required is a review.
I will try to find someone to do so.
I apologise for not doing so earlier.

On Thu, Apr 25, 2019 at 01:12:45PM +0200, Eugeniu Rosca wrote:
> Hi Simon,
> 
> Do we have any chance getting this upstream? If so, would you kindly
> list the acceptance criteria we have to conform to?
> 
> Many thanks.
> 
> On Thu, Apr 11, 2019 at 02:41:03PM +0200, Spyridon Papageorgiou wrote:
> > This patch adds description of TI WL1837 and links interfaces
> > to communicate with the IC, namely the SDIO interface to WLAN.
> > 
> > Signed-off-by: Spyridon Papageorgiou <spapageorgiou@de.adit-jv.com>
> > ---
> >  arch/arm64/boot/dts/renesas/ulcb-kf.dtsi | 49 ++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> > index 7a09576..27851a7 100644
> > --- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> > @@ -38,6 +38,18 @@
> >  		regulator-min-microvolt = <5000000>;
> >  		regulator-max-microvolt = <5000000>;
> >  	};
> > +
> > +	wlan_en: regulator-wlan_en {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "wlan-en-regulator";
> > +
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +
> > +		gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
> > +		startup-delay-us = <70000>;
> > +		enable-active-high;
> > +	};
> >  };
> >  
> >  &can0 {
> > @@ -88,6 +100,13 @@
> >  			line-name = "Audio_Out_OFF";
> >  		};
> >  
> > +		sd-wifi-mux {
> > +			gpio-hog;
> > +			gpios = <5 GPIO_ACTIVE_HIGH>;
> > +			output-low;	/* Connect WL1837 */
> > +			line-name = "SD WiFi mux";
> > +		};
> > +
> >  		hub_pwen {
> >  			gpio-hog;
> >  			gpios = <6 GPIO_ACTIVE_HIGH>;
> > @@ -254,6 +273,12 @@
> >  		function = "scif1";
> >  	};
> >  
> > +	sdhi3_pins: sdhi3 {
> > +		groups = "sdhi3_data4", "sdhi3_ctrl";
> > +		function = "sdhi3";
> > +		power-source = <3300>;
> > +	};
> > +
> >  	usb0_pins: usb0 {
> >  		groups = "usb0";
> >  		function = "usb0";
> > @@ -273,6 +298,30 @@
> >  	status = "okay";
> >  };
> >  
> > +&sdhi3 {
> > +	pinctrl-0 = <&sdhi3_pins>;
> > +	pinctrl-names = "default";
> > +
> > +	vmmc-supply = <&wlan_en>;
> > +	vqmmc-supply = <&wlan_en>;
> > +	bus-width = <4>;
> > +	no-1-8-v;
> > +	non-removable;
> > +	cap-power-off-card;
> > +	keep-power-in-suspend;
> > +	max-frequency = <26000000>;
> > +	status = "okay";
> > +
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	wlcore: wlcore@2 {
> > +		compatible = "ti,wl1837";
> > +		reg = <2>;
> > +		interrupt-parent = <&gpio1>;
> > +		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> > +	};
> > +};
> > +
> >  &usb2_phy0 {
> >  	pinctrl-0 = <&usb0_pins>;
> >  	pinctrl-names = "default";
> > -- 
> > 2.7.4
> > 
> 
> -- 
> Best regards,
> Eugeniu.
>
Simon Horman April 29, 2019, 8:17 a.m. UTC | #3
Hi again,

I have been able to solicit a limited private review of this patch and
have gone ahead and applied it for inclusion in v5.2.

On Fri, Apr 26, 2019 at 11:50:12AM +0200, Simon Horman wrote:
> Hi,
> 
> from my point of view what is required is a review.
> I will try to find someone to do so.
> I apologise for not doing so earlier.
> 
> On Thu, Apr 25, 2019 at 01:12:45PM +0200, Eugeniu Rosca wrote:
> > Hi Simon,
> > 
> > Do we have any chance getting this upstream? If so, would you kindly
> > list the acceptance criteria we have to conform to?
> > 
> > Many thanks.
> > 
> > On Thu, Apr 11, 2019 at 02:41:03PM +0200, Spyridon Papageorgiou wrote:
> > > This patch adds description of TI WL1837 and links interfaces
> > > to communicate with the IC, namely the SDIO interface to WLAN.
> > > 
> > > Signed-off-by: Spyridon Papageorgiou <spapageorgiou@de.adit-jv.com>
> > > ---
> > >  arch/arm64/boot/dts/renesas/ulcb-kf.dtsi | 49 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 49 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> > > index 7a09576..27851a7 100644
> > > --- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> > > @@ -38,6 +38,18 @@
> > >  		regulator-min-microvolt = <5000000>;
> > >  		regulator-max-microvolt = <5000000>;
> > >  	};
> > > +
> > > +	wlan_en: regulator-wlan_en {
> > > +		compatible = "regulator-fixed";
> > > +		regulator-name = "wlan-en-regulator";
> > > +
> > > +		regulator-min-microvolt = <3300000>;
> > > +		regulator-max-microvolt = <3300000>;
> > > +
> > > +		gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
> > > +		startup-delay-us = <70000>;
> > > +		enable-active-high;
> > > +	};
> > >  };
> > >  
> > >  &can0 {
> > > @@ -88,6 +100,13 @@
> > >  			line-name = "Audio_Out_OFF";
> > >  		};
> > >  
> > > +		sd-wifi-mux {
> > > +			gpio-hog;
> > > +			gpios = <5 GPIO_ACTIVE_HIGH>;
> > > +			output-low;	/* Connect WL1837 */
> > > +			line-name = "SD WiFi mux";
> > > +		};
> > > +
> > >  		hub_pwen {
> > >  			gpio-hog;
> > >  			gpios = <6 GPIO_ACTIVE_HIGH>;
> > > @@ -254,6 +273,12 @@
> > >  		function = "scif1";
> > >  	};
> > >  
> > > +	sdhi3_pins: sdhi3 {
> > > +		groups = "sdhi3_data4", "sdhi3_ctrl";
> > > +		function = "sdhi3";
> > > +		power-source = <3300>;
> > > +	};
> > > +
> > >  	usb0_pins: usb0 {
> > >  		groups = "usb0";
> > >  		function = "usb0";
> > > @@ -273,6 +298,30 @@
> > >  	status = "okay";
> > >  };
> > >  
> > > +&sdhi3 {
> > > +	pinctrl-0 = <&sdhi3_pins>;
> > > +	pinctrl-names = "default";
> > > +
> > > +	vmmc-supply = <&wlan_en>;
> > > +	vqmmc-supply = <&wlan_en>;
> > > +	bus-width = <4>;
> > > +	no-1-8-v;
> > > +	non-removable;
> > > +	cap-power-off-card;
> > > +	keep-power-in-suspend;
> > > +	max-frequency = <26000000>;
> > > +	status = "okay";
> > > +
> > > +	#address-cells = <1>;
> > > +	#size-cells = <0>;
> > > +	wlcore: wlcore@2 {
> > > +		compatible = "ti,wl1837";
> > > +		reg = <2>;
> > > +		interrupt-parent = <&gpio1>;
> > > +		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> > > +	};
> > > +};
> > > +
> > >  &usb2_phy0 {
> > >  	pinctrl-0 = <&usb0_pins>;
> > >  	pinctrl-names = "default";
> > > -- 
> > > 2.7.4
> > > 
> > 
> > -- 
> > Best regards,
> > Eugeniu.
> > 
>
Eugeniu Rosca April 29, 2019, 9:25 a.m. UTC | #4
Hi Simon,

On Mon, Apr 29, 2019 at 10:17:48AM +0200, Simon Horman wrote:
> Hi again,
> 
> I have been able to solicit a limited private review of this patch and
> have gone ahead and applied it for inclusion in v5.2.

Thank you. In case of any concerns/reports/fixes applicable to this
patch, we would appreciate if you CC ADIT people. We will then reply
with best possible latency, since we are interested in having a working
WiFi/BT on ULCB-KF.
Geert Uytterhoeven May 28, 2019, 9:19 a.m. UTC | #5
Hi Spyridon,

On Thu, Apr 11, 2019 at 2:42 PM Spyridon Papageorgiou
<spapageorgiou@de.adit-jv.com> wrote:
> This patch adds description of TI WL1837 and links interfaces
> to communicate with the IC, namely the SDIO interface to WLAN.
>
> Signed-off-by: Spyridon Papageorgiou <spapageorgiou@de.adit-jv.com>

Thanks for your patch!

> --- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> +++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> @@ -38,6 +38,18 @@
>                 regulator-min-microvolt = <5000000>;
>                 regulator-max-microvolt = <5000000>;
>         };
> +
> +       wlan_en: regulator-wlan_en {
> +               compatible = "regulator-fixed";
> +               regulator-name = "wlan-en-regulator";
> +
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;

So this is a 3.3V regulator...

> +
> +               gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
> +               startup-delay-us = <70000>;
> +               enable-active-high;
> +       };
>  };
>
>  &can0 {

> @@ -273,6 +298,30 @@
>         status = "okay";
>  };
>
> +&sdhi3 {
> +       pinctrl-0 = <&sdhi3_pins>;
> +       pinctrl-names = "default";
> +
> +       vmmc-supply = <&wlan_en>;
> +       vqmmc-supply = <&wlan_en>;

... used for both card and I/O line power...

> +       bus-width = <4>;
> +       no-1-8-v;

... hence no 1.8V I/O.

However, VIO of WL1837 is provided by W1.8V of regulator U55,
which is 1.8V?

> +       non-removable;
> +       cap-power-off-card;
> +       keep-power-in-suspend;
> +       max-frequency = <26000000>;
> +       status = "okay";
> +
> +       #address-cells = <1>;
> +       #size-cells = <0>;
> +       wlcore: wlcore@2 {
> +               compatible = "ti,wl1837";
> +               reg = <2>;
> +               interrupt-parent = <&gpio1>;
> +               interrupts = <25 IRQ_TYPE_EDGE_FALLING>;

I'm also a bit puzzled by the interrupt type.
On Cat 874, it's IRQ_TYPE_LEVEL_HIGH, cfr.
https://lore.kernel.org/linux-renesas-soc/1557997166-63351-2-git-send-email-biju.das@bp.renesas.com/

On Kingfisher, the IRQ signal is inverted by U104, so I'd expect
IRQ_TYPE_LEVEL_LOW instead of IRQ_TYPE_EDGE_FALLING?

Apart from the above two comments:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
Simon Horman May 31, 2019, 9:37 a.m. UTC | #6
Hi Spyridon,

please respond to Geert's review below and
if appropriate provide an incremental patch.

Thanks in advance,
Simon

On Tue, May 28, 2019 at 11:19:04AM +0200, Geert Uytterhoeven wrote:
> Hi Spyridon,
> 
> On Thu, Apr 11, 2019 at 2:42 PM Spyridon Papageorgiou
> <spapageorgiou@de.adit-jv.com> wrote:
> > This patch adds description of TI WL1837 and links interfaces
> > to communicate with the IC, namely the SDIO interface to WLAN.
> >
> > Signed-off-by: Spyridon Papageorgiou <spapageorgiou@de.adit-jv.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> > @@ -38,6 +38,18 @@
> >                 regulator-min-microvolt = <5000000>;
> >                 regulator-max-microvolt = <5000000>;
> >         };
> > +
> > +       wlan_en: regulator-wlan_en {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "wlan-en-regulator";
> > +
> > +               regulator-min-microvolt = <3300000>;
> > +               regulator-max-microvolt = <3300000>;
> 
> So this is a 3.3V regulator...
> 
> > +
> > +               gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
> > +               startup-delay-us = <70000>;
> > +               enable-active-high;
> > +       };
> >  };
> >
> >  &can0 {
> 
> > @@ -273,6 +298,30 @@
> >         status = "okay";
> >  };
> >
> > +&sdhi3 {
> > +       pinctrl-0 = <&sdhi3_pins>;
> > +       pinctrl-names = "default";
> > +
> > +       vmmc-supply = <&wlan_en>;
> > +       vqmmc-supply = <&wlan_en>;
> 
> ... used for both card and I/O line power...
> 
> > +       bus-width = <4>;
> > +       no-1-8-v;
> 
> ... hence no 1.8V I/O.
> 
> However, VIO of WL1837 is provided by W1.8V of regulator U55,
> which is 1.8V?
> 
> > +       non-removable;
> > +       cap-power-off-card;
> > +       keep-power-in-suspend;
> > +       max-frequency = <26000000>;
> > +       status = "okay";
> > +
> > +       #address-cells = <1>;
> > +       #size-cells = <0>;
> > +       wlcore: wlcore@2 {
> > +               compatible = "ti,wl1837";
> > +               reg = <2>;
> > +               interrupt-parent = <&gpio1>;
> > +               interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> 
> I'm also a bit puzzled by the interrupt type.
> On Cat 874, it's IRQ_TYPE_LEVEL_HIGH, cfr.
> https://lore.kernel.org/linux-renesas-soc/1557997166-63351-2-git-send-email-biju.das@bp.renesas.com/
> 
> On Kingfisher, the IRQ signal is inverted by U104, so I'd expect
> IRQ_TYPE_LEVEL_LOW instead of IRQ_TYPE_EDGE_FALLING?
> 
> Apart from the above two comments:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
>
Eugeniu Rosca May 31, 2019, 1:49 p.m. UTC | #7
Hi Simon,

On Fri, May 31, 2019 at 11:37:02AM +0200, Simon Horman wrote:
> Hi Spyridon,
> 
> please respond to Geert's review below and
> if appropriate provide an incremental patch.
> 
> Thanks in advance,
> Simon
> 

Spyridon is on vacation, so I will handle the open points.
Eugeniu Rosca May 31, 2019, 3:25 p.m. UTC | #8
Hi Geert,

We appreciate your review comments.

On Tue, May 28, 2019 at 11:19:04AM +0200, Geert Uytterhoeven wrote:
[..]
> > +       wlan_en: regulator-wlan_en {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "wlan-en-regulator";
> > +
> > +               regulator-min-microvolt = <3300000>;
> > +               regulator-max-microvolt = <3300000>;
> 
> So this is a 3.3V regulator...
> 
> > +
> > +               gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
> > +               startup-delay-us = <70000>;
> > +               enable-active-high;
> > +       };
> >  };
> >
> >  &can0 {
> 
> > @@ -273,6 +298,30 @@
> >         status = "okay";
> >  };
> >
> > +&sdhi3 {
> > +       pinctrl-0 = <&sdhi3_pins>;
> > +       pinctrl-names = "default";
> > +
> > +       vmmc-supply = <&wlan_en>;
> > +       vqmmc-supply = <&wlan_en>;
> 
> ... used for both card and I/O line power...
> 
> > +       bus-width = <4>;
> > +       no-1-8-v;
> 
> ... hence no 1.8V I/O.
> 
> However, VIO of WL1837 is provided by W1.8V of regulator U55,
> which is 1.8V?

Looking at the KF-M06 schematics, it seems like the SDIO-relevant lines
of WL1837 (U52) are interfaced with the SoC via TXS0108EPWR (U57) which
is there to level-translate from 3.3v (SoC) to 1.8v (WL1837). So,
from SoC perspective, it looks like the lines are 3.3v-powered.

FTR, the test results are independent on the 'no-1-8-v' property.

> > +       non-removable;
> > +       cap-power-off-card;
> > +       keep-power-in-suspend;
> > +       max-frequency = <26000000>;
> > +       status = "okay";
> > +
> > +       #address-cells = <1>;
> > +       #size-cells = <0>;
> > +       wlcore: wlcore@2 {
> > +               compatible = "ti,wl1837";
> > +               reg = <2>;
> > +               interrupt-parent = <&gpio1>;
> > +               interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> 
> I'm also a bit puzzled by the interrupt type.
> On Cat 874, it's IRQ_TYPE_LEVEL_HIGH, cfr.
> https://lore.kernel.org/linux-renesas-soc/1557997166-63351-2-git-send-email-biju.das@bp.renesas.com/
> 
> On Kingfisher, the IRQ signal is inverted by U104, so I'd expect
> IRQ_TYPE_LEVEL_LOW instead of IRQ_TYPE_EDGE_FALLING?

That's an insightful comment, if it simply arose from code review.
I guess we mistakenly relied on [1] during our testing on linux/master.
So, we definitely have to re-spin the patch to make it independent
on [1]. The problem is that by dropping [1] and switching from
IRQ_TYPE_EDGE_FALLING to IRQ_TYPE_LEVEL_LOW, the wifi testing
(particularly 'iwlist wlan0 scan') doesn't pass. We have to give
another thought how to best tackle it.

[1] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch

Thank you.

> 
> Apart from the above two comments:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Geert Uytterhoeven June 3, 2019, 7:26 a.m. UTC | #9
Hi Eugeniu,

On Fri, May 31, 2019 at 5:25 PM Eugeniu Rosca <erosca@de.adit-jv.com> wrote:
> On Tue, May 28, 2019 at 11:19:04AM +0200, Geert Uytterhoeven wrote:
> [..]
> > > +       wlan_en: regulator-wlan_en {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "wlan-en-regulator";
> > > +
> > > +               regulator-min-microvolt = <3300000>;
> > > +               regulator-max-microvolt = <3300000>;
> >
> > So this is a 3.3V regulator...
> >
> > > +
> > > +               gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
> > > +               startup-delay-us = <70000>;
> > > +               enable-active-high;
> > > +       };
> > >  };
> > >
> > >  &can0 {
> >
> > > @@ -273,6 +298,30 @@
> > >         status = "okay";
> > >  };
> > >
> > > +&sdhi3 {
> > > +       pinctrl-0 = <&sdhi3_pins>;
> > > +       pinctrl-names = "default";
> > > +
> > > +       vmmc-supply = <&wlan_en>;
> > > +       vqmmc-supply = <&wlan_en>;
> >
> > ... used for both card and I/O line power...
> >
> > > +       bus-width = <4>;
> > > +       no-1-8-v;
> >
> > ... hence no 1.8V I/O.
> >
> > However, VIO of WL1837 is provided by W1.8V of regulator U55,
> > which is 1.8V?
>
> Looking at the KF-M06 schematics, it seems like the SDIO-relevant lines
> of WL1837 (U52) are interfaced with the SoC via TXS0108EPWR (U57) which
> is there to level-translate from 3.3v (SoC) to 1.8v (WL1837). So,
> from SoC perspective, it looks like the lines are 3.3v-powered.
>
> FTR, the test results are independent on the 'no-1-8-v' property.

Sorry for not noticing the level translator.
So indeed, the WL1837 side is always at 1.8V.
But I believe the SoC side can be either 1.8V or 3.3V, as the level-translator
can handle both, which is confirmed by your testing.

> > > +       non-removable;
> > > +       cap-power-off-card;
> > > +       keep-power-in-suspend;
> > > +       max-frequency = <26000000>;
> > > +       status = "okay";
> > > +
> > > +       #address-cells = <1>;
> > > +       #size-cells = <0>;
> > > +       wlcore: wlcore@2 {
> > > +               compatible = "ti,wl1837";
> > > +               reg = <2>;
> > > +               interrupt-parent = <&gpio1>;
> > > +               interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> >
> > I'm also a bit puzzled by the interrupt type.
> > On Cat 874, it's IRQ_TYPE_LEVEL_HIGH, cfr.
> > https://lore.kernel.org/linux-renesas-soc/1557997166-63351-2-git-send-email-biju.das@bp.renesas.com/
> >
> > On Kingfisher, the IRQ signal is inverted by U104, so I'd expect
> > IRQ_TYPE_LEVEL_LOW instead of IRQ_TYPE_EDGE_FALLING?
>
> That's an insightful comment, if it simply arose from code review.
> I guess we mistakenly relied on [1] during our testing on linux/master.
> So, we definitely have to re-spin the patch to make it independent
> on [1]. The problem is that by dropping [1] and switching from
> IRQ_TYPE_EDGE_FALLING to IRQ_TYPE_LEVEL_LOW, the wifi testing
> (particularly 'iwlist wlan0 scan') doesn't pass. We have to give
> another thought how to best tackle it.
>
> [1] https://github.com/CogentEmbedded/meta-rcar/blob/289fbd4f8354/meta-rcar-gen3-adas/recipes-kernel/linux/linux-renesas/0024-wl18xx-do-not-invert-IRQ-on-WLxxxx-side.patch

Perhaps some configuration in the WL driver may be involved?
It looks like all other DTSes use IRQ_TYPE_LEVEL_HIGH, except for
HiKey 960/970, which use IRQ_TYPE_EDGE_RISING.

Gr{oetje,eeting}s,

                        Geert
Simon Horman June 3, 2019, 11:01 a.m. UTC | #10
On Fri, May 31, 2019 at 03:49:55PM +0200, Eugeniu Rosca wrote:
> Hi Simon,
> 
> On Fri, May 31, 2019 at 11:37:02AM +0200, Simon Horman wrote:
> > Hi Spyridon,
> > 
> > please respond to Geert's review below and
> > if appropriate provide an incremental patch.
> > 
> > Thanks in advance,
> > Simon
> > 
> 
> Spyridon is on vacation, so I will handle the open points.

Thanks Eugeniu,

much appreciated.

Patch
diff mbox series

diff --git a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
index 7a09576..27851a7 100644
--- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
@@ -38,6 +38,18 @@ 
 		regulator-min-microvolt = <5000000>;
 		regulator-max-microvolt = <5000000>;
 	};
+
+	wlan_en: regulator-wlan_en {
+		compatible = "regulator-fixed";
+		regulator-name = "wlan-en-regulator";
+
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&gpio_exp_74 4 GPIO_ACTIVE_HIGH>;
+		startup-delay-us = <70000>;
+		enable-active-high;
+	};
 };
 
 &can0 {
@@ -88,6 +100,13 @@ 
 			line-name = "Audio_Out_OFF";
 		};
 
+		sd-wifi-mux {
+			gpio-hog;
+			gpios = <5 GPIO_ACTIVE_HIGH>;
+			output-low;	/* Connect WL1837 */
+			line-name = "SD WiFi mux";
+		};
+
 		hub_pwen {
 			gpio-hog;
 			gpios = <6 GPIO_ACTIVE_HIGH>;
@@ -254,6 +273,12 @@ 
 		function = "scif1";
 	};
 
+	sdhi3_pins: sdhi3 {
+		groups = "sdhi3_data4", "sdhi3_ctrl";
+		function = "sdhi3";
+		power-source = <3300>;
+	};
+
 	usb0_pins: usb0 {
 		groups = "usb0";
 		function = "usb0";
@@ -273,6 +298,30 @@ 
 	status = "okay";
 };
 
+&sdhi3 {
+	pinctrl-0 = <&sdhi3_pins>;
+	pinctrl-names = "default";
+
+	vmmc-supply = <&wlan_en>;
+	vqmmc-supply = <&wlan_en>;
+	bus-width = <4>;
+	no-1-8-v;
+	non-removable;
+	cap-power-off-card;
+	keep-power-in-suspend;
+	max-frequency = <26000000>;
+	status = "okay";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	wlcore: wlcore@2 {
+		compatible = "ti,wl1837";
+		reg = <2>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+	};
+};
+
 &usb2_phy0 {
 	pinctrl-0 = <&usb0_pins>;
 	pinctrl-names = "default";