| Message ID | 1442197954-8361-1-git-send-email-jk@ozlabs.org |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, Sep 14, 2015 at 12:02 PM, Jeremy Kerr <jk@ozlabs.org> wrote: > > Currently, most astbmc platforms do their own call to prd_init(), but > garrison is out-of-sync. > > This change moves the prd_init call to astbmc_early_init, so we don't > need to enable it on every platform. Good idea. > Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > platforms/astbmc/common.c | 2 ++ > platforms/astbmc/firestone.c | 1 - > platforms/astbmc/habanero.c | 2 -- > platforms/astbmc/palmetto.c | 2 -- > 4 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c > index 8d37785..40a9fc8 100644 > --- a/platforms/astbmc/common.c > +++ b/platforms/astbmc/common.c > @@ -348,4 +348,6 @@ void astbmc_early_init(void) > > /* Setup UART and use it as console with interrupts */ > uart_init(true); > + > + prd_init(); Should it always be last? Do we need a comment? > } > diff --git a/platforms/astbmc/firestone.c b/platforms/astbmc/firestone.c > index 4eed429..aaed13e 100644 > --- a/platforms/astbmc/firestone.c > +++ b/platforms/astbmc/firestone.c > @@ -138,7 +138,6 @@ static bool firestone_probe(void) > > /* Lot of common early inits here */ > astbmc_early_init(); > - prd_init(); > slot_table_init(firestone_phb_table); > > return true; > diff --git a/platforms/astbmc/habanero.c b/platforms/astbmc/habanero.c > index 4ab2cdc..738aa63 100644 > --- a/platforms/astbmc/habanero.c > +++ b/platforms/astbmc/habanero.c > @@ -134,8 +134,6 @@ static bool habanero_probe(void) > /* Lot of common early inits here */ > astbmc_early_init(); > > - prd_init(); > - > slot_table_init(habanero_phb_table); > > return true; > diff --git a/platforms/astbmc/palmetto.c b/platforms/astbmc/palmetto.c > index ca91908..033f103 100644 > --- a/platforms/astbmc/palmetto.c > +++ b/platforms/astbmc/palmetto.c > @@ -39,8 +39,6 @@ static bool palmetto_probe(void) > /* Lot of common early inits here */ > astbmc_early_init(); > > - prd_init(); > - > return true; > } > > -- > 2.1.4 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
Hi Joel, >> --- a/platforms/astbmc/common.c >> +++ b/platforms/astbmc/common.c >> @@ -348,4 +348,6 @@ void astbmc_early_init(void) >> >> /* Setup UART and use it as console with interrupts */ >> uart_init(true); >> + >> + prd_init(); > > Should it always be last? Do we need a comment? There's no need for it to be early. I've added it last as each of the individual platform definitions called prd_init after astbmc_early_init(), so we have the smallest functional change here. Not sure we'd need a comment - it "does what it says on the tin". Cheers, Jeremy
Jeremy Kerr <jk@ozlabs.org> writes: > Hi Joel, > >>> --- a/platforms/astbmc/common.c >>> +++ b/platforms/astbmc/common.c >>> @@ -348,4 +348,6 @@ void astbmc_early_init(void) >>> >>> /* Setup UART and use it as console with interrupts */ >>> uart_init(true); >>> + >>> + prd_init(); >> >> Should it always be last? Do we need a comment? > > There's no need for it to be early. I've added it last as each of the > individual platform definitions called prd_init after > astbmc_early_init(), so we have the smallest functional change here. > > Not sure we'd need a comment - it "does what it says on the tin". Solidly agreed - I casually detest the pattern of "comment above obviously named function" and umm and ahh over the day I'll go through skiboot and rip out all of them. Also, merged as of ef186c6
diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c index 8d37785..40a9fc8 100644 --- a/platforms/astbmc/common.c +++ b/platforms/astbmc/common.c @@ -348,4 +348,6 @@ void astbmc_early_init(void) /* Setup UART and use it as console with interrupts */ uart_init(true); + + prd_init(); } diff --git a/platforms/astbmc/firestone.c b/platforms/astbmc/firestone.c index 4eed429..aaed13e 100644 --- a/platforms/astbmc/firestone.c +++ b/platforms/astbmc/firestone.c @@ -138,7 +138,6 @@ static bool firestone_probe(void) /* Lot of common early inits here */ astbmc_early_init(); - prd_init(); slot_table_init(firestone_phb_table); return true; diff --git a/platforms/astbmc/habanero.c b/platforms/astbmc/habanero.c index 4ab2cdc..738aa63 100644 --- a/platforms/astbmc/habanero.c +++ b/platforms/astbmc/habanero.c @@ -134,8 +134,6 @@ static bool habanero_probe(void) /* Lot of common early inits here */ astbmc_early_init(); - prd_init(); - slot_table_init(habanero_phb_table); return true; diff --git a/platforms/astbmc/palmetto.c b/platforms/astbmc/palmetto.c index ca91908..033f103 100644 --- a/platforms/astbmc/palmetto.c +++ b/platforms/astbmc/palmetto.c @@ -39,8 +39,6 @@ static bool palmetto_probe(void) /* Lot of common early inits here */ astbmc_early_init(); - prd_init(); - return true; }
Currently, most astbmc platforms do their own call to prd_init(), but garrison is out-of-sync. This change moves the prd_init call to astbmc_early_init, so we don't need to enable it on every platform. Signed-off-by: Jeremy Kerr <jk@ozlabs.org> --- platforms/astbmc/common.c | 2 ++ platforms/astbmc/firestone.c | 1 - platforms/astbmc/habanero.c | 2 -- platforms/astbmc/palmetto.c | 2 -- 4 files changed, 2 insertions(+), 5 deletions(-)