Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix renesas pll clock config #79766

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Fix renesas pll clock config #79766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7067d61
dts: renesas: Separate pll p q r into child node
duynguyenxa Oct 10, 2024
a7fa7aa
manifest: Update hal_renesas commit ID
duynguyenxa Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions boards/renesas/ek_ra8d1/ek_ra8d1.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,23 @@
};

&pll {
clocks = <&xtal>;
div = <2>;
mul = <96 0>;
divp = <2>;
freqp = <DT_FREQ_M(480)>;
divq = <2>;
freqq = <DT_FREQ_M(480)>;
divr = <2>;
freqr = <DT_FREQ_M(480)>;
status = "okay";
pllp {
status = "okay";
};

pllq {
status = "okay";
};

pllr {
status = "okay";
};
};


&sciclk {
clocks = <&pll>;
clocks = <&pllp>;
div = <4>;
status = "okay";
};
Expand Down
23 changes: 13 additions & 10 deletions boards/renesas/ek_ra8m1/ek_ra8m1.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,23 @@
};

&pll {
clocks = <&xtal>;
div = <2>;
mul = <96 0>;
divp = <2>;
freqp = <DT_FREQ_M(480)>;
divq = <2>;
freqq = <DT_FREQ_M(480)>;
divr = <2>;
freqr = <DT_FREQ_M(480)>;
status = "okay";
pllp {
status = "okay";
};

pllq {
status = "okay";
};

pllr {
status = "okay";
};
};


&sciclk {
clocks = <&pll>;
clocks = <&pllp>;
div = <4>;
status = "okay";
};
Expand Down
22 changes: 12 additions & 10 deletions boards/renesas/mck_ra8t1/mck_ra8t1.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,20 +60,22 @@
};

&pll {
clocks = <&xtal>;
div = <2>;
mul = <80 0>;
divp = <2>;
freqp = <DT_FREQ_M(480)>;
divq = <2>;
freqq = <DT_FREQ_M(480)>;
divr = <2>;
freqr = <DT_FREQ_M(480)>;
status = "okay";
pllp {
status = "okay";
};

pllq {
status = "okay";
};

pllr {
status = "okay";
};
};

&sciclk {
clocks = <&pll>;
clocks = <&pllp>;
div = <4>;
status = "okay";
};
Expand Down
4 changes: 2 additions & 2 deletions drivers/clock_control/clock_control_renesas_ra_cgc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ static int clock_control_renesas_ra_get_rate(const struct device *dev, clock_con
}

clk_src_rate = R_BSP_SourceClockHzGet(config->clk_src);
clk_div_val = R_FSP_ClockDividerGet(config->clk_div);
clk_div_val = config->clk_div;
*rate = clk_src_rate / clk_div_val;
return 0;
}
Expand Down Expand Up @@ -94,7 +94,7 @@ static const struct clock_control_driver_api clock_control_reneas_ra_api = {
DT_NODE_HAS_PROP(node_id, clocks), \
(RA_CGC_CLK_SRC(DT_CLOCKS_CTLR(node_id))), \
(RA_CGC_CLK_SRC(DT_CLOCKS_CTLR(DT_PARENT(node_id))))), \
.clk_div = RA_CGC_CLK_DIV(node_id, div, 1)}; \
.clk_div = DT_PROP(node_id, div)}; \
DEVICE_DT_DEFINE(node_id, &clock_control_ra_init_pclk, NULL, NULL, \
&node_id##_cfg, PRE_KERNEL_1, \
CONFIG_KERNEL_INIT_PRIORITY_OBJECTS, \
Expand Down
62 changes: 49 additions & 13 deletions dts/arm/renesas/ra/ra8/r7fa8d1xh.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,30 @@
clocks = <&xtal>;
div = <2>;
mul = <96 0>;
divp = <2>;
freqp = <DT_FREQ_M(480)>;
divq = <2>;
freqq = <DT_FREQ_M(480)>;
divr = <2>;
freqr = <DT_FREQ_M(480)>;

pllp: pllp {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};

pllq: pllq {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};

pllr: pllr {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};
status = "disabled";
};

Expand All @@ -68,12 +86,30 @@
/* PLL2 */
div = <2>;
mul = <96 0>;
divp = <2>;
freqp = <DT_FREQ_M(0)>;
divq = <2>;
freqq = <DT_FREQ_M(0)>;
divr = <2>;
freqr = <DT_FREQ_M(0)>;

pll2p: pll2p {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};

pll2q: pll2q {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};

pll2r: pll2r {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};
status = "disabled";
};

Expand All @@ -84,7 +120,7 @@
reg-names = "MSTPA", "MSTPB","MSTPC",
"MSTPD", "MSTPE";
#clock-cells = <0>;
clocks = <&pll>;
clocks = <&pllp>;
status = "okay";

cpuclk: cpuclk {
Expand Down
62 changes: 49 additions & 13 deletions dts/arm/renesas/ra/ra8/r7fa8m1xh.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,30 @@
clocks = <&xtal>;
div = <2>;
mul = <96 0>;
divp = <2>;
freqp = <DT_FREQ_M(480)>;
divq = <2>;
freqq = <DT_FREQ_M(480)>;
divr = <2>;
freqr = <DT_FREQ_M(480)>;

pllp: pllp {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};

pllq: pllq {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};

pllr: pllr {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};
status = "disabled";
};

Expand All @@ -68,12 +86,30 @@
/* PLL2 */
div = <2>;
mul = <96 0>;
divp = <2>;
freqp = <DT_FREQ_M(0)>;
divq = <2>;
freqq = <DT_FREQ_M(0)>;
divr = <2>;
freqr = <DT_FREQ_M(0)>;

pll2p: pll2p {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};

pll2q: pll2q {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};

pll2r: pll2r {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};
status = "disabled";
};

Expand All @@ -84,7 +120,7 @@
reg-names = "MSTPA", "MSTPB","MSTPC",
"MSTPD", "MSTPE";
#clock-cells = <0>;
clocks = <&pll>;
clocks = <&pllp>;
status = "okay";

cpuclk: cpuclk {
Expand Down
65 changes: 49 additions & 16 deletions dts/arm/renesas/ra/ra8/r7fa8t1xh.dtsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,33 +47,66 @@
pll: pll {
compatible = "renesas,ra-cgc-pll";
#clock-cells = <0>;

/* PLL */
clocks = <&xtal>;
div = <2>;
mul = <80 0>;
divp = <2>;
freqp = <DT_FREQ_M(480)>;
divq = <2>;
freqq = <DT_FREQ_M(480)>;
divr = <2>;
freqr = <DT_FREQ_M(480)>;

pllp: pllp {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};

pllq: pllq {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};

pllr: pllr {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(480)>;
status = "disabled";
#clock-cells = <0>;
};
status = "disabled";
};

pll2: pll2 {
compatible = "renesas,ra-cgc-pll";
#clock-cells = <0>;

/* PLL2 */
div = <2>;
mul = <96 0>;
divp = <2>;
freqp = <DT_FREQ_M(0)>;
divq = <2>;
freqq = <DT_FREQ_M(0)>;
divr = <2>;
freqr = <DT_FREQ_M(0)>;

pll2p: pll2p {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};

pll2q: pll2q {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};

pll2r: pll2r {
compatible = "renesas,ra-cgc-pll-out";
div = <2>;
freq = <DT_FREQ_M(0)>;
status = "disabled";
#clock-cells = <0>;
};
status = "disabled";
};

Expand All @@ -84,7 +117,7 @@
reg-names = "MSTPA", "MSTPB","MSTPC",
"MSTPD", "MSTPE";
#clock-cells = <0>;
clocks = <&pll>;
clocks = <&pllp>;
status = "okay";

cpuclk: cpuclk {
Expand Down
19 changes: 19 additions & 0 deletions dts/bindings/clock/renesas,ra-cgc-pll-out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright (c) 2024 Renesas Electronics Corporation
# SPDX-License-Identifier: Apache-2.0

description: Renesas RA Clock Generation Circuit PLL Clock out line

compatible: "renesas,ra-cgc-pll-out"

include: [clock-controller.yaml, base.yaml]

properties:
div:
required: true
type: int
freq:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frequency originally input to the PLL is multiplied and divided by PLLMUL and PLLDIV, and the divided frequency is then further divided by P, Q, and R to determine the frequency.
Is the frequency set here reflecting PLLMUL and PLLDIV?
I would like to find this calculately if possible, but if it becomes too complicated I think this is fine.

Copy link
Member Author

@duynguyenxa duynguyenxa Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soburi , The fix is not relate to the calculation of PLL P Q R frequency but to separate them as device node so that other node can choose them as clock source, the previous patch only allow the node to choose <&pll> as clock source, now we can set <&pllp>, <&pllq>, <&pllr>. Frequency calculation is as your understanding.
Your request here is to create a macro for the frequency calculation instead of set them on device tree, is my understanding correct ?

Copy link
Member

@soburi soburi Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@duynguyenxa

I understood why the change is necessary.
This change separates PLLP, Q, and R into independent nodes, so I think it becomes necessary to correctly express their relationship with the PLL.

What I'm paying attention to is whether the device tree give an appropriate expression for the hardware configuration.

From the register configuration, it appears that PLLP, Q, and R are dividers that use the PLL as their source, so I don't think they have frequency attributes here, these has only 'ratio'.
Instead, I think we need to calculate the relationship to the PLL.
This is why we need a frequency calculation macro.

I think that a "fixed-factor-clock" like definition can be used for this node.

Copy link
Member Author

@duynguyenxa duynguyenxa Oct 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soburi , I got it, I think for each clock node here we will need a clock driver to provide enough information for the child node, current clock design for now there is no good way to get pll frequency. So now, to keep it simple I would like to keep this implementation,
Next patch for clock driver we will update driver for each clock node and change PLL Q P R into fixed-factor-clock.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is no problem with proceeding as is, but if possible, it may be preferable to incorporate this change into 4.0.0 as well, since this would not result in any changes to the external interface.

required: true
type: int

"#clock-cells":
const: 0
Loading
Loading