RE: [PATCH 03/13] staging: dwc2: unshift non-bool register value constants

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx]
> Sent: Wednesday, July 17, 2013 8:10 AM
> 
> Various register fields wider than one bit have constants defined for
> their value. Previously, these registers would define the values as they
> appear in the register, so shifted to the right to the position the
> value appears in the register.
> 
> This commit changes those constants to their natural values (e.g, 0, 1,
> 2, etc.), as they are after shifting the register value to the right.
> This also changes all relevant code to shift the values before comparing
> them with constants.
> 
> This has the advantage that the values can be stored in smaller
> variables (now they always require a u32) and makes the handling of
> these values more consistent with other register fields that represent
> natural numbers instead of enumerations (e.g., number of host channels).

Hi Matthijs,

Are you sure the advantages outweigh the drawbacks and the amount
of code churn here? There are a lot of changes similar to this:

> -	u32 hs_phy_type = hsotg->hwcfg2 & GHWCFG2_HS_PHY_TYPE_MASK;
> -	u32 fs_phy_type = hsotg->hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK;
> +	u32 hs_phy_type = (hsotg->hwcfg2 & GHWCFG2_HS_PHY_TYPE_MASK) >>
> +			  GHWCFG2_HS_PHY_TYPE_SHIFT;
> +	u32 fs_phy_type = (hsotg->hwcfg2 & GHWCFG2_FS_PHY_TYPE_MASK) >>
> +			  GHWCFG2_FS_PHY_TYPE_SHIFT;

I.e we went from two lines of code to four. Other than saving a tiny
amount of RAM by being able to store the config values in a smaller
variable, are there any other advantages?

-- 
Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux