| Message ID | 1349910781-32088-2-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, Let's do it by the book this time :) On Thu, Oct 11, 2012 at 10:12 AM, Simon Glass <sjg@chromium.org> wrote: > This is a ulong for some architectures and just unsigned for others. > Change x86 to be consistent. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > arch/x86/include/asm/u-boot.h | 2 +- > common/cmd_bdinfo.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/u-boot.h b/arch/x86/include/asm/u-boot.h > index da667c5..0671b8d 100644 > --- a/arch/x86/include/asm/u-boot.h > +++ b/arch/x86/include/asm/u-boot.h > @@ -48,7 +48,7 @@ typedef struct bd_info { > unsigned short bi_ethspeed; /* Ethernet speed in Mbps */ > unsigned long bi_intfreq; /* Internal Freq, in MHz */ > unsigned long bi_busfreq; /* Bus Freq, in MHz */ > - unsigned int bi_baudrate; /* Console Baudrate */ > + unsigned long bi_baudrate; /* Console Baudrate */ > unsigned long bi_boot_params; /* where this board expects params */ > struct /* RAM configuration */ > { > diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c > index 23bd8a5..9bc0ebc 100644 > --- a/common/cmd_bdinfo.c > +++ b/common/cmd_bdinfo.c > @@ -439,7 +439,7 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > printf("ip_addr = %s\n", getenv("ipaddr")); > print_mhz("ethspeed", bd->bi_ethspeed); > #endif > - printf("baudrate = %d bps\n", bd->bi_baudrate); > + printf("baudrate = %ld bps\n", bd->bi_baudrate); > > return 0; > } > -- > 1.7.7.3 > Acked-by: Graeme Russ <graeme.russ@gmail.com>
Dear Simon Glass, In message <1349910781-32088-2-git-send-email-sjg@chromium.org> you wrote: > This is a ulong for some architectures and just unsigned for others. > Change x86 to be consistent. Given the limited range for this variable it makes no sense to use a long for this. Please fix this the other way round, i. e. change the architectures that use a long. Thanks. Best regards, Wolfgang Denk
Hi Wolfgang, On Oct 11, 2012 6:32 PM, "Wolfgang Denk" <wd@denx.de> wrote: > > Dear Simon Glass, > > In message <1349910781-32088-2-git-send-email-sjg@chromium.org> you wrote: > > This is a ulong for some architectures and just unsigned for others. > > Change x86 to be consistent. > > Given the limited range for this variable it makes no sense to use a > long for this. Please fix this the other way round, i. e. change the > architectures that use a long. Will unsigned hold 115200? Regards, Graeme
Hi Wolfgang, On Thu, Oct 11, 2012 at 12:32 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear Simon Glass, > > In message <1349910781-32088-2-git-send-email-sjg@chromium.org> you wrote: >> This is a ulong for some architectures and just unsigned for others. >> Change x86 to be consistent. > > Given the limited range for this variable it makes no sense to use a > long for this. Please fix this the other way round, i. e. change the > architectures that use a long. OK I will send out a series that changes them to unsigned long. This sort of thing relates to the unified board init series I was working on earlier it the year, since then all archs can use the same structures for generic fields like baud rate. I hope to get back to that one day. Regards, Simon > > Thanks. > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > Always try to do things in chronological order; it's less confusing > that way.
Hi Simon, On Fri, Oct 12, 2012 at 11:46 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Wolfgang, > > On Thu, Oct 11, 2012 at 12:32 AM, Wolfgang Denk <wd@denx.de> wrote: >> Dear Simon Glass, >> >> In message <1349910781-32088-2-git-send-email-sjg@chromium.org> you wrote: >>> This is a ulong for some architectures and just unsigned for others. >>> Change x86 to be consistent. >> >> Given the limited range for this variable it makes no sense to use a >> long for this. Please fix this the other way round, i. e. change the >> architectures that use a long. > > OK I will send out a series that changes them to unsigned long. Should we just change them all to u32 to be clear on the value range? Regards, Graeme
Hi, On Thu, Oct 11, 2012 at 5:51 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Simon, > > On Fri, Oct 12, 2012 at 11:46 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Wolfgang, >> >> On Thu, Oct 11, 2012 at 12:32 AM, Wolfgang Denk <wd@denx.de> wrote: >>> Dear Simon Glass, >>> >>> In message <1349910781-32088-2-git-send-email-sjg@chromium.org> you wrote: >>>> This is a ulong for some architectures and just unsigned for others. >>>> Change x86 to be consistent. >>> >>> Given the limited range for this variable it makes no sense to use a >>> long for this. Please fix this the other way round, i. e. change the >>> architectures that use a long. >> >> OK I will send out a series that changes them to unsigned long. > > Should we just change them all to u32 to be clear on the value range? Sorry, I meant unsigned int. Are there architectures in U-Boot which use a 16-bit int? Regards, Simon > > Regards, > > Graeme
Hi Simon, On Fri, Oct 12, 2012 at 11:55 AM, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On Thu, Oct 11, 2012 at 5:51 PM, Graeme Russ <graeme.russ@gmail.com> wrote: >> Hi Simon, >> >> On Fri, Oct 12, 2012 at 11:46 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Wolfgang, >>> >>> On Thu, Oct 11, 2012 at 12:32 AM, Wolfgang Denk <wd@denx.de> wrote: >>>> Dear Simon Glass, >>>> >>>> In message <1349910781-32088-2-git-send-email-sjg@chromium.org> you wrote: >>>>> This is a ulong for some architectures and just unsigned for others. >>>>> Change x86 to be consistent. >>>> >>>> Given the limited range for this variable it makes no sense to use a >>>> long for this. Please fix this the other way round, i. e. change the >>>> architectures that use a long. >>> >>> OK I will send out a series that changes them to unsigned long. >> >> Should we just change them all to u32 to be clear on the value range? > > Sorry, I meant unsigned int. > > Are there architectures in U-Boot which use a 16-bit int? The C standard does not guarantee unsigned int will be at least 32-bits. It is possible (although I have not checked it) that compiling 'real-mode' x86 code can produce 16-bit ints (not that we do that in U-Boot) using u32 will always guarantee a 32-bit unsigned value - better to be safe than sorry Regards, Graeme
Hi Graham, On Thu, Oct 11, 2012 at 6:00 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Simon, > > On Fri, Oct 12, 2012 at 11:55 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi, >> >> On Thu, Oct 11, 2012 at 5:51 PM, Graeme Russ <graeme.russ@gmail.com> wrote: >>> Hi Simon, >>> >>> On Fri, Oct 12, 2012 at 11:46 AM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi Wolfgang, >>>> >>>> On Thu, Oct 11, 2012 at 12:32 AM, Wolfgang Denk <wd@denx.de> wrote: >>>>> Dear Simon Glass, >>>>> >>>>> In message <1349910781-32088-2-git-send-email-sjg@chromium.org> you wrote: >>>>>> This is a ulong for some architectures and just unsigned for others. >>>>>> Change x86 to be consistent. >>>>> >>>>> Given the limited range for this variable it makes no sense to use a >>>>> long for this. Please fix this the other way round, i. e. change the >>>>> architectures that use a long. >>>> >>>> OK I will send out a series that changes them to unsigned long. >>> >>> Should we just change them all to u32 to be clear on the value range? >> >> Sorry, I meant unsigned int. >> >> Are there architectures in U-Boot which use a 16-bit int? > > The C standard does not guarantee unsigned int will be at least > 32-bits. It is possible (although I have not checked it) that > compiling 'real-mode' x86 code can produce 16-bit ints (not that we do > that in U-Boot) > > using u32 will always guarantee a 32-bit unsigned value - better to be > safe than sorry > That's fine, but what about all the other fields? Does the same apply here? It seems that everything uses 'unsigned int' for u32: arch/nds32/include/asm/types.h:47:typedef unsigned int u32; arch/avr32/include/asm/types.h:65:typedef unsigned int u32; arch/arm/include/asm/types.h:37:typedef unsigned int u32; arch/sandbox/include/asm/types.h:59:typedef unsigned int u32; arch/blackfin/include/asm/types.h:70:typedef unsigned int u32; arch/openrisc/include/asm/types.h:64:typedef unsigned int u32; arch/mips/include/asm/types.h:62:typedef unsigned int u32; arch/x86/include/asm/types.h:37:typedef unsigned int u32; arch/nios2/include/asm/types.h:45:typedef unsigned int u32; arch/sh/include/asm/types.h:46:typedef unsigned int u32; arch/microblaze/include/asm/types.h:45:typedef unsigned int u32; arch/powerpc/include/asm/types.h:37:typedef unsigned int u32; arch/m68k/include/asm/types.h:37:typedef unsigned int u32; arch/sparc/include/asm/types.h:58:typedef unsigned int u32; I wonder then if we need to decide which to use (int or long). I presume 'long' was used to cope with 16-bit machines, but we don't actually have any (which is presumably Wolfgang's point). If we don't have any, then 'int' is safe. I'm just a bit unsure about changing a single field in these two structures to u32, when all the others are unsigned long/short. For example: typedef struct bd_info { int bi_baudrate; /* serial console baudrate */ ulong bi_arch_number; /* unique id for this board */ ulong bi_boot_params; /* where this board expects params */ unsigned long bi_arm_freq; /* arm frequency */ unsigned long bi_dsp_freq; /* dsp core frequency */ unsigned long bi_ddr_freq; /* ddr frequency */ struct /* RAM configuration */ { ulong start; ulong size; } bi_dram[CONFIG_NR_DRAM_BANKS]; } bd_t; typedef struct global_data { bd_t *bd; unsigned long flags; unsigned long baudrate; unsigned long have_console; /* serial_init() was called */ #ifdef CONFIG_PRE_CONSOLE_BUFFER unsigned long precon_buf_idx; /* Pre-Console buffer index */ #endif unsigned long env_addr; /* Address of Environment struct */ unsigned long env_valid; /* Checksum of Environment valid? */ unsigned long fb_base; /* base address of frame buffer */ #ifdef CONFIG_FSL_ESDHC unsigned long sdhc_clk; #endif #ifdef CONFIG_AT91FAMILY /* "static data" needed by at91's clock.c */ unsigned long cpu_clk_rate_hz; unsigned long main_clk_rate_hz; unsigned long mck_rate_hz; unsigned long plla_rate_hz; unsigned long pllb_rate_hz; unsigned long at91_pllb_usb_init; #endif #ifdef CONFIG_ARM /* "static data" needed by most of timer.c on ARM platforms */ unsigned long timer_rate_hz; unsigned long tbl; unsigned long tbu; unsigned long long timer_reset_value; unsigned long lastinc; Guidance appreciated! .... > Regards, > > Graeme Regards, Simon
diff --git a/arch/x86/include/asm/u-boot.h b/arch/x86/include/asm/u-boot.h index da667c5..0671b8d 100644 --- a/arch/x86/include/asm/u-boot.h +++ b/arch/x86/include/asm/u-boot.h @@ -48,7 +48,7 @@ typedef struct bd_info { unsigned short bi_ethspeed; /* Ethernet speed in Mbps */ unsigned long bi_intfreq; /* Internal Freq, in MHz */ unsigned long bi_busfreq; /* Bus Freq, in MHz */ - unsigned int bi_baudrate; /* Console Baudrate */ + unsigned long bi_baudrate; /* Console Baudrate */ unsigned long bi_boot_params; /* where this board expects params */ struct /* RAM configuration */ { diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c index 23bd8a5..9bc0ebc 100644 --- a/common/cmd_bdinfo.c +++ b/common/cmd_bdinfo.c @@ -439,7 +439,7 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) printf("ip_addr = %s\n", getenv("ipaddr")); print_mhz("ethspeed", bd->bi_ethspeed); #endif - printf("baudrate = %d bps\n", bd->bi_baudrate); + printf("baudrate = %ld bps\n", bd->bi_baudrate); return 0; }
This is a ulong for some architectures and just unsigned for others. Change x86 to be consistent. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/x86/include/asm/u-boot.h | 2 +- common/cmd_bdinfo.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)