Re: [PATCH v3 2/2] watchdog: mediatek: mt7988: add wdt support
From: Daniel Golle
Date: Mon Nov 20 2023 - 12:33:56 EST
On Mon, Nov 20, 2023 at 09:19:46AM -0800, Guenter Roeck wrote:
> On 11/14/23 09:04, Daniel Golle wrote:
> > [...]
> > @@ -89,6 +93,11 @@ static const struct mtk_wdt_data mt7986_data = {
> > .toprgu_sw_rst_num = MT7986_TOPRGU_SW_RST_NUM,
> > };
> > +static const struct mtk_wdt_data mt7988_data = {
> > + .toprgu_sw_rst_num = 24,
>
> Kind of odd to have this defined locally, while the others are in include files,
> but not worth arguing about.
>From I have just learned from Krzysztof Kozlowski those headers shouldn't
even exist in first place, as the listed IDs are not actually referenced
anywhere in the driver, hence they aren't actually bindings [1].
Quote from that thread:
| >>> Where is the driver change using these IDs?
| >> It isn't needed as the driver doesn't list the IDs. [...]
| > Then it is not a binding.
---
Now that they do exist it's too late to change that for everything
already existing, I suppose. However, it also doesn't seem like adding
such a header for MT7988 as well is going to be acknowledged, hence we
will have to live with the inconsistency in the driver in which older
SoCs will obtain the number of resets from a macro in their respective
dt-bindings header while newer SoCs won't have such header and hence
it will have to be defined in the driver itself (as that's also the
only place where that number is being used).
> Please make it a define at the top of the file, though.
Ack. Will do.
[1]: https://lore.kernel.org/lkml/6912f6f406bc45674020681184f3eeca2f2cb63f.1699576174.git.daniel@xxxxxxxxxxxxxx/T/#ef01d7efc6c4fbf1d4830bafe7c90e39746939671