Re: [PATCH] i2c: exynos5: register driver early in subsys initcall to avoid probe defer

[Wolfram Sang]

Hi Marek,

> This problem is known for ages, there were number of attempts to add

Yes, I heard of it, too.

> support for deferred probe or solve the problem of deferred UDC driver
> probe, but none got accepted so far:

Then they need to be worked on. AFAIK the rejection is based on
implementation details, not because the feature itself is not wanted,
or?

> Till now it worked somehow for boards with older i2c bus drivers, which
> were registered in subsys initcall. Boards, which use newer i2c bus drivers
> cannot be used with usb gadgets compiled-in.

Yes, because the above issue needs to be *fixed*, not workarounded. You
still have the option to carry your patch out-of-tree if fixing the
issue is not an option for you.

We already had problems here when someone tried to revert
subsys_initcall to module_init because of other dependency issues on
another particular board. So, the pain should really go where the fixing
is needed.

I hope you understand,

   Wolfram



signature.asc
Description: Digital signature

Re: [PATCH] i2c: exynos5: register driver early in subsys initcall to avoid probe defer

[Wolfram Sang]

On Fri, Oct 30, 2015 at 03:23:28PM +0100, Marek Szyprowski wrote:
> i2c bus drivers should be registered earlier than typical drivers to let
> important i2c devices (like PMICs/regulators) get registered early
> enough to avoid probe defer of all devices in typical embedded system.
> 
> Signed-off-by: Marek Szyprowski 

I don't accept such patches anymore. Some more background:

http://www.spinics.net/lists/linux-i2c/msg13102.html
http://www.spinics.net/lists/linux-i2c/msg16644.html



signature.asc
Description: Digital signature

[PATCH v3] i2c: s3c2410: enable RuntimePM before registering to the core

[Wolfram Sang]

From: Wolfram Sang <wsa+rene...@sang-engineering.com>

The core may register clients attached to this master which may use
funtionality from the master. So, RuntimePM must be enabled before, otherwise
this will fail. While here, move drvdata, too.

Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
---

Changes since v2: don't call runtime pm on adaper if it wasn't registered

Thanks to Krzysztof for testing!

 drivers/i2c/busses/i2c-s3c2410.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 50bfd8cef5f224..5df819610d5280 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1243,17 +1243,19 @@ static int s3c24xx_i2c_probe(struct platform_device 
*pdev)
i2c->adap.nr = i2c->pdata->bus_num;
i2c->adap.dev.of_node = pdev->dev.of_node;
 
+   platform_set_drvdata(pdev, i2c);
+
+   pm_runtime_enable(>dev);
+
ret = i2c_add_numbered_adapter(>adap);
if (ret < 0) {
dev_err(>dev, "failed to add bus to i2c core\n");
+   pm_runtime_disable(>dev);
s3c24xx_i2c_deregister_cpufreq(i2c);
clk_unprepare(i2c->clk);
return ret;
}
 
-   platform_set_drvdata(pdev, i2c);
-
-   pm_runtime_enable(>dev);
pm_runtime_enable(>adap.dev);
 
dev_info(>dev, "%s: S3C I2C adapter\n", dev_name(>adap.dev));
-- 
2.1.4

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

Re: [PATCH v2 3/4] i2c: s3c2410: enable RuntimePM before registering to the core

[Wolfram Sang]

On Sat, Oct 10, 2015 at 03:08:52PM +0900, Krzysztof Kozlowski wrote:
> W dniu 09.10.2015 o 18:39, Wolfram Sang pisze:
> > From: Wolfram Sang <wsa+rene...@sang-engineering.com>
> > 
> > The core may register clients attached to this master which may use
> > funtionality from the master. So, RuntimePM must be enabled before, 
> > otherwise
> > this will fail. While here, move drvdata, too.
> > 
> > Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
> > ---
> >  drivers/i2c/busses/i2c-s3c2410.c | 12 +++-
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-s3c2410.c 
> > b/drivers/i2c/busses/i2c-s3c2410.c
> > index 50bfd8cef5f224..336e34e1ca597d 100644
> > --- a/drivers/i2c/busses/i2c-s3c2410.c
> > +++ b/drivers/i2c/busses/i2c-s3c2410.c
> > @@ -1243,19 +1243,21 @@ static int s3c24xx_i2c_probe(struct platform_device 
> > *pdev)
> > i2c->adap.nr = i2c->pdata->bus_num;
> > i2c->adap.dev.of_node = pdev->dev.of_node;
> >  
> > +   platform_set_drvdata(pdev, i2c);
> > +
> > +   pm_runtime_enable(>dev);
> > +   pm_runtime_enable(>adap.dev);
> > +
> > ret = i2c_add_numbered_adapter(>adap);
> > if (ret < 0) {
> > dev_err(>dev, "failed to add bus to i2c core\n");
> > +   pm_runtime_disable(>adap.dev);
> > +   pm_runtime_disable(>dev);
> > s3c24xx_i2c_deregister_cpufreq(i2c);
> > clk_unprepare(i2c->clk);
> > return ret;
> > }
> >  
> > -   platform_set_drvdata(pdev, i2c);
> > -
> > -   pm_runtime_enable(>dev);
> > -   pm_runtime_enable(>adap.dev);
> > -
> > dev_info(>dev, "%s: S3C I2C adapter\n", dev_name(>adap.dev));
> > return 0;
> >  }
> > 
> 
> Hi,
> 
> Unfortunately on Odroid XU3, Exynos5420, samsung,s3c2440-i2c,
> next-20151009-1-ga93b1a6f5055:

Thanks for testing! Will send v3 in a minute.



signature.asc
Description: Digital signature

[PATCH v2 3/4] i2c: s3c2410: enable RuntimePM before registering to the core

[Wolfram Sang]

From: Wolfram Sang <wsa+rene...@sang-engineering.com>

The core may register clients attached to this master which may use
funtionality from the master. So, RuntimePM must be enabled before, otherwise
this will fail. While here, move drvdata, too.

Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
---
 drivers/i2c/busses/i2c-s3c2410.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 50bfd8cef5f224..336e34e1ca597d 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1243,19 +1243,21 @@ static int s3c24xx_i2c_probe(struct platform_device 
*pdev)
i2c->adap.nr = i2c->pdata->bus_num;
i2c->adap.dev.of_node = pdev->dev.of_node;
 
+   platform_set_drvdata(pdev, i2c);
+
+   pm_runtime_enable(>dev);
+   pm_runtime_enable(>adap.dev);
+
ret = i2c_add_numbered_adapter(>adap);
if (ret < 0) {
dev_err(>dev, "failed to add bus to i2c core\n");
+   pm_runtime_disable(>adap.dev);
+   pm_runtime_disable(>dev);
s3c24xx_i2c_deregister_cpufreq(i2c);
clk_unprepare(i2c->clk);
return ret;
}
 
-   platform_set_drvdata(pdev, i2c);
-
-   pm_runtime_enable(>dev);
-   pm_runtime_enable(>adap.dev);
-
dev_info(>dev, "%s: S3C I2C adapter\n", dev_name(>adap.dev));
return 0;
 }
-- 
2.1.4

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

[PATCH 3/4] i2c: s3c2410: enable RuntimePM before registering to the core

[Wolfram Sang]

From: Wolfram Sang <wsa+rene...@sang-engineering.com>

The core may register clients attached to this master which may use
funtionality from the master. So, RuntimePM must be enabled before, otherwise
this will fail. While here, move drvdata, too.

Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com>
---

Not tested on HW. Found by code review.

 drivers/i2c/busses/i2c-s3c2410.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 50bfd8cef5f224..75f76cd644728a 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1243,6 +1243,11 @@ static int s3c24xx_i2c_probe(struct platform_device 
*pdev)
i2c->adap.nr = i2c->pdata->bus_num;
i2c->adap.dev.of_node = pdev->dev.of_node;
 
+   platform_set_drvdata(pdev, i2c);
+
+   pm_runtime_enable(>dev);
+   pm_runtime_enable(>adap.dev);
+
ret = i2c_add_numbered_adapter(>adap);
if (ret < 0) {
dev_err(>dev, "failed to add bus to i2c core\n");
@@ -1251,11 +1256,6 @@ static int s3c24xx_i2c_probe(struct platform_device 
*pdev)
return ret;
}
 
-   platform_set_drvdata(pdev, i2c);
-
-   pm_runtime_enable(>dev);
-   pm_runtime_enable(>adap.dev);
-
dev_info(>dev, "%s: S3C I2C adapter\n", dev_name(>adap.dev));
return 0;
 }
-- 
2.1.4

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

Re: [PATCH V2] i2c: s3c2410: remove unused variable

[Wolfram Sang]

 @@ -565,7 +564,7 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, 
 unsigned long iicstat)
   tmp = ~S3C2410_IICCON_IRQPEND;
   writel(tmp, i2c-regs + S3C2410_IICCON);
   out:
 - return ret;
 + return;

What about removing out: and return as well?



signature.asc
Description: Digital signature

Re: [PATCH] i2c: s3c2410: remove unused variable

[Wolfram Sang]

On Thu, Jul 16, 2015 at 10:36:16PM +0200, Laurent Navet wrote:
 ret is assigned zero and just used to return. remove it.
 Found by using coccinelle.
 
 Signed-off-by: Laurent Navet laurent.na...@gmail.com

Why not make it void?



signature.asc
Description: Digital signature

Re: [PATCH v2] i2c: Mark adapter devices with pm_runtime_no_callbacks

[Wolfram Sang]

On Thu, Apr 16, 2015 at 01:05:19PM +0100, Charles Keepax wrote:
 Commit 523c5b89640e (i2c: Remove support for legacy PM) removed the PM
 ops from the bus type, which causes the pm operations on the s3c2410
 adapter device to fail (-ENOSUPP in rpm_callback). The adapter device
 doesn't get bound to a driver and as such can't have its own pm_runtime
 callbacks. Previously this was fine as the bus callbacks would have been
 used, but now this can cause devices which use PM runtime and are
 attached over I2C to fail to resume.
 
 This commit fixes this issue by marking all adapter devices with
 pm_runtime_no_callbacks, since they can't have any.
 
 Signed-off-by: Charles Keepax ckee...@opensource.wolfsonmicro.com

Applied to for-current, thanks!

Will push out somewhat later today. Lars-Peter, if you'd like to donate
some ack/review tag until then, this would be much appreciated.



signature.asc
Description: Digital signature

Re: [PATCH v2] i2c: Mark adapter devices with pm_runtime_no_callbacks

[Wolfram Sang]

On Thu, Apr 16, 2015 at 02:28:02PM +0200, Beata Michalska wrote:
 On 04/16/2015 02:05 PM, Charles Keepax wrote:
  Commit 523c5b89640e (i2c: Remove support for legacy PM) removed the PM
  ops from the bus type, which causes the pm operations on the s3c2410
  adapter device to fail (-ENOSUPP in rpm_callback). The adapter device
  doesn't get bound to a driver and as such can't have its own pm_runtime
  callbacks. Previously this was fine as the bus callbacks would have been
  used, but now this can cause devices which use PM runtime and are
  attached over I2C to fail to resume.
 
  This commit fixes this issue by marking all adapter devices with
  pm_runtime_no_callbacks, since they can't have any.
 
  Signed-off-by: Charles Keepax ckee...@opensource.wolfsonmicro.com
 
 Thereis at least one more casewhen an I2C device fails to resume due
 to bus callbacks being dropped(like fimc-is-i2c driver for exynos4-is,
 the same scenario),  so I'm glad to see  it's being handled in on place
 -it saves the trouble.

Thanks for the confirmation! If you could also add a formal 'Acked-by'
next time, this would help me because patchwork collects them for me
automatically.



signature.asc
Description: Digital signature

Re: [PATCH] i2c: s3c2410: fix ABBA deadlock by keeping clock prepared

[Wolfram Sang]

On Mon, Jan 19, 2015 at 05:03:33PM +0100, Paul Osmialowski wrote:
 This patch solves deadlock between clock prepare mutex and regmap mutex 
 reported
 by Tomasz Figa in [1] by implementing solution from [2]: always leave the 
 clock
 of the i2c controller in a prepared state.
 
 [1] https://lkml.org/lkml/2014/7/2/171
 [2] https://lkml.org/lkml/2014/7/2/207
 
 On each i2c transfer handled by s3c24xx_i2c_xfer(), clk_prepare_enable() was
 called, which calls clk_prepare() then clk_enable(). clk_prepare() takes
 prepare_lock mutex before proceeding. Note that i2c transfer functions are
 invoked from many places in kernel, typically with some other additional lock
 held.
 
 It may happen that function on CPU1 (e.g. regmap_update_bits()) has taken a
 mutex (i.e. regmap lock mutex) then it attempts i2c communication in order to
 proceed (so it needs to obtain clock related prepare_lock mutex during 
 transfer
 preparation stage due to clk_prepare() call). At the same time other task on
 CPU0 wants to operate on clock (e.g. to (un)prepare clock for some other 
 reason)
 so it has taken prepare_lock mutex.
 
 CPU0:CPU1:
 clk_disable_unused() regulator_disable()
   clk_prepare_lock()   map-lock(map-lock_arg)
   regmap_read()s3c24xx_i2c_xfer()
 map-lock(map-lock_arg) clk_prepare_lock()
 
 Implemented solution from [2] leaves i2c clock prepared. Preparation is done 
 in
 s3c24xx_i2c_probe() function. Without this patch, it is immediately unprepared
 by clk_disable_unprepare() call. I've replaced this call with clk_disable() 
 and
 I've added clk_unprepare() call in s3c24xx_i2c_remove().
 
 The s3c24xx_i2c_xfer() function now uses clk_enable() instead of
 clk_prepare_enable() (and clk_disable() instead of clk_unprepare_disable()).
 
 Signed-off-by: Paul Osmialowski p.osmialo...@samsung.com

Applied to for-current, thanks!



signature.asc
Description: Digital signature

[PATCH 27/28] ASoC: samsung: drop owner assignment from platform_drivers

[Wolfram Sang]

This platform_driver does not need to set an owner, it will be populated by the
driver core.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
Generated with coccinelle. SmPL file is in the introductory msg. The big
cleanup was pulled in this merge window. This series catches the bits fallen
through. The patches shall go in via the subsystem trees.

 sound/soc/samsung/arndale_rt5631.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/samsung/arndale_rt5631.c 
b/sound/soc/samsung/arndale_rt5631.c
index 1e2b61ca8db2..8bf2e2c4bafb 100644
--- a/sound/soc/samsung/arndale_rt5631.c
+++ b/sound/soc/samsung/arndale_rt5631.c
@@ -135,7 +135,6 @@ MODULE_DEVICE_TABLE(of, samsung_arndale_rt5631_of_match);
 static struct platform_driver arndale_audio_driver = {
.driver = {
.name   = arndale-audio,
-   .owner  = THIS_MODULE,
.pm = snd_soc_pm_ops,
.of_match_table = of_match_ptr(samsung_arndale_rt5631_of_match),
},
-- 
2.1.3

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

[PATCH 00/28] remove .owner for most platform_drivers: the missing bits

[Wolfram Sang]

Generated with coccinelle. The big cleanup was pulled in this merge window.
This series catches the bits fallen through. The patches shall go in via the
subsystem trees. If possible for 3.19 to increase consistency I'd say, but you
decide, of course.

cocci-file used:

@match1@
declarer name module_platform_driver;
declarer name module_platform_driver_probe;
declarer name for_each_node_by_type;
identifier __driver;
@@
(
module_platform_driver(__driver);
|
module_platform_driver_probe(__driver, ...);
)

@fix1 depends on match1@
identifier match1.__driver;
@@
static struct platform_driver __driver = {
.driver = {
-   .owner = THIS_MODULE,
}
};

@match2@
identifier __driver;
@@
(
platform_driver_register(__driver)
|
platform_driver_probe(__driver, ...)
|
platform_create_bundle(__driver, ...)
)

@fix2 depends on match2@
identifier match2.__driver;
@@
static struct platform_driver __driver = {
.driver = {
-   .owner = THIS_MODULE,
}
};

Thanks again to Julia Lawall for support. And hey, we fixed a coccinelle bug on
the way :)


Wolfram Sang (28):
  ARM: mach-exynos: drop owner assignment from platform_drivers
  mips: lantiq: xway: drop owner assignment from platform_drivers
  mips: pci: drop owner assignment from platform_drivers
  char: ipmi: drop owner assignment from platform_drivers
  cpufreq: drop owner assignment from platform_drivers
  dma: drop owner assignment from platform_drivers
  gpio: drop owner assignment from platform_drivers
  gpu: drm: rockchip: drop owner assignment from platform_drivers
  iommu: drop owner assignment from platform_drivers
  net: ethernet: stmicro: stmmac: drop owner assignment from
platform_drivers
  net: wireless: ath: ath5k: drop owner assignment from platform_drivers
  of: drop owner assignment from platform_drivers
  pci: host: drop owner assignment from platform_drivers
  phy: drop owner assignment from platform_drivers
  pinctrl: intel: drop owner assignment from platform_drivers
  rtc: drop owner assignment from platform_drivers
  scsi: drop owner assignment from platform_drivers
  thermal: drop owner assignment from platform_drivers
  thermal: int340x_thermal: drop owner assignment from platform_drivers
  tty: serial: 8250: drop owner assignment from platform_drivers
  usb: gadget: udc: bdc: drop owner assignment from platform_drivers
  watchdog: drop owner assignment from platform_drivers
  ASoC: intel: drop owner assignment from platform_drivers
  ASoC: intel: sst: drop owner assignment from platform_drivers
  ASoC: omap: drop owner assignment from platform_drivers
  ASoC: pxa: drop owner assignment from platform_drivers
  ASoC: samsung: drop owner assignment from platform_drivers
  macintosh: drop owner assignment from platform_drivers

 arch/arm/mach-exynos/pmu.c| 1 -
 arch/mips/lantiq/xway/vmmc.c  | 1 -
 arch/mips/pci/pci-ar2315.c| 1 -
 arch/mips/pci/pci-rt2880.c| 1 -
 drivers/char/ipmi/ipmi_powernv.c  | 1 -
 drivers/cpufreq/ls1x-cpufreq.c| 1 -
 drivers/dma/at_xdmac.c| 1 -
 drivers/gpio/gpio-vf610.c | 1 -
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c   | 1 -
 drivers/iommu/rockchip-iommu.c| 1 -
 drivers/macintosh/windfarm_pm112.c| 1 -
 drivers/macintosh/windfarm_pm72.c | 1 -
 drivers/macintosh/windfarm_rm31.c | 1 -
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
 drivers/net/wireless/ath/ath5k/ahb.c  | 1 -
 drivers/of/unittest.c | 1 -
 drivers/pci/host/pci-layerscape.c | 1 -
 drivers/phy/phy-armada375-usb2.c  | 1 -
 drivers/phy/phy-berlin-usb.c  | 1 -
 drivers/phy/phy-miphy28lp.c   | 1 -
 drivers/pinctrl/intel/pinctrl-cherryview.c| 1 -
 drivers/rtc/rtc-opal.c| 1 -
 drivers/scsi/atari_scsi.c | 1 -
 drivers/scsi/mac_scsi.c   | 1 -
 drivers/scsi/sun3_scsi.c  | 1 -
 drivers/thermal/int340x_thermal/int3400_thermal.c | 1 -
 drivers/thermal/int340x_thermal/int3402_thermal.c | 1 -
 drivers/thermal/rockchip_thermal.c| 1 -
 drivers/tty/serial/8250/8250_omap.c   | 1 -
 drivers/usb/gadget/udc/bdc/bdc_core.c | 1 -
 drivers/watchdog/cadence_wdt.c| 1 -
 drivers/watchdog/meson_wdt.c  | 1 -
 sound/soc/intel/bytcr_dpcm_rt5640.c   | 1 -
 sound/soc/intel/cht_bsw_rt5672.c  | 1 -
 sound/soc/intel

[PATCH 01/28] ARM: mach-exynos: drop owner assignment from platform_drivers

[Wolfram Sang]

This platform_driver does not need to set an owner, it will be populated by the
driver core.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
Generated with coccinelle. SmPL file is in the introductory msg. The big
cleanup was pulled in this merge window. This series catches the bits fallen
through. The patches shall go in via the subsystem trees.

 arch/arm/mach-exynos/pmu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index c15761ca2f18..f629d2df66b0 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -991,7 +991,6 @@ static int exynos_pmu_probe(struct platform_device *pdev)
 static struct platform_driver exynos_pmu_driver = {
.driver  = {
.name   = exynos-pmu,
-   .owner  = THIS_MODULE,
.of_match_table = exynos_pmu_of_device_ids,
},
.probe = exynos_pmu_probe,
-- 
2.1.3

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

Re: [PATCH v6] i2c: s3c2410: Handle i2c sys_cfg register in i2c driver

[Wolfram Sang]

On Mon, Nov 24, 2014 at 02:03:38PM +0530, Pankaj Dubey wrote:
 Let's handle i2c interrupt re-configuration in i2c driver. This will
 help us in removing some soc specific checks from machine files and
 will help in removing static iomapping of SYS register in exynos.c
 
 Also handle saving and restoring of SYS_I2C_CFG register during
 suspend and resume of i2c driver.
 
 CC: Wolfram Sang w...@the-dreams.de
 CC: linux-...@vger.kernel.org
 Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH v4 1/2] i2c: s3c2410: Handle i2c sys_cfg register in i2c driver

[Wolfram Sang]

 I usually don't take DTS patches. They should go via arm-soc. Please say
 so if there are reasons I should take them.
 
 I CC'ed to you because same patch contains changes in i2c driver.

Yes, those should absolutely go via my I2C tree. You need to make a
seperate patch out of the dts changes which then also should go via
samsung-soc, unless Kukjin says he really wants to go the via I2C. But I
guess the latter will just create merge conflicts.



signature.asc
Description: Digital signature

Re: [PATCH v4 1/2] i2c: s3c2410: Handle i2c sys_cfg register in i2c driver

[Wolfram Sang]

On Thu, Oct 30, 2014 at 01:34:29PM +0530, Pankaj Dubey wrote:
 Let's handle i2c interrupt re-configuration in i2c driver. This will
 help us in removing some soc specific checks from machine files and
 will help in removing static iomapping of SYS register in exynos.c
 
 Since only Exynos5250, and Exynos5420 has i2c nodes in DT, added syscon
 based phandle to i2c device nodes of respective SoC DT files.
 
 Also handle saving and restoring of SYS_I2C_CFG register during
 suspend and resume of i2c driver.
 
 CC: Rob Herring robh...@kernel.org
 CC: Randy Dunlap rdun...@infradead.org
 CC: Wolfram Sang w...@the-dreams.de
 CC: Russell King li...@arm.linux.org.uk
 CC: devicet...@vger.kernel.org
 CC: linux-...@vger.kernel.org
 CC: linux-...@vger.kernel.org
 Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com
 ---
  .../devicetree/bindings/i2c/i2c-s3c2410.txt|1 +
  arch/arm/boot/dts/exynos5250.dtsi  |4 +++
  arch/arm/boot/dts/exynos5420.dtsi  |4 +++

I usually don't take DTS patches. They should go via arm-soc. Please say
so if there are reasons I should take them.

 @@ -1084,6 +1092,23 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct 
 s3c24xx_i2c *i2c)
   of_property_read_u32(np, samsung,i2c-slave-addr, pdata-slave_addr);
   of_property_read_u32(np, samsung,i2c-max-bus-freq,
   (u32 *)pdata-frequency);
 + /*
 +  * Exynos5's legacy i2c controller and new high speed i2c
 +  * controller have muxed interrupt sources. By default the
 +  * interrupts for 4-channel HS-I2C controller are enabled.
 +  * If node for first four channels of legacy i2c controller

s/node/nodes/

 +  * are available then re-configure the interrupts via the
 +  * system register.
 +  */
 + id = of_alias_get_id(np, i2c);
 + i2c-sysreg = syscon_regmap_lookup_by_phandle(np,
 + samsung,sysreg-phandle);
 + if (IS_ERR(i2c-sysreg)) {
 + /* As this is not compulsory do not return error */
 + pr_info(i2c-%d skipping re-configuration of interrutps\n, id);

I'd say drop this message. If you want to keep it, it should be dev_dbg.

 + return;
 + }
 + regmap_update_bits(i2c-sysreg, EXYNOS5_SYS_I2C_CFG, BIT(id), 0);
  }

Rest looks good, thanks!


signature.asc
Description: Digital signature

Re: [PATCH] i2c: exynos5: use proper errno for timeout

[Wolfram Sang]

On Fri, Oct 03, 2014 at 01:57:14PM +0200, Wolfram Sang wrote:
 Signed-off-by: Wolfram Sang w...@the-dreams.de

Applied to for-next, thanks!



signature.asc
Description: Digital signature

[PATCH] i2c: exynos5: use proper errno for timeout

[Wolfram Sang]

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 drivers/i2c/busses/i2c-exynos5.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
index 81e6263cd7da..271533d564ec 100644
--- a/drivers/i2c/busses/i2c-exynos5.c
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -457,7 +457,7 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
goto stop;
} else if (int_status  HSI2C_INT_TIMEOUT) {
dev_dbg(i2c-dev, Accessing device timed out\n);
-   i2c-state = -EAGAIN;
+   i2c-state = -ETIMEDOUT;
goto stop;
}
} else if (int_status  HSI2C_INT_I2C) {
@@ -476,7 +476,7 @@ static irqreturn_t exynos5_i2c_irq(int irqno, void *dev_id)
goto stop;
} else if (trans_status  HSI2C_TIMEOUT_AUTO) {
dev_dbg(i2c-dev, Accessing device timed out\n);
-   i2c-state = -EAGAIN;
+   i2c-state = -ETIMEDOUT;
goto stop;
} else if (trans_status  HSI2C_TRANS_DONE) {
i2c-trans_done = 1;
-- 
2.0.0

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

Re: [PATCH 1/2] i2c: exynos: add support for HSI2C module on Exynos7

[Wolfram Sang]

On Tue, Sep 16, 2014 at 03:03:17PM +0530, Naveen Krishna Chatradhi wrote:
 The HSI2C module on Exynos7 differs in the transfer status
 bits. Transfer status bits were moved to INT_ENABLE and
 INT_STATUS registers
 
 This patch adds support for the HSI2C module on Exynos7.
 1. Implementes a hw field in the variant struct to distinguish
the hardware.
 2. Updates the dt-new compatible in dt-binding documenation
 
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Cc: Wolfram Sang w...@the-dreams.de

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH 1/2 v2] i2c: cros-ec-tunnel: Add of match table

[Wolfram Sang]

 Doug posted a patch [0] to fix the issue you are reporting but it was not
 picked. I tested that the patch applies on top of linux-next + the latest
 cros_ec cleanups (that includes a patch for this i2c driver and was taken
 by Lee. So I think is safe for you take Doug's patch through your tree
 since it seems to not cause any conflict with the mfd tree.

OK, thanks for the heads up. I picked it.



signature.asc
Description: Digital signature

Re: [PATCH v4 2/5] i2c: i2c-cros-ec-tunnel: Set retries to 3

[Wolfram Sang]

On Mon, Sep 29, 2014 at 08:50:14AM +0100, Lee Jones wrote:
 On Mon, 29 Sep 2014, Lee Jones wrote:
 
  On Thu, 18 Sep 2014, Javier Martinez Canillas wrote:
  
   From: Derek Basehore dbaseh...@chromium.org
   
   Since the i2c bus can get wedged on the EC sometimes, set the number of 
   retries
   to 3. Since we un-wedge the bus immediately after the wedge happens, this 
   is the
   correct fix since only one transfer will fail.
   
   Signed-off-by: Derek Basehore dbaseh...@chromium.org
   Reviewed-by: Doug Anderson diand...@chromium.org
   Acked-by: Wolfram Sang w...@the-dreams.de
   Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
   ---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 3 +++
1 file changed, 3 insertions(+)
  
  Applied, thanks.
 
 Wolfram,
 
 Do you want a pull-request?

Nope. All fine, there shouldn't be any conflict.



signature.asc
Description: Digital signature

Re: [PATCH 1/2 v2] i2c: cros-ec-tunnel: Add of match table

[Wolfram Sang]

On Fri, Sep 19, 2014 at 10:08:12AM +0200, Sjoerd Simons wrote:
 To enable the cros-ec-tunnel driver to be auto-loaded when build as a
 module add an of match table (and export it) to match the modalias
 information passed on to userspace as the Cros EC MFD driver registers
 the MFD subdevices with an of_compatibility string.
 
 Signed-off-by: Sjoerd Simons sjoerd.sim...@collabora.co.uk
 Reviewed-by: Javier Martinez Canillas javier.marti...@collabora.co.uk

Applied to for-next, thanks!

However, since you are working on this driver: sparse rightfully
complains about:

drivers/i2c/busses/i2c-cros-ec-tunnel.c:97:44: warning: cast truncates bits 
from constant value (1 becomes 0)

I don't know the EC protocol, can you check?



signature.asc
Description: Digital signature

Re: [PATCH] Input: atmel_mxt_ts: Add of node type to the i2c table

[Wolfram Sang]

Funny timing. I am just reviewing the series from Lee and also stumbled
over modaliases, too...

On Thu, Sep 11, 2014 at 10:19:54AM +0100, Nick Dyer wrote:
 On 11/09/14 09:38, Javier Martinez Canillas wrote:
  To expand on what Sjoerd already said and just to be sure everyone is on the
  same page.
  
  The problem is that right now the driver reports the following modalias:
  
  # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias
  i2c:maxtouch
  
  but if you look at the module information, that is not a valid alias:
  
  # modinfo atmel_mxt_ts | grep alias
  alias:  i2c:mXT224
  alias:  i2c:atmel_mxt_tp
  alias:  i2c:atmel_mxt_ts
  alias:  i2c:qt602240_ts
  alias:  of:N*T*Catmel,maxtouch*
  
  which means that udev/kmod can't load the module automatically based on the
  alias information.
  
  The aliases are filled by both MODULE_DEVICE_TABLE(i2c, mxt_id) and
  MODULE_DEVICE_TABLE(of, mxt_of_match) so after Sjoerd's patch:
  
  # cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias
  i2c:maxtouch
  
  # modinfo atmel_mxt_ts | grep alias
  alias:  i2c:mXT224
  alias:  i2c:maxtouch
  alias:  i2c:atmel_mxt_tp
  alias:  i2c:atmel_mxt_ts
  alias:  i2c:qt602240_ts
  
  which matches the reported uevent so the module will be auto-loaded.
  
  This is because the I2C subsystem hardcodes i2c:client-name, if you look 
  at
  drivers/i2c/i2c-core.c:
  
  /* uevent helps with hotplug: modprobe -q $(MODALIAS) */
  static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env 
  *env)
  {
  ...
  if (add_uevent_var(env, MODALIAS=%s%s,
 I2C_MODULE_PREFIX, client-name))
  ...
  }
  
  I've looked at Lee's series and AFAICT that remains the same so I second
  Sjoerd that module auto-loading will continue to be broken.

I think the above code in the i2c core needs a fix. Will have a closer
look after lunch.

 The i2c aliases are a bit confusing. The original device the driver was
 written for was called qt602240, which was renamed by Atmel to mXT224 when
 the chip series was called maXTouch. The driver now actually supports
 many other chips which aren't listed (more than 20 devices that I've
 personally tested). I could add them all, but it would be an extremely long
 list. It may be preferable to use the generic name maXTouch.

This is probably true for some more I2C devices. Like RTCs being
compatible or, most prominent, EEPROMS. I don't want to have a list of
all vendors producing 24c02s if they are all compatible. If generic
entries are frowned upon, I'd agree on a first come, first served policy:
Somebody provides one compatible-property with the vendor which happens
to be on that board, and the others have to reuse that
compatible-property since they are, well, compatible.

 So I think the sensible thing to do here would be to add maxtouch to the
 i2c list to fix the module autoload issue.

This is a workaround. It would make sense, however, to add it because we
want to support i2c_board_info structures.

Regards,

   Wolfram



signature.asc
Description: Digital signature

Re: [PATCH] Input: atmel_mxt_ts: Add of node type to the i2c table

[Wolfram Sang]

  This is a workaround. It would make sense, however, to add it because we
  want to support i2c_board_info structures.
  
 
 I think it really depends if an IP block can be used on non-DT platforms
 (which I think is true for this trackpad) but if a driver is for an IP block
 that can only be used on a DT-only platform (e.g: a PMIC that is controlled
 over I2C and is only compatible with a DT-only SoC) then I don't think we need
 to support the i2c_board_info structure and can get rid of the I2C ID table on
 these drivers once Lee series land.

That is exactly what I meant. It should be only added if there is a
reason other than workaround. If you say, it doesn't make sense on
non-DT, then it should not be added.



signature.asc
Description: Digital signature

Re: [PATCH 1/1] i2c: Kconfig: Enable HSI2C for Exynos5 platform

[Wolfram Sang]

On Wed, Jun 25, 2014 at 03:32:03PM +0530, Sachin Kamat wrote:
 All Exynos5 platforms have HSI2C controllers and are needed by
 various IPs connected to the boards based on these SoCs. Thus
 select this by default for Exynos5 platforms.
 
 Signed-off-by: Sachin Kamat sachin.ka...@samsung.com
 Cc: Doug Anderson diand...@chromium.org

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH 1/1] i2c: Kconfig: Enable HSI2C for Exynos5 platform

[Wolfram Sang]

On Fri, Jul 25, 2014 at 05:06:32PM +0530, Sachin Kamat wrote:
 On Wed, Jul 16, 2014 at 11:10 PM, Wolfram Sang w...@the-dreams.de wrote:
  On Wed, Jun 25, 2014 at 10:55:31AM -0700, Doug Anderson wrote:
  Sachin,
 
  On Wed, Jun 25, 2014 at 3:02 AM, Sachin Kamat sachin.ka...@samsung.com 
  wrote:
   All Exynos5 platforms have HSI2C controllers and are needed by
   various IPs connected to the boards based on these SoCs. Thus
   select this by default for Exynos5 platforms.
  
   Signed-off-by: Sachin Kamat sachin.ka...@samsung.com
   Cc: Doug Anderson diand...@chromium.org
   ---
drivers/i2c/busses/Kconfig |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
  
   diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
   index 9f7d5859cf65..c7918cffe790 100644
   --- a/drivers/i2c/busses/Kconfig
   +++ b/drivers/i2c/busses/Kconfig
   @@ -465,9 +465,9 @@ config I2C_EG20T
config I2C_EXYNOS5
   tristate Exynos5 high-speed I2C driver
   depends on ARCH_EXYNOS5  OF
   +   default y
   help
   - Say Y here to include support for high-speed I2C controller in 
   the
   - Exynos5 based Samsung SoCs.
   + High-speed I2C controller on Exynos5 based Samsung SoCs.
  
config I2C_GPIO
   tristate GPIO-based bitbanging I2C
   --
   1.7.9.5
 
  This seems reasonable to me and I will also take the blame for
  suggesting this.  It's hard to imagine running a real exynos5 system
  without I2C.  One could argue that on an exynos5250 the high speed I2C
  controller is not mandatory (since all the ports can be muxed to use
  the old controller) but on newer exynos5 products you're expected to
  have the main PMIC on one of the i2c ports.  An exynos system without
  access to its PMIC will just sorta limp by.
 
  ...but I've added Arnd to this thread as he expressed some
  reservations about this type of thing, so we'll see what he says.
 
  Ping. Arnd? I have no strong opinion and would take this patch...
 
 Wolfram,
 
 How do you want to proceed with this?

Giving Arnd a last chance to speak, otherwise I'll take it.



signature.asc
Description: Digital signature

Re: [PATCH 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3

[Wolfram Sang]

On Mon, Jul 28, 2014 at 02:19:22PM +0200, Javier Martinez Canillas wrote:
 From: Derek Basehore dbaseh...@chromium.org
 
 Since the i2c bus can get wedged on the EC sometimes, set the number of 
 retries
 to 3. Since we un-wedge the bus immediately after the wedge happens, this is 
 the
 correct fix since only one transfer will fail.
 
 Signed-off-by: Derek Basehore dbaseh...@chromium.org
 Reviewed-by: Doug Anderson diand...@chromium.org
 ---
  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c 
 b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 index 05e033c..a4411da 100644
 --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
 @@ -16,6 +16,8 @@
  #include linux/platform_device.h
  #include linux/slab.h
  
 +#define I2C_MAX_RETRIES 3

I think a define for this is overkill, yet I don't really mind.

Acked-by: Wolfram Sang w...@the-dreams.de

I assume this goes via MFD, let me know if I should take it.

 +
  /**
   * struct ec_i2c_device - Driver data for I2C tunnel
   *
 @@ -290,6 +292,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
   bus-adap.algo_data = bus;
   bus-adap.dev.parent = pdev-dev;
   bus-adap.dev.of_node = np;
 + bus-adap.retries = I2C_MAX_RETRIES;
  
   err = i2c_add_adapter(bus-adap);
   if (err) {
 -- 
 2.0.0.rc2
 


signature.asc
Description: Digital signature

Re: [PATCH 1/2 v2] i2c: exynos5: remove an unnecessary read of FIFO_STATUS register

[Wolfram Sang]

On Thu, Jun 26, 2014 at 10:44:57AM +0530, Naveen Krishna Chatradhi wrote:
 This patch removes an extra read of FIFO_STATUS register in the interrrupt
 service routine. Which is read again before the actual use.
 
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH 2/2 v2] i2c: exynos5: remove extra line and fix an assignment

[Wolfram Sang]

On Thu, Jun 26, 2014 at 10:44:58AM +0530, Naveen Krishna Chatradhi wrote:
 This patch does the following in exynos5_i2c_message_start() function
 1. Fixes an assignment
As, i2c_auto_conf is initialized to '0' at the beginning of the
function and HSI2C_READ_WRITE is defined as (1u  16)
 
Using |= for the first assignment is more readable.
 
 2. Removes an extra line 
 
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH v3] i2c: exynos5: Properly use the noirq variants of suspend/resume

[Wolfram Sang]

On Wed, Jun 25, 2014 at 09:39:20AM -0700, Doug Anderson wrote:
 The original code for the exynos i2c controller registered for the
 noirq variants.  However during review feedback it was moved to
 SIMPLE_DEV_PM_OPS without anyone noticing that it meant we were no
 longer actually noirq (despite functions named
 exynos5_i2c_suspend_noirq and exynos5_i2c_resume_noirq).
 
 i2c controllers that might have wakeup sources on them seem to need to
 resume at noirq time so that the individual drivers can actually read
 the i2c bus to handle their wakeup.
 
 NOTE: I took the original review feedback from Wolfram and added
 poweroff, thaw, freeze, restore.
 
 This patch has only been compile-tested since I don't have all the
 patches needed to make my machine using this i2c driver actually
 suspend/resume.
 
 Signed-off-by: Doug Anderson diand...@chromium.org
 Acked-by: Kukjin Kim kgene@samsung.com

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH v2] i2c: s3c2410: resume the I2C controller earlier

[Wolfram Sang]

On Tue, Jun 24, 2014 at 04:56:54PM -0700, Doug Anderson wrote:
 From: Vincent Palatin vpala...@chromium.org
 
 When the wake-up is triggered by the PMIC RTC, the RTC driver is trying
 to read the PMIC interrupt status over I2C and fails because the I2C
 controller is not resumed yet.
 Let's resume the I2C controller earlier in the _noirq phase
 (as other hardwares are doing), so we can properly get the wake-up
 condition.
 
 Signed-off-by: Vincent Palatin vpala...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org

Applied to for-current (because Tomasz said it fixes issues), thanks! If
you think the exynos patch of the same style should also be in current,
please let me know...



signature.asc
Description: Digital signature

Re: [PATCH 1/1] i2c: Kconfig: Enable HSI2C for Exynos5 platform

[Wolfram Sang]

On Wed, Jun 25, 2014 at 10:55:31AM -0700, Doug Anderson wrote:
 Sachin,
 
 On Wed, Jun 25, 2014 at 3:02 AM, Sachin Kamat sachin.ka...@samsung.com 
 wrote:
  All Exynos5 platforms have HSI2C controllers and are needed by
  various IPs connected to the boards based on these SoCs. Thus
  select this by default for Exynos5 platforms.
 
  Signed-off-by: Sachin Kamat sachin.ka...@samsung.com
  Cc: Doug Anderson diand...@chromium.org
  ---
   drivers/i2c/busses/Kconfig |4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
  index 9f7d5859cf65..c7918cffe790 100644
  --- a/drivers/i2c/busses/Kconfig
  +++ b/drivers/i2c/busses/Kconfig
  @@ -465,9 +465,9 @@ config I2C_EG20T
   config I2C_EXYNOS5
  tristate Exynos5 high-speed I2C driver
  depends on ARCH_EXYNOS5  OF
  +   default y
  help
  - Say Y here to include support for high-speed I2C controller in the
  - Exynos5 based Samsung SoCs.
  + High-speed I2C controller on Exynos5 based Samsung SoCs.
 
   config I2C_GPIO
  tristate GPIO-based bitbanging I2C
  --
  1.7.9.5
 
 This seems reasonable to me and I will also take the blame for
 suggesting this.  It's hard to imagine running a real exynos5 system
 without I2C.  One could argue that on an exynos5250 the high speed I2C
 controller is not mandatory (since all the ports can be muxed to use
 the old controller) but on newer exynos5 products you're expected to
 have the main PMIC on one of the i2c ports.  An exynos system without
 access to its PMIC will just sorta limp by.
 
 ...but I've added Arnd to this thread as he expressed some
 reservations about this type of thing, so we'll see what he says.

Ping. Arnd? I have no strong opinion and would take this patch...

 
 Reviewed-by: Doug Anderson diand...@chromium.org


signature.asc
Description: Digital signature

[PATCH 12/16] i2c: i2c-s3c2410: Drop class based scanning to improve bootup time

[Wolfram Sang]

This driver has been flagged to drop class based instantiation. The removal
improves boot-up time and is unneeded for embedded controllers. Users have been
warned to switch for some time now, so we can actually do the removal. Keep the
DEPRECATED flag, so the core can inform users that the behaviour finally
changed now. After another transition period, this flag can go, too.
While we are here, remove the indentation for the array setup because
such things always break after some time.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 drivers/i2c/busses/i2c-s3c2410.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index e828a1dba0e5..6252c051525a 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1128,11 +1128,11 @@ static int s3c24xx_i2c_probe(struct platform_device 
*pdev)
s3c24xx_i2c_parse_dt(pdev-dev.of_node, i2c);
 
strlcpy(i2c-adap.name, s3c2410-i2c, sizeof(i2c-adap.name));
-   i2c-adap.owner   = THIS_MODULE;
-   i2c-adap.algo= s3c24xx_i2c_algorithm;
+   i2c-adap.owner = THIS_MODULE;
+   i2c-adap.algo = s3c24xx_i2c_algorithm;
i2c-adap.retries = 2;
-   i2c-adap.class   = I2C_CLASS_HWMON | I2C_CLASS_SPD | 
I2C_CLASS_DEPRECATED;
-   i2c-tx_setup = 50;
+   i2c-adap.class = I2C_CLASS_DEPRECATED;
+   i2c-tx_setup = 50;
 
init_waitqueue_head(i2c-wait);
 
-- 
2.0.0

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

[PATCH 00/16] i2c: drop class based instantiaion for selected drivers

[Wolfram Sang]

Some drivers were flagged to remove class based instantiation soon to improve
boot-up time. Originally, I was planning for a longer deprecation time so users
could switch over to some other kind of instantiation. However, the demand for
the speed up is high enough and class based instantiation is used rarely, so
the removal takes place now. To make up for the deprecation time, another
warning is added to the i2c core pointing out that the behaviour has now
changed:

+   /* Warn that the adapter lost class based instantiation */
+   if (adapter-class == I2C_CLASS_DEPRECATED) {
+   dev_dbg(adapter-dev,
+   This adapter dropped support for I2C classes and 
+   won't auto-detect %s devices anymore. If you need it, 
check 
+   'Documentation/i2c/instantiating-devices' for 
alternatives.\n,
+   driver-driver.name);
+   return 0;
+   }

A branch can be found here (with two other cleanups - driver removals):
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/deprecate_stuff

I hope this meets all the needs. Please comment, ack, test...

Thanks,

   Wolfram


Wolfram Sang (16):
  i2c: add debug info when class instantiation was dropped
  i2c: i2c-at91: Drop class based scanning to improve bootup time
  i2c: i2c-bcm2835: Drop class based scanning to improve bootup time
  i2c: i2c-bfin-twi: Drop class based scanning to improve bootup time
  i2c: i2c-davinci: Drop class based scanning to improve bootup time
  i2c: i2c-designware-platdrv: Drop class based scanning to improve
bootup time
  i2c: i2c-mv64xxx: Drop class based scanning to improve bootup time
  i2c: i2c-nomadik: Drop class based scanning to improve bootup time
  i2c: i2c-ocores: Drop class based scanning to improve bootup time
  i2c: i2c-omap: Drop class based scanning to improve bootup time
  i2c: i2c-rcar: Drop class based scanning to improve bootup time
  i2c: i2c-s3c2410: Drop class based scanning to improve bootup time
  i2c: i2c-sirf: Drop class based scanning to improve bootup time
  i2c: i2c-stu300: Drop class based scanning to improve bootup time
  i2c: i2c-tegra: Drop class based scanning to improve bootup time
  i2c: i2c-xiic: Drop class based scanning to improve bootup time

 drivers/i2c/busses/i2c-at91.c   |  2 +-
 drivers/i2c/busses/i2c-bcm2835.c|  2 +-
 drivers/i2c/busses/i2c-bfin-twi.c   |  2 +-
 drivers/i2c/busses/i2c-davinci.c|  2 +-
 drivers/i2c/busses/i2c-designware-platdrv.c |  2 +-
 drivers/i2c/busses/i2c-mv64xxx.c|  2 +-
 drivers/i2c/busses/i2c-nomadik.c|  8 
 drivers/i2c/busses/i2c-ocores.c | 12 ++--
 drivers/i2c/busses/i2c-omap.c   |  2 +-
 drivers/i2c/busses/i2c-rcar.c   | 14 +++---
 drivers/i2c/busses/i2c-s3c2410.c|  8 
 drivers/i2c/busses/i2c-sirf.c   |  2 +-
 drivers/i2c/busses/i2c-stu300.c |  2 +-
 drivers/i2c/busses/i2c-tegra.c  |  2 +-
 drivers/i2c/busses/i2c-xiic.c   | 12 ++--
 drivers/i2c/i2c-core.c  | 10 ++
 16 files changed, 47 insertions(+), 37 deletions(-)

-- 
2.0.0

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

Re: [PATCH 00/16] i2c: drop class based instantiaion for selected drivers

[Wolfram Sang]

  +   /* Warn that the adapter lost class based instantiation */
  +   if (adapter-class == I2C_CLASS_DEPRECATED) {
  +   dev_dbg(adapter-dev,
  +   This adapter dropped support for I2C classes and 
  +   won't auto-detect %s devices anymore. If you need it, 
  check 
  +   'Documentation/i2c/instantiating-devices' for 
  alternatives.\n,
 
 It's usually a bad idea to split kernel messages across source lines

Yes, usually. Not for this message, though, it's specific. The lines
would have been quite long.

 You could at least do the split at the sentence boundary.

OK, this would have been a compromise.



signature.asc
Description: Digital signature

Re: [PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Wolfram Sang]

On Wed, Jun 18, 2014 at 11:14:04AM -0700, Doug Anderson wrote:
 From: Bill Richardson wfric...@chromium.org
 
 Remove the three wrapper functions that talk to the EC without passing all
 the desired arguments and just use the underlying communication function
 that passes everything in a struct intead.
 
 This is internal code refactoring only. Nothing should change.
 
 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 Acked-by: Lee Jones lee.jo...@linaro.org
 Reviewed-by: Simon Glass s...@chromium.org

For the I2C part:

Acked-by: Wolfram Sang w...@the-dreams.de



signature.asc
Description: Digital signature

Re: [PATCH v2 09/10] mfd: cros_ec: ec_dev-cmd_xfer() returns number of bytes received from EC

[Wolfram Sang]

On Wed, Jun 18, 2014 at 11:14:06AM -0700, Doug Anderson wrote:
 From: Bill Richardson wfric...@chromium.org
 
 When communicating with the EC, the cmd_xfer() function should return the
 number of bytes it received from the EC, or negative on error.
 
 Signed-off-by: Bill Richardson wfric...@chromium.org
 Signed-off-by: Doug Anderson diand...@chromium.org
 Acked-by: Lee Jones lee.jo...@linaro.org
 Reviewed-by: Simon Glass s...@chromium.org

For the I2C part:

Acked-by: Wolfram Sang w...@the-dreams.de



signature.asc
Description: Digital signature

Re: [PATCH 1/1] i2c: s3c2410: Remove class based instantiation

[Wolfram Sang]

On Tue, Jun 03, 2014 at 02:30:22PM +0530, Sachin Kamat wrote:
 With multiplatform support enabled for Exynos, class based
 instantiation causes a boot time delay due to I2C_CLASS_HWMON
 triggered probe. Since class based instantiation has been
 deprecated since some time now, remove it for Samsung I2C driver.
 
 Signed-off-by: Sachin Kamat sachin.ka...@samsung.com
 Cc: Olof Johansson o...@lixom.net

In my book, not long enough. I suggest I will take care of the removal.
I'll do it for all class deprecating drivers.



signature.asc
Description: Digital signature

Re: [PATCH 1/1] i2c: s3c2410: Remove class based instantiation

[Wolfram Sang]

 The reason for the removal is the boot delay it is causing on Exynos 5250
 based boards by throwing the following errors continuously.
 
 [   55.005300] s3c-i2c 12ce.i2c: cannot get bus (error -110)
 [   59.005300] s3c-i2c 12ce.i2c: cannot get bus (error -110)
 [   63.005300] s3c-i2c 12ce.i2c: cannot get bus (error -110)

I know. That's why I implemented the deprecated class. Still, users may
need more time to switch over. Keep it an out-of-tree patch until then.



signature.asc
Description: Digital signature

Re: [PATCH 1/1] i2c: s3c2410: Remove class based instantiation

[Wolfram Sang]

 Couldn't it be made configurable with a Kconfig entry? E.g. Enable
 deprecated class-based instantiation under Drivers / I2C? Turning it
 off could make the code ignore the class field completely.

Technically, yes. But we could also wait another cycle and just be done
with it :)



signature.asc
Description: Digital signature

Re: [PATCH v3 1/6] i2c: s3c2410: Handle i2c sys_cfg register in i2c driver

[Wolfram Sang]

On Sat, May 10, 2014 at 04:20:14PM +0900, Pankaj Dubey wrote:
 Let's handle i2c interrupt re-configuration in i2c driver. This will
 help us in removing some soc specific checks from machine files.
 Since only Exynos5250, and Exynos5420 need to do this, added syscon
 based phandle to i2c device nodes of respective SoC DT files.
 Also handle saving and restoring of SYS_I2C_CFG register during
 suspend and resume of i2c driver. This will help in removing soc
 specific check from mach-exynos/pm.c.
 
 CC: Rob Herring robh...@kernel.org
 CC: Randy Dunlap rdun...@infradead.org
 CC: Wolfram Sang w...@the-dreams.de
 CC: Russell King li...@arm.linux.org.uk
 CC: devicet...@vger.kernel.org
 CC: linux-...@vger.kernel.org
 CC: linux-...@vger.kernel.org
 Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com

Shall this go via i2c-tree? Looks to me like it should go with the rest
of the series? Patch is mostly okay (only reviewed the code, not the
bindings), one nit:

 + regmap_update_bits(i2c-sysreg, EXYNOS5_SYS_I2C_CFG, BIT(id), val);

Hardcode the '0' and drop using 'val'?



signature.asc
Description: Digital signature

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Wolfram Sang]

On Fri, May 09, 2014 at 03:54:25PM +0100, Mark Brown wrote:
 On Fri, May 09, 2014 at 08:12:47PM +0530, Naveen Krishna Ch wrote:
  On 9 May 2014 19:21, Mark Brown broo...@kernel.org wrote:
   On Fri, May 09, 2014 at 05:50:00PM +0530, Naveen Krishna Ch wrote:
 
   DRM related drivers like DP, FIMD, HDMI, Mixer wants to be probed ASAP
   during the boot.
   The real problem comes when, one of these drivers do a regulator_get().
 
   If the physical supply  is not enabled/hookedup the regulator_get() call
   assumes that physical supply is present and returns a
   dummy_regulator (But, not an error).
 
   Because of which, Display and several other devices fails to work.
 
   These drivers are buggy, if they geniunely expect and handle a missing
   supply then they should be using regulator_get_optional() to request the
   regulator and even if they don't the use of subsys_initcall() is not
   going to fix anything here - if a dummy regulator is going to be
   returned the time things are probed won't make a difference.
 
 ...
 
  If all the I2C, SPI, DMA, I2C_TUNNEL, DRM based LCD are all mod_probes()
  DRM drivers are probing a head of the PMIC probe. Which is causing the
  display drivers to get dummy_regulators instead of the real supplies.
 
 No, it really won't - I have no idea what you are doing but it's not
 mainline.  If you are getting dummy regulators then you don't have a
 supply present at all and probe ordering isn't going to make a blind
 bit of difference.
 
 Please provide a specific technical description of the problem you are
 seeing in mainline.

+1 Unless we have a clear understanding that this is the only acceptable
solution in mainline, this is really an out-of-tree patch.



signature.asc
Description: Digital signature

Re: [PATCH] i2c: mux: Use subsys_initcall for the i2c-arb-gpio-challenge

[Wolfram Sang]

On Thu, Apr 24, 2014 at 08:35:22PM +0530, Naveen Krishna Chatradhi wrote:
 From: Doug Anderson diand...@chromium.org
 
 Since many drivers rely on FETs that live behind this arbitrator, they
 can't successfully probe until after the arbitrator comes up.  They
 ought to handle things properly with EPROBE_DEFER and still work, but
 that has some downsides:
 
 1. Those drivers don't come up till later in the boot process.  That
really not so nice for the LCD--we want that to init early.
 2. Some drivers have bugs and don't handle EPROBE_DEFER.  Those
drivers should be fixed but not all of them have been fixed yet.
HDMI is one example since DRM doesn't really have good support for
deferring probes.
 
 With this change We end up using the same init level as the main i2c bus.
 
 Signed-off-by: Doug Anderson diand...@chromium.org
 Reviewed-on: https://gerrit.chromium.org/gerrit/57007
 Reviewed-by: Simon Glass s...@chromium.org
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com

Same here. Unless we have a clear understanding that this is the only
acceptable solution in mainline, this is really an out-of-tree patch.



signature.asc
Description: Digital signature

Re: [PATCH] i2c: exynos5: add support for HSI2C on Exynos5260 SoC

[Wolfram Sang]

On Mon, Apr 28, 2014 at 02:29:58PM +0530, Naveen Krishna Chatradhi wrote:
 HSI2C module on Exynos5260 differs from current modules in
 following ways:
 1.  HSI2C on Exynos5260 has fifo_depth of 16bytes
 2.  Module needs to be reset as a part of init sequence.
 
 Hence, Following changes are involved.
 1. Add a new compatible string and Updates the Documentation dt bindings.
 2. Introduce a variant struct to support the changes in H/W
 3. Reset the module during init. Thus, bringing the module back
 to default state irrespective of what firmware did with it.
 
 Signed-off-by: Naveen Krishna Chatradhi ich.nav...@samsung.com
 Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com
 ---

...

 - fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(HSI2C_DEF_TXFIFO_LVL);
 + trig_lvl = (i2c-msg-len  i2c-variant-fifo_depth) ?
 + (i2c-variant-fifo_depth * 3/4) : i2c-msg-len;
 + fifo_ctl |= HSI2C_RXFIFO_TRIGGER_LEVEL(trig_lvl);
 +

Dunno why checkpatch missed the 'space around operator' issue, yet I
fixed it here...

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH] i2c: exynos5: Initialise Samsung High Speed I2C controller early

[Wolfram Sang]

 Kindly, suggest a workable approach for all the subsystems.

Keep this patch out-of-tree? I know that probe ordering causes problems,
and that it needs major efforts. Yet, I understood that adding more and
more subsys_initcall to mainline is not going to help the process unless
essential. Using subsys_initcall has issues, too.



signature.asc
Description: Digital signature

Re: [PATCH v3 6/7] i2c: ChromeOS EC tunnel driver

[Wolfram Sang]

 +I2C bus that tunnels through the ChromeOS EC (cros-ec)
 +==
 +On some ChromeOS board designs we've got a connection to the EC (embedded
 +controller) but no direct connection to some devices on the other side of
 +the EC (like a battery and PMIC).  To get access to those devices we need
 +to tunnel our i2c commands through the EC.
 +
 +The node for this device should be under a cros-ec node like 
 google,cros-ec-spi
 +or google,cros-ec-i2c.
 +
 +
 +Required properties:
 +- compatible: google,cros-ec-i2c-tunnel
 +- google,remote-bus: The EC bus we'd like to talk to.
 +
 +Optional child nodes:
 +- One node per I2C device connected to the tunnelled I2C bus.
 +
 +
 +Example:
 + cros-ec@0 {
 + compatible = google,cros-ec-spi;

Ooookay, now I get it. From the compatible name of this snipplet, I
assumed this node describes only an SPI IP core inside the EC. This is
why I complained about the location of the I2C busses, since placing it
as subnodes of the EC based SPI didn't make sense to me, even though the
connection of the tunnel was SPI. Now I understand that this is the core
driver for the EC, talking to it via SPI. Since this driver is an SPI
child node I would not have expected the -spi suffix. Sorry, for this
confusion :/ Now, the bindings make much more sense to me.

 + google,remote-bus = 0;

I am still not too happy about this one, but it is good enough for now,
I suppose.

Code looks good, so

Reviewed-by: Wolfram Sang w...@the-dreams.de

I don't mind how it gets upstream. I can take it, but you can also keep
it in this series.



signature.asc
Description: Digital signature

Re: [PATCH v3 6/7] i2c: ChromeOS EC tunnel driver

[Wolfram Sang]

On Mon, May 19, 2014 at 06:22:58PM +0100, Lee Jones wrote:
  Code looks good, so
  
  Reviewed-by: Wolfram Sang w...@the-dreams.de
  
  I don't mind how it gets upstream. I can take it, but you can also keep
  it in this series.
 
 Let's keep the series together.  Are you happy with me just merging it
 through MFD, or would you like a pull-request, so you can take it in
 too?

Just merge it...



signature.asc
Description: Digital signature

Re: [PATCH] i2c: s3c2410: resume race fix

[Wolfram Sang]

On Fri, Apr 11, 2014 at 03:19:41PM -0700, Doug Anderson wrote:
 From: Olof Johansson o...@lixom.net
 
 Don't unmark the device as suspended until after it's been re-setup.
 
 The main race would be w.r.t. an i2c driver that gets resumed at the same
 time (asyncronously), that is allowed to do a transfer since suspended
 is set to 0 before reinit, but really should have seen the -EIO return
 instead.
 
 Signed-off-by: Olof Johansson o...@lixom.net
 Signed-off-by: Doug Anderson diand...@chromium.org

Applied to for-current, thanks!



signature.asc
Description: Digital signature

Re: [PATCH v2 6/7] i2c: ChromeOS EC tunnel driver

[Wolfram Sang]

Hi,

just a basic review to keep things rolling...

 On the original Samsung ARM Chromebook these devices were on an I2C
 bus that was shared between the AP and the EC and arbitrated using
 some extranal GPIOs (see i2c-arb-gpio-challenge).
 
 The original arbitration scheme worked well enough but had some
 downsides:
 * It was nonstandard (not using standard I2C multimaster)
 * It only worked if the EC-AP communication was I2C
 * It was relatively hard to debug problems (hard to tell if i2c issues
   were caused by the EC, the AP, or some device on the bus).
 
 On the HP Chromebook 11 the design was changed to:

This paragraph would be a nice update for the gpio-arbitration docs.

 diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt

The bindings should independently be sent to the devicetree list.

 new file mode 100644
 index 000..898f030
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
 @@ -0,0 +1,39 @@
 +I2C bus that tunnels through the ChromeOS EC (cros-ec)
 +==
 +On some ChromeOS board designs we've got a connection to the EC (embedded
 +controller) but no direct connection to some devices on the other side of
 +the EC (like a battery and PMIC).  To get access to those devices we need
 +to tunnel our i2c commands through the EC.
 +
 +The node for this device should be under a cros-ec node like 
 google,cros-ec-spi
 +or google,cros-ec-i2c.
 +
 +
 +Required properties:
 +- compatible: google,cros-ec-i2c-tunnel
 +- google,remote-bus: The EC bus we'd like to talk to.
 +
 +Optional child nodes:
 +- One node per I2C device connected to the tunnelled I2C bus.
 +
 +
 +Example:
 + cros-ec@0 {
 + compatible = google,cros-ec-spi;
 +
 + ...
 +
 + i2c-tunnel {
 + compatible = google,cros-ec-i2c-tunnel;
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + google,remote-bus = 0;
 +
 + battery: sbs-battery@b {
 + compatible = sbs,sbs-battery;
 + reg = 0xb;
 + sbs,poll-retry-count = 1;
 + };
 + };
 + }

Can the tunnel have n busses? How to represent them then? I think the
remote-bus property should go in favor of proper sub-nodes? Wouldn't it
also be more generic to have the tunnel node seperate and reference the
tunnel-mechanism (spi here) as a phandle?

 +/**
 + * ec_i2c_construct_message - construct a message to go to the EC
 + *
 + * This function effectively stuffs the standard i2c_msg format of Linux into
 + * a format that the EC understands.
 + *
 + * @buf: The buffer to fill.  Can pass NULL to count how many bytes the 
 message
 + *   would be.

I wonder about this NULL thing. That means the size is calculated twice.
Why not make two functions instead, one fir size calc, one for setting
up?

 +/**
 + * ec_i2c_parse_response - Parse a response from the EC
 + *
 + * We'll take the EC's response and copy it back into msgs.
 + *
 + * @buf: The buffer to parse.  Can pass NULL to count how many bytes we 
 expect
 + *the response to be. Otherwise we assume that the right number of
 + *bytes are available.

Ditto.

 + result = bus-ec-command_sendrecv(bus-ec, EC_CMD_I2C_PASSTHRU,
 +request, request_len,
 +response, response_len);

This function pointer should be checked against NULL in probe, I
think.

 +static int ec_i2c_probe(struct platform_device *pdev)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct cros_ec_device *ec = dev_get_drvdata(pdev-dev.parent);
 + struct device *dev = pdev-dev;
 + struct ec_i2c_device *bus = NULL;
 + u32 remote_bus;
 + int err;
 +
 + dev_dbg(dev, Probing\n);

Drop. Device core has it already.

 +
 + if (!np) {
 + dev_err(dev, no device node\n);
 + return -ENOENT;
 + }

Can this happen?

 +
 + bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
 + if (bus == NULL) {
 + dev_err(dev, cannot allocate bus device\n);

No need for error strings when allocating.

 + return -ENOMEM;
 + }
 +
 + dev_dbg(dev, ChromeOS EC I2C tunnel adapter\n);

Drop. Device core debug has it, too.

 +
 + return err;
 +}
 +
 +static int ec_i2c_remove(struct platform_device *dev)
 +{
 + struct ec_i2c_device *bus = platform_get_drvdata(dev);
 +
 + platform_set_drvdata(dev, NULL);

Not needed.

 +
 + i2c_del_adapter(bus-adap);
 +
 + return 0;
 +}
 +

Regards,

   Wolfram



signature.asc
Description: Digital signature

Re: [PATCH 00/17] i2c: deprecate class based instantiation for embedded I2C drivers

[Wolfram Sang]

On Mon, Feb 10, 2014 at 11:03:54AM +0100, Wolfram Sang wrote:
 With I2C, class based instantiation means if a master driver has e.g.
 I2C_CLASS_HWMON set, all slave drivers with this class will try to probe a
 device using an array of possible addresses and some heuristics. That creates
 traffic and needs time, even when nothing is connected. This mechanism is
 needed when you do not have another method to describe the slaves. Embedded 
 I2C
 drivers do not need class based instantiation, since there is i2c_board_info 
 or
 devicetree description. Some drivers have the class flags set, though, and it
 has spread further over the years. We can't remove the flags directly, because
 there might be users out there relying on this feature. So, we add a
 deprecation warning if a device is instantiated via class attributes. After
 giving some time to switch over, we can then finally remove the class flags 
 and
 gain boot time.
 
 Patch 1 adds some missing documentation. Patch 2 adds the deprecation feature.
 Patches 3+4 are tested on hardware I need. Patches 5-17 are suggestions for
 drivers I think could benefit from that. For those, acks are needed before I
 will apply them to my tree. If you use a different driver which can also
 benefit from this, just send a patch adding the new DEPRECATED flag.
 
 The series can also be found here:
 
 git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
 i2c/deprecated_class
 
 Thanks,
 
Wolfram
 
 
 Wolfram Sang (17):
   Documentation: i2c: describe devicetree method for instantiating
 devices
   i2c: add deprecation warning for class based instantiation
   i2c: i2c-omap: deprecate class based instantiation
   i2c: i2c-at91: deprecate class based instantiation
   i2c: i2c-bcm2835: deprecate class based instantiation
   i2c: i2c-bfin-twi: deprecate class based instantiation
   i2c: i2c-davinci: deprecate class based instantiation
   i2c: i2c-designware-platdrv: deprecate class based instantiation
   i2c: i2c-mv64xxx: deprecate class based instantiation
   i2c: i2c-nomadik: deprecate class based instantiation
   i2c: i2c-ocores: deprecate class based instantiation
   i2c: i2c-rcar: deprecate class based instantiation
   i2c: i2c-s3c2410: deprecate class based instantiation
   i2c: i2c-sirf: deprecate class based instantiation
   i2c: i2c-stu300: deprecate class based instantiation
   i2c: i2c-tegra: deprecate class based instantiation
   i2c: i2c-xiic: deprecate class based instantiation

I reconsidered and have now applied all patches. That way, all these
drivers can be converted to no class based instantiation in one go after
some grace period. If it turns out, one driver actually needs it, there
is still the grace period left to remove the DEPRECATED flag again.



signature.asc
Description: Digital signature

Re: [PATCH 1/2 v4] i2c: exynos5: add support for HSI2C on Exynos5260 SoC

[Wolfram Sang]

On Fri, Feb 07, 2014 at 10:12:51AM +0530, Naveen Krishna Chatradhi wrote:
 This patch adds a new compatible and uses variant struct to support
 HSI2C module on Exynos5260. Updates the Documentation dt bindings.
 Also resets the module as an init sequence (Needed by Exynos5260).
 
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com

This patch has clearly not been tested :( Build failure!

 +struct exynos_hsi2c_variant {
 + unsigned intfifo_depth;
 +};

Why so many tabs? In general, I'd prefer one space.

 - exynos5_i2c_init(i2c);
 + exynos5_i2c_reset(i2c);

Is this a related change?



signature.asc
Description: Digital signature

Re: [PATCH 2/2 v4] i2c: exynos5: configure fifo_depth based on HSI2C module variant

[Wolfram Sang]

On Fri, Feb 07, 2014 at 10:13:15AM +0530, Naveen Krishna Chatradhi wrote:
 fifo_depth of the HSI2C is not constant
 Exynos5420 and Exynos5250 supports fifo_depth of 64bytes
 Exynos5260 supports fifo_depth of 16bytes.
 
 This patch configures the fifo_depth based on HSI2C modules version.
 
 Signed-off-by: Naveen Krishna Chatradhi ich.nav...@samsung.com
 [For finding out the difference and initial contribution]
 Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com

I know Tomasz said differently, but I prefer the patches squashed (and
the commit message extended).



signature.asc
Description: Digital signature

Re: [PATCH] i2c-s3c2410: Leave the bus disabled unless it is in use

[Wolfram Sang]

On Fri, Feb 07, 2014 at 02:24:09PM +0530, Naveen Krishna Chatradhi wrote:
 From: Simon Glass s...@chromium.org
 
 There is a rather odd feature of the exynos i2c controller that if it
 is left enabled, it can lock itself up with the clk line held low.
 This makes the bus unusable.
 
 Unfortunately, the s3c24xx_i2c_set_master() function does not notice
 this, and reports a timeout. From then on the bus cannot be used until
 the AP is rebooted.
 
 The problem happens when any sort of interrupt occurs (e.g. due to a
 bus transition) when we are not in the middle of a transaction. We
 have seen many instances of this when U-Boot leaves the bus apparently
 happy, but Linux cannot access it.
 
 The current code is therefore pretty fragile.
 
 This fixes things by leaving the bus disabled unless we are actually
 in a transaction. We enable the bus at the start of the transaction and
 disable it at the end. That way we won't get interrupts and will not
 lock up the bus.
 
 It might be possible to clear pending interrupts on start-up, but this
 seems to be a more robust solution. We can't service interrupts when
 we are not in a transaction, and anyway would rather not lock up the
 bus while we try.
 
 Signed-off-by: Simon Glass s...@chromium.org
 Cc: Grant Grundler grund...@chromium.org
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH] i2c: s3c2410: do not put a default class for the adapter

[Wolfram Sang]

On Sat, Feb 15, 2014 at 04:10:39PM +0100, Wolfram Sang wrote:
 On Fri, Feb 07, 2014 at 01:59:55PM +0530, Naveen Krishna Chatradhi wrote:
  From: Vincent Palatin vpala...@chromium.org
  
  Avoid adding I2C_CLASS_HWMON and I2C_CLASS_SPD class flags to all
  Samsung I2C adapters when the I2C mappings are defined in a device tree.
  So the drivers doing an auto-detection by probing busses won't mess-up
  sensitive I2C devices or trigger long timeouts on non-functional busses.
  
  Signed-off-by: Vincent Palatin vpala...@chromium.org
  Reviewed-by: Olof Johansson ol...@chromium.org
  Tested-by: Vincent Palatin vpala...@chromium.org
  Reviewed-by: Doug Anderson diand...@chromium.org

Your Signed-off was missing BTW.

 
 I'd prefer to drop it in favour of this series:
 
 http://lkml.org/lkml/2014/2/10/836
 

I'll go ahead and apply my patch.



signature.asc
Description: Digital signature

Re: [PATCH] i2c-s3c2410: Leave the bus disabled unless it is in use

[Wolfram Sang]

On Wed, Mar 12, 2014 at 08:27:14PM +0100, Wolfram Sang wrote:
 On Fri, Feb 07, 2014 at 02:24:09PM +0530, Naveen Krishna Chatradhi wrote:
  From: Simon Glass s...@chromium.org
  
  There is a rather odd feature of the exynos i2c controller that if it
  is left enabled, it can lock itself up with the clk line held low.
  This makes the bus unusable.
  
  Unfortunately, the s3c24xx_i2c_set_master() function does not notice
  this, and reports a timeout. From then on the bus cannot be used until
  the AP is rebooted.
  
  The problem happens when any sort of interrupt occurs (e.g. due to a
  bus transition) when we are not in the middle of a transaction. We
  have seen many instances of this when U-Boot leaves the bus apparently
  happy, but Linux cannot access it.
  
  The current code is therefore pretty fragile.
  
  This fixes things by leaving the bus disabled unless we are actually
  in a transaction. We enable the bus at the start of the transaction and
  disable it at the end. That way we won't get interrupts and will not
  lock up the bus.
  
  It might be possible to clear pending interrupts on start-up, but this
  seems to be a more robust solution. We can't service interrupts when
  we are not in a transaction, and anyway would rather not lock up the
  bus while we try.
  
  Signed-off-by: Simon Glass s...@chromium.org
  Cc: Grant Grundler grund...@chromium.org
  Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
  Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 
 Applied to for-next, thanks!

Do you consider this relevant for stable?



signature.asc
Description: Digital signature

Re: [PATCH] i2c: s3c2410: do not put a default class for the adapter

[Wolfram Sang]

On Fri, Feb 07, 2014 at 01:59:55PM +0530, Naveen Krishna Chatradhi wrote:
 From: Vincent Palatin vpala...@chromium.org
 
 Avoid adding I2C_CLASS_HWMON and I2C_CLASS_SPD class flags to all
 Samsung I2C adapters when the I2C mappings are defined in a device tree.
 So the drivers doing an auto-detection by probing busses won't mess-up
 sensitive I2C devices or trigger long timeouts on non-functional busses.
 
 Signed-off-by: Vincent Palatin vpala...@chromium.org
 Reviewed-by: Olof Johansson ol...@chromium.org
 Tested-by: Vincent Palatin vpala...@chromium.org
 Reviewed-by: Doug Anderson diand...@chromium.org

I'd prefer to drop it in favour of this series:

http://lkml.org/lkml/2014/2/10/836



signature.asc
Description: Digital signature

[PATCH 13/17] i2c: i2c-s3c2410: deprecate class based instantiation

[Wolfram Sang]

Warn users that class based instantiation is going away soon in favour
of more robust probing and faster bootup times.

Signed-off-by: Wolfram Sang w...@the-dreams.de
Cc: Ben Dooks ben-li...@fluff.org
Cc: Kukjin Kim kgene@samsung.com
---

This patch is a suggestion. Looking for an ack by someone who actually uses
the driver.

 drivers/i2c/busses/i2c-s3c2410.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index bf8fb94..4a623e0 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1062,7 +1062,7 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
i2c-adap.owner   = THIS_MODULE;
i2c-adap.algo= s3c24xx_i2c_algorithm;
i2c-adap.retries = 2;
-   i2c-adap.class   = I2C_CLASS_HWMON | I2C_CLASS_SPD;
+   i2c-adap.class   = I2C_CLASS_HWMON | I2C_CLASS_SPD | 
I2C_CLASS_DEPRECATED;
i2c-tx_setup = 50;
 
init_waitqueue_head(i2c-wait);
-- 
1.8.5.1

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

[PATCH 00/17] i2c: deprecate class based instantiation for embedded I2C drivers

[Wolfram Sang]

With I2C, class based instantiation means if a master driver has e.g.
I2C_CLASS_HWMON set, all slave drivers with this class will try to probe a
device using an array of possible addresses and some heuristics. That creates
traffic and needs time, even when nothing is connected. This mechanism is
needed when you do not have another method to describe the slaves. Embedded I2C
drivers do not need class based instantiation, since there is i2c_board_info or
devicetree description. Some drivers have the class flags set, though, and it
has spread further over the years. We can't remove the flags directly, because
there might be users out there relying on this feature. So, we add a
deprecation warning if a device is instantiated via class attributes. After
giving some time to switch over, we can then finally remove the class flags and
gain boot time.

Patch 1 adds some missing documentation. Patch 2 adds the deprecation feature.
Patches 3+4 are tested on hardware I need. Patches 5-17 are suggestions for
drivers I think could benefit from that. For those, acks are needed before I
will apply them to my tree. If you use a different driver which can also
benefit from this, just send a patch adding the new DEPRECATED flag.

The series can also be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/deprecated_class

Thanks,

   Wolfram


Wolfram Sang (17):
  Documentation: i2c: describe devicetree method for instantiating
devices
  i2c: add deprecation warning for class based instantiation
  i2c: i2c-omap: deprecate class based instantiation
  i2c: i2c-at91: deprecate class based instantiation
  i2c: i2c-bcm2835: deprecate class based instantiation
  i2c: i2c-bfin-twi: deprecate class based instantiation
  i2c: i2c-davinci: deprecate class based instantiation
  i2c: i2c-designware-platdrv: deprecate class based instantiation
  i2c: i2c-mv64xxx: deprecate class based instantiation
  i2c: i2c-nomadik: deprecate class based instantiation
  i2c: i2c-ocores: deprecate class based instantiation
  i2c: i2c-rcar: deprecate class based instantiation
  i2c: i2c-s3c2410: deprecate class based instantiation
  i2c: i2c-sirf: deprecate class based instantiation
  i2c: i2c-stu300: deprecate class based instantiation
  i2c: i2c-tegra: deprecate class based instantiation
  i2c: i2c-xiic: deprecate class based instantiation

 Documentation/i2c/instantiating-devices | 34 +++--
 drivers/i2c/busses/i2c-at91.c   |  2 +-
 drivers/i2c/busses/i2c-bcm2835.c|  2 +-
 drivers/i2c/busses/i2c-bfin-twi.c   |  2 +-
 drivers/i2c/busses/i2c-davinci.c|  2 +-
 drivers/i2c/busses/i2c-designware-platdrv.c |  2 +-
 drivers/i2c/busses/i2c-mv64xxx.c|  2 +-
 drivers/i2c/busses/i2c-nomadik.c|  2 +-
 drivers/i2c/busses/i2c-ocores.c |  2 +-
 drivers/i2c/busses/i2c-omap.c   |  2 +-
 drivers/i2c/busses/i2c-rcar.c   |  2 +-
 drivers/i2c/busses/i2c-s3c2410.c|  2 +-
 drivers/i2c/busses/i2c-sirf.c   |  2 +-
 drivers/i2c/busses/i2c-stu300.c |  2 +-
 drivers/i2c/busses/i2c-tegra.c  |  2 +-
 drivers/i2c/busses/i2c-xiic.c   |  2 +-
 drivers/i2c/i2c-core.c  |  7 ++
 include/linux/i2c.h |  1 +
 18 files changed, 55 insertions(+), 17 deletions(-)

-- 
1.8.5.1

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

Re: [PATCH 1/2 v3] i2c: exynos5: add support for HSI2C on Exynos5260 SoC

[Wolfram Sang]

On Thu, Feb 06, 2014 at 02:50:51PM +0100, Tomasz Figa wrote:
 Also, please use correct addresses of DT ML and Wolfram's e-mail
 (fixed in this message).

And please don't use In-Reply-To when sending new versions of patches.
The message threading became hard to read here...



signature.asc
Description: Digital signature

Re: [PATCH v2] drivers: i2c: silence a compile warning in i2c-s3c2410.c

[Wolfram Sang]

On Wed, Jan 15, 2014 at 10:42:42AM +0900, y...@samsung.com wrote:

Note that your email address is wrong here. I also fixed the description
to i2c: s3c2410: fix quirk usage for 64-bit which is more apropriate.

Other than that: Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH 1/1] drivers: i2c: silence a compile warning in i2c-s3c2410.c

[Wolfram Sang]

On Fri, Jan 10, 2014 at 12:03:45PM +0900, Pankaj Dubey wrote:
 If used 64 bit compiler GCC warns that:
 
 drivers/i2c/busses/i2c-s3c2410.c: In function ‘s3c24xx_get_device_quirks’:
 drivers/i2c/busses/i2c-s3c2410.c:168:10: warning: cast from pointer to integer
 of different size [-Wpointer-to-int-cast]
 
 This patch fixes this by converting unsigned int to unsigned long.

Maybe switch to kernel_ulong_t? Since this is used in mod_devicetable.h
all over.

 
 CC: Kukjin Kim kgene@samsung.com
 CC: Wolfram Sang w...@the-dreams.de
 CC: linux-...@vger.kernel.org
 Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com
 ---
  drivers/i2c/busses/i2c-s3c2410.c |7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-s3c2410.c 
 b/drivers/i2c/busses/i2c-s3c2410.c
 index bf8fb94..447dd98 100644
 --- a/drivers/i2c/busses/i2c-s3c2410.c
 +++ b/drivers/i2c/busses/i2c-s3c2410.c
 @@ -101,7 +101,7 @@ enum s3c24xx_i2c_state {
  
  struct s3c24xx_i2c {
   wait_queue_head_t   wait;
 - unsigned intquirks;
 + unsigned longquirks;

The indentation is corrupted.

   unsigned intsuspended:1;
  
   struct i2c_msg  *msg;
 @@ -160,12 +160,13 @@ MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
   * Get controller type either from device tree or platform device variant.
  */
  
 -static inline unsigned int s3c24xx_get_device_quirks(struct platform_device 
 *pdev)
 +static inline unsigned long
 + s3c24xx_get_device_quirks(struct platform_device *pdev)

Keep it one line please, ignoring the 80 char thing.

  {
   if (pdev-dev.of_node) {
   const struct of_device_id *match;
   match = of_match_node(s3c24xx_i2c_match, pdev-dev.of_node);
 - return (unsigned int)match-data;
 + return (unsigned long)match-data;
   }
  
   return platform_get_device_id(pdev)-driver_data;

The last line also returns kernel_ulong_t. No real difference, but using
it is a little more consistent.



signature.asc
Description: Digital signature

Re: [PATCH V2] i2c: s3c2410 : Add polling mode support

[Wolfram Sang]

Hi,

On Mon, Nov 11, 2013 at 04:50:20PM +0530, Yuvaraj Kumar C D wrote:
 From: Vasanth Ananthan vasanthanant...@gmail.com
 
 This patch adds polling mode support for i2c-s3c2410 driver.The
 SATA PHY controller's CMU and TRSV block's are of I2C register
 map in exynos5250.These blocks can be configured using i2c.
 
 But i2c controller instance on which these block's sits lacks an
 interrupt line.Also the current i2c-s3c2410 driver is only interrupt
 driven, thus a polling mode support is required in the driver for
 supporting this controller. This patch adds this support to the driver.
 
 Changes from V1:
   1.Changed the is_ack() to have even period b/w polls and
 used usleep_range() instead of udelay().

Mileages vary, but I'd like to see revision changes after the ---.

   ret = devm_request_irq(pdev-dev, i2c-irq, s3c24xx_i2c_irq, 0,
 -dev_name(pdev-dev), i2c);
 + dev_name(pdev-dev), i2c);

Unrelated change.

Rest looks good, so I'll fix up the things for you and apply to
for-next, thanks!



signature.asc
Description: Digital signature

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

[Wolfram Sang]

On Tue, Nov 26, 2013 at 09:52:46AM +0530, Naveen Krishna Chatradhi wrote:
 For Exynos4 and Exynos5 SoCs from Samsung the 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 if CONFIG_CPU_FREQ_S3C24XX is enabled.
 
 This change should save a bunch of cpufreq transitions calls
 which does not apply to exynos SoCs.
 
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Acked-by: Kyungmin Park kyungmin.p...@samsung.com
 Reviewed-by: Doug Anderson diand...@chromium.org

Applied to for-next, thanks!



signature.asc
Description: Digital signature

Re: [PATCH v12] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

On Wed, Oct 16, 2013 at 11:00:42AM +0530, Naveen Krishna Chatradhi wrote:
 Adds support for High Speed I2C driver found in Exynos5 and
 later SoCs from Samsung.
 
 Driver only supports Device Tree method.
 
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Signed-off-by: Taekgyun Ko taeggyun...@samsung.com
 Reviewed-by: Simon Glass s...@google.com
 Tested-by: Andrew Bresticker abres...@google.com
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 Signed-off-by: Andrew Bresticker abres...@google.com

Rebased to v3.12-rc4 and Applied to for-next, thanks!



signature.asc
Description: Digital signature

[PATCH 8/9] spi: spi-s3c64xx: replace platform_driver_probe to support deferred probing

[Wolfram Sang]

Subsystems like pinctrl and gpio rightfully make use of deferred probing at
core level. Now, deferred drivers won't be retried if they don't have a .probe
function specified in the driver struct. Fix this driver to have that, so the
devices it supports won't get lost in a deferred probe.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 drivers/spi/spi-s3c64xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 512b889..3df81c6 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1633,6 +1633,7 @@ static struct platform_driver s3c64xx_spi_driver = {
.pm = s3c64xx_spi_pm,
.of_match_table = of_match_ptr(s3c64xx_spi_dt_match),
},
+   .probe = s3c64xx_spi_probe,
.remove = s3c64xx_spi_remove,
.id_table = s3c64xx_spi_driver_ids,
 };
@@ -1640,7 +1641,7 @@ MODULE_ALIAS(platform:s3c64xx-spi);
 
 static int __init s3c64xx_spi_init(void)
 {
-   return platform_driver_probe(s3c64xx_spi_driver, s3c64xx_spi_probe);
+   return platform_driver_register(s3c64xx_spi_driver);
 }
 subsys_initcall(s3c64xx_spi_init);
 
-- 
1.8.4.rc3

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

[PATCH 0/9] fix deferred probing issue of platform_driver_probe

[Wolfram Sang]

We had some issues with deferred probing in the I2C subsystem. This series
attempts to fix a part of it. From the patch description:

===

Subsystems like pinctrl and gpio rightfully make use of deferred probing at
core level. Now, deferred drivers won't be retried if they don't have a .probe
function specified in the driver struct. Fix this driver to have that, so the
devices it supports won't get lost in a deferred probe.

===

I think it makes sense to remove platform_driver_probe from bus masters like
i2c and spi (especially since they are dependant on pinctrl these days). This
is what this series does. I had a look at dma drivers as well, but there are
more things to be considered, so delayed for now.

Please comment. If no objections are raised, I'd like to bring at least the I2C
patches into v3.12, so deferred devices will then be correctly probed. The
series is based on 3.12-rc4 and was compile-tested. It is available at

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
platform_driver_probe_drop

Regards,

   Wolfram


Wolfram Sang (9):
  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
  spi: spi-au1550: replace platform_driver_probe to support deferred
probing
  spi: spi-bfin5xx: replace platform_driver_probe to support deferred
probing
  spi: spi-omap-uwire: replace platform_driver_probe to support deferred
probing
  spi: spi-s3c64xx: replace platform_driver_probe to support deferred
probing
  spi: spi-txx9: replace platform_driver_probe to support deferred
probing

 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-stu300.c | 11 +--
 drivers/spi/spi-au1550.c|  3 ++-
 drivers/spi/spi-bfin5xx.c   |  5 +++--
 drivers/spi/spi-omap-uwire.c|  5 +++--
 drivers/spi/spi-s3c64xx.c   |  3 ++-
 drivers/spi/spi-txx9.c  |  3 ++-
 9 files changed, 28 insertions(+), 21 deletions(-)

-- 
1.8.4.rc3

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

Re: [PATCH] i2c: s3c2410: fix clk_disable/clk_unprepare warnings

[Wolfram Sang]

On Tue, Sep 17, 2013 at 11:31:53AM -0500, Kim Phillips wrote:
 commit d16933b33914a6dff38a4ecbe8edce44a17898e8 i2c: s3c2410: Move
 location of clk_prepare_enable() call in probe function refactored
 clk_enable and clk_disable calls yet neglected to remove the
 clk_disable_unprepare call in the module's remove().

Applied to for-current, thanks!



signature.asc
Description: Digital signature

Re: [PATCH] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

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.

 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.

 
 Signed-off-by: Taekgyun Ko taeggyun...@samsung.com
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Reviewed-by: Simon Glass s...@google.com
 Tested-by: Andrew Bresticker abres...@google.com
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 Signed-off-by: Andrew Bresticker abres...@google.com
 
 ---
 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.

 +- If specified, The bus operates in high-speed mode only if the
 +   clock-frequency is = 1Mhz.

s/The/the/

...

 +/*
 + * 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.

 +
 + writel((HSI2C_FUNC_MODE_I2C | HSI2C_MASTER),
 + i2c-regs + HSI2C_CTL);
 + writel(HSI2C_TRAILING_COUNT, i2c-regs + HSI2C_TRAILIG_CTL);
 +
 + if (i2c-speed_mode == HSI2C_HIGH_SPD) {
 + writel(HSI2C_MASTER_ID(MASTER_ID(i2c-adap.nr)),
 + 

Re: [PATCH V3] i2c: move of helpers into the core

[Wolfram Sang]

On Thu, Aug 22, 2013 at 06:00:14PM +0200, Wolfram Sang wrote:
 I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
 that it is much cleaner to have this in the core. This also removes a
 circular dependency between the helpers and the core, and so we can
 finally register child nodes in the core instead of doing this manually
 in each driver. So, fix the drivers and documentation, too.
 
 Acked-by: Rob Herring rob.herr...@calxeda.com
 Reviewed-by: Felipe Balbi ba...@ti.com
 Acked-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
 Tested-by: Sylwester Nawrocki s.nawro...@samsung.com
 Signed-off-by: Wolfram Sang w...@the-dreams.de

Applied to for-next!


signature.asc
Description: Digital signature

[PATCH V3] i2c: move of helpers into the core

[Wolfram Sang]

I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
that it is much cleaner to have this in the core. This also removes a
circular dependency between the helpers and the core, and so we can
finally register child nodes in the core instead of doing this manually
in each driver. So, fix the drivers and documentation, too.

Acked-by: Rob Herring rob.herr...@calxeda.com
Reviewed-by: Felipe Balbi ba...@ti.com
Acked-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Tested-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Wolfram Sang w...@the-dreams.de
---

V2-V3: Was trying to be too smart by only fixing includes needed.
Took a more general approach this time, converting of_i2c.h
to i2c.h in case i2c.h was not already there. Otherwise
remove it. Improved my build scripts and no build failures,
no complaints from buildbot as well.


 Documentation/acpi/enumeration.txt  |1 -
 arch/powerpc/platforms/44x/warp.c   |1 -
 drivers/gpu/drm/tilcdc/tilcdc_slave.c   |1 -
 drivers/gpu/drm/tilcdc/tilcdc_tfp410.c  |1 -
 drivers/gpu/host1x/drm/output.c |2 +-
 drivers/i2c/busses/i2c-at91.c   |3 -
 drivers/i2c/busses/i2c-cpm.c|6 --
 drivers/i2c/busses/i2c-davinci.c|2 -
 drivers/i2c/busses/i2c-designware-platdrv.c |2 -
 drivers/i2c/busses/i2c-gpio.c   |3 -
 drivers/i2c/busses/i2c-i801.c   |2 -
 drivers/i2c/busses/i2c-ibm_iic.c|4 -
 drivers/i2c/busses/i2c-imx.c|3 -
 drivers/i2c/busses/i2c-mpc.c|2 -
 drivers/i2c/busses/i2c-mv64xxx.c|3 -
 drivers/i2c/busses/i2c-mxs.c|3 -
 drivers/i2c/busses/i2c-nomadik.c|3 -
 drivers/i2c/busses/i2c-ocores.c |3 -
 drivers/i2c/busses/i2c-octeon.c |3 -
 drivers/i2c/busses/i2c-omap.c   |3 -
 drivers/i2c/busses/i2c-pnx.c|3 -
 drivers/i2c/busses/i2c-powermac.c   |9 +-
 drivers/i2c/busses/i2c-pxa.c|2 -
 drivers/i2c/busses/i2c-s3c2410.c|2 -
 drivers/i2c/busses/i2c-sh_mobile.c  |2 -
 drivers/i2c/busses/i2c-sirf.c   |3 -
 drivers/i2c/busses/i2c-stu300.c |2 -
 drivers/i2c/busses/i2c-tegra.c  |3 -
 drivers/i2c/busses/i2c-versatile.c  |2 -
 drivers/i2c/busses/i2c-wmt.c|3 -
 drivers/i2c/busses/i2c-xiic.c   |3 -
 drivers/i2c/i2c-core.c  |  109 +-
 drivers/i2c/i2c-mux.c   |3 -
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c  |1 -
 drivers/i2c/muxes/i2c-mux-gpio.c|1 -
 drivers/i2c/muxes/i2c-mux-pinctrl.c |1 -
 drivers/media/platform/exynos4-is/fimc-is-i2c.c |4 +-
 drivers/media/platform/exynos4-is/fimc-is.c |2 +-
 drivers/media/platform/exynos4-is/media-dev.c   |1 -
 drivers/of/Kconfig  |6 --
 drivers/of/Makefile |1 -
 drivers/of/of_i2c.c |  114 ---
 drivers/staging/imx-drm/imx-tve.c   |2 +-
 include/linux/i2c.h |   20 
 include/linux/of_i2c.h  |   46 -
 sound/soc/fsl/imx-sgtl5000.c|2 +-
 sound/soc/fsl/imx-wm8962.c  |2 +-
 47 files changed, 138 insertions(+), 262 deletions(-)
 delete mode 100644 drivers/of/of_i2c.c
 delete mode 100644 include/linux/of_i2c.h

diff --git a/Documentation/acpi/enumeration.txt 
b/Documentation/acpi/enumeration.txt
index d9be7a9..958266e 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -238,7 +238,6 @@ An I2C bus (controller) driver does:
if (ret)
/* handle error */
 
-   of_i2c_register_devices(adapter);
/* Enumerate the slave devices behind this bus via ACPI */
acpi_i2c_register_devices(adapter);
 
diff --git a/arch/powerpc/platforms/44x/warp.c 
b/arch/powerpc/platforms/44x/warp.c
index 4cfa499..534574a 100644
--- a/arch/powerpc/platforms/44x/warp.c
+++ b/arch/powerpc/platforms/44x/warp.c
@@ -16,7 +16,6 @@
 #include linux/interrupt.h
 #include linux/delay.h
 #include linux/of_gpio.h
-#include linux/of_i2c.h
 #include linux/slab.h
 #include linux/export.h
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c 
b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
index dfffaf0..a19f657 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
@@ -16,7 +16,6 @@
  */
 
 #include linux/i2c.h
-#include linux/of_i2c.h
 #include linux/pinctrl/pinmux.h
 #include linux/pinctrl

[PATCH V2] i2c: move of helpers into the core

[Wolfram Sang]

I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
that it is much cleaner to have this in the core. This also removes a
circular dependency between the helpers and the core, and so we can
finally register child nodes in the core instead of doing this manually
in each driver. So, fix the drivers and documentation, too.

Acked-by: Sylwester Nawrocki s.nawro...@amsung.com
Acked-by: Rob Herring rob.herr...@calxeda.com
Reviewed-by: Felipe Balbi ba...@ti.com
Acked-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Signed-off-by: Wolfram Sang w...@the-dreams.de
---

V1 - V2: * Add #else branch to #if CONFIG_OF
  * EXPORT_SYMBOLs got attached to wrong functions
  * cosmetic change (of - OF)
  * properly based on 3.11-rc4

 Documentation/acpi/enumeration.txt  |1 -
 drivers/i2c/busses/i2c-at91.c   |3 -
 drivers/i2c/busses/i2c-cpm.c|6 --
 drivers/i2c/busses/i2c-davinci.c|2 -
 drivers/i2c/busses/i2c-designware-platdrv.c |2 -
 drivers/i2c/busses/i2c-gpio.c   |3 -
 drivers/i2c/busses/i2c-i801.c   |2 -
 drivers/i2c/busses/i2c-ibm_iic.c|4 -
 drivers/i2c/busses/i2c-imx.c|3 -
 drivers/i2c/busses/i2c-mpc.c|2 -
 drivers/i2c/busses/i2c-mv64xxx.c|3 -
 drivers/i2c/busses/i2c-mxs.c|3 -
 drivers/i2c/busses/i2c-nomadik.c|3 -
 drivers/i2c/busses/i2c-ocores.c |3 -
 drivers/i2c/busses/i2c-octeon.c |3 -
 drivers/i2c/busses/i2c-omap.c   |3 -
 drivers/i2c/busses/i2c-pnx.c|3 -
 drivers/i2c/busses/i2c-powermac.c   |9 +-
 drivers/i2c/busses/i2c-pxa.c|2 -
 drivers/i2c/busses/i2c-s3c2410.c|2 -
 drivers/i2c/busses/i2c-sh_mobile.c  |2 -
 drivers/i2c/busses/i2c-sirf.c   |3 -
 drivers/i2c/busses/i2c-stu300.c |2 -
 drivers/i2c/busses/i2c-tegra.c  |3 -
 drivers/i2c/busses/i2c-versatile.c  |2 -
 drivers/i2c/busses/i2c-wmt.c|3 -
 drivers/i2c/busses/i2c-xiic.c   |3 -
 drivers/i2c/i2c-core.c  |  109 +-
 drivers/i2c/i2c-mux.c   |3 -
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c  |1 -
 drivers/i2c/muxes/i2c-mux-gpio.c|1 -
 drivers/i2c/muxes/i2c-mux-pinctrl.c |1 -
 drivers/media/platform/exynos4-is/fimc-is-i2c.c |3 -
 drivers/of/Kconfig  |6 --
 drivers/of/Makefile |1 -
 drivers/of/of_i2c.c |  114 ---
 include/linux/i2c.h |   20 
 include/linux/of_i2c.h  |   46 -
 38 files changed, 132 insertions(+), 253 deletions(-)
 delete mode 100644 drivers/of/of_i2c.c
 delete mode 100644 include/linux/of_i2c.h

diff --git a/Documentation/acpi/enumeration.txt 
b/Documentation/acpi/enumeration.txt
index d9be7a9..958266e 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -238,7 +238,6 @@ An I2C bus (controller) driver does:
if (ret)
/* handle error */
 
-   of_i2c_register_devices(adapter);
/* Enumerate the slave devices behind this bus via ACPI */
acpi_i2c_register_devices(adapter);
 
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 6bb839b..fd05930 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -28,7 +28,6 @@
 #include linux/module.h
 #include linux/of.h
 #include linux/of_device.h
-#include linux/of_i2c.h
 #include linux/platform_device.h
 #include linux/slab.h
 #include linux/platform_data/dma-atmel.h
@@ -775,8 +774,6 @@ static int at91_twi_probe(struct platform_device *pdev)
return rc;
}
 
-   of_i2c_register_devices(dev-adapter);
-
dev_info(dev-dev, AT91 i2c bus driver.\n);
return 0;
 }
diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 2e1f7eb..b2b8aa9 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -42,7 +42,6 @@
 #include linux/dma-mapping.h
 #include linux/of_device.h
 #include linux/of_platform.h
-#include linux/of_i2c.h
 #include sysdev/fsl_soc.h
 #include asm/cpm.h
 
@@ -681,11 +680,6 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
dev_dbg(ofdev-dev, hw routines for %s registered.\n,
cpm-adap.name);
 
-   /*
-* register OF I2C devices
-*/
-   of_i2c_register_devices(cpm-adap);
-
return 0;
 out_shut:
cpm_i2c_shutdown(cpm);
diff --git a/drivers/i2c/busses/i2c-davinci.c b

[PATCH RESEND] i2c: move of helpers into the core

[Wolfram Sang]

I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
that it is much cleaner to have this in the core. This also removes a
circular dependency between the helpers and the core, and so we can
finally register child nodes in the core instead of doing this manually
in each driver. So, fix the drivers and documentation, too.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---

Sigh, hitting the CC threshold on vger again. So resending to the lists only.
BTW this patch is based on -rc4 and was tested on an AT91 board. More tests
very welcome. Thanks!


 Documentation/acpi/enumeration.txt  |1 -
 drivers/i2c/busses/i2c-at91.c   |3 -
 drivers/i2c/busses/i2c-cpm.c|6 --
 drivers/i2c/busses/i2c-davinci.c|2 -
 drivers/i2c/busses/i2c-designware-platdrv.c |2 -
 drivers/i2c/busses/i2c-gpio.c   |3 -
 drivers/i2c/busses/i2c-i801.c   |2 -
 drivers/i2c/busses/i2c-ibm_iic.c|4 -
 drivers/i2c/busses/i2c-imx.c|3 -
 drivers/i2c/busses/i2c-mpc.c|2 -
 drivers/i2c/busses/i2c-mv64xxx.c|3 -
 drivers/i2c/busses/i2c-mxs.c|3 -
 drivers/i2c/busses/i2c-nomadik.c|3 -
 drivers/i2c/busses/i2c-ocores.c |3 -
 drivers/i2c/busses/i2c-octeon.c |3 -
 drivers/i2c/busses/i2c-omap.c   |3 -
 drivers/i2c/busses/i2c-pnx.c|3 -
 drivers/i2c/busses/i2c-powermac.c   |9 +-
 drivers/i2c/busses/i2c-pxa.c|2 -
 drivers/i2c/busses/i2c-s3c2410.c|2 -
 drivers/i2c/busses/i2c-sh_mobile.c  |2 -
 drivers/i2c/busses/i2c-sirf.c   |3 -
 drivers/i2c/busses/i2c-stu300.c |2 -
 drivers/i2c/busses/i2c-tegra.c  |3 -
 drivers/i2c/busses/i2c-versatile.c  |2 -
 drivers/i2c/busses/i2c-wmt.c|3 -
 drivers/i2c/busses/i2c-xiic.c   |3 -
 drivers/i2c/i2c-core.c  |  107 -
 drivers/i2c/i2c-mux.c   |3 -
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c  |1 -
 drivers/i2c/muxes/i2c-mux-gpio.c|1 -
 drivers/i2c/muxes/i2c-mux-pinctrl.c |1 -
 drivers/media/platform/exynos4-is/fimc-is-i2c.c |3 -
 drivers/of/Kconfig  |6 --
 drivers/of/Makefile |1 -
 drivers/of/of_i2c.c |  114 ---
 include/linux/i2c.h |   20 
 include/linux/of_i2c.h  |   46 -
 38 files changed, 130 insertions(+), 253 deletions(-)
 delete mode 100644 drivers/of/of_i2c.c
 delete mode 100644 include/linux/of_i2c.h

diff --git a/Documentation/acpi/enumeration.txt 
b/Documentation/acpi/enumeration.txt
index d9be7a9..958266e 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -238,7 +238,6 @@ An I2C bus (controller) driver does:
if (ret)
/* handle error */
 
-   of_i2c_register_devices(adapter);
/* Enumerate the slave devices behind this bus via ACPI */
acpi_i2c_register_devices(adapter);
 
diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
index 6bb839b..fd05930 100644
--- a/drivers/i2c/busses/i2c-at91.c
+++ b/drivers/i2c/busses/i2c-at91.c
@@ -28,7 +28,6 @@
 #include linux/module.h
 #include linux/of.h
 #include linux/of_device.h
-#include linux/of_i2c.h
 #include linux/platform_device.h
 #include linux/slab.h
 #include linux/platform_data/dma-atmel.h
@@ -775,8 +774,6 @@ static int at91_twi_probe(struct platform_device *pdev)
return rc;
}
 
-   of_i2c_register_devices(dev-adapter);
-
dev_info(dev-dev, AT91 i2c bus driver.\n);
return 0;
 }
diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 2e1f7eb..b2b8aa9 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -42,7 +42,6 @@
 #include linux/dma-mapping.h
 #include linux/of_device.h
 #include linux/of_platform.h
-#include linux/of_i2c.h
 #include sysdev/fsl_soc.h
 #include asm/cpm.h
 
@@ -681,11 +680,6 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
dev_dbg(ofdev-dev, hw routines for %s registered.\n,
cpm-adap.name);
 
-   /*
-* register OF I2C devices
-*/
-   of_i2c_register_devices(cpm-adap);
-
return 0;
 out_shut:
cpm_i2c_shutdown(cpm);
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index fa55605..62be3b3 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -38,7 +38,6 @@
 #include linux/slab.h
 #include linux

Re: [PATCH RESEND] i2c: move of helpers into the core

[Wolfram Sang]

 However this patch fails to apply onto either v3.11-rc4 or v3.11-rc6:

Argh, did not drop the MPC patch before rebasing :( So either pick the
patch i2c: powermac: fix return path on error before, pull the branch
[1], or force me to resend ;)

Thanks!

[1] git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/core-with-of


signature.asc
Description: Digital signature

Re: [PATCH RESEND] i2c: move of helpers into the core

[Wolfram Sang]

On Mon, Aug 19, 2013 at 09:46:04PM +0200, Thierry Reding wrote:
 On Mon, Aug 19, 2013 at 07:59:40PM +0200, Wolfram Sang wrote:
 [...]
  diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
 [...]
  +#if IS_ENABLED(CONFIG_OF)
  +static void of_i2c_register_devices(struct i2c_adapter *adap)
  +{
 [...]
  +}
 [...]
  +#endif /* CONFIG_OF */
 
 Isn't this missing the dummy implementation for !OF.

Argh, will fix...



signature.asc
Description: Digital signature

Re: [PATCH v10] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

 Once, we fix this problem. i planned to rebase and submit.
 
 Is it okey with you?

Perfectly fine. Thanks for the heads up.



signature.asc
Description: Digital signature

Re: [PATCH v10] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

On Mon, Jul 01, 2013 at 12:25:19PM +0200, Tomasz Figa wrote:
 Hi Naveen,
 
 Looks mostly good, but see some comments inline.

Ping?



