Linux-Watchdog Archive on lore.kernel.org
 help / Atom feed
* [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h
@ 2019-04-15 20:24 Arnd Bergmann
  2019-04-15 20:54 ` Guenter Roeck
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-04-15 20:24 UTC (permalink / raw)
  To: Greg Ungerer
  Cc: Linus Walleij, arm, Arnd Bergmann, Wim Van Sebroeck,
	Guenter Roeck, linux-arm-kernel, linux-kernel, linux-watchdog

drivers should not rely on machine specific headers but
get their information from the platform device.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-ks8695/devices.c | 13 ++++++++++++-
 drivers/watchdog/Kconfig       |  2 +-
 drivers/watchdog/ks8695_wdt.c  | 30 +++++++++++++++++-------------
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-ks8695/devices.c b/arch/arm/mach-ks8695/devices.c
index 61cf20beb45f..57766817d86f 100644
--- a/arch/arm/mach-ks8695/devices.c
+++ b/arch/arm/mach-ks8695/devices.c
@@ -169,11 +169,22 @@ void __init ks8696_add_device_hpna(void)
 /* --------------------------------------------------------------------
  *  Watchdog
  * -------------------------------------------------------------------- */
+#define KS8695_TMR_OFFSET      (0xF0000 + 0xE400)
+#define KS8695_TMR_PA          (KS8695_IO_PA + KS8695_TMR_OFFSET)
+static struct resource ks8695_wdt_resources[] = {
+	[0] = {
+		.name	= "tmr",
+		.start	= KS8695_TMR_PA,
+		.end	= KS8695_TMR_PA + 0xf,
+		.flags	= IORESOURCE_MEM,
+	},
+};
 
 static struct platform_device ks8695_wdt_device = {
 	.name		= "ks8695_wdt",
 	.id		= -1,
-	.num_resources	= 0,
+	.resource	= ks8695_wdt_resources,
+	.num_resources	= ARRAY_SIZE(ks8695_wdt_resources),
 };
 
 static void __init ks8695_add_device_watchdog(void)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 242eea859637..046e01daef57 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -397,7 +397,7 @@ config IXP4XX_WATCHDOG
 
 config KS8695_WATCHDOG
 	tristate "KS8695 watchdog"
-	depends on ARCH_KS8695
+	depends on ARCH_KS8695 || COMPILE_TEST
 	help
 	  Watchdog timer embedded into KS8695 processor. This will reboot your
 	  system when the timeout is reached.
diff --git a/drivers/watchdog/ks8695_wdt.c b/drivers/watchdog/ks8695_wdt.c
index 1e41818a44bc..87c542c2f912 100644
--- a/drivers/watchdog/ks8695_wdt.c
+++ b/drivers/watchdog/ks8695_wdt.c
@@ -23,10 +23,8 @@
 #include <linux/watchdog.h>
 #include <linux/io.h>
 #include <linux/uaccess.h>
-#include <mach/hardware.h>
 
-#define KS8695_TMR_OFFSET	(0xF0000 + 0xE400)
-#define KS8695_TMR_VA		(KS8695_IO_VA + KS8695_TMR_OFFSET)
+#define KS8695_CLOCK_RATE  25000000
 
 /*
  * Timer registers
@@ -57,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 
 static unsigned long ks8695wdt_busy;
 static DEFINE_SPINLOCK(ks8695_lock);
+static void __iomem *tmr_reg;
 
 /* ......................................................................... */
 
@@ -69,8 +68,8 @@ static inline void ks8695_wdt_stop(void)
 
 	spin_lock(&ks8695_lock);
 	/* disable timer0 */
-	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
 	spin_unlock(&ks8695_lock);
 }
 
@@ -84,15 +83,15 @@ static inline void ks8695_wdt_start(void)
 
 	spin_lock(&ks8695_lock);
 	/* disable timer0 */
-	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
 
 	/* program timer0 */
-	__raw_writel(tval | T0TC_WATCHDOG, KS8695_TMR_VA + KS8695_T0TC);
+	__raw_writel(tval | T0TC_WATCHDOG, tmr_reg + KS8695_T0TC);
 
 	/* re-enable timer0 */
-	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
 	spin_unlock(&ks8695_lock);
 }
 
@@ -105,9 +104,9 @@ static inline void ks8695_wdt_reload(void)
 
 	spin_lock(&ks8695_lock);
 	/* disable, then re-enable timer0 */
-	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
 	spin_unlock(&ks8695_lock);
 }
 
@@ -238,6 +237,11 @@ static struct miscdevice ks8695wdt_miscdev = {
 static int ks8695wdt_probe(struct platform_device *pdev)
 {
 	int res;
+	struct resource *resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	tmr_reg = devm_ioremap_resource(&pdev->dev, resource);
+	if (!tmr_reg)
+		return -ENXIO;
 
 	if (ks8695wdt_miscdev.parent)
 		return -EBUSY;
-- 
2.20.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h
  2019-04-15 20:24 [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h Arnd Bergmann
@ 2019-04-15 20:54 ` Guenter Roeck
  2019-04-15 20:58   ` Arnd Bergmann
  2019-04-16 13:21 ` [PATCH 1/6 v2] " Arnd Bergmann
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2019-04-15 20:54 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Greg Ungerer, Linus Walleij, arm, Wim Van Sebroeck,
	linux-arm-kernel, linux-kernel, linux-watchdog

On Mon, Apr 15, 2019 at 10:24:13PM +0200, Arnd Bergmann wrote:
> drivers should not rely on machine specific headers but
> get their information from the platform device.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/mach-ks8695/devices.c | 13 ++++++++++++-
>  drivers/watchdog/Kconfig       |  2 +-
>  drivers/watchdog/ks8695_wdt.c  | 30 +++++++++++++++++-------------
>  3 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/mach-ks8695/devices.c b/arch/arm/mach-ks8695/devices.c
> index 61cf20beb45f..57766817d86f 100644
> --- a/arch/arm/mach-ks8695/devices.c
> +++ b/arch/arm/mach-ks8695/devices.c
> @@ -169,11 +169,22 @@ void __init ks8696_add_device_hpna(void)
>  /* --------------------------------------------------------------------
>   *  Watchdog
>   * -------------------------------------------------------------------- */
> +#define KS8695_TMR_OFFSET      (0xF0000 + 0xE400)
> +#define KS8695_TMR_PA          (KS8695_IO_PA + KS8695_TMR_OFFSET)
> +static struct resource ks8695_wdt_resources[] = {
> +	[0] = {
> +		.name	= "tmr",
> +		.start	= KS8695_TMR_PA,
> +		.end	= KS8695_TMR_PA + 0xf,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +};
>  
>  static struct platform_device ks8695_wdt_device = {
>  	.name		= "ks8695_wdt",
>  	.id		= -1,
> -	.num_resources	= 0,
> +	.resource	= ks8695_wdt_resources,
> +	.num_resources	= ARRAY_SIZE(ks8695_wdt_resources),
>  };
>  
>  static void __init ks8695_add_device_watchdog(void)
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 242eea859637..046e01daef57 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -397,7 +397,7 @@ config IXP4XX_WATCHDOG
>  
>  config KS8695_WATCHDOG
>  	tristate "KS8695 watchdog"
> -	depends on ARCH_KS8695
> +	depends on ARCH_KS8695 || COMPILE_TEST

Is __raw_readl / __raw_writel really available for all architectures / platforms ?

>  	help
>  	  Watchdog timer embedded into KS8695 processor. This will reboot your
>  	  system when the timeout is reached.
> diff --git a/drivers/watchdog/ks8695_wdt.c b/drivers/watchdog/ks8695_wdt.c
> index 1e41818a44bc..87c542c2f912 100644
> --- a/drivers/watchdog/ks8695_wdt.c
> +++ b/drivers/watchdog/ks8695_wdt.c
> @@ -23,10 +23,8 @@
>  #include <linux/watchdog.h>
>  #include <linux/io.h>
>  #include <linux/uaccess.h>
> -#include <mach/hardware.h>
>  
> -#define KS8695_TMR_OFFSET	(0xF0000 + 0xE400)
> -#define KS8695_TMR_VA		(KS8695_IO_VA + KS8695_TMR_OFFSET)
> +#define KS8695_CLOCK_RATE  25000000
>  
>  /*
>   * Timer registers
> @@ -57,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
>  
>  static unsigned long ks8695wdt_busy;
>  static DEFINE_SPINLOCK(ks8695_lock);
> +static void __iomem *tmr_reg;
>  
>  /* ......................................................................... */
>  
> @@ -69,8 +68,8 @@ static inline void ks8695_wdt_stop(void)
>  
>  	spin_lock(&ks8695_lock);
>  	/* disable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
>  	spin_unlock(&ks8695_lock);
>  }
>  
> @@ -84,15 +83,15 @@ static inline void ks8695_wdt_start(void)
>  
>  	spin_lock(&ks8695_lock);
>  	/* disable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
>  
>  	/* program timer0 */
> -	__raw_writel(tval | T0TC_WATCHDOG, KS8695_TMR_VA + KS8695_T0TC);
> +	__raw_writel(tval | T0TC_WATCHDOG, tmr_reg + KS8695_T0TC);
>  
>  	/* re-enable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
>  	spin_unlock(&ks8695_lock);
>  }
>  
> @@ -105,9 +104,9 @@ static inline void ks8695_wdt_reload(void)
>  
>  	spin_lock(&ks8695_lock);
>  	/* disable, then re-enable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
>  	spin_unlock(&ks8695_lock);
>  }
>  
> @@ -238,6 +237,11 @@ static struct miscdevice ks8695wdt_miscdev = {
>  static int ks8695wdt_probe(struct platform_device *pdev)
>  {
>  	int res;
> +	struct resource *resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	tmr_reg = devm_ioremap_resource(&pdev->dev, resource);

Please use devm_platform_ioremap_resource().

Thanks,
Guenter

> +	if (!tmr_reg)
> +		return -ENXIO;
>  
>  	if (ks8695wdt_miscdev.parent)
>  		return -EBUSY;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h
  2019-04-15 20:54 ` Guenter Roeck
@ 2019-04-15 20:58   ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2019-04-15 20:58 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Greg Ungerer, Linus Walleij, arm-soc, Wim Van Sebroeck,
	Linux ARM, Linux Kernel Mailing List, linux-watchdog

On Mon, Apr 15, 2019 at 10:54 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> >
> >  config KS8695_WATCHDOG
> >       tristate "KS8695 watchdog"
> > -     depends on ARCH_KS8695
> > +     depends on ARCH_KS8695 || COMPILE_TEST
>
> Is __raw_readl / __raw_writel really available for all architectures / platforms ?

I'm fairly sure it is these days, only uml and s390 used to be the
exceptions here, but they both added this.

It's possible that something else is missing, I was hoping for the 0-day
bot to tell me if so.

> > @@ -238,6 +237,11 @@ static struct miscdevice ks8695wdt_miscdev = {
> >  static int ks8695wdt_probe(struct platform_device *pdev)
> >  {
> >       int res;
> > +     struct resource *resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +
> > +     tmr_reg = devm_ioremap_resource(&pdev->dev, resource);
>
> Please use devm_platform_ioremap_resource().

Ah, that is the function I was looking for, thanks for the hint.

      Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/6 v2] ARM: ks8695: watchdog: stop using mach/*.h
  2019-04-15 20:24 [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h Arnd Bergmann
  2019-04-15 20:54 ` Guenter Roeck
@ 2019-04-16 13:21 ` " Arnd Bergmann
  2019-04-16 18:09   ` Guenter Roeck
  2019-04-20  2:36 ` [PATCH 1/6] " Greg Ungerer
  2019-05-03  7:02 ` Greg Ungerer
  3 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2019-04-16 13:21 UTC (permalink / raw)
  To: arm
  Cc: Arnd Bergmann, Greg Ungerer, Wim Van Sebroeck, Guenter Roeck,
	linux-arm-kernel, linux-kernel, linux-watchdog

drivers should not rely on machine specific headers but
get their information from the platform device.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-ks8695/devices.c | 13 ++++++++++++-
 drivers/watchdog/Kconfig       |  2 +-
 drivers/watchdog/ks8695_wdt.c  | 29 ++++++++++++++++-------------
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-ks8695/devices.c b/arch/arm/mach-ks8695/devices.c
index 15afeb964280..ba9d0f0f47ac 100644
--- a/arch/arm/mach-ks8695/devices.c
+++ b/arch/arm/mach-ks8695/devices.c
@@ -169,11 +169,22 @@ void __init ks8696_add_device_hpna(void)
 /* --------------------------------------------------------------------
  *  Watchdog
  * -------------------------------------------------------------------- */
+#define KS8695_TMR_OFFSET      (0xF0000 + 0xE400)
+#define KS8695_TMR_PA          (KS8695_IO_PA + KS8695_TMR_OFFSET)
+static struct resource ks8695_wdt_resources[] = {
+	[0] = {
+		.name	= "tmr",
+		.start	= KS8695_TMR_PA,
+		.end	= KS8695_TMR_PA + 0xf,
+		.flags	= IORESOURCE_MEM,
+	},
+};
 
 static struct platform_device ks8695_wdt_device = {
 	.name		= "ks8695_wdt",
 	.id		= -1,
-	.num_resources	= 0,
+	.resource	= ks8695_wdt_resources,
+	.num_resources	= ARRAY_SIZE(ks8695_wdt_resources),
 };
 
 static void __init ks8695_add_device_watchdog(void)
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 242eea859637..046e01daef57 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -397,7 +397,7 @@ config IXP4XX_WATCHDOG
 
 config KS8695_WATCHDOG
 	tristate "KS8695 watchdog"
-	depends on ARCH_KS8695
+	depends on ARCH_KS8695 || COMPILE_TEST
 	help
 	  Watchdog timer embedded into KS8695 processor. This will reboot your
 	  system when the timeout is reached.
diff --git a/drivers/watchdog/ks8695_wdt.c b/drivers/watchdog/ks8695_wdt.c
index 1e41818a44bc..2a11cdfe2ab1 100644
--- a/drivers/watchdog/ks8695_wdt.c
+++ b/drivers/watchdog/ks8695_wdt.c
@@ -23,10 +23,8 @@
 #include <linux/watchdog.h>
 #include <linux/io.h>
 #include <linux/uaccess.h>
-#include <mach/hardware.h>
 
-#define KS8695_TMR_OFFSET	(0xF0000 + 0xE400)
-#define KS8695_TMR_VA		(KS8695_IO_VA + KS8695_TMR_OFFSET)
+#define KS8695_CLOCK_RATE  25000000
 
 /*
  * Timer registers
@@ -57,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 
 static unsigned long ks8695wdt_busy;
 static DEFINE_SPINLOCK(ks8695_lock);
+static void __iomem *tmr_reg;
 
 /* ......................................................................... */
 
@@ -69,8 +68,8 @@ static inline void ks8695_wdt_stop(void)
 
 	spin_lock(&ks8695_lock);
 	/* disable timer0 */
-	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
 	spin_unlock(&ks8695_lock);
 }
 
@@ -84,15 +83,15 @@ static inline void ks8695_wdt_start(void)
 
 	spin_lock(&ks8695_lock);
 	/* disable timer0 */
-	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
 
 	/* program timer0 */
-	__raw_writel(tval | T0TC_WATCHDOG, KS8695_TMR_VA + KS8695_T0TC);
+	__raw_writel(tval | T0TC_WATCHDOG, tmr_reg + KS8695_T0TC);
 
 	/* re-enable timer0 */
-	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
 	spin_unlock(&ks8695_lock);
 }
 
@@ -105,9 +104,9 @@ static inline void ks8695_wdt_reload(void)
 
 	spin_lock(&ks8695_lock);
 	/* disable, then re-enable timer0 */
-	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
-	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
+	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
+	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
 	spin_unlock(&ks8695_lock);
 }
 
@@ -239,6 +238,10 @@ static int ks8695wdt_probe(struct platform_device *pdev)
 {
 	int res;
 
+	tmr_reg = devm_platform_ioremap_resource(pdev, 0);
+	if (!tmr_reg)
+		return -ENXIO;
+
 	if (ks8695wdt_miscdev.parent)
 		return -EBUSY;
 	ks8695wdt_miscdev.parent = &pdev->dev;
-- 
2.20.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6 v2] ARM: ks8695: watchdog: stop using mach/*.h
  2019-04-16 13:21 ` [PATCH 1/6 v2] " Arnd Bergmann
@ 2019-04-16 18:09   ` Guenter Roeck
  0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2019-04-16 18:09 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: arm, Greg Ungerer, Wim Van Sebroeck, linux-arm-kernel,
	linux-kernel, linux-watchdog

On Tue, Apr 16, 2019 at 03:21:41PM +0200, Arnd Bergmann wrote:
> drivers should not rely on machine specific headers but
> get their information from the platform device.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Guenter Roeck <linux@roeck-us.net>

I assume this will be applied through an arm tree since it touches arm code.

Thanks,
Guenter

> ---
>  arch/arm/mach-ks8695/devices.c | 13 ++++++++++++-
>  drivers/watchdog/Kconfig       |  2 +-
>  drivers/watchdog/ks8695_wdt.c  | 29 ++++++++++++++++-------------
>  3 files changed, 29 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/mach-ks8695/devices.c b/arch/arm/mach-ks8695/devices.c
> index 15afeb964280..ba9d0f0f47ac 100644
> --- a/arch/arm/mach-ks8695/devices.c
> +++ b/arch/arm/mach-ks8695/devices.c
> @@ -169,11 +169,22 @@ void __init ks8696_add_device_hpna(void)
>  /* --------------------------------------------------------------------
>   *  Watchdog
>   * -------------------------------------------------------------------- */
> +#define KS8695_TMR_OFFSET      (0xF0000 + 0xE400)
> +#define KS8695_TMR_PA          (KS8695_IO_PA + KS8695_TMR_OFFSET)
> +static struct resource ks8695_wdt_resources[] = {
> +	[0] = {
> +		.name	= "tmr",
> +		.start	= KS8695_TMR_PA,
> +		.end	= KS8695_TMR_PA + 0xf,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +};
>  
>  static struct platform_device ks8695_wdt_device = {
>  	.name		= "ks8695_wdt",
>  	.id		= -1,
> -	.num_resources	= 0,
> +	.resource	= ks8695_wdt_resources,
> +	.num_resources	= ARRAY_SIZE(ks8695_wdt_resources),
>  };
>  
>  static void __init ks8695_add_device_watchdog(void)
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 242eea859637..046e01daef57 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -397,7 +397,7 @@ config IXP4XX_WATCHDOG
>  
>  config KS8695_WATCHDOG
>  	tristate "KS8695 watchdog"
> -	depends on ARCH_KS8695
> +	depends on ARCH_KS8695 || COMPILE_TEST
>  	help
>  	  Watchdog timer embedded into KS8695 processor. This will reboot your
>  	  system when the timeout is reached.
> diff --git a/drivers/watchdog/ks8695_wdt.c b/drivers/watchdog/ks8695_wdt.c
> index 1e41818a44bc..2a11cdfe2ab1 100644
> --- a/drivers/watchdog/ks8695_wdt.c
> +++ b/drivers/watchdog/ks8695_wdt.c
> @@ -23,10 +23,8 @@
>  #include <linux/watchdog.h>
>  #include <linux/io.h>
>  #include <linux/uaccess.h>
> -#include <mach/hardware.h>
>  
> -#define KS8695_TMR_OFFSET	(0xF0000 + 0xE400)
> -#define KS8695_TMR_VA		(KS8695_IO_VA + KS8695_TMR_OFFSET)
> +#define KS8695_CLOCK_RATE  25000000
>  
>  /*
>   * Timer registers
> @@ -57,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
>  
>  static unsigned long ks8695wdt_busy;
>  static DEFINE_SPINLOCK(ks8695_lock);
> +static void __iomem *tmr_reg;
>  
>  /* ......................................................................... */
>  
> @@ -69,8 +68,8 @@ static inline void ks8695_wdt_stop(void)
>  
>  	spin_lock(&ks8695_lock);
>  	/* disable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
>  	spin_unlock(&ks8695_lock);
>  }
>  
> @@ -84,15 +83,15 @@ static inline void ks8695_wdt_start(void)
>  
>  	spin_lock(&ks8695_lock);
>  	/* disable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
>  
>  	/* program timer0 */
> -	__raw_writel(tval | T0TC_WATCHDOG, KS8695_TMR_VA + KS8695_T0TC);
> +	__raw_writel(tval | T0TC_WATCHDOG, tmr_reg + KS8695_T0TC);
>  
>  	/* re-enable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
>  	spin_unlock(&ks8695_lock);
>  }
>  
> @@ -105,9 +104,9 @@ static inline void ks8695_wdt_reload(void)
>  
>  	spin_lock(&ks8695_lock);
>  	/* disable, then re-enable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
>  	spin_unlock(&ks8695_lock);
>  }
>  
> @@ -239,6 +238,10 @@ static int ks8695wdt_probe(struct platform_device *pdev)
>  {
>  	int res;
>  
> +	tmr_reg = devm_platform_ioremap_resource(pdev, 0);
> +	if (!tmr_reg)
> +		return -ENXIO;
> +
>  	if (ks8695wdt_miscdev.parent)
>  		return -EBUSY;
>  	ks8695wdt_miscdev.parent = &pdev->dev;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h
  2019-04-15 20:24 [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h Arnd Bergmann
  2019-04-15 20:54 ` Guenter Roeck
  2019-04-16 13:21 ` [PATCH 1/6 v2] " Arnd Bergmann
@ 2019-04-20  2:36 ` " Greg Ungerer
  2019-05-03  7:02 ` Greg Ungerer
  3 siblings, 0 replies; 10+ messages in thread
From: Greg Ungerer @ 2019-04-20  2:36 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, arm, Wim Van Sebroeck, Guenter Roeck,
	linux-arm-kernel, linux-kernel, linux-watchdog

Hi Arnd,

On 16/4/19 6:24 am, Arnd Bergmann wrote:
> drivers should not rely on machine specific headers but
> get their information from the platform device.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I like the whole series, thanks for doing this.

I haven't looked at the KS8695 in a long time now. I am not sure
that I have any working hardware - but I will have a look around my lab
and see if I can find something.

I'll get back to you with acks and tested bys soon.

Regards
Greg



> ---
>   arch/arm/mach-ks8695/devices.c | 13 ++++++++++++-
>   drivers/watchdog/Kconfig       |  2 +-
>   drivers/watchdog/ks8695_wdt.c  | 30 +++++++++++++++++-------------
>   3 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/mach-ks8695/devices.c b/arch/arm/mach-ks8695/devices.c
> index 61cf20beb45f..57766817d86f 100644
> --- a/arch/arm/mach-ks8695/devices.c
> +++ b/arch/arm/mach-ks8695/devices.c
> @@ -169,11 +169,22 @@ void __init ks8696_add_device_hpna(void)
>   /* --------------------------------------------------------------------
>    *  Watchdog
>    * -------------------------------------------------------------------- */
> +#define KS8695_TMR_OFFSET      (0xF0000 + 0xE400)
> +#define KS8695_TMR_PA          (KS8695_IO_PA + KS8695_TMR_OFFSET)
> +static struct resource ks8695_wdt_resources[] = {
> +	[0] = {
> +		.name	= "tmr",
> +		.start	= KS8695_TMR_PA,
> +		.end	= KS8695_TMR_PA + 0xf,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +};
>   
>   static struct platform_device ks8695_wdt_device = {
>   	.name		= "ks8695_wdt",
>   	.id		= -1,
> -	.num_resources	= 0,
> +	.resource	= ks8695_wdt_resources,
> +	.num_resources	= ARRAY_SIZE(ks8695_wdt_resources),
>   };
>   
>   static void __init ks8695_add_device_watchdog(void)
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 242eea859637..046e01daef57 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -397,7 +397,7 @@ config IXP4XX_WATCHDOG
>   
>   config KS8695_WATCHDOG
>   	tristate "KS8695 watchdog"
> -	depends on ARCH_KS8695
> +	depends on ARCH_KS8695 || COMPILE_TEST
>   	help
>   	  Watchdog timer embedded into KS8695 processor. This will reboot your
>   	  system when the timeout is reached.
> diff --git a/drivers/watchdog/ks8695_wdt.c b/drivers/watchdog/ks8695_wdt.c
> index 1e41818a44bc..87c542c2f912 100644
> --- a/drivers/watchdog/ks8695_wdt.c
> +++ b/drivers/watchdog/ks8695_wdt.c
> @@ -23,10 +23,8 @@
>   #include <linux/watchdog.h>
>   #include <linux/io.h>
>   #include <linux/uaccess.h>
> -#include <mach/hardware.h>
>   
> -#define KS8695_TMR_OFFSET	(0xF0000 + 0xE400)
> -#define KS8695_TMR_VA		(KS8695_IO_VA + KS8695_TMR_OFFSET)
> +#define KS8695_CLOCK_RATE  25000000
>   
>   /*
>    * Timer registers
> @@ -57,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
>   
>   static unsigned long ks8695wdt_busy;
>   static DEFINE_SPINLOCK(ks8695_lock);
> +static void __iomem *tmr_reg;
>   
>   /* ......................................................................... */
>   
> @@ -69,8 +68,8 @@ static inline void ks8695_wdt_stop(void)
>   
>   	spin_lock(&ks8695_lock);
>   	/* disable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
>   	spin_unlock(&ks8695_lock);
>   }
>   
> @@ -84,15 +83,15 @@ static inline void ks8695_wdt_start(void)
>   
>   	spin_lock(&ks8695_lock);
>   	/* disable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
>   
>   	/* program timer0 */
> -	__raw_writel(tval | T0TC_WATCHDOG, KS8695_TMR_VA + KS8695_T0TC);
> +	__raw_writel(tval | T0TC_WATCHDOG, tmr_reg + KS8695_T0TC);
>   
>   	/* re-enable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
>   	spin_unlock(&ks8695_lock);
>   }
>   
> @@ -105,9 +104,9 @@ static inline void ks8695_wdt_reload(void)
>   
>   	spin_lock(&ks8695_lock);
>   	/* disable, then re-enable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
>   	spin_unlock(&ks8695_lock);
>   }
>   
> @@ -238,6 +237,11 @@ static struct miscdevice ks8695wdt_miscdev = {
>   static int ks8695wdt_probe(struct platform_device *pdev)
>   {
>   	int res;
> +	struct resource *resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	tmr_reg = devm_ioremap_resource(&pdev->dev, resource);
> +	if (!tmr_reg)
> +		return -ENXIO;
>   
>   	if (ks8695wdt_miscdev.parent)
>   		return -EBUSY;
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h
  2019-04-15 20:24 [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h Arnd Bergmann
                   ` (2 preceding siblings ...)
  2019-04-20  2:36 ` [PATCH 1/6] " Greg Ungerer
@ 2019-05-03  7:02 ` Greg Ungerer
  2019-05-03  7:16   ` Linus Walleij
  3 siblings, 1 reply; 10+ messages in thread
From: Greg Ungerer @ 2019-05-03  7:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, arm, Wim Van Sebroeck, Guenter Roeck,
	linux-arm-kernel, linux-kernel, linux-watchdog

Hi Arnd,

On 16/4/19 6:24 am, Arnd Bergmann wrote:
> drivers should not rely on machine specific headers but
> get their information from the platform device.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I dug out some old ks8695 based hardware to try this out.
I had a lot of trouble getting anything modern working on it.
In the end I still don't have a reliable test bed to test this properly.

Your patch series works as well as the kernel before the changes,
so I am happy enough to ack them as they are.

Acked-by: Greg Ungerer <gerg@kernel.org>

Ultimately though I am left wondering if the ks8695 support in the
kernel is useful to anyone the way it is at the moment. With a minimal
kernel configuration I can boot up to a shell - but the system is
really unreliable if you try to interactively use it. I don't think
it is the hardware - it seems to run reliably with the old code
it has running from flash on it. I am only testing the new kernel,
running with the existing user space root filesystem on it (which
dates from 2004 :-)

Regards
Greg



>   arch/arm/mach-ks8695/devices.c | 13 ++++++++++++-
>   drivers/watchdog/Kconfig       |  2 +-
>   drivers/watchdog/ks8695_wdt.c  | 30 +++++++++++++++++-------------
>   3 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm/mach-ks8695/devices.c b/arch/arm/mach-ks8695/devices.c
> index 61cf20beb45f..57766817d86f 100644
> --- a/arch/arm/mach-ks8695/devices.c
> +++ b/arch/arm/mach-ks8695/devices.c
> @@ -169,11 +169,22 @@ void __init ks8696_add_device_hpna(void)
>   /* --------------------------------------------------------------------
>    *  Watchdog
>    * -------------------------------------------------------------------- */
> +#define KS8695_TMR_OFFSET      (0xF0000 + 0xE400)
> +#define KS8695_TMR_PA          (KS8695_IO_PA + KS8695_TMR_OFFSET)
> +static struct resource ks8695_wdt_resources[] = {
> +	[0] = {
> +		.name	= "tmr",
> +		.start	= KS8695_TMR_PA,
> +		.end	= KS8695_TMR_PA + 0xf,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +};
>   
>   static struct platform_device ks8695_wdt_device = {
>   	.name		= "ks8695_wdt",
>   	.id		= -1,
> -	.num_resources	= 0,
> +	.resource	= ks8695_wdt_resources,
> +	.num_resources	= ARRAY_SIZE(ks8695_wdt_resources),
>   };
>   
>   static void __init ks8695_add_device_watchdog(void)
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 242eea859637..046e01daef57 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -397,7 +397,7 @@ config IXP4XX_WATCHDOG
>   
>   config KS8695_WATCHDOG
>   	tristate "KS8695 watchdog"
> -	depends on ARCH_KS8695
> +	depends on ARCH_KS8695 || COMPILE_TEST
>   	help
>   	  Watchdog timer embedded into KS8695 processor. This will reboot your
>   	  system when the timeout is reached.
> diff --git a/drivers/watchdog/ks8695_wdt.c b/drivers/watchdog/ks8695_wdt.c
> index 1e41818a44bc..87c542c2f912 100644
> --- a/drivers/watchdog/ks8695_wdt.c
> +++ b/drivers/watchdog/ks8695_wdt.c
> @@ -23,10 +23,8 @@
>   #include <linux/watchdog.h>
>   #include <linux/io.h>
>   #include <linux/uaccess.h>
> -#include <mach/hardware.h>
>   
> -#define KS8695_TMR_OFFSET	(0xF0000 + 0xE400)
> -#define KS8695_TMR_VA		(KS8695_IO_VA + KS8695_TMR_OFFSET)
> +#define KS8695_CLOCK_RATE  25000000
>   
>   /*
>    * Timer registers
> @@ -57,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
>   
>   static unsigned long ks8695wdt_busy;
>   static DEFINE_SPINLOCK(ks8695_lock);
> +static void __iomem *tmr_reg;
>   
>   /* ......................................................................... */
>   
> @@ -69,8 +68,8 @@ static inline void ks8695_wdt_stop(void)
>   
>   	spin_lock(&ks8695_lock);
>   	/* disable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
>   	spin_unlock(&ks8695_lock);
>   }
>   
> @@ -84,15 +83,15 @@ static inline void ks8695_wdt_start(void)
>   
>   	spin_lock(&ks8695_lock);
>   	/* disable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
>   
>   	/* program timer0 */
> -	__raw_writel(tval | T0TC_WATCHDOG, KS8695_TMR_VA + KS8695_T0TC);
> +	__raw_writel(tval | T0TC_WATCHDOG, tmr_reg + KS8695_T0TC);
>   
>   	/* re-enable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
>   	spin_unlock(&ks8695_lock);
>   }
>   
> @@ -105,9 +104,9 @@ static inline void ks8695_wdt_reload(void)
>   
>   	spin_lock(&ks8695_lock);
>   	/* disable, then re-enable timer0 */
> -	tmcon = __raw_readl(KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon & ~TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> -	__raw_writel(tmcon | TMCON_T0EN, KS8695_TMR_VA + KS8695_TMCON);
> +	tmcon = __raw_readl(tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon & ~TMCON_T0EN, tmr_reg + KS8695_TMCON);
> +	__raw_writel(tmcon | TMCON_T0EN, tmr_reg + KS8695_TMCON);
>   	spin_unlock(&ks8695_lock);
>   }
>   
> @@ -238,6 +237,11 @@ static struct miscdevice ks8695wdt_miscdev = {
>   static int ks8695wdt_probe(struct platform_device *pdev)
>   {
>   	int res;
> +	struct resource *resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	tmr_reg = devm_ioremap_resource(&pdev->dev, resource);
> +	if (!tmr_reg)
> +		return -ENXIO;
>   
>   	if (ks8695wdt_miscdev.parent)
>   		return -EBUSY;
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h
  2019-05-03  7:02 ` Greg Ungerer
@ 2019-05-03  7:16   ` Linus Walleij
  2019-05-03 17:06     ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2019-05-03  7:16 UTC (permalink / raw)
  To: Greg Ungerer
  Cc: Arnd Bergmann, arm-soc, Wim Van Sebroeck, Guenter Roeck,
	Linux ARM, linux-kernel, LINUXWATCHDOG

On Fri, May 3, 2019 at 8:02 AM Greg Ungerer <gerg@uclinux.org> wrote:

> I dug out some old ks8695 based hardware to try this out.
> I had a lot of trouble getting anything modern working on it.
> In the end I still don't have a reliable test bed to test this properly.

What is usually used by old ARMv4 systems is OpenWrt or
OpenEmbedded. Those is the only build systems that reliably
produce a userspace for these things now, and it is also the
appropriate size for this kind of systems.

> Ultimately though I am left wondering if the ks8695 support in the
> kernel is useful to anyone the way it is at the moment. With a minimal
> kernel configuration I can boot up to a shell - but the system is
> really unreliable if you try to interactively use it. I don't think
> it is the hardware - it seems to run reliably with the old code
> it has running from flash on it. I am only testing the new kernel,
> running with the existing user space root filesystem on it (which
> dates from 2004 :-)

Personally I think it is a bad sign that this subarch and boards do
not have active OpenWrt support, they are routers after all (right?)
and any active use of networking equipment should use a recent
userspace as well, given all the security bugs that popped up over
the years.

With IXP4xx, Gemini and EP93xx we have found active users and
companies selling the chips and reference designs and even
recommending it for new products (!) at times.  If this is not the
case with KS8695 and no hobbyists are willing to submit it
to OpenWrt and modernize it to use device tree I think it should be
deleted from the kernel.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h
  2019-05-03  7:16   ` Linus Walleij
@ 2019-05-03 17:06     ` Guenter Roeck
  2019-05-04 14:26       ` Greg Ungerer
  0 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2019-05-03 17:06 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Greg Ungerer, Arnd Bergmann, arm-soc, Wim Van Sebroeck,
	Linux ARM, linux-kernel, LINUXWATCHDOG

On Fri, May 03, 2019 at 08:16:05AM +0100, Linus Walleij wrote:
> On Fri, May 3, 2019 at 8:02 AM Greg Ungerer <gerg@uclinux.org> wrote:
> 
> > I dug out some old ks8695 based hardware to try this out.
> > I had a lot of trouble getting anything modern working on it.
> > In the end I still don't have a reliable test bed to test this properly.
> 
> What is usually used by old ARMv4 systems is OpenWrt or
> OpenEmbedded. Those is the only build systems that reliably
> produce a userspace for these things now, and it is also the
> appropriate size for this kind of systems.
> 
> > Ultimately though I am left wondering if the ks8695 support in the
> > kernel is useful to anyone the way it is at the moment. With a minimal
> > kernel configuration I can boot up to a shell - but the system is
> > really unreliable if you try to interactively use it. I don't think
> > it is the hardware - it seems to run reliably with the old code
> > it has running from flash on it. I am only testing the new kernel,
> > running with the existing user space root filesystem on it (which
> > dates from 2004 :-)
> 
> Personally I think it is a bad sign that this subarch and boards do
> not have active OpenWrt support, they are routers after all (right?)
> and any active use of networking equipment should use a recent
> userspace as well, given all the security bugs that popped up over
> the years.
> 
> With IXP4xx, Gemini and EP93xx we have found active users and
> companies selling the chips and reference designs and even
> recommending it for new products (!) at times.  If this is not the
> case with KS8695 and no hobbyists are willing to submit it
> to OpenWrt and modernize it to use device tree I think it should be
> deleted from the kernel.
> 

That may be the best approach if indeed no one is using it,
much less maintaining it.

Guenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h
  2019-05-03 17:06     ` Guenter Roeck
@ 2019-05-04 14:26       ` Greg Ungerer
  0 siblings, 0 replies; 10+ messages in thread
From: Greg Ungerer @ 2019-05-04 14:26 UTC (permalink / raw)
  To: Guenter Roeck, Linus Walleij
  Cc: Arnd Bergmann, arm-soc, Wim Van Sebroeck, Linux ARM,
	linux-kernel, LINUXWATCHDOG


On 4/5/19 3:06 am, Guenter Roeck wrote:
> On Fri, May 03, 2019 at 08:16:05AM +0100, Linus Walleij wrote:
>> On Fri, May 3, 2019 at 8:02 AM Greg Ungerer <gerg@uclinux.org> wrote:
>>
>>> I dug out some old ks8695 based hardware to try this out.
>>> I had a lot of trouble getting anything modern working on it.
>>> In the end I still don't have a reliable test bed to test this properly.
>>
>> What is usually used by old ARMv4 systems is OpenWrt or
>> OpenEmbedded. Those is the only build systems that reliably
>> produce a userspace for these things now, and it is also the
>> appropriate size for this kind of systems.

No, I can produce a user space environment for the KS8695 as well
using the uClinux-dist build system. But that worked even less well
than the old root filesystem that I had (which was also built with
an older version of that build system).

But there is no reason that old root filesystem should not work.
And that is the thing that concerns me a bit here. I could mount
it ok (it was a CRAMFS), it would run up the shell to a shell prompt,
but when I try to run any commands from there they would oops.
I didn't debug any further than that.


>>> Ultimately though I am left wondering if the ks8695 support in the
>>> kernel is useful to anyone the way it is at the moment. With a minimal
>>> kernel configuration I can boot up to a shell - but the system is
>>> really unreliable if you try to interactively use it. I don't think
>>> it is the hardware - it seems to run reliably with the old code
>>> it has running from flash on it. I am only testing the new kernel,
>>> running with the existing user space root filesystem on it (which
>>> dates from 2004 :-)
>>
>> Personally I think it is a bad sign that this subarch and boards do
>> not have active OpenWrt support, they are routers after all (right?)
>> and any active use of networking equipment should use a recent
>> userspace as well, given all the security bugs that popped up over
>> the years.
>>
>> With IXP4xx, Gemini and EP93xx we have found active users and
>> companies selling the chips and reference designs and even
>> recommending it for new products (!) at times.  If this is not the
>> case with KS8695 and no hobbyists are willing to submit it
>> to OpenWrt and modernize it to use device tree I think it should be
>> deleted from the kernel.
>>
> 
> That may be the best approach if indeed no one is using it,
> much less maintaining it.

Well, I for one don't really use it any more. So I don't have a lot
of motivation to maintain it any longer.

Regards
Greg


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15 20:24 [PATCH 1/6] ARM: ks8695: watchdog: stop using mach/*.h Arnd Bergmann
2019-04-15 20:54 ` Guenter Roeck
2019-04-15 20:58   ` Arnd Bergmann
2019-04-16 13:21 ` [PATCH 1/6 v2] " Arnd Bergmann
2019-04-16 18:09   ` Guenter Roeck
2019-04-20  2:36 ` [PATCH 1/6] " Greg Ungerer
2019-05-03  7:02 ` Greg Ungerer
2019-05-03  7:16   ` Linus Walleij
2019-05-03 17:06     ` Guenter Roeck
2019-05-04 14:26       ` Greg Ungerer

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org linux-watchdog@archiver.kernel.org
	public-inbox-index linux-watchdog


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/ public-inbox