Re: [PATCH v3 1/8] dt-bindings: reset: Add mt6589 toprgu reset IDs

Hello Krzystof,
thank you for your patience, I really appreciate it.
I think that a lot of my confusion comes from the fact that I'm trying
to follow the structure that is already upstream, but the whole driver
and bindings could probably be refactored in a more straightforward
way.

> So binding already has reset-cells or not?

The watchdog binding at mediatek,mtk-wdt.yaml [1] already declares the
reset-cells property (optional).
Inside this document the various SoC compatibles are divided into two
lists - one that falls back onto "mediatek,mt6589-wdt" and another one
of explicitly declared devices.

If I am not grossly mistaken, looking at the mtk_wdt.c [2] driver that
handles all these compatibles there aren't really any different
behaviors between any of those watchdogs/reset generators.
In every one of them the .data object only provides the total number
of reset bits in each SoC toprgu, plus a single "has_swsysrst_en" flag
for mt7988 only.
For mt6589 the number of resets is missing, which means its reset
controller isn't enabled (and therefore it's not enabled in any of the
"compatible" devices). We know that this SoC supports watchdog resets,
so there really is no difference between all the SoCs. Both the total
number of resets and the "has_swsysrst_en" flag could become
properties in the binding document and be added to device trees.

This change would avoid lots of redundant compatibles, but it would
break existing dtbs and I don't see a way to keep compatibility. Maybe
an entire new binding+driver and deprecating the current one...?
Still I'd feel more comfortable hearing the voice of experts before
proceeding in this direction.

> We do not take bits, but identifiers of resets.

Currently the existing mtk_wdt.c driver does not use a reset table
that binds identifiers to bits for any of the existing devices. There
are a bunch of mediatek,mt*.h files under dt-bindings/reset [3] that
point directly to reset bits instead of being indexes.
I can introduce a reset table in the driver, but it would break
existing devices as those bits are referred in device trees and they
are often non-contiguous (e.g. [4]). As before, it could be done by
creating a new driver and deprecating the current one, but maybe I
completely misunderstood your point, in which case I apologize.

Thank you for your help,

[1] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/watchdog/mediatek%2Cmtk-wdt.yaml
[2] https://github.com/torvalds/linux/blob/master/drivers/watchdog/mtk_wdt.c
[3] https://github.com/torvalds/linux/tree/master/include/dt-bindings/reset
[4] https://github.com/torvalds/linux/blob/master/include/dt-bindings/reset/mt8195-resets.h
--
Luca Leonardo Scorcia
[email protected]