diff mbox

[RFC,2/7] platform-msi: Factor out allocation/free of private data

Message ID 1444923568-17413-3-git-send-email-marc.zyngier@arm.com
State Not Applicable
Headers show

Commit Message

Marc Zyngier Oct. 15, 2015, 3:39 p.m. UTC 
As we're going to have multiple paths to allocate/free the
platform-msi private data, factor this out into separate
utility functions.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/base/platform-msi.c | 84 ++++++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

Comments

Jiang Liu Oct. 16, 2015, 5:46 a.m. UTC | #1 
On 2015/10/15 23:39, Marc Zyngier wrote:
> As we're going to have multiple paths to allocate/free the
> platform-msi private data, factor this out into separate
> utility functions.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/base/platform-msi.c | 84 ++++++++++++++++++++++++++-------------------
>  1 file changed, 48 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
> index 6148c78..92666cd 100644
> --- a/drivers/base/platform-msi.c
> +++ b/drivers/base/platform-msi.c
> @@ -189,21 +189,11 @@ struct irq_domain *platform_msi_create_irq_domain(struct fwnode_handle *fwnode,
>  	return domain;
>  }
>  
> -/**
> - * platform_msi_domain_alloc_irqs - Allocate MSI interrupts for @dev
> - * @dev:		The device for which to allocate interrupts
> - * @nvec:		The number of interrupts to allocate
> - * @write_msi_msg:	Callback to write an interrupt message for @dev
> - *
> - * Returns:
> - * Zero for success, or an error code in case of failure
> - */
> -int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
> -				   irq_write_msi_msg_t write_msi_msg)
> +static int platform_msi_alloc_priv_data(struct device *dev, unsigned int nvec,
> +					irq_write_msi_msg_t write_msi_msg,
> +					struct platform_msi_priv_data **data)
How about making platform_msi_alloc_priv_data() return a pointer
instead of an int, that may simplify the code a bit.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier Oct. 16, 2015, 8:50 a.m. UTC | #2 
On 16/10/15 06:46, Jiang Liu wrote:
> On 2015/10/15 23:39, Marc Zyngier wrote:
>> As we're going to have multiple paths to allocate/free the
>> platform-msi private data, factor this out into separate
>> utility functions.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/base/platform-msi.c | 84 ++++++++++++++++++++++++++-------------------
>>  1 file changed, 48 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
>> index 6148c78..92666cd 100644
>> --- a/drivers/base/platform-msi.c
>> +++ b/drivers/base/platform-msi.c
>> @@ -189,21 +189,11 @@ struct irq_domain *platform_msi_create_irq_domain(struct fwnode_handle *fwnode,
>>  	return domain;
>>  }
>>  
>> -/**
>> - * platform_msi_domain_alloc_irqs - Allocate MSI interrupts for @dev
>> - * @dev:		The device for which to allocate interrupts
>> - * @nvec:		The number of interrupts to allocate
>> - * @write_msi_msg:	Callback to write an interrupt message for @dev
>> - *
>> - * Returns:
>> - * Zero for success, or an error code in case of failure
>> - */
>> -int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
>> -				   irq_write_msi_msg_t write_msi_msg)
>> +static int platform_msi_alloc_priv_data(struct device *dev, unsigned int nvec,
>> +					irq_write_msi_msg_t write_msi_msg,
>> +					struct platform_msi_priv_data **data)
> How about making platform_msi_alloc_priv_data() return a pointer
> instead of an int, that may simplify the code a bit.
> 

That's a good point. I could encode the error code in the return pointer
(PTR_ERR).

Thanks,

	M.
diff mbox

Patch

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 6148c78..92666cd 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -189,21 +189,11 @@  struct irq_domain *platform_msi_create_irq_domain(struct fwnode_handle *fwnode,
 	return domain;
 }
 
