netdev - Re: [PATCH 1/2] arm: ixp4xx: set cohorent_dma

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: <1374424823.2804.25.camel@deadeye.wl.decadent.org.uk>
Date:	Sun, 21 Jul 2013 17:40:23 +0100
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Krzysztof Halasa <khc@...waw.pl>
CC:	Jonas Gorski <jogo@...nwrt.org>, <linux-kernel@...r.kernel.org>,
	<netdev@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	"David S. Miller" <davem@...emloft.net>,
	Russell King <linux@....linux.org.uk>,
	"Imre Kaloz" <kaloz@...nwrt.org>
Subject: Re: [PATCH 1/2] arm: ixp4xx: set cohorent_dma_mask for ethernet
 platform devices

On Sun, 2013-07-21 at 16:45 +0200, Krzysztof Halasa wrote:
> Jonas Gorski <jogo@...nwrt.org> writes:
> 
> > ARM requires the cohorent_dma_mask set, so set it for the platform
> > devices so that the ethernet driver has access to it.
> 
> I recognize the need to fix this issue and I appreciate your efforts,
> but... I think this patch tries to make the driver functional again at
> all costs and this a very bad idea. The IXP4xx Ethernet MACs are not
> normal platform devices, they are in fact built-in CPU resources.

Sounds like a normal platform device to me...

> The
> platform device structs are only used to set parameters. What the patch
> does is unneeded and IMHO harmful code duplication. It makes completely
> no sense to set DMA masks in code for individual platforms as it's not
> something platforms can decide, or *even should know of*. It's simply
> a CPU attribute, a value that is shared by all IXP4xx CPUs and thus all
> platforms and systems using it.
> 
> This is against the "line of code" count rules (or "rules").
> 
> Also the dev->dev.parent is IMHO a bad idea. The queue numbers and MAC
> addresses are in no way "parents" of Ethernet controllers,

Well, those are the platform data.

> and even if
> they somehow were, I find it rather hard to believe they can have DMA
> masks.

I think the problem is that the platform device and the platform data
have not been properly separated.  The machine-specific setup functions
should be passing the eth_plat_info and MAC ID into a common ixp4xx
function which would then create the platform device with the correct
DMA mask etc.

Ben.

> I think the previous patch (which sets the masks in one place, in
> Ethernet driver code) was better, though not perfect.
> 
> My fault is I haven't fixed it yet. Will try to invent something.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html