This patch needs a commit log. In fact, it can be squashed into the first commit which uses these defines.
> Signed-off-by: Wei-Chun Pan <[email protected]> > --- > include/linux/mfd/imanager2_ec.h | 358 > +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 358 insertions(+) > create mode 100644 include/linux/mfd/imanager2_ec.h > > diff --git a/include/linux/mfd/imanager2_ec.h > b/include/linux/mfd/imanager2_ec.h > new file mode 100644 > index 0000000..bf7d70e > --- /dev/null > +++ b/include/linux/mfd/imanager2_ec.h > @@ -0,0 +1,358 @@ > +/* > + * imanager2_ec.h - MFD driver defines of Advantech EC IT8516/18/28 > + * Copyright (C) 2014 Richard Vidal-Dorsch <[email protected]> > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. This is the looooong version of the licence. Can you find the shorter one. > + */ > + > +#ifndef __IMANAGER2_EC_H__ > +#define __IMANAGER2_EC_H__ > + > +#include <linux/mutex.h> > + > +#define EC_FLAG_IO 0 > +#define EC_FLAG_IO_MAILBOX (1 << 0) > +#define EC_FLAG_MAILBOX (1 << 1) Use BIT(0), BIT(1) instead. > +#define EC_MAX_DEVICE_ID_NUM 0xFF > +#define EC_MAX_ITEM_NUM 32 > + > +struct ec_table { > + u8 devid2itemnum[EC_MAX_DEVICE_ID_NUM]; > + u8 pinnum[EC_MAX_ITEM_NUM]; > + u8 devid[EC_MAX_ITEM_NUM]; > +}; > + > +struct ec_version { > + u16 kernel_ver, > + chip_code, > + prj_id, > + prj_ver; Declare these separately. > +}; > + > +#define EC_MAX_LEN_PROJECT_NAME 8 Find a way to do this dynamically, or use 'const char *' instead. > +struct imanager2 { > + u16 id; > + u32 flag; > + struct mutex lock; /* protects io */ We know what locks do. > + char prj_name[EC_MAX_LEN_PROJECT_NAME + 1]; /* strlen + '\0' */ We know how strings work. > + struct ec_version version; > + struct ec_table table; 'table' is awfully generic. > +}; Use KernelDoc to document the main container. > +/* > + * Definition > + */ This comment is not adding anything to the code here. > +#define EC_TABLE_ITEM_UNUSED 0xFF > +#define EC_TABLE_DID_NODEV 0x00 > +#define EC_TABLE_HWP_NODEV 0xFF > +#define EC_TABLE_NOITEM 0xFF > + > +#define EC_ERROR 0xFF > + > +#define EC_RAM_BANK_SIZE 32 /* 32 bytes size for each bank. */ > +#define EC_RAM_BUFFER_SIZE 256 /* 32 bytes * 8 banks = 256 bytes */ > + > +#define EC_SIO_CMD 0x29C > +#define EC_SIO_DATA 0x29D 12bit commands? Or are these 16bit and should have a leading 0? > +/* Access Mailbox */ > +#define EC_IO_PORT_CMD 0x29A > +#define EC_IO_PORT_DATA 0x299 > + > +#define EC_IO_CMD_READ_OFFSET 0xA0 > +#define EC_IO_CMD_WRITE_OFFSET 0x50 Should these have leading 0's too? > +#define EC_ITE_PORT_OFS 0x29E > +#define EC_ITE_PORT_DATA 0x29F > + > +/* > + * CMD - IO > + */ Drop this. > +/* ADC */ > +#define EC_CMD_ADC_INDEX 0x15 > +#define EC_CMD_ADC_READ_LSB 0x16 > +#define EC_CMD_ADC_READ_MSB 0x1F > +/* HW Control Table */ > +#define EC_CMD_HWCTRLTABLE_INDEX 0x20 > +#define EC_CMD_HWCTRLTABLE_GET_PIN_NUM 0x21 > +#define EC_CMD_HWCTRLTABLE_GET_DEVICE_ID 0x22 > +#define EC_CMD_HWCTRLTABLE_GET_PIN_ACTIVE_POLARITY 0x23 > +/* ACPI RAM */ > +#define EC_CMD_ACPIRAM_READ 0x80 > +#define EC_CMD_ACPIRAM_WRITE 0x81 > +/* Extend RAM */ > +#define EC_CMD_EXTRAM_READ 0x86 > +#define EC_CMD_EXTRAM_WRITE 0x87 > +/* HW RAM */ > +#define EC_CMD_HWRAM_READ 0x88 > +#define EC_CMD_HWRAM_WRITE 0x89 > + > +/* > + * ACPI RAM Address Table > + */ > +/* n = 1 ~ 2 */ Please accompany with words. Random math is seldom helpful. I'm guessing that you mean valid values of 'n' can be 1 or 2, but this is unclear. > +#define EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n) (0x60 + 3 * ((n) - 1)) What is 0x60? Why 3? Please use #defines for these numbers. > +#define EC_ACPIRAM_ADDR_LOCAL_TEMPERATURE(n) \ > + EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n) > +#define EC_ACPIRAM_ADDR_REMOTE_TEMPERATURE(n) \ > + (EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n) + 1) > +#define EC_ACPIRAM_ADDR_WARNING_TEMPERATURE(n)\ > + (EC_ACPIRAM_ADDR_TEMPERATURE_BASE(n) + 2) > + > +/* N = 0 ~ 2 */ Please accompany with words. Random math is seldom helpful. > +#define EC_ACPIRAM_ADDR_FAN_SPEED_BASE(N) (0x70 + 2 * (N)) No magic numbers. Why are you using 'N' here and 'n' before? > +#define EC_ACPIRAM_ADDR_KERNEL_MAJOR_VERSION 0xF8 > +#define EC_ACPIRAM_ADDR_CHIP_VENDOR_CODE 0xFA > +#define EC_ACPIRAM_ADDR_PROJECT_NAME_CODE 0xFC > +#define EC_ACPIRAM_ADDR_FIRMWARE_MAJOR_VERSION 0xFE > + > +/* > + * HW RAM Address Table > + */ > +/* Thermal Source Control RAM 0xB0-0xC7 (N: 0 ~ 3) */ > +#define EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) (0xB0 + 6 * (N)) Same comments as above. > +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_CHANNEL(N) \ > + EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) > +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_ADDR(N) \ > + (EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 1) > +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_CMD(N) \ > + (EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 2) > +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_STATUS(N) \ > + (EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 3) > +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_FAN_CODE(N) \ > + (EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 4) > +#define EC_HWRAM_ADDR_THERMAL_SOURCE_SMB_TEMPERATURE(N) \ > + (EC_HWRAM_ADDR_THERMAL_SOURCE_BASE_ADDR(N) + 5) Place more '\n' spacers in - these are difficult to read. > +/* Fan Control 0xD0-0xEF (N: 0 ~ 3) */ > +#define EC_HWRAM_ADDR_FAN_BASE_ADDR(N) (0xD0 + 0x10 * (N)) > +#define EC_HWRAM_ADDR_FAN_CODE(N) EC_HWRAM_ADDR_FAN_BASE_ADDR(N) > +#define EC_HWRAM_ADDR_FAN_STATUS(N) (EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 1) > +#define EC_HWRAM_ADDR_FAN_CONTROL(N) (EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 2) > +#define EC_HWRAM_ADDR_FAN_TEMP_HI(N) (EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 3) > +#define EC_HWRAM_ADDR_FAN_TEMP_LO(N) (EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 4) > +#define EC_HWRAM_ADDR_FAN_TEMP_LOSTOP(N) \ > + (EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 5) > +#define EC_HWRAM_ADDR_FAN_PWM_HI(N) (EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 6) > +#define EC_HWRAM_ADDR_FAN_PWM_LO(N) (EC_HWRAM_ADDR_FAN_BASE_ADDR(N) + 7) > + > +/* > + * OFS - Mailbox > + */ > +/* Mailbox Structure */ > +#define EC_MAILBOX_OFFSET_CMD 0x00 > +#define EC_MAILBOX_OFFSET_STATUS 0x01 > +#define EC_MAILBOX_OFFSET_PARA 0x02 > +#define EC_MAILBOX_OFFSET_DAT(N) (0x03 + (N)) /* N = 0x00 ~ 0x2C */ > + > +/* > + * CMD - Mailbox > + */ > +/* GPIO */ > +#define EC_CMD_MAILBOX_READ_HW_PIN 0x11 > +#define EC_CMD_MAILBOX_WRITE_HW_PIN 0x12 > +/* Storage */ > +#define EC_CMD_MAILBOX_READ_EC_RAM 0x1E > +#define EC_CMD_MAILBOX_WRITE_EC_RAM 0x1F > +/* OTHERS */ > +#define EC_CMD_MAILBOX_READ_DYNAMIC_TABLE 0x20 > +/* Thermal Protect */ > +#define EC_CMD_MAILBOX_READ_THERMAL_SOURCE 0x42 > +#define EC_CMD_MAILBOX_WRITE_THERMAL_SOURCE 0x43 > +/* Storage */ > +#define EC_CMD_MALLBOX_CLEAR_256_BYTES_BUFFER 0xC0 > +#define EC_CMD_MALLBOX_READ_256_BYTES_BUFFER 0xC1 > +#define EC_CMD_MALLBOX_WRITE_256_BYTES_BUFFER 0xC2 > +#define EC_CMD_MALLBOX_READ_EEPROM_DATA_FROM_256_BYTES_BUFFER 0xC3 > +#define EC_CMD_MALLBOX_WRITE_256_BYTES_BUFFER_INTO_EEPROM_DATA 0xC4 > +/* General Mailbox Command */ > +#define EC_CMD_MAILBOX_GET_FIRMWARE_VERSION_AND_PROJECT_NAME 0xF0 > +#define EC_CMD_MAILBOX_CLEAR_ALL 0xFF > + > +/* > + * Status - Mailbox > + */ > +#define EC_MAILBOX_STATUS_FAIL 0x00 > +#define EC_MAILBOX_STATUS_SUCCESS 0x01 > + > +/* > + * PARA - Mailbox > + */ > +/* RAM Type */ > +#define EC_RAM_BANK_ACPI 0x01 > +#define EC_RAM_BANK_HW 0x02 > +#define EC_RAM_BANK_EXT 0x03 > +#define EC_RAM_BANK_BUFFER 0x06 > +/* Dynamic Type */ > +#define EC_DYNAMIC_DEVICE_ID 0x00 > +#define EC_DYNAMIC_HW_PIN 0x01 > +#define EC_DYNAMIC_POLARITY 0x02 > + > +/* > + * Functions - Mailbox > + */ > +/* command = 0x20 */ > +int imanager2_mbox_get_dynamic_table(u32 ecflag, u8 type, u8 *table); > +/* command = 0x42 */ > +int imanager2_mbox_read_thermalzone(u32 ecflag, u8 zone, u8 *smbid, u8 > *fanid, > + u8 *buf, int *len); > +/* command = 0xC0 */ > +int imanager2_mbox_clear_ram(u32 ecflag); > +/* command = 0xC1 */ > +int imanager2_mbox_read_ram_across_banks(u32 ecflag, u8 *data, int len); > +/* command = 0x1E */ > +int imanager2_mbox_read_ram(u32 ecflag, u8 bank, u8 offset, u8 *buf, u8 len); > +/* command = 0x1F */ > +int imanager2_mbox_write_ram(u32 ecflag, u8 bank, u8 offset, u8 *buf, u8 > len); > +/* command = 0xF0 */ > +int imanager2_mbox_get_project_information(u32 ecflag, u8 *prj_name, > + u16 *kernel_ver, u16 *chip_code, > + u16 *prj_id, u16 *prj_ver); > + > +/* > + * Functions - basic > + */ > +#define IO_FLAG_OBF (1 << 0) /* output buffer full */ > +#define IO_FLAG_IBF (1 << 1) /* input buffer full */ use BIT() > +int ec_inb_after_obf1(u8 *data); > +int ec_outb_after_ibc0(u16 port, u8 data); > +/* ITE mailbox access */ > +int imanager2_mbox_read_data(u32 ecflag, u8 cmd, u8 para, u8 *data, int len); > +int imanager2_mbox_write_data(u32 ecflag, u8 cmd, u8 para, u8 *data, int > len); > +/* only IO access */ > +int imanager2_mbox_io_read(u8 command, u8 offset, u8 *buf, u8 len); > +int imanager2_mbox_io_write(u8 command, u8 offset, u8 *buf, u8 len); > +int imanager2_mbox_io_simple_read(u8 command, u8 *value); > +/* ITE Mailbox & IO access */ > +int imanager2_mbox_read_ram_support_io(u32 ecflag, u8 bank, u8 addr, u8 *buf, > + u8 len); > + > +/* > + * Device ID > + */ Comments with the same information is as the type/variable name are not helpful. Please remove. > +enum ec_device_id { > + /* GPIO */ > + altgpio0 = 0x10, /* 0x10 */ You're joking right? > + altgpio1, > + altgpio2, > + altgpio3, > + altgpio4, > + altgpio5, > + altgpio6, > + altgpio7, > + /* GPIO - Button */ > + btn0, > + btn1, > + btn2, > + btn3, > + btn4, > + btn5, > + btn6, > + btn7, > + /* PWM - Fan */ > + cpufan_2p, /* 0x20 */ Remove these comments. > + cpufan_4p, > + sysfan1_2p, > + sysfan1_4p, > + sysfan2_2p, > + sysfan2_4p, > + /* PWM - Brightness Control */ > + pwmbrightness, > + /* PWM - System Speaker */ > + pwmbeep, > + /* SMBus */ > + smboem0, > + smboem1, > + smboem2, > + smbeeprom, > + smbthermal0, > + smbthermal1, > + smbsecurityeep, > + i2coem, > + /* DAC - Speaker */ > + dacspeaker, /* 0x30 */ > + /* SMBus */ > + smbeep2k = 0x38, > + oemeep, > + oemeep2k, > + peci, > + smboem3, > + smblink, > + smbslv, > + /* GPIO - LED */ > + powerled = 0x40, /* 0x40 */ > + batledg, > + oemled0, > + oemled1, > + oemled2, > + batledr, > + /* SMBus - Smart Battery */ > + smartbat1 = 0x48, > + smartbat2, > + /* ADC */ > + adcmosbat = 0x50, /* 0x50 */ > + adcmosbatx2, > + adcmosbatx10, > + adcbat, > + adcbatx2, > + adcbatx10, > + adc5vs0, > + adc5vs0x2, > + adc5vs0x10, > + adv5vs5, > + adv5vs5x2, > + adv5vs5x10, > + adc33vs0, > + adc33vs0x2, > + adc33vs0x10, > + adc33vs5, > + adc33vs5x2, /* 0x60 */ > + adc33vs5x10, > + adv12vs0, > + adv12vs0x2, > + adv12vs0x10, > + adcvcorea, > + adcvcoreax2, > + adcvcoreax10, > + adcvcoreb, > + adcvcorebx2, > + adcvcorebx10, > + adcdc, > + adcdcx2, > + adcdcx10, > + adcdcstby, > + adcdcstbyx2, > + adcdcstbyx10, /* 0x70 */ > + adcdcother, > + adcdcotherx2, > + adcdcotherx10, > + adccurrent, > + /* IRQ - Watchdog */ > + wdirq = 0x78, > + /* GPIO - Watchdog */ > + wdnmi, > + /* Tacho - Fan */ > + tacho0 = 0x80, /* 0x80 */ > + tacho1, > + tacho2, > + /* PWM - Brightness Control */ > + pwmbrightness2 = 0x88, > + /* GPIO - Backlight Control */ > + brionoff1, > + brionoff2, > +}; > + > +#endif /* __IMANAGER2_EC_H__ */ -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
