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;
Feedback-ID: ifd214418:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-238-g746678b8b6-fm-20230329.001-g746678b8
Mime-Version: 1.0
Message-Id: <3cd2ef19-f902-4493-ad90-00c410aca176@app.fastmail.com>
In-Reply-To: <20230329224736.qzidpkzkdth2lvuq@mercury.elektranox.org>
References: <0dcc1aac-9a6a-4d17-be68-a895cb6120da@app.fastmail.com>
 <CACRpkdZSJCZZEkD0V_wint+a1XKsbCQFuArFvPdFP8RSDsGLMQ@mail.gmail.com>
 <20230329224736.qzidpkzkdth2lvuq@mercury.elektranox.org>
Date:   Thu, 30 Mar 2023 18:00:41 +1000
From:   Alistair <alistair@alistair23.me>
To:     "Sebastian Reichel" <sebastian.reichel@collabora.com>,
        "Linus Walleij" <linus.walleij@linaro.org>
Cc:     ye.xingchen@zte.com.cn, pali@kernel.org, sravanhome@gmail.com,
        linux-pm@vger.kernel.org,
        "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: power_supply_show_property Kernel Oops
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, 30 Mar 2023, at 8:47 AM, Sebastian Reichel wrote:
> Hi,
>=20
> On Wed, Mar 29, 2023 at 04:43:19PM +0200, Linus Walleij wrote:
> > On Wed, Mar 29, 2023 at 1:16=E2=80=AFPM Alistair <alistair@alistair2=
3.me> wrote:
> >=20
> > > [    2.466136]  string from vsnprintf+0x158/0x424
> > > [    2.470603]  vsnprintf from vscnprintf+0x10/0x24
> > > [    2.475241]  vscnprintf from sysfs_emit+0x50/0xac
> > > [    2.479975]  sysfs_emit from power_supply_show_property+0x1d0/0=
x26c
> > > [    2.486269]  power_supply_show_property from add_prop_uevent+0x=
30/0x8c
> > > [    2.492815]  add_prop_uevent from power_supply_uevent+0xb4/0xe4
> > > [    2.498753]  power_supply_uevent from dev_uevent+0xc4/0x21c
> > > [    2.504352]  dev_uevent from kobject_uevent_env+0x1cc/0x510
> > > [    2.509953]  kobject_uevent_env from power_supply_changed_work+=
0x7c/0xb4
> > > [    2.516675]  power_supply_changed_work from process_one_work+0x=
1e8/0x3e8
> > > [    2.523396]  process_one_work from worker_thread+0x2c/0x504
> > > [    2.528986]  worker_thread from kthread+0xcc/0xec
> > > [    2.533716]  kthread from ret_from_fork+0x14/0x24
> > > [    2.538443] Exception stack(0xf0dadfb0 to 0xf0dadff8)
> >=20
> > This looks like running a worker before something this worker is
> > accessing has been set up.
> >=20
> > > As it's on a consumer device I don't have a way to connect a debug=
ger. So I'm
> > > a little stuck on what the problem is. The only related change I s=
ee between
> > > 6.2 and 6.3-rc4 is commit a441f3b90a340e5c94df36c33fb7000193ee0aa7
> > > "power: supply: use sysfs_emit() instead of sprintf() for sysfs sh=
ow()", but
> > > that doesn't look like it would cause this oops.
> >=20
> > Did you try reverting it?
>=20
> Does not look like a race condition with a worker to me. The patch
> adds a couple of properties to the power-supply in an incorrect way.

Thanks for that.

It appears that these two drivers cause the problem:

https://github.com/alistair23/linux/blob/rM2-mainline/drivers/power/supp=
ly/max77818-charger.c
https://github.com/alistair23/linux/blob/rM2-mainline/drivers/regulator/=
max77818-regulator.c=20

as removing those two fixes the oops.

> I did not look deeply, but it's at least missing an update to
> power_supply_attrs. I guess you were 'lucky' that it did not crash
> with v6.2.

Do you mind pointing to an example that does this correctly? I don't see
anything obviously wrong compared to the in-tree max* drivers.

Anything to help point me in the right direction would be greatly
appreciated :)

>=20
> None of the extra properties are acceptable upstream btw.:
>=20
> POWER_SUPPLY_PROP_CURRENT_MAX2:
>     The driver seems to use CURRENT_MAX2 for input current;
>     POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT should be used for that
>=20
> POWER_SUPPLY_PROP_CHARGER_MODE:
>     OTG should be handled via a regulator
>=20
> POWER_SUPPLY_PROP_STATUS_EX:
>     Use extcon for connectors

Thanks! I figured as much. These are just taken from the vendor tree, I =
need to
clean everything up for upstreaming. I just wanted to have a working bas=
e point
to start.

>=20
> Greetings,
>=20
> -- Sebastian
>=20
>=20
> *Attachments:*
>  =E2=80=A2 signature.asc