Re: [U-Boot] Struct SoC access
Dear Scott Wood, >> Then assign struct soc *soc = (struct soc *)0; > > One snag you might hit is that dereferencing a NULL pointer is undefined, > and some versions of GCC assume you won't do this when optimizing. Not sure > if this simple usage would be affected (it seems to mainly be an issue when > comparing a pointer to NULL after dereferencing), and > -fno-delete-null-pointer-checks may help. Its just an idea anyway... Certainly the 1st way I proposed is the easiest to go: #define BLOCK_BASE_ADDR 0xsomething block_t *block = (block_t *)BLOCK_BASE_ADDR; >> Whats a IOCCC? > > The International Obfuscated C Code Contest, possibly a more appropriate > venue for code that defines a 4GB struct. :-) In that case I am quite sure the current AT91 way of defining the hardware addresses to the drivers already qualifies for that ;) (Try to follow the definition of SPI0_BASE...) arch-at91/memory_map.h: ... #ifndef __ASM_ARM_ARCH_MEMORYMAP_H__ #define __ASM_ARM_ARCH_MEMORYMAP_H__ #include #define USART0_BASE AT91_USART0 #define USART1_BASE AT91_USART1 #define USART2_BASE AT91_USART2 #define USART3_BASE (AT91_BASE_SYS + AT91_DBGU) #define SPI0_BASE AT91_BASE_SPI #define SPI1_BASE AT91_BASE_SPI1 #endif /* __ASM_ARM_ARCH_MEMORYMAP_H__ */ ... arch-at91/harware.h: ... #if defined(CONFIG_AT91RM9200) #include #elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20) #include #define AT91_BASE_SPI AT91SAM9260_BASE_SPI0 #define AT91_BASE_SPI1 AT91SAM9260_BASE_SPI1 #define AT91_ID_UHP AT91SAM9260_ID_UHP #define AT91_PMC_UHPAT91SAM926x_PMC_UHP #define AT91_BASE_MMCI AT91SAM9260_BASE_MCI #elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9G10) #include #define AT91_BASE_SPI AT91SAM9261_BASE_SPI0 #define AT91_ID_UHP AT91SAM9261_ID_UHP #define AT91_PMC_UHPAT91SAM926x_PMC_UHP #elif defined(CONFIG_AT91SAM9263) ... arch-at91/at91sam9260.h: ... #ifdef CONFIG_AT91_LEGACY /* * User Peripheral physical base addresses. */ #define AT91SAM9260_BASE_TCB0 0xfffa #define AT91SAM9260_BASE_TC00xfffa #define AT91SAM9260_BASE_TC10xfffa0040 #define AT91SAM9260_BASE_TC20xfffa0080 #define AT91SAM9260_BASE_UDP0xfffa4000 #define AT91SAM9260_BASE_MCI0xfffa8000 #define AT91SAM9260_BASE_TWI0xfffac000 #define AT91SAM9260_BASE_US00xfffb #define AT91SAM9260_BASE_US10xfffb4000 #define AT91SAM9260_BASE_US20xfffb8000 #define AT91SAM9260_BASE_SSC0xfffbc000 #define AT91SAM9260_BASE_ISI0xfffc #define AT91SAM9260_BASE_EMAC 0xfffc4000 #define AT91SAM9260_BASE_SPI0 0xfffc8000 #define AT91SAM9260_BASE_SPI1 0xfffcc000 #define AT91SAM9260_BASE_US30xfffd #define AT91SAM9260_BASE_US40xfffd4000 #define AT91SAM9260_BASE_US50xfffd8000 #define AT91SAM9260_BASE_TCB1 0xfffdc000 #define AT91SAM9260_BASE_TC30xfffdc000 #define AT91SAM9260_BASE_TC40xfffdc040 #define AT91SAM9260_BASE_TC50xfffdc080 #define AT91SAM9260_BASE_ADC0xfffe #define AT91_BASE_SYS 0xe800 ... arch-at91/at91sam9261.h: ... #ifdef CONFIG_AT91_LEGACY /* * User Peripheral physical base addresses. */ #define AT91SAM9261_BASE_TCB0 0xfffa #define AT91SAM9261_BASE_TC00xfffa #define AT91SAM9261_BASE_TC10xfffa0040 #define AT91SAM9261_BASE_TC20xfffa0080 #define AT91SAM9261_BASE_UDP0xfffa4000 #define AT91SAM9261_BASE_MCI0xfffa8000 #define AT91SAM9261_BASE_TWI0xfffac000 #define AT91SAM9261_BASE_US00xfffb #define AT91SAM9261_BASE_US10xfffb4000 #define AT91SAM9261_BASE_US20xfffb8000 #define AT91SAM9261_BASE_SSC0 0xfffbc000 #define AT91SAM9261_BASE_SSC1 0xfffc #define AT91SAM9261_BASE_SSC2 0xfffc4000 #define AT91SAM9261_BASE_SPI0 0xfffc8000 #define AT91SAM9261_BASE_SPI1 0xfffcc000 #define AT91_BASE_SYS 0xea00 ... isn't that obfuscated enough? Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
On Sun, Aug 15, 2010 at 12:15:29AM +0200, Reinhard Meyer wrote: > >> Would the toolchain "gulp" when one defines the whole 4 GB that way? > > > > Why not? > > Since the AT91s have no base address registers at all, the memory layout > is completely fixed. Even chip selects have a fixed address and fixed > size of 256MB each. Therefore it _might_ make sense to completely > define the 4GB in the soc struct. > Then assign struct soc *soc = (struct soc *)0; One snag you might hit is that dereferencing a NULL pointer is undefined, and some versions of GCC assume you won't do this when optimizing. Not sure if this simple usage would be affected (it seems to mainly be an issue when comparing a pointer to NULL after dereferencing), and -fno-delete-null-pointer-checks may help. > Did you read Mike's comment of hardware dependent direct usage of > struct members to access hardware? I thought that was generally > discouraged for several reasons (cache and access sequencing)? > > Whats a IOCCC? The International Obfuscated C Code Contest, possibly a more appropriate venue for code that defines a 4GB struct. :-) -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Wolfgang Denk, > I think we have a misunderstaning ehre - I thought the entries like > "xyz" were indeed "u32" types - buut now I get the impression that > what you have in mind is that they are actually structs describing > hardware blocks. No, thats just spacemakers, otherwise one would have do declare all structs for all hardware blocks in that file (or include a bunch of files). > struct immap is what corresponds to your struct soc above. Yes, with at least all currently used blocks declared as structs, the currently unused ones made up of the proper amount of u32s Since different AT91 SoC might have some blocks missing or at other addresses in another amount etc., but with the same layout of each individual block, it would make sense to keep the block structure definitions in separate files like at91_dbu.h, at91_rstc.h, etc. They would have to be included in the corresponding at91sam9260.h, at91sam9261.h, etc. where the individual soc layouts would be defined. >> Would the toolchain "gulp" when one defines the whole 4 GB that way? > > Why not? Since the AT91s have no base address registers at all, the memory layout is completely fixed. Even chip selects have a fixed address and fixed size of 256MB each. Therefore it _might_ make sense to completely define the 4GB in the soc struct. Then assign struct soc *soc = (struct soc *)0; Did you read Mike's comment of hardware dependent direct usage of struct members to access hardware? I thought that was generally discouraged for several reasons (cache and access sequencing)? Whats a IOCCC? Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Reinhard Meyer, In message <4c6700e5.2070...@emk-elektronik.de> you wrote: > > Would the toolchain "gulp" when one defines the whole 4 GB that way? > > In fact, a rather novel approach (just theorizing here): > > #define SRAM_BASE offsetof(soc.sram) > #define SRAM_SIZE sizeof(soc.sram) > > dbu_t *dbu = (dbu_t *)offsetof(soc.dbu); > > without ever assigning soc the address 0... Urgh... that's a log of pretty heavy assumptions, and not exactly readable / understandable code either. I gues your're not targeting the next IOCCC? 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: w...@denx.de Die Scheu vor Verantwortung ist die Krankheit unserer Zeit. -- Otto von Bismarck ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Reinhard Meyer,
In message <4c66f54d.2060...@emk-elektronik.de> you wrote:
>
> >> I was even thinking of something like
> >>
> >> struct soc {
> >>u32 xyz[0x80]; /* XYZ unit */
> >>u32 dbu[0x80]; /* Debug Unit */
> >>u32 rstc[0x80]; /* Reset Controller */
> >> and so on.
> >
> > This is what PPC used to do; I like that - but ARM people always
> > explained to me that it makes no sense because address space on ARM
> > SoC is only sparely populated.
>
> Even if, that's no reason, on can write "u32 spi0[0x1000]", on AT91
> the spacing of peripherals is 0x4000 bytes, in fact it repeats
> times in its window. The system stuff is like one peripheral with
> its components spaced by 0x200 bytes (hence the 0x80 above).
I think we have a misunderstaning ehre - I thought the entries like
"xyz" were indeed "u32" types - buut now I get the impression that
what you have in mind is that they are actually structs describing
hardware blocks.
Then it should be written like that.
For example, see file "arch/powerpc/include/asm/8xx_immap.h":
struct immap is what corresponds to your struct soc above.
> Would the toolchain "gulp" when one defines the whole 4 GB that way?
Why not?
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: w...@denx.de
It [being a Vulcan] means to adopt a philosophy, a way of life which
is logical and beneficial. We cannot disregard that philosophy merely
for personal gain, no matter how important that gain might be.
-- Spock, "Journey to Babel", stardate 3842.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
On Sat, Aug 14, 2010 at 3:40 PM, Reinhard Meyer wrote: > Dear Mike Frysinger, >> On Sat, Aug 14, 2010 at 3:30 PM, Reinhard Meyer wrote: >>> #define DBU_ADDR 0xsomething (in a SoC header file) >>> >>> dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) >> >> needs to be volatile ... > > Why? The elements are used as parameters to readl/writel functions: > status = readl(&dbu->sr); if you use accessor functions and those guarantee volatility, then you dont need the markings on the pointer > I am quite sure we are NOT supposed to directly access hardware: > status = dbu->sr; it depends on the hardware -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
> Would the toolchain "gulp" when one defines the whole 4 GB that way? In fact, a rather novel approach (just theorizing here): #define SRAM_BASE offsetof(soc.sram) #define SRAM_SIZE sizeof(soc.sram) dbu_t *dbu = (dbu_t *)offsetof(soc.dbu); without ever assigning soc the address 0... Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Wolfgang Denk,
> Then just write in the comment part of the second patch that the other
> one has to be applied first...
Thats fine with me.
>> Anyway, is the method of (for example!)
>>
>> #define DBU_ADDR 0xsomething (in a SoC header file)
>>
>> dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function)
>>
>> OK?
>
> Yes.
Thats the way I do it currently
>> Or do we need to further encapsulate that in a function like
>
> No.
I have seen that somewhere in u-boot.
>> I was even thinking of something like
>>
>> struct soc {
>> u32 xyz[0x80]; /* XYZ unit */
>> u32 dbu[0x80]; /* Debug Unit */
>> u32 rstc[0x80]; /* Reset Controller */
>> and so on.
>
> This is what PPC used to do; I like that - but ARM people always
> explained to me that it makes no sense because address space on ARM
> SoC is only sparely populated.
Even if, that's no reason, on can write "u32 spi0[0x1000]", on AT91
the spacing of peripherals is 0x4000 bytes, in fact it repeats
times in its window. The system stuff is like one peripheral with
its components spaced by 0x200 bytes (hence the 0x80 above).
> I think this looks nice, but as mentioned before - I'm not an ARM
> expert. They tend to do it differently.
I can be done for AT91, but I'm not Don Quichote ;)
Would the toolchain "gulp" when one defines the whole 4 GB that way?
Best Regards,
Reinhard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access (was:Make preparatory patches that initially have no effect?)
Dear Reinhard Meyer,
In message <4c66eeca.5020...@emk-elektronik.de> you wrote:
>
> > Have the first add that file, and the second assume it comes later in
> > the sequence.
>
> You don't mean by "sequence" PATCH 1/n, 2/n, etc? The drivers are so
> independent that that would not really make sense...
Then just write in the comment part of the second patch that the other
one has to be applied first...
> That's a thin line. Although I need only one register of the DBU (for
> example) I think its wise to define all registers in it, and not to
> _reserve[] the unused ones
Right. If you add a function, add all the registers in it.
But don't bother to explain each and every bit in the registers you
never refer to, nor add completely unrelated blocks.
> Anyway, is the method of (for example!)
>
> #define DBU_ADDR 0xsomething (in a SoC header file)
>
> dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function)
>
> OK?
Yes.
> Or do we need to further encapsulate that in a function like
No.
> I was even thinking of something like
>
> struct soc {
> u32 xyz[0x80]; /* XYZ unit */
> u32 dbu[0x80]; /* Debug Unit */
> u32 rstc[0x80]; /* Reset Controller */
> and so on.
This is what PPC used to do; I like that - but ARM people always
explained to me that it makes no sense because address space on ARM
SoC is only sparely populated.
> Then in a driver one could write
> dbu_t *dbu = (dbu_t *)soc.dbu;
> or something along that line
I think this looks nice, but as mentioned before - I'm not an ARM
expert. They tend to do it differently.
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: w...@denx.de
Q: How many DEC repairman does it take to fix a flat ?
A: Five; four to hold the car up and one to swap tires.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access
Dear Mike Frysinger, > On Sat, Aug 14, 2010 at 3:30 PM, Reinhard Meyer wrote: >> #define DBU_ADDR 0xsomething (in a SoC header file) >> >> dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) > > needs to be volatile ... > -mike Why? The elements are used as parameters to readl/writel functions: status = readl(&dbu->sr); I am quite sure we are NOT supposed to directly access hardware: status = dbu->sr; Best Regards, Reinhard ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access (was:Make preparatory patches that initially have no effect?)
On Sat, Aug 14, 2010 at 3:30 PM, Reinhard Meyer wrote: > #define DBU_ADDR 0xsomething (in a SoC header file) > > dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function) needs to be volatile ... -mike ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] Struct SoC access (was:Make preparatory patches that initially have no effect?)
Dear Wolfgang Denk,
> Have the first add that file, and the second assume it comes later in
> the sequence.
You don't mean by "sequence" PATCH 1/n, 2/n, etc? The drivers are so
independent that that would not really make sense...
> The wiki page does not talk about drivers... It's a general rule and
> applies to all sorts of code. Only add what is really used (this also
> refers, for example, to struct definitions for register blocks etc. -
> don't try to provide a complete description of your SoC; add only
> stuff that is actually used by the code).
That's a thin line. Although I need only one register of the DBU (for
example) I think its wise to define all registers in it, and not to
_reserve[] the unused ones
Anyway, is the method of (for example!)
#define DBU_ADDR 0xsomething (in a SoC header file)
dbu_t *dbu = (dbu_t *)DBU_ADDR; (in a function)
OK?
Or do we need to further encapsulate that in a function like
dbut_t *get_dbu_addr(void) {return (dbu_t *)DBU_ADDR;}
I was even thinking of something like
struct soc {
u32 xyz[0x80]; /* XYZ unit */
u32 dbu[0x80]; /* Debug Unit */
u32 rstc[0x80]; /* Reset Controller */
and so on.
Then in a driver one could write
dbu_t *dbu = (dbu_t *)soc.dbu;
or something along that line
Best Regards
Reinhard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
