Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.21\))
Subject: Re: [PATCH 0/9] MIPS: CI20: Add WiFi / Bluetooth support
From:   "H. Nikolaus Schaller" <hns@goldelico.com>
In-Reply-To: <AA802E24-A97B-42B6-90A4-5ECB1E4D9294@goldelico.com>
Date:   Fri, 16 Jun 2023 22:21:06 +0200
Cc:     list@opendingux.net, Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        Conor Dooley <conor+dt@kernel.org>, linux-mips@vger.kernel.org,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Content-Transfer-Encoding: quoted-printable
Message-Id: <5EF6B0D2-2B84-4C98-B799-88062E035EC1@goldelico.com>
References: <20230615084006.79194526F801@goldelico.com>
 <C1924536-FA97-4E75-9D22-99E5AF24EE5B@goldelico.com>
 <A8EBCBA4-D9BA-4C2F-9C91-38128D06EDBA@goldelico.com>
 <AA802E24-A97B-42B6-90A4-5ECB1E4D9294@goldelico.com>
To:     Paul Cercueil <paul@crapouillou.net>
Precedence: bulk

Hi Paul,

>=20
>>>> Since this patch will actually make the various ACT8600 regulators =
work at their specified voltage, maybe the voltage on one of the updated =
regulators is wrong?
>>>>=20
>>>> Maybe change the regulators one by one back to their old name, =
until you find the one that is problematic? That may give us more info.
>>>=20
>>> That is what I also have thought about but have not yet done.
>>> Will try as soon as I find a time slot.
>>=20
>> I have reverted the whole patch (had only a conflict in wifi_io / =
LDO6) and now I can boot.
>>=20
>> But do not see a WiFi or Bluetooth interface.
>>=20
>> So it looks as if the CI20 variants are indeed different. Which would =
also explain why we
>> originally came up with two different solutions to add WiFi.
>>=20
>> Next I will try to bisect the individual changes...
>=20
> It is this and not the regulator names:
>=20
> diff --git a/arch/mips/boot/dts/ingenic/ci20.dts =
b/arch/mips/boot/dts/ingenic/ci20.dts
> index e2221d44e4269..391be48e6427a 100644
> --- a/arch/mips/boot/dts/ingenic/ci20.dts
> +++ b/arch/mips/boot/dts/ingenic/ci20.dts
> @@ -295,7 +295,6 @@ &i2c0 {
>        act8600: act8600@5a {
>                compatible =3D "active-semi,act8600";
>                reg =3D <0x5a>;
> -               status =3D "okay";
>=20
>                regulators {
>                        vddcore: SUDCDC1 {
>=20
>=20
> Now I wonder how it works without status =3D "okay" for you but not =
for me.

I have not found a reason for this and it was difficult to repeat. =
Potentially a bisect
with failed boot and wrong setup of some voltages may have damaged the =
file system on my
SD card (see below). At least I had to fsck -f after running the bisect, =
but I did not
do it for every bisect step. Sometimes bisecting is difficult if =
hardware effects are
involved...

So I started with the series + a revert of the offending patch, added =
some more logging
to the kernel and printk() in the driver.

Results:

- driver is always loaded, so the status =3D "okay" was spurious and not =
the problem.

- Adding/Removing the regulator names also does not make a difference.

- But renaming the DT nodes (e.g. SUDCDC1 -> DCDC1) (with or without =
regulator_name) makes
boot hang with strange errors which indicate that the processor power =
supply is not stable.
Once a while it did even automatically reboot. In most cases there are =
some EXT4 errors
afterwards.

Example:

[    3.003096] EXT4-fs error (device mmcblk0p1): =
ext4_mb_generate_buddy:1100: group 81, block bitmap and bg descriptor =
inconsistent: 30994 vs 31229 free clusters
/sbin/init: error while loading shared libraries: /lib/mipsel-li
[    3.291901] Kernel panic - not syncing: Attempted to kill init! =
exitcode=3D0x00007f00
[    3.305122] Rebooting in 10 seconds..

I have not found a reason but it appears that if the DT nodes do match =
the
struct regulator_desc list, it is different from having them not match.

At least the result of regulator_of_get_init_data() is NULL if there is =
no
match and then some other code path is followed in regulator_register().

So at the moment I think that matching DT node names with the =
act8600_regulators[] list
changes something in the chip initialization which has influence =
depending on hardware
variation. Maybe your board is simply more robust than mine to that.

Deeper analysis will reveal the issue and indicate a solution...

BR,
Nikolaus