Re: [U-Boot] [PATCH v2 02/11] S3C24XX: Add core support for Samsung's S3C24XX SoCs

Dear José Miguel Gonçalves,
Hi Marek,
On 14-09-2012 19:03, Marek Vasut wrote:
Dear José Miguel Gonçalves,
It's getting better :)
Hopefully :-)
[...]
+typedef ulong(*getfreq) (void);
Is this used?
In the array declaration bellow...
Why, these are only values, no ?
+static const getfreq freq_f[] = {
const array const members, no?
Do you mean I should declare it like this:
static const getfreq const freq[] = { ...
Yes
I don't see the point because an array has no other storage besides it's elements. Moreover GCC generates the same object code in both ways.
Type checking, if you ever decided to write into the array, it'll prevent you from doing so.
- get_FCLK,
- get_HCLK,
- get_PCLK,
- get_UCLK,
+};
+static const char freq_c[] = { 'F', 'H', 'P', 'U' };
Same here.
+int print_cpuinfo(void) +{
- int i;
- char buf[32];
- ulong cpuid;
- struct s3c24xx_gpio *const gpio = s3c24xx_get_base_gpio();
- cpuid = readl(&gpio->gstatus[1]);
- printf("CPU: %8s (id %08lX) @ %s MHz\n", s3c24xx_get_cpu_name(),
cpuid, strmhz(buf, get_ARMCLK()));
- for (i = 0; i < ARRAY_SIZE(freq_f); i++)
printf("%cCLK: %8s MHz\n", freq_c[i],
strmhz(buf, freq_f[i] ()));
- return 0;
+}
[...]
+ulong get_HCLK(void) +{
- struct s3c2412_sysctl *const sysctl = s3c2412_get_base_sysctl();
- u32 clkdivn;
- u16 hclk_div, arm_div;
- clkdivn = readl(&sysctl->clkdivn);
- hclk_div = (clkdivn & 0x3) + 1;
- arm_div = ((clkdivn >> 3) & 0x1) + 1;
Magic.
Missed that... I will fix it.
There's more, globally please.
[...]
diff --git a/include/common.h b/include/common.h index 55025c0..36f0636 100644 --- a/include/common.h +++ b/include/common.h @@ -628,6 +628,7 @@ ulong get_OPB_freq (void);
ulong get_PCI_freq (void); #endif #if defined(CONFIG_S3C24X0) || \
defined(CONFIG_S3C24XX) || \
defined(CONFIG_LH7A40X) || \ defined(CONFIG_S3C6400) || \ defined(CONFIG_EP93XX)
What's this change? Split into different patch please.
This is related. I need this to export the functions get_FCLK(), get_HCLK(), get_PCLK() and get_UCLK() used in arch/arm/cpu/arm926ejs/s3c24xx/cpu_info.c
OK
Best regards, José Gonçalves

On Fri, Sep 14, 2012 at 08:25:25PM +0200, Marek Vasut wrote:
Dear José Miguel Gonçalves,
Hi Marek,
On 14-09-2012 19:03, Marek Vasut wrote:
Dear José Miguel Gonçalves,
It's getting better :)
Hopefully :-)
[...]
+typedef ulong(*getfreq) (void);
Is this used?
In the array declaration bellow...
Why, these are only values, no ?
They're function pointers. If they were values the compiler should complain, because "getfreq" is used as the type of the array.
+static const getfreq freq_f[] = {
const array const members, no?
Do you mean I should declare it like this:
static const getfreq const freq[] = { ...
Yes
Why? When can you ever change what an array (not a pointer) points to?
I don't see the point because an array has no other storage besides it's elements. Moreover GCC generates the same object code in both ways.
Type checking, if you ever decided to write into the array, it'll prevent you from doing so.
The first const takes care of that.
-Scott

Dear Scott Wood,
On Fri, Sep 14, 2012 at 08:25:25PM +0200, Marek Vasut wrote:
Dear José Miguel Gonçalves,
Hi Marek,
On 14-09-2012 19:03, Marek Vasut wrote:
Dear José Miguel Gonçalves,
It's getting better :)
Hopefully :-)
[...]
+typedef ulong(*getfreq) (void);
Is this used?
In the array declaration bellow...
Why, these are only values, no ?
They're function pointers. If they were values the compiler should complain, because "getfreq" is used as the type of the array.
+static const getfreq freq_f[] = {
const array const members, no?
Do you mean I should declare it like this:
static const getfreq const freq[] = { ...
Yes
Why? When can you ever change what an array (not a pointer) points to?
Uh oh ... now I see the stuff with the functions. Crazy
I don't see the point because an array has no other storage besides it's elements. Moreover GCC generates the same object code in both ways.
Type checking, if you ever decided to write into the array, it'll prevent you from doing so.
The first const takes care of that.
Doesn't one prevent you from manupulating the elements and the other manipulating the array ?
-Scott
Best regards, Marek Vasut

On Fri, Sep 14, 2012 at 09:07:12PM +0200, Marek Vasut wrote:
Dear Scott Wood,
On Fri, Sep 14, 2012 at 08:25:25PM +0200, Marek Vasut wrote:
Dear José Miguel Gonçalves,
Hi Marek,
On 14-09-2012 19:03, Marek Vasut wrote:
Dear José Miguel Gonçalves,
It's getting better :)
Hopefully :-)
[...]
+typedef ulong(*getfreq) (void);
Is this used?
In the array declaration bellow...
Why, these are only values, no ?
They're function pointers. If they were values the compiler should complain, because "getfreq" is used as the type of the array.
+static const getfreq freq_f[] = {
const array const members, no?
Do you mean I should declare it like this:
static const getfreq const freq[] = { ...
Yes
Why? When can you ever change what an array (not a pointer) points to?
Uh oh ... now I see the stuff with the functions. Crazy
I don't see the point because an array has no other storage besides it's elements. Moreover GCC generates the same object code in both ways.
Type checking, if you ever decided to write into the array, it'll prevent you from doing so.
The first const takes care of that.
Doesn't one prevent you from manupulating the elements and the other manipulating the array ?
No. These are all identical:
const int foo[]; int const foo[]; const int const foo[]; const const const int const const const foo[];
If it were a pointer, then there would be a difference between these:
const int *foo; (pointed-to data is const) int *const foo; (pointer itself is const) const int *const foo; (both are const)
...but still there's no distinction between these:
const int *foo; const int const *foo;
-Scott
participants (2)
-
Marek Vasut
-
Scott Wood