[PATCH RESEND 18/18] megaraid_sas : swap whole register in megasas_register_aen

[PATCH RESEND 18/18] megaraid_sas : swap whole register in megasas_register_aen

[Sumit.Saxena]

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
-- 
1.7.1

Re: [PATCH RESEND 18/18] megaraid_sas : swap whole register in megasas_register_aen

[Hannes Reinecke]

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
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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

Re: [PATCH RESEND 18/18] megaraid_sas : swap whole register in megasas_register_aen

[Tomas Henzl]

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

Re: [PATCH RESEND 18/18] megaraid_sas : swap whole register in megasas_register_aen

[Christoph Hellwig]

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.

RE: [PATCH RESEND 18/18] megaraid_sas : swap whole register in megasas_register_aen

[Sumit Saxena]

>-----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.