Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate

From: jacopo mondi <jacopo@jmondi.org>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Sam Bobrowicz <sam@elite-embedded.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	linux-media@vger.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Mylene Josserand <mylene.josserand@bootlin.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Hugues Fruchet <hugues.fruchet@st.com>,
	Loic Poulain <loic.poulain@linaro.org>,
	Steve Longerbeam <slongerbeam@gmail.com>,
	Daniel Mack <daniel@zonque.org>
Subject: Re: [PATCH v4 01/12] media: ov5640: Adjust the clock based on the expected rate
Date: Thu, 18 Oct 2018 12:03:32 +0200	[thread overview]
Message-ID: <20181018100332.GE17549@w540> (raw)
In-Reply-To: <20181018093152.q5yusjycwbzxnyfq@flea>

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

On Thu, Oct 18, 2018 at 11:31:52AM +0200, Maxime Ripard wrote:
> On Wed, Oct 17, 2018 at 09:51:43PM +0200, jacopo mondi wrote:
> > Hello Sam and Maxime (and other ov5640-ers :)
> >
> > On Wed, Oct 17, 2018 at 10:54:01AM -0700, Sam Bobrowicz wrote:
> > > Hello Maxime and Jacopo (and other ov5640-ers),
> > >
> > > I just submitted my version of this patch to the mailing list as RFC.
> > > It is working on my MIPI platform. Please try it if you have time.
> > > Hopefully we can merge these two into a single patch that doesn't
> > > break any platforms.
> >
> > Thanks, I have seen your patch but it seems to contain a lot of things
> > already part of Maxime's series. Was this intentional?
> >
> > Now the un-pleaseant part: I have just sent out my re-implementation
> > of the MIPI clock tree configuration, based on top of Maxime's series.
> > Both you and me have spent a looot of time on this I'm sure, and now
> > we have two competing implementations.
> >
> > I had a quick look at yours, and for sure there are things I am not
> > taking care of (I'm thinking about the 0x4837 register that seems to
> > be important for your platform), so I think both our implementations
> > can benefits from a comparison. What is important to me is that both
> > you and me don't feel like our work has been wasted, so let's try to
> > find out a way to get the better of the two put together, and possibly
> > applied on top of Maxime's series, so that a v5 of this will work for
> > both MIPI and DVP interfaces. How to do that I'm not sure atm, I think
> > other reviewers might help in that if they want to have a look at both
> > our series :)
>
> IIRC, Sam's system has never worked with the ov5640 driver, and his
> patches now make it work.
>
> Your patches on the other hand make sure that the current series
> doesn't break existing users. So I guess we could merge your current
> patches into the v5 of my rework, and have Sam send his work on top of
> that.
>
> Does that make sense?

It does for me, but it puts the burden on Sam to re-apply his work
on top of [yours+mine] (which is something he would have had to do
anyhow to have his patches accepted, as he would have had to rebase on
top of your series).

I hope to find some more time to look into his series and find out how
hard it would be to add his changes on top of mine, and hopefully help
with this.

Also, testing my patches with DVP would be nice (it should not be
affected at all, but still...)

Thanks
   j

>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

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