Re: [PATCHv4 1/3] dt-bindings: mmc: synopsys-dw-mshc: document "altr,sysmgr-syscon"

[Ulf Hansson]

On Thu, 29 Sept 2022 at 11:39, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 29/09/2022 11:24, Ulf Hansson wrote:
> > On Wed, 28 Sept 2022 at 18:54, Dinh Nguyen <dinguyen@xxxxxxxxxx> wrote:
> >>
> >> Document the optional "altr,sysmgr-syscon" binding that is used to
> >> access the System Manager register that controls the SDMMC clock
> >> phase.
> >>
> >> Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxx>
> >> ---
> >> v4: add else statement
> >> v3: document that the "altr,sysmgr-syscon" binding is only applicable to
> >>     "altr,socfpga-dw-mshc"
> >> v2: document "altr,sysmgr-syscon" in the MMC section
> >> ---
> >>  .../bindings/mmc/synopsys-dw-mshc.yaml        | 31 +++++++++++++++++--
> >>  1 file changed, 28 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml
> >> index ae6d6fca79e2..b73324273464 100644
> >> --- a/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml
> >> +++ b/Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.yaml
> >> @@ -6,9 +6,6 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
> >>
> >>  title: Synopsys Designware Mobile Storage Host Controller Binding
> >>
> >> -allOf:
> >> -  - $ref: "synopsys-dw-mshc-common.yaml#"
> >> -
> >>  maintainers:
> >>    - Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> >>
> >> @@ -38,6 +35,34 @@ properties:
> >>        - const: biu
> >>        - const: ciu
> >>
> >> +  altr,sysmgr-syscon:
> >> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> >> +    items:
> >> +      - items:
> >> +          - description: phandle to the sysmgr node
> >> +          - description: register offset that controls the SDMMC clock phase
> >> +    description:
> >> +      Contains the phandle to System Manager block that contains
> >> +      the SDMMC clock-phase control register. The first value is the pointer
> >> +      to the sysmgr and the 2nd value is the register offset for the SDMMC
> >> +      clock phase register.
> >> +
> >> +allOf:
> >> +  - $ref: "synopsys-dw-mshc-common.yaml#"
> >> +
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            const:
> >> +              - altr,socfpga-dw-mshc
> >> +    then:
> >> +      required:
> >> +        - altr,sysmgr-syscon
> >> +    else:
> >> +      properties:
> >> +        altr,sysmgr-syscon: false
> >
> > So this change will not be backwards compatible with existing DTBs. I
> > noticed that patch2 updates the DTS files for the arm64 platforms, but
> > there seems to be some arm32 platforms too. Isn't this going to be a
> > problem?
> >
>
> The backwards compatibility is actually expressed by the driver. If the
> driver keeps ABI, we can change bindings so that all DTS are being
> updated to pass the checks.

Right.

So, I should probably have responded to patch3 instead, as backwards
compatibility doesn't seem to be supported, unless I am mistaken. But
let's move the discussion over to that thread instead.

>
> On the other hand, the commit should express why it changes the bindings
> in incompatible way - this is lacking here.
>
> Best regards,
> Krzysztof
>

Kind regards
Uffe