Re: [PATCH 1/2] scsi: pm8001: Redefine sas_identify_frame structure

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Fri, 2026-05-15 at 12:15 +0000, Ronja Meyer wrote:
> The sas_identify structure defined by pm8001 doesn't have a CRC
> field.
> Add a new sas_identify_frame_local structure without the CRC field.
> The equivalent change was already made to the pm80xx driver in:
> commit 5990fd57ebea ("scsi: pm80xx: redefine sas_identify_frame
> structure")
> 
> Sending to stable, as this change is required for the fortify-panic
> fix
> later in this chain to apply cleanly.
> 
> Cc: stable@vger.kernel.org
> Fixes: dbf9bfe61571 ("[SCSI] pm8001: add SAS/SATA HBA driver")
> Signed-off-by: Ronja Meyer <rnj@google.com>
> ---
>  drivers/scsi/pm8001/pm8001_hwi.h | 101
> ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.h
> b/drivers/scsi/pm8001/pm8001_hwi.h
> index f1ce8df082b0..14b162f93eb8 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.h
> +++ b/drivers/scsi/pm8001/pm8001_hwi.h
> @@ -133,6 +133,103 @@
>  
>  /* for new SPC controllers MEMBASE III is shared between BIOS and
> DATA */
>  #define GSM_SM_BASE			0x4F0000
> +
> +#ifdef __LITTLE_ENDIAN_BITFIELD
> +struct sas_identify_frame_local {
> +	/* Byte 0 */
> +	u8  frame_type:4;
> +	u8  dev_type:3;
> +	u8  _un0:1;
> +
> +	/* Byte 1 */
> +	u8  _un1;
> +
> +	/* Byte 2 */
> +	union {
> +		struct {
> +			u8  _un20:1;
> +			u8  smp_iport:1;
> +			u8  stp_iport:1;
> +			u8  ssp_iport:1;
> +			u8  _un247:4;
> +		};
> +		u8 initiator_bits;
> +	};
> +
> +	/* Byte 3 */
> +	union {
> +		struct {
> +			u8  _un30:1;
> +			u8 smp_tport:1;
> +			u8 stp_tport:1;
> +			u8 ssp_tport:1;
> +			u8 _un347:4;
> +		};
> +		u8 target_bits;
> +	};
> +
> +	/* Byte 4 - 11 */
> +	u8 _un4_11[8];
> +
> +	/* Byte 12 - 19 */
> +	u8 sas_addr[SAS_ADDR_SIZE];
> +
> +	/* Byte 20 */
> +	u8 phy_id;
> +
> +	u8 _un21_27[7];
> +
> +} __packed;
> +
> +#elif defined(__BIG_ENDIAN_BITFIELD)
> +struct sas_identify_frame_local {
> +	/* Byte 0 */
> +	u8  _un0:1;
> +	u8  dev_type:3;
> +	u8  frame_type:4;
> +
> +	/* Byte 1 */
> +	u8  _un1;
> +
> +	/* Byte 2 */
> +	union {
> +		struct {
> +			u8  _un247:4;
> +			u8  ssp_iport:1;
> +			u8  stp_iport:1;
> +			u8  smp_iport:1;
> +			u8  _un20:1;
> +		};
> +		u8 initiator_bits;
> +	};
> +
> +	/* Byte 3 */
> +	union {
> +		struct {
> +			u8 _un347:4;
> +			u8 ssp_tport:1;
> +			u8 stp_tport:1;
> +			u8 smp_tport:1;
> +			u8 _un30:1;
> +		};
> +		u8 target_bits;
> +	};
> +
> +	/* Byte 4 - 11 */
> +	u8 _un4_11[8];
> +
> +	/* Byte 12 - 19 */
> +	u8 sas_addr[SAS_ADDR_SIZE];
> +
> +	/* Byte 20 */
> +	u8 phy_id;
> +
> +	u8 _un21_27[7];
> +} __packed;
> +#else
> +#error "Bitfield order not defined!"
> +#endif

This is basically a duplicate of what's in pm80xx_hwi.h, which looks a
bit ungainly ... couldn't they be unified?  Additionally, it does seem
we could add a #define to the definition in scsi/sas.h to omit the crc
field and allow you to use its definition for both drivers rather than
having to duplicate it like this.

Regards,

James