Re: [PATCH 5/7] drivers: watchdog: sb_wdog: Fix 32bit linking problems

[Ralf Baechle <ralf@linux-mips.org>]

On Mon, Jun 17, 2013 at 03:00:39PM +0100, Markos Chandras wrote:

> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
> Acked-by: Steven J. Hill <Steven.Hill@imgtec.com>
> Cc: sibyte-users@bitmover.com
> Cc: Wim Van Sebroeck <wim@iguana.be>
> ---
>  drivers/watchdog/sb_wdog.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/sb_wdog.c b/drivers/watchdog/sb_wdog.c
> index 25c7a3f..2ea0427 100644
> --- a/drivers/watchdog/sb_wdog.c
> +++ b/drivers/watchdog/sb_wdog.c
> @@ -170,6 +170,7 @@ static long sbwdog_ioctl(struct file *file, unsigned int cmd,
>  						unsigned long arg)
>  {
>  	int ret = -ENOTTY;
> +	u64 tmp_user_dog;
>  	unsigned long time;
>  	void __user *argp = (void __user *)arg;
>  	int __user *p = argp;
> @@ -208,7 +209,9 @@ static long sbwdog_ioctl(struct file *file, unsigned int cmd,
>  		 * get the remaining count from the ... count register
>  		 * which is 1*8 before the config register
>  		 */
> -		ret = put_user(__raw_readq(user_dog - 8) / 1000000, p);
> +		tmp_user_dog = __raw_readq(user_dog - 8);
> +		tmp_user_dog = do_div(tmp_user_dog, 1000000);
> +		ret = put_user(tmp_user_dog, p);

In effect the code with your change now does:

		ret = put_user(__raw_readq(user_dog - 8) % 1000000, p);

No good.

		tmp_user_dog = __raw_readq(user_dog - 8);
		do_div(tmp_user_dog, 1000000);
		ret = put_user(tmp_user_dog, p);

Should to the right thing.

I'm not surprised you're finding bugs in 32 bit Sibyte kernel.  At heart,
the Sibyte SOCs are 64 bit and their architecture limits them to support
256MB memory with a 32 bit kernel without highmem.  Highmem though
supported is stupid and leaves as the only sane option 64 bit kernels
and virtually every user has done that, not last to get full fp performance
which is only available with the N32 / N64 ABIs.  Oh, and of course many
registers need to be accessed 64 bit wide, which on a 32 bit kernel
requires a local_irq_disable ...  local_irq_enable around the actual
access.

In short, nobody but a few diehard backward folks have ever been using
32 bit kernels on Sibyte hardware, so finding such an issue is not really
a surprise.

  Ralf