Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 3/7] watchdog: JZ4740: Register a restart handler
From: Guenter Roeck @ 2017-12-28 18:40 UTC (permalink / raw)
  To: Paul Cercueil, Ralf Baechle, Rob Herring, Mark Rutland,
	Wim Van Sebroeck
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171228162939.3928-4-paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>

On 12/28/2017 08:29 AM, Paul Cercueil wrote:
> The watchdog driver can restart the system by simply configuring the
> hardware for a timeout of 0 seconds.
> 
> Signed-off-by: Paul Cercueil <paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>

Reviewed-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

> ---
>   drivers/watchdog/jz4740_wdt.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
> index 92d6ca8ceb49..fa7f49a3212c 100644
> --- a/drivers/watchdog/jz4740_wdt.c
> +++ b/drivers/watchdog/jz4740_wdt.c
> @@ -130,6 +130,14 @@ static int jz4740_wdt_stop(struct watchdog_device *wdt_dev)
>   	return 0;
>   }
>   
> +static int jz4740_wdt_restart(struct watchdog_device *wdt_dev,
> +			      unsigned long action, void *data)
> +{
> +	wdt_dev->timeout = 0;
> +	jz4740_wdt_start(wdt_dev);
> +	return 0;
> +}
> +
>   static const struct watchdog_info jz4740_wdt_info = {
>   	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
>   	.identity = "jz4740 Watchdog",
> @@ -141,6 +149,7 @@ static const struct watchdog_ops jz4740_wdt_ops = {
>   	.stop = jz4740_wdt_stop,
>   	.ping = jz4740_wdt_ping,
>   	.set_timeout = jz4740_wdt_set_timeout,
> +	.restart = jz4740_wdt_restart,
>   };
>   
>   #ifdef CONFIG_OF
> 

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

^ permalink raw reply

* Re: [PATCH 2/7] watchdog: jz4740: Use devm_* functions
From: Guenter Roeck @ 2017-12-28 18:40 UTC (permalink / raw)
  To: Paul Cercueil, Ralf Baechle, Rob Herring, Mark Rutland,
	Wim Van Sebroeck
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171228162939.3928-3-paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>

On 12/28/2017 08:29 AM, Paul Cercueil wrote:
> - Use devm_clk_get instead of clk_get
> - Use devm_watchdog_register_device instead of watchdog_register_device
> 
> Signed-off-by: Paul Cercueil <paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>

Reviewed-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

[ the change I suggested earlier should be made in a separate patch ]

> ---
>   drivers/watchdog/jz4740_wdt.c | 27 ++++++++-------------------
>   1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
> index 6955deb100ef..92d6ca8ceb49 100644
> --- a/drivers/watchdog/jz4740_wdt.c
> +++ b/drivers/watchdog/jz4740_wdt.c
> @@ -178,40 +178,29 @@ static int jz4740_wdt_probe(struct platform_device *pdev)
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	drvdata->base = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(drvdata->base)) {
> -		ret = PTR_ERR(drvdata->base);
> -		goto err_out;
> -	}
> +	if (IS_ERR(drvdata->base))
> +		return PTR_ERR(drvdata->base);
>   
> -	drvdata->rtc_clk = clk_get(&pdev->dev, "rtc");
> +	drvdata->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
>   	if (IS_ERR(drvdata->rtc_clk)) {
>   		dev_err(&pdev->dev, "cannot find RTC clock\n");
> -		ret = PTR_ERR(drvdata->rtc_clk);
> -		goto err_out;
> +		return PTR_ERR(drvdata->rtc_clk);
>   	}
>   
> -	ret = watchdog_register_device(&drvdata->wdt);
> +	ret = devm_watchdog_register_device(&pdev->dev, &drvdata->wdt);
>   	if (ret < 0)
> -		goto err_disable_clk;
> +		return ret;
>   
>   	platform_set_drvdata(pdev, drvdata);
> -	return 0;
>   
> -err_disable_clk:
> -	clk_put(drvdata->rtc_clk);
> -err_out:
> -	return ret;
> +	return 0;
>   }
>   
>   static int jz4740_wdt_remove(struct platform_device *pdev)
>   {
>   	struct jz4740_wdt_drvdata *drvdata = platform_get_drvdata(pdev);
>   
> -	jz4740_wdt_stop(&drvdata->wdt);
> -	watchdog_unregister_device(&drvdata->wdt);
> -	clk_put(drvdata->rtc_clk);
> -
> -	return 0;
> +	return jz4740_wdt_stop(&drvdata->wdt);
>   }
>   
>   static struct platform_driver jz4740_wdt_driver = {
> 

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

^ permalink raw reply

* Re: [PATCH v4 04/15] clk: ingenic: Add code to enable/disable PLLs
From: Stephen Boyd @ 2017-12-28 18:39 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Ralf Baechle, Rob Herring, Michael Turquette, Mark Rutland,
	Maarten ter Huurne, devicetree, linux-kernel, linux-mips,
	linux-clk
In-Reply-To: <20171228135634.30000-5-paul@crapouillou.net>

On 12/28, Paul Cercueil wrote:
> This commit permits the PLLs to be dynamically enabled and disabled when
> their children clocks are enabled and disabled.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Acked-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v4 06/15] clk: Add Ingenic jz4770 CGU driver
From: Stephen Boyd @ 2017-12-28 18:38 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Ralf Baechle, Rob Herring, Michael Turquette, Mark Rutland,
	Maarten ter Huurne, devicetree, linux-kernel, linux-mips,
	linux-clk
In-Reply-To: <20171228135634.30000-7-paul@crapouillou.net>

On 12/28, Paul Cercueil wrote:
> Add support for the clocks provided by the CGU in the Ingenic JZ4770
> SoC.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Signed-off-by: Maarten ter Huurne <maarten@treewalker.org>
> ---

Acked-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH 1/7] watchdog: JZ4740: Disable clock after stopping counter
From: Guenter Roeck @ 2017-12-28 18:38 UTC (permalink / raw)
  To: Paul Cercueil, Ralf Baechle, Rob Herring, Mark Rutland,
	Wim Van Sebroeck
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-watchdog-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171228162939.3928-2-paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>

On 12/28/2017 08:29 AM, Paul Cercueil wrote:
> Previously, the clock was disabled first, which makes the watchdog
> component insensitive to register writes.
> 
> Signed-off-by: Paul Cercueil <paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>

Reviewed-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>

> ---
>   drivers/watchdog/jz4740_wdt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
> index 20627f22baf6..6955deb100ef 100644
> --- a/drivers/watchdog/jz4740_wdt.c
> +++ b/drivers/watchdog/jz4740_wdt.c
> @@ -124,8 +124,8 @@ static int jz4740_wdt_stop(struct watchdog_device *wdt_dev)
>   {
>   	struct jz4740_wdt_drvdata *drvdata = watchdog_get_drvdata(wdt_dev);
>   
> -	jz4740_timer_disable_watchdog();
>   	writeb(0x0, drvdata->base + JZ_REG_WDT_COUNTER_ENABLE);
> +	jz4740_timer_disable_watchdog();
>   
>   	return 0;
>   }
> 

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

^ permalink raw reply

* Re: [PATCH v4 05/15] dt-bindings: clock: Add jz4770-cgu.h header
From: Stephen Boyd @ 2017-12-28 18:36 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Ralf Baechle, Rob Herring, Michael Turquette, Mark Rutland,
	Maarten ter Huurne, devicetree, linux-kernel, linux-mips,
	linux-clk
In-Reply-To: <20171228135634.30000-6-paul@crapouillou.net>

On 12/28, Paul Cercueil wrote:
> This will be used from the devicetree bindings to specify the clocks
> that should be obtained from the jz4770-cgu driver.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Acked-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v4 03/15] clk: ingenic: support PLLs with no bypass bit
From: Stephen Boyd @ 2017-12-28 18:36 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Ralf Baechle, Rob Herring, Michael Turquette, Mark Rutland,
	Maarten ter Huurne, devicetree, linux-kernel, linux-mips,
	linux-clk
In-Reply-To: <20171228135634.30000-4-paul@crapouillou.net>

On 12/28, Paul Cercueil wrote:
> The second PLL of the JZ4770 does not have a bypass bit.
> This commit makes it possible to support it with the current common CGU
> code.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Acked-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v4 02/15] clk: ingenic: Fix recalc_rate for clocks with fixed divider
From: Stephen Boyd @ 2017-12-28 18:36 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Ralf Baechle, Rob Herring, Michael Turquette, Mark Rutland,
	Maarten ter Huurne, devicetree, linux-kernel, linux-mips,
	linux-clk
In-Reply-To: <20171228135634.30000-3-paul@crapouillou.net>

On 12/28, Paul Cercueil wrote:
> Previously, the clocks with a fixed divider would report their rate
> as being the same as the one of their parent, independently of the
> divider in use. This commit fixes this behaviour.
> 
> This went unnoticed as neither the jz4740 nor the jz4780 CGU code
> have clocks with fixed dividers yet.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---

Acked-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH 2/7] watchdog: jz4740: Use devm_* functions
From: Guenter Roeck @ 2017-12-28 17:48 UTC (permalink / raw)
  To: Paul Cercueil, Ralf Baechle, Rob Herring, Mark Rutland,
	Wim Van Sebroeck
  Cc: devicetree, linux-mips, linux-kernel, linux-watchdog
In-Reply-To: <20171228162939.3928-3-paul@crapouillou.net>

On 12/28/2017 08:29 AM, Paul Cercueil wrote:
> - Use devm_clk_get instead of clk_get
> - Use devm_watchdog_register_device instead of watchdog_register_device
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>   drivers/watchdog/jz4740_wdt.c | 27 ++++++++-------------------
>   1 file changed, 8 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
> index 6955deb100ef..92d6ca8ceb49 100644
> --- a/drivers/watchdog/jz4740_wdt.c
> +++ b/drivers/watchdog/jz4740_wdt.c
> @@ -178,40 +178,29 @@ static int jz4740_wdt_probe(struct platform_device *pdev)
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   	drvdata->base = devm_ioremap_resource(&pdev->dev, res);
> -	if (IS_ERR(drvdata->base)) {
> -		ret = PTR_ERR(drvdata->base);
> -		goto err_out;
> -	}
> +	if (IS_ERR(drvdata->base))
> +		return PTR_ERR(drvdata->base);
>   
> -	drvdata->rtc_clk = clk_get(&pdev->dev, "rtc");
> +	drvdata->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
>   	if (IS_ERR(drvdata->rtc_clk)) {
>   		dev_err(&pdev->dev, "cannot find RTC clock\n");
> -		ret = PTR_ERR(drvdata->rtc_clk);
> -		goto err_out;
> +		return PTR_ERR(drvdata->rtc_clk);
>   	}
>   
> -	ret = watchdog_register_device(&drvdata->wdt);
> +	ret = devm_watchdog_register_device(&pdev->dev, &drvdata->wdt);
>   	if (ret < 0)
> -		goto err_disable_clk;
> +		return ret;
>   
>   	platform_set_drvdata(pdev, drvdata);
> -	return 0;
>   
> -err_disable_clk:
> -	clk_put(drvdata->rtc_clk);
> -err_out:
> -	return ret;
> +	return 0;
>   }
>   
>   static int jz4740_wdt_remove(struct platform_device *pdev)
>   {
>   	struct jz4740_wdt_drvdata *drvdata = platform_get_drvdata(pdev);
>   
> -	jz4740_wdt_stop(&drvdata->wdt);
> -	watchdog_unregister_device(&drvdata->wdt);
> -	clk_put(drvdata->rtc_clk);
> -
> -	return 0;
> +	return jz4740_wdt_stop(&drvdata->wdt);

If the watchdog is running, the module can not be unloaded. Even if that wasn't
the case, this defeats both WDIOF_MAGICCLOSE and watchdog_set_nowayout().
Are you sure this is what you want ? If so, please call
watchdog_stop_on_unregister() before registration; this clarifies that this
is what you want, and you can drop the remove function.

Thanks,
Guenter

>   }
>   
>   static struct platform_driver jz4740_wdt_driver = {
> 

^ permalink raw reply

* Re: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF
From: Rafael J. Wysocki @ 2017-12-28 17:43 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: JeffyChen, Linux Kernel Mailing List, Bjorn Helgaas, Linux PM,
	Shawn Lin, Brian Norris, Doug Anderson,
	devicetree@vger.kernel.org, Linux PCI, Rob Herring, Frank Rowand
In-Reply-To: <6120485.xubBpvge6h@aspire.rjw.lan>

On Thu, Dec 28, 2017 at 6:29 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, December 28, 2017 5:51:34 PM CET Tony Lindgren wrote:
>> * Rafael J. Wysocki <rafael@kernel.org> [171228 12:21]:
>> > On Thu, Dec 28, 2017 at 5:22 AM, Tony Lindgren <tony@atomide.com> wrote:
>> > > * Rafael J. Wysocki <rafael@kernel.org> [171228 00:51]:
>> > >> On Wed, Dec 27, 2017 at 4:08 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > >> > * Rafael J. Wysocki <rjw@rjwysocki.net> [171227 01:00]:
>> > >> >> On Tuesday, December 26, 2017 2:06:47 AM CET JeffyChen wrote:
>> > >> >> > Hi Rafael,
>> > >> >> >
>> > >> >> > Thanks for your reply :)
>> > >> >> >
>> > >> >> > On 12/26/2017 08:11 AM, Rafael J. Wysocki wrote:
>> > >> >> > >> >+
>> > >> >> > >> >+       dn = pci_device_to_OF_node(ppdev);
>> > >> >> > >> >+       if (!dn)
>> > >> >> > >> >+               return 0;
>> > >> >> > >> >+
>> > >> >> > >> >+       irq = of_irq_get_byname(dn, "wakeup");
>> > >> >> > > Why is this a property of the bridge and not of the device itself?
>> > >> >> >
>> > >> >> > That is suggested by Brian, because in that way, the wakeup pin would
>> > >> >> > not "tied to what exact device is installed (or no device, if it's a slot)."
>> > >> >>
>> > >> >> But I don't think it works when there are two devices using different WAKE#
>> > >> >> interrupt lines under the same bridge.  Or how does it work then?
>> > >> >
>> > >> > It won't work currently for multiple devices but adding more than
>> > >> > one wakeriq per device is doable. And I think we will have other
>> > >> > cases where multiple wakeirqs are connected to a single device, so
>> > >> > that issue should be sorted out sooner or later.
>> > >> >
>> > >> > And if requesting wakeirq for the PCI WAKE# lines at the PCI
>> > >> > controller does the job, then maybe that's all we need to start with.
>> > >>
>> > >> These are expected to be out-of-band, so not having anything to do
>> > >> with the Root Complex.
>> > >>
>> > >> In-band PME Messages go through the PCIe hierarchy, but that is a
>> > >> standard mechanism and it is supported already.
>> > >>
>> > >> WAKE# are platform-specific, pretty much by definition and I guess
>> > >> that on most ARM boards they are just going to be some kind of GPIO
>> > >> pins.
>> > >
>> > > OK. So probably supporting the following two configurations
>> > > should be enough then:
>> > >
>> > > 1. One or more WAKE# lines configured as a wakeirq for the PCI
>> > >    controller
>> > >
>> > >    When the wakeirq calls pm_wakeup_event() for the PCI controller
>> > >    device driver, the PCI controller wakes up and can deal with
>> > >    it's child devices
>> >
>> > But this shouldn't be necessary at all.  Or if it is, I wonder why
>> > that's the case.
>>
>> Well Brian had a concern where we would have to implement PM runtime
>> for all device drivers for PCI devices.
>
> Why would we?
>
>> > I'm assuming that we're talking about PCI Express here, which has two
>> > wakeup mechanisms defined, one of which is based on using PME Messages
>> > (Beacon) and the second one is WAKE#:
>> >
>> > "The WAKE# mechanism uses sideband signaling to implement wakeup
>> > functionality. WAKE# is
>> > an “open drain” signal asserted by components requesting wakeup and
>> > observed by the associated
>> > power controller."
>> >
>> > (from PCIe Base Spec 3.0).  [And there's a diagram showing the routing
>> > of WAKE# in two cases in Figure 5-4: Conceptual Diagrams Showing Two
>> > Example Cases of WAKE# Routing.]
>>
>> Thanks for the pointer, I had not seen that :) So the use cases
>> I was trying to describe above are similar to the wiring in the
>> PCIe Base Spec 3.0 "Figure 5-4" , but numbered the other way around.
>>
>> > Note that WAKE# is defined to be "observed by the associated power
>> > controller", so I'm not sure what the PCI controller's role in the
>> > handing of it is at all.
>>
>> To me it seems the "switch" part stays at least partially powered
>> and then in-band PME messages are used after host is woken up to
>> figure out which WAKE# triggered?
>
> Yes, that's my understanding too.

To be precise, it is not quite possible to figure out which WAKE#
triggered, if they are sharing the line, without looking into the
config spaces of the devices below the switch.  The switch is not
expected to do that AFAICS.  It only generates a PME message meaning
"wakeup is being signaled somewhere below" and the PME driver that
handles the Root Port receiving it should look at the PME Status bits
of the devices below the switch (the pme.c driver does that IIRC or at
least it should do that ;-)).

Still, the handling of WAKE# doesn't need to cover this case AFAICS.

Thanks,
Rafael

^ permalink raw reply

* Re: [PATCH 5/7] MIPS: jz4780: dts: Fix watchdog node
From: Mathieu Malaterre @ 2017-12-28 17:33 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Ralf Baechle, Rob Herring, Mark Rutland, Wim Van Sebroeck,
	Guenter Roeck, devicetree, linux-mips, linux-kernel,
	linux-watchdog
In-Reply-To: <20171228162939.3928-6-paul@crapouillou.net>

Hi Paul,

On Thu, Dec 28, 2017 at 5:29 PM, Paul Cercueil <paul@crapouillou.net> wrote:
> - The previous node requested a memory area of 0x100 bytes, while the
>   driver only manipulates four registers present in the first 0x10 bytes.
>
> - The driver requests for the "rtc" clock, but the previous node did not
>   provide any.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  arch/mips/boot/dts/ingenic/jz4780.dtsi | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index 9b5794667aee..a52f59bf58c7 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -221,7 +221,10 @@
>
>         watchdog: watchdog@10002000 {
>                 compatible = "ingenic,jz4780-watchdog";
> -               reg = <0x10002000 0x100>;
> +               reg = <0x10002000 0x10>;
> +
> +               clocks = <&cgu JZ4780_CLK_RTCLK>;
> +               clock-names = "rtc";
>         };
>
>         nemc: nemc@13410000 {
> --
> 2.11.0
>
>

Looks good, thanks for fixing my mess. Tested on MIPS Creator CI20:

Dec 28 17:27:50 ci20 watchdog[17531]: starting daemon (5.14):
Dec 28 17:27:50 ci20 watchdog[17531]: int=1s realtime=yes sync=no
soft=no mla=0 mem=0
Dec 28 17:27:50 ci20 watchdog[17531]: ping: no machine to check
Dec 28 17:27:50 ci20 watchdog[17531]: file: no file to check
Dec 28 17:27:50 ci20 watchdog[17531]: pidfile: no server process to check
Dec 28 17:27:50 ci20 watchdog[17531]: interface: no interface to check
Dec 28 17:27:50 ci20 watchdog[17531]: temperature: no sensors to check
Dec 28 17:27:50 ci20 watchdog[17531]: test=none(0) repair=none(0)
alive=/dev/watchdog heartbeat=none to=root no_act=no force=no
Dec 28 17:27:50 ci20 watchdog[17531]: watchdog now set to 60 seconds
Dec 28 17:27:50 ci20 watchdog[17531]: hardware watchdog identity:
jz4740 Watchdog

pkill + reboot = ok

Reviewed-by: Mathieu Malaterre <malat@debian.org>

^ permalink raw reply

* Re: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF
From: Rafael J. Wysocki @ 2017-12-28 17:29 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Rafael J. Wysocki, JeffyChen, Linux Kernel Mailing List,
	Bjorn Helgaas, Linux PM, Shawn Lin, Brian Norris, Doug Anderson,
	devicetree@vger.kernel.org, Linux PCI, Rob Herring, Frank Rowand
In-Reply-To: <20171228165134.GH3875@atomide.com>

On Thursday, December 28, 2017 5:51:34 PM CET Tony Lindgren wrote:
> * Rafael J. Wysocki <rafael@kernel.org> [171228 12:21]:
> > On Thu, Dec 28, 2017 at 5:22 AM, Tony Lindgren <tony@atomide.com> wrote:
> > > * Rafael J. Wysocki <rafael@kernel.org> [171228 00:51]:
> > >> On Wed, Dec 27, 2017 at 4:08 PM, Tony Lindgren <tony@atomide.com> wrote:
> > >> > * Rafael J. Wysocki <rjw@rjwysocki.net> [171227 01:00]:
> > >> >> On Tuesday, December 26, 2017 2:06:47 AM CET JeffyChen wrote:
> > >> >> > Hi Rafael,
> > >> >> >
> > >> >> > Thanks for your reply :)
> > >> >> >
> > >> >> > On 12/26/2017 08:11 AM, Rafael J. Wysocki wrote:
> > >> >> > >> >+
> > >> >> > >> >+       dn = pci_device_to_OF_node(ppdev);
> > >> >> > >> >+       if (!dn)
> > >> >> > >> >+               return 0;
> > >> >> > >> >+
> > >> >> > >> >+       irq = of_irq_get_byname(dn, "wakeup");
> > >> >> > > Why is this a property of the bridge and not of the device itself?
> > >> >> >
> > >> >> > That is suggested by Brian, because in that way, the wakeup pin would
> > >> >> > not "tied to what exact device is installed (or no device, if it's a slot)."
> > >> >>
> > >> >> But I don't think it works when there are two devices using different WAKE#
> > >> >> interrupt lines under the same bridge.  Or how does it work then?
> > >> >
> > >> > It won't work currently for multiple devices but adding more than
> > >> > one wakeriq per device is doable. And I think we will have other
> > >> > cases where multiple wakeirqs are connected to a single device, so
> > >> > that issue should be sorted out sooner or later.
> > >> >
> > >> > And if requesting wakeirq for the PCI WAKE# lines at the PCI
> > >> > controller does the job, then maybe that's all we need to start with.
> > >>
> > >> These are expected to be out-of-band, so not having anything to do
> > >> with the Root Complex.
> > >>
> > >> In-band PME Messages go through the PCIe hierarchy, but that is a
> > >> standard mechanism and it is supported already.
> > >>
> > >> WAKE# are platform-specific, pretty much by definition and I guess
> > >> that on most ARM boards they are just going to be some kind of GPIO
> > >> pins.
> > >
> > > OK. So probably supporting the following two configurations
> > > should be enough then:
> > >
> > > 1. One or more WAKE# lines configured as a wakeirq for the PCI
> > >    controller
> > >
> > >    When the wakeirq calls pm_wakeup_event() for the PCI controller
> > >    device driver, the PCI controller wakes up and can deal with
> > >    it's child devices
> > 
> > But this shouldn't be necessary at all.  Or if it is, I wonder why
> > that's the case.
> 
> Well Brian had a concern where we would have to implement PM runtime
> for all device drivers for PCI devices.

Why would we?

> > I'm assuming that we're talking about PCI Express here, which has two
> > wakeup mechanisms defined, one of which is based on using PME Messages
> > (Beacon) and the second one is WAKE#:
> > 
> > "The WAKE# mechanism uses sideband signaling to implement wakeup
> > functionality. WAKE# is
> > an “open drain” signal asserted by components requesting wakeup and
> > observed by the associated
> > power controller."
> > 
> > (from PCIe Base Spec 3.0).  [And there's a diagram showing the routing
> > of WAKE# in two cases in Figure 5-4: Conceptual Diagrams Showing Two
> > Example Cases of WAKE# Routing.]
> 
> Thanks for the pointer, I had not seen that :) So the use cases
> I was trying to describe above are similar to the wiring in the
> PCIe Base Spec 3.0 "Figure 5-4" , but numbered the other way around.
> 
> > Note that WAKE# is defined to be "observed by the associated power
> > controller", so I'm not sure what the PCI controller's role in the
> > handing of it is at all.
> 
> To me it seems the "switch" part stays at least partially powered
> and then in-band PME messages are used after host is woken up to
> figure out which WAKE# triggered?

Yes, that's my understanding too.

> > > 2. Optionally a WAKE# line from a PCI device configured as wakeirq
> > >    for the PCI device driver
> > >
> > >    In this case calling the PM runtime resume in the child
> > >    PCI device will also wake up the parent PCI controller,
> > >    and then the PCI controller can deal with it's children
> > >
> > > Seems like this series is pretty close to 1 above except
> > > we need to have a list of wakeirqs per device instead of
> > > just one. And option 2 should already work as long as the
> > > PCI device driver parses and configures the wakeirq.
> > 
> > Well, this is confusing, because as I said above, option 1 doesn't
> > look relevant even.
> 
> So isn't my option 1 above similar to the PCIe spec "Figure 5-4"
> case 2?

No, it isn't, because in that case there is no practical difference
between WAKE# and an in-band PME message sent by the device (Beacon)
from the software perspective.

WAKE# causes the switch to send a PME message upstream and that is
handled by the Root Complex through the standard mechanism already
supported by our existing PME driver (drivers/pci/pcie/pme.c).

> Anyways, let's standardize on the "Figure 5-4" naming
> from now to avoid confusion :)

OK

> > >> > Then in addition to that, we could do the following to allow
> > >> > PCI devices to request the wakeirq from the PCI controller:
> > >> >
> > >> > 1. PCI controller or framework implements a chained irq for
> > >> >    the WAKE# lines assuming it can mask/unmask the WAKE# lines
> > >> >
> > >> > 2. PCI devices then can just request the wakeirq from the PCI
> > >> >    controller
> > >> >
> > >> > And that's about it. Optionally we could leave out the dependency
> > >> > to having PCI devices implement PM runtime and just resume the
> > >> > parent (PCI controller) if PCI devices has not implemented
> > >> > PM runtime.
> > >>
> > >> So if my understanding is correct, DT should give you the WAKE# IRQ
> > >> for the given endpoint PCI device and you only are expected to request
> > >> it.   The rest should just follow from the other pieces of information
> > >> in the DT.
> > >
> > > Yeah and it seems that we should allow configuring both cases
> > > 1 and 2 above.
> > >
> > >> With the quite obvious caveat that the same IRQ may be used as WAKE#
> > >> for multiple endpoint devices (which BTW need not be under the same
> > >> bridge even).
> > >
> > > And with the shared interrupts we can't do the masking/unmasking
> > > automatically..
> > 
> > Or we need to use reference counting (so actually the wakeup IRQs are
> > not dedicated).
> 
> Yeah. FYI, for the dedicated wakeirq cases I have, we need to keep
> them masked during runtime to avoid tons of interrupts as they
> are often wired to the RX pins.

OK

BTW, enable_irq_wake() should take care of the sharing, shouldn't it?

But the WAKE# thing is not just for waking up the system from sleep states,
it is for runtime PM's wakeup signaling too.

> > >> >> > >> >+       if (irq == -EPROBE_DEFER)
> > >> >> > > Braces here, please.
> > >> >> > ok, will fix in the next version.
> > >> >> >
> > >> >> > >
> > >> >> > >> >+               return irq;
> > >> >> > >> >+       /* Ignore other errors, since a missing wakeup is non-fatal. */
> > >> >> > >> >+       else if (irq < 0) {
> > >> >> > >> >+               dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
> > >> >> > >> >+               return 0;
> > >> >> > >> >+       }
> > >> >> > >> >+
> > >> >> > >> >+       device_init_wakeup(&pdev->dev, true);
> > >> >> > > Why do you call this before dev_pm_set_dedicated_wake_irq()?
> > >> >> >
> > >> >> > hmmm, i thought so too, but it turns out the dedicated wake irq
> > >> >> > framework requires device_init_wakeup(dev, true) before attach the wake irq:
> > >> >> >
> > >> >> > int device_wakeup_attach_irq(struct device *dev,
> > >> >> >                               struct wake_irq *wakeirq)
> > >> >> > {
> > >> >> >          struct wakeup_source *ws;
> > >> >> >
> > >> >> >          ws = dev->power.wakeup;
> > >> >> >          if (!ws) {
> > >> >> >                  dev_err(dev, "forgot to call device_init_wakeup?\n");
> > >> >> >                  return -EINVAL;
> > >> >> >
> > >> >>
> > >> >> Well, that's a framework issue, fair enough.
> > >> >>
> > >> >> That said, what if user space removes the wakeup source from under you
> > >> >> concurrently via sysfs?  Tony?
> > >> >
> > >> > Hmm sounds racy, need to take a look.
> > >>
> > >> Not only racy, as I don't see anything to prevent user space from
> > >> making the dev->power.wakeup wakeup source go away via sysfs at any
> > >> time *after* the IRQ has been requested.
> > >
> > > Currently nothing happens with wakeirqs if there's no struct
> > > wakeup_source. On device_wakeup_enable() we call device_wakeup_attach()
> > > that just copies dev->power.wakeirq to ws->wakeirq. And when struct
> > > wake_source is freed the device should be active and wakeirq
> > > disabled. Or are you seeing other issues here?
> > 
> > I'm suspicious about one thing, but I need to look deeper into the code. :-)

So we are fine except for the race and we need the wakeirq field in wakeup
sources to automatically arm the wakeup IRQs during suspend.

If I'm not mistaken, we only need something like the patch below (untested).

> OK. My response time will be laggy this week in case you find
> something that needs urgent fixing :)

No worries. :-)

---
 drivers/base/power/wakeirq.c |    9 ++++-----
 drivers/base/power/wakeup.c  |    2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/base/power/wakeirq.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeirq.c
+++ linux-pm/drivers/base/power/wakeirq.c
@@ -33,7 +33,6 @@ static int dev_pm_attach_wake_irq(struct
 				  struct wake_irq *wirq)
 {
 	unsigned long flags;
-	int err;
 
 	if (!dev || !wirq)
 		return -EINVAL;
@@ -45,12 +44,12 @@ static int dev_pm_attach_wake_irq(struct
 		return -EEXIST;
 	}
 
-	err = device_wakeup_attach_irq(dev, wirq);
-	if (!err)
-		dev->power.wakeirq = wirq;
+	dev->power.wakeirq = wirq;
+	if (dev->power.wakeup)
+		device_wakeup_attach_irq(dev, wirq);
 
 	spin_unlock_irqrestore(&dev->power.lock, flags);
-	return err;
+	return 0;
 }
 
 /**
Index: linux-pm/drivers/base/power/wakeup.c
===================================================================
--- linux-pm.orig/drivers/base/power/wakeup.c
+++ linux-pm/drivers/base/power/wakeup.c
@@ -303,7 +303,7 @@ int device_wakeup_attach_irq(struct devi
 	}
 
 	if (ws->wakeirq)
-		return -EEXIST;
+		dev_err(dev, "Leftover wakeup IRQ found, overriding\n");
 
 	ws->wakeirq = wakeirq;
 	return 0;

^ permalink raw reply

* Re: [PATCH net-next v9 0/2] add UniPhier AVE ethernet support
From: David Miller @ 2017-12-28 17:11 UTC (permalink / raw)
  To: hayashi.kunihiko
  Cc: netdev, andrew, f.fainelli, robh+dt, mark.rutland,
	linux-arm-kernel, linux-kernel, devicetree, yamada.masahiro,
	masami.hiramatsu, jaswinder.singh
In-Reply-To: <1514444292-20643-1-git-send-email-hayashi.kunihiko@socionext.com>

From: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Date: Thu, 28 Dec 2017 15:58:10 +0900

> This series adds support for Socionext AVE ethernet controller implemented
> on UniPhier SoCs. This driver supports RGMII/RMII modes.

Series applied.

^ permalink raw reply

* Re: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF
From: Tony Lindgren @ 2017-12-28 16:51 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, JeffyChen, Linux Kernel Mailing List,
	Bjorn Helgaas, Linux PM, Shawn Lin, Brian Norris, Doug Anderson,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux PCI,
	Rob Herring, Frank Rowand
In-Reply-To: <CAJZ5v0hpK0_vjX3HinCpsFuKffVUn3d5EnqXdz0P893aRZgnRw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

* Rafael J. Wysocki <rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [171228 12:21]:
> On Thu, Dec 28, 2017 at 5:22 AM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> > * Rafael J. Wysocki <rafael-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [171228 00:51]:
> >> On Wed, Dec 27, 2017 at 4:08 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
> >> > * Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org> [171227 01:00]:
> >> >> On Tuesday, December 26, 2017 2:06:47 AM CET JeffyChen wrote:
> >> >> > Hi Rafael,
> >> >> >
> >> >> > Thanks for your reply :)
> >> >> >
> >> >> > On 12/26/2017 08:11 AM, Rafael J. Wysocki wrote:
> >> >> > >> >+
> >> >> > >> >+       dn = pci_device_to_OF_node(ppdev);
> >> >> > >> >+       if (!dn)
> >> >> > >> >+               return 0;
> >> >> > >> >+
> >> >> > >> >+       irq = of_irq_get_byname(dn, "wakeup");
> >> >> > > Why is this a property of the bridge and not of the device itself?
> >> >> >
> >> >> > That is suggested by Brian, because in that way, the wakeup pin would
> >> >> > not "tied to what exact device is installed (or no device, if it's a slot)."
> >> >>
> >> >> But I don't think it works when there are two devices using different WAKE#
> >> >> interrupt lines under the same bridge.  Or how does it work then?
> >> >
> >> > It won't work currently for multiple devices but adding more than
> >> > one wakeriq per device is doable. And I think we will have other
> >> > cases where multiple wakeirqs are connected to a single device, so
> >> > that issue should be sorted out sooner or later.
> >> >
> >> > And if requesting wakeirq for the PCI WAKE# lines at the PCI
> >> > controller does the job, then maybe that's all we need to start with.
> >>
> >> These are expected to be out-of-band, so not having anything to do
> >> with the Root Complex.
> >>
> >> In-band PME Messages go through the PCIe hierarchy, but that is a
> >> standard mechanism and it is supported already.
> >>
> >> WAKE# are platform-specific, pretty much by definition and I guess
> >> that on most ARM boards they are just going to be some kind of GPIO
> >> pins.
> >
> > OK. So probably supporting the following two configurations
> > should be enough then:
> >
> > 1. One or more WAKE# lines configured as a wakeirq for the PCI
> >    controller
> >
> >    When the wakeirq calls pm_wakeup_event() for the PCI controller
> >    device driver, the PCI controller wakes up and can deal with
> >    it's child devices
> 
> But this shouldn't be necessary at all.  Or if it is, I wonder why
> that's the case.

Well Brian had a concern where we would have to implement PM runtime
for all device drivers for PCI devices.

> I'm assuming that we're talking about PCI Express here, which has two
> wakeup mechanisms defined, one of which is based on using PME Messages
> (Beacon) and the second one is WAKE#:
> 
> "The WAKE# mechanism uses sideband signaling to implement wakeup
> functionality. WAKE# is
> an “open drain” signal asserted by components requesting wakeup and
> observed by the associated
> power controller."
> 
> (from PCIe Base Spec 3.0).  [And there's a diagram showing the routing
> of WAKE# in two cases in Figure 5-4: Conceptual Diagrams Showing Two
> Example Cases of WAKE# Routing.]

Thanks for the pointer, I had not seen that :) So the use cases
I was trying to describe above are similar to the wiring in the
PCIe Base Spec 3.0 "Figure 5-4" , but numbered the other way around.

> Note that WAKE# is defined to be "observed by the associated power
> controller", so I'm not sure what the PCI controller's role in the
> handing of it is at all.

To me it seems the "switch" part stays at least partially powered
and then in-band PME messages are used after host is woken up to
figure out which WAKE# triggered?

> > 2. Optionally a WAKE# line from a PCI device configured as wakeirq
> >    for the PCI device driver
> >
> >    In this case calling the PM runtime resume in the child
> >    PCI device will also wake up the parent PCI controller,
> >    and then the PCI controller can deal with it's children
> >
> > Seems like this series is pretty close to 1 above except
> > we need to have a list of wakeirqs per device instead of
> > just one. And option 2 should already work as long as the
> > PCI device driver parses and configures the wakeirq.
> 
> Well, this is confusing, because as I said above, option 1 doesn't
> look relevant even.

So isn't my option 1 above similar to the PCIe spec "Figure 5-4"
case 2? Anyways, let's standardize on the "Figure 5-4" naming
from now to avoid confusion :)

> >> > Then in addition to that, we could do the following to allow
> >> > PCI devices to request the wakeirq from the PCI controller:
> >> >
> >> > 1. PCI controller or framework implements a chained irq for
> >> >    the WAKE# lines assuming it can mask/unmask the WAKE# lines
> >> >
> >> > 2. PCI devices then can just request the wakeirq from the PCI
> >> >    controller
> >> >
> >> > And that's about it. Optionally we could leave out the dependency
> >> > to having PCI devices implement PM runtime and just resume the
> >> > parent (PCI controller) if PCI devices has not implemented
> >> > PM runtime.
> >>
> >> So if my understanding is correct, DT should give you the WAKE# IRQ
> >> for the given endpoint PCI device and you only are expected to request
> >> it.   The rest should just follow from the other pieces of information
> >> in the DT.
> >
> > Yeah and it seems that we should allow configuring both cases
> > 1 and 2 above.
> >
> >> With the quite obvious caveat that the same IRQ may be used as WAKE#
> >> for multiple endpoint devices (which BTW need not be under the same
> >> bridge even).
> >
> > And with the shared interrupts we can't do the masking/unmasking
> > automatically..
> 
> Or we need to use reference counting (so actually the wakeup IRQs are
> not dedicated).

Yeah. FYI, for the dedicated wakeirq cases I have, we need to keep
them masked during runtime to avoid tons of interrupts as they
are often wired to the RX pins.

> >> >> > >> >+       if (irq == -EPROBE_DEFER)
> >> >> > > Braces here, please.
> >> >> > ok, will fix in the next version.
> >> >> >
> >> >> > >
> >> >> > >> >+               return irq;
> >> >> > >> >+       /* Ignore other errors, since a missing wakeup is non-fatal. */
> >> >> > >> >+       else if (irq < 0) {
> >> >> > >> >+               dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
> >> >> > >> >+               return 0;
> >> >> > >> >+       }
> >> >> > >> >+
> >> >> > >> >+       device_init_wakeup(&pdev->dev, true);
> >> >> > > Why do you call this before dev_pm_set_dedicated_wake_irq()?
> >> >> >
> >> >> > hmmm, i thought so too, but it turns out the dedicated wake irq
> >> >> > framework requires device_init_wakeup(dev, true) before attach the wake irq:
> >> >> >
> >> >> > int device_wakeup_attach_irq(struct device *dev,
> >> >> >                               struct wake_irq *wakeirq)
> >> >> > {
> >> >> >          struct wakeup_source *ws;
> >> >> >
> >> >> >          ws = dev->power.wakeup;
> >> >> >          if (!ws) {
> >> >> >                  dev_err(dev, "forgot to call device_init_wakeup?\n");
> >> >> >                  return -EINVAL;
> >> >> >
> >> >>
> >> >> Well, that's a framework issue, fair enough.
> >> >>
> >> >> That said, what if user space removes the wakeup source from under you
> >> >> concurrently via sysfs?  Tony?
> >> >
> >> > Hmm sounds racy, need to take a look.
> >>
> >> Not only racy, as I don't see anything to prevent user space from
> >> making the dev->power.wakeup wakeup source go away via sysfs at any
> >> time *after* the IRQ has been requested.
> >
> > Currently nothing happens with wakeirqs if there's no struct
> > wakeup_source. On device_wakeup_enable() we call device_wakeup_attach()
> > that just copies dev->power.wakeirq to ws->wakeirq. And when struct
> > wake_source is freed the device should be active and wakeirq
> > disabled. Or are you seeing other issues here?
> 
> I'm suspicious about one thing, but I need to look deeper into the code. :-)

OK. My response time will be laggy this week in case you find
something that needs urgent fixing :)

Regards,

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

^ permalink raw reply

* Re: [PATCH v2 0/2] sun8i-a83t: Add touchscreen support on TBS A711
From: Mylene JOSSERAND @ 2017-12-28 16:46 UTC (permalink / raw)
  To: dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <20171228163336.28131-1-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Hello,

Le Thu, 28 Dec 2017 17:33:34 +0100,
Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> a écrit :

> Hello everyone,
> 
> This is a V2 of the patch series that adds touchscreen support
> (FocalTech EDT-FT5x06 Polytouch) for TBS A711 (Allwinner sun8i-a83t SoC).
> Based on last linux-next (next-20171222).
> 
> Changes since v1:
>    - Remove patches 01 and 02 as Chen-Yu Tsai sent a similar patch:
>    https://patchwork.kernel.org/patch/10111431/
>    and it is merged on last next-20171222.
>    (See commit f066f46ce5a5 "ARM: dts: sun8i: a83t: Add I2C device nodes and pinmux settings")
>    - Update regulator according to Dmitry Torokhov's review: remove "optional"
>    suffix while retrieving the regulator, rename it into "vcc" instead of
>    "power" and add bindings documentation.

I notice that I forgot the second review of Dmitry about reset/wake
gpios so I will send a V3 with the modifications.

Thanks,

Mylène

>    - Update device tree according to Maxime Ripard's review: remove the
>    label and rename the node.
>    - Squash patch 03 with patch 05 to add I2C0 and touchscreen's node
>    in one patch (see patch 02).
> 
> Patch 01: Add support for regulator in the FocalTech touchscreen driver
> because A711 tablet is using a regulator to power-up the touchscreen.
> Patch 02: Add i2c0 and touchscreen's node for A711 TBS tablet.
> 
> Thank you in advance for any review.
> Best regards,
> Mylène
> 
> Mylène Josserand (2):
>   Input: edt-ft5x06 - Add support for regulator
>   arm: dts: sun8i: a83t: a711: Add touchscreen node
> 
>  .../bindings/input/touchscreen/edt-ft5x06.txt      |  1 +
>  arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts          | 16 +++++++++++
>  drivers/input/touchscreen/edt-ft5x06.c             | 33 ++++++++++++++++++++++
>  3 files changed, 50 insertions(+)
> 

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

^ permalink raw reply

* [PATCH v2 2/2] arm: dts: sun8i: a83t: a711: Add touchscreen node
From: Mylène Josserand @ 2017-12-28 16:33 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, mark.rutland, linux, maxime.ripard,
	wens
  Cc: thomas.petazzoni, devicetree, linux-kernel, quentin.schulz,
	linux-input, mylene.josserand, linux-arm-kernel
In-Reply-To: <20171228163336.28131-1-mylene.josserand@free-electrons.com>

Tha A711 tablet has a FocalTech EDT-FT5x06 Polytouch touchscreen.
It is connected via I2C0. The reset line is PD5, the interrupt
line is PL7 and the VCC supply is the ldo_io0 regulator.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
index a021ee6da396..7840f9aa9094 100644
--- a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
@@ -105,6 +105,22 @@
 	status = "okay";
 };
 
