Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v6 0/3] Move recovery/coredump configuration to sysfs
To:     Rishabh Bhatnagar <rishabhb@codeaurora.org>,
        "linux-remoteproc@vger.kernel.org" <linux-remoteproc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>
CC:     "tsoni@codeaurora.org" <tsoni@codeaurora.org>,
        "psodagud@codeaurora.org" <psodagud@codeaurora.org>,
        "sidgup@codeaurora.org" <sidgup@codeaurora.org>
References: <1601331456-20432-1-git-send-email-rishabhb@codeaurora.org>
From:   Arnaud POULIQUEN <arnaud.pouliquen@st.com>
Message-ID: <8222f5fa-2acc-a765-a728-6aad9ed88068@st.com>
Date:   Tue, 29 Sep 2020 10:44:18 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <1601331456-20432-1-git-send-email-rishabhb@codeaurora.org>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 9/29/20 12:17 AM, Rishabh Bhatnagar wrote:
> From Android R onwards Google has restricted access to debugfs in user
> and user-debug builds. This restricts access to most of the features
> exposed through debugfs. 'Coredump' and 'Recovery' are critical
> interfaces that are required for remoteproc to work on Qualcomm Chipsets. 
> Coredump configuration needs to be set to "inline" in debug/test builds
> and "disabled" in production builds. Whereas recovery needs to be
> "disabled" for debugging purposes and "enabled" on production builds.
> This patch series removes the recovery/coredump entries from debugfs
> and moves them to sysfs. Also, this disables the coredump collection
> by default as this is a requirement for production devices.
> 
> Changelog:
> 
> v6 -> v5:
> - Disable coredump collection by default
> - Rename the "default" configuration to "enabled" to avoid confusion
> 
> v5 -> v4:
> - Fix the cover-letter of tha patch series.
> 
> v4 -> v3:
> - Remove the feature flag to expose recovery/coredump
> 
> v3 -> v2:
> - Remove the coredump/recovery entries from debugfs

Sorry i missed this and some associated discussion in V2...

I have also some concerns about the ABI breaks.
In ST and I suppose in several companies we have some 
test environments that use the debugfs to generate and/or get
the core dump.

Even if the stability of the debugfs is not guaranteed it would
be nice to keep both interface.

It seems that it is possible to create symbolic link in the debugfs
thanks to the "debugfs_create_symlink" function.
This seems allowing to keep files in both place without duplicating the code.
To be honest i have never used this function so I'm not 100% sure that this
would do the job...
But if you think that this could be a good compromise, i can test it.

Regards,
Arnaud

> - Expose recovery/coredump from sysfs under a feature flag
> 
> v1 -> v2:
> - Correct the contact name in the sysfs documentation.
> - Remove the redundant write documentation for coredump/recovery sysfs
> - Add a feature flag to make this interface switch configurable.
> 
> Rishabh Bhatnagar (3):
>   remoteproc: Move coredump configuration to sysfs
>   remoteproc: Move recovery configuration to sysfs
>   remoteproc: Change default dump configuration to "disabled"
> 
>  Documentation/ABI/testing/sysfs-class-remoteproc |  46 +++++++
>  drivers/remoteproc/remoteproc_coredump.c         |   6 +-
>  drivers/remoteproc/remoteproc_debugfs.c          | 168 -----------------------
>  drivers/remoteproc/remoteproc_sysfs.c            | 120 ++++++++++++++++
>  include/linux/remoteproc.h                       |   8 +-
>  5 files changed, 173 insertions(+), 175 deletions(-)
>