On 13.02.2025 10:13 AM, Stephan Gerhold wrote: > On Wed, Feb 12, 2025 at 10:01:59PM +0100, Konrad Dybcio wrote: >> On 12.02.2025 6:03 PM, Stephan Gerhold wrote: >>> num-channels and qcom,num-ees are required when there are no clocks >>> specified in the device tree, because we have no reliable way to read them >>> from the hardware registers if we cannot ensure the BAM hardware is up when >>> the device is being probed. >>> >>> This has often been forgotten when adding new SoC device trees, so make >>> this clear by describing this requirement in the schema. >>> >>> Signed-off-by: Stephan Gerhold <stephan.gerhold@xxxxxxxxxx> >>> --- >>> Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml >>> index 3ad0d9b1fbc5e4f83dd316d1ad79773c288748ba..5f7e7763615578717651014cfd52745ea2132115 100644 >>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml >>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml >>> @@ -90,8 +90,12 @@ required: >>> anyOf: >>> - required: >>> - qcom,powered-remotely >>> + - num-channels >>> + - qcom,num-ees >>> - required: >>> - qcom,controlled-remotely >>> + - num-channels >>> + - qcom,num-ees >> >> I think I'd rather see these deprecated and add the clock everywhere.. >> Do we know which one we need to add on newer platforms? Or maybe it's >> been transformed into an icc path? > > This isn't feasible, there are too many different setups. Also often the > BAM power management is tightly integrated into the consumer interface. > To give a short excerpt (I'm sure there are even more obscure uses): > > - BLSP BAM (UART, I2C, SPI on older SoCs): > 1. Enable GCC_BLSP_AHB_CLK > -> This is what the bam_dma driver supports currently. > > - Crypto BAM: Either > OR 1. Vote for single RPM clock > OR 1. Enable 3 separate clocks (CE, CE_AHB, CE_AXI) > OR 1. Vote dummy bandwidth for interconnect > > - BAM DMUX (WWAN on older SoCs): > 1. Start modem firmware > 2. Wait for BAM DMUX service to be up > 3. Vote for power up via the BAM-DMUX-specific SMEM state > 4. Hope the firmware agrees and brings up the BAM > > - SLIMbus BAM (audio on some SoCs): > 1. Start ADSP firmware > 2. Wait for QMI SLIMBUS service to be up via QRTR > 3. Vote for power up via SLIMbus-specific QMI messages > 4. Hope the firmware agrees and brings up the BAM > > Especially for the last two, we can't implement support for those > consumer-specific interfaces in the BAM driver. Implementing support for > the 3 variants of the Crypto BAM would be possible, but it's honestly > the least interesting use case of all these. It's not really clear why > we are bothing with the crypto engine on newer SoCs at all, see e.g. [1]. > > [1]: https://lore.kernel.org/linux-arm-msm/20250118080604.GA721573@sol.localdomain/ > >> Reading back things from this piece of HW only to add it to DT to avoid >> reading it later is a really messy solution. > > In retrospect, it could have been cleaner to avoid describing the BAM as > device node independent of the consumer. We wouldn't have this problem > if the BAM driver would only probe when the consumer is already ready. > > But I think specifying num-channels in the device tree is the cleanest > way out of this mess. I have a second patch series ready that drops > qcom,num-ees and validates the num-channels once it's safe reading from > the BAM registers. That way, you just need one boot test to ensure the > device tree description is really correct. Thanks for the detailed explanation! Do you think it could maybe make sense to expose a clock/power-domain from the modem/adsp rproc and feed it to the DMUX / SLIM instances when an appropriate ping arrives? This way we'd also defer probing the drivers until the device is actually accessible. Konrad
