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=-6.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=no 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 BDB14C00A89 for ; Thu, 5 Nov 2020 07:47:25 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 412D920936 for ; Thu, 5 Nov 2020 07:47:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="TITEejhL"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="PUIS5sSO" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 412D920936 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.microsoft.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:MIME-Version:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:References:In-Reply-To:Message-Id:Date:Subject:To: From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=FoKYIVUzb0+CoXqAlFzazpUUP+cTi+TpiPJVr5Aba58=; b=TITEejhLlLzjlrfsUBLA7gimIN m4Bgg7xe4kWKSiOAo+3gx684xoqp73Sj4L1LKVM3hoLuiOjwa3KZebXyYhhHk7z2rFwAc745QCCz5 6G4cKkojV74taqNgAoWrsD7U8zO9xAFRhdXlwM1Ub8v8mFjX75veJiVFw/JXcVoLQXt+W2flZbaaY 0ES5Bx4HxSS/BMNOXkD9TdBn/zylm4WG+Jl6CCG4kIUxVkuKbE1auoOnfXoj2t3xGTjLecVjYe44C OMigG+U4HqsEtRoweHqCypwY0TBmKLeeKyWwJ3ebPli8ykIgqMlSw9yeTiRFsHUDTkTjz6k6gmqpq s45hJ7Xg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kaZys-0005jC-Ov; Thu, 05 Nov 2020 07:46:50 +0000 Received: from linux.microsoft.com ([13.77.154.182]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kaZyp-0005iK-I4 for linux-arm-kernel@lists.infradead.org; Thu, 05 Nov 2020 07:46:48 +0000 Received: by linux.microsoft.com (Postfix, from userid 1046) id 5B48D20B4905; Wed, 4 Nov 2020 23:46:45 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 5B48D20B4905 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1604562405; bh=+4vC5pdHWhZcsC4RLqsaXzruecfzwJvr9SPbBGML92k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=PUIS5sSOund47b2bGCatf6rcJhvYsR39zR8hdPoD83WmfHmrpB6rxs14MbPhsoEd0 KFNmGAaZHP9tI4uCmlbnRUskomHcymw9Iya0xpHsON9Wgsm0Eu8BBtKoryEpgkXCdR nF4PlSCGPSYxyH65Bt0DWHlMENv9ctS8mwcvB7R0= From: Dhananjay Phadke To: ray.jui@broadcom.com Subject: Re: [PATCH v3 5/6] i2c: iproc: handle master read request Date: Wed, 4 Nov 2020 23:46:45 -0800 Message-Id: <1604562405-25414-1-git-send-email-dphadke@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <97a8d7bf-e877-c03a-f789-16cac9bb2643@broadcom.com> References: <97a8d7bf-e877-c03a-f789-16cac9bb2643@broadcom.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201105_024647_679597_307B25DA X-CRM114-Status: GOOD ( 14.70 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: lori.hikichi@broadcom.com, f.fainelli@gmail.com, sbranden@broadcom.com, rayagonda.kokatanur@broadcom.com, rjui@broadcom.com, brendanhiggins@google.com, linux-kernel@vger.kernel.org, wsa@kernel.org, bcm-kernel-feedback-list@broadcom.com, dphadke@linux.microsoft.com, andriy.shevchenko@linux.intel.com, linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, 4 Nov 2020 10:01:06 -0800, Ray Jui wrote: >>>> +#define MAX_SLAVE_RX_PER_INT 10 >>>> >>> >>>> In patch [3/6], you've enabled IS_S_RX_THLD_SHIFT in slave ISR bitmask, >>>> however it's not actually used in processing rx events. >>>> >>>> Instead of hardcoding this threshold here, it's better to add a >>>> device-tree knob for rx threshold, program it in controller and handle >>>> that RX_THLD interrupt. This will give more flexibility to drain the rx >>>> fifo earlier than - >>>> (1) waiting for FIFO_FULL interrupt for transactions > 64B. >>>> (2) waiting for start of read transaction in case of master write-read. >> >> Yes this is one way to implement. >> But do you see any issue in batching 64 bytes at a time in case of >> transaction > 64 Bytes. >> I feel batching will be more efficient as it avoids more number of >> interrupts and hence context switch. >> >>> >>> The Device Tree is really intended to describe the hardware FIFO size, >>> not watermarks, as those tend to be more of a policy/work load decision. >>> Maybe this is something that can be added as a module parameter, or >>> configurable via ioctl() at some point. >> > >Yes, DT can have properties to describe the FIFO size, if there happens >to be some variants in the HW blocks in different versions. But that is >not the case here. DT should not be used to control SW/use case specific >behavior. So the suggestion was to set HW threshold for rx fifo interrupt, not really a SW property. By setting it in DT, makes it easier to customize for target system, module param needs or ioctl makes it dependent on userpsace to configure it. The need for tasklet seems to arise from the fact that many bytes are left in the fifo. If there's a common problem here, such tasklet would be needed in i2c subsys rather than controller specific tweak, akin to how networking uses NAPI or adding block transactions to the interface? For master write-read event, it seems both IS_S_RD_EVENT_SHIFT and IS_S_RX_EVENT_SHIFT are detected, which implies that core is late to drain rx fifo i.e. write is complete and the read has started on the bus? Thanks, Dhananjay _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel