Re: [PATCH v3 2/6] usb: typec: ucsi: add ccg command framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ajay,

Sorry for the late response, but I hit a compiler warning after doing
bisect test.

On Fri, Feb 01, 2019 at 05:05:06PM -0800, Ajay Gupta wrote:
> +/* CCGx response codes */
> +enum ccg_resp_code {
> +	CMD_NO_RESP             = 0x00,
> +	CMD_SUCCESS             = 0x02,
> +	FLASH_DATA_AVAILABLE    = 0x03,
> +	CMD_INVALID             = 0x05,
> +	FLASH_UPDATE_FAIL       = 0x07,
> +	INVALID_FW              = 0x08,
> +	INVALID_ARG             = 0x09,
> +	CMD_NOT_SUPPORT         = 0x0A,
> +	TRANSACTION_FAIL        = 0x0C,
> +	PD_CMD_FAIL             = 0x0D,
> +	UNDEF_ERROR             = 0x0F,
> +	INVALID_RESP		= 0x10,
> +};

Side note. There are no users for these, so I'm not sure you need to
add them in this patch.

> +#define CCG_EVENT_MAX	(EVENT_INDEX + 43)
> +
> +struct ccg_cmd {
> +	u16 reg;
> +	u32 data;
> +	int len;
> +	int delay; /* ms delay for cmd timeout  */
> +};
> +
> +struct ccg_resp {
> +	u8 code;
> +	u8 length;
> +};
> +
>  struct ucsi_ccg {
>  	struct device *dev;
>  	struct ucsi *ucsi;
> @@ -58,17 +111,14 @@ struct ucsi_ccg {
>  	struct ccg_dev_info info;
>  	/* version info for boot, primary and secondary */
>  	struct version_info version[FW2 + 1];
> +	/* CCG HPI communication flags */
> +	unsigned long flags;
> +#define RESET_PENDING	0
> +#define DEV_CMD_PENDING	1
> +	struct ccg_resp dev_resp;
> +	u8 cmd_resp;
>  };
>  
> -#define CCGX_RAB_DEVICE_MODE			0x0000
> -#define CCGX_RAB_INTR_REG			0x0006
> -#define CCGX_RAB_READ_ALL_VER			0x0010
> -#define CCGX_RAB_READ_FW2_VER			0x0020
> -#define CCGX_RAB_UCSI_CONTROL			0x0039
> -#define CCGX_RAB_UCSI_CONTROL_START		BIT(0)
> -#define CCGX_RAB_UCSI_CONTROL_STOP		BIT(1)
> -#define CCGX_RAB_UCSI_DATA_BLOCK(offset)	(0xf000 | ((offset) & 0xff))
> -
>  static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
>  {
>  	struct i2c_client *client = uc->client;
> @@ -276,6 +326,110 @@ static int get_fw_info(struct ucsi_ccg *uc)
>  	return 0;
>  }
>  
> +static inline bool invalid_async_evt(int code)
> +{
> +	return (code >= CCG_EVENT_MAX) || (code < EVENT_INDEX);
> +}
> +
> +static void ccg_process_response(struct ucsi_ccg *uc)
> +{
> +	struct device *dev = uc->dev;
> +
> +	if (uc->dev_resp.code & ASYNC_EVENT) {
> +		if (uc->dev_resp.code == RESET_COMPLETE) {
> +			if (test_bit(RESET_PENDING, &uc->flags))
> +				uc->cmd_resp = uc->dev_resp.code;
> +			get_fw_info(uc);
> +		}
> +		if (invalid_async_evt(uc->dev_resp.code))
> +			dev_err(dev, "invalid async evt %d\n",
> +				uc->dev_resp.code);
> +	} else {
> +		if (test_bit(DEV_CMD_PENDING, &uc->flags)) {
> +			uc->cmd_resp = uc->dev_resp.code;
> +			clear_bit(DEV_CMD_PENDING, &uc->flags);
> +		} else {
> +			dev_err(dev, "dev resp 0x%04x but no cmd pending\n",
> +				uc->dev_resp.code);
> +		}
> +	}
> +}
> +
> +static int ccg_read_response(struct ucsi_ccg *uc)
> +{
> +	unsigned long target = jiffies + msecs_to_jiffies(1000);
> +	struct device *dev = uc->dev;
> +	u8 intval;
> +	int status;
> +
> +	/* wait for interrupt status to get updated */
> +	do {
> +		status = ccg_read(uc, CCGX_RAB_INTR_REG, &intval,
> +				  sizeof(intval));
> +		if (status < 0)
> +			return status;
> +
> +		if (intval & DEV_INT)
> +			break;
> +		usleep_range(500, 600);
> +	} while (time_is_after_jiffies(target));
> +
> +	if (time_is_before_jiffies(target)) {
> +		dev_err(dev, "response timeout error\n");
> +		return -ETIME;
> +	}
> +
> +	status = ccg_read(uc, CCGX_RAB_RESPONSE, (u8 *)&uc->dev_resp,
> +			  sizeof(uc->dev_resp));
> +	if (status < 0)
> +		return status;
> +
> +	status = ccg_write(uc, CCGX_RAB_INTR_REG, &intval, sizeof(intval));
> +	if (status < 0)
> +		return status;
> +
> +	return 0;
> +}
> +
> +/* Caller must hold uc->lock */
> +static int ccg_send_command(struct ucsi_ccg *uc, struct ccg_cmd *cmd)
> +{
> +	struct device *dev = uc->dev;
> +	int ret;
> +
> +	switch (cmd->reg & 0xF000) {
> +	case DEV_REG_IDX:
> +		set_bit(DEV_CMD_PENDING, &uc->flags);
> +		break;
> +	default:
> +		dev_err(dev, "invalid cmd register\n");
> +		break;
> +	}
> +
> +	ret = ccg_write(uc, cmd->reg, (u8 *)&cmd->data, cmd->len);
> +	if (ret < 0)
> +		return ret;
> +
> +	msleep(cmd->delay);
> +
> +	ret = ccg_read_response(uc);
> +	if (ret < 0) {
> +		dev_err(dev, "response read error\n");
> +		switch (cmd->reg & 0xF000) {
> +		case DEV_REG_IDX:
> +			clear_bit(DEV_CMD_PENDING, &uc->flags);
> +			break;
> +		default:
> +			dev_err(dev, "invalid cmd register\n");
> +			break;
> +		}
> +		return -EIO;
> +	}
> +	ccg_process_response(uc);
> +
> +	return uc->cmd_resp;
> +}

I did my little bisect test, and noticed that this is giving a
compiler warning. Nothing is calling the function yet.


thanks,

-- 
heikki



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux