* [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).