Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 20 Apr 2023 08:58:01 +0200
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Trilok Soni <quic_tsoni@quicinc.com>
Cc:     Souradeep Chowdhury <quic_schowdhu@quicinc.com>,
        Arnd Bergmann <arnd@arndb.de>, Andy Gross <agross@kernel.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Rob Herring <robh+dt@kernel.org>, Alex Elder <elder@ieee.org>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        Sibi Sankar <quic_sibis@quicinc.com>,
        Rajendra Nayak <quic_rjendra@quicinc.com>
Subject: Re: [PATCH V22 2/3] misc: dcc: Add driver support for Data Capture
 and Compare unit(DCC)
Message-ID: <ZEDieaIsf7ptqvHq@kroah.com>
References: <cover.1681829664.git.quic_schowdhu@quicinc.com>
 <e4f41fa61d9dd66f68bbd7650c6fbf96810c3569.1681829664.git.quic_schowdhu@quicinc.com>
 <2023041833-alienate-trash-f4da@gregkh>
 <f1456dd7-5dcf-d91a-459c-65efca4a3444@quicinc.com>
 <50844899-b047-42fd-807a-db7136e5e590@app.fastmail.com>
 <44834c75-4db7-ec8a-9367-c6b83fa96b22@quicinc.com>
 <9808142e-d6e6-71bc-f362-09b878cb3b87@quicinc.com>
 <2024b144-42cf-1044-258c-2dc6c6af0d88@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <2024b144-42cf-1044-258c-2dc6c6af0d88@quicinc.com>
Precedence: bulk

On Wed, Apr 19, 2023 at 09:10:14AM -0700, Trilok Soni wrote:
> On 4/19/2023 9:08 AM, Trilok Soni wrote:
> > On 4/19/2023 3:20 AM, Souradeep Chowdhury wrote:
> > > 
> > > 
> > > On 4/19/2023 1:00 PM, Arnd Bergmann wrote:
> > > > On Wed, Apr 19, 2023, at 09:00, Souradeep Chowdhury wrote:
> > > > > On 4/18/2023 9:15 PM, Greg Kroah-Hartman wrote:
> > > > > > 
> > > > > > > The following is the justification of using debugfs
> > > > > > > interface over the
> > > > > > > other alternatives like sysfs/ioctls
> > > > > > > 
> > > > > > > i) As can be seen from the debugfs attribute
> > > > > > > descriptions, some of the
> > > > > > > debugfs attribute files here contains multiple
> > > > > > > arguments which needs to
> > > > > > > be accepted from the user. This goes against the
> > > > > > > design style of sysfs.
> > > > > > > 
> > > > > > > ii) The user input patterns have been made simple
> > > > > > > and convenient in this
> > > > > > > case with the use of debugfs interface as user
> > > > > > > doesn't need to shuffle
> > > > > > > between different files to execute one instruction as was the case on
> > > > > > > using other alternatives.
> > > > > > 
> > > > > > Why do you have debugfs and also a misc device?? How are they related?
> > > > > > Why both?? Why not just one?? What userspace tools are going to use
> > > > > > either of these interfaces and where are they published
> > > > > > to show how this
> > > > > > all was tested?
> > > > > 
> > > > > DCC has two fundamental steps of usage:-
> > > > > 
> > > > > 1.Configuring the register addresses on the dcc_sram which is done by
> > > > > user through the debugfs interface. For example:-
> > > > > 
> > > > > echo R 0x10c004 > /sys/kernel/debug/dcc/../3/config
> > > > > 
> > > > > Here we are configuring the register addresses for list 3, the 'R'
> > > > > indicates a read operation, so this register value will be read
> > > > > in case of a software trigger or kernel panic/watchdog bite and
> > > > > dumped into the dcc_sram.
> > > > 
> > > > Can you describe why the register location needs to be
> > > > runtime configurable? I would have expected this type of setting
> > > > to be part of the devicetree, which already describes other
> > > > parts that interact with sram devices.
> > > 
> > > Register addresses are made runtime configurable to give the user the
> > > option of going for a software trigger. So the user can debug issues
> > > during run-time as well. These register locations are arbitrary
> > > and is configured by the user for debugging purposes and is not
> > > related to the DCC hardware itself.
> > 
> > Please note that we don't want to recompile the devicetree for new
> > settings since these registers can be set by team of engineers who are
> > debugging system level issues with various IPs across the SOCs. You
> > don't want to recompile the images while reproducing the system hangs/IP
> > watchdogs etc;
> 
> ...and also these registers list is not fixed, it will vary based on the
> problem you are seeing and debugging on the SOC across the IPs.

Then all of this should be documented in the driver, and in the
changelog please.

thanks,

greg k-h