Re: [PATCH 4/5] tools/nolibc: add csky support

[Willy Tarreau <w@1wt.eu>]

On Mon, Sep 30, 2024 at 07:23:39AM +0200, Thomas Weißschuh wrote:
> > > [0] gcc 13.2.0 and 14.2.0 from kernel.org crosstools
> > > [1] https://github.com/XUANTIE-RV/qemu/
> > > [2]
> > 
> > I think you wanted to place a link or something above for [2].
> 
> [2] was supposed to be inline patches for QEMU, I'll try to make that a
> bit clearer.
> 
> > > diff --git a/target/csky/cpu-param.h b/target/csky/cpu-param.h
> > > index 80554cc0fc03..9181b602a26f 100644
> > > --- a/target/csky/cpu-param.h
> > > +++ b/target/csky/cpu-param.h
> > (...)
> > > diff --git a/target/csky/op_vdsp2.c b/target/csky/op_vdsp2.c
> > > index a9985a03be33..d953f5ea94fe 100644
> > > --- a/target/csky/op_vdsp2.c
> > > +++ b/target/csky/op_vdsp2.c
> > 
> > Also, the first two patches look like fixes for the arch itself, they
> > should really go outside of the nolibc development tree, at least
> > because they might have to be backported to some stable branches,
> > or later fixed/reverted in case they wouldn't be optimal.
> 
> As mentioned above, these are patches for qemu, not Linux.
> I don't know enough about QEMU or C-SKY to know if these are the
> generally correct fixes. But they seem to work well enough for nolibc.

Ah I understand now. The problem with external patches inlined like this
is that it's hard to split them apart from the rest of the patch. For
example just doing patch -p1 < patch.mbox will fail, trying to patch
non-existing files.

What I'm used to doing when quoting code/patches/etc in messages is to
indent them by 2 or more chars. That could be sufficient to explain what
needs to be fixed in the upstream project without being taken for a part
of the patchset, especially for such tiny patches. In this case the quoted
part could include only the strict minimum (i.e. no diff --git header etc).
Just a suggestion.

Cheers,
Willy