* [PATCH v2 0/5] input: misc: Add IBM Operation Panel driver
@ 2020-09-08 20:00 Eddie James
2020-09-08 20:00 ` [PATCH v2 1/5] dt-bindings: input: Add documentation for IBM Operation Panel Eddie James
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Eddie James @ 2020-09-08 20:00 UTC (permalink / raw)
To: linux-input
Cc: devicetree, linux-kernel, linux-aspeed, linux-i2c, joel, andrew,
benh, brendanhiggins, dmitry.torokhov, robh+dt, wsa, rentao.bupt,
ryan_chen
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 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 | 39 ++++
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 | 192 ++++++++++++++++++
8 files changed, 273 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/ibm,op-panel.yaml
create mode 100644 drivers/input/misc/ibm-panel.c
--
2.26.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/5] dt-bindings: input: Add documentation for IBM Operation Panel
2020-09-08 20:00 [PATCH v2 0/5] input: misc: Add IBM Operation Panel driver Eddie James
@ 2020-09-08 20:00 ` Eddie James
2020-09-08 20:00 ` [PATCH v2 2/5] input: misc: Add IBM Operation Panel driver Eddie James
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Eddie James @ 2020-09-08 20:00 UTC (permalink / raw)
To: linux-input
Cc: devicetree, linux-kernel, linux-aspeed, linux-i2c, joel, andrew,
benh, brendanhiggins, dmitry.torokhov, robh+dt, wsa, rentao.bupt,
ryan_chen
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.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Acked-by: Joel Stanley <joel@jms.id.au>
---
.../bindings/input/ibm,op-panel.yaml | 39 +++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/ibm,op-panel.yaml
diff --git a/Documentation/devicetree/bindings/input/ibm,op-panel.yaml b/Documentation/devicetree/bindings/input/ibm,op-panel.yaml
new file mode 100644
index 000000000000..5f068fce93ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ibm,op-panel.yaml
@@ -0,0 +1,39 @@
+# 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 <eajames@linux.ibm.com>
+
+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
+
+examples:
+ - |
+ #include <dt-bindings/i2c/i2c.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ibm-op-panel@62 {
+ compatible = "ibm,op-panel";
+ reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 4a5d6797a206..0d83eada50e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8345,6 +8345,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 <eajames@linux.ibm.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/input/ibm,op-panel.yaml
+
IBM Power 842 compression accelerator
M: Haren Myneni <haren@us.ibm.com>
S: Supported
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/5] input: misc: Add IBM Operation Panel driver
2020-09-08 20:00 [PATCH v2 0/5] input: misc: Add IBM Operation Panel driver Eddie James
2020-09-08 20:00 ` [PATCH v2 1/5] dt-bindings: input: Add documentation for IBM Operation Panel Eddie James
@ 2020-09-08 20:00 ` Eddie James
2020-09-08 20:40 ` wsa
2020-09-08 20:00 ` [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits Eddie James
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Eddie James @ 2020-09-08 20:00 UTC (permalink / raw)
To: linux-input
Cc: devicetree, linux-kernel, linux-aspeed, linux-i2c, joel, andrew,
benh, brendanhiggins, dmitry.torokhov, robh+dt, wsa, rentao.bupt,
ryan_chen
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.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
MAINTAINERS | 1 +
drivers/input/misc/Kconfig | 18 ++++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/ibm-panel.c | 192 +++++++++++++++++++++++++++++++++
4 files changed, 212 insertions(+)
create mode 100644 drivers/input/misc/ibm-panel.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 0d83eada50e4..0f9605c4cfc6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8350,6 +8350,7 @@ M: Eddie James <eajames@linux.ibm.com>
L: linux-input@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 <haren@us.ibm.com>
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 362e8a01980c..65ab1ce7b259 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -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
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index a48e5f2d859d..7e9edf0a142b 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -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
diff --git a/drivers/input/misc/ibm-panel.c b/drivers/input/misc/ibm-panel.c
new file mode 100644
index 000000000000..7b147b090c2a
--- /dev/null
+++ b/drivers/input/misc/ibm-panel.c
@@ -0,0 +1,192 @@
+// 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 command_size)
+{
+ u8 i;
+ u8 chksum;
+ u16 sum = 0;
+ int pressed;
+ int released;
+
+ if (command_size != sizeof(panel->command)) {
+ dev_dbg(&panel->input->dev, "command size incorrect\n");
+ return;
+ }
+
+ 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) {
+ sum += panel->command[i];
+ if (sum & 0xff00) {
+ sum &= 0xff;
+ sum++;
+ }
+ }
+
+ chksum = sum & 0xff;
+ chksum = ~chksum;
+ chksum++;
+
+ 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;
+
+ 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)
+{
+ u8 command_size = 0;
+ 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:
+ command_size = 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;
+ default:
+ break;
+ }
+
+ spin_unlock_irqrestore(&panel->lock, flags);
+
+ if (command_size)
+ ibm_panel_process_command(panel, command_size);
+
+ 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;
+
+ 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);
+
+ rc = input_register_device(panel->input);
+ if (rc) {
+ dev_err(&client->dev, "Failed to register input device: %d\n",
+ 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);
+ dev_err(&client->dev,
+ "Failed to register I2C slave device: %d\n", rc);
+ return rc;
+ }
+
+ return 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);
+
+ return rc;
+}
+
+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 <eajames@linux.ibm.com>");
+MODULE_DESCRIPTION("IBM Operation Panel Driver");
+MODULE_LICENSE("GPL");
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits
2020-09-08 20:00 [PATCH v2 0/5] input: misc: Add IBM Operation Panel driver Eddie James
2020-09-08 20:00 ` [PATCH v2 1/5] dt-bindings: input: Add documentation for IBM Operation Panel Eddie James
2020-09-08 20:00 ` [PATCH v2 2/5] input: misc: Add IBM Operation Panel driver Eddie James
@ 2020-09-08 20:00 ` Eddie James
2020-09-09 2:59 ` Tao Ren
` (2 more replies)
2020-09-08 20:01 ` [PATCH v2 4/5] ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device Eddie James
2020-09-08 20:01 ` [PATCH v2 5/5] ARM: dts: Aspeed: Rainier: " Eddie James
4 siblings, 3 replies; 11+ messages in thread
From: Eddie James @ 2020-09-08 20:00 UTC (permalink / raw)
To: linux-input
Cc: devicetree, linux-kernel, linux-aspeed, linux-i2c, joel, andrew,
benh, brendanhiggins, dmitry.torokhov, robh+dt, wsa, rentao.bupt,
ryan_chen
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.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/i2c/busses/i2c-aspeed.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 31268074c422..2a388911038a 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -69,6 +69,7 @@
* These share bit definitions, so use the same values for the enable &
* status bits.
*/
+#define ASPEED_I2CD_INTR_ALL 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_ALL;
irq_remaining = irq_received;
#if IS_ENABLED(CONFIG_I2C_SLAVE)
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/5] ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device
2020-09-08 20:00 [PATCH v2 0/5] input: misc: Add IBM Operation Panel driver Eddie James
` (2 preceding siblings ...)
2020-09-08 20:00 ` [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits Eddie James
@ 2020-09-08 20:01 ` Eddie James
2020-09-09 6:56 ` Joel Stanley
2020-09-08 20:01 ` [PATCH v2 5/5] ARM: dts: Aspeed: Rainier: " Eddie James
4 siblings, 1 reply; 11+ messages in thread
From: Eddie James @ 2020-09-08 20:01 UTC (permalink / raw)
To: linux-input
Cc: devicetree, linux-kernel, linux-aspeed, linux-i2c, joel, andrew,
benh, brendanhiggins, dmitry.torokhov, robh+dt, wsa, rentao.bupt,
ryan_chen
Set I2C bus 0 to multi-master mode and add the panel device that will
register as a slave.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
index 5f4ee67ac787..4d070d6ba09f 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
@@ -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@62 {
+ compatible = "ibm,op-panel";
+ reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+ };
};
&i2c1 {
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 5/5] ARM: dts: Aspeed: Rainier: Add IBM Operation Panel I2C device
2020-09-08 20:00 [PATCH v2 0/5] input: misc: Add IBM Operation Panel driver Eddie James
` (3 preceding siblings ...)
2020-09-08 20:01 ` [PATCH v2 4/5] ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device Eddie James
@ 2020-09-08 20:01 ` Eddie James
4 siblings, 0 replies; 11+ messages in thread
From: Eddie James @ 2020-09-08 20:01 UTC (permalink / raw)
To: linux-input
Cc: devicetree, linux-kernel, linux-aspeed, linux-i2c, joel, andrew,
benh, brendanhiggins, dmitry.torokhov, robh+dt, wsa, rentao.bupt,
ryan_chen
Set I2C bus 7 to multi-master mode and add the panel device that will
register as a slave.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
index b94421f6cbd5..50d528444f5d 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts
@@ -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@53 {
};
&i2c7 {
+ multi-master;
status = "okay";
si7021-a20@20 {
@@ -831,6 +833,11 @@ gpio@15 {
};
};
+ ibm-panel@62 {
+ compatible = "ibm,op-panel";
+ reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+ };
+
dps: dps310@76 {
compatible = "infineon,dps310";
reg = <0x76>;
--
2.26.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/5] input: misc: Add IBM Operation Panel driver
2020-09-08 20:00 ` [PATCH v2 2/5] input: misc: Add IBM Operation Panel driver Eddie James
@ 2020-09-08 20:40 ` wsa
0 siblings, 0 replies; 11+ messages in thread
From: wsa @ 2020-09-08 20:40 UTC (permalink / raw)
To: Eddie James
Cc: linux-input, devicetree, linux-kernel, linux-aspeed, linux-i2c,
joel, andrew, benh, brendanhiggins, dmitry.torokhov, robh+dt,
rentao.bupt, ryan_chen
[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]
Hi Eddie,
> + switch (event) {
> + case I2C_SLAVE_STOP:
> + command_size = 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;
> + default:
> + break;
> + }
Sorry, I missed this in my last review. READ states are mandatory, so
you will need something like this:
+ case I2C_SLAVE_READ_REQUESTED:
+ case I2C_SLAVE_READ_PROCESSED:
+ *val = 0xff;
+ break;
> + if (command_size)
> + ibm_panel_process_command(panel, command_size);
I wondered if you could check for the correct command_size here, so no
need to call into the function when the size doesn't match?
> + rc = i2c_slave_register(client, ibm_panel_i2c_slave_cb);
> + if (rc) {
> + input_unregister_device(panel->input);
> + dev_err(&client->dev,
> + "Failed to register I2C slave device: %d\n", rc);
This dev_err can go. The core will print messages if something goes
wrong.
The rest looks good from an I2C point of view.
I'd think this all should go via the input tree?
Kind regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits
2020-09-08 20:00 ` [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits Eddie James
@ 2020-09-09 2:59 ` Tao Ren
2020-09-09 7:59 ` kernel test robot
2020-09-09 8:01 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: Tao Ren @ 2020-09-09 2:59 UTC (permalink / raw)
To: Eddie James
Cc: linux-input, devicetree, linux-kernel, linux-aspeed, linux-i2c,
joel, andrew, benh, brendanhiggins, dmitry.torokhov, robh+dt,
wsa, ryan_chen
On Tue, Sep 08, 2020 at 03:00:59PM -0500, Eddie James wrote:
> 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.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Tao Ren <rentao.bupt@gmail.com>
> ---
> drivers/i2c/busses/i2c-aspeed.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 31268074c422..2a388911038a 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -69,6 +69,7 @@
> * These share bit definitions, so use the same values for the enable &
> * status bits.
> */
> +#define ASPEED_I2CD_INTR_ALL 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_ALL;
> irq_remaining = irq_received;
>
> #if IS_ENABLED(CONFIG_I2C_SLAVE)
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/5] ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device
2020-09-08 20:01 ` [PATCH v2 4/5] ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device Eddie James
@ 2020-09-09 6:56 ` Joel Stanley
0 siblings, 0 replies; 11+ messages in thread
From: Joel Stanley @ 2020-09-09 6:56 UTC (permalink / raw)
To: Eddie James
Cc: linux-input, devicetree, Linux Kernel Mailing List, linux-aspeed,
linux-i2c, Andrew Jeffery, Benjamin Herrenschmidt,
Brendan Higgins, dmitry.torokhov, Rob Herring, wsa, Tao Ren,
Ryan Chen
On Tue, 8 Sep 2020 at 20:01, Eddie James <eajames@linux.ibm.com> wrote:
>
> Set I2C bus 0 to multi-master mode and add the panel device that will
> register as a slave.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
I will take this and the rainier dts patch through the aspeed tree so
we don't get conflicts.
Eddie, when you send v3, you can omit them.
Cheers,
Joel
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
> index 5f4ee67ac787..4d070d6ba09f 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-tacoma.dts
> @@ -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@62 {
> + compatible = "ibm,op-panel";
> + reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
> + };
> };
>
> &i2c1 {
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits
2020-09-08 20:00 ` [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits Eddie James
2020-09-09 2:59 ` Tao Ren
@ 2020-09-09 7:59 ` kernel test robot
2020-09-09 8:01 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-09-09 7:59 UTC (permalink / raw)
To: Eddie James, linux-input
Cc: kbuild-all, clang-built-linux, devicetree, linux-kernel,
linux-aspeed, linux-i2c, joel, andrew, benh, brendanhiggins,
dmitry.torokhov
[-- Attachment #1: Type: text/plain, Size: 5188 bytes --]
Hi Eddie,
I love your patch! Perhaps something to improve:
[auto build test WARNING on joel-aspeed/for-next]
[also build test WARNING on wsa/i2c/for-next linus/master v5.9-rc4 next-20200908]
[cannot apply to input/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Eddie-James/input-misc-Add-IBM-Operation-Panel-driver/20200909-120637
base: https://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git for-next
config: x86_64-randconfig-a016-20200909 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 8893d0816ccdf8998d2e21b5430e9d6abe7ef465)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-aspeed.c:88:9: warning: 'ASPEED_I2CD_INTR_ALL' macro redefined [-Wmacro-redefined]
#define ASPEED_I2CD_INTR_ALL \
^
drivers/i2c/busses/i2c-aspeed.c:72:9: note: previous definition is here
#define ASPEED_I2CD_INTR_ALL 0xf000ffff
^
1 warning generated.
# https://github.com/0day-ci/linux/commit/6d9e57527652ab1a348f68145e41e6a19d5dc0dc
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eddie-James/input-misc-Add-IBM-Operation-Panel-driver/20200909-120637
git checkout 6d9e57527652ab1a348f68145e41e6a19d5dc0dc
vim +/ASPEED_I2CD_INTR_ALL +88 drivers/i2c/busses/i2c-aspeed.c
f327c686d3ba44e Brendan Higgins 2017-06-20 65
f327c686d3ba44e Brendan Higgins 2017-06-20 66 /* 0x0c : I2CD Interrupt Control Register &
f327c686d3ba44e Brendan Higgins 2017-06-20 67 * 0x10 : I2CD Interrupt Status Register
f327c686d3ba44e Brendan Higgins 2017-06-20 68 *
f327c686d3ba44e Brendan Higgins 2017-06-20 69 * These share bit definitions, so use the same values for the enable &
f327c686d3ba44e Brendan Higgins 2017-06-20 70 * status bits.
f327c686d3ba44e Brendan Higgins 2017-06-20 71 */
6d9e57527652ab1 Eddie James 2020-09-08 72 #define ASPEED_I2CD_INTR_ALL 0xf000ffff
f327c686d3ba44e Brendan Higgins 2017-06-20 73 #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
f327c686d3ba44e Brendan Higgins 2017-06-20 74 #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
f9eb91350bb20b3 Brendan Higgins 2017-06-20 75 #define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
f327c686d3ba44e Brendan Higgins 2017-06-20 76 #define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
f327c686d3ba44e Brendan Higgins 2017-06-20 77 #define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
f327c686d3ba44e Brendan Higgins 2017-06-20 78 #define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
f327c686d3ba44e Brendan Higgins 2017-06-20 79 #define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
f327c686d3ba44e Brendan Higgins 2017-06-20 80 #define ASPEED_I2CD_INTR_RX_DONE BIT(2)
f327c686d3ba44e Brendan Higgins 2017-06-20 81 #define ASPEED_I2CD_INTR_TX_NAK BIT(1)
f327c686d3ba44e Brendan Higgins 2017-06-20 82 #define ASPEED_I2CD_INTR_TX_ACK BIT(0)
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 83 #define ASPEED_I2CD_INTR_MASTER_ERRORS \
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 84 (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 85 ASPEED_I2CD_INTR_SCL_TIMEOUT | \
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 86 ASPEED_I2CD_INTR_ABNORMAL | \
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 87 ASPEED_I2CD_INTR_ARBIT_LOSS)
f327c686d3ba44e Brendan Higgins 2017-06-20 @88 #define ASPEED_I2CD_INTR_ALL \
f327c686d3ba44e Brendan Higgins 2017-06-20 89 (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
f327c686d3ba44e Brendan Higgins 2017-06-20 90 ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
f327c686d3ba44e Brendan Higgins 2017-06-20 91 ASPEED_I2CD_INTR_SCL_TIMEOUT | \
f327c686d3ba44e Brendan Higgins 2017-06-20 92 ASPEED_I2CD_INTR_ABNORMAL | \
f327c686d3ba44e Brendan Higgins 2017-06-20 93 ASPEED_I2CD_INTR_NORMAL_STOP | \
f327c686d3ba44e Brendan Higgins 2017-06-20 94 ASPEED_I2CD_INTR_ARBIT_LOSS | \
f327c686d3ba44e Brendan Higgins 2017-06-20 95 ASPEED_I2CD_INTR_RX_DONE | \
f327c686d3ba44e Brendan Higgins 2017-06-20 96 ASPEED_I2CD_INTR_TX_NAK | \
f327c686d3ba44e Brendan Higgins 2017-06-20 97 ASPEED_I2CD_INTR_TX_ACK)
f327c686d3ba44e Brendan Higgins 2017-06-20 98
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37048 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits
2020-09-08 20:00 ` [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits Eddie James
2020-09-09 2:59 ` Tao Ren
2020-09-09 7:59 ` kernel test robot
@ 2020-09-09 8:01 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2020-09-09 8:01 UTC (permalink / raw)
To: Eddie James, linux-input
Cc: kbuild-all, devicetree, linux-kernel, linux-aspeed, linux-i2c,
joel, andrew, benh, brendanhiggins, dmitry.torokhov
[-- Attachment #1: Type: text/plain, Size: 4921 bytes --]
Hi Eddie,
I love your patch! Perhaps something to improve:
[auto build test WARNING on joel-aspeed/for-next]
[also build test WARNING on wsa/i2c/for-next linus/master v5.9-rc4 next-20200908]
[cannot apply to input/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Eddie-James/input-misc-Add-IBM-Operation-Panel-driver/20200909-120637
base: https://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git for-next
config: h8300-randconfig-r016-20200909 (attached as .config)
compiler: h8300-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=h8300
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-aspeed.c:88: warning: "ASPEED_I2CD_INTR_ALL" redefined
88 | #define ASPEED_I2CD_INTR_ALL \
|
drivers/i2c/busses/i2c-aspeed.c:72: note: this is the location of the previous definition
72 | #define ASPEED_I2CD_INTR_ALL 0xf000ffff
|
# https://github.com/0day-ci/linux/commit/6d9e57527652ab1a348f68145e41e6a19d5dc0dc
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eddie-James/input-misc-Add-IBM-Operation-Panel-driver/20200909-120637
git checkout 6d9e57527652ab1a348f68145e41e6a19d5dc0dc
vim +/ASPEED_I2CD_INTR_ALL +88 drivers/i2c/busses/i2c-aspeed.c
f327c686d3ba44e Brendan Higgins 2017-06-20 65
f327c686d3ba44e Brendan Higgins 2017-06-20 66 /* 0x0c : I2CD Interrupt Control Register &
f327c686d3ba44e Brendan Higgins 2017-06-20 67 * 0x10 : I2CD Interrupt Status Register
f327c686d3ba44e Brendan Higgins 2017-06-20 68 *
f327c686d3ba44e Brendan Higgins 2017-06-20 69 * These share bit definitions, so use the same values for the enable &
f327c686d3ba44e Brendan Higgins 2017-06-20 70 * status bits.
f327c686d3ba44e Brendan Higgins 2017-06-20 71 */
6d9e57527652ab1 Eddie James 2020-09-08 72 #define ASPEED_I2CD_INTR_ALL 0xf000ffff
f327c686d3ba44e Brendan Higgins 2017-06-20 73 #define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
f327c686d3ba44e Brendan Higgins 2017-06-20 74 #define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
f9eb91350bb20b3 Brendan Higgins 2017-06-20 75 #define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
f327c686d3ba44e Brendan Higgins 2017-06-20 76 #define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
f327c686d3ba44e Brendan Higgins 2017-06-20 77 #define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
f327c686d3ba44e Brendan Higgins 2017-06-20 78 #define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
f327c686d3ba44e Brendan Higgins 2017-06-20 79 #define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
f327c686d3ba44e Brendan Higgins 2017-06-20 80 #define ASPEED_I2CD_INTR_RX_DONE BIT(2)
f327c686d3ba44e Brendan Higgins 2017-06-20 81 #define ASPEED_I2CD_INTR_TX_NAK BIT(1)
f327c686d3ba44e Brendan Higgins 2017-06-20 82 #define ASPEED_I2CD_INTR_TX_ACK BIT(0)
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 83 #define ASPEED_I2CD_INTR_MASTER_ERRORS \
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 84 (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 85 ASPEED_I2CD_INTR_SCL_TIMEOUT | \
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 86 ASPEED_I2CD_INTR_ABNORMAL | \
3e9efc3299dd78a Jae Hyun Yoo 2018-08-23 87 ASPEED_I2CD_INTR_ARBIT_LOSS)
f327c686d3ba44e Brendan Higgins 2017-06-20 @88 #define ASPEED_I2CD_INTR_ALL \
f327c686d3ba44e Brendan Higgins 2017-06-20 89 (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
f327c686d3ba44e Brendan Higgins 2017-06-20 90 ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
f327c686d3ba44e Brendan Higgins 2017-06-20 91 ASPEED_I2CD_INTR_SCL_TIMEOUT | \
f327c686d3ba44e Brendan Higgins 2017-06-20 92 ASPEED_I2CD_INTR_ABNORMAL | \
f327c686d3ba44e Brendan Higgins 2017-06-20 93 ASPEED_I2CD_INTR_NORMAL_STOP | \
f327c686d3ba44e Brendan Higgins 2017-06-20 94 ASPEED_I2CD_INTR_ARBIT_LOSS | \
f327c686d3ba44e Brendan Higgins 2017-06-20 95 ASPEED_I2CD_INTR_RX_DONE | \
f327c686d3ba44e Brendan Higgins 2017-06-20 96 ASPEED_I2CD_INTR_TX_NAK | \
f327c686d3ba44e Brendan Higgins 2017-06-20 97 ASPEED_I2CD_INTR_TX_ACK)
f327c686d3ba44e Brendan Higgins 2017-06-20 98
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29190 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-09-09 8:07 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-08 20:00 [PATCH v2 0/5] input: misc: Add IBM Operation Panel driver Eddie James
2020-09-08 20:00 ` [PATCH v2 1/5] dt-bindings: input: Add documentation for IBM Operation Panel Eddie James
2020-09-08 20:00 ` [PATCH v2 2/5] input: misc: Add IBM Operation Panel driver Eddie James
2020-09-08 20:40 ` wsa
2020-09-08 20:00 ` [PATCH v2 3/5] i2c: aspeed: Mask IRQ status to relevant bits Eddie James
2020-09-09 2:59 ` Tao Ren
2020-09-09 7:59 ` kernel test robot
2020-09-09 8:01 ` kernel test robot
2020-09-08 20:01 ` [PATCH v2 4/5] ARM: dts: Aspeed: Tacoma: Add IBM Operation Panel I2C device Eddie James
2020-09-09 6:56 ` Joel Stanley
2020-09-08 20:01 ` [PATCH v2 5/5] ARM: dts: Aspeed: Rainier: " Eddie James
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).