Skip to content

ASoC: SOF: Intel: hda: fix error handling during FW boot - #2343

Merged
plbossart merged 1 commit into
thesofproject:topic/sof-devfrom
ranj063:fix/fw_boot_error
Aug 10, 2020
Merged

ASoC: SOF: Intel: hda: fix error handling during FW boot#2343
plbossart merged 1 commit into
thesofproject:topic/sof-devfrom
ranj063:fix/fw_boot_error

Conversation

@ranj063

@ranj063ranj063 commented Aug 4, 2020•
edited
Loading

Copy link
Copy Markdown
Collaborator

If get_stream_with_tag() returns a NULL pointer, the stream
prepared in cl_stream_prepare() should still be cleaned up. But,
this is impossible because if the tag returned by cl_stream_prepare()
is valid, get_stream_with_tag() would theoretically never fail to
find a valis stream and there is no need to check the error case.

This can be simplified by returning a pointer to struct
hdac_ext_stream from cl_stream_prepare() if a stream is successfully
prepared or NULL otherwise. If cl_stream_prepare() is successful,
it must be cleaned up after FW boot irrespective of whether boot is
successful or not. This change renders the function
get_stream_with_tag() redundant and can be removed.

Replaces #2342

@ranj063
ranj063force-pushed the fix/fw_boot_error branch 3 times, most recently from c0dea36 to c33b5dfCompareAugust 5, 2020 03:00
lyakh
lyakh requested changes Aug 5, 2020
View reviewed changes

@lyakhlyakh left a comment

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.

A nice simplification! Just a couple of minor improvement suggestions

Comment threadsound/soc/sof/intel/hda-loader.c Outdated
Comment threadsound/soc/sof/intel/hda-loader.c Outdated
@ranj063
ranj063 requested a review from lyakhAugust 5, 2020 16:51
lyakh
lyakh requested changes Aug 6, 2020
View reviewed changes
Comment threadsound/soc/sof/intel/hda-loader.c Outdated
Comment threadsound/soc/sof/intel/hda-loader.c Outdated
@ranj063
ranj063 requested a review from lyakhAugust 6, 2020 13:04
lyakh
lyakh requested changes Aug 7, 2020
View reviewed changes
Comment threadsound/soc/sof/intel/hda-loader.c Outdated
@ranj063
ranj063 requested a review from lyakhAugust 7, 2020 15:30
plbossart
plbossart requested changes Aug 7, 2020
View reviewed changes

@plbossartplbossart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The commit message still mentions returning a NULL, when you really return an ERR_PTR() value.
Also the first paragraph is convoluted and sounds like circular logic. A clearer set of logical propositions might be easier to read.

@ranj063
ASoC: SOF: Intel: hda: Simplify error handling during FW boot
27b2873
Modify cl_stream_prepare() to return a pointer to the prepared stream
if successful or ERR_PTR() otherwise. This would simplify the error
paths in hda_dsp_cl_boot_firmware() and hda_dsp_cl_boot_firmware_iccmax()
to perform the stream cleanup after FW boot. This change also renders
the function get_stream_with_tag() redundant.
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
@ranj063

ranj063 commented Aug 7, 2020

Copy link
Copy Markdown
CollaboratorAuthor

The commit message still mentions returning a NULL, when you really return an ERR_PTR() value.
Also the first paragraph is convoluted and sounds like circular logic. A clearer set of logical propositions might be easier to read.

@plbossart how about now?

@ranj063
ranj063 requested a review from plbossartAugust 7, 2020 16:44
@plbossart
plbossart merged commit 1ab1be7 into thesofproject:topic/sof-devAug 10, 2020
@ranj063
ranj063 deleted the fix/fw_boot_error branch February 13, 2022 05:08
oder-chiou pushed a commit to oder-chiou/linux-soundwire that referenced this pull request May 5, 2026
@kirankrishnappa-intel@holtmann
Bluetooth: Fix alt settings for incoming SCO with transparent coding …
06d213d
…format
For incoming SCO connection with transparent coding format, alt setting
of CVSD is getting applied instead of Transparent.
Before fix:
< HCI Command: Accept Synchron.. (0x01|0x0029) plen 21 thesofproject#2196 [hci0] 321.342548
Address: 1C:CC:D6:E2:EA:80 (Xiaomi Communications Co Ltd)
Transmit bandwidth: 8000
Receive bandwidth: 8000
Max latency: 13
Setting: 0x0003
Input Coding: Linear
Input Data Format: 1's complement
Input Sample Size: 8-bit
# of bits padding at MSB: 0
Air Coding Format: Transparent Data
Retransmission effort: Optimize for link quality (0x02)
Packet type: 0x003f
HV1 may be used
HV2 may be used
HV3 may be used
EV3 may be used
EV4 may be used
EV5 may be used
> HCI Event: Command Status (0x0f) plen 4 thesofproject#2197 [hci0] 321.343585
Accept Synchronous Connection Request (0x01|0x0029) ncmd 1
Status: Success (0x00)
> HCI Event: Synchronous Connect Comp.. (0x2c) plen 17 thesofproject#2198 [hci0] 321.351666
Status: Success (0x00)
Handle: 257
Address: 1C:CC:D6:E2:EA:80 (Xiaomi Communications Co Ltd)
Link type: eSCO (0x02)
Transmission interval: 0x0c
Retransmission window: 0x04
RX packet length: 60
TX packet length: 60
Air mode: Transparent (0x03)
........
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2336 [hci0] 321.383655
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#2337 [hci0] 321.389558
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2338 [hci0] 321.393615
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2339 [hci0] 321.393618
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2340 [hci0] 321.393618
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#2341 [hci0] 321.397070
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2342 [hci0] 321.403622
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2343 [hci0] 321.403625
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2344 [hci0] 321.403625
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2345 [hci0] 321.403625
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#2346 [hci0] 321.404569
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#2347 [hci0] 321.412091
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2348 [hci0] 321.413626
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2349 [hci0] 321.413630
> SCO Data RX: Handle 257 flags 0x00 dlen 48 thesofproject#2350 [hci0] 321.413630
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#2351 [hci0] 321.419674
After fix:
< HCI Command: Accept Synchronou.. (0x01|0x0029) plen 21 thesofproject#309 [hci0] 49.439693
Address: 1C:CC:D6:E2:EA:80 (Xiaomi Communications Co Ltd)
Transmit bandwidth: 8000
Receive bandwidth: 8000
Max latency: 13
Setting: 0x0003
Input Coding: Linear
Input Data Format: 1's complement
Input Sample Size: 8-bit
# of bits padding at MSB: 0
Air Coding Format: Transparent Data
Retransmission effort: Optimize for link quality (0x02)
Packet type: 0x003f
HV1 may be used
HV2 may be used
HV3 may be used
EV3 may be used
EV4 may be used
EV5 may be used
> HCI Event: Command Status (0x0f) plen 4 thesofproject#310 [hci0] 49.440308
Accept Synchronous Connection Request (0x01|0x0029) ncmd 1
Status: Success (0x00)
> HCI Event: Synchronous Connect Complete (0x2c) plen 17 thesofproject#311 [hci0] 49.449308
Status: Success (0x00)
Handle: 257
Address: 1C:CC:D6:E2:EA:80 (Xiaomi Communications Co Ltd)
Link type: eSCO (0x02)
Transmission interval: 0x0c
Retransmission window: 0x04
RX packet length: 60
TX packet length: 60
Air mode: Transparent (0x03)
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#312 [hci0] 49.450421
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#313 [hci0] 49.457927
> HCI Event: Max Slots Change (0x1b) plen 3 thesofproject#314 [hci0] 49.460345
Handle: 256
Max slots: 5
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#315 [hci0] 49.465453
> SCO Data RX: Handle 257 flags 0x00 dlen 60 thesofproject#316 [hci0] 49.470502
> SCO Data RX: Handle 257 flags 0x00 dlen 60 thesofproject#317 [hci0] 49.470519
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#318 [hci0] 49.472996
> SCO Data RX: Handle 257 flags 0x00 dlen 60 thesofproject#319 [hci0] 49.480412
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#320 [hci0] 49.480492
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#321 [hci0] 49.487989
> SCO Data RX: Handle 257 flags 0x00 dlen 60 thesofproject#322 [hci0] 49.490303
< SCO Data TX: Handle 257 flags 0x00 dlen 60 thesofproject#323 [hci0] 49.495496
> SCO Data RX: Handle 257 flags 0x00 dlen 60 thesofproject#324 [hci0] 49.500304
> SCO Data RX: Handle 257 flags 0x00 dlen 60 thesofproject#325 [hci0] 49.500311
Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: Lokendra Singh <lokendra.singh@intel.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@plbossartplbossartplbossart approved these changes
@lyakhlyakhlyakh approved these changes
@bardliaobardliaoAwaiting requested review from bardliaobardliao is a code owner
@dbalutadbalutaAwaiting requested review from dbaluta
@kv2019ikv2019iAwaiting requested review from kv2019i
@lgirdwoodlgirdwoodAwaiting requested review from lgirdwood

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ranj063@plbossart@lyakh