+&i2c0 {
+	clock-frequency = <400000>;
+	status = "okay";
+
+	touchscreen@38 {
+		compatible = "edt,edt-ft5x06";
+		reg = <0x38>;
+		interrupt-parent = <&r_pio>;
+		interrupts = <0 7 IRQ_TYPE_EDGE_FALLING>;
+		reset-gpios = <&pio 3 5 GPIO_ACTIVE_LOW>;
+		vcc-supply = <&reg_ldo_io0>;
+		touchscreen-size-x = <1024>;
+		touchscreen-size-y = <600>;
+	};
+};
+
 &mmc0 {
 	vmmc-supply = <&reg_dcdc1>;
 	pinctrl-names = "default";
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Mylène Josserand @ 2017-12-28 16:33 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, mark.rutland, linux, maxime.ripard,
	wens
  Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
	mylene.josserand, thomas.petazzoni, quentin.schulz
In-Reply-To: <20171228163336.28131-1-mylene.josserand@free-electrons.com>

Add the support of regulator to use it as VCC source.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 .../bindings/input/touchscreen/edt-ft5x06.txt      |  1 +
 drivers/input/touchscreen/edt-ft5x06.c             | 33 ++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index 025cf8c9324a..48e975b9c1aa 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -30,6 +30,7 @@ Required properties:
 Optional properties:
  - reset-gpios: GPIO specification for the RESET input
  - wake-gpios:  GPIO specification for the WAKE input
+ - vcc-supply:  Regulator that supplies the touchscreen
 
  - pinctrl-names: should be "default"
  - pinctrl-0:   a phandle pointing to the pin settings for the
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index c53a3d7239e7..5ee14a25a382 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -39,6 +39,7 @@
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
 #include <linux/of_device.h>
+#include <linux/regulator/consumer.h>
 
 #define WORK_REGISTER_THRESHOLD		0x00
 #define WORK_REGISTER_REPORT_RATE	0x08
@@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
 	struct touchscreen_properties prop;
 	u16 num_x;
 	u16 num_y;
+	struct regulator *vcc;
 
 	struct gpio_desc *reset_gpio;
 	struct gpio_desc *wake_gpio;
@@ -993,6 +995,23 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
 
 	tsdata->max_support_points = chip_data->max_support_points;
 
+	tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
+	if (IS_ERR(tsdata->vcc)) {
+		error = PTR_ERR(tsdata->vcc);
+		dev_err(&client->dev, "failed to request regulator: %d\n",
+			error);
+		return error;
+	};
+
+	if (tsdata->vcc) {
+		error = regulator_enable(tsdata->vcc);
+		if (error < 0) {
+			dev_err(&client->dev, "failed to enable vcc: %d\n",
+				error);
+			return error;
+		}
+	}
+
 	tsdata->reset_gpio = devm_gpiod_get_optional(&client->dev,
 						     "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(tsdata->reset_gpio)) {
@@ -1122,20 +1141,34 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
 static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
 
 	if (device_may_wakeup(dev))
 		enable_irq_wake(client->irq);
 
+	if (tsdata->vcc)
+		regulator_disable(tsdata->vcc);
+
 	return 0;
 }
 
 static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
+	int ret;
 
 	if (device_may_wakeup(dev))
 		disable_irq_wake(client->irq);
 
+	if (tsdata->vcc) {
+		ret = regulator_enable(tsdata->vcc);
+		if (ret < 0) {
+			dev_err(dev, "failed to enable vcc: %d\n", ret);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v2 0/2] sun8i-a83t: Add touchscreen support on TBS A711
From: Mylène Josserand @ 2017-12-28 16:33 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, mark.rutland, linux, maxime.ripard,
	wens
  Cc: linux-arm-kernel, linux-input, devicetree, linux-kernel,
	mylene.josserand, thomas.petazzoni, quentin.schulz

Hello everyone,

This is a V2 of the patch series that adds touchscreen support
(FocalTech EDT-FT5x06 Polytouch) for TBS A711 (Allwinner sun8i-a83t SoC).
Based on last linux-next (next-20171222).

Changes since v1:
   - Remove patches 01 and 02 as Chen-Yu Tsai sent a similar patch:
   https://patchwork.kernel.org/patch/10111431/
   and it is merged on last next-20171222.
   (See commit f066f46ce5a5 "ARM: dts: sun8i: a83t: Add I2C device nodes and pinmux settings")
   - Update regulator according to Dmitry Torokhov's review: remove "optional"
   suffix while retrieving the regulator, rename it into "vcc" instead of
   "power" and add bindings documentation.
   - Update device tree according to Maxime Ripard's review: remove the
   label and rename the node.
   - Squash patch 03 with patch 05 to add I2C0 and touchscreen's node
   in one patch (see patch 02).

Patch 01: Add support for regulator in the FocalTech touchscreen driver
because A711 tablet is using a regulator to power-up the touchscreen.
Patch 02: Add i2c0 and touchscreen's node for A711 TBS tablet.

Thank you in advance for any review.
Best regards,
Mylène

Mylène Josserand (2):
  Input: edt-ft5x06 - Add support for regulator
  arm: dts: sun8i: a83t: a711: Add touchscreen node

 .../bindings/input/touchscreen/edt-ft5x06.txt      |  1 +
 arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts          | 16 +++++++++++
 drivers/input/touchscreen/edt-ft5x06.c             | 33 ++++++++++++++++++++++
 3 files changed, 50 insertions(+)

-- 
2.11.0

^ permalink raw reply

* [PATCH 7/7] MIPS: jz4740: Drop old platform reset code
From: Paul Cercueil @ 2017-12-28 16:29 UTC (permalink / raw)
  To: Ralf Baechle, Rob Herring, Mark Rutland, Wim Van Sebroeck,
	Guenter Roeck
  Cc: devicetree, linux-mips, linux-kernel, linux-watchdog,
	Paul Cercueil
In-Reply-To: <20171228162939.3928-1-paul@crapouillou.net>

This work is now performed by the watchdog driver directly.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 arch/mips/jz4740/reset.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/arch/mips/jz4740/reset.c b/arch/mips/jz4740/reset.c
index 67780c4b6573..5bf0cf44b55f 100644
--- a/arch/mips/jz4740/reset.c
+++ b/arch/mips/jz4740/reset.c
@@ -12,18 +12,9 @@
  *
  */
 
-#include <linux/clk.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/pm.h>
-
 #include <asm/reboot.h>
 
-#include <asm/mach-jz4740/base.h>
-#include <asm/mach-jz4740/timer.h>
-
 #include "reset.h"
-#include "clock.h"
 
 static void jz4740_halt(void)
 {
@@ -36,29 +27,7 @@ static void jz4740_halt(void)
 	}
 }
 
-#define JZ_REG_WDT_DATA 0x00
-#define JZ_REG_WDT_COUNTER_ENABLE 0x04
-#define JZ_REG_WDT_COUNTER 0x08
-#define JZ_REG_WDT_CTRL 0x0c
-
-static void jz4740_restart(char *command)
-{
-	void __iomem *wdt_base = ioremap(JZ4740_WDT_BASE_ADDR, 0x0f);
-
-	jz4740_timer_enable_watchdog();
-
-	writeb(0, wdt_base + JZ_REG_WDT_COUNTER_ENABLE);
-
-	writew(0, wdt_base + JZ_REG_WDT_COUNTER);
-	writew(0, wdt_base + JZ_REG_WDT_DATA);
-	writew(BIT(2), wdt_base + JZ_REG_WDT_CTRL);
-
-	writeb(1, wdt_base + JZ_REG_WDT_COUNTER_ENABLE);
-	jz4740_halt();
-}
-
 void jz4740_reset_init(void)
 {
-	_machine_restart = jz4740_restart;
 	_machine_halt = jz4740_halt;
 }
-- 
2.11.0

^ permalink raw reply related

* [PATCH 6/7] MIPS: qi_lb60: Enable the jz4740-wdt driver
From: Paul Cercueil @ 2017-12-28 16:29 UTC (permalink / raw)
  To: Ralf Baechle, Rob Herring, Mark Rutland, Wim Van Sebroeck,
	Guenter Roeck
  Cc: devicetree, linux-mips, linux-kernel, linux-watchdog,
	Paul Cercueil
In-Reply-To: <20171228162939.3928-1-paul@crapouillou.net>

The watchdog is an useful piece of hardware, so there's no reason not to
enable it.

This commit enables the Kconfig option in the qi_lb60 defconfig.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 arch/mips/configs/qi_lb60_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/configs/qi_lb60_defconfig b/arch/mips/configs/qi_lb60_defconfig
index 3f1333517405..ba8e1c56b626 100644
--- a/arch/mips/configs/qi_lb60_defconfig
+++ b/arch/mips/configs/qi_lb60_defconfig
@@ -73,6 +73,8 @@ CONFIG_POWER_SUPPLY=y
 CONFIG_BATTERY_JZ4740=y
 CONFIG_CHARGER_GPIO=y
 # CONFIG_HWMON is not set
+CONFIG_WATCHDOG=y
+CONFIG_JZ4740_WDT=y
 CONFIG_MFD_JZ4740_ADC=y
 CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
-- 
2.11.0

^ permalink raw reply related

* [PATCH 5/7] MIPS: jz4780: dts: Fix watchdog node
From: Paul Cercueil @ 2017-12-28 16:29 UTC (permalink / raw)
  To: Ralf Baechle, Rob Herring, Mark Rutland, Wim Van Sebroeck,
	Guenter Roeck
  Cc: devicetree, linux-mips, linux-kernel, linux-watchdog,
	Paul Cercueil
In-Reply-To: <20171228162939.3928-1-paul@crapouillou.net>

- The previous node requested a memory area of 0x100 bytes, while the
  driver only manipulates four registers present in the first 0x10 bytes.

- The driver requests for the "rtc" clock, but the previous node did not
  provide any.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 arch/mips/boot/dts/ingenic/jz4780.dtsi | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
index 9b5794667aee..a52f59bf58c7 100644
--- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
@@ -221,7 +221,10 @@
 
 	watchdog: watchdog@10002000 {
 		compatible = "ingenic,jz4780-watchdog";
-		reg = <0x10002000 0x100>;
+		reg = <0x10002000 0x10>;
+
+		clocks = <&cgu JZ4780_CLK_RTCLK>;
+		clock-names = "rtc";
 	};
 
 	nemc: nemc@13410000 {
-- 
2.11.0

^ permalink raw reply related

* [PATCH 4/7] MIPS: jz4740: dts: Add bindings for the jz4740-wdt driver
From: Paul Cercueil @ 2017-12-28 16:29 UTC (permalink / raw)
  To: Ralf Baechle, Rob Herring, Mark Rutland, Wim Van Sebroeck,
	Guenter Roeck
  Cc: devicetree, linux-mips, linux-kernel, linux-watchdog,
	Paul Cercueil
In-Reply-To: <20171228162939.3928-1-paul@crapouillou.net>

Also remove the watchdog platform_device from platform.c, since it
wasn't used anywhere anyway.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 arch/mips/boot/dts/ingenic/jz4740.dtsi |  8 ++++++++
 arch/mips/jz4740/platform.c            | 16 ----------------
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
index cd5185bb90ae..26c6b561d6f7 100644
--- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
@@ -45,6 +45,14 @@
 		#clock-cells = <1>;
 	};
 
+	watchdog: watchdog@10002000 {
+		compatible = "ingenic,jz4740-watchdog";
+		reg = <0x10002000 0x10>;
+
+		clocks = <&cgu JZ4740_CLK_RTC>;
+		clock-names = "rtc";
+	};
+
 	rtc_dev: rtc@10003000 {
 		compatible = "ingenic,jz4740-rtc";
 		reg = <0x10003000 0x40>;
diff --git a/arch/mips/jz4740/platform.c b/arch/mips/jz4740/platform.c
index 5b7cdd67a9d9..cbc5f8e87230 100644
--- a/arch/mips/jz4740/platform.c
+++ b/arch/mips/jz4740/platform.c
@@ -233,22 +233,6 @@ struct platform_device jz4740_adc_device = {
 	.resource	= jz4740_adc_resources,
 };
 
-/* Watchdog */
-static struct resource jz4740_wdt_resources[] = {
-	{
-		.start = JZ4740_WDT_BASE_ADDR,
-		.end   = JZ4740_WDT_BASE_ADDR + 0x10 - 1,
-		.flags = IORESOURCE_MEM,
-	},
-};
-
-struct platform_device jz4740_wdt_device = {
-	.name	       = "jz4740-wdt",
-	.id	       = -1,
-	.num_resources = ARRAY_SIZE(jz4740_wdt_resources),
-	.resource      = jz4740_wdt_resources,
-};
-
 /* PWM */
 struct platform_device jz4740_pwm_device = {
 	.name = "jz4740-pwm",
-- 
2.11.0

^ permalink raw reply related

* [PATCH 3/7] watchdog: JZ4740: Register a restart handler
From: Paul Cercueil @ 2017-12-28 16:29 UTC (permalink / raw)
  To: Ralf Baechle, Rob Herring, Mark Rutland, Wim Van Sebroeck,
	Guenter Roeck
  Cc: devicetree, linux-mips, linux-kernel, linux-watchdog,
	Paul Cercueil
In-Reply-To: <20171228162939.3928-1-paul@crapouillou.net>

The watchdog driver can restart the system by simply configuring the
hardware for a timeout of 0 seconds.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/watchdog/jz4740_wdt.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
index 92d6ca8ceb49..fa7f49a3212c 100644
--- a/drivers/watchdog/jz4740_wdt.c
+++ b/drivers/watchdog/jz4740_wdt.c
@@ -130,6 +130,14 @@ static int jz4740_wdt_stop(struct watchdog_device *wdt_dev)
 	return 0;
 }
 
+static int jz4740_wdt_restart(struct watchdog_device *wdt_dev,
+			      unsigned long action, void *data)
+{
+	wdt_dev->timeout = 0;
+	jz4740_wdt_start(wdt_dev);
+	return 0;
+}
+
 static const struct watchdog_info jz4740_wdt_info = {
 	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
 	.identity = "jz4740 Watchdog",
@@ -141,6 +149,7 @@ static const struct watchdog_ops jz4740_wdt_ops = {
 	.stop = jz4740_wdt_stop,
 	.ping = jz4740_wdt_ping,
 	.set_timeout = jz4740_wdt_set_timeout,
+	.restart = jz4740_wdt_restart,
 };
 
 #ifdef CONFIG_OF
-- 
2.11.0

^ permalink raw reply related

* [PATCH 2/7] watchdog: jz4740: Use devm_* functions
From: Paul Cercueil @ 2017-12-28 16:29 UTC (permalink / raw)
  To: Ralf Baechle, Rob Herring, Mark Rutland, Wim Van Sebroeck,
	Guenter Roeck
  Cc: devicetree, linux-mips, linux-kernel, linux-watchdog,
	Paul Cercueil
In-Reply-To: <20171228162939.3928-1-paul@crapouillou.net>

- Use devm_clk_get instead of clk_get
- Use devm_watchdog_register_device instead of watchdog_register_device

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/watchdog/jz4740_wdt.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
index 6955deb100ef..92d6ca8ceb49 100644
--- a/drivers/watchdog/jz4740_wdt.c
+++ b/drivers/watchdog/jz4740_wdt.c
@@ -178,40 +178,29 @@ static int jz4740_wdt_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	drvdata->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(drvdata->base)) {
-		ret = PTR_ERR(drvdata->base);
-		goto err_out;
-	}
+	if (IS_ERR(drvdata->base))
+		return PTR_ERR(drvdata->base);
 
-	drvdata->rtc_clk = clk_get(&pdev->dev, "rtc");
+	drvdata->rtc_clk = devm_clk_get(&pdev->dev, "rtc");
 	if (IS_ERR(drvdata->rtc_clk)) {
 		dev_err(&pdev->dev, "cannot find RTC clock\n");
-		ret = PTR_ERR(drvdata->rtc_clk);
-		goto err_out;
+		return PTR_ERR(drvdata->rtc_clk);
 	}
 
-	ret = watchdog_register_device(&drvdata->wdt);
+	ret = devm_watchdog_register_device(&pdev->dev, &drvdata->wdt);
 	if (ret < 0)
-		goto err_disable_clk;
+		return ret;
 
 	platform_set_drvdata(pdev, drvdata);
-	return 0;
 
-err_disable_clk:
-	clk_put(drvdata->rtc_clk);
-err_out:
-	return ret;
+	return 0;
 }
 
 static int jz4740_wdt_remove(struct platform_device *pdev)
 {
 	struct jz4740_wdt_drvdata *drvdata = platform_get_drvdata(pdev);
 
-	jz4740_wdt_stop(&drvdata->wdt);
-	watchdog_unregister_device(&drvdata->wdt);
-	clk_put(drvdata->rtc_clk);
-
-	return 0;
+	return jz4740_wdt_stop(&drvdata->wdt);
 }
 
 static struct platform_driver jz4740_wdt_driver = {
-- 
2.11.0

^ permalink raw reply related

* [PATCH 1/7] watchdog: JZ4740: Disable clock after stopping counter
From: Paul Cercueil @ 2017-12-28 16:29 UTC (permalink / raw)
  To: Ralf Baechle, Rob Herring, Mark Rutland, Wim Van Sebroeck,
	Guenter Roeck
  Cc: devicetree, linux-mips, linux-kernel, linux-watchdog,
	Paul Cercueil
In-Reply-To: <20171228162939.3928-1-paul@crapouillou.net>

Previously, the clock was disabled first, which makes the watchdog
component insensitive to register writes.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/watchdog/jz4740_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c
index 20627f22baf6..6955deb100ef 100644
--- a/drivers/watchdog/jz4740_wdt.c
+++ b/drivers/watchdog/jz4740_wdt.c
@@ -124,8 +124,8 @@ static int jz4740_wdt_stop(struct watchdog_device *wdt_dev)
 {
 	struct jz4740_wdt_drvdata *drvdata = watchdog_get_drvdata(wdt_dev);
 
-	jz4740_timer_disable_watchdog();
 	writeb(0x0, drvdata->base + JZ_REG_WDT_COUNTER_ENABLE);
+	jz4740_timer_disable_watchdog();
 
 	return 0;
 }
-- 
2.11.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox