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

[Sanil, Shruthi]

Regards,
Shruthi

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> Sent: Wednesday, March 2, 2022 9:57 PM
> To: Sanil, Shruthi <shruthi.sanil@xxxxxxxxx>; tglx@xxxxxxxxxxxxx;
> robh+dt@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx
> Cc: andriy.shevchenko@xxxxxxxxxxxxxxx; mgross@xxxxxxxxxxxxxxx; Thokala,
> Srikanth <srikanth.thokala@xxxxxxxxx>; Raja Subramanian, Lakshmi Bai
> <lakshmi.bai.raja.subramanian@xxxxxxxxx>; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@xxxxxxxxx>
> Subject: Re: [PATCH v8 2/2] clocksource: Add Intel Keem Bay timer support
> 
> On 02/03/2022 17:07, Sanil, Shruthi wrote:
> >> -----Original Message----- From: Daniel Lezcano
> >> <daniel.lezcano@xxxxxxxxxx> Sent: Wednesday, March 2, 2022 3:54 PM
> >> To: Sanil, Shruthi <shruthi.sanil@xxxxxxxxx>; tglx@xxxxxxxxxxxxx;
> >> robh+dt@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> >> devicetree@xxxxxxxxxxxxxxx Cc: andriy.shevchenko@xxxxxxxxxxxxxxx;
> >> mgross@xxxxxxxxxxxxxxx; Thokala, Srikanth
> >> <srikanth.thokala@xxxxxxxxx>; Raja Subramanian, Lakshmi Bai
> >> <lakshmi.bai.raja.subramanian@xxxxxxxxx>; Sangannavar,
> >> Mallikarjunappa <mallikarjunappa.sangannavar@xxxxxxxxx> Subject:
> >> Re: [PATCH v8 2/2] clocksource: Add Intel Keem Bay timer support
> >>
> >> On 02/03/2022 11:12, 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.

> 
> >> -- <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
> 
> 
> --
> <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