diff mbox series

[next,4/6] soc: ti: k3-ringacc: add request pair of rings api.

Message ID 20200701103030.29684-5-grygorii.strashko@ti.com (mailing list archive)
State Not Applicable
Headers show
Series soc: ti: k3-ringacc: updates | expand

Commit Message

Grygorii Strashko July 1, 2020, 10:30 a.m. UTC 
Add new API k3_ringacc_request_rings_pair() to request pair of rings at
once, as in the most cases Rings are used with DMA channels, which need to
request pair of rings - one to feed DMA with descriptors (TX/RX FDQ) and
one to receive completions (RX/TX CQ). This will allow to simplify Ringacc
API users.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/soc/ti/k3-ringacc.c       | 24 ++++++++++++++++++++++++
 include/linux/soc/ti/k3-ringacc.h |  4 ++++
 2 files changed, 28 insertions(+)

Comments

Peter Ujfalusi July 1, 2020, 11:54 a.m. UTC | #1 
Hi Grygorii,

On 01/07/2020 13.30, Grygorii Strashko wrote:
> Add new API k3_ringacc_request_rings_pair() to request pair of rings at
> once, as in the most cases Rings are used with DMA channels, which need to
> request pair of rings - one to feed DMA with descriptors (TX/RX FDQ) and
> one to receive completions (RX/TX CQ). This will allow to simplify Ringacc
> API users.
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/soc/ti/k3-ringacc.c       | 24 ++++++++++++++++++++++++
>  include/linux/soc/ti/k3-ringacc.h |  4 ++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
> index 8a8f31d59e24..4cf1150de88e 100644
> --- a/drivers/soc/ti/k3-ringacc.c
> +++ b/drivers/soc/ti/k3-ringacc.c
> @@ -322,6 +322,30 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>  }
>  EXPORT_SYMBOL_GPL(k3_ringacc_request_ring);
>  
> +int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
> +				  int fwd_id, int compl_id,
> +				  struct k3_ring **fwd_ring,
> +				  struct k3_ring **compl_ring)

Would you consider re-arranging the parameter list to:
int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
				  struct k3_ring **fwd_ring, int fwd_id,
				  struct k3_ring **compl_ring, int compl_id)

> +{
> +	int ret = 0;
> +
> +	if (!fwd_ring || !compl_ring)
> +		return -EINVAL;
> +
> +	*fwd_ring = k3_ringacc_request_ring(ringacc, fwd_id, 0);
> +	if (!(*fwd_ring))
> +		return -ENODEV;
> +
> +	*compl_ring = k3_ringacc_request_ring(ringacc, compl_id, 0);
> +	if (!(*compl_ring)) {
> +		k3_ringacc_ring_free(*fwd_ring);
> +		ret = -ENODEV;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(k3_ringacc_request_rings_pair);
> +
>  static void k3_ringacc_ring_reset_sci(struct k3_ring *ring)
>  {
>  	struct k3_ringacc *ringacc = ring->parent;
> diff --git a/include/linux/soc/ti/k3-ringacc.h b/include/linux/soc/ti/k3-ringacc.h
> index 26f73df0a524..7ac115432fa1 100644
> --- a/include/linux/soc/ti/k3-ringacc.h
> +++ b/include/linux/soc/ti/k3-ringacc.h
> @@ -107,6 +107,10 @@ struct k3_ringacc *of_k3_ringacc_get_by_phandle(struct device_node *np,
>  struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>  					int id, u32 flags);
>  
> +int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
> +				  int fwd_id, int compl_id,
> +				  struct k3_ring **fwd_ring,
> +				  struct k3_ring **compl_ring);
>  /**
>   * k3_ringacc_ring_reset - ring reset
>   * @ring: pointer on Ring
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Grygorii Strashko July 1, 2020, 12:12 p.m. UTC | #2 
On 01/07/2020 14:54, Peter Ujfalusi wrote:
> Hi Grygorii,
> 
> On 01/07/2020 13.30, Grygorii Strashko wrote:
>> Add new API k3_ringacc_request_rings_pair() to request pair of rings at
>> once, as in the most cases Rings are used with DMA channels, which need to
>> request pair of rings - one to feed DMA with descriptors (TX/RX FDQ) and
>> one to receive completions (RX/TX CQ). This will allow to simplify Ringacc
>> API users.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   drivers/soc/ti/k3-ringacc.c       | 24 ++++++++++++++++++++++++
>>   include/linux/soc/ti/k3-ringacc.h |  4 ++++
>>   2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
>> index 8a8f31d59e24..4cf1150de88e 100644
>> --- a/drivers/soc/ti/k3-ringacc.c
>> +++ b/drivers/soc/ti/k3-ringacc.c
>> @@ -322,6 +322,30 @@ struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
>>   }
>>   EXPORT_SYMBOL_GPL(k3_ringacc_request_ring);
>>   
>> +int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
>> +				  int fwd_id, int compl_id,
>> +				  struct k3_ring **fwd_ring,
>> +				  struct k3_ring **compl_ring)
> 
> Would you consider re-arranging the parameter list to:
> int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
> 				  struct k3_ring **fwd_ring, int fwd_id,
> 				  struct k3_ring **compl_ring, int compl_id)
> 

i think it's more common to have input parameters first.

>> +{
>> +	int ret = 0;
>> +
>> +	if (!fwd_ring || !compl_ring)
>> +		return -EINVAL;
>> +
>> +	*fwd_ring = k3_ringacc_request_ring(ringacc, fwd_id, 0);
>> +	if (!(*fwd_ring))
>> +		return -ENODEV;
>> +
>> +	*compl_ring = k3_ringacc_request_ring(ringacc, compl_id, 0);
>> +	if (!(*compl_ring)) {
>> +		k3_ringacc_ring_free(*fwd_ring);
>> +		ret = -ENODEV;
>> +	}
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(k3_ringacc_request_rings_pair);
>> +
Peter Ujfalusi July 1, 2020, 12:33 p.m. UTC | #3 
On 01/07/2020 15.12, Grygorii Strashko wrote:
> 
> 
> On 01/07/2020 14:54, Peter Ujfalusi wrote:
>> Hi Grygorii,
>>
>> On 01/07/2020 13.30, Grygorii Strashko wrote:
>>> Add new API k3_ringacc_request_rings_pair() to request pair of rings at
>>> once, as in the most cases Rings are used with DMA channels, which
>>> need to
>>> request pair of rings - one to feed DMA with descriptors (TX/RX FDQ) and
>>> one to receive completions (RX/TX CQ). This will allow to simplify
>>> Ringacc
>>> API users.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>>   drivers/soc/ti/k3-ringacc.c       | 24 ++++++++++++++++++++++++
>>>   include/linux/soc/ti/k3-ringacc.h |  4 ++++
>>>   2 files changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
>>> index 8a8f31d59e24..4cf1150de88e 100644
>>> --- a/drivers/soc/ti/k3-ringacc.c
>>> +++ b/drivers/soc/ti/k3-ringacc.c
>>> @@ -322,6 +322,30 @@ struct k3_ring *k3_ringacc_request_ring(struct
>>> k3_ringacc *ringacc,
>>>   }
>>>   EXPORT_SYMBOL_GPL(k3_ringacc_request_ring);
>>>   +int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
>>> +                  int fwd_id, int compl_id,
>>> +                  struct k3_ring **fwd_ring,
>>> +                  struct k3_ring **compl_ring)
>>
>> Would you consider re-arranging the parameter list to:
>> int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
>>                   struct k3_ring **fwd_ring, int fwd_id,
>>                   struct k3_ring **compl_ring, int compl_id)
>>
> 
> i think it's more common to have input parameters first.

That's true. I just like parameters grouped.
(ringacc, fwd_id, fwd_ring, compl_id, compl_ring)

having said that, I don't have objection to leave things as they are.

> 
>>> +{
>>> +    int ret = 0;
>>> +
>>> +    if (!fwd_ring || !compl_ring)
>>> +        return -EINVAL;
>>> +
>>> +    *fwd_ring = k3_ringacc_request_ring(ringacc, fwd_id, 0);
>>> +    if (!(*fwd_ring))
>>> +        return -ENODEV;
>>> +
>>> +    *compl_ring = k3_ringacc_request_ring(ringacc, compl_id, 0);
>>> +    if (!(*compl_ring)) {
>>> +        k3_ringacc_ring_free(*fwd_ring);
>>> +        ret = -ENODEV;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(k3_ringacc_request_rings_pair);
>>> +
> 
> 
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
diff mbox series

Patch

diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
index 8a8f31d59e24..4cf1150de88e 100644
--- a/drivers/soc/ti/k3-ringacc.c
+++ b/drivers/soc/ti/k3-ringacc.c
@@ -322,6 +322,30 @@  struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
 }
 EXPORT_SYMBOL_GPL(k3_ringacc_request_ring);
 
+int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
+				  int fwd_id, int compl_id,
+				  struct k3_ring **fwd_ring,
+				  struct k3_ring **compl_ring)
+{
+	int ret = 0;
+
+	if (!fwd_ring || !compl_ring)
+		return -EINVAL;
+
+	*fwd_ring = k3_ringacc_request_ring(ringacc, fwd_id, 0);
+	if (!(*fwd_ring))
+		return -ENODEV;
+
+	*compl_ring = k3_ringacc_request_ring(ringacc, compl_id, 0);
+	if (!(*compl_ring)) {
+		k3_ringacc_ring_free(*fwd_ring);
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(k3_ringacc_request_rings_pair);
+
 static void k3_ringacc_ring_reset_sci(struct k3_ring *ring)
 {
 	struct k3_ringacc *ringacc = ring->parent;
diff --git a/include/linux/soc/ti/k3-ringacc.h b/include/linux/soc/ti/k3-ringacc.h
index 26f73df0a524..7ac115432fa1 100644
--- a/include/linux/soc/ti/k3-ringacc.h
+++ b/include/linux/soc/ti/k3-ringacc.h
@@ -107,6 +107,10 @@  struct k3_ringacc *of_k3_ringacc_get_by_phandle(struct device_node *np,
 struct k3_ring *k3_ringacc_request_ring(struct k3_ringacc *ringacc,
 					int id, u32 flags);
 
+int k3_ringacc_request_rings_pair(struct k3_ringacc *ringacc,
+				  int fwd_id, int compl_id,
+				  struct k3_ring **fwd_ring,
+				  struct k3_ring **compl_ring);
 /**
  * k3_ringacc_ring_reset - ring reset
  * @ring: pointer on Ring