* [U-Boot] [PATCH] misc: Add simple driver for some Nuvoton NCT6102D devices @ 2016-07-13 6:03 Stefan Roese 2016-07-13 9:45 ` Bin Meng 0 siblings, 1 reply; 4+ messages in thread From: Stefan Roese @ 2016-07-13 6:03 UTC (permalink / raw) To: u-boot This simple driver provides some functions to control some of the integrated devices. The watchdog is enabled per default. This driver adds a function to disable the watchdog. Also the internal legacy UART (io address 0x3f8/0x2f8) is enabled per default. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Bin Meng <bmeng.cn@gmail.com> Cc: Simon Glass <sjg@chromium.org> --- drivers/misc/Kconfig | 8 ++++++ drivers/misc/Makefile | 1 + drivers/misc/nuvoton_nct6102d.c | 57 +++++++++++++++++++++++++++++++++++++++++ include/nuvoton_nct6102d.h | 29 +++++++++++++++++++++ 4 files changed, 95 insertions(+) create mode 100644 drivers/misc/nuvoton_nct6102d.c create mode 100644 include/nuvoton_nct6102d.h diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 2373037..5d212a3 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -90,6 +90,14 @@ config MXC_OCOTP Programmable memory pages that are stored on the some Freescale i.MX processors. +config NUVOTON_NCT6102D + bool "Enable Nuvoton NCT6102D Super I/O driver" + help + If you say Y here, you will get support for the Nuvoton + NCT6102D Super I/O driver. This can be used to enable or + disable the legacy UART, the watchdog or other devices + in the Nuvoton Super IO chips on X86 platforms. + config PWRSEQ bool "Enable power-sequencing drivers" depends on DM diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 066639b..1b0c7b6 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_I2C_EEPROM) += i2c_eeprom.o obj-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o obj-$(CONFIG_MXC_OCOTP) += mxc_ocotp.o obj-$(CONFIG_MXS_OCOTP) += mxs_ocotp.o +obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o obj-$(CONFIG_NS87308) += ns87308.o obj-$(CONFIG_PDSP188x) += pdsp188x.o obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o diff --git a/drivers/misc/nuvoton_nct6102d.c b/drivers/misc/nuvoton_nct6102d.c new file mode 100644 index 0000000..38355db --- /dev/null +++ b/drivers/misc/nuvoton_nct6102d.c @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2016 Stefan Roese <sr@denx.de> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <nuvoton_nct6102d.h> +#include <asm/io.h> +#include <asm/pnp_def.h> + +static void superio_outb(int reg, int val) +{ + outb(reg, WDT_EFER); + outb(val, WDT_EFDR); +} + +static inline int superio_inb(int reg) +{ + outb(reg, WDT_EFER); + return inb(WDT_EFDR); +} + +static int superio_enter(void) +{ + outb(0x87, WDT_EFER); /* Enter extended function mode */ + outb(0x87, WDT_EFER); /* Again according to manual */ + + return 0; +} + +static void superio_select(int ld) +{ + superio_outb(0x07, ld); +} + +static void superio_exit(void) +{ + outb(0xAA, WDT_EFER); /* Leave extended function mode */ +} + +/* + * The Nuvoton NCT6102D starts per default after reset with both, + * the internal watchdog and the internal legacy UART enabled. This + * code provides functions to disable the watchdog and the UART + * if this is needed on platforms. + */ +int nct6102d_wdt_disable(void) +{ + superio_enter(); + /* Select logical device for WDT */ + superio_select(NCT6102D_LD_WDT); + superio_outb(NCT6102D_WDT_TIMEOUT, 0x00); + superio_exit(); + + return 0; +} diff --git a/include/nuvoton_nct6102d.h b/include/nuvoton_nct6102d.h new file mode 100644 index 0000000..297cef4 --- /dev/null +++ b/include/nuvoton_nct6102d.h @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2016 Stefan Roese <sr@denx.de> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _NUVOTON_NCT6102D_H_ +#define _NUVOTON_NCT6102D_H_ + +/* I/O address of Nuvoton Super IO chip */ +#define NCT6102D_IO_PORT 0x4e + +/* Extended Function Enable Registers */ +#define WDT_EFER (NCT6102D_IO_PORT + 0) +/* Extended Function Index Register (same as EFER) */ +#define WDT_EFIR (NCT6102D_IO_PORT + 0) +/* Extended Function Data Register */ +#define WDT_EFDR (WDT_EFIR + 1) + +/* Logical device number */ +#define NCT6102D_LD_UARTA 0x02 +#define NCT6102D_LD_WDT 0x08 + +#define NCT6102D_UARTA_ENABLE 0x30 +#define NCT6102D_WDT_TIMEOUT 0xf1 + +int nct6102d_wdt_disable(void); + +#endif /* _NUVOTON_NCT6102D_H_ */ -- 2.9.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] misc: Add simple driver for some Nuvoton NCT6102D devices 2016-07-13 6:03 [U-Boot] [PATCH] misc: Add simple driver for some Nuvoton NCT6102D devices Stefan Roese @ 2016-07-13 9:45 ` Bin Meng 2016-07-13 10:39 ` Stefan Roese 0 siblings, 1 reply; 4+ messages in thread From: Bin Meng @ 2016-07-13 9:45 UTC (permalink / raw) To: u-boot Hi Stefan, On Wed, Jul 13, 2016 at 2:03 PM, Stefan Roese <sr@denx.de> wrote: > This simple driver provides some functions to control some of the > integrated devices. The watchdog is enabled per default. This driver > adds a function to disable the watchdog. Also the internal legacy > UART (io address 0x3f8/0x2f8) is enabled per default. > > Signed-off-by: Stefan Roese <sr@denx.de> > Cc: Bin Meng <bmeng.cn@gmail.com> > Cc: Simon Glass <sjg@chromium.org> > --- > drivers/misc/Kconfig | 8 ++++++ > drivers/misc/Makefile | 1 + > drivers/misc/nuvoton_nct6102d.c | 57 +++++++++++++++++++++++++++++++++++++++++ > include/nuvoton_nct6102d.h | 29 +++++++++++++++++++++ > 4 files changed, 95 insertions(+) > create mode 100644 drivers/misc/nuvoton_nct6102d.c > create mode 100644 include/nuvoton_nct6102d.h > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 2373037..5d212a3 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -90,6 +90,14 @@ config MXC_OCOTP > Programmable memory pages that are stored on the some > Freescale i.MX processors. > > +config NUVOTON_NCT6102D > + bool "Enable Nuvoton NCT6102D Super I/O driver" > + help > + If you say Y here, you will get support for the Nuvoton > + NCT6102D Super I/O driver. This can be used to enable or > + disable the legacy UART, the watchdog or other devices > + in the Nuvoton Super IO chips on X86 platforms. > + > config PWRSEQ > bool "Enable power-sequencing drivers" > depends on DM > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index 066639b..1b0c7b6 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -24,6 +24,7 @@ obj-$(CONFIG_I2C_EEPROM) += i2c_eeprom.o > obj-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o > obj-$(CONFIG_MXC_OCOTP) += mxc_ocotp.o > obj-$(CONFIG_MXS_OCOTP) += mxs_ocotp.o > +obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o > obj-$(CONFIG_NS87308) += ns87308.o > obj-$(CONFIG_PDSP188x) += pdsp188x.o > obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o > diff --git a/drivers/misc/nuvoton_nct6102d.c b/drivers/misc/nuvoton_nct6102d.c > new file mode 100644 > index 0000000..38355db > --- /dev/null > +++ b/drivers/misc/nuvoton_nct6102d.c > @@ -0,0 +1,57 @@ > +/* > + * Copyright (C) 2016 Stefan Roese <sr@denx.de> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <nuvoton_nct6102d.h> > +#include <asm/io.h> > +#include <asm/pnp_def.h> > + > +static void superio_outb(int reg, int val) > +{ > + outb(reg, WDT_EFER); > + outb(val, WDT_EFDR); > +} > + > +static inline int superio_inb(int reg) > +{ > + outb(reg, WDT_EFER); > + return inb(WDT_EFDR); > +} > + > +static int superio_enter(void) > +{ > + outb(0x87, WDT_EFER); /* Enter extended function mode */ Can we use a macro for 0x87 if possible? > + outb(0x87, WDT_EFER); /* Again according to manual */ > + > + return 0; > +} > + > +static void superio_select(int ld) > +{ > + superio_outb(0x07, ld); Can we use a macro for 0x07 if possible? > +} > + > +static void superio_exit(void) > +{ > + outb(0xAA, WDT_EFER); /* Leave extended function mode */ Can we use a macro for 0xaa (lower case) if possible? > +} > + > +/* > + * The Nuvoton NCT6102D starts per default after reset with both, > + * the internal watchdog and the internal legacy UART enabled. This > + * code provides functions to disable the watchdog and the UART > + * if this is needed on platforms. > + */ > +int nct6102d_wdt_disable(void) > +{ > + superio_enter(); > + /* Select logical device for WDT */ > + superio_select(NCT6102D_LD_WDT); > + superio_outb(NCT6102D_WDT_TIMEOUT, 0x00); > + superio_exit(); > + > + return 0; > +} > diff --git a/include/nuvoton_nct6102d.h b/include/nuvoton_nct6102d.h > new file mode 100644 > index 0000000..297cef4 > --- /dev/null > +++ b/include/nuvoton_nct6102d.h > @@ -0,0 +1,29 @@ > +/* > + * Copyright (C) 2016 Stefan Roese <sr@denx.de> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _NUVOTON_NCT6102D_H_ > +#define _NUVOTON_NCT6102D_H_ > + > +/* I/O address of Nuvoton Super IO chip */ > +#define NCT6102D_IO_PORT 0x4e Is this I/O address configurable at several predefined address? eg: can be 0x4e, or 0x6e depending on some I/O strap pins. If it is configurable, maybe we need add a parameter for the APIs that this driver provides for the caller to pass the I/O address. > + > +/* Extended Function Enable Registers */ > +#define WDT_EFER (NCT6102D_IO_PORT + 0) WDT_ prefix looks odd to me? Should it be NCT_? > +/* Extended Function Index Register (same as EFER) */ > +#define WDT_EFIR (NCT6102D_IO_PORT + 0) > +/* Extended Function Data Register */ > +#define WDT_EFDR (WDT_EFIR + 1) > + > +/* Logical device number */ > +#define NCT6102D_LD_UARTA 0x02 Is this needed? > +#define NCT6102D_LD_WDT 0x08 > + > +#define NCT6102D_UARTA_ENABLE 0x30 Is this needed? > +#define NCT6102D_WDT_TIMEOUT 0xf1 > + > +int nct6102d_wdt_disable(void); > + > +#endif /* _NUVOTON_NCT6102D_H_ */ > -- Regards, Bin ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] misc: Add simple driver for some Nuvoton NCT6102D devices 2016-07-13 9:45 ` Bin Meng @ 2016-07-13 10:39 ` Stefan Roese 2016-07-14 0:47 ` Bin Meng 0 siblings, 1 reply; 4+ messages in thread From: Stefan Roese @ 2016-07-13 10:39 UTC (permalink / raw) To: u-boot Hi Bin, On 13.07.2016 11:45, Bin Meng wrote: > On Wed, Jul 13, 2016 at 2:03 PM, Stefan Roese <sr@denx.de> wrote: >> This simple driver provides some functions to control some of the >> integrated devices. The watchdog is enabled per default. This driver >> adds a function to disable the watchdog. Also the internal legacy >> UART (io address 0x3f8/0x2f8) is enabled per default. >> >> Signed-off-by: Stefan Roese <sr@denx.de> >> Cc: Bin Meng <bmeng.cn@gmail.com> >> Cc: Simon Glass <sjg@chromium.org> >> --- >> drivers/misc/Kconfig | 8 ++++++ >> drivers/misc/Makefile | 1 + >> drivers/misc/nuvoton_nct6102d.c | 57 +++++++++++++++++++++++++++++++++++++++++ >> include/nuvoton_nct6102d.h | 29 +++++++++++++++++++++ >> 4 files changed, 95 insertions(+) >> create mode 100644 drivers/misc/nuvoton_nct6102d.c >> create mode 100644 include/nuvoton_nct6102d.h >> >> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig >> index 2373037..5d212a3 100644 >> --- a/drivers/misc/Kconfig >> +++ b/drivers/misc/Kconfig >> @@ -90,6 +90,14 @@ config MXC_OCOTP >> Programmable memory pages that are stored on the some >> Freescale i.MX processors. >> >> +config NUVOTON_NCT6102D >> + bool "Enable Nuvoton NCT6102D Super I/O driver" >> + help >> + If you say Y here, you will get support for the Nuvoton >> + NCT6102D Super I/O driver. This can be used to enable or >> + disable the legacy UART, the watchdog or other devices >> + in the Nuvoton Super IO chips on X86 platforms. >> + >> config PWRSEQ >> bool "Enable power-sequencing drivers" >> depends on DM >> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile >> index 066639b..1b0c7b6 100644 >> --- a/drivers/misc/Makefile >> +++ b/drivers/misc/Makefile >> @@ -24,6 +24,7 @@ obj-$(CONFIG_I2C_EEPROM) += i2c_eeprom.o >> obj-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o >> obj-$(CONFIG_MXC_OCOTP) += mxc_ocotp.o >> obj-$(CONFIG_MXS_OCOTP) += mxs_ocotp.o >> +obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o >> obj-$(CONFIG_NS87308) += ns87308.o >> obj-$(CONFIG_PDSP188x) += pdsp188x.o >> obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o >> diff --git a/drivers/misc/nuvoton_nct6102d.c b/drivers/misc/nuvoton_nct6102d.c >> new file mode 100644 >> index 0000000..38355db >> --- /dev/null >> +++ b/drivers/misc/nuvoton_nct6102d.c >> @@ -0,0 +1,57 @@ >> +/* >> + * Copyright (C) 2016 Stefan Roese <sr@denx.de> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include <common.h> >> +#include <nuvoton_nct6102d.h> >> +#include <asm/io.h> >> +#include <asm/pnp_def.h> >> + >> +static void superio_outb(int reg, int val) >> +{ >> + outb(reg, WDT_EFER); >> + outb(val, WDT_EFDR); >> +} >> + >> +static inline int superio_inb(int reg) >> +{ >> + outb(reg, WDT_EFER); >> + return inb(WDT_EFDR); >> +} >> + >> +static int superio_enter(void) >> +{ >> + outb(0x87, WDT_EFER); /* Enter extended function mode */ > > Can we use a macro for 0x87 if possible? Sure. Will change in v2. >> + outb(0x87, WDT_EFER); /* Again according to manual */ >> + >> + return 0; >> +} >> + >> +static void superio_select(int ld) >> +{ >> + superio_outb(0x07, ld); > > Can we use a macro for 0x07 if possible? Okay. >> +} >> + >> +static void superio_exit(void) >> +{ >> + outb(0xAA, WDT_EFER); /* Leave extended function mode */ > > Can we use a macro for 0xaa (lower case) if possible? Okay. >> +} >> + >> +/* >> + * The Nuvoton NCT6102D starts per default after reset with both, >> + * the internal watchdog and the internal legacy UART enabled. This >> + * code provides functions to disable the watchdog and the UART >> + * if this is needed on platforms. >> + */ >> +int nct6102d_wdt_disable(void) >> +{ >> + superio_enter(); >> + /* Select logical device for WDT */ >> + superio_select(NCT6102D_LD_WDT); >> + superio_outb(NCT6102D_WDT_TIMEOUT, 0x00); >> + superio_exit(); >> + >> + return 0; >> +} >> diff --git a/include/nuvoton_nct6102d.h b/include/nuvoton_nct6102d.h >> new file mode 100644 >> index 0000000..297cef4 >> --- /dev/null >> +++ b/include/nuvoton_nct6102d.h >> @@ -0,0 +1,29 @@ >> +/* >> + * Copyright (C) 2016 Stefan Roese <sr@denx.de> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#ifndef _NUVOTON_NCT6102D_H_ >> +#define _NUVOTON_NCT6102D_H_ >> + >> +/* I/O address of Nuvoton Super IO chip */ >> +#define NCT6102D_IO_PORT 0x4e > > Is this I/O address configurable at several predefined address? eg: > can be 0x4e, or 0x6e depending on some I/O strap pins. If it is > configurable, maybe we need add a parameter for the APIs that this > driver provides for the caller to pass the I/O address. Other similar Super IO chips may have different IO addresses, yes. Not sure if this Nuvoton can be configured to a different address. We could add some sort of autodetection, similar to the one implemented in the Linux watchdog driver I've cloned most of this "driver" from (drivers/watchdog/w83627hf_wdt.c). I would vote to postpone such a change to a later date when this multiple device support is needed. >> + >> +/* Extended Function Enable Registers */ >> +#define WDT_EFER (NCT6102D_IO_PORT + 0) > > WDT_ prefix looks odd to me? Should it be NCT_? Makes sense, yes. >> +/* Extended Function Index Register (same as EFER) */ >> +#define WDT_EFIR (NCT6102D_IO_PORT + 0) >> +/* Extended Function Data Register */ >> +#define WDT_EFDR (WDT_EFIR + 1) >> + >> +/* Logical device number */ >> +#define NCT6102D_LD_UARTA 0x02 > > Is this needed? No. Its a remnant from my UART tests. I could remove it in v2 but it might be helpful if somebody want to work on this driver for UART support. >> +#define NCT6102D_LD_WDT 0x08 >> + >> +#define NCT6102D_UARTA_ENABLE 0x30 > > Is this needed? Again, I can remove it in v2 if thats preferred. Thanks for the review. Thanks, Stefan ^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] misc: Add simple driver for some Nuvoton NCT6102D devices 2016-07-13 10:39 ` Stefan Roese @ 2016-07-14 0:47 ` Bin Meng 0 siblings, 0 replies; 4+ messages in thread From: Bin Meng @ 2016-07-14 0:47 UTC (permalink / raw) To: u-boot Hi Stefan, On Wed, Jul 13, 2016 at 6:39 PM, Stefan Roese <sr@denx.de> wrote: > Hi Bin, > > > On 13.07.2016 11:45, Bin Meng wrote: >> >> On Wed, Jul 13, 2016 at 2:03 PM, Stefan Roese <sr@denx.de> wrote: >>> >>> This simple driver provides some functions to control some of the >>> integrated devices. The watchdog is enabled per default. This driver >>> adds a function to disable the watchdog. Also the internal legacy >>> UART (io address 0x3f8/0x2f8) is enabled per default. >>> >>> Signed-off-by: Stefan Roese <sr@denx.de> >>> Cc: Bin Meng <bmeng.cn@gmail.com> >>> Cc: Simon Glass <sjg@chromium.org> >>> --- >>> drivers/misc/Kconfig | 8 ++++++ >>> drivers/misc/Makefile | 1 + >>> drivers/misc/nuvoton_nct6102d.c | 57 >>> +++++++++++++++++++++++++++++++++++++++++ >>> include/nuvoton_nct6102d.h | 29 +++++++++++++++++++++ >>> 4 files changed, 95 insertions(+) >>> create mode 100644 drivers/misc/nuvoton_nct6102d.c >>> create mode 100644 include/nuvoton_nct6102d.h >>> >>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig >>> index 2373037..5d212a3 100644 >>> --- a/drivers/misc/Kconfig >>> +++ b/drivers/misc/Kconfig >>> @@ -90,6 +90,14 @@ config MXC_OCOTP >>> Programmable memory pages that are stored on the some >>> Freescale i.MX processors. >>> >>> +config NUVOTON_NCT6102D >>> + bool "Enable Nuvoton NCT6102D Super I/O driver" >>> + help >>> + If you say Y here, you will get support for the Nuvoton >>> + NCT6102D Super I/O driver. This can be used to enable or >>> + disable the legacy UART, the watchdog or other devices >>> + in the Nuvoton Super IO chips on X86 platforms. >>> + >>> config PWRSEQ >>> bool "Enable power-sequencing drivers" >>> depends on DM >>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile >>> index 066639b..1b0c7b6 100644 >>> --- a/drivers/misc/Makefile >>> +++ b/drivers/misc/Makefile >>> @@ -24,6 +24,7 @@ obj-$(CONFIG_I2C_EEPROM) += i2c_eeprom.o >>> obj-$(CONFIG_FSL_MC9SDZ60) += mc9sdz60.o >>> obj-$(CONFIG_MXC_OCOTP) += mxc_ocotp.o >>> obj-$(CONFIG_MXS_OCOTP) += mxs_ocotp.o >>> +obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o >>> obj-$(CONFIG_NS87308) += ns87308.o >>> obj-$(CONFIG_PDSP188x) += pdsp188x.o >>> obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o >>> diff --git a/drivers/misc/nuvoton_nct6102d.c >>> b/drivers/misc/nuvoton_nct6102d.c >>> new file mode 100644 >>> index 0000000..38355db >>> --- /dev/null >>> +++ b/drivers/misc/nuvoton_nct6102d.c >>> @@ -0,0 +1,57 @@ >>> +/* >>> + * Copyright (C) 2016 Stefan Roese <sr@denx.de> >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#include <common.h> >>> +#include <nuvoton_nct6102d.h> >>> +#include <asm/io.h> >>> +#include <asm/pnp_def.h> >>> + >>> +static void superio_outb(int reg, int val) >>> +{ >>> + outb(reg, WDT_EFER); >>> + outb(val, WDT_EFDR); >>> +} >>> + >>> +static inline int superio_inb(int reg) >>> +{ >>> + outb(reg, WDT_EFER); >>> + return inb(WDT_EFDR); >>> +} >>> + >>> +static int superio_enter(void) >>> +{ >>> + outb(0x87, WDT_EFER); /* Enter extended function mode */ >> >> >> Can we use a macro for 0x87 if possible? > > > Sure. Will change in v2. > >>> + outb(0x87, WDT_EFER); /* Again according to manual */ >>> + >>> + return 0; >>> +} >>> + >>> +static void superio_select(int ld) >>> +{ >>> + superio_outb(0x07, ld); >> >> >> Can we use a macro for 0x07 if possible? > > > Okay. > >>> +} >>> + >>> +static void superio_exit(void) >>> +{ >>> + outb(0xAA, WDT_EFER); /* Leave extended function mode */ >> >> >> Can we use a macro for 0xaa (lower case) if possible? > > > Okay. > > >>> +} >>> + >>> +/* >>> + * The Nuvoton NCT6102D starts per default after reset with both, >>> + * the internal watchdog and the internal legacy UART enabled. This >>> + * code provides functions to disable the watchdog and the UART >>> + * if this is needed on platforms. >>> + */ >>> +int nct6102d_wdt_disable(void) >>> +{ >>> + superio_enter(); >>> + /* Select logical device for WDT */ >>> + superio_select(NCT6102D_LD_WDT); >>> + superio_outb(NCT6102D_WDT_TIMEOUT, 0x00); >>> + superio_exit(); >>> + >>> + return 0; >>> +} >>> diff --git a/include/nuvoton_nct6102d.h b/include/nuvoton_nct6102d.h >>> new file mode 100644 >>> index 0000000..297cef4 >>> --- /dev/null >>> +++ b/include/nuvoton_nct6102d.h >>> @@ -0,0 +1,29 @@ >>> +/* >>> + * Copyright (C) 2016 Stefan Roese <sr@denx.de> >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#ifndef _NUVOTON_NCT6102D_H_ >>> +#define _NUVOTON_NCT6102D_H_ >>> + >>> +/* I/O address of Nuvoton Super IO chip */ >>> +#define NCT6102D_IO_PORT 0x4e >> >> >> Is this I/O address configurable at several predefined address? eg: >> can be 0x4e, or 0x6e depending on some I/O strap pins. If it is >> configurable, maybe we need add a parameter for the APIs that this >> driver provides for the caller to pass the I/O address. > > > Other similar Super IO chips may have different IO addresses, yes. > Not sure if this Nuvoton can be configured to a different address. > We could add some sort of autodetection, similar to the one > implemented in the Linux watchdog driver I've cloned most of this > "driver" from (drivers/watchdog/w83627hf_wdt.c). I would vote to > postpone such a change to a later date when this multiple device > support is needed. OK. > >>> + >>> +/* Extended Function Enable Registers */ >>> +#define WDT_EFER (NCT6102D_IO_PORT + 0) >> >> >> WDT_ prefix looks odd to me? Should it be NCT_? > > > Makes sense, yes. > >>> +/* Extended Function Index Register (same as EFER) */ >>> +#define WDT_EFIR (NCT6102D_IO_PORT + 0) >>> +/* Extended Function Data Register */ >>> +#define WDT_EFDR (WDT_EFIR + 1) >>> + >>> +/* Logical device number */ >>> +#define NCT6102D_LD_UARTA 0x02 >> >> >> Is this needed? > > > No. Its a remnant from my UART tests. I could remove it in v2 but it > might be helpful if somebody want to work on this driver for UART > support. Sure, let's leave it there. > >>> +#define NCT6102D_LD_WDT 0x08 >>> + >>> +#define NCT6102D_UARTA_ENABLE 0x30 >> >> >> Is this needed? > > > Again, I can remove it in v2 if thats preferred. > > Thanks for the review. Regards, Bin ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-14 0:47 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-07-13 6:03 [U-Boot] [PATCH] misc: Add simple driver for some Nuvoton NCT6102D devices Stefan Roese 2016-07-13 9:45 ` Bin Meng 2016-07-13 10:39 ` Stefan Roese 2016-07-14 0:47 ` Bin Meng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox