Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Simon Guinot]

[-- Attachment #1: Type: text/plain, Size: 5355 bytes --]

On Thu, Aug 01, 2013 at 08:52:32AM -0700, Guenter Roeck wrote:
> On 08/01/2013 06:46 AM, Simon Guinot wrote:
> >On Mon, Jul 29, 2013 at 05:59:08PM +0200, Linus Walleij wrote:
> >>On Mon, Jul 22, 2013 at 11:50 AM, Simon Guinot
> >><simon.guinot@sequanux.org> wrote:
> >>
> >>>This patch adds support for the GPIOs found on the Fintek super-I/O
> >>>chips F71882FG and F71889F.
> >>>
> >>>A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
> >>>is used to connect the low-bandwidth devices. Among others functions the
> >>>F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
> >>>controller, an hardware monitoring controller and some GPIO pins.
> >>>
> >>>Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
> >>>The GPIOs are used to control the LEDs and the hard drive power.
> >>>
> >>>Signed-off-by: Simon Guinot <simon.guinot@sequanux.org>
> >>>---
> >>>Changes since v2:
> >>>- Remove useless NULL setters for driver data.
> >
> >Hi Linus,
> >
> >>
> >>Given the recent discussion with Rafael I want to have an
> >>extended discussion of this patch.
> >>
> >>It is my current understanding that:
> >>
> >>- It is possible to define the whereabouts of the SuperIO
> >>   chips using ACPI
> >
> >Agreed.
> >
> >>- It is possible for developers to influence the source
> >>   AML for the DSDT tables of these systems.
> >
> >I am not sure about that. Let's consider the LaCie x86-based boards.
> >LaCie only adds a few devices on the top of a motherboard provided by
> >an another manufacturer. In turns, this last gets a Super-I/O from an
> >another manufacturer. In my understanding, the Super-I/O manufacturer is
> >responsible for registering the PNP IDs (one per device functionality).
> >
> >LaCie may have enough leverage to obtain some modifications on the ACPI
> >DSDT tables but about the PNP IDs registration, let's say it is less
> >that certain. The problem is that LaCie don't have any contacts with the
> >Super-I/O manufacturer.
> >
> >I have to say that all this process is not as easy as adding a node in
> >a dts file.
> >
> >>- It is the proper thing to do.
> >
> >Yes, it may be.
> >
> >>- So we should atleast support ACPI probing with the
> >>   port-based detection as a final fallback if all else fails.
> >>
> >>Why can I not get something like:
> >>
> >>#include <linux/acpi.h>
> >>(...)
> >>static const struct acpi_device_id gpio_acpi_match[] = {
> >>         { "FOOBAR", 0 },
> >
> >After some checks on my boards, it appears that there is no PNP ID
> >available for the Super-I/O GPIO functionality (or any others). Moreover
> >I think this IDs don't have been registered to Microsoft by Fintech
> >(the super-I/O manufacturer).
> >
> >How do you envisage the follow-up ?
> >
> >>         { }
> >>};
> >>MODULE_DEVICE_TABLE(acpi, gpio_acpi_match);
> >>
> >>static struct platform_driver gpio_driver = {
> >>         (...)
> >>         .driver         = {
> >>                  (...)
> >>                 .acpi_match_table = ACPI_PTR(gpio_acpi_match),
> >>         },
> >>};
> >
> >It seems to me that the ACPI probing is the easiest part. How do you see
> >the ioport probing fallback ?
> >
> >I can only figure out broken solutions:
> >
> >1. From the init function, we could check that the PNP IDs are well
> >    available in the ACPI DSDT tables before registering the platform
> >    driver. If not, we could fall back on the ioport probing method.
> >    I don't know if checking the DSDT tables is even possible. It is at
> >    least weird and it defeats completely the purpos of acpi_match_table.
> >2. In a late initcall, we could check that the driver is well
> >    registered else fall back on the ioport detection.
> >    As GPIOs may be needed early, I don't think this method is suitable.
> >
> >And I have no more ideas...
> >
> 3. Implement Super-IO detection in the the ACPI platform driver.
>    If there is no ACPI device entry for a detected Super-IO chip's sub-function(s),
>    fake it and create the respective platform device(s).
> 
> Just as kludgy as your proposed solutions, but at least it would move Super-IO detection
> to one file and let all Super-IO drivers use the ACPI match table.
> 
> Drawbacks:
> - Only works for x86 (ie it would limit SuperIO drivers to x86). Not sure if that is
>   a real limitation - are SuperIO chips used on other platforms ?
> - It would require us to define fake PNP IDs for the various SuperIO functions.
> - It may fail if a firmware / chip vendor ever adds real PNP IDs for the various
>   sub-functions and those start showing up in ACPI tables (maybe that doesn't matter
>   as much as the drivers would have to be updated anyway to match the real IDs).

As an additional drawback, this solution involves systematic I/O port
probing from the ACPI platform driver, even if the platform don't need
it. This may increase the conflicts risk in comparison with probes from
some end drivers (which are more likely built as modules and not loaded
automatically). Except that, moving all this Super-I/O detection in a
same place would be indeed a great benefit.

Linus, how do you see the follow-up for the gpio-f7188x driver ? Let me
know your advice.

Thanks,

Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Linus Walleij]

On Thu, Aug 1, 2013 at 3:46 PM, Simon Guinot <simon.guinot@sequanux.org> wrote:
> On Mon, Jul 29, 2013 at 05:59:08PM +0200, Linus Walleij wrote:

>> - So we should atleast support ACPI probing with the
>>   port-based detection as a final fallback if all else fails.
>>
>> Why can I not get something like:
>>
>> #include <linux/acpi.h>
>> (...)
>> static const struct acpi_device_id gpio_acpi_match[] = {
>>         { "FOOBAR", 0 },
>
> After some checks on my boards, it appears that there is no PNP ID
> available for the Super-I/O GPIO functionality (or any others). Moreover
> I think this IDs don't have been registered to Microsoft by Fintech
> (the super-I/O manufacturer).
>
> How do you envisage the follow-up ?

Well shall we just apply this as-is then, with ISA-style port
IO probing and all?

I think Rafael said something about it being possible for us
to register our own kernel ACPI PNP IDs (as if: there is no
road here, but if someone starts to walk here, a road will
soon become, and we take the first step then).

But overall I am a bit confused: I am hearing from one end
of the x86 community that ACPI is the way to go for
configuring platform devices on x86, yet stuff like this is
popping up from independent vendors, and get integrated
on boards with no ACPI tables in sight.

Over at ksummit-discuss we have had a thread about
whether device tree should be used in such cases, but
that is not clear either.

Basically I'm a bit confused because the x86 community
is talking with so many voices and I'm not used to it,
and I don't know if they actually have a common vision.

So I'm cc:ing some of my x86 friends to see if they
are OK with this port-probing approach before I merge
the driver.... Rafael, HPA, Matthew?

Yours,
Linus Walleij

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Matthew Garrett]

On Thu, Aug 29, 2013 at 02:39:33PM +0200, Linus Walleij wrote:

> I think Rafael said something about it being possible for us
> to register our own kernel ACPI PNP IDs (as if: there is no
> road here, but if someone starts to walk here, a road will
> soon become, and we take the first step then).

It'd be straightforward to register the LNX PnP prefix and have someone 
take responsibility for assigning numbers, but really a generic vendor 
string should only be used when defining programming models rather than 
specific devices.

> But overall I am a bit confused: I am hearing from one end
> of the x86 community that ACPI is the way to go for
> configuring platform devices on x86, yet stuff like this is
> popping up from independent vendors, and get integrated
> on boards with no ACPI tables in sight.

ACPI is usually used to describe systems, and the normal ACPI way of 
handling GPIO devices is to expose the device at the other end of the 
GPIO lines and then provide AML for toggling the lines. Attaching an 
actual driver to the device would interfere with that, so nobody writes 
an actual driver.

> Over at ksummit-discuss we have had a thread about
> whether device tree should be used in such cases, but
> that is not clear either.

If a vendor doesn't provide any way to autoprobe a device, there's no 
way to autoprobe a device. That usually means that you're not expected 
to use that device.

> Basically I'm a bit confused because the x86 community
> is talking with so many voices and I'm not used to it,
> and I don't know if they actually have a common vision.

x86 is driven by the vendors, not us. If the vendors don't provide ACPI 
entries for a device then the choices are to either use port probing or 
refuse to support that device. We've traditionally gone for the former.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Guenter Roeck]

On 08/29/2013 05:57 AM, Matthew Garrett wrote:
> On Thu, Aug 29, 2013 at 02:39:33PM +0200, Linus Walleij wrote:
>
>> I think Rafael said something about it being possible for us
>> to register our own kernel ACPI PNP IDs (as if: there is no
>> road here, but if someone starts to walk here, a road will
>> soon become, and we take the first step then).
>
> It'd be straightforward to register the LNX PnP prefix and have someone
> take responsibility for assigning numbers, but really a generic vendor
> string should only be used when defining programming models rather than
> specific devices.
>
>> But overall I am a bit confused: I am hearing from one end
>> of the x86 community that ACPI is the way to go for
>> configuring platform devices on x86, yet stuff like this is
>> popping up from independent vendors, and get integrated
>> on boards with no ACPI tables in sight.
>
> ACPI is usually used to describe systems, and the normal ACPI way of
> handling GPIO devices is to expose the device at the other end of the
> GPIO lines and then provide AML for toggling the lines. Attaching an
> actual driver to the device would interfere with that, so nobody writes
> an actual driver.
>
>> Over at ksummit-discuss we have had a thread about
>> whether device tree should be used in such cases, but
>> that is not clear either.
>
> If a vendor doesn't provide any way to autoprobe a device, there's no
> way to autoprobe a device. That usually means that you're not expected
> to use that device.
>

Pretty radical. Following your advice, should we remove all watchdog
and hwmon drivers for all SuperIO chips out there, plus any existing
gpio drivers (drivers/char/pc8736x_gpio.c might be a candidate) ?

Oh, and the parallel port driver also detects super-io chips directly,
so maybe the respective code should be removed as well. I am sure there
is more code that can be removed.

Or is the idea to say "no acpi, no new driver" ? Just wondering -
I have a GPIO driver for Nuvoton chips on my back-burner; that would be
necessary to access some fan controls connected to gpio pins on some boards.
If this is a no-go, I'll happily drop it from my list of things to do,
and just tell the user community that Linux won't support their hardware
due to policy reasons.

Guenter

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Matthew Garrett]

On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> On 08/29/2013 05:57 AM, Matthew Garrett wrote:
> >If a vendor doesn't provide any way to autoprobe a device, there's no
> >way to autoprobe a device. That usually means that you're not expected
> >to use that device.
> >
> 
> Pretty radical. Following your advice, should we remove all watchdog
> and hwmon drivers for all SuperIO chips out there, plus any existing
> gpio drivers (drivers/char/pc8736x_gpio.c might be a candidate) ?

The majority of board vendors clearly don't expect the OS to drive the 
hwmon chips - they're there for the benefit of ACPI and SMM code. That 
doesn't mean that there's no benefit in having drivers for them, just 
that the board vendors don't care about that use case and so won't do 
anything to make it easier.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Guenter Roeck]

On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
> On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> > On 08/29/2013 05:57 AM, Matthew Garrett wrote:
> > >If a vendor doesn't provide any way to autoprobe a device, there's no
> > >way to autoprobe a device. That usually means that you're not expected
> > >to use that device.
> > >
> > 
> > Pretty radical. Following your advice, should we remove all watchdog
> > and hwmon drivers for all SuperIO chips out there, plus any existing
> > gpio drivers (drivers/char/pc8736x_gpio.c might be a candidate) ?
> 
> The majority of board vendors clearly don't expect the OS to drive the 
> hwmon chips - they're there for the benefit of ACPI and SMM code. That 
> doesn't mean that there's no benefit in having drivers for them, just 
> that the board vendors don't care about that use case and so won't do 
> anything to make it easier.
> 
Actually, not entirely true. Some vendors even provide software running on
Windows to access those chips (including access to fans controlled through
GPIO pins) and to provide information to the user. It might be more accurate
to say that some board vendors don't care about Linux (or about providing
access through ACPI, for that matter).

Question here is what the Linux kernel community's policy is going to be
to handle such cases. Pragmatic or dogmatic ?

Guenter

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Matthew Garrett]

On Thu, Aug 29, 2013 at 09:08:30AM -0700, Guenter Roeck wrote:
> On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
> > On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> > The majority of board vendors clearly don't expect the OS to drive the 
> > hwmon chips - they're there for the benefit of ACPI and SMM code. That 
> > doesn't mean that there's no benefit in having drivers for them, just 
> > that the board vendors don't care about that use case and so won't do 
> > anything to make it easier.
> > 
> Actually, not entirely true. Some vendors even provide software running on
> Windows to access those chips (including access to fans controlled through
> GPIO pins) and to provide information to the user. It might be more accurate
> to say that some board vendors don't care about Linux (or about providing
> access through ACPI, for that matter).

Eh. If they're really providing code that just assumes that hardware is 
present and bangs on it, what choice do we have? You've checked that the 
machines in question don't have a magic ACPI device that provides 
resource information?

> Question here is what the Linux kernel community's policy is going to be
> to handle such cases. Pragmatic or dogmatic ?

I don't see any benefit in changing the status quo. Sometimes hardware 
is just shit. The majority of x86 vendors certainly don't care about 
anything we do, so it's not like we're in a position to force them to 
change.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Guenter Roeck]

On Thu, Aug 29, 2013 at 05:25:51PM +0100, Matthew Garrett wrote:
> On Thu, Aug 29, 2013 at 09:08:30AM -0700, Guenter Roeck wrote:
> > On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
> > > On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> > > The majority of board vendors clearly don't expect the OS to drive the 
> > > hwmon chips - they're there for the benefit of ACPI and SMM code. That 
> > > doesn't mean that there's no benefit in having drivers for them, just 
> > > that the board vendors don't care about that use case and so won't do 
> > > anything to make it easier.
> > > 
> > Actually, not entirely true. Some vendors even provide software running on
> > Windows to access those chips (including access to fans controlled through
> > GPIO pins) and to provide information to the user. It might be more accurate
> > to say that some board vendors don't care about Linux (or about providing
> > access through ACPI, for that matter).
> 
> Eh. If they're really providing code that just assumes that hardware is 
> present and bangs on it, what choice do we have? You've checked that the 
> machines in question don't have a magic ACPI device that provides 
> resource information?
> 
Not to my knowledge. One vendor doing this extensively is ASRock. They have
a Windows based tuning aplication which lets users even set chip voltages
through SuperIO GPIO pins. Not that I would recommend supporting anything
like that in Linux ;).

I also have a couple of Intel motherboards which don't support ACPI methods 
to access hardware monitoring information or the watchdog on the SuperIO chip.
Guess they don't want me to use that information ;).

There is lots of hardware monitoring tools available for Windows. Practically
none of them uses ACPI to collect the information from the hardware. Instead,
they use the same method we use in the Linux kernel to detect the SuperIO chip.
An open source tool is http://openhardwaremonitor.org/. 

> > Question here is what the Linux kernel community's policy is going to be
> > to handle such cases. Pragmatic or dogmatic ?
> 
> I don't see any benefit in changing the status quo. Sometimes hardware 
> is just shit. The majority of x86 vendors certainly don't care about 
> anything we do, so it's not like we're in a position to force them to 
> change.
> 
Me not either. It just sounded to me that this was the suggestion, so I was
asking for clarification. It doesn't make sense for people like me to submit
a driver into the kernel only to have it rejected for non-technical reasons.

Guenter

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Simon Guinot]

[-- Attachment #1: Type: text/plain, Size: 1990 bytes --]

On Thu, Aug 29, 2013 at 05:25:51PM +0100, Matthew Garrett wrote:
> On Thu, Aug 29, 2013 at 09:08:30AM -0700, Guenter Roeck wrote:
> > On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:
> > > On Thu, Aug 29, 2013 at 06:41:41AM -0700, Guenter Roeck wrote:
> > > The majority of board vendors clearly don't expect the OS to drive the 
> > > hwmon chips - they're there for the benefit of ACPI and SMM code. That 
> > > doesn't mean that there's no benefit in having drivers for them, just 
> > > that the board vendors don't care about that use case and so won't do 
> > > anything to make it easier.
> > > 
> > Actually, not entirely true. Some vendors even provide software running on
> > Windows to access those chips (including access to fans controlled through
> > GPIO pins) and to provide information to the user. It might be more accurate
> > to say that some board vendors don't care about Linux (or about providing
> > access through ACPI, for that matter).
> 
> Eh. If they're really providing code that just assumes that hardware is 
> present and bangs on it, what choice do we have? You've checked that the 
> machines in question don't have a magic ACPI device that provides 
> resource information?
> 
> > Question here is what the Linux kernel community's policy is going to be
> > to handle such cases. Pragmatic or dogmatic ?
> 
> I don't see any benefit in changing the status quo. Sometimes hardware 
> is just shit. The majority of x86 vendors certainly don't care about 
> anything we do, so it's not like we're in a position to force them to 
> change.

Almost all the LaCie NASes are running a Linux based OS. I can assure
you that LaCie really takes care of Linux. Now, the main reason why the
Super-I/O ACPI support is missing on this boards is a lack of knowledge.
Thanks to this discussion, I am now aware of that. Then, for the next
products, I will do my best to get AMLs or at least ACPI IDs.

Thanks,

Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Matthew Garrett]

On Fri, Aug 30, 2013 at 12:20:54AM +0200, Simon Guinot wrote:

> Almost all the LaCie NASes are running a Linux based OS. I can assure
> you that LaCie really takes care of Linux. Now, the main reason why the
> Super-I/O ACPI support is missing on this boards is a lack of knowledge.
> Thanks to this discussion, I am now aware of that. Then, for the next
> products, I will do my best to get AMLs or at least ACPI IDs.

Awesome! Let me know if you need any advice on this.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Linus Walleij]

On Thu, Aug 29, 2013 at 6:08 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Thu, Aug 29, 2013 at 04:37:38PM +0100, Matthew Garrett wrote:

>> The majority of board vendors clearly don't expect the OS to drive the
>> hwmon chips - they're there for the benefit of ACPI and SMM code. That
>> doesn't mean that there's no benefit in having drivers for them, just
>> that the board vendors don't care about that use case and so won't do
>> anything to make it easier.
>>
> Actually, not entirely true. Some vendors even provide software running on
> Windows to access those chips (including access to fans controlled through
> GPIO pins) and to provide information to the user. It might be more accurate
> to say that some board vendors don't care about Linux (or about providing
> access through ACPI, for that matter).
>
> Question here is what the Linux kernel community's policy is going to be
> to handle such cases. Pragmatic or dogmatic ?

The GPIO maintainer is going to be pragmatic, because it's cool to
be able to control HWMON things on one's workstation.

The kernel is a funnier place with these drivers, so I'm applying the
patch.

Yours,
Linus Walleij

Re: [PATCH v3] gpio: add GPIO support for F71882FG and F71889F

[Linus Walleij]

On Mon, Jul 22, 2013 at 11:50 AM, Simon Guinot
<simon.guinot@sequanux.org> wrote:

> This patch adds support for the GPIOs found on the Fintek super-I/O
> chips F71882FG and F71889F.
>
> A super-I/O is a legacy I/O controller embedded on x86 motherboards. It
> is used to connect the low-bandwidth devices. Among others functions the
> F71882FG/F71889F provides: a parallel port, two serial ports, a keyboard
> controller, an hardware monitoring controller and some GPIO pins.
>
> Note that this super-I/Os are embedded on some Atom-based LaCie NASes.
> The GPIOs are used to control the LEDs and the hard drive power.
>
> Signed-off-by: Simon Guinot <simon.guinot@sequanux.org>
> ---
> Changes since v2:
> - Remove useless NULL setters for driver data.

OK we had some discussion on this: there seem to be no alternatives
to port probing, and hacking these board drivers is mighty cool, so
patch applied.

Yours,
Linus Walleij