[v2,1/3] soc/tegra: fuse: Cache values of straps and Chip ID registers
diff mbox series

Message ID 20191111212637.22648-2-digetx@gmail.com
State New
Headers show
Series
  • Minor cleanup of tegra-apbmisc.c
Related show

Commit Message

Dmitry Osipenko Nov. 11, 2019, 9:26 p.m. UTC
There is no need to re-read Chip ID and HW straps out from hardware each
time, it is a bit nicer to cache the values in memory.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/soc/tegra/fuse/tegra-apbmisc.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Comments

Michał Mirosław Nov. 12, 2019, 4:44 a.m. UTC | #1
On Tue, Nov 12, 2019 at 12:26:35AM +0300, Dmitry Osipenko wrote:
> There is no need to re-read Chip ID and HW straps out from hardware each
> time, it is a bit nicer to cache the values in memory.
[...]
> @@ -103,6 +97,7 @@ void __init tegra_init_revision(void)
>  
>  void __init tegra_init_apbmisc(void)
>  {
> +	void __iomem *apbmisc_base, *strapping_base;
>  	struct resource apbmisc, straps;
>  	struct device_node *np;
>  
> @@ -162,10 +157,14 @@ void __init tegra_init_apbmisc(void)
>  	apbmisc_base = ioremap_nocache(apbmisc.start, resource_size(&apbmisc));
>  	if (!apbmisc_base)
>  		pr_err("failed to map APBMISC registers\n");
> +	else
> +		chipid = readl_relaxed(apbmisc_base + 4);
>  
>  	strapping_base = ioremap_nocache(straps.start, resource_size(&straps));
>  	if (!strapping_base)
>  		pr_err("failed to map strapping options registers\n");
> +	else
> +		strapping = readl_relaxed(strapping_base);
>  
>  	long_ram_code = of_property_read_bool(np, "nvidia,long-ram-code");
>  }

Since this no longer uses the mappings after init, you could iounmap()
them here.

Best Regards,
Michał Mirosław
Dmitry Osipenko Nov. 12, 2019, 1:34 p.m. UTC | #2
12.11.2019 07:44, Michał Mirosław пишет:
> On Tue, Nov 12, 2019 at 12:26:35AM +0300, Dmitry Osipenko wrote:
>> There is no need to re-read Chip ID and HW straps out from hardware each
>> time, it is a bit nicer to cache the values in memory.
> [...]
>> @@ -103,6 +97,7 @@ void __init tegra_init_revision(void)
>>  
>>  void __init tegra_init_apbmisc(void)
>>  {
>> +	void __iomem *apbmisc_base, *strapping_base;
>>  	struct resource apbmisc, straps;
>>  	struct device_node *np;
>>  
>> @@ -162,10 +157,14 @@ void __init tegra_init_apbmisc(void)
>>  	apbmisc_base = ioremap_nocache(apbmisc.start, resource_size(&apbmisc));
>>  	if (!apbmisc_base)
>>  		pr_err("failed to map APBMISC registers\n");
>> +	else
>> +		chipid = readl_relaxed(apbmisc_base + 4);
>>  
>>  	strapping_base = ioremap_nocache(straps.start, resource_size(&straps));
>>  	if (!strapping_base)
>>  		pr_err("failed to map strapping options registers\n");
>> +	else
>> +		strapping = readl_relaxed(strapping_base);
>>  
>>  	long_ram_code = of_property_read_bool(np, "nvidia,long-ram-code");
>>  }
> 
> Since this no longer uses the mappings after init, you could iounmap()
> them here.

Yes, it could be done. Although, that won't do much on ARM32 because APB
registers are statically mapped in arch/arm/mach-tegra/io.c.

Anyways, it should be good to have regs unmapped just for consistency.
Thank you for the suggestion, I'll add a patch for the unmapping.

Patch
diff mbox series

diff --git a/drivers/soc/tegra/fuse/tegra-apbmisc.c b/drivers/soc/tegra/fuse/tegra-apbmisc.c
index df76778af601..54aeea1b4500 100644
--- a/drivers/soc/tegra/fuse/tegra-apbmisc.c
+++ b/drivers/soc/tegra/fuse/tegra-apbmisc.c
@@ -21,18 +21,15 @@ 
 #define PMC_STRAPPING_OPT_A_RAM_CODE_MASK_SHORT	\
 	(0x3 << PMC_STRAPPING_OPT_A_RAM_CODE_SHIFT)
 
-static void __iomem *apbmisc_base;
-static void __iomem *strapping_base;
 static bool long_ram_code;
+static u32 strapping;
+static u32 chipid;
 
 u32 tegra_read_chipid(void)
 {
-	if (!apbmisc_base) {
-		WARN(1, "Tegra Chip ID not yet available\n");
-		return 0;
-	}
+	WARN(!chipid, "Tegra Chip ID not yet available\n");
 
-	return readl_relaxed(apbmisc_base + 4);
+	return chipid;
 }
 
 u8 tegra_get_chip_id(void)
@@ -42,10 +39,7 @@  u8 tegra_get_chip_id(void)
 
 u32 tegra_read_straps(void)
 {
-	if (strapping_base)
-		return readl_relaxed(strapping_base);
-	else
-		return 0;
+	return strapping;
 }
 
 u32 tegra_read_ram_code(void)
@@ -103,6 +97,7 @@  void __init tegra_init_revision(void)
 
 void __init tegra_init_apbmisc(void)
 {
+	void __iomem *apbmisc_base, *strapping_base;
 	struct resource apbmisc, straps;
 	struct device_node *np;
 
@@ -162,10 +157,14 @@  void __init tegra_init_apbmisc(void)
 	apbmisc_base = ioremap_nocache(apbmisc.start, resource_size(&apbmisc));
 	if (!apbmisc_base)
 		pr_err("failed to map APBMISC registers\n");
+	else
+		chipid = readl_relaxed(apbmisc_base + 4);
 
 	strapping_base = ioremap_nocache(straps.start, resource_size(&straps));
 	if (!strapping_base)
 		pr_err("failed to map strapping options registers\n");
+	else
+		strapping = readl_relaxed(strapping_base);
 
 	long_ram_code = of_property_read_bool(np, "nvidia,long-ram-code");
 }