[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <196f84ea-6567-4fe3-9bee-743bb289223e@app.fastmail.com>
Date: Fri, 09 May 2025 18:13:09 +0200
From: "Sven Peter" <sven@...npeter.dev>
To: "Alyssa Rosenzweig" <alyssa@...enzweig.io>
Cc: "Janne Grunau" <j@...nau.net>, "Neal Gompa" <neal@...pa.dev>,
"Hector Martin" <marcan@...can.st>,
"Linus Walleij" <linus.walleij@...aro.org>,
"Bartosz Golaszewski" <brgl@...ev.pl>, "Rob Herring" <robh@...nel.org>,
"Krzysztof Kozlowski" <krzk+dt@...nel.org>,
"Conor Dooley" <conor+dt@...nel.org>, "Sebastian Reichel" <sre@...nel.org>,
"Lee Jones" <lee@...nel.org>, "Marc Zyngier" <maz@...nel.org>,
"Russell King" <rmk+kernel@...linux.org.uk>, asahi@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org
Subject: Re: [PATCH v4 6/9] power: reset: macsmc-reboot: Add driver for rebooting via
Apple SMC
thanks for the review!
On Fri, May 9, 2025, at 15:05, Alyssa Rosenzweig wrote:
>> + for (i = 0; i < ARRAY_SIZE(ac_power_mode_map); i++)
>> + if (mode == ac_power_mode_map[i])
>> + len += scnprintf(buf+len, PAGE_SIZE-len,
>> + "[%s] ", ac_power_modes[i]);
>> + else
>> + len += scnprintf(buf+len, PAGE_SIZE-len,
>> + "%s ", ac_power_modes[i]);
>
> Nit: { braces } at least on the for loop...
Will fix that.
>
> (And might be more concise as
>
> for (i = 0; i < ARRAY_SIZE(ac_power_mode_map); i++) {
> bool match = (mode == ac_power_mode_map[i]);
> len += scnprintf(buf+len, PAGE_SIZE-len,
> match ? "[%s] " : "%s ", ac_power_modes[i]);
> }
>
> though IDK how people feel about it.)
imho it's less readable that way but I don't have a strong opinion
either way.
>
>> + mdelay(100);
>> + WARN_ON(1);
>
> ...What?
This is done in a few drivers in drivers/power/reboot. If we haven't
rebooted after 100ms something's wrong with SMC. I'll add a comment.
drivers/power/reset/tps65086-restart.c: WARN_ON(1);
drivers/power/reset/atc260x-poweroff.c: WARN_ONCE(1, "Unable to power off system\n");
drivers/power/reset/mt6323-poweroff.c: WARN_ONCE(1, "Unable to power off system\n");
drivers/power/reset/gpio-restart.c: WARN_ON(1);
drivers/power/reset/regulator-poweroff.c: WARN_ON(1);
>
>> + if (ret)
>> + return dev_err_probe(&pdev->dev, ret,
>> + "Failed to register power-off handler\n");
>> +
>> +
>> + ret = devm_register_sys_off_handler(&pdev->dev, SYS_OFF_MODE_RESTART_PREPARE,
>> + SYS_OFF_PRIO_HIGH, macsmc_prepare_atomic, reboot);
>
> Nit: squash double blank-line
will do.
thanks,
Sven
Powered by blists - more mailing lists