On 01/06/2021 08:58, Nandor Han wrote:
Hi and thanks for your answers.
On 5/28/21 11:23 AM, Srinivas Kandagatla wrote:
On 05/05/2021 11:42, Nandor Han wrote:
In order to have a robust system we want to be able to identify and take
actions if a boot loop occurs. This is possible by using the bootcount
feature, which can be used to identify the number of times device has
booted since bootcount was last time reset. Bootcount feature (1)
requires a collaboration between bootloader and user-space, where
the bootloader will increase a counter and user-space reset it.
If the counter is not reset and a pre-established threshold is reached,
bootloader can react and take action.
This is the kernel side implementation, which can be used to
identify the number of times device has booted since bootcount was
last time reset.
If I understand this correctly, this driver is basically exposing a
nvmem cell via sysfs.
Firstly, This sounds like totally a generic functionality that needs
to go into nvmem core rather than individual drivers.
Do you see any reason for this not be in core?
I agree that exposing a NVMEM cell via sysfs does look as a generic
functionality. However, the bootcount feature contains also a magic
value that needs to be taken in consideration when extracting the
bootcount value. The size of the field storing the magic and value combo
is configurable as well. The driver will handle this values
transparentlry for the user and expose only the validated
bootcount value. In case we will only use a generic implementation for
exposing a NVMEM cell via sysfs the aformention functionality will have
to be handled by userspace and this will force the userspace to have
knolwdge about bootcount value format and magic since they will have
to implement it's own functionality about this. In the current solution
the user only have to reset the value to 0 and that's it, the driver
will take care of the rest.
Should this not live in userspace HAL, kernel would provide an abstract
interface. User space in this case which is programming the bootcount is
already aware of this, so am hoping that it would be able to encapsulate
the magic as well with in.
Instead of accessing sysfs directly, its always recommended to access it
via a some abstraction HAL programs, so as to not break the userspace
across kernel releases, more info at
./Documentation/admin-guide/sysfs-rules.rst
Other problem with having this in kernel is that we would endup with
endless number of drivers for each nvmem cell which is totally not
necessary.
Personally I do not want to endup in such a situation where people start
writing drivers for each cell.
Secondly, creating sysfs entries like this in probe will race with
userspace udev. udev might not notice this new entry in such cases.
Thanks for point this out. I will have a look how to fix this. I'll
appriciate any advice.
There is a good document from Greg KH,
http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
--srini
Thirdly, You would need to document this in Documentation/ABI/
I'll do that.
Finally I noticed that the changes to snvs_lpgpr.c have not been cced
to the original author.
Sorry, my mistake. I will add it in the next patch-set.
<snip>
