Re: [U-Boot] [PATCH 1/4] gpio: Adds GPIO driver support for Armada100
----- "Prafulla Wadaskar" prafulla@marvell.com wrote:
That can be thought of while adding support for othe SoCs. Preferably define register struct in asm/arch/gpio.h
Regards.. Prafulla . .
..snip.. (quoting from another reply..)
You have to follow all :-), more reviewers more better code output. BASE+OFFSET strictly not recommended.
I think lei and me are suggesting similar things, macros should be used precisely, the code should be small and smarter.
Hi Prafulla,
I agree that macros make code look smaller and smarter. Now if you see the registers of GPIO they are not in order, I mean i cannot group together particular gpio set. can i do it this way,
e.g. struct armdgpio_gplr_register { u32 gplr0; u32 gplr1; u32 gplr2; u8 pad[some_value]; //this padding is going to be big u32 gplr3; }
then while using this particular set i can just use ARMD1_GPLR_BASE ( = ARMD1_GPIO_BASE + GPLR_OFFSET). moreover i am not using all the registers so i define only those register sets which are in use. what you say about this?
Regards, Ajay Bhargav
Dear Ajay Bhargav,
In message 1207509190.33599.1311140198703.JavaMail.root@ahm.einfochips.com you wrote:
e.g. struct armdgpio_gplr_register { u32 gplr0; u32 gplr1; u32 gplr2; u8 pad[some_value]; //this padding is going to be big u32 gplr3; }
Is there any specific reason for not using u32 for the padding as well?
then while using this particular set i can just use ARMD1_GPLR_BASE ( ARMD1_GPIO_BASE + GPLR_OFFSET). moreover i am not using all the registers so i define only those register sets which are in use. what you say about this?
Why would you need this BASE + OFFSET notation when using a C struct for the registers? Thi smakes little sense to me.
Best regards,
Wolfgang Denk
Dear Wolfgang,
Is there any specific reason for not using u32 for the padding as well?
nothing specific. It makes easy to find number of bytes than words.
Why would you need this BASE + OFFSET notation when using a C struct for the registers? Thi smakes little sense to me.
Well I did use the C struct method, if you see my patches submitted earlier but according to Prafulla and Lie structure size is too big. so they want me to use a mix of C struct and BASE + OFFSET notation. so I thought to break the big C struct into smaller grouped structures pointing each group with GPIO base + group offset. I would be glad if you can suggest me something better or smarter.
Regards, Ajay Bhargav
Hi Ajay,
On Wed, Jul 20, 2011 at 2:36 PM, Ajay Bhargav ajay.bhargav@einfochips.com wrote:
Dear Wolfgang,
Is there any specific reason for not using u32 for the padding as well?
nothing specific. It makes easy to find number of bytes than words.
Why would you need this BASE + OFFSET notation when using a C struct for the registers? Thi smakes little sense to me.
Well I did use the C struct method, if you see my patches submitted earlier but according to Prafulla and Lie structure size is too big. so they want me to use a mix of C struct and BASE + OFFSET notation. so I thought to break the big C struct into smaller grouped structures pointing each group with GPIO base + group offset. I would be glad if you can suggest me something better or smarter.
I think we make thing complicated here. For GPIO driver, the only structure we need to define is the GPIO register itself, like GPIO_PLR, GPIO_PDR, etc...
For the previous long huge structure and include your recent short version is still not good enough. What we want to get is the base address, and based on the register structure to do explicit work. So either get the base address based on MACRO, or get from a function I think both should be ok for me. But please not continue work on how to define the base address into a structure.
Best regards, Lei
Hi Lei,
I think we make thing complicated here. For GPIO driver, the only structure we need to define is the GPIO register itself, like GPIO_PLR, GPIO_PDR, etc...
I got your point, but lemme show you where the problem is.. GPIO_PLR0 0x0000 GPIO_PLR1 0x0004 GPIO_PLR2 0x0008 GPIO_PLR3 0x0100
till 3 registers its fine... fourth register is way out of league.
Regards, Ajay Bhargav
Hi Ajay,
On Wed, Jul 20, 2011 at 6:43 PM, Ajay Bhargav ajay.bhargav@einfochips.com wrote:
Hi Lei,
I think we make thing complicated here. For GPIO driver, the only structure we need to define is the GPIO register itself, like GPIO_PLR, GPIO_PDR, etc...
I got your point, but lemme show you where the problem is.. GPIO_PLR0 0x0000 GPIO_PLR1 0x0004 GPIO_PLR2 0x0008 GPIO_PLR3 0x0100
till 3 registers its fine... fourth register is way out of league.
I know this is a bit tricky, but couldn't be included in a logic hidden by MACRO or funcion?
Best regards, Lei
----- "Lei Wen" adrian.wenl@gmail.com wrote:
Hi Ajay,
On Wed, Jul 20, 2011 at 6:43 PM, Ajay Bhargav ajay.bhargav@einfochips.com wrote:
Hi Lei,
I think we make thing complicated here. For GPIO driver, the only structure we need to define is the GPIO register itself, like GPIO_PLR, GPIO_PDR,
etc...
I got your point, but lemme show you where the problem is.. GPIO_PLR0 0x0000 GPIO_PLR1 0x0004 GPIO_PLR2 0x0008 GPIO_PLR3 0x0100
till 3 registers its fine... fourth register is way out of league.
I know this is a bit tricky, but couldn't be included in a logic hidden by MACRO or funcion?
Best regards, Lei
Hi Lei,
I am rewriting the whole logic. Will submit the patch soon :) I will use function to get base address.
Regards, Ajay Bhargav
Dear Ajay Bhargav,
In message 1499501074.35394.1311158638174.JavaMail.root@ahm.einfochips.com you wrote:
Hi Lei,
I think we make thing complicated here. For GPIO driver, the only structure we need to define is the GPIO register itself, like GPIO_PLR, GPIO_PDR, etc...
I got your point, but lemme show you where the problem is.. GPIO_PLR0 0x0000 GPIO_PLR1 0x0004 GPIO_PLR2 0x0008 GPIO_PLR3 0x0100
till 3 registers its fine... fourth register is way out of league.
In which way? Just because there is a gap between? No problem. Insert a filler element to your struct.
Best regards,
Wolfgang Denk
Dear Ajay Bhargav,
In message 794341286.33968.1311143785027.JavaMail.root@ahm.einfochips.com you wrote:
Is there any specific reason for not using u32 for the padding as well?
nothing specific. It makes easy to find number of bytes than words.
If there is no specific reason (which I could not think of either), then please use u32 consistently.
If you think in number of bytes, feel free to write
u32 reserved[128/sizeof(u32)];
Why would you need this BASE + OFFSET notation when using a C struct for the registers? Thi smakes little sense to me.
Well I did use the C struct method, if you see my patches submitted earlier but according to Prafulla and Lie structure size is too big. so they want me
Who cares about the size of the struct? We never alocate any memory for such a structure - it is just an overlay over the existing register space, so nobody cares if this is 16 kB of 16 MB.
to use a mix of C struct and BASE + OFFSET notation. so I thought to break the big C struct into smaller grouped structures pointing each group with GPIO base + group offset. I would be glad if you can suggest me something better or smarter.
This may make sense of you have separate, logically independent IP blocks. But the reason for such a desicion is if the data belong together in any way or not.
The "size" of such a structure is completely irrelevant.
Best regards,
Wolfgang Denk
participants (3)
-
Ajay Bhargav -
Lei Wen -
Wolfgang Denk