| Message ID | 20220923210934.280034-2-mailingradian@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | SDM670 GPI DMA support | expand |
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. > 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. > > 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. > > 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. Rob
> 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.
On 23-09-22, 23:26, Krzysztof Kozlowski 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. :) > 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 You should have access :-) > IP blocks. So knowing this IP we have two versions, one was initial sdm845 that should be the base compatible. Then second should be sm8350 which was the version we need ee_offset to be added, so these two can be the base ones for future... My 0.02 Thanks
diff --git a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml index eabf8a76d3a0..25bc1a6de794 100644 --- a/Documentation/devicetree/bindings/dma/qcom,gpi.yaml +++ b/Documentation/devicetree/bindings/dma/qcom,gpi.yaml @@ -18,14 +18,15 @@ allOf: properties: compatible: - enum: - - qcom,sc7280-gpi-dma - - qcom,sdm845-gpi-dma - - qcom,sm6350-gpi-dma - - qcom,sm8150-gpi-dma - - qcom,sm8250-gpi-dma - - qcom,sm8350-gpi-dma - - qcom,sm8450-gpi-dma + - enum: + - qcom,sc7280-gpi-dma + - qcom,sdm845-gpi-dma + - qcom,sm6350-gpi-dma + - qcom,sm8150-gpi-dma + - qcom,sm8250-gpi-dma + - qcom,sm8350-gpi-dma + - qcom,sm8450-gpi-dma + - const: qcom,gpi-dma reg: maxItems: 1
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. Signed-off-by: Richard Acayan <mailingradian@gmail.com> --- .../devicetree/bindings/dma/qcom,gpi.yaml | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)