* Halil Pasic <[email protected]> [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
