linux-next: "amba: Remove AMBA level regulator support" commit.

linux-next: "amba: Remove AMBA level regulator support" commit.

[Paul Gortmaker]

Hi Mark,

The commit in linux-next:

-----------------
commit b2bc9e0ee349db84035d8141e85babc7986594d7
Author: Mark Brown <broonie@sirena.org.uk>
Date:   Mon Apr 2 12:30:35 2012 +0100

    ARM: 7366/1: amba: Remove AMBA level regulator support
-----------------

is pointed at by git bisect for the u8500_defconfig build failure:

drivers/dma/ste_dma40.c:2708:3: error: implicit declaration of function
'regulator_disable' [-Werror=implicit-function-declaration]
drivers/dma/ste_dma40.c:2747:3: error: implicit declaration of function
'regulator_enable' [-Werror=implicit-function-declaration]
drivers/dma/ste_dma40.c:3260:3: error: implicit declaration of function
'regulator_get' [-Werror=implicit-function-declaration]
drivers/dma/ste_dma40.c:3271:4: error: implicit declaration of function
'regulator_put' [-Werror=implicit-function-declaration]

http://kisskb.ellerman.id.au/kisskb/buildresult/6095691/

If, like my earlier mail this week, this is something you've already
fixed, then please provide me a link, and I'll gladly include that in
the triage report, so you don't get needless extra mails about it.

Thanks,
Paul.
--

git bisect start
# good: [0034102808e0dbbf3a2394b82b1bb40b5778de9e] Linux 3.4-rc2
git bisect good 0034102808e0dbbf3a2394b82b1bb40b5778de9e
# bad: [87487c4514a94892ce478dc012da6d12a98ae360] Add linux-next specific files for 20120412
git bisect bad 87487c4514a94892ce478dc012da6d12a98ae360
# bad: [45701d47f211d6ce72ee2c1725904d2e173eb29f] Merge remote-tracking branch 'sound/for-next'
git bisect bad 45701d47f211d6ce72ee2c1725904d2e173eb29f
# good: [5d2acf8cc7515129a5d3aed7b0e0be18dbcf524f] Merge branch 'topic/misc' into for-next
git bisect good 5d2acf8cc7515129a5d3aed7b0e0be18dbcf524f
# bad: [846c4b315b6faae8f5ede661d80c222d669adbda] Merge remote-tracking branch 'target-merge/for-next-merge'
git bisect bad 846c4b315b6faae8f5ede661d80c222d669adbda
# bad: [9235a323f99492120565935040b6be61e687d0c6] Merge remote-tracking branch 'gfs2/master'
git bisect bad 9235a323f99492120565935040b6be61e687d0c6
# bad: [6c944bb0c84fa1a03af503e926d7bd3a4a7e7376] Merge remote-tracking branch 'usb.current/usb-linus'
git bisect bad 6c944bb0c84fa1a03af503e926d7bd3a4a7e7376
# bad: [ef75674b6700ad96cac1e1c377cf9809cea6a369] Merge remote-tracking branch 'net/master'
git bisect bad ef75674b6700ad96cac1e1c377cf9809cea6a369
# good: [39f86a608a3e0f0164bd1540acf87696cfdfb5bb] Merge branch 'v4l_for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect good 39f86a608a3e0f0164bd1540acf87696cfdfb5bb
# good: [a1ada086062101533eb0f841d3884137688091ec] Merge tag 'sound-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good a1ada086062101533eb0f841d3884137688091ec
# good: [87151b8689d890dfb495081f7be9b9e257f7a2df] net: allow pskb_expand_head() to get maximum tailroom
git bisect good 87151b8689d890dfb495081f7be9b9e257f7a2df
# good: [f549e088b806f44b6ab6eeef0cb71ced1d2488dd] Merge tag 'rdma-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband
git bisect good f549e088b806f44b6ab6eeef0cb71ced1d2488dd
# bad: [b2bc9e0ee349db84035d8141e85babc7986594d7] ARM: 7366/1: amba: Remove AMBA level regulator support
git bisect bad b2bc9e0ee349db84035d8141e85babc7986594d7
# good: [34af657916332e89564566bc8d35e3e06cc0c236] ARM: 7377/1: vic: re-read status register before dispatching each IRQ handler
git bisect good 34af657916332e89564566bc8d35e3e06cc0c236

linux-next: "amba: Remove AMBA level regulator support" commit.

[Paul Gortmaker]

Hi Mark,

The commit in linux-next:

-----------------
commit b2bc9e0ee349db84035d8141e85babc7986594d7
Author: Mark Brown <broonie@sirena.org.uk>
Date:   Mon Apr 2 12:30:35 2012 +0100

    ARM: 7366/1: amba: Remove AMBA level regulator support
-----------------

is pointed at by git bisect for the u8500_defconfig build failure:

drivers/dma/ste_dma40.c:2708:3: error: implicit declaration of function
'regulator_disable' [-Werror=implicit-function-declaration]
drivers/dma/ste_dma40.c:2747:3: error: implicit declaration of function
'regulator_enable' [-Werror=implicit-function-declaration]
drivers/dma/ste_dma40.c:3260:3: error: implicit declaration of function
'regulator_get' [-Werror=implicit-function-declaration]
drivers/dma/ste_dma40.c:3271:4: error: implicit declaration of function
'regulator_put' [-Werror=implicit-function-declaration]

http://kisskb.ellerman.id.au/kisskb/buildresult/6095691/

If, like my earlier mail this week, this is something you've already
fixed, then please provide me a link, and I'll gladly include that in
the triage report, so you don't get needless extra mails about it.

Thanks,
Paul.
--

git bisect start
# good: [0034102808e0dbbf3a2394b82b1bb40b5778de9e] Linux 3.4-rc2
git bisect good 0034102808e0dbbf3a2394b82b1bb40b5778de9e
# bad: [87487c4514a94892ce478dc012da6d12a98ae360] Add linux-next specific files for 20120412
git bisect bad 87487c4514a94892ce478dc012da6d12a98ae360
# bad: [45701d47f211d6ce72ee2c1725904d2e173eb29f] Merge remote-tracking branch 'sound/for-next'
git bisect bad 45701d47f211d6ce72ee2c1725904d2e173eb29f
# good: [5d2acf8cc7515129a5d3aed7b0e0be18dbcf524f] Merge branch 'topic/misc' into for-next
git bisect good 5d2acf8cc7515129a5d3aed7b0e0be18dbcf524f
# bad: [846c4b315b6faae8f5ede661d80c222d669adbda] Merge remote-tracking branch 'target-merge/for-next-merge'
git bisect bad 846c4b315b6faae8f5ede661d80c222d669adbda
# bad: [9235a323f99492120565935040b6be61e687d0c6] Merge remote-tracking branch 'gfs2/master'
git bisect bad 9235a323f99492120565935040b6be61e687d0c6
# bad: [6c944bb0c84fa1a03af503e926d7bd3a4a7e7376] Merge remote-tracking branch 'usb.current/usb-linus'
git bisect bad 6c944bb0c84fa1a03af503e926d7bd3a4a7e7376
# bad: [ef75674b6700ad96cac1e1c377cf9809cea6a369] Merge remote-tracking branch 'net/master'
git bisect bad ef75674b6700ad96cac1e1c377cf9809cea6a369
# good: [39f86a608a3e0f0164bd1540acf87696cfdfb5bb] Merge branch 'v4l_for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect good 39f86a608a3e0f0164bd1540acf87696cfdfb5bb
# good: [a1ada086062101533eb0f841d3884137688091ec] Merge tag 'sound-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good a1ada086062101533eb0f841d3884137688091ec
# good: [87151b8689d890dfb495081f7be9b9e257f7a2df] net: allow pskb_expand_head() to get maximum tailroom
git bisect good 87151b8689d890dfb495081f7be9b9e257f7a2df
# good: [f549e088b806f44b6ab6eeef0cb71ced1d2488dd] Merge tag 'rdma-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband
git bisect good f549e088b806f44b6ab6eeef0cb71ced1d2488dd
# bad: [b2bc9e0ee349db84035d8141e85babc7986594d7] ARM: 7366/1: amba: Remove AMBA level regulator support
git bisect bad b2bc9e0ee349db84035d8141e85babc7986594d7
# good: [34af657916332e89564566bc8d35e3e06cc0c236] ARM: 7377/1: vic: re-read status register before dispatching each IRQ handler
git bisect good 34af657916332e89564566bc8d35e3e06cc0c236

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Shawn Guo]

On 13 April 2012 09:22, Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> Hi Mark,
>
> The commit in linux-next:
>
> -----------------
> commit b2bc9e0ee349db84035d8141e85babc7986594d7
> Author: Mark Brown <broonie@sirena.org.uk>
> Date:   Mon Apr 2 12:30:35 2012 +0100
>
>    ARM: 7366/1: amba: Remove AMBA level regulator support
> -----------------
>
> is pointed at by git bisect for the u8500_defconfig build failure:
>
> drivers/dma/ste_dma40.c:2708:3: error: implicit declaration of function
> 'regulator_disable' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:2747:3: error: implicit declaration of function
> 'regulator_enable' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:3260:3: error: implicit declaration of function
> 'regulator_get' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:3271:4: error: implicit declaration of function
> 'regulator_put' [-Werror=implicit-function-declaration]
>
That's because Mark's patch removes "#include
<linux/regulator/consumer.h>" included indirectly by
drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
really directly includes consumer.h.

Regards,
Shawn

linux-next: "amba: Remove AMBA level regulator support" commit.

[Shawn Guo]

On 13 April 2012 09:22, Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> Hi Mark,
>
> The commit in linux-next:
>
> -----------------
> commit b2bc9e0ee349db84035d8141e85babc7986594d7
> Author: Mark Brown <broonie@sirena.org.uk>
> Date: ? Mon Apr 2 12:30:35 2012 +0100
>
> ? ?ARM: 7366/1: amba: Remove AMBA level regulator support
> -----------------
>
> is pointed at by git bisect for the u8500_defconfig build failure:
>
> drivers/dma/ste_dma40.c:2708:3: error: implicit declaration of function
> 'regulator_disable' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:2747:3: error: implicit declaration of function
> 'regulator_enable' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:3260:3: error: implicit declaration of function
> 'regulator_get' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:3271:4: error: implicit declaration of function
> 'regulator_put' [-Werror=implicit-function-declaration]
>
That's because Mark's patch removes "#include
<linux/regulator/consumer.h>" included indirectly by
drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
really directly includes consumer.h.

Regards,
Shawn

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 568 bytes --]

On Thu, Apr 12, 2012 at 09:22:31PM -0400, Paul Gortmaker wrote:

> is pointed at by git bisect for the u8500_defconfig build failure:

> If, like my earlier mail this week, this is something you've already
> fixed, then please provide me a link, and I'll gladly include that in
> the triage report, so you don't get needless extra mails about it.

Not me, but Linus W has a patch.  No idea about a link off the top of my
head, google should turn something up I guess.  It's exactly the same
sort of implicit header dependency issue that your cleanups keep
triggering.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

On Thu, Apr 12, 2012 at 09:22:31PM -0400, Paul Gortmaker wrote:

> is pointed at by git bisect for the u8500_defconfig build failure:

> If, like my earlier mail this week, this is something you've already
> fixed, then please provide me a link, and I'll gladly include that in
> the triage report, so you don't get needless extra mails about it.

Not me, but Linus W has a patch.  No idea about a link off the top of my
head, google should turn something up I guess.  It's exactly the same
sort of implicit header dependency issue that your cleanups keep
triggering.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120413/9f2a6a67/attachment.sig>

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Russell King - ARM Linux]

On Fri, Apr 13, 2012 at 10:01:52AM +0800, Shawn Guo wrote:
> On 13 April 2012 09:22, Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> > Hi Mark,
> >
> > The commit in linux-next:
> >
> > -----------------
> > commit b2bc9e0ee349db84035d8141e85babc7986594d7
> > Author: Mark Brown <broonie@sirena.org.uk>
> > Date:   Mon Apr 2 12:30:35 2012 +0100
> >
> >    ARM: 7366/1: amba: Remove AMBA level regulator support
> > -----------------
> >
> > is pointed at by git bisect for the u8500_defconfig build failure:
> >
> > drivers/dma/ste_dma40.c:2708:3: error: implicit declaration of function
> > 'regulator_disable' [-Werror=implicit-function-declaration]
> > drivers/dma/ste_dma40.c:2747:3: error: implicit declaration of function
> > 'regulator_enable' [-Werror=implicit-function-declaration]
> > drivers/dma/ste_dma40.c:3260:3: error: implicit declaration of function
> > 'regulator_get' [-Werror=implicit-function-declaration]
> > drivers/dma/ste_dma40.c:3271:4: error: implicit declaration of function
> > 'regulator_put' [-Werror=implicit-function-declaration]
> >
> That's because Mark's patch removes "#include
> <linux/regulator/consumer.h>" included indirectly by
> drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
> really directly includes consumer.h.

Okay, I'll drop this patch because it's causing regressions, so it can't
be pushed as a 'fix' during -rc.

linux-next: "amba: Remove AMBA level regulator support" commit.

[Russell King - ARM Linux]

On Fri, Apr 13, 2012 at 10:01:52AM +0800, Shawn Guo wrote:
> On 13 April 2012 09:22, Paul Gortmaker <paul.gortmaker@windriver.com> wrote:
> > Hi Mark,
> >
> > The commit in linux-next:
> >
> > -----------------
> > commit b2bc9e0ee349db84035d8141e85babc7986594d7
> > Author: Mark Brown <broonie@sirena.org.uk>
> > Date: ? Mon Apr 2 12:30:35 2012 +0100
> >
> > ? ?ARM: 7366/1: amba: Remove AMBA level regulator support
> > -----------------
> >
> > is pointed at by git bisect for the u8500_defconfig build failure:
> >
> > drivers/dma/ste_dma40.c:2708:3: error: implicit declaration of function
> > 'regulator_disable' [-Werror=implicit-function-declaration]
> > drivers/dma/ste_dma40.c:2747:3: error: implicit declaration of function
> > 'regulator_enable' [-Werror=implicit-function-declaration]
> > drivers/dma/ste_dma40.c:3260:3: error: implicit declaration of function
> > 'regulator_get' [-Werror=implicit-function-declaration]
> > drivers/dma/ste_dma40.c:3271:4: error: implicit declaration of function
> > 'regulator_put' [-Werror=implicit-function-declaration]
> >
> That's because Mark's patch removes "#include
> <linux/regulator/consumer.h>" included indirectly by
> drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
> really directly includes consumer.h.

Okay, I'll drop this patch because it's causing regressions, so it can't
be pushed as a 'fix' during -rc.

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

On Fri, Apr 13, 2012 at 11:32:07AM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 13, 2012 at 10:01:52AM +0800, Shawn Guo wrote:

> > That's because Mark's patch removes "#include
> > <linux/regulator/consumer.h>" included indirectly by
> > drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
> > really directly includes consumer.h.

> Okay, I'll drop this patch because it's causing regressions, so it can't
> be pushed as a 'fix' during -rc.

I did say we should've been pushing Shawn's patch in as a minimal fix
for 3.4...  Alternatively if you're happy with the code just keeping the
header in place should avoid any issues, the one regression was just an
implicit header dependency.

Regardless of what happens for 3.4 we should keep the removal for -next,
it's clear that we don't want the bus doing this and it's causing
breakage for the non-ST platforms.  Do I need to resend the patch to the
patch system or will the existing copy be OK?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

On Fri, Apr 13, 2012 at 11:32:07AM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 13, 2012 at 10:01:52AM +0800, Shawn Guo wrote:

> > That's because Mark's patch removes "#include
> > <linux/regulator/consumer.h>" included indirectly by
> > drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
> > really directly includes consumer.h.

> Okay, I'll drop this patch because it's causing regressions, so it can't
> be pushed as a 'fix' during -rc.

I did say we should've been pushing Shawn's patch in as a minimal fix
for 3.4...  Alternatively if you're happy with the code just keeping the
header in place should avoid any issues, the one regression was just an
implicit header dependency.

Regardless of what happens for 3.4 we should keep the removal for -next,
it's clear that we don't want the bus doing this and it's causing
breakage for the non-ST platforms.  Do I need to resend the patch to the
patch system or will the existing copy be OK?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120413/34135552/attachment.sig>

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Russell King - ARM Linux]

On Fri, Apr 13, 2012 at 11:44:52AM +0100, Mark Brown wrote:
> On Fri, Apr 13, 2012 at 11:32:07AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Apr 13, 2012 at 10:01:52AM +0800, Shawn Guo wrote:
> 
> > > That's because Mark's patch removes "#include
> > > <linux/regulator/consumer.h>" included indirectly by
> > > drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
> > > really directly includes consumer.h.
> 
> > Okay, I'll drop this patch because it's causing regressions, so it can't
> > be pushed as a 'fix' during -rc.
> 
> I did say we should've been pushing Shawn's patch in as a minimal fix
> for 3.4...

What patch?

> Alternatively if you're happy with the code just keeping the
> header in place should avoid any issues, the one regression was just an
> implicit header dependency.
> 
> Regardless of what happens for 3.4 we should keep the removal for -next,
> it's clear that we don't want the bus doing this and it's causing
> breakage for the non-ST platforms.  Do I need to resend the patch to the
> patch system or will the existing copy be OK?

What I want is something that doesn't cause a regression for 3.4.

linux-next: "amba: Remove AMBA level regulator support" commit.

[Russell King - ARM Linux]

On Fri, Apr 13, 2012 at 11:44:52AM +0100, Mark Brown wrote:
> On Fri, Apr 13, 2012 at 11:32:07AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Apr 13, 2012 at 10:01:52AM +0800, Shawn Guo wrote:
> 
> > > That's because Mark's patch removes "#include
> > > <linux/regulator/consumer.h>" included indirectly by
> > > drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
> > > really directly includes consumer.h.
> 
> > Okay, I'll drop this patch because it's causing regressions, so it can't
> > be pushed as a 'fix' during -rc.
> 
> I did say we should've been pushing Shawn's patch in as a minimal fix
> for 3.4...

What patch?

> Alternatively if you're happy with the code just keeping the
> header in place should avoid any issues, the one regression was just an
> implicit header dependency.
> 
> Regardless of what happens for 3.4 we should keep the removal for -next,
> it's clear that we don't want the bus doing this and it's causing
> breakage for the non-ST platforms.  Do I need to resend the patch to the
> patch system or will the existing copy be OK?

What I want is something that doesn't cause a regression for 3.4.

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

On Fri, Apr 13, 2012 at 11:49:00AM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 13, 2012 at 11:44:52AM +0100, Mark Brown wrote:

> > I did say we should've been pushing Shawn's patch in as a minimal fix
> > for 3.4...

> What patch?

The original one which just changed the return code that's checked for.

> > Regardless of what happens for 3.4 we should keep the removal for -next,
> > it's clear that we don't want the bus doing this and it's causing
> > breakage for the non-ST platforms.  Do I need to resend the patch to the
> > patch system or will the existing copy be OK?

> What I want is something that doesn't cause a regression for 3.4.

Well, if the overall result is something other than removing the code in
3.5 then we'll just get further regressions down the line.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

On Fri, Apr 13, 2012 at 11:49:00AM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 13, 2012 at 11:44:52AM +0100, Mark Brown wrote:

> > I did say we should've been pushing Shawn's patch in as a minimal fix
> > for 3.4...

> What patch?

The original one which just changed the return code that's checked for.

> > Regardless of what happens for 3.4 we should keep the removal for -next,
> > it's clear that we don't want the bus doing this and it's causing
> > breakage for the non-ST platforms.  Do I need to resend the patch to the
> > patch system or will the existing copy be OK?

> What I want is something that doesn't cause a regression for 3.4.

Well, if the overall result is something other than removing the code in
3.5 then we'll just get further regressions down the line.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120413/f4fac225/attachment.sig>

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

On Fri, Apr 13, 2012 at 11:49:00AM +0100, Russell King - ARM Linux wrote:

> What I want is something that doesn't cause a regression for 3.4.

Oh, and note that there's already a fix for the missing header although
with all the git trees I've no idea what the process is for getting it
into mainline.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

On Fri, Apr 13, 2012 at 11:49:00AM +0100, Russell King - ARM Linux wrote:

> What I want is something that doesn't cause a regression for 3.4.

Oh, and note that there's already a fix for the missing header although
with all the git trees I've no idea what the process is for getting it
into mainline.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120413/c9690b88/attachment.sig>

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Fabio Estevam]

On Fri, Apr 13, 2012 at 8:25 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

>> > I did say we should've been pushing Shawn's patch in as a minimal fix
>> > for 3.4...
>
>> What patch?
>
> The original one which just changed the return code that's checked for.

I guess you are referring to this one:
http://www.spinics.net/lists/arm-kernel/msg167196.html

linux-next: "amba: Remove AMBA level regulator support" commit.

[Fabio Estevam]

On Fri, Apr 13, 2012 at 8:25 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

>> > I did say we should've been pushing Shawn's patch in as a minimal fix
>> > for 3.4...
>
>> What patch?
>
> The original one which just changed the return code that's checked for.

I guess you are referring to this one:
http://www.spinics.net/lists/arm-kernel/msg167196.html

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Russell King - ARM Linux]

On Fri, Apr 13, 2012 at 01:07:07PM +0100, Mark Brown wrote:
> On Fri, Apr 13, 2012 at 11:49:00AM +0100, Russell King - ARM Linux wrote:
> 
> > What I want is something that doesn't cause a regression for 3.4.
> 
> Oh, and note that there's already a fix for the missing header although
> with all the git trees I've no idea what the process is for getting it
> into mainline.

That really doesn't make any difference about whether I can push 7366/1
as is or not.  If the fix for the missing header is going through some
other git tree, I can't push 7366/1 until that fix has gone in - and
I'm not going to be tracking when that happens.

TBH, its something that _you_ need to manage - you created this regression
in the first place by changing the regulator API without first reviewing
all the callsites.  Grep is a wonderful tool for finding those.  So I'll
leave it entirely up to you to figure out how to fix the AMBA regression
you caused in a sane way in -rc - and without causing any additional
regressions by doing so.

If you want me to apply 7367/1 instead, then please say so directly.  If
you want 7366/1 plus the header file fixed, then that needs to be figured
out.

linux-next: "amba: Remove AMBA level regulator support" commit.

[Russell King - ARM Linux]

On Fri, Apr 13, 2012 at 01:07:07PM +0100, Mark Brown wrote:
> On Fri, Apr 13, 2012 at 11:49:00AM +0100, Russell King - ARM Linux wrote:
> 
> > What I want is something that doesn't cause a regression for 3.4.
> 
> Oh, and note that there's already a fix for the missing header although
> with all the git trees I've no idea what the process is for getting it
> into mainline.

That really doesn't make any difference about whether I can push 7366/1
as is or not.  If the fix for the missing header is going through some
other git tree, I can't push 7366/1 until that fix has gone in - and
I'm not going to be tracking when that happens.

TBH, its something that _you_ need to manage - you created this regression
in the first place by changing the regulator API without first reviewing
all the callsites.  Grep is a wonderful tool for finding those.  So I'll
leave it entirely up to you to figure out how to fix the AMBA regression
you caused in a sane way in -rc - and without causing any additional
regressions by doing so.

If you want me to apply 7367/1 instead, then please say so directly.  If
you want 7366/1 plus the header file fixed, then that needs to be figured
out.

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Linus Walleij]

On Fri, Apr 13, 2012 at 3:22 AM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:

> is pointed at by git bisect for the u8500_defconfig build failure:
>
> drivers/dma/ste_dma40.c:2708:3: error: implicit declaration of function
> 'regulator_disable' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:2747:3: error: implicit declaration of function
> 'regulator_enable' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:3260:3: error: implicit declaration of function
> 'regulator_get' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:3271:4: error: implicit declaration of function
> 'regulator_put' [-Werror=implicit-function-declaration]

I've already sent a patch to the DMA maintainer to fix this.
It was a bug in the DMA driver actually - it did not include the proper
headers - so it just got triggered by this patch.
http://marc.info/?l=linux-kernel&m=133424730706377&w=2

Yours,
Linus Walleij

linux-next: "amba: Remove AMBA level regulator support" commit.

[Linus Walleij]

On Fri, Apr 13, 2012 at 3:22 AM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:

> is pointed at by git bisect for the u8500_defconfig build failure:
>
> drivers/dma/ste_dma40.c:2708:3: error: implicit declaration of function
> 'regulator_disable' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:2747:3: error: implicit declaration of function
> 'regulator_enable' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:3260:3: error: implicit declaration of function
> 'regulator_get' [-Werror=implicit-function-declaration]
> drivers/dma/ste_dma40.c:3271:4: error: implicit declaration of function
> 'regulator_put' [-Werror=implicit-function-declaration]

I've already sent a patch to the DMA maintainer to fix this.
It was a bug in the DMA driver actually - it did not include the proper
headers - so it just got triggered by this patch.
http://marc.info/?l=linux-kernel&m=133424730706377&w=2

Yours,
Linus Walleij

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Linus Walleij]

On Fri, Apr 13, 2012 at 12:32 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Apr 13, 2012 at 10:01:52AM +0800, Shawn Guo wrote:
>> That's because Mark's patch removes "#include
>> <linux/regulator/consumer.h>" included indirectly by
>> drivers/dma/ste_dma40.c from linux/amba/bus.h.  The ste_dma40.c should
>> really directly includes consumer.h.
>
> Okay, I'll drop this patch because it's causing regressions, so it can't
> be pushed as a 'fix' during -rc.

Actually *not* having Marks patch also causes a regression on Ux500,
albeit not at compile time, but at runtime.

So for me as ux500 maintainer it's perfectly OK to have compilation
broken until that DMA patch arrives (should just be an issue of the
DMA maintainer picking it up), but if others feel differently
by all means hold it off until the other one arrives.

Yours,
Linus Walleij

linux-next: "amba: Remove AMBA level regulator support" commit.

[Linus Walleij]

On Fri, Apr 13, 2012 at 12:32 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Apr 13, 2012 at 10:01:52AM +0800, Shawn Guo wrote:
>> That's because Mark's patch removes "#include
>> <linux/regulator/consumer.h>" included indirectly by
>> drivers/dma/ste_dma40.c from linux/amba/bus.h. ?The ste_dma40.c should
>> really directly includes consumer.h.
>
> Okay, I'll drop this patch because it's causing regressions, so it can't
> be pushed as a 'fix' during -rc.

Actually *not* having Marks patch also causes a regression on Ux500,
albeit not at compile time, but at runtime.

So for me as ux500 maintainer it's perfectly OK to have compilation
broken until that DMA patch arrives (should just be an issue of the
DMA maintainer picking it up), but if others feel differently
by all means hold it off until the other one arrives.

Yours,
Linus Walleij

Re: linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 3209 bytes --]

On Fri, Apr 13, 2012 at 01:19:14PM +0100, Russell King - ARM Linux wrote:

> That really doesn't make any difference about whether I can push 7366/1
> as is or not.  If the fix for the missing header is going through some
> other git tree, I can't push 7366/1 until that fix has gone in - and
> I'm not going to be tracking when that happens.

I've got quite limited visibility on how this stuff gets handled; the
patch tracking system always feels like a bit of a black hole to me as
I'm never sure when to send things to it or when they might get applied
and the usual mechanisms for indicating how things get applied (tags in
the header and comments after the ---) aren't available.

I have to say was rather surprised when I realised you had applied my
change for -rc rather than for -next.

> TBH, its something that _you_ need to manage - you created this regression
> in the first place by changing the regulator API without first reviewing
> all the callsites.  Grep is a wonderful tool for finding those.  So I'll

You know as well as I do that a grep of all the users isn't going to
turn up everything reliably, this is one of the reasons why people
should be testing -next (which apparently nobody had been doing on any
of the affected platforms).

Looking at the code I suspect that my initial read was that if anything
hit that case we'd crash as vcore is left with an error pointer, though
unusually for such code the IS_ERR() is actually used at every call
site so it actually managed to work.  Plus the fact that the code was
never supposed to work in the first place, of course.

> leave it entirely up to you to figure out how to fix the AMBA regression
> you caused in a sane way in -rc - and without causing any additional
> regressions by doing so.

> If you want me to apply 7367/1 instead, then please say so directly.  If
> you want 7366/1 plus the header file fixed, then that needs to be figured
> out.

Right from my initial reply to it I've said that 7367/1 was a good,
minimal fix for 3.4 but that for 3.5 we should be doing 7366/1 or
thereabouts.  Like I say I was quite surprised when you applied the
larger fix for 3.4, though there are good arguments in favour of doing
that in that it removes an API which nobody should be using.  So long as
what we end up with in 3.5 is 7366/x (which I think everyone agrees is
the goal) I'm not too fussed about which solution is adopted for 3.4.

There's two ways to go about this:

 - Apply 7366/3 (which keeps regulator/consumer.h in linux/amba.h but is
   otherwise the same as 7366/1) and then either leave things or clean
   up the header in 3.5.

 - Apply 7367/1 for 3.4 and then rebase 7366/x after it and put that in
   -next for inclusion in 3.5.

Which one is chosen is a matter of taste for you.  I'd initially
expected the second approach but as it seems you're comfortable with the
larger patch we may as well go with it, the header file should be the
only dependency.

For the benefit of people reading the mails 7366/x is the change to
remove the regulator usage from AMBA (amba: Remove AMBA level regulator
support) and 7367/x is the change to just change the return code (amba:
adapt to regulator probe deferral change).

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

linux-next: "amba: Remove AMBA level regulator support" commit.

[Mark Brown]

On Fri, Apr 13, 2012 at 01:19:14PM +0100, Russell King - ARM Linux wrote:

> That really doesn't make any difference about whether I can push 7366/1
> as is or not.  If the fix for the missing header is going through some
> other git tree, I can't push 7366/1 until that fix has gone in - and
> I'm not going to be tracking when that happens.

I've got quite limited visibility on how this stuff gets handled; the
patch tracking system always feels like a bit of a black hole to me as
I'm never sure when to send things to it or when they might get applied
and the usual mechanisms for indicating how things get applied (tags in
the header and comments after the ---) aren't available.

I have to say was rather surprised when I realised you had applied my
change for -rc rather than for -next.

> TBH, its something that _you_ need to manage - you created this regression
> in the first place by changing the regulator API without first reviewing
> all the callsites.  Grep is a wonderful tool for finding those.  So I'll

You know as well as I do that a grep of all the users isn't going to
turn up everything reliably, this is one of the reasons why people
should be testing -next (which apparently nobody had been doing on any
of the affected platforms).

Looking at the code I suspect that my initial read was that if anything
hit that case we'd crash as vcore is left with an error pointer, though
unusually for such code the IS_ERR() is actually used at every call
site so it actually managed to work.  Plus the fact that the code was
never supposed to work in the first place, of course.

> leave it entirely up to you to figure out how to fix the AMBA regression
> you caused in a sane way in -rc - and without causing any additional
> regressions by doing so.

> If you want me to apply 7367/1 instead, then please say so directly.  If
> you want 7366/1 plus the header file fixed, then that needs to be figured
> out.

Right from my initial reply to it I've said that 7367/1 was a good,
minimal fix for 3.4 but that for 3.5 we should be doing 7366/1 or
thereabouts.  Like I say I was quite surprised when you applied the
larger fix for 3.4, though there are good arguments in favour of doing
that in that it removes an API which nobody should be using.  So long as
what we end up with in 3.5 is 7366/x (which I think everyone agrees is
the goal) I'm not too fussed about which solution is adopted for 3.4.

There's two ways to go about this:

 - Apply 7366/3 (which keeps regulator/consumer.h in linux/amba.h but is
   otherwise the same as 7366/1) and then either leave things or clean
   up the header in 3.5.

 - Apply 7367/1 for 3.4 and then rebase 7366/x after it and put that in
   -next for inclusion in 3.5.

Which one is chosen is a matter of taste for you.  I'd initially
expected the second approach but as it seems you're comfortable with the
larger patch we may as well go with it, the header file should be the
only dependency.

For the benefit of people reading the mails 7366/x is the change to
remove the regulator usage from AMBA (amba: Remove AMBA level regulator
support) and 7367/x is the change to just change the return code (amba:
adapt to regulator probe deferral change).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120413/ad988d8c/attachment-0001.sig>