From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.9 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC80AC4708F for ; Tue, 1 Jun 2021 07:59:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9B89C6136E for ; Tue, 1 Jun 2021 07:59:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233240AbhFAIAp (ORCPT ); Tue, 1 Jun 2021 04:00:45 -0400 Received: from hel-mailgw-01.vaisala.com ([193.143.230.17]:52740 "EHLO hel-mailgw-01.vaisala.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233182AbhFAIAn (ORCPT ); Tue, 1 Jun 2021 04:00:43 -0400 Received: from HEL-SMTP.corp.vaisala.com (HEL-SMTP.corp.vaisala.com [172.24.1.225]) by hel-mailgw-01.vaisala.com (Postfix) with ESMTP id 93191601D6F2; Tue, 1 Jun 2021 10:58:55 +0300 (EEST) Received: from localhost.localdomain ([172.24.252.62]) by HEL-SMTP.corp.vaisala.com over TLS secured channel with Microsoft SMTPSVC(8.5.9600.16384); Tue, 1 Jun 2021 10:59:00 +0300 Subject: Re: [PATCH v4 2/4] nvmem: bootcount: add bootcount driver To: Srinivas Kandagatla , robh+dt@kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Cc: =?UTF-8?B?VmVzYSBKw6TDpHNrZWzDpGluZW4=?= , Tomas Melin References: <43e36704e9acbf89b3b29113554d3a79417d42db.1620211180.git.nandor.han@vaisala.com> From: Nandor Han Message-ID: Date: Tue, 1 Jun 2021 10:58:59 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-OriginalArrivalTime: 01 Jun 2021 07:59:00.0348 (UTC) FILETIME=[FB2AD3C0:01D756BB] Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > 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. > > 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. -- Regards, Nandor