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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 74BBCC001E0 for ; Mon, 23 Oct 2023 13:34:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 693CE10E1DE; Mon, 23 Oct 2023 13:34:25 +0000 (UTC) Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by gabe.freedesktop.org (Postfix) with ESMTPS id A125189A4E for ; Mon, 23 Oct 2023 13:34:21 +0000 (UTC) Received: by mail-wm1-x336.google.com with SMTP id 5b1f17b1804b1-4083dbc43cfso24218945e9.3 for ; Mon, 23 Oct 2023 06:34:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1698068060; x=1698672860; darn=lists.freedesktop.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=ufOU/q/Ep1kAY17X99uhm7aprCNDlkkpHRrj3TivtTs=; b=H7WvVF75IM5WPe+q3Tb5+oThUtgadDKn6Bk3uy7WYZvpyTRccbT9J9J77UrvGvL+dy gvT8O420safmOda7ILkDkWzAo7c7loO5ZV8Jyrn0AFsnDdGQJ+hkjpb4p8zj+UvRuvXo X4QRYICeWK8GGi6luiuE0cMJf5mu/XEF8+5CD2KGfQfWAkO3t4/ywL0noIPuri+F0yfN FV1aXeUnTB1iXXcStB+bK4OtW7hqHAIpitUMTL5vwWDoNJ+6MiBgEdV1guwZHE31q8p3 LAgViSIplfnCHqMBk/vz5mwzb9mbkl7Yb9BV56YDP554bThDJBRS4FaM/dqCe6QaZxy/ hXQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698068060; x=1698672860; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ufOU/q/Ep1kAY17X99uhm7aprCNDlkkpHRrj3TivtTs=; b=efI7d8VsdUDRkhRUCSm/EpY9xWsAxQClIa6WqfWr5UtuNqlVYP4UaDw9bvN6RJ8Mgy 7L2zyr+VjO9fLLahM+RmOBxojnk2yJ9v7SIzzyOxkp+RruZ/ukG5Y+dxPerHqmgj7jqD O/6bEE2SgjVBqIasEGJomu7TJiQCqqPKiUZ+hXA4d1qv4akamIW1KZbfcPzLViZhSreP N9d1C3UhdYdvgmKNcE+3SSYMCyjtWdb2rTb5ZtYX60au511sFIQDXWTlN+Hr2TNwkE2h FFLRuwLsf8ZMv/NBnwY5gxLGGVeAh1gvKJGu2cAmFQSwaPp2xANkr94lM3R/5GE9ZVGR qOfw== X-Gm-Message-State: AOJu0YwCmSEuoq+zlLf4OywmK4y7wELODCz+xTUl+AFMtR+48EJoKtu3 tT2jdTWwiLoZeTE363boRiJ/4Q== X-Google-Smtp-Source: AGHT+IHwl+E4YFajmY7IWxRpWgtSmDl22RT8PM7nnOAmIYDUhgS2AbK4FuptOkLbmHVvg59kegsf/Q== X-Received: by 2002:a05:600c:35c5:b0:405:4002:825a with SMTP id r5-20020a05600c35c500b004054002825amr7494266wmq.13.1698068059941; Mon, 23 Oct 2023 06:34:19 -0700 (PDT) Received: from aspen.lan (aztw-34-b2-v4wan-166919-cust780.vm26.cable.virginm.net. [82.37.195.13]) by smtp.gmail.com with ESMTPSA id 1-20020a05600c028100b004077219aed5sm14125564wmk.6.2023.10.23.06.34.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Oct 2023 06:34:19 -0700 (PDT) Date: Mon, 23 Oct 2023 14:34:17 +0100 From: Daniel Thompson To: Sean Young Message-ID: <20231023133417.GE49511@aspen.lan> References: <90728c06-4c6c-b3d2-4723-c24711be2fa5@redhat.com> <20231019105118.64gdzzixwqrztjir@pengutronix.de> <01a505ac-320f-3819-a58d-2b82c1bf2a86@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [Intel-gfx] [PATCH v3 1/3] pwm: make it possible to apply pwm changes in atomic context X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-fbdev@vger.kernel.org, linux-doc@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Pavel Machek , David Airlie , linux-leds@vger.kernel.org, Ivaylo Dimitrov , Jonathan Corbet , Helge Deller , Lee Jones , Javier Martinez Canillas , Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Ilpo =?iso-8859-1?Q?J=E4rvinen?= , Guenter Roeck , linux-media@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-pwm@vger.kernel.org, Jean Delvare , intel-gfx@lists.freedesktop.org, linux-input@vger.kernel.org, Mark Gross , Hans de Goede , Mark Brown , Rodrigo Vivi , Mauro Carvalho Chehab , linux-arm-kernel@lists.infradead.org, linux-hwmon@vger.kernel.org, Support Opensource , Jingoo Han , Dmitry Torokhov , Liam Girdwood , Daniel Vetter Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Sun, Oct 22, 2023 at 11:46:22AM +0100, Sean Young wrote: > Hi Hans, > > On Sat, Oct 21, 2023 at 11:08:22AM +0200, Hans de Goede wrote: > > On 10/19/23 12:51, Uwe Kleine-König wrote: > > > On Wed, Oct 18, 2023 at 03:57:48PM +0200, Hans de Goede wrote: > > >> On 10/17/23 11:17, Sean Young wrote: > > > I think it's very subjective if you consider this > > > churn or not. > > > > I consider it churn because I don't think adding a postfix > > for what is the default/expected behavior is a good idea > > (with GPIOs not sleeping is the expected behavior). > > > > I agree that this is very subjective and very much goes > > into the territory of bikeshedding. So please consider > > the above my 2 cents on this and lets leave it at that. > > You have a valid point. Let's focus on having descriptive function names. For a couple of days I've been trying to resist the bikeshedding (esp. given the changes to backlight are tiny) so I'll try to keep it as brief as I can: 1. I dislike the do_it() and do_it_cansleep() pairing. It is difficult to detect when a client driver calls do_it() by mistake. In fact a latent bug of this nature can only be detected by runtime testing with the small number of PWMs that do not support configuration from an atomic context. In contrast do_it() and do_it_atomic()[*] means that although incorrectly calling do_it() from an atomic context can be pretty catastrophic it is also trivially detected (with any PWM driver) simply by running with CONFIG_DEBUG_ATOMIC_SLEEP. No objections (beyond churn) to fully spelt out pairings such as do_it_cansleep() and do_it_atomic()[*]! 2. If there is an API rename can we make sure the patch contains no other changes (e.g. don't introduce any new API in the same patch). Seperating renames makes the patches easier to review! It makes each one smaller and easier to review! Daniel. [*] or do_it_nosleep()... etc.