[PATCH] mailbox: tegra-hsp: use devm_kstrdup_const()

[PATCH] mailbox: tegra-hsp: use devm_kstrdup_const()

[Bartosz Golaszewski]

Use devm_kstrdup_const() in the tegra-hsp driver. This mostly serves as
an example of how to use this new routine to shrink driver code.

Also use devm_kzalloc() instead of regular kzalloc() to shrink the
driver even more.

Doorbell objects are only removed in the driver's remove callback so
it's safe to convert all memory allocations to devres.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/mailbox/tegra-hsp.c | 41 ++++++++-----------------------------
 1 file changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index 0cde356c11ab..106c94dedbf1 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -183,14 +183,15 @@ static irqreturn_t tegra_hsp_doorbell_irq(int irq, void *data)
 }
 
 static struct tegra_hsp_channel *
-tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
-			  unsigned int master, unsigned int index)
+tegra_hsp_doorbell_create(struct device *dev, struct tegra_hsp *hsp,
+			  const char *name, unsigned int master,
+			  unsigned int index)
 {
 	struct tegra_hsp_doorbell *db;
 	unsigned int offset;
 	unsigned long flags;
 
-	db = kzalloc(sizeof(*db), GFP_KERNEL);
+	db = devm_kzalloc(dev, sizeof(*db), GFP_KERNEL);
 	if (!db)
 		return ERR_PTR(-ENOMEM);
 
@@ -200,7 +201,7 @@ tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
 	db->channel.regs = hsp->regs + offset;
 	db->channel.hsp = hsp;
 
-	db->name = kstrdup_const(name, GFP_KERNEL);
+	db->name = devm_kstrdup_const(dev, name, GFP_KERNEL);
 	db->master = master;
 	db->index = index;
 
@@ -211,13 +212,6 @@ tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
 	return &db->channel;
 }
 
-static void __tegra_hsp_doorbell_destroy(struct tegra_hsp_doorbell *db)
-{
-	list_del(&db->list);
-	kfree_const(db->name);
-	kfree(db);
-}
-
 static int tegra_hsp_doorbell_send_data(struct mbox_chan *chan, void *data)
 {
 	struct tegra_hsp_doorbell *db = chan->con_priv;
@@ -332,31 +326,16 @@ static struct mbox_chan *of_tegra_hsp_xlate(struct mbox_controller *mbox,
 	return chan ?: ERR_PTR(-EBUSY);
 }
 
-static void tegra_hsp_remove_doorbells(struct tegra_hsp *hsp)
-{
-	struct tegra_hsp_doorbell *db, *tmp;
-	unsigned long flags;
-
-	spin_lock_irqsave(&hsp->lock, flags);
-
-	list_for_each_entry_safe(db, tmp, &hsp->doorbells, list)
-		__tegra_hsp_doorbell_destroy(db);
-
-	spin_unlock_irqrestore(&hsp->lock, flags);
-}
-
-static int tegra_hsp_add_doorbells(struct tegra_hsp *hsp)
+static int tegra_hsp_add_doorbells(struct device *dev, struct tegra_hsp *hsp)
 {
 	const struct tegra_hsp_db_map *map = hsp->soc->map;
 	struct tegra_hsp_channel *channel;
 
 	while (map->name) {
-		channel = tegra_hsp_doorbell_create(hsp, map->name,
+		channel = tegra_hsp_doorbell_create(dev, hsp, map->name,
 						    map->master, map->index);
-		if (IS_ERR(channel)) {
-			tegra_hsp_remove_doorbells(hsp);
+		if (IS_ERR(channel))
 			return PTR_ERR(channel);
-		}
 
 		map++;
 	}
@@ -412,7 +391,7 @@ static int tegra_hsp_probe(struct platform_device *pdev)
 	if (!hsp->mbox.chans)
 		return -ENOMEM;
 
-	err = tegra_hsp_add_doorbells(hsp);
+	err = tegra_hsp_add_doorbells(&pdev->dev, hsp);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to add doorbells: %d\n", err);
 		return err;
@@ -423,7 +402,6 @@ static int tegra_hsp_probe(struct platform_device *pdev)
 	err = mbox_controller_register(&hsp->mbox);
 	if (err) {
 		dev_err(&pdev->dev, "failed to register mailbox: %d\n", err);
-		tegra_hsp_remove_doorbells(hsp);
 		return err;
 	}
 
@@ -443,7 +421,6 @@ static int tegra_hsp_remove(struct platform_device *pdev)
 	struct tegra_hsp *hsp = platform_get_drvdata(pdev);
 
 	mbox_controller_unregister(&hsp->mbox);
-	tegra_hsp_remove_doorbells(hsp);
 
 	return 0;
 }
-- 
2.19.1

Re: [PATCH] mailbox: tegra-hsp: use devm_kstrdup_const()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

On Thu, Nov 08, 2018 at 05:46:10PM +0100, Bartosz Golaszewski wrote:
> Use devm_kstrdup_const() in the tegra-hsp driver. This mostly serves as
> an example of how to use this new routine to shrink driver code.
> 
> Also use devm_kzalloc() instead of regular kzalloc() to shrink the
> driver even more.
> 
> Doorbell objects are only removed in the driver's remove callback so
> it's safe to convert all memory allocations to devres.
> 
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> ---
>  drivers/mailbox/tegra-hsp.c | 41 ++++++++-----------------------------
>  1 file changed, 9 insertions(+), 32 deletions(-)

This looks really nice. I'm currently working on a series of patches to
add shared mailboxes to the HSP driver. Part of that series adds struct
device *dev to struct tegra_hsp, which in turn would simplify this
patch a little.

Looking at some of the changes here it also seems like they would
conflict with my patches in some places. Would you mind if I pick this
patch up into my series and resolve the conflicts there?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] mailbox: tegra-hsp: use devm_kstrdup_const()

[Bartosz Golaszewski]

wt., 13 lis 2018 o 22:11 Thierry Reding <thierry.reding@gmail.com> napisał(a):
>
> On Thu, Nov 08, 2018 at 05:46:10PM +0100, Bartosz Golaszewski wrote:
> > Use devm_kstrdup_const() in the tegra-hsp driver. This mostly serves as
> > an example of how to use this new routine to shrink driver code.
> >
> > Also use devm_kzalloc() instead of regular kzalloc() to shrink the
> > driver even more.
> >
> > Doorbell objects are only removed in the driver's remove callback so
> > it's safe to convert all memory allocations to devres.
> >
> > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> > ---
> >  drivers/mailbox/tegra-hsp.c | 41 ++++++++-----------------------------
> >  1 file changed, 9 insertions(+), 32 deletions(-)
>
> This looks really nice. I'm currently working on a series of patches to
> add shared mailboxes to the HSP driver. Part of that series adds struct
> device *dev to struct tegra_hsp, which in turn would simplify this
> patch a little.
>
> Looking at some of the changes here it also seems like they would
> conflict with my patches in some places. Would you mind if I pick this
> patch up into my series and resolve the conflicts there?
>

Not at all, please do. I just want to add at least one example user of
the routine (devm_kstrdup_const) I introduced in the last merge
window.

Best regards,
Bartosz Golaszewski