Re: [Qemu-devel] [PATCH v6 5/6] fdc_test: update media_change test

[Pavel Hrdina <phrdina@redhat.com>]

On 06/25/2012 11:50 AM, Kevin Wolf wrote:
> Am 22.06.2012 12:33, schrieb Pavel Hrdina:
>> After rewrite DSKCHG bit handling the test has to be updated. Now
>> is needed to seek to different track to clear DSKCHG bit.
>>
>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>> ---
>>   tests/fdc-test.c |   29 +++++++++++++++++++++--------
>>   1 files changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/fdc-test.c b/tests/fdc-test.c
>> index 610e2f1..5280eff 100644
>> --- a/tests/fdc-test.c
>> +++ b/tests/fdc-test.c
>> @@ -156,19 +156,20 @@ static uint8_t send_read_command(void)
>>       return ret;
>>   }
>>
>> -static void send_step_pulse(void)
>> +static void send_step_pulse(bool chg_cyl)
>>   {
>>       int drive = 0;
>>       int head = 0;
>> -    static int cyl = 0;
>> +    int cyl = 0;
>> +
>> +    if (chg_cyl)
>> +        cyl = (cyl + 1) % 4;
> Why do you remove the static? Previously, the function would cycle
> through cylinders 1-4. Now it's always 0 if !chg_cyl and 1 if chg_cyl. I
> think you need to fix this.
>
> I also don't quite understand why you move the increment to here instead
> of leaving it as the bottom. It doesn't look wrong, but pointless.
Now the functionality is different. I need seek to cylinder 0 or 1. 
After every change of media an actual cylinder is set to 0 and I need to 
try seek to cylinder 0 to test, that after this seek the DSKCHG bit is 
still set. Then I need to seek to different cylinder to reset the DSKCHG 
and for that seek to 1 is enough.

I'll chagne the

cyl = (cyl + 1) % 4; to cyl = 1;

>>
>>       floppy_send(CMD_SEEK);
>>       floppy_send(head<<  2 | drive);
>>       g_assert(!get_irq(FLOPPY_IRQ));
>>       floppy_send(cyl);
>>       ack_irq();
>> -
>> -    cyl = (cyl + 1) % 4;
>>   }
>>
>>   static uint8_t cmos_read(uint8_t reg)
>> @@ -195,8 +196,7 @@ static void test_no_media_on_start(void)
>>       assert_bit_set(dir, DSKCHG);
>>       dir = inb(FLOPPY_BASE + reg_dir);
>>       assert_bit_set(dir, DSKCHG);
>> -    send_step_pulse();
>> -    send_step_pulse();
>> +    send_step_pulse(1);
> This is a bool parameter, so you should pass true/false instead of 0/1.
>
> Other than that, the test case additions look good and I'll accept it as
> sufficient for the DSKCHG handling change. It would be great, though, to
> add some more cases that test the implicit seeks during commands like
> READ (ideally one for each command that can seek).
>
> Kevin
I'll fix the bool parameter and then I'll create new patches for more 
test cases.

Pavel