On Mon, 2 Dec 2019 19:18:20 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
> On 2019-12-02 15:55, Cornelia Huck wrote:
> > On Thu, 28 Nov 2019 13:46:06 +0100
> > Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
> >> + ccw[0].code = code ;
> >
> > Extra ' ' before ';'
>
> yes, thanks
>
> >
> >> + ccw[0].flags = CCW_F_PCI;
> >
> > Huh, what's that PCI for?
>
> Program Control Interruption
Yes; but why do you need it? Doesn't the QEMU device provide you with
an interrupt for the final status? I don't think PCI makes sense unless
you want a notification for the progress through a chain.
>
> I will add a comment :)
Good idea; the PCI is bound to confuse people :)
>
> >
> >> + ccw[0].count = count;
> >> + ccw[0].data = (int)(unsigned long)data;
> >
> > Can you be sure that data is always below 2G?
>
> Currently yes, the program is loaded at 0x10000 and is quite small
> also doing a test does not hurt for the case the function is used in
> another test someday.
Nod.
>
> >
> >> + orb_p->intparm = 0xcafec0ca;
> >> + orb_p->ctrl = ORB_F_INIT_IRQ|ORB_F_FORMAT|ORB_F_LPM_DFLT;
> >> + orb_p->cpa = (unsigned int) (unsigned long)&ccw[0];
> >> +
> >> + report_prefix_push("Start Subchannel");
> >> + ret = ssch(test_device_sid, orb_p);
> >> + if (ret) {
> >> + report("ssch cc=%d", 0, ret);
> >> + report_prefix_pop();
> >> + return 0;
> >> + }
> >> + report_prefix_pop();
> >> + return 1;
> >> +}
> >> +
> >> +static void test_sense(void)
> >> +{
> >> + int success;
> >> +
> >> + enable_io_irq();
> >> +
> >> + success = start_subchannel(CCW_CMD_SENSE_ID, buffer, sizeof(senseid));
> >> + if (!success) {
> >> + report("start_subchannel failed", 0);
> >> + return;
> >> + }
> >> +
> >> + senseid.cu_type = buffer[2] | (buffer[1] << 8);
> >> + delay(1000);
> >> +
> >> + /* Sense ID is non packed cut_type is at offset +1 byte */
> >> + if (senseid.cu_type == PONG_CU)
> >> + report("cu_type: expect c0ca, got %04x", 1, senseid.cu_type);
> >> + else
> >> + report("cu_type: expect c0ca, got %04x", 0, senseid.cu_type);
> >> +}
> >
> > I'm not really convinced by that logic here. This will fall apart if
> > you don't have your pong device exactly in the right place, and it does
> > not make it easy to extend this for more devices in the future.
>
> Wanted to keep things simple. PONG must be the first valid channel.
> also, should be documented at least.
Yes, please :)
>
> >
> > What about the following:
> > - do a stsch() loop (basically re-use your first patch)
> > - for each I/O subchannel with dnv=1, do SenseID
> > - use the first (?) device with a c0ca CU type as your test device
> >
> > Or maybe I'm overthinking this? It just does not strike me as very
> > robust and reusable.
>
> I can do it.
>
> Thanks for the comments,
>
> Best regards,
> Pierre
>
