linux-kernel - Re: [PATCH 2/4] watchdog: max63xx: cleanup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1529155290.150136.1422631687228.JavaMail.root@mail>
Date:	Fri, 30 Jan 2015 10:28:07 -0500 (EST)
From:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To:	Guenter Roeck <linux@...ck-us.net>
Cc:	Wim Van Sebroeck <wim@...ana.be>, linux-kernel@...r.kernel.org,
	kernel@...oirfairelinux.com, linux-watchdog@...r.kernel.org
Subject: Re: [PATCH 2/4] watchdog: max63xx: cleanup

Hi Guenter,

> > -#define MAX6369_WDSET	(7 << 0)
> > -#define MAX6369_WDI	(1 << 3)
> 
> Not really sure I understand why you remove those constants.
> that is personal preference, not cleanup. Someone else might
> submit another cleanup later on and re-introduce them.

Indeed, I should have explained this. My primary intention with this patchset
is to bring GPIO support to this driver. MAX6369_WDSET, which is a mask, is
specific to the memory mapped support. I first intented to rename it to
MAX6369_MMAP_WDSET to explicitly mention the connection, but I ended up
thinking that a clear comment in the related functions would be much clearer
that a generic named macro.

These masks are explained below in this patch:

In max63xx_mmap_ping():
+       /* WDI is the 4th bit of the memory mapped byte */
+       __raw_writeb(val | BIT(3), data->base);
+       __raw_writeb(val & ~BIT(3), data->base);

and max63xx_mmap_set():
+       /* SET0, SET1, SET2 are the 3 lower bits of the memory mapped byte */
+       val = __raw_readb(data->base);
+       val &= ~7;
+       val |= set & 7;
+       __raw_writeb(val, data->base);

Also, if it is possible for a platform to map these bits in a different way,
some platform settings would have to be added and thus these macros won't be
relevant.

Thanks,
-v
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/