All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers
@ 2021-05-16 16:12 Dmitry Osipenko
  2021-05-16 16:12 ` [PATCH v2 1/4] soc/tegra: fuse: Add missing stubs Dmitry Osipenko
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:12 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Krzysztof Kozlowski,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

This series enables compile-testing for all of NVIDIA Tegra memory
drivers.

Changelog:

v2: - Added patch which should fix compilation warning of tegra124-emc driver
      on 64bit platforms that was reported by kernel build robot. Thanks
      to Nathan Chancellor for the suggested fix.

        memory: tegra124: Fix compilation warnings on 64bit platforms

    - Added missing stubs to the tegra-clk header in another new patch. This
      was also reported by kernel build robot for v1.

        clk: tegra: Add stubs needed for compile-testing

    - The memory/tegra/Kconfig now uses `if TEGRA_MC`, which was suggested
      by Krzysztof Kozlowski to v1. This makes Tegra Kconfig to look consistent
      with the Exynos Kconfig.

Dmitry Osipenko (4):
  soc/tegra: fuse: Add missing stubs
  clk: tegra: Add stubs needed for compile-testing
  memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  memory: tegra: Enable compile testing for all drivers

 drivers/memory/tegra/Kconfig        | 16 ++++++++++------
 drivers/memory/tegra/tegra124-emc.c |  4 ++--
 include/linux/clk/tegra.h           | 28 ++++++++++++++++++++++++----
 include/soc/tegra/fuse.h            | 20 +++++++++++++++++---
 4 files changed, 53 insertions(+), 15 deletions(-)

-- 
2.30.2


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

* [PATCH v2 1/4] soc/tegra: fuse: Add missing stubs
  2021-05-16 16:12 [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Dmitry Osipenko
@ 2021-05-16 16:12 ` Dmitry Osipenko
  2021-05-16 16:12 ` [PATCH v2 2/4] clk: tegra: Add stubs needed for compile-testing Dmitry Osipenko
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:12 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Krzysztof Kozlowski,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

Add missing stubs that will allow Tegra memory driver to be compile-tested
by kernel build bots.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 include/soc/tegra/fuse.h | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/soc/tegra/fuse.h b/include/soc/tegra/fuse.h
index 78cbc787a4dc..990701f788bc 100644
--- a/include/soc/tegra/fuse.h
+++ b/include/soc/tegra/fuse.h
@@ -52,14 +52,28 @@ struct tegra_sku_info {
 	enum tegra_revision revision;
 };
 
+#ifdef CONFIG_ARCH_TEGRA
+extern struct tegra_sku_info tegra_sku_info;
 u32 tegra_read_straps(void);
 u32 tegra_read_ram_code(void);
 int tegra_fuse_readl(unsigned long offset, u32 *value);
-
-#ifdef CONFIG_ARCH_TEGRA
-extern struct tegra_sku_info tegra_sku_info;
 #else
 static struct tegra_sku_info tegra_sku_info __maybe_unused;
+
+static inline u32 tegra_read_straps(void)
+{
+	return 0;
+}
+
+static inline u32 tegra_read_ram_code(void)
+{
+	return 0;
+}
+
+static inline int tegra_fuse_readl(unsigned long offset, u32 *value)
+{
+	return -ENODEV;
+}
 #endif
 
 struct device *tegra_soc_device_register(void);
-- 
2.30.2


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

* [PATCH v2 2/4] clk: tegra: Add stubs needed for compile-testing
  2021-05-16 16:12 [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Dmitry Osipenko
  2021-05-16 16:12 ` [PATCH v2 1/4] soc/tegra: fuse: Add missing stubs Dmitry Osipenko
@ 2021-05-16 16:12 ` Dmitry Osipenko
  2021-05-16 16:12 ` [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms Dmitry Osipenko
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:12 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Krzysztof Kozlowski,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

Add stubs needed for compile-testing of Tegra memory drivers.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 include/linux/clk/tegra.h | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h
index f7ff722a03dd..d540b2879c26 100644
--- a/include/linux/clk/tegra.h
+++ b/include/linux/clk/tegra.h
@@ -144,17 +144,37 @@ typedef long (tegra20_clk_emc_round_cb)(unsigned long rate,
 					unsigned long min_rate,
 					unsigned long max_rate,
 					void *arg);
+typedef int (tegra124_emc_prepare_timing_change_cb)(struct tegra_emc *emc,
+						    unsigned long rate);
+typedef void (tegra124_emc_complete_timing_change_cb)(struct tegra_emc *emc,
+						      unsigned long rate);
 
+#ifdef CONFIG_ARCH_TEGRA
 void tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
 					void *cb_arg);
 int tegra20_clk_prepare_emc_mc_same_freq(struct clk *emc_clk, bool same);
 
-typedef int (tegra124_emc_prepare_timing_change_cb)(struct tegra_emc *emc,
-						    unsigned long rate);
-typedef void (tegra124_emc_complete_timing_change_cb)(struct tegra_emc *emc,
-						      unsigned long rate);
 void tegra124_clk_set_emc_callbacks(tegra124_emc_prepare_timing_change_cb *prep_cb,
 				    tegra124_emc_complete_timing_change_cb *complete_cb);
+#else
+static inline void
+tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
+				   void *cb_arg)
+{
+}
+
+static inline int
+tegra20_clk_prepare_emc_mc_same_freq(struct clk *emc_clk, bool same)
+{
+	return 0;
+}
+
+static inline void
+tegra124_clk_set_emc_callbacks(tegra124_emc_prepare_timing_change_cb *prep_cb,
+			       tegra124_emc_complete_timing_change_cb *complete_cb)
+{
+}
+#endif
 
 struct tegra210_clk_emc_config {
 	unsigned long rate;
-- 
2.30.2


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

* [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-16 16:12 [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Dmitry Osipenko
  2021-05-16 16:12 ` [PATCH v2 1/4] soc/tegra: fuse: Add missing stubs Dmitry Osipenko
  2021-05-16 16:12 ` [PATCH v2 2/4] clk: tegra: Add stubs needed for compile-testing Dmitry Osipenko
@ 2021-05-16 16:12 ` Dmitry Osipenko
  2021-05-17  6:26   ` Nathan Chancellor
                     ` (2 more replies)
  2021-05-16 16:12 ` [PATCH v2 4/4] memory: tegra: Enable compile testing for all drivers Dmitry Osipenko
  2021-05-17 11:32 ` [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Krzysztof Kozlowski
  4 siblings, 3 replies; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:12 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Krzysztof Kozlowski,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

Fix compilation warning on 64bit platforms caused by implicit promotion
of 32bit signed integer to a 64bit unsigned value which happens after
enabling compile-testing of the driver.

Suggested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/memory/tegra/tegra124-emc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
index 5699d909abc2..c9eb948cf4df 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -272,8 +272,8 @@
 #define EMC_PUTERM_ADJ				0x574
 
 #define DRAM_DEV_SEL_ALL			0
-#define DRAM_DEV_SEL_0				(2 << 30)
-#define DRAM_DEV_SEL_1				(1 << 30)
+#define DRAM_DEV_SEL_0				(2u << 30)
+#define DRAM_DEV_SEL_1				(1u << 30)
 
 #define EMC_CFG_POWER_FEATURES_MASK		\
 	(EMC_CFG_DYN_SREF | EMC_CFG_DRAM_ACPD | EMC_CFG_DRAM_CLKSTOP_SR | \
-- 
2.30.2


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

* [PATCH v2 4/4] memory: tegra: Enable compile testing for all drivers
  2021-05-16 16:12 [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2021-05-16 16:12 ` [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms Dmitry Osipenko
@ 2021-05-16 16:12 ` Dmitry Osipenko
  2021-05-17 11:33   ` Krzysztof Kozlowski
  2021-05-17 11:32 ` [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Krzysztof Kozlowski
  4 siblings, 1 reply; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-16 16:12 UTC (permalink / raw)
  To: Thierry Reding, Jonathan Hunter, Krzysztof Kozlowski,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

Enable compile testing for all Tegra memory drivers.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/memory/tegra/Kconfig | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index a70967a56e52..c63ffa74ab94 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -2,16 +2,18 @@
 config TEGRA_MC
 	bool "NVIDIA Tegra Memory Controller support"
 	default y
-	depends on ARCH_TEGRA
+	depends on ARCH_TEGRA || COMPILE_TEST
 	select INTERCONNECT
 	help
 	  This driver supports the Memory Controller (MC) hardware found on
 	  NVIDIA Tegra SoCs.
 
+if TEGRA_MC
+
 config TEGRA20_EMC
 	tristate "NVIDIA Tegra20 External Memory Controller driver"
 	default y
-	depends on TEGRA_MC && ARCH_TEGRA_2x_SOC
+	depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
 	select DEVFREQ_GOV_SIMPLE_ONDEMAND
 	select PM_DEVFREQ
 	help
@@ -23,7 +25,7 @@ config TEGRA20_EMC
 config TEGRA30_EMC
 	tristate "NVIDIA Tegra30 External Memory Controller driver"
 	default y
-	depends on TEGRA_MC && ARCH_TEGRA_3x_SOC
+	depends on ARCH_TEGRA_3x_SOC || COMPILE_TEST
 	select PM_OPP
 	help
 	  This driver is for the External Memory Controller (EMC) found on
@@ -34,8 +36,8 @@ config TEGRA30_EMC
 config TEGRA124_EMC
 	tristate "NVIDIA Tegra124 External Memory Controller driver"
 	default y
-	depends on TEGRA_MC && ARCH_TEGRA_124_SOC
-	select TEGRA124_CLK_EMC
+	depends on ARCH_TEGRA_124_SOC || COMPILE_TEST
+	select TEGRA124_CLK_EMC if ARCH_TEGRA
 	select PM_OPP
 	help
 	  This driver is for the External Memory Controller (EMC) found on
@@ -49,10 +51,12 @@ config TEGRA210_EMC_TABLE
 
 config TEGRA210_EMC
 	tristate "NVIDIA Tegra210 External Memory Controller driver"
-	depends on TEGRA_MC && ARCH_TEGRA_210_SOC
+	depends on ARCH_TEGRA_210_SOC || COMPILE_TEST
 	select TEGRA210_EMC_TABLE
 	help
 	  This driver is for the External Memory Controller (EMC) found on
 	  Tegra210 chips. The EMC controls the external DRAM on the board.
 	  This driver is required to change memory timings / clock rate for
 	  external memory.
+
+endif
-- 
2.30.2


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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-16 16:12 ` [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms Dmitry Osipenko
@ 2021-05-17  6:26   ` Nathan Chancellor
  2021-05-17 11:28   ` Krzysztof Kozlowski
  2021-05-17 14:24   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 16+ messages in thread
From: Nathan Chancellor @ 2021-05-17  6:26 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter,
	Krzysztof Kozlowski, Stephen Boyd, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

On 5/16/2021 9:12 AM, Dmitry Osipenko wrote:
> Fix compilation warning on 64bit platforms caused by implicit promotion
> of 32bit signed integer to a 64bit unsigned value which happens after
> enabling compile-testing of the driver.
> 
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>   drivers/memory/tegra/tegra124-emc.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
> index 5699d909abc2..c9eb948cf4df 100644
> --- a/drivers/memory/tegra/tegra124-emc.c
> +++ b/drivers/memory/tegra/tegra124-emc.c
> @@ -272,8 +272,8 @@
>   #define EMC_PUTERM_ADJ				0x574
>   
>   #define DRAM_DEV_SEL_ALL			0
> -#define DRAM_DEV_SEL_0				(2 << 30)
> -#define DRAM_DEV_SEL_1				(1 << 30)
> +#define DRAM_DEV_SEL_0				(2u << 30)
> +#define DRAM_DEV_SEL_1				(1u << 30)
>   
>   #define EMC_CFG_POWER_FEATURES_MASK		\
>   	(EMC_CFG_DYN_SREF | EMC_CFG_DRAM_ACPD | EMC_CFG_DRAM_CLKSTOP_SR | \
> 


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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-16 16:12 ` [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms Dmitry Osipenko
  2021-05-17  6:26   ` Nathan Chancellor
@ 2021-05-17 11:28   ` Krzysztof Kozlowski
  2021-05-17 13:35     ` Dmitry Osipenko
  2021-05-17 14:24   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-17 11:28 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Stephen Boyd,
	Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

On 16/05/2021 12:12, Dmitry Osipenko wrote:
> Fix compilation warning on 64bit platforms caused by implicit promotion
> of 32bit signed integer to a 64bit unsigned value which happens after
> enabling compile-testing of the driver.
> 
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/memory/tegra/tegra124-emc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
> index 5699d909abc2..c9eb948cf4df 100644
> --- a/drivers/memory/tegra/tegra124-emc.c
> +++ b/drivers/memory/tegra/tegra124-emc.c
> @@ -272,8 +272,8 @@
>  #define EMC_PUTERM_ADJ				0x574
>  
>  #define DRAM_DEV_SEL_ALL			0
> -#define DRAM_DEV_SEL_0				(2 << 30)
> -#define DRAM_DEV_SEL_1				(1 << 30)
> +#define DRAM_DEV_SEL_0				(2u << 30)
> +#define DRAM_DEV_SEL_1				(1u << 30)

Why not using BIT()? This would make even this 2<<30 less awkard...

Best regards,
Krzysztof

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

* Re: [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers
  2021-05-16 16:12 [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2021-05-16 16:12 ` [PATCH v2 4/4] memory: tegra: Enable compile testing for all drivers Dmitry Osipenko
@ 2021-05-17 11:32 ` Krzysztof Kozlowski
  4 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-17 11:32 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Dmitry Osipenko, linux-kernel, Mikko Perttunen, linux-tegra,
	linux-clk, Jonathan Hunter, Stephen Boyd, Nathan Chancellor

On 16/05/2021 12:12, Dmitry Osipenko wrote:
> This series enables compile-testing for all of NVIDIA Tegra memory
> drivers.
> 
> Changelog:
> 
> v2: - Added patch which should fix compilation warning of tegra124-emc driver
>       on 64bit platforms that was reported by kernel build robot. Thanks
>       to Nathan Chancellor for the suggested fix.
> 
>         memory: tegra124: Fix compilation warnings on 64bit platforms
> 
>     - Added missing stubs to the tegra-clk header in another new patch. This
>       was also reported by kernel build robot for v1.
> 
>         clk: tegra: Add stubs needed for compile-testing
> 
>     - The memory/tegra/Kconfig now uses `if TEGRA_MC`, which was suggested
>       by Krzysztof Kozlowski to v1. This makes Tegra Kconfig to look consistent
>       with the Exynos Kconfig.
> 

Hi Thierry,

The memory drivers part depends on soc and clk patches. There is also
another series from Dmitry (devm_tegra_core_dev_init_opp_table()) with
memory-soc dependency.  I guess it makes sense you take everything via
soc-tegra, but just in case, can you keep the memory drivers on
dedicated branch?


Best regards,
Krzysztof

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

* Re: [PATCH v2 4/4] memory: tegra: Enable compile testing for all drivers
  2021-05-16 16:12 ` [PATCH v2 4/4] memory: tegra: Enable compile testing for all drivers Dmitry Osipenko
@ 2021-05-17 11:33   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-17 11:33 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Stephen Boyd,
	Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

On 16/05/2021 12:12, Dmitry Osipenko wrote:
> Enable compile testing for all Tegra memory drivers.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/memory/tegra/Kconfig | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
This depends on clk/soc changes, so I am fine to take it via other tree:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Best regards,
Krzysztof

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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-17 11:28   ` Krzysztof Kozlowski
@ 2021-05-17 13:35     ` Dmitry Osipenko
  2021-05-17 13:39       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-17 13:35 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

17.05.2021 14:28, Krzysztof Kozlowski пишет:
> On 16/05/2021 12:12, Dmitry Osipenko wrote:
>> Fix compilation warning on 64bit platforms caused by implicit promotion
>> of 32bit signed integer to a 64bit unsigned value which happens after
>> enabling compile-testing of the driver.
>>
>> Suggested-by: Nathan Chancellor <nathan@kernel.org>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/memory/tegra/tegra124-emc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
>> index 5699d909abc2..c9eb948cf4df 100644
>> --- a/drivers/memory/tegra/tegra124-emc.c
>> +++ b/drivers/memory/tegra/tegra124-emc.c
>> @@ -272,8 +272,8 @@
>>  #define EMC_PUTERM_ADJ				0x574
>>  
>>  #define DRAM_DEV_SEL_ALL			0
>> -#define DRAM_DEV_SEL_0				(2 << 30)
>> -#define DRAM_DEV_SEL_1				(1 << 30)
>> +#define DRAM_DEV_SEL_0				(2u << 30)
>> +#define DRAM_DEV_SEL_1				(1u << 30)
> 
> Why not using BIT()? This would make even this 2<<30 less awkard...

The bitfield 31:30 is a enum, 3 is a wrong value. Formally it's
incorrect to use the BIT() macro here.

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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-17 13:35     ` Dmitry Osipenko
@ 2021-05-17 13:39       ` Krzysztof Kozlowski
  2021-05-17 13:47         ` Dmitry Osipenko
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-17 13:39 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Stephen Boyd,
	Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

On 17/05/2021 09:35, Dmitry Osipenko wrote:
> 17.05.2021 14:28, Krzysztof Kozlowski пишет:
>> On 16/05/2021 12:12, Dmitry Osipenko wrote:
>>> Fix compilation warning on 64bit platforms caused by implicit promotion
>>> of 32bit signed integer to a 64bit unsigned value which happens after
>>> enabling compile-testing of the driver.
>>>
>>> Suggested-by: Nathan Chancellor <nathan@kernel.org>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>  drivers/memory/tegra/tegra124-emc.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
>>> index 5699d909abc2..c9eb948cf4df 100644
>>> --- a/drivers/memory/tegra/tegra124-emc.c
>>> +++ b/drivers/memory/tegra/tegra124-emc.c
>>> @@ -272,8 +272,8 @@
>>>  #define EMC_PUTERM_ADJ				0x574
>>>  
>>>  #define DRAM_DEV_SEL_ALL			0
>>> -#define DRAM_DEV_SEL_0				(2 << 30)
>>> -#define DRAM_DEV_SEL_1				(1 << 30)
>>> +#define DRAM_DEV_SEL_0				(2u << 30)
>>> +#define DRAM_DEV_SEL_1				(1u << 30)
>>
>> Why not using BIT()? This would make even this 2<<30 less awkard...
> 
> The bitfield 31:30 is a enum, 3 is a wrong value. Formally it's
> incorrect to use the BIT() macro here.

Why "3"? BIT(31) is the same as 2<<30. It's common to use BIT for
register fields which do not accept all possible values. Now you
basically reimplement BIT() which is error-prone.


Best regards,
Krzysztof

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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-17 13:39       ` Krzysztof Kozlowski
@ 2021-05-17 13:47         ` Dmitry Osipenko
  2021-05-17 14:04           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-17 13:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

17.05.2021 16:39, Krzysztof Kozlowski пишет:
> On 17/05/2021 09:35, Dmitry Osipenko wrote:
>> 17.05.2021 14:28, Krzysztof Kozlowski пишет:
>>> On 16/05/2021 12:12, Dmitry Osipenko wrote:
>>>> Fix compilation warning on 64bit platforms caused by implicit promotion
>>>> of 32bit signed integer to a 64bit unsigned value which happens after
>>>> enabling compile-testing of the driver.
>>>>
>>>> Suggested-by: Nathan Chancellor <nathan@kernel.org>
>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>> ---
>>>>  drivers/memory/tegra/tegra124-emc.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
>>>> index 5699d909abc2..c9eb948cf4df 100644
>>>> --- a/drivers/memory/tegra/tegra124-emc.c
>>>> +++ b/drivers/memory/tegra/tegra124-emc.c
>>>> @@ -272,8 +272,8 @@
>>>>  #define EMC_PUTERM_ADJ				0x574
>>>>  
>>>>  #define DRAM_DEV_SEL_ALL			0
>>>> -#define DRAM_DEV_SEL_0				(2 << 30)
>>>> -#define DRAM_DEV_SEL_1				(1 << 30)
>>>> +#define DRAM_DEV_SEL_0				(2u << 30)
>>>> +#define DRAM_DEV_SEL_1				(1u << 30)
>>>
>>> Why not using BIT()? This would make even this 2<<30 less awkard...
>>
>> The bitfield 31:30 is a enum, 3 is a wrong value. Formally it's
>> incorrect to use the BIT() macro here.
> 
> Why "3"? BIT(31) is the same as 2<<30.

By 3 I meant BIT(31)|BIT(30). This bitfield is explicitly designated as
a enum in the hardware documentation.

> It's common to use BIT for
> register fields which do not accept all possible values. Now you
> basically reimplement BIT() which is error-prone.

Could you please show couple examples? The common practice today is to
use FIELD_PREP helpers, but this driver was written before these helpers
existed.

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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-17 13:47         ` Dmitry Osipenko
@ 2021-05-17 14:04           ` Krzysztof Kozlowski
  2021-05-17 14:23             ` Dmitry Osipenko
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-17 14:04 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Stephen Boyd,
	Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

On 17/05/2021 09:47, Dmitry Osipenko wrote:
> 17.05.2021 16:39, Krzysztof Kozlowski пишет:
>>>>>  #define DRAM_DEV_SEL_ALL			0
>>>>> -#define DRAM_DEV_SEL_0				(2 << 30)
>>>>> -#define DRAM_DEV_SEL_1				(1 << 30)
>>>>> +#define DRAM_DEV_SEL_0				(2u << 30)
>>>>> +#define DRAM_DEV_SEL_1				(1u << 30)
>>>>
>>>> Why not using BIT()? This would make even this 2<<30 less awkard...
>>>
>>> The bitfield 31:30 is a enum, 3 is a wrong value. Formally it's
>>> incorrect to use the BIT() macro here.
>>
>> Why "3"? BIT(31) is the same as 2<<30.
> 
> By 3 I meant BIT(31)|BIT(30). This bitfield is explicitly designated as
> a enum in the hardware documentation.

I understand it and using BIT() here does not mean someone has to set
both of them. BIT() is a helper pointing out that you want to toggle one
bit. It does not mean that it is allowed to do so always!

> 
>> It's common to use BIT for
>> register fields which do not accept all possible values. Now you
>> basically reimplement BIT() which is error-prone.
> 
> Could you please show couple examples? The common practice today is to
> use FIELD_PREP helpers, but this driver was written before these helpers
> existed.


There are plenty of such examples so I guess it would be easier to ask
you to provide counter ones. Few IT for enum-like registers found within 2 minutes:

https://elixir.bootlin.com/linux/latest/C/ident/MAX77620_CNFG_GPIO_INT_MASK
https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/max77650-regulator.c#L18
https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/tps6524x-regulator.c#L62
https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/tps80031-regulator.c#L39
https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/da9121-regulator.h#L200
https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/da9121-regulator.h#L231

Best regards,
Krzysztof

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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-17 14:04           ` Krzysztof Kozlowski
@ 2021-05-17 14:23             ` Dmitry Osipenko
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-17 14:23 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

17.05.2021 17:04, Krzysztof Kozlowski пишет:
> On 17/05/2021 09:47, Dmitry Osipenko wrote:
>> 17.05.2021 16:39, Krzysztof Kozlowski пишет:
>>>>>>  #define DRAM_DEV_SEL_ALL			0
>>>>>> -#define DRAM_DEV_SEL_0				(2 << 30)
>>>>>> -#define DRAM_DEV_SEL_1				(1 << 30)
>>>>>> +#define DRAM_DEV_SEL_0				(2u << 30)
>>>>>> +#define DRAM_DEV_SEL_1				(1u << 30)
>>>>>
>>>>> Why not using BIT()? This would make even this 2<<30 less awkard...
>>>>
>>>> The bitfield 31:30 is a enum, 3 is a wrong value. Formally it's
>>>> incorrect to use the BIT() macro here.
>>>
>>> Why "3"? BIT(31) is the same as 2<<30.
>>
>> By 3 I meant BIT(31)|BIT(30). This bitfield is explicitly designated as
>> a enum in the hardware documentation.
> 
> I understand it and using BIT() here does not mean someone has to set
> both of them. BIT() is a helper pointing out that you want to toggle one
> bit. It does not mean that it is allowed to do so always!
> 
>>
>>> It's common to use BIT for
>>> register fields which do not accept all possible values. Now you
>>> basically reimplement BIT() which is error-prone.
>>
>> Could you please show couple examples? The common practice today is to
>> use FIELD_PREP helpers, but this driver was written before these helpers
>> existed.
> 
> 
> There are plenty of such examples so I guess it would be easier to ask
> you to provide counter ones. Few IT for enum-like registers found within 2 minutes:
> 
> https://elixir.bootlin.com/linux/latest/C/ident/MAX77620_CNFG_GPIO_INT_MASK
> https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/max77650-regulator.c#L18
> https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/tps6524x-regulator.c#L62
> https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/tps80031-regulator.c#L39
> https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/da9121-regulator.h#L200
> https://elixir.bootlin.com/linux/v5.13-rc2/source/drivers/regulator/da9121-regulator.h#L231

Alright, I'll use the BIT macro in the v3.

I also realized now that the tegra30-emc drivers needs the same change.

Thank you for the review.

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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-16 16:12 ` [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms Dmitry Osipenko
  2021-05-17  6:26   ` Nathan Chancellor
  2021-05-17 11:28   ` Krzysztof Kozlowski
@ 2021-05-17 14:24   ` Krzysztof Kozlowski
  2021-05-17 14:53     ` Dmitry Osipenko
  2 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2021-05-17 14:24 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Stephen Boyd,
	Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

On 16/05/2021 12:12, Dmitry Osipenko wrote:
> Fix compilation warning on 64bit platforms caused by implicit promotion
> of 32bit signed integer to a 64bit unsigned value which happens after
> enabling compile-testing of the driver.
> 
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
The patch was not suggested by Nathan but it was:
Reported-by: kernel test robot <lkp@intel.com>

Nathan however provided analysis and proper solution, so co-developed or
his SoB fits better. This is not that important as comment above -
including robot's credits.

Best regards,
Krzysztof

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

* Re: [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms
  2021-05-17 14:24   ` Krzysztof Kozlowski
@ 2021-05-17 14:53     ` Dmitry Osipenko
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Osipenko @ 2021-05-17 14:53 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Thierry Reding, Jonathan Hunter,
	Stephen Boyd, Nathan Chancellor, Mikko Perttunen
  Cc: linux-kernel, linux-tegra, linux-clk

17.05.2021 17:24, Krzysztof Kozlowski пишет:
> On 16/05/2021 12:12, Dmitry Osipenko wrote:
>> Fix compilation warning on 64bit platforms caused by implicit promotion
>> of 32bit signed integer to a 64bit unsigned value which happens after
>> enabling compile-testing of the driver.
>>
>> Suggested-by: Nathan Chancellor <nathan@kernel.org>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> The patch was not suggested by Nathan but it was:
> Reported-by: kernel test robot <lkp@intel.com>
> 
> Nathan however provided analysis and proper solution, so co-developed or
> his SoB fits better. This is not that important as comment above -
> including robot's credits.

I'll update the tags in v3, thank you.

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

end of thread, other threads:[~2021-05-17 16:24 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-16 16:12 [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Dmitry Osipenko
2021-05-16 16:12 ` [PATCH v2 1/4] soc/tegra: fuse: Add missing stubs Dmitry Osipenko
2021-05-16 16:12 ` [PATCH v2 2/4] clk: tegra: Add stubs needed for compile-testing Dmitry Osipenko
2021-05-16 16:12 ` [PATCH v2 3/4] memory: tegra124-emc: Fix compilation warnings on 64bit platforms Dmitry Osipenko
2021-05-17  6:26   ` Nathan Chancellor
2021-05-17 11:28   ` Krzysztof Kozlowski
2021-05-17 13:35     ` Dmitry Osipenko
2021-05-17 13:39       ` Krzysztof Kozlowski
2021-05-17 13:47         ` Dmitry Osipenko
2021-05-17 14:04           ` Krzysztof Kozlowski
2021-05-17 14:23             ` Dmitry Osipenko
2021-05-17 14:24   ` Krzysztof Kozlowski
2021-05-17 14:53     ` Dmitry Osipenko
2021-05-16 16:12 ` [PATCH v2 4/4] memory: tegra: Enable compile testing for all drivers Dmitry Osipenko
2021-05-17 11:33   ` Krzysztof Kozlowski
2021-05-17 11:32 ` [PATCH v2 0/4] Enable compile-testing of Tegra memory drivers Krzysztof Kozlowski

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