The patterns for individual SoC families grew up to be pretty complex,
containing lots of special cases and optional suffixes. Split them per
the suffix to make it easier to extend SoC patterns.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 Documentation/devicetree/bindings/arm/qcom-soc.yaml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/qcom-soc.yaml b/Documentation/devicetree/bindings/arm/qcom-soc.yaml
index d0751a572af39eecbbd2f8323a6c3c94b3fdeeac..c67dcda4c8169dd72e9b5d5ca4926991a730f67c 100644
--- a/Documentation/devicetree/bindings/arm/qcom-soc.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom-soc.yaml
@@ -31,8 +31,10 @@ properties:
   compatible:
     oneOf:
       # Preferred naming style for compatibles of SoC components:
-      - pattern: "^qcom,(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sa|sc|sd[amx]|sm|x1e)[0-9]+(pro)?-.*$"
-      - pattern: "^qcom,(sa|sc)8[0-9]+[a-z][a-z]?-.*$"
+      - pattern: "^qcom,(apq|ipq|mdm|msm|qcm|qcs|q[dr]u|sa|sc|sd[amx]|sm|x1e)[0-9]+-.*$"
+      - pattern: "^qcom,msm8[0-9]+pro-.*$"
+      - pattern: "^qcom,sa[0-9]+p-.*$"
+      - pattern: "^qcom,sc[0-9]+(x|xp)-.*$"
 
       # Legacy namings - variations of existing patterns/compatibles are OK,
       # but do not add completely new entries to these:

-- 
2.39.5

On Fri, Nov 01, 2024 at 02:49:22AM +0200, Dmitry Baryshkov wrote:
> The patterns for individual SoC families grew up to be pretty complex,
> containing lots of special cases and optional suffixes. Split them per
> the suffix to make it easier to extend SoC patterns.

This is doing something quite different - split is not important here.
Instead you narrow the patterns significantly and disallow things like
msm8994pro, sc8280p or sc8280px, and allow things like sa5200p.

I don't see here much of pattern simplifying - dropping (pro)? really
makes little difference.

Best regards,
Krzysztof

On Fri, Nov 01, 2024 at 08:26:04AM +0100, Krzysztof Kozlowski wrote:
> On Fri, Nov 01, 2024 at 02:49:22AM +0200, Dmitry Baryshkov wrote:
> > The patterns for individual SoC families grew up to be pretty complex,
> > containing lots of special cases and optional suffixes. Split them per
> > the suffix to make it easier to extend SoC patterns.
> 
> This is doing something quite different - split is not important here.
> Instead you narrow the patterns significantly and disallow things like
> msm8994pro, sc8280p or sc8280px, and allow things like sa5200p.

Just for the sake of correctness, msm8994pro is still allowed, if I'm
not mistaken.

> I don't see here much of pattern simplifying - dropping (pro)? really
> makes little difference.

Patterns are simplified by being explicit. E.g. in the previous
iteration I completely didn't notice the intersection of the |p that I
have added with the existing [a-z][a-z]? pattern. If you think that
sa5200p should be disallowed, I can tune the numeric part of the
pattern. And sc8280p / sc8280px should not be allowed in the first
place, such platforms don't exist.

-- 
With best wishes
Dmitry

On 01/11/2024 08:47, Dmitry Baryshkov wrote:
> On Fri, Nov 01, 2024 at 08:26:04AM +0100, Krzysztof Kozlowski wrote:
>> On Fri, Nov 01, 2024 at 02:49:22AM +0200, Dmitry Baryshkov wrote:
>>> The patterns for individual SoC families grew up to be pretty complex,
>>> containing lots of special cases and optional suffixes. Split them per
>>> the suffix to make it easier to extend SoC patterns.
>>
>> This is doing something quite different - split is not important here.
>> Instead you narrow the patterns significantly and disallow things like
>> msm8994pro, sc8280p or sc8280px, and allow things like sa5200p.
> 
> Just for the sake of correctness, msm8994pro is still allowed, if I'm
> not mistaken.
> 
>> I don't see here much of pattern simplifying - dropping (pro)? really
>> makes little difference.
> 
> Patterns are simplified by being explicit. E.g. in the previous
> iteration I completely didn't notice the intersection of the |p that I
> have added with the existing [a-z][a-z]? pattern. If you think that
> sa5200p should be disallowed, I can tune the numeric part of the
> pattern. And sc8280p / sc8280px should not be allowed in the first
> place, such platforms don't exist.

