linux-kernel - Re: [PATCH] watchdog: f71808e

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] [day] [month] [year] [list]

Message-ID: <57577C09.7060009@roeck-us.net>
Date:	Tue, 7 Jun 2016 18:59:37 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	"Ji-Ze Hong (Peter Hong)" <hpeter@...il.com>, wim@...ana.be
Cc:	linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org,
	peter_hong@...tek.com.tw, tom_tsai@...tek.com.tw,
	"Ji-Ze Hong (Peter Hong)" <hpeter+linux_kernel@...il.com>
Subject: Re: [PATCH] watchdog: f71808e_wdt: Add F81866 support

On 06/07/2016 06:40 PM, Ji-Ze Hong (Peter Hong) wrote:
> Hi Guenter,
>
> Guenter Roeck 於 2016/6/7 下午 10:06 寫道:
>> On 06/05/2016 11:58 PM, Ji-Ze Hong (Peter Hong) wrote:
>>> +#define SIO_REG_PORT_SEL    0x27    /* F81866 Multi-Function Register */
>>> +#define SIO_REG_GPIO1        0x2c    /*
>>> +                     * GPIO1 Control Register when 27h
>>> +                     * BIT3:2 = 01 & BIT0 = 0
>>> +                     *
>>> +                     * The PIN 70(GPIO15/WDTRST) is
>>> +                     * controlled by 2Ch:
>>> +                     * BIT5: 0 -> WDTRST#
>>> +                     *       1 -> GPIO15
>>> +                     */
>>> +
>> Please keep register defines in order, and move the explanation
>> to where the register values are set.
>
> OK. BTW, should I rename SIO_REG_PORT_SEL & SIO_REG_GPIO1 to
> SIO_F81866_REG_PORT_SEL & SIO_F81866_REG_GPIO1? It's only used by
> F81866.
>

Might be a good idea.

Thanks,
Guenter

>>> +    case f81866:
>>> +        /* Set pin 70 to WDTRST# */
>>> +        superio_clear_bit(watchdog.sioaddr, SIO_REG_PORT_SEL, 9);
>>> +        superio_set_bit(watchdog.sioaddr, SIO_REG_PORT_SEL, 4);
>>> +        superio_clear_bit(watchdog.sioaddr, SIO_REG_GPIO1, 0x20);
>>
>> Using BIT() here would be a bit nicer. Yes, I know, it isn't done
>> everywhere
>> in this driver, but that doesn't mean we should not do it in added code.
>
> I'll use BIT() to re-write it.
>
> Thanks