On 23-01-22, 15:46, Ansuel Smith wrote:
> On Sun, Jan 23, 2022 at 05:22:18PM +0530, Vinod Koul wrote:
> > On 17-01-22, 01:26, Ansuel Smith wrote:
> > > Convert latch function to readl pool macro to tidy things up.
> > >
> > > Signed-off-by: Ansuel Smith <ansuelsmth@xxxxxxxxx>
> > > ---
> > > drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 17 +++++------------
> > > 1 file changed, 5 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> > > index 6788e0e8272a..ab2d1431546d 100644
> > > --- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> > > +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> > > @@ -112,6 +112,9 @@
> > > #define SS_CR_READ_REG BIT(0)
> > > #define SS_CR_WRITE_REG BIT(0)
> > >
> > > +#define LATCH_SLEEP 40
> > > +#define LATCH_TIMEOUT 100
> > > +
> > > struct usb_phy {
> > > void __iomem *base;
> > > struct device *dev;
> > > @@ -156,19 +159,9 @@ static inline void usb_phy_write_readback(struct usb_phy *phy_dwc3,
> > >
> > > static int wait_for_latch(void __iomem *addr)
> > > {
> > > - u32 retry = 10;
> > > -
> > > - while (true) {
> > > - if (!readl(addr))
> > > - break;
> >
> > we break if read returns non zero value...
> >
> > Do you know what is the value expected?
> >
>
> If I understand the logic here, we write a value and we wait for it to
> get applied. To confirm that we execute a writel and then we readl the
> same address until it does return a value. That is the way used to
> understand that the write process has finished and that the value has
> been applied/we can write again.
>
> > > -
> > > - if (--retry == 0)
> > > - return -ETIMEDOUT;
> > > -
> > > - usleep_range(10, 20);
> > > - }
> > > + u32 val;
> >
> > Okay this contains garbage..
>
> I think I didn't understand, val value will get replaced by readl in
> the pool_timeout function.
>
> > >
> > > - return 0;
> > > + return readl_poll_timeout(addr, val, !val, LATCH_SLEEP, LATCH_TIMEOUT);
> >
> > and we are waiting for it read a garbage value!
> >
>
> Again could be very confused and wrong but the pool_timeout macro does
> the exact same thing of the wait_for_latch function with th only
> difference of handling the sleep differently. We put in val the return
> of readl and the break condition as !val. Or I didn't understand the
> concern about garbage value.
Sorry I read the readl_poll_timeout wrongly, this seems correct, I will
pick these now.
--
~Vinod
