Re: [PATCH 1/1] iommu/tegra: smmu: Add DMA window parser

From: Stephen Warren
Date: Fri May 18 2012 - 11:15:38 EST


On 05/18/2012 12:43 AM, Hiroshi DOYU wrote:
> This code was based on:
> "arch/microblaze/kernel/prom_parse.c"
> "arch/powerpc/kernel/prom_parse.c"
>
> Can be promoted as a global function for general use.
>
> Signed-off-by: Hiroshi DOYU <hdoyu@xxxxxxxxxx>

Hiroshi,

Once this is accepted, it might be worth following up on the original
of_get_dma_window thread and noting that once it's approved, an updated
version of the patch will be needed to remove this code at the same time.

A couple comments below though:

> +static int of_get_dma_window(struct device_node *dn,
> + const char *propname, int index,
> + unsigned long *busno,
> + dma_addr_t *addr, size_t *size)
> +{
> + const __be32 *dma_window, *end;
> + int bytes, cur_index = 0;

Since bytes is an out parameter from of_get_property, do you need to
wrap the declaration in uninitialized_var() to prevent a compile warning
like the other patch you sent me downstream?

> + if (!dn || !addr || !size)
> + return -EINVAL;
> +
> + if (!propname)
> + propname = "dma-window";
> +
> + dma_window = of_get_property(dn, propname, &bytes);
> + if (!dma_window)
> + return -ENODEV;
> + end = dma_window + bytes / sizeof(*dma_window);
> +
> + while (dma_window < end) {
> + u32 cells;
> + const void *prop;
> +
> + /* busno is always one cell */
> + if (busno)
> + *busno = be32_to_cpup(dma_window++);

Shouldn't the ++ happen even if (!busno)?

Once those are fixed, in the interests of fixing the compile error,

Acked-by: Stephen Warren <swarren@xxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/