linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] memory: tegra: Fixes for COMPILE_TEST
@ 2021-06-09 11:28 Thierry Reding
  2021-06-09 11:28 ` [PATCH 1/2] memory: tegra: Add missing dependencies Thierry Reding
  2021-06-09 11:28 ` [PATCH 2/2] reset: Add compile-test stubs Thierry Reding
  0 siblings, 2 replies; 19+ messages in thread
From: Thierry Reding @ 2021-06-09 11:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Philipp Zabel
  Cc: Dmitry Osipenko, Jon Hunter, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

After COMPILE_TEST was enabled for the Tegra MC driver, Krzysztof
reported that this can fail to build on x86 configurations because some
dependencies are not explicitly pulled in.

Fix this by adding missing dependencies for OF_RESERVED_MEM and by
providing compile-test stubs for reset controller registration API.

Note that I was initially trying to fix the reset controller problem by
selecting RESET_CONTROLLER. This works but is discouraged because it can
create circular dependencies. And sure enough, changing that "select" to
a "depends on" triggered a circular dependency because there are already
quite a few drivers that select RESET_CONTROLLER. I suppose that's fine
as long as everybody uses "select" rather than "depends on", but it is
not a very robust solution.

Dmitry and Krzysztof were both in favour of adding the reset controller
stubs, so that's what I went with.

Thierry

Thierry Reding (2):
  memory: tegra: Add missing dependencies
  reset: Add compile-test stubs

 drivers/memory/tegra/Kconfig     |  2 ++
 include/linux/reset-controller.h | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

-- 
2.31.1


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

