Message-ID: <4485f880f7b41bdb833d1153682530cd4bc3ed94.camel@crapouillou.net>
Date:   Wed, 15 Feb 2023 21:19:24 +0000
From:   Paul Cercueil <paul@...pouillou.net>
To:     "H. Nikolaus Schaller" <hns@...delico.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Arnd Bergmann <arnd@...db.de>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Rafał Miłecki <rafal@...ecki.pl>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Riccardo Mottola <riccardo.mottola@...ero.it>,
        Paul Boddie <paul@...die.org.uk>,
        linux-mips <linux-mips@...r.kernel.org>,
        OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS 
        <devicetree@...r.kernel.org>,
        linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Discussions about the Letux Kernel 
        <letux-kernel@...nphoenux.org>
Subject: Re: [RFC 0/3] Make WLAN and Bluetooth basically work for CI20

Hi Nikolaus,

Le mercredi 15 février 2023 à 21:32 +0100, H. Nikolaus Schaller a
écrit :
> Hi Paul,
> I wasn't aware of your work and it could have saved me a lot of time
> to
> experiment and try to make my patch set work.
> 
> But we are still lucky, as we can merge ideas and find the really
> best solution.
> 
> Anyways you can take my patch 1/3 and 3/3 and we have only to discuss
> details of 2/3.
> 
> Interesting is your setup of several fixed regulators using
> regulator-settling-time-us
> to take care of power up sequencing. What I don't understand is the
> fixedregulator-5
> which has no controlling gpio but is always on. What is it needed
> for? Maybe just to
> be used to silence v(q)mmc-supply warnings etc.?

It's wired to the various inputs of the ACT8600 chip. The driver will
complain if they are not connected to anything. And while we're at it,
it will also silence the v(q)mmc-supply warnings, yes.

> But you do not really switch or enable/disable power of the SDIO or
> bluetooth UART
> interface?

The badly named "bt_power" regulator powers on the chip, while the
"wifi_io" regulator controls the voltage on the VDDIO pins. So these
are enabled/disabled when needed.

> Is it necessary to add a compatible of "ingenic,iw8103-fmac"? If it
> works with the
> standard "brcm,bcm4329-fmac" driver (note that I have "brcm,bcm4330-
> fmac" because that
> is how the module identifies itself) there seems to be no need for
> another compatible
> and a special bindings documentation.

The chip has a different name so we can't guarantee that it works
exactly like the BCM4330. So that was my reason behind using a new
compatible string with a fallback.

However I sent a patch to add the new compatible string to the
documentation and it got refused and I was told to just use the
brcm,bcm4330-fmac, so I'll just do that.

> For bluetooth you could add brcm,bt-pcm-int-params = [01 02 00 01
> 01]; so that we
> can eventually add some PCM stream to the sound setup.

Sure, does that work though?

One (unrelated) note about Bluetooth, I didn't get it to work properly;
it works enough to detect my keyboard and allow me to pair with it
(typing the password on the BT keyboard) but it will never connect
properly after that.

> Finally you have made node labels more consistent by calling them
> wifi* while I made
> them wlan0*... Well, just a matter of taste.

Actually the node names should use as much as possible the generic
names specified in the devicetree specification
(https://www.devicetree.org/specifications/) and "wifi" is one of
those.

> Then I tried to take your tree, add my defconfig (because it seems as
> if you have not
> updated configs), but I could only see 
> 
> [    0.929072] Bluetooth: hci0: BCM: firmware Patch file not found,
> tried:
> [    0.935704] Bluetooth: hci0: BCM: 'brcm/BCM4330B1.img,ci20.hcd'
> [    0.941683] Bluetooth: hci0: BCM: 'brcm/BCM4330B1.hcd'
> [    0.946827] Bluetooth: hci0: BCM: 'brcm/BCM.img,ci20.hcd'
> [    0.952278] Bluetooth: hci0: BCM: 'brcm/BCM.hcd'
> 
> and then the kernel is stuck.
> 
> So what is your defconfig that I can test your build?

I test with the OpenDingux userspace so my defconfig is probably very
different from yours. What are you using?

About the crash: make sure you have the *very* latest WiFi firmware
from
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/

I do remember that one particular version of the firmware will crash
the kernel. You also don't need the .hcd firmware for Bluetooth to
(somewhat) work.

> BR and thanks,
> Nikolaus

Cheers,
-Paul

> > Am 15.02.2023 um 18:55 schrieb Paul Cercueil
> > <paul@...pouillou.net>:
> > 
> > Hi Nikolaus,
> > 
> > Le mercredi 15 février 2023 à 18:31 +0100, H. Nikolaus Schaller a
> > écrit :
> > > RFC V1 2023-02-15 18:31:58:
> > > With the applied patch
> > > 
> > > rtc: jz4740: Register clock provider for the CLK32K pin
> > > 
> > > by Paul Cercueil <paul@...pouillou.net> it is now possible to
> > > make
> > > the
> > > CI20 WiFi and Bluetooth setup do something reasonable.
> > > 
> > > This series adds #clock-cells to the jz4780 and fixes the device
> > > tree
> > > for
> > > the CI20. It contains two ugly workarounds since BL_WAKE and
> > > WL_WAKE
> > > can not
> > > be controlled as shutdown-gpios or device-wakeup-gpios by the
> > > drivers.
> > > We also update the power setup and sequencing and add a clock
> > > chain
> > > for
> > > the 32 kHz clock.
> > > Finally, we enable some required CONFIGs for the CI20.
> > 
> > I do have my own patchset that I wanted to send in time for 6.3-
> > rc1,
> > but the RTC patchset was merged very late so I was waiting for -rc1
> > to
> > be out before sending it to Thomas.
> > 
> > I think my DT changes are better as I have no workarounds. I need
> > to
> > clean up the commits (and have proper commit messages) but you can
> > find
> > my current work there:
> > https://github.com/OpenDingux/linux/commits/for-upstream-ci20
> > 
> > Cheers,
> > -Paul
> > 
> > > 
> > > Tested on CI20 with v6.2-rc6.
> > > 
> > > H. Nikolaus Schaller (3):
> > >   MIPS: DTS: jz4780: add #clock-cells to rtc_dev
> > >   MIPS: DTS: CI20: fixes for WiFi/Bluetooth
> > >   MIPS: configs: ci20: enable drivers we need for WiFi/Bluetooth
> > > 
> > >  arch/mips/boot/dts/ingenic/ci20.dts    | 77
> > > ++++++++++++++++++++++--
> > > --
> > >  arch/mips/boot/dts/ingenic/jz4780.dtsi |  2 +
> > >  arch/mips/configs/ci20_defconfig       | 18 +++++-
> > >  3 files changed, 84 insertions(+), 13 deletions(-)
> > > 
> > 
> 

Powered by blists - more mailing lists