| Message ID | 20240730-clk-u64-v3-2-4d2b19edaa6e@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: add assigned-clock-rates-u64 | expand |
Quoting Peng Fan (OSS) (2024-07-30 01:57:55) > From: Peng Fan <peng.fan@nxp.com> > > i.MX95 System Management Control Firmware(SCMI) manages the clock > function, it exposes PLL VCO which could support up to 5GHz rate that > exceeds UINT32_MAX. So add assigned-clock-rates-u64 support > to set rate that exceeds UINT32_MAX. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/clk/clk-conf.c | 42 +++++++++++++++++++++++++++++++++++++----- > 1 file changed, 37 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c > index 058420562020..684e0c0738b3 100644 > --- a/drivers/clk/clk-conf.c > +++ b/drivers/clk/clk-conf.c > @@ -81,11 +81,44 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier) > static int __set_clk_rates(struct device_node *node, bool clk_supplier) > { > struct of_phandle_args clkspec; > - int rc, index = 0; > + int rc, count, index; > struct clk *clk; > - u32 rate; > + u32 *rates __free(kfree); > + bool rate_64 = false; > + > + count = of_property_count_u64_elems(node, "assigned-clock-rates-u64"); > + if (count <= 0) { > + count = of_property_count_u32_elems(node, "assigned-clock-rates"); > + if (count <= 0) > + return 0; > + > + rates = kcalloc(count, sizeof(u32), GFP_KERNEL); > + if (!rates) > + return -ENOMEM; > + rc = of_property_read_variable_u32_array(node, > + "assigned-clock-rates", > + rates, > + 1, count); > + } else { > + rates = kcalloc(count, sizeof(u64), GFP_KERNEL); > + if (!rates) > + return -ENOMEM; > + rc = of_property_read_variable_u64_array(node, > + "assigned-clock-rates-u64", > + (u64 *)rates, > + 1, count); > + rate_64 = true; > + } Can this be less indented somehow? u64 *rates_64 __free(kfree) = NULL; u32 *rates __free(kfree) = NULL; int count_64, count; count = of_property_count_u32_elems(node, "assigned-clock-rates"); count_64 = of_property_count_u64_elems(node, "assigned-clock-rates-u64"); if (count_64 > 0) { count = count_64; rates_64 = kcalloc(count, sizeof(*rates_64), GFP_KERNEL); if (!rates_64) return -ENOMEM; rc = of_property_read_u64_array(node, "assigned-clock-rates-u64", rates_64, count); } else if (count > 0) { rates = kcalloc(count, sizeof(*rates), GFP_KERNEL)); if (!rates) return -ENOMEM; rc = of_property_read_u32_array(node, "assigned-clock-rates", rates, count); } else { return 0; } if (rc) return rc; for (index = 0; index < count; index++) { unsigned long rate; if (rates_64) rate = rates_64[index]; else rate = rates[index]; > + > + > + for (index = 0; index < count; index++) { > + unsigned long rate; > + > + if (rate_64) > + rate = ((u64 *)rates)[index]; Please no casts. > + else > + rate = rates[index]; >
Hi Stephen, > Subject: Re: [PATCH v3 2/2] clk: clk-conf: support assigned-clock-rates- > u64 > > Quoting Peng Fan (OSS) (2024-07-30 01:57:55) > > From: Peng Fan <peng.fan@nxp.com> > > > > i.MX95 System Management Control Firmware(SCMI) manages the > clock > > function, it exposes PLL VCO which could support up to 5GHz rate > that > > exceeds UINT32_MAX. So add assigned-clock-rates-u64 support to > set > > rate that exceeds UINT32_MAX. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/clk/clk-conf.c | 42 > > +++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 37 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c index > > 058420562020..684e0c0738b3 100644 > > --- a/drivers/clk/clk-conf.c > > +++ b/drivers/clk/clk-conf.c > > @@ -81,11 +81,44 @@ static int __set_clk_parents(struct > device_node > > *node, bool clk_supplier) static int __set_clk_rates(struct > > device_node *node, bool clk_supplier) { > > struct of_phandle_args clkspec; > > - int rc, index = 0; > > + int rc, count, index; > > struct clk *clk; > > - u32 rate; > > + u32 *rates __free(kfree); > > + bool rate_64 = false; > > + > > + count = of_property_count_u64_elems(node, "assigned-clock- > rates-u64"); > > + if (count <= 0) { > > + count = of_property_count_u32_elems(node, "assigned- > clock-rates"); > > + if (count <= 0) > > + return 0; > > + > > + rates = kcalloc(count, sizeof(u32), GFP_KERNEL); > > + if (!rates) > > + return -ENOMEM; > > + rc = of_property_read_variable_u32_array(node, > > + "assigned-clock-rates", > > + rates, > > + 1, count); > > + } else { > > + rates = kcalloc(count, sizeof(u64), GFP_KERNEL); > > + if (!rates) > > + return -ENOMEM; > > + rc = of_property_read_variable_u64_array(node, > > + "assigned-clock-rates-u64", > > + (u64 *)rates, > > + 1, count); > > + rate_64 = true; > > + } > > Can this be less indented somehow? > > u64 *rates_64 __free(kfree) = NULL; > u32 *rates __free(kfree) = NULL; > int count_64, count; > > count = of_property_count_u32_elems(node, "assigned-clock- > rates"); > count_64 = of_property_count_u64_elems(node, "assigned- > clock-rates-u64"); > if (count_64 > 0) { > count = count_64; > rates_64 = kcalloc(count, sizeof(*rates_64), > GFP_KERNEL); > if (!rates_64) > return -ENOMEM; > > rc = of_property_read_u64_array(node, > "assigned-clock- > rates-u64", > rates_64, count); > } else if (count > 0) { > rates = kcalloc(count, sizeof(*rates), GFP_KERNEL)); > if (!rates) > return -ENOMEM; > > rc = of_property_read_u32_array(node, "assigned- > clock-rates", > rates, count); > } else { > return 0; > } > > if (rc) > return rc; > > for (index = 0; index < count; index++) { > unsigned long rate; > > if (rates_64) > rate = rates_64[index]; > else > rate = rates[index]; > Thanks for writing down the code piece, looks good. > > + > > + > > + for (index = 0; index < count; index++) { > > + unsigned long rate; > > + > > + if (rate_64) > > + rate = ((u64 *)rates)[index]; > > Please no casts. sure. Thanks, Peng. > > > + else > > + rate = rates[index]; > >
diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c index 058420562020..684e0c0738b3 100644 --- a/drivers/clk/clk-conf.c +++ b/drivers/clk/clk-conf.c @@ -81,11 +81,44 @@ static int __set_clk_parents(struct device_node *node, bool clk_supplier) static int __set_clk_rates(struct device_node *node, bool clk_supplier) { struct of_phandle_args clkspec; - int rc, index = 0; + int rc, count, index; struct clk *clk; - u32 rate; + u32 *rates __free(kfree); + bool rate_64 = false; + + count = of_property_count_u64_elems(node, "assigned-clock-rates-u64"); + if (count <= 0) { + count = of_property_count_u32_elems(node, "assigned-clock-rates"); + if (count <= 0) + return 0; + + rates = kcalloc(count, sizeof(u32), GFP_KERNEL); + if (!rates) + return -ENOMEM; + rc = of_property_read_variable_u32_array(node, + "assigned-clock-rates", + rates, + 1, count); + } else { + rates = kcalloc(count, sizeof(u64), GFP_KERNEL); + if (!rates) + return -ENOMEM; + rc = of_property_read_variable_u64_array(node, + "assigned-clock-rates-u64", + (u64 *)rates, + 1, count); + rate_64 = true; + } + + + for (index = 0; index < count; index++) { + unsigned long rate; + + if (rate_64) + rate = ((u64 *)rates)[index]; + else + rate = rates[index]; - of_property_for_each_u32(node, "assigned-clock-rates", rate) { if (rate) { rc = of_parse_phandle_with_args(node, "assigned-clocks", "#clock-cells", index, &clkspec); @@ -112,12 +145,11 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) rc = clk_set_rate(clk, rate); if (rc < 0) - pr_err("clk: couldn't set %s clk rate to %u (%d), current rate: %lu\n", + pr_err("clk: couldn't set %s clk rate to %lu (%d), current rate: %lu\n", __clk_get_name(clk), rate, rc, clk_get_rate(clk)); clk_put(clk); } - index++; } return 0; }