linux-kernel - Re: [Openipmi-developer] [PATCH v5 1/3] i2c: aspeed: Add slave

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210812133655.GT3406@minyard.net>
Date:   Thu, 12 Aug 2021 08:36:55 -0500
From:   Corey Minyard <minyard@....org>
To:     Wolfram Sang <wsa@...nel.org>,
        Quan Nguyen <quan@...amperecomputing.com>,
        Rob Herring <robh+dt@...nel.org>,
        Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>,
        Brendan Higgins <brendanhiggins@...gle.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        openipmi-developer@...ts.sourceforge.net,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-aspeed@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        linux-i2c@...r.kernel.org,
        Open Source Submission <patches@...erecomputing.com>,
        Phong Vo <phong@...amperecomputing.com>,
        "Thang Q . Nguyen" <thang@...amperecomputing.com>
Subject: Re: [Openipmi-developer] [PATCH v5 1/3] i2c: aspeed: Add
 slave_enable() to toggle slave mode

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@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openipmi-developer