* [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework @ 2024-01-04 14:52 Abel Vesa 2024-01-04 14:52 ` [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances Abel Vesa ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Abel Vesa @ 2024-01-04 14:52 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I Cc: linux-arm-msm, linux-phy, linux-kernel, Abel Vesa Found the first issue (from first patch) while adding support for X Elite (X1E80100) which comes with more than one repeaters. The second fix is just bonus. --- Abel Vesa (2): phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances phy: qualcomm: eusb2-repeater: Drop the redundant zeroing drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) --- base-commit: 0fef202ac2f8e6d9ad21aead648278f1226b9053 change-id: 20240104-phy-qcom-eusb2-repeater-fixes-c9201113032c Best regards, -- Abel Vesa <abel.vesa@linaro.org> -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances 2024-01-04 14:52 [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework Abel Vesa @ 2024-01-04 14:52 ` Abel Vesa 2024-01-04 19:12 ` Abel Vesa 2024-01-04 22:46 ` Konrad Dybcio 2024-01-04 14:52 ` [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing Abel Vesa 2024-01-04 22:58 ` [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework Elliot Berman 2 siblings, 2 replies; 11+ messages in thread From: Abel Vesa @ 2024-01-04 14:52 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I Cc: linux-arm-msm, linux-phy, linux-kernel, Abel Vesa The global regmap fields offsets currently get incremented with the base address of the repeater. This issue doesn't get noticed unless the probe defers or there are multiple repeaters on that platform. So instead of incrementing the global ones, copy them for each instance of the repeater. Fixes: 4ba2e52718c0 ("phy: qualcomm: phy-qcom-eusb2-repeater: Use regmap_fields") Signed-off-by: Abel Vesa <abel.vesa@linaro.org> --- drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c index a623f092b11f..5f5862a68b73 100644 --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c @@ -101,6 +101,7 @@ struct eusb2_repeater { struct regmap_field *regs[F_NUM_FIELDS]; struct phy *phy; struct regulator_bulk_data *vregs; + struct reg_field *regfields; const struct eusb2_repeater_cfg *cfg; enum phy_mode mode; }; @@ -140,8 +141,8 @@ static int eusb2_repeater_init_vregs(struct eusb2_repeater *rptr) static int eusb2_repeater_init(struct phy *phy) { - struct reg_field *regfields = eusb2_repeater_tune_reg_fields; struct eusb2_repeater *rptr = phy_get_drvdata(phy); + struct reg_field *regfields = rptr->regfields; struct device_node *np = rptr->dev->of_node; u32 init_tbl[F_NUM_TUNE_FIELDS] = { 0 }; u8 override; @@ -262,15 +263,21 @@ static int eusb2_repeater_probe(struct platform_device *pdev) if (!regmap) return -ENODEV; + rptr->regfields = devm_kmemdup(dev, eusb2_repeater_tune_reg_fields, + sizeof(eusb2_repeater_tune_reg_fields), + GFP_KERNEL); + if (!rptr->regfields) + return -ENOMEM; + ret = of_property_read_u32(np, "reg", &res); if (ret < 0) return ret; for (i = 0; i < F_NUM_FIELDS; i++) - eusb2_repeater_tune_reg_fields[i].reg += res; + rptr->regfields[i].reg += res; ret = devm_regmap_field_bulk_alloc(dev, regmap, rptr->regs, - eusb2_repeater_tune_reg_fields, + rptr->regfields, F_NUM_FIELDS); if (ret) return ret; -- 2.34.1 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances 2024-01-04 14:52 ` [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances Abel Vesa @ 2024-01-04 19:12 ` Abel Vesa 2024-01-04 22:46 ` Konrad Dybcio 1 sibling, 0 replies; 11+ messages in thread From: Abel Vesa @ 2024-01-04 19:12 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I Cc: linux-arm-msm, linux-phy, linux-kernel On 24-01-04 16:52:11, Abel Vesa wrote: > The global regmap fields offsets currently get incremented with the base > address of the repeater. This issue doesn't get noticed unless the probe > defers or there are multiple repeaters on that platform. So instead of > incrementing the global ones, copy them for each instance of the > repeater. > > Fixes: 4ba2e52718c0 ("phy: qualcomm: phy-qcom-eusb2-repeater: Use regmap_fields") > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- > drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > index a623f092b11f..5f5862a68b73 100644 > --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > @@ -101,6 +101,7 @@ struct eusb2_repeater { > struct regmap_field *regs[F_NUM_FIELDS]; > struct phy *phy; > struct regulator_bulk_data *vregs; > + struct reg_field *regfields; > const struct eusb2_repeater_cfg *cfg; > enum phy_mode mode; > }; > @@ -140,8 +141,8 @@ static int eusb2_repeater_init_vregs(struct eusb2_repeater *rptr) > > static int eusb2_repeater_init(struct phy *phy) > { > - struct reg_field *regfields = eusb2_repeater_tune_reg_fields; > struct eusb2_repeater *rptr = phy_get_drvdata(phy); > + struct reg_field *regfields = rptr->regfields; Oups, this is not needed. Will drop after I get some more comments on this. > struct device_node *np = rptr->dev->of_node; > u32 init_tbl[F_NUM_TUNE_FIELDS] = { 0 }; > u8 override; > @@ -262,15 +263,21 @@ static int eusb2_repeater_probe(struct platform_device *pdev) > if (!regmap) > return -ENODEV; > > + rptr->regfields = devm_kmemdup(dev, eusb2_repeater_tune_reg_fields, > + sizeof(eusb2_repeater_tune_reg_fields), > + GFP_KERNEL); > + if (!rptr->regfields) > + return -ENOMEM; > + > ret = of_property_read_u32(np, "reg", &res); > if (ret < 0) > return ret; > > for (i = 0; i < F_NUM_FIELDS; i++) > - eusb2_repeater_tune_reg_fields[i].reg += res; > + rptr->regfields[i].reg += res; > > ret = devm_regmap_field_bulk_alloc(dev, regmap, rptr->regs, > - eusb2_repeater_tune_reg_fields, > + rptr->regfields, > F_NUM_FIELDS); > if (ret) > return ret; > > -- > 2.34.1 > -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances 2024-01-04 14:52 ` [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances Abel Vesa 2024-01-04 19:12 ` Abel Vesa @ 2024-01-04 22:46 ` Konrad Dybcio 1 sibling, 0 replies; 11+ messages in thread From: Konrad Dybcio @ 2024-01-04 22:46 UTC (permalink / raw) To: Abel Vesa, Bjorn Andersson, Vinod Koul, Kishon Vijay Abraham I Cc: linux-arm-msm, linux-phy, linux-kernel On 4.01.2024 15:52, Abel Vesa wrote: > The global regmap fields offsets currently get incremented with the base > address of the repeater. This issue doesn't get noticed unless the probe > defers or there are multiple repeaters on that platform. So instead of > incrementing the global ones, copy them for each instance of the > repeater. > > Fixes: 4ba2e52718c0 ("phy: qualcomm: phy-qcom-eusb2-repeater: Use regmap_fields") > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- Ohh I wouldn't have thought about this.. Nice spot! [...] > - struct reg_field *regfields = eusb2_repeater_tune_reg_fields; > struct eusb2_repeater *rptr = phy_get_drvdata(phy); > + struct reg_field *regfields = rptr->regfields; Without this: Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing 2024-01-04 14:52 [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework Abel Vesa 2024-01-04 14:52 ` [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances Abel Vesa @ 2024-01-04 14:52 ` Abel Vesa 2024-01-04 22:50 ` Konrad Dybcio 2024-01-06 17:30 ` kernel test robot 2024-01-04 22:58 ` [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework Elliot Berman 2 siblings, 2 replies; 11+ messages in thread From: Abel Vesa @ 2024-01-04 14:52 UTC (permalink / raw) To: Bjorn Andersson, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I Cc: linux-arm-msm, linux-phy, linux-kernel, Abel Vesa The local init_tlb is already zero initialized, so the entire zeroing loop is useless in this case, since the initial values are copied over anyway, before being written. Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs") Signed-off-by: Abel Vesa <abel.vesa@linaro.org> --- drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c index 5f5862a68b73..3060c0749797 100644 --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c @@ -156,16 +156,6 @@ static int eusb2_repeater_init(struct phy *phy) regmap_field_update_bits(rptr->regs[F_EN_CTL1], EUSB2_RPTR_EN, EUSB2_RPTR_EN); - for (i = 0; i < F_NUM_TUNE_FIELDS; i++) { - if (init_tbl[i]) { - regmap_field_update_bits(rptr->regs[i], init_tbl[i], init_tbl[i]); - } else { - /* Write 0 if there's no value set */ - u32 mask = GENMASK(regfields[i].msb, regfields[i].lsb); - - regmap_field_update_bits(rptr->regs[i], mask, 0); - } - } memcpy(init_tbl, rptr->cfg->init_tbl, sizeof(init_tbl)); if (!of_property_read_u8(np, "qcom,tune-usb2-amplitude", &override)) -- 2.34.1 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing 2024-01-04 14:52 ` [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing Abel Vesa @ 2024-01-04 22:50 ` Konrad Dybcio 2024-01-05 9:16 ` Abel Vesa 2024-01-06 17:30 ` kernel test robot 1 sibling, 1 reply; 11+ messages in thread From: Konrad Dybcio @ 2024-01-04 22:50 UTC (permalink / raw) To: Abel Vesa, Bjorn Andersson, Vinod Koul, Kishon Vijay Abraham I Cc: linux-arm-msm, linux-phy, linux-kernel On 4.01.2024 15:52, Abel Vesa wrote: > The local init_tlb is already zero initialized, so the entire zeroing loop > is useless in this case, since the initial values are copied over anyway, > before being written. > > Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs") > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > --- That's another good spot.. partial struct initialization of pm8550b_init_tbl zeroes out the uninitialized fields. > drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > index 5f5862a68b73..3060c0749797 100644 > --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > @@ -156,16 +156,6 @@ static int eusb2_repeater_init(struct phy *phy) > > regmap_field_update_bits(rptr->regs[F_EN_CTL1], EUSB2_RPTR_EN, EUSB2_RPTR_EN); > > - for (i = 0; i < F_NUM_TUNE_FIELDS; i++) { > - if (init_tbl[i]) { > - regmap_field_update_bits(rptr->regs[i], init_tbl[i], init_tbl[i]); > - } else { > - /* Write 0 if there's no value set */ > - u32 mask = GENMASK(regfields[i].msb, regfields[i].lsb); > - > - regmap_field_update_bits(rptr->regs[i], mask, 0); > - } > - } > memcpy(init_tbl, rptr->cfg->init_tbl, sizeof(init_tbl)); I think this patchset can be made even better, this memcpy is also useless and we can simply initialize init_tbl=rptr->cfg->init_tbl. Konrad -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing 2024-01-04 22:50 ` Konrad Dybcio @ 2024-01-05 9:16 ` Abel Vesa 2024-01-05 10:09 ` Konrad Dybcio 0 siblings, 1 reply; 11+ messages in thread From: Abel Vesa @ 2024-01-05 9:16 UTC (permalink / raw) To: Konrad Dybcio Cc: Bjorn Andersson, Vinod Koul, Kishon Vijay Abraham I, linux-arm-msm, linux-phy, linux-kernel On 24-01-04 23:50:48, Konrad Dybcio wrote: > On 4.01.2024 15:52, Abel Vesa wrote: > > The local init_tlb is already zero initialized, so the entire zeroing loop > > is useless in this case, since the initial values are copied over anyway, > > before being written. > > > > Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs") > > Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > > --- > > That's another good spot.. partial struct initialization of > pm8550b_init_tbl zeroes out the uninitialized fields. > > > > drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 10 ---------- > > 1 file changed, 10 deletions(-) > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > > index 5f5862a68b73..3060c0749797 100644 > > --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > > +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > > @@ -156,16 +156,6 @@ static int eusb2_repeater_init(struct phy *phy) > > > > regmap_field_update_bits(rptr->regs[F_EN_CTL1], EUSB2_RPTR_EN, EUSB2_RPTR_EN); > > > > - for (i = 0; i < F_NUM_TUNE_FIELDS; i++) { > > - if (init_tbl[i]) { > > - regmap_field_update_bits(rptr->regs[i], init_tbl[i], init_tbl[i]); > > - } else { > > - /* Write 0 if there's no value set */ > > - u32 mask = GENMASK(regfields[i].msb, regfields[i].lsb); > > - > > - regmap_field_update_bits(rptr->regs[i], mask, 0); > > - } > > - } > > memcpy(init_tbl, rptr->cfg->init_tbl, sizeof(init_tbl)); > > I think this patchset can be made even better, this memcpy is also > useless and we can simply initialize init_tbl=rptr->cfg->init_tbl. Actually no. The init_tbl in cfg is a pointer to const. Plus, if you do that, you will end up with the same situation like in the other patch, as there are some overrides based on DT values below this. But now that I've had another look, maybe doing the exact same thing as the other patch does (kmemdup) will probably look better anyway, specially if we do that on probe. > > Konrad -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing 2024-01-05 9:16 ` Abel Vesa @ 2024-01-05 10:09 ` Konrad Dybcio 2024-01-05 10:19 ` Abel Vesa 0 siblings, 1 reply; 11+ messages in thread From: Konrad Dybcio @ 2024-01-05 10:09 UTC (permalink / raw) To: Abel Vesa Cc: Bjorn Andersson, Vinod Koul, Kishon Vijay Abraham I, linux-arm-msm, linux-phy, linux-kernel On 5.01.2024 10:16, Abel Vesa wrote: > On 24-01-04 23:50:48, Konrad Dybcio wrote: >> On 4.01.2024 15:52, Abel Vesa wrote: >>> The local init_tlb is already zero initialized, so the entire zeroing loop >>> is useless in this case, since the initial values are copied over anyway, >>> before being written. >>> >>> Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs") >>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> >>> --- >> >> That's another good spot.. partial struct initialization of >> pm8550b_init_tbl zeroes out the uninitialized fields. >> >> >>> drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 10 ---------- >>> 1 file changed, 10 deletions(-) >>> >>> diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c >>> index 5f5862a68b73..3060c0749797 100644 >>> --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c >>> +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c >>> @@ -156,16 +156,6 @@ static int eusb2_repeater_init(struct phy *phy) >>> >>> regmap_field_update_bits(rptr->regs[F_EN_CTL1], EUSB2_RPTR_EN, EUSB2_RPTR_EN); >>> >>> - for (i = 0; i < F_NUM_TUNE_FIELDS; i++) { >>> - if (init_tbl[i]) { >>> - regmap_field_update_bits(rptr->regs[i], init_tbl[i], init_tbl[i]); >>> - } else { >>> - /* Write 0 if there's no value set */ >>> - u32 mask = GENMASK(regfields[i].msb, regfields[i].lsb); >>> - >>> - regmap_field_update_bits(rptr->regs[i], mask, 0); >>> - } >>> - } >>> memcpy(init_tbl, rptr->cfg->init_tbl, sizeof(init_tbl)); >> >> I think this patchset can be made even better, this memcpy is also >> useless and we can simply initialize init_tbl=rptr->cfg->init_tbl. > > Actually no. The init_tbl in cfg is a pointer to const. Plus, if you do > that, you will end up with the same situation like in the other patch, > as there are some overrides based on DT values below this. Hm, you're right. Maybe we should simple store *base and stop modifying these tables then. There's not a whole lot of regmap_rw calls, so making the first argument "rptr->base + rptr->regs[ASDF]" shouldn't add much fluff. Then we can make the cfg referernce const. Konrad > > But now that I've had another look, maybe doing the exact same thing as > the other patch does (kmemdup) will probably look better anyway, > specially if we do that on probe. > >> >> Konrad -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing 2024-01-05 10:09 ` Konrad Dybcio @ 2024-01-05 10:19 ` Abel Vesa 0 siblings, 0 replies; 11+ messages in thread From: Abel Vesa @ 2024-01-05 10:19 UTC (permalink / raw) To: Konrad Dybcio Cc: Bjorn Andersson, Vinod Koul, Kishon Vijay Abraham I, linux-arm-msm, linux-phy, linux-kernel On 24-01-05 11:09:33, Konrad Dybcio wrote: > On 5.01.2024 10:16, Abel Vesa wrote: > > On 24-01-04 23:50:48, Konrad Dybcio wrote: > >> On 4.01.2024 15:52, Abel Vesa wrote: > >>> The local init_tlb is already zero initialized, so the entire zeroing loop > >>> is useless in this case, since the initial values are copied over anyway, > >>> before being written. > >>> > >>> Fixes: 99a517a582fc ("phy: qualcomm: phy-qcom-eusb2-repeater: Zero out untouched tuning regs") > >>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org> > >>> --- > >> > >> That's another good spot.. partial struct initialization of > >> pm8550b_init_tbl zeroes out the uninitialized fields. > >> > >> > >>> drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 10 ---------- > >>> 1 file changed, 10 deletions(-) > >>> > >>> diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > >>> index 5f5862a68b73..3060c0749797 100644 > >>> --- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > >>> +++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c > >>> @@ -156,16 +156,6 @@ static int eusb2_repeater_init(struct phy *phy) > >>> > >>> regmap_field_update_bits(rptr->regs[F_EN_CTL1], EUSB2_RPTR_EN, EUSB2_RPTR_EN); > >>> > >>> - for (i = 0; i < F_NUM_TUNE_FIELDS; i++) { > >>> - if (init_tbl[i]) { > >>> - regmap_field_update_bits(rptr->regs[i], init_tbl[i], init_tbl[i]); > >>> - } else { > >>> - /* Write 0 if there's no value set */ > >>> - u32 mask = GENMASK(regfields[i].msb, regfields[i].lsb); > >>> - > >>> - regmap_field_update_bits(rptr->regs[i], mask, 0); > >>> - } > >>> - } > >>> memcpy(init_tbl, rptr->cfg->init_tbl, sizeof(init_tbl)); > >> > >> I think this patchset can be made even better, this memcpy is also > >> useless and we can simply initialize init_tbl=rptr->cfg->init_tbl. > > > > Actually no. The init_tbl in cfg is a pointer to const. Plus, if you do > > that, you will end up with the same situation like in the other patch, > > as there are some overrides based on DT values below this. > > Hm, you're right. Maybe we should simple store *base and stop > modifying these tables then. There's not a whole lot of regmap_rw > calls, so making the first argument "rptr->base + rptr->regs[ASDF]" > shouldn't add much fluff. Then we can make the cfg referernce const. > Oh, sorry, did not see your reply in time before sending v2. Have a look at v2 and we can decide if we want something different then. https://lore.kernel.org/all/20240105-phy-qcom-eusb2-repeater-fixes-v2-0-775d98e7df05@linaro.org/ Thanks for reviewing. > Konrad > > > > > But now that I've had another look, maybe doing the exact same thing as > > the other patch does (kmemdup) will probably look better anyway, > > specially if we do that on probe. > > > >> > >> Konrad -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing 2024-01-04 14:52 ` [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing Abel Vesa 2024-01-04 22:50 ` Konrad Dybcio @ 2024-01-06 17:30 ` kernel test robot 1 sibling, 0 replies; 11+ messages in thread From: kernel test robot @ 2024-01-06 17:30 UTC (permalink / raw) To: Abel Vesa, Bjorn Andersson, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I Cc: oe-kbuild-all, linux-arm-msm, linux-phy, linux-kernel, Abel Vesa Hi Abel, kernel test robot noticed the following build warnings: [auto build test WARNING on 0fef202ac2f8e6d9ad21aead648278f1226b9053] url: https://github.com/intel-lab-lkp/linux/commits/Abel-Vesa/phy-qualcomm-eusb2-repeater-Fix-the-regfields-for-multiple-instances/20240104-225513 base: 0fef202ac2f8e6d9ad21aead648278f1226b9053 patch link: https://lore.kernel.org/r/20240104-phy-qcom-eusb2-repeater-fixes-v1-2-047b7b6b8333%40linaro.org patch subject: [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing config: i386-buildonly-randconfig-004-20240106 (https://download.01.org/0day-ci/archive/20240107/202401070143.pnnuXAwC-lkp@intel.com/config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240107/202401070143.pnnuXAwC-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202401070143.pnnuXAwC-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c: In function 'eusb2_repeater_init': >> drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c:145:20: warning: unused variable 'regfields' [-Wunused-variable] struct reg_field *regfields = rptr->regfields; ^~~~~~~~~ vim +/regfields +145 drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c 56d77c9a10d97d Abel Vesa 2023-02-08 141 56d77c9a10d97d Abel Vesa 2023-02-08 142 static int eusb2_repeater_init(struct phy *phy) 56d77c9a10d97d Abel Vesa 2023-02-08 143 { 56d77c9a10d97d Abel Vesa 2023-02-08 144 struct eusb2_repeater *rptr = phy_get_drvdata(phy); ac0aae0074102c Abel Vesa 2024-01-04 @145 struct reg_field *regfields = rptr->regfields; 56156a76e765d3 Konrad Dybcio 2023-09-13 146 struct device_node *np = rptr->dev->of_node; 56156a76e765d3 Konrad Dybcio 2023-09-13 147 u32 init_tbl[F_NUM_TUNE_FIELDS] = { 0 }; 56156a76e765d3 Konrad Dybcio 2023-09-13 148 u8 override; 56d77c9a10d97d Abel Vesa 2023-02-08 149 u32 val; 56d77c9a10d97d Abel Vesa 2023-02-08 150 int ret; 56d77c9a10d97d Abel Vesa 2023-02-08 151 int i; 56d77c9a10d97d Abel Vesa 2023-02-08 152 56d77c9a10d97d Abel Vesa 2023-02-08 153 ret = regulator_bulk_enable(rptr->cfg->num_vregs, rptr->vregs); 56d77c9a10d97d Abel Vesa 2023-02-08 154 if (ret) 56d77c9a10d97d Abel Vesa 2023-02-08 155 return ret; 56d77c9a10d97d Abel Vesa 2023-02-08 156 4ba2e52718c0ce Konrad Dybcio 2023-09-13 157 regmap_field_update_bits(rptr->regs[F_EN_CTL1], EUSB2_RPTR_EN, EUSB2_RPTR_EN); 56d77c9a10d97d Abel Vesa 2023-02-08 158 56156a76e765d3 Konrad Dybcio 2023-09-13 159 memcpy(init_tbl, rptr->cfg->init_tbl, sizeof(init_tbl)); 56156a76e765d3 Konrad Dybcio 2023-09-13 160 56156a76e765d3 Konrad Dybcio 2023-09-13 161 if (!of_property_read_u8(np, "qcom,tune-usb2-amplitude", &override)) 56156a76e765d3 Konrad Dybcio 2023-09-13 162 init_tbl[F_TUNE_IUSB2] = override; 56156a76e765d3 Konrad Dybcio 2023-09-13 163 56156a76e765d3 Konrad Dybcio 2023-09-13 164 if (!of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &override)) 56156a76e765d3 Konrad Dybcio 2023-09-13 165 init_tbl[F_TUNE_HSDISC] = override; 56156a76e765d3 Konrad Dybcio 2023-09-13 166 56156a76e765d3 Konrad Dybcio 2023-09-13 167 if (!of_property_read_u8(np, "qcom,tune-usb2-preem", &override)) 56156a76e765d3 Konrad Dybcio 2023-09-13 168 init_tbl[F_TUNE_USB2_PREEM] = override; 56156a76e765d3 Konrad Dybcio 2023-09-13 169 56156a76e765d3 Konrad Dybcio 2023-09-13 170 for (i = 0; i < F_NUM_TUNE_FIELDS; i++) 56156a76e765d3 Konrad Dybcio 2023-09-13 171 regmap_field_update_bits(rptr->regs[i], init_tbl[i], init_tbl[i]); 56d77c9a10d97d Abel Vesa 2023-02-08 172 4ba2e52718c0ce Konrad Dybcio 2023-09-13 173 ret = regmap_field_read_poll_timeout(rptr->regs[F_RPTR_STATUS], 4ba2e52718c0ce Konrad Dybcio 2023-09-13 174 val, val & RPTR_OK, 10, 5); 56d77c9a10d97d Abel Vesa 2023-02-08 175 if (ret) 56d77c9a10d97d Abel Vesa 2023-02-08 176 dev_err(rptr->dev, "initialization timed-out\n"); 56d77c9a10d97d Abel Vesa 2023-02-08 177 56d77c9a10d97d Abel Vesa 2023-02-08 178 return ret; 56d77c9a10d97d Abel Vesa 2023-02-08 179 } 56d77c9a10d97d Abel Vesa 2023-02-08 180 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework 2024-01-04 14:52 [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework Abel Vesa 2024-01-04 14:52 ` [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances Abel Vesa 2024-01-04 14:52 ` [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing Abel Vesa @ 2024-01-04 22:58 ` Elliot Berman 2 siblings, 0 replies; 11+ messages in thread From: Elliot Berman @ 2024-01-04 22:58 UTC (permalink / raw) To: Abel Vesa, Bjorn Andersson, Konrad Dybcio, Vinod Koul, Kishon Vijay Abraham I Cc: linux-arm-msm, linux-phy, linux-kernel On 1/4/2024 6:52 AM, Abel Vesa wrote: > Found the first issue (from first patch) while adding support > for X Elite (X1E80100) which comes with more than one repeaters. > The second fix is just bonus. > Tested-by: Elliot Berman <quic_eberman@quicinc.com> # sm8650-qrd > --- > Abel Vesa (2): > phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances > phy: qualcomm: eusb2-repeater: Drop the redundant zeroing > > drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > --- > base-commit: 0fef202ac2f8e6d9ad21aead648278f1226b9053 > change-id: 20240104-phy-qcom-eusb2-repeater-fixes-c9201113032c > > Best regards, -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-01-06 17:32 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-04 14:52 [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework Abel Vesa 2024-01-04 14:52 ` [PATCH 1/2] phy: qualcomm: eusb2-repeater: Fix the regfields for multiple instances Abel Vesa 2024-01-04 19:12 ` Abel Vesa 2024-01-04 22:46 ` Konrad Dybcio 2024-01-04 14:52 ` [PATCH 2/2] phy: qualcomm: eusb2-repeater: Drop the redundant zeroing Abel Vesa 2024-01-04 22:50 ` Konrad Dybcio 2024-01-05 9:16 ` Abel Vesa 2024-01-05 10:09 ` Konrad Dybcio 2024-01-05 10:19 ` Abel Vesa 2024-01-06 17:30 ` kernel test robot 2024-01-04 22:58 ` [PATCH 0/2] phy: qualcomm: eusb2-repeater: Some fixes after the regmap rework Elliot Berman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).