Hi Marek, On Mon, Feb 18, 2019 at 2:48 PM Marek Vasut <marek.vasut@xxxxxxxxx> wrote: > On 2/18/19 2:41 PM, Geert Uytterhoeven wrote: > > On Mon, Feb 18, 2019 at 2:38 PM Marek Vasut <marek.vasut@xxxxxxxxx> wrote: > >> On 2/18/19 2:32 PM, Geert Uytterhoeven wrote: > >>>>>> 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? > >>>> > >>>> For one, It's unrelated register to POCCTRL, so I don't want to mix them > >>> > >>> That's why the array is called pinmux_ioctrl_reg[], not pinmux_pocctrl_reg[]: > >>> it is meant to cover various I/O control registers, including POCCTRL and > >>> TDSEL, to be saved/restored for PSCI system suspend. > >> > >> I thought the array is called pinmux_ioctrl_reg[] because that's what > >> the pocctrl was called in older datasheets ? At least that's how you > >> explained it on IRC last time. > > > > Ah, that's where the misunderstanding comes from: both POCCTRLx and > > TDSELy registers are sometimes called IOCTRLz registers. > > > > Then shouldn't we rename IOCTRL30 to POCCTRL first, and then add TDSEL > into the list ? Sure. I've already done so, but haven't sent out the patches yet. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx 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