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

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 11/19/21 12:02, Thomas Huth wrote:
> 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?
> 
> 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.

OK. For pflash:
Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>