| 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 28/05/2025 16:37, 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. > > >> (?!wsa) >> Because qcom,x-wsa8845 should be matched and cause warnings. > > This is now confusing. I thought that the main job for the pattern above > is to avoid selecting wsa88xx amplifiers in the first place. Or, if I can > quote yourself: "What is WSA8815 that it should be here?" > > If said wsa8845 with incorrect or correct should be selected by that pattern > then why not just leave that pattern as it is then? I am lost. I guess I wanted to catch x-wsa8845 as well, but now never mind. It is not a soc so does not really matter for this file. > >> 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]+.*$" Here should be: s/wsa/w/ Best regards, Krzysztof
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. Best regards, Krzysztof
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 Konrad
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
On 5/29/25 6:58 PM, Krzysztof Kozlowski wrote: > 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. Aaaah ok it simply wasn't in the email context Konrad
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(-)