Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1726124 > unrolled thread
| Started by | Takashi Iwai <tiwai@suse.de> |
|---|---|
| First post | 2017-09-04 16:50 +0200 |
| Last post | 2017-09-07 10:10 +0200 |
| Articles | 12 on this page of 52 — 6 participants |
Back to article view | Back to linux.kernel
[PATCH v5 0/3] Dollar Cove TI PMIC support for Intel Cherry Trail Takashi Iwai <tiwai@suse.de> - 2017-09-04 16:50 +0200
[PATCH v5 2/3] platform/x86: Add support for Dollar Cove TI power button Takashi Iwai <tiwai@suse.de> - 2017-09-04 16:50 +0200
[PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-04 16:50 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-05 09:30 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-05 09:50 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Hans de Goede <hdegoede@redhat.com> - 2017-09-05 10:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-05 10:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-05 10:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-05 10:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-05 10:30 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-05 11:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-07 11:40 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 13:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-09-07 13:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 13:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-07 13:50 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 14:30 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-07 15:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 15:30 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-07 13:50 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 14:30 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-07 14:50 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 15:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-07 15:40 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 16:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-05 11:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-05 11:40 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC "Rafael J. Wysocki" <rafael@kernel.org> - 2017-09-05 12:40 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 10:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-09-06 12:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 12:50 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 13:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-09-07 00:30 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 09:40 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-09-07 13:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-09-07 13:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-09-07 13:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 13:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 10:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-06 10:30 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 11:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-06 12:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-09-06 12:40 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 13:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 12:50 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-06 13:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-09-06 13:20 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 16:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-06 16:40 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-06 17:00 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Takashi Iwai <tiwai@suse.de> - 2017-09-06 17:10 +0200
Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC Lee Jones <lee.jones@linaro.org> - 2017-09-07 10:10 +0200
Page 3 of 3 — ← Prev page 1 2 [3]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-06 11:10 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umEvq-7XC-57@gated-at.bofh.it> |
| In reply to | #1727217 |
On Wed, 06 Sep 2017, Takashi Iwai wrote: > On Wed, 06 Sep 2017 09:54:44 +0200, > Lee Jones wrote: > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote: > > > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com> > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > > > > > > > > --- > > > > > > > > > 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@linaro.org> > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or > > > > > > > create an immutable branch (again). But how to handle it, when to do > > > > > > > it, It's all up to you guys. > > > > > > > > > > > > If there aren't any build dependencies between the patches, each of > > > > > > the patches should be applied through their own trees. What are the > > > > > > build-time dependencies? Are there any? > > > > > > > > > > No, there is no strict build-time dependency. It's just that I don't > > > > > see it nice to have a commit for a dead code, partly for testing > > > > > purpose and partly for code consistency. But if this makes > > > > > maintenance easier, I'm happy with that, too, of course. > > > > > > > > There won't be any dead code. All of the subsystem trees are pulled > > > > into -next [0] where the build bots can operate on the patches as a > > > > whole. > > > > > > But the merge order isn't guaranteed, i.e. at the commit of other tree > > > for this new stuff, it's a dead code without merging the MFD stuff > > > beforehand. e.g. Imagine to perform the git bisection. It's not > > > about the whole tree, but about the each commit. > > > > Only *building* is relevant for bisection until the whole feature > > lands. > > Why only building? > > When merging through several tress, commits for the same series are > scattered completely although they are softly tied. This sucks when > you perform git bisection, e.g. if you have an issue in the middle of > the patch series. It still works, but it jumps unnecessarily too far > away and back before reaching to the point, and kconfig appears / > disappears inconsistently (the dependent kconfig gone in the middle). > And, this is about the release kernel (4.15 or whatever). Think about how bisection works. You state a good commit and a bad one. The good commit will be when the feature last worked, which will not be until the feature has fully landed. Bisect will not check any point prior to this date. If there aren't any build deps, each Maintainer will apply patches into their own tree. These will be merged together in -next where they can be tested, both manually and by the 0-days. Once the merge window is opened all patches will be sucked into -rc1. If the feature works here, then it you could use -rc1 as your 'good' commit. If it doesn't, then this could indicate a merge error or a missing piece of the set, either way bisect wouldn't help you. > Basically, my complaint here comes with my user's hat on. It *is* > indeed worse than a straight application of patches in some levels. > It's unavoidable if you do in that way. I disagree. I user wouldn't set the 'good' commit at any point prior to the feature working at least once. This will not happen if any parts were missing. The order in which the pieces are applied is irrelevant if there aren't any build deps between them. If you bisect between them, then the driver simply will not build. No problem. > OTOH, with maintainer's hat on, I do agree with that it'll make things > often easier. Judging with these merits and demerits, I find it's > acceptable, too. > > > No one is going to bisect the function of a feature until it > > is present. So long as there aren't any build-time dependencies then > > we're good, > > > > > And I won't be surprised if 0-day build bot gets a new feature to > > > inspect the kconfig files, spot a dead kconfig entry and warn > > > maintainers at each commit, too :) > > > > > > 0-days don't check for that and static analysers only check releases. > > How can you guarantee that 0-days will not do that in future? > I learned that I shouldn't be too naive about 0-day bot facility :) Due to the way we operate, testing for dead code in between releases would be foolish, since there would be too many false positives. Static analysis is best for this and they are normally run on releases. > In anyway, as I already mentioned, I'm fine with taking changes > individually in each tree. No need for further bike-shedding. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [next] | [standalone]
| From | Takashi Iwai <tiwai@suse.de> |
|---|---|
| Date | 2017-09-06 12:10 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umFrs-dJ-17@gated-at.bofh.it> |
| In reply to | #1727267 |
On Wed, 06 Sep 2017 11:05:04 +0200,
Lee Jones wrote:
>
> On Wed, 06 Sep 2017, Takashi Iwai wrote:
>
> > On Wed, 06 Sep 2017 09:54:44 +0200,
> > Lee Jones wrote:
> > >
> > > On Tue, 05 Sep 2017, Takashi Iwai wrote:
> > >
> > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com>
> > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > ---
> > > > > > > > > > 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@linaro.org>
> > > > > > > >
> > > > > > > > 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.
> > > > > >
> > > > > >
> > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or
> > > > > > > > create an immutable branch (again). But how to handle it, when to do
> > > > > > > > it, It's all up to you guys.
> > > > > > >
> > > > > > > If there aren't any build dependencies between the patches, each of
> > > > > > > the patches should be applied through their own trees. What are the
> > > > > > > build-time dependencies? Are there any?
> > > > > >
> > > > > > No, there is no strict build-time dependency. It's just that I don't
> > > > > > see it nice to have a commit for a dead code, partly for testing
> > > > > > purpose and partly for code consistency. But if this makes
> > > > > > maintenance easier, I'm happy with that, too, of course.
> > > > >
> > > > > There won't be any dead code. All of the subsystem trees are pulled
> > > > > into -next [0] where the build bots can operate on the patches as a
> > > > > whole.
> > > >
> > > > But the merge order isn't guaranteed, i.e. at the commit of other tree
> > > > for this new stuff, it's a dead code without merging the MFD stuff
> > > > beforehand. e.g. Imagine to perform the git bisection. It's not
> > > > about the whole tree, but about the each commit.
> > >
> > > Only *building* is relevant for bisection until the whole feature
> > > lands.
> >
> > Why only building?
> >
> > When merging through several tress, commits for the same series are
> > scattered completely although they are softly tied. This sucks when
> > you perform git bisection, e.g. if you have an issue in the middle of
> > the patch series. It still works, but it jumps unnecessarily too far
> > away and back before reaching to the point, and kconfig appears /
> > disappears inconsistently (the dependent kconfig gone in the middle).
> > And, this is about the release kernel (4.15 or whatever).
>
> Think about how bisection works. You state a good commit and a bad
> one. The good commit will be when the feature last worked, which will
> not be until the feature has fully landed. Bisect will not check any
> point prior to this date.
>
> If there aren't any build deps, each Maintainer will apply patches
> into their own tree. These will be merged together in -next where
> they can be tested, both manually and by the 0-days. Once the merge
> window is opened all patches will be sucked into -rc1. If the feature
> works here, then it you could use -rc1 as your 'good' commit. If it
> doesn't, then this could indicate a merge error or a missing piece of
> the set, either way bisect wouldn't help you.
Not really.
First of all, most of user start testing from the release kernel, so
you can't trust that RC covered all test cases. (Who can blame users
who didn't use / test RC?)
Second, you ignore the fact that the development continues after
merging *this* patchset. What if a breakage is introduced after this
patch? (See below)
They often need a full bisection between the previous release and the
current release.
> > Basically, my complaint here comes with my user's hat on. It *is*
> > indeed worse than a straight application of patches in some levels.
> > It's unavoidable if you do in that way.
>
> I disagree. I user wouldn't set the 'good' commit at any point prior
> to the feature working at least once.
Again no, not all users do test at the same time. A device driver
may support multiple devices / platforms, and it might be that the bug
manifests itself only on a certain system that no one has tested
beforehand; it's a typical case we often see after the releases.
> This will not happen if any
> parts were missing. The order in which the pieces are applied is
> irrelevant if there aren't any build deps between them. If you bisect
> between them, then the driver simply will not build. No problem.
The bisection is required not only for build errors but also for
functional tests. Sometimes it's the only way to spot the culprit of
a functional regression.
Imagine the following case: both MFD and platform drivers are merged
into individual trees separately. And, some change in platform driver
after this patch series broke some functionality mistakenly.
Now, suppose that the platform tree is merged to Linus tree before MFD
tree, that is, the situation is like:
MFD (commit A1) -- ... -------------------------+
platform (commit B1) -> broken change (B2) -+ |
v v
--M1--M2--> Linus RC1
git-bisection can inspect M2, which shows already broken. Then it
inspects M1, but you can't evaluate it because the target platform
driver can't be built without A1. At this point, you're stuck.
But in this case, B1 is correct and B2 is the culprit. How can you
spot it out?
OTOH, if the merge history honors the functional dependency, you can
bisect properly.
thanks,
Takashi
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2017-09-06 12:40 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umFUt-oN-3@gated-at.bofh.it> |
| In reply to | #1727311 |
On Wednesday, September 6, 2017 12:06:50 PM CEST Takashi Iwai wrote: > On Wed, 06 Sep 2017 11:05:04 +0200, > Lee Jones wrote: > > > > On Wed, 06 Sep 2017, Takashi Iwai wrote: > > > > > On Wed, 06 Sep 2017 09:54:44 +0200, > > > Lee Jones wrote: > > > > > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote: > > > > > > > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com> > > > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > > > > > > > > > > --- > > > > > > > > > > > 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@linaro.org> > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or > > > > > > > > > create an immutable branch (again). But how to handle it, when to do > > > > > > > > > it, It's all up to you guys. > > > > > > > > > > > > > > > > If there aren't any build dependencies between the patches, each of > > > > > > > > the patches should be applied through their own trees. What are the > > > > > > > > build-time dependencies? Are there any? > > > > > > > > > > > > > > No, there is no strict build-time dependency. It's just that I don't > > > > > > > see it nice to have a commit for a dead code, partly for testing > > > > > > > purpose and partly for code consistency. But if this makes > > > > > > > maintenance easier, I'm happy with that, too, of course. > > > > > > > > > > > > There won't be any dead code. All of the subsystem trees are pulled > > > > > > into -next [0] where the build bots can operate on the patches as a > > > > > > whole. > > > > > > > > > > But the merge order isn't guaranteed, i.e. at the commit of other tree > > > > > for this new stuff, it's a dead code without merging the MFD stuff > > > > > beforehand. e.g. Imagine to perform the git bisection. It's not > > > > > about the whole tree, but about the each commit. > > > > > > > > Only *building* is relevant for bisection until the whole feature > > > > lands. > > > > > > Why only building? > > > > > > When merging through several tress, commits for the same series are > > > scattered completely although they are softly tied. This sucks when > > > you perform git bisection, e.g. if you have an issue in the middle of > > > the patch series. It still works, but it jumps unnecessarily too far > > > away and back before reaching to the point, and kconfig appears / > > > disappears inconsistently (the dependent kconfig gone in the middle). > > > And, this is about the release kernel (4.15 or whatever). > > > > Think about how bisection works. You state a good commit and a bad > > one. The good commit will be when the feature last worked, which will > > not be until the feature has fully landed. Bisect will not check any > > point prior to this date. > > > > If there aren't any build deps, each Maintainer will apply patches > > into their own tree. These will be merged together in -next where > > they can be tested, both manually and by the 0-days. Once the merge > > window is opened all patches will be sucked into -rc1. If the feature > > works here, then it you could use -rc1 as your 'good' commit. If it > > doesn't, then this could indicate a merge error or a missing piece of > > the set, either way bisect wouldn't help you. > > Not really. > > First of all, most of user start testing from the release kernel, so > you can't trust that RC covered all test cases. (Who can blame users > who didn't use / test RC?) > > Second, you ignore the fact that the development continues after > merging *this* patchset. What if a breakage is introduced after this > patch? (See below) > > They often need a full bisection between the previous release and the > current release. > > > > Basically, my complaint here comes with my user's hat on. It *is* > > > indeed worse than a straight application of patches in some levels. > > > It's unavoidable if you do in that way. > > > > I disagree. I user wouldn't set the 'good' commit at any point prior > > to the feature working at least once. > > Again no, not all users do test at the same time. A device driver > may support multiple devices / platforms, and it might be that the bug > manifests itself only on a certain system that no one has tested > beforehand; it's a typical case we often see after the releases. > > > This will not happen if any > > parts were missing. The order in which the pieces are applied is > > irrelevant if there aren't any build deps between them. If you bisect > > between them, then the driver simply will not build. No problem. > > The bisection is required not only for build errors but also for > functional tests. Sometimes it's the only way to spot the culprit of > a functional regression. > > Imagine the following case: both MFD and platform drivers are merged > into individual trees separately. And, some change in platform driver > after this patch series broke some functionality mistakenly. > > Now, suppose that the platform tree is merged to Linus tree before MFD > tree, that is, the situation is like: > > MFD (commit A1) -- ... -------------------------+ > platform (commit B1) -> broken change (B2) -+ | > v v > --M1--M2--> Linus RC1 > > git-bisection can inspect M2, which shows already broken. Then it > inspects M1, but you can't evaluate it because the target platform > driver can't be built without A1. At this point, you're stuck. > But in this case, B1 is correct and B2 is the culprit. How can you > spot it out? > > OTOH, if the merge history honors the functional dependency, you can > bisect properly. Exactly and not only that. If the git history reflects the logical dependencies between patches, you should be able to figure out the reason why the code was changed this way (sometimes you can't anyway if commit logs suck, for example, but this is a different problem) which quite often is essential for debugging, backporting and similar. Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-06 13:00 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umGdQ-wH-23@gated-at.bofh.it> |
| In reply to | #1727324 |
On Wed, 06 Sep 2017, Rafael J. Wysocki wrote: > On Wednesday, September 6, 2017 12:06:50 PM CEST Takashi Iwai wrote: > > On Wed, 06 Sep 2017 11:05:04 +0200, > > Lee Jones wrote: > > > > > > On Wed, 06 Sep 2017, Takashi Iwai wrote: > > > > > > > On Wed, 06 Sep 2017 09:54:44 +0200, > > > > Lee Jones wrote: > > > > > > > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote: > > > > > > > > > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com> > > > > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > > > > > > > > > > > --- > > > > > > > > > > > > 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@linaro.org> > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or > > > > > > > > > > create an immutable branch (again). But how to handle it, when to do > > > > > > > > > > it, It's all up to you guys. > > > > > > > > > > > > > > > > > > If there aren't any build dependencies between the patches, each of > > > > > > > > > the patches should be applied through their own trees. What are the > > > > > > > > > build-time dependencies? Are there any? > > > > > > > > > > > > > > > > No, there is no strict build-time dependency. It's just that I don't > > > > > > > > see it nice to have a commit for a dead code, partly for testing > > > > > > > > purpose and partly for code consistency. But if this makes > > > > > > > > maintenance easier, I'm happy with that, too, of course. > > > > > > > > > > > > > > There won't be any dead code. All of the subsystem trees are pulled > > > > > > > into -next [0] where the build bots can operate on the patches as a > > > > > > > whole. > > > > > > > > > > > > But the merge order isn't guaranteed, i.e. at the commit of other tree > > > > > > for this new stuff, it's a dead code without merging the MFD stuff > > > > > > beforehand. e.g. Imagine to perform the git bisection. It's not > > > > > > about the whole tree, but about the each commit. > > > > > > > > > > Only *building* is relevant for bisection until the whole feature > > > > > lands. > > > > > > > > Why only building? > > > > > > > > When merging through several tress, commits for the same series are > > > > scattered completely although they are softly tied. This sucks when > > > > you perform git bisection, e.g. if you have an issue in the middle of > > > > the patch series. It still works, but it jumps unnecessarily too far > > > > away and back before reaching to the point, and kconfig appears / > > > > disappears inconsistently (the dependent kconfig gone in the middle). > > > > And, this is about the release kernel (4.15 or whatever). > > > > > > Think about how bisection works. You state a good commit and a bad > > > one. The good commit will be when the feature last worked, which will > > > not be until the feature has fully landed. Bisect will not check any > > > point prior to this date. > > > > > > If there aren't any build deps, each Maintainer will apply patches > > > into their own tree. These will be merged together in -next where > > > they can be tested, both manually and by the 0-days. Once the merge > > > window is opened all patches will be sucked into -rc1. If the feature > > > works here, then it you could use -rc1 as your 'good' commit. If it > > > doesn't, then this could indicate a merge error or a missing piece of > > > the set, either way bisect wouldn't help you. > > > > Not really. > > > > First of all, most of user start testing from the release kernel, so > > you can't trust that RC covered all test cases. (Who can blame users > > who didn't use / test RC?) > > > > Second, you ignore the fact that the development continues after > > merging *this* patchset. What if a breakage is introduced after this > > patch? (See below) > > > > They often need a full bisection between the previous release and the > > current release. > > > > > > Basically, my complaint here comes with my user's hat on. It *is* > > > > indeed worse than a straight application of patches in some levels. > > > > It's unavoidable if you do in that way. > > > > > > I disagree. I user wouldn't set the 'good' commit at any point prior > > > to the feature working at least once. > > > > Again no, not all users do test at the same time. A device driver > > may support multiple devices / platforms, and it might be that the bug > > manifests itself only on a certain system that no one has tested > > beforehand; it's a typical case we often see after the releases. > > > > > This will not happen if any > > > parts were missing. The order in which the pieces are applied is > > > irrelevant if there aren't any build deps between them. If you bisect > > > between them, then the driver simply will not build. No problem. > > > > The bisection is required not only for build errors but also for > > functional tests. Sometimes it's the only way to spot the culprit of > > a functional regression. > > > > Imagine the following case: both MFD and platform drivers are merged > > into individual trees separately. And, some change in platform driver > > after this patch series broke some functionality mistakenly. > > > > Now, suppose that the platform tree is merged to Linus tree before MFD > > tree, that is, the situation is like: > > > > MFD (commit A1) -- ... -------------------------+ > > platform (commit B1) -> broken change (B2) -+ | > > v v > > --M1--M2--> Linus RC1 > > > > git-bisection can inspect M2, which shows already broken. Then it > > inspects M1, but you can't evaluate it because the target platform > > driver can't be built without A1. At this point, you're stuck. > > But in this case, B1 is correct and B2 is the culprit. How can you > > spot it out? > > > > OTOH, if the merge history honors the functional dependency, you can > > bisect properly. > > Exactly and not only that. > > If the git history reflects the logical dependencies between patches, > you should be able to figure out the reason why the code was changed > this way (sometimes you can't anyway if commit logs suck, for example, > but this is a different problem) which quite often is essential for > debugging, backporting and similar. The example you reference is a really small corner case (I don't recall an occurrence) and is very easy to debug without bisect. The pros of taking patches in via their pre-defined subsystem trees far outweighs the slight possibility and the debugging complexity of this example. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-06 12:50 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umG4a-sH-17@gated-at.bofh.it> |
| In reply to | #1727311 |
On Wed, 06 Sep 2017, Takashi Iwai wrote:
> On Wed, 06 Sep 2017 11:05:04 +0200,
> Lee Jones wrote:
> >
> > On Wed, 06 Sep 2017, Takashi Iwai wrote:
> >
> > > On Wed, 06 Sep 2017 09:54:44 +0200,
> > > Lee Jones wrote:
> > > >
> > > > On Tue, 05 Sep 2017, Takashi Iwai wrote:
> > > >
> > > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com>
> > > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > > ---
> > > > > > > > > > > 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@linaro.org>
> > > > > > > > >
> > > > > > > > > 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.
> > > > > > >
> > > > > > >
> > > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or
> > > > > > > > > create an immutable branch (again). But how to handle it, when to do
> > > > > > > > > it, It's all up to you guys.
> > > > > > > >
> > > > > > > > If there aren't any build dependencies between the patches, each of
> > > > > > > > the patches should be applied through their own trees. What are the
> > > > > > > > build-time dependencies? Are there any?
> > > > > > >
> > > > > > > No, there is no strict build-time dependency. It's just that I don't
> > > > > > > see it nice to have a commit for a dead code, partly for testing
> > > > > > > purpose and partly for code consistency. But if this makes
> > > > > > > maintenance easier, I'm happy with that, too, of course.
> > > > > >
> > > > > > There won't be any dead code. All of the subsystem trees are pulled
> > > > > > into -next [0] where the build bots can operate on the patches as a
> > > > > > whole.
> > > > >
> > > > > But the merge order isn't guaranteed, i.e. at the commit of other tree
> > > > > for this new stuff, it's a dead code without merging the MFD stuff
> > > > > beforehand. e.g. Imagine to perform the git bisection. It's not
> > > > > about the whole tree, but about the each commit.
> > > >
> > > > Only *building* is relevant for bisection until the whole feature
> > > > lands.
> > >
> > > Why only building?
> > >
> > > When merging through several tress, commits for the same series are
> > > scattered completely although they are softly tied. This sucks when
> > > you perform git bisection, e.g. if you have an issue in the middle of
> > > the patch series. It still works, but it jumps unnecessarily too far
> > > away and back before reaching to the point, and kconfig appears /
> > > disappears inconsistently (the dependent kconfig gone in the middle).
> > > And, this is about the release kernel (4.15 or whatever).
> >
> > Think about how bisection works. You state a good commit and a bad
> > one. The good commit will be when the feature last worked, which will
> > not be until the feature has fully landed. Bisect will not check any
> > point prior to this date.
> >
> > If there aren't any build deps, each Maintainer will apply patches
> > into their own tree. These will be merged together in -next where
> > they can be tested, both manually and by the 0-days. Once the merge
> > window is opened all patches will be sucked into -rc1. If the feature
> > works here, then it you could use -rc1 as your 'good' commit. If it
> > doesn't, then this could indicate a merge error or a missing piece of
> > the set, either way bisect wouldn't help you.
>
> Not really.
>
> First of all, most of user start testing from the release kernel, so
> you can't trust that RC covered all test cases. (Who can blame users
> who didn't use / test RC?)
That's fine. We are not talking about spreading the merge of a
patch-set over different releases, or even release candidates. All
non-bugfix patches should be in by -rc1.
> Second, you ignore the fact that the development continues after
> merging *this* patchset. What if a breakage is introduced after this
> patch? (See below)
>
> They often need a full bisection between the previous release and the
> current release.
You cannot bisect a specific function back before it was merged. It's
impossible.
P1 ---> P4 ---> P3 ---> P2
^
Bisect only starts working here. Prior to this point the feature
doesn't build at all. If it builds, but breaks, then that is a build
dependency and is a different use-case to what we are discussing here.
> > > Basically, my complaint here comes with my user's hat on. It *is*
> > > indeed worse than a straight application of patches in some levels.
> > > It's unavoidable if you do in that way.
> >
> > I disagree. I user wouldn't set the 'good' commit at any point prior
> > to the feature working at least once.
>
> Again no, not all users do test at the same time. A device driver
> may support multiple devices / platforms, and it might be that the bug
> manifests itself only on a certain system that no one has tested
> beforehand; it's a typical case we often see after the releases.
If it is possible for a patch-set to break functionality, then again,
this is a dependency and needs to be handled differently.
That is not the case with your set.
> > This will not happen if any
> > parts were missing. The order in which the pieces are applied is
> > irrelevant if there aren't any build deps between them. If you bisect
> > between them, then the driver simply will not build. No problem.
>
> The bisection is required not only for build errors but also for
> functional tests. Sometimes it's the only way to spot the culprit of
> a functional regression.
>
> Imagine the following case: both MFD and platform drivers are merged
> into individual trees separately. And, some change in platform driver
> after this patch series broke some functionality mistakenly.
>
> Now, suppose that the platform tree is merged to Linus tree before MFD
> tree, that is, the situation is like:
>
> MFD (commit A1) -- ... -------------------------+
> platform (commit B1) -> broken change (B2) -+ |
> v v
> --M1--M2--> Linus RC1
>
> git-bisection can inspect M2, which shows already broken. Then it
> inspects M1, but you can't evaluate it because the target platform
> driver can't be built without A1. At this point, you're stuck.
> But in this case, B1 is correct and B2 is the culprit. How can you
> spot it out?
>
> OTOH, if the merge history honors the functional dependency, you can
> bisect properly.
You wouldn't use bisect for that use-case. We only use bisect for
features that have a working commit in Mainline. The use-case you
mention would be pretty trivial to debug without bisect.
When we tout "history must be bisectable", we mean that it must remain
possible to bisect existing functionality. Bisecting functionality
which is yet to fully land upstream is foolish.
New drivers almost never (if ever) go in as a fully working piece.
The registration parts (DT, Platform, etc) go in via one (or several)
tree(s) and the driver parts are normally split up into subsystems and
go in via their own repos. If there are *build*, *API* or *merge*
time dependencies, then we can use immutable branches and merge via
a single tree. Failing that, we split them.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [next] | [standalone]
| From | Takashi Iwai <tiwai@suse.de> |
|---|---|
| Date | 2017-09-06 13:00 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umGdQ-wH-33@gated-at.bofh.it> |
| In reply to | #1727332 |
On Wed, 06 Sep 2017 12:40:40 +0200, Lee Jones wrote: > > On Wed, 06 Sep 2017, Takashi Iwai wrote: > > > On Wed, 06 Sep 2017 11:05:04 +0200, > > Lee Jones wrote: > > > > > > On Wed, 06 Sep 2017, Takashi Iwai wrote: > > > > > > > On Wed, 06 Sep 2017 09:54:44 +0200, > > > > Lee Jones wrote: > > > > > > > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote: > > > > > > > > > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com> > > > > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > > > > > > > > > > > --- > > > > > > > > > > > > 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@linaro.org> > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or > > > > > > > > > > create an immutable branch (again). But how to handle it, when to do > > > > > > > > > > it, It's all up to you guys. > > > > > > > > > > > > > > > > > > If there aren't any build dependencies between the patches, each of > > > > > > > > > the patches should be applied through their own trees. What are the > > > > > > > > > build-time dependencies? Are there any? > > > > > > > > > > > > > > > > No, there is no strict build-time dependency. It's just that I don't > > > > > > > > see it nice to have a commit for a dead code, partly for testing > > > > > > > > purpose and partly for code consistency. But if this makes > > > > > > > > maintenance easier, I'm happy with that, too, of course. > > > > > > > > > > > > > > There won't be any dead code. All of the subsystem trees are pulled > > > > > > > into -next [0] where the build bots can operate on the patches as a > > > > > > > whole. > > > > > > > > > > > > But the merge order isn't guaranteed, i.e. at the commit of other tree > > > > > > for this new stuff, it's a dead code without merging the MFD stuff > > > > > > beforehand. e.g. Imagine to perform the git bisection. It's not > > > > > > about the whole tree, but about the each commit. > > > > > > > > > > Only *building* is relevant for bisection until the whole feature > > > > > lands. > > > > > > > > Why only building? > > > > > > > > When merging through several tress, commits for the same series are > > > > scattered completely although they are softly tied. This sucks when > > > > you perform git bisection, e.g. if you have an issue in the middle of > > > > the patch series. It still works, but it jumps unnecessarily too far > > > > away and back before reaching to the point, and kconfig appears / > > > > disappears inconsistently (the dependent kconfig gone in the middle). > > > > And, this is about the release kernel (4.15 or whatever). > > > > > > Think about how bisection works. You state a good commit and a bad > > > one. The good commit will be when the feature last worked, which will > > > not be until the feature has fully landed. Bisect will not check any > > > point prior to this date. > > > > > > If there aren't any build deps, each Maintainer will apply patches > > > into their own tree. These will be merged together in -next where > > > they can be tested, both manually and by the 0-days. Once the merge > > > window is opened all patches will be sucked into -rc1. If the feature > > > works here, then it you could use -rc1 as your 'good' commit. If it > > > doesn't, then this could indicate a merge error or a missing piece of > > > the set, either way bisect wouldn't help you. > > > > Not really. > > > > First of all, most of user start testing from the release kernel, so > > you can't trust that RC covered all test cases. (Who can blame users > > who didn't use / test RC?) > > That's fine. We are not talking about spreading the merge of a > patch-set over different releases, or even release candidates. All > non-bugfix patches should be in by -rc1. > > > Second, you ignore the fact that the development continues after > > merging *this* patchset. What if a breakage is introduced after this > > patch? (See below) > > > > They often need a full bisection between the previous release and the > > current release. > > You cannot bisect a specific function back before it was merged. It's > impossible. > > P1 ---> P4 ---> P3 ---> P2 > ^ > Bisect only starts working here. Prior to this point the feature > doesn't build at all. If it builds, but breaks, then that is a build > dependency and is a different use-case to what we are discussing here. OK, let me rephrase. With the scenario above, user *cannot* perform bisect. Meanwhile, with the straight merge, you can bisect a breakage. That's a significant difference. I.e. in the case where both commits A1, B1 and B2 are merged through tree A an B. B2 is the breakage. With separate tree merges, you cannot bisect, while the straight merge allows you to bisect B2. Takashi
[toc] | [prev] | [next] | [standalone]
| From | "Rafael J. Wysocki" <rjw@rjwysocki.net> |
|---|---|
| Date | 2017-09-06 13:20 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umGxc-Yr-17@gated-at.bofh.it> |
| In reply to | #1727346 |
On Wednesday, September 6, 2017 12:58:55 PM CEST Takashi Iwai wrote: > On Wed, 06 Sep 2017 12:40:40 +0200, > Lee Jones wrote: > > > > On Wed, 06 Sep 2017, Takashi Iwai wrote: > > > > > On Wed, 06 Sep 2017 11:05:04 +0200, > > > Lee Jones wrote: > > > > > > > > On Wed, 06 Sep 2017, Takashi Iwai wrote: > > > > > > > > > On Wed, 06 Sep 2017 09:54:44 +0200, > > > > > Lee Jones wrote: > > > > > > > > > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote: > > > > > > > > > > > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com> > > > > > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > > > > > > > > > > > > --- > > > > > > > > > > > > > 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@linaro.org> > > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or > > > > > > > > > > > create an immutable branch (again). But how to handle it, when to do > > > > > > > > > > > it, It's all up to you guys. > > > > > > > > > > > > > > > > > > > > If there aren't any build dependencies between the patches, each of > > > > > > > > > > the patches should be applied through their own trees. What are the > > > > > > > > > > build-time dependencies? Are there any? > > > > > > > > > > > > > > > > > > No, there is no strict build-time dependency. It's just that I don't > > > > > > > > > see it nice to have a commit for a dead code, partly for testing > > > > > > > > > purpose and partly for code consistency. But if this makes > > > > > > > > > maintenance easier, I'm happy with that, too, of course. > > > > > > > > > > > > > > > > There won't be any dead code. All of the subsystem trees are pulled > > > > > > > > into -next [0] where the build bots can operate on the patches as a > > > > > > > > whole. > > > > > > > > > > > > > > But the merge order isn't guaranteed, i.e. at the commit of other tree > > > > > > > for this new stuff, it's a dead code without merging the MFD stuff > > > > > > > beforehand. e.g. Imagine to perform the git bisection. It's not > > > > > > > about the whole tree, but about the each commit. > > > > > > > > > > > > Only *building* is relevant for bisection until the whole feature > > > > > > lands. > > > > > > > > > > Why only building? > > > > > > > > > > When merging through several tress, commits for the same series are > > > > > scattered completely although they are softly tied. This sucks when > > > > > you perform git bisection, e.g. if you have an issue in the middle of > > > > > the patch series. It still works, but it jumps unnecessarily too far > > > > > away and back before reaching to the point, and kconfig appears / > > > > > disappears inconsistently (the dependent kconfig gone in the middle). > > > > > And, this is about the release kernel (4.15 or whatever). > > > > > > > > Think about how bisection works. You state a good commit and a bad > > > > one. The good commit will be when the feature last worked, which will > > > > not be until the feature has fully landed. Bisect will not check any > > > > point prior to this date. > > > > > > > > If there aren't any build deps, each Maintainer will apply patches > > > > into their own tree. These will be merged together in -next where > > > > they can be tested, both manually and by the 0-days. Once the merge > > > > window is opened all patches will be sucked into -rc1. If the feature > > > > works here, then it you could use -rc1 as your 'good' commit. If it > > > > doesn't, then this could indicate a merge error or a missing piece of > > > > the set, either way bisect wouldn't help you. > > > > > > Not really. > > > > > > First of all, most of user start testing from the release kernel, so > > > you can't trust that RC covered all test cases. (Who can blame users > > > who didn't use / test RC?) > > > > That's fine. We are not talking about spreading the merge of a > > patch-set over different releases, or even release candidates. All > > non-bugfix patches should be in by -rc1. > > > > > Second, you ignore the fact that the development continues after > > > merging *this* patchset. What if a breakage is introduced after this > > > patch? (See below) > > > > > > They often need a full bisection between the previous release and the > > > current release. > > > > You cannot bisect a specific function back before it was merged. It's > > impossible. > > > > P1 ---> P4 ---> P3 ---> P2 > > ^ > > Bisect only starts working here. Prior to this point the feature > > doesn't build at all. If it builds, but breaks, then that is a build > > dependency and is a different use-case to what we are discussing here. > > OK, let me rephrase. With the scenario above, user *cannot* perform > bisect. Meanwhile, with the straight merge, you can bisect a > breakage. That's a significant difference. > > I.e. in the case where both commits A1, B1 and B2 are merged through > tree A an B. B2 is the breakage. With separate tree merges, you > cannot bisect, while the straight merge allows you to bisect B2. Right, and that's really fundamental, because then the user can tell you "look, this commit doesn't work for me" instead of just "this kernel doesn't work for me" and now you need to spend *your* time on trying to figure out which commit may be at fault. So speaking of benefits, I really prefer to avoid spending my time on such things. :-) Thanks, Rafael
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-06 16:00 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umJ22-2yd-9@gated-at.bofh.it> |
| In reply to | #1727364 |
On Wed, 06 Sep 2017, Rafael J. Wysocki wrote:
> On Wednesday, September 6, 2017 12:58:55 PM CEST Takashi Iwai wrote:
> > On Wed, 06 Sep 2017 12:40:40 +0200,
> > Lee Jones wrote:
> > >
> > > On Wed, 06 Sep 2017, Takashi Iwai wrote:
> > >
> > > > On Wed, 06 Sep 2017 11:05:04 +0200,
> > > > Lee Jones wrote:
> > > > >
> > > > > On Wed, 06 Sep 2017, Takashi Iwai wrote:
> > > > >
> > > > > > On Wed, 06 Sep 2017 09:54:44 +0200,
> > > > > > Lee Jones wrote:
> > > > > > >
> > > > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote:
> > > > > > >
> > > > > > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com>
> > > > > > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > 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@linaro.org>
> > > > > > > > > > > >
> > > > > > > > > > > > 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.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or
> > > > > > > > > > > > create an immutable branch (again). But how to handle it, when to do
> > > > > > > > > > > > it, It's all up to you guys.
> > > > > > > > > > >
> > > > > > > > > > > If there aren't any build dependencies between the patches, each of
> > > > > > > > > > > the patches should be applied through their own trees. What are the
> > > > > > > > > > > build-time dependencies? Are there any?
> > > > > > > > > >
> > > > > > > > > > No, there is no strict build-time dependency. It's just that I don't
> > > > > > > > > > see it nice to have a commit for a dead code, partly for testing
> > > > > > > > > > purpose and partly for code consistency. But if this makes
> > > > > > > > > > maintenance easier, I'm happy with that, too, of course.
> > > > > > > > >
> > > > > > > > > There won't be any dead code. All of the subsystem trees are pulled
> > > > > > > > > into -next [0] where the build bots can operate on the patches as a
> > > > > > > > > whole.
> > > > > > > >
> > > > > > > > But the merge order isn't guaranteed, i.e. at the commit of other tree
> > > > > > > > for this new stuff, it's a dead code without merging the MFD stuff
> > > > > > > > beforehand. e.g. Imagine to perform the git bisection. It's not
> > > > > > > > about the whole tree, but about the each commit.
> > > > > > >
> > > > > > > Only *building* is relevant for bisection until the whole feature
> > > > > > > lands.
> > > > > >
> > > > > > Why only building?
> > > > > >
> > > > > > When merging through several tress, commits for the same series are
> > > > > > scattered completely although they are softly tied. This sucks when
> > > > > > you perform git bisection, e.g. if you have an issue in the middle of
> > > > > > the patch series. It still works, but it jumps unnecessarily too far
> > > > > > away and back before reaching to the point, and kconfig appears /
> > > > > > disappears inconsistently (the dependent kconfig gone in the middle).
> > > > > > And, this is about the release kernel (4.15 or whatever).
> > > > >
> > > > > Think about how bisection works. You state a good commit and a bad
> > > > > one. The good commit will be when the feature last worked, which will
> > > > > not be until the feature has fully landed. Bisect will not check any
> > > > > point prior to this date.
> > > > >
> > > > > If there aren't any build deps, each Maintainer will apply patches
> > > > > into their own tree. These will be merged together in -next where
> > > > > they can be tested, both manually and by the 0-days. Once the merge
> > > > > window is opened all patches will be sucked into -rc1. If the feature
> > > > > works here, then it you could use -rc1 as your 'good' commit. If it
> > > > > doesn't, then this could indicate a merge error or a missing piece of
> > > > > the set, either way bisect wouldn't help you.
> > > >
> > > > Not really.
> > > >
> > > > First of all, most of user start testing from the release kernel, so
> > > > you can't trust that RC covered all test cases. (Who can blame users
> > > > who didn't use / test RC?)
> > >
> > > That's fine. We are not talking about spreading the merge of a
> > > patch-set over different releases, or even release candidates. All
> > > non-bugfix patches should be in by -rc1.
> > >
> > > > Second, you ignore the fact that the development continues after
> > > > merging *this* patchset. What if a breakage is introduced after this
> > > > patch? (See below)
> > > >
> > > > They often need a full bisection between the previous release and the
> > > > current release.
> > >
> > > You cannot bisect a specific function back before it was merged. It's
> > > impossible.
> > >
> > > P1 ---> P4 ---> P3 ---> P2
> > > ^
> > > Bisect only starts working here. Prior to this point the feature
> > > doesn't build at all. If it builds, but breaks, then that is a build
> > > dependency and is a different use-case to what we are discussing here.
> >
> > OK, let me rephrase. With the scenario above, user *cannot* perform
> > bisect. Meanwhile, with the straight merge, you can bisect a
> > breakage. That's a significant difference.
> >
> > I.e. in the case where both commits A1, B1 and B2 are merged through
> > tree A an B. B2 is the breakage. With separate tree merges, you
> > cannot bisect, while the straight merge allows you to bisect B2.
>
> Right, and that's really fundamental, because then the user can tell you
> "look, this commit doesn't work for me" instead of just "this kernel
> doesn't work for me" and now you need to spend *your* time on trying to
> figure out which commit may be at fault.
>
> So speaking of benefits, I really prefer to avoid spending my time on
> such things. :-)
The toss-up is between splitting the patch-set up and *maybe* spending
time on debugging in the small chance of this occurring OR
*definitely* spending time creating immutable branches and sending out
pull-requests in the *hope* that all the other Maintainers involved
are diligent enough to merge it in order to avoid conflicts during
merge time.
In the circles I spend time in ("we"), the former is the favourite.
Until this point (and from this point going forward) we have taken the
decision that the default is to take individual patches through their
own trees. The only time this differs (unless other arrangements are
made e.g. PATCH 3/3) is when there are; build, merge or API
dependencies between them. The same stance is taken with
driver/platform data and driver/other driver.
Let me put one of the issues into context:
For those reading along that do not know, Multi-Functional Devices are
usually single pieces of silicon which provide many functions
(e.g. LED Controllers, Voltage Regulation, Power Management, Sensors,
Timers, GPIO/Pinctrl Controllers, Watchdog Timers, Real-Time Clocks,
etc etc). The driver which sits in drivers/mfd acts as the parent
device and registers its children which live in their own subsystems.
Almost all of the patch-sets I receive touch multiple subsystems.
Moreover, when the recently described dependencies occur, it is
usually I who creates the immutable branches and sends out the
pull-requests.
If I had to go through the immutable branch/pull-request process for
every patch-set I receive, there would be very little time to conduct
duties pertaining to my proper job. Ergo, why we apply our own
patches, unless there is a good reason (already described) to apply
others too.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [next] | [standalone]
| From | Takashi Iwai <tiwai@suse.de> |
|---|---|
| Date | 2017-09-06 16:40 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umJEK-32z-25@gated-at.bofh.it> |
| In reply to | #1727497 |
On Wed, 06 Sep 2017 15:51:13 +0200,
Lee Jones wrote:
>
> On Wed, 06 Sep 2017, Rafael J. Wysocki wrote:
>
> > On Wednesday, September 6, 2017 12:58:55 PM CEST Takashi Iwai wrote:
> > > On Wed, 06 Sep 2017 12:40:40 +0200,
> > > Lee Jones wrote:
> > > >
> > > > On Wed, 06 Sep 2017, Takashi Iwai wrote:
> > > >
> > > > > On Wed, 06 Sep 2017 11:05:04 +0200,
> > > > > Lee Jones wrote:
> > > > > >
> > > > > > On Wed, 06 Sep 2017, Takashi Iwai wrote:
> > > > > >
> > > > > > > On Wed, 06 Sep 2017 09:54:44 +0200,
> > > > > > > Lee Jones wrote:
> > > > > > > >
> > > > > > > > On Tue, 05 Sep 2017, Takashi Iwai wrote:
> > > > > > > >
> > > > > > > > > On Tue, 05 Sep 2017 10:53:41 +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@linux.intel.com>
> > > > > > > > > > > > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > > > > > > > > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > > 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@linaro.org>
> > > > > > > > > > > > >
> > > > > > > > > > > > > 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.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > > IMO, it'd be great if you can carry all stuff through MFD tree; or
> > > > > > > > > > > > > create an immutable branch (again). But how to handle it, when to do
> > > > > > > > > > > > > it, It's all up to you guys.
> > > > > > > > > > > >
> > > > > > > > > > > > If there aren't any build dependencies between the patches, each of
> > > > > > > > > > > > the patches should be applied through their own trees. What are the
> > > > > > > > > > > > build-time dependencies? Are there any?
> > > > > > > > > > >
> > > > > > > > > > > No, there is no strict build-time dependency. It's just that I don't
> > > > > > > > > > > see it nice to have a commit for a dead code, partly for testing
> > > > > > > > > > > purpose and partly for code consistency. But if this makes
> > > > > > > > > > > maintenance easier, I'm happy with that, too, of course.
> > > > > > > > > >
> > > > > > > > > > There won't be any dead code. All of the subsystem trees are pulled
> > > > > > > > > > into -next [0] where the build bots can operate on the patches as a
> > > > > > > > > > whole.
> > > > > > > > >
> > > > > > > > > But the merge order isn't guaranteed, i.e. at the commit of other tree
> > > > > > > > > for this new stuff, it's a dead code without merging the MFD stuff
> > > > > > > > > beforehand. e.g. Imagine to perform the git bisection. It's not
> > > > > > > > > about the whole tree, but about the each commit.
> > > > > > > >
> > > > > > > > Only *building* is relevant for bisection until the whole feature
> > > > > > > > lands.
> > > > > > >
> > > > > > > Why only building?
> > > > > > >
> > > > > > > When merging through several tress, commits for the same series are
> > > > > > > scattered completely although they are softly tied. This sucks when
> > > > > > > you perform git bisection, e.g. if you have an issue in the middle of
> > > > > > > the patch series. It still works, but it jumps unnecessarily too far
> > > > > > > away and back before reaching to the point, and kconfig appears /
> > > > > > > disappears inconsistently (the dependent kconfig gone in the middle).
> > > > > > > And, this is about the release kernel (4.15 or whatever).
> > > > > >
> > > > > > Think about how bisection works. You state a good commit and a bad
> > > > > > one. The good commit will be when the feature last worked, which will
> > > > > > not be until the feature has fully landed. Bisect will not check any
> > > > > > point prior to this date.
> > > > > >
> > > > > > If there aren't any build deps, each Maintainer will apply patches
> > > > > > into their own tree. These will be merged together in -next where
> > > > > > they can be tested, both manually and by the 0-days. Once the merge
> > > > > > window is opened all patches will be sucked into -rc1. If the feature
> > > > > > works here, then it you could use -rc1 as your 'good' commit. If it
> > > > > > doesn't, then this could indicate a merge error or a missing piece of
> > > > > > the set, either way bisect wouldn't help you.
> > > > >
> > > > > Not really.
> > > > >
> > > > > First of all, most of user start testing from the release kernel, so
> > > > > you can't trust that RC covered all test cases. (Who can blame users
> > > > > who didn't use / test RC?)
> > > >
> > > > That's fine. We are not talking about spreading the merge of a
> > > > patch-set over different releases, or even release candidates. All
> > > > non-bugfix patches should be in by -rc1.
> > > >
> > > > > Second, you ignore the fact that the development continues after
> > > > > merging *this* patchset. What if a breakage is introduced after this
> > > > > patch? (See below)
> > > > >
> > > > > They often need a full bisection between the previous release and the
> > > > > current release.
> > > >
> > > > You cannot bisect a specific function back before it was merged. It's
> > > > impossible.
> > > >
> > > > P1 ---> P4 ---> P3 ---> P2
> > > > ^
> > > > Bisect only starts working here. Prior to this point the feature
> > > > doesn't build at all. If it builds, but breaks, then that is a build
> > > > dependency and is a different use-case to what we are discussing here.
> > >
> > > OK, let me rephrase. With the scenario above, user *cannot* perform
> > > bisect. Meanwhile, with the straight merge, you can bisect a
> > > breakage. That's a significant difference.
> > >
> > > I.e. in the case where both commits A1, B1 and B2 are merged through
> > > tree A an B. B2 is the breakage. With separate tree merges, you
> > > cannot bisect, while the straight merge allows you to bisect B2.
> >
> > Right, and that's really fundamental, because then the user can tell you
> > "look, this commit doesn't work for me" instead of just "this kernel
> > doesn't work for me" and now you need to spend *your* time on trying to
> > figure out which commit may be at fault.
> >
> > So speaking of benefits, I really prefer to avoid spending my time on
> > such things. :-)
>
> The toss-up is between splitting the patch-set up and *maybe* spending
> time on debugging in the small chance of this occurring OR
> *definitely* spending time creating immutable branches and sending out
> pull-requests in the *hope* that all the other Maintainers involved
> are diligent enough to merge it in order to avoid conflicts during
> merge time.
>
> In the circles I spend time in ("we"), the former is the favourite.
This certainly depends on the complexity. For a small patchset,
merging in a shot is often a safer option. That is, when all parties
agree, you can just apply all patches to your branch -- that's all.
Other parties can simply pull this from your branch if needed. Of
course, the branch needs to be immutable for that, but usually it's no
big problem. (Ideally speaking, all the published branches should be
persistent in anyway.)
IIUC, it's the way Andy and Rafael suggested in the thread, and also
seen in many other subsystems occasionally.
> Until this point (and from this point going forward) we have taken the
> decision that the default is to take individual patches through their
> own trees. The only time this differs (unless other arrangements are
> made e.g. PATCH 3/3) is when there are; build, merge or API
> dependencies between them. The same stance is taken with
> driver/platform data and driver/other driver.
>
> Let me put one of the issues into context:
>
> For those reading along that do not know, Multi-Functional Devices are
> usually single pieces of silicon which provide many functions
> (e.g. LED Controllers, Voltage Regulation, Power Management, Sensors,
> Timers, GPIO/Pinctrl Controllers, Watchdog Timers, Real-Time Clocks,
> etc etc). The driver which sits in drivers/mfd acts as the parent
> device and registers its children which live in their own subsystems.
>
> Almost all of the patch-sets I receive touch multiple subsystems.
> Moreover, when the recently described dependencies occur, it is
> usually I who creates the immutable branches and sends out the
> pull-requests.
>
> If I had to go through the immutable branch/pull-request process for
> every patch-set I receive, there would be very little time to conduct
> duties pertaining to my proper job. Ergo, why we apply our own
> patches, unless there is a good reason (already described) to apply
> others too.
Hm, I never thought that creating an immutable branch were so
difficult. Isn't it just a simple branch-off either from the certain
upstream point (final release or rc) or from your stable branch?
thanks,
Takashi
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-06 17:00 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umJY7-3cf-21@gated-at.bofh.it> |
| In reply to | #1727554 |
> > > Right, and that's really fundamental, because then the user can tell you
> > > "look, this commit doesn't work for me" instead of just "this kernel
> > > doesn't work for me" and now you need to spend *your* time on trying to
> > > figure out which commit may be at fault.
> > >
> > > So speaking of benefits, I really prefer to avoid spending my time on
> > > such things. :-)
> >
> > The toss-up is between splitting the patch-set up and *maybe* spending
> > time on debugging in the small chance of this occurring OR
> > *definitely* spending time creating immutable branches and sending out
> > pull-requests in the *hope* that all the other Maintainers involved
> > are diligent enough to merge it in order to avoid conflicts during
> > merge time.
> >
> > In the circles I spend time in ("we"), the former is the favourite.
>
> This certainly depends on the complexity. For a small patchset,
> merging in a shot is often a safer option. That is, when all parties
> agree, you can just apply all patches to your branch -- that's all.
> Other parties can simply pull this from your branch if needed. Of
> course, the branch needs to be immutable for that, but usually it's no
> big problem. (Ideally speaking, all the published branches should be
> persistent in anyway.)
>
> IIUC, it's the way Andy and Rafael suggested in the thread, and also
> seen in many other subsystems occasionally.
>
> > Until this point (and from this point going forward) we have taken the
> > decision that the default is to take individual patches through their
> > own trees. The only time this differs (unless other arrangements are
> > made e.g. PATCH 3/3) is when there are; build, merge or API
> > dependencies between them. The same stance is taken with
> > driver/platform data and driver/other driver.
> >
> > Let me put one of the issues into context:
> >
> > For those reading along that do not know, Multi-Functional Devices are
> > usually single pieces of silicon which provide many functions
> > (e.g. LED Controllers, Voltage Regulation, Power Management, Sensors,
> > Timers, GPIO/Pinctrl Controllers, Watchdog Timers, Real-Time Clocks,
> > etc etc). The driver which sits in drivers/mfd acts as the parent
> > device and registers its children which live in their own subsystems.
> >
> > Almost all of the patch-sets I receive touch multiple subsystems.
> > Moreover, when the recently described dependencies occur, it is
> > usually I who creates the immutable branches and sends out the
> > pull-requests.
> >
> > If I had to go through the immutable branch/pull-request process for
> > every patch-set I receive, there would be very little time to conduct
> > duties pertaining to my proper job. Ergo, why we apply our own
> > patches, unless there is a good reason (already described) to apply
> > others too.
>
> Hm, I never thought that creating an immutable branch were so
> difficult. Isn't it just a simple branch-off either from the certain
> upstream point (final release or rc) or from your stable branch?
You'd be surprised.
When applying patches, I normally apply them to a mail folder for
further processing. This works great for patches that are applied to
my main branch, but this does not work for immutable branches. These
have to be applied on their own, thus need a 'special' or at least an
empty folder.
- Checkout a new branch based on the same (or earlier, but I always
use the same) commit as the main branch.
- 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.
- Tag and sign the branch with a suitable tag name and tag
description.
- Push branch and tag
- Create pull-request text
- Send a formatted email with the pull-request text.
This process is quite a lot more involved than simply applying
relevant patches to your main branch, and as I say, if this was
required for all patch-sets I am sent or involved in, it would really
eat into my day.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [next] | [standalone]
| From | Takashi Iwai <tiwai@suse.de> |
|---|---|
| Date | 2017-09-06 17:10 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <umK7N-3vz-61@gated-at.bofh.it> |
| In reply to | #1727568 |
On Wed, 06 Sep 2017 16:54:32 +0200,
Lee Jones wrote:
>
> > > > Right, and that's really fundamental, because then the user can tell you
> > > > "look, this commit doesn't work for me" instead of just "this kernel
> > > > doesn't work for me" and now you need to spend *your* time on trying to
> > > > figure out which commit may be at fault.
> > > >
> > > > So speaking of benefits, I really prefer to avoid spending my time on
> > > > such things. :-)
> > >
> > > The toss-up is between splitting the patch-set up and *maybe* spending
> > > time on debugging in the small chance of this occurring OR
> > > *definitely* spending time creating immutable branches and sending out
> > > pull-requests in the *hope* that all the other Maintainers involved
> > > are diligent enough to merge it in order to avoid conflicts during
> > > merge time.
> > >
> > > In the circles I spend time in ("we"), the former is the favourite.
> >
> > This certainly depends on the complexity. For a small patchset,
> > merging in a shot is often a safer option. That is, when all parties
> > agree, you can just apply all patches to your branch -- that's all.
> > Other parties can simply pull this from your branch if needed. Of
> > course, the branch needs to be immutable for that, but usually it's no
> > big problem. (Ideally speaking, all the published branches should be
> > persistent in anyway.)
> >
> > IIUC, it's the way Andy and Rafael suggested in the thread, and also
> > seen in many other subsystems occasionally.
> >
> > > Until this point (and from this point going forward) we have taken the
> > > decision that the default is to take individual patches through their
> > > own trees. The only time this differs (unless other arrangements are
> > > made e.g. PATCH 3/3) is when there are; build, merge or API
> > > dependencies between them. The same stance is taken with
> > > driver/platform data and driver/other driver.
> > >
> > > Let me put one of the issues into context:
> > >
> > > For those reading along that do not know, Multi-Functional Devices are
> > > usually single pieces of silicon which provide many functions
> > > (e.g. LED Controllers, Voltage Regulation, Power Management, Sensors,
> > > Timers, GPIO/Pinctrl Controllers, Watchdog Timers, Real-Time Clocks,
> > > etc etc). The driver which sits in drivers/mfd acts as the parent
> > > device and registers its children which live in their own subsystems.
> > >
> > > Almost all of the patch-sets I receive touch multiple subsystems.
> > > Moreover, when the recently described dependencies occur, it is
> > > usually I who creates the immutable branches and sends out the
> > > pull-requests.
> > >
> > > If I had to go through the immutable branch/pull-request process for
> > > every patch-set I receive, there would be very little time to conduct
> > > duties pertaining to my proper job. Ergo, why we apply our own
> > > patches, unless there is a good reason (already described) to apply
> > > others too.
> >
> > Hm, I never thought that creating an immutable branch were so
> > difficult. Isn't it just a simple branch-off either from the certain
> > upstream point (final release or rc) or from your stable branch?
>
> You'd be surprised.
>
> When applying patches, I normally apply them to a mail folder for
> further processing. This works great for patches that are applied to
> my main branch, but this does not work for immutable branches. These
> have to be applied on their own, thus need a 'special' or at least an
> empty folder.
Well, this isn't always requested -- at least, the simple case like
this one doesn't need to treat so much specially. We just need a
persistent branch that will be never rebased. It's the only
requirement.
That said, it's even enough just to branch off from your normal MFD
development branch, by a simple guarantee that you won't rebase *that*
branch any longer.
So, it's also a kind of "immutable branch", but it's much easier than
your regular workflow for the complex merge below.
Of course, a "cleaner" branch is preferred, but it doesn't matter much
as long as all stuff there will be merged to upstream sooner or
later.
thanks,
Takashi
>
> - Checkout a new branch based on the same (or earlier, but I always
> use the same) commit as the main branch.
>
> - 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.
>
> - Tag and sign the branch with a suitable tag name and tag
> description.
>
> - Push branch and tag
>
> - Create pull-request text
>
> - Send a formatted email with the pull-request text.
>
> This process is quite a lot more involved than simply applying
> relevant patches to your main branch, and as I say, if this was
> required for all patch-sets I am sent or involved in, it would really
> eat into my day.
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
>
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-07 10:10 +0200 |
| Subject | Re: [PATCH v5 1/3] mfd: Add support for Cherry Trail Dollar Cove TI PMIC |
| Message-ID | <un02S-5LU-5@gated-at.bofh.it> |
| In reply to | #1726127 |
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@linux.intel.com> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > 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 Applied for v4.15, thanks. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [standalone]
Page 3 of 3 — ← Prev page 1 2 [3]
Back to top | Article view | linux.kernel
csiph-web