| Message ID | 201504201237.t3KCbWt3016906@palmhbs0.lsi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/20/2015 02:35 PM, Sumit.Saxena@avagotech.com wrote: > Swap the whole 32 bits we read from the hardware instead of swapping > just the 16bits we care about in place later. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/scsi/megaraid/megaraid_sas_base.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c > index e863701..fa6c3c9 100644 > --- a/drivers/scsi/megaraid/megaraid_sas_base.c > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c > @@ -4923,8 +4923,9 @@ megasas_register_aen(struct megasas_instance *instance, u32 seq_num, > > if (instance->aen_cmd) { > > - prev_aen.word = instance->aen_cmd->frame->dcmd.mbox.w[1]; > - prev_aen.members.locale = le16_to_cpu(prev_aen.members.locale); > + prev_aen.word = > + le32_to_cpu(instance->aen_cmd->frame->dcmd.mbox.w[1]); > + prev_aen.members.locale = prev_aen.members.locale; > > /* > * A class whose enum value is smaller is inclusive of all > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
On 04/20/2015 02:35 PM, Sumit.Saxena@avagotech.com wrote: > Swap the whole 32 bits we read from the hardware instead of swapping > just the 16bits we care about in place later. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/scsi/megaraid/megaraid_sas_base.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c > index e863701..fa6c3c9 100644 > --- a/drivers/scsi/megaraid/megaraid_sas_base.c > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c > @@ -4923,8 +4923,9 @@ megasas_register_aen(struct megasas_instance *instance, u32 seq_num, > > if (instance->aen_cmd) { > > - prev_aen.word = instance->aen_cmd->frame->dcmd.mbox.w[1]; > - prev_aen.members.locale = le16_to_cpu(prev_aen.members.locale); > + prev_aen.word = > + le32_to_cpu(instance->aen_cmd->frame->dcmd.mbox.w[1]); > + prev_aen.members.locale = prev_aen.members.locale; This^ looks odd, do we need the last assignment? Tomas > > /* > * A class whose enum value is smaller is inclusive of all -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 21, 2015 at 02:33:12PM +0200, Tomas Henzl wrote: > On 04/20/2015 02:35 PM, Sumit.Saxena@avagotech.com wrote: > > Swap the whole 32 bits we read from the hardware instead of swapping > > just the 16bits we care about in place later. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > drivers/scsi/megaraid/megaraid_sas_base.c | 5 +++-- > > 1 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c > > index e863701..fa6c3c9 100644 > > --- a/drivers/scsi/megaraid/megaraid_sas_base.c > > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c > > @@ -4923,8 +4923,9 @@ megasas_register_aen(struct megasas_instance *instance, u32 seq_num, > > > > if (instance->aen_cmd) { > > > > - prev_aen.word = instance->aen_cmd->frame->dcmd.mbox.w[1]; > > - prev_aen.members.locale = le16_to_cpu(prev_aen.members.locale); > > + prev_aen.word = > > + le32_to_cpu(instance->aen_cmd->frame->dcmd.mbox.w[1]); > > + prev_aen.members.locale = prev_aen.members.locale; > > This^ looks odd, do we need the last assignment? Obviously not, I was just in mechnical mode when doing this. It can be dropped for the repost. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>-----Original Message----- >From: Christoph Hellwig [mailto:hch@infradead.org] >Sent: Tuesday, April 21, 2015 6:07 PM >To: Tomas Henzl >Cc: Sumit.Saxena@avagotech.com; linux-scsi@vger.kernel.org; >martin.petersen@oracle.com; hch@infradead.org; >jbottomley@parallels.com; kashyap.desai@avagotech.com >Subject: Re: [PATCH RESEND 18/18] megaraid_sas : swap whole register in >megasas_register_aen > >On Tue, Apr 21, 2015 at 02:33:12PM +0200, Tomas Henzl wrote: >> On 04/20/2015 02:35 PM, Sumit.Saxena@avagotech.com wrote: >> > Swap the whole 32 bits we read from the hardware instead of swapping >> > just the 16bits we care about in place later. >> > >> > Signed-off-by: Christoph Hellwig <hch@lst.de> >> > --- >> > drivers/scsi/megaraid/megaraid_sas_base.c | 5 +++-- >> > 1 files changed, 3 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c >> > b/drivers/scsi/megaraid/megaraid_sas_base.c >> > index e863701..fa6c3c9 100644 >> > --- a/drivers/scsi/megaraid/megaraid_sas_base.c >> > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c >> > @@ -4923,8 +4923,9 @@ megasas_register_aen(struct megasas_instance >> > *instance, u32 seq_num, >> > >> > if (instance->aen_cmd) { >> > >> > - prev_aen.word = instance->aen_cmd->frame- >>dcmd.mbox.w[1]; >> > - prev_aen.members.locale = >le16_to_cpu(prev_aen.members.locale); >> > + prev_aen.word = >> > + le32_to_cpu(instance->aen_cmd->frame- >>dcmd.mbox.w[1]); >> > + prev_aen.members.locale = prev_aen.members.locale; >> >> This^ looks odd, do we need the last assignment? > >Obviously not, I was just in mechnical mode when doing this. > >It can be dropped for the repost. Will drop it in resubmission of the patch. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index e863701..fa6c3c9 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -4923,8 +4923,9 @@ megasas_register_aen(struct megasas_instance *instance, u32 seq_num, if (instance->aen_cmd) { - prev_aen.word = instance->aen_cmd->frame->dcmd.mbox.w[1]; - prev_aen.members.locale = le16_to_cpu(prev_aen.members.locale); + prev_aen.word = + le32_to_cpu(instance->aen_cmd->frame->dcmd.mbox.w[1]); + prev_aen.members.locale = prev_aen.members.locale; /* * A class whose enum value is smaller is inclusive of all
Swap the whole 32 bits we read from the hardware instead of swapping just the 16bits we care about in place later. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/scsi/megaraid/megaraid_sas_base.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)