All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-09-21  9:01 ` sean.wang
  0 siblings, 0 replies; 12+ messages in thread
From: sean.wang @ 2017-09-21  9:01 UTC (permalink / raw)
  To: matthias.bgg, linux-mediatek, jdelvare, arnd
  Cc: linux-arm-kernel, linux-kernel, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

MTK_PMIC_WRAP is the basic and required configuration for those various
MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
allows users tending to have the enablement for those PMICs.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/soc/mediatek/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index a2fcd7f..d513629 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -15,7 +15,7 @@ config MTK_INFRACFG
 config MTK_PMIC_WRAP
 	tristate "MediaTek PMIC Wrapper Support"
 	depends on ARCH_MEDIATEK
-	depends on RESET_CONTROLLER
+	select RESET_CONTROLLER
 	select REGMAP
 	help
 	  Say yes here to add support for MediaTek PMIC Wrapper found
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-09-21  9:01 ` sean.wang
  0 siblings, 0 replies; 12+ messages in thread
From: sean.wang @ 2017-09-21  9:01 UTC (permalink / raw)
  To: matthias.bgg, linux-mediatek, jdelvare, arnd
  Cc: linux-arm-kernel, linux-kernel, Sean Wang

From: Sean Wang <sean.wang@mediatek.com>

MTK_PMIC_WRAP is the basic and required configuration for those various
MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
allows users tending to have the enablement for those PMICs.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/soc/mediatek/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index a2fcd7f..d513629 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -15,7 +15,7 @@ config MTK_INFRACFG
 config MTK_PMIC_WRAP
 	tristate "MediaTek PMIC Wrapper Support"
 	depends on ARCH_MEDIATEK
-	depends on RESET_CONTROLLER
+	select RESET_CONTROLLER
 	select REGMAP
 	help
 	  Say yes here to add support for MediaTek PMIC Wrapper found
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-09-21  9:01 ` sean.wang
  0 siblings, 0 replies; 12+ messages in thread
From: sean.wang at mediatek.com @ 2017-09-21  9:01 UTC (permalink / raw)
  To: linux-arm-kernel

From: Sean Wang <sean.wang@mediatek.com>

MTK_PMIC_WRAP is the basic and required configuration for those various
MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
allows users tending to have the enablement for those PMICs.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/soc/mediatek/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index a2fcd7f..d513629 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -15,7 +15,7 @@ config MTK_INFRACFG
 config MTK_PMIC_WRAP
 	tristate "MediaTek PMIC Wrapper Support"
 	depends on ARCH_MEDIATEK
-	depends on RESET_CONTROLLER
+	select RESET_CONTROLLER
 	select REGMAP
 	help
 	  Say yes here to add support for MediaTek PMIC Wrapper found
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
  2017-09-21  9:01 ` sean.wang
  (?)
@ 2017-09-26  6:00   ` Jean Delvare
  -1 siblings, 0 replies; 12+ messages in thread
From: Jean Delvare @ 2017-09-26  6:00 UTC (permalink / raw)
  To: sean.wang
  Cc: matthias.bgg, linux-mediatek, arnd, linux-arm-kernel, linux-kernel

On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> MTK_PMIC_WRAP is the basic and required configuration for those various
> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
> allows users tending to have the enablement for those PMICs.

I can't really make sense of the sentence above, sorry, and can't see
how it matches the change below anyway. MTK_PMIC_WRAP is already a
visible symbol before this change. The change is probably good in
itself, but please try to come up with a better description.

> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  drivers/soc/mediatek/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index a2fcd7f..d513629 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -15,7 +15,7 @@ config MTK_INFRACFG
>  config MTK_PMIC_WRAP
>  	tristate "MediaTek PMIC Wrapper Support"
>  	depends on ARCH_MEDIATEK
> -	depends on RESET_CONTROLLER
> +	select RESET_CONTROLLER
>  	select REGMAP
>  	help
>  	  Say yes here to add support for MediaTek PMIC Wrapper found


-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-09-26  6:00   ` Jean Delvare
  0 siblings, 0 replies; 12+ messages in thread
From: Jean Delvare @ 2017-09-26  6:00 UTC (permalink / raw)
  To: sean.wang
  Cc: matthias.bgg, linux-mediatek, arnd, linux-arm-kernel, linux-kernel

On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> MTK_PMIC_WRAP is the basic and required configuration for those various
> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
> allows users tending to have the enablement for those PMICs.

I can't really make sense of the sentence above, sorry, and can't see
how it matches the change below anyway. MTK_PMIC_WRAP is already a
visible symbol before this change. The change is probably good in
itself, but please try to come up with a better description.

> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  drivers/soc/mediatek/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index a2fcd7f..d513629 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -15,7 +15,7 @@ config MTK_INFRACFG
>  config MTK_PMIC_WRAP
>  	tristate "MediaTek PMIC Wrapper Support"
>  	depends on ARCH_MEDIATEK
> -	depends on RESET_CONTROLLER
> +	select RESET_CONTROLLER
>  	select REGMAP
>  	help
>  	  Say yes here to add support for MediaTek PMIC Wrapper found


-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-09-26  6:00   ` Jean Delvare
  0 siblings, 0 replies; 12+ messages in thread
From: Jean Delvare @ 2017-09-26  6:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang at mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> MTK_PMIC_WRAP is the basic and required configuration for those various
> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
> allows users tending to have the enablement for those PMICs.

I can't really make sense of the sentence above, sorry, and can't see
how it matches the change below anyway. MTK_PMIC_WRAP is already a
visible symbol before this change. The change is probably good in
itself, but please try to come up with a better description.

> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  drivers/soc/mediatek/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> index a2fcd7f..d513629 100644
> --- a/drivers/soc/mediatek/Kconfig
> +++ b/drivers/soc/mediatek/Kconfig
> @@ -15,7 +15,7 @@ config MTK_INFRACFG
>  config MTK_PMIC_WRAP
>  	tristate "MediaTek PMIC Wrapper Support"
>  	depends on ARCH_MEDIATEK
> -	depends on RESET_CONTROLLER
> +	select RESET_CONTROLLER
>  	select REGMAP
>  	help
>  	  Say yes here to add support for MediaTek PMIC Wrapper found


-- 
Jean Delvare
SUSE L3 Support

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
  2017-09-26  6:00   ` Jean Delvare
  (?)
@ 2017-10-02 11:21     ` Arnd Bergmann
  -1 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2017-10-02 11:21 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Sean Wang, Matthias Brugger, moderated list:ARM/Mediatek SoC...,
	Linux ARM, Linux Kernel Mailing List

On Tue, Sep 26, 2017 at 8:00 AM, Jean Delvare <jdelvare@suse.de> wrote:
> On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang@mediatek.com wrote:
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> MTK_PMIC_WRAP is the basic and required configuration for those various
>> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
>> allows users tending to have the enablement for those PMICs.
>
> I can't really make sense of the sentence above, sorry, and can't see
> how it matches the change below anyway. MTK_PMIC_WRAP is already a
> visible symbol before this change. The change is probably good in
> itself, but please try to come up with a better description.
>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>>  drivers/soc/mediatek/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
>> index a2fcd7f..d513629 100644
>> --- a/drivers/soc/mediatek/Kconfig
>> +++ b/drivers/soc/mediatek/Kconfig
>> @@ -15,7 +15,7 @@ config MTK_INFRACFG
>>  config MTK_PMIC_WRAP
>>       tristate "MediaTek PMIC Wrapper Support"
>>       depends on ARCH_MEDIATEK
>> -     depends on RESET_CONTROLLER
>> +     select RESET_CONTROLLER
>>       select REGMAP

Your other patch now removes the ARCH_MEDIATEK dependency
and allows enabling the driver for COMPILE_TEST on architectures
that might not have ARCH_HAS_RESET_CONTROLLER set,
so you cannot 'select' the symbol in general.

I think a better solution is to leave the 'depends on
RESET_CONTROLLER' present here, but instead add
'select RESET_CONTROLLER' to the ARCH_MEDIATEK
definition.

Generally speaking, we don't want to mix 'select' and 'depends on'
statements for the same symbol, but if we do, it should be done
consistently in a very clear hierarchy. In case of RESET_CONTROLLER,
the rule seems to be to 'select' it from architectures and platforms,
while using 'depends on' from device drivers that absolutely require it.

Note that for compile-testing, it should be fine to rely on the fallback
definitions in include/linux/reset.h, so you might be able to just
remove the 'depends on' statement completely.

In any case, please try to be clearer about what the patch
actually tries to achieve. Did you run into a build error without
it? If so, please copy the exact build error message into the
patch description.

      Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-10-02 11:21     ` Arnd Bergmann
  0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2017-10-02 11:21 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Sean Wang, Matthias Brugger, moderated list:ARM/Mediatek SoC...,
	Linux ARM, Linux Kernel Mailing List

On Tue, Sep 26, 2017 at 8:00 AM, Jean Delvare <jdelvare@suse.de> wrote:
> On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang@mediatek.com wrote:
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> MTK_PMIC_WRAP is the basic and required configuration for those various
>> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
>> allows users tending to have the enablement for those PMICs.
>
> I can't really make sense of the sentence above, sorry, and can't see
> how it matches the change below anyway. MTK_PMIC_WRAP is already a
> visible symbol before this change. The change is probably good in
> itself, but please try to come up with a better description.
>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>>  drivers/soc/mediatek/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
>> index a2fcd7f..d513629 100644
>> --- a/drivers/soc/mediatek/Kconfig
>> +++ b/drivers/soc/mediatek/Kconfig
>> @@ -15,7 +15,7 @@ config MTK_INFRACFG
>>  config MTK_PMIC_WRAP
>>       tristate "MediaTek PMIC Wrapper Support"
>>       depends on ARCH_MEDIATEK
>> -     depends on RESET_CONTROLLER
>> +     select RESET_CONTROLLER
>>       select REGMAP

Your other patch now removes the ARCH_MEDIATEK dependency
and allows enabling the driver for COMPILE_TEST on architectures
that might not have ARCH_HAS_RESET_CONTROLLER set,
so you cannot 'select' the symbol in general.

I think a better solution is to leave the 'depends on
RESET_CONTROLLER' present here, but instead add
'select RESET_CONTROLLER' to the ARCH_MEDIATEK
definition.

Generally speaking, we don't want to mix 'select' and 'depends on'
statements for the same symbol, but if we do, it should be done
consistently in a very clear hierarchy. In case of RESET_CONTROLLER,
the rule seems to be to 'select' it from architectures and platforms,
while using 'depends on' from device drivers that absolutely require it.

Note that for compile-testing, it should be fine to rely on the fallback
definitions in include/linux/reset.h, so you might be able to just
remove the 'depends on' statement completely.

