RE: [PATCH] driver: input :touchscreen : add Raydium I2C touch driver

From: "Jeffrey Lin (林義章)" <Jeffrey.Lin@rad-ic.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	"jeffrey.lin" <yajohn@gmail.com>
Cc: "rydberg@euromail.se" <rydberg@euromail.se>,
	"grant.likely@linaro.org" <grant.likely@linaro.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"jeesw@melfas.com" <jeesw@melfas.com>,
	"bleung@chromium.org" <bleung@chromium.org>,
	"scott.liu@emc.com.tw" <scott.liu@emc.com.tw>,
	"Roger Yang (楊鎮瑋)" <Roger.Yang@rad-ic.com>,
	"KP Li (李昆倍)" <KP.Li@rad-ic.com>,
	"Albert Shieh (謝欣瑋)" <Albert.Shieh@rad-ic.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: RE: [PATCH] driver: input :touchscreen : add Raydium I2C touch driver
Date: Mon, 11 Apr 2016 09:57:21 +0000	[thread overview]
Message-ID: <9BE4B49389AB2F41A64A32DFC844F1DF73F1BDB8@RADHCMS5.corp.rad-ic.com> (raw)
In-Reply-To: <20160411082428.GA26066@dtor-ws>

Hi Dmitry:
Thank you for your response.
	Actually, we didn't implement FW update function on the chromeos device, we just copy function from our previous android device. I'll fix that on the next version. Raydium controller doesn't have access bytes limit, but my previous android experimental device had 60 bytes limit.

About reformatting command structure, do you want me to reform all my commands as you said in the mail or just in case of fw update function? If not just in case of FW update, it's better to meet maximum buffer size, like "MAX_PACKET_SIZE".

Best Regards
----------------------------------------------------------------------
Jeffrey Lin,林義章
瑞鼎科技
Raydium Semiconductor Corporation
Tel:(03)666-1818 Ext.4163
Fax:(03)666-1919

-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
Sent: Monday, April 11, 2016 4:24 PM
To: jeffrey.lin
Cc: rydberg@euromail.se; grant.likely@linaro.org; robh+dt@kernel.org; jeesw@melfas.com; bleung@chromium.org; scott.liu@emc.com.tw; Jeffrey Lin (林義章); Roger Yang (楊鎮瑋); KP Li (李昆倍); Albert Shieh (謝欣瑋); linux-kernel@vger.kernel.org; linux-input@vger.kernel.org; devicetree@vger.kernel.org
Subject: Re: [PATCH] driver: input :touchscreen : add Raydium I2C touch driver

Hi Jeffrey,

On Fri, Mar 25, 2016 at 01:21:09PM +0800, jeffrey.lin wrote:
> +#define MAX_PACKET_SIZE		60
...
> +#define RAYDIUM_FW_PAGESIZE	128
...
> +
> +static int raydium_i2c_send(struct i2c_client *client,
> +	u8 addr, u8 *data, size_t len)
> +{
> +	u8 buf[MAX_PACKET_SIZE + 1];
> +	int tries = 0;
> +
> +	if (len > MAX_PACKET_SIZE)
> +		return -EINVAL;
...
> +static int raydium_i2c_fw_write_page(struct i2c_client *client,
> +				const void *page)
> +{
> +	static const u8 ack_ok[] = { 0x55, 0xAA };
> +	u8 buf[2];
> +	int retry;
> +	int error;
> +
> +	for (retry = 0; retry < MAX_FW_UPDATE_RETRIES; retry++) {
> +		error = raydium_i2c_send(client, CMD_BOOT_WRT,
> +			(u8 *)page, RAYDIUM_FW_PAGESIZE);
> +		if (error) {
> +			dev_err(&client->dev,
> +				"BLDR Write Page failed: %d\n", error);
> +			continue;
> +		}

Given the above definitions of MAX_PACKET_SIZE and RAYDIUM_FW_PAGESIZE I do not believe that firmware update is working.
What is the biggest buffer that can be sent to the device? Maybe we should allocate it dynamically?

I am also wondering about formatting command structure as byte sequence.
Would it be better to define it as:

struct raidium_cmd {
	u32 bank;
	int len;
	u8 cmd[RAIDIUM_MAX_CMD_LEN];
}

What is the longest command that the controller supports?

(Do not resubmit the driver yet, let's discuss first).

Thanks.

--
Dmitry