Re: [PATCH] arm: mvebu: mbus: Fix mbus driver to work also after U-Boot relocation

2022-08-24 Thread Chris Packham
Hi Pali,

On 23/08/22 11:00, Pali Rohár wrote:
> On Wednesday 17 August 2022 08:17:36 Stefan Roese wrote:
>> On 10.08.22 14:46, Pali Rohár wrote:
>>> mbus driver is initialized from arch_cpu_init() callback which is called
>>> before relocation. This driver stores lot of functions and structure
>>> pointers into global variables, so it is data position dependent.
>>>
>>> Therefore after relocations all pointers are invalid and driver does not
>>> work anymore as all pointers referes to the old memory, which overlaps with
>>> CONFIG_SYS_LOAD_ADDR and ${loadaddr}.
>>>
>>> For example U-Boot fuse command crashes if loadaddr memory is cleared or
>>> rewritten by some image loaded by U-Boot load command.
>>>
>>> mw.w ${loadaddr} 0x0 1
>>> fuse read 0 1 2
>>>
>>> Fix this issue by removing of all mbus global variables in which are stored
>>> pointers to structures or functions which changes during relocation. And
>>> replace it by direct function calls (not via pointers). With this change
>>> fuse command finally works.
>>>
>>> Signed-off-by: Pali Rohár 
>> Reviewed-by: Stefan Roese 
>>
>> Thanks,
>> Stefan
> Stefan, this is something which is needed to have fixed. Could you test
> this change on your boards and prepare for merging to master branch?
>
> Chris, could you also test this change for possible regressions?
>
> I have tested it on A385 Turris Omnia.

I see I'm a little late to the testing party but I've just checked what 
was merged on an x530 and it's all good there.

>>> ---
>>>arch/arm/mach-kirkwood/include/mach/cpu.h |   3 -
>>>arch/arm/mach-mvebu/include/mach/cpu.h|   3 -
>>>arch/arm/mach-mvebu/mbus.c| 167 +-
>>>board/alliedtelesis/x530/x530.c   |   2 +-
>>>board/maxbcm/maxbcm.c |   8 +-
>>>board/theadorable/theadorable.c   |   4 +-
>>>include/linux/mbus.h  |  13 +-
>>>7 files changed, 75 insertions(+), 125 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-kirkwood/include/mach/cpu.h 
>>> b/arch/arm/mach-kirkwood/include/mach/cpu.h
>>> index 71c546f9acf6..d8639c60352b 100644
>>> --- a/arch/arm/mach-kirkwood/include/mach/cpu.h
>>> +++ b/arch/arm/mach-kirkwood/include/mach/cpu.h
>>> @@ -144,9 +144,6 @@ struct kwgpio_registers {
>>> u32 irq_level;
>>>};
>>> -/* Needed for dynamic (board-specific) mbus configuration */
>>> -extern struct mvebu_mbus_state mbus_state;
>>> -
>>>/*
>>> * functions
>>> */
>>> diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h 
>>> b/arch/arm/mach-mvebu/include/mach/cpu.h
>>> index 689c96bd4eac..d9fa1f32aa52 100644
>>> --- a/arch/arm/mach-mvebu/include/mach/cpu.h
>>> +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
>>> @@ -122,9 +122,6 @@ struct sar_freq_modes {
>>> u32 d_clk;
>>>};
>>> -/* Needed for dynamic (board-specific) mbus configuration */
>>> -extern struct mvebu_mbus_state mbus_state;
>>> -
>>>/*
>>> * functions
>>> */
>>> diff --git a/arch/arm/mach-mvebu/mbus.c b/arch/arm/mach-mvebu/mbus.c
>>> index 3b1b9f73ebf6..7092f6cc10c2 100644
>>> --- a/arch/arm/mach-mvebu/mbus.c
>>> +++ b/arch/arm/mach-mvebu/mbus.c
>>> @@ -88,31 +88,34 @@
>>>#define DOVE_DDR_BASE_CS_OFF(n) ((n) << 4)
>>> -struct mvebu_mbus_state;
>>> -
>>> -struct mvebu_mbus_soc_data {
>>> -   unsigned int num_wins;
>>> -   unsigned int num_remappable_wins;
>>> -   unsigned int (*win_cfg_offset)(const int win);
>>> -   void (*setup_cpu_target)(struct mvebu_mbus_state *s);
>>> -};
>>> -
>>> -struct mvebu_mbus_state mbus_state
>>> -   __section(".data");
>>>static struct mbus_dram_target_info mbus_dram_info
>>> __section(".data");
>>> +#if defined(CONFIG_ARCH_MVEBU)
>>> +   #define MVEBU_MBUS_NUM_WINS 20
>>> +   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 8
>>> +   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) 
>>> armada_370_xp_mbus_win_offset(win)
>>> +#elif defined(CONFIG_ARCH_KIRKWOOD)
>>> +   #define MVEBU_MBUS_NUM_WINS 8
>>> +   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 4
>>> +   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) orion5x_mbus_win_offset(win)
>>> +#else
>>> +   #error "No supported architecture"
>>> +#endif
>>> +
>>> +static unsigned int armada_370_xp_mbus_win_offset(int win);
>>> +static unsigned int orion5x_mbus_win_offset(int win);
>>> +
>>>/*
>>> * Functions to manipulate the address decoding windows
>>> */
>>> -static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,
>>> -  int win, int *enabled, u64 *base,
>>> +static void mvebu_mbus_read_window(int win, int *enabled, u64 *base,
>>>u32 *size, u8 *target, u8 *attr,
>>>u64 *remap)
>>>{
>>> -   void __iomem *addr = mbus->mbuswins_base +
>>> -   mbus->soc->win_cfg_offset(win);
>>> +   void __iomem *addr = (void __iomem *)MVEBU_CPU_WIN_BASE +
>>> +   MVEBU_MBUS_WIN_CFG_OFFSET(win);
>>> u32 basereg = readl(addr + WIN_BASE_OFF);
>>> u32 

Re: [PATCH] arm: mvebu: mbus: Fix mbus driver to work also after U-Boot relocation

2022-08-23 Thread Stefan Roese

On 10.08.22 14:46, Pali Rohár wrote:

mbus driver is initialized from arch_cpu_init() callback which is called
before relocation. This driver stores lot of functions and structure
pointers into global variables, so it is data position dependent.

Therefore after relocations all pointers are invalid and driver does not
work anymore as all pointers referes to the old memory, which overlaps with
CONFIG_SYS_LOAD_ADDR and ${loadaddr}.

For example U-Boot fuse command crashes if loadaddr memory is cleared or
rewritten by some image loaded by U-Boot load command.

   mw.w ${loadaddr} 0x0 1
   fuse read 0 1 2

Fix this issue by removing of all mbus global variables in which are stored
pointers to structures or functions which changes during relocation. And
replace it by direct function calls (not via pointers). With this change
fuse command finally works.

Signed-off-by: Pali Rohár 


Applied to u-boot-marvell/master

Thanks,
Stefan


---
  arch/arm/mach-kirkwood/include/mach/cpu.h |   3 -
  arch/arm/mach-mvebu/include/mach/cpu.h|   3 -
  arch/arm/mach-mvebu/mbus.c| 167 +-
  board/alliedtelesis/x530/x530.c   |   2 +-
  board/maxbcm/maxbcm.c |   8 +-
  board/theadorable/theadorable.c   |   4 +-
  include/linux/mbus.h  |  13 +-
  7 files changed, 75 insertions(+), 125 deletions(-)

diff --git a/arch/arm/mach-kirkwood/include/mach/cpu.h 
b/arch/arm/mach-kirkwood/include/mach/cpu.h
index 71c546f9acf6..d8639c60352b 100644
--- a/arch/arm/mach-kirkwood/include/mach/cpu.h
+++ b/arch/arm/mach-kirkwood/include/mach/cpu.h
@@ -144,9 +144,6 @@ struct kwgpio_registers {
u32 irq_level;
  };
  
-/* Needed for dynamic (board-specific) mbus configuration */

-extern struct mvebu_mbus_state mbus_state;
-
  /*
   * functions
   */
diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h 
b/arch/arm/mach-mvebu/include/mach/cpu.h
index 689c96bd4eac..d9fa1f32aa52 100644
--- a/arch/arm/mach-mvebu/include/mach/cpu.h
+++ b/arch/arm/mach-mvebu/include/mach/cpu.h
@@ -122,9 +122,6 @@ struct sar_freq_modes {
u32 d_clk;
  };
  
-/* Needed for dynamic (board-specific) mbus configuration */

-extern struct mvebu_mbus_state mbus_state;
-
  /*
   * functions
   */
diff --git a/arch/arm/mach-mvebu/mbus.c b/arch/arm/mach-mvebu/mbus.c
index 3b1b9f73ebf6..7092f6cc10c2 100644
--- a/arch/arm/mach-mvebu/mbus.c
+++ b/arch/arm/mach-mvebu/mbus.c
@@ -88,31 +88,34 @@
  
  #define DOVE_DDR_BASE_CS_OFF(n) ((n) << 4)
  
-struct mvebu_mbus_state;

-
-struct mvebu_mbus_soc_data {
-   unsigned int num_wins;
-   unsigned int num_remappable_wins;
-   unsigned int (*win_cfg_offset)(const int win);
-   void (*setup_cpu_target)(struct mvebu_mbus_state *s);
-};
-
-struct mvebu_mbus_state mbus_state
-   __section(".data");
  static struct mbus_dram_target_info mbus_dram_info
__section(".data");
  
+#if defined(CONFIG_ARCH_MVEBU)

+   #define MVEBU_MBUS_NUM_WINS 20
+   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 8
+   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) 
armada_370_xp_mbus_win_offset(win)
+#elif defined(CONFIG_ARCH_KIRKWOOD)
+   #define MVEBU_MBUS_NUM_WINS 8
+   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 4
+   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) orion5x_mbus_win_offset(win)
+#else
+   #error "No supported architecture"
+#endif
+
+static unsigned int armada_370_xp_mbus_win_offset(int win);
+static unsigned int orion5x_mbus_win_offset(int win);
+
  /*
   * Functions to manipulate the address decoding windows
   */
  
-static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,

-  int win, int *enabled, u64 *base,
+static void mvebu_mbus_read_window(int win, int *enabled, u64 *base,
   u32 *size, u8 *target, u8 *attr,
   u64 *remap)
  {
-   void __iomem *addr = mbus->mbuswins_base +
-   mbus->soc->win_cfg_offset(win);
+   void __iomem *addr = (void __iomem *)MVEBU_CPU_WIN_BASE +
+   MVEBU_MBUS_WIN_CFG_OFFSET(win);
u32 basereg = readl(addr + WIN_BASE_OFF);
u32 ctrlreg = readl(addr + WIN_CTRL_OFF);
  
@@ -133,7 +136,7 @@ static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,

*attr = (ctrlreg & WIN_CTRL_ATTR_MASK) >> WIN_CTRL_ATTR_SHIFT;
  
  	if (remap) {

-   if (win < mbus->soc->num_remappable_wins) {
+   if (win < MVEBU_MBUS_NUM_REMAPPABLE_WINS) {
u32 remap_low = readl(addr + WIN_REMAP_LO_OFF);
u32 remap_hi  = readl(addr + WIN_REMAP_HI_OFF);
*remap = ((u64)remap_hi << 32) | remap_low;
@@ -143,27 +146,25 @@ static void mvebu_mbus_read_window(struct 
mvebu_mbus_state *mbus,
}
  }
  
-static void mvebu_mbus_disable_window(struct mvebu_mbus_state *mbus,

- int win)
+static void 

Re: [PATCH] arm: mvebu: mbus: Fix mbus driver to work also after U-Boot relocation

2022-08-22 Thread Tony Dinh
Hi Pali,

On Mon, Aug 22, 2022 at 4:00 PM Pali Rohár  wrote:
>
> On Wednesday 17 August 2022 08:17:36 Stefan Roese wrote:
> > On 10.08.22 14:46, Pali Rohár wrote:
> > > mbus driver is initialized from arch_cpu_init() callback which is called
> > > before relocation. This driver stores lot of functions and structure
> > > pointers into global variables, so it is data position dependent.
> > >
> > > Therefore after relocations all pointers are invalid and driver does not
> > > work anymore as all pointers referes to the old memory, which overlaps 
> > > with
> > > CONFIG_SYS_LOAD_ADDR and ${loadaddr}.
> > >
> > > For example U-Boot fuse command crashes if loadaddr memory is cleared or
> > > rewritten by some image loaded by U-Boot load command.
> > >
> > >mw.w ${loadaddr} 0x0 1
> > >fuse read 0 1 2
> > >
> > > Fix this issue by removing of all mbus global variables in which are 
> > > stored
> > > pointers to structures or functions which changes during relocation. And
> > > replace it by direct function calls (not via pointers). With this change
> > > fuse command finally works.
> > >
> > > Signed-off-by: Pali Rohár 
> >
> > Reviewed-by: Stefan Roese 
> >
> > Thanks,
> > Stefan
>
> Stefan, this is something which is needed to have fixed. Could you test
> this change on your boards and prepare for merging to master branch?
>
> Chris, could you also test this change for possible regressions?
>
> I have tested it on A385 Turris Omnia.

I did a couple regression tests and both are running fine!

Cloud Engines Pogo V4 (88F6192 SoC)
Zyxel NSA310S (88F6702 SoC)

All the best,
Tony

>
> > > ---
> > >   arch/arm/mach-kirkwood/include/mach/cpu.h |   3 -
> > >   arch/arm/mach-mvebu/include/mach/cpu.h|   3 -
> > >   arch/arm/mach-mvebu/mbus.c| 167 +-
> > >   board/alliedtelesis/x530/x530.c   |   2 +-
> > >   board/maxbcm/maxbcm.c |   8 +-
> > >   board/theadorable/theadorable.c   |   4 +-
> > >   include/linux/mbus.h  |  13 +-
> > >   7 files changed, 75 insertions(+), 125 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-kirkwood/include/mach/cpu.h 
> > > b/arch/arm/mach-kirkwood/include/mach/cpu.h
> > > index 71c546f9acf6..d8639c60352b 100644
> > > --- a/arch/arm/mach-kirkwood/include/mach/cpu.h
> > > +++ b/arch/arm/mach-kirkwood/include/mach/cpu.h
> > > @@ -144,9 +144,6 @@ struct kwgpio_registers {
> > > u32 irq_level;
> > >   };
> > > -/* Needed for dynamic (board-specific) mbus configuration */
> > > -extern struct mvebu_mbus_state mbus_state;
> > > -
> > >   /*
> > >* functions
> > >*/
> > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h 
> > > b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > index 689c96bd4eac..d9fa1f32aa52 100644
> > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > @@ -122,9 +122,6 @@ struct sar_freq_modes {
> > > u32 d_clk;
> > >   };
> > > -/* Needed for dynamic (board-specific) mbus configuration */
> > > -extern struct mvebu_mbus_state mbus_state;
> > > -
> > >   /*
> > >* functions
> > >*/
> > > diff --git a/arch/arm/mach-mvebu/mbus.c b/arch/arm/mach-mvebu/mbus.c
> > > index 3b1b9f73ebf6..7092f6cc10c2 100644
> > > --- a/arch/arm/mach-mvebu/mbus.c
> > > +++ b/arch/arm/mach-mvebu/mbus.c
> > > @@ -88,31 +88,34 @@
> > >   #define DOVE_DDR_BASE_CS_OFF(n) ((n) << 4)
> > > -struct mvebu_mbus_state;
> > > -
> > > -struct mvebu_mbus_soc_data {
> > > -   unsigned int num_wins;
> > > -   unsigned int num_remappable_wins;
> > > -   unsigned int (*win_cfg_offset)(const int win);
> > > -   void (*setup_cpu_target)(struct mvebu_mbus_state *s);
> > > -};
> > > -
> > > -struct mvebu_mbus_state mbus_state
> > > -   __section(".data");
> > >   static struct mbus_dram_target_info mbus_dram_info
> > > __section(".data");
> > > +#if defined(CONFIG_ARCH_MVEBU)
> > > +   #define MVEBU_MBUS_NUM_WINS 20
> > > +   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 8
> > > +   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) 
> > > armada_370_xp_mbus_win_offset(win)
> > > +#elif defined(CONFIG_ARCH_KIRKWOOD)
> > > +   #define MVEBU_MBUS_NUM_WINS 8
> > > +   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 4
> > > +   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) orion5x_mbus_win_offset(win)
> > > +#else
> > > +   #error "No supported architecture"
> > > +#endif
> > > +
> > > +static unsigned int armada_370_xp_mbus_win_offset(int win);
> > > +static unsigned int orion5x_mbus_win_offset(int win);
> > > +
> > >   /*
> > >* Functions to manipulate the address decoding windows
> > >*/
> > > -static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,
> > > -  int win, int *enabled, u64 *base,
> > > +static void mvebu_mbus_read_window(int win, int *enabled, u64 *base,
> > >u32 *size, u8 *target, u8 *attr,
> > >u64 *remap)
> > >   {
> > > -   void __iomem *addr = 

Re: [PATCH] arm: mvebu: mbus: Fix mbus driver to work also after U-Boot relocation

2022-08-22 Thread Pali Rohár
On Wednesday 17 August 2022 08:17:36 Stefan Roese wrote:
> On 10.08.22 14:46, Pali Rohár wrote:
> > mbus driver is initialized from arch_cpu_init() callback which is called
> > before relocation. This driver stores lot of functions and structure
> > pointers into global variables, so it is data position dependent.
> > 
> > Therefore after relocations all pointers are invalid and driver does not
> > work anymore as all pointers referes to the old memory, which overlaps with
> > CONFIG_SYS_LOAD_ADDR and ${loadaddr}.
> > 
> > For example U-Boot fuse command crashes if loadaddr memory is cleared or
> > rewritten by some image loaded by U-Boot load command.
> > 
> >mw.w ${loadaddr} 0x0 1
> >fuse read 0 1 2
> > 
> > Fix this issue by removing of all mbus global variables in which are stored
> > pointers to structures or functions which changes during relocation. And
> > replace it by direct function calls (not via pointers). With this change
> > fuse command finally works.
> > 
> > Signed-off-by: Pali Rohár 
> 
> Reviewed-by: Stefan Roese 
> 
> Thanks,
> Stefan

Stefan, this is something which is needed to have fixed. Could you test
this change on your boards and prepare for merging to master branch?

Chris, could you also test this change for possible regressions?

I have tested it on A385 Turris Omnia.

> > ---
> >   arch/arm/mach-kirkwood/include/mach/cpu.h |   3 -
> >   arch/arm/mach-mvebu/include/mach/cpu.h|   3 -
> >   arch/arm/mach-mvebu/mbus.c| 167 +-
> >   board/alliedtelesis/x530/x530.c   |   2 +-
> >   board/maxbcm/maxbcm.c |   8 +-
> >   board/theadorable/theadorable.c   |   4 +-
> >   include/linux/mbus.h  |  13 +-
> >   7 files changed, 75 insertions(+), 125 deletions(-)
> > 
> > diff --git a/arch/arm/mach-kirkwood/include/mach/cpu.h 
> > b/arch/arm/mach-kirkwood/include/mach/cpu.h
> > index 71c546f9acf6..d8639c60352b 100644
> > --- a/arch/arm/mach-kirkwood/include/mach/cpu.h
> > +++ b/arch/arm/mach-kirkwood/include/mach/cpu.h
> > @@ -144,9 +144,6 @@ struct kwgpio_registers {
> > u32 irq_level;
> >   };
> > -/* Needed for dynamic (board-specific) mbus configuration */
> > -extern struct mvebu_mbus_state mbus_state;
> > -
> >   /*
> >* functions
> >*/
> > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h 
> > b/arch/arm/mach-mvebu/include/mach/cpu.h
> > index 689c96bd4eac..d9fa1f32aa52 100644
> > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > @@ -122,9 +122,6 @@ struct sar_freq_modes {
> > u32 d_clk;
> >   };
> > -/* Needed for dynamic (board-specific) mbus configuration */
> > -extern struct mvebu_mbus_state mbus_state;
> > -
> >   /*
> >* functions
> >*/
> > diff --git a/arch/arm/mach-mvebu/mbus.c b/arch/arm/mach-mvebu/mbus.c
> > index 3b1b9f73ebf6..7092f6cc10c2 100644
> > --- a/arch/arm/mach-mvebu/mbus.c
> > +++ b/arch/arm/mach-mvebu/mbus.c
> > @@ -88,31 +88,34 @@
> >   #define DOVE_DDR_BASE_CS_OFF(n) ((n) << 4)
> > -struct mvebu_mbus_state;
> > -
> > -struct mvebu_mbus_soc_data {
> > -   unsigned int num_wins;
> > -   unsigned int num_remappable_wins;
> > -   unsigned int (*win_cfg_offset)(const int win);
> > -   void (*setup_cpu_target)(struct mvebu_mbus_state *s);
> > -};
> > -
> > -struct mvebu_mbus_state mbus_state
> > -   __section(".data");
> >   static struct mbus_dram_target_info mbus_dram_info
> > __section(".data");
> > +#if defined(CONFIG_ARCH_MVEBU)
> > +   #define MVEBU_MBUS_NUM_WINS 20
> > +   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 8
> > +   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) 
> > armada_370_xp_mbus_win_offset(win)
> > +#elif defined(CONFIG_ARCH_KIRKWOOD)
> > +   #define MVEBU_MBUS_NUM_WINS 8
> > +   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 4
> > +   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) orion5x_mbus_win_offset(win)
> > +#else
> > +   #error "No supported architecture"
> > +#endif
> > +
> > +static unsigned int armada_370_xp_mbus_win_offset(int win);
> > +static unsigned int orion5x_mbus_win_offset(int win);
> > +
> >   /*
> >* Functions to manipulate the address decoding windows
> >*/
> > -static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,
> > -  int win, int *enabled, u64 *base,
> > +static void mvebu_mbus_read_window(int win, int *enabled, u64 *base,
> >u32 *size, u8 *target, u8 *attr,
> >u64 *remap)
> >   {
> > -   void __iomem *addr = mbus->mbuswins_base +
> > -   mbus->soc->win_cfg_offset(win);
> > +   void __iomem *addr = (void __iomem *)MVEBU_CPU_WIN_BASE +
> > +   MVEBU_MBUS_WIN_CFG_OFFSET(win);
> > u32 basereg = readl(addr + WIN_BASE_OFF);
> > u32 ctrlreg = readl(addr + WIN_CTRL_OFF);
> > @@ -133,7 +136,7 @@ static void mvebu_mbus_read_window(struct 
> > mvebu_mbus_state *mbus,
> > *attr = (ctrlreg & WIN_CTRL_ATTR_MASK) >> 

Re: [PATCH] arm: mvebu: mbus: Fix mbus driver to work also after U-Boot relocation

2022-08-17 Thread Stefan Roese

On 10.08.22 14:46, Pali Rohár wrote:

mbus driver is initialized from arch_cpu_init() callback which is called
before relocation. This driver stores lot of functions and structure
pointers into global variables, so it is data position dependent.

Therefore after relocations all pointers are invalid and driver does not
work anymore as all pointers referes to the old memory, which overlaps with
CONFIG_SYS_LOAD_ADDR and ${loadaddr}.

For example U-Boot fuse command crashes if loadaddr memory is cleared or
rewritten by some image loaded by U-Boot load command.

   mw.w ${loadaddr} 0x0 1
   fuse read 0 1 2

Fix this issue by removing of all mbus global variables in which are stored
pointers to structures or functions which changes during relocation. And
replace it by direct function calls (not via pointers). With this change
fuse command finally works.

Signed-off-by: Pali Rohár 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  arch/arm/mach-kirkwood/include/mach/cpu.h |   3 -
  arch/arm/mach-mvebu/include/mach/cpu.h|   3 -
  arch/arm/mach-mvebu/mbus.c| 167 +-
  board/alliedtelesis/x530/x530.c   |   2 +-
  board/maxbcm/maxbcm.c |   8 +-
  board/theadorable/theadorable.c   |   4 +-
  include/linux/mbus.h  |  13 +-
  7 files changed, 75 insertions(+), 125 deletions(-)

diff --git a/arch/arm/mach-kirkwood/include/mach/cpu.h 
b/arch/arm/mach-kirkwood/include/mach/cpu.h
index 71c546f9acf6..d8639c60352b 100644
--- a/arch/arm/mach-kirkwood/include/mach/cpu.h
+++ b/arch/arm/mach-kirkwood/include/mach/cpu.h
@@ -144,9 +144,6 @@ struct kwgpio_registers {
u32 irq_level;
  };
  
-/* Needed for dynamic (board-specific) mbus configuration */

-extern struct mvebu_mbus_state mbus_state;
-
  /*
   * functions
   */
diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h 
b/arch/arm/mach-mvebu/include/mach/cpu.h
index 689c96bd4eac..d9fa1f32aa52 100644
--- a/arch/arm/mach-mvebu/include/mach/cpu.h
+++ b/arch/arm/mach-mvebu/include/mach/cpu.h
@@ -122,9 +122,6 @@ struct sar_freq_modes {
u32 d_clk;
  };
  
-/* Needed for dynamic (board-specific) mbus configuration */

-extern struct mvebu_mbus_state mbus_state;
-
  /*
   * functions
   */
diff --git a/arch/arm/mach-mvebu/mbus.c b/arch/arm/mach-mvebu/mbus.c
index 3b1b9f73ebf6..7092f6cc10c2 100644
--- a/arch/arm/mach-mvebu/mbus.c
+++ b/arch/arm/mach-mvebu/mbus.c
@@ -88,31 +88,34 @@
  
  #define DOVE_DDR_BASE_CS_OFF(n) ((n) << 4)
  
-struct mvebu_mbus_state;

-
-struct mvebu_mbus_soc_data {
-   unsigned int num_wins;
-   unsigned int num_remappable_wins;
-   unsigned int (*win_cfg_offset)(const int win);
-   void (*setup_cpu_target)(struct mvebu_mbus_state *s);
-};
-
-struct mvebu_mbus_state mbus_state
-   __section(".data");
  static struct mbus_dram_target_info mbus_dram_info
__section(".data");
  
+#if defined(CONFIG_ARCH_MVEBU)

+   #define MVEBU_MBUS_NUM_WINS 20
+   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 8
+   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) 
armada_370_xp_mbus_win_offset(win)
+#elif defined(CONFIG_ARCH_KIRKWOOD)
+   #define MVEBU_MBUS_NUM_WINS 8
+   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 4
+   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) orion5x_mbus_win_offset(win)
+#else
+   #error "No supported architecture"
+#endif
+
+static unsigned int armada_370_xp_mbus_win_offset(int win);
+static unsigned int orion5x_mbus_win_offset(int win);
+
  /*
   * Functions to manipulate the address decoding windows
   */
  
