[U-Boot] [PATCH 05/10] superio: Add SMSC SIO1007 driver

Simon Glass sjg at chromium.org
Tue Jan 26 18:03:27 CET 2016


Hi Bin,

On 26 January 2016 at 01:29, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Wed, Jan 6, 2016 at 8:24 AM, Simon Glass <sjg at chromium.org> wrote:
> > Hi Bin,
> >
> > On 20 December 2015 at 19:42, Bin Meng <bmeng.cn at gmail.com> wrote:
> >> Hi Simon,
> >>
> >> On Sat, Dec 19, 2015 at 10:52 AM, Simon Glass <sjg at chromium.org> wrote:
> >>> Hi Bin,
> >>>
> >>> On 11 December 2015 at 03:55, Bin Meng <bmeng.cn at gmail.com> wrote:
> >>>> The SMSC SIO1007 superio chipset integrates two ns16550 compatible
> >>>> serial ports for legacy applications, 16 GPIO pins and some other
> >>>> functionalities like power management.
> >>>>
> >>>> This adds a simple driver to enable serial port and handle GPIO.
> >>>>
> >>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> >>>> ---
> >>>>
> >>>>  drivers/misc/Makefile       |   1 +
> >>>>  drivers/misc/smsc_sio1007.c | 126 ++++++++++++++++++++++++++++++++++++++++++++
> >>>>  include/smsc_sio1007.h      | 115 ++++++++++++++++++++++++++++++++++++++++
> >>>>  3 files changed, 242 insertions(+)
> >>>>  create mode 100644 drivers/misc/smsc_sio1007.c
> >>>>  create mode 100644 include/smsc_sio1007.h
> >>>>
> >>>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> >>>> index aa137f5..6952f8ce 100644
> >>>> --- a/drivers/misc/Makefile
> >>>> +++ b/drivers/misc/Makefile
> >>>> @@ -29,6 +29,7 @@ ifdef CONFIG_DM_I2C
> >>>>  obj-$(CONFIG_SANDBOX) += i2c_eeprom_emul.o
> >>>>  endif
> >>>>  obj-$(CONFIG_SMSC_LPC47M) += smsc_lpc47m.o
> >>>> +obj-$(CONFIG_SMSC_SIO1007) += smsc_sio1007.o
> >>>>  obj-$(CONFIG_STATUS_LED) += status_led.o
> >>>>  obj-$(CONFIG_SANDBOX) += swap_case.o
> >>>>  obj-$(CONFIG_SANDBOX) += syscon_sandbox.o
> >>>> diff --git a/drivers/misc/smsc_sio1007.c b/drivers/misc/smsc_sio1007.c
> >>>> new file mode 100644
> >>>> index 0000000..79e9e15
> >>>> --- /dev/null
> >>>> +++ b/drivers/misc/smsc_sio1007.c
> >>>> @@ -0,0 +1,126 @@
> >>>> +/*
> >>>> + * Copyright (C) 2015, Bin Meng <bmeng.cn at gmail.com>
> >>>> + *
> >>>> + * SPDX-License-Identifier:    GPL-2.0+
> >>>> + */
> >>>> +
> >>>> +#include <common.h>
> >>>> +#include <asm/io.h>
> >>>> +#include <errno.h>
> >>>> +#include <smsc_sio1007.h>
> >>>> +
> >>>> +static inline u8 sio1007_read(int port, int reg)
> >>>> +{
> >>>> +       outb(reg, port);
> >>>> +
> >>>> +       return inb(port + 1);
> >>>> +}
> >>>> +
> >>>> +static inline void sio1007_write(int port, int reg, int val)
> >>>> +{
> >>>> +       outb(reg, port);
> >>>> +       outb(val, port + 1);
> >>>> +}
> >>>> +
> >>>> +static inline void sio1007_clrsetbits(int port, int reg, u8 clr, u8 set)
> >>>> +{
> >>>> +       sio1007_write(port, reg, (sio1007_read(port, reg) & ~clr) | set);
> >>>> +}
> >>>> +
> >>>> +void sio1007_enable_serial(int port, int num, int iobase, int irq)
> >>>> +{
> >>>> +       if (num < 0 || num > SIO1007_UART_NUM)
> >>>> +               return;
> >>>> +
> >>>> +       /* enter configuration state */
> >>>> +       outb(0x55, port);
> >>>> +
> >>>> +       /* power on serial port and set up its i/o base & irq */
> >>>> +       if (!num) {
> >>>> +               sio1007_clrsetbits(port, DEV_POWER_CTRL, 0, UART1_POWER_ON);
> >>>> +               sio1007_clrsetbits(port, UART1_IOBASE, 0xfe, iobase >> 2);
> >>>> +               sio1007_clrsetbits(port, UART_IRQ, 0xf0, irq << 4);
> >>>> +       } else {
> >>>> +               sio1007_clrsetbits(port, DEV_POWER_CTRL, 0, UART2_POWER_ON);
> >>>> +               sio1007_clrsetbits(port, UART2_IOBASE, 0xfe, iobase >> 2);
> >>>> +               sio1007_clrsetbits(port, UART_IRQ, 0x0f, irq);
> >>>> +       }
> >>>> +
> >>>> +       /* exit configuration state */
> >>>> +       outb(0xaa, port);
> >>>> +}
> >>>> +
> >>>> +void sio1007_enable_runtime(int port, int iobase)
> >>>> +{
> >>>> +       /* enter configuration state */
> >>>> +       outb(0x55, port);
> >>>> +
> >>>> +       /* set i/o base for the runtime register block */
> >>>> +       sio1007_clrsetbits(port, RTR_IOBASE_LOW, 0, iobase >> 4);
> >>>> +       sio1007_clrsetbits(port, RTR_IOBASE_HIGH, 0, iobase >> 12);
> >>>> +       /* turn on address decoding for this block */
> >>>> +       sio1007_clrsetbits(port, DEV_ACTIVATE, 0, RTR_EN);
> >>>> +
> >>>> +       /* exit configuration state */
> >>>> +       outb(0xaa, port);
> >>>> +}
> >>>> +
> >>>> +void sio1007_gpio_config(int port, int gpio, int dir, int pol, int type)
> >>>> +{
> >>>> +       int reg = GPIO0_DIR;
> >>>> +
> >>>> +       if (gpio < 0 || gpio > SIO1007_GPIO_NUM)
> >>>> +               return;
> >>>> +       if (gpio >= GPIO_NUM_PER_GROUP) {
> >>>> +               reg = GPIO1_DIR;
> >>>> +               gpio -= GPIO_NUM_PER_GROUP;
> >>>> +       }
> >>>> +
> >>>> +       /* enter configuration state */
> >>>> +       outb(0x55, port);
> >>>> +
> >>>> +       /* set gpio pin direction, polority and type */
> >>>> +       sio1007_clrsetbits(port, reg, 1 << gpio, dir << gpio);
> >>>> +       sio1007_clrsetbits(port, reg + 1, 1 << gpio, pol << gpio);
> >>>> +       sio1007_clrsetbits(port, reg + 2, 1 << gpio, type << gpio);
> >>>> +
> >>>> +       /* exit configuration state */
> >>>> +       outb(0xaa, port);
> >>>> +}
> >>>> +
> >>>> +int sio1007_gpio_get_value(int port, int gpio)
> >>>> +{
> >>>> +       int reg = GPIO0_DATA;
> >>>> +       int val;
> >>>> +
> >>>> +       if (gpio < 0 || gpio > SIO1007_GPIO_NUM)
> >>>> +               return -EINVAL;
> >>>> +       if (gpio >= GPIO_NUM_PER_GROUP) {
> >>>> +               reg = GPIO1_DATA;
> >>>> +               gpio -= GPIO_NUM_PER_GROUP;
> >>>> +       }
> >>>> +
> >>>> +       val = inb(port + reg);
> >>>> +       if (val & (1 << gpio))
> >>>> +               return 1;
> >>>> +       else
> >>>> +               return 0;
> >>>> +}
> >>>> +
> >>>> +void sio1007_gpio_set_value(int port, int gpio, int val)
> >>>> +{
> >>>> +       int reg = GPIO0_DATA;
> >>>> +       u8 data;
> >>>> +
> >>>> +       if (gpio < 0 || gpio > SIO1007_GPIO_NUM)
> >>>> +               return;
> >>>> +       if (gpio >= GPIO_NUM_PER_GROUP) {
> >>>> +               reg = GPIO1_DATA;
> >>>> +               gpio -= GPIO_NUM_PER_GROUP;
> >>>> +       }
> >>>> +
> >>>> +       data = inb(port + reg);
> >>>> +       data &= ~(1 << gpio);
> >>>> +       data |= (val << gpio);
> >>>> +       outb(data, port + reg);
> >>>> +}
> >>>
> >>> This should be modeled as a GPIO driver in the GPIO uclass.
> >>>
> >>>> diff --git a/include/smsc_sio1007.h b/include/smsc_sio1007.h
> >>>> new file mode 100644
> >>>> index 0000000..eff57a7
> >>>> --- /dev/null
> >>>> +++ b/include/smsc_sio1007.h
> >>>> @@ -0,0 +1,115 @@
> >>>> +/*
> >>>> + * Copyright (C) 2015, Bin Meng <bmeng.cn at gmail.com>
> >>>> + *
> >>>> + * SPDX-License-Identifier:    GPL-2.0+
> >>>> + */
> >>>> +
> >>>> +#ifndef _SMSC_SIO1007_H_
> >>>> +#define _SMSC_SIO1007_H_
> >>>> +
> >>>> +/*
> >>>> + * The I/O base address of SIO1007 at power-up is determined by the SYSOPT0
> >>>> + * and SYSOPT1 pins at the deasserting edge of PCIRST#. The combination of
> >>>> + * SYSOPT0 and SYSOPT1 determines one of the following addresses.
> >>>> + */
> >>>> +#define SIO1007_IOPORT0                0x002e
> >>>> +#define SIO1007_IOPORT1                0x004e
> >>>> +#define SIO1007_IOPORT2                0x162e
> >>>> +#define SIO1007_IOPORT3                0x164e
> >>>> +
> >>>> +/* SIO1007 registers */
> >>>> +
> >>>> +#define DEV_POWER_CTRL         0x02
> >>>> +#define UART1_POWER_ON         (1 << 3)
> >>>> +#define UART2_POWER_ON         (1 << 7)
> >>>> +
> >>>> +#define UART1_IOBASE           0x24
> >>>> +#define UART2_IOBASE           0x25
> >>>> +#define UART_IRQ               0x28
> >>>> +
> >>>> +#define RTR_IOBASE_HIGH                0x21
> >>>> +#define RTR_IOBASE_LOW         0x30
> >>>> +
> >>>> +#define GPIO0_DIR              0x31
> >>>> +#define GPIO1_DIR              0x35
> >>>> +#define GPIO_DIR_INPUT         0
> >>>> +#define GPIO_DIR_OUTPUT                1
> >>>> +
> >>>> +#define GPIO0_POL              0x32
> >>>> +#define GPIO1_POL              0x36
> >>>> +#define GPIO_POL_NO_INVERT     0
> >>>> +#define GPIO_POL_INVERT                1
> >>>> +
> >>>> +#define GPIO0_TYPE             0x33
> >>>> +#define GPIO1_TYPE             0x37
> >>>> +#define GPIO_TYPE_PUSH_PULL    0
> >>>> +#define GPIO_TYPE_OPEN_DRAIN   1
> >>>> +
> >>>> +#define DEV_ACTIVATE           0x3a
> >>>> +#define RTR_EN                 (1 << 1)
> >>>> +
> >>>> +/* Runtime register offset */
> >>>> +
> >>>> +#define GPIO0_DATA             0xc
> >>>> +#define GPIO1_DATA             0xe
> >>>> +
> >>>> +/* Number of serial ports supported */
> >>>> +#define SIO1007_UART_NUM       2
> >>>> +
> >>>> +/* Number of gpio pins supported */
> >>>> +#define GPIO_NUM_PER_GROUP     8
> >>>> +#define GPIO_GROUP_NUM         2
> >>>> +#define SIO1007_GPIO_NUM       (GPIO_NUM_PER_GROUP * GPIO_GROUP_NUM)
> >>>> +
> >>>> +/**
> >>>> + * Configure the I/O port address of the specified serial device and
> >>>> + * enable the serial device.
> >>>> + *
> >>>> + * @port:      SIO1007 I/O port address
> >>>> + * @num:       serial device number (0 or 1)
> >>>> + * @iobase:    processor I/O port address to assign to this serial device
> >>>> + * @irq:       processor IRQ number to assign to this serial device
> >>>> + */
> >>>> +void sio1007_enable_serial(int port, int num, int iobase, int irq);
> >>>
> >>> I wonder if we need a parent device which provides access to these
> >>> features, and then a GPIO child device?
> >>
> >> I was thinking we create a superio uclass for this, but I found it's
> >> not generic enough to abstract the ops for this uclass, if we take a
> >> look at the existing SMSC LPC47M and this new one. Given these
> >> routines will only need to be called just once during boot up, I
> >> decided not to use driver model on this type of device.
> >>
> >
> > But won't we want to use the 'gpio' command to access these GPIOs?
> >
>
> I don't see value of accessing them after boot. Also as you can see,
> these superio chipsets are really not good candidates to model. To
> operate on a specific GPIO pin, we need access two IO ports which
> looks quite weird, but unfortunately that is how the chipset was
> designed :(

I'm not sure why the access method matters, and I don't understand why
a GPIO would only be used on boot. But clearly you do, so I'm OK with
it.

>
> [snip]
>
> Regards,
> Bin

Regards,
Simon


More information about the U-Boot mailing list