Received: by 2002:a25:5b86:0:0:0:0:0 with SMTP id p128csp424262ybb; Thu, 28 Mar 2019 05:26:05 -0700 (PDT) X-Google-Smtp-Source: APXvYqzLPmeSre45wkMbsabdqun6ZNVhSnZ6kfxKazcw92CA8b6FGfeQks2zhtRNCpfFynHqf9+8 X-Received: by 2002:a63:5a4b:: with SMTP id k11mr39280887pgm.119.1553775965166; Thu, 28 Mar 2019 05:26:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1553775965; cv=none; d=google.com; s=arc-20160816; b=WmmIegEia2HvY1+Ra6uczDEJhw9pHD8pFWfPeNBV/pb2lJkTv0xg6zjz9zVrYvLfvT /WsomS/e80sRp8ABL3LzQPGsNeBGkaDoyBpBOeLQqBtfVu2jD3igM3gkv1ayS4vXCXj2 FD9CblcdLEbIHCExvBujDd+nRL3j1sn3gBzqNuSsaOLK0gjR7FeOIRIyJ/euzso6pouc tQeTdMHpA3+/f5ylf7mBR/52szE22MlgjsQe2AdTOHoMEWkC1rkqrYh4tLu8WDA6GZwm ZNdg6/jWUAqkkqpaOa2vXeXO8TA8lIO+8bxuVQ7VH0Hj8zcBULiH7hEc3UdasO0A7/Al NJxA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dkim-signature; bh=Zfb1rKeBLtVHVUw32/S4m5WUS/BSiXJ/wEQMyqmIJBU=; b=rvt/D1kYtgLdlvDe9AlI/zwJCheiRQAVgIpzgLcHiDeZE0bcfEaXFUY49KvcERxoZx 9US5hXvHKzrLCOXzG2E6En+t8QTGA8wA+SqXH9r9mGmz2NBQ1J0SYVp87aJ4Kbl1TMzK gU5vZC/Zqb7ss/pH8aHMRjwr4z3QqmlWPePzGgqGbLU+G8fnczupC4yNHt3vdBFdHieO DY0zfPn3nVmNNvA8iRdgA8lAAIK973Ftqbttux3CMv74JOPPuMtTKScfkgcR7GXZmI18 nX9MQ5FijN+aE7fv+ucDtwcdHldDezmLyHlSffrfauhL1S9zCz6QxYdWOfTXj0Mwi4oF H7gw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=jnPnitFs; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id l91si16242374plb.336.2019.03.28.05.25.49; Thu, 28 Mar 2019 05:26:05 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=jnPnitFs; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726299AbfC1MXf (ORCPT + 99 others); Thu, 28 Mar 2019 08:23:35 -0400 Received: from mail-qk1-f193.google.com ([209.85.222.193]:33398 "EHLO mail-qk1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725846AbfC1MXf (ORCPT ); Thu, 28 Mar 2019 08:23:35 -0400 Received: by mail-qk1-f193.google.com with SMTP id k189so11980569qkc.0 for ; Thu, 28 Mar 2019 05:23:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=Zfb1rKeBLtVHVUw32/S4m5WUS/BSiXJ/wEQMyqmIJBU=; b=jnPnitFsxjGqJZyK37N1W/MM23bZXgqCsFQ0HC6bjz314FXe4oMNXVi10c1oyvFf/Q 8IRT/AflNOxI163Msxqi9i6Z8JC2iGm0Zol2Mm0NJH+1+Gwe5scITUnVk3vSUj4b9Qvy Nab67GoLOkEXEGtQv93Dk4m9VjX9Os0gW9LBDW3IynpfXApgnsAzBe0PDOC7w4HyjIYO FVcfaktfSY+UyAR7zDDDVnU0v+/wFA9oMNsGg7/BIEGLOer+d3V+mVg3qpWvnS+sCs4j mJ7/OjfhRK7lia5yY/m056UhCaCTPCfqeI+m3j9Ld1l/QQhAfVxefomgpKrAweru4Udc CxYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=Zfb1rKeBLtVHVUw32/S4m5WUS/BSiXJ/wEQMyqmIJBU=; b=qXUJzBbQa3iS7we69DoJRz8W6zg525LMpqVzXIcAUGQE5jZAjK9CtY/+yhFsc8+fSs pZMdLt+W1q/EKl+bUTA5jwU685uViRlkxSU4tL2US1eV99+0ZRquNA58diqdFhG3P2MH q8goKu0XpFqncwMG+3DORHyJ0EWBBH3pYIfzErkS5/E0QNGsbBU5wnESsRrvImBucXn9 OVFlCO2ZxrJeVodJDAcprLHDoTJVGKp4Dc7POruE9HgJlW6eZVxMGz7jlmxfcOcwuNsd 1r9iPf26JdlunlW3aLBp/gR+6IRtjW7v/05aN3dpLC+qqsATXtz3bKLe3/3/1rFL3ehW +7uQ== X-Gm-Message-State: APjAAAXc/mPmqnfcL10+XKjqsHTs0j39J0PlMcHrq+fIdd6c2IEdzNv4 hQiYdIBoqgM2bTLSamCoIz2jploJWd8= X-Received: by 2002:ae9:e894:: with SMTP id a142mr26224562qkg.14.1553775813891; Thu, 28 Mar 2019 05:23:33 -0700 (PDT) Received: from jfddesk.Sonatest.net (ipagstaticip-d73c7528-4de5-0861-800b-03d8b15e3869.sdsl.bell.ca. [174.94.156.236]) by smtp.gmail.com with ESMTPSA id e15sm12092362qkl.80.2019.03.28.05.23.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Mar 2019 05:23:33 -0700 (PDT) From: Jean-Francois Dagenais To: linux-kernel@vger.kernel.org, greg@kroah.com Cc: zbr@ioremap.net, manio@skyboo.net, Jean-Francois Dagenais Subject: [PATCH v3] w1: ds2408: reset on output_write retry with readback Date: Thu, 28 Mar 2019 08:22:44 -0400 Message-Id: <20190328122244.32126-1-jeff.dagenais@gmail.com> X-Mailer: git-send-email 2.11.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org When we have success in 'Channel Access Write' but reading back latch states fails, a write is retried without doing a proper slave reset. This leads to protocol errors as the slave treats the next 'Channel Access Write' as the continuation of previous command. This commit is fixing this by making sure if the retry loop re-runs, a reset is performed, whatever the failure (CONFIRM_BYTE or the read back). The loop was quite due for a cleanup and this change mandated it. By isolating the CONFIG_W1_SLAVE_DS2408_READBACK case into it's own function, we vastly reduce the visual and branching(runtime and compile-time) noise. Reported-by: Mariusz Bialonczyk Tested-by: Mariusz Bialonczyk Signed-off-by: Jean-Francois Dagenais --- drivers/w1/slaves/w1_ds2408.c | 76 ++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/drivers/w1/slaves/w1_ds2408.c b/drivers/w1/slaves/w1_ds2408.c index b535d5ec35b6..92e8f0755b9a 100644 --- a/drivers/w1/slaves/w1_ds2408.c +++ b/drivers/w1/slaves/w1_ds2408.c @@ -138,14 +138,37 @@ static ssize_t status_control_read(struct file *filp, struct kobject *kobj, W1_F29_REG_CONTROL_AND_STATUS, buf); } +#ifdef fCONFIG_W1_SLAVE_DS2408_READBACK +static bool optional_read_back_valid(struct w1_slave *sl, u8 expected) +{ + u8 w1_buf[3]; + + if (w1_reset_resume_command(sl->master)) + return false; + + w1_buf[0] = W1_F29_FUNC_READ_PIO_REGS; + w1_buf[1] = W1_F29_REG_OUTPUT_LATCH_STATE; + w1_buf[2] = 0; + + w1_write_block(sl->master, w1_buf, 3); + + return (w1_read_8(sl->master) == expected); +} +#else +static bool optional_read_back_valid(struct w1_slave *sl, u8 expected) +{ + return true; +} +#endif + static ssize_t output_write(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buf, loff_t off, size_t count) { struct w1_slave *sl = kobj_to_w1_slave(kobj); u8 w1_buf[3]; - u8 readBack; unsigned int retries = W1_F29_RETRIES; + ssize_t bytes_written = -EIO; if (count != 1 || off != 0) return -EFAULT; @@ -155,54 +178,33 @@ static ssize_t output_write(struct file *filp, struct kobject *kobj, dev_dbg(&sl->dev, "mutex locked"); if (w1_reset_select_slave(sl)) - goto error; + goto out; - while (retries--) { + do { w1_buf[0] = W1_F29_FUNC_CHANN_ACCESS_WRITE; w1_buf[1] = *buf; w1_buf[2] = ~(*buf); - w1_write_block(sl->master, w1_buf, 3); - readBack = w1_read_8(sl->master); + w1_write_block(sl->master, w1_buf, 3); - if (readBack != W1_F29_SUCCESS_CONFIRM_BYTE) { - if (w1_reset_resume_command(sl->master)) - goto error; - /* try again, the slave is ready for a command */ - continue; + if (w1_read_8(sl->master) == W1_F29_SUCCESS_CONFIRM_BYTE && + optional_read_back_valid(sl, *buf)) { + bytes_written = 1; + goto out; } -#ifdef CONFIG_W1_SLAVE_DS2408_READBACK - /* here the master could read another byte which - would be the PIO reg (the actual pin logic state) - since in this driver we don't know which pins are - in and outs, there's no value to read the state and - compare. with (*buf) so end this command abruptly: */ if (w1_reset_resume_command(sl->master)) - goto error; + goto out; /* unrecoverable error */ + /* try again, the slave is ready for a command */ + } while (--retries); - /* go read back the output latches */ - /* (the direct effect of the write above) */ - w1_buf[0] = W1_F29_FUNC_READ_PIO_REGS; - w1_buf[1] = W1_F29_REG_OUTPUT_LATCH_STATE; - w1_buf[2] = 0; - w1_write_block(sl->master, w1_buf, 3); - /* read the result of the READ_PIO_REGS command */ - if (w1_read_8(sl->master) == *buf) -#endif - { - /* success! */ - mutex_unlock(&sl->master->bus_mutex); - dev_dbg(&sl->dev, - "mutex unlocked, retries:%d", retries); - return 1; - } - } -error: +out: mutex_unlock(&sl->master->bus_mutex); - dev_dbg(&sl->dev, "mutex unlocked in error, retries:%d", retries); - return -EIO; + dev_dbg(&sl->dev, "%s, mutex unlocked retries:%d\n", + (bytes_written > 0) ? "succeeded" : "error", retries); + + return bytes_written; } -- 2.11.0