Re: [PATCH] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Jarkko Nikula]

Hi

On 12/24/2015 04:30 PM, Fu, Zhonghui wrote:

Now, PM core supports asynchronous suspend/resume mode for devices
during system suspend/resume, and the power state transition of one
device may be completed in separate kernel thread. PM core ensures
all power state transition dependency between devices. This patch
enables designware i2c controllers to suspend/resume asynchronously.
This will take advantage of multicore and improve system suspend/resume
speed. After enabling all i2c devices, i2c adapters and i2c controllers
on ASUS T100TA tablet, the system suspend-to-idle time is reduced to
about 510ms from 750ms, and the system resume time is reduced to about
790ms from 900ms.


Nice reduction :-)


diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 6b00061..395130b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -230,6 +230,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
}

adap = >adapter;
+   device_enable_async_suspend(>dev);
adap->owner = THIS_MODULE;
adap->class = I2C_CLASS_DEPRECATED;
ACPI_COMPANION_SET(>dev, ACPI_COMPANION(>dev));


Does device_enable_async_suspend() need to be called before enabling 
runtime PM? I suppose not since there appears to have also related sysfs 
node for toggling it runtime.


I'm thinking if you could move the device_enable_async_suspend() call 
into drivers/i2c/busses/i2c-designware-core.c: i2c_dw_probe() and then 
also PCI enumerated adapter could take advantage of it.


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

Re: [PATCH v2 0/2] i2c:dw: Add APM X-Gene ACPI I2C device support

[Jarkko Nikula]

On 12/16/2015 11:04 AM, Mika Westerberg wrote:

On Tue, Dec 15, 2015 at 11:20:03AM -0800, Loc Ho wrote:

Can we not just add an AML method that will return the operation clock
frequency that the I2C driver can use? If the method doesn't existed,
we will just bail and do nothing or assume what ever default behavior?


Why would you add yet another method? There already are existing ACPI
*CNT methods that you can use, and are used in systems out there.


One thing to remember that *CNT methods allow more precise machine and 
per bus specific tunings against different HW characteristics like 
pull-up resistance, line load, propagation delay, etc.


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

[RFC] i2c: Prevent runtime suspend of parent during adapter registration

[Jarkko Nikula]

If runtime PM of parent device is enabled before it registers the I2C
adapter there can be unnecessary runtime suspend during adapter device
registration. This can happen when adapter is registered from parent's
probe and if parent device is initially active platform device.

In that case power.usage_count of parent device is zero and
pm_runtime_get()/pm_runtime_put() cycle during probe could put the
parent into runtime suspend. This happens when the i2c_register_adapter()
calls the device_register():

i2c_register_adapter
  device_register
device_add
  bus_probe_device
device_initial_probe
  __device_attach
if (dev->parent) pm_runtime_get_sync(dev->parent)
...
if (dev->parent) pm_runtime_put(dev->parent)

After that i2c_register_adapter() continues registering I2C slave
devices. In case slave device probe does I2C transfers the parent will
resume again and thus get a needless runtime suspend/resume cycle during
adapter registration.

Prevent this while retaining the runtime PM status of parent by only
incrementing/decrementing parent device power usage count during I2C
adapter registration. That makes sure there won't be spurious runtime PM
status changes for parent's probe and lets the driver core to idle the
device after probe finishes.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
I noticed this with i2c-designware-platdrv.c which started to do so
after commit 36d48fb5766a ("i2c: designware-platdrv: enable RuntimePM
before registering to the core"). I guess the same could happen also
with a few other I2C drivers that enable runtime PM similar way. For
instance i2c-at91.c, i2c-cadence.c, i2c-hix5hd2.c and i2c-qup.c.
That made me thinking if issue might be best to handle in i2c-core.c.

Device core drivers/base/dd.c: driver_probe_device() or
drivers/base/platform.c: platform_drv_probe() could be other
alternatives but that would cause a regression to a driver that
purposively tries to suspend the device in its probe().
---
 drivers/i2c/i2c-core.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index ba8eb087f224..4f8b5c80cf1e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1556,6 +1556,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
dev_set_name(>dev, "i2c-%d", adap->nr);
adap->dev.bus = _bus_type;
adap->dev.type = _adapter_type;
+   if (adap->dev.parent)
+   pm_runtime_get_noresume(adap->dev.parent);
res = device_register(>dev);
if (res)
goto out_list;
@@ -1617,6 +1619,8 @@ exit_recovery:
mutex_lock(_lock);
bus_for_each_drv(_bus_type, NULL, adap, __process_new_adapter);
mutex_unlock(_lock);
+   if (adap->dev.parent)
+   pm_runtime_put_noidle(adap->dev.parent);
 
return 0;
 
@@ -1624,6 +1628,8 @@ out_list:
mutex_lock(_lock);
idr_remove(_adapter_idr, adap->nr);
mutex_unlock(_lock);
+   if (adap->dev.parent)
+   pm_runtime_put_noidle(adap->dev.parent);
return res;
 }
 
-- 
2.6.2

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

Re: [PATCH v2] I2C: designware: fix IO timeout issue for AMD controller

[Jarkko Nikula]

On 12/11/2015 02:02 PM, Xiangliang Yu wrote:

Because of some hardware limitation, AMD I2C controller can't
trigger pending interrupt if interrupt status has been changed
after clearing interrupt status bits. Then, I2C will lost
interrupt and IO timeout.

According to hardware design, this patch implements a workaround
to disable i2c controller interrupt and re-enable i2c interrupt
before exiting ISR.

To reduce the performance impacts on other vendors, use unlikely
function to check flag in ISR.
---
Changes in v2:
 - pass flags with ->driver_data
 - unmask interrupt right after masking

Signed-off-by: Xiangliang Yu <xiangliang...@amd.com>
---
  drivers/i2c/busses/i2c-designware-core.c| 6 ++
  drivers/i2c/busses/i2c-designware-core.h| 1 +
  drivers/i2c/busses/i2c-designware-platdrv.c | 7 ++-
  3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 8c48b27..de7fbbb 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -813,6 +813,12 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
  tx_aborted:
if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
complete(>cmd_complete);
+   else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
+   /* workaround to trigger pending interrupt */
+   stat = dw_readl(dev, DW_IC_INTR_MASK);
+   i2c_dw_disable_int(dev);
+   dw_writel(dev, stat, DW_IC_INTR_MASK);
+   }


Looks ok to me.

Acked-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] i2c: designware: Keep pm_runtime_enable/_disable calls in sync

[Jarkko Nikula]

On an hardware shared I2C bus (certain Intel Baytrail SoC platforms) the
runtime PM disable depth keeps increasing over repeated modprobe/rmmod
cycle because pm_runtime_disable() is called without checking should it
be disabled already because of bus sharing.

This hasn't made any other harm than dev->power.disable_depth keeps
increasing but keep it sync by calling pm_runtime_disable() only when
runtime PM is not disabled.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579ecb5a4..1308666b054b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -240,12 +240,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
}
 
r = i2c_dw_probe(dev);
-   if (r) {
+   if (r && !dev->pm_runtime_disabled)
pm_runtime_disable(>dev);
-   return r;
-   }
 
-   return 0;
+   return r;
 }
 
 static int dw_i2c_plat_remove(struct platform_device *pdev)
@@ -260,7 +258,8 @@ static int dw_i2c_plat_remove(struct platform_device *pdev)
 
pm_runtime_dont_use_autosuspend(>dev);
pm_runtime_put_sync(>dev);
-   pm_runtime_disable(>dev);
+   if (!dev->pm_runtime_disabled)
+   pm_runtime_disable(>dev);
 
return 0;
 }
-- 
2.6.2

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

[PATCH 2/2] i2c: designware: Allow build Baytrail semaphore support when IOSF_MBI=m

[Jarkko Nikula]

I believe i2c-designware-baytrail.c doesn't have strict dependency that
Intel SoC IOSF Sideband support must be always built-in in order to be
able to compile support for Intel Baytrail I2C bus sharing HW semaphore.

Redefine build dependencies so that CONFIG_IOSF_MBI=y is required only
when CONFIG_I2C_DESIGNWARE_PLATFORM is built-in.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
Hi David. Can you ack/nak this patch as I'm not fully familiar with this
HW semaphore can there be problems when IOSF_MBI is built as a module.
At least I'm getting similar sensible looking "punit semaphore
acquired/held for x ms" debug messages when I modprobe/rmmod
i2c_designware_platform independently is the CONFIG_IOSF_MBI=y or =m.
---
 drivers/i2c/busses/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 69c46fe13777..76f4d024def0 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -490,7 +490,9 @@ config I2C_DESIGNWARE_PCI
 
 config I2C_DESIGNWARE_BAYTRAIL
bool "Intel Baytrail I2C semaphore support"
-   depends on I2C_DESIGNWARE_PLATFORM && IOSF_MBI=y && ACPI
+   depends on ACPI
+   depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
+  (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)
help
  This driver enables managed host access to the PMIC I2C bus on select
  Intel BayTrail platforms using the X-Powers AXP288 PMIC. It allows
-- 
2.6.2

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

Re: [PATCH 2/2] i2c: designware: Allow build Baytrail semaphore support when IOSF_MBI=m

[Jarkko Nikula]

On 12/10/2015 02:59 PM, Andy Shevchenko wrote:

On Thu, 2015-12-10 at 13:48 +0200, Jarkko Nikula wrote:

I believe i2c-designware-baytrail.c doesn't have strict dependency
that
Intel SoC IOSF Sideband support must be always built-in in order to
be
able to compile support for Intel Baytrail I2C bus sharing HW
semaphore.

Redefine build dependencies so that CONFIG_IOSF_MBI=y is required
only
when CONFIG_I2C_DESIGNWARE_PLATFORM is built-in.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
Hi David. Can you ack/nak this patch as I'm not fully familiar with
this
HW semaphore can there be problems when IOSF_MBI is built as a
module.




At least I'm getting similar sensible looking "punit semaphore
acquired/held for x ms" debug messages when I modprobe/rmmod
i2c_designware_platform independently is the CONFIG_IOSF_MBI=y or =m.
---
  drivers/i2c/busses/Kconfig | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 69c46fe13777..76f4d024def0 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -490,7 +490,9 @@ config I2C_DESIGNWARE_PCI

  config I2C_DESIGNWARE_BAYTRAIL
bool "Intel Baytrail I2C semaphore support"
-   depends on I2C_DESIGNWARE_PLATFORM && IOSF_MBI=y && ACPI
+   depends on ACPI
+   depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
+  (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)


Would it be more readable in the following way

depends on ACPI
depends on I2C_DESIGNWARE_PLATFORM
depends on IOSF_MBI if I2C_DESIGNWARE_PLATFORM=m
depends on IOSF_MBI=y if I2C_DESIGNWARE_PLATFORM=y

For my eyes it looks a bit more complex but I think it's matter of 
taste. However, the syntax you are proposing is not supported for 
"depends on" like it is for "select" statements.


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

Re: [PATCH RESEND] i2c: Remove setting for 1 second timeout from adapter drivers

[Jarkko Nikula]

On 12/03/2015 10:52 PM, Wolfram Sang wrote:

On Wed, Dec 02, 2015 at 01:02:34PM +0200, Jarkko Nikula wrote:

I2C adapter drivers that are using 1 second timeout can leave the timeout
unset and let the i2c-core.c: i2c_register_adapter() to set it instead.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>


My take on this is that I prefer the explicit (and thus visible)
assignment. What the core does was intended as a fallback to prevent
strange things happening with a 0 value.

Fair enough. It's hard to say if some of these really needs to be 1 
second instead of being stetson guessed or copy-pasted. Now if we ever 
change the fall back value from HZ to HZ/10 or similar we might get a 
regression.


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

[PATCH RESEND] i2c: Remove setting for 1 second timeout from adapter drivers

[Jarkko Nikula]

I2C adapter drivers that are using 1 second timeout can leave the timeout
unset and let the i2c-core.c: i2c_register_adapter() to set it instead.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-ibm_iic.c  | 1 -
 drivers/i2c/busses/i2c-iop3xx.c   | 1 -
 drivers/i2c/busses/i2c-mpc.c  | 1 -
 drivers/i2c/busses/i2c-mv64xxx.c  | 5 -
 drivers/i2c/busses/i2c-pca-platform.c | 1 -
 5 files changed, 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index ab492301581a..5dc0bb7e5dbc 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -748,7 +748,6 @@ static int iic_probe(struct platform_device *ofdev)
i2c_set_adapdata(adap, dev);
adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
adap->algo = _algo;
-   adap->timeout = HZ;
 
ret = i2c_add_adapter(adap);
if (ret  < 0) {
diff --git a/drivers/i2c/busses/i2c-iop3xx.c b/drivers/i2c/busses/i2c-iop3xx.c
index 72d6161cf77c..c4ed8281bec2 100644
--- a/drivers/i2c/busses/i2c-iop3xx.c
+++ b/drivers/i2c/busses/i2c-iop3xx.c
@@ -479,7 +479,6 @@ iop3xx_i2c_probe(struct platform_device *pdev)
/*
 * Default values...should these come in from board code?
 */
-   new_adapter->timeout = HZ;
new_adapter->algo = _i2c_algo;
 
init_waitqueue_head(_data->waitq);
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 48ecffecc0ed..b4bb64ffe3cd 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -640,7 +640,6 @@ static const struct i2c_algorithm mpc_algo = {
 static struct i2c_adapter mpc_ops = {
.owner = THIS_MODULE,
.algo = _algo,
-   .timeout = HZ,
 };
 
 static const struct of_device_id mpc_i2c_of_match[];
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 43207f52e5a3..eaef7763f5fc 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -839,11 +839,6 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
reset_control_deassert(drv_data->rstc);
}
 
-   /* Its not yet defined how timeouts will be specified in device tree.
-* So hard code the value to 1 second.
-*/
-   drv_data->adapter.timeout = HZ;
-
device = of_match_device(mv64xxx_i2c_of_match_table, dev);
if (!device)
return -ENODEV;
diff --git a/drivers/i2c/busses/i2c-pca-platform.c 
b/drivers/i2c/busses/i2c-pca-platform.c
index 3bd2e7d06e4b..2b930df0e6b5 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -183,7 +183,6 @@ static int i2c_pca_pf_probe(struct platform_device *pdev)
i2c->algo_data.i2c_clock = platform_data->i2c_clock_speed;
i2c->gpio = platform_data->gpio;
} else {
-   i2c->adap.timeout = HZ;
i2c->algo_data.i2c_clock = 59000;
i2c->gpio = -1;
}
-- 
2.6.2

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

Re: [PATCH v1 13/13] i2c: designware: Convert to use unified device property API

[Jarkko Nikula]

On 11/24/2015 12:22 PM, Andy Shevchenko wrote:

From: Mika Westerberg <mika.westerb...@linux.intel.com>

With ACPI _DSD (introduced in ACPI v5.1) it is now possible to pass device
configuration information from ACPI in addition to DT. In order to support
this, convert the driver to use the unified device property accessors
instead of DT specific.

Change to ordering a bit so that we first try platform data and if that's
not available look from device properties. ACPI *CNT methods are then used
as last resort to override everything else.

Signed-off-by: Mika Westerberg <mika.westerb...@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
  drivers/i2c/busses/i2c-designware-platdrv.c | 48 +
  1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579e..e9062be 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -36,6 +36,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -156,33 +157,28 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
/* fast mode by default because of legacy reasons */
clk_freq = 40;

-   if (has_acpi_companion(>dev)) {
-   dw_i2c_acpi_configure(pdev);
-   } else if (pdev->dev.of_node) {
-   of_property_read_u32(pdev->dev.of_node,
-   "i2c-sda-hold-time-ns", );
-
-   of_property_read_u32(pdev->dev.of_node,
-"i2c-sda-falling-time-ns",
->sda_falling_time);
-   of_property_read_u32(pdev->dev.of_node,
-"i2c-scl-falling-time-ns",
->scl_falling_time);
-
-   of_property_read_u32(pdev->dev.of_node, "clock-frequency",
-_freq);
-
-   /* Only standard mode at 100kHz and fast mode at 400kHz
-* are supported.
-*/
-   if (clk_freq != 10 && clk_freq != 40) {
-   dev_err(>dev, "Only 100kHz and 400kHz supported");
-   return -EINVAL;
-   }
+   if ((pdata = dev_get_platdata(>dev))) {
+   clk_freq = pdata->i2c_scl_freq;
} else {
-   pdata = dev_get_platdata(>dev);
-   if (pdata)
-   clk_freq = pdata->i2c_scl_freq;
+   device_property_read_u32(>dev, "i2c-sda-hold-time-ns",
+);
+   device_property_read_u32(>dev, "i2c-sda-falling-time-ns",
+>sda_falling_time);
+   device_property_read_u32(>dev, "i2c-scl-falling-time-ns",
+>scl_falling_time);
+   device_property_read_u32(>dev, "clock-frequency",
+_freq);


Mika, Andy: Was this one able to go separately? At least it builds 
without rest of the set but is there anything that could break DT based 
system if there are no patches 1-8/13?


Acked-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: designware platdrv and runtime pm?

[Jarkko Nikula]

On 09.11.2015 23:15, Wolfram Sang wrote:

Hi guys,

while handling the merge conflict for the designware-platdrv, I noticed
an asymmetry in the runtime PM handling. Currently, code looks like
this:

if (dev->pm_runtime_disabled) {
pm_runtime_forbid(>dev);
} else {
pm_runtime_set_autosuspend_delay(>dev, 1000);
pm_runtime_use_autosuspend(>dev);
pm_runtime_set_active(>dev);
pm_runtime_enable(>dev);
}

r = i2c_dw_probe(dev);
if (r) {
pm_runtime_disable(>dev);
return r;
}

But shouldn't the above error path (and the remove path) also take
dev->pm_runtime_disabled into account and act accordingly?

I think you are right. Which brings another question to my mind do we 
need to have a patch to linux-stable too?


David: Your original commit 894acb2f823b ("i2c: designware: Add Intel 
Baytrail PMIC I2C bus support") doesn't add pm_runtime_disabled test to 
dw_i2c_remove(). I guess there is possibility power down the shared 
controller by having CONFIG_I2C_DESIGNWARE_PLATFORM=m and then unloading 
the driver?


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

Re: [PATCH 1/1] I2C: designware: fix IO timeout issue for AMD controller

[Jarkko Nikula]

On 06.11.2015 06:34, Yu, Xiangliang wrote:

-Original Message-
From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]

--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -783,6 +783,9 @@ irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)

stat = i2c_dw_read_clear_intrbits(dev);


What if the status changes right here, before you go and mask the interrupt?

Have no effect, because i2c controller can't trigger next interrupt.

Does it mean possible lost interrupt too? I guess it can be debugged by 
placing a few ms long mdelay() between clearing and masking.


How frequent is this timeout? I guess lost interrupt is somehow nearly 
related to clearing, masking and unmasking if that cycle helps. One 
thing that comes to my mind is masking needed and could plain unmasking 
be also a working workaround?


Have you tried to print DW_IC_INTR_STAT, DW_IC_INTR_MASK and 
DW_IC_RAW_INTR_STAT when timeout occurs? E.g. just after printing the 
timeout out error in i2c_dw_xfer(). Probably good in i2c_dw_isr() too if 
if doesn't produce too much data and make debugging impossible.


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

[PATCH] platform/chrome: Fix i2c-designware adapter name

[Jarkko Nikula]

Commit d80d134182ba ("i2c: designware: Move common probe code into
i2c_dw_probe()") caused the I2C adapter lookup code here to fail for PCI
enumerated i2c-designware because commit changed the adapter name but
didn't update it here.

Fix the I2C adapter lookup by using the "Synopsys DesignWare I2C adapter"
name.

Reported-by: Jeremiah Mahler <jmmah...@gmail.com>
Fixes: d80d134182ba ("i2c: designware: Move common probe code into 
i2c_dw_probe()")
Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
Hi Jeremiah. This is the same diff I had in a reply to your bug report.
Can you test does this fix work for you as I don't have the HW.
---
 drivers/platform/chrome/chromeos_laptop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/chromeos_laptop.c 
b/drivers/platform/chrome/chromeos_laptop.c
index 02072749fff3..2b441e9ae593 100644
--- a/drivers/platform/chrome/chromeos_laptop.c
+++ b/drivers/platform/chrome/chromeos_laptop.c
@@ -47,8 +47,8 @@ static const char *i2c_adapter_names[] = {
"SMBus I801 adapter",
"i915 gmbus vga",
"i915 gmbus panel",
-   "i2c-designware-pci",
-   "i2c-designware-pci",
+   "Synopsys DesignWare I2C adapter",
+   "Synopsys DesignWare I2C adapter",
 };
 
 /* Keep this enum consistent with i2c_adapter_names */
-- 
2.6.2

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

Re: Issues with touchpad / touchscreen on yoga 900

[Jarkko Nikula]

Hi

On 10/29/2015 08:30 PM, Wolfram Sang wrote:

On Thu, Oct 29, 2015 at 11:29:13AM -0600, Kevin Fenzi wrote:

Greetings.

I'm having problems with a lenovo yoga 900 not seeing it's touchscreen
or touchpad.

Downstream bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1275718

The problem parts of dmesg seem to be:

Oct 26 15:20:08 localhost kernel: i2c_designware i2c_designware.1: 
i2c_dw_handle_tx_abort: lost arbitration
Oct 26 15:20:08 localhost kernel: i2c_hid i2c-SYNA2B29:00: hid_descr_cmd failed
Oct 26 15:20:08 localhost kernel: i2c_designware i2c_designware.2: 
i2c_dw_handle_tx_abort: lost arbitration
Oct 26 15:20:08 localhost kernel: i2c_hid i2c-ELAN21EF:00:
hid_descr_cmd failed

This is with 4.3.0-0.rc7.git1.1.fc24.x86_64 (post rc7 4.3.0).

Happy to gather further info or try patches. ;)


Could you try this test patch from Mika that prints the parameters it 
gets from ACPI and forces using calculated ones (your quirk was supposed 
to be doing the same):


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

Another fix from Mika that comes to my mind is "HID: multitouch: Fetch 
feature reports on demand for Win8 devices" which is sitting in 
linux-next. Not sure is it related but probably worth to try.


https://lkml.org/lkml/2015/9/28/404

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

Re: [PATCH] i2c: designware: Don't mask TX_EMPTY if write is in progress

[Jarkko Nikula]

Hi

On 10/30/2015 09:37 AM, Jisheng Zhang wrote:

Currently when i2c_msg index search is completed, TX_EMPTY interrupt
will be masked. But if the size of i2c_msg data is longer than the size
of the tx buffer, we still need TX_EMPTY interrupt, otherwise we will
get "controller timed out" error.

This patch fixes this issue by only masking TX_EMPTY if i2c_msg index
search is completed and write is not in progress.

Signed-off-by: Jisheng Zhang 
---
  drivers/i2c/busses/i2c-designware-core.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 7441cdc..a2eb212 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -542,10 +542,11 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
}

/*
-* If i2c_msg index search is completed, we don't need TX_EMPTY
-* interrupt any more.
+* If i2c_msg index search is completed and writing is not in progress,
+* we don't need TX_EMPTY interrupt any more.
 */
-   if (dev->msg_write_idx == dev->msgs_num)
+   if (dev->msg_write_idx == dev->msgs_num &&
+   !(dev->status & STATUS_WRITE_IN_PROGRESS))
intr_mask &= ~DW_IC_INTR_TX_EMPTY;

This looks a possible scenario here. What I'm wondering how is the 
transfer ending and what condition in i2c_dw_isr() causes the complete() 
call?


Assuming we break the for loop in i2c_dw_xfer_msg() with buf_len > 0 and 
dev->msg_write_idx == dev->msgs_num how we get back inside the for loop 
for writing the remaining bytes and stop command?


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

[PATCH] i2c: i801: Document Intel DNV and Broxton

[Jarkko Nikula]

Add missing entries into i2c-i801 documentation and Kconfig about recently
added Intel DNV and Broxton.

Suggested-by: Jean Delvare <jdelv...@suse.de>
Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
Cc: Mika Westerberg <mika.westerb...@linux.intel.com>
---
 Documentation/i2c/busses/i2c-i801 | 2 ++
 drivers/i2c/busses/Kconfig| 2 ++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/i2c/busses/i2c-i801 
b/Documentation/i2c/busses/i2c-i801
index 82f48f774afb..6a4b1af724f8 100644
--- a/Documentation/i2c/busses/i2c-i801
+++ b/Documentation/i2c/busses/i2c-i801
@@ -30,6 +30,8 @@ Supported adapters:
   * Intel BayTrail (SOC)
   * Intel Sunrise Point-H (PCH)
   * Intel Sunrise Point-LP (PCH)
+  * Intel DNV (SOC)
+  * Intel Broxton (SOC)
Datasheets: Publicly available at the Intel website
 
 On Intel Patsburg and later chipsets, both the normal host SMBus controller
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 0774f18f5716..45e1a7ec617d 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -124,6 +124,8 @@ config I2C_I801
BayTrail (SOC)
Sunrise Point-H (PCH)
Sunrise Point-LP (PCH)
+   DNV (SOC)
+   Broxton (SOC)
 
  This driver can also be built as a module.  If so, the module
  will be called i2c-i801.
-- 
2.6.1

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

Re: [PATCH] i2c: i801: Add support for Intel Broxton

[Jarkko Nikula]

Hi

On 10/25/2015 12:50 PM, Jean Delvare wrote:

@@ -204,6 +205,7 @@
  #define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS  0xa123
  #define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS 0x9d23
  #define PCI_DEVICE_ID_INTEL_DNV_SMBUS 0x19df
+#define PCI_DEVICE_ID_INTEL_BROXTON_SMBUS  0x5ad4


Can you please get this added to pci.ids?

http://pci-ids.ucw.cz/read/PC/8086


We seems to miss quite many other recent IDs too. I'll take this forward.


  struct i801_mux_config {
char *gpio_chip;
@@ -866,6 +868,7 @@ static const struct pci_device_id i801_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 
PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 
PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_DNV_SMBUS) },
+   { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BROXTON_SMBUS) },
{ 0, }
  };



Does this one not have FEATURE_TCO as DNV does? If it does, you'll need
to add a line for it in the switch block in i801_probe().


No, it's specific to sunrisepoint and dnv only at the moment.


Please also update Documentation/i2c/busses/i2c-i801 and
drivers/i2c/busses/Kconfig.


Ok, will do.

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

Re: [PATCH v4 2/2] i2c: add ACPI support for I2C mux ports

[Jarkko Nikula]

On 10/23/2015 04:40 PM, Mika Westerberg wrote:

On Fri, Oct 23, 2015 at 04:13:11PM +0300, Mika Westerberg wrote:

On Fri, Oct 23, 2015 at 12:16:11PM +0200, Wolfram Sang wrote:

On Fri, Oct 23, 2015 at 11:40:13AM +0300, Mika Westerberg wrote:

On Thu, Oct 22, 2015 at 02:17:42AM -0700, Dustin Byford wrote:

Ah, forgot to mention that the i2c-designware-pcidrv.c misses include of
 so that needs to be fixed. Otherwise we get:

drivers/i2c/busses/i2c-designware-pcidrv.c: In function ‘i2c_dw_pci_probe’:
drivers/i2c/busses/i2c-designware-pcidrv.c:259:2: error: implicit declaration 
of function ‘ACPI_COMPANION_SET’ [-Werror=implicit-function-declaration]
drivers/i2c/busses/i2c-designware-pcidrv.c:259:33: error: implicit declaration 
of function ‘ACPI_COMPANION’ [-Werror=implicit-function-declaration]

Please also check that patches apply on top of i2c/for-next due recently 
applied cleanup patches from me.


I think commit d80d134182ba ("i2c: designware: Move common probe code 
into i2c_dw_probe()") makes your patch only a 2 liner to 
drivers/i2c/busses/i2c-designware-core.c :-)


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

[PATCH] i2c: i801: Add support for Intel Broxton

[Jarkko Nikula]

This patch adds the SMBUS PCI ID of Intel Broxton.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
This goes on top of Mika's "[PATCH] i2c: i801: Add support for Intel DNV":
http://marc.info/?l=linux-i2c=14447401042=2
---
 drivers/i2c/busses/i2c-i801.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 47c2ddf76264..d8219bc2ac4e 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -61,6 +61,7 @@
  * Sunrise Point-H (PCH)   0xa123  32  hardyes yes yes
  * Sunrise Point-LP (PCH)  0x9d23  32  hardyes yes yes
  * DNV (SOC)   0x19df  32  hardyes yes yes
+ * Broxton (SOC)   0x5ad4  32  hardyes yes yes
  *
  * Features supported by this driver:
  * Software PECno
@@ -204,6 +205,7 @@
 #define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS   0xa123
 #define PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS  0x9d23
 #define PCI_DEVICE_ID_INTEL_DNV_SMBUS  0x19df
+#define PCI_DEVICE_ID_INTEL_BROXTON_SMBUS  0x5ad4
 
 struct i801_mux_config {
char *gpio_chip;
@@ -866,6 +868,7 @@ static const struct pci_device_id i801_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 
PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 
PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_SMBUS) },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_DNV_SMBUS) },
+   { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BROXTON_SMBUS) },
{ 0, }
 };
 
-- 
2.6.1

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

Re: [PATCH v2 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Jarkko Nikula]

Hi

On 10/20/2015 07:32 PM, Wolfram Sang wrote:

There was a merge conflict with a bugfix from i2c/for-current. I think
it is okay, but you may want to double check my i2c/for-next.

Looks like pm_runtime_disable() got dropped from your 36d48fb5766a 
("i2c: designware-platdrv: enable RuntimePM before registering to the 
core") while handling the merge conflict. I'll send a fix.



What about this irq-clearing-in-probe thingie on top of this series? :)

I'll have a look at it. What's not entirely clear to me would it be 
no-op or not. HW is actually disabled after i2c_dw_init() which is 
called before requesting the interrupt but is not clear to me from the 
spec does HW clear interrupts while it goes idle.


So as a result I'd expect either a explicit interrupt clearing patch (to 
be more robust against potential unmasking changes) or a comment in 
__i2c_dw_enable() :-)


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

[PATCH] i2c: designware: Disable runtime PM in case i2c_dw_probe() fails

[Jarkko Nikula]

Call to pm_runtime_disable() got dropped while handling the merge conflict
between commit 36d48fb5766a ("i2c: designware-platdrv: enable RuntimePM
before registering to the core") and commit d80d134182ba ("i2c: designware:
Move common probe code into i2c_dw_probe()").

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index eb55760ccd9f..8fd9d4a18bd5 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -261,8 +261,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
}
 
r = i2c_dw_probe(dev);
-   if (r)
+   if (r) {
+   pm_runtime_disable(>dev);
return r;
+   }
 
return 0;
 }
-- 
2.6.1

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

Re: linux-next: Tree for Oct 21 (drivers/i2c/busses/i2c-designware-platdrv.c)

[Jarkko Nikula]

Hi

On 10/21/2015 11:28 PM, Randy Dunlap wrote:

On 10/20/15 23:16, Stephen Rothwell wrote:

Hi all,

There will be no linux-next releases after tomorrow until Nov 2 (kernel
summit).

Changes since 20151020:



on i386 or x86_64:

when CONFIG_PM_SLEEP is not enabled:

../drivers/i2c/busses/i2c-designware-platdrv.c:340:13: error: 
'dw_i2c_plat_prepare' undeclared here (not in a function)
   .prepare = dw_i2c_plat_prepare,
  ^
../drivers/i2c/busses/i2c-designware-platdrv.c:341:14: error: 
'dw_i2c_plat_complete' undeclared here (not in a function)
   .complete = dw_i2c_plat_complete,
   ^

Here's the fix for this:

http://marc.info/?l=linux-i2c=144541136917692=2

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

[PATCH] i2c: designware: Fix build error when !CONFIG_PM_SLEEP

[Jarkko Nikula]

Commit 6ad6fde3970c ("i2c: designware: Rename platform driver probe and PM
functions") introduced "'dw_i2c_plat_prepare' undeclared here" and
"'dw_i2c_plat_complete' undeclared here" build errors when CONFIG_PM_SLEEP
is not set.

Fix this by renaming NULL defined dw_i2c_prepare and dw_i2c_complete PM
hooks to dw_i2c_plat_prepare and dw_i2c_plat_complete since this was
obviously missing from the commit.

Reported-by: kbuild test robot <fengguang...@intel.com>
Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index f6b252a8ffd1..eb55760ccd9f 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -307,8 +307,8 @@ static void dw_i2c_plat_complete(struct device *dev)
pm_request_resume(dev);
 }
 #else
-#define dw_i2c_prepare NULL
-#define dw_i2c_completeNULL
+#define dw_i2c_plat_prepareNULL
+#define dw_i2c_plat_complete   NULL
 #endif
 
 #ifdef CONFIG_PM
-- 
2.6.1

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

Re: [PATCH 0/6] i2c: designware: Code duplication removal and cleanups

[Jarkko Nikula]

Hi

On 10/15/2015 03:18 PM, Wolfram Sang wrote:

Jarkko, would you be interested in maintaining the designware driver?
For any non-trivial patch to this driver, I'd need assistance.

Yes I can. I think you could add also Mika and Andy as all of us are 
actively supporting it.


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

Re: [PATCH 2/2] i2c: designware: enable High-speed mode for pcidrv

[Jarkko Nikula]

On 10/15/2015 08:46 AM, Xiang Wang wrote:

1. "bus speed mode" is a bit different from other parameters. Actually
it can be determined by the speed setting of "i2c devices" in ACPI
(I2CSerialBus). E.g. If i2c device uses 3MHz, we use High-speed mode
for this i2c bus.
2. If we hardcode speed setting in pci driver, we lose the
flexibility. A high-speed device may be connected to this i2c bus on
this board, but may be connected to another i2c bus on another board
design.

In this patch, we enumerate the i2c device in ACPI table to determine
the frequency setting. Then we set corresponding speed mode for this
i2c controller. The ACPI stuff is common for pci and plat driver. If
board design changes, we only change BIOS.

In conclusion, we have 2 solutions to set the i2c controller speed
mode (pci driver):
1) use hardcode value in pci driver
2) use frequency setting of "i2c device" in ACPI table (more flexible,
but looks a bit strange)

Do you have any preference/suggestions for above solutions? Thanks


I don't think we can hard code especially the high-speed mode because 
most typically buses are populated with slower devices.


Things are a bit more clear when ACPI provides timing parameters for the 
bus (for standard and fast speed modes at the moment in 
i2c-designware-platdrv.c: dw_i2c_acpi_configure()) but still I think the 
ACPI namespace walk may be needed against potential BIOS 
misconfigurations. For instance if it provides timing parameters for all 
speeds but there are devices with lower speed on the same bus.


I'd take these timing parameters as configuration data for bus features 
but actual speed (speed bits in IC_CON register) is defined separately. 
To me it looks only way to achieve that is to pick slowest device from 
I2cSerialBus resource descriptors.


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

[PATCH v2 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Jarkko Nikula]

There is some code duplication in i2c-designware-platdrv and
i2c-designware-pcidrv probe functions. What is even worse that duplication
requires i2c_dw_xfer(), i2c_dw_func() and i2c_dw_isr() i2c-designware-core
functions to be exported.

Therefore move common code into new i2c_dw_probe() and make functions above
local to i2c-designware-core.

While merging the code patch does following functional changes:

- I2C Adapter name will be "Synopsys DesignWare I2C adapter". Previously it
  was used for platform and ACPI devices but PCI device used
  "i2c-designware-pci".
- Using device name for interrupt name. Previous it was platform device name,
  ACPI device name or "i2c-designware-pci".
- Error code from devm_request_irq() and i2c_add_numbered_adapter() will be
  printed in case of error.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
v2:
- Adapter name changed to "Synopsys DesignWare I2C adapter"
- Using device name for interrupt name
---
 drivers/i2c/busses/i2c-designware-core.c| 49 +
 drivers/i2c/busses/i2c-designware-core.h|  5 +--
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 30 ++
 drivers/i2c/busses/i2c-designware-platdrv.c | 28 ++---
 4 files changed, 49 insertions(+), 63 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 52272a01c679..8c48b27ba059 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -618,7 +618,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 /*
  * Prepare controller for a transaction and call i2c_dw_xfer_msg
  */
-int
+static int
 i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 {
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
@@ -702,14 +702,17 @@ done_nolock:
 
return ret;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_xfer);
 
-u32 i2c_dw_func(struct i2c_adapter *adap)
+static u32 i2c_dw_func(struct i2c_adapter *adap)
 {
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
return dev->functionality;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_func);
+
+static struct i2c_algorithm i2c_dw_algo = {
+   .master_xfer= i2c_dw_xfer,
+   .functionality  = i2c_dw_func,
+};
 
 static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
 {
@@ -770,7 +773,7 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev 
*dev)
  * Interrupt service routine. This gets called whenever an I2C interrupt
  * occurs.
  */
-irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
+static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 {
struct dw_i2c_dev *dev = dev_id;
u32 stat, enabled;
@@ -813,7 +816,6 @@ tx_aborted:
 
return IRQ_HANDLED;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_isr);
 
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
@@ -838,5 +840,40 @@ u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev)
 }
 EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
 
+int i2c_dw_probe(struct dw_i2c_dev *dev)
+{
+   struct i2c_adapter *adap = >adapter;
+   int r;
+
+   init_completion(>cmd_complete);
+   mutex_init(>lock);
+
+   r = i2c_dw_init(dev);
+   if (r)
+   return r;
+
+   snprintf(adap->name, sizeof(adap->name),
+"Synopsys DesignWare I2C adapter");
+   adap->algo = _dw_algo;
+   adap->dev.parent = dev->dev;
+   i2c_set_adapdata(adap, dev);
+
+   i2c_dw_disable_int(dev);
+   r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED,
+dev_name(dev->dev), dev);
+   if (r) {
+   dev_err(dev->dev, "failure requesting irq %i: %d\n",
+   dev->irq, r);
+   return r;
+   }
+
+   r = i2c_add_numbered_adapter(adap);
+   if (r)
+   dev_err(dev->dev, "failure adding adapter: %d\n", r);
+
+   return r;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_probe);
+
 MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
 MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-core.h 
b/drivers/i2c/busses/i2c-designware-core.h
index 0e73b8672402..1d50898e7b24 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -113,13 +113,10 @@ struct dw_i2c_dev {
 #define ACCESS_16BIT   0x0002
 
 extern int i2c_dw_init(struct dw_i2c_dev *dev);
-extern int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
-   int num);
-extern u32 i2c_dw_func(struct i2c_adapter *adap);
-extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
+extern int i2c_dw_probe(struct dw_i2c_dev *dev);
 
 #if IS_ENABLED(CONFIG_I2C_

[PATCH] i2c: Remove setting for 1 second timeout from adapter drivers

[Jarkko Nikula]

I2C adapter drivers that are using 1 second timeout can leave the timeout
unset and let the i2c-core.c: i2c_register_adapter() to set it instead.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-ibm_iic.c  | 1 -
 drivers/i2c/busses/i2c-iop3xx.c   | 1 -
 drivers/i2c/busses/i2c-mpc.c  | 1 -
 drivers/i2c/busses/i2c-mv64xxx.c  | 5 -
 drivers/i2c/busses/i2c-pca-platform.c | 1 -
 5 files changed, 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 722f839cfa3c..13fda39acffe 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -748,7 +748,6 @@ static int iic_probe(struct platform_device *ofdev)
i2c_set_adapdata(adap, dev);
adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
adap->algo = _algo;
-   adap->timeout = HZ;
 
ret = i2c_add_adapter(adap);
if (ret  < 0) {
diff --git a/drivers/i2c/busses/i2c-iop3xx.c b/drivers/i2c/busses/i2c-iop3xx.c
index 72d6161cf77c..c4ed8281bec2 100644
--- a/drivers/i2c/busses/i2c-iop3xx.c
+++ b/drivers/i2c/busses/i2c-iop3xx.c
@@ -479,7 +479,6 @@ iop3xx_i2c_probe(struct platform_device *pdev)
/*
 * Default values...should these come in from board code?
 */
-   new_adapter->timeout = HZ;
new_adapter->algo = _i2c_algo;
 
init_waitqueue_head(_data->waitq);
diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 48ecffecc0ed..b4bb64ffe3cd 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -640,7 +640,6 @@ static const struct i2c_algorithm mpc_algo = {
 static struct i2c_adapter mpc_ops = {
.owner = THIS_MODULE,
.algo = _algo,
-   .timeout = HZ,
 };
 
 static const struct of_device_id mpc_i2c_of_match[];
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 30059c1df2a3..2640390be5dd 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -832,11 +832,6 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
reset_control_deassert(drv_data->rstc);
}
 
-   /* Its not yet defined how timeouts will be specified in device tree.
-* So hard code the value to 1 second.
-*/
-   drv_data->adapter.timeout = HZ;
-
device = of_match_device(mv64xxx_i2c_of_match_table, dev);
if (!device)
return -ENODEV;
diff --git a/drivers/i2c/busses/i2c-pca-platform.c 
b/drivers/i2c/busses/i2c-pca-platform.c
index 3bd2e7d06e4b..2b930df0e6b5 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -183,7 +183,6 @@ static int i2c_pca_pf_probe(struct platform_device *pdev)
i2c->algo_data.i2c_clock = platform_data->i2c_clock_speed;
i2c->gpio = platform_data->gpio;
} else {
-   i2c->adap.timeout = HZ;
i2c->algo_data.i2c_clock = 59000;
i2c->gpio = -1;
}
-- 
2.5.1

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

[PATCH 4/6] i2c: designware: Make dw_readl() and dw_writel() static

[Jarkko Nikula]

dw_readl() and dw_writel() are not used outside of i2c-designware-core and
they are not exported so make them static and remove their forward
declaration.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-core.c | 4 ++--
 drivers/i2c/busses/i2c-designware-core.h | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index c9e93a76a455..52272a01c679 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -165,7 +165,7 @@ static char *abort_sources[] = {
"lost arbitration",
 };
 
-u32 dw_readl(struct dw_i2c_dev *dev, int offset)
+static u32 dw_readl(struct dw_i2c_dev *dev, int offset)
 {
u32 value;
 
@@ -181,7 +181,7 @@ u32 dw_readl(struct dw_i2c_dev *dev, int offset)
return value;
 }
 
-void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
+static void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
 {
if (dev->accessor_flags & ACCESS_SWAP)
b = swab32(b);
diff --git a/drivers/i2c/busses/i2c-designware-core.h 
b/drivers/i2c/busses/i2c-designware-core.h
index 10c79038834d..0e73b8672402 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -112,8 +112,6 @@ struct dw_i2c_dev {
 #define ACCESS_SWAP0x0001
 #define ACCESS_16BIT   0x0002
 
-extern u32 dw_readl(struct dw_i2c_dev *dev, int offset);
-extern void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset);
 extern int i2c_dw_init(struct dw_i2c_dev *dev);
 extern int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
int num);
-- 
2.5.0

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

[PATCH 6/6] i2c: designware: Move common probe code into i2c_dw_probe()

[Jarkko Nikula]

There is some code duplication in i2c-designware-platdrv and
i2c-designware-pcidrv probe functions. What is even worse that duplication
requires i2c_dw_xfer(), i2c_dw_func() and i2c_dw_isr() i2c-designware-core
functions to be exported.

Therefore move common code into new i2c_dw_probe() and make functions above
local to i2c-designware-core.

While merging the code patch does following functional changes:

- I2C Adapter name will be "i2c-designware". Previously adapter name was
  "Synopsys DesignWare I2C adapter" for platform and ACPI devices and
  "i2c-designware-pci" for PCI devices.
- Interrupt name too will be "i2c-designware". Previous it was platform
  device name, ACPI device name or "i2c-designware-pci".
- Error code from devm_request_irq() and i2c_add_numbered_adapter() will be
  printed in case of error.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-core.c| 48 +
 drivers/i2c/busses/i2c-designware-core.h|  5 +--
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 30 ++
 drivers/i2c/busses/i2c-designware-platdrv.c | 28 ++---
 4 files changed, 48 insertions(+), 63 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 52272a01c679..501c8ac0cf14 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -618,7 +618,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 /*
  * Prepare controller for a transaction and call i2c_dw_xfer_msg
  */
-int
+static int
 i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 {
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
@@ -702,14 +702,17 @@ done_nolock:
 
return ret;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_xfer);
 
-u32 i2c_dw_func(struct i2c_adapter *adap)
+static u32 i2c_dw_func(struct i2c_adapter *adap)
 {
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
return dev->functionality;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_func);
+
+static struct i2c_algorithm i2c_dw_algo = {
+   .master_xfer= i2c_dw_xfer,
+   .functionality  = i2c_dw_func,
+};
 
 static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
 {
@@ -770,7 +773,7 @@ static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev 
*dev)
  * Interrupt service routine. This gets called whenever an I2C interrupt
  * occurs.
  */
-irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
+static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 {
struct dw_i2c_dev *dev = dev_id;
u32 stat, enabled;
@@ -813,7 +816,6 @@ tx_aborted:
 
return IRQ_HANDLED;
 }
-EXPORT_SYMBOL_GPL(i2c_dw_isr);
 
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
@@ -838,5 +840,39 @@ u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev)
 }
 EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param);
 
+int i2c_dw_probe(struct dw_i2c_dev *dev)
+{
+   struct i2c_adapter *adap = >adapter;
+   int r;
+
+   init_completion(>cmd_complete);
+   mutex_init(>lock);
+
+   r = i2c_dw_init(dev);
+   if (r)
+   return r;
+
+   snprintf(adap->name, sizeof(adap->name), "i2c-designware");
+   adap->algo = _dw_algo;
+   adap->dev.parent = dev->dev;
+   i2c_set_adapdata(adap, dev);
+
+   i2c_dw_disable_int(dev);
+   r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr, IRQF_SHARED,
+adap->name, dev);
+   if (r) {
+   dev_err(dev->dev, "failure requesting irq %i: %d\n",
+   dev->irq, r);
+   return r;
+   }
+
+   r = i2c_add_numbered_adapter(adap);
+   if (r)
+   dev_err(dev->dev, "failure adding adapter: %d\n", r);
+
+   return r;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_probe);
+
 MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter core");
 MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-core.h 
b/drivers/i2c/busses/i2c-designware-core.h
index 0e73b8672402..1d50898e7b24 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -113,13 +113,10 @@ struct dw_i2c_dev {
 #define ACCESS_16BIT   0x0002
 
 extern int i2c_dw_init(struct dw_i2c_dev *dev);
-extern int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[],
-   int num);
-extern u32 i2c_dw_func(struct i2c_adapter *adap);
-extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
+extern int i2c_dw_probe(struct dw_i2c_dev *dev);
 
 #if IS_ENABLED(CONFIG_I2C_DESIGNWARE_BAYTRAIL)
 extern int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busse

[PATCH 5/6] i2c: designware: Rename platform driver probe and PM functions

[Jarkko Nikula]

Make it easier to distinguish between i2c-designware-platdrv and
i2c-designware-core functions and to be consistent with
i2c-designware-pcidrv.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3dd2de31a2f8..17167cd4812d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -133,7 +133,7 @@ static inline int dw_i2c_acpi_configure(struct 
platform_device *pdev)
 static inline void dw_i2c_acpi_unconfigure(struct platform_device *pdev) { }
 #endif
 
-static int dw_i2c_probe(struct platform_device *pdev)
+static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
struct dw_i2c_dev *dev;
struct i2c_adapter *adap;
@@ -271,7 +271,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
return 0;
 }
 
-static int dw_i2c_remove(struct platform_device *pdev)
+static int dw_i2c_plat_remove(struct platform_device *pdev)
 {
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
 
@@ -300,12 +300,12 @@ MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #endif
 
 #ifdef CONFIG_PM_SLEEP
-static int dw_i2c_prepare(struct device *dev)
+static int dw_i2c_plat_prepare(struct device *dev)
 {
return pm_runtime_suspended(dev);
 }
 
-static void dw_i2c_complete(struct device *dev)
+static void dw_i2c_plat_complete(struct device *dev)
 {
if (dev->power.direct_complete)
pm_request_resume(dev);
@@ -316,7 +316,7 @@ static void dw_i2c_complete(struct device *dev)
 #endif
 
 #ifdef CONFIG_PM
-static int dw_i2c_suspend(struct device *dev)
+static int dw_i2c_plat_suspend(struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
@@ -327,7 +327,7 @@ static int dw_i2c_suspend(struct device *dev)
return 0;
 }
 
-static int dw_i2c_resume(struct device *dev)
+static int dw_i2c_plat_resume(struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
@@ -341,10 +341,10 @@ static int dw_i2c_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
-   .prepare = dw_i2c_prepare,
-   .complete = dw_i2c_complete,
-   SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_suspend, dw_i2c_resume)
-   SET_RUNTIME_PM_OPS(dw_i2c_suspend, dw_i2c_resume, NULL)
+   .prepare = dw_i2c_plat_prepare,
+   .complete = dw_i2c_plat_complete,
+   SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
+   SET_RUNTIME_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume, NULL)
 };
 
 #define DW_I2C_DEV_PMOPS (_i2c_dev_pm_ops)
@@ -356,8 +356,8 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
 MODULE_ALIAS("platform:i2c_designware");
 
 static struct platform_driver dw_i2c_driver = {
-   .probe = dw_i2c_probe,
-   .remove = dw_i2c_remove,
+   .probe = dw_i2c_plat_probe,
+   .remove = dw_i2c_plat_remove,
.driver = {
.name   = "i2c_designware",
.of_match_table = of_match_ptr(dw_i2c_of_match),
-- 
2.5.0

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

[PATCH 3/6] i2c: designware: Remove unused functions

[Jarkko Nikula]

i2c_dw_is_enabled() became unused by the commit be58eda775c8
("i2c: designware-pci: Cleanup driver power management") and
i2c_dw_enable() by the commit 3a48d1c08fe0 ("i2c: prevent spurious
interrupt on Designware controllers").

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-core.c | 13 -
 drivers/i2c/busses/i2c-designware-core.h |  2 --
 2 files changed, 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index dec0af7d0471..c9e93a76a455 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -815,19 +815,6 @@ tx_aborted:
 }
 EXPORT_SYMBOL_GPL(i2c_dw_isr);
 
-void i2c_dw_enable(struct dw_i2c_dev *dev)
-{
-   /* Enable the adapter */
-   __i2c_dw_enable(dev, true);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_enable);
-
-u32 i2c_dw_is_enabled(struct dw_i2c_dev *dev)
-{
-   return dw_readl(dev, DW_IC_ENABLE);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_is_enabled);
-
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
/* Disable controller */
diff --git a/drivers/i2c/busses/i2c-designware-core.h 
b/drivers/i2c/busses/i2c-designware-core.h
index f44aeeeb91c6..10c79038834d 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -119,8 +119,6 @@ extern int i2c_dw_xfer(struct i2c_adapter *adap, struct 
i2c_msg msgs[],
int num);
 extern u32 i2c_dw_func(struct i2c_adapter *adap);
 extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
-extern void i2c_dw_enable(struct dw_i2c_dev *dev);
-extern u32 i2c_dw_is_enabled(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
-- 
2.5.0

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

[PATCH 0/6] i2c: designware: Code duplication removal and cleanups

[Jarkko Nikula]

It bothered to me to see "static struct i2c_algorithm i2c_dw_algo {}"
defined twice both in i2c-designware-pcidrv.c and
i2c-designware-platdrv.c and so many exported i2c-designware-core
functions.

It turned out some of them became unused or are local and there were
also duplicated probe code that I moved to new common i2c_dw_probe().

Object sizes below before and after this set from CONFIG_X86_64=y build.

   textdata bss dec hex filename
   64391096   075351d6f 
drivers/i2c/busses/i2c-designware-core.ko
   51231588  1667271a47 drivers/i2c/busses/i2c-designware-pci.ko
   52741096  16638618f2 
drivers/i2c/busses/i2c-designware-platform.ko
  168363780  32   2064850a8 (TOTALS)

   textdata bss dec hex filename
   72251120  16836120a9 
drivers/i2c/busses/i2c-designware-core.ko
   42811524   0580516ad drivers/i2c/busses/i2c-designware-pci.ko
   43371072   054091521 
drivers/i2c/busses/i2c-designware-platform.ko
  158433716  16   195754c77 (TOTALS)

Jarkko Nikula (6):
  i2c: designware: Remove interrupt clearing from i2c_dw_pci_probe()
  i2c: designware: Disable interrupts before requesting PCI device
interrupt
  i2c: designware: Remove unused functions
  i2c: designware: Make dw_readl() and dw_writel() static
  i2c: designware: Rename platform driver probe and PM functions
  i2c: designware: Move common probe code into i2c_dw_probe()

 drivers/i2c/busses/i2c-designware-core.c| 73 ++---
 drivers/i2c/busses/i2c-designware-core.h| 10 +---
 drivers/i2c/busses/i2c-designware-pcidrv.c  | 31 ++--
 drivers/i2c/busses/i2c-designware-platdrv.c | 52 ++--
 4 files changed, 63 insertions(+), 103 deletions(-)

-- 
2.5.0

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

[PATCH 1/6] i2c: designware: Remove interrupt clearing from i2c_dw_pci_probe()

[Jarkko Nikula]

There is no need to clear interrupts in i2c_dw_pci_probe() since only place
where interrupts are unmasked is i2c_dw_xfer_init() and there interrupts
are always cleared after commit 2a2d95e9d6d2 ("i2c: designware: always
clear interrupts before enabling them").

This allows to cleanup the code and replace i2c_dw_clear_int() in
i2c_dw_xfer_init() by direct register read as there are no other callers.

Signed-off-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-core.c   | 8 +---
 drivers/i2c/busses/i2c-designware-core.h   | 1 -
 drivers/i2c/busses/i2c-designware-pcidrv.c | 1 -
 3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 7441cdc1b34a..dec0af7d0471 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -438,7 +438,7 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
__i2c_dw_enable(dev, true);
 
/* Clear and enable interrupts */
-   i2c_dw_clear_int(dev);
+   dw_readl(dev, DW_IC_CLR_INTR);
dw_writel(dev, DW_IC_INTR_DEFAULT_MASK, DW_IC_INTR_MASK);
 }
 
@@ -839,12 +839,6 @@ void i2c_dw_disable(struct dw_i2c_dev *dev)
 }
 EXPORT_SYMBOL_GPL(i2c_dw_disable);
 
-void i2c_dw_clear_int(struct dw_i2c_dev *dev)
-{
-   dw_readl(dev, DW_IC_CLR_INTR);
-}
-EXPORT_SYMBOL_GPL(i2c_dw_clear_int);
-
 void i2c_dw_disable_int(struct dw_i2c_dev *dev)
 {
dw_writel(dev, 0, DW_IC_INTR_MASK);
diff --git a/drivers/i2c/busses/i2c-designware-core.h 
b/drivers/i2c/busses/i2c-designware-core.h
index 9630222abf32..f44aeeeb91c6 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -122,7 +122,6 @@ extern irqreturn_t i2c_dw_isr(int this_irq, void *dev_id);
 extern void i2c_dw_enable(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_is_enabled(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable(struct dw_i2c_dev *dev);
-extern void i2c_dw_clear_int(struct dw_i2c_dev *dev);
 extern void i2c_dw_disable_int(struct dw_i2c_dev *dev);
 extern u32 i2c_dw_read_comp_param(struct dw_i2c_dev *dev);
 
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c 
b/drivers/i2c/busses/i2c-designware-pcidrv.c
index df23e8c30e6f..e477dddcdae5 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -268,7 +268,6 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
}
 
i2c_dw_disable_int(dev);
-   i2c_dw_clear_int(dev);
r = i2c_add_numbered_adapter(adap);
if (r) {
dev_err(>dev, "failure adding adapter\n");
-- 
2.5.0

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

Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves

[Jarkko Nikula]

On 08/25/2015 08:03 AM, Dustin Byford wrote:

On Mon Aug 24 13:52, Jarkko Nikula wrote:


I don't know how common ACPI enumerated I2C hwmon devices are but I guess
they exists since Dustin notices this issue and wrote With that change,
/sys/bus/i2c/devices/i2c-0-004a, for example, became
/sys/bus/i2c/devices/i2c-PNP:xx.


I wouldn't take my particular use case as evidence they are common, or
even they really exist.  I got here because I loaded a hwmon driver
using the _DSD/PRP0001 mechanism in ACPI.  In that case, those devices
have never worked with 'sensors'  Therefore, in that context, this
change is just fixing a bug.

So does this then fall more into new use case rather than being a 
regression as such?


What I'm trying to understand what kind of regression we actually do 
have here? Is it about listing the devices or does it also cease to show 
for instance temperature sensor readings?


If it requires to add a match table to existing driver in order to 
trigger this then at least stable tree is not worry here.


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

[RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves

[Jarkko Nikula]

Commit 70762abb9f89 (i2c: Use stable dev_name for ACPI enumerated I2C
slaves) broke the lm-sensors which relies on I2C hwmon slave devices under
/sys/bus/i2c/devices/ to be named as x-00yz. However if those hwmon
devices are ACPI 5 enumerated their name became i2c-INTABCD:ij and sysfs
code in lm-sensors does not find them anymore:

lib/sysfs.c:665:
if ((!subsys || !strcmp(subsys, i2c)) 
sscanf(dev_name, %hd-%x, entry.chip.bus.nr,
   entry.chip.addr) == 2) {

This patch fixes this by reverting back the old device naming in i2c-core
but at the same avoids regression to ALSA SoC drivers that depend on stable
device binding.

Reverted I2C slave device naming is handled in ALSA SoC core by using the
ACPI name as a component name if device is ACPI probed. This keeps the
component binding in ALSA SoC stable and requires only minimal changes to
affected machine drivers.

Fixes: 70762abb9f89 i2c: Use stable dev_name for ACPI enumerated I2C slaves
Reported-by: Dustin Byford dus...@cumulusnetworks.com
Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
This is on top of 4.2.0-rc8. I didn't check exact kernel versions where this
still applies but I know we would need a few different versions for older
stable versions. Mainly because of added and moved drivers in
sound/soc/intel/.

This is for discussion so I didn't cc sta...@vger.kernel.org yet. I was
thinking would it work if we'd keep the stable name but have an another
symlink in /sys/bus/i2c/devices/ that uses x-00yz name. However this
feels ill-use of devices directory and probably causes more troubles
elsewhere.

I don't know how common ACPI enumerated I2C hwmon devices are but I guess
they exists since Dustin notices this issue and wrote With that change,
/sys/bus/i2c/devices/i2c-0-004a, for example, became
/sys/bus/i2c/devices/i2c-PNP:xx.

It could be possible there is use for the new device naming. I found a
comment from Documentation/hid/hid-sensor.txt introduced by commit
b2eafd7282fd (HID: sensor: Update document for custom sensor) that seems
to indicate current i2c-INTABCD:xy naming may be used from userspace too but
I didn't find examples of that.
---
 drivers/i2c/i2c-core.c   | 21 -
 sound/soc/intel/boards/broadwell.c   |  6 +++---
 sound/soc/intel/boards/byt-max98090.c|  2 +-
 sound/soc/intel/boards/byt-rt5640.c  |  2 +-
 sound/soc/intel/boards/bytcr_rt5640.c|  2 +-
 sound/soc/intel/boards/cht_bsw_max98090_ti.c |  4 ++--
 sound/soc/intel/boards/cht_bsw_rt5645.c  |  4 ++--
 sound/soc/intel/boards/cht_bsw_rt5672.c  |  6 +++---
 sound/soc/intel/boards/haswell.c |  2 +-
 sound/soc/soc-core.c | 10 ++
 10 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index c83e4d13cfc5..e9c227b9dc92 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -913,22 +913,6 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
-static void i2c_dev_set_name(struct i2c_adapter *adap,
-struct i2c_client *client)
-{
-   struct acpi_device *adev = ACPI_COMPANION(client-dev);
-
-   if (adev) {
-   dev_set_name(client-dev, i2c-%s, acpi_dev_name(adev));
-   return;
-   }
-
-   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
-   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
-client-addr | ((client-flags  I2C_CLIENT_TEN)
-? 0xa000 : 0));
-}
-
 /**
  * i2c_new_device - instantiate an i2c device
  * @adap: the adapter managing the device
@@ -987,7 +971,10 @@ i2c_new_device(struct i2c_adapter *adap, struct 
i2c_board_info const *info)
client-dev.of_node = info-of_node;
client-dev.fwnode = info-fwnode;
 
-   i2c_dev_set_name(adap, client);
+   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
+   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
+client-addr | ((client-flags  I2C_CLIENT_TEN)
+? 0xa000 : 0));
status = device_register(client-dev);
if (status)
goto out_err;
diff --git a/sound/soc/intel/boards/broadwell.c 
b/sound/soc/intel/boards/broadwell.c
index 8bafaf6ceab1..3abcf0d7682e 100644
--- a/sound/soc/intel/boards/broadwell.c
+++ b/sound/soc/intel/boards/broadwell.c
@@ -205,7 +205,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = {
.cpu_dai_name = snd-soc-dummy-dai,
.platform_name = snd-soc-dummy,
.no_pcm = 1,
-   .codec_name = i2c-INT343A:00,
+   .codec_name = INT343A:00,
.codec_dai_name = rt286-aif1,
.init = broadwell_rt286_codec_init,
.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF

Re: [RFC 0/1] i2c: acpi: revert setting a stable device name

[Jarkko Nikula]

On 15.08.2015 18:13, Guenter Roeck wrote:

On Fri, Aug 14, 2015 at 12:37:13PM -0700, Dustin Byford wrote:

70762ab from 11/2014 (i2c: Use stable dev_name for ACPI enumerated I2C
slaves) modified the sysfs-visible dev_name() for ACPI enumerated I2C
devices.  With that change, /sys/bus/i2c/devices/i2c-0-004a, for
example, became /sys/bus/i2c/devices/i2c-PNP:xx

That causes problems for userspace code such as 'sensors' which does
this:

lib/sysfs.c:665:
if ((!subsys || !strcmp(subsys, i2c)) 
 sscanf(dev_name, %hd-%x, entry.chip.bus.nr,
entry.chip.addr) == 2) {
...

Therefore, in theory, sensors that were previously visible by running
'sensors' no longer show up.  On the other hand, there are probably few,
if any, cases of this because ACPI enumerated I2C hwmon devices are not
common.

I'm not defending the 'sensors' code, I'm sure there are better ways to
discover a hwmon I2C device from userspace.  But, I'm also not sure


Is it necessary to defend user space applications nowadays if a kernel change
breaks a well established ABI ? WHat happened to Thou Shalt Not Break
Userspace ?

I absolutely agree that i2c bus renumbering across reboots is a problem.
However, it seems to me that 70762ab doesn't solve that problem, it just
paints it over. And, as you have noticed, it introduces new problems along
the way.

This indeed is unwanted and needs to be fixed. Looking at the 
lib/sysfs.c also SPI chip detection breaks because of commit 
e13ac47bec20 (spi: Use stable dev_name for ACPI enumerated SPI slaves).


However plain revert won't work as it breaks those ALSA SoC drivers that 
depend on this. Good thing is that matching there is purely kernel space 
and at quick look git grep 'i2c-.*:[0-9]' and git grep 
'spi-.*:[0-9]' don't find other in-kernel users.



70762ab achieved its stated goal in a meaningful way.  Won't
i2c-acpi_dev_name also vary with ACPI scan order, BIOS settings,
firmware upgrades, etc...?

If I remember correctly ACPI ID should not ever change and instance id 
:xy after INTABCD:xy should also be visible and keep the order even if 
device is disabled or not plugged. But I'm not absolute sure about this.


At least on a test platform that allow disable devices will show those 
devices off-line (/sys/bus/acpi/devices/INTABCD:xy/status == 0).


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

Re: [PATCH 2/2] i2c: designware: Remove adapter name from debug print in i2c_dw_isr()

[Jarkko Nikula]

On 07.08.2015 14:10, Wolfram Sang wrote:

On Fri, Aug 07, 2015 at 01:43:28PM +0300, Jarkko Nikula wrote:

Printing adapter name is irrelevant from this debug print and makes output
needlessly long. Having already device and functions names printed here is
enough for debugging.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com


So much about the double space :) I think I'll squash those two.

Fine to me. Actually long Synopsys DesignWare I2C adapter printed 
bothered me most.


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

[PATCH 1/2] i2c: designware: Remove space between variable and its value in debug print

[Jarkko Nikula]

Remove extra space from enabled= 0x to make debug print a bit more dense
and to be in sync with stat=0x. While at it, use %#x for printing 0x
prefixed hexadecimal values.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-designware-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 6f19a33773fe..7310051d4a50 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -777,7 +777,7 @@ irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 
enabled = dw_readl(dev, DW_IC_ENABLE);
stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
-   dev_dbg(dev-dev, %s:  %s enabled= 0x%x stat=0x%x\n, __func__,
+   dev_dbg(dev-dev, %s:  %s enabled=%#x stat=%#x\n, __func__,
dev-adapter.name, enabled, stat);
if (!enabled || !(stat  ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
-- 
2.4.6

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

[PATCH 2/2] i2c: designware: Remove adapter name from debug print in i2c_dw_isr()

[Jarkko Nikula]

Printing adapter name is irrelevant from this debug print and makes output
needlessly long. Having already device and functions names printed here is
enough for debugging.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-designware-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 7310051d4a50..cc98b2a75591 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -777,8 +777,8 @@ irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 
enabled = dw_readl(dev, DW_IC_ENABLE);
stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
-   dev_dbg(dev-dev, %s:  %s enabled=%#x stat=%#x\n, __func__,
-   dev-adapter.name, enabled, stat);
+   dev_dbg(dev-dev, %s: enabled=%#x stat=%#x\n, __func__,
+   enabled, stat);
if (!enabled || !(stat  ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
 
-- 
2.4.6

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

[PATCH] i2c: designware: Make debug print in i2c_dw_isr() shorter

[Jarkko Nikula]

Printing adapter name is irrelevant from this debug print and makes output
needlessly long. Having already device and functions names printed here is
enough for debugging.

While at it remove extra space from enabled= 0x and use %#x for
printing 0x prefixed hexadecimal values.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
This is my previous [PATCH 1/2] i2c: designware: Remove space between
variable and its value in debug print and [PATCH 2/2] i2c: designware:
Remove adapter name from debug print in i2c_dw_isr() squashed.
---
 drivers/i2c/busses/i2c-designware-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 6f19a33773fe..cc98b2a75591 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -777,8 +777,8 @@ irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 
enabled = dw_readl(dev, DW_IC_ENABLE);
stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
-   dev_dbg(dev-dev, %s:  %s enabled= 0x%x stat=0x%x\n, __func__,
-   dev-adapter.name, enabled, stat);
+   dev_dbg(dev-dev, %s: enabled=%#x stat=%#x\n, __func__,
+   enabled, stat);
if (!enabled || !(stat  ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
 
-- 
2.4.6

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

[PATCH 1/2] i2c: core: Remove needless structure member zero initialization

[Jarkko Nikula]

No need to clear one struct i2c_client member variable since memset has
already cleared all of them. Remove also one space intendation from err
label.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/i2c-core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 987c124432c5..051aa3a811a8 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -283,7 +283,6 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address 
command,
memset(client, 0, sizeof(client));
client.adapter = adapter;
client.addr = sb-slave_address;
-   client.flags = 0;
 
if (sb-access_mode == ACPI_I2C_10BIT_MODE)
client.flags |= I2C_CLIENT_TEN;
@@ -361,7 +360,7 @@ acpi_i2c_space_handler(u32 function, acpi_physical_address 
command,
 
gsb-status = status;
 
- err:
+err:
ACPI_FREE(ares);
return ret;
 }
-- 
2.1.4

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

Re: [PATCHv2 4/5] i2c: i801: Remove pci_enable_device() call from i801_resume()

[Jarkko Nikula]

Hi

On 02/13/2015 12:33 PM, Jean Delvare wrote:

Hi Jarkko,

On Wed, 11 Feb 2015 14:32:07 +0200, Jarkko Nikula wrote:

Since pci_disable_device() is not called from i801_suspend() and power
state is set already it means that subsequent pci_enable_device() calls do
practically nothing but monotonically increase struct pci_dev enable_cnt.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
  drivers/i2c/busses/i2c-i801.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b1d725d758bb..5fb35464f693 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1324,7 +1324,7 @@ static int i801_resume(struct pci_dev *dev)
  {
pci_set_power_state(dev, PCI_D0);
pci_restore_state(dev);
-   return pci_enable_device(dev);
+   return 0;
  }
  #else
  #define i801_suspend NULL


This looks reasonable but have you tested this change on a range of
actual laptops to make sure it has no unexpected side effect?

Unfortunately I have only limited amount of test hw which are working 
fine even if PCI device is disabled so I cannot hit those issues that 
were seen in the past.


I suppose this patch unlikely cause regression since if you look at the 
call chain pci_enable_device() - pci_enable_device_flags() it doesn't 
go beyond taking the current power state since enable_cnt is always 
greater than 1.


drivers/pci/pci.c: pci_enable_device_flags():

if (dev-pm_cap) {
u16 pmcsr;
pci_read_config_word(dev, dev-pm_cap + PCI_PM_CTRL, pmcsr);
dev-current_state = (pmcsr  PCI_PM_CTRL_STATE_MASK);
}

if (atomic_inc_return(dev-enable_cnt)  1)
return 0;   /* already enabled */

To me it seems current power state taking is practically no-op when 
device is enabled since pci_set_power_state() calls in i801_suspend() 
and i801_resume() have already cached it.


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

[PATCHv3 5/5] i2c: i801: Use managed pcim_* PCI device initialization and reservation

[Jarkko Nikula]

Simplifies the code a bit and makes easier to disable PCI device on driver
detach by removing the pcim_pin_device() call in the future if needed.

Reason why i2c-i801.c doesn't ever call pci_disable_device() was because it
made some systems to hang during power-off. See commit d6fcb3b9cf77
([PATCH] i2c-i801.c: don't pci_disable_device() after it was just enabled)
and
http://marc.info/?l=linux-kernelm=115160053309535w=2

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
Changes from v2:
- over 80 characters long pcim_iomap_regions line splitted
- gotos and error labels removed
---
 drivers/i2c/busses/i2c-i801.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5fb35464f693..5ecbb3fdc27e 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1180,35 +1180,35 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
}
priv-features = ~disable_features;
 
-   err = pci_enable_device(dev);
+   err = pcim_enable_device(dev);
if (err) {
dev_err(dev-dev, Failed to enable SMBus PCI device (%d)\n,
err);
-   goto exit;
+   return err;
}
+   pcim_pin_device(dev);
 
/* Determine the address of the SMBus area */
priv-smba = pci_resource_start(dev, SMBBAR);
if (!priv-smba) {
dev_err(dev-dev,
SMBus base address uninitialized, upgrade BIOS\n);
-   err = -ENODEV;
-   goto exit;
+   return -ENODEV;
}
 
err = acpi_check_resource_conflict(dev-resource[SMBBAR]);
if (err) {
-   err = -ENODEV;
-   goto exit;
+   return -ENODEV;
}
 
-   err = pci_request_region(dev, SMBBAR, dev_driver_string(dev-dev));
+   err = pcim_iomap_regions(dev, 1  SMBBAR,
+dev_driver_string(dev-dev));
if (err) {
dev_err(dev-dev,
Failed to request SMBus region 0x%lx-0x%Lx\n,
priv-smba,
(unsigned long long)pci_resource_end(dev, SMBBAR));
-   goto exit;
+   return err;
}
 
pci_read_config_byte(priv-pci_dev, SMBHSTCFG, temp);
@@ -1276,7 +1276,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
err = i2c_add_adapter(priv-adapter);
if (err) {
dev_err(dev-dev, Failed to add SMBus adapter\n);
-   goto exit_release;
+   return err;
}
 
i801_probe_optional_slaves(priv);
@@ -1286,11 +1286,6 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
pci_set_drvdata(dev, priv);
 
return 0;
-
-exit_release:
-   pci_release_region(dev, SMBBAR);
-exit:
-   return err;
 }
 
 static void i801_remove(struct pci_dev *dev)
@@ -1301,8 +1296,6 @@ static void i801_remove(struct pci_dev *dev)
i2c_del_adapter(priv-adapter);
pci_write_config_byte(dev, SMBHSTCFG, priv-original_hstcfg);
 
-   pci_release_region(dev, SMBBAR);
-
/*
 * do not call pci_disable_device(dev) since it can cause hard hangs on
 * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)
-- 
2.1.4

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

[PATCHv3 4/5] i2c: i801: Remove pci_enable_device() call from i801_resume()

[Jarkko Nikula]

Since pci_disable_device() is not called from i801_suspend() and power
state is set already it means that subsequent pci_enable_device() calls do
practically nothing but monotonically increase struct pci_dev enable_cnt.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-i801.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b1d725d758bb..5fb35464f693 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1324,7 +1324,7 @@ static int i801_resume(struct pci_dev *dev)
 {
pci_set_power_state(dev, PCI_D0);
pci_restore_state(dev);
-   return pci_enable_device(dev);
+   return 0;
 }
 #else
 #define i801_suspend NULL
-- 
2.1.4

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

[PATCHv3 3/5] i2c: i801: Use managed devm_* memory and irq allocation

[Jarkko Nikula]

This simplifies the error and remove paths.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
Reviewed-by: Jean Delvare jdelv...@suse.de
---
 drivers/i2c/busses/i2c-i801.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5f827dfc671a..b1d725d758bb 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1138,7 +1138,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
int err, i;
struct i801_priv *priv;
 
-   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+   priv = devm_kzalloc(dev-dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
 
@@ -1253,8 +1253,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
if (priv-features  FEATURE_IRQ) {
init_waitqueue_head(priv-waitq);
 
-   err = request_irq(dev-irq, i801_isr, IRQF_SHARED,
- dev_driver_string(dev-dev), priv);
+   err = devm_request_irq(dev-dev, dev-irq, i801_isr,
+  IRQF_SHARED,
+  dev_driver_string(dev-dev), priv);
if (err) {
dev_err(dev-dev, Failed to allocate irq %d: %d\n,
dev-irq, err);
@@ -1275,7 +1276,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
err = i2c_add_adapter(priv-adapter);
if (err) {
dev_err(dev-dev, Failed to add SMBus adapter\n);
-   goto exit_free_irq;
+   goto exit_release;
}
 
i801_probe_optional_slaves(priv);
@@ -1286,12 +1287,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
 
return 0;
 
-exit_free_irq:
-   if (priv-features  FEATURE_IRQ)
-   free_irq(dev-irq, priv);
+exit_release:
pci_release_region(dev, SMBBAR);
 exit:
-   kfree(priv);
return err;
 }
 
@@ -1303,11 +1301,8 @@ static void i801_remove(struct pci_dev *dev)
i2c_del_adapter(priv-adapter);
pci_write_config_byte(dev, SMBHSTCFG, priv-original_hstcfg);
 
-   if (priv-features  FEATURE_IRQ)
-   free_irq(dev-irq, priv);
pci_release_region(dev, SMBBAR);
 
-   kfree(priv);
/*
 * do not call pci_disable_device(dev) since it can cause hard hangs on
 * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)
-- 
2.1.4

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

[PATCHv3 1/5] i2c: i801: Don't break user-visible strings

[Jarkko Nikula]

It makes more difficult to grep these error prints from sources if they are
split to multiple source lines.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
Reviewed-by: Jean Delvare jdelv...@suse.de
---
 drivers/i2c/busses/i2c-i801.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 8fafb254e42a..7d1f4a478c54 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1192,8 +1192,8 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
/* Determine the address of the SMBus area */
priv-smba = pci_resource_start(dev, SMBBAR);
if (!priv-smba) {
-   dev_err(dev-dev, SMBus base address uninitialized, 
-   upgrade BIOS\n);
+   dev_err(dev-dev,
+   SMBus base address uninitialized, upgrade BIOS\n);
err = -ENODEV;
goto exit;
}
@@ -1206,8 +1206,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
 
err = pci_request_region(dev, SMBBAR, i801_driver.name);
if (err) {
-   dev_err(dev-dev, Failed to request SMBus region 
-   0x%lx-0x%Lx\n, priv-smba,
+   dev_err(dev-dev,
+   Failed to request SMBus region 0x%lx-0x%Lx\n,
+   priv-smba,
(unsigned long long)pci_resource_end(dev, SMBBAR));
goto exit;
}
-- 
2.1.4

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

[PATCHv3 2/5] i2c: i801: Remove i801_driver forward declaration

[Jarkko Nikula]

struct pci_driver i801_driver forward declaration is needed only for
accessing the name field. Remove it and use dev_driver_string() instead.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
Reviewed-by: Jean Delvare jdelv...@suse.de
---
 drivers/i2c/busses/i2c-i801.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 7d1f4a478c54..5f827dfc671a 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -223,8 +223,6 @@ struct i801_priv {
 #endif
 };
 
-static struct pci_driver i801_driver;
-
 #define FEATURE_SMBUS_PEC  (1  0)
 #define FEATURE_BLOCK_BUFFER   (1  1)
 #define FEATURE_BLOCK_PROC (1  2)
@@ -1204,7 +1202,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
goto exit;
}
 
-   err = pci_request_region(dev, SMBBAR, i801_driver.name);
+   err = pci_request_region(dev, SMBBAR, dev_driver_string(dev-dev));
if (err) {
dev_err(dev-dev,
Failed to request SMBus region 0x%lx-0x%Lx\n,
@@ -1256,7 +1254,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
init_waitqueue_head(priv-waitq);
 
err = request_irq(dev-irq, i801_isr, IRQF_SHARED,
- i801_driver.name, priv);
+ dev_driver_string(dev-dev), priv);
if (err) {
dev_err(dev-dev, Failed to allocate irq %d: %d\n,
dev-irq, err);
-- 
2.1.4

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

[PATCHv2 0/5] i2c: i801: Few cleanups

[Jarkko Nikula]

Hi Jean

Patches 2-3/5 are otherwise the same I sent last week but now on top
of 1/5.

Jarkko Nikula (5):
  i2c: i801: Don't break user-visible strings
  i2c: i801: Remove i801_driver forward declaration
  i2c: i801: Use managed devm_* memory and irq allocation
  i2c: i801: Remove pci_enable_device() call from i801_resume()
  i2c: i801: Use managed pcim_* PCI device initialization and
reservation

 drivers/i2c/busses/i2c-i801.c | 37 ++---
 1 file changed, 14 insertions(+), 23 deletions(-)

-- 
2.1.4

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

[PATCHv2 3/5] i2c: i801: Use managed devm_* memory and irq allocation

[Jarkko Nikula]

This simplifies the error and remove paths.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-i801.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5f827dfc671a..b1d725d758bb 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1138,7 +1138,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
int err, i;
struct i801_priv *priv;
 
-   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+   priv = devm_kzalloc(dev-dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
 
@@ -1253,8 +1253,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
if (priv-features  FEATURE_IRQ) {
init_waitqueue_head(priv-waitq);
 
-   err = request_irq(dev-irq, i801_isr, IRQF_SHARED,
- dev_driver_string(dev-dev), priv);
+   err = devm_request_irq(dev-dev, dev-irq, i801_isr,
+  IRQF_SHARED,
+  dev_driver_string(dev-dev), priv);
if (err) {
dev_err(dev-dev, Failed to allocate irq %d: %d\n,
dev-irq, err);
@@ -1275,7 +1276,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
err = i2c_add_adapter(priv-adapter);
if (err) {
dev_err(dev-dev, Failed to add SMBus adapter\n);
-   goto exit_free_irq;
+   goto exit_release;
}
 
i801_probe_optional_slaves(priv);
@@ -1286,12 +1287,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
 
return 0;
 
-exit_free_irq:
-   if (priv-features  FEATURE_IRQ)
-   free_irq(dev-irq, priv);
+exit_release:
pci_release_region(dev, SMBBAR);
 exit:
-   kfree(priv);
return err;
 }
 
@@ -1303,11 +1301,8 @@ static void i801_remove(struct pci_dev *dev)
i2c_del_adapter(priv-adapter);
pci_write_config_byte(dev, SMBHSTCFG, priv-original_hstcfg);
 
-   if (priv-features  FEATURE_IRQ)
-   free_irq(dev-irq, priv);
pci_release_region(dev, SMBBAR);
 
-   kfree(priv);
/*
 * do not call pci_disable_device(dev) since it can cause hard hangs on
 * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)
-- 
2.1.4

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

[PATCHv2 4/5] i2c: i801: Remove pci_enable_device() call from i801_resume()

[Jarkko Nikula]

Since pci_disable_device() is not called from i801_suspend() and power
state is set already it means that subsequent pci_enable_device() calls do
practically nothing but monotonically increase struct pci_dev enable_cnt.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-i801.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index b1d725d758bb..5fb35464f693 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1324,7 +1324,7 @@ static int i801_resume(struct pci_dev *dev)
 {
pci_set_power_state(dev, PCI_D0);
pci_restore_state(dev);
-   return pci_enable_device(dev);
+   return 0;
 }
 #else
 #define i801_suspend NULL
-- 
2.1.4

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

[PATCHv2 1/5] i2c: i801: Don't break user-visible strings

[Jarkko Nikula]

It makes more difficult to grep these error prints from sources if they are
split to multiple source lines.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-i801.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 8fafb254e42a..7d1f4a478c54 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1192,8 +1192,8 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
/* Determine the address of the SMBus area */
priv-smba = pci_resource_start(dev, SMBBAR);
if (!priv-smba) {
-   dev_err(dev-dev, SMBus base address uninitialized, 
-   upgrade BIOS\n);
+   dev_err(dev-dev,
+   SMBus base address uninitialized, upgrade BIOS\n);
err = -ENODEV;
goto exit;
}
@@ -1206,8 +1206,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
 
err = pci_request_region(dev, SMBBAR, i801_driver.name);
if (err) {
-   dev_err(dev-dev, Failed to request SMBus region 
-   0x%lx-0x%Lx\n, priv-smba,
+   dev_err(dev-dev,
+   Failed to request SMBus region 0x%lx-0x%Lx\n,
+   priv-smba,
(unsigned long long)pci_resource_end(dev, SMBBAR));
goto exit;
}
-- 
2.1.4

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

[PATCHv2 2/5] i2c: i801: Remove i801_driver forward declaration

[Jarkko Nikula]

struct pci_driver i801_driver forward declaration is needed only for
accessing the name field. Remove it and use dev_driver_string() instead.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
Reviewed-by: Jean Delvare jdelv...@suse.de
---
 drivers/i2c/busses/i2c-i801.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 7d1f4a478c54..5f827dfc671a 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -223,8 +223,6 @@ struct i801_priv {
 #endif
 };
 
-static struct pci_driver i801_driver;
-
 #define FEATURE_SMBUS_PEC  (1  0)
 #define FEATURE_BLOCK_BUFFER   (1  1)
 #define FEATURE_BLOCK_PROC (1  2)
@@ -1204,7 +1202,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
goto exit;
}
 
-   err = pci_request_region(dev, SMBBAR, i801_driver.name);
+   err = pci_request_region(dev, SMBBAR, dev_driver_string(dev-dev));
if (err) {
dev_err(dev-dev,
Failed to request SMBus region 0x%lx-0x%Lx\n,
@@ -1256,7 +1254,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
init_waitqueue_head(priv-waitq);
 
err = request_irq(dev-irq, i801_isr, IRQF_SHARED,
- i801_driver.name, priv);
+ dev_driver_string(dev-dev), priv);
if (err) {
dev_err(dev-dev, Failed to allocate irq %d: %d\n,
dev-irq, err);
-- 
2.1.4

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

[PATCHv2 5/5] i2c: i801: Use managed pcim_* PCI device initialization and reservation

[Jarkko Nikula]

Simplifies the code a bit and makes easier to disable PCI device on driver
detach by removing the pcim_pin_device() call in the future if needed.

Reason why i2c-i801.c doesn't ever call pci_disable_device() was because it
made some systems to hang during power-off. See commit d6fcb3b9cf77
([PATCH] i2c-i801.c: don't pci_disable_device() after it was just enabled)
and
http://marc.info/?l=linux-kernelm=115160053309535w=2

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-i801.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 5fb35464f693..9f7b69743233 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1180,12 +1180,13 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
}
priv-features = ~disable_features;
 
-   err = pci_enable_device(dev);
+   err = pcim_enable_device(dev);
if (err) {
dev_err(dev-dev, Failed to enable SMBus PCI device (%d)\n,
err);
goto exit;
}
+   pcim_pin_device(dev);
 
/* Determine the address of the SMBus area */
priv-smba = pci_resource_start(dev, SMBBAR);
@@ -1202,7 +1203,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
goto exit;
}
 
-   err = pci_request_region(dev, SMBBAR, dev_driver_string(dev-dev));
+   err = pcim_iomap_regions(dev, 1  SMBBAR, 
dev_driver_string(dev-dev));
if (err) {
dev_err(dev-dev,
Failed to request SMBus region 0x%lx-0x%Lx\n,
@@ -1276,7 +1277,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
err = i2c_add_adapter(priv-adapter);
if (err) {
dev_err(dev-dev, Failed to add SMBus adapter\n);
-   goto exit_release;
+   goto exit;
}
 
i801_probe_optional_slaves(priv);
@@ -1287,8 +1288,6 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
 
return 0;
 
-exit_release:
-   pci_release_region(dev, SMBBAR);
 exit:
return err;
 }
@@ -1301,8 +1300,6 @@ static void i801_remove(struct pci_dev *dev)
i2c_del_adapter(priv-adapter);
pci_write_config_byte(dev, SMBHSTCFG, priv-original_hstcfg);
 
-   pci_release_region(dev, SMBBAR);
-
/*
 * do not call pci_disable_device(dev) since it can cause hard hangs on
 * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)
-- 
2.1.4

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

Re: [PATCH v2] i2c: designware: fixup return handling of wait_for_completion_timeout

[Jarkko Nikula]

On 02/10/2015 10:11 AM, Nicholas Mc Guire wrote:

return type of wait_for_completion_timeout is unsigned long not int, rather
than introducing a new variable the wait_for_completion_timeout is moved
into the if condition as the return value is only used to detect timeout.

Signed-off-by: Nicholas Mc Guire hof...@osadl.org
---

v2: Aside from the newly added variable having the wrong type (...) the
 preferred solution is to simply wrap wait_for_completion_timeout into
 the condition as the remaining jiffies is actually not used - suggested
 be Jarkko Nikula jarkko.nik...@linux.intel.com.

Patch was compile tested with x86_64_defconfig +
CONFIG_X86_AMD_PLATFORM_DEVICE=y, CONFIG_I2C_DESIGNWARE_PLATFORM=m
(implies CONFIG_I2C_DESIGNWARE_CORE=y)

Patch is against 3.19.0-rc7 (localversion-next is -next-20150209)

  drivers/i2c/busses/i2c-designware-core.c |3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 6e25c01..6f19a33 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -656,8 +656,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
i2c_dw_xfer_init(dev);

/* wait for tx to complete */
-   ret = wait_for_completion_timeout(dev-cmd_complete, HZ);
-   if (ret == 0) {
+   if (!wait_for_completion_timeout(dev-cmd_complete, HZ)) {
dev_err(dev-dev, controller timed out\n);
/* i2c_dw_init implicitly disables the adapter */
i2c_dw_init(dev);


Reviewed-by: Jarkko Nikula jarkko.nik...@linux.intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] i2c-designware: fixup of wait_for_completion_timeout return handling

[Jarkko Nikula]

Hi

On 02/09/2015 06:35 PM, Nicholas Mc Guire wrote:

return type of wait_for_completion_timeout is unsigned long not int.
An appropriate return variable is introduced and assignment fixed up.

Signed-off-by: Nicholas Mc Guire hof...@osadl.org
---

Patch was compile tested with x86_64_defconfig +
CONFIG_X86_AMD_PLATFORM_DEVICE=y, CONFIG_I2C_DESIGNWARE_PLATFORM=m
(implies CONFIG_I2C_DESIGNWARE_CORE=y)

Patch is against 3.19.0-rc7 (localversion-next is -next-20150209)

  drivers/i2c/busses/i2c-designware-core.c |5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 6e25c01..05934e0 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -623,6 +623,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
  {
struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
int ret;
+   unsigned int timeout;


Not unsigned long.


@@ -656,8 +657,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
i2c_dw_xfer_init(dev);

/* wait for tx to complete */
-   ret = wait_for_completion_timeout(dev-cmd_complete, HZ);
-   if (ret == 0) {
+   timeout = wait_for_completion_timeout(dev-cmd_complete, HZ);
+   if (timeout == 0) {
dev_err(dev-dev, controller timed out\n);
/* i2c_dw_init implicitly disables the adapter */
i2c_dw_init(dev);

I'd say better to test directly with if 
(!wait_for_completion_timeout()) since remaining jiffies or potential 
error code from wait_for_completion_timeout() is not used here.


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

[PATCH 1/2] i2c: i801: Remove i801_driver forward declaration

[Jarkko Nikula]

struct pci_driver i801_driver forward declaration is needed only for
accessing the name field. Remove it and use dev_driver_string() instead.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-i801.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 8fafb254e42a..e31aa987b281 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -223,8 +223,6 @@ struct i801_priv {
 #endif
 };
 
-static struct pci_driver i801_driver;
-
 #define FEATURE_SMBUS_PEC  (1  0)
 #define FEATURE_BLOCK_BUFFER   (1  1)
 #define FEATURE_BLOCK_PROC (1  2)
@@ -1204,7 +1202,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
goto exit;
}
 
-   err = pci_request_region(dev, SMBBAR, i801_driver.name);
+   err = pci_request_region(dev, SMBBAR, dev_driver_string(dev-dev));
if (err) {
dev_err(dev-dev, Failed to request SMBus region 
0x%lx-0x%Lx\n, priv-smba,
@@ -1255,7 +1253,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
init_waitqueue_head(priv-waitq);
 
err = request_irq(dev-irq, i801_isr, IRQF_SHARED,
- i801_driver.name, priv);
+ dev_driver_string(dev-dev), priv);
if (err) {
dev_err(dev-dev, Failed to allocate irq %d: %d\n,
dev-irq, err);
-- 
2.1.4

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

[PATCH 2/2] i2c: i801: Use managed devm_* memory and irq allocation

[Jarkko Nikula]

This simplifies the error and remove paths.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-i801.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index e31aa987b281..b4369d1835ca 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -1138,7 +1138,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
int err, i;
struct i801_priv *priv;
 
-   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+   priv = devm_kzalloc(dev-dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
 
@@ -1252,8 +1252,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
if (priv-features  FEATURE_IRQ) {
init_waitqueue_head(priv-waitq);
 
-   err = request_irq(dev-irq, i801_isr, IRQF_SHARED,
- dev_driver_string(dev-dev), priv);
+   err = devm_request_irq(dev-dev, dev-irq, i801_isr,
+  IRQF_SHARED,
+  dev_driver_string(dev-dev), priv);
if (err) {
dev_err(dev-dev, Failed to allocate irq %d: %d\n,
dev-irq, err);
@@ -1274,7 +1275,7 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
err = i2c_add_adapter(priv-adapter);
if (err) {
dev_err(dev-dev, Failed to add SMBus adapter\n);
-   goto exit_free_irq;
+   goto exit_release;
}
 
i801_probe_optional_slaves(priv);
@@ -1285,12 +1286,9 @@ static int i801_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
 
return 0;
 
-exit_free_irq:
-   if (priv-features  FEATURE_IRQ)
-   free_irq(dev-irq, priv);
+exit_release:
pci_release_region(dev, SMBBAR);
 exit:
-   kfree(priv);
return err;
 }
 
@@ -1302,11 +1300,8 @@ static void i801_remove(struct pci_dev *dev)
i2c_del_adapter(priv-adapter);
pci_write_config_byte(dev, SMBHSTCFG, priv-original_hstcfg);
 
-   if (priv-features  FEATURE_IRQ)
-   free_irq(dev-irq, priv);
pci_release_region(dev, SMBBAR);
 
-   kfree(priv);
/*
 * do not call pci_disable_device(dev) since it can cause hard hangs on
 * some systems during power-off (eg. Fujitsu-Siemens Lifebook E8010)
-- 
2.1.4

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

Re: [PATCH 2/2] i2c: i801: Use managed devm_* memory and irq allocation

[Jarkko Nikula]

On 02/06/2015 04:51 PM, Jean Delvare wrote:

Hi Jarkko,

On Fri,  6 Feb 2015 15:56:53 +0200, Jarkko Nikula wrote:

This simplifies the error and remove paths.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---

Looks good, but what about also converting pci_request_region to
devm_request_region?

Yep, in progress but won't finish today so wanted to send these two 
first :-)


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

[PATCH] i2c: designware: Do not calculate SCL timing parameters needlessly

[Jarkko Nikula]

Do SCL timing parameter calculation conditionally depending are custom
parameters provided since calculated values will get instantly overwritten
by provided parameters.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/busses/i2c-designware-core.c | 45 
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c 
b/drivers/i2c/busses/i2c-designware-core.c
index 23628b7bfb8d..2c7749774a5f 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -309,40 +309,39 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
sda_falling_time = dev-sda_falling_time ?: 300; /* ns */
scl_falling_time = dev-scl_falling_time ?: 300; /* ns */
 
-   /* Standard-mode */
-   hcnt = i2c_dw_scl_hcnt(input_clock_khz,
-   4000,   /* tHD;STA = tHIGH = 4.0 us */
-   sda_falling_time,
-   0,  /* 0: DW default, 1: Ideal */
-   0); /* No offset */
-   lcnt = i2c_dw_scl_lcnt(input_clock_khz,
-   4700,   /* tLOW = 4.7 us */
-   scl_falling_time,
-   0); /* No offset */
-
-   /* Allow platforms to specify the ideal HCNT and LCNT values */
+   /* Set SCL timing parameters for standard-mode */
if (dev-ss_hcnt  dev-ss_lcnt) {
hcnt = dev-ss_hcnt;
lcnt = dev-ss_lcnt;
+   } else {
+   hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+   4000,   /* tHD;STA = tHIGH = 4.0 us */
+   sda_falling_time,
+   0,  /* 0: DW default, 1: Ideal */
+   0); /* No offset */
+   lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+   4700,   /* tLOW = 4.7 us */
+   scl_falling_time,
+   0); /* No offset */
}
dw_writel(dev, hcnt, DW_IC_SS_SCL_HCNT);
dw_writel(dev, lcnt, DW_IC_SS_SCL_LCNT);
dev_dbg(dev-dev, Standard-mode HCNT:LCNT = %d:%d\n, hcnt, lcnt);
 
-   /* Fast-mode */
-   hcnt = i2c_dw_scl_hcnt(input_clock_khz,
-   600,/* tHD;STA = tHIGH = 0.6 us */
-   sda_falling_time,
-   0,  /* 0: DW default, 1: Ideal */
-   0); /* No offset */
-   lcnt = i2c_dw_scl_lcnt(input_clock_khz,
-   1300,   /* tLOW = 1.3 us */
-   scl_falling_time,
-   0); /* No offset */
-
+   /* Set SCL timing parameters for fast-mode */
if (dev-fs_hcnt  dev-fs_lcnt) {
hcnt = dev-fs_hcnt;
lcnt = dev-fs_lcnt;
+   } else {
+   hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+   600,/* tHD;STA = tHIGH = 0.6 us */
+   sda_falling_time,
+   0,  /* 0: DW default, 1: Ideal */
+   0); /* No offset */
+   lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+   1300,   /* tLOW = 1.3 us */
+   scl_falling_time,
+   0); /* No offset */
}
dw_writel(dev, hcnt, DW_IC_FS_SCL_HCNT);
dw_writel(dev, lcnt, DW_IC_FS_SCL_LCNT);
-- 
2.1.4

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

Re: [PATCH v1 3/3] i2c: designware-pci: no need to provide clk_khz

[Jarkko Nikula]

On 01/22/2015 04:44 PM, Andy Shevchenko wrote:

The clk_khz field makes sense only if SS counters are not provided. Since we
provide them for Haswell and Baytrail explicitly we can omit the clk_khz
parameter.

Signed-off-by: Andy Shevchenko andriy.shevche...@linux.intel.com
---
  drivers/i2c/busses/i2c-designware-pcidrv.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c 
b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 2bbd2a7..506cbe8 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -112,7 +112,6 @@ static struct dw_pci_controller dw_pci_controllers[] = {
.bus_cfg = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
.tx_fifo_depth = 32,
.rx_fifo_depth = 32,
-   .clk_khz = 10,
.functionality = I2C_FUNC_10BIT_ADDR,
.scl_sda_cfg = byt_config,
},
@@ -121,7 +120,6 @@ static struct dw_pci_controller dw_pci_controllers[] = {
.bus_cfg = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
.tx_fifo_depth = 32,
.rx_fifo_depth = 32,
-   .clk_khz = 10,
.functionality = I2C_FUNC_10BIT_ADDR,
.scl_sda_cfg = hsw_config,
},

Yes, there are no other use for the clock rate than calculate these SS 
counters in drivers/i2c/busses/i2c-designware-core.c, and which will get 
overwritten by provided values.


Reviewed-by: Jarkko Nikula jarkko.nik...@linux.intel.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 1/2] i2c-designware: Add i2c bus locking support

[Jarkko Nikula]

Hi

On 12/02/2014 02:09 AM, David E. Box wrote:

Adds support for acquiring and releasing a hardware bus lock in the i2c
designware core transfer function. This is needed for i2c bus controllers
that are shared with but not controlled by the kernel.

Signed-off-by: David E. Box david.e@linux.intel.com
---
  drivers/i2c/busses/i2c-designware-core.c| 11 +++
  drivers/i2c/busses/i2c-designware-core.h|  6 ++
  drivers/i2c/busses/i2c-designware-platdrv.c | 18 +-
  3 files changed, 30 insertions(+), 5 deletions(-)


...

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c 
b/drivers/i2c/busses/i2c-designware-platdrv.c
index a743115..afdff3b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -261,10 +261,16 @@ static int dw_i2c_probe(struct platform_device *pdev)
return r;
}
  
-	pm_runtime_set_autosuspend_delay(pdev-dev, 1000);

-   pm_runtime_use_autosuspend(pdev-dev);
-   pm_runtime_set_active(pdev-dev);
-   pm_runtime_enable(pdev-dev);
+   i2c_dw_eval_lock_support(dev);

i2c_dw_eval_lock_support() is added in the next patch.

+
+   if (dev-pm_runtime_disabled) {
+   pm_runtime_forbid(pdev-dev);
+   } else {
+   pm_runtime_set_autosuspend_delay(pdev-dev, 1000);
+   pm_runtime_use_autosuspend(pdev-dev);
+   pm_runtime_set_active(pdev-dev);
+   pm_runtime_enable(pdev-dev);
+   }
  
  	return 0;

  }
@@ -314,7 +320,9 @@ static int dw_i2c_resume(struct device *dev)
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
  
  	clk_prepare_enable(i_dev-clk);

-   i2c_dw_init(i_dev);
+
+   if (!i_dev-pm_runtime_disabled)
+   i2c_dw_init(i_dev);
  
Should there be similar conditional call or locking around i2c_dw_init() 
and i2c_dw_disable_int() also in dw_i2c_probe()?


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

Re: [PATCH 3/4] fix module autoloading for ACPI enumerated devices

[Jarkko Nikula]

On 01/17/2014 05:57 PM, Mark Brown wrote:

On Fri, Jan 17, 2014 at 09:37:56AM +0200, Jarkko Nikula wrote:

Sidenote: actually this modalias/module loading issue is different
and not related to stable ACPI i2c/spi slave device names.

Oh, I'd been under the impression that it was the rewrite that was
triggering this?

IIRC issue has been there since when ACPI slave device support was added.

I have a partial fix for it in cf9eb39c6f7b (spi: Fix modalias for ACPI 
enumerated SPI devices) and when doing similar change for i2c Rui 
pointed me that he has a better fix that takes care of _CID string and 
platform code too.


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

Re: [PATCH 3/4] fix module autoloading for ACPI enumerated devices

[Jarkko Nikula]

On 01/16/2014 09:46 PM, Mark Brown wrote:

On Thu, Jan 16, 2014 at 09:05:09PM +0800, Zhang Rui wrote:

On Thu, 2014-01-16 at 13:27 +0100, Wolfram Sang wrote:
This seems a gap to me but I'm not 100% sure.
I saw Grant Likely introduced the OF style MODALIAS to platform bus, and
OF style registration/binding to i2c bus, maybe he has an answer for
this.

This is needed for ACPI because we rewrite the device names to give us
stable names.  With OF for I2C and SPI nothing bus specific is done that
affects this stuff so the default behaviour works.
Sidenote: actually this modalias/module loading issue is different and 
not related to stable ACPI i2c/spi slave device names.


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

[PATCHv3 3/3] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Jarkko Nikula]

Current spi bus_num.chip_select spix.y based device naming scheme may not
be stable enough to be used in name based matching, for instance within
ALSA SoC subsystem.

This can be problem in PC kind of platforms if there are changes in SPI bus
configuration, amount of busses or probe order.

This patch addresses the problem by using the ACPI device name with
spi- prefix for ACPI enumerated SPI slave. For them device name
spix.y becomes spi-INTABCD:ij.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/spi/spi.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index af4f971..d9140bb 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -352,6 +352,19 @@ struct spi_device *spi_alloc_device(struct spi_master 
*master)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_device);
 
+static void spi_dev_set_name(struct spi_device *spi)
+{
+   struct acpi_device *adev = ACPI_COMPANION(spi-dev);
+
+   if (adev) {
+   dev_set_name(spi-dev, spi-%s, dev_name(adev-dev));
+   return;
+   }
+
+   dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
+spi-chip_select);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -378,9 +391,7 @@ int spi_add_device(struct spi_device *spi)
}
 
/* Set the bus ID string */
-   dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
-   spi-chip_select);
-
+   spi_dev_set_name(spi);
 
/* We need to make sure there's no other device with this
 * chipselect **BEFORE** we call setup(), else we'll trash
-- 
1.8.4.2

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

[PATCHv3 1/3] ACPI: Provide struct acpi_device stub for !CONFIG_ACPI builds

[Jarkko Nikula]

We have a few cases where we want to access struct device dev field in
struct acpi_device from generic code that is build also without CONFIG_ACPI.

Provide here a minimal struct acpi_device stub that allows to build such a
code without adding new #ifdef CONFIG_ACPI churn. This should not increase
section sizes if code is protected by ACPI_COMPANION() runtime checks as
those will be optimized out by later compiler stages in case CONFIG_ACPI is
not set.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
Rafael, instead of this we could also have an accessor but adev-dev looked
better in actual code and size vmlinux didn't change for x86_64_defconfig
with CONFIG_ACPI is not set nor for omap2plus_defconfig.
---
 include/linux/acpi.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0da49ed..df444e1 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -414,6 +414,10 @@ static inline bool acpi_driver_match_device(struct device 
*dev,
 
 #else  /* !CONFIG_ACPI */
 
+struct acpi_device {
+   struct device dev;
+};
+
 #define acpi_disabled 1
 
 #define ACPI_COMPANION(dev)(NULL)
-- 
1.8.4.2

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

[PATCHv3 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

Hi

Third version of the set that changes I2C and SPI slave device names
to be generated from stable ACPI device names on ACPI 5 based systems
instead of using bus numbers which could change.

Slave device name change goes as

x-00yz - i2c-INTABCD:ij
spix.y - spi-INTABCD:ij

This version is actually a bit cleaner thanks to Rafael's
ACPI / driver core: Store an ACPI device pointer in struct acpi_dev_node
in git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm

Set goes on top of linux-pm/linux-next due dependency to 1/3. As we are now
in merge-window I'm fine if 1/3 goes in first and I queue 2-3/3 later
to i2c and spi trees.

Jarkko Nikula (3):
  ACPI: Provide struct acpi_device stub for !CONFIG_ACPI builds
  i2c: Use stable dev_name for ACPI enumerated I2C slaves
  spi: Use stable dev_name for ACPI enumerated SPI slaves

 drivers/i2c/i2c-core.c | 21 +
 drivers/spi/spi.c  | 17 ++---
 include/linux/acpi.h   |  4 
 3 files changed, 35 insertions(+), 7 deletions(-)

-- 
1.8.4.2

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

[PATCHv3 2/3] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Current I2C adapter id - client address x-00yy based device naming scheme
is not always stable enough to be used in name based matching, for instance
within ALSA SoC subsystem.

This is problematic in PC kind of platforms where I2C adapter numbers can
change due variable amount of bus controllers, probe order, add-on cards or
just because of BIOS settings.

This patch addresses the problem by using the ACPI device name with
i2c- prefix for ACPI enumerated I2C slaves. For them device name
x-00yz becomes i2c-INTABCD:ij after this patch.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/i2c-core.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index f74af33..b8f2c32 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -618,6 +618,22 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
+static void i2c_dev_set_name(struct i2c_adapter *adap,
+struct i2c_client *client)
+{
+   struct acpi_device *adev = ACPI_COMPANION(client-dev);
+
+   if (adev) {
+   dev_set_name(client-dev, i2c-%s, dev_name(adev-dev));
+   return;
+   }
+
+   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
+   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
+client-addr | ((client-flags  I2C_CLIENT_TEN)
+? 0xa000 : 0));
+}
+
 /**
  * i2c_new_device - instantiate an i2c device
  * @adap: the adapter managing the device
@@ -676,10 +692,7 @@ i2c_new_device(struct i2c_adapter *adap, struct 
i2c_board_info const *info)
client-dev.of_node = info-of_node;
ACPI_COMPANION_SET(client-dev, info-acpi_node.companion);
 
-   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
-   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
-client-addr | ((client-flags  I2C_CLIENT_TEN)
-? 0xa000 : 0));
+   i2c_dev_set_name(adap, client);
status = device_register(client-dev);
if (status)
goto out_err;
-- 
1.8.4.2

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

Re: [alsa-devel] [PATCHv3 1/3] ACPI: Provide struct acpi_device stub for !CONFIG_ACPI builds

[Jarkko Nikula]

On 11/14/2013 12:03 PM, Takashi Iwai wrote:

At Thu, 14 Nov 2013 11:00:59 +0200,
Jarkko Nikula wrote:

We have a few cases where we want to access struct device dev field in
struct acpi_device from generic code that is build also without CONFIG_ACPI.

Provide here a minimal struct acpi_device stub that allows to build such a
code without adding new #ifdef CONFIG_ACPI churn. This should not increase
section sizes if code is protected by ACPI_COMPANION() runtime checks as
those will be optimized out by later compiler stages in case CONFIG_ACPI is
not set.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
Rafael, instead of this we could also have an accessor but adev-dev looked
better in actual code and size vmlinux didn't change for x86_64_defconfig
with CONFIG_ACPI is not set nor for omap2plus_defconfig.

This looks too hackish, IMO.  Defining a different dummy struct often
gives more headaches.  Thinking of the potential risk by misuses, it'd
be simpler and safer to define a macro like acpi_dev_name() instead.

BTW, is linux/device.h already included in !CONFIG_ACPI case?


It is included unconditionally in include/linux/acpi.h.

Your acpi_dev_name() proposal may be good enough for now as adev-dev 
access in generic code (now under CONFIG_ACPI) seems to be anyway around 
dev_name, using only pointer to struct acpi_device or has more things 
that prevents immediate #ifdef CONFIG_ACPI removal there.


But one problem at time. I'll cook a version with acpi_dev_name.

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

[PATCHv4 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

v4:
This version adds and uses acpi_dev_name() accessor instead defining
struct acpi_device stub for !CONFIG_ACPI 

v3:
Third version of the set that changes I2C and SPI slave device names
to be generated from stable ACPI device names on ACPI 5 based systems
instead of using bus numbers which could change.

Slave device name change goes as

x-00yz - i2c-INTABCD:ij
spix.y - spi-INTABCD:ij

This version is actually a bit cleaner thanks to Rafael's
ACPI / driver core: Store an ACPI device pointer in struct acpi_dev_node
in git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm

Set goes on top of linux-pm/linux-next due dependency to 1/3. As we are now
in merge-window I'm fine if 1/3 goes in first and I queue 2-3/3 later
to i2c and spi trees.

Jarkko Nikula (3):
  ACPI: Provide acpi_dev_name accessor for struct acpi_device device
name
  i2c: Use stable dev_name for ACPI enumerated I2C slaves
  spi: Use stable dev_name for ACPI enumerated SPI slaves

 drivers/i2c/i2c-core.c | 21 +
 drivers/spi/spi.c  | 17 ++---
 include/linux/acpi.h   | 10 ++
 3 files changed, 41 insertions(+), 7 deletions(-)

-- 
1.8.4.2

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

[PATCHv4 2/3] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Current I2C adapter id - client address x-00yy based device naming scheme
is not always stable enough to be used in name based matching, for instance
within ALSA SoC subsystem.

This is problematic in PC kind of platforms where I2C adapter numbers can
change due variable amount of bus controllers, probe order, add-on cards or
just because of BIOS settings.

This patch addresses the problem by using the ACPI device name with
i2c- prefix for ACPI enumerated I2C slaves. For them device name
x-00yz becomes i2c-INTABCD:ij after this patch.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/i2c-core.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index f74af33..bc9a294 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -618,6 +618,22 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
+static void i2c_dev_set_name(struct i2c_adapter *adap,
+struct i2c_client *client)
+{
+   struct acpi_device *adev = ACPI_COMPANION(client-dev);
+
+   if (adev) {
+   dev_set_name(client-dev, i2c-%s, acpi_dev_name(adev));
+   return;
+   }
+
+   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
+   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
+client-addr | ((client-flags  I2C_CLIENT_TEN)
+? 0xa000 : 0));
+}
+
 /**
  * i2c_new_device - instantiate an i2c device
  * @adap: the adapter managing the device
@@ -676,10 +692,7 @@ i2c_new_device(struct i2c_adapter *adap, struct 
i2c_board_info const *info)
client-dev.of_node = info-of_node;
ACPI_COMPANION_SET(client-dev, info-acpi_node.companion);
 
-   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
-   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
-client-addr | ((client-flags  I2C_CLIENT_TEN)
-? 0xa000 : 0));
+   i2c_dev_set_name(adap, client);
status = device_register(client-dev);
if (status)
goto out_err;
-- 
1.8.4.2

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

[PATCHv4 1/3] ACPI: Provide acpi_dev_name accessor for struct acpi_device device name

[Jarkko Nikula]

struct acpi_device fields are only available when CONFIG_ACPI is set. We may
find use for dev_name(adev-dev) in generic code that is build also without
CONFIG_ACPI is set but currently this requires #ifdef CONFIG_ACPI churn.

Provide here an accessor that returns dev_name of embedded struct device dev
in struct acpi_device or NULL depending on CONFIG_ACPI setting.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 include/linux/acpi.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0da49ed..844e7ff 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -53,6 +53,11 @@ static inline acpi_handle acpi_device_handle(struct 
acpi_device *adev)
 #define ACPI_COMPANION_SET(dev, adev)  ACPI_COMPANION(dev) = (adev)
 #define ACPI_HANDLE(dev)   acpi_device_handle(ACPI_COMPANION(dev))
 
+static inline const char *acpi_dev_name(struct acpi_device *adev)
+{
+   return dev_name(adev-dev);
+}
+
 enum acpi_irq_model_id {
ACPI_IRQ_MODEL_PIC = 0,
ACPI_IRQ_MODEL_IOAPIC,
@@ -420,6 +425,11 @@ static inline bool acpi_driver_match_device(struct device 
*dev,
 #define ACPI_COMPANION_SET(dev, adev)  do { } while (0)
 #define ACPI_HANDLE(dev)   (NULL)
 
+static inline const char *acpi_dev_name(struct acpi_device *adev)
+{
+   return NULL;
+}
+
 static inline void acpi_early_init(void) { }
 
 static inline int early_acpi_boot_init(void)
-- 
1.8.4.2

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

[PATCHv4 3/3] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Jarkko Nikula]

Current spi bus_num.chip_select spix.y based device naming scheme may not
be stable enough to be used in name based matching, for instance within
ALSA SoC subsystem.

This can be problem in PC kind of platforms if there are changes in SPI bus
configuration, amount of busses or probe order.

This patch addresses the problem by using the ACPI device name with
spi- prefix for ACPI enumerated SPI slave. For them device name
spix.y becomes spi-INTABCD:ij.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
Acked-by: Mark Brown broo...@linaro.org
---
Mark's ack added as diff between v3 and v4 is just:
-   dev_set_name(spi-dev, spi-%s, dev_name(adev-dev));
+   dev_set_name(spi-dev, spi-%s, acpi_dev_name(adev));
---
 drivers/spi/spi.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index af4f971..be619e0 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -352,6 +352,19 @@ struct spi_device *spi_alloc_device(struct spi_master 
*master)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_device);
 
+static void spi_dev_set_name(struct spi_device *spi)
+{
+   struct acpi_device *adev = ACPI_COMPANION(spi-dev);
+
+   if (adev) {
+   dev_set_name(spi-dev, spi-%s, acpi_dev_name(adev));
+   return;
+   }
+
+   dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
+spi-chip_select);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -378,9 +391,7 @@ int spi_add_device(struct spi_device *spi)
}
 
/* Set the bus ID string */
-   dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
-   spi-chip_select);
-
+   spi_dev_set_name(spi);
 
/* We need to make sure there's no other device with this
 * chipselect **BEFORE** we call setup(), else we'll trash
-- 
1.8.4.2

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

Re: [PATCHv2 1/3] ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI builds

[Jarkko Nikula]

On Mon, Nov 04, 2013 at 02:00:45AM +0100, Rafael J. Wysocki wrote:
 On Saturday, November 02, 2013 11:18:31 PM Rafael J. Wysocki wrote:
  This turns out the cause build problems to happen on some architectures.
  
  I guess it's sufficient to simply define a stub struct acpi_device as
  
  struct acpi_device {
  struct device dev;
  };
  
  for !CONFIG_ACPI instead.
 
 Generally, it is a bad idea to #include acpi_bus.h for !CONFIG_ACPI.
 
Ouch, indeed.

 The appended patch works for me, sorry for stealing part of your changelog.
 
No problem. Thanks for fixing this up.

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

[PATCHv2 1/3] ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI builds

[Jarkko Nikula]

We may find use for struct acpi_device and acpi_bus_get_device() in generic
code without wanting to add #if IS_ENABLED(CONFIG_ACPI) churn there.

Code can use ACPI_HANDLE() test to let compiler optimize out code blocks
that are not used in !CONFIG_ACPI builds but structure definitions and
function stubs must be available.

This patch moves CONFIG_ACPI test so that struct acpi_device definition
is available for all builds that include acpi_bus.h and provides a stub for
acpi_bus_get_device().

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 include/acpi/acpi_bus.h | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 15100f6..232b37d3 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
 bool acpi_bay_match(acpi_handle handle);
 bool acpi_dock_match(acpi_handle handle);
 
-#ifdef CONFIG_ACPI
-
 #include linux/proc_fs.h
 
 #define ACPI_BUS_FILE_ROOT acpi
@@ -315,6 +313,8 @@ struct acpi_device {
void (*remove)(struct acpi_device *);
 };
 
+#if IS_ENABLED(CONFIG_ACPI)
+
 static inline void *acpi_driver_data(struct acpi_device *d)
 {
return d-driver_data;
@@ -532,6 +532,11 @@ static inline bool acpi_device_can_poweroff(struct 
acpi_device *adev)
 
 static inline int register_acpi_bus_type(void *bus) { return 0; }
 static inline int unregister_acpi_bus_type(void *bus) { return 0; }
+static inline int acpi_bus_get_device(acpi_handle handle,
+ struct acpi_device **device)
+{
+   return -ENODEV;
+}
 
 #endif /* CONFIG_ACPI */
 
-- 
1.8.4.rc3

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

[PATCHv2 3/3] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Jarkko Nikula]

Current spi bus_num.chip_select spix.y based device naming scheme may not
be stable enough to be used in name based matching, for instance within
ALSA SoC subsystem.

This can be problem in PC kind of platforms if there are changes in SPI bus
configuration, amount of busses or probe order.

This patch addresses the problem by using the ACPI device name with
spi- prefix for ACPI enumerated SPI slave. For them device name
spix.y becomes spi-INTABCD:ij.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/spi/spi.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 740f9dd..4c0c801 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -38,6 +38,7 @@
 #include linux/kthread.h
 #include linux/ioport.h
 #include linux/acpi.h
+#include acpi/acpi_bus.h
 
 static void spidev_release(struct device *dev)
 {
@@ -352,6 +353,21 @@ struct spi_device *spi_alloc_device(struct spi_master 
*master)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_device);
 
+void spi_dev_set_name(struct spi_device *spi)
+{
+   if (ACPI_HANDLE(spi-dev)) {
+   struct acpi_device *adev;
+   if (!acpi_bus_get_device(ACPI_HANDLE(spi-dev), adev)) {
+   dev_set_name(spi-dev, spi-%s,
+dev_name(adev-dev));
+   return;
+   }
+   }
+
+   dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
+spi-chip_select);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -378,9 +394,7 @@ int spi_add_device(struct spi_device *spi)
}
 
/* Set the bus ID string */
-   dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
-   spi-chip_select);
-
+   spi_dev_set_name(spi);
 
/* We need to make sure there's no other device with this
 * chipselect **BEFORE** we call setup(), else we'll trash
-- 
1.8.4.rc3

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

[PATCHv2 2/3] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Current I2C adapter id - client address x-00yy based device naming scheme
is not always stable enough to be used in name based matching, for instance
within ALSA SoC subsystem.

This is problematic in PC kind of platforms where I2C adapter numbers can
change due variable amount of bus controllers, probe order, add-on cards or
just because of BIOS settings.

This patch addresses the problem by using the ACPI device name with
i2c- prefix for ACPI enumerated I2C slaves. For them device name
x-00yz becomes i2c-INTABCD:ij after this patch.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
 drivers/i2c/i2c-core.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 75ba860..9126c2e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -48,6 +48,7 @@
 #include linux/rwsem.h
 #include linux/pm_runtime.h
 #include linux/acpi.h
+#include acpi/acpi_bus.h
 #include asm/uaccess.h
 
 #include i2c-core.h
@@ -618,6 +619,24 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
+static void i2c_dev_set_name(struct i2c_adapter *adap,
+struct i2c_client *client)
+{
+   if (ACPI_HANDLE(client-dev)) {
+   struct acpi_device *adev;
+   if (!acpi_bus_get_device(ACPI_HANDLE(client-dev), adev)) {
+   dev_set_name(client-dev, i2c-%s,
+dev_name(adev-dev));
+   return;
+   }
+   }
+
+   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
+   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
+client-addr | ((client-flags  I2C_CLIENT_TEN)
+? 0xa000 : 0));
+}
+
 /**
  * i2c_new_device - instantiate an i2c device
  * @adap: the adapter managing the device
@@ -676,10 +695,7 @@ i2c_new_device(struct i2c_adapter *adap, struct 
i2c_board_info const *info)
client-dev.of_node = info-of_node;
ACPI_HANDLE_SET(client-dev, info-acpi_node.handle);
 
-   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
-   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
-client-addr | ((client-flags  I2C_CLIENT_TEN)
-? 0xa000 : 0));
+   i2c_dev_set_name(adap, client);
status = device_register(client-dev);
if (status)
goto out_err;
-- 
1.8.4.rc3

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

[PATCHv2 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

Hi

Second version of the set that changes I2C and SPI slave device names
to be generated from stable ACPI device names on ACPI 5 based systems
instead of using bus numbers which could change.

Slave device name change goes as

x-00yz - i2c-INTABCD:ij
spix.y - spi-INTABCD:ij

This version adds patch to include/acpi/acpi_bus.h that allow us to
remove #if IS_ENABLED(CONFIG_ACPI) checks that were added in the
first version.

Set goes on top linux-pm/linux-next commit e56b4d2.

First version here:
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-October/067737.html

Jarkko Nikula (3):
  ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI
builds
  i2c: Use stable dev_name for ACPI enumerated I2C slaves
  spi: Use stable dev_name for ACPI enumerated SPI slaves

 drivers/i2c/i2c-core.c  | 24 
 drivers/spi/spi.c   | 20 +---
 include/acpi/acpi_bus.h |  9 +++--
 3 files changed, 44 insertions(+), 9 deletions(-)

-- 
1.8.4.rc3

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

Re: [PATCHv2 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

On 11/01/2013 03:20 PM, Wolfram Sang wrote:

On Fri, Nov 01, 2013 at 02:18:06PM +0100, Rafael J. Wysocki wrote:


Looks good to me.  If there are no objections, I can merge these through my 
tree.

Which is basically fine with me. Do you want to have it in 3.13 already?
I mean renaming the devices could lead to regressions, so I'd rather be
conservative and aim for 3.14.

Valid concern. Quick grep below doesn't reveal any obvious device name 
matching outside of sound/soc/ but of course it doesn't prove it to be 
impossible.


git grep '[0-9]\-00' |grep name
git grep 'spi[0-9].[0-9]' |grep name

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

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Hi Rafael

On 10/25/2013 05:08 PM, Rafael J. Wysocki wrote:

On Friday, October 25, 2013  04:30:23 PM Jarkko Nikula wrote:


 Hmm, not only. Referencing to dev field in struct acpi_device by
 dev_name(adev-dev) here will fail too.

 Well, something is not quite right here.

 One of the *reasons* for having ACPI_HANDLE() defined this way is to
 avoid using explicit CONFIG_ACPI checks, so if that doesn't work,
 then all of that becomes a bit pointless.

One possible thing to do is to let structure definitions to be available 
for non-ACPI builds. Then compiler won't fail on structure access which 
will be anyway optimized away by later compiler stages.


With a quick test below vmlinux section sizes don't change for couple 
non-ACPI build tests and allow to get rid off IS_ENABLED(CONFIG_ACPI) 
test in this patch. It's very minimal as it only moves the CONFIG_ACPI 
test just after the struct acpi_device definition and defines 
acpi_bus_get_device for non-ACPI case.


What I don't know how consistent it is as there are still couple 
structure definitions under CONFIG_ACPI, doesn't touch other acpi 
headers and requires to include acpi_bus.h in driver (or move acpi_bus.h 
include in linux/acpi.h currently under CONFIG_ACPI).


--
Jarkko

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index d901982..7ab7870 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
 bool acpi_bay_match(acpi_handle handle);
 bool acpi_dock_match(acpi_handle handle);

-#ifdef CONFIG_ACPI
-
 #include linux/proc_fs.h

 #define ACPI_BUS_FILE_ROOTacpi
@@ -314,6 +312,8 @@ struct acpi_device {
 void (*remove)(struct acpi_device *);
 };

+#if IS_ENABLED(CONFIG_ACPI)
+
 static inline void *acpi_driver_data(struct acpi_device *d)
 {
 return d-driver_data;
@@ -531,6 +531,8 @@ static inline bool acpi_device_can_poweroff(struct 
acpi_device *adev)


 static inline int register_acpi_bus_type(void *bus) { return 0; }
 static inline int unregister_acpi_bus_type(void *bus) { return 0; }
+static inline int acpi_bus_get_device(acpi_handle handle,
+  struct acpi_device **device) { return 0; }

 #endif/* CONFIG_ACPI */

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

[RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Current I2C adapter id - client address x-00yy based device naming scheme
is not always stable enough to be used in name based matching, for instance
within ALSA SoC subsystem.

This is problematic in PC kind of platforms where I2C adapter numbers can
change due variable amount of bus controllers, probe order, add-on cards or
just because of BIOS settings.

This patch addresses the problem by using the ACPI device name with
i2c- prefix for ACPI enumerated I2C slaves. For them device name
x-00yz becomes i2c-INTABCD:ij after this patch.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
I'm not sure when would acpi_bus_get_device fail and how realistic is that here.
I put minimalistic error handling here which just falls back to old
adapter-addr naming scheme.
---
 drivers/i2c/i2c-core.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 111b2c6..8d6f3e5 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -613,6 +613,25 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
+static void i2c_dev_set_name(struct i2c_adapter *adap,
+struct i2c_client *client)
+{
+#if IS_ENABLED(CONFIG_ACPI)
+   if (ACPI_HANDLE(client-dev)) {
+   struct acpi_device *adev;
+   if (!acpi_bus_get_device(ACPI_HANDLE(client-dev), adev)) {
+   dev_set_name(client-dev, i2c-%s,
+dev_name(adev-dev));
+   return;
+   }
+   }
+#endif
+   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
+   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
+client-addr | ((client-flags  I2C_CLIENT_TEN)
+? 0xa000 : 0));
+}
+
 /**
  * i2c_new_device - instantiate an i2c device
  * @adap: the adapter managing the device
@@ -671,10 +690,7 @@ i2c_new_device(struct i2c_adapter *adap, struct 
i2c_board_info const *info)
client-dev.of_node = info-of_node;
ACPI_HANDLE_SET(client-dev, info-acpi_node.handle);
 
-   /* For 10-bit clients, add an arbitrary offset to avoid collisions */
-   dev_set_name(client-dev, %d-%04x, i2c_adapter_id(adap),
-client-addr | ((client-flags  I2C_CLIENT_TEN)
-? 0xa000 : 0));
+   i2c_dev_set_name(adap, client);
status = device_register(client-dev);
if (status)
goto out_err;
-- 
1.8.4.rc3

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

[RFC 0/2] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

Hi

We've run into problem of changing I2C device names while developing
ALSA SoC drivers for x86 based systems. Changing device names makes
more difficult to match devices by their name. Which is what we use
within ASoC subsystem.

These changing names comes from changing adapter/bus numbering which
could occur due variable amount of bus controllers, probe order, add-on
cards or different BIOS settings.

Patches here try to solve the problem on ACPI 5 based systems by
using stable ACPI device name with a i2c-/spi- prefix for I2C/SPI
slave device names.

Both patches are independent from each other and can go through their
own subsystems.

Jarkko Nikula (2):
  i2c: Use stable dev_name for ACPI enumerated I2C slaves
  spi: Use stable dev_name for ACPI enumerated SPI slaves

 drivers/i2c/i2c-core.c | 24 
 drivers/spi/spi.c  | 21 ++---
 2 files changed, 38 insertions(+), 7 deletions(-)

-- 
1.8.4.rc3

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

[RFC 2/2] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Jarkko Nikula]

Current spi bus_num.chip_select spix.y based device naming scheme may not
be stable enough to be used in name based matching, for instance within
ALSA SoC subsystem.

This can be problem in PC kind of platforms if there are changes in SPI bus
configuration, amount of busses or probe order.

This patch addresses the problem by using the ACPI device name with
spi- prefix for ACPI enumerated SPI slave. For them device name
spix.y becomes spi-INTABCD:ij.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
---
may not be and This can be problem since I don't know how likely is to
have changes in SPI bus configuration in PC kind of devices. I assume yes as
this is already problem with I2C devices.
---
 drivers/spi/spi.c | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8d05acc..2ca997f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -345,6 +345,23 @@ struct spi_device *spi_alloc_device(struct spi_master 
*master)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_device);
 
+void spi_dev_set_name(struct spi_device *spi)
+{
+#if IS_ENABLED(CONFIG_ACPI)
+   if (ACPI_HANDLE(spi-dev)) {
+   struct acpi_device *adev;
+   if (!acpi_bus_get_device(ACPI_HANDLE(spi-dev), adev)) {
+   dev_set_name(spi-dev, spi-%s,
+dev_name(adev-dev));
+   return;
+   }
+   }
+#endif
+
+   dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
+spi-chip_select);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -371,9 +388,7 @@ int spi_add_device(struct spi_device *spi)
}
 
/* Set the bus ID string */
-   dev_set_name(spi-dev, %s.%u, dev_name(spi-master-dev),
-   spi-chip_select);
-
+   spi_dev_set_name(spi);
 
/* We need to make sure there's no other device with this
 * chipselect **BEFORE** we call setup(), else we'll trash
-- 
1.8.4.rc3

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

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Hi

On 10/25/2013 03:52 PM, Rafael J. Wysocki wrote:
  
+static void i2c_dev_set_name(struct i2c_adapter *adap,

+struct i2c_client *client)
+{
+#if IS_ENABLED(CONFIG_ACPI)
+   if (ACPI_HANDLE(client-dev)) {
ACPI_HANDLE() already contains an is CONFIG_ACPI enabled? check, so the #if
around the if (ACPI_HANDLE()) {} is redundant.

Similarly for the SPI patch.

Well, acpi_bus_get_device() is not available for non-ACPI builds and at 
least the gcc I used for test build didn't optimize that block away.


--
Jarkko

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

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

On 10/25/2013 04:18 PM, Rafael J. Wysocki wrote:

On Friday, October 25, 2013 03:55:29 PM Jarkko Nikula wrote:


Well, acpi_bus_get_device() is not available for non-ACPI builds and at
least the gcc I used for test build didn't optimize that block away.

Well, it should.  ACPI_HANDLE() translates to (NULL) if CONFIG_ACPI
is not defined and that causes the check to become if (NULL) which
should always be dropped by the compiler.

My very vague memory says the same. I don't know is this gcc version or 
flag dependent behavior. I got the build error from both i386 build 
using gcc 4.8.1 and arm build by using make ARCH=arm omap2plus_defconfig 
and gcc-4.7-arm-linux-gnueabi 4.7.2-4.

Does providing a stub acpi_bus_get_device() for !CONFIG_ACPI actually help?


Hmm, not only. Referencing to dev field in struct acpi_device by 
dev_name(adev-dev) here will fail too.


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

Re: [PATCH] i2c: Fix modalias for ACPI enumerated I2C devices

[Jarkko Nikula]

On 10/16/2013 03:04 AM, Grant Likely wrote:

On Wed, Oct 16, 2013 at 12:47 AM, Rafael J. Wysockir...@sisk.pl  wrote:

On Tuesday, October 15, 2013 04:31:43 PM Greg Kroah-Hartman wrote:

On Tue, Oct 15, 2013 at 11:24:01PM +0200, Rafael J. Wysocki wrote:

On Tuesday, October 15, 2013 01:48:29 PM Greg Kroah-Hartman wrote:

On Tue, Oct 15, 2013 at 10:37:02PM +0200, Rafael J. Wysocki wrote:

On Tuesday, October 15, 2013 07:44:44 PM Zhang Rui wrote:

I have a question about the device uevent and modalias sysfs
attributes.
what is the relationship between these two?
Am I right to say that, if there is the MODALIAS field in uevent file,
this field must be consistent with the content in modalias attribute?

Well, if it isn't, it's pretty pointless, right?

static int platform_uevent(struct device *dev, struct kobj_uevent_env
*env)
{
 struct platform_device  *pdev = to_platform_device(dev);
 int rc;

 /* Some devices have extra OF data and an OF-style MODALIAS */
 rc = of_device_uevent_modalias(dev, env);
 if (rc != -ENODEV)
 return rc;

 add_uevent_var(env, MODALIAS=%s%s, PLATFORM_MODULE_PREFIX,
 pdev-name);
 return 0;
}

