From patchwork Fri Aug 2 07:50:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Serge Semin X-Patchwork-Id: 816477 Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 274A01C0DC2; Fri, 2 Aug 2024 07:51:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722585087; cv=none; b=kuR5xkYq6z7v5nmuoxT8BHa/UqYVurdWWmvHW4dQtD69n1mH7d5PNBOdMyAfxLhXnHqH8o0jvwyxt9s2E/6CgGTqx06rzRfLnU2lx2UtQ4s7J6ppkG2bT9hJwdCUTSEK+DhSgm9azu37zTJGgM/omlroCBpx1Bn8aABkG8LaU5o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722585087; c=relaxed/simple; bh=su5EHwRLySV2iEk+gTBhJK/g0SjIhgNjvQOoXYOe8Yo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=T40vvVOJfBkBdBCcOB3MhbiuOSRNAjvNJ+jJSBnLV0cHbooo6AVfopM1ylIITEFm7mBtVSfztj73iKSKHf0ksPUbZ1v2LwKzB0CvwBn8zLEIx2qEUE21ZdZ+sDGvvuUTywE70IVQwbBouRkcASfdnqAT0PWNu9llxIQHc8QHWHM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=AouBUM+7; arc=none smtp.client-ip=209.85.208.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="AouBUM+7" Received: by mail-lj1-f172.google.com with SMTP id 38308e7fff4ca-2ef1c12ae23so89411721fa.0; Fri, 02 Aug 2024 00:51:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1722585083; x=1723189883; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=YnL3k25TEcmvFnfbei6l1RtxotXrQ2yPeSs0Dsdedfs=; b=AouBUM+7AKqsk+YvHkitU9PzfsfEWCfajj8rozb1GZ+IRqoq1+XQ9TU1VKophA5j7U yV9uUsPszS/LLGKgo0zFhFvy66Ur7zZnuKzKuZjDSBo8YuQhdbjEeybMen5grPw38uyf qafOkIpe2dUSwZyXh+6lcDlwZIv1giyXO+opUAApVLjOeCmBzxQBx2jUS7srfqLhbkeB ovhepCKtOawtNVvECDDRnCyg+Z+dpXvMCqfg8/eArqOQT14aKbwVm5DV1+wNUmAJSTAE zpbUQTAGiIFopPq3f96E7fBFqiqf5oFl3UE+XoYrOrvvS6wteT+svHcGIo2F7IOZKz+H aOeQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722585083; x=1723189883; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YnL3k25TEcmvFnfbei6l1RtxotXrQ2yPeSs0Dsdedfs=; b=RGAlo+MkqQBmQaJjeRQiN3H8V7SISkB50PWrwYe0h/t3EyRRbWvzoSUtHhuilv5EpK BnFlqEaBeyk47ngu/oEevuKqbDnwB6L67JhZYALVJ68E2fGwvYXy7dim5lGuygHgdZA8 CoMae1VvZo08YBPXx7v3wYstdSOQS4Ujqs5wi4Y3eUbkUjvEdvRMML/uqdelJncdJ3Ql Ov2kLtfZfKzOnxK3P1GvcJ8WwpX8j+/DtigMOs41zrK821SzVGnVpPJF8X+eESwyulAY vCTw+wn1aB189W5ffGdMNbFZwlLpm3kJdd+MKyAkJyeu237pb7MVQgJaUKxDTwwW9K8r QOzg== X-Forwarded-Encrypted: i=1; AJvYcCXmji63v9RMarKr9YoWQZZCHpHimXFUzMiFK7B3d8xTAOQNkVCHfK3mJN8E/TXg/YMV2/kpzwxqH4FarSTwumRKK4M/zrQ2Mp0maKIwbGWHVWYA3f1JYMSK5LGulfQx9EKKXTpdixqV75ksdJ9wDtcms94LIz0/2+CWNxZ4HiAZYp3HE9CA X-Gm-Message-State: AOJu0YyhkqDUouodLTwB3rzJsvAyAUdPMGyh1DkFKEA8sMc12aXq+Ine WKHuzFM8LC9za1EjvGJ/kLwETpTC+/TwwaLEu5advznLoQu7gyD0JrT28fLN X-Google-Smtp-Source: AGHT+IHKjvP7PFAmbaWrO8exrT6bgGgVnNhGXJcu31lAQ8H/yBK6eGvQpk3cUaNsC1DoGIVFeYytfA== X-Received: by 2002:a2e:7006:0:b0:2ef:2016:262e with SMTP id 38308e7fff4ca-2f15a9f028fmr19547581fa.0.1722585083056; Fri, 02 Aug 2024 00:51:23 -0700 (PDT) Received: from localhost ([178.176.56.174]) by smtp.gmail.com with ESMTPSA id 38308e7fff4ca-2f15e17ee5bsm1029281fa.11.2024.08.02.00.51.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 02 Aug 2024 00:51:22 -0700 (PDT) From: Serge Semin To: Viresh Kumar , Andy Shevchenko , Andy Shevchenko , Vinod Koul Cc: Serge Semin , =?utf-8?q?Ilpo_J=C3=A4rvinen?= , Greg Kroah-Hartman , Jiri Slaby , dmaengine@vger.kernel.org, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, Viresh Kumar Subject: [PATCH RESEND v4 2/6] dmaengine: dw: Add memory bus width verification Date: Fri, 2 Aug 2024 10:50:47 +0300 Message-ID: <20240802075100.6475-3-fancer.lancer@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240802075100.6475-1-fancer.lancer@gmail.com> References: <20240802075100.6475-1-fancer.lancer@gmail.com> Precedence: bulk X-Mailing-List: linux-serial@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Currently in case of the DEV_TO_MEM or MEM_TO_DEV DMA transfers the memory data width (single transfer width) is determined based on the buffer length, buffer base address or DMA master-channel max address width capability. It isn't enough in case of the channel disabling prior the block transfer is finished. Here is what DW AHB DMA IP-core databook says regarding the port suspension (DMA-transfer pause) implementation in the controller: "When CTLx.SRC_TR_WIDTH < CTLx.DST_TR_WIDTH and the CFGx.CH_SUSP bit is high, the CFGx.FIFO_EMPTY is asserted once the contents of the FIFO do not permit a single word of CTLx.DST_TR_WIDTH to be formed. However, there may still be data in the channel FIFO, but not enough to form a single transfer of CTLx.DST_TR_WIDTH. In this scenario, once the channel is disabled, the remaining data in the channel FIFO is not transferred to the destination peripheral." So in case if the port gets to be suspended and then disabled it's possible to have the data silently discarded even though the controller reported that FIFO is empty and the CTLx.BLOCK_TS indicated the dropped data already received from the source device. This looks as if the data somehow got lost on a way from the peripheral device to memory and causes problems for instance in the DW APB UART driver, which pauses and disables the DMA-transfer as soon as the recv data timeout happens. Here is the way it looks: Memory <------- DMA FIFO <------ UART FIFO <---------------- UART DST_TR_WIDTH -+--------| | | | | | | No more data Current lvl -+--------| |---------+- DMA-burst lvl | | |---------+- Leftover data | | |---------+- SRC_TR_WIDTH -+--------+-------+---------+ In the example above: no more data is getting received over the UART port and BLOCK_TS is not even close to be fully received; some data is left in the UART FIFO, but not enough to perform a bursted DMA-xfer to the DMA FIFO; some data is left in the DMA FIFO, but not enough to be passed further to the system memory in a single transfer. In this situation the 8250 UART driver catches the recv timeout interrupt, pauses the DMA-transfer and terminates it completely, after which the IRQ handler manually fetches the leftover data from the UART FIFO into the recv-buffer. But since the DMA-channel has been disabled with the data left in the DMA FIFO, that data will be just discarded and the recv-buffer will have a gap of the "current lvl" size in the recv-buffer at the tail of the lately received data portion. So the data will be lost just due to the misconfigured DMA transfer. Note this is only relevant for the case of the transfer suspension and _disabling_. No problem will happen if the transfer will be re-enabled afterwards or the block transfer is fully completed. In the later case the "FIFO flush mode" will be executed at the transfer final stage in order to push out the data left in the DMA FIFO. In order to fix the denoted problem the DW AHB DMA-engine driver needs to make sure that the _bursted_ source transfer width is greater or equal to the single destination transfer (note the HW databook describes more strict constraint than actually required). Since the peripheral-device side is prescribed by the client driver logic, the memory-side can be only used for that. The solution can be easily implemented for the DEV_TO_MEM transfers just by adjusting the memory-channel address width. Sadly it's not that easy for the MEM_TO_DEV transfers since the mem-to-dma burst size is normally dynamically determined by the controller. So the only thing that can be done is to make sure that memory-side address width is greater than the peripheral device address width. Fixes: a09820043c9e ("dw_dmac: autoconfigure data_width or get it via platform data") Signed-off-by: Serge Semin --- Changelog v2: - Add a in-situ comment regarding why the memory-side bus width verification was required. (Andy) - Convert "err" to "ret" variable. (Andy) --- drivers/dma/dw/core.c | 51 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c index d4c694b0f55a..4987bfa10461 100644 --- a/drivers/dma/dw/core.c +++ b/drivers/dma/dw/core.c @@ -622,12 +622,10 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, struct dw_desc *prev; struct dw_desc *first; u32 ctllo, ctlhi; - u8 m_master = dwc->dws.m_master; - u8 lms = DWC_LLP_LMS(m_master); + u8 lms = DWC_LLP_LMS(dwc->dws.m_master); dma_addr_t reg; unsigned int reg_width; unsigned int mem_width; - unsigned int data_width = dw->pdata->data_width[m_master]; unsigned int i; struct scatterlist *sg; size_t total_len = 0; @@ -661,7 +659,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, mem = sg_dma_address(sg); len = sg_dma_len(sg); - mem_width = __ffs(data_width | mem | len); + mem_width = __ffs(sconfig->src_addr_width | mem | len); slave_sg_todev_fill_desc: desc = dwc_desc_get(dwc); @@ -721,7 +719,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl, lli_write(desc, sar, reg); lli_write(desc, dar, mem); lli_write(desc, ctlhi, ctlhi); - mem_width = __ffs(data_width | mem); + mem_width = __ffs(sconfig->dst_addr_width | mem); lli_write(desc, ctllo, ctllo | DWC_CTLL_DST_WIDTH(mem_width)); desc->len = dlen; @@ -813,6 +811,41 @@ static int dwc_verify_p_buswidth(struct dma_chan *chan) return 0; } +static int dwc_verify_m_buswidth(struct dma_chan *chan) +{ + struct dw_dma_chan *dwc = to_dw_dma_chan(chan); + struct dw_dma *dw = to_dw_dma(chan->device); + u32 reg_width, reg_burst, mem_width; + + mem_width = dw->pdata->data_width[dwc->dws.m_master]; + + /* + * It's possible to have a data portion locked in the DMA FIFO in case + * of the channel suspension. Subsequent channel disabling will cause + * that data silent loss. In order to prevent that maintain the src and + * dst transfer widths coherency by means of the relation: + * (CTLx.SRC_TR_WIDTH * CTLx.SRC_MSIZE >= CTLx.DST_TR_WIDTH) + * Look for the details in the commit message that brings this change. + * + * Note the DMA configs utilized in the calculations below must have + * been verified to have correct values by this method call. + */ + if (dwc->dma_sconfig.direction == DMA_MEM_TO_DEV) { + reg_width = dwc->dma_sconfig.dst_addr_width; + if (mem_width < reg_width) + return -EINVAL; + + dwc->dma_sconfig.src_addr_width = mem_width; + } else if (dwc->dma_sconfig.direction == DMA_DEV_TO_MEM) { + reg_width = dwc->dma_sconfig.src_addr_width; + reg_burst = rounddown_pow_of_two(dwc->dma_sconfig.src_maxburst); + + dwc->dma_sconfig.dst_addr_width = min(mem_width, reg_width * reg_burst); + } + + return 0; +} + static int dwc_config(struct dma_chan *chan, struct dma_slave_config *sconfig) { struct dw_dma_chan *dwc = to_dw_dma_chan(chan); @@ -822,14 +855,18 @@ static int dwc_config(struct dma_chan *chan, struct dma_slave_config *sconfig) memcpy(&dwc->dma_sconfig, sconfig, sizeof(*sconfig)); dwc->dma_sconfig.src_maxburst = - clamp(dwc->dma_sconfig.src_maxburst, 0U, dwc->max_burst); + clamp(dwc->dma_sconfig.src_maxburst, 1U, dwc->max_burst); dwc->dma_sconfig.dst_maxburst = - clamp(dwc->dma_sconfig.dst_maxburst, 0U, dwc->max_burst); + clamp(dwc->dma_sconfig.dst_maxburst, 1U, dwc->max_burst); ret = dwc_verify_p_buswidth(chan); if (ret) return ret; + ret = dwc_verify_m_buswidth(chan); + if (ret) + return ret; + dw->encode_maxburst(dwc, &dwc->dma_sconfig.src_maxburst); dw->encode_maxburst(dwc, &dwc->dma_sconfig.dst_maxburst);