[Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions

Cédric Le Goater posted 3 patches 4 years, 11 months ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test asan passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190506142042.28096-1-clg@kaod.org
Maintainers: Andrew Jeffery <andrew@aj.id.au>, "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Joel Stanley <joel@jms.id.au>
include/hw/arm/aspeed_soc.h |  40 ++++++-
hw/arm/aspeed.c             |   8 +-
hw/arm/aspeed_soc.c         | 226 ++++++++++++++++++++++--------------
3 files changed, 184 insertions(+), 90 deletions(-)
[Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Cédric Le Goater 4 years, 11 months ago
Hello,

Here is a series adding a couple of cleanups to the Aspeed SoCs to
prepare ground for extensions and new SoCs.

Thanks,

C.

Changes since v1:

 - moved enum defining the Aspeed controller names under aspeed_soc.h
 - removed AspeedSoCInfo 'sdram_base' field
 - fixed clang compilation

Cédric Le Goater (3):
  aspeed: add a per SoC mapping for the interrupt space
  aspeed: add a per SoC mapping for the memory space
  aspeed: use sysbus_init_child_obj() to initialize children

 include/hw/arm/aspeed_soc.h |  40 ++++++-
 hw/arm/aspeed.c             |   8 +-
 hw/arm/aspeed_soc.c         | 226 ++++++++++++++++++++++--------------
 3 files changed, 184 insertions(+), 90 deletions(-)

-- 
2.20.1


Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Cédric Le Goater 4 years, 11 months ago
Hello,

On 5/6/19 4:20 PM, Cédric Le Goater wrote:
> Hello,
> 
> Here is a series adding a couple of cleanups to the Aspeed SoCs to
> prepare ground for extensions and new SoCs.
> 
> Thanks,
> 
> C.
> 
> Changes since v1:
> 
>  - moved enum defining the Aspeed controller names under aspeed_soc.h
>  - removed AspeedSoCInfo 'sdram_base' field
>  - fixed clang compilation
> 
> Cédric Le Goater (3):
>   aspeed: add a per SoC mapping for the interrupt space
>   aspeed: add a per SoC mapping for the memory space

I think these two patches are fine to go even if Philippe's comments 
are not addressed. There are valid but not a blocker to me.  

>   aspeed: use sysbus_init_child_obj() to initialize children

Philippe has taken over this patch in a larger series which will go 
through Eduardo's tree, if I understood well the emails. When merged, 
we can try to re-merge the RTC patchset from Joel. I think we made 
things a little more complex than they should have been. 

Thanks,

C.

>  include/hw/arm/aspeed_soc.h |  40 ++++++-
>  hw/arm/aspeed.c             |   8 +-
>  hw/arm/aspeed_soc.c         | 226 ++++++++++++++++++++++--------------
>  3 files changed, 184 insertions(+), 90 deletions(-)
> 


Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Philippe Mathieu-Daudé 4 years, 11 months ago
On 5/20/19 9:47 AM, Cédric Le Goater wrote:
> Hello,
> 
> On 5/6/19 4:20 PM, Cédric Le Goater wrote:
>> Hello,
>>
>> Here is a series adding a couple of cleanups to the Aspeed SoCs to
>> prepare ground for extensions and new SoCs.
>>
>> Thanks,
>>
>> C.
>>
>> Changes since v1:
>>
>>  - moved enum defining the Aspeed controller names under aspeed_soc.h
>>  - removed AspeedSoCInfo 'sdram_base' field
>>  - fixed clang compilation
>>
>> Cédric Le Goater (3):
>>   aspeed: add a per SoC mapping for the interrupt space
>>   aspeed: add a per SoC mapping for the memory space
> 
> I think these two patches are fine to go even if Philippe's comments 
> are not addressed. There are valid but not a blocker to me.  

OK, so:

patches 1 & 2:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Peter, can you apply them?

> 
>>   aspeed: use sysbus_init_child_obj() to initialize children
> 
> Philippe has taken over this patch in a larger series which will go 
> through Eduardo's tree, if I understood well the emails. When merged, 
> we can try to re-merge the RTC patchset from Joel. I think we made 
> things a little more complex than they should have been. 

Sorry if I made things more complex. I went on PTO after sending
"hw/arm: Use object_initialize_child for correct reference counting" [*]
then was slow to address Thomas/Markus comments.
Then maybe I should start pinging maintainer more aggressively when my
series are reviewed but not merged, to not delay further developments.

I took note of your comment and will try to keep things simple the next
time.

Regards,

Phil.

> 
> Thanks,
> 
> C.


Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Cédric Le Goater 4 years, 11 months ago
Hello,

On 5/20/19 1:09 PM, Philippe Mathieu-Daudé wrote:
> On 5/20/19 9:47 AM, Cédric Le Goater wrote:
>> Hello,
>>
>> On 5/6/19 4:20 PM, Cédric Le Goater wrote:
>>> Hello,
>>>
>>> Here is a series adding a couple of cleanups to the Aspeed SoCs to
>>> prepare ground for extensions and new SoCs.
>>>
>>> Thanks,
>>>
>>> C.
>>>
>>> Changes since v1:
>>>
>>>  - moved enum defining the Aspeed controller names under aspeed_soc.h
>>>  - removed AspeedSoCInfo 'sdram_base' field
>>>  - fixed clang compilation
>>>
>>> Cédric Le Goater (3):
>>>   aspeed: add a per SoC mapping for the interrupt space
>>>   aspeed: add a per SoC mapping for the memory space
>>
>> I think these two patches are fine to go even if Philippe's comments 
>> are not addressed. There are valid but not a blocker to me.  
> 
> OK, so:
> 
> patches 1 & 2:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Peter, can you apply them?
> 
>>
>>>   aspeed: use sysbus_init_child_obj() to initialize children
>>
>> Philippe has taken over this patch in a larger series which will go 
>> through Eduardo's tree, if I understood well the emails. When merged, 
>> we can try to re-merge the RTC patchset from Joel. I think we made 
>> things a little more complex than they should have been. 
> 
> Sorry if I made things more complex. I went on PTO after sending

PTO ?

> "hw/arm: Use object_initialize_child for correct reference counting" [*]
> then was slow to address Thomas/Markus comments.
> Then maybe I should start pinging maintainer more aggressively when my
> series are reviewed but not merged, to not delay further developments.

Well, I don't know if there is a good method for transversal patchsets 
like this one. I guess it depends on the area. 

The overall merging process became more complex that expected after our 
three simple patchsets (Yours, Joel's and mine) collided. 
 
> I took note of your comment and will try to keep things simple the next
> time.

It's not a big issue. We have time to provide fixes before 4.1 is out. 
Let's put some energy to move on and get code merged.

Peter, 

do you want me to resend with only the two first patches and include 
Joel's in the same series ? I would leave out the part Philippe is 
covering in his object_initialize_child() patchset.

Thanks,

C.


Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Cédric Le Goater 4 years, 11 months ago
> Peter, 
> 
> do you want me to resend with only the two first patches and include 
> Joel's in the same series ? I would leave out the part Philippe is 
> covering in his object_initialize_child() patchset.

Nope, we can not do that, conflicts arise. I suppose the easier is wait
for Philippe's patchset to be merged and then rebase.


C.

Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Philippe Mathieu-Daudé 4 years, 11 months ago
On 5/20/19 3:32 PM, Cédric Le Goater wrote:
>> Peter, 
>>
>> do you want me to resend with only the two first patches and include 
>> Joel's in the same series ? I would leave out the part Philippe is 
>> covering in his object_initialize_child() patchset.
> 
> Nope, we can not do that, conflicts arise. I suppose the easier is wait
> for Philippe's patchset to be merged and then rebase.

Eduardo said he'll send a pull request during the week.

Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Peter Maydell 4 years, 11 months ago
On Mon, 20 May 2019 at 17:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 5/20/19 3:32 PM, Cédric Le Goater wrote:
> >> Peter,
> >>
> >> do you want me to resend with only the two first patches and include
> >> Joel's in the same series ? I would leave out the part Philippe is
> >> covering in his object_initialize_child() patchset.
> >
> > Nope, we can not do that, conflicts arise. I suppose the easier is wait
> > for Philippe's patchset to be merged and then rebase.
>
> Eduardo said he'll send a pull request during the week.

I am now completely lost about the status of these patches,
so I'm just dropping this series from my to-review queue.
Please send a fresh patchset once any dependencies have
got into master.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Cédric Le Goater 4 years, 11 months ago
On 5/23/19 2:52 PM, Peter Maydell wrote:
> On Mon, 20 May 2019 at 17:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> On 5/20/19 3:32 PM, Cédric Le Goater wrote:
>>>> Peter,
>>>>
>>>> do you want me to resend with only the two first patches and include
>>>> Joel's in the same series ? I would leave out the part Philippe is
>>>> covering in his object_initialize_child() patchset.
>>>
>>> Nope, we can not do that, conflicts arise. I suppose the easier is wait
>>> for Philippe's patchset to be merged and then rebase.
>>
>> Eduardo said he'll send a pull request during the week.
> 
> I am now completely lost about the status of these patches,
> so I'm just dropping this series from my to-review queue.

yes. It's ok.

> Please send a fresh patchset once any dependencies have
> got into master.

I plan to send a larger one once Eduardo's patchset is merged.

Thanks,

C.

Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
Posted by Eduardo Habkost 4 years, 11 months ago
On Thu, May 23, 2019 at 03:03:11PM +0200, Cédric Le Goater wrote:
> On 5/23/19 2:52 PM, Peter Maydell wrote:
> > On Mon, 20 May 2019 at 17:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>
> >> On 5/20/19 3:32 PM, Cédric Le Goater wrote:
> >>>> Peter,
> >>>>
> >>>> do you want me to resend with only the two first patches and include
> >>>> Joel's in the same series ? I would leave out the part Philippe is
> >>>> covering in his object_initialize_child() patchset.
> >>>
> >>> Nope, we can not do that, conflicts arise. I suppose the easier is wait
> >>> for Philippe's patchset to be merged and then rebase.
> >>
> >> Eduardo said he'll send a pull request during the week.
> > 
> > I am now completely lost about the status of these patches,
> > so I'm just dropping this series from my to-review queue.
> 
> yes. It's ok.
> 
> > Please send a fresh patchset once any dependencies have
> > got into master.
> 
> I plan to send a larger one once Eduardo's patchset is merged.

I've just submitted a pull request including the
object_initialize_child() patches from Philippe:
  [PULL 00/17] Machine Core queue, 2019-05-24

Sorry for the delay.

-- 
Eduardo