[U-Boot] [PATCH] reset: socfpga: release more A10 peripherals out of reset

[Simon Goldschmidt]

Am 24.05.2019 um 14:16 schrieb Chee, Tien Fong:
> On Fri, 2019-05-24 at 14:00 +0200, Simon Goldschmidt wrote:
>> On Fri, May 24, 2019 at 1:57 PM Chee, Tien Fong
>> <tien.fong.chee at intel.com> wrote:
>>>
>>>
>>> On Fri, 2019-05-24 at 13:53 +0200, Simon Goldschmidt wrote:
>>>>
>>>> On Fri, May 24, 2019 at 1:44 PM Marek Vasut <marex at denx.de>
>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 5/24/19 1:40 PM, tien.fong.chee at intel.com wrote:
>>>>>>
>>>>>>
>>>>>> From: Tien Fong Chee <tien.fong.chee at intel.com>
>>>>>>
>>>>>> Current implementation almost release all peripherals out of
>>>>>> reset for
>>>>>> gen5, but A10 has more peripherals than gen5, hence this
>>>>>> patch is
>>>>>> required
>>>>>> to release the rest of peripherals to support old kernels.
>>>>>>
>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>

Even if this is old, I'm ok with it, since it's only "temporary" until 
the Linux drivers can handle RST, so:

Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>

>>>>>> ---
>>>>>>   drivers/reset/reset-socfpga.c | 3 +++
>>>>>>   1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/reset/reset-socfpga.c
>>>>>> b/drivers/reset/reset-
>>>>>> socfpga.c
>>>>>> index cb8312619f..d8b8b25fc3 100644
>>>>>> --- a/drivers/reset/reset-socfpga.c
>>>>>> +++ b/drivers/reset/reset-socfpga.c
>>>>>> @@ -127,6 +127,9 @@ static int socfpga_reset_remove(struct
>>>>>> udevice *dev)
>>>>>>        if (socfpga_reset_keep_enabled()) {
>>>>>>                puts("Deasserting all peripheral resets\n");
>>>>>>                writel(0, data->modrst_base + 4);
>>>>>> +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
>>>>>> +             writel(0, data->modrst_base + 8);
>>>>> This should be using match on compatible string.
>>>>> Which register is this modrst_base + 8 ?
>>>> Well, even modrst_base + 4 is not really ideal. After all,
>>>> modrst_base is a
>>>> dts-given offset into the rstmgr's registers, so there is not
>>>> really
>>>> a
>>>> comptile time known constant to do this.
>>>>
>>>> Maybe it would be better to use an offset to the device's
>>>> membase.
>>>> However, as this is a backwards-compatibility workaround, there's
>>>> no
>>>> use to
>>>> add this into the devicetree.
>>>>
>>>> And unfortunately, both gen5 and a10 use the same compatible
>>>> string,
>>>> which
>>>> probably would have to be changed.
>>>>
>>>> Regards,
>>>> Simon
>>> This register is per1modrst, here is the link https://www.intel.com
>>> /con
>>> tent/www/us/en/programmable/hps/arria-
>>> 10/hps.html#reg_soc_top/sfo1429890576520.html
>>> This whole mechanism should be removed once Dinh has fixed this
>>> issue
>>> at Linux v4.14-lts or moving to Linux v5.0 onwards.
>> I agree it can be removed in the future, but is it really enough to
>> have this
>> in 4.14-lts? I'm not so sure that we should prevent people from using
>> mainline U-Boot with older kernels...
> 
> I have no strong opinion on this, but how old the kernel U-Boot want to
> support, subject to the maintainer :) .
> 
>>
>> Regards,
>> Simon
>>
>>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>> +#endif
>>>>>>        }
>>>>>>
>>>>>>        return 0;
>>>>>>
>>>>> --
>>>>> Best regards,
>>>>> Marek Vasut