[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rc
From: |
Dong Jia Shi |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] s390x/css: generate solicited crw for rchp completion signaling |
Date: |
Wed, 2 Aug 2017 09:20:41 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
* Halil Pasic <address@hidden> [2017-08-01 17:16:37 +0200]:
>
>
> On 08/01/2017 09:57 AM, Dong Jia Shi wrote:
> [..]
> > --- a/hw/s390x/css.c
> > +++ b/hw/s390x/css.c
> > @@ -1745,10 +1745,10 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid)
> > }
> >
> > /* We don't really use a channel path, so we're done here. */
> > - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT,
> > + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1,
> > channel_subsys.max_cssid > 0 ? 1 : 0, chpid);
> > if (channel_subsys.max_cssid > 0) {
> > - css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 0, real_cssid << 8);
> > + css_queue_crw(CRW_RSC_CHP, CRW_ERC_INIT, 1, 0, real_cssid << 8);
> > }
> > return 0;
> > }
> > @@ -2028,7 +2028,8 @@ void css_subch_assign(uint8_t cssid, uint8_t ssid,
> > uint16_t schid,
> > }
> > }
> >
> > -void css_queue_crw(uint8_t rsc, uint8_t erc, int chain, uint16_t rsid)
> > +void css_queue_crw(uint8_t rsc, uint8_t erc, int solicited,
> > + int chain, uint16_t rsid)
>
> I think you could make the parameters solicited and chain bool (AFAIU
> they are conceptually bool) for clearer semantic. If you go with that
> you could also get rid of the superfluous ternary operator ( we have
> stuff like some_cond ? 1 : 0 for the chain parameter in more than
> one place.
>
> Btw. I find bool flags easy to mix up and difficult to read. I have no better
> idea how to write this (in C) though. I was considering throwing chain and
> solicited together into a single flags parameter, but looking at the client
> code
> it does not look like a good idea.
I think I just need to get used to differet tastes. ;)
>
> Besides the cosmetic considerations above LGTM
Thanks!
--
Dong Jia Shi