Hi Simon,
On Tue, Mar 13, 2018 at 9:05 PM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
> On Thu, Mar 01, 2018 at 11:20:11AM +0100, Geert Uytterhoeven wrote:
>> On Fri, Feb 23, 2018 at 9:14 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
>> > On Thu, Feb 22, 2018 at 09:38:39AM +0100, Geert Uytterhoeven wrote:
>> >> On Wed, Feb 21, 2018 at 7:32 PM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
>> >> > On Wed, Feb 21, 2018 at 05:30:12PM +0100, Geert Uytterhoeven wrote:
>> >> >> On Wed, Feb 21, 2018 at 5:13 PM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
>> >> >> > On Tue, Feb 20, 2018 at 01:51:28PM +0100, Geert Uytterhoeven wrote:
>> >> >> >> On Mon, Feb 12, 2018 at 6:44 PM, Fabrizio Castro
>> >> >> >> <fabrizio.castro@xxxxxxxxxxxxxx> wrote:
>> >> >> >> > this series has been around for some time as RFC, and it has collected
>> >> >> >> > useful comments from the community along the way.
>> >> >> >> > The solution proposed by this patch set works for most R-Car Gen2 and
>> >> >> >> > RZ/G1 devices, but not all of them. We now know that for some R-Car
>> >> >> >> > Gen2 early revisions there is no proper software fix. Anyway, no
>> >> >> >> > product has been built around early revisions, but development boards
>> >> >> >> > mounting early revisions (basically prototypes) are still out there.
>> >> >> >> > As a result, this series isn't enabling the internal watchdog on R-Car
>> >> >> >> > Gen2 boards, developers may enable it in board specific device trees
>> >> >> >> > if needed.
>> >> >> >> > This series has been tested by me on the iwg20d, iwg22d, Lager, Alt,
>> >> >> >> > and Koelsch boards.
>> >> >> >> >
>> >> >> >> > The problem
>> >> >> >> > ===========
>> >> >> >> > To deal with SMP on R-Car Gen2 and RZ/G1, we install a reset vector
>> >> >> >> > to ICRAM1 and we program the [S]BAR registers so that when we turn ON
>> >> >> >> > the non-boot CPUs they are redirected to the reset vector installed by
>> >> >> >> > Linux in ICRAM1, and eventually they continue the execution to RAM,
>> >> >> >> > where the SMP bring-up code will take care of the rest.
>> >> >> >> > The content of the [S]BAR registers survives a watchdog triggered reset,
>> >> >> >> > and as such after the watchdog fires the boot core will try and execute
>> >> >> >> > the SMP bring-up code instead of jumping to the bootrom code.
>> >> >> >> >
>> >> >> >> > The fix
>> >> >> >> > =======
>> >> >> >> > The main strategy for the solution is to let the reset vector decide
>> >> >> >> > if it needs to jump to shmobile_boot_fn or to the bootrom code.
>> >> >> >> > In a watchdog triggered reset scenario, since the [S]BAR registers keep
>> >> >> >> > their values, the boot CPU will jump into the newly designed reset
>> >> >> >> > vector, the assembly routine will eventually test WOVF (a bit in register
>> >> >> >> > RWTCSRA that indicates if the watchdog counter has overflown, the value
>> >> >> >> > of this bit gets retained in this scenario), and jump to the bootrom code
>> >> >> >> > which will in turn load up the bootloader, etc.
>> >> >> >> > When bringing up SMP or using CPU hotplug, the reset vector will jump
>> >> >> >> > to shmobile_boot_fn instead.
>> >> >> >> >
>> >> >> >> > Thank you All for your help.
>> >> >> >> >
>> >> >> >> > Best regards,
>> >> >> >> >
>> >> >> >> > Fabrizio Castro (26):
>> >> >> >> > ARM: shmobile: Add watchdog support
>> >> >> >> > ARM: dts: r8a7743: Adjust SMP routine size
>> >> >> >> > ARM: dts: r8a7745: Adjust SMP routine size
>> >> >> >> > ARM: dts: r8a7790: Adjust SMP routine size
>> >> >> >> > ARM: dts: r8a7791: Adjust SMP routine size
>> >> >> >> > ARM: dts: r8a7792: Adjust SMP routine size
>> >> >> >> > ARM: dts: r8a7793: Adjust SMP routine size
>> >> >> >> > ARM: dts: r8a7794: Adjust SMP routine size
>> >> >> >> > soc: renesas: rcar-rst: Enable watchdog as reset trigger for Gen2
>> >> >> >> > ARM: shmobile: rcar-gen2: Add watchdog support
>> >> >> >> > dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support
>> >> >> >> > watchdog: renesas_wdt: Add R-Car Gen2 support
>> >> >> >> > watchdog: renesas_wdt: Add restart handler
>> >> >> >> > ARM: shmobile: defconfig: Enable RENESAS_WDT_GEN
>> >> >> >> > clk: renesas: r8a7743: Add rwdt clock
>> >> >> >> > clk: renesas: r8a7745: Add rwdt clock
>> >> >> >> > clk: renesas: r8a7790: Add rwdt clock
>> >> >> >> > clk: renesas: r8a7791/r8a7793: Add rwdt clock
>> >> >> >> > clk: renesas: r8a7794: Add rwdt clock
>> >> >> >> > ARM: dts: r8a7743: Add watchdog support to SoC dtsi
>> >> >> >> > ARM: dts: r8a7745: Add watchdog support to SoC dtsi
>> >> >> >> > ARM: dts: r8a7790: Add watchdog support to SoC dtsi
>> >> >> >> > ARM: dts: r8a7791: Add watchdog support to SoC dtsi
>> >> >> >> > ARM: dts: r8a7794: Add watchdog support to SoC dtsi
>> >> >> >> > ARM: dts: iwg20m: Add watchdog support to SoM dtsi
>> >> >> >> > ARM: dts: iwg22m: Add watchdog support to SoM dtsi
>> >> >> >> >
>> >> >> >> > .../devicetree/bindings/watchdog/renesas-wdt.txt | 19 ++++++--
>> >> >> >> > arch/arm/boot/dts/r8a7743-iwg20m.dtsi | 5 ++
>> >> >> >> > arch/arm/boot/dts/r8a7743.dtsi | 12 ++++-
>> >> >> >> > arch/arm/boot/dts/r8a7745-iwg22m.dtsi | 5 ++
>> >> >> >> > arch/arm/boot/dts/r8a7745.dtsi | 12 ++++-
>> >> >> >> > arch/arm/boot/dts/r8a7790.dtsi | 12 ++++-
>> >> >> >> > arch/arm/boot/dts/r8a7791.dtsi | 12 ++++-
>> >> >> >> > arch/arm/boot/dts/r8a7792.dtsi | 2 +-
>> >> >> >> > arch/arm/boot/dts/r8a7793.dtsi | 2 +-
>> >> >> >> > arch/arm/boot/dts/r8a7794.dtsi | 12 ++++-
>> >> >> >> > arch/arm/configs/shmobile_defconfig | 1 +
>> >> >> >> > arch/arm/mach-shmobile/common.h | 6 +++
>> >> >> >> > arch/arm/mach-shmobile/headsmp.S | 55 ++++++++++++++++++++++
>> >> >> >> > arch/arm/mach-shmobile/platsmp-apmu.c | 1 +
>> >> >> >> > arch/arm/mach-shmobile/pm-rcar-gen2.c | 15 ++++--
>> >> >> >> > drivers/clk/renesas/r8a7743-cpg-mssr.c | 2 +
>> >> >> >> > drivers/clk/renesas/r8a7745-cpg-mssr.c | 2 +
>> >> >> >> > drivers/clk/renesas/r8a7790-cpg-mssr.c | 2 +
>> >> >> >> > drivers/clk/renesas/r8a7791-cpg-mssr.c | 2 +
>> >> >> >> > drivers/clk/renesas/r8a7794-cpg-mssr.c | 2 +
>> >> >> >> > drivers/soc/renesas/rcar-rst.c | 35 +++++++++++---
>> >> >> >> > drivers/watchdog/renesas_wdt.c | 39 +++++++++++++--
>> >> >> >>
>> >> >> >> Thanks, I've queued the clock patches in clk-renesas-for-v4.17, as they are
>> >> >> >> a hard dependency for:
>> >> >> >> (1) The new reset vector in arch/arm/mach-shmobile/
>> >> >> >> (2) The watchdog driver.
>> >> >> >>
>> >> >> >> Note that the watchdog driver itself (2) has a hard dependency on the
>> >> >> >> new reset vector (1).
>> >> >> >
>> >> >> > Thanks, I have marked the watchdog driver patch as Deferred.
>> >> >> > Please resubmit or otherwise ping me once they dependencies
>> >> >> > are in an rc release.
>> >> >>
>> >> >> I gave the dependencies a bit more thought.
>> >> >>
>> >> >> I think we can gain once release cycle if you would postpone the DTS patches
>> >> >> (both SMP routine size adjustments and RWDT device node additions) to v4.18.
>> >> >> Then all other parts can be upstreamed in parallel in v4.17, as nothing
>> >> >> will be activated before the DTS parts are in.
>> >> >>
>> >> >> Does this look sane?
>> >> >
>> >> > What about new DTS on old kernels of a specific vintage?
>> >>
>> >> New DTS would work fine on v4.16 and older, which don't have any of the
>> >> other patches from this series.
>> >> New DTS would work fine on v4.17, assumed all other patches from this
>> >> series get into v4.17.
>> >>
>> >> The only issue would be a new DTS on a kernel that has some but not all
>> >> other patches from this series.
>> >
>> > Thanks for the analysis. I think this is the key point. What is the risk of
>> > that happening?
>>
>> I think the risk is fairly low.
>> You control when the DTS changes go in, and also when they are backported
>> to LTSI.
>>
>> >> If we want to fast-track everything into v4.17, we need a stable branch, to be
>> >> pulled by all 5 actors (clk-renesas, mach-shmobile, soc-renesas, watchdog,
>> >> renesas-dts).
>> >
>> > It seems that watchdog is the only item on that list that neither you nor I
>> > sign off on. So I conclude that if the watchdog patch goes in early enough
>> > in the cycle then we ought to be in good shape.
>> >
>> > None the less, is it worth the risk?
>>
>> Do we want the functionality early, or can we wait until v4.19?
>
> I understand that the watchdog driver has been accepted and that as things
> currently stand if it is enabled in the build it will be enabled in DT and
> a crash will result. From my POV that is a regression so I have decided
> to adopt the alternate plan proposed by Geert above: in short drop DT
> patches (to avoid enabling broken driver) and enqueue mach-shmobile
> patches (which don't make things any worse.
>
> Patch-by-patch things are as follows.
>
> Dropped from "[PATCH v5 00/26] Fix watchdog on Renesas R-Car Gen2 and RZ/G1":
>
> 979d28f29742 ARM: dts: r8a7743: Adjust SMP routine size
> 0b205f679f5d ARM: dts: r8a7745: Adjust SMP routine size
> 2ad21a1d78b3 ARM: dts: r8a7790: Adjust SMP routine size
> 82c978459c77 ARM: dts: r8a7791: Adjust SMP routine size
> d303698e6ff3 ARM: dts: r8a7792: Adjust SMP routine size
> d8b8b9a1e295 ARM: dts: r8a7793: Adjust SMP routine size
> 70b3ab369773 ARM: dts: r8a7794: Adjust SMP routine size
>
> ebf26cf1b1de ARM: dts: r8a7743: Add watchdog support to SoC dtsi
> 5a4566ab3777 ARM: dts: r8a7745: Add watchdog support to SoC dtsi
> acd58e2bb1e5 ARM: dts: r8a7790: Add watchdog support to SoC dtsi
> e3f89168e72f ARM: dts: r8a7791: Add watchdog support to SoC dtsi
> e76a6a69a1fc ARM: dts: r8a7794: Add watchdog support to SoC dtsi
> 242cc1ab7f7c ARM: dts: iwg20m: Add watchdog support to SoM dtsi
> 748ed07f6c7c ARM: dts: iwg22m: Add watchdog support to SoM dtsi
>
>
> Dropped from "[PATCH/RFC 00/11] ARM: dts: rcar-gen2: Enable watchdog support":
>
> c257202482e2 ARM: dts: r8a7792: Add RWDT node
> 01ec04dec8eb ARM: dts: r8a7793: Add RWDT node
> 8e6ebe4f2f94 ARM: dts: lager: Enable watchdog support
> f223ad0198f6 ARM: dts: koelsch: Enable watchdog support
> 1d14d2bb700d ARM: dts: porter: Enable watchdog support
> bf71652fc955 ARM: dts: blanche: Enable watchdog support
> 13a0b10aeea4 ARM: dts: wheat: Enable watchdog support
> 4e247bf110b8 ARM: dts: gose: Enable watchdog support
> 2dcbd24bbf5e ARM: dts: alt: Enable watchdog support
> 7f6844c900b4 ARM: dts: silk: Enable watchdog support
>
> And applied:
>
> [v5] ARM: shmobile: rcar-gen2: Add watchdog support
> [v5] ARM: shmobile: Add watchdog support
>
> In the case of the last patch above I removed the #ifdef from the header file.
Thank you, sounds good to me.
BTW, who will take "dt-bindings: watchdog: renesas-wdt: Add R-Car Gen2 support"?
Günter, Wim: will you take this through the watchdog tree, too?
Thanks again!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
