Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965330AbbBCLxV (ORCPT ); Tue, 3 Feb 2015 06:53:21 -0500 Received: from foss-mx-na.foss.arm.com ([217.140.108.86]:40817 "EHLO foss-mx-na.foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964991AbbBCLxS (ORCPT ); Tue, 3 Feb 2015 06:53:18 -0500 Date: Tue, 3 Feb 2015 11:52:50 +0000 From: Mark Rutland To: Roman Volkov Cc: Dmitry Torokhov , Rob Herring , Pawel Moll , Ian Campbell , Kumar Gala , "grant.likely@linaro.org" , Hans de Goede , Jiri Kosina , Wolfram Sang , "linux-input@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , Roman Volkov , Tony Prisk Subject: Re: [PATCH 5/5] i8042: Add i8042_dt.h glue for DT support Message-ID: <20150203115249.GB30866@leverpostej> References: <1422913730-12663-1-git-send-email-v1ron@v1ros.org> <1422913730-12663-5-git-send-email-v1ron@v1ros.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1422913730-12663-5-git-send-email-v1ron@v1ros.org> Thread-Topic: [PATCH 5/5] i8042: Add i8042_dt.h glue for DT support Accept-Language: en-GB, en-US Content-Language: en-US User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4320 Lines: 148 On Mon, Feb 02, 2015 at 09:48:50PM +0000, Roman Volkov wrote: > This header file designed to be similar to other glue layers found > for i8042. The difference is that interrupt numbers, device address, > and other information should be retrieved from the device tree. > > Signed-off-by: Tony Prisk > Signed-off-by: Roman Volkov > --- > drivers/input/serio/i8042-dt.h | 112 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 112 insertions(+) > create mode 100644 drivers/input/serio/i8042-dt.h > > diff --git a/drivers/input/serio/i8042-dt.h b/drivers/input/serio/i8042-dt.h > new file mode 100644 > index 0000000..0d1a344 > --- /dev/null > +++ b/drivers/input/serio/i8042-dt.h > @@ -0,0 +1,112 @@ > +#ifndef _I8042_DT_H > +#define _I8042_DT_H > + > +#include > +#include > +#include > + > +/* > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + */ > + > +static void __iomem *i8042_base; > +static unsigned int i8042_command_reg; > +static unsigned int i8042_status_reg; > +static unsigned int i8042_data_reg; > +#define I8042_COMMAND_REG i8042_command_reg > +#define I8042_STATUS_REG i8042_status_reg > +#define I8042_DATA_REG i8042_data_reg > + > +/* > + * Names. > + */ > + > +static const char *i8042_kbd_phys_desc; > +static const char *i8042_aux_phys_desc; > +static const char *i8042_mux_phys_desc; > +#define I8042_KBD_PHYS_DESC i8042_kbd_phys_desc > +#define I8042_AUX_PHYS_DESC i8042_aux_phys_desc > +#define I8042_MUX_PHYS_DESC i8042_mux_phys_desc > + > +/* > + * IRQs. > + */ > +static int i8042_kbd_irq; > +static int i8042_aux_irq; > +#define I8042_KBD_IRQ i8042_kbd_irq > +#define I8042_AUX_IRQ i8042_aux_irq That's a lot of static values. Surely nothing physically prevents the use of multiple i8042 chips? > + > +static inline int i8042_read_data(void) > +{ > + return readb(i8042_base + i8042_data_reg); > +} > + > +static inline int i8042_read_status(void) > +{ > + return readb(i8042_base + i8042_status_reg); > +} > + > +static inline void i8042_write_data(int val) > +{ > + writeb(val, i8042_base + i8042_data_reg); > +} > + > +static inline void i8042_write_command(int val) > +{ > + writeb(val, i8042_base + i8042_command_reg); > +} > + > +static inline int i8042_platform_init(struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + int status; > + > + i8042_base = of_iomap(np, 0); > + if (!i8042_base) > + return -ENOMEM; > + > + status = of_property_read_u32(np, "command-reg", &i8042_command_reg); > + if (status) > + return status; > + > + status = of_property_read_u32(np, "status-reg", &i8042_status_reg); > + if (status) > + return status; > + > + status = of_property_read_u32(np, "data-reg", &i8042_data_reg); > + if (status) > + return status; You should probably validate that these are within the range provided in the reg property. You also need to clean up if you fail. It looks like here and below we leak the i8042_base mapping if we decide to fail. > + > + i8042_kbd_irq = irq_of_parse_and_map(np, 0); > + i8042_aux_irq = irq_of_parse_and_map(np, 1); You can use platform_get_irq(pdev, N) here, the IRQ will already have been parsed by the core. > + status = of_property_read_string(np, "linux,kbd_phys_desc", > + &i8042_kbd_phys_desc); > + if (status) > + i8042_kbd_phys_desc = "i8042/serio0"; > + > + status = of_property_read_string(np, "linux,aux_phys_desc", > + &i8042_aux_phys_desc); > + if (status) > + i8042_aux_phys_desc = "i8042/serio1"; > + > + status = of_property_read_string(np, "linux,mux_phys_desc", > + &i8042_mux_phys_desc); > + if (status) > + i8042_mux_phys_desc = "i8042/serio%d"; User-provided values as format strings? Not a good idea. What exactly are these used for? > + > + if (of_get_property(np, "init-reset", NULL)) > + i8042_reset = true; Use of_property_read_bool. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/