Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751887AbdGFRCm (ORCPT ); Thu, 6 Jul 2017 13:02:42 -0400 Received: from mail-pg0-f51.google.com ([74.125.83.51]:36367 "EHLO mail-pg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750970AbdGFRCl (ORCPT ); Thu, 6 Jul 2017 13:02:41 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170705215117.152807-1-venture@google.com> From: Patrick Venture Date: Thu, 6 Jul 2017 10:02:19 -0700 Message-ID: Subject: Re: [PATCH v3] drivers/misc: (aspeed-lpc-snoop): Add ast2400 to compat To: Patrick Venture , Joel Stanley , Rob Lippert , Greg KH Cc: Rick Altherr , Arnd Bergmann , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4516 Lines: 117 On Thu, Jul 6, 2017 at 10:00 AM, Patrick Venture wrote: > On Wed, Jul 5, 2017 at 2:51 PM, Patrick Venture wrote: >> This driver can be used on the aspeed ast2400 with minor >> modifications. >> >> Tested: ast2400 on quanta-q71l >> >> Signed-off-by: Patrick Venture >> --- >> v3: added .data object to determine behavior difference between ast2400 and >> ast2500. >> v2: added aspeed-g5 area because ast2400 doesn't use those bits. >> also updated commit message. >> --- >> drivers/misc/aspeed-lpc-snoop.c | 34 +++++++++++++++++++++++++++++----- >> 1 file changed, 29 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/misc/aspeed-lpc-snoop.c b/drivers/misc/aspeed-lpc-snoop.c >> index 593905565b74..696ad20d8f46 100644 >> --- a/drivers/misc/aspeed-lpc-snoop.c >> +++ b/drivers/misc/aspeed-lpc-snoop.c >> @@ -20,6 +20,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> >> @@ -51,6 +52,13 @@ >> #define HICRB_ENSNP0D BIT(14) >> #define HICRB_ENSNP1D BIT(15) >> >> +struct aspeed_lpc_snoop_model_data { >> + /* The ast2400 has bits 14 and 15 as reserved, whereas the ast2500 >> + * can use them. >> + */ >> + unsigned int has_hicrb_ensnp; >> +}; >> + >> struct aspeed_lpc_snoop { >> struct regmap *regmap; >> int irq; >> @@ -123,10 +131,13 @@ static int aspeed_lpc_snoop_config_irq(struct aspeed_lpc_snoop *lpc_snoop, >> } >> >> static int aspeed_lpc_enable_snoop(struct aspeed_lpc_snoop *lpc_snoop, >> - int channel, u16 lpc_port) >> + struct device *dev, >> + int channel, u16 lpc_port) >> { >> int rc = 0; >> u32 hicr5_en, snpwadr_mask, snpwadr_shift, hicrb_en; >> + const struct aspeed_lpc_snoop_model_data *model_data = >> + of_device_get_match_data(dev); >> >> /* Create FIFO datastructure */ >> rc = kfifo_alloc(&lpc_snoop->snoop_fifo[channel], >> @@ -155,7 +166,9 @@ static int aspeed_lpc_enable_snoop(struct aspeed_lpc_snoop *lpc_snoop, >> 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, HICRB, hicrb_en, hicrb_en); >> + if (model_data->has_hicrb_ensnp) >> + regmap_update_bits(lpc_snoop->regmap, HICRB, >> + hicrb_en, hicrb_en); >> >> return rc; >> } >> @@ -213,14 +226,14 @@ static int aspeed_lpc_snoop_probe(struct platform_device *pdev) >> if (rc) >> return rc; >> >> - rc = aspeed_lpc_enable_snoop(lpc_snoop, 0, port); >> + rc = aspeed_lpc_enable_snoop(lpc_snoop, dev, 0, port); >> if (rc) >> return rc; >> >> /* Configuration of 2nd snoop channel port is optional */ >> if (of_property_read_u32_index(dev->of_node, "snoop-ports", >> 1, &port) == 0) { >> - rc = aspeed_lpc_enable_snoop(lpc_snoop, 1, port); >> + rc = aspeed_lpc_enable_snoop(lpc_snoop, dev, 1, port); >> if (rc) >> aspeed_lpc_disable_snoop(lpc_snoop, 0); >> } >> @@ -239,8 +252,19 @@ static int aspeed_lpc_snoop_remove(struct platform_device *pdev) >> return 0; >> } >> >> +static const struct aspeed_lpc_snoop_model_data ast2400_model_data = { >> + .has_hicrb_ensnp = 0, >> +}; >> + >> +static const struct aspeed_lpc_snoop_model_data ast2500_model_data = { >> + .has_hicrb_ensnp = 1, >> +}; >> + >> static const struct of_device_id aspeed_lpc_snoop_match[] = { >> - { .compatible = "aspeed,ast2500-lpc-snoop" }, >> + { .compatible = "aspeed,ast2500-lpc-snoop", >> + .data = &ast2400_model_data }, >> + { .compatible = "aspeed,ast2400-lpc-snoop", >> + .data = &ast2500_model_data }, >> { }, > > This looks backwards. Let me check in a non-diff and then resubmit. > Since the register setting is harmless on the ast2400, it had a null effect for my testing, but ast2500 would have an issue. :D >> }; >> >> -- >> 2.13.2.725.g09c95d1e9-goog >>