Patchwork [U-Boot,1/8] tegra: usb: convert USB_PORTS_MAX to be a define

login
register
mail settings
Submitter Lucas Stach
Date Oct. 30, 2012, 9:22 a.m.
Message ID <1351588973-20699-1-git-send-email-dev@lynxeye.de>
Download mbox | patch
Permalink /patch/195336/
State Superseded
Delegated to: Marek Vasut
Headers show

Comments

Lucas Stach - Oct. 30, 2012, 9:22 a.m.
No point in having this as an enum. Also while at it set it to the real hardware
maximum for both Tegra 2 and Tegra 3. If new Tegra hardware includes more
USB controllers we can always bump the limit then.

Signed-off-by: Lucas Stach <dev@lynxeye.de>
---
 arch/arm/cpu/armv7/tegra20/usb.c | 4 +---
 1 Datei geändert, 1 Zeile hinzugefügt(+), 3 Zeilen entfernt(-)
Simon Glass - Oct. 30, 2012, 1:09 p.m.
Hi Lucas,

On Tue, Oct 30, 2012 at 2:22 AM, Lucas Stach <dev@lynxeye.de> wrote:
> No point in having this as an enum. Also while at it set it to the real hardware
> maximum for both Tegra 2 and Tegra 3. If new Tegra hardware includes more
> USB controllers we can always bump the limit then.
>
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> ---
>  arch/arm/cpu/armv7/tegra20/usb.c | 4 +---
>  1 Datei geändert, 1 Zeile hinzugefügt(+), 3 Zeilen entfernt(-)
>
> diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
> index 1bccf2b..9fd1edc 100644
> --- a/arch/arm/cpu/armv7/tegra20/usb.c
> +++ b/arch/arm/cpu/armv7/tegra20/usb.c
> @@ -43,9 +43,7 @@
>         #endif
>  #endif
>
> -enum {
> -       USB_PORTS_MAX   = 4,                    /* Maximum ports we allow */
> -};
> +#define USB_PORTS_MAX  3               /* Maximum ports we allow */

That's fine with me if you wan to change it.

I tend to use enums most of the time. It shows up as a symbol in the
debugger, avoids bracketed expressions, side-effects and the like, and
works well when numbering multiple things (automatic increment). It's
also a welcome language feature IMO - use it or lose it :-) But in
this case the benefit is small.

>
>  /* Parameters we need for USB */
>  enum {
> --
> 1.7.11.7
>

Regards,
Simon
Marek Vasut - Oct. 30, 2012, 1:11 p.m.
Dear Simon Glass,

> Hi Lucas,
> 
> On Tue, Oct 30, 2012 at 2:22 AM, Lucas Stach <dev@lynxeye.de> wrote:
> > No point in having this as an enum. Also while at it set it to the real
> > hardware maximum for both Tegra 2 and Tegra 3. If new Tegra hardware
> > includes more USB controllers we can always bump the limit then.
> > 
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> > 
> >  arch/arm/cpu/armv7/tegra20/usb.c | 4 +---
> >  1 Datei geändert, 1 Zeile hinzugefügt(+), 3 Zeilen entfernt(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/tegra20/usb.c
> > b/arch/arm/cpu/armv7/tegra20/usb.c index 1bccf2b..9fd1edc 100644
> > --- a/arch/arm/cpu/armv7/tegra20/usb.c
> > +++ b/arch/arm/cpu/armv7/tegra20/usb.c
> > @@ -43,9 +43,7 @@
> > 
> >         #endif
> >  
> >  #endif
> > 
> > -enum {
> > -       USB_PORTS_MAX   = 4,                    /* Maximum ports we allow
> > */ -};
> > +#define USB_PORTS_MAX  3               /* Maximum ports we allow */
> 
> That's fine with me if you wan to change it.
> 
> I tend to use enums most of the time. It shows up as a symbol in the
> debugger, avoids bracketed expressions, side-effects and the like, and
> works well when numbering multiple things (automatic increment). It's
> also a welcome language feature IMO - use it or lose it :-) But in
> this case the benefit is small.

What about using static const int ?

> >  /* Parameters we need for USB */
> >  enum {
> > 
> > --
> > 1.7.11.7
> 
> Regards,
> Simon

Best regards,
Marek Vasut
Simon Glass - Oct. 30, 2012, 1:40 p.m.
Hi Marek,

On Tue, Oct 30, 2012 at 6:11 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Simon Glass,
>
>> Hi Lucas,
>>
>> On Tue, Oct 30, 2012 at 2:22 AM, Lucas Stach <dev@lynxeye.de> wrote:
>> > No point in having this as an enum. Also while at it set it to the real
>> > hardware maximum for both Tegra 2 and Tegra 3. If new Tegra hardware
>> > includes more USB controllers we can always bump the limit then.
>> >
>> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
>> > ---
>> >
>> >  arch/arm/cpu/armv7/tegra20/usb.c | 4 +---
>> >  1 Datei geändert, 1 Zeile hinzugefügt(+), 3 Zeilen entfernt(-)
>> >
>> > diff --git a/arch/arm/cpu/armv7/tegra20/usb.c
>> > b/arch/arm/cpu/armv7/tegra20/usb.c index 1bccf2b..9fd1edc 100644
>> > --- a/arch/arm/cpu/armv7/tegra20/usb.c
>> > +++ b/arch/arm/cpu/armv7/tegra20/usb.c
>> > @@ -43,9 +43,7 @@
>> >
>> >         #endif
>> >
>> >  #endif
>> >
>> > -enum {
>> > -       USB_PORTS_MAX   = 4,                    /* Maximum ports we allow
>> > */ -};
>> > +#define USB_PORTS_MAX  3               /* Maximum ports we allow */
>>
>> That's fine with me if you wan to change it.
>>
>> I tend to use enums most of the time. It shows up as a symbol in the
>> debugger, avoids bracketed expressions, side-effects and the like, and
>> works well when numbering multiple things (automatic increment). It's
>> also a welcome language feature IMO - use it or lose it :-) But in
>> this case the benefit is small.
>
> What about using static const int ?

That's fine too.

[snip]

>
> Best regards,
> Marek Vasut

Regards,
Simon
Marek Vasut - Nov. 2, 2012, 8:41 p.m.
Dear Lucas Stach,

[...]

Seeing Simon's comments, I expect new version of this series, right?

Best regards,
Marek Vasut

Patch

diff --git a/arch/arm/cpu/armv7/tegra20/usb.c b/arch/arm/cpu/armv7/tegra20/usb.c
index 1bccf2b..9fd1edc 100644
--- a/arch/arm/cpu/armv7/tegra20/usb.c
+++ b/arch/arm/cpu/armv7/tegra20/usb.c
@@ -43,9 +43,7 @@ 
 	#endif
 #endif
 
-enum {
-	USB_PORTS_MAX	= 4,			/* Maximum ports we allow */
-};
+#define USB_PORTS_MAX	3		/* Maximum ports we allow */
 
 /* Parameters we need for USB */
 enum {