[PATCH 1/4] clk: scmi: support set parent

[PATCH 1/4] clk: scmi: support set parent

[alice.guo]

From: Peng Fan <peng.fan@nxp.com>

Support SCMI CLK set parent.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
---
 drivers/clk/clk_scmi.c   | 20 ++++++++++++++++++++
 include/scmi_protocols.h | 19 +++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index e42d2032d4..84333cdd0c 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -178,11 +178,31 @@ static int scmi_clk_probe(struct udevice *dev)
 	return 0;
 }
 
+static int scmi_clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	struct scmi_clk_parent_set_in in = {
+		.clock_id = clk->id,
+		.parent_clk = parent->id,
+	};
+	struct scmi_clk_parent_set_out out;
+	struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_CLOCK,
+					  SCMI_CLOCK_PARENT_SET,
+					  in, out);
+	int ret;
+
+	ret = devm_scmi_process_msg(clk->dev, &msg);
+	if (ret < 0)
+		return ret;
+
+	return scmi_to_linux_errno(out.status);
+}
+
 static const struct clk_ops scmi_clk_ops = {
 	.enable = scmi_clk_enable,
 	.disable = scmi_clk_disable,
 	.get_rate = scmi_clk_get_rate,
 	.set_rate = scmi_clk_set_rate,
+	.set_parent = scmi_clk_set_parent,
 };
 
 U_BOOT_DRIVER(scmi_clock) = {
diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
index e52c740cb3..e1e4117e11 100644
--- a/include/scmi_protocols.h
+++ b/include/scmi_protocols.h
@@ -732,6 +732,7 @@ enum scmi_clock_message_id {
 	SCMI_CLOCK_RATE_SET = 0x5,
 	SCMI_CLOCK_RATE_GET = 0x6,
 	SCMI_CLOCK_CONFIG_SET = 0x7,
+	SCMI_CLOCK_PARENT_SET = 0x10
 };
 
 #define SCMI_CLK_PROTO_ATTR_COUNT_MASK	GENMASK(15, 0)
@@ -835,6 +836,24 @@ struct scmi_clk_rate_set_out {
 	s32 status;
 };
 
+/**
+ * struct scmi_clk_parent_state_in - Message payload for CLOCK_PARENT_SET command
+ * @clock_id:		SCMI clock ID
+ * @parent_clk:		SCMI clock ID
+ */
+struct scmi_clk_parent_set_in {
+	u32 clock_id;
+	u32 parent_clk;
+};
+
+/**
+ * struct scmi_clk_parent_set_out - Response payload for CLOCK_PARENT_SET command
+ * @status:	SCMI command status
+ */
+struct scmi_clk_parent_set_out {
+	s32 status;
+};
+
 /*
  * SCMI Reset Domain Protocol
  */
-- 
2.34.1

[PATCH 2/4] scmi_protocols: Update parent clock set ID

[alice.guo]

From: Ye Li <ye.li@nxp.com>

According to SCMI v3.2, switch to use 0xD for parent clock set.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 include/scmi_protocols.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h
index e1e4117e11..36e46f4072 100644
--- a/include/scmi_protocols.h
+++ b/include/scmi_protocols.h
@@ -732,7 +732,7 @@ enum scmi_clock_message_id {
 	SCMI_CLOCK_RATE_SET = 0x5,
 	SCMI_CLOCK_RATE_GET = 0x6,
 	SCMI_CLOCK_CONFIG_SET = 0x7,
-	SCMI_CLOCK_PARENT_SET = 0x10
+	SCMI_CLOCK_PARENT_SET = 0xD
 };
 
 #define SCMI_CLK_PROTO_ATTR_COUNT_MASK	GENMASK(15, 0)
-- 
2.34.1

[PATCH 3/4] clk: scmi: support probe in pre-reloc stage

[alice.guo]

From: Peng Fan <peng.fan@nxp.com>

Support the clk scmi driver probe in pre-reloc stage.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
---
 drivers/clk/clk_scmi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 84333cdd0c..a01292c479 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -210,4 +210,5 @@ U_BOOT_DRIVER(scmi_clock) = {
 	.id = UCLASS_CLK,
 	.ops = &scmi_clk_ops,
 	.probe = scmi_clk_probe,
+	.flags = DM_FLAG_PRE_RELOC,
 };
-- 
2.34.1

[PATCH 4/4] clk: scmi: Add workaround for set_rate/enable/disable

[alice.guo]

From: Ye Li <ye.li@nxp.com>

Add workaround to set_rate/enable/disable to bus clock that SM
will reply DENIED error.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk_scmi.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index a01292c479..a860a653ba 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -73,7 +73,13 @@ static int scmi_clk_gate(struct clk *clk, int enable)
 	if (ret)
 		return ret;
 
-	return scmi_to_linux_errno(out.status);
+	ret = scmi_to_linux_errno(out.status);
+	if (ret == -EACCES) {
+		debug("Ignore %s enable failure\n", clk_hw_get_name(clk));
+		ret = 0;
+	}
+
+	return ret;
 }
 
 static int scmi_clk_enable(struct clk *clk)
@@ -108,7 +114,7 @@ static ulong scmi_clk_get_rate(struct clk *clk)
 	return (ulong)(((u64)out.rate_msb << 32) | out.rate_lsb);
 }
 
-static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
+static ulong __scmi_clk_set_rate(struct clk *clk, ulong rate)
 {
 	struct scmi_clk_rate_set_in in = {
 		.clock_id = clk->id,
@@ -133,6 +139,17 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
 	return scmi_clk_get_rate(clk);
 }
 
+static ulong scmi_clk_set_rate(struct clk *clk, ulong rate)
+{
+	ulong orig_rate;
+
+	orig_rate = scmi_clk_get_rate(clk);
+	if (orig_rate == rate)
+		return orig_rate;
+
+	return __scmi_clk_set_rate(clk, rate);
+}
+
 static int scmi_clk_probe(struct udevice *dev)
 {
 	struct clk *clk;
-- 
2.34.1

Re: [PATCH 4/4] clk: scmi: Add workaround for set_rate/enable/disable

[Tom Rini]

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

On Sun, Oct 06, 2024 at 05:38:25PM +0800, alice.guo@oss.nxp.com wrote:
> From: Ye Li <ye.li@nxp.com>
> 
> Add workaround to set_rate/enable/disable to bus clock that SM
> will reply DENIED error.
> 
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>

In general, please include a cover letter so it will be clearer what the
overall goal is, especially once merged.

> ---
>  drivers/clk/clk_scmi.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
> index a01292c479..a860a653ba 100644
> --- a/drivers/clk/clk_scmi.c
> +++ b/drivers/clk/clk_scmi.c
> @@ -73,7 +73,13 @@ static int scmi_clk_gate(struct clk *clk, int enable)
>  	if (ret)
>  		return ret;
>  
> -	return scmi_to_linux_errno(out.status);
> +	ret = scmi_to_linux_errno(out.status);
> +	if (ret == -EACCES) {
> +		debug("Ignore %s enable failure\n", clk_hw_get_name(clk));
> +		ret = 0;
> +	}
> +
> +	return ret;
>  }

This seems like a generic change being made globally and not a
work-around for a specific problem on (some?) iMX families. Has this
been tested on other platforms?

-- 
Tom

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

RE: [PATCH 4/4] clk: scmi: Add workaround for set_rate/enable/disable

[Peng Fan]

> Subject: Re: [PATCH 4/4] clk: scmi: Add workaround for
> set_rate/enable/disable
> 
> On Sun, Oct 06, 2024 at 05:38:25PM +0800, alice.guo@oss.nxp.com
> wrote:
> > From: Ye Li <ye.li@nxp.com>
> >
> > Add workaround to set_rate/enable/disable to bus clock that SM will
> > reply DENIED error.
> >
> > Signed-off-by: Ye Li <ye.li@nxp.com>
> > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> 
> In general, please include a cover letter so it will be clearer what the
> overall goal is, especially once merged.
> 
> > ---
> >  drivers/clk/clk_scmi.c | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index
> > a01292c479..a860a653ba 100644
> > --- a/drivers/clk/clk_scmi.c
> > +++ b/drivers/clk/clk_scmi.c
> > @@ -73,7 +73,13 @@ static int scmi_clk_gate(struct clk *clk, int
> enable)
> >  	if (ret)
> >  		return ret;
> >
> > -	return scmi_to_linux_errno(out.status);
> > +	ret = scmi_to_linux_errno(out.status);
> > +	if (ret == -EACCES) {
> > +		debug("Ignore %s enable failure\n",
> clk_hw_get_name(clk));
> > +		ret = 0;
> > +	}
> > +
> > +	return ret;
> >  }
> 
> This seems like a generic change being made globally and not a work-
> around for a specific problem on (some?) iMX families.

We have changed Linux upstream to this behavior, but better
in firmware/clock.c as linux,
https://elixir.bootlin.com/linux/v6.11.2/source/drivers/firmware/arm_scmi/clock.c#L761

Regards,
Peng.

 Has this been
> tested on other platforms?
> 
> --
> Tom

Re: [PATCH 4/4] clk: scmi: Add workaround for set_rate/enable/disable

[Tom Rini]

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

On Tue, Oct 08, 2024 at 03:02:13AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH 4/4] clk: scmi: Add workaround for
> > set_rate/enable/disable
> > 
> > On Sun, Oct 06, 2024 at 05:38:25PM +0800, alice.guo@oss.nxp.com
> > wrote:
> > > From: Ye Li <ye.li@nxp.com>
> > >
> > > Add workaround to set_rate/enable/disable to bus clock that SM will
> > > reply DENIED error.
> > >
> > > Signed-off-by: Ye Li <ye.li@nxp.com>
> > > Signed-off-by: Alice Guo <alice.guo@nxp.com>
> > > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > 
> > In general, please include a cover letter so it will be clearer what the
> > overall goal is, especially once merged.
> > 
> > > ---
> > >  drivers/clk/clk_scmi.c | 21 +++++++++++++++++++--
> > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index
> > > a01292c479..a860a653ba 100644
> > > --- a/drivers/clk/clk_scmi.c
> > > +++ b/drivers/clk/clk_scmi.c
> > > @@ -73,7 +73,13 @@ static int scmi_clk_gate(struct clk *clk, int
> > enable)
> > >  	if (ret)
> > >  		return ret;
> > >
> > > -	return scmi_to_linux_errno(out.status);
> > > +	ret = scmi_to_linux_errno(out.status);
> > > +	if (ret == -EACCES) {
> > > +		debug("Ignore %s enable failure\n",
> > clk_hw_get_name(clk));
> > > +		ret = 0;
> > > +	}
> > > +
> > > +	return ret;
> > >  }
> > 
> > This seems like a generic change being made globally and not a work-
> > around for a specific problem on (some?) iMX families.
> 
> We have changed Linux upstream to this behavior, but better
> in firmware/clock.c as linux,
> https://elixir.bootlin.com/linux/v6.11.2/source/drivers/firmware/arm_scmi/clock.c#L761

Please mention stuff like that in the commit message, as well as the
cover letter overview of what you're changing / why, thanks.

-- 
Tom

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