| Message ID | 1608123453-1423-1-git-send-email-claudiu.beznea@microchip.com |
|---|---|
| Headers | show |
| Series | power: reset: at91-sama5d2_shdwc: add support for SAMA7G5 | expand |
On 16/12/2020 14:57:31+0200, Claudiu Beznea wrote: > According to datasheet WKUPDBC mask is b/w bits 26..24. > > Fixes: f80cb48843987 ("power: reset: at91-shdwc: add new shutdown controller driver") > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > drivers/power/reset/at91-sama5d2_shdwc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c > index e90368ed6f71..3996167f676f 100644 > --- a/drivers/power/reset/at91-sama5d2_shdwc.c > +++ b/drivers/power/reset/at91-sama5d2_shdwc.c > @@ -37,7 +37,7 @@ > > #define AT91_SHDW_MR 0x04 /* Shut Down Mode Register */ > #define AT91_SHDW_WKUPDBC_SHIFT 24 > -#define AT91_SHDW_WKUPDBC_MASK GENMASK(31, 16) > +#define AT91_SHDW_WKUPDBC_MASK GENMASK(26, 24) > #define AT91_SHDW_WKUPDBC(x) (((x) << AT91_SHDW_WKUPDBC_SHIFT) \ > & AT91_SHDW_WKUPDBC_MASK) > Maybe you would switch to FIELD_PREP to remove the need for 2 of those 3 macros and would have made the issue apparent right away.
On 16/12/2020 14:57:33+0200, Claudiu Beznea wrote: > Add support for SAMA7G5 by adding proper struct reg_config structure > and since SAMA7G5 is not currently on LPDDR setups the commit also > avoid the mapping of DDR controller. > Honestly, I wouldn't leave the LPDDR part out because there is no guarantee anyone will think about it when they have a design with LPDDR and as a consequence, their device will behave properly but will be very short lived. > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > drivers/power/reset/at91-sama5d2_shdwc.c | 72 ++++++++++++++++++++++++-------- > 1 file changed, 54 insertions(+), 18 deletions(-) > > diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c > index 3996167f676f..a3342c8c3728 100644 > --- a/drivers/power/reset/at91-sama5d2_shdwc.c > +++ b/drivers/power/reset/at91-sama5d2_shdwc.c > @@ -78,9 +78,15 @@ struct pmc_reg_config { > u8 mckr; > }; > > +struct ddrc_reg_config { > + u32 type_offset; > + u32 type_mask; > +}; > + > struct reg_config { > struct shdwc_reg_config shdwc; > struct pmc_reg_config pmc; > + struct ddrc_reg_config ddrc; > }; > > struct shdwc { > @@ -262,6 +268,10 @@ static const struct reg_config sama5d2_reg_config = { > .pmc = { > .mckr = 0x30, > }, > + .ddrc = { > + .type_offset = AT91_DDRSDRC_MDR, > + .type_mask = AT91_DDRSDRC_MD > + }, > }; > > static const struct reg_config sam9x60_reg_config = { > @@ -275,6 +285,23 @@ static const struct reg_config sam9x60_reg_config = { > .pmc = { > .mckr = 0x28, > }, > + .ddrc = { > + .type_offset = AT91_DDRSDRC_MDR, > + .type_mask = AT91_DDRSDRC_MD > + }, > +}; > + > +static const struct reg_config sama7g5_reg_config = { > + .shdwc = { > + .wkup_pin_input = 0, > + .mr_rtcwk_shift = 17, > + .mr_rttwk_shift = 16, > + .sr_rtcwk_shift = 5, > + .sr_rttwk_shift = 4, > + }, > + .pmc = { > + .mckr = 0x28, > + }, > }; > > static const struct of_device_id at91_shdwc_of_match[] = { > @@ -285,6 +312,10 @@ static const struct of_device_id at91_shdwc_of_match[] = { > { > .compatible = "microchip,sam9x60-shdwc", > .data = &sam9x60_reg_config, > + }, > + { > + .compatible = "microchip,sama7g5-shdwc", > + .data = &sama7g5_reg_config, > }, { > /*sentinel*/ > } > @@ -294,6 +325,7 @@ MODULE_DEVICE_TABLE(of, at91_shdwc_of_match); > static const struct of_device_id at91_pmc_ids[] = { > { .compatible = "atmel,sama5d2-pmc" }, > { .compatible = "microchip,sam9x60-pmc" }, > + { .compatible = "microchip,sama7g5-pmc" }, > { /* Sentinel. */ } > }; > > @@ -355,30 +387,34 @@ static int __init at91_shdwc_probe(struct platform_device *pdev) > goto clk_disable; > } > > - np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc"); > - if (!np) { > - ret = -ENODEV; > - goto unmap; > - } > + if (at91_shdwc->rcfg->ddrc.type_mask) { > + np = of_find_compatible_node(NULL, NULL, > + "atmel,sama5d3-ddramc"); > + if (!np) { > + ret = -ENODEV; > + goto unmap; > + } > > - at91_shdwc->mpddrc_base = of_iomap(np, 0); > - of_node_put(np); > + at91_shdwc->mpddrc_base = of_iomap(np, 0); > + of_node_put(np); > > - if (!at91_shdwc->mpddrc_base) { > - ret = -ENOMEM; > - goto unmap; > + if (!at91_shdwc->mpddrc_base) { > + ret = -ENOMEM; > + goto unmap; > + } > + > + ddr_type = readl(at91_shdwc->mpddrc_base + > + at91_shdwc->rcfg->ddrc.type_offset) & > + at91_shdwc->rcfg->ddrc.type_mask; > + if (ddr_type != AT91_DDRSDRC_MD_LPDDR2 && > + ddr_type != AT91_DDRSDRC_MD_LPDDR3) { > + iounmap(at91_shdwc->mpddrc_base); > + at91_shdwc->mpddrc_base = NULL; > + } > } > > pm_power_off = at91_poweroff; > > - ddr_type = readl(at91_shdwc->mpddrc_base + AT91_DDRSDRC_MDR) & > - AT91_DDRSDRC_MD; > - if (ddr_type != AT91_DDRSDRC_MD_LPDDR2 && > - ddr_type != AT91_DDRSDRC_MD_LPDDR3) { > - iounmap(at91_shdwc->mpddrc_base); > - at91_shdwc->mpddrc_base = NULL; > - } > - > return 0; > > unmap: > -- > 2.7.4 >
On 16.12.2020 15:45, Alexandre Belloni wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 16/12/2020 14:57:33+0200, Claudiu Beznea wrote: >> Add support for SAMA7G5 by adding proper struct reg_config structure >> and since SAMA7G5 is not currently on LPDDR setups the commit also >> avoid the mapping of DDR controller. >> > > Honestly, I wouldn't leave the LPDDR part out because there is no > guarantee anyone will think about it when they have a design with LPDDR > and as a consequence, their device will behave properly but will be > very short lived. The idea wasn't to leave it out. It has to be handled differently on SAMA7G5 as it embeds a different DDR controller. Thank you, Claudiu > >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> --- >> drivers/power/reset/at91-sama5d2_shdwc.c | 72 ++++++++++++++++++++++++-------- >> 1 file changed, 54 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c >> index 3996167f676f..a3342c8c3728 100644 >> --- a/drivers/power/reset/at91-sama5d2_shdwc.c >> +++ b/drivers/power/reset/at91-sama5d2_shdwc.c >> @@ -78,9 +78,15 @@ struct pmc_reg_config { >> u8 mckr; >> }; >> >> +struct ddrc_reg_config { >> + u32 type_offset; >> + u32 type_mask; >> +}; >> + >> struct reg_config { >> struct shdwc_reg_config shdwc; >> struct pmc_reg_config pmc; >> + struct ddrc_reg_config ddrc; >> }; >> >> struct shdwc { >> @@ -262,6 +268,10 @@ static const struct reg_config sama5d2_reg_config = { >> .pmc = { >> .mckr = 0x30, >> }, >> + .ddrc = { >> + .type_offset = AT91_DDRSDRC_MDR, >> + .type_mask = AT91_DDRSDRC_MD >> + }, >> }; >> >> static const struct reg_config sam9x60_reg_config = { >> @@ -275,6 +285,23 @@ static const struct reg_config sam9x60_reg_config = { >> .pmc = { >> .mckr = 0x28, >> }, >> + .ddrc = { >> + .type_offset = AT91_DDRSDRC_MDR, >> + .type_mask = AT91_DDRSDRC_MD >> + }, >> +}; >> + >> +static const struct reg_config sama7g5_reg_config = { >> + .shdwc = { >> + .wkup_pin_input = 0, >> + .mr_rtcwk_shift = 17, >> + .mr_rttwk_shift = 16, >> + .sr_rtcwk_shift = 5, >> + .sr_rttwk_shift = 4, >> + }, >> + .pmc = { >> + .mckr = 0x28, >> + }, >> }; >> >> static const struct of_device_id at91_shdwc_of_match[] = { >> @@ -285,6 +312,10 @@ static const struct of_device_id at91_shdwc_of_match[] = { >> { >> .compatible = "microchip,sam9x60-shdwc", >> .data = &sam9x60_reg_config, >> + }, >> + { >> + .compatible = "microchip,sama7g5-shdwc", >> + .data = &sama7g5_reg_config, >> }, { >> /*sentinel*/ >> } >> @@ -294,6 +325,7 @@ MODULE_DEVICE_TABLE(of, at91_shdwc_of_match); >> static const struct of_device_id at91_pmc_ids[] = { >> { .compatible = "atmel,sama5d2-pmc" }, >> { .compatible = "microchip,sam9x60-pmc" }, >> + { .compatible = "microchip,sama7g5-pmc" }, >> { /* Sentinel. */ } >> }; >> >> @@ -355,30 +387,34 @@ static int __init at91_shdwc_probe(struct platform_device *pdev) >> goto clk_disable; >> } >> >> - np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc"); >> - if (!np) { >> - ret = -ENODEV; >> - goto unmap; >> - } >> + if (at91_shdwc->rcfg->ddrc.type_mask) { >> + np = of_find_compatible_node(NULL, NULL, >> + "atmel,sama5d3-ddramc"); >> + if (!np) { >> + ret = -ENODEV; >> + goto unmap; >> + } >> >> - at91_shdwc->mpddrc_base = of_iomap(np, 0); >> - of_node_put(np); >> + at91_shdwc->mpddrc_base = of_iomap(np, 0); >> + of_node_put(np); >> >> - if (!at91_shdwc->mpddrc_base) { >> - ret = -ENOMEM; >> - goto unmap; >> + if (!at91_shdwc->mpddrc_base) { >> + ret = -ENOMEM; >> + goto unmap; >> + } >> + >> + ddr_type = readl(at91_shdwc->mpddrc_base + >> + at91_shdwc->rcfg->ddrc.type_offset) & >> + at91_shdwc->rcfg->ddrc.type_mask; >> + if (ddr_type != AT91_DDRSDRC_MD_LPDDR2 && >> + ddr_type != AT91_DDRSDRC_MD_LPDDR3) { >> + iounmap(at91_shdwc->mpddrc_base); >> + at91_shdwc->mpddrc_base = NULL; >> + } >> } >> >> pm_power_off = at91_poweroff; >> >> - ddr_type = readl(at91_shdwc->mpddrc_base + AT91_DDRSDRC_MDR) & >> - AT91_DDRSDRC_MD; >> - if (ddr_type != AT91_DDRSDRC_MD_LPDDR2 && >> - ddr_type != AT91_DDRSDRC_MD_LPDDR3) { >> - iounmap(at91_shdwc->mpddrc_base); >> - at91_shdwc->mpddrc_base = NULL; >> - } >> - >> return 0; >> >> unmap: >> -- >> 2.7.4 >> > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >
On 16.12.2020 15:45, Alexandre Belloni wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 16/12/2020 14:57:33+0200, Claudiu Beznea wrote: >> Add support for SAMA7G5 by adding proper struct reg_config structure >> and since SAMA7G5 is not currently on LPDDR setups the commit also >> avoid the mapping of DDR controller. >> > > Honestly, I wouldn't leave the LPDDR part out because there is no > guarantee anyone will think about it when they have a design with LPDDR > and as a consequence, their device will behave properly but will be > very short lived. The idea wasn't to leave it out. It has to be handled differently on SAMA7G5 as it embeds a different DDR controller and at the moment we have no setup with LPDDR. Thank you, Claudiu > >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> --- >> drivers/power/reset/at91-sama5d2_shdwc.c | 72 ++++++++++++++++++++++++-------- >> 1 file changed, 54 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c >> index 3996167f676f..a3342c8c3728 100644 >> --- a/drivers/power/reset/at91-sama5d2_shdwc.c >> +++ b/drivers/power/reset/at91-sama5d2_shdwc.c >> @@ -78,9 +78,15 @@ struct pmc_reg_config { >> u8 mckr; >> }; >> >> +struct ddrc_reg_config { >> + u32 type_offset; >> + u32 type_mask; >> +}; >> + >> struct reg_config { >> struct shdwc_reg_config shdwc; >> struct pmc_reg_config pmc; >> + struct ddrc_reg_config ddrc; >> }; >> >> struct shdwc { >> @@ -262,6 +268,10 @@ static const struct reg_config sama5d2_reg_config = { >> .pmc = { >> .mckr = 0x30, >> }, >> + .ddrc = { >> + .type_offset = AT91_DDRSDRC_MDR, >> + .type_mask = AT91_DDRSDRC_MD >> + }, >> }; >> >> static const struct reg_config sam9x60_reg_config = { >> @@ -275,6 +285,23 @@ static const struct reg_config sam9x60_reg_config = { >> .pmc = { >> .mckr = 0x28, >> }, >> + .ddrc = { >> + .type_offset = AT91_DDRSDRC_MDR, >> + .type_mask = AT91_DDRSDRC_MD >> + }, >> +}; >> + >> +static const struct reg_config sama7g5_reg_config = { >> + .shdwc = { >> + .wkup_pin_input = 0, >> + .mr_rtcwk_shift = 17, >> + .mr_rttwk_shift = 16, >> + .sr_rtcwk_shift = 5, >> + .sr_rttwk_shift = 4, >> + }, >> + .pmc = { >> + .mckr = 0x28, >> + }, >> }; >> >> static const struct of_device_id at91_shdwc_of_match[] = { >> @@ -285,6 +312,10 @@ static const struct of_device_id at91_shdwc_of_match[] = { >> { >> .compatible = "microchip,sam9x60-shdwc", >> .data = &sam9x60_reg_config, >> + }, >> + { >> + .compatible = "microchip,sama7g5-shdwc", >> + .data = &sama7g5_reg_config, >> }, { >> /*sentinel*/ >> } >> @@ -294,6 +325,7 @@ MODULE_DEVICE_TABLE(of, at91_shdwc_of_match); >> static const struct of_device_id at91_pmc_ids[] = { >> { .compatible = "atmel,sama5d2-pmc" }, >> { .compatible = "microchip,sam9x60-pmc" }, >> + { .compatible = "microchip,sama7g5-pmc" }, >> { /* Sentinel. */ } >> }; >> >> @@ -355,30 +387,34 @@ static int __init at91_shdwc_probe(struct platform_device *pdev) >> goto clk_disable; >> } >> >> - np = of_find_compatible_node(NULL, NULL, "atmel,sama5d3-ddramc"); >> - if (!np) { >> - ret = -ENODEV; >> - goto unmap; >> - } >> + if (at91_shdwc->rcfg->ddrc.type_mask) { >> + np = of_find_compatible_node(NULL, NULL, >> + "atmel,sama5d3-ddramc"); >> + if (!np) { >> + ret = -ENODEV; >> + goto unmap; >> + } >> >> - at91_shdwc->mpddrc_base = of_iomap(np, 0); >> - of_node_put(np); >> + at91_shdwc->mpddrc_base = of_iomap(np, 0); >> + of_node_put(np); >> >> - if (!at91_shdwc->mpddrc_base) { >> - ret = -ENOMEM; >> - goto unmap; >> + if (!at91_shdwc->mpddrc_base) { >> + ret = -ENOMEM; >> + goto unmap; >> + } >> + >> + ddr_type = readl(at91_shdwc->mpddrc_base + >> + at91_shdwc->rcfg->ddrc.type_offset) & >> + at91_shdwc->rcfg->ddrc.type_mask; >> + if (ddr_type != AT91_DDRSDRC_MD_LPDDR2 && >> + ddr_type != AT91_DDRSDRC_MD_LPDDR3) { >> + iounmap(at91_shdwc->mpddrc_base); >> + at91_shdwc->mpddrc_base = NULL; >> + } >> } >> >> pm_power_off = at91_poweroff; >> >> - ddr_type = readl(at91_shdwc->mpddrc_base + AT91_DDRSDRC_MDR) & >> - AT91_DDRSDRC_MD; >> - if (ddr_type != AT91_DDRSDRC_MD_LPDDR2 && >> - ddr_type != AT91_DDRSDRC_MD_LPDDR3) { >> - iounmap(at91_shdwc->mpddrc_base); >> - at91_shdwc->mpddrc_base = NULL; >> - } >> - >> return 0; >> >> unmap: >> -- >> 2.7.4 >> > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >
Hi, On Wed, Dec 16, 2020 at 02:57:30PM +0200, Claudiu Beznea wrote: > This series adds support for SAMA7G5 shutdown controller. > > Thank you, > Claudiu Beznea > > Claudiu Beznea (3): > power: reset: at91-sama5d2_shdwc: fix wkupdbc mask > dt-bindings: atmel-sysreg: add microchip,sama7g5-shdwc > power: reset: at91-sama5d2_shdwc: add support for sama7g5 > > .../devicetree/bindings/arm/atmel-sysregs.txt | 5 +- > drivers/power/reset/at91-sama5d2_shdwc.c | 74 ++++++++++++++++------ > 2 files changed, 58 insertions(+), 21 deletions(-) Thanks, queued. -- Sebastian