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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAEth8oHbWVKQo=TPt0VZU7QCXqMupsnxwjVZ8ODUQdfBOeZCqA@mail.gmail.com>
Date: Thu, 23 Oct 2025 17:39:03 +0800
From: Kate Hsuan <hpa@...hat.com>
To: Hans de Goede <hansg@...nel.org>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9] media: Add t4ka3 camera sensor driver

Hi Hans,


On Wed, Oct 22, 2025 at 7:09 PM Hans de Goede <hansg@...nel.org> wrote:
>
> Hi,
>
> On 21-Oct-25 10:03, kate Hsuan wrote:
> > From: Kate Hsuan <hpa@...hat.com>
> >
> > Add the t4ka3 driver from:
> > https://github.com/kitakar5525/surface3-atomisp-cameras.git
> >
> > With many cleanups / changes (almost a full rewrite) to make it suitable
> > for upstream:
> >
> > * Remove the VCM and VCM-OTP support, the mainline kernel models VCMs and
> >   calibration data eeproms as separate v4l2-subdev-s.
> >
> > * Remove the integration-factor t4ka3_get_intg_factor() support and IOCTL,
> >   this provided info to userspace through an atomisp private IOCTL.
> >
> > * Turn atomisp specific exposure/gain IOCTL into standard v4l2 controls.
> >
> > * Use normal ACPI power-management in combination with runtime-pm support
> >   instead of atomisp specific GMIN power-management code.
> >
> > * Turn into a standard V4L2 sensor driver using
> >   v4l2_async_register_subdev_sensor().
> >
> > * Add vblank, hblank, and link-freq controls; drop get_frame_interval().
> >
> > * Use CCI register helpers.
> >
> > * Calculate values for modes instead of using fixed register-value lists,
> >   allowing arbritrary modes.
> >
> > * Add get_selection() and set_selection() support
> >
> > * Add a CSI2 bus configuration check
> >
> > This been tested on a Xiaomi Mipad2 tablet which has a T4KA3 sensor with
> > DW9761 VCM as back sensor.
> >
> > Co-developed-by: Hans de Goede <hansg@...nel.org>
> > Signed-off-by: Hans de Goede <hansg@...nel.org>
> > Signed-off-by: Kate Hsuan <hpa@...hat.com>
> > ---
> > Changes in v9:
> > 1. Apply Hans' fix patch to fix the lock issue and squash it into this
> >    patch.
> > https://lore.kernel.org/linux-media/33dd5660-efb6-47e0-9672-f3ae65751185@kernel.org/
>
> Thank you for the new version. Some small review remarks below.
>

Thank you for reviewing.

> ...
>
> > +static void t4ka3_fill_format(struct t4ka3_data *sensor,
> > +                           struct v4l2_mbus_framefmt *fmt,
> > +                           unsigned int width, unsigned int height)
> > +{
> > +     memset(fmt, 0, sizeof(*fmt));
> > +     fmt->width = width;
> > +     fmt->height = height;
> > +     fmt->field = V4L2_FIELD_NONE;
> > +     fmt->colorspace = V4L2_COLORSPACE_SRGB;
>
> Since the sensor gives raw uncalibrated color data this should be:
>
>         fmt->colorspace = V4L2_COLORSPACE_RAW;
Okay
>
> > +     t4ka3_set_bayer_order(sensor, fmt);
> > +}
>
> ...
>
> > +static int t4ka3_set_pad_format(struct v4l2_subdev *sd,
> > +                             struct v4l2_subdev_state *sd_state,
> > +                             struct v4l2_subdev_format *format)
> > +{
> > +     struct t4ka3_data *sensor = to_t4ka3_sensor(sd);
> > +     struct v4l2_mbus_framefmt *try_fmt;
> > +     struct v4l2_mbus_framefmt *fmt;
> > +     const struct v4l2_rect *crop;
> > +     unsigned int width, height;
> > +     int min, max, def, ret = 0;
> > +
> > +     crop = t4ka3_get_active_crop(sensor);
> > +     fmt = t4ka3_get_active_format(sensor);
> > +
> > +     /* Limit set_fmt max size to crop width / height */
> > +     width = clamp_val(ALIGN(format->format.width, 2),
> > +                       T4KA3_MIN_CROP_WIDTH, crop->width);
> > +     height = clamp_val(ALIGN(format->format.height, 2),
> > +                        T4KA3_MIN_CROP_HEIGHT, crop->height);
> > +     t4ka3_fill_format(sensor, &format->format, width, height);
> > +
> > +     if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> > +             try_fmt = v4l2_subdev_state_get_format(sd_state, 0);
> > +             *try_fmt = format->format;
> > +             return 0;
> > +     }
> > +
> > +     if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE && sensor->streaming)
> > +             return -EBUSY;
> > +
> > +     *v4l2_subdev_state_get_format(sd_state, 0) = format->format;
> > +
> > +     if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> > +             return 0;
> > +
> > +     t4ka3_calc_mode(sensor);
> > +
> > +     /* vblank range is height dependent adjust and reset to default */
> > +     t4ka3_get_vblank_limits(sensor, &min, &max, &def);
> > +     ret = __v4l2_ctrl_modify_range(sensor->ctrls.vblank, min, max, 1, def);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.vblank, def);
> > +     if (ret)
> > +             return ret;
> > +
> > +     def = T4KA3_ACTIVE_WIDTH - fmt->width;
>
> This should be:
>
>         def = T4KA3_PIXELS_PER_LINE - fmt->width;
Okay. It is the maximum width and makes the hblank wider.
>
> > +     ret = __v4l2_ctrl_modify_range(sensor->ctrls.hblank, def, def, 1, def);
> > +     if (ret)
> > +             return ret;
>
> Add a blank line here please.
Okay
>
> > +     ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.hblank, def);
> > +     if (ret)
> > +             return ret;
> > +
> > +     /* exposure range depends on vts which may have changed */
> > +     ret = t4ka3_update_exposure_range(sensor);
>
> This is not necessary as the t4ka3_s_ctrl(vblank) call above
> calls t4ka3_s_ctrl(VBLANK) which already does this, so the comment
> and the call can be dropped and then the "return ret" here:

Got it and I traced the code a bit.
The t4ka3_update_exposure_range() is invoked when
__v4l2_ctrl_s_ctrl(); is called.
So, I'll remove it.
>
> > +     return ret;
>
> can be replaced by a simple "return 0;"
>
> > +}
>
> ...
>
> > +static int t4ka3_enable_stream(struct v4l2_subdev *sd, struct v4l2_subdev_state *state,
> > +                            u32 pad, u64 streams_mask)
> > +{
> > +     struct t4ka3_data *sensor = to_t4ka3_sensor(sd);
> > +     int ret;
> > +
> > +     ret = pm_runtime_get_sync(sensor->sd.dev);
> > +     if (ret < 0) {
> > +             dev_err(sensor->dev, "power-up err.\n");
> > +             goto error_exit;
>
> This is wrong even on failure pm_runtime_get...() still
> increases the pm-counter, so this should be goto error_powerdown; and
> then the unused error_exit label can be removed.

Okay. I'll correct this.
>
> > +     }
> > +
> > +     cci_multi_reg_write(sensor->regmap, t4ka3_init_config,
> > +                         ARRAY_SIZE(t4ka3_init_config), &ret);
> > +     /* enable group hold */
> > +     cci_write(sensor->regmap, T4KA3_REG_PARAM_HOLD, 1, &ret);
> > +     cci_multi_reg_write(sensor->regmap, t4ka3_pre_mode_set_regs,
> > +                         ARRAY_SIZE(t4ka3_pre_mode_set_regs), &ret);
> > +     if (ret)
> > +             goto error_powerdown;
> > +
> > +     ret = t4ka3_set_mode(sensor);
> > +     if (ret)
> > +             goto error_powerdown;
> > +
> > +     ret = cci_multi_reg_write(sensor->regmap, t4ka3_post_mode_set_regs,
> > +                               ARRAY_SIZE(t4ka3_post_mode_set_regs), NULL);
> > +     if (ret)
> > +             goto error_powerdown;
> > +
> > +     /* Restore value of all ctrls */
> > +     ret = __v4l2_ctrl_handler_setup(&sensor->ctrls.handler);
> > +     if (ret)
> > +             goto error_powerdown;
> > +
> > +     /* disable group hold */
> > +     cci_write(sensor->regmap, T4KA3_REG_PARAM_HOLD, 0, &ret);
> > +     cci_write(sensor->regmap, T4KA3_REG_STREAM, 1, &ret);
> > +     if (ret)
> > +             goto error_powerdown;
> > +
> > +     sensor->streaming = 1;
> > +
> > +     return ret;
>
> Please use return 0 here.
>
> > +
> > +error_powerdown:
> > +     pm_runtime_put(sensor->sd.dev);
> > +error_exit:
> > +     return ret;
> > +}
>
> ...
>
> > +static int t4ka3_set_selection(struct v4l2_subdev *sd,
> > +                            struct v4l2_subdev_state *state,
> > +                            struct v4l2_subdev_selection *sel)
> > +{
> > +     struct t4ka3_data *sensor = to_t4ka3_sensor(sd);
> > +     struct v4l2_mbus_framefmt *format;
> > +     struct v4l2_rect *crop;
> > +     struct v4l2_rect rect;
> > +
> > +     if (sel->target != V4L2_SEL_TGT_CROP)
> > +             return -EINVAL;
> > +
> > +     /*
> > +      * Clamp the boundaries of the crop rectangle to the size of the sensor
> > +      * pixel array. Align to multiples of 2 to ensure Bayer pattern isn't
> > +      * disrupted.
> > +      */
> > +     rect.left = clamp_val(ALIGN(sel->r.left, 2),
> > +                           T4KA3_NATIVE_START_LEFT, T4KA3_NATIVE_WIDTH);
> > +     rect.top = clamp_val(ALIGN(sel->r.top, 2),
> > +                          T4KA3_NATIVE_START_TOP, T4KA3_NATIVE_HEIGHT);
>
> It is better to replace these 2 clamps:
>
> > +     rect.width = clamp_val(ALIGN(sel->r.width, 2),
> > +                            T4KA3_MIN_CROP_WIDTH, T4KA3_NATIVE_WIDTH);
> > +     rect.height = clamp_val(ALIGN(sel->r.height, 2),
> > +                             T4KA3_MIN_CROP_HEIGHT, T4KA3_NATIVE_HEIGHT);
>
> with:
>
>         rect.width = clamp_val(ALIGN(sel->r.width, 2), T4KA3_MIN_CROP_WIDTH,
>                                T4KA3_NATIVE_WIDTH - rect.left);
>         rect.height = clamp_val(ALIGN(sel->r.height, 2), T4KA3_MIN_CROP_HEIGHT,
>                                 T4KA3_NATIVE_HEIGHT - rect.top);
>
> then this first clamp for width / height also makes sure width / height
> are not too large and then this comment + 2 more "clamps":
>
> > +     /* Make sure the crop rectangle isn't outside the bounds of the array */
> > +     rect.width = min_t(unsigned int, rect.width,
> > +                        T4KA3_NATIVE_WIDTH - rect.left);
> > +     rect.height = min_t(unsigned int, rect.height,
> > +                         T4KA3_NATIVE_HEIGHT - rect.top);
>
> can be dropped.

Okay. Simplifying the clamp is better.

>
> > +
> > +     crop = v4l2_subdev_state_get_crop(state, sel->pad);
> > +
> > +     *crop = rect;
>
> This assignment needs to be moved to after the check below if the crop
> size has changed. When doing this assignment first then the below check
> for changed size will always fail because crop and rect are now the same.
>
> (after moving down please group the assingment with the "sel->r = rect"
> assignment)
Okay
>
> > +
> > +     if (rect.width != crop->width || rect.height != crop->height) {
> > +             /*
> > +              * Reset the output image size if the crop rectangle size has
> > +              * been modified.
> > +              */
> > +             format = v4l2_subdev_state_get_format(state, sel->pad);
> > +             format->width = rect.width;
> > +             format->height = rect.height;
> > +             if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
> > +                     t4ka3_calc_mode(sensor);
> > +     }
> > +
> > +     sel->r = rect;


> > +
> > +     return 0;
> > +}
> > +
> > +static int
> > +t4ka3_enum_mbus_code(struct v4l2_subdev *sd,
> > +                  struct v4l2_subdev_state *sd_state,
> > +                  struct v4l2_subdev_mbus_code_enum *code)
> > +{
> > +     if (code->index)
> > +             return -EINVAL;
> > +
> > +     code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> > +     return 0;
> > +}
> > +
> > +static int t4ka3_enum_frame_size(struct v4l2_subdev *sd,
> > +                              struct v4l2_subdev_state *sd_state,
> > +                              struct v4l2_subdev_frame_size_enum *fse)
> > +{
> > +     struct v4l2_rect *crop;
> > +
> > +     if (fse->index >= T4KA3_FRAME_SIZES)
> > +             return -EINVAL;
> > +
> > +     crop = v4l2_subdev_state_get_crop(sd_state, fse->pad);
> > +     if (!crop)
> > +             return -EINVAL;
>
> There is no need to check for !crop. Now that the active-state
> from the subdev is used there will always be a crop rect.

got it.

>
> > +
> > +     fse->min_width = crop->width / (fse->index + 1);
> > +     fse->min_height = crop->height / (fse->index + 1);
> > +     fse->max_width = fse->min_width;
> > +     fse->max_height = fse->min_height;
> > +
> > +     return 0;
> > +}
>
> ...
>
> > +static int t4ka3_init_controls(struct t4ka3_data *sensor)
> > +{
> > +     const struct v4l2_ctrl_ops *ops = &t4ka3_ctrl_ops;
> > +     struct t4ka3_ctrls *ctrls = &sensor->ctrls;
> > +     struct v4l2_ctrl_handler *hdl = &ctrls->handler;
> > +     struct v4l2_fwnode_device_properties props;
> > +     int ret, min, max, def;
> > +     static const char * const test_pattern_menu[] = {
> > +             "Disabled",
> > +             "Solid White",
> > +             "Color Bars",
> > +             "Gradient",
> > +             "Random Data",
> > +     };
> > +
> > +     v4l2_ctrl_handler_init(hdl, 11);
> > +
> > +     hdl->lock = &sensor->lock;
> > +
> > +     ctrls->vflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
> > +     ctrls->hflip = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
> > +
> > +     ctrls->test_pattern = v4l2_ctrl_new_std_menu_items(hdl, ops,
> > +                                                        V4L2_CID_TEST_PATTERN,
> > +                                                        ARRAY_SIZE(test_pattern_menu) - 1,
> > +                                                        0, 0, test_pattern_menu);
> > +     ctrls->link_freq = v4l2_ctrl_new_int_menu(hdl, NULL, V4L2_CID_LINK_FREQ,
> > +                                               0, 0, sensor->link_freq);
> > +     ctrls->pixel_rate = v4l2_ctrl_new_std(hdl, NULL, V4L2_CID_PIXEL_RATE,
> > +                                           0, T4KA3_PIXEL_RATE,
> > +                                           1, T4KA3_PIXEL_RATE);
> > +
> > +     v4l2_subdev_lock_state(sensor->sd.active_state);
> > +     t4ka3_calc_mode(sensor);
> > +     t4ka3_get_vblank_limits(sensor, &min, &max, &def);
> > +     v4l2_subdev_unlock_state(sensor->sd.active_state);
> > +
> > +     ctrls->vblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_VBLANK, min, max, 1, def);
> > +
> > +     def = T4KA3_ACTIVE_WIDTH;
>
> This should be :
>
>         def = T4KA3_PIXELS_PER_LINE - T4KA3_ACTIVE_WIDTH;
Okay

