[Xen-devel,RFC,07/19] xen/dts: Use unsigned int for MMIO and IRQ index

There is no reason to use signed integer for an index. Futhermore, this will
avoid possible issue when theses functions will be exposed to the guest
via new hypercalls.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/common/device_tree.c      |   10 +++++-----
 xen/include/xen/device_tree.h |    7 ++++---
 2 files changed, 9 insertions(+), 8 deletions(-)

On Mon, 16 Jun 2014, Julien Grall wrote:
> There is no reason to use signed integer for an index. Futhermore, this will
> avoid possible issue when theses functions will be exposed to the guest
> via new hypercalls.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/common/device_tree.c      |   10 +++++-----
>  xen/include/xen/device_tree.h |    7 ++++---
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index f0b17a3..4736e0d 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -876,7 +876,7 @@ static const struct dt_bus *dt_match_bus(const struct dt_device_node *np)
>  }
>  
>  static const __be32 *dt_get_address(const struct dt_device_node *dev,
> -                                    int index, u64 *size,
> +                                    unsigned index, u64 *size,

It is the same thing but you might as well use unsigned int.


>                                      unsigned int *flags)
>  {
>      const __be32 *prop;
> @@ -1063,7 +1063,7 @@ bail:
>  }
>  
>  /* dt_device_address - Translate device tree address and return it */
> -int dt_device_get_address(const struct dt_device_node *dev, int index,
> +int dt_device_get_address(const struct dt_device_node *dev, unsigned int index,
>                            u64 *addr, u64 *size)
>  {
>      const __be32 *addrp;
> @@ -1386,7 +1386,7 @@ fail:
>      return -EINVAL;
>  }
>  
> -int dt_device_get_raw_irq(const struct dt_device_node *device, int index,
> +int dt_device_get_raw_irq(const struct dt_device_node *device, uint32_t index,

Why are you changing the other indexes to unsigned int and this one to
uint32_t?


>                            struct dt_raw_irq *out_irq)
>  {
>      const struct dt_device_node *p;
> @@ -1394,7 +1394,7 @@ int dt_device_get_raw_irq(const struct dt_device_node *device, int index,
>      u32 intsize, intlen;
>      int res = -EINVAL;
>  
> -    dt_dprintk("dt_device_get_raw_irq: dev=%s, index=%d\n",
> +    dt_dprintk("dt_device_get_raw_irq: dev=%s, index=%u\n",
>                 device->full_name, index);
>  
>      /* Get the interrupts property */
> @@ -1445,7 +1445,7 @@ int dt_irq_translate(const struct dt_raw_irq *raw,
>                          &out_irq->irq, &out_irq->type);
>  }
>  
> -int dt_device_get_irq(const struct dt_device_node *device, int index,
> +int dt_device_get_irq(const struct dt_device_node *device, uint32_t index,

ditto


>                        struct dt_irq *out_irq)
>  {
>      struct dt_raw_irq raw;
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 25db076..e413447 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -502,7 +502,7 @@ const struct dt_device_node *dt_get_parent(const struct dt_device_node *node);
>   * This function resolves an address, walking the tree, for a give
>   * device-tree node. It returns 0 on success.
>   */
> -int dt_device_get_address(const struct dt_device_node *dev, int index,
> +int dt_device_get_address(const struct dt_device_node *dev, unsigned int index,
>                            u64 *addr, u64 *size);
>  
>  /**
> @@ -532,7 +532,7 @@ unsigned int dt_number_of_address(const struct dt_device_node *device);
>   * This function resolves an interrupt, walking the tree, for a given
>   * device-tree node. It's the high level pendant to dt_device_get_raw_irq().
>   */
> -int dt_device_get_irq(const struct dt_device_node *device, int index,
> +int dt_device_get_irq(const struct dt_device_node *device, unsigned int index,
>                        struct dt_irq *irq);
>  
>  /**
> @@ -544,7 +544,8 @@ int dt_device_get_irq(const struct dt_device_node *device, int index,
>   * This function resolves an interrupt for a device, no translation is
>   * made. dt_irq_translate can be called after.
>   */
> -int dt_device_get_raw_irq(const struct dt_device_node *device, int index,
> +int dt_device_get_raw_irq(const struct dt_device_node *device,
> +                          unsigned int index,
>                            struct dt_raw_irq *irq);
>  
>  /**
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

Hi Stefano,

On 06/18/2014 07:54 PM, Stefano Stabellini wrote:
> On Mon, 16 Jun 2014, Julien Grall wrote:
>> There is no reason to use signed integer for an index. Futhermore, this will
>> avoid possible issue when theses functions will be exposed to the guest
>> via new hypercalls.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  xen/common/device_tree.c      |   10 +++++-----
>>  xen/include/xen/device_tree.h |    7 ++++---
>>  2 files changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
>> index f0b17a3..4736e0d 100644
>> --- a/xen/common/device_tree.c
>> +++ b/xen/common/device_tree.c
>> @@ -876,7 +876,7 @@ static const struct dt_bus *dt_match_bus(const struct dt_device_node *np)
>>  }
>>  
>>  static const __be32 *dt_get_address(const struct dt_device_node *dev,
>> -                                    int index, u64 *size,
>> +                                    unsigned index, u64 *size,
> 
> It is the same thing but you might as well use unsigned int.

I forgot to add int here. I will do in the next version.

>>                                      unsigned int *flags)
>>  {
>>      const __be32 *prop;
>> @@ -1063,7 +1063,7 @@ bail:
>>  }
>>  
>>  /* dt_device_address - Translate device tree address and return it */
>> -int dt_device_get_address(const struct dt_device_node *dev, int index,
>> +int dt_device_get_address(const struct dt_device_node *dev, unsigned int index,
>>                            u64 *addr, u64 *size)
>>  {
>>      const __be32 *addrp;
>> @@ -1386,7 +1386,7 @@ fail:
>>      return -EINVAL;
>>  }
>>  
>> -int dt_device_get_raw_irq(const struct dt_device_node *device, int index,
>> +int dt_device_get_raw_irq(const struct dt_device_node *device, uint32_t index,
> 
> Why are you changing the other indexes to unsigned int and this one to
> uint32_t?

I don't remember. I will change it to unsigned int.

>>                            struct dt_raw_irq *out_irq)
>>  {
>>      const struct dt_device_node *p;
>> @@ -1394,7 +1394,7 @@ int dt_device_get_raw_irq(const struct dt_device_node *device, int index,
>>      u32 intsize, intlen;
>>      int res = -EINVAL;
>>  
>> -    dt_dprintk("dt_device_get_raw_irq: dev=%s, index=%d\n",
>> +    dt_dprintk("dt_device_get_raw_irq: dev=%s, index=%u\n",
>>                 device->full_name, index);
>>  
>>      /* Get the interrupts property */
>> @@ -1445,7 +1445,7 @@ int dt_irq_translate(const struct dt_raw_irq *raw,
>>                          &out_irq->irq, &out_irq->type);
>>  }
>>  
>> -int dt_device_get_irq(const struct dt_device_node *device, int index,
>> +int dt_device_get_irq(const struct dt_device_node *device, uint32_t index,
> 
> ditto

Same here :/.

Regards,