Hi Martin,
On Mon, Jul 31, 2023 at 11:24 PM Martin K. Petersen
<martin.petersen@xxxxxxxxxx> wrote:
>
>
> Hi Ranjan!
>
> Since this patch is a candidate for stable it should be as simple as
> possible. I don't understand all the complexity introduced to
> accommodate the new retry_count argument.
>
> > static inline u32
> > -_base_readl_aero(const volatile void __iomem *addr)
> > +_base_readl_aero(const volatile void __iomem *addr, u8 retry_count)
> > {
> > u32 i = 0, ret_val;
> >
> > do {
> > ret_val = readl(addr);
> > i++;
> > - } while (ret_val == 0 && i < 3);
> > + } while (ret_val == 0 && i < retry_count);
>
> _base_readl_aero() is going to return as soon as the register is
> non-zero. Why is it important that some register reads are only retried
> 3 times instead of 30? Why can't you just bump the "3" above to "30" and
> make it a one line change?
>This HW bug impacts only a few registers. However, for code simplicity, we added retry logic(of 3 retries count) for all registers exposed to the driver.
Now the recommendation is:
- Increase retry count from 3 to 30 for only impacted registers.
- We also do not want to reduce the retries from 3 to 1 for
non-impacted registers to avoid any regression.
Hence, this patch retries upto 30 times for impacted registers and 3
times for non-impacted registers.
Thanks,
Ranjan
> --
> Martin K. Petersen Oracle Linux Engineering
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
