Re: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series

2013-10-11 Thread Naveen Krishna Ch 
On 12 October 2013 11:12, Tomasz Figa  wrote:
> On Saturday 12 of October 2013 04:28:51 Tomasz Figa wrote:
>> [Fixing incorrent mail addresses and dropping the old DT ML.]
>>
>> On Saturday 12 of October 2013 04:22:04 Tomasz Figa wrote:
>> > Hi Naveen,
>> >
>> > On Friday 11 of October 2013 16:56:54 Naveen Krishna Chatradhi wrote:
>> > > The exynos5 i2c clock is based on a fixed 66 MHz peripheral clock, and
>> > > therefore is completely independent of the cpu frequency.
>> > > Thus, registering for a CPU freq notifier is very wasteful.
>> > >
>> > > This patch modifes the code such that, i2c bus registers to
>> > > cpu_freq_transition only for non Exynos SoCs.
>> > >
>> > > This change should save a bunch of cpufreq transitions calls
>> > > which does not apply to exynos SoCs.
>> >
>> > The idea is fine, although...
>> >
>> > > Signed-off-by: Naveen Krishna Chatradhi 
>> > > ---
>> > >  drivers/i2c/busses/i2c-s3c2410.c |4 ++--
>> > >  1 file changed, 2 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c 
>> > > b/drivers/i2c/busses/i2c-s3c2410.c
>> > > index cab1c91..d062fa7 100644
>> > > --- a/drivers/i2c/busses/i2c-s3c2410.c
>> > > +++ b/drivers/i2c/busses/i2c-s3c2410.c
>> > > @@ -123,7 +123,7 @@ struct s3c24xx_i2c {
>> > >   struct s3c2410_platform_i2c *pdata;
>> > >   int gpios[2];
>> > >   struct pinctrl  *pctrl;
>> > > -#ifdef CONFIG_CPU_FREQ
>> > > +#if defined(CONFIG_CPU_FREQ) && !defined(CONFIG_ARCH_EXYNOS)
>> >
>> > ...this is not a good coding practice, especially when already having
>> > multiplatform kernels in sight.
>> >
>> > The best way would be to check on which SoC we are running at runtime,
>> > but since this might need changing a lot of code, then at least I would
>> > change this from !defined(EXYNOS) to defined(S3C24XX), so it is not being
>> > compiled in when S3C24XX support is not enabled and if it's enabled then
>> > the notifier will be registered as a safe fallback that will run correctly
>> > on all platforms.
>
> Actually you can simply check for CONFIG_CPU_FREQ_S3C24XX here.
sure, this makes it more meaningful. will make the modifications and resubmit.
>
> Best regards,
> Tomasz
>



-- 
Shine bright,
(: Nav :)
--
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

[PULL REQUEST] i2c for 3.12

2013-10-11 Thread Wolfram Sang 
Linus,

we had various reports of problems with deferred probing in the I2C
subsystem, so this pull requst is a little bigger than usual. Most
issues should be addressed now so devices will be found correctly. A few
ususal driver bugfixes are in here, too. Please pull.

Thanks,

   Wolfram


The following changes since commit d0e639c9e06d44e713170031fe05fb60ebe680af:

  Linux 3.12-rc4 (2013-10-06 14:00:20 -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 2737de460e33df89461a59b247d3bfd477101785:

  i2c: i2c-mux-pinctrl: use deferred probe when adapter not found (2013-10-10 
10:22:35 +0200)


Ionut Nicu (2):
  i2c: i2c-mux-gpio: don't ignore of_get_named_gpio errors
  i2c: i2c-mux-gpio: use deferred probing

Jean Delvare (1):
  i2c: Not all adapters have a parent

Taras Kondratiuk (1):
  i2c: omap: Clear ARDY bit twice

Wolfram Sang (6):
  i2c: i2c-designware-platdrv: replace platform_driver_probe to support 
deferred probing
  i2c: i2c-imx: replace platform_driver_probe to support deferred probing
  i2c: i2c-mxs: replace platform_driver_probe to support deferred probing
  i2c: i2c-stu300: replace platform_driver_probe to support deferred probing
  i2c: i2c-arb-gpio-challenge: use deferred probe when adapter not found
  i2c: i2c-mux-pinctrl: use deferred probe when adapter not found

 drivers/i2c/busses/i2c-designware-platdrv.c |  5 +++--
 drivers/i2c/busses/i2c-imx.c| 11 ++-
 drivers/i2c/busses/i2c-mxs.c|  3 ++-
 drivers/i2c/busses/i2c-omap.c   |  3 +++
 drivers/i2c/busses/i2c-stu300.c | 11 +--
 drivers/i2c/i2c-core.c  |  3 +++
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c  |  2 +-
 drivers/i2c/muxes/i2c-mux-gpio.c| 14 +-
 drivers/i2c/muxes/i2c-mux-pinctrl.c |  4 ++--
 9 files changed, 34 insertions(+), 22 deletions(-)


signature.asc
Description: Digital signature

Re: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series

2013-10-11 Thread Tomasz Figa 
On Saturday 12 of October 2013 04:28:51 Tomasz Figa wrote:
> [Fixing incorrent mail addresses and dropping the old DT ML.]
> 
> On Saturday 12 of October 2013 04:22:04 Tomasz Figa wrote:
> > Hi Naveen,
> > 
> > On Friday 11 of October 2013 16:56:54 Naveen Krishna Chatradhi wrote:
> > > The exynos5 i2c clock is based on a fixed 66 MHz peripheral clock, and
> > > therefore is completely independent of the cpu frequency.
> > > Thus, registering for a CPU freq notifier is very wasteful.
> > > 
> > > This patch modifes the code such that, i2c bus registers to
> > > cpu_freq_transition only for non Exynos SoCs.
> > > 
> > > This change should save a bunch of cpufreq transitions calls
> > > which does not apply to exynos SoCs.
> > 
> > The idea is fine, although...
> > 
> > > Signed-off-by: Naveen Krishna Chatradhi 
> > > ---
> > >  drivers/i2c/busses/i2c-s3c2410.c |4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/i2c/busses/i2c-s3c2410.c 
> > > b/drivers/i2c/busses/i2c-s3c2410.c
> > > index cab1c91..d062fa7 100644
> > > --- a/drivers/i2c/busses/i2c-s3c2410.c
> > > +++ b/drivers/i2c/busses/i2c-s3c2410.c
> > > @@ -123,7 +123,7 @@ struct s3c24xx_i2c {
> > >   struct s3c2410_platform_i2c *pdata;
> > >   int gpios[2];
> > >   struct pinctrl  *pctrl;
> > > -#ifdef CONFIG_CPU_FREQ
> > > +#if defined(CONFIG_CPU_FREQ) && !defined(CONFIG_ARCH_EXYNOS)
> > 
> > ...this is not a good coding practice, especially when already having
> > multiplatform kernels in sight.
> > 
> > The best way would be to check on which SoC we are running at runtime,
> > but since this might need changing a lot of code, then at least I would
> > change this from !defined(EXYNOS) to defined(S3C24XX), so it is not being
> > compiled in when S3C24XX support is not enabled and if it's enabled then
> > the notifier will be registered as a safe fallback that will run correctly
> > on all platforms.

Actually you can simply check for CONFIG_CPU_FREQ_S3C24XX here.

Best regards,
Tomasz

--
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: Fix modalias for ACPI enumerated I2C devices

2013-10-11 Thread Mika Westerberg 
On Sat, Oct 12, 2013 at 12:16:02AM +0200, Rafael J. Wysocki wrote:
> > I think that this is intentional. We don't want that the i2c modalias
> > matches with the ACPI device (like with the i2c:INTABCD). Instead use ACPI
> > IDs that are added to the driver to match with the ACPI device.
> 
> Well, I'm not really sure this was intentional, but I wonder how other bus
> types work in that respect?

We have the same for platform bus, if that's what you are asking.

It probably doesn't hurt to have this patch applied but it might cause
inadvertent match if for some reason there is an I2C client driver that
happens to have INTABCD I2C id in its list.
--
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: s3c2410: dont need CPU_FREQ transitions for exynos series

2013-10-11 Thread Tomasz Figa 
[Fixing incorrent mail addresses and dropping the old DT ML.]

On Saturday 12 of October 2013 04:22:04 Tomasz Figa wrote:
> Hi Naveen,
> 
> On Friday 11 of October 2013 16:56:54 Naveen Krishna Chatradhi wrote:
> > The exynos5 i2c clock is based on a fixed 66 MHz peripheral clock, and
> > therefore is completely independent of the cpu frequency.
> > Thus, registering for a CPU freq notifier is very wasteful.
> > 
> > This patch modifes the code such that, i2c bus registers to
> > cpu_freq_transition only for non Exynos SoCs.
> > 
> > This change should save a bunch of cpufreq transitions calls
> > which does not apply to exynos SoCs.
> 
> The idea is fine, although...
> 
> > Signed-off-by: Naveen Krishna Chatradhi 
> > ---
> >  drivers/i2c/busses/i2c-s3c2410.c |4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-s3c2410.c 
> > b/drivers/i2c/busses/i2c-s3c2410.c
> > index cab1c91..d062fa7 100644
> > --- a/drivers/i2c/busses/i2c-s3c2410.c
> > +++ b/drivers/i2c/busses/i2c-s3c2410.c
> > @@ -123,7 +123,7 @@ struct s3c24xx_i2c {
> > struct s3c2410_platform_i2c *pdata;
> > int gpios[2];
> > struct pinctrl  *pctrl;
> > -#ifdef CONFIG_CPU_FREQ
> > +#if defined(CONFIG_CPU_FREQ) && !defined(CONFIG_ARCH_EXYNOS)
> 
> ...this is not a good coding practice, especially when already having
> multiplatform kernels in sight.
> 
> The best way would be to check on which SoC we are running at runtime,
> but since this might need changing a lot of code, then at least I would
> change this from !defined(EXYNOS) to defined(S3C24XX), so it is not being
> compiled in when S3C24XX support is not enabled and if it's enabled then
> the notifier will be registered as a safe fallback that will run correctly
> on all platforms.
> 
> Best regards,
> Tomasz
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 
> 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: [PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series

2013-10-11 Thread Tomasz Figa 
Hi Naveen,

On Friday 11 of October 2013 16:56:54 Naveen Krishna Chatradhi wrote:
> The exynos5 i2c clock is based on a fixed 66 MHz peripheral clock, and
> therefore is completely independent of the cpu frequency.
> Thus, registering for a CPU freq notifier is very wasteful.
> 
> This patch modifes the code such that, i2c bus registers to
> cpu_freq_transition only for non Exynos SoCs.
> 
> This change should save a bunch of cpufreq transitions calls
> which does not apply to exynos SoCs.

The idea is fine, although...

> Signed-off-by: Naveen Krishna Chatradhi 
> ---
>  drivers/i2c/busses/i2c-s3c2410.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c 
> b/drivers/i2c/busses/i2c-s3c2410.c
> index cab1c91..d062fa7 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -123,7 +123,7 @@ struct s3c24xx_i2c {
>   struct s3c2410_platform_i2c *pdata;
>   int gpios[2];
>   struct pinctrl  *pctrl;
> -#ifdef CONFIG_CPU_FREQ
> +#if defined(CONFIG_CPU_FREQ) && !defined(CONFIG_ARCH_EXYNOS)

...this is not a good coding practice, especially when already having
multiplatform kernels in sight.

The best way would be to check on which SoC we are running at runtime,
but since this might need changing a lot of code, then at least I would
change this from !defined(EXYNOS) to defined(S3C24XX), so it is not being
compiled in when S3C24XX support is not enabled and if it's enabled then
the notifier will be registered as a safe fallback that will run correctly
on all platforms.

Best regards,
Tomasz

--
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: Fix modalias for ACPI enumerated I2C devices

2013-10-11 Thread Rafael J. Wysocki 
On Friday, October 11, 2013 05:49:46 PM Mika Westerberg wrote:
> +Rafael
> 
> On Thu, Oct 10, 2013 at 05:17:49PM +0300, Jarkko Nikula wrote:
> > There is a minor fault about ACPI enumerated I2C devices with their modalias
> > attribute. Now modalias is set by device instance not by hardware ID.
> > For example "i2c:INTABCD:00", "i2c:INTABCD:01" etc.
> 
> I think that this is intentional. We don't want that the i2c modalias
> matches with the ACPI device (like with the i2c:INTABCD). Instead use ACPI
> IDs that are added to the driver to match with the ACPI device.

Well, I'm not really sure this was intentional, but I wonder how other bus
types work in that respect?

> > This means each device instance gets different modalias which does match
> > with generated modules.alias. Currently this is not problem as matching can
> > happen also with "acpi:INTABCD" modalias.
> > 
> > Fix this by using ACPI hardware ID.
> > 
> > Signed-off-by: Jarkko Nikula 
> > Cc: Mika Westerberg 
> > ---
> > Generated on top of v3.12-rc4-29-g0e7a3ed.
> > ---
> >  drivers/i2c/i2c-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 29d3f04..6dd0c53 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -,7 +,7 @@ static acpi_status acpi_i2c_add_device(acpi_handle 
> > handle, u32 level,
> > if (ret < 0 || !info.addr)
> > return AE_OK;
> >  
> > -   strlcpy(info.type, dev_name(&adev->dev), sizeof(info.type));
> > +   strlcpy(info.type, acpi_device_hid(adev), sizeof(info.type));
> > if (!i2c_new_device(adapter, &info)) {
> > dev_err(&adapter->dev,
> > "failed to add I2C device %s from ACPI\n",
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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: Fix modalias for ACPI enumerated I2C devices

2013-10-11 Thread Mika Westerberg 
+Rafael

On Thu, Oct 10, 2013 at 05:17:49PM +0300, Jarkko Nikula wrote:
> There is a minor fault about ACPI enumerated I2C devices with their modalias
> attribute. Now modalias is set by device instance not by hardware ID.
> For example "i2c:INTABCD:00", "i2c:INTABCD:01" etc.

I think that this is intentional. We don't want that the i2c modalias
matches with the ACPI device (like with the i2c:INTABCD). Instead use ACPI
IDs that are added to the driver to match with the ACPI device.

> This means each device instance gets different modalias which does match
> with generated modules.alias. Currently this is not problem as matching can
> happen also with "acpi:INTABCD" modalias.
> 
> Fix this by using ACPI hardware ID.
> 
> Signed-off-by: Jarkko Nikula 
> Cc: Mika Westerberg 
> ---
> Generated on top of v3.12-rc4-29-g0e7a3ed.
> ---
>  drivers/i2c/i2c-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 29d3f04..6dd0c53 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -,7 +,7 @@ static acpi_status acpi_i2c_add_device(acpi_handle 
> handle, u32 level,
>   if (ret < 0 || !info.addr)
>   return AE_OK;
>  
> - strlcpy(info.type, dev_name(&adev->dev), sizeof(info.type));
> + strlcpy(info.type, acpi_device_hid(adev), sizeof(info.type));
>   if (!i2c_new_device(adapter, &info)) {
>   dev_err(&adapter->dev,
>   "failed to add I2C device %s from ACPI\n",
> -- 
> 1.8.4.rc3
--
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

[PATCH 1/2] i2c-mux-gpio: use gpio_set_value_cansleep()

2013-10-11 Thread Ionut Nicu 
Some gpio chips may have get/set operations that
can sleep. gpio_set_value() only works for chips
which do not sleep, for the others we will get a
kernel warning. Using gpio_set_value_cansleep()
will work for both chips that do sleep and those
who don't.

Signed-off-by: Ionut Nicu 
---
 drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index a764da7..4ad9e71 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -30,8 +30,8 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, 
unsigned val)
int i;
 
for (i = 0; i < mux->data.n_gpios; i++)
-   gpio_set_value(mux->gpio_base + mux->data.gpios[i],
-  val & (1 << i));
+   gpio_set_value_cansleep(mux->gpio_base + mux->data.gpios[i],
+   val & (1 << i));
 }
 
 static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan)
-- 
1.7.1

--
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: exynos5: add High Speed I2C controller driver

2013-10-11 Thread Naveen Krishna Ch 
On 8 September 2013 22:33, Wolfram Sang  wrote:
>
> On Wed, Aug 21, 2013 at 02:54:37PM +0530, Naveen Krishna Ch wrote:
>> Adds support for High Speed I2C driver found in Exynos5 and
>> later SoCs from Samsung.
>>
>> Highspeed mode is a minor change in the i2c protocol.
>> Starts with
>> 1. start condition,
>> 2. 8-bit master ID code (1xxx)
>> 3. followed by a NACK bit
>> Once the above conditions are met, the bus is now operates in highspeed mode.
>> The rest of the I2C protocol applies the same.
>
> The description is correct, but it is not a change in the protocol. This
> is fully specified in the I2C specs.
Understood
>
>> Driver only supports Device Tree method.
>>
>> Changes since v1:
>> 1. Added FIFO functionality
>> 2. Added High speed mode functionality
>> 3. Remove SMBUS_QUICK
>> 4. Remove the debugfs functionality
>> 5. Use devm_* functions where ever possible
>> 6. Driver is free from GPIO configs
>> 7. Use OF data string "clock-frequency" to get the bus operating frequencies
>> 8. Split the clock divisor calculation function
>> 9. Add resets for the failed transacton cases
>> 10. Removed retries as core does retries if -EAGAIN is returned
>> 11. Removed mode from device tree info (use speed to distinguish
>> the mode of operation)
>> 12. Use wait_for_completion_timeout as the interruptible case is not tested 
>> well
>> 13. few other bug fixes and cosmetic changes
>> 14. Removed the untested runtime_pm code
>> 15. Removed the retries as core does that
>> 16. Use adap.nr instead of alias
>> 17. Added spinlocks around the irq code
>> 18. Use i2c_add_numbered_adapter() instead of using aliases
>
> Changes since v1 are irrelevant for the patch description.
Will remove
>
>>
>> Signed-off-by: Taekgyun Ko 
>> Signed-off-by: Naveen Krishna Chatradhi 
>> Reviewed-by: Simon Glass 
>> Tested-by: Andrew Bresticker 
>> Signed-off-by: Yuvaraj Kumar C D 
>> Signed-off-by: Andrew Bresticker 
>>
>> ---
>> Wolfram and Thomas Figa thanks for reviewing the code.
>>
>> Changes since v10:
>> 1. Remove the incomplete runtime_pm code
>> 2. Correct the error checks as suggested by Thomas
>> 3. Use i2c_add_numbered_adapter() as suggested
>> 4. Modified the irq routine to handle the specific interrupts
>> 5. Added spinlocks around the irq code
>> 6. Remove the "mode" of operation field from device tree node and use the
>>clock-frequency to decide the mode.
>>
>>  .../devicetree/bindings/i2c/i2c-exynos5.txt|   44 ++
>>  drivers/i2c/busses/Kconfig |7 +
>>  drivers/i2c/busses/Makefile|1 +
>>  drivers/i2c/busses/i2c-exynos5.c   |  799 
>> 
>>  4 files changed, 851 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
>>  create mode 100644 drivers/i2c/busses/i2c-exynos5.c
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt 
>> b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
>> new file mode 100644
>> index 000..805e018
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
>> @@ -0,0 +1,44 @@
>> +* Samsung's High Speed I2C controller
>> +
>> +The Samsung's High Speed I2C controller is used to interface with I2C 
>> devices
>> +at various speeds ranging from 100khz to 3.4Mhz.
>> +
>> +Required properties:
>> +  - compatible: value should be.
>> +  -> "samsung,exynos5-hsi2c", for i2c compatible with exynos5 hsi2c.
>> +  - reg: physical base address of the controller and length of memory mapped
>> +region.
>> +  - interrupts: interrupt number to the cpu.
>> +  - #address-cells: always 1 (for i2c addresses)
>> +  - #size-cells: always 0
>> +
>> +  - Pinctrl:
>> +- pinctrl-0: Pin control group to be used for this controller.
>> +- pinctrl-names: Should contain only one value - "default".
>> +
>> +Optional properties:
>> +  - clock-frequency: Desired operating frequency in Hz of the bus.
>> +-> If not specified, the default value is 100khz in fast-speed mode and
>> +   1Mhz in high-speed mode.
>
> Description doesn't match the current code.
Will correct
>
>> +-> If specified, The bus operates in high-speed mode only if the
>> +   clock-frequency is >= 1Mhz.
>
> s/The/the/
Will correct
>
> ...
>
>> +/*
>> + * exynos5_i2c_init: configures the controller for I2C functionality
>> + * Programs I2C controller for Master mode operation
>> + */
>> +static void exynos5_i2c_init(struct exynos5_i2c *i2c)
>> +{
>> + u32 i2c_conf = readl(i2c->regs + HSI2C_CONF);
>> + u32 i2c_timeout = readl(i2c->regs + HSI2C_TIMEOUT);
>> +
>> + /* Disable timeout */
>> + i2c_timeout &= ~HSI2C_TIMEOUT_EN;
>> + writel(i2c_timeout, i2c->regs + HSI2C_TIMEOUT);
>
> Just curious: Can't you use HSI2C_TIMEOUT and wait_for_completion
> instead of wait_for_completion_timeout? If so, it might save you a
> little bit of overhead.
With timeout enabled, few transactions were timing out.
wait_for_completion_timeout seems

[PATCH] i2c: s3c2410: dont need CPU_FREQ transitions for exynos series

2013-10-11 Thread Naveen Krishna Chatradhi 
The exynos5 i2c clock is based on a fixed 66 MHz peripheral clock, and
therefore is completely independent of the cpu frequency.
Thus, registering for a CPU freq notifier is very wasteful.

This patch modifes the code such that, i2c bus registers to
cpu_freq_transition only for non Exynos SoCs.

This change should save a bunch of cpufreq transitions calls
which does not apply to exynos SoCs.

Signed-off-by: Naveen Krishna Chatradhi 
---
 drivers/i2c/busses/i2c-s3c2410.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index cab1c91..d062fa7 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -123,7 +123,7 @@ struct s3c24xx_i2c {
struct s3c2410_platform_i2c *pdata;
int gpios[2];
struct pinctrl  *pctrl;
-#ifdef CONFIG_CPU_FREQ
+#if defined(CONFIG_CPU_FREQ) && !defined(CONFIG_ARCH_EXYNOS)
struct notifier_block   freq_transition;
 #endif
 };
@@ -843,7 +843,7 @@ static int s3c24xx_i2c_clockrate(struct s3c24xx_i2c *i2c, 
unsigned int *got)
return 0;
 }
 
-#ifdef CONFIG_CPU_FREQ
+#if defined(CONFIG_CPU_FREQ) && !defined(CONFIG_ARCH_EXYNOS)
 
 #define freq_to_i2c(_n) container_of(_n, struct s3c24xx_i2c, freq_transition)
 
-- 
1.7.9.5

--
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 1/2] i2c-mux-gpio: use gpio_set_value_cansleep()

2013-10-11 Thread Peter Korsgaard 
> "IN" == Ionut Nicu  writes:

IN> Some gpio chips may have get/set operations that
IN> can sleep. gpio_set_value() only works for chips
IN> which do not sleep, for the others we will get a
IN> kernel warning. Using gpio_set_value_cansleep()
IN> will work for both chips that do sleep and those
IN> who don't.

IN> Signed-off-by: Ionut Nicu 
IN> ---
IN>  drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
IN>  1 files changed, 2 insertions(+), 2 deletions(-)

IN> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c 
b/drivers/i2c/muxes/i2c-mux-gpio.c
IN> index a764da7..550e094 100644
IN> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
IN> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
IN> @@ -30,8 +30,8 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, 
unsigned val)
IN> int i;

IN> for (i = 0; i < mux->data.n_gpios; i++)
IN> -   gpio_set_value(mux->gpio_base + mux->data.gpios[i],
IN> -  val & (1 << i));
IN> +   gpio_set_value_cansleep(mux->gpio_base + mux->data.gpios[i],
IN> +   val & (1 << i));

The indentation of the 2nd line seems wrong (should match
mux->gpio_base), otherwise it looks good:

Acked-by: Peter Korsgaard 

--
Sorry about disclaimer - It's out of my control.
Bye, Peter Korsgaard
This message is subject to the following terms and conditions: MAIL 
DISCLAIMER
--
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

[PATCH 2/2] i2c-mux-gpio: use reg value for i2c_add_mux_adapter

2013-10-11 Thread Ionut Nicu 
The i2c-mux driver requires that the chan_id parameter
passed to the i2c_add_mux_adapter() function is equal
to the reg value for that adapter:

for_each_child_of_node(mux_dev->of_node, child) {
ret = of_property_read_u32(child, "reg", ®);
if (ret)
continue;
if (chan_id == reg) {
priv->adap.dev.of_node = child;
break;
}
}

The i2c-mux-gpio driver uses an internal logical index
for chan_id when calling i2c_add_mux_adapter() instead
of using the reg value.

Because of this, there will problems in selecting the
right adapter when the i2c-mux-gpio's index into
mux->data.values doesn't match the reg value.

An example of such a case:

mux->data.values = { 1, 0 }

For chan_id = 0, i2c-mux will bind the adapter to the
of_node with reg = <0>, but when it will call the
select() callback with chan_id set to 0, the i2c-mux-gpio
will use it as an index into mux->data.values and it will
actually select the bus with reg = <1>.

Signed-off-by: Ionut Nicu 
---
 drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 550e094..ed3e849 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -38,7 +38,7 @@ static int i2c_mux_gpio_select(struct i2c_adapter *adap, void 
*data, u32 chan)
 {
struct gpiomux *mux = data;
 
-   i2c_mux_gpio_set(mux, mux->data.values[chan]);
+   i2c_mux_gpio_set(mux, chan);
 
return 0;
 }
@@ -228,7 +228,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
unsigned int class = mux->data.classes ? mux->data.classes[i] : 
0;
 
mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
-  i, class,
+  mux->data.values[i], class,
   i2c_mux_gpio_select, 
deselect);
if (!mux->adap[i]) {
ret = -ENODEV;
-- 
1.7.1

--
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

[PATCH 1/2] i2c-mux-gpio: use gpio_set_value_cansleep()

2013-10-11 Thread Ionut Nicu 
Some gpio chips may have get/set operations that
can sleep. gpio_set_value() only works for chips
which do not sleep, for the others we will get a
kernel warning. Using gpio_set_value_cansleep()
will work for both chips that do sleep and those
who don't.

Signed-off-by: Ionut Nicu 
---
 drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index a764da7..550e094 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -30,8 +30,8 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, 
unsigned val)
int i;
 
for (i = 0; i < mux->data.n_gpios; i++)
-   gpio_set_value(mux->gpio_base + mux->data.gpios[i],
-  val & (1 << i));
+   gpio_set_value_cansleep(mux->gpio_base + mux->data.gpios[i],
+   val & (1 << i));
 }
 
 static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan)
-- 
1.7.1

--
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 2/2] i2c-mux-gpio: eliminate i2c channel order assumptions

2013-10-11 Thread Ionut Nicu 
Hi,

On 10.10.2013 12:34, Alexander Sverdlin wrote:
> Hi!
> 
> On 10/10/2013 10:39 AM, Ionut Nicu wrote:
>> The i2c-mux driver uses the chan_id parameter provided
>> in i2c_add_mux_adapter as a parameter to the select
>> and deselect callbacks while the i2c-mux-gpio driver
>> uses the chan_id as an index in the mux->data.values
>> array.
>>
>> A simple example of where this doesn't work is when we
>> have a device tree like this:
>>
>> i2cmux {
>>  i2c@1 {
>>  reg = <1>;
>>  ...
>>  };
>>
>>  i2c@0 {
>>  reg = <0>;
>>  ...
>>  };
>> };
>>
>> The mux->data.values array will be { 1, 0 }, but when
>> the i2-mux driver will try to select channel 0, the
>> i2c-mux-gpio driver will actually use values[0], hence 1
>> as the gpio selection value.
> 
> The patch itself is correct, but the description is not precise,
> I suppose... i2c-mux-gpio is consistent inside itself, it will
> receive for every child adapter the value it has configured.
> The problem happens inside i2c-mux.c, i2c_add_mux_adapter():
> 
>   for_each_child_of_node(mux_dev->of_node, child) {
>   ret = of_property_read_u32(child, "reg", ®);
>   if (ret)
>   continue;
>   if (chan_id == reg) {
>   priv->adap.dev.of_node = child;
> 
> Which means, i2c-mux-gpio MUST pass reg, not its logical index inside
> array. Otherwise node will not be correctly assigned and i2c-mux will
> have problems selecting right adapter for the multiplexed devices.
> 
>> Signed-off-by: Ionut Nicu 
> 
> So, for the code itself
> 
> Acked-by: Alexander Sverdlin 
> 

You are right, the patch description is not so good. I will try to change
it so it's clearer for everyone what I'm trying to fix here and after that
I will re-submit the series.

>> ---
>>  drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c 
>> b/drivers/i2c/muxes/i2c-mux-gpio.c
>> index b5f17ef..3505d0e 100644
>> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
>> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
>> @@ -43,7 +43,7 @@ static int i2c_mux_gpio_select(struct i2c_adapter *adap, 
>> void *data, u32 chan)
>>  {
>>  struct gpiomux *mux = data;
>>  
>> -i2c_mux_gpio_set(mux, mux->data.values[chan]);
>> +i2c_mux_gpio_set(mux, chan);
>>  
>>  return 0;
>>  }
>> @@ -233,7 +233,7 @@ static int i2c_mux_gpio_probe(struct platform_device 
>> *pdev)
>>  unsigned int class = mux->data.classes ? mux->data.classes[i] : 
>> 0;
>>  
>>  mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
>> -   i, class,
>> +   mux->data.values[i], class,
>> i2c_mux_gpio_select, 
>> deselect);
>>  if (!mux->adap[i]) {
>>  ret = -ENODEV;
>>
> 

--
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