Messages in this thread | | | Subject | Re: [PATCH] watchdog: f71808e_wdt: Add F81866 support | From | Guenter Roeck <> | Date | Tue, 7 Jun 2016 18:59:37 -0700 |
| |
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
| |