Skip to contentSkip to conversation

input: misc: Add IBM Operation Panel driver

PATCH Applied Eddie James 5 participants 14 messages Sep 9, 2020 – Sep 13, 2020

Subsystems (7)
lore
Share & tools
lore

Eddie James

author maintainer
#0 lore raw

v3 cover input: misc: Add IBM Operation Panel driver

This series adds support for input from the IBM Operation Panel, which is a simple controller with three buttons and an LCD display meant for interacting with a server. It's connected over I2C, typically to a service processor. This series only supports the input from the panel, in which the panel masters the I2C bus and sends data to the host system when someone presses a button on the controller.

Changes since v2:
- Add "additionalProperties: false" to dts doc
- Refactor switch statement in the input driver; check command size and call the processing function within the STOP case
- Use a different definition name for Aspeed interrupt status mask

Changes since v1:
- Redo DTS documentation example to use I2C_OWN_SLAVE_ADDRESS
- Reject commands received in the input driver that are too long
- Add a definition for the interrupt status mask in the Aspeed I2C driver
- Use I2C_OWN_SLAVE_ADDRESS for both dts additions

Eddie James (5):
dt-bindings: input: Add documentation for IBM Operation Panel
input: misc: Add IBM Operation Panel driver
i2c: aspeed: Mask IRQ status to relevant bits
ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device
ARM: dts: Aspeed: Rainier: Add IBM Operation Panel I2C device

 .../bindings/input/ibm,op-panel.yaml          |  41 ++++
 MAINTAINERS                                   |   7 +
 arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts  |   7 +
 arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts   |   7 +
 drivers/i2c/busses/i2c-aspeed.c               |   2 +
 drivers/input/misc/Kconfig                    |  18 ++
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/ibm-panel.c                | 189 ++++++++++++++++++
 8 files changed, 272 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/ibm,op-panel.yaml
 create mode 100644 drivers/input/misc/ibm-panel.c
Signature · 1 line
2.26.2
author maintainer
#1 lore raw

PATCH v3 1/5 dt-bindings: input: Add documentation for IBM Operation Panel

Document the bindings for the IBM Operation Panel, which provides a simple interface to control a server. It has a display and three buttons.
Also update MAINTAINERS for the new file.

---
 .../bindings/input/ibm,op-panel.yaml          | 41 +++++++++++++++++++
 MAINTAINERS                                   |  6 +++
 2 files changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/ibm,op-panel.yaml
Documentation/devicetree/bindings/input/ibm,op-panel.yaml new file +41 -0
new file mode 100644
index 000000000000..52c4a6275a77
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/ibm,op-panel.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IBM Operation Panel
+
+maintainers:
+  - Eddie James <[email protected]>
+
+description: |
+  The IBM Operation Panel provides a simple interface to control the connected
+  server. It has a display and three buttons: two directional arrows and one
+  'Enter' button.
+
+properties:
+  compatible:
+    const: ibm,op-panel
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/i2c/i2c.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ibm-op-panel at 62 {
+            compatible = "ibm,op-panel";
+            reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+        };
+    };
MAINTAINERS +6 -0
index 3c0692404907..28408d29d873 100644
@@ -8346,6 +8346,12 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/aegl/linux.git
 F:	Documentation/ia64/
 F:	arch/ia64/
 
+IBM Operation Panel Input Driver
+M:	Eddie James <[email protected]>
+L:	linux-input at vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/ibm,op-panel.yaml
+
 IBM Power 842 compression accelerator
 M:	Haren Myneni <[email protected]>
 S:	Supported
Signature · 1 line
2.26.2
author maintainer
#2 lore raw

PATCH v3 2/5 input: misc: Add IBM Operation Panel driver

In reply to Eddie James

Add a driver to get the button events from the panel and provide them to userspace with the input subsystem. The panel is connected with I2C and controls the bus, so the driver registers as an I2C slave device.

---
 MAINTAINERS                    |   1 +
 drivers/input/misc/Kconfig     |  18 ++++
 drivers/input/misc/Makefile    |   1 +
 drivers/input/misc/ibm-panel.c | 189 +++++++++++++++++++++++++++++++++
 4 files changed, 209 insertions(+)
 create mode 100644 drivers/input/misc/ibm-panel.c
MAINTAINERS +1 -0
index 28408d29d873..5429da957a1a 100644
@@ -8351,6 +8351,7 @@ M:	Eddie James <[email protected]>
 L:	linux-input at vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/input/ibm,op-panel.yaml
+F:	drivers/input/misc/ibm-panel.c
 
 IBM Power 842 compression accelerator
 M:	Haren Myneni <[email protected]>
drivers/input/misc/Kconfig +18 -0
index 362e8a01980c..65ab1ce7b259 100644
@@ -708,6 +708,24 @@ config INPUT_ADXL34X_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called adxl34x-spi.
 
+config INPUT_IBM_PANEL
+	tristate "IBM Operation Panel driver"
+	depends on I2C_SLAVE || COMPILE_TEST
+	help
+	  Say Y here if you have an IBM Operation Panel connected to your system
+	  over I2C. The panel is typically connected only to a system's service
+	  processor (BMC).
+
+	  If unsure, say N.
+
+	  The Operation Panel is a controller with some buttons and an LCD
+	  display that allows someone with physical access to the system to
+	  perform various administrative tasks. This driver only supports the part
+	  of the controller that sends commands to the system.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ibm-panel.
+
 config INPUT_IMS_PCU
 	tristate "IMS Passenger Control Unit driver"
 	depends on USB
drivers/input/misc/Makefile +1 -0
index a48e5f2d859d..7e9edf0a142b 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
 obj-$(CONFIG_INPUT_GPIO_VIBRA)		+= gpio-vibra.o
 obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
+obj-$(CONFIG_INPUT_IBM_PANEL)		+= ibm-panel.o
 obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
 obj-$(CONFIG_INPUT_IQS269A)		+= iqs269a.o
 obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
drivers/input/misc/ibm-panel.c new file +189 -0

189 lines not on this page · raw message

Signature · 1 line
2.26.2
Reviewed-by on v3 2/5
#3 lore raw
Quoted 7 lines · Eddie James

Add a driver to get the button events from the panel and provide them to userspace with the input subsystem. The panel is connected with I2C and controls the bus, so the driver registers as an I2C slave device.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20200910/26171566/attachment.sig>

maintainer
#4 lore raw
In reply to Eddie James

Hi Eddie,

Quoted 111 lines · Eddie James

Add a driver to get the button events from the panel and provide them to userspace with the input subsystem. The panel is connected with I2C and controls the bus, so the driver registers as an I2C slave device.

---
 MAINTAINERS                    |   1 +
 drivers/input/misc/Kconfig     |  18 ++++
 drivers/input/misc/Makefile    |   1 +
 drivers/input/misc/ibm-panel.c | 189 +++++++++++++++++++++++++++++++++
 4 files changed, 209 insertions(+)
 create mode 100644 drivers/input/misc/ibm-panel.c
MAINTAINERS +1 -0
index 28408d29d873..5429da957a1a 100644
@@ -8351,6 +8351,7 @@ M:	Eddie James <[email protected]>
 L:	linux-input at vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/input/ibm,op-panel.yaml
+F:	drivers/input/misc/ibm-panel.c
 
 IBM Power 842 compression accelerator
 M:	Haren Myneni <[email protected]>
drivers/input/misc/Kconfig +18 -0
index 362e8a01980c..65ab1ce7b259 100644
@@ -708,6 +708,24 @@ config INPUT_ADXL34X_SPI
 	  To compile this driver as a module, choose M here: the
 	  module will be called adxl34x-spi.
 
+config INPUT_IBM_PANEL
+	tristate "IBM Operation Panel driver"
+	depends on I2C_SLAVE || COMPILE_TEST
+	help
+	  Say Y here if you have an IBM Operation Panel connected to your system
+	  over I2C. The panel is typically connected only to a system's service
+	  processor (BMC).
+
+	  If unsure, say N.
+
+	  The Operation Panel is a controller with some buttons and an LCD
+	  display that allows someone with physical access to the system to
+	  perform various administrative tasks. This driver only supports the part
+	  of the controller that sends commands to the system.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ibm-panel.
+
 config INPUT_IMS_PCU
 	tristate "IMS Passenger Control Unit driver"
 	depends on USB
drivers/input/misc/Makefile +1 -0
index a48e5f2d859d..7e9edf0a142b 100644
@@ -38,6 +38,7 @@ obj-$(CONFIG_INPUT_GPIO_DECODER)	+= gpio_decoder.o
 obj-$(CONFIG_INPUT_GPIO_VIBRA)		+= gpio-vibra.o
 obj-$(CONFIG_INPUT_HISI_POWERKEY)	+= hisi_powerkey.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
