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

[Hiroshi Doyu]

Hi Stephen, Joerg,

Stephen Warren <swarren@xxxxxxxxxxxxx> wrote @ Fri, 18 May 2012 17:15:34 +0200:

> 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)?

Hm...the above code happens to work with our "dma-window" format right
now, but most likely this seems incmpatible with "ibm,dma-window"
format. I couldn't find the official format previously. I'm wondering
that those "(ibm,)dma-window" and "(ibm,)dma-range" may be able to be
generally used since those parameters are quite common to any
IOMMUs. I'll hold off those patches at least this time

Joerg, as Stephen proposed originally. Could you please revert or drop
the original one this time?

  [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

PS: I'm being off-line'd mostly next four weeks, and my response would
be delayed.
--
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/