* [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-09 11:28 [PATCH 0/2] memory: tegra: Fixes for COMPILE_TEST Thierry Reding
@ 2021-06-09 11:28 ` Thierry Reding
  2021-06-09 11:58   ` Dmitry Osipenko
  2021-06-17  0:35   ` kernel test robot
  2021-06-09 11:28 ` [PATCH 2/2] reset: Add compile-test stubs Thierry Reding
  1 sibling, 2 replies; 19+ messages in thread
From: Thierry Reding @ 2021-06-09 11:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Philipp Zabel
  Cc: Dmitry Osipenko, Jon Hunter, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

When enabling the COMPILE_TEST Kconfig option, the Tegra memory
controller can be built without ARCH_TEGRA being selected. However, the
driver implicitly depends on some symbols pulled in via ARCH_TEGRA,
which causes the build to break.

Add explicit dependencies for OF_EARLY_FLATTREE and OF_RESERVED_MEM to
the Tegra MC Kconfig option to make sure they are selected even if
ARCH_TEGRA is not.

Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/memory/tegra/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index f9bae36c03a3..ecfb071fc4f4 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -48,6 +48,8 @@ config TEGRA124_EMC
 config TEGRA210_EMC_TABLE
 	bool
 	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
+	select OF_EARLY_FLATTREE
+	select OF_RESERVED_MEM
 
 config TEGRA210_EMC
 	tristate "NVIDIA Tegra210 External Memory Controller driver"
-- 
2.31.1


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

* [PATCH 2/2] reset: Add compile-test stubs
  2021-06-09 11:28 [PATCH 0/2] memory: tegra: Fixes for COMPILE_TEST Thierry Reding
  2021-06-09 11:28 ` [PATCH 1/2] memory: tegra: Add missing dependencies Thierry Reding
@ 2021-06-09 11:28 ` Thierry Reding
  2021-06-09 11:47   ` Philipp Zabel
  1 sibling, 1 reply; 19+ messages in thread
From: Thierry Reding @ 2021-06-09 11:28 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Philipp Zabel
  Cc: Dmitry Osipenko, Jon Hunter, linux-tegra, linux-kernel

From: Thierry Reding <treding@nvidia.com>

Add stubs for the reset controller registration functions to allow
building reset controller provider drivers with the COMPILE_TEST
Kconfig option enabled.

Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Suggested-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 include/linux/reset-controller.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/linux/reset-controller.h b/include/linux/reset-controller.h
index ec35814e0bbb..0fa4f60e1186 100644
--- a/include/linux/reset-controller.h
+++ b/include/linux/reset-controller.h
@@ -79,6 +79,7 @@ struct reset_controller_dev {
 	unsigned int nr_resets;
 };
 
+#if IS_ENABLED(CONFIG_RESET_CONTROLLER)
 int reset_controller_register(struct reset_controller_dev *rcdev);
 void reset_controller_unregister(struct reset_controller_dev *rcdev);
 
@@ -88,5 +89,26 @@ int devm_reset_controller_register(struct device *dev,
 
 void reset_controller_add_lookup(struct reset_control_lookup *lookup,
 				 unsigned int num_entries);
+#else
+static inline int reset_controller_register(struct reset_controller_dev *rcdev)
+{
+	return 0;
+}
+
+static inline void reset_controller_unregister(struct reset_controller_dev *rcdev)
+{
+}
+
+static inline int devm_reset_controller_register(struct device *dev,
+						 struct reset_controller_dev *rcdev)
+{
+	return 0;
+}
+
+static inline void reset_controller_add_lookup(struct reset_control_lookup *lookup,
+					       unsigned int num_entries)
+{
+}
+#endif
 
 #endif
-- 
2.31.1


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

* Re: [PATCH 2/2] reset: Add compile-test stubs
  2021-06-09 11:28 ` [PATCH 2/2] reset: Add compile-test stubs Thierry Reding
@ 2021-06-09 11:47   ` Philipp Zabel
  0 siblings, 0 replies; 19+ messages in thread
From: Philipp Zabel @ 2021-06-09 11:47 UTC (permalink / raw)
  To: Thierry Reding, Krzysztof Kozlowski
  Cc: Dmitry Osipenko, Jon Hunter, linux-tegra, linux-kernel

Hi Thierry,

On Wed, 2021-06-09 at 13:28 +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Add stubs for the reset controller registration functions to allow
> building reset controller provider drivers with the COMPILE_TEST
> Kconfig option enabled.
> 
> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Suggested-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Thank you, I've applied patch 2 to reset/next.

regards
Philipp

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-09 11:28 ` [PATCH 1/2] memory: tegra: Add missing dependencies Thierry Reding
@ 2021-06-09 11:58   ` Dmitry Osipenko
  2021-06-09 13:19     ` Krzysztof Kozlowski
  2021-06-17  0:35   ` kernel test robot
  1 sibling, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2021-06-09 11:58 UTC (permalink / raw)
  To: Thierry Reding, Krzysztof Kozlowski, Philipp Zabel
  Cc: Jon Hunter, linux-tegra, linux-kernel

09.06.2021 14:28, Thierry Reding пишет:
> From: Thierry Reding <treding@nvidia.com>
> 
> When enabling the COMPILE_TEST Kconfig option, the Tegra memory
> controller can be built without ARCH_TEGRA being selected. However, the
> driver implicitly depends on some symbols pulled in via ARCH_TEGRA,
> which causes the build to break.
> 
> Add explicit dependencies for OF_EARLY_FLATTREE and OF_RESERVED_MEM to
> the Tegra MC Kconfig option to make sure they are selected even if
> ARCH_TEGRA is not.
> 
> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/memory/tegra/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> index f9bae36c03a3..ecfb071fc4f4 100644
> --- a/drivers/memory/tegra/Kconfig
> +++ b/drivers/memory/tegra/Kconfig
> @@ -48,6 +48,8 @@ config TEGRA124_EMC
>  config TEGRA210_EMC_TABLE
>  	bool
>  	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
> +	select OF_EARLY_FLATTREE
> +	select OF_RESERVED_MEM
>  
>  config TEGRA210_EMC
>  	tristate "NVIDIA Tegra210 External Memory Controller driver"
> 

Will this work if CONFIG_OF is disabled?

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-09 11:58   ` Dmitry Osipenko
@ 2021-06-09 13:19     ` Krzysztof Kozlowski
  2021-06-09 16:57       ` Dmitry Osipenko
  2021-06-09 17:00       ` Thierry Reding
  0 siblings, 2 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-09 13:19 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Philipp Zabel
  Cc: Jon Hunter, linux-tegra, linux-kernel

On 09/06/2021 13:58, Dmitry Osipenko wrote:
> 09.06.2021 14:28, Thierry Reding пишет:
>> From: Thierry Reding <treding@nvidia.com>
>>
>> When enabling the COMPILE_TEST Kconfig option, the Tegra memory
>> controller can be built without ARCH_TEGRA being selected. However, the
>> driver implicitly depends on some symbols pulled in via ARCH_TEGRA,
>> which causes the build to break.
>>
>> Add explicit dependencies for OF_EARLY_FLATTREE and OF_RESERVED_MEM to
>> the Tegra MC Kconfig option to make sure they are selected even if
>> ARCH_TEGRA is not.
>>
>> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>> ---
>>  drivers/memory/tegra/Kconfig | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>> index f9bae36c03a3..ecfb071fc4f4 100644
>> --- a/drivers/memory/tegra/Kconfig
>> +++ b/drivers/memory/tegra/Kconfig
>> @@ -48,6 +48,8 @@ config TEGRA124_EMC
>>  config TEGRA210_EMC_TABLE
>>  	bool
>>  	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
>> +	select OF_EARLY_FLATTREE
>> +	select OF_RESERVED_MEM
>>  
>>  config TEGRA210_EMC
>>  	tristate "NVIDIA Tegra210 External Memory Controller driver"
>>
> 
> Will this work if CONFIG_OF is disabled?

Yeah, good question. That's why I propose "depends on". No issues with
unmet or circular dependencies.


Best regards,
Krzysztof

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-09 13:19     ` Krzysztof Kozlowski
@ 2021-06-09 16:57       ` Dmitry Osipenko
  2021-06-10  6:43         ` Krzysztof Kozlowski
  2021-06-09 17:00       ` Thierry Reding
  1 sibling, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2021-06-09 16:57 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding, Philipp Zabel
  Cc: Jon Hunter, linux-tegra, linux-kernel

09.06.2021 16:19, Krzysztof Kozlowski пишет:
> On 09/06/2021 13:58, Dmitry Osipenko wrote:
>> 09.06.2021 14:28, Thierry Reding пишет:
>>> From: Thierry Reding <treding@nvidia.com>
>>>
>>> When enabling the COMPILE_TEST Kconfig option, the Tegra memory
>>> controller can be built without ARCH_TEGRA being selected. However, the
>>> driver implicitly depends on some symbols pulled in via ARCH_TEGRA,
>>> which causes the build to break.
>>>
>>> Add explicit dependencies for OF_EARLY_FLATTREE and OF_RESERVED_MEM to
>>> the Tegra MC Kconfig option to make sure they are selected even if
>>> ARCH_TEGRA is not.
>>>
>>> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>> ---
>>>  drivers/memory/tegra/Kconfig | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>>> index f9bae36c03a3..ecfb071fc4f4 100644
>>> --- a/drivers/memory/tegra/Kconfig
>>> +++ b/drivers/memory/tegra/Kconfig
>>> @@ -48,6 +48,8 @@ config TEGRA124_EMC
>>>  config TEGRA210_EMC_TABLE
>>>  	bool
>>>  	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
>>> +	select OF_EARLY_FLATTREE
>>> +	select OF_RESERVED_MEM
>>>  
>>>  config TEGRA210_EMC
>>>  	tristate "NVIDIA Tegra210 External Memory Controller driver"
>>>
>>
>> Will this work if CONFIG_OF is disabled?
> 
> Yeah, good question. That's why I propose "depends on". No issues with
> unmet or circular dependencies.

What about to add stub for RESERVEDMEM_OF_DECLARE() + CONFIG_OF_RESERVED_MEM=n?

diff --git a/include/linux/of.h b/include/linux/of.h
index d8db8d3592fd..9c2e71e202d1 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1329,6 +1329,12 @@ static inline int of_get_available_child_count(const struct device_node *np)
 	return num;
 }
 
+#define _OF_DECLARE_STUB(table, name, compat, fn, fn_type)		\
+	static const struct of_device_id __of_table_##name		\
+		__attribute__((unused))					\
+		 = { .compatible = compat,				\
+		     .data = (fn == (fn_type)NULL) ? fn : fn }
+
 #if defined(CONFIG_OF) && !defined(MODULE)
 #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
 	static const struct of_device_id __of_table_##name		\
@@ -1338,10 +1344,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
 		     .data = (fn == (fn_type)NULL) ? fn : fn  }
 #else
 #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
-	static const struct of_device_id __of_table_##name		\
-		__attribute__((unused))					\
-		 = { .compatible = compat,				\
-		     .data = (fn == (fn_type)NULL) ? fn : fn }
+	_OF_DECLARE_STUB(table, name, compat, fn, fn_type)
 #endif
 
 typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index 76e4a0fffba4..4de2a24cadc9 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -27,11 +27,11 @@ struct reserved_mem_ops {
 
 typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
 
+#ifdef CONFIG_OF_RESERVED_MEM
+
 #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
 	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
 
-#ifdef CONFIG_OF_RESERVED_MEM
-
 int of_reserved_mem_device_init_by_idx(struct device *dev,
 				       struct device_node *np, int idx);
 int of_reserved_mem_device_init_by_name(struct device *dev,
@@ -41,6 +41,10 @@ void of_reserved_mem_device_release(struct device *dev);
 
 struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
 #else
+
+#define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
+	_OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
+
 static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
 					struct device_node *np, int idx)
 {



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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-09 13:19     ` Krzysztof Kozlowski
  2021-06-09 16:57       ` Dmitry Osipenko
@ 2021-06-09 17:00       ` Thierry Reding
  2021-06-10  6:42         ` Krzysztof Kozlowski
  1 sibling, 1 reply; 19+ messages in thread
From: Thierry Reding @ 2021-06-09 17:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Dmitry Osipenko, Philipp Zabel, Jon Hunter, linux-tegra, linux-kernel

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

On Wed, Jun 09, 2021 at 03:19:12PM +0200, Krzysztof Kozlowski wrote:
> On 09/06/2021 13:58, Dmitry Osipenko wrote:
> > 09.06.2021 14:28, Thierry Reding пишет:
> >> From: Thierry Reding <treding@nvidia.com>
> >>
> >> When enabling the COMPILE_TEST Kconfig option, the Tegra memory
> >> controller can be built without ARCH_TEGRA being selected. However, the
> >> driver implicitly depends on some symbols pulled in via ARCH_TEGRA,
> >> which causes the build to break.
> >>
> >> Add explicit dependencies for OF_EARLY_FLATTREE and OF_RESERVED_MEM to
> >> the Tegra MC Kconfig option to make sure they are selected even if
> >> ARCH_TEGRA is not.
> >>
> >> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> >> Signed-off-by: Thierry Reding <treding@nvidia.com>
> >> ---
> >>  drivers/memory/tegra/Kconfig | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> >> index f9bae36c03a3..ecfb071fc4f4 100644
> >> --- a/drivers/memory/tegra/Kconfig
> >> +++ b/drivers/memory/tegra/Kconfig
> >> @@ -48,6 +48,8 @@ config TEGRA124_EMC
> >>  config TEGRA210_EMC_TABLE
> >>  	bool
> >>  	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
> >> +	select OF_EARLY_FLATTREE
> >> +	select OF_RESERVED_MEM
> >>  
> >>  config TEGRA210_EMC
> >>  	tristate "NVIDIA Tegra210 External Memory Controller driver"
> >>
> > 
> > Will this work if CONFIG_OF is disabled?
> 
> Yeah, good question. That's why I propose "depends on". No issues with
> unmet or circular dependencies.

I couldn't find a way to make this work with "depends on" because
OF_RESERVED_MEM is not user-visible and the only way to get it enabled
is if something also selects OF_EARLY_FLATTREE, which is only ever done
at the architecture Kconfig level (and for OF unit testing).

So switching this to a "depends on" causes the TEGRA210_EMC never to get
enabled.

However, with OF disabled, the above causes issues because it can lead
to unmet direct dependencies. That, in turn, can be fixed by appending
&& OF to the COMPILE_TEST branch, which seems like a good enough
compromise.

Here's what I have on top of the above patch and that seems to do the
trick.

--- >8 ---
diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index ecfb071fc4f4..1c553895160c 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -47,13 +47,13 @@ config TEGRA124_EMC
 
 config TEGRA210_EMC_TABLE
 	bool
-	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
+	depends on ARCH_TEGRA_210_SOC || (COMPILE_TEST && OF)
 	select OF_EARLY_FLATTREE
 	select OF_RESERVED_MEM
 
 config TEGRA210_EMC
 	tristate "NVIDIA Tegra210 External Memory Controller driver"
-	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
+	depends on ARCH_TEGRA_210_SOC || (COMPILE_TEST && OF)
 	select TEGRA210_EMC_TABLE
 	help
 	  This driver is for the External Memory Controller (EMC) found on
--- >8 ---

So in a nutshell this will only get compile-tested if OF is enabled, but
then it will select OF_RESERVED_MEM and OF_EARLY_FLATTREE to get the
required symbols.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-09 17:00       ` Thierry Reding
@ 2021-06-10  6:42         ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-10  6:42 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dmitry Osipenko, Philipp Zabel, Jon Hunter, linux-tegra, linux-kernel

On 09/06/2021 19:00, Thierry Reding wrote:
> On Wed, Jun 09, 2021 at 03:19:12PM +0200, Krzysztof Kozlowski wrote:
>> On 09/06/2021 13:58, Dmitry Osipenko wrote:
>>> 09.06.2021 14:28, Thierry Reding пишет:
>>>> From: Thierry Reding <treding@nvidia.com>
>>>>
>>>> When enabling the COMPILE_TEST Kconfig option, the Tegra memory
>>>> controller can be built without ARCH_TEGRA being selected. However, the
>>>> driver implicitly depends on some symbols pulled in via ARCH_TEGRA,
>>>> which causes the build to break.
>>>>
>>>> Add explicit dependencies for OF_EARLY_FLATTREE and OF_RESERVED_MEM to
>>>> the Tegra MC Kconfig option to make sure they are selected even if
>>>> ARCH_TEGRA is not.
>>>>
>>>> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>> ---
>>>>  drivers/memory/tegra/Kconfig | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>>>> index f9bae36c03a3..ecfb071fc4f4 100644
>>>> --- a/drivers/memory/tegra/Kconfig
>>>> +++ b/drivers/memory/tegra/Kconfig
>>>> @@ -48,6 +48,8 @@ config TEGRA124_EMC
>>>>  config TEGRA210_EMC_TABLE
>>>>  	bool
>>>>  	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
>>>> +	select OF_EARLY_FLATTREE
>>>> +	select OF_RESERVED_MEM
>>>>  
>>>>  config TEGRA210_EMC
>>>>  	tristate "NVIDIA Tegra210 External Memory Controller driver"
>>>>
>>>
>>> Will this work if CONFIG_OF is disabled?
>>
>> Yeah, good question. That's why I propose "depends on". No issues with
>> unmet or circular dependencies.
> 
> I couldn't find a way to make this work with "depends on" because
> OF_RESERVED_MEM is not user-visible and the only way to get it enabled
> is if something also selects OF_EARLY_FLATTREE, which is only ever done
> at the architecture Kconfig level (and for OF unit testing).
> 
> So switching this to a "depends on" causes the TEGRA210_EMC never to get
> enabled.

Right.

> 
> However, with OF disabled, the above causes issues because it can lead
> to unmet direct dependencies. That, in turn, can be fixed by appending
> && OF to the COMPILE_TEST branch, which seems like a good enough
> compromise.
> 
> Here's what I have on top of the above patch and that seems to do the
> trick.
> 
> --- >8 ---
> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> index ecfb071fc4f4..1c553895160c 100644
> --- a/drivers/memory/tegra/Kconfig
> +++ b/drivers/memory/tegra/Kconfig
> @@ -47,13 +47,13 @@ config TEGRA124_EMC
>  
>  config TEGRA210_EMC_TABLE
>  	bool
> -	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
> +	depends on ARCH_TEGRA_210_SOC || (COMPILE_TEST && OF)
>  	select OF_EARLY_FLATTREE
>  	select OF_RESERVED_MEM
>  
>  config TEGRA210_EMC
>  	tristate "NVIDIA Tegra210 External Memory Controller driver"
> -	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
> +	depends on ARCH_TEGRA_210_SOC || (COMPILE_TEST && OF)
>  	select TEGRA210_EMC_TABLE
>  	help
>  	  This driver is for the External Memory Controller (EMC) found on
> --- >8 ---
> 
> So in a nutshell this will only get compile-tested if OF is enabled, but
> then it will select OF_RESERVED_MEM and OF_EARLY_FLATTREE to get the
> required symbols.

Could be also separate "depends on OF", even though it is included in
ARCH_TEGRA_XXX, but I am fine with this here.

Best regards,
Krzysztof

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-09 16:57       ` Dmitry Osipenko
@ 2021-06-10  6:43         ` Krzysztof Kozlowski
  2021-06-10 15:50           ` Dmitry Osipenko
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-10  6:43 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Philipp Zabel
  Cc: Jon Hunter, linux-tegra, linux-kernel

On 09/06/2021 18:57, Dmitry Osipenko wrote:
> 09.06.2021 16:19, Krzysztof Kozlowski пишет:
>> On 09/06/2021 13:58, Dmitry Osipenko wrote:
>>> 09.06.2021 14:28, Thierry Reding пишет:
>>>> From: Thierry Reding <treding@nvidia.com>
>>>>
>>>> When enabling the COMPILE_TEST Kconfig option, the Tegra memory
>>>> controller can be built without ARCH_TEGRA being selected. However, the
>>>> driver implicitly depends on some symbols pulled in via ARCH_TEGRA,
>>>> which causes the build to break.
>>>>
>>>> Add explicit dependencies for OF_EARLY_FLATTREE and OF_RESERVED_MEM to
>>>> the Tegra MC Kconfig option to make sure they are selected even if
>>>> ARCH_TEGRA is not.
>>>>
>>>> Reported-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>>> Signed-off-by: Thierry Reding <treding@nvidia.com>
>>>> ---
>>>>  drivers/memory/tegra/Kconfig | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>>>> index f9bae36c03a3..ecfb071fc4f4 100644
>>>> --- a/drivers/memory/tegra/Kconfig
>>>> +++ b/drivers/memory/tegra/Kconfig
>>>> @@ -48,6 +48,8 @@ config TEGRA124_EMC
>>>>  config TEGRA210_EMC_TABLE
>>>>  	bool
>>>>  	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
>>>> +	select OF_EARLY_FLATTREE
>>>> +	select OF_RESERVED_MEM
>>>>  
>>>>  config TEGRA210_EMC
>>>>  	tristate "NVIDIA Tegra210 External Memory Controller driver"
>>>>
>>>
>>> Will this work if CONFIG_OF is disabled?
>>
>> Yeah, good question. That's why I propose "depends on". No issues with
>> unmet or circular dependencies.
> 
> What about to add stub for RESERVEDMEM_OF_DECLARE() + CONFIG_OF_RESERVED_MEM=n?
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index d8db8d3592fd..9c2e71e202d1 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1329,6 +1329,12 @@ static inline int of_get_available_child_count(const struct device_node *np)
>  	return num;
>  }
>  
> +#define _OF_DECLARE_STUB(table, name, compat, fn, fn_type)		\
> +	static const struct of_device_id __of_table_##name		\
> +		__attribute__((unused))					\
> +		 = { .compatible = compat,				\
> +		     .data = (fn == (fn_type)NULL) ? fn : fn }
> +
>  #if defined(CONFIG_OF) && !defined(MODULE)
>  #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
>  	static const struct of_device_id __of_table_##name		\
> @@ -1338,10 +1344,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
>  		     .data = (fn == (fn_type)NULL) ? fn : fn  }
>  #else
>  #define _OF_DECLARE(table, name, compat, fn, fn_type)			\
> -	static const struct of_device_id __of_table_##name		\
> -		__attribute__((unused))					\
> -		 = { .compatible = compat,				\
> -		     .data = (fn == (fn_type)NULL) ? fn : fn }
> +	_OF_DECLARE_STUB(table, name, compat, fn, fn_type)
>  #endif
>  
>  typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 76e4a0fffba4..4de2a24cadc9 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -27,11 +27,11 @@ struct reserved_mem_ops {
>  
>  typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem);
>  
> +#ifdef CONFIG_OF_RESERVED_MEM
> +
>  #define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
>  	_OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn)
>  
> -#ifdef CONFIG_OF_RESERVED_MEM
> -
>  int of_reserved_mem_device_init_by_idx(struct device *dev,
>  				       struct device_node *np, int idx);
>  int of_reserved_mem_device_init_by_name(struct device *dev,
> @@ -41,6 +41,10 @@ void of_reserved_mem_device_release(struct device *dev);
>  
>  struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
>  #else
> +
> +#define RESERVEDMEM_OF_DECLARE(name, compat, init)			\
> +	_OF_DECLARE_STUB(reservedmem, name, compat, init, reservedmem_of_init_fn)
> +
>  static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
>  					struct device_node *np, int idx)
>  {

The stubs might be good idea anyway, but the driver explicitly needs for
runtime working reservedmem, so it should select it.

Best regards,
Krzysztof

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-10  6:43         ` Krzysztof Kozlowski
@ 2021-06-10 15:50           ` Dmitry Osipenko
  2021-06-10 16:23             ` Dmitry Osipenko
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2021-06-10 15:50 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding, Philipp Zabel
  Cc: Jon Hunter, linux-tegra, linux-kernel

10.06.2021 09:43, Krzysztof Kozlowski пишет:
> The stubs might be good idea anyway, but the driver explicitly needs for
> runtime working reservedmem, so it should select it.

The OF and reservedmem are both selected by the ARCH for the runtime
use. They may not be selected in the case of compile-testing.

Both OF core and reservedmem provide stubs needed for compile-testing,
it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding
the missing stub should be a more appropriate solution than adding extra
Kconfig dependencies, IMO.

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-10 15:50           ` Dmitry Osipenko
@ 2021-06-10 16:23             ` Dmitry Osipenko
  2021-06-11  6:50               ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2021-06-10 16:23 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding, Philipp Zabel
  Cc: Jon Hunter, linux-tegra, linux-kernel

10.06.2021 18:50, Dmitry Osipenko пишет:
> 10.06.2021 09:43, Krzysztof Kozlowski пишет:
>> The stubs might be good idea anyway, but the driver explicitly needs for
>> runtime working reservedmem, so it should select it.
> 
> The OF and reservedmem are both selected by the ARCH for the runtime
> use. They may not be selected in the case of compile-testing.
> 
> Both OF core and reservedmem provide stubs needed for compile-testing,
> it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding
> the missing stub should be a more appropriate solution than adding extra
> Kconfig dependencies, IMO.
> 

I will send the patch. If OF maintainers will have objections, then
adding the dependency may become a more reasonable option.

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-10 16:23             ` Dmitry Osipenko
@ 2021-06-11  6:50               ` Krzysztof Kozlowski
  2021-06-11  7:21                 ` Dmitry Osipenko
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-11  6:50 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Philipp Zabel
  Cc: Jon Hunter, linux-tegra, linux-kernel

On 10/06/2021 18:23, Dmitry Osipenko wrote:
> 10.06.2021 18:50, Dmitry Osipenko пишет:
>> 10.06.2021 09:43, Krzysztof Kozlowski пишет:
>>> The stubs might be good idea anyway, but the driver explicitly needs for
>>> runtime working reservedmem, so it should select it.
>>
>> The OF and reservedmem are both selected by the ARCH for the runtime
>> use. They may not be selected in the case of compile-testing.
>>
>> Both OF core and reservedmem provide stubs needed for compile-testing,
>> it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding
>> the missing stub should be a more appropriate solution than adding extra
>> Kconfig dependencies, IMO.

Ah, in such case everything looks good. Stubs is indeed proper choice.

> I will send the patch. If OF maintainers will have objections, then
> adding the dependency may become a more reasonable option.

Thanks!


Best regards,
Krzysztof

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-11  6:50               ` Krzysztof Kozlowski
@ 2021-06-11  7:21                 ` Dmitry Osipenko
  2021-06-11 11:00                   ` Thierry Reding
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2021-06-11  7:21 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding
  Cc: Jon Hunter, linux-tegra, linux-kernel, Philipp Zabel

11.06.2021 09:50, Krzysztof Kozlowski пишет:
> On 10/06/2021 18:23, Dmitry Osipenko wrote:
>> 10.06.2021 18:50, Dmitry Osipenko пишет:
>>> 10.06.2021 09:43, Krzysztof Kozlowski пишет:
>>>> The stubs might be good idea anyway, but the driver explicitly needs for
>>>> runtime working reservedmem, so it should select it.
>>>
>>> The OF and reservedmem are both selected by the ARCH for the runtime
>>> use. They may not be selected in the case of compile-testing.
>>>
>>> Both OF core and reservedmem provide stubs needed for compile-testing,
>>> it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding
>>> the missing stub should be a more appropriate solution than adding extra
>>> Kconfig dependencies, IMO.
> 
> Ah, in such case everything looks good. Stubs is indeed proper choice.

Although, I see that there are only two Kconfigs that have
OF_RESERVED_MEM, one defines the OF_RESERVED_MEM, the other is QCOM
Kconfig which depends on OF_RESERVED_MEM. The OF_RESERVED_MEM is enabled
by default in defconfig.

You're right, we need the Kconfig change to be entirely correct, since
driver won't work properly without OF_RESERVED_MEM.

config TEGRA210_EMC
	tristate "NVIDIA Tegra210 External Memory Controller driver"
-	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
+	depends on (ARCH_TEGRA_210_SOC && OF_RESERVED_MEM) || COMPILE_TEST

I will send that change later today.

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-11  7:21                 ` Dmitry Osipenko
@ 2021-06-11 11:00                   ` Thierry Reding
  2021-06-11 13:40                     ` Dmitry Osipenko
  0 siblings, 1 reply; 19+ messages in thread
From: Thierry Reding @ 2021-06-11 11:00 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: Krzysztof Kozlowski, Jon Hunter, linux-tegra, linux-kernel,
	Philipp Zabel

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

On Fri, Jun 11, 2021 at 10:21:41AM +0300, Dmitry Osipenko wrote:
> 11.06.2021 09:50, Krzysztof Kozlowski пишет:
> > On 10/06/2021 18:23, Dmitry Osipenko wrote:
> >> 10.06.2021 18:50, Dmitry Osipenko пишет:
> >>> 10.06.2021 09:43, Krzysztof Kozlowski пишет:
> >>>> The stubs might be good idea anyway, but the driver explicitly needs for
> >>>> runtime working reservedmem, so it should select it.
> >>>
> >>> The OF and reservedmem are both selected by the ARCH for the runtime
> >>> use. They may not be selected in the case of compile-testing.
> >>>
> >>> Both OF core and reservedmem provide stubs needed for compile-testing,
> >>> it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding
> >>> the missing stub should be a more appropriate solution than adding extra
> >>> Kconfig dependencies, IMO.
> > 
> > Ah, in such case everything looks good. Stubs is indeed proper choice.
> 
> Although, I see that there are only two Kconfigs that have
> OF_RESERVED_MEM, one defines the OF_RESERVED_MEM, the other is QCOM
> Kconfig which depends on OF_RESERVED_MEM. The OF_RESERVED_MEM is enabled
> by default in defconfig.
> 
> You're right, we need the Kconfig change to be entirely correct, since
> driver won't work properly without OF_RESERVED_MEM.
> 
> config TEGRA210_EMC
> 	tristate "NVIDIA Tegra210 External Memory Controller driver"
> -	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
> +	depends on (ARCH_TEGRA_210_SOC && OF_RESERVED_MEM) || COMPILE_TEST
> 
> I will send that change later today.

That's completely unnecessary. OF_RESERVED_MEM is enabled by default if
OF_EARLY_FLATTREE is enabled, which it is for ARM64 and that is always
enabled for ARCH_TEGRA_210_SOC.

What Krzysztof had originally proposed, as far as I understand, is to
add "depends on OF_RESERVED_MEM" so that the dependency is always there
(including the COMPILE_TEST case). However, that's a bit problematic, as
I said earlier, because OF_RESERVED_MEM is not user-visible and neither
is OF_EARLY_FLATTREE, so there's no way to enable OF_RESERVED_MEM unless
the architecture selected it, which it doesn't on x86, so it kind of
defeats the purpose of COMPILE_TEST.

So I think if this really has to be compile-test enabled, the only way
to do that is to either make this select OF_EARLY_FLATTREE, or add the
stubs.

Another option would perhaps be to enable OF_UNITTEST along with
COMPILE_TEST, since that also pulls in OF_EARLY_FLATTREE and would allow
this driver to be built even on x86.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-11 11:00                   ` Thierry Reding
@ 2021-06-11 13:40                     ` Dmitry Osipenko
  2021-06-14 11:50                       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Dmitry Osipenko @ 2021-06-11 13:40 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Krzysztof Kozlowski, Jon Hunter, linux-tegra, linux-kernel,
	Philipp Zabel

11.06.2021 14:00, Thierry Reding пишет:
> On Fri, Jun 11, 2021 at 10:21:41AM +0300, Dmitry Osipenko wrote:
>> 11.06.2021 09:50, Krzysztof Kozlowski пишет:
>>> On 10/06/2021 18:23, Dmitry Osipenko wrote:
>>>> 10.06.2021 18:50, Dmitry Osipenko пишет:
>>>>> 10.06.2021 09:43, Krzysztof Kozlowski пишет:
>>>>>> The stubs might be good idea anyway, but the driver explicitly needs for
>>>>>> runtime working reservedmem, so it should select it.
>>>>>
>>>>> The OF and reservedmem are both selected by the ARCH for the runtime
>>>>> use. They may not be selected in the case of compile-testing.
>>>>>
>>>>> Both OF core and reservedmem provide stubs needed for compile-testing,
>>>>> it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding
>>>>> the missing stub should be a more appropriate solution than adding extra
>>>>> Kconfig dependencies, IMO.
>>>
>>> Ah, in such case everything looks good. Stubs is indeed proper choice.
>>
>> Although, I see that there are only two Kconfigs that have
>> OF_RESERVED_MEM, one defines the OF_RESERVED_MEM, the other is QCOM
>> Kconfig which depends on OF_RESERVED_MEM. The OF_RESERVED_MEM is enabled
>> by default in defconfig.
>>
>> You're right, we need the Kconfig change to be entirely correct, since
>> driver won't work properly without OF_RESERVED_MEM.
>>
>> config TEGRA210_EMC
>> 	tristate "NVIDIA Tegra210 External Memory Controller driver"
>> -	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
>> +	depends on (ARCH_TEGRA_210_SOC && OF_RESERVED_MEM) || COMPILE_TEST
>>
>> I will send that change later today.
> 
> That's completely unnecessary. OF_RESERVED_MEM is enabled by default if
> OF_EARLY_FLATTREE is enabled, which it is for ARM64 and that is always
> enabled for ARCH_TEGRA_210_SOC.

But it doesn't stop you from disabling OF_RESERVED_MEM. The Kconfig
dependencies should reflect the build and runtime requirements of the
driver, otherwise only driver author knows which config options are need.

> What Krzysztof had originally proposed, as far as I understand, is to
> add "depends on OF_RESERVED_MEM" so that the dependency is always there
> (including the COMPILE_TEST case). However, that's a bit problematic, as
> I said earlier, because OF_RESERVED_MEM is not user-visible and neither
> is OF_EARLY_FLATTREE, so there's no way to enable OF_RESERVED_MEM unless
> the architecture selected it, which it doesn't on x86, so it kind of
> defeats the purpose of COMPILE_TEST.

Indeed, the QCOM driver isn't compile-tested as much as it could be.
That driver already shouldn't have any problems with compile-testing,
maybe some stubs were missing when driver was originally added.

> So I think if this really has to be compile-test enabled, the only way
> to do that is to either make this select OF_EARLY_FLATTREE, or add the
> stubs.
> 
> Another option would perhaps be to enable OF_UNITTEST along with
> COMPILE_TEST, since that also pulls in OF_EARLY_FLATTREE and would allow
> this driver to be built even on x86.

For the universal compile-testing it should be enough to fix the stub.
We may consider other options if it won't be enough, thank you for the
suggestions.

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-11 13:40                     ` Dmitry Osipenko
@ 2021-06-14 11:50                       ` Krzysztof Kozlowski
  2021-06-14 14:16                         ` Dmitry Osipenko
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2021-06-14 11:50 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding
  Cc: Jon Hunter, linux-tegra, linux-kernel, Philipp Zabel

On 11/06/2021 15:40, Dmitry Osipenko wrote:
> 11.06.2021 14:00, Thierry Reding пишет:
>> On Fri, Jun 11, 2021 at 10:21:41AM +0300, Dmitry Osipenko wrote:
>>> 11.06.2021 09:50, Krzysztof Kozlowski пишет:
>>>> On 10/06/2021 18:23, Dmitry Osipenko wrote:
>>>>> 10.06.2021 18:50, Dmitry Osipenko пишет:
>>>>>> 10.06.2021 09:43, Krzysztof Kozlowski пишет:
>>>>>>> The stubs might be good idea anyway, but the driver explicitly needs for
>>>>>>> runtime working reservedmem, so it should select it.
>>>>>>
>>>>>> The OF and reservedmem are both selected by the ARCH for the runtime
>>>>>> use. They may not be selected in the case of compile-testing.
>>>>>>
>>>>>> Both OF core and reservedmem provide stubs needed for compile-testing,
>>>>>> it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding
>>>>>> the missing stub should be a more appropriate solution than adding extra
>>>>>> Kconfig dependencies, IMO.
>>>>
>>>> Ah, in such case everything looks good. Stubs is indeed proper choice.
>>>
>>> Although, I see that there are only two Kconfigs that have
>>> OF_RESERVED_MEM, one defines the OF_RESERVED_MEM, the other is QCOM
>>> Kconfig which depends on OF_RESERVED_MEM. The OF_RESERVED_MEM is enabled
>>> by default in defconfig.
>>>
>>> You're right, we need the Kconfig change to be entirely correct, since
>>> driver won't work properly without OF_RESERVED_MEM.
>>>
>>> config TEGRA210_EMC
>>> 	tristate "NVIDIA Tegra210 External Memory Controller driver"
>>> -	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
>>> +	depends on (ARCH_TEGRA_210_SOC && OF_RESERVED_MEM) || COMPILE_TEST
>>>
>>> I will send that change later today.
>>
>> That's completely unnecessary. OF_RESERVED_MEM is enabled by default if
>> OF_EARLY_FLATTREE is enabled, which it is for ARM64 and that is always
>> enabled for ARCH_TEGRA_210_SOC.
> 
> But it doesn't stop you from disabling OF_RESERVED_MEM. The Kconfig
> dependencies should reflect the build and runtime requirements of the
> driver, otherwise only driver author knows which config options are need.

OF_RESERVED_MEM is not selectable, so it cannot be disabled for regular
builds.

When I proposed to add dependencies, I indeed did not check whether
these are selectable symbols. My advise was not accurate.

Usually we do not add dependencies to each driver on each kernel
non-selectable feature. It would be too much (depends on HAS_IOMEM,
REGMAP, OF, MFD_SYSCON, OF_RESERVED_MEM and probably many more)... There
are of course exceptions and mistakes (I think I should not add
OF_ADDRESS to OMAP_GPMC).

If such features are non-selectable, usually architecture enforces them
so the driver does not need to. For compile testing, these features
should come with stubs. If there are no stubs, the dependency for
compile testing could be added:
ARCH_TEGRA_210_SOC || (COMPILE_TEST && OF_RESERVED_MEM)

We add the dependencies for selectable options which driver needs, e.g.
DEVFREQ, CPUFREQ, PM_xxx

Since you proposed already to add stubs for OF_RESERVED_MEM, adding
dependency on it does not bring any benefit.

Best regards,
Krzysztof

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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-14 11:50                       ` Krzysztof Kozlowski
@ 2021-06-14 14:16                         ` Dmitry Osipenko
  0 siblings, 0 replies; 19+ messages in thread
From: Dmitry Osipenko @ 2021-06-14 14:16 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding
  Cc: Jon Hunter, linux-tegra, linux-kernel, Philipp Zabel

14.06.2021 14:50, Krzysztof Kozlowski пишет:
> On 11/06/2021 15:40, Dmitry Osipenko wrote:
>> 11.06.2021 14:00, Thierry Reding пишет:
>>> On Fri, Jun 11, 2021 at 10:21:41AM +0300, Dmitry Osipenko wrote:
>>>> 11.06.2021 09:50, Krzysztof Kozlowski пишет:
>>>>> On 10/06/2021 18:23, Dmitry Osipenko wrote:
>>>>>> 10.06.2021 18:50, Dmitry Osipenko пишет:
>>>>>>> 10.06.2021 09:43, Krzysztof Kozlowski пишет:
>>>>>>>> The stubs might be good idea anyway, but the driver explicitly needs for
>>>>>>>> runtime working reservedmem, so it should select it.
>>>>>>>
>>>>>>> The OF and reservedmem are both selected by the ARCH for the runtime
>>>>>>> use. They may not be selected in the case of compile-testing.
>>>>>>>
>>>>>>> Both OF core and reservedmem provide stubs needed for compile-testing,
>>>>>>> it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding
>>>>>>> the missing stub should be a more appropriate solution than adding extra
>>>>>>> Kconfig dependencies, IMO.
>>>>>
>>>>> Ah, in such case everything looks good. Stubs is indeed proper choice.
>>>>
>>>> Although, I see that there are only two Kconfigs that have
>>>> OF_RESERVED_MEM, one defines the OF_RESERVED_MEM, the other is QCOM
>>>> Kconfig which depends on OF_RESERVED_MEM. The OF_RESERVED_MEM is enabled
>>>> by default in defconfig.
>>>>
>>>> You're right, we need the Kconfig change to be entirely correct, since
>>>> driver won't work properly without OF_RESERVED_MEM.
>>>>
>>>> config TEGRA210_EMC
>>>> 	tristate "NVIDIA Tegra210 External Memory Controller driver"
>>>> -	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
>>>> +	depends on (ARCH_TEGRA_210_SOC && OF_RESERVED_MEM) || COMPILE_TEST
>>>>
>>>> I will send that change later today.
>>>
>>> That's completely unnecessary. OF_RESERVED_MEM is enabled by default if
>>> OF_EARLY_FLATTREE is enabled, which it is for ARM64 and that is always
>>> enabled for ARCH_TEGRA_210_SOC.
>>
>> But it doesn't stop you from disabling OF_RESERVED_MEM. The Kconfig
>> dependencies should reflect the build and runtime requirements of the
>> driver, otherwise only driver author knows which config options are need.
> 
> OF_RESERVED_MEM is not selectable, so it cannot be disabled for regular
> builds.
> 
> When I proposed to add dependencies, I indeed did not check whether
> these are selectable symbols. My advise was not accurate.
> 
> Usually we do not add dependencies to each driver on each kernel
> non-selectable feature. It would be too much (depends on HAS_IOMEM,
> REGMAP, OF, MFD_SYSCON, OF_RESERVED_MEM and probably many more)... There
> are of course exceptions and mistakes (I think I should not add
> OF_ADDRESS to OMAP_GPMC).
> 
> If such features are non-selectable, usually architecture enforces them
> so the driver does not need to. For compile testing, these features
> should come with stubs. If there are no stubs, the dependency for
> compile testing could be added:
> ARCH_TEGRA_210_SOC || (COMPILE_TEST && OF_RESERVED_MEM)
> 
> We add the dependencies for selectable options which driver needs, e.g.
> DEVFREQ, CPUFREQ, PM_xxx
> 
> Since you proposed already to add stubs for OF_RESERVED_MEM, adding
> dependency on it does not bring any benefit.

Right, I also missed that OF_RESERVED_MEM is non-selectable.


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

* Re: [PATCH 1/2] memory: tegra: Add missing dependencies
  2021-06-09 11:28 ` [PATCH 1/2] memory: tegra: Add missing dependencies Thierry Reding
  2021-06-09 11:58   ` Dmitry Osipenko
@ 2021-06-17  0:35   ` kernel test robot
  1 sibling, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-06-17  0:35 UTC (permalink / raw)
  To: Thierry Reding, Krzysztof Kozlowski, Philipp Zabel
  Cc: kbuild-all, Dmitry Osipenko, Jon Hunter, linux-tegra, linux-kernel

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

Hi Thierry,

I love your patch! Yet something to improve:

[auto build test ERROR on tegra/for-next]
[also build test ERROR on next-20210616]
[cannot apply to pza/reset/next tegra-drm/drm/tegra/for-next v5.13-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Thierry-Reding/memory-tegra-Fixes-for-COMPILE_TEST/20210616-154340
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git for-next
config: parisc-randconfig-c023-20210616 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0d0e9cbf83822694f35eca16dce8c5406b4f464f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Thierry-Reding/memory-tegra-Fixes-for-COMPILE_TEST/20210616-154340
        git checkout 0d0e9cbf83822694f35eca16dce8c5406b4f464f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/tty/serial/earlycon.c: In function 'of_setup_earlycon':
>> drivers/tty/serial/earlycon.c:258:9: error: implicit declaration of function 'of_flat_dt_translate_address' [-Werror=implicit-function-declaration]
     258 |  addr = of_flat_dt_translate_address(node);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for OF_EARLY_FLATTREE
   Depends on OF
   Selected by
   - TEGRA210_EMC_TABLE && MEMORY && TEGRA_MC && (ARCH_TEGRA_210_SOC || COMPILE_TEST
   WARNING: unmet direct dependencies detected for OF_RESERVED_MEM
   Depends on OF && OF_EARLY_FLATTREE
   Selected by
   - TEGRA210_EMC_TABLE && MEMORY && TEGRA_MC && (ARCH_TEGRA_210_SOC || COMPILE_TEST


vim +/of_flat_dt_translate_address +258 drivers/tty/serial/earlycon.c

8477614d9f7c5c Peter Hurley       2016-01-16  245  
c90fe9c0394b06 Peter Hurley       2016-01-16  246  int __init of_setup_earlycon(const struct earlycon_id *match,
088da2a17619cf Peter Hurley       2016-01-16  247  			     unsigned long node,
4d118c9a866590 Peter Hurley       2016-01-16  248  			     const char *options)
b0b6abd34c1b50 Rob Herring        2014-03-27  249  {
b0b6abd34c1b50 Rob Herring        2014-03-27  250  	int err;
b0b6abd34c1b50 Rob Herring        2014-03-27  251  	struct uart_port *port = &early_console_dev.port;
088da2a17619cf Peter Hurley       2016-01-16  252  	const __be32 *val;
088da2a17619cf Peter Hurley       2016-01-16  253  	bool big_endian;
c90fe9c0394b06 Peter Hurley       2016-01-16  254  	u64 addr;
b0b6abd34c1b50 Rob Herring        2014-03-27  255  
e1dd3bef6d03c9 Geert Uytterhoeven 2015-11-27  256  	spin_lock_init(&port->lock);
b0b6abd34c1b50 Rob Herring        2014-03-27  257  	port->iotype = UPIO_MEM;
c90fe9c0394b06 Peter Hurley       2016-01-16 @258  	addr = of_flat_dt_translate_address(node);
c90fe9c0394b06 Peter Hurley       2016-01-16  259  	if (addr == OF_BAD_ADDR) {
c90fe9c0394b06 Peter Hurley       2016-01-16  260  		pr_warn("[%s] bad address\n", match->name);
c90fe9c0394b06 Peter Hurley       2016-01-16  261  		return -ENXIO;
c90fe9c0394b06 Peter Hurley       2016-01-16  262  	}
b0b6abd34c1b50 Rob Herring        2014-03-27  263  	port->mapbase = addr;
b0b6abd34c1b50 Rob Herring        2014-03-27  264  
088da2a17619cf Peter Hurley       2016-01-16  265  	val = of_get_flat_dt_prop(node, "reg-offset", NULL);
088da2a17619cf Peter Hurley       2016-01-16  266  	if (val)
088da2a17619cf Peter Hurley       2016-01-16  267  		port->mapbase += be32_to_cpu(*val);
1f66dd36bb1843 Greentime Hu       2018-02-13  268  	port->membase = earlycon_map(port->mapbase, SZ_4K);
1f66dd36bb1843 Greentime Hu       2018-02-13  269  
088da2a17619cf Peter Hurley       2016-01-16  270  	val = of_get_flat_dt_prop(node, "reg-shift", NULL);
088da2a17619cf Peter Hurley       2016-01-16  271  	if (val)
088da2a17619cf Peter Hurley       2016-01-16  272  		port->regshift = be32_to_cpu(*val);
088da2a17619cf Peter Hurley       2016-01-16  273  	big_endian = of_get_flat_dt_prop(node, "big-endian", NULL) != NULL ||
088da2a17619cf Peter Hurley       2016-01-16  274  		(IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) &&
088da2a17619cf Peter Hurley       2016-01-16  275  		 of_get_flat_dt_prop(node, "native-endian", NULL) != NULL);
088da2a17619cf Peter Hurley       2016-01-16  276  	val = of_get_flat_dt_prop(node, "reg-io-width", NULL);
088da2a17619cf Peter Hurley       2016-01-16  277  	if (val) {
088da2a17619cf Peter Hurley       2016-01-16  278  		switch (be32_to_cpu(*val)) {
088da2a17619cf Peter Hurley       2016-01-16  279  		case 1:
088da2a17619cf Peter Hurley       2016-01-16  280  			port->iotype = UPIO_MEM;
088da2a17619cf Peter Hurley       2016-01-16  281  			break;
088da2a17619cf Peter Hurley       2016-01-16  282  		case 2:
088da2a17619cf Peter Hurley       2016-01-16  283  			port->iotype = UPIO_MEM16;
088da2a17619cf Peter Hurley       2016-01-16  284  			break;
088da2a17619cf Peter Hurley       2016-01-16  285  		case 4:
088da2a17619cf Peter Hurley       2016-01-16  286  			port->iotype = (big_endian) ? UPIO_MEM32BE : UPIO_MEM32;
088da2a17619cf Peter Hurley       2016-01-16  287  			break;
088da2a17619cf Peter Hurley       2016-01-16  288  		default:
088da2a17619cf Peter Hurley       2016-01-16  289  			pr_warn("[%s] unsupported reg-io-width\n", match->name);
088da2a17619cf Peter Hurley       2016-01-16  290  			return -EINVAL;
088da2a17619cf Peter Hurley       2016-01-16  291  		}
088da2a17619cf Peter Hurley       2016-01-16  292  	}
088da2a17619cf Peter Hurley       2016-01-16  293  
31cb9a8575ca04 Eugeniy Paltsev    2017-08-21  294  	val = of_get_flat_dt_prop(node, "current-speed", NULL);
31cb9a8575ca04 Eugeniy Paltsev    2017-08-21  295  	if (val)
31cb9a8575ca04 Eugeniy Paltsev    2017-08-21  296  		early_console_dev.baud = be32_to_cpu(*val);
31cb9a8575ca04 Eugeniy Paltsev    2017-08-21  297  
814453adea7d08 Michal Simek       2018-04-10  298  	val = of_get_flat_dt_prop(node, "clock-frequency", NULL);
814453adea7d08 Michal Simek       2018-04-10  299  	if (val)
814453adea7d08 Michal Simek       2018-04-10  300  		port->uartclk = be32_to_cpu(*val);
814453adea7d08 Michal Simek       2018-04-10  301  
4d118c9a866590 Peter Hurley       2016-01-16  302  	if (options) {
31cb9a8575ca04 Eugeniy Paltsev    2017-08-21  303  		early_console_dev.baud = simple_strtoul(options, NULL, 0);
4d118c9a866590 Peter Hurley       2016-01-16  304  		strlcpy(early_console_dev.options, options,
4d118c9a866590 Peter Hurley       2016-01-16  305  			sizeof(early_console_dev.options));
4d118c9a866590 Peter Hurley       2016-01-16  306  	}
05d961320ba624 Peter Hurley       2016-01-16  307  	earlycon_init(&early_console_dev, match->name);
4d118c9a866590 Peter Hurley       2016-01-16  308  	err = match->setup(&early_console_dev, options);
f28295cc8ce14b Hsin-Yi Wang       2020-09-15  309  	earlycon_print_info(&early_console_dev);
b0b6abd34c1b50 Rob Herring        2014-03-27  310  	if (err < 0)
b0b6abd34c1b50 Rob Herring        2014-03-27  311  		return err;
b0b6abd34c1b50 Rob Herring        2014-03-27  312  	if (!early_console_dev.con->write)
b0b6abd34c1b50 Rob Herring        2014-03-27  313  		return -ENODEV;
b0b6abd34c1b50 Rob Herring        2014-03-27  314  
b0b6abd34c1b50 Rob Herring        2014-03-27  315  
b0b6abd34c1b50 Rob Herring        2014-03-27  316  	register_console(early_console_dev.con);
b0b6abd34c1b50 Rob Herring        2014-03-27  317  	return 0;
b0b6abd34c1b50 Rob Herring        2014-03-27  318  }
8477614d9f7c5c Peter Hurley       2016-01-16  319  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28656 bytes --]

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

end of thread, other threads:[~2021-06-17  0:36 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 11:28 [PATCH 0/2] memory: tegra: Fixes for COMPILE_TEST Thierry Reding
2021-06-09 11:28 ` [PATCH 1/2] memory: tegra: Add missing dependencies Thierry Reding
2021-06-09 11:58   ` Dmitry Osipenko
2021-06-09 13:19     ` Krzysztof Kozlowski
2021-06-09 16:57       ` Dmitry Osipenko
2021-06-10  6:43         ` Krzysztof Kozlowski
2021-06-10 15:50           ` Dmitry Osipenko
2021-06-10 16:23             ` Dmitry Osipenko
2021-06-11  6:50               ` Krzysztof Kozlowski
2021-06-11  7:21                 ` Dmitry Osipenko
2021-06-11 11:00                   ` Thierry Reding
2021-06-11 13:40                     ` Dmitry Osipenko
2021-06-14 11:50                       ` Krzysztof Kozlowski
2021-06-14 14:16                         ` Dmitry Osipenko
2021-06-09 17:00       ` Thierry Reding
2021-06-10  6:42         ` Krzysztof Kozlowski
2021-06-17  0:35   ` kernel test robot
2021-06-09 11:28 ` [PATCH 2/2] reset: Add compile-test stubs Thierry Reding
2021-06-09 11:47   ` Philipp Zabel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).