diff mbox series

[v2,12/12] watchdog: s3c2410: Add Exynos850 support

Message ID 20211031122216.30212-13-semen.protsenko@linaro.org (mailing list archive)
State Superseded
Headers show
Series watchdog: s3c2410: Add Exynos850 support | expand

Commit Message

Sam Protsenko Oct. 31, 2021, 12:22 p.m. UTC
Exynos850 is a bit different from SoCs already supported in WDT driver:
  - AUTOMATIC_WDT_RESET_DISABLE register is removed, so its value is
    always 0; .disable_auto_reset callback is not set for that reason
  - MASK_WDT_RESET_REQUEST register is replaced with
    CLUSTERx_NONCPU_IN_EN register; instead of masking (disabling) WDT
    reset interrupt it's now enabled with the same value; .mask_reset
    callback is reused for that functionality though
  - To make WDT functional, WDT counter needs to be enabled in
    CLUSTERx_NONCPU_OUT register; it's done using .enable_counter
    callback

Also Exynos850 has two CPU clusters, each has its own dedicated WDT
instance. Different PMU registers and bits are used for each cluster. So
driver data is now modified in probe, adding needed info depending on
cluster index passed from device tree.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
  - Used single compatible for Exynos850, populating missing driver data in
    probe
  - Added "index" property to specify CPU cluster index

 drivers/watchdog/s3c2410_wdt.c | 68 +++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 2 deletions(-)

Comments

kernel test robot Oct. 31, 2021, 3:15 p.m. UTC | #1
Hi Sam,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on groeck-staging/hwmon-next linus/master v5.15-rc7 next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sam-Protsenko/watchdog-s3c2410-Add-Exynos850-support/20211031-202352
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: hexagon-randconfig-r045-20211031 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 82ed106567063ea269c6d5669278b733e173a42f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ff70b806cc12adc068bb54865f81d3aa29a69471
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sam-Protsenko/watchdog-s3c2410-Add-Exynos850-support/20211031-202352
        git checkout ff70b806cc12adc068bb54865f81d3aa29a69471
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/watchdog/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/watchdog/s3c2410_wdt.c:592:18: error: use of undeclared identifier 'drv_data_exynos850'
           if (variant == &drv_data_exynos850) {
                           ^
   1 error generated.


vim +/drv_data_exynos850 +592 drivers/watchdog/s3c2410_wdt.c

   571	
   572	static inline const struct s3c2410_wdt_variant *
   573	s3c2410_get_wdt_drv_data(struct platform_device *pdev)
   574	{
   575		const struct s3c2410_wdt_variant *variant;
   576		struct s3c2410_wdt_variant *data;
   577		struct device *dev = &pdev->dev;
   578	
   579		variant = of_device_get_match_data(dev);
   580		if (!variant) {
   581			/* Device matched by platform_device_id */
   582			variant = (struct s3c2410_wdt_variant *)
   583				   platform_get_device_id(pdev)->driver_data;
   584		}
   585	
   586		/* Have to copy driver data over to keep its const qualifier intact */
   587		data = devm_kmemdup(dev, variant, sizeof(*variant), GFP_KERNEL);
   588		if (!data)
   589			return NULL;
   590	
   591		/* Populate missing fields for Exynos850 w.r.t. cluster index */
 > 592		if (variant == &drv_data_exynos850) {
   593			u32 index;
   594			int err;
   595	
   596			err = of_property_read_u32(dev->of_node, "samsung,index",
   597						   &index);
   598			if (err) {
   599				dev_err(dev, "failed to get cluster index\n");
   600				return NULL;
   601			}
   602	
   603			switch (index) {
   604			case 0:
   605				data->mask_reset_reg = EXYNOS850_CLUSTER0_NONCPU_INT_EN;
   606				data->rst_stat_bit = EXYNOS850_CLUSTER0_WDTRESET_BIT;
   607				data->cnt_en_reg = EXYNOS850_CLUSTER0_NONCPU_OUT;
   608				break;
   609			case 1:
   610				data->mask_reset_reg = EXYNOS850_CLUSTER1_NONCPU_INT_EN;
   611				data->rst_stat_bit = EXYNOS850_CLUSTER1_WDTRESET_BIT;
   612				data->cnt_en_reg = EXYNOS850_CLUSTER1_NONCPU_OUT;
   613				break;
   614			default:
   615				dev_err(dev, "wrong cluster index: %u\n", index);
   616				return NULL;
   617			}
   618		}
   619	
   620		return data;
   621	}
   622	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Oct. 31, 2021, 4:09 p.m. UTC | #2
Hi Sam,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on groeck-staging/hwmon-next linus/master v5.15-rc7 next-20211029]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sam-Protsenko/watchdog-s3c2410-Add-Exynos850-support/20211031-202352
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: xtensa-randconfig-r012-20211031 (attached as .config)
compiler: xtensa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ff70b806cc12adc068bb54865f81d3aa29a69471
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sam-Protsenko/watchdog-s3c2410-Add-Exynos850-support/20211031-202352
        git checkout ff70b806cc12adc068bb54865f81d3aa29a69471
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash drivers/watchdog/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/watchdog/s3c2410_wdt.c: In function 's3c2410_get_wdt_drv_data':
>> drivers/watchdog/s3c2410_wdt.c:592:25: error: 'drv_data_exynos850' undeclared (first use in this function)
     592 |         if (variant == &drv_data_exynos850) {
         |                         ^~~~~~~~~~~~~~~~~~
   drivers/watchdog/s3c2410_wdt.c:592:25: note: each undeclared identifier is reported only once for each function it appears in


vim +/drv_data_exynos850 +592 drivers/watchdog/s3c2410_wdt.c

   571	
   572	static inline const struct s3c2410_wdt_variant *
   573	s3c2410_get_wdt_drv_data(struct platform_device *pdev)
   574	{
   575		const struct s3c2410_wdt_variant *variant;
   576		struct s3c2410_wdt_variant *data;
   577		struct device *dev = &pdev->dev;
   578	
   579		variant = of_device_get_match_data(dev);
   580		if (!variant) {
   581			/* Device matched by platform_device_id */
   582			variant = (struct s3c2410_wdt_variant *)
   583				   platform_get_device_id(pdev)->driver_data;
   584		}
   585	
   586		/* Have to copy driver data over to keep its const qualifier intact */
   587		data = devm_kmemdup(dev, variant, sizeof(*variant), GFP_KERNEL);
   588		if (!data)
   589			return NULL;
   590	
   591		/* Populate missing fields for Exynos850 w.r.t. cluster index */
 > 592		if (variant == &drv_data_exynos850) {
   593			u32 index;
   594			int err;
   595	
   596			err = of_property_read_u32(dev->of_node, "samsung,index",
   597						   &index);
   598			if (err) {
   599				dev_err(dev, "failed to get cluster index\n");
   600				return NULL;
   601			}
   602	
   603			switch (index) {
   604			case 0:
   605				data->mask_reset_reg = EXYNOS850_CLUSTER0_NONCPU_INT_EN;
   606				data->rst_stat_bit = EXYNOS850_CLUSTER0_WDTRESET_BIT;
   607				data->cnt_en_reg = EXYNOS850_CLUSTER0_NONCPU_OUT;
   608				break;
   609			case 1:
   610				data->mask_reset_reg = EXYNOS850_CLUSTER1_NONCPU_INT_EN;
   611				data->rst_stat_bit = EXYNOS850_CLUSTER1_WDTRESET_BIT;
   612				data->cnt_en_reg = EXYNOS850_CLUSTER1_NONCPU_OUT;
   613				break;
   614			default:
   615				dev_err(dev, "wrong cluster index: %u\n", index);
   616				return NULL;
   617			}
   618		}
   619	
   620		return data;
   621	}
   622	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Krzysztof Kozlowski Nov. 2, 2021, 10:26 a.m. UTC | #3
On 31/10/2021 13:22, Sam Protsenko wrote:
> Exynos850 is a bit different from SoCs already supported in WDT driver:
>   - AUTOMATIC_WDT_RESET_DISABLE register is removed, so its value is
>     always 0; .disable_auto_reset callback is not set for that reason
>   - MASK_WDT_RESET_REQUEST register is replaced with
>     CLUSTERx_NONCPU_IN_EN register; instead of masking (disabling) WDT
>     reset interrupt it's now enabled with the same value; .mask_reset
>     callback is reused for that functionality though
>   - To make WDT functional, WDT counter needs to be enabled in
>     CLUSTERx_NONCPU_OUT register; it's done using .enable_counter
>     callback
> 
> Also Exynos850 has two CPU clusters, each has its own dedicated WDT
> instance. Different PMU registers and bits are used for each cluster. So
> driver data is now modified in probe, adding needed info depending on
> cluster index passed from device tree.
> 
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v2:
>   - Used single compatible for Exynos850, populating missing driver data in
>     probe
>   - Added "index" property to specify CPU cluster index
> 
>  drivers/watchdog/s3c2410_wdt.c | 68 +++++++++++++++++++++++++++++++++-
>  1 file changed, 66 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 8fdda2ede1c3..457b725c30ac 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -56,6 +56,14 @@
>  #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
>  #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
>  #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c
> +#define EXYNOS850_CLUSTER0_NONCPU_OUT		0x1220
> +#define EXYNOS850_CLUSTER0_NONCPU_INT_EN	0x1244
> +#define EXYNOS850_CLUSTER1_NONCPU_OUT		0x1620
> +#define EXYNOS850_CLUSTER1_NONCPU_INT_EN	0x1644
> +
> +#define EXYNOS850_CLUSTER0_WDTRESET_BIT		24
> +#define EXYNOS850_CLUSTER1_WDTRESET_BIT		23
> +
>  #define QUIRK_HAS_WTCLRINT_REG			(1 << 0)
>  #define QUIRK_HAS_PMU_MASK_RESET		(1 << 1)
>  #define QUIRK_HAS_PMU_RST_STAT			(1 << 2)
> @@ -171,6 +179,21 @@ static const struct s3c2410_wdt_variant drv_data_exynos7 = {
>  		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
>  };
>  
> +static const struct s3c2410_wdt_variant drv_data_exynos850 = {
> +	/*
> +	 * Next fields will be set in probe(), based on cluster index:
> +	 *   - .mask_reset_reg
> +	 *   - .rst_stat_bit
> +	 *   - .cnt_en_reg
> +	 */
> +	.mask_reset_inv = true,
> +	.mask_bit = 2,
> +	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> +	.cnt_en_bit = 7,
> +	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> +		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
> +};
> +
>  static const struct of_device_id s3c2410_wdt_match[] = {
>  	{ .compatible = "samsung,s3c2410-wdt",
>  	  .data = &drv_data_s3c2410 },
> @@ -182,6 +205,8 @@ static const struct of_device_id s3c2410_wdt_match[] = {
>  	  .data = &drv_data_exynos5420 },
>  	{ .compatible = "samsung,exynos7-wdt",
>  	  .data = &drv_data_exynos7 },
> +	{ .compatible = "samsung,exynos850-wdt",
> +	  .data = &drv_data_exynos850 },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
> @@ -548,15 +573,51 @@ static inline const struct s3c2410_wdt_variant *
>  s3c2410_get_wdt_drv_data(struct platform_device *pdev)
>  {
>  	const struct s3c2410_wdt_variant *variant;
> +	struct s3c2410_wdt_variant *data;
> +	struct device *dev = &pdev->dev;
>  
> -	variant = of_device_get_match_data(&pdev->dev);
> +	variant = of_device_get_match_data(dev);
>  	if (!variant) {
>  		/* Device matched by platform_device_id */
>  		variant = (struct s3c2410_wdt_variant *)
>  			   platform_get_device_id(pdev)->driver_data;
>  	}
>  
> -	return variant;
> +	/* Have to copy driver data over to keep its const qualifier intact */
> +	data = devm_kmemdup(dev, variant, sizeof(*variant), GFP_KERNEL);
> +	if (!data)
> +		return NULL;
> +
> +	/* Populate missing fields for Exynos850 w.r.t. cluster index */
> +	if (variant == &drv_data_exynos850) {
> +		u32 index;
> +		int err;

Another approach is to:
1. Define two variants for Exynos850 (s3c2410_wdt_variants), kind of
like before,
2. if (variant == &drv_data_exynos850)
a. Read the index
b. If index is 0, return first variant,
c. If index is 1, return the second variant,
d. Else - NULL.

This way you won't need to copy the memory on the fly, just use
different const data. Benefits: less memory allocations, entire drvdata
set in one place (so nicely visible), drvdata populated safely via const.

> +
> +		err = of_property_read_u32(dev->of_node, "samsung,index",
> +					   &index);
> +		if (err) {
> +			dev_err(dev, "failed to get cluster index\n");
> +			return NULL;
> +		}
> +
> +		switch (index) {
> +		case 0:
> +			data->mask_reset_reg = EXYNOS850_CLUSTER0_NONCPU_INT_EN;
> +			data->rst_stat_bit = EXYNOS850_CLUSTER0_WDTRESET_BIT;
> +			data->cnt_en_reg = EXYNOS850_CLUSTER0_NONCPU_OUT;
> +			break;
> +		case 1:
> +			data->mask_reset_reg = EXYNOS850_CLUSTER1_NONCPU_INT_EN;
> +			data->rst_stat_bit = EXYNOS850_CLUSTER1_WDTRESET_BIT;
> +			data->cnt_en_reg = EXYNOS850_CLUSTER1_NONCPU_OUT;
> +			break;
> +		default:
> +			dev_err(dev, "wrong cluster index: %u\n", index);
> +			return NULL;
> +		}
> +	}
> +
> +	return data;
>  }
>  
>  static int s3c2410wdt_probe(struct platform_device *pdev)
> @@ -576,6 +637,9 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>  	wdt->wdt_device = s3c2410_wdd;
>  
>  	wdt->drv_data = s3c2410_get_wdt_drv_data(pdev);
> +	if (!wdt->drv_data)
> +		return -EINVAL;
> +
>  	if (wdt->drv_data->quirks & QUIRKS_HAVE_PMUREG) {
>  		wdt->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
>  						"samsung,syscon-phandle");
> 


Best regards,
Krzysztof
Sam Protsenko Nov. 7, 2021, 4:17 p.m. UTC | #4
On Tue, 2 Nov 2021 at 12:27, Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 31/10/2021 13:22, Sam Protsenko wrote:
> > Exynos850 is a bit different from SoCs already supported in WDT driver:
> >   - AUTOMATIC_WDT_RESET_DISABLE register is removed, so its value is
> >     always 0; .disable_auto_reset callback is not set for that reason
> >   - MASK_WDT_RESET_REQUEST register is replaced with
> >     CLUSTERx_NONCPU_IN_EN register; instead of masking (disabling) WDT
> >     reset interrupt it's now enabled with the same value; .mask_reset
> >     callback is reused for that functionality though
> >   - To make WDT functional, WDT counter needs to be enabled in
> >     CLUSTERx_NONCPU_OUT register; it's done using .enable_counter
> >     callback
> >
> > Also Exynos850 has two CPU clusters, each has its own dedicated WDT
> > instance. Different PMU registers and bits are used for each cluster. So
> > driver data is now modified in probe, adding needed info depending on
> > cluster index passed from device tree.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> > Changes in v2:
> >   - Used single compatible for Exynos850, populating missing driver data in
> >     probe
> >   - Added "index" property to specify CPU cluster index
> >
> >  drivers/watchdog/s3c2410_wdt.c | 68 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 66 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> > index 8fdda2ede1c3..457b725c30ac 100644
> > --- a/drivers/watchdog/s3c2410_wdt.c
> > +++ b/drivers/watchdog/s3c2410_wdt.c
> > @@ -56,6 +56,14 @@
> >  #define EXYNOS5_RST_STAT_REG_OFFSET          0x0404
> >  #define EXYNOS5_WDT_DISABLE_REG_OFFSET               0x0408
> >  #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET    0x040c
> > +#define EXYNOS850_CLUSTER0_NONCPU_OUT                0x1220
> > +#define EXYNOS850_CLUSTER0_NONCPU_INT_EN     0x1244
> > +#define EXYNOS850_CLUSTER1_NONCPU_OUT                0x1620
> > +#define EXYNOS850_CLUSTER1_NONCPU_INT_EN     0x1644
> > +
> > +#define EXYNOS850_CLUSTER0_WDTRESET_BIT              24
> > +#define EXYNOS850_CLUSTER1_WDTRESET_BIT              23
> > +
> >  #define QUIRK_HAS_WTCLRINT_REG                       (1 << 0)
> >  #define QUIRK_HAS_PMU_MASK_RESET             (1 << 1)
> >  #define QUIRK_HAS_PMU_RST_STAT                       (1 << 2)
> > @@ -171,6 +179,21 @@ static const struct s3c2410_wdt_variant drv_data_exynos7 = {
> >                 QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
> >  };
> >
> > +static const struct s3c2410_wdt_variant drv_data_exynos850 = {
> > +     /*
> > +      * Next fields will be set in probe(), based on cluster index:
> > +      *   - .mask_reset_reg
> > +      *   - .rst_stat_bit
> > +      *   - .cnt_en_reg
> > +      */
> > +     .mask_reset_inv = true,
> > +     .mask_bit = 2,
> > +     .rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
> > +     .cnt_en_bit = 7,
> > +     .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
> > +               QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
> > +};
> > +
> >  static const struct of_device_id s3c2410_wdt_match[] = {
> >       { .compatible = "samsung,s3c2410-wdt",
> >         .data = &drv_data_s3c2410 },
> > @@ -182,6 +205,8 @@ static const struct of_device_id s3c2410_wdt_match[] = {
> >         .data = &drv_data_exynos5420 },
> >       { .compatible = "samsung,exynos7-wdt",
> >         .data = &drv_data_exynos7 },
> > +     { .compatible = "samsung,exynos850-wdt",
> > +       .data = &drv_data_exynos850 },
> >       {},
> >  };
> >  MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
> > @@ -548,15 +573,51 @@ static inline const struct s3c2410_wdt_variant *
> >  s3c2410_get_wdt_drv_data(struct platform_device *pdev)
> >  {
> >       const struct s3c2410_wdt_variant *variant;
> > +     struct s3c2410_wdt_variant *data;
> > +     struct device *dev = &pdev->dev;
> >
> > -     variant = of_device_get_match_data(&pdev->dev);
> > +     variant = of_device_get_match_data(dev);
> >       if (!variant) {
> >               /* Device matched by platform_device_id */
> >               variant = (struct s3c2410_wdt_variant *)
> >                          platform_get_device_id(pdev)->driver_data;
> >       }
> >
> > -     return variant;
> > +     /* Have to copy driver data over to keep its const qualifier intact */
> > +     data = devm_kmemdup(dev, variant, sizeof(*variant), GFP_KERNEL);
> > +     if (!data)
> > +             return NULL;
> > +
> > +     /* Populate missing fields for Exynos850 w.r.t. cluster index */
> > +     if (variant == &drv_data_exynos850) {
> > +             u32 index;
> > +             int err;
>
> Another approach is to:
> 1. Define two variants for Exynos850 (s3c2410_wdt_variants), kind of
> like before,
> 2. if (variant == &drv_data_exynos850)
> a. Read the index
> b. If index is 0, return first variant,
> c. If index is 1, return the second variant,
> d. Else - NULL.
>
> This way you won't need to copy the memory on the fly, just use
> different const data. Benefits: less memory allocations, entire drvdata
> set in one place (so nicely visible), drvdata populated safely via const.
>

That's definitely better. Not sure how I missed that. Anyway, thanks
for review. Will send v3 soon, addressing all your comments (except
the one about src_clk in PATCH 10/12 -- need to discuss it further, I
guess).

> > +
> > +             err = of_property_read_u32(dev->of_node, "samsung,index",
> > +                                        &index);
> > +             if (err) {
> > +                     dev_err(dev, "failed to get cluster index\n");
> > +                     return NULL;
> > +             }
> > +
> > +             switch (index) {
> > +             case 0:
> > +                     data->mask_reset_reg = EXYNOS850_CLUSTER0_NONCPU_INT_EN;
> > +                     data->rst_stat_bit = EXYNOS850_CLUSTER0_WDTRESET_BIT;
> > +                     data->cnt_en_reg = EXYNOS850_CLUSTER0_NONCPU_OUT;
> > +                     break;
> > +             case 1:
> > +                     data->mask_reset_reg = EXYNOS850_CLUSTER1_NONCPU_INT_EN;
> > +                     data->rst_stat_bit = EXYNOS850_CLUSTER1_WDTRESET_BIT;
> > +                     data->cnt_en_reg = EXYNOS850_CLUSTER1_NONCPU_OUT;
> > +                     break;
> > +             default:
> > +                     dev_err(dev, "wrong cluster index: %u\n", index);
> > +                     return NULL;
> > +             }
> > +     }
> > +
> > +     return data;
> >  }
> >
> >  static int s3c2410wdt_probe(struct platform_device *pdev)
> > @@ -576,6 +637,9 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
> >       wdt->wdt_device = s3c2410_wdd;
> >
> >       wdt->drv_data = s3c2410_get_wdt_drv_data(pdev);
> > +     if (!wdt->drv_data)
> > +             return -EINVAL;
> > +
> >       if (wdt->drv_data->quirks & QUIRKS_HAVE_PMUREG) {
> >               wdt->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
> >                                               "samsung,syscon-phandle");
> >
>
>
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 8fdda2ede1c3..457b725c30ac 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -56,6 +56,14 @@ 
 #define EXYNOS5_RST_STAT_REG_OFFSET		0x0404
 #define EXYNOS5_WDT_DISABLE_REG_OFFSET		0x0408
 #define EXYNOS5_WDT_MASK_RESET_REG_OFFSET	0x040c
+#define EXYNOS850_CLUSTER0_NONCPU_OUT		0x1220
+#define EXYNOS850_CLUSTER0_NONCPU_INT_EN	0x1244
+#define EXYNOS850_CLUSTER1_NONCPU_OUT		0x1620
+#define EXYNOS850_CLUSTER1_NONCPU_INT_EN	0x1644
+
+#define EXYNOS850_CLUSTER0_WDTRESET_BIT		24
+#define EXYNOS850_CLUSTER1_WDTRESET_BIT		23
+
 #define QUIRK_HAS_WTCLRINT_REG			(1 << 0)
 #define QUIRK_HAS_PMU_MASK_RESET		(1 << 1)
 #define QUIRK_HAS_PMU_RST_STAT			(1 << 2)
@@ -171,6 +179,21 @@  static const struct s3c2410_wdt_variant drv_data_exynos7 = {
 		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_AUTO_DISABLE,
 };
 
+static const struct s3c2410_wdt_variant drv_data_exynos850 = {
+	/*
+	 * Next fields will be set in probe(), based on cluster index:
+	 *   - .mask_reset_reg
+	 *   - .rst_stat_bit
+	 *   - .cnt_en_reg
+	 */
+	.mask_reset_inv = true,
+	.mask_bit = 2,
+	.rst_stat_reg = EXYNOS5_RST_STAT_REG_OFFSET,
+	.cnt_en_bit = 7,
+	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET | \
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
+};
+
 static const struct of_device_id s3c2410_wdt_match[] = {
 	{ .compatible = "samsung,s3c2410-wdt",
 	  .data = &drv_data_s3c2410 },
@@ -182,6 +205,8 @@  static const struct of_device_id s3c2410_wdt_match[] = {
 	  .data = &drv_data_exynos5420 },
 	{ .compatible = "samsung,exynos7-wdt",
 	  .data = &drv_data_exynos7 },
+	{ .compatible = "samsung,exynos850-wdt",
+	  .data = &drv_data_exynos850 },
 	{},
 };
 MODULE_DEVICE_TABLE(of, s3c2410_wdt_match);
@@ -548,15 +573,51 @@  static inline const struct s3c2410_wdt_variant *
 s3c2410_get_wdt_drv_data(struct platform_device *pdev)
 {
 	const struct s3c2410_wdt_variant *variant;
+	struct s3c2410_wdt_variant *data;
+	struct device *dev = &pdev->dev;
 
-	variant = of_device_get_match_data(&pdev->dev);
+	variant = of_device_get_match_data(dev);
 	if (!variant) {
 		/* Device matched by platform_device_id */
 		variant = (struct s3c2410_wdt_variant *)
 			   platform_get_device_id(pdev)->driver_data;
 	}
 
-	return variant;
+	/* Have to copy driver data over to keep its const qualifier intact */
+	data = devm_kmemdup(dev, variant, sizeof(*variant), GFP_KERNEL);
+	if (!data)
+		return NULL;
+
+	/* Populate missing fields for Exynos850 w.r.t. cluster index */
+	if (variant == &drv_data_exynos850) {
+		u32 index;
+		int err;
+
+		err = of_property_read_u32(dev->of_node, "samsung,index",
+					   &index);
+		if (err) {
+			dev_err(dev, "failed to get cluster index\n");
+			return NULL;
+		}
+
+		switch (index) {
+		case 0:
+			data->mask_reset_reg = EXYNOS850_CLUSTER0_NONCPU_INT_EN;
+			data->rst_stat_bit = EXYNOS850_CLUSTER0_WDTRESET_BIT;
+			data->cnt_en_reg = EXYNOS850_CLUSTER0_NONCPU_OUT;
+			break;
+		case 1:
+			data->mask_reset_reg = EXYNOS850_CLUSTER1_NONCPU_INT_EN;
+			data->rst_stat_bit = EXYNOS850_CLUSTER1_WDTRESET_BIT;
+			data->cnt_en_reg = EXYNOS850_CLUSTER1_NONCPU_OUT;
+			break;
+		default:
+			dev_err(dev, "wrong cluster index: %u\n", index);
+			return NULL;
+		}
+	}
+
+	return data;
 }
 
 static int s3c2410wdt_probe(struct platform_device *pdev)
@@ -576,6 +637,9 @@  static int s3c2410wdt_probe(struct platform_device *pdev)
 	wdt->wdt_device = s3c2410_wdd;
 
 	wdt->drv_data = s3c2410_get_wdt_drv_data(pdev);
+	if (!wdt->drv_data)
+		return -EINVAL;
+
 	if (wdt->drv_data->quirks & QUIRKS_HAVE_PMUREG) {
 		wdt->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
 						"samsung,syscon-phandle");