| Message ID | 1579227117-6310-1-git-send-email-aleksandar.markovic@rt-rk.com |
|---|---|
| Headers | show |
| Series | linux-user: Fix some issues in termbits.h files | expand |
Patchew URL: https://patchew.org/QEMU/1579227117-6310-1-git-send-email-aleksandar.markovic@rt-rk.com/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 1579227117-6310-1-git-send-email-aleksandar.markovic@rt-rk.com Type: series Subject: [PATCH 0/4] linux-user: Fix some issues in termbits.h files === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === From https://github.com/patchew-project/qemu * [new tag] patchew/1579227117-6310-1-git-send-email-aleksandar.markovic@rt-rk.com -> patchew/1579227117-6310-1-git-send-email-aleksandar.markovic@rt-rk.com Switched to a new branch 'test' 2a18394 linux-user: Fix some constants in remaining termbits.h 11acaf6 linux-user: xtensa: Fix some constants in termbits.h 5f64098 linux-user: mips: Synchronize termbits.h with kernel 0d5231a linux-user: alpha: Synchronize termbits.h with kernel === OUTPUT BEGIN === 1/4 Checking commit 0d5231a5c58f (linux-user: alpha: Synchronize termbits.h with kernel) 2/4 Checking commit 5f640980e8f3 (linux-user: mips: Synchronize termbits.h with kernel) ERROR: if this code is redundant consider removing it #83: FILE: linux-user/mips/termbits.h:60: +#if 0 total: 1 errors, 0 warnings, 187 lines checked Patch 2/4 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/4 Checking commit 11acaf6b0460 (linux-user: xtensa: Fix some constants in termbits.h) 4/4 Checking commit 2a18394b0d3c (linux-user: Fix some constants in remaining termbits.h) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/1579227117-6310-1-git-send-email-aleksandar.markovic@rt-rk.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
Le 17/01/2020 à 03:11, Aleksandar Markovic a écrit : > From: Aleksandar Markovic <amarkovic@wavecomp.com> > > This series is a spin-off of v5 of earlier series "linux-user: Misc > patches for 5.0", that became too large to manage. I will submit the > rest of that large series separately. > > Files linux-user/<target>/termbits.h seem to be in a very bad shape: > unsynchronized with kernel, containing wrong elements expressed in > terms of host instead of target, many being updated wrt kernel > content at various times, and on top of that all contain visually > very ugly combinations of tabs and spaces. > > This series attempts to fix great majority of issues in termbits. > > Alpha's and mips' termbits.h were in the worst shape, missing large > bits and pieces, and for them as complete as possible synchronization > with kernel code is done - this constitutes the first two patches. > > Xtensa's termbits.h contained the most elements wrongly expressed in > terms of host instead of target, and that is the reason the changes > in this file are placed in a separate, third, patch. Previous "R-B" > given by Max Filippov was transferred to this patch only. > > The fourth patch fixes remaining elements wrongly expressed in > terms of host instead of target. > > As an additional note, structures "serial_iso7816" and "serial_rs485" > (at times mentioned as the third argument of certain ioctls) are > platform-independant in kernel, and do not need "target_" variant > in QEMU. Also, structure "winsize" (also appearing as the third > ioctl's argument at times) is defined at multiple places in kernel > (for several architectures) in kernel, but all such definitions are > identical, and, therefore, it also does not need "target_" variant > in QEMU. > > A checkpatch warning related to "#if 0" in patch 2 is benign, and > should be ignored. > > Aleksandar Markovic (4): > linux-user: alpha: Synchronize termbits.h with kernel > linux-user: mips: Synchronize termbits.h with kernel > linux-user: xtensa: Fix some constants in termbits.h > linux-user: Fix some constants in remaining termbits.h > > linux-user/aarch64/termbits.h | 4 +- > linux-user/alpha/termbits.h | 82 ++++++++++++++-- > linux-user/arm/termbits.h | 4 +- > linux-user/cris/termbits.h | 4 +- > linux-user/hppa/termbits.h | 4 +- > linux-user/i386/termbits.h | 4 +- > linux-user/m68k/termbits.h | 4 +- > linux-user/microblaze/termbits.h | 4 +- > linux-user/mips/termbits.h | 140 ++++++++++++++++---------- > linux-user/nios2/termbits.h | 4 +- > linux-user/openrisc/termbits.h | 14 +-- > linux-user/ppc/termbits.h | 4 +- > linux-user/riscv/termbits.h | 4 +- > linux-user/s390x/termbits.h | 26 ++--- > linux-user/sh4/termbits.h | 4 +- > linux-user/sparc/termbits.h | 4 +- > linux-user/sparc64/termbits.h | 4 +- > linux-user/tilegx/termbits.h | 12 ++- > linux-user/x86_64/termbits.h | 26 +++-- > linux-user/xtensa/termbits.h | 207 +++++++++++++++++++++------------------ > 20 files changed, 353 insertions(+), 206 deletions(-) > I think we should first introduce a linux-user/generic/termbits.h as we have an asm-generic/termbits.h in the kernel and use it with all the targets except alpha, mips, hppa, sparc and xtensa. I think this linux-user/generic/termbits.h could be copied from linux-user/openrisc/termbits.h (without the ioctl definitions) Then you could update the remaining ones. Thanks, Laurent
> I think we should first introduce a linux-user/generic/termbits.h as we > have an asm-generic/termbits.h in the kernel and use it with all the > targets except alpha, mips, hppa, sparc and xtensa. > > I think this linux-user/generic/termbits.h could be copied from > linux-user/openrisc/termbits.h (without the ioctl definitions) > > Then you could update the remaining ones. > I agree with you, Laurent, that would be the cleanest implementation. However, I think it requires at least several days of meticulous dev work, that I can't afford at this moment. May I ask you to accept this series as is for 5.0, as a sort of bridge towards the implementation you described? It certainly fixes a majority of termbits-related bugs, many of them remained latent just by fact that XXX and TARGET_XXX were identical. The most affected targets, xtensa, mips and alpha should be cleaned up by this series wrt termbits, and for great majority of issues are cleaned up for all platforms. I just don't have enough time resources to additionally devote to this problem. Sincerely, Aleksandar > Thanks, > Laurent >
Le 19/03/2020 à 17:24, Aleksandar Markovic a écrit : >> I think we should first introduce a linux-user/generic/termbits.h as we >> have an asm-generic/termbits.h in the kernel and use it with all the >> targets except alpha, mips, hppa, sparc and xtensa. >> >> I think this linux-user/generic/termbits.h could be copied from >> linux-user/openrisc/termbits.h (without the ioctl definitions) >> >> Then you could update the remaining ones. >> > > I agree with you, Laurent, that would be the cleanest > implementation. > > However, I think it requires at least several days of meticulous > dev work, that I can't afford at this moment. May I ask you to > accept this series as is for 5.0, as a sort of bridge towards > the implementation you described? It certainly fixes a majority > of termbits-related bugs, many of them remained latent just > by fact that XXX and TARGET_XXX were identical. The most > affected targets, xtensa, mips and alpha should be cleaned up > by this series wrt termbits, and for great majority of issues > are cleaned up for all platforms. > > I just don't have enough time resources to additionally > devote to this problem. ok, but I need to review and test them, I don't know if I will have enough time for that. I will try... Thanks, Laurent
On Thursday, March 19, 2020, Laurent Vivier <laurent@vivier.eu> wrote: > Le 19/03/2020 à 17:24, Aleksandar Markovic a écrit : > >> I think we should first introduce a linux-user/generic/termbits.h as we > >> have an asm-generic/termbits.h in the kernel and use it with all the > >> targets except alpha, mips, hppa, sparc and xtensa. > >> > >> I think this linux-user/generic/termbits.h could be copied from > >> linux-user/openrisc/termbits.h (without the ioctl definitions) > >> > >> Then you could update the remaining ones. > >> > > > > I agree with you, Laurent, that would be the cleanest > > implementation. > > > > However, I think it requires at least several days of meticulous > > dev work, that I can't afford at this moment. May I ask you to > > accept this series as is for 5.0, as a sort of bridge towards > > the implementation you described? It certainly fixes a majority > > of termbits-related bugs, many of them remained latent just > > by fact that XXX and TARGET_XXX were identical. The most > > affected targets, xtensa, mips and alpha should be cleaned up > > by this series wrt termbits, and for great majority of issues > > are cleaned up for all platforms. > > > > I just don't have enough time resources to additionally > > devote to this problem. > > ok, but I need to review and test them, I don't know if I will have > enough time for that. I will try... > > OK, thanks! Aleksandar > Thanks, > Laurent >
From: Aleksandar Markovic <amarkovic@wavecomp.com> This series is a spin-off of v5 of earlier series "linux-user: Misc patches for 5.0", that became too large to manage. I will submit the rest of that large series separately. Files linux-user/<target>/termbits.h seem to be in a very bad shape: unsynchronized with kernel, containing wrong elements expressed in terms of host instead of target, many being updated wrt kernel content at various times, and on top of that all contain visually very ugly combinations of tabs and spaces. This series attempts to fix great majority of issues in termbits. Alpha's and mips' termbits.h were in the worst shape, missing large bits and pieces, and for them as complete as possible synchronization with kernel code is done - this constitutes the first two patches. Xtensa's termbits.h contained the most elements wrongly expressed in terms of host instead of target, and that is the reason the changes in this file are placed in a separate, third, patch. Previous "R-B" given by Max Filippov was transferred to this patch only. The fourth patch fixes remaining elements wrongly expressed in terms of host instead of target. As an additional note, structures "serial_iso7816" and "serial_rs485" (at times mentioned as the third argument of certain ioctls) are platform-independant in kernel, and do not need "target_" variant in QEMU. Also, structure "winsize" (also appearing as the third ioctl's argument at times) is defined at multiple places in kernel (for several architectures) in kernel, but all such definitions are identical, and, therefore, it also does not need "target_" variant in QEMU. A checkpatch warning related to "#if 0" in patch 2 is benign, and should be ignored. Aleksandar Markovic (4): linux-user: alpha: Synchronize termbits.h with kernel linux-user: mips: Synchronize termbits.h with kernel linux-user: xtensa: Fix some constants in termbits.h linux-user: Fix some constants in remaining termbits.h linux-user/aarch64/termbits.h | 4 +- linux-user/alpha/termbits.h | 82 ++++++++++++++-- linux-user/arm/termbits.h | 4 +- linux-user/cris/termbits.h | 4 +- linux-user/hppa/termbits.h | 4 +- linux-user/i386/termbits.h | 4 +- linux-user/m68k/termbits.h | 4 +- linux-user/microblaze/termbits.h | 4 +- linux-user/mips/termbits.h | 140 ++++++++++++++++---------- linux-user/nios2/termbits.h | 4 +- linux-user/openrisc/termbits.h | 14 +-- linux-user/ppc/termbits.h | 4 +- linux-user/riscv/termbits.h | 4 +- linux-user/s390x/termbits.h | 26 ++--- linux-user/sh4/termbits.h | 4 +- linux-user/sparc/termbits.h | 4 +- linux-user/sparc64/termbits.h | 4 +- linux-user/tilegx/termbits.h | 12 ++- linux-user/x86_64/termbits.h | 26 +++-- linux-user/xtensa/termbits.h | 207 +++++++++++++++++++++------------------ 20 files changed, 353 insertions(+), 206 deletions(-)