Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Randy Dunlap]

On 06/26/16 23:39, Stephen Rothwell wrote:
> Hi all,
> 
> Changes since 20160624:
> 

on i386, when CONFIG_OF is not enabled ...
but OF_GPIO is enabled due to this in drivers/gpio/Kconfig:

config OF_GPIO
	def_bool y
	depends on OF || COMPILE_TEST

(above from commit 1e4a80640338924b9f9fd7a121ac31d08134410a
from Alexander Stein <alexanders83@web.de>)


../drivers/pinctrl/bcm/pinctrl-iproc-gpio.c:381:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
../drivers/pinctrl/bcm/pinctrl-cygnus-mux.c:739:20: error: 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a function)
../drivers/pinctrl/bcm/pinctrl-nsp-gpio.c:365:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)

because that function is only present when CONFIG_OF is enabled.


Also, why does that commit (1e4a80640338924b9f9fd7a121ac31d08134410a)
not have any other S-O-B lines in it?  like whoever merged it?


-- 
~Randy

Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Arnd Bergmann]

On Monday, June 27, 2016 11:15:25 AM CEST Randy Dunlap wrote:
> On 06/26/16 23:39, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Changes since 20160624:
> > 
> 
> on i386, when CONFIG_OF is not enabled ...
> but OF_GPIO is enabled due to this in drivers/gpio/Kconfig:
> 
> config OF_GPIO
>         def_bool y
>         depends on OF || COMPILE_TEST
> 
> (above from commit 1e4a80640338924b9f9fd7a121ac31d08134410a
> from Alexander Stein <alexanders83@web.de>)
> 
> 
> ../drivers/pinctrl/bcm/pinctrl-iproc-gpio.c:381:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
> ../drivers/pinctrl/bcm/pinctrl-cygnus-mux.c:739:20: error: 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a function)
> ../drivers/pinctrl/bcm/pinctrl-nsp-gpio.c:365:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
> 
> because that function is only present when CONFIG_OF is enabled.
> 
> 
> Also, why does that commit (1e4a80640338924b9f9fd7a121ac31d08134410a)
> not have any other S-O-B lines in it?  like whoever merged it?

(adding Linus Walleij and Alexander Stein to Cc)

Unfortunately, the changelog also doesn't say what triggered that
change. I guess there was a driver that did 'select OF_GPIO'
and had a dependency on COMPILE_TEST, though I don't see any
driver doing that in linux-next.

I think the commit should just be reverted, it clearly breaks
stuff, and whatever needs it can be fixed in a better way.

Specifically, we have three ways that the OF_GPIO symbol 
is used:

a) it is implicitly enabled when both GPIOLIB and OF are turned on
b) some drivers 'depends on OF_GPIO'
c) some other drivers 'select OF_GPIO'

The combination of b) and c) can easily lead to circular dependencies,
and is also confusing. It would be better to change all of those
to 'depends on'.

	Arnd

Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Alexander Stein]

On Monday 27 June 2016 23:36:29, Arnd Bergmann wrote:
> On Monday, June 27, 2016 11:15:25 AM CEST Randy Dunlap wrote:
> > On 06/26/16 23:39, Stephen Rothwell wrote:
> > > Hi all,
> > 
> > > Changes since 20160624:
> > on i386, when CONFIG_OF is not enabled ...
> > but OF_GPIO is enabled due to this in drivers/gpio/Kconfig:
> > 
> > config OF_GPIO
> > 
> >         def_bool y
> >         depends on OF || COMPILE_TEST
> > 
> > (above from commit 1e4a80640338924b9f9fd7a121ac31d08134410a
> > from Alexander Stein <alexanders83@web.de>)
> > 
> > 
> > ../drivers/pinctrl/bcm/pinctrl-iproc-gpio.c:381:20: error:
> > 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
> > ../drivers/pinctrl/bcm/pinctrl-cygnus-mux.c:739:20: error:
> > 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a
> > function) ../drivers/pinctrl/bcm/pinctrl-nsp-gpio.c:365:20: error:
> > 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
> > 
> > because that function is only present when CONFIG_OF is enabled.
> > 
> > 
> > Also, why does that commit (1e4a80640338924b9f9fd7a121ac31d08134410a)
> > not have any other S-O-B lines in it?  like whoever merged it?
> 
> (adding Linus Walleij and Alexander Stein to Cc)
> 
> Unfortunately, the changelog also doesn't say what triggered that
> change. I guess there was a driver that did 'select OF_GPIO'
> and had a dependency on COMPILE_TEST, though I don't see any
> driver doing that in linux-next.

It was the first change of the changes done in my branch 
https://github.com/AlexanderStein/linux/tree/gpio-compile-test
 in order to compile test as much gpio drivers as possible. Yet, pinctrl 
drivers were out of my scope.

> I think the commit should just be reverted, it clearly breaks
> stuff, and whatever needs it can be fixed in a better way.

Why not depend PINCTRL_IPROC_GPIO on CONFIG_OF as Randy stated 
pinconf_generic_dt_node_to_map_pin and friends are only present if CONFIG_OF 
is enabled. Depending on OF_GPIO only is enough under the assumption a) you 
made below.
Also what about providing dummy implementation for those missing function in 
case CONFIG_OF is disabled, my preference as more code can be compile tested.

> Specifically, we have three ways that the OF_GPIO symbol
> is used:
> 
> a) it is implicitly enabled when both GPIOLIB and OF are turned on

or if COMPILE_TEST is enabled

I think a problem in this and possibly other cases is that COMPILE_TEST 
loosens the Kconfig dependencies to some degree. How to proceed? Allow compile 
testing drivers only if some optional settings are enabled?

Best regards,
Alexander

Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Randy Dunlap]

On 06/28/16 00:00, Alexander Stein wrote:
> On Monday 27 June 2016 23:36:29, Arnd Bergmann wrote:
>> On Monday, June 27, 2016 11:15:25 AM CEST Randy Dunlap wrote:
>>> On 06/26/16 23:39, Stephen Rothwell wrote:
>>>> Hi all,
>>>
>>>> Changes since 20160624:
>>> on i386, when CONFIG_OF is not enabled ...
>>> but OF_GPIO is enabled due to this in drivers/gpio/Kconfig:
>>>
>>> config OF_GPIO
>>>
>>>         def_bool y
>>>         depends on OF || COMPILE_TEST
>>>
>>> (above from commit 1e4a80640338924b9f9fd7a121ac31d08134410a
>>> from Alexander Stein <alexanders83@web.de>)
>>>
>>>
>>> ../drivers/pinctrl/bcm/pinctrl-iproc-gpio.c:381:20: error:
>>> 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
>>> ../drivers/pinctrl/bcm/pinctrl-cygnus-mux.c:739:20: error:
>>> 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a
>>> function) ../drivers/pinctrl/bcm/pinctrl-nsp-gpio.c:365:20: error:
>>> 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
>>>
>>> because that function is only present when CONFIG_OF is enabled.
>>>
>>>
>>> Also, why does that commit (1e4a80640338924b9f9fd7a121ac31d08134410a)
>>> not have any other S-O-B lines in it?  like whoever merged it?
>>
>> (adding Linus Walleij and Alexander Stein to Cc)
>>
>> Unfortunately, the changelog also doesn't say what triggered that
>> change. I guess there was a driver that did 'select OF_GPIO'
>> and had a dependency on COMPILE_TEST, though I don't see any
>> driver doing that in linux-next.
> 
> It was the first change of the changes done in my branch 
> https://github.com/AlexanderStein/linux/tree/gpio-compile-test
>  in order to compile test as much gpio drivers as possible. Yet, pinctrl 
> drivers were out of my scope.
> 
>> I think the commit should just be reverted, it clearly breaks
>> stuff, and whatever needs it can be fixed in a better way.
> 
> Why not depend PINCTRL_IPROC_GPIO on CONFIG_OF as Randy stated 
> pinconf_generic_dt_node_to_map_pin and friends are only present if CONFIG_OF 
> is enabled. Depending on OF_GPIO only is enough under the assumption a) you 
> made below.
> Also what about providing dummy implementation for those missing function in 
> case CONFIG_OF is disabled, my preference as more code can be compile tested.

IMO we shouldn't have to make function stubs just for COMPILE_TEST.


>> Specifically, we have three ways that the OF_GPIO symbol
>> is used:
>>
>> a) it is implicitly enabled when both GPIOLIB and OF are turned on
> 
> or if COMPILE_TEST is enabled
> 
> I think a problem in this and possibly other cases is that COMPILE_TEST 
> loosens the Kconfig dependencies to some degree. How to proceed? Allow compile 
> testing drivers only if some optional settings are enabled?


-- 
~Randy

Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Arnd Bergmann]

On Tuesday, June 28, 2016 9:00:47 AM CEST Alexander Stein wrote:
> On Monday 27 June 2016 23:36:29, Arnd Bergmann wrote:
> > On Monday, June 27, 2016 11:15:25 AM CEST Randy Dunlap wrote:
> > > On 06/26/16 23:39, Stephen Rothwell wrote:
> > > > Hi all,
> > > 
> > > > Changes since 20160624:
> > > on i386, when CONFIG_OF is not enabled ...
> > > but OF_GPIO is enabled due to this in drivers/gpio/Kconfig:
> > > 
> > > config OF_GPIO
> > > 
> > >         def_bool y
> > >         depends on OF || COMPILE_TEST
> > > 
> > > (above from commit 1e4a80640338924b9f9fd7a121ac31d08134410a
> > > from Alexander Stein <alexanders83@web.de>)
> > > 
> > > 
> > > ../drivers/pinctrl/bcm/pinctrl-iproc-gpio.c:381:20: error:
> > > 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
> > > ../drivers/pinctrl/bcm/pinctrl-cygnus-mux.c:739:20: error:
> > > 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a
> > > function) ../drivers/pinctrl/bcm/pinctrl-nsp-gpio.c:365:20: error:
> > > 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
> > > 
> > > because that function is only present when CONFIG_OF is enabled.
> > > 
> > > 
> > > Also, why does that commit (1e4a80640338924b9f9fd7a121ac31d08134410a)
> > > not have any other S-O-B lines in it?  like whoever merged it?
> > 
> > (adding Linus Walleij and Alexander Stein to Cc)
> > 
> > Unfortunately, the changelog also doesn't say what triggered that
> > change. I guess there was a driver that did 'select OF_GPIO'
> > and had a dependency on COMPILE_TEST, though I don't see any
> > driver doing that in linux-next.
> 
> It was the first change of the changes done in my branch 
> https://github.com/AlexanderStein/linux/tree/gpio-compile-test
>  in order to compile test as much gpio drivers as possible. Yet, pinctrl 
> drivers were out of my scope.

I see

> > I think the commit should just be reverted, it clearly breaks
> > stuff, and whatever needs it can be fixed in a better way.
> 
> Why not depend PINCTRL_IPROC_GPIO on CONFIG_OF as Randy stated 
> pinconf_generic_dt_node_to_map_pin and friends are only present if CONFIG_OF 
> is enabled. Depending on OF_GPIO only is enough under the assumption a) you 
> made below.
> Also what about providing dummy implementation for those missing function in 
> case CONFIG_OF is disabled, my preference as more code can be compile tested.
>
> > Specifically, we have three ways that the OF_GPIO symbol
> > is used:
> > 
> > a) it is implicitly enabled when both GPIOLIB and OF are turned on
> 
> or if COMPILE_TEST is enabled
> 
> I think a problem in this and possibly other cases is that COMPILE_TEST 
> loosens the Kconfig dependencies to some degree. How to proceed? Allow compile 
> testing drivers only if some optional settings are enabled?

I think we can start by reverting your patch: for compile-testing we can
always enable CONFIG_OF these days, which will also turn on OF_GPIO.

Compile-testing drivers that need OF_GPIO while OF is disabled makes
no sense to me, it's not a configuration that anybody can ever use
and it adds no build coverage.

I think we should also drop all 'select OF_GPIO' statements, AFAICT
we have just proven that they are all redundant because they would
have otherwise broken things. This should be a separate cleanup patch.

	Arnd

Re: Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Alexander Stein]

On Tuesday 28 June 2016, 10:37:52 wrote Arnd Bergmann:
> > > I think the commit should just be reverted, it clearly breaks
> > > stuff, and whatever needs it can be fixed in a better way.
> > 
> > Why not depend PINCTRL_IPROC_GPIO on CONFIG_OF as Randy stated 
> > pinconf_generic_dt_node_to_map_pin and friends are only present if 
CONFIG_OF 
> > is enabled. Depending on OF_GPIO only is enough under the assumption a) 
you 
> > made below.
> > Also what about providing dummy implementation for those missing function 
in 
> > case CONFIG_OF is disabled, my preference as more code can be compile 
tested.
> >
> > > Specifically, we have three ways that the OF_GPIO symbol
> > > is used:
> > > 
> > > a) it is implicitly enabled when both GPIOLIB and OF are turned on
> > 
> > or if COMPILE_TEST is enabled
> > 
> > I think a problem in this and possibly other cases is that COMPILE_TEST 
> > loosens the Kconfig dependencies to some degree. How to proceed? Allow 
compile 
> > testing drivers only if some optional settings are enabled?
> 
> I think we can start by reverting your patch: for compile-testing we can
> always enable CONFIG_OF these days, which will also turn on OF_GPIO.