This means that the OF-style MODALIAS is not shown in modalias sysfs
attribute.
is this a bug?

Here is an example from one DT based system:

cat /sys/bus/platform/devices/4807.i2c/uevent
DRIVER=omap_i2c
OF_NAME=i2c
OF_FULLNAME=/ocp/i2c@4807
OF_COMPATIBLE_0=ti,omap4-i2c
OF_COMPATIBLE_N=1
MODALIAS=of:Ni2cTNULLCti,omap4-i2c

cat /sys/bus/platform/devices/4807.i2c/modalias
platform:4807.i2c

And a device on that I2C bus:

cat /sys/bus/platform/devices/rtc.11/uevent
DRIVER=twl_rtc
OF_NAME=rtc
OF_FULLNAME=/ocp/i2c@4807/twl@48/rtc
OF_COMPATIBLE_0=ti,twl4030-rtc
OF_COMPATIBLE_N=1
MODALIAS=of:NrtcTNULLCti,twl4030-rtc

cat /sys/bus/platform/devices/rtc.11/modalias
platform:rtc.11

Unfortunately I cannot debug above example further at the moment is 
there failing or needless modprobe calls. Maybe device tree experts know 
better?


--
Jarkko

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

Re: [PATCH] i2c: Fix modalias for ACPI enumerated I2C devices

