| Message ID | 20230526062626.1180-1-bbhushan2@marvell.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [1/2,v8] dt-bindings: watchdog: marvell GTI system watchdog driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
Yo Bharat, On Fri, May 26, 2023 at 11:56:25AM +0530, Bharat Bhushan wrote: > Add binding documentation for the Marvell GTI system > watchdog driver. > > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> > --- > v8: > - Compatible name as per soc name I am sorry, but I do not understand this. > > .../watchdog/marvell,octeontx2-wdt.yaml | 73 +++++++++++++++++++ > 1 file changed, 73 insertions(+) > create mode 100644 Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml > > diff --git a/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml > new file mode 100644 > index 000000000000..3c642359d960 > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml > @@ -0,0 +1,73 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/watchdog/marvell,octeontx2-wdt.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Marvell Global Timer (GTI) system watchdog > + > +allOf: > + - $ref: watchdog.yaml# From v7: Put allOf after maintainers:. > + > +maintainers: > + - Bharat Bhushan <bbhushan2@marvell.com> > + > +properties: > + compatible: > + enum: > + - marvell,cn9670-wdt > + - marvell,cn9880-wdt > + - marvell,cnf9535-wdt > + - marvell,cn10624-wdt > + - marvell,cn10308-wdt > + - marvell,cnf10518-wdt static const struct of_device_id gti_wdt_of_match[] = { { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, { .compatible = "marvell,cn9880-wdt", .data = &match_data_octeontx2}, { .compatible = "marvell,cnf9535-wdt", .data = &match_data_octeontx2}, { .compatible = "marvell,cn10624-wdt", .data = &match_data_cn10k}, { .compatible = "marvell,cn10308-wdt", .data = &match_data_cn10k}, { .compatible = "marvell,cnf10518-wdt", .data = &match_data_cn10k}, This is a fat hint that you should be using fallback compatibles here. You even had a fallback setup in your last revision, but you seem to have removed it alongside the removal of the wildcards. Why did you do that? > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + minItems: 1 From v7: maxItems instead. You see it is different than above properties? > + > + clock-names: > + minItems: 1 From v7: Need to define names. Cheers, Conor.
Hi Conor, Please see inline > -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: Saturday, May 27, 2023 1:07 AM > To: Bharat Bhushan <bbhushan2@marvell.com> > Cc: wim@linux-watchdog.org; linux@roeck-us.net; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org; linux-watchdog@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Sunil Kovvuri > Goutham <sgoutham@marvell.com> > Subject: [EXT] Re: [PATCH 1/2 v8] dt-bindings: watchdog: marvell GTI system > watchdog driver > > External Email > > ---------------------------------------------------------------------- > Yo Bharat, > > On Fri, May 26, 2023 at 11:56:25AM +0530, Bharat Bhushan wrote: > > Add binding documentation for the Marvell GTI system watchdog driver. > > > > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> > > --- > > v8: > > - Compatible name as per soc name > > I am sorry, but I do not understand this. I mean to say replaced soc family name from compatible with actual soc names > > > > > .../watchdog/marvell,octeontx2-wdt.yaml | 73 +++++++++++++++++++ > > 1 file changed, 73 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yam > > l > > b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yam > > l > > new file mode 100644 > > index 000000000000..3c642359d960 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt > > +++ .yaml > > @@ -0,0 +1,73 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +http://devicetree.org/schemas/watchdog/marvell,octeontx2-wdt.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Marvell Global Timer (GTI) system watchdog > > + > > +allOf: > > + - $ref: watchdog.yaml# > > From v7: > Put allOf after maintainers:. Thanks for pointing, I am sorry, missed almost all comments on v7. Will resolve this and below as well in next version > > > + > > +maintainers: > > + - Bharat Bhushan <bbhushan2@marvell.com> > > + > > +properties: > > + compatible: > > + enum: > > + - marvell,cn9670-wdt > > + - marvell,cn9880-wdt > > + - marvell,cnf9535-wdt > > + - marvell,cn10624-wdt > > + - marvell,cn10308-wdt > > + - marvell,cnf10518-wdt > > static const struct of_device_id gti_wdt_of_match[] = { > { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, > { .compatible = "marvell,cn9880-wdt", .data = &match_data_octeontx2}, > { .compatible = "marvell,cnf9535-wdt", .data = &match_data_octeontx2}, > { .compatible = "marvell,cn10624-wdt", .data = &match_data_cn10k}, > { .compatible = "marvell,cn10308-wdt", .data = &match_data_cn10k}, > { .compatible = "marvell,cnf10518-wdt", .data = &match_data_cn10k}, > > This is a fat hint that you should be using fallback compatibles here. > You even had a fallback setup in your last revision, but you seem to have > removed it alongside the removal of the wildcards. Why did you do that? Not sure I understand this comment, Compatible in last version was as below: + properties: + compatible: + oneOf: + - const: marvell,octeontx2-wdt + - items: + - enum: + - marvell,octeontx2-95xx-wdt + - marvell,octeontx2-96xx-wdt + - marvell,octeontx2-98xx-wdt + - const: marvell,octeontx2-wdt + - const: marvell,cn10k-wdt + - items: + - enum: + - marvell,cn10kx-wdt + - marvell,cnf10kx-wdt + - const: marvell,cn10k-wdt By fallback do you mean " const: marvell,cn10k-wdt" and " const: marvell,octeontx2-wdt" ? If yes I removed because "octeontx2" and "cn10k" are soc family name and no a specific soc. Thanks -Bharat > > > + > > + reg: > > + maxItems: 1 > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + minItems: 1 > > From v7: > maxItems instead. You see it is different than above properties? > > > + > > + clock-names: > > + minItems: 1 > > From v7: > Need to define names. > > Cheers, > Conor.
On Sat, May 27, 2023 at 02:53:25PM +0000, Bharat Bhushan wrote: > From: Conor Dooley <conor@kernel.org> > > On Fri, May 26, 2023 at 11:56:25AM +0530, Bharat Bhushan wrote: > > > +properties: > > > + compatible: > > > + enum: > > > + - marvell,cn9670-wdt > > > + - marvell,cn9880-wdt > > > + - marvell,cnf9535-wdt > > > + - marvell,cn10624-wdt > > > + - marvell,cn10308-wdt > > > + - marvell,cnf10518-wdt > > > > static const struct of_device_id gti_wdt_of_match[] = { > > { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, > > { .compatible = "marvell,cn9880-wdt", .data = &match_data_octeontx2}, > > { .compatible = "marvell,cnf9535-wdt", .data = &match_data_octeontx2}, > > { .compatible = "marvell,cn10624-wdt", .data = &match_data_cn10k}, > > { .compatible = "marvell,cn10308-wdt", .data = &match_data_cn10k}, > > { .compatible = "marvell,cnf10518-wdt", .data = &match_data_cn10k}, > > > > This is a fat hint that you should be using fallback compatibles here. > > You even had a fallback setup in your last revision, but you seem to have > > removed it alongside the removal of the wildcards. Why did you do that? > > Not sure I understand this comment, Compatible in last version was as below: > > + properties: > + compatible: > + oneOf: > + - const: marvell,octeontx2-wdt > + - items: > + - enum: > + - marvell,octeontx2-95xx-wdt > + - marvell,octeontx2-96xx-wdt > + - marvell,octeontx2-98xx-wdt > + - const: marvell,octeontx2-wdt > + - const: marvell,cn10k-wdt > + - items: > + - enum: > + - marvell,cn10kx-wdt > + - marvell,cnf10kx-wdt > + - const: marvell,cn10k-wdt > > By fallback do you mean " const: marvell,cn10k-wdt" and > "const: marvell,octeontx2-wdt" ? If yes I removed because "octeontx2" > and "cn10k" are soc family name and no a specific soc. No, I meant that you should permit compatible = "marvell,cn9880-wdt", "marvell,cn9670-wdt"; and compatible = "marvell,cnf9535-wdt", "marvell,cn9670-wdt"; and compatible = "marvell,cn9670-wdt"; so the driver only needs to contain { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, instead of { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, { .compatible = "marvell,cn9880-wdt", .data = &match_data_octeontx2}, { .compatible = "marvell,cnf9535-wdt", .data = &match_data_octeontx2}, Note that using fallback compatibles is separate from using wildcards, and I was not suggesting that you go back to wildcards ;) Cheers, Conor.
Please see inline > -----Original Message----- > From: Conor Dooley <conor@kernel.org> > Sent: Saturday, May 27, 2023 9:40 PM > To: Bharat Bhushan <bbhushan2@marvell.com> > Cc: wim@linux-watchdog.org; linux@roeck-us.net; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org; linux-watchdog@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Sunil Kovvuri > Goutham <sgoutham@marvell.com> > Subject: Re: [EXT] Re: [PATCH 1/2 v8] dt-bindings: watchdog: marvell GTI system > watchdog driver > > On Sat, May 27, 2023 at 02:53:25PM +0000, Bharat Bhushan wrote: > > From: Conor Dooley <conor@kernel.org> > > > On Fri, May 26, 2023 at 11:56:25AM +0530, Bharat Bhushan wrote: > > > > > +properties: > > > > + compatible: > > > > + enum: > > > > + - marvell,cn9670-wdt > > > > + - marvell,cn9880-wdt > > > > + - marvell,cnf9535-wdt > > > > + - marvell,cn10624-wdt > > > > + - marvell,cn10308-wdt > > > > + - marvell,cnf10518-wdt > > > > > > static const struct of_device_id gti_wdt_of_match[] = { > > > { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, > > > { .compatible = "marvell,cn9880-wdt", .data = &match_data_octeontx2}, > > > { .compatible = "marvell,cnf9535-wdt", .data = &match_data_octeontx2}, > > > { .compatible = "marvell,cn10624-wdt", .data = &match_data_cn10k}, > > > { .compatible = "marvell,cn10308-wdt", .data = &match_data_cn10k}, > > > { .compatible = "marvell,cnf10518-wdt", .data = > > > &match_data_cn10k}, > > > > > > This is a fat hint that you should be using fallback compatibles here. > > > You even had a fallback setup in your last revision, but you seem to > > > have removed it alongside the removal of the wildcards. Why did you do > that? > > > > Not sure I understand this comment, Compatible in last version was as below: > > > > + properties: > > + compatible: > > + oneOf: > > + - const: marvell,octeontx2-wdt > > + - items: > > + - enum: > > + - marvell,octeontx2-95xx-wdt > > + - marvell,octeontx2-96xx-wdt > > + - marvell,octeontx2-98xx-wdt > > + - const: marvell,octeontx2-wdt > > + - const: marvell,cn10k-wdt > > + - items: > > + - enum: > > + - marvell,cn10kx-wdt > > + - marvell,cnf10kx-wdt > > + - const: marvell,cn10k-wdt > > > > By fallback do you mean " const: marvell,cn10k-wdt" and > > "const: marvell,octeontx2-wdt" ? If yes I removed because "octeontx2" > > and "cn10k" are soc family name and no a specific soc. > > No, I meant that you should permit > compatible = "marvell,cn9880-wdt", "marvell,cn9670-wdt"; and > compatible = "marvell,cnf9535-wdt", "marvell,cn9670-wdt"; and > compatible = "marvell,cn9670-wdt"; > so the driver only needs to contain > { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, > instead of > { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, > { .compatible = "marvell,cn9880-wdt", .data = &match_data_octeontx2}, > { .compatible = "marvell,cnf9535-wdt", .data = &match_data_octeontx2}, > > Note that using fallback compatibles is separate from using wildcards, and I was > not suggesting that you go back to wildcards ;) Fallback you mentioned make code look simple. Is below representation correct for above mentioned fallback? properties: compatible: oneOf: - const: marvell,cn9670-wdt - items: - enum: - marvell,cn9880-wdt - marvell,cnf9535-wdt - const: marvell,cn9670-wdt - const: marvell,cn10624-wdt - items: - enum: - marvell,cn10308-wdt - marvell,cnf10518-wdt - const: marvell,cn10624-wdt And driver will have { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, { .compatible = "marvell,cn10624-wdt", .data = &match_data_cn10k}, Thanks -Bharat > > Cheers, > Conor.
Hey Bharat, On Mon, May 29, 2023 at 06:14:07AM +0000, Bharat Bhushan wrote: > Fallback you mentioned make code look simple. Is below representation correct for above mentioned fallback? > > properties: > compatible: > oneOf: > - const: marvell,cn9670-wdt > - items: > - enum: > - marvell,cn9880-wdt > - marvell,cnf9535-wdt > - const: marvell,cn9670-wdt > - const: marvell,cn10624-wdt > - items: > - enum: > - marvell,cn10308-wdt > - marvell,cnf10518-wdt > - const: marvell,cn10624-wdt Instead of having const: bits for each of the single-compatible ones, if you are not going to add descriptions, I'd probably do: properties: compatible: oneOf: - enum: - marvell,cn9670-wdt - marvell,cn10624-wdt - items: - enum: - marvell,cn9880-wdt - marvell,cnf9535-wdt - const: marvell,cn9670-wdt - items: - enum: - marvell,cn10308-wdt - marvell,cnf10518-wdt - const: marvell,cn10624-wdt > And driver will have > { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, > { .compatible = "marvell,cn10624-wdt", .data = &match_data_cn10k}, Otherwise, looks good to me. You should probably also rename the file to match one of the compatibles contained in it. Thanks, Conor.
On 5/25/23 23:26, Bharat Bhushan wrote: > This patch add support for Marvell GTI watchdog. Global timer > unit (GTI) support hardware watchdog timer. Software programs > watchdog timer to generate interrupt on first timeout, second > timeout is configured to be ignored and system reboots on > third timeout. > > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> > --- > v8: > - Compatible names as per soc names > - This driver run on ARM64 based architecture, Added 64BIT config > dependency to avoid compilation error related to readq/writeq on > 32 as platform (Hexagon) > In the future, please provide complete change logs. > drivers/watchdog/Kconfig | 13 ++ > drivers/watchdog/Makefile | 1 + > drivers/watchdog/marvell_gti_wdt.c | 346 +++++++++++++++++++++++++++++ > 3 files changed, 360 insertions(+) > create mode 100644 drivers/watchdog/marvell_gti_wdt.c > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index f22138709bf5..bbdc20f33dbf 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -1779,6 +1779,19 @@ config OCTEON_WDT > from the first interrupt, it is then only poked when the > device is written. > > +config MARVELL_GTI_WDT > + tristate "Marvell GTI Watchdog driver" > + depends on ARCH_THUNDER || (COMPILE_TEST && 64BIT) > + default y Does this watchdog exist on all Thunder processors ? If not please drop. > + select WATCHDOG_CORE > + help > + Marvell GTI hardware supports watchdog timer. First timeout > + works as watchdog pretimeout and installed interrupt handler > + will be called on first timeout. Hardware can generate interrupt > + to SCP on second timeout but it is not enabled, So second > + timeout is ignored. If device poke does not happen then system > + will reboot on third timeout. > + > config BCM2835_WDT > tristate "Broadcom BCM2835 hardware watchdog" > depends on ARCH_BCM2835 || (OF && COMPILE_TEST) > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile > index b4c4ccf2d703..a164cd161ef3 100644 > --- a/drivers/watchdog/Makefile > +++ b/drivers/watchdog/Makefile > @@ -98,6 +98,7 @@ obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o > obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o > obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o > obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o > +obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o > > # X86 (i386 + ia64 + x86_64) Architecture > obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o > diff --git a/drivers/watchdog/marvell_gti_wdt.c b/drivers/watchdog/marvell_gti_wdt.c > new file mode 100644 > index 000000000000..976bb9306115 > --- /dev/null > +++ b/drivers/watchdog/marvell_gti_wdt.c > @@ -0,0 +1,346 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Marvell GTI Watchdog driver > + * > + * Copyright (C) 2023 Marvell. > + */ > + > +#include <linux/module.h> > +#include <linux/interrupt.h> > +#include <linux/of_platform.h> > +#include <linux/platform_device.h> > +#include <linux/watchdog.h> > +#include <linux/clk.h> > + Alphabetic order, please. Also, include io.h since you are using io functions (writeq). > +/* > + * Hardware supports following mode of operation: > + * 1) Interrupt Only: > + * This will generate the interrupt to arm core whenever timeout happens. > + * > + * 2) Interrupt + del3t (Interrupt to firmware (SCP processor)). > + * This will generate interrupt to arm core on 1st timeout happens > + * This will generate interrupt to SCP processor on 2nd timeout happens > + * > + * 3) Interrupt + Interrupt to SCP processor (called delt3t) + reboot. > + * This will generate interrupt to arm core on 1st timeout happens > + * Will generate interrupt to SCP processor on 2nd timeout happens, > + * if interrupt is configured. > + * Reboot on 3rd timeout. > + * > + * Driver will use hardware in mode-3 above so that system can reboot in case > + * a hardware hang. Also h/w is configured not to generate SCP interrupt, so > + * effectively 2nd timeout is ignored within hardware. > + * > + * First timeout is effectively watchdog pretimeout. > + */ > + > +/* GTI CWD Watchdog (GTI_CWD_WDOG) Register */ > +#define GTI_CWD_WDOG(reg_offset) (0x8 * reg_offset) reg_offset in () > +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST 0x3 > +#define GTI_CWD_WDOG_MODE_MASK GENMASK_ULL(1, 0) > +#define GTI_CWD_WDOG_LEN_SHIFT 4 > +#define GTI_CWD_WDOG_LEN_MASK GENMASK_ULL(19, 4) > +#define GTI_CWD_WDOG_CNT_SHIFT 20 > +#define GTI_CWD_WDOG_CNT_MASK GENMASK_ULL(43, 20) > + > +/* GTI CWD Watchdog Interrupt (GTI_CWD_INT) Register */ > +#define GTI_CWD_INT 0x200 > +#define GTI_CWD_INT_PENDING_STATUS(bit) (1 << bit) > + > +/* GTI CWD Watchdog Interrupt Enable Clear (GTI_CWD_INT_ENA_CLR) Register */ > +#define GTI_CWD_INT_ENA_CLR 0x210 > +#define GTI_CWD_INT_ENA_CLR_VAL(bit) (1 << bit) Please use BIT(). > + > +/* GTI CWD Watchdog Interrupt Enable Set (GTI_CWD_INT_ENA_SET) Register */ > +#define GTI_CWD_INT_ENA_SET 0x218 > +#define GTI_CWD_INT_ENA_SET_VAL(bit) (1 << bit) > + > +/* GTI CWD Watchdog Poke (GTI_CWD_POKE) Registers */ > +#define GTI_CWD_POKE(reg_offset) (0x10000 + 0x8 * reg_offset) > +#define GTI_CWD_POKE_VAL 1 > + > +struct gti_match_data { > + u32 gti_num_timers; > +}; > + > +static const struct gti_match_data match_data_octeontx2 = { The driver depends on ARCH_THUNDER but not ARCH_THUNDER2. > + .gti_num_timers = 54, > +}; > + > +static const struct gti_match_data match_data_cn10k = { > + .gti_num_timers = 64, > +}; > + > +struct gti_wdt_priv { > + struct watchdog_device wdev; > + void __iomem *base; > + u32 clock_freq; > + struct clk *sclk; > + /* wdt_timer_idx used for timer to be used for system watchdog */ > + u32 wdt_timer_idx; > + const struct gti_match_data *data; > +}; > + > +static irqreturn_t gti_wdt_interrupt(int irq, void *data) > +{ > + struct watchdog_device *wdev = data; > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + > + /* Clear Interrupt Pending Status */ > + writeq(GTI_CWD_INT_PENDING_STATUS(priv->wdt_timer_idx), > + priv->base + GTI_CWD_INT); > + > + watchdog_notify_pretimeout(wdev); > + > + return IRQ_HANDLED; > +} > + > +static int gti_wdt_ping(struct watchdog_device *wdev) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + > + writeq(GTI_CWD_POKE_VAL, > + priv->base + GTI_CWD_POKE(priv->wdt_timer_idx)); > + > + return 0; > +} > + > +static int gti_wdt_start(struct watchdog_device *wdev) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + u64 regval; > + > + if (!wdev->pretimeout) > + return -EINVAL; > + > + set_bit(WDOG_HW_RUNNING, &wdev->status); > + > + /* Clear any pending interrupt */ > + writeq(GTI_CWD_INT_PENDING_STATUS(priv->wdt_timer_idx), > + priv->base + GTI_CWD_INT); > + > + /* Enable Interrupt */ > + writeq(GTI_CWD_INT_ENA_SET_VAL(priv->wdt_timer_idx), > + priv->base + GTI_CWD_INT_ENA_SET); > + > + /* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */ > + regval = readq(priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > + regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST; > + writeq(regval, priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > + > + return 0; > +} > + > +static int gti_wdt_stop(struct watchdog_device *wdev) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + u64 regval; > + > + /* Disable Interrupt */ > + writeq(GTI_CWD_INT_ENA_CLR_VAL(priv->wdt_timer_idx), > + priv->base + GTI_CWD_INT_ENA_CLR); > + > + /* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */ > + regval = readq(priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > + regval &= ~GTI_CWD_WDOG_MODE_MASK; > + writeq(regval, priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > + > + return 0; > +} > + > +static int gti_wdt_settimeout(struct watchdog_device *wdev, > + unsigned int timeout) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + u64 timeout_wdog, regval; > + > + /* Update new timeout */ > + wdev->timeout = timeout; > + > + /* Pretimeout is 1/3 of timeout */ > + wdev->pretimeout = timeout / 3; > + > + /* Get clock cycles from pretimeout */ > + timeout_wdog = (u64)priv->clock_freq * wdev->pretimeout; > + > + /* Watchdog counts in 1024 cycle steps */ > + timeout_wdog = timeout_wdog >> 10; > + > + /* GTI_CWD_WDOG.CNT: reload counter is 16-bit */ > + timeout_wdog = (timeout_wdog + 0xff) >> 8; > + if (timeout_wdog >= 0x10000) > + timeout_wdog = 0xffff; > + > + /* > + * GTI_CWD_WDOG.LEN is 24bit, lower 8-bits should be zero and > + * upper 16-bits are same as GTI_CWD_WDOG.CNT > + */ > + regval = readq(priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > + regval &= GTI_CWD_WDOG_MODE_MASK; > + regval |= (timeout_wdog << (GTI_CWD_WDOG_CNT_SHIFT + 8)) | > + (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT); > + writeq(regval, priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > + > + return 0; > +} > + > +static int gti_wdt_set_pretimeout(struct watchdog_device *wdev, > + unsigned int timeout) > +{ > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > + struct watchdog_device *wdog_dev = &priv->wdev; > + > + /* pretimeout should 1/3 of max_timeout */ > + if (timeout * 3 <= wdog_dev->max_timeout) > + return gti_wdt_settimeout(wdev, timeout * 3); > + > + return -EINVAL; > +} > + > +static void gti_clk_disable_unprepare(void *data) > +{ > + clk_disable_unprepare(data); > +} > + > +static int gti_wdt_get_cntfrq(struct platform_device *pdev, > + struct gti_wdt_priv *priv) > +{ > + int err; > + > + priv->sclk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(priv->sclk)) > + return PTR_ERR(priv->sclk); > + > + err = clk_prepare_enable(priv->sclk); > + if (err) > + return err; > + Any reason for not using devm_clk_get_enabled() ? > + err = devm_add_action_or_reset(&pdev->dev, > + gti_clk_disable_unprepare, priv->sclk); > + if (err) > + return err; > + > + priv->clock_freq = clk_get_rate(priv->sclk); > + if (!priv->clock_freq) > + return -EINVAL; > + > + return 0; > +} > + > +static const struct watchdog_info gti_wdt_ident = { > + .identity = "Marvell GTI watchdog", > + .options = WDIOF_SETTIMEOUT | WDIOF_PRETIMEOUT | WDIOF_KEEPALIVEPING | > + WDIOF_MAGICCLOSE | WDIOF_CARDRESET, > +}; > + > +static const struct watchdog_ops gti_wdt_ops = { > + .owner = THIS_MODULE, > + .start = gti_wdt_start, > + .stop = gti_wdt_stop, > + .ping = gti_wdt_ping, > + .set_timeout = gti_wdt_settimeout, > + .set_pretimeout = gti_wdt_set_pretimeout, > +}; > + > +static int gti_wdt_probe(struct platform_device *pdev) > +{ > + struct gti_wdt_priv *priv; > + struct device *dev = &pdev->dev; > + struct watchdog_device *wdog_dev; > + u64 max_pretimeout; > + u32 wdt_idx; > + int irq; > + int err; > + > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(priv->base)) > + return dev_err_probe(&pdev->dev, PTR_ERR(priv->base), > + "reg property not valid/found\n"); > + > + err = gti_wdt_get_cntfrq(pdev, priv); > + if (err) > + return dev_err_probe(&pdev->dev, err, > + "GTI clock frequency not valid/found"); > + > + priv->data = of_device_get_match_data(dev); > + > + /* default use last timer for watchdog */ > + priv->wdt_timer_idx = priv->data->gti_num_timers - 1; > + > + err = of_property_read_u32(dev->of_node, "marvell,wdt-timer-index", > + &wdt_idx); > + if (!err) { > + if (wdt_idx >= priv->data->gti_num_timers) > + return dev_err_probe(&pdev->dev, err, > + "GTI wdog timer index not valid"); > + > + priv->wdt_timer_idx = wdt_idx; > + } > + > + wdog_dev = &priv->wdev; > + wdog_dev->info = >i_wdt_ident, > + wdog_dev->ops = >i_wdt_ops, > + wdog_dev->parent = dev; > + /* > + * Watchdog counter is 24 bit where lower 8 bits are zeros > + * This counter decrements every 1024 clock cycles. > + */ > + max_pretimeout = (GTI_CWD_WDOG_CNT_MASK >> GTI_CWD_WDOG_CNT_SHIFT); > + max_pretimeout &= ~0xFFUL; > + max_pretimeout = (max_pretimeout * 1024) / priv->clock_freq; > + wdog_dev->pretimeout = max_pretimeout; > + > + /* Maximum timeout is 3 times the pretimeout */ > + wdog_dev->max_timeout = max_pretimeout * 3; > + /* Minimum first timeout (pretimeout) is 1, so min_timeout as 3 */ > + wdog_dev->min_timeout = 3; > + wdog_dev->timeout = wdog_dev->pretimeout; > + > + watchdog_set_drvdata(wdog_dev, priv); > + platform_set_drvdata(pdev, priv); > + gti_wdt_settimeout(wdog_dev, wdog_dev->timeout); > + watchdog_stop_on_reboot(wdog_dev); > + watchdog_stop_on_unregister(wdog_dev); > + > + err = devm_watchdog_register_device(dev, wdog_dev); > + if (err) > + return err; > + > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return dev_err_probe(&pdev->dev, irq, "IRQ resource not found\n"); > + > + err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0, > + pdev->name, &priv->wdev); > + if (err) > + return dev_err_probe(dev, err, "Failed to register interrupt handler\n"); > + > + dev_info(dev, "Watchdog enabled (timeout=%d sec)\n", wdog_dev->timeout); > + return 0; > +} > + > +static const struct of_device_id gti_wdt_of_match[] = { > + { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, > + { .compatible = "marvell,cn9880-wdt", .data = &match_data_octeontx2}, > + { .compatible = "marvell,cnf9535-wdt", .data = &match_data_octeontx2}, > + { .compatible = "marvell,cn10624-wdt", .data = &match_data_cn10k}, > + { .compatible = "marvell,cn10308-wdt", .data = &match_data_cn10k}, > + { .compatible = "marvell,cnf10518-wdt", .data = &match_data_cn10k}, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, gti_wdt_of_match); > + > +static struct platform_driver gti_wdt_driver = { > + .driver = { > + .name = "gti-wdt", > + .of_match_table = gti_wdt_of_match, > + }, > + .probe = gti_wdt_probe, > +}; > +module_platform_driver(gti_wdt_driver); > + > +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>"); > +MODULE_DESCRIPTION("Marvell GTI watchdog driver");
Please see inline > -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Monday, May 29, 2023 7:08 PM > To: Bharat Bhushan <bbhushan2@marvell.com>; wim@linux-watchdog.org; > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; linux- > watchdog@vger.kernel.org; devicetree@vger.kernel.org; linux- > kernel@vger.kernel.org; Sunil Kovvuri Goutham <sgoutham@marvell.com> > Subject: [EXT] Re: [PATCH 2/2 v8] Watchdog: Add marvell GTI watchdog driver > > External Email > > ---------------------------------------------------------------------- > On 5/25/23 23:26, Bharat Bhushan wrote: > > This patch add support for Marvell GTI watchdog. Global timer unit > > (GTI) support hardware watchdog timer. Software programs watchdog > > timer to generate interrupt on first timeout, second timeout is > > configured to be ignored and system reboots on third timeout. > > > > Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> > > --- > > v8: > > - Compatible names as per soc names > > - This driver run on ARM64 based architecture, Added 64BIT config > > dependency to avoid compilation error related to readq/writeq on > > 32 as platform (Hexagon) > > > > In the future, please provide complete change logs. > > > drivers/watchdog/Kconfig | 13 ++ > > drivers/watchdog/Makefile | 1 + > > drivers/watchdog/marvell_gti_wdt.c | 346 > +++++++++++++++++++++++++++++ > > 3 files changed, 360 insertions(+) > > create mode 100644 drivers/watchdog/marvell_gti_wdt.c > > > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index > > f22138709bf5..bbdc20f33dbf 100644 > > --- a/drivers/watchdog/Kconfig > > +++ b/drivers/watchdog/Kconfig > > @@ -1779,6 +1779,19 @@ config OCTEON_WDT > > from the first interrupt, it is then only poked when the > > device is written. > > > > +config MARVELL_GTI_WDT > > + tristate "Marvell GTI Watchdog driver" > > + depends on ARCH_THUNDER || (COMPILE_TEST && 64BIT) > > + default y > > Does this watchdog exist on all Thunder processors ? > If not please drop. This driver is not for all thunder but Octeon tx2 and cn10k series of processor (which are arm64 based). > > > + select WATCHDOG_CORE > > + help > > + Marvell GTI hardware supports watchdog timer. First timeout > > + works as watchdog pretimeout and installed interrupt handler > > + will be called on first timeout. Hardware can generate interrupt > > + to SCP on second timeout but it is not enabled, So second > > + timeout is ignored. If device poke does not happen then system > > + will reboot on third timeout. > > + > > config BCM2835_WDT > > tristate "Broadcom BCM2835 hardware watchdog" > > depends on ARCH_BCM2835 || (OF && COMPILE_TEST) diff --git > > a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index > > b4c4ccf2d703..a164cd161ef3 100644 > > --- a/drivers/watchdog/Makefile > > +++ b/drivers/watchdog/Makefile > > @@ -98,6 +98,7 @@ obj-$(CONFIG_VISCONTI_WATCHDOG) += visconti_wdt.o > > obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o > > obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o > > obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o > > +obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o > > > > # X86 (i386 + ia64 + x86_64) Architecture > > obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o diff --git > > a/drivers/watchdog/marvell_gti_wdt.c > > b/drivers/watchdog/marvell_gti_wdt.c > > new file mode 100644 > > index 000000000000..976bb9306115 > > --- /dev/null > > +++ b/drivers/watchdog/marvell_gti_wdt.c > > @@ -0,0 +1,346 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Marvell GTI Watchdog driver > > + * > > + * Copyright (C) 2023 Marvell. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/interrupt.h> > > +#include <linux/of_platform.h> > > +#include <linux/platform_device.h> > > +#include <linux/watchdog.h> > > +#include <linux/clk.h> > > + > Alphabetic order, please. Also, include io.h since you are using io functions > (writeq). Ok, > > > +/* > > + * Hardware supports following mode of operation: > > + * 1) Interrupt Only: > > + * This will generate the interrupt to arm core whenever timeout happens. > > + * > > + * 2) Interrupt + del3t (Interrupt to firmware (SCP processor)). > > + * This will generate interrupt to arm core on 1st timeout happens > > + * This will generate interrupt to SCP processor on 2nd timeout happens > > + * > > + * 3) Interrupt + Interrupt to SCP processor (called delt3t) + reboot. > > + * This will generate interrupt to arm core on 1st timeout happens > > + * Will generate interrupt to SCP processor on 2nd timeout happens, > > + * if interrupt is configured. > > + * Reboot on 3rd timeout. > > + * > > + * Driver will use hardware in mode-3 above so that system can reboot > > +in case > > + * a hardware hang. Also h/w is configured not to generate SCP > > +interrupt, so > > + * effectively 2nd timeout is ignored within hardware. > > + * > > + * First timeout is effectively watchdog pretimeout. > > + */ > > + > > +/* GTI CWD Watchdog (GTI_CWD_WDOG) Register */ > > +#define GTI_CWD_WDOG(reg_offset) (0x8 * reg_offset) > > reg_offset in () Ok. > > > +#define GTI_CWD_WDOG_MODE_INT_DEL3T_RST 0x3 > > +#define GTI_CWD_WDOG_MODE_MASK GENMASK_ULL(1, 0) > > +#define GTI_CWD_WDOG_LEN_SHIFT 4 > > +#define GTI_CWD_WDOG_LEN_MASK GENMASK_ULL(19, 4) > > +#define GTI_CWD_WDOG_CNT_SHIFT 20 > > +#define GTI_CWD_WDOG_CNT_MASK GENMASK_ULL(43, 20) > > + > > +/* GTI CWD Watchdog Interrupt (GTI_CWD_INT) Register */ > > +#define GTI_CWD_INT 0x200 > > +#define GTI_CWD_INT_PENDING_STATUS(bit) (1 << bit) > > + > > +/* GTI CWD Watchdog Interrupt Enable Clear (GTI_CWD_INT_ENA_CLR) > Register */ > > +#define GTI_CWD_INT_ENA_CLR 0x210 > > +#define GTI_CWD_INT_ENA_CLR_VAL(bit) (1 << bit) > > Please use BIT(). Okay, > > > + > > +/* GTI CWD Watchdog Interrupt Enable Set (GTI_CWD_INT_ENA_SET) > Register */ > > +#define GTI_CWD_INT_ENA_SET 0x218 > > +#define GTI_CWD_INT_ENA_SET_VAL(bit) (1 << bit) > > + > > +/* GTI CWD Watchdog Poke (GTI_CWD_POKE) Registers */ > > +#define GTI_CWD_POKE(reg_offset) (0x10000 + 0x8 * reg_offset) > > +#define GTI_CWD_POKE_VAL 1 > > + > > +struct gti_match_data { > > + u32 gti_num_timers; > > +}; > > + > > +static const struct gti_match_data match_data_octeontx2 = { > > The driver depends on ARCH_THUNDER but not ARCH_THUNDER2. ARCH_THUNDER2 if different series. > > > + .gti_num_timers = 54, > > +}; > > + > > +static const struct gti_match_data match_data_cn10k = { > > + .gti_num_timers = 64, > > +}; > > + > > +struct gti_wdt_priv { > > + struct watchdog_device wdev; > > + void __iomem *base; > > + u32 clock_freq; > > + struct clk *sclk; > > + /* wdt_timer_idx used for timer to be used for system watchdog */ > > + u32 wdt_timer_idx; > > + const struct gti_match_data *data; > > +}; > > + > > +static irqreturn_t gti_wdt_interrupt(int irq, void *data) { > > + struct watchdog_device *wdev = data; > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + > > + /* Clear Interrupt Pending Status */ > > + writeq(GTI_CWD_INT_PENDING_STATUS(priv->wdt_timer_idx), > > + priv->base + GTI_CWD_INT); > > + > > + watchdog_notify_pretimeout(wdev); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static int gti_wdt_ping(struct watchdog_device *wdev) { > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + > > + writeq(GTI_CWD_POKE_VAL, > > + priv->base + GTI_CWD_POKE(priv->wdt_timer_idx)); > > + > > + return 0; > > +} > > + > > +static int gti_wdt_start(struct watchdog_device *wdev) { > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + u64 regval; > > + > > + if (!wdev->pretimeout) > > + return -EINVAL; > > + > > + set_bit(WDOG_HW_RUNNING, &wdev->status); > > + > > + /* Clear any pending interrupt */ > > + writeq(GTI_CWD_INT_PENDING_STATUS(priv->wdt_timer_idx), > > + priv->base + GTI_CWD_INT); > > + > > + /* Enable Interrupt */ > > + writeq(GTI_CWD_INT_ENA_SET_VAL(priv->wdt_timer_idx), > > + priv->base + GTI_CWD_INT_ENA_SET); > > + > > + /* Set (Interrupt + SCP interrupt (DEL3T) + core domain reset) Mode */ > > + regval = readq(priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > > + regval |= GTI_CWD_WDOG_MODE_INT_DEL3T_RST; > > + writeq(regval, priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > > + > > + return 0; > > +} > > + > > +static int gti_wdt_stop(struct watchdog_device *wdev) { > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + u64 regval; > > + > > + /* Disable Interrupt */ > > + writeq(GTI_CWD_INT_ENA_CLR_VAL(priv->wdt_timer_idx), > > + priv->base + GTI_CWD_INT_ENA_CLR); > > + > > + /* Set GTI_CWD_WDOG.Mode = 0 to stop the timer */ > > + regval = readq(priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > > + regval &= ~GTI_CWD_WDOG_MODE_MASK; > > + writeq(regval, priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > > + > > + return 0; > > +} > > + > > +static int gti_wdt_settimeout(struct watchdog_device *wdev, > > + unsigned int timeout) > > +{ > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + u64 timeout_wdog, regval; > > + > > + /* Update new timeout */ > > + wdev->timeout = timeout; > > + > > + /* Pretimeout is 1/3 of timeout */ > > + wdev->pretimeout = timeout / 3; > > + > > + /* Get clock cycles from pretimeout */ > > + timeout_wdog = (u64)priv->clock_freq * wdev->pretimeout; > > + > > + /* Watchdog counts in 1024 cycle steps */ > > + timeout_wdog = timeout_wdog >> 10; > > + > > + /* GTI_CWD_WDOG.CNT: reload counter is 16-bit */ > > + timeout_wdog = (timeout_wdog + 0xff) >> 8; > > + if (timeout_wdog >= 0x10000) > > + timeout_wdog = 0xffff; > > + > > + /* > > + * GTI_CWD_WDOG.LEN is 24bit, lower 8-bits should be zero and > > + * upper 16-bits are same as GTI_CWD_WDOG.CNT > > + */ > > + regval = readq(priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > > + regval &= GTI_CWD_WDOG_MODE_MASK; > > + regval |= (timeout_wdog << (GTI_CWD_WDOG_CNT_SHIFT + 8)) | > > + (timeout_wdog << GTI_CWD_WDOG_LEN_SHIFT); > > + writeq(regval, priv->base + GTI_CWD_WDOG(priv->wdt_timer_idx)); > > + > > + return 0; > > +} > > + > > +static int gti_wdt_set_pretimeout(struct watchdog_device *wdev, > > + unsigned int timeout) > > +{ > > + struct gti_wdt_priv *priv = watchdog_get_drvdata(wdev); > > + struct watchdog_device *wdog_dev = &priv->wdev; > > + > > + /* pretimeout should 1/3 of max_timeout */ > > + if (timeout * 3 <= wdog_dev->max_timeout) > > + return gti_wdt_settimeout(wdev, timeout * 3); > > + > > + return -EINVAL; > > +} > > + > > +static void gti_clk_disable_unprepare(void *data) { > > + clk_disable_unprepare(data); > > +} > > + > > +static int gti_wdt_get_cntfrq(struct platform_device *pdev, > > + struct gti_wdt_priv *priv) > > +{ > > + int err; > > + > > + priv->sclk = devm_clk_get(&pdev->dev, NULL); > > + if (IS_ERR(priv->sclk)) > > + return PTR_ERR(priv->sclk); > > + > > + err = clk_prepare_enable(priv->sclk); > > + if (err) > > + return err; > > + > > Any reason for not using devm_clk_get_enabled() ? No reason, just missed it. Thanks -Bharat > > > + err = devm_add_action_or_reset(&pdev->dev, > > + gti_clk_disable_unprepare, priv->sclk); > > + if (err) > > + return err; > > + > > + priv->clock_freq = clk_get_rate(priv->sclk); > > + if (!priv->clock_freq) > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > +static const struct watchdog_info gti_wdt_ident = { > > + .identity = "Marvell GTI watchdog", > > + .options = WDIOF_SETTIMEOUT | WDIOF_PRETIMEOUT | > WDIOF_KEEPALIVEPING | > > + WDIOF_MAGICCLOSE | WDIOF_CARDRESET, }; > > + > > +static const struct watchdog_ops gti_wdt_ops = { > > + .owner = THIS_MODULE, > > + .start = gti_wdt_start, > > + .stop = gti_wdt_stop, > > + .ping = gti_wdt_ping, > > + .set_timeout = gti_wdt_settimeout, > > + .set_pretimeout = gti_wdt_set_pretimeout, }; > > + > > +static int gti_wdt_probe(struct platform_device *pdev) { > > + struct gti_wdt_priv *priv; > > + struct device *dev = &pdev->dev; > > + struct watchdog_device *wdog_dev; > > + u64 max_pretimeout; > > + u32 wdt_idx; > > + int irq; > > + int err; > > + > > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(priv->base)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(priv->base), > > + "reg property not valid/found\n"); > > + > > + err = gti_wdt_get_cntfrq(pdev, priv); > > + if (err) > > + return dev_err_probe(&pdev->dev, err, > > + "GTI clock frequency not valid/found"); > > + > > + priv->data = of_device_get_match_data(dev); > > + > > + /* default use last timer for watchdog */ > > + priv->wdt_timer_idx = priv->data->gti_num_timers - 1; > > + > > + err = of_property_read_u32(dev->of_node, "marvell,wdt-timer-index", > > + &wdt_idx); > > + if (!err) { > > + if (wdt_idx >= priv->data->gti_num_timers) > > + return dev_err_probe(&pdev->dev, err, > > + "GTI wdog timer index not valid"); > > + > > + priv->wdt_timer_idx = wdt_idx; > > + } > > + > > + wdog_dev = &priv->wdev; > > + wdog_dev->info = >i_wdt_ident, > > + wdog_dev->ops = >i_wdt_ops, > > + wdog_dev->parent = dev; > > + /* > > + * Watchdog counter is 24 bit where lower 8 bits are zeros > > + * This counter decrements every 1024 clock cycles. > > + */ > > + max_pretimeout = (GTI_CWD_WDOG_CNT_MASK >> > GTI_CWD_WDOG_CNT_SHIFT); > > + max_pretimeout &= ~0xFFUL; > > + max_pretimeout = (max_pretimeout * 1024) / priv->clock_freq; > > + wdog_dev->pretimeout = max_pretimeout; > > + > > + /* Maximum timeout is 3 times the pretimeout */ > > + wdog_dev->max_timeout = max_pretimeout * 3; > > + /* Minimum first timeout (pretimeout) is 1, so min_timeout as 3 */ > > + wdog_dev->min_timeout = 3; > > + wdog_dev->timeout = wdog_dev->pretimeout; > > + > > + watchdog_set_drvdata(wdog_dev, priv); > > + platform_set_drvdata(pdev, priv); > > + gti_wdt_settimeout(wdog_dev, wdog_dev->timeout); > > + watchdog_stop_on_reboot(wdog_dev); > > + watchdog_stop_on_unregister(wdog_dev); > > + > > + err = devm_watchdog_register_device(dev, wdog_dev); > > + if (err) > > + return err; > > + > > + irq = platform_get_irq(pdev, 0); > > + if (irq < 0) > > + return dev_err_probe(&pdev->dev, irq, "IRQ resource not > found\n"); > > + > > + err = devm_request_irq(dev, irq, gti_wdt_interrupt, 0, > > + pdev->name, &priv->wdev); > > + if (err) > > + return dev_err_probe(dev, err, "Failed to register interrupt > > +handler\n"); > > + > > + dev_info(dev, "Watchdog enabled (timeout=%d sec)\n", wdog_dev- > >timeout); > > + return 0; > > +} > > + > > +static const struct of_device_id gti_wdt_of_match[] = { > > + { .compatible = "marvell,cn9670-wdt", .data = &match_data_octeontx2}, > > + { .compatible = "marvell,cn9880-wdt", .data = &match_data_octeontx2}, > > + { .compatible = "marvell,cnf9535-wdt", .data = &match_data_octeontx2}, > > + { .compatible = "marvell,cn10624-wdt", .data = &match_data_cn10k}, > > + { .compatible = "marvell,cn10308-wdt", .data = &match_data_cn10k}, > > + { .compatible = "marvell,cnf10518-wdt", .data = &match_data_cn10k}, > > + { }, > > +}; > > +MODULE_DEVICE_TABLE(of, gti_wdt_of_match); > > + > > +static struct platform_driver gti_wdt_driver = { > > + .driver = { > > + .name = "gti-wdt", > > + .of_match_table = gti_wdt_of_match, > > + }, > > + .probe = gti_wdt_probe, > > +}; > > +module_platform_driver(gti_wdt_driver); > > + > > +MODULE_AUTHOR("Bharat Bhushan <bbhushan2@marvell.com>"); > > +MODULE_DESCRIPTION("Marvell GTI watchdog driver");
diff --git a/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml new file mode 100644 index 000000000000..3c642359d960 --- /dev/null +++ b/Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/watchdog/marvell,octeontx2-wdt.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell Global Timer (GTI) system watchdog + +allOf: + - $ref: watchdog.yaml# + +maintainers: + - Bharat Bhushan <bbhushan2@marvell.com> + +properties: + compatible: + enum: + - marvell,cn9670-wdt + - marvell,cn9880-wdt + - marvell,cnf9535-wdt + - marvell,cn10624-wdt + - marvell,cn10308-wdt + - marvell,cnf10518-wdt + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + minItems: 1 + + clock-names: + minItems: 1 + + marvell,wdt-timer-index: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 63 + description: + An SoC have many timers (up to 64), firmware can reserve one or more timer + for some other use case and configures one of the global timer as watchdog + timer. Firmware will update this field with the timer number configured + as watchdog timer. + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + soc { + #address-cells = <2>; + #size-cells = <2>; + + watchdog@802000040000 { + compatible = "marvell,cn9670-wdt"; + reg = <0x00008020 0x00040000 0x00000000 0x00020000>; + interrupts = <GIC_SPI 38 IRQ_TYPE_EDGE_RISING>; + clocks = <&sclk>; + clock-names = "ref_clk"; + marvell,wdt-timer-index = <63>; + }; + }; + +...
Add binding documentation for the Marvell GTI system watchdog driver. Signed-off-by: Bharat Bhushan <bbhushan2@marvell.com> --- v8: - Compatible name as per soc name .../watchdog/marvell,octeontx2-wdt.yaml | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/marvell,octeontx2-wdt.yaml