My intention was to compile test (device tree) drivers also without CONFIG_OF 
which is disabled by defconfig default on arch/x86/configs/*

> Compile-testing drivers that need OF_GPIO while OF is disabled makes
> no sense to me, it's not a configuration that anybody can ever use
> and it adds no build coverage.

That seems sensible. If we can agree that anyone who wants to compile test 
drivers has to enable CONFIG_OF even it is not needed for the current 
configuration.

Best regards,
Alexander

Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Linus Walleij]

On Mon, Jun 27, 2016 at 8:15 PM, Randy Dunlap <rdunlap@infradead.org> wrote:

> On 06/26/16 23:39, Stephen Rothwell wrote:
>> Hi all,
>>
>> Changes since 20160624:
>>
>
> on i386, when CONFIG_OF is not enabled ...
> but OF_GPIO is enabled due to this in drivers/gpio/Kconfig:
>
> config OF_GPIO
>         def_bool y
>         depends on OF || COMPILE_TEST
>
> (above from commit 1e4a80640338924b9f9fd7a121ac31d08134410a
> from Alexander Stein <alexanders83@web.de>)
>
>
> ../drivers/pinctrl/bcm/pinctrl-iproc-gpio.c:381:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
> ../drivers/pinctrl/bcm/pinctrl-cygnus-mux.c:739:20: error: 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a function)
> ../drivers/pinctrl/bcm/pinctrl-nsp-gpio.c:365:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
>
> because that function is only present when CONFIG_OF is enabled.
>
>
> Also, why does that commit (1e4a80640338924b9f9fd7a121ac31d08134410a)
> not have any other S-O-B lines in it?  like whoever merged it?

I merged it I think, Alex made a long series enabling compile
testing and I started to cherry-pick the first commits to let
them trickle in.

I was worried about it because some of the patches caused
severe build problems on some archs.

It's a  bit tricky to know what to do here: we want compile
coverage to get proper testing, when we turn it on we get regressions,
so trying to improve things make things break and it becomes a
vicious circle of trouble. I don't know what the biggest pain is ...

I don't really see the conclusion of this discussion thread, whether
I should revert the patch or not? For fixes or next?

Yours,
Linus Walleij

Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Randy Dunlap]

On 07/04/16 02:46, Linus Walleij wrote:
> On Mon, Jun 27, 2016 at 8:15 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> 
>> On 06/26/16 23:39, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> Changes since 20160624:
>>>
>>
>> on i386, when CONFIG_OF is not enabled ...
>> but OF_GPIO is enabled due to this in drivers/gpio/Kconfig:
>>
>> config OF_GPIO
>>         def_bool y
>>         depends on OF || COMPILE_TEST
>>
>> (above from commit 1e4a80640338924b9f9fd7a121ac31d08134410a
>> from Alexander Stein <alexanders83@web.de>)
>>
>>
>> ../drivers/pinctrl/bcm/pinctrl-iproc-gpio.c:381:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
>> ../drivers/pinctrl/bcm/pinctrl-cygnus-mux.c:739:20: error: 'pinconf_generic_dt_node_to_map_group' undeclared here (not in a function)
>> ../drivers/pinctrl/bcm/pinctrl-nsp-gpio.c:365:20: error: 'pinconf_generic_dt_node_to_map_pin' undeclared here (not in a function)
>>
>> because that function is only present when CONFIG_OF is enabled.
>>
>>
>> Also, why does that commit (1e4a80640338924b9f9fd7a121ac31d08134410a)
>> not have any other S-O-B lines in it?  like whoever merged it?
> 
> I merged it I think, Alex made a long series enabling compile
> testing and I started to cherry-pick the first commits to let
> them trickle in.

I guess that when you do a git pull of a series of patches, you
sign the pull commit but not each patch in the series?
That could explain it.

> I was worried about it because some of the patches caused
> severe build problems on some archs.
> 
> It's a  bit tricky to know what to do here: we want compile
> coverage to get proper testing, when we turn it on we get regressions,
> so trying to improve things make things break and it becomes a
> vicious circle of trouble. I don't know what the biggest pain is ...
> 
> I don't really see the conclusion of this discussion thread, whether
> I should revert the patch or not? For fixes or next?

I agree with Arnd that this particular commit should be reverted
(1e4a80640338924b9f9fd7a121ac31d08134410a).


-- 
~Randy

Re: linux-next: Tree for Jun 27 (pinctrl && !CONFIG_OF)

[Linus Walleij]

On Mon, Jul 4, 2016 at 6:12 PM, Randy Dunlap <rdunlap@infradead.org> wrote:

>> I merged it I think, Alex made a long series enabling compile
>> testing and I started to cherry-pick the first commits to let
>> them trickle in.
>
> I guess that when you do a git pull of a series of patches, you
> sign the pull commit but not each patch in the series?
> That could explain it.

I think I cherry-picked it actually. But yeah.

> I agree with Arnd that this particular commit should be reverted
> (1e4a80640338924b9f9fd7a121ac31d08134410a).

I've reverted it for fixes.

Yours,
Linus Walleij