From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753920AbbFSG22 (ORCPT ); Fri, 19 Jun 2015 02:28:28 -0400 Received: from demumfd002.nsn-inter.net ([93.183.12.31]:51871 "EHLO demumfd002.nsn-inter.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751418AbbFSG2T (ORCPT ); Fri, 19 Jun 2015 02:28:19 -0400 Subject: Re: [Patch v3] driver/i2c/mux: Add register-based mux i2c-mux-reg To: ext York Sun , wsa@the-dreams.de References: <1434657458-16553-1-git-send-email-yorksun@freescale.com> Cc: linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org, Paul Bolle , Peter Korsgaard From: Alexander Sverdlin Message-ID: <5583B678.6050402@nokia.com> Date: Fri, 19 Jun 2015 08:28:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <1434657458-16553-1-git-send-email-yorksun@freescale.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-purgate-type: clean X-purgate-Ad: Categorized by eleven eXpurgate (R) http://www.eleven.de X-purgate: clean X-purgate: This mail is considered clean (visit http://www.eleven.de for further information) X-purgate-size: 17292 X-purgate-ID: 151667::1434695291-000058B4-6F99ABE1/0/0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi! On 18/06/15 21:57, ext York Sun wrote: > Based on i2c-mux-gpio driver, similarly the register-based mux > switch from one bus to another by setting a single register. > The register can be on PCIe bus, local bus, or any memory-mapped > address. The endianness of such register can be specified in device > tree if used, or in platform data. > > Signed-off-by: York Sun > CC: Wolfram Sang > CC: Paul Bolle > CC: Peter Korsgaard > CC: Alexander Sverdlin I can think about external FPGA applications performing MUX function, so the code looks useful to me. Acked-by: Alexander Sverdlin > --- > According to Alexander's feedback, readback is added. Different sizes > are supported. I stick with iowrite but adding an option to use iowrite > big endian in in case needed. Both big- and little-endian are tested. > Comments are updated. > > Change log: > v3: Add support of both big- and little-endian register > Add readback after writing to register > Add no-read option. By default, readback is alowed. > Fix using chan_id transferred back from i2c-mux. It was mistakenly > used as an index. It is actually the data to be written. > > v2: Update to GPLv2+ licence header > Use iowrite instead of direct dereference the pointer to write register > Add support of difference register size of 1/2/4 bytes > Remove i2c_put_adapter(parent) in probe fucntion > Replace multiple dev_info() with dev_dbg() > Add idle_in_use variable to gate using idle value > Add __iomem for register pointer > Move platform data header file to include/linux/platform_data/ > > .../devicetree/bindings/i2c/i2c-mux-reg.txt | 75 +++++ > drivers/i2c/muxes/Kconfig | 11 + > drivers/i2c/muxes/Makefile | 1 + > drivers/i2c/muxes/i2c-mux-reg.c | 299 ++++++++++++++++++++ > include/linux/platform_data/i2c-mux-reg.h | 44 +++ > 5 files changed, 430 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-reg.txt > create mode 100644 drivers/i2c/muxes/i2c-mux-reg.c > create mode 100644 include/linux/platform_data/i2c-mux-reg.h > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-reg.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-reg.txt > new file mode 100644 > index 0000000..6e3197f > --- /dev/null > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-reg.txt > @@ -0,0 +1,75 @@ > +Register-based I2C Bus Mux > + > +This binding describes an I2C bus multiplexer that uses a single register > +to route the I2C signals. > + > +Required properties: > +- compatible: i2c-mux-reg > +- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side > + port is connected to. > +* Standard I2C mux properties. See mux.txt in this directory. > +* I2C child bus nodes. See mux.txt in this directory. > + > +Optional properties: > +- reg: this pair of specifies the register to control the mux. > + The depends on its parent node. It can be any memory-mapped > + address. The size must be either 1, 2, or 4 bytes. If reg is omitted, the > + resource of this device will be used. > +- little-endian: The existence indicates the register is in little endian. > + If omitted, the endianness of the host will be used. > +- no-read: The existence indicates reading the register is not allowed. > +- idle-state: value to set the muxer to when idle. When no value is > + given, it defaults to the last value used. > + > +For each i2c child node, an I2C child bus will be created. They will > +be numbered based on their order in the device tree. > + > +Whenever an access is made to a device on a child bus, the value set > +in the revelant node's reg property will be output to the register. > + > +If an idle state is defined, using the idle-state (optional) property, > +whenever an access is not being made to a device on a child bus, the > +register will be set according to the idle value. > + > +If an idle state is not defined, the most recently used value will be > +left programmed into the register. > + > +Example of a mux on PCIe card, the host is a powerpc SoC (big endian): > + > + i2c-mux { > + /* the depends on the address translation > + * of the parent device. If omitted, device resource > + * will be used instead. The size is to determine > + * whether iowrite32, iowrite16, or iowrite8 will be used. > + */ > + reg = <0x6028 0x4>; > + little-endian; /* little endian register on PCIe */ > + compatible = "i2c-mux-reg"; > + #address-cells = <1>; > + #size-cells = <0>; > + i2c-parent = <&i2c1>; > + i2c@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + si5338: clock-generator@70 { > + compatible = "silabs,si5338"; > + reg = <0x70>; > + /* other stuff */ > + }; > + }; > + > + i2c@1 { > + /* data is written using iowrite32 */ > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + si5338: clock-generator@70 { > + compatible = "silabs,si5338"; > + reg = <0x70>; > + /* other stuff */ > + }; > + }; > + }; > diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig > index f6d313e..77c1257 100644 > --- a/drivers/i2c/muxes/Kconfig > +++ b/drivers/i2c/muxes/Kconfig > @@ -29,6 +29,17 @@ config I2C_MUX_GPIO > This driver can also be built as a module. If so, the module > will be called i2c-mux-gpio. > > +config I2C_MUX_REG > + tristate "Register-based I2C multiplexer" > + help > + If you say yes to this option, support will be included for a > + register based I2C multiplexer. This driver provides access to > + I2C busses connected through a MUX, which is controlled > + by a sinple register. > + > + This driver can also be built as a module. If so, the module > + will be called i2c-mux-reg. > + > config I2C_MUX_PCA9541 > tristate "NXP PCA9541 I2C Master Selector" > help > diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile > index 465778b..bc517bb 100644 > --- a/drivers/i2c/muxes/Makefile > +++ b/drivers/i2c/muxes/Makefile > @@ -4,6 +4,7 @@ > obj-$(CONFIG_I2C_ARB_GPIO_CHALLENGE) += i2c-arb-gpio-challenge.o > > obj-$(CONFIG_I2C_MUX_GPIO) += i2c-mux-gpio.o > +obj-$(CONFIG_I2C_MUX_REG) += i2c-mux-reg.o > obj-$(CONFIG_I2C_MUX_PCA9541) += i2c-mux-pca9541.o > obj-$(CONFIG_I2C_MUX_PCA954x) += i2c-mux-pca954x.o > obj-$(CONFIG_I2C_MUX_PINCTRL) += i2c-mux-pinctrl.o > diff --git a/drivers/i2c/muxes/i2c-mux-reg.c b/drivers/i2c/muxes/i2c-mux-reg.c > new file mode 100644 > index 0000000..1692aec > --- /dev/null > +++ b/drivers/i2c/muxes/i2c-mux-reg.c > @@ -0,0 +1,299 @@ > +/* > + * I2C multiplexer using a single register > + * > + * Copyright 2015 Freescale Semiconductor > + * York Sun > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +struct regmux { > + struct i2c_adapter *parent; > + struct i2c_adapter **adap; /* child busses */ > + struct i2c_mux_reg_platform_data data; > +}; > + > +static int i2c_mux_reg_set(const struct regmux *mux, unsigned int chan_id) > +{ > + if (!mux->data.reg) > + return -EINVAL; > + > + switch (mux->data.reg_size) { > + case 4: > + if (mux->data.little_endian) { > + iowrite32(chan_id, mux->data.reg); > + if (!mux->data.no_read) > + ioread32(mux->data.reg); > + } else { > + iowrite32be(chan_id, mux->data.reg); > + if (!mux->data.no_read) > + ioread32(mux->data.reg); > + } > + break; > + case 2: > + if (mux->data.little_endian) { > + iowrite16(chan_id, mux->data.reg); > + if (!mux->data.no_read) > + ioread16(mux->data.reg); > + } else { > + iowrite16be(chan_id, mux->data.reg); > + if (!mux->data.no_read) > + ioread16be(mux->data.reg); > + } > + break; > + case 1: > + iowrite8(chan_id, mux->data.reg); > + if (!mux->data.no_read) > + ioread8(mux->data.reg); > + break; > + default: > + pr_err("Invalid register size\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int i2c_mux_reg_select(struct i2c_adapter *adap, void *data, > + unsigned int chan) > +{ > + struct regmux *mux = data; > + > + return i2c_mux_reg_set(mux, chan); > +} > + > +static int i2c_mux_reg_deselect(struct i2c_adapter *adap, void *data, > + unsigned int chan) > +{ > + struct regmux *mux = data; > + > + if (mux->data.idle_in_use) > + return i2c_mux_reg_set(mux, mux->data.idle); > + > + return 0; > +} > + > +#ifdef CONFIG_OF > +static int i2c_mux_reg_probe_dt(struct regmux *mux, > + struct platform_device *pdev) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct device_node *adapter_np, *child; > + struct i2c_adapter *adapter; > + struct resource res; > + unsigned *values; > + int i = 0; > + > + if (!np) > + return -ENODEV; > + > + adapter_np = of_parse_phandle(np, "i2c-parent", 0); > + if (!adapter_np) { > + dev_err(&pdev->dev, "Cannot parse i2c-parent\n"); > + return -ENODEV; > + } > + adapter = of_find_i2c_adapter_by_node(adapter_np); > + if (!adapter) { > + dev_err(&pdev->dev, "Cannot find parent bus\n"); > + return -EPROBE_DEFER; > + } > + mux->parent = adapter; > + mux->data.parent = i2c_adapter_id(adapter); > + put_device(&adapter->dev); > + > + mux->data.n_values = of_get_child_count(np); > + if (of_find_property(np, "little-endian", NULL)) { > + mux->data.little_endian = true; > + } else { > +#ifdef __LITTLE_ENDIAN__ > + mux->data.little_endian = true; > +#elif defined(__BIG_ENDIAN__) > + mux->data.little_endian = false; > +#else > +#error Endianness not defined? > +#endif > + } > + if (of_find_property(np, "no-read", NULL)) > + mux->data.no_read = true; > + else > + mux->data.no_read = false; > + > + values = devm_kzalloc(&pdev->dev, > + sizeof(*mux->data.values) * mux->data.n_values, > + GFP_KERNEL); > + if (!values) { > + dev_err(&pdev->dev, "Cannot allocate values array"); > + return -ENOMEM; > + } > + > + for_each_child_of_node(np, child) { > + of_property_read_u32(child, "reg", values + i); > + i++; > + } > + mux->data.values = values; > + > + if (!of_property_read_u32(np, "idle-state", &mux->data.idle)) > + mux->data.idle_in_use = true; > + > + /* map address from "reg" if exists */ > + if (of_address_to_resource(np, 0, &res)) { > + mux->data.reg_size = resource_size(&res); > + if (mux->data.reg_size > 4) { > + dev_err(&pdev->dev, "Invalid address size\n"); > + return -EINVAL; > + } > + mux->data.reg = devm_ioremap_resource(&pdev->dev, &res); > + if (IS_ERR(mux->data.reg)) > + return PTR_ERR(mux->data.reg); > + } > + > + return 0; > +} > +#else > +static int i2c_mux_reg_probe_dt(struct gpiomux *mux, > + struct platform_device *pdev) > +{ > + return 0; > +} > +#endif > + > +static int i2c_mux_reg_probe(struct platform_device *pdev) > +{ > + struct regmux *mux; > + struct i2c_adapter *parent; > + struct resource *res; > + int (*deselect)(struct i2c_adapter *, void *, u32); > + unsigned int initial_state, class; > + int i, ret, nr; > + > + mux = devm_kzalloc(&pdev->dev, sizeof(*mux), GFP_KERNEL); > + if (!mux) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, mux); > + > + if (dev_get_platdata(&pdev->dev)) { > + memcpy(&mux->data, dev_get_platdata(&pdev->dev), > + sizeof(mux->data)); > + > + parent = i2c_get_adapter(mux->data.parent); > + if (!parent) { > + dev_err(&pdev->dev, "Parent adapter (%d) not found\n", > + mux->data.parent); > + return -EPROBE_DEFER; > + } > + mux->parent = parent; > + } else { > + ret = i2c_mux_reg_probe_dt(mux, pdev); > + if (ret < 0) { > + dev_err(&pdev->dev, "Error parsing device tree"); > + return ret; > + } > + } > + > + if (!mux->data.reg) { > + dev_info(&pdev->dev, > + "Register not set, using platform resource\n"); > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + mux->data.reg_size = resource_size(res); > + if (mux->data.reg_size > 4) { > + dev_err(&pdev->dev, "Invalid resource size\n"); > + return -EINVAL; > + } > + mux->data.reg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(mux->data.reg)) > + return PTR_ERR(mux->data.reg); > + } > + > + mux->adap = devm_kzalloc(&pdev->dev, > + sizeof(*mux->adap) * mux->data.n_values, > + GFP_KERNEL); > + if (!mux->adap) { > + dev_err(&pdev->dev, "Cannot allocate i2c_adapter structure"); > + return -ENOMEM; > + } > + > + if (mux->data.idle_in_use) { > + initial_state = mux->data.idle; > + deselect = i2c_mux_reg_deselect; > + } else { > + initial_state = mux->data.values[0]; > + deselect = NULL; > + } > + > + for (i = 0; i < mux->data.n_values; i++) { > + nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0; > + class = mux->data.classes ? mux->data.classes[i] : 0; > + > + mux->adap[i] = i2c_add_mux_adapter(mux->parent, &pdev->dev, mux, > + nr, mux->data.values[i], > + class, i2c_mux_reg_select, > + deselect); > + if (!mux->adap[i]) { > + ret = -ENODEV; > + dev_err(&pdev->dev, "Failed to add adapter %d\n", i); > + goto add_adapter_failed; > + } > + } > + > + dev_dbg(&pdev->dev, "%d port mux on %s adapter\n", > + mux->data.n_values, mux->parent->name); > + > + return 0; > + > +add_adapter_failed: > + for (; i > 0; i--) > + i2c_del_mux_adapter(mux->adap[i - 1]); > + > + return ret; > +} > + > +static int i2c_mux_reg_remove(struct platform_device *pdev) > +{ > + struct regmux *mux = platform_get_drvdata(pdev); > + int i; > + > + for (i = 0; i < mux->data.n_values; i++) > + i2c_del_mux_adapter(mux->adap[i]); > + > + i2c_put_adapter(mux->parent); > + > + dev_dbg(&pdev->dev, "Removed\n"); > + > + return 0; > +} > + > +static const struct of_device_id i2c_mux_reg_of_match[] = { > + { .compatible = "i2c-mux-reg", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, i2c_mux_reg_of_match); > + > +static struct platform_driver i2c_mux_reg_driver = { > + .probe = i2c_mux_reg_probe, > + .remove = i2c_mux_reg_remove, > + .driver = { > + .owner = THIS_MODULE, > + .name = "i2c-mux-reg", > + }, > +}; > + > +module_platform_driver(i2c_mux_reg_driver); > + > +MODULE_DESCRIPTION("Register-based I2C multiplexer driver"); > +MODULE_AUTHOR("York Sun "); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:i2c-mux-reg"); > diff --git a/include/linux/platform_data/i2c-mux-reg.h b/include/linux/platform_data/i2c-mux-reg.h > new file mode 100644 > index 0000000..cf0cc1e > --- /dev/null > +++ b/include/linux/platform_data/i2c-mux-reg.h > @@ -0,0 +1,44 @@ > +/* > + * I2C multiplexer using a single register > + * > + * Copyright 2015 Freescale Semiconductor > + * York Sun > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + */ > + > +#ifndef __LINUX_PLATFORM_DATA_I2C_MUX_REG_H > +#define __LINUX_PLATFORM_DATA_I2C_MUX_REG_H > + > +/** > + * struct i2c_mux_reg_platform_data - Platform-dependent data for i2c-mux-reg > + * @parent: Parent I2C bus adapter number > + * @base_nr: Base I2C bus number to number adapters from or zero for dynamic > + * @values: Array of value for each channel > + * @n_values: Number of multiplexer channels > + * @little_endian: Indicating if the register is in little endian > + * @no_read: Reading the register is not allowed by hardware > + * @classes: Optional I2C auto-detection classes > + * @idle: Value to write to mux when idle > + * @idle_in_use: indicate if idle value is in use > + * @reg: Virtual address of the register to switch channel > + * @reg_size: register size in bytes > + */ > +struct i2c_mux_reg_platform_data { > + int parent; > + int base_nr; > + const unsigned int *values; > + int n_values; > + bool little_endian; > + bool no_read; > + const unsigned int *classes; > + u32 idle; > + bool idle_in_use; > + void __iomem *reg; > + resource_size_t reg_size; > +}; > + > +#endif /* __LINUX_PLATFORM_DATA_I2C_MUX_REG_H */ -- Best regards, Alexander Sverdlin.