Re: [PATCH] drivers: usb :fsl: Add support for USB controller version-2.5

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

 



On Mon, Sep 29, 2014 at 03:46:02AM +0000, nikhil.badola@xxxxxxxxxxxxx wrote:
> >-----Original Message-----
> >From: Greg KH [mailto:greg@xxxxxxxxx]
> >Sent: Wednesday, September 24, 2014 10:19 AM
> >To: Badola Nikhil-B46172
> >Cc: linux-usb@xxxxxxxxxxxxxxx
> >Subject: Re: [PATCH] drivers: usb :fsl: Add support for USB controller version-2.5
> >
> >On Thu, Aug 21, 2014 at 12:56:22PM +0530, Nikhil Badola wrote:
> >> Add support for USB controller version-2.5 used in
> >> T4240 rev2.0, T1024, B3421, T1040, T2080, LS1021A.
> >>
> >> Signed-off-by: Nikhil Badola <nikhil.badola@xxxxxxxxxxxxx>
> >> ---
> >> 	- Depends on commit 990c2c7829d98517228f2b2ff14919c83b75e124
> >> 	  drivers: usb: fsl: Check IP version 2.4 for mph USB controller
> >>
> >>  drivers/usb/host/fsl-mph-dr-of.c | 5 +++++
> >>  include/linux/fsl_devices.h      | 1 +
> >>  2 files changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/usb/host/fsl-mph-dr-of.c
> >> b/drivers/usb/host/fsl-mph-dr-of.c
> >> index e0315de..45b9e36 100644
> >> --- a/drivers/usb/host/fsl-mph-dr-of.c
> >> +++ b/drivers/usb/host/fsl-mph-dr-of.c
> >> @@ -127,6 +127,7 @@ static int usb_get_ver_info(struct device_node *np)
> >>  	 * returns 1 for usb controller version 1.6
> >>  	 * returns 2 for usb controller version 2.2
> >>  	 * returns 3 for usb controller version 2.4
> >> +	 * returns 4 for usb controller version 2.5
> >>  	 * returns 0 otherwise
> >>  	 */
> >>  	if (of_device_is_compatible(np, "fsl-usb2-dr")) { @@ -136,6 +137,8
> >> @@ static int usb_get_ver_info(struct device_node *np)
> >>  			ver = FSL_USB_VER_2_2;
> >>  		else if (of_device_is_compatible(np, "fsl-usb2-dr-v2.4"))
> >>  			ver = FSL_USB_VER_2_4;
> >> +		else if (of_device_is_compatible(np, "fsl-usb2-dr-v2.5"))
> >> +			ver = FSL_USB_VER_2_5;
> >>  		else /* for previous controller versions */
> >>  			ver = FSL_USB_VER_OLD;
> >>
> >> @@ -153,6 +156,8 @@ static int usb_get_ver_info(struct device_node *np)
> >>  			ver = FSL_USB_VER_2_2;
> >>  		else if (of_device_is_compatible(np, "fsl-usb2-mph-v2.4"))
> >>  			ver = FSL_USB_VER_2_4;
> >> +		else if (of_device_is_compatible(np, "fsl-usb2-mph-v2.5"))
> >> +			ver = FSL_USB_VER_2_5;
> >>  		else /* for previous controller versions */
> >>  			ver = FSL_USB_VER_OLD;
> >>  	}
> >> diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
> >> index a82296a..2a2f56b 100644
> >> --- a/include/linux/fsl_devices.h
> >> +++ b/include/linux/fsl_devices.h
> >> @@ -24,6 +24,7 @@
> >>  #define FSL_USB_VER_1_6		1
> >>  #define FSL_USB_VER_2_2		2
> >>  #define FSL_USB_VER_2_4		3
> >> +#define FSL_USB_VER_2_5		4
> >
> >Why not just keep going and add the rest of the numbers while you are at it?
> 
> This is the last controller version of this family hence there would not be any requirement 
> to add further numbers.

People always bring products back, you never know this :)

> >Seriously, what are these two patches doing?  You just set the value, never do
> >anything with it, what good is it?
> 
> We indeed use these macros for controller version specific code, for example in following snippet from ehci-fsl.c
> if (pdata->have_sysif_regs &&
>             pdata->controller_ver > FSL_USB_VER_1_6 &&
>             (phy_mode == FSL_USB2_PHY_ULPI)) {
>                 /* check PHY_CLK_VALID to get phy clk valid */
> .
> .
> If we don't set the macros then by default FSL_USB_VER_OLD, which is 0, is assigned as
> controller version and in that case phy_clk_valid bit would not be checked for controller version 2.4 and 2.5. 

You are relying on a define to be > a specific value, which seems wrong
as it's impossible to figure this type of thing out.  Please use an
enumerated type at the very least if you want to test this type of
thing.

thanks,

greg k-h
--
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