adsp-6.18.31-y: Rework remoteproc, kconfig structure, minor 6.12->6.18 fixes#3332
adsp-6.18.31-y: Rework remoteproc, kconfig structure, minor 6.12->6.18 fixes#3332gastmaier wants to merge 33 commits into
Conversation
|
Changed the base so I don't break your testing when I merge into |
gastmaier
force-pushed
the
public/adsp-6.18.0/philip/rebase-adsp-6.12.89
branch
4 times, most recently
from
f3c8fbe to
6e14b0d
Compare
|
the non-compiled are well justified now: only one can be compiled in a defconfig, if both are touched, only one will be enabled. Same as above, only one (sc59x, sc58x, sc57x) can be enabled for a defconfig, if touch all, 2 will fail. the first is a limitation of the driver structure ("2 drivers one compatible") and the second a limitation of the infer script+defconfig |
gastmaier
force-pushed
the
public/adsp-6.18.0/philip/rebase-adsp-6.12.89
branch
2 times, most recently
from
df7c5da to
9e00a47
Compare
The ADI System Protection Unit (SPU) is a bus-level access controller gating secure/non-secure master access to peripherals. On SC59x (AArch64) it is inaccessible from EL1 and requires an SMC call to program. Add a minimal stub header with adi_spu_set_securep() as a no-op placeholder until a proper drivers/bus/adi_spu.c driver exists. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Despite being a stub, add the placeholder at the correct location for future implementation. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Despite being a stub, add the placeholder at the correct location for future implementation. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
To allow to compile-test in multiple architecture, and also explicit mark the architectural debt, isolate architecture specific calls and defines. First step to follow the patterns of drivers/remoteproc/stm32_rproc.c Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Use kernel abstractions instead of requiring arch-specific code in higher level drivers. drivers/soc/adi/mach-sc5xx/rcu.c Added a reset_controller_dev embedded in struct adi_rcu. 2-cell reset specifier [coreid, function] (function 0=CRST pulse, 1=start/stop). Reads adi,core-stop-irqs DT property so the stop-core IRQ is no longer passed from the remoteproc driver. drivers/soc/adi/mach-sc5xx/icc.c Added a mbox_controller embedded in struct adi_tru. Uses a 8-slot dynamic channel array — xlate maps TRU master IDs to slots on demand. send_data fires adi_tru_trigger(). txdone_poll = true (fire-and-forget semantics). drivers/remoteproc/adi_remoteproc.c - Replaced adi_rcu_*/adi_tru_* calls with reset_control_* and mbox_send_message() - SVECT writes now use syscon_regmap_lookup_by_phandle() + regmap_write() - Local ADI_RCU_REG_SVECT1/2 offsets defined with #ifdef CONFIG_ARCH_SC58X - Probe simplified: remove get_adi_rcu_from_node/get_adi_tru_from_node/put_* calls Signed-off-by: Jorge Marques <jorge.marques@analog.com>
At f051481 ("remoteproc: adi_remoteproc: replace arch-specific") Kernel abstractions instead of arch-specific code in is used in remoteproc. Adjust the devicetrees to be compatble with the new required handles: - RCU nodes: added "syscon" compatible, #reset-cells = <2>, adi,core-stop-irqs - TRU nodes: added #mbox-cells = <1> - Remoteproc nodes: removed core-irq/adi,tru/adi,tru-master-id; added resets/reset-names/mboxes/mbox-names Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Add ADI_MACH_SC5XX and ADI_MACH_SC59X symbols to group the SC5XX family depending on the architecture. Auto enable if COMPILE_TEST is set. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
gastmaier
force-pushed
the
public/adsp-6.18.0/philip/rebase-adsp-6.12.89
branch
from
9e00a47 to
493fddd
Compare
Drop the forward declaration of set_spu_securep_msec() from include/linux/soc/adi/cpu.h. The function stubs that previously provided this symbol have been removed. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Split the monolithic depends line into two separate instances and add COMPILE_TEST. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Introduce a dedicated ADI_ADSP_TIMER Kconfig symbol that depends on
the appropriate ADSP SoC configs or COMPILE_TEST, and selects TIMER_OF
and COUNTER. Replace the arch-conditional Makefile rules with the new
symbol, which is the proper way to gate driver compilation.
Add MODULE_IMPORT_NS("COUNTER") to resolve undefined references to
devm_counter_alloc(), counter_priv(), and devm_counter_add() that
appear when the counter subsystem symbols are not exported to the
driver's namespace.
Fixes: 07ea154 ("clocksource: Add support for ADSP-SC5xx generic timer")
Signed-off-by: Jorge Marques <jorge.marques@analog.com>
The gpio_chip.set callback now expects an int return value, but adsp_gpio_set_value() still returns void. This causes a build failure when assigning the callback to struct gpio_chip.set. Update adsp_gpio_set_value() to return int and return 0 after programming the GPIO state. No functional change is intended. Fixes: 40485a4 ("gpio: Add GPIO port driver for ADSP-SC5xxx SoCs") Signed-off-by: Ozan Durgut <ozan.durgut@analog.com>
Allow the driver to be built on non-ADSP platforms by appending COMPILE_TEST to the architecture dependency in Kconfig. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Allow the driver to be built on non-ADSP platforms by appending COMPILE_TEST to the architecture dependency in Kconfig. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Allow the driver to be built on non-ADSP platforms by appending COMPILE_TEST to the architecture dependencies in Kconfig. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Remove the erroneous const qualifier from adsp_sru_ctrl_probe()'s return type. Qualifying a plain integer return type with const is meaningless and generates a 'type qualifiers ignored on function return type' compiler warning. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Drop the inline set_spu_securep_msec() placeholder that was left as a TODO in drivers/soc/adi/mach-sc59x/core.c. The Security Processing Unit is inaccessible from EL1, so no stub is needed in kernel space. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Drop spu.c from the Makefile and delete the file. It contained a do-nothing set_spu_securep_msec() stub kept for build compatibility. Since all in-tree callers have been removed the stub is no longer needed, and carrying a dead source file would be misleading. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Drop config symbols that no longer exist in the current kernel (CONFIG_LOCALVERSION, CONFIG_EMBEDDED, CONFIG_SLAB, CONFIG_SERIAL_ADI_UART4, CONFIG_TTY_PRINTK, CONFIG_HW_RANDOM_ADI, CONFIG_GPIO_SYSFS, CONFIG_ADI_ADSP_IRQ, CONFIG_AUTOFS4_FS, CONFIG_CRYPTO_TEST, CONFIG_CRYPTO_DEV_ADI_CRC, CONFIG_CRC_CCITT, CONFIG_DEBUG_INFO, CONFIG_DEBUG_FS, CONFIG_EARLY_PRINTK). Running make savedefconfig would silently discard these anyway; removing them prevents spurious kconfig warnings during builds. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Drop config symbols that no longer exist in the current kernel (CONFIG_LOCALVERSION, CONFIG_USELIB, CONFIG_EVENTFD, CONFIG_AIO, CONFIG_EMBEDDED, CONFIG_SLAB, CONFIG_SERIAL_ADI_UART4, CONFIG_TTY_PRINTK, CONFIG_HW_RANDOM_ADI, CONFIG_MMC_DW_PLTFM, CONFIG_RTC_DRV_ADI2, CONFIG_ADI_ADSP_IRQ, CONFIG_AUTOFS4_FS, CONFIG_CRYPTO_TEST, CONFIG_CRYPTO_DEV_ADI_CRC, CONFIG_CRC_CCITT, CONFIG_DEBUG_INFO, CONFIG_DEBUG_PAGEALLOC, CONFIG_DEBUG_LL, CONFIG_EARLY_PRINTK). Removing them prevents spurious kconfig warnings during builds. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Drop stale config symbols that no longer exist in the current kernel and add CONFIG_ADI_REMOTEPROC=y which is required for SHARC core support on the SC594-SOM. Removing the obsolete symbols prevents spurious kconfig warnings during builds. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Drop config symbols that no longer exist in the current kernel. Removing them prevents spurious kconfig warnings during builds. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Drop stale config symbols that have been removed or renamed upstream and add options required for the current kernel: CONFIG_SCSI, CONFIG_BLK_DEV_SD, CONFIG_USB_STORAGE, CONFIG_RPMSG_CTRL, and ARM64 crypto accelerators. Removing the obsolete symbols prevents spurious kconfig warnings during builds. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Drop stale config symbols that have been removed or renamed upstream and add options required for the current kernel. Removing the obsolete symbols prevents spurious kconfig warnings during builds. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Replace the coarse ARCH_SC5XX symbol used in the 'default' and 'depends on' lines of the ADI SoC Kconfig entries with the explicit per-family symbols ARCH_SC59X_64, ARCH_SC59X, ARCH_SC58X, and ARCH_SC57X. This ensures the entries are only auto-selected on the correct SoC families and that COMPILE_TEST coverage is properly scoped. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
gastmaier
force-pushed
the
public/adsp-6.18.0/philip/rebase-adsp-6.12.89
branch
from
d8d853b to
437133f
Compare
SND_SC5XX_SPORT_SHARC and SND_SC5XX_SHARC_ALSA_CARD both link against icap_linux_kernel_rpmsg.o, which uses sk_buff queues for transport buffering. Since Linux 6.9, kfree_skb() calls sk_skb_reason_drop(), so the sk_buff symbols are only available when CONFIG_NET is enabled. Add 'depends on NET' to SND_SC5XX_SPORT_SHARC and append NET to SND_SC5XX_SHARC_ALSA_CARD. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
gastmaier
changed the title
artursartamonovsadi
left a comment
artursartamonovsadi
left a comment
There was a problem hiding this comment.
Went trough remoteproc related changes. Tested on SC598-SOM RevE run regular echo example tests https://analogdevicesinc.github.io/lnxdsp-adi-meta/development/RPMsg.html#testing seems works ok. Did also start/stop cores all behaved as expected.
artursartamonovsadi
requested review from
CalebEthridgeADI,
qasim-ijaz and
vasbimpikasadi
|
|
||
| #ifndef __LINUX_SOC_ADI_SPU_H | ||
| #define __LINUX_SOC_ADI_SPU_H | ||
|
|
There was a problem hiding this comment.
#include <linux/types.h>
| #include <linux/delay.h> | ||
| #include <linux/dma-mapping.h> | ||
|
|
||
| #include <linux/soc/adi/cpu.h> |
There was a problem hiding this comment.
Left over from rebasing? Should be removed?
There was a problem hiding this comment.
And then the commit message title doesn't match the files changed?
| #define sm_atomic_write(i, v) iowrite16(v, i) | ||
| #define arm_core_id() 0 | ||
|
|
||
| #ifdef CONFIG_ARM |
There was a problem hiding this comment.
Looks like these macros aren't used anymore...
There was a problem hiding this comment.
While the invalidate_dcache_range and flush_dcache_range macros exist in the headers, they are effectively dead code in the current project. The drivers follow kernel best practices by delegating cache coherency to the memory subsystem and using standard synchronization primitives rather than architecture-specific assembly macros.
| of_property_read_u32_index(np, "adi,core-stop-irqs", 0, | ||
| &adi_rcu->core_stop_irq[1]); | ||
| of_property_read_u32_index(np, "adi,core-stop-irqs", 1, | ||
| &adi_rcu->core_stop_irq[2]); |
There was a problem hiding this comment.
The return values from these of_property_read_u32_index() calls are not
checked. The commit migrates the core stop IRQ information from the
remoteproc DT node (where it was "core-irq") to the RCU DT node (where it
is now "adi,core-stop-irqs").If the "adi,core-stop-irqs" property is missing from the device tree (e.g. if
the DT is not updated in sync with the driver), core_stop_irq[1] and
core_stop_irq[2] will remain zero.Later, when adi_rcu_rst_assert() calls adi_rcu_stop_core() with this
zero value:drivers/soc/adi/mach-sc5xx/rcu.c:adi_rcu_rst_assert() {
...
if (ADI_RCU_RST_FUNC(id) == ADI_RCU_RST_START)
return adi_rcu_stop_core(rcu, coreid, rcu->core_stop_irq[coreid]);
...
}drivers/soc/adi/mach-sc5xx/rcu.c:adi_rcu_stop_core() {
...
sec_set_ssi_coreid(rcu->sec, coreirq, coreid);
sec_enable_ssi(rcu->sec, coreirq, false, true);
sec_enable_sci(rcu->sec, coreid);
sec_raise_irq(rcu->sec, coreirq);
...
}It will raise IRQ 0, which is incorrect. Should the property read be validated
to ensure the device tree has been updated with the required information?
A little more context for the above change:
|
| #else | ||
| #define ADI_RCU_REG_SVECT1 0x30 | ||
| #define ADI_RCU_REG_SVECT2 0x34 | ||
| #endif |
There was a problem hiding this comment.
- Local ADI_RCU_REG_SVECT1/2 offsets defined with #ifdef CONFIG_ARCH_SC58X
Move the definitions from rcu.h because remoteproc.c is the only user of those definitions, but smells funny to have RCU definitions in the remoteproc driver, rather than something like devicetree.
For SVECT register offsets (hardcoded in driver):
#ifdef CONFIG_ARCH_SC58X
#define ADI_RCU_REG_SVECT1 0x24 ← Hardware-specific values in code
#define ADI_RCU_REG_SVECT2 0x28
#else
#define ADI_RCU_REG_SVECT1 0x30
#define ADI_RCU_REG_SVECT2 0x34
#endif
Remoteproc driver still has chip-specific knowledge!Why This Is Wrong
The whole point of using syscon/regmap and standard abstractions was to decouple the remoteproc driver from platform-specific details. But now the remoteproc driver still needs to know:
- Which chip family it's running on (CONFIG_ARCH_SC58X)
- What the RCU register layout is (SVECT offsets)
The suggested solutions:
Option 1: RCU driver provides SVECT API
// In RCU driver
int adi_rcu_set_svect(struct device_node *rcu_node, int coreid, u32 vector);// Remoteproc calls via syscon phandle lookup
The RCU driver knows its own register layout based on compatible string.Option 2: DT properties (like stop-irqs)
rcu@... {
compatible = "adi,sc58x-rcu"; // ← Implies register layout
reg = <0x... 0x1000>;
adi,core-stop-irqs = <12 13>;
// Register offsets determined by compatible string
};Option 3: Reset controller for vector setting
Extend the reset controller to handle SVECT writes, perhaps as a separate "set boot vector" operation.
There was a problem hiding this comment.
arch/arm/boot/dts/ti/omap/omap5.dtsi
dsp: dsp {
compatible = "ti,omap5-dsp";
ti,bootreg = <&scm_conf 0x304 0>;
iommus = <&mmu_dsp>;
resets = <&prm_dsp 0>;
clocks = <&dsp_clkctrl OMAP5_MMU_DSP_CLKCTRL 0>;
firmware-name = "omap5-dsp-fw.xe64T";
mboxes = <&mailbox &mbox_dsp>;
status = "disabled";
};
| adi_rcu->rcdev.of_node = dev->of_node; | ||
| adi_rcu->rcdev.of_reset_n_cells = 2; | ||
| adi_rcu->rcdev.of_xlate = adi_rcu_rst_xlate; | ||
| adi_rcu->rcdev.nr_resets = 64; |
There was a problem hiding this comment.
Is nr_resets needed here? Looks like reset core only uses it when neither fwnode_xlate nor of_xlate is defined
| put_adi_rcu(rproc_data->rcu); | ||
| rproc_del(rproc_data->rproc); | ||
| rproc_free(rproc_data->rproc); | ||
| mbox_free_channel(rproc_data->kick_chan); |
There was a problem hiding this comment.
might be worth cleaning up the workqueue in remove() as probe does via destroy_workqueue()
5 participants
PR Description
reduced to 33 commits.
the relevant are:
46d7b87..e91ce71
especially:
bb79247
since reworks remoteproc, including devicetree nodes! requires HW testing!
SPU rework is just to make arch-agnostic, and since it is a stub, it is harmless.
Why the ci still fails?
Note
This series now succeeds because the original series was already merged, touching now less files, the comments below are considering the whole o.g. series.
Cannot be compiled together, they define the same methods, a future rework should make it not to exclusory, instead, devicetree 'selectable'.
Only one can be compiled, I hardcoded
ARCH_SC59Xat https://github.com/analogdevicesinc/linux/blob/ci/ci/symbols_depend.py#L17 forsc59x-ezkit.c, sosc58x-ezkit.candsc57x-ezkit.cwon't compile. current limitation.Maybe revisit and hardcode pipe
sc58x-ezkit.candsc57x-ezkit.cto suppress?PR Type
PR Checklist