Linux-Watchdog Archive mirror
 help / color / mirror / Atom feed
From: Daniel Golle <daniel@makrotopia.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: devicetree@vger.kernel.org,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@collabora.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	linux-watchdog@vger.kernel.org,
	Guenter Roeck <linux@roeck-us.net>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
	Conor Dooley <conor+dt@kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: watchdog: mediatek,mtk-wdt: add MT7988 watchdog and toprgu
Date: Fri, 10 Nov 2023 20:45:48 +0000	[thread overview]
Message-ID: <ZU6WfOUF7owz7ZLN@makrotopia.org> (raw)
In-Reply-To: <49cd75fd-962f-417c-9196-3c9edd42e4d5@linaro.org>

On Fri, Nov 10, 2023 at 09:00:26PM +0100, Krzysztof Kozlowski wrote:
> On 10/11/2023 16:20, Krzysztof Kozlowski wrote:
> > On 10/11/2023 09:09, Krzysztof Kozlowski wrote:
> >> On 10/11/2023 01:30, Daniel Golle wrote:
> >>> Add binding description for mediatek,mt7988-wdt.
> >>>
> >>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> >>> ---
> >>
> >> ...
> >>
> >>> diff --git a/include/dt-bindings/reset/mediatek,mt7988-resets.h b/include/dt-bindings/reset/mediatek,mt7988-resets.h
> >>> new file mode 100644
> >>> index 0000000000000..fa7c937505e08
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/reset/mediatek,mt7988-resets.h
> >>> @@ -0,0 +1,12 @@
> >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> >>> +
> >>> +/* TOPRGU resets */
> >>> +#define MT7988_TOPRGU_SGMII0_GRST		1
> >>> +#define MT7988_TOPRGU_SGMII1_GRST		2
> >>> +#define MT7988_TOPRGU_XFI0_GRST			12
> >>> +#define MT7988_TOPRGU_XFI1_GRST			13
> >>> +#define MT7988_TOPRGU_XFI_PEXTP0_GRST		14
> >>> +#define MT7988_TOPRGU_XFI_PEXTP1_GRST		15
> >>> +#define MT7988_TOPRGU_XFI_PLL_GRST		16
> >>
> >> IDs should start from 0 or 1 and increment by 1. If these are not IDs,
> >> then you do not need them in the bindings.
> >>
> >> Where is the driver change using these IDs?

It isn't needed as the driver doesn't list the IDs. If that would
be true, it would be sufficient to put them into a header next to the
driver or defined inside the driver C file.

The defined IDs here are intended to be used in device tree files.

> > 
> > You nicely skipped my email and keep pushing the idea of putting this
> > into separate patch.
> > 
> > No. Respond to received comments.
> > 
> >>
> >>> +
> >>> +#define MT7988_TOPRGU_SW_RST_NUM		24
> >>
> >> Why 24? I see 7. 

Because the wdt on MT7988 has a total of 24 resets. Most of them are
(currently, as there are no GPL drops, no publicly available devices,
...) undocumented and are not used in Linux **at this point**. Having
to change the driver every time a new reset is discovered or needed to
be used is tideous, so I thought the best would be -- as we know the
total number of resets -- to already define that, as it's safe to do
and won't need to change.

> >> Why having it in the bindings in the first place.

This line can indeed go into the driver, it's not used anywhere else.
I was merely immitating the style of all the existing binding headers
for similar SoCs and didn't want to stick-out style-wise, also in terms
of the added code to the driver which relies on that number being
defined in the header for all other SoCs.

> >>
> >> It's quite likely I asked the same question about other bindings for
> >> Mediatek. I will be asking every time till this is fixed.
> > 
> > No response to this, either.
> 
> You still did not respond here. To none of the points here. It's my
> third ping because I want this to be resolved. But ignoring my emails,
> and skipping paragraphs of my replies will not lead anywhere.

I have answered to this before:
The driver does NOT have any internal list of names of individual
resets, it relies on the reset number from device tree matching the bit
in the controller, just like for any other MediaTek toprgu already
supported by mtk-wdt.c.

  reply	other threads:[~2023-11-10 20:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-10  0:30 [PATCH 1/2] dt-bindings: watchdog: mediatek,mtk-wdt: add MT7988 watchdog and toprgu Daniel Golle
2023-11-10  0:30 ` [PATCH 2/2] watchdog: mediatek: mt7988: add wdt support Daniel Golle
2023-11-10  5:24   ` Guenter Roeck
2023-11-10 11:55   ` AngeloGioacchino Del Regno
2023-11-10 15:13     ` Guenter Roeck
2023-11-10  8:09 ` [PATCH 1/2] dt-bindings: watchdog: mediatek,mtk-wdt: add MT7988 watchdog and toprgu Krzysztof Kozlowski
2023-11-10 15:20   ` Krzysztof Kozlowski
2023-11-10 20:00     ` Krzysztof Kozlowski
2023-11-10 20:45       ` Daniel Golle [this message]
2023-11-11  7:55         ` Krzysztof Kozlowski
2023-11-10 11:56 ` AngeloGioacchino Del Regno
2023-11-10 14:17   ` Daniel Golle
2023-11-10 14:20     ` Krzysztof Kozlowski
2023-11-10 14:40       ` Daniel Golle
2023-11-10 14:46         ` Krzysztof Kozlowski
2023-11-10 14:51           ` Daniel Golle
2023-11-10 15:07             ` Krzysztof Kozlowski
2023-11-10 15:12               ` Daniel Golle
2023-11-10 15:15                 ` Krzysztof Kozlowski
2023-11-10 15:21                   ` Krzysztof Kozlowski
2023-11-10 17:07                     ` Daniel Golle
2023-11-10 19:58                       ` Krzysztof Kozlowski
2023-11-10 20:18                         ` Daniel Golle
2023-11-10 20:21                           ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZU6WfOUF7owz7ZLN@makrotopia.org \
    --to=daniel@makrotopia.org \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=matthias.bgg@gmail.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robh+dt@kernel.org \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).