From: Eric Nelson <eric.nelson@boundarydevices.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] i.MX6: imx_ccm is a constant that points to a register set
Date: Tue, 06 Mar 2012 07:34:29 -0700 [thread overview]
Message-ID: <4F562075.6070907@boundarydevices.com> (raw)
In-Reply-To: <4F5601B2.9020202@denx.de>
On 03/06/2012 05:23 AM, Stefano Babic wrote:
> On 05/03/2012 17:34, Eric Nelson wrote:
>
> Hi Eric,
>
>> If we're going to use globals to point at register banks, they
>> should be constant and visible.
>> ---
>
> ok, but why are we going to use global pointers ? We have global
> constants (in imx-regs.h) and each part of code sets its local copy
> without using global pointers, that require also some rules and naming
> conventions to avoid conflicts.
>
I'm just trying to follow convention here. My preference would be
to make these constants visible to the compiler in some way.
>> arch/arm/cpu/armv7/mx6/clock.c | 2 +-
>> arch/arm/include/asm/arch-mx6/imx-regs.h | 2 ++
>> 2 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
>> index ef98563..eb1f09b 100644
>> --- a/arch/arm/cpu/armv7/mx6/clock.c
>> +++ b/arch/arm/cpu/armv7/mx6/clock.c
>> @@ -34,7 +34,7 @@ enum pll_clocks {
>> PLL_ENET, /* ENET PLL */
>> };
>>
>> -struct imx_ccm_reg *imx_ccm = (struct imx_ccm_reg *)CCM_BASE_ADDR;
>> +struct imx_ccm_reg *const imx_ccm = (struct imx_ccm_reg *)CCM_BASE_ADDR;
>
> As far as I see, this is used only in clock.c - and it must be static.
>
I ran into this when trying to fix setup_sata() in mx6qsabrelite.c as shown in
this patch.
The same sort of question (how to define the register sets) will occur for
other register sets.
For the iomux, I used a #define, but that seems wrong too
(not least because it isn't UPPERCASE).
#define iomuxc ((struct iomuxc_base_regs *)IOMUXC_BASE_ADDR)
The other way around this is to hide all register accesses behind the
drivers. IOW, add routines for each of the accesses and hide the
details in clock.c
void mx6_enable_sata_clock();
...
void mx6_enable_enet_pll();
but there will almost certainly be board-specific tweaks and I'm not
sure the extra level of abstraction helps.
Let me know your thoughts about how to structure this stuff.
Regards,
Eric
next prev parent reply other threads:[~2012-03-06 14:34 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-05 16:34 [U-Boot] [PATCH] i.MX6: imx_ccm is a constant that points to a register set Eric Nelson
2012-03-06 12:23 ` Stefano Babic
2012-03-06 14:34 ` Eric Nelson [this message]
2012-03-07 10:43 ` Stefano Babic
2012-03-07 15:54 ` Eric Nelson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F562075.6070907@boundarydevices.com \
--to=eric.nelson@boundarydevices.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox