[L] Change in coreboot[main]: superio/ite/common: Add common driver for GPIO and LED configuration

[Michał Żygowski (Code Review)]

Attention is currently required from: Nicholas Sudsgaard, Nico Huber.

Michał Żygowski has posted comments on this change by Michał Żygowski. ( https://review.coreboot.org/c/coreboot/+/83355?usp=email )

Change subject: superio/ite/common: Add common driver for GPIO and LED configuration
......................................................................

Patch Set 3:

(10 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/83355/comment/6a22d92c_1dacb046?usp... :
PS1, Line 22: - Refactors the mainboards' code to use the new driver where possible
> When a patch has hundreds of lines and a list of things it does […]

Split into 3:
- this one
- CB:83468 (enabling the driver for in chip Kconfig)
- CB:83469 (MB code refactoring)

File src/mainboard/google/beltino/variants/mccloud/led.c:

https://review.coreboot.org/c/coreboot/+/83355/comment/4f992d5c_c32717cf?usp... :
PS1, Line 18: ITE_LED_SHORT_PULSE_DISABLE,
            : 		ITE_LED_PINMAP_CLEAR_DISABLE,
            : 		ITE_LED_OUTPUT_LOW_DISABLE,
            : 		ITE_LED_FREQ_1HZ);
> Random thought: Making the enum values masks (i.e. with the 1 shifted already) […]

I have added gpio_ctrl and led_ctrl for these properties/flags, although I have left the frequency as a separate parameter.

File src/mainboard/samsung/stumpy/smihandler.c:

https://review.coreboot.org/c/coreboot/+/83355/comment/af5b83d5_603ac0df?usp... :
PS1, Line 25: 		ite_reg_write(GPIO_DEV, IT8772F_GPIO_REG_SELECT(3), 0x20);
> Yes, setup_sio_gpios already does it and further writes should not be necessary.

Done in CB:83469

https://review.coreboot.org/c/coreboot/+/83355/comment/9553f431_eb7bce44?usp... :
PS1, Line 39: 		ite_reg_write(GPIO_DEV, IT8772F_GPIO_REG_SELECT(3), 0x20);
> Same.

Done in CB:83469

https://review.coreboot.org/c/coreboot/+/83355/comment/a3de068c_7f40d524?usp... :
PS1, Line 49: 			ITE_LED_FREQ_1HZ);
> Right, enabling SIMPLE_IO_MODE in the ite_gpio_setup call should already disable the LED blinking fu […]

Done in CB:83469

File src/superio/ite/common/gpio.c:

https://review.coreboot.org/c/coreboot/+/83355/comment/c20e78a2_9a0f4b7b?usp... :
PS1, Line 16: This ITE SIO GPIO driver support up to 10 GPIO sets
> "ITE SIO GPIO drivers only support up to 10 GPIO sets" sounds more natural as an error message.

Changed per suggestion

https://review.coreboot.org/c/coreboot/+/83355/comment/2ab25276_dd42fe53?usp... :
PS1, Line 130: 	 */
> Right, I will revisit these datasheets and fix

Added the function which checks for the register presence like for the other 2 registers. Haven't found any other chips which would have some oddities here.

File src/superio/ite/common/ite_gpio.h:

https://review.coreboot.org/c/coreboot/+/83355/comment/e415e5f0_f3415c81?usp... :
PS1, Line 15: ITE_GPIO_PULLUP_DIS,
            : 	ITE_GPIO_PULLUP_EN
> To keep the naming consistent with the other enums. […]

It has been converted to a single flag: ITE_GPIO_PULLUP_ENABLE

https://review.coreboot.org/c/coreboot/+/83355/comment/41548814_33b25d60?usp... :
PS1, Line 63: ITE_LED_FREQ_2Hz_DUTY_50 = 3,
            : 	ITE_LED_FREQ_0P25HZ_DUTY_25 = 4,
            : 	ITE_LED_FREQ_0P25HZ_DUTY_75 = 5,
            : 	ITE_LED_FREQ_0P125HZ_DUTY_25 = 6,
            : 	ITE_LED_FREQ_0P125Hz_DUTY_75 = 7,
            : 	ITE_LED_FREQ_0P4Hz_DUTY_20 = 8,
            : 	ITE_LED_FREQ_0P5Hz_DUTY_50 = 16,
            : 	ITE_LED_FREQ_0P125Hz_DUTY_50 = 24,
> Is there a reason the 'z' is not capitalized for some of these enums? […]

Applied.

File src/superio/ite/it8772f/it8772f.h:

https://review.coreboot.org/c/coreboot/+/83355/comment/01c5a181_ae782c65?usp... :
PS1, Line 16: 
            : /* GPIO Polarity Select: 1: Inverting, 0: Non-inverting */
            : #define IT8772F_GPIO_REG_POLARITY(x) (0xb0 + (x))
            : 
            : /* GPIO Internal Pull-up: 1: Enable, 0: Disable */
            : #define IT8772F_GPIO_REG_PULLUP(x)   (0xb8 + (x))
            : 
            : /* GPIO Function Select: 1: Simple I/O, 0: Alternate function */
            : #define IT8772F_GPIO_REG_ENABLE(x)   (0xc0 + (x))
            : 
            : /* GPIO Mode: 0: input mode, 1: output mode */
            : #define IT8772F_GPIO_REG_OUTPUT(x)   (0xc8 + (x))
            :
> Forgot to leave IT8772F_GPIO_REG_SELECT only. […]

Removed in CB:83469

ITE_GPIO_REG_SELECT moved to superio/ite/common/ite_gpio.h in this patch.