From: qianggui.song <Qianggui.Song@amlogic.com> To: Jerome Brunet <jbrunet@baylibre.com>, Linus Walleij <linus.walleij@linaro.org> Cc: Neil Armstrong <narmstrong@baylibre.com>, Kevin Hilman <khilman@baylibre.com>, Martin Blumenstingl <martin.blumenstingl@googlemail.com>, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, <linux-arm-kernel@lists.infradead.org>, <linux-amlogic@lists.infradead.org>, <linux-kernel@vger.kernel.org>, <linux-gpio@vger.kernel.org> Subject: Re: [PATCH 3/3] pinctrl: meson: add pinctrl driver support for Meson-S4 Soc Date: Thu, 16 Dec 2021 14:07:24 +0800 [thread overview] Message-ID: <0a3efa6f-f048-ee04-a2d6-30f337f3d484@amlogic.com> (raw) In-Reply-To: <1j35mv31c1.fsf@starbuckisacylon.baylibre.com> Hi, Jerome On 2021/12/14 下午6:00, Jerome Brunet wrote: > > > On Tue 14 Dec 2021 at 10:21, Qianggui Song <qianggui.song@amlogic.com> wrote: > >> Add new pinctrl driver for Amlogic's Meson-S4 SoC which share the >> same register laytout as the previous Meson-A1. >> >> Signed-off-by: Qianggui Song <qianggui.song@amlogic.com> >> --- > > [...] > >> + >> +/* Bank D func1 */ >> +static const unsigned int uart_b_tx_d_pins[] = { GPIOD_0 }; >> +static const unsigned int uart_b_rx_d_pins[] = { GPIOD_1 }; >> +static const unsigned int uart_b_cts_d_pins[] = { GPIOD_2 }; >> +static const unsigned int uart_b_rts_d_pins[] = { GPIOD_3 }; > > Your S805x2 documenation says the 4 pins above are HDMI related for func 1 > >> +static const unsigned int remote_out_pins[] = { GPIOD_4 }; >> +static const unsigned int remote_in_pins[] = { GPIOD_5 }; > > SPDIF for those 2 > >> +static const unsigned int jtag_1_clk_pins[] = { GPIOD_6 }; >> +static const unsigned int jtag_1_tms_pins[] = { GPIOD_7 }; >> +static const unsigned int jtag_1_tdi_pins[] = { GPIOD_8 }; >> +static const unsigned int jtag_1_tdo_pins[] = { GPIOD_9 }; > > I2C for those 4 > > It goes on like this for the rest of GPIO Bank D and I did not check the > other banks > > Could you please clarify > * Does the S4 applies to S805X2, S905Y4, Both ? > * Is the S805X2 Datasheet Revision 0.4 accurate ? Explain the above confuse here, yes, the S4 does apply to both S805X2 and S905Y4, and I create this .c driver based on S805X2 Datasheet Revision 0.5. I have no S805X2 Datasheet 0.4 on hand however our reference board works well based on Revision 0.5 configuration. > >> +static const unsigned int clk12_24_pins[] = { GPIOD_10 }; >> +static const unsigned int pwm_g_hiz_pins[] = { GPIOD_11 }; >> + > > [...] > >> + >> +static const char * const tdm_groups[] = { >> + "tdm_d2_c", "tdm_d3_c", "tdm_fs1_c", "tdm_d4_c", "tdm_d5_c", >> + "tdm_fs1_d", "tdm_d4_d", "tdm_d3_d", "tdm_d2_d", "tdm_sclk1_d", >> + "tdm_sclk1_h", "tdm_fs1_h", "tdm_d2_h", "tdm_d3_h", "tdm_d4_h", >> + "tdm_d1", "tdm_d0", "tdm_fs0", "tdm_sclk0", "tdm_fs2", "tdm_sclk2", >> + "tdm_d4_z", "tdm_d5_z", "tdm_d6", "tdm_d7" >> +}; > > On previous chip, there were pin assigned to tdm A, B or C. > On this generation, it seems you have added a second level on pinmuxing > to re-route the audio pins to different controllers > > In such case, I don't think they belong in the same group. > Depending on settins, D2 and D3 could be unrelated > > I think each audio pin should have its own group (one group for D3, one > D4, etc ...) > According to our audio colleague, on this chip, tdm A/B/C can choose which pins are routed to their controllers freely by writing special registers, say, tdm_d2_c can be assigned to any of tdm a, b and c by demand, so no need to specify a/b/c words any more. >> + >> +static const char * const mclk_groups[] = { >> + "mclk_1_c", "mclk_1_d", "mclk_1_h", "mclk_2" >> + >> +}; > > mclk_1 and mclk_2 should be in separate groups > Okay,will fix it in next patch. >> + >> +static const char * const remote_out_groups[] = { >> + "remote_out" >> +}; >> + >> +static const char * const remote_in_groups[] = { >> + "remote_in" >> +}; >> + >> +static const char * const clk12_24_groups[] = { >> + "clk12_24" >> +}; >> + >> +static const char * const clk_32k_in_groups[] = { >> + "clk_32k_in" >> +}; >> + >> +static const char * const pwm_a_hiz_groups[] = { >> + "pwm_a_hiz" >> +}; >> + > > I'm curious to know what the pwm hiz function is ? > This is a special mode of pwm and short for high-intensity zone. If this mode is enable, when pwm is in "on" time, pin of pwm become hiz state, the high level is not driven by chips but a pull-up resistor. > . > _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
WARNING: multiple messages have this Message-ID (diff)
From: qianggui.song <Qianggui.Song@amlogic.com> To: Jerome Brunet <jbrunet@baylibre.com>, Linus Walleij <linus.walleij@linaro.org> Cc: Neil Armstrong <narmstrong@baylibre.com>, Kevin Hilman <khilman@baylibre.com>, Martin Blumenstingl <martin.blumenstingl@googlemail.com>, Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>, <linux-arm-kernel@lists.infradead.org>, <linux-amlogic@lists.infradead.org>, <linux-kernel@vger.kernel.org>, <linux-gpio@vger.kernel.org> Subject: Re: [PATCH 3/3] pinctrl: meson: add pinctrl driver support for Meson-S4 Soc Date: Thu, 16 Dec 2021 14:07:24 +0800 [thread overview] Message-ID: <0a3efa6f-f048-ee04-a2d6-30f337f3d484@amlogic.com> (raw) In-Reply-To: <1j35mv31c1.fsf@starbuckisacylon.baylibre.com> Hi, Jerome On 2021/12/14 下午6:00, Jerome Brunet wrote: > > > On Tue 14 Dec 2021 at 10:21, Qianggui Song <qianggui.song@amlogic.com> wrote: > >> Add new pinctrl driver for Amlogic's Meson-S4 SoC which share the >> same register laytout as the previous Meson-A1. >> >> Signed-off-by: Qianggui Song <qianggui.song@amlogic.com> >> --- > > [...] > >> + >> +/* Bank D func1 */ >> +static const unsigned int uart_b_tx_d_pins[] = { GPIOD_0 }; >> +static const unsigned int uart_b_rx_d_pins[] = { GPIOD_1 }; >> +static const unsigned int uart_b_cts_d_pins[] = { GPIOD_2 }; >> +static const unsigned int uart_b_rts_d_pins[] = { GPIOD_3 }; > > Your S805x2 documenation says the 4 pins above are HDMI related for func 1 > >> +static const unsigned int remote_out_pins[] = { GPIOD_4 }; >> +static const unsigned int remote_in_pins[] = { GPIOD_5 }; > > SPDIF for those 2 > >> +static const unsigned int jtag_1_clk_pins[] = { GPIOD_6 }; >> +static const unsigned int jtag_1_tms_pins[] = { GPIOD_7 }; >> +static const unsigned int jtag_1_tdi_pins[] = { GPIOD_8 }; >> +static const unsigned int jtag_1_tdo_pins[] = { GPIOD_9 }; > > I2C for those 4 > > It goes on like this for the rest of GPIO Bank D and I did not check the > other banks > > Could you please clarify > * Does the S4 applies to S805X2, S905Y4, Both ? > * Is the S805X2 Datasheet Revision 0.4 accurate ? Explain the above confuse here, yes, the S4 does apply to both S805X2 and S905Y4, and I create this .c driver based on S805X2 Datasheet Revision 0.5. I have no S805X2 Datasheet 0.4 on hand however our reference board works well based on Revision 0.5 configuration. > >> +static const unsigned int clk12_24_pins[] = { GPIOD_10 }; >> +static const unsigned int pwm_g_hiz_pins[] = { GPIOD_11 }; >> + > > [...] > >> + >> +static const char * const tdm_groups[] = { >> + "tdm_d2_c", "tdm_d3_c", "tdm_fs1_c", "tdm_d4_c", "tdm_d5_c", >> + "tdm_fs1_d", "tdm_d4_d", "tdm_d3_d", "tdm_d2_d", "tdm_sclk1_d", >> + "tdm_sclk1_h", "tdm_fs1_h", "tdm_d2_h", "tdm_d3_h", "tdm_d4_h", >> + "tdm_d1", "tdm_d0", "tdm_fs0", "tdm_sclk0", "tdm_fs2", "tdm_sclk2", >> + "tdm_d4_z", "tdm_d5_z", "tdm_d6", "tdm_d7" >> +}; > > On previous chip, there were pin assigned to tdm A, B or C. > On this generation, it seems you have added a second level on pinmuxing > to re-route the audio pins to different controllers > > In such case, I don't think they belong in the same group. > Depending on settins, D2 and D3 could be unrelated > > I think each audio pin should have its own group (one group for D3, one > D4, etc ...) > According to our audio colleague, on this chip, tdm A/B/C can choose which pins are routed to their controllers freely by writing special registers, say, tdm_d2_c can be assigned to any of tdm a, b and c by demand, so no need to specify a/b/c words any more. >> + >> +static const char * const mclk_groups[] = { >> + "mclk_1_c", "mclk_1_d", "mclk_1_h", "mclk_2" >> + >> +}; > > mclk_1 and mclk_2 should be in separate groups > Okay,will fix it in next patch. >> + >> +static const char * const remote_out_groups[] = { >> + "remote_out" >> +}; >> + >> +static const char * const remote_in_groups[] = { >> + "remote_in" >> +}; >> + >> +static const char * const clk12_24_groups[] = { >> + "clk12_24" >> +}; >> + >> +static const char * const clk_32k_in_groups[] = { >> + "clk_32k_in" >> +}; >> + >> +static const char * const pwm_a_hiz_groups[] = { >> + "pwm_a_hiz" >> +}; >> + > > I'm curious to know what the pwm hiz function is ? > This is a special mode of pwm and short for high-intensity zone. If this mode is enable, when pwm is in "on" time, pin of pwm become hiz state, the high level is not driven by chips but a pull-up resistor. > . > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-12-16 6:06 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-14 2:20 [PATCH 0/3] pinctrl: meson-s4: add pinctrl driver Qianggui Song 2021-12-14 2:20 ` Qianggui Song 2021-12-14 2:20 ` Qianggui Song 2021-12-14 2:20 ` [PATCH 1/3] dt-bindings: pinctrl: meson: Add compatible for S4 Qianggui Song 2021-12-14 2:20 ` Qianggui Song 2021-12-14 2:20 ` Qianggui Song 2021-12-14 2:20 ` [PATCH 2/3] dt-bindings: gpio: Add a header file for Amlogic Meson S4 Qianggui Song 2021-12-14 2:20 ` Qianggui Song 2021-12-14 2:20 ` Qianggui Song 2021-12-15 20:29 ` Rob Herring 2021-12-15 20:29 ` Rob Herring 2021-12-15 20:29 ` Rob Herring 2021-12-14 2:21 ` [PATCH 3/3] pinctrl: meson: add pinctrl driver support for Meson-S4 Soc Qianggui Song 2021-12-14 2:21 ` Qianggui Song 2021-12-14 2:21 ` Qianggui Song 2021-12-14 10:00 ` Jerome Brunet 2021-12-14 10:00 ` Jerome Brunet 2021-12-14 10:00 ` Jerome Brunet 2021-12-16 6:07 ` qianggui.song [this message] 2021-12-16 6:07 ` qianggui.song 2021-12-16 9:20 ` Jerome Brunet 2021-12-16 9:20 ` Jerome Brunet 2021-12-16 9:20 ` Jerome Brunet 2021-12-17 9:23 ` qianggui.song 2021-12-17 9:23 ` qianggui.song 2021-12-14 11:57 ` Andy Shevchenko 2021-12-14 11:57 ` Andy Shevchenko 2021-12-14 11:57 ` Andy Shevchenko 2021-12-16 3:24 ` qianggui.song 2021-12-16 3:24 ` qianggui.song
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=0a3efa6f-f048-ee04-a2d6-30f337f3d484@amlogic.com \ --to=qianggui.song@amlogic.com \ --cc=jbrunet@baylibre.com \ --cc=khilman@baylibre.com \ --cc=linus.walleij@linaro.org \ --cc=linux-amlogic@lists.infradead.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=martin.blumenstingl@googlemail.com \ --cc=narmstrong@baylibre.com \ --cc=robh+dt@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.