From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752266Ab0CZAZp (ORCPT ); Thu, 25 Mar 2010 20:25:45 -0400 Received: from mga07.intel.com ([143.182.124.22]:36715 "EHLO azsmga101.ch.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751646Ab0CZAZn (ORCPT ); Thu, 25 Mar 2010 20:25:43 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.51,310,1267430400"; d="scan'208";a="258832810" Date: Fri, 26 Mar 2010 01:26:30 +0100 From: Samuel Ortiz To: "Ira W. Snyder" Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, socketcan-core@lists.berlios.de Subject: Re: [PATCH 1/3] mfd: add support for Janz CMOD-IO PCI MODULbus Carrier Board Message-ID: <20100326002629.GA31389@sortiz.org> References: <> <1268930324-29841-2-git-send-email-iws@ovro.caltech.edu> <20100319163849.GB30409@sortiz.org> <20100319182209.GD13672@ovro.caltech.edu> <20100325225929.GA20618@sortiz.org> <20100325232244.GG10454@ovro.caltech.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100325232244.GG10454@ovro.caltech.edu> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 25, 2010 at 04:22:44PM -0700, Ira W. Snyder wrote: > On Thu, Mar 25, 2010 at 11:59:30PM +0100, Samuel Ortiz wrote: > > Hi Ira, > > > > First of all, sorry for the late reply. Then my answers: > > > > On Fri, Mar 19, 2010 at 11:22:09AM -0700, Ira W. Snyder wrote: > > > > > > > > +/* > > > > > + * Subdevice Support > > > > > + */ > > > > Please use the mfd-core API for building and registering platform sub devices. > > > > The pieces of code below should shrink significantly. > > > > > > > > > > Using this framework, how is it possible to create the devices that I > > > do down below. For each subdevice, I need three resources: > > > > > > 1) MODULbus registers -- PCI BAR3 + (0x200 * module_num) > > > 2) PLX Control Registers -- PCI BAR4 > > > 3) IRQ > > > > > > Specifically, the way IORESOURCE_MEM resources are copied seems wrong. > > > They start at the base address of only one resource and use the offsets > > > provided in the struct mfd_cell. See the if-statement at > > > drivers/mfd/mfd-core.c line 48. > > > > > > I need two use two different parent resources. The mfd_add_devices() > > > function doesn't support this. > > I would still like you to use the mfd-core API. Here is my proposal: > > > > 1) I modify mfd_add_device() to support a NULL mem_base argument. When > > mem_base is NULL, we would have: > > > > res[r].parent = NULL and res[r].start = cell->resources[r].start; > > > > The platform code will use iomem_resource as the parent for this resource. > > > > I don't know the implications of using iomem_resource as the parent > resource. If you think it is ok, I have no objections. > > If it helps, I can provide the PCI resource as a parent resource in my > resources. Then, when mem_base is NULL, the mfd-core could do this: > > res[r].parent = cell->resources[r].parent > > This is basically what I did in my patch. I used the PCI resource as the > parent of all child resources. I don't know if that is safe, but it > works. :) > > The mfd_add_device() function does this for IORESOURCE_IO resources. It > only tries to be smart for IORESOURCE_MEM and IORESOURCE_IRQ resources. I pushed an mfd-core change that basically falls back to the default resource copying when mem_base is NULL. That should allow you to use the API now. > > > > > +#define PCI_VENDOR_ID_JANZ 0x13c3 > > > > That probably belongs to pci_ids.h > > > > > > > > > > Should I add a patch to the series for this? > > Either that or merge the pci_ids.h changes with this patch. > > > > I guess it is a trivial enough change to merge with this patch. > > I'll wait for your patch to the mfd-core API before making changes and > sending out the next round of updates. Very nice. The above mentioned change in my for-next branch, commit 6802a325f541bbea3168cf61ba239443193e1f9a. Cheers, Samuel. -- Intel Open Source Technology Centre http://oss.intel.com/