Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754326AbbGXIse (ORCPT ); Fri, 24 Jul 2015 04:48:34 -0400 Received: from eusmtp01.atmel.com ([212.144.249.242]:33986 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753741AbbGXIsO (ORCPT ); Fri, 24 Jul 2015 04:48:14 -0400 Message-ID: <55B1FB87.4040508@atmel.com> Date: Fri, 24 Jul 2015 10:47:03 +0200 From: Nicolas Ferre Organization: atmel User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Lee Jones , Cyrille Pitchen CC: , , , , , , , , , , Subject: Re: [PATCH v7 2/2] mfd: atmel-flexcom: add a driver for Atmel Flexible Serial Communication Unit References: <55921f08b2d5d14256141edbe12287fba55860c4.1437669004.git.cyrille.pitchen@atmel.com> <20150724084355.GY3436@x1> In-Reply-To: <20150724084355.GY3436@x1> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.161.30.18] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7007 Lines: 208 Le 24/07/2015 10:43, Lee Jones a écrit : > On Thu, 23 Jul 2015, Cyrille Pitchen wrote: > >> This driver supports the new Atmel Flexcom. The Flexcom is a wrapper which >> integrates one SPI controller, one I2C controller and one USART. Only one >> function can be enabled at a time. This driver selects the function once >> for all, when the Flexcom is probed, using the "reg" property of the first >> (should be unique) available DT child node. >> >> This driver has chosen to present the Flexcom to the system as a MFD so >> the implementation is seamless for the existing Atmel SPI, I2C and USART >> drivers. >> >> Also the Flexcom embeds FIFOs: the latest patches of the SPI, I2C and >> USART drivers take advantage of this new feature. >> >> Signed-off-by: Cyrille Pitchen >> --- >> drivers/mfd/Kconfig | 11 +++++ >> drivers/mfd/Makefile | 1 + >> drivers/mfd/atmel-flexcom.c | 113 ++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 125 insertions(+) >> create mode 100644 drivers/mfd/atmel-flexcom.c >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> index 653815950aa2..2c75472c679c 100644 >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -60,6 +60,17 @@ config MFD_AAT2870_CORE >> additional drivers must be enabled in order to use the >> functionality of the device. >> >> +config MFD_ATMEL_FLEXCOM >> + tristate "Atmel Flexcom (Flexible Serial Communication Unit)" >> + select MFD_CORE >> + depends on OF >> + help >> + Select this to get support for Atmel Flexcom. This is a wrapper >> + which embeds a SPI controller, a I2C controller and a USART. Only >> + one function can be used at a time. The choice is done at boot time >> + by the probe function of this MFD driver according to a device tree >> + property. > > Please grep for QCOM_GSBI in drivers/soc/qcom/* > > Similar right? > > Perhaps this should live in drivers/soc/atmel/* instead? > >> config MFD_ATMEL_HLCDC >> tristate "Atmel HLCDC (High-end LCD Controller)" >> select MFD_CORE >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >> index ea40e076cb61..0705eb2d873d 100644 >> --- a/drivers/mfd/Makefile >> +++ b/drivers/mfd/Makefile >> @@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC) += qcom-spmi-pmic.o >> obj-$(CONFIG_TPS65911_COMPARATOR) += tps65911-comparator.o >> obj-$(CONFIG_MFD_TPS65090) += tps65090.o >> obj-$(CONFIG_MFD_AAT2870_CORE) += aat2870-core.o >> +obj-$(CONFIG_MFD_ATMEL_FLEXCOM) += atmel-flexcom.o >> obj-$(CONFIG_MFD_ATMEL_HLCDC) += atmel-hlcdc.o >> obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o >> obj-$(CONFIG_MFD_PALMAS) += palmas.o >> diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c >> new file mode 100644 >> index 000000000000..0d06b70696b0 >> --- /dev/null >> +++ b/drivers/mfd/atmel-flexcom.c >> @@ -0,0 +1,113 @@ >> +/* >> + * Driver for Atmel Flexcom >> + * >> + * Copyright (C) 2015 Atmel Corporation >> + * >> + * Author: Cyrille Pitchen >> + * >> + * 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. >> + * >> + * This program is distributed in the hope that it will be useful, but WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> + * more details. >> + * >> + * You should have received a copy of the GNU General Public License along with >> + * this program. If not, see . >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/* I/O register offsets */ >> +#define FLEX_MR 0x0 /* Mode Register */ >> +#define FLEX_VERSION 0xfc /* Version Register */ >> + >> +/* Mode Register bit fields */ >> +#define FLEX_MR_OPMODE_MASK 0x3 >> +#define FLEX_MR_OPMODE_NO_COM 0x0 >> +#define FLEX_MR_OPMODE_USART 0x1 >> +#define FLEX_MR_OPMODE_SPI 0x2 >> +#define FLEX_MR_OPMODE_TWI 0x3 >> + >> + >> +static int atmel_flexcom_probe(struct platform_device *pdev) >> +{ >> + struct device_node *child, *np = pdev->dev.of_node; >> + struct clk *clk; >> + struct resource *res; >> + void __iomem *base; >> + u32 opmode; >> + int err; >> + >> + child = of_get_next_available_child(np, NULL); >> + if (!child) >> + return -ENODEV; >> + >> + /* >> + * The Operating Mode is stored into the first u32 of the reg property >> + * of the child. >> + */ >> + err = of_property_read_u32_index(child, "reg", 0, &opmode); >> + of_node_put(child); > > Don't think you need to of_node_put() after a u32 read. Well, it's after an of_get_next_available_child() which is similar to of_get_next_child(). It is said to release the node by calling of_node_put()... So I suspect it's necessary. Bye, > >> + if (err) >> + return -EINVAL; > > Why are you making up your own return value. > > Just return err. > >> + if ((opmode == FLEX_MR_OPMODE_NO_COM) || >> + (opmode & ~FLEX_MR_OPMODE_MASK)) >> + return -EINVAL; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + base = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(base)) >> + return PTR_ERR(base); >> + >> + clk = devm_clk_get(&pdev->dev, NULL); >> + if (IS_ERR(clk)) >> + return PTR_ERR(clk); >> + >> + err = clk_prepare_enable(clk); >> + if (err) >> + return err; >> + >> + /* >> + * Set the Operating Mode in the Mode Register: only the selected device >> + * is clocked. Hence, registers of the other serial devices remain >> + * inaccessible and are read as zero. Also the external I/O lines of the >> + * Flexcom are muxed to reach the selected device. >> + */ >> + writel(opmode, base + FLEX_MR); >> + >> + clk_disable_unprepare(clk); >> + >> + return of_platform_populate(np, NULL, NULL, &pdev->dev); >> +} >> + >> +static const struct of_device_id atmel_flexcom_of_match[] = { >> + { .compatible = "atmel,sama5d2-flexcom" }, >> + { /* sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match); >> + >> +static struct platform_driver atmel_flexcom_driver = { >> + .probe = atmel_flexcom_probe, >> + .driver = { >> + .name = "atmel_flexcom", >> + .of_match_table = atmel_flexcom_of_match, >> + }, >> +}; >> + >> +module_platform_driver(atmel_flexcom_driver); >> + >> +MODULE_AUTHOR("Cyrille Pitchen "); >> +MODULE_DESCRIPTION("Atmel Flexcom MFD driver"); >> +MODULE_LICENSE("GPL v2"); > -- Nicolas Ferre -- 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/