Hello,
This is the 3rd spin of the series refactoring the keyboard-controller-style
device drivers in the IPMI subsystem.
v2 can be found (in two parts because yay patch workflow mistakes) at:
Cover letter:
https://lore.kernel.org/linux-arm-kernel/[email protected]/
Patches:
https://lore.kernel.org/linux-arm-kernel/[email protected]/
Several significant changes in v3:
1. The series is rebased onto v5.13-rc1
2. v5.13-rc1 includes Chiawei's patches reworking the LPC devicetree bindings,
so they're no-longer required in the series.
3. After some discussion with Arnd[1] and investigating the serio subsystem,
I've replaced the "raw" KCS driver (patch 16/21 in v2) with a serio adaptor
(patch 11/16 in this series). The adaptor allows us to take advantage of the
existing chardevs provided by serio.
[1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
Finally, I've also addressed Zev Weiss' review comments where I thought it was
required. These comments covered a lot of minor issues across (almost) all the
patches, so it's best to review from a clean slate rather than attempt to review
the differences between spins.
Previously:
Changes in v2 include:
* A rebase onto v5.12-rc2
* Incorporation of off-list feedback on SerIRQ configuration from
Chiawei
* Further validation on hardware for ASPEED KCS devices 2, 3 and 4
* Lifting the existing single-open constraint of the IPMI chardev
* Fixes addressing Rob's feedback on the conversion of the ASPEED KCS
binding to dt-schema
* Fixes addressing Rob's feedback on the new aspeed,lpc-interrupts
property definition for the ASPEED KCS binding
Please test and review!
Andrew
Andrew Jeffery (16):
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: Don't enforce single-open policy in the kernel
ipmi: kcs_bmc: Add serio adaptor
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
ipmi: kcs_bmc_aspeed: Optionally apply status address
.../bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 106 +++
.../bindings/ipmi/aspeed-kcs-bmc.txt | 33 -
drivers/char/ipmi/Kconfig | 27 +
drivers/char/ipmi/Makefile | 2 +
drivers/char/ipmi/kcs_bmc.c | 526 ++++-----------
drivers/char/ipmi/kcs_bmc.h | 92 +--
drivers/char/ipmi/kcs_bmc_aspeed.c | 635 +++++++++++++-----
drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 568 ++++++++++++++++
drivers/char/ipmi/kcs_bmc_client.h | 48 ++
drivers/char/ipmi/kcs_bmc_device.h | 22 +
drivers/char/ipmi/kcs_bmc_npcm7xx.c | 94 ++-
drivers/char/ipmi/kcs_bmc_serio.c | 151 +++++
12 files changed, 1582 insertions(+), 722 deletions(-)
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_client.h
create mode 100644 drivers/char/ipmi/kcs_bmc_device.h
create mode 100644 drivers/char/ipmi/kcs_bmc_serio.c
--
2.27.0
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]>
Reviewed-by: Zev Weiss <[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 c94d36e195be..06628ca69750 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
@@ -343,6 +351,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
Rename the functions in preparation for separating the IPMI chardev out
from the KCS BMC core.
Signed-off-by: Andrew Jeffery <[email protected]>
Reviewed-by: Zev Weiss <[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
Hi Corey,
On Mon, 10 May 2021, at 15:11, Andrew Jeffery wrote:
> Hello,
>
> This is the 3rd spin of the series refactoring the keyboard-controller-style
> device drivers in the IPMI subsystem.
>
> v2 can be found (in two parts because yay patch workflow mistakes) at:
>
> Cover letter:
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> Patches:
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> Several significant changes in v3:
>
> 1. The series is rebased onto v5.13-rc1
>
> 2. v5.13-rc1 includes Chiawei's patches reworking the LPC devicetree bindings,
> so they're no-longer required in the series.
>
> 3. After some discussion with Arnd[1] and investigating the serio subsystem,
> I've replaced the "raw" KCS driver (patch 16/21 in v2) with a serio adaptor
> (patch 11/16 in this series). The adaptor allows us to take advantage of the
> existing chardevs provided by serio.
>
> [1]
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> Finally, I've also addressed Zev Weiss' review comments where I thought it was
> required. These comments covered a lot of minor issues across (almost) all the
> patches, so it's best to review from a clean slate rather than attempt to review
> the differences between spins.
I backported this series for OpenBMC and posting those patches provoked
some feedback:
* A bug identified in patch 9/18 for the Nuvoton driver where we enable
the OBE interrupt:
https://lore.kernel.org/openbmc/HK2PR03MB4371F006185ADBBF812A5892AE509@HK2PR03MB4371.apcprd03.prod.outlook.com/
* A discussion on patch 10/18 about lifting the single-open constraint
https://lore.kernel.org/openbmc/CAPnigKku-EjOnV9gsmnXzH=XZxSU78iLeccNbsK8k2_4b4UwSg@mail.gmail.com/
I need to do a v4 to fix the bug in the Nuvoton driver. Did you have any
feedback for the remaining patches or thoughts on the discussions linked
above? I'd like to incorporate whatever I can into the series before
respinning.
Cheers,
Andrew
On Thu, May 20, 2021 at 04:21:31PM +0930, Andrew Jeffery wrote:
> Hi Corey,
>
> On Mon, 10 May 2021, at 15:11, Andrew Jeffery wrote:
> > Hello,
> >
> > This is the 3rd spin of the series refactoring the keyboard-controller-style
> > device drivers in the IPMI subsystem.
> >
> > v2 can be found (in two parts because yay patch workflow mistakes) at:
> >
> > Cover letter:
> > https://lore.kernel.org/linux-arm-kernel/[email protected]/
> >
> > Patches:
> > https://lore.kernel.org/linux-arm-kernel/[email protected]/
> >
> > Several significant changes in v3:
> >
> > 1. The series is rebased onto v5.13-rc1
> >
> > 2. v5.13-rc1 includes Chiawei's patches reworking the LPC devicetree bindings,
> > so they're no-longer required in the series.
> >
> > 3. After some discussion with Arnd[1] and investigating the serio subsystem,
> > I've replaced the "raw" KCS driver (patch 16/21 in v2) with a serio adaptor
> > (patch 11/16 in this series). The adaptor allows us to take advantage of the
> > existing chardevs provided by serio.
> >
> > [1]
> > https://lore.kernel.org/linux-arm-kernel/[email protected]/
> >
> > Finally, I've also addressed Zev Weiss' review comments where I thought it was
> > required. These comments covered a lot of minor issues across (almost) all the
> > patches, so it's best to review from a clean slate rather than attempt to review
> > the differences between spins.
>
> I backported this series for OpenBMC and posting those patches provoked
> some feedback:
>
> * A bug identified in patch 9/18 for the Nuvoton driver where we enable
> the OBE interrupt:
>
> https://lore.kernel.org/openbmc/HK2PR03MB4371F006185ADBBF812A5892AE509@HK2PR03MB4371.apcprd03.prod.outlook.com/
>
> * A discussion on patch 10/18 about lifting the single-open constraint
>
> https://lore.kernel.org/openbmc/CAPnigKku-EjOnV9gsmnXzH=XZxSU78iLeccNbsK8k2_4b4UwSg@mail.gmail.com/
>
> I need to do a v4 to fix the bug in the Nuvoton driver. Did you have any
> feedback for the remaining patches or thoughts on the discussions linked
> above? I'd like to incorporate whatever I can into the series before
> respinning.
This will take a little while to review, but I'll try to get to it
today.
Thanks,
-corey
>
> Cheers,
>
> Andrew
On Mon, May 10, 2021 at 03:11:57PM +0930, Andrew Jeffery wrote:
> Hello,
>
> This is the 3rd spin of the series refactoring the keyboard-controller-style
> device drivers in the IPMI subsystem.
This is a nice set of cleanups outside of just allowing raw access.
I'll let you handle Zev's comments and a few of mine.
I almost hate to ask this, but would there be value in allowing the BT
driver to use this abstract interface? Or maybe it would be just too
hard to get a common abstraction, more work than it's worth. It's
surprising that more people don't want BT as it's vastly superior to
KCS. Just a thought for now. I guess there's SMIC, but hopefully
nobody wants that.
-corey
>
> v2 can be found (in two parts because yay patch workflow mistakes) at:
>
> Cover letter:
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> Patches:
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> Several significant changes in v3:
>
> 1. The series is rebased onto v5.13-rc1
>
> 2. v5.13-rc1 includes Chiawei's patches reworking the LPC devicetree bindings,
> so they're no-longer required in the series.
>
> 3. After some discussion with Arnd[1] and investigating the serio subsystem,
> I've replaced the "raw" KCS driver (patch 16/21 in v2) with a serio adaptor
> (patch 11/16 in this series). The adaptor allows us to take advantage of the
> existing chardevs provided by serio.
>
> [1] https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> Finally, I've also addressed Zev Weiss' review comments where I thought it was
> required. These comments covered a lot of minor issues across (almost) all the
> patches, so it's best to review from a clean slate rather than attempt to review
> the differences between spins.
>
> Previously:
>
> Changes in v2 include:
>
> * A rebase onto v5.12-rc2
> * Incorporation of off-list feedback on SerIRQ configuration from
> Chiawei
> * Further validation on hardware for ASPEED KCS devices 2, 3 and 4
> * Lifting the existing single-open constraint of the IPMI chardev
> * Fixes addressing Rob's feedback on the conversion of the ASPEED KCS
> binding to dt-schema
> * Fixes addressing Rob's feedback on the new aspeed,lpc-interrupts
> property definition for the ASPEED KCS binding
>
> Please test and review!
>
> Andrew
>
> Andrew Jeffery (16):
> 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: Don't enforce single-open policy in the kernel
> ipmi: kcs_bmc: Add serio adaptor
> 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
> ipmi: kcs_bmc_aspeed: Optionally apply status address
>
> .../bindings/ipmi/aspeed,ast2400-kcs-bmc.yaml | 106 +++
> .../bindings/ipmi/aspeed-kcs-bmc.txt | 33 -
> drivers/char/ipmi/Kconfig | 27 +
> drivers/char/ipmi/Makefile | 2 +
> drivers/char/ipmi/kcs_bmc.c | 526 ++++-----------
> drivers/char/ipmi/kcs_bmc.h | 92 +--
> drivers/char/ipmi/kcs_bmc_aspeed.c | 635 +++++++++++++-----
> drivers/char/ipmi/kcs_bmc_cdev_ipmi.c | 568 ++++++++++++++++
> drivers/char/ipmi/kcs_bmc_client.h | 48 ++
> drivers/char/ipmi/kcs_bmc_device.h | 22 +
> drivers/char/ipmi/kcs_bmc_npcm7xx.c | 94 ++-
> drivers/char/ipmi/kcs_bmc_serio.c | 151 +++++
> 12 files changed, 1582 insertions(+), 722 deletions(-)
> 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_client.h
> create mode 100644 drivers/char/ipmi/kcs_bmc_device.h
> create mode 100644 drivers/char/ipmi/kcs_bmc_serio.c
>
> --
> 2.27.0
>
Hi Corey,
On Sat, 22 May 2021, at 03:06, Corey Minyard wrote:
> On Mon, May 10, 2021 at 03:11:57PM +0930, Andrew Jeffery wrote:
> > Hello,
> >
> > This is the 3rd spin of the series refactoring the keyboard-controller-style
> > device drivers in the IPMI subsystem.
>
> This is a nice set of cleanups outside of just allowing raw access.
> I'll let you handle Zev's comments and a few of mine.
Thanks for taking the time to review the series. I'll address the
comments from you both in v4.
>
> I almost hate to ask this, but would there be value in allowing the BT
> driver to use this abstract interface?
Hmm. Possibly, but it's not something I've looked at yet. If we did
want to go down that path I don't think it would be too difficult, but
I don't have a need to touch the BT side of it right now.
> Or maybe it would be just too
> hard to get a common abstraction, more work than it's worth. It's
> surprising that more people don't want BT as it's vastly superior to
> KCS.
For bulk data, certainly. However for the use-cases I have I'm using
the KCS interface as a control channel that isn't data intensive.
Interrupts, a small command set (256 values are more than enough) and a
status byte are all I'm really after, so BT is more than I need.
Plus for the systems I'm working on we're still using BT for in-band
IPMI while we transition to MCTP/PLDM. The current BT implementation is
working fine for that :)
Cheers,
Andrew