-static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,

-  int win, int *enabled, u64 *base,
+static void mvebu_mbus_read_window(int win, int *enabled, u64 *base,
   u32 *size, u8 *target, u8 *attr,
   u64 *remap)
  {
-   void __iomem *addr = mbus->mbuswins_base +
-   mbus->soc->win_cfg_offset(win);
+   void __iomem *addr = (void __iomem *)MVEBU_CPU_WIN_BASE +
+   MVEBU_MBUS_WIN_CFG_OFFSET(win);
u32 basereg = readl(addr + WIN_BASE_OFF);
u32 ctrlreg = readl(addr + WIN_CTRL_OFF);
  
@@ -133,7 +136,7 @@ static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,

*attr = (ctrlreg & WIN_CTRL_ATTR_MASK) >> WIN_CTRL_ATTR_SHIFT;
  
  	if (remap) {

-   if (win < mbus->soc->num_remappable_wins) {
+   if (win < MVEBU_MBUS_NUM_REMAPPABLE_WINS) {
u32 remap_low = readl(addr + WIN_REMAP_LO_OFF);
u32 remap_hi  = readl(addr + WIN_REMAP_HI_OFF);
*remap = ((u64)remap_hi << 32) | remap_low;
@@ -143,27 +146,25 @@ static void mvebu_mbus_read_window(struct 
mvebu_mbus_state *mbus,
}
  }
  
-static void mvebu_mbus_disable_window(struct mvebu_mbus_state *mbus,

- int win)
+static void mvebu_mbus_disable_window(int 

Re: [PATCH] arm: mvebu: mbus: Fix mbus driver to work also after U-Boot relocation

2022-08-10 Thread Chris Packham
Hi Pali,

On 11/08/22 00:46, Pali Rohár wrote:
> mbus driver is initialized from arch_cpu_init() callback which is called
> before relocation. This driver stores lot of functions and structure
> pointers into global variables, so it is data position dependent.
>
> Therefore after relocations all pointers are invalid and driver does not
> work anymore as all pointers referes to the old memory, which overlaps with
> CONFIG_SYS_LOAD_ADDR and ${loadaddr}.
>
> For example U-Boot fuse command crashes if loadaddr memory is cleared or
> rewritten by some image loaded by U-Boot load command.
>
>mw.w ${loadaddr} 0x0 1
>fuse read 0 1 2
>
> Fix this issue by removing of all mbus global variables in which are stored
> pointers to structures or functions which changes during relocation. And
> replace it by direct function calls (not via pointers). With this change
> fuse command finally works.
>
> Signed-off-by: Pali Rohár 

Reviewed-by: Chris Packham 

> ---
>   arch/arm/mach-kirkwood/include/mach/cpu.h |   3 -
>   arch/arm/mach-mvebu/include/mach/cpu.h|   3 -
>   arch/arm/mach-mvebu/mbus.c| 167 +-
>   board/alliedtelesis/x530/x530.c   |   2 +-
>   board/maxbcm/maxbcm.c |   8 +-
>   board/theadorable/theadorable.c   |   4 +-
>   include/linux/mbus.h  |  13 +-
>   7 files changed, 75 insertions(+), 125 deletions(-)
>
> diff --git a/arch/arm/mach-kirkwood/include/mach/cpu.h 
> b/arch/arm/mach-kirkwood/include/mach/cpu.h
> index 71c546f9acf6..d8639c60352b 100644
> --- a/arch/arm/mach-kirkwood/include/mach/cpu.h
> +++ b/arch/arm/mach-kirkwood/include/mach/cpu.h
> @@ -144,9 +144,6 @@ struct kwgpio_registers {
>   u32 irq_level;
>   };
>   
> -/* Needed for dynamic (board-specific) mbus configuration */
> -extern struct mvebu_mbus_state mbus_state;
> -
>   /*
>* functions
>*/
> diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h 
> b/arch/arm/mach-mvebu/include/mach/cpu.h
> index 689c96bd4eac..d9fa1f32aa52 100644
> --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> @@ -122,9 +122,6 @@ struct sar_freq_modes {
>   u32 d_clk;
>   };
>   
> -/* Needed for dynamic (board-specific) mbus configuration */
> -extern struct mvebu_mbus_state mbus_state;
> -
>   /*
>* functions
>*/
> diff --git a/arch/arm/mach-mvebu/mbus.c b/arch/arm/mach-mvebu/mbus.c
> index 3b1b9f73ebf6..7092f6cc10c2 100644
> --- a/arch/arm/mach-mvebu/mbus.c
> +++ b/arch/arm/mach-mvebu/mbus.c
> @@ -88,31 +88,34 @@
>   
>   #define DOVE_DDR_BASE_CS_OFF(n) ((n) << 4)
>   
> -struct mvebu_mbus_state;
> -
> -struct mvebu_mbus_soc_data {
> - unsigned int num_wins;
> - unsigned int num_remappable_wins;
> - unsigned int (*win_cfg_offset)(const int win);
> - void (*setup_cpu_target)(struct mvebu_mbus_state *s);
> -};
> -
> -struct mvebu_mbus_state mbus_state
> - __section(".data");
>   static struct mbus_dram_target_info mbus_dram_info
>   __section(".data");
>   
> +#if defined(CONFIG_ARCH_MVEBU)
> + #define MVEBU_MBUS_NUM_WINS 20
> + #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 8
> + #define MVEBU_MBUS_WIN_CFG_OFFSET(win) 
> armada_370_xp_mbus_win_offset(win)
> +#elif defined(CONFIG_ARCH_KIRKWOOD)
> + #define MVEBU_MBUS_NUM_WINS 8
> + #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 4
> + #define MVEBU_MBUS_WIN_CFG_OFFSET(win) orion5x_mbus_win_offset(win)
> +#else
> + #error "No supported architecture"
> +#endif
> +
> +static unsigned int armada_370_xp_mbus_win_offset(int win);
> +static unsigned int orion5x_mbus_win_offset(int win);
> +
>   /*
>* Functions to manipulate the address decoding windows
>*/
>   
> -static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,
> -int win, int *enabled, u64 *base,
> +static void mvebu_mbus_read_window(int win, int *enabled, u64 *base,
>  u32 *size, u8 *target, u8 *attr,
>  u64 *remap)
>   {
> - void __iomem *addr = mbus->mbuswins_base +
> - mbus->soc->win_cfg_offset(win);
> + void __iomem *addr = (void __iomem *)MVEBU_CPU_WIN_BASE +
> + MVEBU_MBUS_WIN_CFG_OFFSET(win);
>   u32 basereg = readl(addr + WIN_BASE_OFF);
>   u32 ctrlreg = readl(addr + WIN_CTRL_OFF);
>   
> @@ -133,7 +136,7 @@ static void mvebu_mbus_read_window(struct 
> mvebu_mbus_state *mbus,
>   *attr = (ctrlreg & WIN_CTRL_ATTR_MASK) >> WIN_CTRL_ATTR_SHIFT;
>   
>   if (remap) {
> - if (win < mbus->soc->num_remappable_wins) {
> + if (win < MVEBU_MBUS_NUM_REMAPPABLE_WINS) {
>   u32 remap_low = readl(addr + WIN_REMAP_LO_OFF);
>   u32 remap_hi  = readl(addr + WIN_REMAP_HI_OFF);
>   *remap = ((u64)remap_hi << 32) | remap_low;
> @@ -143,27 +146,25 @@ static void mvebu_mbus_read_window(struct 
> mvebu_mbus_state *mbus,
>

[PATCH] arm: mvebu: mbus: Fix mbus driver to work also after U-Boot relocation

2022-08-10 Thread Pali Rohár
mbus driver is initialized from arch_cpu_init() callback which is called
before relocation. This driver stores lot of functions and structure
pointers into global variables, so it is data position dependent.

Therefore after relocations all pointers are invalid and driver does not
work anymore as all pointers referes to the old memory, which overlaps with
CONFIG_SYS_LOAD_ADDR and ${loadaddr}.

For example U-Boot fuse command crashes if loadaddr memory is cleared or
rewritten by some image loaded by U-Boot load command.

  mw.w ${loadaddr} 0x0 1
  fuse read 0 1 2

Fix this issue by removing of all mbus global variables in which are stored
pointers to structures or functions which changes during relocation. And
replace it by direct function calls (not via pointers). With this change
fuse command finally works.

Signed-off-by: Pali Rohár 
---
 arch/arm/mach-kirkwood/include/mach/cpu.h |   3 -
 arch/arm/mach-mvebu/include/mach/cpu.h|   3 -
 arch/arm/mach-mvebu/mbus.c| 167 +-
 board/alliedtelesis/x530/x530.c   |   2 +-
 board/maxbcm/maxbcm.c |   8 +-
 board/theadorable/theadorable.c   |   4 +-
 include/linux/mbus.h  |  13 +-
 7 files changed, 75 insertions(+), 125 deletions(-)

diff --git a/arch/arm/mach-kirkwood/include/mach/cpu.h 
b/arch/arm/mach-kirkwood/include/mach/cpu.h
index 71c546f9acf6..d8639c60352b 100644
--- a/arch/arm/mach-kirkwood/include/mach/cpu.h
+++ b/arch/arm/mach-kirkwood/include/mach/cpu.h
@@ -144,9 +144,6 @@ struct kwgpio_registers {
u32 irq_level;
 };
 
-/* Needed for dynamic (board-specific) mbus configuration */
-extern struct mvebu_mbus_state mbus_state;
-
 /*
  * functions
  */
diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h 
b/arch/arm/mach-mvebu/include/mach/cpu.h
index 689c96bd4eac..d9fa1f32aa52 100644
--- a/arch/arm/mach-mvebu/include/mach/cpu.h
+++ b/arch/arm/mach-mvebu/include/mach/cpu.h
@@ -122,9 +122,6 @@ struct sar_freq_modes {
u32 d_clk;
 };
 
-/* Needed for dynamic (board-specific) mbus configuration */
-extern struct mvebu_mbus_state mbus_state;
-
 /*
  * functions
  */
diff --git a/arch/arm/mach-mvebu/mbus.c b/arch/arm/mach-mvebu/mbus.c
index 3b1b9f73ebf6..7092f6cc10c2 100644
--- a/arch/arm/mach-mvebu/mbus.c
+++ b/arch/arm/mach-mvebu/mbus.c
@@ -88,31 +88,34 @@
 
 #define DOVE_DDR_BASE_CS_OFF(n) ((n) << 4)
 
-struct mvebu_mbus_state;
-
-struct mvebu_mbus_soc_data {
-   unsigned int num_wins;
-   unsigned int num_remappable_wins;
-   unsigned int (*win_cfg_offset)(const int win);
-   void (*setup_cpu_target)(struct mvebu_mbus_state *s);
-};
-
-struct mvebu_mbus_state mbus_state
-   __section(".data");
 static struct mbus_dram_target_info mbus_dram_info
__section(".data");
 
+#if defined(CONFIG_ARCH_MVEBU)
+   #define MVEBU_MBUS_NUM_WINS 20
+   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 8
+   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) 
armada_370_xp_mbus_win_offset(win)
+#elif defined(CONFIG_ARCH_KIRKWOOD)
+   #define MVEBU_MBUS_NUM_WINS 8
+   #define MVEBU_MBUS_NUM_REMAPPABLE_WINS 4
+   #define MVEBU_MBUS_WIN_CFG_OFFSET(win) orion5x_mbus_win_offset(win)
+#else
+   #error "No supported architecture"
+#endif
+
+static unsigned int armada_370_xp_mbus_win_offset(int win);
+static unsigned int orion5x_mbus_win_offset(int win);
+
 /*
  * Functions to manipulate the address decoding windows
  */
 
-static void mvebu_mbus_read_window(struct mvebu_mbus_state *mbus,
-  int win, int *enabled, u64 *base,
+static void mvebu_mbus_read_window(int win, int *enabled, u64 *base,
   u32 *size, u8 *target, u8 *attr,
   u64 *remap)
 {
-   void __iomem *addr = mbus->mbuswins_base +
-   mbus->soc->win_cfg_offset(win);
+   void __iomem *addr = (void __iomem *)MVEBU_CPU_WIN_BASE +
+   MVEBU_MBUS_WIN_CFG_OFFSET(win);
u32 basereg = readl(addr + WIN_BASE_OFF);
u32 ctrlreg = readl(addr + WIN_CTRL_OFF);
 
@@ -133,7 +136,7 @@ static void mvebu_mbus_read_window(struct mvebu_mbus_state 
*mbus,
*attr = (ctrlreg & WIN_CTRL_ATTR_MASK) >> WIN_CTRL_ATTR_SHIFT;
 
if (remap) {
-   if (win < mbus->soc->num_remappable_wins) {
+   if (win < MVEBU_MBUS_NUM_REMAPPABLE_WINS) {
u32 remap_low = readl(addr + WIN_REMAP_LO_OFF);
u32 remap_hi  = readl(addr + WIN_REMAP_HI_OFF);
*remap = ((u64)remap_hi << 32) | remap_low;
@@ -143,27 +146,25 @@ static void mvebu_mbus_read_window(struct 
mvebu_mbus_state *mbus,
}
 }
 
-static void mvebu_mbus_disable_window(struct mvebu_mbus_state *mbus,
- int win)
+static void mvebu_mbus_disable_window(int win)
 {
void __iomem *addr;
 
-   addr = mbus->mbuswins_base + mbus->soc->win_cfg_offset(win);
+   addr =