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 141BDC388F2 for ; Fri, 6 Nov 2020 17:42:26 +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 81DE3217A0 for ; Fri, 6 Nov 2020 17:42: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="QW5ZdFgw"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="bcYquINM" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 81DE3217A0 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=bTeh/XBXAnp4VKXvIrplgMiWha5gqVh+AdWsePXGCn4=; b=QW5ZdFgwV9IJ4JbTniWP4DbJdc QqC5gecP3NoFfe4Dcr2RFFbI0lS2LbK9mmKZuvVmeYAwHidlHU4LEHxqSWKxeqTfs6QgysXABhfEw F6H4bZAK0LmG16okcDPETSpp6fr49unhWf0GtqTg5mUf3r1JzFrxU3IBijTo/bx6wQ+fLDI944oxk K/Wt1t8w7MVDLhoInEsEXgJ3W5OdG8oIk1FdVpj2iozM3ARHI2NWX758a3bNRbvkldVv3nwrdfdSw 6hWUadg/Y81j0LDk2I9vEBAEyGr6ILTRq7fKd2QLcnXeSwE8ZZuQWe/O0zpE97ktHSSJZYBqnZTkI rhvlgvyw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kb5jv-00017Z-6v; Fri, 06 Nov 2020 17:41:31 +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 1kb5js-00016q-IL for linux-arm-kernel@lists.infradead.org; Fri, 06 Nov 2020 17:41:29 +0000 Received: by linux.microsoft.com (Postfix, from userid 1046) id 226BF20BE4A6; Fri, 6 Nov 2020 09:41:26 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 226BF20BE4A6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1604684486; bh=/E8p26BZs+ElXJp0kzG7i/GtDQpHpgqTRJRJOBKzc7c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bcYquINMUl7AEexnx6z825WlUruPKZPoDg6YR5G5OiuWryDDlOi3zdvH5XVoy/t7g 7yDwQxPun5EDf9OFv7INJ+kgnKh7aByCLuPGd7D2b6BsroV0beZRzydMIVDSXSmHbh kyx8Fe7Yi1eI+e4e5LBmwYyoIEcEylyxVIzTgbPw= From: Dhananjay Phadke To: rayagonda.kokatanur@broadcom.com Subject: Re: [PATCH v3 5/6] i2c: iproc: handle master read request Date: Fri, 6 Nov 2020 09:41:26 -0800 Message-Id: <1604684486-16272-1-git-send-email-dphadke@linux.microsoft.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: References: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201106_124128_732253_19AC14AF X-CRM114-Status: GOOD ( 13.97 ) 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, rjui@broadcom.com, brendanhiggins@google.com, linux-kernel@vger.kernel.org, wsa@kernel.org, bcm-kernel-feedback-list@broadcom.com, dphadke@linux.microsoft.com, ray.jui@broadcom.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 Thu, 5 Nov 2020 15:13:04 +0530, Rayagonda Kokatanur wrote: >> 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? > >Yes it's true that for master write-read events both >IS_S_RD_EVENT_SHIFT and IS_S_RX_EVENT_SHIFT are coming together. >So before the slave starts transmitting data to the master, it should >first read all data from rx-fifo i.e. complete master write and then >process master read. > >To minimise interrupt overhead, we are batching 64bytes. >To keep isr running for less time, we are using a tasklet. >Again to keep the tasklet not running for more than 20u, we have set >max of 10 bytes data read from rx-fifo per tasklet run. > >If we start processing everything in isr and using rx threshold >interrupt, then isr will run for a longer time and this may hog the >system. >For example, to process 10 bytes it takes 20us, to process 30 bytes it >takes 60us and so on. >So is it okay to run isr for so long ? > >Keeping all this in mind we thought a tasklet would be a good option >and kept max of 10 bytes read per tasklet. > >Please let me know if you still feel we should not use a tasklet and >don't batch 64 bytes. Deferring to tasklet is OK, could use a kernel thread (i.e. threaded_irq) as i2c rate is quite low. But do enable rx_threshold and read out early. This will avoid fifo full or master write-read situation where lot of bytes must be drained from rx fifo before serving tx fifo (avoid tx underrun). Best would have been setting up DMA into mem (some controllers seem capable). In absence of that, it's a trade off: if rx intr threshold is low, there will be more interrupts, but less time spent in each. Default could still be 64B or no-thresh (allow override in dtb). Few other comments - >+ /* schedule tasklet to read data later */ >+ tasklet_schedule(&iproc_i2c->slave_rx_tasklet); >+ >+ /* clear only IS_S_RX_EVENT_SHIFT interrupt */ >+ iproc_i2c_wr_reg(iproc_i2c, IS_OFFSET, >+ BIT(IS_S_RX_EVENT_SHIFT)); >+ } Why clearing one rx interrupt bit here after scheduling tasklet? Should all that be done by tasklet? Also should just return after scheduling tasklet? Thanks, Dhananjay _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel