All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: hisi_sas: silence a static checker warning
@ 2017-06-23 15:15 ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-06-23 15:15 UTC (permalink / raw)
  To: John Garry
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, kernel-janitors

phy->phy_type is a u64.  We only ever use the first two bits so it's a
bit over kill perhaps.  Anyway, let's declare the flags as ULL as well
because that lets us do things like "phy->phy_type &= ~PORT_TYPE_SAS;".
In the current code, static checkers complain that that would
unintentionally clear the high 32 bits as well.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 4fc23087a939..b23245aeab74 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -56,8 +56,8 @@
 struct hisi_hba;
 
 enum {
-	PORT_TYPE_SAS = (1U << 1),
-	PORT_TYPE_SATA = (1U << 0),
+	PORT_TYPE_SAS = (1ULL << 1),
+	PORT_TYPE_SATA = (1ULL << 0),
 };
 
 enum dev_status {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] scsi: hisi_sas: silence a static checker warning
@ 2017-06-23 15:15 ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-06-23 15:15 UTC (permalink / raw)
  To: John Garry
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, kernel-janitors

phy->phy_type is a u64.  We only ever use the first two bits so it's a
bit over kill perhaps.  Anyway, let's declare the flags as ULL as well
because that lets us do things like "phy->phy_type &= ~PORT_TYPE_SAS;".
In the current code, static checkers complain that that would
unintentionally clear the high 32 bits as well.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 4fc23087a939..b23245aeab74 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -56,8 +56,8 @@
 struct hisi_hba;
 
 enum {
-	PORT_TYPE_SAS = (1U << 1),
-	PORT_TYPE_SATA = (1U << 0),
+	PORT_TYPE_SAS = (1ULL << 1),
+	PORT_TYPE_SATA = (1ULL << 0),
 };
 
 enum dev_status {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: hisi_sas: silence a static checker warning
  2017-06-23 15:15 ` Dan Carpenter
@ 2017-06-23 15:25   ` John Garry
  -1 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2017-06-23 15:25 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
	kernel-janitors, Linuxarm

On 23/06/2017 16:15, Dan Carpenter wrote:
> phy->phy_type is a u64.  We only ever use the first two bits so it's a
> bit over kill perhaps.

Hi Dan,

Right, u64 is unneeded and u32 would suffice, so I think that would be a 
more appropriate fix. And if we change hisi_sas_phy.phy_type to u32, I 
would want to reorder this structure to improve packing efficiency.

I can make this change unless you really want to...

Thanks,
John

Anyway, let's declare the flags as ULL as well
> because that lets us do things like "phy->phy_type &= ~PORT_TYPE_SAS;".
> In the current code, static checkers complain that that would
> unintentionally clear the high 32 bits as well.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
> index 4fc23087a939..b23245aeab74 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas.h
> +++ b/drivers/scsi/hisi_sas/hisi_sas.h
> @@ -56,8 +56,8 @@
>  struct hisi_hba;
>
>  enum {
> -	PORT_TYPE_SAS = (1U << 1),
> -	PORT_TYPE_SATA = (1U << 0),
> +	PORT_TYPE_SAS = (1ULL << 1),
> +	PORT_TYPE_SATA = (1ULL << 0),
>  };
>
>  enum dev_status {
>
> .
>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: hisi_sas: silence a static checker warning
@ 2017-06-23 15:25   ` John Garry
  0 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2017-06-23 15:25 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
	kernel-janitors, Linuxarm

On 23/06/2017 16:15, Dan Carpenter wrote:
> phy->phy_type is a u64.  We only ever use the first two bits so it's a
> bit over kill perhaps.

Hi Dan,

Right, u64 is unneeded and u32 would suffice, so I think that would be a 
more appropriate fix. And if we change hisi_sas_phy.phy_type to u32, I 
would want to reorder this structure to improve packing efficiency.

I can make this change unless you really want to...

Thanks,
John

Anyway, let's declare the flags as ULL as well
> because that lets us do things like "phy->phy_type &= ~PORT_TYPE_SAS;".
> In the current code, static checkers complain that that would
> unintentionally clear the high 32 bits as well.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
> index 4fc23087a939..b23245aeab74 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas.h
> +++ b/drivers/scsi/hisi_sas/hisi_sas.h
> @@ -56,8 +56,8 @@
>  struct hisi_hba;
>
>  enum {
> -	PORT_TYPE_SAS = (1U << 1),
> -	PORT_TYPE_SATA = (1U << 0),
> +	PORT_TYPE_SAS = (1ULL << 1),
> +	PORT_TYPE_SATA = (1ULL << 0),
>  };
>
>  enum dev_status {
>
> .
>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: hisi_sas: silence a static checker warning
  2017-06-23 15:25   ` John Garry
@ 2017-06-23 15:33     ` Dan Carpenter
  -1 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-06-23 15:33 UTC (permalink / raw)
  To: John Garry
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
	kernel-janitors, Linuxarm

On Fri, Jun 23, 2017 at 04:25:27PM +0100, John Garry wrote:
> On 23/06/2017 16:15, Dan Carpenter wrote:
> > phy->phy_type is a u64.  We only ever use the first two bits so it's a
> > bit over kill perhaps.
> 
> Hi Dan,
> 
> Right, u64 is unneeded and u32 would suffice, so I think that would be a
> more appropriate fix. And if we change hisi_sas_phy.phy_type to u32, I would
> want to reorder this structure to improve packing efficiency.
> 
> I can make this change unless you really want to...
> 

Feel free to make the change.  :)

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] scsi: hisi_sas: silence a static checker warning
@ 2017-06-23 15:33     ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-06-23 15:33 UTC (permalink / raw)
  To: John Garry
  Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi,
	kernel-janitors, Linuxarm

On Fri, Jun 23, 2017 at 04:25:27PM +0100, John Garry wrote:
> On 23/06/2017 16:15, Dan Carpenter wrote:
> > phy->phy_type is a u64.  We only ever use the first two bits so it's a
> > bit over kill perhaps.
> 
> Hi Dan,
> 
> Right, u64 is unneeded and u32 would suffice, so I think that would be a
> more appropriate fix. And if we change hisi_sas_phy.phy_type to u32, I would
> want to reorder this structure to improve packing efficiency.
> 
> I can make this change unless you really want to...
> 

Feel free to make the change.  :)

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-06-23 15:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-23 15:15 [PATCH] scsi: hisi_sas: silence a static checker warning Dan Carpenter
2017-06-23 15:15 ` Dan Carpenter
2017-06-23 15:25 ` John Garry
2017-06-23 15:25   ` John Garry
2017-06-23 15:33   ` Dan Carpenter
2017-06-23 15:33     ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.