[PATCH 0/3] Enable Security ID for H616

[PATCH 0/3] Enable Security ID for H616

[Martin Botka]

Hello,

This patch series enables Security ID (SID) support for AllWinner H616.

SID support is an requirement for few things:
    - Thermal sensors
    - CPU Binning ID (cpufreq will use this)
    - EPhy

This patch series doesn't add support for the consumers.
Those will come in the future via their own series.

Cheers,
Martin

Signed-off-by: Martin Botka <martin.botka@somainline.org>
---
Martin Botka (3):
      dt-bindings: nvmem: SID: Add binding for H616 SID controller
      nvmem: sunxi_sid: Add support for H616 SID
      arm64: dts: allwinner: h616: Add SID controller node

 .../devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml         | 1 +
 arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi                     | 7 +++++++
 drivers/nvmem/sunxi_sid.c                                          | 1 +
 3 files changed, 9 insertions(+)
---
base-commit: 52a93d39b17dc7eb98b6aa3edb93943248e03b2f
change-id: 20230811-sid-h616-37b773e59571

Best regards,
-- 
Martin Botka <martin.botka@somainline.org>

[PATCH 1/3] dt-bindings: nvmem: SID: Add binding for H616 SID controller

[Martin Botka]

Add binding for the SID controller found in H616 SoC

Signed-off-by: Martin Botka <martin.botka@somainline.org>
---
 Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml b/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
index 296001e7f498..2ec0a1b8f803 100644
--- a/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
+++ b/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
@@ -27,6 +27,7 @@ properties:
           - const: allwinner,sun50i-a64-sid
       - const: allwinner,sun50i-h5-sid
       - const: allwinner,sun50i-h6-sid
+      - const: allwinner,sun50i-h616-sid
 
   reg:
     maxItems: 1

-- 
2.41.0

[PATCH 2/3] nvmem: sunxi_sid: Add support for H616 SID

[Martin Botka]

Add support for the H616 SID controller.

The config can be reused from A64.

Signed-off-by: Martin Botka <martin.botka@somainline.org>
---
 drivers/nvmem/sunxi_sid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
index a970f1741cc6..df6fb5e0b724 100644
--- a/drivers/nvmem/sunxi_sid.c
+++ b/drivers/nvmem/sunxi_sid.c
@@ -216,6 +216,7 @@ static const struct of_device_id sunxi_sid_of_match[] = {
 	{ .compatible = "allwinner,sun50i-a64-sid", .data = &sun50i_a64_cfg },
 	{ .compatible = "allwinner,sun50i-h5-sid", .data = &sun50i_a64_cfg },
 	{ .compatible = "allwinner,sun50i-h6-sid", .data = &sun50i_h6_cfg },
+	{ .compatible = "allwinner,sun50i-h616-sid", .data = &sun50i_a64_cfg },
 	{/* sentinel */},
 };
 MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);

-- 
2.41.0

[PATCH 3/3] arm64: dts: allwinner: h616: Add SID controller node

[Martin Botka]

Add node for the H616 SID controller

Signed-off-by: Martin Botka <martin.botka@somainline.org>
---
 arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
index 74aed0d232a9..5c5187a22537 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
@@ -133,6 +133,13 @@ ccu: clock@3001000 {
 			#reset-cells = <1>;
 		};
 
+		sid: efuse@3006000 {
+			compatible = "allwinner,sun50i-h616-sid";
+			reg = <0x03006000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+		};
+
 		watchdog: watchdog@30090a0 {
 			compatible = "allwinner,sun50i-h616-wdt",
 				     "allwinner,sun6i-a31-wdt";

-- 
2.41.0

Re: [PATCH 2/3] nvmem: sunxi_sid: Add support for H616 SID

[Andre Przywara]

On Fri, 11 Aug 2023 13:02:35 +0200
Martin Botka <martin.botka@somainline.org> wrote:

Hi Martin,

> Add support for the H616 SID controller.

thanks for upstreaming this!

> 
> The config can be reused from A64.
> 
> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> ---
>  drivers/nvmem/sunxi_sid.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
> index a970f1741cc6..df6fb5e0b724 100644
> --- a/drivers/nvmem/sunxi_sid.c
> +++ b/drivers/nvmem/sunxi_sid.c
> @@ -216,6 +216,7 @@ static const struct of_device_id sunxi_sid_of_match[] = {
>  	{ .compatible = "allwinner,sun50i-a64-sid", .data = &sun50i_a64_cfg },
>  	{ .compatible = "allwinner,sun50i-h5-sid", .data = &sun50i_a64_cfg },
>  	{ .compatible = "allwinner,sun50i-h6-sid", .data = &sun50i_h6_cfg },
> +	{ .compatible = "allwinner,sun50i-h616-sid", .data = &sun50i_a64_cfg },

You don't need a new compatible string, then.
Just use 
  compatible = "allwinner,sun50i-h616-sid", "allwinner,sun50i-a64-sid";
in the DT, and add that combo to the binding, then you don't need any
driver patches at all - with the added benefit of that already working
with existing kernels.

Though I wonder if that has really only 256 bytes of fuses? Do we have any
evidence of that?

Cheers,
Andre

>  	{/* sentinel */},
>  };
>  MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
> 

Re: [PATCH 2/3] nvmem: sunxi_sid: Add support for H616 SID

[Martin Botka]

On Fri, Aug 11 2023 at 12:17:54 PM +01:00:00, Andre Przywara 
<andre.przywara@arm.com> wrote:
> On Fri, 11 Aug 2023 13:02:35 +0200
> Martin Botka <martin.botka@somainline.org> wrote:
> 
> Hi Martin,
> 
>>  Add support for the H616 SID controller.
> 
> thanks for upstreaming this!
> 
>> 
>>  The config can be reused from A64.
>> 
>>  Signed-off-by: Martin Botka <martin.botka@somainline.org>
>>  ---
>>   drivers/nvmem/sunxi_sid.c | 1 +
>>   1 file changed, 1 insertion(+)
>> 
>>  diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
>>  index a970f1741cc6..df6fb5e0b724 100644
>>  --- a/drivers/nvmem/sunxi_sid.c
>>  +++ b/drivers/nvmem/sunxi_sid.c
>>  @@ -216,6 +216,7 @@ static const struct of_device_id 
>> sunxi_sid_of_match[] = {
>>   	{ .compatible = "allwinner,sun50i-a64-sid", .data = 
>> &sun50i_a64_cfg },
>>   	{ .compatible = "allwinner,sun50i-h5-sid", .data = 
>> &sun50i_a64_cfg },
>>   	{ .compatible = "allwinner,sun50i-h6-sid", .data = &sun50i_h6_cfg 
>> },
>>  +	{ .compatible = "allwinner,sun50i-h616-sid", .data = 
>> &sun50i_a64_cfg },
> 
> You don't need a new compatible string, then.
> Just use
>   compatible = "allwinner,sun50i-h616-sid", 
> "allwinner,sun50i-a64-sid";
> in the DT, and add that combo to the binding, then you don't need any
> driver patches at all - with the added benefit of that already working
> with existing kernels.
I added it cause H5 and A83T were added after their DT binding was in 
and they also use A64 config.
But I will drop this patch and use a64 compatible if that is the 
prefered way :)
> 
> Though I wonder if that has really only 256 bytes of fuses? Do we 
> have any
> evidence of that?
Datasheet has 0 info regarding this. The way i got the size and offset 
was to check the vendor code.
The values matched A64.
> 
> Cheers,
> Andre
> 
>>   	{/* sentinel */},
>>   };
>>   MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
>> 
> 

Re: [PATCH 1/3] dt-bindings: nvmem: SID: Add binding for H616 SID controller

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

On Fri, Aug 11, 2023 at 01:02:34PM +0200, Martin Botka wrote:
> Add binding for the SID controller found in H616 SoC
> 
> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> ---
>  Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml b/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
> index 296001e7f498..2ec0a1b8f803 100644
> --- a/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
> @@ -27,6 +27,7 @@ properties:
>            - const: allwinner,sun50i-a64-sid
>        - const: allwinner,sun50i-h5-sid
>        - const: allwinner,sun50i-h6-sid
> +      - const: allwinner,sun50i-h616-sid

As said by Andre, use a fallback compatible here please.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: nvmem: SID: Add binding for H616 SID controller

[Martin Botka]

On Fri, Aug 11 2023 at 04:23:11 PM +01:00:00, Conor Dooley 
<conor@kernel.org> wrote:
> On Fri, Aug 11, 2023 at 01:02:34PM +0200, Martin Botka wrote:
>>  Add binding for the SID controller found in H616 SoC
>> 
>>  Signed-off-by: Martin Botka <martin.botka@somainline.org>
>>  ---
>>   
>> Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml 
>> | 1 +
>>   1 file changed, 1 insertion(+)
>> 
>>  diff --git 
>> a/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml 
>> b/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
>>  index 296001e7f498..2ec0a1b8f803 100644
>>  --- 
>> a/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
>>  +++ 
>> b/Documentation/devicetree/bindings/nvmem/allwinner,sun4i-a10-sid.yaml
>>  @@ -27,6 +27,7 @@ properties:
>>             - const: allwinner,sun50i-a64-sid
>>         - const: allwinner,sun50i-h5-sid
>>         - const: allwinner,sun50i-h6-sid
>>  +      - const: allwinner,sun50i-h616-sid
> 
> As said by Andre, use a fallback compatible here please.

Will do.
Cheers,
Martin

Re: [PATCH 2/3] nvmem: sunxi_sid: Add support for H616 SID

[Andre Przywara]

On Fri, 11 Aug 2023 15:12:34 +0200
Martin Botka <martin.botka@somainline.org> wrote:

Hi,

Maxime, Srinivas, can you possibly shed some light on the exact scope of
this nvmem DT binding? See below for more details ...

> On Fri, Aug 11 2023 at 12:17:54 PM +01:00:00, Andre Przywara 
> <andre.przywara@arm.com> wrote:
> > On Fri, 11 Aug 2023 13:02:35 +0200
> > Martin Botka <martin.botka@somainline.org> wrote:
> > 
> > Hi Martin,
> >   
> >>  Add support for the H616 SID controller.  
> > 
> > thanks for upstreaming this!
> >   
> >> 
> >>  The config can be reused from A64.
> >> 
> >>  Signed-off-by: Martin Botka <martin.botka@somainline.org>
> >>  ---
> >>   drivers/nvmem/sunxi_sid.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >> 
> >>  diff --git a/drivers/nvmem/sunxi_sid.c b/drivers/nvmem/sunxi_sid.c
> >>  index a970f1741cc6..df6fb5e0b724 100644
> >>  --- a/drivers/nvmem/sunxi_sid.c
> >>  +++ b/drivers/nvmem/sunxi_sid.c
> >>  @@ -216,6 +216,7 @@ static const struct of_device_id 
> >> sunxi_sid_of_match[] = {
> >>   	{ .compatible = "allwinner,sun50i-a64-sid", .data = 
> >> &sun50i_a64_cfg },
> >>   	{ .compatible = "allwinner,sun50i-h5-sid", .data = 
> >> &sun50i_a64_cfg },
> >>   	{ .compatible = "allwinner,sun50i-h6-sid", .data = &sun50i_h6_cfg 
> >> },
> >>  +	{ .compatible = "allwinner,sun50i-h616-sid", .data = 
> >> &sun50i_a64_cfg },  
> > 
> > You don't need a new compatible string, then.
> > Just use
> >   compatible = "allwinner,sun50i-h616-sid", 
> > "allwinner,sun50i-a64-sid";
> > in the DT, and add that combo to the binding, then you don't need any
> > driver patches at all - with the added benefit of that already working
> > with existing kernels.  
> I added it cause H5 and A83T were added after their DT binding was in 
> and they also use A64 config.

Yeah, I saw this, and going back in the commit messages I found this:
"The H5 SoC have a SID controller that looks like the one in A64, the
cells are in the same offset but doesn't contain the same data (thermal
sensor calibration for example)."

So does the compatible string for the NVMEM *controller* really
encompass the location, encoding and meaning of all its NVMEM cells?
Isn't that described independently by the child nodes, that put names
to addresses, and that are referenced by other DT nodes?
Or was it originally not the case, and the scope changed over time?

It's certainly no biggie to use a single compatible string, and add a
single line to the driver (like in this patch), but it would be good to
know if this is really the intention.

Cheers,
Andre

> But I will drop this patch and use a64 compatible if that is the 
> prefered way :)
> > 
> > Though I wonder if that has really only 256 bytes of fuses? Do we 
> > have any
> > evidence of that?  
> Datasheet has 0 info regarding this. The way i got the size and offset 
> was to check the vendor code.
> The values matched A64.
> > 
> > Cheers,
> > Andre
> >   
> >>   	{/* sentinel */},
> >>   };
> >>   MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
> >>   
> >   
> 
> 
>