You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "pkarashchenko (via GitHub)" <gi...@apache.org> on 2024/05/20 22:02:33 UTC
[PR] arch/arm/samv7: fix USART1 TXD pinmux [nuttx]
pkarashchenko opened a new pull request, #12382:
URL: https://github.com/apache/nuttx/pull/12382
## Summary
Fix pinmux of SAMv7 USART1 TXD
## Impact
Improve SAMv7 USART1
## Testing
Tested similar change with SAMv7 LIN on USART1
--
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
Re: [PR] arch/arm/samv7: fix USART1 TXD pinmux [nuttx]
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on PR #12382:
URL: https://github.com/apache/nuttx/pull/12382#issuecomment-2124385085
> Should not this be already set in https://github.com/apache/nuttx/blob/master/arch/arm/src/samv7/sam_gpio.c#L466? And just raise compile time error if both `USART1` and `CONFIG_SAMV7_JTAG_FULL_ENABLE` is set?
I didn't notice the code in `sam_gpio.c` and only found https://github.com/apache/nuttx/blob/4b2b26c0b05f4c034d781a6215971e1675623486/arch/arm/src/samv7/sam_lowputc.c#L335
Indeed, maybe it is a good idea to rise an error. I've checked my config and indeed the `CONFIG_SAMV7_JTAG_FULL_ENABLE` is set in my case, so rising an error may fix the inconsistency in the code
--
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
Re: [PR] arch/arm/samv7: fix USART1 TXD pinmux [nuttx]
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #12382:
URL: https://github.com/apache/nuttx/pull/12382#discussion_r1609642076
##########
arch/arm/src/samv7/sam_serial_spi.c:
##########
@@ -69,16 +69,16 @@
struct sam_serial_spi_pins_s
{
- uint32_t mosi;
- uint32_t miso;
- uint32_t sck;
- uint32_t nss;
+ const uint32_t mosi;
+ const uint32_t miso;
+ const uint32_t sck;
+ const uint32_t nss;
};
struct sam_serial_spi_s
{
struct sam_serial_spi_pins_s pins;
- uint32_t base; /* SPI controller register base address */
+ const uint32_t base; /* SPI controller register base address */
Review Comment:
I can revert. It was just from logical perspective as the values of the base address should not change
--
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
Re: [PR] arch/arm/samv7: fix USART1 TXD pinmux [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #12382:
URL: https://github.com/apache/nuttx/pull/12382#discussion_r1607447792
##########
arch/arm/src/samv7/sam_serial_spi.c:
##########
@@ -69,16 +69,16 @@
struct sam_serial_spi_pins_s
{
- uint32_t mosi;
- uint32_t miso;
- uint32_t sck;
- uint32_t nss;
+ const uint32_t mosi;
Review Comment:
why add const to integer fields?
##########
arch/arm/src/samv7/sam_serial_spi.c:
##########
@@ -69,16 +69,16 @@
struct sam_serial_spi_pins_s
{
- uint32_t mosi;
- uint32_t miso;
- uint32_t sck;
- uint32_t nss;
+ const uint32_t mosi;
+ const uint32_t miso;
+ const uint32_t sck;
+ const uint32_t nss;
};
struct sam_serial_spi_s
{
struct sam_serial_spi_pins_s pins;
- uint32_t base; /* SPI controller register base address */
+ const uint32_t base; /* SPI controller register base address */
Review Comment:
ditto
--
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
Re: [PR] arch/arm/samv7: enforce compilation error for incompatible configuration options [nuttx]
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on PR #12382:
URL: https://github.com/apache/nuttx/pull/12382#issuecomment-2124678984
@michallenc please check
--
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
Re: [PR] arch/arm/samv7: enforce compilation error for incompatible configuration options [nuttx]
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #12382:
URL: https://github.com/apache/nuttx/pull/12382#discussion_r1609897210
##########
arch/arm/src/samv7/sam_lowputc.c:
##########
@@ -327,14 +327,10 @@ void sam_lowsetup(void)
*/
# if defined(CONFIG_SAMV7_JTAG_FULL_ENABLE)
-# warning CONFIG_SAMV7_JTAG_FULL_ENABLE is incompatible with CONFIG_SAMV7_USART1.
-# warning The SYSIO Pin4 must be bound to PB4 to use USART1
+# error CONFIG_SAMV7_JTAG_FULL_ENABLE is incompatible with CONFIG_SAMV7_USART1.
Review Comment:
Ok. I can remove one from here
--
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
Re: [PR] arch/arm/samv7: fix USART1 TXD pinmux [nuttx]
Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on PR #12382:
URL: https://github.com/apache/nuttx/pull/12382#issuecomment-2121932616
Should not this be already set in https://github.com/apache/nuttx/blob/master/arch/arm/src/samv7/sam_gpio.c#L466? And just raise compile time error if both `USART1` and `CONFIG_SAMV7_JTAG_FULL_ENABLE` is set?
--
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
Re: [PR] arch/arm/samv7: enforce compilation error for incompatible configuration options [nuttx]
Posted by "michallenc (via GitHub)" <gi...@apache.org>.
michallenc commented on code in PR #12382:
URL: https://github.com/apache/nuttx/pull/12382#discussion_r1609889789
##########
arch/arm/src/samv7/sam_lowputc.c:
##########
@@ -327,14 +327,10 @@ void sam_lowsetup(void)
*/
# if defined(CONFIG_SAMV7_JTAG_FULL_ENABLE)
-# warning CONFIG_SAMV7_JTAG_FULL_ENABLE is incompatible with CONFIG_SAMV7_USART1.
-# warning The SYSIO Pin4 must be bound to PB4 to use USART1
+# error CONFIG_SAMV7_JTAG_FULL_ENABLE is incompatible with CONFIG_SAMV7_USART1.
Review Comment:
This should now be redundant with the same check in `sam_gpio.c`, no?
--
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
Re: [PR] arch/arm/samv7: fix USART1 TXD pinmux [nuttx]
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #12382:
URL: https://github.com/apache/nuttx/pull/12382#discussion_r1609641822
##########
arch/arm/src/samv7/sam_serial_spi.c:
##########
@@ -69,16 +69,16 @@
struct sam_serial_spi_pins_s
{
- uint32_t mosi;
- uint32_t miso;
- uint32_t sck;
- uint32_t nss;
+ const uint32_t mosi;
Review Comment:
I can revert. It was just from logical perspective as the values of the pin should not change
--
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
Re: [PR] arch/arm/samv7: enforce compilation error for incompatible configuration options [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #12382:
URL: https://github.com/apache/nuttx/pull/12382
--
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