On 9/29/20 4:33 PM, Bjorn Andersson wrote: > On Tue 29 Sep 03:44 CDT 2020, Arnaud POULIQUEN wrote: > >> >> >> 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. > > Debugfs is not an ABI... > >> 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. >> > > I do however acknowledge the inconvenience you're facing... > >> Even if the stability of the debugfs is not guaranteed it would >> be nice to keep both interface. >> > > ...and I wouldn't mind keeping the debugfs interface around, at least > for some time to allow people to transition their tools/muscle memory. > >> 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. >> > > The duplicated code is rather simple, so I don't mind the duplication - > for now. > > > So, how about we add the sysfs pieces of Rishabh's patches, leave out > the debugfs and then in a while (e.g. one LTS) we remove the debugfs > code? This smooth transition seems to me a very good compromise. Thanks, Arnaud > > Regards, > Bjorn > >> 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(-) >>>
