On Thu, 2020-12-03 at 09:05 +0100, Bartosz Golaszewski wrote:

On Mon, Nov 23, 2020 at 7:38 PM Nicolas Saenz Julienne
[off-list ref] wrote:

quoted

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 rpi_firmware_put()
which will permit automatically decrease the reference count upon
unbinding consumer drivers.

Suggested-by: Uwe Kleine-König <redacted>
Signed-off-by: Nicolas Saenz Julienne <redacted>
---

Changes since v3:
- Use kref instead of waiting on refcount

 drivers/firmware/raspberrypi.c             | 37 +++++++++++++++++++---
 include/soc/bcm2835/raspberrypi-firmware.h |  2 ++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c
index 30259dc9b805..ed793aef7851 100644
--- a/drivers/firmware/raspberrypi.c
+++ b/drivers/firmware/raspberrypi.c

@@ -7,6 +7,7 @@

  */

 #include <linux/dma-mapping.h>
+#include <linux/kref.h>
 #include <linux/mailbox_client.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>

@@ -27,6 +28,8 @@ struct rpi_firmware {

        struct mbox_chan *chan; /* The property channel. */
        struct completion c;
        u32 enabled;
+
+       struct kref consumers;
 };

 static DEFINE_MUTEX(transaction_lock);

@@ -225,12 +228,27 @@ static void rpi_register_clk_driver(struct device *dev)

                                                -1, NULL, 0);
 }

+static void rpi_firmware_delete(struct kref *kref)
+{
+       struct rpi_firmware *fw = container_of(kref, struct rpi_firmware,
+                                              consumers);
+
+       mbox_free_channel(fw->chan);
+       kfree(fw);
+}
+
+void rpi_firmware_put(struct rpi_firmware *fw)
+{
+       kref_put(&fw->consumers, rpi_firmware_delete);
+}
+EXPORT_SYMBOL_GPL(rpi_firmware_put);
+
 static int rpi_firmware_probe(struct platform_device *pdev)
 {
        struct device *dev = &pdev->dev;
        struct rpi_firmware *fw;

-       fw = devm_kzalloc(dev, sizeof(*fw), GFP_KERNEL);

One nit from my side: maybe add a comment here saying that you really
want to use non-managed kzalloc() because you're going to get people
blindly converting it to devm_kzalloc() very soon.

Good point, I'll change it.

Regards,
Nicolas