LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Samuel Ortiz <sameo@linux.intel.com>
To: Felipe Balbi <balbi@ti.com>
Cc: Greg KH <gregkh@suse.de>,
	Grant Likely <grant.likely@secretlab.ca>,
	Andres Salomon <dilinger@queued.net>,
	linux-kernel@vger.kernel.org,
	Mark Brown <broonie@opensource.wolfsonmicro.com>,
	khali@linux-fr.org, ben-linux@fluff.org,
	Peter Korsgaard <jacmet@sunsite.dk>,
	Mauro Carvalho Chehab <mchehab@infradead.org>,
	David Brownell <dbrownell@users.sourceforge.net>,
	linux-i2c@vger.kernel.org, linux-media@vger.kernel.org,
	netdev@vger.kernel.org, spi-devel-general@lists.sourceforge.net,
	Mocean Laboratories <info@mocean-labs.com>
Subject: Re: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers
Date: Thu, 7 Apr 2011 15:40:23 +0200	[thread overview]
Message-ID: <20110407133717.GA3923@sortiz-mobl> (raw)
In-Reply-To: <20110406185902.GN25654@legolas.emea.dhcp.ti.com>

Hi Felipe,

On Wed, Apr 06, 2011 at 09:59:02PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Apr 06, 2011 at 08:47:34PM +0200, Samuel Ortiz wrote:
> > > > > What is a "MFD cell pointer" and why is it needed in struct device?
> > > > An MFD cell is an MFD instantiated device.
> > > > MFD (Multi Function Device) drivers instantiate platform devices. Those
> > > > devices drivers sometimes need a platform data pointer, sometimes an MFD
> > > > specific pointer, and sometimes both. Also, some of those drivers have been
> > > > implemented as MFD sub drivers, while others know nothing about MFD and just
> > > > expect a plain platform_data pointer.
> > > 
> > > That sounds like a bug in those drivers, why not fix them to properly
> > > pass in the correct pointer?
> > Because they're drivers for generic IPs, not MFD ones. By forcing them to use
> > MFD specific structure and APIs, we make it more difficult for platform code
> > to instantiate them.
> 
> I agree. What I do on those cases is to have a simple platform_device
> for the core IP driver and use platform_device_id tables to do runtime
> checks of the small differences. If one platform X doesn't use a
> platform_bus, it uses e.g. PCI, then you make a PCI "bridge" which
> allocates a platform_device with the correct name and adds that to the
> driver model.
I see, thanks.
Below is a patch for the Xilinx SPI example. Although this would fix the
issue, we'd still have to do that on device per device basis. I had a similar
solution where MFD drivers would set a flag for sub drivers that don't need
any of the MFD bits. In that case the MFD core code would just forward the
platform data, instead of embedding it through an MFD cell.

Cheers,
Samuel.

