nuttxpr commented on PR #16286: URL: https://github.com/apache/nuttx/pull/16286#issuecomment-2838047417
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. While it provides a basic summary and mentions testing, it lacks crucial details. Here's a breakdown of what's missing: * **Summary:** While it states the "what," it doesn't explain the "why" and "how" deeply enough. It needs to explain *why* the existing code didn't handle multiport ax99100 correctly and *how* this change fixes that. For example, what specific logic was added or modified in `uart_pci_16550.c`? * **Impact:** This section is far too brief. It needs to explicitly answer each question with "YES" or "NO" followed by details if "YES." For example: * **Is new feature added?** NO * **Is existing feature changed?** YES (Fixes multiport support for ax99100, enabling use of multiple serial ports simultaneously.) * **Impact on user...** Potentially YES if users were attempting to use multiple ports before and encountering issues. Describe how the user experience changes. * **Impact on build...** Likely NO, but confirm. * **Impact on hardware...** YES (Specifically affects systems using ax99100 PCI serial cards.) * **Impact on documentation...** Possibly YES if this behavior wasn't documented or was documented incorrectly. * **Impact on security...** NO (unless this fix addresses a security vulnerability). * **Impact on compatibility...** Possibly YES if the change affects interaction with other drivers or software. * **Testing:** While testing is mentioned, it's insufficiently described. * **Build Host(s):** Provide specific OS, CPU architecture, and compiler version. * **Target(s):** Specify the architecture (e.g., x86, ARM) and board configuration used for testing. * **Testing logs:** Include actual log output *before* and *after* the change to demonstrate the fix. Simply saying "tested" doesn't provide verifiable evidence of the change's effectiveness. **Example of an improved description:** ## Summary The existing driver for the ax99100 PCI serial card in `serial/uart_pci_16550.c` incorrectly handled multiport configurations. The ax99100 exposes each serial port as a separate PCI device function. The previous driver implementation did not enumerate these functions correctly, preventing the use of multiple serial ports on a single card. This change modifies the driver to properly enumerate and initialize each PCI function, enabling support for multiple serial ports simultaneously. Fixes [NuttX Issue #XXX](if applicable). ## Impact * Is new feature added? NO * Is existing feature changed? YES (Fixes multiport support for ax99100 serial cards, allowing users to utilize multiple ports simultaneously. Previously, only one port would function correctly.) * Impact on user? YES (Users with ax99100 cards can now use all available serial ports. No code changes are required for applications, but they may now function correctly where they previously failed.) * Impact on build? NO * Impact on hardware? YES (Specifically affects systems with ax99100 PCI serial cards. Other hardware is unaffected.) * Impact on documentation? YES (The documentation should be updated to reflect the corrected multiport support for the ax99100.) * Impact on security? NO * Impact on compatibility? NO (This change is backwards compatible and should not affect interoperability.) ## Testing I confirm that changes are verified on local setup and work as intended: * Build Host(s): Linux (Ubuntu 20.04), x86_64, GCC 9.4.0 * Target(s): QEMU (x86), sim:nsh config Testing logs before change: ``` (Shows only one port working / other ports unresponsive) nsh> echo test > /dev/ttyS1 nsh> cat /dev/ttyS2 (No output) ``` Testing logs after change: ``` (Shows both ports working) nsh> echo test > /dev/ttyS1 nsh> cat /dev/ttyS1 test nsh> echo test2 > /dev/ttyS2 nsh> cat /dev/ttyS2 test2 ``` -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
