[Deepin-Kernel-SIG] [linux 6.6-y] [Montage] montage: update Mont-TSSE driver #623
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
update Montage Mont-TSSE driver from 1.0.0 to 1.1.2 in 6.6. New version has optimized the code structure and IPC function and fixed some bugs. Link: https://gitee.com/OpenCloudOS/OpenCloudOS-Kernel/pulls/311 Signed-off-by: Carrie.Cai <carrie.cai@montage-tech.com> [ WangYuli: Fix some conflict ] Signed-off-by: WangYuli <wangyuli@uniontech.com>
Reviewer's Guide by SourceryThis pull request updates the Montage Mont-TSSE driver from version 1.0.0 to 1.1.2. The update includes a refactored IPC mechanism using a new message structure and dispatching method, updates to the firmware loading process with firmware version retrieval, and refactored device management with handle-based device access. The copyright year has also been updated. Sequence diagram for Host to Device Firmware LoadingsequenceDiagram
participant Host
participant TSSE Driver
participant TSSE Device
Host->>TSSE Driver: Requests firmware load (tsse_fw_manual_load_ipc)
TSSE Driver->>TSSE Driver: Initializes IPC (ipc_hw_init)
TSSE Driver->>TSSE Driver: Sends host init message (host_init_msg)
TSSE Driver->>TSSE Device: Writes message to H2M (HOST2MAIN_IPC_OFFSET)
TSSE Driver->>TSSE Device: Sets interrupt (HOST2MAIN_INTR_SET_OFFSET)
TSSE Device->>TSSE Driver: Generates interrupt
TSSE Driver->>TSSE Driver: Handles interrupt (tsse_ipc_d2h_irqhandler)
TSSE Driver->>TSSE Driver: Processes message (tsse_ipc_bh_handler)
TSSE Driver->>TSSE Driver: Routes service (service_rout)
TSSE Driver->>TSSE Driver: Executes firmware service (fw_service)
TSSE Driver->>TSSE Device: Writes firmware data to device memory
TSSE Driver->>TSSE Device: Writes message to H2M (HOST2MAIN_IPC_OFFSET)
TSSE Driver->>TSSE Device: Sets interrupt (HOST2MAIN_INTR_SET_OFFSET)
Updated class diagram for TSSE IPCclassDiagram
class tsse_ipc {
-struct device *dev
-struct pci_dev *pdev
-void __iomem *virt_addr
-struct mutex list_lock
-struct tasklet_struct ipc_handle
-tsse_d2h_ipc_handler d2h_handlers[IPC_MESSAGE_CLASS_NUM]
-u32 im_inited
}
class tsse_dev {
}
class tsse_d2h_ipc_handler{
<<interface>>
+int handle(int handle, void *msg_payload, uint32_t payload_length)
}
tsse_ipc -- tsse_dev : has a
tsse_ipc .. tsse_d2h_ipc_handler : Uses
note for tsse_ipc "Updated tsse_ipc class with d2h_handlers and im_inited field"
Class diagram for TSSE ServiceclassDiagram
class tsse_service_instance {
-u8 service_opened
-u8 service_name[TSSE_IM_SERVICE_NAME_LEN]
-int device_handle
-tsse_im_cb_func cb
-u64 service_epid
}
class tsse_im_cb_func{
<<interface>>
+int handle(tsse_im_service_handle handle, void *msg_payload, u32 payload_length)
}
class tsse_dev {
}
tsse_service_instance -- tsse_dev : Uses
tsse_service_instance .. tsse_im_cb_func : Uses
note for tsse_service_instance "Represents an instance of a TSSE service"
Class diagram for TSSE IPC MessageclassDiagram
class tsse_ipc_msg {
-u16 type
-u16 msg_len
-u32 rev
-u64 epid
-u8 data[]
}
note for tsse_ipc_msg "Represents the structure of an IPC message"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
Author
|
Test passed on clang-19. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Avenger-285714 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider using dev_printk instead of pr_info/pr_err to control verbosity.
- It looks like you're adding a lot of new files - consider adding a short description of each in the commit message.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue 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.
| #define SET_BIT(bitmap, bit) (bitmap[(bit) / 8] |= (1 << ((bit) % 8))) | ||
| #define CLEAR_BIT(bitmap, bit) (bitmap[(bit) / 8] &= ~(1 << ((bit) % 8))) | ||
|
|
||
| static u8 service_id_bitmap[SERVICE_BITMAP_SIZE] = {0}; |
There was a problem hiding this comment.
suggestion (bug_risk): Consider adding locking for bitmap accesses.
The global 'service_id_bitmap' and the 'current_max_service_id' are accessed and updated in tsse_available_service_id without any explicit synchronization. In a concurrent environment, especially considering these functions might be invoked from different contexts, it would be safer to protect these accesses with a spinlock or another appropriate synchronization mechanism.
|
|
||
| writel(0x0, tsseipc->virt_addr + MAIN2HOST_INTR_SET_OFFSET); | ||
| tasklet_schedule(&tsseipc->ipc_handle); | ||
| dev_err(tsseipc->dev, "irq%d\n", irq); |
There was a problem hiding this comment.
suggestion: Consider lowering the log level for IRQ handler messages.
Logging every IRQ as an error may lead to log spam if these interrupts are frequent and expected. If this message is informational, consider using dev_info or dev_dbg instead of dev_err to avoid unnecessary error logging.
Suggested change
| dev_err(tsseipc->dev, "irq%d\n", irq); | |
| dev_info(tsseipc->dev, "irq%d\n", irq); |
| return ret; | ||
| } | ||
|
|
||
| int tsse_service_msg_send( |
There was a problem hiding this comment.
issue (complexity): Consider creating a common helper function to encapsulate the request setup and handling pattern used in several functions to reduce code duplication.
Consider consolidating the common request‐setup and handling pattern. For example, several functions allocate a request object and a user data block, initialize a completion, fill the header fields, send the message, and then call the post‐process function. Consolidating that flow into a common helper function will simplify the code and reduce duplication.
For example:
static int tsse_service_common_request(void *req, size_t req_size,
tsse_im_service_handle handle,
u32 service_cmd,
post_process_func pp_func)
{
struct tsse_service_user_data *user_data;
int ret;
user_data = kzalloc(sizeof(*user_data), GFP_ATOMIC);
if (!user_data) {
kfree(req);
return -ENOMEM;
}
init_completion(&user_data->req_completion);
/* Assuming all reqs start with tsse_service_req header */
((struct tsse_service_req *)req)->hdr.msg_type = IM_MSG_TYPE_REQ;
((struct tsse_service_req *)req)->hdr.cookie = (u64)user_data;
ret = tsse_service_msg_send(handle, service_cmd, req, req_size);
return service_request_post_process(ret, req, user_data, handle, pp_func);
}Then refactor functions like tsse_service_open, tsse_service_close, and tsse_services_query_request to use this helper:
int tsse_service_open(tsse_im_service_handle handle)
{
struct tsse_service_open_req *req;
int ret;
ret = tsse_alloc_service_epid(handle);
if (ret)
return ret;
req = kzalloc(sizeof(*req), GFP_ATOMIC);
if (!req)
return -ENOMEM;
/* Populate open specific data */
tsse_service_instance_cast(handle)->service_opened = 0;
memcpy(req->service_name, tsse_service_instance_cast(handle)->service_name,
TSSE_IM_SERVICE_NAME_LEN);
return tsse_service_common_request((void *)req, sizeof(*req),
handle, TSSE_SERVICE_CMD_OPEN,
tsse_service_open_post_process);
}This refactoring pinpoints the shared pattern and reduces the repetitive code while keeping functionality intact.
|
[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 805de0b
into
deepin-community:linux-6.6.y
4 of 5 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
update Montage Mont-TSSE driver from 1.0.0 to 1.1.2 in 6.6. New version has optimized the code structure and IPC function and fixed some bugs.
Link: https://gitee.com/OpenCloudOS/OpenCloudOS-Kernel/pulls/311
[ WangYuli: Fix some conflict ]
Summary by Sourcery
This pull request updates the Montage Mont-TSSE driver from version 1.0.0 to 1.1.2, which includes code structure optimizations, IPC function improvements, and bug fixes. It also adds a mechanism to disable ASPM completely to avoid performance issues.
Bug Fixes:
Enhancements:
Build:
CI: