Message ID | 20191213231803.20766-1-wak@google.com |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | npcm7xx-lpc-bpc: Rework driver | expand |
Hi William, Thanks a lot for working on this. Thanks for raising issues and sent great solutions for it. Appreciate it! In general, we prefer to modify the existing driver and not to do a rework. (I think it can be done in this case), please let us know what do you think. On Sat, 14 Dec 2019 at 01:19, William A. Kennington III <wak@google.com> wrote: > This provides a number of fixes: > - Multiple file handles are now supported, previously using multiple > readers would cause a race in the kfifo and spew garbage to the > readers. > Indeed, there is not a support for a multiple readers; I prefer to have only a single reader per channel because I don't see too much use from the OpenBMC/other application multiple readers, so the RCU use (that is coded great in your rework) is not much needed and cause a little over head. Do you need a multiple readers in the OpenBMC user space? > - iowrite{8,16,32} are now supported correctly. Previously, the > dwcapture code would get confused by values added to the fifo for 16 > bit writes > dwcapture need to be used only with 4 byte writes, of course the host can still write in {8,16,32} but it can cause a some issues. for example writing only 16 bit from the higher address, and not getting bit 8 at byte 0 for separating between the words. I believe the issue is when writing lower 16 bits from the host(am I correct?), we think it can solved with padding the same as one 8 bits write. > - Reads from the device now only return a single post code. Previously > the read call would emit all of the post code data in a single > syscall. This was broken because it wouldn't account for partial post > code writes into the fifo, meaning the reader could get partial > 4-byte codes for dwcap. > But is seems that if the user not using dwcap each read will be 1 byte only? what if the user like to read more than one byte at a one read? as I mentioned above the dwcap is tricky (probably we needed to explained the use of it better) so the write of 16 bit can be problematic. I think we can solve the 16 bit write by doing a padding the same as we doing when having 8 bit write, and when dwcap is enabling We can read only 4 byte multiplier. > > Tested: > Ran as a module with multiple readers and saw the correct values > reaching all of the readers. Also tested adding and removing readers > at runtime and reloading the kernel module and validating the > register state. > > Change-Id: Ic979f523ccc7cda76a2328c5f8c869aa25d7204d > Signed-off-by: William A. Kennington III <wak@google.com> > --- > drivers/misc/npcm7xx-lpc-bpc.c | 388 ++++++++++++++++++++------------- > 1 file changed, 235 insertions(+), 153 deletions(-) > > diff --git a/drivers/misc/npcm7xx-lpc-bpc.c > b/drivers/misc/npcm7xx-lpc-bpc.c > index e014e07cd4a46..b04323c4f932d 100644 > --- a/drivers/misc/npcm7xx-lpc-bpc.c > +++ b/drivers/misc/npcm7xx-lpc-bpc.c > @@ -10,6 +10,7 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > +#include <linux/slab.h> > #include <linux/miscdevice.h> > #include <linux/poll.h> > > @@ -28,270 +29,348 @@ > #define NPCM7XX_BPCFA1L_REG 0x10 //BIOS POST Code FIFO Address 1 LSB > #define NPCM7XX_BPCFA1M_REG 0x12 //BIOS POST Code FIFO Address 1 MSB > > -/*BIOS regiser data*/ > +/* BIOS regiser data */ > #define FIFO_IOADDR1_ENABLE 0x80 > #define FIFO_IOADDR2_ENABLE 0x40 > > /* BPC interface package and structure definition */ > -#define BPC_KFIFO_SIZE 0x400 > +#define BPC_KFIFO_SIZE 0x100 > > -/*BPC regiser data*/ > +/* BPC regiser data */ > #define FIFO_DATA_VALID 0x80 > #define FIFO_OVERFLOW 0x20 > #define FIFO_READY_INT_ENABLE 0x8 > #define FIFO_DWCAPTURE 0x4 > #define FIFO_ADDR_DECODE 0x1 > > -/*Host Reset*/ > +/* Host Reset */ > #define HOST_RESET_INT_ENABLE 0x10 > #define HOST_RESET_CHANGED 0x40 > > +struct npcm7xx_code { > + u32 data; > + u8 len; > +}; > + > +struct npcm7xx_bpc_file_data { > + struct list_head list; > + struct npcm7xx_bpc_channel *ch; > + DECLARE_KFIFO(codes, struct npcm7xx_code, BPC_KFIFO_SIZE); > + bool host_reset; > +}; > + > struct npcm7xx_bpc_channel { > - struct npcm7xx_bpc *data; > - struct kfifo fifo; > + struct npcm7xx_bpc *drv; > wait_queue_head_t wq; > - bool host_reset; > + struct list_head files; > struct miscdevice miscdev; > }; > > struct npcm7xx_bpc { > void __iomem *base; > + struct npcm7xx_bpc_channel chs[NUM_BPC_CHANNELS]; > int irq; > bool en_dwcap; > - struct npcm7xx_bpc_channel ch[NUM_BPC_CHANNELS]; > }; > > -static struct npcm7xx_bpc_channel *npcm7xx_file_to_ch(struct file *file) > +static int npcm7xx_bpc_open(struct inode *inode, struct file *file) > { > - return container_of(file->private_data, struct npcm7xx_bpc_channel, > - miscdev); > + struct npcm7xx_bpc_file_data *data; > + > + data = kmalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + INIT_KFIFO(data->codes); > + data->ch = container_of(file->private_data, > + struct npcm7xx_bpc_channel, miscdev); > + data->host_reset = false; > + > + file->private_data = data; > + list_add_rcu(&data->list, &data->ch->files); > + return 0; > +} > + > +static int npcm7xx_bpc_release(struct inode *inode, struct file *file) > +{ > + struct npcm7xx_bpc_file_data *data = file->private_data; > + > + if (!data) > + return -EIO; > + > + list_del_rcu(&data->list); > + synchronize_rcu(); > + > + file->private_data = NULL; > + kfree(data); > + return 0; > } > > static ssize_t npcm7xx_bpc_read(struct file *file, char __user *buffer, > size_t count, loff_t *ppos) > { > - struct npcm7xx_bpc_channel *chan = npcm7xx_file_to_ch(file); > - struct npcm7xx_bpc *lpc_bpc = chan->data; > - unsigned int copied; > + struct npcm7xx_bpc_file_data *data = file->private_data; > + struct npcm7xx_code code; > int ret = 0; > - int cond_size = 1; > - > - if (lpc_bpc->en_dwcap) > - cond_size = 3; > > - if (kfifo_len(&chan->fifo) < cond_size) { > + while (!kfifo_get(&data->codes, &code)) { > if (file->f_flags & O_NONBLOCK) > return -EAGAIN; > > ret = wait_event_interruptible > - (chan->wq, kfifo_len(&chan->fifo) > cond_size); > + (data->ch->wq, kfifo_len(&data->codes) > 0); > if (ret == -ERESTARTSYS) > return -EINTR; > } > > - ret = kfifo_to_user(&chan->fifo, buffer, count, &copied); > + if (code.len < count) > + count = code.len; > > - return ret ? ret : copied; > + ret = copy_to_user(buffer, &code.data, count); > + if (ret != 0) > + return -EFAULT; > + > + return count; > } > > static __poll_t npcm7xx_bpc_poll(struct file *file, > struct poll_table_struct *pt) > { > - struct npcm7xx_bpc_channel *chan = npcm7xx_file_to_ch(file); > + struct npcm7xx_bpc_file_data *data = file->private_data; > __poll_t mask = 0; > > - poll_wait(file, &chan->wq, pt); > - if (!kfifo_is_empty(&chan->fifo)) > + poll_wait(file, &data->ch->wq, pt); > + if (!kfifo_is_empty(&data->codes)) > mask |= POLLIN; > > - if (chan->host_reset) { > + if (data->host_reset) { > mask |= POLLHUP; > - chan->host_reset = false; > + data->host_reset = false; > } > > return mask; > } > > -static const struct file_operations npcm7xx_bpc_fops = { > +static const struct file_operations npcm7xx_bpc_channel_fops = { > .owner = THIS_MODULE, > + .open = npcm7xx_bpc_open, > + .release = npcm7xx_bpc_release, > .read = npcm7xx_bpc_read, > .poll = npcm7xx_bpc_poll, > .llseek = noop_llseek, > }; > > -static irqreturn_t npcm7xx_bpc_irq(int irq, void *arg) > +static void npcm7xx_bpc_channel_update(struct npcm7xx_bpc_channel *ch, > + const struct npcm7xx_code *code) > { > - struct npcm7xx_bpc *lpc_bpc = arg; > - u8 fifo_st; > - u8 host_st; > - u8 addr_index = 0; > - u8 Data; > - u8 padzero[3] = {0}; > - u8 last_addr_bit = 0; > - bool isr_flag = false; > - > - fifo_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFSTAT_REG); > - while (FIFO_DATA_VALID & fifo_st) { > - /* If dwcapture enabled only channel 0 (FIFO 0) used */ > - if (!lpc_bpc->en_dwcap) > - addr_index = fifo_st & FIFO_ADDR_DECODE; > - else > - last_addr_bit = fifo_st & FIFO_ADDR_DECODE; > - > - /*Read data from FIFO to clear interrupt*/ > - Data = ioread8(lpc_bpc->base + NPCM7XX_BPCFDATA_REG); > - if (kfifo_is_full(&lpc_bpc->ch[addr_index].fifo)) > - kfifo_skip(&lpc_bpc->ch[addr_index].fifo); > - kfifo_put(&lpc_bpc->ch[addr_index].fifo, Data); > - if (fifo_st & FIFO_OVERFLOW) > - pr_info("BIOS Post Codes FIFO Overflow!!!\n"); > + struct npcm7xx_bpc_file_data *data; > > - fifo_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFSTAT_REG); > - if (lpc_bpc->en_dwcap && last_addr_bit) { > - if ((fifo_st & FIFO_ADDR_DECODE) || > - ((FIFO_DATA_VALID & fifo_st) == 0)) { > - while > (kfifo_avail(&lpc_bpc->ch[addr_index].fifo) < DW_PAD_SIZE) > - > kfifo_skip(&lpc_bpc->ch[addr_index].fifo); > - kfifo_in(&lpc_bpc->ch[addr_index].fifo, > - padzero, DW_PAD_SIZE); > - } > + if (!ch->drv) { > + pr_warn("BIOS Post Code Update for unconfigured > channel\n"); > + return; > + } > + > + list_for_each_entry_rcu(data, &ch->files, list) { > + if (kfifo_is_full(&data->codes)) > + kfifo_skip(&data->codes); > + kfifo_put(&data->codes, *code); > + } > +} > + > +static void npcm7xx_bpc_channel_wake(struct npcm7xx_bpc_channel *ch) > +{ > + if (!ch->drv) > + return; > + > + wake_up_interruptible(&ch->wq); > +} > + > +static void npcm7xx_bpc_host_reset(struct npcm7xx_bpc *bpc) > +{ > + struct npcm7xx_bpc_file_data *data; > + u8 i; > + > + for (i = 0; i < NUM_BPC_CHANNELS; ++i) { > + if (!bpc->chs[i].drv) > + continue; > + list_for_each_entry_rcu(data, &bpc->chs[i].files, list) { > + data->host_reset = true; > } > - isr_flag = true; > } > +} > + > +static irqreturn_t npcm7xx_bpc_irq(int irq, void *arg) > +{ > + struct npcm7xx_bpc *bpc = arg; > + struct npcm7xx_code code = { > + .len = 0, > + .data = 0, > + }; > + bool ch_wake[NUM_BPC_CHANNELS] = {}; > + u8 read_byte; > + u8 status; > + u8 ch_i; > + bool reg_valid; > + irqreturn_t ret = IRQ_NONE; > + > + rcu_read_lock(); > + > + while (true) { > + status = ioread8(bpc->base + NPCM7XX_BPCFSTAT_REG); > + reg_valid = status & FIFO_DATA_VALID; > + if (code.len > 0 && (!reg_valid || !bpc->en_dwcap || > + status & FIFO_ADDR_DECODE)) { > + npcm7xx_bpc_channel_update(&bpc->chs[ch_i], &code); > + ch_wake[ch_i] = true; > + code.len = 0; > + code.data = 0; > + } > + if (!reg_valid) > + break; > > - host_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFMSTAT_REG); > - if (host_st & HOST_RESET_CHANGED) { > - iowrite8(HOST_RESET_CHANGED, > - lpc_bpc->base + NPCM7XX_BPCFMSTAT_REG); > - lpc_bpc->ch[addr_index].host_reset = true; > - isr_flag = true; > + if (status & FIFO_OVERFLOW) > + pr_info("BIOS Post Codes FIFO Overflow!!!\n"); > + > + ch_i = bpc->en_dwcap ? 0 : status & FIFO_ADDR_DECODE; > + read_byte = ioread8(bpc->base + NPCM7XX_BPCFDATA_REG); > + code.data |= read_byte << (code.len++ << 3); > } > > - if (isr_flag) { > - wake_up_interruptible(&lpc_bpc->ch[addr_index].wq); > - return IRQ_HANDLED; > + status = ioread8(bpc->base + NPCM7XX_BPCFMSTAT_REG); > + if (status & HOST_RESET_CHANGED) { > + iowrite8(HOST_RESET_CHANGED, bpc->base + > NPCM7XX_BPCFMSTAT_REG); > + npcm7xx_bpc_host_reset(bpc); > + for (ch_i = 0; ch_i < NUM_BPC_CHANNELS; ++ch_i) > + ch_wake[ch_i] = true; > } > > - return IRQ_NONE; > + rcu_read_unlock(); > + > + for (ch_i = 0; ch_i < NUM_BPC_CHANNELS; ++ch_i) > + if (ch_wake[ch_i]) { > + npcm7xx_bpc_channel_wake(&bpc->chs[ch_i]); > + ret = IRQ_HANDLED; > + } > + > + return ret; > } > > -static int npcm7xx_bpc_config_irq(struct npcm7xx_bpc *lpc_bpc, > +static int npcm7xx_bpc_config_irq(struct npcm7xx_bpc *bpc, > struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > int rc; > > - lpc_bpc->irq = platform_get_irq(pdev, 0); > - if (lpc_bpc->irq < 0) { > + bpc->irq = platform_get_irq(pdev, 0); > + if (bpc->irq < 0) { > dev_err(dev, "get IRQ failed\n"); > - return lpc_bpc->irq; > + return bpc->irq; > } > > - rc = devm_request_irq(dev, lpc_bpc->irq, > + rc = devm_request_irq(dev, bpc->irq, > npcm7xx_bpc_irq, IRQF_SHARED, > - DEVICE_NAME, lpc_bpc); > + DEVICE_NAME, bpc); > if (rc < 0) { > - dev_warn(dev, "Unable to request IRQ %d\n", lpc_bpc->irq); > + dev_err(dev, "Unable to request IRQ %d\n", bpc->irq); > return rc; > } > > return 0; > } > > -static int npcm7xx_enable_bpc(struct npcm7xx_bpc *lpc_bpc, struct device > *dev, > - int channel, u16 lpc_port) > +static int npcm7xx_bpc_channel_enable(struct npcm7xx_bpc *bpc, struct > device *dev, > + int channel, u16 lpc_port) > { > + struct npcm7xx_bpc_channel *ch = &bpc->chs[channel]; > int rc; > u8 addr_en, reg_en; > > - init_waitqueue_head(&lpc_bpc->ch[channel].wq); > - > - rc = kfifo_alloc(&lpc_bpc->ch[channel].fifo, > - BPC_KFIFO_SIZE, GFP_KERNEL); > - if (rc) > - return rc; > + init_waitqueue_head(&ch->wq); > + INIT_LIST_HEAD(&ch->files); > > - lpc_bpc->ch[channel].miscdev.minor = MISC_DYNAMIC_MINOR; > - lpc_bpc->ch[channel].miscdev.name = > + ch->miscdev.minor = MISC_DYNAMIC_MINOR; > + ch->miscdev.name = > devm_kasprintf(dev, GFP_KERNEL, "%s%d", DEVICE_NAME, > channel); > - lpc_bpc->ch[channel].miscdev.fops = &npcm7xx_bpc_fops; > - lpc_bpc->ch[channel].miscdev.parent = dev; > - rc = misc_register(&lpc_bpc->ch[channel].miscdev); > + ch->miscdev.fops = &npcm7xx_bpc_channel_fops; > + ch->miscdev.parent = dev; > + rc = misc_register(&ch->miscdev); > if (rc) > return rc; > > - lpc_bpc->ch[channel].data = lpc_bpc; > - lpc_bpc->ch[channel].host_reset = false; > - > - /* Enable LPC snoop channel at requested port */ > switch (channel) { > case 0: > addr_en = FIFO_IOADDR1_ENABLE; > iowrite8((u8)lpc_port & 0xFF, > - lpc_bpc->base + NPCM7XX_BPCFA1L_REG); > + bpc->base + NPCM7XX_BPCFA1L_REG); > iowrite8((u8)(lpc_port >> 8), > - lpc_bpc->base + NPCM7XX_BPCFA1M_REG); > + bpc->base + NPCM7XX_BPCFA1M_REG); > break; > case 1: > addr_en = FIFO_IOADDR2_ENABLE; > iowrite8((u8)lpc_port & 0xFF, > - lpc_bpc->base + NPCM7XX_BPCFA2L_REG); > + bpc->base + NPCM7XX_BPCFA2L_REG); > iowrite8((u8)(lpc_port >> 8), > - lpc_bpc->base + NPCM7XX_BPCFA2M_REG); > + bpc->base + NPCM7XX_BPCFA2M_REG); > break; > default: > + misc_deregister(&ch->miscdev); > return -EINVAL; > } > > - if (lpc_bpc->en_dwcap) > + if (bpc->en_dwcap) > addr_en = FIFO_DWCAPTURE; > > - /* > - * Enable FIFO Ready Interrupt, FIFO Capture of I/O addr, > - * and Host Reset > - */ > - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); > - iowrite8(reg_en | addr_en | FIFO_READY_INT_ENABLE | > - HOST_RESET_INT_ENABLE, lpc_bpc->base + > NPCM7XX_BPCFEN_REG); > + reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); > + iowrite8(reg_en | addr_en, bpc->base + NPCM7XX_BPCFEN_REG); > > + smp_mb(); > + ch->drv = bpc; > return 0; > } > > -static void npcm7xx_disable_bpc(struct npcm7xx_bpc *lpc_bpc, int channel) > +static void npcm7xx_bpc_channel_disable(struct npcm7xx_bpc *bpc, int > channel) > { > - u8 reg_en; > + struct npcm7xx_bpc_channel *ch = &bpc->chs[channel]; > + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); > + > + if (!ch->drv) > + return; > + ch->drv = NULL; > > switch (channel) { > case 0: > - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); > - if (lpc_bpc->en_dwcap) > - iowrite8(reg_en & ~FIFO_DWCAPTURE, > - lpc_bpc->base + NPCM7XX_BPCFEN_REG); > - else > - iowrite8(reg_en & ~FIFO_IOADDR1_ENABLE, > - lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + iowrite8(reg_en & ~(FIFO_DWCAPTURE | FIFO_IOADDR1_ENABLE), > + bpc->base + NPCM7XX_BPCFEN_REG); > break; > case 1: > - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); > iowrite8(reg_en & ~FIFO_IOADDR2_ENABLE, > - lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + bpc->base + NPCM7XX_BPCFEN_REG); > break; > default: > return; > } > > - if (!(reg_en & (FIFO_IOADDR1_ENABLE | FIFO_IOADDR2_ENABLE))) > - iowrite8(reg_en & > - ~(FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE), > - lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + misc_deregister(&ch->miscdev); > +} > > - kfifo_free(&lpc_bpc->ch[channel].fifo); > - misc_deregister(&lpc_bpc->ch[channel].miscdev); > +static void npcm7xx_bpc_reset(struct npcm7xx_bpc *bpc) > +{ > + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); > + reg_en &= ~(FIFO_IOADDR1_ENABLE | FIFO_IOADDR2_ENABLE | > FIFO_DWCAPTURE | > + FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE); > + iowrite8(reg_en, bpc->base + NPCM7XX_BPCFEN_REG); > +} > + > +static void npcm7xx_bpc_enable_irq(struct npcm7xx_bpc *bpc) > +{ > + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); > + reg_en |= FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE; > + iowrite8(reg_en, bpc->base + NPCM7XX_BPCFEN_REG); > } > > static int npcm7xx_bpc_probe(struct platform_device *pdev) > { > - struct npcm7xx_bpc *lpc_bpc; > + struct npcm7xx_bpc *bpc; > struct resource *res; > struct device *dev; > u32 port; > @@ -299,8 +378,8 @@ static int npcm7xx_bpc_probe(struct platform_device > *pdev) > > dev = &pdev->dev; > > - lpc_bpc = devm_kzalloc(dev, sizeof(*lpc_bpc), GFP_KERNEL); > - if (!lpc_bpc) > + bpc = devm_kzalloc(dev, sizeof(*bpc), GFP_KERNEL); > + if (!bpc) > return -ENOMEM; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -310,11 +389,11 @@ static int npcm7xx_bpc_probe(struct platform_device > *pdev) > } > > dev_dbg(dev, "BIOS post code base resource is %pR\n", res); > - lpc_bpc->base = devm_ioremap_resource(dev, res); > - if (IS_ERR(lpc_bpc->base)) > - return PTR_ERR(lpc_bpc->base); > + bpc->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(bpc->base)) > + return PTR_ERR(bpc->base); > > - dev_set_drvdata(&pdev->dev, lpc_bpc); > + dev_set_drvdata(&pdev->dev, bpc); > > rc = of_property_read_u32_index(dev->of_node, "monitor-ports", 0, > &port); > @@ -323,14 +402,16 @@ static int npcm7xx_bpc_probe(struct platform_device > *pdev) > return -ENODEV; > } > > - lpc_bpc->en_dwcap = > + bpc->en_dwcap = > of_property_read_bool(dev->of_node, "bpc-en-dwcapture"); > > - rc = npcm7xx_bpc_config_irq(lpc_bpc, pdev); > + npcm7xx_bpc_reset(bpc); > + rc = npcm7xx_bpc_config_irq(bpc, pdev); > if (rc) > return rc; > + npcm7xx_bpc_enable_irq(bpc); > > - rc = npcm7xx_enable_bpc(lpc_bpc, dev, 0, port); > + rc = npcm7xx_bpc_channel_enable(bpc, dev, 0, port); > if (rc) { > dev_err(dev, "Enable BIOS post code I/O port 0 failed\n"); > return rc; > @@ -340,35 +421,36 @@ static int npcm7xx_bpc_probe(struct platform_device > *pdev) > * Configuration of second BPC channel port is optional > * Double-Word Capture ignoring address 2 > */ > - if (!lpc_bpc->en_dwcap) { > - if (of_property_read_u32_index(dev->of_node, > "monitor-ports", > - 1, &port) == 0) { > - rc = npcm7xx_enable_bpc(lpc_bpc, dev, 1, port); > + rc = of_property_read_u32_index(dev->of_node, "monitor-ports", 1, > + &port); > + if (rc == 0) { > + if (!bpc->en_dwcap) { > + rc = npcm7xx_bpc_channel_enable(bpc, dev, 1, port); > if (rc) { > - dev_err(dev, "Enable BIOS post code I/O > port 1 failed, disable I/O port 0\n"); > - npcm7xx_disable_bpc(lpc_bpc, 0); > + dev_err(dev, "Enable BIOS post code I/O > port 1 failed\n"); > + npcm7xx_bpc_channel_disable(bpc, 0); > + npcm7xx_bpc_reset(bpc); > return rc; > } > + } else { > + dev_warn(dev, "Ignoring monitor port 1 with > DWCAP\n"); > } > } > > - pr_info("npcm7xx BIOS post code probe\n"); > - > - return rc; > + return 0; > } > > static int npcm7xx_bpc_remove(struct platform_device *pdev) > { > - struct npcm7xx_bpc *lpc_bpc = dev_get_drvdata(&pdev->dev); > - u8 reg_en; > - > - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + struct npcm7xx_bpc *bpc = dev_get_drvdata(&pdev->dev); > + u8 i; > > - if (reg_en & FIFO_IOADDR1_ENABLE) > - npcm7xx_disable_bpc(lpc_bpc, 0); > - if (reg_en & FIFO_IOADDR2_ENABLE) > - npcm7xx_disable_bpc(lpc_bpc, 1); > + if (!bpc) > + return 0; > > + for (i = 0; i < NUM_BPC_CHANNELS; ++i) > + npcm7xx_bpc_channel_disable(bpc, i); > + npcm7xx_bpc_reset(bpc); > return 0; > } > > -- > 2.24.1 > > on the upstream side, We didn’t succeed to upstream BPC driver because Linux community wants to create a BMC driver framework in the Linux kernel driver folder for all the BMC unique modules and not using misc framework. (Joel is leading this :-)) Probably the driver will modify once we will have the BMC framework. Thanks, Tomer
Hi William, On Fri, 13 Dec 2019 at 23:18, William A. Kennington III <wak@google.com> wrote: > > This provides a number of fixes: > - Multiple file handles are now supported, previously using multiple > readers would cause a race in the kfifo and spew garbage to the > readers. > - iowrite{8,16,32} are now supported correctly. Previously, the > dwcapture code would get confused by values added to the fifo for 16 > bit writes > - Reads from the device now only return a single post code. Previously > the read call would emit all of the post code data in a single > syscall. This was broken because it wouldn't account for partial post > code writes into the fifo, meaning the reader could get partial > 4-byte codes for dwcap. Nice. This driver isn't upstream, so between yourself and Tomer I suggest you decide a direction forward. Perhaps you could take on the submission of the driver, with your patch included? We wanted to consolidate the small BMC drivers for LPC and similar functions into a drivers/soc/bmc directory. From there opportunties to share userspace interfaces between BMCs (Aspeed, Nuvoton, and potentially others) would arise. The NPCM BPC and the Aspeed snoop drivers provide similar functionality, do you think they could share a single userspace interface? Cheers, Joel > > Tested: > Ran as a module with multiple readers and saw the correct values > reaching all of the readers. Also tested adding and removing readers > at runtime and reloading the kernel module and validating the > register state. > > Change-Id: Ic979f523ccc7cda76a2328c5f8c869aa25d7204d > Signed-off-by: William A. Kennington III <wak@google.com> > --- > drivers/misc/npcm7xx-lpc-bpc.c | 388 ++++++++++++++++++++------------- > 1 file changed, 235 insertions(+), 153 deletions(-) > > diff --git a/drivers/misc/npcm7xx-lpc-bpc.c b/drivers/misc/npcm7xx-lpc-bpc.c > index e014e07cd4a46..b04323c4f932d 100644 > --- a/drivers/misc/npcm7xx-lpc-bpc.c > +++ b/drivers/misc/npcm7xx-lpc-bpc.c > @@ -10,6 +10,7 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > +#include <linux/slab.h> > #include <linux/miscdevice.h> > #include <linux/poll.h> > > @@ -28,270 +29,348 @@ > #define NPCM7XX_BPCFA1L_REG 0x10 //BIOS POST Code FIFO Address 1 LSB > #define NPCM7XX_BPCFA1M_REG 0x12 //BIOS POST Code FIFO Address 1 MSB > > -/*BIOS regiser data*/ > +/* BIOS regiser data */ > #define FIFO_IOADDR1_ENABLE 0x80 > #define FIFO_IOADDR2_ENABLE 0x40 > > /* BPC interface package and structure definition */ > -#define BPC_KFIFO_SIZE 0x400 > +#define BPC_KFIFO_SIZE 0x100 > > -/*BPC regiser data*/ > +/* BPC regiser data */ > #define FIFO_DATA_VALID 0x80 > #define FIFO_OVERFLOW 0x20 > #define FIFO_READY_INT_ENABLE 0x8 > #define FIFO_DWCAPTURE 0x4 > #define FIFO_ADDR_DECODE 0x1 > > -/*Host Reset*/ > +/* Host Reset */ > #define HOST_RESET_INT_ENABLE 0x10 > #define HOST_RESET_CHANGED 0x40 > > +struct npcm7xx_code { > + u32 data; > + u8 len; > +}; > + > +struct npcm7xx_bpc_file_data { > + struct list_head list; > + struct npcm7xx_bpc_channel *ch; > + DECLARE_KFIFO(codes, struct npcm7xx_code, BPC_KFIFO_SIZE); > + bool host_reset; > +}; > + > struct npcm7xx_bpc_channel { > - struct npcm7xx_bpc *data; > - struct kfifo fifo; > + struct npcm7xx_bpc *drv; > wait_queue_head_t wq; > - bool host_reset; > + struct list_head files; > struct miscdevice miscdev; > }; > > struct npcm7xx_bpc { > void __iomem *base; > + struct npcm7xx_bpc_channel chs[NUM_BPC_CHANNELS]; > int irq; > bool en_dwcap; > - struct npcm7xx_bpc_channel ch[NUM_BPC_CHANNELS]; > }; > > -static struct npcm7xx_bpc_channel *npcm7xx_file_to_ch(struct file *file) > +static int npcm7xx_bpc_open(struct inode *inode, struct file *file) > { > - return container_of(file->private_data, struct npcm7xx_bpc_channel, > - miscdev); > + struct npcm7xx_bpc_file_data *data; > + > + data = kmalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + INIT_KFIFO(data->codes); > + data->ch = container_of(file->private_data, > + struct npcm7xx_bpc_channel, miscdev); > + data->host_reset = false; > + > + file->private_data = data; > + list_add_rcu(&data->list, &data->ch->files); > + return 0; > +} > + > +static int npcm7xx_bpc_release(struct inode *inode, struct file *file) > +{ > + struct npcm7xx_bpc_file_data *data = file->private_data; > + > + if (!data) > + return -EIO; > + > + list_del_rcu(&data->list); > + synchronize_rcu(); > + > + file->private_data = NULL; > + kfree(data); > + return 0; > } > > static ssize_t npcm7xx_bpc_read(struct file *file, char __user *buffer, > size_t count, loff_t *ppos) > { > - struct npcm7xx_bpc_channel *chan = npcm7xx_file_to_ch(file); > - struct npcm7xx_bpc *lpc_bpc = chan->data; > - unsigned int copied; > + struct npcm7xx_bpc_file_data *data = file->private_data; > + struct npcm7xx_code code; > int ret = 0; > - int cond_size = 1; > - > - if (lpc_bpc->en_dwcap) > - cond_size = 3; > > - if (kfifo_len(&chan->fifo) < cond_size) { > + while (!kfifo_get(&data->codes, &code)) { > if (file->f_flags & O_NONBLOCK) > return -EAGAIN; > > ret = wait_event_interruptible > - (chan->wq, kfifo_len(&chan->fifo) > cond_size); > + (data->ch->wq, kfifo_len(&data->codes) > 0); > if (ret == -ERESTARTSYS) > return -EINTR; > } > > - ret = kfifo_to_user(&chan->fifo, buffer, count, &copied); > + if (code.len < count) > + count = code.len; > > - return ret ? ret : copied; > + ret = copy_to_user(buffer, &code.data, count); > + if (ret != 0) > + return -EFAULT; > + > + return count; > } > > static __poll_t npcm7xx_bpc_poll(struct file *file, > struct poll_table_struct *pt) > { > - struct npcm7xx_bpc_channel *chan = npcm7xx_file_to_ch(file); > + struct npcm7xx_bpc_file_data *data = file->private_data; > __poll_t mask = 0; > > - poll_wait(file, &chan->wq, pt); > - if (!kfifo_is_empty(&chan->fifo)) > + poll_wait(file, &data->ch->wq, pt); > + if (!kfifo_is_empty(&data->codes)) > mask |= POLLIN; > > - if (chan->host_reset) { > + if (data->host_reset) { > mask |= POLLHUP; > - chan->host_reset = false; > + data->host_reset = false; > } > > return mask; > } > > -static const struct file_operations npcm7xx_bpc_fops = { > +static const struct file_operations npcm7xx_bpc_channel_fops = { > .owner = THIS_MODULE, > + .open = npcm7xx_bpc_open, > + .release = npcm7xx_bpc_release, > .read = npcm7xx_bpc_read, > .poll = npcm7xx_bpc_poll, > .llseek = noop_llseek, > }; > > -static irqreturn_t npcm7xx_bpc_irq(int irq, void *arg) > +static void npcm7xx_bpc_channel_update(struct npcm7xx_bpc_channel *ch, > + const struct npcm7xx_code *code) > { > - struct npcm7xx_bpc *lpc_bpc = arg; > - u8 fifo_st; > - u8 host_st; > - u8 addr_index = 0; > - u8 Data; > - u8 padzero[3] = {0}; > - u8 last_addr_bit = 0; > - bool isr_flag = false; > - > - fifo_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFSTAT_REG); > - while (FIFO_DATA_VALID & fifo_st) { > - /* If dwcapture enabled only channel 0 (FIFO 0) used */ > - if (!lpc_bpc->en_dwcap) > - addr_index = fifo_st & FIFO_ADDR_DECODE; > - else > - last_addr_bit = fifo_st & FIFO_ADDR_DECODE; > - > - /*Read data from FIFO to clear interrupt*/ > - Data = ioread8(lpc_bpc->base + NPCM7XX_BPCFDATA_REG); > - if (kfifo_is_full(&lpc_bpc->ch[addr_index].fifo)) > - kfifo_skip(&lpc_bpc->ch[addr_index].fifo); > - kfifo_put(&lpc_bpc->ch[addr_index].fifo, Data); > - if (fifo_st & FIFO_OVERFLOW) > - pr_info("BIOS Post Codes FIFO Overflow!!!\n"); > + struct npcm7xx_bpc_file_data *data; > > - fifo_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFSTAT_REG); > - if (lpc_bpc->en_dwcap && last_addr_bit) { > - if ((fifo_st & FIFO_ADDR_DECODE) || > - ((FIFO_DATA_VALID & fifo_st) == 0)) { > - while (kfifo_avail(&lpc_bpc->ch[addr_index].fifo) < DW_PAD_SIZE) > - kfifo_skip(&lpc_bpc->ch[addr_index].fifo); > - kfifo_in(&lpc_bpc->ch[addr_index].fifo, > - padzero, DW_PAD_SIZE); > - } > + if (!ch->drv) { > + pr_warn("BIOS Post Code Update for unconfigured channel\n"); > + return; > + } > + > + list_for_each_entry_rcu(data, &ch->files, list) { > + if (kfifo_is_full(&data->codes)) > + kfifo_skip(&data->codes); > + kfifo_put(&data->codes, *code); > + } > +} > + > +static void npcm7xx_bpc_channel_wake(struct npcm7xx_bpc_channel *ch) > +{ > + if (!ch->drv) > + return; > + > + wake_up_interruptible(&ch->wq); > +} > + > +static void npcm7xx_bpc_host_reset(struct npcm7xx_bpc *bpc) > +{ > + struct npcm7xx_bpc_file_data *data; > + u8 i; > + > + for (i = 0; i < NUM_BPC_CHANNELS; ++i) { > + if (!bpc->chs[i].drv) > + continue; > + list_for_each_entry_rcu(data, &bpc->chs[i].files, list) { > + data->host_reset = true; > } > - isr_flag = true; > } > +} > + > +static irqreturn_t npcm7xx_bpc_irq(int irq, void *arg) > +{ > + struct npcm7xx_bpc *bpc = arg; > + struct npcm7xx_code code = { > + .len = 0, > + .data = 0, > + }; > + bool ch_wake[NUM_BPC_CHANNELS] = {}; > + u8 read_byte; > + u8 status; > + u8 ch_i; > + bool reg_valid; > + irqreturn_t ret = IRQ_NONE; > + > + rcu_read_lock(); > + > + while (true) { > + status = ioread8(bpc->base + NPCM7XX_BPCFSTAT_REG); > + reg_valid = status & FIFO_DATA_VALID; > + if (code.len > 0 && (!reg_valid || !bpc->en_dwcap || > + status & FIFO_ADDR_DECODE)) { > + npcm7xx_bpc_channel_update(&bpc->chs[ch_i], &code); > + ch_wake[ch_i] = true; > + code.len = 0; > + code.data = 0; > + } > + if (!reg_valid) > + break; > > - host_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFMSTAT_REG); > - if (host_st & HOST_RESET_CHANGED) { > - iowrite8(HOST_RESET_CHANGED, > - lpc_bpc->base + NPCM7XX_BPCFMSTAT_REG); > - lpc_bpc->ch[addr_index].host_reset = true; > - isr_flag = true; > + if (status & FIFO_OVERFLOW) > + pr_info("BIOS Post Codes FIFO Overflow!!!\n"); > + > + ch_i = bpc->en_dwcap ? 0 : status & FIFO_ADDR_DECODE; > + read_byte = ioread8(bpc->base + NPCM7XX_BPCFDATA_REG); > + code.data |= read_byte << (code.len++ << 3); > } > > - if (isr_flag) { > - wake_up_interruptible(&lpc_bpc->ch[addr_index].wq); > - return IRQ_HANDLED; > + status = ioread8(bpc->base + NPCM7XX_BPCFMSTAT_REG); > + if (status & HOST_RESET_CHANGED) { > + iowrite8(HOST_RESET_CHANGED, bpc->base + NPCM7XX_BPCFMSTAT_REG); > + npcm7xx_bpc_host_reset(bpc); > + for (ch_i = 0; ch_i < NUM_BPC_CHANNELS; ++ch_i) > + ch_wake[ch_i] = true; > } > > - return IRQ_NONE; > + rcu_read_unlock(); > + > + for (ch_i = 0; ch_i < NUM_BPC_CHANNELS; ++ch_i) > + if (ch_wake[ch_i]) { > + npcm7xx_bpc_channel_wake(&bpc->chs[ch_i]); > + ret = IRQ_HANDLED; > + } > + > + return ret; > } > > -static int npcm7xx_bpc_config_irq(struct npcm7xx_bpc *lpc_bpc, > +static int npcm7xx_bpc_config_irq(struct npcm7xx_bpc *bpc, > struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > int rc; > > - lpc_bpc->irq = platform_get_irq(pdev, 0); > - if (lpc_bpc->irq < 0) { > + bpc->irq = platform_get_irq(pdev, 0); > + if (bpc->irq < 0) { > dev_err(dev, "get IRQ failed\n"); > - return lpc_bpc->irq; > + return bpc->irq; > } > > - rc = devm_request_irq(dev, lpc_bpc->irq, > + rc = devm_request_irq(dev, bpc->irq, > npcm7xx_bpc_irq, IRQF_SHARED, > - DEVICE_NAME, lpc_bpc); > + DEVICE_NAME, bpc); > if (rc < 0) { > - dev_warn(dev, "Unable to request IRQ %d\n", lpc_bpc->irq); > + dev_err(dev, "Unable to request IRQ %d\n", bpc->irq); > return rc; > } > > return 0; > } > > -static int npcm7xx_enable_bpc(struct npcm7xx_bpc *lpc_bpc, struct device *dev, > - int channel, u16 lpc_port) > +static int npcm7xx_bpc_channel_enable(struct npcm7xx_bpc *bpc, struct device *dev, > + int channel, u16 lpc_port) > { > + struct npcm7xx_bpc_channel *ch = &bpc->chs[channel]; > int rc; > u8 addr_en, reg_en; > > - init_waitqueue_head(&lpc_bpc->ch[channel].wq); > - > - rc = kfifo_alloc(&lpc_bpc->ch[channel].fifo, > - BPC_KFIFO_SIZE, GFP_KERNEL); > - if (rc) > - return rc; > + init_waitqueue_head(&ch->wq); > + INIT_LIST_HEAD(&ch->files); > > - lpc_bpc->ch[channel].miscdev.minor = MISC_DYNAMIC_MINOR; > - lpc_bpc->ch[channel].miscdev.name = > + ch->miscdev.minor = MISC_DYNAMIC_MINOR; > + ch->miscdev.name = > devm_kasprintf(dev, GFP_KERNEL, "%s%d", DEVICE_NAME, channel); > - lpc_bpc->ch[channel].miscdev.fops = &npcm7xx_bpc_fops; > - lpc_bpc->ch[channel].miscdev.parent = dev; > - rc = misc_register(&lpc_bpc->ch[channel].miscdev); > + ch->miscdev.fops = &npcm7xx_bpc_channel_fops; > + ch->miscdev.parent = dev; > + rc = misc_register(&ch->miscdev); > if (rc) > return rc; > > - lpc_bpc->ch[channel].data = lpc_bpc; > - lpc_bpc->ch[channel].host_reset = false; > - > - /* Enable LPC snoop channel at requested port */ > switch (channel) { > case 0: > addr_en = FIFO_IOADDR1_ENABLE; > iowrite8((u8)lpc_port & 0xFF, > - lpc_bpc->base + NPCM7XX_BPCFA1L_REG); > + bpc->base + NPCM7XX_BPCFA1L_REG); > iowrite8((u8)(lpc_port >> 8), > - lpc_bpc->base + NPCM7XX_BPCFA1M_REG); > + bpc->base + NPCM7XX_BPCFA1M_REG); > break; > case 1: > addr_en = FIFO_IOADDR2_ENABLE; > iowrite8((u8)lpc_port & 0xFF, > - lpc_bpc->base + NPCM7XX_BPCFA2L_REG); > + bpc->base + NPCM7XX_BPCFA2L_REG); > iowrite8((u8)(lpc_port >> 8), > - lpc_bpc->base + NPCM7XX_BPCFA2M_REG); > + bpc->base + NPCM7XX_BPCFA2M_REG); > break; > default: > + misc_deregister(&ch->miscdev); > return -EINVAL; > } > > - if (lpc_bpc->en_dwcap) > + if (bpc->en_dwcap) > addr_en = FIFO_DWCAPTURE; > > - /* > - * Enable FIFO Ready Interrupt, FIFO Capture of I/O addr, > - * and Host Reset > - */ > - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); > - iowrite8(reg_en | addr_en | FIFO_READY_INT_ENABLE | > - HOST_RESET_INT_ENABLE, lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); > + iowrite8(reg_en | addr_en, bpc->base + NPCM7XX_BPCFEN_REG); > > + smp_mb(); > + ch->drv = bpc; > return 0; > } > > -static void npcm7xx_disable_bpc(struct npcm7xx_bpc *lpc_bpc, int channel) > +static void npcm7xx_bpc_channel_disable(struct npcm7xx_bpc *bpc, int channel) > { > - u8 reg_en; > + struct npcm7xx_bpc_channel *ch = &bpc->chs[channel]; > + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); > + > + if (!ch->drv) > + return; > + ch->drv = NULL; > > switch (channel) { > case 0: > - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); > - if (lpc_bpc->en_dwcap) > - iowrite8(reg_en & ~FIFO_DWCAPTURE, > - lpc_bpc->base + NPCM7XX_BPCFEN_REG); > - else > - iowrite8(reg_en & ~FIFO_IOADDR1_ENABLE, > - lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + iowrite8(reg_en & ~(FIFO_DWCAPTURE | FIFO_IOADDR1_ENABLE), > + bpc->base + NPCM7XX_BPCFEN_REG); > break; > case 1: > - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); > iowrite8(reg_en & ~FIFO_IOADDR2_ENABLE, > - lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + bpc->base + NPCM7XX_BPCFEN_REG); > break; > default: > return; > } > > - if (!(reg_en & (FIFO_IOADDR1_ENABLE | FIFO_IOADDR2_ENABLE))) > - iowrite8(reg_en & > - ~(FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE), > - lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + misc_deregister(&ch->miscdev); > +} > > - kfifo_free(&lpc_bpc->ch[channel].fifo); > - misc_deregister(&lpc_bpc->ch[channel].miscdev); > +static void npcm7xx_bpc_reset(struct npcm7xx_bpc *bpc) > +{ > + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); > + reg_en &= ~(FIFO_IOADDR1_ENABLE | FIFO_IOADDR2_ENABLE | FIFO_DWCAPTURE | > + FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE); > + iowrite8(reg_en, bpc->base + NPCM7XX_BPCFEN_REG); > +} > + > +static void npcm7xx_bpc_enable_irq(struct npcm7xx_bpc *bpc) > +{ > + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); > + reg_en |= FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE; > + iowrite8(reg_en, bpc->base + NPCM7XX_BPCFEN_REG); > } > > static int npcm7xx_bpc_probe(struct platform_device *pdev) > { > - struct npcm7xx_bpc *lpc_bpc; > + struct npcm7xx_bpc *bpc; > struct resource *res; > struct device *dev; > u32 port; > @@ -299,8 +378,8 @@ static int npcm7xx_bpc_probe(struct platform_device *pdev) > > dev = &pdev->dev; > > - lpc_bpc = devm_kzalloc(dev, sizeof(*lpc_bpc), GFP_KERNEL); > - if (!lpc_bpc) > + bpc = devm_kzalloc(dev, sizeof(*bpc), GFP_KERNEL); > + if (!bpc) > return -ENOMEM; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -310,11 +389,11 @@ static int npcm7xx_bpc_probe(struct platform_device *pdev) > } > > dev_dbg(dev, "BIOS post code base resource is %pR\n", res); > - lpc_bpc->base = devm_ioremap_resource(dev, res); > - if (IS_ERR(lpc_bpc->base)) > - return PTR_ERR(lpc_bpc->base); > + bpc->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(bpc->base)) > + return PTR_ERR(bpc->base); > > - dev_set_drvdata(&pdev->dev, lpc_bpc); > + dev_set_drvdata(&pdev->dev, bpc); > > rc = of_property_read_u32_index(dev->of_node, "monitor-ports", 0, > &port); > @@ -323,14 +402,16 @@ static int npcm7xx_bpc_probe(struct platform_device *pdev) > return -ENODEV; > } > > - lpc_bpc->en_dwcap = > + bpc->en_dwcap = > of_property_read_bool(dev->of_node, "bpc-en-dwcapture"); > > - rc = npcm7xx_bpc_config_irq(lpc_bpc, pdev); > + npcm7xx_bpc_reset(bpc); > + rc = npcm7xx_bpc_config_irq(bpc, pdev); > if (rc) > return rc; > + npcm7xx_bpc_enable_irq(bpc); > > - rc = npcm7xx_enable_bpc(lpc_bpc, dev, 0, port); > + rc = npcm7xx_bpc_channel_enable(bpc, dev, 0, port); > if (rc) { > dev_err(dev, "Enable BIOS post code I/O port 0 failed\n"); > return rc; > @@ -340,35 +421,36 @@ static int npcm7xx_bpc_probe(struct platform_device *pdev) > * Configuration of second BPC channel port is optional > * Double-Word Capture ignoring address 2 > */ > - if (!lpc_bpc->en_dwcap) { > - if (of_property_read_u32_index(dev->of_node, "monitor-ports", > - 1, &port) == 0) { > - rc = npcm7xx_enable_bpc(lpc_bpc, dev, 1, port); > + rc = of_property_read_u32_index(dev->of_node, "monitor-ports", 1, > + &port); > + if (rc == 0) { > + if (!bpc->en_dwcap) { > + rc = npcm7xx_bpc_channel_enable(bpc, dev, 1, port); > if (rc) { > - dev_err(dev, "Enable BIOS post code I/O port 1 failed, disable I/O port 0\n"); > - npcm7xx_disable_bpc(lpc_bpc, 0); > + dev_err(dev, "Enable BIOS post code I/O port 1 failed\n"); > + npcm7xx_bpc_channel_disable(bpc, 0); > + npcm7xx_bpc_reset(bpc); > return rc; > } > + } else { > + dev_warn(dev, "Ignoring monitor port 1 with DWCAP\n"); > } > } > > - pr_info("npcm7xx BIOS post code probe\n"); > - > - return rc; > + return 0; > } > > static int npcm7xx_bpc_remove(struct platform_device *pdev) > { > - struct npcm7xx_bpc *lpc_bpc = dev_get_drvdata(&pdev->dev); > - u8 reg_en; > - > - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); > + struct npcm7xx_bpc *bpc = dev_get_drvdata(&pdev->dev); > + u8 i; > > - if (reg_en & FIFO_IOADDR1_ENABLE) > - npcm7xx_disable_bpc(lpc_bpc, 0); > - if (reg_en & FIFO_IOADDR2_ENABLE) > - npcm7xx_disable_bpc(lpc_bpc, 1); > + if (!bpc) > + return 0; > > + for (i = 0; i < NUM_BPC_CHANNELS; ++i) > + npcm7xx_bpc_channel_disable(bpc, i); > + npcm7xx_bpc_reset(bpc); > return 0; > } > > -- > 2.24.1 >
diff --git a/drivers/misc/npcm7xx-lpc-bpc.c b/drivers/misc/npcm7xx-lpc-bpc.c index e014e07cd4a46..b04323c4f932d 100644 --- a/drivers/misc/npcm7xx-lpc-bpc.c +++ b/drivers/misc/npcm7xx-lpc-bpc.c @@ -10,6 +10,7 @@ #include <linux/of.h> #include <linux/platform_device.h> #include <linux/regmap.h> +#include <linux/slab.h> #include <linux/miscdevice.h> #include <linux/poll.h> @@ -28,270 +29,348 @@ #define NPCM7XX_BPCFA1L_REG 0x10 //BIOS POST Code FIFO Address 1 LSB #define NPCM7XX_BPCFA1M_REG 0x12 //BIOS POST Code FIFO Address 1 MSB -/*BIOS regiser data*/ +/* BIOS regiser data */ #define FIFO_IOADDR1_ENABLE 0x80 #define FIFO_IOADDR2_ENABLE 0x40 /* BPC interface package and structure definition */ -#define BPC_KFIFO_SIZE 0x400 +#define BPC_KFIFO_SIZE 0x100 -/*BPC regiser data*/ +/* BPC regiser data */ #define FIFO_DATA_VALID 0x80 #define FIFO_OVERFLOW 0x20 #define FIFO_READY_INT_ENABLE 0x8 #define FIFO_DWCAPTURE 0x4 #define FIFO_ADDR_DECODE 0x1 -/*Host Reset*/ +/* Host Reset */ #define HOST_RESET_INT_ENABLE 0x10 #define HOST_RESET_CHANGED 0x40 +struct npcm7xx_code { + u32 data; + u8 len; +}; + +struct npcm7xx_bpc_file_data { + struct list_head list; + struct npcm7xx_bpc_channel *ch; + DECLARE_KFIFO(codes, struct npcm7xx_code, BPC_KFIFO_SIZE); + bool host_reset; +}; + struct npcm7xx_bpc_channel { - struct npcm7xx_bpc *data; - struct kfifo fifo; + struct npcm7xx_bpc *drv; wait_queue_head_t wq; - bool host_reset; + struct list_head files; struct miscdevice miscdev; }; struct npcm7xx_bpc { void __iomem *base; + struct npcm7xx_bpc_channel chs[NUM_BPC_CHANNELS]; int irq; bool en_dwcap; - struct npcm7xx_bpc_channel ch[NUM_BPC_CHANNELS]; }; -static struct npcm7xx_bpc_channel *npcm7xx_file_to_ch(struct file *file) +static int npcm7xx_bpc_open(struct inode *inode, struct file *file) { - return container_of(file->private_data, struct npcm7xx_bpc_channel, - miscdev); + struct npcm7xx_bpc_file_data *data; + + data = kmalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + INIT_KFIFO(data->codes); + data->ch = container_of(file->private_data, + struct npcm7xx_bpc_channel, miscdev); + data->host_reset = false; + + file->private_data = data; + list_add_rcu(&data->list, &data->ch->files); + return 0; +} + +static int npcm7xx_bpc_release(struct inode *inode, struct file *file) +{ + struct npcm7xx_bpc_file_data *data = file->private_data; + + if (!data) + return -EIO; + + list_del_rcu(&data->list); + synchronize_rcu(); + + file->private_data = NULL; + kfree(data); + return 0; } static ssize_t npcm7xx_bpc_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos) { - struct npcm7xx_bpc_channel *chan = npcm7xx_file_to_ch(file); - struct npcm7xx_bpc *lpc_bpc = chan->data; - unsigned int copied; + struct npcm7xx_bpc_file_data *data = file->private_data; + struct npcm7xx_code code; int ret = 0; - int cond_size = 1; - - if (lpc_bpc->en_dwcap) - cond_size = 3; - if (kfifo_len(&chan->fifo) < cond_size) { + while (!kfifo_get(&data->codes, &code)) { if (file->f_flags & O_NONBLOCK) return -EAGAIN; ret = wait_event_interruptible - (chan->wq, kfifo_len(&chan->fifo) > cond_size); + (data->ch->wq, kfifo_len(&data->codes) > 0); if (ret == -ERESTARTSYS) return -EINTR; } - ret = kfifo_to_user(&chan->fifo, buffer, count, &copied); + if (code.len < count) + count = code.len; - return ret ? ret : copied; + ret = copy_to_user(buffer, &code.data, count); + if (ret != 0) + return -EFAULT; + + return count; } static __poll_t npcm7xx_bpc_poll(struct file *file, struct poll_table_struct *pt) { - struct npcm7xx_bpc_channel *chan = npcm7xx_file_to_ch(file); + struct npcm7xx_bpc_file_data *data = file->private_data; __poll_t mask = 0; - poll_wait(file, &chan->wq, pt); - if (!kfifo_is_empty(&chan->fifo)) + poll_wait(file, &data->ch->wq, pt); + if (!kfifo_is_empty(&data->codes)) mask |= POLLIN; - if (chan->host_reset) { + if (data->host_reset) { mask |= POLLHUP; - chan->host_reset = false; + data->host_reset = false; } return mask; } -static const struct file_operations npcm7xx_bpc_fops = { +static const struct file_operations npcm7xx_bpc_channel_fops = { .owner = THIS_MODULE, + .open = npcm7xx_bpc_open, + .release = npcm7xx_bpc_release, .read = npcm7xx_bpc_read, .poll = npcm7xx_bpc_poll, .llseek = noop_llseek, }; -static irqreturn_t npcm7xx_bpc_irq(int irq, void *arg) +static void npcm7xx_bpc_channel_update(struct npcm7xx_bpc_channel *ch, + const struct npcm7xx_code *code) { - struct npcm7xx_bpc *lpc_bpc = arg; - u8 fifo_st; - u8 host_st; - u8 addr_index = 0; - u8 Data; - u8 padzero[3] = {0}; - u8 last_addr_bit = 0; - bool isr_flag = false; - - fifo_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFSTAT_REG); - while (FIFO_DATA_VALID & fifo_st) { - /* If dwcapture enabled only channel 0 (FIFO 0) used */ - if (!lpc_bpc->en_dwcap) - addr_index = fifo_st & FIFO_ADDR_DECODE; - else - last_addr_bit = fifo_st & FIFO_ADDR_DECODE; - - /*Read data from FIFO to clear interrupt*/ - Data = ioread8(lpc_bpc->base + NPCM7XX_BPCFDATA_REG); - if (kfifo_is_full(&lpc_bpc->ch[addr_index].fifo)) - kfifo_skip(&lpc_bpc->ch[addr_index].fifo); - kfifo_put(&lpc_bpc->ch[addr_index].fifo, Data); - if (fifo_st & FIFO_OVERFLOW) - pr_info("BIOS Post Codes FIFO Overflow!!!\n"); + struct npcm7xx_bpc_file_data *data; - fifo_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFSTAT_REG); - if (lpc_bpc->en_dwcap && last_addr_bit) { - if ((fifo_st & FIFO_ADDR_DECODE) || - ((FIFO_DATA_VALID & fifo_st) == 0)) { - while (kfifo_avail(&lpc_bpc->ch[addr_index].fifo) < DW_PAD_SIZE) - kfifo_skip(&lpc_bpc->ch[addr_index].fifo); - kfifo_in(&lpc_bpc->ch[addr_index].fifo, - padzero, DW_PAD_SIZE); - } + if (!ch->drv) { + pr_warn("BIOS Post Code Update for unconfigured channel\n"); + return; + } + + list_for_each_entry_rcu(data, &ch->files, list) { + if (kfifo_is_full(&data->codes)) + kfifo_skip(&data->codes); + kfifo_put(&data->codes, *code); + } +} + +static void npcm7xx_bpc_channel_wake(struct npcm7xx_bpc_channel *ch) +{ + if (!ch->drv) + return; + + wake_up_interruptible(&ch->wq); +} + +static void npcm7xx_bpc_host_reset(struct npcm7xx_bpc *bpc) +{ + struct npcm7xx_bpc_file_data *data; + u8 i; + + for (i = 0; i < NUM_BPC_CHANNELS; ++i) { + if (!bpc->chs[i].drv) + continue; + list_for_each_entry_rcu(data, &bpc->chs[i].files, list) { + data->host_reset = true; } - isr_flag = true; } +} + +static irqreturn_t npcm7xx_bpc_irq(int irq, void *arg) +{ + struct npcm7xx_bpc *bpc = arg; + struct npcm7xx_code code = { + .len = 0, + .data = 0, + }; + bool ch_wake[NUM_BPC_CHANNELS] = {}; + u8 read_byte; + u8 status; + u8 ch_i; + bool reg_valid; + irqreturn_t ret = IRQ_NONE; + + rcu_read_lock(); + + while (true) { + status = ioread8(bpc->base + NPCM7XX_BPCFSTAT_REG); + reg_valid = status & FIFO_DATA_VALID; + if (code.len > 0 && (!reg_valid || !bpc->en_dwcap || + status & FIFO_ADDR_DECODE)) { + npcm7xx_bpc_channel_update(&bpc->chs[ch_i], &code); + ch_wake[ch_i] = true; + code.len = 0; + code.data = 0; + } + if (!reg_valid) + break; - host_st = ioread8(lpc_bpc->base + NPCM7XX_BPCFMSTAT_REG); - if (host_st & HOST_RESET_CHANGED) { - iowrite8(HOST_RESET_CHANGED, - lpc_bpc->base + NPCM7XX_BPCFMSTAT_REG); - lpc_bpc->ch[addr_index].host_reset = true; - isr_flag = true; + if (status & FIFO_OVERFLOW) + pr_info("BIOS Post Codes FIFO Overflow!!!\n"); + + ch_i = bpc->en_dwcap ? 0 : status & FIFO_ADDR_DECODE; + read_byte = ioread8(bpc->base + NPCM7XX_BPCFDATA_REG); + code.data |= read_byte << (code.len++ << 3); } - if (isr_flag) { - wake_up_interruptible(&lpc_bpc->ch[addr_index].wq); - return IRQ_HANDLED; + status = ioread8(bpc->base + NPCM7XX_BPCFMSTAT_REG); + if (status & HOST_RESET_CHANGED) { + iowrite8(HOST_RESET_CHANGED, bpc->base + NPCM7XX_BPCFMSTAT_REG); + npcm7xx_bpc_host_reset(bpc); + for (ch_i = 0; ch_i < NUM_BPC_CHANNELS; ++ch_i) + ch_wake[ch_i] = true; } - return IRQ_NONE; + rcu_read_unlock(); + + for (ch_i = 0; ch_i < NUM_BPC_CHANNELS; ++ch_i) + if (ch_wake[ch_i]) { + npcm7xx_bpc_channel_wake(&bpc->chs[ch_i]); + ret = IRQ_HANDLED; + } + + return ret; } -static int npcm7xx_bpc_config_irq(struct npcm7xx_bpc *lpc_bpc, +static int npcm7xx_bpc_config_irq(struct npcm7xx_bpc *bpc, struct platform_device *pdev) { struct device *dev = &pdev->dev; int rc; - lpc_bpc->irq = platform_get_irq(pdev, 0); - if (lpc_bpc->irq < 0) { + bpc->irq = platform_get_irq(pdev, 0); + if (bpc->irq < 0) { dev_err(dev, "get IRQ failed\n"); - return lpc_bpc->irq; + return bpc->irq; } - rc = devm_request_irq(dev, lpc_bpc->irq, + rc = devm_request_irq(dev, bpc->irq, npcm7xx_bpc_irq, IRQF_SHARED, - DEVICE_NAME, lpc_bpc); + DEVICE_NAME, bpc); if (rc < 0) { - dev_warn(dev, "Unable to request IRQ %d\n", lpc_bpc->irq); + dev_err(dev, "Unable to request IRQ %d\n", bpc->irq); return rc; } return 0; } -static int npcm7xx_enable_bpc(struct npcm7xx_bpc *lpc_bpc, struct device *dev, - int channel, u16 lpc_port) +static int npcm7xx_bpc_channel_enable(struct npcm7xx_bpc *bpc, struct device *dev, + int channel, u16 lpc_port) { + struct npcm7xx_bpc_channel *ch = &bpc->chs[channel]; int rc; u8 addr_en, reg_en; - init_waitqueue_head(&lpc_bpc->ch[channel].wq); - - rc = kfifo_alloc(&lpc_bpc->ch[channel].fifo, - BPC_KFIFO_SIZE, GFP_KERNEL); - if (rc) - return rc; + init_waitqueue_head(&ch->wq); + INIT_LIST_HEAD(&ch->files); - lpc_bpc->ch[channel].miscdev.minor = MISC_DYNAMIC_MINOR; - lpc_bpc->ch[channel].miscdev.name = + ch->miscdev.minor = MISC_DYNAMIC_MINOR; + ch->miscdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s%d", DEVICE_NAME, channel); - lpc_bpc->ch[channel].miscdev.fops = &npcm7xx_bpc_fops; - lpc_bpc->ch[channel].miscdev.parent = dev; - rc = misc_register(&lpc_bpc->ch[channel].miscdev); + ch->miscdev.fops = &npcm7xx_bpc_channel_fops; + ch->miscdev.parent = dev; + rc = misc_register(&ch->miscdev); if (rc) return rc; - lpc_bpc->ch[channel].data = lpc_bpc; - lpc_bpc->ch[channel].host_reset = false; - - /* Enable LPC snoop channel at requested port */ switch (channel) { case 0: addr_en = FIFO_IOADDR1_ENABLE; iowrite8((u8)lpc_port & 0xFF, - lpc_bpc->base + NPCM7XX_BPCFA1L_REG); + bpc->base + NPCM7XX_BPCFA1L_REG); iowrite8((u8)(lpc_port >> 8), - lpc_bpc->base + NPCM7XX_BPCFA1M_REG); + bpc->base + NPCM7XX_BPCFA1M_REG); break; case 1: addr_en = FIFO_IOADDR2_ENABLE; iowrite8((u8)lpc_port & 0xFF, - lpc_bpc->base + NPCM7XX_BPCFA2L_REG); + bpc->base + NPCM7XX_BPCFA2L_REG); iowrite8((u8)(lpc_port >> 8), - lpc_bpc->base + NPCM7XX_BPCFA2M_REG); + bpc->base + NPCM7XX_BPCFA2M_REG); break; default: + misc_deregister(&ch->miscdev); return -EINVAL; } - if (lpc_bpc->en_dwcap) + if (bpc->en_dwcap) addr_en = FIFO_DWCAPTURE; - /* - * Enable FIFO Ready Interrupt, FIFO Capture of I/O addr, - * and Host Reset - */ - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); - iowrite8(reg_en | addr_en | FIFO_READY_INT_ENABLE | - HOST_RESET_INT_ENABLE, lpc_bpc->base + NPCM7XX_BPCFEN_REG); + reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); + iowrite8(reg_en | addr_en, bpc->base + NPCM7XX_BPCFEN_REG); + smp_mb(); + ch->drv = bpc; return 0; } -static void npcm7xx_disable_bpc(struct npcm7xx_bpc *lpc_bpc, int channel) +static void npcm7xx_bpc_channel_disable(struct npcm7xx_bpc *bpc, int channel) { - u8 reg_en; + struct npcm7xx_bpc_channel *ch = &bpc->chs[channel]; + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); + + if (!ch->drv) + return; + ch->drv = NULL; switch (channel) { case 0: - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); - if (lpc_bpc->en_dwcap) - iowrite8(reg_en & ~FIFO_DWCAPTURE, - lpc_bpc->base + NPCM7XX_BPCFEN_REG); - else - iowrite8(reg_en & ~FIFO_IOADDR1_ENABLE, - lpc_bpc->base + NPCM7XX_BPCFEN_REG); + iowrite8(reg_en & ~(FIFO_DWCAPTURE | FIFO_IOADDR1_ENABLE), + bpc->base + NPCM7XX_BPCFEN_REG); break; case 1: - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); iowrite8(reg_en & ~FIFO_IOADDR2_ENABLE, - lpc_bpc->base + NPCM7XX_BPCFEN_REG); + bpc->base + NPCM7XX_BPCFEN_REG); break; default: return; } - if (!(reg_en & (FIFO_IOADDR1_ENABLE | FIFO_IOADDR2_ENABLE))) - iowrite8(reg_en & - ~(FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE), - lpc_bpc->base + NPCM7XX_BPCFEN_REG); + misc_deregister(&ch->miscdev); +} - kfifo_free(&lpc_bpc->ch[channel].fifo); - misc_deregister(&lpc_bpc->ch[channel].miscdev); +static void npcm7xx_bpc_reset(struct npcm7xx_bpc *bpc) +{ + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); + reg_en &= ~(FIFO_IOADDR1_ENABLE | FIFO_IOADDR2_ENABLE | FIFO_DWCAPTURE | + FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE); + iowrite8(reg_en, bpc->base + NPCM7XX_BPCFEN_REG); +} + +static void npcm7xx_bpc_enable_irq(struct npcm7xx_bpc *bpc) +{ + u8 reg_en = ioread8(bpc->base + NPCM7XX_BPCFEN_REG); + reg_en |= FIFO_READY_INT_ENABLE | HOST_RESET_INT_ENABLE; + iowrite8(reg_en, bpc->base + NPCM7XX_BPCFEN_REG); } static int npcm7xx_bpc_probe(struct platform_device *pdev) { - struct npcm7xx_bpc *lpc_bpc; + struct npcm7xx_bpc *bpc; struct resource *res; struct device *dev; u32 port; @@ -299,8 +378,8 @@ static int npcm7xx_bpc_probe(struct platform_device *pdev) dev = &pdev->dev; - lpc_bpc = devm_kzalloc(dev, sizeof(*lpc_bpc), GFP_KERNEL); - if (!lpc_bpc) + bpc = devm_kzalloc(dev, sizeof(*bpc), GFP_KERNEL); + if (!bpc) return -ENOMEM; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -310,11 +389,11 @@ static int npcm7xx_bpc_probe(struct platform_device *pdev) } dev_dbg(dev, "BIOS post code base resource is %pR\n", res); - lpc_bpc->base = devm_ioremap_resource(dev, res); - if (IS_ERR(lpc_bpc->base)) - return PTR_ERR(lpc_bpc->base); + bpc->base = devm_ioremap_resource(dev, res); + if (IS_ERR(bpc->base)) + return PTR_ERR(bpc->base); - dev_set_drvdata(&pdev->dev, lpc_bpc); + dev_set_drvdata(&pdev->dev, bpc); rc = of_property_read_u32_index(dev->of_node, "monitor-ports", 0, &port); @@ -323,14 +402,16 @@ static int npcm7xx_bpc_probe(struct platform_device *pdev) return -ENODEV; } - lpc_bpc->en_dwcap = + bpc->en_dwcap = of_property_read_bool(dev->of_node, "bpc-en-dwcapture"); - rc = npcm7xx_bpc_config_irq(lpc_bpc, pdev); + npcm7xx_bpc_reset(bpc); + rc = npcm7xx_bpc_config_irq(bpc, pdev); if (rc) return rc; + npcm7xx_bpc_enable_irq(bpc); - rc = npcm7xx_enable_bpc(lpc_bpc, dev, 0, port); + rc = npcm7xx_bpc_channel_enable(bpc, dev, 0, port); if (rc) { dev_err(dev, "Enable BIOS post code I/O port 0 failed\n"); return rc; @@ -340,35 +421,36 @@ static int npcm7xx_bpc_probe(struct platform_device *pdev) * Configuration of second BPC channel port is optional * Double-Word Capture ignoring address 2 */ - if (!lpc_bpc->en_dwcap) { - if (of_property_read_u32_index(dev->of_node, "monitor-ports", - 1, &port) == 0) { - rc = npcm7xx_enable_bpc(lpc_bpc, dev, 1, port); + rc = of_property_read_u32_index(dev->of_node, "monitor-ports", 1, + &port); + if (rc == 0) { + if (!bpc->en_dwcap) { + rc = npcm7xx_bpc_channel_enable(bpc, dev, 1, port); if (rc) { - dev_err(dev, "Enable BIOS post code I/O port 1 failed, disable I/O port 0\n"); - npcm7xx_disable_bpc(lpc_bpc, 0); + dev_err(dev, "Enable BIOS post code I/O port 1 failed\n"); + npcm7xx_bpc_channel_disable(bpc, 0); + npcm7xx_bpc_reset(bpc); return rc; } + } else { + dev_warn(dev, "Ignoring monitor port 1 with DWCAP\n"); } } - pr_info("npcm7xx BIOS post code probe\n"); - - return rc; + return 0; } static int npcm7xx_bpc_remove(struct platform_device *pdev) { - struct npcm7xx_bpc *lpc_bpc = dev_get_drvdata(&pdev->dev); - u8 reg_en; - - reg_en = ioread8(lpc_bpc->base + NPCM7XX_BPCFEN_REG); + struct npcm7xx_bpc *bpc = dev_get_drvdata(&pdev->dev); + u8 i; - if (reg_en & FIFO_IOADDR1_ENABLE) - npcm7xx_disable_bpc(lpc_bpc, 0); - if (reg_en & FIFO_IOADDR2_ENABLE) - npcm7xx_disable_bpc(lpc_bpc, 1); + if (!bpc) + return 0; + for (i = 0; i < NUM_BPC_CHANNELS; ++i) + npcm7xx_bpc_channel_disable(bpc, i); + npcm7xx_bpc_reset(bpc); return 0; }
This provides a number of fixes: - Multiple file handles are now supported, previously using multiple readers would cause a race in the kfifo and spew garbage to the readers. - iowrite{8,16,32} are now supported correctly. Previously, the dwcapture code would get confused by values added to the fifo for 16 bit writes - Reads from the device now only return a single post code. Previously the read call would emit all of the post code data in a single syscall. This was broken because it wouldn't account for partial post code writes into the fifo, meaning the reader could get partial 4-byte codes for dwcap. Tested: Ran as a module with multiple readers and saw the correct values reaching all of the readers. Also tested adding and removing readers at runtime and reloading the kernel module and validating the register state. Change-Id: Ic979f523ccc7cda76a2328c5f8c869aa25d7204d Signed-off-by: William A. Kennington III <wak@google.com> --- drivers/misc/npcm7xx-lpc-bpc.c | 388 ++++++++++++++++++++------------- 1 file changed, 235 insertions(+), 153 deletions(-)