| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <546DD158.8010200@samsung.com>
Date: Thu, 20 Nov 2014 12:32:40 +0100
From: Karol Wrona <k.wrona@...sung.com>
To: Antonio Borneo <borneo.antonio@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
Hartmut Knaack <knaack.h@....de>, linux-iio@...r.kernel.org,
Arnd Bergmann <arnd@...db.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>
Subject: Re: [PATCH v4 1/1] misc: st32fwu: Add stm32 upgrade protocol handling
On 11/20/2014 07:18 AM, Antonio Borneo wrote:
> On Sat, Nov 8, 2014 at 9:29 PM, Karol Wrona <k.wrona@...sung.com> wrote:
>> Adds stm32 bootloader protocol handling.
>>
>
> Hi Karol,
>
> Sorry for not being able to reply earlier. I'm finally back after a
> period off-line.
>
> I have implemented a first version of SPI flash upgrade in the
> user-mode tool "stm32flash". I will submit it upstream after further
> cleanup.
> Anyway, now I have much clear idea about the SPI bootloader protocol
> and differences with I2C and UART variants (already in "stm32flash").
>
> There are few details in your driver that don't match the bootloader
> protocol over SPI described in AN4268.
> Can you please check with the documents you have and with your HW?
> I'm using the document Rev.2 available in ST website at
> http://www.st.com/web/en/resource/technical/document/application_note/DM00081379.pdf
> Cannot find a previous revision to compare them.
Ok, I will fix them.
>
>> SPI transfers are done using DMA safe buffer which is allocated once per
>> spi upgrade life cycle. Now it supports only SPI bus but it looks that UART
>> or I2C are quite similar and it can be used as start platform for implementing
>> their handling.
>>
>> It was tested on STM32F401 MCU.
>>
>> Change-Id: I5e5b441310c897ff822e65041531d80ea0e7426c
>> Signed-off-by: Karol Wrona <k.wrona@...sung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@...sung.com>
>> ---
>> drivers/misc/Kconfig | 1 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/stm32fwu/Kconfig | 6 +
>> drivers/misc/stm32fwu/Makefile | 1 +
>> drivers/misc/stm32fwu/stm32_core.c | 403 ++++++++++++++++++++++++++++++++++++
>> drivers/misc/stm32fwu/stm32_core.h | 81 ++++++++
>> drivers/misc/stm32fwu/stm32_spi.c | 108 ++++++++++
>> include/linux/stm32fwu.h | 49 +++++
>> 8 files changed, 650 insertions(+)
>> create mode 100644 drivers/misc/stm32fwu/Kconfig
>> create mode 100644 drivers/misc/stm32fwu/Makefile
>> create mode 100644 drivers/misc/stm32fwu/stm32_core.c
>> create mode 100644 drivers/misc/stm32fwu/stm32_core.h
>> create mode 100644 drivers/misc/stm32fwu/stm32_spi.c
>> create mode 100644 include/linux/stm32fwu.h
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index bbeb451..b2e68c1 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -528,4 +528,5 @@ source "drivers/misc/mic/Kconfig"
>> source "drivers/misc/genwqe/Kconfig"
>> source "drivers/misc/echo/Kconfig"
>> source "drivers/misc/cxl/Kconfig"
>> +source "drivers/misc/stm32fwu/Kconfig"
>> endmenu
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 7d5c4cd..88c8999 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -56,3 +56,4 @@ obj-$(CONFIG_GENWQE) += genwqe/
>> obj-$(CONFIG_ECHO) += echo/
>> obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
>> obj-$(CONFIG_CXL_BASE) += cxl/
>> +obj-$(CONFIG_STM32_UPGRADE_PROTOCOL) += stm32fwu/
>> diff --git a/drivers/misc/stm32fwu/Kconfig b/drivers/misc/stm32fwu/Kconfig
>> new file mode 100644
>> index 0000000..1484441
>> --- /dev/null
>> +++ b/drivers/misc/stm32fwu/Kconfig
>> @@ -0,0 +1,6 @@
>> +config STM32_UPGRADE_PROTOCOL
>> + tristate "STM32 upgrade protocol support"
>> + depends on SPI
>> + help
>> + STM32 microcontroller bootloader upgrade protocol.
>> + Say Y if you want to use it.
>> diff --git a/drivers/misc/stm32fwu/Makefile b/drivers/misc/stm32fwu/Makefile
>> new file mode 100644
>> index 0000000..1617530
>> --- /dev/null
>> +++ b/drivers/misc/stm32fwu/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_STM32_UPGRADE_PROTOCOL) += stm32_core.o stm32_spi.o
>> diff --git a/drivers/misc/stm32fwu/stm32_core.c b/drivers/misc/stm32fwu/stm32_core.c
>> new file mode 100644
>> index 0000000..4c48cd8
>> --- /dev/null
>> +++ b/drivers/misc/stm32fwu/stm32_core.c
>> @@ -0,0 +1,403 @@
>> +/*
>> + * Copyright (C) 2014, Samsung Electronics Co. Ltd. All Rights Reserved.
>> + *
>> + * 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 2 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.
>> + *
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/stm32fwu.h>
>> +#include "stm32_core.h"
>> +
>> +static inline int stm32fwu_wait_for_ack(struct stm32fwu_fw *fw, u32 retries)
>> +{
>> + return fw->wait_for_ack(fw, retries);
>> +}
>> +
>> +static inline int stm32fwu_send_cmd(struct stm32fwu_fw *fw, u8 cmd,
>> + u32 timeout)
>> +{
>> + struct stm32fwu_cmd cm = {
>> + .cmd = cmd,
>> + .neg_cmd = ~cmd,
>> + .timeout = timeout,
>> + };
>> +
>> + return fw->send_cmd(fw, &cm);
>> +}
>> +
>> +static inline int stm32fwu_write(struct stm32fwu_fw *fw, const u8 *buf,
>> + u32 len)
>> +{
>> + return fw->write(fw, buf, len);
>> +}
>> +
>> +static inline int stm32fwu_read(struct stm32fwu_fw *fw, u8 *buf, u32 len)
>> +{
>> + return fw->read(fw, buf, len);
>> +}
>> +
>> +static int send_addr(struct stm32fwu_fw *fw, u32 addr)
>> +{
>> + int ret;
>> + u8 *buffer = fw->buffer;
>> +
>> + buffer[0] = addr >> 24;
>> + buffer[1] = addr >> 16;
>> + buffer[2] = addr >> 8;
>> + buffer[3] = addr;
>> + buffer[4] = buffer[0] ^ buffer[1] ^ buffer[2] ^ buffer[3];
>> +
>> + ret = stm32fwu_write(fw, buffer, STM32FWU_ADDR_LEN);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return stm32fwu_wait_for_ack(fw, STM32FWU_COMMON_COUNT);
>> +}
>> +
>> +static int fwu_write(struct stm32fwu_fw *fw, u32 addr, const u8 *buffer,
>> + u32 len)
>> +{
>> + u8 *sbuf = fw->buffer;
>> + u8 xor = 0;
>> + int ret;
>> + u32 i, aligned_len = len;
>> +
>> + if (len > STM32FWU_MAX_TRANSFER_SIZE) {
>> + dev_err(fw->dev, "More than 256 bytes per transfer\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = stm32fwu_send_cmd(fw, STM32FWU_WRITE_MEM_CMD,
>> + STM32FWU_CMD_COUNT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = send_addr(fw, addr);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /*
>> + * the same buffer is used for commands and data so the order really
>> + * matters here
>> + */
>> + memcpy(&sbuf[1], buffer, len);
>> +
>> + /*
>> + * Just in case - check if chunk is a multiple of 4
>> + * generally the most of writes will be 256 bytes long.
>> + * I wonder if bootcode should worry about alignment.
>> + * According AN4286 it should be a multiple of 2 but other notes
>> + * say that it should be 4
>> + */
>> + if (!IS_ALIGNED(len, 4)) {
>> + aligned_len = ALIGN(len, 4);
>> + for (i = aligned_len; i > len; --i)
>> + sbuf[i] = 0xff;
>> + }
>> +
>> + /*
>> + * according AN4286 here is sent 0 < N < 255, it looks like max index,
>> + * not size
>> + */
>> + sbuf[0] = aligned_len - 1;
>> +
>> + /* compute checksum */
>> + for (i = 0; i < aligned_len + 1; ++i)
>> + xor ^= sbuf[i];
>> + sbuf[aligned_len + 1] = xor;
>> +
>> + /* NOTE AN4286: in some conditions the master has to wait 1 ms here */
>> + usleep_range(1000, 1100);
>
> I cannot find this 1 ms delay in AN4286.
> Is it really needed?
> By the way, here we are in the core file. You claim that the delay is
> required by SPI only (NOTE AN4286), so it should go in SPI files.
>
OK, I will be moved.
>> +
>> + ret = stm32fwu_write(fw, sbuf, aligned_len + 2);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return stm32fwu_wait_for_ack(fw, STM32FWU_COMMON_COUNT);
>> +}
>> +
>> +static int fwu_erase(struct stm32fwu_fw *fw)
>> +{
>> + int ret;
>> + u8 *sbuf = fw->buffer;
>> +
>> + ret = stm32fwu_send_cmd(fw, STM32FWU_ERASE_CMD, STM32FWU_CMD_COUNT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* global erase */
>> + sbuf[0] = 0xff;
>> + sbuf[1] = 0xff;
>> + sbuf[2] = 0x00;
>> +
>> + ret = stm32fwu_write(fw, sbuf, STM32FWU_ERASE_CMD_LEN);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return stm32fwu_wait_for_ack(fw, STM32FWU_ERASE_COUNT);
>> +}
>> +
>> +/**
>> + * stm32fwu_get() - gets bootloader information frame
>> + * @fw: fw object.
>> + *
>> + * Read bootloader information:
>> + *
>> + * Byte 1 bootloader version (0 < version < 255), example: 0x10 = version 1.0.
>> + *
>> + * Byte 2 0x00 (Get command)
>> + *
>> + * Byte 3 0x01 (Get Version)
>> + *
>> + * Byte 4 0x02 (Get ID)
>> + *
>> + * Byte 5 0x11 (Read Memory command)
>> + *
>> + * Byte 6 0x21 (Go command)
>> + *
>> + * Byte 7 0x31 (Write Memory command)
>> + *
>> + * Byte 8 0x44 (Erase command)
>> + *
>> + * Byte 9 0x63 (Write Protect command)
>> + *
>> + * Byte 10 0x73 (Write Unprotect command)
>> + *
>> + * Byte 11 0x82 (Readout Protect command)
>> + *
>> + * Byte 12 0x92 (Readout Unprotect command)
>> + *
>> + * Return: read byte count or error code.
>> + */
>> +int stm32fwu_get(struct stm32fwu_fw *fw)
>> +{
>> + int ret, count;
>> +
>> + ret = stm32fwu_send_cmd(fw, STM32FWU_GET_CMD, STM32FWU_CMD_COUNT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = stm32fwu_read(fw, &fw->buffer[STM32FWU_MAX_BUFFER_SIZE], 1);
>> + if (ret < 0)
>> + return ret;
>> +
>> + count = fw->buffer[STM32FWU_MAX_BUFFER_SIZE];
>> + if (count >= STM32FWU_MAX_TRANSFER_SIZE)
>> + return -EINVAL;
>
> AN4286 specifies that a dummy byte have to be read before reading data
> (page 7, Figure 5).
> With current code you would get the dummy byte as first byte in the buffer.
> This dummy byte is specific for SPI so the byte skip should better be
> in SPI files.
OK
>
>> +
>> + ret = stm32fwu_read(fw, &fw->buffer[STM32FWU_MAX_BUFFER_SIZE], count);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = stm32fwu_wait_for_ack(fw, STM32FWU_COMMON_COUNT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return count;
>> +}
>> +EXPORT_SYMBOL(stm32fwu_get);
>> +
>> +/**
>> + * stm32fwu_get_version() - gets bootloader information frame
>> + * @fw: fw object.
>> + *
>> + * Return: < 0 - error code, any positive value means success.
>> + */
>> +int stm32fwu_get_version(struct stm32fwu_fw *fw)
>> +{
>> + int ret;
>> +
>> + ret = stm32fwu_send_cmd(fw, STM32FWU_GET_CMD, STM32FWU_CMD_COUNT);
>> + if (ret < 0)
>> + return ret;
>
> Also here, as for stm32fwu_get(), you need to skip one byte before
> reading the data.
>
>> +
>> + ret = stm32fwu_read(fw, &fw->buffer[STM32FWU_MAX_BUFFER_SIZE], 1);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = stm32fwu_wait_for_ack(fw, STM32FWU_COMMON_COUNT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return fw->buffer[STM32FWU_MAX_BUFFER_SIZE];
>> +}
>> +EXPORT_SYMBOL(stm32fwu_get_version);
>> +
>> +/**
>> + * stm32fwu_send_sync() - sends bootloader synchronization frame
>> + * @fw: fw object.
>> + *
>> + * It is used during SPI upgrade.
>> + *
>> + * Return: if succeed the number of ack waiting cycles or error code.
>> + */
>
> This sync is specific for SPI. Should be in another file.
>
OK, I will be moved.
>> +int stm32fwu_send_sync(struct stm32fwu_fw *fw)
>> +{
>> + int ret;
>> +
>> + dev_info(fw->dev, "send sync byte for upgrade\n");
>> +
>> + fw->buffer[0] = STM32FWU_SPI_SOF;
>> +
>> + ret = stm32fwu_write(fw, fw->buffer, 1);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return stm32fwu_wait_for_ack(fw, STM32FWU_CMD_COUNT);
>> +}
>> +EXPORT_SYMBOL(stm32fwu_send_sync);
>> +
>> +/**
>> + * stm32fwu_update() - runs all firmware update work
>> + * @fw: fw object.
>> + *
>> + * Return: status code < 0 if error.
>> + */
>> +int stm32fwu_update(struct stm32fwu_fw *fw)
>> +{
>> + int ret = 0;
>> + u32 pos = 0;
>> + u32 fw_addr = STM32FWU_APP_ADDR;
>> + u32 block = STM32FWU_MAX_TRANSFER_SIZE;
>> + u32 count = 0, err_count = 0, retry_count = 0, remaining = fw->fw_len;
>> +
>> + dev_info(fw->dev, "%s start\n", __func__);
>> +
>> + ret = fwu_erase(fw);
>> + if (ret < 0) {
>> + dev_err(fw->dev, "%s, fw_erase_stm failed %d\n", __func__,
>> + ret);
>> + return ret;
>> + }
>> +
>> + while (remaining > 0) {
>> + if (block > remaining)
>> + block = remaining;
>> +
>> + while (retry_count < 3) {
>> + ret = fwu_write(fw, fw_addr, fw->fw_data + pos, block);
>> + if (ret < 0) {
>> + dev_err(fw->dev,
>> + "Err %d writing to addr 0x%08X\n", ret,
>> + fw_addr);
>> + retry_count++;
>> + err_count++;
>> + continue;
>> + }
>> + retry_count = 0;
>> + break;
>> + }
>> +
>> + if (ret < 0) {
>> + dev_err(fw->dev,
>> + "Writing MEM failed: %d, retry count: %d\n",
>> + ret, err_count);
>> + return ret;
>> + }
>> +
>> + remaining -= block;
>> + pos += block;
>> + fw_addr += block;
>> + if (count++ == 20) {
>> + dev_info(fw->dev, "Updated %u bytes / %u bytes\n",
>> + pos, fw->fw_len);
>> + count = 0;
>> + }
>> + }
>> +
>> + dev_info(fw->dev,
>> + "Firmware download success.(%d bytes, retry %d)\n", pos,
>> + err_count);
>> +
>> + ret = stm32fwu_send_cmd(fw, STM32FWU_GO_ADDR_CMD, STM32FWU_CMD_COUNT);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return send_addr(fw, STM32FWU_APP_ADDR);
>> +}
>> +EXPORT_SYMBOL(stm32fwu_update);
>> +
>> +/**
>> + * stm32fwu_init() - creates firmware upgrade instance
>> + * @dev: Pointer to device.
>> + * @iface: Interface type.
>> + * @data: Pointer to firmware data.
>> + * @len: Length of firmware data in bytes.
>> + *
>> + * It allocates the place for fwu structure and transfer buffer so it should
>> + * be deinitialized by stm32fw_destroy(). Firmware upgrade is simple operation
>> + * and it was assumed that it will be called from one context so there is no
>> + * locking for fwu functions.
>> + *
>> + * Return: Pointer to firmware upgrade structure.
>> + */
>> +struct stm32fwu_fw *stm32fwu_init(struct device *dev,
>> + enum stm32fwu_iface iface, const u8 *data,
>> + u32 len)
>> +{
>> + struct stm32fwu_fw *fw;
>> +
>> + if (iface >= STM32_MAX || iface < 0) {
>> + dev_err(dev, "wrong iface type\n");
>> + return NULL;
>> + }
>> +
>> + fw = devm_kzalloc(dev, sizeof(*fw), GFP_KERNEL);
>> + if (fw == NULL)
>> + return NULL;
>> +
>> + /* 2 times for tx and rx buffer */
>> + fw->buffer = devm_kzalloc(dev, 2 * STM32FWU_MAX_BUFFER_SIZE,
>> + GFP_KERNEL | GFP_DMA);
>> + if (fw->buffer == NULL) {
>> + devm_kfree(dev, fw);
>> + return NULL;
>> + }
>> +
>> + fw->dev = dev;
>> + fw->fw_len = len;
>> + fw->fw_data = data;
>> +
>> + switch (iface) {
>> + case STM32_SPI:
>> + stm32fwu_spi_init(fw);
>> + break;
>> + default:
>> + pr_err("wrong interface\n");
>> + goto _err;
>> + }
>> +
>> + return fw;
>> +_err:
>> + devm_kfree(dev, fw->buffer);
>> + devm_kfree(dev, fw);
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL(stm32fwu_init);
>> +
>> +/**
>> + * stm32fwu_destroy() - cleans up fwu structure and buffer.
>> + * @fw: Pointer to fwu structure.
>> + */
>> +void stm32fwu_destroy(struct stm32fwu_fw *fw)
>> +{
>> + devm_kfree(fw->dev, fw->buffer);
>> + devm_kfree(fw->dev, fw);
>> +}
>> +EXPORT_SYMBOL(stm32fwu_destroy);
>> +
>> +MODULE_AUTHOR("Karol Wrona <k.wrona@...sung.com>");
>> +MODULE_DESCRIPTION("STM32 upgrade protocol");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/misc/stm32fwu/stm32_core.h b/drivers/misc/stm32fwu/stm32_core.h
>> new file mode 100644
>> index 0000000..9624327
>> --- /dev/null
>> +++ b/drivers/misc/stm32fwu/stm32_core.h
>> @@ -0,0 +1,81 @@
>> +/*
>> + * Copyright (C) 2014, Samsung Electronics Co. Ltd. All Rights Reserved.
>> + *
>> + * 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 2 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.
>> + *
>> + */
>> +
>> +#ifndef _STM32_CORE_H_
>> +#define _STM32_CORE_H_
>> +
>> +#define STM32FWU_ADDR_LEN 5
>> +#define STM32FWU_ERASE_CMD_LEN 3
>> +
>> +/* Protocol */
>> +#define STM32FWU_SPI_SOF 0x5a
>> +#define STM32FWU_ACK 0x79
>> +#define STM32FWU_ACK2 0xf9
>
> I cannot find any reference to this ACK2 0xf9 in any STM32 document.
> Can you please point to the right document?
>
Tis is confusing. I left this because this was used in some company sources but
I think it can be removed. I will check it.
>> +#define STM32FWU_NACK 0x1f
>> +
>> +#define STM32FWU_MAX_TRANSFER_SIZE 256
>> +#define STM32FWU_MAX_BUFFER_SIZE 260
>> +
>> +#define STM32FWU_APP_ADDR 0x08000000
>> +
>> +/* Retries counts */
>> +#define STM32FWU_ERASE_COUNT 18000
>> +#define STM32FWU_CMD_COUNT 30
>> +#define STM32FWU_COMMON_COUNT 20
>> +
>> +/* Commands */
>> +#define STM32FWU_WRITE_MEM_CMD 0x31
>> +#define STM32FWU_GO_ADDR_CMD 0x21
>> +#define STM32FWU_ERASE_CMD 0x44
>> +#define STM32FWU_GET_CMD 0x00
>> +#define STM32FWU_GET_VERSION_CMD 0x01
>> +
>> +/**
>> + * struct stm32fwu_cmd - Upgrade command
>> + * @cmd: Fwu command.
>> + * @neg_cmd: Bit negation of Fwu command used xor'ed in STM.
>> + * @timeout: Number of retries.
>> + */
>> +struct stm32fwu_cmd {
>> + u8 cmd;
>> + u8 neg_cmd;
>> + u32 timeout;
>> +};
>> +
>> +/**
>> + * struct stm32fwu_fw - Generic representation for STM32 fw upgrade
>> + * @dev: Pointer to device.
>> + * @wait_for_ack: ACK callback.
>> + * @send_cmd: Command callback.
>> + * @write: Write callback.
>> + * @read: Read callback.
>> + * @fw_data: Pointer to firmware binary.
>> + * @fw_len: The length of fw bin.
>> + * @buffer: Pointer to SPI buffer: should be DMA safe
>> + */
>> +struct stm32fwu_fw {
>> + int (*wait_for_ack)(struct stm32fwu_fw *fw, u32 retries);
>> + int (*send_cmd)(struct stm32fwu_fw *fw, struct stm32fwu_cmd *cmd);
>> + int (*write)(struct stm32fwu_fw *fw, const u8 *buf, u32 len);
>> + int (*read)(struct stm32fwu_fw *fw, u8 *buf, u32 len);
>> + const u8 *fw_data;
>> + u32 fw_len;
>> + u8 *buffer;
>> + struct device *dev;
>> +};
>> +
>> +void stm32fwu_spi_init(struct stm32fwu_fw *fw);
>> +
>> +#endif /*_STM32_CORE_H_ */
>> diff --git a/drivers/misc/stm32fwu/stm32_spi.c b/drivers/misc/stm32fwu/stm32_spi.c
>> new file mode 100644
>> index 0000000..7905baf
>> --- /dev/null
>> +++ b/drivers/misc/stm32fwu/stm32_spi.c
>> @@ -0,0 +1,108 @@
>> +/*
>> + * Copyright (C) 2014, Samsung Electronics Co. Ltd. All Rights Reserved.
>> + *
>> + * 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 2 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.
>> + *
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/module.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/stm32fwu.h>
>> +#include "stm32_core.h"
>> +
>> +static int stm32fwu_spi_write(struct stm32fwu_fw *fw, const u8 *buf, u32 len)
>> +{
>> + struct spi_device *sdev = to_spi_device(fw->dev);
>> +
>> + struct spi_message m;
>> + struct spi_transfer t = {
>> + .len = len,
>> + .tx_buf = buf,
>> + .bits_per_word = 8,
>> + };
>> +
>> + spi_message_init(&m);
>> + spi_message_add_tail(&t, &m);
>> + return spi_sync(sdev, &m);
>> +}
>> +
>> +static int stm32fwu_spi_read(struct stm32fwu_fw *fw, u8 *buf, u32 len)
>> +{
>> + struct spi_device *sdev = to_spi_device(fw->dev);
>> +
>> + struct spi_message m;
>> + struct spi_transfer t = {
>> + .len = len,
>> + .rx_buf = buf,
>> + .bits_per_word = 8,
>> + };
>> +
>> + spi_message_init(&m);
>> + spi_message_add_tail(&t, &m);
>
> This is the proper place in SPI specific file where you should skip
> the first dummy byte before read data.
> But then you cannot re-use this function to wait for the ACK; need
> another read function for it.
>
>> + return spi_sync(sdev, &m);
>> +}
>> +
>> +static int stm32fwu_spi_wait_for_ack(struct stm32fwu_fw *fw, u32 retries)
>> +{
>> + int ret, i = 0;
>> + u8 *buf = &fw->buffer[STM32FWU_MAX_BUFFER_SIZE];
>> +
>
> According to AN4286, here you should first skip one dummy byte, then
> read ACK (and then send back ACK).
> In the loop below you skip everything that is not ACK (or ACK2), so
> probably it works fine and skips the dummy byte too.
> But AN4286 does not specify the value of the dummy byte to skip. What
> if it is equal to ACK?
>
>> + while (i < retries) {
>> + ret = stm32fwu_spi_read(fw, buf, 1);
>> + if (ret < 0) {
>> + dev_err(fw->dev,
>> + "firmware upgrade wait for ack fail\n");
>> + return ret;
>> + }
>> +
>> + if (fw->buffer[STM32FWU_MAX_BUFFER_SIZE] == STM32FWU_ACK ||
>> + fw->buffer[STM32FWU_MAX_BUFFER_SIZE] == STM32FWU_ACK2) {
>
> Again, no evidence of ACK2 in documentation.
>
> Also, you cannot just return. AN4286 (figure 2) requires to send back
> a byte ACK to the bootloader.
>
>> + return i;
>> + }
>> +
>> + if (fw->buffer[STM32FWU_MAX_BUFFER_SIZE] == STM32FWU_NACK)
>
> Also here, send back a byte ACK to bootloader before return.
>
> Regards,
> Antonio
>
>> + return -EPROTO;
>> +
>> + usleep_range(1000, 1100);
>> + i++;
>> + }
>> +
>> + dev_err(fw->dev, "fw ack timeout\n");
>> +
>> + return -ETIMEDOUT;
>> +}
>> +
>> +static int stm32fwu_spi_send_cmd(struct stm32fwu_fw *fw,
>> + struct stm32fwu_cmd *cmd)
>> +{
>> + int ret;
>> +
>> + fw->buffer[0] = STM32FWU_SPI_SOF;
>> + fw->buffer[1] = cmd->cmd;
>> + fw->buffer[2] = cmd->neg_cmd;
>> +
>> + ret = stm32fwu_spi_write(fw, fw->buffer, 3);
>> + if (ret < 0) {
>> + dev_err(fw->dev, "fw cmd write fail\n");
>> + return ret;
>> + }
>> +
>> + return stm32fwu_spi_wait_for_ack(fw, cmd->timeout);
>> +}
>> +
>> +void stm32fwu_spi_init(struct stm32fwu_fw *fw)
>> +{
>> + fw->write = stm32fwu_spi_write;
>> + fw->read = stm32fwu_spi_read;
>> + fw->wait_for_ack = stm32fwu_spi_wait_for_ack;
>> + fw->send_cmd = stm32fwu_spi_send_cmd;
>> +}
>> diff --git a/include/linux/stm32fwu.h b/include/linux/stm32fwu.h
>> new file mode 100644
>> index 0000000..3084ffd
>> --- /dev/null
>> +++ b/include/linux/stm32fwu.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + * Copyright (C) 2014, Samsung Electronics Co. Ltd. All Rights Reserved.
>> + *
>> + * 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 2 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.
>> + *
>> + */
>> +
>> +#ifndef _STM32FWU_H_
>> +#define _STM32FWU_H_
>> +
>> +/**
>> + * enum stm32fwu_iface - upgrade interfaces for STM32 MCU's
>> + * @STM32_SPI: SPI interface.
>> + * @STM32_MAX: terminator.
>> + */
>> +enum stm32fwu_iface {
>> + STM32_SPI,
>> + /*
>> + * Generally for future use, i.e. UART upgrade algorithm looks pretty
>> + * similar.
>> + */
>> + STM32_MAX
>> +};
>> +
>> +struct stm32fwu_fw;
>> +
>> +struct stm32fwu_fw *stm32fwu_init(struct device *dev,
>> + enum stm32fwu_iface iface, const u8 *data,
>> + u32 len);
>> +
>> +void stm32fwu_destroy(struct stm32fwu_fw *fw);
>> +
>> +int stm32fwu_send_sync(struct stm32fwu_fw *fw);
>> +
>> +int stm32fwu_update(struct stm32fwu_fw *fw);
>> +
>> +int stm32fwu_get(struct stm32fwu_fw *fw);
>> +
>> +int stm32fwu_get_version(struct stm32fwu_fw *fw);
>> +
>> +#endif /* _STM32FWU_H_ */
>> --
>> 1.7.9.5
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Thanks,
Karol
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists