Repository navigation
reduce IPC platform driver API #740
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -167,20 +167,6 @@ static irqreturn_t spi_irq_handler(int irq __maybe_unused, void *context) | |
| return IRQ_NONE; | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment. @lyakh can you do me a favor and not bundle things aren't ready for upstream with things that are. Specifically Haswell/SKL/KBL/Compressed/SPI are not shared upstream just yet, so it helps if your patches follow this split.
Collaborator
Author
There was a problem hiding this comment. sorry, what branch should I be making patches against? I was using topic/sof-dev, should I use integration/... instead?
Collaborator
There was a problem hiding this comment. @lyakh we should use topic/sof-dev but if you look at the patches posted upstream HSW/KBL/SKL/Compressed/SPI are not included in them. So it will be easier to maintain the squash/rebase cycles if you split for ex. your commit: "ASoC: SOF: eliminate redundant mailbox reads" into 2. One intended for the files in the upstream patches and another one for those not in it (hsw.c etc).
Collaborator
Author
There was a problem hiding this comment. I see, but how should I know which ones are and which ones aren't included? I think we need a separate branch, really.
Member
There was a problem hiding this comment. @lyakh check topic/sof-dev-rebase, everything after 'add build support' is not upstream yet. Ideally your patches should apply cleanly on the initial batch. Thanks!
Collaborator
Author
There was a problem hiding this comment. @plbossart let me take a semi-qualified guess: is this the commit, that I should be branching from?
|
||
| static int spi_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) | ||
| { | ||
| /* send the message */ | ||
|
|
@@ -221,6 +207,23 @@ static int spi_get_reply(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) | |
| return ret; | ||
| } | ||
|
|
||
| static irqreturn_t spi_irq_thread(int irq __maybe_unused, void *context __maybe_unused) | ||
| { | ||
| struct snd_sof_dev *sdev = context; | ||
|
|
||
| // read SPI data into local buffer and determine IPC cmd or reply | ||
|
|
||
| /* | ||
| * if reply. Handle Immediate reply from DSP Core and set DSP | ||
| * state to ready | ||
| */ | ||
|
|
||
| /* if cmd, Handle messages from DSP Core */ | ||
| spi_get_reply(sdev, sdev->msg); | ||
|
|
||
| return IRQ_HANDLED; | ||
| } | ||
|
|
||
| /* | ||
| * Probe and remove. | ||
| */ | ||
|
|
@@ -265,11 +268,6 @@ static int spi_sof_remove(struct snd_sof_dev *sdev) | |
| return 0; | ||
| } | ||
|
|
||
| /* SPI SOF ops */ | ||
| const struct snd_sof_dsp_ops snd_sof_spi_ops = { | ||
| /* device init */ | ||
|
|
@@ -290,9 +288,7 @@ const struct snd_sof_dsp_ops snd_sof_spi_ops = { | |
|
|
||
| /* ipc */ | ||
| .send_msg = spi_send_msg, | ||
| .fw_ready = spi_fw_ready, | ||
|
|
||
| /* debug */ | ||
| .debug_map = NULL/*spi_debugfs*/, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,7 +67,9 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = { | |
| SOF_DEBUGFS_ACCESS_ALWAYS}, | ||
| }; | ||
|
|
||
| static void bdw_host_done(struct snd_sof_dev *sdev); | ||
| static void bdw_dsp_done(struct snd_sof_dev *sdev); | ||
| static void bdw_get_reply(struct snd_sof_dev *sdev); | ||
|
|
||
| /* | ||
| * DSP Control. | ||
|
|
@@ -288,8 +290,10 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) | |
| * because the done bit can't be set in cmd_done function | ||
| * which is triggered by msg | ||
| */ | ||
| bdw_get_reply(sdev); | ||
| snd_sof_ipc_reply(sdev, ipcx); | ||
|
|
||
| bdw_dsp_done(sdev); | ||
| } | ||
|
|
||
| ipcd = snd_sof_dsp_read(sdev, BDW_DSP_BAR, SHIM_IPCD); | ||
|
|
@@ -309,6 +313,8 @@ static irqreturn_t bdw_irq_thread(int irq, void *context) | |
| } else { | ||
| snd_sof_ipc_msgs_rx(sdev); | ||
| } | ||
|
|
||
| bdw_host_done(sdev); | ||
| } | ||
|
|
||
| return IRQ_HANDLED; | ||
|
|
@@ -475,59 +481,61 @@ static int bdw_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) | |
| return 0; | ||
| } | ||
|
|
||
| static void bdw_get_reply(struct snd_sof_dev *sdev) | ||
| { | ||
| struct snd_sof_ipc_msg *msg = sdev->msg; | ||
| struct sof_ipc_reply reply; | ||
| unsigned long flags; | ||
| int ret = 0; | ||
|
|
||
| /* get reply */ | ||
| sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply)); | ||
|
|
||
| spin_lock_irqsave(&sdev->ipc_lock, flags); | ||
|
|
||
| if (reply.error < 0) { | ||
| memcpy(msg->reply_data, &reply, sizeof(reply)); | ||
| ret = reply.error; | ||
| } else { | ||
|
Member
There was a problem hiding this comment. @lyakh please add a more complete prefix in your patches, e.g. for this one this should be ASoC: SOF: Intel since you are only touching intel-specific patches.
Member
There was a problem hiding this comment. @lyakh this comment still applies. ASoC: SOF: Intel prefix please, it makes things easier to track for reviewers (some of whom may not care about Intel specifics) |
||
| /* reply correct size ? */ | ||
| if (reply.hdr.size != msg->reply_size) { | ||
|
Collaborator
There was a problem hiding this comment. So, besides removing the else clause your patch also modifies the format for dev_err. I think that should be the subject of another patch. |
||
| dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n", | ||
| msg->reply_size, reply.hdr.size); | ||
| ret = -EINVAL; | ||
| } | ||
|
|
||
| /* read the message */ | ||
| if (msg->reply_size > 0) | ||
| sof_mailbox_read(sdev, sdev->host_box.offset, | ||
| msg->reply_data, msg->reply_size); | ||
| } | ||
|
|
||
| msg->reply_error = ret; | ||
|
|
||
| spin_unlock_irqrestore(&sdev->ipc_lock, flags); | ||
| } | ||
|
|
||
| static void bdw_host_done(struct snd_sof_dev *sdev) | ||
| { | ||
| /* clear BUSY bit and set DONE bit - accept new messages */ | ||
| snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IPCD, | ||
| SHIM_IPCD_BUSY | SHIM_IPCD_DONE, | ||
| SHIM_IPCD_DONE); | ||
|
|
||
| /* unmask busy interrupt */ | ||
| snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IMRX, | ||
| SHIM_IMRX_BUSY, 0); | ||
| } | ||
|
|
||
| static void bdw_dsp_done(struct snd_sof_dev *sdev) | ||
| { | ||
| /* clear DONE bit - tell DSP we have completed */ | ||
| snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IPCX, | ||
| SHIM_IPCX_DONE, 0); | ||
|
|
||
| /* unmask Done interrupt */ | ||
| snd_sof_dsp_update_bits_unlocked(sdev, BDW_DSP_BAR, SHIM_IMRX, | ||
| SHIM_IMRX_DONE, 0); | ||
| } | ||
|
|
||
| /* | ||
|
|
@@ -664,9 +672,7 @@ const struct snd_sof_dsp_ops sof_bdw_ops = { | |
|
|
||
| /* ipc */ | ||
| .send_msg = bdw_send_msg, | ||
| .fw_ready = bdw_fw_ready, | ||
|
|
||
| /* debug */ | ||
| .debug_map = bdw_debugfs, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am probably getting on your nerves but even this needs to be split in 3. My topic/sof-def-rebase have haswell, SKL and SPI added in 3 separate steps.