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 2021/02/19 08:13:20 UTC

[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #2870: Support for ESP32-C3 GPIO Driver

Ouss4 opened a new pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870


   ## Summary
   Add support for the ESP32-C3 GPIO:
   1. IO MUX and GPIO Matrix,
   2. GPIO interrupts,
   3. Board logic for the GPIO example.
   ## Impact
   New driver for the ESP32-C3 no impact on other chips.
   ## Testing
   Provided gpio defconfig was used for testing on the ESP32-C3 devkit.
   


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] v01d commented on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782207482


   > I still don't have a strong argument in favour of moving towards that organisation for now. I feel like with only one board, it will confuse people a little and doesn't show the full benefit.
   
   My intention is to avoid layout out code which promotes copying as it is difficult to remain vigilant about duplicated code in subsquent PRs. I feel it is better to avoid it altogether from the start. 
   
   > In another note, the GPIO board logic isn't shared for other boards. There are examples for STM32 and ESP32 (where the common folder is used), both use separate GPIO board logic for each board. I think the way that file is implemented will make us expose definitions (GPIOXX) that are not usually exposed in board.h.
   
   I see, so maybe it would be worth revisiting what information does this code require so that is easier to move to common code. I guess we could do that later on.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782385127


   > My intention is to avoid layout out code which promotes copying as it is difficult to remain vigilant about duplicated code in subsquent PRs. I feel it is better to avoid it altogether from the start.
   
   Yes, I understand and agree with your point.
   I think our discussion has drifted a little bit from the purpose of this PR, we will definitely come back to the board's organisation in the upcoming days when we have more drivers supported.  However, as mentioned, the gpio board logic will either stay specific to each board or need some changes to make it a bit more generic.
   


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] v01d merged pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
v01d merged pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870


   


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782202497


   I still don't have a strong argument in favour of moving towards that organisation for now. I feel like with only one board, it will confuse people a little and doesn't show the full benefit.
   
   In another note, the GPIO board logic isn't shared for other boards.  There are examples for STM32 and ESP32 (where the common folder is used), both use separate GPIO board logic for each board.  I think the way that file is implemented will make us expose definitions (GPIOXX) that are not usually exposed in board.h.


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] v01d edited a comment on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
v01d edited a comment on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782185520


   I realized now the issue is not about different esp32c3 chips but boards. The point is that even though there are only two official boards, there will most likely be others in the future (maybe unofficial). It would be better to avoid board logic for other boards to have to copy this implementation. I think for this case the gpio lower-half is quite self-contained. Maybe it would be good to do this now and avoid copies of this file to be made by others in the future. What do you think?


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] v01d commented on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782071695


   Could we put the board-level file into esp32c3/common? This file does not really depend on the board itself and this way we avoid repetition of implementation (assuming another esp32c3 chip appears). 


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] v01d commented on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782185520


   I realized now the issue is not about different esp32c3 chips but boards. The point is that even though there are only two official boards, there will most likely be others in the future (maybe unofficial). It would be better to force board logic for other boards to have to copy this implementation. I think for this case the gpio lower-half is quite self-contained. Maybe it would be good to do this now and avoid copies of this file to be made by others in the future. What do you think?


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782818863


   @v01d Any concerns regarding this one?  Can we go ahead with a merge?


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] Ouss4 edited a comment on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
Ouss4 edited a comment on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782385127


   > My intention is to avoid layout out code which promotes copying as it is difficult to remain vigilant about duplicated code in subsquent PRs. I feel it is better to avoid it altogether from the start.
   
   Yes, I understand and agree with your point.
   I think, however, that our discussion has drifted a little bit from the purpose of this PR, we will definitely come back to the board's organisation in the upcoming days when we have more drivers supported.  However, as mentioned, the gpio board logic will either stay specific to each board or need some changes to make it a bit more generic.
   


----------------------------------------------------------------
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.

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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #2870: Support for ESP32-C3 GPIO Driver

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2870:
URL: https://github.com/apache/incubator-nuttx/pull/2870#issuecomment-782091551


   > Could we put the board-level file into esp32c3/common? This file does not really depend on the board itself and this way we avoid repetition of implementation (assuming another esp32c3 chip appears).
   
   I thought about this one when we created the initial support.  For now we only have one board* this is why we decided to use the simple/old board organisation, i.e. no common and drivers folders.  The moment we have another board to support we will update the organisation.
   
   *Actually there are 2 ESP32-C3 boards, but from our perspective they are the same.


----------------------------------------------------------------
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.

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