From aae779c28aff37868a3a3c2b53de583305d4b65b Mon Sep 17 00:00:00 2001 From: James Byrne Date: Fri, 29 May 2015 11:16:58 +0100 Subject: [PATCH 1/4] Fix compiler warning. Fix assignment that caused an "initialization from incompatible pointer type" warning. --- atsha204-i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atsha204-i2c.c b/atsha204-i2c.c index f26033a..6f41267 100644 --- a/atsha204-i2c.c +++ b/atsha204-i2c.c @@ -148,7 +148,7 @@ bool atsha204_crc16_matches(const u8 *buf, const u8 len, const u16 crc) bool atsha204_check_rsp_crc16(const u8 *buf, const u8 len) { - u16 *rec_crc = &buf[len - 2]; + const u16 *rec_crc = (const u16 *)&buf[len - 2]; return atsha204_crc16_matches(buf, len - 2, cpu_to_le16(*rec_crc)); } From 284142dbc4ab34aee8615f5fb7ea679821d89898 Mon Sep 17 00:00:00 2001 From: James Byrne Date: Fri, 29 May 2015 14:33:33 +0100 Subject: [PATCH 2/4] Improve Makefile. This commit makes a number of improvements to the Makefile: - Test KERNELRELEASE to separate the parts used by kbuild from the parts used when invoked normally. - Use standard aliases for $(MAKE) and $(CC) instead of using them explicitly. - Separate the building of the test program, give it a proper dependency and simplify the recipe. - Add a KOPTIONS variable that can be used to allow cross-compilation. --- Makefile | 47 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 8f8a6f5..e773237 100644 --- a/Makefile +++ b/Makefile @@ -1,33 +1,52 @@ +ifneq ($(KERNELRELEASE),) +# If KERNELRELEASE is defined then we have been included by the kernel's +# makefiles and are actually building the module. + obj-m := atsha204-i2c.o + +# Enable CFLAGS to run DEBUG MODE +#CFLAGS_atsha204-i2c.o := -DDEBUG + +else + +# To cross-compile this module invoke make with KDIR set to the kernel directory +# and KOPTIONS set to the required extra kernel build options, for example: +# make KDIR=$MYPROJ/mykernel KOPTIONS="ARCH=arm CROSS_COMPILE=$MYPROJ/toolchain/bin/arm-cortexa5-linux-gnueabihf-" + KDIR ?= /lib/modules/`uname -r`/build MDIR ?= /lib/modules/`uname -r`/kernel/drivers/char/ -SRC = atsha204-i2c.c atsha204-i2c.h -# Enable CFLAG to run DEBUG MODE -#CFLAGS_atsha204-i2c.o := -DDEBUG -all: - make -C $(KDIR) M=$$PWD modules -# make testing code - gcc -c $$PWD/test/test.c - gcc $$PWD/test/test.c -o $$PWD/test/test +SRC := atsha204-i2c.c atsha204-i2c.h +MODULE := atsha204-i2c.ko + +.PHONY: all module clean install check modules_install + +all: module test/test + +module: + $(MAKE) -C $(KDIR) $(KOPTIONS) M=$(CURDIR) modules + +test/test: test/test.c + $(CC) $^ -o $@ clean: - make -C $(KDIR) M=$$PWD clean - -rm -rf $$PWD/test/test.o $$PWD/test/test TAGS + $(MAKE) -C $(KDIR) $(KOPTIONS) M=$(CURDIR) clean + rm -f test/test TAGS install: - sudo cp atsha204-i2c.ko $(MDIR) - -sudo insmod atsha204-i2c.ko + sudo cp $(MODULE) $(MDIR) + -sudo insmod $(MODULE) -echo atsha204-i2c 0x60 | sudo tee /sys/class/i2c-adapter/i2c-1/new_device sudo chgrp i2c /dev/atsha0 sudo chmod 664 /dev/atsha0 - check: ./test/test modules_install: - cp atsha204-i2c.ko $(MDIR) + cp $(MODULE) $(MDIR) TAGS: etags $(SRC) + +endif From 61b390e5ef7c31aeab9ebe5d8c262a02d990e4dd Mon Sep 17 00:00:00 2001 From: James Byrne Date: Wed, 17 Jun 2015 14:26:56 +0100 Subject: [PATCH 3/4] Prevent crash when calling hwrng_register(). The call to hwrng_register() can cause an immediate callback to atsha204_i2c_get_random(). Since the global_chip variable is NULL at this point, this causes a crash. Fix this by setting global_chip before the call. --- atsha204-i2c.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/atsha204-i2c.c b/atsha204-i2c.c index 6f41267..7672912 100644 --- a/atsha204-i2c.c +++ b/atsha204-i2c.c @@ -404,8 +404,8 @@ int atsha204_i2c_release(struct inode *inode, struct file *filep) struct atsha204_chip *atsha204_i2c_register_hardware(struct device *dev, struct i2c_client *client) { - struct atsha204_chip *chip; + int rc; if ((chip = kzalloc(sizeof(*chip), GFP_KERNEL)) == NULL) goto out_null; @@ -425,11 +425,11 @@ struct atsha204_chip *atsha204_i2c_register_hardware(struct device *dev, dev_err(dev, "%s\n", "Failed to add device"); goto put_device; } - else{ - int rc = hwrng_register(&atsha204_i2c_rng); - dev_dbg(dev, "%s%d\n", "HWRNG result: ", rc); - } + global_chip = chip; + + rc = hwrng_register(&atsha204_i2c_rng); + dev_dbg(dev, "%s%d\n", "HWRNG result: ", rc); return chip; From ad5c3479ae01ee978764699832e1d33de2206b52 Mon Sep 17 00:00:00 2001 From: James Byrne Date: Wed, 17 Jun 2015 14:20:52 +0100 Subject: [PATCH 4/4] Improve device wake-up logic and debug messages. The atsha204_i2c_wakeup() function did not work reliably. To wake the chip it is necessary to hold SDA low for at least tWLO (60us), then delay for tWHI (500us for ATECC108A; 2.5ms for ATSHA204). There is no way to explicitly hold the SDA line low in Linux, so instead attempt to send 0 to address 0; this will hold SDA low for 8 clock cycles (sending the address), which will be more than 60us if the I2C clock speed is less than 133kHz. The limitation this imposes on the clock speed is restrictive, but this logic does at least work reliably when the clock speed is under this threshold. This commit also tidies up the debug so that dev_dbg() is used throughout, and reorders some of the messages so that they come out in a logical order that makes debugging easier. --- atsha204-i2c.c | 90 ++++++++++++++++++++++++++++---------------------- atsha204-i2c.h | 3 ++ 2 files changed, 54 insertions(+), 39 deletions(-) diff --git a/atsha204-i2c.c b/atsha204-i2c.c index 7672912..65cb6b1 100644 --- a/atsha204-i2c.c +++ b/atsha204-i2c.c @@ -48,7 +48,7 @@ int atsha204_i2c_get_random(u8 *to_fill, const size_t max) memcpy(to_fill, &recv.ptr[1], rnd_len); rc = rnd_len; dev_info(global_chip->dev, "%s: %d\n", - "Returning randoom bytes", rc); + "Returning random bytes", rc); } } @@ -73,14 +73,14 @@ int atsha204_i2c_transaction(struct atsha204_chip *chip, mutex_lock(&chip->transaction_mutex); dev_dbg(chip->dev, "%s\n", "About to send to device."); - print_hex_dump_bytes("Sending : ", DUMP_PREFIX_OFFSET, - to_send, to_send_len); - /* Begin i2c transactions */ if ((rc = atsha204_i2c_wakeup(chip->client))) goto out; + print_hex_dump_bytes("Sending : ", DUMP_PREFIX_OFFSET, + to_send, to_send_len); + if ((rc = i2c_master_send(chip->client, to_send, to_send_len)) != to_send_len) goto out; @@ -99,8 +99,6 @@ int atsha204_i2c_transaction(struct atsha204_chip *chip, memcpy(recv_buf, status_packet, sizeof(status_packet)); rc = i2c_master_recv(chip->client, recv_buf + 4, packet_len - 4); - atsha204_i2c_idle(chip->client); - /* Store the entire packet. Other functions must check the CRC and strip of the length byte */ buf->ptr = recv_buf; @@ -110,6 +108,8 @@ int atsha204_i2c_transaction(struct atsha204_chip *chip, print_hex_dump_bytes("Received: ", DUMP_PREFIX_OFFSET, recv_buf, packet_len); + atsha204_i2c_idle(chip->client); + rc = to_send_len; out: mutex_unlock(&chip->transaction_mutex); @@ -176,54 +176,69 @@ int atsha204_i2c_validate_rsp(const struct atsha204_buffer *packet, int atsha204_i2c_wakeup(const struct i2c_client *client) { + const struct device *dev = &client->dev; bool is_awake = false; int retval = -ENODEV; - + struct i2c_msg msg; u8 buf[4] = {0}; - unsigned short int try_con = 1; + /* Set wake-up token message */ + msg.addr = 0; + msg.flags = 0; + msg.buf = buf; + msg.len = 1; + while (!is_awake){ - if (4 == i2c_master_send(client, buf, 4)){ - pr_debug("%s\n", "ATSHA204 Device is awake."); + dev_dbg(dev, "Send wake-up (%u)\n", try_con); + /* To wake up the device you need to hold SDA low for at least + * 60us (tWLO). There is no way to do this explicitly in + * Linux, so attempt to send 0 to address 0. This will hold + * SDA low for 8 clock cycles (sending the address), which + * will work as long as the I2C clock speed is less than + * 133kHz. + */ + buf[0] = 0; + i2c_transfer(client->adapter, &msg, 1); + /* Delay for tWHI before reading the response. */ + udelay(client->addr == 0x60 ? ATECC108_W_HI : ATSHA204_W_HI); + /* Read the response. */ + if (4 == i2c_master_recv(client, buf, 4)){ + dev_dbg(dev, "Chip is awake\n"); is_awake = true; - if (4 == i2c_master_recv(client, buf, 4)){ - pr_debug("%s", "ATSHA204 Received wakeup\n"); - } - - - if (atsha204_check_rsp_crc16(buf,4)) + if (atsha204_check_rsp_crc16(buf,4)){ + if (buf[1] == 0x11){ + dev_dbg(dev, "Wakeup response OK\n"); retval = 0; - else - pr_err("%s\n", "ATSHA204 Wakeup CRC failure"); - - - } - else{ - /* is_awake is already false */ - pr_info("Attempting Wakeup : %u\n",try_con); - if(try_con >= 10){ - pr_err("Wakeup Failed. No Device"); - return retval; + } else { + dev_err(dev, "Wakeup response incorrect (0x%x)\n", buf[1]); + retval = EIO; + } + } else { + dev_err(dev, "Wakeup CRC failure\n"); + retval = EIO; + } + } else { + if(++try_con >= 5){ + dev_err(dev, "Wakeup failed. No Device\n"); + break; } } - - ++try_con; } - retval = 0; return retval; - } int atsha204_i2c_idle(const struct i2c_client *client) { + const struct device *dev = &client->dev; int rc; u8 idle_cmd[1] = {0x02}; + dev_dbg(dev, "Send idle\n"); rc = i2c_master_send(client, idle_cmd, 1); return rc; @@ -232,13 +247,15 @@ int atsha204_i2c_idle(const struct i2c_client *client) int atsha204_i2c_sleep(const struct i2c_client *client) { + const struct device *dev = &client->dev; int retval; char to_send[1] = {ATSHA204_SLEEP}; + dev_dbg(dev, "Send sleep\n"); if ((retval = i2c_master_send(client,to_send,1)) == 1) retval = 0; else - pr_err("%s: 0x%x\n", "ATSHA204 failed to sleep", client->addr); + dev_err(dev, "Failed to sleep\n"); return retval; @@ -453,9 +470,6 @@ int atsha204_i2c_probe(struct i2c_client *client, return -ENODEV; if((result = atsha204_i2c_wakeup(client)) == 0){ - pr_debug("%s: 0x%x\n", "ATSHA204 Device is awake", - client->addr); - atsha204_i2c_idle(client); if ((chip = atsha204_i2c_register_hardware(dev, client)) @@ -469,10 +483,8 @@ int atsha204_i2c_probe(struct i2c_client *client, result = atsha204_sysfs_add_device(chip); } - else{ - pr_err("%s: 0x%x\n", "ATSHA204 device failed to wake", - client->addr); + dev_err(dev, "Device failed to wake\n"); } return result; @@ -481,7 +493,7 @@ int atsha204_i2c_probe(struct i2c_client *client, int atsha204_i2c_remove(struct i2c_client *client) { - struct device *dev = &(client->dev); + const struct device *dev = &(client->dev); struct atsha204_chip *chip = dev_get_drvdata(dev); if (chip){ diff --git a/atsha204-i2c.h b/atsha204-i2c.h index d4f6bc6..cceaa8c 100644 --- a/atsha204-i2c.h +++ b/atsha204-i2c.h @@ -21,6 +21,9 @@ #define ATSHA204_SLEEP 0x01 #define ATSHA204_RNG_NAME "atsha-rng" +#define ATECC108_W_HI (500) // 500us for ATECC108A +#define ATSHA204_W_HI (2500) // 2.5ms for ATSHA204 + struct atsha204_chip { struct device *dev;