All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: Jean Delvare <khali@linux-fr.org>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] gpio: New driver for the Intel 82801 (ICH) GPIO pins
Date: Tue, 19 Apr 2011 15:54:06 +0100	[thread overview]
Message-ID: <20110419155406.1da0e7fb@lxorguk.ukuu.org.uk> (raw)
In-Reply-To: <BANLkTinbO_p=sKtFJtjgpo4yrEVopq9vrw@mail.gmail.com>

> I'm not at all concerned about this.  Users *must* know what a pin is
> when they start fiddling with it.  If you ask a pin to do something
> that it cannot do, then it just isn't going to work.  There's no real
> point in trying to enforce any behaviour in the API.

Disagree 8) - errors are better than silently and undetectably failing.
That might not be an error code but ideally it would WARN() or
similar so that errors get caught and fixed and you don't instead find
out something was poking the wrong pin and worked by chance a year later.

> In this particular case, you've got a PCI device which looks to be
> going into config space to get some information about how the chip is
> layed out.  What I would do is keep your existing pci probe & remove
> hooks, but use them to create and register child basic_mmio_gpio
> platform_devices for each gpio bank.

That means more devices, weird sysfs heirarchies and lots of mess with
runtime power management in the general case.

If the basic_mmio_gpio code is properly abstracted it ought not to need
magic extra platform devices.

That aside it'll be less code to keep it separate than do all the messing
around with platform devices so it seems to be a gross overdoing of 'lets
make everything use the same code however big a hammer is needed'

If it was a platform device it might make sense, if the config was
platform config to create a device it would certainly make sense, in the
PCI case it's smaller, cleaner and saner not to add insane layers of glue
and indirection IMHO.

Alan

  reply	other threads:[~2011-04-19 14:53 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-19 12:53 [PATCH] gpio: New driver for the Intel 82801 (ICH) GPIO pins Jean Delvare
2011-04-19 14:44 ` Grant Likely
2011-04-19 14:54   ` Alan Cox [this message]
2011-04-19 15:05     ` Grant Likely
2011-04-19 15:57       ` Alan Cox
2011-04-19 16:40         ` Anton Vorontsov
2011-04-19 17:08           ` Alan Cox
2011-04-19 20:30             ` Anton Vorontsov
2011-04-19 21:16               ` Alan Cox
2011-04-19 21:20                 ` Alan Cox
2011-04-23 13:45   ` Jean Delvare
2011-04-23 14:47     ` Alan Cox
2011-05-19 11:33       ` Jean Delvare
2011-05-27  3:09 ` Grant Likely
2012-02-02  2:31 ` Guenter Roeck
2012-02-02  7:49   ` Jean Delvare
2012-02-02 17:35     ` Guenter Roeck
2012-02-02 19:56       ` Peter Tyser
2012-02-02 22:02         ` Guenter Roeck
2012-02-02 23:25           ` [PATCH 1/3] mfd: Add LPC driver for Intel ICH chipsets Aaron Sierra
2012-02-03  6:43             ` Guenter Roeck
2012-02-03 15:34               ` Aaron Sierra
2012-02-03 19:14             ` Guenter Roeck
2012-02-03 19:35               ` Aaron Sierra
2012-02-03 19:45                 ` Guenter Roeck
2012-02-03 22:50                   ` Aaron Sierra
2012-02-04  8:45                     ` Jean Delvare
2012-02-04 16:45                       ` Guenter Roeck
2012-02-07 19:56                         ` [PATCH 1/3 v2] " Aaron Sierra
2012-02-07 20:15                           ` Guenter Roeck
2012-02-07 20:31                             ` Jean Delvare
2012-02-07 20:43                               ` Guenter Roeck
2012-02-07 21:00                             ` Aaron Sierra
2012-02-07 21:09                               ` Guenter Roeck
2012-02-02 23:27           ` [PATCH 2/3] gpio: Add support for Intel ICHx/3100/Series[56] GPIO Aaron Sierra
2012-02-03 20:19             ` Guenter Roeck
2012-02-07 19:58               ` [PATCH 2/3 v2] " Aaron Sierra
2012-02-07 20:42                 ` Guenter Roeck
2012-02-07 22:07                 ` Jean Delvare
2012-02-07 23:25                   ` Aaron Sierra
2012-02-02 23:29           ` [PATCH 3/3] watchdog: Convert iTCO_wdt driver to mfd model Aaron Sierra
2012-02-07 19:59             ` [PATCH 3/3 v2] " Aaron Sierra
2012-02-07 21:07               ` Guenter Roeck
2012-02-08 17:48                 ` Aaron Sierra

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=20110419155406.1da0e7fb@lxorguk.ukuu.org.uk \
    --to=alan@lxorguk.ukuu.org.uk \
    --cc=grant.likely@secretlab.ca \
    --cc=khali@linux-fr.org \
    --cc=linux-kernel@vger.kernel.org \
    /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.