From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (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 B286730FC0D for ; Tue, 2 Dec 2025 12:01:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764676920; cv=none; b=i9mFJMq8XL2UwcC92HFOFCt2dsQET4tBPEGWdWt1bfibxQ/4I7XjOJbL7g/ergeivkNlmdS22XIGi9O8gFtVGtubidayLdqg48W+9/q+MzoDaqrNOkahwvC9WmIbXNuCTNqGC92emcazt5i8zz7FC5XRKmflaRheZhatUEBjIrs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764676920; c=relaxed/simple; bh=7K9mUWAwEL2WcGDkUZv5F7uNiXkKQLBXx4jkyHnbFTU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Mkqxq5kPecIPys0jfgKMrNtZaiNZdEptvvQ85aAH9mR53YKK4oYpPdeOqgg7pOi7vNSjBSJr8TZL2rARck5xu/uHFHaQhnzIzcaiKf85t2VWKHLR1BldHS9QqxBU8c8fy+YkYLJw0bnbT1wmR418RkWzpLKjijrgPeqJH2+3tqk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=LhBJergK; arc=none smtp.client-ip=209.85.128.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="LhBJergK" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-47778b23f64so31360635e9.0 for ; Tue, 02 Dec 2025 04:01:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1764676917; x=1765281717; darn=vger.kernel.org; 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=6id7bBwtKWfp+iW0Yj4nXKFNWSuqfCTlCcCWbRL2xcE=; b=LhBJergKvcTFGD5CGfe3XYjLOhMeS8m4Xikbvo85m+apzO7e0yeGlqXFhVLgP2R8HN ebt0dhH240lIhOojdfHzpvrDNVHMsM5hubvdPFikxlYPpz5wl2uvWq+4oZMkEpWC+ccx 8M+ZguSkgSOE1qPFhd17W1o6Ciag5gInJKL2HhcZJ3cLrWAN5TU44PS8hj+TD+Ut4BTY uB6jqkLUIXJHmmKAo86qioVBY2u5PXk/i8dmy+E9mFhS/rbegPM5wJUIoocheDMVRh8s 8+2DCwnQ/225GktFmAc1jRlOkJFWekJH+l++PKetd1DkEj9prhw2CiM8JCNITvUT5lMj NC+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764676917; x=1765281717; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=6id7bBwtKWfp+iW0Yj4nXKFNWSuqfCTlCcCWbRL2xcE=; b=g2Vx+zyuyA2GGKQ1lHqqFRTRyt1wFyRqhRETbmhkfwSxKZaf5pGQ/+tBHDuiNhu93f 6+u8wEpHMZVPnG7aOZVdrEEJkGbts2Xfi/sat57tG68lwkSH07fSqjewfmhiEixFe9Sm 3RhvjXPBGYjCUxfFhIS++FO4f9x6nRWZdJQ7r9GflaHonayaBu/coDB4t1zf8eCaJO1w GdyPDFV6270f4ZhIwzBZxbkbpqJ1oeYML18v8lKHPJ/VTTuDgRt8dFz4piKuue0PSrIv QGQ9xcONQ8LHuhFbdLOI7jHLn3xa6auvPspQ7gpODF2kFSo+INehtTlF+EdlnY66pTW2 oDYQ== X-Forwarded-Encrypted: i=1; AJvYcCXIXFM9Z+drz04OD5ssz6VFTcWzGsbDsbDng4cF3sVr5kHyrTPsr0zueuzcERmTpSJ6qdZ1R+eKAqVIqpA=@vger.kernel.org X-Gm-Message-State: AOJu0YykqUw0qz2l/V76eLqPCEKXMbOjFtCqg4USUFv3KZPwNfvVjNEj 06UhotC/rfgBuQap6SPA52UeQARFziuck82Jem612hHLWuLlwFd7VCB2tzA0fkFEh0f5MA+6JTX U0la7Ur4P5g== X-Gm-Gg: ASbGncuPzxCMGzIGGZaqZ3xP5lOHd2OHFuNatwFo6/02UgglmTSJs5PUK4M+oVos78J HPuS93pZAh3zir+ot63SpwpJJh5bfw0j6rXhB+Ul9f5NAx9iOyGigcwHFlKl8RPb8Q3UfBW//ui nYTlZwTPBZXc10BoJOTxIxnBf6WslhNNaGfGS0BWSnCi9WJWfunSN4/kjfpksLBbOEo//ho8BZ1 EGtysOu4lKK4G4ySBOe8+qb/C2AtxBYAPIXlLjnLUdA74QjPGabpmXw9fabGU6ExZvnpUFxM/0I 6TiedqQhDmqphbihSOCFRs+oYz8RCWfcXDebE6VoDQwO9a6agiIwiyJA3ZQEgCc1ylW52Vd0WXu wB8j2o83bNulWyTA0TIKqQQ61yyKMtJ4phJ4QkHpZXE5RpR3FsPOZBiCkMq7loQFj9dXRq4SE8L SuOeRGZiOUL5+X95r/2oSE1aQXDvk= X-Google-Smtp-Source: AGHT+IEh/NLlOuvtojaeo8HI9XO7bfRFqhGXmwfYsgYpNIBUYckNTYbpdBcFb7hIJh2Hs0BfENzYzw== X-Received: by 2002:a05:600c:1c25:b0:46e:4586:57e4 with SMTP id 5b1f17b1804b1-477c114ed70mr578484785e9.24.1764676916831; Tue, 02 Dec 2025 04:01:56 -0800 (PST) Received: from [192.168.1.221] ([5.31.29.35]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4791165b1fesm294627815e9.15.2025.12.02.04.01.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 02 Dec 2025 04:01:55 -0800 (PST) Message-ID: Date: Tue, 2 Dec 2025 14:01:52 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] shdmac: Remove misleading TODO comment in dmae_set_chcr To: Amin Cc: vkoul@kernel.org, thomasandreatta2000@gmail.com, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org References: <20251128152947.304976-2-amin.gattout@gmail.com> Content-Language: en-US From: Eugen Hristev In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 12/2/25 13:54, Amin wrote: > Thanks Eugen, > > I could not reproduce any concrete issue, and after re-reading the > flow I think your assessment is correct: > If the channel is busy, CHCR is indeed never written, so in that sense > the TODO comment was accurate. > > Given this, removing the busy check seems more consistent than > removing the comment... > My patch was therefore not addressing the real problem, sorry for the noise. > > If you agree, I can prepare a follow-up patch that removes > the`dmae_is_busy()` check, or leave things as they are if that is > preferred. I am not sure whether any of the solutions is right. The commit that introduced the comment is pretty old (2011) so it would be difficult to understand the reasoning. Maybe someone else who is more involved in working with this driver could give more insight. Eugen > > Amin > > > Le mar. 2 déc. 2025 à 11:12, Eugen Hristev a écrit : >> >> >> >> On 11/28/25 17:29, Amin GATTOUT wrote: >>> The comment suggested that the dmae_is_busy() check in dmae_set_chcr() >>> is superfluous and could be removed. However, this check serves as an >>> important safety net to prevent configuration of a DMA channel while >> >> I find this a bit odd overall, because apparently nobody checks the >> result of dmae_set_chcr() . >> So if it is such an important safety check, why is the result never >> checked ? >> As it looks, the caller doesn't care and continue as usual. The >> difference would be that chcr is never actually written if the channel >> is busy. Which looks strange. And "unexpected hardware behavior in edge >> cases" is quite vague. Do you have a scenario when an issue would happen ? >> dmae_set_chcr() gets called on resume() and setup_xfer(). Is it possible >> that in fact dmae_set_chcr() is not called correctly then ? Maybe this >> chcr should be written at a different time when we are sure the dma is >> not busy ? >> Or why is it even possible to have the dma busy when calling it ? >> >> Eugen >> >>> it is active. Keeping it helps ensure transfer integrity and avoids >>> unexpected hardware behavior in edge cases. >>> >>> Signed-off-by: Amin GATTOUT >>> --- >>> drivers/dma/sh/shdmac.c | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/drivers/dma/sh/shdmac.c b/drivers/dma/sh/shdmac.c >>> index 603e15102e45..d0e0437ad916 100644 >>> --- a/drivers/dma/sh/shdmac.c >>> +++ b/drivers/dma/sh/shdmac.c >>> @@ -243,7 +243,6 @@ static void dmae_init(struct sh_dmae_chan *sh_chan) >>> >>> static int dmae_set_chcr(struct sh_dmae_chan *sh_chan, u32 val) >>> { >>> - /* If DMA is active, cannot set CHCR. TODO: remove this superfluous check */ >>> if (dmae_is_busy(sh_chan)) >>> return -EBUSY; >>> >>