xiaoxiang781216 commented on code in PR #18837:
URL: https://github.com/apache/nuttx/pull/18837#discussion_r3567621758
##########
drivers/serial/uart_pl011.c:
##########
@@ -1010,67 +724,42 @@ static int pl011_setup(FAR struct uart_dev_s *dev)
***************************************************************************/
/***************************************************************************
- * Name: pl011_earlyserialinit
+ * Name: pl011_dev_init
*
* Description:
- * see nuttx/serial/uart_pl011.h
+ * Initialize a PL011 device with its configuration settings.
*
- ***************************************************************************/
-
-void pl011_earlyserialinit(void)
-{
- /* Enable the console UART. The other UARTs will be initialized if and
- * when they are first opened.
- */
-#ifdef CONSOLE_DEV
- CONSOLE_DEV.isconsole = true;
- pl011_setup(&CONSOLE_DEV);
-#endif
-}
-
-/***************************************************************************
- * Name: pl011_serialinit
+ * Arguments:
+ * - dev The PL011 device struct to initialize
*
- * Description:
- * Register serial console and serial ports. This assumes that
- * pl011_earlyserialinit was called previously.
+ * Return:
+ * The `struct uart_dev_s` for registering this PL011 device, or NULL on
+ * failure.
*
***************************************************************************/
-void pl011_serialinit(void)
Review Comment:
> But defining in defconfig causes problems for some targets. The RPi4B has
5 interfaces, which is beyond what is defined in Kconfig.
we can add a new set of Kconfig.
> It also doesn't have an IRQ for each interface, it shares one single IRQ.
So to reuse the PL011 driver code, it needs some access to the internals so it
can register for multiple interfaces without modifying the PL011 Kconfig as a
bandaid solution (my next PR will handle the multiple vs single IRQ issue).
>
For the special case, let add a new struct/function, so the chip/board could
do some customization. But please keep the origin usage.
> I see it as we're choosing between this definition of PL011 interfaces in
code which requires similar definitions for each interface, or continually
patching Kconfig options to modify the driver options to fit with every target.
I think duplication in C is more expressive and worth the minimal duplication.
I understand why you don't like the duplication, but if we keep the Kconfig
option there will then be two different ways of handling PL011 interfaces which
I think is more confusing/unclean.
>
> Any thoughts?
let's keep the old method and your new method:
Make simple things simple, and complex things possible.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
