[U-Boot] [PATCH 06/10] arm: socfpga: arria10: Added drivers for Arria10 Reset Manager

[Marek Vasut]

On 12/19/2016 07:53 AM, Chee, Tien Fong wrote:
> On Jum, 2016-12-09 at 13:51 +0100, Marek Vasut wrote:
>> On 12/09/2016 11:04 AM, Chee, Tien Fong wrote:
>>>
>>> On Rab, 2016-12-07 at 14:58 +0100, Marek Vasut wrote:
>>>>
>>>> On 12/07/2016 12:58 PM, Chee, Tien Fong wrote:
>>>>>
>>>>>
>>>>> On Sel, 2016-12-06 at 13:55 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>> On 12/06/2016 09:08 AM, Chee Tien Fong wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> From: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>>>
>>>>>>> Drivers for reset manager is restructured such that common
>>>>>>> functions,
>>>>>>> gen5 drivers and Arria10 drivers are moved to
>>>>>>> reset_manager.c,
>>>>>>> reset_manager_gen5.c and reset_manager_arria10.c
>>>>>>> respectively.
>>>>>>>
>>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>>> Cc: Marek Vasut <marex at denx.de>
>>>>>>> Cc: Dinh Nguyen <dinguyen at kernel.org>
>>>>>>> Cc: Chin Liang See <chin.liang.see at intel.com>
>>>>>>> Cc: Tien Fong <skywindctf at gmail.com>
>>>> [...]
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> +void reset_deassert_dedicated_peripherals(void)
>>>>>>> +{
>>>>>>> +	int i;
>>>>>>> +	u32 mask0 = 0;
>>>>>>> +	u32 mask1 = 0;
>>>>>>> +	u32 pinmux_addr =
>>>>>>> SOCFPGA_PINMUX_DEDICATED_IO_ADDRESS;
>>>>>>> +	u32 mask = 0;
>>>>>>> +#if defined(CONFIG_MMC)
>>>>>>> +	mask |= ALT_RSTMGR_PER0MODRST_SDMMCECC_SET_MSK;
>>>>>>> +#elif defined(CONFIG_CADENCE_QSPI)
>>>>>>> +	mask |= ALT_RSTMGR_PER0MODRST_QSPIECC_SET_MSK;
>>>>>>> +#elif defined(CONFIG_NAND_DENALI)
>>>>>>> +	mask |= ALT_RSTMGR_PER0MODRST_NANDECC_SET_MSK;
>>>>>>> +#else
>>>>>> Shouldn't this come from OF instead of being ifdef'd ?
>>>>>>
>>>>> What is OF?
>>>> Device Tree (Open Firmware).
>>>>
>>>>>
>>>>>
>>>>> what is your suggestion to make this function generic for
>>>>> all type of flash?
>>>> Pull it from OF ?
>>>>
>>> Why you prefer device tree implementation over #define in
>>> defconfig,
>>> because there is performance penalty.
>> Because we are moving away from excessive random #defines and toward
>> having one single binary where you could exchange just the DT and run
>> it on multiple boards, just like Linux, that is the ultimate goal.
>>
>> Also, this is not performance critical code, is it.
>>
> This code just to release peripherals from reset, not performance
> critical codes. However, our defconfigs based on flash type booting, so
> this is why i din't use the DT, since we have flash type determined
> from defconfig. Since DT is ultimate goal, i can change to DT
> implementation.

You should really only unreset the peripherals which you need and,
ideally, only when you need them. So it should be the driver which
handles the peripheral reset, not the common code (unless there is
a reason for the common code to do it).

-- 
Best regards,
Marek Vasut