[v2,2/9] dmaengine: edma: Use enum for eDMA binding type (legacy vs TPCC)

Commit Message

Peter Ujfalusi Sept. 21, 2016, 11:07 a.m. UTC

On 09/21/16 13:31, Arnd Bergmann wrote:
> On Wednesday, September 21, 2016 1:26:30 PM CEST Peter Ujfalusi wrote:
>> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
>> index c2098a4b4dcf..4c8818278fcc 100644
>> --- a/drivers/dma/edma.c
>> +++ b/drivers/dma/edma.c
>> @@ -261,8 +261,11 @@ static const struct edmacc_param dummy_paramset = {
>>         .ccnt = 1,
>>  };
>>  
>> -#define EDMA_BINDING_LEGACY    0
>> -#define EDMA_BINDING_TPCC      1
>> +enum edma_binding_type {
>> +       EDMA_BINDING_LEGACY = 0,
>> +       EDMA_BINDING_TPCC,
>> +};



>> +
>>  static const struct of_device_id edma_of_ids[] = {
>>         {
>>                 .compatible = "ti,edma3",
>> @@ -2184,7 +2187,8 @@ static int edma_probe(struct platform_device *pdev)
>>                 const struct of_device_id *match;
>>  
>>                 match = of_match_node(edma_of_ids, node);
>> -               if (match && (u32)match->data == EDMA_BINDING_TPCC)
>> +               if (match &&
>> +                   (enum edma_binding_type)match->data == EDMA_BINDING_TPCC)
>>                         legacy_mode = false;
>>  
>>                 info = edma_setup_info_from_dt(dev, legacy_mode);
>> -- 
>> 2.10.0
>>
> 
> Are you sure this works on all architectures? IIRC the size of an enum
> is implementation defined, so this could still fail sometimes.

True, some arch have HW type for enum or something similar.

> 
> I tend to use 'uintptr_t' for the cast instead.

What about keeping the defines and:



same for the ti-dma-crossbar.

Comments

Arnd Bergmann Sept. 21, 2016, 11:38 a.m. UTC | #1

On Wednesday, September 21, 2016 2:07:22 PM CEST Peter Ujfalusi wrote:
> > 
> > I tend to use 'uintptr_t' for the cast instead.
> 
> What about keeping the defines and:
> 
> diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
> index 3e9606b08340..493fdf30e8b8 100644
> --- a/drivers/dma/edma.c
> +++ b/drivers/dma/edma.c
> @@ -263,14 +263,19 @@ static const struct edmacc_param dummy_paramset = {
> 
>  #define EDMA_BINDING_LEGACY    0
>  #define EDMA_BINDING_TPCC      1
> +static const u32 edma_binding_type[] = {
> +       [EDMA_BINDING_LEGACY] = EDMA_BINDING_LEGACY,
> +       [EDMA_BINDING_TPCC] = EDMA_BINDING_TPCC,
> +};
> +
>  static const struct of_device_id edma_of_ids[] = {
>         {
>                 .compatible = "ti,edma3",
> -               .data = (void *)EDMA_BINDING_LEGACY,
> +               .data = (void *)&edma_binding_type[EDMA_BINDING_LEGACY],
>         },
>         {
>                 .compatible = "ti,edma3-tpcc",
> -               .data = (void *)EDMA_BINDING_TPCC,
> +               .data = (void *)&edma_binding_type[EDMA_BINDING_TPCC],
>         },
>         {}

You can drop the cast to (void *) here, otherwise looks good.

	Arnd

Patch

diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c
index 3e9606b08340..493fdf30e8b8 100644
--- a/drivers/dma/edma.c
+++ b/drivers/dma/edma.c
@@ -263,14 +263,19 @@  static const struct edmacc_param dummy_paramset = {

 #define EDMA_BINDING_LEGACY	0
 #define EDMA_BINDING_TPCC	1
+static const u32 edma_binding_type[] = {
+	[EDMA_BINDING_LEGACY] = EDMA_BINDING_LEGACY,
+	[EDMA_BINDING_TPCC] = EDMA_BINDING_TPCC,
+};
+
 static const struct of_device_id edma_of_ids[] = {
 	{
 		.compatible = "ti,edma3",
-		.data = (void *)EDMA_BINDING_LEGACY,
+		.data = (void *)&edma_binding_type[EDMA_BINDING_LEGACY],
 	},
 	{
 		.compatible = "ti,edma3-tpcc",
-		.data = (void *)EDMA_BINDING_TPCC,
+		.data = (void *)&edma_binding_type[EDMA_BINDING_TPCC],
 	},
 	{}
 };
@@ -2183,7 +2188,7 @@  static int edma_probe(struct platform_device *pdev)
 		const struct of_device_id *match;

 		match = of_match_node(edma_of_ids, node);
-		if (match && (u32)match->data == EDMA_BINDING_TPCC)
+		if (match && (*(u32*)match->data) == EDMA_BINDING_TPCC)
 			legacy_mode = false;

 		info = edma_setup_info_from_dt(dev, legacy_mode);