Re: [PATCH] arm64: dts: ulcb-kf: Add support for TI WL1837

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Eugeniu Rosca <erosca@de.adit-jv.com>
Cc: Spyridon Papageorgiou <spapageorgiou@de.adit-jv.com>,
	Simon Horman <horms@verge.net.au>,
	Magnus Damm <magnus.damm@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Tobias Franzen <tfranzen@de.adit-jv.com>,
	Biju Das <biju.das@bp.renesas.com>,
	Eugeniu Rosca <roscaeugeniu@gmail.com>
Subject: Re: [PATCH] arm64: dts: ulcb-kf: Add support for TI WL1837
Date: Mon, 3 Jun 2019 09:26:57 +0200	[thread overview]
Message-ID: <CAMuHMdXbx=-zrY-6iJeJLH1XS38GSr0m_rUMmA7wHjwi_a=cWw@mail.gmail.com> (raw)
In-Reply-To: <20190531152522.GB28755@vmlxhi-102.adit-jv.com>

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

-- 
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