[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 1/8] target/mips: Clean up helper.c
From: |
Aleksandar Markovic |
Subject: |
Re: [PATCH v4 1/8] target/mips: Clean up helper.c |
Date: |
Mon, 14 Oct 2019 10:10:01 +0200 |
On Monday, October 14, 2019, Markus Armbruster <address@hidden> wrote:
Aleksandar Markovic <address@hidden> writes:
> From: Aleksandar Markovic <address@hidden>
>
> Mostly fix errors and warnings reported by 'checkpatch.pl -f'.
>
> Signed-off-by: Aleksandar Markovic <address@hidden>
> ---
> 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
[...]
[PATCH v4 8/8] target/mips: msa: Split helpers for HADD_<S|U>.<H|W|D>, Aleksandar Markovic, 2019/10/13
[PATCH v4 7/8] target/mips: msa: Split helpers for ADD<_A|S_A|S_S|S_U|V>.<B|H|W|D>, Aleksandar Markovic, 2019/10/13
[PATCH v4 6/8] target/mips: msa: Split helpers for ILV<EV|OD|L|R>.<B|H|W|D>, Aleksandar Markovic, 2019/10/13
[PATCH v4 2/8] target/mips: Clean up op_helper.c, Aleksandar Markovic, 2019/10/13
Re: [PATCH v4 0/8] target/mips: Misc cleanups for September/October 2019, no-reply, 2019/10/13