[PATCH] bsp/raspberrypi: Updated the console API.

[Christian Mauderer]

On 03/01/2020 16:54, Vijay Kumar Banerjee wrote:
> 
> 
> On Fri, Jan 3, 2020, 8:53 PM Niteesh <gsnb.gn at gmail.com
> <mailto:gsnb.gn at gmail.com>> wrote:
> 
>     On Fri, Jan 3, 2020 at 7:32 PM Vijay Kumar Banerjee
>     <vijaykumar9597 at gmail.com <mailto:vijaykumar9597 at gmail.com>> wrote:
> 
>         Hi Niteesh,
>         A few formatting suggestions:
> 
> 
>             diff --git a/bsps/arm/raspberrypi/console/console-config.c
>             b/bsps/arm/raspberrypi/console/console-config.c
>             index d2186c918b..4000fd1c50 100644
>             --- a/bsps/arm/raspberrypi/console/console-config.c
>             +++ b/bsps/arm/raspberrypi/console/console-config.c
>             @@ -19,50 +19,132 @@
>               */
> 
>              #include <rtems/bspIo.h>
>             -
>             -#include <libchip/serial.h>
> 
>         You removed it here and added it below
> 
>             +#include <rtems/console.h>
>             +#include <rtems/sysinit.h>
> 
>              #include <bspopts.h>
>             -#include <bsp/irq.h>
>             -#include <bsp/usart.h>
> 
>         same here 
> 
>             +#include <bsp.h>
>              #include <bsp/raspberrypi.h>
>             +#include <bsp/usart.h>
> 
>         added here ^ 
> 
>             +#include <bsp/arm-pl011.h>
>              #include <bsp/fbcons.h>
>             +#include <bsp/console-termios.h>
>             +#include <bsp/fdt.h>
>             +#include <bsp/fatal.h>
>             +
>             +#include <libfdt.h>
>             +#include <libchip/serial.h> 
> 
>         here
> 
>     Is there any particular reason for doing it, or is it just a preference?
> 
> For RTEMS, I don't see any preference mentioned in the docs for the
> order or includes:
> https://docs.rtems.org/branches/master/eng/coding-conventions.html
> 
> In libbsd, however, the FreeBSD style guide is followed which has a
> preference for the order of header includes:
> https://www.freebsd.org/cgi/man.cgi?query=style&apropos=0&sektion=9
> 
> Apart from that, this is a redundant change. Is there a reason to change
> the order?

I agree that it is an unnecessary change. I missed it during review.
Such changes should be avoided to keep the history clean and make it
simpler to see what really changed. Maybe you can revert the lines? The
rest of the patch is still OK.

> 
>         This is just a small change and I would suggest you wait for at
>         least a day
>         before sending a v2 of the patch so that you can add changes
>         suggested
>         by other people (if any). Congrats on getting it working ;-)
> 
>     Thank you. 
> 
>         Best,
>         Vijay 
> 
>             +
>             +#define UART0     "/dev/ttyS0"
>             +#define FBCONS    "/dev/fbcons"
>             +
> 
> 
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>