| Message ID | 20170623151544.57jlfbl5apofl6hj@mwanda (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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 { > > . >
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
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 {
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>