On 11/28/2023 2:47 PM, Manivannan Sadhasivam wrote:
On Thu, Nov 23, 2023 at 12:46:29AM -0800, Can Guo wrote:
On SM8550, two sets of UFS PHY settings are provided, one set is to support
HS-G5, another set is to support HS-G4 and lower gears. The two sets of PHY
settings are programming different values to different registers, mixing
the two sets and/or overwriting one set with another set is definitely not
blessed by UFS PHY designers.
To add HS-G5 support for SM8550, split the two sets of PHY settings into
their dedicated overlay tables, only the common parts of the two sets of
PHY settings are left in the .tbls.
Consider we are going to add even higher gear support in future, to avoid
adding more tables with different names, rename the .tbls_hs_g4 and make it
an array, a size of 2 is enough as of now.
In this case, .tbls alone is not a complete set of PHY settings, so either
tbls_hs_overlay[0] or tbls_hs_overlay[1] must be applied on top of the
.tbls to become a complete set of PHY settings.
Thanks for the update! This really helps in minimizing the changes for future
gears.
Signed-off-by: Can Guo <quic_cang@xxxxxxxxxxx>
---
drivers/phy/qualcomm/phy-qcom-qmp-pcs-ufs-v6.h | 2 +
drivers/phy/qualcomm/phy-qcom-qmp-qserdes-com-v6.h | 2 +
.../qualcomm/phy-qcom-qmp-qserdes-txrx-ufs-v6.h | 9 ++
drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 174 ++++++++++++++++++---
4 files changed, 166 insertions(+), 21 deletions(-)
[...]
-static void qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
+static bool qmp_ufs_match_gear_overlay(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg, int *i)
+{
+ u32 max_gear, floor_max_gear = cfg->max_supported_gear;
+ bool found = false;
+ int j;
+
+ for (j = 0; j < NUM_OVERLAY; j ++) {
+ max_gear = cfg->tbls_hs_overlay[j].max_gear;
+
+ if (max_gear == 0)
Is this condition possible for hs_overlay tables?
Yes, now there are 2 overlays for SM8550, but only one overlay for the
rest targets. For those who has only one overlay, this check fits them.
+ continue;
+
+ /* Direct matching, bail */
+ if (qmp->submode == max_gear) {
+ *i = j;
+ return true;
+ }
+
+ /* If no direct matching, the lowest gear is the best matching */
+ if (max_gear < floor_max_gear) {
Can you start the loop from max? If looks odd to set the matching params in the
first iteration itself and then checking the next one.
OK, will start from j = NUM_VERLAY - 1; in next version. When I wrote
the code, I was not expecting the max is always in the last overlay,
they can come in any orders.
+ *i = j;
+ found = true;
+ floor_max_gear = max_gear;
+ }
+ }
+
+ return found;
+}
+
+static int qmp_ufs_init_registers(struct qmp_ufs *qmp, const struct qmp_phy_cfg *cfg)
{
+ bool apply_overlay;
+ int i;
+
+ if (qmp->submode > cfg->max_supported_gear || qmp->submode == 0) {
+ dev_err(qmp->dev, "Invalid PHY submode %u\n", qmp->submode);
+ return -EINVAL;
+ }
This check should be moved to qmp_ufs_set_mode().
OK.
Thanks,
Can Guo.
Rest LGTM.
- Mani
