diff mbox series

[RFC,2/8] soc: samsung: Exynos chipid driver update

Message ID 20190404171735.12815-3-s.nawrocki@samsung.com (mailing list archive)
State RFC
Headers show
Series [RFC,1/8] soc: samsung: Add exynos chipid driver support | expand

Commit Message

Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics April 4, 2019, 5:17 p.m. UTC 
This patch adds definition of selected CHIP ID register offsets
and register bit field definitions for Exynos5422 SoC.

exynos_chipid_read() helper function is added to allow reading
the CHIP ID block registers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/soc/samsung/exynos-chipid.c | 16 +++++-----
 drivers/soc/samsung/exynos-chipid.h | 48 +++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 7 deletions(-)
 create mode 100644 drivers/soc/samsung/exynos-chipid.h

Comments

Krzysztof Kozlowski April 5, 2019, 9:22 a.m. UTC | #1 
On Thu, 4 Apr 2019 at 19:22, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> This patch adds definition of selected CHIP ID register offsets
> and register bit field definitions for Exynos5422 SoC.
>
> exynos_chipid_read() helper function is added to allow reading
> the CHIP ID block registers.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/soc/samsung/exynos-chipid.c | 16 +++++-----
>  drivers/soc/samsung/exynos-chipid.h | 48 +++++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/soc/samsung/exynos-chipid.h
>
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> index 5cb018807817..4920f0ef2c55 100644
> --- a/drivers/soc/samsung/exynos-chipid.c
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -16,10 +16,7 @@
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
>
> -#define EXYNOS_SUBREV_MASK     (0xF << 4)
> -#define EXYNOS_MAINREV_MASK    (0xF << 0)
> -#define EXYNOS_REV_MASK                (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
> -#define EXYNOS_MASK            0xFFFFF000

Create hex-addresses in first commit in lower case.

> +#include "exynos-chipid.h"
>
>  static const struct exynos_soc_id {
>         const char *name;
> @@ -40,6 +37,13 @@ static const struct exynos_soc_id {
>         { "EXYNOS5433", 0xE5433000 },
>  };
>
> +static void __iomem *exynos_chipid_base;

No, this was removed in Pankaj's version v6 -> v7 and I do not want it
to be file-scope. Having it file-scope is error prone and prevents
having multiple instances (I do not expect having more than one but
still code should be rather nicely generic).

> +
> +unsigned int exynos_chipid_read(unsigned int offset)
> +{
> +       return readl_relaxed(exynos_chipid_base + offset);
> +}
> +
>  static const char * __init product_id_to_soc_id(unsigned int product_id)
>  {
>         int i;
> @@ -53,7 +57,6 @@ static const char * __init product_id_to_soc_id(unsigned int product_id)
>  int __init exynos_chipid_early_init(void)
>  {
>         struct soc_device_attribute *soc_dev_attr;
> -       void __iomem *exynos_chipid_base;
>         struct soc_device *soc_dev;
>         struct device_node *root;
>         struct device_node *np;
> @@ -73,9 +76,8 @@ int __init exynos_chipid_early_init(void)
>                 return -ENXIO;
>         }
>
> -       product_id = readl_relaxed(exynos_chipid_base);
> +       product_id = exynos_chipid_read(EXYNOS_CHIPID_REG_PRO_ID);
>         revision = product_id & EXYNOS_REV_MASK;
> -       iounmap(exynos_chipid_base);
>
>         soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
>         if (!soc_dev_attr)
> diff --git a/drivers/soc/samsung/exynos-chipid.h b/drivers/soc/samsung/exynos-chipid.h
> new file mode 100644
> index 000000000000..826a12c25fa2
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.h
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Samsung Electronics Co., Ltd.
> + *           http://www.samsung.com/
> + *
> + * EXYNOS - CHIP ID support
> + */
> +
> +#define EXYNOS_CHIPID_REG_PRO_ID       0x00
> + #define EXYNOS_SUBREV_MASK            (0xf << 4)
> + #define EXYNOS_MAINREV_MASK           (0xf << 0)
> + #define EXYNOS_REV_MASK               (EXYNOS_SUBREV_MASK | \
> +                                        EXYNOS_MAINREV_MASK)
> + #define EXYNOS_MASK                   0xfffff000
> +
> +#define EXYNOS_CHIPID_REG_PKG_ID       0x04

Can you comment that these are fields from this register?
The same in second one.

Best regards,
Krzysztof

> + #define EXYNOS5422_IDS_OFFSET         24
> + #define EXYNOS5422_IDS_MASK           0xff
> + #define EXYNOS5422_USESG_OFFSET       3
> + #define EXYNOS5422_USESG_MASK         0x01
> + #define EXYNOS5422_SG_OFFSET          0
> + #define EXYNOS5422_SG_MASK            0x07
> + #define EXYNOS5422_TABLE_OFFSET       8
> + #define EXYNOS5422_TABLE_MASK         0x03
> + #define EXYNOS5422_SG_A_OFFSET                17
> + #define EXYNOS5422_SG_A_MASK          0x0f
> + #define EXYNOS5422_SG_B_OFFSET                21
> + #define EXYNOS5422_SG_B_MASK          0x03
> + #define EXYNOS5422_SG_BSIGN_OFFSET    23
> + #define EXYNOS5422_SG_BSIGN_MASK      0x01
> + #define EXYNOS5422_BIN2_OFFSET                12
> + #define EXYNOS5422_BIN2_MASK          0x01
> +
> +#define EXYNOS_CHIPID_REG_LOT_ID       0x14
> +
> +#define EXYNOS_CHIPID_AUX_INFO         0x1c
> + #define EXYNOS5422_TMCB_OFFSET                0
> + #define EXYNOS5422_TMCB_MASK          0x7f
> + #define EXYNOS5422_ARM_UP_OFFSET      8
> + #define EXYNOS5422_ARM_UP_MASK                0x03
> + #define EXYNOS5422_ARM_DN_OFFSET      10
> + #define EXYNOS5422_ARM_DN_MASK                0x03
> + #define EXYNOS5422_KFC_UP_OFFSET      12
> + #define EXYNOS5422_KFC_UP_MASK                0x03
> + #define EXYNOS5422_KFC_DN_OFFSET      14
> + #define EXYNOS5422_KFC_DN_MASK                0x03
> +
> +unsigned int exynos_chipid_read(unsigned int offset);
> --
> 2.17.1
>
Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics April 5, 2019, 10:04 a.m. UTC | #2 
On 4/5/19 11:22, Krzysztof Kozlowski wrote:
> On Thu, 4 Apr 2019 at 19:22, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>
>> This patch adds definition of selected CHIP ID register offsets
>> and register bit field definitions for Exynos5422 SoC.
>>
>> exynos_chipid_read() helper function is added to allow reading
>> the CHIP ID block registers.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>>  drivers/soc/samsung/exynos-chipid.c | 16 +++++-----
>>  drivers/soc/samsung/exynos-chipid.h | 48 +++++++++++++++++++++++++++++
>>  2 files changed, 57 insertions(+), 7 deletions(-)
>>  create mode 100644 drivers/soc/samsung/exynos-chipid.h
>>
>> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
>> index 5cb018807817..4920f0ef2c55 100644
>> --- a/drivers/soc/samsung/exynos-chipid.c
>> +++ b/drivers/soc/samsung/exynos-chipid.c
>> @@ -16,10 +16,7 @@

>> -#define EXYNOS_SUBREV_MASK     (0xF << 4)
>> -#define EXYNOS_MAINREV_MASK    (0xF << 0)
>> -#define EXYNOS_REV_MASK                (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
>> -#define EXYNOS_MASK            0xFFFFF000
> 
> Create hex-addresses in first commit in lower case.

>> +static void __iomem *exynos_chipid_base;
> 
> No, this was removed in Pankaj's version v6 -> v7 and I do not want it
> to be file-scope. Having it file-scope is error prone and prevents
> having multiple instances (I do not expect having more than one but
> still code should be rather nicely generic).

I'm going to switch to using regmap so that will not be needed any 
more, together with exynos_chipid_read() helper. Then the header
file would be moved to include/linux/soc/samsung.
 
>> +unsigned int exynos_chipid_read(unsigned int offset)
>> +{
>> +       return readl_relaxed(exynos_chipid_base + offset);
>> +}

>> diff --git a/drivers/soc/samsung/exynos-chipid.h b/drivers/soc/samsung/exynos-chipid.h
>> new file mode 100644
>> index 000000000000..826a12c25fa2
>> --- /dev/null
>> +++ b/drivers/soc/samsung/exynos-chipid.h
>> @@ -0,0 +1,48 @@

>> +#define EXYNOS_CHIPID_REG_PRO_ID       0x00
>> + #define EXYNOS_SUBREV_MASK            (0xf << 4)
>> + #define EXYNOS_MAINREV_MASK           (0xf << 0)
>> + #define EXYNOS_REV_MASK               (EXYNOS_SUBREV_MASK | \
>> +                                        EXYNOS_MAINREV_MASK)
>> + #define EXYNOS_MASK                   0xfffff000
>> +
>> +#define EXYNOS_CHIPID_REG_PKG_ID       0x04
> 
> Can you comment that these are fields from this register?
> The same in second one.

OK, I will add some comment here.
 
>> + #define EXYNOS5422_IDS_OFFSET         24
>> + #define EXYNOS5422_IDS_MASK           0xff
diff mbox series

Patch

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index 5cb018807817..4920f0ef2c55 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -16,10 +16,7 @@ 
 #include <linux/slab.h>
 #include <linux/sys_soc.h>
 
-#define EXYNOS_SUBREV_MASK	(0xF << 4)
-#define EXYNOS_MAINREV_MASK	(0xF << 0)
-#define EXYNOS_REV_MASK		(EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
-#define EXYNOS_MASK		0xFFFFF000
+#include "exynos-chipid.h"
 
 static const struct exynos_soc_id {
 	const char *name;
@@ -40,6 +37,13 @@  static const struct exynos_soc_id {
 	{ "EXYNOS5433", 0xE5433000 },
 };
 
+static void __iomem *exynos_chipid_base;
+
+unsigned int exynos_chipid_read(unsigned int offset)
+{
+	return readl_relaxed(exynos_chipid_base + offset);
+}
+
 static const char * __init product_id_to_soc_id(unsigned int product_id)
 {
 	int i;
@@ -53,7 +57,6 @@  static const char * __init product_id_to_soc_id(unsigned int product_id)
 int __init exynos_chipid_early_init(void)
 {
 	struct soc_device_attribute *soc_dev_attr;
-	void __iomem *exynos_chipid_base;
 	struct soc_device *soc_dev;
 	struct device_node *root;
 	struct device_node *np;
@@ -73,9 +76,8 @@  int __init exynos_chipid_early_init(void)
 		return -ENXIO;
 	}
 
-	product_id = readl_relaxed(exynos_chipid_base);
+	product_id = exynos_chipid_read(EXYNOS_CHIPID_REG_PRO_ID);
 	revision = product_id & EXYNOS_REV_MASK;
-	iounmap(exynos_chipid_base);
 
 	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
 	if (!soc_dev_attr)
diff --git a/drivers/soc/samsung/exynos-chipid.h b/drivers/soc/samsung/exynos-chipid.h
new file mode 100644
index 000000000000..826a12c25fa2
--- /dev/null
+++ b/drivers/soc/samsung/exynos-chipid.h
@@ -0,0 +1,48 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Samsung Electronics Co., Ltd.
+ *	      http://www.samsung.com/
+ *
+ * EXYNOS - CHIP ID support
+ */
+
+#define EXYNOS_CHIPID_REG_PRO_ID	0x00
+ #define EXYNOS_SUBREV_MASK		(0xf << 4)
+ #define EXYNOS_MAINREV_MASK		(0xf << 0)
+ #define EXYNOS_REV_MASK		(EXYNOS_SUBREV_MASK | \
+					 EXYNOS_MAINREV_MASK)
+ #define EXYNOS_MASK			0xfffff000
+
+#define EXYNOS_CHIPID_REG_PKG_ID	0x04
+ #define EXYNOS5422_IDS_OFFSET		24
+ #define EXYNOS5422_IDS_MASK		0xff
+ #define EXYNOS5422_USESG_OFFSET	3
+ #define EXYNOS5422_USESG_MASK		0x01
+ #define EXYNOS5422_SG_OFFSET		0
+ #define EXYNOS5422_SG_MASK		0x07
+ #define EXYNOS5422_TABLE_OFFSET	8
+ #define EXYNOS5422_TABLE_MASK		0x03
+ #define EXYNOS5422_SG_A_OFFSET		17
+ #define EXYNOS5422_SG_A_MASK		0x0f
+ #define EXYNOS5422_SG_B_OFFSET		21
+ #define EXYNOS5422_SG_B_MASK		0x03
+ #define EXYNOS5422_SG_BSIGN_OFFSET	23
+ #define EXYNOS5422_SG_BSIGN_MASK	0x01
+ #define EXYNOS5422_BIN2_OFFSET		12
+ #define EXYNOS5422_BIN2_MASK		0x01
+
+#define EXYNOS_CHIPID_REG_LOT_ID	0x14
+
+#define EXYNOS_CHIPID_AUX_INFO		0x1c
+ #define EXYNOS5422_TMCB_OFFSET		0
+ #define EXYNOS5422_TMCB_MASK		0x7f
+ #define EXYNOS5422_ARM_UP_OFFSET	8
+ #define EXYNOS5422_ARM_UP_MASK		0x03
+ #define EXYNOS5422_ARM_DN_OFFSET	10
+ #define EXYNOS5422_ARM_DN_MASK		0x03
+ #define EXYNOS5422_KFC_UP_OFFSET	12
+ #define EXYNOS5422_KFC_UP_MASK		0x03
+ #define EXYNOS5422_KFC_DN_OFFSET	14
+ #define EXYNOS5422_KFC_DN_MASK		0x03
+
+unsigned int exynos_chipid_read(unsigned int offset);