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,
Stephan
