[PATCH v5 1/6] hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC

Philippe Mathieu-Daudé posted 6 patches 4 years, 8 months ago
There is a newer version of this series
[PATCH v5 1/6] hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
isa_superio_realize() calls isa_fdc_init_drives(), which is defined
in hw/block/fdc.c, so ISA_SUPERIO needs to select the FDC symbol.

Reported-by: John Snow <jsnow@redhat.com>
Fixes: c0ff3795143 ("Introduce a CONFIG_ISA_SUPERIO switch for isa-superio.c")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/isa/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 55e0003ce40..7216f66a54a 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -17,6 +17,7 @@ config ISA_SUPERIO
     bool
     select ISA_BUS
     select PCKBD
+    select FDC
 
 config PC87312
     bool
-- 
2.26.3

Re: [PATCH v5 1/6] hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC
Posted by Thomas Huth 4 years, 8 months ago
On 18/05/2021 21.32, Philippe Mathieu-Daudé wrote:
> isa_superio_realize() calls isa_fdc_init_drives(), which is defined
> in hw/block/fdc.c, so ISA_SUPERIO needs to select the FDC symbol.

If I get that right, not all superio chipsets provide a floppy drive 
(there's this "k->floppy.is_enabled" check in there) ... but for boards that 
don't need the FDC, this would currently require a stub for that function - 
so unless someone adds such a stub, you're right, this currently requires 
the FDC code, thus:

Reviewed-by: Thomas Huth <thuth@redhat.com>

> Reported-by: John Snow <jsnow@redhat.com>
> Fixes: c0ff3795143 ("Introduce a CONFIG_ISA_SUPERIO switch for isa-superio.c")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   hw/isa/Kconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> index 55e0003ce40..7216f66a54a 100644
> --- a/hw/isa/Kconfig
> +++ b/hw/isa/Kconfig
> @@ -17,6 +17,7 @@ config ISA_SUPERIO
>       bool
>       select ISA_BUS
>       select PCKBD
> +    select FDC
>   
>   config PC87312
>       bool
> 


Re: [PATCH v5 1/6] hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 5/19/21 10:23 AM, Thomas Huth wrote:
> On 18/05/2021 21.32, Philippe Mathieu-Daudé wrote:
>> isa_superio_realize() calls isa_fdc_init_drives(), which is defined
>> in hw/block/fdc.c, so ISA_SUPERIO needs to select the FDC symbol.
> 
> If I get that right, not all superio chipsets provide a floppy drive
> (there's this "k->floppy.is_enabled" check in there) ... but for boards
> that don't need the FDC, this would currently require a stub for that
> function

Good point. I'll try to get it right.


Re: [PATCH v5 1/6] hw/isa/Kconfig: Fix missing dependency ISA_SUPERIO -> FDC
Posted by Thomas Huth 4 years, 8 months ago
On 19/05/2021 13.05, Philippe Mathieu-Daudé wrote:
> On 5/19/21 10:23 AM, Thomas Huth wrote:
>> On 18/05/2021 21.32, Philippe Mathieu-Daudé wrote:
>>> isa_superio_realize() calls isa_fdc_init_drives(), which is defined
>>> in hw/block/fdc.c, so ISA_SUPERIO needs to select the FDC symbol.
>>
>> If I get that right, not all superio chipsets provide a floppy drive
>> (there's this "k->floppy.is_enabled" check in there) ... but for boards
>> that don't need the FDC, this would currently require a stub for that
>> function
> 
> Good point. I'll try to get it right.

As discussed in v6, all superio chips currently have a way to use an FDC, so 
this version of the patch is fine:

Reviewed-by: Thomas Huth <thuth@redhat.com>