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=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_2 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 235B2C31E40 for ; Fri, 9 Aug 2019 15:51:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 01FAF2089E for ; Fri, 9 Aug 2019 15:51:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2436861AbfHIPvp (ORCPT ); Fri, 9 Aug 2019 11:51:45 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:53532 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2436723AbfHIPvp (ORCPT ); Fri, 9 Aug 2019 11:51:45 -0400 Received: from localhost (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id E1DC928CF3E; Fri, 9 Aug 2019 16:51:42 +0100 (BST) Date: Fri, 9 Aug 2019 17:51:40 +0200 From: Boris Brezillon To: Tomer Maimon Cc: Mark Brown , Rob Herring , Mark Rutland , Vignesh Raghavendra , Boris Brezillon , Avi Fishman , Tali Perry , Patrick Venture , Nancy Yuen , Benjamin Fair , linux-spi@vger.kernel.org, devicetree , OpenBMC Maillist , Linux Kernel Mailing List Subject: Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver Message-ID: <20190809175140.77747c8d@collabora.com> In-Reply-To: References: <20190808131448.349161-1-tmaimon77@gmail.com> <20190808131448.349161-3-tmaimon77@gmail.com> <20190808173232.4d79d698@collabora.com> <20190809172557.346e7c41@collabora.com> Organization: Collabora X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 9 Aug 2019 18:47:08 +0300 Tomer Maimon wrote: > On Fri, 9 Aug 2019 at 18:26, Boris Brezillon > wrote: > > > On Fri, 9 Aug 2019 18:26:23 +0300 > > Tomer Maimon wrote: > > > > > Hi Boris, > > > > > > Thanks a lot for your comment. > > > > > > On Thu, 8 Aug 2019 at 18:32, Boris Brezillon < > > boris.brezillon@collabora.com> > > > wrote: > > > > > > > On Thu, 8 Aug 2019 16:14:48 +0300 > > > > Tomer Maimon wrote: > > > > > > > > > > > > > + > > > > > +static const struct spi_controller_mem_ops npcm_fiu_mem_ops = { > > > > > + .exec_op = npcm_fiu_exec_op, > > > > > > > > No npcm_supports_op()? That's suspicious, especially after looking at > > > > the npcm_fiu_exec_op() (and the functions called from there) where the > > > > requested ->buswidth seems to be completely ignored... > > > > > > > > Sorry but I do not fully understand it, do you mean a support for the > > > buswidth? > > > If yes it been done in the UMA functions as follow: > > > > > > uma_cfg |= ilog2(op->cmd.buswidth); > > > uma_cfg |= ilog2(op->addr.buswidth) << > > > NPCM_FIU_UMA_CFG_ADBPCK_SHIFT; > > > uma_cfg |= ilog2(op->data.buswidth) << > > > NPCM_FIU_UMA_CFG_WDBPCK_SHIFT; > > > uma_cfg |= op->addr.nbytes << > > NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT; > > > regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, > > op->addr.val); > > > > > > > Hm, the default supports_op() implementation might be just fine for > > your use case. But there's one thing you still need to check: the > > number of addr cycles (or address size as you call it in this driver). > > Looks like your IP is limited to 4 address cycles, if I'm right, you > > should reject any operation that have op->addr.nbytes > 4. I also > > > Indeed our IP limited to 4 address cycle (bytes) do we have NOR Flash with > more than 32bit address? spi-mem is not only about spi-nor, it can be used for any kind of memory (NOR, NAND, SRAM, ...) or even to communicate with an FGPA, so yes, you have to take care of that. > I will add this limitation thanks! > > > wonder if there's a limitation on the data size you can have on a > > single transfer. If there's one you should implement ->adjust_op() too. > > > there is a limitation in a single transfer but I handle it in the > npcm_fiu_manualwrite > function. > Do you suggest to use ->adjust_op() instead? Yes, should be exposed through ->adjust_op() => the caller needs to know when a new operation (one containing an opcode+address) is issued, because sometimes such splits are not supported by the memory.