[U-Boot] [PATCH v3 02/19] arm: socfpga: Restructure reset manager driver

[Ley Foon Tan]

On Fri, Mar 31, 2017 at 5:19 AM, Dinh Nguyen <dinh.linux at gmail.com> wrote:
> On Thu, Mar 30, 2017 at 8:08 AM, Ley Foon Tan <ley.foon.tan at intel.com> wrote:
>> Restructure reset manager driver in the preparation to support A10.
>> Move the Gen5 specific code to gen5 files. Minor update in socfpga_per_reset().
>> No functional change.
>>
>> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
>> ---
>>  arch/arm/mach-socfpga/Makefile                     |  2 +-
>>  arch/arm/mach-socfpga/include/mach/reset_manager.h | 48 ++---------
>>  .../mach-socfpga/include/mach/reset_manager_gen5.h | 47 +++++++++++
>>  arch/arm/mach-socfpga/reset_manager.c              | 93 +---------------------
>>  .../{reset_manager.c => reset_manager_gen5.c}      | 51 ++++++------
>>  5 files changed, 81 insertions(+), 160 deletions(-)
>>  create mode 100644 arch/arm/mach-socfpga/include/mach/reset_manager_gen5.h
>>  copy arch/arm/mach-socfpga/{reset_manager.c => reset_manager_gen5.c} (75%)
>>
>> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
>> index b76de4c..97819ac 100644
>> --- a/arch/arm/mach-socfpga/Makefile
>> +++ b/arch/arm/mach-socfpga/Makefile
>> @@ -14,7 +14,7 @@ obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o
>
> [snip]
>>
>> -void socfpga_bridges_reset(int enable);
>> +int socfpga_bridges_reset(int enable);
>
> You're changing the return type, to me, that's a functional change.
Will update the commit message.
>
> [snip]
>
>>  /*
>>   * Write the reset manager register to cause reset
>> @@ -61,8 +20,8 @@ void socfpga_per_reset_all(void)
>>  void reset_cpu(ulong addr)
>>  {
>>         /* request a warm reset */
>> -       writel((1 << RSTMGR_CTRL_SWWARMRSTREQ_LSB),
>> -               &reset_manager_base->ctrl);
>> +       writel(1 << RSTMGR_CTRL_SWWARMRSTREQ_LSB,
>> +              &reset_manager_base->ctrl);
>
> What's the reason for the above change?
Requested by Marek to remove extra parenthesis.
>
> [snip]
>>
>> @@ -90,9 +84,9 @@ void socfpga_bridges_reset(int enable)
>>  #define L3REGS_REMAP_HPS2FPGA_MASK     0x08
>>  #define L3REGS_REMAP_OCRAM_MASK                0x01
>>
>> -void socfpga_bridges_reset(int enable)
>> +int socfpga_bridges_reset(int enable)
>>  {
>> -       const uint32_t l3mask = L3REGS_REMAP_LWHPS2FPGA_MASK |
>> +       const u32 l3mask = L3REGS_REMAP_LWHPS2FPGA_MASK |
>>                                 L3REGS_REMAP_HPS2FPGA_MASK |
>>                                 L3REGS_REMAP_OCRAM_MASK;
>>
>> @@ -107,7 +101,7 @@ void socfpga_bridges_reset(int enable)
>>                 if (!fpgamgr_test_fpga_ready()) {
>>                         /* FPGA not ready, do nothing. */
>>                         printf("%s: FPGA not ready, aborting.\n", __func__);
>> -                       return;
>> +                       return -EINVAL;
>>                 }
>>
>>                 /* brdmodrst */
>> @@ -116,5 +110,6 @@ void socfpga_bridges_reset(int enable)
>>                 /* Remap the bridges into memory map */
>>                 writel(l3mask, SOCFPGA_L3REGS_ADDRESS);
>>         }
>> +       return 0;
>
> How are you intending to use these return values?
Will display error when error and hang().

Regards
Ley Foon