Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC

[Takashi Iwai]

On Thu, 07 Sep 2017 14:24:00 +0200,
Lee Jones wrote:
> 
> On Thu, 07 Sep 2017, Takashi Iwai wrote:
> 
> > On Thu, 07 Sep 2017 13:17:47 +0200,
> > Lee Jones wrote:
> > > 
> > > On Thu, 07 Sep 2017, Rafael J. Wysocki wrote:
> > > 
> > > > On Thursday, September 7, 2017 12:53:48 PM CEST Lee Jones wrote:
> > > > > On Thu, 07 Sep 2017, Takashi Iwai wrote:
> > > > > 
> > > > > > On Tue, 05 Sep 2017 10:54:49 +0200,
> > > > > > Lee Jones wrote:
> > > > > > > 
> > > > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote:
> > > > > > > 
> > > > > > > > On Tue, 05 Sep 2017 10:10:49 +0200,
> > > > > > > > Lee Jones wrote:
> > > > > > > > > 
> > > > > > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote:
> > > > > > > > > 
> > > > > > > > > > On Tue, 05 Sep 2017 09:24:51 +0200,
> > > > > > > > > > Lee Jones wrote:
> > > > > > > > > > > 
> > > > > > > > > > > On Mon, 04 Sep 2017, Takashi Iwai wrote:
> > > > > > > > > > > 
> > > > > > > > > > > > This patch adds the MFD driver for Dollar Cove (TI version) PMIC with
> > > > > > > > > > > > ACPI INT33F5 that is found on some Intel Cherry Trail devices.
> > > > > > > > > > > > The driver is based on the original work by Intel, found at:
> > > > > > > > > > > >   https://github.com/01org/ProductionKernelQuilts
> > > > > > > > > > > > 
> > > > > > > > > > > > This is a minimal version for adding the basic resources.  Currently,
> > > > > > > > > > > > only ACPI PMIC opregion and the external power-button are used.
> > > > > > > > > > > > 
> > > > > > > > > > > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=193891
> > > > > > > > > > > > Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> > > > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> > > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > > > > > > > > > > > ---
> > > > > > > > > > > > v4->v5:
> > > > > > > > > > > > * Minor coding-style fixes suggested by Lee
> > > > > > > > > > > > * Put GPL text
> > > > > > > > > > > > v3->v4:
> > > > > > > > > > > > * no change for this patch
> > > > > > > > > > > > v2->v3:
> > > > > > > > > > > > * Rename dc_ti with chtdc_ti in all places
> > > > > > > > > > > > * Driver/kconfig renames accordingly
> > > > > > > > > > > > * Added acks by Andy and Mika
> > > > > > > > > > > > v1->v2:
> > > > > > > > > > > > * Minor cleanups as suggested by Andy
> > > > > > > > > > > > 
> > > > > > > > > > > >  drivers/mfd/Kconfig                   |  13 +++
> > > > > > > > > > > >  drivers/mfd/Makefile                  |   1 +
> > > > > > > > > > > >  drivers/mfd/intel_soc_pmic_chtdc_ti.c | 184 ++++++++++++++++++++++++++++++++++
> > > > > > > > > > > >  3 files changed, 198 insertions(+)
> > > > > > > > > > > >  create mode 100644 drivers/mfd/intel_soc_pmic_chtdc_ti.c
> > > > > > > > > > > 
> > > > > > > > > > > For my own reference:
> > > > > > > > > > >   Acked-for-MFD-by: Lee Jones <lee.jones@xxxxxxxxxx>
> > > > > > > > > > 
> > > > > > > > > > Thanks!
> > > > > > > > > > 
> > > > > > > > > > Now the question is how to deal with these.  It's no critical things,
> > > > > > > > > > so I'm OK to postpone for 4.15.  OTOH, it's really a new
> > > > > > > > > > device-specific stuff, thus it can't break anything else, and it'd be
> > > > > > > > > > fairly safe to add it for 4.14 although it's at a bit late stage.
> > > > > > > > > 
> > > > > > > > > Yes, you are over 2 weeks late for v4.14.  It will have to be v4.15.
> > > > > > > > 
> > > > > > > > OK, I'll ring your bells again once when 4.15 development is opened.
> > > > > > > 
> > > > > > > Please don't.  Just collect all the Acks you have received and sent
> > > > > > > out the set again changing [PATCH] for [RESEND].  Only if there
> > > > > > > haven't been any code changes of course. 
> > > > > >   
> > > > > > You seem to have applied the patches in some branch, but still do I
> > > > > > need to resend the whole patches?
> > > > > 
> > > > > That's up to the Platform Maintainers.
> > > > > 
> > > > > Since the MFD and ACPI are applied, you do not need to resend those.
> > > > > 
> > > > > > BTW, was patch 2/3 applied?  I miss your notification mail.
> > > > > 
> > > > > Patch 2 needs to be applied into the Platform tree.
> > > > 
> > > > Actually, that's drivers/platform/x86/ so it falls under
> > > > X86 PLATFORM DRIVERS and it has already been ACKed by Andy who is
> > > > one of the maintainers of that.
> > > > 
> > > > Andy, Darren, can patch [2/3] from this series go in via the MFD tree?
> > > 
> > > ... without an immutable branch.
> > 
> > Why can't it be immutable?  You just don't branch off the commit
> > containing these three patches, and don't touch any longer on that, so
> > that other trees can pull it in; it's all what we need.
> 
> Firstly, the MFD main branch is not immutable.

That's bad in general :)

> Secondly, you cannot simply pull a patch in from another tree.

Many trees do it sometimes as long as the branch is guaranteed to be
persistent / immutable, prepared for the next kernel.

> All of
> the patches before it and your local base will be pulled in too.  This
> is why we create purposely created immutable branches which *only*
> contain the patches required by other repos.

Yes, ideally we should have a clean base for such a shared branch.

> > > Which also means all changes to this
> > > file due for v4.15 would have to come in via the MFD tree too.
> > > 
> > > This is sub optimal IMHO.  I'd rather it was fed through the proper
> > > tree(s), so we can continue our normal flow for subsequent changes.
> > 
> > Other trees sometimes pull this kind of "immutable" branches, too.
> > I see no big problem there.
> 
> There are no valid reason to create an IB for this case.

I don't understand why makes it so difficult.  Looking at the
procedure you mentioned in this thread:

==
1. Checkout a new branch based on the same (or earlier, but I always
use the same) commit as the main branch.

2. Apply the patches in the normal way, only this time you usually need
to interactively rebase and 'reword' them to add any missing
Acks/Reviewed-bys.

3. Tag and sign the branch with a suitable tag name and tag
description. 

4. Push branch and tag

5. Create pull-request text

6. Send a formatted email with the pull-request text.
==

1 is easy, you can take 4.14-rc1 once when released.  This kind of
  thing is done by many people regularly for opening a topic
  development branch.

2 is the very standard procedure.  You'd need to add tags in anyway
  even for normal branches.

3, the signed tagging can be optional, only when requested.
  Of course, it's safer to have one, but not mandatory.
  Branch naming?  It's not more difficult than thinking of the next
  android version code name :)

4, normal procedure. 

5 and 6 are relaxed in this case, just a one-line notification mail to
  this thread should suffice.


... so I really can't see any specially time-consuming or exhausting
step in the above.  Usually most time-consuming stuff is rather the
code review and the build test.  But this isn't different from the
normal case.

Or am I missing anything?


thanks,

Takashi