| Message ID | 20230306170817.3806-4-they@mint.lgbt |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: qcom: sm6125: UFS and xiaomi-laurel-sprout support | expand |
On 6.03.2023 18:08, Lux Aliaga wrote: > The SM6125 UFS PHY is compatible with the one from SM6115. Add a > compatible for it and modify the config from SM6115 to make them > compatible with the SC8280XP binding > > Signed-off-by: Lux Aliaga <they@mint.lgbt> > Reviewed-by: Martin Botka <martin.botka@somainline.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > index 318eea35b972..44c29fdfc551 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > @@ -620,6 +620,13 @@ static const char * const qmp_phy_vreg_l[] = { > "vdda-phy", "vdda-pll", > }; > > +static const struct qmp_ufs_offsets qmp_ufs_offsets_v3_660 = { > + .serdes = 0, > + .pcs = 0xc00, > + .tx = 0x400, > + .rx = 0x600, > +}; > + > static const struct qmp_ufs_offsets qmp_ufs_offsets_v5 = { > .serdes = 0, > .pcs = 0xc00, > @@ -693,6 +700,8 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = { > static const struct qmp_phy_cfg sm6115_ufsphy_cfg = { > .lanes = 1, > > + .offsets = &qmp_ufs_offsets_v3_660, Will this not trigger OOB r/w for the users of qcom,sm6115-smp-ufs-phy which specify the regions separately (old binding style)? Konrad > + > .serdes_tbl = sm6115_ufsphy_serdes_tbl, > .serdes_tbl_num = ARRAY_SIZE(sm6115_ufsphy_serdes_tbl), > .tx_tbl = sm6115_ufsphy_tx_tbl, > @@ -1172,6 +1181,9 @@ static const struct of_device_id qmp_ufs_of_match_table[] = { > }, { > .compatible = "qcom,sm6115-qmp-ufs-phy", > .data = &sm6115_ufsphy_cfg, > + }, { > + .compatible = "qcom,sm6125-qmp-ufs-phy", > + .data = &sm6115_ufsphy_cfg, > }, { > .compatible = "qcom,sm6350-qmp-ufs-phy", > .data = &sdm845_ufsphy_cfg,
On Wed, Mar 08, 2023 at 11:09:48AM +0100, Konrad Dybcio wrote: > > > On 6.03.2023 18:08, Lux Aliaga wrote: > > The SM6125 UFS PHY is compatible with the one from SM6115. Add a > > compatible for it and modify the config from SM6115 to make them > > compatible with the SC8280XP binding > > > > Signed-off-by: Lux Aliaga <they@mint.lgbt> > > Reviewed-by: Martin Botka <martin.botka@somainline.org> > > --- > > drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > index 318eea35b972..44c29fdfc551 100644 > > --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > @@ -620,6 +620,13 @@ static const char * const qmp_phy_vreg_l[] = { > > "vdda-phy", "vdda-pll", > > }; > > > > +static const struct qmp_ufs_offsets qmp_ufs_offsets_v3_660 = { > > + .serdes = 0, > > + .pcs = 0xc00, > > + .tx = 0x400, > > + .rx = 0x600, > > +}; > > + > > static const struct qmp_ufs_offsets qmp_ufs_offsets_v5 = { > > .serdes = 0, > > .pcs = 0xc00, > > @@ -693,6 +700,8 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = { > > static const struct qmp_phy_cfg sm6115_ufsphy_cfg = { > > .lanes = 1, > > > > + .offsets = &qmp_ufs_offsets_v3_660, > Will this not trigger OOB r/w for the users of qcom,sm6115-smp-ufs-phy > which specify the regions separately (old binding style)? No, that should work fine. But looks like this series needs to be rebased on 6.3-rc1 as these offsets are now already set in mainline. Johan
On Wed, Mar 08, 2023 at 12:15:39PM +0100, Konrad Dybcio wrote: > > > On 8.03.2023 12:02, Johan Hovold wrote: > > On Wed, Mar 08, 2023 at 11:09:48AM +0100, Konrad Dybcio wrote: > >> > >> > >> On 6.03.2023 18:08, Lux Aliaga wrote: > >>> The SM6125 UFS PHY is compatible with the one from SM6115. Add a > >>> compatible for it and modify the config from SM6115 to make them > >>> compatible with the SC8280XP binding > >>> > >>> Signed-off-by: Lux Aliaga <they@mint.lgbt> > >>> Reviewed-by: Martin Botka <martin.botka@somainline.org> > >>> --- > >>> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 12 ++++++++++++ > >>> 1 file changed, 12 insertions(+) > >>> > >>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > >>> index 318eea35b972..44c29fdfc551 100644 > >>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > >>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > >>> @@ -620,6 +620,13 @@ static const char * const qmp_phy_vreg_l[] = { > >>> "vdda-phy", "vdda-pll", > >>> }; > >>> > >>> +static const struct qmp_ufs_offsets qmp_ufs_offsets_v3_660 = { > >>> + .serdes = 0, > >>> + .pcs = 0xc00, > >>> + .tx = 0x400, > >>> + .rx = 0x600, > >>> +}; > >>> + > >>> static const struct qmp_ufs_offsets qmp_ufs_offsets_v5 = { > >>> .serdes = 0, > >>> .pcs = 0xc00, > >>> @@ -693,6 +700,8 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = { > >>> static const struct qmp_phy_cfg sm6115_ufsphy_cfg = { > >>> .lanes = 1, > >>> > >>> + .offsets = &qmp_ufs_offsets_v3_660, > >> Will this not trigger OOB r/w for the users of qcom,sm6115-smp-ufs-phy > >> which specify the regions separately (old binding style)? > > > > No, that should work fine. > So do you think the SM6115 binding could be updated too? Or should > we keep it as-is for ABI purposes?.. They could be and the possibility has been raised. I think it may be more important to convert the old combo-phy binding (it's on my list, but I keep getting preempted), but at some point we can get rid of the legacy UFS binding as well. > > But looks like this series needs to be rebased on 6.3-rc1 as these > > offsets are now already set in mainline. > ..Or did you do that already and I can't find it? It seems a previous version of this patch was merged almost two months ago. 9b9e29af984c ("phy: qcom-qmp: Add SM6125 UFS PHY support") Not sure what failed here. Johan
On 8.03.2023 12:23, Johan Hovold wrote: > On Wed, Mar 08, 2023 at 12:15:39PM +0100, Konrad Dybcio wrote: >> >> >> On 8.03.2023 12:02, Johan Hovold wrote: >>> On Wed, Mar 08, 2023 at 11:09:48AM +0100, Konrad Dybcio wrote: >>>> >>>> >>>> On 6.03.2023 18:08, Lux Aliaga wrote: >>>>> The SM6125 UFS PHY is compatible with the one from SM6115. Add a >>>>> compatible for it and modify the config from SM6115 to make them >>>>> compatible with the SC8280XP binding >>>>> >>>>> Signed-off-by: Lux Aliaga <they@mint.lgbt> >>>>> Reviewed-by: Martin Botka <martin.botka@somainline.org> >>>>> --- >>>>> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 12 ++++++++++++ >>>>> 1 file changed, 12 insertions(+) >>>>> >>>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >>>>> index 318eea35b972..44c29fdfc551 100644 >>>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >>>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >>>>> @@ -620,6 +620,13 @@ static const char * const qmp_phy_vreg_l[] = { >>>>> "vdda-phy", "vdda-pll", >>>>> }; >>>>> >>>>> +static const struct qmp_ufs_offsets qmp_ufs_offsets_v3_660 = { >>>>> + .serdes = 0, >>>>> + .pcs = 0xc00, >>>>> + .tx = 0x400, >>>>> + .rx = 0x600, >>>>> +}; >>>>> + >>>>> static const struct qmp_ufs_offsets qmp_ufs_offsets_v5 = { >>>>> .serdes = 0, >>>>> .pcs = 0xc00, >>>>> @@ -693,6 +700,8 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = { >>>>> static const struct qmp_phy_cfg sm6115_ufsphy_cfg = { >>>>> .lanes = 1, >>>>> >>>>> + .offsets = &qmp_ufs_offsets_v3_660, >>>> Will this not trigger OOB r/w for the users of qcom,sm6115-smp-ufs-phy >>>> which specify the regions separately (old binding style)? >>> >>> No, that should work fine. >> So do you think the SM6115 binding could be updated too? Or should >> we keep it as-is for ABI purposes?.. > > They could be and the possibility has been raised. I think it may be > more important to convert the old combo-phy binding (it's on my list, > but I keep getting preempted), but at some point we can get rid of the > legacy UFS binding as well. Okay sounds good! > >>> But looks like this series needs to be rebased on 6.3-rc1 as these >>> offsets are now already set in mainline. >> ..Or did you do that already and I can't find it? > > It seems a previous version of this patch was merged almost two months > ago. > > 9b9e29af984c ("phy: qcom-qmp: Add SM6125 UFS PHY support") > > Not sure what failed here. My eyes :) Konrad > > Johan
On Wed, Mar 08, 2023 at 08:37:48AM -0300, Lux Aliaga wrote: > On 8 March 2023 08:23:57 GMT-03:00, Johan Hovold <johan@kernel.org> wrote: > >It seems a previous version of this patch was merged almost two months > >ago. > > > > 9b9e29af984c ("phy: qcom-qmp: Add SM6125 UFS PHY support") > > > >Not sure what failed here. > Yes, but it received some comments regarding using v5 offsets instead > of v3-660. I could spin off this change into a new patch if necessary. Once a patch has been applied, you generally need to do any further changes incrementally on top. It seems Dmitry renamed the struct himself after the patch was applied in this case. Johan
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c index 318eea35b972..44c29fdfc551 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c @@ -620,6 +620,13 @@ static const char * const qmp_phy_vreg_l[] = { "vdda-phy", "vdda-pll", }; +static const struct qmp_ufs_offsets qmp_ufs_offsets_v3_660 = { + .serdes = 0, + .pcs = 0xc00, + .tx = 0x400, + .rx = 0x600, +}; + static const struct qmp_ufs_offsets qmp_ufs_offsets_v5 = { .serdes = 0, .pcs = 0xc00, @@ -693,6 +700,8 @@ static const struct qmp_phy_cfg sdm845_ufsphy_cfg = { static const struct qmp_phy_cfg sm6115_ufsphy_cfg = { .lanes = 1, + .offsets = &qmp_ufs_offsets_v3_660, + .serdes_tbl = sm6115_ufsphy_serdes_tbl, .serdes_tbl_num = ARRAY_SIZE(sm6115_ufsphy_serdes_tbl), .tx_tbl = sm6115_ufsphy_tx_tbl, @@ -1172,6 +1181,9 @@ static const struct of_device_id qmp_ufs_of_match_table[] = { }, { .compatible = "qcom,sm6115-qmp-ufs-phy", .data = &sm6115_ufsphy_cfg, + }, { + .compatible = "qcom,sm6125-qmp-ufs-phy", + .data = &sm6115_ufsphy_cfg, }, { .compatible = "qcom,sm6350-qmp-ufs-phy", .data = &sdm845_ufsphy_cfg,