> -----Original Message----- > From: Greg KH [mailto:greg@xxxxxxxxx] > Sent: Monday, September 29, 2014 11:56 AM > To: Badola Nikhil-B46172 > Cc: linux-usb@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] drivers: usb :fsl: Add support for USB controller version- > 2.5 > > 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. Shall I float the patch for replacing macros with enum type on top of this patch? -- 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
