All of lore.kernel.org
 help / color / mirror / Atom feed
From: jacopo mondi <jacopo@jmondi.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Sakari Ailus <sakari.ailus@iki.fi>,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	laurent.pinchart@ideasonboard.com, mchehab@kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
	linux-sh@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/4] media: i2c: Copy mt9t112 soc_camera sensor driver
Date: Thu, 15 Mar 2018 17:20:08 +0100	[thread overview]
Message-ID: <20180315162008.GA31710@w540> (raw)
In-Reply-To: <d1cfdb88-ec5d-5229-6fd7-0916905fc8e8@xs4all.nl>

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

Hi Hans,

On Thu, Mar 15, 2018 at 08:30:21AM -0700, Hans Verkuil wrote:
> On 03/15/2018 07:38 AM, jacopo mondi wrote:
> > Hi Sakari,
> >    thanks for looking into this!
> >
> > On Thu, Mar 15, 2018 at 01:35:34PM +0200, Sakari Ailus wrote:
> >> Hi Jacopo,
> >>
> >> I wonder if it'd make sense to just make all the changes to the driver and
> >> then have it reviewed; I'm not sure the old driver can be said to have been
> >> in a known-good state that'd be useful to compare against. I think you did
> >> that with another driver as well.
> >>
> >
> > Well, I understand this is still debated, and I see your point.
> > As far as I can tell the driver had been developed to work with SH4
> > Ecovec boards and there tested.
> >
> > I'm not sure I fully got you here though. Are you proposing to
> > squash my next patch that cleans up the driver into this one and
> > propose it as a completely new driver to be reviewed from scratch?
> >
> > In the two previous driver I touched in this "remove soc_camera"
> > journey (ov772x and tw9910) I have followed this same pattern: copy
> > the soc_camera driver without removing the existing one, and pile on
> > top my changes/cleanups in another patch. Then port the board code to
> > use the new sensor driver, and the new CEU driver as well.
> >
> > Also, how would you like to proceed here? Hans sent a pull request for
> > the series, should I go with incremental changes on top of this?
>
> I don't want to postpone this conversion. The i2c/mt9t112.c is bug-compatible
> with i2c/soc-camera/mt9t112.c which is good enough for me. Being able to
> remove soc-camera in the (hopefully very) near future is the most important
> thing here.
>
> Once Jacopo can actually test the sensor, then that's a good time to review
> the driver in more detail.
>
> This reminded me that I actually started testing this sensor a year
> ago (I bought the same sensor on ebay, I completely forgot about that!).
>
> My attempt is here:
>
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=mt9t112
>
> I never finished it because I had no documentation on the pinout and never
> got around to hooking my oscilloscope up to it to figure this out. I was
> testing this with the atmel-isc.c driver.
>
> This might be of some use to you, Jacopo, once you have the sensor.

Thanks for the info. I'll see what I can do. I don't have register
level document, and if the module is the same you have neither a
pinout description. This is going to be fun :/

I'll then refrain from sending more patches for this series/driver
until we cannot actually test the sensor, fixes apart, if any, of course.

Thanks
   j

>
> Regards,
>
> 	Hans

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: jacopo mondi <jacopo@jmondi.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Sakari Ailus <sakari.ailus@iki.fi>,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	laurent.pinchart@ideasonboard.com, mchehab@kernel.org,
	linux-renesas-soc@vger.kernel.org, linux-media@vger.kernel.org,
	linux-sh@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/4] media: i2c: Copy mt9t112 soc_camera sensor driver
Date: Thu, 15 Mar 2018 16:20:08 +0000	[thread overview]
Message-ID: <20180315162008.GA31710@w540> (raw)
In-Reply-To: <d1cfdb88-ec5d-5229-6fd7-0916905fc8e8@xs4all.nl>

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

Hi Hans,

On Thu, Mar 15, 2018 at 08:30:21AM -0700, Hans Verkuil wrote:
> On 03/15/2018 07:38 AM, jacopo mondi wrote:
> > Hi Sakari,
> >    thanks for looking into this!
> >
> > On Thu, Mar 15, 2018 at 01:35:34PM +0200, Sakari Ailus wrote:
> >> Hi Jacopo,
> >>
> >> I wonder if it'd make sense to just make all the changes to the driver and
> >> then have it reviewed; I'm not sure the old driver can be said to have been
> >> in a known-good state that'd be useful to compare against. I think you did
> >> that with another driver as well.
> >>
> >
> > Well, I understand this is still debated, and I see your point.
> > As far as I can tell the driver had been developed to work with SH4
> > Ecovec boards and there tested.
> >
> > I'm not sure I fully got you here though. Are you proposing to
> > squash my next patch that cleans up the driver into this one and
> > propose it as a completely new driver to be reviewed from scratch?
> >
> > In the two previous driver I touched in this "remove soc_camera"
> > journey (ov772x and tw9910) I have followed this same pattern: copy
> > the soc_camera driver without removing the existing one, and pile on
> > top my changes/cleanups in another patch. Then port the board code to
> > use the new sensor driver, and the new CEU driver as well.
> >
> > Also, how would you like to proceed here? Hans sent a pull request for
> > the series, should I go with incremental changes on top of this?
>
> I don't want to postpone this conversion. The i2c/mt9t112.c is bug-compatible
> with i2c/soc-camera/mt9t112.c which is good enough for me. Being able to
> remove soc-camera in the (hopefully very) near future is the most important
> thing here.
>
> Once Jacopo can actually test the sensor, then that's a good time to review
> the driver in more detail.
>
> This reminded me that I actually started testing this sensor a year
> ago (I bought the same sensor on ebay, I completely forgot about that!).
>
> My attempt is here:
>
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=mt9t112
>
> I never finished it because I had no documentation on the pinout and never
> got around to hooking my oscilloscope up to it to figure this out. I was
> testing this with the atmel-isc.c driver.
>
> This might be of some use to you, Jacopo, once you have the sensor.

Thanks for the info. I'll see what I can do. I don't have register
level document, and if the module is the same you have neither a
pinout description. This is going to be fun :/

I'll then refrain from sending more patches for this series/driver
until we cannot actually test the sensor, fixes apart, if any, of course.

Thanks
   j

>
> Regards,
>
> 	Hans

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2018-03-15 16:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-12 13:43 [PATCH v2 0/4] Renesas CEU: SH7724 ECOVEC + Aptina mt9t112 Jacopo Mondi
2018-03-12 13:43 ` Jacopo Mondi
2018-03-12 13:43 ` [PATCH v2 1/4] media: i2c: Copy mt9t112 soc_camera sensor driver Jacopo Mondi
2018-03-12 13:43   ` Jacopo Mondi
2018-03-15 11:35   ` Sakari Ailus
2018-03-15 11:35     ` Sakari Ailus
2018-03-15 14:38     ` jacopo mondi
2018-03-15 14:38       ` jacopo mondi
2018-03-15 15:30       ` Hans Verkuil
2018-03-15 15:30         ` Hans Verkuil
2018-03-15 16:20         ` jacopo mondi [this message]
2018-03-15 16:20           ` jacopo mondi
2018-03-12 13:43 ` [PATCH v2 2/4] media: i2c: mt9t112: Remove soc_camera dependencies Jacopo Mondi
2018-03-12 13:43   ` Jacopo Mondi
2018-03-12 13:43 ` [PATCH v2 3/4] arch: sh: ecovec: Use new renesas-ceu camera driver Jacopo Mondi
2018-03-12 13:43   ` Jacopo Mondi
2018-03-12 13:43 ` [PATCH v2 4/4] media: MAINTAINERS: Add entry for Aptina MT9T112 Jacopo Mondi
2018-03-12 13:43   ` Jacopo Mondi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180315162008.GA31710@w540 \
    --to=jacopo@jmondi.org \
    --cc=hverkuil@xs4all.nl \
    --cc=jacopo+renesas@jmondi.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.