diff mbox series

[v2] drivers: soc: atmel: fix the args passed to AT91_SOC for sam9x7 SoC

Message ID 20250220055302.284558-1-manikandan.m@microchip.com (mailing list archive)
State New
Headers show
Series [v2] drivers: soc: atmel: fix the args passed to AT91_SOC for sam9x7 SoC | expand

Commit Message

Manikandan Muralidharan Feb. 20, 2025, 5:53 a.m. UTC 
From: Dharma Balasubiramani <dharma.b@microchip.com>

Fix the arguments passed to the AT91_SOC for sam9x7 SoC
updating the SoC revision is skipped since the DBGU Chip ID Register
in sam9x7 SoC does not store the current version of the device.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
[manikandan.m@microchip.com: update CIDR Macros, skip updating the SoC
revision]
Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
---
changes in v2:
- update the version_mask to 0 for sam9x7 SoC to skip upating the SoC revision
- add AT91_CIDR_VALUE_MASK to mask bits 0 to 30 as per the datasheet
- update the SAM9X7_CIDR_MATCH macro
---
 drivers/soc/atmel/soc.c | 42 +++++++++++++++++++++++------------------
 drivers/soc/atmel/soc.h |  4 ++--
 2 files changed, 26 insertions(+), 20 deletions(-)

Comments

Claudiu Beznea Feb. 28, 2025, 9:29 a.m. UTC | #1 
Hi, Manikandan,

On 20.02.2025 07:53, Manikandan Muralidharan wrote:
> From: Dharma Balasubiramani <dharma.b@microchip.com>
> 
> Fix the arguments passed to the AT91_SOC for sam9x7 SoC
> updating the SoC revision is skipped since the DBGU Chip ID Register
> in sam9x7 SoC does not store the current version of the device.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> [manikandan.m@microchip.com: update CIDR Macros, skip updating the SoC
> revision]
> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
> ---
> changes in v2:
> - update the version_mask to 0 for sam9x7 SoC to skip upating the SoC revision
> - add AT91_CIDR_VALUE_MASK to mask bits 0 to 30 as per the datasheet
> - update the SAM9X7_CIDR_MATCH macro
> ---
>  drivers/soc/atmel/soc.c | 42 +++++++++++++++++++++++------------------
>  drivers/soc/atmel/soc.h |  4 ++--
>  2 files changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/soc/atmel/soc.c b/drivers/soc/atmel/soc.c
> index 298b542dd1c0..3e0b8ba4f8ba 100644
> --- a/drivers/soc/atmel/soc.c
> +++ b/drivers/soc/atmel/soc.c
> @@ -26,6 +26,7 @@
>  #define AT91_CIDR_VERSION_MASK_SAMA7G5	GENMASK(3, 0)
>  #define AT91_CIDR_EXT			BIT(31)
>  #define AT91_CIDR_MATCH_MASK		GENMASK(30, 5)
> +#define AT91_CIDR_VALUE_MASK		GENMASK(30, 0)

I would keep it as AT91_CIDR_MASK_SAM9X7 to match with the already existing
AT91_CIDR_MASK_SAMA7G5

>  #define AT91_CIDR_MASK_SAMA7G5		GENMASK(27, 5)
>  
>  static const struct at91_soc socs[] __initconst = {
> @@ -102,26 +103,26 @@ static const struct at91_soc socs[] __initconst = {
>  		 "sam9x60 8MiB SDRAM SiP", "sam9x60"),
>  #endif
>  #ifdef CONFIG_SOC_SAM9X7
> -	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
> -		 AT91_CIDR_VERSION_MASK, SAM9X70_EXID_MATCH,
> +	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
> +		 0, SAM9X70_EXID_MATCH,
>  		 "sam9x70", "sam9x7"),
> -	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
> -		 AT91_CIDR_VERSION_MASK, SAM9X72_EXID_MATCH,
> +	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
> +		 0, SAM9X72_EXID_MATCH,
>  		 "sam9x72", "sam9x7"),
> -	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
> -		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
> +	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
> +		 0, SAM9X75_EXID_MATCH,
>  		 "sam9x75", "sam9x7"),
> -	AT91_SOC(SAM9X7_CIDR_MATCH, SAM9X75_D1M_EXID_MATCH,
> -		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
> +	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
> +		 0, SAM9X75_D1M_EXID_MATCH,
>  		 "sam9x75 16MB DDR2 SiP", "sam9x7"),
> -	AT91_SOC(SAM9X7_CIDR_MATCH, SAM9X75_D5M_EXID_MATCH,
> -		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
> +	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
> +		 0, SAM9X75_D5M_EXID_MATCH,
>  		 "sam9x75 64MB DDR2 SiP", "sam9x7"),
> -	AT91_SOC(SAM9X7_CIDR_MATCH, SAM9X75_D1G_EXID_MATCH,
> -		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
> +	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
> +		 0, SAM9X75_D1G_EXID_MATCH,
>  		 "sam9x75 125MB DDR3L SiP ", "sam9x7"),
> -	AT91_SOC(SAM9X7_CIDR_MATCH, SAM9X75_D2G_EXID_MATCH,
> -		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
> +	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
> +		 0, SAM9X75_D2G_EXID_MATCH,
>  		 "sam9x75 250MB DDR3L SiP", "sam9x7"),
>  #endif
>  #ifdef CONFIG_SOC_SAMA5
> @@ -370,8 +371,10 @@ struct soc_device * __init at91_soc_init(const struct at91_soc *socs)
>  
>  	soc_dev_attr->family = soc->family;
>  	soc_dev_attr->soc_id = soc->name;
> -	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%X",
> -					   AT91_CIDR_VERSION(cidr, soc->version_mask));
> +	if (soc->version_mask)
> +		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%X",
> +						   AT91_CIDR_VERSION(cidr, soc->version_mask));
> +
>  	soc_dev = soc_device_register(soc_dev_attr);
>  	if (IS_ERR(soc_dev)) {
>  		kfree(soc_dev_attr->revision);
> @@ -382,8 +385,11 @@ struct soc_device * __init at91_soc_init(const struct at91_soc *socs)
>  
>  	if (soc->family)
>  		pr_info("Detected SoC family: %s\n", soc->family);
> -	pr_info("Detected SoC: %s, revision %X\n", soc->name,
> -		AT91_CIDR_VERSION(cidr, soc->version_mask));
> +	if (soc->version_mask)
> +		pr_info("Detected SoC: %s, revision %X\n", soc->name,
> +			AT91_CIDR_VERSION(cidr, soc->version_mask));
> +	else
> +		pr_info("Detected SoC: %s\n", soc->name);

I'll update it as while applying as follows. Please check and let me know
if all good with you:

@@ -386,8 +389,10 @@ struct soc_device * __init at91_soc_init(const struct
at91_soc *socs)

        if (soc->family)
                pr_info("Detected SoC family: %s\n", soc->family);
-       pr_info("Detected SoC: %s, revision %X\n", soc->name,
-               AT91_CIDR_VERSION(cidr, soc->version_mask));
+       pr_info("Detected SoC: %s", soc->name);
+       if (soc->version_mask)
+               pr_info(", revision %X", AT91_CIDR_VERSION(cidr,
soc->version_mask));
+       pr_info("\n");


Thank you,
Claudiu

>  
>  	return soc_dev;
>  }
> diff --git a/drivers/soc/atmel/soc.h b/drivers/soc/atmel/soc.h
> index 2c78e54255f7..2503a80856bc 100644
> --- a/drivers/soc/atmel/soc.h
> +++ b/drivers/soc/atmel/soc.h
> @@ -44,7 +44,7 @@ at91_soc_init(const struct at91_soc *socs);
>  #define AT91SAM9X5_CIDR_MATCH		0x019a05a0
>  #define AT91SAM9N12_CIDR_MATCH		0x019a07a0
>  #define SAM9X60_CIDR_MATCH		0x019b35a0
> -#define SAM9X7_CIDR_MATCH		0x09750020
> +#define SAM9X7_CIDR_MATCH		0x09750030
>  #define SAMA7G5_CIDR_MATCH		0x00162100
>  
>  #define AT91SAM9M11_EXID_MATCH		0x00000001
> @@ -69,11 +69,11 @@ at91_soc_init(const struct at91_soc *socs);
>  
>  #define SAM9X70_EXID_MATCH		0x00000005
>  #define SAM9X72_EXID_MATCH		0x00000004
> +#define SAM9X75_EXID_MATCH		0x00000000
>  #define SAM9X75_D1G_EXID_MATCH		0x00000018
>  #define SAM9X75_D2G_EXID_MATCH		0x00000020
>  #define SAM9X75_D1M_EXID_MATCH		0x00000003
>  #define SAM9X75_D5M_EXID_MATCH		0x00000010
> -#define SAM9X75_EXID_MATCH		0x00000000
>  
>  #define SAMA7G51_EXID_MATCH		0x3
>  #define SAMA7G52_EXID_MATCH		0x2
Claudiu Beznea Feb. 28, 2025, 10:45 a.m. UTC | #2 
On 28.02.2025 12:05, Manikandan.M@microchip.com wrote:
> Hi Claudiu,
> 
> Apologies,
> We've just received confirmation that the sam9x7 SoC does include an 
> version ID [3:0] in the DBGU Chip ID Register and this will be updated 
> soon in the datasheet.
> Therefore, we can continue using the AT91_CIDR_MATCH_MASK for SAM9X7. I 
> will send an updated patch with the new Version Mask Macro.

Thank you for letting me know!
Claudiu
diff mbox series

Patch

diff --git a/drivers/soc/atmel/soc.c b/drivers/soc/atmel/soc.c
index 298b542dd1c0..3e0b8ba4f8ba 100644
--- a/drivers/soc/atmel/soc.c
+++ b/drivers/soc/atmel/soc.c
@@ -26,6 +26,7 @@ 
 #define AT91_CIDR_VERSION_MASK_SAMA7G5	GENMASK(3, 0)
 #define AT91_CIDR_EXT			BIT(31)
 #define AT91_CIDR_MATCH_MASK		GENMASK(30, 5)
+#define AT91_CIDR_VALUE_MASK		GENMASK(30, 0)
 #define AT91_CIDR_MASK_SAMA7G5		GENMASK(27, 5)
 
 static const struct at91_soc socs[] __initconst = {
@@ -102,26 +103,26 @@  static const struct at91_soc socs[] __initconst = {
 		 "sam9x60 8MiB SDRAM SiP", "sam9x60"),
 #endif
 #ifdef CONFIG_SOC_SAM9X7
-	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
-		 AT91_CIDR_VERSION_MASK, SAM9X70_EXID_MATCH,
+	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
+		 0, SAM9X70_EXID_MATCH,
 		 "sam9x70", "sam9x7"),
-	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
-		 AT91_CIDR_VERSION_MASK, SAM9X72_EXID_MATCH,
+	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
+		 0, SAM9X72_EXID_MATCH,
 		 "sam9x72", "sam9x7"),
-	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
-		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
+	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
+		 0, SAM9X75_EXID_MATCH,
 		 "sam9x75", "sam9x7"),
-	AT91_SOC(SAM9X7_CIDR_MATCH, SAM9X75_D1M_EXID_MATCH,
-		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
+	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
+		 0, SAM9X75_D1M_EXID_MATCH,
 		 "sam9x75 16MB DDR2 SiP", "sam9x7"),
-	AT91_SOC(SAM9X7_CIDR_MATCH, SAM9X75_D5M_EXID_MATCH,
-		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
+	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
+		 0, SAM9X75_D5M_EXID_MATCH,
 		 "sam9x75 64MB DDR2 SiP", "sam9x7"),
-	AT91_SOC(SAM9X7_CIDR_MATCH, SAM9X75_D1G_EXID_MATCH,
-		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
+	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
+		 0, SAM9X75_D1G_EXID_MATCH,
 		 "sam9x75 125MB DDR3L SiP ", "sam9x7"),
-	AT91_SOC(SAM9X7_CIDR_MATCH, SAM9X75_D2G_EXID_MATCH,
-		 AT91_CIDR_VERSION_MASK, SAM9X75_EXID_MATCH,
+	AT91_SOC(SAM9X7_CIDR_MATCH, AT91_CIDR_VALUE_MASK,
+		 0, SAM9X75_D2G_EXID_MATCH,
 		 "sam9x75 250MB DDR3L SiP", "sam9x7"),
 #endif
 #ifdef CONFIG_SOC_SAMA5
@@ -370,8 +371,10 @@  struct soc_device * __init at91_soc_init(const struct at91_soc *socs)
 
 	soc_dev_attr->family = soc->family;
 	soc_dev_attr->soc_id = soc->name;
-	soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%X",
-					   AT91_CIDR_VERSION(cidr, soc->version_mask));
+	if (soc->version_mask)
+		soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%X",
+						   AT91_CIDR_VERSION(cidr, soc->version_mask));
+
 	soc_dev = soc_device_register(soc_dev_attr);
 	if (IS_ERR(soc_dev)) {
 		kfree(soc_dev_attr->revision);
@@ -382,8 +385,11 @@  struct soc_device * __init at91_soc_init(const struct at91_soc *socs)
 
 	if (soc->family)
 		pr_info("Detected SoC family: %s\n", soc->family);
-	pr_info("Detected SoC: %s, revision %X\n", soc->name,
-		AT91_CIDR_VERSION(cidr, soc->version_mask));
+	if (soc->version_mask)
+		pr_info("Detected SoC: %s, revision %X\n", soc->name,
+			AT91_CIDR_VERSION(cidr, soc->version_mask));
+	else
+		pr_info("Detected SoC: %s\n", soc->name);
 
 	return soc_dev;
 }
diff --git a/drivers/soc/atmel/soc.h b/drivers/soc/atmel/soc.h
index 2c78e54255f7..2503a80856bc 100644
--- a/drivers/soc/atmel/soc.h
+++ b/drivers/soc/atmel/soc.h
@@ -44,7 +44,7 @@  at91_soc_init(const struct at91_soc *socs);
 #define AT91SAM9X5_CIDR_MATCH		0x019a05a0
 #define AT91SAM9N12_CIDR_MATCH		0x019a07a0
 #define SAM9X60_CIDR_MATCH		0x019b35a0
-#define SAM9X7_CIDR_MATCH		0x09750020
+#define SAM9X7_CIDR_MATCH		0x09750030
 #define SAMA7G5_CIDR_MATCH		0x00162100
 
 #define AT91SAM9M11_EXID_MATCH		0x00000001
@@ -69,11 +69,11 @@  at91_soc_init(const struct at91_soc *socs);
 
 #define SAM9X70_EXID_MATCH		0x00000005
 #define SAM9X72_EXID_MATCH		0x00000004
+#define SAM9X75_EXID_MATCH		0x00000000
 #define SAM9X75_D1G_EXID_MATCH		0x00000018
 #define SAM9X75_D2G_EXID_MATCH		0x00000020
 #define SAM9X75_D1M_EXID_MATCH		0x00000003
 #define SAM9X75_D5M_EXID_MATCH		0x00000010
-#define SAM9X75_EXID_MATCH		0x00000000
 
 #define SAMA7G51_EXID_MATCH		0x3
 #define SAMA7G52_EXID_MATCH		0x2