IT87xx GPIO and other drivers
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?
Signature · 6 lines
Diego Elio Pettenò — Flameeyes [email protected] — http://blog.flameeyes.eu/ -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 50 lines · 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_wdtfor most it87xx chips (including it8712);
-it8712f_wdtwhich supports the "Smart Guardian" watchdog;
-gpio_it8761efor that single IT87xx gpio;
- mygpio_it87driver 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
_shouldnot_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.
Quoted 3 lines
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 8 lines · Diego Elio Pettenò
The GPIO pins on most of the it87xx chips are also multi-function, and
_shouldnot_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:
Quoted 10 lines
- 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 16/04/2012 23:47, Linus Walleij ha scritto:
Quoted 7 lines
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?
Quoted 2 lines
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!
Signature · 6 lines
Diego Elio Pettenò — Flameeyes [email protected] — http://blog.flameeyes.eu/ -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 3 lines · Diego Elio Pettenò
Does that mean that when the user enables the MFD, they get gpio, pinctrl and watchdog at once, or should it still have multiple
Kconfigentries 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html