From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 11796C43381 for ; Mon, 1 Apr 2019 21:34:01 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CD94E20651 for ; Mon, 1 Apr 2019 21:34:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="jfuo6xHi"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=broadcom.com header.i=@broadcom.com header.b="d9gDFFsG" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CD94E20651 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=broadcom.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=sY4+UD9fH+otxpwez6KaUAYvIneG79yIwzk6uL8BPzk=; b=jfuo6xHibYRJ2+ RGOGGpMeOz45rb4G/tnGr6HR/4L30Y6tuF9LP9GqBOCmg0J1ycruewop33CrsBqTYqR3SF3B4FXsU +4i6WrSO/UFCNAbeHLQmkZF44op45mr2sIuDhoJO2Wq3cZpit0GGq2eSEFG8ZT6t7qbVtrjczUGeu RbgaCrL9lZscz+DcIlEEI3s4KLJTLVFMlLCMehuYn3NhIxh/AjAYGny/rSwMiLpX3BUbmwPPDfSus EgWH0EaOMMDlJL2qhRJjQ7oH/V4x5C/WomHHoC+RpfdWqeVJp5jjn2xWjbP/LTF/7YH4XN50CGvhN WfFwLt3s5Usr1TJCf+lw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hB4Z1-0000zo-Ng; Mon, 01 Apr 2019 21:33:55 +0000 Received: from mail-pf1-x442.google.com ([2607:f8b0:4864:20::442]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hB4Yz-0000zU-Ek for linux-arm-kernel@lists.infradead.org; Mon, 01 Apr 2019 21:33:54 +0000 Received: by mail-pf1-x442.google.com with SMTP id 188so5214165pfd.8 for ; Mon, 01 Apr 2019 14:33:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=waGRKUCpcQwPKFT21CsxQU1yb4YT8gwMDwn9XUnCJSw=; b=d9gDFFsGxr+LHlkaoBRJWC3biT5puZJk+/8a5wy+xwkG+xg+VRpwaAM/dkfS4G68NN 5gn0ygPBnR7G6zL5KgK6DUaNAs/y+VKrpPjtRPSpz7rtpJQV3Caa0aIlVOOc8loXX+QG KDQcDE4Q7QnEPUMxppuY3YDLUWacZGEcMDf/0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=waGRKUCpcQwPKFT21CsxQU1yb4YT8gwMDwn9XUnCJSw=; b=RTo/T4yeQqFeoahDNCpMF0SASA/6RsnTefXtvJVKgQzMkHB6aircK8FIjT6uHoKOSg ENlvxCeK3rmW/OsTIrG/u2A8wUoIO35OZ2/oya0yULFr68L4gsEU2DxzetCa5kWZ1BZq yaM3JRRYLaxh+CqqTaSqj167jMZmRUC+oGvbql17JeqHo7HzYy2vAjftmu4EF6hkv8b2 85LXcHgD37Zb2Q4Io4Lilsu3f74Ip6PRm5jV+QrtCEGi5R9ae9gtd7hTE6RW0GEfdkYH /NamlBN2eibw+f5t7NFGy/W4WkWnyzJbvA/6KnQxUTvjUbc/6DX6iB2oM5QFpgV2vGIs ZPVg== X-Gm-Message-State: APjAAAVYAU+l0XTaATONjicRWTbLN9SpTPyT4UR4yYKJEQ5s6kNQG7Ny rHDri2NpZDBWGO7LT5vVRbeSxQ== X-Google-Smtp-Source: APXvYqxHyTIzfvsmBLQIkRy1uuy4c8dK1RGtWrjD+9N/QigRz8r3NTb0e6Wd3/w6lT8/pLs0cNFt4w== X-Received: by 2002:a63:78ce:: with SMTP id t197mr50095684pgc.314.1554154432220; Mon, 01 Apr 2019 14:33:52 -0700 (PDT) Received: from [10.136.8.252] ([192.19.228.250]) by smtp.gmail.com with ESMTPSA id v15sm14576501pff.105.2019.04.01.14.33.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 01 Apr 2019 14:33:51 -0700 (PDT) Subject: Re: [PATCH v5 2/8] i2c: iproc: Add slave mode support To: Wolfram Sang References: <20190214175725.60462-1-ray.jui@broadcom.com> <20190214175725.60462-3-ray.jui@broadcom.com> <20190327221452.GA15396@kunai> From: Ray Jui Message-ID: Date: Mon, 1 Apr 2019 14:33:47 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.0 MIME-Version: 1.0 In-Reply-To: <20190327221452.GA15396@kunai> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190401_143353_518576_8DC9FF4E X-CRM114-Status: GOOD ( 19.83 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree@vger.kernel.org, Rayagonda Kokatanur , linux-kernel@vger.kernel.org, Shreesha Rajashekar , Rob Herring , bcm-kernel-feedback-list@broadcom.com, linux-i2c@vger.kernel.org, Michael Cheng , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Wolfram/Rayagonda, On 3/27/2019 3:14 PM, Wolfram Sang wrote: > >> +static void bcm_iproc_i2c_slave_init( >> + struct bcm_iproc_i2c_dev *iproc_i2c, bool need_reset) >> +{ >> + u32 val; >> + >> + if (need_reset) { >> + /* put controller in reset */ >> + val = readl(iproc_i2c->base + CFG_OFFSET); >> + val |= BIT(CFG_RESET_SHIFT); >> + writel(val, iproc_i2c->base + CFG_OFFSET); >> + >> + /* wait 100 usec per spec */ >> + udelay(100); >> + >> + /* bring controller out of reset */ >> + val &= ~(BIT(CFG_RESET_SHIFT)); >> + writel(val, iproc_i2c->base + CFG_OFFSET); >> + } >> + >> + /* flush TX/RX FIFOs */ >> + val = (BIT(S_FIFO_RX_FLUSH_SHIFT) | BIT(S_FIFO_TX_FLUSH_SHIFT)); >> + writel(val, iproc_i2c->base + S_FIFO_CTRL_OFFSET); > > Will flushing FIFOs work when a slave is register while a master > transfer is on-going at the same time? > Okay, as you pointed out in a subsequent email, this can't happen. >> + >> + /* RANDOM SLAVE STRETCH time - 20ms*/ > > What is a "random stretch time"? 20ms sounds like a lot. Also, missing > space before comment terminator. > Rayagonda, Could you please help to comment on the choice of the 20 ms to allow clock stretch from the slave? In probably all cases, the slave should not need more than 1 ms? 20 ms does seem way too long as Wolfram pointed out. Will fix the missing space before comment terminator. >> @@ -224,22 +473,25 @@ static int bcm_iproc_i2c_init(struct bcm_iproc_i2c_dev *iproc_i2c) >> >> /* put controller in reset */ >> val = readl(iproc_i2c->base + CFG_OFFSET); >> - val |= 1 << CFG_RESET_SHIFT; >> - val &= ~(1 << CFG_EN_SHIFT); >> + val |= BIT(CFG_RESET_SHIFT); >> + val &= ~(BIT(CFG_EN_SHIFT)); >> writel(val, iproc_i2c->base + CFG_OFFSET); >> >> /* wait 100 usec per spec */ >> udelay(100); >> >> /* bring controller out of reset */ >> - val &= ~(1 << CFG_RESET_SHIFT); >> + val &= ~(BIT(CFG_RESET_SHIFT)); >> writel(val, iproc_i2c->base + CFG_OFFSET); >> >> /* flush TX/RX FIFOs and set RX FIFO threshold to zero */ >> - val = (1 << M_FIFO_RX_FLUSH_SHIFT) | (1 << M_FIFO_TX_FLUSH_SHIFT); >> + val = (BIT(M_FIFO_RX_FLUSH_SHIFT) | BIT(M_FIFO_TX_FLUSH_SHIFT)); >> writel(val, iproc_i2c->base + M_FIFO_CTRL_OFFSET); >> /* disable all interrupts */ >> - writel(0, iproc_i2c->base + IE_OFFSET); >> + val = readl(iproc_i2c->base + IE_OFFSET); >> + val &= ~(IE_M_ALL_INTERRUPT_MASK << >> + IE_M_ALL_INTERRUPT_SHIFT); >> + writel(val, iproc_i2c->base + IE_OFFSET); > > This block looks unrelated, but I won't be too strict here... > >> + case M_CMD_STATUS_FIFO_UNDERRUN: >> + dev_dbg(iproc_i2c->device, "FIFO under-run\n"); >> + return -ENXIO; >> + >> + case M_CMD_STATUS_RX_FIFO_FULL: >> + dev_dbg(iproc_i2c->device, "Master Rx FIFO full > 10ms\n"); >> + return -ETIMEDOUT; >> + > > ... however, this looks really unrelated to me. This is about master > transmission, or? This should be submitted in a separate commit. Will do that in the next iteration of patch series. > > Rest looks OK. > Thanks, Ray _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel