mbox series

[v2,00/14] PPC440 devices misc clean up

Message ID cover.1688586835.git.balaton@eik.bme.hu (mailing list archive)
Headers show
Series PPC440 devices misc clean up | expand

Message

BALATON Zoltan July 5, 2023, 8:12 p.m. UTC
These are some small misc clean ups to PPC440 related device models
which is all I have ready for now.

v2:
- Added R-b tags from Philippe
- Addressed review comments
- Added new patch to rename parent field of PPC460EXPCIEState to parent_obj

Patches needing review: 6 7 10-13

BALATON Zoltan (14):
  ppc440: Change ppc460ex_pcie_init() parameter type
  ppc440: Add cpu link property to PCIe controller model
  ppc440: Add a macro to shorten PCIe controller DCR registration
  ppc440: Rename parent field of PPC460EXPCIEState to match code style
  ppc440: Rename local variable in dcr_read_pcie()
  ppc440: Stop using system io region for PCIe buses
  ppc/sam460ex: Remove address_space_mem local variable
  ppc440: Add busnum property to PCIe controller model
  ppc440: Remove ppc460ex_pcie_init legacy init function
  ppc4xx_pci: Rename QOM type name define
  ppc4xx_pci: Add define for ppc4xx-host-bridge type name
  ppc440_pcix: Rename QOM type define abd move it to common header
  ppc440_pcix: Don't use iomem for regs
  ppc440_pcix: Stop using system io region for PCI bus

 hw/ppc/ppc440.h         |   1 -
 hw/ppc/ppc440_bamboo.c  |   3 +-
 hw/ppc/ppc440_pcix.c    |  28 +++---
 hw/ppc/ppc440_uc.c      | 192 +++++++++++++++++-----------------------
 hw/ppc/ppc4xx_pci.c     |  10 +--
 hw/ppc/sam460ex.c       |  33 ++++---
 include/hw/ppc/ppc4xx.h |   5 +-
 7 files changed, 129 insertions(+), 143 deletions(-)

Comments

BALATON Zoltan July 5, 2023, 8:20 p.m. UTC | #1
On Wed, 5 Jul 2023, BALATON Zoltan wrote:
> These are some small misc clean ups to PPC440 related device models
> which is all I have ready for now.

Sorry, typo in email addresses in cc. Should I send it again or you can 
pick up from the list?

Regards,
BALATON Zoltan

> v2:
> - Added R-b tags from Philippe
> - Addressed review comments
> - Added new patch to rename parent field of PPC460EXPCIEState to parent_obj
>
> Patches needing review: 6 7 10-13
>
> BALATON Zoltan (14):
>  ppc440: Change ppc460ex_pcie_init() parameter type
>  ppc440: Add cpu link property to PCIe controller model
>  ppc440: Add a macro to shorten PCIe controller DCR registration
>  ppc440: Rename parent field of PPC460EXPCIEState to match code style
>  ppc440: Rename local variable in dcr_read_pcie()
>  ppc440: Stop using system io region for PCIe buses
>  ppc/sam460ex: Remove address_space_mem local variable
>  ppc440: Add busnum property to PCIe controller model
>  ppc440: Remove ppc460ex_pcie_init legacy init function
>  ppc4xx_pci: Rename QOM type name define
>  ppc4xx_pci: Add define for ppc4xx-host-bridge type name
>  ppc440_pcix: Rename QOM type define abd move it to common header
>  ppc440_pcix: Don't use iomem for regs
>  ppc440_pcix: Stop using system io region for PCI bus
>
> hw/ppc/ppc440.h         |   1 -
> hw/ppc/ppc440_bamboo.c  |   3 +-
> hw/ppc/ppc440_pcix.c    |  28 +++---
> hw/ppc/ppc440_uc.c      | 192 +++++++++++++++++-----------------------
> hw/ppc/ppc4xx_pci.c     |  10 +--
> hw/ppc/sam460ex.c       |  33 ++++---
> include/hw/ppc/ppc4xx.h |   5 +-
> 7 files changed, 129 insertions(+), 143 deletions(-)
>
>
Daniel Henrique Barboza July 6, 2023, 12:48 a.m. UTC | #2
Zoltan,

Patches 1-9 are queued. Don't need to re-send those.


Thanks,

Daniel

On 7/5/23 17:12, BALATON Zoltan wrote:
> These are some small misc clean ups to PPC440 related device models
> which is all I have ready for now.
> 
> v2:
> - Added R-b tags from Philippe
> - Addressed review comments
> - Added new patch to rename parent field of PPC460EXPCIEState to parent_obj
> 
> Patches needing review: 6 7 10-13
> 
> BALATON Zoltan (14):
>    ppc440: Change ppc460ex_pcie_init() parameter type
>    ppc440: Add cpu link property to PCIe controller model
>    ppc440: Add a macro to shorten PCIe controller DCR registration
>    ppc440: Rename parent field of PPC460EXPCIEState to match code style
>    ppc440: Rename local variable in dcr_read_pcie()
>    ppc440: Stop using system io region for PCIe buses
>    ppc/sam460ex: Remove address_space_mem local variable
>    ppc440: Add busnum property to PCIe controller model
>    ppc440: Remove ppc460ex_pcie_init legacy init function
>    ppc4xx_pci: Rename QOM type name define
>    ppc4xx_pci: Add define for ppc4xx-host-bridge type name
>    ppc440_pcix: Rename QOM type define abd move it to common header
>    ppc440_pcix: Don't use iomem for regs
>    ppc440_pcix: Stop using system io region for PCI bus
> 
>   hw/ppc/ppc440.h         |   1 -
>   hw/ppc/ppc440_bamboo.c  |   3 +-
>   hw/ppc/ppc440_pcix.c    |  28 +++---
>   hw/ppc/ppc440_uc.c      | 192 +++++++++++++++++-----------------------
>   hw/ppc/ppc4xx_pci.c     |  10 +--
>   hw/ppc/sam460ex.c       |  33 ++++---
>   include/hw/ppc/ppc4xx.h |   5 +-
>   7 files changed, 129 insertions(+), 143 deletions(-)
>
BALATON Zoltan July 6, 2023, 1:09 a.m. UTC | #3
On Wed, 5 Jul 2023, Daniel Henrique Barboza wrote:
> Zoltan,
>
> Patches 1-9 are queued. Don't need to re-send those.

Thanks, the last two patches are also reviewed and they don't depend on 
the ones before so you could queue those too.

The only outstanding patches are those 3 that rename the type defines to 
match their string values. We could come up with better names but those 
suggested by Philippe are too long IMO so at least the patches in this 
series clean up the current mess and we could rename these later. I'd 
rather not change the string values too much as those are what QOM 
actually uses to ideintify the types but we're free to change the defines. 
Currently we have:
#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
and then a "ppc4xx-host-bridge" type without a define which is another 
type which is quite confusing. I may have partly created this mess back 
when I first tried to add sam460ex and did not know much about this but at 
least I'd like to improve it a little and resolve some of it now.

Regards,
BALATON Zoltan
Daniel Henrique Barboza July 6, 2023, 7:13 a.m. UTC | #4
On 7/5/23 22:09, BALATON Zoltan wrote:
> On Wed, 5 Jul 2023, Daniel Henrique Barboza wrote:
>> Zoltan,
>>
>> Patches 1-9 are queued. Don't need to re-send those.
> 
> Thanks, the last two patches are also reviewed and they don't depend on the ones before so you could queue those too.

Just queued patch 13.

Patch 14 doesn't apply in ppc-next even after applying patch 13:

$ git am -s -m \[PATCH\ v2\ 14_14\]\ ppc440_pcix\:\ Stop\ using\ system\ io\ region\ for\ PCI\ bus\ -\ BALATON\ Zoltan\ \<balaton@eik.bme.hu\>\ -\ 2023-07-05\ 1712.eml
Applying: ppc440_pcix: Stop using system io region for PCI bus
error: patch failed: hw/ppc/ppc440_pcix.c:490
error: hw/ppc/ppc440_pcix.c: patch does not apply
error: patch failed: hw/ppc/sam460ex.c:441
error: hw/ppc/sam460ex.c: patch does not apply
Patch failed at 0001 ppc440_pcix: Stop using system io region for PCI bus
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

I suspect we need  some of the previous (10, 11, 12) to apply it cleanly.



Thanks,

Daniel

> 
> The only outstanding patches are those 3 that rename the type defines to match their string values. We could come up with better names but those suggested by Philippe are too long IMO so at least the patches in this series clean up the current mess and we could rename these later. I'd rather not change the string values too much as those are what QOM actually uses to ideintify the types but we're free to change the defines. Currently we have:
> #define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
> and then a "ppc4xx-host-bridge" type without a define which is another type which is quite confusing. I may have partly created this mess back when I first tried to add sam460ex and did not know much about this but at least I'd like to improve it a little and resolve some of it now.
> 
> Regards,
> BALATON Zoltan
BALATON Zoltan July 6, 2023, 11:19 a.m. UTC | #5
On Thu, 6 Jul 2023, Daniel Henrique Barboza wrote:
> On 7/5/23 22:09, BALATON Zoltan wrote:
>> On Wed, 5 Jul 2023, Daniel Henrique Barboza wrote:
>>> Zoltan,
>>> 
>>> Patches 1-9 are queued. Don't need to re-send those.
>> 
>> Thanks, the last two patches are also reviewed and they don't depend on the 
>> ones before so you could queue those too.
>
> Just queued patch 13.
>
> Patch 14 doesn't apply in ppc-next even after applying patch 13:
>
> $ git am -s -m \[PATCH\ v2\ 14_14\]\ ppc440_pcix\:\ Stop\ using\ system\ io\ 
> region\ for\ PCI\ bus\ -\ BALATON\ Zoltan\ \<balaton@eik.bme.hu\>\ -\ 
> 2023-07-05\ 1712.eml
> Applying: ppc440_pcix: Stop using system io region for PCI bus
> error: patch failed: hw/ppc/ppc440_pcix.c:490
> error: hw/ppc/ppc440_pcix.c: patch does not apply
> error: patch failed: hw/ppc/sam460ex.c:441
> error: hw/ppc/sam460ex.c: patch does not apply
> Patch failed at 0001 ppc440_pcix: Stop using system io region for PCI bus
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> I suspect we need  some of the previous (10, 11, 12) to apply it cleanly.

There was only one place in context that differs. All I did was an 
interactive rebase with moving it to the front and it did not have any 
conflict. I've resent a v3 series with that so you can take the first 
patch from that (or the rest of that if we can reach a decision on 
naming). The first attempt had wrong dates so I've resent the series. 
Sorry about that.

Regards,
BALATON Zoltan