[v1] Misc hw/ide legacy clean up

[PATCH 0/8] Misc hw/ide legacy clean up

Posted by BALATON Zoltan 4 years, 1 month ago

These are some clean ups to remove more legacy init functions and
lessen dependence on include/hw/ide.h with some simplifications in
board code. There should be no functional change.

BALATON Zoltan (8):
  hw/ide: Get rid of piix3_init functions
  hw/ide: Get rid of piix4_init function
  hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h
  hw/ide: Move MAX_IDE_BUS define to one header
  hw/ide/pci.c: Coding style update to fix checkpatch errors
  hw/ide: Do ide_drive_get() within pci_ide_create_devs()
  hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h
  hw/ide: Remove unneeded inclusion of hw/ide.h

 hw/alpha/dp264.c              | 15 +++------------
 hw/hppa/hppa_sys.h            |  1 -
 hw/hppa/machine.c             |  3 ---
 hw/i386/pc_piix.c             | 20 +++++++++-----------
 hw/ide/ahci_internal.h        |  1 +
 hw/ide/pci.c                  | 10 ++++++----
 hw/ide/piix.c                 | 31 +------------------------------
 hw/isa/piix4.c                | 14 +++++---------
 hw/mips/mips_fulong2e.c       |  6 +-----
 hw/mips/mips_malta.c          |  6 ++----
 hw/mips/mips_r4k.c            |  4 +---
 hw/ppc/mac_newworld.c         |  2 --
 hw/ppc/mac_oldworld.c         |  2 --
 hw/ppc/prep.c                 |  3 ---
 hw/sparc64/sun4u.c            |  7 +------
 include/hw/ide.h              |  6 ------
 include/hw/ide/internal.h     |  3 +++
 include/hw/ide/pci.h          |  3 ++-
 include/hw/misc/macio/macio.h |  1 +
 include/hw/southbridge/piix.h |  3 +--
 20 files changed, 37 insertions(+), 104 deletions(-)

-- 
2.21.1

Re: [PATCH 0/8] Misc hw/ide legacy clean up

Posted by Mark Cave-Ayland 4 years, 1 month ago

On 13/03/2020 21:14, BALATON Zoltan wrote:

> These are some clean ups to remove more legacy init functions and
> lessen dependence on include/hw/ide.h with some simplifications in
> board code. There should be no functional change.
> 
> BALATON Zoltan (8):
>   hw/ide: Get rid of piix3_init functions
>   hw/ide: Get rid of piix4_init function
>   hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h
>   hw/ide: Move MAX_IDE_BUS define to one header
>   hw/ide/pci.c: Coding style update to fix checkpatch errors
>   hw/ide: Do ide_drive_get() within pci_ide_create_devs()
>   hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h
>   hw/ide: Remove unneeded inclusion of hw/ide.h
> 
>  hw/alpha/dp264.c              | 15 +++------------
>  hw/hppa/hppa_sys.h            |  1 -
>  hw/hppa/machine.c             |  3 ---
>  hw/i386/pc_piix.c             | 20 +++++++++-----------
>  hw/ide/ahci_internal.h        |  1 +
>  hw/ide/pci.c                  | 10 ++++++----
>  hw/ide/piix.c                 | 31 +------------------------------
>  hw/isa/piix4.c                | 14 +++++---------
>  hw/mips/mips_fulong2e.c       |  6 +-----
>  hw/mips/mips_malta.c          |  6 ++----
>  hw/mips/mips_r4k.c            |  4 +---
>  hw/ppc/mac_newworld.c         |  2 --
>  hw/ppc/mac_oldworld.c         |  2 --
>  hw/ppc/prep.c                 |  3 ---
>  hw/sparc64/sun4u.c            |  7 +------
>  include/hw/ide.h              |  6 ------
>  include/hw/ide/internal.h     |  3 +++
>  include/hw/ide/pci.h          |  3 ++-
>  include/hw/misc/macio/macio.h |  1 +
>  include/hw/southbridge/piix.h |  3 +--
>  20 files changed, 37 insertions(+), 104 deletions(-)

This looks like a good clean-up to me, but certainly it would be good to get a second
opinion from people more familiar with the IDE code internals.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [PATCH 0/8] Misc hw/ide legacy clean up

Posted by Paolo Bonzini 4 years, 1 month ago

On 14/03/20 12:45, Mark Cave-Ayland wrote:
> On 13/03/2020 21:14, BALATON Zoltan wrote:
> 
>> These are some clean ups to remove more legacy init functions and
>> lessen dependence on include/hw/ide.h with some simplifications in
>> board code. There should be no functional change.
>>
>> BALATON Zoltan (8):
>>   hw/ide: Get rid of piix3_init functions
>>   hw/ide: Get rid of piix4_init function
>>   hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h
>>   hw/ide: Move MAX_IDE_BUS define to one header
>>   hw/ide/pci.c: Coding style update to fix checkpatch errors
>>   hw/ide: Do ide_drive_get() within pci_ide_create_devs()
>>   hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h
>>   hw/ide: Remove unneeded inclusion of hw/ide.h
>>
>>  hw/alpha/dp264.c              | 15 +++------------
>>  hw/hppa/hppa_sys.h            |  1 -
>>  hw/hppa/machine.c             |  3 ---
>>  hw/i386/pc_piix.c             | 20 +++++++++-----------
>>  hw/ide/ahci_internal.h        |  1 +
>>  hw/ide/pci.c                  | 10 ++++++----
>>  hw/ide/piix.c                 | 31 +------------------------------
>>  hw/isa/piix4.c                | 14 +++++---------
>>  hw/mips/mips_fulong2e.c       |  6 +-----
>>  hw/mips/mips_malta.c          |  6 ++----
>>  hw/mips/mips_r4k.c            |  4 +---
>>  hw/ppc/mac_newworld.c         |  2 --
>>  hw/ppc/mac_oldworld.c         |  2 --
>>  hw/ppc/prep.c                 |  3 ---
>>  hw/sparc64/sun4u.c            |  7 +------
>>  include/hw/ide.h              |  6 ------
>>  include/hw/ide/internal.h     |  3 +++
>>  include/hw/ide/pci.h          |  3 ++-
>>  include/hw/misc/macio/macio.h |  1 +
>>  include/hw/southbridge/piix.h |  3 +--
>>  20 files changed, 37 insertions(+), 104 deletions(-)
> 
> This looks like a good clean-up to me, but certainly it would be good to get a second
> opinion from people more familiar with the IDE code internals.
> 
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Yes, it looks good to me.  Thanks!

Paolo

Re: [PATCH 0/8] Misc hw/ide legacy clean up

Posted by Markus Armbruster 4 years, 1 month ago

BALATON Zoltan <balaton@eik.bme.hu> writes:

> These are some clean ups to remove more legacy init functions and
> lessen dependence on include/hw/ide.h with some simplifications in
> board code. There should be no functional change.

PATCH 1 could quote precedence more clearly in the commit message, but
that's detail.

I don't like PATCH 4.

PATCH 1-3,5-8:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH 0/8] Misc hw/ide legacy clean up

Posted by BALATON Zoltan 4 years, 1 month ago

On Mon, 16 Mar 2020, Markus Armbruster wrote:
> BALATON Zoltan <balaton@eik.bme.hu> writes:
>> These are some clean ups to remove more legacy init functions and
>> lessen dependence on include/hw/ide.h with some simplifications in
>> board code. There should be no functional change.
>
> PATCH 1 could quote precedence more clearly in the commit message, but
> that's detail.
>
> I don't like PATCH 4.

Sent alternative v2 version of patch 7 so you can drop patch 4 if you 
like, the rest of the series should apply unchanged. Note that there might 
be some places where MAX_IDE_BUS is defined but not used and current code 
probably has assumption about this being 2 elsewhere and would break with 
any other value so other than philosophical there should be no reason to 
keep this defined everywhere.

> PATCH 1-3,5-8:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks.

Regards,
BALATON Zoltan

Re: [PATCH 0/8] Misc hw/ide legacy clean up

Posted by BALATON Zoltan 4 years, 1 month ago

On Mon, 16 Mar 2020, BALATON Zoltan wrote:
> On Mon, 16 Mar 2020, Markus Armbruster wrote:
>> BALATON Zoltan <balaton@eik.bme.hu> writes:
>>> These are some clean ups to remove more legacy init functions and
>>> lessen dependence on include/hw/ide.h with some simplifications in
>>> board code. There should be no functional change.
>> 
>> PATCH 1 could quote precedence more clearly in the commit message, but
>> that's detail.
>> 
>> I don't like PATCH 4.
>
> Sent alternative v2 version of patch 7 so you can drop patch 4 if you like,

and patch 6 v2 also sent that is affected as well if you drop patch 4.

> the rest of the series should apply unchanged. Note that there might be some 
> places where MAX_IDE_BUS is defined but not used and current code probably 
> has assumption about this being 2 elsewhere and would break with any other 
> value so other than philosophical there should be no reason to keep this 
> defined everywhere.
>
>> PATCH 1-3,5-8:
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> Thanks.
>
> Regards,
> BALATON Zoltan
>

Re: [PATCH 0/8] Misc hw/ide legacy clean up

Posted by John Snow 4 years, 1 month ago


On 3/16/20 9:41 AM, BALATON Zoltan wrote:
> On Mon, 16 Mar 2020, BALATON Zoltan wrote:
>> On Mon, 16 Mar 2020, Markus Armbruster wrote:
>>> BALATON Zoltan <balaton@eik.bme.hu> writes:
>>>> These are some clean ups to remove more legacy init functions and
>>>> lessen dependence on include/hw/ide.h with some simplifications in
>>>> board code. There should be no functional change.
>>>
>>> PATCH 1 could quote precedence more clearly in the commit message, but
>>> that's detail.
>>>
>>> I don't like PATCH 4.
>>
>> Sent alternative v2 version of patch 7 so you can drop patch 4 if you
>> like,
> 
> and patch 6 v2 also sent that is affected as well if you drop patch 4.
> 
>> the rest of the series should apply unchanged. Note that there might
>> be some places where MAX_IDE_BUS is defined but not used and current
>> code probably has assumption about this being 2 elsewhere and would
>> break with any other value so other than philosophical there should be
>> no reason to keep this defined everywhere.
>>
>>> PATCH 1-3,5-8:
>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>
>> Thanks.
>>
>> Regards,
>> BALATON Zoltan
>>
> 

Can you do me a favor and send a proper v2 of the whole series, with
review tags applied?

--js