RE: [PATCH v14 03/10] USB/ppc4xx: Add Synopsys DWC OTG Core Interface Layer (CIL)

[Tirumala Marri <tmarri@apm.com>]

<> +void dwc_otg_enable_global_interrupts(struct core_if *core_if)
<> +{
<> +	u32 ahbcfg = 0;
<> +
<> +	ahbcfg |= DWC_AHBCFG_GLBL_INT_MASK;
<> +	dwc_reg_modify(core_if->core_global_regs, DWC_GAHBCFG, 0,
<> +		     ahbcfg);
<
<What is the point of initializing the variable to 0, and then
<immediately
<ORing it with a value? Why not just:
<
<	u32 ahbcfg = DWC_AHBCFG_GLBL_INT_MASK;
<
<	dwc_reg_modify(core_if->core_global_regs, DWC_GAHBCFG,
<		     ahbcfg, 0);
<
<or even simpler:
<
<	dwc_reg_modify(core_if->core_global_regs, DWC_GAHBCFG,
<		       DWC_AHBCFG_GLBL_INT_MASK, 0);
<
<? Same for other places in this file.

[Tirumala Marri] I can definitely change this at some places.
This had to do because there was suggestion modify the API to accept
The offset. It happened to be some of the accesses doesn't have offsets.
<
<> +/**
<> + * Tests if the current hardware is using a full speed phy.
<> + */
<> +static inline int full_speed_phy(struct core_if *core_if)
<> +{
<> +	if ((DWC_HWCFG2_FS_PHY_TYPE_RD(core_if->hwcfg2) == 2 &&
<> +	     DWC_HWCFG2_FS_PHY_TYPE_RD(core_if->hwcfg2) == 1 &&
<
<Surely this is a typo? You are checking the same variable for containing
<two different values at the same time. From looking at similar code in
<the dwc_otg_core_init() function, I think you meant to write:

[Tirumala Marri] Will correct