In any case, please try to be clearer about what the patch
actually tries to achieve. Did you run into a build error without
it? If so, please copy the exact build error message into the
patch description.

      Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-10-02 11:21     ` Arnd Bergmann
  0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2017-10-02 11:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 26, 2017 at 8:00 AM, Jean Delvare <jdelvare@suse.de> wrote:
> On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang at mediatek.com wrote:
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> MTK_PMIC_WRAP is the basic and required configuration for those various
>> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
>> allows users tending to have the enablement for those PMICs.
>
> I can't really make sense of the sentence above, sorry, and can't see
> how it matches the change below anyway. MTK_PMIC_WRAP is already a
> visible symbol before this change. The change is probably good in
> itself, but please try to come up with a better description.
>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>>  drivers/soc/mediatek/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
>> index a2fcd7f..d513629 100644
>> --- a/drivers/soc/mediatek/Kconfig
>> +++ b/drivers/soc/mediatek/Kconfig
>> @@ -15,7 +15,7 @@ config MTK_INFRACFG
>>  config MTK_PMIC_WRAP
>>       tristate "MediaTek PMIC Wrapper Support"
>>       depends on ARCH_MEDIATEK
>> -     depends on RESET_CONTROLLER
>> +     select RESET_CONTROLLER
>>       select REGMAP

Your other patch now removes the ARCH_MEDIATEK dependency
and allows enabling the driver for COMPILE_TEST on architectures
that might not have ARCH_HAS_RESET_CONTROLLER set,
so you cannot 'select' the symbol in general.

I think a better solution is to leave the 'depends on
RESET_CONTROLLER' present here, but instead add
'select RESET_CONTROLLER' to the ARCH_MEDIATEK
definition.

Generally speaking, we don't want to mix 'select' and 'depends on'
statements for the same symbol, but if we do, it should be done
consistently in a very clear hierarchy. In case of RESET_CONTROLLER,
the rule seems to be to 'select' it from architectures and platforms,
while using 'depends on' from device drivers that absolutely require it.

Note that for compile-testing, it should be fine to rely on the fallback
definitions in include/linux/reset.h, so you might be able to just
remove the 'depends on' statement completely.

In any case, please try to be clearer about what the patch
actually tries to achieve. Did you run into a build error without
it? If so, please copy the exact build error message into the
patch description.

      Arnd

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
  2017-10-02 11:21     ` Arnd Bergmann
  (?)
@ 2017-10-05  6:33       ` Sean Wang
  -1 siblings, 0 replies; 12+ messages in thread
From: Sean Wang @ 2017-10-05  6:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jean Delvare, Matthias Brugger,
	moderated list:ARM/Mediatek SoC...,
	Linux ARM, Linux Kernel Mailing List

On Mon, 2017-10-02 at 13:21 +0200, Arnd Bergmann wrote:
> On Tue, Sep 26, 2017 at 8:00 AM, Jean Delvare <jdelvare@suse.de> wrote:
> > On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang@mediatek.com wrote:
> >> From: Sean Wang <sean.wang@mediatek.com>
> >>
> >> MTK_PMIC_WRAP is the basic and required configuration for those various
> >> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
> >> allows users tending to have the enablement for those PMICs.
> >
> > I can't really make sense of the sentence above, sorry, and can't see
> > how it matches the change below anyway. MTK_PMIC_WRAP is already a
> > visible symbol before this change. The change is probably good in
> > itself, but please try to come up with a better description.
> >
> >> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> >> ---
> >>  drivers/soc/mediatek/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> >> index a2fcd7f..d513629 100644
> >> --- a/drivers/soc/mediatek/Kconfig
> >> +++ b/drivers/soc/mediatek/Kconfig
> >> @@ -15,7 +15,7 @@ config MTK_INFRACFG
> >>  config MTK_PMIC_WRAP
> >>       tristate "MediaTek PMIC Wrapper Support"
> >>       depends on ARCH_MEDIATEK
> >> -     depends on RESET_CONTROLLER
> >> +     select RESET_CONTROLLER
> >>       select REGMAP
> 
> Your other patch now removes the ARCH_MEDIATEK dependency
> and allows enabling the driver for COMPILE_TEST on architectures
> that might not have ARCH_HAS_RESET_CONTROLLER set,
> so you cannot 'select' the symbol in general.
> 
> I think a better solution is to leave the 'depends on
> RESET_CONTROLLER' present here, but instead add
> 'select RESET_CONTROLLER' to the ARCH_MEDIATEK
> definition.
> 
> Generally speaking, we don't want to mix 'select' and 'depends on'
> statements for the same symbol, but if we do, it should be done
> consistently in a very clear hierarchy. In case of RESET_CONTROLLER,
> the rule seems to be to 'select' it from architectures and platforms,
> while using 'depends on' from device drivers that absolutely require it.
> 
> Note that for compile-testing, it should be fine to rely on the fallback
> definitions in include/linux/reset.h, so you might be able to just
> remove the 'depends on' statement completely.
> 
> In any case, please try to be clearer about what the patch
> actually tries to achieve. Did you run into a build error without
> it? If so, please copy the exact build error message into the
> patch description.
> 
>       Arnd

Thanks for your both suggestions and idea

No any compile error is present prior to the patch.

In the initial thought, I simply would like to allow the configuration
item able to be visible in menuconfig without visiting all dependencies
for it, especially for those SoC-required drivers when the corresponding
architecture target is already picked up.

I prefer to the better solution leaving 'depends on RESET_CONTROLLER'
present here, and add additional 'select ARCH_RESET_CONTROLLER' to the
ARCH_MEDIATEK definition, which could reach the goal initially I'd try
to. So I will update them in this way in the next version, including a
better description of the patch.

	Sean 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-10-05  6:33       ` Sean Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Wang @ 2017-10-05  6:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jean Delvare, Matthias Brugger,
	moderated list:ARM/Mediatek SoC...,
	Linux ARM, Linux Kernel Mailing List

On Mon, 2017-10-02 at 13:21 +0200, Arnd Bergmann wrote:
> On Tue, Sep 26, 2017 at 8:00 AM, Jean Delvare <jdelvare@suse.de> wrote:
> > On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang@mediatek.com wrote:
> >> From: Sean Wang <sean.wang@mediatek.com>
> >>
> >> MTK_PMIC_WRAP is the basic and required configuration for those various
> >> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
> >> allows users tending to have the enablement for those PMICs.
> >
> > I can't really make sense of the sentence above, sorry, and can't see
> > how it matches the change below anyway. MTK_PMIC_WRAP is already a
> > visible symbol before this change. The change is probably good in
> > itself, but please try to come up with a better description.
> >
> >> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> >> ---
> >>  drivers/soc/mediatek/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> >> index a2fcd7f..d513629 100644
> >> --- a/drivers/soc/mediatek/Kconfig
> >> +++ b/drivers/soc/mediatek/Kconfig
> >> @@ -15,7 +15,7 @@ config MTK_INFRACFG
> >>  config MTK_PMIC_WRAP
> >>       tristate "MediaTek PMIC Wrapper Support"
> >>       depends on ARCH_MEDIATEK
> >> -     depends on RESET_CONTROLLER
> >> +     select RESET_CONTROLLER
> >>       select REGMAP
> 
> Your other patch now removes the ARCH_MEDIATEK dependency
> and allows enabling the driver for COMPILE_TEST on architectures
> that might not have ARCH_HAS_RESET_CONTROLLER set,
> so you cannot 'select' the symbol in general.
> 
> I think a better solution is to leave the 'depends on
> RESET_CONTROLLER' present here, but instead add
> 'select RESET_CONTROLLER' to the ARCH_MEDIATEK
> definition.
> 
> Generally speaking, we don't want to mix 'select' and 'depends on'
> statements for the same symbol, but if we do, it should be done
> consistently in a very clear hierarchy. In case of RESET_CONTROLLER,
> the rule seems to be to 'select' it from architectures and platforms,
> while using 'depends on' from device drivers that absolutely require it.
> 
> Note that for compile-testing, it should be fine to rely on the fallback
> definitions in include/linux/reset.h, so you might be able to just
> remove the 'depends on' statement completely.
> 
> In any case, please try to be clearer about what the patch
> actually tries to achieve. Did you run into a build error without
> it? If so, please copy the exact build error message into the
> patch description.
> 
>       Arnd

Thanks for your both suggestions and idea

No any compile error is present prior to the patch.

In the initial thought, I simply would like to allow the configuration
item able to be visible in menuconfig without visiting all dependencies
for it, especially for those SoC-required drivers when the corresponding
architecture target is already picked up.

I prefer to the better solution leaving 'depends on RESET_CONTROLLER'
present here, and add additional 'select ARCH_RESET_CONTROLLER' to the
ARCH_MEDIATEK definition, which could reach the goal initially I'd try
to. So I will update them in this way in the next version, including a
better description of the patch.

	Sean 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols
@ 2017-10-05  6:33       ` Sean Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Sean Wang @ 2017-10-05  6:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2017-10-02 at 13:21 +0200, Arnd Bergmann wrote:
> On Tue, Sep 26, 2017 at 8:00 AM, Jean Delvare <jdelvare@suse.de> wrote:
> > On Thu, 21 Sep 2017 17:01:05 +0800, sean.wang at mediatek.com wrote:
> >> From: Sean Wang <sean.wang@mediatek.com>
> >>
> >> MTK_PMIC_WRAP is the basic and required configuration for those various
> >> MediaTek PMICs, so turning MTK_PMIC_WRAP into visible symbols easily
> >> allows users tending to have the enablement for those PMICs.
> >
> > I can't really make sense of the sentence above, sorry, and can't see
> > how it matches the change below anyway. MTK_PMIC_WRAP is already a
> > visible symbol before this change. The change is probably good in
> > itself, but please try to come up with a better description.
> >
> >> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> >> ---
> >>  drivers/soc/mediatek/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
> >> index a2fcd7f..d513629 100644
> >> --- a/drivers/soc/mediatek/Kconfig
> >> +++ b/drivers/soc/mediatek/Kconfig
> >> @@ -15,7 +15,7 @@ config MTK_INFRACFG
> >>  config MTK_PMIC_WRAP
> >>       tristate "MediaTek PMIC Wrapper Support"
> >>       depends on ARCH_MEDIATEK
> >> -     depends on RESET_CONTROLLER
> >> +     select RESET_CONTROLLER
> >>       select REGMAP
> 
> Your other patch now removes the ARCH_MEDIATEK dependency
> and allows enabling the driver for COMPILE_TEST on architectures
> that might not have ARCH_HAS_RESET_CONTROLLER set,
> so you cannot 'select' the symbol in general.
> 
> I think a better solution is to leave the 'depends on
> RESET_CONTROLLER' present here, but instead add
> 'select RESET_CONTROLLER' to the ARCH_MEDIATEK
> definition.
> 
> Generally speaking, we don't want to mix 'select' and 'depends on'
> statements for the same symbol, but if we do, it should be done
> consistently in a very clear hierarchy. In case of RESET_CONTROLLER,
> the rule seems to be to 'select' it from architectures and platforms,
> while using 'depends on' from device drivers that absolutely require it.
> 
> Note that for compile-testing, it should be fine to rely on the fallback
> definitions in include/linux/reset.h, so you might be able to just
> remove the 'depends on' statement completely.
> 
> In any case, please try to be clearer about what the patch
> actually tries to achieve. Did you run into a build error without
> it? If so, please copy the exact build error message into the
> patch description.
> 
>       Arnd

Thanks for your both suggestions and idea

No any compile error is present prior to the patch.

In the initial thought, I simply would like to allow the configuration
item able to be visible in menuconfig without visiting all dependencies
for it, especially for those SoC-required drivers when the corresponding
architecture target is already picked up.

I prefer to the better solution leaving 'depends on RESET_CONTROLLER'
present here, and add additional 'select ARCH_RESET_CONTROLLER' to the
ARCH_MEDIATEK definition, which could reach the goal initially I'd try
to. So I will update them in this way in the next version, including a
better description of the patch.

	Sean 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-10-05  6:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-21  9:01 [PATCH] soc: mediatek: turn MTK_PMIC_WRAP into visible symbols sean.wang
2017-09-21  9:01 ` sean.wang at mediatek.com
2017-09-21  9:01 ` sean.wang
2017-09-26  6:00 ` Jean Delvare
2017-09-26  6:00   ` Jean Delvare
2017-09-26  6:00   ` Jean Delvare
2017-10-02 11:21   ` Arnd Bergmann
2017-10-02 11:21     ` Arnd Bergmann
2017-10-02 11:21     ` Arnd Bergmann
2017-10-05  6:33     ` Sean Wang
2017-10-05  6:33       ` Sean Wang
2017-10-05  6:33       ` Sean Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.