[PATCH 2/3] dt-bindings: serial: 8250: update for aspeed,sirq-active-high

[PATCH 2/3] dt-bindings: serial: 8250: update for aspeed,sirq-active-high

[Zev Weiss]

Update DT bindings documentation for the new incarnation of the
aspeed,sirq-polarity-sense property.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 Documentation/devicetree/bindings/serial/8250.yaml | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index f54cae9ff7b2..0bbb7121f720 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -13,7 +13,7 @@ allOf:
   - $ref: /schemas/serial.yaml#
   - if:
       required:
-        - aspeed,sirq-polarity-sense
+        - aspeed,sirq-active-high
     then:
       properties:
         compatible:
@@ -181,13 +181,11 @@ properties:
   rng-gpios: true
   dcd-gpios: true
 
-  aspeed,sirq-polarity-sense:
-    $ref: /schemas/types.yaml#/definitions/phandle-array
+  aspeed,sirq-active-high:
+    type: boolean
     description: |
-      Phandle to aspeed,ast2500-scu compatible syscon alongside register
-      offset and bit number to identify how the SIRQ polarity should be
-      configured. One possible data source is the LPC/eSPI mode bit. Only
-      applicable to aspeed,ast2500-vuart.
+      Set to indicate that the SIRQ polarity is active-high (default
+      is active-low).  Only applicable to aspeed,ast2500-vuart.
 
 required:
   - reg
@@ -227,7 +225,7 @@ examples:
         interrupts = <8>;
         clocks = <&syscon ASPEED_CLK_APB>;
         no-loopback-test;
-        aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
+        aspeed,sirq-active-high;
     };
 
 ...
-- 
2.31.1

[PATCH 3/3] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Zev Weiss]

This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
mini-ITX board that we hope can provide a decent platform for OpenBMC
development.

This initial device-tree provides the necessary configuration for
basic BMC functionality such as host power control, serial console and
KVM support, and POST code snooping.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  | 188 ++++++++++++++++++
 1 file changed, 188 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts

diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
new file mode 100644
index 000000000000..27b34c3cf67a
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+#include "aspeed-g5.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/i2c/i2c.h>
+
+/{
+	model = "ASRock E3C246D4I BMC";
+	compatible = "aspeed,ast2500";
+
+	aliases {
+		serial4 = &uart5;
+	};
+
+	chosen {
+		stdout-path = &uart5;
+		bootargs = "console=tty0 console=ttyS4,115200 earlyprintk";
+	};
+
+	memory@80000000 {
+		reg = <0x80000000 0x20000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		heartbeat {
+			/* BMC_HB_LED_N */
+			gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "timer";
+		};
+
+		system-fault {
+			/* SYSTEM_FAULT_LED_N */
+			gpios = <&gpio ASPEED_GPIO(Z, 2) GPIO_ACTIVE_LOW>;
+			panic-indicator;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		uid-button {
+			label = "uid-button";
+			gpios = <&gpio ASPEED_GPIO(F, 1) GPIO_ACTIVE_LOW>;
+			linux,code = <ASPEED_GPIO(F, 1)>;
+		};
+	};
+
+	iio-hwmon {
+		compatible = "iio-hwmon";
+		io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, <&adc 4>,
+			<&adc 5>, <&adc 6>, <&adc 7>, <&adc 8>, <&adc 9>,
+			<&adc 10>, <&adc 11>, <&adc 12>;
+	};
+};
+
+&fmc {
+	status = "okay";
+	flash@0 {
+		status = "okay";
+		m25p,fast-read;
+		label = "bmc";
+		spi-max-frequency = <100000000>; /* 100 MHz */
+#include "openbmc-flash-layout.dtsi"
+	};
+};
+
+&uart5 {
+	status = "okay";
+};
+
+&vuart {
+	status = "okay";
+	aspeed,sirq-active-high;
+};
+
+&mac0 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>;
+};
+
+&i2c1 {
+	status = "okay";
+
+	/* thermal sensor, one diode run to a disconnected header */
+	w83773g@4c {
+		compatible = "nuvoton,w83773g";
+		reg = <0x4c>;
+	};
+};
+
+&i2c3 {
+	status = "okay";
+
+	/* FRU EEPROM */
+	eeprom@57 {
+		compatible = "st,24c128", "atmel,24c128";
+		reg = <0x57>;
+		pagesize = <16>;
+	};
+};
+
+&video {
+	status = "okay";
+};
+
+&vhub {
+	status = "okay";
+};
+
+&lpc_ctrl {
+	status = "okay";
+};
+
+&lpc_snoop {
+	status = "okay";
+	snoop-ports = <0x80>;
+};
+
+&gpio {
+	status = "okay";
+	gpio-line-names =
+		/*  A */ "BMC_MAC1_INTB", "BMC_MAC2_INTB", "NMI_BTN_N", "BMC_NMI",
+			"", "", "", "",
+		/*  B */ "", "", "", "", "", "IRQ_BMC_PCH_SMI_LPC_N", "", "",
+		/*  C */ "", "", "", "", "", "", "", "",
+		/*  D */ "BMC_PSIN", "BMC_PSOUT", "BMC_RESETCON", "RESETCON",
+			"", "", "", "",
+		/*  E */ "", "", "", "", "", "", "", "",
+		/*  F */ "LOCATORLED_STATUS_N", "LOCATORBTN", "", "",
+			"", "", "BMC_PCH_SCI_LPC", "BMC_NCSI_MUX_CTL",
+		/*  G */ "HWM_BAT_EN", "CHASSIS_ID0", "CHASSIS_ID1", "CHASSIS_ID2",
+			"BMC_ALERT1_N_R", "BMC_ALERT2_N_R", "BMC_ALERT3_N", "SML0ALERT",
+		/*  H */ "FM_ME_RCVR_N", "O_PWROK", "SKL_CNL_R", "D4_DIMM_EVENT_3V_N",
+			"MFG_MODE_N", "BMC_RTCRST", "BMC_HB_LED_N", "BMC_CASEOPEN",
+		/*  I */ "", "", "", "", "", "", "", "",
+		/*  J */ "BMC_READY", "BMC_PCH_BIOS_CS_N", "BMC_SMI", "",
+			"", "", "", "",
+		/*  K */ "", "", "", "", "", "", "", "",
+		/*  L */ "BMC_CTS1", "BMC_DCD1", "BMC_DSR1", "BMC_RI1",
+			"BMC_DTR1", "BMC_RTS1", "BMC_TXD1", "BMC_RXD1",
+		/*  M */ "BMC_LAN0_DIS_N", "BMC_LAN1_DIS_N", "", "",
+			"", "", "", "",
+		/*  N */ "", "", "", "", "", "", "", "",
+		/*  O */ "", "", "", "", "", "", "", "",
+		/*  P */ "", "", "", "", "", "", "", "",
+		/*  Q */ "", "", "", "",
+			"BMC_SBM_PRESENT_1_N", "BMC_SBM_PRESENT_2_N",
+			"BMC_SBM_PRESENT_3_N", "BMC_PCIE_WAKE_N",
+		/*  R */ "", "", "", "", "", "", "", "",
+		/*  S */ "PCHHOT_BMC_N", "", "RSMRST",
+			"", "", "", "", "",
+		/*  T */ "", "", "", "", "", "", "", "",
+		/*  U */ "", "", "", "", "", "", "", "",
+		/*  V */ "", "", "", "", "", "", "", "",
+		/*  W */ "PS_PWROK", /* dummy always-high signal */
+			"", "", "", "", "", "", "",
+		/*  X */ "", "", "", "", "", "", "", "",
+		/*  Y */ "SLP_S3", "SLP_S5", "", "", "", "", "", "",
+		/*  Z */ "CPU_CATERR_BMC_PCH_N", "", "SYSTEM_FAULT_LED_N", "BMC_THROTTLE_N",
+			"", "", "", "",
+		/* AA */ "CPU1_THERMTRIP_LATCH_N", "", "CPU1_PROCHOT_N", "",
+			"", "", "IRQ_SMI_ACTIVE_N", "FM_BIOS_POST_CMPLT_N",
+		/* AB */ "", "", "ME_OVERRIDE", "BMC_DMI_MODIFY",
+			"", "", "", "",
+		/* AC */ "LAD0", "LAD1", "LAD2", "LAD3",
+			"CK_33M_BMC", "LFRAME", "SERIRQ", "S_PLTRST";
+
+	/* Assert BMC_READY so BIOS doesn't sit around waiting for it */
+	bmc-ready {
+		gpio-hog;
+		gpios = <ASPEED_GPIO(J, 0) GPIO_ACTIVE_LOW>;
+		output-high;
+	};
+};
+
+&adc {
+	status = "okay";
+};
+
+&kcs3 {
+	status = "okay";
+	aspeed,lpc-io-reg = <0xca2>;
+};
-- 
2.31.1

Re: [PATCH 2/3] dt-bindings: serial: 8250: update for aspeed,sirq-active-high

[Rob Herring]

On Mon, Mar 29, 2021 at 07:23:37PM -0500, Zev Weiss wrote:
> Update DT bindings documentation for the new incarnation of the
> aspeed,sirq-polarity-sense property.

Why?

This isn't a compatible change.

> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
>  Documentation/devicetree/bindings/serial/8250.yaml | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> index f54cae9ff7b2..0bbb7121f720 100644
> --- a/Documentation/devicetree/bindings/serial/8250.yaml
> +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> @@ -13,7 +13,7 @@ allOf:
>    - $ref: /schemas/serial.yaml#
>    - if:
>        required:
> -        - aspeed,sirq-polarity-sense
> +        - aspeed,sirq-active-high
>      then:
>        properties:
>          compatible:
> @@ -181,13 +181,11 @@ properties:
>    rng-gpios: true
>    dcd-gpios: true
>  
> -  aspeed,sirq-polarity-sense:
> -    $ref: /schemas/types.yaml#/definitions/phandle-array
> +  aspeed,sirq-active-high:
> +    type: boolean
>      description: |
> -      Phandle to aspeed,ast2500-scu compatible syscon alongside register
> -      offset and bit number to identify how the SIRQ polarity should be
> -      configured. One possible data source is the LPC/eSPI mode bit. Only
> -      applicable to aspeed,ast2500-vuart.
> +      Set to indicate that the SIRQ polarity is active-high (default
> +      is active-low).  Only applicable to aspeed,ast2500-vuart.
>  
>  required:
>    - reg
> @@ -227,7 +225,7 @@ examples:
>          interrupts = <8>;
>          clocks = <&syscon ASPEED_CLK_APB>;
>          no-loopback-test;
> -        aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
> +        aspeed,sirq-active-high;
>      };
>  
>  ...
> -- 
> 2.31.1
> 

Re: [PATCH 2/3] dt-bindings: serial: 8250: update for aspeed,sirq-active-high

[Zev Weiss]

On Tue, Mar 30, 2021 at 05:39:02PM CDT, Rob Herring wrote:
>On Mon, Mar 29, 2021 at 07:23:37PM -0500, Zev Weiss wrote:
>> Update DT bindings documentation for the new incarnation of the
>> aspeed,sirq-polarity-sense property.
>
>Why?
>
>This isn't a compatible change.
>

Ah, sorry -- that was a misunderstanding on my end.  I'll resend a 
compatible v2 shortly.


Zev

Re: [PATCH 2/3] dt-bindings: serial: 8250: update for aspeed,sirq-active-high

[Joel Stanley]

On Tue, 30 Mar 2021 at 22:39, Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Mar 29, 2021 at 07:23:37PM -0500, Zev Weiss wrote:
> > Update DT bindings documentation for the new incarnation of the
> > aspeed,sirq-polarity-sense property.
>
> Why?
>
> This isn't a compatible change.

We want to depreciate support for this property. It should have never
been added to the bindings; in it's current form it describes a
relationship that afaict doesn't exist ("This unrelated register over
here dictates the polarity of your virtual serial port IRQ"). See
https://lore.kernel.org/lkml/20200812112400.2406734-1-joel@jms.id.au/

The intent is to remove it from both the bindings and the code.
There's already no users of it in any device tree.

How would you like Zev to go about doing this?

Cheers,

Joel

>
> >
> > Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> > ---
> >  Documentation/devicetree/bindings/serial/8250.yaml | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> > index f54cae9ff7b2..0bbb7121f720 100644
> > --- a/Documentation/devicetree/bindings/serial/8250.yaml
> > +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> > @@ -13,7 +13,7 @@ allOf:
> >    - $ref: /schemas/serial.yaml#
> >    - if:
> >        required:
> > -        - aspeed,sirq-polarity-sense
> > +        - aspeed,sirq-active-high
> >      then:
> >        properties:
> >          compatible:
> > @@ -181,13 +181,11 @@ properties:
> >    rng-gpios: true
> >    dcd-gpios: true
> >
> > -  aspeed,sirq-polarity-sense:
> > -    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +  aspeed,sirq-active-high:
> > +    type: boolean
> >      description: |
> > -      Phandle to aspeed,ast2500-scu compatible syscon alongside register
> > -      offset and bit number to identify how the SIRQ polarity should be
> > -      configured. One possible data source is the LPC/eSPI mode bit. Only
> > -      applicable to aspeed,ast2500-vuart.
> > +      Set to indicate that the SIRQ polarity is active-high (default
> > +      is active-low).  Only applicable to aspeed,ast2500-vuart.
> >
> >  required:
> >    - reg
> > @@ -227,7 +225,7 @@ examples:
> >          interrupts = <8>;
> >          clocks = <&syscon ASPEED_CLK_APB>;
> >          no-loopback-test;
> > -        aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
> > +        aspeed,sirq-active-high;
> >      };
> >
> >  ...
> > --
> > 2.31.1
> >

Re: [PATCH 3/3] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Joel Stanley]

On Tue, 30 Mar 2021 at 00:25, Zev Weiss <zev@bewilderbeest.net> wrote:
>
> This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
> mini-ITX board that we hope can provide a decent platform for OpenBMC
> development.
>
> This initial device-tree provides the necessary configuration for
> basic BMC functionality such as host power control, serial console and
> KVM support, and POST code snooping.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>

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

> ---
>  .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  | 188 ++++++++++++++++++
>  1 file changed, 188 insertions(+)
>  create mode 100644 arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
> new file mode 100644
> index 000000000000..27b34c3cf67a
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +#include "aspeed-g5.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +#include <dt-bindings/i2c/i2c.h>
> +
> +/{
> +       model = "ASRock E3C246D4I BMC";
> +       compatible = "aspeed,ast2500";

Convention is to add a compatible for the board. I'll add
asrock,e3c246d4Ii-bmc when I apply the patch.

> +&vuart {
> +       status = "okay";
> +       aspeed,sirq-active-high;

We don't have support for this yet, but I'll leave it in and you will
need to send a follow up if the property changes.

Cheers,

Joel

Re: [PATCH 3/3] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Joel Stanley]

On Wed, 31 Mar 2021 at 07:41, Joel Stanley <joel@jms.id.au> wrote:
>
> On Tue, 30 Mar 2021 at 00:25, Zev Weiss <zev@bewilderbeest.net> wrote:
> >
> > This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
> > mini-ITX board that we hope can provide a decent platform for OpenBMC
> > development.
> >
> > This initial device-tree provides the necessary configuration for
> > basic BMC functionality such as host power control, serial console and
> > KVM support, and POST code snooping.
> >
> > Signed-off-by: Zev Weiss <zev@bewilderbeest.net>

> > +&vuart {
> > +       status = "okay";
> > +       aspeed,sirq-active-high;
>
> We don't have support for this yet, but I'll leave it in and you will
> need to send a follow up if the property changes.

Oh, I missed that this was part of your series to add support for that
property. Please keep the device tree out of the series when you
re-send the vuart patches. They go through different trees, so it's
easier if you send them separately in this case.

Re: [PATCH 3/3] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Joel Stanley]

On Wed, 31 Mar 2021 at 07:41, Joel Stanley <joel@jms.id.au> wrote:
>
> On Tue, 30 Mar 2021 at 00:25, Zev Weiss <zev@bewilderbeest.net> wrote:
> >
> > This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
> > mini-ITX board that we hope can provide a decent platform for OpenBMC
> > development.
> >
> > This initial device-tree provides the necessary configuration for
> > basic BMC functionality such as host power control, serial console and
> > KVM support, and POST code snooping.
> >
> > Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
>
> > ---
> >  .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  | 188 ++++++++++++++++++
> >  1 file changed, 188 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
> >
> > diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
> > new file mode 100644
> > index 000000000000..27b34c3cf67a
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts

When adding the device tree please also add it to the makefile in
arch/arm/boot/dts.

Re: [PATCH 3/3] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Andrew Jeffery]

On Tue, 30 Mar 2021, at 10:53, Zev Weiss wrote:
> This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
> mini-ITX board that we hope can provide a decent platform for OpenBMC
> development.
> 
> This initial device-tree provides the necessary configuration for
> basic BMC functionality such as host power control, serial console and
> KVM support, and POST code snooping.
> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
>  .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  | 188 ++++++++++++++++++
>  1 file changed, 188 insertions(+)
>  create mode 100644 arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
> 
> diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts 
> b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
> new file mode 100644
> index 000000000000..27b34c3cf67a
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +#include "aspeed-g5.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +#include <dt-bindings/i2c/i2c.h>
> +
> +/{
> +	model = "ASRock E3C246D4I BMC";
> +	compatible = "aspeed,ast2500";
> +
> +	aliases {
> +		serial4 = &uart5;
> +	};
> +
> +	chosen {
> +		stdout-path = &uart5;
> +		bootargs = "console=tty0 console=ttyS4,115200 earlyprintk";
> +	};
> +
> +	memory@80000000 {
> +		reg = <0x80000000 0x20000000>;
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		heartbeat {
> +			/* BMC_HB_LED_N */
> +			gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_LOW>;
> +			linux,default-trigger = "timer";
> +		};
> +
> +		system-fault {
> +			/* SYSTEM_FAULT_LED_N */
> +			gpios = <&gpio ASPEED_GPIO(Z, 2) GPIO_ACTIVE_LOW>;
> +			panic-indicator;
> +		};
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		uid-button {
> +			label = "uid-button";
> +			gpios = <&gpio ASPEED_GPIO(F, 1) GPIO_ACTIVE_LOW>;
> +			linux,code = <ASPEED_GPIO(F, 1)>;
> +		};
> +	};
> +
> +	iio-hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, <&adc 4>,
> +			<&adc 5>, <&adc 6>, <&adc 7>, <&adc 8>, <&adc 9>,
> +			<&adc 10>, <&adc 11>, <&adc 12>;
> +	};
> +};

You're hooking up the ADC lines to the iio-hwmon bridge...
> +
> +&adc {
> +	status = "okay";
> +};

But you haven't requested the ADC lines from pinmux here.

It will *happen* to work as expected because ADC is the default mux 
state for the pins, but by not requesting the lines you're leaving the 
pins available for a conflicting request, which can be annoying to 
debug.

> +
> +&kcs3 {
> +	status = "okay";
> +	aspeed,lpc-io-reg = <0xca2>;
> +};

Given you need KCS support, do you mind testing my KCS series?

https://lore.kernel.org/linux-arm-kernel/20210319062752.145730-1-andrew@aj.id.au/

The cover letter got detached, and is here:

https://lore.kernel.org/linux-arm-kernel/20210319061952.145040-1-andrew@aj.id.au/

Andrew

Re: [PATCH 3/3] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Zev Weiss]

On Wed, Mar 31, 2021 at 07:04:51PM CDT, Andrew Jeffery wrote:
>
>
>On Tue, 30 Mar 2021, at 10:53, Zev Weiss wrote:
>> This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
>> mini-ITX board that we hope can provide a decent platform for OpenBMC
>> development.
>>
>> This initial device-tree provides the necessary configuration for
>> basic BMC functionality such as host power control, serial console and
>> KVM support, and POST code snooping.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>>  .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  | 188 ++++++++++++++++++
>>  1 file changed, 188 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>>
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>> b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>> new file mode 100644
>> index 000000000000..27b34c3cf67a
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>> @@ -0,0 +1,188 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/dts-v1/;
>> +
>> +#include "aspeed-g5.dtsi"
>> +#include <dt-bindings/gpio/aspeed-gpio.h>
>> +#include <dt-bindings/i2c/i2c.h>
>> +
>> +/{
>> +	model = "ASRock E3C246D4I BMC";
>> +	compatible = "aspeed,ast2500";
>> +
>> +	aliases {
>> +		serial4 = &uart5;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = &uart5;
>> +		bootargs = "console=tty0 console=ttyS4,115200 earlyprintk";
>> +	};
>> +
>> +	memory@80000000 {
>> +		reg = <0x80000000 0x20000000>;
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +
>> +		heartbeat {
>> +			/* BMC_HB_LED_N */
>> +			gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_LOW>;
>> +			linux,default-trigger = "timer";
>> +		};
>> +
>> +		system-fault {
>> +			/* SYSTEM_FAULT_LED_N */
>> +			gpios = <&gpio ASPEED_GPIO(Z, 2) GPIO_ACTIVE_LOW>;
>> +			panic-indicator;
>> +		};
>> +	};
>> +
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +
>> +		uid-button {
>> +			label = "uid-button";
>> +			gpios = <&gpio ASPEED_GPIO(F, 1) GPIO_ACTIVE_LOW>;
>> +			linux,code = <ASPEED_GPIO(F, 1)>;
>> +		};
>> +	};
>> +
>> +	iio-hwmon {
>> +		compatible = "iio-hwmon";
>> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, <&adc 4>,
>> +			<&adc 5>, <&adc 6>, <&adc 7>, <&adc 8>, <&adc 9>,
>> +			<&adc 10>, <&adc 11>, <&adc 12>;
>> +	};
>> +};
>
>You're hooking up the ADC lines to the iio-hwmon bridge...
>> +
>> +&adc {
>> +	status = "okay";
>> +};
>
>But you haven't requested the ADC lines from pinmux here.
>
>It will *happen* to work as expected because ADC is the default mux
>state for the pins, but by not requesting the lines you're leaving the
>pins available for a conflicting request, which can be annoying to
>debug.
>

Ack, thanks -- will fix & resend.

