Re: [PATCH v5] media: add imx319 camera sensor driver

[Bing Bu Cao <bingbu.cao@linux.intel.com>]

On 09/18/2018 05:49 PM, Tomasz Figa wrote:
> Hi Bingbu,
>
> On Mon, Sep 17, 2018 at 2:53 PM <bingbu.cao@intel.com> wrote:
>> From: Bingbu Cao <bingbu.cao@intel.com>
>>
>> Add a v4l2 sub-device driver for the Sony imx319 image sensor.
>> This is a camera sensor using the i2c bus for control and the
>> csi-2 bus for data.
> Please see my comments inline. Also, I'd appreciate being CCed on
> related work in the future.
Ack.
Sorry, will add you into the cc-list.
>
> [snip]
>> +
>> +static const char * const imx319_test_pattern_menu[] = {
>> +       "Disabled",
>> +       "100% color bars",
>> +       "Solid color",
>> +       "Fade to gray color bars",
>> +       "PN9"
>> +};
>> +
>> +static const int imx319_test_pattern_val[] = {
>> +       IMX319_TEST_PATTERN_DISABLED,
>> +       IMX319_TEST_PATTERN_COLOR_BARS,
>> +       IMX319_TEST_PATTERN_SOLID_COLOR,
>> +       IMX319_TEST_PATTERN_GRAY_COLOR_BARS,
>> +       IMX319_TEST_PATTERN_PN9,
>> +};
> This array is not needed. All the entries are equal to corresponding
> indices, i.e. the array is equivalent to { 0, 1, 2, 3, 4 }. We can use
> ctrl->val directly.
Ack.
> [snip]
>
>> +/* Write a list of registers */
>> +static int imx319_write_regs(struct imx319 *imx319,
>> +                             const struct imx319_reg *regs, u32 len)
>> +{
>> +       struct i2c_client *client = v4l2_get_subdevdata(&imx319->sd);
>> +       int ret;
>> +       u32 i;
>> +
>> +       for (i = 0; i < len; i++) {
>> +               ret = imx319_write_reg(imx319, regs[i].address, 1, regs[i].val);
>> +               if (ret) {
>> +                       dev_err_ratelimited(&client->dev,
>> +
> Hmm, the message is clipped here. Let me see if it's something with my
> email client...
The code here:

1827 for (i = 0; i < len; i++) {
1828 ret = imx319_write_reg(imx319, regs[i].address, 1, regs[i].val);
1829 if (ret) {
1830 dev_err_ratelimited(&client->dev,
1831 "write reg 0x%4.4x return err %d",
1832 regs[i].address, ret);
1833 return ret;
1834 }
1835 } Same as the code shown on your client?

>
> Best regards,
> Tomasz
>