---
 drivers/mfd/timberdale.c |    8 ++++----
 drivers/spi/xilinx_spi.c |   19 ++++++++++++++++++-
 include/linux/mfd/core.h |    3 +++
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
index 94c6c8a..c9220ce 100644
--- a/drivers/mfd/timberdale.c
+++ b/drivers/mfd/timberdale.c
@@ -416,7 +416,7 @@ static __devinitdata struct mfd_cell timberdale_cells_bar0_cfg0[] = {
 		.mfd_data = &timberdale_radio_platform_data,
 	},
 	{
-		.name = "xilinx_spi",
+		.name = "mfd_xilinx_spi",
 		.num_resources = ARRAY_SIZE(timberdale_spi_resources),
 		.resources = timberdale_spi_resources,
 		.mfd_data = &timberdale_xspi_platform_data,
@@ -476,7 +476,7 @@ static __devinitdata struct mfd_cell timberdale_cells_bar0_cfg1[] = {
 		.mfd_data = &timberdale_radio_platform_data,
 	},
 	{
-		.name = "xilinx_spi",
+		.name = "mfd_xilinx_spi",
 		.num_resources = ARRAY_SIZE(timberdale_spi_resources),
 		.resources = timberdale_spi_resources,
 		.mfd_data = &timberdale_xspi_platform_data,
@@ -526,7 +526,7 @@ static __devinitdata struct mfd_cell timberdale_cells_bar0_cfg2[] = {
 		.mfd_data = &timberdale_radio_platform_data,
 	},
 	{
-		.name = "xilinx_spi",
+		.name = "mfd_xilinx_spi",
 		.num_resources = ARRAY_SIZE(timberdale_spi_resources),
 		.resources = timberdale_spi_resources,
 		.mfd_data = &timberdale_xspi_platform_data,
@@ -570,7 +570,7 @@ static __devinitdata struct mfd_cell timberdale_cells_bar0_cfg3[] = {
 		.mfd_data = &timberdale_radio_platform_data,
 	},
 	{
-		.name = "xilinx_spi",
+		.name = "mfd_xilinx_spi",
 		.num_resources = ARRAY_SIZE(timberdale_spi_resources),
 		.resources = timberdale_spi_resources,
 		.mfd_data = &timberdale_xspi_platform_data,
diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index c69c6f2..3287b84 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -471,7 +471,11 @@ static int __devinit xilinx_spi_probe(struct platform_device *dev)
 	struct spi_master *master;
 	u8 i;
 
-	pdata = mfd_get_data(dev);
+	if (platform_get_device_id(dev) &&
+	    platform_get_device_id(dev)->driver_data & MFD_PLATFORM_DEVICE)
+		pdata = mfd_get_data(dev);
+	else
+		pdata = dev->dev.platform_data;
 	if (pdata) {
 		num_cs = pdata->num_chipselect;
 		little_endian = pdata->little_endian;
@@ -530,6 +534,18 @@ static int __devexit xilinx_spi_remove(struct platform_device *dev)
 /* work with hotplug and coldplug */
 MODULE_ALIAS("platform:" XILINX_SPI_NAME);
 
+static const struct platform_device_id xilinx_spi_id_table[] = {
+	{
+		.name	= XILINX_SPI_NAME,
+	},
+	{
+		.name	= "mfd_xilinx_spi",
+		.driver_data = MFD_PLATFORM_DEVICE,
+	},
+	{  },	/* Terminating Entry */
+};
+MODULE_DEVICE_TABLE(platform, xilinx_spi_id_table);
+
 static struct platform_driver xilinx_spi_driver = {
 	.probe = xilinx_spi_probe,
 	.remove = __devexit_p(xilinx_spi_remove),
@@ -538,6 +554,7 @@ static struct platform_driver xilinx_spi_driver = {
 		.owner = THIS_MODULE,
 		.of_match_table = xilinx_spi_of_match,
 	},
+	.id_table	= xilinx_spi_id_table,
 };
 
 static int __init xilinx_spi_pltfm_init(void)
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index ad1b19a..13f31f4 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -89,6 +89,9 @@ static inline const struct mfd_cell *mfd_get_cell(struct platform_device *pdev)
 	return pdev->dev.platform_data;
 }
 
+/* */
+#define MFD_PLATFORM_DEVICE BIT(0)
+
 /*
  * Given a platform device that's been created by mfd_add_devices(), fetch
  * the .mfd_data entry from the mfd_cell that created it.


-- 
Intel Open Source Technology Centre
http://oss.intel.com/

  parent reply	other threads:[~2011-04-07 13:40 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-03  3:54 [RFC] [PATCH 0/19] mfd sharing support Andres Salomon
2011-02-03  3:55 ` [PATCH 01/19] mfd-core: unconditionally add mfd_cell to every platform_device Andres Salomon
2011-02-03  3:58 ` [PATCH 02/19] jz4740: mfd_cell is now implicitly available to drivers Andres Salomon
2011-02-03  8:09   ` Jean Delvare
2011-02-03  4:01 ` [PATCH 03/19] ab3550: " Andres Salomon
2011-02-04  8:20   ` Mattias Wallin
2011-02-03  4:03 ` [PATCH 04/19] ab3100: " Andres Salomon
2011-02-03 12:52   ` Linus Walleij
2011-02-03  4:04 ` [PATCH 05/19] asic3: " Andres Salomon
2011-02-03  4:05 ` [PATCH 06/19] htc-pasic3: " Andres Salomon
2011-02-03  4:08 ` [PATCH 07/19] timberdale: " Andres Salomon
2011-03-31 23:05   ` Grant Likely
2011-04-01 11:20     ` Samuel Ortiz
2011-04-01 17:47       ` Andres Salomon
2011-04-01 17:56         ` Grant Likely
2011-04-01 18:00           ` Grant Likely
2011-04-01 23:52           ` Samuel Ortiz
2011-04-01 23:58             ` Grant Likely
2011-04-02  0:10               ` Andres Salomon
2011-04-04 10:03               ` Samuel Ortiz
2011-04-05  3:04                 ` Grant Likely
2011-04-06 15:23                   ` Samuel Ortiz
2011-04-06 15:58                     ` Greg KH
2011-04-06 17:05                       ` Samuel Ortiz
2011-04-06 17:16                         ` Ben Hutchings
2011-04-06 17:51                           ` Samuel Ortiz
2011-04-06 18:07                             ` Ben Hutchings
2011-04-06 17:56                         ` Greg KH
2011-04-06 18:25                           ` Andres Salomon
2011-04-06 18:38                             ` Greg KH
2011-04-07  8:04                               ` Grant Likely
2011-04-06 18:47                           ` Samuel Ortiz
2011-04-06 18:59                             ` Felipe Balbi
2011-04-06 22:09                               ` Greg KH
2011-04-07  8:09                                 ` Felipe Balbi
2011-04-07 13:40                               ` Samuel Ortiz [this message]
2011-04-07 14:35                                 ` Grant Likely
2011-04-07 15:03                                   ` Samuel Ortiz
2011-04-07 18:06                                     ` Grant Likely
2011-04-07 16:24                   ` Grant Likely
2011-04-01 18:26         ` Samuel Ortiz
2011-02-03  4:09 ` [PATCH 08/19] t7166xb: " Andres Salomon
2011-02-03  4:11 ` [PATCH 09/19] wl1273: " Andres Salomon
2011-02-03  4:12 ` [PATCH 10/19] sh_mobile_sdhi: " Andres Salomon
2011-02-03  4:13 ` [PATCH 11/19] tc6393xb: " Andres Salomon
2011-02-03  4:15 ` [PATCH 12/19] twl4030: " Andres Salomon
2011-02-03  6:05   ` Dmitry Torokhov
2011-02-03  6:39     ` Andres Salomon
2011-02-03  6:53       ` Dmitry Torokhov
2011-02-03  7:03         ` Andres Salomon
2011-02-03  9:31           ` Mark Brown
2011-02-05  2:39             ` Andres Salomon
2011-02-05  3:25               ` Andres Salomon
2011-02-03 12:23           ` Peter Ujfalusi
2011-02-04 10:41           ` Uwe Kleine-König
2011-02-03  4:16 ` [PATCH 13/19] tc6387xb: " Andres Salomon
2011-02-03  4:17 ` [PATCH 14/19] janz: " Andres Salomon
2011-02-03  4:20 ` [PATCH 15/19] mc13xxx: " Andres Salomon
2011-02-04  9:34   ` Uwe Kleine-König
2011-02-04 10:13     ` Uwe Kleine-König
2011-02-04 10:16     ` Andres Salomon
2011-02-03  4:21 ` [PATCH 16/19] mfd-core: drop platform_data/data_size from core mfd_cell struct Andres Salomon
2011-02-03  4:22 ` [PATCH 17/19] mfd-core: add refcounting support to mfd_cells Andres Salomon
2011-02-03  4:23 ` [PATCH 18/19] mfd-core: add platform_device sharing support for mfd Andres Salomon
2011-02-03  4:26 ` [PATCH 19/19] cs5535-mfd: add sharing for acpi/pms cells Andres Salomon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110407133717.GA3923@sortiz-mobl \
    --to=sameo@linux.intel.com \
    --cc=balbi@ti.com \
    --cc=ben-linux@fluff.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=dbrownell@users.sourceforge.net \
    --cc=dilinger@queued.net \
    --cc=grant.likely@secretlab.ca \
    --cc=gregkh@suse.de \
    --cc=info@mocean-labs.com \
    --cc=jacmet@sunsite.dk \
    --cc=khali@linux-fr.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=netdev@vger.kernel.org \
    --cc=spi-devel-general@lists.sourceforge.net \
    --subject='Re: [PATCH 07/19] timberdale: mfd_cell is now implicitly available to drivers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).