Skip to content

drivers: dai: sai: don't crash on underrun/overrun #77236

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
merged 1 commit into from
Aug 27, 2024
Merged

drivers: dai: sai: don't crash on underrun/overrun #77236

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
14 changes: 4 additions & 10 deletions drivers/dai/nxp/sai/sai.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,20 +95,14 @@ void sai_isr(const void *parameter)

/* check for TX FIFO error */
if (SAI_TX_RX_STATUS_IS_SET(DAI_DIR_TX, data->regmap, kSAI_FIFOErrorFlag)) {
LOG_ERR("FIFO underrun detected");
/* TODO: this will crash the program and should be addressed as
* mentioned in TODO list's 2).
*/
z_irq_spurious(NULL);
LOG_WRN("FIFO underrun detected");
SAI_TX_RX_STATUS_CLEAR(DAI_DIR_TX, data->regmap, kSAI_FIFOErrorFlag);
}

/* check for RX FIFO error */
if (SAI_TX_RX_STATUS_IS_SET(DAI_DIR_RX, data->regmap, kSAI_FIFOErrorFlag)) {
LOG_ERR("FIFO overrun detected");
/* TODO: this will crash the program and should be addressed as
* mentioned in TODO list's 2).
*/
z_irq_spurious(NULL);
LOG_WRN("FIFO overrun detected");
SAI_TX_RX_STATUS_CLEAR(DAI_DIR_RX, data->regmap, kSAI_FIFOErrorFlag);
}
}

Expand Down
5 changes: 5 additions & 0 deletions drivers/dai/nxp/sai/sai.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@
((dir) == DAI_DIR_RX ? ((UINT_TO_I2S(regmap))->RCSR & (which)) : \
((UINT_TO_I2S(regmap))->TCSR & (which)))

/* used to clear status flags */
#define SAI_TX_RX_STATUS_CLEAR(dir, regmap, which) \
((dir) == DAI_DIR_RX ? SAI_RxClearStatusFlags(UINT_TO_I2S(regmap), which) \
: SAI_TxClearStatusFlags(UINT_TO_I2S(regmap), which))

Check notice on line 216 in drivers/dai/nxp/sai/sai.h

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

drivers/dai/nxp/sai/sai.h:216 
-#define SAI_TX_RX_STATUS_CLEAR(dir, regmap, which)						\
-	((dir) == DAI_DIR_RX ? SAI_RxClearStatusFlags(UINT_TO_I2S(regmap), which)		\
+#define SAI_TX_RX_STATUS_CLEAR(dir, regmap, which)                                                 \
+	((dir) == DAI_DIR_RX ? SAI_RxClearStatusFlags(UINT_TO_I2S(regmap), which)                  \

Check notice on line 216 in drivers/dai/nxp/sai/sai.h

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

drivers/dai/nxp/sai/sai.h:216 
-#define SAI_TX_RX_STATUS_CLEAR(dir, regmap, which)						\
-	((dir) == DAI_DIR_RX ? SAI_RxClearStatusFlags(UINT_TO_I2S(regmap), which)		\
+#define SAI_TX_RX_STATUS_CLEAR(dir, regmap, which)                                                 \
+	((dir) == DAI_DIR_RX ? SAI_RxClearStatusFlags(UINT_TO_I2S(regmap), which)                  \

/* used to retrieve the SYNC direction. Use this macro when you know for sure
Copy link
Contributor

Choose a reason for hiding this comment

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

@LaurentiuM1234 change looks good to me. Will approve once this warning is fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tweaked the formatting of the macro as the clang-format suggestion did improve the code readability IMO. Warning still persists because of the position of \. I don't think this new suggestion improves code readability so unless we're required to make new code clang-format-compliant I see no reason to go with it.

* you have 1 SYNC direction with 1 ASYNC direction.
*/
Expand Down
Loading