All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wim Van Sebroeck <wim@iguana.be>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "Samuel Ortiz" <sameo@linux.intel.com>,
	linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Pádraig Brady" <P@draigBrady.com>
Subject: Re: mfd: Core driver for Winbond chips
Date: Tue, 9 Apr 2013 19:31:15 +0200	[thread overview]
Message-ID: <20130409173115.GF7867@spo001.leaseweb.com> (raw)
In-Reply-To: <20130409161838.GB27050@roeck-us.net>

Hi Guenter,

> > > I was waiting for feedback from Wim, who submitted a similar driver, about his
> > > thoughts. Key question is how to reserve access to the shared resource - either
> > > through an exported function in the mfd driver requesting a mutex, or through
> > > request_muxed_region(). I am going back and forth myself on which one is better.
> > > 
> > > Maybe it does not really matter, but using a function has the slight advantage
> > > that it auto-loads and locks the mfd module while one of its client modules
> > > is loaded. If we use request_muxed_region, that is not the case and the client
> > > module must use another means to request and lock the mfd module.
> > > 
> > > Maybe you have an opinion ?
> > 
> > This is indeed the main issue that has to be solved. Both options will work.
> > I like the auto-load and lock, but I need to look at the request_muxed_region
> > code again first before I can see what the possible drawbacks are :-).
> > 
> One drawback of using request_muxed_region is that it needs a return value
> from superio_enter. Also, it needs some code in the client driver init function
> to ensure that the mfd driver gets loaded, and possibly a call to __module_get()
> in the client driver probe function to keep the mfd driver loaded.
> 
> winbond_superio_enter() would not need a return value and could use
> devm_request_region. We could also consider allocating the hwmon memory space in
> the mfd driver and pass it as resource to the client drivers, which would remove
> a few more lines of code from those.
> 
> Overall I am slightly in favor of using an exported function.

I looked at commit 8b6d043b7ee2d1b819dc833d677ea2aead71a0c0 (which implements
request_muxed_region). You indeed need some extra code for loading the lowl-level
mfd driver. So I am also in favour of the exported function.

Kind regards,
Wim.


  reply	other threads:[~2013-04-09 17:31 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-10 23:14 [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure Guenter Roeck
2013-03-10 23:14 ` [PATCH v2 1/7] " Guenter Roeck
2013-03-10 23:14   ` Guenter Roeck
2013-03-10 23:14 ` [PATCH v2 2/7] watchdog: w83627hf: Enable watchdog only once Guenter Roeck
2013-03-19 17:26   ` Pádraig Brady
2013-03-19 17:26     ` Pádraig Brady
2013-03-19 20:02     ` Guenter Roeck
2013-03-19 20:02       ` Guenter Roeck
2013-03-21 18:40       ` Pádraig Brady
2013-03-21 18:40         ` Pádraig Brady
2013-03-10 23:14 ` [PATCH v2 3/7] watchdog: w83627hf: Enable watchdog device only if not already enabled Guenter Roeck
2013-03-10 23:14 ` [PATCH v2 4/7] watchdog: w83627hf: Use helper functions to access superio registers Guenter Roeck
2013-03-10 23:14 ` [PATCH v2 5/7] watchdog: w83627hf: Auto-detect IO address and supported chips Guenter Roeck
2013-03-10 23:14 ` [PATCH v2 6/7] watchdog: w83627hf: Add support for W83697HF and W83697UG Guenter Roeck
2013-03-10 23:15 ` [PATCH v2 7/7] watchdog: Remove drivers " Guenter Roeck
2013-03-10 23:15   ` Guenter Roeck
2013-03-22 20:52 ` [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure Wim Van Sebroeck
2013-03-22 21:09   ` [RFC] winbond Super-I/O MFD driver Wim Van Sebroeck
2013-03-22 21:09     ` Wim Van Sebroeck
2013-03-23  0:28   ` [PATCH v2 0/8] watchdog: w83627hf: Convert to watchdog infrastructure Guenter Roeck
2013-03-23 12:57     ` Wim Van Sebroeck
2013-03-23 15:01       ` Guenter Roeck
2013-03-23 15:15       ` mfd: Core driver for W836{2389}7[T]HF Guenter Roeck
2013-03-23 17:49     ` mfd: Core driver for Winbond chips Guenter Roeck
2013-03-24  2:39       ` Guenter Roeck
2013-04-09  9:37       ` Samuel Ortiz
2013-04-09 11:36         ` Guenter Roeck
2013-04-09 11:45           ` Wim Van Sebroeck
2013-04-09 16:18             ` Guenter Roeck
2013-04-09 17:31               ` Wim Van Sebroeck [this message]
2013-04-10  0:36                 ` Guenter Roeck
2013-04-10 20:59                   ` Wim Van Sebroeck
2013-04-29 15:00                     ` Guenter Roeck
2013-04-09 11:37         ` Wim Van Sebroeck

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=20130409173115.GF7867@spo001.leaseweb.com \
    --to=wim@iguana.be \
    --cc=P@draigBrady.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=sameo@linux.intel.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.