Re: [Qemu-devel] [PATCH v3 6/9] sm501: Do not clear read only bits when writing registers

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2466 bytes --]

On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote:
> When writing registers that have read only bits we have to avoid
> changing these bits as they may have non zero values. Make sure we use
> the correct masks to mask out read only and reserved bits when
> changing registers.
> 
> Also remove extra spaces from dram_control and arbitration_control
> assignments.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> v3: Not only preserve read only bits but also allow clearing r/w
> bits

Applied to ppc-for-3.0, thanks.

> 
>  hw/display/sm501.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> index e47be99..ca0840f 100644
> --- a/hw/display/sm501.c
> +++ b/hw/display/sm501.c
> @@ -836,27 +836,30 @@ static void sm501_system_config_write(void *opaque, hwaddr addr,
>  
>      switch (addr) {
>      case SM501_SYSTEM_CONTROL:
> -        s->system_control = value & 0xE300B8F7;
> +        s->system_control &= 0x10DB0000;
> +        s->system_control |= value & 0xEF00B8F7;
>          break;
>      case SM501_MISC_CONTROL:
> -        s->misc_control = value & 0xFF7FFF20;
> +        s->misc_control &= 0xEF;
> +        s->misc_control |= value & 0xFF7FFF10;
>          break;
>      case SM501_GPIO31_0_CONTROL:
>          s->gpio_31_0_control = value;
>          break;
>      case SM501_GPIO63_32_CONTROL:
> -        s->gpio_63_32_control = value;
> +        s->gpio_63_32_control = value & 0xFF80FFFF;
>          break;
>      case SM501_DRAM_CONTROL:
>          s->local_mem_size_index = (value >> 13) & 0x7;
>          /* TODO : check validity of size change */
> -        s->dram_control |=  value & 0x7FFFFFC3;
> +        s->dram_control &= 0x80000000;
> +        s->dram_control |= value & 0x7FFFFFC3;
>          break;
>      case SM501_ARBTRTN_CONTROL:
> -        s->arbitration_control =  value & 0x37777777;
> +        s->arbitration_control = value & 0x37777777;
>          break;
>      case SM501_IRQ_MASK:
> -        s->irq_mask = value;
> +        s->irq_mask = value & 0xFFDF3F5F;
>          break;
>      case SM501_MISC_TIMING:
>          s->misc_timing = value & 0xF31F1FFF;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]