On Wed, Jan 23, 2019 at 09:49:25AM +0000, Igor Russkikh wrote:
>
> >
> > Hi Igor
> >
> > err = readx_poll_timeout(hw_atl_itr_res_irq_get, self, alt_itr_res,
> > alt_itr_res == 0, 10, 1000);
> >
> > The advantage of using readx_poll_timeout is that it is used by lots
> > of other drivers and works. It is much better to use core
> > infrastructure, then build your own.
>
> Hi Andrew, agreed, but driver have more incompatible places with constructs
> like
>
> AQ_HW_WAIT_FOR(orig_stats_val !=
> (aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR) &
> BIT(CAPS_HI_STATISTICS)),
> 1U, 10000U);
You can define a little helper:
static u32 aq_hw_read_mpi_state2_addr(struct aq_hw_s *self)
{
return aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR);
}
Then
readx_poll_timeout(u32 aq_hw_read_mpi_state2_addr, self,
stats_val, orig_stats_val != stats_val &
BIT(CAPS_HI_STATISTICS));
Given you have the comment:
/* Wait FW to report back */
You think the current code is not very readable. So you could actually
have:
static int sq_fw2_update_stats_fw_wait(aq_hw_s *self, u32 orig_stats_val)
{
u32 stats_val;
return readx_poll_timeout(u32 aq_hw_read_mpi_state2_addr, self,
stats_val,
orig_stats_val != stats_val &
BIT(CAPS_HI_STATISTICS),
1, 1000);
}
You see this sort of construct in quite a lot of drivers.
> Found duplicating readl_poll_timeout declaration here:
> https://elixir.bootlin.com/linux/latest/source/drivers/phy/qualcomm/phy-qcom-ufs-i.h#L27
> Not sure what's the reason, but may worth cleaning it up.
Thanks.
Andrew