>
> > +     ctrls->hblank = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_HBLANK,
> > +                                       def, def, 1, def);
> > +
> > +     max = T4KA3_LINES_PER_FRAME_30FPS - T4KA3_COARSE_INTEGRATION_TIME_MARGIN;
> > +     ctrls->exposure = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_EXPOSURE,
> > +                                         0, max, 1, max);
> > +
> > +     ctrls->gain = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_ANALOGUE_GAIN,
> > +                                     T4KA3_MIN_GLOBAL_GAIN_SUPPORTED,
> > +                                     T4KA3_MAX_GLOBAL_GAIN_SUPPORTED,
> > +                                     1, T4KA3_MIN_GLOBAL_GAIN_SUPPORTED);
> > +
> > +     ret = v4l2_fwnode_device_parse(sensor->dev, &props);
> > +     if (ret)
> > +             return ret;
> > +
> > +     v4l2_ctrl_new_fwnode_properties(hdl, ops, &props);
> > +
> > +     if (hdl->error)
> > +             return hdl->error;
> > +
> > +     ctrls->vflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> > +     ctrls->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT;
> > +     ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +     ctrls->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > +     sensor->sd.ctrl_handler = hdl;
> > +     return 0;
> > +}
>
> ...
>
> > +static void t4ka3_remove(struct i2c_client *client)
> > +{
> > +     struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > +     struct t4ka3_data *sensor = to_t4ka3_sensor(sd);
> > +
> > +     v4l2_async_unregister_subdev(&sensor->sd);
>
> You are missing a v4l2_subdev_cleanup(sd); here (this is the
> cleanup for the recently added v4l2_subdev_init_finalize() call.
I read the document again and I found it. v4l2_subdev_cleanup() has to
be called when removing the module.

>
> > +     media_entity_cleanup(&sensor->sd.entity);
> > +     v4l2_ctrl_handler_free(&sensor->ctrls.handler);
>
> Also lets move media_entity_cleanup() last so that things
> are in the exact reverse order of probe(), so that will
> result in:
>
>         v4l2_async_unregister_subdev(&sensor->sd);
>         v4l2_ctrl_handler_free(&sensor->ctrls.handler);
>         v4l2_subdev_cleanup(sd);
>         media_entity_cleanup(&sensor->sd.entity);
I'll carefully deal with the call sequence.
>
>
> > +
> > +     /*
> > +      * Disable runtime PM. In case runtime PM is disabled in the kernel,
> > +      * make sure to turn power off manually.
> > +      */
> > +     pm_runtime_disable(&client->dev);
> > +     if (!pm_runtime_status_suspended(&client->dev))
> > +             t4ka3_pm_suspend(&client->dev);
> > +     pm_runtime_set_suspended(&client->dev);
> > +}
> > +
> > +static int t4ka3_probe(struct i2c_client *client)
> > +{
> > +     struct t4ka3_data *sensor;
> > +     int ret;
> > +
> > +     /* allocate sensor device & init sub device */
> > +     sensor = devm_kzalloc(&client->dev, sizeof(*sensor), GFP_KERNEL);
> > +     if (!sensor)
> > +             return -ENOMEM;
> > +
> > +     sensor->dev = &client->dev;
> > +
> > +     ret = t4ka3_check_hwcfg(sensor);
> > +     if (ret)
> > +             return ret;
> > +
> > +     mutex_init(&sensor->lock);
> > +
> > +     sensor->link_freq[0] = T4KA3_LINK_FREQ;
> > +
> > +     v4l2_i2c_subdev_init(&sensor->sd, client, &t4ka3_ops);
> > +     sensor->sd.internal_ops = &t4ka3_internal_ops;
> > +
> > +     sensor->powerdown_gpio = devm_gpiod_get(&client->dev, "powerdown",
> > +                                             GPIOD_OUT_HIGH);
> > +     if (IS_ERR(sensor->powerdown_gpio))
> > +             return dev_err_probe(&client->dev, PTR_ERR(sensor->powerdown_gpio),
> > +                                  "getting powerdown GPIO\n");
> > +
> > +     sensor->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
> > +                                                  GPIOD_OUT_HIGH);
> > +     if (IS_ERR(sensor->reset_gpio))
> > +             return dev_err_probe(&client->dev, PTR_ERR(sensor->reset_gpio),
> > +                                  "getting reset GPIO\n");
> > +
> > +     sensor->regmap = devm_cci_regmap_init_i2c(client, 16);
> > +     if (IS_ERR(sensor->regmap))
> > +             return PTR_ERR(sensor->regmap);
> > +
> > +     ret = t4ka3_pm_resume(sensor->dev);
> > +     if (ret)
> > +             return ret;
> > +
> > +     pm_runtime_set_active(&client->dev);
> > +     pm_runtime_get_noresume(&client->dev);
> > +     pm_runtime_enable(&client->dev);
> > +
> > +     sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > +     sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
> > +     sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > +     ret = media_entity_pads_init(&sensor->sd.entity, 1, &sensor->pad);
> > +     if (ret)
> > +             goto err_media_entity;
>
> If functions fail, there matching cleanup function should not be
> called, so this should be a "goto err_pm_disable;"

My bad. Since the media_entity_pads_init() fails, the corresponding
structures haven't been initialised or allocated.
So, disable the pm.
>
> > +
> > +     sensor->sd.state_lock = sensor->ctrls.handler.lock;
> > +     ret = v4l2_subdev_init_finalize(&sensor->sd);
> > +     if (ret < 0) {
> > +             dev_err(&client->dev, "failed to init subdev: %d", ret);
> > +             goto err_subdev_entry;
>
> and this should be a "goto err_media_entity_cleanup;"
The same as above.

>
> > +     }
> > +
> > +     ret = t4ka3_init_controls(sensor);
> > +     if (ret)
> > +             goto err_controls;
>
> controls are special and do need have v4l2_ctrl_handler_free()
> called on error to init, so this is correct,
>
> > +
> > +     ret = v4l2_async_register_subdev_sensor(&sensor->sd);
> > +     if (ret)
> > +             goto err_subdev_entry;
>
> But this should also be "goto err_controls;"
okay.
>
> > +
> > +     pm_runtime_set_autosuspend_delay(&client->dev, 1000);
> > +     pm_runtime_use_autosuspend(&client->dev);
> > +     pm_runtime_put_autosuspend(&client->dev);
> > +
> > +     return 0;
> > +
> > +err_subdev_entry:
> > +     v4l2_subdev_cleanup(&sensor->sd);
> > +
> > +err_controls:
> > +     v4l2_ctrl_handler_free(&sensor->ctrls.handler);
>
> and the order of these 2 is wrong, now if t4ka3_init_controls()
> fails, the v4l2_subdev_cleanup() is skipped even though
> v4l2_subdev_init_finalize() has run.
>
> > +
> > +err_media_entity:
> > +     media_entity_cleanup(&sensor->sd.entity);
> > +     pm_runtime_disable(&client->dev);
> > +     pm_runtime_put_noidle(&client->dev);
>
Got it.

I'll propose a v10 patch to include all the fixes. :)

> probe() has done a t4ka3_pm_resume() before it gets
> here, so this needs a t4ka3_pm_suspend() to match.
>
> After all this is fixed, the error exit paths
> with the gotos should look like this:
>
> err_controls:
>         v4l2_ctrl_handler_free(&sensor->ctrls.handler);
>         v4l2_subdev_cleanup(&sensor->sd);
>
> err_media_entity:
>         media_entity_cleanup(&sensor->sd.entity);
>
> err_pm_disable:
>         pm_runtime_disable(&client->dev);
>         pm_runtime_put_noidle(&client->dev);
>         t4ka3_pm_suspend(&client->dev);
>
> > +
> > +     return ret;
> > +}
> > +
> > +static struct acpi_device_id t4ka3_acpi_match[] = {
> > +     { "XMCC0003" },
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(acpi, t4ka3_acpi_match);
> > +
> > +static struct i2c_driver t4ka3_driver = {
> > +     .driver = {
> > +             .name = "t4ka3",
> > +             .acpi_match_table = ACPI_PTR(t4ka3_acpi_match),
> > +             .pm = pm_sleep_ptr(&t4ka3_pm_ops),
> > +     },
> > +     .probe = t4ka3_probe,
> > +     .remove = t4ka3_remove,
> > +};
> > +module_i2c_driver(t4ka3_driver)
> > +
> > +MODULE_DESCRIPTION("A low-level driver for T4KA3 sensor");
> > +MODULE_AUTHOR("HARVEY LV <harvey.lv@...el.com>");
> > +MODULE_AUTHOR("Kate Hsuan <hpa@...hat.com>");
> > +MODULE_LICENSE("GPL");
>
> Regards,
>
> Hans
>
>


-- 
BR,
Kate


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