signature.asc
Description: Digital signature

Re: [PATCH v10] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

On Wed, Jun 19, 2013 at 04:18:25PM +0530, Naveen Krishna Chatradhi wrote:
 Adds support for High Speed I2C driver found in Exynos5 and
 later SoCs from Samsung.
 
 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
 
 Signed-off-by: Taekgyun Ko taeggyun...@samsung.com
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Reviewed-by: Simon Glass s...@google.com
 Tested-by: Andrew Bresticker abres...@google.com
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com
 Signed-off-by: Andrew Bresticker abres...@google.com
 ---
 
 +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.

? If not specified, the default is 100kHz. There is no way to get 1MHz,
or?

...

 +static int exynos5_i2c_probe(struct platform_device *pdev)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct exynos5_i2c *i2c;
 + struct resource *mem;
 + unsigned int op_clock;

My compiler says:

drivers/i2c/busses/i2c-exynos5.c: In function ‘exynos5_i2c_probe’:
drivers/i2c/busses/i2c-exynos5.c:687:5: warning: ‘op_clock’ may be used 
uninitialized in this function [-Wuninitialized]

so...

 + int ret;
 +
 + if (!np) {
 + dev_err(pdev-dev, no device node\n);
 + return -ENOENT;
 + }
 +
 + i2c = devm_kzalloc(pdev-dev, sizeof(struct exynos5_i2c), GFP_KERNEL);
 + if (!i2c) {
 + dev_err(pdev-dev, no memory for state\n);
 + return -ENOMEM;
 + }
 +
 + if (of_property_read_u32(np, clock-frequency, op_clock)) {
 + i2c-speed_mode = HSI2C_FAST_SPD;
 + i2c-fs_clock = HSI2C_FS_TX_CLOCK;
 + }
 +
 + if (op_clock = HSI2C_HS_TX_CLOCK) {

... this should be 'else if'

 + i2c-speed_mode = HSI2C_HIGH_SPD;
 + i2c-fs_clock = HSI2C_FS_TX_CLOCK;
 + i2c-hs_clock = op_clock;
 + } else {
 + i2c-speed_mode = HSI2C_FAST_SPD;
 + i2c-fs_clock = op_clock;
 + }

...

 + i2c-bus_id = of_alias_get_id(i2c-adap.dev.of_node, hsi2c);

Huh, the core already gets an alias for you. Can't you use 'adap.nr'?

Rest looks good.

Thanks!


signature.asc
Description: Digital signature

Re: [PATCH v9] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

Hi Naveen,

 +Optional properties:
 +  - samsung,hs-mode: Mode of operation, High speed or Fast speed mode. If not
 +specified, default value is 0.

This was probably overlooked from my last review: Why can't you simply
enable hs-mode when clock-frequency is  1MBit?

 +Example:
 +
 +hsi2c@12ca {
 + compatible = samsung,exynos5-hsi2c;
 + reg = 0x12ca 0x100;
 + interrupts = 56;
 + clock-frequency = 10;
 +
 + /* Pinctrl variant begins here */
 + pinctrl-0 = i2c4_bus;
 + pinctrl-names = default;
 + /* Pinctrl variant ends here */

I'd think the two comments above are not needed.

 +/*
 + * exynos5_i2c_wait_bus_idle
 + *
 + * Wait for the transaction to complete (indicated by the TRANS_DONE bit
 + * being set), and, if this is the last message in a transfer, wait for the
 + * MASTER_BUSY bit to be cleared.
 + *
 + * Returns -EBUSY if the bus cannot be bought to idle

s/bought/brought/

 +static int exynos5_i2c_xfer_msg(struct exynos5_i2c *i2c,
 +   struct i2c_msg *msgs, int stop)
 +{
 + unsigned long timeout;
 + int ret;
 +
 + i2c-msg = msgs;
 + i2c-msg_ptr = 0;
 + i2c-msg_len = 0;
 + i2c-trans_done = 0;
 +
 + INIT_COMPLETION(i2c-msg_complete);
 +
 + exynos5_i2c_message_start(i2c, stop);
 +
 + ret = wait_for_completion_interruptible_timeout
 + (i2c-msg_complete, EXYNOS5_I2C_TIMEOUT);

Have you tested with SIGINT? Most drivers removed the _interruptible_
version of waiting since they couldn't get handling the signals proper
and the bus locked up.

 + if (ret = 0)
 + timeout = ret;
 + else
 + return ret;
 +
 + ret = i2c-state;
 +
 + if ((timeout == 0) || (ret  0)) {
 + exynos5_i2c_reset(i2c);
 + if (timeout == 0) {
 + dev_warn(i2c-dev, %s timeout\n,
 +  (msgs-flags  I2C_M_RD) ? rx : tx);
 + return ret;
 + } else if (ret == -EAGAIN) {
 + return ret;
 + }
 + }
 +
 + /*
 +  * If this is the last message to be transfered (stop == 1)
 +  * Then check if the bus can be brought back to idle.
 +  *
 +  * Return -EBUSY if the bus still busy.
 +  */
 + if (exynos5_i2c_wait_bus_idle(i2c, stop))
 + return -EBUSY;
 +
 + /* Return the state as in interrupt routine */
 + return ret;
 +}
 +
 +static int exynos5_i2c_xfer(struct i2c_adapter *adap,
 + struct i2c_msg *msgs, int num)
 +{
 + struct exynos5_i2c *i2c = (struct exynos5_i2c *)adap-algo_data;
 + struct i2c_msg *msgs_ptr = msgs;
 + int retry, i = 0;
 + int ret = 0, ret_pm;
 + int stop = 0;
 +
 + if (i2c-suspended) {
 + dev_err(i2c-dev, HS-I2C is not initialzed.\n);
 + return -EIO;
 + }
 +
 + ret_pm = pm_runtime_get_sync(i2c-dev);
 + if (IS_ERR_VALUE(ret_pm)) {
 + ret = -EIO;
 + goto out;
 + }
 +
 + clk_prepare_enable(i2c-clk);
 +
 + for (retry = 0; retry  adap-retries; retry++) {

You don't need to retry. The core does it if you return -EAGAIN.

 + for (i = 0; i  num; i++) {
 + stop = (i == num - 1);
 +
 + ret = exynos5_i2c_xfer_msg(i2c, msgs_ptr, stop);
 + msgs_ptr++;
 +
 + if (ret == -EAGAIN) {
 + msgs_ptr = msgs;
 + break;
 + } else if (ret  0) {
 + goto out;
 + }
 + }
 +
 + if ((i == num)  (ret != -EAGAIN))
 + break;
 +
 + dev_dbg(i2c-dev, retrying transfer (%d)\n, retry);
 +
 + udelay(100);
 + }
 +
 + if (i == num) {
 + ret = num;
 + } else {
 + /* Only one message, cannot access the device */
 + if (i == 1)
 + ret = -EREMOTEIO;
 + else
 + ret = i;
 +
 + dev_warn(i2c-dev, xfer message failed\n);
 + }
 +
 + out:
 + clk_disable_unprepare(i2c-clk);
 + pm_runtime_mark_last_busy(i2c-dev);
 + pm_runtime_put_autosuspend(i2c-dev);
 + return ret;
 +}
 +
 +static u32 exynos5_i2c_func(struct i2c_adapter *adap)
 +{
 + return I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL  ~I2C_FUNC_SMBUS_QUICK);
 +}
 +
 +static const struct i2c_algorithm exynos5_i2c_algorithm = {
 + .master_xfer= exynos5_i2c_xfer,
 + .functionality  = exynos5_i2c_func,
 +};
 +
 +static int exynos5_i2c_probe(struct platform_device *pdev)
 +{
 + struct device_node *np = pdev-dev.of_node;
 + struct exynos5_i2c *i2c;
 + struct resource *mem;
 + int ret;
 +
 + if (!np) {
 + dev_err(pdev-dev, no device node\n);
 + return -ENOENT;
 + }
 +
 + i2c = devm_kzalloc(pdev-dev, sizeof(struct 

Re: [PATCH 30/33] arch/arm/plat-samsung: don't check resource with devm_ioremap_resource

[Wolfram Sang]

On Tue, May 21, 2013 at 01:38:23AM +0900, Kukjin Kim wrote:
 On 05/16/13 20:15, Wolfram Sang wrote:
 devm_ioremap_resource does sanity checks on the given resource. No need to
 duplicate this in the driver.
 
 Signed-off-by: Wolfram Sangw...@the-dreams.de
 
 Looks good to me, applied into Samsung tree.
 
 Wolfram, if you want to take this into your tree, let me know.

It is already upstream :)

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

[PATCH 30/33] arch/arm/plat-samsung: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 arch/arm/plat-samsung/adc.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/plat-samsung/adc.c b/arch/arm/plat-samsung/adc.c
index ca07cb1..79690f2 100644
--- a/arch/arm/plat-samsung/adc.c
+++ b/arch/arm/plat-samsung/adc.c
@@ -381,11 +381,6 @@ static int s3c_adc_probe(struct platform_device *pdev)
}
 
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!regs) {
-   dev_err(dev, failed to find registers\n);
-   return -ENXIO;
-   }
-
adc-regs = devm_ioremap_resource(dev, regs);
if (IS_ERR(adc-regs))
return PTR_ERR(adc-regs);
-- 
1.7.10.4

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

[PATCH 08/33] drivers/i2c/busses: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 drivers/i2c/busses/i2c-s3c2410.c |5 -
 drivers/i2c/busses/i2c-sirf.c|6 --
 drivers/i2c/busses/i2c-tegra.c   |5 -
 3 files changed, 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 6e8ee92..cab1c91 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -1082,11 +1082,6 @@ static int s3c24xx_i2c_probe(struct platform_device 
*pdev)
/* map the registers */
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (res == NULL) {
-   dev_err(pdev-dev, cannot find IO resource\n);
-   return -ENOENT;
-   }
-
i2c-regs = devm_ioremap_resource(pdev-dev, res);
 
if (IS_ERR(i2c-regs))
diff --git a/drivers/i2c/busses/i2c-sirf.c b/drivers/i2c/busses/i2c-sirf.c
index 5a7ad24..a63c7d5 100644
--- a/drivers/i2c/busses/i2c-sirf.c
+++ b/drivers/i2c/busses/i2c-sirf.c
@@ -303,12 +303,6 @@ static int i2c_sirfsoc_probe(struct platform_device *pdev)
adap-class = I2C_CLASS_HWMON;
 
mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (mem_res == NULL) {
-   dev_err(pdev-dev, Unable to get MEM resource\n);
-   err = -EINVAL;
-   goto out;
-   }
-
siic-base = devm_ioremap_resource(pdev-dev, mem_res);
if (IS_ERR(siic-base)) {
err = PTR_ERR(siic-base);
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index b60ff90..9aa1b60 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -714,11 +714,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
int ret = 0;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(pdev-dev, no mem resource\n);
-   return -EINVAL;
-   }
-
base = devm_ioremap_resource(pdev-dev, res);
if (IS_ERR(base))
return PTR_ERR(base);
-- 
1.7.10.4

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

[RFC 39/42] arch/arm/plat-samsung: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 arch/arm/plat-samsung/adc.c |5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/plat-samsung/adc.c b/arch/arm/plat-samsung/adc.c
index ca07cb1..79690f2 100644
--- a/arch/arm/plat-samsung/adc.c
+++ b/arch/arm/plat-samsung/adc.c
@@ -381,11 +381,6 @@ static int s3c_adc_probe(struct platform_device *pdev)
}
 
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!regs) {
-   dev_err(dev, failed to find registers\n);
-   return -ENXIO;
-   }
-
adc-regs = devm_ioremap_resource(dev, regs);
if (IS_ERR(adc-regs))
return PTR_ERR(adc-regs);
-- 
1.7.10.4

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

[RFC 23/42] drivers/spi: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 drivers/spi/spi-bcm63xx.c|8 +---
 drivers/spi/spi-ep93xx.c |8 +---
 drivers/spi/spi-omap2-mcspi.c|7 +--
 drivers/spi/spi-s3c64xx.c|7 +--
 drivers/spi/spi-sirf.c   |7 +--
 drivers/spi/spi-tegra114.c   |7 +--
 drivers/spi/spi-tegra20-sflash.c |5 -
 drivers/spi/spi-tegra20-slink.c  |7 +--
 8 files changed, 7 insertions(+), 49 deletions(-)

diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
index a4ec5f4..9804ee0 100644
--- a/drivers/spi/spi-bcm63xx.c
+++ b/drivers/spi/spi-bcm63xx.c
@@ -378,13 +378,6 @@ static int bcm63xx_spi_probe(struct platform_device *pdev)
struct bcm63xx_spi *bs;
int ret;
 
-   r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!r) {
-   dev_err(dev, no iomem\n);
-   ret = -ENXIO;
-   goto out;
-   }
-
irq = platform_get_irq(pdev, 0);
if (irq  0) {
dev_err(dev, no irq\n);
@@ -411,6 +404,7 @@ static int bcm63xx_spi_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, master);
bs-pdev = pdev;
 
+   r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
bs-regs = devm_ioremap_resource(pdev-dev, r);
if (IS_ERR(bs-regs)) {
ret = PTR_ERR(bs-regs);
diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c
index d7bac60..213cb7a3 100644
--- a/drivers/spi/spi-ep93xx.c
+++ b/drivers/spi/spi-ep93xx.c
@@ -1076,15 +1076,9 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
goto fail_put_clock;
}
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(pdev-dev, unable to get iomem resource\n);
-   error = -ENODEV;
-   goto fail_put_clock;
-   }
-
espi-sspdr_phys = res-start + SSPDR;
 
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
espi-regs_base = devm_ioremap_resource(pdev-dev, res);
if (IS_ERR(espi-regs_base)) {
error = PTR_ERR(espi-regs_base);
diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index 86d2158..31c054e 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -1228,16 +1228,11 @@ static int omap2_mcspi_probe(struct platform_device 
*pdev)
}
regs_offset = pdata-regs_offset;
 
-   r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (r == NULL) {
-   status = -ENODEV;
-   goto free_master;
-   }
-
r-start += regs_offset;
r-end += regs_offset;
mcspi-phys = r-start;
 
+   r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
mcspi-base = devm_ioremap_resource(pdev-dev, r);
if (IS_ERR(mcspi-base)) {
status = PTR_ERR(mcspi-base);
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 5000586..fd763cc 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1243,12 +1243,6 @@ static int s3c64xx_spi_probe(struct platform_device 
*pdev)
return -ENODEV;
}
 
-   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (mem_res == NULL) {
-   dev_err(pdev-dev, Unable to get SPI MEM resource\n);
-   return -ENXIO;
-   }
-
irq = platform_get_irq(pdev, 0);
if (irq  0) {
dev_warn(pdev-dev, Failed to get IRQ: %d\n, irq);
@@ -1318,6 +1312,7 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)
/* the spi-mode bits understood by this driver: */
master-mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 
+   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sdd-regs = devm_ioremap_resource(pdev-dev, mem_res);
if (IS_ERR(sdd-regs)) {
ret = PTR_ERR(sdd-regs);
diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c
index 0808cd5..318ca5b 100644
--- a/drivers/spi/spi-sirf.c
+++ b/drivers/spi/spi-sirf.c
@@ -502,12 +502,6 @@ static int spi_sirfsoc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, master);
sspi = spi_master_get_devdata(master);
 
-   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!mem_res) {
-   dev_err(pdev-dev, Unable to get IO resource\n);
-   ret = -ENODEV;
-   goto free_master;
-   }
master-num_chipselect = num_cs;
 
for (i = 0; i  master-num_chipselect; i++) {
@@ -534,6 +528,7 @@ static int spi_sirfsoc_probe(struct platform_device *pdev)
}
}
 
+   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sspi-base = devm_ioremap_resource(pdev-dev, mem_res);
if (IS_ERR

[RFC 09/42] drivers/i2c/busses: don't check resource with devm_ioremap_resource

[Wolfram Sang]

devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.

Signed-off-by: Wolfram Sang w...@the-dreams.de
---
 drivers/i2c/busses/i2c-davinci.c|6 +-
 drivers/i2c/busses/i2c-designware-platdrv.c |6 +-
 drivers/i2c/busses/i2c-imx.c|6 +-
 drivers/i2c/busses/i2c-omap.c   |6 +-
 drivers/i2c/busses/i2c-rcar.c   |7 +--
 drivers/i2c/busses/i2c-s3c2410.c|5 -
 drivers/i2c/busses/i2c-sirf.c   |6 --
 drivers/i2c/busses/i2c-tegra.c  |5 -
 8 files changed, 5 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index cf20e06..2ed3e7b 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -647,11 +647,6 @@ static int davinci_i2c_probe(struct platform_device *pdev)
int r;
 
/* NOTE: driver uses the static register mapping */
-   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!mem) {
-   dev_err(pdev-dev, no mem resource?\n);
-   return -ENODEV;
-   }
 
irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!irq) {
@@ -697,6 +692,7 @@ static int davinci_i2c_probe(struct platform_device *pdev)
return -ENODEV;
clk_prepare_enable(dev-clk);
 
+   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
dev-base = devm_ioremap_resource(pdev-dev, mem);
if (IS_ERR(dev-base)) {
r = PTR_ERR(dev-base);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 8ec9133..6f3c612 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -87,11 +87,6 @@ static int dw_i2c_probe(struct platform_device *pdev)
int irq, r;
 
/* NOTE: driver uses the static register mapping */
-   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!mem) {
-   dev_err(pdev-dev, no mem resource?\n);
-   return -EINVAL;
-   }
 
irq = platform_get_irq(pdev, 0);
if (irq  0) {
@@ -103,6 +98,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
if (!dev)
return -ENOMEM;
 
+   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
dev-base = devm_ioremap_resource(pdev-dev, mem);
if (IS_ERR(dev-base))
return PTR_ERR(dev-base);
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 82f20c6..b274ca1 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -500,17 +500,13 @@ static int __init i2c_imx_probe(struct platform_device 
*pdev)
 
dev_dbg(pdev-dev, %s\n, __func__);
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(pdev-dev, can't get device resources\n);
-   return -ENOENT;
-   }
irq = platform_get_irq(pdev, 0);
if (irq  0) {
dev_err(pdev-dev, can't get irq number\n);
return -ENOENT;
}
 
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(pdev-dev, res);
if (IS_ERR(base))
return PTR_ERR(base);
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index e02f9e3..8498ab7 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1085,11 +1085,6 @@ omap_i2c_probe(struct platform_device *pdev)
u16 minor, major, scheme;
 
/* NOTE: driver uses the static register mapping */
-   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!mem) {
-   dev_err(pdev-dev, no mem resource?\n);
-   return -ENODEV;
-   }
 
irq = platform_get_irq(pdev, 0);
if (irq  0) {
@@ -1103,6 +1098,7 @@ omap_i2c_probe(struct platform_device *pdev)
return -ENOMEM;
}
 
+   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
dev-base = devm_ioremap_resource(pdev-dev, mem);
if (IS_ERR(dev-base))
return PTR_ERR(dev-base);
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 4ba4a95..0fc5858 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -623,12 +623,6 @@ static int rcar_i2c_probe(struct platform_device *pdev)
u32 bus_speed;
int ret;
 
-   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (!res) {
-   dev_err(dev, no mmio resources\n);
-   return -ENODEV;
-   }
-
priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
if (!priv) {
dev_err(dev, no mem for private data\n);
@@ -642,6 +636,7 @@ static int rcar_i2c_probe(struct

Re: Fwd: [PATCH v7] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

Hi,

  +  - Samsung GPIO variant (deprecated):
  +- gpios: The order of the gpios should be the following: SDA, SCL.
  +  The gpio specifier depends on the gpio controller.
 
  Huh? Why should we support a deprecated method with a new driver?
 

This was left unanswered. I am curious.

  +Optional properties:
  +  - samsung,hs-mode: Mode of operation, High speed or Fast speed mode. If 
  not
  +specified, default value is 0.
  +  - samsung,hs-clock-freq: Desired operating frequency in Hz of the bus.
  +If not specified, the default value in Hz is 10.
  +  - samsung,fs-clock-freq: Desired operarting frequency in Hz of the bus.
  +If not specified, the default value in Hz is 10.
 
  NACK! We have a generic binding for defining the bus speed. And
  shouldn't hs-mode be set depending on the bus speed?

Please use clock-frequency here, like other drivers do.

  + /* In auto mode the length of xfer cannot be 0 */
  + if (i2c-msg-len == 0)
  + i2c_auto_conf |= 0x1;
 
  So you send some byte then? Why not reject the message?
 This is to support the probing the devices (i2cdetect cases)

No! This is not a proper SMBUS_QUICK if you send a byte! If it doesn't
work without sending data, then your device does not support it. This is
not uncommon. Please check the smbus specs if you are unsure.

  + i2c-regs = of_iomap(np, 0);
 
  devm_ioremap_resource()
 This was a comment from Thomas on v1.
 https://lkml.org/lkml/2012/11/27/264
 
 Kindly, suggest me which one is more optimal in this case.

Optimal is difficult here, but devm_* has momentum and I prefer
consistency.

 Thanks for your valuable time and comments

You're welcome! Thanks for the submission.

   Wolfram

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

Re: [PATCH] I2C: EXYNOS: High speed mode clock setting for HSI2C

[Wolfram Sang]

On Fri, Apr 19, 2013 at 05:26:23PM +0530, Yuvaraj Kumar C D wrote:
 This patch configure the High speed mode timing register using the
 clock speed mentioned in the dts file.Also it configure the MASTER_ID
 for High speed i2c transfer.
 For i2c high speed transaction, tarnsaction initially starts with the
 fast mode i,e 400Kbits/sec and then switches to high speed mode.For this
 purpose it requires to set up timing value for fast mode and high speed
 mode.
 
 Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com

Since the exynos5 driver needs to be resent anyhow, I assume this will
be included in the next version? Please note that I had comments
regarding the bindings of the bus speed. We have a generic binding for
that already.

Thanks,

   Wolfram

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

Re: [PATCH v7] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

Hi,

thanks for the submission.

On Thu, Apr 04, 2013 at 09:52:01PM -0700, Naveen Krishna Chatradhi wrote:
 From: Naveen Krishna Chatradhi ch.nav...@samsung.com
 
 Adds support for High Speed I2C driver found in Exynos5 and
 later SoCs from Samsung.
 This driver currently supports Auto mode.

Either explain the limitation of this mode or just leave this sentence.

 Driver only supports Device Tree method.
 Note: Added debugfs support for registers view, not tested.

Then leave it out, please.

 
 Signed-off-by: Taekgyun Ko taeggyun...@samsung.com
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Reviewed-by: Simon Glass s...@google.com
 Tested-by: Andrew Bresticker abres...@google.com
 ---
 change since v6:
 1. clock divisor function hs split to handle the error cases
 2. Other irq types are handled
 3. FIFO are handled more efficiently in TX and RX
 4. More function description added
 5. handled the return cases in xfer_msg function
 
  .../devicetree/bindings/i2c/i2c-exynos5.txt|   50 ++
  drivers/i2c/busses/Kconfig |7 +
  drivers/i2c/busses/Makefile|1 +
  drivers/i2c/busses/i2c-exynos5.c   |  934 
 
  4 files changed, 992 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..0bc9347
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
 @@ -0,0 +1,50 @@
 +* 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.
 +  (a) 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.
 +
 +  - Samsung GPIO variant (deprecated):
 +- gpios: The order of the gpios should be the following: SDA, SCL.
 +  The gpio specifier depends on the gpio controller.

Huh? Why should we support a deprecated method with a new driver?

 +  - Pinctrl variant (preferred, if available):
 +- pinctrl-0: Pin control group to be used for this controller.
 +- pinctrl-names: Should contain only one value - default.
 +
 +Optional properties:
 +  - samsung,hs-mode: Mode of operation, High speed or Fast speed mode. If not
 +specified, default value is 0.
 +  - samsung,hs-clock-freq: Desired operating frequency in Hz of the bus.
 +If not specified, the default value in Hz is 10.
 +  - samsung,fs-clock-freq: Desired operarting frequency in Hz of the bus.
 +If not specified, the default value in Hz is 10.

NACK! We have a generic binding for defining the bus speed. And
shouldn't hs-mode be set depending on the bus speed?

 diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
 index adfee98..9fbfa01 100644
 --- a/drivers/i2c/busses/Kconfig
 +++ b/drivers/i2c/busses/Kconfig
 @@ -434,6 +434,13 @@ config I2C_EG20T
 ML7213/ML7223/ML7831 is companion chip for Intel Atom E6xx series.
 ML7213/ML7223/ML7831 is completely compatible for Intel EG20T PCH.
  
 +config I2C_EXYNOS5
 + tristate Exynos5 high-speed I2C driver
 + depends on ARCH_EXYNOS5  OF
 + help
 +   Say Y here to include support for High-speed I2C controller in the
 +   Exynos5 based Samsung SoCs.

s/High/high/

 +struct exynos5_i2c {
 + struct i2c_adapter  adap;
 + unsigned intsuspended:1;
 +
 + struct i2c_msg  *msg;
 + struct completion   msg_complete;
 + unsigned intmsg_ptr;
 + unsigned intmsg_len;
 +
 + unsigned intirq;
 +
 + void __iomem*regs;
 + struct clk  *clk;
 + struct device   *dev;
 + int state;
 +
 + /* GPIO lines for SDA/SCL*/
 + int gpios[2];
 +
 + /* Controller operating frequency */
 + unsigned intclock;
 + unsigned intclk_cycle;
 + unsigned intclk_div;
 +
 + /* HSI2C Controller can operate in
 +  * 1. High speed upto 3.4Mbps
 +  * 2. Fast speed upto 1Mbps
 +  */
 + int speed_mode;
 +};

Only one space as indentation after the type, please.

 +
 +static const struct of_device_id exynos5_i2c_match[] = {
 + { .compatible = samsung,exynos5-hsi2c },
 + {},
 +};
 +MODULE_DEVICE_TABLE(of, exynos5_i2c_match);
 +
 +static inline void exynos5_i2c_stop(struct exynos5_i2c *i2c)
 +{
 + writel(0, i2c-regs + HSI2C_INT_ENABLE);
 +
 + complete(i2c-msg_complete);
 +}

I wonder if this needs to be a seperate 

Re: [PATCH] i2c: s3c2410: Add SMBus emulation for block read

[Wolfram Sang]

 I don't know if that can happen, but if the block has length 0 (that is,
 only consists of a single byte that is read as 0) the is_msglast() test
 returns 0 which might be wrong. Maybe even if it cannot regularily
 happen handle it in a sane way?

Length of 0 is illegal according to the specs, but a check won't hurt.

 Also I wonder if error checking for byte  I2C_SMBUS_BLOCK_MAX would be
 needed to prevent a buffer overrun?

Yes. Although I have seen devices able to send more than that, yet
adding support for that is a seperate issue.

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

Re: [PATCH 08/30] i2c: s3c2410: make header file local

[Wolfram Sang]

On Thu, Apr 11, 2013 at 02:04:50AM +0200, Arnd Bergmann wrote:

 No other file in the kernel besides i2c-s3c2410.c uses the current
 plat/regs-iic.h, so we can simply move the header file to live in the
 same directory as the driver, as a preparation to multiplatform builds.

What about putting the regs in the driver itself?

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

Re: [PATCH] i2c: s3c2410: Add SMBus emulation for block read

[Wolfram Sang]

On Tue, Mar 26, 2013 at 07:59:56PM +0530, Prasanna Kumar wrote:
 From: Jaemin Yoo jmin@samsung.com
 
 SMBus read and write are supported by the emulation layer of i2c
 framework if the controller doesn't have SMBus features.
 
 I2C_M_RECV_LEN flag is used to let i2c drivers know rx length is not
 yet determined but will be read to the first byte in rx buffer.
 
 s3c2410 doesn't handle this flag. So only one byte is read from slave.
 There fore following two features are added to the driver code.
 
 1. skip rx length check if I2C_M_RECV_LEN is set and the length is 1.
 2. add actual bytes to the rx length after reading first bytes if
I2C_M_RECV_LEN.
 
 I2C_M_RECV_LEN is only set for SMBus command. So this code does not
 affect legacy codes which only use i2c command for s3c2410.
 
 Signed-off-by: Jaemin Yoo jmin@samsung.com
 Tested-by: Prasanna Kumar prasanna...@samsung.com

Applied to for-next, thanks!

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

Re: [RFC: PATCH v5] i2c: exynos5: add High Speed I2C controller driver

[Wolfram Sang]

  +   /* CLK_DIV max is 256 */
  +   for (i = 0; i  256; i++) {
  +   utemp1 = utemp0 / (i + 1);
  +   /* SCLK_L/H max is 256 / 2 */
  +   if (utemp1  128) {
 I think TSCLK_L and TSCLK_H both can be configured upto 255.Why
 limiting it to  128 ?
 By limiting it to  128 dont we achieve lesser SCL clock?

Thanks for reviewing but please quote only the relevant part of the
message (like I did). This improves readability a lot.
--
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

Re: [PATCH v2 1/2] i2c: s3c2410: move mach/regs-iic.h into i2c-s3c2410 device driver

[Wolfram Sang]

On Thu, Mar 21, 2013 at 03:09:25PM +0100, Heiko Stübner wrote:
 The register definitions are only used in the driver itself.
 This also removes the last dependency on plat/ includes from the
 i2c driver.
 
 Signed-off-by: Heiko Stuebner he...@sntech.de
 Acked-by: Kukjin Kim kgene@samsung.com

Both applied to for-next, thanks!
--
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

Re: [PATCH] i2c: s3c24xx: allow device core to setup default pin configuration

[Wolfram Sang]

On Mon, Mar 04, 2013 at 07:12:53PM +0530, Thomas Abraham wrote:
 With device core now able to setup the default pin configuration,
 the call to devm_pinctrl_get_select_default can be removed. And
 the pin configuration code based on the deprecated Samsung specific
 gpio bindings is also removed.
 
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
 ---
 
 Hi Heiko, Tomasz,
 
 We have to make a choice on the path forward for pinctrl support
 on Samsung platforms. We have three cases to deal with.
 
 A. Samsung platforms without DT support.
 B. Samsung platforms with DT support using old Samsung specific
gpio bindings for pin-configuration (s3c24xx and s3x64xx).
 C. Samsung platforms with DT support using using pinctrl based
pin-configurations (Exynos4 and Exynos5).
 
 For [A], we just let the platform specific callbacks handle the
 pin setup. For [B] and [C], based on Linus Walleij's pin grab
 by device core patch and subsequent discussions with him on the
 mailing list, would it be acceptable that we discontinue support
 for [B] in Samsung SoC device drivers. This will impact your
 current DT work on s3c24xx and s3c64xx platforms. Pinctrl is
 inevitable and we have to migrate to it. Instead of workarounds
 to maintain support for [B], it might be better that we migrate
 s3c24xx and s3c64xx platforms to pinctrl.
 
 Please do let us know your opinion on this.

Dropping [B] sounds good, but I think it makes sense to wait with this
patch until pinctrl support for s3c24xx is in place?

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

Re: [PATCH 1/2] i2c: s3c2410: move mach/regs-iic.h into i2c-s3c2410 device driver

[Wolfram Sang]

 I'm not sure anymore ... probably some linux-next. I'll redo them on top of 
 your (i2c) for-next branch.

Thanks, but please use 3.9-rc3.
--
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

Re: [PATCH v4 2/2] i2c: pxa: Use i2c-core to get bus number now

[Wolfram Sang]

On Fri, Mar 01, 2013 at 08:57:32AM -0800, Doug Anderson wrote:
 The commit: i2c-core: dt: Pick i2c bus number from i2c alias if
 present adds support for automatically picking the bus number based
 on the alias ID.  Remove the now unnecessary code from i2c-pxa that
 did the same thing.
 
 Signed-off-by: Doug Anderson diand...@chromium.org

Thanks, applied for-next.
--
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

Re: [PATCH v4 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Wolfram Sang]

On Fri, Mar 01, 2013 at 08:57:31AM -0800, Doug Anderson wrote:
 This allows you to get the equivalent functionality of
 i2c_add_numbered_adapter() with all data in the device tree and no
 special case code in your driver.  This is a common device tree
 technique.
 
 For quick reference, the FDT syntax for using an alias to provide an
 ID looks like:
   aliases {
 i2c0 = i2c_0;
 i2c1 = i2c_1;
   };
 
 Signed-off-by: Doug Anderson diand...@chromium.org

Thanks, applied. I'll try to get it into 3.9.

 + /* Handled by wrappers */
 + if (WARN_ON(adap-nr == -1))
 + return -EINVAL;

I removed this check, though. We know our callers, all static.

Thanks,

   Wolfram

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

Re: [PATCH v3 0/2] Add automatic bus number support for i2c busses with device tree

[Wolfram Sang]

  Doug Anderson (2):
i2c-core: dt: Pick i2c bus number from i2c alias if present
i2c: pxa: Use i2c-core to get bus number now
 
   drivers/i2c/busses/i2c-pxa.c | 20 
   drivers/i2c/i2c-core.c   | 54 
  
   2 files changed, 49 insertions(+), 25 deletions(-)
 
 Is there anything else needed for this patch series?  It's been
 hanging around for quite a while and it would be nice to see it land.
 If you're waiting for additional acks or reviews I'll ask around and
 see if I can get them...

Regarding patch 1, I was waiting for the idr changes to hit mainline.
They are mainline now, but since the removal of MAX_IDR_MASK your patch
doesn't apply anymore :( Can you rebase and retest, please? I'd like to
get it into 3.9, still.

For patch 2, some Tested-by would be nice; but it will probably make it
into my for-next somewhen after rc1 is out.

Thanks,

   Wolfram

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

Re: [PATCH 1/2] i2c: s3c2410: move mach/regs-iic.h into i2c-s3c2410 device driver

[Wolfram Sang]

Hi Kukjin,

(please don't quote the whole patch for those few lines)

 Looks good to me on this series:
 Acked-by: Kukjin Kim kgene@samsung.com
 
 Hi Wolfram, if you're ok on this, please pick up into your tree with my ack.

Will do, but not for 3.9 anymore. Thanks!

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Wolfram Sang]

Hi,

  At the moment perhaps the strongest argument for why this particular
  arbitration scheme is needed is that's what the EC (embedded
  controller) on the ARM Chromebook uses.  There have been several
  arguments in-house about whether this was the most ideal way to
  structure things, but that's what we shipped with.  Thus, if we want
  to be able to talk to i2c devices on this bus (contains the keyboard,
  battery, and a bunch of power switches) we need something in the
  system that implements this arbitration scheme.
 
 
 There was some discussion about using the built-in I2C multi-master
 support but no one had had a good experience getting it to work
 reliably in a real system so we ended up deciding not to rely on this.

So, were there no experiences at all or bad experiences when trying? I
am interested in hearing what I2C multi master restrictions might exist
in practice.

Thanks,

   Wolfram

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