From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 20EA8C4CECE for ; Mon, 14 Oct 2019 08:11:11 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CE1A72054F for ; Mon, 14 Oct 2019 08:11:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="i3v6nwnk" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CE1A72054F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:45526 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iJvRd-0005Th-HG for qemu-devel@archiver.kernel.org; Mon, 14 Oct 2019 04:11:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51367) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iJvQd-0004xh-4z for qemu-devel@nongnu.org; Mon, 14 Oct 2019 04:10:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iJvQa-0003tY-TX for qemu-devel@nongnu.org; Mon, 14 Oct 2019 04:10:06 -0400 Received: from mail-oi1-x243.google.com ([2607:f8b0:4864:20::243]:43705) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iJvQa-0003rt-NR for qemu-devel@nongnu.org; Mon, 14 Oct 2019 04:10:04 -0400 Received: by mail-oi1-x243.google.com with SMTP id t84so13020412oih.10 for ; Mon, 14 Oct 2019 01:10:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=UWhMEtb8QXln0RTdqREfA2kOqvTC70sU7b8OgzgNbYo=; b=i3v6nwnkBxR+Ex2f8ff3wbBiB/E1JO2LPwMJbdN2x4kqw51TyCwBn6r+3jzC6Sy4gT POEo6DYzgplCDDlx8qcdoY+P2otcBsDNOk/PKMwrZgZ+kAYQtVjR1qKoCCzcjEWUN4Wg QQZPtLONA2mzU+uSOjkm633hT+lgsQdvxKulDoWRLVVzyRTrrOAjEkoGBqOUrci0YMS/ XFRXgpbqI50JR8N9b9xVEkILXMe0PJkVUhaKiz+v1JbxZA67zmct0E8OVXVy6xVbt7ez urw1a0JReFPHJwno2lf7tbkzew7mefTfk+3BgWDJM6i3p0Y5LFBsqpFPnNviiZ1IivMu 0zWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=UWhMEtb8QXln0RTdqREfA2kOqvTC70sU7b8OgzgNbYo=; b=DUuKyN6IrU54l2BCGGkgTtmZzEQBGrsJCt8ElxMjlWK8g4jniLnkAyfQAQbgE88t5W 5fe/aHcVZJ+eZQQFfxKITYcpJ0hbWyoXcshD/pF4PZD5WrNU9ticQXXTw3siuR75K3XA 8Ao0D1uwSs8ililvf65RrBDPuxXd8LQc/C/GzxwvXvRE+RsNlQnPKBu/kXSX7Ud5Gp88 tEu5p6lLCvufKcbbkmutVTQc8yUU2XoAK2uHysYBV/PpCzgEJ0abTZN5z7slgMTSSELd 2R9T+oCNtCtw+3zYmcEMsqFSR9XM8sI+MvZcRXaEXxxVJ1w1l+kHv5WUTDYKwVyw25fg TajA== X-Gm-Message-State: APjAAAWbwoEV65vrinqVZ/ZUhIPNhLiurJRAlQjmF3zk4m1fpreGxZts jxdx4+b5MrAxdv5CxjSf35d4AmCLCvZUh8zKbwQ= X-Google-Smtp-Source: APXvYqw1aq9J1jBc5cHWy/SqGgf7yiYlwSEqzlEJkvWQ8TpQvU6v5tkZE4vgmiiRs6SsCYakDu5xlL54VFf23fPT96E= X-Received: by 2002:a54:460c:: with SMTP id p12mr23695625oip.62.1571040602353; Mon, 14 Oct 2019 01:10:02 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a9d:340a:0:0:0:0:0 with HTTP; Mon, 14 Oct 2019 01:10:01 -0700 (PDT) In-Reply-To: <87a7a36awg.fsf@dusky.pond.sub.org> References: <1570991178-5511-1-git-send-email-aleksandar.markovic@rt-rk.com> <1570991178-5511-2-git-send-email-aleksandar.markovic@rt-rk.com> <87a7a36awg.fsf@dusky.pond.sub.org> From: Aleksandar Markovic Date: Mon, 14 Oct 2019 10:10:01 +0200 Message-ID: Subject: Re: [PATCH v4 1/8] target/mips: Clean up helper.c To: Markus Armbruster Content-Type: multipart/alternative; boundary="00000000000007f6800594da656e" X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::243 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Aleksandar Markovic , "aleksandar.rikalo@rt-rk.com" , "qemu-devel@nongnu.org" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000007f6800594da656e Content-Type: text/plain; charset="UTF-8" On Monday, October 14, 2019, Markus Armbruster wrote: > Aleksandar Markovic writes: > > > From: Aleksandar Markovic > > > > Mostly fix errors and warnings reported by 'checkpatch.pl -f'. > > > > Signed-off-by: Aleksandar Markovic > > --- > > target/mips/helper.c | 128 ++++++++++++++++++++++++++++++ > +-------------------- > > 1 file changed, 78 insertions(+), 50 deletions(-) > > > > diff --git a/target/mips/helper.c b/target/mips/helper.c > > index a2b6459..2411a2c 100644 > > --- a/target/mips/helper.c > > +++ b/target/mips/helper.c > > @@ -39,8 +39,8 @@ enum { > > #if !defined(CONFIG_USER_ONLY) > > > > /* no MMU emulation */ > > -int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > > - target_ulong address, int rw, int access_type) > > +int no_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > > + target_ulong address, int rw, int access_type) > > { > > *physical = address; > > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > > @@ -48,26 +48,28 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr > *physical, int *prot, > > } > > > > /* fixed mapping MMU emulation */ > > -int fixed_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int > *prot, > > - target_ulong address, int rw, int > access_type) > > +int fixed_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int > *prot, > > + target_ulong address, int rw, int access_type) > > { > > if (address <= (int32_t)0x7FFFFFFFUL) { > > - if (!(env->CP0_Status & (1 << CP0St_ERL))) > > + if (!(env->CP0_Status & (1 << CP0St_ERL))) { > > *physical = address + 0x40000000UL; > > - else > > + } else { > > *physical = address; > > - } else if (address <= (int32_t)0xBFFFFFFFUL) > > + } > > + } else if (address <= (int32_t)0xBFFFFFFFUL) { > > *physical = address & 0x1FFFFFFF; > > - else > > + } else { > > *physical = address; > > + } > > > > *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; > > return TLBRET_MATCH; > > } > > > > /* MIPS32/MIPS64 R4000-style MMU emulation */ > > -int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, > > - target_ulong address, int rw, int access_type) > > +int r4k_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > > + target_ulong address, int rw, int access_type) > > { > > uint16_t ASID = env->CP0_EntryHi & env->CP0_EntryHi_ASID_mask; > > int i; > > @@ -99,8 +101,9 @@ int r4k_map_address (CPUMIPSState *env, hwaddr > *physical, int *prot, > > if (rw != MMU_DATA_STORE || (n ? tlb->D1 : tlb->D0)) { > > *physical = tlb->PFN[n] | (address & (mask >> 1)); > > *prot = PAGE_READ; > > - if (n ? tlb->D1 : tlb->D0) > > + if (n ? tlb->D1 : tlb->D0) { > > *prot |= PAGE_WRITE; > > + } > > if (!(n ? tlb->XI1 : tlb->XI0)) { > > *prot |= PAGE_EXEC; > > } > > @@ -130,8 +133,11 @@ static int is_seg_am_mapped(unsigned int am, bool > eu, int mmu_idx) > > int32_t adetlb_mask; > > > > switch (mmu_idx) { > > - case 3 /* ERL */: > > - /* If EU is set, always unmapped */ > > + case 3: > > + /* > > + * ERL > > + * If EU is set, always unmapped > > + */ > > if (eu) { > > return 0; > > } > > This changes from the usual way we format switch case comments to an > unusual way. > > If you want to pursue this change, please put it in a separate patch, > so this one is really about fixing "errors and warnings reported by > 'checkpatch.pl -f'", as your commit message promises. > > Hi, Markus. Thank you for your response. There must be some misunderstanding here: The line: case 3 /* ERL */: generates a checkpatch warning. I don't know why I would put it in a separate patch, if this patch is about fixing checkpatch warnings. Please explain. Secondly, I don't see that this is a usual way we format switch statement. I found just several cases in the whole QEMU code base (and you claimed in previous comments that there are thousands). I am just guessing that you somehow mixed this line with the line: case 3: /* ERL */ that would have not generated checkpatch warning. I don't see any reason to change this patch. Please let me know it you still think I should do something else. And you are welcome to analyse any patches of mine. Aleksandar > [...] > > --00000000000007f6800594da656e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

On Monday, October 14, 2019, Markus Armbruster <armbru@redhat.com> wrote:
Aleksandar Markovic <aleksandar.markovic@rt-rk.com> writes:

> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>
> Mostly fix errors and warnings reported by 'checkpatch.pl -f'.
>
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>=C2=A0 target/mips/helper.c | 128 +++++++++++++++++++++++++++++++-= -------------------
>=C2=A0 1 file changed, 78 insertions(+), 50 deletions(-)
>
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index a2b6459..2411a2c 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -39,8 +39,8 @@ enum {
>=C2=A0 #if !defined(CONFIG_USER_ONLY)
>=C2=A0
>=C2=A0 /* no MMU emulation */
> -int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *pro= t,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 target_ulong address, int rw, int access_type)
> +int no_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int = *prot,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0target_ulong address, int rw, int access_type)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 *physical =3D address;
>=C2=A0 =C2=A0 =C2=A0 *prot =3D PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> @@ -48,26 +48,28 @@ int no_mmu_map_address (CPUMIPSState *env, hwaddr = *physical, int *prot,
>=C2=A0 }
>=C2=A0
>=C2=A0 /* fixed mapping MMU emulation */
> -int fixed_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *= prot,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0target_ulong address, int rw, int access_type)<= br> > +int fixed_mmu_map_address(CPUMIPSState *env, hwaddr *physical, i= nt *prot,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 target_ulong address, int rw, int access_type)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 if (address <=3D (int32_t)0x7FFFFFFFUL) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!(env->CP0_Status & (1 <<= ; CP0St_ERL)))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!(env->CP0_Status & (1 <<= ; CP0St_ERL))) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *physical =3D address = + 0x40000000UL;
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 else
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 } else {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *physical =3D address;=
> -=C2=A0 =C2=A0 } else if (address <=3D (int32_t)0xBFFFFFFFUL)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> +=C2=A0 =C2=A0 } else if (address <=3D (int32_t)0xBFFFFFFFUL) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *physical =3D address & 0x1FFFFF= FF;
> -=C2=A0 =C2=A0 else
> +=C2=A0 =C2=A0 } else {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *physical =3D address;
> +=C2=A0 =C2=A0 }
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 *prot =3D PAGE_READ | PAGE_WRITE | PAGE_EXEC;
>=C2=A0 =C2=A0 =C2=A0 return TLBRET_MATCH;
>=C2=A0 }
>=C2=A0
>=C2=A0 /* MIPS32/MIPS64 R4000-style MMU emulation */
> -int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,<= br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0target_ulong address, int rw, int access_type)
> +int r4k_map_address(CPUMIPSState *env, hwaddr *physical, int *prot, > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= target_ulong address, int rw, int access_type)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 uint16_t ASID =3D env->CP0_EntryHi & env-&g= t;CP0_EntryHi_ASID_mask;
>=C2=A0 =C2=A0 =C2=A0 int i;
> @@ -99,8 +101,9 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *phy= sical, int *prot,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (rw !=3D MMU_DATA_S= TORE || (n ? tlb->D1 : tlb->D0)) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *physica= l =3D tlb->PFN[n] | (address & (mask >> 1));
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *prot = =3D PAGE_READ;
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (n ? tlb-&= gt;D1 : tlb->D0)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (n ? tlb-&= gt;D1 : tlb->D0) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 *prot |=3D PAGE_WRITE;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!(n = ? tlb->XI1 : tlb->XI0)) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 *prot |=3D PAGE_EXEC;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> @@ -130,8 +133,11 @@ static int is_seg_am_mapped(unsigned int am, bool= eu, int mmu_idx)
>=C2=A0 =C2=A0 =C2=A0 int32_t adetlb_mask;
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 switch (mmu_idx) {
> -=C2=A0 =C2=A0 case 3 /* ERL */:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* If EU is set, always unmapped */
> +=C2=A0 =C2=A0 case 3:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 /*
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* ERL
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* If EU is set, always unmapped
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (eu) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return 0;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }

This changes from the usual way we format switch case comments to an
unusual way.

If you want to pursue this change, please put it in a separate patch,
so this one is really about fixing "errors and warnings reported by 'checkpatch.pl -= f'", as your commit message promises.



Hi, Markus. Thank you f= or your response.

There must be some misunderstand= ing here:

The line:

=C2= =A0=C2=A0 case 3 /* ERL */:

generates a checkp= atch warning. I don't know why I would put it in a separate patch, if t= his patch is about fixing checkpatch warnings. Please explain.
Secondly, I don't see that this is a usual way we format s= witch statement. I found just several cases in the whole QEMU code base (an= d you claimed in previous comments that there are thousands).
I am just guessing that you somehow mixed this line with the li= ne:

=C2=A0 =C2=A0case 3: /* ERL */
<= br>
that would have not generated checkpatch warning.
<= br>
I don't see any reason to change this patch. Please let m= e know it you still think I should do something else. And you are welcome t= o analyse any patches of mine.

Aleksandar
=C2=A0
[...]

--00000000000007f6800594da656e--