Re: [PATCH V2] watchdog: mpcore: Add DT probing support for ARM mpcore watchdog

[Marc Zyngier <marc.zyngier@arm.com>]

On 23/04/12 15:21, viresh kumar wrote:
> On 4/23/12, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> Ah, that explains why it worked. I suppose you do have additional
>> patches that fix the IRQ request bit?
> 
> Missed this earlier. That patchset is in review currently and have already
> fixed that. :)

Right. Please post that patch as a prerequisite for any other change.

>> Given that no in-tree platform seem to be using this watchdog (at least
>> a quick grep didn't reveal anything), I'd be inclined to simply change
>> the offset in smp_twd.h and let them break.
> 
> Even i can't find any users of it. :)
> 
> @Wim: Please apply following patch before this one.

Probably it would be better to keep everything in a single series,
including the above IRQ fixes. It would surely make things easier for
the maintainer and other people who are reviewing the changes you're
making to the driver.

> Here we go:
> 
> From: Viresh Kumar <viresh.kumar@st.com>
> Date: Mon, 23 Apr 2012 19:39:47 +0530
> Subject: [PATCH] ARM: SMP_TWD: WDOG: Start registers from 0x00 instead of
>  0x20
> 
> TWD_WDOG is at offset 0x20 from TWD base address. Current register offsets
> contain this extra 0x20 offset, i.e. users were required to pass base address of
> TWD instead of WDOG to WDOG driver.
> 
> Change this, so that users can pass base address of WDOG to WDOG driver instead
> of TWD module. For this, subtract 0x20 from offsets of WDOG registers.
> 
> This could break any current users of TWD_WDOG, but i couldn't find any users of
> this driver in current Linux tree. So, haven't fixed any platform code. If some
> platforms are broken please report to me, so that we can get them fixed in this
> patch only.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  arch/arm/include/asm/smp_twd.h |   17 +++++++++++------
>  1 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/include/asm/smp_twd.h b/arch/arm/include/asm/smp_twd.h
> index 57857d1..ff3f67e 100644
> --- a/arch/arm/include/asm/smp_twd.h
> +++ b/arch/arm/include/asm/smp_twd.h
> @@ -6,12 +6,17 @@
>  #define TWD_TIMER_CONTROL		0x08
>  #define TWD_TIMER_INTSTAT		0x0C
> 
> -#define TWD_WDOG_LOAD			0x20
> -#define TWD_WDOG_COUNTER		0x24
> -#define TWD_WDOG_CONTROL		0x28
> -#define TWD_WDOG_INTSTAT		0x2C
> -#define TWD_WDOG_RESETSTAT		0x30
> -#define TWD_WDOG_DISABLE		0x34
> +/*
> + * TWD_WDOG is at offset 0x20 from TWD base address. Following register offsets
> + * doesn't contain this extra 0x20 offset, i.e. users of TWD_WDOG
> must pass base
> + * address of WDOG to WDOG driver instead of TWD module.
> + */
> +#define TWD_WDOG_LOAD			0x00
> +#define TWD_WDOG_COUNTER		0x04
> +#define TWD_WDOG_CONTROL		0x08
> +#define TWD_WDOG_INTSTAT		0x0C
> +#define TWD_WDOG_RESETSTAT		0x10
> +#define TWD_WDOG_DISABLE		0x14
> 
>  #define TWD_WDOG_LOAD_MIN		0x00000000
>  #define TWD_WDOG_LOAD_MAX		0xFFFFFFFF
> 

Now that we made it that far, why not moving the TWD_WDOG_* #defines to
the driver itself? Nobody else should care about it anyway...

	M.
-- 
Jazz is not dead. It just smells funny...

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