Skip to content

ASoC: SOF: trace: fix trace doesn't update issue #248

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

Closed
wants to merge 3 commits into from
Closed

ASoC: SOF: trace: fix trace doesn't update issue #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
21e9ca1
ASoC: SOF: trace: fix a put stream failed issue
keyonjie Nov 7, 2018
2848117
ASoC: SOF: trace: only init wait_queue one time
keyonjie Nov 7, 2018
593520e
ASoC: SOF: trace: fix trace is cleared after suspend/resume issue
keyonjie Nov 7, 2018
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
3 changes: 2 additions & 1 deletion sound/soc/sof/intel/hda-loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format,
hstream->format_val = format;
hstream->bufsize = size;

ret = hda_dsp_stream_hw_params(sdev, stream, dmab, NULL);
ret = hda_dsp_stream_hw_params(sdev, stream, dmab,
NULL, SOF_HDA_CL_STREAM);
if (ret < 0) {
dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret);
goto error;
Expand Down
3 changes: 2 additions & 1 deletion sound/soc/sof/intel/hda-pcm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev,
(params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) &&
(params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP);

ret = hda_dsp_stream_hw_params(sdev, stream, dmab, params);
ret = hda_dsp_stream_hw_params(sdev, stream, dmab,
params, SOF_HDA_AUDIO_STREAM);
if (ret < 0) {
dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret);
return ret;
Expand Down
79 changes: 54 additions & 25 deletions sound/soc/sof/intel/hda-stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,31 +88,46 @@ static int hda_setup_bdle(struct snd_sof_dev *sdev,
/*
* set up Buffer Descriptor List (BDL) for host memory transfer
* BDL describes the location of the individual buffers and is little endian.
*
* The typical BDL entries after set up should looks like this:
*
* bdl[0] from the init_offset, n entries before wrap,
* the e-1 full entries, and the last e entry.
*
* init_offset
* |
* |------|...........|---------S-------|------|........|--------|-------|
* bdl[n] bdl[n+j] bdl[e-1] | bdl[0] bdl[1] bdl[m] bdl[n-2] bdl[n-1]
Copy link
Collaborator

@ranj063 ranj063 Nov 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@keyonjie this is so difficult to follow. If I understand it correctly there are only 2 cases:
if init_offset ==0,there are bufsize/periodbytes (+1 in case it is not evenly divisible) periods
else there are 2 periods (or entries as you call it) right?

I am confused with n,m,j and e in the above diagram.

Copy link
Author

@keyonjie keyonjie Nov 9, 2018

Choose a reason for hiding this comment

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

@ranj063 the 2nd case what you mentioned can be more complicated, there can be > 2 periods there.
What the comment I wrote here is aim to abstract all possible cases to this common one.

You can ignore n, m, j, e here, though they are useful when writing code in logic that Pierre mentioned:

  1. from init_offset to first period start: bdl[0]
  2. then all regular periods, bdl[m]
  3. then end of buffer alignment, bdl[n-1] ---- We have n items before buffer end;
  4. then wrap until last period, bdl[e-2]
  5. and again remaining bytes, bdl[e-1]. ---- We have e items in total.

Copy link
Collaborator

@ranj063 ranj063 Nov 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@keyonjie Yes, I understand the logic and ther is not doubt it is correct.

If I may make a suggestion: can we treat the case of a non-zero init_offset as 2 separate buffers,
then we end up with just as many bld entries except that the partial segments will be only at the end of the 2 buffer areas. For example:
Case 1: init_offset = 0
num of period_bytes size bdl entries = 5
num of partial bdl entries = 1

init_offset = 0
|
|------|------|------|------|------|---|

Case 2: init_offset = 16
num of period_bytes size bdl entries = 4
num of partial bdl entries = 2

init_offset = 16

|------|------|----|------|------|-----|

So the partial fragments are at the end of the 2 areas ( 0 - init_offset -1, init_offset - bufsize).

But with the current case there will be 3 partial entries.

Anyway, this is only to improve maintainability. I am fine if the code is merged as is too.

Copy link
Author

@keyonjie keyonjie Nov 13, 2018

Choose a reason for hiding this comment

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

@keyonjie Yes, I understand the logic and ther is not doubt it is correct.

If I may make a suggestion: can we treat the case of a non-zero init_offset as 2 separate buffers,
then we end up with just as many bld entries except that the partial segments will be only at the end of the 2 buffer areas. For example:
Case 1: init_offset = 0
num of period_bytes size bdl entries = 5
num of partial bdl entries = 1

init_offset = 0
|
|------|------|------|------|------|---|

Case 2: init_offset = 16
num of period_bytes size bdl entries = 4
num of partial bdl entries = 2

init_offset = 16

|------|------|----|------|------|-----|

So the partial fragments are at the end of the 2 areas ( 0 - init_offset -1, init_offset - bufsize).

But with the current case there will be 3 partial entries.

Sorry I don't catch you here, there will be only 2 partial entries in your sample.

Anyway, this is only to improve maintainability. I am fine if the code is merged as is too.

Copy link
Collaborator

@ranj063 ranj063 Nov 13, 2018

Choose a reason for hiding this comment

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

@keyonjie what I meant was just break up the stream->bufsize into 2 and then assign bdl entries individually for [init_offset, buf_size) and then [0, init_offset -1). This way the logic will be a lot easier to follow. Let me take a swipe at it today and send you a patch

Copy link
Author

@keyonjie keyonjie Nov 14, 2018

Choose a reason for hiding this comment

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

@ranj063 the init_offset may be used for normal audio stream also, so we still need assign bdl entries for periods when init_offset!=0.

*/
int hda_dsp_stream_setup_bdl(struct snd_sof_dev *sdev,
struct snd_dma_buffer *dmab,
struct hdac_stream *stream)
struct hdac_stream *stream, u32 init_offset)
{
struct sof_intel_dsp_bdl *bdl;
int i, offset, period_bytes, periods;
int remain, ioc;
unsigned int i, period_bytes, entries;
u32 bytes, offset, size = 0;
bool ioc, set_ioc, is_wrap = false;

period_bytes = stream->period_bytes;
dev_dbg(sdev->dev, "period_bytes:0x%x\n", period_bytes);
if (!period_bytes)
period_bytes = stream->bufsize;

periods = stream->bufsize / period_bytes;
entries = stream->bufsize / period_bytes;

dev_dbg(sdev->dev, "periods:%d\n", periods);
/* count the last offset fragment entry */
if (stream->bufsize % period_bytes)
entries++;

remain = stream->bufsize % period_bytes;
if (remain)
periods++;
/* count the last index fragment entry */
if (init_offset % period_bytes)
entries++;

dev_dbg(sdev->dev, "entries:%d\n", entries);

/* program the initial BDL entries */
bdl = (struct sof_intel_dsp_bdl *)stream->bdl.area;
offset = 0;
offset = init_offset;
stream->frags = 0;

/*
Expand All @@ -122,19 +137,28 @@ int hda_dsp_stream_setup_bdl(struct snd_sof_dev *sdev,
ioc = sdev->hda->no_ipc_position ?
!stream->no_period_wakeup : 0;

for (i = 0; i < periods; i++) {
if (i == (periods - 1) && remain)
/* set the last small entry */
offset = hda_setup_bdle(sdev, dmab,
stream, &bdl, offset,
remain, 0);
for (i = 0; i < entries; i++) {
bytes = period_bytes - offset % period_bytes;
if (is_wrap)
bytes = min(bytes, init_offset - offset);
else
offset = hda_setup_bdle(sdev, dmab,
stream, &bdl, offset,
period_bytes, ioc);
bytes = min(bytes, stream->bufsize - offset);

/* only set ioc at period bundary */
set_ioc = (offset + bytes) % period_bytes ? 0 : 1;
offset = hda_setup_bdle(sdev, dmab,
stream, &bdl, offset,
bytes, set_ioc && ioc);

if (offset == stream->bufsize) {
offset = 0;
is_wrap = true;
}

size += bytes;
}
Copy link
Member

@plbossart plbossart Nov 8, 2018

Choose a reason for hiding this comment

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

I am unable to determine if the code is correct or not, it's significantly changed from the initial code and I am hesitant to sign-off here. Maybe it's correct, but it's not self-explanatory nor easy to debug/maintain. It'd be easier to deal with logical steps, such as from init_offset to first period start, then all regular periods, then end of buffer alignment, then wrap until last period and again remaining bytes?

Copy link
Collaborator

@ranj063 ranj063 Nov 9, 2018

Choose a reason for hiding this comment

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

@plbossart I've verified that the logic here is correct. quite impressive @keyonjie

But might be easier to follow if you break it up like pierre suggests.

Copy link
Author

@keyonjie keyonjie Nov 9, 2018

Choose a reason for hiding this comment

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

hmm, actually I spent hours to abstract and make it as simple as what it is now, otherwise, it will be inevitable lengthy and low efficiency with those 5 steps Pierre mentioned.

Copy link
Member

@plbossart plbossart Nov 9, 2018

Choose a reason for hiding this comment

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

You may have spent hours but I have no way of knowing if it works or not.
How many times have we faced issues with corner cases in this sort of code?
Please.

Copy link
Author

@keyonjie keyonjie Nov 13, 2018

Choose a reason for hiding this comment

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

So, shall I try add more comments with logic analyzing?

Copy link
Member

@plbossart plbossart Nov 13, 2018

Choose a reason for hiding this comment

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

The ask is to find a way to make things self-explanatory with all corner cases handled. You can do this with simple code or comments, your choice.


return offset;
return size;
}

int hda_dsp_stream_spib_config(struct snd_sof_dev *sdev,
Expand Down Expand Up @@ -344,19 +368,21 @@ int hda_dsp_stream_trigger(struct snd_sof_dev *sdev,
}

/*
* prepare for common hdac registers settings, for both code loader
* prepare for common hdac registers settings, for code loader, dma trace,
* and normal stream.
*/
int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev,
struct hdac_ext_stream *stream,
struct snd_dma_buffer *dmab,
struct snd_pcm_hw_params *params)
struct snd_pcm_hw_params *params,
enum sof_hda_tream_type stream_type)
{
struct hdac_bus *bus = sof_to_bus(sdev);
struct hdac_stream *hstream = &stream->hstream;
int sd_offset = SOF_STREAM_SD_OFFSET(hstream);
int ret, timeout = HDA_DSP_STREAM_RESET_TIMEOUT;
u32 val, mask;
u32 init_offset = 0;

if (!stream) {
dev_err(sdev->dev, "error: no stream available\n");
Expand Down Expand Up @@ -436,10 +462,13 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev,

hstream->frags = 0;

ret = hda_dsp_stream_setup_bdl(sdev, dmab, hstream);
if (ret < 0) {
dev_err(sdev->dev, "error: set up of BDL failed\n");
return ret;
if (stream_type == SOF_HDA_TRACE_STREAM)
init_offset = sdev->trace_init_offset;

ret = hda_dsp_stream_setup_bdl(sdev, dmab, hstream, init_offset);
if (ret != hstream->bufsize) {
dev_err(sdev->dev, "error: set up of BDL failed, bufsize:0x%x, only 0x%x set\n", hstream->bufsize, ret);
return -EFAULT;
}

/* set up stream descriptor for DMA */
Expand Down
4 changes: 2 additions & 2 deletions sound/soc/sof/intel/hda-trace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ static int hda_dsp_trace_prepare(struct snd_sof_dev *sdev)

hstream->bufsize = sdev->dmatb.bytes;

ret = hda_dsp_stream_hw_params(sdev, stream, dmab, NULL);
ret = hda_dsp_stream_hw_params(sdev, stream, dmab,
NULL, SOF_HDA_TRACE_STREAM);
if (ret < 0)
dev_err(sdev->dev, "error: hdac prepare failed: %x\n", ret);

Expand Down Expand Up @@ -75,7 +76,6 @@ int hda_dsp_trace_release(struct snd_sof_dev *sdev)

if (sdev->hda->dtrace_stream) {
hstream = &sdev->hda->dtrace_stream->hstream;
hstream->opened = false;
hda_dsp_stream_put_cstream(sdev,
hstream->stream_tag);
sdev->hda->dtrace_stream = NULL;
Expand Down
11 changes: 9 additions & 2 deletions sound/soc/sof/intel/hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,12 @@ struct sof_intel_dsp_desc {
struct snd_sof_dsp_ops *ops;
};

enum sof_hda_tream_type {
SOF_HDA_AUDIO_STREAM = 0, /* normal playback/capture stream */
SOF_HDA_CL_STREAM, /* code loader stream */
SOF_HDA_TRACE_STREAM, /* dma trace capture stream */
};

#define SOF_HDA_PLAYBACK_STREAMS 16
#define SOF_HDA_CAPTURE_STREAMS 16
#define SOF_HDA_PLAYBACK 0
Expand Down Expand Up @@ -442,14 +448,15 @@ void hda_dsp_stream_free(struct snd_sof_dev *sdev);
int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev,
struct hdac_ext_stream *stream,
struct snd_dma_buffer *dmab,
struct snd_pcm_hw_params *params);
struct snd_pcm_hw_params *params,
enum sof_hda_tream_type stream_type);
Copy link
Member

@plbossart plbossart Nov 9, 2018

Choose a reason for hiding this comment

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

and typo here as well

Copy link
Author

@keyonjie keyonjie Nov 13, 2018

Choose a reason for hiding this comment

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

thanks, will change.

int hda_dsp_stream_trigger(struct snd_sof_dev *sdev,
struct hdac_ext_stream *stream, int cmd);
irqreturn_t hda_dsp_stream_interrupt(int irq, void *context);
irqreturn_t hda_dsp_stream_threaded_handler(int irq, void *context);
int hda_dsp_stream_setup_bdl(struct snd_sof_dev *sdev,
struct snd_dma_buffer *dmab,
struct hdac_stream *stream);
struct hdac_stream *stream, u32 init_offset);

struct hdac_ext_stream *
hda_dsp_stream_get(struct snd_sof_dev *sdev, int direction);
Expand Down
3 changes: 2 additions & 1 deletion sound/soc/sof/sof-priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,8 @@ struct snd_sof_dev {
struct snd_dma_buffer dmatp;
int dma_trace_pages;
wait_queue_head_t trace_sleep;
u32 host_offset;
u32 trace_init_offset; /* restore trace offset upon resume */
u32 trace_offset;
bool dtrace_is_enabled;
bool dtrace_error;

Expand Down
44 changes: 34 additions & 10 deletions sound/soc/sof/trace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,23 @@ static size_t sof_wait_trace_avail(struct snd_sof_dev *sdev,
loff_t pos, size_t buffer_size)
{
wait_queue_entry_t wait;
u32 host_offset;

/* count the initial offset to get the buffer offset */
host_offset = (sdev->trace_init_offset + sdev->trace_offset) %
buffer_size;

/*
* If host offset is less than local pos, it means write pointer of
* host DMA buffer has been wrapped. We should output the trace data
* at the end of host DMA buffer at first.
*/
if (sdev->host_offset < pos)
if (host_offset < pos)
return buffer_size - pos;

/* If there is available trace data now, it is unnecessary to wait. */
if (sdev->host_offset > pos)
return sdev->host_offset - pos;
if (host_offset > pos)
return host_offset - pos;

/* wait for available trace data from FW */
init_waitqueue_entry(&wait, current);
Expand All @@ -58,11 +63,15 @@ static size_t sof_wait_trace_avail(struct snd_sof_dev *sdev,
remove_wait_queue(&sdev->trace_sleep, &wait);

out:
/* update buffer offset after wait done */
host_offset = (sdev->trace_init_offset + sdev->trace_offset) %
buffer_size;

/* return bytes available for copy */
if (sdev->host_offset < pos)
if (host_offset < pos)
return buffer_size - pos;
else
return sdev->host_offset - pos;
return host_offset - pos;
}
Copy link
Member

@plbossart plbossart Nov 8, 2018

Choose a reason for hiding this comment

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

I don't like copy/paste code with different comments, can this be a helper function?

Copy link
Author

@keyonjie keyonjie Nov 9, 2018

Choose a reason for hiding this comment

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

hmm, make sense, let me refine it.


static ssize_t sof_dfsentry_trace_read(struct file *file, char __user *buffer,
Expand Down Expand Up @@ -154,16 +163,24 @@ int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev)
if (sdev->dtrace_is_enabled)
return -EINVAL;

/* update init_offset, and reinitial offset to 0 */
sdev->trace_init_offset = (sdev->trace_init_offset +
sdev->trace_offset) %
sdev->dmatb.bytes;
sdev->trace_offset = 0;

/* set IPC parameters */
params.hdr.size = sizeof(params);
params.hdr.cmd = SOF_IPC_GLB_TRACE_MSG | SOF_IPC_TRACE_DMA_PARAMS;
params.buffer.phy_addr = sdev->dmatp.addr;
params.buffer.size = sdev->dmatb.bytes;
params.buffer.offset = 0;
params.buffer.pages = sdev->dma_trace_pages;

init_waitqueue_head(&sdev->trace_sleep);
sdev->host_offset = 0;
/*
* send offset to FW to require copying new logs
* start from new offset if possible.
*/
params.buffer.offset = sdev->trace_init_offset;
Copy link
Member

@plbossart plbossart Nov 8, 2018

Choose a reason for hiding this comment

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

I am now officially confused. I understand trace_init_offset and the offset to be used on resume. Now you are using it as a dynamic value that's passed to firmware, not sure I get what you are trying to do.

Copy link
Author

@keyonjie keyonjie Nov 9, 2018

Choose a reason for hiding this comment

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

This is used to cover that case on SKL- platforms where we don't have host DMA, as I mentioned in the PR commit message:

Todo: for SKL- platforms where not using BDL entries, FW to support
non-0 offset need to be implemented for those platforms, please refer
to this:
thesofproject/sof#544

Copy link
Member

@plbossart plbossart Nov 9, 2018

Choose a reason for hiding this comment

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

Then let's be clear: I will not merge a hardware-agnostic kernel patch that will only work with SKL+ firmware. Our team has the charter to deal with legacy platforms so the firmware changes need to happen first.

Copy link
Author

@keyonjie keyonjie Nov 13, 2018

Choose a reason for hiding this comment

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

hmm, this was the biggest concern from me when writing this patch, let me raise #544 and try if we can speed up the implementation in FW side.

Copy link
Member

@plbossart plbossart Nov 13, 2018

Choose a reason for hiding this comment

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

Something is wrong with your references, torvalds#544 does not refer to any firmware issues

Copy link
Author

@keyonjie keyonjie Nov 14, 2018

Choose a reason for hiding this comment

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

sorry, it should be thesofproject/sof#544.
by the way, @plbossart , just synced with @ranj063 , we actually haven't enable PM on SKL- platforms yet, so init_offset there are always 0, we this PR won't impact the current behaviors on those platforms.


ret = snd_sof_dma_trace_init(sdev, &params.stream_tag);
if (ret < 0) {
Expand Down Expand Up @@ -201,6 +218,8 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev)

/* set false before start initialization */
sdev->dtrace_is_enabled = false;
sdev->trace_offset = 0;
sdev->trace_init_offset = 0;

/* allocate trace page table buffer */
ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, sdev->parent,
Expand Down Expand Up @@ -235,6 +254,10 @@ int snd_sof_init_trace(struct snd_sof_dev *sdev)
goto table_err;
}

init_waitqueue_head(&sdev->trace_sleep);

init_waitqueue_head(&sdev->trace_sleep);
Copy link
Member

@plbossart plbossart Nov 8, 2018

Choose a reason for hiding this comment

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

why are we initializing the waitqueue twice?

Copy link
Author

@keyonjie keyonjie Nov 9, 2018

Choose a reason for hiding this comment

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

sorry, this is refining error with 'patch am', will fix it, thanks.


ret = snd_sof_init_trace_ipc(sdev);
if (ret < 0)
goto table_err;
Expand All @@ -251,8 +274,9 @@ EXPORT_SYMBOL(snd_sof_init_trace);
int snd_sof_trace_update_pos(struct snd_sof_dev *sdev,
struct sof_ipc_dma_trace_posn *posn)
{
if (sdev->dtrace_is_enabled && sdev->host_offset != posn->host_offset) {
sdev->host_offset = posn->host_offset;
if (sdev->dtrace_is_enabled &&
sdev->trace_offset != posn->host_offset) {
sdev->trace_offset = posn->host_offset;
wake_up(&sdev->trace_sleep);
Copy link
Member

@plbossart plbossart Nov 8, 2018

Choose a reason for hiding this comment

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

The code is becoming unreadable. Please keep the initial definition of host_offset and init_host_offset, the introduction of 'trace' makes things difficult to follow.

Copy link
Author

@keyonjie keyonjie Nov 9, 2018

Choose a reason for hiding this comment

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

I agree with Liam that 'trace' is better than 'host' here, as it is member of sdev, but only used for trace(not for other streams).

Copy link
Member

@plbossart plbossart Nov 9, 2018

Choose a reason for hiding this comment

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

then change the dma_trace_posn structure to use trace_offset... It makes no sense to use different fields in different structures when it's the same concept.

Copy link
Author

@keyonjie keyonjie Nov 13, 2018

Choose a reason for hiding this comment

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

the dma_trace_posn struct is replied from FW, looks host_offset better there. the sdev is only used in driver, so they have actually different standpoint, can we just keep this?

Copy link
Member

@plbossart plbossart Nov 13, 2018

Choose a reason for hiding this comment

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

I don't understand your reply. Why would the same concept be named differently if this is firmware or driver?

}

Expand Down