You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/10/25 16:23:52 UTC

[GitHub] [incubator-nuttx] tmedicci opened a new pull request, #7429: audio/i2s: add i2s_mclkfrequency method to the I2S lower half

tmedicci opened a new pull request, #7429:
URL: https://github.com/apache/incubator-nuttx/pull/7429

   ## Summary
   This method allows the codec driver to set a specific mclk frequency for the I2S lower half, allowing to change it on run time according to the supported master clock frequency of the audio codec, the sample rate and the data width of the audio file being played.
   Also, add support for setting I2S's mclk on CS4344 audio codec and on ESP32's I2S driver.
   
   ## Impact
   Implementation does not impact existing audio codec drivers and I2S drivers as the method would be NULL.
   
   ## Testing
   Internal CI and hardware testing using ESP32-DevKitC board + Audio Codec CS4344.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7429: audio/i2s: add i2s_mclkfrequency method to the I2S lower half

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7429:
URL: https://github.com/apache/incubator-nuttx/pull/7429#discussion_r1005885662


##########
include/nuttx/audio/i2s.h:
##########
@@ -286,6 +309,11 @@ struct i2s_ops_s
                             FAR void *arg,
                             uint32_t timeout);
 
+  /* Master Clock methods */
+
+  CODE uint32_t (*i2s_mclkfrequency)(FAR struct i2s_dev_s *dev,

Review Comment:
   The call of i2s_mclkfrequency already check the pointer isn't NULL, so it isn't a problem.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] tmedicci commented on a diff in pull request #7429: audio/i2s: add i2s_mclkfrequency method to the I2S lower half

Posted by GitBox <gi...@apache.org>.
tmedicci commented on code in PR #7429:
URL: https://github.com/apache/incubator-nuttx/pull/7429#discussion_r1005540843


##########
include/nuttx/audio/i2s.h:
##########
@@ -286,6 +309,11 @@ struct i2s_ops_s
                             FAR void *arg,
                             uint32_t timeout);
 
+  /* Master Clock methods */
+
+  CODE uint32_t (*i2s_mclkfrequency)(FAR struct i2s_dev_s *dev,

Review Comment:
   Hi!
   
   All users define these method on `i2s_ops_s` by designated initializers: https://github.com/apache/incubator-nuttx/blob/d1d46335df6cc1bc63f1cd7e7bffe3735b8c275d/arch/arm/src/rp2040/rp2040_i2s.c#L218
   
   Thus, there shouldn't have any problems regarding I2S drivers that don't implement the method.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #7429: audio/i2s: add i2s_mclkfrequency method to the I2S lower half

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7429:
URL: https://github.com/apache/incubator-nuttx/pull/7429


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7429: audio/i2s: add i2s_mclkfrequency method to the I2S lower half

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7429:
URL: https://github.com/apache/incubator-nuttx/pull/7429#discussion_r1004942723


##########
include/nuttx/audio/i2s.h:
##########
@@ -286,6 +309,11 @@ struct i2s_ops_s
                             FAR void *arg,
                             uint32_t timeout);
 
+  /* Master Clock methods */
+
+  CODE uint32_t (*i2s_mclkfrequency)(FAR struct i2s_dev_s *dev,

Review Comment:
   Are the all i2s_ops_s users updated to handle insertion of the new method accordingly?



##########
include/nuttx/audio/i2s.h:
##########
@@ -286,6 +309,11 @@ struct i2s_ops_s
                             FAR void *arg,
                             uint32_t timeout);
 
+  /* Master Clock methods */
+
+  CODE uint32_t (*i2s_mclkfrequency)(FAR struct i2s_dev_s *dev,

Review Comment:
   Are the all `i2s_ops_s` users updated to handle insertion of the new method accordingly?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7429: audio/i2s: add i2s_mclkfrequency method to the I2S lower half

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7429:
URL: https://github.com/apache/incubator-nuttx/pull/7429#discussion_r1004942723


##########
include/nuttx/audio/i2s.h:
##########
@@ -286,6 +309,11 @@ struct i2s_ops_s
                             FAR void *arg,
                             uint32_t timeout);
 
+  /* Master Clock methods */
+
+  CODE uint32_t (*i2s_mclkfrequency)(FAR struct i2s_dev_s *dev,

Review Comment:
   Are the all i2c_ops_s users updated to handle insertion of the new method accordingly?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] tmedicci commented on a diff in pull request #7429: audio/i2s: add i2s_mclkfrequency method to the I2S lower half

Posted by GitBox <gi...@apache.org>.
tmedicci commented on code in PR #7429:
URL: https://github.com/apache/incubator-nuttx/pull/7429#discussion_r1005540843


##########
include/nuttx/audio/i2s.h:
##########
@@ -286,6 +309,11 @@ struct i2s_ops_s
                             FAR void *arg,
                             uint32_t timeout);
 
+  /* Master Clock methods */
+
+  CODE uint32_t (*i2s_mclkfrequency)(FAR struct i2s_dev_s *dev,

Review Comment:
   Hi!
   
   All users define these the method by designated initializers: https://github.com/apache/incubator-nuttx/blob/d1d46335df6cc1bc63f1cd7e7bffe3735b8c275d/arch/arm/src/rp2040/rp2040_i2s.c#L218
   
   Thus, there shouldn't have any problems regarding I2S drivers that don't implement the method.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] tmedicci commented on pull request #7429: audio/i2s: add i2s_mclkfrequency method to the I2S lower half

Posted by GitBox <gi...@apache.org>.
tmedicci commented on PR #7429:
URL: https://github.com/apache/incubator-nuttx/pull/7429#issuecomment-1290829333

   Following https://github.com/apache/incubator-nuttx/pull/7421#issuecomment-1289502232
   
   I've tested and found that the audio codec also supports 16KHz and 22,05KHz sample rates. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org