Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755939Ab3GUQka (ORCPT ); Sun, 21 Jul 2013 12:40:30 -0400 Received: from webmail.solarflare.com ([12.187.104.25]:26390 "EHLO webmail.solarflare.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755868Ab3GUQk2 (ORCPT ); Sun, 21 Jul 2013 12:40:28 -0400 Message-ID: <1374424823.2804.25.camel@deadeye.wl.decadent.org.uk> Subject: Re: [PATCH 1/2] arm: ixp4xx: set cohorent_dma_mask for ethernet platform devices From: Ben Hutchings To: Krzysztof Halasa CC: Jonas Gorski , , , , "David S. Miller" , Russell King , "Imre Kaloz" Date: Sun, 21 Jul 2013 17:40:23 +0100 In-Reply-To: References: <1374315339-31469-1-git-send-email-jogo@openwrt.org> <1374315339-31469-2-git-send-email-jogo@openwrt.org> Organization: Solarflare Communications Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.4.4-3 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Originating-IP: [88.96.1.126] X-TM-AS-Product-Ver: SMEX-10.0.0.1412-7.000.1014-20028.005 X-TM-AS-Result: No--30.556500-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2238 Lines: 55 On Sun, 2013-07-21 at 16:45 +0200, Krzysztof Halasa wrote: > Jonas Gorski 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 linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/