| Message ID | 874k0nlrbw.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | pinctrl: renesas: r8a779g0: Add pins, groups and functions | expand |
Hi Morimoto-san, On Tue, Jun 14, 2022 at 7:58 AM Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > From: LUU HOAI <hoai.luu.ub@renesas.com> > > This patch adds initial pinctrl support for the R8A779G0 (V4H) SoC, > including bias, drive strength and voltage control. > > [Morimoto merged Kihara-san's MODSEL8 fixup patch, > cleanuped white space, care reserved bit on each configs, > fixup comments, etc] > Signed-off-by: LUU HOAI <hoai.luu.ub@renesas.com> > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Thanks for your patch! > --- /dev/null > +++ b/drivers/pinctrl/renesas/pfc-r8a779g0.c > +static const u16 pinmux_data[] = { > + /* IP0SR8 */ > + PINMUX_IPSR_MSEL(IP0SR8_3_0, SCL0, SEL_SCL0_0), > + PINMUX_IPSR_MSEL(IP0SR8_7_4, SDA0, SEL_SDA0_0), These are definitely still wrong, and not fixed by any of your subsequent patches: to enable I2C0 functionality, the corresponding SEL_{SCL,SDA}0 bit should be set to 1, not 0. Same for I2C[1-5] below. > + PINMUX_IPSR_MSEL(IP0SR8_11_8, SCL1, SEL_SCL1_0), > + PINMUX_IPSR_MSEL(IP0SR8_15_12, SDA1, SEL_SDA1_0), > + PINMUX_IPSR_MSEL(IP0SR8_19_16, SCL2, SEL_SCL2_0), > + PINMUX_IPSR_MSEL(IP0SR8_23_20, SDA2, SEL_SDA2_0), > + PINMUX_IPSR_MSEL(IP0SR8_27_24, SCL3, SEL_SCL3_0), > + PINMUX_IPSR_MSEL(IP0SR8_31_28, SDA3, SEL_SDA3_0), > + > + /* IP1SR8 */ > + PINMUX_IPSR_MSEL(IP1SR8_3_0, SCL4, SEL_SCL4_0), > + PINMUX_IPSR_MSEL(IP1SR8_3_0, HRX2, SEL_SCL4_0), > + PINMUX_IPSR_MSEL(IP1SR8_3_0, SCK4, SEL_SCL4_0), > + > + PINMUX_IPSR_MSEL(IP1SR8_7_4, SDA4, SEL_SDA4_0), > + PINMUX_IPSR_MSEL(IP1SR8_7_4, HTX2, SEL_SDA4_0), > + PINMUX_IPSR_MSEL(IP1SR8_7_4, CTS4_N, SEL_SDA4_0), > + > + PINMUX_IPSR_MSEL(IP1SR8_11_8, SCL5, SEL_SCL5_0), > + PINMUX_IPSR_MSEL(IP1SR8_11_8, HRTS2_N, SEL_SCL5_0), > + PINMUX_IPSR_MSEL(IP1SR8_11_8, RTS4_N, SEL_SCL5_0), > + > + PINMUX_IPSR_MSEL(IP1SR8_15_12, SDA5, SEL_SDA5_0), > + PINMUX_IPSR_MSEL(IP1SR8_15_12, SCIF_CLK2, SEL_SDA5_0), > + > + PINMUX_IPSR_GPSR(IP1SR8_19_16, HCTS2_N), > + PINMUX_IPSR_GPSR(IP1SR8_19_16, TX4), > + > + PINMUX_IPSR_GPSR(IP1SR8_23_20, HSCK2), > + PINMUX_IPSR_GPSR(IP1SR8_23_20, RX4), > +}; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Hi Geert Thank you for review. > > + /* IP0SR8 */ > > + PINMUX_IPSR_MSEL(IP0SR8_3_0, SCL0, SEL_SCL0_0), > > + PINMUX_IPSR_MSEL(IP0SR8_7_4, SDA0, SEL_SDA0_0), > > These are definitely still wrong, and not fixed by any of your > subsequent patches: to enable I2C0 functionality, the corresponding > SEL_{SCL,SDA}0 bit should be set to 1, not 0. > Same for I2C[1-5] below. There was such patch, and this patch series merged it. Now I'm asking why the patch was needed. I will fix and re-post v4. But, 1 things. As I mentioned before, I want to keep original patch as much as possible, even though it has bug (missing pins, having unnecessary settings). In other words, the difference from original patch is only cleanup:ed. I will keep current style on v4, too. About suffix, I'm asking it to Doc team. Thank you for your help !! Best regards --- Kuninori Morimoto
Hi Morimoto-san, On Mon, Jun 20, 2022 at 2:18 AM Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > > > + /* IP0SR8 */ > > > + PINMUX_IPSR_MSEL(IP0SR8_3_0, SCL0, SEL_SCL0_0), > > > + PINMUX_IPSR_MSEL(IP0SR8_7_4, SDA0, SEL_SDA0_0), > > > > These are definitely still wrong, and not fixed by any of your > > subsequent patches: to enable I2C0 functionality, the corresponding > > SEL_{SCL,SDA}0 bit should be set to 1, not 0. > > Same for I2C[1-5] below. > > There was such patch, and this patch series merged it. I don't see such a patch in this series? > Now I'm asking why the patch was needed. Sorry, I don't understand. > I will fix and re-post v4. > > But, 1 things. As I mentioned before, I want to keep original patch as much as possible, > even though it has bug (missing pins, having unnecessary settings). > In other words, the difference from original patch is only cleanup:ed. > I will keep current style on v4, too. OK. > About suffix, I'm asking it to Doc team. Thank you! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Hi Geert > > > These are definitely still wrong, and not fixed by any of your > > > subsequent patches: to enable I2C0 functionality, the corresponding > > > SEL_{SCL,SDA}0 bit should be set to 1, not 0. > > > Same for I2C[1-5] below. > > > > There was such patch, and this patch series merged it. > > I don't see such a patch in this series? > > > Now I'm asking why the patch was needed. > > Sorry, I don't understand. Sorry for un-clear explaination. [03/21] and [04/21] merged some/many BSP original patches into one. [03/21] is including I2C fixup patch from Kihara-san which fixup the SEL_{SCL,SDA} value from 1 to 0, but this fixup seems strange, and you are now pointing it. I'm not sure why this patch was needed, now, I'm asking. https://github.com/renesas-rcar/linux-bsp/commit/2155127dacd61a4bb2f6f316f716a9338799a786 Thank you for your help !! Best regards --- Kuninori Morimoto