Re: [PATCH V6 0/7] Add driver support for Data Capture and Compare Engine(DCC) for SM8150,SC7280,SC7180,SDM845

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/16/2021 9:18 PM, Thara Gopinath wrote:


On 8/10/21 1:54 PM, Souradeep Chowdhury wrote:
DCC(Data Capture and Compare) is a DMA engine designed for debugging purposes.In case of a system crash or manual software triggers by the user the DCC hardware stores the value at the register addresses which can be used for debugging purposes.The DCC driver provides the user with sysfs interface to configure the register addresses.The options that the DCC hardware provides include reading from registers,writing to registers,first reading and then writing to registers and looping
through the values of the same register.

In certain cases a register write needs to be executed for accessing the rest of the registers, also the user might want to record the changing values of a register with time for which he has the
option to use the loop feature.

Hello Souradeep,

First of all, I think this is very a useful feature to have. I have some generic design related queries/comments on driver and the interface exposed to the user space. Also, I do not understand the h/w well here, so feel free to correct me if I am wrong.

1. Linked list looks like a very internal feature to the h/w. It really is not an info that user should be aware of. I tried reading the code a bit. IUC, every time a s/w trigger is issued the configs in all the enabled linked lists are executed. The final ram dump that you get from /dev/dcc_sram is a dump of contents from all the enabled list? Is this understanding correct ? And we are talking of at-most 4 linked list? If yes, I think it might be better to have a folder per linked list with config, config_write etc. Also if possible it will be better to dump the results to a file in the specific folder instead of reading from /dev/dcc_sram. If no, there is no real need for user to know the linked list, right? Choosing of linked list can be done by kernel driver in this case with no input needed from user.

2. Now to the sysfs interface itself, I know lot of thought has gone into sysfs vs debugfs considerations. But, have you considered using netlink interface instead of sysfs. Netlink interface is used for asynchronous communication between kernel and user space. In case of DCC, the communication appears to be asynchronous, where in user asks the kernel to capture some info and kernel can indicate back to user when the info is captured. Also the entire mess surrounding echoing addr / value / offset repeatedly into a sysfs entry can be avoided using netlink interface.

Hello Thara,

Thanks for your review comments. Following are some points from my end


1) Each linked list represent a particular block of memory in DCC_SRAM which is preserved for that particular list. That is why offset calculation is done on the driver based on the linked list chosen by the user.

    This choice needs to be made by the user since the number for the linked list chosen is specific to the registers used to debug a particular component.  Also we are giving the user flexibility to configure multiple

    linked lists at one go so that even if we don't have a separate folder for it , the dumps are collected as a separate list of registers. Also there are certain curr_list values which may be supported by the dcc

    hardware but may not be accessible to the user and so the choice cannot be made arbitrarily from the driver.


2) From opensource, I can see that Netlink has been used in most of the cases where we need to notify stats to the user by taking the advantage of asynchronous communication. In this case, that requirement is not

    there since it is mostly one way communication from user to kernel. Also since this is used for debugging purposes perhaps sysfs adds more reliability than Netlink. In case of Netlink we have the additional

     overhead of dealing with socket calls. Let me know otherwise.


Thanks,

Souradeep








[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux