| Message ID | 20241101-sar2130p-dt-v3-1-61597eaf0c37@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: add QAR2130P support | expand |
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.
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 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 >
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:
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(-)