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 X-Spam-Level: X-Spam-Status: No, score=-9.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 40B92C4741F for ; Thu, 5 Nov 2020 09:14:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D7AA82071A for ; Thu, 5 Nov 2020 09:14:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=baylibre-com.20150623.gappssmtp.com header.i=@baylibre-com.20150623.gappssmtp.com header.b="h2AbCJ6x" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730906AbgKEJOA (ORCPT ); Thu, 5 Nov 2020 04:14:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51558 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731044AbgKEJN7 (ORCPT ); Thu, 5 Nov 2020 04:13:59 -0500 Received: from mail-ej1-x641.google.com (mail-ej1-x641.google.com [IPv6:2a00:1450:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F3D3AC0613D6 for ; Thu, 5 Nov 2020 01:13:57 -0800 (PST) Received: by mail-ej1-x641.google.com with SMTP id cw8so1542718ejb.8 for ; Thu, 05 Nov 2020 01:13:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=cosNbBc+pGCwoieZsWO9CcMsgFAi+qgVJh6/ZaKe1v0=; b=h2AbCJ6xtX9calmobLzMDdUmOt4016ZAljqf+kWZm3SpJ5dyWf5xX8QV+8uAKO7KzB whVd90vtF4kEq0FjKmV2eXJCusoVmf8b1ZXm4g19A6c9d3zpTGecCZcl0xPaOrSMnUzM OGxi6KTPLoJsWq0pzXzySThLXXe1GQN0O9JNP7hJsN0mTV3NqA97jL96kLNinAgMKhRQ 8LQz93+7bzMlzUxaAtidPL1DLIRzbOEahVB/7cnJeyFT+Us2VOVrMCBQcnomz5KE/6GC gF+l6kEcCMCIt86RD/mLxN3qSNhWZV0UIk0wGcZ/d2v9DKiSjhWGarH/Lc5p3RkZiIJ/ zceA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=cosNbBc+pGCwoieZsWO9CcMsgFAi+qgVJh6/ZaKe1v0=; b=WYs9/pdF20q19uBm4+VWcPsCxTozm/8DOYumww8Aq4zzAIs+MhaGQQ/njJDQxJSzrv mXoNWpdB3rFFgifgzgcubHPh1zi7UuIGk3unA3MutbAi0h+G4reD+LTBqDEy8Zdo7hNa eIX8VhIsoe0GGEbTKTArZfrf+p7EwJNX+iYWhwZkYrFRB6LYpZ0JUmP6KAf/lDt41zSy 9CQ0GQsIHmrxx1RfeJ1EvWJiSf942STHQF6Dwi0C0Mt7daaEH29JXcERo2wbpjixXhug 0ola5DZ8JaK9Ix9eR9KlxcEb2LB2UoMAfhmNkfh8YE2Wsd7ba6oWUQ9RQxq9es2fsDak 2nKA== X-Gm-Message-State: AOAM530wMUpwqApbCXX72ooGkLSt3vSlnUbr19EzRddnAE/K++zXjN05 6PPFS7f2/JXtpGiAqpMq9Z8TiT+T386Ztg8b4XJoNA== X-Google-Smtp-Source: ABdhPJyFgzr6/OtoGqHbIjxgJayFJFC6ooeiWsm56YhTmEcoAALq1JBufGgUh3laISQqZelpAaKQMgQ7rYi7EiPI/xk= X-Received: by 2002:a17:907:420d:: with SMTP id oh21mr1350987ejb.429.1604567636515; Thu, 05 Nov 2020 01:13:56 -0800 (PST) MIME-Version: 1.0 References: <20201104103938.1286-1-nsaenzjulienne@suse.de> <20201104103938.1286-2-nsaenzjulienne@suse.de> In-Reply-To: <20201104103938.1286-2-nsaenzjulienne@suse.de> From: Bartosz Golaszewski Date: Thu, 5 Nov 2020 10:13:45 +0100 Message-ID: Subject: Re: [PATCH v3 01/11] firmware: raspberrypi: Introduce devm_rpi_firmware_get() To: Nicolas Saenz Julienne Cc: =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , LKML , Florian Fainelli , Ray Jui , Scott Branden , bcm-kernel-feedback-list@broadcom.com, linux-pwm@vger.kernel.org, arm-soc , linux-devicetree , wahrenst@gmx.net, Linux Input , Dmitry Torokhov , Greg KH , devel@driverdev.osuosl.org, Philipp Zabel , linux-gpio , Linus Walleij , linux-clk , Stephen Boyd , linux-rpi-kernel@lists.infradead.org, Andy Shevchenko Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On Wed, Nov 4, 2020 at 11:39 AM Nicolas Saenz Julienne wrote: > > When unbinding the firmware device we need to make sure it has no > consumers left. Otherwise we'd leave them with a firmware handle > pointing at freed memory. > > Keep a reference count of all consumers and introduce > devm_rpi_firmware_get() which will automatically decrease the reference > count upon unbinding consumer drivers. > > Suggested-by: Uwe Kleine-K=C3=B6nig > Signed-off-by: Nicolas Saenz Julienne > > --- > > Changes since v2: > - Create devm_rpi_firmware_get() > > drivers/firmware/raspberrypi.c | 46 ++++++++++++++++++++++ > include/soc/bcm2835/raspberrypi-firmware.h | 8 ++++ > 2 files changed, 54 insertions(+) > > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberryp= i.c > index 2371d08bdd17..74bdb3bde9dc 100644 > --- a/drivers/firmware/raspberrypi.c > +++ b/drivers/firmware/raspberrypi.c > @@ -11,7 +11,9 @@ > #include > #include > #include > +#include > #include > +#include > #include > > #define MBOX_MSG(chan, data28) (((data28) & ~0xf) | ((chan) & 0x= f)) > @@ -27,6 +29,9 @@ struct rpi_firmware { > struct mbox_chan *chan; /* The property channel. */ > struct completion c; > u32 enabled; > + > + refcount_t consumers; > + wait_queue_head_t wait; > }; > > static DEFINE_MUTEX(transaction_lock); > @@ -247,6 +252,8 @@ static int rpi_firmware_probe(struct platform_device = *pdev) > } > > init_completion(&fw->c); > + refcount_set(&fw->consumers, 1); > + init_waitqueue_head(&fw->wait); > > platform_set_drvdata(pdev, fw); > > @@ -275,11 +282,21 @@ static int rpi_firmware_remove(struct platform_devi= ce *pdev) > rpi_hwmon =3D NULL; > platform_device_unregister(rpi_clk); > rpi_clk =3D NULL; > + > + wait_event(fw->wait, refcount_dec_if_one(&fw->consumers)); > mbox_free_channel(fw->chan); > > return 0; > } > > +static void rpi_firmware_put(void *data) > +{ > + struct rpi_firmware *fw =3D data; > + > + refcount_dec(&fw->consumers); > + wake_up(&fw->wait); > +} > + > /** > * rpi_firmware_get - Get pointer to rpi_firmware structure. > * @firmware_node: Pointer to the firmware Device Tree node. > @@ -297,6 +314,35 @@ struct rpi_firmware *rpi_firmware_get(struct device_= node *firmware_node) > } > EXPORT_SYMBOL_GPL(rpi_firmware_get); > > +/** > + * devm_rpi_firmware_get - Get pointer to rpi_firmware structure. > + * @firmware_node: Pointer to the firmware Device Tree node. > + * > + * Returns NULL is the firmware device is not ready. > + */ > +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev, > + struct device_node *firmware_n= ode) > +{ > + struct platform_device *pdev =3D of_find_device_by_node(firmware_= node); > + struct rpi_firmware *fw; > + > + if (!pdev) > + return NULL; > + > + fw =3D platform_get_drvdata(pdev); > + if (!fw) > + return NULL; > + > + if (!refcount_inc_not_zero(&fw->consumers)) > + return NULL; > + > + if (devm_add_action_or_reset(dev, rpi_firmware_put, fw)) > + return NULL; > + > + return fw; > +} > +EXPORT_SYMBOL_GPL(devm_rpi_firmware_get); Usually I'd expect the devres variant to simply call rpi_firmware_get() and then schedule a release callback which would call whatever function is the release counterpart for it currently. Devres actions are for drivers which want to schedule some more unusual tasks at driver detach. Any reason for designing it this way? Bartosz > + > static const struct of_device_id rpi_firmware_of_match[] =3D { > { .compatible =3D "raspberrypi,bcm2835-firmware", }, > {}, > diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm= 2835/raspberrypi-firmware.h > index cc9cdbc66403..8fe64f53a394 100644 > --- a/include/soc/bcm2835/raspberrypi-firmware.h > +++ b/include/soc/bcm2835/raspberrypi-firmware.h > @@ -141,6 +141,8 @@ int rpi_firmware_property(struct rpi_firmware *fw, > int rpi_firmware_property_list(struct rpi_firmware *fw, > void *data, size_t tag_size); > struct rpi_firmware *rpi_firmware_get(struct device_node *firmware_node)= ; > +struct rpi_firmware *devm_rpi_firmware_get(struct device *dev, > + struct device_node *firmware_n= ode); > #else > static inline int rpi_firmware_property(struct rpi_firmware *fw, u32 tag= , > void *data, size_t len) > @@ -158,6 +160,12 @@ static inline struct rpi_firmware *rpi_firmware_get(= struct device_node *firmware > { > return NULL; > } > + > +static inline struct rpi_firmware *devm_rpi_firmware_get(struct device *= dev, > + struct device_node *firmware_node= ) > +{ > + return NULL; > +} > #endif > > #endif /* __SOC_RASPBERRY_FIRMWARE_H__ */ > -- > 2.29.1 >