Hi Geert-san,
Thank you for your review!
> From: Geert Uytterhoeven, Sent: Wednesday, April 27, 2022 10:10 PM
>
> On Wed, Apr 20, 2022 at 11:39 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
<snip>
> > +static void ufs_renesas_reg_control(struct ufs_hba *hba,
> > + const struct ufs_renesas_init_param *p)
> > +{
> > + static u32 save[MAX_INDEX];
> > + int ret;
> > + u32 val;
> > +
> > + pr_debug("%s: %d %04x %08x, %08x, %d\n", __func__, p->mode, p->reg,
> > + p->u.val, p->mask, p->index);
>
> Do you need this?
> If yes, perhaps dev_dbg(hba->dev, ...)?
Umm, I don't think so because the parameters are hardcoded.
So, I'll remove all pr_debug.
> > +
> > + WARN_ON(p->index >= MAX_INDEX);
> > +
> > + switch (p->mode) {
> > + case MODE_RESTORE:
> > + ufshcd_writel(hba, save[p->index], p->reg);
> > + break;
> > + case MODE_SET:
> > + pr_debug("%s: %d %x %x\n", __func__, p->index, save[p->index],
> > + p->u.set);
>
> Likewise.
I'll remove it.
> > + save[p->index] |= p->u.set;
> > + break;
> > + case MODE_SAVE:
> > + save[p->index] = ufshcd_readl(hba, p->reg) & p->mask;
> > + pr_debug("%s: index = %d, save = %08x\n", __func__,
> > + p->index, save[p->index]);
>
> Likewise.
I'll remove it too.
> > + break;
> > + case MODE_POLL:
> > + ret = readl_poll_timeout_atomic(hba->mmio_base + p->reg,
> > + val,
> > + (val & p->mask) == p->u.expected,
> > + 10, 1000);
> > + if (ret)
> > + pr_err("%s: poll failed %d (%08x, %08x, %08x)\n",
> > + __func__, ret, val, p->mask, p->u.expected);
> > + break;
> > + case MODE_WAIT:
> > + if (p->u.delay_us > 1000)
> > + mdelay(p->u.delay_us / 1000);
>
> mdelay(DIV_ROUND_UP(p->u.delay_us, 1000));
> (cfr. include/linux/delay.h:ndelay())
I got it. I'll fix it.
>
> > + else
> > + udelay(p->u.delay_us);
> > + break;
> > + case MODE_WRITE:
> > + ufshcd_writel(hba, p->u.val, p->reg);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +static void ufs_renesas_pre_init(struct ufs_hba *hba)
> > +{
> > + const struct ufs_renesas_init_param *p = ufs_param;
> > + int i;
>
> unsigned int i
I'll fix it.
> > +
> > + for (i = 0; i < ARRAY_SIZE(ufs_param); i++)
> > + ufs_renesas_reg_control(hba, &p[i]);
> > +}
>
> > +static const struct of_device_id __maybe_unused ufs_renesas_of_match[] = {
> > + { .compatible = "renesas,r8a779f0-ufs" },
>
> As pointed out by the kernel test robot, this lack a sentinel.
Yes. I have already fixed this on v5.
Best regards,
Yoshihiro Shimoda
