From: Corey Minyard <minyard@acm.org>
To: linux-aspeed@lists.ozlabs.org
Subject: [Openipmi-developer] [PATCH v5 1/3] i2c: aspeed: Add slave_enable() to toggle slave mode
Date: Thu, 12 Aug 2021 08:36:55 -0500 [thread overview]
Message-ID: <20210812133655.GT3406@minyard.net> (raw)
In-Reply-To: <YRTQP9sX0hkTJMTx@shikoro>
On Thu, Aug 12, 2021 at 09:39:43AM +0200, Wolfram Sang wrote:
> 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.
I am sorry, I'll pull it out.
-corey
>
> @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
>
> _______________________________________________
> Openipmi-developer mailing list
> Openipmi-developer at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer
WARNING: multiple messages have this Message-ID (diff)
From: Corey Minyard <minyard@acm.org>
To: Wolfram Sang <wsa@kernel.org>,
Quan Nguyen <quan@os.amperecomputing.com>,
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: [Openipmi-developer] [PATCH v5 1/3] i2c: aspeed: Add slave_enable() to toggle slave mode
Date: Thu, 12 Aug 2021 08:36:55 -0500 [thread overview]
Message-ID: <20210812133655.GT3406@minyard.net> (raw)
In-Reply-To: <YRTQP9sX0hkTJMTx@shikoro>
On Thu, Aug 12, 2021 at 09:39:43AM +0200, Wolfram Sang wrote:
> 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.
I am sorry, I'll pull it out.
-corey
>
> @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
>
> _______________________________________________
> Openipmi-developer mailing list
> Openipmi-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer
WARNING: multiple messages have this Message-ID (diff)
From: Corey Minyard <minyard@acm.org>
To: Wolfram Sang <wsa@kernel.org>,
Quan Nguyen <quan@os.amperecomputing.com>,
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: [Openipmi-developer] [PATCH v5 1/3] i2c: aspeed: Add slave_enable() to toggle slave mode
Date: Thu, 12 Aug 2021 08:36:55 -0500 [thread overview]
Message-ID: <20210812133655.GT3406@minyard.net> (raw)
In-Reply-To: <YRTQP9sX0hkTJMTx@shikoro>
On Thu, Aug 12, 2021 at 09:39:43AM +0200, Wolfram Sang wrote:
> 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.
I am sorry, I'll pull it out.
-corey
>
> @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
>
> _______________________________________________
> Openipmi-developer mailing list
> Openipmi-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-08-12 13:36 UTC|newest]
Thread overview: 39+ 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 ` Quan Nguyen
2021-07-14 3:38 ` Quan Nguyen
2021-07-14 3:38 ` [PATCH v5 1/3] i2c: aspeed: Add slave_enable() to toggle slave mode Quan Nguyen
2021-07-14 3:38 ` Quan Nguyen
2021-07-14 3:38 ` Quan Nguyen
2021-08-12 7:39 ` Wolfram Sang
2021-08-12 7:39 ` Wolfram Sang
2021-08-12 7:39 ` Wolfram Sang
2021-08-12 13:36 ` Corey Minyard [this message]
2021-08-12 13:36 ` [Openipmi-developer] " Corey Minyard
2021-08-12 13:36 ` Corey Minyard
2021-11-29 19:22 ` Wolfram Sang
2021-11-29 19:22 ` Wolfram Sang
2021-11-29 19:22 ` Wolfram Sang
2021-11-30 2:08 ` Quan Nguyen
2021-11-30 2:08 ` Quan Nguyen
2021-11-30 2:08 ` Quan Nguyen
2021-11-30 10:02 ` Wolfram Sang
2021-11-30 10:02 ` Wolfram Sang
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 ` Quan Nguyen
2021-07-14 3:38 ` Quan Nguyen
2021-07-14 3:38 ` [PATCH v5 3/3] bindings: ipmi: Add binding for " Quan Nguyen
2021-07-14 3:38 ` Quan Nguyen
2021-07-14 3:38 ` Quan Nguyen
2021-07-15 17:43 ` Rob Herring
2021-07-15 17:43 ` Rob Herring
2021-07-15 17:43 ` Rob Herring
2021-07-16 2:45 ` Quan Nguyen
2021-07-16 2:45 ` Quan Nguyen
2021-07-16 2:45 ` Quan Nguyen
2021-07-15 23:32 ` [PATCH v5 0/3] Add " Corey Minyard
2021-07-15 23:32 ` Corey Minyard
2021-07-15 23:32 ` Corey Minyard
2021-07-16 2:47 ` Quan Nguyen
2021-07-16 2:47 ` Quan Nguyen
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=20210812133655.GT3406@minyard.net \
--to=minyard@acm.org \
--cc=linux-aspeed@lists.ozlabs.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.