Skip to content

Conversation

sunil-abraham
Copy link
Contributor

This update introduces more functionality to UART G1 driver, including implementation of interrupt API.
Additionally, Device Tree bindings are added to support SERCOM modules in both 'n' and 'p' SoC variants.

@sunil-abraham sunil-abraham marked this pull request as ready for review October 6, 2025 12:29
@zephyrbot zephyrbot added platform: Microchip MEC Microchip MEC Platform area: Devicetree Bindings platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) area: UART Universal Asynchronous Receiver-Transmitter area: Boards/SoCs labels Oct 6, 2025
@dcpleung
Copy link
Member

dcpleung commented Oct 6, 2025

Could you amend the commit summary from ... uart g1: microchip:... to ... uart: microchip/g1: ... where the g1 part is associated with microchip?

Add more functionality in uart driver.
Add bindings for sercom in n and p SOC variants.

Signed-off-by: Sunil Abraham <[email protected]>
Add more functionality in uart driver.
Implement interrupt API.

Signed-off-by: Sunil Abraham <[email protected]>
Copy link

sonarqubecloud bot commented Oct 7, 2025

@sunil-abraham
Copy link
Contributor Author

Could you amend the commit summary from ... uart g1: microchip:... to ... uart: microchip/g1: ... where the g1 part is associated with microchip?

Yes, I have updated the commit summary as per your suggestion.

@kartben kartben requested a review from Copilot October 8, 2025 07:56
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the Microchip UART G1 driver by implementing interrupt-driven UART functionality and expanding device tree support for SERCOM modules across different SoC variants.

  • Adds comprehensive interrupt-driven API implementation including TX/RX interrupt handling, FIFO operations, and error management
  • Introduces device tree bindings for SERCOM6 and SERCOM7 modules in both 'n' and 'p' SoC variants
  • Adds new run-in-standby-en property to control UART behavior in standby sleep mode

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
dts/bindings/serial/microchip,sercom-g1-uart.yaml Adds run-in-standby-en property for standby mode configuration
dts/arm/microchip/sam/sam_d5x_e5x/common/samd5xe5x_p.dtsi Defines SERCOM6/7 modules for 'p' variant SoCs
dts/arm/microchip/sam/sam_d5x_e5x/common/samd5xe5x_n.dtsi Defines SERCOM6/7 modules for 'n' variant SoCs
Multiple .dtsi files Include appropriate variant-specific SERCOM definitions
drivers/serial/uart_mchp_sercom_g1.c Implements interrupt-driven API and runtime configuration support
drivers/serial/Kconfig.mchp Enables interrupt support for the driver

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

int retval = UART_SUCCESS;

do {
uart_enable(regs, clock_external, false, false);
Copy link
Preview

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

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

The function call passes false for the run_in_standby parameter, but this should use the configuration value (cfg->run_in_standby_en == 1) ? true : false to maintain consistency with the initialization function.

Suggested change
uart_enable(regs, clock_external, false, false);
uart_enable(regs, clock_external, (cfg->run_in_standby_en == 1) ? true : false, false);

Copilot uses AI. Check for mistakes.

Comment on lines 575 to +576
* @param clock_external Boolean to check external or internal clock
* @param run_in_standby Boolean to check if run_in_standby is configured.
Copy link
Preview

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

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

Missing documentation for the new run_in_standby parameter in the function's docstring.

Suggested change
* @param clock_external Boolean to check external or internal clock
* @param run_in_standby Boolean to check if run_in_standby is configured.
* @param clock_external Boolean to check external or internal clock.
* @param run_in_standby Boolean to enable UART operation in standby mode.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards/SoCs area: Devicetree Bindings area: UART Universal Asynchronous Receiver-Transmitter platform: Microchip MEC Microchip MEC Platform platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants