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

Path	csiph.com!eternal-september.org!feeder.eternal-september.org!news.mixmin.net!enother.net!enother.net!enother.net!peer02.fr7!futter-mich.highwinds-media.com!news.highwinds-media.com!fu-berlin.de!bofh.it!news.nic.it!robomod
From	Pavel Machek <pavel@ucw.cz>
Newsgroups	linux.kernel
Subject	Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor
Date	Tue, 13 Dec 2016 22:20:02 +0100
Message-ID	<sO2Eq-6bg-27@gated-at.bofh.it> (permalink)
References	<svxfH-5uo-3@gated-at.bofh.it> <svxz4-5CF-7@gated-at.bofh.it>
MIME-Version	1.0
Content-Type	multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="uAKRQypu60I7Lcqm"
Content-Disposition	inline
User-Agent	Mutt/1.5.23 (2014-03-12)
Sender	robomod@news.nic.it
List-ID	<linux-kernel.vger.kernel.org>
X-Mailing-List	linux-kernel@vger.kernel.org
Approved	robomod@news.nic.it
Lines	88
Organization	linux.* mail to news gateway
X-Original-Cc	ivo.g.dimitrov.75@gmail.com, sre@kernel.org, pali.rohar@gmail.com, linux-media@vger.kernel.org, galak@codeaurora.org, mchehab@osg.samsung.com, linux-kernel@vger.kernel.org
X-Original-Date	Tue, 13 Dec 2016 22:05:06 +0100
X-Original-Message-ID	<20161213210506.GA11569@amd>
X-Original-References	<20161023200355.GA5391@amd> <20161023201954.GI9460@valkosipuli.retiisi.org.uk>
X-Original-Sender	linux-kernel-owner@vger.kernel.org
X-Received-Bytes	3741
X-Received-Body-CRC	1620037758
Xref	csiph.com linux.kernel:1541533

Show key headers only | View raw

[Multipart message — attachments visible in raw view] - view raw

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

Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Pavel Machek <pavel@ucw.cz> - 2016-12-13 22:20 +0100
  Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor Sakari Ailus <sakari.ailus@iki.fi> - 2016-12-18 23:00 +0100

csiph-web