On 3/1/19 1:26 PM, Simon Goldschmidt wrote:
> On Fri, Mar 1, 2019 at 1:15 PM Marek Vasut <ma...@denx.de> wrote:
>>
>> On 3/1/19 1:14 PM, Simon Goldschmidt wrote:
>>> On Fri, Mar 1, 2019 at 1:00 PM Marek Vasut <ma...@denx.de> wrote:
>>>>
>>>> On 2/26/19 9:31 PM, Simon Goldschmidt wrote:
>>>>> This adds code to take peripherals out of reset based on an environment
>>>>> variable. This is in preparation for removing the code that does this from
>>>>> SPL.
>>>>>
>>>>> However, some drivers even in current Linux cannot handle peripheral
>>>>> reset,
>>>>> so until this works, we need a compatibility workaround.
>>>>>
>>>>> This workaround is implemented in the 'assert' and 'remove' callbacks of
>>>>> this reset driver: the 'assert' callback does not disable peripherals that
>>>>> were already taken out of reset, while the 'remove' callback, which is
>>>>> called on OS_PREPARE, deasserts all peripheral resets if the environment
>>>>> variable "socfpga_legacy_reset_compat" is set to 1, which is what the gen5
>>>>> SPL did up to now.
>>>>>
>>>>> This is in preparation to clean up the SPL and implementing proper reset
>>>>> handling for U-Boot.
>>>>>
>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>>>> - fix falcon mode in SPL should work, too
>>>>> - change env var name "socfpga_permodrst_ungate" to
>>>>> "socfpga_legacy_reset_compat"
>>>>> - in compat mode, don't reset peripherals once they are enabled
>>>>>
>>>>> Changes in v2:
>>>>> - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment
>>>>> variable "socfpga_permodrst_ungate"
>>>>>
>>>>> drivers/reset/reset-socfpga.c | 44 +++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 44 insertions(+)
>>>>>
>>>>> diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
>>>>> index b2acfcd2ec..39d0b9e8f2 100644
>>>>> --- a/drivers/reset/reset-socfpga.c
>>>>> +++ b/drivers/reset/reset-socfpga.c
>>>>> @@ -27,6 +27,36 @@ struct socfpga_reset_data {
>>>>> void __iomem *membase;
>>>>> };
>>>>>
>>>>> +/*
>>>>> + * For compatibility with Kernels that don't support peripheral reset,
>>>>> this
>>>>> + * driver can keep the old behaviour of not asserting peripheral reset
>>>>> before
>>>>> + * starting the OS and deasserting all peripheral resets (enabling all
>>>>> + * peripherals).
>>>>> + *
>>>>> + * For that, the reset driver checks the environment variable
>>>>> + * "socfpga_legacy_reset_compat". If this variable is '1', perihperals
>>>>> are not
>>>>> + * reset again once taken out of reset and all peripherals in
>>>>> 'permodrst' are
>>>>> + * taken out of reset before booting into the OS.
>>>>> + * Note that this should be required for gen5 systems only that are
>>>>> running
>>>>> + * Linux kernels without proper peripheral reset support for all drivers
>>>>> used.
>>>>> + */
>>>>> +static bool socfpga_reset_keep_enabled(void)
>>>>> +{
>>>>> +#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
>>>>> + const char *env_str;
>>>>> + long val;
>>>>> +
>>>>> + env_str = env_get("socfpga_legacy_reset_compat");
>>>>> + if (env_str) {
>>>>> + val = simple_strtol(env_str, NULL, 0);
>>>>> + if (val == 1)
>>>>> + return true;
>>>>> + }
>>>>> +#endif
>>>>> +
>>>>> + return false;
>>>>> +}
>>>>> +
>>>>> static int socfpga_reset_assert(struct reset_ctl *reset_ctl)
>>>>> {
>>>>> struct socfpga_reset_data *data = dev_get_priv(reset_ctl->dev);
>>>>> @@ -89,6 +119,18 @@ static int socfpga_reset_probe(struct udevice *dev)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> +static int socfpga_reset_remove(struct udevice *dev)
>>>>> +{
>>>>> + struct socfpga_reset_data *data = dev_get_priv(dev);
>>>>> +
>>>>> + if (socfpga_reset_keep_enabled()) {
>>>>> + puts("Deasserting all peripheral resets\n");
>>>>> + writel(0, data->membase + 4);
>>>>
>>>> Isn't permodreset at +0x14 ?
>>>
>>> Yes it is. However, data->membase does not point to the actual base
>>> address of the rstmgr but to the start of the "modrst" register group.
>>>
>>> And permodreset is at offset 4 in this view.
>>>
>>> While this looks a bit odd, it ensures the driver can be used for gen5
>>> and a10 (and proably for s10 as well).
>>>
>>
>> Some comment explaining this would be nice :)
>
> Well, I guess you're right there. I found that odd myself when starting
> to read the driver. But then I just ended copying the code from the other
> callbacks.
>
> It would probably be a good idea to rename 'membase' to something
> more appropriate given its usage...
>
> So I'll send v4 soon ;-)
Please do, otherwise the patchset is good :)
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