[Jarkko Nikula]

On 10/12/2013 08:04 AM, Mika Westerberg wrote:

On Sat, Oct 12, 2013 at 12:16:02AM +0200, Rafael J. Wysocki wrote:

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

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

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

Do we? I don't recall seeing per device modaliases on other platforms on 
their platform buses.


And actually I don't see that happening in drivers/base/platform.c: 
platform_uevent() either where just pdev-name is used but not pdev-id 
(which is used with pdev-name for dev_set_name()).


This makes me thinking that perhaps pdevinfo.name = 
dev_name(adev-dev); in drivers/acpi/acpi_platform.c: 
acpi_create_platform_device() should be fixed too as now modalias for 
ACPI registered platform devices differ from platform devices that are 
registered in other subsystems (e.g. regulatory, pcspkr, alarmtimer, etc 
devices)?


I can send a patch for that.

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

Re: [PATCH] i2c: Fix modalias for ACPI enumerated I2C devices

[Jarkko Nikula]

On 10/12/2013 07:18 PM, Mika Westerberg wrote:

If we have two ACPI enumerated devices, they have following modalias:

   i2c-device0: i2c:INTABCD:00
acpi:INTABCD

   i2c-device1: i2c:INTABCD:01
acpi:INTABCD

Likelihood that some random I2C driver has INTABCD:00 or INTABCD:01 ids in
their list is minimal. However, when you turn it to this:


   i2c-device0: i2c:INTABCD
acpi:INTABCD

   i2c-device1: i2c:INTABCD
acpi:INTABCD

It might be possible that we get a match that isn't supposed to happen.
Well, OK it is pretty remote but anyway :-)
Well, name conflicts could occur of course but still I don't think we 
should generate illegal or wrong modaliases. I'm not an udev expert but 
I suppose trying to load nonexisting drivers (i2c_INTABCD:xy) could slow 
booting a little and perhaps pollute needlessly error log compared to if 
it can see that driver is already loaded or tries to load the same 
driver again.


I don't think name conflicts can pose too big risk as they are trivial 
to fix in sources and can be queued to stable too.


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

Re: [PATCH] i2c: Fix modalias for ACPI enumerated I2C devices

[Jarkko Nikula]

On 10/14/2013 12:23 PM, Zhang Rui wrote:

Hi,

On Thu, 2013-10-10 at 17:17 +0300, Jarkko Nikula wrote:

There is a minor fault about ACPI enumerated I2C devices with their modalias
attribute. Now modalias is set by device instance not by hardware ID.
For example i2c:INTABCD:00, i2c:INTABCD:01 etc.

This means each device instance gets different modalias which does match
with generated modules.alias. Currently this is not problem as matching can
happen also with acpi:INTABCD modalias.


IMO, this is not the proper fix for the modalias problem because ACPI
enumerated I2C device may have compatible ids.
Instead, we should export all the compatible ids as the modules alias of
the ACPI enumerated I2C device.

can you please take a look at the patch I sent out earlier?
https://patchwork.kernel.org/patch/3034991/
https://patchwork.kernel.org/patch/3035041/
https://patchwork.kernel.org/patch/3035021/
I see. This makes sense as it avoids that same device has two different 
modaliases from both acpi and other subsystem.


How about modalias nodes in sysfs, should they also reflect what is 
matching uvent?


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

Re: [PATCH 1/3] ACPI: add module autoloading support for ACPI enumerated devices

[Jarkko Nikula]

Hi

On 10/14/2013 02:16 PM, Zhang Rui wrote:

On Mon, 2013-10-14 at 13:14 +0300, Mika Westerberg wrote:

For the three patches.

I wonder if we should do the same for SPI bus as well?


yes, you are right.
Actually, I should check for buses that supports
acpi_driver_match_device() and fix them all.
Will add it in next version.

You could have my pre- Tested-by: Jarkko Nikula 
jarkko.nik...@linux.intel.com.


With your set I see bunch of failing modprobe calls from udev for 
i2c:INTABCD:xy and platform:INTDCBA:xy modaliases gone.


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

[PATCH] i2c: Fix modalias for ACPI enumerated I2C devices

[Jarkko Nikula]

There is a minor fault about ACPI enumerated I2C devices with their modalias
attribute. Now modalias is set by device instance not by hardware ID.
For example i2c:INTABCD:00, i2c:INTABCD:01 etc.

This means each device instance gets different modalias which does match
with generated modules.alias. Currently this is not problem as matching can
happen also with acpi:INTABCD modalias.

Fix this by using ACPI hardware ID.

Signed-off-by: Jarkko Nikula jarkko.nik...@linux.intel.com
Cc: Mika Westerberg mika.westerb...@linux.intel.com
---
Generated on top of v3.12-rc4-29-g0e7a3ed.
---
 drivers/i2c/i2c-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 29d3f04..6dd0c53 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -,7 +,7 @@ static acpi_status acpi_i2c_add_device(acpi_handle 
handle, u32 level,
if (ret  0 || !info.addr)
return AE_OK;
 
-   strlcpy(info.type, dev_name(adev-dev), sizeof(info.type));
+   strlcpy(info.type, acpi_device_hid(adev), sizeof(info.type));
if (!i2c_new_device(adapter, info)) {
dev_err(adapter-dev,
failed to add I2C device %s from ACPI\n,
-- 
1.8.4.rc3

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

Re: [PATCH] i2c: omap: fix OOPS in omap_i2c_unidle() during probe

[Jarkko Nikula]

On Tue, 11 May 2010 10:01:22 +0300
Mika Westerberg ext-mika.1.westerb...@nokia.com wrote:

 I believe this is already in mainline:
 
 http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7c6bd2010fced38444c9fd658f4c6ce61bd185bf
 
Ah, yeah. I was using the sound-2.6.git tree which is not yet updated
any newer than 2.6.34-rc4 and that's why I didn't see it. Sorry about
line noise.


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

Re: [PATCH] i2c: omap: fix OOPS in omap_i2c_unidle() during probe

[Jarkko Nikula]

On Wed, 31 Mar 2010 04:27:41 -0600 (MDT)
Paul Walmsley p...@pwsan.com wrote:

  This patch moves register shift setting before any register accesses are 
  done.
  
  Signed-off-by: Mika Westerberg ext-mika.1.westerb...@nokia.com
  Cc: Cory Maccarrone darkstar6...@gmail.com
 
 Tested-by: Paul Walmsley p...@pwsan.com
 
BTW, Tony had the same fix buried in an another thread:

http://marc.info/?l=linux-omapm=126826012627677w=2

You could add my tested by as well to which one goes in but the fix
should go for 2.6.34 as the mainline doesn't boot otherwise on OMAP.

Tested-by: Jarkko Nikula jhnik...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html