+obj-$(CONFIG_INPUT_IBM_PANEL)		+= ibm-panel.o
 obj-$(CONFIG_INPUT_IMS_PCU)		+= ims-pcu.o
 obj-$(CONFIG_INPUT_IQS269A)		+= iqs269a.o
 obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
drivers/input/misc/ibm-panel.c new file +37 -0
new file mode 100644
index 000000000000..7329f4641636
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) IBM Corporation 2020
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/spinlock.h>
+
+#define DEVICE_NAME	"ibm-panel"
+
+struct ibm_panel {
+	u8 idx;
+	u8 command[11];
+	spinlock_t lock;	/* protects writes to idx and command */
+	struct input_dev *input;
+};
+
+static void ibm_panel_process_command(struct ibm_panel *panel)
+{
+	u8 i;
+	u8 chksum;
+	u16 sum = 0;
+	int pressed;
+	int released;
+
+	if (panel->command[0] != 0xff && panel->command[1] != 0xf0) {
+		dev_dbg(&panel->input->dev, "command invalid\n");
+		return;
+	}
+
+	for (i = 0; i < sizeof(panel->command) - 1; ++i) {

drivers/input/misc/ibm-panel.c line 47

Quoted 10 lines
drivers/input/misc/ibm-panel.c new file +10 -0
+		sum += panel->command[i];
+		if (sum & 0xff00) {
+			sum &= 0xff;
+			sum++;
+		}
+	}
+
+	chksum = sum & 0xff;
+	chksum = ~chksum;
+	chksum++;

Maybe move checksum calculation into a separate function?

drivers/input/misc/ibm-panel.c line 55

Quoted 8 lines
drivers/input/misc/ibm-panel.c new file +8 -0
+
+	if (chksum != panel->command[sizeof(panel->command) - 1]) {
+		dev_dbg(&panel->input->dev, "command failed checksum\n");
+		return;
+	}
+
+	released = panel->command[2] & 0x80;
+	pressed = released ? 0 : 1;
	pressed = !(panel->command[2] & BIT(7));

or "pressed = !released;" if you want to keep both.

Quoted 74 lines
drivers/input/misc/ibm-panel.c new file +74 -0
+
+	switch (panel->command[2] & 0xf) {
+	case 0:
+		input_report_key(panel->input, BTN_NORTH, pressed);
+		break;
+	case 1:
+		input_report_key(panel->input, BTN_SOUTH, pressed);
+		break;
+	case 2:
+		input_report_key(panel->input, BTN_SELECT, pressed);
+		break;
+	default:
+		dev_dbg(&panel->input->dev, "unknown command %u\n",
+			panel->command[2] & 0xf);
+		return;
+	}
+
+	input_sync(panel->input);
+}
+
+static int ibm_panel_i2c_slave_cb(struct i2c_client *client,
+				  enum i2c_slave_event event, u8 *val)
+{
+	unsigned long flags;
+	struct ibm_panel *panel = i2c_get_clientdata(client);
+
+	dev_dbg(&panel->input->dev, "event: %u data: %02x\n", event, *val);
+
+	spin_lock_irqsave(&panel->lock, flags);
+
+	switch (event) {
+	case I2C_SLAVE_STOP:
+		if (panel->idx == sizeof(panel->command))
+			ibm_panel_process_command(panel);
+		else
+			dev_dbg(&panel->input->dev,
+				"command incorrect size %u\n", panel->idx);
+		fallthrough;
+	case I2C_SLAVE_WRITE_REQUESTED:
+		panel->idx = 0;
+		break;
+	case I2C_SLAVE_WRITE_RECEIVED:
+		if (panel->idx < sizeof(panel->command))
+			panel->command[panel->idx++] = *val;
+		else
+			/*
+			 * The command is too long and therefore invalid, so set the index
+			 * to it's largest possible value. When a STOP is finally received,
+			 * the command will be rejected upon processing.
+			 */
+			panel->idx = U8_MAX;
+		break;
+	case I2C_SLAVE_READ_REQUESTED:
+	case I2C_SLAVE_READ_PROCESSED:
+		*val = 0xff;
+		break;
+	default:
+		break;
+	}
+
+	spin_unlock_irqrestore(&panel->lock, flags);
+
+	return 0;
+}
+
+static int ibm_panel_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	int rc;
+	struct ibm_panel *panel = devm_kzalloc(&client->dev, sizeof(*panel),
+					       GFP_KERNEL);
+
+	if (!panel)
+		return -ENOMEM;

drivers/input/misc/ibm-panel.c line 139

Quoted 10 lines
drivers/input/misc/ibm-panel.c new file +10 -0
+
+	panel->input = devm_input_allocate_device(&client->dev);
+	if (!panel->input)
+		return -ENOMEM;
+
+	panel->input->name = client->name;
+	panel->input->id.bustype = BUS_I2C;
+	input_set_capability(panel->input, EV_KEY, BTN_NORTH);
+	input_set_capability(panel->input, EV_KEY, BTN_SOUTH);
+	input_set_capability(panel->input, EV_KEY, BTN_SELECT);

North/South/Select are gamepad buttons, not general purpose ones. I think you should not hard-code keymap in the driver, but rather use device property to specify keymap that makes sense for the particular board's application.

Quoted 4 lines
drivers/input/misc/ibm-panel.c new file +4 -0
+
+	rc = input_register_device(panel->input);
+	if (rc) {
+		dev_err(&client->dev, "Failed to register input device: %d\n",

drivers/input/misc/ibm-panel.c line 153

Quoted 10 lines
drivers/input/misc/ibm-panel.c new file +10 -0
+			rc);
+		return rc;
+	}
+
+	spin_lock_init(&panel->lock);
+
+	i2c_set_clientdata(client, panel);
+	rc = i2c_slave_register(client, ibm_panel_i2c_slave_cb);
+	if (rc) {
+		input_unregister_device(panel->input);

You are using devm, there is no need to manually unregister input device.

Quoted 4 lines
drivers/input/misc/ibm-panel.c new file +4 -0
+		return rc;
+	}
+
+	return 0;

drivers/input/misc/ibm-panel.c line 167

Quoted 10 lines
drivers/input/misc/ibm-panel.c new file +10 -0
+}
+
+static int ibm_panel_remove(struct i2c_client *client)
+{
+	int rc;
+	struct ibm_panel *panel = i2c_get_clientdata(client);
+
+	rc = i2c_slave_unregister(client);
+
+	input_unregister_device(panel->input);

This is not needed.

drivers/input/misc/ibm-panel.c line 169

Quoted 2 lines
drivers/input/misc/ibm-panel.c new file +2 -0
+
+	return rc;

The remove operation is not reversible, so there is no need to return error here. Just log en error if i2c_slave_unregister() fails if you want, and return 0.

Quoted 23 lines
drivers/input/misc/ibm-panel.c new file +20 -0
+}
+
+static const struct of_device_id ibm_panel_match[] = {
+	{ .compatible = "ibm,op-panel" },
+	{ }
+};
+
+static struct i2c_driver ibm_panel_driver = {
+	.driver = {
+		.name = DEVICE_NAME,
+		.of_match_table = ibm_panel_match,
+	},
+	.probe = ibm_panel_probe,
+	.remove = ibm_panel_remove,
+};
+module_i2c_driver(ibm_panel_driver);
+
+MODULE_AUTHOR("Eddie James <[email protected]>");
+MODULE_DESCRIPTION("IBM Operation Panel Driver");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.26.2

Thanks.

Signature · 1 line
Dmitry
author maintainer
#5 lore raw

PATCH v3 3/5 i2c: aspeed: Mask IRQ status to relevant bits

In reply to Eddie James

Mask the IRQ status to only the bits that the driver checks. This prevents excessive driver warnings when operating in slave mode when additional bits are set that the driver doesn't handle.

---
 drivers/i2c/busses/i2c-aspeed.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/i2c/busses/i2c-aspeed.c +2 -0
index 31268074c422..724bf30600d6 100644
@@ -69,6 +69,7 @@
  * These share bit definitions, so use the same values for the enable &
  * status bits.
  */
+#define ASPEED_I2CD_INTR_RECV_MASK			0xf000ffff
 #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT			BIT(14)
 #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE		BIT(13)
 #define ASPEED_I2CD_INTR_SLAVE_MATCH			BIT(7)
@@ -604,6 +605,7 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 	writel(irq_received & ~ASPEED_I2CD_INTR_RX_DONE,
 	       bus->base + ASPEED_I2C_INTR_STS_REG);
 	readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+	irq_received &= ASPEED_I2CD_INTR_RECV_MASK;
 	irq_remaining = irq_received;
 
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
Signature · 1 line
2.26.2
#6 lore raw

