From: Andrew Jeffery <andrew@codeconstruct.com.au>
To: linux-aspeed@lists.ozlabs.org
Cc: Joel Stanley <joel@jms.id.au>,
Henry Martin <bsdhenrymartin@gmail.com>,
Jean Delvare <jdelvare@suse.de>,
Patrick Rudolph <patrick.rudolph@9elements.com>,
Andrew Geissler <geissonator@yahoo.com>,
Ninad Palsule <ninad@linux.ibm.com>,
Patrick Venture <venture@google.com>,
Robert Lippert <roblip@gmail.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
Andrew Jeffery <andrew@codeconstruct.com.au>
Subject: [PATCH 7/7] soc: aspeed: lpc-snoop: Lift channel config to const structs
Date: Fri, 11 Apr 2025 10:38:37 +0930 [thread overview]
Message-ID: <20250411-aspeed-lpc-snoop-fixes-v1-7-64f522e3ad6f@codeconstruct.com.au> (raw)
In-Reply-To: <20250411-aspeed-lpc-snoop-fixes-v1-0-64f522e3ad6f@codeconstruct.com.au>
The shifts and masks for each channel are defined by hardware and
are not something that changes at runtime. Accordingly, describe the
information in an array of const structs and associate elements with
each channel instance, removing the need for the switch and handling of
its default case.
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
---
drivers/soc/aspeed/aspeed-lpc-snoop.c | 82 +++++++++++++++++------------------
1 file changed, 41 insertions(+), 41 deletions(-)
diff --git a/drivers/soc/aspeed/aspeed-lpc-snoop.c b/drivers/soc/aspeed/aspeed-lpc-snoop.c
index 0b2044fd79b1be08dfa33bfcaf249b020c909bb9..b54d8fbf7b83ebadd4fe1b16cbddf07a0bfac868 100644
--- a/drivers/soc/aspeed/aspeed-lpc-snoop.c
+++ b/drivers/soc/aspeed/aspeed-lpc-snoop.c
@@ -10,6 +10,7 @@
* 0x80 writes made by the BIOS during the boot process.
*/
+#include "linux/ratelimit.h"
#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/interrupt.h>
@@ -57,7 +58,15 @@ struct aspeed_lpc_snoop_model_data {
unsigned int has_hicrb_ensnp;
};
+struct aspeed_lpc_snoop_channel_cfg {
+ u32 hicr5_en;
+ u32 snpwadr_mask;
+ u32 snpwadr_shift;
+ u32 hicrb_en;
+};
+
struct aspeed_lpc_snoop_channel {
+ const struct aspeed_lpc_snoop_channel_cfg *cfg;
bool enabled;
struct kfifo fifo;
wait_queue_head_t wq;
@@ -188,7 +197,6 @@ static int aspeed_lpc_enable_snoop(struct aspeed_lpc_snoop *lpc_snoop,
int index, u16 lpc_port)
{
const struct aspeed_lpc_snoop_model_data *model_data;
- u32 hicr5_en, snpwadr_mask, snpwadr_shift, hicrb_en;
struct aspeed_lpc_snoop_channel *channel;
int rc = 0;
@@ -200,6 +208,9 @@ static int aspeed_lpc_enable_snoop(struct aspeed_lpc_snoop *lpc_snoop,
if (channel->enabled)
return -EBUSY;
+ if (WARN_ONCE(!channel->cfg, "snoop channel %d lacks required config", index))
+ return -EINVAL;
+
init_waitqueue_head(&channel->wq);
channel->miscdev.minor = MISC_DYNAMIC_MINOR;
@@ -220,39 +231,20 @@ static int aspeed_lpc_enable_snoop(struct aspeed_lpc_snoop *lpc_snoop,
goto err_free_fifo;
/* Enable LPC snoop channel at requested port */
- switch (index) {
- case 0:
- hicr5_en = HICR5_EN_SNP0W | HICR5_ENINT_SNP0W;
- snpwadr_mask = SNPWADR_CH0_MASK;
- snpwadr_shift = SNPWADR_CH0_SHIFT;
- hicrb_en = HICRB_ENSNP0D;
- break;
- case 1:
- hicr5_en = HICR5_EN_SNP1W | HICR5_ENINT_SNP1W;
- snpwadr_mask = SNPWADR_CH1_MASK;
- snpwadr_shift = SNPWADR_CH1_SHIFT;
- hicrb_en = HICRB_ENSNP1D;
- break;
- default:
- rc = -EINVAL;
- goto err_misc_deregister;
- }
-
- /* Enable LPC snoop channel at requested port */
- regmap_update_bits(lpc_snoop->regmap, HICR5, hicr5_en, hicr5_en);
- regmap_update_bits(lpc_snoop->regmap, SNPWADR, snpwadr_mask,
- lpc_port << snpwadr_shift);
+ regmap_update_bits(lpc_snoop->regmap, HICR5, channel->cfg->hicr5_en,
+ channel->cfg->hicr5_en);
+ regmap_update_bits(lpc_snoop->regmap, SNPWADR, channel->cfg->snpwadr_mask,
+ lpc_port << channel->cfg->snpwadr_shift);
model_data = of_device_get_match_data(dev);
if (model_data && model_data->has_hicrb_ensnp)
- regmap_update_bits(lpc_snoop->regmap, HICRB, hicrb_en, hicrb_en);
+ regmap_update_bits(lpc_snoop->regmap, HICRB, channel->cfg->hicrb_en,
+ channel->cfg->hicrb_en);
channel->enabled = true;
return 0;
-err_misc_deregister:
- misc_deregister(&lpc_snoop->chan[index].miscdev);
err_free_fifo:
kfifo_free(&lpc_snoop->chan[index].fifo);
return rc;
@@ -272,21 +264,7 @@ static void aspeed_lpc_disable_snoop(struct aspeed_lpc_snoop *lpc_snoop,
if (!channel->enabled)
return;
- /* Disable interrupts along with the device */
- switch (index) {
- case 0:
- regmap_update_bits(lpc_snoop->regmap, HICR5,
- HICR5_EN_SNP0W | HICR5_ENINT_SNP0W,
- 0);
- break;
- case 1:
- regmap_update_bits(lpc_snoop->regmap, HICR5,
- HICR5_EN_SNP1W | HICR5_ENINT_SNP1W,
- 0);
- break;
- default:
- return;
- }
+ regmap_update_bits(lpc_snoop->regmap, HICR5, channel->cfg->hicr5_en, 0);
channel->enabled = false;
/* Consider improving safety wrt concurrent reader(s) */
@@ -294,6 +272,21 @@ static void aspeed_lpc_disable_snoop(struct aspeed_lpc_snoop *lpc_snoop,
kfifo_free(&channel->fifo);
}
+static const struct aspeed_lpc_snoop_channel_cfg channel_cfgs[] = {
+ {
+ .hicr5_en = HICR5_EN_SNP0W | HICR5_ENINT_SNP0W,
+ .snpwadr_mask = SNPWADR_CH0_MASK,
+ .snpwadr_shift = SNPWADR_CH0_SHIFT,
+ .hicrb_en = HICRB_ENSNP0D,
+ },
+ {
+ .hicr5_en = HICR5_EN_SNP1W | HICR5_ENINT_SNP1W,
+ .snpwadr_mask = SNPWADR_CH1_MASK,
+ .snpwadr_shift = SNPWADR_CH1_SHIFT,
+ .hicrb_en = HICRB_ENSNP1D,
+ },
+};
+
static int aspeed_lpc_snoop_probe(struct platform_device *pdev)
{
struct aspeed_lpc_snoop *lpc_snoop;
@@ -308,6 +301,13 @@ static int aspeed_lpc_snoop_probe(struct platform_device *pdev)
if (!lpc_snoop)
return -ENOMEM;
+ static_assert(ARRAY_SIZE(channel_cfgs) == ARRAY_SIZE(lpc_snoop->chan),
+ "Broken implementation assumption regarding cfg count");
+ static_assert(ARRAY_SIZE(lpc_snoop->chan) == 2,
+ "Broken implementation assumption regarding channel count");
+ lpc_snoop->chan[0].cfg = &channel_cfgs[0];
+ lpc_snoop->chan[1].cfg = &channel_cfgs[1];
+
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") &&
--
2.39.5
prev parent reply other threads:[~2025-04-11 1:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-11 1:08 [PATCH 0/7] soc: aspeed: lpc-snoop: Miscellaneous fixes Andrew Jeffery
2025-04-11 1:08 ` [PATCH 1/7] soc: aspeed: lpc-snoop: Cleanup resources in stack-order Andrew Jeffery
2025-04-11 1:08 ` [PATCH 2/7] soc: aspeed: lpc-snoop: Don't disable channels that aren't enabled Andrew Jeffery
2025-04-11 1:08 ` [PATCH 3/7] soc: aspeed: lpc-snoop: Ensure model_data is valid Andrew Jeffery
2025-04-11 1:08 ` [PATCH 4/7] soc: aspeed: lpc-snoop: Constrain parameters in channel paths Andrew Jeffery
2025-04-11 1:08 ` [PATCH 5/7] soc: aspeed: lpc-snoop: Rename 'channel' to 'index' " Andrew Jeffery
2025-04-11 1:08 ` [PATCH 6/7] soc: aspeed: lpc-snoop: Rearrange " Andrew Jeffery
2025-04-11 1:08 ` Andrew Jeffery [this message]
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=20250411-aspeed-lpc-snoop-fixes-v1-7-64f522e3ad6f@codeconstruct.com.au \
--to=andrew@codeconstruct.com.au \
--cc=bsdhenrymartin@gmail.com \
--cc=geissonator@yahoo.com \
--cc=jdelvare@suse.de \
--cc=joel@jms.id.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-aspeed@lists.ozlabs.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ninad@linux.ibm.com \
--cc=patrick.rudolph@9elements.com \
--cc=roblip@gmail.com \
--cc=venture@google.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).