* [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