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:11:42 +0930 [thread overview]
Message-ID: <df0d31e9-c4e1-4cf0-b800-c5dadfb43420@www.fastmail.com> (raw)
In-Reply-To: <YKdfeJJM/4LYFKe4@packtop>
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;
> >+ 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.
Unfortunately due to hardware/firmware limitations I wasn't able to test channel 1.
>
> >+ break;
> >+ case 2:
> >+ if (!(hw_type == LPC_TYIRQX_LOW || hw_type == LPC_TYIRQX_HIGH))
> >+ return -EINVAL;
> >+
> >+ mask = LPC_HICR5_SEL2IRQX | LPC_HICR5_ID2IRQX_MASK;
> >+ val = (id << LPC_HICR5_ID2IRQX_SHIFT);
> >+ val |= (hw_type == LPC_TYIRQX_HIGH) ? LPC_HICR5_SEL2IRQX : 0;
> >+ regmap_update_bits(priv->map, LPC_HICR5, mask, val);
> >+
> >+ break;
> >+ case 3:
> >+ if (!(hw_type == LPC_TYIRQX_LOW || hw_type == LPC_TYIRQX_HIGH))
> >+ return -EINVAL;
> >+
> >+ mask = LPC_HICR5_SEL3IRQX | LPC_HICR5_ID3IRQX_MASK;
> >+ val = (id << LPC_HICR5_ID3IRQX_SHIFT);
> >+ val |= (hw_type == LPC_TYIRQX_HIGH) ? LPC_HICR5_SEL3IRQX : 0;
> >+ regmap_update_bits(priv->map, LPC_HICR5, mask, val);
> >+
> >+ break;
> >+ case 4:
> >+ mask = LPC_HICRC_ID4IRQX_MASK | LPC_HICRC_TY4IRQX_MASK | LPC_HICRC_OBF4_AUTO_CLR;
> >+ val = (id << LPC_HICRC_ID4IRQX_SHIFT) | (hw_type << LPC_HICRC_TY4IRQX_SHIFT);
> >+ regmap_update_bits(priv->map, LPC_HICRC, mask, val);
> >+ break;
> >+ default:
> >+ dev_warn(priv->kcs_bmc.dev,
> >+ "SerIRQ configuration not supported on KCS channel %d\n",
> >+ priv->kcs_bmc.channel);
> >+ return -EINVAL;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> > static void aspeed_kcs_enable_channel(struct kcs_bmc_device *kcs_bmc, bool enable)
> > {
> > struct aspeed_kcs_bmc *priv = to_aspeed_kcs_bmc(kcs_bmc);
> >@@ -262,7 +419,7 @@ static irqreturn_t aspeed_kcs_irq(int irq, void *arg)
> > return kcs_bmc_handle_event(kcs_bmc);
> > }
> >
> >-static int aspeed_kcs_config_irq(struct kcs_bmc_device *kcs_bmc,
> >+static int aspeed_kcs_config_downstream_irq(struct kcs_bmc_device *kcs_bmc,
> > struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> >@@ -366,6 +523,8 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
> > struct aspeed_kcs_bmc *priv;
> > struct device_node *np;
> > int rc, channel, addr;
> >+ bool have_upstream_irq;
> >+ u32 upstream_irq[2];
> >
> > np = pdev->dev.of_node->parent;
> > if (!of_device_is_compatible(np, "aspeed,ast2400-lpc-v2") &&
> >@@ -374,6 +533,7 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
> > dev_err(&pdev->dev, "unsupported LPC device binding\n");
> > return -ENODEV;
> > }
> >+
> > ops = of_device_get_match_data(&pdev->dev);
> > if (!ops)
> > return -EINVAL;
> >@@ -386,6 +546,13 @@ static int aspeed_kcs_probe(struct platform_device *pdev)
> > if (addr < 0)
> > return addr;
> >
> >+ np = pdev->dev.of_node;
> >+ rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", upstream_irq, 2);
> >+ if ((rc && rc != -EINVAL))
>
> I think we could probably get by with slightly fewer parens here...
Yeah, not sure what happened there :)
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-06-08 0:46 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 [this message]
2021-06-08 0:55 ` Andrew Jeffery
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=df0d31e9-c4e1-4cf0-b800-c5dadfb43420@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).