Re: [PATCH v2 2/2] MIPS: SGI-IP30: shut down cleanly on a power button press

[Imre Kaloz <kaloz@kernel.org>]

>> +	local_irq_disable();
>> +	if (atomic_xchg(&ip30_power_off_claimed, 1))
>> +		for (;;)
>> +			cpu_relax();
>
> As reported by Sashiko on v1 [1], can another CPU still race on the
> indirect RTC index/data registers and bank selection while
> ds1685_rtc_poweroff() is running?

On dual-CPU Octanes the second CPU can still move the index or the
bank. v3 will stop the other CPU with smp_send_stop() before touching
HEART or the RTC.

>>  	ds1685_rtc_poweroff(to_platform_device(dev));
>
> Doesn't ds1685_rtc_poweroff() sleep in msleep(20) before cutting power,
> violating the atomic context established by local_irq_disable() and
> nbcon_cpu_emergency_enter()?

Not with the dependency named in the cover letter. Commit 21a829af60a7
("rtc: ds1685: do not sleep in the power-off handler"), in v7.4-rc1,
replaces that msleep() with mdelay().

>> +#ifdef CONFIG_SGI_IP30
>> +	.plat_prepare_poweroff = ip30_prepare_poweroff,
>> +#endif
>
> What happens on a power button press when CONFIG_SGI_IP30=y and
> CONFIG_RTC_DRV_DS1685_FAMILY=m?

The button stops cutting the power, as the built-in
ip30_machine_power_off() cannot reach the module. v3 will set the hook
only when rtc-ds1685 is built in, so a modular driver will power off
on a press as it did before this series, and the second-press branch
will return.


Thanks,
Imre