>> +
>> +&kcs3 {
>> +	status = "okay";
>> +	aspeed,lpc-io-reg = <0xca2>;
>> +};
>
>Given you need KCS support, do you mind testing my KCS series?
>
>https://lore.kernel.org/linux-arm-kernel/20210319062752.145730-1-andrew@aj.id.au/
>

Sure, I'll try to give that a shot and report back in the next day or 
two.

>The cover letter got detached, and is here:
>
>https://lore.kernel.org/linux-arm-kernel/20210319061952.145040-1-andrew@aj.id.au/
>
>Andrew

[PATCH v2 1/3] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[Zev Weiss]

This property ties SIRQ polarity to SCU register bits that don't
necessarily have any direct relationship to it; the only use of it
was removed in commit c82bf6e133d30e0f9172a20807814fa28aef0f67.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 Documentation/devicetree/bindings/serial/8250.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index f54cae9ff7b2..491b9297432d 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -188,6 +188,7 @@ properties:
       offset and bit number to identify how the SIRQ polarity should be
       configured. One possible data source is the LPC/eSPI mode bit. Only
       applicable to aspeed,ast2500-vuart.
+    deprecated: true
 
 required:
   - reg
-- 
2.31.1

[PATCH v2 3/3] dt-bindings: serial: 8250: add aspeed,sirq-active-high

[Zev Weiss]

This provides a simpler, more direct alternative to the deprecated
aspeed,sirq-polarity-sense property for indicating the polarity of
the Aspeed VUART's SIRQ line.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
---
 Documentation/devicetree/bindings/serial/8250.yaml | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index 491b9297432d..e79bb6ab9d2c 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -12,8 +12,9 @@ maintainers:
 allOf:
   - $ref: /schemas/serial.yaml#
   - if:
-      required:
-        - aspeed,sirq-polarity-sense
+      anyOf:
+        - required: [ aspeed,sirq-active-high ]
+        - required: [ aspeed,sirq-polarity-sense ]
     then:
       properties:
         compatible:
@@ -190,6 +191,12 @@ properties:
       applicable to aspeed,ast2500-vuart.
     deprecated: true
 
+  aspeed,sirq-active-high:
+    type: boolean
+    description: |
+      Set to indicate that the SIRQ polarity is active-high (default
+      is active-low).  Only applicable to aspeed,ast2500-vuart.
+
 required:
   - reg
   - interrupts
@@ -228,7 +235,7 @@ examples:
         interrupts = <8>;
         clocks = <&syscon ASPEED_CLK_APB>;
         no-loopback-test;
-        aspeed,sirq-polarity-sense = <&syscon 0x70 25>;
+        aspeed,sirq-active-high;
     };
 
 ...
-- 
2.31.1

[PATCH] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Zev Weiss]

This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
mini-ITX board that we hope can provide a decent platform for OpenBMC
development.

This initial device-tree provides the necessary configuration for
basic BMC functionality such as host power control, serial console and
KVM support, and POST code snooping.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/boot/dts/Makefile                    |   1 +
 .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  | 202 ++++++++++++++++++
 2 files changed, 203 insertions(+)
 create mode 100644 arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 8e5d4ab4e75e..b12911262ca1 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -1406,6 +1406,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
 	aspeed-bmc-ampere-mtjade.dtb \
 	aspeed-bmc-arm-centriq2400-rep.dtb \
 	aspeed-bmc-arm-stardragon4800-rep2.dtb \
+	aspeed-bmc-asrock-e3c246d4i.dts \
 	aspeed-bmc-bytedance-g220a.dtb \
 	aspeed-bmc-facebook-cmm.dtb \
 	aspeed-bmc-facebook-galaxy100.dtb \
diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
new file mode 100644
index 000000000000..dcab6e78dfa4
--- /dev/null
+++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
@@ -0,0 +1,202 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+#include "aspeed-g5.dtsi"
+#include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/i2c/i2c.h>
+
+/{
+	model = "ASRock E3C246D4I BMC";
+	compatible = "asrock,e3c246d4i-bmc", "aspeed,ast2500";
+
+	aliases {
+		serial4 = &uart5;
+	};
+
+	chosen {
+		stdout-path = &uart5;
+		bootargs = "console=tty0 console=ttyS4,115200 earlyprintk";
+	};
+
+	memory@80000000 {
+		reg = <0x80000000 0x20000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		heartbeat {
+			/* BMC_HB_LED_N */
+			gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "timer";
+		};
+
+		system-fault {
+			/* SYSTEM_FAULT_LED_N */
+			gpios = <&gpio ASPEED_GPIO(Z, 2) GPIO_ACTIVE_LOW>;
+			panic-indicator;
+		};
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		uid-button {
+			label = "uid-button";
+			gpios = <&gpio ASPEED_GPIO(F, 1) GPIO_ACTIVE_LOW>;
+			linux,code = <ASPEED_GPIO(F, 1)>;
+		};
+	};
+
+	iio-hwmon {
+		compatible = "iio-hwmon";
+		io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, <&adc 4>,
+			<&adc 5>, <&adc 6>, <&adc 7>, <&adc 8>, <&adc 9>,
+			<&adc 10>, <&adc 11>, <&adc 12>;
+	};
+};
+
+&fmc {
+	status = "okay";
+	flash@0 {
+		status = "okay";
+		m25p,fast-read;
+		label = "bmc";
+		spi-max-frequency = <100000000>; /* 100 MHz */
+#include "openbmc-flash-layout.dtsi"
+	};
+};
+
+&uart5 {
+	status = "okay";
+};
+
+&vuart {
+	status = "okay";
+	aspeed,sirq-active-high;
+};
+
+&mac0 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>;
+};
+
+&i2c1 {
+	status = "okay";
+
+	/* thermal sensor, one diode run to a disconnected header */
+	w83773g@4c {
+		compatible = "nuvoton,w83773g";
+		reg = <0x4c>;
+	};
+};
+
+&i2c3 {
+	status = "okay";
+
+	/* FRU EEPROM */
+	eeprom@57 {
+		compatible = "st,24c128", "atmel,24c128";
+		reg = <0x57>;
+		pagesize = <16>;
+	};
+};
+
+&video {
+	status = "okay";
+};
+
+&vhub {
+	status = "okay";
+};
+
+&lpc_ctrl {
+	status = "okay";
+};
+
+&lpc_snoop {
+	status = "okay";
+	snoop-ports = <0x80>;
+};
+
+&gpio {
+	status = "okay";
+	gpio-line-names =
+		/*  A */ "BMC_MAC1_INTB", "BMC_MAC2_INTB", "NMI_BTN_N", "BMC_NMI",
+			"", "", "", "",
+		/*  B */ "", "", "", "", "", "IRQ_BMC_PCH_SMI_LPC_N", "", "",
+		/*  C */ "", "", "", "", "", "", "", "",
+		/*  D */ "BMC_PSIN", "BMC_PSOUT", "BMC_RESETCON", "RESETCON",
+			"", "", "", "",
+		/*  E */ "", "", "", "", "", "", "", "",
+		/*  F */ "LOCATORLED_STATUS_N", "LOCATORBTN", "", "",
+			"", "", "BMC_PCH_SCI_LPC", "BMC_NCSI_MUX_CTL",
+		/*  G */ "HWM_BAT_EN", "CHASSIS_ID0", "CHASSIS_ID1", "CHASSIS_ID2",
+			"BMC_ALERT1_N_R", "BMC_ALERT2_N_R", "BMC_ALERT3_N", "SML0ALERT",
+		/*  H */ "FM_ME_RCVR_N", "O_PWROK", "SKL_CNL_R", "D4_DIMM_EVENT_3V_N",
+			"MFG_MODE_N", "BMC_RTCRST", "BMC_HB_LED_N", "BMC_CASEOPEN",
+		/*  I */ "", "", "", "", "", "", "", "",
+		/*  J */ "BMC_READY", "BMC_PCH_BIOS_CS_N", "BMC_SMI", "",
+			"", "", "", "",
+		/*  K */ "", "", "", "", "", "", "", "",
+		/*  L */ "BMC_CTS1", "BMC_DCD1", "BMC_DSR1", "BMC_RI1",
+			"BMC_DTR1", "BMC_RTS1", "BMC_TXD1", "BMC_RXD1",
+		/*  M */ "BMC_LAN0_DIS_N", "BMC_LAN1_DIS_N", "", "",
+			"", "", "", "",
+		/*  N */ "", "", "", "", "", "", "", "",
+		/*  O */ "", "", "", "", "", "", "", "",
+		/*  P */ "", "", "", "", "", "", "", "",
+		/*  Q */ "", "", "", "",
+			"BMC_SBM_PRESENT_1_N", "BMC_SBM_PRESENT_2_N",
+			"BMC_SBM_PRESENT_3_N", "BMC_PCIE_WAKE_N",
+		/*  R */ "", "", "", "", "", "", "", "",
+		/*  S */ "PCHHOT_BMC_N", "", "RSMRST",
+			"", "", "", "", "",
+		/*  T */ "", "", "", "", "", "", "", "",
+		/*  U */ "", "", "", "", "", "", "", "",
+		/*  V */ "", "", "", "", "", "", "", "",
+		/*  W */ "PS_PWROK", /* dummy always-high signal */
+			"", "", "", "", "", "", "",
+		/*  X */ "", "", "", "", "", "", "", "",
+		/*  Y */ "SLP_S3", "SLP_S5", "", "", "", "", "", "",
+		/*  Z */ "CPU_CATERR_BMC_PCH_N", "", "SYSTEM_FAULT_LED_N", "BMC_THROTTLE_N",
+			"", "", "", "",
+		/* AA */ "CPU1_THERMTRIP_LATCH_N", "", "CPU1_PROCHOT_N", "",
+			"", "", "IRQ_SMI_ACTIVE_N", "FM_BIOS_POST_CMPLT_N",
+		/* AB */ "", "", "ME_OVERRIDE", "BMC_DMI_MODIFY",
+			"", "", "", "",
+		/* AC */ "LAD0", "LAD1", "LAD2", "LAD3",
+			"CK_33M_BMC", "LFRAME", "SERIRQ", "S_PLTRST";
+
+	/* Assert BMC_READY so BIOS doesn't sit around waiting for it */
+	bmc-ready {
+		gpio-hog;
+		gpios = <ASPEED_GPIO(J, 0) GPIO_ACTIVE_LOW>;
+		output-high;
+	};
+};
+
+&adc {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_adc0_default
+			&pinctrl_adc1_default
+			&pinctrl_adc2_default
+			&pinctrl_adc3_default
+			&pinctrl_adc4_default
+			&pinctrl_adc5_default
+			&pinctrl_adc6_default
+			&pinctrl_adc7_default
+			&pinctrl_adc8_default
+			&pinctrl_adc9_default
+			&pinctrl_adc10_default
+			&pinctrl_adc11_default
+			&pinctrl_adc12_default>;
+};
+
+&kcs3 {
+	status = "okay";
+	aspeed,lpc-io-reg = <0xca2>;
+};
-- 
2.31.1

