Received-SPF: pass (google.com: domain of linux-kernel+bounces-23768-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <cd223912-ac76-4b5d-93c3-92b1f89b9820@quicinc.com>
Date: Thu, 11 Jan 2024 20:37:03 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] watchdog: qcom: fine tune the max timeout value
 calculation
To: Guenter Roeck <linux@roeck-us.net>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Wim Van
 Sebroeck <wim@linux-watchdog.org>
CC: <linux-arm-msm@vger.kernel.org>, <linux-watchdog@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20240111-wdt-v1-1-28c648b3b1f3@quicinc.com>
 <73eda412-18cf-4c1e-8764-8341c2254659@roeck-us.net>
Content-Language: en-US
From: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
In-Reply-To: <73eda412-18cf-4c1e-8764-8341c2254659@roeck-us.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


On 1/11/2024 8:21 PM, Guenter Roeck wrote:
> On 1/11/24 01:49, Kathiravan Thirumoorthy wrote:
>> To determine the max_timeout value, the below calculation is used.
>>
>>     max_timeout = 0x10000000 / clk_rate
>>
>> cat 
>> /sys/devices/platform/soc@0/b017000.watchdog/watchdog/watchdog0/max_timeout
>> 8388
>>
>> However, this is not valid for all the platforms. IPQ SoCs starting from
>> IPQ40xx and recent Snapdragron SoCs also has the bark and bite time field
>> length of 20bits, which can hold max up to 32 seconds if the clk_rate is
>> 32KHz.
>>
>> If the user tries to configure the timeout more than 32s, then the value
>> will be truncated and the actual value will not be reflected in the HW.
>>
>> To avoid this, lets add a variable called max_tick_count in the device 
>> data,
>> which defines max counter value of the WDT controller. Using this, 
>> max-timeout
>> will be calculated in runtime for various WDT contorllers.
>>
>> With this change, we get the proper max_timeout as below and restricts
>> the user from configuring the timeout higher than this.
>>
>> cat 
>> /sys/devices/platform/soc@0/b017000.watchdog/watchdog/watchdog0/max_timeout
>> 32
>>
>> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
> 
> [ ... ]
> 
>> @@ -268,11 +271,11 @@ static int qcom_wdt_probe(struct platform_device 
>> *pdev)
>>           wdt->wdd.bootstatus = WDIOF_CARDRESET;
>>       /*
>> -     * If 'timeout-sec' unspecified in devicetree, assume a 30 second
>> -     * default, unless the max timeout is less than 30 seconds, then use
>> +     * If 'timeout-sec' unspecified in devicetree, assume a 32 second
>> +     * default, unless the max timeout is less than 32 seconds, then use
>>        * the max instead.
>>        */
>> -    wdt->wdd.timeout = min(wdt->wdd.max_timeout, 30U);
>> +    wdt->wdd.timeout = min(wdt->wdd.max_timeout, 32U);
> 
> This is an unrelated change which would have to be discussed separately,
> in a separate patch. But I really don't see the point of it.


Ack, will revert this change in V2.


> 
> Guenter
>