On Wed, 4 Dec 2019, at 01:01, Rob Herring wrote: > On Tue, Dec 3, 2019 at 6:36 AM Andrew Jeffery <andrew@xxxxxxxx> wrote: > > > > The v2 binding utilises reg and renames some of the v1 properties. > > > > Signed-off-by: Andrew Jeffery <andrew@xxxxxxxx> > > --- > > Documentation/devicetree/bindings/ipmi/aspeed-kcs-bmc.txt | 20 +++++--- > > 1 file changed, 14 insertions(+), 6 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/ipmi/aspeed-kcs-bmc.txt b/Documentation/devicetree/bindings/ipmi/aspeed-kcs-bmc.txt > > index d98a9bf45d6c..76b180ebbde4 100644 > > --- a/Documentation/devicetree/bindings/ipmi/aspeed-kcs-bmc.txt > > +++ b/Documentation/devicetree/bindings/ipmi/aspeed-kcs-bmc.txt > > @@ -1,9 +1,10 @@ > > -* Aspeed KCS (Keyboard Controller Style) IPMI interface > > +# Aspeed KCS (Keyboard Controller Style) IPMI interface > > > > The Aspeed SOCs (AST2400 and AST2500) are commonly used as BMCs > > (Baseboard Management Controllers) and the KCS interface can be > > used to perform in-band IPMI communication with their host. > > > > +## v1 > > Required properties: > > - compatible : should be one of > > "aspeed,ast2400-kcs-bmc" > > @@ -12,14 +13,21 @@ Required properties: > > - kcs_chan : The LPC channel number in the controller > > - kcs_addr : The host CPU IO map address > > > > +## v2 > > +Required properties: > > +- compatible : should be one of > > + "aspeed,ast2400-kcs-bmc-v2" > > + "aspeed,ast2500-kcs-bmc-v2" > > +- reg : The address and size of the IDR, ODR and STR registers > > +- interrupts : interrupt generated by the controller > > +- slave-reg : The host CPU IO map address > > aspeed,slave-reg I don't agree, as it's not an aspeed-specific behaviour. This property controls where the device appears in the host's LPC IO address space, which is a common problem for any LPC IO device exposed by the BMC to the host. > > > > > Example: > > > > - kcs3: kcs3@0 { > > - compatible = "aspeed,ast2500-kcs-bmc"; > > - reg = <0x0 0x80>; > > + kcs3: kcs@24 { > > + compatible = "aspeed,ast2500-kcs-bmc-v2"; > > + reg = <0x24 0x1>, <0x30 0x1>, <0x3c 0x1>; > > What are the other registers in this address space? I'm not so sure > this is an improvement if you end up with a bunch of nodes with single > registers. Put into practice the bindings give the following patch: on the AST2500: diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi index e8feb8b66a2f..5d51f469cbf0 100644 --- a/arch/arm/boot/dts/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed-g5.dtsi @@ -399,22 +399,22 @@ #size-cells = <1>; ranges = <0x0 0x0 0x80>; - kcs1: kcs1@0 { - compatible = "aspeed,ast2500-kcs-bmc"; + kcs1: kcs@24 { + compatible = "aspeed,ast2500-kcs-bmc-v2"; + reg = <0x24 0x1>, <0x30 0x1>, <0x3c 0x1>; interrupts = <8>; - kcs_chan = <1>; status = "disabled"; }; - kcs2: kcs2@0 { - compatible = "aspeed,ast2500-kcs-bmc"; + kcs2: kcs@28 { + compatible = "aspeed,ast2500-kcs-bmc-v2"; + reg = <0x28 0x1>, <0x34 0x1>, <0x40 0x1>; interrupts = <8>; - kcs_chan = <2>; status = "disabled"; }; - kcs3: kcs3@0 { - compatible = "aspeed,ast2500-kcs-bmc"; + kcs3: kcs@2c { + compatible = "aspeed,ast2500-kcs-bmc-v2"; + reg = <0x2c 0x1>, <0x38 0x1>, <0x44 0x1>; interrupts = <8>; - kcs_chan = <3>; status = "disabled"; }; }; @@ -428,10 +428,10 @@ #size-cells = <1>; ranges = <0x0 0x80 0x1e0>; - kcs4: kcs4@0 { - compatible = "aspeed,ast2500-kcs-bmc"; + kcs4: kcs@94 { + compatible = "aspeed,ast2500-kcs-bmc-v2"; + reg = <0x94 0x1>, <0x98 0x1>, <0x9c 0x1>; interrupts = <8>; - kcs_chan = <4>; status = "disabled"; }; The aim is to fix these warnings which appear for every aspeed-based devicetree: arch/arm/boot/dts/aspeed-g5.dtsi:376.19-381.8: Warning (unit_address_vs_reg): /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs1@0: node has a unit name, but no reg property arch/arm/boot/dts/aspeed-g5.dtsi:382.19-387.8: Warning (unit_address_vs_reg): /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs2@0: node has a unit name, but no reg property arch/arm/boot/dts/aspeed-g5.dtsi:388.19-393.8: Warning (unit_address_vs_reg): /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs3@0: node has a unit name, but no reg property arch/arm/boot/dts/aspeed-g5.dtsi:405.19-410.8: Warning (unit_address_vs_reg): /ahb/apb/lpc@1e789000/lpc-host@80/kcs4@0: node has a unit name, but no reg property arch/arm/boot/dts/aspeed-g5.dtsi:376.19-381.8: Warning (unique_unit_address): /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs1@0: duplicate unit-address (also used in node /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs2@0) arch/arm/boot/dts/aspeed-g5.dtsi:376.19-381.8: Warning (unique_unit_address): /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs1@0: duplicate unit-address (also used in node /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs3@0) arch/arm/boot/dts/aspeed-g5.dtsi:382.19-387.8: Warning (unique_unit_address): /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs2@0: duplicate unit-address (also used in node /ahb/apb/lpc@1e789000/lpc-bmc@0/kcs3@0) arch/arm/boot/dts/aspeed-g5.dtsi:405.19-410.8: Warning (unique_unit_address): /ahb/apb/lpc@1e789000/lpc-host@80/kcs4@0: duplicate unit-address (also used in node /ahb/apb/lpc@1e789000/lpc-host@80/lpc-ctrl@0) Andrew