Re: [PATCH v3 RESEND 1/2] scsi: mpt3sas: add IO Unit Page 7 config accessor

[Damien Le Moal <dlemoal@kernel.org>]

On 2026/06/10 0:44, Louis Sautier wrote:
> Add mpt3sas_config_get_iounit_pg7(), mirroring the existing iounit
> page accessors. Used by the hwmon driver added in the following patch
> to read the IOC and board temperatures.
> 
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Louis Sautier <sautier.louis@gmail.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_base.h   |  2 ++
>  drivers/scsi/mpt3sas/mpt3sas_config.c | 36 +++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
> index d4597d058705..c655742d0dde 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.h
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
> @@ -1904,6 +1904,8 @@ int mpt3sas_config_get_iounit_pg3(struct MPT3SAS_ADAPTER *ioc,
>  	Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage3_t *config_page, u16 sz);
>  int mpt3sas_config_set_iounit_pg1(struct MPT3SAS_ADAPTER *ioc, Mpi2ConfigReply_t
>  	*mpi_reply, Mpi2IOUnitPage1_t *config_page);
> +int mpt3sas_config_get_iounit_pg7(struct MPT3SAS_ADAPTER *ioc,
> +	Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage7_t *config_page);
>  int mpt3sas_config_get_iounit_pg8(struct MPT3SAS_ADAPTER *ioc, Mpi2ConfigReply_t
>  	*mpi_reply, Mpi2IOUnitPage8_t *config_page);
>  int mpt3sas_config_get_sas_iounit_pg1(struct MPT3SAS_ADAPTER *ioc,
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_config.c b/drivers/scsi/mpt3sas/mpt3sas_config.c
> index 45ac853e1289..ef07825046bc 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_config.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_config.c
> @@ -991,6 +991,42 @@ mpt3sas_config_get_iounit_pg3(struct MPT3SAS_ADAPTER *ioc,
>  	return r;
>  }
>  
> +/**
> + * mpt3sas_config_get_iounit_pg7 - obtain iounit page 7
> + * @ioc: per adapter object
> + * @mpi_reply: reply mf payload returned from firmware
> + * @config_page: contents of the config page
> + * Context: sleep.
> + *
> + * Return: 0 for success, non-zero for failure.
> + */
> +int
> +mpt3sas_config_get_iounit_pg7(struct MPT3SAS_ADAPTER *ioc,

Please do not break the line after "int"

> +	Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage7_t *config_page)
> +{
> +	Mpi2ConfigRequest_t mpi_request;
> +	int r;
> +
> +	memset(&mpi_request, 0, sizeof(Mpi2ConfigRequest_t));
> +	mpi_request.Function = MPI2_FUNCTION_CONFIG;
> +	mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_HEADER;
> +	mpi_request.Header.PageType = MPI2_CONFIG_PAGETYPE_IO_UNIT;
> +	mpi_request.Header.PageNumber = 7;
> +	mpi_request.Header.PageVersion = MPI2_IOUNITPAGE7_PAGEVERSION;
> +	ioc->build_zero_len_sge_mpi(ioc, &mpi_request.PageBufferSGE);
> +	r = _config_request(ioc, &mpi_request, mpi_reply,
> +	    MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, NULL, 0);

	r = _config_request(ioc, &mpi_request, mpi_reply,
			    MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, NULL, 0);

is a lot nicer to read.

> +	if (r)
> +		goto out;
> +
> +	mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_READ_CURRENT;
> +	r = _config_request(ioc, &mpi_request, mpi_reply,
> +	    MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page,
> +	    sizeof(*config_page));

Same here, please align the arguments.

	r = _config_request(ioc, &mpi_request, mpi_reply,
			    MPT3_CONFIG_PAGE_DEFAULT_TIMEOUT,
			    config_page, sizeof(*config_page));

With that, looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research