On Thu, Jun 27, 2024 at 12:03:38PM +0200, AngeloGioacchino Del Regno wrote: > Il 27/06/24 11:57, Conor Dooley ha scritto: > > On Thu, Jun 27, 2024 at 11:53:00AM +0200, AngeloGioacchino Del Regno wrote: > > > Il 27/06/24 11:47, Conor Dooley ha scritto: > > > > On Thu, Jun 27, 2024 at 11:33:47AM +0200, AngeloGioacchino Del Regno wrote: > > > > > Il 13/06/24 14:47, Lorenzo Bianconi ha scritto: > > > > > > Introduce reset capability to EN7581 device-tree clock binding > > > > > > documentation. > > > > > > > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx> > > > > > > --- > > > > > > .../bindings/clock/airoha,en7523-scu.yaml | 25 ++++++- > > > > > > .../dt-bindings/reset/airoha,en7581-reset.h | 66 +++++++++++++++++++ > > > > > > 2 files changed, 90 insertions(+), 1 deletion(-) > > > > > > create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml > > > > > > index 3f4266637733..84353fd09428 100644 > > > > > > --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml > > > > > > +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml > > > > > > @@ -35,7 +35,7 @@ properties: > > > > > > reg: > > > > > > minItems: 2 > > > > > > - maxItems: 3 > > > > > > + maxItems: 4 > > > > > > "#clock-cells": > > > > > > description: > > > > > > @@ -43,6 +43,10 @@ properties: > > > > > > clocks. > > > > > > const: 1 > > > > > > + '#reset-cells': > > > > > > + description: ID of the controller reset line > > > > > > + const: 1 > > > > > > + > > > > > > required: > > > > > > - compatible > > > > > > - reg > > > > > > @@ -60,6 +64,8 @@ allOf: > > > > > > - description: scu base address > > > > > > - description: misc scu base address > > > > > > + '#reset-cells': false > > > > > > + > > > > > > - if: > > > > > > properties: > > > > > > compatible: > > > > > > @@ -70,6 +76,7 @@ allOf: > > > > > > items: > > > > > > - description: scu base address > > > > > > - description: misc scu base address > > > > > > + - description: reset base address > > > > > > > > > > Are you sure that the indentation is correct? :-) > > > > > > > > > > After fixing the indentation, > > > > > > > > > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> > > > > > > > > > > > - description: pb scu base address > > > > > > > > The indentation actually looks okay when I apply this locally, but how is > > > > it backwards compatible to add this register in the middle of the list?? > > > > > > It's not, and this is actually something done on purpose - there is no DT using > > > this binding yet (here, nor uboot), and Lorenzo acknowledged the mistake before > > > it was too late... > > > > > > At least this time, it wasn't a misattention :-P > > > > The rationale for this being okay really should be in the commit > > message... > > > > > Btw, as far as I know, the reset base address is in between misc scu and pb scu, > > > that's why it was put there in the middle. > > > > ...but I don't really see why this needs to be done incompatibly at all > > in the first place. Just put it at the end of the list, there's no rule > > that the order of reg properties needs to match the MMIO addresses. > > > > It's just a perfection thing... but whatever, if that's unacceptable for you, > putting it at the end of the list isn't a huge deal anyway. > > Your call - it's okay for me either way. It has not been in a release yet, so it's not a hard block for me, however, I would want to see a commit message update and a Fixes: tag. The patch should also appear in 6.10, rather than being 6.11 material.
Attachment:
signature.asc
Description: PGP signature
