Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
User-Agent: Cyrus-JMAP/3.3.0-324-g0f99587-fm-20200916.004-g0f995879
Mime-Version: 1.0
Message-Id: <61537381-b1ea-48d3-b445-a33e355f8338@www.fastmail.com>
In-Reply-To: <20200916155651.GA90122@roeck-us.net>
References: <20200914122811.3295678-1-andrew@aj.id.au>
 <20200914122811.3295678-3-andrew@aj.id.au>
 <71067b18-c4bc-533a-0069-f21069c5fd0d@roeck-us.net>
 <48962472-b025-4b0d-90e9-60469bebf206@www.fastmail.com>
 <20200916155651.GA90122@roeck-us.net>
Date:   Thu, 17 Sep 2020 10:03:09 +0930
From:   "Andrew Jeffery" <andrew@aj.id.au>
To:     "Guenter Roeck" <linux@roeck-us.net>
Cc:     linux-hwmon@vger.kernel.org, linux-i2c@vger.kernel.org,
        "Jean Delvare" <jdelvare@suse.com>, wsa@kernel.org,
        "Joel Stanley" <joel@jms.id.au>, linux-kernel@vger.kernel.org
Subject: =?UTF-8?Q?Re:_[RFC_PATCH_2/2]_hwmon:_(pmbus/ucd9000)_Throttle_SMBus_tran?=
 =?UTF-8?Q?sfers_to_avoid_poor_behaviour?=
Content-Type: text/plain
Precedence: bulk


On Thu, 17 Sep 2020, at 01:26, Guenter Roeck wrote:
> > I've had a look at these two examples. As you suggest the delays in zl6100.c 
> > look pretty similar to what this series implements in the i2c core. I'm finding 
> > it hard to dislodge the feeling that open-coding the waits is error prone, but 
> > to avoid that and not implement the waits in the i2c core means having almost 
> > duplicate implementations of handlers for i2c_smbus_{read,write}*() and 
> > pmbus_{read,write}*() calls in the driver.
> > 
> 
> Not sure I can follow you here. Anyway, it seems to me that you are set on
> an implementation in the i2c core. I personally don't like that approach,

Not really set on it, but it does seem convenient. I'm looking at whether 
delays resolve the issues we have with the max31785 as well (I have a bunch of 
patches that introduce retries under the various circumstances we've hit poor 
behaviour).

> but I'll accept a change in the ucd9000 driver to make use of it. Please
> leave the zl6100 code alone, though - it took me long enough to get that
> working, and I won't have time to test any changes.

No worries. If you don't have time to test changes it reduces the motivation to 
find a general approach, and so maybe isolating the work-arounds to the ucd9000 
is the way to go.

Thanks for the feedback.

Andrew