I am fine with this, but extend the commit msg with some good rationale.
Have in mind that the point of this pattern was *not* to validate SoCs
names. sa5200p is fine, sc8180p is fine and all others are fine, sc8280z
as well, because we do not want to grow this pattern with every new model.

The only, single point of this entire binding is to disallow incorrect
order of block names in compatible. Not validate the SoC names. If you
need narrower patterns to achieve that objective, sure. If you need
narrower patterns to validate SoC names, then nope.

Best regards,
Krzysztof

On Fri, Nov 01, 2024 at 09:37:23AM +0100, Krzysztof Kozlowski wrote:
> On 01/11/2024 08:47, Dmitry Baryshkov wrote:
> > On Fri, Nov 01, 2024 at 08:26:04AM +0100, Krzysztof Kozlowski wrote:
> >> On Fri, Nov 01, 2024 at 02:49:22AM +0200, Dmitry Baryshkov wrote:
> >>> The patterns for individual SoC families grew up to be pretty complex,
> >>> containing lots of special cases and optional suffixes. Split them per
> >>> the suffix to make it easier to extend SoC patterns.
> >>
> >> This is doing something quite different - split is not important here.
> >> Instead you narrow the patterns significantly and disallow things like
> >> msm8994pro, sc8280p or sc8280px, and allow things like sa5200p.
> > 
> > Just for the sake of correctness, msm8994pro is still allowed, if I'm
> > not mistaken.
> > 
> >> I don't see here much of pattern simplifying - dropping (pro)? really
> >> makes little difference.
> > 
> > Patterns are simplified by being explicit. E.g. in the previous
> > iteration I completely didn't notice the intersection of the |p that I
> > have added with the existing [a-z][a-z]? pattern. If you think that
> > sa5200p should be disallowed, I can tune the numeric part of the
> > pattern. And sc8280p / sc8280px should not be allowed in the first
> > place, such platforms don't exist.
> 
> I am fine with this, but extend the commit msg with some good rationale.
> Have in mind that the point of this pattern was *not* to validate SoCs
> names. sa5200p is fine, sc8180p is fine and all others are fine, sc8280z
> as well, because we do not want to grow this pattern with every new model.
> 
> The only, single point of this entire binding is to disallow incorrect
> order of block names in compatible. Not validate the SoC names. If you
> need narrower patterns to achieve that objective, sure. If you need
> narrower patterns to validate SoC names, then nope.

I need narrower patterns to simplify adding new SoCs.
Another option is to define a mega-pattern like
qcom,(msm|sm|sd[am]|.....)[0-9]+[a-z]*-.* . Frankly speaking I'm fine
with that approach too.

-- 
With best wishes
Dmitry

On 01/11/2024 09:52, Dmitry Baryshkov wrote:
> On Fri, Nov 01, 2024 at 09:37:23AM +0100, Krzysztof Kozlowski wrote:
>> On 01/11/2024 08:47, Dmitry Baryshkov wrote:
>>> On Fri, Nov 01, 2024 at 08:26:04AM +0100, Krzysztof Kozlowski wrote:
>>>> On Fri, Nov 01, 2024 at 02:49:22AM +0200, Dmitry Baryshkov wrote:
>>>>> The patterns for individual SoC families grew up to be pretty complex,
>>>>> containing lots of special cases and optional suffixes. Split them per
>>>>> the suffix to make it easier to extend SoC patterns.
>>>>
>>>> This is doing something quite different - split is not important here.
>>>> Instead you narrow the patterns significantly and disallow things like
>>>> msm8994pro, sc8280p or sc8280px, and allow things like sa5200p.
>>>
>>> Just for the sake of correctness, msm8994pro is still allowed, if I'm
>>> not mistaken.
>>>
>>>> I don't see here much of pattern simplifying - dropping (pro)? really
>>>> makes little difference.
>>>
>>> Patterns are simplified by being explicit. E.g. in the previous
>>> iteration I completely didn't notice the intersection of the |p that I
>>> have added with the existing [a-z][a-z]? pattern. If you think that
>>> sa5200p should be disallowed, I can tune the numeric part of the
>>> pattern. And sc8280p / sc8280px should not be allowed in the first
>>> place, such platforms don't exist.
>>
>> I am fine with this, but extend the commit msg with some good rationale.
>> Have in mind that the point of this pattern was *not* to validate SoCs
>> names. sa5200p is fine, sc8180p is fine and all others are fine, sc8280z
>> as well, because we do not want to grow this pattern with every new model.
>>
>> The only, single point of this entire binding is to disallow incorrect
>> order of block names in compatible. Not validate the SoC names. If you
>> need narrower patterns to achieve that objective, sure. If you need
>> narrower patterns to validate SoC names, then nope.
> 
> I need narrower patterns to simplify adding new SoCs.
> Another option is to define a mega-pattern like
> qcom,(msm|sm|sd[am]|.....)[0-9]+[a-z]*-.* . Frankly speaking I'm fine
> with that approach too.

I do not see how narrower patterns, changing:
"^qcom,(sa|sc)8[0-9]+[a-z][a-z]?-.*$"
into
pattern: "^qcom,sa[0-9]+p-.*$"

instead of
pattern: "^qcom,sa[0-9]+[a-z]-.*$"
is needed for that. It's true that 'p' is simpler than '[a-z]' but if
this results in new commit next time we have sa8995r, then benefit is lost.

Best regards,
Krzysztof

On Fri, 1 Nov 2024 at 11:23, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 01/11/2024 09:52, Dmitry Baryshkov wrote:
> > On Fri, Nov 01, 2024 at 09:37:23AM +0100, Krzysztof Kozlowski wrote:
> >> On 01/11/2024 08:47, Dmitry Baryshkov wrote:
> >>> On Fri, Nov 01, 2024 at 08:26:04AM +0100, Krzysztof Kozlowski wrote:
> >>>> On Fri, Nov 01, 2024 at 02:49:22AM +0200, Dmitry Baryshkov wrote:
> >>>>> The patterns for individual SoC families grew up to be pretty complex,
> >>>>> containing lots of special cases and optional suffixes. Split them per
> >>>>> the suffix to make it easier to extend SoC patterns.
> >>>>
> >>>> This is doing something quite different - split is not important here.
> >>>> Instead you narrow the patterns significantly and disallow things like
> >>>> msm8994pro, sc8280p or sc8280px, and allow things like sa5200p.
> >>>
> >>> Just for the sake of correctness, msm8994pro is still allowed, if I'm
> >>> not mistaken.
> >>>
> >>>> I don't see here much of pattern simplifying - dropping (pro)? really
> >>>> makes little difference.
> >>>
> >>> Patterns are simplified by being explicit. E.g. in the previous
> >>> iteration I completely didn't notice the intersection of the |p that I
> >>> have added with the existing [a-z][a-z]? pattern. If you think that
> >>> sa5200p should be disallowed, I can tune the numeric part of the
> >>> pattern. And sc8280p / sc8280px should not be allowed in the first
> >>> place, such platforms don't exist.
> >>
> >> I am fine with this, but extend the commit msg with some good rationale.
> >> Have in mind that the point of this pattern was *not* to validate SoCs
> >> names. sa5200p is fine, sc8180p is fine and all others are fine, sc8280z
> >> as well, because we do not want to grow this pattern with every new model.
> >>
> >> The only, single point of this entire binding is to disallow incorrect
> >> order of block names in compatible. Not validate the SoC names. If you
> >> need narrower patterns to achieve that objective, sure. If you need
> >> narrower patterns to validate SoC names, then nope.
> >
> > I need narrower patterns to simplify adding new SoCs.
> > Another option is to define a mega-pattern like
> > qcom,(msm|sm|sd[am]|.....)[0-9]+[a-z]*-.* . Frankly speaking I'm fine
> > with that approach too.
>
> I do not see how narrower patterns, changing:
> "^qcom,(sa|sc)8[0-9]+[a-z][a-z]?-.*$"
> into
> pattern: "^qcom,sa[0-9]+p-.*$"
>
> instead of
> pattern: "^qcom,sa[0-9]+[a-z]-.*$"
> is needed for that. It's true that 'p' is simpler than '[a-z]' but if
> this results in new commit next time we have sa8995r, then benefit is lost.

Ok, let's drop it, I will just update the qcom-soc.yaml to handle
sar21330p via the "^qcom,(sa|sar|sc)8[0-9]+[a-z][a-z]?-.*$"

>
> Best regards,
> Krzysztof
>


-- 
With best wishes
Dmitry