From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: linux-doc@vger.kernel.org, dri-devel@lists.freedesktop.org,
platform-driver-x86@vger.kernel.org,
linux-hardening@vger.kernel.org, linux-riscv@lists.infradead.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-leds@vger.kernel.org, chrome-platform@lists.linux.dev,
linux-samsung-soc@vger.kernel.org,
Bartosz Golaszewski <brgl@bgdev.pl>,
linux-staging@lists.linux.dev,
linux-rockchip@lists.infradead.org, linux-sunxi@lists.linux.dev,
linux-pwm@vger.kernel.org, greybus-dev@lists.linaro.org,
linux-mediatek@lists.infradead.org,
linux-rpi-kernel@lists.infradead.org,
linux-amlogic@lists.infradead.org,
linux-arm-kernel@lists.infradead.org, linux-mips@vger.kernel.org,
asahi@lists.linux.dev, kernel@pengutronix.de
Subject: Re: [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips
Date: Tue, 12 Dec 2023 22:05:09 +0100 [thread overview]
Message-ID: <20231212210509.focpb63fbmahqij3@pengutronix.de> (raw)
In-Reply-To: <ZXbzcFTnDTKoZAta@orome.fritz.box>
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
Hello Thierry,
On Mon, Dec 11, 2023 at 12:33:04PM +0100, Thierry Reding wrote:
> On Fri, Dec 08, 2023 at 07:50:33PM +0100, Uwe Kleine-König wrote:
> > You don't need to touch all drivers because you didn't change struct
> > pwm_chip::dev yet. (If you really want, you don't need to change that,
> > but then you have some duplication as chip->dev holds the same value as
> > priv->dev.parent in the end.)
>
> I don't think that's a problem. These are for two logically separate
> things, after all.
How are they different? I'd say one is the initializer for the other and
(ideally) unused after that. With that interpretation they are indeed
different, but then it's ugly that the initializer keeps staying around.
> Duplication can also sometimes be useful to simplify
> things. There are plently of cases where we use local variables for the
> same reason.
local variables go away though after the respective function is left.
chip->dev and its copy priv->dev.parent stay around for the full
lifetime of the chip.
> > > @@ -58,23 +60,24 @@ static struct pwm_chip *pwmchip_find_by_name(const char *name)
> > >
> > > static int pwm_device_request(struct pwm_device *pwm, const char *label)
> > > {
> > > + struct pwm_chip *chip = pwm->priv->chip;
> >
> > With my approach getting the chip of a struct pwm_device is only one
> > pointer dereference away. You need two.
>
> None of the functions here are called very often, so even if this isn't
> optimized away it would hardly matter.
I'd say pwm_apply_state() at least matters. Also I think that making a
slow path quicker is a good thing.
I wonder how we'll converge to an approach that can go into the
mainline given that we both have our strong opinions.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
prev parent reply other threads:[~2023-12-12 21:05 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-06 11:43 [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips Uwe Kleine-König
2023-12-06 11:43 ` [PATCH v4 006/115] pwm: cros-ec: Change prototype of helpers to prepare further changes Uwe Kleine-König
2023-12-07 2:06 ` Tzung-Bi Shih
2023-12-06 11:43 ` [PATCH v4 013/115] pwm: cros-ec: Make use of pwmchip_parent() macro Uwe Kleine-König
2023-12-06 11:44 ` [PATCH v4 057/115] pwm: cros-ec: Make use of devm_pwmchip_alloc() function Uwe Kleine-König
2023-12-07 2:07 ` Tzung-Bi Shih
2023-12-08 15:41 ` [PATCH v4 000/115] pwm: Fix lifetime issues for pwm_chips Thierry Reding
2023-12-08 18:50 ` Uwe Kleine-König
2023-12-11 11:33 ` Thierry Reding
2023-12-11 12:18 ` Uwe Kleine-König
2023-12-12 21:05 ` Uwe Kleine-König [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231212210509.focpb63fbmahqij3@pengutronix.de \
--to=u.kleine-koenig@pengutronix.de \
--cc=asahi@lists.linux.dev \
--cc=brgl@bgdev.pl \
--cc=chrome-platform@lists.linux.dev \
--cc=dri-devel@lists.freedesktop.org \
--cc=greybus-dev@lists.linaro.org \
--cc=kernel@pengutronix.de \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-rpi-kernel@lists.infradead.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux-sunxi@lists.linux.dev \
--cc=platform-driver-x86@vger.kernel.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).