| Message ID | 20220222145854.358646-10-krzysztof.kozlowski@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: ufs: add common platform bindings + fixes | expand |
On 22/02/2022 19:16, Bjorn Andersson wrote: > On Tue 22 Feb 06:58 PST 2022, Krzysztof Kozlowski wrote: > >> The 'freq-table-hz' is not correct in DT schema, because '-hz' suffix >> defines uint32 type, not an array. Therefore deprecate 'freq-table-hz' >> and use 'freq-table' instead. >> > > Patch looks good in itself, but why don't we use opp-table to describe > the performance states? > > In particular looking at the two columns of frequencies for various > Qualcomm boards they require different performance-states. > > A concrete example is sm8350.dtsi, which specifies 75MHz and 300MHz as > the first frequency pair. The lower level requires the VDD_CX power rail > to be at least &rpmhpd_opp_low_svs, the higher frequency has a > required-opps of &rpmhpd_opp_nom. > > > As this isn't possible to express in the current binding we've just been > forced to always run at a higher voltage level and kept this in the todo > list. > > But rather than migrating freq-table-hz to freq-table and then having to > introduce an opp table to express the power constraints, could we > perhaps skip the intermediate step? > > Or would you have any other suggestion about how we can represent the > required-opps level together with the freq-table (if that's what we want > to stick with). Usage of OPP tables is interesting solution. It would solve your problem of power rail levels. This would need several opp-tables - one for each clock, which is not a big problem. The problem is that I do not have any UFS hardware (none of my Samsung Exynos boards have UFS... I don't have even arm64 Exynos chips :( ), so implementing it theoretically will be painful. OTOH, I believe that having a working dtschema is very useful. Having dtschema without errors/warnings is even worth some churn/intermediary work. The intermediary work is also not that big. Once proper OPP is implemented, we will have "just" two deprecated properties in the bindings. Best regards, Krzysztof
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c index 87975d1a21c8..2b192477d158 100644 --- a/drivers/scsi/ufs/ufshcd-pltfrm.c +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c @@ -47,8 +47,9 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba) if (cnt <= 0) goto out; - if (!of_get_property(np, "freq-table-hz", &len)) { - dev_info(dev, "freq-table-hz property not specified\n"); + if (!of_get_property(np, "freq-table", &len) || + !of_get_property(np, "freq-table-hz", &len)) { + dev_info(dev, "freq-table property not specified\n"); goto out; } @@ -57,7 +58,7 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba) sz = len / sizeof(*clkfreq); if (sz != 2 * cnt) { - dev_err(dev, "%s len mismatch\n", "freq-table-hz"); + dev_err(dev, "%s len mismatch\n", "freq-table"); ret = -EINVAL; goto out; } @@ -69,12 +70,16 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba) goto out; } - ret = of_property_read_u32_array(np, "freq-table-hz", + ret = of_property_read_u32_array(np, "freq-table", clkfreq, sz); if (ret && (ret != -EINVAL)) { - dev_err(dev, "%s: error reading array %d\n", - "freq-table-hz", ret); - return ret; + ret = of_property_read_u32_array(np, "freq-table-hz", + clkfreq, sz); + if (ret && (ret != -EINVAL)) { + dev_err(dev, "%s: error reading array %d\n", + "freq-table", ret); + return ret; + } } for (i = 0; i < sz; i += 2) { @@ -99,7 +104,7 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba) if (!strcmp(name, "ref_clk")) clki->keep_link_active = true; - dev_dbg(dev, "%s: min %u max %u name %s\n", "freq-table-hz", + dev_dbg(dev, "%s: min %u max %u name %s\n", "freq-table", clki->min_freq, clki->max_freq, clki->name); list_add_tail(&clki->list, &hba->clk_list_head); }
The 'freq-table-hz' is not correct in DT schema, because '-hz' suffix defines uint32 type, not an array. Therefore deprecate 'freq-table-hz' and use 'freq-table' instead. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> --- drivers/scsi/ufs/ufshcd-pltfrm.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)