Re: [PATCH] soc: qcom: rpmh-rsc: Factor "tcs_reg_addr" and "tcs_cmd_addr" calculation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Apr 14, 2020 at 10:58 AM Joe Perches <joe@xxxxxxxxxxx> wrote:
>
> On Tue, 2020-04-14 at 10:41 -0700, Douglas Anderson wrote:
> > We can make some of the register access functions more readable by
> > factoring out the calculations a little bit.
>
> unrelated trivia:
>
> > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> []
> >  static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int tcs_id,
> >                              u32 data)
> >  {
> > -     writel(data, drv->tcs_base + RSC_DRV_TCS_OFFSET * tcs_id + reg);
> > +     writel(data, tcs_reg_addr(drv, reg, tcs_id));
> >       for (;;) {
> > -             if (data == readl(drv->tcs_base + reg +
> > -                               RSC_DRV_TCS_OFFSET * tcs_id))
> > +             if (data == readl(tcs_reg_addr(drv, reg, tcs_id)))
> >                       break;
> >               udelay(1);
> >       }
>
> There a lockup potential here.
>
> It might be better to use some max loop counter with
> an error/warning emitted instead of a continuous retry.

Yeah, I noticed that too but I assumed that it was probably OK.  I
think in this case it's really just confirming that the write made it
across the bus since it's checking the same bit that it's writing.
...but I wouldn't be opposed to this changing to use
readl_poll_timeout().

-Doug



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux