| Message ID | 1328270849-22324-1-git-send-email-daniel.lezcano@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi!
Our comments:
- function names don't match commit comment disable/enable vs recouple/decouple. Decouple is a better name than disable, because GIC is not really disabled it is just disconnected.
- there is no reason to place these functions inside the db8500-prcmu.c file. There is so much stuff in the PRCMU register base so we can not have everything in one file. Why not have it as it is?
- why the gic_mask function?
- The original code has been updated and now looks like this:
/* Decouple GIC from the interrupt bus */
void ux500_pm_gic_decouple(void)
{
prcmu_write_masked(PRCM_A9_MASK_REQ,
PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ,
PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ);
(void)prcmu_read(PRCM_A9_MASK_REQ);
udelay(GIC_FREEZE_DELAY); /* Wait for the GIC to freeze */
}
/* Recouple GIC with the interrupt bus */
void ux500_pm_gic_recouple(void)
{
prcmu_write_masked(PRCM_A9_MASK_REQ,
PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ,
0);
}
BR
Rickard Andersson& Jonas Aberg
> This patch allows to decouple and recouple the gic from the PRCMU.
> This is needed to put the A9 core in retention mode with the cpuidle
> driver.
>
> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org>
> ---
> drivers/mfd/db8500-prcmu.c | 42 ++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/db8500-prcmu.h | 2 +
> include/linux/mfd/dbx500-prcmu.h | 16 ++++++++++++++
> 3 files changed, 60 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
> index af8e0ef..c708431 100644
> --- a/drivers/mfd/db8500-prcmu.c
> +++ b/drivers/mfd/db8500-prcmu.c
> @@ -528,6 +528,10 @@ static const char *hwacc_ret_regulator_name[NUM_HW_ACC] = {
>
> #define PRCMU_PLLDSI_LOCKP_LOCKED 0x3
>
> +#define PRCMU_A9_MASK_REQ 0x00000328
> +#define PRCMU_A9_MASK_REQ_MASK 0x00000001
> +#define PRCMU_A9_MASK_ACK 0x0000032C
> +
> static struct {
> u8 project_number;
> u8 api_version;
> @@ -787,6 +791,44 @@ int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll)
> return 0;
> }
>
> +/*
> + * prcmu_gic_mask - apply the mask to the mask request
> + * register. This mask has the bits [1-31] reserved and
> + * the request applies to the bit 0 of this register.
> + */
> +static inline void prcmu_gic_mask(u32 mask)
> +{
> + u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
> + writel((val& ~PRCMU_A9_MASK_REQ_MASK) | mask,
> + _PRCMU_BASE + PRCMU_A9_MASK_REQ);
> +}
> +
> +/*
> + * db8500_prcmu_gic_disable - This function decouple the gic
> + * from the prcmu.
> + */
> +void db8500_prcmu_gic_disable(void)
> +{
> + prcmu_gic_mask(PRCMU_A9_MASK_REQ_MASK);
> +
> + /* Wait for gic mask register update */
> + while (!(readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ)&
> + PRCMU_A9_MASK_REQ_MASK))
> + cpu_relax();
> +
> + /* Wait a few cycles for the gic mask completion */
> + udelay(1);
> +}
> +
> +/*
> + * db8500_prcmu_gic_enable - This function recouple the gic
> + * with the prcmu.
> + */
> +void db8500_prcmu_gic_enable(void)
> +{
> + prcmu_gic_mask(0);
> +}
> +
> /* This function should only be called while mb0_transfer.lock is held. */
> static void config_wakeups(void)
> {
> diff --git a/include/linux/mfd/db8500-prcmu.h b/include/linux/mfd/db8500-prcmu.h
> index 60d27f7..4a1032c 100644
> --- a/include/linux/mfd/db8500-prcmu.h
> +++ b/include/linux/mfd/db8500-prcmu.h
> @@ -536,6 +536,8 @@ int prcmu_load_a9wdog(u8 id, u32 val);
>
> void db8500_prcmu_system_reset(u16 reset_code);
> int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll);
> +void db8500_prcmu_gic_disable(void);
> +void db8500_prcmu_gic_enable(void);
> void db8500_prcmu_enable_wakeups(u32 wakeups);
> int db8500_prcmu_set_epod(u16 epod_id, u8 epod_state);
> int db8500_prcmu_request_clock(u8 clock, bool enable);
> diff --git a/include/linux/mfd/dbx500-prcmu.h b/include/linux/mfd/dbx500-prcmu.h
> index bac942f..a5fee69 100644
> --- a/include/linux/mfd/dbx500-prcmu.h
> +++ b/include/linux/mfd/dbx500-prcmu.h
> @@ -237,6 +237,22 @@ static inline int prcmu_set_power_state(u8 state, bool keep_ulp_clk,
> keep_ap_pll);
> }
>
> +static inline void prcmu_gic_disable(void)
> +{
> + if (machine_is_u5500())
> + return;
> + else
> + return db8500_prcmu_gic_disable();
> +}
> +
> +static inline void prcmu_gic_enable(void)
> +{
> + if (machine_is_u5500())
> + return;
> + else
> + return db8500_prcmu_gic_enable();
> +}
> +
> static inline int prcmu_set_epod(u16 epod_id, u8 epod_state)
> {
> if (machine_is_u5500())
On 02/06/2012 10:19 AM, Rickard Andersson wrote: > Hi! > > Our comments: Thanks Rickard and Jonas for your comments. > - function names don't match commit comment disable/enable vs > recouple/decouple. Decouple is a better name than disable, because GIC > is not really disabled it is just disconnected. Ok, that makes sense. > - there is no reason to place these functions inside the db8500-prcmu.c > file. There is so much stuff in the PRCMU register base so we can not > have everything in one file. Why not have it as it is? Why spread the prcmu code when it is related to the prcmu ? Linus ? What do you think ? > - why the gic_mask function? Because the register has 31 bits reserved which could be used later without modifying this function. > - The original code has been updated and now looks like this: That is the same code, except the while loop where, if this code assumption is correct, means we will do only one iteration in the loop. Thanks -- Daniel > /* Decouple GIC from the interrupt bus */ > void ux500_pm_gic_decouple(void) > { > prcmu_write_masked(PRCM_A9_MASK_REQ, > PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ, > PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ); > > (void)prcmu_read(PRCM_A9_MASK_REQ); > > udelay(GIC_FREEZE_DELAY); /* Wait for the GIC to freeze */ > } > > /* Recouple GIC with the interrupt bus */ > void ux500_pm_gic_recouple(void) > { > prcmu_write_masked(PRCM_A9_MASK_REQ, > PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ, > 0); > } > > BR > Rickard Andersson& Jonas Aberg > >> This patch allows to decouple and recouple the gic from the PRCMU. >> This is needed to put the A9 core in retention mode with the cpuidle >> driver. >> >> Signed-off-by: Daniel Lezcano<daniel.lezcano@linaro.org> >> --- >> drivers/mfd/db8500-prcmu.c | 42 ++++++++++++++++++++++++++++++++++++++ >> include/linux/mfd/db8500-prcmu.h | 2 + >> include/linux/mfd/dbx500-prcmu.h | 16 ++++++++++++++ >> 3 files changed, 60 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c >> index af8e0ef..c708431 100644 >> --- a/drivers/mfd/db8500-prcmu.c >> +++ b/drivers/mfd/db8500-prcmu.c >> @@ -528,6 +528,10 @@ static const char >> *hwacc_ret_regulator_name[NUM_HW_ACC] = { >> >> #define PRCMU_PLLDSI_LOCKP_LOCKED 0x3 >> >> +#define PRCMU_A9_MASK_REQ 0x00000328 >> +#define PRCMU_A9_MASK_REQ_MASK 0x00000001 >> +#define PRCMU_A9_MASK_ACK 0x0000032C >> + >> static struct { >> u8 project_number; >> u8 api_version; >> @@ -787,6 +791,44 @@ int db8500_prcmu_set_power_state(u8 state, bool >> keep_ulp_clk, bool keep_ap_pll) >> return 0; >> } >> >> +/* >> + * prcmu_gic_mask - apply the mask to the mask request >> + * register. This mask has the bits [1-31] reserved and >> + * the request applies to the bit 0 of this register. >> + */ >> +static inline void prcmu_gic_mask(u32 mask) >> +{ >> + u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ); >> + writel((val& ~PRCMU_A9_MASK_REQ_MASK) | mask, >> + _PRCMU_BASE + PRCMU_A9_MASK_REQ); >> +} >> + >> +/* >> + * db8500_prcmu_gic_disable - This function decouple the gic >> + * from the prcmu. >> + */ >> +void db8500_prcmu_gic_disable(void) >> +{ >> + prcmu_gic_mask(PRCMU_A9_MASK_REQ_MASK); >> + >> + /* Wait for gic mask register update */ >> + while (!(readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ)& >> + PRCMU_A9_MASK_REQ_MASK)) >> + cpu_relax(); >> + >> + /* Wait a few cycles for the gic mask completion */ >> + udelay(1); >> +} >> + >> +/* >> + * db8500_prcmu_gic_enable - This function recouple the gic >> + * with the prcmu. >> + */ >> +void db8500_prcmu_gic_enable(void) >> +{ >> + prcmu_gic_mask(0); >> +} >> + >> /* This function should only be called while mb0_transfer.lock is >> held. */ >> static void config_wakeups(void) >> { >> diff --git a/include/linux/mfd/db8500-prcmu.h >> b/include/linux/mfd/db8500-prcmu.h >> index 60d27f7..4a1032c 100644 >> --- a/include/linux/mfd/db8500-prcmu.h >> +++ b/include/linux/mfd/db8500-prcmu.h >> @@ -536,6 +536,8 @@ int prcmu_load_a9wdog(u8 id, u32 val); >> >> void db8500_prcmu_system_reset(u16 reset_code); >> int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool >> keep_ap_pll); >> +void db8500_prcmu_gic_disable(void); >> +void db8500_prcmu_gic_enable(void); >> void db8500_prcmu_enable_wakeups(u32 wakeups); >> int db8500_prcmu_set_epod(u16 epod_id, u8 epod_state); >> int db8500_prcmu_request_clock(u8 clock, bool enable); >> diff --git a/include/linux/mfd/dbx500-prcmu.h >> b/include/linux/mfd/dbx500-prcmu.h >> index bac942f..a5fee69 100644 >> --- a/include/linux/mfd/dbx500-prcmu.h >> +++ b/include/linux/mfd/dbx500-prcmu.h >> @@ -237,6 +237,22 @@ static inline int prcmu_set_power_state(u8 state, >> bool keep_ulp_clk, >> keep_ap_pll); >> } >> >> +static inline void prcmu_gic_disable(void) >> +{ >> + if (machine_is_u5500()) >> + return; >> + else >> + return db8500_prcmu_gic_disable(); >> +} >> + >> +static inline void prcmu_gic_enable(void) >> +{ >> + if (machine_is_u5500()) >> + return; >> + else >> + return db8500_prcmu_gic_enable(); >> +} >> + >> static inline int prcmu_set_epod(u16 epod_id, u8 epod_state) >> { >> if (machine_is_u5500()) >
On Mon, Feb 6, 2012 at 3:10 PM, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > On 02/06/2012 10:19 AM, Rickard Andersson wrote: >> - there is no reason to place these functions inside the db8500-prcmu.c >> file. There is so much stuff in the PRCMU register base so we can not >> have everything in one file. Why not have it as it is? > > Why spread the prcmu code when it is related to the prcmu ? > > Linus ? What do you think ? I have no strong opinions on this. When I discussed it with Mattias who wrote much of the driver we concluded that the PRCMU drivers needs a lot of shaping up. Mainly it may need to be split into two parts: one which is a "pure" MFD device (register access part) one MFD firmware interaction part, and then a platform-specific (U8500) part. This would help getting code in more logical places. For now I would suggest not moving stuff around, I thin Mattias is onto cleaning up the structure as per above and we could be messing it up for him, which is not good. Yours, Linus Walleij
On 02/06/2012 06:37 PM, Linus Walleij wrote: > On Mon, Feb 6, 2012 at 3:10 PM, Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: >> On 02/06/2012 10:19 AM, Rickard Andersson wrote: > >>> - there is no reason to place these functions inside the db8500-prcmu.c >>> file. There is so much stuff in the PRCMU register base so we can not >>> have everything in one file. Why not have it as it is? >> >> Why spread the prcmu code when it is related to the prcmu ? >> >> Linus ? What do you think ? > > I have no strong opinions on this. When I discussed it with Mattias > who wrote much of the driver we concluded that the PRCMU drivers needs > a lot of shaping up. Mainly it may need to be split into two parts: one > which is a "pure" MFD device (register access part) one MFD firmware > interaction part, and then a platform-specific (U8500) part. > > This would help getting code in more logical places. > > For now I would suggest not moving stuff around, I thin Mattias is onto > cleaning up the structure as per above and we could be messing it up > for him, which is not good. Ok, I will resend the patch with the name changed to decouple/recouple. Is the patch acceptable as it is ? Thanks -- Daniel
On 02/06/2012 03:10 PM, Daniel Lezcano wrote: > On 02/06/2012 10:19 AM, Rickard Andersson wrote: >> Hi! >> >> Our comments: > Thanks Rickard and Jonas for your comments. > >> - function names don't match commit comment disable/enable vs >> recouple/decouple. Decouple is a better name than disable, because GIC >> is not really disabled it is just disconnected. > Ok, that makes sense. > >> - there is no reason to place these functions inside the db8500-prcmu.c >> file. There is so much stuff in the PRCMU register base so we can not >> have everything in one file. Why not have it as it is? > Why spread the prcmu code when it is related to the prcmu ? > > Linus ? What do you think ? > >> - why the gic_mask function? > Because the register has 31 bits reserved which could be used later > without modifying this function. This is will most likely not happen. If that happens we could add that function then. We should remove it for now. >> - The original code has been updated and now looks like this: > That is the same code, except the while loop where, if this code > assumption is correct, means we will do only one iteration in the loop. Yes, but in theory you can be stuck forever in that loop, so please remove the loop. BR Rickard
On 02/07/2012 09:39 AM, Rickard Andersson wrote: > On 02/06/2012 03:10 PM, Daniel Lezcano wrote: >> On 02/06/2012 10:19 AM, Rickard Andersson wrote: >>> Hi! >>> >>> Our comments: >> Thanks Rickard and Jonas for your comments. >> >>> - function names don't match commit comment disable/enable vs >>> recouple/decouple. Decouple is a better name than disable, because GIC >>> is not really disabled it is just disconnected. >> Ok, that makes sense. >> >>> - there is no reason to place these functions inside the db8500-prcmu.c >>> file. There is so much stuff in the PRCMU register base so we can not >>> have everything in one file. Why not have it as it is? >> Why spread the prcmu code when it is related to the prcmu ? >> >> Linus ? What do you think ? >> >>> - why the gic_mask function? >> Because the register has 31 bits reserved which could be used later >> without modifying this function. > This is will most likely not happen. If that happens we could add that > function then. We should remove it for now. >>> - The original code has been updated and now looks like this: >> That is the same code, except the while loop where, if this code >> assumption is correct, means we will do only one iteration in the loop. > Yes, but in theory you can be stuck forever in that loop, so please > remove the loop. Mmmh, can you elaborate please ? I think a comment and/or an error should be returned in this case, no ?
On 02/07/2012 10:22 AM, Daniel Lezcano wrote: > On 02/07/2012 09:39 AM, Rickard Andersson wrote: >> On 02/06/2012 03:10 PM, Daniel Lezcano wrote: >>> On 02/06/2012 10:19 AM, Rickard Andersson wrote: >>>> Hi! >>>> >>>> Our comments: >>> Thanks Rickard and Jonas for your comments. >>> >>>> - function names don't match commit comment disable/enable vs >>>> recouple/decouple. Decouple is a better name than disable, because GIC >>>> is not really disabled it is just disconnected. >>> Ok, that makes sense. >>> >>>> - there is no reason to place these functions inside the db8500-prcmu.c >>>> file. There is so much stuff in the PRCMU register base so we can not >>>> have everything in one file. Why not have it as it is? >>> Why spread the prcmu code when it is related to the prcmu ? >>> >>> Linus ? What do you think ? >>> >>>> - why the gic_mask function? >>> Because the register has 31 bits reserved which could be used later >>> without modifying this function. >> This is will most likely not happen. If that happens we could add that >> function then. We should remove it for now. >>>> - The original code has been updated and now looks like this: >>> That is the same code, except the while loop where, if this code >>> assumption is correct, means we will do only one iteration in the loop. >> Yes, but in theory you can be stuck forever in that loop, so please >> remove the loop. > Mmmh, can you elaborate please ? I think a comment and/or an error > should be returned in this case, no ? > We do not need the loop. PRCMU FW does decouple of the GIC and if they for some reason does that at the same time (they should not) we will be stuck.
On 02/07/2012 10:57 AM, Rickard Andersson wrote: > On 02/07/2012 10:22 AM, Daniel Lezcano wrote: >> On 02/07/2012 09:39 AM, Rickard Andersson wrote: >>> On 02/06/2012 03:10 PM, Daniel Lezcano wrote: >>>> On 02/06/2012 10:19 AM, Rickard Andersson wrote: >>>>> Hi! >>>>> >>>>> Our comments: >>>> Thanks Rickard and Jonas for your comments. >>>> >>>>> - function names don't match commit comment disable/enable vs >>>>> recouple/decouple. Decouple is a better name than disable, because GIC >>>>> is not really disabled it is just disconnected. >>>> Ok, that makes sense. >>>> >>>>> - there is no reason to place these functions inside the >>>>> db8500-prcmu.c >>>>> file. There is so much stuff in the PRCMU register base so we can not >>>>> have everything in one file. Why not have it as it is? >>>> Why spread the prcmu code when it is related to the prcmu ? >>>> >>>> Linus ? What do you think ? >>>> >>>>> - why the gic_mask function? >>>> Because the register has 31 bits reserved which could be used later >>>> without modifying this function. >>> This is will most likely not happen. If that happens we could add that >>> function then. We should remove it for now. >>>>> - The original code has been updated and now looks like this: >>>> That is the same code, except the while loop where, if this code >>>> assumption is correct, means we will do only one iteration in the loop. >>> Yes, but in theory you can be stuck forever in that loop, so please >>> remove the loop. >> Mmmh, can you elaborate please ? I think a comment and/or an error >> should be returned in this case, no ? >> > We do not need the loop. PRCMU FW does decouple of the GIC and if they > for some reason does that at the same time (they should not) we will be > stuck. Sorry, but I still don't get it. What is "they" ? Do you mean one cpu can decouples the cpu while the other cpu recouples it ? Taking the routine, why is needed the "read" if we don't care the return value ? void ux500_pm_gic_decouple(void) { prcmu_write_masked(PRCM_A9_MASK_REQ, PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ, PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ); (void)prcmu_read(PRCM_A9_MASK_REQ); udelay(GIC_FREEZE_DELAY); /* Wait for the GIC to freeze */ } Thanks -- Daniel
On 7 February 2012 15:15, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > On 02/07/2012 10:57 AM, Rickard Andersson wrote: >> >> On 02/07/2012 10:22 AM, Daniel Lezcano wrote: >>> >>> On 02/07/2012 09:39 AM, Rickard Andersson wrote: >>>> >>>> On 02/06/2012 03:10 PM, Daniel Lezcano wrote: >>>>> >>>>> On 02/06/2012 10:19 AM, Rickard Andersson wrote: >>>>>> >>>>>> Hi! >>>>>> >>>>>> Our comments: >>>>> >>>>> Thanks Rickard and Jonas for your comments. >>>>> >>>>>> - function names don't match commit comment disable/enable vs >>>>>> recouple/decouple. Decouple is a better name than disable, because GIC >>>>>> is not really disabled it is just disconnected. >>>>> >>>>> Ok, that makes sense. >>>>> >>>>>> - there is no reason to place these functions inside the >>>>>> db8500-prcmu.c >>>>>> file. There is so much stuff in the PRCMU register base so we can not >>>>>> have everything in one file. Why not have it as it is? >>>>> >>>>> Why spread the prcmu code when it is related to the prcmu ? >>>>> >>>>> Linus ? What do you think ? >>>>> >>>>>> - why the gic_mask function? >>>>> >>>>> Because the register has 31 bits reserved which could be used later >>>>> without modifying this function. >>>> >>>> This is will most likely not happen. If that happens we could add that >>>> function then. We should remove it for now. >>>>>> >>>>>> - The original code has been updated and now looks like this: >>>>> >>>>> That is the same code, except the while loop where, if this code >>>>> assumption is correct, means we will do only one iteration in the loop. >>>> >>>> Yes, but in theory you can be stuck forever in that loop, so please >>>> remove the loop. >>> >>> Mmmh, can you elaborate please ? I think a comment and/or an error >>> should be returned in this case, no ? >>> >> We do not need the loop. PRCMU FW does decouple of the GIC and if they >> for some reason does that at the same time (they should not) we will be >> stuck. > > > Sorry, but I still don't get it. What is "they" ? Do you mean one cpu can > decouples the cpu while the other cpu recouples it ? > Normally, the cpu decouples the gic and the prcmu recouples it so if cpu decouples the gic and the prcmu recouples it between the write and the read access, you will never read the value PRCMU_A9_MASK_REQ_MASK and stay in your loop. This should not occurs because the prcmu has no reason to recouple at this moment > Taking the routine, why is needed the "read" if we don't care the return > value ? IMHO, the read access is used to ensure that your write access has really been done in the register and is not still in the system bus > > > void ux500_pm_gic_decouple(void) > { > prcmu_write_masked(PRCM_A9_MASK_REQ, > PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ, > PRCM_A9_MASK_REQ_PRCM_A9_MASK_REQ); > > (void)prcmu_read(PRCM_A9_MASK_REQ); > > udelay(GIC_FREEZE_DELAY); /* Wait for the GIC to freeze */ > } > > Thanks > -- Daniel > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog > > > _______________________________________________ > linaro-dev mailing list > linaro-dev@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c index af8e0ef..c708431 100644 --- a/drivers/mfd/db8500-prcmu.c +++ b/drivers/mfd/db8500-prcmu.c @@ -528,6 +528,10 @@ static const char *hwacc_ret_regulator_name[NUM_HW_ACC] = { #define PRCMU_PLLDSI_LOCKP_LOCKED 0x3 +#define PRCMU_A9_MASK_REQ 0x00000328 +#define PRCMU_A9_MASK_REQ_MASK 0x00000001 +#define PRCMU_A9_MASK_ACK 0x0000032C + static struct { u8 project_number; u8 api_version; @@ -787,6 +791,44 @@ int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll) return 0; } +/* + * prcmu_gic_mask - apply the mask to the mask request + * register. This mask has the bits [1-31] reserved and + * the request applies to the bit 0 of this register. + */ +static inline void prcmu_gic_mask(u32 mask) +{ + u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ); + writel((val & ~PRCMU_A9_MASK_REQ_MASK) | mask, + _PRCMU_BASE + PRCMU_A9_MASK_REQ); +} + +/* + * db8500_prcmu_gic_disable - This function decouple the gic + * from the prcmu. + */ +void db8500_prcmu_gic_disable(void) +{ + prcmu_gic_mask(PRCMU_A9_MASK_REQ_MASK); + + /* Wait for gic mask register update */ + while (!(readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ) & + PRCMU_A9_MASK_REQ_MASK)) + cpu_relax(); + + /* Wait a few cycles for the gic mask completion */ + udelay(1); +} + +/* + * db8500_prcmu_gic_enable - This function recouple the gic + * with the prcmu. + */ +void db8500_prcmu_gic_enable(void) +{ + prcmu_gic_mask(0); +} + /* This function should only be called while mb0_transfer.lock is held. */ static void config_wakeups(void) { diff --git a/include/linux/mfd/db8500-prcmu.h b/include/linux/mfd/db8500-prcmu.h index 60d27f7..4a1032c 100644 --- a/include/linux/mfd/db8500-prcmu.h +++ b/include/linux/mfd/db8500-prcmu.h @@ -536,6 +536,8 @@ int prcmu_load_a9wdog(u8 id, u32 val); void db8500_prcmu_system_reset(u16 reset_code); int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll); +void db8500_prcmu_gic_disable(void); +void db8500_prcmu_gic_enable(void); void db8500_prcmu_enable_wakeups(u32 wakeups); int db8500_prcmu_set_epod(u16 epod_id, u8 epod_state); int db8500_prcmu_request_clock(u8 clock, bool enable); diff --git a/include/linux/mfd/dbx500-prcmu.h b/include/linux/mfd/dbx500-prcmu.h index bac942f..a5fee69 100644 --- a/include/linux/mfd/dbx500-prcmu.h +++ b/include/linux/mfd/dbx500-prcmu.h @@ -237,6 +237,22 @@ static inline int prcmu_set_power_state(u8 state, bool keep_ulp_clk, keep_ap_pll); } +static inline void prcmu_gic_disable(void) +{ + if (machine_is_u5500()) + return; + else + return db8500_prcmu_gic_disable(); +} + +static inline void prcmu_gic_enable(void) +{ + if (machine_is_u5500()) + return; + else + return db8500_prcmu_gic_enable(); +} + static inline int prcmu_set_epod(u16 epod_id, u8 epod_state) { if (machine_is_u5500())
This patch allows to decouple and recouple the gic from the PRCMU. This is needed to put the A9 core in retention mode with the cpuidle driver. Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/mfd/db8500-prcmu.c | 42 ++++++++++++++++++++++++++++++++++++++ include/linux/mfd/db8500-prcmu.h | 2 + include/linux/mfd/dbx500-prcmu.h | 16 ++++++++++++++ 3 files changed, 60 insertions(+), 0 deletions(-)