[v3,15/16] boot-order-test: Support fw_cfg in I/O space

Next commit needs it.

Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/boot-order-test.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Markus Armbruster <armbru@redhat.com> writes:

> Next commit needs it.
>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/boot-order-test.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
> index 7b1edc1..d1d99f8 100644
> --- a/tests/boot-order-test.c
> +++ b/tests/boot-order-test.c
> @@ -133,23 +133,31 @@ static void test_prep_boot_order(void)
>      test_boot_orders("prep", read_boot_order_prep, test_cases_prep);
>  }
>  
> -static void read_fw_cfg(uint64_t cfg_addr, uint16_t cmd,
> +static void read_fw_cfg(uint64_t cfg_addr, bool addr_is_io, uint16_t cmd,
>                          void *buf, size_t len)

I missed it earlier, but you can use libqos/fw_cfg.h for this.

Regards,

Anthony Liguori

>  {
>      uint8_t *p = buf;
>      size_t i;
>  
> -    writew(cfg_addr, cmd);
> -    for (i = 0; i < len; i++) {
> -        p[i] = readb(cfg_addr + 2);
> +    if (addr_is_io) {
> +        outw(cfg_addr, cmd);
> +        for (i = 0; i < len; i++) {
> +            p[i] = inb(cfg_addr + 1);
> +        }
> +    } else {
> +        writew(cfg_addr, cmd);
> +        for (i = 0; i < len; i++) {
> +            p[i] = readb(cfg_addr + 2);
> +        }
>      }
>  }
>  
> -static uint16_t read_fw_cfg_i16(uint64_t cfg_addr, uint16_t cmd)
> +static uint16_t read_fw_cfg_i16(uint64_t cfg_addr, bool addr_is_io,
> +                                    uint16_t cmd)
>  {
>      uint16_t value;
>  
> -    read_fw_cfg(cfg_addr, cmd, &value, sizeof(value));
> +    read_fw_cfg(cfg_addr, addr_is_io, cmd, &value, sizeof(value));
>      return le16_to_cpu(value);
>  }
>  
> @@ -157,7 +165,7 @@ static uint16_t read_fw_cfg_i16(uint64_t cfg_addr, uint16_t cmd)
>  
>  static uint64_t read_boot_order_pmac(void)
>  {
> -    return read_fw_cfg_i16(PMAC_CFG_ADDR, FW_CFG_BOOT_DEVICE);
> +    return read_fw_cfg_i16(PMAC_CFG_ADDR, false, FW_CFG_BOOT_DEVICE);
>  }
>  
>  static const boot_order_test test_cases_fw_cfg[] = {
> @@ -182,7 +190,7 @@ static void test_pmac_newworld_boot_order(void)
>  
>  static uint64_t read_boot_order_sun4m(void)
>  {
> -    return read_fw_cfg_i16(SUN4M_CFG_ADDR, FW_CFG_BOOT_DEVICE);
> +    return read_fw_cfg_i16(SUN4M_CFG_ADDR, false, FW_CFG_BOOT_DEVICE);
>  }
>  
>  static void test_sun4m_boot_order(void)
> -- 
> 1.7.11.7

Am 14.06.2013 15:53, schrieb Anthony Liguori:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Next commit needs it.
>>
>> Cc: Blue Swirl <blauwirbel@gmail.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/boot-order-test.c | 24 ++++++++++++++++--------
>>  1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
>> index 7b1edc1..d1d99f8 100644
>> --- a/tests/boot-order-test.c
>> +++ b/tests/boot-order-test.c
>> @@ -133,23 +133,31 @@ static void test_prep_boot_order(void)
>>      test_boot_orders("prep", read_boot_order_prep, test_cases_prep);
>>  }
>>  
>> -static void read_fw_cfg(uint64_t cfg_addr, uint16_t cmd,
>> +static void read_fw_cfg(uint64_t cfg_addr, bool addr_is_io, uint16_t cmd,
>>                          void *buf, size_t len)
> 
> I missed it earlier, but you can use libqos/fw_cfg.h for this.

Agree, but that didn't exist at the time. :-)

Regards,
Andreas

Anthony Liguori <aliguori@us.ibm.com> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Next commit needs it.
>>
>> Cc: Blue Swirl <blauwirbel@gmail.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/boot-order-test.c | 24 ++++++++++++++++--------
>>  1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
>> index 7b1edc1..d1d99f8 100644
>> --- a/tests/boot-order-test.c
>> +++ b/tests/boot-order-test.c
>> @@ -133,23 +133,31 @@ static void test_prep_boot_order(void)
>>      test_boot_orders("prep", read_boot_order_prep, test_cases_prep);
>>  }
>>  
>> -static void read_fw_cfg(uint64_t cfg_addr, uint16_t cmd,
>> +static void read_fw_cfg(uint64_t cfg_addr, bool addr_is_io, uint16_t cmd,
>>                          void *buf, size_t len)
>
> I missed it earlier, but you can use libqos/fw_cfg.h for this.

Missed on rebase, thanks for pointing it out.

Two options:

(1) You commit this minor code duplication now, and I promise to clean
    it up in a follow-up series.

(2) You tell me to rework this series so it uses libqos/fw_cfg.h from
    the start.

I'd prefer (1).

Markus Armbruster <armbru@redhat.com> writes:

> Anthony Liguori <aliguori@us.ibm.com> writes:
>
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> Next commit needs it.
>>>
>>> Cc: Blue Swirl <blauwirbel@gmail.com>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  tests/boot-order-test.c | 24 ++++++++++++++++--------
>>>  1 file changed, 16 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
>>> index 7b1edc1..d1d99f8 100644
>>> --- a/tests/boot-order-test.c
>>> +++ b/tests/boot-order-test.c
>>> @@ -133,23 +133,31 @@ static void test_prep_boot_order(void)
>>>      test_boot_orders("prep", read_boot_order_prep, test_cases_prep);
>>>  }
>>>  
>>> -static void read_fw_cfg(uint64_t cfg_addr, uint16_t cmd,
>>> +static void read_fw_cfg(uint64_t cfg_addr, bool addr_is_io, uint16_t cmd,
>>>                          void *buf, size_t len)
>>
>> I missed it earlier, but you can use libqos/fw_cfg.h for this.
>
> Missed on rebase, thanks for pointing it out.
>
> Two options:
>
> (1) You commit this minor code duplication now, and I promise to clean
>     it up in a follow-up series.
>
> (2) You tell me to rework this series so it uses libqos/fw_cfg.h from
>     the start.
>
> I'd prefer (1).

(3) Commit PATCH 01-06/16 now (which passed your review), bounce the
rest back to me.

Still prefer (1), though :)