* Re: [PATCH V3 RESEND] pwm: meson: add pwm support for S4 [not found] <20231016052457.1191838-1-junyi.zhao@amlogic.com> @ 2023-10-16 22:37 ` Yixun Lan 2023-10-23 19:40 ` George Stark 2023-10-23 20:07 ` George Stark 1 sibling, 1 reply; 6+ messages in thread From: Yixun Lan @ 2023-10-16 22:37 UTC (permalink / raw) To: JunYi Zhao Cc: thierry.reding, u.kleine-koenig, neil.armstrong, khilman, jbrunet, martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic, linux-kernel Hi JunYi On 13:24 Mon 16 Oct , JunYi Zhao wrote: > From: "junyi.zhao" <junyi.zhao@amlogic.com> > > Support PWM for S4 soc. > Now the PWM clock input is done in independent CLKCTRL registers. > And no more in the PWM registers. > PWM needs to obtain an external clock source. > > Signed-off-by: junyi.zhao <junyi.zhao@amlogic.com> > --- > V2 -> V3: > Rebase and Review the latest upstream code again. > After reconstruction, stick to the previous code as much as possible. > drivers/pwm/pwm-meson.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c > index 25519cddc2a9..fe9fd75747c4 100644 > --- a/drivers/pwm/pwm-meson.c > +++ b/drivers/pwm/pwm-meson.c > @@ -99,6 +99,7 @@ struct meson_pwm_channel { > struct meson_pwm_data { > const char * const *parent_names; > unsigned int num_parents; > + unsigned int extern_clk; > }; > > struct meson_pwm { > @@ -396,6 +397,10 @@ static const struct meson_pwm_data pwm_g12a_ao_cd_data = { > .num_parents = ARRAY_SIZE(pwm_g12a_ao_cd_parent_names), > }; > > +static const struct meson_pwm_data pwm_s4_data = { > + .extern_clk = true, > +}; > + > static const struct of_device_id meson_pwm_matches[] = { > { > .compatible = "amlogic,meson8b-pwm", > @@ -429,6 +434,10 @@ static const struct of_device_id meson_pwm_matches[] = { > .compatible = "amlogic,meson-g12a-ao-pwm-cd", > .data = &pwm_g12a_ao_cd_data > }, > + { > + .compatible = "amlogic,s4-pwm", > + .data = &pwm_s4_data, > + }, > {}, > }; > MODULE_DEVICE_TABLE(of, meson_pwm_matches); > @@ -451,6 +460,16 @@ static int meson_pwm_init_channels(struct meson_pwm *meson) > struct clk_parent_data div_parent = {}, gate_parent = {}; > struct clk_init_data init = {}; > > + if (meson->data->extern_clk) { > + snprintf(name, sizeof(name), "clkin%u", i); > + channel->clk = devm_clk_get(dev, name); use devm_clk_get_optional() which would save you from introducing the 'extern_clk' variable > + if (IS_ERR(channel->clk)) { > + dev_err(meson->chip.dev, "can't get device clock\n"); > + return PTR_ERR(channel->clk); > + } > + continue; > + } > + > snprintf(name, sizeof(name), "%s#mux%u", dev_name(dev), i); > > init.name = name; > > base-commit: 4d2c646ac07cf4a35ef1c4a935a1a4fd6c6b1a36 > -- > 2.41.0 > > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic -- Yixun Lan (dlan) Gentoo Linux Developer GPG Key ID AABEFD55 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 RESEND] pwm: meson: add pwm support for S4 2023-10-16 22:37 ` [PATCH V3 RESEND] pwm: meson: add pwm support for S4 Yixun Lan @ 2023-10-23 19:40 ` George Stark 2023-10-27 9:22 ` Junyi Zhao 0 siblings, 1 reply; 6+ messages in thread From: George Stark @ 2023-10-23 19:40 UTC (permalink / raw) To: Yixun Lan, JunYi Zhao Cc: thierry.reding, u.kleine-koenig, neil.armstrong, khilman, jbrunet, martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic, linux-kernel Hello Yixun Lan On 10/17/23 01:37, Yixun Lan wrote: > Hi JunYi > > On 13:24 Mon 16 Oct , JunYi Zhao wrote: >> From: "junyi.zhao" <junyi.zhao@amlogic.com> >> >> Support PWM for S4 soc. >> Now the PWM clock input is done in independent CLKCTRL registers. >> And no more in the PWM registers. >> PWM needs to obtain an external clock source. >> >> Signed-off-by: junyi.zhao <junyi.zhao@amlogic.com> >> --- >> V2 -> V3: >> Rebase and Review the latest upstream code again. >> After reconstruction, stick to the previous code as much as possible. >> drivers/pwm/pwm-meson.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c >> index 25519cddc2a9..fe9fd75747c4 100644 >> --- a/drivers/pwm/pwm-meson.c >> +++ b/drivers/pwm/pwm-meson.c >> @@ -99,6 +99,7 @@ struct meson_pwm_channel { >> struct meson_pwm_data { >> const char * const *parent_names; >> unsigned int num_parents; >> + unsigned int extern_clk; >> }; >> >> struct meson_pwm { >> @@ -396,6 +397,10 @@ static const struct meson_pwm_data pwm_g12a_ao_cd_data = { >> .num_parents = ARRAY_SIZE(pwm_g12a_ao_cd_parent_names), >> }; >> >> +static const struct meson_pwm_data pwm_s4_data = { >> + .extern_clk = true, >> +}; >> + >> static const struct of_device_id meson_pwm_matches[] = { >> { >> .compatible = "amlogic,meson8b-pwm", >> @@ -429,6 +434,10 @@ static const struct of_device_id meson_pwm_matches[] = { >> .compatible = "amlogic,meson-g12a-ao-pwm-cd", >> .data = &pwm_g12a_ao_cd_data >> }, >> + { >> + .compatible = "amlogic,s4-pwm", >> + .data = &pwm_s4_data, >> + }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, meson_pwm_matches); >> @@ -451,6 +460,16 @@ static int meson_pwm_init_channels(struct meson_pwm *meson) >> struct clk_parent_data div_parent = {}, gate_parent = {}; >> struct clk_init_data init = {}; >> >> + if (meson->data->extern_clk) { >> + snprintf(name, sizeof(name), "clkin%u", i); >> + channel->clk = devm_clk_get(dev, name); > use devm_clk_get_optional() which would save you from introducing > the 'extern_clk' variable On S4 and other recent chips PWM clock resides in separate IP and it must be provided to driver in order to control PWM frequency. So this clock is not optional. > >> + if (IS_ERR(channel->clk)) { >> + dev_err(meson->chip.dev, "can't get device clock\n"); >> + return PTR_ERR(channel->clk); >> + } >> + continue; >> + } >> + >> snprintf(name, sizeof(name), "%s#mux%u", dev_name(dev), i); >> >> init.name = name; >> >> base-commit: 4d2c646ac07cf4a35ef1c4a935a1a4fd6c6b1a36 >> -- >> 2.41.0 >> >> >> _______________________________________________ >> linux-amlogic mailing list >> linux-amlogic@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-amlogic > -- Best regards George ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 RESEND] pwm: meson: add pwm support for S4 2023-10-23 19:40 ` George Stark @ 2023-10-27 9:22 ` Junyi Zhao 0 siblings, 0 replies; 6+ messages in thread From: Junyi Zhao @ 2023-10-27 9:22 UTC (permalink / raw) To: George Stark, Yixun Lan Cc: thierry.reding, u.kleine-koenig, neil.armstrong, khilman, jbrunet, martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic, linux-kernel Hello YiXun， CLK config has been separated on HW. No more in pwmchip. And s4 datasheet about clk updated already. On 2023/10/24 3:40, George Stark wrote: > [你通常不会收到来自 gnstark@salutedevices.com 的电子邮件。请访问 > https://aka.ms/LearnAboutSenderIdentification，以了解这一点为什么很重要] > > [ EXTERNAL EMAIL ] > > Hello Yixun Lan > > On 10/17/23 01:37, Yixun Lan wrote: >> Hi JunYi >> >> On 13:24 Mon 16 Oct , JunYi Zhao wrote: >>> From: "junyi.zhao" <junyi.zhao@amlogic.com> >>> >>> Support PWM for S4 soc. >>> Now the PWM clock input is done in independent CLKCTRL registers. >>> And no more in the PWM registers. >>> PWM needs to obtain an external clock source. >>> >>> Signed-off-by: junyi.zhao <junyi.zhao@amlogic.com> >>> --- >>> V2 -> V3: >>> Rebase and Review the latest upstream code again. >>> After reconstruction, stick to the previous code as much as possible. >>> drivers/pwm/pwm-meson.c | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> >>> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c >>> index 25519cddc2a9..fe9fd75747c4 100644 >>> --- a/drivers/pwm/pwm-meson.c >>> +++ b/drivers/pwm/pwm-meson.c >>> @@ -99,6 +99,7 @@ struct meson_pwm_channel { >>> struct meson_pwm_data { >>> const char * const *parent_names; >>> unsigned int num_parents; >>> + unsigned int extern_clk; >>> }; >>> >>> struct meson_pwm { >>> @@ -396,6 +397,10 @@ static const struct meson_pwm_data >>> pwm_g12a_ao_cd_data = { >>> .num_parents = ARRAY_SIZE(pwm_g12a_ao_cd_parent_names), >>> }; >>> >>> +static const struct meson_pwm_data pwm_s4_data = { >>> + .extern_clk = true, >>> +}; >>> + >>> static const struct of_device_id meson_pwm_matches[] = { >>> { >>> .compatible = "amlogic,meson8b-pwm", >>> @@ -429,6 +434,10 @@ static const struct of_device_id >>> meson_pwm_matches[] = { >>> .compatible = "amlogic,meson-g12a-ao-pwm-cd", >>> .data = &pwm_g12a_ao_cd_data >>> }, >>> + { >>> + .compatible = "amlogic,s4-pwm", >>> + .data = &pwm_s4_data, >>> + }, >>> {}, >>> }; >>> MODULE_DEVICE_TABLE(of, meson_pwm_matches); >>> @@ -451,6 +460,16 @@ static int meson_pwm_init_channels(struct >>> meson_pwm *meson) >>> struct clk_parent_data div_parent = {}, gate_parent = {}; >>> struct clk_init_data init = {}; >>> >>> + if (meson->data->extern_clk) { >>> + snprintf(name, sizeof(name), "clkin%u", i); >>> + channel->clk = devm_clk_get(dev, name); >> use devm_clk_get_optional() which would save you from introducing >> the 'extern_clk' variable > > On S4 and other recent chips PWM clock resides in separate IP and it > must be provided to driver in order to control PWM frequency. So this > clock is not optional. > >> >>> + if (IS_ERR(channel->clk)) { >>> + dev_err(meson->chip.dev, "can't get >>> device clock\n"); >>> + return PTR_ERR(channel->clk); >>> + } >>> + continue; >>> + } >>> + >>> snprintf(name, sizeof(name), "%s#mux%u", dev_name(dev), i); >>> >>> init.name = name; >>> >>> base-commit: 4d2c646ac07cf4a35ef1c4a935a1a4fd6c6b1a36 >>> -- >>> 2.41.0 >>> >>> >>> _______________________________________________ >>> linux-amlogic mailing list >>> linux-amlogic@lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-amlogic >> > > -- > Best regards > George ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 RESEND] pwm: meson: add pwm support for S4 [not found] <20231016052457.1191838-1-junyi.zhao@amlogic.com> 2023-10-16 22:37 ` [PATCH V3 RESEND] pwm: meson: add pwm support for S4 Yixun Lan @ 2023-10-23 20:07 ` George Stark 2023-10-27 9:34 ` Junyi Zhao 2023-10-27 10:00 ` Jerome Brunet 1 sibling, 2 replies; 6+ messages in thread From: George Stark @ 2023-10-23 20:07 UTC (permalink / raw) To: JunYi Zhao, thierry.reding, u.kleine-koenig, neil.armstrong, khilman, jbrunet, martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic, linux-kernel Hello JunYi Zhao On 10/16/23 08:24, JunYi Zhao wrote: > From: "junyi.zhao" <junyi.zhao@amlogic.com> > > Support PWM for S4 soc. > Now the PWM clock input is done in independent CLKCTRL registers. > And no more in the PWM registers. > PWM needs to obtain an external clock source. > > Signed-off-by: junyi.zhao <junyi.zhao@amlogic.com> > --- > V2 -> V3: > Rebase and Review the latest upstream code again. > After reconstruction, stick to the previous code as much as possible. > drivers/pwm/pwm-meson.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c > index 25519cddc2a9..fe9fd75747c4 100644 > --- a/drivers/pwm/pwm-meson.c > +++ b/drivers/pwm/pwm-meson.c > @@ -99,6 +99,7 @@ struct meson_pwm_channel { > struct meson_pwm_data { > const char * const *parent_names; > unsigned int num_parents; > + unsigned int extern_clk; may be bool extern_clk; > }; > > struct meson_pwm { > @@ -396,6 +397,10 @@ static const struct meson_pwm_data pwm_g12a_ao_cd_data = { > .num_parents = ARRAY_SIZE(pwm_g12a_ao_cd_parent_names), > }; > > +static const struct meson_pwm_data pwm_s4_data = { > + .extern_clk = true, > +}; > + > static const struct of_device_id meson_pwm_matches[] = { > { > .compatible = "amlogic,meson8b-pwm", > @@ -429,6 +434,10 @@ static const struct of_device_id meson_pwm_matches[] = { > .compatible = "amlogic,meson-g12a-ao-pwm-cd", > .data = &pwm_g12a_ao_cd_data > }, > + { > + .compatible = "amlogic,s4-pwm", > + .data = &pwm_s4_data, > + }, > {}, > }; > MODULE_DEVICE_TABLE(of, meson_pwm_matches); > @@ -451,6 +460,16 @@ static int meson_pwm_init_channels(struct meson_pwm *meson) > struct clk_parent_data div_parent = {}, gate_parent = {}; > struct clk_init_data init = {}; > > + if (meson->data->extern_clk) { > + snprintf(name, sizeof(name), "clkin%u", i); > + channel->clk = devm_clk_get(dev, name); > + if (IS_ERR(channel->clk)) { > + dev_err(meson->chip.dev, "can't get device clock\n"); > + return PTR_ERR(channel->clk); > + } > + continue; > + } > + > snprintf(name, sizeof(name), "%s#mux%u", dev_name(dev), i); > > init.name = name; > > base-commit: 4d2c646ac07cf4a35ef1c4a935a1a4fd6c6b1a36 -- Best regards George ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 RESEND] pwm: meson: add pwm support for S4 2023-10-23 20:07 ` George Stark @ 2023-10-27 9:34 ` Junyi Zhao 2023-10-27 10:00 ` Jerome Brunet 1 sibling, 0 replies; 6+ messages in thread From: Junyi Zhao @ 2023-10-27 9:34 UTC (permalink / raw) To: George Stark, thierry.reding, u.kleine-koenig, neil.armstrong, khilman, jbrunet, martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic, linux-kernel Hi George, thank u for your review. I will change it. On 2023/10/24 4:07, George Stark wrote: > [你通常不会收到来自 gnstark@salutedevices.com 的电子邮件。请访问 > https://aka.ms/LearnAboutSenderIdentification，以了解这一点为什么很重要] > > [ EXTERNAL EMAIL ] > > Hello JunYi Zhao > > On 10/16/23 08:24, JunYi Zhao wrote: >> From: "junyi.zhao" <junyi.zhao@amlogic.com> >> >> Support PWM for S4 soc. >> Now the PWM clock input is done in independent CLKCTRL registers. >> And no more in the PWM registers. >> PWM needs to obtain an external clock source. >> >> Signed-off-by: junyi.zhao <junyi.zhao@amlogic.com> >> --- >> V2 -> V3: >> Rebase and Review the latest upstream code again. >> After reconstruction, stick to the previous code as much as possible. >> drivers/pwm/pwm-meson.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c >> index 25519cddc2a9..fe9fd75747c4 100644 >> --- a/drivers/pwm/pwm-meson.c >> +++ b/drivers/pwm/pwm-meson.c >> @@ -99,6 +99,7 @@ struct meson_pwm_channel { >> struct meson_pwm_data { >> const char * const *parent_names; >> unsigned int num_parents; >> + unsigned int extern_clk; > may be bool extern_clk; >> }; >> >> struct meson_pwm { >> @@ -396,6 +397,10 @@ static const struct meson_pwm_data >> pwm_g12a_ao_cd_data = { >> .num_parents = ARRAY_SIZE(pwm_g12a_ao_cd_parent_names), >> }; >> >> +static const struct meson_pwm_data pwm_s4_data = { >> + .extern_clk = true, >> +}; >> + >> static const struct of_device_id meson_pwm_matches[] = { >> { >> .compatible = "amlogic,meson8b-pwm", >> @@ -429,6 +434,10 @@ static const struct of_device_id >> meson_pwm_matches[] = { >> .compatible = "amlogic,meson-g12a-ao-pwm-cd", >> .data = &pwm_g12a_ao_cd_data >> }, >> + { >> + .compatible = "amlogic,s4-pwm", >> + .data = &pwm_s4_data, >> + }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, meson_pwm_matches); >> @@ -451,6 +460,16 @@ static int meson_pwm_init_channels(struct >> meson_pwm *meson) >> struct clk_parent_data div_parent = {}, gate_parent = {}; >> struct clk_init_data init = {}; >> >> + if (meson->data->extern_clk) { >> + snprintf(name, sizeof(name), "clkin%u", i); >> + channel->clk = devm_clk_get(dev, name); >> + if (IS_ERR(channel->clk)) { >> + dev_err(meson->chip.dev, "can't get >> device clock\n"); >> + return PTR_ERR(channel->clk); >> + } >> + continue; >> + } >> + >> snprintf(name, sizeof(name), "%s#mux%u", dev_name(dev), i); >> >> init.name = name; >> >> base-commit: 4d2c646ac07cf4a35ef1c4a935a1a4fd6c6b1a36 > > -- > Best regards > George ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 RESEND] pwm: meson: add pwm support for S4 2023-10-23 20:07 ` George Stark 2023-10-27 9:34 ` Junyi Zhao @ 2023-10-27 10:00 ` Jerome Brunet 1 sibling, 0 replies; 6+ messages in thread From: Jerome Brunet @ 2023-10-27 10:00 UTC (permalink / raw) To: George Stark, JunYi Zhao, thierry.reding, u.kleine-koenig, neil.armstrong, khilman, martin.blumenstingl, linux-pwm, linux-arm-kernel, linux-amlogic, linux-kernel On Mon 23 Oct 2023 at 23:07, George Stark <gnstark@salutedevices.com> wrote: > Hello JunYi Zhao > > On 10/16/23 08:24, JunYi Zhao wrote: >> From: "junyi.zhao" <junyi.zhao@amlogic.com> >> Support PWM for S4 soc. >> Now the PWM clock input is done in independent CLKCTRL registers. >> And no more in the PWM registers. >> PWM needs to obtain an external clock source. >> Signed-off-by: junyi.zhao <junyi.zhao@amlogic.com> >> --- >> V2 -> V3: >> Rebase and Review the latest upstream code again. >> After reconstruction, stick to the previous code as much as possible. >> drivers/pwm/pwm-meson.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c >> index 25519cddc2a9..fe9fd75747c4 100644 >> --- a/drivers/pwm/pwm-meson.c >> +++ b/drivers/pwm/pwm-meson.c >> @@ -99,6 +99,7 @@ struct meson_pwm_channel { >> struct meson_pwm_data { >> const char * const *parent_names; >> unsigned int num_parents; >> + unsigned int extern_clk; > may be bool extern_clk; >> }; >> struct meson_pwm { >> @@ -396,6 +397,10 @@ static const struct meson_pwm_data pwm_g12a_ao_cd_data = { >> .num_parents = ARRAY_SIZE(pwm_g12a_ao_cd_parent_names), >> }; >> +static const struct meson_pwm_data pwm_s4_data = { >> + .extern_clk = true, >> +}; >> + >> static const struct of_device_id meson_pwm_matches[] = { >> { >> .compatible = "amlogic,meson8b-pwm", >> @@ -429,6 +434,10 @@ static const struct of_device_id meson_pwm_matches[] = { >> .compatible = "amlogic,meson-g12a-ao-pwm-cd", >> .data = &pwm_g12a_ao_cd_data >> }, >> + { >> + .compatible = "amlogic,s4-pwm", >> + .data = &pwm_s4_data, >> + }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, meson_pwm_matches); >> @@ -451,6 +460,16 @@ static int meson_pwm_init_channels(struct meson_pwm *meson) >> struct clk_parent_data div_parent = {}, gate_parent = {}; >> struct clk_init_data init = {}; >> + if (meson->data->extern_clk) { Instead of hacking through the existing registration function, it be much better to provide the clock registration function as on ops in dt data. Also, as Neil pointed out on the v2 [1], the meaning of clkin0/1 is changed on this SoC. * On previous SoC, it was a reference to clock input the PWM block should select from the hard-coded list it has (should fix that someday) * Now it is directly the input the PWM block must claim. You need to update the bindings accordingly for the S4 [1]: https://lore.kernel.org/linux-amlogic/07581fb8-0cd9-5b76-6fa3-1d1a7353d944@baylibre.com >> + snprintf(name, sizeof(name), "clkin%u", i); >> + channel->clk = devm_clk_get(dev, name); >> + if (IS_ERR(channel->clk)) { >> + dev_err(meson->chip.dev, "can't get device clock\n"); >> + return PTR_ERR(channel->clk); >> + } >> + continue; >> + } >> + >> snprintf(name, sizeof(name), "%s#mux%u", dev_name(dev), i); >> init.name = name; >> base-commit: 4d2c646ac07cf4a35ef1c4a935a1a4fd6c6b1a36 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-27 10:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20231016052457.1191838-1-junyi.zhao@amlogic.com>
2023-10-16 22:37 ` [PATCH V3 RESEND] pwm: meson: add pwm support for S4 Yixun Lan
2023-10-23 19:40 ` George Stark
2023-10-27 9:22 ` Junyi Zhao
2023-10-23 20:07 ` George Stark
2023-10-27 9:34 ` Junyi Zhao
2023-10-27 10:00 ` Jerome Brunet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox