[U-Boot] [PATCH] misc: Add simple driver for some Nuvoton NCT6102D devices
Bin Meng
bmeng.cn at gmail.com
Thu Jul 14 02:47:05 CEST 2016
Hi Stefan,
On Wed, Jul 13, 2016 at 6:39 PM, Stefan Roese <sr at 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 at 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 at denx.de>
>>> Cc: Bin Meng <bmeng.cn at gmail.com>
>>> Cc: Simon Glass <sjg at 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 at 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 at 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
More information about the U-Boot
mailing list