Re: [PATCH 2/2] firmware: exynos-acpm: allow use during system shutdown

[Tudor Ambarus]

On 3/21/25 4:40 PM, André Draszik wrote:
> We need to access the PMIC during late system shutdown and at that time
> we are not allowed to sleep anymore.
> 
> To make this case work, detect this condition and use busy waiting via
> udelay() instead of usleep_range() in that situation.
> 
> The code isn't switched over to udelay() unconditionally so as to not
> waste resources during normal operation. acpm_may_sleep() was heavily
> inspired by the I2C subsystem's i2c_in_atomic_xfer_mode().
> 
> Signed-off-by: André Draszik <andre.draszik@xxxxxxxxxx>

Reviewed-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>
> 
> ---
> udelay(10) causes a checkpatch warning (it suggests to use
> usleep_range() instead for usec >= 10), but that's exactly what we can
> not do.
> Reducing the udelay to be smaller will generally cause the loop to be
> iterated more than once, which I wanted to avoid.
> I could reflow the code to hide the actual value from checkpatch, e.g.
> with the help of a local variable if that is preferred to ignoring the
> checkpatch warning.
> ---
>  drivers/firmware/samsung/exynos-acpm.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/samsung/exynos-acpm.c b/drivers/firmware/samsung/exynos-acpm.c
> index d7ed6b77a957af5db5beba7deecce13ac7b30fd2..33cde6e88e2c0773fdd36c80927c77d3bcb44135 100644
> --- a/drivers/firmware/samsung/exynos-acpm.c
> +++ b/drivers/firmware/samsung/exynos-acpm.c
> @@ -15,6 +15,8 @@
>  #include <linux/firmware/samsung/exynos-acpm-protocol.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
> +#include <linux/irqflags.h>
> +#include <linux/kernel.h>
>  #include <linux/mailbox/exynos-message.h>
>  #include <linux/mailbox_client.h>
>  #include <linux/module.h>
> @@ -24,6 +26,7 @@
>  #include <linux/of_address.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/preempt.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
> @@ -272,6 +275,17 @@ static int acpm_get_rx(struct acpm_chan *achan, const struct acpm_xfer *xfer)
>  	return 0;
>  }
>  
> +/*
> + * When ACPM transfers happen very late, e.g. to access a PMIC when powering
> + * down, we can not sleep. We do want to sleep in the normal case, though, to
> + * avoid wasting CPU cycles!
> + */
> +static bool acpm_may_sleep(void)
> +{
> +	return system_state <= SYSTEM_RUNNING ||
> +		(IS_ENABLED(CONFIG_PREEMPT_COUNT) ? preemptible() : !irqs_disabled());
> +}
> +
>  /**
>   * acpm_dequeue_by_polling() - RX dequeue by polling.
>   * @achan:	ACPM channel info.
> @@ -299,7 +313,10 @@ static int acpm_dequeue_by_polling(struct acpm_chan *achan,
>  			return 0;
>  
>  		/* Determined experimentally. */
> -		usleep_range(20, 30);
> +		if (!acpm_may_sleep())
> +			udelay(10);
> +		else
> +			usleep_range(20, 30);
>  	} while (!ktime_after(ktime_get(), timeout));
>  
>  	dev_err(dev, "Timeout! ch:%u s:%u bitmap:%lx.\n",
>