* Halil Pasic <pa...@linux.vnet.ibm.com> [2017-07-26 18:45:34 +0200]:
[...] > >>> @@ -946,6 +953,17 @@ static void sch_handle_start_func_virtual(SubchDev > >>> *sch) > >>> suspend_allowed = true; > >>> } > >>> sch->last_cmd_valid = false; > >>> + if (sch->channel_prog & (CCW1_ADDR_MASK | > >>> + sch->ccw_fmt_1 ? 0 : 0xff000000)) { > >> I have problem in recognizing the operator precedence here: > >> (CCW1_ADDR_MASK | sch->ccw_fmt_1 ? 0 : 0xff000000) > >> > >> Bitwise '|' has higher precedence than '?:', so the above equals to: > >> (CCW1_ADDR_MASK | sch->ccw_fmt_1) ? 0 : 0xff000000 > >> > >> So you will always get a '0', no? > >> > > > > I'm afraid you are right. Good catch! This was supposed to be > > (CCW1_ADDR_MASK | (sch->ccw_fmt_1 ? 0 : 0xff000000)) > > > > > >>> + /* generate channel program check */ > >>> + s->ctrl &= ~SCSW_ACTL_START_PEND; > >>> + s->cstat = SCSW_CSTAT_PROG_CHECK; > >>> + s->ctrl &= ~SCSW_CTRL_MASK_STCTL; > >>> + s->ctrl |= SCSW_STCTL_PRIMARY | SCSW_STCTL_SECONDARY | > >>> + SCSW_STCTL_ALERT | SCSW_STCTL_STATUS_PEND; > >>> + s->cpa = sch->channel_prog + 8; > >>> + return; > >>> + } > >> I think you could let css_interpret_ccw() do the sanity check on its > >> @ccw_addr parameter when (sch->last_cmd_valid == false), to reuse the > >> code of generating channel program check. > >> > > > > I'm working on factoring out the code manipulating SCSW (among others). If > > we > > do that this will look nicer. What you propose is certainly viable, althoug > > maybe little less straight forward. > > > > Yet another option would be to use a label and jump into the loop (AFAIR > > that > > would be also valid). > > > > Let us see what is Connie's opinion. > > > > After re-examining the PoP I'm inclined to say we have to check this on every > iteration because of how "main-storage location is unavailable" is defined in > this context: the definition depends on the ccw format. Sounds natural! > There is nothing about this in the ccw chaining section of the pop but > it can be found in the I/O interrupts chapter. > > I think I will have to redo this patch :/ Ok. > > Regards, > Halil > > > > >>> do { > >>> ret = css_interpret_ccw(sch, sch->channel_prog, suspend_allowed); > >>> switch (ret) { > >>> -- > >>> 2.11.2 > >>> > >> > > > > > > -- Dong Jia Shi