diff mbox

[U-Boot] driver/mxc_i2c: Move static data structure to global_data

Message ID 1392069772-24742-1-git-send-email-yorksun@freescale.com
State Accepted
Delegated to: Heiko Schocher
Headers show

Commit Message

York Sun Feb. 10, 2014, 10:02 p.m. UTC
This driver needs a data structure in SRAM before SDRAM is available.
This is not alway the case using .data section. Moving this data
structure to global_data guarantees it is writable.

Signed-off-by: York Sun <yorksun@freescale.com>
CC: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c             |   18 ++++++++----------
 include/asm-generic/global_data.h |    3 +++
 2 files changed, 11 insertions(+), 10 deletions(-)

Comments

Tom Rini Feb. 10, 2014, 10:10 p.m. UTC | #1
On Mon, Feb 10, 2014 at 02:02:52PM -0800, York Sun wrote:

> This driver needs a data structure in SRAM before SDRAM is available.
> This is not alway the case using .data section. Moving this data
> structure to global_data guarantees it is writable.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>
> CC: Troy Kisky <troy.kisky@boundarydevices.com>

If you need something in SRAM then you need to place it in that section,
see arch/arm/cpu/armv7/am33xx/u-boot-spl.lds for example
York Sun Feb. 10, 2014, 10:28 p.m. UTC | #2
On 02/10/2014 02:10 PM, Tom Rini wrote:
> On Mon, Feb 10, 2014 at 02:02:52PM -0800, York Sun wrote:
> 
>> This driver needs a data structure in SRAM before SDRAM is available.
>> This is not alway the case using .data section. Moving this data
>> structure to global_data guarantees it is writable.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
>> CC: Troy Kisky <troy.kisky@boundarydevices.com>
> 
> If you need something in SRAM then you need to place it in that section,
> see arch/arm/cpu/armv7/am33xx/u-boot-spl.lds for example
> 

I am not sure if it is a similar situation. But anyway, I am open to suggestions.

For this driver, the variable needs to be writable. I guess it wasn't a problem
for existing platforms which probably call this driver after relocation.

Does the SRAM code/data relocate to SDRAM? I don't want this variable to stay in
SRAM once u-boot relocates to normal memory.

York
Tom Rini Feb. 10, 2014, 10:45 p.m. UTC | #3
On Mon, Feb 10, 2014 at 02:28:01PM -0800, York Sun wrote:
> On 02/10/2014 02:10 PM, Tom Rini wrote:
> > On Mon, Feb 10, 2014 at 02:02:52PM -0800, York Sun wrote:
> > 
> >> This driver needs a data structure in SRAM before SDRAM is available.
> >> This is not alway the case using .data section. Moving this data
> >> structure to global_data guarantees it is writable.
> >>
> >> Signed-off-by: York Sun <yorksun@freescale.com>
> >> CC: Troy Kisky <troy.kisky@boundarydevices.com>
> > 
> > If you need something in SRAM then you need to place it in that section,
> > see arch/arm/cpu/armv7/am33xx/u-boot-spl.lds for example
> > 
> 
> I am not sure if it is a similar situation. But anyway, I am open to
> suggestions.
> 
> For this driver, the variable needs to be writable. I guess it wasn't
> a problem for existing platforms which probably call this driver after
> relocation.
> 
> Does the SRAM code/data relocate to SDRAM? I don't want this variable
> to stay in SRAM once u-boot relocates to normal memory.

So in this case the problem is still within SPL right?  You would put
the parts you need to have in SRAM during SPL and DDR in full U-Boot
with __attribute__((section(".data.srdata"))) and in the SoC's linker
scripts make sure that drivers/i2c/built-in.o(.data.srdata) ends up in
the appropriate place for each case.

I say all of this as I think we really want to avoid expanding on
gd->arch-> stuff if we can (which reminds me, your patch expands on main
gd not gd->arch so NAK on that either way).
York Sun Feb. 10, 2014, 10:47 p.m. UTC | #4
On 02/10/2014 02:45 PM, Tom Rini wrote:
> On Mon, Feb 10, 2014 at 02:28:01PM -0800, York Sun wrote:
>> On 02/10/2014 02:10 PM, Tom Rini wrote:
>>> On Mon, Feb 10, 2014 at 02:02:52PM -0800, York Sun wrote:
>>>
>>>> This driver needs a data structure in SRAM before SDRAM is available.
>>>> This is not alway the case using .data section. Moving this data
>>>> structure to global_data guarantees it is writable.
>>>>
>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>> CC: Troy Kisky <troy.kisky@boundarydevices.com>
>>>
>>> If you need something in SRAM then you need to place it in that section,
>>> see arch/arm/cpu/armv7/am33xx/u-boot-spl.lds for example
>>>
>>
>> I am not sure if it is a similar situation. But anyway, I am open to
>> suggestions.
>>
>> For this driver, the variable needs to be writable. I guess it wasn't
>> a problem for existing platforms which probably call this driver after
>> relocation.
>>
>> Does the SRAM code/data relocate to SDRAM? I don't want this variable
>> to stay in SRAM once u-boot relocates to normal memory.
> 
> So in this case the problem is still within SPL right?  You would put
> the parts you need to have in SRAM during SPL and DDR in full U-Boot
> with __attribute__((section(".data.srdata"))) and in the SoC's linker
> scripts make sure that drivers/i2c/built-in.o(.data.srdata) ends up in
> the appropriate place for each case.
> 
> I say all of this as I think we really want to avoid expanding on
> gd->arch-> stuff if we can (which reminds me, your patch expands on main
> gd not gd->arch so NAK on that either way).
> 

I didn't like this change either. Let me explore the linker script to see if I
can do it the other way.

York
York Sun Feb. 11, 2014, 2:34 a.m. UTC | #5
On 02/10/2014 02:02 PM, York Sun wrote:
> This driver needs a data structure in SRAM before SDRAM is available.
> This is not alway the case using .data section. Moving this data
> structure to global_data guarantees it is writable.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>
> CC: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  drivers/i2c/mxc_i2c.c             |   18 ++++++++----------
>  include/asm-generic/global_data.h |    3 +++
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 

Troy,

Following Tom's suggestion, I am trying to use linker script to put the srdata
into SRAM. But I still have a concern regarding initializing the srdata. I don't
see it is initialized anywhere. Do you presume the data is wiped out before the
driver runs? If that's the case, I need to clear the data somewhere in my code.

York
York Sun Feb. 11, 2014, 6:01 p.m. UTC | #6
On 02/10/2014 02:45 PM, Tom Rini wrote:
> On Mon, Feb 10, 2014 at 02:28:01PM -0800, York Sun wrote:
>> On 02/10/2014 02:10 PM, Tom Rini wrote:
>>> On Mon, Feb 10, 2014 at 02:02:52PM -0800, York Sun wrote:
>>>
>>>> This driver needs a data structure in SRAM before SDRAM is available.
>>>> This is not alway the case using .data section. Moving this data
>>>> structure to global_data guarantees it is writable.
>>>>
>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>> CC: Troy Kisky <troy.kisky@boundarydevices.com>
>>>
>>> If you need something in SRAM then you need to place it in that section,
>>> see arch/arm/cpu/armv7/am33xx/u-boot-spl.lds for example
>>>
>>
>> I am not sure if it is a similar situation. But anyway, I am open to
>> suggestions.
>>
>> For this driver, the variable needs to be writable. I guess it wasn't
>> a problem for existing platforms which probably call this driver after
>> relocation.
>>
>> Does the SRAM code/data relocate to SDRAM? I don't want this variable
>> to stay in SRAM once u-boot relocates to normal memory.
> 
> So in this case the problem is still within SPL right?  You would put
> the parts you need to have in SRAM during SPL and DDR in full U-Boot
> with __attribute__((section(".data.srdata"))) and in the SoC's linker
> scripts make sure that drivers/i2c/built-in.o(.data.srdata) ends up in
> the appropriate place for each case.
> 
> I say all of this as I think we really want to avoid expanding on
> gd->arch-> stuff if we can (which reminds me, your patch expands on main
> gd not gd->arch so NAK on that either way).
> 

I tried the linker script and made it work. But it is not what I want. I expect
this variable relocates to DDR, but the offset is not right.
I am not using SPL. This is full u-boot, which boots from flash, calls i2c
driver, initializes DDR, then relocates. Putting this variable into stack might
be the solution. Suggestions?

York
Troy Kisky Feb. 11, 2014, 7:25 p.m. UTC | #7
On 2/10/2014 7:34 PM, York Sun wrote:
> On 02/10/2014 02:02 PM, York Sun wrote:
>> This driver needs a data structure in SRAM before SDRAM is available.
>> This is not alway the case using .data section. Moving this data
>> structure to global_data guarantees it is writable.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
>> CC: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
>>   drivers/i2c/mxc_i2c.c             |   18 ++++++++----------
>>   include/asm-generic/global_data.h |    3 +++
>>   2 files changed, 11 insertions(+), 10 deletions(-)
>>
> Troy,
>
> Following Tom's suggestion, I am trying to use linker script to put the srdata
> into SRAM. But I still have a concern regarding initializing the srdata. I don't
> see it is initialized anywhere. Do you presume the data is wiped out before the
> driver runs? If that's the case, I need to clear the data somewhere in my code.
>
> York
>
> .
>
As I understand it, the .data section follows the .text section and is 
relocated with it. Thus,
if your program is loaded into sram, the .data will be there as well. 
The .data section
also contains the initial value, so no need for code to initialize it. 
If later your code
is relocated to sdram, the .data section should be relocated as well. 
But if the data
has pointers initialized in code to point at sram, those will not be 
relocated and
will need some kind of fixup routine. I'm not very familiar with the SPL 
code
so take whatever I say with a grain of salt.

Troy
York Sun Feb. 11, 2014, 7:46 p.m. UTC | #8
On 02/11/2014 11:25 AM, Troy Kisky wrote:
> On 2/10/2014 7:34 PM, York Sun wrote:
>> On 02/10/2014 02:02 PM, York Sun wrote:
>>> This driver needs a data structure in SRAM before SDRAM is available.
>>> This is not alway the case using .data section. Moving this data
>>> structure to global_data guarantees it is writable.
>>>
>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>> CC: Troy Kisky <troy.kisky@boundarydevices.com>
>>> ---
>>>   drivers/i2c/mxc_i2c.c             |   18 ++++++++----------
>>>   include/asm-generic/global_data.h |    3 +++
>>>   2 files changed, 11 insertions(+), 10 deletions(-)
>>>
>> Troy,
>>
>> Following Tom's suggestion, I am trying to use linker script to put the srdata
>> into SRAM. But I still have a concern regarding initializing the srdata. I don't
>> see it is initialized anywhere. Do you presume the data is wiped out before the
>> driver runs? If that's the case, I need to clear the data somewhere in my code.
>>
>> York
>>
>> .
>>
> As I understand it, the .data section follows the .text section and is 
> relocated with it. Thus,
> if your program is loaded into sram, the .data will be there as well. 
> The .data section
> also contains the initial value, so no need for code to initialize it. 
> If later your code
> is relocated to sdram, the .data section should be relocated as well. 
> But if the data
> has pointers initialized in code to point at sram, those will not be 
> relocated and
> will need some kind of fixup routine. I'm not very familiar with the SPL 
> code
> so take whatever I say with a grain of salt.

Troy,

Thanks for the insight. I am not using SPL either. I need this driver to run
before u-boot relocates to DDR. Everything is in flash. I need to find a
writable location for the variable srdata. I can put this section in linker script

       .data.sram :
       {
               drivers/i2c/built-in.o (.data)
       } > sram

It works before and after the relocation. But the address after relocation is
not what I want. It is simply the original SRAM address plus the offset. It
lands the variable into DDR below U-boot. I am hoping to find a way to put it
into stack.

York
Wolfgang Denk Feb. 11, 2014, 7:59 p.m. UTC | #9
Dear York Sun,

In message <52FA7DFD.5060406@freescale.com> you wrote:
>
> Thanks for the insight. I am not using SPL either. I need this driver to run
> before u-boot relocates to DDR. Everything is in flash. I need to find a
> writable location for the variable srdata. I can put this section in linker script
> 
>        .data.sram :
>        {
>                drivers/i2c/built-in.o (.data)
>        } > sram
}

Please do not invent totally new ways to have writable data before
relocation.  Use the existing machanisms.  While running from flash,
we have but what little memory we can find in on-chip memory or SRAM
or data chace; we use this for the stack and global data (which should
be kept as small as possible, to allow for a as much stack as possible).

