Re: [PATCH] i2c-nomadik: fix kerneldoc warning

2011-06-27 Thread Linus Walleij

On 06/28/2011 12:04 AM, Ben Dooks wrote:

On Wed, Jun 15, 2011 at 12:42:02PM +0200, Linus Walleij wrote:
   

From: Linus Walleij

There was a missing struct item in the kerneldoc, add it and fix
another pretty-printing formatting issue with a missing space.
 

weird, it didn't apply to 3.0-rc4.
   


In my topic branch I also have a patch from Srinidhi Kasagar
titled "i2c-nomadik: Do not use _interruptible_ variant call",
that went on the list some time back, could it be that you
missed that one?

If you want to I can resend it.

Thanks,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Question about i2c_xxx function on CONFIG_I2C

2011-06-27 Thread kuninori . morimoto . gx

Dear Jean

Thank you for your advice

> I don't think this is a good idea in general. If the kernel lacks I2C
> support, and your code uses it, then this is dead code, and you're much
> better excluding that code completely from your build. Most likely
> you'll be able to drop whole functions. I wouldn't count on the
> compiler to optimize it all properly, and you said you want a very
> small kernel.
> 
> Of course we could make better comments if we could see your actual
> code. Maybe some ifdef magic in i2c.h would make sense for a few items,
> as we already do for i2c_register_board_info for example.

I understand, thank you.
I use #ifdef for it.

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c: add Faraday FTIIC010 I2C controller support

2011-06-27 Thread Ben Dooks
On Tue, Jun 14, 2011 at 02:19:29PM +0800, Po-Yu Chuang wrote:
> From: Po-Yu Chuang 
> 
> Support for Faraday FTIIC010 I2C controller. It is used on
> Faraday A320, A369 and some other ARM SoC's.
> 
> Signed-off-by: Po-Yu Chuang 
> ---
>  drivers/i2c/busses/Kconfig|7 +
>  drivers/i2c/busses/Makefile   |1 +
>  drivers/i2c/busses/i2c-ftiic010.c |  434 
> +
>  drivers/i2c/busses/i2c-ftiic010.h |  155 +
>  4 files changed, 597 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/i2c/busses/i2c-ftiic010.c
>  create mode 100644 drivers/i2c/busses/i2c-ftiic010.h
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 326652f..612c2b1 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -358,6 +358,13 @@ config I2C_DESIGNWARE
> This driver can also be built as a module.  If so, the module
> will be called i2c-designware.
>  
> +config I2C_FTIIC010
> + tristate "Faraday FTIIC010 I2C controller"
> + depends on HAVE_CLK
> + help
> +   Support for Faraday FTIIC010 I2C controller. It is used on
> +   Faraday A320, A369 and some other ARM SoC's.
> +

is there a better depends line for this? is there an ARCH_xxx or some
other way of ensuring this doesn't get shown for all systems that
happen to support CLK?

>  config I2C_GPIO
>   tristate "GPIO-based bitbanging I2C"
>   depends on GENERIC_GPIO
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index e6cf294..c49570d 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_I2C_BLACKFIN_TWI)  += i2c-bfin-twi.o
>  obj-$(CONFIG_I2C_CPM)+= i2c-cpm.o
>  obj-$(CONFIG_I2C_DAVINCI)+= i2c-davinci.o
>  obj-$(CONFIG_I2C_DESIGNWARE) += i2c-designware.o
> +obj-$(CONFIG_I2C_FTIIC010)   += i2c-ftiic010.o
>  obj-$(CONFIG_I2C_GPIO)   += i2c-gpio.o
>  obj-$(CONFIG_I2C_HIGHLANDER) += i2c-highlander.o
>  obj-$(CONFIG_I2C_IBM_IIC)+= i2c-ibm_iic.o
> diff --git a/drivers/i2c/busses/i2c-ftiic010.c 
> b/drivers/i2c/busses/i2c-ftiic010.c
> new file mode 100644
> index 000..ed86f06
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-ftiic010.c
> @@ -0,0 +1,434 @@
> +/*
> + * Faraday FTIIC010 I2C Controller
> + *
> + * (C) Copyright 2010-2011 Faraday Technology
> + * Po-Yu Chuang 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "i2c-ftiic010.h"
> +
> +#define SCL_SPEED(100 * 1000)
> +#define MAX_RETRY1000
> +
> +#define GSR  5
> +#define TSR  5
> +
> +struct ftiic010 {
> + struct resource *res;
> + void __iomem *base;
> + int irq;
> + struct clk *clk;
> + struct i2c_adapter adapter;
> +};
> +
> +/**
> + * internal functions
> + 
> */
> +static void ftiic010_reset(struct ftiic010 *ftiic010)
> +{
> + int cr = FTIIC010_CR_I2C_RST;
> +
> + iowrite32(cr, ftiic010->base + FTIIC010_OFFSET_CR);
> +
> + /* wait until reset bit cleared by hw */
> + while (ioread32(ftiic010->base + FTIIC010_OFFSET_CR) & 
> FTIIC010_CR_I2C_RST)
> + ;

how about calls to cpu_relax() during these loops?
also, do we ever expect timeout?

> +}


I think iowrite32/ioread32 are ok on these.


> +
> +static void ftiic010_set_clock_speed(struct ftiic010 *ftiic010, int hz)
> +{
> + unsigned long pclk;
> + int cdr;
> +
> + pclk = clk_get_rate(ftiic010->clk);
> +
> + cdr = (pclk / hz - GSR) / 2 - 2;
> + cdr &= FTIIC010_CDR_MASK;
> +
> + dev_dbg(&ftiic010->adapter.dev, "  [CDR] = %08x\n", cdr);
> + iowrite32(cdr, ftiic010->base + FTIIC010_OFFSET_CDR);
> +}
> +
> +static void ftiic010_set_tgsr(struct ftiic010 *ftiic010, int tsr, int gsr)
> +{
> + int tgsr;
> +
> + tgsr = FTIIC010_TGSR_TSR(tsr);
> + tgsr |= FTIIC010_TGSR_GSR(gsr);
> +
> + dev_dbg(&ftiic010->adapter.dev, "  [TGSR] = %08x\n", tgsr);
> + iowrite32(tgsr, ftiic010->base + FTIIC010_OFFSET_TGSR);
> +}
> +
> +static void ftiic010_hw_ini

Re: [PATCH] i2c-nomadik: fix kerneldoc warning

2011-06-27 Thread Ben Dooks
On Wed, Jun 15, 2011 at 12:42:02PM +0200, Linus Walleij wrote:
> From: Linus Walleij 
> 
> There was a missing struct item in the kerneldoc, add it and fix
> another pretty-printing formatting issue with a missing space.

weird, it didn't apply to 3.0-rc4.

i'll let the two things in one patch slide for this one.
 
> Signed-off-by: Linus Walleij 
> ---
>  drivers/i2c/busses/i2c-nomadik.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-nomadik.c 
> b/drivers/i2c/busses/i2c-nomadik.c
> index f9b8854..b228e09 100644
> --- a/drivers/i2c/busses/i2c-nomadik.c
> +++ b/drivers/i2c/busses/i2c-nomadik.c
> @@ -146,6 +146,7 @@ struct i2c_nmk_client {
>   * @stop: stop condition
>   * @xfer_complete: acknowledge completion for a I2C message
>   * @result: controller propogated result
> + * @regulator: pointer to i2c regulator
>   * @busy: Busy doing transfer
>   */
>  struct nmk_i2c_dev {
> @@ -509,7 +510,7 @@ static int write_i2c(struct nmk_i2c_dev *dev)
>  
>   if (timeout < 0) {
>   dev_err(&dev->pdev->dev,
> - "wait_for_completion_timeout"
> + "wait_for_completion_timeout "
>   "returned %d waiting for event\n", timeout);
>   status = timeout;
>   }
> -- 
> 1.7.3.2
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 00/18] I2C: OMAP: I2C fixes, removal of cpu_is... from driver

2011-06-27 Thread Kevin Hilman
On Wed, 2011-06-15 at 16:33 -0700, Kevin Hilman wrote:
> Hi Ben,
> 
> Ben Dooks  writes:
> 
> > On Wed, Jun 01, 2011 at 05:19:26PM -0700, Kevin Hilman wrote:
> >> Ben, 
> >> 
> >> Please pull in this series from Andy Green for the next merge window.
> >> 
> >> v4 is simply a rebase of Andy's v3 against v3.0-rc1 with some basic
> >> sanity testing against with the latest kernel.
> >
> > These look much more like functionality changes than actual fixes, and
> > are much more suited for inclusion into -next for once the next stable
> > release is out.
> 
> Correct, this is a rework of functionality, not a set of fixes.  This
> series is targeted for the next release (v3.1), not for fixes
> (v3.0-rc).  
> 
> I was requesting for it to be included in your queue for -next and for
> the next merge window.
> 
> Tony has already been merging this in his omap-testing branch so it is
> has been getting lots of testing on OMAP platforms already.

Ping.  I don't see this in linux-next yet.

Are you planning to queue this for v3.1?

Thanks,

Kevin


--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 00/18] I2C: OMAP: I2C fixes, removal of cpu_is... from driver

2011-06-27 Thread Kevin Hilman
On Mon, 2011-06-27 at 15:12 -0700, Kevin Hilman wrote:

> Ping.  I don't see this in linux-next yet.
> 
> Are you planning to queue this for v3.1?
> 

Oops, pushed send too soon on this...

As this series touches various data files in arch/arm/mach-omap2/* and
we have a handful of other changes to that data for this merge window, I
think it would be better for conflict avoidance if this series is merged
via Tony's linux-omap tree.

With your Ack, we'll merge it via OMAP.

Thanks,

Kevin



--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c: Allow i2c_add_numbered_adapter() to assign a bus id.

2011-06-27 Thread Grant Likely
On Mon, Jun 27, 2011 at 12:55 PM, Grant Likely
 wrote:
> Currently, if an i2c bus driver supports both static and dynamic bus
> ids, it needs to choose between calling i2c_add_numbered_adapter() and
> i2c_add_adapter().  This patch makes i2c_add_numbered_adapter()
> redirect to i2c_add_adapter() if the requested bus id is -1.
>
> Signed-off-by: Grant Likely 

Oops, forgot to edit the email before sending the patch.

This patch is as-yet untested other than build testing, but I want to
get feedback.  With the move to DT on ARM, there are going to be a lot
more i2c bus drivers that need to support both static and dynamically
allocated busses, and it is very likely that it will be needed in the
v3.1 merge window.

Ben/Jean, *IF* this patch tests out okay, and *IF* I get an ack from
you, it will probably need to have this commit in my devicetree/next
branch.  If so, then I'll either need to commit it myself, or have it
put into a separate topic branch that both of us can merge into our
trees.  What is your preference.

John: this is the patch that I asked you to write earlier today, but I
think one of the TI folks will run into the same issue, so I wanted to
get it drafted and onto the list ASAP.

g.

> ---
>  drivers/i2c/busses/i2c-cpm.c   |    7 ++-
>  drivers/i2c/busses/i2c-pxa.c   |    7 ++-
>  drivers/i2c/busses/i2c-s6000.c |    5 +
>  drivers/i2c/i2c-core.c         |    5 +
>  4 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
> index 3a20961..b1d9cd2 100644
> --- a/drivers/i2c/busses/i2c-cpm.c
> +++ b/drivers/i2c/busses/i2c-cpm.c
> @@ -662,11 +662,8 @@ static int __devinit cpm_i2c_probe(struct 
> platform_device *ofdev)
>        /* register new adapter to i2c module... */
>
>        data = of_get_property(ofdev->dev.of_node, "linux,i2c-index", &len);
> -       if (data && len == 4) {
> -               cpm->adap.nr = *data;
> -               result = i2c_add_numbered_adapter(&cpm->adap);
> -       } else
> -               result = i2c_add_adapter(&cpm->adap);
> +       cpm->adap.nr = (data && len == 4) ? be32_to_cpup(data) : -1;
> +       result = i2c_add_numbered_adapter(&cpm->adap);
>
>        if (result < 0) {
>                dev_err(&ofdev->dev, "Unable to register with I2C\n");
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index f59224a..d603646 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -1079,7 +1079,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
>         * The reason to do so is to avoid sysfs names that only make
>         * sense when there are multiple adapters.
>         */
> -       i2c->adap.nr = dev->id != -1 ? dev->id : 0;
> +       i2c->adap.nr = dev->id;
>        snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
>                 i2c->adap.nr);
>
> @@ -1142,10 +1142,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
>        i2c->adap.dev.of_node = dev->dev.of_node;
>  #endif
>
> -       if (i2c_type == REGS_CE4100)
> -               ret = i2c_add_adapter(&i2c->adap);
> -       else
> -               ret = i2c_add_numbered_adapter(&i2c->adap);
> +       ret = i2c_add_numbered_adapter(&i2c->adap);
>        if (ret < 0) {
>                printk(KERN_INFO "I2C: Failed to add bus\n");
>                goto eadapt;
> diff --git a/drivers/i2c/busses/i2c-s6000.c b/drivers/i2c/busses/i2c-s6000.c
> index cb5d01e..c64ba73 100644
> --- a/drivers/i2c/busses/i2c-s6000.c
> +++ b/drivers/i2c/busses/i2c-s6000.c
> @@ -341,10 +341,7 @@ static int __devinit s6i2c_probe(struct platform_device 
> *dev)
>        i2c_wr16(iface, S6_I2C_TXTL, 0);
>
>        platform_set_drvdata(dev, iface);
> -       if (bus_num < 0)
> -               rc = i2c_add_adapter(p_adap);
> -       else
> -               rc = i2c_add_numbered_adapter(p_adap);
> +       rc = i2c_add_numbered_adapter(p_adap);
>        if (rc)
>                goto err_irq_free;
>        return 0;
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 9a58994..131079a 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -925,6 +925,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
>  * or otherwise built in to the system's mainboard, and where i2c_board_info
>  * is used to properly configure I2C devices.
>  *
> + * If the requested bus number is set to -1, then this function will behave
> + * identically to i2c_add_adapter, and will dynamically assign a bus number.
> + *
>  * If no devices have pre-been declared for this bus, then be sure to
>  * register the adapter before any dynamically allocated ones.  Otherwise
>  * the required bus ID may not be available.
> @@ -940,6 +943,8 @@ int i2c_add_numbered_adapter(struct i2c_adapter *adap)
>        int     id;
>        int     status;
>
> +       if (adap->nr == -1) /* -1 means dynamically assign bus id */
> +               return i2c_add_adapter(adap);
>        if (adap->nr & ~M

[PATCH] i2c: Allow i2c_add_numbered_adapter() to assign a bus id.

2011-06-27 Thread Grant Likely
Currently, if an i2c bus driver supports both static and dynamic bus
ids, it needs to choose between calling i2c_add_numbered_adapter() and
i2c_add_adapter().  This patch makes i2c_add_numbered_adapter()
redirect to i2c_add_adapter() if the requested bus id is -1.

Signed-off-by: Grant Likely 
---
 drivers/i2c/busses/i2c-cpm.c   |7 ++-
 drivers/i2c/busses/i2c-pxa.c   |7 ++-
 drivers/i2c/busses/i2c-s6000.c |5 +
 drivers/i2c/i2c-core.c |5 +
 4 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 3a20961..b1d9cd2 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -662,11 +662,8 @@ static int __devinit cpm_i2c_probe(struct platform_device 
*ofdev)
/* register new adapter to i2c module... */
 
data = of_get_property(ofdev->dev.of_node, "linux,i2c-index", &len);
-   if (data && len == 4) {
-   cpm->adap.nr = *data;
-   result = i2c_add_numbered_adapter(&cpm->adap);
-   } else
-   result = i2c_add_adapter(&cpm->adap);
+   cpm->adap.nr = (data && len == 4) ? be32_to_cpup(data) : -1;
+   result = i2c_add_numbered_adapter(&cpm->adap);
 
if (result < 0) {
dev_err(&ofdev->dev, "Unable to register with I2C\n");
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index f59224a..d603646 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1079,7 +1079,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 * The reason to do so is to avoid sysfs names that only make
 * sense when there are multiple adapters.
 */
-   i2c->adap.nr = dev->id != -1 ? dev->id : 0;
+   i2c->adap.nr = dev->id;
snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
 i2c->adap.nr);
 
@@ -1142,10 +1142,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
i2c->adap.dev.of_node = dev->dev.of_node;
 #endif
 
-   if (i2c_type == REGS_CE4100)
-   ret = i2c_add_adapter(&i2c->adap);
-   else
-   ret = i2c_add_numbered_adapter(&i2c->adap);
+   ret = i2c_add_numbered_adapter(&i2c->adap);
if (ret < 0) {
printk(KERN_INFO "I2C: Failed to add bus\n");
goto eadapt;
diff --git a/drivers/i2c/busses/i2c-s6000.c b/drivers/i2c/busses/i2c-s6000.c
index cb5d01e..c64ba73 100644
--- a/drivers/i2c/busses/i2c-s6000.c
+++ b/drivers/i2c/busses/i2c-s6000.c
@@ -341,10 +341,7 @@ static int __devinit s6i2c_probe(struct platform_device 
*dev)
i2c_wr16(iface, S6_I2C_TXTL, 0);
 
platform_set_drvdata(dev, iface);
-   if (bus_num < 0)
-   rc = i2c_add_adapter(p_adap);
-   else
-   rc = i2c_add_numbered_adapter(p_adap);
+   rc = i2c_add_numbered_adapter(p_adap);
if (rc)
goto err_irq_free;
return 0;
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 9a58994..131079a 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -925,6 +925,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
  * or otherwise built in to the system's mainboard, and where i2c_board_info
  * is used to properly configure I2C devices.
  *
+ * If the requested bus number is set to -1, then this function will behave
+ * identically to i2c_add_adapter, and will dynamically assign a bus number.
+ *
  * If no devices have pre-been declared for this bus, then be sure to
  * register the adapter before any dynamically allocated ones.  Otherwise
  * the required bus ID may not be available.
@@ -940,6 +943,8 @@ int i2c_add_numbered_adapter(struct i2c_adapter *adap)
int id;
int status;
 
+   if (adap->nr == -1) /* -1 means dynamically assign bus id */
+   return i2c_add_adapter(adap);
if (adap->nr & ~MAX_ID_MASK)
return -EINVAL;
 

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Question about i2c_xxx function on CONFIG_I2C

2011-06-27 Thread Zhang, Shijie
Hi, I think you can config the kernel using make menuconfig command, go to 
Device Driver--->I2C Suppport

Check what you want to disselect. 


-Original Message-
From: linux-i2c-ow...@vger.kernel.org [mailto:linux-i2c-ow...@vger.kernel.org] 
On Behalf Of kuninori.morimoto...@renesas.com
Sent: Monday, June 27, 2011 3:00 PM
To: Linux-I2C
Subject: Question about i2c_xxx function on CONFIG_I2C


Dear all

I'm using i2c_xxx function on some board.
And sometimes I need very small kernel which doesn't need CONFIG_I2C.

But then (.config doesn't have CONFIG_I2C), the compile will fail.
like this

error: implicit declaration of function 'i2c_get_adapter'
error: implicit declaration of function 'i2c_transfer'

In this case, should I use #ifdef CONFIG_I2C in my code to solve this compile 
issue ?
Or is below #else in i2c.h good idea ?

--- i2c.h -
#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
...
extern struct i2c_adapter *i2c_get_adapter(int nr);
...
#else
...
#define i2c_get_adapter(nr) NULL
...
#endif


Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Need to use a I2C EEPROM on normal x86 architecture

2011-06-27 Thread Christian Gmeiner
2011/6/23 Jean Delvare :
> Hi Christian,
>
> On Tue, 21 Jun 2011 13:54:52 +0200, Christian Gmeiner wrote:
>> Hi community,
>>
>> I am working on an embedded x86 device, which has an at24 based
>> eeprom. I am using
>> the i2c_eg20t driver to access the i2c bus. To be able to access the
>> eeprom in a separated
>> driver I did this:
>>
>> /* technical description of our used EEPROM */
>> static struct at24_platform_data custom_i2c_eeprom_info = {
>>       .byte_len       = EEPROM_BYTE_LEN,
>>       .page_size      = 16,
>>       .flags          = 0,
>
> Note that you don't have to mention struct members with value 0 (or
> NULL), as this is the default.
>

Thanks for this hint.

>>       .setup          = content_read,
>>       .context        = NULL,
>> };
>>
>> /* EEPROM at24 */
>> static struct i2c_board_info __initdata i2c_info[] =  {
>>       {
>>               I2C_BOARD_INFO("24c04", 0x50),
>>               .platform_data  = &custom_i2c_eeprom_info,
>>       },
>> };
>>
>> In the init function of my custom driver I do this:
>>
>>       /* register known devices on i2c bus */
>>       status = i2c_register_board_info(0, i2c_info, ARRAY_SIZE(i2c_info))
>
> Out of curiosity, where did you put this code? Does x86 finally support
> per-machine initialization as e.g. arm does?
>

I have an other x86 based target machine, which runs a 2.6.36.4
kernel, where I created
a new driver under drivers/misc called custom_eeprom.c. The driver is
used to access
some special values stored in eeprom easily from userspace via /proc.

static int __init custom_eeprom_init(void)
{

...

/* register known devices on i2c bus */
status = i2c_register_board_info(0, i2c_info, ARRAY_SIZE(i2c_info));

/* create procfs entries */

...

return ret;
}

It is AMD LX800 based an I use the scx200_acb i2c driver, modified to
use i2c_add_numbered_adapter().
I did a small test with 3.0-rc4 on the LX800 target, but it get an
oops.. so there seems to be some changes in the involved subsystems.

>>
>> Now I run in some troubles... see
>> http://www.spinics.net/lists/linux-i2c/msg02022.html
>
> I see that I already replied to this post...
>
>> What options do I have to get this running? I could use
>> i2c_add_numbered_adapter, but I don't
>> want to touch too much from mainline kernel sources.
>
> It seems difficult to use i2c_add_numbered_adapter() unconditionally, as
> i2c-eg20t is a PCI driver so you don't get to pass platform data to it.
> Furthermore, i2c_add_numbered_adapter() is only suitable if machine
> setup code could be run before any device driver is initialized;
> otherwise odds are that another driver will have picked the i2c bus
> number you wanted. I am unsure if this is possible at all on x86 at the
> moment.
>
> The way I would do it is from i2c-eg20t itself. Take a look at i2c-i801
> for an example: at the end of the probe function, there is
> hardware-specific code to instantiate a few I2C devices. If you have a
> way to uniquely, reliably detect that you are running on your specific
> target system, you can do the same.
>
> I don't think it is particularly nice, BTW, but this is the only way I
> found so far with what the i2c subsystem core offers. If anyone has
> suggestions how to improve this, please speak up.
>
> If you want to be able to use i2c_add_numbered_adapter() conditionally
> without the help of platform data, then you need a hint from i2c-core.
> Would the following patch help you? If it does, and others show
> interest, and there are no objections, this could go upstream in kernel
> 3.1.
>

I get an oops quite early in kernel bootup... I will try to catch it
and if you are
interested I will post it here.

> ---
>  drivers/i2c/busses/i2c-eg20t.c |    7 ++-
>  drivers/i2c/i2c-boardinfo.c    |   20 
>  include/linux/i2c.h            |    5 +
>  3 files changed, 31 insertions(+), 1 deletion(-)
>
> --- linux-3.0-rc4.orig/drivers/i2c/i2c-boardinfo.c      2011-05-20 
> 10:42:40.0 +0200
> +++ linux-3.0-rc4/drivers/i2c/i2c-boardinfo.c   2011-06-23 10:15:56.0 
> +0200
> @@ -90,3 +90,23 @@ i2c_register_board_info(int busnum,
>
>        return status;
>  }
> +
> +/**
> + * i2c_adapter_is_static - let drivers know if their bus is static
> + * @busnum: identifies the bus
> + *
> + * After calling this function, i2c bus drivers can decide whether
> + * to call i2c_add_adapter or i2c_add_numbered_adapter.
> + */
> +int
> +i2c_adapter_is_static(int busnum)
> +{
> +       int is_static;
> +
> +       down_write(&__i2c_board_lock);
> +       is_static = busnum < __i2c_first_dynamic_bus_num;
> +       up_write(&__i2c_board_lock);
> +
> +       return is_static;
> +}
> +EXPORT_SYMBOL_GPL(i2c_adapter_is_static);
> --- linux-3.0-rc4.orig/include/linux/i2c.h      2011-06-21 10:32:32.0 
> +0200
> +++ linux-3.0-rc4/include/linux/i2c.h   2011-06-23 09:58:21.0 +0200
> @@ -306,6 +306,7 @@ extern void i2c_unregister_device(struc

Re: Question about i2c_xxx function on CONFIG_I2C

2011-06-27 Thread Jean Delvare
Hi Morimoto,

On Mon, 27 Jun 2011 16:00:08 +0900, kuninori.morimoto...@renesas.com wrote:
> I'm using i2c_xxx function on some board.
> And sometimes I need very small kernel which doesn't need CONFIG_I2C.
> 
> But then (.config doesn't have CONFIG_I2C), the compile will fail.
> like this
> 
> error: implicit declaration of function 'i2c_get_adapter'
> error: implicit declaration of function 'i2c_transfer'
> 
> In this case, should I use #ifdef CONFIG_I2C in my code to solve this compile 
> issue ?

Yes.

> Or is below #else in i2c.h good idea ?
> 
> --- i2c.h -
> #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
> ...
> extern struct i2c_adapter *i2c_get_adapter(int nr);
> ...
> #else
> ...
> #define i2c_get_adapter(nr) NULL
> ...
> #endif

I don't think this is a good idea in general. If the kernel lacks I2C
support, and your code uses it, then this is dead code, and you're much
better excluding that code completely from your build. Most likely
you'll be able to drop whole functions. I wouldn't count on the
compiler to optimize it all properly, and you said you want a very
small kernel.

Of course we could make better comments if we could see your actual
code. Maybe some ifdef magic in i2c.h would make sense for a few items,
as we already do for i2c_register_board_info for example.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Question about i2c_xxx function on CONFIG_I2C

2011-06-27 Thread kuninori . morimoto . gx

Dear all

I'm using i2c_xxx function on some board.
And sometimes I need very small kernel which doesn't need CONFIG_I2C.

But then (.config doesn't have CONFIG_I2C), the compile will fail.
like this

error: implicit declaration of function 'i2c_get_adapter'
error: implicit declaration of function 'i2c_transfer'

In this case, should I use #ifdef CONFIG_I2C in my code to solve this compile 
issue ?
Or is below #else in i2c.h good idea ?

--- i2c.h -
#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
...
extern struct i2c_adapter *i2c_get_adapter(int nr);
...
#else
...
#define i2c_get_adapter(nr) NULL
...
#endif


Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html