| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130213203429.GA12948@roeck-us.net>
Date: Wed, 13 Feb 2013 12:34:29 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Doug Anderson <dianders@...omium.org>
Cc: Wolfram Sang <w.sang@...gutronix.de>,
Simon Glass <sjg@...omium.org>,
Naveen Krishna Chatradhi <ch.naveen@...sung.com>,
Grant Likely <grant.likely@...retlab.ca>,
Yuvaraj Kumar <yuvaraj.cd@...il.com>,
Ben Dooks <ben.dooks@...ethink.co.uk>,
u.kleine-koenig@...gutronix.de,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
Girish Shivananjappa <girish.shivananjappa@...aro.org>,
bhushan.r@...sung.com, sreekumar.c@...sung.com,
Prashanth G <prashanth.g@...sung.com>,
Olof Johansson <olof@...om.net>,
Daniel Kurtz <djkurtz@...omium.org>,
Grant Grundler <grundler@...omium.org>,
"Ben Dooks (embedded platforms)" <ben-linux@...ff.org>,
Peter Korsgaard <peter.korsgaard@...co.com>,
Jean Delvare <khali@...ux-fr.org>,
David Daney <david.daney@...ium.com>,
Axel Lin <axel.lin@...il.com>,
Stephen Warren <swarren@...dia.com>,
Barry Song <baohua.song@....com>,
Guan Xuetao <gxt@...c.pku.edu.cn>, linux-i2c@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 4/4] i2c-mux: i2c_add_mux_adapter() should use -1 for
auto bus num
On Wed, Feb 13, 2013 at 10:02:12AM -0800, Doug Anderson wrote:
> The force_nr parameter to i2c_add_mux_adapter() uses 0 to signify that
> we don't want to force the bus number of the adapter. This is
> non-ideal because:
> * 0 is actually a valid bus number to request
> * i2c_add_numbered_adapter() (which i2c_add_mux_adapter() calls) uses
> -1 to mean the same thing. That means extra logic in
> i2c_add_mux_adapter().
>
> Fix i2c_add_mux_adapter() to use -1 and update all mux drivers
> accordingly.
>
> Signed-off-by: Doug Anderson <dianders@...omium.org>
> ---
> Notes:
> - If there's a good reason that force_nr uses 0 for auto then feel
> free to drop this patch. I've place it at the end of the series to
> make it easy to just drop it.
>
> drivers/i2c/i2c-mux.c | 10 +++-------
> drivers/i2c/muxes/i2c-arbitrator.c | 2 +-
> drivers/i2c/muxes/i2c-mux-gpio.c | 2 +-
> drivers/i2c/muxes/i2c-mux-pca9541.c | 2 +-
> drivers/i2c/muxes/i2c-mux-pca954x.c | 2 +-
> drivers/i2c/muxes/i2c-mux-pinctrl.c | 4 ++--
> include/linux/i2c-mux.h | 2 +-
> 7 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index d94e0ce..8ad1a56 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -103,7 +103,7 @@ static unsigned int i2c_mux_parent_classes(struct i2c_adapter *parent)
>
> struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
> struct device *mux_dev,
> - void *mux_priv, u32 force_nr, u32 chan_id,
> + void *mux_priv, int force_nr, u32 chan_id,
> unsigned int class,
> int (*select) (struct i2c_adapter *,
> void *, u32),
> @@ -168,12 +168,8 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
> }
> }
>
> - if (force_nr) {
> - priv->adap.nr = force_nr;
> - ret = i2c_add_numbered_adapter(&priv->adap);
> - } else {
> - ret = i2c_add_adapter(&priv->adap);
> - }
> + priv->adap.nr = force_nr;
> + ret = i2c_add_numbered_adapter(&priv->adap);
> if (ret < 0) {
> dev_err(&parent->dev,
> "failed to add mux-adapter (error=%d)\n",
> diff --git a/drivers/i2c/muxes/i2c-arbitrator.c b/drivers/i2c/muxes/i2c-arbitrator.c
> index c3bbdf7..89d0d06 100644
> --- a/drivers/i2c/muxes/i2c-arbitrator.c
> +++ b/drivers/i2c/muxes/i2c-arbitrator.c
> @@ -173,7 +173,7 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
> arb->wait_free_us = 50000;
>
> /* Actually add the mux adapter */
> - arb->child = i2c_add_mux_adapter(arb->parent, &pdev->dev, arb, 0, 0, 0,
> + arb->child = i2c_add_mux_adapter(arb->parent, &pdev->dev, arb, -1, 0, 0,
> i2c_arbitrator_select,
> i2c_arbitrator_deselect);
> if (WARN_ON(!arb->child)) {
> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
> index 9f50ef0..301ed0b 100644
> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
> @@ -208,7 +208,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
> }
>
> for (i = 0; i < mux->data.n_values; i++) {
> - u32 nr = mux->data.base_nr ? (mux->data.base_nr + i) : 0;
> + int nr = mux->data.base_nr ? (mux->data.base_nr + i) : -1;
> unsigned int class = mux->data.classes ? mux->data.classes[i] : 0;
>
> mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
> diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
> index f3b8f9a..a58b3c2 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
> @@ -350,7 +350,7 @@ static int pca9541_probe(struct i2c_client *client,
>
> /* Create mux adapter */
>
> - force = 0;
> + force = -1;
> if (pdata)
> force = pdata->modes[0].adap_id;
> data->mux_adap = i2c_add_mux_adapter(adap, &client->dev, client,
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 8e43872..4663ce6 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -215,7 +215,7 @@ static int pca954x_probe(struct i2c_client *client,
>
> /* Now create an adapter for each channel */
> for (num = 0; num < chips[data->type].nchans; num++) {
> - force = 0; /* dynamic adap number */
> + force = -1; /* dynamic adap number */
> class = 0; /* no class by default */
> if (pdata) {
> if (num < pdata->num_modes) {
> diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
> index a43c0ce..401ff5d 100644
> --- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
> +++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
> @@ -217,8 +217,8 @@ static int i2c_mux_pinctrl_probe(struct platform_device *pdev)
> }
>
> for (i = 0; i < mux->pdata->bus_count; i++) {
> - u32 bus = mux->pdata->base_bus_num ?
> - (mux->pdata->base_bus_num + i) : 0;
> + int bus = mux->pdata->base_bus_num ?
> + (mux->pdata->base_bus_num + i) : -1;
Maybe this should be:
int bus = mux->pdata->base_bus_num >= 0 ?
(mux->pdata->base_bus_num + i) : -1;
Otherwise you still can not assign bus number 0 as 1st bus. Also, if base_bus_num
ends up being provided as -1, the result would be pretty much unpredictable.
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists