linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Milton Miller <miltonm@bga.com>
To: Jean Delvare <khali@linux-fr.org>
Cc: linux-kernel@vger.kernel.org,
	"Hans de Goede" <j.w.r.degoede@hhs.nl>,
	"Samuel Ortiz" <samuel@sortiz.org>,
	lm-sensors@lm-sensors.org, linuxppc-dev@ozlabs.org,
	"David Hubbard" <david.c.hubbard@gmail.com>
Subject: Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
Date: Mon, 14 Jul 2008 12:09:03 -0500	[thread overview]
Message-ID: <88f5162604470179b3c6ebfe729a46f5@bga.com> (raw)
In-Reply-To: <20080714095914.0644ac5d@hyperion.delvare>

On Jul 14, 2008, at 2:59 AM, Jean Delvare wrote:
> On Sun, 13 Jul 2008 15:26:56 -0600, David Hubbard wrote:
>> Hi Hans,
>>
>>>> I propose writing a subsystem driver. (Is that properly called "The
>>>> SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will
>>>> put together some code and submit it for review, and maintain it.
>>>>
>>>> Some hwmon chips have odd / unique probe sequences. IMHO this is 
>>>> where
>>>> the design needs to be inspected. One of those is the w83627ehf, 
>>>> which
>>>> Jean and Hans are familiar enough with to check my work.
>>>>
>>>> Thoughts?
>>>
>>> I'm afraid that making this a "bus" will be a bit overkill. Jim's 
>>> patches
>>> are quite simple and seem todo the job.
>>>
>>> Also keep in mind that most users will be platform devices which 
>>> just want
>>> to use the superio registers to find out the baseaddress of their 
>>> logical
>>> device, a whole bus seems overkill for this, would the hwmon driver 
>>> then
>>> need to be a superio_driver (as well as an platform_driver) or can 
>>> the bus
>>> be queried / used
>>> without having to be a bustype-driver?
>>
>> I think that's a valid point. I am willing to keep it small, but I
>> would prefer to follow the pattern set in other subsystems. It may be
>> my lack of experience in designing a subsystem showing here! What are
>> some alternative ways to implement it?
>>
>> In other words, Jim's patches are a good start, but maybe I am
>> misunderstanding them. I see it as the superio-locks module, a driver
>> that other drivers would depend on / auto-load. Is that something
>> other subsystems also do?
>
> Well, there are two approaches to the problem. The first approach
> (which I think Jim took in his patches? I don't really remember) is to
> simply solve the problem of concurrent I/O access to the Super-I/O
> configuration ports (typically 0x2e/0x2f or 0x4e/0x4f). That would be a
> simple driver requesting the ports in question and exporting an API for
> other drivers to access them in a safe way.
>
> The second approach is to make it a whole subsystem, as David is
> suggesting. The Super-I/O driver would then not only request the I/O
> ports, it would also identify which Super-I/O is there, and it would
> create devices (in the Linux device driver model sense of the term) for
> every logical device we are interested in (amongst which the hwmon
> ones.) The hwmon drivers would have to be converted from platform
> drivers to superio drivers.
>
> Each approach has its advantages. The first one is rather simple and
> also very generic in nature. It could be reused for other purposes. The
> second one offers automatic loading of hwmon drivers, which would be
> nice to have.
>
> There's probably a middle way which would keep the simplicity of the
> first approach while still allowing for driver auto-loading, without
> changing the bus type of all drivers. It would probably take some
> research though.

I haven't done the research, but it might be keep superio as
a platform driver, and keep the clients as platform drivers.  Only
have the superio driver probe and discover the subcomponent
addresses and then create the platform devices as children
instead of having each driver create its own platform device.
(This all assumes they are all platform devices in sysfs, I have
not looked).

This is all because in the platform bus the bus driver does not
discover the addresses but relies on drivers or platform setup code.

> Me, I don't really care which path we choose, given that I am not the
> one who will take care of it. All I have to say is that this has been
> discussed several times over the past 2 years and nothing came out of
> it (as far as the mainline kernel is concerned, at least) so whatever
> is done now, what really matters is that it makes it into the kernel
> quickly. We have some drivers missing features because of this (for
> example real-time reading of VID pin values.)
>
> This should also not prevent us from fixing the drivers now so that
> they temporarily request the Super-I/O ports they are using, as Milton
> suggested. Not only we don't know when we will have something better to
> offer, but it might also help us find conflicts between drivers, so
> that we know which drivers should make use of the future superio
> driver. This could also reveal conflicts with PNP BIOS reservations,
> etc. Milton, will you write a patch?

Well, that is the second time you asked me, so I guess I should respond.

While I it is possible for me to write this patch, my schedule and
priority list predict it would not be before the merge window closes.  
In
fact, I'm not sure when it would come out.   It might be argued it could
go in early -rc, but I would fear somebody's chip will not be detected
with the additional check.  For example, the port may reserved as mother
board resources or something.  Also, I have no hardware to test any
proposed patch, so it would be compile check only.

milton


  reply	other threads:[~2008-07-14 17:08 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-10 21:12 [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region Milton Miller
2008-07-10 21:14 ` mtd: remove __PPC__ hardcoded address from nand/diskonchip and devices/docprobe Milton Miller
2008-07-10 21:33 ` [lm-sensors] [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region Hans de Goede
2008-07-10 21:51   ` Milton Miller
2008-07-11  6:52     ` Jean Delvare
2008-07-11  7:27       ` Hans de Goede
2008-07-11  7:36         ` Jean Delvare
2008-07-13  6:31           ` Hans de Goede
2008-07-13 21:11             ` [lm-sensors] " David Hubbard
2008-07-13 21:22               ` Hans de Goede
2008-07-13 21:26                 ` David Hubbard
2008-07-14  7:59                   ` Jean Delvare
2008-07-14 17:09                     ` Milton Miller [this message]
2008-07-14 17:30                       ` [lm-sensors] " Hans de Goede
2008-07-14 17:55                         ` David Hubbard
2008-07-15  8:36                           ` Jean Delvare
2008-07-15 15:31                             ` David Hubbard
2008-07-16  7:46                               ` Jean Delvare
2008-07-16  8:09                                 ` Rene Herman
2008-07-15  8:28                       ` Jean Delvare
     [not found] ` <for-27-patch9@bga.com>
2008-07-12 20:02   ` [PATCH/RESEND] pci: dynids.use_driver_data considered harmful Milton Miller
2008-07-12 20:17     ` Greg KH
2008-07-12 20:58       ` Jean Delvare
2008-07-12 21:17       ` Milton Miller
2008-07-12 21:29         ` Milton Miller
     [not found]   ` <20080712041137.GA5933@kroah.com>
2008-07-12 21:08     ` [PATCH/RFC] " Milton Miller
2008-07-12 22:48       ` Milton Miller
2008-07-16 10:18         ` Milton Miller
2008-07-17  7:07           ` Greg KH
2008-07-17 14:36             ` Milton Miller
2008-08-06  7:31             ` Jean Delvare
2008-08-14 22:12               ` Greg KH
2008-08-15 14:50                 ` Milton Miller
2008-08-15 15:50                 ` Jean Delvare
2008-08-15 17:46                   ` Jesse Barnes
2008-08-15 18:55                     ` Jean Delvare
2008-08-15 19:15                       ` Jesse Barnes
2008-08-16  6:22                         ` Greg KH
2008-08-17 19:06                           ` Jean Delvare
2008-08-18  3:50                             ` Greg KH
2008-08-18 17:13                             ` Jesse Barnes
2008-08-18 20:41                             ` Jesse Barnes
2008-08-19 18:01                             ` Milton Miller
2008-08-06  7:22           ` Jean Delvare

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=88f5162604470179b3c6ebfe729a46f5@bga.com \
    --to=miltonm@bga.com \
    --cc=david.c.hubbard@gmail.com \
    --cc=j.w.r.degoede@hhs.nl \
    --cc=khali@linux-fr.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=lm-sensors@lm-sensors.org \
    --cc=samuel@sortiz.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 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).