[PATCH 1/4] dt-bindings: hwinfo: samsung,exynos-chipid: add gs101-chipid compatible

[PATCH 1/4] dt-bindings: hwinfo: samsung,exynos-chipid: add gs101-chipid compatible

[Alexey Klimov]

Add "google,gs101-chipid" compatible string to binding document.

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 .../devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml        | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml b/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
index 780ccb5ee9b4..b1d933808b6c 100644
--- a/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
+++ b/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
@@ -13,6 +13,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - google,gs101-chipid
           - samsung,exynos4210-chipid
           - samsung,exynos850-chipid
       - items:
-- 
2.43.0

[PATCH 2/4] arm64: dts: exynos: gs101: add chipid node

[Alexey Klimov]

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 arch/arm64/boot/dts/exynos/google/gs101.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
index d838e3a7af6e..156fec2575bc 100644
--- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
+++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
@@ -283,6 +283,11 @@ soc: soc@0 {
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x0 0x40000000>;
 
+		chipid@10000000 {
+			compatible = "google,gs101-chipid";
+			reg = <0x10000000 0xd000>;
+		};
+
 		cmu_misc: clock-controller@10010000 {
 			compatible = "google,gs101-cmu-misc";
 			reg = <0x10010000 0x8000>;
-- 
2.43.0

[PATCH 3/4] soc: samsung: exynos-chipid: add Google Tensor gs101 SoC support

[Alexey Klimov]

Add GS101 information to soc_ids table and related entries to other
places. This SoC product id is "0x09845000".

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index b1118d37779e..7fee6094db12 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -60,6 +60,8 @@ static const struct exynos_soc_id {
 	{ "EXYNOS850", 0xE3830000 },
 	{ "EXYNOSAUTOV9", 0xAAA80000 },
 	{ "EXYNOSAUTOV920", 0x0A920000 },
+	/* Compatible with: google,gs101-chipid */
+	{ "GS101", 0x09845000 },
 };
 
 static const char *product_id_to_soc_id(unsigned int product_id)
@@ -178,8 +180,17 @@ static const struct exynos_chipid_variant exynos850_chipid_drv_data = {
 	.sub_rev_shift	= 16,
 };
 
+static const struct exynos_chipid_variant gs101_chipid_drv_data = {
+	.rev_reg	= 0x10,
+	.main_rev_shift	= 0,
+	.sub_rev_shift	= 16,
+};
+
 static const struct of_device_id exynos_chipid_of_device_ids[] = {
 	{
+		.compatible	= "google,gs101-chipid",
+		.data		= &gs101_chipid_drv_data,
+	}, {
 		.compatible	= "samsung,exynos4210-chipid",
 		.data		= &exynos4210_chipid_drv_data,
 	}, {
-- 
2.43.0

[PATCH 4/4] soc: samsung: exynos-chipid: fix revision calculation for gs101

[Alexey Klimov]

The main revision for gs101 SoC is not reported in the CHIPID_REV
register. The gs101 Product ID and revisions registers have a behaviour
split between old Exynos SoCs and new SoCs. The sub-revision is
reported in CHIPID_REV register in [19:16] bits but main revision
is still present in Product ID [7:0].

To construct soc_info->revision correctly for gs101 the main_rev
should not be reset from a value read from CHIPID_REV.

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 drivers/soc/samsung/exynos-chipid.c       | 20 ++++++++++++++++----
 include/linux/soc/samsung/exynos-chipid.h |  1 +
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index 7fee6094db12..3b952ffd8cf7 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -87,14 +87,26 @@ static int exynos_chipid_get_chipid_info(struct regmap *regmap,
 	soc_info->product_id = val & EXYNOS_MASK;
 
 	if (data->rev_reg != EXYNOS_CHIPID_REG_PRO_ID) {
-		ret = regmap_read(regmap, data->rev_reg, &val);
+		unsigned int val2;
+
+		ret = regmap_read(regmap, data->rev_reg, &val2);
 		if (ret < 0)
 			return ret;
+
+		if (data->main_rev_shift == 0)
+			main_rev = (val >> data->main_rev_shift)
+				   & EXYNOS_REV_PART_MASK_GS101;
+		else
+			main_rev = (val2 >> data->main_rev_shift)
+				   & EXYNOS_REV_PART_MASK;
+
+		sub_rev = (val2 >> data->sub_rev_shift) & EXYNOS_REV_PART_MASK;
+	} else {
+		main_rev = (val >> data->main_rev_shift) & EXYNOS_REV_PART_MASK;
+		sub_rev = (val >> data->sub_rev_shift) & EXYNOS_REV_PART_MASK;
 	}
-	main_rev = (val >> data->main_rev_shift) & EXYNOS_REV_PART_MASK;
-	sub_rev = (val >> data->sub_rev_shift) & EXYNOS_REV_PART_MASK;
-	soc_info->revision = (main_rev << EXYNOS_REV_PART_SHIFT) | sub_rev;
 
+	soc_info->revision = (main_rev << EXYNOS_REV_PART_SHIFT) | sub_rev;
 	return 0;
 }
 
diff --git a/include/linux/soc/samsung/exynos-chipid.h b/include/linux/soc/samsung/exynos-chipid.h
index 62f0e2531068..1eb13068f513 100644
--- a/include/linux/soc/samsung/exynos-chipid.h
+++ b/include/linux/soc/samsung/exynos-chipid.h
@@ -10,6 +10,7 @@
 
 #define EXYNOS_CHIPID_REG_PRO_ID	0x00
 #define EXYNOS_REV_PART_MASK		0xf
+#define EXYNOS_REV_PART_MASK_GS101	0xff
 #define EXYNOS_REV_PART_SHIFT		4
 #define EXYNOS_MASK			0xfffff000
 
-- 
2.43.0

Re: [PATCH 1/4] dt-bindings: hwinfo: samsung,exynos-chipid: add gs101-chipid compatible

[Peter Griffin]

Hi Alexey,

On Thu, 1 Feb 2024 at 17:22, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>
> Add "google,gs101-chipid" compatible string to binding document.
>
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---

Thanks for your contribution.
Reviewed-by: Peter Griffin <peter.griffin@linaro.org>






>  .../devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml        | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml b/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
> index 780ccb5ee9b4..b1d933808b6c 100644
> --- a/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
> +++ b/Documentation/devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml
> @@ -13,6 +13,7 @@ properties:
>    compatible:
>      oneOf:
>        - enum:
> +          - google,gs101-chipid
>            - samsung,exynos4210-chipid
>            - samsung,exynos850-chipid
>        - items:
> --
> 2.43.0
>

Re: [PATCH 2/4] arm64: dts: exynos: gs101: add chipid node

[Peter Griffin]

Hi Alexey & Krysztof,

On Thu, 1 Feb 2024 at 17:22, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> index d838e3a7af6e..156fec2575bc 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> @@ -283,6 +283,11 @@ soc: soc@0 {
>                 #size-cells = <1>;
>                 ranges = <0x0 0x0 0x0 0x40000000>;
>
> +               chipid@10000000 {
> +                       compatible = "google,gs101-chipid";
> +                       reg = <0x10000000 0xd000>;
> +               };
> +

I was wondering about the 0xd000 size here, as most upstream platforms
use a chipid size of 0x100 or 0x24. I see the downstream gs101 kernel
also uses 0xd000. Looking a bit more, that is because gs-chipid.c also
has support for dumping other areas of the OTP SFR bank like asv table
(offset 0x9000) hpm_asv (offset 0xa000) and hw_tune (0xc000).

I checked Exynos850 and that also has ASV tables at those same offsets
above, but it currently uses a chipid size of 0x100 upstream.
Exynos-asv.c driver is part of exynos-chipid.c upstream so it seems
reasonable to have the increased size including those SFR registers.
Currently exynos-asv.c driver only supports Exynos5422 upstream.

@Krzysztof - From a process PoV what is the best/correct thing to do
here? Have the increased size in DT that includes ASV parts of the OTP
bank from the get-go?

Thanks,

Peter.

Re: [PATCH 3/4] soc: samsung: exynos-chipid: add Google Tensor gs101 SoC support

[Peter Griffin]

On Thu, 1 Feb 2024 at 17:22, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>
> Add GS101 information to soc_ids table and related entries to other
> places. This SoC product id is "0x09845000".
>
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>



>  drivers/soc/samsung/exynos-chipid.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> index b1118d37779e..7fee6094db12 100644
> --- a/drivers/soc/samsung/exynos-chipid.c
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -60,6 +60,8 @@ static const struct exynos_soc_id {
>         { "EXYNOS850", 0xE3830000 },
>         { "EXYNOSAUTOV9", 0xAAA80000 },
>         { "EXYNOSAUTOV920", 0x0A920000 },
> +       /* Compatible with: google,gs101-chipid */
> +       { "GS101", 0x09845000 },
>  };
>
>  static const char *product_id_to_soc_id(unsigned int product_id)
> @@ -178,8 +180,17 @@ static const struct exynos_chipid_variant exynos850_chipid_drv_data = {
>         .sub_rev_shift  = 16,
>  };
>
> +static const struct exynos_chipid_variant gs101_chipid_drv_data = {
> +       .rev_reg        = 0x10,
> +       .main_rev_shift = 0,
> +       .sub_rev_shift  = 16,
> +};
> +
>  static const struct of_device_id exynos_chipid_of_device_ids[] = {
>         {
> +               .compatible     = "google,gs101-chipid",
> +               .data           = &gs101_chipid_drv_data,
> +       }, {
>                 .compatible     = "samsung,exynos4210-chipid",
>                 .data           = &exynos4210_chipid_drv_data,
>         }, {
> --
> 2.43.0
>

Re: [PATCH 4/4] soc: samsung: exynos-chipid: fix revision calculation for gs101

[Peter Griffin]

Hi Alexey,

On Thu, 1 Feb 2024 at 17:22, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>
> The main revision for gs101 SoC is not reported in the CHIPID_REV
> register. The gs101 Product ID and revisions registers have a behaviour
> split between old Exynos SoCs and new SoCs. The sub-revision is
> reported in CHIPID_REV register in [19:16] bits but main revision
> is still present in Product ID [7:0].
>
> To construct soc_info->revision correctly for gs101 the main_rev
> should not be reset from a value read from CHIPID_REV.
>

I think it would also be worth adding in the commit message how the
main_rev and sub_rev relate to the a0, b0, b1 reported by the
bootloader.

> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  drivers/soc/samsung/exynos-chipid.c       | 20 ++++++++++++++++----
>  include/linux/soc/samsung/exynos-chipid.h |  1 +
>  2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
> index 7fee6094db12..3b952ffd8cf7 100644
> --- a/drivers/soc/samsung/exynos-chipid.c
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -87,14 +87,26 @@ static int exynos_chipid_get_chipid_info(struct regmap *regmap,
>         soc_info->product_id = val & EXYNOS_MASK;
>
>         if (data->rev_reg != EXYNOS_CHIPID_REG_PRO_ID) {
> -               ret = regmap_read(regmap, data->rev_reg, &val);
> +               unsigned int val2;
> +
> +               ret = regmap_read(regmap, data->rev_reg, &val2);
>                 if (ret < 0)
>                         return ret;
> +
> +               if (data->main_rev_shift == 0)
> +                       main_rev = (val >> data->main_rev_shift)
> +                                  & EXYNOS_REV_PART_MASK_GS101;

Looks like it can be simplified to
main_rev = val & EXYNOS_REV_PART_MASK_GS101;

Peter

Re: [PATCH 1/4] dt-bindings: hwinfo: samsung,exynos-chipid: add gs101-chipid compatible

[Rob Herring]

On Thu, 01 Feb 2024 17:22:21 +0000, Alexey Klimov wrote:
> Add "google,gs101-chipid" compatible string to binding document.
> 
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  .../devicetree/bindings/hwinfo/samsung,exynos-chipid.yaml        | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/4] arm64: dts: exynos: gs101: add chipid node

[Krzysztof Kozlowski]

On 05/02/2024 15:36, Peter Griffin wrote:
> Hi Alexey & Krysztof,
> 
> On Thu, 1 Feb 2024 at 17:22, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>>
>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>> ---
>>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> index d838e3a7af6e..156fec2575bc 100644
>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>> @@ -283,6 +283,11 @@ soc: soc@0 {
>>                 #size-cells = <1>;
>>                 ranges = <0x0 0x0 0x0 0x40000000>;
>>
>> +               chipid@10000000 {
>> +                       compatible = "google,gs101-chipid";
>> +                       reg = <0x10000000 0xd000>;
>> +               };
>> +
> 
> I was wondering about the 0xd000 size here, as most upstream platforms
> use a chipid size of 0x100 or 0x24. I see the downstream gs101 kernel
> also uses 0xd000. Looking a bit more, that is because gs-chipid.c also
> has support for dumping other areas of the OTP SFR bank like asv table
> (offset 0x9000) hpm_asv (offset 0xa000) and hw_tune (0xc000).
> 
> I checked Exynos850 and that also has ASV tables at those same offsets
> above, but it currently uses a chipid size of 0x100 upstream.
> Exynos-asv.c driver is part of exynos-chipid.c upstream so it seems
> reasonable to have the increased size including those SFR registers.
> Currently exynos-asv.c driver only supports Exynos5422 upstream.
> 
> @Krzysztof - From a process PoV what is the best/correct thing to do
> here? Have the increased size in DT that includes ASV parts of the OTP
> bank from the get-go?

ChipID so far had only size of 0x30 or something like that. What you
refer to does not look like old ChipID but full blown OTP, which also
includes ChipID. Although I am not entirely sure about that, either.
Depends whether they share clocks, for example.

I don't have any GS101 information so I don't know what's there. It
seems you ask me to give you decision based on guessing... If you have
one block, so if there is OTP, which contains ChipID, then define OTP.
Not ChipID+OTP.

I think Exynos850 DTSI is wrong here. That's OTP block, not ChipID.

Best regards,
Krzysztof

Re: [PATCH 2/4] arm64: dts: exynos: gs101: add chipid node

[Krzysztof Kozlowski]

On 01/02/2024 18:22, Alexey Klimov wrote:
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> index d838e3a7af6e..156fec2575bc 100644
> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> @@ -283,6 +283,11 @@ soc: soc@0 {
>  		#size-cells = <1>;
>  		ranges = <0x0 0x0 0x0 0x40000000>;
>  
> +		chipid@10000000 {
> +			compatible = "google,gs101-chipid";
> +			reg = <0x10000000 0xd000>;

ChipID has size around 0x20 or 0x30, not 0xd000. Maybe you are defining
some other device like OTP?

Best regards,
Krzysztof

Re: [PATCH 2/4] arm64: dts: exynos: gs101: add chipid node

[Peter Griffin]

Hi Krzysztof,

Thanks for your feedback.

On Tue, 6 Feb 2024 at 10:10, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 05/02/2024 15:36, Peter Griffin wrote:
> > Hi Alexey & Krysztof,
> >
> > On Thu, 1 Feb 2024 at 17:22, Alexey Klimov <alexey.klimov@linaro.org> wrote:
> >>
> >> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> >> ---
> >>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> >> index d838e3a7af6e..156fec2575bc 100644
> >> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> >> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
> >> @@ -283,6 +283,11 @@ soc: soc@0 {
> >>                 #size-cells = <1>;
> >>                 ranges = <0x0 0x0 0x0 0x40000000>;
> >>
> >> +               chipid@10000000 {
> >> +                       compatible = "google,gs101-chipid";
> >> +                       reg = <0x10000000 0xd000>;
> >> +               };
> >> +
> >
> > I was wondering about the 0xd000 size here, as most upstream platforms
> > use a chipid size of 0x100 or 0x24. I see the downstream gs101 kernel
> > also uses 0xd000. Looking a bit more, that is because gs-chipid.c also
> > has support for dumping other areas of the OTP SFR bank like asv table
> > (offset 0x9000) hpm_asv (offset 0xa000) and hw_tune (0xc000).
> >
> > I checked Exynos850 and that also has ASV tables at those same offsets
> > above, but it currently uses a chipid size of 0x100 upstream.
> > Exynos-asv.c driver is part of exynos-chipid.c upstream so it seems
> > reasonable to have the increased size including those SFR registers.
> > Currently exynos-asv.c driver only supports Exynos5422 upstream.
> >
> > @Krzysztof - From a process PoV what is the best/correct thing to do
> > here? Have the increased size in DT that includes ASV parts of the OTP
> > bank from the get-go?
>
> ChipID so far had only size of 0x30 or something like that. What you
> refer to does not look like old ChipID but full blown OTP, which also
> includes ChipID.

OK so in some previous Exynos SoCs chipid had its own separate memory
mapped SFRs as well as being present in the OTP area?

>  Although I am not entirely sure about that, either.
> Depends whether they share clocks, for example.

This address is the OTP area, and I can't see chipid regs mentioned
anywhere else in the memory map other than OTP. Unfortunately there
are lots of separate docs for different IP blocks, so it isn't just a
case of searching a giant SoC TRM pdf.

e850 though looks to be the same (the address defined in DT is the otp
area), that is one large PDF and the chipid regs aren't mentioned
anywhere else, Given the chipid reg offset is the same (0x10000000)
for exynosautov9.dtsi, exynosautov920.dtsi, exynos850.dtsi, exynos7885
and exynos5433 I suspect this could be the same for all those SoCs as
well.

>
> I don't have any GS101 information so I don't know what's there. It
> seems you ask me to give you decision based on guessing... If you have
> one block, so if there is OTP, which contains ChipID, then define OTP.

I believe there is one block that contains ChipID, therefore based on
the above info we should define full OTP size?

> Not ChipID+OTP.
>
> I think Exynos850 DTSI is wrong here. That's OTP block, not ChipID.

Yes agreed, and quite possibly the other Exynos SoCs as well.

Thanks,

Peter.

Re: [PATCH 2/4] arm64: dts: exynos: gs101: add chipid node

[Krzysztof Kozlowski]

On 07/02/2024 15:11, Peter Griffin wrote:
> Hi Krzysztof,
> 
> Thanks for your feedback.
> 
> On Tue, 6 Feb 2024 at 10:10, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 05/02/2024 15:36, Peter Griffin wrote:
>>> Hi Alexey & Krysztof,
>>>
>>> On Thu, 1 Feb 2024 at 17:22, Alexey Klimov <alexey.klimov@linaro.org> wrote:
>>>>
>>>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>>>> ---
>>>>  arch/arm64/boot/dts/exynos/google/gs101.dtsi | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> index d838e3a7af6e..156fec2575bc 100644
>>>> --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi
>>>> @@ -283,6 +283,11 @@ soc: soc@0 {
>>>>                 #size-cells = <1>;
>>>>                 ranges = <0x0 0x0 0x0 0x40000000>;
>>>>
>>>> +               chipid@10000000 {
>>>> +                       compatible = "google,gs101-chipid";
>>>> +                       reg = <0x10000000 0xd000>;
>>>> +               };
>>>> +
>>>
>>> I was wondering about the 0xd000 size here, as most upstream platforms
>>> use a chipid size of 0x100 or 0x24. I see the downstream gs101 kernel
>>> also uses 0xd000. Looking a bit more, that is because gs-chipid.c also
>>> has support for dumping other areas of the OTP SFR bank like asv table
>>> (offset 0x9000) hpm_asv (offset 0xa000) and hw_tune (0xc000).
>>>
>>> I checked Exynos850 and that also has ASV tables at those same offsets
>>> above, but it currently uses a chipid size of 0x100 upstream.
>>> Exynos-asv.c driver is part of exynos-chipid.c upstream so it seems
>>> reasonable to have the increased size including those SFR registers.
>>> Currently exynos-asv.c driver only supports Exynos5422 upstream.
>>>
>>> @Krzysztof - From a process PoV what is the best/correct thing to do
>>> here? Have the increased size in DT that includes ASV parts of the OTP
>>> bank from the get-go?
>>
>> ChipID so far had only size of 0x30 or something like that. What you
>> refer to does not look like old ChipID but full blown OTP, which also
>> includes ChipID.
> 
> OK so in some previous Exynos SoCs chipid had its own separate memory
> mapped SFRs as well as being present in the OTP area?

None of the Exynos I know, have OTP area. There was only chipid. It
seems that few newer designs come with OTP, in entirely separate address
space. Exynos850 looks like the first which comes with integrated chipid
into OTP, so OTP is not separate address.

> 
>>  Although I am not entirely sure about that, either.
>> Depends whether they share clocks, for example.
> 
> This address is the OTP area, and I can't see chipid regs mentioned
> anywhere else in the memory map other than OTP. Unfortunately there
> are lots of separate docs for different IP blocks, so it isn't just a
> case of searching a giant SoC TRM pdf.
> 
> e850 though looks to be the same (the address defined in DT is the otp
> area), that is one large PDF and the chipid regs aren't mentioned
> anywhere else, Given the chipid reg offset is the same (0x10000000)
> for exynosautov9.dtsi, exynosautov920.dtsi, exynos850.dtsi, exynos7885
> and exynos5433 I suspect this could be the same for all those SoCs as
> well.
> 
>>
>> I don't have any GS101 information so I don't know what's there. It
>> seems you ask me to give you decision based on guessing... If you have
>> one block, so if there is OTP, which contains ChipID, then define OTP.
> 
> I believe there is one block that contains ChipID, therefore based on
> the above info we should define full OTP size?
> 
>> Not ChipID+OTP.
>>
>> I think Exynos850 DTSI is wrong here. That's OTP block, not ChipID.
> 
> Yes agreed, and quite possibly the other Exynos SoCs as well.

If ChipID and OTP are in the same block (in OTP), then assume they both
might need the same clocks or some other resources. Therefore we should
not model them as two separate device nodes ChipID and OTP. Instead
there should be one device node with entire OTP address space, which
should not use ChipID compatible to avoid confusion.

Best regards,
Krzysztof