From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752652AbdF0I3M (ORCPT ); Tue, 27 Jun 2017 04:29:12 -0400 Received: from mail-qk0-f169.google.com ([209.85.220.169]:33914 "EHLO mail-qk0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752549AbdF0I2d (ORCPT ); Tue, 27 Jun 2017 04:28:33 -0400 MIME-Version: 1.0 In-Reply-To: <20170623184305.xqqhjgrbdmgk6lsr@ninjato> References: <20170620211516.13095-1-brendanhiggins@google.com> <20170620211516.13095-4-brendanhiggins@google.com> <20170623184305.xqqhjgrbdmgk6lsr@ninjato> From: Brendan Higgins Date: Tue, 27 Jun 2017 01:28:31 -0700 Message-ID: Subject: Re: [PATCH v11 3/4] i2c: aspeed: added driver for Aspeed I2C To: Wolfram Sang Cc: Rob Herring , Mark Rutland , gregkh@linuxfoundation.org, davem@davemloft.net, mchehab@kernel.org, Joel Stanley , martin.petersen@oracle.com, Benjamin Herrenschmidt , linux-i2c@vger.kernel.org, devicetree@vger.kernel.org, Linux Kernel Mailing List , OpenBMC Maillist Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 23, 2017 at 11:43 AM, Wolfram Sang wrote: > On Tue, Jun 20, 2017 at 02:15:15PM -0700, Brendan Higgins wrote: >> Added initial master support for Aspeed I2C controller. Supports >> fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed. >> >> Signed-off-by: Brendan Higgins > > Applied to for-next, thanks for all the hard work! Thanks for the patience! > > One question however which can be solved incrementally if needed: > >> + if (command & ASPEED_I2CD_SDA_LINE_STS) { >> + /* Bus is idle: no recovery needed. */ >> + if (command & ASPEED_I2CD_SCL_LINE_STS) >> + goto out; >> + dev_dbg(bus->dev, "SCL hung (state %x), attempting recovery\n", >> + command); >> + >> + reinit_completion(&bus->cmd_complete); >> + writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG); > > If SCL is stuck low, how do you want to send a STOP? > Fair point. I should probably drop that in the future and just do a reset, and even then, doing a reset is probably just wishful thinking. If a slave is holding down SCL, we are pretty screwed.