linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: "Andrew Jeffery" <andrew@aj.id.au>
To: "Zev Weiss" <zweiss@equinix.com>
Cc: "openipmi-developer@lists.sourceforge.net"
	<openipmi-developer@lists.sourceforge.net>,
	 "openbmc@lists.ozlabs.org" <openbmc@lists.ozlabs.org>,
	"Corey Minyard" <minyard@acm.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"Tomer Maimon" <tmaimon77@gmail.com>,
	"linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
	"Avi Fishman" <avifishman70@gmail.com>,
	"Patrick Venture" <venture@google.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Tali Perry" <tali.perry1@gmail.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Chia-Wei, Wang" <chiawei_wang@aspeedtech.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	 "Benjamin Fair" <benjaminfair@google.com>,
	"Arnd Bergmann" <arnd@arndb.de>
Subject: Re: [PATCH v3 14/16] ipmi: kcs_bmc_aspeed: Implement KCS SerIRQ configuration
Date: Tue, 08 Jun 2021 10:25:04 +0930	[thread overview]
Message-ID: <e5487cab-2ca0-46c2-a644-46d8b3070eac@www.fastmail.com> (raw)
In-Reply-To: <df0d31e9-c4e1-4cf0-b800-c5dadfb43420@www.fastmail.com>



On Tue, 8 Jun 2021, at 10:11, Andrew Jeffery wrote:
> 
> 
> On Fri, 21 May 2021, at 16:51, Zev Weiss wrote:
> > On Mon, May 10, 2021 at 12:42:11AM CDT, Andrew Jeffery wrote:
> > >Apply the SerIRQ ID and level/sense behaviours from the devicetree if
> > >provided.
> > >
> > >Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > >---
> > > drivers/char/ipmi/kcs_bmc_aspeed.c | 182 ++++++++++++++++++++++++++++-
> > > 1 file changed, 180 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/drivers/char/ipmi/kcs_bmc_aspeed.c b/drivers/char/ipmi/kcs_bmc_aspeed.c
> > >index 8a0b1e18e945..9b81806b4dcb 100644
> > >--- a/drivers/char/ipmi/kcs_bmc_aspeed.c
> > >+++ b/drivers/char/ipmi/kcs_bmc_aspeed.c
> > >@@ -9,6 +9,7 @@
> > > #include <linux/errno.h>
> > > #include <linux/interrupt.h>
> > > #include <linux/io.h>
> > >+#include <linux/irq.h>
> > > #include <linux/mfd/syscon.h>
> > > #include <linux/module.h>
> > > #include <linux/of.h>
> > >@@ -28,6 +29,22 @@
> > >
> > > #define KCS_CHANNEL_MAX     4
> > >
> > >+/*
> > >+ * Field class descriptions
> > >+ *
> > >+ * LPCyE	Enable LPC channel y
> > >+ * IBFIEy	Input Buffer Full IRQ Enable for LPC channel y
> > >+ * IRQxEy	Assert SerIRQ x for LPC channel y (Deprecated, use IDyIRQX, IRQXEy)
> > >+ * IDyIRQX	Use the specified 4-bit SerIRQ for LPC channel y
> > >+ * SELyIRQX	SerIRQ polarity for LPC channel y (low: 0, high: 1)
> > >+ * IRQXEy	Assert the SerIRQ specified in IDyIRQX for LPC channel y
> > >+ */
> > >+
> > >+#define LPC_TYIRQX_LOW       0b00
> > >+#define LPC_TYIRQX_HIGH      0b01
> > >+#define LPC_TYIRQX_RSVD      0b10
> > >+#define LPC_TYIRQX_RISING    0b11
> > >+
> > > #define LPC_HICR0            0x000
> > > #define     LPC_HICR0_LPC3E          BIT(7)
> > > #define     LPC_HICR0_LPC2E          BIT(6)
> > >@@ -39,6 +56,19 @@
> > > #define LPC_HICR4            0x010
> > > #define     LPC_HICR4_LADR12AS       BIT(7)
> > > #define     LPC_HICR4_KCSENBL        BIT(2)
> > >+#define LPC_SIRQCR0	     0x070
> > >+/* IRQ{12,1}E1 are deprecated as of AST2600 A3 but necessary for prior chips */
> > >+#define     LPC_SIRQCR0_IRQ12E1	     BIT(1)
> > >+#define     LPC_SIRQCR0_IRQ1E1	     BIT(0)
> > >+#define LPC_HICR5	     0x080
> > >+#define     LPC_HICR5_ID3IRQX_MASK   GENMASK(23, 20)
> > >+#define     LPC_HICR5_ID3IRQX_SHIFT  20
> > >+#define     LPC_HICR5_ID2IRQX_MASK   GENMASK(19, 16)
> > >+#define     LPC_HICR5_ID2IRQX_SHIFT  16
> > >+#define     LPC_HICR5_SEL3IRQX       BIT(15)
> > >+#define     LPC_HICR5_IRQXE3         BIT(14)
> > >+#define     LPC_HICR5_SEL2IRQX       BIT(13)
> > >+#define     LPC_HICR5_IRQXE2         BIT(12)
> > > #define LPC_LADR3H           0x014
> > > #define LPC_LADR3L           0x018
> > > #define LPC_LADR12H          0x01C
> > >@@ -55,6 +85,13 @@
> > > #define LPC_HICRB            0x100
> > > #define     LPC_HICRB_IBFIF4         BIT(1)
> > > #define     LPC_HICRB_LPC4E          BIT(0)
> > >+#define LPC_HICRC            0x104
> > >+#define     LPC_HICRC_ID4IRQX_MASK   GENMASK(7, 4)
> > >+#define     LPC_HICRC_ID4IRQX_SHIFT  4
> > >+#define     LPC_HICRC_TY4IRQX_MASK   GENMASK(3, 2)
> > >+#define     LPC_HICRC_TY4IRQX_SHIFT  2
> > >+#define     LPC_HICRC_OBF4_AUTO_CLR  BIT(1)
> > >+#define     LPC_HICRC_IRQXE4         BIT(0)
> > > #define LPC_LADR4            0x110
> > > #define LPC_IDR4             0x114
> > > #define LPC_ODR4             0x118
> > >@@ -62,11 +99,21 @@
> > >
> > > #define OBE_POLL_PERIOD	     (HZ / 2)
> > >
> > >+enum aspeed_kcs_irq_mode {
> > >+	aspeed_kcs_irq_none,
> > >+	aspeed_kcs_irq_serirq,
> > >+};
> > >+
> > > struct aspeed_kcs_bmc {
> > > 	struct kcs_bmc_device kcs_bmc;
> > >
> > > 	struct regmap *map;
> > >
> > >+	struct {
> > >+		enum aspeed_kcs_irq_mode mode;
> > >+		int id;
> > >+	} upstream_irq;
> > >+
> > > 	struct {
> > > 		spinlock_t lock;
> > > 		bool remove;
> > >@@ -103,6 +150,49 @@ static void aspeed_kcs_outb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 data)
> > >
> > > 	rc = regmap_write(priv->map, reg, data);
> > > 	WARN(rc != 0, "regmap_write() failed: %d\n", rc);
> > >+
> > >+	/* Trigger the upstream IRQ on ODR writes, if enabled */
> > >+
> > >+	switch (reg) {
> > >+	case LPC_ODR1:
> > >+	case LPC_ODR2:
> > >+	case LPC_ODR3:
> > >+	case LPC_ODR4:
> > >+		break;
> > >+	default:
> > >+		return;
> > >+	}
> > >+
> > >+	if (priv->upstream_irq.mode != aspeed_kcs_irq_serirq)
> > >+		return;
> > >+
> > >+	switch (kcs_bmc->channel) {
> > >+	case 1:
> > >+		switch (priv->upstream_irq.id) {
> > >+		case 12:
> > >+			regmap_update_bits(priv->map, LPC_SIRQCR0, LPC_SIRQCR0_IRQ12E1,
> > >+					   LPC_SIRQCR0_IRQ12E1);
> > >+			break;
> > >+		case 1:
> > >+			regmap_update_bits(priv->map, LPC_SIRQCR0, LPC_SIRQCR0_IRQ1E1,
> > >+					   LPC_SIRQCR0_IRQ1E1);
> > >+			break;

This is the code supporting the comment below.

> > >+		default:
> > >+			break;
> > >+		}
> > >+		break;
> > >+	case 2:
> > >+		regmap_update_bits(priv->map, LPC_HICR5, LPC_HICR5_IRQXE2, LPC_HICR5_IRQXE2);
> > >+		break;
> > >+	case 3:
> > >+		regmap_update_bits(priv->map, LPC_HICR5, LPC_HICR5_IRQXE3, LPC_HICR5_IRQXE3);
> > >+		break;
> > >+	case 4:
> > >+		regmap_update_bits(priv->map, LPC_HICRC, LPC_HICRC_IRQXE4, LPC_HICRC_IRQXE4);
> > >+		break;
> > >+	default:
> > >+		break;
> > >+	}
> > > }
> > >
> > > static void aspeed_kcs_updateb(struct kcs_bmc_device *kcs_bmc, u32 reg, u8 mask, u8 val)
> > >@@ -161,6 +251,73 @@ static void aspeed_kcs_set_address(struct kcs_bmc_device *kcs_bmc, u16 addr)
> > > 	}
> > > }
> > >
> > >+static inline int aspeed_kcs_map_serirq_type(u32 dt_type)
> > >+{
> > >+	switch (dt_type) {
> > >+	case IRQ_TYPE_EDGE_RISING:
> > >+		return LPC_TYIRQX_RISING;
> > >+	case IRQ_TYPE_LEVEL_HIGH:
> > >+		return LPC_TYIRQX_HIGH;
> > >+	case IRQ_TYPE_LEVEL_LOW:
> > >+		return LPC_TYIRQX_LOW;
> > >+	default:
> > >+		return -EINVAL;
> > >+	}
> > >+}
> > >+
> > >+static int aspeed_kcs_config_upstream_irq(struct aspeed_kcs_bmc *priv, u32 id, u32 dt_type)
> > >+{
> > >+	unsigned int mask, val, hw_type;
> > >+
> > >+	if (id > 15)
> > >+		return -EINVAL;
> > >+
> > >+	hw_type = aspeed_kcs_map_serirq_type(dt_type);
> > >+	if (hw_type < 0)
> > >+		return hw_type;
> > >+
> > >+	priv->upstream_irq.mode = aspeed_kcs_irq_serirq;
> > >+	priv->upstream_irq.id = id;
> > >+
> > >+	switch (priv->kcs_bmc.channel) {
> > >+	case 1:
> > >+		/* Needs IRQxE1 rather than (ID1IRQX, SEL1IRQX, IRQXE1) before AST2600 A3 */
> > 
> > I'm struggling a bit with understanding this comment, and relating it to
> > the code -- it sounds like "we need to do things one way on A3 and
> > later, and another way on pre-A3", but then...we just break without
> > doing anything at all either way.  Can you clarify any further?
> 
> Hah! You're struggling because it doesn't make any sense, the code's 
> gone missing :/ I'll fix that up.

Wait, no, this is fine. I just refreshed my memory on what's happening here. This function just configures the SerIRQ - in the case of channel 1 no configuration is necessary as it only has a fixed set of IRQs that we can associate with it. To enable one of them, we just set the appropriate bit in aspeed_kcs_outb() above.

Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-06-08  0:57 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-10  5:41 [PATCH v3 00/16] ipmi: Allow raw access to KCS devices Andrew Jeffery
2021-05-10  5:41 ` [PATCH v3 01/16] ipmi: kcs_bmc_aspeed: Use of match data to extract KCS properties Andrew Jeffery
2021-05-21  7:17   ` Zev Weiss
2021-05-10  5:41 ` [PATCH v3 02/16] ipmi: kcs_bmc: Make status update atomic Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 03/16] ipmi: kcs_bmc: Rename {read, write}_{status, data}() functions Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 04/16] ipmi: kcs_bmc: Split out kcs_bmc_cdev_ipmi Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 05/16] ipmi: kcs_bmc: Turn the driver data-structures inside-out Andrew Jeffery
2021-05-21 17:14   ` Corey Minyard
2021-05-24  0:53     ` Andrew Jeffery
2021-05-24 15:41       ` [Openipmi-developer] " Corey Minyard
2021-05-25  0:12         ` Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 06/16] ipmi: kcs_bmc: Split headers into device and client Andrew Jeffery
2021-05-21  7:18   ` Zev Weiss
2021-05-10  5:42 ` [PATCH v3 07/16] ipmi: kcs_bmc: Strip private client data from struct kcs_bmc Andrew Jeffery
2021-05-21  7:18   ` Zev Weiss
2021-05-10  5:42 ` [PATCH v3 08/16] ipmi: kcs_bmc: Decouple the IPMI chardev from the core Andrew Jeffery
2021-05-21  7:19   ` Zev Weiss
2021-05-10  5:42 ` [PATCH v3 09/16] ipmi: kcs_bmc: Allow clients to control KCS IRQ state Andrew Jeffery
2021-05-21  7:19   ` Zev Weiss
2021-05-10  5:42 ` [PATCH v3 10/16] ipmi: kcs_bmc: Don't enforce single-open policy in the kernel Andrew Jeffery
2021-05-21 17:30   ` Corey Minyard
2021-05-24  0:39     ` Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 11/16] ipmi: kcs_bmc: Add serio adaptor Andrew Jeffery
2021-05-21  7:20   ` Zev Weiss
2021-06-08  0:37     ` Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 12/16] dt-bindings: ipmi: Convert ASPEED KCS binding to schema Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 13/16] dt-bindings: ipmi: Add optional SerIRQ property to ASPEED KCS devices Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 14/16] ipmi: kcs_bmc_aspeed: Implement KCS SerIRQ configuration Andrew Jeffery
2021-05-21  7:21   ` Zev Weiss
2021-06-08  0:41     ` Andrew Jeffery
2021-06-08  0:55       ` Andrew Jeffery [this message]
2021-05-10  5:42 ` [PATCH v3 15/16] ipmi: kcs_bmc_aspeed: Fix IBFIE typo from datasheet Andrew Jeffery
2021-05-10  5:42 ` [PATCH v3 16/16] ipmi: kcs_bmc_aspeed: Optionally apply status address Andrew Jeffery
2021-05-20  6:51 ` [PATCH v3 00/16] ipmi: Allow raw access to KCS devices Andrew Jeffery
2021-05-20 13:33   ` Corey Minyard
2021-05-21 17:36 ` Corey Minyard
2021-05-24  0:36   ` Andrew Jeffery

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e5487cab-2ca0-46c2-a644-46d8b3070eac@www.fastmail.com \
    --to=andrew@aj.id.au \
    --cc=arnd@arndb.de \
    --cc=avifishman70@gmail.com \
    --cc=benjaminfair@google.com \
    --cc=chiawei_wang@aspeedtech.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minyard@acm.org \
    --cc=openbmc@lists.ozlabs.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    --cc=robh+dt@kernel.org \
    --cc=tali.perry1@gmail.com \
    --cc=tmaimon77@gmail.com \
    --cc=venture@google.com \
    --cc=zweiss@equinix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).