* RE: [patch v4] i2c: mux: mellanox: add driver
From: Vadim Pasternak @ 2016-09-13 18:37 UTC (permalink / raw)
To: Peter Rosin, wsa@the-dreams.de
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
jiri@resnulli.us, Michael Shych
In-Reply-To: <9ee51a2a-83c4-f96d-09dd-5eeda233b3b5@axentia.se>
> -----Original Message-----
> From: Peter Rosin [mailto:peda@axentia.se]
> Sent: Tuesday, September 13, 2016 5:36 PM
> To: Vadim Pasternak <vadimp@mellanox.com>; wsa@the-dreams.de
> Cc: linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; jiri@resnulli.us;
> Michael Shych <michaelsh@mellanox.com>
> Subject: Re: [patch v4] i2c: mux: mellanox: add driver
>
> It's shaping up, a few last nitpicks...
>
> On 2016-09-13 16:38, vadimp@mellanox.com wrote:
> > From: Vadim Pasternak <vadimp@mellanox.com>
> >
> > This driver allows I2C routing controlled through CPLD select
> > registers on wide range of Mellanox systems (CPLD Lattice device).
>
> s/wide/a wide/
>
> But is it really still "a wide range" now that i2c-mux-reg handles most CPLD mux
> types?
Yes. For all 1U systems we have port CPLD, where CPLD attached to i2c bus. And all QSFP are connected through this i2c CPLD muxes.
So, we'll use both mux drivers on most of our systems.
Thanks you very much for all your reviews.
>
> > MUX selection is provided by digital and analog HW. Analog part is not
> > under SW control.
> > Digital part is under CPLD control (channel selection/de-selection).
> >
> > Connectivity schema.
> > i2c-mlxcpld Digital Analog
> > driver
> > *--------* * -> mux1 (virt bus2) -> mux ->|
> > | I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux ->|
> > | bridge | bus 1 *---------* |
> > | logic |---------------------> * mux reg * |
> > | in CPLD| *---------* |
> > *--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux ->|
> > | driver | |
> > | *---------------* | Devices
> > | * CPLD (i2c bus)* select |
> > | * registers for *--------*
> > | * mux selection * deselect
> > | *---------------*
> > | |
> > <--------> <----------->
> > i2c cntrl Board cntrl reg
> > reg space space (mux select,
> > IO, LED, WD, info)
> >
> > i2c-mux-mlxpcld does not necessarily require i2c-mlxcpld. It can be
> > use
>
> s/use/used/
>
> > along with another bus driver, and still control i2c routing through
> > CPLD mux selection, in case the system is equipped with CPLD capable
> > of mux selection control.
> >
> > The Kconfig currently controlling compilation of this code is:
> > drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
> >
> > Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> > Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> > Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> > ---
> > v3->v4
> > Fixes issues pointed out by Peter:
> > - Remove mlxcpld_mux_type;
> > - Remove member type from mux control structure;
> > - Remove variable muxes;
> > - mlxcpld_mux_reg_write - set address from client;
> > - mlxcpld_mux_reg_write - try smbus_xfer
> > - mlxcpld_mux_select_chan - remove compare with for mux type;
> > - mlxcpld_mux_probe - use I2C_FUNC_SMBUS_WRITE_BYTE_DATA instead
> of
> > I2C_FUNC_SMBUS_BYTE
> > - mlxcpld_mux_probe - remove switch statement
> > - mlxcpld_mux_probe - update comment;
> > - mlxcpld_mux_probe -change logic in testing num_adaps;
> > - remove members first_channel and addr from mlxcpld_mux_plat_data
> > structure;
> > v2->v3
> > Fixes issues pointed out by Peter:
> > - Fix several comments;
> > - In MAINTAINERS file use linux-i2c instead of linux-kernel;
> > - Place include files in alphabetic order;
> > - When channel select fail, set last_chan to zero for cleaning last value;
> > - Return with error from probe if there is no platform data;
> > - Use i2c_mux_reg driver for the lpc_access cases:
> > it fit well for Mellanox system with LPC access - has been tested and it
> > works good.
> > - Limit this driver to i2c_access only one device (mlxcpld_mux_module).
> > v1->v2
> > Fixes issues pointed out by Peter:
> > - changes in commit cover massage;
> > - change leg to channel in comments;
> > - missed return err;
> > - use platform data mux array rather the offset from 1st channel in probe
> > routine;
> > - remove owner field from driver structure;
> > - use module_i2c_driver macros, instead if __init and __exit;
> > - remove deselect on exit flag;
> > - add record to MAINTAINER file;
> > ---
> > MAINTAINERS | 8 ++
> > drivers/i2c/muxes/Kconfig | 11 ++
> > drivers/i2c/muxes/Makefile | 1 +
> > drivers/i2c/muxes/i2c-mux-mlxcpld.c | 224
> ++++++++++++++++++++++++++++++++++++
> > include/linux/i2c/mlxcpld.h | 52 +++++++++
> > 5 files changed, 296 insertions(+)
> > create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
> > create mode 100644 include/linux/i2c/mlxcpld.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index 0bbe4b1..be83d69 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7655,6 +7655,14 @@ W: http://www.mellanox.com
> > Q: http://patchwork.ozlabs.org/project/netdev/list/
> > F: drivers/net/ethernet/mellanox/mlxsw/
> >
> > +MELLANOX MLXCPLD I2C MUX DRIVER
> > +M: Vadim Pasternak <vadimp@mellanox.com>
> > +M: Michael Shych <michaelsh@mellanox.com>
> > +L: linux-i2c@vger.kernel.org
> > +S: Supported
> > +W: http://www.mellanox.com
> > +F: drivers/i2c/muxes/i2c-mux-mlxcpld.c
> > +
> > SOFT-ROCE DRIVER (rxe)
> > M: Moni Shoua <monis@mellanox.com>
> > L: linux-rdma@vger.kernel.org
> > diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> > index e280c8e..b7ab144 100644
> > --- a/drivers/i2c/muxes/Kconfig
> > +++ b/drivers/i2c/muxes/Kconfig
> > @@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
> > demultiplexer that uses the pinctrl subsystem. This is useful if you
> > want to change the I2C master at run-time depending on features.
> >
> > +config I2C_MUX_MLXCPLD
> > + tristate "Mellanox CPLD based I2C multiplexer"
> > + help
> > + If you say yes to this option, support will be included for a
> > + CPLD based I2C multiplexer. This driver provides access to
> > + I2C busses connected through a MUX, which is controlled
> > + by a CPLD registers.
>
> s/registers/register/
>
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called i2c-mux-mlxcpld.
> > +
> > endmenu
> > diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
> > index 7c267c2..e5c990e 100644
> > --- a/drivers/i2c/muxes/Makefile
> > +++ b/drivers/i2c/muxes/Makefile
> > @@ -10,5 +10,6 @@ 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
> > obj-$(CONFIG_I2C_MUX_REG) += i2c-mux-reg.o
> > +obj-$(CONFIG_I2C_MUX_MLXCPLD) += i2c-mux-mlxcpld.o
>
> Please keep these lines sorted too.
>
> >
> > ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git
> > a/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> > b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> > new file mode 100644
> > index 0000000..08d3b1a
> > --- /dev/null
> > +++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> > @@ -0,0 +1,224 @@
> > +/*
> > + * drivers/i2c/muxes/i2c-mux-mlxcpld.c
> > + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> > + * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are
> met:
> > + *
> > + * 1. Redistributions of source code must retain the above copyright
> > + * notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + * notice, this list of conditions and the following disclaimer in the
> > + * documentation and/or other materials provided with the distribution.
> > + * 3. Neither the names of the copyright holders nor the names of its
> > + * contributors may be used to endorse or promote products derived from
> > + * this software without specific prior written permission.
> > + *
> > + * Alternatively, this software may be distributed under the terms of
> > +the
> > + * GNU General Public License ("GPL") version 2 as published by the
> > +Free
> > + * Software Foundation.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS "AS IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> LIMITED
> > +TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> PARTICULAR
> > +PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> > +CONTRIBUTORS BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
> > +OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> PROCUREMENT
> > +OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> > +BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> > +WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> > +OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> > +ADVISED OF THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/i2c.h>
> > +#include <linux/i2c-mux.h>
> > +#include <linux/io.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/version.h>
> > +#include <linux/i2c/mlxcpld.h>
> > +
> > +#define CPLD_MUX_MAX_NCHANS 8
> > +
> > +/* mlxcpld_mux - mux control structure:
> > + * @last_chan - last register value
> > + * @client - I2C device client
> > + */
> > +struct mlxcpld_mux {
> > + u8 last_chan;
> > + struct i2c_client *client;
> > +};
> > +
> > +/* MUX logic description.
> > + * Driver can support different mux control logic, according to CPLD
> > + * implementation.
> > + *
> > + * Connectivity schema.
> > + *
> > + * i2c-mlxcpld Digital Analog
> > + * driver
> > + * *--------* * -> mux1 (virt bus2) -> mux -> |
> > + * | I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux -> |
> > + * | bridge | bus 1 *---------* |
> > + * | logic |---------------------> * mux reg * |
> > + * | in CPLD| *---------* |
> > + * *--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux -> |
> > + * | driver | |
> > + * | *---------------* | Devices
> > + * | * CPLD (i2c bus)* select |
> > + * | * registers for *--------*
> > + * | * mux selection * deselect
> > + * | *---------------*
> > + * | |
> > + * <--------> <----------->
> > + * i2c cntrl Board cntrl reg
> > + * reg space space (mux select,
> > + * IO, LED, WD, info)
> > + *
> > + */
> > +
> > +static const struct i2c_device_id mlxcpld_mux_id[] = {
> > + { "mlxcpld_mux_module", 0 },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, mlxcpld_mux_id);
> > +
> > +/* Write to mux register. Don't use i2c_transfer() and
> > + * i2c_smbus_xfer() for this as they will try to lock adapter a
> > +second time */
>
> This comment has a strange shortish first line that disturbs me, and you are
> missing the ending period.
>
> /* Write to mux register. Don't use i2c_transfer() and i2c_smbus_xfer()
> * for this as they will try to lock adapter a second time.
> */
>
> > +static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
> > + struct i2c_client *client, u8 val) {
> > + struct mlxcpld_mux_plat_data *pdata =
> > +dev_get_platdata(&client->dev);
> > +
> > + if (adap->algo->master_xfer) {
> > + struct i2c_msg msg;
> > + u8 msgbuf[] = {pdata->sel_reg_addr, val};
> > +
> > + msg.addr = client->addr;
> > + msg.flags = 0;
> > + msg.len = 2;
> > + msg.buf = msgbuf;
> > + return __i2c_transfer(adap, &msg, 1);
> > + } else if (adap->algo->smbus_xfer) {
> > + union i2c_smbus_data data;
> > +
> > + data.byte = val;
> > + return adap->algo->smbus_xfer(adap, client->addr,
> > + client->flags, I2C_SMBUS_WRITE,
> > + pdata->sel_reg_addr,
> > + I2C_SMBUS_BYTE_DATA, &data);
> > + } else
> > + return -ENODEV;
> > +}
> > +
> > +static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32
> > +chan) {
> > + struct mlxcpld_mux *data = i2c_mux_priv(muxc);
> > + struct i2c_client *client = data->client;
> > + u8 regval = chan + 1;
> > + int err = 0;
> > +
> > + /* Only select the channel if its different from the last channel */
> > + if (data->last_chan != regval) {
> > + err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
> > + if (err)
> > + data->last_chan = 0;
> > + else
> > + data->last_chan = regval;
> > + }
> > +
> > + return err;
> > +}
> > +
> > +static int mlxcpld_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
> > +{
> > + struct mlxcpld_mux *data = i2c_mux_priv(muxc);
> > + struct i2c_client *client = data->client;
> > +
> > + /* Deselect active channel */
> > + data->last_chan = 0;
> > +
> > + return mlxcpld_mux_reg_write(muxc->parent, client, data->last_chan);
> > +}
> > +
> > +/* I2C init/probing/exit functions */ static int
> > +mlxcpld_mux_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id) {
> > + struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
> > + struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client-
> >dev);
> > + struct i2c_mux_core *muxc;
> > + int num, force;
> > + struct mlxcpld_mux *data;
> > + int err;
> > +
> > + if (!pdata)
> > + return -EINVAL;
> > +
> > + if (!i2c_check_functionality(adap,
> I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
> > + return -ENODEV;
> > +
> > + muxc = i2c_mux_alloc(adap, &client->dev, CPLD_MUX_MAX_NCHANS,
> > + sizeof(*data), 0, mlxcpld_mux_select_chan,
> > + mlxcpld_mux_deselect);
> > + if (!muxc)
> > + return -ENOMEM;
> > +
> > + data = i2c_mux_priv(muxc);
> > + i2c_set_clientdata(client, muxc);
> > + data->client = client;
> > + data->last_chan = 0; /* force the first selection */
> > +
> > + /* Create an adapter for each channel. */
> > + for (num = 0; num < CPLD_MUX_MAX_NCHANS; num++) {
> > + force = 0; /* dynamic adap number */
>
> This assignment can be dropped.
>
> > + if (num >= pdata->num_adaps)
> > + /* discard unconfigured channels */
> > + break;
> > +
> > + force = pdata->adap_ids[num];
> > +
> > + err = i2c_mux_add_adapter(muxc, force, num, 0);
> > + if (err) {
> > + dev_err(&client->dev, "failed to register multiplexed
> adapter %d as bus %d\n",
> > + num, force);
>
> There is generic error messages for all failures inside i2c_mux_add_adapter, so
> this message can be dropped. If the generic messages are not sufficient, please
> expand them instead of adding extra messages.
>
> > + goto virt_reg_failed;
> > + }
> > + }
> > +
> > + return 0;
> > +
> > +virt_reg_failed:
> > + i2c_mux_del_adapters(muxc);
> > + return err;
> > +}
> > +
> > +static int mlxcpld_mux_remove(struct i2c_client *client) {
> > + struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> > +
> > + i2c_mux_del_adapters(muxc);
> > + return 0;
> > +}
> > +
> > +static struct i2c_driver mlxcpld_mux_driver = {
> > + .driver = {
> > + .name = "mlxcpld-mux",
> > + },
> > + .probe = mlxcpld_mux_probe,
> > + .remove = mlxcpld_mux_remove,
> > + .id_table = mlxcpld_mux_id,
> > +};
> > +
> > +module_i2c_driver(mlxcpld_mux_driver);
> > +
> > +MODULE_AUTHOR("Michael Shych (michaels@mellanox.com)");
> > +MODULE_DESCRIPTION("Mellanox I2C-CPLD-MUX driver");
> > +MODULE_LICENSE("GPL v2"); MODULE_ALIAS("platform:i2c-mux-mlxcpld");
> > diff --git a/include/linux/i2c/mlxcpld.h b/include/linux/i2c/mlxcpld.h
> > new file mode 100644 index 0000000..b08dcb1
> > --- /dev/null
> > +++ b/include/linux/i2c/mlxcpld.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * mlxcpld.h - Mellanox I2C multiplexer support in CPLD
> > + *
> > + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> > + * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are
> met:
> > + *
> > + * 1. Redistributions of source code must retain the above copyright
> > + * notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + * notice, this list of conditions and the following disclaimer in the
> > + * documentation and/or other materials provided with the distribution.
> > + * 3. Neither the names of the copyright holders nor the names of its
> > + * contributors may be used to endorse or promote products derived from
> > + * this software without specific prior written permission.
> > + *
> > + * Alternatively, this software may be distributed under the terms of
> > +the
> > + * GNU General Public License ("GPL") version 2 as published by the
> > +Free
> > + * Software Foundation.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS "AS IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> LIMITED
> > +TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> PARTICULAR
> > +PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> > +CONTRIBUTORS BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
> > +OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> PROCUREMENT
> > +OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> > +BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> > +WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> > +OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> > +ADVISED OF THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + */
> > +
> > +#ifndef _LINUX_I2C_MLXCPLD_H
> > +#define _LINUX_I2C_MLXCPLD_H
> > +
> > +/* Platform data for the CPLD I2C multiplexers */
> > +
> > +/* mlxcpld_mux_plat_data - per mux data, used with
> > +i2c_register_board_info
> > + * @adap_ids - adapter array
> > + * @num_adaps - number of adapters
> > + * @sel_reg_addr - mux select register offset in CPLD space */
> > +struct mlxcpld_mux_plat_data {
> > + int *adap_ids;
> > + int num_adaps;
> > + int sel_reg_addr;
> > +};
> > +
> > +#endif /* _LINUX_I2C_MLXCPLD_H */
> >
^ permalink raw reply
* Re: [PATCH v1 0/2] i2c: Stop i2c modules being unloaded while in use.
From: Baxter, Jim @ 2016-09-13 16:55 UTC (permalink / raw)
To: Peter Rosin, linux-i2c
Cc: Peter Korsgaard, Wolfram Sang, linux-kernel, Frkuska, Joshua,
jiwang
In-Reply-To: <51c45bae-aa5d-bfad-e9d0-d95deb3dc380@axentia.se>
Hi Peter,
> nitpick: Patch subjects for the second patch is wrong.
>
> "reparented" is a bit dual when dealing with i2c adapter trees.
> i2c_mux_add_owned_adapter is perhaps clearer?
Agreed, I will update that.
>
>
> Aside from that, I'm not using modules much and need some enlightenment
> as to why the i2c_del_mux_adapter() call in i2c_mux_gpio_remove() is not
> sufficient and what exactly the problem is? Why would someone/something
> unload the i2c-mux module prematurely?
It is not a normal operation to remove the i2c gpio mux, however systemd
could unload modules out of order if users are restarting services
incorrectly and cause unintended side-effects. This change would stop an
i2c-mux that maybe controlling a voltage regulator from being unloaded
and disabling power to parts of the system unexpectedly.
>
>
> Would it be an alternative to make i2c-mux a proper kernel object of
> some kind? I mean, why do not all other mux users also need to modify
> the owner? Why is i2c-mux-gpio special?
>
i2c-mux-gpio is not special, the code inserted by
[PATCH v1 1/2] i2c-mux: add i2c_mux_add_reparented_adapter api could be
used by other muxes if required.
Best regards
Jim
^ permalink raw reply
* Re: [patch v4] i2c: mux: mellanox: add driver
From: Peter Rosin @ 2016-09-13 14:36 UTC (permalink / raw)
To: vadimp, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych
In-Reply-To: <1473777525-187890-1-git-send-email-vadimp@mellanox.com>
It's shaping up, a few last nitpicks...
On 2016-09-13 16:38, vadimp@mellanox.com wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
>
> This driver allows I2C routing controlled through CPLD select registers on
> wide range of Mellanox systems (CPLD Lattice device).
s/wide/a wide/
But is it really still "a wide range" now that i2c-mux-reg handles most
CPLD mux types?
> MUX selection is provided by digital and analog HW. Analog part is not
> under SW control.
> Digital part is under CPLD control (channel selection/de-selection).
>
> Connectivity schema.
> i2c-mlxcpld Digital Analog
> driver
> *--------* * -> mux1 (virt bus2) -> mux ->|
> | I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux ->|
> | bridge | bus 1 *---------* |
> | logic |---------------------> * mux reg * |
> | in CPLD| *---------* |
> *--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux ->|
> | driver | |
> | *---------------* | Devices
> | * CPLD (i2c bus)* select |
> | * registers for *--------*
> | * mux selection * deselect
> | *---------------*
> | |
> <--------> <----------->
> i2c cntrl Board cntrl reg
> reg space space (mux select,
> IO, LED, WD, info)
>
> i2c-mux-mlxpcld does not necessarily require i2c-mlxcpld. It can be use
s/use/used/
> along with another bus driver, and still control i2c routing through CPLD
> mux selection, in case the system is equipped with CPLD capable of mux
> selection control.
>
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
>
> Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v3->v4
> Fixes issues pointed out by Peter:
> - Remove mlxcpld_mux_type;
> - Remove member type from mux control structure;
> - Remove variable muxes;
> - mlxcpld_mux_reg_write - set address from client;
> - mlxcpld_mux_reg_write - try smbus_xfer
> - mlxcpld_mux_select_chan - remove compare with for mux type;
> - mlxcpld_mux_probe - use I2C_FUNC_SMBUS_WRITE_BYTE_DATA instead of
> I2C_FUNC_SMBUS_BYTE
> - mlxcpld_mux_probe - remove switch statement
> - mlxcpld_mux_probe - update comment;
> - mlxcpld_mux_probe -change logic in testing num_adaps;
> - remove members first_channel and addr from mlxcpld_mux_plat_data
> structure;
> v2->v3
> Fixes issues pointed out by Peter:
> - Fix several comments;
> - In MAINTAINERS file use linux-i2c instead of linux-kernel;
> - Place include files in alphabetic order;
> - When channel select fail, set last_chan to zero for cleaning last value;
> - Return with error from probe if there is no platform data;
> - Use i2c_mux_reg driver for the lpc_access cases:
> it fit well for Mellanox system with LPC access - has been tested and it
> works good.
> - Limit this driver to i2c_access only one device (mlxcpld_mux_module).
> v1->v2
> Fixes issues pointed out by Peter:
> - changes in commit cover massage;
> - change leg to channel in comments;
> - missed return err;
> - use platform data mux array rather the offset from 1st channel in probe
> routine;
> - remove owner field from driver structure;
> - use module_i2c_driver macros, instead if __init and __exit;
> - remove deselect on exit flag;
> - add record to MAINTAINER file;
> ---
> MAINTAINERS | 8 ++
> drivers/i2c/muxes/Kconfig | 11 ++
> drivers/i2c/muxes/Makefile | 1 +
> drivers/i2c/muxes/i2c-mux-mlxcpld.c | 224 ++++++++++++++++++++++++++++++++++++
> include/linux/i2c/mlxcpld.h | 52 +++++++++
> 5 files changed, 296 insertions(+)
> create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
> create mode 100644 include/linux/i2c/mlxcpld.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0bbe4b1..be83d69 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7655,6 +7655,14 @@ W: http://www.mellanox.com
> Q: http://patchwork.ozlabs.org/project/netdev/list/
> F: drivers/net/ethernet/mellanox/mlxsw/
>
> +MELLANOX MLXCPLD I2C MUX DRIVER
> +M: Vadim Pasternak <vadimp@mellanox.com>
> +M: Michael Shych <michaelsh@mellanox.com>
> +L: linux-i2c@vger.kernel.org
> +S: Supported
> +W: http://www.mellanox.com
> +F: drivers/i2c/muxes/i2c-mux-mlxcpld.c
> +
> SOFT-ROCE DRIVER (rxe)
> M: Moni Shoua <monis@mellanox.com>
> L: linux-rdma@vger.kernel.org
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index e280c8e..b7ab144 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
> demultiplexer that uses the pinctrl subsystem. This is useful if you
> want to change the I2C master at run-time depending on features.
>
> +config I2C_MUX_MLXCPLD
> + tristate "Mellanox CPLD based I2C multiplexer"
> + help
> + If you say yes to this option, support will be included for a
> + CPLD based I2C multiplexer. This driver provides access to
> + I2C busses connected through a MUX, which is controlled
> + by a CPLD registers.
s/registers/register/
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-mux-mlxcpld.
> +
> endmenu
> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
> index 7c267c2..e5c990e 100644
> --- a/drivers/i2c/muxes/Makefile
> +++ b/drivers/i2c/muxes/Makefile
> @@ -10,5 +10,6 @@ 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
> obj-$(CONFIG_I2C_MUX_REG) += i2c-mux-reg.o
> +obj-$(CONFIG_I2C_MUX_MLXCPLD) += i2c-mux-mlxcpld.o
Please keep these lines sorted too.
>
> ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> new file mode 100644
> index 0000000..08d3b1a
> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> @@ -0,0 +1,224 @@
> +/*
> + * drivers/i2c/muxes/i2c-mux-mlxcpld.c
> + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the names of the copyright holders nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") version 2 as published by the Free
> + * Software Foundation.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-mux.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/version.h>
> +#include <linux/i2c/mlxcpld.h>
> +
> +#define CPLD_MUX_MAX_NCHANS 8
> +
> +/* mlxcpld_mux - mux control structure:
> + * @last_chan - last register value
> + * @client - I2C device client
> + */
> +struct mlxcpld_mux {
> + u8 last_chan;
> + struct i2c_client *client;
> +};
> +
> +/* MUX logic description.
> + * Driver can support different mux control logic, according to CPLD
> + * implementation.
> + *
> + * Connectivity schema.
> + *
> + * i2c-mlxcpld Digital Analog
> + * driver
> + * *--------* * -> mux1 (virt bus2) -> mux -> |
> + * | I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux -> |
> + * | bridge | bus 1 *---------* |
> + * | logic |---------------------> * mux reg * |
> + * | in CPLD| *---------* |
> + * *--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux -> |
> + * | driver | |
> + * | *---------------* | Devices
> + * | * CPLD (i2c bus)* select |
> + * | * registers for *--------*
> + * | * mux selection * deselect
> + * | *---------------*
> + * | |
> + * <--------> <----------->
> + * i2c cntrl Board cntrl reg
> + * reg space space (mux select,
> + * IO, LED, WD, info)
> + *
> + */
> +
> +static const struct i2c_device_id mlxcpld_mux_id[] = {
> + { "mlxcpld_mux_module", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, mlxcpld_mux_id);
> +
> +/* Write to mux register. Don't use i2c_transfer() and
> + * i2c_smbus_xfer() for this as they will try to lock adapter a second time
> + */
This comment has a strange shortish first line that disturbs me, and
you are missing the ending period.
/* Write to mux register. Don't use i2c_transfer() and i2c_smbus_xfer()
* for this as they will try to lock adapter a second time.
*/
> +static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
> + struct i2c_client *client, u8 val)
> +{
> + struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
> +
> + if (adap->algo->master_xfer) {
> + struct i2c_msg msg;
> + u8 msgbuf[] = {pdata->sel_reg_addr, val};
> +
> + msg.addr = client->addr;
> + msg.flags = 0;
> + msg.len = 2;
> + msg.buf = msgbuf;
> + return __i2c_transfer(adap, &msg, 1);
> + } else if (adap->algo->smbus_xfer) {
> + union i2c_smbus_data data;
> +
> + data.byte = val;
> + return adap->algo->smbus_xfer(adap, client->addr,
> + client->flags, I2C_SMBUS_WRITE,
> + pdata->sel_reg_addr,
> + I2C_SMBUS_BYTE_DATA, &data);
> + } else
> + return -ENODEV;
> +}
> +
> +static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
> +{
> + struct mlxcpld_mux *data = i2c_mux_priv(muxc);
> + struct i2c_client *client = data->client;
> + u8 regval = chan + 1;
> + int err = 0;
> +
> + /* Only select the channel if its different from the last channel */
> + if (data->last_chan != regval) {
> + err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
> + if (err)
> + data->last_chan = 0;
> + else
> + data->last_chan = regval;
> + }
> +
> + return err;
> +}
> +
> +static int mlxcpld_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
> +{
> + struct mlxcpld_mux *data = i2c_mux_priv(muxc);
> + struct i2c_client *client = data->client;
> +
> + /* Deselect active channel */
> + data->last_chan = 0;
> +
> + return mlxcpld_mux_reg_write(muxc->parent, client, data->last_chan);
> +}
> +
> +/* I2C init/probing/exit functions */
> +static int mlxcpld_mux_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
> + struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
> + struct i2c_mux_core *muxc;
> + int num, force;
> + struct mlxcpld_mux *data;
> + int err;
> +
> + if (!pdata)
> + return -EINVAL;
> +
> + if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
> + return -ENODEV;
> +
> + muxc = i2c_mux_alloc(adap, &client->dev, CPLD_MUX_MAX_NCHANS,
> + sizeof(*data), 0, mlxcpld_mux_select_chan,
> + mlxcpld_mux_deselect);
> + if (!muxc)
> + return -ENOMEM;
> +
> + data = i2c_mux_priv(muxc);
> + i2c_set_clientdata(client, muxc);
> + data->client = client;
> + data->last_chan = 0; /* force the first selection */
> +
> + /* Create an adapter for each channel. */
> + for (num = 0; num < CPLD_MUX_MAX_NCHANS; num++) {
> + force = 0; /* dynamic adap number */
This assignment can be dropped.
> + if (num >= pdata->num_adaps)
> + /* discard unconfigured channels */
> + break;
> +
> + force = pdata->adap_ids[num];
> +
> + err = i2c_mux_add_adapter(muxc, force, num, 0);
> + if (err) {
> + dev_err(&client->dev, "failed to register multiplexed adapter %d as bus %d\n",
> + num, force);
There is generic error messages for all failures inside i2c_mux_add_adapter,
so this message can be dropped. If the generic messages are not sufficient,
please expand them instead of adding extra messages.
> + goto virt_reg_failed;
> + }
> + }
> +
> + return 0;
> +
> +virt_reg_failed:
> + i2c_mux_del_adapters(muxc);
> + return err;
> +}
> +
> +static int mlxcpld_mux_remove(struct i2c_client *client)
> +{
> + struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> +
> + i2c_mux_del_adapters(muxc);
> + return 0;
> +}
> +
> +static struct i2c_driver mlxcpld_mux_driver = {
> + .driver = {
> + .name = "mlxcpld-mux",
> + },
> + .probe = mlxcpld_mux_probe,
> + .remove = mlxcpld_mux_remove,
> + .id_table = mlxcpld_mux_id,
> +};
> +
> +module_i2c_driver(mlxcpld_mux_driver);
> +
> +MODULE_AUTHOR("Michael Shych (michaels@mellanox.com)");
> +MODULE_DESCRIPTION("Mellanox I2C-CPLD-MUX driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:i2c-mux-mlxcpld");
> diff --git a/include/linux/i2c/mlxcpld.h b/include/linux/i2c/mlxcpld.h
> new file mode 100644
> index 0000000..b08dcb1
> --- /dev/null
> +++ b/include/linux/i2c/mlxcpld.h
> @@ -0,0 +1,52 @@
> +/*
> + * mlxcpld.h - Mellanox I2C multiplexer support in CPLD
> + *
> + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the names of the copyright holders nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") version 2 as published by the Free
> + * Software Foundation.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _LINUX_I2C_MLXCPLD_H
> +#define _LINUX_I2C_MLXCPLD_H
> +
> +/* Platform data for the CPLD I2C multiplexers */
> +
> +/* mlxcpld_mux_plat_data - per mux data, used with i2c_register_board_info
> + * @adap_ids - adapter array
> + * @num_adaps - number of adapters
> + * @sel_reg_addr - mux select register offset in CPLD space
> + */
> +struct mlxcpld_mux_plat_data {
> + int *adap_ids;
> + int num_adaps;
> + int sel_reg_addr;
> +};
> +
> +#endif /* _LINUX_I2C_MLXCPLD_H */
>
^ permalink raw reply
* [patch v4] i2c: mux: mellanox: add driver
From: vadimp @ 2016-09-13 14:38 UTC (permalink / raw)
To: wsa, peda; +Cc: linux-i2c, linux-kernel, jiri, Vadim Pasternak, Michael Shych
From: Vadim Pasternak <vadimp@mellanox.com>
This driver allows I2C routing controlled through CPLD select registers on
wide range of Mellanox systems (CPLD Lattice device).
MUX selection is provided by digital and analog HW. Analog part is not
under SW control.
Digital part is under CPLD control (channel selection/de-selection).
Connectivity schema.
i2c-mlxcpld Digital Analog
driver
*--------* * -> mux1 (virt bus2) -> mux ->|
| I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux ->|
| bridge | bus 1 *---------* |
| logic |---------------------> * mux reg * |
| in CPLD| *---------* |
*--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux ->|
| driver | |
| *---------------* | Devices
| * CPLD (i2c bus)* select |
| * registers for *--------*
| * mux selection * deselect
| *---------------*
| |
<--------> <----------->
i2c cntrl Board cntrl reg
reg space space (mux select,
IO, LED, WD, info)
i2c-mux-mlxpcld does not necessarily require i2c-mlxcpld. It can be use
along with another bus driver, and still control i2c routing through CPLD
mux selection, in case the system is equipped with CPLD capable of mux
selection control.
The Kconfig currently controlling compilation of this code is:
drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
Signed-off-by: Michael Shych <michaelsh@mellanox.com>
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
v3->v4
Fixes issues pointed out by Peter:
- Remove mlxcpld_mux_type;
- Remove member type from mux control structure;
- Remove variable muxes;
- mlxcpld_mux_reg_write - set address from client;
- mlxcpld_mux_reg_write - try smbus_xfer
- mlxcpld_mux_select_chan - remove compare with for mux type;
- mlxcpld_mux_probe - use I2C_FUNC_SMBUS_WRITE_BYTE_DATA instead of
I2C_FUNC_SMBUS_BYTE
- mlxcpld_mux_probe - remove switch statement
- mlxcpld_mux_probe - update comment;
- mlxcpld_mux_probe -change logic in testing num_adaps;
- remove members first_channel and addr from mlxcpld_mux_plat_data
structure;
v2->v3
Fixes issues pointed out by Peter:
- Fix several comments;
- In MAINTAINERS file use linux-i2c instead of linux-kernel;
- Place include files in alphabetic order;
- When channel select fail, set last_chan to zero for cleaning last value;
- Return with error from probe if there is no platform data;
- Use i2c_mux_reg driver for the lpc_access cases:
it fit well for Mellanox system with LPC access - has been tested and it
works good.
- Limit this driver to i2c_access only one device (mlxcpld_mux_module).
v1->v2
Fixes issues pointed out by Peter:
- changes in commit cover massage;
- change leg to channel in comments;
- missed return err;
- use platform data mux array rather the offset from 1st channel in probe
routine;
- remove owner field from driver structure;
- use module_i2c_driver macros, instead if __init and __exit;
- remove deselect on exit flag;
- add record to MAINTAINER file;
---
MAINTAINERS | 8 ++
drivers/i2c/muxes/Kconfig | 11 ++
drivers/i2c/muxes/Makefile | 1 +
drivers/i2c/muxes/i2c-mux-mlxcpld.c | 224 ++++++++++++++++++++++++++++++++++++
include/linux/i2c/mlxcpld.h | 52 +++++++++
5 files changed, 296 insertions(+)
create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
create mode 100644 include/linux/i2c/mlxcpld.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 0bbe4b1..be83d69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7655,6 +7655,14 @@ W: http://www.mellanox.com
Q: http://patchwork.ozlabs.org/project/netdev/list/
F: drivers/net/ethernet/mellanox/mlxsw/
+MELLANOX MLXCPLD I2C MUX DRIVER
+M: Vadim Pasternak <vadimp@mellanox.com>
+M: Michael Shych <michaelsh@mellanox.com>
+L: linux-i2c@vger.kernel.org
+S: Supported
+W: http://www.mellanox.com
+F: drivers/i2c/muxes/i2c-mux-mlxcpld.c
+
SOFT-ROCE DRIVER (rxe)
M: Moni Shoua <monis@mellanox.com>
L: linux-rdma@vger.kernel.org
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index e280c8e..b7ab144 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
demultiplexer that uses the pinctrl subsystem. This is useful if you
want to change the I2C master at run-time depending on features.
+config I2C_MUX_MLXCPLD
+ tristate "Mellanox CPLD based I2C multiplexer"
+ help
+ If you say yes to this option, support will be included for a
+ CPLD based I2C multiplexer. This driver provides access to
+ I2C busses connected through a MUX, which is controlled
+ by a CPLD registers.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-mux-mlxcpld.
+
endmenu
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 7c267c2..e5c990e 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -10,5 +10,6 @@ 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
obj-$(CONFIG_I2C_MUX_REG) += i2c-mux-reg.o
+obj-$(CONFIG_I2C_MUX_MLXCPLD) += i2c-mux-mlxcpld.o
ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
new file mode 100644
index 0000000..08d3b1a
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
@@ -0,0 +1,224 @@
+/*
+ * drivers/i2c/muxes/i2c-mux-mlxcpld.c
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/version.h>
+#include <linux/i2c/mlxcpld.h>
+
+#define CPLD_MUX_MAX_NCHANS 8
+
+/* mlxcpld_mux - mux control structure:
+ * @last_chan - last register value
+ * @client - I2C device client
+ */
+struct mlxcpld_mux {
+ u8 last_chan;
+ struct i2c_client *client;
+};
+
+/* MUX logic description.
+ * Driver can support different mux control logic, according to CPLD
+ * implementation.
+ *
+ * Connectivity schema.
+ *
+ * i2c-mlxcpld Digital Analog
+ * driver
+ * *--------* * -> mux1 (virt bus2) -> mux -> |
+ * | I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux -> |
+ * | bridge | bus 1 *---------* |
+ * | logic |---------------------> * mux reg * |
+ * | in CPLD| *---------* |
+ * *--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux -> |
+ * | driver | |
+ * | *---------------* | Devices
+ * | * CPLD (i2c bus)* select |
+ * | * registers for *--------*
+ * | * mux selection * deselect
+ * | *---------------*
+ * | |
+ * <--------> <----------->
+ * i2c cntrl Board cntrl reg
+ * reg space space (mux select,
+ * IO, LED, WD, info)
+ *
+ */
+
+static const struct i2c_device_id mlxcpld_mux_id[] = {
+ { "mlxcpld_mux_module", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mlxcpld_mux_id);
+
+/* Write to mux register. Don't use i2c_transfer() and
+ * i2c_smbus_xfer() for this as they will try to lock adapter a second time
+ */
+static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
+ struct i2c_client *client, u8 val)
+{
+ struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
+
+ if (adap->algo->master_xfer) {
+ struct i2c_msg msg;
+ u8 msgbuf[] = {pdata->sel_reg_addr, val};
+
+ msg.addr = client->addr;
+ msg.flags = 0;
+ msg.len = 2;
+ msg.buf = msgbuf;
+ return __i2c_transfer(adap, &msg, 1);
+ } else if (adap->algo->smbus_xfer) {
+ union i2c_smbus_data data;
+
+ data.byte = val;
+ return adap->algo->smbus_xfer(adap, client->addr,
+ client->flags, I2C_SMBUS_WRITE,
+ pdata->sel_reg_addr,
+ I2C_SMBUS_BYTE_DATA, &data);
+ } else
+ return -ENODEV;
+}
+
+static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
+{
+ struct mlxcpld_mux *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
+ u8 regval = chan + 1;
+ int err = 0;
+
+ /* Only select the channel if its different from the last channel */
+ if (data->last_chan != regval) {
+ err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
+ if (err)
+ data->last_chan = 0;
+ else
+ data->last_chan = regval;
+ }
+
+ return err;
+}
+
+static int mlxcpld_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
+{
+ struct mlxcpld_mux *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
+
+ /* Deselect active channel */
+ data->last_chan = 0;
+
+ return mlxcpld_mux_reg_write(muxc->parent, client, data->last_chan);
+}
+
+/* I2C init/probing/exit functions */
+static int mlxcpld_mux_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
+ struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
+ struct i2c_mux_core *muxc;
+ int num, force;
+ struct mlxcpld_mux *data;
+ int err;
+
+ if (!pdata)
+ return -EINVAL;
+
+ if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
+ return -ENODEV;
+
+ muxc = i2c_mux_alloc(adap, &client->dev, CPLD_MUX_MAX_NCHANS,
+ sizeof(*data), 0, mlxcpld_mux_select_chan,
+ mlxcpld_mux_deselect);
+ if (!muxc)
+ return -ENOMEM;
+
+ data = i2c_mux_priv(muxc);
+ i2c_set_clientdata(client, muxc);
+ data->client = client;
+ data->last_chan = 0; /* force the first selection */
+
+ /* Create an adapter for each channel. */
+ for (num = 0; num < CPLD_MUX_MAX_NCHANS; num++) {
+ force = 0; /* dynamic adap number */
+ if (num >= pdata->num_adaps)
+ /* discard unconfigured channels */
+ break;
+
+ force = pdata->adap_ids[num];
+
+ err = i2c_mux_add_adapter(muxc, force, num, 0);
+ if (err) {
+ dev_err(&client->dev, "failed to register multiplexed adapter %d as bus %d\n",
+ num, force);
+ goto virt_reg_failed;
+ }
+ }
+
+ return 0;
+
+virt_reg_failed:
+ i2c_mux_del_adapters(muxc);
+ return err;
+}
+
+static int mlxcpld_mux_remove(struct i2c_client *client)
+{
+ struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+
+ i2c_mux_del_adapters(muxc);
+ return 0;
+}
+
+static struct i2c_driver mlxcpld_mux_driver = {
+ .driver = {
+ .name = "mlxcpld-mux",
+ },
+ .probe = mlxcpld_mux_probe,
+ .remove = mlxcpld_mux_remove,
+ .id_table = mlxcpld_mux_id,
+};
+
+module_i2c_driver(mlxcpld_mux_driver);
+
+MODULE_AUTHOR("Michael Shych (michaels@mellanox.com)");
+MODULE_DESCRIPTION("Mellanox I2C-CPLD-MUX driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:i2c-mux-mlxcpld");
diff --git a/include/linux/i2c/mlxcpld.h b/include/linux/i2c/mlxcpld.h
new file mode 100644
index 0000000..b08dcb1
--- /dev/null
+++ b/include/linux/i2c/mlxcpld.h
@@ -0,0 +1,52 @@
+/*
+ * mlxcpld.h - Mellanox I2C multiplexer support in CPLD
+ *
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _LINUX_I2C_MLXCPLD_H
+#define _LINUX_I2C_MLXCPLD_H
+
+/* Platform data for the CPLD I2C multiplexers */
+
+/* mlxcpld_mux_plat_data - per mux data, used with i2c_register_board_info
+ * @adap_ids - adapter array
+ * @num_adaps - number of adapters
+ * @sel_reg_addr - mux select register offset in CPLD space
+ */
+struct mlxcpld_mux_plat_data {
+ int *adap_ids;
+ int num_adaps;
+ int sel_reg_addr;
+};
+
+#endif /* _LINUX_I2C_MLXCPLD_H */
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] i2c: qup: skip qup_i2c_suspend if the device is already runtime suspended
From: Sudeep Holla @ 2016-09-13 12:38 UTC (permalink / raw)
To: open list, linux-i2c, linux-soc, linux-arm-msm
Cc: Sudeep Holla, Andy Gross, David Brown, Wolfram Sang
In-Reply-To: <1472124219-1627-1-git-send-email-sudeep.holla@arm.com>
On Thu, Aug 25, 2016 at 12:23 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> If the i2c device is already runtime suspended, if qup_i2c_suspend is
> executed during suspend-to-idle or suspend-to-ram it will result in the
> following splat:
>
> WARNING: CPU: 3 PID: 1593 at drivers/clk/clk.c:476 clk_core_unprepare+0x80/0x90
> Modules linked in:
>
> CPU: 3 PID: 1593 Comm: bash Tainted: G W 4.8.0-rc3 #14
> Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> PC is at clk_core_unprepare+0x80/0x90
> LR is at clk_unprepare+0x28/0x40
> pc : [<ffff0000086eecf0>] lr : [<ffff0000086f0c58>] pstate: 60000145
> Call trace:
> clk_core_unprepare+0x80/0x90
> qup_i2c_disable_clocks+0x2c/0x68
> qup_i2c_suspend+0x10/0x20
> platform_pm_suspend+0x24/0x68
> dpm_run_callback.isra.7+0x1c/0x70
> __device_suspend+0xf4/0x298
> dpm_suspend+0x10c/0x228
> dpm_suspend_start+0x68/0x78
> suspend_devices_and_enter+0xb8/0x460
> pm_suspend+0x1ec/0x240
> state_store+0x84/0xf8
> kobj_attr_store+0x14/0x28
> sysfs_kf_write+0x48/0x58
> kernfs_fop_write+0x15c/0x1f8
> __vfs_write+0x1c/0x100
> vfs_write+0x9c/0x1b8
> SyS_write+0x44/0xa0
> el0_svc_naked+0x24/0x28
>
> This patch fixes the issue by executing qup_i2c_pm_suspend_runtime
> conditionally in qup_i2c_suspend.
>
Gentle ping !
Regards,
Sudeep
^ permalink raw reply
* RE: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
From: Y.B. Lu @ 2016-09-13 7:23 UTC (permalink / raw)
To: Scott Wood, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Arnd Bergmann
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Russell King, Bhupesh Sharma,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Santosh Shilimkar,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jochen Friedrich, X.B. Xie,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Claudiu Manoil, Kumar Gala, Leo Li,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1473722714.30217.196.camel-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org>
> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Scott Wood
> Sent: Tuesday, September 13, 2016 7:25 AM
> To: Y.B. Lu; linux-mmc@vger.kernel.org; ulf.hansson@linaro.org; Arnd
> Bergmann
> Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> foundation.org; netdev@vger.kernel.org; Mark Rutland; Rob Herring;
> Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil; Bhupesh
> Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B. Xie
> Subject: Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
>
> On Mon, 2016-09-12 at 06:39 +0000, Y.B. Lu wrote:
> > Hi Scott,
> >
> > Thanks for your review :)
> > See my comment inline.
> >
> > >
> > > -----Original Message-----
> > > From: Scott Wood [mailto:oss@buserror.net]
> > > Sent: Friday, September 09, 2016 11:47 AM
> > > To: Y.B. Lu; linux-mmc@vger.kernel.org; ulf.hansson@linaro.org; Arnd
> > > Bergmann
> > > Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org;
> > > linux-arm- kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > linux- clk@vger.kernel.org; linux-i2c@vger.kernel.org;
> > > iommu@lists.linux- foundation.org; netdev@vger.kernel.org; Mark
> > > Rutland; Rob Herring; Russell King; Jochen Friedrich; Joerg Roedel;
> > > Claudiu Manoil; Bhupesh Sharma; Qiang Zhao; Kumar Gala; Santosh
> > > Shilimkar; Leo Li; X.B. Xie
> > > Subject: Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ
> > > platforms
> > >
> > > On Tue, 2016-09-06 at 16:28 +0800, Yangbo Lu wrote:
> > > >
> > > > The global utilities block controls power management, I/O device
> > > > enabling, power-onreset(POR) configuration monitoring, alternate
> > > > function selection for multiplexed signals,and clock control.
> > > >
> > > > This patch adds a driver to manage and access global utilities
> block.
> > > > Initially only reading SVR and registering soc device are supported.
> > > > Other guts accesses, such as reading RCW, should eventually be
> > > > moved into this driver as well.
> > > >
> > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > > > Signed-off-by: Scott Wood <oss@buserror.net>
> > > Don't put my signoff on patches that I didn't put it on myself.
> > > Definitely don't put mine *after* yours on patches that were last
> > > modified by you.
> > >
> > > If you want to mention that the soc_id encoding was my suggestion,
> > > then do so explicitly.
> > >
> > [Lu Yangbo-B47093] I found your 'signoff' on this patch at below link.
> > http://patchwork.ozlabs.org/patch/649211/
> >
> > So, let me just change the order in next version ?
> > Signed-off-by: Scott Wood <oss@buserror.net>
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
>
> No. This isn't my patch so my signoff shouldn't be on it.
[Lu Yangbo-B47093] Ok, will remove it.
>
> > [Lu Yangbo-B47093] It's a good idea to move die into .family I think.
> > In my opinion, it's better to keep svr and name in soc_id just like
> > your suggestion above.
> > >
> > > {
> > > .soc_id = "svr:0x85490010,name:T1023E,",
> > > .family = "QorIQ T1024",
> > > }
> > The user probably don’t like to learn the svr value. What they want is
> > just to match the soc they use.
> > It's convenient to use name+rev for them to match a soc.
>
> What the user should want 99% of the time is to match the die (plus
> revision), not the soc.
>
> > Regarding shrinking the table, I think it's hard to use svr+mask.
> > Because I find many platforms use different masks.
> > We couldn’t know the mask according svr value.
>
> The mask would be part of the table:
>
> {
> {
> .die = "T1024",
> .svr = 0x85400000,
> .mask = 0xfff00000,
> },
> {
> .die = "T1040",
> .svr = 0x85200000,
> .mask = 0xfff00000,
> },
> {
> .die = "LS1088A",
> .svr = 0x87030000,
> .mask = 0xffff0000,
> },
> ...
> }
>
> There's a small risk that we get the mask wrong and a different die is
> created that matches an existing table, but it doesn't seem too likely,
> and can easily be fixed with a kernel update if it happens.
>
[Lu Yangbo-B47093] You mean we will not define soc device attribute for each soc and we will define attribute for each die instead, right?
If so, when we want to match a specific soc we need to use its svr value in code. If it's acceptable, I can try in next version.
> BTW, aren't ls2080a and ls2085a the same die? And is there no non-E
> version of LS2080A/LS2040A?
[Lu Yangbo-B47093] I checked all the svr values in chip errata doc "Revision level to part marking cross-reference" table.
I found ls2080a and ls2085a were in two separate doc. And I didn’t find non-E version of LS2080A/LS2040A in chip errata doc.
Do you know is there any other doc we can confirm this?
>
> > > > + do {
> > > > + if (!matches->soc_id)
> > > > + return NULL;
> > > > + if (glob_match(svr_match, matches->soc_id))
> > > > + break;
> > > > + } while (matches++);
> > > Are you expecting "matches++" to ever evaluate as false?
> > [Lu Yangbo-B47093] Yes, this is used to match the soc we use in
> > qoriq_soc array until getting true.
> > We need to get the name and die information defined in array.
>
> I'm not asking whether the glob_match will ever return true. I'm saying
> that "matches++" will never become NULL.
[Lu Yangbo-B47093] The matches++ will never become NULL while it will return NULL after matching for all the members in array.
>
> > > > + /* Register soc device */
> > > > + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> > > > + if (!soc_dev_attr) {
> > > > + ret = -ENOMEM;
> > > > + goto out_unmap;
> > > > + }
> > > Couldn't this be statically allocated?
> > [Lu Yangbo-B47093] Do you mean we define this struct statically ?
> >
> > static struct soc_device_attribute soc_dev_attr;
>
> Yes.
>
[Lu Yangbo-B47093] It's ok to define it statically. Is there any need to do that?
> > > > +
> > > > + soc_dev = soc_device_register(soc_dev_attr);
> > > > + if (IS_ERR(soc_dev)) {
> > > > + ret = -ENODEV;
> > > Why are you changing the error code?
> > [Lu Yangbo-B47093] What error code should we use ? :)
>
> ret = PTR_ERR(soc_dev);
[Lu Yangbo-B47093] Ok.. will do that.
>
> + }
> > > > + return 0;
> > > > +out:
> > > > + kfree(soc_dev_attr->machine);
> > > > + kfree(soc_dev_attr->family);
> > > > + kfree(soc_dev_attr->soc_id);
> > > > + kfree(soc_dev_attr->revision);
> > > > + kfree(soc_dev_attr);
> > > > +out_unmap:
> > > > + iounmap(guts->regs);
> > > > +out_free:
> > > > + kfree(guts);
> > > devm
> > [Lu Yangbo-B47093] What's the devm meaning here :)
>
> If you allocate these with devm_kzalloc(), devm_kasprintf(),
> devm_kstrdup(), etc. then they will be freed automatically when the
> device is unbound.
>
> >
> > >
> > >
> > > >
> > > > +static int fsl_guts_remove(struct platform_device *dev) {
> > > > + kfree(soc_dev_attr->machine);
> > > > + kfree(soc_dev_attr->family);
> > > > + kfree(soc_dev_attr->soc_id);
> > > > + kfree(soc_dev_attr->revision);
> > > > + kfree(soc_dev_attr);
> > > > + soc_device_unregister(soc_dev);
> > > > + iounmap(guts->regs);
> > > > + kfree(guts);
> > > > + return 0;
> > > > +}
> > > Don't free the memory before you unregister the device that uses it
> > > (moot if you use devm).
> > [Lu Yangbo-B47093] The soc.c driver mentions that.
> > Ensure soc_dev->attr is freed prior to calling soc_device_unregister.
>
> That comment is wrong. Freeing the memory first creates a race condition
> that could result in accessing freed memory, if something accesses the
> soc device in parallel with unbinding.
>
[Lu Yangbo-B47093] Ok, will unregister the device first. Thanks.
> -Scott
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* Re: [PATCH v7] i2c: imx: make bus recovery through pinctrl optional
From: Uwe Kleine-König @ 2016-09-13 6:47 UTC (permalink / raw)
To: Li Yang
Cc: Wolfram Sang, linux-i2c, linux-kernel, linux-gpio,
linux-arm-kernel, Gao Pan
In-Reply-To: <1473718950-6667-1-git-send-email-leoyang.li@nxp.com>
On Mon, Sep 12, 2016 at 05:22:30PM -0500, Li Yang wrote:
> Since commit 1c4b6c3bcf30 ("i2c: imx: implement bus recovery") the
> driver starts to use gpio/pinctrl to support optional bus recovery
> feature. But pinctrl is not always usable. There are platforms such
> as ls1021a and ls1043a that don't support pinctrl, and it could just
> be broken due to old/broken device tree. The patch makes it really
> optional that the probe function won't bailout on pinctrl problems
> instead it just disables bus recovery and prints out notification when
> there is problem with pinctrl. Since pinctrl is only used by bus
> recovery in this driver, move pinctrl initialization into bus recovery
> init function to prevent confusion.
>
> Signed-off-by: Li Yang <leoyang.li@nxp.com>
> Cc: Gao Pan <pandy.gao@nxp.com>
> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
From: Scott Wood @ 2016-09-12 23:25 UTC (permalink / raw)
To: Y.B. Lu, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Arnd Bergmann
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Russell King, Bhupesh Sharma,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Santosh Shilimkar,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jochen Friedrich, X.B. Xie,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Claudiu Manoil, Kumar Gala, Leo Li,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <HE1PR04MB08892D8354E38EC32F549E98F8FF0-6LN7OEpIatX1kPMWxTxe+c9NdZoXdze2vxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
On Mon, 2016-09-12 at 06:39 +0000, Y.B. Lu wrote:
> Hi Scott,
>
> Thanks for your review :)
> See my comment inline.
>
> >
> > -----Original Message-----
> > From: Scott Wood [mailto:oss@buserror.net]
> > Sent: Friday, September 09, 2016 11:47 AM
> > To: Y.B. Lu; linux-mmc@vger.kernel.org; ulf.hansson@linaro.org; Arnd
> > Bergmann
> > Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> > clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> > foundation.org; netdev@vger.kernel.org; Mark Rutland; Rob Herring;
> > Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil; Bhupesh
> > Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B. Xie
> > Subject: Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
> >
> > On Tue, 2016-09-06 at 16:28 +0800, Yangbo Lu wrote:
> > >
> > > The global utilities block controls power management, I/O device
> > > enabling, power-onreset(POR) configuration monitoring, alternate
> > > function selection for multiplexed signals,and clock control.
> > >
> > > This patch adds a driver to manage and access global utilities block.
> > > Initially only reading SVR and registering soc device are supported.
> > > Other guts accesses, such as reading RCW, should eventually be moved
> > > into this driver as well.
> > >
> > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > > Signed-off-by: Scott Wood <oss@buserror.net>
> > Don't put my signoff on patches that I didn't put it on
> > myself. Definitely don't put mine *after* yours on patches that were
> > last modified by you.
> >
> > If you want to mention that the soc_id encoding was my suggestion, then
> > do so explicitly.
> >
> [Lu Yangbo-B47093] I found your 'signoff' on this patch at below link.
> http://patchwork.ozlabs.org/patch/649211/
>
> So, let me just change the order in next version ?
> Signed-off-by: Scott Wood <oss@buserror.net>
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
No. This isn't my patch so my signoff shouldn't be on it.
> [Lu Yangbo-B47093] It's a good idea to move die into .family I think.
> In my opinion, it's better to keep svr and name in soc_id just like your
> suggestion above.
> >
> > {
> > .soc_id = "svr:0x85490010,name:T1023E,",
> > .family = "QorIQ T1024",
> > }
> The user probably don’t like to learn the svr value. What they want is just
> to match the soc they use.
> It's convenient to use name+rev for them to match a soc.
What the user should want 99% of the time is to match the die (plus revision),
not the soc.
> Regarding shrinking the table, I think it's hard to use svr+mask. Because I
> find many platforms use different masks.
> We couldn’t know the mask according svr value.
The mask would be part of the table:
{
{
.die = "T1024",
.svr = 0x85400000,
.mask = 0xfff00000,
},
{
.die = "T1040",
.svr = 0x85200000,
.mask = 0xfff00000,
},
{
.die = "LS1088A",
.svr = 0x87030000,
.mask = 0xffff0000,
},
...
}
There's a small risk that we get the mask wrong and a different die is created
that matches an existing table, but it doesn't seem too likely, and can easily
be fixed with a kernel update if it happens.
BTW, aren't ls2080a and ls2085a the same die? And is there no non-E version
of LS2080A/LS2040A?
> > > + do {
> > > + if (!matches->soc_id)
> > > + return NULL;
> > > + if (glob_match(svr_match, matches->soc_id))
> > > + break;
> > > + } while (matches++);
> > Are you expecting "matches++" to ever evaluate as false?
> [Lu Yangbo-B47093] Yes, this is used to match the soc we use in qoriq_soc
> array until getting true.
> We need to get the name and die information defined in array.
I'm not asking whether the glob_match will ever return true. I'm saying that
"matches++" will never become NULL.
> > > + /* Register soc device */
> > > + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> > > + if (!soc_dev_attr) {
> > > + ret = -ENOMEM;
> > > + goto out_unmap;
> > > + }
> > Couldn't this be statically allocated?
> [Lu Yangbo-B47093] Do you mean we define this struct statically ?
>
> static struct soc_device_attribute soc_dev_attr;
Yes.
> > > +
> > > + soc_dev = soc_device_register(soc_dev_attr);
> > > + if (IS_ERR(soc_dev)) {
> > > + ret = -ENODEV;
> > Why are you changing the error code?
> [Lu Yangbo-B47093] What error code should we use ? :)
ret = PTR_ERR(soc_dev);
+ }
> > > + return 0;
> > > +out:
> > > + kfree(soc_dev_attr->machine);
> > > + kfree(soc_dev_attr->family);
> > > + kfree(soc_dev_attr->soc_id);
> > > + kfree(soc_dev_attr->revision);
> > > + kfree(soc_dev_attr);
> > > +out_unmap:
> > > + iounmap(guts->regs);
> > > +out_free:
> > > + kfree(guts);
> > devm
> [Lu Yangbo-B47093] What's the devm meaning here :)
If you allocate these with devm_kzalloc(), devm_kasprintf(), devm_kstrdup(),
etc. then they will be freed automatically when the device is unbound.
>
> >
> >
> > >
> > > +static int fsl_guts_remove(struct platform_device *dev) {
> > > + kfree(soc_dev_attr->machine);
> > > + kfree(soc_dev_attr->family);
> > > + kfree(soc_dev_attr->soc_id);
> > > + kfree(soc_dev_attr->revision);
> > > + kfree(soc_dev_attr);
> > > + soc_device_unregister(soc_dev);
> > > + iounmap(guts->regs);
> > > + kfree(guts);
> > > + return 0;
> > > +}
> > Don't free the memory before you unregister the device that uses it (moot
> > if you use devm).
> [Lu Yangbo-B47093] The soc.c driver mentions that.
> Ensure soc_dev->attr is freed prior to calling soc_device_unregister.
That comment is wrong. Freeing the memory first creates a race condition that
could result in accessing freed memory, if something accesses the soc device
in parallel with unbinding.
-Scott
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* [PATCH v7] i2c: imx: make bus recovery through pinctrl optional
From: Li Yang @ 2016-09-12 22:22 UTC (permalink / raw)
To: Wolfram Sang, linux-i2c, linux-kernel, linux-gpio,
linux-arm-kernel
Cc: Uwe Kleine-König, Gao Pan, Li Yang
Since commit 1c4b6c3bcf30 ("i2c: imx: implement bus recovery") the
driver starts to use gpio/pinctrl to support optional bus recovery
feature. But pinctrl is not always usable. There are platforms such
as ls1021a and ls1043a that don't support pinctrl, and it could just
be broken due to old/broken device tree. The patch makes it really
optional that the probe function won't bailout on pinctrl problems
instead it just disables bus recovery and prints out notification when
there is problem with pinctrl. Since pinctrl is only used by bus
recovery in this driver, move pinctrl initialization into bus recovery
init function to prevent confusion.
Signed-off-by: Li Yang <leoyang.li@nxp.com>
Cc: Gao Pan <pandy.gao@nxp.com>
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
v7:
Move pinctrl init into the i2c_imx_init_recovery_info()
Update commit message and code comment
v6:
Update code comment
v5:
Revert the last minute change of recovery info initialization timing, it
will cause problem if initialized after i2c_add_numbered_adapter()
v4:
Remove the use of IS_ERR_OR_NULL
Move the condition judgement to i2c_imx_init_recovery_info()
Change the timing of recovery initialization to be after bus registration
v3:
Rebased to Wolfram's for-next branch
Added acked-by from Linus Walleij
Update to use new nxp email addresses due to company merge
drivers/i2c/busses/i2c-imx.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 1844bc9..54ce9c1 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -984,11 +984,24 @@ static void i2c_imx_unprepare_recovery(struct i2c_adapter *adap)
pinctrl_select_state(i2c_imx->pinctrl, i2c_imx->pinctrl_pins_default);
}
-static void i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
+/*
+ * We switch SCL and SDA to their GPIO function and do some bitbanging
+ * for bus recovery. These alternative pinmux settings can be
+ * described in the device tree by a separate pinctrl state "gpio". If
+ * this is missing this is not a big problem, the only implication is
+ * that we can't do bus recovery.
+ */
+static int i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
struct platform_device *pdev)
{
struct i2c_bus_recovery_info *rinfo = &i2c_imx->rinfo;
+ i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
+ if (!i2c_imx->pinctrl || IS_ERR(i2c_imx->pinctrl)) {
+ dev_info(&pdev->dev, "can't get pinctrl, bus recovery not supported\n");
+ return PTR_ERR(i2c_imx->pinctrl);
+ }
+
i2c_imx->pinctrl_pins_default = pinctrl_lookup_state(i2c_imx->pinctrl,
PINCTRL_STATE_DEFAULT);
i2c_imx->pinctrl_pins_gpio = pinctrl_lookup_state(i2c_imx->pinctrl,
@@ -1001,7 +1014,7 @@ static void i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
IS_ERR(i2c_imx->pinctrl_pins_default) ||
IS_ERR(i2c_imx->pinctrl_pins_gpio)) {
dev_dbg(&pdev->dev, "recovery information incomplete\n");
- return;
+ return 0;
}
dev_dbg(&pdev->dev, "using scl-gpio %d and sda-gpio %d for recovery\n",
@@ -1011,6 +1024,8 @@ static void i2c_imx_init_recovery_info(struct imx_i2c_struct *i2c_imx,
rinfo->unprepare_recovery = i2c_imx_unprepare_recovery;
rinfo->recover_bus = i2c_generic_gpio_recovery;
i2c_imx->adapter.bus_recovery_info = rinfo;
+
+ return 0;
}
static u32 i2c_imx_func(struct i2c_adapter *adapter)
@@ -1081,12 +1096,6 @@ static int i2c_imx_probe(struct platform_device *pdev)
return ret;
}
- i2c_imx->pinctrl = devm_pinctrl_get(&pdev->dev);
- if (IS_ERR(i2c_imx->pinctrl)) {
- ret = PTR_ERR(i2c_imx->pinctrl);
- goto clk_disable;
- }
-
/* Request IRQ */
ret = devm_request_irq(&pdev->dev, irq, i2c_imx_isr, 0,
pdev->name, i2c_imx);
@@ -1125,7 +1134,11 @@ static int i2c_imx_probe(struct platform_device *pdev)
i2c_imx, IMX_I2C_I2CR);
imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
- i2c_imx_init_recovery_info(i2c_imx, pdev);
+ /* Init optional bus recovery function */
+ ret = i2c_imx_init_recovery_info(i2c_imx, pdev);
+ /* Give it another chance if pinctrl used is not ready yet */
+ if (ret == -EPROBE_DEFER)
+ goto rpm_disable;
/* Add I2C adapter */
ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
--
1.9.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Leo Li @ 2016-09-12 18:29 UTC (permalink / raw)
To: Gao Pan, Ying Zhang
Cc: Uwe Kleine-König, linux-i2c@vger.kernel.org, Wolfram Sang,
linux-arm-kernel@lists.infradead.org, linux-gpio, Li Yang,
Stefan Agner, lkml, Tracy Smith
In-Reply-To: <CADRPPNSLZ-Ck+40LZLdpEXPaBm+61AtrpRPgAcTCojWSsTBQDw@mail.gmail.com>
On Mon, Sep 12, 2016 at 11:35 AM, Leo Li <pku.leo@gmail.com> wrote:
> On Wed, Sep 7, 2016 at 5:07 AM, Tracy Smith <tlsmith3777@gmail.com> wrote:
>> Hello, bus recovery is needed generally speaking because of potential
>> protocol errors that might cause a failure condition hanging the bus.
>>
>> It happens frequently during bring-up of new I2C devices because firmware in
>> I2C controllers fail to handle properly protocol errors.
>>
>> Can NXP add bus recovery for the LS1021A and LS1043A in a separate patch--
>> unless there is no HW bus recovery mechanism?
>>
>> The concern is while fixing I.MX, NXP will fail to fix the driver bus
>> recovery for the LS1021A and LS1043A and the bus will hang.
>>
>> If bus recovery is supported on the LS1021A and the LS1043A, a patch should
>> be provided or added in this patch instead of simply disabling bus recovery.
>> Request NXP to consider the patch if there is HW support for bus recovery.
>
> FYI. http://patchwork.ozlabs.org/patch/573879/
Hi Gao Pan,
Do you think the operations in the proposed patch is also usable on
the I.MX SoC for bus recovery? If so, would it be better to align the
bus recovery implementation for both I.MX and QorIQ to use the same
logic inside the IP?
Regards,
Leo
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Leo Li @ 2016-09-12 16:47 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Stefan Agner, Gao Pan, Wolfram Sang, lkml, Li Yang, linux-gpio,
linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20160909205926.dbxmvhqvfsqzk6dx@pengutronix.de>
On Fri, Sep 9, 2016 at 3:59 PM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Hello,
>
> On Fri, Sep 09, 2016 at 01:34:31PM -0700, Stefan Agner wrote:
>> Yeah it is a bit a wording thing: In my understanding, pinctrl is
>> required on SoC's witch have a pin controller... It is just that the
>> driver does not need to get the pinctrl by itself because the stack is
>> taking care of it implicitly. And yes, that makes the particular example
>> not a real world example.
>
> At first I thought, too, that it's a fatal problem if getting the
> pinctrl stuff fails. IMHO that shows that the comments (or the code) are
> still not good enough.
Ya. If it has confused more than one people, it is likely to confuse
more. I agree with you that we should make it more clear.
Thanks,
Leo
^ permalink raw reply
* Re: [patch v3] i2c: mux: mellanox: add driver
From: Peter Rosin @ 2016-09-12 16:05 UTC (permalink / raw)
To: vadimp, wsa; +Cc: linux-i2c, linux-kernel, jiri, Michael Shych
In-Reply-To: <1473438676-64981-1-git-send-email-vadimp@mellanox.com>
First off, sorry for not catching all issues at once, I'm not an experienced
reviewer...
On 2016-09-09 18:31, vadimp@mellanox.com wrote:
> From: Vadim Pasternak <vadimp@mellanox.com>
>
> This driver allows I2C routing controlled through CPLD select registers on
> wide range of Mellanox systems (CPLD Lattice device).
> MUX selection is provided by digital and analog HW. Analog part is not
> under SW control.
> Digital part is under CPLD control (channel selection/de-selection).
>
> Connectivity schema.
> i2c-mlxcpld Digital Analog
> driver
> *--------* * -> mux1 (virt bus2) -> mux ->|
> | I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux ->|
> | bridge | bus 1 *---------* |
> | logic |---------------------> * mux reg * |
> | in CPLD| *---------* |
> *--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux ->|
> | driver | |
> | *---------------* | Devices
> | * CPLD (i2c bus)* select |
> | * registers for *--------*
> | * mux selection * deselect
> | *---------------*
> | |
> <--------> <----------->
> i2c cntrl Board cntrl reg
> reg space space (mux select,
> IO, LED, WD, info)
>
> i2c-mux-mlxpcld does not necessarily require i2c-mlxcpld. It can be use
> along with another bus driver, and still control i2c routing through CPLD
> mux selection, in case the system is equipped with CPLD capable of mux
> selection control.
>
> The Kconfig currently controlling compilation of this code is:
> drivers/i2c/muxes/Kconfig:config I2C_MUX_MLXCPLD
>
> Signed-off-by: Michael Shych <michaelsh@mellanox.com>
> Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v2->v3
> Fixes issues pointed out by Peter:
> - Fix several comments;
> - In MAINTAINERS file use linux-i2c instead of linux-kernel;
> - Place include files in alphabetic order;
> - When channel select fail, set last_chan to zero for cleaning last value;
> - Return with error from probe if there is no platform data;
> - Use i2c_mux_reg driver for the lpc_access cases:
> it fit well for Mellanox system with LPC access - has been tested and it
> works good.
> - Limit this driver to i2c_access only one device (mlxcpld_mux_module).
> v1->v2
> Fixes issues pointed out by Peter:
> - changes in commit cover massage;
> - change leg to channel in comments;
> - missed return err;
> - use platform data mux array rather the offset from 1st channel in probe
> routine;
> - remove owner field from driver structure;
> - use module_i2c_driver macros, instead if __init and __exit;
> - remove deselect on exit flag;
> - add record to MAINTAINER file;
> ---
> MAINTAINERS | 8 ++
> drivers/i2c/muxes/Kconfig | 11 ++
> drivers/i2c/muxes/Makefile | 1 +
> drivers/i2c/muxes/i2c-mux-mlxcpld.c | 257 ++++++++++++++++++++++++++++++++++++
> include/linux/i2c/mlxcpld.h | 57 ++++++++
> 5 files changed, 334 insertions(+)
> create mode 100644 drivers/i2c/muxes/i2c-mux-mlxcpld.c
> create mode 100644 include/linux/i2c/mlxcpld.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0bbe4b1..be83d69 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7655,6 +7655,14 @@ W: http://www.mellanox.com
> Q: http://patchwork.ozlabs.org/project/netdev/list/
> F: drivers/net/ethernet/mellanox/mlxsw/
>
> +MELLANOX MLXCPLD I2C MUX DRIVER
> +M: Vadim Pasternak <vadimp@mellanox.com>
> +M: Michael Shych <michaelsh@mellanox.com>
> +L: linux-i2c@vger.kernel.org
> +S: Supported
> +W: http://www.mellanox.com
> +F: drivers/i2c/muxes/i2c-mux-mlxcpld.c
> +
> SOFT-ROCE DRIVER (rxe)
> M: Moni Shoua <monis@mellanox.com>
> L: linux-rdma@vger.kernel.org
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index e280c8e..b7ab144 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -81,4 +81,15 @@ config I2C_DEMUX_PINCTRL
> demultiplexer that uses the pinctrl subsystem. This is useful if you
> want to change the I2C master at run-time depending on features.
>
> +config I2C_MUX_MLXCPLD
> + tristate "Mellanox CPLD based I2C multiplexer"
> + help
> + If you say yes to this option, support will be included for a
> + CPLD based I2C multiplexer. This driver provides access to
> + I2C busses connected through a MUX, which is controlled
> + by a CPLD registers.
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-mux-mlxcpld.
> +
> endmenu
> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
> index 7c267c2..e5c990e 100644
> --- a/drivers/i2c/muxes/Makefile
> +++ b/drivers/i2c/muxes/Makefile
> @@ -10,5 +10,6 @@ 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
> obj-$(CONFIG_I2C_MUX_REG) += i2c-mux-reg.o
> +obj-$(CONFIG_I2C_MUX_MLXCPLD) += i2c-mux-mlxcpld.o
>
> ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
> diff --git a/drivers/i2c/muxes/i2c-mux-mlxcpld.c b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> new file mode 100644
> index 0000000..0ca3d55
> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-mux-mlxcpld.c
> @@ -0,0 +1,257 @@
> +/*
> + * drivers/i2c/muxes/i2c-mux-mlxcpld.c
> + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the names of the copyright holders nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") version 2 as published by the Free
> + * Software Foundation.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-mux.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/version.h>
> +#include <linux/i2c/mlxcpld.h>
> +
> +#define CPLD_MUX_MAX_NCHANS 8
> +
> +/*
> + * mlxcpld_mux types - kind of mux supported by driver:
> + * @mlxcpld_mux_module - I2C access; 8 channels/legs.
> + */
> +enum mlxcpld_mux_type {
> + mlxcpld_mux_module,
> +};
This enum is now pointless.
> +
> +/* mlxcpld_mux - mux control structure:
> + * @type - mux type
> + * @last_chan - last register value
> + * @client - I2C device client
> + */
> +struct mlxcpld_mux {
> + enum mlxcpld_mux_type type;
This member is now pointless.
> + u8 last_chan;
> + struct i2c_client *client;
> +};
> +
> +/* mlxcpld_mux_desc - mux descriptor structure:
> + * @nchans - number of channels
> + */
> +struct mlxcpld_mux_desc {
> + u8 nchans;
> +};
> +
> +/* MUX logic description.
> + * Driver can support different mux control logic, according to CPLD
> + * implementation.
> + *
> + * Connectivity schema.
> + *
> + * i2c-mlxcpld Digital Analog
> + * driver
> + * *--------* * -> mux1 (virt bus2) -> mux -> |
> + * | I2CLPC | i2c physical * -> mux2 (virt bus3) -> mux -> |
> + * | bridge | bus 1 *---------* |
> + * | logic |---------------------> * mux reg * |
> + * | in CPLD| *---------* |
> + * *--------* i2c-mux-mlxpcld ^ * -> muxn (virt busn) -> mux -> |
> + * | driver | |
> + * | *---------------* | Devices
> + * | * CPLD (i2c bus)* select |
> + * | * registers for *--------*
> + * | * mux selection * deselect
> + * | *---------------*
> + * | |
> + * <--------> <----------->
> + * i2c cntrl Board cntrl reg
> + * reg space space (mux select,
> + * IO, LED, WD, info)
> + *
> + */
> +static const struct mlxcpld_mux_desc muxes[] = {
> + [mlxcpld_mux_module] = {
> + .nchans = CPLD_MUX_MAX_NCHANS,
> + },
> +};
This variable is now pointless.
> +
> +static const struct i2c_device_id mlxcpld_mux_id[] = {
> + { "mlxcpld_mux_module", mlxcpld_mux_module },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, mlxcpld_mux_id);
> +
> +/* Write to mux register. Don't use i2c_transfer() and
> + * i2c_smbus_xfer() for this as they will try to lock adapter a second time
> + */
> +static int mlxcpld_mux_reg_write(struct i2c_adapter *adap,
> + struct i2c_client *client, u8 val)
> +{
> + struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
> +
> + if (adap->algo->master_xfer) {
> + struct i2c_msg msg;
> + u8 msgbuf[] = {pdata->sel_reg_addr, val};
> +
> + msg.addr = pdata->addr;
Maybe use client->addr here?
> + msg.flags = 0;
> + msg.len = 2;
> + msg.buf = msgbuf;
> + return __i2c_transfer(adap, &msg, 1);
> + }
> + dev_err(&client->dev, "SMBus isn't supported on this adapter\n");
This error message is misleading, since you are not checking for the
smbus_xfer operation so you can't really say that it's not supported.
Maybe add an else branch instead of the dev_err like so (completely untested):
} else {
union i2c_smbus_data data;
data.byte = val;
ret = adap->algo->smbus_xfer(adap, client->addr,
client->flags, I2C_SMBUS_WRITE,
pdata->sel_reg_addr,
I2C_SMBUS_BYTE_DATA, &data);
}
> +
> + return -ENODEV;
> +}
> +
> +static int mlxcpld_mux_select_chan(struct i2c_mux_core *muxc, u32 chan)
> +{
> + struct mlxcpld_mux *data = i2c_mux_priv(muxc);
> + struct i2c_client *client = data->client;
> + u8 regval;
> + int err = 0;
> +
> + if (data->type == mlxcpld_mux_module)
This comparison is now pointless.
> + regval = chan + 1;
> + else
> + return -ENXIO;
> +
> + /* Only select the channel if its different from the last channel */
> + if (data->last_chan != regval) {
> + err = mlxcpld_mux_reg_write(muxc->parent, client, regval);
> + if (err)
> + data->last_chan = 0;
> + else
> + data->last_chan = regval;
> + }
> +
> + return err;
> +}
> +
> +static int mlxcpld_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
> +{
> + struct mlxcpld_mux *data = i2c_mux_priv(muxc);
> + struct i2c_client *client = data->client;
> +
> + /* Deselect active channel */
> + data->last_chan = 0;
> +
> + return mlxcpld_mux_reg_write(muxc->parent, client, data->last_chan);
> +}
> +
> +/* I2C init/probing/exit functions */
> +static int mlxcpld_mux_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct i2c_adapter *adap = to_i2c_adapter(client->dev.parent);
> + struct mlxcpld_mux_plat_data *pdata = dev_get_platdata(&client->dev);
> + struct i2c_mux_core *muxc;
> + int num, force;
> + u8 nchans;
> + struct mlxcpld_mux *data;
> + int err;
> +
> + if (!pdata)
> + return -EINVAL;
> +
> + if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
> + return -ENODEV;
This is not what you actually end up using, I think. You use a register
number and a value, and you always write so I2C_FUNC_SMBUS_WRITE_BYTE_DATA
seems like a better fit. That is what you should check for.
> +
> + switch (id->driver_data) {
> + case mlxcpld_mux_module:
> + nchans = muxes[id->driver_data].nchans;
> + break;
> + default:
> + return -EINVAL;
> + }
This switch statement is now pointless. nchans always ends up 8.
> +
> + muxc = i2c_mux_alloc(adap, &client->dev, nchans, sizeof(*data), 0,
> + mlxcpld_mux_select_chan, mlxcpld_mux_deselect);
> + if (!muxc)
> + return -ENOMEM;
> +
> + data = i2c_mux_priv(muxc);
> + i2c_set_clientdata(client, muxc);
> + data->client = client;
> + data->type = id->driver_data;
> + data->last_chan = 0; /* force the first selection */
> +
> + /* Only in mlxcpld_mux_tor first_channel can be different.
> + * In other mlxcpld_mux types channel numbering begin from 1
> + * Now create an adapter for each channel
> + */
The comment needs an update.
> + for (num = 0; num < muxes[data->type].nchans; num++) {
> + force = 0; /* dynamic adap number */
This assignment is pointless.
> + if (num < pdata->num_adaps)
> + force = pdata->adap_ids[num];
> + else
> + /* discard unconfigured channels */
> + break;
> +
I would write:
if (num >= pdata->num_adaps)
/* discard unconfigured channels */
break;
force = pdata->adap_ids[num];
> + err = i2c_mux_add_adapter(muxc, force, num, 0);
> + if (err) {
> + dev_err(&client->dev, "failed to register multiplexed adapter %d as bus %d\n",
> + num, force);
> + goto virt_reg_failed;
> + }
> + }
> +
> + return 0;
> +
> +virt_reg_failed:
> + i2c_mux_del_adapters(muxc);
> + return err;
> +}
> +
> +static int mlxcpld_mux_remove(struct i2c_client *client)
> +{
> + struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> +
> + i2c_mux_del_adapters(muxc);
> + return 0;
> +}
> +
> +static struct i2c_driver mlxcpld_mux_driver = {
> + .driver = {
> + .name = "mlxcpld-mux",
> + },
> + .probe = mlxcpld_mux_probe,
> + .remove = mlxcpld_mux_remove,
> + .id_table = mlxcpld_mux_id,
> +};
> +
> +module_i2c_driver(mlxcpld_mux_driver);
> +
> +MODULE_AUTHOR("Michael Shych (michaels@mellanox.com)");
> +MODULE_DESCRIPTION("Mellanox I2C-CPLD-MUX driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:i2c-mux-mlxcpld");
> diff --git a/include/linux/i2c/mlxcpld.h b/include/linux/i2c/mlxcpld.h
> new file mode 100644
> index 0000000..1b46e5e
> --- /dev/null
> +++ b/include/linux/i2c/mlxcpld.h
> @@ -0,0 +1,57 @@
> +/*
> + * mlxcpld.h - Mellanox I2C multiplexer support in CPLD
> + *
> + * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
> + * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the names of the copyright holders nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * Alternatively, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") version 2 as published by the Free
> + * Software Foundation.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _LINUX_I2C_MLXCPLD_H
> +#define _LINUX_I2C_MLXCPLD_H
> +
> +/* Platform data for the CPLD I2C multiplexers */
> +
> +/* mlxcpld_mux_plat_data - per mux data, used with i2c_register_board_info
> + * @adap_ids - adapter array
> + * @num_adaps - number of adapters
> + * @sel_reg_addr - mux select register offset in CPLD space
> + * @first_channel - first channel to start virtual busses vector
> + * @addr - address of mux device - set to mux select register offset on LPC
> + * connected CPLDs or to I2C address on I2C conncted CPLDs
> + */
> +struct mlxcpld_mux_plat_data {
> + int *adap_ids;
> + int num_adaps;
> + int sel_reg_addr;
sel_reg_addr may be static information?
> + int first_channel;
This member is now pointless.
> + int addr;
This member is pointless if you can use client->addr in mlxcpld_mux_reg_write().
> +};
> +
> +#endif /* _LINUX_I2C_MLXCPLD_H */
>
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Leo Li @ 2016-09-12 16:35 UTC (permalink / raw)
To: Tracy Smith
Cc: Uwe Kleine-König, linux-i2c@vger.kernel.org, Wolfram Sang,
linux-arm-kernel@lists.infradead.org, linux-gpio, Li Yang,
Stefan Agner, Gao Pan, lkml
In-Reply-To: <CAChUvXMPDZB7=ZArQ5+6ae42N=Bsgv_OczGXnyhBjEdjt_yvsw@mail.gmail.com>
On Wed, Sep 7, 2016 at 5:07 AM, Tracy Smith <tlsmith3777@gmail.com> wrote:
> Hello, bus recovery is needed generally speaking because of potential
> protocol errors that might cause a failure condition hanging the bus.
>
> It happens frequently during bring-up of new I2C devices because firmware in
> I2C controllers fail to handle properly protocol errors.
>
> Can NXP add bus recovery for the LS1021A and LS1043A in a separate patch--
> unless there is no HW bus recovery mechanism?
>
> The concern is while fixing I.MX, NXP will fail to fix the driver bus
> recovery for the LS1021A and LS1043A and the bus will hang.
>
> If bus recovery is supported on the LS1021A and the LS1043A, a patch should
> be provided or added in this patch instead of simply disabling bus recovery.
> Request NXP to consider the patch if there is HW support for bus recovery.
FYI. http://patchwork.ozlabs.org/patch/573879/ This seem to be the
patch you are asking for. I have asked the original developer to
update the patch according to Wolfram's comment and work together with
the current pinctrl/gpio based recovery. In the meanwhile you can
make use of the patch for now for LS1021A and LS1043A if necessary.
Regards,
Leo
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Leo Li @ 2016-09-12 16:27 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Stefan Agner, Gao Pan, Wolfram Sang, lkml, Li Yang, linux-gpio,
linux-i2c@vger.kernel.org, Uwe Kleine-König,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20160912102136.04c0728d@ipc1.ka-ro>
On Mon, Sep 12, 2016 at 3:21 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> Hi,
>
> On Fri, 9 Sep 2016 14:37:12 -0500 Leo Li wrote:
>> On Fri, Sep 9, 2016 at 11:51 AM, Stefan Agner <stefan@agner.ch> wrote:
>> > On 2016-09-08 16:57, Leo Li wrote:
> [...]
>> >> people fix problem that they don't really care by deliberately enlarge
>> >> the problem. That's why we don't panic() on any error we found. For
>> >> those who do care about the bus recovery, they can get the information
>> >> from the console.
>> >
>> > IMHO, it is just stupid to ignore errors and then let the developer
>> > later on trace back what the initial issue was. Error out early is a
>> > common sense software design principle...
>> >
>> > I am not asking for a panic(), I am just suggesting to only ignore
>> > pinctrl if it returns -ENODEV, the case you care are about.
>>
>> It was just an analogy for enlarging the problem for getting
>> attention. But you probably thought that it was not enlarging the
>> problem as you think pinctrl is required by the driver instead of an
>> optional thing.
>>
> The I2C bus recovery is a feature which is not used during normal
> operation and you won't find that it's not working unless something
> unusual happens (probably only in a productive environment). It may even
> be hard to trigger a condition to test the feature.
> Thus IMO it is vital that the driver complains loudly if something is
> missing to make this feature work if requested.
I agree. A message is already added in this patch to mention that bus
recovery is not supported due to pinctrl when there is a problem.
Regards,
Leo
^ permalink raw reply
* Re: [PATCH v5] i2c: imx: make bus recovery through pinctrl optional
From: Lothar Waßmann @ 2016-09-12 8:21 UTC (permalink / raw)
To: Leo Li
Cc: Stefan Agner, Gao Pan, Wolfram Sang, lkml, Li Yang, linux-gpio,
linux-i2c@vger.kernel.org, Uwe Kleine-König,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CADRPPNS3PecYRaAMR+0RuzU+gE3y5ZN-x7HzqggrdDsN-CkArg@mail.gmail.com>
Hi,
On Fri, 9 Sep 2016 14:37:12 -0500 Leo Li wrote:
> On Fri, Sep 9, 2016 at 11:51 AM, Stefan Agner <stefan@agner.ch> wrote:
> > On 2016-09-08 16:57, Leo Li wrote:
[...]
> >> people fix problem that they don't really care by deliberately enlarge
> >> the problem. That's why we don't panic() on any error we found. For
> >> those who do care about the bus recovery, they can get the information
> >> from the console.
> >
> > IMHO, it is just stupid to ignore errors and then let the developer
> > later on trace back what the initial issue was. Error out early is a
> > common sense software design principle...
> >
> > I am not asking for a panic(), I am just suggesting to only ignore
> > pinctrl if it returns -ENODEV, the case you care are about.
>
> It was just an analogy for enlarging the problem for getting
> attention. But you probably thought that it was not enlarging the
> problem as you think pinctrl is required by the driver instead of an
> optional thing.
>
The I2C bus recovery is a feature which is not used during normal
operation and you won't find that it's not working unless something
unusual happens (probably only in a productive environment). It may even
be hard to trigger a condition to test the feature.
Thus IMO it is vital that the driver complains loudly if something is
missing to make this feature work if requested.
Lothar Waßmann
^ permalink raw reply
* RE: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
From: Y.B. Lu @ 2016-09-12 6:39 UTC (permalink / raw)
To: Scott Wood, linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
Arnd Bergmann
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Russell King, Bhupesh Sharma,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Santosh Shilimkar,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jochen Friedrich, X.B. Xie,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Claudiu Manoil, Kumar Gala, Leo Li,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1473392840.30217.170.camel-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org>
Hi Scott,
Thanks for your review :)
See my comment inline.
> -----Original Message-----
> From: Scott Wood [mailto:oss@buserror.net]
> Sent: Friday, September 09, 2016 11:47 AM
> To: Y.B. Lu; linux-mmc@vger.kernel.org; ulf.hansson@linaro.org; Arnd
> Bergmann
> Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> clk@vger.kernel.org; linux-i2c@vger.kernel.org; iommu@lists.linux-
> foundation.org; netdev@vger.kernel.org; Mark Rutland; Rob Herring;
> Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil; Bhupesh
> Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B. Xie
> Subject: Re: [v11, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
>
> On Tue, 2016-09-06 at 16:28 +0800, Yangbo Lu wrote:
> > The global utilities block controls power management, I/O device
> > enabling, power-onreset(POR) configuration monitoring, alternate
> > function selection for multiplexed signals,and clock control.
> >
> > This patch adds a driver to manage and access global utilities block.
> > Initially only reading SVR and registering soc device are supported.
> > Other guts accesses, such as reading RCW, should eventually be moved
> > into this driver as well.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > Signed-off-by: Scott Wood <oss@buserror.net>
>
> Don't put my signoff on patches that I didn't put it on
> myself. Definitely don't put mine *after* yours on patches that were
> last modified by you.
>
> If you want to mention that the soc_id encoding was my suggestion, then
> do so explicitly.
>
[Lu Yangbo-B47093] I found your 'signoff' on this patch at below link.
http://patchwork.ozlabs.org/patch/649211/
So, let me just change the order in next version ?
Signed-off-by: Scott Wood <oss@buserror.net>
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > +/* SoC attribute definition for QorIQ platform */ static const struct
> > +soc_device_attribute qoriq_soc[] = { #ifdef CONFIG_PPC
> > + /*
> > + * Power Architecture-based SoCs T Series
> > + */
> > +
> > + /* SoC: T1024/T1014/T1023/T1013 Rev: 1.0 */
> > + { .soc_id = "svr:0x85400010,name:T1024,die:T1024",
> > + .revision = "1.0",
> > + },
> > + { .soc_id = "svr:0x85480010,name:T1024E,die:T1024",
> > + .revision = "1.0",
> > + },
>
> Revision could be computed from the low 8 bits of SVR (just as you do for
> unknown SVRs).
>
[Lu Yangbo-B47093] Yes, you're right. Will remove it here.
> We could move the die name into .family:
>
> {
> .soc_id = "svr:0x85490010,name:T1023E,",
> .family = "QorIQ T1024",
> }
>
> I see you dropped svre (and the trailing comma), though I guess the vast
> majority of potential users will be looking at .family. In which case do
> we even need name? If we just make the soc_id be "svr:0xnnnnnnnn" then
> we could shrink the table to an svr+mask that identifies each die. I'd
> still want to keep the "svr:" even if we're giving up on the general
> tagging system, to make it clear what the number refers to, and to
> provide some defense against users who match only against soc_id rather
> than soc_id+family. Or we could go further and format soc_id as "QorIQ
> SVR 0xnnnnnnnn" so that soc_id-only matches are fully acceptable rather
> than just less dangerous.
[Lu Yangbo-B47093] It's a good idea to move die into .family I think.
In my opinion, it's better to keep svr and name in soc_id just like your suggestion above.
> {
> .soc_id = "svr:0x85490010,name:T1023E,",
> .family = "QorIQ T1024",
> }
The user probably don’t like to learn the svr value. What they want is just to match the soc they use.
It's convenient to use name+rev for them to match a soc.
Regarding shrinking the table, I think it's hard to use svr+mask. Because I find many platforms use different masks.
We couldn’t know the mask according svr value.
>
> > +static const struct soc_device_attribute *fsl_soc_device_match(
> > + unsigned int svr, const struct soc_device_attribute *matches) {
> > + char svr_match[50];
> > + int n;
> > +
> > + n = sprintf(svr_match, "*%08x*", svr);
>
> n = sprintf(svr_match, "svr:0x%08x,*", svr);
>
> (according to the current encoding)
>
[Lu Yangbo-B47093] Ok. Will do that.
> > +
> > + do {
> > + if (!matches->soc_id)
> > + return NULL;
> > + if (glob_match(svr_match, matches->soc_id))
> > + break;
> > + } while (matches++);
>
> Are you expecting "matches++" to ever evaluate as false?
[Lu Yangbo-B47093] Yes, this is used to match the soc we use in qoriq_soc array until getting true.
We need to get the name and die information defined in array.
>
> > + /* Register soc device */
> > + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> > + if (!soc_dev_attr) {
> > + ret = -ENOMEM;
> > + goto out_unmap;
> > + }
>
> Couldn't this be statically allocated?
[Lu Yangbo-B47093] Do you mean we define this struct statically ?
static struct soc_device_attribute soc_dev_attr;
>
> > +
> > + machine = of_flat_dt_get_machine_name();
> > + if (machine)
> > + soc_dev_attr->machine = kasprintf(GFP_KERNEL, "%s",
> > machine);
> > +
> > + soc_dev_attr->family = kasprintf(GFP_KERNEL, "QorIQ");
> > +
> > + svr = fsl_guts_get_svr();
> > + fsl_soc = fsl_soc_device_match(svr, qoriq_soc);
> > + if (fsl_soc) {
> > + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%s",
> > + fsl_soc->soc_id);
>
> You can use kstrdup() if you're just copying the string as is.
[Lu Yangbo-B47093] Ok. Will do that.
>
> > + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%s",
> > + fsl_soc->revision);
> > + } else {
> > + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "0x%08x",
> > svr);
>
> kasprintf(GFP_KERNEL, "svr:0x%08x,", svr);
[Lu Yangbo-B47093] Sorry, will add that.
>
>
> > +
> > + soc_dev = soc_device_register(soc_dev_attr);
> > + if (IS_ERR(soc_dev)) {
> > + ret = -ENODEV;
>
> Why are you changing the error code?
[Lu Yangbo-B47093] What error code should we use ? :)
>
> > + goto out;
> > + } else {
>
> Unnecessary "else".
[Lu Yangbo-B47093] Oh.. Correct!
>
> > + pr_info("Detected: %s\n", soc_dev_attr->machine);
>
> Machine: %s
[Lu Yangbo-B47093] Ok. Will do that.
>
> > + pr_info("Detected SoC family: %s\n", soc_dev_attr->family);
> > + pr_info("Detected SoC ID: %s, revision: %s\n",
> > + soc_dev_attr->soc_id, soc_dev_attr->revision);
>
> s/Detected //g
[Lu Yangbo-B47093] Ok, will do that.
>
>
> > + }
> > + return 0;
> > +out:
> > + kfree(soc_dev_attr->machine);
> > + kfree(soc_dev_attr->family);
> > + kfree(soc_dev_attr->soc_id);
> > + kfree(soc_dev_attr->revision);
> > + kfree(soc_dev_attr);
> > +out_unmap:
> > + iounmap(guts->regs);
> > +out_free:
> > + kfree(guts);
>
> devm
[Lu Yangbo-B47093] What's the devm meaning here :)
>
> > +static int fsl_guts_remove(struct platform_device *dev) {
> > + kfree(soc_dev_attr->machine);
> > + kfree(soc_dev_attr->family);
> > + kfree(soc_dev_attr->soc_id);
> > + kfree(soc_dev_attr->revision);
> > + kfree(soc_dev_attr);
> > + soc_device_unregister(soc_dev);
> > + iounmap(guts->regs);
> > + kfree(guts);
> > + return 0;
> > +}
>
> Don't free the memory before you unregister the device that uses it (moot
> if you use devm).
[Lu Yangbo-B47093] The soc.c driver mentions that.
Ensure soc_dev->attr is freed prior to calling soc_device_unregister.
>
> >
> > +#ifdef CONFIG_FSL_GUTS
> > +unsigned int fsl_guts_get_svr(void);
> > +#endif
>
> Don't ifdef prototypes (unless you're going to provide a stub
> alternative).
[Lu Yangbo-B47093] Ok, will remove ifdef.
>
> -Scott
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply
* Re: RESEND:i2c-eg20t: fix race between i2c init and interrupt enable
From: Yadi @ 2016-09-12 0:41 UTC (permalink / raw)
To: wsa, jdelvare; +Cc: linux-i2c
In-Reply-To: <1473240337-5694-1-git-send-email-yadi.hu@windriver.com>
ping.... any objection?
Yadi
On 2016年09月07日 17:25, Yadi Hu wrote:
> the eg20t driver call request_irq() function before the pch_base_address,
> base address of i2c controller's register, isassigned an effective value.
>
> it is one possible scenario that an interrupt which isn't inside eg20t
> arrives immediately after request_irq() is executed when i2c controller
> shares an interrupt number with others. since the interrupt handler
> pch_i2c_handler() has already active as shared action, it will be called
> and read its own register to determine if this interrupt is from itself.
>
> At that moment, since base address of i2c registers is not remapped
> in kernel space yet,so the INT handler will access an illegal address
> and then a error occurs.
>
> Bad IO access at port 0x18 (return inl(port))
> Call Trace:
> [<c102c733>] warn_slowpath_common+0x73/0xa0
> [<c13109f5>] ? bad_io_access+0x45/0x50
> [<c13109f5>] ? bad_io_access+0x45/0x50
> [<c102c804>] warn_slowpath_fmt+0x34/0x40
> [<c13109f5>] bad_io_access+0x45/0x50
> [<c1310b62>] ioread32+0x22/0x40
> [<c14c60db>] pch_i2c_handler+0x3b/0x120
> [<c1092f34>] handle_irq_event_percpu+0x64/0x330
> [<c109323b>] handle_irq_event+0x3b/0x60
> [<c1095b50>] ? unmask_irq+0x30/0x30
> [<c1095ba0>] handle_fasteoi_irq+0x50/0xe0
> <IRQ> [<c16e7e78>] ? do_IRQ+0x48/0xc0
> [<c16e7f6f>] ? smp_apic_timer_interrupt+0x7f/0x17a
> [<c16e7da9>] ? common_interrupt+0x29/0x30
> [<c1008ebb>] ? mwait_idle+0x8b/0x2c0
> [<c1009e8e>] ? cpu_idle+0x9e/0xc0
> [<c16be408>] ? rest_init+0x6c/0x74
> [<c19f770a>] ? start_kernel+0x311/0x318
> [<c19f7231>] ? repair_env_string+0x51/0x51
> [<c19f7078>] ? i386_start_kernel+0x78/0x7d
> ---[ end trace eb3a1028f468a140 ]---
> i2c_eg20t 0000:05:0c.2: pch_i2c_handler :I2C-3 mode(0) is not supported
>
> after testing last patch, replacing request_irq() location is not a
> good idea,it might produce a little time windows ,in which the interrupts
> occurring inside will be omitted.
>
> the new patch adds a check point on interrupt handler in case field
> 'pch_base_address' has not been initialed.
>
>
> Yadi Hu (1)
> i2c-eg20t: fix race between i2c init and interrupt enable
>
> drivers/i2c/busses/i2c-eg20t.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
^ permalink raw reply
* Re: [PATCH] Drivers: i2c: busses: Use max macro instead of ternary operator
From: Bhumika Goyal @ 2016-09-10 19:24 UTC (permalink / raw)
To: Joe Perches; +Cc: wsa, linux-i2c, linux-kernel
In-Reply-To: <1473535309.19464.5.camel@perches.com>
On Sun, Sep 11, 2016 at 12:51 AM, Joe Perches <joe@perches.com> wrote:
> On Sun, 2016-09-11 at 00:38 +0530, Bhumika Goyal wrote:
>> Replace ternary operators with macro 'max' as it is shorter
>> and thus increases code readability. Macro max returns
>> the maximum of the two compared values.
>> Done using coccinelle:
>>
>> @@
>> type T;
>> T x;
>> T y;
>> @@
>> (
>> - x < y ? x : y
>> + min(x,y)
>> - x > y ? x : y
>> + max(x,y)
>> )
>
> Hello.
>
> Please make sure to inspect and if appropriate modify the coccinelle
> output for the 'best' coding style, for whatever definition of 'best'
> you might use, before submitting automatically produced patches.
>
>> diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
> []
>> @@ -133,10 +133,10 @@ static const char * const jz4780_i2c_abrt_src[] = {
>>
>> #define JZ4780_I2C_ENB_I2C BIT(0)
>>
>> -#define JZ4780_I2CSHCNT_ADJUST(n) (((n) - 8) < 6 ? 6 : ((n) - 8))
>> -#define JZ4780_I2CSLCNT_ADJUST(n) (((n) - 1) < 8 ? 8 : ((n) - 1))
>> -#define JZ4780_I2CFHCNT_ADJUST(n) (((n) - 8) < 6 ? 6 : ((n) - 8))
>> -#define JZ4780_I2CFLCNT_ADJUST(n) (((n) - 1) < 8 ? 8 : ((n) - 1))
>> +#define JZ4780_I2CSHCNT_ADJUST(n) (max(6, (n) - 8))
>> +#define JZ4780_I2CSLCNT_ADJUST(n) (max(8, (n) - 1))
>> +#define JZ4780_I2CFHCNT_ADJUST(n) (max(6, (n) - 8))
>> +#define JZ4780_I2CFLCNT_ADJUST(n) (max(8, (n) - 1))
>
> For instance: the parentheses are unnecessary and could be removed
> like the BIT macro above.
Thanks for pointing it out. I will resend the patch.
Thanks,
Bhumika
>>
^ permalink raw reply
* Re: [PATCH] Drivers: i2c: busses: Use max macro instead of ternary operator
From: Joe Perches @ 2016-09-10 19:21 UTC (permalink / raw)
To: Bhumika Goyal, wsa, linux-i2c, linux-kernel
In-Reply-To: <1473534490-16592-1-git-send-email-bhumirks@gmail.com>
On Sun, 2016-09-11 at 00:38 +0530, Bhumika Goyal wrote:
> Replace ternary operators with macro 'max' as it is shorter
> and thus increases code readability. Macro max returns
> the maximum of the two compared values.
> Done using coccinelle:
>
> @@
> type T;
> T x;
> T y;
> @@
> (
> - x < y ? x : y
> + min(x,y)
> - x > y ? x : y
> + max(x,y)
> )
Hello.
Please make sure to inspect and if appropriate modify the coccinelle
output for the 'best' coding style, for whatever definition of 'best'
you might use, before submitting automatically produced patches.
> diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
[]
> @@ -133,10 +133,10 @@ static const char * const jz4780_i2c_abrt_src[] = {
>
> #define JZ4780_I2C_ENB_I2C BIT(0)
>
> -#define JZ4780_I2CSHCNT_ADJUST(n) (((n) - 8) < 6 ? 6 : ((n) - 8))
> -#define JZ4780_I2CSLCNT_ADJUST(n) (((n) - 1) < 8 ? 8 : ((n) - 1))
> -#define JZ4780_I2CFHCNT_ADJUST(n) (((n) - 8) < 6 ? 6 : ((n) - 8))
> -#define JZ4780_I2CFLCNT_ADJUST(n) (((n) - 1) < 8 ? 8 : ((n) - 1))
> +#define JZ4780_I2CSHCNT_ADJUST(n) (max(6, (n) - 8))
> +#define JZ4780_I2CSLCNT_ADJUST(n) (max(8, (n) - 1))
> +#define JZ4780_I2CFHCNT_ADJUST(n) (max(6, (n) - 8))
> +#define JZ4780_I2CFLCNT_ADJUST(n) (max(8, (n) - 1))
For instance: the parentheses are unnecessary and could be removed
like the BIT macro above.
>
^ permalink raw reply
* [PATCH] Drivers: i2c: busses: Use max macro instead of ternary operator
From: Bhumika Goyal @ 2016-09-10 19:08 UTC (permalink / raw)
To: wsa, linux-i2c, linux-kernel; +Cc: Bhumika Goyal
Replace ternary operators with macro 'max' as it is shorter
and thus increases code readability. Macro max returns
the maximum of the two compared values.
Done using coccinelle:
@@
type T;
T x;
T y;
@@
(
- x < y ? x : y
+ min(x,y)
|
- x > y ? x : y
+ max(x,y)
)
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/i2c/busses/i2c-jz4780.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
index cd98725..b6c6f90 100644
--- a/drivers/i2c/busses/i2c-jz4780.c
+++ b/drivers/i2c/busses/i2c-jz4780.c
@@ -133,10 +133,10 @@ static const char * const jz4780_i2c_abrt_src[] = {
#define JZ4780_I2C_ENB_I2C BIT(0)
-#define JZ4780_I2CSHCNT_ADJUST(n) (((n) - 8) < 6 ? 6 : ((n) - 8))
-#define JZ4780_I2CSLCNT_ADJUST(n) (((n) - 1) < 8 ? 8 : ((n) - 1))
-#define JZ4780_I2CFHCNT_ADJUST(n) (((n) - 8) < 6 ? 6 : ((n) - 8))
-#define JZ4780_I2CFLCNT_ADJUST(n) (((n) - 1) < 8 ? 8 : ((n) - 1))
+#define JZ4780_I2CSHCNT_ADJUST(n) (max(6, (n) - 8))
+#define JZ4780_I2CSLCNT_ADJUST(n) (max(8, (n) - 1))
+#define JZ4780_I2CFHCNT_ADJUST(n) (max(6, (n) - 8))
+#define JZ4780_I2CFLCNT_ADJUST(n) (max(8, (n) - 1))
#define JZ4780_I2C_FIFO_LEN 16
#define TX_LEVEL 3
--
1.9.1
^ permalink raw reply related
* [PATCH] Drivers: i2c: busses: Use max macro instead of ternary operator
From: Bhumika Goyal @ 2016-09-10 18:56 UTC (permalink / raw)
To: wsa, linux-i2c; +Cc: Bhumika Goyal
Replace ternary operators with macro 'max' as it is shorter
and thus increases code readability. Macro max returns
the maximum of the two compared values.
Done using coccinelle:
@@
type T;
T x;
T y;
@@
(
- x < y ? x : y
+ min(x,y)
|
- x > y ? x : y
+ max(x,y)
)
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/i2c/busses/i2c-jz4780.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-jz4780.c b/drivers/i2c/busses/i2c-jz4780.c
index cd98725..b6c6f90 100644
--- a/drivers/i2c/busses/i2c-jz4780.c
+++ b/drivers/i2c/busses/i2c-jz4780.c
@@ -133,10 +133,10 @@ static const char * const jz4780_i2c_abrt_src[] = {
#define JZ4780_I2C_ENB_I2C BIT(0)
-#define JZ4780_I2CSHCNT_ADJUST(n) (((n) - 8) < 6 ? 6 : ((n) - 8))
-#define JZ4780_I2CSLCNT_ADJUST(n) (((n) - 1) < 8 ? 8 : ((n) - 1))
-#define JZ4780_I2CFHCNT_ADJUST(n) (((n) - 8) < 6 ? 6 : ((n) - 8))
-#define JZ4780_I2CFLCNT_ADJUST(n) (((n) - 1) < 8 ? 8 : ((n) - 1))
+#define JZ4780_I2CSHCNT_ADJUST(n) (max(6, (n) - 8))
+#define JZ4780_I2CSLCNT_ADJUST(n) (max(8, (n) - 1))
+#define JZ4780_I2CFHCNT_ADJUST(n) (max(6, (n) - 8))
+#define JZ4780_I2CFLCNT_ADJUST(n) (max(8, (n) - 1))
#define JZ4780_I2C_FIFO_LEN 16
#define TX_LEVEL 3
--
1.9.1
^ permalink raw reply related
* [PULL REQUEST] i2c for 4.8
From: Wolfram Sang @ 2016-09-10 14:07 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]
Linus,
here is a pull request from I2C. Mostly driver bugfixes, but also a few
cleanups which are nice to have out of the way. Please pull.
Thanks,
Wolfram
The following changes since commit fa8410b355251fd30341662a40ac6b22d3e38468:
Linux 4.8-rc3 (2016-08-21 16:14:10 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current
for you to fetch changes up to cbfff439c54f37fc363b1d365183fa61af43585c:
i2c: rk3x: Restore clock settings at resume time (2016-09-08 22:50:33 +0200)
----------------------------------------------------------------
Doug Anderson (1):
i2c: rk3x: Restore clock settings at resume time
Geert Uytterhoeven (1):
i2c: Spelling s/acknowedge/acknowledge/
Wolfram Sang (6):
i2c: mux: demux-pinctrl: invalidate properly when switching fails
i2c: sh_mobile: use proper device with dma_mapping_error
i2c: rcar: use proper device with dma_mapping_error
i2c: bcm-kona: fix inconsistent indenting
i2c: mux: demux-pinctrl: run properly with multiple instances
Documentation: i2c: slave-interface: add note for driver development
Zhuo-hao Lee (1):
i2c: designware: save the preset value of DW_IC_SDA_HOLD
Documentation/i2c/slave-interface | 5 +++++
drivers/i2c/busses/i2c-bcm-kona.c | 2 +-
drivers/i2c/busses/i2c-cadence.c | 2 +-
drivers/i2c/busses/i2c-designware-core.c | 16 ++++++++++------
drivers/i2c/busses/i2c-rcar.c | 2 +-
drivers/i2c/busses/i2c-rk3x.c | 14 +++++++++++++-
drivers/i2c/busses/i2c-sh_mobile.c | 2 +-
drivers/i2c/muxes/i2c-demux-pinctrl.c | 15 +++++++++++----
8 files changed, 43 insertions(+), 15 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v2 2/2] i2c: aspeed: added documentation for Aspeed I2C driver
From: Brendan Higgins @ 2016-09-10 1:55 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland
Cc: linux-i2c, devicetree, linux-kernel, openbmc, joel, jk,
Brendan Higgins
In-Reply-To: <1473472551-11149-1-git-send-email-brendanhiggins@google.com>
Added device tree binding documentation for Aspeed I2C controller and
busses.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Changes for v2:
- None
---
.../devicetree/bindings/i2c/i2c-aspeed.txt | 63 ++++++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
new file mode 100644
index 0000000..df68f2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -0,0 +1,63 @@
+Device tree configuration for the I2C controller and busses on the AST24XX
+and AST25XX SoCs.
+
+Controller:
+
+ Required Properties:
+ - #address-cells : should be 1
+ - #size-cells : should be 1
+ - #interrupt-cells : should be 1
+ - compatible : should be "aspeed,ast2400-i2c-controller"
+ or "aspeed,ast2500-i2c-controller"
+ - reg : address start and range of controller
+ - ranges : defines address offset and range for busses
+ - interrupts : interrupt number
+ - clocks : root clock of bus, should reference the APB
+ clock
+ - clock-ranges : specifies that child busses can inherit clocks
+ - interrupt-controller : denotes that the controller receives and fires
+ new interrupts for child busses
+
+Bus:
+
+ Required Properties:
+ - #address-cells : should be 1
+ - #size-cells : should be 0
+ - reg : address offset and range of bus
+ - compatible : should be "aspeed,ast2400-i2c-bus"
+ or "aspeed,ast2500-i2c-bus"
+ - bus : the bus's number
+ - interrupts : interrupt number
+
+ Optional Properties:
+ - clock-frequency : frequency of the bus clock in Hz
+ defaults to 100 kHz when not specified
+
+Example:
+
+i2c: i2c@1e78a000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ #interrupt-cells = <1>;
+
+ compatible = "aspeed,ast2400-i2c-controller";
+ reg = <0x1e78a000 0x40>;
+ ranges = <0 0x1e78a000 0x1000>;
+ interrupts = <12>;
+ clocks = <&clk_apb>;
+ clock-ranges;
+ interrupt-controller;
+
+ i2c0: i2c-bus@40 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x40 0x40>;
+ compatible = "aspeed,ast2400-i2c-bus";
+ bus = <0>;
+ clock-frequency = <100000>;
+ status = "disabled";
+ interrupts = <0>;
+ interrupt-parent = <&i2c>;
+ };
+};
+
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v2 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2016-09-10 1:55 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, joel-U3u1mxZcP9KHXe+LvDLADg,
jk-mnsaURCQ41sdnm+yROfE0A, Brendan Higgins
In-Reply-To: <201609100847.j1qUadzy%fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Added initial master and slave support for Aspeed I2C controller.
Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
Aspeed.
Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
Changes for v2:
- Added single module_init (multiple was breaking some builds).
---
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-aspeed.c | 831 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 842 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-aspeed.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 5c3993b..0178c6c 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -998,6 +998,16 @@ config I2C_RCAR
This driver can also be built as a module. If so, the module
will be called i2c-rcar.
+config I2C_ASPEED
+ tristate "Aspeed AST2xxx SoC I2C Controller"
+ depends on (ARCH_ASPEED || COMPILE_TEST) && OF
+ help
+ If you say yes to this option, support will be included for the
+ Aspeed AST2xxx SoC I2C controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-aspeed.
+
comment "External I2C/SMBus adapter drivers"
config I2C_DIOLAN_U2C
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 37f2819..49631cd 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -96,6 +96,7 @@ obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
obj-$(CONFIG_I2C_XLR) += i2c-xlr.o
obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
+obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
# External I2C/SMBus adapter drivers
obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
new file mode 100644
index 0000000..b19f13c
--- /dev/null
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -0,0 +1,831 @@
+/*
+ * I2C adapter for the ASPEED I2C bus.
+ *
+ * Copyright (C) 2012-2020 ASPEED Technology Inc.
+ * Copyright 2016 IBM Corporation
+ * Copyright 2016 Google, Inc.
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/slab.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+
+/* I2C Register */
+#define ASPEED_I2C_FUN_CTRL_REG 0x00
+#define ASPEED_I2C_AC_TIMING_REG1 0x04
+#define ASPEED_I2C_AC_TIMING_REG2 0x08
+#define ASPEED_I2C_INTR_CTRL_REG 0x0c
+#define ASPEED_I2C_INTR_STS_REG 0x10
+#define ASPEED_I2C_CMD_REG 0x14
+#define ASPEED_I2C_DEV_ADDR_REG 0x18
+#define ASPEED_I2C_BYTE_BUF_REG 0x20
+
+#define ASPEED_I2C_NUM_BUS 14
+
+/* Global Register Definition */
+/* 0x00 : I2C Interrupt Status Register */
+/* 0x08 : I2C Interrupt Target Assignment */
+
+/* Device Register Definition */
+/* 0x00 : I2CD Function Control Register */
+#define ASPEED_I2CD_MULTI_MASTER_DIS BIT(15)
+#define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
+#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
+#define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
+#define ASPEED_I2CD_SLAVE_EN BIT(1)
+#define ASPEED_I2CD_MASTER_EN BIT(0)
+
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define ASPEED_NO_TIMEOUT_CTRL 0
+
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
+#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
+#define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
+#define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
+#define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
+#define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
+#define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
+#define ASPEED_I2CD_INTR_RX_DONE BIT(2)
+#define ASPEED_I2CD_INTR_TX_NAK BIT(1)
+#define ASPEED_I2CD_INTR_TX_ACK BIT(0)
+
+/* 0x14 : I2CD Command/Status Register */
+#define ASPEED_I2CD_SCL_LINE_STS BIT(18)
+#define ASPEED_I2CD_SDA_LINE_STS BIT(17)
+#define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
+#define ASPEED_I2CD_BUS_RECOVER_CMD BIT(11)
+
+/* Command Bit */
+#define ASPEED_I2CD_M_STOP_CMD BIT(5)
+#define ASPEED_I2CD_M_S_RX_CMD_LAST BIT(4)
+#define ASPEED_I2CD_M_RX_CMD BIT(3)
+#define ASPEED_I2CD_S_TX_CMD BIT(2)
+#define ASPEED_I2CD_M_TX_CMD BIT(1)
+#define ASPEED_I2CD_M_START_CMD BIT(0)
+
+/* 0x18 : I2CD Slave Device Address Register */
+#define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
+
+enum aspeed_i2c_slave_state {
+ ASPEED_I2C_SLAVE_START,
+ ASPEED_I2C_SLAVE_READ_REQUESTED,
+ ASPEED_I2C_SLAVE_READ_PROCESSED,
+ ASPEED_I2C_SLAVE_WRITE_REQUESTED,
+ ASPEED_I2C_SLAVE_WRITE_RECEIVED,
+ ASPEED_I2C_SLAVE_STOP,
+};
+
+struct aspeed_i2c_bus {
+ struct i2c_adapter adap;
+ struct device *dev;
+ void __iomem *base;
+ spinlock_t lock;
+ struct completion cmd_complete;
+ int irq;
+ /* Transaction state. */
+ struct i2c_msg *msg;
+ int msg_pos;
+ u32 cmd_err;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ struct i2c_client *slave;
+ enum aspeed_i2c_slave_state slave_state;
+#endif
+};
+
+struct aspeed_i2c_controller {
+ struct device *dev;
+ void __iomem *base;
+ int irq;
+ struct irq_domain *irq_domain;
+};
+
+static inline void aspeed_i2c_write(struct aspeed_i2c_bus *bus, u32 val,
+ u32 reg)
+{
+ writel(val, bus->base + reg);
+}
+
+static inline u32 aspeed_i2c_read(struct aspeed_i2c_bus *bus, u32 reg)
+{
+ return readl(bus->base + reg);
+}
+
+static u8 aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
+{
+ u32 command;
+ unsigned long time_left;
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
+ /* Bus is idle: no recovery needed. */
+ if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
+ (command & ASPEED_I2CD_SCL_LINE_STS))
+ goto out;
+
+ dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
+ command);
+
+ /* Bus held: put bus in stop state. */
+ if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
+ !(command & ASPEED_I2CD_SCL_LINE_STS)) {
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
+ ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete, bus->adap.timeout * HZ);
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (time_left == 0)
+ ret = -ETIMEDOUT;
+ else if (bus->cmd_err)
+ ret = -EIO;
+ /* Bus error. */
+ } else if (!(command & ASPEED_I2CD_SDA_LINE_STS)) {
+ aspeed_i2c_write(bus, ASPEED_I2CD_BUS_RECOVER_CMD,
+ ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete, bus->adap.timeout * HZ);
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (time_left == 0)
+ ret = -ETIMEDOUT;
+ else if (bus->cmd_err)
+ ret = -EIO;
+ /* Recovery failed. */
+ else if (!(aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
+ ASPEED_I2CD_SDA_LINE_STS))
+ ret = -EIO;
+ }
+
+out:
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return ret;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
+{
+ bool irq_handled = true;
+ u32 command;
+ u32 irq_status;
+ u32 status_ack = 0;
+ u8 value;
+ struct i2c_client *slave = bus->slave;
+
+ spin_lock(&bus->lock);
+ if (!slave) {
+ irq_handled = false;
+ goto out;
+ }
+ command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
+ irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
+
+ /* Slave was requested, restart state machine. */
+ if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
+ status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
+ bus->slave_state = ASPEED_I2C_SLAVE_START;
+ }
+ /* Slave is not currently active, irq was for someone else. */
+ if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+ irq_handled = false;
+ goto out;
+ }
+
+ dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
+ irq_status, command);
+
+ /* Slave was sent something. */
+ if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
+ value = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
+ /* Handle address frame. */
+ if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
+ if (value & 0x1)
+ bus->slave_state =
+ ASPEED_I2C_SLAVE_READ_REQUESTED;
+ else
+ bus->slave_state =
+ ASPEED_I2C_SLAVE_WRITE_REQUESTED;
+ }
+ status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+ }
+
+ /* Slave was asked to stop. */
+ if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+ status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ }
+ if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
+ status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ }
+
+ if (bus->slave_state == ASPEED_I2C_SLAVE_READ_REQUESTED) {
+ if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
+ dev_err(bus->dev, "Unexpected ACK on read request.\n");
+ bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
+
+ i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
+ aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
+ aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
+ } else if (bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
+ status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+ if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
+ dev_err(bus->dev,
+ "Expected ACK after processed read.\n");
+ i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
+ aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
+ aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
+ } else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_REQUESTED) {
+ bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
+ i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+ } else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED) {
+ i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+ } else if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+ i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
+ }
+
+ if (status_ack != irq_status)
+ dev_err(bus->dev,
+ "irq handled != irq. expected %x, but was %x\n",
+ irq_status, status_ack);
+ aspeed_i2c_write(bus, status_ack, ASPEED_I2C_INTR_STS_REG);
+
+out:
+ spin_unlock(&bus->lock);
+ return irq_handled;
+}
+#endif
+
+static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
+{
+ const u32 errs = ASPEED_I2CD_INTR_ARBIT_LOSS |
+ ASPEED_I2CD_INTR_ABNORMAL |
+ ASPEED_I2CD_INTR_SCL_TIMEOUT |
+ ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
+ ASPEED_I2CD_INTR_TX_NAK;
+ u32 irq_status;
+
+ spin_lock(&bus->lock);
+ irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
+ bus->cmd_err = irq_status & errs;
+
+ dev_dbg(bus->dev, "master irq status 0x%08x\n", irq_status);
+
+ /* No message to transfer. */
+ if (bus->cmd_err ||
+ (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) ||
+ (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE)) {
+ complete(&bus->cmd_complete);
+ goto out;
+ } else if (!bus->msg || bus->msg_pos >= bus->msg->len)
+ goto out;
+
+ if ((bus->msg->flags & I2C_M_RD) &&
+ (irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
+ bus->msg->buf[bus->msg_pos++] = aspeed_i2c_read(
+ bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
+ if (bus->msg_pos + 1 < bus->msg->len)
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD,
+ ASPEED_I2C_CMD_REG);
+ else if (bus->msg_pos < bus->msg->len)
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD |
+ ASPEED_I2CD_M_S_RX_CMD_LAST,
+ ASPEED_I2C_CMD_REG);
+ } else if (!(bus->msg->flags & I2C_M_RD) &&
+ (irq_status & ASPEED_I2CD_INTR_TX_ACK)) {
+ aspeed_i2c_write(bus, bus->msg->buf[bus->msg_pos++],
+ ASPEED_I2C_BYTE_BUF_REG);
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_TX_CMD, ASPEED_I2C_CMD_REG);
+ }
+
+ /* Transmission complete: notify caller. */
+ if (bus->msg_pos >= bus->msg->len)
+ complete(&bus->cmd_complete);
+out:
+ aspeed_i2c_write(bus, irq_status, ASPEED_I2C_INTR_STS_REG);
+ spin_unlock(&bus->lock);
+ return true;
+}
+
+static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
+{
+ struct aspeed_i2c_bus *bus = dev_id;
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ if (aspeed_i2c_slave_irq(bus)) {
+ dev_dbg(bus->dev, "irq handled by slave.\n");
+ return IRQ_HANDLED;
+ }
+#endif
+ if (aspeed_i2c_master_irq(bus)) {
+ dev_dbg(bus->dev, "irq handled by master.\n");
+ return IRQ_HANDLED;
+ }
+ dev_err(bus->dev, "irq not handled properly!\n");
+ return IRQ_HANDLED;
+}
+
+static int aspeed_i2c_master_single_xfer(struct i2c_adapter *adap,
+ struct i2c_msg *msg)
+{
+ struct aspeed_i2c_bus *bus = adap->algo_data;
+ unsigned long flags;
+ u8 slave_addr;
+ u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
+ int ret = msg->len;
+ unsigned long time_left;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ bus->msg = msg;
+ bus->msg_pos = 0;
+ slave_addr = msg->addr << 1;
+ if (msg->flags & I2C_M_RD) {
+ slave_addr |= 1;
+ command |= ASPEED_I2CD_M_RX_CMD;
+ if (msg->len == 1)
+ command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+ }
+ aspeed_i2c_write(bus, slave_addr, ASPEED_I2C_BYTE_BUF_REG);
+ aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete, bus->adap.timeout * HZ * msg->len);
+ if (time_left == 0)
+ return -ETIMEDOUT;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (bus->cmd_err)
+ ret = -EIO;
+ bus->msg = NULL;
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ return ret;
+}
+
+static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
+ struct i2c_msg *msgs, int num)
+{
+ struct aspeed_i2c_bus *bus = adap->algo_data;
+ int ret;
+ int i;
+ unsigned long flags;
+ unsigned long time_left;
+
+ /* If bus is busy, attempt recovery. We assume a single master
+ * environment.
+ */
+ if (aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
+ ASPEED_I2CD_BUS_BUSY_STS) {
+ ret = aspeed_i2c_recover_bus(bus);
+ if (ret)
+ return ret;
+ }
+
+ for (i = 0; i < num; i++) {
+ ret = aspeed_i2c_master_single_xfer(adap, &msgs[i]);
+ if (ret < 0)
+ break;
+ /* TODO: Support other forms of I2C protocol mangling. */
+ if (msgs[i].flags & I2C_M_STOP) {
+ spin_lock_irqsave(&bus->lock, flags);
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
+ ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete,
+ bus->adap.timeout * HZ);
+ if (time_left == 0)
+ return -ETIMEDOUT;
+ }
+ }
+
+ spin_lock_irqsave(&bus->lock, flags);
+ aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD, ASPEED_I2C_CMD_REG);
+ reinit_completion(&bus->cmd_complete);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_interruptible_timeout(
+ &bus->cmd_complete, bus->adap.timeout * HZ);
+ if (time_left == 0)
+ return -ETIMEDOUT;
+
+ /* If nothing went wrong, return number of messages transferred. */
+ if (ret < 0)
+ return ret;
+ else
+ return i;
+}
+
+static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static int aspeed_i2c_reg_slave(struct i2c_client *client)
+{
+ struct aspeed_i2c_bus *bus;
+ unsigned long flags;
+ u32 addr_reg_val;
+ u32 func_ctrl_reg_val;
+
+ bus = client->adapter->algo_data;
+ spin_lock_irqsave(&bus->lock, flags);
+ if (bus->slave) {
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return -EINVAL;
+ }
+
+ /* Set slave addr. */
+ addr_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_DEV_ADDR_REG);
+ addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
+ addr_reg_val |= client->addr & ASPEED_I2CD_DEV_ADDR_MASK;
+ aspeed_i2c_write(bus, addr_reg_val, ASPEED_I2C_DEV_ADDR_REG);
+
+ /* Switch from master mode to slave mode. */
+ func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
+ func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
+ func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
+ aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
+
+ bus->slave = client;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return 0;
+}
+
+static int aspeed_i2c_unreg_slave(struct i2c_client *client)
+{
+ struct aspeed_i2c_bus *bus = client->adapter->algo_data;
+ unsigned long flags;
+ u32 func_ctrl_reg_val;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (!bus->slave) {
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return -EINVAL;
+ }
+
+ /* Switch from slave mode to master mode. */
+ func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
+ func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
+ func_ctrl_reg_val |= ASPEED_I2CD_MASTER_EN;
+ aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
+
+ bus->slave = NULL;
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return 0;
+}
+#endif
+
+static const struct i2c_algorithm aspeed_i2c_algo = {
+ .master_xfer = aspeed_i2c_master_xfer,
+ .functionality = aspeed_i2c_functionality,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ .reg_slave = aspeed_i2c_reg_slave,
+ .unreg_slave = aspeed_i2c_unreg_slave,
+#endif
+};
+
+static u32 aspeed_i2c_get_clk_reg_val(u32 divider_ratio)
+{
+ unsigned int inc = 0, div;
+ u32 scl_low, scl_high, data;
+
+ for (div = 0; divider_ratio >= 16; div++) {
+ inc |= (divider_ratio & 1);
+ divider_ratio >>= 1;
+ }
+ divider_ratio += inc;
+ scl_low = (divider_ratio >> 1) - 1;
+ scl_high = divider_ratio - scl_low - 2;
+ data = 0x77700300 | (scl_high << 16) | (scl_low << 12) | div;
+ return data;
+}
+
+static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
+ struct platform_device *pdev)
+{
+ struct clk *pclk;
+ u32 clk_freq;
+ u32 divider_ratio;
+ int ret;
+
+ pclk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(pclk)) {
+ dev_err(&pdev->dev, "clk_get failed\n");
+ return PTR_ERR(pclk);
+ }
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "clock-frequency", &clk_freq);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "Could not read clock-frequency property\n");
+ clk_freq = 100000;
+ }
+ divider_ratio = clk_get_rate(pclk) / clk_freq;
+ /* We just need the clock rate, we don't actually use the clk object. */
+ devm_clk_put(&pdev->dev, pclk);
+
+ /* Set AC Timing */
+ if (clk_freq / 1000 > 400) {
+ aspeed_i2c_write(bus, aspeed_i2c_read(bus,
+ ASPEED_I2C_FUN_CTRL_REG) |
+ ASPEED_I2CD_M_HIGH_SPEED_EN |
+ ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
+ ASPEED_I2CD_SDA_DRIVE_1T_EN,
+ ASPEED_I2C_FUN_CTRL_REG);
+
+ aspeed_i2c_write(bus, 0x3, ASPEED_I2C_AC_TIMING_REG2);
+ aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
+ ASPEED_I2C_AC_TIMING_REG1);
+ } else {
+ aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
+ ASPEED_I2C_AC_TIMING_REG1);
+ aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
+ ASPEED_I2C_AC_TIMING_REG2);
+ }
+
+ return 0;
+}
+
+static void noop(struct irq_data *data) { }
+
+static struct irq_chip aspeed_i2c_irqchip = {
+ .name = "ast-i2c",
+ .irq_unmask = noop,
+ .irq_mask = noop,
+};
+
+static int aspeed_i2c_probe_bus(struct platform_device *pdev)
+{
+ struct aspeed_i2c_bus *bus;
+ struct aspeed_i2c_controller *controller =
+ dev_get_drvdata(pdev->dev.parent);
+ struct resource *res;
+ int ret, bus_num, irq;
+
+ bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
+ if (!bus)
+ return -ENOMEM;
+
+ ret = of_property_read_u32(pdev->dev.of_node, "bus", &bus_num);
+ if (ret)
+ return -ENXIO;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ bus->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(bus->base))
+ return PTR_ERR(bus->base);
+
+ bus->irq = platform_get_irq(pdev, 0);
+ if (bus->irq < 0) {
+ dev_err(&pdev->dev, "platform_get_irq failed\n");
+ return -ENXIO;
+ }
+
+ irq = irq_create_mapping(controller->irq_domain, bus_num);
+ irq_set_chip_data(irq, controller);
+ irq_set_chip_and_handler(irq, &aspeed_i2c_irqchip, handle_simple_irq);
+ ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
+ 0, dev_name(&pdev->dev), bus);
+ if (ret) {
+ dev_err(&pdev->dev, "devm_request_irq failed\n");
+ return -ENXIO;
+ }
+
+ /* Initialize the I2C adapter */
+ spin_lock_init(&bus->lock);
+ init_completion(&bus->cmd_complete);
+ bus->adap.nr = bus_num;
+ bus->adap.owner = THIS_MODULE;
+ bus->adap.retries = 0;
+ bus->adap.timeout = 5;
+ bus->adap.algo = &aspeed_i2c_algo;
+ bus->adap.algo_data = bus;
+ bus->adap.dev.parent = &pdev->dev;
+ bus->adap.dev.of_node = pdev->dev.of_node;
+ snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c-%d",
+ bus_num);
+
+ bus->dev = &pdev->dev;
+
+ /* reset device: disable master & slave functions */
+ aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
+
+ ret = aspeed_i2c_init_clk(bus, pdev);
+ if (ret < 0)
+ return ret;
+
+ /* Enable Master Mode */
+ aspeed_i2c_write(bus, aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG) |
+ ASPEED_I2CD_MASTER_EN |
+ ASPEED_I2CD_MULTI_MASTER_DIS, ASPEED_I2C_FUN_CTRL_REG);
+
+ /* Set interrupt generation of I2C controller */
+ aspeed_i2c_write(bus, ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
+ ASPEED_I2CD_INTR_BUS_RECOVER_DONE |
+ ASPEED_I2CD_INTR_SCL_TIMEOUT |
+ ASPEED_I2CD_INTR_ABNORMAL |
+ ASPEED_I2CD_INTR_NORMAL_STOP |
+ ASPEED_I2CD_INTR_ARBIT_LOSS |
+ ASPEED_I2CD_INTR_RX_DONE |
+ ASPEED_I2CD_INTR_TX_NAK |
+ ASPEED_I2CD_INTR_TX_ACK,
+ ASPEED_I2C_INTR_CTRL_REG);
+
+ ret = i2c_add_numbered_adapter(&bus->adap);
+ if (ret < 0)
+ return -ENXIO;
+
+ platform_set_drvdata(pdev, bus);
+
+ dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
+ bus->adap.nr, bus->irq);
+
+ return 0;
+}
+
+static int aspeed_i2c_remove_bus(struct platform_device *pdev)
+{
+ struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&bus->adap);
+ return 0;
+}
+
+static const struct of_device_id aspeed_i2c_bus_of_table[] = {
+ { .compatible = "aspeed,ast2400-i2c-bus", },
+ { .compatible = "aspeed,ast2500-i2c-bus", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
+
+static struct platform_driver aspeed_i2c_bus_driver = {
+ .probe = aspeed_i2c_probe_bus,
+ .remove = aspeed_i2c_remove_bus,
+ .driver = {
+ .name = "ast-i2c-bus",
+ .of_match_table = aspeed_i2c_bus_of_table,
+ },
+};
+
+static void aspeed_i2c_controller_irq(struct irq_desc *desc)
+{
+ struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
+ unsigned long p, status;
+ unsigned int bus_irq;
+
+ status = readl(c->base);
+ for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
+ bus_irq = irq_find_mapping(c->irq_domain, p);
+ generic_handle_irq(bus_irq);
+ }
+}
+
+static int aspeed_i2c_probe_controller(struct platform_device *pdev)
+{
+ struct aspeed_i2c_controller *controller;
+ struct device_node *np;
+ struct resource *res;
+
+ controller = kzalloc(sizeof(*controller), GFP_KERNEL);
+ if (!controller)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ controller->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(controller->base))
+ return PTR_ERR(controller->base);
+
+ controller->irq = platform_get_irq(pdev, 0);
+ if (controller->irq < 0) {
+ dev_err(&pdev->dev, "no platform IRQ\n");
+ return -ENXIO;
+ }
+
+ controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
+ ASPEED_I2C_NUM_BUS, &irq_domain_simple_ops, NULL);
+ if (!controller->irq_domain) {
+ dev_err(&pdev->dev, "no IRQ domain\n");
+ return -ENXIO;
+ }
+ controller->irq_domain->name = "ast-i2c-domain";
+
+ irq_set_chained_handler_and_data(controller->irq,
+ aspeed_i2c_controller_irq, controller);
+
+ controller->dev = &pdev->dev;
+
+ platform_set_drvdata(pdev, controller);
+
+ dev_info(controller->dev, "i2c controller registered, irq %d\n",
+ controller->irq);
+
+ for_each_child_of_node(pdev->dev.of_node, np) {
+ int ret;
+ u32 bus_num;
+ char bus_id[sizeof("i2c-12345")];
+
+ /*
+ * Set a useful name derived from the bus number; the device
+ * tree should provide us with one that corresponds to the
+ * hardware numbering. If the property is missing the
+ * probe would fail so just skip it here.
+ */
+
+ ret = of_property_read_u32(np, "bus", &bus_num);
+ if (ret)
+ continue;
+
+ ret = snprintf(bus_id, sizeof(bus_id), "i2c-%u", bus_num);
+ if (ret >= sizeof(bus_id))
+ continue;
+
+ of_platform_device_create(np, bus_id, &pdev->dev);
+ of_node_put(np);
+ }
+
+ return 0;
+}
+
+static int aspeed_i2c_remove_controller(struct platform_device *pdev)
+{
+ struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
+
+ irq_domain_remove(controller->irq_domain);
+ return 0;
+}
+
+static const struct of_device_id aspeed_i2c_controller_of_table[] = {
+ { .compatible = "aspeed,ast2400-i2c-controller", },
+ { .compatible = "aspeed,ast2500-i2c-controller", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_controller_of_table);
+
+static struct platform_driver aspeed_i2c_controller_driver = {
+ .probe = aspeed_i2c_probe_controller,
+ .remove = aspeed_i2c_remove_controller,
+ .driver = {
+ .name = "ast-i2c-controller",
+ .of_match_table = aspeed_i2c_controller_of_table,
+ },
+};
+
+static int __init aspeed_i2c_driver_init(void)
+{
+ int ret;
+
+ ret = platform_driver_register(&aspeed_i2c_controller_driver);
+ if (ret < 0)
+ return ret;
+ return platform_driver_register(&aspeed_i2c_bus_driver);
+}
+module_init(aspeed_i2c_driver_init);
+
+static void __exit aspeed_i2c_driver_exit(void)
+{
+ platform_driver_unregister(&aspeed_i2c_bus_driver);
+ platform_driver_unregister(&aspeed_i2c_controller_driver);
+}
+module_exit(aspeed_i2c_driver_exit);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
+MODULE_LICENSE("GPL");
--
2.8.0.rc3.226.g39d4020
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 1/2] i2c: aspeed: added driver for Aspeed I2C
From: kbuild test robot @ 2016-09-10 0:48 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, joel-U3u1mxZcP9KHXe+LvDLADg,
jk-mnsaURCQ41sdnm+yROfE0A, Brendan Higgins
In-Reply-To: <1473450870-10333-2-git-send-email-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6646 bytes --]
Hi Brendan,
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on v4.8-rc5 next-20160909]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Brendan-Higgins/i2c-aspeed-added-driver-for-Aspeed-I2C/20160910-035912
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: blackfin-allmodconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 4.6.3
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
All errors (new ones prefixed by >>):
drivers/i2c/busses/i2c-aspeed.c:697:1: error: 'aspeed_i2c_of_table' undeclared here (not in a function)
>> drivers/i2c/busses/i2c-aspeed.c:812:1: error: redefinition of '__inittest'
drivers/i2c/busses/i2c-aspeed.c:707:1: note: previous definition of '__inittest' was here
>> drivers/i2c/busses/i2c-aspeed.c:812:1: error: redefinition of 'init_module'
drivers/i2c/busses/i2c-aspeed.c:707:1: note: previous definition of 'init_module' was here
>> drivers/i2c/busses/i2c-aspeed.c:812:1: error: redefinition of '__exittest'
drivers/i2c/busses/i2c-aspeed.c:707:1: note: previous definition of '__exittest' was here
>> drivers/i2c/busses/i2c-aspeed.c:812:1: error: redefinition of 'cleanup_module'
drivers/i2c/busses/i2c-aspeed.c:707:1: note: previous definition of 'cleanup_module' was here
>> drivers/i2c/busses/i2c-aspeed.c:801:1: error: '__mod_of__aspeed_i2c_of_table_device_table' aliased to undefined symbol 'aspeed_i2c_of_table'
>> drivers/i2c/busses/i2c-aspeed.c:801:1: error: '__mod_of__aspeed_i2c_of_table_device_table' aliased to undefined symbol 'aspeed_i2c_of_table'
vim +/__inittest +812 drivers/i2c/busses/i2c-aspeed.c
691
692 static const struct of_device_id aspeed_i2c_bus_of_table[] = {
693 { .compatible = "aspeed,ast2400-i2c-bus", },
694 { .compatible = "aspeed,ast2500-i2c-bus", },
695 { },
696 };
> 697 MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
698
699 static struct platform_driver aspeed_i2c_bus_driver = {
700 .probe = aspeed_i2c_probe_bus,
701 .remove = aspeed_i2c_remove_bus,
702 .driver = {
703 .name = "ast-i2c-bus",
704 .of_match_table = aspeed_i2c_bus_of_table,
705 },
706 };
707 module_platform_driver(aspeed_i2c_bus_driver);
708
709 static void aspeed_i2c_controller_irq(struct irq_desc *desc)
710 {
711 struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
712 unsigned long p, status;
713 unsigned int bus_irq;
714
715 status = readl(c->base);
716 for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
717 bus_irq = irq_find_mapping(c->irq_domain, p);
718 generic_handle_irq(bus_irq);
719 }
720 }
721
722 static int aspeed_i2c_probe_controller(struct platform_device *pdev)
723 {
724 struct aspeed_i2c_controller *controller;
725 struct device_node *np;
726 struct resource *res;
727
728 controller = kzalloc(sizeof(*controller), GFP_KERNEL);
729 if (!controller)
730 return -ENOMEM;
731
732 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
733 controller->base = devm_ioremap_resource(&pdev->dev, res);
734 if (IS_ERR(controller->base))
735 return PTR_ERR(controller->base);
736
737 controller->irq = platform_get_irq(pdev, 0);
738 if (controller->irq < 0) {
739 dev_err(&pdev->dev, "no platform IRQ\n");
740 return -ENXIO;
741 }
742
743 controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
744 ASPEED_I2C_NUM_BUS, &irq_domain_simple_ops, NULL);
745 if (!controller->irq_domain) {
746 dev_err(&pdev->dev, "no IRQ domain\n");
747 return -ENXIO;
748 }
749 controller->irq_domain->name = "ast-i2c-domain";
750
751 irq_set_chained_handler_and_data(controller->irq,
752 aspeed_i2c_controller_irq, controller);
753
754 controller->dev = &pdev->dev;
755
756 platform_set_drvdata(pdev, controller);
757
758 dev_info(controller->dev, "i2c controller registered, irq %d\n",
759 controller->irq);
760
761 for_each_child_of_node(pdev->dev.of_node, np) {
762 int ret;
763 u32 bus_num;
764 char bus_id[sizeof("i2c-12345")];
765
766 /*
767 * Set a useful name derived from the bus number; the device
768 * tree should provide us with one that corresponds to the
769 * hardware numbering. If the property is missing the
770 * probe would fail so just skip it here.
771 */
772
773 ret = of_property_read_u32(np, "bus", &bus_num);
774 if (ret)
775 continue;
776
777 ret = snprintf(bus_id, sizeof(bus_id), "i2c-%u", bus_num);
778 if (ret >= sizeof(bus_id))
779 continue;
780
781 of_platform_device_create(np, bus_id, &pdev->dev);
782 of_node_put(np);
783 }
784
785 return 0;
786 }
787
788 static int aspeed_i2c_remove_controller(struct platform_device *pdev)
789 {
790 struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
791
792 irq_domain_remove(controller->irq_domain);
793 return 0;
794 }
795
796 static const struct of_device_id aspeed_i2c_controller_of_table[] = {
797 { .compatible = "aspeed,ast2400-i2c-controller", },
798 { .compatible = "aspeed,ast2500-i2c-controller", },
799 { },
800 };
> 801 MODULE_DEVICE_TABLE(of, aspeed_i2c_of_table);
802
803 static struct platform_driver aspeed_i2c_controller_driver = {
804 .probe = aspeed_i2c_probe_controller,
805 .remove = aspeed_i2c_remove_controller,
806 .driver = {
807 .name = "ast-i2c-controller",
808 .of_match_table = aspeed_i2c_controller_of_table,
809 },
810 };
811
> 812 module_platform_driver(aspeed_i2c_controller_driver);
813
814 MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
815 MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 41250 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox