| Message ID | 20130515090449.GA3494@gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi Lee, On Wed, May 15, 2013 at 10:04:49AM +0100, Lee Jones wrote: > Hi Sam, > > Since v1 of my MFD -fixes pull-request, I have applied a few more > patches which really should be entered into the -rc:s. As it appears > that you have not yet pulled v1, I figured sending a v2 in its place > would be appropriate, so here it is. This almost looks good, I just have one comment: > Fabio Baltieri (5): > mfd: abx500-core: Fix sparse warning > mfd: ab8500-sysctrl: Fix sparse warning > mfd: ab8500-sysctrl: Set sysctrl_dev during probe > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata Unless I'm missing something here, this one is not really rcN material, so I'd appreciate if you could queue it to your for-mfd branch instead. The rest is pulled and applied, thanks again. Cheers, Samuel.
Hello Samuel, On Fri, May 17, 2013 at 12:43:37AM +0200, Samuel Ortiz wrote: > > Fabio Baltieri (5): > > mfd: abx500-core: Fix sparse warning > > mfd: ab8500-sysctrl: Fix sparse warning > > mfd: ab8500-sysctrl: Set sysctrl_dev during probe > > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata > Unless I'm missing something here, this one is not really rcN material, so > I'd appreciate if you could queue it to your for-mfd branch instead. The last two: mfd: ab8500-sysctrl: Set sysctrl_dev during probe mfd: ab8500-sysctrl: Let sysctrl driver work without pdata are actually fixes for bugs introduced in this merge window that inhibited the ab8500-sysctl driver as a consequence. Would you reconsider pulling just those? Thanks, Fabio
On Fri, May 17, 2013 at 9:51 AM, Fabio Baltieri <fabio.baltieri@linaro.org> wrote: > Hello Samuel, (...) > The last two: > > mfd: ab8500-sysctrl: Set sysctrl_dev during probe > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata > > are actually fixes for bugs introduced in this merge window that > inhibited the ab8500-sysctl driver as a consequence. Would you > reconsider pulling just those? A good way to make sure that patches fixing regressions are applied to the -rc series is to indicate in the commit message or even the heading that it fixes a regression. I usually try to begin such commits with the text: this patch fixes a regression caused by change 018745435 "foo: fix bar"... Then copying in some crash dump text never hurts :-) It makes it very easy for us as subsystem maintainers to pick regression fixes. Yours, Linus Walleij
On Fri, May 17, 2013 at 10:25:10AM +0200, Linus Walleij wrote: > On Fri, May 17, 2013 at 9:51 AM, Fabio Baltieri > <fabio.baltieri@linaro.org> wrote: > > Hello Samuel, > (...) > > The last two: > > > > mfd: ab8500-sysctrl: Set sysctrl_dev during probe > > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata > > > > are actually fixes for bugs introduced in this merge window that > > inhibited the ab8500-sysctl driver as a consequence. Would you > > reconsider pulling just those? > > A good way to make sure that patches fixing regressions are applied > to the -rc series is to indicate in the commit message or even the > heading that it fixes a regression. > > I usually try to begin such commits with the text: > > this patch fixes a regression caused by change 018745435 > "foo: fix bar"... > > Then copying in some crash dump text never hurts :-) > > It makes it very easy for us as subsystem maintainers to pick > regression fixes. Right, I actually pointed to the culprit commit (applied during the merge window) in the message, but I'll try to be more direct next time. I also agree on the crash dump, but unfortunately this one was failing silently. :-) Thanks for the tip! Fabio
Hi Fabio, On Fri, May 17, 2013 at 09:51:40AM +0200, Fabio Baltieri wrote: > Hello Samuel, > > On Fri, May 17, 2013 at 12:43:37AM +0200, Samuel Ortiz wrote: > > > Fabio Baltieri (5): > > > mfd: abx500-core: Fix sparse warning > > > mfd: ab8500-sysctrl: Fix sparse warning > > > mfd: ab8500-sysctrl: Set sysctrl_dev during probe > > > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata > > Unless I'm missing something here, this one is not really rcN material, so > > I'd appreciate if you could queue it to your for-mfd branch instead. > > The last two: > > mfd: ab8500-sysctrl: Set sysctrl_dev during probe This one is already in mfd-fixes, the commit log was clear enough. > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata This one is not, as the commit log is not showing it actually fixes something but only makes the code cleaner. If it really fixes something, could you please provide me with a commit log that describes what exactly it fixes ? Cheers, Samuel.
Hi Fabio, On Tue, May 21, 2013 at 11:12:30AM +0200, Fabio Baltieri wrote: > On Tue, May 21, 2013 at 10:56:50AM +0200, Samuel Ortiz wrote: > > On Fri, May 17, 2013 at 09:51:40AM +0200, Fabio Baltieri wrote: > > > Hello Samuel, > > > > > > On Fri, May 17, 2013 at 12:43:37AM +0200, Samuel Ortiz wrote: > > > > > Fabio Baltieri (5): > > > > > mfd: abx500-core: Fix sparse warning > > > > > mfd: ab8500-sysctrl: Fix sparse warning > > > > > mfd: ab8500-sysctrl: Set sysctrl_dev during probe > > > > > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata > > > > Unless I'm missing something here, this one is not really rcN material, so > > > > I'd appreciate if you could queue it to your for-mfd branch instead. > > > > > > The last two: > > > > > > mfd: ab8500-sysctrl: Set sysctrl_dev during probe > > This one is already in mfd-fixes, the commit log was clear enough. > > Great! > > > > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata > > This one is not, as the commit log is not showing it actually fixes something > > but only makes the code cleaner. If it really fixes something, could you > > please provide me with a commit log that describes what exactly it fixes ? > > Ok, the fix here is that right now that driver is initialized without a > specific pdata (that is plat->sysctrl), so enforcing it breaks existing > platforms for no reason. To make the commit more clear I would just > point that out as in (just last two sentences): > > --- >8 --- > mfd: ab8500-sysctrl: Let sysctrl driver work without pdata > > A check for a valid plat->sysctrl was introduced in: > > 2377e52 mfd: ab8500-sysctrl: Error check clean up > > but the driver works just fine even without that initialization data, > and enforcing it breaks existing platforms for no reason. > > This patch removes the check and let the driver go ahead with probe. > --- >8 --- > > Is it better? Slightly. Next time please describe how it breaks. I applied and pushed this patch now, I'll send a pull request to Linus tomorrow morning. Cheers, Samuel.
Hi Sam, Since v1 of my MFD -fixes pull-request, I have applied a few more patches which really should be entered into the -rc:s. As it appears that you have not yet pulled v1, I figured sending a v2 in its place would be appropriate, so here it is. ----- The following changes since commit f722406faae2d073cc1d01063d1123c35425939e: Linux 3.10-rc1 (2013-05-11 17:14:08 -0700) are available in the git repository at: git://git.linaro.org/people/ljones/mfd.git for-mfd-fixes for you to fetch changes up to 27df65e061f51f6bd2c8e9ad6b505c8dbab6460c: mfd: db8500-prcmu: Update stored DSI PLL divider value (2013-05-14 15:58:04 +0100) ---------------------------------------------------------------- Arnd Bergmann (1): mfd: ab8500: Debugfs code depends on gpadc Fabio Baltieri (5): mfd: abx500-core: Fix sparse warning mfd: ab8500-sysctrl: Fix sparse warning mfd: ab8500-sysctrl: Set sysctrl_dev during probe mfd: ab8500-sysctrl: Let sysctrl driver work without pdata mfd: ab8500-sysctrl: Always enable pm_power_off handler Lee Jones (4): mfd: ab8500-gpadc: Suppress 'ignoring regulator_enable() return value' warning mfd: ab8500-core: Use the correct driver name when enabling gpio/pinctrl mfd: db8500-prcmu: Supply the pdata_size attribute for db8500-thermal mfd: ab8500-core: Pass GPADC compatible string to MFD core Linus Walleij (1): mfd: ab8500: Pass AB8500 IRQ to debugfs code by resource Ulf Hansson (1): mfd: db8500-prcmu: Update stored DSI PLL divider value drivers/mfd/Kconfig | 2 +- drivers/mfd/ab8500-core.c | 24 +++++++++++++++++++++--- drivers/mfd/ab8500-debugfs.c | 16 ++++++++++++---- drivers/mfd/ab8500-gpadc.c | 7 +++++-- drivers/mfd/ab8500-sysctrl.c | 15 ++++++++++----- drivers/mfd/abx500-core.c | 2 +- drivers/mfd/db8500-prcmu.c | 3 +++ include/linux/mfd/abx500/ab8500.h | 2 -- 8 files changed, 53 insertions(+), 18 deletions(-)