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 86754C48BC4 for ; Thu, 15 Feb 2024 13:51:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:MIME-Version: Content-Transfer-Encoding:Content-Type:References:In-Reply-To:Date:Cc:To:From :Subject:Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=5t9mkBRSLuX6Whm+p+USkACVg0A1ZTFUPoQmMKDP/ks=; b=B3PmhCZrRnQVYTal+T4vw0MOfv H/V9SIYfc5Wg9v34S+oiMABY8yQGLwnyquAdAv9bIDbNkqCRt3mGoaj68Y2QatxwE6t1ldrYI3UCl V6bs6I1pJukqAzlc26715IYxNGnoEP5g0n6J7e3JLzlJ4Coe70yB7Bs4E6H+Tq/GcXf6jL5cwfh1Z 4vqTEwPQdiVTuUkdZL7qzpEXE/Iv+ombpyiAF0Vx3XLs4UuhcazFkCMH8fBEz3C9Cu1CE1BClYuMQ ocaQgjPwV7KCPUcDUmiHDvPZnu1mO6yYr/adxkTfMPqKgSuhtO0g4pgvP2zAGAtJsqiiWb3Ws8zOn q/j4H6zA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rac9M-0000000GSQt-0HE7; Thu, 15 Feb 2024 13:51:40 +0000 Received: from mail-wm1-x329.google.com ([2a00:1450:4864:20::329]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rac9H-0000000GSPs-3wlj; Thu, 15 Feb 2024 13:51:37 +0000 Received: by mail-wm1-x329.google.com with SMTP id 5b1f17b1804b1-411a5b8765bso5293765e9.1; Thu, 15 Feb 2024 05:51:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708005094; x=1708609894; darn=lists.infradead.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=5t9mkBRSLuX6Whm+p+USkACVg0A1ZTFUPoQmMKDP/ks=; b=lOS6X2lM7Z0Ji0t0jnlx04RdLs68zFspL9JVSzJuEuO4bD7MWxLYhgGpJdFS/tIu6b 6ShFzqG9pWol2ck1YQRJIHcLWKdWlfbSp+Awm7BL+SYKUtcj02qrODaQdp6z3U1Zkzzu ch6lwCJZlliPvTdG27wUq23bKsqhrUSM0ybEkRHS/CKP5KH6a6LCzXeaSSVIpLGy2gWF c5EBnpY0GbcvyvSuMfLOUdd0F+B50RkBuUdL8JvjezoFjsdCGcrED2icOUptBXQBDlp0 sF/L+y9O0g/3gMoXbi5Deet3wJDqXeMH2gYhyVz5Es+H6epJUJC2cKLc0nzvbR0M7ilY C3YA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708005094; x=1708609894; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=5t9mkBRSLuX6Whm+p+USkACVg0A1ZTFUPoQmMKDP/ks=; b=b/YMijkqXobKrsekW3csN2+WbpFQTscWcUZMIPSpR3a5GieyjkJymsfbY29+pqM48k IFmd2M5S9VWnimV39zeXQpR8NWs3WSWsVD9aqM/5lP8lNK6HC7V2w0EjGY0ylREST2QZ gHQcgms72VNJ1W1rAxVbB967lVLPqNjsr6BhNUVyNYoDyxWxrTYxsHeHEaJYnkEWzzQb 98LdkSgB34CDi6M+G7sJZtjs6lXOZvPtetSXanz208Miaj6MvQDfPKWnsrJp+pbKc2C+ wZR0WahH1fLV/NGcR6RU8QVkwfmPx7wn0jO9Hg5wQzl0KhBwvnhV7n5pws81abJ0X8r0 eWNA== X-Forwarded-Encrypted: i=1; AJvYcCVrpNtI7fnVr1MbtOw7HnhLVGzqXFKomZXhfVkk9fTgd0uhXMxO0wraGHNLeWi92ZaWijQ3YJHmZ39ULpoj0sF/JG1V6l7gU14vH/BygrsS70O+u48WLCaAqVQPQCo35iYvG3W3Ih+hS7cz4jPfpdekwe474UuwGeCaImbznOjXkVFtFfEslm+GQCgxuTSTi971o4u6AW5z5PM+RcSPxYnxACLd7FCa4/sQsX36twlOta43MkIFwOusmLF+dgJrM4H9OmEOH8WdAkJs5zE5yPiiBq9XhKwzeke2D+byEn7wnRBxSPEEgPAOhCqUmra4ZVD7433LNHsU2/aeFWoDBMPsRt+t04bl X-Gm-Message-State: AOJu0Yz0i3E1IASMiY9618s/5brZl7R3MPn/osjklotJHGEwNT7Qd/W8 ZqNMHxNGaZYiFBwTJXjwQ8S61HF7scM3Bo+dCIfMZAnO8aMgEO7v X-Google-Smtp-Source: AGHT+IH7k9jN1LEPjklm49kSSJGmdKPhxpWNQecPQqPdKhvRMqEI/U195h3M4bWUY8DUrcLpEFVYaQ== X-Received: by 2002:a05:600c:3503:b0:412:7d0:d83 with SMTP id h3-20020a05600c350300b0041207d00d83mr1601111wmq.16.1708005093670; Thu, 15 Feb 2024 05:51:33 -0800 (PST) Received: from ?IPv6:2001:a61:3456:4e01:6ae:b55a:bd1d:57fc? ([2001:a61:3456:4e01:6ae:b55a:bd1d:57fc]) by smtp.gmail.com with ESMTPSA id i7-20020a05600c290700b00410add3af79sm5061337wmd.23.2024.02.15.05.51.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Feb 2024 05:51:33 -0800 (PST) Message-ID: <63b248efcbd62a121610cbf37ea0339bd87c99e7.camel@gmail.com> Subject: Re: [PATCH v6 003/164] pwm: Provide pwmchip_alloc() function and a devm variant of it From: Nuno =?ISO-8859-1?Q?S=E1?= To: Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , Andy Shevchenko Cc: Alexandre Belloni , Michael Walle , Heiko Stuebner , linux-doc@vger.kernel.org, Linus Walleij , Paul Walmsley , Alexandre Torgue , Nicolas Ferre , Paul Cercueil , linux-tegra@vger.kernel.org, Conor Dooley , Thierry Reding , James Clark , Pavel Machek , Broadcom internal kernel review list , Guenter Roeck , chrome-platform@lists.linux.dev, Nobuhiro Iwamatsu , Fabio Estevam , linux-riscv@lists.infradead.org, Alyssa Rosenzweig , Jerome Brunet , Rob Herring , Samuel Holland , linux-samsung-soc@vger.kernel.org, Bjorn Andersson , Florian Fainelli , Jonathan Corbet , Sean Anderson , Benson Leung , Bartosz Golaszewski , Lee Jones , Jernej Skrabec , Jonathan Hunter , Hammer Hsieh , linux-rockchip@lists.infradead.org, Chen-Yu Tsai , Michal Simek , NXP Linux Team , linux-leds@vger.kernel.org, Ilpo =?ISO-8859-1?Q?J=E4rvinen?= , Alim Akhtar , linux-mips@vger.kernel.org, linux-sunxi@lists.linux.dev, platform-driver-x86@vger.kernel.org, linux-pwm@vger.kernel.org, Kees Cook , Sven Peter , Martin Blumenstingl , Ray Jui , Sascha Hauer , Jonathan =?ISO-8859-1?Q?Neusch=E4fer?= , Vladimir Zapolskiy , Hans de Goede , Mark Brown , linux-mediatek@lists.infradead.org, linux-rpi-kernel@lists.infradead.org, Baolin Wang , Jonathan Cameron , Matthias Brugger , linux-amlogic@lists.infradead.org, Orson Zhai , Mika Westerberg , kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org, AngeloGioacchino Del Regno , Neil Armstrong , Alexander Shiyan , Scott Branden , linux-gpio@vger.kernel.org, Daire McNamara , Chunyan Zhang , Hector Martin , linux-stm32@st-md-mailman.stormreply.com, Claudiu Beznea , Krzysztof Kozlowski , Fabrice Gasnier , Palmer Dabbelt , asahi@lists.linux.dev, Maxime Coquelin , Kevin Hilman , Shawn Guo , Anjelique Melendez Date: Thu, 15 Feb 2024 14:51:31 +0100 In-Reply-To: References: <9577d6053a5a52536057dc8654ff567181c2da82.1707900770.git.u.kleine-koenig@pengutronix.de> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.50.3 (3.50.3-1.fc39) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240215_055136_004044_845A055B X-CRM114-Status: GOOD ( 31.44 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On Thu, 2024-02-15 at 13:01 +0100, Uwe Kleine-K=C3=B6nig wrote: > On Wed, Feb 14, 2024 at 02:49:26PM +0200, Andy Shevchenko wrote: > > On Wed, Feb 14, 2024 at 10:30:50AM +0100, Uwe Kleine-K=C3=B6nig wrote: > > > This function allocates a struct pwm_chip and driver data. Compared t= o > > > the status quo the split into pwm_chip and driver data is new, otherw= ise > > > it doesn't change anything relevant (yet). > > >=20 > > > The intention is that after all drivers are switched to use this > > > allocation function, its possible to add a struct device to struct > > > pwm_chip to properly track the latter's lifetime without touching all > > > drivers again. Proper lifetime tracking is a necessary precondition t= o > > > introduce character device support for PWMs (that implements atomic > > > setting and doesn't suffer from the sysfs overhead of the /sys/class/= pwm > > > userspace support). > > >=20 > > > The new function pwmchip_priv() (obviously?) only works for chips > > > allocated with pwmchip_alloc(). > >=20 > > ... > >=20 > > > +#define PWMCHIP_ALIGN ARCH_DMA_MINALIGN > > > + > > > +static void *pwmchip_priv(struct pwm_chip *chip) > > > +{ > > > + return (void *)chip + ALIGN(sizeof(*chip), PWMCHIP_ALIGN); > > > +} > >=20 > > Why not use dma_get_cache_alignment() ? >=20 > Hmm, that function returns 1 if ARCH_HAS_DMA_MINALIGN isn't defined. The > idea of using ARCH_DMA_MINALIGN was to ensure that the priv data has the > same minimal alignment as kmalloc(). Took my inspriration from > https://lore.kernel.org/r/20240209-counter-align-fix-v2-1-5777ea0a2722@an= alog.com > . The implementation of dma_get_cache_alignment suggests that not all > archs provide ARCH_DMA_MINALIGN? Also there is ARCH_KMALLOC_MINALIGN. > Hmm, don't know yet what to do here. Here it goes my 2 cents... AFAIK, ARCH_DMA_MINALIGN gives you the same alig= nment guarantees than devm_kmalloc() for instance. In some archs it will effectiv= ely be the same as ARCH_KMALLOC_MINALIGN. Now, I think it only matters if the owners o= f private data intend to have a DMA safe buffer in their structs. If that is the case= , we need to ensure a proper alignment for that structure. In IIO for example, the co= nstruct is like this: https://elixir.bootlin.com/linux/latest/source/drivers/iio/dac/ltc2688.c#L9= 6 The buffers should come last in the struct so they are alone in the line. I= n IIO, Jonathan has a strict policy for this. Like, even if you just want to trans= fer 2/4 bytes via spi, we need to make the buffer safe (apparently there are some c= ontrollers only doing DMA - even for small transfers). I would say that if unsure, go with ARCH_DMA_MINALIGN. You just might waste= some space in some archs. OTOH, if you think DMA is not really a thing for pwm c= hips, you might go ARCH_KMALLOC_MINALIGN. And since you already have your own PWMCHIP= _ALIGN, it should be easy to change the requirements down the road (if needed). That said, I'm not familiar with dma_get_cache_alignment(). - Nuno S=C3=A1