So ideally keep this data on the stack, and if there is no way around
it, in the global data structure.

Best regards,

Wolfgang Denk
York Sun Feb. 11, 2014, 8:03 p.m. UTC | #10
On 02/11/2014 11:59 AM, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <52FA7DFD.5060406@freescale.com> you wrote:
>>
>> Thanks for the insight. I am not using SPL either. I need this driver to run
>> before u-boot relocates to DDR. Everything is in flash. I need to find a
>> writable location for the variable srdata. I can put this section in linker script
>>
>>        .data.sram :
>>        {
>>                drivers/i2c/built-in.o (.data)
>>        } > sram
> }
> 
> Please do not invent totally new ways to have writable data before
> relocation.  Use the existing machanisms.  While running from flash,
> we have but what little memory we can find in on-chip memory or SRAM
> or data chace; we use this for the stack and global data (which should
> be kept as small as possible, to allow for a as much stack as possible).
> 
> So ideally keep this data on the stack, and if there is no way around
> it, in the global data structure.
> 

Agreed. I tried to use global data at first, which upsets Tom. Tom suggested to
use linker script. I guess he was under the impression I was using SPL. Let me
try harder to use stack.

York
Wolfgang Denk Feb. 11, 2014, 8:57 p.m. UTC | #11
Dear York,

In message <52FA8205.1090206@freescale.com> you wrote:
>
> > So ideally keep this data on the stack, and if there is no way around
> > it, in the global data structure.
> 
> Agreed. I tried to use global data at first, which upsets Tom. Tom suggested to
> use linker script. I guess he was under the impression I was using SPL. Let me
> try harder to use stack.

well, to do something with the linker script, you need some memory
somewhere you can use for this purpose.  Your example showed SRAM. so
if you do have SRAM on that board - why do you not use it for stack
and GD?  Where is your steck and GD right now?

Best regards,

Wolfgang Denk
York Sun Feb. 11, 2014, 9:02 p.m. UTC | #12
On 02/11/2014 12:57 PM, Wolfgang Denk wrote:
> Dear York,
> 
> In message <52FA8205.1090206@freescale.com> you wrote:
>>
>>> So ideally keep this data on the stack, and if there is no way around
>>> it, in the global data structure.
>>
>> Agreed. I tried to use global data at first, which upsets Tom. Tom suggested to
>> use linker script. I guess he was under the impression I was using SPL. Let me
>> try harder to use stack.
> 
> well, to do something with the linker script, you need some memory
> somewhere you can use for this purpose.  Your example showed SRAM. so
> if you do have SRAM on that board - why do you not use it for stack
> and GD?  Where is your steck and GD right now?
> 

The initial stack and GD are in SRAM. Of course they are moved to SDRAM after
initialization. I intend to spare SRAM for other purpose after relocation.
I am scratching my head trying to figure out how to put this variable "srdata"
in mxc_i2c.c into stack. Please give me some guidance if you have the idea on
top of your head.

York
York Sun Feb. 11, 2014, 9:46 p.m. UTC | #13
On 02/11/2014 10:01 AM, York Sun wrote:
> On 02/10/2014 02:45 PM, Tom Rini wrote:
>> On Mon, Feb 10, 2014 at 02:28:01PM -0800, York Sun wrote:
>>> On 02/10/2014 02:10 PM, Tom Rini wrote:
>>>> On Mon, Feb 10, 2014 at 02:02:52PM -0800, York Sun wrote:
>>>>
>>>>> This driver needs a data structure in SRAM before SDRAM is available.
>>>>> This is not alway the case using .data section. Moving this data
>>>>> structure to global_data guarantees it is writable.
>>>>>
>>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>>> CC: Troy Kisky <troy.kisky@boundarydevices.com>
>>>>
>>>> If you need something in SRAM then you need to place it in that section,
>>>> see arch/arm/cpu/armv7/am33xx/u-boot-spl.lds for example
>>>>
>>>
>>> I am not sure if it is a similar situation. But anyway, I am open to
>>> suggestions.
>>>
>>> For this driver, the variable needs to be writable. I guess it wasn't
>>> a problem for existing platforms which probably call this driver after
>>> relocation.
>>>
>>> Does the SRAM code/data relocate to SDRAM? I don't want this variable
>>> to stay in SRAM once u-boot relocates to normal memory.
>>
>> So in this case the problem is still within SPL right?  You would put
>> the parts you need to have in SRAM during SPL and DDR in full U-Boot
>> with __attribute__((section(".data.srdata"))) and in the SoC's linker
>> scripts make sure that drivers/i2c/built-in.o(.data.srdata) ends up in
>> the appropriate place for each case.
>>
>> I say all of this as I think we really want to avoid expanding on
>> gd->arch-> stuff if we can (which reminds me, your patch expands on main
>> gd not gd->arch so NAK on that either way).
>>
> 
> I tried the linker script and made it work. But it is not what I want. I expect
> this variable relocates to DDR, but the offset is not right.
> I am not using SPL. This is full u-boot, which boots from flash, calls i2c
> driver, initializes DDR, then relocates. Putting this variable into stack might
> be the solution. Suggestions?
> 