Re: [PATCH v3 3/5] i2c: aspeed: Mask IRQ status to relevant bits

Quoted 6 lines · Eddie James

Mask the IRQ status to only the bits that the driver checks. This prevents excessive driver warnings when operating in slave mode when additional bits are set that the driver doesn't handle.

I reconsidered and applied it now because this helps whenever slave mode is used. So, applied to for-current, thanks!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20200910/01546629/attachment.sig>

#7 lore raw

Re: [PATCH v3 3/5] i2c: aspeed: Mask IRQ status to relevant bits

Quoted 10 lines · Wolfram Sang
Quoted 6 lines · Eddie James

Mask the IRQ status to only the bits that the driver checks. This prevents excessive driver warnings when operating in slave mode when additional bits are set that the driver doesn't handle.

I reconsidered and applied it now because this helps whenever slave mode is used. So, applied to for-current, thanks!

If someone could provide a Fixes tag, that would be welcome. For me, not knowing the HW it doesn't look trivial to determine.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20200910/bbb2cc01/attachment.sig>

#8 lore raw

Re: [PATCH v3 3/5] i2c: aspeed: Mask IRQ status to relevant bits

In reply to Eddie James
Quoted 7 lines · Eddie James

Mask the IRQ status to only the bits that the driver checks. This prevents excessive driver warnings when operating in slave mode when additional bits are set that the driver doesn't handle.

Sorry, looks like I didn't get my comment in in time.

Looks good in principle. One minor comment below:

drivers/i2c/busses/i2c-aspeed.c line 72

Quoted 13 lines
---
 drivers/i2c/busses/i2c-aspeed.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/i2c/busses/i2c-aspeed.c +1 -0
index 31268074c422..724bf30600d6 100644
@@ -69,6 +69,7 @@
  * These share bit definitions, so use the same values for the enable &
  * status bits.
  */
+#define ASPEED_I2CD_INTR_RECV_MASK                     0xf000ffff

Could we define ASPEED_I2CD_INTR_RECV_MASK to be ASPEED_I2CD_INTR_ALL ?

Quoted 14 lines
drivers/i2c/busses/i2c-aspeed.c +1 -1
 #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT                        BIT(14)
 #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE              BIT(13)
 #define ASPEED_I2CD_INTR_SLAVE_MATCH                   BIT(7)
@@ -604,6 +605,7 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
        writel(irq_received & ~ASPEED_I2CD_INTR_RX_DONE,
               bus->base + ASPEED_I2C_INTR_STS_REG);
        readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+       irq_received &= ASPEED_I2CD_INTR_RECV_MASK;
        irq_remaining = irq_received;
 
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
--

2.26.2

author maintainer
#9 lore raw

Re: [PATCH v3 3/5] i2c: aspeed: Mask IRQ status to relevant bits

Quoted 15 lines · Brendan Higgins
Quoted 6 lines · Eddie James

Mask the IRQ status to only the bits that the driver checks. This prevents excessive driver warnings when operating in slave mode when additional bits are set that the driver doesn't handle.

Sorry, looks like I didn't get my comment in in time.

Looks good in principle. One minor comment below:

Quoted 4 lines
---
  drivers/i2c/busses/i2c-aspeed.c | 2 ++
  1 file changed, 2 insertions(+)
Quoted 10 lines

drivers/i2c/busses/i2c-aspeed.c line 72

Quoted 9 lines
drivers/i2c/busses/i2c-aspeed.c +1 -0
index 31268074c422..724bf30600d6 100644
@@ -69,6 +69,7 @@
   * These share bit definitions, so use the same values for the enable &
   * status bits.
   */
+#define ASPEED_I2CD_INTR_RECV_MASK                     0xf000ffff

Could we define ASPEED_I2CD_INTR_RECV_MASK to be ASPEED_I2CD_INTR_ALL ?

That was my original thought... there is another define for that already a few lines down though.

Thanks,

Eddie

Quoted 15 lines
Quoted 15 lines
drivers/i2c/busses/i2c-aspeed.c +1 -1
  #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT                        BIT(14)
  #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE              BIT(13)
  #define ASPEED_I2CD_INTR_SLAVE_MATCH                   BIT(7)
