| Message ID | 20190415202501.941196-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/6] ARM: ks8695: watchdog: stop using mach/*.h | expand |
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;
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
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; >
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; >
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
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
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
On Sat, May 4, 2019 at 4:27 PM Greg Ungerer <gerg@kernel.org> wrote: > 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: > >>> 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. Looking around on the internet, I found that Micrel at some point had their own openwrt fork for ks8695, but I can't find a copy any more, as the micrel.com domain is no longer used after the acquisition by Microchip. https://wikidevi.com/wiki/Micrel has a list of devices based on ks8695, and it seems that most of these are rather memory limited, which is a problem for recent openwrt builds. Only two of the 17 listed devices have the absolute minimum of 4MB flash and 32MB RAM for openwrt, two more have 8/32 and one or two have 4/64, but all these configurations are too limited for the web U/I now. > >> 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. I came across my patches while rebasing my backlog to 5.3-rc1. Should I save the (very small) trouble of sending them out again and just remove the platform then? Arnd
On Mon, Jul 22, 2019 at 7:44 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Sat, May 4, 2019 at 4:27 PM Greg Ungerer <gerg@kernel.org> wrote: > > 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: > > >>> 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. > > Looking around on the internet, I found that Micrel at some point > had their own openwrt fork for ks8695, but I can't find a copy > any more, as the micrel.com domain is no longer used after the > acquisition by Microchip. > > https://wikidevi.com/wiki/Micrel has a list of devices based on > ks8695, and it seems that most of these are rather memory > limited, which is a problem for recent openwrt builds. > > Only two of the 17 listed devices have the absolute minimum of 4MB > flash and 32MB RAM for openwrt, two more have 8/32 and one > or two have 4/64, but all these configurations are too limited for the > web U/I now. > > > >> 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. > > I came across my patches while rebasing my backlog to 5.3-rc1. > > Should I save the (very small) trouble of sending them out again > and just remove the platform then? Given the lack of response/interest from users, I'm OK with removing it. If someone shows up wanting support, we'll have a good opportunity to discuss testing/modernization involving someone actively using the hardware. -Olof
Hi Arnd, On 23/7/19 12:44 am, Arnd Bergmann wrote: > On Sat, May 4, 2019 at 4:27 PM Greg Ungerer <gerg@kernel.org> wrote: >> 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: >>>>> 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. > > Looking around on the internet, I found that Micrel at some point > had their own openwrt fork for ks8695, but I can't find a copy > any more, as the micrel.com domain is no longer used after the > acquisition by Microchip. I build it with uClinux-dist, https://sourceforge.net/projects/uclinux/files/uClinux%20Stable/. And again I can build for it, it just doesn't currently work in any sort of reasonable way. So I get the impression it hasn't worked for a while and nobody has noticed. > https://wikidevi.com/wiki/Micrel has a list of devices based on > ks8695, and it seems that most of these are rather memory > limited, which is a problem for recent openwrt builds. > > Only two of the 17 listed devices have the absolute minimum of 4MB > flash and 32MB RAM for openwrt, two more have 8/32 and one > or two have 4/64, but all these configurations are too limited for the > web U/I now. >>>> 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. > > I came across my patches while rebasing my backlog to 5.3-rc1. > > Should I save the (very small) trouble of sending them out again > and just remove the platform then? At this time I have no issue with removing it. Regards Greg
On Mon, Jul 29, 2019 at 2:53 PM Greg Ungerer <gerg@kernel.org> wrote: > On 23/7/19 12:44 am, Arnd Bergmann wrote: > > On Sat, May 4, 2019 at 4:27 PM Greg Ungerer <gerg@kernel.org> wrote: > >> On 4/5/19 3:06 am, Guenter Roeck wrote: > >>> On Fri, May 03, 2019 at 08:16:05AM +0100, Linus Walleij wrote: > >>>> 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. > > > > I came across my patches while rebasing my backlog to 5.3-rc1. > > > > Should I save the (very small) trouble of sending them out again > > and just remove the platform then? > > At this time I have no issue with removing it. Ok, let's do that then. For reference, this is what I think we should do for the remaining ARM9 based platforms that never gained multiplatform support, as time permits: netx: is now removed ks8695: remove next w90x900: probably remove, need to confirm with last known users davinci: almost multiplatform now, should be done in 5.4 ep93xx: convert to common-clk, generic-irq, then enable multiplatform (linusw is on it) omap1: convert to common-clk, change pcmcia driver to common I/O space method, use dma_pfn_offset for virt_to_bus, add ugly hacks for cpu_is_omap*() and omap_readl(), then enable multiplatform (arnd has started this) lpc32xx: clean up header files so we can build last 6 drivers independently, then move to multiplatform, probably after 5.4 I have patches for this somewhere. s3c24xx: change 18 drivers that still use mach/* headers, get creative about mach-bast ISA I/O space Arnd
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;
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(-)