From patchwork Mon Sep 2 00:39:10 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Hutchings X-Patchwork-Id: 2852630 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B15FAC0AB5 for ; Mon, 2 Sep 2013 00:39:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 74B2220264 for ; Mon, 2 Sep 2013 00:39:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 55AAD20258 for ; Mon, 2 Sep 2013 00:39:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757675Ab3IBAjO (ORCPT ); Sun, 1 Sep 2013 20:39:14 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:60565 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757612Ab3IBAjN (ORCPT ); Sun, 1 Sep 2013 20:39:13 -0400 Received: from deadeye.wl.decadent.org.uk ([192.168.4.101]) by shadbolt.decadent.org.uk with esmtps (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1VGIAh-000525-Ul; Mon, 02 Sep 2013 01:39:12 +0100 Received: from ben by deadeye.wl.decadent.org.uk with local (Exim 4.80) (envelope-from ) id 1VGIAg-0005By-Mg; Mon, 02 Sep 2013 01:39:10 +0100 Message-ID: <1378082350.25743.60.camel@deadeye.wl.decadent.org.uk> Subject: [PATCH 1/4] [media] lirc_bt829: Make it a proper PCI driver From: Ben Hutchings To: Jarod Wilson , Mauro Carvalho Chehab Cc: linux-media@vger.kernel.org, devel@driverdev.osuosl.org Date: Mon, 02 Sep 2013 01:39:10 +0100 In-Reply-To: <1378082213.25743.58.camel@deadeye.wl.decadent.org.uk> References: <1378082213.25743.58.camel@deadeye.wl.decadent.org.uk> X-Mailer: Evolution 3.4.4-3 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 192.168.4.101 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Replace static variables with a device structure and pass pointers to this into all the functions that need it. Fold init_module(), do_pci_probe() and atir_init_start() into a single probe function. Use dev_err() to provide context for logging. This also fixes a device reference leak, as the driver wasn't calling pci_dev_put(). Signed-off-by: Ben Hutchings --- drivers/staging/media/lirc/lirc_bt829.c | 276 +++++++++++++++++--------------- 1 file changed, 144 insertions(+), 132 deletions(-) diff --git a/drivers/staging/media/lirc/lirc_bt829.c b/drivers/staging/media/lirc/lirc_bt829.c index fa31ee7..c277bf3 100644 --- a/drivers/staging/media/lirc/lirc_bt829.c +++ b/drivers/staging/media/lirc/lirc_bt829.c @@ -30,25 +30,32 @@ #include -static int poll_main(void); -static int atir_init_start(void); +struct atir_device { + int minor; + unsigned char *pci_addr_lin; + struct lirc_driver driver; +}; -static void write_index(unsigned char index, unsigned int value); -static unsigned int read_index(unsigned char index); +static int poll_main(struct atir_device *atir); -static void do_i2c_start(void); -static void do_i2c_stop(void); +static void write_index(struct atir_device *atir, unsigned char index, + unsigned int value); +static unsigned int read_index(struct atir_device *atir, unsigned char index); -static void seems_wr_byte(unsigned char al); -static unsigned char seems_rd_byte(void); +static void do_i2c_start(struct atir_device *atir); +static void do_i2c_stop(struct atir_device *atir); -static unsigned int read_index(unsigned char al); -static void write_index(unsigned char ah, unsigned int edx); +static void seems_wr_byte(struct atir_device *atir, unsigned char al); +static unsigned char seems_rd_byte(struct atir_device *atir); + +static unsigned int read_index(struct atir_device *atir, unsigned char al); +static void write_index(struct atir_device *atir, unsigned char ah, + unsigned int edx); static void cycle_delay(int cycle); -static void do_set_bits(unsigned char bl); -static unsigned char do_get_bits(void); +static void do_set_bits(struct atir_device *atir, unsigned char bl); +static unsigned char do_get_bits(struct atir_device *atir); #define DATA_PCI_OFF 0x7FFC00 #define WAIT_CYCLE 20 @@ -62,41 +69,12 @@ static bool debug; printk(KERN_DEBUG DRIVER_NAME ": "fmt, ## args); \ } while (0) -static int atir_minor; -static unsigned long pci_addr_phys; -static unsigned char *pci_addr_lin; - -static struct lirc_driver atir_driver; - -static struct pci_dev *do_pci_probe(void) -{ - struct pci_dev *my_dev; - my_dev = pci_get_device(PCI_VENDOR_ID_ATI, - PCI_DEVICE_ID_ATI_264VT, NULL); - if (my_dev) { - pr_err("Using device: %s\n", pci_name(my_dev)); - pci_addr_phys = 0; - if (my_dev->resource[0].flags & IORESOURCE_MEM) { - pci_addr_phys = my_dev->resource[0].start; - pr_info("memory at 0x%08X\n", - (unsigned int)pci_addr_phys); - } - if (pci_addr_phys == 0) { - pr_err("no memory resource ?\n"); - return NULL; - } - } else { - pr_err("pci_probe failed\n"); - return NULL; - } - return my_dev; -} - static int atir_add_to_buf(void *data, struct lirc_buffer *buf) { + struct atir_device *atir = data; unsigned char key; int status; - status = poll_main(); + status = poll_main(atir); key = (status >> 8) & 0xFF; if (status & 0xFF) { dprintk("reading key %02X\n", key); @@ -117,172 +95,191 @@ static void atir_set_use_dec(void *data) dprintk("driver is closed\n"); } -int init_module(void) +static int atir_pci_probe(struct pci_dev *pdev, + const struct pci_device_id *entry) { - struct pci_dev *pdev; - - pdev = do_pci_probe(); - if (pdev == NULL) - return -ENODEV; - - if (!atir_init_start()) - return -ENODEV; - - strcpy(atir_driver.name, "ATIR"); - atir_driver.minor = -1; - atir_driver.code_length = 8; - atir_driver.sample_rate = 10; - atir_driver.data = 0; - atir_driver.add_to_buf = atir_add_to_buf; - atir_driver.set_use_inc = atir_set_use_inc; - atir_driver.set_use_dec = atir_set_use_dec; - atir_driver.dev = &pdev->dev; - atir_driver.owner = THIS_MODULE; - - atir_minor = lirc_register_driver(&atir_driver); - if (atir_minor < 0) { - pr_err("failed to register driver!\n"); - return atir_minor; + struct atir_device *atir; + unsigned long pci_addr_phys; + int rc; + + atir = kzalloc(sizeof(*atir), GFP_KERNEL); + if (!atir) + return -ENOMEM; + + pci_set_drvdata(pdev, atir); + + if (!(pdev->resource[0].flags & IORESOURCE_MEM)) { + dev_err(&pdev->dev, "no memory resource ?\n"); + rc = -ENODEV; + goto err_free; } - dprintk("driver is registered on minor %d\n", atir_minor); - return 0; -} + pci_addr_phys = pdev->resource[0].start; + dev_info(&pdev->dev, "memory at 0x%08X\n", + (unsigned int)pci_addr_phys); + atir->pci_addr_lin = ioremap(pci_addr_phys + DATA_PCI_OFF, 0x400); + if (atir->pci_addr_lin == 0) { + dev_err(&pdev->dev, "pci mem must be mapped\n"); + rc = -ENODEV; + goto err_free; + } -void cleanup_module(void) -{ - lirc_unregister_driver(atir_minor); + strcpy(atir->driver.name, "ATIR"); + atir->driver.minor = -1; + atir->driver.code_length = 8; + atir->driver.sample_rate = 10; + atir->driver.data = atir; + atir->driver.add_to_buf = atir_add_to_buf; + atir->driver.set_use_inc = atir_set_use_inc; + atir->driver.set_use_dec = atir_set_use_dec; + atir->driver.dev = &pdev->dev; + atir->driver.owner = THIS_MODULE; + + atir->minor = lirc_register_driver(&atir->driver); + if (atir->minor < 0) { + dev_err(&pdev->dev, "failed to register driver!\n"); + rc = atir->minor; + goto err_free; + } + dprintk("driver is registered on minor %d\n", atir->minor); + + return 0; + +err_free: + pci_set_drvdata(pdev, NULL); + kfree(atir); + return rc; } -static int atir_init_start(void) +static void atir_pci_remove(struct pci_dev *pdev) { - pci_addr_lin = ioremap(pci_addr_phys + DATA_PCI_OFF, 0x400); - if (pci_addr_lin == 0) { - pr_info("pci mem must be mapped\n"); - return 0; - } - return 1; + struct atir_device *atir = pci_get_drvdata(pdev); + + lirc_unregister_driver(atir->minor); + pci_set_drvdata(pdev, NULL); + kfree(atir); } + static void cycle_delay(int cycle) { udelay(WAIT_CYCLE*cycle); } -static int poll_main(void) +static int poll_main(struct atir_device *atir) { unsigned char status_high, status_low; - do_i2c_start(); + do_i2c_start(atir); - seems_wr_byte(0xAA); - seems_wr_byte(0x01); + seems_wr_byte(atir, 0xAA); + seems_wr_byte(atir, 0x01); - do_i2c_start(); + do_i2c_start(atir); - seems_wr_byte(0xAB); + seems_wr_byte(atir, 0xAB); - status_low = seems_rd_byte(); - status_high = seems_rd_byte(); + status_low = seems_rd_byte(atir); + status_high = seems_rd_byte(atir); - do_i2c_stop(); + do_i2c_stop(atir); return (status_high << 8) | status_low; } -static void do_i2c_start(void) +static void do_i2c_start(struct atir_device *atir) { - do_set_bits(3); + do_set_bits(atir, 3); cycle_delay(4); - do_set_bits(1); + do_set_bits(atir, 1); cycle_delay(7); - do_set_bits(0); + do_set_bits(atir, 0); cycle_delay(2); } -static void do_i2c_stop(void) +static void do_i2c_stop(struct atir_device *atir) { unsigned char bits; - bits = do_get_bits() & 0xFD; - do_set_bits(bits); + bits = do_get_bits(atir) & 0xFD; + do_set_bits(atir, bits); cycle_delay(1); bits |= 1; - do_set_bits(bits); + do_set_bits(atir, bits); cycle_delay(2); bits |= 2; - do_set_bits(bits); + do_set_bits(atir, bits); bits = 3; - do_set_bits(bits); + do_set_bits(atir, bits); cycle_delay(2); } -static void seems_wr_byte(unsigned char value) +static void seems_wr_byte(struct atir_device *atir, unsigned char value) { int i; unsigned char reg; - reg = do_get_bits(); + reg = do_get_bits(atir); for (i = 0; i < 8; i++) { if (value & 0x80) reg |= 0x02; else reg &= 0xFD; - do_set_bits(reg); + do_set_bits(atir, reg); cycle_delay(1); reg |= 1; - do_set_bits(reg); + do_set_bits(atir, reg); cycle_delay(1); reg &= 0xFE; - do_set_bits(reg); + do_set_bits(atir, reg); cycle_delay(1); value <<= 1; } cycle_delay(2); reg |= 2; - do_set_bits(reg); + do_set_bits(atir, reg); reg |= 1; - do_set_bits(reg); + do_set_bits(atir, reg); cycle_delay(1); - do_get_bits(); + do_get_bits(atir); reg &= 0xFE; - do_set_bits(reg); + do_set_bits(atir, reg); cycle_delay(3); } -static unsigned char seems_rd_byte(void) +static unsigned char seems_rd_byte(struct atir_device *atir) { int i; int rd_byte; unsigned char bits_2, bits_1; - bits_1 = do_get_bits() | 2; - do_set_bits(bits_1); + bits_1 = do_get_bits(atir) | 2; + do_set_bits(atir, bits_1); rd_byte = 0; for (i = 0; i < 8; i++) { bits_1 &= 0xFE; - do_set_bits(bits_1); + do_set_bits(atir, bits_1); cycle_delay(2); bits_1 |= 1; - do_set_bits(bits_1); + do_set_bits(atir, bits_1); cycle_delay(1); - bits_2 = do_get_bits(); + bits_2 = do_get_bits(atir); if (bits_2 & 2) rd_byte |= 1; @@ -293,15 +290,15 @@ static unsigned char seems_rd_byte(void) if (bits_2 == 0) bits_1 |= 2; - do_set_bits(bits_1); + do_set_bits(atir, bits_1); cycle_delay(2); bits_1 |= 1; - do_set_bits(bits_1); + do_set_bits(atir, bits_1); cycle_delay(3); bits_1 &= 0xFE; - do_set_bits(bits_1); + do_set_bits(atir, bits_1); cycle_delay(2); rd_byte >>= 1; @@ -309,10 +306,10 @@ static unsigned char seems_rd_byte(void) return rd_byte; } -static void do_set_bits(unsigned char new_bits) +static void do_set_bits(struct atir_device *atir, unsigned char new_bits) { int reg_val; - reg_val = read_index(0x34); + reg_val = read_index(atir, 0x34); if (new_bits & 2) { reg_val &= 0xFFFFFFDF; reg_val |= 1; @@ -321,36 +318,36 @@ static void do_set_bits(unsigned char new_bits) reg_val |= 0x20; } reg_val |= 0x10; - write_index(0x34, reg_val); + write_index(atir, 0x34, reg_val); - reg_val = read_index(0x31); + reg_val = read_index(atir, 0x31); if (new_bits & 1) reg_val |= 0x1000000; else reg_val &= 0xFEFFFFFF; reg_val |= 0x8000000; - write_index(0x31, reg_val); + write_index(atir, 0x31, reg_val); } -static unsigned char do_get_bits(void) +static unsigned char do_get_bits(struct atir_device *atir) { unsigned char bits; int reg_val; - reg_val = read_index(0x34); + reg_val = read_index(atir, 0x34); reg_val |= 0x10; reg_val &= 0xFFFFFFDF; - write_index(0x34, reg_val); + write_index(atir, 0x34, reg_val); - reg_val = read_index(0x34); + reg_val = read_index(atir, 0x34); bits = 0; if (reg_val & 8) bits |= 2; else bits &= 0xFD; - reg_val = read_index(0x31); + reg_val = read_index(atir, 0x31); if (reg_val & 0x1000000) bits |= 1; else @@ -359,26 +356,41 @@ static unsigned char do_get_bits(void) return bits; } -static unsigned int read_index(unsigned char index) +static unsigned int read_index(struct atir_device *atir, unsigned char index) { unsigned char *addr; unsigned int value; /* addr = pci_addr_lin + DATA_PCI_OFF + ((index & 0xFF) << 2); */ - addr = pci_addr_lin + ((index & 0xFF) << 2); + addr = atir->pci_addr_lin + ((index & 0xFF) << 2); value = readl(addr); return value; } -static void write_index(unsigned char index, unsigned int reg_val) +static void write_index(struct atir_device *atir, unsigned char index, + unsigned int reg_val) { unsigned char *addr; - addr = pci_addr_lin + ((index & 0xFF) << 2); + addr = atir->pci_addr_lin + ((index & 0xFF) << 2); writel(reg_val, addr); } +static DEFINE_PCI_DEVICE_TABLE(atir_pci_table) = { + { PCI_DEVICE(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_264VT) }, + { 0 } +}; + +static struct pci_driver atir_pci_driver = { + .name = KBUILD_MODNAME, + .id_table = atir_pci_table, + .probe = atir_pci_probe, + .remove = atir_pci_remove, +}; +module_pci_driver(atir_pci_driver); + MODULE_AUTHOR("Froenchenko Leonid"); MODULE_DESCRIPTION("IR remote driver for bt829 based TV cards"); MODULE_LICENSE("GPL"); +MODULE_DEVICE_TABLE(pci, atir_pci_table); module_param(debug, bool, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(debug, "Debug enabled or not");