Re: [PATCH 3/5] drivers:usb:fsl: Replace macros with enumerated type

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, May 26, 2015 at 05:15:48PM +0530, Nikhil Badola wrote:
> Replace macros with enumerated type to represent usb ip
> controller version
> 
> Signed-off-by: Nikhil Badola <nikhil.badola@freescale.com>
> ---
>  include/linux/fsl_devices.h | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
> index 2a2f56b..6447b7b 100644
> --- a/include/linux/fsl_devices.h
> +++ b/include/linux/fsl_devices.h
> @@ -20,11 +20,6 @@
>  #define FSL_UTMI_PHY_DLY	10	/*As per P1010RM, delay for UTMI
>  				PHY CLK to become stable - 10ms*/
>  #define FSL_USB_PHY_CLK_TIMEOUT	10000	/* uSec */
> -#define FSL_USB_VER_OLD		0
> -#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
>  
>  #include <linux/types.h>
>  
> @@ -52,6 +47,14 @@
>   *
>   */
>  
> +enum fsl_usb2_controller_ver {
> +	FSL_USB_VER_OLD = 0,
> +	FSL_USB_VER_1_6,
> +	FSL_USB_VER_2_2,
> +	FSL_USB_VER_2_4,
> +	FSL_USB_VER_2_5,
> +};

Don't you want to change that function that uses these to return the
enumerated type and not an integer?

Also, I don't think the C standard forces the non-numberd values to be
sequential, so I think the compiler could assign FSL_USB_VER_1_6 to be
45 and then FSL_USB_VER_2_2 to be 55 and so on.  So it's much better, if
you are going to be testing values, to be explicit about the values.
Please set them all here.

thanks,

greg k-h