| 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
| ||
|
Date: Tue, 8 Sep 2020 12:42:51 +0300
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Tomasz Figa <tfiga@...omium.org>
Cc: linux-media@...r.kernel.org,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Hao He <hao.he@...land.com.cn>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, drinkcat@...omium.org,
Xingyu Wu <wuxy@...land.com.cn>, dongchun.zhu@...iatek.com,
sj.huang@...iatek.com, darfur_liu@...reinc.com, hao.he7@...il.com
Subject: Re: [PATCH v4 4/4] media: i2c: gc5035: Add OTP configuration handling
Hi Tomasz,
Thanks for the patch.
On Wed, Sep 02, 2020 at 10:48:13PM +0000, Tomasz Figa wrote:
> From: Hao He <hao.he@...land.com.cn>
>
> The sensor OTP holds values for various configuration registers
> deteremined at manufacturing time and dead pixel correction tables. Add
> code to load both from the OTP and initialize the sensor appropriately.
>
> Signed-off-by: Hao He <hao.he@...land.com.cn>
> Signed-off-by: Xingyu Wu <wuxy@...land.com.cn>
> Signed-off-by: Tomasz Figa <tfiga@...omium.org>
> ---
> drivers/media/i2c/gc5035.c | 478 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 478 insertions(+)
>
> diff --git a/drivers/media/i2c/gc5035.c b/drivers/media/i2c/gc5035.c
> index 12e1b3a430b5..61645cec6948 100644
> --- a/drivers/media/i2c/gc5035.c
> +++ b/drivers/media/i2c/gc5035.c
> @@ -81,6 +81,57 @@
> #define GC5035_TEST_PATTERN_ENABLE 0x11
> #define GC5035_TEST_PATTERN_DISABLE 0x10
>
> +/* Page 2 registers */
> +
> +/* OTP access registers */
> +#define GC5035_REG_OTP_MODE 0xf3
> +#define GC5035_OTP_PRE_READ 0x20
> +#define GC5035_OTP_READ_MODE 0x12
> +#define GC5035_OTP_READ_DONE 0x00
> +#define GC5035_REG_OTP_DATA 0x6c
> +#define GC5035_REG_OTP_ACCESS_ADDR_H 0x69
> +#define GC5035_REG_OTP_ACCESS_ADDR_L 0x6a
> +#define GC5035_OTP_ACCESS_ADDR_H_MASK 0x1f
> +#define GC5035_OTP_ADDR_MASK 0x1fff
> +#define GC5035_OTP_ADDR_SHIFT 3
> +#define GC5035_REG_DD_TOTALNUM_H 0x01
> +#define GC5035_REG_DD_TOTALNUM_L 0x02
> +#define GC5035_DD_TOTALNUM_H_MASK 0x07
> +#define GC5035_REG_DD_LOAD_STATUS 0x06
> +#define GC5035_OTP_BIT_LOAD BIT(0)
> +
> +/* OTP-related definitions */
> +
> +#define GC5035_OTP_ID_SIZE 9
> +#define GC5035_OTP_ID_DATA_OFFSET 0x0020
> +#define GC5035_OTP_DATA_LENGTH 1024
> +
> +/* OTP DPC parameters */
> +#define GC5035_OTP_DPC_FLAG_OFFSET 0x0068
> +#define GC5035_OTP_DPC_FLAG_MASK 0x03
> +#define GC5035_OTP_FLAG_EMPTY 0x00
> +#define GC5035_OTP_FLAG_VALID 0x01
> +#define GC5035_OTP_DPC_TOTAL_NUMBER_OFFSET 0x0070
> +#define GC5035_OTP_DPC_ERROR_NUMBER_OFFSET 0x0078
> +
> +/* OTP register parameters */
> +#define GC5035_OTP_REG_FLAG_OFFSET 0x0880
> +#define GC5035_OTP_REG_DATA_OFFSET 0x0888
> +#define GC5035_OTP_REG_ADDR_OFFSET 1
> +#define GC5035_OTP_REG_VAL_OFFSET 2
> +#define GC5035_OTP_PAGE_FLAG_OFFSET 3
> +#define GC5035_OTP_PER_PAGE_SIZE 4
> +#define GC5035_OTP_REG_PAGE_MASK 0x07
> +#define GC5035_OTP_REG_MAX_GROUP 5
> +#define GC5035_OTP_REG_BYTE_PER_GROUP 5
> +#define GC5035_OTP_REG_PER_GROUP 2
> +#define GC5035_OTP_REG_BYTE_PER_REG 2
> +#define GC5035_OTP_REG_DATA_SIZE 25
> +#define GC5035_OTP_REG_SIZE 10
> +
> +#define GC5035_DD_DELAY_US (10 * 1000)
> +#define GC5035_DD_TIMEOUT_US (100 * 1000)
> +
> static const char * const gc5035_supplies[] = {
> /*
> * Requested separately due to power sequencing needs:
> @@ -95,6 +146,21 @@ struct gc5035_regval {
> u8 val;
> };
>
> +struct gc5035_reg {
> + u8 page;
> + struct gc5035_regval regval;
> +};
> +
> +struct gc5035_otp_regs {
> + unsigned int num_regs;
> + struct gc5035_reg regs[GC5035_OTP_REG_SIZE];
> +};
> +
> +struct gc5035_dpc {
> + bool valid;
> + unsigned int total_num;
> +};
> +
> struct gc5035_mode {
> u32 width;
> u32 height;
> @@ -122,6 +188,11 @@ struct gc5035 {
> struct v4l2_ctrl *hblank;
> struct v4l2_ctrl *vblank;
>
> + bool otp_read;
> + u8 otp_id[GC5035_OTP_ID_SIZE];
> + struct gc5035_dpc dpc;
> + struct gc5035_otp_regs otp_regs;
> +
> /*
> * Serialize control access, get/set format, get selection
> * and start streaming.
> @@ -136,6 +207,69 @@ static inline struct gc5035 *to_gc5035(struct v4l2_subdev *sd)
> return container_of(sd, struct gc5035, subdev);
> }
>
> +static const struct gc5035_regval gc5035_otp_init_regs[] = {
> + {0xfc, 0x01},
> + {0xf4, 0x40},
> + {0xf5, 0xe9},
> + {0xf6, 0x14},
> + {0xf8, 0x49},
> + {0xf9, 0x82},
> + {0xfa, 0x00},
> + {0xfc, 0x81},
> + {0xfe, 0x00},
> + {0x36, 0x01},
> + {0xd3, 0x87},
> + {0x36, 0x00},
> + {0x33, 0x00},
> + {0xf7, 0x01},
> + {0xfc, 0x8e},
> + {0xfe, 0x00},
> + {0xee, 0x30},
> + {0xfa, 0x10},
> + {0xf5, 0xe9},
> + {0xfe, 0x02},
> + {0x67, 0xc0},
> + {0x59, 0x3f},
> + {0x55, 0x84},
> + {0x65, 0x80},
> + {0x66, 0x03},
> + {0xfe, 0x00},
> +};
> +
> +static const struct gc5035_regval gc5035_otp_exit_regs[] = {
> + {0xfe, 0x02},
> + {0x67, 0x00},
> + {0xfe, 0x00},
> + {0xfa, 0x00},
> +};
> +
> +static const struct gc5035_regval gc5035_dd_auto_load_regs[] = {
> + {0xfe, 0x02},
> + {0xbe, 0x00},
> + {0xa9, 0x01},
> + {0x09, 0x33},
> +};
> +
> +static const struct gc5035_regval gc5035_otp_dd_regs[] = {
> + {0x03, 0x00},
> + {0x04, 0x80},
> + {0x95, 0x0a},
> + {0x96, 0x30},
> + {0x97, 0x0a},
> + {0x98, 0x32},
> + {0x99, 0x07},
> + {0x9a, 0xa9},
> + {0xf3, 0x80},
> +};
> +
> +static const struct gc5035_regval gc5035_otp_dd_enable_regs[] = {
> + {0xbe, 0x01},
> + {0x09, 0x00},
> + {0xfe, 0x01},
> + {0x80, 0x02},
> + {0xfe, 0x00},
> +};
> +
> /*
> * Xclk 24Mhz
> * Pclk 87.6Mhz
> @@ -763,6 +897,346 @@ static int gc5035_read_reg(struct gc5035 *gc5035, u8 reg, u8 *val)
> return 0;
> }
>
> +static int gc5035_otp_read_data(struct gc5035 *gc5035, u16 bit_addr, u8 *data,
> + size_t length)
> +{
> + size_t i;
> + int ret;
> +
> + if (WARN_ON(bit_addr % 8))
> + return -EINVAL;
> +
> + if (WARN_ON(bit_addr / 8 + length > GC5035_OTP_DATA_LENGTH))
> + return -EINVAL;
> +
> + ret = gc5035_write_reg(gc5035, GC5035_PAGE_REG, 2);
> + if (ret)
> + return ret;
> +
> + ret = gc5035_write_reg(gc5035, GC5035_REG_OTP_ACCESS_ADDR_H,
> + (bit_addr >> 8) &
> + GC5035_OTP_ACCESS_ADDR_H_MASK);
> + if (ret)
> + return ret;
> +
> + ret = gc5035_write_reg(gc5035, GC5035_REG_OTP_ACCESS_ADDR_L,
> + bit_addr & 0xff);
> + if (ret)
> + return ret;
> +
> + ret = gc5035_write_reg(gc5035, GC5035_REG_OTP_MODE,
> + GC5035_OTP_PRE_READ);
> + if (ret)
> + goto out_read_done;
> +
> + ret = gc5035_write_reg(gc5035, GC5035_REG_OTP_MODE,
> + GC5035_OTP_READ_MODE);
> + if (ret)
> + goto out_read_done;
> +
> + for (i = 0; i < length; i++) {
> + ret = gc5035_read_reg(gc5035, GC5035_REG_OTP_DATA, &data[i]);
> + if (ret)
> + goto out_read_done;
> + }
> +
> +out_read_done:
> + gc5035_write_reg(gc5035, GC5035_REG_OTP_MODE, GC5035_OTP_READ_DONE);
> +
> + return ret;
> +}
> +
> +static int gc5035_read_otp_regs(struct gc5035 *gc5035)
> +{
> + struct device *dev = &gc5035->client->dev;
> + struct gc5035_otp_regs *otp_regs = &gc5035->otp_regs;
> + u8 regs[GC5035_OTP_REG_DATA_SIZE] = {0};
> + unsigned int i, j;
> + u8 flag;
> + int ret;
> +
> + ret = gc5035_otp_read_data(gc5035, GC5035_OTP_REG_FLAG_OFFSET,
> + &flag, 1);
> + if (ret) {
> + dev_err(dev, "failed to read otp reg flag\n");
> + return ret;
> + }
> +
> + dev_dbg(dev, "register update flag = 0x%x\n", flag);
> +
> + gc5035->otp_regs.num_regs = 0;
> + if (flag != GC5035_OTP_FLAG_VALID)
> + return 0;
> +
> + ret = gc5035_otp_read_data(gc5035, GC5035_OTP_REG_DATA_OFFSET,
> + regs, sizeof(regs));
> + if (ret) {
> + dev_err(dev, "failed to read otp reg data\n");
> + return ret;
> + }
> +
> + for (i = 0; i < GC5035_OTP_REG_MAX_GROUP; i++) {
> + unsigned int base_group = i * GC5035_OTP_REG_BYTE_PER_GROUP;
> +
> + for (j = 0; j < GC5035_OTP_REG_PER_GROUP; j++) {
> + struct gc5035_reg *reg;
> +
> + if (!(regs[base_group] &
> + BIT((GC5035_OTP_PER_PAGE_SIZE * j +
> + GC5035_OTP_PAGE_FLAG_OFFSET))))
> + continue;
> +
> + reg = &otp_regs->regs[otp_regs->num_regs++];
> + reg->page = (regs[base_group] >>
> + (GC5035_OTP_PER_PAGE_SIZE * j)) &
> + GC5035_OTP_REG_PAGE_MASK;
> + reg->regval.addr = regs[base_group + j *
> + GC5035_OTP_REG_BYTE_PER_REG +
> + GC5035_OTP_REG_ADDR_OFFSET];
> + reg->regval.val = regs[base_group + j *
> + GC5035_OTP_REG_BYTE_PER_REG +
> + GC5035_OTP_REG_VAL_OFFSET];
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int gc5035_read_dpc(struct gc5035 *gc5035)
> +{
> + struct device *dev = &gc5035->client->dev;
> + struct gc5035_dpc *dpc = &gc5035->dpc;
> + u8 dpc_flag = 0;
> + u8 error_number = 0;
> + u8 total_number = 0;
> + int ret;
> +
> + ret = gc5035_otp_read_data(gc5035, GC5035_OTP_DPC_FLAG_OFFSET,
> + &dpc_flag, 1);
> + if (ret) {
> + dev_err(dev, "failed to read dpc flag\n");
> + return ret;
> + }
> +
> + dev_dbg(dev, "dpc flag = 0x%x\n", dpc_flag);
> +
> + dpc->valid = false;
> +
> + switch (dpc_flag & GC5035_OTP_DPC_FLAG_MASK) {
> + case GC5035_OTP_FLAG_EMPTY:
> + dev_dbg(dev, "dpc info is empty!!\n");
> + break;
> +
> + case GC5035_OTP_FLAG_VALID:
> + dev_dbg(dev, "dpc info is valid!\n");
> + ret = gc5035_otp_read_data(gc5035,
> + GC5035_OTP_DPC_TOTAL_NUMBER_OFFSET,
> + &total_number, 1);
> + if (ret) {
> + dev_err(dev, "failed to read dpc total number\n");
> + return ret;
> + }
> +
> + ret = gc5035_otp_read_data(gc5035,
> + GC5035_OTP_DPC_ERROR_NUMBER_OFFSET,
> + &error_number, 1);
> + if (ret) {
> + dev_err(dev, "failed to read dpc error number\n");
> + return ret;
> + }
> +
> + dpc->total_num = total_number + error_number;
> + dpc->valid = true;
> + dev_dbg(dev, "total_num = %d\n", dpc->total_num);
> + break;
> +
> + default:
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int gc5035_otp_read_sensor_info(struct gc5035 *gc5035)
> +{
> + int ret;
> +
> + ret = gc5035_read_dpc(gc5035);
> + if (ret)
> + return ret;
> +
> + return gc5035_read_otp_regs(gc5035);
> +}
> +
> +static int gc5035_check_dd_load_status(struct gc5035 *gc5035)
> +{
> + u8 status;
> + int ret;
> +
> + ret = gc5035_read_reg(gc5035, GC5035_REG_DD_LOAD_STATUS, &status);
> + if (ret)
> + return ret;
> +
> + if (status & GC5035_OTP_BIT_LOAD)
> + return status;
> + else
> + return 0;
> +}
> +
> +static int gc5035_otp_update_dd(struct gc5035 *gc5035)
> +{
> + struct device *dev = &gc5035->client->dev;
> + struct gc5035_dpc *dpc = &gc5035->dpc;
> + int val, ret;
> +
> + if (!dpc->valid) {
> + dev_dbg(dev, "DPC table invalid, not updating DD.\n");
> + return 0;
> + }
> +
> + dev_dbg(dev, "DD auto load start\n");
> +
> + ret = gc5035_write_array(gc5035, gc5035_dd_auto_load_regs,
> + ARRAY_SIZE(gc5035_dd_auto_load_regs));
> + if (ret) {
> + dev_err(dev, "failed to write dd auto load reg\n");
> + return ret;
> + }
> +
> + ret = gc5035_write_reg(gc5035, GC5035_REG_DD_TOTALNUM_H,
> + (dpc->total_num >> 8) &
> + GC5035_DD_TOTALNUM_H_MASK);
> + if (ret)
> + return ret;
> +
> + ret = gc5035_write_reg(gc5035, GC5035_REG_DD_TOTALNUM_L,
> + dpc->total_num & 0xff);
> + if (ret)
> + return ret;
> +
> + ret = gc5035_write_array(gc5035, gc5035_otp_dd_regs,
> + ARRAY_SIZE(gc5035_otp_dd_regs));
> + if (ret)
> + return ret;
> +
> + /* Wait for DD to finish loading automatically */
> + ret = readx_poll_timeout(gc5035_check_dd_load_status, gc5035,
> + val, val <= 0, GC5035_DD_DELAY_US,
> + GC5035_DD_TIMEOUT_US);
> + if (ret < 0) {
> + dev_err(dev, "DD load timeout\n");
> + return -EFAULT;
> + }
> + if (val < 0) {
> + dev_err(dev, "DD load failure\n");
> + return val;
> + }
> +
> + ret = gc5035_write_array(gc5035, gc5035_otp_dd_enable_regs,
> + ARRAY_SIZE(gc5035_otp_dd_enable_regs));
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int gc5035_otp_update_regs(struct gc5035 *gc5035)
> +{
> + struct device *dev = &gc5035->client->dev;
> + struct gc5035_otp_regs *otp_regs = &gc5035->otp_regs;
> + unsigned int i;
> + int ret;
> +
> + dev_dbg(dev, "reg count = %d\n", otp_regs->num_regs);
> +
> + for (i = 0; i < otp_regs->num_regs; i++) {
> + ret = gc5035_write_reg(gc5035, GC5035_PAGE_REG,
> + otp_regs->regs[i].page);
> + if (ret)
> + return ret;
> +
> + ret = gc5035_write_reg(gc5035,
> + otp_regs->regs[i].regval.addr,
> + otp_regs->regs[i].regval.val);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int gc5035_otp_update(struct gc5035 *gc5035)
> +{
> + struct device *dev = &gc5035->client->dev;
> + int ret;
> +
> + ret = gc5035_otp_update_dd(gc5035);
> + if (ret) {
> + dev_err(dev, "failed to update otp dd\n");
> + return ret;
> + }
> +
> + ret = gc5035_otp_update_regs(gc5035);
> + if (ret) {
> + dev_err(dev, "failed to update otp regs\n");
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int gc5035_set_otp_config(struct gc5035 *gc5035)
> +{
> + struct device *dev = &gc5035->client->dev;
> + u8 otp_id[GC5035_OTP_ID_SIZE];
> + int ret;
> +
> + ret = gc5035_write_array(gc5035, gc5035_otp_init_regs,
> + ARRAY_SIZE(gc5035_otp_init_regs));
> + if (ret) {
> + dev_err(dev, "failed to write otp init reg\n");
> + return ret;
> + }
> +
> + ret = gc5035_otp_read_data(gc5035, GC5035_OTP_ID_DATA_OFFSET,
> + &otp_id[0], GC5035_OTP_ID_SIZE);
Is this read needed every time when streaming is about to start?
I guess it's not wrong but it seems unnecessary on subsequent times.
> + if (ret) {
> + dev_err(dev, "failed to read otp id\n");
> + goto out_otp_exit;
> + }
> +
> + if (!gc5035->otp_read || memcmp(gc5035->otp_id, otp_id, sizeof(otp_id))) {
> + dev_dbg(dev, "reading OTP configuration\n");
> +
> + memset(&gc5035->otp_regs, 0, sizeof(gc5035->otp_regs));
> + memset(&gc5035->dpc, 0, sizeof(gc5035->dpc));
> +
> + memcpy(gc5035->otp_id, otp_id, sizeof(gc5035->otp_id));
> +
> + ret = gc5035_otp_read_sensor_info(gc5035);
> + if (ret < 0) {
> + dev_err(dev, "failed to read otp info\n");
> + goto out_otp_exit;
> + }
> +
> + gc5035->otp_read = true;
> + }
> +
> + ret = gc5035_otp_update(gc5035);
> + if (ret < 0)
> + return ret;
> +
> +out_otp_exit:
> + ret = gc5035_write_array(gc5035, gc5035_otp_exit_regs,
> + ARRAY_SIZE(gc5035_otp_exit_regs));
> + if (ret) {
> + dev_err(dev, "failed to write otp exit reg\n");
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> static int gc5035_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_subdev_pad_config *cfg,
> struct v4l2_subdev_format *fmt)
> @@ -859,6 +1333,10 @@ static int __gc5035_start_stream(struct gc5035 *gc5035)
> if (ret)
> return ret;
>
> + ret = gc5035_set_otp_config(gc5035);
> + if (ret)
> + return ret;
> +
> ret = gc5035_write_array(gc5035, gc5035->cur_mode->reg_list,
> gc5035->cur_mode->num_regs);
> if (ret)
--
Kind regards,
Sakari Ailus
Powered by blists - more mailing lists