diff mbox series

[RFC,dev-5.1,3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

Message ID 20190620194922.15093-4-jae.hyun.yoo@linux.intel.com
State Superseded, archived
Headers show
Series Aspeed I2C buffer/DMA mode support | expand

Commit Message

Jae Hyun Yoo June 20, 2019, 7:49 p.m. UTC 
This commit adds I2C SRAM enabling control for AST2500 SoC to
support buffer mode and DMA mode transfer. The SRAM is enabled by
default in AST2400 SoC.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Ryan Chen June 21, 2019, 12:33 a.m. UTC | #1 
Hello Jae,
	The i2c register setting must after scu reset. - APEED_I2C_SRAM_BUFFER_EN
	My recommend aspeed-i2c-ic.c need be probe after scu reset. And all others i2c bus is no needed for scu reset. 

Ryan 

-----Original Message-----
From: openbmc [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On Behalf Of Jae Hyun Yoo
Sent: Friday, June 21, 2019 3:49 AM
To: Brendan Higgins <brendanhiggins@google.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>
Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

This commit adds I2C SRAM enabling control for AST2500 SoC to support buffer mode and DMA mode transfer. The SRAM is enabled by default in AST2400 SoC.

Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
---
 drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
index f20200af0992..99985b22a9fa 100644
--- a/drivers/irqchip/irq-aspeed-i2c-ic.c
+++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
@@ -18,6 +18,9 @@
 #include <linux/of_irq.h>
 #include <linux/io.h>
 
+/* I2C Global Control Register (AST2500) */
+#define ASPEED_I2C_GLOBAL_CTRL_REG	0xc
+#define  ASPEED_I2C_SRAM_BUFFER_EN	BIT(0)
 
 #define ASPEED_I2C_IC_NUM_BUS 14
 
@@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct device_node *node,
 	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
 					 aspeed_i2c_ic_irq_handler, i2c_ic);
 
+	/* Enable I2C SRAM buffer in case of AST2500 */
+	if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
+		writel(ASPEED_I2C_SRAM_BUFFER_EN,
+		       i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
+
 	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
 
 	return 0;
--
2.22.0
Jae Hyun Yoo June 21, 2019, 6:41 p.m. UTC | #2 
On 6/20/2019 5:33 PM, Ryan Chen wrote:
> Hello Jae,
> 	The i2c register setting must after scu reset. - APEED_I2C_SRAM_BUFFER_EN
> 	My recommend aspeed-i2c-ic.c need be probe after scu reset. And all others i2c bus is no needed for scu reset.

Hello Ryan,

This module is registered after the SCU reset.
Thank you for the information.

Regards,
Jae

> 
> Ryan
> 
> -----Original Message-----
> From: openbmc [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On Behalf Of Jae Hyun Yoo
> Sent: Friday, June 21, 2019 3:49 AM
> To: Brendan Higgins <brendanhiggins@google.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>
> Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control
> 
> This commit adds I2C SRAM enabling control for AST2500 SoC to support buffer mode and DMA mode transfer. The SRAM is enabled by default in AST2400 SoC.
> 
> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
> ---
>   drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
> index f20200af0992..99985b22a9fa 100644
> --- a/drivers/irqchip/irq-aspeed-i2c-ic.c
> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
> @@ -18,6 +18,9 @@
>   #include <linux/of_irq.h>
>   #include <linux/io.h>
>   
> +/* I2C Global Control Register (AST2500) */
> +#define ASPEED_I2C_GLOBAL_CTRL_REG	0xc
> +#define  ASPEED_I2C_SRAM_BUFFER_EN	BIT(0)
>   
>   #define ASPEED_I2C_IC_NUM_BUS 14
>   
> @@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct device_node *node,
>   	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
>   					 aspeed_i2c_ic_irq_handler, i2c_ic);
>   
> +	/* Enable I2C SRAM buffer in case of AST2500 */
> +	if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
> +		writel(ASPEED_I2C_SRAM_BUFFER_EN,
> +		       i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
> +
>   	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
>   
>   	return 0;
> --
> 2.22.0
>
Jae Hyun Yoo June 25, 2019, 5:23 p.m. UTC | #3 
On 6/21/2019 11:41 AM, Jae Hyun Yoo wrote:
> On 6/20/2019 5:33 PM, Ryan Chen wrote:
>> Hello Jae,
>>     The i2c register setting must after scu reset. - 
>> APEED_I2C_SRAM_BUFFER_EN
>>     My recommend aspeed-i2c-ic.c need be probe after scu reset. And 
>> all others i2c bus is no needed for scu reset.
> 
> Hello Ryan,
> 
> This module is registered after the SCU reset.
> Thank you for the information.
> 
> Regards,
> Jae

Hello Ryan,

I got your point now. You meant the I2C H/W reset through SCU04
register, right? I'll move the SRAM buffer enable control from
irq-aspeed-i2c-ic module to i2c-aspeed module so that the SRAM can be
enabled correctly.

Thanks for your pointing it out.

Jae

>>
>> Ryan
>>
>> -----Original Message-----
>> From: openbmc 
>> [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On 
>> Behalf Of Jae Hyun Yoo
>> Sent: Friday, June 21, 2019 3:49 AM
>> To: Brendan Higgins <brendanhiggins@google.com>; Benjamin 
>> Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater 
>> <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery 
>> <andrew@aj.id.au>
>> Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM 
>> enabling control
>>
>> This commit adds I2C SRAM enabling control for AST2500 SoC to support 
>> buffer mode and DMA mode transfer. The SRAM is enabled by default in 
>> AST2400 SoC.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>>   drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c 
>> b/drivers/irqchip/irq-aspeed-i2c-ic.c
>> index f20200af0992..99985b22a9fa 100644
>> --- a/drivers/irqchip/irq-aspeed-i2c-ic.c
>> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
>> @@ -18,6 +18,9 @@
>>   #include <linux/of_irq.h>
>>   #include <linux/io.h>
>> +/* I2C Global Control Register (AST2500) */
>> +#define ASPEED_I2C_GLOBAL_CTRL_REG    0xc
>> +#define  ASPEED_I2C_SRAM_BUFFER_EN    BIT(0)
>>   #define ASPEED_I2C_IC_NUM_BUS 14
>> @@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct 
>> device_node *node,
>>       irq_set_chained_handler_and_data(i2c_ic->parent_irq,
>>                        aspeed_i2c_ic_irq_handler, i2c_ic);
>> +    /* Enable I2C SRAM buffer in case of AST2500 */
>> +    if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
>> +        writel(ASPEED_I2C_SRAM_BUFFER_EN,
>> +               i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
>> +
>>       pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
>>       return 0;
>> -- 
>> 2.22.0
>>
Ryan Chen June 26, 2019, 5:10 a.m. UTC | #4 
Hello Jae,
	Actually, my recommend is following.
	1. Move i2c scu reset to irq-aspeed-i2c-ic.c and also keep SRAM buffer enable.
	2. remove i2c each bus reset. 
Ryan 

-----Original Message-----
From: Jae Hyun Yoo [mailto:jae.hyun.yoo@linux.intel.com] 
Sent: Wednesday, June 26, 2019 1:23 AM
To: Ryan Chen <ryan_chen@aspeedtech.com>; Brendan Higgins <brendanhiggins@google.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>
Cc: openbmc@lists.ozlabs.org
Subject: Re: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control

On 6/21/2019 11:41 AM, Jae Hyun Yoo wrote:
> On 6/20/2019 5:33 PM, Ryan Chen wrote:
>> Hello Jae,
>>     The i2c register setting must after scu reset. - 
>> APEED_I2C_SRAM_BUFFER_EN
>>     My recommend aspeed-i2c-ic.c need be probe after scu reset. And 
>> all others i2c bus is no needed for scu reset.
> 
> Hello Ryan,
> 
> This module is registered after the SCU reset.
> Thank you for the information.
> 
> Regards,
> Jae

Hello Ryan,

I got your point now. You meant the I2C H/W reset through SCU04 register, right? I'll move the SRAM buffer enable control from irq-aspeed-i2c-ic module to i2c-aspeed module so that the SRAM can be enabled correctly.

Thanks for your pointing it out.

Jae

>>
>> Ryan
>>
>> -----Original Message-----
>> From: openbmc
>> [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On 
>> Behalf Of Jae Hyun Yoo
>> Sent: Friday, June 21, 2019 3:49 AM
>> To: Brendan Higgins <brendanhiggins@google.com>; Benjamin 
>> Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater 
>> <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery 
>> <andrew@aj.id.au>
>> Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo 
>> <jae.hyun.yoo@linux.intel.com>
>> Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM 
>> enabling control
>>
>> This commit adds I2C SRAM enabling control for AST2500 SoC to support 
>> buffer mode and DMA mode transfer. The SRAM is enabled by default in
>> AST2400 SoC.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>>   drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c
>> b/drivers/irqchip/irq-aspeed-i2c-ic.c
>> index f20200af0992..99985b22a9fa 100644
>> --- a/drivers/irqchip/irq-aspeed-i2c-ic.c
>> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
>> @@ -18,6 +18,9 @@
>>   #include <linux/of_irq.h>
>>   #include <linux/io.h>
>> +/* I2C Global Control Register (AST2500) */ #define 
>> +ASPEED_I2C_GLOBAL_CTRL_REG    0xc #define  ASPEED_I2C_SRAM_BUFFER_EN    
>> +BIT(0)
>>   #define ASPEED_I2C_IC_NUM_BUS 14
>> @@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct 
>> device_node *node,
>>       irq_set_chained_handler_and_data(i2c_ic->parent_irq,
>>                        aspeed_i2c_ic_irq_handler, i2c_ic);
>> +    /* Enable I2C SRAM buffer in case of AST2500 */
>> +    if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
>> +        writel(ASPEED_I2C_SRAM_BUFFER_EN,
>> +               i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
>> +
>>       pr_info("i2c controller registered, irq %d\n", 
>> i2c_ic->parent_irq);
>>       return 0;
>> --
>> 2.22.0
>>
Jae Hyun Yoo June 26, 2019, 9:18 p.m. UTC | #5 
On 6/25/2019 10:10 PM, Ryan Chen wrote:
> Hello Jae,
> 	Actually, my recommend is following.
> 	1. Move i2c scu reset to irq-aspeed-i2c-ic.c and also keep SRAM buffer enable.
> 	2. remove i2c each bus reset.

Hello Ryan,

My thought is, irq-aspeed-i2c-ic module should do things only irqchip
drivers do. i2c-aspeed module has been doing the I2C SCU reset control
correctly so far without making any problem.

Regards,
Jae

> Ryan
> 
> -----Original Message-----
> From: Jae Hyun Yoo [mailto:jae.hyun.yoo@linux.intel.com]
> Sent: Wednesday, June 26, 2019 1:23 AM
> To: Ryan Chen <ryan_chen@aspeedtech.com>; Brendan Higgins <brendanhiggins@google.com>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery <andrew@aj.id.au>
> Cc: openbmc@lists.ozlabs.org
> Subject: Re: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM enabling control
> 
> On 6/21/2019 11:41 AM, Jae Hyun Yoo wrote:
>> On 6/20/2019 5:33 PM, Ryan Chen wrote:
>>> Hello Jae,
>>>      The i2c register setting must after scu reset. -
>>> APEED_I2C_SRAM_BUFFER_EN
>>>      My recommend aspeed-i2c-ic.c need be probe after scu reset. And
>>> all others i2c bus is no needed for scu reset.
>>
>> Hello Ryan,
>>
>> This module is registered after the SCU reset.
>> Thank you for the information.
>>
>> Regards,
>> Jae
> 
> Hello Ryan,
> 
> I got your point now. You meant the I2C H/W reset through SCU04 register, right? I'll move the SRAM buffer enable control from irq-aspeed-i2c-ic module to i2c-aspeed module so that the SRAM can be enabled correctly.
> 
> Thanks for your pointing it out.
> 
> Jae
> 
>>>
>>> Ryan
>>>
>>> -----Original Message-----
>>> From: openbmc
>>> [mailto:openbmc-bounces+ryan_chen=aspeedtech.com@lists.ozlabs.org] On
>>> Behalf Of Jae Hyun Yoo
>>> Sent: Friday, June 21, 2019 3:49 AM
>>> To: Brendan Higgins <brendanhiggins@google.com>; Benjamin
>>> Herrenschmidt <benh@kernel.crashing.org>; C?ric Le Goater
>>> <clg@kaod.org>; Joel Stanley <joel@jms.id.au>; Andrew Jeffery
>>> <andrew@aj.id.au>
>>> Cc: openbmc@lists.ozlabs.org; Jae Hyun Yoo
>>> <jae.hyun.yoo@linux.intel.com>
>>> Subject: [RFC PATCH dev-5.1 3/6] irqchip/aspeed-i2c-ic: add I2C SRAM
>>> enabling control
>>>
>>> This commit adds I2C SRAM enabling control for AST2500 SoC to support
>>> buffer mode and DMA mode transfer. The SRAM is enabled by default in
>>> AST2400 SoC.
>>>
>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>> ---
>>>    drivers/irqchip/irq-aspeed-i2c-ic.c | 8 ++++++++
>>>    1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c
>>> b/drivers/irqchip/irq-aspeed-i2c-ic.c
>>> index f20200af0992..99985b22a9fa 100644
>>> --- a/drivers/irqchip/irq-aspeed-i2c-ic.c
>>> +++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
>>> @@ -18,6 +18,9 @@
>>>    #include <linux/of_irq.h>
>>>    #include <linux/io.h>
>>> +/* I2C Global Control Register (AST2500) */ #define
>>> +ASPEED_I2C_GLOBAL_CTRL_REG    0xc #define  ASPEED_I2C_SRAM_BUFFER_EN
>>> +BIT(0)
>>>    #define ASPEED_I2C_IC_NUM_BUS 14
>>> @@ -100,6 +103,11 @@ static int __init aspeed_i2c_ic_of_init(struct
>>> device_node *node,
>>>        irq_set_chained_handler_and_data(i2c_ic->parent_irq,
>>>                         aspeed_i2c_ic_irq_handler, i2c_ic);
>>> +    /* Enable I2C SRAM buffer in case of AST2500 */
>>> +    if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
>>> +        writel(ASPEED_I2C_SRAM_BUFFER_EN,
>>> +               i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
>>> +
>>>        pr_info("i2c controller registered, irq %d\n",
>>> i2c_ic->parent_irq);
>>>        return 0;
>>> --
>>> 2.22.0
>>>
diff mbox series

Patch

diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
index f20200af0992..99985b22a9fa 100644
--- a/drivers/irqchip/irq-aspeed-i2c-ic.c
+++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
@@ -18,6 +18,9 @@ 
 #include <linux/of_irq.h>
 #include <linux/io.h>
 
+/* I2C Global Control Register (AST2500) */
+#define ASPEED_I2C_GLOBAL_CTRL_REG	0xc
+#define  ASPEED_I2C_SRAM_BUFFER_EN	BIT(0)
 
 #define ASPEED_I2C_IC_NUM_BUS 14
 
@@ -100,6 +103,11 @@  static int __init aspeed_i2c_ic_of_init(struct device_node *node,
 	irq_set_chained_handler_and_data(i2c_ic->parent_irq,
 					 aspeed_i2c_ic_irq_handler, i2c_ic);
 
+	/* Enable I2C SRAM buffer in case of AST2500 */
+	if (of_device_is_compatible(node, "aspeed,ast2500-i2c-ic"))
+		writel(ASPEED_I2C_SRAM_BUFFER_EN,
+		       i2c_ic->base + ASPEED_I2C_GLOBAL_CTRL_REG);
+
 	pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
 
 	return 0;