@@ -604,6 +605,7 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
         writel(irq_received & ~ASPEED_I2CD_INTR_RX_DONE,
                bus->base + ASPEED_I2C_INTR_STS_REG);
         readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+       irq_received &= ASPEED_I2CD_INTR_RECV_MASK;
         irq_remaining = irq_received;
 
  #if IS_ENABLED(CONFIG_I2C_SLAVE)
--

2.26.2

author maintainer
#10 lore raw

PATCH v3 4/5 ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device

In reply to Eddie James

Set I2C bus 0 to multi-master mode and add the panel device that will register as a slave.

---
 arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts +7 -0
index 5f4ee67ac787..4d070d6ba09f 100644
@@ -4,6 +4,7 @@
 
 #include "aspeed-g6.dtsi"
 #include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/i2c/i2c.h>
 #include <dt-bindings/leds/leds-pca955x.h>
 
 / {
@@ -438,7 +439,13 @@ aliases {
 };
 
 &i2c0 {
+	multi-master;
 	status = "okay";
+
+	ibm-panel at 62 {
+		compatible = "ibm,op-panel";
+		reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+	};
 };
 
 &i2c1 {
Signature · 1 line
2.26.2
maintainer Applied
#11 lore raw

Re: [PATCH v3 4/5] ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device

Quoted 6 lines · Eddie James

Set I2C bus 0 to multi-master mode and add the panel device that will register as a slave.

Applied to the aspeed tree for v5.10.

Cheers,

Joel

Quoted 33 lines
---
 arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts +7 -1
index 5f4ee67ac787..4d070d6ba09f 100644
@@ -4,6 +4,7 @@
 
 #include "aspeed-g6.dtsi"
 #include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/i2c/i2c.h>
 #include <dt-bindings/leds/leds-pca955x.h>
 
 / {
@@ -438,7 +439,13 @@ aliases {
 };
 
 &i2c0 {
+       multi-master;
        status = "okay";
+
+       ibm-panel at 62 {
+               compatible = "ibm,op-panel";
+               reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+       };
 };
 
 &i2c1 {
--

2.26.2

author maintainer
#12 lore raw

PATCH v3 5/5 ARM: dts: Aspeed: Rainier: Add IBM Operation Panel I2C device

In reply to Eddie James

Set I2C bus 7 to multi-master mode and add the panel device that will register as a slave.

---
 arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +7 -0
index b94421f6cbd5..50d528444f5d 100644
@@ -4,6 +4,7 @@
 
 #include "aspeed-g6.dtsi"
 #include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/i2c/i2c.h>
 #include <dt-bindings/leds/leds-pca955x.h>
 
 / {
@@ -698,6 +699,7 @@ eeprom at 53 {
 };
 
 &i2c7 {
+	multi-master;
 	status = "okay";
 
 	si7021-a20 at 20 {
@@ -831,6 +833,11 @@ gpio at 15 {
 		};
 	};
 
+	ibm-panel at 62 {
+		compatible = "ibm,op-panel";
+		reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+	};
+
 	dps: dps310 at 76 {
 		compatible = "infineon,dps310";
 		reg = <0x76>;
Signature · 1 line
2.26.2
maintainer Applied
#13 lore raw

Re: [PATCH v3 5/5] ARM: dts: Aspeed: Rainier: Add IBM Operation Panel I2C device

Quoted 6 lines · Eddie James

Set I2C bus 7 to multi-master mode and add the panel device that will register as a slave.

Applied to the aspeed tree for v5.10.

Cheers,

Joel

Quoted 39 lines
---
 arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +7 -1
index b94421f6cbd5..50d528444f5d 100644
@@ -4,6 +4,7 @@
 
 #include "aspeed-g6.dtsi"
 #include <dt-bindings/gpio/aspeed-gpio.h>
+#include <dt-bindings/i2c/i2c.h>
 #include <dt-bindings/leds/leds-pca955x.h>
 
 / {
@@ -698,6 +699,7 @@ eeprom at 53 {
 };
 
 &i2c7 {
+       multi-master;
        status = "okay";
 
        si7021-a20 at 20 {
@@ -831,6 +833,11 @@ gpio at 15 {
                };
        };
 
+       ibm-panel at 62 {
+               compatible = "ibm,op-panel";
+               reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+       };
+
        dps: dps310 at 76 {
                compatible = "infineon,dps310";
                reg = <0x76>;
--

2.26.2