Re: [PATCH 3/3] power: reset: at91-sama5d2_shdwc: add support for sama7g5

[<Claudiu.Beznea@microchip.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
>