Thanks. We will move that to pm80xx_hwi.h. Regards, Viswas G > -----Original Message----- > From: Jack Wang [mailto:xjtuwjp@xxxxxxxxx] > Sent: Wednesday, August 30, 2017 9:34 PM > To: Viswas G <viswas.g@xxxxxxxxxxxxx> > Cc: linux-scsi@xxxxxxxxxxxxxxx; Vasanthalakshmi Tharmarajan > <vasanthalakshmi.thar@xxxxxxxxxxxxx> > Subject: Re: [PATCH 1/6] pm80xx : redefine sas_identify_frame structure > > EXTERNAL EMAIL > > > 2017-08-30 17:41 GMT+02:00 Viswas G <viswas.g@xxxxxxxxxxxxx>: > > Hi Jack, > > > > The firmware expectation is that there is no 4 byte CRC. This causes the > IOMB to be generated incorrectly for PHY_START. Local structure for SAS > Identify included in the driver so that it matches the Programmer's Manual > and Firmware expectations. > > > > The sas_identify struct from the kernel includes the checksum word (32- > bits) . So it is 32 bytes rather than 28 bytes. The changed size for the sas > identify frame means that the "spasti" field for PHY_START is at a different > offset than documented, word 11 rather than word 10. We're not changing > the phy analog settings so this doesn't really matter, but the docs indicate > that the sas frame's crc isn't included. So having a local definition will be > better than taking the system definition. > > > > Regards, > > Viswas G > > Hi Viswas, > > The spasti field is only for pm80xx, not for pm8001, I think it's > better to move sas_identify_frame_local to pm80xx_hwi.h. > > Regards, > Jack > > > > > >> -----Original Message----- > >> From: Jack Wang [mailto:xjtuwjp@xxxxxxxxx] > >> Sent: Tuesday, August 29, 2017 4:49 PM > >> To: Viswas G <viswas.g@xxxxxxxxxxxxx> > >> Cc: linux-scsi@xxxxxxxxxxxxxxx; Vasanthalakshmi Tharmarajan > >> <vasanthalakshmi.thar@xxxxxxxxxxxxx> > >> Subject: Re: [PATCH 1/6] pm80xx : redefine sas_identify_frame structure > >> > >> EXTERNAL EMAIL > >> > >> > >> 2015-01-30 7:06 GMT+01:00 Viswas G <Viswas.G@xxxxxxxxxxxxx>: > >> > sas_identify structure defined by pm80xx doesn't have CRC field. > >> > So added a new sas_identify structure without CRC. > >> > > >> > Signed-off-by: Raj Dinesh <Raj.Dinesh@xxxxxxxxxxxxx> > >> > Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx> > >> > --- > >> > drivers/scsi/pm8001/pm8001_hwi.h | 2 +- > >> > drivers/scsi/pm8001/pm8001_sas.h | 95 > >> ++++++++++++++++++++++++++++++++++++++++ > >> > drivers/scsi/pm8001/pm80xx_hwi.h | 2 +- > >> > 3 files changed, 97 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/drivers/scsi/pm8001/pm8001_hwi.h > >> b/drivers/scsi/pm8001/pm8001_hwi.h > >> > index e4867e690c84..f4331afe9b0b 100644 > >> > --- a/drivers/scsi/pm8001/pm8001_hwi.h > >> > +++ b/drivers/scsi/pm8001/pm8001_hwi.h > >> > @@ -157,7 +157,7 @@ struct mpi_msg_hdr{ > >> > struct phy_start_req { > >> > __le32 tag; > >> > __le32 ase_sh_lm_slr_phyid; > >> > - struct sas_identify_frame sas_identify; > >> > + struct sas_identify_frame_local sas_identify; > >> > u32 reserved[5]; > >> > } __attribute__((packed, aligned(4))); > >> > > >> > diff --git a/drivers/scsi/pm8001/pm8001_sas.h > >> b/drivers/scsi/pm8001/pm8001_sas.h > >> > index e81a8fa7ef1a..2e17505ed5b8 100644 > >> > --- a/drivers/scsi/pm8001/pm8001_sas.h > >> > +++ b/drivers/scsi/pm8001/pm8001_sas.h > >> > @@ -118,6 +118,101 @@ extern const struct pm8001_dispatch > >> pm8001_80xx_dispatch; > >> > struct pm8001_hba_info; > >> > struct pm8001_ccb_info; > >> > struct pm8001_device; > >> > +#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 > >> > /* define task management IU */ > >> > struct pm8001_tmf_task { > >> > u8 tmf; > >> > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h > >> b/drivers/scsi/pm8001/pm80xx_hwi.h > >> > index 7a443bad6163..1ee2ec210065 100644 > >> > --- a/drivers/scsi/pm8001/pm80xx_hwi.h > >> > +++ b/drivers/scsi/pm8001/pm80xx_hwi.h > >> > @@ -248,7 +248,7 @@ struct mpi_msg_hdr { > >> > struct phy_start_req { > >> > __le32 tag; > >> > __le32 ase_sh_lm_slr_phyid; > >> > - struct sas_identify_frame sas_identify; /* 28 Bytes */ > >> > + struct sas_identify_frame_local sas_identify; /* 28 Bytes */ > >> > __le32 spasti; > >> > u32 reserved[21]; > >> > } __attribute__((packed, aligned(4))); > >> > -- > >> > 2.12.3 > >> > > >> Did this cause any trouble? I guest not, as we does memset for > >> phy_start_req, why bother? > >> > >> Jack
