Skip to content

drivers: Remove unused unused host_msg field #877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgirdwood merged 1 commit into from
Jan 24, 2019
Merged

drivers: Remove unused unused host_msg field #877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions src/drivers/intel/baytrail/ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ static void irq_handler(void *arg)
{
uint32_t isr;
uint32_t imrd;
uint32_t msg = 0;

/* Interrupt arrived, check src */
isr = shim_read(SHIM_ISRD);
Expand All @@ -115,17 +114,14 @@ static void irq_handler(void *arg)
shim_write(SHIM_IMRD, shim_read(SHIM_IMRD) | SHIM_IMRD_BUSY);
interrupt_clear(PLATFORM_IPC_INTERRUPT);

msg = shim_read(SHIM_IPCXL);

/* TODO: place message in Q and process later */
/* It's not Q ATM, may overwrite */
if (_ipc->host_pending) {
trace_ipc_error("ipc: dropping msg 0x%x", msg);
trace_ipc_error("ipc: dropping msg");
trace_ipc_error(" isr 0x%x imrd 0x%x ipcxh 0x%x",
isr, shim_read(SHIM_IMRD),
shim_read(SHIM_IPCXH));
} else {
_ipc->host_msg = msg;

@ranj063 ranj063 Jan 23, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbaluta if you remove this, there's no need for the msg variable too?

@dbaluta dbaluta Jan 23, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063, true. Unless, we want to keep the debugging message consistent.

`
msg = shim_read(SHIM_IPCXL);

	/* TODO: place message in Q and process later */
	/* It's not Q ATM, may overwrite */
	if (_ipc->host_pending) {
		trace_ipc_error("ipc: dropping msg 0x%x", msg);
		trace_ipc_error(" isr 0x%x imrd 0x%x ipcxh 0x%x",
				isr, shim_read(SHIM_IMRD),
				shim_read(SHIM_IPCXH));

`

But I guess, msg will always be 0.

In my opinion, we should merge the patch as it is now. And then when we really figure out that this is a relict from the past to also remove it.

@ranj063 ranj063 Jan 23, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbaluta yes. I think its redundant. So we should just remove the trace logs and msg altogether.

@dbaluta dbaluta Jan 23, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063, so do you mean something like this.

https://pastebin.com/xsFYtwXv

Sorry for using pastebin but inserting code here just kills me.

So, the best thing to do this would be to attach another patch to this series, where we really explain why in case of _ipc->host_pending == true we just do nothing and also do not print any error message.

@ranj063 ranj063 Jan 23, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbaluta the second trace error message is still useful to indicate if we do drop a message. I was only suggesting to remove the msg part of it because it is not really useful.

@dbaluta dbaluta Jan 23, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 ok, it's clear now. will respin.

_ipc->host_pending = 1;
ipc_schedule_process(_ipc);
}
Expand All @@ -139,8 +135,6 @@ void ipc_platform_do_cmd(struct ipc *ipc)
uint32_t ipcxh;
int32_t err;

trace_ipc("ipc: msg rx -> 0x%x", ipc->host_msg);

/* perform command and return any error */
err = ipc_cmd();
if (err > 0) {
Expand Down
12 changes: 1 addition & 11 deletions src/drivers/intel/cavs/ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ extern struct ipc *_ipc;
static void irq_handler(void *arg)
{
uint32_t dipcctl;
uint32_t msg = 0;

#if defined(CONFIG_APOLLOLAKE)
uint32_t dipct;
Expand Down Expand Up @@ -92,16 +91,10 @@ static void irq_handler(void *arg)
/* mask Busy interrupt */
ipc_write(IPC_DIPCCTL, dipcctl & ~IPC_DIPCCTL_IPCTBIE);

#if defined(CONFIG_APOLLOLAKE)
msg = dipct & IPC_DIPCT_MSG_MASK;
#else
msg = dipctdr & IPC_DIPCTDR_MSG_MASK;
#endif

/* TODO: place message in Q and process later */
/* It's not Q ATM, may overwrite */
if (_ipc->host_pending) {
trace_ipc_error("ipc: dropping msg 0x%x", msg);
trace_ipc_error("ipc: dropping msg");
#if defined(CONFIG_APOLLOLAKE)
trace_ipc_error(" dipct 0x%x dipcie 0x%x dipcctl 0x%x",
dipct, dipcie, ipc_read(IPC_DIPCCTL));
Expand All @@ -111,7 +104,6 @@ static void irq_handler(void *arg)
ipc_read(IPC_DIPCCTL));
#endif
} else {
_ipc->host_msg = msg;
_ipc->host_pending = 1;
ipc_schedule_process(_ipc);
}
Expand Down Expand Up @@ -148,8 +140,6 @@ void ipc_platform_do_cmd(struct ipc *ipc)
struct sof_ipc_reply reply;
int32_t err;

trace_ipc("ipc: msg rx -> 0x%x", ipc->host_msg);

/* perform command and return any error */
err = ipc_cmd();
if (err > 0) {
Expand Down
2 changes: 0 additions & 2 deletions src/drivers/intel/cavs/sue-ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ void ipc_platform_do_cmd(struct ipc *ipc)
struct sof_ipc_reply reply;
int32_t err;

trace_ipc("ipc: msg rx -> 0x%x", ipc->host_msg);

/* perform command and return any error */
err = ipc_cmd();
if (err > 0) {
Expand Down
8 changes: 1 addition & 7 deletions src/drivers/intel/haswell/ipc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ static void do_notify(void)
static void irq_handler(void *arg)
{
uint32_t isr, imrd;
uint32_t msg = 0;

/* Interrupt arrived, check src */
isr = shim_read(SHIM_ISRD);
Expand All @@ -110,17 +109,14 @@ static void irq_handler(void *arg)
shim_write(SHIM_IMRD, shim_read(SHIM_IMRD) | SHIM_IMRD_BUSY);
interrupt_clear(PLATFORM_IPC_INTERRUPT);

msg = shim_read(SHIM_IPCX);

/* TODO: place message in Q and process later */
/* It's not Q ATM, may overwrite */
if (_ipc->host_pending) {
trace_ipc_error("ipc: dropping msg 0x%x", msg);
trace_ipc_error("ipc: dropping msg");
trace_ipc_error(" isr 0x%x imrd 0x%x ipcx 0x%x",
isr, shim_read(SHIM_IMRD),
shim_read(SHIM_IPCX));
} else {
_ipc->host_msg = msg;
_ipc->host_pending = 1;
ipc_schedule_process(_ipc);
}
Expand All @@ -133,8 +129,6 @@ void ipc_platform_do_cmd(struct ipc *ipc)
struct sof_ipc_reply reply;
int32_t err;

trace_ipc("ipc: msg rx -> 0x%x", ipc->host_msg);

/* perform command and return any error */
err = ipc_cmd();
if (err > 0) {
Expand Down
2 changes: 0 additions & 2 deletions src/include/sof/ipc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@ struct ipc_shared_context {
};

struct ipc {
/* messaging */
uint32_t host_msg; /* current message from host */
uint32_t host_pending;
spinlock_t lock;
void *comp_data;
Expand Down