[PATCH] bus: mhi: host: Change the trace string for the userspace tools mapping
User space tools can't map strings if we use directly, as the string
address is internal to kernel.
So add trace point strings for the user space tools to map strings
properly.
Signed-off-by: Krishna chaitanya chundru
---
drivers/bus/mhi/host/main.c | 4 ++--
drivers/bus/mhi/host/trace.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index 2d38f6005da6..15d657af9b5b 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -1340,7 +1340,7 @@ static int mhi_update_channel_state(struct mhi_controller
*mhi_cntrl,
enum mhi_cmd_type cmd = MHI_CMD_NOP;
int ret;
- trace_mhi_channel_command_start(mhi_cntrl, mhi_chan, to_state,
"Updating");
+ trace_mhi_channel_command_start(mhi_cntrl, mhi_chan, to_state,
TPS("Updating"));
switch (to_state) {
case MHI_CH_STATE_TYPE_RESET:
write_lock_irq(_chan->lock);
@@ -1407,7 +1407,7 @@ static int mhi_update_channel_state(struct mhi_controller
*mhi_cntrl,
write_unlock_irq(_chan->lock);
}
- trace_mhi_channel_command_end(mhi_cntrl, mhi_chan, to_state, "Updated");
+ trace_mhi_channel_command_end(mhi_cntrl, mhi_chan, to_state,
TPS("Updated"));
exit_channel_update:
mhi_cntrl->runtime_put(mhi_cntrl);
mhi_device_put(mhi_cntrl->mhi_dev);
diff --git a/drivers/bus/mhi/host/trace.h b/drivers/bus/mhi/host/trace.h
index d12a98d44272..368515dcb22d 100644
--- a/drivers/bus/mhi/host/trace.h
+++ b/drivers/bus/mhi/host/trace.h
@@ -84,6 +84,8 @@ DEV_ST_TRANSITION_LIST
#define dev_st_trans(a, b) { DEV_ST_TRANSITION_##a, b },
#define dev_st_trans_end(a, b) { DEV_ST_TRANSITION_##a, b }
+#define TPS(x) tracepoint_string(x)
+
TRACE_EVENT(mhi_gen_tre,
TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
---
base-commit: ceeb64f41fe6a1eb9fc56d583983a81f8f3dd058
change-id: 20240218-ftrace_string-7677762aa63c
Best regards,
--
Krishna chaitanya chundru
Re: [PATCH v11] bus: mhi: host: Add tracing support
On 2/6/2024 11:56 AM, Manivannan Sadhasivam wrote:
On Tue, Feb 06, 2024 at 10:02:05AM +0530, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Change the implementation of the arrays which has enum to strings mapping
to make it consistent in both trace header file and other files.
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
There are a lot of checkpatch errors. Please fix them and resubmit.
- Mani
Hi Mani,
The errors which is pointing in the checkpatch are false positive, those
errors are being shown from v2 onwards and kernel test boot is also not
throwing any errors for it.
I checked with internal team they said these errors are false positive.
Thanks & Regards,
Krishna Chaitanya.
Reviewed-by: Manivannan Sadhasivam
Reviewed-by: "Steven Rostedt (Google)"
---
Changes in v11:
- Rebased with mhi next.
- Link to v10:
https://lore.kernel.org/r/20240131-ftrace_support-v10-1-4349306b8...@quicinc.com
Changes in v10:
- Modified command_start and command_end traces to take string as input to
mention correct
- string as suggested by mani
- As sugggested by mani modified the print format from lower format to upper
case format.
- Link to v9:
https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com
Changes in v9:
- Change the implementations of some array so that the strings to enum mapping
- is same in both trace header and other files as suggested by steve.
- Link to v8:
https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/common.h| 38 +++---
drivers/bus/mhi/host/init.c | 64 +
drivers/bus/mhi/host/internal.h | 41 ++
drivers/bus/mhi/host/main.c | 19 ++-
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h| 280
6 files changed, 384 insertions(+), 65 deletions(-)
diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
index f794b9c8049e..dda340aaed95 100644
--- a/drivers/bus/mhi/common.h
+++ b/drivers/bus/mhi/common.h
@@ -297,30 +297,30 @@ struct mhi_ring_element {
__le32 dword[2];
};
+#define MHI_STATE_LIST\
+ mhi_state(RESET,"RESET") \
+ mhi_state(READY,"READY") \
+ mhi_state(M0, "M0") \
+ mhi_state(M1, "M1") \
+ mhi_state(M2, "M2") \
+ mhi_state(M3, "M3") \
+ mhi_state(M3_FAST, "M3_FAST")\
+ mhi_state(BHI,
[PATCH v11] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Change the implementation of the arrays which has enum to strings mapping
to make it consistent in both trace header file and other files.
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
Reviewed-by: Manivannan Sadhasivam
Reviewed-by: "Steven Rostedt (Google)"
---
Changes in v11:
- Rebased with mhi next.
- Link to v10:
https://lore.kernel.org/r/20240131-ftrace_support-v10-1-4349306b8...@quicinc.com
Changes in v10:
- Modified command_start and command_end traces to take string as input to
mention correct
- string as suggested by mani
- As sugggested by mani modified the print format from lower format to upper
case format.
- Link to v9:
https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com
Changes in v9:
- Change the implementations of some array so that the strings to enum mapping
- is same in both trace header and other files as suggested by steve.
- Link to v8:
https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/common.h| 38 +++---
drivers/bus/mhi/host/init.c | 64 +
drivers/bus/mhi/host/internal.h | 41 ++
drivers/bus/mhi/host/main.c | 19 ++-
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h| 280
6 files changed, 384 insertions(+), 65 deletions(-)
diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
index f794b9c8049e..dda340aaed95 100644
--- a/drivers/bus/mhi/common.h
+++ b/drivers/bus/mhi/common.h
@@ -297,30 +297,30 @@ struct mhi_ring_element {
__le32 dword[2];
};
+#define MHI_STATE_LIST \
+ mhi_state(RESET,"RESET")\
+ mhi_state(READY,"READY")\
+ mhi_state(M0, "M0") \
+ mhi_state(M1, "M1") \
+ mhi_state(M2, "M2") \
+ mhi_state(M3, "M3") \
+ mhi_state(M3_FAST, "M3_FAST") \
+ mhi_state(BHI, "BHI") \
+ mhi_state_end(SYS_ERR, "SYS ERROR")
+
+#undef mhi_state
+#undef mhi_state_end
+
+#define mhi_state(a, b)case MHI_STATE_##a: return b;
+#define mhi_state_end(a, b)case MHI_STATE_##a: return b;
+
static inline const char *mhi_state_str(enum mhi_state state)
{
switch (state) {
- case MHI_STATE_RESET:
- return "RESET";
- case MHI_STATE_READY:
- return &quo
[PATCH v10] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Change the implementation of the arrays which has enum to strings mapping
to make it consistent in both trace header file and other files.
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
Reviewed-by: "Steven Rostedt (Google)"
---
Changes in v10:
- Modified command_start and command_end traces to take string as input to
mention correct
- string as suggested by mani
- As sugggested by mani modified the print format from lower format to upper
case format.
- Link to v9:
https://lore.kernel.org/r/20240105-ftrace_support-v9-1-a2dca64cc...@quicinc.com
Changes in v9:
- Change the implementations of some array so that the strings to enum mapping
- is same in both trace header and other files as suggested by steve.
- Link to v8:
https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/common.h| 38 +++---
drivers/bus/mhi/host/init.c | 63 +
drivers/bus/mhi/host/internal.h | 40 ++
drivers/bus/mhi/host/main.c | 19 ++-
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h| 280
6 files changed, 383 insertions(+), 64 deletions(-)
diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
index f794b9c8049e..dda340aaed95 100644
--- a/drivers/bus/mhi/common.h
+++ b/drivers/bus/mhi/common.h
@@ -297,30 +297,30 @@ struct mhi_ring_element {
__le32 dword[2];
};
+#define MHI_STATE_LIST \
+ mhi_state(RESET,"RESET")\
+ mhi_state(READY,"READY")\
+ mhi_state(M0, "M0") \
+ mhi_state(M1, "M1") \
+ mhi_state(M2, "M2") \
+ mhi_state(M3, "M3") \
+ mhi_state(M3_FAST, "M3_FAST") \
+ mhi_state(BHI, "BHI") \
+ mhi_state_end(SYS_ERR, "SYS ERROR")
+
+#undef mhi_state
+#undef mhi_state_end
+
+#define mhi_state(a, b)case MHI_STATE_##a: return b;
+#define mhi_state_end(a, b)case MHI_STATE_##a: return b;
+
static inline const char *mhi_state_str(enum mhi_state state)
{
switch (state) {
- case MHI_STATE_RESET:
- return "RESET";
- case MHI_STATE_READY:
- return "READY";
- case MHI_STATE_M0:
- return "M0";
- case MHI_STATE_M1:
- return "M1";
- case MHI_STATE_M2:
-
Re: [PATCH v9] bus: mhi: host: Add tracing support
On 1/30/2024 11:56 PM, Manivannan Sadhasivam wrote: On Tue, Jan 30, 2024 at 09:22:52AM -0500, Steven Rostedt wrote: On Tue, 30 Jan 2024 13:41:52 +0530 Manivannan Sadhasivam wrote: So same trace will get printed for both mhi_channel_command_start() and mhi_channel_command_end()? The trace output will also include the tracepoint name. That is, it will have the same content but will be preceded with: mhi_channel_command_start: ... mhi_channel_command_end: ... Yes, but the message will be the same: mhi_channel_command_start: chan%d: Updating state to: mhi_channel_command_end: chan%d: Updating state to: Either only one of the trace should be present or the second one should print, "mhi_channel_command_end: chan%d: Updated state to:" - Mani I will try to pass a string to updated for mhi_channel_command_end & updating for mhi_channel_command_start in my next patch. - Krishna Chaitanya.
Re: [PATCH v9] bus: mhi: host: Add tracing support
On 1/30/2024 1:41 PM, Manivannan Sadhasivam wrote:
On Fri, Jan 05, 2024 at 05:53:03PM +0530, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Change the implementation of the arrays which has enum to strings mapping
to make it consistent in both trace header file and other files.
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
Few nitpicks below.
I will make suggested changes in next patch.
- Krishna Chaitanya.
Reviewed-by: "Steven Rostedt (Google)"
---
Changes in v9:
- Change the implementations of some array so that the strings to enum mapping
- is same in both trace header and other files as suggested by steve.
- Link to v8:
https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/common.h| 38 +++---
drivers/bus/mhi/host/init.c | 63 +
drivers/bus/mhi/host/internal.h | 40 ++
drivers/bus/mhi/host/main.c | 19 ++-
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h| 275
6 files changed, 378 insertions(+), 64 deletions(-)
[...]
+TRACE_EVENT(mhi_gen_tre,
+
+ TP_PROTO(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
+struct mhi_ring_element *mhi_tre),
+
+ TP_ARGS(mhi_cntrl, mhi_chan, mhi_tre),
+
+ TP_STRUCT__entry(
+ __string(name, mhi_cntrl->mhi_dev->name)
+ __field(int, ch_num)
+ __field(void *, wp)
+ __field(__le64, tre_ptr)
+ __field(__le32, dword0)
+ __field(__le32, dword1)
+ ),
+
+ TP_fast_assign(
+ __assign_str(name, mhi_cntrl->mhi_dev->name);
+ __entry->ch_num = mhi_chan->chan;
+ __entry->wp = mhi_tre;
+ __entry->tre_ptr = mhi_tre->ptr;
+ __entry->dword0 = mhi_tre->dword[0];
+ __entry->dword1 = mhi_tre->dword[1];
+ ),
+
+ TP_printk("%s: Chan: %d Tre: 0x%p Tre buf: 0x%llx dword0: 0x%08x dword1:
0x%08x\n",
Use caps for printing the acronyms everywhere. Like TRE, DWORD etc...
+ __get_str(name), __entry->ch_num, __entry->wp,
__entry->tre_ptr,
+ __entry->dword0, __entry->dword1)
+);
+
+TRACE_EVENT(mhi_intvec_states,
+
+ TP_PROTO(struct mhi_controller *mhi_cntrl, int dev_ee, int dev_state),
+
+ TP_ARGS(mhi_cntrl, dev_ee, dev_state),
+
+ TP_STRUCT__entry(
+ __string(name, mhi_cntrl->mhi_dev->name)
+ __field(in
Re: [PATCH v9] bus: mhi: host: Add tracing support
On 1/8/2024 6:52 PM, Krishna Chaitanya Chundru wrote:
Hi Steven,
Even though I added your reviewed-by tag, I incorporated changes
mentioned in the previous patch.
Can you please review it once.
Thanks & Regards,
Krishna Chaitanya.
Hi Steven,
Can you please review it once.
Thanks & Regards,
Krishna Chaitanya.
On 1/5/2024 5:53 PM, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Change the implementation of the arrays which has enum to strings mapping
to make it consistent in both trace header file and other files.
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
Reviewed-by: "Steven Rostedt (Google)"
---
Changes in v9:
- Change the implementations of some array so that the strings to enum
mapping
- is same in both trace header and other files as suggested by steve.
- Link to v8:
https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the
address to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to
rebasing.
- In the defconfig FTRACE config is not enabled due to that the
compilation issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to
drivers/bus/mhi/host/ so that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid
holes in the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver
headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using
__print_symbolic() as suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/common.h | 38 +++---
drivers/bus/mhi/host/init.c | 63 +
drivers/bus/mhi/host/internal.h | 40 ++
drivers/bus/mhi/host/main.c | 19 ++-
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h | 275
6 files changed, 378 insertions(+), 64 deletions(-)
diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
index f794b9c8049e..dda340aaed95 100644
--- a/drivers/bus/mhi/common.h
+++ b/drivers/bus/mhi/common.h
@@ -297,30 +297,30 @@ struct mhi_ring_element {
__le32 dword[2];
};
+#define MHI_STATE_LIST \
+ mhi_state(RESET, "RESET") \
+ mhi_state(READY, "READY") \
+ mhi_state(M0, "M0") \
+ mhi_state(M1, "M1") \
+ mhi_state(M2, "M2") \
+ mhi_state(M3, "M3") \
+ mhi_state(M3_FAST, "M3_FAST") \
+ mhi_state(BHI, "BHI") \
+ mhi_state_end(SYS_ERR, "SYS ERROR")
+
+#undef mhi_state
+#undef mhi_state_end
+
+#define mhi_state(a, b) case MHI_STATE_##a: return b;
+#define mhi_state_end(a, b) case MHI_STATE_##a: return b;
+
static inline const char *mhi_state_str(enum mhi_state state)
{
switch (state) {
- case MHI_STATE_RESET:
- return "RESET";
- case MHI_STATE_READY:
- return "READY";
- case MHI_STATE_M0:
- return "M0";
- case MHI_STATE_M1:
- return "M1";
- case MHI_STATE_M2:
- return "M2"
Re: [PATCH v9] bus: mhi: host: Add tracing support
Hi Steven,
Even though I added your reviewed-by tag, I incorporated changes
mentioned in the previous patch.
Can you please review it once.
Thanks & Regards,
Krishna Chaitanya.
On 1/5/2024 5:53 PM, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Change the implementation of the arrays which has enum to strings mapping
to make it consistent in both trace header file and other files.
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
Reviewed-by: "Steven Rostedt (Google)"
---
Changes in v9:
- Change the implementations of some array so that the strings to enum mapping
- is same in both trace header and other files as suggested by steve.
- Link to v8:
https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/common.h| 38 +++---
drivers/bus/mhi/host/init.c | 63 +
drivers/bus/mhi/host/internal.h | 40 ++
drivers/bus/mhi/host/main.c | 19 ++-
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h| 275
6 files changed, 378 insertions(+), 64 deletions(-)
diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
index f794b9c8049e..dda340aaed95 100644
--- a/drivers/bus/mhi/common.h
+++ b/drivers/bus/mhi/common.h
@@ -297,30 +297,30 @@ struct mhi_ring_element {
__le32 dword[2];
};
+#define MHI_STATE_LIST\
+ mhi_state(RESET,"RESET") \
+ mhi_state(READY,"READY") \
+ mhi_state(M0, "M0") \
+ mhi_state(M1, "M1") \
+ mhi_state(M2, "M2") \
+ mhi_state(M3, "M3") \
+ mhi_state(M3_FAST, "M3_FAST")\
+ mhi_state(BHI, "BHI")\
+ mhi_state_end(SYS_ERR, "SYS ERROR")
+
+#undef mhi_state
+#undef mhi_state_end
+
+#define mhi_state(a, b)case MHI_STATE_##a: return b;
+#define mhi_state_end(a, b)case MHI_STATE_##a: return b;
+
static inline const char *mhi_state_str(enum mhi_state state)
{
switch (state) {
- case MHI_STATE_RESET:
- return "RESET";
- case MHI_STATE_READY:
- return "READY";
- case MHI_STATE_M0:
- return "M0";
- case MHI_STATE_M1:
- return "M1";
- case MHI_STATE_M2:
- return "M2";
- case MHI_STATE_M3:
- return &q
[PATCH v9] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Change the implementation of the arrays which has enum to strings mapping
to make it consistent in both trace header file and other files.
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
Reviewed-by: "Steven Rostedt (Google)"
---
Changes in v9:
- Change the implementations of some array so that the strings to enum mapping
- is same in both trace header and other files as suggested by steve.
- Link to v8:
https://lore.kernel.org/r/20231207-ftrace_support-v8-1-7f62d4558...@quicinc.com
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/common.h| 38 +++---
drivers/bus/mhi/host/init.c | 63 +
drivers/bus/mhi/host/internal.h | 40 ++
drivers/bus/mhi/host/main.c | 19 ++-
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h| 275
6 files changed, 378 insertions(+), 64 deletions(-)
diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
index f794b9c8049e..dda340aaed95 100644
--- a/drivers/bus/mhi/common.h
+++ b/drivers/bus/mhi/common.h
@@ -297,30 +297,30 @@ struct mhi_ring_element {
__le32 dword[2];
};
+#define MHI_STATE_LIST \
+ mhi_state(RESET,"RESET")\
+ mhi_state(READY,"READY")\
+ mhi_state(M0, "M0") \
+ mhi_state(M1, "M1") \
+ mhi_state(M2, "M2") \
+ mhi_state(M3, "M3") \
+ mhi_state(M3_FAST, "M3_FAST") \
+ mhi_state(BHI, "BHI") \
+ mhi_state_end(SYS_ERR, "SYS ERROR")
+
+#undef mhi_state
+#undef mhi_state_end
+
+#define mhi_state(a, b)case MHI_STATE_##a: return b;
+#define mhi_state_end(a, b)case MHI_STATE_##a: return b;
+
static inline const char *mhi_state_str(enum mhi_state state)
{
switch (state) {
- case MHI_STATE_RESET:
- return "RESET";
- case MHI_STATE_READY:
- return "READY";
- case MHI_STATE_M0:
- return "M0";
- case MHI_STATE_M1:
- return "M1";
- case MHI_STATE_M2:
- return "M2";
- case MHI_STATE_M3:
- return "M3";
- case MHI_STATE_M3_FAST:
- return "M3 FAST";
- case MHI_STATE_BHI:
- return "BHI";
- case MHI_STATE_SYS_ERR:
- return "SYS ERROR";
+
Re: [PATCH v8] bus: mhi: host: Add tracing support
On 1/5/2024 10:47 AM, Baochen Qiang wrote:
On 1/4/2024 12:47 PM, Krishna Chaitanya Chundru wrote:
Hi Steven,
Can you please review this.
Thanks & Regards,
Krishna Chaitanya.
On 12/7/2023 10:00 AM, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Is there a reason why debug messages have to be removed? From the view
of MHI controller, we often need MHI logs to debug, so is it possbile
to preserve those logs?
The trace is being replaced by the debug message and it would be
preferred to have one mechanism or the other, not both.
you will still see these logs in traces.
- Krishna Chaitanya.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign
as suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print
the address to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to
rebasing.
- In the defconfig FTRACE config is not enabled due to that the
compilation issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to
drivers/bus/mhi/host/ so that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid
holes in the buffer.
- removed the mhi_to_physical function as this can give security
issues.
- removed macros to define strings as we can get those from driver
headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using
__print_symbolic() as suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by
bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/main.c | 19 ++--
drivers/bus/mhi/host/pm.c | 7 +-
drivers/bus/mhi/host/trace.h | 205
+++
4 files changed, 221 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..189f4786403e 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -15,6 +15,7 @@
#include
#include
#include "internal.h"
+#include "trace.h"
int __must_check mhi_read_reg(struct mhi_controller *mhi_cntrl,
void __iomem *base, u32 offset, u32 *out)
@@ -491,11 +492,8 @@ irqreturn_t mhi_intvec_threaded_handler(int
irq_number, void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,8 @@ int mhi_process_ctrl_ev_ring(struct
mhi_controller *mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl, local_rp);
+
switc
Re: [PATCH v8] bus: mhi: host: Add tracing support
On 1/4/2024 9:31 PM, Steven Rostedt wrote:
On Thu, 7 Dec 2023 10:00:47 +0530
Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
So this looks good from a tracing POV.
Reviewed-by: Steven Rostedt (Google)
But I do have some more comments that could be done by new patches.
+TRACE_EVENT(mhi_intvec_states,
+
+ TP_PROTO(struct mhi_controller *mhi_cntrl, int dev_ee, int dev_state),
+
+ TP_ARGS(mhi_cntrl, dev_ee, dev_state),
+
+ TP_STRUCT__entry(
+ __string(name, mhi_cntrl->mhi_dev->name)
+ __field(int, local_ee)
+ __field(int, state)
+ __field(int, dev_ee)
+ __field(int, dev_state)
+ ),
+
+ TP_fast_assign(
+ __assign_str(name, mhi_cntrl->mhi_dev->name);
+ __entry->local_ee = mhi_cntrl->ee;
+ __entry->state = mhi_cntrl->dev_state;
+ __entry->dev_ee = dev_ee;
+ __entry->dev_state = dev_state;
+ ),
+
+ TP_printk("%s: local ee: %s state: %s device ee: %s state: %s\n",
+ __get_str(name),
+ TO_MHI_EXEC_STR(__entry->local_ee),
+ mhi_state_str(__entry->state),
+ TO_MHI_EXEC_STR(__entry->dev_ee),
+ mhi_state_str(__entry->dev_state))
So the above may have issues with user space parsing.
For one, that mhi_state_str() is:
static inline const char *mhi_state_str(enum mhi_state state)
{
switch (state) {
case MHI_STATE_RESET:
return "RESET";
case MHI_STATE_READY:
return "READY";
case MHI_STATE_M0:
return "M0";
case MHI_STATE_M1:
return "M1";
case MHI_STATE_M2:
return "M2";
case MHI_STATE_M3:
return "M3";
case MHI_STATE_M3_FAST:
return "M3 FAST";
case MHI_STATE_BHI:
return "BHI";
case MHI_STATE_SYS_ERR:
return "SYS ERROR";
default:
return "Unknown state";
}
};
Which if this could be changed into:
#define MHI_STATE_LIST \
EM(RESET,"RESET") \
EM(READY,"READY") \
EM(M0, "M0")\
EM(M1, "M1")\
EM(M2, "M2")\
EM(M3, "M3")\
EM(M3_FAST, "M3_FAST") \
EM(BHI, "BHI") \
EMe(SYS_ERR, "SYS ERROR")
#undef EM
#undef EMe
#define EM(a, b) case MHI_STATE_##a: return b;
#define EMe(a, b) case MHI_STATE_##a: return b;
static inline const char *mhi_state_str(enum mhi_state state)
{
switch (state) {
MHI_STATE_LIST
default:
return "Unknown state";
}
Then you could use that macro in the trace header:
#undef EM
#undef EMe
#define EM(a, b)TRACE_DEFINE_ENUM(MHI_STATE_##a);
#define EMe(a, b) TRACE_DEFINE_ENUM(MHI_STATE_##a);
MHI_STATE_LIST
And in the print fmts:
#undef EM
#undef EMe
#define EM(a, b) { MHI_STATE_##a, b },
#define EMe(a, b) { MHI_STATE_##a, b }
TP_printk("%s: local ee: %s state: %s device ee: %s state: %s\n",
__get_str(name),
TO_MHI_EXEC_STR(__entry->local_ee),
__print_symbolic(__entry->state), MHI_STATE_LIST),
TO_MHI_EXEC_STR(__entry->dev_ee),
__print_symbolic(__entry->dev_state, MHI_STATE_LIST))
And that will be exported to user space in the
/sys/kernel/tracing/events/*/*/format file, as:
__print_symbolic(REC->state, {
{ MHI_STATE_RESET, "RESET"},
{ MHI_STATE_READY, "READY"},
{ MHI_STATE_M0, "M0"},
{ MHI_STATE_M1, "M1"},
{ MHI_STATE_M2, "M2"},
{ MHI_STATE_M3, "M3"},
{ MHI_STATE_M3_FAST, "M3 FAST&qu
Re: [PATCH v8] bus: mhi: host: Add tracing support
Hi Steven,
Can you please review this.
Thanks & Regards,
Krishna Chaitanya.
On 12/7/2023 10:00 AM, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/main.c | 19 ++--
drivers/bus/mhi/host/pm.c| 7 +-
drivers/bus/mhi/host/trace.h | 205 +++
4 files changed, 221 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..189f4786403e 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -15,6 +15,7 @@
#include
#include
#include "internal.h"
+#include "trace.h"
int __must_check mhi_read_reg(struct mhi_controller *mhi_cntrl,
void __iomem *base, u32 offset, u32 *out)
@@ -491,11 +492,8 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,8 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl, local_rp);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +997,8 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_data_event(mhi_cntrl, local_rp);
+
chan = MHI_TRE_GET_EV_CH
Re: [PATCH v8] bus: mhi: host: Add tracing support
Hi Steven,
Can you review it once.
Thanks & Regards,
Krishna Chaitanya.
On 12/7/2023 10:00 AM, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/main.c | 19 ++--
drivers/bus/mhi/host/pm.c| 7 +-
drivers/bus/mhi/host/trace.h | 205 +++
4 files changed, 221 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..189f4786403e 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -15,6 +15,7 @@
#include
#include
#include "internal.h"
+#include "trace.h"
int __must_check mhi_read_reg(struct mhi_controller *mhi_cntrl,
void __iomem *base, u32 offset, u32 *out)
@@ -491,11 +492,8 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,8 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl, local_rp);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +997,8 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_data_event(mhi_cntrl, local_rp);
+
chan = MHI_TRE_GET_EV_CHID(local_rp)
[PATCH v8] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v8:
- Pass the structure and derefernce the variables in TP_fast_assign as
suggested by steve
- Link to v7:
https://lore.kernel.org/r/20231206-ftrace_support-v7-1-aca49a042...@quicinc.com
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/main.c | 19 ++--
drivers/bus/mhi/host/pm.c| 7 +-
drivers/bus/mhi/host/trace.h | 205 +++
4 files changed, 221 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..189f4786403e 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -15,6 +15,7 @@
#include
#include
#include "internal.h"
+#include "trace.h"
int __must_check mhi_read_reg(struct mhi_controller *mhi_cntrl,
void __iomem *base, u32 offset, u32 *out)
@@ -491,11 +492,8 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,8 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl, local_rp);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +997,8 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_data_event(mhi_cntrl, local_rp);
+
chan = MHI_TRE_GET_EV_CHID(local_rp);
WARN_ON(chan >= mhi_cntrl->max_chan);
@@ -1235,6 +1237,7 @@ int mhi_gen_tre(struct mhi_controller *mhi_
Re: [PATCH v7] bus: mhi: host: Add tracing support
On 12/6/2023 9:25 PM, Steven Rostedt wrote:
On Wed, 6 Dec 2023 21:12:57 +0530
Krishna chaitanya chundru wrote:
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..507cf3351a97 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -15,6 +15,7 @@
#include
#include
#include "internal.h"
+#include "trace.h"
int __must_check mhi_read_reg(struct mhi_controller *mhi_cntrl,
void __iomem *base, u32 offset, u32 *out)
@@ -491,11 +492,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
I'm not sure if this was discussed before, but why not just pass in the
mhi_cntrl pointer and do the assigning in the TRACE_EVENT()
TP_fast_assign() portion?
It will save on setting up the parameters.
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +831,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl->mhi_dev->name, local_rp,
+local_rp->ptr, local_rp->dword[0],
+local_rp->dword[1]);
And here..
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +1000,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_data_event(mhi_cntrl->mhi_dev->name, local_rp, local_rp->ptr,
+local_rp->dword[0], local_rp->dword[1]);
and here..
for the local_rp. Especially since you are just repeating what you send
into the two events. You can consolidate it down to just adding that in the
TP_fast_assign() of the shared DECLARE_EVENT_CLASS().
+
chan = MHI_TRE_GET_EV_CHID(local_rp);
WARN_ON(chan >= mhi_cntrl->max_chan);
@@ -1235,6 +1241,8 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct
mhi_chan *mhi_chan,
mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(info->len);
mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(bei, eot, eob, chain);
+ trace_mhi_gen_tre(mhi_cntrl->mhi_dev->name, mhi_chan->chan, mhi_tre,
+ mhi_tre->ptr, mhi_tre->dword[0], mhi_tre->dword[1]);
Same here. You can pass in the mhi_tre pointer.
/* increment WP */
mhi_add_ring_element(mhi_cntrl, tre_ring);
mhi_add_ring_element(mhi_cntrl, buf_ring);
@@ -1327,9 +1335,7 @@ static int mhi_update_channel_state(struct mhi_controller
*mhi_cntrl,
enum mhi_cmd_type cmd = MHI_CMD_NOP;
int ret;
- dev_dbg(dev, "%d: Updating channel state to: %s\n", mhi_chan->chan,
- TO_CH_STATE_TYPE_STR(to_state));
-
+ trace_mhi_channel_command_start(mhi_cntrl->mhi_dev->name,
mhi_chan->chan, to_state);
And here..
switch (to_state) {
case MHI_CH_STATE_TYPE_RESET:
write_lock_irq(_chan->lock);
@@ -1396,9 +1402,7 @@ static int mhi_update_channel_state(struct mhi_controller
*mhi_cntrl,
write_unlock_irq(_chan->lock);
}
- dev_dbg(dev, "%d: Channel state change to %s successful\n",
- mhi_chan->chan, TO_CH_STATE_TYPE_STR(to_state));
-
+ trace_mhi_channel_command_end(mhi_cntrl->mhi_dev->name, mhi_chan->chan,
to_state);
and here..
Where you can also update the DECLARE_EVENT_CLASS() to directly access the
mhi_cntrl and mhi_chan pointers. Sometimes it's better to do the
dereference from inside the TP_fast_assign. That way, things like eprobes
and bpf tracing can also have access to the full structure if needed.
-- Steve
Hi Steve,
I will make changes as suggested in the next patch series.
- Krishna Chaitanya.
[PATCH v7] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v7:
- change log format as pointed by mani.
- Link to v6:
https://lore.kernel.org/r/20231204-ftrace_support-v6-1-9b206546d...@quicinc.com
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/main.c | 24 ++---
drivers/bus/mhi/host/pm.c| 7 +-
drivers/bus/mhi/host/trace.h | 208 +++
4 files changed, 229 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..507cf3351a97 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -15,6 +15,7 @@
#include
#include
#include "internal.h"
+#include "trace.h"
int __must_check mhi_read_reg(struct mhi_controller *mhi_cntrl,
void __iomem *base, u32 offset, u32 *out)
@@ -491,11 +492,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +831,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl->mhi_dev->name, local_rp,
+local_rp->ptr, local_rp->dword[0],
+local_rp->dword[1]);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +1000,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_data_event(mhi_cntrl->mhi_dev->name, local_rp,
local_rp->ptr,
+local_rp->dword[0], local_rp->dword[1]);
[PATCH v6] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v6:
- use 'rp' directly as suggested by jeffrey.
- Link to v5:
https://lore.kernel.org/r/20231127-ftrace_support-v5-1-eb67daead...@quicinc.com
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 23 +++--
drivers/bus/mhi/host/pm.c | 6 +-
drivers/bus/mhi/host/trace.h| 208
5 files changed, 228 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a02a71605907 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include "trace.h"
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..0d7e068e713a 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -491,11 +491,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl->mhi_dev->name, local_rp,
+local_rp->ptr, local_rp->dword[0],
+local_rp->dword[1]);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +999,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_data_event(mhi_cntrl->mhi_dev->name, local_rp,
local_rp->ptr,
+local_rp->dword[0], local_rp-&
Re: [PATCH v5] bus: mhi: host: Add tracing support
On 12/1/2023 10:31 PM, Jeffrey Hugo wrote:
On 11/27/2023 4:09 AM, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the
address to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to
rebasing.
- In the defconfig FTRACE config is not enabled due to that the
compilation issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to
drivers/bus/mhi/host/ so that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid
holes in the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver
headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using
__print_symbolic() as suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 23 +++--
drivers/bus/mhi/host/pm.c | 6 +-
drivers/bus/mhi/host/trace.h | 208
5 files changed, 228 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h
b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a02a71605907 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include "trace.h"
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..0d7e068e713a 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -491,11 +491,9 @@ irqreturn_t mhi_intvec_threaded_handler(int
irq_number, void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,10 @@ int mhi_process_ctrl_ev_ring(struct
mhi_controller *mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl->mhi_dev->name, local_rp,
+ local_rp->ptr, local_rp->dword[0],
+ local_rp->dword[1]);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +999,9 @@ int mhi_process_data_event_ring(struct
mhi_controller *mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_data_event(mhi_cntrl->mhi_dev->name, local_rp,
local_rp->ptr,
+ local_rp->dword[0], local_rp->dword[1]);
+
chan = MHI_TRE_GET_EV_CHID(local_rp);
WARN_ON(chan >= mhi_cntrl->max_chan);
@@ -1235,6 +1240,8 @@ int mhi_gen_tre(struct mhi_con
[PATCH v5] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever the trace events are added, debug messages are removed.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v5:
- Use DECLARE_EVENT_CLASS for multiple events as suggested by steve.
- Instead of converting to u64 to print address, use %px to print the address
to avoid
- warnings in some platforms.
- Link to v4:
https://lore.kernel.org/r/2023-ftrace_support-v4-1-c83602399...@quicinc.com
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 23 +++--
drivers/bus/mhi/host/pm.c | 6 +-
drivers/bus/mhi/host/trace.h| 208
5 files changed, 228 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a02a71605907 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include "trace.h"
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..0d7e068e713a 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -491,11 +491,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_ctrl_event(mhi_cntrl->mhi_dev->name, local_rp,
+local_rp->ptr, local_rp->dword[0],
+local_rp->dword[1]);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +999,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_data_event(mhi_cntrl->mhi_dev->name, local_rp,
local_rp->ptr,
+local_rp->dword[0], local_rp->dword[1]);
+
chan = MHI_TRE_GET_EV_CHID(local_rp);
WARN_ON(chan >= mhi_cntrl->max_chan);
@@ -1235,6 +1240,8 @@ int mh
Re: [PATCH v4] bus: mhi: host: Add tracing support
On 11/12/2023 1:07 AM, Steven Rostedt wrote:
On Sat, 11 Nov 2023 11:25:22 +0530
Krishna chaitanya chundru wrote:
diff --git a/drivers/bus/mhi/host/trace.h b/drivers/bus/mhi/host/trace.h
new file mode 100644
index ..0e99318f5d08
--- /dev/null
+++ b/drivers/bus/mhi/host/trace.h
+
+TRACE_EVENT(mhi_update_channel_state_start,
+
+ TP_PROTO(const char *name, int ch_num, int state),
+
+ TP_ARGS(name, ch_num, state),
+
+ TP_STRUCT__entry(
+ __string(name, name)
+ __field(int, ch_num)
+ __field(int, state)
+ ),
+
+ TP_fast_assign(
+ __assign_str(name, name);
+ __entry->ch_num = ch_num;
+ __entry->state = state;
+ ),
+
+ TP_printk("%s: ch%d: Updating state to: %s\n",
+ __get_str(name), __entry->ch_num,
+ TO_CH_STATE_TYPE_STR(__entry->state))
+);
+
+TRACE_EVENT(mhi_update_channel_state_end,
+
+ TP_PROTO(const char *name, int ch_num, int state),
+
+ TP_ARGS(name, ch_num, state),
+
+ TP_STRUCT__entry(
+ __string(name, name)
+ __field(int, ch_num)
+ __field(int, state)
+ ),
+
+ TP_fast_assign(
+ __assign_str(name, name);
+ __entry->ch_num = ch_num;
+ __entry->state = state;
+ ),
+
+ TP_printk("%s: ch%d: Updated state to: %s\n",
+ __get_str(name), __entry->ch_num,
+ TO_CH_STATE_TYPE_STR(__entry->state))
+);
+
The above three events have the same format. You can save kilobytes of
memory by converting them into a DECLARE_EVENT_CLASS() and use
DEFINE_EVENT() for each event.
A TRACE_EVENT() macro is really just a wrapper around
DECLARE_EVENT_CLASS() and DEFINE_EVENT(). The DECLARE_EVENT_CLASS()
does the bulk of the work and adds the most memory footprint. By
breaking it apart for several events, it does save memory.
Whenever you can use a single DECLARE_EVENT_CLASS() for multiple
events, I strongly suggest doing so.
Thanks,
-- Steve
Sure steve I will change as suggested in my next patch.
- Krishna Chaitanya.
+#endif
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE trace
+
+#include
---
base-commit: 3006adf3be79cde4d14b1800b963b82b6e5572e0
change-id: 20231005-ftrace_support-6869d4156139
Best regards,
[PATCH v4] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever we added trace events removing debug messages.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v4:
- Fix compilation issues in previous patch which happended due to rebasing.
- In the defconfig FTRACE config is not enabled due to that the compilation
issue is not
- seen in my workspace.
- Link to v3:
https://lore.kernel.org/r/2023-ftrace_support-v3-1-f358d2911...@quicinc.com
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 24 +++--
drivers/bus/mhi/host/pm.c | 6 +-
drivers/bus/mhi/host/trace.h| 225
5 files changed, 246 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a02a71605907 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include "trace.h"
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..923d51904f35 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -491,11 +491,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_process_ctrl_ev_ring(mhi_cntrl->mhi_dev->name,
local_rp,
+ local_rp->ptr,
local_rp->dword[0],
+ local_rp->dword[1]);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +999,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_process_data_event_ring(mhi_cntrl->mhi_dev->name,
local_rp->ptr,
+ local_rp->dword[0],
local_rp->dword[1]);
+
chan = MHI_TRE_GET_EV_CHID(local_rp);
WARN_ON(chan >= mhi_cntrl->max_chan);
@@ -1235,6 +1240,8 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct
mhi_chan *mhi_chan,
mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(info->len);
mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(bei, eot, eob, chain);
+ trace_mhi_gen_tre(mhi_cntrl->mhi_
[PATCH v3] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_mhi_states
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Where ever we added trace events removing debug messages.
Signed-off-by: Krishna chaitanya chundru
---
Changes in v3:
- move trace header file from include/trace/events to drivers/bus/mhi/host/ so
that
- we can include driver header files.
- Use macros directly in the trace events as suggested Jeffrey Hugo.
- Reorder the structure in the events as suggested by steve to avoid holes in
the buffer.
- removed the mhi_to_physical function as this can give security issues.
- removed macros to define strings as we can get those from driver headers.
- Link to v2:
https://lore.kernel.org/r/20231013-ftrace_support-v2-1-6e893ce01...@quicinc.com
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 24 +++--
drivers/bus/mhi/host/pm.c | 6 +-
drivers/bus/mhi/host/trace.h| 224
5 files changed, 245 insertions(+), 13 deletions(-)
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..6acb85f4c5f8 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a02a71605907 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include "trace.h"
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..7fe8b2dd9e24 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -491,11 +491,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_states(mhi_cntrl->mhi_dev->name, mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +830,10 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_process_ctrl_ev_ring(mhi_cntrl->mhi_dev->name,
(u64)local_rp,
+ local_rp->ptr,
local_rp->dword[0],
+ local_rp->dword[1]);
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +999,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_process_data_event_ring(mhi_cntrl->mhi_dev->name,
local_rp->ptr,
+ local_rp->dword[0],
local_rp->dword[1]);
+
chan = MHI_TRE_GET_EV_CHID(local_rp);
WARN_ON(chan >= mhi_cntrl->max_chan);
@@ -1235,6 +1240,8 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct
mhi_chan *mhi_chan,
mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(info->len);
mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(bei, eot, eob, chain);
+ trace_mhi_gen_tre(mhi_cntrl->mhi_dev->name, mhi_chan->chan,
(u64)mhi_tre,
+ mhi_tre->ptr, mhi_tre->dword[0], mhi_tre->dword[1]);
/* increment WP */
mhi_add_ring_element(mhi_cntrl, tre_ring);
mhi_add_ring_element(mhi_cntrl, buf_ring);
@@ -1327,9 +1334,8 @@ sta
Re: [PATCH v2] bus: mhi: host: Add tracing support
On 10/27/2023 8:59 PM, Jeffrey Hugo wrote:
On 10/23/2023 1:11 AM, Krishna Chaitanya Chundru wrote:
On 10/20/2023 8:33 PM, Jeffrey Hugo wrote:
On 10/13/2023 3:52 AM, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_threaded_handler
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Signed-off-by: Krishna chaitanya chundru
---
Changes in v2:
- Passing the raw state into the trace event and using
__print_symbolic() as suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by
bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
MAINTAINERS | 1 +
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 32 +++--
drivers/bus/mhi/host/pm.c | 6 +-
include/trace/events/mhi_host.h | 287
6 files changed, 317 insertions(+), 13 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 35977b269d5e..4339c668a6ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13862,6 +13862,7 @@ F: Documentation/mhi/
F: drivers/bus/mhi/
F: drivers/pci/endpoint/functions/pci-epf-mhi.c
F: include/linux/mhi.h
+F: include/trace/events/mhi_host.h
MICROBLAZE ARCHITECTURE
M: Michal Simek
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..3afa90a204fd 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include
This feels redundant to me. A few lines ago we included internal.h,
and internal.h includes trace/events/mhi_host.h
As Steve mentioned, this is mandatory step for creating trace points
& trace events.
I understand this creates the trace points, and that needs to be done
in C code. It dtill seems redundant because we are including the
header twice (and I am aware trace has the special multi-header read
functionality for this).
The duplicate include still feels weird, but I have not come up with a
better way to structure this.
We will use this way for now then, we will check in parallel if there
is a way to avoid this and change it in the future.
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h
b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a80a317a59a9 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..fcdb728ba49f 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -246,6 +246,11 @@ static void *mhi_to_virtual(struct mhi_ring
*ring, dma_addr_t addr)
return (addr - ring->iommu_base) + ring->base;
}
+dma_addr_t mhi_to_physical(struct mhi_ring *ring, void *addr)
+{
+ return (addr - ring->base) + ring->iommu_base;
+}
This seems to be poorly named since we are using the iommu_base
which suggests we are converting to an IOVA.
Why do we need this though? This seems like it might be a security
issue, or at the very least, not preferred, and I'm struggling to
figure out what value this provides to you are I when looking at the
log.
I will rename the function to reflect it is converting to IOVA.
We MHI TRE we write the IOVA address, to correlate between TRE events
in the MHI ring and event we are processing we want to log the IOVA
address.
As we are logging only IOVA address which is provided in the
devicetree and not the original physical address we are not expecting
any security issues here.
Correct me if I was wrong.
The IOVA is not provided by DT, it is a runtime allocated value
provided by the IOMMU, if present. If not present, then it is a
physical address.
Remember, x86 does not use devicetree.
While the IOVA (with an iommu) is not technically a physical address,
but is treated as such by the device. I can imagine an attacker doing
bad things if they get a hold of the value.
Sure we will remove it.
Still, you haven't indicated why this is useful.
The TRE ring elements has address in the IOVA format when we want to
correlate the address with the TRE elements in the dumps it will easier
with this way.
Anyway we will not expose this as you suggested as it might expose
physical address in some platforms.
+
Re: [PATCH v2] bus: mhi: host: Add tracing support
On 10/20/2023 8:33 PM, Jeffrey Hugo wrote:
On 10/13/2023 3:52 AM, Krishna chaitanya chundru wrote:
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_threaded_handler
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Signed-off-by: Krishna chaitanya chundru
---
Changes in v2:
- Passing the raw state into the trace event and using
__print_symbolic() as suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
MAINTAINERS | 1 +
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 32 +++--
drivers/bus/mhi/host/pm.c | 6 +-
include/trace/events/mhi_host.h | 287
6 files changed, 317 insertions(+), 13 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 35977b269d5e..4339c668a6ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13862,6 +13862,7 @@ F: Documentation/mhi/
F: drivers/bus/mhi/
F: drivers/pci/endpoint/functions/pci-epf-mhi.c
F: include/linux/mhi.h
+F: include/trace/events/mhi_host.h
MICROBLAZE ARCHITECTURE
M: Michal Simek
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..3afa90a204fd 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include
This feels redundant to me. A few lines ago we included internal.h,
and internal.h includes trace/events/mhi_host.h
As Steve mentioned, this is mandatory step for creating trace points &
trace events.
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h
b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a80a317a59a9 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..fcdb728ba49f 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -246,6 +246,11 @@ static void *mhi_to_virtual(struct mhi_ring
*ring, dma_addr_t addr)
return (addr - ring->iommu_base) + ring->base;
}
+dma_addr_t mhi_to_physical(struct mhi_ring *ring, void *addr)
+{
+ return (addr - ring->base) + ring->iommu_base;
+}
This seems to be poorly named since we are using the iommu_base which
suggests we are converting to an IOVA.
Why do we need this though? This seems like it might be a security
issue, or at the very least, not preferred, and I'm struggling to
figure out what value this provides to you are I when looking at the log.
I will rename the function to reflect it is converting to IOVA.
We MHI TRE we write the IOVA address, to correlate between TRE events in
the MHI ring and event we are processing we want to log the IOVA address.
As we are logging only IOVA address which is provided in the devicetree
and not the original physical address we are not expecting any security
issues here.
Correct me if I was wrong.
+
static void mhi_add_ring_element(struct mhi_controller *mhi_cntrl,
struct mhi_ring *ring)
{
@@ -491,11 +496,9 @@ irqreturn_t mhi_intvec_threaded_handler(int
irq_number, void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_threaded_handler(mhi_cntrl->mhi_dev->name,
mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
Why are we removing the debug message when adding this trace? The
commit text doesn't say. (Looks like you do this several times,
assume this comment applies to all isntances)
I will add this in the commit text in my next patch.
Just a query is recommended to keep both debug message and trace events.
If yes we will not remove the debug messages.
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +835,12 @@ int mhi_process_ctrl_ev_ring(struct
mhi_controller *mhi_cntrl,
while (dev_rp
Re: [PATCH v2] bus: mhi: host: Add tracing support
On 10/16/2023 8:43 PM, Steven Rostedt wrote:
On Fri, 13 Oct 2023 15:22:19 +0530
Krishna chaitanya chundru wrote:
+++ b/include/trace/events/mhi_host.h
@@ -0,0 +1,287 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM mhi_host
+
+#if !defined(_TRACE_EVENT_MHI_HOST_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_EVENT_MHI_HOST_H
+
+#include
+#define MHI_STATE \
+ EM(MHI_STATE_RESET, "RESET") \
+ EM(MHI_STATE_READY, "READY") \
+ EM(MHI_STATE_M0,"M0") \
+ EM(MHI_STATE_M1,"M1") \
+ EM(MHI_STATE_M2,"M2") \
+ EM(MHI_STATE_M3,"M3") \
+ EM(MHI_STATE_M3_FAST, "M3 FAST")\
+ EM(MHI_STATE_BHI, "BHI")\
+ EMe(MHI_STATE_SYS_ERR, "SYS ERROR")
+
+#define MHI_EE \
+ EM(MHI_EE_PBL, "PRIMARY BOOTLOADER") \
+ EM(MHI_EE_SBL, "SECONDARY BOOTLOADER") \
+ EM(MHI_EE_AMSS, "MISSION MODE") \
+ EM(MHI_EE_RDDM, "RAMDUMP DOWNLOAD MODE") \
+ EM(MHI_EE_WFW, "WLAN FIRMWARE") \
+ EM(MHI_EE_PTHRU,"PASS THROUGH") \
+ EM(MHI_EE_EDL, "EMERGENCY DOWNLOAD") \
+ EM(MHI_EE_FP, "FLASH PROGRAMMER") \
+ EM(MHI_EE_DISABLE_TRANSITION, "DISABLE")\
+ EMe(MHI_EE_NOT_SUPPORTED, "NOT SUPPORTED")
+
+#define MHI_PM_STATE \
+ EM(MHI_PM_STATE_DISABLE,"DISABLE")\
+ EM(MHI_PM_STATE_POR,"POWER ON RESET") \
+ EM(MHI_PM_STATE_M0, "M0") \
+ EM(MHI_PM_STATE_M2, "M2") \
+ EM(MHI_PM_STATE_M3_ENTER, "M?->M3") \
+ EM(MHI_PM_STATE_M3, "M3") \
+ EM(MHI_PM_STATE_M3_EXIT,"M3->M0") \
+ EM(MHI_PM_STATE_FW_DL_ERR, "Firmware Download Error")\
+ EM(MHI_PM_STATE_SYS_ERR_DETECT, "SYS ERROR Detect") \
+ EM(MHI_PM_STATE_SYS_ERR_PROCESS,"SYS ERROR Process") \
+ EM(MHI_PM_STATE_SHUTDOWN_PROCESS, "SHUTDOWN Process") \
+ EMe(MHI_PM_STATE_LD_ERR_FATAL_DETECT, "Linkdown or Error Fatal
Detect")
+
+#define MHI_CH_STATE \
+ EM(MHI_CH_STATE_TYPE_RESET, "RESET") \
+ EM(MHI_CH_STATE_TYPE_STOP, "STOP")\
+ EMe(MHI_CH_STATE_TYPE_START, "START")
+
+#define MHI_DEV_ST_TRANSITION \
+ EM(DEV_ST_TRANSITION_PBL, "PBL")\
+ EM(DEV_ST_TRANSITION_READY, "READY") \
+ EM(DEV_ST_TRANSITION_SBL, "SBL")\
+ EM(DEV_ST_TRANSITION_MISSION_MODE, "MISSION MODE") \
+ EM(DEV_ST_TRANSITION_FP,"FLASH PROGRAMMER") \
+ EM(DEV_ST_TRANSITION_SYS_ERR, "SYS ERROR") \
+ EMe(DEV_ST_TRANSITION_DISABLE, "DISABLE")
+
+/* Enums require being exported to userspace, for user tool parsing */
+#undef EM
+#undef EMe
+#defineEM(a, b)TRACE_DEFINE_ENUM(a);
+#defineEMe(a, b) TRACE_DEFINE_ENUM(a);
+
+MHI_STATE
+MHI_EE
+MHI_PM_STATE
+MHI_CH_STATE
+MHI_DEV_ST_TRANSITION
+
+/*
+ * Now redefine the EM() and EMe() macros to map the enums to the strings
+ * that will be printed in the output.
+ */
+#undef EM
+#undef EMe
+#define EM(a, b) {a, b},
+#define EMe(a, b) {a, b}
+
+TRACE_EVENT(mhi_gen_tre,
+
+ TP_PROTO(const char *name, int ch_num, u64 wp, __le64 tre_ptr,
+__le32 dword0, __le32 dword1),
+
+ TP_ARGS(name, ch_num, wp, tre_ptr, dword0, dword1),
+
+ TP_STRUCT__entry(
+ __string(name, name)
+ __field(int, ch_num)
This is fine as __string() is four bytes in the event (2 bytes for offset
where the string exists, and 2 bytes for its length).
+ __field(u64,
Re: [PATCH] bus: mhi: host: Add tracing support
On 10/6/2023 4:10 AM, Bjorn Andersson wrote:
On Thu, Oct 05, 2023 at 03:55:20PM +0530, Krishna chaitanya chundru wrote:
This change adds ftrace support for following:
1. mhi_intvec_threaded_handler
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
This is not the best "problem description".
Usage:
echo 1 > /sys/kernel/debug/tracing/events/mhi_host/enable
cat /sys/kernel/debug/tracing/trace
This does not need to be included in the commit message, how to use the
tracing framework is documented elsewhere.
Removed this now.
[..]
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..499590437e9b 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -491,11 +491,10 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_threaded_handler(mhi_cntrl->mhi_dev->name, TO_MHI_EXEC_STR(mhi_cntrl->ee),
+ mhi_state_str(mhi_cntrl->dev_state),
+ TO_MHI_EXEC_STR(ee),
mhi_state_str(state));
All these helper functions that translates a state to a string, pass the
raw state into the trace event and use __print_symbolic() in your
TP_printk() instead.
This will allow you to read the state, but you can have tools act of the
numerical value.
(This comment applies to all the trace events)
changed the events as you suggested.
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
[..]
diff --git a/include/trace/events/mhi_host.h b/include/trace/events/mhi_host.h
[..]
+
+TRACE_EVENT(mhi_pm_st_worker,
Why is this trace event called "worker", isn't the event a
"mhi_pm_state_transition"?
Don't just name your trace event based on the function that triggers
them, but what they represent and make sure they carry useful
information to understand the system.
If you want to trace the flow through your functions, you can use e.g.
ftrace.
Regards,
Bjorn
Changed as you suggested.
- KC
[PATCH v2] bus: mhi: host: Add tracing support
This change adds ftrace support for following functions which
helps in debugging the issues when there is Channel state & MHI
state change and also when we receive data and control events:
1. mhi_intvec_threaded_handler
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Signed-off-by: Krishna chaitanya chundru
---
Changes in v2:
- Passing the raw state into the trace event and using __print_symbolic() as
suggested by bjorn.
- Change mhi_pm_st_worker to mhi_pm_st_transition as suggested by bjorn.
- Fixed the kernel test rebot issues.
- Link to v1:
https://lore.kernel.org/r/20231005-ftrace_support-v1-1-23a2f394f...@quicinc.com
---
MAINTAINERS | 1 +
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 32 +++--
drivers/bus/mhi/host/pm.c | 6 +-
include/trace/events/mhi_host.h | 287
6 files changed, 317 insertions(+), 13 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 35977b269d5e..4339c668a6ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13862,6 +13862,7 @@ F: Documentation/mhi/
F: drivers/bus/mhi/
F: drivers/pci/endpoint/functions/pci-epf-mhi.c
F: include/linux/mhi.h
+F: include/trace/events/mhi_host.h
MICROBLAZE ARCHITECTURE
M: Michal Simek
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..3afa90a204fd 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a80a317a59a9 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..fcdb728ba49f 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -246,6 +246,11 @@ static void *mhi_to_virtual(struct mhi_ring *ring,
dma_addr_t addr)
return (addr - ring->iommu_base) + ring->base;
}
+dma_addr_t mhi_to_physical(struct mhi_ring *ring, void *addr)
+{
+ return (addr - ring->base) + ring->iommu_base;
+}
+
static void mhi_add_ring_element(struct mhi_controller *mhi_cntrl,
struct mhi_ring *ring)
{
@@ -491,11 +496,9 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_threaded_handler(mhi_cntrl->mhi_dev->name,
mhi_cntrl->ee,
+ mhi_cntrl->dev_state, ee, state);
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +835,12 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_process_ctrl_ev_ring(mhi_cntrl->mhi_dev->name,
+ mhi_to_physical(ev_ring,
local_rp),
+ local_rp->ptr,
local_rp->dword[0],
+ local_rp->dword[1],
+ MHI_TRE_GET_EV_STATE(local_rp));
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +1006,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_process_data_event_ring(mhi_cntrl->mhi_dev->name,
local_rp->ptr,
+ local_rp->dword[0],
local_rp->dword[1]);
+
chan = MHI_TRE_GET_EV_CHID(local_rp);
WARN_ON(chan >= mhi_cntrl->max_chan);
@@ -1235,6 +1247,9 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct
mhi_chan *mhi_chan,
mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(info->len);
mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(bei, eot, eob
[PATCH] bus: mhi: host: Add tracing support
This change adds ftrace support for following:
1. mhi_intvec_threaded_handler
2. mhi_process_data_event_ring
3. mhi_process_ctrl_ev_ring
4. mhi_gen_tre
5. mhi_update_channel_state
6. mhi_tryset_pm_state
7. mhi_pm_st_worker
Usage:
echo 1 > /sys/kernel/debug/tracing/events/mhi_host/enable
cat /sys/kernel/debug/tracing/trace
Signed-off-by: Krishna chaitanya chundru
---
MAINTAINERS | 1 +
drivers/bus/mhi/host/init.c | 3 +
drivers/bus/mhi/host/internal.h | 1 +
drivers/bus/mhi/host/main.c | 27 --
drivers/bus/mhi/host/pm.c | 7 +-
include/trace/events/mhi_host.h | 207
6 files changed, 234 insertions(+), 12 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 35977b269d5e..4339c668a6ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13862,6 +13862,7 @@ F: Documentation/mhi/
F: drivers/bus/mhi/
F: drivers/pci/endpoint/functions/pci-epf-mhi.c
F: include/linux/mhi.h
+F: include/trace/events/mhi_host.h
MICROBLAZE ARCHITECTURE
M: Michal Simek
diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c
index f78aefd2d7a3..3afa90a204fd 100644
--- a/drivers/bus/mhi/host/init.c
+++ b/drivers/bus/mhi/host/init.c
@@ -20,6 +20,9 @@
#include
#include "internal.h"
+#define CREATE_TRACE_POINTS
+#include
+
static DEFINE_IDA(mhi_controller_ida);
const char * const mhi_ee_str[MHI_EE_MAX] = {
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index 2e139e76de4c..a80a317a59a9 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -7,6 +7,7 @@
#ifndef _MHI_INT_H
#define _MHI_INT_H
+#include
#include "../common.h"
extern struct bus_type mhi_bus_type;
diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
index dcf627b36e82..499590437e9b 100644
--- a/drivers/bus/mhi/host/main.c
+++ b/drivers/bus/mhi/host/main.c
@@ -491,11 +491,10 @@ irqreturn_t mhi_intvec_threaded_handler(int irq_number,
void *priv)
state = mhi_get_mhi_state(mhi_cntrl);
ee = mhi_get_exec_env(mhi_cntrl);
- dev_dbg(dev, "local ee: %s state: %s device ee: %s state: %s\n",
- TO_MHI_EXEC_STR(mhi_cntrl->ee),
- mhi_state_str(mhi_cntrl->dev_state),
- TO_MHI_EXEC_STR(ee), mhi_state_str(state));
+ trace_mhi_intvec_threaded_handler(mhi_cntrl->mhi_dev->name,
TO_MHI_EXEC_STR(mhi_cntrl->ee),
+ mhi_state_str(mhi_cntrl->dev_state),
+ TO_MHI_EXEC_STR(ee),
mhi_state_str(state));
if (state == MHI_STATE_SYS_ERR) {
dev_dbg(dev, "System error detected\n");
pm_state = mhi_tryset_pm_state(mhi_cntrl,
@@ -832,6 +831,11 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_process_ctrl_ev_ring(mhi_cntrl->mhi_dev->name,
(u64)(local_rp),
+ local_rp->ptr,
local_rp->dword[0],
+ local_rp->dword[1],
+
mhi_state_str(MHI_TRE_GET_EV_STATE(local_rp)));
+
switch (type) {
case MHI_PKT_TYPE_BW_REQ_EVENT:
{
@@ -997,6 +1001,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
while (dev_rp != local_rp && event_quota > 0) {
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
+ trace_mhi_process_data_event_ring(mhi_cntrl->mhi_dev->name,
local_rp->ptr,
+ local_rp->dword[0],
local_rp->dword[1]);
+
chan = MHI_TRE_GET_EV_CHID(local_rp);
WARN_ON(chan >= mhi_cntrl->max_chan);
@@ -1235,6 +1242,8 @@ int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct
mhi_chan *mhi_chan,
mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(info->len);
mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(bei, eot, eob, chain);
+ trace_mhi_gen_tre(mhi_cntrl->mhi_dev->name, mhi_chan->chan,
(u64)(mhi_tre),
+ mhi_tre->ptr, mhi_tre->dword[0], mhi_tre->dword[1]);
/* increment WP */
mhi_add_ring_element(mhi_cntrl, tre_ring);
mhi_add_ring_element(mhi_cntrl, buf_ring);
@@ -1327,9 +1336,8 @@ static int mhi_update_channel_state(struct mhi_controller
*mhi_cntrl,
enum mhi_cmd_type cmd = MHI_CMD_NOP;
int ret;
- dev_dbg(dev, "%d: Updating channel state to: %s\n", mhi_chan->chan,
- TO_CH_STATE_TYPE_STR(to_state));
-
+ trace_mhi_update_channel_state_start(mhi_cntrl->mhi_dev->name,
mhi_
Module versioning + Missing CRC in symvers + export tracepoints
Hi, I am seeing an issue of no CRC being generated in the Module.symvers for a driver module even when CONFIG_MODVERSIONS Is enabled, this causes modpost warnings about missing versioning. The module in questions only exports tracepoint related symbols (as struct tracepoint is part of the module CRC), I have seen this with other modules also e.g. iwlwifi with CONFIG_MODVERSIONS. Though I am trying on 5.12.-rc2, also, seeing this issue with older kernels with CONFIG_MODVERSIONS enabled e.g. 4.15.0, Below are a couple of snippets to demonstrate the issue. modpost warnings === WARNING: modpost: EXPORT symbol "__tracepoint_iwlwifi_dev_ucode_event" [drivers/net/wireless/intel/iwlwifi//iwlwifi.ko] version generation failed, symbol will not be versioned. WARNING: modpost: EXPORT symbol "iwl_remove_notification" [drivers/net/wireless/intel/iwlwifi//iwlwifi.ko] version generation failed, symbol will not be versioned. Module.symvers (after modpost) == 0x iwl_remove_notification drivers/net/wireless/intel/iwlwifi//iwlwifi EXPORT_SYMBOL_GPL 0x __tracepoint_iwlwifi_dev_ucode_event drivers/net/wireless/intel/iwlwifi//iwlwifi EXPORT_SYMBOL Any ideas?
Re: [PATCH] [v6] wireless: Initial driver submission for pureLiFi STA devices
On Mon, Oct 19, 2020 at 4:01 PM Srinivasan Raju wrote: > > This introduces the pureLiFi LiFi driver for LiFi-X, LiFi-XC > and LiFi-XL USB devices. > > This driver implementation has been based on the zd1211rw driver. > > Driver is based on 802.11 softMAC Architecture and uses > native 802.11 for configuration and management. > > The driver is compiled and tested in ARM, x86 architectures and > compiled in powerpc architecture. > > Reported-by: kernel test robot > Signed-off-by: Srinivasan Raju > --- > MAINTAINERS|5 + > drivers/net/wireless/Kconfig |1 + > drivers/net/wireless/Makefile |1 + > drivers/net/wireless/purelifi/Kconfig | 27 + > drivers/net/wireless/purelifi/Makefile |3 + > drivers/net/wireless/purelifi/chip.c | 94 ++ > drivers/net/wireless/purelifi/chip.h | 82 ++ > drivers/net/wireless/purelifi/intf.h | 38 + > drivers/net/wireless/purelifi/mac.c| 860 + > drivers/net/wireless/purelifi/mac.h| 180 +++ > drivers/net/wireless/purelifi/usb.c| 1627 > drivers/net/wireless/purelifi/usb.h| 148 +++ > 12 files changed, 3066 insertions(+) > create mode 100644 drivers/net/wireless/purelifi/Kconfig > create mode 100644 drivers/net/wireless/purelifi/Makefile > create mode 100644 drivers/net/wireless/purelifi/chip.c > create mode 100644 drivers/net/wireless/purelifi/chip.h > create mode 100644 drivers/net/wireless/purelifi/intf.h > create mode 100644 drivers/net/wireless/purelifi/mac.c > create mode 100644 drivers/net/wireless/purelifi/mac.h > create mode 100644 drivers/net/wireless/purelifi/usb.c > create mode 100644 drivers/net/wireless/purelifi/usb.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index c80f87d7258c..150f592fb6e4 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14108,6 +14108,11 @@ T: git git://linuxtv.org/media_tree.git > F: Documentation/admin-guide/media/pulse8-cec.rst > F: drivers/media/cec/usb/pulse8/ > > +PUREILIFI USB DRIVER > +M: Srinivasan Raju > +S: Maintained > +F: drivers/net/wireless/purelifi > + > PVRUSB2 VIDEO4LINUX DRIVER > M: Mike Isely > L: pvru...@isely.net (subscribers-only) > diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig > index 170a64e67709..b87da3139f94 100644 > --- a/drivers/net/wireless/Kconfig > +++ b/drivers/net/wireless/Kconfig > @@ -48,6 +48,7 @@ source "drivers/net/wireless/st/Kconfig" > source "drivers/net/wireless/ti/Kconfig" > source "drivers/net/wireless/zydas/Kconfig" > source "drivers/net/wireless/quantenna/Kconfig" > +source "drivers/net/wireless/purelifi/Kconfig" > > config PCMCIA_RAYCS > tristate "Aviator/Raytheon 2.4GHz wireless support" > diff --git a/drivers/net/wireless/Makefile b/drivers/net/wireless/Makefile > index 80b324499786..e9fc770026f0 100644 > --- a/drivers/net/wireless/Makefile > +++ b/drivers/net/wireless/Makefile > @@ -20,6 +20,7 @@ obj-$(CONFIG_WLAN_VENDOR_ST) += st/ > obj-$(CONFIG_WLAN_VENDOR_TI) += ti/ > obj-$(CONFIG_WLAN_VENDOR_ZYDAS) += zydas/ > obj-$(CONFIG_WLAN_VENDOR_QUANTENNA) += quantenna/ > +obj-$(CONFIG_WLAN_VENDOR_PURELIFI) += purelifi/ > > # 16-bit wireless PCMCIA client drivers > obj-$(CONFIG_PCMCIA_RAYCS) += ray_cs.o > diff --git a/drivers/net/wireless/purelifi/Kconfig > b/drivers/net/wireless/purelifi/Kconfig > new file mode 100644 > index ..f6630791df9d > --- /dev/null > +++ b/drivers/net/wireless/purelifi/Kconfig > @@ -0,0 +1,27 @@ > +# SPDX-License-Identifier: GPL-2.0 > +config WLAN_VENDOR_PURELIFI > + bool "pureLiFi devices" > + default y > + help > + If you have a pureLiFi device, say Y. > + > + Note that the answer to this question doesn't directly affect the > + kernel: saying N will just cause the configurator to skip all the > + questions about these cards. If you say Y, you will be asked for > + your specific card in the following questions. > + > +if WLAN_VENDOR_PURELIFI > + > +config PURELIFI > + > + tristate "pureLiFi device support" > + depends on CFG80211 && MAC80211 && USB > + help > + This driver makes the adapter appear as a normal WLAN interface. > + > + The pureLiFi device requires external STA firmware to be loaded. > + > + To compile this driver as a module, choose M here: the module will > + be called purelifi. > + > +endif # WLAN_VENDOR_PURELIFI > diff --git a/drivers/net/wireless/purelifi/Makefile > b/drivers/net/wireless/purelifi/Makefile > new file mode 100644 > index ..1f20055e741f > --- /dev/null > +++ b/drivers/net/wireless/purelifi/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-$(CONFIG_PURELIFI) := purelifi.o > +purelifi-objs += chip.o usb.o mac.o > diff --git a/drivers/net/wireless/purelifi/chip.c > b/drivers/net/wireless/purelifi/chip.c > new file mode 100644 >
Re: [PATCH] wireless: fixup genregdb.awk for remove of antenna gain from wireless-regd
On Tue, Jul 15, 2014 at 2:49 AM, Luis R. Rodriguez
wrote:
> From: "Luis R. Rodriguez"
>
> Since "wireless-regdb: remove antenna gain" was merged in the
> wireless-regdb tree, the awk script parser has been incompatible
> with the 'official' regulatory database. This fixes that up.
> Without this change the max EIRP is set to 0 making 802.11 devices
> useless.
>
> The fragile nature of the awk parser must be replaced, but ideas
> over how to do that in the most scalable way are being reviewed.
> In the meantime update the documentation for CFG80211_INTERNAL_REGDB
> so folks are aware of expectations for now.
>
> Reported-by: John Walker
> Reported-by: Krishna Chaitanya
> Signed-off-by: Luis R. Rodriguez
> ---
>
> !!! Note !!!
>
> This means older kernels that upgrade wireless-regdb and use
> CFG80211_INTERNAL_REGDB are bust too and they should then merge
> the latest updates to the awk script if they want to synch the
> wireless-regdb files with the kernel builds. The affected Linux
> distributions would be the users of CFG80211_INTERNAL_REGDB which
> should consists of OpenWrt which is reported to have this fixed
> already and mobile platforms.
>
> We are looking at a way to not have to deal with parsers all together
> at build time, for that discussion see:
>
> https://lkml.org/lkml/2014/7/14/706
>
> net/wireless/Kconfig | 6 ++
> net/wireless/genregdb.awk | 35 ++-
> 2 files changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
> index 405f3c4..29c8675 100644
> --- a/net/wireless/Kconfig
> +++ b/net/wireless/Kconfig
> @@ -162,6 +162,12 @@ config CFG80211_INTERNAL_REGDB
> and includes code to query that database. This is an alternative
> to using CRDA for defining regulatory rules for the kernel.
>
> + Using this option requires some parsing of the db.txt at build time,
> + the parser will be upkept with the latest wireless-regdb updates but
> + older wireless-regdb formats will be ignored. The parser may later
> + be replaced to avoid issues with conflicts on versions of
> + wireless-regdb.
> +
> For details see:
>
> http://wireless.kernel.org/en/developers/Regulatory
> diff --git a/net/wireless/genregdb.awk b/net/wireless/genregdb.awk
> index 40c37fc..baf2426 100644
> --- a/net/wireless/genregdb.awk
> +++ b/net/wireless/genregdb.awk
> @@ -51,32 +51,41 @@ function parse_country_head() {
>
> function parse_reg_rule()
> {
> + flag_starts_at = 7
> +
> start = $1
> sub(/\(/, "", start)
> end = $3
> bw = $5
> sub(/\),/, "", bw)
> - gain = $6
> - sub(/\(/, "", gain)
> - sub(/,/, "", gain)
> - power = $7
> - sub(/\)/, "", power)
> - sub(/,/, "", power)
> + gain = 0
> + power = $6
> # power might be in mW...
> - units = $8
> + units = $7
> + dfs_cac = 0
> +
> + sub(/\(/, "", power)
> + sub(/\),/, "", power)
> + sub(/\),/, "", units)
> sub(/\)/, "", units)
> - sub(/,/, "", units)
> - dfs_cac = $9
> +
> if (units == "mW") {
> + flag_starts_at = 8
> power = 10 * log(power)/log(10)
> + if ($8 ~ /[[:digit:]]/) {
> + flag_starts_at = 9
> + dfs_cac = $8
> + }
> } else {
> - dfs_cac = $8
> + if ($7 ~ /[[:digit:]]/) {
> + flag_starts_at = 8
> + dfs_cac = $7
> + }
> }
> - sub(/,/, "", dfs_cac)
> sub(/\(/, "", dfs_cac)
> - sub(/\)/, "", dfs_cac)
> + sub(/\),/, "", dfs_cac)
> flagstr = ""
> - for (i=8; i<=NF; i++)
> + for (i=flag_starts_at; i<=NF; i++)
> flagstr = flagstr $i
> split(flagstr, flagarray, ",")
> flags = ""
Acked-by:Krishna Chaitanya
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] wireless: fixup genregdb.awk for remove of antenna gain from wireless-regd
On Tue, Jul 15, 2014 at 2:49 AM, Luis R. Rodriguez
mcg...@do-not-panic.com wrote:
From: Luis R. Rodriguez mcg...@suse.com
Since wireless-regdb: remove antenna gain was merged in the
wireless-regdb tree, the awk script parser has been incompatible
with the 'official' regulatory database. This fixes that up.
Without this change the max EIRP is set to 0 making 802.11 devices
useless.
The fragile nature of the awk parser must be replaced, but ideas
over how to do that in the most scalable way are being reviewed.
In the meantime update the documentation for CFG80211_INTERNAL_REGDB
so folks are aware of expectations for now.
Reported-by: John Walker j...@x109.net
Reported-by: Krishna Chaitanya chaitanya.m...@gmail.com
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
!!! Note !!!
This means older kernels that upgrade wireless-regdb and use
CFG80211_INTERNAL_REGDB are bust too and they should then merge
the latest updates to the awk script if they want to synch the
wireless-regdb files with the kernel builds. The affected Linux
distributions would be the users of CFG80211_INTERNAL_REGDB which
should consists of OpenWrt which is reported to have this fixed
already and mobile platforms.
We are looking at a way to not have to deal with parsers all together
at build time, for that discussion see:
https://lkml.org/lkml/2014/7/14/706
net/wireless/Kconfig | 6 ++
net/wireless/genregdb.awk | 35 ++-
2 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index 405f3c4..29c8675 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -162,6 +162,12 @@ config CFG80211_INTERNAL_REGDB
and includes code to query that database. This is an alternative
to using CRDA for defining regulatory rules for the kernel.
+ Using this option requires some parsing of the db.txt at build time,
+ the parser will be upkept with the latest wireless-regdb updates but
+ older wireless-regdb formats will be ignored. The parser may later
+ be replaced to avoid issues with conflicts on versions of
+ wireless-regdb.
+
For details see:
http://wireless.kernel.org/en/developers/Regulatory
diff --git a/net/wireless/genregdb.awk b/net/wireless/genregdb.awk
index 40c37fc..baf2426 100644
--- a/net/wireless/genregdb.awk
+++ b/net/wireless/genregdb.awk
@@ -51,32 +51,41 @@ function parse_country_head() {
function parse_reg_rule()
{
+ flag_starts_at = 7
+
start = $1
sub(/\(/, , start)
end = $3
bw = $5
sub(/\),/, , bw)
- gain = $6
- sub(/\(/, , gain)
- sub(/,/, , gain)
- power = $7
- sub(/\)/, , power)
- sub(/,/, , power)
+ gain = 0
+ power = $6
# power might be in mW...
- units = $8
+ units = $7
+ dfs_cac = 0
+
+ sub(/\(/, , power)
+ sub(/\),/, , power)
+ sub(/\),/, , units)
sub(/\)/, , units)
- sub(/,/, , units)
- dfs_cac = $9
+
if (units == mW) {
+ flag_starts_at = 8
power = 10 * log(power)/log(10)
+ if ($8 ~ /[[:digit:]]/) {
+ flag_starts_at = 9
+ dfs_cac = $8
+ }
} else {
- dfs_cac = $8
+ if ($7 ~ /[[:digit:]]/) {
+ flag_starts_at = 8
+ dfs_cac = $7
+ }
}
- sub(/,/, , dfs_cac)
sub(/\(/, , dfs_cac)
- sub(/\)/, , dfs_cac)
+ sub(/\),/, , dfs_cac)
flagstr =
- for (i=8; i=NF; i++)
+ for (i=flag_starts_at; i=NF; i++)
flagstr = flagstr $i
split(flagstr, flagarray, ,)
flags =
Acked-by:Krishna Chaitanya chaitanya.m...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [mac80211_hwsim] BUG: unable to handle kernel paging request at ce1db404
On Sat, Mar 15, 2014 at 9:11 PM, Johannes Berg wrote: > On Sat, 2014-03-15 at 21:03 +0530, Krishna Chaitanya wrote: > >> > > what RC are u using? Default should be minstrel, i dont see >> > > a reason for rc alloc to fail (remote reason kmalloc failure), >> > > so did you disable RC completely? No prints either w.r.t RC either in >> > > dmesg? >> > >> > Pay attention to the .config. >> > >> Missed the attachment, thanks for pointing. >> As guessed the rate control is empty causing the registration fail. > > It still shouldn't crash though. Looks like there's a fix in this > thread, can somebody verify & post it? > Yes, it should not crash. The change suggested by martin is not correct there is no double free as the the list he mentioned will be empty. (Only after successful registration we will add the radio to the list) the problem here is platform_driver_unregister internally calls the driver_unregister which tries to get the kobject through get_device, but we have already freed the kobject using device_unregister (which calls device_del which frees the kobject). In other failures cases we use mac80211_hwsim_free() and return, so the call to platform_driver_unregister is not there, hence no crash. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mac80211_hwsim] BUG: unable to handle kernel paging request at ce1db404
On Sat, Mar 15, 2014 at 8:50 PM, Johannes Berg wrote: > > On Thu, 2014-03-13 at 02:15 +0530, Krishna Chaitanya wrote: > > > From the logs it looks like "rate_control_alloc" is failed, > > causing ieee80211_register_hw to fail triggering the crash. > > Yes. > > > what RC are u using? Default should be minstrel, i dont see > > a reason for rc alloc to fail (remote reason kmalloc failure), > > so did you disable RC completely? No prints either w.r.t RC either in > > dmesg? > > Pay attention to the .config. > Missed the attachment, thanks for pointing. As guessed the rate control is empty causing the registration fail. CONFIG_MAC80211=y # CONFIG_MAC80211_RC_PID is not set # CONFIG_MAC80211_RC_MINSTREL is not set CONFIG_MAC80211_RC_DEFAULT="" -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mac80211_hwsim] BUG: unable to handle kernel paging request at ce1db404
On Sat, Mar 15, 2014 at 8:50 PM, Johannes Berg johan...@sipsolutions.net wrote: On Thu, 2014-03-13 at 02:15 +0530, Krishna Chaitanya wrote: From the logs it looks like rate_control_alloc is failed, causing ieee80211_register_hw to fail triggering the crash. Yes. what RC are u using? Default should be minstrel, i dont see a reason for rc alloc to fail (remote reason kmalloc failure), so did you disable RC completely? No prints either w.r.t RC either in dmesg? Pay attention to the .config. Missed the attachment, thanks for pointing. As guessed the rate control is empty causing the registration fail. CONFIG_MAC80211=y # CONFIG_MAC80211_RC_PID is not set # CONFIG_MAC80211_RC_MINSTREL is not set CONFIG_MAC80211_RC_DEFAULT= -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mac80211_hwsim] BUG: unable to handle kernel paging request at ce1db404
On Sat, Mar 15, 2014 at 9:11 PM, Johannes Berg johan...@sipsolutions.net wrote: On Sat, 2014-03-15 at 21:03 +0530, Krishna Chaitanya wrote: what RC are u using? Default should be minstrel, i dont see a reason for rc alloc to fail (remote reason kmalloc failure), so did you disable RC completely? No prints either w.r.t RC either in dmesg? Pay attention to the .config. Missed the attachment, thanks for pointing. As guessed the rate control is empty causing the registration fail. It still shouldn't crash though. Looks like there's a fix in this thread, can somebody verify post it? Yes, it should not crash. The change suggested by martin is not correct there is no double free as the the list he mentioned will be empty. (Only after successful registration we will add the radio to the list) the problem here is platform_driver_unregister internally calls the driver_unregister which tries to get the kobject through get_device, but we have already freed the kobject using device_unregister (which calls device_del which frees the kobject). In other failures cases we use mac80211_hwsim_free() and return, so the call to platform_driver_unregister is not there, hence no crash. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mac80211_hwsim] BUG: unable to handle kernel paging request at ce1db404
On Wed, Mar 12, 2014 at 11:04 PM, Martin Pitt wrote: > > Fengguang Wu [2014-03-08 20:11 +0800]: > > [4.429993] mac80211_hwsim: ieee80211_register_hw failed (-2) > > [...] > > [4.431924] [] get_device+0xf/0x17 > > [4.431924] [] driver_detach+0x38/0x8f > > [4.431924] [] bus_remove_driver+0x53/0x66 > > [4.431924] [] driver_unregister+0x38/0x3d > > [4.431924] [] platform_driver_unregister+0xb/0xd > > [4.431924] [] init_mac80211_hwsim+0x3a5/0x3b6 > > > So that first message is from > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/mac80211_hwsim.c?id=9ea927748#n2428 > > At this point we registered the platform driver and the class, and it > should have created two devices (at least for the default radios=2). > What's odd is that I don't see this printk in your kernel log: > > mac80211_hwsim: Initializing radio %d > > If for some reasons "radio" is 0, it would not show this and not > initialize data->dev, but then you shouldn't get to > ieee80211_register_hw() either as it's in the same loop. So that's a > bit of a mystery to me. > > On failure, above ieee80211_register_hw() jumps to the cleanup: > > | failed_hw: > | device_unregister(data->dev); > | failed_drvdata: > | ieee80211_free_hw(hw); > | failed: > | mac80211_hwsim_free(); > | failed_unregister_driver: > | driver_unregister(_hwsim_driver); > | return err; > | } > > > The mac80211_hwsim_free() function again calls > device_unregister(data->dev) for a list (not sure which, I'm not > certain how to interpret > > list_for_each_entry_safe(data, tmpdata, , list) > > ) Could that be the double free causing the memory corruption? > > If you are in a position to do quick builds and tests, does the crash > go away with this? > > printk(KERN_DEBUG "mac80211_hwsim: device_bind_driver failed > (%d)\n", >err); > - goto failed_hw; > + goto failed_drvdata; > } > > (I'm not claiming that this is correct, just taking a stab at > understanding what happens) If not, does it go away with changing the > goto to failed_unregister_driver()? > >From the logs it looks like "rate_control_alloc" is failed, causing ieee80211_register_hw to fail triggering the crash. what RC are u using? Default should be minstrel, i dont see a reason for rc alloc to fail (remote reason kmalloc failure), so did you disable RC completely? No prints either w.r.t RC either in dmesg? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [mac80211_hwsim] BUG: unable to handle kernel paging request at ce1db404
On Wed, Mar 12, 2014 at 11:04 PM, Martin Pitt martin.p...@ubuntu.com wrote: Fengguang Wu [2014-03-08 20:11 +0800]: [4.429993] mac80211_hwsim: ieee80211_register_hw failed (-2) [...] [4.431924] [c12377de] get_device+0xf/0x17 [4.431924] [c123a165] driver_detach+0x38/0x8f [4.431924] [c1239433] bus_remove_driver+0x53/0x66 [4.431924] [c123a535] driver_unregister+0x38/0x3d [4.431924] [c123b3aa] platform_driver_unregister+0xb/0xd [4.431924] [c1c4ac9f] init_mac80211_hwsim+0x3a5/0x3b6 So that first message is from http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/mac80211_hwsim.c?id=9ea927748#n2428 At this point we registered the platform driver and the class, and it should have created two devices (at least for the default radios=2). What's odd is that I don't see this printk in your kernel log: mac80211_hwsim: Initializing radio %d If for some reasons radio is 0, it would not show this and not initialize data-dev, but then you shouldn't get to ieee80211_register_hw() either as it's in the same loop. So that's a bit of a mystery to me. On failure, above ieee80211_register_hw() jumps to the cleanup: | failed_hw: | device_unregister(data-dev); | failed_drvdata: | ieee80211_free_hw(hw); | failed: | mac80211_hwsim_free(); | failed_unregister_driver: | driver_unregister(mac80211_hwsim_driver); | return err; | } The mac80211_hwsim_free() function again calls device_unregister(data-dev) for a list (not sure which, I'm not certain how to interpret list_for_each_entry_safe(data, tmpdata, tmplist, list) ) Could that be the double free causing the memory corruption? If you are in a position to do quick builds and tests, does the crash go away with this? printk(KERN_DEBUG mac80211_hwsim: device_bind_driver failed (%d)\n, err); - goto failed_hw; + goto failed_drvdata; } (I'm not claiming that this is correct, just taking a stab at understanding what happens) If not, does it go away with changing the goto to failed_unregister_driver()? From the logs it looks like rate_control_alloc is failed, causing ieee80211_register_hw to fail triggering the crash. what RC are u using? Default should be minstrel, i dont see a reason for rc alloc to fail (remote reason kmalloc failure), so did you disable RC completely? No prints either w.r.t RC either in dmesg? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] p54usb: fix leaks at failure path in p54u_probe()
On Sun, Mar 9, 2014 at 4:38 AM, Christian Lamparter
wrote:
> On Sunday, March 09, 2014 04:14:32 AM Krishna Chaitanya wrote:
>> On Sat, Mar 8, 2014 at 2:41 AM, Alexey Khoroshilov
>> wrote:
>> > If p54u_load_firmware() fails, p54u_probe() does not deallocate
>> > already allocated resources. The patch adds proper failure handling.
>> >
>> > Found by Linux Driver Verification project (linuxtesting.org).
>> >
>> > Signed-off-by: Alexey Khoroshilov
>> > ---
>> > drivers/net/wireless/p54/p54usb.c | 4
>> > 1 file changed, 4 insertions(+)
>> >
>> > diff --git a/drivers/net/wireless/p54/p54usb.c
>> > b/drivers/net/wireless/p54/p54usb.c
>> > index 6e635cfa24c8..5df74503fd4f 100644
>> > --- a/drivers/net/wireless/p54/p54usb.c
>> > +++ b/drivers/net/wireless/p54/p54usb.c
>> > @@ -1053,6 +1053,10 @@ static int p54u_probe(struct usb_interface *intf,
>> > priv->upload_fw = p54u_upload_firmware_net2280;
>> > }
>> > err = p54u_load_firmware(dev, intf);
>> > + if (err) {
>> > + usb_put_dev(udev);
>> > + p54_free_common(dev);
>> > + }
>> > return err;
>> > }
>> The load_firmware puts down the reference
>> in case of error. Only free is required here.
> No, the put is required too... But let me explain:
>
> p54u_load_firmware calls usb_get_dev(udev) before it requests the firmware
> load. The Reason is: the firmware callback is usually run in another thread
> (usually it's pretty quick, but due to timeouts it could take up to 60 seconds
> - or at least it did when I wrote it). Therefore I found it appropriate to
> give
> that request callback its "reference++" as it needs the "udev" too (e.g.: for
> dev_info, dev_err and releasing the driver if the device couldn't be
> initialized).
>
Thanks Christian and Alexey, you answered my next question as well :-).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] p54usb: fix leaks at failure path in p54u_probe()
On Sat, Mar 8, 2014 at 2:41 AM, Alexey Khoroshilov
wrote:
> If p54u_load_firmware() fails, p54u_probe() does not deallocate
> already allocated resources. The patch adds proper failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov
> ---
> drivers/net/wireless/p54/p54usb.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/p54/p54usb.c
> b/drivers/net/wireless/p54/p54usb.c
> index 6e635cfa24c8..5df74503fd4f 100644
> --- a/drivers/net/wireless/p54/p54usb.c
> +++ b/drivers/net/wireless/p54/p54usb.c
> @@ -1053,6 +1053,10 @@ static int p54u_probe(struct usb_interface *intf,
> priv->upload_fw = p54u_upload_firmware_net2280;
> }
> err = p54u_load_firmware(dev, intf);
> + if (err) {
> + usb_put_dev(udev);
> + p54_free_common(dev);
> + }
> return err;
> }
The load_firmware puts down the reference
in case of error. Only free is required here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] p54usb: fix leaks at failure path in p54u_probe()
On Sat, Mar 8, 2014 at 2:41 AM, Alexey Khoroshilov
khoroshi...@ispras.ru wrote:
If p54u_load_firmware() fails, p54u_probe() does not deallocate
already allocated resources. The patch adds proper failure handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov khoroshi...@ispras.ru
---
drivers/net/wireless/p54/p54usb.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/p54/p54usb.c
b/drivers/net/wireless/p54/p54usb.c
index 6e635cfa24c8..5df74503fd4f 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -1053,6 +1053,10 @@ static int p54u_probe(struct usb_interface *intf,
priv-upload_fw = p54u_upload_firmware_net2280;
}
err = p54u_load_firmware(dev, intf);
+ if (err) {
+ usb_put_dev(udev);
+ p54_free_common(dev);
+ }
return err;
}
The load_firmware puts down the reference
in case of error. Only free is required here.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] p54usb: fix leaks at failure path in p54u_probe()
On Sun, Mar 9, 2014 at 4:38 AM, Christian Lamparter
chunk...@googlemail.com wrote:
On Sunday, March 09, 2014 04:14:32 AM Krishna Chaitanya wrote:
On Sat, Mar 8, 2014 at 2:41 AM, Alexey Khoroshilov
khoroshi...@ispras.ru wrote:
If p54u_load_firmware() fails, p54u_probe() does not deallocate
already allocated resources. The patch adds proper failure handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov khoroshi...@ispras.ru
---
drivers/net/wireless/p54/p54usb.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/p54/p54usb.c
b/drivers/net/wireless/p54/p54usb.c
index 6e635cfa24c8..5df74503fd4f 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -1053,6 +1053,10 @@ static int p54u_probe(struct usb_interface *intf,
priv-upload_fw = p54u_upload_firmware_net2280;
}
err = p54u_load_firmware(dev, intf);
+ if (err) {
+ usb_put_dev(udev);
+ p54_free_common(dev);
+ }
return err;
}
The load_firmware puts down the reference
in case of error. Only free is required here.
No, the put is required too... But let me explain:
p54u_load_firmware calls usb_get_dev(udev) before it requests the firmware
load. The Reason is: the firmware callback is usually run in another thread
(usually it's pretty quick, but due to timeouts it could take up to 60 seconds
- or at least it did when I wrote it). Therefore I found it appropriate to
give
that request callback its reference++ as it needs the udev too (e.g.: for
dev_info, dev_err and releasing the driver if the device couldn't be
initialized).
Thanks Christian and Alexey, you answered my next question as well :-).
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Query] Using DMA API's with struct dev created from device_create
Hi, In cases where the BUS interface is system fabric itself, where no probing is required, so kernel doesn't provide the "dev" pointer (unlike USB and PCI). The driver is a wireless driver based on mac80211. So we used device_create to device, which is passed to set_wiphy_dev, it works fine. But when we pass the same device to dma_map_single it is causing a system panic and reboot (no call stack), instead if i pass NULL it works just fine. How do we deal with "struct dev" in such cases? -- Thanks, Regards, Chaitanya T K. P.S. Please CC me to the replies. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[Query] Using DMA API's with struct dev created from device_create
Hi, In cases where the BUS interface is system fabric itself, where no probing is required, so kernel doesn't provide the dev pointer (unlike USB and PCI). The driver is a wireless driver based on mac80211. So we used device_create to device, which is passed to set_wiphy_dev, it works fine. But when we pass the same device to dma_map_single it is causing a system panic and reboot (no call stack), instead if i pass NULL it works just fine. How do we deal with struct dev in such cases? -- Thanks, Regards, Chaitanya T K. P.S. Please CC me to the replies. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] mac80211:Resetting connection monitor timers in transmit path
On Wed, Oct 23, 2013 at 3:42 PM, Dhahira Thesneem wrote: > > Reset connection monitor timers when we are able to successfully transmit > data to an AP. > > Signed-off-by: Dhahira Thesneem > --- > net/mac80211/tx.c |6 -- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c > index 3456c04..e7725cf 100644 > --- a/net/mac80211/tx.c > +++ b/net/mac80211/tx.c > @@ -1700,7 +1700,8 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff > *skb, > > ieee80211_xmit(sdata, skb, chan->band); > rcu_read_unlock(); > - > + /*To reset connection monitor timers, after successful transmission*/ > + ieee80211_sta_reset_conn_monitor(sdata); > return NETDEV_TX_OK; > > fail_rcu: > @@ -2139,7 +2140,8 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff > *skb, > > ieee80211_xmit(sdata, skb, band); > rcu_read_unlock(); > - > + /*To reset connection monitor timers, after successful transmission*/ > + ieee80211_sta_reset_conn_monitor(sdata); > return NETDEV_TX_OK; > > fail_rcu: > -- Successful data transmission should be checked in the tx_status not after we transmit. In fact its already taken care in status.c: through ieee80211_sta_tx_notify. NACK. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] mac80211:Resetting connection monitor timers in transmit path
On Wed, Oct 23, 2013 at 3:42 PM, Dhahira Thesneem dhahira.thesn...@mistralsolutions.com wrote: Reset connection monitor timers when we are able to successfully transmit data to an AP. Signed-off-by: Dhahira Thesneem dhahira.thesn...@mistralsolutions.com --- net/mac80211/tx.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 3456c04..e7725cf 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -1700,7 +1700,8 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff *skb, ieee80211_xmit(sdata, skb, chan-band); rcu_read_unlock(); - + /*To reset connection monitor timers, after successful transmission*/ + ieee80211_sta_reset_conn_monitor(sdata); return NETDEV_TX_OK; fail_rcu: @@ -2139,7 +2140,8 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb, ieee80211_xmit(sdata, skb, band); rcu_read_unlock(); - + /*To reset connection monitor timers, after successful transmission*/ + ieee80211_sta_reset_conn_monitor(sdata); return NETDEV_TX_OK; fail_rcu: -- Successful data transmission should be checked in the tx_status not after we transmit. In fact its already taken care in status.c: through ieee80211_sta_tx_notify. NACK. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
