On 10 December 2016 at 02:05, BALATON Zoltan <bala...@eik.bme.hu> wrote:
> Values are not used yet, only stored to allow clients to initialise
> these without failing as long as no 2D engine function is called that
> would use the written value.
>
> Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
> ---
>
> v2: Fixed mask of video_control register for a read only bit
> Changed IRQ status register to write ignored as IRQ is not implemented
>
> hw/display/sm501.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/sm501.c b/hw/display/sm501.c
> index caa7e5c..af5e4db 100644
> --- a/hw/display/sm501.c
> +++ b/hw/display/sm501.c
> @@ -511,6 +511,8 @@ typedef struct SM501State {
> uint32_t dc_panel_hwc_color_1_2;
> uint32_t dc_panel_hwc_color_3;
>
> + uint32_t dc_video_control;
> +
> uint32_t dc_crt_control;
> uint32_t dc_crt_fb_addr;
> uint32_t dc_crt_fb_offset;
> @@ -530,13 +532,20 @@ typedef struct SM501State {
> uint32_t twoD_control;
> uint32_t twoD_pitch;
> uint32_t twoD_foreground;
> + uint32_t twoD_background;
> uint32_t twoD_stretch;
> + uint32_t twoD_color_compare;
> uint32_t twoD_color_compare_mask;
> uint32_t twoD_mask;
> + uint32_t twoD_clip_tl;
> + uint32_t twoD_clip_br;
> + uint32_t twoD_mono_pattern_low;
> + uint32_t twoD_mono_pattern_high;
> uint32_t twoD_window_width;
> uint32_t twoD_source_base;
> uint32_t twoD_destination_base;
> -
> + uint32_t twoD_alpha;
> + uint32_t twoD_wrap;
> } SM501State;
You're still implementing almost all of these as writes to structure
fields which can never be read. Either of these:
(1) a write function case which writes to the struct field, plus
a read function case which returns the value in the struct field
(2) a write function which does nothing (and no read function case,
or a read function case which returns 0)
[you can usually bunch these up into a lot of case FOO: which
share a /* unimplemented, ignore writes */ body]
are OK, but a write function which writes to the struct field but
nothing ever reading that field doesn't make sense.
(You can also LOG_UNIMP for dummy stuff like this if you like, but
that's not something we insist on.)
thanks
-- PMM
