lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <de0dc054-0813-fab1-d7ac-2f1d7ba61fd8@linux.vnet.ibm.com>
Date:   Wed, 28 Jun 2017 09:29:50 -0500
From:   Christopher Bostic <cbostic@...ux.vnet.ibm.com>
To:     Guenter Roeck <linux@...ck-us.net>, wim@...ana.be,
        robh+dt@...nel.org, mark.rutland@....com, joel@....id.au,
        linux-watchdog@...r.kernel.org, devicetree@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] drivers/watchdog: ASPEED reference dev tree
 properties for config



On 6/28/17 6:31 AM, Guenter Roeck wrote:
> On 06/27/2017 02:17 PM, Christopher Bostic wrote:
>> Reference the system device tree when configuring the watchdog
>> engines.  Set external signal mode on timeout if specified.
>> Set system reset on timeout if specified.
>>
>> Signed-off-by: Christopher Bostic <cbostic@...ux.vnet.ibm.com>
>> ---
>> v2 - Change of_get_property() to of_property_read_bool()
>>     - Remove redundant check for NULL struct device_node pointer
>>     - Optional property names now start with prefix 'aspeed,'
>> ---
>>   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..71ce5f5 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 (of_property_read_bool(np, "aspeed,sys-reset"))
>> +        wdt->ctrl |= WDT_CTRL_RESET_SYSTEM;
>> +
>
> For backward compatibility, this should default to WDT_CTRL_RESET_SYSTEM
> if no optional property is provided.
>
I had the logic inverted for this property in a previous patch.  The 
property was 'no-system-reset' so that when not present the default was 
to set WDT_CTRL_RESET_SYSTEM.   As it is in this patch, the only way to 
indicate that no system reset is to be done is to not specify the 
property.  No system reset is desired under circumstances when another 
wdt engine is to be responsible for this.   Given the issue with 
backward compatibility that's not a solution.  Given this, would 
creating a property 'no-system-reset' be acceptable?

Thanks,
Chris

>> +    if (of_property_read_bool(np, "aspeed,external-signal"))
>> +        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);
>>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