Re: [PATCH 0/3] Fixed ability to enable console polling for certain BSPs

[Sebastian Huber]

On 26/03/2019 19:11, Joel Sherrill wrote:



On Tue, Mar 26, 2019 at 11:55 AM Sebastian Huber 
> wrote:


- Am 26. Mrz 2019 um 16:17 schrieb joel j...@rtems.org
:

> On Tue, Mar 26, 2019 at 9:36 AM Sebastian Huber <
> sebastian.hu...@embedded-brains.de
> wrote:
>
>> On 26/03/2019 14:56, Lou Woods wrote:
>> > From: Lou Woods 
>> >
>> > I discovered that I was not able to turn off interrupt-based
console
>> mode on the
>> > Xilinx Zynq BSP under Qemu via the configure command.  The
fix simply
>> exchanges
>> > #ifdef XXX_CONSOLE_USE_INTERRUPTS for
>> > #if XXX_CONSOLE_USE_INTERRUPTS for the BSP specific console code.
>>
>> You can disable it via the configure command line, just use
>>
>> XXX_CONSOLE_USE_INTERRUPTS=
>>
>> I have never seen setting it to nothing ever documented anywhere in
> the RTEMS pantheon of material. All examples I have ever seen set
> these to a value
>
>
>> These feature defines are not uniformly used, some use #if
other use
>> #ifdef.
>>
>
> Changing these ifdef CONSOLE_USE to #if ensures they work if the
value
> is set to nothing or 0. I don't see how this isn't an improvement.

I just wanted to point out that this patch changes something that
works into something else that works also. 



Works for more cases. The configure.ac  sets it 
to 1. Setting it to 0 is a logical choice looking at the configure.ac 
 for what to do.


If I didn't know XXX= results in undefined, I doubt that is common 
knowledge.


If you need to tinker with the BSP options as a normal user you already 
have a problem. How do you know that you can call


./c/src/lib/libbsp/arm/xilinx-zynq/configure --help

to get the available options? Only a hand full of BSPs have a 
documentation section in


https://docs.rtems.org/branches/master/user/bsps/index.html

Anyway, the patch set is fine, but it would be nice to think also about 
improving the documentation since this is the root cause for the problem 
from my point of view.



The change is somewhat arbitrary.  It should be accompanied with a
recommendation in the BSP developer documentation.  There should
be also a ticket for this to change everything from #ifdef XYZ to
#if XYZ, if we agree that this is actually better.  Only for BSP
options, for stuff which is available via , or does it
stop, when we change all the #if defined(RTEMS_SMP) to #if RTEMS_SMP?


Setting something to 0 and using ifdef do not evaluate to the same 
thing so for BSP options, I would tend to lean to #if since setting it 
to 0 and nothing are equivalent. Plus may of the BSP options do take 
values so this helps make them consistent.


For the RTEMS feature flags, there is no associated value and never 
has been. It is a simple defined=true, not defined=false. I know there 
is no functional difference, but I would say that those should stay to 
ifdef since that's the intent. They also are directly from configure 
arguments which are explicitly enable/disable so no oddity with user 
set values.


This means the user interface to set the defines determines if a feature 
define uses undefined/defined or a zero/non-zero value.


--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH 0/3] Fixed ability to enable console polling for certain BSPs

[Joel Sherrill]

On Tue, Mar 26, 2019 at 11:55 AM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> - Am 26. Mrz 2019 um 16:17 schrieb joel j...@rtems.org:
>
> > On Tue, Mar 26, 2019 at 9:36 AM Sebastian Huber <
> > sebastian.hu...@embedded-brains.de> wrote:
> >
> >> On 26/03/2019 14:56, Lou Woods wrote:
> >> > From: Lou Woods 
> >> >
> >> > I discovered that I was not able to turn off interrupt-based console
> >> mode on the
> >> > Xilinx Zynq BSP under Qemu via the configure command.  The fix simply
> >> exchanges
> >> > #ifdef XXX_CONSOLE_USE_INTERRUPTS for
> >> > #if XXX_CONSOLE_USE_INTERRUPTS for the BSP specific console code.
> >>
> >> You can disable it via the configure command line, just use
> >>
> >> XXX_CONSOLE_USE_INTERRUPTS=
> >>
> >> I have never seen setting it to nothing ever documented anywhere in
> > the RTEMS pantheon of material. All examples I have ever seen set
> > these to a value
> >
> >
> >> These feature defines are not uniformly used, some use #if other use
> >> #ifdef.
> >>
> >
> > Changing these ifdef CONSOLE_USE to #if ensures they work if the value
> > is set to nothing or 0. I don't see how this isn't an improvement.
>
> I just wanted to point out that this patch changes something that works
> into something else that works also.


Works for more cases. The configure.ac sets it to 1. Setting it to 0 is a
logical choice looking at the configure.ac for what to do.

If I didn't know XXX= results in undefined, I doubt that is common
knowledge.


> The change is somewhat arbitrary.  It should be accompanied with a
> recommendation in the BSP developer documentation.  There should be also a
> ticket for this to change everything from #ifdef XYZ to #if XYZ, if we
> agree that this is actually better.  Only for BSP options, for stuff which
> is available via , or does it stop, when we change all the #if
> defined(RTEMS_SMP) to #if RTEMS_SMP?
>

Setting something to 0 and using ifdef do not evaluate to the same thing so
for BSP options, I would tend to lean to #if since setting it to 0 and
nothing are equivalent. Plus may of the BSP options do take values so this
helps make them consistent.

For the RTEMS feature flags, there is no associated value and never has
been. It is a simple defined=true, not defined=false. I know there is no
functional difference, but I would say that those should stay to ifdef
since that's the intent. They also are directly from configure arguments
which are explicitly enable/disable so no oddity with user set values.

--joel

>
> For example BSP_FDT_IS_SUPPORTED is also used in libbsd:
>
> rtemsbsd/include/rtems/bsd/local/opt_platform.h
>
> Some BSPs use a define in  another uses a BSP option.
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH 0/3] Fixed ability to enable console polling for certain BSPs

[Sebastian Huber]

- Am 26. Mrz 2019 um 16:17 schrieb joel j...@rtems.org:

> On Tue, Mar 26, 2019 at 9:36 AM Sebastian Huber <
> sebastian.hu...@embedded-brains.de> wrote:
> 
>> On 26/03/2019 14:56, Lou Woods wrote:
>> > From: Lou Woods 
>> >
>> > I discovered that I was not able to turn off interrupt-based console
>> mode on the
>> > Xilinx Zynq BSP under Qemu via the configure command.  The fix simply
>> exchanges
>> > #ifdef XXX_CONSOLE_USE_INTERRUPTS for
>> > #if XXX_CONSOLE_USE_INTERRUPTS for the BSP specific console code.
>>
>> You can disable it via the configure command line, just use
>>
>> XXX_CONSOLE_USE_INTERRUPTS=
>>
>> I have never seen setting it to nothing ever documented anywhere in
> the RTEMS pantheon of material. All examples I have ever seen set
> these to a value
> 
> 
>> These feature defines are not uniformly used, some use #if other use
>> #ifdef.
>>
> 
> Changing these ifdef CONSOLE_USE to #if ensures they work if the value
> is set to nothing or 0. I don't see how this isn't an improvement.

I just wanted to point out that this patch changes something that works into 
something else that works also. The change is somewhat arbitrary.  It should be 
accompanied with a recommendation in the BSP developer documentation.  There 
should be also a ticket for this to change everything from #ifdef XYZ to #if 
XYZ, if we agree that this is actually better.  Only for BSP options, for stuff 
which is available via , or does it stop, when we change all the #if 
defined(RTEMS_SMP) to #if RTEMS_SMP?

For example BSP_FDT_IS_SUPPORTED is also used in libbsd:

rtemsbsd/include/rtems/bsd/local/opt_platform.h

Some BSPs use a define in  another uses a BSP option.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH 0/3] Fixed ability to enable console polling for certain BSPs

[Joel Sherrill]

On Tue, Mar 26, 2019 at 9:36 AM Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> On 26/03/2019 14:56, Lou Woods wrote:
> > From: Lou Woods 
> >
> > I discovered that I was not able to turn off interrupt-based console
> mode on the
> > Xilinx Zynq BSP under Qemu via the configure command.  The fix simply
> exchanges
> > #ifdef XXX_CONSOLE_USE_INTERRUPTS for
> > #if XXX_CONSOLE_USE_INTERRUPTS for the BSP specific console code.
>
> You can disable it via the configure command line, just use
>
> XXX_CONSOLE_USE_INTERRUPTS=
>
> I have never seen setting it to nothing ever documented anywhere in
the RTEMS pantheon of material. All examples I have ever seen set
these to a value


> These feature defines are not uniformly used, some use #if other use
> #ifdef.
>

Changing these ifdef CONSOLE_USE to #if ensures they work if the value
is set to nothing or 0. I don't see how this isn't an improvement.

--joel


> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax : +49 89 189 47 41-09
> E-Mail  : sebastian.hu...@embedded-brains.de
> PGP : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH 0/3] Fixed ability to enable console polling for certain BSPs

[Sebastian Huber]

On 26/03/2019 14:56, Lou Woods wrote:

From: Lou Woods 

I discovered that I was not able to turn off interrupt-based console mode on the
Xilinx Zynq BSP under Qemu via the configure command.  The fix simply exchanges
#ifdef XXX_CONSOLE_USE_INTERRUPTS for
#if XXX_CONSOLE_USE_INTERRUPTS for the BSP specific console code.


You can disable it via the configure command line, just use

XXX_CONSOLE_USE_INTERRUPTS=

These feature defines are not uniformly used, some use #if other use #ifdef.

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH 0/3] Fixed ability to enable console polling for certain BSPs

[Lou Woods]

From: Lou Woods 

I discovered that I was not able to turn off interrupt-based console mode on 
the 
Xilinx Zynq BSP under Qemu via the configure command.  The fix simply exchanges 
#ifdef XXX_CONSOLE_USE_INTERRUPTS for 
#if XXX_CONSOLE_USE_INTERRUPTS for the BSP specific console code.

The error pattern existed for to atsamv and imx7, so they were fixed as well.  
I've tested the fix on the Zynq Qemu BSP and compiled the other two BSPs.

Lou Woods (3):
  Fixed ability to enable polling on the console using configure for
Zynq
  Fixed ability to enable polling on the console using configure for
atsamv
  Fixed ability to enable polling on the console using configure for
imx7

 bsps/arm/atsam/console/console.c | 28 ++--
 bsps/arm/imx/console/console-config.c| 16 
 bsps/arm/xilinx-zynq/console/zynq-uart.c | 12 ++--
 3 files changed, 28 insertions(+), 28 deletions(-)

-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel