Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755612AbcJYHHt (ORCPT ); Tue, 25 Oct 2016 03:07:49 -0400 Received: from 2.mo3.mail-out.ovh.net ([46.105.75.36]:57578 "EHLO 2.mo3.mail-out.ovh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750804AbcJYHHr (ORCPT ); Tue, 25 Oct 2016 03:07:47 -0400 X-Greylist: delayed 120023 seconds by postgrey-1.27 at vger.kernel.org; Tue, 25 Oct 2016 03:07:46 EDT Date: Tue, 25 Oct 2016 09:07:24 +0200 From: Lukasz Majewski To: Stefan Agner Cc: Boris Brezillon , Thierry Reding , linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org, Fabio Estevam , Fabio Estevam , Lothar Wassmann , Bhuvanchandra DV , kernel@pengutronix.de Subject: Re: [PATCH 0/6] pwm: imx: Provide atomic operation for IMX PWM driver Message-ID: <20161025090724.727c10bc@jawa> In-Reply-To: <35a950b7ff431e0545bbb259ba69b483@agner.ch> References: <1477259146-19167-1-git-send-email-l.majewski@majess.pl> <20161024173607.36bc2cb9@bbrezillon> <20161024232615.77d4ea28@jawa> <35a950b7ff431e0545bbb259ba69b483@agner.ch> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/6.BrC5b98dV1RSa+MU+emnv"; protocol="application/pgp-signature" X-Ovh-Tracer-Id: 342273572941054535 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeelvddrieeigdduudelucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddm Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5289 Lines: 161 --Sig_/6.BrC5b98dV1RSa+MU+emnv Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Stefan, > Hi Lukasz, >=20 > Thanks for your work, great to see this coming along! :-) >=20 > On 2016-10-24 23:26, Lukasz Majewski wrote: > > Hi Boris, > >=20 > >> On Sun, 23 Oct 2016 23:45:40 +0200 > >> Lukasz Majewski wrote: > >> > >> > This patch set brings atomic operation to i.MX's PWMv2 driver. > >> > > >> > This work has been supported and suggested by Boris Brezillon [1] > >> > and Stefan Agner, by showing how simple the transition could > >> > be :-). > >> > > >> > It has been divided into several steps: > >> > - Separate PWMv1 commits from "generic" and non atomic PWM code. > >> > > >> > NOTE: Since I do not have board with PWMv1, I would like to ask > >> > somebody for testing > >> > > >> > - Move some imx_config_v2 code to separate functions > >> > > >> > - Provide PWM atomic implementation (the ->apply() driver) in a > >> > single patch for better readability. > >> > > >> > - Remove redundant PWM code (disable, enable, config callbacks) > >> > > >> > - Clean up the driver infrastructure > >> > > >> > - Provide "polarity_supported" flag to indicate support for > >> > polarity inversion > >> > > >> > This work should be applied on top of following commits: > >> > > >> > http://patchwork.ozlabs.org/patch/679706/ > > [2] > >=20 > >> > http://patchwork.ozlabs.org/patch/679707/ > > [3] > >=20 > >> > http://patchwork.ozlabs.org/patch/679680/ > >> > >> I'm not sure I follow the logic here. Has patch [1] already been > >> applied? If that's not the case, then you should just drop it and > >> put your changes on top of mainline. > >> > >> [1]http://patchwork.ozlabs.org/patch/679680/ > >=20 > > Patches [2] and [3] have been developed initially by Lothar and > > subsequently picked up by Bhuvanchandra. There is no issue with > > them. >=20 > As such none of this will get merged since all patchset have known > flaws... >=20 > Generally, it is ok to refer to other patchset being a prerequisite, > but that only makes sense if those patch set are still actively > worked on (by somebody other than you). >=20 > In this case I really recommend to create a new, complete patchset. >=20 > >=20 > > The patch [1] is a bit more tricky. The work has been done by > > Bhuvanchandra, which adds DTS and core support for polarity > > inversion. > >=20 > > This code works and utilizes the "old" PWM API with enable, disable > > and config. However, Stefan had some comments about the placement > > for the polarity setting (in the .config_v2()) and proposed switch > > to atomic API. >=20 > Part of the reason I advocated for the atomic API is to make adding > the polarity functionality easier. It does not archive this goal if > we add the "flawed" code first and then transition to the atomic API. >=20 > >=20 > > To make things easier and cleaner, I decided to put my atomic API > > rework on top of those patches. In this way I can credit the > > previous work and avoid rewriting DTS polarity inversion code > > already developed and validated by Bhuvanchandra. >=20 > When you apply the patches using git apply, the authorship and > signoffs will stay. There is no problem in including other peoples > work into your patchset, credit will still be given. If you have to > change another persons patch, you typically also add your signoff to > show that you worked on it too. I do wanted to reuse as much work as possible (especially that the code was working). >=20 > Here is how I would do it: >=20 > 1. Start a new branch from mainline (or even -next). With the newest mainline 4.9-rcX SHA:0c2b6dc4fd4fa13796b319aae969a009f03222c6 the i.MX6q is not booting. Apparently I do need to wait for things to calm down. The last working version is v4.8, which PWM's code is the same as v4.7. > 2. Implement the transition to the new atomic API and test it as such > alone (this way we have no polarity support influence yet, just clean > transition to a new API) I _just_ needed to add polarity support (by setting one bit) to the driver, so I ended up with rewriting the whole PWM i.MX driver :-). > 3. Cherry pick the PWM core changes for the optional 2/3 args driver > support (they should apply cleanly) > 4. Cherry pick (they likely will fail to merge) or reimplement the PWM > polarity driver changes on top of atomic API > 5. Cherry pick device tree changes >=20 > With this approach we'll end up with a nice history where we should > end up with a fully functional PWM system between every patch. I'm fine with proposed approach. I will prepare v2 of patches soon. >=20 > Btw, past perfect tense is not really usual in commit messages. > SubmittingPatches chapter 2 has some tips on writing good commit > messages: > https://www.kernel.org/doc/Documentation/SubmittingPatches OK. Thanks you for your support, Best regards, =C5=81ukasz Majewski >=20 > -- > Stefan --Sig_/6.BrC5b98dV1RSa+MU+emnv Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlgPBLIACgkQf9/hG2YwgjFLtgCgzgTqnVOV3iFp12RsW9nX2XBW /TEAn1JaO81ULIhp37c2bA5s9zk6e7+9 =cBed -----END PGP SIGNATURE----- --Sig_/6.BrC5b98dV1RSa+MU+emnv--