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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4FB66796.1030007@wwwdotorg.org>
Date:	Fri, 18 May 2012 09:15:34 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Hiroshi DOYU <hdoyu@...dia.com>
CC:	joerg.roedel@....com, iommu@...ts.linux-foundation.org,
	linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org,
	Grant Likely <grant.likely@...retlab.ca>,
	Rob Herring <rob.herring@...xeda.com>,
	Thierry Reding <thierry.reding@...onic-design.de>,
	devicetree-discuss@...ts.ozlabs.org
Subject: Re: [PATCH 1/1] iommu/tegra: smmu: Add DMA window parser

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@...dia.com>

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@...dotorg.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/