Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor

Pavel Machek
2016-12-13T21:07:30Z

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

Hi!

I have finally found the old mail you were refering to. Let me go
through it.

> > +/*
> > + * Convert exposure time `us' to rows. Modify `us' to make it to
> > + * correspond to the actual exposure time.
> > + */
> > +static int et8ek8_exposure_us_to_rows(struct et8ek8_sensor *sensor, u32 *us)
>
> Should a driver do something like this to begin with?
>
> The smiapp driver does use the native unit of exposure (lines) for the
> control and I think the et8ek8 driver should do so as well.
>
> The HBLANK, VBLANK and PIXEL_RATE controls are used to provide the user with
> enough information to perform the conversion (if necessary).

Well... I believe exposure in usec is preffered format for userspace
to work with (because then it can change resolution and keep camera
settings) but I see kernel code is quite ugly. Let me see what I can do...

> > + if (ret) {
> > + dev_warn(dev, "can't get clock-frequency\n");
> > + return ret;
> > + }
> > +
> > + mutex_init(&sensor->power_lock);
>
> mutex_destroy() should be called on the mutex if probe fails after this and
> in remove().

Ok.

> > +static int __exit et8ek8_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *subdev = i2c_get_clientdata(client);
> > + struct et8ek8_sensor *sensor = to_et8ek8_sensor(subdev);
> > +
> > + if (sensor->power_count) {
> > + gpiod_set_value(sensor->reset, 0);
> > + clk_disable_unprepare(sensor->ext_clk);
>
> How about the regulator? Could you call et8ek8_power_off() instead?

Hmm. Actually if we hit this, it indicates something funny is going
on, no? I guess I'll add WARN_ON there...

> > +++ b/drivers/media/i2c/et8ek8/et8ek8_reg.h
> > @@ -0,0 +1,96 @@
> > +/*
> > + * et8ek8.h
>
> et8ek8_reg.h

Ok.

Thanks for patience,
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]