On 11/23/2023 8:35 PM, Dmitry Baryshkov wrote:
On Thu, 23 Nov 2023 at 10:47, Can Guo <quic_cang@xxxxxxxxxxx> 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.
Signed-off-by: Can Guo <quic_cang@xxxxxxxxxxx>
-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)
You can simply return int from this function. -EINVAL would mean that
the setting was not found. Also this can make max_supported_gear
unused.
I will return int in next version, but I'd like to keep the
max_support_gear, because for platforms which only .tbls is provided (no
overlay case), we need the max_supported_gear to tell whether the
requested submode is exceeding the capability provided by the PHY settings.
+{
+ 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)
+ 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) {
+ *i = j;
+ found = true;
+ floor_max_gear = max_gear;
+ }
We know that the table is sorted. So we can return an index of the
first setting that fits.
For SM8550, it is OK, because no-G5 settings are in overlay[0] and G5
settings are in overlay[1], applying one overlay is a must.
.tbls | support nothing as it is incomplete
.tbls + .tbls_hs_overlay[0] | support G4 and lower gears
.tb.s + .tbls_hs_overlay[1] | support G5
But for previously added platforms, no. I put it this way for two reasons -
1. In case the tables are not sorted.
2. For previously added targets, whose configs support G4 and no-G4:
.tbls | support G3 and lower gears
.tbls + .tbls_hs_overlay | support G4
if we anways return an index of the first setting that fits, for these
targets, the G4 settings would always be programmed, no matter UFS
driver requests for G2/G3/G4. On these targets, as dual UFS init is
there to find the most power saving PHY settings, when UFS driver
requests for G2/G3, .tbls_hs_overlay should NOT be applied. Otherwise,
it defeats all the efforts which Mani had spent for the dual UFS init.
Thanks,
Can Guo.
+ }
+
+ 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;
+ }
+
+ apply_overlay = qmp_ufs_match_gear_overlay(qmp, cfg, &i);
+
qmp_ufs_serdes_init(qmp, &cfg->tbls);
+ if (apply_overlay)
+ qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_overlay[i]);
+
if (qmp->mode == PHY_MODE_UFS_HS_B)
qmp_ufs_serdes_init(qmp, &cfg->tbls_hs_b);
+
qmp_ufs_lanes_init(qmp, &cfg->tbls);
- if (qmp->submode == UFS_HS_G4)
- qmp_ufs_lanes_init(qmp, &cfg->tbls_hs_g4);
+ if (apply_overlay)
+ qmp_ufs_lanes_init(qmp, &cfg->tbls_hs_overlay[i]);
+
qmp_ufs_pcs_init(qmp, &cfg->tbls);
- if (qmp->submode == UFS_HS_G4)
- qmp_ufs_pcs_init(qmp, &cfg->tbls_hs_g4);
+ if (apply_overlay)
+ qmp_ufs_pcs_init(qmp, &cfg->tbls_hs_overlay[i]);
+
+ return 0;
}
static int qmp_ufs_com_init(struct qmp_ufs *qmp)
@@ -1331,7 +1461,9 @@ static int qmp_ufs_power_on(struct phy *phy)
unsigned int val;
int ret;
- qmp_ufs_init_registers(qmp, cfg);
+ ret = qmp_ufs_init_registers(qmp, cfg);
+ if (ret)
+ return ret;
ret = reset_control_deassert(qmp->ufs_reset);
if (ret)
--
2.7.4
