From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Danin Date: Tue, 29 Apr 2014 11:15:07 +0400 Subject: [U-Boot] [PATCH v2 3/6] ARM: tegra: i2c: add nvec driver In-Reply-To: <535F3C01.10300@denx.de> References: <[PATCH 0/3] ARM: tegra: add nvec keyboard support for paz00> <1398561270-25091-1-git-send-email-danindrey@mail.ru> <1398561270-25091-4-git-send-email-danindrey@mail.ru> <535F3C01.10300@denx.de> Message-ID: <535F517B.8040702@mail.ru> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 29.04.2014 9:43, Heiko Schocher wrote: > Hello Andrey, > Hello Heiko. First of all, thank you for the review! > Am 27.04.2014 03:14, schrieb Andrey Danin: >> Signed-off-by: Andrey Danin >> CC: Stephen Warren >> CC: Marc Dietrich >> CC: Julian Andres Klode >> CC: ac100 at lists.launchpad.net >> --- >> Changes for v2: >> - NVEC driver was reworked to use tegra-i2c >> >> arch/arm/include/asm/arch-tegra/tegra_nvec.h | 130 ++++++++++++ >> board/nvidia/common/board.c | 12 ++ >> drivers/i2c/Makefile | 1 + >> drivers/i2c/tegra_nvec.c | 294 >> ++++++++++++++++++++++++++ >> include/fdtdec.h | 1 + >> lib/fdtdec.c | 1 + >> 6 files changed, 439 insertions(+) >> create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec.h >> create mode 100644 drivers/i2c/tegra_nvec.c >> >> diff --git a/arch/arm/include/asm/arch-tegra/tegra_nvec.h >> b/arch/arm/include/asm/arch-tegra/tegra_nvec.h >> new file mode 100644 >> index 0000000..f1cec0d >> --- /dev/null >> +++ b/arch/arm/include/asm/arch-tegra/tegra_nvec.h >> @@ -0,0 +1,130 @@ >> +/* >> + * (C) Copyright 2014 >> + * Andrey Danin >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#ifndef _TEGRA_NVEC_H_ >> +#define _TEGRA_NVEC_H_ >> + >> +#define I2C_CNFG 0x00 >> +#define I2C_CNFG_PACKET_MODE_EN (1<<10) >> +#define I2C_CNFG_NEW_MASTER_SFM (1<<11) >> +#define I2C_CNFG_DEBOUNCE_CNT_SHIFT 12 >> + >> +#define I2C_SL_CNFG 0x20 >> +#define I2C_SL_NEWSL (1<<2) >> +#define I2C_SL_NACK (1<<1) >> +#define I2C_SL_RESP (1<<0) >> +#define I2C_SL_IRQ (1<<3) >> +#define END_TRANS (1<<4) >> +#define RCVD (1<<2) >> +#define RNW (1<<1) >> + >> +#define I2C_SL_RCVD 0x24 >> +#define I2C_SL_STATUS 0x28 >> +#define I2C_SL_ADDR1 0x2c >> +#define I2C_SL_ADDR2 0x30 >> +#define I2C_SL_DELAY_COUNT 0x3c >> + >> + > > please only one line, check globally. > I will. Thanks for pointing. > [...] >> diff --git a/drivers/i2c/tegra_nvec.c b/drivers/i2c/tegra_nvec.c >> new file mode 100644 >> index 0000000..b568988 >> --- /dev/null >> +++ b/drivers/i2c/tegra_nvec.c >> @@ -0,0 +1,294 @@ >> +/* >> + * (C) Copyright 2014 >> + * Andrey Danin >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#ifndef CONFIG_SYS_I2C_TEGRA_NVEC >> +#error "You should enable CONFIG_SYS_I2C_TEGRA_NVEC" >> +#endif >> + >> +DECLARE_GLOBAL_DATA_PTR; >> + >> +struct nvec_periph devices[NVEC_LAST_MSG]; >> + >> + >> +struct nvec_t { >> + int i2c_bus; >> + struct fdt_gpio_state gpio; >> +} nvec_data; >> + >> + >> +/* nvec commands */ >> +char noop[] = { NVEC_CNTL, CNTL_NOOP }; >> + >> +void nvec_signal_request(void) >> +{ >> + gpio_set_value(nvec_data.gpio.gpio, 0); >> +} >> + >> + >> +void nvec_clear_request(void) >> +{ >> + gpio_set_value(nvec_data.gpio.gpio, 1); >> +} >> + >> + >> +int nvec_msg_is_event(const unsigned char *msg) >> +{ >> + return msg[0]>> 7; >> +} >> + >> + >> +int nvec_msg_event_type(const unsigned char *msg) >> +{ >> + return msg[0]& 0x0f; >> +} >> + >> + >> +/** >> + * Process incoming io message. >> + * If message is keyboard event then key code will >> + * be added to keys buffer. >> + * >> + * @param nvec nvec state struct >> + */ >> +void nvec_process_msg(struct nvec_t *nvec, struct i2c_transaction >> *trans) >> +{ >> + (void) nvec; >> + > > no empty line needed. > >> + const unsigned char *msg = (const unsigned char *)&trans->rx_buf[1]; >> + int event_type; >> + >> + if (!nvec_msg_is_event(msg)) >> + return; >> + >> + event_type = nvec_msg_event_type(msg); >> + >> + if (event_type< NVEC_KEYBOARD || event_type>= NVEC_LAST_MSG) >> + return; >> + >> + if (devices[event_type].process_msg) >> + devices[event_type].process_msg(msg); >> +} >> + >> + >> +/** >> + * Perform complete io operation (read or write). >> + * NOTE: function will wait NVEC_TIMEOUT_MIN (20ms) >> + * before status check to avoid nvec hang. >> + * >> + * @param nvec nvec state struct >> + * @param wait_for_ec if 1(NVEC_WAIT_FOR_EC) operation >> + * timeout is NVEC_TIMEOUT_MAX (600ms), >> + * otherwise function will return if io >> + * is not ready. >> + * >> + * @return nvec_io_* code >> + */ >> +int nvec_do_io(struct nvec_t *nvec) >> +{ >> + static unsigned int prev_timer; >> + unsigned int poll_start_ms = get_timer(prev_timer); >> + struct i2c_transaction trans; >> + int res; >> + >> + if (poll_start_ms> 30000) { >> + if (prev_timer != 0) >> + nvec_do_request(noop, sizeof(noop)); >> + prev_timer = get_timer(0); >> + } >> + >> + memset(&trans, 0, sizeof(trans)); >> + trans.start_timeout = 1; >> + trans.timeout = 6000; >> + >> + res = i2c_slave_io(&trans); >> + if (res == 0) { >> + nvec_process_msg(nvec,&trans); >> + return 0; >> + } >> + >> + if (res != -1) >> + debug("Error: i2c slave io failed with code %d\n", res); >> + >> + return -1; >> +} >> + >> +/** >> + * Send request and read response. If write or read failed >> + * operation will be repeated NVEC_ATTEMPTS_MAX times. >> + * >> + * @param buf request data >> + * @param size request data size >> + * @return 0 if ok, -1 on error >> + */ >> +int nvec_do_request(char *buf, int size) >> +{ >> + int res; >> + struct i2c_transaction trans; >> + int i; >> + >> + nvec_signal_request(); >> + >> + /* Request */ >> + for (i = 0; i< 10; ++i) { >> + memset(&trans, 0, sizeof(trans)); >> + trans.start_timeout = 600; >> + trans.timeout = 6000; >> + >> + trans.tx_buf[0] = (char)size; >> + memcpy(&trans.tx_buf[1], buf, size); >> + trans.tx_size = size + 1; >> + >> + res = i2c_slave_io(&trans); >> + if (res == 0) { >> + if (trans.tx_pos == trans.tx_size) >> + break; >> + >> + debug("Request was not sent completely"); >> + } else if (res != -1) { >> + debug("Unknown error while slave io"); >> + } >> + } >> + nvec_clear_request(); >> + if (res != 0) { >> + error("nvec failed to perform request\n"); >> + return -1; >> + } >> + >> + /* Response */ >> + for (i = 0; i< 10; ++i) { >> + memset(&trans, 0, sizeof(trans)); >> + trans.start_timeout = 600; >> + trans.timeout = 6000; >> + >> + res = i2c_slave_io(&trans); >> + if (res == 0) >> + break; >> + } >> + if (res != 0) { >> + error("nvec failed to read response\n"); >> + return -1; >> + } >> + >> + /* TODO Parse response */ > > ? > > Is this not a complete driver? > The driver is complete. This part is not mandatory because request and response are processed synchronously. I will write a proper comment here. >> + >> + return 0; >> +} >> + >> + >> +/** >> + * Decode the nvec information from the fdt. >> + * >> + * @param blob fdt blob >> + * @param nvec nvec device sturct >> + * @return 0 if ok, -ve on error > > What is "-ve" ? > It means -error_code. I will rework return codes, comments, style issues according to all your remarks. >> + */ >> +static int nvec_decode_config(const void *blob, >> + struct nvec_t *nvec) >> +{ >> + int node, parent; >> + int i2c_bus; >> + >> + node = fdtdec_next_compatible(blob, 0, COMPAT_NVIDIA_TEGRA20_NVEC); >> + if (node< 0) { >> + error("Cannot find NVEC node in fdt\n"); >> + return node; >> + } >> + >> + parent = fdt_parent_offset(blob, node); >> + if (parent< 0) { >> + debug("%s: Cannot find node parent\n", __func__); >> + return -1; >> + } >> + >> + i2c_bus = i2c_get_bus_num_fdt(parent); >> + if (i2c_bus< 0) >> + return -1; >> + nvec->i2c_bus = i2c_bus; >> + >> + if (fdtdec_decode_gpio(blob, node, "request-gpios", >> + &nvec->gpio)) { >> + error("No NVEC request gpio\n"); >> + return -1; >> + } >> + >> + debug("NVEC: i2c:%d, gpio:%s(%u)\n", >> + nvec->i2c_bus, nvec->gpio.name, nvec->gpio.gpio); >> + return 0; >> +} >> + >> + >> +int board_nvec_init(void) >> +{ >> + int res = 0; >> + int i; >> + >> + if (nvec_decode_config(gd->fdt_blob,&nvec_data)) { >> + error("Can't parse NVEC node in device tree\n"); >> + return -1; >> + } >> + >> + debug("NVEC initialization...\n"); >> + >> + res = gpio_request(nvec_data.gpio.gpio, NULL); >> + if (res != 0) >> + error("NVEC: err, gpio_request\n"); >> + res = gpio_direction_output(nvec_data.gpio.gpio, 1); >> + if (res != 0) >> + error("NVEC: err, gpio_direction\n"); >> + res = gpio_set_value(nvec_data.gpio.gpio, 1); >> + if (res != 0) >> + error("NVEC: err, gpio_set_value\n"); >> + udelay(100); >> + >> + i2c_set_bus_num(nvec_data.i2c_bus); >> + >> + for (i = NVEC_KEYBOARD; i< NVEC_LAST_MSG; ++i) >> + if (devices[i].start) { >> + debug("Starting device %d(0x%x)\n", i, i); >> + devices[i].start(); >> + } >> + >> + return 1; > > Why "return 1"? > >> +} >> + >> + >> +int nvec_read_events(void) >> +{ >> + int res; >> + int cnt = 0; >> + >> + while (++cnt<= 8) { >> + res = nvec_do_io(&nvec_data); >> + if (res) >> + break; >> + >> + /* TODO Process nvec communication errors */ > > Again a "TODO" ? > I will implement. >> + } >> + >> + return cnt; >> +} >> + >> + >> +int nvec_register_periph(int msg_type, struct nvec_periph *periph) >> +{ >> + if (devices[msg_type].start || devices[msg_type].process_msg) { >> + error("Device for msg %d already registered\n", msg_type); >> + return -1; >> + } >> + >> + devices[msg_type] = *periph; >> + return 0; >> +} >> diff --git a/include/fdtdec.h b/include/fdtdec.h >> index 19bab79..7b84335 100644 >> --- a/include/fdtdec.h >> +++ b/include/fdtdec.h >> @@ -64,6 +64,7 @@ enum fdt_compat_id { >> COMPAT_NVIDIA_TEGRA20_SDMMC, /* Tegra20 SDMMC controller */ >> COMPAT_NVIDIA_TEGRA20_SFLASH, /* Tegra 2 SPI flash controller */ >> COMPAT_NVIDIA_TEGRA20_SLINK, /* Tegra 2 SPI SLINK controller */ >> + COMPAT_NVIDIA_TEGRA20_NVEC, /* Tegra 2 EC controller */ >> COMPAT_NVIDIA_TEGRA114_SPI, /* Tegra 114 SPI controller */ >> COMPAT_SMSC_LAN9215, /* SMSC 10/100 Ethernet LAN9215 */ >> COMPAT_SAMSUNG_EXYNOS5_SROMC, /* Exynos5 SROMC */ >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c >> index 1fecab3..82df701 100644 >> --- a/lib/fdtdec.c >> +++ b/lib/fdtdec.c >> @@ -37,6 +37,7 @@ static const char * const compat_names[COMPAT_COUNT] >> = { >> COMPAT(NVIDIA_TEGRA20_SDMMC, "nvidia,tegra20-sdhci"), >> COMPAT(NVIDIA_TEGRA20_SFLASH, "nvidia,tegra20-sflash"), >> COMPAT(NVIDIA_TEGRA20_SLINK, "nvidia,tegra20-slink"), >> + COMPAT(NVIDIA_TEGRA20_NVEC, "nvidia,tegra20-nvec"), >> COMPAT(NVIDIA_TEGRA114_SPI, "nvidia,tegra114-spi"), >> COMPAT(SMSC_LAN9215, "smsc,lan9215"), >> COMPAT(SAMSUNG_EXYNOS5_SROMC, "samsung,exynos-sromc"), > > bye, > Heiko Thanks.