Re: [PATCH] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Joel Stanley]

Hi Zev,

On Thu, 1 Apr 2021 at 02:57, Zev Weiss <zev@bewilderbeest.net> wrote:
>
> This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
> mini-ITX board that we hope can provide a decent platform for OpenBMC
> development.
>
> This initial device-tree provides the necessary configuration for
> basic BMC functionality such as host power control, serial console and
> KVM support, and POST code snooping.

The patch looks good! Some minor things below.

When sending subsequent versions, make sure to add -v N to your git
format-patch to mark it as the Nth version.

You've also set this to be threaded with a previous version of the
patch. We normally don't do that, and in this case it's doubly
confusing as you've split this patch out from the previous series.

I noticed you cc'd soc@kernel.org. We normally only do this when we
want the soc maintainers to apply a patch directly without going
through another maintainer. In this case the patch should go through
the aspeed maintainer's tree (me), so you don't need to cc that
address.

> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---

This spot just here is where you should put the changes between v1 and v2.

>  arch/arm/boot/dts/Makefile                    |   1 +
>  .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  | 202 ++++++++++++++++++
>  2 files changed, 203 insertions(+)
>  create mode 100644 arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 8e5d4ab4e75e..b12911262ca1 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1406,6 +1406,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>         aspeed-bmc-ampere-mtjade.dtb \
>         aspeed-bmc-arm-centriq2400-rep.dtb \
>         aspeed-bmc-arm-stardragon4800-rep2.dtb \
> +       aspeed-bmc-asrock-e3c246d4i.dts \

This should be the output name (.dtb).

>         aspeed-bmc-bytedance-g220a.dtb \
>         aspeed-bmc-facebook-cmm.dtb \
>         aspeed-bmc-facebook-galaxy100.dtb \
> diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
> new file mode 100644
> index 000000000000..dcab6e78dfa4
> --- /dev/null
> +++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts

The device tree itself looks good!

If you fix up the things I mentioned and send a v3 I will apply it.

Cheers,

Joel

> @@ -0,0 +1,202 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +#include "aspeed-g5.dtsi"
> +#include <dt-bindings/gpio/aspeed-gpio.h>
> +#include <dt-bindings/i2c/i2c.h>
> +
> +/{
> +       model = "ASRock E3C246D4I BMC";
> +       compatible = "asrock,e3c246d4i-bmc", "aspeed,ast2500";
> +
> +       aliases {
> +               serial4 = &uart5;
> +       };
> +
> +       chosen {
> +               stdout-path = &uart5;
> +               bootargs = "console=tty0 console=ttyS4,115200 earlyprintk";
> +       };
> +
> +       memory@80000000 {
> +               reg = <0x80000000 0x20000000>;
> +       };
> +
> +       leds {
> +               compatible = "gpio-leds";
> +
> +               heartbeat {
> +                       /* BMC_HB_LED_N */
> +                       gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_LOW>;
> +                       linux,default-trigger = "timer";
> +               };
> +
> +               system-fault {
> +                       /* SYSTEM_FAULT_LED_N */
> +                       gpios = <&gpio ASPEED_GPIO(Z, 2) GPIO_ACTIVE_LOW>;
> +                       panic-indicator;
> +               };
> +       };
> +
> +       gpio-keys {
> +               compatible = "gpio-keys";
> +
> +               uid-button {
> +                       label = "uid-button";
> +                       gpios = <&gpio ASPEED_GPIO(F, 1) GPIO_ACTIVE_LOW>;
> +                       linux,code = <ASPEED_GPIO(F, 1)>;
> +               };
> +       };
> +
> +       iio-hwmon {
> +               compatible = "iio-hwmon";
> +               io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, <&adc 4>,
> +                       <&adc 5>, <&adc 6>, <&adc 7>, <&adc 8>, <&adc 9>,
> +                       <&adc 10>, <&adc 11>, <&adc 12>;
> +       };
> +};
> +
> +&fmc {
> +       status = "okay";
> +       flash@0 {
> +               status = "okay";
> +               m25p,fast-read;
> +               label = "bmc";
> +               spi-max-frequency = <100000000>; /* 100 MHz */
> +#include "openbmc-flash-layout.dtsi"
> +       };
> +};
> +
> +&uart5 {
> +       status = "okay";
> +};
> +
> +&vuart {
> +       status = "okay";
> +       aspeed,sirq-active-high;
> +};
> +
> +&mac0 {
> +       status = "okay";
> +
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>;
> +};
> +
> +&i2c1 {
> +       status = "okay";
> +
> +       /* thermal sensor, one diode run to a disconnected header */
> +       w83773g@4c {
> +               compatible = "nuvoton,w83773g";
> +               reg = <0x4c>;
> +       };
> +};
> +
> +&i2c3 {
> +       status = "okay";
> +
> +       /* FRU EEPROM */
> +       eeprom@57 {
> +               compatible = "st,24c128", "atmel,24c128";
> +               reg = <0x57>;
> +               pagesize = <16>;
> +       };
> +};
> +
> +&video {
> +       status = "okay";
> +};
> +
> +&vhub {
> +       status = "okay";
> +};
> +
> +&lpc_ctrl {
> +       status = "okay";
> +};
> +
> +&lpc_snoop {
> +       status = "okay";
> +       snoop-ports = <0x80>;
> +};
> +
> +&gpio {
> +       status = "okay";
> +       gpio-line-names =
> +               /*  A */ "BMC_MAC1_INTB", "BMC_MAC2_INTB", "NMI_BTN_N", "BMC_NMI",
> +                       "", "", "", "",
> +               /*  B */ "", "", "", "", "", "IRQ_BMC_PCH_SMI_LPC_N", "", "",
> +               /*  C */ "", "", "", "", "", "", "", "",
> +               /*  D */ "BMC_PSIN", "BMC_PSOUT", "BMC_RESETCON", "RESETCON",
> +                       "", "", "", "",
> +               /*  E */ "", "", "", "", "", "", "", "",
> +               /*  F */ "LOCATORLED_STATUS_N", "LOCATORBTN", "", "",
> +                       "", "", "BMC_PCH_SCI_LPC", "BMC_NCSI_MUX_CTL",
> +               /*  G */ "HWM_BAT_EN", "CHASSIS_ID0", "CHASSIS_ID1", "CHASSIS_ID2",
> +                       "BMC_ALERT1_N_R", "BMC_ALERT2_N_R", "BMC_ALERT3_N", "SML0ALERT",
> +               /*  H */ "FM_ME_RCVR_N", "O_PWROK", "SKL_CNL_R", "D4_DIMM_EVENT_3V_N",
> +                       "MFG_MODE_N", "BMC_RTCRST", "BMC_HB_LED_N", "BMC_CASEOPEN",
> +               /*  I */ "", "", "", "", "", "", "", "",
> +               /*  J */ "BMC_READY", "BMC_PCH_BIOS_CS_N", "BMC_SMI", "",
> +                       "", "", "", "",
> +               /*  K */ "", "", "", "", "", "", "", "",
> +               /*  L */ "BMC_CTS1", "BMC_DCD1", "BMC_DSR1", "BMC_RI1",
> +                       "BMC_DTR1", "BMC_RTS1", "BMC_TXD1", "BMC_RXD1",
> +               /*  M */ "BMC_LAN0_DIS_N", "BMC_LAN1_DIS_N", "", "",
> +                       "", "", "", "",
> +               /*  N */ "", "", "", "", "", "", "", "",
> +               /*  O */ "", "", "", "", "", "", "", "",
> +               /*  P */ "", "", "", "", "", "", "", "",
> +               /*  Q */ "", "", "", "",
> +                       "BMC_SBM_PRESENT_1_N", "BMC_SBM_PRESENT_2_N",
> +                       "BMC_SBM_PRESENT_3_N", "BMC_PCIE_WAKE_N",
> +               /*  R */ "", "", "", "", "", "", "", "",
> +               /*  S */ "PCHHOT_BMC_N", "", "RSMRST",
> +                       "", "", "", "", "",
> +               /*  T */ "", "", "", "", "", "", "", "",
> +               /*  U */ "", "", "", "", "", "", "", "",
> +               /*  V */ "", "", "", "", "", "", "", "",
> +               /*  W */ "PS_PWROK", /* dummy always-high signal */
> +                       "", "", "", "", "", "", "",
> +               /*  X */ "", "", "", "", "", "", "", "",
> +               /*  Y */ "SLP_S3", "SLP_S5", "", "", "", "", "", "",
> +               /*  Z */ "CPU_CATERR_BMC_PCH_N", "", "SYSTEM_FAULT_LED_N", "BMC_THROTTLE_N",
> +                       "", "", "", "",
> +               /* AA */ "CPU1_THERMTRIP_LATCH_N", "", "CPU1_PROCHOT_N", "",
> +                       "", "", "IRQ_SMI_ACTIVE_N", "FM_BIOS_POST_CMPLT_N",
> +               /* AB */ "", "", "ME_OVERRIDE", "BMC_DMI_MODIFY",
> +                       "", "", "", "",
> +               /* AC */ "LAD0", "LAD1", "LAD2", "LAD3",
> +                       "CK_33M_BMC", "LFRAME", "SERIRQ", "S_PLTRST";
> +
> +       /* Assert BMC_READY so BIOS doesn't sit around waiting for it */
> +       bmc-ready {
> +               gpio-hog;
> +               gpios = <ASPEED_GPIO(J, 0) GPIO_ACTIVE_LOW>;
> +               output-high;
> +       };
> +};
> +
> +&adc {
> +       status = "okay";
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pinctrl_adc0_default
> +                       &pinctrl_adc1_default
> +                       &pinctrl_adc2_default
> +                       &pinctrl_adc3_default
> +                       &pinctrl_adc4_default
> +                       &pinctrl_adc5_default
> +                       &pinctrl_adc6_default
> +                       &pinctrl_adc7_default
> +                       &pinctrl_adc8_default
> +                       &pinctrl_adc9_default
> +                       &pinctrl_adc10_default
> +                       &pinctrl_adc11_default
> +                       &pinctrl_adc12_default>;
> +};
> +
> +&kcs3 {
> +       status = "okay";
> +       aspeed,lpc-io-reg = <0xca2>;
> +};
> --
> 2.31.1
>

Re: [PATCH v2 1/3] dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense

[Joel Stanley]

On Thu, 1 Apr 2021 at 00:57, Zev Weiss <zev@bewilderbeest.net> wrote:
>
> This property ties SIRQ polarity to SCU register bits that don't
> necessarily have any direct relationship to it; the only use of it
> was removed in commit c82bf6e133d30e0f9172a20807814fa28aef0f67.
>
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>

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

> ---
>  Documentation/devicetree/bindings/serial/8250.yaml | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
> index f54cae9ff7b2..491b9297432d 100644
> --- a/Documentation/devicetree/bindings/serial/8250.yaml
> +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> @@ -188,6 +188,7 @@ properties:
>        offset and bit number to identify how the SIRQ polarity should be
>        configured. One possible data source is the LPC/eSPI mode bit. Only
>        applicable to aspeed,ast2500-vuart.
> +    deprecated: true
>
>  required:
>    - reg
> --
> 2.31.1
>

Re: [PATCH v2 3/3] dt-bindings: serial: 8250: add aspeed,sirq-active-high

[Andrew Jeffery]

On Thu, 1 Apr 2021, at 11:27, Zev Weiss wrote:
> This provides a simpler, more direct alternative to the deprecated
> aspeed,sirq-polarity-sense property for indicating the polarity of
> the Aspeed VUART's SIRQ line.
> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
>  Documentation/devicetree/bindings/serial/8250.yaml | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml 
> b/Documentation/devicetree/bindings/serial/8250.yaml
> index 491b9297432d..e79bb6ab9d2c 100644
> --- a/Documentation/devicetree/bindings/serial/8250.yaml
> +++ b/Documentation/devicetree/bindings/serial/8250.yaml
> @@ -12,8 +12,9 @@ maintainers:
>  allOf:
>    - $ref: /schemas/serial.yaml#
>    - if:
> -      required:
> -        - aspeed,sirq-polarity-sense
> +      anyOf:
> +        - required: [ aspeed,sirq-active-high ]

Do you think we could make use of the approach I put forward here?

https://lore.kernel.org/openbmc/20210319062752.145730-18-andrew@aj.id.au/T/#u

Andrew

Re: [PATCH] ARM: dts: aspeed: add ASRock E3C246D4I BMC

[Zev Weiss]

On Wed, Mar 31, 2021 at 10:51:42PM CDT, Joel Stanley wrote:
>Hi Zev,
>
>On Thu, 1 Apr 2021 at 02:57, Zev Weiss <zev@bewilderbeest.net> wrote:
>>
>> This is a relatively low-cost AST2500-based Xeon E-2100/E-2200 series
>> mini-ITX board that we hope can provide a decent platform for OpenBMC
>> development.
>>
>> This initial device-tree provides the necessary configuration for
>> basic BMC functionality such as host power control, serial console and
>> KVM support, and POST code snooping.
>
>The patch looks good! Some minor things below.
>
>When sending subsequent versions, make sure to add -v N to your git
>format-patch to mark it as the Nth version.
>
>You've also set this to be threaded with a previous version of the
>patch. We normally don't do that, and in this case it's doubly
>confusing as you've split this patch out from the previous series.
>
>I noticed you cc'd soc@kernel.org. We normally only do this when we
>want the soc maintainers to apply a patch directly without going
>through another maintainer. In this case the patch should go through
>the aspeed maintainer's tree (me), so you don't need to cc that
>address.
>

Hmm, that came from using './scripts/get_maintainer.pl --no-rolestats' 
with git send-email's --cc-cmd flag; does there happen to be a similarly 
easy alternative that wouldn't do the "wrong" thing there?

Ack on the rest, will send v3 soon.


Thanks,
Zev

>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>> ---
>
>This spot just here is where you should put the changes between v1 and v2.
>
>>  arch/arm/boot/dts/Makefile                    |   1 +
>>  .../boot/dts/aspeed-bmc-asrock-e3c246d4i.dts  | 202 ++++++++++++++++++
>>  2 files changed, 203 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 8e5d4ab4e75e..b12911262ca1 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -1406,6 +1406,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>>         aspeed-bmc-ampere-mtjade.dtb \
>>         aspeed-bmc-arm-centriq2400-rep.dtb \
>>         aspeed-bmc-arm-stardragon4800-rep2.dtb \
>> +       aspeed-bmc-asrock-e3c246d4i.dts \
>
>This should be the output name (.dtb).
>
>>         aspeed-bmc-bytedance-g220a.dtb \
>>         aspeed-bmc-facebook-cmm.dtb \
>>         aspeed-bmc-facebook-galaxy100.dtb \
>> diff --git a/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>> new file mode 100644
>> index 000000000000..dcab6e78dfa4
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/aspeed-bmc-asrock-e3c246d4i.dts
>
>The device tree itself looks good!
>
>If you fix up the things I mentioned and send a v3 I will apply it.
>
>Cheers,
>
>Joel
>
>> @@ -0,0 +1,202 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/dts-v1/;
>> +
>> +#include "aspeed-g5.dtsi"
>> +#include <dt-bindings/gpio/aspeed-gpio.h>
>> +#include <dt-bindings/i2c/i2c.h>
>> +
>> +/{
>> +       model = "ASRock E3C246D4I BMC";
>> +       compatible = "asrock,e3c246d4i-bmc", "aspeed,ast2500";
>> +
>> +       aliases {
>> +               serial4 = &uart5;
>> +       };
>> +
>> +       chosen {
>> +               stdout-path = &uart5;
>> +               bootargs = "console=tty0 console=ttyS4,115200 earlyprintk";
>> +       };
>> +
>> +       memory@80000000 {
>> +               reg = <0x80000000 0x20000000>;
>> +       };
>> +
>> +       leds {
>> +               compatible = "gpio-leds";
>> +
>> +               heartbeat {
>> +                       /* BMC_HB_LED_N */
>> +                       gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_LOW>;
>> +                       linux,default-trigger = "timer";
>> +               };
>> +
>> +               system-fault {
>> +                       /* SYSTEM_FAULT_LED_N */
>> +                       gpios = <&gpio ASPEED_GPIO(Z, 2) GPIO_ACTIVE_LOW>;
>> +                       panic-indicator;
>> +               };
>> +       };
>> +
>> +       gpio-keys {
>> +               compatible = "gpio-keys";
>> +
>> +               uid-button {
>> +                       label = "uid-button";
>> +                       gpios = <&gpio ASPEED_GPIO(F, 1) GPIO_ACTIVE_LOW>;
>> +                       linux,code = <ASPEED_GPIO(F, 1)>;
>> +               };
>> +       };
>> +
>> +       iio-hwmon {
>> +               compatible = "iio-hwmon";
>> +               io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>, <&adc 4>,
>> +                       <&adc 5>, <&adc 6>, <&adc 7>, <&adc 8>, <&adc 9>,
>> +                       <&adc 10>, <&adc 11>, <&adc 12>;
>> +       };
>> +};
>> +
>> +&fmc {
>> +       status = "okay";
>> +       flash@0 {
>> +               status = "okay";
>> +               m25p,fast-read;
>> +               label = "bmc";
>> +               spi-max-frequency = <100000000>; /* 100 MHz */
>> +#include "openbmc-flash-layout.dtsi"
>> +       };
>> +};
>> +
>> +&uart5 {
>> +       status = "okay";
>> +};
>> +
>> +&vuart {
>> +       status = "okay";
>> +       aspeed,sirq-active-high;
>> +};
>> +
>> +&mac0 {
>> +       status = "okay";
>> +
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&pinctrl_rgmii1_default &pinctrl_mdio1_default>;
>> +};
>> +
>> +&i2c1 {
>> +       status = "okay";
>> +
>> +       /* thermal sensor, one diode run to a disconnected header */
>> +       w83773g@4c {
>> +               compatible = "nuvoton,w83773g";
>> +               reg = <0x4c>;
>> +       };
>> +};
>> +
>> +&i2c3 {
>> +       status = "okay";
>> +
>> +       /* FRU EEPROM */
>> +       eeprom@57 {
>> +               compatible = "st,24c128", "atmel,24c128";
>> +               reg = <0x57>;
>> +               pagesize = <16>;
>> +       };
>> +};
>> +
>> +&video {
>> +       status = "okay";
>> +};
>> +
>> +&vhub {
>> +       status = "okay";
>> +};
>> +
>> +&lpc_ctrl {
>> +       status = "okay";
>> +};
>> +
>> +&lpc_snoop {
>> +       status = "okay";
>> +       snoop-ports = <0x80>;
>> +};
>> +
>> +&gpio {
>> +       status = "okay";
>> +       gpio-line-names =
>> +               /*  A */ "BMC_MAC1_INTB", "BMC_MAC2_INTB", "NMI_BTN_N", "BMC_NMI",
>> +                       "", "", "", "",
>> +               /*  B */ "", "", "", "", "", "IRQ_BMC_PCH_SMI_LPC_N", "", "",
>> +               /*  C */ "", "", "", "", "", "", "", "",
>> +               /*  D */ "BMC_PSIN", "BMC_PSOUT", "BMC_RESETCON", "RESETCON",
>> +                       "", "", "", "",
>> +               /*  E */ "", "", "", "", "", "", "", "",
>> +               /*  F */ "LOCATORLED_STATUS_N", "LOCATORBTN", "", "",
>> +                       "", "", "BMC_PCH_SCI_LPC", "BMC_NCSI_MUX_CTL",
>> +               /*  G */ "HWM_BAT_EN", "CHASSIS_ID0", "CHASSIS_ID1", "CHASSIS_ID2",
>> +                       "BMC_ALERT1_N_R", "BMC_ALERT2_N_R", "BMC_ALERT3_N", "SML0ALERT",
>> +               /*  H */ "FM_ME_RCVR_N", "O_PWROK", "SKL_CNL_R", "D4_DIMM_EVENT_3V_N",
>> +                       "MFG_MODE_N", "BMC_RTCRST", "BMC_HB_LED_N", "BMC_CASEOPEN",
>> +               /*  I */ "", "", "", "", "", "", "", "",
>> +               /*  J */ "BMC_READY", "BMC_PCH_BIOS_CS_N", "BMC_SMI", "",
>> +                       "", "", "", "",
>> +               /*  K */ "", "", "", "", "", "", "", "",
>> +               /*  L */ "BMC_CTS1", "BMC_DCD1", "BMC_DSR1", "BMC_RI1",
>> +                       "BMC_DTR1", "BMC_RTS1", "BMC_TXD1", "BMC_RXD1",
>> +               /*  M */ "BMC_LAN0_DIS_N", "BMC_LAN1_DIS_N", "", "",
>> +                       "", "", "", "",
>> +               /*  N */ "", "", "", "", "", "", "", "",
>> +               /*  O */ "", "", "", "", "", "", "", "",
>> +               /*  P */ "", "", "", "", "", "", "", "",
>> +               /*  Q */ "", "", "", "",
>> +                       "BMC_SBM_PRESENT_1_N", "BMC_SBM_PRESENT_2_N",
>> +                       "BMC_SBM_PRESENT_3_N", "BMC_PCIE_WAKE_N",
>> +               /*  R */ "", "", "", "", "", "", "", "",
>> +               /*  S */ "PCHHOT_BMC_N", "", "RSMRST",
>> +                       "", "", "", "", "",
>> +               /*  T */ "", "", "", "", "", "", "", "",
>> +               /*  U */ "", "", "", "", "", "", "", "",
>> +               /*  V */ "", "", "", "", "", "", "", "",
>> +               /*  W */ "PS_PWROK", /* dummy always-high signal */
>> +                       "", "", "", "", "", "", "",
>> +               /*  X */ "", "", "", "", "", "", "", "",
>> +               /*  Y */ "SLP_S3", "SLP_S5", "", "", "", "", "", "",
>> +               /*  Z */ "CPU_CATERR_BMC_PCH_N", "", "SYSTEM_FAULT_LED_N", "BMC_THROTTLE_N",
>> +                       "", "", "", "",
>> +               /* AA */ "CPU1_THERMTRIP_LATCH_N", "", "CPU1_PROCHOT_N", "",
>> +                       "", "", "IRQ_SMI_ACTIVE_N", "FM_BIOS_POST_CMPLT_N",
>> +               /* AB */ "", "", "ME_OVERRIDE", "BMC_DMI_MODIFY",
>> +                       "", "", "", "",
>> +               /* AC */ "LAD0", "LAD1", "LAD2", "LAD3",
>> +                       "CK_33M_BMC", "LFRAME", "SERIRQ", "S_PLTRST";
>> +
>> +       /* Assert BMC_READY so BIOS doesn't sit around waiting for it */
>> +       bmc-ready {
>> +               gpio-hog;
>> +               gpios = <ASPEED_GPIO(J, 0) GPIO_ACTIVE_LOW>;
>> +               output-high;
>> +       };
>> +};
>> +
>> +&adc {
>> +       status = "okay";
>> +       pinctrl-names = "default";
>> +       pinctrl-0 = <&pinctrl_adc0_default
>> +                       &pinctrl_adc1_default
>> +                       &pinctrl_adc2_default
>> +                       &pinctrl_adc3_default
>> +                       &pinctrl_adc4_default
>> +                       &pinctrl_adc5_default
>> +                       &pinctrl_adc6_default
>> +                       &pinctrl_adc7_default
>> +                       &pinctrl_adc8_default
>> +                       &pinctrl_adc9_default
>> +                       &pinctrl_adc10_default
>> +                       &pinctrl_adc11_default
>> +                       &pinctrl_adc12_default>;
>> +};
>> +
>> +&kcs3 {
>> +       status = "okay";
>> +       aspeed,lpc-io-reg = <0xca2>;
>> +};
>> --
>> 2.31.1
>>

Re: [PATCH v2 3/3] dt-bindings: serial: 8250: add aspeed,sirq-active-high

[Zev Weiss]

On Wed, Mar 31, 2021 at 11:04:44PM CDT, Andrew Jeffery wrote:
>
>
>On Thu, 1 Apr 2021, at 11:27, Zev Weiss wrote:
>> This provides a simpler, more direct alternative to the deprecated
>> aspeed,sirq-polarity-sense property for indicating the polarity of
>> the Aspeed VUART's SIRQ line.
>>
>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
>> ---
>>  Documentation/devicetree/bindings/serial/8250.yaml | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/8250.yaml
>> b/Documentation/devicetree/bindings/serial/8250.yaml
>> index 491b9297432d..e79bb6ab9d2c 100644
>> --- a/Documentation/devicetree/bindings/serial/8250.yaml
>> +++ b/Documentation/devicetree/bindings/serial/8250.yaml
>> @@ -12,8 +12,9 @@ maintainers:
>>  allOf:
>>    - $ref: /schemas/serial.yaml#
>>    - if:
>> -      required:
>> -        - aspeed,sirq-polarity-sense
>> +      anyOf:
>> +        - required: [ aspeed,sirq-active-high ]
>
>Do you think we could make use of the approach I put forward here?
>
>https://lore.kernel.org/openbmc/20210319062752.145730-18-andrew@aj.id.au/T/#u
>
>Andrew

If you mean using a u32 property (say aspeed,sirq-polarity) with an 
explicit IRQ_TYPE_LEVEL_{LOW,HIGH} instead of a present/absent bool,
sure, I guess that seems like a somewhat clearer, more orthogonal 
arrangement.


Zev

Re: [PATCH v2 3/3] dt-bindings: serial: 8250: add aspeed, sirq-active-high

[Rob Herring]

On Wed, 31 Mar 2021 19:57:02 -0500, Zev Weiss wrote:
> This provides a simpler, more direct alternative to the deprecated
> aspeed,sirq-polarity-sense property for indicating the polarity of
> the Aspeed VUART's SIRQ line.
> 
> Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
> ---
>  Documentation/devicetree/bindings/serial/8250.yaml | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/serial/8250.yaml:16:30: [warning] too few spaces after comma (commas)
./Documentation/devicetree/bindings/serial/8250.yaml:17:30: [warning] too few spaces after comma (commas)

dtschema/dtc warnings/errors:

See https://patchwork.ozlabs.org/patch/1460791

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.