[linux-6.6.y] LoongArch: Fix some issues and new feature support #599
Conversation
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
Reviewer's Guide by SourceryThis pull request includes several backports from upstream, adds Loongson-3C600 multi-chip support, adds support for LoongArch AVEC interrupt controller, adds Loongson-3 CPU hwmon driver support, fixes Loongson-7A2000 related issues, fixes i2c-hid touchpad anomaly, and fixes the latest version of AST driver anomaly. The changes span across multiple files and include improvements to GPIO, memory management, interrupt handling, PCI quirks, and driver support. Updated class diagram for irq-loongson-eiointc.cclassDiagram
class eiointc_priv {
+node: int
+vec_count: int
+node_map: nodemask_t
+domain: irq_domain
}
class extioi_node_map {
+bits: DECLARE_BITMAP
}
note for eiointc_priv "Added eio_node_map"
note for extioi_node_map "Added extioi_node_maps"
Updated class diagram for pch_msi_dataclassDiagram
class pch_msi_data {
+dev: device
+msi_domain: irq_domain
+pci_segment: int
+irq: int
}
note for pch_msi_data "Added pch_msi_acpi_init_avec"
Updated class diagram for loongarch CPU featuresclassDiagram
class cpuinfo_loongarch {
+options: unsigned long
}
note for cpuinfo_loongarch "Added LOONGARCH_CPU_AVECINT"
class loongarch_cpu_features {
+CPU_FEATURE_AVECINT: int
}
note for loongarch_cpu_features "Added CPU_FEATURE_AVECINT"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi @AaronDot. Thanks for your PR. 😃 |
|
Hi @AaronDot. Thanks for your PR. I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @AaronDot - I've reviewed your changes - here's some feedback:
Overall Comments:
- The commit message is very detailed and helpful, but consider summarizing the changes in a more concise way in the title.
- This pull request introduces a lot of new features and bug fixes, so it would be helpful to have a high-level overview of the changes in the commit message.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
332
to
341
| /* wait ready */ | ||
| do { | ||
| j = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff); | ||
| } while ((j & 0x40) == 0); | ||
| } | ||
|
|
||
| void ast_post_gpu(struct drm_device *dev) | ||
| { | ||
| struct ast_private *ast = to_ast_private(dev); | ||
| struct pci_dev *pdev = to_pci_dev(dev->dev); |
There was a problem hiding this comment.
suggestion (bug_risk): Add timeout protection to the busy‐wait loop in ast_post_chip_2300.
Without a timeout, misbehaving hardware could cause an infinite loop. Adding a counter or timeout mechanism would improve robustness.
Suggested implementation:
#include <drm/drm_print.h>
#include "ast_dram_tables.h"
#include "ast_drv.h"
#include <linux/jiffies.h> /* wait ready with timeout */
{
unsigned long timeout = jiffies + msecs_to_jiffies(1000); /* 1000ms timeout */
do {
reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
if (time_after_eq(jiffies, timeout)) {
DRM_ERROR("ast_post_chip_2300: timed out waiting for hardware ready\n");
break;
}
} while ((reg & 0x40) == 0);
}This change adds a 1000ms timeout period. Depending on your hardware expectations and performance requirements, you may adjust the timeout value (e.g. msecs_to_jiffies(500) for 500ms). Also ensure that including <linux/jiffies.h> is acceptable within your code base.
| ast_write32(ast, 0xf004, r & 0xffff0000); | ||
| ast_write32(ast, 0xf000, 0x1); | ||
|
|
||
| do { |
There was a problem hiding this comment.
suggestion: Implement timeout handling for the MCLK2X lock loop.
The current busy‐wait loop relies solely on hardware signaling. A timeout or error detection here could prevent a hang if the expected flag is never set.
Suggested implementation:
/* Wait MCLK2X lock to MCLK with timeout handling */
{
unsigned long timeout = jiffies + msecs_to_jiffies(1000); /* 1s timeout */
do {
data = ast_mindwm(ast, 0x1E6E001C);
if (time_after(jiffies, timeout)) {
DRM_ERROR("MCLK2X lock timeout\n");
break;
}
} while (!(data & 0x08000000));
}Ensure the necessary headers are included at the top of the file if they are not present:
#include <linux/jiffies.h>
#include <linux/delay.h>
#include <drm/drm_print.h>
These headers provide the jiffies API, timeout conversion macros, and DRM error logging.
| ^ ^ ^ | ||
| | | | | ||
| +---------+ +----------+ +---------+ +-------+ | ||
| | EIOINTC | | AVECINTC | | LIOINTC | <-- | UARTs | |
There was a problem hiding this comment.
issue (typo): Typo: "UARTS" should be "UART"
Suggested change
| | EIOINTC | | AVECINTC | | LIOINTC | <-- | UARTs | | |
| | EIOINTC | | AVECINTC | | LIOINTC | <-- | UART | |
| return sprintf(buf, "%d\n", value); | ||
| } | ||
|
|
||
| static SENSOR_DEVICE_ATTR(temp1_input, 0444, get_cpu_temp, NULL, 1); |
There was a problem hiding this comment.
issue (complexity): Consider using macros to define sensor attributes and build the attribute array to reduce code repetition.
You can reduce the repetitive definitions by using helper macros to both generate the sensor attributes and build the attributes array. For example, you might do:
// Macro to declare sensor attributes for a given index 'n'
#define SENSOR_ATTR_ENTRY(n) \
static SENSOR_DEVICE_ATTR(temp##n##_input, 0444, get_cpu_temp, NULL, n); \
static SENSOR_DEVICE_ATTR(temp##n##_label, 0444, cpu_temp_label, NULL, n)
// Generate the sensor attributes (for indices 1..16)
SENSOR_ATTR_ENTRY(1);
SENSOR_ATTR_ENTRY(2);
SENSOR_ATTR_ENTRY(3);
SENSOR_ATTR_ENTRY(4);
SENSOR_ATTR_ENTRY(5);
SENSOR_ATTR_ENTRY(6);
SENSOR_ATTR_ENTRY(7);
SENSOR_ATTR_ENTRY(8);
SENSOR_ATTR_ENTRY(9);
SENSOR_ATTR_ENTRY(10);
SENSOR_ATTR_ENTRY(11);
SENSOR_ATTR_ENTRY(12);
SENSOR_ATTR_ENTRY(13);
SENSOR_ATTR_ENTRY(14);
SENSOR_ATTR_ENTRY(15);
SENSOR_ATTR_ENTRY(16);
// Macro to add sensor attribute pointers into the array
#define SENSOR_ATTR_ITEMS(n) \
&sensor_dev_attr_temp##n##_input.dev_attr.attr, \
&sensor_dev_attr_temp##n##_label.dev_attr.attr
// Build the attribute array using the macro
static struct attribute *cpu_hwmon_attributes[] = {
SENSOR_ATTR_ITEMS(1),
SENSOR_ATTR_ITEMS(2),
SENSOR_ATTR_ITEMS(3),
SENSOR_ATTR_ITEMS(4),
SENSOR_ATTR_ITEMS(5),
SENSOR_ATTR_ITEMS(6),
SENSOR_ATTR_ITEMS(7),
SENSOR_ATTR_ITEMS(8),
SENSOR_ATTR_ITEMS(9),
SENSOR_ATTR_ITEMS(10),
SENSOR_ATTR_ITEMS(11),
SENSOR_ATTR_ITEMS(12),
SENSOR_ATTR_ITEMS(13),
SENSOR_ATTR_ITEMS(14),
SENSOR_ATTR_ITEMS(15),
SENSOR_ATTR_ITEMS(16),
NULL
};This approach keeps functionality intact and improves readability and maintainability by removing repetitive code blocks.
| #include <drm/drm_print.h> | ||
| #include "ast_drv.h" | ||
|
|
||
| int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata) |
There was a problem hiding this comment.
issue (complexity): Consider extracting the EDID ready check and the wait loop into helper functions to improve readability and maintainability.
Consider extracting repeated register condition checks and retry loops into helper functions. For example, you can factor out the EDID-ready check and the wait‐loop into separate routines to reduce nesting and duplication. For instance:
static bool edid_ready(struct ast_private *ast)
{
return ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, ASTDP_MCU_FW_EXECUTING) &&
ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDC, ASTDP_LINK_SUCCESS) &&
ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDF, ASTDP_HPD) &&
ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5, ASTDP_HOST_EDID_READ_DONE_MASK);
}
static int wait_for_edid_valid(struct ast_private *ast, u8 pointer)
{
int retry = 0;
/* Wait until EDID valid flag and read pointer match expectation */
while ((ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD7, ASTDP_EDID_VALID_FLAG_MASK) != 0x01) ||
(ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD6, ASTDP_EDID_READ_POINTER_MASK) != pointer)) {
if (!edid_ready(ast))
return -EINVAL; // or appropriate error code
mdelay(retry + 1);
if (++retry > 200)
return -ETIMEDOUT;
}
return 0;
}Then in your ast_astdp_read_edid function, you can greatly simplify the logic:
if (!edid_ready(ast))
goto err_astdp_edid_not_ready;
ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5, (u8)~ASTDP_HOST_EDID_READ_DONE_MASK, 0x00);
for (i = 0; i < 32; i++) {
ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE4, ASTDP_AND_CLEAR_MASK, (u8)i);
if (wait_for_edid_valid(ast, i))
goto err_astdp_jump_out_loop_of_edid;
/* read the 4 EDID bytes */
*(ediddata) = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD8, ASTDP_EDID_READ_DATA_MASK);
*(ediddata + 1) = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD9, ASTDP_EDID_READ_DATA_MASK);
*(ediddata + 2) = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDA, ASTDP_EDID_READ_DATA_MASK);
*(ediddata + 3) = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xDB, ASTDP_EDID_READ_DATA_MASK);
if (i == 31) {
*(ediddata + 3) += *(ediddata + 2);
*(ediddata + 2) = 0;
}
ediddata += 4;
}This approach keeps the functionality intact while reducing nesting and duplication, making the code easier to read and maintain.
Member
|
/lgtm |
opsiff
previously requested changes
Feb 16, 2025
opsiff
reviewed
Feb 16, 2025
| CONFIG_DRM_QXL=m | ||
| CONFIG_DRM_VIRTIO_GPU=m | ||
| CONFIG_DRM_LOONGSON=y | ||
| CONFIG_DRM_LOONGSON=m |
Member
|
建议upstream的commit一并标明upstream的commit sha256号 |
Contributor
Author
好的，等该系列补丁疑问都解决后， 我会加上 upstream commit！ |
Contributor
Author
|
Update: |
To be compatible with OLD firmware which has no _DMA method, we should use arch specific phys_to_dma. Signed-off-by: Hongchen Zhang <zhanghongchen@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
commit d23b779 upstream. LoongArch has hardware page coloring for L1 Cache, so we don't have cache aliases. But SFB (Store Fill Buffer) still has aliases. So we define SHMLBA to SZ_64K previously. But there are losts of applications use PAGE_SIZE rather than SHMLBA to mmap() file pages and shared pages. Of course we can fix them one by one, but not easy. On the other hand, we can simply disable SFB for 4KB page size to fix cache alias (there will be performance decrease, but acceptable), and in future we will fix SFB in hardware. So we can safely define SHMLBA to PAGE_SIZE (use the generic shmparam.h) to make life easier. Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
commit 8e02c3b upstream. Currently, only TLB-based ioremap() support writecombine, so add the counterpart for DMW-based ioremap() with help of DMW2. The base address (WRITECOMBINE_BASE) is configured as 0xa000000000000000. DMW3 is unused by kernel now, however firmware may leave garbage in them and interfere kernel's address mapping. So clear it as necessary. BTW, centralize the DMW configuration to macro SETUP_DMWINS. Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
commit 82bf60a upstream. LoongArch doesn't have cache aliases, so flush_dcache_page() is a no-op. There is a generic implementation for this case in include/asm-generic/ cacheflush.h. So remove the superfluous flush_dcache_page() definition, which also silences such build warnings: In file included from crypto/scompress.c:12: include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone': include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but not used [-Wunused-but-set-variable] 76 | struct page *page; | ^~~~ crypto/scompress.c: In function 'scomp_acomp_comp_decomp': >> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunused-variable] 174 | struct page *dst_page = sg_page(req->dst); | Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202403091614.NeUw5zcv-lkp@intel.com/ Suggested-by: Barry Song <baohua@kernel.org> Acked-by: Barry Song <baohua@kernel.org> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
…ference commit 4574815 upstream. As very well explained in commit 20a004e ("arm64: mm: Use READ_ONCE/WRITE_ONCE when accessing page tables"), an architecture whose page table walker can modify the PTE in parallel must use READ_ONCE()/ WRITE_ONCE() macro to avoid any compiler transformation. So apply that to LoongArch which is such an architecture, in order to avoid potential problems. Similar to commit edf9556 ("riscv: Use accessors to page table entries instead of direct dereference"). Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
commit f93f67d upstream. LoongArch has similar problems explained in commit 7f0b1bf ("arm64: Fix barriers used for page table modifications"), when hardware page table walker (PTW) enabled, speculative accesses may cause spurious page fault in kernel space. Theoretically, in order to completely avoid spurious page fault we need a "dbar + ibar" pair between the page table modifications and the subsequent memory accesses using the corresponding virtual address. But "ibar" is too heavy for performace, so we only use a "dbar 0b11000" in set_pte(). And let spurious_fault() filter the rest rare spurious page faults which should be avoided by "ibar". Besides, we replace the llsc loop with amo in set_pte() which has better performace, and refactor mmu_context.h to 1) avoid any load/store/branch instructions between the writing of CSR.ASID & CSR.PGDL, 2) ensure flush tlb operation is after updating ASID. Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
commit d2f8671 upstream. There are two pages in one TLB entry on LoongArch system. For kernel space, it requires both two pte entries (buddies) with PAGE_GLOBAL bit set, otherwise HW treats it as non-global tlb, there will be potential problems if tlb entry for kernel space is not global. Such as fail to flush kernel tlb with the function local_flush_tlb_kernel_range() which supposed only flush tlb with global bit. Kernel address space areas include percpu, vmalloc, vmemmap, fixmap and kasan areas. For these areas both two consecutive page table entries should be enabled with PAGE_GLOBAL bit. So with function set_pte() and pte_clear(), pte buddy entry is checked and set besides its own pte entry. However it is not atomic operation to set both two pte entries, there is problem with test_vmalloc test case. So function kernel_pte_init() is added to init a pte table when it is created for kernel address space, and the default initial pte value is PAGE_GLOBAL rather than zero at beginning. Then only its own pte entry need update with function set_pte() and pte_clear(), nothing to do with the pte buddy entry. Signed-off-by: Bibo Mao <maobibo@loongson.cn> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
commit ae16f05 upstream. Introduce the advanced extended interrupt controllers (AVECINTC). This feature will allow each core to have 256 independent interrupt vectors and MSI interrupts can be independently routed to any vector on any CPU. Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn> Co-developed-by: Liupu Wang <wangliupu@loongson.cn> Signed-off-by: Liupu Wang <wangliupu@loongson.cn> Co-developed-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
This add CPU HWMon (temperature sensor) platform driver for Loongson-3. Tested-by: Xi Ruoyao <xry111@xry111.site> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Kexy Biscuit <kexybiscuit@aosc.io> Signed-off-by: gaojuxin <gaojuxin@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Signed-off-by: wanghongliang <wanghongliang@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Signed-off-by: wanghongliang <wanghongliang@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
commit 2af2573 upstream. avecintc_init() enables the Advanced Interrupt Controller (AVEC) of the boot CPU node, but nothing enables the AVEC on secondary nodes. Move the enablement to the CPU hotplug callback so that secondary nodes get the AVEC enabled too. In theory enabling it once per node would be sufficient, but redundant enabling does no hard, so keep the code simple and do it unconditionally. Signed-off-by: Tianyang Zhang <zhangtianyang@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Signed-off-by: wanghongliang <wanghongliang@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
This is similar to commit 62b6dee ("PCI/portdrv: Prevent LS7A Bus Master clearing on shutdown"), which prevents LS7A Bus Master clearing on kexec. Only skip Bus Master clearing on bridges because endpoint devices still need it. Signed-off-by: Ming Wang <wangming01@loongson.cn> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
The CPU may not be able to access the PCI header of the device if it is in low-power mode. Signed-off-by: Zhao Qunqin <zhaoqunqin@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
commit 1f35a0c upstream. Beginning with Loongson-3C6000, there can be up to 8 PCI hosts for multi-chip machines. To support these machines, increase the number of entries in mcfg_quirks to 8. Link: https://lore.kernel.org/r/20240726092911.2042656-1-chenhuacai@loongson.cn Signed-off-by: Haowei Zheng <zhenghaowei@loongson.cn> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Loongson drm driver will not be used in the future, use loongson drm dkms package instead. Signed-off-by: Hongchen Zhang <zhanghongchen@loongson.cn>
The policy->cur was not being initialized properly during CPU initialization, leading to it always reporting 0. This commit addresses this issue by setting the initial frequency to the normal maximum frequency. This ensures that the current frequency reflects the actual CPU operating frequency. Signed-off-by: Ming Wang <wangming01@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Loongson's DWMAC device may take nearly two seconds to complete DMA reset, however, the default waiting time for reset is 200 milliseconds. Fixes: 803fc61 ("net: stmmac: dwmac-loongson: Add Loongson Multi-channels GMAC support") Signed-off-by: Qunqin Zhao <zhaoqunqin@loongson.cn> Reviewed-by: Huacai Chen <chenhuacai@loongson.cn> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
Contributor
Author
|
更新：
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sourcery-ai[bot] 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sourcery-ai[bot] 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 |
Avenger-285714
merged commit b1c3e27
into
deepin-community:linux-6.6.y
6 of 8 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Introduction to the patch set:
Summary by Sourcery
Add support for the LoongArch AVEC interrupt controller, Loongson-3C600 multi-chip, and Loongson-3 CPU hwmon driver. Backport patches from upstream. Fix issues related to Loongson-7A2000, i2c-hid touchpad, and AST driver.
New Features:
Bug Fixes:
Enhancements: