[RFC] IT87xx GPIO and other drivers

[RFC] IT87xx GPIO and other drivers

[Diego Elio Pettenò]

Hi all, sorry if I'm mailing lots of you at once, but I'm afraid I got
my hands on a can of worms right now...

As some of you remember I've been working on a driver for supporting the
GPIO functions of the IT8728F Super I/O chip, as well as making the WDT
driver to work on this chip.

After trying to make a bit of sense of it all, I'm concerned that the
only _correct_ way to handle this would be to ahve a set of drivers that
work together, rather than a number of drivers that all do their own
part. Currently we have:

 - hwmon/it87 which supports most it87xx chips;
 - it87_wdt for most it87xx chips (including it8712);
 - it8712f_wdt which supports the "Smart Guardian" watchdog;
 - gpio_it8761e for that single IT87xx gpio;
 - my gpio_it87 driver that works with it8728f and should work with
it8761e (for what I can tell from the other driver), and Guillaume has
code for IT8712 as well (which variant?).

What issues are there with this situation?

All these drivers use to some extent the Super I/O addresses (0x2e/0x2f)
to read and write to its registers, including detection code which is
replicated for each of them. The functions to read and write superio
registers is also duplicate.

Only some of these drivers (namely gpio-it8761e for what I can tell)
support checking both 0x2e/0x2f and 0x4e/0x4f, which is an alternative
addressing for the superio register handling.

The GPIO pins on most of the it87xx chips are also multi-function, and
_should not_ be user-visible, but for some of them it might make sense
to (for instance it should be possible to drive some of the LEDs on
it8728f-based motherboards by replacing the functions of some PINs to GPIO).

For what I can tell, it should probably be a good idea to have something
along these lines, but I'm _not_ a driver expert:

 - mfd-it87xx: a platform driver, which probes the superio to check it
to be an it87xx chip, and then reserve resources for the other drivers;
 - hwmon/it87: no longer probes autonomously for which chip it is, and
where it is;
 - it87_wdt: ibidem;
 - it8712_wdt: no clue about it, but I guess the same;
 - gpio-it87: ibidem again;
 - pinctrl-it87: abstracts support for the various pin-choice registers;
 - led-it87: possibly to drive the power/network/hdd leds, akin to what
happens with some laptops, and embedded systems.

Does a plan like this make sense? Denis have you still access to an
it8761e board?

-- 
Diego Elio Pettenò — Flameeyes
flameeyes@flameeyes.eu — http://blog.flameeyes.eu/

[RFC] IT87xx GPIO and other drivers

[Diego Elio Pettenò]

Hi all, sorry if I'm mailing lots of you at once, but I'm afraid I got
my hands on a can of worms right now...

As some of you remember I've been working on a driver for supporting the
GPIO functions of the IT8728F Super I/O chip, as well as making the WDT
driver to work on this chip.

After trying to make a bit of sense of it all, I'm concerned that the
only _correct_ way to handle this would be to ahve a set of drivers that
work together, rather than a number of drivers that all do their own
part. Currently we have:

 - hwmon/it87 which supports most it87xx chips;
 - it87_wdt for most it87xx chips (including it8712);
 - it8712f_wdt which supports the "Smart Guardian" watchdog;
 - gpio_it8761e for that single IT87xx gpio;
 - my gpio_it87 driver that works with it8728f and should work with
it8761e (for what I can tell from the other driver), and Guillaume has
code for IT8712 as well (which variant?).

What issues are there with this situation?

All these drivers use to some extent the Super I/O addresses (0x2e/0x2f)
to read and write to its registers, including detection code which is
replicated for each of them. The functions to read and write superio
registers is also duplicate.

Only some of these drivers (namely gpio-it8761e for what I can tell)
support checking both 0x2e/0x2f and 0x4e/0x4f, which is an alternative
addressing for the superio register handling.

The GPIO pins on most of the it87xx chips are also multi-function, and
_should not_ be user-visible, but for some of them it might make sense
to (for instance it should be possible to drive some of the LEDs on
it8728f-based motherboards by replacing the functions of some PINs to GPIO).

For what I can tell, it should probably be a good idea to have something
along these lines, but I'm _not_ a driver expert:

 - mfd-it87xx: a platform driver, which probes the superio to check it
to be an it87xx chip, and then reserve resources for the other drivers;
 - hwmon/it87: no longer probes autonomously for which chip it is, and
where it is;
 - it87_wdt: ibidem;
 - it8712_wdt: no clue about it, but I guess the same;
 - gpio-it87: ibidem again;
 - pinctrl-it87: abstracts support for the various pin-choice registers;
 - led-it87: possibly to drive the power/network/hdd leds, akin to what
happens with some laptops, and embedded systems.

Does a plan like this make sense? Denis have you still access to an
it8761e board?

-- 
Diego Elio Pettenò — Flameeyes
flameeyes@flameeyes.eu — http://blog.flameeyes.eu/
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] IT87xx GPIO and other drivers

[Guenter Roeck]

On Mon, 2012-04-16 at 15:51 -0400, Diego Elio Pettenò wrote:
> Hi all, sorry if I'm mailing lots of you at once, but I'm afraid I got
> my hands on a can of worms right now...
> 
> As some of you remember I've been working on a driver for supporting the
> GPIO functions of the IT8728F Super I/O chip, as well as making the WDT
> driver to work on this chip.
> 
> After trying to make a bit of sense of it all, I'm concerned that the
> only _correct_ way to handle this would be to ahve a set of drivers that
> work together, rather than a number of drivers that all do their own
> part. Currently we have:
> 
>  - hwmon/it87 which supports most it87xx chips;
>  - it87_wdt for most it87xx chips (including it8712);
>  - it8712f_wdt which supports the "Smart Guardian" watchdog;
>  - gpio_it8761e for that single IT87xx gpio;
>  - my gpio_it87 driver that works with it8728f and should work with
> it8761e (for what I can tell from the other driver), and Guillaume has
> code for IT8712 as well (which variant?).
> 
> What issues are there with this situation?
> 
> All these drivers use to some extent the Super I/O addresses (0x2e/0x2f)
> to read and write to its registers, including detection code which is
> replicated for each of them. The functions to read and write superio
> registers is also duplicate.
> 
> Only some of these drivers (namely gpio-it8761e for what I can tell)
> support checking both 0x2e/0x2f and 0x4e/0x4f, which is an alternative
> addressing for the superio register handling.
> 
> The GPIO pins on most of the it87xx chips are also multi-function, and
> _should not_ be user-visible, but for some of them it might make sense
> to (for instance it should be possible to drive some of the LEDs on
> it8728f-based motherboards by replacing the functions of some PINs to GPIO).
> 
> For what I can tell, it should probably be a good idea to have something
> along these lines, but I'm _not_ a driver expert:
> 
>  - mfd-it87xx: a platform driver, which probes the superio to check it
> to be an it87xx chip, and then reserve resources for the other drivers;
>  - hwmon/it87: no longer probes autonomously for which chip it is, and
> where it is;
>  - it87_wdt: ibidem;
>  - it8712_wdt: no clue about it, but I guess the same;
>  - gpio-it87: ibidem again;
>  - pinctrl-it87: abstracts support for the various pin-choice registers;
>  - led-it87: possibly to drive the power/network/hdd leds, akin to what
> happens with some laptops, and embedded systems.
> 
> Does a plan like this make sense? Denis have you still access to an
> it8761e board?
> 
Yes, that sounds about right.

Guenter

Re: [RFC] IT87xx GPIO and other drivers

[Guenter Roeck]

On Mon, 2012-04-16 at 15:51 -0400, Diego Elio Pettenò wrote:
> Hi all, sorry if I'm mailing lots of you at once, but I'm afraid I got
> my hands on a can of worms right now...
> 
> As some of you remember I've been working on a driver for supporting the
> GPIO functions of the IT8728F Super I/O chip, as well as making the WDT
> driver to work on this chip.
> 
> After trying to make a bit of sense of it all, I'm concerned that the
> only _correct_ way to handle this would be to ahve a set of drivers that
> work together, rather than a number of drivers that all do their own
> part. Currently we have:
> 
>  - hwmon/it87 which supports most it87xx chips;
>  - it87_wdt for most it87xx chips (including it8712);
>  - it8712f_wdt which supports the "Smart Guardian" watchdog;
>  - gpio_it8761e for that single IT87xx gpio;
>  - my gpio_it87 driver that works with it8728f and should work with
> it8761e (for what I can tell from the other driver), and Guillaume has
> code for IT8712 as well (which variant?).
> 
> What issues are there with this situation?
> 
> All these drivers use to some extent the Super I/O addresses (0x2e/0x2f)
> to read and write to its registers, including detection code which is
> replicated for each of them. The functions to read and write superio
> registers is also duplicate.
> 
> Only some of these drivers (namely gpio-it8761e for what I can tell)
> support checking both 0x2e/0x2f and 0x4e/0x4f, which is an alternative
> addressing for the superio register handling.
> 
> The GPIO pins on most of the it87xx chips are also multi-function, and
> _should not_ be user-visible, but for some of them it might make sense
> to (for instance it should be possible to drive some of the LEDs on
> it8728f-based motherboards by replacing the functions of some PINs to GPIO).
> 
> For what I can tell, it should probably be a good idea to have something
> along these lines, but I'm _not_ a driver expert:
> 
>  - mfd-it87xx: a platform driver, which probes the superio to check it
> to be an it87xx chip, and then reserve resources for the other drivers;
>  - hwmon/it87: no longer probes autonomously for which chip it is, and
> where it is;
>  - it87_wdt: ibidem;
>  - it8712_wdt: no clue about it, but I guess the same;
>  - gpio-it87: ibidem again;
>  - pinctrl-it87: abstracts support for the various pin-choice registers;
>  - led-it87: possibly to drive the power/network/hdd leds, akin to what
> happens with some laptops, and embedded systems.
> 
> Does a plan like this make sense? Denis have you still access to an
> it8761e board?
> 
Yes, that sounds about right.

Guenter


--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] IT87xx GPIO and other drivers

[Linus Walleij]

On Mon, Apr 16, 2012 at 9:51 PM, Diego Elio Pettenò
<flameeyes@flameeyes.eu> wrote:

> The GPIO pins on most of the it87xx chips are also multi-function, and
> _should not_ be user-visible, but for some of them it might make sense
> to (for instance it should be possible to drive some of the LEDs on
> it8728f-based motherboards by replacing the functions of some PINs to GPIO).
>
> For what I can tell, it should probably be a good idea to have something
> along these lines, but I'm _not_ a driver expert:
>
>  - mfd-it87xx: a platform driver, which probes the superio to check it
> to be an it87xx chip, and then reserve resources for the other drivers;
>  - hwmon/it87: no longer probes autonomously for which chip it is, and
> where it is;
>  - it87_wdt: ibidem;
>  - it8712_wdt: no clue about it, but I guess the same;
>  - gpio-it87: ibidem again;
>  - pinctrl-it87: abstracts support for the various pin-choice registers;
>  - led-it87: possibly to drive the power/network/hdd leds, akin to what
> happens with some laptops, and embedded systems.

I would suggest merging gpio-it87 and pinctrl-it87 into one driver
in drivers/pinctrl-it87.c. I don't know for sure however, since it depends
on hardware: usually these is a tight dependence between GPIO and
pinctrl (IIRC this was the case with SuperIO), and then it often makes
a lot of sense to create a composite driver, in order to just have one
state container (cookie) to pass around in the functions, and to remap
a register range only once.

The concept is explained in my pinctrl talk on a high level:
http://www.df.lth.se/~triad/papers/pincontrol.pdf

Yours,
Linus Walleij

Re: [RFC] IT87xx GPIO and other drivers

[Linus Walleij]

On Mon, Apr 16, 2012 at 9:51 PM, Diego Elio Pettenò
<flameeyes@flameeyes.eu> wrote:

> The GPIO pins on most of the it87xx chips are also multi-function, and
> _should not_ be user-visible, but for some of them it might make sense
> to (for instance it should be possible to drive some of the LEDs on
> it8728f-based motherboards by replacing the functions of some PINs to GPIO).
>
> For what I can tell, it should probably be a good idea to have something
> along these lines, but I'm _not_ a driver expert:
>
>  - mfd-it87xx: a platform driver, which probes the superio to check it
> to be an it87xx chip, and then reserve resources for the other drivers;
>  - hwmon/it87: no longer probes autonomously for which chip it is, and
> where it is;
>  - it87_wdt: ibidem;
>  - it8712_wdt: no clue about it, but I guess the same;
>  - gpio-it87: ibidem again;
>  - pinctrl-it87: abstracts support for the various pin-choice registers;
>  - led-it87: possibly to drive the power/network/hdd leds, akin to what
> happens with some laptops, and embedded systems.

I would suggest merging gpio-it87 and pinctrl-it87 into one driver
in drivers/pinctrl-it87.c. I don't know for sure however, since it depends
on hardware: usually these is a tight dependence between GPIO and
pinctrl (IIRC this was the case with SuperIO), and then it often makes
a lot of sense to create a composite driver, in order to just have one
state container (cookie) to pass around in the functions, and to remap
a register range only once.

The concept is explained in my pinctrl talk on a high level:
http://www.df.lth.se/~triad/papers/pincontrol.pdf

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] IT87xx GPIO and other drivers

[Diego Elio Pettenò]

Il 16/04/2012 23:47, Linus Walleij ha scritto:
> I would suggest merging gpio-it87 and pinctrl-it87 into one driver
> in drivers/pinctrl-it87.c. I don't know for sure however, since it depends
> on hardware: usually these is a tight dependence between GPIO and
> pinctrl (IIRC this was the case with SuperIO), and then it often makes
> a lot of sense to create a composite driver, in order to just have one
> state container (cookie) to pass around in the functions, and to remap
> a register range only once.

So you mean having the MFD actually handle most of the work, and that's
it? If I did read the source correctly, that might actually work well,
as hwmon/it87 is only using the SuperIO for detection (which MFD can
take care of), while the others would be using the same functions to
read/write to the SuperIO registers.

Does that mean that when the user enables the MFD, they get gpio,
pinctrl and watchdog at once, or should it still have multiple Kconfig
entries which select it?

> The concept is explained in my pinctrl talk on a high level:
> http://www.df.lth.se/~triad/papers/pincontrol.pdf

Okay will read through it, thanks!

-- 
Diego Elio Pettenò — Flameeyes
flameeyes@flameeyes.eu — http://blog.flameeyes.eu/

Re: [RFC] IT87xx GPIO and other drivers

[Diego Elio Pettenò]

Il 16/04/2012 23:47, Linus Walleij ha scritto:
> I would suggest merging gpio-it87 and pinctrl-it87 into one driver
> in drivers/pinctrl-it87.c. I don't know for sure however, since it depends
> on hardware: usually these is a tight dependence between GPIO and
> pinctrl (IIRC this was the case with SuperIO), and then it often makes
> a lot of sense to create a composite driver, in order to just have one
> state container (cookie) to pass around in the functions, and to remap
> a register range only once.

So you mean having the MFD actually handle most of the work, and that's
it? If I did read the source correctly, that might actually work well,
as hwmon/it87 is only using the SuperIO for detection (which MFD can
take care of), while the others would be using the same functions to
read/write to the SuperIO registers.

Does that mean that when the user enables the MFD, they get gpio,
pinctrl and watchdog at once, or should it still have multiple Kconfig
entries which select it?

> The concept is explained in my pinctrl talk on a high level:
> http://www.df.lth.se/~triad/papers/pincontrol.pdf

Okay will read through it, thanks!

-- 
Diego Elio Pettenò — Flameeyes
flameeyes@flameeyes.eu — http://blog.flameeyes.eu/
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] IT87xx GPIO and other drivers

[Linus Walleij]

On Tue, Apr 17, 2012 at 4:51 PM, Diego Elio Pettenò
<flameeyes@flameeyes.eu> wrote:

> Does that mean that when the user enables the MFD, they get gpio,
> pinctrl and watchdog at once, or should it still have multiple Kconfig
> entries which select it?

No I just meant that the driver file in drivers/pinctrl/pinctrl-it87.c should
implement pinctrl, gpiolib (struct gpio_chip) and any GPIO
IRQs (struct irq_chip) in that one and the same file.

The MFD device will spawn cells/children as usual for HWMON and
other stuff, but the cell named "pinctrl-it87" (or however you
name it) should take care of all the pinctrl and GPIO stuff.

Yours,
Linus Walleij

Re: [RFC] IT87xx GPIO and other drivers

[Linus Walleij]

On Tue, Apr 17, 2012 at 4:51 PM, Diego Elio Pettenò
<flameeyes@flameeyes.eu> wrote:

> Does that mean that when the user enables the MFD, they get gpio,
> pinctrl and watchdog at once, or should it still have multiple Kconfig
> entries which select it?

No I just meant that the driver file in drivers/pinctrl/pinctrl-it87.c should
implement pinctrl, gpiolib (struct gpio_chip) and any GPIO
IRQs (struct irq_chip) in that one and the same file.

The MFD device will spawn cells/children as usual for HWMON and
other stuff, but the cell named "pinctrl-it87" (or however you
name it) should take care of all the pinctrl and GPIO stuff.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html