| Message ID | 20250522-rb2_audio_v3-v3-2-9eeb08cab9dc@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qrb4210-rb2: add wsa audio playback and capture support | expand |
On Thu, May 22, 2025 at 06:40:52PM GMT, Alexey Klimov wrote: > The pattern matching incorrectly selects "wsa" because of "sa" substring > and evaluates it as a SoC component or block. > > Wsa88xx are family of amplifiers and should not be evaluated here. > > Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org> > --- > Documentation/devicetree/bindings/arm/qcom-soc.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/arm/qcom-soc.yaml b/Documentation/devicetree/bindings/arm/qcom-soc.yaml > index a77d68dcad4e52e4fee43729ac8dc1caf957262e..99521813a04ca416fe90454a811c4a13143efce3 100644 > --- a/Documentation/devicetree/bindings/arm/qcom-soc.yaml > +++ b/Documentation/devicetree/bindings/arm/qcom-soc.yaml > @@ -23,7 +23,7 @@ description: | > select: > properties: > compatible: > - pattern: "^qcom,.*(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sa|sar|sc|sd[amx]|sm|x1[ep])[0-9]+.*$" > + pattern: "^qcom,(?!.*wsa)(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sa|sar|sc|sd[amx]|smx1[ep])[0-9]+.*$" Why dropping front .*? Are you sure this matches what we want - so incorrect compatibles? To me it breaks the entire point of this select, so I am sure you did not test whether it still works. To remind: this is to select incorrect compatibles. (?!wsa) Because qcom,x-wsa8845 should be matched and cause warnings. And probably we are getting past the point of readability, so could you try: compatible: anyOf: - pattern: "^qcom,.*(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sar|sc|sd[amx]|sm|x1[ep])[0-9]+.*$" - pattern: "^qcom,.*(?!wsa)sa[0-9]+.*$" > required: > - compatible > > > -- > 2.47.2 >
On 29/05/2025 18:34, Konrad Dybcio wrote: > On 5/29/25 8:58 AM, Krzysztof Kozlowski wrote: >> On 28/05/2025 18:58, Konrad Dybcio wrote: >>> On 5/28/25 4:37 PM, Alexey Klimov wrote: >>>> On Fri May 23, 2025 at 9:12 AM BST, Krzysztof Kozlowski wrote: >>>>> On Thu, May 22, 2025 at 06:40:52PM GMT, Alexey Klimov wrote: >>>>>> The pattern matching incorrectly selects "wsa" because of "sa" substring >>>>>> and evaluates it as a SoC component or block. >>>>>> >>>>>> Wsa88xx are family of amplifiers and should not be evaluated here. >>>>>> >>>>>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org> >>>>>> --- >>>>>> Documentation/devicetree/bindings/arm/qcom-soc.yaml | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/arm/qcom-soc.yaml b/Documentation/devicetree/bindings/arm/qcom-soc.yaml >>>>>> index a77d68dcad4e52e4fee43729ac8dc1caf957262e..99521813a04ca416fe90454a811c4a13143efce3 100644 >>>>>> --- a/Documentation/devicetree/bindings/arm/qcom-soc.yaml >>>>>> +++ b/Documentation/devicetree/bindings/arm/qcom-soc.yaml >>>>>> @@ -23,7 +23,7 @@ description: | >>>>>> select: >>>>>> properties: >>>>>> compatible: >>>>>> - pattern: "^qcom,.*(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sa|sar|sc|sd[amx]|sm|x1[ep])[0-9]+.*$" >>>>>> + pattern: "^qcom,(?!.*wsa)(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sa|sar|sc|sd[amx]|smx1[ep])[0-9]+.*$" >>>>> >>>>> Why dropping front .*? Are you sure this matches what we want - so >>>>> incorrect compatibles? To me it breaks the entire point of this select, >>>>> so I am sure you did not test whether it still works. To remind: this is >>>>> to select incorrect compatibles. >>>> >>>> Thanks, great point. I tested it with regular dtbs checks with different >>>> dtb files but I didn't check if it selects incorrect compatibles. >>> >>> Maybe we can introduce a '-' before or after the socname, to also officially >>> disallow using other connecting characters >> >> It is already there. > > Pardon, but I don't see it, only in the 0-9 group Then maybe we talk about different things? Because the one to fulfill your request - to officially disallow using other characters, which is part of the goal of this binding - is here: "^qcom,(apq| <snip> |sc|sd[amx]|sm|x1[ep])[0-9]+(pro)?-.*$ -----------^ That's the hyphen after soc name. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/arm/qcom-soc.yaml b/Documentation/devicetree/bindings/arm/qcom-soc.yaml index a77d68dcad4e52e4fee43729ac8dc1caf957262e..99521813a04ca416fe90454a811c4a13143efce3 100644 --- a/Documentation/devicetree/bindings/arm/qcom-soc.yaml +++ b/Documentation/devicetree/bindings/arm/qcom-soc.yaml @@ -23,7 +23,7 @@ description: | select: properties: compatible: - pattern: "^qcom,.*(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sa|sar|sc|sd[amx]|sm|x1[ep])[0-9]+.*$" + pattern: "^qcom,(?!.*wsa)(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sa|sar|sc|sd[amx]|smx1[ep])[0-9]+.*$" required: - compatible
The pattern matching incorrectly selects "wsa" because of "sa" substring and evaluates it as a SoC component or block. Wsa88xx are family of amplifiers and should not be evaluated here. Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org> --- Documentation/devicetree/bindings/arm/qcom-soc.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)