devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wolfram Sang <wsa@kernel.org>
To: Quan Nguyen <quan@os.amperecomputing.com>
Cc: Corey Minyard <minyard@acm.org>, Rob Herring <robh+dt@kernel.org>,
	Joel Stanley <joel@jms.id.au>, Andrew Jeffery <andrew@aj.id.au>,
	Brendan Higgins <brendanhiggins@google.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	openipmi-developer@lists.sourceforge.net,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-i2c@vger.kernel.org,
	Open Source Submission <patches@amperecomputing.com>,
	Phong Vo <phong@os.amperecomputing.com>,
	"Thang Q . Nguyen" <thang@os.amperecomputing.com>
Subject: Re: [PATCH v5 1/3] i2c: aspeed: Add slave_enable() to toggle slave mode
Date: Thu, 12 Aug 2021 09:39:43 +0200	[thread overview]
Message-ID: <YRTQP9sX0hkTJMTx@shikoro> (raw)
In-Reply-To: <20210714033833.11640-2-quan@os.amperecomputing.com>

[-- Attachment #1: Type: text/plain, Size: 1466 bytes --]

Hi all,

On Wed, Jul 14, 2021 at 10:38:31AM +0700, Quan Nguyen wrote:
> Slave needs time to prepare the response data before Master could
> enquiry via read transaction. However, there is no mechanism for
> i2c-aspeed Slave to notify Master that it needs more time to process
> and this make Master side to time out when trying to get the response.
> 
> This commit introduces the slave_enable() callback in struct
> i2c_algorithm for Slave to temporary stop the Slave mode while working
> on the response and re-enable the Slave when response data ready.

Sorry that I couldn't chime in earlier, but NAK!

>  include/linux/i2c.h             |  2 ++

@Corey: Please do not change this file without my ACK. It is not a
trivial change but an API extenstion and that should really be acked by
the subsystem maintainer, in this case me. I was really surprised to see
this in linux-next already.

@all: Plus, I neither like the API (because it doesn't look generic to
me but mostly handling one issue needed here) nor do I fully understand
the use case. Normally, when a read is requested and the backend needs
time to deliver the data, the hardware should stretch the SCL clock
until some data register is finally written to. If it doesn't do it for
whatever reason, this is a quirky hardware in my book and needs handling
in the driver only. So, what is special with this HW? Can't we solve it
differently?

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-08-12  7:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14  3:38 [PATCH v5 0/3] Add SSIF BMC driver Quan Nguyen
2021-07-14  3:38 ` [PATCH v5 1/3] i2c: aspeed: Add slave_enable() to toggle slave mode Quan Nguyen
2021-08-12  7:39   ` Wolfram Sang [this message]
2021-08-12 13:36     ` [Openipmi-developer] " Corey Minyard
2021-11-29 19:22     ` Wolfram Sang
2021-11-30  2:08       ` Quan Nguyen
2021-11-30 10:02         ` Wolfram Sang
2021-07-14  3:38 ` [PATCH v5 2/3] ipmi: ssif_bmc: Add SSIF BMC driver Quan Nguyen
2021-07-14  3:38 ` [PATCH v5 3/3] bindings: ipmi: Add binding for " Quan Nguyen
2021-07-15 17:43   ` Rob Herring
2021-07-16  2:45     ` Quan Nguyen
2021-07-15 23:32 ` [PATCH v5 0/3] Add " Corey Minyard
2021-07-16  2:47   ` Quan Nguyen

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=YRTQP9sX0hkTJMTx@shikoro \
    --to=wsa@kernel.org \
    --cc=andrew@aj.id.au \
    --cc=benh@kernel.crashing.org \
    --cc=brendanhiggins@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=joel@jms.id.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minyard@acm.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    --cc=patches@amperecomputing.com \
    --cc=phong@os.amperecomputing.com \
    --cc=quan@os.amperecomputing.com \
    --cc=robh+dt@kernel.org \
    --cc=thang@os.amperecomputing.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).