[v4,2/2] usb: typec: ucsi: add firmware flashing support

[Oliver Neukum <oneukum@suse.com>]

On Mi, 2019-02-06 at 13:39 -0800, Ajay Gupta wrote:
> From: Ajay Gupta <ajayg@nvidia.com>

Hi,

> 
> CCGx has two copies of the firmware in addition to the bootloader.
> If the device is running FW1, FW2 can be updated with the new version.
> Dual firmware mode allows the CCG device to stay in a PD contract and
> support USB PD and Type-C functionality while a firmware update is in
> progress.
> 
> First we read the currently flashed firmware version of both
> primary and secondary firmware and then compare it with
> version of firmware file to determine if flashing is required.

I am wondering about the security implications of this.
You are persistently changing firmware of a device. It looks
to me like there ought to be a check for capable'(CFAP_SYS_RAWIO)
in the firmware update path.

> Command framework is added to support sending commands to CCGx
> controller. We wait for response after sending the command and then
> read the response from RAB_RESPONSE register.
> 
> Below commands are supported,
>         - ENTER_FLASHING
>         - RESET
>         - PDPORT_ENABLE
>         - JUMP_TO_BOOT
>         - FLASH_ROW_RW
>         - VALIDATE_FW
> 
> Command specific mutex lock is also added to sync between driver
> and user threads.
> 
> PD port number information is added which is required while sending
> PD_PORT_ENABLE command
> 
> Signed-off-by: Ajay Gupta <ajayg@nvidia.com>

[..]

> +struct ccg_cmd {
> +	u16 reg;
> +	u32 data;
> +	int len;
> +	int delay; /* ms delay for cmd timeout  */

unsigned? A negative delay is an interesting concept.

[..]

> +static int ccg_cmd_reset(struct ucsi_ccg *uc, bool extra_delay)
> +{
> +	struct ccg_cmd cmd;
> +	u8 *p;
> +	int ret;
> +
> +	p = (u8 *)&cmd.data;
> +	cmd.reg = CCGX_RAB_RESET_REQ;
> +	p[0] = RESET_SIG;
> +	p[1] = CMD_RESET_DEV;
> +	cmd.len = 2;
> +	cmd.delay = 2000 + (extra_delay ? 3000 : 0);

This is a maximum, right?
If a reset fails, this is very bad. So why not always allow
the maximum timeout?

> +	mutex_lock(&uc->lock);
> +
> +	set_bit(RESET_PENDING, &uc->flags);
> +
> +	ret = ccg_send_command(uc, &cmd);
> +	if (ret != RESET_COMPLETE)
> +		goto err_clear_flag;
> +
> +	ret = 0;
> +
> +err_clear_flag:
> +	clear_bit(RESET_PENDING, &uc->flags);
> +
> +	mutex_unlock(&uc->lock);
> +
> +	return ret;
> +}

[..]

> +static int
> +ccg_cmd_write_flash_row(struct ucsi_ccg *uc, u16 row,
> +			const void *data, u8 fcmd)
> +{
> +	struct i2c_client *client = uc->client;
> +	struct ccg_cmd cmd;
> +	u8 buf[CCG4_ROW_SIZE + 2];
> +	u8 *p;
> +	int ret;
> +
> +	/* Copy the data into the flash read/write memory. */
> +	buf[0] = REG_FLASH_RW_MEM & 0xFF;
> +	buf[1] = REG_FLASH_RW_MEM >> 8;

We have macros for converting endianness. Please use them. That is
a kind of generic issue with this patch.

> +
> +	memcpy(buf + 2, data, CCG4_ROW_SIZE);
> +
> +	mutex_lock(&uc->lock);
> +
> +	ret = i2c_master_send(client, buf, CCG4_ROW_SIZE + 2);
> +	if (ret != CCG4_ROW_SIZE + 2) {
> +		dev_err(uc->dev, "REG_FLASH_RW_MEM write fail %d\n", ret);
> +		return ret < 0 ? ret : -EIO;
> +	}
> +
> +	/* Use the FLASH_ROW_READ_WRITE register to trigger */
> +	/* writing of data to the desired flash row */
> +	p = (u8 *)&cmd.data;
> +	cmd.reg = CCGX_RAB_FLASH_ROW_RW;
> +	p[0] = FLASH_SIG;
> +	p[1] = fcmd;
> +	p[2] = row & 0xFF;
> +	p[3] = row >> 8;

Again. The macros.

> +	cmd.len = 4;
> +	cmd.delay = 50;
> +	if (fcmd == FLASH_FWCT_SIG_WR_CMD)
> +		cmd.delay += 400;
> +	if (row == 510)
> +		cmd.delay += 220;
> +	ret = ccg_send_command(uc, &cmd);
> +
> +	mutex_unlock(&uc->lock);
> +
> +	if (ret != CMD_SUCCESS) {
> +		dev_err(uc->dev, "write flash row failed ret=%d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}