LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andres Salomon <dilinger@queued.net>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Samuel Ortiz <sameo@linux.intel.com>,
	linux-kernel@vger.kernel.org,
	Peter Ujfalusi <peter.ujfalusi@nokia.com>,
	Liam Girdwood <lrg@slimlogic.co.uk>,
	Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.de>,
	Timur Tabi <timur@freescale.com>,
	linux-input@vger.kernel.org, alsa-devel@alsa-project.org
Subject: Re: [PATCH 12/19] twl4030: mfd_cell is now implicitly available to drivers
Date: Fri, 4 Feb 2011 18:39:13 -0800	[thread overview]
Message-ID: <20110204183913.318e6664@debxo> (raw)
In-Reply-To: <20110203093154.GC21591@opensource.wolfsonmicro.com>

On Thu, 3 Feb 2011 09:31:54 +0000
Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:

> On Wed, Feb 02, 2011 at 11:03:26PM -0800, Andres Salomon wrote:
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > > Then they are doing it incorrectly. One possible way is to have
> > > parent device carry relevant data in its drvdata and have
> > > children get it from there.
> 
> > I believe some drivers are even using the parent device already.
> > See drivers/leds/leds-mc13783.c, for example, whose parent device
> > drvdata is used to pass around a struct mc13783 to its children.
> > Sounds like a possibility, will need to look into it further.
> 
> That's the current best practice approach.

One of the main reasons to have the cell data in the platform
device rather than its parent device is because the parent device will
have the entire array of cells.  This isn't helpful when you want to
know specifically which cell's .enable hook to call.  One possibility
is to use the pdev->id field.  Currently, mfd-core allows drivers to
override this per-cell; depending upon how drivers make use of this,
I'm tempted to get rid of it from the mfd_cell struct and just always
have it be an index into the mfd_cell array (dictated by
mfd_add_devices).  Of course, wm831x-core/wm831x-dcdc does some pretty
weird stuff with ->id, so I still need to figure out if what it's doing
is really necessary.

I'm also struggling with a way to have cells automatically saved
by mfd-core. One (ugly) option would be to allow an mfd driver to set
drvdata, and mfd-core (in mfd_add_devices) would allocate a struct that
includes a pointer to drvdata as well as to the mfd_cells.  Drvdata
would be updated to point to this new struct instead.  In one scenario,
this requires evil macro redefining; in another, a weird API addition
to mfd_add_devices to pass the size of what's pointed to by the
parent's drvdata.  I don't like this option very much.

Another option is to require structs that mfd drivers assign to
drvdata to include a pointer to cells, but I'd really like a way to
enforce that with the compiler (BUILD_BUG_ON comes to mind).
It also runs into the problem of not knowing the type in drvdata. I'm
imagining something like:

#define mfd_get_cell(pdev, type) (((type *)platform_get_drvdata(pdev))->cell)

Even that would require the weird API of mfd_add_devices needing to be
passed the type of what's pointed to by the parent's drvdata.


  reply	other threads:[~2011-02-05  2: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
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 [this message]
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=20110204183913.318e6664@debxo \
    --to=dilinger@queued.net \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lrg@slimlogic.co.uk \
    --cc=perex@perex.cz \
    --cc=peter.ujfalusi@nokia.com \
    --cc=sameo@linux.intel.com \
    --cc=timur@freescale.com \
    --cc=tiwai@suse.de \
    --subject='Re: [PATCH 12/19] twl4030: 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).