Tom,

I haven't found an easy way to put this variable into stack. Would you
reconsider global data, as I originally proposed in the patch?

York
Wolfgang Denk Feb. 11, 2014, 10:12 p.m. UTC | #14
Dear York,

In message <52FA8FDB.3030808@freescale.com> you wrote:
>
> > well, to do something with the linker script, you need some memory
> > somewhere you can use for this purpose.  Your example showed SRAM. so
> > if you do have SRAM on that board - why do you not use it for stack
> > and GD?  Where is your steck and GD right now?
> 
> The initial stack and GD are in SRAM. Of course they are moved to SDRAM after
> initialization. I intend to spare SRAM for other purpose after relocation.

Well, after relocation GD has also been relocated, so your SRAM would
be comletely unused.

> I am scratching my head trying to figure out how to put this variable "srdata"
> in mxc_i2c.c into stack. Please give me some guidance if you have the idea on
> top of your head.

I have no idea which code you are talking about - sorry.

Best regards,

Wolfgang Denk
York Sun Feb. 11, 2014, 10:20 p.m. UTC | #15
On 02/11/2014 02:12 PM, Wolfgang Denk wrote:
> Dear York,
> 
> In message <52FA8FDB.3030808@freescale.com> you wrote:
>>
>>> well, to do something with the linker script, you need some memory
>>> somewhere you can use for this purpose.  Your example showed SRAM. so
>>> if you do have SRAM on that board - why do you not use it for stack
>>> and GD?  Where is your steck and GD right now?
>>
>> The initial stack and GD are in SRAM. Of course they are moved to SDRAM after
>> initialization. I intend to spare SRAM for other purpose after relocation.
> 
> Well, after relocation GD has also been relocated, so your SRAM would
> be comletely unused.

Sounds like you are OK with using GD for this patch. Let's wait to hear from
Tom. He nacked this idea.

> 
>> I am scratching my head trying to figure out how to put this variable "srdata"
>> in mxc_i2c.c into stack. Please give me some guidance if you have the idea on
>> top of your head.
> 
> I have no idea which code you are talking about - sorry.
> 

http://patchwork.ozlabs.org/patch/319073/


-/*
- * For SPL boot some boards need i2c before SDRAM is initialized so force
- * variables to live in SRAM
- */
-static struct sram_data __attribute__((section(".data"))) srdata;
-

I moved this variable into GD. Is there a alternative way to do it? I tried to
use stack but didn't find the solution.

York
Wolfgang Denk Feb. 12, 2014, 2:27 p.m. UTC | #16
Dear York,

In message <52FAA233.6090403@freescale.com> you wrote:
>
> > Well, after relocation GD has also been relocated, so your SRAM would
> > be comletely unused.
> 
> Sounds like you are OK with using GD for this patch. Let's wait to hear from
> Tom. He nacked this idea.

I don't say I think this is a good change.  I just tried to explain to
you that the SRAM will be unused after relocation completed.  Tom
probably has the same problem as I: I cannot understand why you are
changing the code.  If it's been working as is before, then why would
it stop now?

> -/*
> - * For SPL boot some boards need i2c before SDRAM is initialized so force
> - * variables to live in SRAM
> - */
> -static struct sram_data __attribute__((section(".data"))) srdata;
> -
> 
> I moved this variable into GD. Is there a alternative way to do it? I tried to
> use stack but didn't find the solution.

But why did you move it?  It was working before, right?  So what has
changed, and why cannot you fix it in a way so the variable remains
where it is, i. e. without the need to move it to GD?

Best regards,

Wolfgang Denk
Tom Rini Feb. 12, 2014, 2:41 p.m. UTC | #17
On Tue, Feb 11, 2014 at 01:46:08PM -0800, York Sun wrote:
> On 02/11/2014 10:01 AM, York Sun wrote:
> > On 02/10/2014 02:45 PM, Tom Rini wrote:
> >> On Mon, Feb 10, 2014 at 02:28:01PM -0800, York Sun wrote:
> >>> On 02/10/2014 02:10 PM, Tom Rini wrote:
> >>>> On Mon, Feb 10, 2014 at 02:02:52PM -0800, York Sun wrote:
> >>>>
> >>>>> This driver needs a data structure in SRAM before SDRAM is available.
> >>>>> This is not alway the case using .data section. Moving this data
> >>>>> structure to global_data guarantees it is writable.
> >>>>>
> >>>>> Signed-off-by: York Sun <yorksun@freescale.com>
> >>>>> CC: Troy Kisky <troy.kisky@boundarydevices.com>
> >>>>
> >>>> If you need something in SRAM then you need to place it in that section,
> >>>> see arch/arm/cpu/armv7/am33xx/u-boot-spl.lds for example
> >>>>
> >>>
> >>> I am not sure if it is a similar situation. But anyway, I am open to
> >>> suggestions.
> >>>
> >>> For this driver, the variable needs to be writable. I guess it wasn't
> >>> a problem for existing platforms which probably call this driver after
> >>> relocation.
> >>>
> >>> Does the SRAM code/data relocate to SDRAM? I don't want this variable
> >>> to stay in SRAM once u-boot relocates to normal memory.
> >>
> >> So in this case the problem is still within SPL right?  You would put
> >> the parts you need to have in SRAM during SPL and DDR in full U-Boot
> >> with __attribute__((section(".data.srdata"))) and in the SoC's linker
> >> scripts make sure that drivers/i2c/built-in.o(.data.srdata) ends up in
> >> the appropriate place for each case.
> >>
> >> I say all of this as I think we really want to avoid expanding on
> >> gd->arch-> stuff if we can (which reminds me, your patch expands on main
> >> gd not gd->arch so NAK on that either way).
> >>
> > 
> > I tried the linker script and made it work. But it is not what I want. I expect
> > this variable relocates to DDR, but the offset is not right.
> > I am not using SPL. This is full u-boot, which boots from flash, calls i2c
> > driver, initializes DDR, then relocates. Putting this variable into stack might
> > be the solution. Suggestions?
> 
> I haven't found an easy way to put this variable into stack. Would you
> reconsider global data, as I originally proposed in the patch?

Since we've gone around and looked at the other possibilities, yeah, OK,
we can put this into gd->arch (not just top level gd).  Thanks for
looking!
Tom Rini Feb. 12, 2014, 2:43 p.m. UTC | #18
On Wed, Feb 12, 2014 at 03:27:58PM +0100, Wolfgang Denk wrote:
> Dear York,
> 
> In message <52FAA233.6090403@freescale.com> you wrote:
> >
> > > Well, after relocation GD has also been relocated, so your SRAM would
> > > be comletely unused.
> > 
> > Sounds like you are OK with using GD for this patch. Let's wait to hear from
> > Tom. He nacked this idea.
> 
> I don't say I think this is a good change.  I just tried to explain to
> you that the SRAM will be unused after relocation completed.  Tom
> probably has the same problem as I: I cannot understand why you are
> changing the code.  If it's been working as is before, then why would
> it stop now?

I think I see it.  We had been concerned with the SPL case, and the
current driver work-around is how we do it (as to not litter gd->arch
with lots of things just for drivers we need so very very early in SPL).
York now has a different setup where this i2c block is being reused,
where SRAM is big enough (apparently) to load the whole of U-Boot into
and start execution from, but we still want to relocate into DDR.  This
is a problem over in TI-land we've avoided by simply saying "lets keep
using SPL anyhow".
York Sun Feb. 12, 2014, 5:56 p.m. UTC | #19
On 02/12/2014 06:43 AM, Tom Rini wrote:
> On Wed, Feb 12, 2014 at 03:27:58PM +0100, Wolfgang Denk wrote:
>> Dear York,
>>
>> In message <52FAA233.6090403@freescale.com> you wrote:
>>>
>>>> Well, after relocation GD has also been relocated, so your SRAM would
>>>> be comletely unused.
>>>
>>> Sounds like you are OK with using GD for this patch. Let's wait to hear from
>>> Tom. He nacked this idea.
>>
>> I don't say I think this is a good change.  I just tried to explain to
>> you that the SRAM will be unused after relocation completed.  Tom
>> probably has the same problem as I: I cannot understand why you are
>> changing the code.  If it's been working as is before, then why would
>> it stop now?
> 
> I think I see it.  We had been concerned with the SPL case, and the
> current driver work-around is how we do it (as to not litter gd->arch
> with lots of things just for drivers we need so very very early in SPL).
> York now has a different setup where this i2c block is being reused,
> where SRAM is big enough (apparently) to load the whole of U-Boot into
> and start execution from, but we still want to relocate into DDR.  This
> is a problem over in TI-land we've avoided by simply saying "lets keep
> using SPL anyhow".
> 

A minor correction, I am not putting u-boot in SRAM. I am reusing this mxc_i2c
driver. It has been working, but my situation is different. I am not using SPL.
U-boot runs in flash when the driver is called. I have SRAM to host stack and
GD. This driver requires writable memory. Obviously I have only three options,
use stack, GD, or make a special treatment to use SRAM. As we discussed and I
tried, using linker script to link it to SRAM is messy and the relocation result
is bad. I can't find a good way to use stack either. That leaves me the best
option is to use GD.

York
diff mbox

Patch

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 595019b..48468d7 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -22,6 +22,8 @@ 
 #include <i2c.h>
 #include <watchdog.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 #ifdef I2C_QUIRK_REG
 struct mxc_i2c_regs {
 	uint8_t		iadr;
@@ -411,12 +413,6 @@  struct sram_data {
 	struct i2c_parms i2c_data[3];
 };
 
-/*
- * For SPL boot some boards need i2c before SDRAM is initialized so force
- * variables to live in SRAM
- */
-static struct sram_data __attribute__((section(".data"))) srdata;
-
 static void * const i2c_bases[] = {
 #if defined(CONFIG_MX25)
 	(void *)IMX_I2C_BASE,
@@ -445,9 +441,10 @@  void *i2c_get_base(struct i2c_adapter *adap)
 
 static struct i2c_parms *i2c_get_parms(void *base)
 {
+	struct sram_data *srdata = (void *)gd->srdata;
 	int i = 0;
-	struct i2c_parms *p = srdata.i2c_data;
-	while (i < ARRAY_SIZE(srdata.i2c_data)) {
+	struct i2c_parms *p = srdata->i2c_data;
+	while (i < ARRAY_SIZE(srdata->i2c_data)) {
 		if (p->base == base)
 			return p;
 		p++;
@@ -490,8 +487,9 @@  static int mxc_i2c_probe(struct i2c_adapter *adap, uint8_t chip)
 void bus_i2c_init(void *base, int speed, int unused,
 		int (*idle_bus_fn)(void *p), void *idle_bus_data)
 {
+	struct sram_data *srdata = (void *)gd->srdata;
 	int i = 0;
-	struct i2c_parms *p = srdata.i2c_data;
+	struct i2c_parms *p = srdata->i2c_data;
 	if (!base)
 		return;
 	for (;;) {
@@ -505,7 +503,7 @@  void bus_i2c_init(void *base, int speed, int unused,
 		}
 		p++;
 		i++;
-		if (i >= ARRAY_SIZE(srdata.i2c_data))
+		if (i >= ARRAY_SIZE(srdata->i2c_data))
 			return;
 	}
 	bus_i2c_set_bus_speed(base, speed);
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 0de0bea..cc62861 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -72,6 +72,9 @@  typedef struct global_data {
 #if defined(CONFIG_SYS_I2C)
 	int		cur_i2c_bus;	/* current used i2c bus */
 #endif
+#ifdef CONFIG_SYS_I2C_MXC
+	void *srdata[10];
+#endif
 	unsigned long timebase_h;
 	unsigned long timebase_l;
 	struct arch_global_data arch;	/* architecture-specific data */