Re: [PATCH v2 1/4] dt-bindings: dma: qcom: gpi: add fallback

[Richard Acayan <mailingradian@gmail.com>]

> On Fri, Sep 23, 2022 at 06:20:28PM -0400, Richard Acayan wrote:
> > > On 23/09/2022 23:09, Richard Acayan wrote:
> > > > The drivers are transitioning from matching against lists of specific
> > > > compatible strings to matching against smaller lists of more generic
> > > > compatible strings. Add a fallback compatible string in the schema to
> > > > support this change.
> > > 
> > > Thanks for the patch. I wished we discussed it a bit more. :)
> > 
> > Ah, sorry for not replying to your original suggestion. I didn't see the
> > opportunity for discussion as this new series wasn't that hard to come up
> > with.
> > 
> > > qcom,gpi-dma does not look like specific enough to be correct fallback,
> > > at least not for all of the devices. I propose either a IP block version
> > > (which is tricky without access to documentation) or just one of the SoC
> > > IP blocks.
> > 
> > Solution 1:
> > 
> > Yes, I could use something like qcom,sdm845-gpi-dma. It would be weird to
> > see the compatible strings for that, though:
> 
> Why is it weird? That's how 'compatible' works. You are saying a new 
> implementation is compatible with an older implementation.

Oh, I didn't think of it like that. I found it weird how I need to specify both
sm8150 and sdm845 in the same dts, but now it makes sense. I guess I thought
fallback needed to be generic, and didn't think it could just specify an older
version of the hardware.

> 
> 
> >     compatible = "qcom,sdm670-gpi-dma", "qcom,sdm845-gpi-dma";
> > 
> 
> >     // This would need to be valid in dt schema, suggesting solution 2
> >     compatible = "qcom,sdm845-gpi-dma";
> >     // This just doesn't make sense
> >     compatible = "qcom,sdm845-gpi-dma", "qcom,sdm845-gpi-dma";
> 
> Is your question how to get the first one to work, but not the second 
> one? You need 'oneOf' with at least an entry for each case with 
> different number of compatible strings (1 and 2 entries). There are 
> lot's of examples in the tree.

No, I thought it would be tempting to use the first one for other device trees,
but you maintainers know not to allow that so it doesn't matter as much.

> 
> > 
> >     compatible = "qcom,sm8150-gpi-dma", "qcom,sdm845-gpi-dma";
> > 
> >     compatible = "qcom,sm8250-gpi-dma", "qcom,sdm845-gpi-dma";
> > 
> > Solution 2:
> > 
> > I could stray from the "soc-specific compat", "fallback compat" and just
> > have "qcom,sdm845-gpi-dma" for every SoC.
> 
> No.
> 
> > Solution 3:
> > 
> > I found the original mailing list archive for this driver:
> > 
> > https://lore.kernel.org/linux-arm-msm/20200824084712.2526079-1-vkoul@kernel.org/
> > https://lore.kernel.org/linux-arm-msm/20200918062955.2095156-1-vkoul@kernel.org/
> > 
> > It seems like the author originally handled the ee_offset as a dt property
> > and removed it. It was removed because it was a Qualcomm-specific property.
> > One option would be to bring this back against the author's wishes (or ask
> > the author about it, since they are a recipient).
> 
> No.

Ah, simple rejections with one word. You don't have to elaborate, I see why
these aren't a good fit.

> 
> > 
> > Solution 4:
> > 
> > You mentioned there being an xPU3 block here:
> > 
> > https://lore.kernel.org/linux-arm-msm/e3bfa28a-ecbc-7a57-a996-042650043514@linaro.org/
> > 
> > Maybe it's fine to have qcom,gpi-dma-v3?
> 
> I don't like made up version numbers. QCom does or did have very 
> specific version numbers, but in the end they it tended to be 1 or maybe 
> 2 SoCs per version. So not really beneficial.

I got this from using the number in xPU3, because I thought xPU3 was the
specific hardware that had ee_offset = 0. This might not be what Krzysztof meant
and perhaps I just wasn't reading properly.

Thank you for your response. You cleared up a lot of thoughts that I had about
the presented solution.