2017-07-05 21:51:26

by Patrick Venture

[permalink] [raw]
Subject: [PATCH v3] drivers/misc: (aspeed-lpc-snoop): Add ast2400 to compat

This driver can be used on the aspeed ast2400 with minor
modifications.

Tested: ast2400 on quanta-q71l

Signed-off-by: Patrick Venture <[email protected]>
---
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 <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>

@@ -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 },
{ },
};

--
2.13.2.725.g09c95d1e9-goog


2017-07-06 17:00:30

by Patrick Venture

[permalink] [raw]
Subject: Re: [PATCH v3] drivers/misc: (aspeed-lpc-snoop): Add ast2400 to compat

On Wed, Jul 5, 2017 at 2:51 PM, Patrick Venture <[email protected]> wrote:
> This driver can be used on the aspeed ast2400 with minor
> modifications.
>
> Tested: ast2400 on quanta-q71l
>
> Signed-off-by: Patrick Venture <[email protected]>
> ---
> 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 <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
>
> @@ -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.

> };
>
> --
> 2.13.2.725.g09c95d1e9-goog
>

2017-07-06 17:02:42

by Patrick Venture

[permalink] [raw]
Subject: Re: [PATCH v3] drivers/misc: (aspeed-lpc-snoop): Add ast2400 to compat

On Thu, Jul 6, 2017 at 10:00 AM, Patrick Venture <[email protected]> wrote:
> On Wed, Jul 5, 2017 at 2:51 PM, Patrick Venture <[email protected]> wrote:
>> This driver can be used on the aspeed ast2400 with minor
>> modifications.
>>
>> Tested: ast2400 on quanta-q71l
>>
>> Signed-off-by: Patrick Venture <[email protected]>
>> ---
>> 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 <linux/mfd/syscon.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> +#include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> #include <linux/regmap.h>
>>
>> @@ -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
>>