Re: [PATCH for-6.2] hw/misc/sifive_u_otp: Use IF_PFLASH for the OTP device instead of IF_NONE

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

> On 19/11/2021 11.40, Philippe Mathieu-Daudé wrote:
>> On 11/19/21 11:25, Thomas Huth wrote:
>>> Configuring a drive with "if=none" is meant for creation of a backend
>>> only, it should not get automatically assigned to a device frontend.
>>> Use "if=pflash" for the One-Time-Programmable device instead (like
>>> it is e.g. also done for the efuse device in hw/arm/xlnx-zcu102.c).
>>>
>>> Since the old way of configuring the device has already been published
>>> with the previous QEMU versions, we cannot remove this immediately, but
>>> have to deprecate it and support it for at least two more releases.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   docs/about/deprecated.rst | 6 ++++++
>>>   hw/misc/sifive_u_otp.c    | 9 ++++++++-
>>>   2 files changed, 14 insertions(+), 1 deletion(-)
>> 
>>> diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c
>>> index 18aa0bd55d..cf6098ff2c 100644
>>> --- a/hw/misc/sifive_u_otp.c
>>> +++ b/hw/misc/sifive_u_otp.c
>>> @@ -209,7 +209,14 @@ static void sifive_u_otp_realize(DeviceState *dev, 
>>> Error **errp)
>>>                             TYPE_SIFIVE_U_OTP, SIFIVE_U_OTP_REG_SIZE);
>>>       sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
>>>   -    dinfo = drive_get_next(IF_NONE);
>>> +    dinfo = drive_get_next(IF_PFLASH);
>>> +    if (!dinfo) {
>>> +        dinfo = drive_get_next(IF_NONE);
>> 
>> Isn't it a bug to call drive_get_next() from DeviceRealize()?
>> Shouldn't drive_get_next() be restricted to the MachineClass?

drive_get_next() needs to die:

    Subject: [PATCH v2 00/13] Eliminate drive_get_next()
    Message-Id: <20211117163409.3587705-1-armbru@redhat.com>

Not for 6.2.

> Yes, that would certainly be better - but considering that we are
> already past RC1 of the 6.2 release, I'd rather prefer to keep this
> patch rather as small as possible and do such refactorings during the
> next development cycle instead.

Concur.

Your patch conflicts with mine.  No worries, I'll rebase.

Reviewed-by: Markus Armbruster <armbru@redhat.com>