Re: [Xen-devel] [PATCH 03/12 v3] xen/arm: vpl011: Add pl011 uart emulation in Xen

[Julien Grall <julien.grall@xxxxxxx>]

Hi Bhupinder,

On 01/06/17 14:34, Bhupinder Thakur wrote:
> On 26 May 2017 at 19:12, Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx> wrote:
> > > > +
> > > > +    switch ( vpl011_reg )
> > > > +    {
> > > > +    case DR:
> > >
> > > As mentioned above, you could do:
> > >
> > > {
> > >      uint8_t ch;
> > >      ....
> > > }
> > >
> > > > +        vpl011_read_data(v->domain, &ch);
> > > > +        *r = ch;
> > >
> > > Please use vreg_reg32_extract(...).
> > ok.
> >
> > > > +        break;
> > >
> > > I admit I would prefer the "return 1;" here rather than "break;". This makes
> > > easier to follow the emulation for a given register.
> > >
> > > I would even be in favor of duplicating the "if ( !vpl011... )" in each case
> > > for the same reason.
> Do you mean that I should repeat the vpl011_reg32_check_access() and
> return for each switch case?
I think it would make easier to follow the emulation if you repeat the 
if (vpl011_reg32_check_access) and the return 1 in each case.
So each register emulation is standalone.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel