Hi Joel,
On 9/26/2018 8:11 PM, Joel Stanley wrote:
On Thu, 27 Sep 2018 at 01:58, Jae Hyun Yoo <jae.hyun.yoo@xxxxxxxxxxxxxxx> wrote:
+/* Timeout */
+#define ASPEED_I2C_BUS_TIMEOUT_US_DEFAULT (5 * 1000 * 1000)
The 5 seconds time out is way too long. On a system that doesn't have
functional i2c, this holds up boot for a long time as most i2c client
drivers try to initialise their device and fail. I realise you're not
changing the value, but we should pick a better default. 1 second?
Half a second?
I agree with you. We could probably use 1 second as default which can
cover the most of general cases. If so, we don't need to make the
default setting in this driver because i2c-core-base will default
adap->timeout to 1 second if the value is 0 when a driver registers an
adapter. Will fix this code.
+ ret = of_property_read_u32(pdev->dev.of_node, "aspeed,timeout",
+ &timeout_us);
Can we make this binding generic? It's not specific to aspeed's
hardware. Getting the value could even part of the i2c core.
I read the previous thread with Wolfram. I think this would still fit
with what Wolfram suggested, but please forgive my jetlagged brain if
I've missed something.
It followed the way of the existing i2c-mpc driver uses 'fsl,timeout'
for the same purpose. Though, I also want to make it as a generic as you
suggested like 'timeout' in milliseconds unit, not in microseconds unit.
If making it a generic property is acceptable, I'll fix it too.
Wolfram, can you please share your thought on it?
Thanks,
Jae
