On 12.02.2025 6:03 PM, Stephan Gerhold wrote: > When we don't have a clock specified in the device tree, we have no way to > ensure the BAM is on. This is often the case for remotely-controlled or > remotely-powered BAM instances. In this case, we need to read num-channels > from the DT to have all the necessary information to complete probing. > > However, at the moment invalid device trees without clock and without > num-channels still continue probing, because the error handling is missing > return statements. The driver will then later try to read the number of > channels from the registers. This is unsafe, because it relies on boot > firmware and lucky timing to succeed. Unfortunately, the lack of proper > error handling here has been abused for several Qualcomm SoCs upstream, > causing early boot crashes in several situations [1, 2]. > > Avoid these early crashes by erroring out when any of the required DT > properties are missing. Note that this will break some of the existing DTs > upstream (mainly BAM instances related to the crypto engine). However, > clearly these DTs have never been tested properly, since the error in the > kernel log was just ignored. It's safer to disable the crypto engine for > these broken DTBs. > > [1]: https://lore.kernel.org/r/CY01EKQVWE36.B9X5TDXAREPF@xxxxxxxxxxxxx/ > [2]: https://lore.kernel.org/r/20230626145959.646747-1-krzysztof.kozlowski@xxxxxxxxxx/ > > Cc: stable@xxxxxxxxxxxxxxx > Fixes: 48d163b1aa6e ("dmaengine: qcom: bam_dma: get num-channels and num-ees from dt") > Signed-off-by: Stephan Gerhold <stephan.gerhold@xxxxxxxxxx> > --- > drivers/dma/qcom/bam_dma.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c > index c14557efd577046adc74fa83fd45eb239977b5fa..a2f1f8902c7f88398a5412e8673e24b3c10bb86f 100644 > --- a/drivers/dma/qcom/bam_dma.c > +++ b/drivers/dma/qcom/bam_dma.c > @@ -1291,13 +1291,17 @@ static int bam_dma_probe(struct platform_device *pdev) > if (!bdev->bamclk) { > ret = of_property_read_u32(pdev->dev.of_node, "num-channels", > &bdev->num_channels); > - if (ret) > + if (ret) { > dev_err(bdev->dev, "num-channels unspecified in dt\n"); > + return ret; > + } > > ret = of_property_read_u32(pdev->dev.of_node, "qcom,num-ees", > &bdev->num_ees); > - if (ret) > + if (ret) { > dev_err(bdev->dev, "num-ees unspecified in dt\n"); > + return ret; > + } I like dev_err_probe, but this works too Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx> Konrad
