2021-02-19 14:29:47

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH 00/19] ipmi: Allow raw access to KCS devices

Hello,

This series is a bit of a mix of things, but its primary purpose is to
expose BMC KCS IPMI devices to userspace in a way that enables userspace
to talk to host firmware using protocols that are not IPMI. The new
chardev implementation exposes the Input Data Register (IDR), Output
Data Register (ODR) and Status Register (STR) via read() and write(),
and implements poll() for event monitoring.

The existing /dev/ipmi-kcs* chardev interface exposes the KCS devices in
a way which encoded the IPMI protocol in its behaviour. However, as
LPC[0] KCS devices give us bi-directional interrupts between the host
and a BMC with both a data and status byte, they are useful for purposes
beyond IPMI.

As a concrete example, libmctp[1] implements a vendor-defined
host-interface MCTP[2] binding using a combination of LPC Firmware
cycles for bulk data transfer and a KCS device via LPC IO cycles for
out-of-band protocol control messages[3]. This gives a throughput
improvement over the standard KCS binding[4] while continuing to exploit
the ease of setup of the LPC bus for early boot firmware on the host
processor.

The series takes a bit of a winding path to achieve its aim:

1. It begins with patches 1-5 put together by Chia-Wei, which I've
rebased on v5.11. These fix the ASPEED LPC bindings and other non-KCS
LPC-related ASPEED device drivers in a way that enables the SerIRQ
patches at the end of the series. With Joel's review I'm hoping these 5
can go through the aspeed tree, and that the rest can go through the
IPMI tree.

2. Next, patches 6-13 fairly heavily refactor the KCS support in the
IPMI part of the tree, re-architecting things such that it's possible to
support multiple chardev implementations sitting on top of the ASPEED
and Nuvoton device drivers. However, the KCS code didn't really have
great separation of concerns as it stood, so even if we disregard the
multiple-chardev support I think the cleanups are worthwhile.

3. Patch 14 adds some interrupt management capabilities to the KCS
device drivers in preparation for patch 15, which introduces the new
"raw" KCS device interface. I'm not stoked about the device name/path,
so if people are looking to bikeshed something then feel free to lay
into that.

4. The remaining patches switch the ASPEED KCS devicetree binding to
dt-schema, add a new interrupt property to describe the SerIRQ behaviour
of the device and finally clean up Serial IRQ support in the ASPEED KCS
driver.

Rob: The dt-binding patches still come before the relevant driver
changes, I tried to keep the two close together in the series, hence the
bindings changes not being patches 1 and 2.

I've exercised the series under qemu with the rainier-bmc machine plus
some preliminary KCS support[5]. I've also substituted this series in
place of a hacky out-of-tree driver that we've been using for the
libmctp stack and successfully booted the host processor under our
internal full-platform simulation tools for a Rainier system.

Note that this work touches the Nuvoton driver as well as ASPEED's, but
I don't have the capability to test those changes or the IPMI chardev
path. Tested-by tags would be much appreciated if you can exercise one
or both.

Please review!

Andrew

[0] https://www.intel.com/content/dam/www/program/design/us/en/documents/low-pin-count-interface-specification.pdf
[1] https://github.com/openbmc/libmctp/
[2] https://www.dmtf.org/sites/default/files/standards/documents/DSP0236_1.3.1.pdf
[3] https://github.com/openbmc/libmctp/blob/master/docs/bindings/vendor-astlpc.md
[4] https://www.dmtf.org/sites/default/files/standards/documents/DSP0254_1.0.0.pdf
[5] https://github.com/amboar/qemu/tree/kcs

Andrew Jeffery (14):
ipmi: kcs_bmc_aspeed: Use of match data to extract KCS properties
ipmi: kcs_bmc: Make status update atomic
ipmi: kcs_bmc: Rename {read,write}_{status,data}() functions
ipmi: kcs_bmc: Split out kcs_bmc_cdev_ipmi
ipmi: kcs_bmc: Turn the driver data-structures inside-out
ipmi: kcs_bmc: Split headers into device and client
ipmi: kcs_bmc: Strip private client data from struct kcs_bmc
ipmi: kcs_bmc: Decouple the IPMI chardev from the core
ipmi: kcs_bmc: Allow clients to control KCS IRQ state
ipmi: kcs_bmc: Add a "raw" character device interface
dt-bindings: ipmi: Convert ASPEED KCS binding to schema
dt-bindings: ipmi: Add optional SerIRQ property to ASPEED KCS devices
ipmi: kcs_bmc_aspeed: Implement KCS SerIRQ configuration
ipmi: kcs_bmc_aspeed: Fix IBFIE typo from datasheet

Chia-Wei, Wang (5):
dt-bindings: aspeed-lpc: Remove LPC partitioning
ARM: dts: Remove LPC BMC and Host partitions
ipmi: kcs: aspeed: Adapt to new LPC DTS layout
pinctrl: aspeed-g5: Adapt to new LPC device tree layout
soc: aspeed: Adapt to new LPC device tree layout

Documentation/ABI/testing/dev-raw-kcs | 25 +
.../bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 106 ++++
.../bindings/ipmi/aspeed-kcs-bmc.txt | 33 -
.../devicetree/bindings/mfd/aspeed-lpc.txt | 100 +--
arch/arm/boot/dts/aspeed-g4.dtsi | 68 +--
arch/arm/boot/dts/aspeed-g5.dtsi | 119 ++--
arch/arm/boot/dts/aspeed-g6.dtsi | 121 ++--
drivers/char/ipmi/Kconfig | 30 +
drivers/char/ipmi/Makefile | 2 +
drivers/char/ipmi/kcs_bmc.c | 532 +++++-----------
drivers/char/ipmi/kcs_bmc.h | 94 +--
drivers/char/ipmi/kcs_bmc_aspeed.c | 536 ++++++++++------
drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 570 ++++++++++++++++++
drivers/char/ipmi/kcs_bmc_cdev_raw.c | 442 ++++++++++++++
drivers/char/ipmi/kcs_bmc_client.h | 47 ++
drivers/char/ipmi/kcs_bmc_device.h | 20 +
drivers/char/ipmi/kcs_bmc_npcm7xx.c | 98 ++-
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 17 +-
drivers/soc/aspeed/aspeed-lpc-ctrl.c | 20 +-
drivers/soc/aspeed/aspeed-lpc-snoop.c | 23 +-
20 files changed, 2021 insertions(+), 982 deletions(-)
create mode 100644 Documentation/ABI/testing/dev-raw-kcs
create mode 100644 Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
delete mode 100644 Documentation/devicetree/bindings/ipmi/aspeed-kcs-bmc.txt
create mode 100644 drivers/char/ipmi/kcs_bmc_cdev_ipmi.c
create mode 100644 drivers/char/ipmi/kcs_bmc_cdev_raw.c
create mode 100644 drivers/char/ipmi/kcs_bmc_client.h
create mode 100644 drivers/char/ipmi/kcs_bmc_device.h

--
2.27.0


2021-02-19 14:29:56

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH 08/19] ipmi: kcs_bmc: Rename {read,write}_{status,data}() functions

Rename the functions in preparation for separating the IPMI chardev out
from the KCS BMC core.

Signed-off-by: Andrew Jeffery <[email protected]>
---
drivers/char/ipmi/kcs_bmc.c | 52 ++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
index 58fb1a7bd50d..c4336c1f2d6d 100644
--- a/drivers/char/ipmi/kcs_bmc.c
+++ b/drivers/char/ipmi/kcs_bmc.c
@@ -45,42 +45,42 @@ enum kcs_states {
#define KCS_CMD_WRITE_END 0x62
#define KCS_CMD_READ_BYTE 0x68

-static inline u8 read_data(struct kcs_bmc *kcs_bmc)
+static inline u8 kcs_bmc_read_data(struct kcs_bmc *kcs_bmc)
{
return kcs_bmc->io_inputb(kcs_bmc, kcs_bmc->ioreg.idr);
}

-static inline void write_data(struct kcs_bmc *kcs_bmc, u8 data)
+static inline void kcs_bmc_write_data(struct kcs_bmc *kcs_bmc, u8 data)
{
kcs_bmc->io_outputb(kcs_bmc, kcs_bmc->ioreg.odr, data);
}

-static inline u8 read_status(struct kcs_bmc *kcs_bmc)
+static inline u8 kcs_bmc_read_status(struct kcs_bmc *kcs_bmc)
{
return kcs_bmc->io_inputb(kcs_bmc, kcs_bmc->ioreg.str);
}

-static inline void write_status(struct kcs_bmc *kcs_bmc, u8 data)
+static inline void kcs_bmc_write_status(struct kcs_bmc *kcs_bmc, u8 data)
{
kcs_bmc->io_outputb(kcs_bmc, kcs_bmc->ioreg.str, data);
}

-static void update_status_bits(struct kcs_bmc *kcs_bmc, u8 mask, u8 val)
+static void kcs_bmc_update_status(struct kcs_bmc *kcs_bmc, u8 mask, u8 val)
{
kcs_bmc->io_updateb(kcs_bmc, kcs_bmc->ioreg.str, mask, val);
}

static inline void set_state(struct kcs_bmc *kcs_bmc, u8 state)
{
- update_status_bits(kcs_bmc, KCS_STATUS_STATE_MASK,
+ kcs_bmc_update_status(kcs_bmc, KCS_STATUS_STATE_MASK,
KCS_STATUS_STATE(state));
}

static void kcs_force_abort(struct kcs_bmc *kcs_bmc)
{
set_state(kcs_bmc, ERROR_STATE);
- read_data(kcs_bmc);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_read_data(kcs_bmc);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);

kcs_bmc->phase = KCS_PHASE_ERROR;
kcs_bmc->data_in_avail = false;
@@ -99,9 +99,9 @@ static void kcs_bmc_handle_data(struct kcs_bmc *kcs_bmc)
case KCS_PHASE_WRITE_DATA:
if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
set_state(kcs_bmc, WRITE_STATE);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
- read_data(kcs_bmc);
+ kcs_bmc_read_data(kcs_bmc);
} else {
kcs_force_abort(kcs_bmc);
kcs_bmc->error = KCS_LENGTH_ERROR;
@@ -112,7 +112,7 @@ static void kcs_bmc_handle_data(struct kcs_bmc *kcs_bmc)
if (kcs_bmc->data_in_idx < KCS_MSG_BUFSIZ) {
set_state(kcs_bmc, READ_STATE);
kcs_bmc->data_in[kcs_bmc->data_in_idx++] =
- read_data(kcs_bmc);
+ kcs_bmc_read_data(kcs_bmc);
kcs_bmc->phase = KCS_PHASE_WRITE_DONE;
kcs_bmc->data_in_avail = true;
wake_up_interruptible(&kcs_bmc->queue);
@@ -126,34 +126,34 @@ static void kcs_bmc_handle_data(struct kcs_bmc *kcs_bmc)
if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len)
set_state(kcs_bmc, IDLE_STATE);

- data = read_data(kcs_bmc);
+ data = kcs_bmc_read_data(kcs_bmc);
if (data != KCS_CMD_READ_BYTE) {
set_state(kcs_bmc, ERROR_STATE);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
break;
}

if (kcs_bmc->data_out_idx == kcs_bmc->data_out_len) {
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
kcs_bmc->phase = KCS_PHASE_IDLE;
break;
}

- write_data(kcs_bmc,
+ kcs_bmc_write_data(kcs_bmc,
kcs_bmc->data_out[kcs_bmc->data_out_idx++]);
break;

case KCS_PHASE_ABORT_ERROR1:
set_state(kcs_bmc, READ_STATE);
- read_data(kcs_bmc);
- write_data(kcs_bmc, kcs_bmc->error);
+ kcs_bmc_read_data(kcs_bmc);
+ kcs_bmc_write_data(kcs_bmc, kcs_bmc->error);
kcs_bmc->phase = KCS_PHASE_ABORT_ERROR2;
break;

case KCS_PHASE_ABORT_ERROR2:
set_state(kcs_bmc, IDLE_STATE);
- read_data(kcs_bmc);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_read_data(kcs_bmc);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);
kcs_bmc->phase = KCS_PHASE_IDLE;
break;

@@ -168,9 +168,9 @@ static void kcs_bmc_handle_cmd(struct kcs_bmc *kcs_bmc)
u8 cmd;

set_state(kcs_bmc, WRITE_STATE);
- write_data(kcs_bmc, KCS_ZERO_DATA);
+ kcs_bmc_write_data(kcs_bmc, KCS_ZERO_DATA);

- cmd = read_data(kcs_bmc);
+ cmd = kcs_bmc_read_data(kcs_bmc);
switch (cmd) {
case KCS_CMD_WRITE_START:
kcs_bmc->phase = KCS_PHASE_WRITE_START;
@@ -212,7 +212,7 @@ int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc)

spin_lock_irqsave(&kcs_bmc->lock, flags);

- status = read_status(kcs_bmc);
+ status = kcs_bmc_read_status(kcs_bmc);
if (status & KCS_STATUS_IBF) {
if (!kcs_bmc->running)
kcs_force_abort(kcs_bmc);
@@ -350,7 +350,7 @@ static ssize_t kcs_bmc_write(struct file *filp, const char __user *buf,
kcs_bmc->data_out_idx = 1;
kcs_bmc->data_out_len = count;
memcpy(kcs_bmc->data_out, kcs_bmc->kbuffer, count);
- write_data(kcs_bmc, kcs_bmc->data_out[0]);
+ kcs_bmc_write_data(kcs_bmc, kcs_bmc->data_out[0]);
ret = count;
} else {
ret = -EINVAL;
@@ -373,13 +373,11 @@ static long kcs_bmc_ioctl(struct file *filp, unsigned int cmd,

switch (cmd) {
case IPMI_BMC_IOCTL_SET_SMS_ATN:
- update_status_bits(kcs_bmc, KCS_STATUS_SMS_ATN,
- KCS_STATUS_SMS_ATN);
+ kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, KCS_STATUS_SMS_ATN);
break;

case IPMI_BMC_IOCTL_CLEAR_SMS_ATN:
- update_status_bits(kcs_bmc, KCS_STATUS_SMS_ATN,
- 0);
+ kcs_bmc_update_status(kcs_bmc, KCS_STATUS_SMS_ATN, 0);
break;

case IPMI_BMC_IOCTL_FORCE_ABORT:
--
2.27.0

2021-02-19 14:30:58

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH 05/19] soc: aspeed: Adapt to new LPC device tree layout

From: "Chia-Wei, Wang" <[email protected]>

Add check against LPC device v2 compatible string to
ensure that the fixed device tree layout is adopted.
The LPC register offsets are also fixed accordingly.

Signed-off-by: Chia-Wei Wang <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
Tested-by: Andrew Jeffery <[email protected]>
---
drivers/soc/aspeed/aspeed-lpc-ctrl.c | 20 ++++++++++++++------
drivers/soc/aspeed/aspeed-lpc-snoop.c | 23 +++++++++++++++--------
2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/soc/aspeed/aspeed-lpc-ctrl.c b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
index 439bcd6b8c4a..c557ffd0992c 100644
--- a/drivers/soc/aspeed/aspeed-lpc-ctrl.c
+++ b/drivers/soc/aspeed/aspeed-lpc-ctrl.c
@@ -18,15 +18,15 @@

#define DEVICE_NAME "aspeed-lpc-ctrl"

-#define HICR5 0x0
+#define HICR5 0x80
#define HICR5_ENL2H BIT(8)
#define HICR5_ENFWH BIT(10)

-#define HICR6 0x4
+#define HICR6 0x84
#define SW_FWH2AHB BIT(17)

-#define HICR7 0x8
-#define HICR8 0xc
+#define HICR7 0x88
+#define HICR8 0x8c

struct aspeed_lpc_ctrl {
struct miscdevice miscdev;
@@ -215,6 +215,7 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
struct device_node *node;
struct resource resm;
struct device *dev;
+ struct device_node *np;
int rc;

dev = &pdev->dev;
@@ -270,8 +271,15 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
}
}

- lpc_ctrl->regmap = syscon_node_to_regmap(
- pdev->dev.parent->of_node);
+ np = pdev->dev.parent->of_node;
+ if (!of_device_is_compatible(np, "aspeed,ast2400-lpc-v2") &&
+ !of_device_is_compatible(np, "aspeed,ast2500-lpc-v2") &&
+ !of_device_is_compatible(np, "aspeed,ast2600-lpc-v2")) {
+ dev_err(dev, "unsupported LPC device binding\n");
+ return -ENODEV;
+ }
+
+ lpc_ctrl->regmap = syscon_node_to_regmap(np);
if (IS_ERR(lpc_ctrl->regmap)) {
dev_err(dev, "Couldn't get regmap\n");
return -ENODEV;
diff --git a/drivers/soc/aspeed/aspeed-lpc-snoop.c b/drivers/soc/aspeed/aspeed-lpc-snoop.c
index 682ba0eb4eba..ab0f0a54fea6 100644
--- a/drivers/soc/aspeed/aspeed-lpc-snoop.c
+++ b/drivers/soc/aspeed/aspeed-lpc-snoop.c
@@ -28,26 +28,25 @@
#define NUM_SNOOP_CHANNELS 2
#define SNOOP_FIFO_SIZE 2048

-#define HICR5 0x0
+#define HICR5 0x80
#define HICR5_EN_SNP0W BIT(0)
#define HICR5_ENINT_SNP0W BIT(1)
#define HICR5_EN_SNP1W BIT(2)
#define HICR5_ENINT_SNP1W BIT(3)
-
-#define HICR6 0x4
+#define HICR6 0x84
#define HICR6_STR_SNP0W BIT(0)
#define HICR6_STR_SNP1W BIT(1)
-#define SNPWADR 0x10
+#define SNPWADR 0x90
#define SNPWADR_CH0_MASK GENMASK(15, 0)
#define SNPWADR_CH0_SHIFT 0
#define SNPWADR_CH1_MASK GENMASK(31, 16)
#define SNPWADR_CH1_SHIFT 16
-#define SNPWDR 0x14
+#define SNPWDR 0x94
#define SNPWDR_CH0_MASK GENMASK(7, 0)
#define SNPWDR_CH0_SHIFT 0
#define SNPWDR_CH1_MASK GENMASK(15, 8)
#define SNPWDR_CH1_SHIFT 8
-#define HICRB 0x80
+#define HICRB 0x100
#define HICRB_ENSNP0D BIT(14)
#define HICRB_ENSNP1D BIT(15)

@@ -258,6 +257,7 @@ static int aspeed_lpc_snoop_probe(struct platform_device *pdev)
{
struct aspeed_lpc_snoop *lpc_snoop;
struct device *dev;
+ struct device_node *np;
u32 port;
int rc;

@@ -267,8 +267,15 @@ static int aspeed_lpc_snoop_probe(struct platform_device *pdev)
if (!lpc_snoop)
return -ENOMEM;

- lpc_snoop->regmap = syscon_node_to_regmap(
- pdev->dev.parent->of_node);
+ np = pdev->dev.parent->of_node;
+ if (!of_device_is_compatible(np, "aspeed,ast2400-lpc-v2") &&
+ !of_device_is_compatible(np, "aspeed,ast2500-lpc-v2") &&
+ !of_device_is_compatible(np, "aspeed,ast2600-lpc-v2")) {
+ dev_err(dev, "unsupported LPC device binding\n");
+ return -ENODEV;
+ }
+
+ lpc_snoop->regmap = syscon_node_to_regmap(np);
if (IS_ERR(lpc_snoop->regmap)) {
dev_err(dev, "Couldn't get regmap\n");
return -ENODEV;
--
2.27.0

2021-02-19 14:31:00

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH 04/19] pinctrl: aspeed-g5: Adapt to new LPC device tree layout

From: "Chia-Wei, Wang" <[email protected]>

Add check against LPC device v2 compatible string to
ensure that the fixed device tree layout is adopted.
The LPC register offsets are also fixed accordingly.

Signed-off-by: Chia-Wei Wang <[email protected]>
Reviewed-by: Andrew Jeffery <[email protected]>
Tested-by: Andrew Jeffery <[email protected]>
Acked-by: Linus Walleij <[email protected]>
---
drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
index 0cab4c2576e2..996ebcba4d38 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
@@ -60,7 +60,7 @@
#define COND2 { ASPEED_IP_SCU, SCU94, GENMASK(1, 0), 0, 0 }

/* LHCR0 is offset from the end of the H8S/2168-compatible registers */
-#define LHCR0 0x20
+#define LHCR0 0xa0
#define GFX064 0x64

#define B14 0
@@ -2648,14 +2648,19 @@ static struct regmap *aspeed_g5_acquire_regmap(struct aspeed_pinmux_data *ctx,
}

if (ip == ASPEED_IP_LPC) {
- struct device_node *node;
+ struct device_node *np;
struct regmap *map;

- node = of_parse_phandle(ctx->dev->of_node,
+ np = of_parse_phandle(ctx->dev->of_node,
"aspeed,external-nodes", 1);
- if (node) {
- map = syscon_node_to_regmap(node->parent);
- of_node_put(node);
+ if (np) {
+ if (!of_device_is_compatible(np->parent, "aspeed,ast2400-lpc-v2") &&
+ !of_device_is_compatible(np->parent, "aspeed,ast2500-lpc-v2") &&
+ !of_device_is_compatible(np->parent, "aspeed,ast2600-lpc-v2"))
+ return ERR_PTR(-ENODEV);
+
+ map = syscon_node_to_regmap(np->parent);
+ of_node_put(np);
if (IS_ERR(map))
return map;
} else
--
2.27.0

2021-02-19 14:31:18

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH 07/19] ipmi: kcs_bmc: Make status update atomic

Enable more efficient implementation of read-modify-write sequences.
Both device drivers for the KCS BMC stack use regmaps. The new callback
allows us to exploit regmap_update_bits().

Signed-off-by: Andrew Jeffery <[email protected]>
---
drivers/char/ipmi/kcs_bmc.c | 7 +------
drivers/char/ipmi/kcs_bmc.h | 1 +
drivers/char/ipmi/kcs_bmc_aspeed.c | 9 +++++++++
drivers/char/ipmi/kcs_bmc_npcm7xx.c | 10 ++++++++++
4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
index f292e74bd4a5..58fb1a7bd50d 100644
--- a/drivers/char/ipmi/kcs_bmc.c
+++ b/drivers/char/ipmi/kcs_bmc.c
@@ -67,12 +67,7 @@ static inline void write_status(struct kcs_bmc *kcs_bmc, u8 data)

static void update_status_bits(struct kcs_bmc *kcs_bmc, u8 mask, u8 val)
{
- u8 tmp = read_status(kcs_bmc);
-
- tmp &= ~mask;
- tmp |= val & mask;
-
- write_status(kcs_bmc, tmp);
+ kcs_bmc->io_updateb(kcs_bmc, kcs_bmc->ioreg.str, mask, val);
}

static inline void set_state(struct kcs_bmc *kcs_bmc, u8 state)
diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h
index eb9ea4ce78b8..970f53892f2d 100644
--- a/drivers/char/ipmi/kcs_bmc.h
+++ b/drivers/char/ipmi/kcs_bmc.h
@@ -76,6 +76,7 @@ struct kcs_bmc {
struct kcs_ioreg ioreg;
u8 (*io_inputb)(struct kcs_bmc *kcs_bmc, u32 reg);
void (*io_outputb)(struct kcs_bmc *kcs_bmc, u32 reg, u8 b);
+ void (*io_updateb)(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val);

enum kcs_phases phase;
enum kcs_errors error;
diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
index 061f53676206..630cf095560e 100644
--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
@@ -90,6 +90,14 @@ static void aspeed_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
WARN(rc != 0, "regmap_write() failed: %d\n", rc);
}

+static void aspeed_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 val)
+{
+ struct aspeed_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
+ int rc;
+
+ rc = regmap_update_bits(priv->map, reg, mask, val);
+ WARN(rc != 0, "regmap_update_bits() failed: %d\n", rc);
+}

/*
* AST_usrGuide_KCS.pdf
@@ -342,6 +350,7 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
kcs_bmc->ioreg = ast_kcs_bmc_ioregs[channel - 1];
kcs_bmc->io_inputb = aspeed_kcs_inb;
kcs_bmc->io_outputb = aspeed_kcs_outb;
+ kcs_bmc->io_updateb = aspeed_kcs_updateb;

addr = ops->get_io_address(pdev);
if (addr < 0)
diff --git a/drivers/char/ipmi/kcs_bmc_npcm7xx.c b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
index 722f7391fe1f..1f44aadec9e8 100644
--- a/drivers/char/ipmi/kcs_bmc_npcm7xx.c
+++ b/drivers/char/ipmi/kcs_bmc_npcm7xx.c
@@ -97,6 +97,15 @@ static void npcm7xx_kcs_outb(struct kcs_bmc *kcs_bmc, u32 reg, u8 data)
WARN(rc != 0, "regmap_write() failed: %d\n", rc);
}

+static void npcm7xx_kcs_updateb(struct kcs_bmc *kcs_bmc, u32 reg, u8 mask, u8 data)
+{
+ struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
+ int rc;
+
+ rc = regmap_update_bits(priv->map, reg, mask, data);
+ WARN(rc != 0, "regmap_update_bits() failed: %d\n", rc);
+}
+
static void npcm7xx_kcs_enable_channel(struct kcs_bmc *kcs_bmc, bool enable)
{
struct npcm7xx_kcs_bmc *priv = kcs_bmc_priv(kcs_bmc);
@@ -163,6 +172,7 @@ static int npcm7xx_kcs_probe(struct platform_device *pdev)
kcs_bmc->ioreg.str = priv->reg->sts;
kcs_bmc->io_inputb = npcm7xx_kcs_inb;
kcs_bmc->io_outputb = npcm7xx_kcs_outb;
+ kcs_bmc->io_updateb = npcm7xx_kcs_updateb;

dev_set_drvdata(dev, kcs_bmc);

--
2.27.0

2021-02-19 14:31:50

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH 17/19] dt-bindings: ipmi: Add optional SerIRQ property to ASPEED KCS devices

Allocating IO and IRQ resources to LPC devices is in-theory an operation
for the host, however ASPEED don't appear to expose this capability
outside the BMC (e.g. SuperIO). Instead, we are left with BMC-internal
registers for managing these resources, so introduce a devicetree
property for KCS devices to describe SerIRQ properties.

Signed-off-by: Andrew Jeffery <[email protected]>
---
.../bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
index 1c1cc4265948..808475a2c2ca 100644
--- a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
+++ b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
@@ -47,6 +47,18 @@ properties:
channels the status address is derived from the data address, but the
status address may be optionally provided.

+ aspeed,lpc-interrupts:
+ $ref: "/schemas/types.yaml#/definitions/uint32-matrix"
+ minItems: 1
+ maxItems: 1
+ description: |
+ A 2-cell property expressing the LPC SerIRQ number and the interrupt
+ level/sense encoding (specified in the standard fashion).
+
+ Note that the generated interrupt is issued from the BMC to the host, and
+ thus the target interrupt controller is not captured by the BMC's
+ devicetree.
+
kcs_chan:
deprecated: true
$ref: '/schemas/types.yaml#/definitions/uint32'
@@ -84,9 +96,11 @@ allOf:

examples:
- |
+ #include <dt-bindings/interrupt-controller/irq.h>
kcs3: kcs@24 {
compatible = "aspeed,ast2600-kcs-bmc";
reg = <0x24 0x1>, <0x30 0x1>, <0x3c 0x1>;
aspeed,lpc-io-reg = <0xca2>;
+ aspeed,lpc-interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
interrupts = <8>;
};
--
2.27.0

2021-02-19 14:36:09

by Andrew Jeffery

[permalink] [raw]
Subject: [PATCH 15/19] ipmi: kcs_bmc: Add a "raw" character device interface

The existing IPMI chardev encodes IPMI behaviours as the name suggests.
However, KCS devices are useful beyond IPMI (or keyboards), as they
provide a means to generate IRQs and exchange arbitrary data between a
BMC and its host system.

Implement a "raw" KCS character device that exposes the IDR, ODR and STR
registers to userspace via read() and write() implemented on a character
device:

+--------+--------+---------+
| Offset | read() | write() |
+--------+--------+---------+
| 0 | IDR | ODR |
+--------+--------+---------+
| 1 | STR | STR |
+--------+--------+---------+

This interface allows userspace to implement arbitrary (though somewhat
inefficient) protocols for exchanging information between a BMC and host
firmware. Conceptually the KCS interface can be used as an out-of-band
machanism for interrupt-signaled control messages while bulk data
transfers occur over more appropriate interfaces between the BMC and the
host (which may lack their own interrupt mechanism, e.g. LPC FW cycles).

poll() is provided, which will wait for IBF or OBE conditions for data
reads and writes respectively. Reads of STR on its own never blocks,
though accessing both offsets in the one system call may block if the
data registers are not ready.

Signed-off-by: Andrew Jeffery <[email protected]>
---
Documentation/ABI/testing/dev-raw-kcs | 25 ++
drivers/char/ipmi/Kconfig | 17 +
drivers/char/ipmi/Makefile | 1 +
drivers/char/ipmi/kcs_bmc_cdev_raw.c | 442 ++++++++++++++++++++++++++
4 files changed, 485 insertions(+)
create mode 100644 Documentation/ABI/testing/dev-raw-kcs
create mode 100644 drivers/char/ipmi/kcs_bmc_cdev_raw.c

diff --git a/Documentation/ABI/testing/dev-raw-kcs b/Documentation/ABI/testing/dev-raw-kcs
new file mode 100644
index 000000000000..06e7e2071562
--- /dev/null
+++ b/Documentation/ABI/testing/dev-raw-kcs
@@ -0,0 +1,25 @@
+What: /dev/raw-kcs*
+Date: 2021-02-15
+KernelVersion: 5.13
+Contact: [email protected]
+Contact: [email protected]
+Contact: Andrew Jeffery <[email protected]>
+Description: ``/dev/raw-kcs*`` exposes to userspace the data and
+ status registers of Keyboard-Controller-Style (KCS) IPMI
+ interfaces via read() and write() syscalls. Direct
+ exposure of the data and status registers enables
+ inefficient but arbitrary protocols to be implemented
+ over the device. A typical approach is to use KCS
+ devices for out-of-band signalling for bulk data
+ transfers over other interfaces between a Baseboard
+ Management Controller and its host.
+
+ +--------+--------+---------+
+ | Offset | read() | write() |
+ +--------+--------+---------+
+ | 0 | IDR | ODR |
+ +--------+--------+---------+
+ | 1 | STR | STR |
+ +--------+--------+---------+
+
+Users: libmctp: https://github.com/openbmc/libmctp
diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index bc5f81899b62..273ac1a1f870 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -137,6 +137,23 @@ config IPMI_KCS_BMC_CDEV_IPMI
This support is also available as a module. The module will be
called kcs_bmc_cdev_ipmi.

+config IPMI_KCS_BMC_CDEV_RAW
+ depends on IPMI_KCS_BMC
+ tristate "Raw character device interface for BMC KCS devices"
+ help
+ Provides a BMC-side character device directly exposing the
+ data and status registers of a KCS device to userspace. While
+ KCS devices are commonly used to implement IPMI message
+ passing, they provide a general interface for exchange of
+ interrupts, data and status information between the BMC and
+ its host.
+
+ Say YES if you wish to use the KCS devices to implement
+ protocols that are not IPMI.
+
+ This support is also available as a module. The module will be
+ called kcs_bmc_cdev_raw.
+
config ASPEED_BT_IPMI_BMC
depends on ARCH_ASPEED || COMPILE_TEST
depends on REGMAP && REGMAP_MMIO && MFD_SYSCON
diff --git a/drivers/char/ipmi/Makefile b/drivers/char/ipmi/Makefile
index fcfa676afddb..c8cc248ddd90 100644
--- a/drivers/char/ipmi/Makefile
+++ b/drivers/char/ipmi/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
obj-$(CONFIG_IPMI_KCS_BMC) += kcs_bmc.o
obj-$(CONFIG_IPMI_KCS_BMC_CDEV_IPMI) += kcs_bmc_cdev_ipmi.o
+obj-$(CONFIG_IPMI_KCS_BMC_CDEV_RAW) += kcs_bmc_cdev_raw.o
obj-$(CONFIG_ASPEED_BT_IPMI_BMC) += bt-bmc.o
obj-$(CONFIG_ASPEED_KCS_IPMI_BMC) += kcs_bmc_aspeed.o
obj-$(CONFIG_NPCM7XX_KCS_IPMI_BMC) += kcs_bmc_npcm7xx.o
diff --git a/drivers/char/ipmi/kcs_bmc_cdev_raw.c b/drivers/char/ipmi/kcs_bmc_cdev_raw.c
new file mode 100644
index 000000000000..7580f432e8d8
--- /dev/null
+++ b/drivers/char/ipmi/kcs_bmc_cdev_raw.c
@@ -0,0 +1,442 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (c) 2021 IBM Corp. */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/list.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/poll.h>
+
+#include "kcs_bmc_client.h"
+
+#define DEVICE_NAME "raw-kcs"
+
+struct kcs_bmc_raw {
+ struct list_head entry;
+
+ struct kcs_bmc_client client;
+
+ wait_queue_head_t queue;
+ u8 events;
+ bool writable;
+ bool readable;
+ u8 idr;
+
+ struct miscdevice miscdev;
+};
+
+static inline struct kcs_bmc_raw *client_to_kcs_bmc_raw(struct kcs_bmc_client *client)
+{
+ return container_of(client, struct kcs_bmc_raw, client);
+}
+
+/* Call under priv->queue.lock */
+static void kcs_bmc_raw_update_event_mask(struct kcs_bmc_raw *priv, u8 mask, u8 state)
+{
+ kcs_bmc_update_event_mask(priv->client.dev, mask, state);
+ priv->events &= ~mask;
+ priv->events |= state & mask;
+}
+
+static int kcs_bmc_raw_event(struct kcs_bmc_client *client)
+{
+ struct kcs_bmc_raw *priv;
+ struct device *dev;
+ u8 status, handled;
+
+ priv = client_to_kcs_bmc_raw(client);
+ dev = priv->miscdev.this_device;
+
+ spin_lock(&priv->queue.lock);
+
+ status = kcs_bmc_read_status(client->dev);
+ handled = 0;
+
+ if ((priv->events & KCS_BMC_EVENT_TYPE_IBF) && (status & KCS_BMC_STR_IBF)) {
+ if (priv->readable)
+ dev_err(dev, "Storm brewing!");
+
+ dev_dbg(dev, "Disabling IDR events for back-pressure\n");
+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_IBF, 0);
+ priv->idr = kcs_bmc_read_data(client->dev);
+ priv->readable = true;
+
+ dev_dbg(dev, "IDR read, waking waiters\n");
+ wake_up_locked(&priv->queue);
+
+ handled |= KCS_BMC_EVENT_TYPE_IBF;
+ }
+
+ if ((priv->events & KCS_BMC_EVENT_TYPE_OBE) && !(status & KCS_BMC_STR_OBF)) {
+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_OBE, 0);
+ priv->writable = true;
+
+ dev_dbg(dev, "ODR writable, waking waiters\n");
+ wake_up_locked(&priv->queue);
+
+ handled |= KCS_BMC_EVENT_TYPE_OBE;
+ }
+
+ spin_unlock(&priv->queue.lock);
+
+ return handled ? KCS_BMC_EVENT_HANDLED : KCS_BMC_EVENT_NONE;
+}
+
+static const struct kcs_bmc_client_ops kcs_bmc_raw_client_ops = {
+ .event = kcs_bmc_raw_event,
+};
+
+static inline struct kcs_bmc_raw *file_to_kcs_bmc_raw(struct file *filp)
+{
+ return container_of(filp->private_data, struct kcs_bmc_raw, miscdev);
+}
+
+static int kcs_bmc_raw_open(struct inode *inode, struct file *filp)
+{
+ struct kcs_bmc_raw *priv = file_to_kcs_bmc_raw(filp);
+
+ return kcs_bmc_enable_device(priv->client.dev, &priv->client);
+}
+
+static bool kcs_bmc_raw_prepare_obe(struct kcs_bmc_raw *priv)
+{
+ bool writable;
+
+ /* Enable the OBE event so we can catch the host clearing OBF */
+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_OBE, KCS_BMC_EVENT_TYPE_OBE);
+
+ /* Now that we'll catch an OBE event, check if it's already occurred */
+ writable = !(kcs_bmc_read_status(priv->client.dev) & KCS_BMC_STR_OBF);
+
+ /* If OBF is clear we've missed the OBE event, so disable it */
+ if (writable)
+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_OBE, 0);
+
+ return writable;
+}
+
+static __poll_t kcs_bmc_raw_poll(struct file *filp, poll_table *wait)
+{
+ struct kcs_bmc_raw *priv;
+ __poll_t events = 0;
+
+ priv = file_to_kcs_bmc_raw(filp);
+
+ poll_wait(filp, &priv->queue, wait);
+
+ spin_lock_irq(&priv->queue.lock);
+ if (kcs_bmc_raw_prepare_obe(priv))
+ events |= (EPOLLOUT | EPOLLWRNORM);
+
+ if (priv->readable || (kcs_bmc_read_status(priv->client.dev) & KCS_BMC_STR_IBF))
+ events |= (EPOLLIN | EPOLLRDNORM);
+ spin_unlock_irq(&priv->queue.lock);
+
+ return events;
+}
+
+static ssize_t kcs_bmc_raw_read(struct file *filp, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct kcs_bmc_device *kcs_bmc;
+ struct kcs_bmc_raw *priv;
+ bool read_idr, read_str;
+ struct device *dev;
+ u8 idr, str;
+ ssize_t rc;
+
+ priv = file_to_kcs_bmc_raw(filp);
+ kcs_bmc = priv->client.dev;
+ dev = priv->miscdev.this_device;
+
+ if (!count)
+ return 0;
+
+ if (count > 2 || *ppos > 1)
+ return -EINVAL;
+
+ if (*ppos + count > 2)
+ return -EINVAL;
+
+ read_idr = (*ppos == 0);
+ read_str = (*ppos == 1) || (count == 2);
+
+ spin_lock_irq(&priv->queue.lock);
+ if (read_idr) {
+ dev_dbg(dev, "Waiting for IBF\n");
+ str = kcs_bmc_read_status(kcs_bmc);
+ if ((filp->f_flags & O_NONBLOCK) && (str & KCS_BMC_STR_IBF)) {
+ rc = -EWOULDBLOCK;
+ goto out;
+ }
+
+ rc = wait_event_interruptible_locked(priv->queue,
+ priv->readable || (str & KCS_BMC_STR_IBF));
+ if (rc < 0)
+ goto out;
+
+ if (signal_pending(current)) {
+ dev_dbg(dev, "Interrupted waiting for IBF\n");
+ rc = -EINTR;
+ goto out;
+ }
+
+ /*
+ * Re-enable events prior to possible read of IDR (which clears
+ * IBF) to ensure we receive interrupts for subsequent writes
+ * to IDR. Writes to IDR by the host should not occur while IBF
+ * is set.
+ */
+ dev_dbg(dev, "Woken by IBF, enabling IRQ\n");
+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_IBF,
+ KCS_BMC_EVENT_TYPE_IBF);
+
+ /* Read data out of IDR into internal storage if necessary */
+ if (!priv->readable) {
+ WARN(!(str & KCS_BMC_STR_IBF), "Unknown reason for wakeup!");
+
+ priv->idr = kcs_bmc_read_data(kcs_bmc);
+ }
+
+ /* Copy data from internal storage to userspace */
+ idr = priv->idr;
+
+ /* We're done consuming the internally stored value */
+ priv->readable = false;
+ }
+
+ if (read_str) {
+ str = kcs_bmc_read_status(kcs_bmc);
+ if (*ppos == 0 || priv->readable)
+ /*
+ * If we got this far with `*ppos == 0` then we've read
+ * data out of IDR, so set IBF when reporting back to
+ * userspace so userspace knows the IDR value is valid.
+ */
+ str |= KCS_BMC_STR_IBF;
+
+ dev_dbg(dev, "Read status 0x%x\n", str);
+
+ }
+
+ rc = count;
+out:
+ spin_unlock_irq(&priv->queue.lock);
+
+ if (rc < 0)
+ return rc;
+
+ /* Now copy the data in to the userspace buffer */
+
+ if (read_idr)
+ if (copy_to_user(buf++, &idr, sizeof(idr)))
+ return -EFAULT;
+
+ if (read_str)
+ if (copy_to_user(buf, &str, sizeof(str)))
+ return -EFAULT;
+
+ return count;
+}
+
+static ssize_t kcs_bmc_raw_write(struct file *filp, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct kcs_bmc_device *kcs_bmc;
+ bool write_odr, write_str;
+ struct kcs_bmc_raw *priv;
+ struct device *dev;
+ uint8_t data[2];
+ ssize_t result;
+ u8 str;
+
+ priv = file_to_kcs_bmc_raw(filp);
+ kcs_bmc = priv->client.dev;
+ dev = priv->miscdev.this_device;
+
+ if (!count)
+ return count;
+
+ if (count > 2)
+ return -EINVAL;
+
+ if (*ppos >= 2)
+ return -EINVAL;
+
+ if (*ppos + count > 2)
+ return -EINVAL;
+
+ if (copy_from_user(data, buf, count))
+ return -EFAULT;
+
+ write_odr = (*ppos == 0);
+ write_str = (*ppos == 1) || (count == 2);
+
+ spin_lock_irq(&priv->queue.lock);
+
+ /* Always write status before data, we generate the SerIRQ by writing ODR */
+ if (write_str) {
+ /* The index of STR in the userspace buffer depends on whether ODR is written */
+ str = data[*ppos == 0];
+ if (!(str & KCS_BMC_STR_OBF))
+ dev_warn(dev, "Clearing OBF with status write: 0x%x\n", str);
+ dev_dbg(dev, "Writing status 0x%x\n", str);
+ kcs_bmc_write_status(kcs_bmc, str);
+ }
+
+ if (write_odr) {
+ /* If we're writing ODR it's always the first byte in the buffer */
+ u8 odr = data[0];
+
+ str = kcs_bmc_read_status(kcs_bmc);
+ if (str & KCS_BMC_STR_OBF) {
+ if (filp->f_flags & O_NONBLOCK) {
+ result = -EWOULDBLOCK;
+ goto out;
+ }
+
+ priv->writable = kcs_bmc_raw_prepare_obe(priv);
+
+ /* Now either OBF is already clear, or we'll get an OBE event to wake us */
+ dev_dbg(dev, "Waiting for OBF to clear\n");
+ wait_event_interruptible_locked(priv->queue, priv->writable);
+
+ if (signal_pending(current)) {
+ kcs_bmc_raw_update_event_mask(priv, KCS_BMC_EVENT_TYPE_OBE, 0);
+ result = -EINTR;
+ goto out;
+ }
+
+ WARN_ON(kcs_bmc_read_status(kcs_bmc) & KCS_BMC_STR_OBF);
+ }
+
+ dev_dbg(dev, "Writing 0x%x to ODR\n", odr);
+ kcs_bmc_write_data(kcs_bmc, odr);
+ }
+
+out:
+ spin_unlock_irq(&priv->queue.lock);
+
+ return count;
+}
+
+static int kcs_bmc_raw_release(struct inode *inode, struct file *filp)
+{
+ struct kcs_bmc_raw *priv = file_to_kcs_bmc_raw(filp);
+
+ kcs_bmc_disable_device(priv->client.dev, &priv->client);
+
+ return 0;
+}
+
+static const struct file_operations kcs_bmc_raw_fops = {
+ .owner = THIS_MODULE,
+ .open = kcs_bmc_raw_open,
+ .llseek = no_seek_end_llseek,
+ .read = kcs_bmc_raw_read,
+ .write = kcs_bmc_raw_write,
+ .poll = kcs_bmc_raw_poll,
+ .release = kcs_bmc_raw_release,
+};
+
+static DEFINE_SPINLOCK(kcs_bmc_raw_instances_lock);
+static LIST_HEAD(kcs_bmc_raw_instances);
+
+static int kcs_bmc_raw_attach_cdev(struct kcs_bmc_device *kcs_bmc)
+{
+ struct kcs_bmc_raw *priv;
+ int rc;
+
+ priv = devm_kzalloc(kcs_bmc->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->client.dev = kcs_bmc;
+ priv->client.ops = &kcs_bmc_raw_client_ops;
+
+ init_waitqueue_head(&priv->queue);
+ priv->writable = false;
+ priv->readable = false;
+
+ priv->miscdev.minor = MISC_DYNAMIC_MINOR;
+ priv->miscdev.name = devm_kasprintf(kcs_bmc->dev, GFP_KERNEL, "%s%u", DEVICE_NAME,
+ kcs_bmc->channel);
+ if (!priv->miscdev.name)
+ return -EINVAL;
+
+ priv->miscdev.fops = &kcs_bmc_raw_fops;
+
+ /* Initialise our expected events. Listen for IBF but ignore OBE until necessary */
+ kcs_bmc_raw_update_event_mask(priv, (KCS_BMC_EVENT_TYPE_IBF | KCS_BMC_EVENT_TYPE_OBE),
+ KCS_BMC_EVENT_TYPE_IBF);
+
+ rc = misc_register(&priv->miscdev);
+ if (rc) {
+ dev_err(kcs_bmc->dev, "Unable to register device\n");
+ return rc;
+ }
+
+ spin_lock_irq(&kcs_bmc_raw_instances_lock);
+ list_add(&priv->entry, &kcs_bmc_raw_instances);
+ spin_unlock_irq(&kcs_bmc_raw_instances_lock);
+
+ dev_info(kcs_bmc->dev, "Initialised raw client for channel %d", kcs_bmc->channel);
+
+ return 0;
+}
+
+static int kcs_bmc_raw_detach_cdev(struct kcs_bmc_device *kcs_bmc)
+{
+ struct kcs_bmc_raw *priv, *pos;
+
+ spin_lock_irq(&kcs_bmc_raw_instances_lock);
+ list_for_each_entry(pos, &kcs_bmc_raw_instances, entry) {
+ if (pos->client.dev == kcs_bmc) {
+ priv = pos;
+ list_del(&pos->entry);
+ break;
+ }
+ }
+ spin_unlock_irq(&kcs_bmc_raw_instances_lock);
+
+ if (!priv)
+ return 0;
+
+ misc_deregister(&priv->miscdev);
+ kcs_bmc_disable_device(kcs_bmc, &priv->client);
+ devm_kfree(priv->client.dev->dev, priv);
+
+ return 0;
+}
+
+static const struct kcs_bmc_cdev_ops kcs_bmc_raw_cdev_ops = {
+ .add_device = kcs_bmc_raw_attach_cdev,
+ .remove_device = kcs_bmc_raw_detach_cdev,
+};
+
+static struct kcs_bmc_cdev kcs_bmc_raw_cdev = {
+ .ops = &kcs_bmc_raw_cdev_ops,
+};
+
+static int kcs_bmc_raw_init(void)
+{
+ return kcs_bmc_register_cdev(&kcs_bmc_raw_cdev);
+}
+module_init(kcs_bmc_raw_init);
+
+static void kcs_bmc_raw_exit(void)
+{
+ int rc;
+
+ rc = kcs_bmc_unregister_cdev(&kcs_bmc_raw_cdev);
+ if (rc)
+ pr_warn("Failed to remove KCS BMC client: %d", rc);
+}
+module_exit(kcs_bmc_raw_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Andrew Jeffery <[email protected]>");
+MODULE_DESCRIPTION("Character device for raw access to a KCS device");
--
2.27.0

2021-03-05 23:11:49

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 17/19] dt-bindings: ipmi: Add optional SerIRQ property to ASPEED KCS devices

On Sat, Feb 20, 2021 at 12:55:21AM +1030, Andrew Jeffery wrote:
> Allocating IO and IRQ resources to LPC devices is in-theory an operation
> for the host, however ASPEED don't appear to expose this capability
> outside the BMC (e.g. SuperIO). Instead, we are left with BMC-internal
> registers for managing these resources, so introduce a devicetree
> property for KCS devices to describe SerIRQ properties.
>
> Signed-off-by: Andrew Jeffery <[email protected]>
> ---
> .../bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
> index 1c1cc4265948..808475a2c2ca 100644
> --- a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
> +++ b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
> @@ -47,6 +47,18 @@ properties:
> channels the status address is derived from the data address, but the
> status address may be optionally provided.
>
> + aspeed,lpc-interrupts:
> + $ref: "/schemas/types.yaml#/definitions/uint32-matrix"
> + minItems: 1
> + maxItems: 1
> + description: |
> + A 2-cell property expressing the LPC SerIRQ number and the interrupt
> + level/sense encoding (specified in the standard fashion).

That would be uint32-array with 'maxItems: 2'.

> +
> + Note that the generated interrupt is issued from the BMC to the host, and
> + thus the target interrupt controller is not captured by the BMC's
> + devicetree.
> +
> kcs_chan:
> deprecated: true
> $ref: '/schemas/types.yaml#/definitions/uint32'
> @@ -84,9 +96,11 @@ allOf:
>
> examples:
> - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> kcs3: kcs@24 {
> compatible = "aspeed,ast2600-kcs-bmc";
> reg = <0x24 0x1>, <0x30 0x1>, <0x3c 0x1>;
> aspeed,lpc-io-reg = <0xca2>;
> + aspeed,lpc-interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> interrupts = <8>;
> };
> --
> 2.27.0
>

2021-03-09 02:50:02

by Andrew Jeffery

[permalink] [raw]
Subject: Re: [PATCH 17/19] dt-bindings: ipmi: Add optional SerIRQ property to ASPEED KCS devices



On Sat, 6 Mar 2021, at 09:39, Rob Herring wrote:
> On Sat, Feb 20, 2021 at 12:55:21AM +1030, Andrew Jeffery wrote:
> > Allocating IO and IRQ resources to LPC devices is in-theory an operation
> > for the host, however ASPEED don't appear to expose this capability
> > outside the BMC (e.g. SuperIO). Instead, we are left with BMC-internal
> > registers for managing these resources, so introduce a devicetree
> > property for KCS devices to describe SerIRQ properties.
> >
> > Signed-off-by: Andrew Jeffery <[email protected]>
> > ---
> > .../bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
> > index 1c1cc4265948..808475a2c2ca 100644
> > --- a/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
> > +++ b/Documentation/devicetree/bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml
> > @@ -47,6 +47,18 @@ properties:
> > channels the status address is derived from the data address, but the
> > status address may be optionally provided.
> >
> > + aspeed,lpc-interrupts:
> > + $ref: "/schemas/types.yaml#/definitions/uint32-matrix"
> > + minItems: 1
> > + maxItems: 1
> > + description: |
> > + A 2-cell property expressing the LPC SerIRQ number and the interrupt
> > + level/sense encoding (specified in the standard fashion).
>
> That would be uint32-array with 'maxItems: 2'.
>

Ah, thanks.