Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1674152 > unrolled thread
| Started by | Guenter Roeck <linux@roeck-us.net> |
|---|---|
| First post | 2017-06-25 00:20 +0200 |
| Last post | 2017-06-27 04:20 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [2/2] drivers/watchdog: ASPEED reference dev tree properties for config Guenter Roeck <linux@roeck-us.net> - 2017-06-25 00:20 +0200
Re: [2/2] drivers/watchdog: ASPEED reference dev tree properties for config Christopher Bostic <cbostic@linux.vnet.ibm.com> - 2017-06-27 04:20 +0200
| From | Guenter Roeck <linux@roeck-us.net> |
|---|---|
| Date | 2017-06-25 00:20 +0200 |
| Subject | Re: [2/2] drivers/watchdog: ASPEED reference dev tree properties for config |
| Message-ID | <tW1zj-54a-1@gated-at.bofh.it> |
On Tue, Jun 13, 2017 at 03:38:18PM -0500, Christopher Bostic wrote:
> Reference the system device tree when configuring the watchdog.
> Configure for external signal generation if optional attribute
> 'external-signal' is present in device tree. Configure for
> reset system after timeout if the optional attribute
> 'no-system-reset' is not present. Disabling system reset may be
> required in situations where one of the other watchdogs in the
> systems is responsible for this.
>
> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
> ---
> drivers/watchdog/aspeed_wdt.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
> index 1c65258..9deb4be 100644
> --- a/drivers/watchdog/aspeed_wdt.c
> +++ b/drivers/watchdog/aspeed_wdt.c
> @@ -140,6 +140,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> {
> struct aspeed_wdt *wdt;
> struct resource *res;
> + struct device_node *np;
> int ret;
>
> wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> @@ -170,8 +171,16 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
> * the SOC and not the full chip
> */
> wdt->ctrl = WDT_CTRL_RESET_MODE_SOC |
> - WDT_CTRL_1MHZ_CLK |
> - WDT_CTRL_RESET_SYSTEM;
> + WDT_CTRL_1MHZ_CLK;
> +
> + np = pdev->dev.of_node;
> + if (np && !of_get_property(np, "no-system-reset", NULL))
> + wdt->ctrl |= WDT_CTRL_RESET_SYSTEM;
> +
Please use of_property_read_bool(). Also, checking np for NULL is unnecessary
since the called function does that. Note that the above code as written is
wrong - if np is NULL, WDT_CTRL_RESET_SYSTEM will no longer be set.
The new properties should probably start with "aspeed," but I'll leave that
up to DT maintainers to decide.
> + if (np && of_get_property(np, "external-signal", NULL))
> + wdt->ctrl |= WDT_CTRL_WDT_EXT;
> +
> + writel(wdt->ctrl, wdt->base + WDT_CTRL);
>
> if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) {
> aspeed_wdt_start(&wdt->wdd);
[toc] | [next] | [standalone]
| From | Christopher Bostic <cbostic@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-06-27 04:20 +0200 |
| Message-ID | <tWOgG-2mL-31@gated-at.bofh.it> |
| In reply to | #1674152 |
On 6/24/17 5:17 PM, Guenter Roeck wrote:
> On Tue, Jun 13, 2017 at 03:38:18PM -0500, Christopher Bostic wrote:
>> Reference the system device tree when configuring the watchdog.
>> Configure for external signal generation if optional attribute
>> 'external-signal' is present in device tree. Configure for
>> reset system after timeout if the optional attribute
>> 'no-system-reset' is not present. Disabling system reset may be
>> required in situations where one of the other watchdogs in the
>> systems is responsible for this.
>>
>> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
>> ---
>> drivers/watchdog/aspeed_wdt.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/watchdog/aspeed_wdt.c b/drivers/watchdog/aspeed_wdt.c
>> index 1c65258..9deb4be 100644
>> --- a/drivers/watchdog/aspeed_wdt.c
>> +++ b/drivers/watchdog/aspeed_wdt.c
>> @@ -140,6 +140,7 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>> {
>> struct aspeed_wdt *wdt;
>> struct resource *res;
>> + struct device_node *np;
>> int ret;
>>
>> wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>> @@ -170,8 +171,16 @@ static int aspeed_wdt_probe(struct platform_device *pdev)
>> * the SOC and not the full chip
>> */
>> wdt->ctrl = WDT_CTRL_RESET_MODE_SOC |
>> - WDT_CTRL_1MHZ_CLK |
>> - WDT_CTRL_RESET_SYSTEM;
>> + WDT_CTRL_1MHZ_CLK;
>> +
>> + np = pdev->dev.of_node;
>> + if (np && !of_get_property(np, "no-system-reset", NULL))
>> + wdt->ctrl |= WDT_CTRL_RESET_SYSTEM;
>> +
> Please use of_property_read_bool(). Also, checking np for NULL is unnecessary
> since the called function does that. Note that the above code as written is
> wrong - if np is NULL, WDT_CTRL_RESET_SYSTEM will no longer be set.
Will make the suggested changes.
Thanks
-Chris
> The new properties should probably start with "aspeed," but I'll leave that
> up to DT maintainers to decide.
>
>> + if (np && of_get_property(np, "external-signal", NULL))
>> + wdt->ctrl |= WDT_CTRL_WDT_EXT;
>> +
>> + writel(wdt->ctrl, wdt->base + WDT_CTRL);
>>
>> if (readl(wdt->base + WDT_CTRL) & WDT_CTRL_ENABLE) {
>> aspeed_wdt_start(&wdt->wdd);
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web