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