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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 268A9C64EC4 for ; Wed, 8 Mar 2023 09:06:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=OAHrmcJcPWPWv/SddOw6/aDCBj28MPG8l+gfrDnLCps=; b=PifZHBAYcR7L9z KMlg089YX5T3fRrxfemXATuY+nn90wKKBUhjc44xWQXg0ULEkMKYVY18BIlivCx9am4IhxbHZ7rcg wzjTWGO5Edh6qMSE+7ae8IoxpdmEQxog90YZYGTNf//EEUiXanPxHUwEYCJOEx+nrTl3aUkcvMwa7 u/uIkkQaaqw3LjhTSLLtBY9BexdSTEMRJ1wdOxR84rfmE/51XCa4ug3fqGiAIFXsEka2XPNW1J3JV 14wkO+pWGtMb1Ts6YXrNBWFKv8LaodkX1OrAm76UZAVBDpvz0ygCBJk1xzFFfTZrKzQA38Gp1hUkf SXqfgH4dn3KRbm/5aTOg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pZpjX-0045yM-Km; Wed, 08 Mar 2023 09:05:15 +0000 Received: from mail-wm1-x332.google.com ([2a00:1450:4864:20::332]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pZpjP-0045uP-Fl for linux-arm-kernel@lists.infradead.org; Wed, 08 Mar 2023 09:05:10 +0000 Received: by mail-wm1-x332.google.com with SMTP id t25-20020a1c7719000000b003eb052cc5ccso730458wmi.4 for ; Wed, 08 Mar 2023 01:05:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1678266299; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=kA8QaXHaQv2zTvTrF1F5tUB/9uaPE/un/rBzeew06Qk=; b=q89OEdbdQ8/5wEe6qFgIWzh3vd6s4xaRgf89QS/3X4cOBtJctnoIMQxxEkA1D1BiHI 0rPMGxYv1A8HM3xCu5rwnUu1Ej1pGr2yhOovW1vpKhfjmrhnTXnSrnkTPEb2jQaLNjCu gjPvDpbonBmkmrER6l1D5vxT9FfWi6H61jHjcQqvRP49uhXANQfBL/nTJmVo9mW1K9+n yBrlg1+msaFsX+hZi1yQGq1ALarkY7lxAgSKSy51rXIuUTDYzdRIQ4Td+1aCJOW/HPYu E7T1Lo6gbBZA2NMhBQNGnjngG9/h7JB3LioUT5eLeFK7zU7ue/RKGy/ITsQAn5o7NDuO SNdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678266299; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kA8QaXHaQv2zTvTrF1F5tUB/9uaPE/un/rBzeew06Qk=; b=2tC7rv3roiXAYfH0EOiPFCXDFPxthtOYTAvExnPvQpeLxKpOUA90GSaQC5whiDIhMi mt3+Eh/9Kgi1CyPZ1XhhDoeboJWCH9DT3JycUyry5HsRg1FX3HbVd8rlF4VQ4iP/wCrL BYW6V7Gd1zlG2ON9wFMCOTdiy82xWxMAAelIdH921kkkb/9ZnIwo1JOFm7WnfzzbuMx6 Gqi0ftCxzKYTmZkf2YA2/IEvHM/VeYPO8VzqmtAETraDC9o4pI+ELHVO1EJP3y/jBNgp eBCDo1XAoaMNgQO/UeqHfGiJwDulo9OUaLYdu1LlfndbCG/9bSGe3q/QAhQRy6RnMynx t31w== X-Gm-Message-State: AO0yUKURS/SLBQmH3P2MOgQ4sOcLJtGsO8S5o2p1iH9pu1HmRWZXcgPu k7ownhhQ+r0ItmRVkebFt7Umdg== X-Google-Smtp-Source: AK7set/L9zFk+/3muQ4XCE6qXO8h4slPi9HcknR81cfBA77IySEK46FikrIsHFywPlVNMXfACef86g== X-Received: by 2002:a05:600c:1907:b0:3eb:29fe:7b9e with SMTP id j7-20020a05600c190700b003eb29fe7b9emr14295268wmq.17.1678266299463; Wed, 08 Mar 2023 01:04:59 -0800 (PST) Received: from [192.168.2.107] ([79.115.63.78]) by smtp.gmail.com with ESMTPSA id v38-20020a05600c4da600b003eb68bb61c8sm14645299wmp.3.2023.03.08.01.04.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 08 Mar 2023 01:04:58 -0800 (PST) Message-ID: <1766f6ef-d9d8-04f7-a6bf-0ea6bc0b3d23@linaro.org> Date: Wed, 8 Mar 2023 09:04:55 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] spi: Replace `dummy.nbytes` with `dummy.ncycles` To: Serge Semin , Sergiu.Moga@microchip.com, Mark Brown , Tudor Ambarus , Pratyush Yadav , Michael Walle References: <20220911174551.653599-1-sergiu.moga@microchip.com> <20220925220304.buk3yuqoh6vszfci@mobilestation> <18e6e8a8-6412-7e31-21e0-6becd4400ac1@microchip.com> <20220926172454.kbpzck7med5bopre@mobilestation> Content-Language: en-US From: Tudor Ambarus In-Reply-To: <20220926172454.kbpzck7med5bopre@mobilestation> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230308_010507_584572_C417A8FC X-CRM114-Status: GOOD ( 28.10 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alexandre.belloni@bootlin.com, vigneshr@ti.com, linux-aspeed@lists.ozlabs.org, alexandre.torgue@foss.st.com, tali.perry1@gmail.com, linux-mtd@lists.infradead.org, miquel.raynal@bootlin.com, linux-spi@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, tmaimon77@gmail.com, benjaminfair@google.com, kdasu.kdev@gmail.com, richard@nod.at, chin-ting_kuo@aspeedtech.com, michal.simek@xilinx.com, haibo.chen@nxp.com, openbmc@lists.ozlabs.org, yuenn@google.com, bcm-kernel-feedback-list@broadcom.com, joel@jms.id.au, linux-rockchip@lists.infradead.org, avifishman70@gmail.com, john.garry@huawei.com, linux-mediatek@lists.infradead.org, clg@kaod.org, matthias.bgg@gmail.com, han.xu@nxp.com, linux-arm-kernel@lists.infradead.org, andrew@aj.id.au, venture@google.com, heiko@sntech.de, linux-kernel@vger.kernel.org, yogeshgaur.83@gmail.com, mcoquelin.stm32@gmail.com, Claudiu.Beznea@microchip.com 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 Hi, Sarge, On 9/26/22 18:24, Serge Semin wrote: >>>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c >>>> index f2c64006f8d7..cc8ca824f912 100644 >>>> --- a/drivers/mtd/spi-nor/core.c >>>> +++ b/drivers/mtd/spi-nor/core.c >>>> @@ -88,7 +88,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor, >>>> if (op->addr.nbytes) >>>> op->addr.buswidth = spi_nor_get_protocol_addr_nbits(proto); >>>> >>> >>> >>>> - if (op->dummy.nbytes) >>>> + if (op->dummy.ncycles) >>>> op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto); >>>> >>>> if (op->data.nbytes) >>>> @@ -106,9 +106,6 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor, >>>> op->dummy.dtr = true; >>>> op->data.dtr = true; >>>> >>>> - /* 2 bytes per clock cycle in DTR mode. */ >>>> - op->dummy.nbytes *= 2; >>>> - >>>> ext = spi_nor_get_cmd_ext(nor, op); >>>> op->cmd.opcode = (op->cmd.opcode << 8) | ext; >>>> op->cmd.nbytes = 2; >>>> @@ -207,10 +204,7 @@ static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from, >>>> >>>> spi_nor_spimem_setup_op(nor, &op, nor->read_proto); >>>> >>>> - /* convert the dummy cycles to the number of bytes */ >>>> - op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8; >>>> - if (spi_nor_protocol_is_dtr(nor->read_proto)) >>>> - op.dummy.nbytes *= 2; >>>> + op.dummy.ncycles = nor->read_dummy; >>> So according to this modification and what is done in the rest of the >>> patch, the dummy part of the SPI-mem operations now contains the number >>> of cycles only. Am I right to think that it means a number of dummy >>> clock oscillations? (Judging from what I've seen in the HW-manuals of >>> the SPI NOR memory devices most likely I am...) >> >> >> Yes, you are correct. >> I confirm. >> >>> If so the "ncycles" field >>> is now free from the "data" semantic. Then what is the meaning of the >>> "buswidth and "dtr" fields in the spi_mem_op.dummy field? >>> >> >> It is still meaningful as it is used for the conversion by some drivers >> to nbytes and I do not see how it goes out of the specification in any >> way. So, at least for now, I do not see any reason to remove these fields. > I do see the way these fields are used in the SPI-mem drivers. I was > wondering what do these bits mean in the framework of the SPI-mem > core? AFAICS from the specification the dummy cycles are irrelevant to > the data bus state. It says "the master tri-states the bus during > 'dummy' cycles." If so I don't see a reason to have the DTR and > buswidth fields in the spi_mem_op structure anymore. The number of > cycles could be calculated right on the initialization stage based on > the SPI NOR/NAND requirements. > > @Mark, @Tudor, @Pratyush, what do you think? > In an ideal world, where both the controller and the device talk about dummy number of cycles, I would agree with you, buswidth and dtr should not be relevant for the number of dummy cycles. But it seems that there are old controllers (e.g. spi-hisi-sfc-v3xx.c, spi-mt65xx.c, spi-mxic.c) that support buswidths > 1 and work only with dummy nbytes, they are not capable of specifying a smaller granularity (ncycles). Thus the older controllers would have to convert the dummy ncycles to dummy nbytes. Since mixed transfer modes are a thing (see jesd251, it talks about 4S-4D-4D), where single transfer mode (S) can be mixed with double transfer mode (D) for a command, the controller would have to guess the buswidth and dtr of the dummy. Shall they replicate the buswidth and dtr of the address or of the data? There's no rule for that. So we're forced to keep the dummy.{dtr, buswidth} fields by the old controllers that don't understand dummy ncycles. I'm going to send a v2 of this patch, I'll add you in To. Thanks for taking the time to review this patch! Cheers, ta _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel