pinctrl-msm build error on Linus' tree

pinctrl-msm build error on Linus' tree

[Josh Boyer]

We're hitting a build error on ARM with Linus' tree as of Linux
v3.17-2860-gef0625b70dac :

drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_setup_pm_reset':
drivers/pinctrl/qcom/pinctrl-msm.c:875:4: error: implicit declaration
of function 'register_restart_handler'
[-Werror=implicit-function-declaration]
    if (register_restart_handler(&pctrl->restart_nb))
    ^
drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_remove':
drivers/pinctrl/qcom/pinctrl-msm.c:949:2: error: implicit declaration
of function 'unregister_restart_handler'
[-Werror=implicit-function-declaration]
  unregister_restart_handler(&pctrl->restart_nb);
  ^
cc1: some warnings being treated as errors

Looking at the git logs it seems this was added via:

commit cf1fc187628913070c3e418ce0e205732435aa2f
Author: Josh Cartwright <joshc@codeaurora.org>
Date:   Tue Sep 23 15:59:53 2014 -0500

    pinctrl: qcom: use restart_notifier mechanism for ps_hold


However, there is literally nothing else in the tree that calls or
provides those functions:

[jwboyer@vader linux]$ git grep unregister_restart_handler
drivers/pinctrl/qcom/pinctrl-msm.c:     unregister_restart_handler(&pctrl->resta
[jwboyer@vader linux]$


I'm rather confused.  How was this commit built and tested?

josh

Full build log here:
http://koji.fedoraproject.org/koji/getfile?taskID=7803166&name=build.log

Re: pinctrl-msm build error on Linus' tree

[Josh Cartwright]

Hey Josh-

On Wed, Oct 08, 2014 at 06:42:55PM -0400, Josh Boyer wrote:
> We're hitting a build error on ARM with Linus' tree as of Linux
> v3.17-2860-gef0625b70dac :
> 
> drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_setup_pm_reset':
> drivers/pinctrl/qcom/pinctrl-msm.c:875:4: error: implicit declaration
> of function 'register_restart_handler'
> [-Werror=implicit-function-declaration]
>     if (register_restart_handler(&pctrl->restart_nb))
>     ^
> drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_remove':
> drivers/pinctrl/qcom/pinctrl-msm.c:949:2: error: implicit declaration
> of function 'unregister_restart_handler'
> [-Werror=implicit-function-declaration]
>   unregister_restart_handler(&pctrl->restart_nb);
>   ^
> cc1: some warnings being treated as errors
> 
> Looking at the git logs it seems this was added via:
> 
> commit cf1fc187628913070c3e418ce0e205732435aa2f
> Author: Josh Cartwright <joshc@codeaurora.org>
> Date:   Tue Sep 23 15:59:53 2014 -0500
> 
>     pinctrl: qcom: use restart_notifier mechanism for ps_hold
> 
> 
> However, there is literally nothing else in the tree that calls or
> provides those functions:
> 
> [jwboyer@vader linux]$ git grep unregister_restart_handler
> drivers/pinctrl/qcom/pinctrl-msm.c:     unregister_restart_handler(&pctrl->resta
> [jwboyer@vader linux]$
> 
> 
> I'm rather confused.  How was this commit built and tested?

This may be my fault for not clearly stating that this particular patch
was dependent on Guenter's restart notifier patchset.  This patch was
applied to linusw's tree without these patches.   Perhaps it should be
dropped for now until the restart_notifier stuff lands?

   Josh

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: pinctrl-msm build error on Linus' tree

[Josh Boyer]

On Wed, Oct 8, 2014 at 6:49 PM, Josh Cartwright <joshc@codeaurora.org> wrote:
> Hey Josh-
>
> On Wed, Oct 08, 2014 at 06:42:55PM -0400, Josh Boyer wrote:
>> We're hitting a build error on ARM with Linus' tree as of Linux
>> v3.17-2860-gef0625b70dac :
>>
>> drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_setup_pm_reset':
>> drivers/pinctrl/qcom/pinctrl-msm.c:875:4: error: implicit declaration
>> of function 'register_restart_handler'
>> [-Werror=implicit-function-declaration]
>>     if (register_restart_handler(&pctrl->restart_nb))
>>     ^
>> drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_remove':
>> drivers/pinctrl/qcom/pinctrl-msm.c:949:2: error: implicit declaration
>> of function 'unregister_restart_handler'
>> [-Werror=implicit-function-declaration]
>>   unregister_restart_handler(&pctrl->restart_nb);
>>   ^
>> cc1: some warnings being treated as errors
>>
>> Looking at the git logs it seems this was added via:
>>
>> commit cf1fc187628913070c3e418ce0e205732435aa2f
>> Author: Josh Cartwright <joshc@codeaurora.org>
>> Date:   Tue Sep 23 15:59:53 2014 -0500
>>
>>     pinctrl: qcom: use restart_notifier mechanism for ps_hold
>>
>>
>> However, there is literally nothing else in the tree that calls or
>> provides those functions:
>>
>> [jwboyer@vader linux]$ git grep unregister_restart_handler
>> drivers/pinctrl/qcom/pinctrl-msm.c:     unregister_restart_handler(&pctrl->resta
>> [jwboyer@vader linux]$
>>
>>
>> I'm rather confused.  How was this commit built and tested?
>
> This may be my fault for not clearly stating that this particular patch
> was dependent on Guenter's restart notifier patchset.  This patch was
> applied to linusw's tree without these patches.   Perhaps it should be
> dropped for now until the restart_notifier stuff lands?

Thanks, that explains things.  I'm not sure if Linus would do a revert
at this point.  I guess it depends on how fast he's going to merge
Guenter's patches.

Is there a reason this wasn't all brought in via one tree?  Seems like
it should have been.

josh

Re: pinctrl-msm build error on Linus' tree

[Guenter Roeck]

On Wed, Oct 08, 2014 at 06:42:55PM -0400, Josh Boyer wrote:
> We're hitting a build error on ARM with Linus' tree as of Linux
> v3.17-2860-gef0625b70dac :
> 
> drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_setup_pm_reset':
> drivers/pinctrl/qcom/pinctrl-msm.c:875:4: error: implicit declaration
> of function 'register_restart_handler'
> [-Werror=implicit-function-declaration]
>     if (register_restart_handler(&pctrl->restart_nb))
>     ^
> drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_remove':
> drivers/pinctrl/qcom/pinctrl-msm.c:949:2: error: implicit declaration
> of function 'unregister_restart_handler'
> [-Werror=implicit-function-declaration]
>   unregister_restart_handler(&pctrl->restart_nb);
>   ^
> cc1: some warnings being treated as errors
> 
> Looking at the git logs it seems this was added via:
> 
> commit cf1fc187628913070c3e418ce0e205732435aa2f
> Author: Josh Cartwright <joshc@codeaurora.org>
> Date:   Tue Sep 23 15:59:53 2014 -0500
> 
>     pinctrl: qcom: use restart_notifier mechanism for ps_hold
> 
> 
> However, there is literally nothing else in the tree that calls or
> provides those functions:
> 
> [jwboyer@vader linux]$ git grep unregister_restart_handler
> drivers/pinctrl/qcom/pinctrl-msm.c:     unregister_restart_handler(&pctrl->resta
> [jwboyer@vader linux]$
> 
> 
> I'm rather confused.  How was this commit built and tested?
> 

Looks like the pinctrl tree did not include the merge with the immutable
branch with the necessary infrastructure in its pull request to Linus :-(.
As for how it was tested in the pinctrl tree, no idea. Maybe pinctrl-msm has
some dependency which was missing in the pinctrl tree and came in through
a different pull request.

For this to build, you'll need to merge

git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git restart-handler-for-v3.18

which was supposed to be merged by all trees using it, to avoid exactly
this problem (clk, power/restart, and watchdog are the other trees I am
aware of).

Best solution might be for Linus to pull the above tag directly, after
screaming at me appropriately. Sorry, I thought we had this well covered.

Guenter

Re: pinctrl-msm build error on Linus' tree

[Guenter Roeck]

On Wed, Oct 08, 2014 at 07:09:18PM -0400, Josh Boyer wrote:
> On Wed, Oct 8, 2014 at 6:49 PM, Josh Cartwright <joshc@codeaurora.org> wrote:
> > Hey Josh-
> >
> > On Wed, Oct 08, 2014 at 06:42:55PM -0400, Josh Boyer wrote:
> >> We're hitting a build error on ARM with Linus' tree as of Linux
> >> v3.17-2860-gef0625b70dac :
> >>
> >> drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_setup_pm_reset':
> >> drivers/pinctrl/qcom/pinctrl-msm.c:875:4: error: implicit declaration
> >> of function 'register_restart_handler'
> >> [-Werror=implicit-function-declaration]
> >>     if (register_restart_handler(&pctrl->restart_nb))
> >>     ^
> >> drivers/pinctrl/qcom/pinctrl-msm.c: In function 'msm_pinctrl_remove':
> >> drivers/pinctrl/qcom/pinctrl-msm.c:949:2: error: implicit declaration
> >> of function 'unregister_restart_handler'
> >> [-Werror=implicit-function-declaration]
> >>   unregister_restart_handler(&pctrl->restart_nb);
> >>   ^
> >> cc1: some warnings being treated as errors
> >>
> >> Looking at the git logs it seems this was added via:
> >>
> >> commit cf1fc187628913070c3e418ce0e205732435aa2f
> >> Author: Josh Cartwright <joshc@codeaurora.org>
> >> Date:   Tue Sep 23 15:59:53 2014 -0500
> >>
> >>     pinctrl: qcom: use restart_notifier mechanism for ps_hold
> >>
> >>
> >> However, there is literally nothing else in the tree that calls or
> >> provides those functions:
> >>
> >> [jwboyer@vader linux]$ git grep unregister_restart_handler
> >> drivers/pinctrl/qcom/pinctrl-msm.c:     unregister_restart_handler(&pctrl->resta
> >> [jwboyer@vader linux]$
> >>
> >>
> >> I'm rather confused.  How was this commit built and tested?
> >
> > This may be my fault for not clearly stating that this particular patch
> > was dependent on Guenter's restart notifier patchset.  This patch was
> > applied to linusw's tree without these patches.   Perhaps it should be
> > dropped for now until the restart_notifier stuff lands?
> 
> Thanks, that explains things.  I'm not sure if Linus would do a revert
> at this point.  I guess it depends on how fast he's going to merge
> Guenter's patches.
> 
> Is there a reason this wasn't all brought in via one tree?  Seems like
> it should have been.
> 
Too many dependencies all over the place, and thus risk to create merge-time
conflicts. Besides pinctrl, the restart handler infrastructure is already
used by clock, power/restart, and watchdog drivers.

The idea was for me to provide a pointer to an immutable branch
with the infrastructure (which I did), and for all trees using it
to merge with that branch. This way the infrastructure would have been merged
automatically with its first user, and no separate pull request would have
been necessary. Again, I thought we had this well covered. Apparently not,
and of course the one tree who didn't merge the infrastructure got into
mainline first :-(.

Guenter

Re: pinctrl-msm build error on Linus' tree

[Linus Walleij]

On Thu, Oct 9, 2014 at 1:12 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> On Wed, Oct 08, 2014 at 06:42:55PM -0400, Josh Boyer wrote:

>> However, there is literally nothing else in the tree that calls or
>> provides those functions:
>>
>> [jwboyer@vader linux]$ git grep unregister_restart_handler
>> drivers/pinctrl/qcom/pinctrl-msm.c:     unregister_restart_handler(&pctrl->resta
>> [jwboyer@vader linux]$
>>
>>
>> I'm rather confused.  How was this commit built and tested?

I guess the dependent tree was being pulled into linux-next before
the pinctrl tree, and so the end result was working?

> Looks like the pinctrl tree did not include the merge with the immutable
> branch with the necessary infrastructure in its pull request to Linus :-(.

Yeah maybe I missed some pull request for that, such things happen.

> As for how it was tested in the pinctrl tree, no idea. Maybe pinctrl-msm has
> some dependency which was missing in the pinctrl tree and came in through
> a different pull request.

See above.

Anyway, it is a minor platform, not x86_64. So don't exaggerate
this thing, and besides the restart notifiers are great.

If we go down the route of trying to always avoid all trouble in the
world by adding more procedure I guess we shouldn't shoehorn
too much into the same merge window and should have this
postponed to v3.19. But I don't know if it buys us so much.

Yours,
Linus Walleij

Re: pinctrl-msm build error on Linus' tree

[Guenter Roeck]

On Thu, Oct 09, 2014 at 09:34:03AM +0200, Linus Walleij wrote:
> On Thu, Oct 9, 2014 at 1:12 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> > On Wed, Oct 08, 2014 at 06:42:55PM -0400, Josh Boyer wrote:
> 
> >> However, there is literally nothing else in the tree that calls or
> >> provides those functions:
> >>
> >> [jwboyer@vader linux]$ git grep unregister_restart_handler
> >> drivers/pinctrl/qcom/pinctrl-msm.c:     unregister_restart_handler(&pctrl->resta
> >> [jwboyer@vader linux]$
> >>
> >>
> >> I'm rather confused.  How was this commit built and tested?
> 
> I guess the dependent tree was being pulled into linux-next before
> the pinctrl tree, and so the end result was working?
> 
> > Looks like the pinctrl tree did not include the merge with the immutable
> > branch with the necessary infrastructure in its pull request to Linus :-(.
> 
> Yeah maybe I missed some pull request for that, such things happen.
> 
> > As for how it was tested in the pinctrl tree, no idea. Maybe pinctrl-msm has
> > some dependency which was missing in the pinctrl tree and came in through
> > a different pull request.
> 
> See above.
> 
> Anyway, it is a minor platform, not x86_64. So don't exaggerate
> this thing, and besides the restart notifiers are great.
> 
> If we go down the route of trying to always avoid all trouble in the
> world by adding more procedure I guess we shouldn't shoehorn

Not a matter of procedure, but it would have been easy to avoid.
I take at least part of the blame here because I did not follow up
with you to make sure that you merge the infrastructure into your tree.

> too much into the same merge window and should have this
> postponed to v3.19. But I don't know if it buys us so much.
> 
That was the original idea, but the restart handler was way more successful
that I thought it would or could be, so things got pulled in a bit.

Anyway, I tend to agree - there are now several other build failures in
Linus' tree which it inherited from linux-next. I consider those worse,
especially since the majority if not all of those problems were known
from -next but ignored. Wonder what the value of -next is if people
don't care if it builds or not :-(.

But then Linus ignored my pull request (so far), so maybe I did manage to
upset him ;-).

Guenter

Re: pinctrl-msm build error on Linus' tree

[Linus Walleij]

On Thu, Oct 9, 2014 at 6:07 PM, Guenter Roeck <linux@roeck-us.net> wrote:

> But then Linus ignored my pull request (so far), so maybe I did manage to
> upset him ;-).

Did you send it as a separate mail with [GIT PULL] in the Subject:?

Else that is likely the problem. He gets a lot of noise.

Yours,
Linus Walleij

Re: pinctrl-msm build error on Linus' tree

[Guenter Roeck]

On 10/10/2014 01:42 AM, Linus Walleij wrote:
> On Thu, Oct 9, 2014 at 6:07 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>
>> But then Linus ignored my pull request (so far), so maybe I did manage to
>> upset him ;-).
>
> Did you send it as a separate mail with [GIT PULL] in the Subject:?
>

Yes, I did. Two, actually, one for the reset handler and one for the hwmon subsystem.

Guenter

Re: pinctrl-msm build error on Linus' tree

[Josh Boyer]

On Fri, Oct 10, 2014 at 12:02 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 10/10/2014 01:42 AM, Linus Walleij wrote:
>>
>> On Thu, Oct 9, 2014 at 6:07 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>>> But then Linus ignored my pull request (so far), so maybe I did manage to
>>> upset him ;-).
>>
>>
>> Did you send it as a separate mail with [GIT PULL] in the Subject:?
>>
>
> Yes, I did. Two, actually, one for the reset handler and one for the hwmon
> subsystem.

As I understand things, he's travelling.  I guess be patient?

josh

Re: pinctrl-msm build error on Linus' tree

[Guenter Roeck]

On 10/10/2014 09:05 AM, Josh Boyer wrote:
> On Fri, Oct 10, 2014 at 12:02 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>> On 10/10/2014 01:42 AM, Linus Walleij wrote:
>>>
>>> On Thu, Oct 9, 2014 at 6:07 PM, Guenter Roeck <linux@roeck-us.net> wrote:
>>>
>>>> But then Linus ignored my pull request (so far), so maybe I did manage to
>>>> upset him ;-).
>>>
>>>
>>> Did you send it as a separate mail with [GIT PULL] in the Subject:?
>>>
>>
>> Yes, I did. Two, actually, one for the reset handler and one for the hwmon
>> subsystem.
>
> As I understand things, he's travelling.  I guess be patient?
>

Yes, I know. I'll wait until next week before I resend, if necessary.

It might as well be that the e-mail I sent it from is or was on a spam list -
I copied too many people on my poweroff handler patch series, which got
me banned at numerous places.

Guenter