[Deepin-Kernel-SIG] [linux 6.12-y] [Upstream] pwm: Add Loongson PWM controller support#1718
Open
opsiff wants to merge 2 commits into
Open
[Deepin-Kernel-SIG] [linux 6.12-y] [Upstream] pwm: Add Loongson PWM controller support#1718opsiff wants to merge 2 commits into
opsiff wants to merge 2 commits into
Conversation
This commit adds a generic PWM framework driver for the PWM controller found on Loongson family chips. Acked-by: Huacai Chen <chenhuacai@loongson.cn> Co-developed-by: Juxin Gao <gaojuxin@loongson.cn> Signed-off-by: Juxin Gao <gaojuxin@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> Link: https://lore.kernel.org/r/76050a903a8015422fb9261ad88c7d9cc2edbbd8.1743403075.git.zhoubinbin@loongson.cn Signed-off-by: Uwe Kleine-König <ukleinek@kernel.org> (cherry picked from commit 2b62c89) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
deepin inclusion category: bugfix users report LL300 Laptop works failed with new driver and kernel, work around by builtin the pwm driver until loongson find a proper way. Signed-off-by: Wentao Guan <guanwentao@uniontech.com> (cherry picked from commit eef9634) Signed-off-by: Wentao Guan <guanwentao@uniontech.com>
Reviewer's GuideAdds a new Loongson PWM controller driver and wires it into the PWM subsystem, including build, config, and power-management support for LoongArch/ACPI/DT platforms. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In pwm_loongson_probe(), the error path after devm_clk_rate_exclusive_get() uses PTR_ERR(ddata->clk) instead of the local ret value, which will report an incorrect error code if clk retrieval succeeded but exclusive rate failed.
- pwm_loongson_suspend()/resume() unconditionally call clk_disable_unprepare()/clk_prepare_enable() on ddata->clk even though the clock is optional and may be NULL (ACPI/default-frequency case); guard these calls with a ddata->clk check.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In pwm_loongson_probe(), the error path after devm_clk_rate_exclusive_get() uses PTR_ERR(ddata->clk) instead of the local ret value, which will report an incorrect error code if clk retrieval succeeded but exclusive rate failed.
- pwm_loongson_suspend()/resume() unconditionally call clk_disable_unprepare()/clk_prepare_enable() on ddata->clk even though the clock is optional and may be NULL (ACPI/default-frequency case); guard these calls with a ddata->clk check.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new PWM controller driver for Loongson (LS7A/Loongson family) and hooks it into the PWM subsystem build and the Deepin LoongArch desktop defconfig.
Changes:
- Add a new
pwm-loongsonplatform driver with DT + ACPI match tables. - Wire the driver into
drivers/pwm/Makefileand add aCONFIG_PWM_LOONGSONKconfig option. - Enable
CONFIG_PWM_LOONGSON=yindeepin_loongarch_desktop_defconfig.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| MAINTAINERS | Adds a maintainer entry and file patterns for the new Loongson PWM driver/binding. |
| drivers/pwm/pwm-loongson.c | Introduces the Loongson PWM controller driver implementation. |
| drivers/pwm/Makefile | Adds build rule for pwm-loongson.o under CONFIG_PWM_LOONGSON. |
| drivers/pwm/Kconfig | Adds CONFIG_PWM_LOONGSON Kconfig entry. |
| arch/loongarch/configs/deepin_loongarch_desktop_defconfig | Enables Loongson PWM driver in the Deepin LoongArch desktop configuration. |
Comments suppressed due to low confidence (1)
drivers/pwm/pwm-loongson.c:133
- Same truncation/clamping issue as duty_cycle: the u64 result of mul_u64_u64_div_u64() is assigned to a u32 before comparing against U32_MAX, so the clamp won't work for large period_ns values. Use a u64 temporary and only cast to u32 after clamping.
/* period = period_ns * ddata->clk_rate / NSEC_PER_SEC */
period = mul_u64_u64_div_u64(period_ns, ddata->clk_rate, NSEC_PER_SEC);
if (period > U32_MAX)
period = U32_MAX;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+122
to
+132
| struct pwm_loongson_ddata *ddata = to_pwm_loongson_ddata(chip); | ||
|
|
||
| /* duty = duty_ns * ddata->clk_rate / NSEC_PER_SEC */ | ||
| duty = mul_u64_u64_div_u64(duty_ns, ddata->clk_rate, NSEC_PER_SEC); | ||
| if (duty > U32_MAX) | ||
| duty = U32_MAX; | ||
|
|
||
| /* period = period_ns * ddata->clk_rate / NSEC_PER_SEC */ | ||
| period = mul_u64_u64_div_u64(period_ns, ddata->clk_rate, NSEC_PER_SEC); | ||
| if (period > U32_MAX) | ||
| period = U32_MAX; |
Comment on lines
+79
to
+83
| u16 val; | ||
| struct pwm_loongson_ddata *ddata = to_pwm_loongson_ddata(chip); | ||
|
|
||
| val = pwm_loongson_readl(ddata, LOONGSON_PWM_REG_CTRL); | ||
|
|
| if (ddata->clk) { | ||
| ret = devm_clk_rate_exclusive_get(dev, ddata->clk); | ||
| if (ret) | ||
| return dev_err_probe(dev, PTR_ERR(ddata->clk), |
Comment on lines
+13384
to
+13389
| LOONGSON PWM DRIVER | ||
| M: Binbin Zhou <zhoubinbin@loongson.cn> | ||
| L: linux-pwm@vger.kernel.org | ||
| S: Maintained | ||
| F: Documentation/devicetree/bindings/pwm/loongson,ls7a-pwm.yaml | ||
| F: drivers/pwm/pwm-loongson.c |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add support for the Loongson PWM controller and integrate its driver into the LoongArch desktop configuration.
New Features:
Enhancements: