Hi Marek,

On Sat, Feb 16, 2019 at 2:49 PM <marek.va...@gmail.com> wrote:
> The TDSELCTRL register is responsible for configuring the SDHI clock
> return path delay and may be adjusted by the bootloader. Retain the
> value across suspend/resume to prevent hardware instability after
> resume.
>
> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>

Thanks for your patch!

Looks good to me. Two comments though.

R-Car D3 (r8a77995) also has a TDSEL register, but it is not handled by
your patch.  Adding that would mean s/SDHI/SDHI and MMC/.

> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -657,6 +657,10 @@ static unsigned int sh_pfc_walk_regs(struct sh_pfc *pfc,
>                 for (i = 0; pfc->info->ioctrl_regs[i].reg; i++)
>                         do_reg(pfc, pfc->info->ioctrl_regs[i].reg, n++);
>
> +       if (pfc->info->tdsel_regs)
> +               for (i = 0; pfc->info->tdsel_regs[i].reg; i++)
> +                       do_reg(pfc, pfc->info->tdsel_regs[i].reg, n++);
> +
>         return n;
>  }

> --- a/drivers/pinctrl/sh-pfc/sh_pfc.h
> +++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
> @@ -176,6 +176,10 @@ struct pinmux_ioctrl_reg {
>         u32 reg;
>  };
>
> +struct pinmux_tdsel_reg {
> +       u32 reg;
> +};
> +
>  struct pinmux_data_reg {
>         u32 reg;
>         u8 reg_width;
> @@ -270,6 +274,7 @@ struct sh_pfc_soc_info {
>         const struct pinmux_drive_reg *drive_regs;
>         const struct pinmux_bias_reg *bias_regs;
>         const struct pinmux_ioctrl_reg *ioctrl_regs;
> +       const struct pinmux_tdsel_reg *tdsel_regs;
>         const struct pinmux_data_reg *data_regs;
>
>         const u16 *pinmux_data;

Is there any special reason why you added a new block of registers with
separate handling, instead of adding TDSEL to the existing
pinmux_ioctrl_reg[] arrays, which list other IOCTRL registers like
POCCTRL?

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Reply via email to