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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 07 Sep 2017, Takashi Iwai wrote:

> 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?

Yes, clearly. ;)

Okay, I think we've spent enough time discussing this now.  All of the
information you need is contained in my previous emails.  You now know
all of the reasons for why we don't create immutable branches for
every patch-set that comes in -- just ones which real dependencies.

It's time to agree to disagree and get on with some real work. :)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux