| Message ID | db1b69270ca4d2335e6fb2455ee56552373757fc.1488068248.git.balaton@eik.bme.hu |
|---|---|
| State | New |
| Headers | show |
On 25 February 2017 at 21:47, BALATON Zoltan <balaton@eik.bme.hu> wrote: > Clients normally only write to these registers, nothing is known to > ever read them but they are documented as read/write so allow clients > to also read the values. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/display/sm501.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) Oh, the read code is in a separate patch? Please put all the handling for a particular register in the same patch. If you think the resulting patch is too long you can split it into several patches each of which handles a group of registers if you like (but it probably isn't necessary). thanks -- PMM
On Thu, 2 Mar 2017, Peter Maydell wrote: > On 25 February 2017 at 21:47, BALATON Zoltan <balaton@eik.bme.hu> wrote: >> Clients normally only write to these registers, nothing is known to >> ever read them but they are documented as read/write so allow clients >> to also read the values. >> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/display/sm501.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 57 insertions(+) > > Oh, the read code is in a separate patch? Please put all the handling > for a particular register in the same patch. If you think the > resulting patch is too long you can split it into several patches > each of which handles a group of registers if you like (but it > probably isn't necessary). So is it OK to squash this in the previous patch adding write and make the two a single patch? > > thanks > -- PMM > >
On 2 March 2017 at 20:22, BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Thu, 2 Mar 2017, Peter Maydell wrote: >> >> On 25 February 2017 at 21:47, BALATON Zoltan <balaton@eik.bme.hu> wrote: >>> >>> Clients normally only write to these registers, nothing is known to >>> ever read them but they are documented as read/write so allow clients >>> to also read the values. >>> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- >>> hw/display/sm501.c | 57 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 57 insertions(+) >> >> >> Oh, the read code is in a separate patch? Please put all the handling >> for a particular register in the same patch. If you think the >> resulting patch is too long you can split it into several patches >> each of which handles a group of registers if you like (but it >> probably isn't necessary). > > > So is it OK to squash this in the previous patch adding write and make the > two a single patch? Yes please. thanks -- PMM
diff --git a/hw/display/sm501.c b/hw/display/sm501.c index af5e4db..32223f6 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -1149,9 +1149,66 @@ static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr, SM501_DPRINTF("sm501 2d engine regs : read addr=%x\n", (int)addr); switch (addr) { + case SM501_2D_SOURCE: + ret = s->twoD_source; + break; + case SM501_2D_DESTINATION: + ret = s->twoD_destination; + break; + case SM501_2D_DIMENSION: + ret = s->twoD_dimension; + break; + case SM501_2D_CONTROL: + ret = s->twoD_control; + break; + case SM501_2D_PITCH: + ret = s->twoD_pitch; + break; + case SM501_2D_FOREGROUND: + ret = s->twoD_foreground; + break; + case SM501_2D_BACKGROUND: + ret = s->twoD_background; + break; + case SM501_2D_STRETCH: + ret = s->twoD_stretch; + break; + case SM501_2D_COLOR_COMPARE: + ret = s->twoD_color_compare; + break; + case SM501_2D_COLOR_COMPARE_MASK: + ret = s->twoD_color_compare_mask; + break; + case SM501_2D_MASK: + ret = s->twoD_mask; + break; + case SM501_2D_CLIP_TL: + ret = s->twoD_clip_tl; + break; + case SM501_2D_CLIP_BR: + ret = s->twoD_clip_br; + break; + case SM501_2D_MONO_PATTERN_LOW: + ret = s->twoD_mono_pattern_low; + break; + case SM501_2D_MONO_PATTERN_HIGH: + ret = s->twoD_mono_pattern_high; + break; + case SM501_2D_WINDOW_WIDTH: + ret = s->twoD_window_width; + break; case SM501_2D_SOURCE_BASE: ret = s->twoD_source_base; break; + case SM501_2D_DESTINATION_BASE: + ret = s->twoD_destination_base; + break; + case SM501_2D_ALPHA: + ret = s->twoD_alpha; + break; + case SM501_2D_WRAP: + ret = s->twoD_wrap; + break; case SM501_2D_STATUS: ret = 0; /* Should return interrupt status */ break;
Clients normally only write to these registers, nothing is known to ever read them but they are documented as read/write so allow clients to also read the values. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/display/sm501.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)