[RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings

[RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings

[Andrew Jeffery]

Hello,

The aim of this series is to minimise/eliminate all the warnings from the
ASPEED devicetrees. It mostly achieves its goal, as outlined below.

Using `aspeed_g5_defconfig` we started with the follow warning count:

    $ make dtbs 2>&1 >/dev/null | wc -l
    218

and after the full series is applied we have:

    $ make dtbs 2>&1 >/dev/null | wc -l
    2

for a 100x reduction.

Getting there though isn't without some potential controversy, which I've saved
for the last half of the series. The following patches I think are in pretty
good shape:

  ARM: dts: aspeed-g5: Move EDAC node to APB
  ARM: dts: aspeed-g5: Use recommended generic node name for SDMC
  ARM: dts: aspeed-g5: Fix aspeed,external-nodes description
  ARM: dts: vesnin: Add unit address for memory node
  ARM: dts: fp5280g2: Cleanup gpio-keys-polled properties
  ARM: dts: swift: Cleanup gpio-keys-polled properties
  ARM: dts: witherspoon: Cleanup gpio-keys-polled properties
  ARM: dts: aspeed: Cleanup lpc-ctrl and snoop regs
  ARM: dts: ibm-power9-dual: Add a unit address for OCC nodes

With these patches applied we get to:

    $ make dtbs 2>&1 >/dev/null | wc -l
    144

So they make a dent, but fail to clean up the bulk of the issues. From here
I've mixed in some binding and driver changes with subsequent updates to the
devicetrees:

  dt-bindings: pinctrl: aspeed: Add reg property as a hint
  dt-bindings: misc: Document reg for aspeed,p2a-ctrl nodes
  ARM: dts: aspeed: Add reg hints to syscon children
  dt-bindings: ipmi: aspeed: Introduce a v2 binding for KCS
  ipmi: kcs: Finish configuring ASPEED KCS device before enable
  ipmi: kcs: aspeed: Implement v2 bindings
  ARM: dts: aspeed-g5: Change KCS nodes to v2 binding
  ARM: dts: aspeed-g5: Sort LPC child nodes by unit address

By `dt-bindings: ipmi: aspeed: Introduce a v2 binding for KCS` the warnings are
reduced to:

    $ make dtbs 2>&1 >/dev/null | wc -l
    125

The bang-for-buck is in fixing up the KCS bindings which removes all-but-two of
the remaining warnings (which we can't feasibly remove), but doing so forces
code changes (which I'd avoided up until this point).

Reflecting broadly on the fixes, I think I've made a mistake way back by using
syscon/simple-mfds to expose the innards of the SCU and LPC controllers in the
devicetree. This series cleans up what's currently there, but I have half a
mind to rev the SCU and LPC bindings to not use simple-mfd and instead have a
driver implementation that uses `platform_device_register_full()` or similar to
deal with the mess.

Rob - I'm looking for your thoughts here and on the series, I've never felt
entirely comfortable with what I cooked up. Your advice would be appreciated.

Anyway, everyone, please let me know your thoughts on the bits relevant to you.
If we can agree on a way forward I'll split up the series for subsequent
submissions so it isn't such a spam-fest.

Cheers,

Andrew

Andrew Jeffery (17):
  ARM: dts: aspeed-g5: Move EDAC node to APB
  ARM: dts: aspeed-g5: Use recommended generic node name for SDMC
  ARM: dts: aspeed-g5: Fix aspeed,external-nodes description
  ARM: dts: vesnin: Add unit address for memory node
  ARM: dts: fp5280g2: Cleanup gpio-keys-polled properties
  ARM: dts: swift: Cleanup gpio-keys-polled properties
  ARM: dts: witherspoon: Cleanup gpio-keys-polled properties
  ARM: dts: aspeed: Cleanup lpc-ctrl and snoop regs
  ARM: dts: ibm-power9-dual: Add a unit address for OCC nodes
  dt-bindings: pinctrl: aspeed: Add reg property as a hint
  dt-bindings: misc: Document reg for aspeed,p2a-ctrl nodes
  ARM: dts: aspeed: Add reg hints to syscon children
  dt-bindings: ipmi: aspeed: Introduce a v2 binding for KCS
  ipmi: kcs: Finish configuring ASPEED KCS device before enable
  ipmi: kcs: aspeed: Implement v2 bindings
  ARM: dts: aspeed-g5: Change KCS nodes to v2 binding
  ARM: dts: aspeed-g5: Sort LPC child nodes by unit address

 .../bindings/ipmi/aspeed-kcs-bmc.txt          |  20 ++-
 .../bindings/misc/aspeed-p2a-ctrl.txt         |   1 +
 .../pinctrl/aspeed,ast2400-pinctrl.yaml       |   3 +
 .../pinctrl/aspeed,ast2500-pinctrl.yaml       |   3 +
 .../dts/aspeed-bmc-arm-centriq2400-rep.dts    |   4 -
 .../aspeed-bmc-arm-stardragon4800-rep2.dts    |   4 -
 arch/arm/boot/dts/aspeed-bmc-facebook-cmm.dts |   4 -
 .../arm/boot/dts/aspeed-bmc-facebook-yamp.dts |   4 -
 .../boot/dts/aspeed-bmc-inspur-fp5280g2.dts   |   6 -
 .../arm/boot/dts/aspeed-bmc-intel-s2600wf.dts |   4 -
 arch/arm/boot/dts/aspeed-bmc-opp-lanyang.dts  |   4 -
 arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts  |   4 -
 arch/arm/boot/dts/aspeed-bmc-opp-swift.dts    |   6 -
 arch/arm/boot/dts/aspeed-bmc-opp-vesnin.dts   |   2 +-
 .../boot/dts/aspeed-bmc-opp-witherspoon.dts   |   6 -
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts    |   2 -
 arch/arm/boot/dts/aspeed-g4.dtsi              |  21 ++-
 arch/arm/boot/dts/aspeed-g5.dtsi              |  73 ++++----
 arch/arm/boot/dts/ibm-power9-dual.dtsi        |   4 +-
 drivers/char/ipmi/kcs_bmc_aspeed.c            | 163 ++++++++++++++----
 20 files changed, 205 insertions(+), 133 deletions(-)

-- 
2.20.1

[RFC PATCH 10/17] dt-bindings: pinctrl: aspeed: Add reg property as a hint

[Andrew Jeffery]

The pinctrl node sits under a syscon device, and can assume offsets from
the base of the syscon based on the compatible. However, for devicetree
correctness allow a reg property to be specified, which an associated
driver may choose to use to discover associated resources.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-gpio@vger.kernel.org
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 .../devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml    | 3 +++
 .../devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml    | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
index 125599a2dc5e..629a55c167ae 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml
@@ -25,6 +25,9 @@ properties:
     enum:
       - aspeed,ast2400-pinctrl
       - aspeed,g4-pinctrl
+  reg:
+    description: |
+      A hint for the memory regions associated with the pin-controller
 
 patternProperties:
   '^.*$':
diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
index 3e6d85318577..2e8971e23a02 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml
@@ -25,6 +25,9 @@ properties:
     enum:
       - aspeed,ast2500-pinctrl
       - aspeed,g5-pinctrl
+  reg:
+    description: |
+      A hint for the memory regions associated with the pin-controller
   aspeed,external-nodes:
     minItems: 2
     maxItems: 2
-- 
2.20.1

Re: [RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings

[Linus Walleij]

On Fri, Jul 26, 2019 at 7:40 AM Andrew Jeffery <andrew@aj.id.au> wrote:

> The aim of this series is to minimise/eliminate all the warnings from the
> ASPEED devicetrees. It mostly achieves its goal, as outlined below.

I suppose it will all be merged in  the Aspeed tree?
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings

[Andrew Jeffery]

On Tue, 30 Jul 2019, at 07:25, Linus Walleij wrote:
> On Fri, Jul 26, 2019 at 7:40 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> 
> > The aim of this series is to minimise/eliminate all the warnings from the
> > ASPEED devicetrees. It mostly achieves its goal, as outlined below.
> 
> I suppose it will all be merged in  the Aspeed tree?
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yeah, if you're happy for that. Thanks.

Andrew

Re: [RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings

[Rob Herring]

On Thu, Jul 25, 2019 at 11:40 PM Andrew Jeffery <andrew@aj.id.au> wrote:
>
> Hello,
>
> The aim of this series is to minimise/eliminate all the warnings from the
> ASPEED devicetrees. It mostly achieves its goal, as outlined below.
>
> Using `aspeed_g5_defconfig` we started with the follow warning count:
>
>     $ make dtbs 2>&1 >/dev/null | wc -l
>     218
>
> and after the full series is applied we have:
>
>     $ make dtbs 2>&1 >/dev/null | wc -l
>     2
>
> for a 100x reduction.
>
> Getting there though isn't without some potential controversy, which I've saved
> for the last half of the series. The following patches I think are in pretty
> good shape:
>
>   ARM: dts: aspeed-g5: Move EDAC node to APB
>   ARM: dts: aspeed-g5: Use recommended generic node name for SDMC
>   ARM: dts: aspeed-g5: Fix aspeed,external-nodes description
>   ARM: dts: vesnin: Add unit address for memory node
>   ARM: dts: fp5280g2: Cleanup gpio-keys-polled properties
>   ARM: dts: swift: Cleanup gpio-keys-polled properties
>   ARM: dts: witherspoon: Cleanup gpio-keys-polled properties
>   ARM: dts: aspeed: Cleanup lpc-ctrl and snoop regs
>   ARM: dts: ibm-power9-dual: Add a unit address for OCC nodes
>
> With these patches applied we get to:
>
>     $ make dtbs 2>&1 >/dev/null | wc -l
>     144
>
> So they make a dent, but fail to clean up the bulk of the issues. From here
> I've mixed in some binding and driver changes with subsequent updates to the
> devicetrees:
>
>   dt-bindings: pinctrl: aspeed: Add reg property as a hint
>   dt-bindings: misc: Document reg for aspeed,p2a-ctrl nodes
>   ARM: dts: aspeed: Add reg hints to syscon children
>   dt-bindings: ipmi: aspeed: Introduce a v2 binding for KCS
>   ipmi: kcs: Finish configuring ASPEED KCS device before enable
>   ipmi: kcs: aspeed: Implement v2 bindings
>   ARM: dts: aspeed-g5: Change KCS nodes to v2 binding
>   ARM: dts: aspeed-g5: Sort LPC child nodes by unit address
>
> By `dt-bindings: ipmi: aspeed: Introduce a v2 binding for KCS` the warnings are
> reduced to:
>
>     $ make dtbs 2>&1 >/dev/null | wc -l
>     125
>
> The bang-for-buck is in fixing up the KCS bindings which removes all-but-two of
> the remaining warnings (which we can't feasibly remove), but doing so forces
> code changes (which I'd avoided up until this point).
>
> Reflecting broadly on the fixes, I think I've made a mistake way back by using
> syscon/simple-mfds to expose the innards of the SCU and LPC controllers in the
> devicetree. This series cleans up what's currently there, but I have half a
> mind to rev the SCU and LPC bindings to not use simple-mfd and instead have a
> driver implementation that uses `platform_device_register_full()` or similar to
> deal with the mess.
>
> Rob - I'm looking for your thoughts here and on the series, I've never felt
> entirely comfortable with what I cooked up. Your advice would be appreciated.

The series generally looks fine to me from a quick scan. As far as
dropping 'simple-mfd', having less fine grained description in DT is
generally my preference. It comes down to whether what you have
defined is maintainable. As most of it is just additions, I think what
you have is fine. Maybe keep all this in mind for the next chip
depending how the SCU and LPC change.

Rob

Re: [RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings

[Andrew Jeffery]

On Tue, 30 Jul 2019, at 10:23, Rob Herring wrote:
> On Thu, Jul 25, 2019 at 11:40 PM Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> > Hello,
> >
> > The aim of this series is to minimise/eliminate all the warnings from the
> > ASPEED devicetrees. It mostly achieves its goal, as outlined below.
> >
> > Using `aspeed_g5_defconfig` we started with the follow warning count:
> >
> >     $ make dtbs 2>&1 >/dev/null | wc -l
> >     218
> >
> > and after the full series is applied we have:
> >
> >     $ make dtbs 2>&1 >/dev/null | wc -l
> >     2
> >
> > for a 100x reduction.
> >
> > Getting there though isn't without some potential controversy, which I've saved
> > for the last half of the series. The following patches I think are in pretty
> > good shape:
> >
> >   ARM: dts: aspeed-g5: Move EDAC node to APB
> >   ARM: dts: aspeed-g5: Use recommended generic node name for SDMC
> >   ARM: dts: aspeed-g5: Fix aspeed,external-nodes description
> >   ARM: dts: vesnin: Add unit address for memory node
> >   ARM: dts: fp5280g2: Cleanup gpio-keys-polled properties
> >   ARM: dts: swift: Cleanup gpio-keys-polled properties
> >   ARM: dts: witherspoon: Cleanup gpio-keys-polled properties
> >   ARM: dts: aspeed: Cleanup lpc-ctrl and snoop regs
> >   ARM: dts: ibm-power9-dual: Add a unit address for OCC nodes
> >
> > With these patches applied we get to:
> >
> >     $ make dtbs 2>&1 >/dev/null | wc -l
> >     144
> >
> > So they make a dent, but fail to clean up the bulk of the issues. From here
> > I've mixed in some binding and driver changes with subsequent updates to the
> > devicetrees:
> >
> >   dt-bindings: pinctrl: aspeed: Add reg property as a hint
> >   dt-bindings: misc: Document reg for aspeed,p2a-ctrl nodes
> >   ARM: dts: aspeed: Add reg hints to syscon children
> >   dt-bindings: ipmi: aspeed: Introduce a v2 binding for KCS
> >   ipmi: kcs: Finish configuring ASPEED KCS device before enable
> >   ipmi: kcs: aspeed: Implement v2 bindings
> >   ARM: dts: aspeed-g5: Change KCS nodes to v2 binding
> >   ARM: dts: aspeed-g5: Sort LPC child nodes by unit address
> >
> > By `dt-bindings: ipmi: aspeed: Introduce a v2 binding for KCS` the warnings are
> > reduced to:
> >
> >     $ make dtbs 2>&1 >/dev/null | wc -l
> >     125
> >
> > The bang-for-buck is in fixing up the KCS bindings which removes all-but-two of
> > the remaining warnings (which we can't feasibly remove), but doing so forces
> > code changes (which I'd avoided up until this point).
> >
> > Reflecting broadly on the fixes, I think I've made a mistake way back by using
> > syscon/simple-mfds to expose the innards of the SCU and LPC controllers in the
> > devicetree. This series cleans up what's currently there, but I have half a
> > mind to rev the SCU and LPC bindings to not use simple-mfd and instead have a
> > driver implementation that uses `platform_device_register_full()` or similar to
> > deal with the mess.
> >
> > Rob - I'm looking for your thoughts here and on the series, I've never felt
> > entirely comfortable with what I cooked up. Your advice would be appreciated.
> 
> The series generally looks fine to me from a quick scan. As far as
> dropping 'simple-mfd', having less fine grained description in DT is
> generally my preference. It comes down to whether what you have
> defined is maintainable. As most of it is just additions, I think what
> you have is fine. Maybe keep all this in mind for the next chip
> depending how the SCU and LPC change.

Okay, I think the timing of that suggestion is good given where things are with
the AST2600. I'll keep that in mind.

Consensus so far seems to be that the series is fine. I'll split it up and send out
the sub-series to the relevant lists with the acks accumulated here.

Thanks all for taking a look.

Andrew

Re: [RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings

[Joel Stanley]

On Tue, 30 Jul 2019 at 01:09, Andrew Jeffery <andrew@aj.id.au> wrote:

> > > The bang-for-buck is in fixing up the KCS bindings which removes all-but-two of
> > > the remaining warnings (which we can't feasibly remove), but doing so forces
> > > code changes (which I'd avoided up until this point).
> > >
> > > Reflecting broadly on the fixes, I think I've made a mistake way back by using
> > > syscon/simple-mfds to expose the innards of the SCU and LPC controllers in the
> > > devicetree. This series cleans up what's currently there, but I have half a
> > > mind to rev the SCU and LPC bindings to not use simple-mfd and instead have a
> > > driver implementation that uses `platform_device_register_full()` or similar to
> > > deal with the mess.
> > >
> > > Rob - I'm looking for your thoughts here and on the series, I've never felt
> > > entirely comfortable with what I cooked up. Your advice would be appreciated.
> >
> > The series generally looks fine to me from a quick scan. As far as
> > dropping 'simple-mfd', having less fine grained description in DT is
> > generally my preference. It comes down to whether what you have
> > defined is maintainable. As most of it is just additions, I think what
> > you have is fine. Maybe keep all this in mind for the next chip
> > depending how the SCU and LPC change.
>
> Okay, I think the timing of that suggestion is good given where things are with
> the AST2600. I'll keep that in mind.
>
> Consensus so far seems to be that the series is fine. I'll split it up and send out
> the sub-series to the relevant lists with the acks accumulated here.

The series look good. I suggest posting the KCS bindings and driver
changes as their own series to go through the IPMI tree.

Please add my tag to all the patches except the OCC one, which I need
to do some investigation in to.

Reviewed-by: Joel Stanley <joel@jms.id.au>

The others can go via the aspeed tree. Perhaps post them as their own
series too so I don't get confused and apply the wrong ones. That way
if Rob wants to send his reviewed-by he can.

Cheers,

Joel

Re: [RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings

[Andrew Jeffery]

On Fri, 2 Aug 2019, at 15:21, Joel Stanley wrote:
> On Tue, 30 Jul 2019 at 01:09, Andrew Jeffery <andrew@aj.id.au> wrote:
> 
> > > > The bang-for-buck is in fixing up the KCS bindings which removes all-but-two of
> > > > the remaining warnings (which we can't feasibly remove), but doing so forces
> > > > code changes (which I'd avoided up until this point).
> > > >
> > > > Reflecting broadly on the fixes, I think I've made a mistake way back by using
> > > > syscon/simple-mfds to expose the innards of the SCU and LPC controllers in the
> > > > devicetree. This series cleans up what's currently there, but I have half a
> > > > mind to rev the SCU and LPC bindings to not use simple-mfd and instead have a
> > > > driver implementation that uses `platform_device_register_full()` or similar to
> > > > deal with the mess.
> > > >
> > > > Rob - I'm looking for your thoughts here and on the series, I've never felt
> > > > entirely comfortable with what I cooked up. Your advice would be appreciated.
> > >
> > > The series generally looks fine to me from a quick scan. As far as
> > > dropping 'simple-mfd', having less fine grained description in DT is
> > > generally my preference. It comes down to whether what you have
> > > defined is maintainable. As most of it is just additions, I think what
> > > you have is fine. Maybe keep all this in mind for the next chip
> > > depending how the SCU and LPC change.
> >
> > Okay, I think the timing of that suggestion is good given where things are with
> > the AST2600. I'll keep that in mind.
> >
> > Consensus so far seems to be that the series is fine. I'll split it up and send out
> > the sub-series to the relevant lists with the acks accumulated here.
> 
> The series look good. I suggest posting the KCS bindings and driver
> changes as their own series to go through the IPMI tree.

Yeah, that was the plan.

> 
> Please add my tag to all the patches except the OCC one, which I need
> to do some investigation in to.
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>

Thanks, will do.

> 
> The others can go via the aspeed tree. Perhaps post them as their own
> series too so I don't get confused and apply the wrong ones. That way
> if Rob wants to send his reviewed-by he can.

SGTM.

Cheers,

Andrew