[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/4] smbus: Fix spd_data_generate() error API violation
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 2/4] smbus: Fix spd_data_generate() error API violation |
Date: |
Fri, 24 Apr 2020 13:23:21 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Philippe Mathieu-Daudé <address@hidden> writes:
> On 4/24/20 11:45 AM, Markus Armbruster wrote:
>> BALATON Zoltan <address@hidden> writes:
>>> On Tue, 21 Apr 2020, Markus Armbruster wrote:
> [...]
>>>>
>>>> Here's what I actually disagree with:
>>>>
>>>> 1. QEMU warning the user about its choice of memory type, but only
>>>> sometimes. Why warn? There is nothing wrong, and there is nothing the
>>>> user can do to avoid the condition that triggers the warning.
>>>
>>> The memory size that triggers the warning is specified by the user so
>>> the user can do someting about it.
>>
>> There is no way to trigger the warning. If we dropped PATCH 1 instead
>> of fixing it as I did in v2, then the only way to trigger the warning is
>> -M sam460ex -m 64 or -m 32, and the only way to avoid it is to don't do
>> that.
>>
>> Why would a user care whether he gets DDR or DDR2 memory?
>
> To use a different firmware code path!
Let's see how that works out for users.
Assume machine foobar needs a non-default firmware for "small" memory
sizes, such as -m 64.
Alice doesn't know. She starts QEMU like this:
$ qemu-system-foo -M foobar -m 64
It hangs early in boot. Not good.
Except the warning from spd_data_generate() rides to her rescue!
qemu-system-foo: warning: Memory size is too small for SDRAM type,
adjusting type
Since Alice is rather sharp, the warning makes her realize immediately
that she needs to use different firmware, like this:
$ qemu-system-foo -M foobar -m 64 -bios roms/foobar/firmware-small.bin
Okay, I'm done telling my fairy tale.
If you want helpful, make it work out of the box: default to the
firmware that actually works, don't make users guess which one they
need.
But one more time: this is all theoretical. We're talking about unused,
broken code. If you want to keep it, please add users, and fix it up.
[PATCH 1/4] sam460ex: Revert change to SPD memory type for <= 128 MiB, Markus Armbruster, 2020/04/20
[PATCH 4/4] smbus: Fix spd_data_generate() for number of banks > 2, Markus Armbruster, 2020/04/20