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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 20BAAC6FD1D for ; Mon, 27 Mar 2023 16:26:03 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2C70485CCA; Mon, 27 Mar 2023 18:25:43 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=amazon.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=amazon.de header.i=@amazon.de header.b="dzwvxuMv"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CF2A285C42; Mon, 27 Mar 2023 17:51:02 +0200 (CEST) Received: from smtp-fw-80007.amazon.com (smtp-fw-80007.amazon.com [99.78.197.218]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id CF4A685BD7 for ; Mon, 27 Mar 2023 17:50:57 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=amazon.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=prvs=443c9913a=ptyadav@amazon.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.de; i=@amazon.de; q=dns/txt; s=amazon201209; t=1679932259; x=1711468259; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=C0NMA1gsJSTHxz49yeXG/XcXWbdoNPmLc2RValdSeS4=; b=dzwvxuMvikFEgsv7bIaDp94e5vl7j4kGPD+b7xtJX2rHI/GViVwK+fu1 z3VsdBBd6htEHqZSpfufc9NzjR4+JQ3eiTxJTQKl9R34uMiQIV+vZUydt iFEsWiQADR9D7aY+W9iJS8/HFmyBeNLQ/ebKxxuvuByBVJDL9r/KQ81Xt o=; X-IronPort-AV: E=Sophos;i="5.98,295,1673913600"; d="scan'208";a="197950365" Received: from pdx4-co-svc-p1-lb2-vlan3.amazon.com (HELO email-inbound-relay-pdx-2b-m6i4x-0ec33b60.us-west-2.amazon.com) ([10.25.36.214]) by smtp-border-fw-80007.pdx80.corp.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Mar 2023 15:50:54 +0000 Received: from EX19MTAUWB002.ant.amazon.com (pdx1-ws-svc-p6-lb9-vlan2.pdx.amazon.com [10.236.137.194]) by email-inbound-relay-pdx-2b-m6i4x-0ec33b60.us-west-2.amazon.com (Postfix) with ESMTPS id D1721A0B84; Mon, 27 Mar 2023 15:50:52 +0000 (UTC) Received: from EX19MTAUWC001.ant.amazon.com (10.250.64.145) by EX19MTAUWB002.ant.amazon.com (10.250.64.231) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.25; Mon, 27 Mar 2023 15:50:44 +0000 Received: from dev-dsk-ptyadav-1c-37607b33.eu-west-1.amazon.com (10.15.11.255) by mail-relay.amazon.com (10.250.64.145) with Microsoft SMTP Server id 15.2.1118.25 via Frontend Transport; Mon, 27 Mar 2023 15:50:44 +0000 Received: by dev-dsk-ptyadav-1c-37607b33.eu-west-1.amazon.com (Postfix, from userid 23027615) id C558420E17; Mon, 27 Mar 2023 17:50:42 +0200 (CEST) From: Pratyush Yadav To: Dhruva Gole CC: Jagan Teki , Vignesh , Vaishnav Achath , , "Apurva Nandan" Subject: Re: [PATCH 2/2] spi: cadence-quadspi: Use STIG mode for all ops with small payload References: <20230323164408.1043725-1-d-gole@ti.com> <20230323164408.1043725-3-d-gole@ti.com> Date: Mon, 27 Mar 2023 17:50:42 +0200 In-Reply-To: <20230323164408.1043725-3-d-gole@ti.com> (Dhruva Gole's message of "Thu, 23 Mar 2023 22:14:08 +0530") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Mailman-Approved-At: Mon, 27 Mar 2023 18:25:32 +0200 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean On Thu, Mar 23 2023, Dhruva Gole wrote: > OSPI controller supports all types of op variants in STIG mode, > only limitation being that the data payload should be less than > 8 bytes when not using memory banks. > > STIG mode is more stable for operations that send small data > payload and is more efficient than using DMA for few bytes of > memory accesses. It overcomes the limitation of minimum 4 bytes > read from flash into RAM seen in DAC mode. > > Use STIG mode for all read and write operations that require > data input/output of less than 8 bytes from the flash, and thereby > support all four phases, cmd/address/dummy/data, through OSPI STIG. > > Signed-off-by: Apurva Nandan > Signed-off-by: Dhruva Gole > --- > drivers/spi/cadence_qspi.c | 5 ++-- > drivers/spi/cadence_qspi_apb.c | 44 ++++++++++++++++++---------------- > 2 files changed, 26 insertions(+), 23 deletions(-) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index a858a62888e4..f931e4cf3e2f 100644 > --- a/drivers/spi/cadence_qspi.c > +++ b/drivers/spi/cadence_qspi.c > @@ -312,13 +312,12 @@ static int cadence_spi_mem_exec_op(struct spi_slave *spi, > * which is unsupported on some flash devices during register > * reads, prefer STIG mode for such small reads. > */ > - if (!op->addr.nbytes || What if someone sends us a command that has no address phase but reads more than CQSPI_STIG_DATA_LEN_MAX bytes? You would happily send it to cadence_qspi_apb_read_setup(), which would then do reg |= (op->addr.nbytes - 1) causing an underflow and having corrputing the register. Since there is no way to execute a command with no address phase and data bytes > CQSPI_STIG_DATA_LEN_MAX, you should add a check for this in the supports_op() hook. > - op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX) > + if (op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX) > mode = CQSPI_STIG_READ; > else > mode = CQSPI_READ; > } else { > - if (!op->addr.nbytes || !op->data.buf.out) > + if (op->data.nbytes <= CQSPI_STIG_DATA_LEN_MAX) Same here. > mode = CQSPI_STIG_WRITE; > else > mode = CQSPI_WRITE; > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c > index 2b04b58124a5..25b5fc292e07 100644 > --- a/drivers/spi/cadence_qspi_apb.c > +++ b/drivers/spi/cadence_qspi_apb.c > @@ -374,6 +374,8 @@ int cadence_qspi_apb_exec_flash_cmd(void *reg_base, unsigned int reg) > if (!cadence_qspi_wait_idle(reg_base)) > return -EIO; > > + /* Flush the CMDCTRL reg after the execution */ > + writel(0, reg_base + CQSPI_REG_CMDCTRL); Do this in a separate patch with its own explanation please. > return 0; > } > > @@ -460,11 +462,6 @@ int cadence_qspi_apb_command_read(struct cadence_spi_priv *priv, > unsigned int dummy_clk; > u8 opcode; > > - if (rxlen > CQSPI_STIG_DATA_LEN_MAX || !rxbuf) { > - printf("QSPI: Invalid input arguments rxlen %u\n", rxlen); > - return -EINVAL; > - } > - Ok. > if (priv->dtr) > opcode = op->cmd.opcode >> 8; > else > @@ -547,26 +544,12 @@ int cadence_qspi_apb_command_write(struct cadence_spi_priv *priv, > unsigned int reg = 0; > unsigned int wr_data; > unsigned int wr_len; > + unsigned int dummy_clk; > unsigned int txlen = op->data.nbytes; > const void *txbuf = op->data.buf.out; > void *reg_base = priv->regbase; > - u32 addr; > u8 opcode; > > - /* Reorder address to SPI bus order if only transferring address */ > - if (!txlen) { > - addr = cpu_to_be32(op->addr.val); > - if (op->addr.nbytes == 3) > - addr >>= 8; > - txbuf = &addr; > - txlen = op->addr.nbytes; > - } You should explain why you are removing this. > - > - if (txlen > CQSPI_STIG_DATA_LEN_MAX) { > - printf("QSPI: Invalid input arguments txlen %u\n", txlen); > - return -EINVAL; > - } > - > if (priv->dtr) > opcode = op->cmd.opcode >> 8; > else > @@ -574,6 +557,27 @@ int cadence_qspi_apb_command_write(struct cadence_spi_priv *priv, > > reg |= opcode << CQSPI_REG_CMDCTRL_OPCODE_LSB; > > + /* setup ADDR BIT field */ > + if (op->addr.nbytes) { > + writel(op->addr.val, priv->regbase + CQSPI_REG_CMDADDRESS); > + /* > + * address bytes are zero indexed > + */ > + reg |= (((op->addr.nbytes - 1) & > + CQSPI_REG_CMDCTRL_ADD_BYTES_MASK) << > + CQSPI_REG_CMDCTRL_ADD_BYTES_LSB); > + reg |= (0x1 << CQSPI_REG_CMDCTRL_ADDR_EN_LSB); > + } > + > + /* Set up dummy cycles. */ > + dummy_clk = cadence_qspi_calc_dummy(op, priv->dtr); > + if (dummy_clk > CQSPI_DUMMY_CLKS_MAX) > + return -EOPNOTSUPP; > + > + if (dummy_clk) > + reg |= (dummy_clk & CQSPI_REG_CMDCTRL_DUMMY_MASK) > + << CQSPI_REG_CMDCTRL_DUMMY_LSB; > + Ok. > if (txlen) { > /* writing data = yes */ > reg |= (0x1 << CQSPI_REG_CMDCTRL_WR_EN_LSB); -- Regards, Pratyush Yadav Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879