-/**
- * platform_msi_domain_alloc_irqs - Allocate MSI interrupts for @dev
- * @dev:		The device for which to allocate interrupts
- * @nvec:		The number of interrupts to allocate
- * @write_msi_msg:	Callback to write an interrupt message for @dev
- *
- * Returns:
- * Zero for success, or an error code in case of failure
- */
-int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
-				   irq_write_msi_msg_t write_msi_msg)
+static int platform_msi_alloc_priv_data(struct device *dev, unsigned int nvec,
+					irq_write_msi_msg_t write_msi_msg,
+					struct platform_msi_priv_data **data)
 {
-	struct platform_msi_priv_data *priv_data;
-	int err;
-
+	struct platform_msi_priv_data *datap;
 	/*
 	 * Limit the number of interrupts to 256 per device. Should we
 	 * need to bump this up, DEV_ID_SHIFT should be adjusted
@@ -222,22 +212,51 @@  int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
 	if (!list_empty(dev_to_msi_list(dev)))
 		return -EBUSY;
 
-	priv_data = kzalloc(sizeof(*priv_data), GFP_KERNEL);
-	if (!priv_data)
+	datap = *data = kzalloc(sizeof(**data), GFP_KERNEL);
+	if (!datap)
 		return -ENOMEM;
 
-	priv_data->devid = ida_simple_get(&platform_msi_devid_ida,
-					  0, 1 << DEV_ID_SHIFT, GFP_KERNEL);
-	if (priv_data->devid < 0) {
-		err = priv_data->devid;
-		goto out_free_data;
+	datap->devid = ida_simple_get(&platform_msi_devid_ida,
+				      0, 1 << DEV_ID_SHIFT, GFP_KERNEL);
+	if (datap->devid < 0) {
+		int err = datap->devid;
+		kfree(*data);
+		return err;
 	}
 
-	priv_data->write_msg = write_msi_msg;
+	datap->write_msg = write_msi_msg;
+
+	return 0;
+}
+
+static void platform_msi_free_priv_data(struct platform_msi_priv_data *data)
+{
+	ida_simple_remove(&platform_msi_devid_ida, data->devid);
+	kfree(data);
+}
+
+/**
+ * platform_msi_domain_alloc_irqs - Allocate MSI interrupts for @dev
+ * @dev:		The device for which to allocate interrupts
+ * @nvec:		The number of interrupts to allocate
+ * @write_msi_msg:	Callback to write an interrupt message for @dev
+ *
+ * Returns:
+ * Zero for success, or an error code in case of failure
+ */
+int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
+				   irq_write_msi_msg_t write_msi_msg)
+{
+	struct platform_msi_priv_data *priv_data;
+	int err;
+
+	err = platform_msi_alloc_priv_data(dev, nvec, write_msi_msg, &priv_data);
+	if (err)
+		return err;
 
 	err = platform_msi_alloc_descs(dev, nvec, priv_data);
 	if (err)
-		goto out_free_id;
+		goto out_free_priv_data;
 
 	err = msi_domain_alloc_irqs(dev->msi_domain, dev, nvec);
 	if (err)
@@ -247,10 +266,8 @@  int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
 
 out_free_desc:
 	platform_msi_free_descs(dev, 0, nvec);
-out_free_id:
-	ida_simple_remove(&platform_msi_devid_ida, priv_data->devid);
-out_free_data:
-	kfree(priv_data);
+out_free_priv_data:
+	platform_msi_free_priv_data(priv_data);
 
 	return err;
 }
@@ -261,16 +278,11 @@  out_free_data:
  */
 void platform_msi_domain_free_irqs(struct device *dev)
 {
-	struct msi_desc *desc;
-
-	desc = first_msi_entry(dev);
-	if (desc) {
-		struct platform_msi_priv_data *data;
-
-		data = desc->platform.msi_priv_data;
+	if (!list_empty(dev_to_msi_list(dev))) {
+		struct msi_desc *desc;
 
-		ida_simple_remove(&platform_msi_devid_ida, data->devid);
-		kfree(data);
+		desc = first_msi_entry(dev);
+		platform_msi_free_priv_data(desc->platform.msi_priv_data);
 	}
 
 	msi_domain_free_irqs(dev->msi_domain, dev);