From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Wed, 13 Jul 2016 12:39:34 +0200 Subject: [U-Boot] [PATCH] misc: Add simple driver for some Nuvoton NCT6102D devices In-Reply-To: References: <20160713060326.30209-1-sr@denx.de> Message-ID: <57861A66.3070505@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bin, On 13.07.2016 11:45, Bin Meng wrote: > On Wed, Jul 13, 2016 at 2:03 PM, Stefan Roese 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 >> Cc: Bin Meng >> Cc: Simon Glass >> --- >> 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 >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> + >> +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 >> + * >> + * 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