On Tue, 2 Apr 2024 at 15:37, Caleb Connolly <caleb.conno...@linaro.org> wrote:
>
> Hi Sumit,
>
> On 01/04/2024 06:46, Sumit Garg wrote:
> > On Thu, 28 Mar 2024 at 23:29, Caleb Connolly <caleb.conno...@linaro.org>
> > wrote:
> >>
> >> From: Bhupesh Sharma <bhupesh.li...@gmail.com>
> >>
> >> Some Qualcomm SoCs newer than SDM845 feature a so-called "7nm phy"
> >> driver, notable the SM8250 SoC which will gain U-Boot support in
> >> upcoming patches.
> >>
> >> Introduce a driver based on the Linux driver.
> >>
> >> Signed-off-by: Bhupesh Sharma <bhupesh.sha...@linaro.org>
> >> [code cleanup, align symbol names with Linux, switch to clk/reset_bulk
> >> APIs]
> >> Signed-off-by: Caleb Connolly <caleb.conno...@linaro.org>
> >> ---
> >> drivers/phy/qcom/Kconfig | 8 ++
> >> drivers/phy/qcom/Makefile | 1 +
> >> drivers/phy/qcom/phy-qcom-snps-femto-v2.c | 207
> >> ++++++++++++++++++++++++++++++
> >> 3 files changed, 216 insertions(+)
> >>
> ...
> >> diff --git a/drivers/phy/qcom/phy-qcom-snps-femto-v2.c
> >> b/drivers/phy/qcom/phy-qcom-snps-femto-v2.c
> >> new file mode 100644
> >> index 000000000000..58eb01972402
> >> --- /dev/null
> >> +++ b/drivers/phy/qcom/phy-qcom-snps-femto-v2.c
> ...
> >> +static inline void qcom_snps_hsphy_write_mask(void __iomem *base, u32
> >> offset,
> >> + u32 mask, u32 val)
> >> +{
> >> + u32 reg;
> >> +
> >> + reg = readl_relaxed(base + offset);
> >> +
> >> + reg &= ~mask;
> >> + reg |= val & mask;
> >> + writel_relaxed(reg, base + offset);
> >> +
> >> + /* Ensure above write is completed */
> >> + readl_relaxed(base + offset);
> >
> > It looks like you have missed addressing comments related to this API.
> > Again, why do we need this special handling in U-Boot? Why not just
> > clrsetbits_le32()?
>
> I have tried to find more info about this, but from what I can tell it's
> important (my guess is that it's a weird bus arbitration thing with the
> QMP phy's).
>
> If I replace this with clrsetbits_le32() then my SM8250 board crashdumps
> while probing USB. So it is needed (and I'm wondering now if I ought to
> switch back to this for the dwc3 qcom glue as well).
Strange, probably it's due to some strict device memory ordering
rules. If you can add some reasoning in comments for this API then
feel free to add:
Acked-by: Sumit Garg <sumit.g...@linaro.org>
-Sumit
