Hi Andrew,
On 08/01/25 20:33, Andrew Davis wrote:
On 1/3/25 4:12 AM, Beleswar Padhi wrote:
This series refactors a lot of functions & callbacks from
ti_k3_dsp_remoteproc.c
and ti_k3_m4_remoteproc.c drivers. This is the third and final series
as part of
the refactoring of K3 remoteproc drivers. The patches for internal
refactoring
and bug fixes of TI K3 R5 remoteproc driver has been already
posted[0][1]. Since
the R5 driver has worked out separate data structures and reset logic
than the
DSP/M4 drivers, I have excluded R5 from this refactoring.
Diffstat looks great, 765 (+), 1164 (-), good to see all that
duplicated code
factored away. But R5 is the largest of the 3 drivers and really needs
it the
most.
Looking at the data structure in R5 preventing this I see what should be
the normal "struct k3_rproc" structure is really split into two,
"struct k3_r5_rproc" and "struct k3_r5_core". The first containing a
single
instance of the latter. There is no reason for this split I can see, just
combine the two structs.
Next, there are some members of the struct that we don't need, such as
atcm_enable and the others that are only used in probe (or functions
called as part of probe). We only use these as a way to collect this
info in one function, and use in a later one. Instead you could either
fetch this info at the time of use. Or move these members into the
cluster level "struct k3_r5_cluster".
Speaking of "struct k3_r5_cluster", it is silly for cluster to keep a
list
of cores. There are two, and will only ever be two. No clue why a list
was
chosen as the data structure to hold two pointers, switch this two an
array
of size two, or even just two pointers. This also cleans up a bunch of
the
weird "list_for_each" logic and loops that have to then check if they
have
found with core0 or core1. Instead, just directly access core0 or core1.
That gets rid of member "struct list_head" from the combined struct,
and would you look at that, the struct now matches DSP/M4 :)
I'd suggest doing the above fixups to R5 first, then you can do
this series here after that and include R5.
Thanks a lot for suggesting this detailed plan! I agree with your
assessment, and I will post a series addressing this.
Thanks,
Beleswar
Thanks,
Andrew
NOTE:
This series is _dependent_ upon the [PATCH 2/3] of below series:
https://lore.kernel.org/all/20241224091457.1050233-3-b-padhi@xxxxxx/
Testing Done:
1. Tested boot of C66x DSPs, C71x DSPs across Jacinto J7* devices in
Remoteproc
mode and IPC-Only mode.
2. Tested boot of M4F core _only_ in _AM62xx SK_ board in Remoteproc
mode and
IPC-Only mode.
3. Tested Core stop and detach operations from sysfs for C66x DSPs,
C71x DSPs
and M4F.
4. Tested device removal paths by executing 'modprobe -r
ti_k3_dsp_remoteproc'
and 'modprobe -r ti_k3_m4_remoteproc'.
5. Tested usecases where firmware not available at device probe time,
but later
in sysfs, able to load firmware into a remotecore and start it.
[C66x, C71x, M4]
6. Tested that each patch in this series generates no new
warnings/errors.
v8: Changelog:
1. Broken down refactoring into patches, each patch dealing with one
function
for ease in review. [Andrew]
Links to older versions:
v7: https://lore.kernel.org/all/20240202175538.1705-1-hnagalla@xxxxxx/
v6: https://lore.kernel.org/all/20230913111644.29889-1-hnagalla@xxxxxx/
v5: https://lore.kernel.org/all/20230808044529.25925-1-hnagalla@xxxxxx/
v4: https://lore.kernel.org/all/20230801141117.2559-1-hnagalla@xxxxxx/
v3:
https://lore.kernel.org/all/20230302171450.1598576-1-martyn.welch@xxxxxxxxxxxxx/
v2:
https://lore.kernel.org/all/20230301111323.1532479-4-martyn.welch@xxxxxxxxxxxxx/
v1: https://lore.kernel.org/all/20220110040650.18186-1-hnagalla@xxxxxx/
Thanks,
Beleswar
[0]:
https://lore.kernel.org/all/20241219110545.1898883-1-b-padhi@xxxxxx/
[1]:
https://lore.kernel.org/all/20241224091457.1050233-1-b-padhi@xxxxxx/
Beleswar Padhi (20):
remoteproc: k3-m4: Prevent Mailbox level IPC with detached core
remoteproc: k3: Refactor shared data structures
remoteproc: k3: Refactor mailbox rx_callback functions into common
driver
remoteproc: k3: Refactor .kick rproc ops into common driver
remoteproc: k3-m4: Use k3_rproc_mem_data structure for memory info
remoteproc: k3: Refactor rproc_reset() implementation into common
driver
remoteproc: k3: Refactor rproc_release() implementation into common
driver
remoteproc: k3: Refactor rproc_request_mbox() implementations into
common driver
remoteproc: k3: Refactor .prepare rproc ops into common driver
remoteproc: k3: Refactor .unprepare rproc ops into common driver
remoteproc: k3: Refactor .start rproc ops into common driver
remoteproc: k3: Refactor .stop rproc ops into common driver
remoteproc: k3: Refactor .attach rproc ops into common driver
remoteproc: k3: Refactor .detach rproc ops into common driver
remoteproc: k3: Refactor .get_loaded_rsc_table ops into common driver
remoteproc: k3: Refactor .da_to_va rproc ops into common driver
remoteproc: k3: Refactor of_get_memories() functions into common
driver
remoteproc: k3: Refactor mem_release() functions into common driver
remoteproc: k3: Refactor reserved_mem_init() functions into common
driver
remoteproc: k3: Refactor release_tsp() functions into common driver
drivers/remoteproc/Makefile | 4 +-
drivers/remoteproc/ti_k3_common.c | 586 ++++++++++++++++++++
drivers/remoteproc/ti_k3_common.h | 113 ++++
drivers/remoteproc/ti_k3_dsp_remoteproc.c | 643 +---------------------
drivers/remoteproc/ti_k3_m4_remoteproc.c | 583 ++------------------
5 files changed, 765 insertions(+), 1164 deletions(-)
create mode 100644 drivers/remoteproc/ti_k3_common.c
create mode 100644 drivers/remoteproc/ti_k3_common.h
