[00/11] watchdog: Consolidate FTWDT010 derivatives

Message ID 20170812184318.10144-1-linus.walleij@linaro.org
Headers show
Series
  • watchdog: Consolidate FTWDT010 derivatives
Related show

Message

Linus Walleij Aug. 12, 2017, 6:43 p.m.
The MOXA ART and Aspeed watchdogs are clearly based on the
Faraday Technology FTWDT010 IP block.

This series consolidates the drivers into one by extending
the Gemini driver to be as generic as possible, renaming it
to ftwdt010_wdt and merging the two other drivers into it.

As similar approach was used for the FTTMR010 driver in the
past.

The series ends with two patches that will be applied to
the ARM SoC tree to fix up the PCLK annotations, but these
are not needed to make the consolidation, patches 1-9 can
be applied directly to the watchdog tree to perform the
consolidation.

Obviously I would like the MOXA ART and Aspeed maintainers
to test the series to see that it still works as expected for
them afterwards.

The patch series is available at:
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik.git
Branch ftwdt010

Linus Walleij (11):
  watchdog: gemini/ftwdt010: rename DT bindings
  watchdog: gemini/ftwdt010: rename driver and symbols
  watchdog: ftwdt010: Make interrupt optional
  watchdog: ftwdt010: Add clock support
  watchdog: ftwdt010: Add restart support
  watchdog: ftwdt010: Extend DT bindings to derivatives
  watchdog: ftwdt010: Delete surplus bindings
  watchdog: ftwdt010/moxart: Merge MOXA ART into FTWDT010
  watchdog: ftwdt010/aspeed: Merge Aspeed into FTWDT010
  ARM: dts: fix PCLK name on Gemini and MOXA ART
  ARM: dts: Add PCLK to the Aspeed watchdogs

 .../devicetree/bindings/watchdog/aspeed-wdt.txt    |  16 -
 .../bindings/watchdog/cortina,gemin-watchdog.txt   |  17 --
 .../bindings/watchdog/cortina,gemini-watchdog.txt  |  17 --
 .../bindings/watchdog/faraday,ftwdt010.txt         |  34 +++
 .../bindings/watchdog/moxa,moxart-watchdog.txt     |  15 -
 arch/arm/boot/dts/aspeed-g4.dtsi                   |   7 +-
 arch/arm/boot/dts/aspeed-g5.dtsi                   |  12 +-
 arch/arm/boot/dts/gemini.dtsi                      |   3 +-
 arch/arm/boot/dts/moxart.dtsi                      |   3 +-
 drivers/watchdog/Kconfig                           |  39 +--
 drivers/watchdog/Makefile                          |   4 +-
 drivers/watchdog/aspeed_wdt.c                      | 200 ------------
 drivers/watchdog/ftwdt010_wdt.c                    | 338 +++++++++++++++++++++
 drivers/watchdog/gemini_wdt.c                      | 229 --------------
 drivers/watchdog/moxart_wdt.c                      | 178 -----------
 15 files changed, 401 insertions(+), 711 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/watchdog/aspeed-wdt.txt
 delete mode 100644 Documentation/devicetree/bindings/watchdog/cortina,gemin-watchdog.txt
 delete mode 100644 Documentation/devicetree/bindings/watchdog/cortina,gemini-watchdog.txt
 create mode 100644 Documentation/devicetree/bindings/watchdog/faraday,ftwdt010.txt
 delete mode 100644 Documentation/devicetree/bindings/watchdog/moxa,moxart-watchdog.txt
 delete mode 100644 drivers/watchdog/aspeed_wdt.c
 create mode 100644 drivers/watchdog/ftwdt010_wdt.c
 delete mode 100644 drivers/watchdog/gemini_wdt.c
 delete mode 100644 drivers/watchdog/moxart_wdt.c

-- 
2.13.4

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Joel Stanley Aug. 14, 2017, 1:24 a.m. | #1
Hi Linus,

On Sun, Aug 13, 2017 at 4:13 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> The MOXA ART and Aspeed watchdogs are clearly based on the

> Faraday Technology FTWDT010 IP block.


They have a similar register interface, but I'm told they are not the same IP.

We've got some patches on the list that add some extra registers to
the driver for the ast2500. If we decide to merge the drivers, that
support will need to be included.

Andrew was working on that, I'll let him follow up on the details.

> This series consolidates the drivers into one by extending

> the Gemini driver to be as generic as possible, renaming it

> to ftwdt010_wdt and merging the two other drivers into it.

>

> As similar approach was used for the FTTMR010 driver in the

> past.

>

> The series ends with two patches that will be applied to

> the ARM SoC tree to fix up the PCLK annotations, but these

> are not needed to make the consolidation, patches 1-9 can

> be applied directly to the watchdog tree to perform the

> consolidation.


The clock isn't called PCLK in the Aspeed documentation (similarly for
the timer, but I was too slow to speak up in that case).

I'm trying to find some time to write a proper clock driver so it's
clear how the clocks are set out in the Aspeed.

Cheers,

Joel
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Jeffery Aug. 14, 2017, 3:08 a.m. | #2
On Mon, 2017-08-14 at 10:54 +0930, Joel Stanley wrote:
> Hi Linus,


> On Sun, Aug 13, 2017 at 4:13 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

> > The MOXA ART and Aspeed watchdogs are clearly based on the

> > Faraday Technology FTWDT010 IP block.


> They have a similar register interface, but I'm told they are not the same IP.


> We've got some patches on the list that add some extra registers to

> the driver for the ast2500. If we decide to merge the drivers, that

> support will need to be included.


> Andrew was working on that, I'll let him follow up on the details.


There are two series on the lists expanding driver support for the Aspeed
watchdog, one from Chris Bostic and another from myself:

1. [PATCH v5 0/2] Add ASPEED watchdog device tree properties:

	https://lkml.org/lkml/2017/7/17/777 

2. [PATCH 0/2] watchdog: aspeed: External reset signal properties:

	https://www.spinics.net/lists/kernel/msg2570666.html 

I don't have the datasheets for either the Moxa or Faraday SoCs, so I can't
assess how the support I've added for the external pulse properties on Aspeed
hardware impacts/is impacted by the merge. Chris' changes on the otherhand look
like they could be generalised. At least, the vendor prefix on the devicetree
properties he defined could perhaps be changed from aspeed to faraday.

Cheers,

Andrew

PS: Patch 10/11 failed to apply for me against several trees, failing on the
hunk for arch/arm/boot/dts/gemini.dtsi. Is there an unmentioned dependency?

> > This series consolidates the drivers into one by extending

> > the Gemini driver to be as generic as possible, renaming it

> > to ftwdt010_wdt and merging the two other drivers into it.

> > 

> > As similar approach was used for the FTTMR010 driver in the

> > past.

> > 

> > The series ends with two patches that will be applied to

> > the ARM SoC tree to fix up the PCLK annotations, but these

> > are not needed to make the consolidation, patches 1-9 can

> > be applied directly to the watchdog tree to perform the

> > consolidation.


> The clock isn't called PCLK in the Aspeed documentation (similarly for

> the timer, but I was too slow to speak up in that case).


> I'm trying to find some time to write a proper clock driver so it's

> clear how the clocks are set out in the Aspeed.


> Cheers,


> Joel
Linus Walleij Aug. 14, 2017, 12:31 p.m. | #3
On Mon, Aug 14, 2017 at 3:24 AM, Joel Stanley <joel@jms.id.au> wrote:
> On Sun, Aug 13, 2017 at 4:13 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> The MOXA ART and Aspeed watchdogs are clearly based on the

>> Faraday Technology FTWDT010 IP block.

>

> They have a similar register interface, but I'm told they are not the same IP.


They are too similar to not be related somehow.

I guess it is one of those Shanzhai-mindset things where IP VHDL
or Verilog code is being copied around at silicon foundries in Asia
and turn up in different chips "independently" of each other.

It doesn't really matter if they "are" the same (as in: silicon vendor
admits that they are), if it walks like a duck, act and talks like a duck,
it is a duck. And we use the same driver.

> We've got some patches on the list that add some extra registers to

> the driver for the ast2500. If we decide to merge the drivers, that

> support will need to be included.


Hm I was not aware, need to read up on it.

> Andrew was working on that, I'll let him follow up on the details.

>

> The clock isn't called PCLK in the Aspeed documentation (similarly for

> the timer, but I was too slow to speak up in that case).


"PCLK" is just short for "peripheral block", just like other such
shorthands like "APB" (AMBA peripheral bridge clock). It's
a generic term.

Preferrably it should use the name from the IP vendor, but when in
conflict about names, it's too much trouble to use different names
IMO so I think "PCLK" is just fine.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Aug. 14, 2017, 12:36 p.m. | #4
On Mon, Aug 14, 2017 at 5:08 AM, Andrew Jeffery <andrew@aj.id.au> wrote:

> There are two series on the lists expanding driver support for the Aspeed

> watchdog, one from Chris Bostic and another from myself:

>

> 1. [PATCH v5 0/2] Add ASPEED watchdog device tree properties:

>

>         https://lkml.org/lkml/2017/7/17/777


Looks all right. The SoC vs whole chip reset signal big is an
Aspeed-specific extension AFAICT.

>

> 2. [PATCH 0/2] watchdog: aspeed: External reset signal properties:

>

>         https://www.spinics.net/lists/kernel/msg2570666.html


The external reset exists also in the Faraday FTWDT010 block, in the
same bit. (No coincidence...) but neither the Gemini or the MOXA ART
is using it as far as I know.

> I don't have the datasheets for either the Moxa or Faraday SoCs, so I can't

> assess how the support I've added for the external pulse properties on Aspeed

> hardware impacts/is impacted by the merge.


It's no big deal, I can rewrite the patches in the end of the series on top
of that stuff also adding this functionality.

> Chris' changes on the otherhand look

> like they could be generalised. At least, the vendor prefix on the devicetree

> properties he defined could perhaps be changed from aspeed to faraday.


The "aspeed,reset-type" is fine since it is Aspeed-only.

External reset is part of the Faraday IP block so it should be
"faraday,external-reset-signal" or so.

I will try to see if I can find the patches and comment directly.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Aug. 14, 2017, 12:39 p.m. | #5
On Sat, Aug 12, 2017 at 8:43 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

> The MOXA ART and Aspeed watchdogs are clearly based on the

> Faraday Technology FTWDT010 IP block.


Wim/Guenther: it is perfectly fine to just stop merging the series after
the say top 3 or top 4 ones etc. The do not need to be applied on an
all-or-nothing basis.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html