Re: Introduction + incoming RISC-V port(s)

[Robert Lipe]

On Mon, Jun 14, 2021 at 1:02 PM Alan Carvalho de Assis 
wrote:

> Normally only common code on NuttX needs to follow the C89 standard
>

The presubmit flips out about C99 comments. Even that's too radical. So
much for being king. :-)


> Also should use tools/checkpatch.sh to check your patch/files before
> submitting the PR.
>

Yes, I should have. Presubmit kicked it back.

I'm guessing with a name like "inviolables", I'm going to change nothing by
saying it, but
this whitespace convention is totally alien to me. There is just So Much
Whitespace.
Is there a clang format or something that implements this?

> There's a LOT of duplication within arch/risc-v and where there is

See my recent PR for an example of just how much redundant, copy-paste,
machine-edit is really necessary for a new port.


> And even a draft PR:
> https://github.com/apache/incubator-nuttx/pull/2822

That's a PR that seems to have collapsed under its own weight. What needs
to happen to get that going? Should I separate the "delete files" from
"edit files" and resubmit that as two different, hopefully more reviewable
PRs?

That doesn't even really scratch the surface of the problem, though. Look
at how many copies of everything exists within the various risc-v
directories. You can't even really tell what's the same because all the
#defines have the target name in them, though.

If we climb ot of the the risc-v trees, it looks like we have dozens of
different 16550 implementations, each that was created as a copy + machine
edit.

This doesn't seem to trouble anyone else, so maybe I should let that go,
too.

Re: device tree, there's a PR that's been pending for a year almost to the
day. That also seems to have coasted to a halt without a clear rejection or
path to acceptance. Working Device Tree would have made most of the BeagleV
- and the upcoming D1 - ports almost trivial.

I'm probably rocking too many boats at once. Maybe I should tuck my head
down and go with this project's One True Ways.

Thanx,
RJL

Re: Introduction + incoming RISC-V port(s)

[Alan Carvalho de Assis]

Hi Robert,

On 6/22/21, Robert Lipe  wrote:
> On Mon, Jun 14, 2021 at 1:02 PM Alan Carvalho de Assis 
> wrote:
>
>> Normally only common code on NuttX needs to follow the C89 standard
>>
>
> The presubmit flips out about C99 comments. Even that's too radical. So
> much for being king. :-)
>

You can use C99, but you cannot forgot to follow the Coding Style, //
comments shouldn't be used :-)

>
>> Also should use tools/checkpatch.sh to check your patch/files before
>> submitting the PR.
>>
>
> Yes, I should have. Presubmit kicked it back.
>

You don't need to submit a new PR, you can fix the current one and
push force it.

> I'm guessing with a name like "inviolables", I'm going to change nothing by
> saying it, but
> this whitespace convention is totally alien to me. There is just So Much
> Whitespace.

Yes, but it makes the code more pleasant to read and easier to stop issues.

> Is there a clang format or something that implements this?
>

Not yet, some people already tried it, but the NuttX Coding Style is
not easy to fulfill, its needs someone with deep knowledge about
formatting tool to get it done correctly. Until now everybody who
tried failed miserably.

>> There's a LOT of duplication within arch/risc-v and where there is
>
> See my recent PR for an example of just how much redundant, copy-paste,
> machine-edit is really necessary for a new port.
>

Well, if you spot codes that should be removed you can move it do
/common, but you need to adapt other boards using it.

>
>> And even a draft PR:
>> https://github.com/apache/incubator-nuttx/pull/2822
>
> That's a PR that seems to have collapsed under its own weight. What needs
> to happen to get that going? Should I separate the "delete files" from
> "edit files" and resubmit that as two different, hopefully more reviewable
> PRs?
>

I think you don't need to care about it now, just submit your work and
someone taking care of PR #2822 will need to take care of you board
files later.

> That doesn't even really scratch the surface of the problem, though. Look
> at how many copies of everything exists within the various risc-v
> directories. You can't even really tell what's the same because all the
> #defines have the target name in them, though.
>

Ok, but an issue by time. First is your PR #3965 :-)

> If we climb ot of the the risc-v trees, it looks like we have dozens of
> different 16550 implementations, each that was created as a copy + machine
> edit.
>

Well, if they are common code defined in the specification, then it
should be moved to common code.
If it is similar peripheral drivers, sometime duplication could be a
good thing, it will avoid hundreds of #ifdef.
See the arch/stm32 people tried to put all together, but ST already
make some small modifications in a family of other that breaks other
devices.
After separating stm32l4, f7, etc, brings are easy to fix, but it
generates the duplication issue, sometimes a fix needs to be applied
to all arch/stm32yxx.

> This doesn't seem to trouble anyone else, so maybe I should let that go,
> too.
>

For sure! Unless you want to suffer a lot of pain to fix everything ;-)

> Re: device tree, there's a PR that's been pending for a year almost to the
> day. That also seems to have coasted to a halt without a clear rejection or
> path to acceptance. Working Device Tree would have made most of the BeagleV
> - and the upcoming D1 - ports almost trivial.
>
> I'm probably rocking too many boats at once. Maybe I should tuck my head
> down and go with this project's One True Ways.
>

Exactly, the bast way to each an elephant is a bit by time! :-)

BR,

Alan

Re: Introduction + incoming RISC-V port(s)

[Alan Carvalho de Assis]

s/forgot/forget/

On 6/22/21, Alan Carvalho de Assis  wrote:
> Hi Robert,
>
> On 6/22/21, Robert Lipe  wrote:
>> On Mon, Jun 14, 2021 at 1:02 PM Alan Carvalho de Assis
>> 
>> wrote:
>>
>>> Normally only common code on NuttX needs to follow the C89 standard
>>>
>>
>> The presubmit flips out about C99 comments. Even that's too radical. So
>> much for being king. :-)
>>
>
> You can use C99, but you cannot forgot to follow the Coding Style, //
> comments shouldn't be used :-)
>
>>
>>> Also should use tools/checkpatch.sh to check your patch/files before
>>> submitting the PR.
>>>
>>
>> Yes, I should have. Presubmit kicked it back.
>>
>
> You don't need to submit a new PR, you can fix the current one and
> push force it.
>
>> I'm guessing with a name like "inviolables", I'm going to change nothing
>> by
>> saying it, but
>> this whitespace convention is totally alien to me. There is just So Much
>> Whitespace.
>
> Yes, but it makes the code more pleasant to read and easier to stop issues.
>
>> Is there a clang format or something that implements this?
>>
>
> Not yet, some people already tried it, but the NuttX Coding Style is
> not easy to fulfill, its needs someone with deep knowledge about
> formatting tool to get it done correctly. Until now everybody who
> tried failed miserably.
>
>>> There's a LOT of duplication within arch/risc-v and where there is
>>
>> See my recent PR for an example of just how much redundant, copy-paste,
>> machine-edit is really necessary for a new port.
>>
>
> Well, if you spot codes that should be removed you can move it do
> /common, but you need to adapt other boards using it.
>
>>
>>> And even a draft PR:
>>> https://github.com/apache/incubator-nuttx/pull/2822
>>
>> That's a PR that seems to have collapsed under its own weight. What needs
>> to happen to get that going? Should I separate the "delete files" from
>> "edit files" and resubmit that as two different, hopefully more
>> reviewable
>> PRs?
>>
>
> I think you don't need to care about it now, just submit your work and
> someone taking care of PR #2822 will need to take care of you board
> files later.
>
>> That doesn't even really scratch the surface of the problem, though. Look
>> at how many copies of everything exists within the various risc-v
>> directories. You can't even really tell what's the same because all the
>> #defines have the target name in them, though.
>>
>
> Ok, but an issue by time. First is your PR #3965 :-)
>
>> If we climb ot of the the risc-v trees, it looks like we have dozens of
>> different 16550 implementations, each that was created as a copy +
>> machine
>> edit.
>>
>
> Well, if they are common code defined in the specification, then it
> should be moved to common code.
> If it is similar peripheral drivers, sometime duplication could be a
> good thing, it will avoid hundreds of #ifdef.
> See the arch/stm32 people tried to put all together, but ST already
> make some small modifications in a family of other that breaks other
> devices.
> After separating stm32l4, f7, etc, brings are easy to fix, but it
> generates the duplication issue, sometimes a fix needs to be applied
> to all arch/stm32yxx.
>
>> This doesn't seem to trouble anyone else, so maybe I should let that go,
>> too.
>>
>
> For sure! Unless you want to suffer a lot of pain to fix everything ;-)
>
>> Re: device tree, there's a PR that's been pending for a year almost to
>> the
>> day. That also seems to have coasted to a halt without a clear rejection
>> or
>> path to acceptance. Working Device Tree would have made most of the
>> BeagleV
>> - and the upcoming D1 - ports almost trivial.
>>
>> I'm probably rocking too many boats at once. Maybe I should tuck my head
>> down and go with this project's One True Ways.
>>
>
> Exactly, the bast way to each an elephant is a bit by time! :-)
>
> BR,
>
> Alan
>

Re: Introduction + incoming RISC-V port(s)

[Robert Lipe]

>
>
> > The presubmit flips out about C99 comments. Even that's too radical. So
> > much for being king. :-)
>
> You can use C99, but you cannot forgot to follow the Coding Style, //
> comments shouldn't be used :-)
>

It's actually worse than that. You can use C99 comments at the end of a line
but not at the beginning

frobnicate();  // this is accepted
// this is an error

This is perfectly legal C99. That's 20+ years ago and was widely accepted
before then. I'd venture that the number of significant (not a college
kid's
project) RISC-V compilers that don't accept C99 is zero.

I see // comments in 48 *.[ch] files. That ship has sailed.


>> Also should use tools/checkpatch.sh to check your patch/files before
> >> submitting the PR.
> >
> > Yes, I should have. Presubmit kicked it back.
> >
>
> You don't need to submit a new PR, you can fix the current one and
> push force it.
>

Parlance breakdown, but, yes. I've iterated on that same PR. I found that
the build is assuming that a specific -Wbarf flag is implied by -Wall, when
in the official RISC-V GCC builds, it's not, so I never saw the
warning/error.
In my own project, I've found -Wall almost useless because it varies from
version to version. Better to turn on the -W flags you actually care about
so
they're version controlled.

So I've kicked that back in, but it appears I have to wait for a (gasp)
human
to push a button. I assume this is to stop script kiddies from submitting
bitcoin
harvesters in the presubmit, but it really hoses a workflow as it's like
submitting
a box of punched cards across the table into the Holy Room where computing
is done and then awaiting a stack of greenbar with the results later in the
day.

Hopefully, you're convinced I'm an actual human at this point (even if I
complain
too much :-) so can someone please clear my bozo bit so I wrap these up and
carry on? I don't want to be another patch sitting in a year-long review
queue.


> Is there a clang format or something that implements this?
>
> Not yet, some people already tried it, but the NuttX Coding Style is
> not easy to fulfill, its needs someone with deep knowledge about
> formatting tool to get it done correctly. Until now everybody who
> tried failed miserably.
>

This should set off alarms. If you can't teach the leading code
parser/formatters
the convention, it should be a hint that the convention is weird. (Sorry.)
I sure can't
find an easy way to set editor preferences in Sublime, vi, or the like so
I'm back to
counting spaces by hand.

I won't keep grinding my bootheel on this, but I'll say it out loud: it's
really off putting
for contributors. Probably like most pros, I can look past Google style vs.
LLVM vs
Chrome or whatever because I've seen (and written) mountains of code in it,
but
the mental burden is pushed to the tools. Here, you're just adding a
barrier to entry.

In my own project, I had a pet style that I tried to enforce. I found it
solve a lot of
problems to just reformat the whole thing one day, declare clang-format
style=google
to be the one way, and never worry about whether a curly is a trailer or a
header
again. Leave it to the tools and let developers focus on developing instead
of counting
spaces and stars in a formatted box.

>> There's a LOT of duplication within arch/risc-v and where there is
> >
> > See my recent PR for an example of just how much redundant, copy-paste,
>

It looks like adding a new board (that's trivially different from other
boards) is 55+
files added or created. There are also files marked "don't edit" that you
have
to edit.

Again, it's a barrier to entry.

On the plus side, while I'm not a fan of the kconfig system, the actual
build is nice
and speedy once the dependencies are generated. They parallelize nicely and
the
build mostly stays out of the way. That's pretty sweet.

Setting an fswatch on the generated image and restarting the emulator to
reboot
makes it all really seamless.

>> https://github.com/apache/incubator-nuttx/pull/2822
> I think you don't need to care about it now, just submit your work and
>

OK, I'll just keep adding to the pileup.

Ok, but an issue by time. First is your PR #3965 :-)
>

Yep. Once that's landed, I'll accept nominations/guidance on where to go
next.
This board is beefier than most of the targets in Nuttx. It has a network
interface
and USB host and the ability to do virtual memory and so on. I can't find
other
ports enabling RV39 so that may just be out of scope for Nuttx. I know IRQs
need work and it needs a real GPIO system. (I have boards
 arriving next week
that'll help with that once I get them soldered up.)

I have it passing the parallel prime number test.  Enabling SMP would
probably
be reasonable. (It'd be easier with Device Tree, but that doesn't seem
within
easy reach right now.)

Do you have a handy way to bootstrap filesystem and storage? I don't really
want
to be flashing SD cards and rebooting after every build. What'