On 03.11.2015 18:16, Pavel Fedin wrote:
> Implement handling properties in subnodes and adding child devices to the
> system. Child devices will not be added if configuration fails.
> 
> Since the driver now does more than suspend-resume support, dependency on
> CONFIG_PM is removed.
> 
> Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
> ---
>  arch/arm/mach-exynos/Kconfig      |  2 +-
>  drivers/soc/samsung/Kconfig       |  2 +-
>  drivers/soc/samsung/exynos-srom.c | 60 
> +++++++++++++++++++++++++++++++++++++--
>  3 files changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index 83c85f5..c22dc42 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -16,7 +16,7 @@ menuconfig ARCH_EXYNOS
>       select ARM_GIC
>       select COMMON_CLK_SAMSUNG
>       select EXYNOS_THERMAL
> -     select EXYNOS_SROM if PM
> +     select EXYNOS_SROM
>       select HAVE_ARM_SCU if SMP
>       select HAVE_S3C2410_I2C if I2C
>       select HAVE_S3C2410_WATCHDOG if WATCHDOG
> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> index 2833b5b..ea4bc2a 100644
> --- a/drivers/soc/samsung/Kconfig
> +++ b/drivers/soc/samsung/Kconfig
> @@ -8,6 +8,6 @@ config SOC_SAMSUNG
>  
>  config EXYNOS_SROM
>       bool
> -     depends on ARM && ARCH_EXYNOS && PM
> +     depends on ARM && ARCH_EXYNOS
>  
>  endmenu
> diff --git a/drivers/soc/samsung/exynos-srom.c 
> b/drivers/soc/samsung/exynos-srom.c
> index 57a232d..49b5c9e 100644
> --- a/drivers/soc/samsung/exynos-srom.c
> +++ b/drivers/soc/samsung/exynos-srom.c
> @@ -14,6 +14,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> @@ -67,11 +68,50 @@ static struct exynos_srom_reg_dump 
> *exynos_srom_alloc_reg_dump(
>       return rd;
>  }
>  
> +static int decode_sromc(struct exynos_srom *srom, struct device_node *np)

I missed that one previously: add prefix and more descriptive name, like:
exynos_srom_parse_child()

> +{
> +     u32 bank, width, pmc;
> +     u32 timing[6];
> +     u32 cs, bw;
> +
> +     if (of_property_read_u32(np, "reg", &bank))
> +             return -EINVAL;
> +     if (of_property_read_u32(np, "reg-io-width", &width))
> +             width = 1;
> +     if (of_property_read_u32(np, "samsung,srom-page-mode", &pmc))
> +             pmc = 0;
> +     if (of_property_read_u32_array(np, "samsung,srom-timing", timing,
> +                                    ARRAY_SIZE(timing)))
> +             return -EINVAL;
> +
> +     bank *= 4; /* Convert bank into shift/offset */
> +
> +     cs = 1 << EXYNOS_SROM_BW__BYTEENABLE__SHIFT;
> +     if (width == 2)
> +             cs |= 1 << EXYNOS_SROM_BW__DATAWIDTH__SHIFT;
> +
> +     bw = __raw_readl(srom->reg_base + EXYNOS_SROM_BW);
> +     bw = (bw & ~(EXYNOS_SROM_BW__CS_MASK << bank)) | (cs << bank);
> +     __raw_writel(bw, srom->reg_base + EXYNOS_SROM_BW);
> +
> +     __raw_writel((pmc << EXYNOS_SROM_BCX__PMC__SHIFT) |
> +                 (timing[0] << EXYNOS_SROM_BCX__TACP__SHIFT) |
> +                 (timing[1] << EXYNOS_SROM_BCX__TCAH__SHIFT) |
> +                 (timing[2] << EXYNOS_SROM_BCX__TCOH__SHIFT) |
> +                 (timing[3] << EXYNOS_SROM_BCX__TACC__SHIFT) |
> +                 (timing[4] << EXYNOS_SROM_BCX__TCOS__SHIFT) |
> +                 (timing[5] << EXYNOS_SROM_BCX__TACS__SHIFT),
> +                 srom->reg_base + EXYNOS_SROM_BC0 + bank);
> +
> +     return 0;
> +}
> +
>  static int exynos_srom_probe(struct platform_device *pdev)
>  {
> -     struct device_node *np;
> +     struct device_node *np, *child;
>       struct exynos_srom *srom;
>       struct device *dev = &pdev->dev;
> +     bool error = false;

The 'error' name is misleading - like error for entire probe which is
not true.

Instead split it to separate function like:

+static int exynos_srom_parse_children(....) {
+       int ret = 0;
+
+       for_each_child_of_node(np, child) {
+               ret = exynos_srom_parse_child(srom, child);
+               if (ret) {
+                       dev_err(dev,
+                               "Could not decode bank configuration for %s: 
%d\n",
+                               child->name, ret);
+                       break;
+               }
+       }
+
+       return ret;
+}

Best regards,
Krzysztof

>  
>       np = dev->of_node;
>       if (!np) {
> @@ -100,7 +140,23 @@ static int exynos_srom_probe(struct platform_device 
> *pdev)
>               return -ENOMEM;
>       }
>  
> -     return 0;
> +     for_each_child_of_node(np, child) {
> +             if (decode_sromc(srom, child)) {
> +                     dev_err(dev,
> +                             "Could not decode bank configuration for %s\n",
> +                             child->name);
> +                     error = true;
> +             }
> +     }
> +
> +     /*
> +      * If any bank failed to configure, we still provide suspend/resume,
> +      * but do not probe child devices
> +      */
> +     if (error)
> +             return 0;
> +
> +     return of_platform_populate(np, NULL, NULL, dev);
>  }
>  
>  static int exynos_srom_remove(struct platform_device *pdev)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to