Re: [PATCH v7 4/9] iio: ssp_sensors: factor out mcu enable/disable helper(s)

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 26 Apr 2026 14:47:05 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:

> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> 
> Re-factor the enable and disable MCU logic into small helper functions
> this simplify transfer flow to follow.
> 
> No functional change intended.
> 
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
>  drivers/iio/common/ssp_sensors/ssp_dev.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/common/ssp_sensors/ssp_dev.c b/drivers/iio/common/ssp_sensors/ssp_dev.c
> index 51730dae5871..9b8642193176 100644
> --- a/drivers/iio/common/ssp_sensors/ssp_dev.c
> +++ b/drivers/iio/common/ssp_sensors/ssp_dev.c
> @@ -89,7 +89,7 @@ static void ssp_sync_available_sensors(struct ssp_data *data)
>  			"SSP_MSG2SSP_AP_MCU_SET_DUMPMODE failed\n");
>  }
>  
> -static void ssp_enable_mcu(struct ssp_data *data, bool enable)
> +static void ssp_set_mcu(struct ssp_data *data, bool enable)
>  {
>  	dev_info(&data->spi->dev, "current shutdown = %d, old = %d\n", enable,
>  		 data->shut_down);
> @@ -108,6 +108,16 @@ static void ssp_enable_mcu(struct ssp_data *data, bool enable)
>  	}
>  }
>  
> +static inline void ssp_enable_mcu(struct ssp_data *data)
> +{
> +	ssp_set_mcu(data, true);
> +}
> +
> +static inline void ssp_disable_mcu(struct ssp_data *data)
> +{
> +	ssp_set_mcu(data, false);
> +}
There is no significant shared code in ssp_set_mcu() for true vs false. Also never add
inlines in c files without very strong perf data to back them up.
So something like:

static void ssp_enable_mcu(struct ssp_data *data)
{
	dev_info(&data->spi->dev, "current shutdown = %d, old = %d\n", enable,
		 data->shut_down);

	if (!data->shutdown) {
		dev_warn(&data->spi->dev, "current shutdown = %d, old = %d\n",
			 enable, data->shut_down);

// I think this is debug only. So I'd drop the print as something that shouldn't
// be in code after development is done.

		return;
	}
	data->shut_down = false;
	enable_irq(data->spi->irq);
	enable_irq_wake(data->spi->irq);
}

and similar for disable_mcu()

FWIW I don't like having a dev_info() print there. So demote that to dev_dbg().

> +
>  /*
>   * This function is the first one which communicates with the mcu so it is
>   * possible that the first attempt will fail
> @@ -146,10 +156,10 @@ static int ssp_check_fwbl(struct ssp_data *data)
>  
>  static void ssp_reset_mcu(struct ssp_data *data)
>  {
> -	ssp_enable_mcu(data, false);
> +	ssp_disable_mcu(data);
>  	ssp_clean_pending_list(data);
>  	ssp_toggle_mcu_reset_gpio(data);
> -	ssp_enable_mcu(data, true);
> +	ssp_enable_mcu(data);
>  }
>  
>  static void ssp_wdt_work_func(struct work_struct *work)
> @@ -584,7 +594,7 @@ static void ssp_remove(struct spi_device *spi)
>  		dev_err(&data->spi->dev,
>  			"SSP_MSG2SSP_AP_STATUS_SHUTDOWN failed\n");
>  
> -	ssp_enable_mcu(data, false);
> +	ssp_disable_mcu(data);
>  	ssp_disable_wdt_timer(data);
>  
>  	ssp_clean_pending_list(data);