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
--
2.26.2
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 <[email protected]>
Reviewed-by: Tao Ren <[email protected]>
---
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..724bf30600d6 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_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)
--
2.26.2
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 <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Acked-by: Joel Stanley <[email protected]>
---
.../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
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..52c4a6275a77
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ibm,op-panel.yaml
@@ -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@62 {
+ compatible = "ibm,op-panel";
+ reg = <(0x62 | I2C_OWN_SLAVE_ADDRESS)>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 3c0692404907..28408d29d873 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -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: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/input/ibm,op-panel.yaml
+
IBM Power 842 compression accelerator
M: Haren Myneni <[email protected]>
S: Supported
--
2.26.2
On Wed, Sep 09, 2020 at 03:30:57PM -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 <[email protected]>
> Reviewed-by: Tao Ren <[email protected]>
I reconsidered and applied it now because this helps whenever slave mode
is used. So, applied to for-current, thanks!
On Thu, Sep 10, 2020 at 08:18:13AM +0200, Wolfram Sang wrote:
> On Wed, Sep 09, 2020 at 03:30:57PM -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 <[email protected]>
> > Reviewed-by: Tao Ren <[email protected]>
>
> 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.
On Wed, Sep 9, 2020 at 1:31 PM Eddie James <[email protected]> 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 <[email protected]>
> Reviewed-by: Tao Ren <[email protected]>
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 | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 31268074c422..724bf30600d6 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_RECV_MASK 0xf000ffff
Could we define ASPEED_I2CD_INTR_RECV_MASK to be ASPEED_I2CD_INTR_ALL ?
> #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
>
On 9/10/20 4:00 AM, Brendan Higgins wrote:
> On Wed, Sep 9, 2020 at 1:31 PM Eddie James <[email protected]> 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 <[email protected]>
>> Reviewed-by: Tao Ren <[email protected]>
> 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 | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>> index 31268074c422..724bf30600d6 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_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
>
>> #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
>>