[PATCH 0/2] Add support fdp1 device for Salvator-XS M3-N

[PATCH 0/2] Add support fdp1 device for Salvator-XS M3-N

[Nguyen An Hoan]

From: Hoan Nguyen An <na-hoan@jinso.co.jp>

These patches add fdp1 device support for M3-N r8a77965 board.
Please review for me, thank you!

Hoan Nguyen An (2):
  arm64: dts: r8a77965: add FDP1 device nodes
  clk: renesas: r8a77965: Add FDP clock

 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 10 ++++++++++
 drivers/clk/renesas/r8a77965-cpg-mssr.c   |  1 +
 2 files changed, 11 insertions(+)

-- 
2.7.4

[PATCH 1/2] arm64: dts: r8a77965: add FDP1 device nodes

[Nguyen An Hoan]

From: Hoan Nguyen An <na-hoan@jinso.co.jp>

Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
---
 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
index 9c4f405..bef519f 100644
--- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
@@ -1578,6 +1578,16 @@
 			status = "disabled";
 		};
 
+		fdp1@fe940000 {
+			compatible = "renesas,fdp1";
+			reg = <0 0xfe940000 0 0x2400>;
+			interrupts = <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 119>;
+			power-domains = <&sysc R8A77965_PD_A3VP>;
+			resets = <&cpg 119>;
+			renesas,fcp = <&fcpf0>;
+		};
+
 		fcpf0: fcp@fe950000 {
 			compatible = "renesas,fcpf";
 			reg = <0 0xfe950000 0 0x200>;
-- 
2.7.4

Re: [PATCH 1/2] arm64: dts: r8a77965: add FDP1 device nodes

[Laurent Pinchart]

Hello Nguyen An,

Thank you for the patch.

On Friday, 24 August 2018 07:52:28 EEST Nguyen An Hoan wrote:
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>

You're missing a commit message. I agree that for simple patches like this one 
the subject line often contains enough information, but adding a commit 
message is still a good practice that we try to enforce through the kernel. 
For instance, looking at git history for r8a7796, you could use

"The r8a77965 has a single FDP1 instance."

> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Simon, could you update the commit message when taking this patch in your 
tree, to avoid the need for a v2 ?

> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index 9c4f405..bef519f 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -1578,6 +1578,16 @@
>  			status = "disabled";
>  		};
> 
> +		fdp1@fe940000 {
> +			compatible = "renesas,fdp1";
> +			reg = <0 0xfe940000 0 0x2400>;
> +			interrupts = <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 119>;
> +			power-domains = <&sysc R8A77965_PD_A3VP>;
> +			resets = <&cpg 119>;
> +			renesas,fcp = <&fcpf0>;
> +		};
> +
>  		fcpf0: fcp@fe950000 {
>  			compatible = "renesas,fcpf";
>  			reg = <0 0xfe950000 0 0x200>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] arm64: dts: r8a77965: add FDP1 device nodes

[Simon Horman]

On Fri, Aug 24, 2018 at 11:45:52AM +0300, Laurent Pinchart wrote:
> Hello Nguyen An,
> 
> Thank you for the patch.
> 
> On Friday, 24 August 2018 07:52:28 EEST Nguyen An Hoan wrote:
> > From: Hoan Nguyen An <na-hoan@jinso.co.jp>
> 
> You're missing a commit message. I agree that for simple patches like this one 
> the subject line often contains enough information, but adding a commit 
> message is still a good practice that we try to enforce through the kernel. 
> For instance, looking at git history for r8a7796, you could use
> 
> "The r8a77965 has a single FDP1 instance."
> 
> > Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Simon, could you update the commit message when taking this patch in your 
> tree, to avoid the need for a v2 ?

Yes, can do.

Can I confirm that it is safe, from a regression point of view,
to apply this patch without patch 2/2?

> > ---
> >  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index 9c4f405..bef519f 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> > @@ -1578,6 +1578,16 @@
> >  			status = "disabled";
> >  		};
> > 
> > +		fdp1@fe940000 {
> > +			compatible = "renesas,fdp1";
> > +			reg = <0 0xfe940000 0 0x2400>;
> > +			interrupts = <GIC_SPI 262 IRQ_TYPE_LEVEL_HIGH>;
> > +			clocks = <&cpg CPG_MOD 119>;
> > +			power-domains = <&sysc R8A77965_PD_A3VP>;
> > +			resets = <&cpg 119>;
> > +			renesas,fcp = <&fcpf0>;
> > +		};
> > +
> >  		fcpf0: fcp@fe950000 {
> >  			compatible = "renesas,fcpf";
> >  			reg = <0 0xfe950000 0 0x200>;
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

Re: [PATCH 1/2] arm64: dts: r8a77965: add FDP1 device nodes

[Laurent Pinchart]

Hi Simon,

On Monday, 27 August 2018 15:57:05 EEST Simon Horman wrote:
> On Fri, Aug 24, 2018 at 11:45:52AM +0300, Laurent Pinchart wrote:
> > On Friday, 24 August 2018 07:52:28 EEST Nguyen An Hoan wrote:
> > > From: Hoan Nguyen An <na-hoan@jinso.co.jp>
> > 
> > You're missing a commit message. I agree that for simple patches like this
> > one the subject line often contains enough information, but adding a
> > commit message is still a good practice that we try to enforce through
> > the kernel. For instance, looking at git history for r8a7796, you could
> > use
> > 
> > "The r8a77965 has a single FDP1 instance."
> > 
> > > Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> > 
> > Apart from that,
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Simon, could you update the commit message when taking this patch in your
> > tree, to avoid the need for a v2 ?
> 
> Yes, can do.
> 
> Can I confirm that it is safe, from a regression point of view,
> to apply this patch without patch 2/2?

As far as I know it is safe, yes.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/2] arm64: dts: r8a77965: add FDP1 device nodes

[Simon Horman]

On Tue, Aug 28, 2018 at 12:56:18PM +0300, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Monday, 27 August 2018 15:57:05 EEST Simon Horman wrote:
> > On Fri, Aug 24, 2018 at 11:45:52AM +0300, Laurent Pinchart wrote:
> > > On Friday, 24 August 2018 07:52:28 EEST Nguyen An Hoan wrote:
> > > > From: Hoan Nguyen An <na-hoan@jinso.co.jp>
> > > 
> > > You're missing a commit message. I agree that for simple patches like this
> > > one the subject line often contains enough information, but adding a
> > > commit message is still a good practice that we try to enforce through
> > > the kernel. For instance, looking at git history for r8a7796, you could
> > > use
> > > 
> > > "The r8a77965 has a single FDP1 instance."
> > > 
> > > > Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> > > 
> > > Apart from that,
> > > 
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > > Simon, could you update the commit message when taking this patch in your
> > > tree, to avoid the need for a v2 ?
> > 
> > Yes, can do.
> > 
> > Can I confirm that it is safe, from a regression point of view,
> > to apply this patch without patch 2/2?
> 
> As far as I know it is safe, yes.

Thanks, applied for v4.20.

Re: [1/2] arm64: dts: r8a77965: add FDP1 device nodes

[Nguyen An Hoan]

Laurent-san, Simon-san

> You're missing a commit message. I agree that for simple patches like this one 
> the subject line often contains enough information, but adding a commit 
> message is still a good practice that we try to enforce through the kernel.
...
> Yes, can do.

I understand!
Thank you very much for your reviews and comments!

[PATCH 2/2] clk: renesas: r8a77965: Add FDP clock

[Nguyen An Hoan]

From: Hoan Nguyen An <na-hoan@jinso.co.jp>

Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
---
 drivers/clk/renesas/r8a77965-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/renesas/r8a77965-cpg-mssr.c b/drivers/clk/renesas/r8a77965-cpg-mssr.c
index 312f9fe..d0847dc 100644
--- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
@@ -112,6 +112,7 @@ static const struct cpg_core_clk r8a77965_core_clks[] __initconst = {
 };
 
 static const struct mssr_mod_clk r8a77965_mod_clks[] __initconst = {
+	DEF_MOD("fdp0",			119,	R8A77965_CLK_S0D1),
 	DEF_MOD("scif5",		202,	R8A77965_CLK_S3D4),
 	DEF_MOD("scif4",		203,	R8A77965_CLK_S3D4),
 	DEF_MOD("scif3",		204,	R8A77965_CLK_S3D4),
-- 
2.7.4

Re: [PATCH 2/2] clk: renesas: r8a77965: Add FDP clock

[Laurent Pinchart]

Hello Nguyen An,

Thank you for the patch.

On Friday, 24 August 2018 07:52:29 EEST Nguyen An Hoan wrote:
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>

Here too a commit message would be nice.

> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
> ---
>  drivers/clk/renesas/r8a77965-cpg-mssr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/renesas/r8a77965-cpg-mssr.c
> b/drivers/clk/renesas/r8a77965-cpg-mssr.c index 312f9fe..d0847dc 100644
> --- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
> @@ -112,6 +112,7 @@ static const struct cpg_core_clk r8a77965_core_clks[]
> __initconst = { };
> 
>  static const struct mssr_mod_clk r8a77965_mod_clks[] __initconst = {
> +	DEF_MOD("fdp0",			119,	R8A77965_CLK_S0D1),

I haven't found information in the datasheet to confirm whether the parent 
clock is correct. As it doesn't matter too much given that the parent clock 
doesn't need to be controlled, and the FDP driver doesn't care about the clock 
frequency, we can start with this without any problem.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	DEF_MOD("scif5",		202,	R8A77965_CLK_S3D4),
>  	DEF_MOD("scif4",		203,	R8A77965_CLK_S3D4),
>  	DEF_MOD("scif3",		204,	R8A77965_CLK_S3D4),

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] clk: renesas: r8a77965: Add FDP clock

[Geert Uytterhoeven]

Hi Hoan,

On Fri, Aug 24, 2018 at 6:52 AM Nguyen An Hoan <na-hoan@jinso.co.jp> wrote:
> From: Hoan Nguyen An <na-hoan@jinso.co.jp>
>
> Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>

Thanks for your patch!

> --- a/drivers/clk/renesas/r8a77965-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a77965-cpg-mssr.c
> @@ -112,6 +112,7 @@ static const struct cpg_core_clk r8a77965_core_clks[] __initconst = {
>  };
>
>  static const struct mssr_mod_clk r8a77965_mod_clks[] __initconst = {
> +       DEF_MOD("fdp0",                 119,    R8A77965_CLK_S0D1),

In the datasheet, and in drivers for other SoCs, this clock is called fdp1-0.

>         DEF_MOD("scif5",                202,    R8A77965_CLK_S3D4),
>         DEF_MOD("scif4",                203,    R8A77965_CLK_S3D4),
>         DEF_MOD("scif3",                204,    R8A77965_CLK_S3D4),

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Will queue in clk-renesas-for-v4.20, with the clock name fixed, and a commit
message added (stolen from the r8a7796 commit ;-).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds