Re: [RFC PATCH dev-5.1 5/6] i2c: aspeed: add buffer mode transfer support

From: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
To: Tao Ren <taoren@fb.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" <openbmc@lists.ozlabs.org>
Subject: Re: [RFC PATCH dev-5.1 5/6] i2c: aspeed: add buffer mode transfer support
Date: Tue, 25 Jun 2019 10:18:58 -0700	[thread overview]
Message-ID: <02216a49-c263-1f37-56aa-9c8e7ee0a980@linux.intel.com> (raw)
In-Reply-To: <0801acf7-3c24-0a6c-c454-43a8521adb89@fb.com>

On 6/24/2019 4:54 PM, Tao Ren wrote:
> On 6/21/19 3:34 PM, Jae Hyun Yoo wrote:
>> On 6/21/2019 3:29 PM, Tao Ren wrote:
>>> On 6/20/19 12:49 PM, Jae Hyun Yoo wrote:
>>>> Byte mode currently this driver uses makes lots of interrupt call
>>>> which isn't good for performance and it makes the driver very
>>>> timing sensitive. To improve performance of the driver, this commit
>>>> adds buffer mode transfer support which uses I2C SRAM buffer
>>>> instead of using a single byte buffer.
>>>>
>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>>
>>> Let me apply the patch to my ast2500 BMC platform and will share results earlier next week.
>>
>> Thanks Tao! It would be helpful.
> 
> Hi Jae,
> 
> I applied patches 1-5 (except DMA mode) to my local tree: buffer mode is enabled by default, and reg_base/buf_base also looks correct to me, but I'm seeing some pca9548/lm75 driver binding failures on CMM BMC. I'm wondering if I missed some dependency patches as I'm still working on kernel 5.0 (although i2c-aspeed.c is up to date)?

Hi Tao,

It seems that I2C SRAM isn't enabled in your system. I forgot that Intel
local tree has I2C driver in u-boot so SRAM is enabled because u-boot
makes an I2C reset, but in other OpenBMC systems, it would not work
correctly because SRAM enabling control is triggered before an I2C
reset. I will move the SRAM control code from irq-aspeed-i2c-ic module
to i2c-aspeed module just after the deasserting of I2C reset. Will
submit v2 soon.

> 
> BTW, USB is enabled in my image because both EHCI and UHCI are needed. But I guess it won't impact buffer mode?

This patch series adds DMA support code but it doesn't set the transfer
mode to DMA. It sets buffer mode as default transfer mode. Even in case
you enable DMA, if CONFIG_USB_UHCI_ASPEED is enabled then buffer mode
will be selected instead of DMA by checking code in i2c-aspeed module.

Thanks,
Jae

> 
> Let me see if I can quickly set up kernel 5.1 and re-run testing; meanwhile, kindly let me know what information will be helpful for your debugging.
> 
> 
> Cheers,
> 
> Tao
> 