linux-kernel - RE: [PATCH v8 2/2] clocksource: Add Intel Keem Bay timer support

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]

Date:   Thu, 3 Mar 2022 10:23:43 +0000
From:   "Sanil, Shruthi" <shruthi.sanil@...el.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
CC:     "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "mgross@...ux.intel.com" <mgross@...ux.intel.com>,
        "Thokala, Srikanth" <srikanth.thokala@...el.com>,
        "Raja Subramanian, Lakshmi Bai" 
        <lakshmi.bai.raja.subramanian@...el.com>,
        "Sangannavar, Mallikarjunappa" 
        <mallikarjunappa.sangannavar@...el.com>
Subject: RE: [PATCH v8 2/2] clocksource: Add Intel Keem Bay timer support

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@...aro.org>
> Sent: Thursday, March 3, 2022 3:48 PM
> To: Sanil, Shruthi <shruthi.sanil@...el.com>; tglx@...utronix.de;
> robh+dt@...nel.org; linux-kernel@...r.kernel.org;
> devicetree@...r.kernel.org
> Cc: andriy.shevchenko@...ux.intel.com; mgross@...ux.intel.com; Thokala,
> Srikanth <srikanth.thokala@...el.com>; Raja Subramanian, Lakshmi Bai
> <lakshmi.bai.raja.subramanian@...el.com>; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@...el.com>
> Subject: Re: [PATCH v8 2/2] clocksource: Add Intel Keem Bay timer support
> 
> On 03/03/2022 07:18, Sanil, Shruthi wrote:
> 
> [ ... ]
> 
> >>>>>>> +	if (!(val & TIM_CONFIG_PRESCALER_ENABLE)) { +
> >>>>>>> pr_err("%pOF: Prescaler is not enabled\n", np); +		ret =
> >>>>>>> -ENODEV; +	}
> >>>>>>
> >>>>>> Why bail out instead of enabling the prescalar ?
> >>>>>
> >>>>> Because it is a secure register and it would be updated by the
> >>>>> bootloader.
> >>>> Should it be considered as a firmware bug ?
> >>>
> >>> No. This is a common driver across products in the series and
> >>> enablement of this bit depends on the project requirements. Hence to
> >>> be sure from driver, we added this check to avoid initialization of
> >>> the driver in the case where it cannot be functional.
> >>
> >> I'm not sure to get the meaning of 'project requirements' but (for my
> >> understanding) why not describe the timer in the DT for such
> >> projects?
> >>
> >
> > OK, I understand your point now. We can control the driver
> > initialization from device tree binding rather than add a check in the
> > driver. But isn't it good to have a check, if enabling of the bit is
> > missed out in the FW? This can help in debugging.
> 
> So if the description is in the DT but the prescaler bit is not enabled then the
> firmware is buggy, IIUC. Yeah, this check would help, may be add more
Yes, right. It would mean the FW is buggy.

> context in the error message, eg. "Firmware has not enabled the prescaler
> bit" or something like that
> 
> Thanks for the clarification

Yes, Sure. I'll update the comment accordingly.
Thank you 😊

> 
>    -- D.
> 
> 
> 
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-
> blog/> Blog