| Message ID | 20240521-s4-pwm-v5-1-0c91f5fa32cd@amlogic.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Amlogic S4 PWM | expand |
Hello Kelvin, Junyi On 5/21/24 11:31, Kelvin Zhang via B4 Relay wrote: > From: Junyi Zhao <junyi.zhao@amlogic.com> > > This patch adds support for Amlogic S4 PWM. Please take a look at https://www.kernel.org/doc/html/v6.9/process/submitting-patches.html#describe-your-changes It should be something like Add support for Amlogic S4 PWM. > > Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com> > Signed-off-by: Kelvin Zhang <kelvin.zhang@amlogic.com> > --- > drivers/pwm/pwm-meson.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c > index b2f97dfb01bb..9fea28a51921 100644 > --- a/drivers/pwm/pwm-meson.c > +++ b/drivers/pwm/pwm-meson.c > @@ -460,6 +460,51 @@ static int meson_pwm_init_channels_meson8b_v2(struct pwm_chip *chip) > return meson_pwm_init_clocks_meson8b(chip, mux_parent_data); > } > > +static void meson_pwm_s4_put_clk(void *data) > +{ > + int i; > + struct meson_pwm *meson; > + struct meson_pwm_channel *channel; > + > + meson = (struct meson_pwm *)data; You can initialize meson variable along with declaration; type casting is not needed > + for (i = 0; i < MESON_NUM_PWMS; i++) { > + channel = &meson->channels[i]; > + clk_put(channel->clk); > + } you can save 3 lines just by using clk_put(meson->channels[i].clk); > +} > + > +static int meson_pwm_init_channels_meson_s4(struct pwm_chip *chip) > +{ > + int i, ret; > + struct device *dev = pwmchip_parent(chip); > + struct device_node *np = dev->of_node; > + struct meson_pwm *meson = to_meson_pwm(chip); > + struct meson_pwm_channel *channel; > + > + for (i = 0; i < MESON_NUM_PWMS; i++) { > + channel = &meson->channels[i]; > + channel->clk = of_clk_get(np, i); > + if (IS_ERR(channel->clk)) { > + ret = PTR_ERR(channel->clk); > + dev_err_probe(dev, ret, "Failed to get clk\n"); > + goto err; > + } > + } > + ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk, meson); > + if (ret) > + return ret; > + > + return 0; > + > +err: > + while (--i >= 0) { > + channel = &meson->channels[i]; > + clk_put(channel->clk); > + } > + > + return ret; > +} > + > static const struct meson_pwm_data pwm_meson8b_data = { > .parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" }, > .channels_init = meson_pwm_init_channels_meson8b_legacy, > @@ -498,6 +543,10 @@ static const struct meson_pwm_data pwm_meson8_v2_data = { > .channels_init = meson_pwm_init_channels_meson8b_v2, > }; > > +static const struct meson_pwm_data pwm_meson_s4_data = { > + .channels_init = meson_pwm_init_channels_meson_s4, > +}; > + according to already existing soc-specific named vars and functions new names should be static const struct meson_pwm_data pwm_s4_data and static int meson_pwm_init_channels_s4(struct pwm_chip *chip) > static const struct of_device_id meson_pwm_matches[] = { > { > .compatible = "amlogic,meson8-pwm-v2", > @@ -536,6 +585,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,meson-s4-pwm", > + .data = &pwm_meson_s4_data > + }, > {}, > }; > MODULE_DEVICE_TABLE(of, meson_pwm_matches); >
On 2024/5/22 2:50, George Stark wrote: > [ EXTERNAL EMAIL ] > > Hello Kelvin, Junyi > > On 5/21/24 11:31, Kelvin Zhang via B4 Relay wrote: >> From: Junyi Zhao <junyi.zhao@amlogic.com> >> >> This patch adds support for Amlogic S4 PWM. > > Please take a look at > https://www.kernel.org/doc/html/v6.9/process/submitting-patches.html#describe-your-changes > > It should be something like > Add support for Amlogic S4 PWM. > > >> >> Signed-off-by: Junyi Zhao <junyi.zhao@amlogic.com> >> Signed-off-by: Kelvin Zhang <kelvin.zhang@amlogic.com> >> --- >> drivers/pwm/pwm-meson.c | 53 >> +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 53 insertions(+) >> >> diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c >> index b2f97dfb01bb..9fea28a51921 100644 >> --- a/drivers/pwm/pwm-meson.c >> +++ b/drivers/pwm/pwm-meson.c >> @@ -460,6 +460,51 @@ static int >> meson_pwm_init_channels_meson8b_v2(struct pwm_chip *chip) >> return meson_pwm_init_clocks_meson8b(chip, mux_parent_data); >> } >> >> +static void meson_pwm_s4_put_clk(void *data) >> +{ >> + int i; >> + struct meson_pwm *meson; >> + struct meson_pwm_channel *channel; >> + >> + meson = (struct meson_pwm *)data; > You can initialize meson variable along with declaration; type casting > is not needed > >> + for (i = 0; i < MESON_NUM_PWMS; i++) { >> + channel = &meson->channels[i]; >> + clk_put(channel->clk); >> + } > you can save 3 lines just by using clk_put(meson->channels[i].clk); > >> +} >> + >> +static int meson_pwm_init_channels_meson_s4(struct pwm_chip *chip) >> +{ >> + int i, ret; >> + struct device *dev = pwmchip_parent(chip); >> + struct device_node *np = dev->of_node; >> + struct meson_pwm *meson = to_meson_pwm(chip); >> + struct meson_pwm_channel *channel; >> + >> + for (i = 0; i < MESON_NUM_PWMS; i++) { >> + channel = &meson->channels[i]; >> + channel->clk = of_clk_get(np, i); >> + if (IS_ERR(channel->clk)) { >> + ret = PTR_ERR(channel->clk); >> + dev_err_probe(dev, ret, "Failed to get clk\n"); >> + goto err; >> + } >> + } >> + ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk, meson); >> + if (ret) >> + return ret; >> + >> + return 0; >> + >> +err: >> + while (--i >= 0) { >> + channel = &meson->channels[i]; >> + clk_put(channel->clk); >> + } >> + >> + return ret; >> +} >> + >> static const struct meson_pwm_data pwm_meson8b_data = { >> .parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" }, >> .channels_init = meson_pwm_init_channels_meson8b_legacy, >> @@ -498,6 +543,10 @@ static const struct meson_pwm_data >> pwm_meson8_v2_data = { >> .channels_init = meson_pwm_init_channels_meson8b_v2, >> }; >> >> +static const struct meson_pwm_data pwm_meson_s4_data = { >> + .channels_init = meson_pwm_init_channels_meson_s4, >> +}; >> + > according to already existing soc-specific named vars and functions > new names should be > static const struct meson_pwm_data pwm_s4_data > and > static int meson_pwm_init_channels_s4(struct pwm_chip *chip) > >> static const struct of_device_id meson_pwm_matches[] = { >> { >> .compatible = "amlogic,meson8-pwm-v2", >> @@ -536,6 +585,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,meson-s4-pwm", >> + .data = &pwm_meson_s4_data >> + }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, meson_pwm_matches); >> > > -- > Best regards > George Thks for your suggestion. i will check it. -- Junyi
diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c index b2f97dfb01bb..9fea28a51921 100644 --- a/drivers/pwm/pwm-meson.c +++ b/drivers/pwm/pwm-meson.c @@ -460,6 +460,51 @@ static int meson_pwm_init_channels_meson8b_v2(struct pwm_chip *chip) return meson_pwm_init_clocks_meson8b(chip, mux_parent_data); } +static void meson_pwm_s4_put_clk(void *data) +{ + int i; + struct meson_pwm *meson; + struct meson_pwm_channel *channel; + + meson = (struct meson_pwm *)data; + for (i = 0; i < MESON_NUM_PWMS; i++) { + channel = &meson->channels[i]; + clk_put(channel->clk); + } +} + +static int meson_pwm_init_channels_meson_s4(struct pwm_chip *chip) +{ + int i, ret; + struct device *dev = pwmchip_parent(chip); + struct device_node *np = dev->of_node; + struct meson_pwm *meson = to_meson_pwm(chip); + struct meson_pwm_channel *channel; + + for (i = 0; i < MESON_NUM_PWMS; i++) { + channel = &meson->channels[i]; + channel->clk = of_clk_get(np, i); + if (IS_ERR(channel->clk)) { + ret = PTR_ERR(channel->clk); + dev_err_probe(dev, ret, "Failed to get clk\n"); + goto err; + } + } + ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk, meson); + if (ret) + return ret; + + return 0; + +err: + while (--i >= 0) { + channel = &meson->channels[i]; + clk_put(channel->clk); + } + + return ret; +} + static const struct meson_pwm_data pwm_meson8b_data = { .parent_names = { "xtal", NULL, "fclk_div4", "fclk_div3" }, .channels_init = meson_pwm_init_channels_meson8b_legacy, @@ -498,6 +543,10 @@ static const struct meson_pwm_data pwm_meson8_v2_data = { .channels_init = meson_pwm_init_channels_meson8b_v2, }; +static const struct meson_pwm_data pwm_meson_s4_data = { + .channels_init = meson_pwm_init_channels_meson_s4, +}; + static const struct of_device_id meson_pwm_matches[] = { { .compatible = "amlogic,meson8-pwm-v2", @@ -536,6 +585,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,meson-s4-pwm", + .data = &pwm_meson_s4_data + }, {}, }; MODULE_DEVICE_TABLE(of, meson_pwm_matches);