Re: [PATCH v3 2/2] watchdog: mediatek: mt7988: add wdt support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/20/23 09:33, Daniel Golle wrote:
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).


As I said, not worth arguing about. However, it seems to me that "too late
to change that for everything" isn't really correct. If MTxxxx_TOPRGU_RST_NUM
isn't supposed to be in devicetree include files, all those defines could be
removed from the from there and be added to the watchdog driver. I don't know
about the other defines in include/dt-bindings/reset/mediatek,mtXXXX-resets.h -
many of those _are_ used in dtsi files, but many others are not.

In summary, while I don't really know/understand what is supposed to be defined
in include/dt-bindings/, whatever is known to _not_ to be there (such as the
total number of reset pins on a SoC) could be moved into the driver(s) using it.

Of course, it might well be that there is a rule saying that anything in
include/dt-bindings/ must not be removed from it even if it is not supposed
to be there. In that case, my apologies for the noise.

Thanks,
Guenter




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux