Linux I2C development
 help / color / mirror / Atom feed
* [PATCH] i2c/pasemi: Remove hardcoded bus numbers on smbus
From: Darren Stevens @ 2017-01-24 12:51 UTC (permalink / raw)
  To: wsa, linux-i2c, linux-kernel
  Cc: Pat Wall, R.T.Dickinson, linuxppc-dev, Christian Zigotzky

[-- Attachment #1: Type: text/plain, Size: 178 bytes --]

  AmigaOS...........: http://yam.ch/
  Unix/MacOS/Windows: http://www.mozilla.com/thunderbird/

General information about MIME can be found at:
http://en.wikipedia.org/wiki/MIME

[-- Attachment #2: Type: text/plain, Size: 397 bytes --]

The pasemi smbus controller uses PCI_FUNC(dev->devfn) to define which
number bus to attach to, however this fails when something else is 
probed first, for example an ATI Radeon graphics card will claim 9 or
10 busses, including the ones the pasemi wants.
Patch the driver to call i2c_add_adapter rather than
i2c_add_numbered_adapter.

Signed-off-by: Darren Stevens <darren@stevens-zone.net>

---

[-- Attachment #3: i2c.patch --]
[-- Type: text/plain, Size: 826 bytes --]

diff --git a/drivers/i2c/busses/i2c-pasemi.c b/drivers/i2c/busses/i2c-pasemi.c
index df1dbc9..05847fd 100644
--- a/drivers/i2c/busses/i2c-pasemi.c
+++ b/drivers/i2c/busses/i2c-pasemi.c
@@ -365,7 +365,6 @@ static int pasemi_smb_probe(struct pci_dev *dev,
 	smbus->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	smbus->adapter.algo = &smbus_algorithm;
 	smbus->adapter.algo_data = smbus;
-	smbus->adapter.nr = PCI_FUNC(dev->devfn);
 
 	/* set up the sysfs linkage to our parent device */
 	smbus->adapter.dev.parent = &dev->dev;
@@ -373,7 +372,7 @@ static int pasemi_smb_probe(struct pci_dev *dev,
 	reg_write(smbus, REG_CTL, (CTL_MTR | CTL_MRR |
 		  (CLK_100K_DIV & CTL_CLK_M)));
 
-	error = i2c_add_numbered_adapter(&smbus->adapter);
+	error = i2c_add_adapter(&smbus->adapter);
 	if (error)
 		goto out_release_region;
 

^ permalink raw reply related

* Re: [PATCH v2 06/13] i2c: designware-baytrail: Disallow the CPU to enter C6 or C7 while holding the punit semaphore
From: Andy Shevchenko @ 2017-01-24  9:51 UTC (permalink / raw)
  To: Hans de Goede, Daniel Vetter, Jani Nikula,
	Ville Syrjälä, Jarkko Nikula, Wolfram Sang, Len Brown,
	Thomas Gleixner, H . Peter Anvin
  Cc: intel-gfx, dri-devel, Mika Westerberg, Takashi Iwai,
	russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-7-hdegoede@redhat.com>

On Mon, 2017-01-23 at 22:09 +0100, Hans de Goede wrote:
> On my cherrytrail tablet with axp288 pmic, just doing a bunch of
> repeated
> reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the tablet
> in
> 1 - 3 runs guaranteed.
> 
> This seems to be causes by the cpu trying to enter C6 or C7 while we
> hold
> the punit bus semaphore, at which point everything just hangs.
> 
> Avoid this by disallowing the CPU to enter C6 or C7 before acquiring
> the
> punit bus semaphore.


> Changes in v5:
> -Update the pm_qos for a latency of 0 *before* requesting the
> semaphore,
>  instead of doing it while waiting for the request to be acked

So, that's why you put to reset_semaphore() instead of
baytrail_i2c_release(), right?

I perhaps missed your answer to my comment about that.

> > @@ -56,6 +57,8 @@ static void reset_semaphore(struct dw_i2c_dev
> > *dev)
>  	data &= ~PUNIT_SEMAPHORE_BIT;
>  	if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE,
> PUNIT_SEMAPHORE, data))
>  		dev_err(dev->dev, "iosf failed to reset punit
> semaphore during write\n");
> +
> +	pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
>  }

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH] i2c: Fix error print when address is invalid
From: Wolfram Sang @ 2017-01-24  7:54 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: linux-kernel, linux-i2c
In-Reply-To: <20170124022424.18212-1-stephen.boyd@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

On Mon, Jan 23, 2017 at 06:24:24PM -0800, Stephen Boyd wrote:
> When the address is invalid, we print out the address that's
> in info.addr, but that member of info hasn't been assigned yet.
> It's assigned in the line after we check the address. This causes
> the print to show something like
> 
>      i2c i2c-1: of_i2c: invalid addr=0 on ...
> 
> when we want something like
> 
>      i2c i2c-1: of_i2c: invalid addr=0xaf on ...
> 
> Just pass addr here, and update the print format to indicate hex
> as well.
> 
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

Thanks, yet already fixed since v4.10-rc4 with 6f724fb3039522 ("i2c:
print correct device invalid address").


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH] i2c: Fix error print when address is invalid
From: Stephen Boyd @ 2017-01-24  2:24 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-kernel, linux-i2c

When the address is invalid, we print out the address that's
in info.addr, but that member of info hasn't been assigned yet.
It's assigned in the line after we check the address. This causes
the print to show something like

     i2c i2c-1: of_i2c: invalid addr=0 on ...

when we want something like

     i2c i2c-1: of_i2c: invalid addr=0xaf on ...

Just pass addr here, and update the print format to indicate hex
as well.

Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/i2c/i2c-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index cf9e396d7702..9fec1c232d68 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1707,8 +1707,8 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 	}
 
 	if (i2c_check_addr_validity(addr, info.flags)) {
-		dev_err(&adap->dev, "of_i2c: invalid addr=%x on %s\n",
-			info.addr, node->full_name);
+		dev_err(&adap->dev, "of_i2c: invalid addr=%#x on %s\n",
+			addr, node->full_name);
 		return ERR_PTR(-EINVAL);
 	}
 
-- 
2.10.0.297.gf6727b0

^ permalink raw reply related

* Re: [PATCH] i2c: w1: Add devicetree match for DS2482
From: Evgeniy Polyakov @ 2017-01-24  2:12 UTC (permalink / raw)
  To: Colin Godsey, Ben Gardner
  Cc: Wolfram Sang, linux-i2c@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAExLzncPfvBekS82roFp8D4ceZ_Dknr2KhNYUwSBihL+NiQFAg@mail.gmail.com>

Hi Colin

22.01.2017, 20:55, "Colin Godsey" <crgodsey@gmail.com>:
> Adds the OF devicetree match for the DS2482 I2C-1wire master and adds a listing to the I2C trivial devicetree bindings doc. Based on a proposed patch by Hector Palacios.

Looks good to me.
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>

^ permalink raw reply

* [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Hans de Goede, intel-gfx, dri-devel, Mika Westerberg,
	Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
around P-Unit write accesses.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: tagorereddy <tagore.chandan@gmail.com>
---
Changes in v2:
-Spelling: P-Unit, PMIC
-Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
---
 drivers/gpu/drm/i915/intel_display.c    | 6 ++++++
 drivers/gpu/drm/i915/intel_pm.c         | 9 +++++++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++++++++
 3 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5604701..13e5152 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -47,6 +47,7 @@
 #include <drm/drm_rect.h>
 #include <linux/dma_remapping.h>
 #include <linux/reservation.h>
+#include <asm/iosf_mbi.h>
 
 static bool is_mmio_work(struct intel_flip_work *work)
 {
@@ -6421,6 +6422,8 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 		cmd = 0;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
+	iosf_mbi_punit_acquire();
+
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 	val &= ~DSPFREQGUAR_MASK;
 	val |= (cmd << DSPFREQGUAR_SHIFT);
@@ -6430,6 +6433,7 @@ static void valleyview_set_cdclk(struct drm_device *dev, int cdclk)
 		     50)) {
 		DRM_ERROR("timed out waiting for CDclk change\n");
 	}
+	iosf_mbi_punit_release();
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	mutex_lock(&dev_priv->sb_lock);
@@ -6497,6 +6501,7 @@ static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
 	cmd = DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, cdclk) - 1;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
+	iosf_mbi_punit_acquire();
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 	val &= ~DSPFREQGUAR_MASK_CHV;
 	val |= (cmd << DSPFREQGUAR_SHIFT_CHV);
@@ -6506,6 +6511,7 @@ static void cherryview_set_cdclk(struct drm_device *dev, int cdclk)
 		     50)) {
 		DRM_ERROR("timed out waiting for CDclk change\n");
 	}
+	iosf_mbi_punit_release();
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
 	intel_update_cdclk(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 249623d..adff84a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -32,6 +32,7 @@
 #include "../../../platform/x86/intel_ips.h"
 #include <linux/module.h>
 #include <drm/drm_atomic_helper.h>
+#include <asm/iosf_mbi.h>
 
 /**
  * DOC: RC6
@@ -276,6 +277,7 @@ static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool enable)
 	u32 val;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
+	iosf_mbi_punit_acquire();
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
 	if (enable)
@@ -290,6 +292,7 @@ static void chv_set_memory_dvfs(struct drm_i915_private *dev_priv, bool enable)
 		      FORCE_DDR_FREQ_REQ_ACK) == 0, 3))
 		DRM_ERROR("timed out waiting for Punit DDR DVFS request\n");
 
+	iosf_mbi_punit_release();
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
@@ -298,6 +301,7 @@ static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
 	u32 val;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
+	iosf_mbi_punit_acquire();
 
 	val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 	if (enable)
@@ -306,6 +310,7 @@ static void chv_set_memory_pm5(struct drm_i915_private *dev_priv, bool enable)
 		val &= ~DSP_MAXFIFO_PM5_ENABLE;
 	vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
 
+	iosf_mbi_punit_release();
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
@@ -4553,6 +4558,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 
 	if (IS_CHERRYVIEW(dev_priv)) {
 		mutex_lock(&dev_priv->rps.hw_lock);
+		iosf_mbi_punit_acquire();
 
 		val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 		if (val & DSP_MAXFIFO_PM5_ENABLE)
@@ -4582,6 +4588,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
 				wm->level = VLV_WM_LEVEL_DDR_DVFS;
 		}
 
+		iosf_mbi_punit_release();
 		mutex_unlock(&dev_priv->rps.hw_lock);
 	}
 
@@ -4988,7 +4995,9 @@ static void valleyview_set_rps(struct drm_i915_private *dev_priv, u8 val)
 	I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val));
 
 	if (val != dev_priv->rps.cur_freq) {
+		iosf_mbi_punit_acquire();
 		vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val);
+		iosf_mbi_punit_release();
 		if (!IS_CHERRYVIEW(dev_priv))
 			gen6_set_rps_thresholds(dev_priv, val);
 	}
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index c0b7e95..e66bcc8 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -28,6 +28,7 @@
 
 #include <linux/pm_runtime.h>
 #include <linux/vgaarb.h>
+#include <asm/iosf_mbi.h>
 
 #include "i915_drv.h"
 #include "intel_drv.h"
@@ -1027,6 +1028,8 @@ static void vlv_set_power_well(struct drm_i915_private *dev_priv,
 	if (COND)
 		goto out;
 
+	iosf_mbi_punit_acquire();
+
 	ctrl = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_CTRL);
 	ctrl &= ~mask;
 	ctrl |= state;
@@ -1037,6 +1040,8 @@ static void vlv_set_power_well(struct drm_i915_private *dev_priv,
 			  state,
 			  vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_CTRL));
 
+	iosf_mbi_punit_release();
+
 #undef COND
 
 out:
@@ -1643,6 +1648,8 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
 	if (COND)
 		goto out;
 
+	iosf_mbi_punit_acquire();
+
 	ctrl = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
 	ctrl &= ~DP_SSC_MASK(pipe);
 	ctrl |= enable ? DP_SSC_PWR_ON(pipe) : DP_SSC_PWR_GATE(pipe);
@@ -1653,6 +1660,8 @@ static void chv_set_pipe_power_well(struct drm_i915_private *dev_priv,
 			  state,
 			  vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ));
 
+	iosf_mbi_punit_release();
+
 #undef COND
 
 out:
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 12/13] drm/i915: Listen for PMIC bus access notifications
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Hans de Goede, intel-gfx, dri-devel, Mika Westerberg,
	Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Listen for PMIC bus access notifications and get FORCEWAKE_ALL while
the bus is accessed to avoid needing to do any forcewakes, which need
PMIC bus access, while the PMIC bus is busy:

This fixes errors like these showing up in dmesg, usually followed
by a gfx or system freeze:

[drm:fw_domains_get [i915]] *ERROR* render: timed out waiting for forcewake ack request.
[drm:fw_domains_get [i915]] *MEDIA* render: timed out waiting for forcewake ack request.
i2c_designware 808622C1:06: punit semaphore timed out, resetting
i2c_designware 808622C1:06: PUNIT SEM: 2
i2c_designware 808622C1:06: couldn't acquire bus ownership

Downside of this approach is that it causes wakeups whenever the PMIC
bus is accessed. Unfortunately we cannot simply wait for the PMIC bus
to go idle when we hit a race, as forcewakes may be done from interrupt
handlers where we cannot sleep to wait for the i2c PMIC bus access to
finish.

Note that the notifications and thus the wakeups will only happen on
baytrail / cherrytrail devices using PMICs with a shared i2c bus for
P-Unit and host PMIC access (i2c busses with a _SEM method in their
APCI node), e.g. an axp288 PMIC.

I plan to write some patches for drivers accessing the PMIC bus to
limit their bus accesses to a bare minimum (e.g. cache registers, do not
update battery level more often then 4 times a minute), to limit the
amount of wakeups.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: tagorereddy <tagore.chandan@gmail.com>
---
Changes in v2:
-Spelling: P-Unit, PMIC
---
 drivers/gpu/drm/i915/i915_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_uncore.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c717329..52f7dde 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -721,6 +721,7 @@ struct intel_uncore {
 	const struct intel_forcewake_range *fw_domains_table;
 	unsigned int fw_domains_table_entries;
 
+	struct notifier_block pmic_bus_access_nb;
 	struct intel_uncore_funcs funcs;
 
 	unsigned fifo_count;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 3767307..175fe02 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -25,6 +25,7 @@
 #include "intel_drv.h"
 #include "i915_vgpu.h"
 
+#include <asm/iosf_mbi.h>
 #include <linux/pm_runtime.h>
 
 #define FORCEWAKE_ACK_TIMEOUT_MS 50
@@ -429,12 +430,16 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 
 void intel_uncore_suspend(struct drm_i915_private *dev_priv)
 {
+	iosf_mbi_unregister_pmic_bus_access_notifier(
+		&dev_priv->uncore.pmic_bus_access_nb);
 	__intel_uncore_forcewake_reset(dev_priv, false);
 }
 
 void intel_uncore_resume(struct drm_i915_private *dev_priv)
 {
 	__intel_uncore_early_sanitize(dev_priv, true);
+	iosf_mbi_register_pmic_bus_access_notifier(
+		&dev_priv->uncore.pmic_bus_access_nb);
 	i915_check_and_clear_faults(dev_priv);
 }
 
@@ -1390,6 +1395,28 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 	dev_priv->uncore.fw_domains_table_entries = ARRAY_SIZE((d)); \
 }
 
+static int i915_pmic_bus_access_notifier(struct notifier_block *nb,
+					 unsigned long action, void *data)
+{
+	struct drm_i915_private *dev_priv = container_of(nb,
+			struct drm_i915_private, uncore.pmic_bus_access_nb);
+
+	switch (action) {
+	case MBI_PMIC_BUS_ACCESS_BEGIN:
+		/*
+		 * forcewake all to make sure that we don't need to forcewake
+		 * any power-planes while the pmic bus is busy.
+		 */
+		intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+		break;
+	case MBI_PMIC_BUS_ACCESS_END:
+		intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
 void intel_uncore_init(struct drm_i915_private *dev_priv)
 {
 	i915_check_vgpu(dev_priv);
@@ -1399,6 +1426,8 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 	__intel_uncore_early_sanitize(dev_priv, false);
 
 	dev_priv->uncore.unclaimed_mmio_check = 1;
+	dev_priv->uncore.pmic_bus_access_nb.notifier_call =
+		i915_pmic_bus_access_notifier;
 
 	switch (INTEL_INFO(dev_priv)->gen) {
 	default:
@@ -1458,6 +1487,9 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		ASSIGN_READ_MMIO_VFUNCS(vgpu);
 	}
 
+	iosf_mbi_register_pmic_bus_access_notifier(
+		&dev_priv->uncore.pmic_bus_access_nb);
+
 	i915_check_and_clear_faults(dev_priv);
 }
 #undef ASSIGN_WRITE_MMIO_VFUNCS
@@ -1465,6 +1497,9 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 
 void intel_uncore_fini(struct drm_i915_private *dev_priv)
 {
+	iosf_mbi_unregister_pmic_bus_access_notifier(
+		&dev_priv->uncore.pmic_bus_access_nb);
+
 	/* Paranoia: make sure we have disabled everything before we exit. */
 	intel_uncore_sanitize(dev_priv);
 	__intel_uncore_forcewake_reset(dev_priv, false);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 11/13] drm/i915: Add intel_uncore_suspend / resume functions
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Hans de Goede, intel-gfx, dri-devel, Mika Westerberg,
	Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Rename intel_uncore_early_sanitize to intel_uncore_resume, dropping the
(always true) restore_forcewake argument and add a new intel_uncore_resume
function to replace the intel_uncore_forcewake_reset(dev_priv, false)
calls done from the suspend / runtime_suspend functions and make
intel_uncore_forcewake_reset private.

This is a preparation patch for adding PMIC bus access notifier support.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: tagorereddy <tagore.chandan@gmail.com>
---
Changes in v2:
-Spelling: P-Unit, PMIC
---
 drivers/gpu/drm/i915/i915_drv.c     |  6 +++---
 drivers/gpu/drm/i915/i915_drv.h     |  6 ++----
 drivers/gpu/drm/i915/intel_uncore.c | 18 +++++++++++-------
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index aefab9a..5a62d7a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1445,7 +1445,7 @@ static int i915_drm_suspend(struct drm_device *dev)
 	opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
 	intel_opregion_notify_adapter(dev_priv, opregion_target_state);
 
-	intel_uncore_forcewake_reset(dev_priv, false);
+	intel_uncore_suspend(dev_priv);
 	intel_opregion_unregister(dev_priv);
 
 	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
@@ -1690,7 +1690,7 @@ static int i915_drm_resume_early(struct drm_device *dev)
 		DRM_ERROR("Resume prepare failed: %d, continuing anyway\n",
 			  ret);
 
-	intel_uncore_early_sanitize(dev_priv, true);
+	intel_uncore_resume(dev_priv);
 
 	if (IS_GEN9_LP(dev_priv)) {
 		if (!dev_priv->suspended_to_idle)
@@ -2344,7 +2344,7 @@ static int intel_runtime_suspend(struct device *kdev)
 		return ret;
 	}
 
-	intel_uncore_forcewake_reset(dev_priv, false);
+	intel_uncore_suspend(dev_priv);
 
 	enable_rpm_wakeref_asserts(dev_priv);
 	WARN_ON_ONCE(atomic_read(&dev_priv->pm.wakeref_count));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e9b4ece..c717329 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2976,14 +2976,12 @@ int intel_irq_install(struct drm_i915_private *dev_priv);
 void intel_irq_uninstall(struct drm_i915_private *dev_priv);
 
 extern void intel_uncore_sanitize(struct drm_i915_private *dev_priv);
-extern void intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
-					bool restore_forcewake);
 extern void intel_uncore_init(struct drm_i915_private *dev_priv);
 extern bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv);
 extern bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv);
 extern void intel_uncore_fini(struct drm_i915_private *dev_priv);
-extern void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
-					 bool restore);
+extern void intel_uncore_suspend(struct drm_i915_private *dev_priv);
+extern void intel_uncore_resume(struct drm_i915_private *dev_priv);
 const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
 void intel_uncore_forcewake_get(struct drm_i915_private *dev_priv,
 				enum forcewake_domains domains);
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index abe0888..3767307 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -250,7 +250,7 @@ intel_uncore_fw_release_timer(struct hrtimer *timer)
 	return HRTIMER_NORESTART;
 }
 
-void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
+static void __intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv,
 				  bool restore)
 {
 	unsigned long irqflags;
@@ -424,13 +424,17 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST))
 		info->has_decoupled_mmio = false;
 
-	intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
+	__intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
 }
 
-void intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
-				 bool restore_forcewake)
+void intel_uncore_suspend(struct drm_i915_private *dev_priv)
 {
-	__intel_uncore_early_sanitize(dev_priv, restore_forcewake);
+	__intel_uncore_forcewake_reset(dev_priv, false);
+}
+
+void intel_uncore_resume(struct drm_i915_private *dev_priv)
+{
+	__intel_uncore_early_sanitize(dev_priv, true);
 	i915_check_and_clear_faults(dev_priv);
 }
 
@@ -1463,7 +1467,7 @@ void intel_uncore_fini(struct drm_i915_private *dev_priv)
 {
 	/* Paranoia: make sure we have disabled everything before we exit. */
 	intel_uncore_sanitize(dev_priv);
-	intel_uncore_forcewake_reset(dev_priv, false);
+	__intel_uncore_forcewake_reset(dev_priv, false);
 }
 
 #define GEN_RANGE(l, h) GENMASK((h) - 1, (l) - 1)
@@ -1679,7 +1683,7 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
 
 	ret = gen6_hw_domain_reset(dev_priv, hw_mask);
 
-	intel_uncore_forcewake_reset(dev_priv, true);
+	__intel_uncore_forcewake_reset(dev_priv, true);
 
 	return ret;
 }
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 09/13] i2c: designware-baytrail: Acquire P-Unit access on bus acquire
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Hans de Goede, intel-gfx, dri-devel, Mika Westerberg,
	Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Acquire P-Unit access to stop others from accessing the P-Unit while the
PMIC i2c bus is in use. This is necessary because accessing the P-Unit
from the kernel may result in the P-Unit trying to access the PMIC i2c
bus, which results in a hang when it happens while we own the PMIC i2c
bus semaphore.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: tagorereddy <tagore.chandan@gmail.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
Changes in v2:
-Spelling: P-Unit, PMIC
-Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 3effc9a..7eddc3b 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -62,6 +62,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
 		dev_err(dev->dev, "iosf failed to reset punit semaphore during write\n");
 
 	pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
+
+	iosf_mbi_punit_release();
 }
 
 static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
@@ -79,6 +81,8 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 	if (!dev->release_lock)
 		return 0;
 
+	iosf_mbi_punit_acquire();
+
 	/*
 	 * Disallow the CPU to enter C6 or C7 state, entering these states
 	 * requires the punit to talk to the pmic and if this happens while
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 07/13] i2c: designware-baytrail: Fix race when resetting the semaphore
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Hans de Goede, intel-gfx, dri-devel, Mika Westerberg,
	Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Use iosf_mbi_modify instead of iosf_mbi_read + iosf_mbi_write so that
we keep the iosf_mbi_lock locked during the read-modify-write done to
reset the semaphore.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
Changes in v5:
-New patch in v5 of this patch-set
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 650a700..8df529c 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -47,15 +47,8 @@ static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
 
 static void reset_semaphore(struct dw_i2c_dev *dev)
 {
-	u32 data;
-
-	if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &data)) {
-		dev_err(dev->dev, "iosf failed to reset punit semaphore during read\n");
-		return;
-	}
-
-	data &= ~PUNIT_SEMAPHORE_BIT;
-	if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, PUNIT_SEMAPHORE, data))
+	if (iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE,
+			    0, PUNIT_SEMAPHORE_BIT))
 		dev_err(dev->dev, "iosf failed to reset punit semaphore during write\n");
 
 	pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 06/13] i2c: designware-baytrail: Disallow the CPU to enter C6 or C7 while holding the punit semaphore
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Hans de Goede, intel-gfx, dri-devel, Mika Westerberg,
	Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

On my cherrytrail tablet with axp288 pmic, just doing a bunch of repeated
reads from the pmic, e.g. "i2cdump -y 14 0x34" would lookup the tablet in
1 - 3 runs guaranteed.

This seems to be causes by the cpu trying to enter C6 or C7 while we hold
the punit bus semaphore, at which point everything just hangs.

Avoid this by disallowing the CPU to enter C6 or C7 before acquiring the
punit bus semaphore.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
Changes in v2:
-New patch in v2 of this set
Changes in v3:
-Change commit message and comment in the code from "force the CPU to C1"
 to "Disallow the CPU to enter C6 or C7", as the CPU may still be in either
 C0 or C1 with the request pm_qos
Changes in v4:
-Rename i2c_dw_eval_lock_support to i2c_dw_probe_lock_support so that we can
 add a matching i2c_dw_remove_lock_support cleanup function
-Move qm_pos removal to new i2c_dw_remove_lock_support function
-Move pm_qos_add_request to the end of i2c_dw_probe_lock_support
Changes in v5:
-Update the pm_qos for a latency of 0 *before* requesting the semaphore,
 instead of doing it while waiting for the request to be acked
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 24 ++++++++++++++++++++++--
 drivers/i2c/busses/i2c-designware-core.h     |  9 +++++++--
 drivers/i2c/busses/i2c-designware-platdrv.c  |  4 +++-
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index cf02222..650a700 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/pm_qos.h>
 
 #include <asm/iosf_mbi.h>
 
@@ -56,6 +57,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
 	data &= ~PUNIT_SEMAPHORE_BIT;
 	if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, PUNIT_SEMAPHORE, data))
 		dev_err(dev->dev, "iosf failed to reset punit semaphore during write\n");
+
+	pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
 }
 
 static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
@@ -72,11 +75,18 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 	if (!dev->release_lock)
 		return 0;
 
+	/*
+	 * Disallow the CPU to enter C6 or C7 state, entering these states
+	 * requires the punit to talk to the pmic and if this happens while
+	 * we're holding the semaphore, the SoC hangs.
+	 */
+	pm_qos_update_request(&dev->pm_qos, 0);
+
 	/* host driver writes to side band semaphore register */
 	ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, PUNIT_SEMAPHORE, sem);
 	if (ret) {
 		dev_err(dev->dev, "iosf punit semaphore request failed\n");
-		return ret;
+		goto out;
 	}
 
 	/* host driver waits for bit 0 to be set in semaphore register */
@@ -95,6 +105,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 	} while (time_before(jiffies, end));
 
 	dev_err(dev->dev, "punit semaphore timed out, resetting\n");
+out:
 	reset_semaphore(dev);
 
 	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &sem);
@@ -121,7 +132,7 @@ static void baytrail_i2c_release(struct dw_i2c_dev *dev)
 		jiffies_to_msecs(jiffies - acquired));
 }
 
-int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev)
+int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev)
 {
 	acpi_status status;
 	unsigned long long shared_host = 0;
@@ -149,5 +160,14 @@ int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev)
 	dev->release_lock = baytrail_i2c_release;
 	dev->pm_runtime_disabled = true;
 
+	pm_qos_add_request(&dev->pm_qos, PM_QOS_CPU_DMA_LATENCY,
+			   PM_QOS_DEFAULT_VALUE);
+
 	return 0;
 }
+
+void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev)
+{
+	if (dev->acquire_lock)
+		pm_qos_remove_request(&dev->pm_qos);
+}
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 2c50571..94a5fd1 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -23,6 +23,7 @@
  */
 
 #include <linux/i2c.h>
+#include <linux/pm_qos.h>
 
 #define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C |			\
 					I2C_FUNC_SMBUS_BYTE |		\
@@ -75,6 +76,7 @@
  * @fp_lcnt: fast plus LCNT value
  * @hs_hcnt: high speed HCNT value
  * @hs_lcnt: high speed LCNT value
+ * @pm_qos: pm_qos_request used while holding a hardware lock on the bus
  * @acquire_lock: function to acquire a hardware lock on the bus
  * @release_lock: function to release a hardware lock on the bus
  * @pm_runtime_disabled: true if pm runtime is disabled
@@ -122,6 +124,7 @@ struct dw_i2c_dev {
 	u16			fp_lcnt;
 	u16			hs_hcnt;
 	u16			hs_lcnt;
+	struct pm_qos_request	pm_qos;
 	int			(*acquire_lock)(struct dw_i2c_dev *dev);
 	void			(*release_lock)(struct dw_i2c_dev *dev);
 	bool			pm_runtime_disabled;
@@ -139,7 +142,9 @@ 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);
+extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
+extern void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev);
 #else
-static inline int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev) { return 0; }
+static inline int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) { return 0; }
+static inline void i2c_dw_remove_lock_support(struct dw_i2c_dev *dev) {}
 #endif
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3eede7b..d474db0 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -238,7 +238,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	r = i2c_dw_eval_lock_support(dev);
+	r = i2c_dw_probe_lock_support(dev);
 	if (r)
 		return r;
 
@@ -307,6 +307,8 @@ static int dw_i2c_plat_remove(struct platform_device *pdev)
 	if (!dev->pm_runtime_disabled)
 		pm_runtime_disable(&pdev->dev);
 
+	i2c_dw_remove_lock_support(dev);
+
 	return 0;
 }
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 10/13] i2c: designware-baytrail: Call pmic_bus_access_notifier_chain
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Hans de Goede, intel-gfx, dri-devel, Mika Westerberg,
	Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Call the iosf_mbi pmic_bus_access_notifier_chain on bus acquire / release.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: tagorereddy <tagore.chandan@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
Changes in v2:
-Spelling: P-Unit, PMIC
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 7eddc3b..1749a0f 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -63,6 +63,8 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
 
 	pm_qos_update_request(&dev->pm_qos, PM_QOS_DEFAULT_VALUE);
 
+	iosf_mbi_call_pmic_bus_access_notifier_chain(MBI_PMIC_BUS_ACCESS_END,
+						     NULL);
 	iosf_mbi_punit_release();
 }
 
@@ -82,6 +84,8 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 		return 0;
 
 	iosf_mbi_punit_acquire();
+	iosf_mbi_call_pmic_bus_access_notifier_chain(MBI_PMIC_BUS_ACCESS_BEGIN,
+						     NULL);
 
 	/*
 	 * Disallow the CPU to enter C6 or C7 state, entering these states
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 08/13] i2c: designware-baytrail: Add support for cherrytrail
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Takashi Iwai, russianneuromancer @ ya . ru, intel-gfx, dri-devel,
	Hans de Goede, linux-i2c, Mika Westerberg
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

The cherrytrail punit has the pmic i2c bus access semaphore at a
different register address.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
Changes in v2:
-Adjust for accessor_flags -> flags rename
-Add flags field to struct dw_pci_controller
-Add get_sem_addr() helper replacing MODEL_CHERRYTRAIL flag checking in
 PUNIT_SEMAPHORE macro
Changes in v3:
-Add a gap between ACCESS_* and MODEL_* flags as reserved space for
 future ACCESS_* flags
Changes in v4:
-s/CHV/CHT/
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 19 +++++++++++++++----
 drivers/i2c/busses/i2c-designware-core.h     |  2 ++
 drivers/i2c/busses/i2c-designware-pcidrv.c   | 26 +++++++++++++++++++-------
 drivers/i2c/busses/i2c-designware-platdrv.c  |  2 +-
 4 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 8df529c..3effc9a 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -24,17 +24,27 @@
 
 #define SEMAPHORE_TIMEOUT	100
 #define PUNIT_SEMAPHORE		0x7
+#define PUNIT_SEMAPHORE_CHT	0x10e
 #define PUNIT_SEMAPHORE_BIT	BIT(0)
 #define PUNIT_SEMAPHORE_ACQUIRE	BIT(1)
 
 static unsigned long acquired;
 
+static u32 get_sem_addr(struct dw_i2c_dev *dev)
+{
+	if (dev->flags & MODEL_CHERRYTRAIL)
+		return PUNIT_SEMAPHORE_CHT;
+	else
+		return PUNIT_SEMAPHORE;
+}
+
 static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
 {
+	u32 addr = get_sem_addr(dev);
 	u32 data;
 	int ret;
 
-	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &data);
+	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr, &data);
 	if (ret) {
 		dev_err(dev->dev, "iosf failed to read punit semaphore\n");
 		return ret;
@@ -47,7 +57,7 @@ static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
 
 static void reset_semaphore(struct dw_i2c_dev *dev)
 {
-	if (iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE,
+	if (iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, get_sem_addr(dev),
 			    0, PUNIT_SEMAPHORE_BIT))
 		dev_err(dev->dev, "iosf failed to reset punit semaphore during write\n");
 
@@ -56,6 +66,7 @@ static void reset_semaphore(struct dw_i2c_dev *dev)
 
 static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 {
+	u32 addr = get_sem_addr(dev);
 	u32 sem = PUNIT_SEMAPHORE_ACQUIRE;
 	int ret;
 	unsigned long start, end;
@@ -76,7 +87,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 	pm_qos_update_request(&dev->pm_qos, 0);
 
 	/* host driver writes to side band semaphore register */
-	ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, PUNIT_SEMAPHORE, sem);
+	ret = iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, addr, sem);
 	if (ret) {
 		dev_err(dev->dev, "iosf punit semaphore request failed\n");
 		goto out;
@@ -101,7 +112,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 out:
 	reset_semaphore(dev);
 
-	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &sem);
+	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, addr, &sem);
 	if (ret)
 		dev_err(dev->dev, "iosf failed to read punit semaphore\n");
 	else
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 94a5fd1..a8e74ca 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -135,6 +135,8 @@ struct dw_i2c_dev {
 #define ACCESS_16BIT		0x00000002
 #define ACCESS_INTR_MASK	0x00000004
 
+#define MODEL_CHERRYTRAIL	0x00000100
+
 extern int i2c_dw_init(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);
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d6423cf..ed485b6 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -45,6 +45,7 @@ enum dw_pci_ctl_id_t {
 	medfield,
 	merrifield,
 	baytrail,
+	cherrytrail,
 	haswell,
 };
 
@@ -63,6 +64,7 @@ struct dw_pci_controller {
 	u32 rx_fifo_depth;
 	u32 clk_khz;
 	u32 functionality;
+	u32 flags;
 	struct dw_scl_sda_cfg *scl_sda_cfg;
 	int (*setup)(struct pci_dev *pdev, struct dw_pci_controller *c);
 };
@@ -170,6 +172,15 @@ static struct dw_pci_controller dw_pci_controllers[] = {
 		.functionality = I2C_FUNC_10BIT_ADDR,
 		.scl_sda_cfg = &hsw_config,
 	},
+	[cherrytrail] = {
+		.bus_num = -1,
+		.bus_cfg = INTEL_MID_STD_CFG | DW_IC_CON_SPEED_FAST,
+		.tx_fifo_depth = 32,
+		.rx_fifo_depth = 32,
+		.functionality = I2C_FUNC_10BIT_ADDR,
+		.flags = MODEL_CHERRYTRAIL,
+		.scl_sda_cfg = &byt_config,
+	},
 };
 
 #ifdef CONFIG_PM
@@ -237,6 +248,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	dev->base = pcim_iomap_table(pdev)[0];
 	dev->dev = &pdev->dev;
 	dev->irq = pdev->irq;
+	dev->flags |= controller->flags;
 
 	if (controller->setup) {
 		r = controller->setup(pdev, controller);
@@ -317,13 +329,13 @@ static const struct pci_device_id i2_designware_pci_ids[] = {
 	{ PCI_VDEVICE(INTEL, 0x9c61), haswell },
 	{ PCI_VDEVICE(INTEL, 0x9c62), haswell },
 	/* Braswell / Cherrytrail */
-	{ PCI_VDEVICE(INTEL, 0x22C1), baytrail },
-	{ PCI_VDEVICE(INTEL, 0x22C2), baytrail },
-	{ PCI_VDEVICE(INTEL, 0x22C3), baytrail },
-	{ PCI_VDEVICE(INTEL, 0x22C4), baytrail },
-	{ PCI_VDEVICE(INTEL, 0x22C5), baytrail },
-	{ PCI_VDEVICE(INTEL, 0x22C6), baytrail },
-	{ PCI_VDEVICE(INTEL, 0x22C7), baytrail },
+	{ PCI_VDEVICE(INTEL, 0x22C1), cherrytrail },
+	{ PCI_VDEVICE(INTEL, 0x22C2), cherrytrail },
+	{ PCI_VDEVICE(INTEL, 0x22C3), cherrytrail },
+	{ PCI_VDEVICE(INTEL, 0x22C4), cherrytrail },
+	{ PCI_VDEVICE(INTEL, 0x22C5), cherrytrail },
+	{ PCI_VDEVICE(INTEL, 0x22C6), cherrytrail },
+	{ PCI_VDEVICE(INTEL, 0x22C7), cherrytrail },
 	{ 0,}
 };
 MODULE_DEVICE_TABLE(pci, i2_designware_pci_ids);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d474db0..df0ff7d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -123,7 +123,7 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
 	{ "INT3432", 0 },
 	{ "INT3433", 0 },
 	{ "80860F41", 0 },
-	{ "808622C1", 0 },
+	{ "808622C1", MODEL_CHERRYTRAIL },
 	{ "AMD0010", ACCESS_INTR_MASK },
 	{ "AMDI0010", ACCESS_INTR_MASK },
 	{ "AMDI0510", 0 },
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [PATCH v2 05/13] i2c: designware-baytrail: Only check iosf_mbi_available() for shared hosts
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Takashi Iwai, russianneuromancer @ ya . ru, intel-gfx, dri-devel,
	Hans de Goede, linux-i2c, Mika Westerberg
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

If (!shared_host) simply return 0, this avoids delaying the probe if
iosf_mbi_available() returns false when an i2c bus is not using the
punit semaphore.

Also move the if (!iosf_mbi_available()) check to above the
dev_info, so that we do not repeat the dev_info on every probe
until iosf_mbi_available() returns true.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
Changes in v2:
-New patch in v2 of this set
Changes in v3:
-Use if (!shared_host) return 0, to simplify the non-shared_host path
 and to avoid nested ifs
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index a3f581c..cf02222 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -138,15 +138,16 @@ int i2c_dw_eval_lock_support(struct dw_i2c_dev *dev)
 	if (ACPI_FAILURE(status))
 		return 0;
 
-	if (shared_host) {
-		dev_info(dev->dev, "I2C bus managed by PUNIT\n");
-		dev->acquire_lock = baytrail_i2c_acquire;
-		dev->release_lock = baytrail_i2c_release;
-		dev->pm_runtime_disabled = true;
-	}
+	if (!shared_host)
+		return 0;
 
 	if (!iosf_mbi_available())
 		return -EPROBE_DEFER;
 
+	dev_info(dev->dev, "I2C bus managed by PUNIT\n");
+	dev->acquire_lock = baytrail_i2c_acquire;
+	dev->release_lock = baytrail_i2c_release;
+	dev->pm_runtime_disabled = true;
+
 	return 0;
 }
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [PATCH v2 04/13] i2c: designware-baytrail: Pass dw_i2c_dev into helper functions
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Takashi Iwai, russianneuromancer @ ya . ru, intel-gfx, dri-devel,
	Hans de Goede, linux-i2c, Mika Westerberg
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Pass dw_i2c_dev into the helper functions, this is a preparation patch
for the punit semaphore fixes done in the other patches in this set.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
 drivers/i2c/busses/i2c-designware-baytrail.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-baytrail.c b/drivers/i2c/busses/i2c-designware-baytrail.c
index 1590ad0..a3f581c 100644
--- a/drivers/i2c/busses/i2c-designware-baytrail.c
+++ b/drivers/i2c/busses/i2c-designware-baytrail.c
@@ -28,14 +28,14 @@
 
 static unsigned long acquired;
 
-static int get_sem(struct device *dev, u32 *sem)
+static int get_sem(struct dw_i2c_dev *dev, u32 *sem)
 {
 	u32 data;
 	int ret;
 
 	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &data);
 	if (ret) {
-		dev_err(dev, "iosf failed to read punit semaphore\n");
+		dev_err(dev->dev, "iosf failed to read punit semaphore\n");
 		return ret;
 	}
 
@@ -44,18 +44,18 @@ static int get_sem(struct device *dev, u32 *sem)
 	return 0;
 }
 
-static void reset_semaphore(struct device *dev)
+static void reset_semaphore(struct dw_i2c_dev *dev)
 {
 	u32 data;
 
 	if (iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &data)) {
-		dev_err(dev, "iosf failed to reset punit semaphore during read\n");
+		dev_err(dev->dev, "iosf failed to reset punit semaphore during read\n");
 		return;
 	}
 
 	data &= ~PUNIT_SEMAPHORE_BIT;
 	if (iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, PUNIT_SEMAPHORE, data))
-		dev_err(dev, "iosf failed to reset punit semaphore during write\n");
+		dev_err(dev->dev, "iosf failed to reset punit semaphore during write\n");
 }
 
 static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
@@ -83,7 +83,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 	start = jiffies;
 	end = start + msecs_to_jiffies(SEMAPHORE_TIMEOUT);
 	do {
-		ret = get_sem(dev->dev, &sem);
+		ret = get_sem(dev, &sem);
 		if (!ret && sem) {
 			acquired = jiffies;
 			dev_dbg(dev->dev, "punit semaphore acquired after %ums\n",
@@ -95,7 +95,7 @@ static int baytrail_i2c_acquire(struct dw_i2c_dev *dev)
 	} while (time_before(jiffies, end));
 
 	dev_err(dev->dev, "punit semaphore timed out, resetting\n");
-	reset_semaphore(dev->dev);
+	reset_semaphore(dev);
 
 	ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, PUNIT_SEMAPHORE, &sem);
 	if (ret)
@@ -116,7 +116,7 @@ static void baytrail_i2c_release(struct dw_i2c_dev *dev)
 	if (!dev->acquire_lock)
 		return;
 
-	reset_semaphore(dev->dev);
+	reset_semaphore(dev);
 	dev_dbg(dev->dev, "punit semaphore held for %ums\n",
 		jiffies_to_msecs(jiffies - acquired));
 }
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [PATCH v2 03/13] i2c: designware: Rename accessor_flags to flags
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: russianneuromancer @ ya . ru, intel-gfx, dri-devel, Hans de Goede,
	linux-i2c, Mika Westerberg
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Rename accessor_flags to flags, so that we can use the field for
other flags too. This is a preparation patch for adding cherrytrail
support to the punit semaphore code.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
 drivers/i2c/busses/i2c-designware-core.c    | 14 +++++++-------
 drivers/i2c/busses/i2c-designware-core.h    |  2 +-
 drivers/i2c/busses/i2c-designware-platdrv.c |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 6d81c56..8c3ba42 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -177,13 +177,13 @@ static u32 dw_readl(struct dw_i2c_dev *dev, int offset)
 {
 	u32 value;
 
-	if (dev->accessor_flags & ACCESS_16BIT)
+	if (dev->flags & ACCESS_16BIT)
 		value = readw_relaxed(dev->base + offset) |
 			(readw_relaxed(dev->base + offset + 2) << 16);
 	else
 		value = readl_relaxed(dev->base + offset);
 
-	if (dev->accessor_flags & ACCESS_SWAP)
+	if (dev->flags & ACCESS_SWAP)
 		return swab32(value);
 	else
 		return value;
@@ -191,10 +191,10 @@ static u32 dw_readl(struct dw_i2c_dev *dev, int offset)
 
 static void dw_writel(struct dw_i2c_dev *dev, u32 b, int offset)
 {
-	if (dev->accessor_flags & ACCESS_SWAP)
+	if (dev->flags & ACCESS_SWAP)
 		b = swab32(b);
 
-	if (dev->accessor_flags & ACCESS_16BIT) {
+	if (dev->flags & ACCESS_16BIT) {
 		writew_relaxed((u16)b, dev->base + offset);
 		writew_relaxed((u16)(b >> 16), dev->base + offset + 2);
 	} else {
@@ -339,10 +339,10 @@ int i2c_dw_init(struct dw_i2c_dev *dev)
 	reg = dw_readl(dev, DW_IC_COMP_TYPE);
 	if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
 		/* Configure register endianess access */
-		dev->accessor_flags |= ACCESS_SWAP;
+		dev->flags |= ACCESS_SWAP;
 	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
 		/* Configure register access mode 16bit */
-		dev->accessor_flags |= ACCESS_16BIT;
+		dev->flags |= ACCESS_16BIT;
 	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
 		dev_err(dev->dev, "Unknown Synopsys component type: "
 			"0x%08x\n", reg);
@@ -926,7 +926,7 @@ 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(&dev->cmd_complete);
-	else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
+	else if (unlikely(dev->flags & ACCESS_INTR_MASK)) {
 		/* workaround to trigger pending interrupt */
 		stat = dw_readl(dev, DW_IC_INTR_MASK);
 		i2c_dw_disable_int(dev);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 26250b4..2c50571 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -103,7 +103,7 @@ struct dw_i2c_dev {
 	unsigned int		status;
 	u32			abort_source;
 	int			irq;
-	u32			accessor_flags;
+	u32			flags;
 	struct i2c_adapter	adapter;
 	u32			functionality;
 	u32			master_cfg;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 6ce4313..3eede7b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -112,7 +112,7 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 
 	id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
 	if (id && id->driver_data)
-		dev->accessor_flags |= (u32)id->driver_data;
+		dev->flags |= (u32)id->driver_data;
 
 	return 0;
 }
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCH v2 02/13] x86/platform/intel/iosf_mbi: Add a PMIC bus access notifier
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Takashi Iwai, russianneuromancer @ ya . ru, intel-gfx, dri-devel,
	Hans de Goede, linux-i2c, Mika Westerberg
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

Some drivers may need to acquire P-Unit managed resources from interrupt
context, where they cannot call iosf_mbi_punit_acquire().

This commit adds a notifier chain which allows a driver to get notified
(in a process context) before other drivers start accessing the PMIC bus,
so that the driver can acquire any resources, which it may need during
the window the other driver is accessing the PMIC, before hand.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: tagorereddy <tagore.chandan@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes in v2:
-Spelling: P-Unit, PMIC
-Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
---
 arch/x86/include/asm/iosf_mbi.h    | 54 ++++++++++++++++++++++++++++++++++++++
 arch/x86/platform/intel/iosf_mbi.c | 36 +++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h
index f6119d0..59e8f8b 100644
--- a/arch/x86/include/asm/iosf_mbi.h
+++ b/arch/x86/include/asm/iosf_mbi.h
@@ -47,6 +47,10 @@
 #define QRK_MBI_UNIT_MM		0x05
 #define QRK_MBI_UNIT_SOC	0x31
 
+/* Action values for the pmic_bus_access_notifier functions */
+#define MBI_PMIC_BUS_ACCESS_BEGIN	1
+#define MBI_PMIC_BUS_ACCESS_END		2
+
 #if IS_ENABLED(CONFIG_IOSF_MBI)
 
 bool iosf_mbi_available(void);
@@ -115,6 +119,38 @@ void iosf_mbi_punit_acquire(void);
  */
 void iosf_mbi_punit_release(void);
 
+/**
+ * iosf_mbi_register_pmic_bus_access_notifier - Register PMIC bus notifier
+ *
+ * This function can be used by drivers which may need to acquire P-Unit
+ * managed resources from interrupt context, where iosf_mbi_punit_acquire()
+ * can not be used.
+ *
+ * This function allows a driver to register a notifier to get notified (in a
+ * process context) before other drivers start accessing the PMIC bus.
+ *
+ * This allows the driver to acquire any resources, which it may need during
+ * the window the other driver is accessing the PMIC, before hand.
+ *
+ * @nb: notifier_block to register
+ */
+int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block *nb);
+
+/**
+ * iosf_mbi_register_pmic_bus_access_notifier - Unregister PMIC bus notifier
+ *
+ * @nb: notifier_block to unregister
+ */
+int iosf_mbi_unregister_pmic_bus_access_notifier(struct notifier_block *nb);
+
+/**
+ * iosf_mbi_call_pmic_bus_access_notifier_chain - Call PMIC bus notifier chain
+ *
+ * @val: action to pass into listener's notifier_call function
+ * @v: data pointer to pass into listener's notifier_call function
+ */
+int iosf_mbi_call_pmic_bus_access_notifier_chain(unsigned long val, void *v);
+
 #else /* CONFIG_IOSF_MBI is not enabled */
 static inline
 bool iosf_mbi_available(void)
@@ -146,6 +182,24 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
 static inline void iosf_mbi_punit_acquire(void) {}
 static inline void iosf_mbi_punit_release(void) {}
 
+static inline
+int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
+
+static inline
+int iosf_mbi_unregister_pmic_bus_access_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
+
+static inline
+int iosf_mbi_call_pmic_bus_access_notifier_chain(unsigned long val, void *v)
+{
+	return 0;
+}
+
 #endif /* CONFIG_IOSF_MBI */
 
 #endif /* IOSF_MBI_SYMS_H */
diff --git a/arch/x86/platform/intel/iosf_mbi.c b/arch/x86/platform/intel/iosf_mbi.c
index ed24fa9f..a952ac1 100644
--- a/arch/x86/platform/intel/iosf_mbi.c
+++ b/arch/x86/platform/intel/iosf_mbi.c
@@ -35,6 +35,7 @@
 static struct pci_dev *mbi_pdev;
 static DEFINE_SPINLOCK(iosf_mbi_lock);
 static DEFINE_MUTEX(iosf_mbi_punit_mutex);
+static BLOCKING_NOTIFIER_HEAD(iosf_mbi_pmic_bus_access_notifier);
 
 static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
 {
@@ -203,6 +204,41 @@ void iosf_mbi_punit_release(void)
 }
 EXPORT_SYMBOL(iosf_mbi_punit_release);
 
+int iosf_mbi_register_pmic_bus_access_notifier(struct notifier_block *nb)
+{
+	int ret;
+
+	/* Wait for the bus to go inactive before registering */
+	mutex_lock(&iosf_mbi_punit_mutex);
+	ret = blocking_notifier_chain_register(
+				&iosf_mbi_pmic_bus_access_notifier, nb);
+	mutex_unlock(&iosf_mbi_punit_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL(iosf_mbi_register_pmic_bus_access_notifier);
+
+int iosf_mbi_unregister_pmic_bus_access_notifier(struct notifier_block *nb)
+{
+	int ret;
+
+	/* Wait for the bus to go inactive before unregistering */
+	mutex_lock(&iosf_mbi_punit_mutex);
+	ret = blocking_notifier_chain_unregister(
+				&iosf_mbi_pmic_bus_access_notifier, nb);
+	mutex_unlock(&iosf_mbi_punit_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL(iosf_mbi_unregister_pmic_bus_access_notifier);
+
+int iosf_mbi_call_pmic_bus_access_notifier_chain(unsigned long val, void *v)
+{
+	return blocking_notifier_call_chain(
+				&iosf_mbi_pmic_bus_access_notifier, val, v);
+}
+EXPORT_SYMBOL(iosf_mbi_call_pmic_bus_access_notifier_chain);
+
 #ifdef CONFIG_IOSF_MBI_DEBUG
 static u32	dbg_mdr;
 static u32	dbg_mcr;
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [PATCH v2 01/13] x86/platform/intel/iosf_mbi: Add a mutex for P-Unit access
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: Takashi Iwai, russianneuromancer @ ya . ru, intel-gfx, dri-devel,
	Hans de Goede, linux-i2c, Mika Westerberg
In-Reply-To: <20170123210958.18410-1-hdegoede@redhat.com>

One some systems the P-Unit accesses the PMIC to change various voltages
through the same bus as other kernel drivers use for e.g. battery
monitoring.

If a driver sends requests to the P-Unit which require the P-Unit to access
the PMIC bus while another driver is also accessing the PMIC bus various
bad things happen.

This commit adds a mutex to protect the P-Unit against simultaneous
accesses and 2 functions to lock / unlock this mutex.

Note on these systems the i2c-bus driver will request a sempahore from the
P-Unit for exclusive access to the PMIC bus when i2c drivers are accessing
it, but this does not appear to be sufficient, we still need to avoid
making certain P-Unit requests during the access window to avoid problems.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: tagorereddy <tagore.chandan@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes in v2:
-Rename iosf_mbi_punit_lock/_unlock to _acquire/_release rename
-Spelling: P-Unit, PMIC
---
 arch/x86/include/asm/iosf_mbi.h    | 31 +++++++++++++++++++++++++++++++
 arch/x86/platform/intel/iosf_mbi.c | 13 +++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/arch/x86/include/asm/iosf_mbi.h b/arch/x86/include/asm/iosf_mbi.h
index b41ee16..f6119d0 100644
--- a/arch/x86/include/asm/iosf_mbi.h
+++ b/arch/x86/include/asm/iosf_mbi.h
@@ -88,6 +88,33 @@ int iosf_mbi_write(u8 port, u8 opcode, u32 offset, u32 mdr);
  */
 int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask);
 
+/**
+ * iosf_mbi_punit_acquire() - Acquire access to the P-Unit
+ *
+ * One some systems the P-Unit accesses the PMIC to change various voltages
+ * through the same bus as other kernel drivers use for e.g. battery monitoring.
+ *
+ * If a driver sends requests to the P-Unit which require the P-Unit to access
+ * the PMIC bus while another driver is also accessing the PMIC bus various bad
+ * things happen.
+ *
+ * To avoid these problems this function must be called before accessing the
+ * P-Unit or the PMIC, be it through iosf_mbi* functions or through other means.
+ *
+ * Note on these systems the i2c-bus driver will request a sempahore from the
+ * P-Unit for exclusive access to the PMIC bus when i2c drivers are accessing
+ * it, but this does not appear to be sufficient, we still need to avoid making
+ * certain P-Unit requests during the access window to avoid problems.
+ *
+ * This function locks a mutex, as such it may sleep.
+ */
+void iosf_mbi_punit_acquire(void);
+
+/**
+ * iosf_mbi_punit_release() - Release access to the P-Unit
+ */
+void iosf_mbi_punit_release(void);
+
 #else /* CONFIG_IOSF_MBI is not enabled */
 static inline
 bool iosf_mbi_available(void)
@@ -115,6 +142,10 @@ int iosf_mbi_modify(u8 port, u8 opcode, u32 offset, u32 mdr, u32 mask)
 	WARN(1, "IOSF_MBI driver not available");
 	return -EPERM;
 }
+
+static inline void iosf_mbi_punit_acquire(void) {}
+static inline void iosf_mbi_punit_release(void) {}
+
 #endif /* CONFIG_IOSF_MBI */
 
 #endif /* IOSF_MBI_SYMS_H */
diff --git a/arch/x86/platform/intel/iosf_mbi.c b/arch/x86/platform/intel/iosf_mbi.c
index edf2c54..ed24fa9f 100644
--- a/arch/x86/platform/intel/iosf_mbi.c
+++ b/arch/x86/platform/intel/iosf_mbi.c
@@ -34,6 +34,7 @@
 
 static struct pci_dev *mbi_pdev;
 static DEFINE_SPINLOCK(iosf_mbi_lock);
+static DEFINE_MUTEX(iosf_mbi_punit_mutex);
 
 static inline u32 iosf_mbi_form_mcr(u8 op, u8 port, u8 offset)
 {
@@ -190,6 +191,18 @@ bool iosf_mbi_available(void)
 }
 EXPORT_SYMBOL(iosf_mbi_available);
 
+void iosf_mbi_punit_acquire(void)
+{
+	mutex_lock(&iosf_mbi_punit_mutex);
+}
+EXPORT_SYMBOL(iosf_mbi_punit_acquire);
+
+void iosf_mbi_punit_release(void)
+{
+	mutex_unlock(&iosf_mbi_punit_mutex);
+}
+EXPORT_SYMBOL(iosf_mbi_punit_release);
+
 #ifdef CONFIG_IOSF_MBI_DEBUG
 static u32	dbg_mdr;
 static u32	dbg_mcr;
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [PATCH v2 00/13] coordinate cht i2c-pmic and i915-punit accesses
From: Hans de Goede @ 2017-01-23 21:09 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Ville Syrjälä,
	Jarkko Nikula, Wolfram Sang, Len Brown, Andy Shevchenko,
	Thomas Gleixner, H . Peter Anvin
  Cc: russianneuromancer @ ya . ru, intel-gfx, dri-devel, Hans de Goede,
	linux-i2c, Mika Westerberg

Hi All,

Here is v2 of my cht i2c-pmic and i915-punit access coordination
patchset. New in this version are some minor spelling / style fixes
and more importantly the inclusion of 6 extra i2c-designware patches.

These 6 patches are ready for merging, they have Wolfram's (the i2c
maintainer's) ack. The problem is the 6th i2c patch which fixes the
punit semaphore support in i2c-designware-baytrail.c, which fixes
the i2c-designware not binding to the pmic (axp288) i2c bus. But with
this fixed we actually start triggering the coordination problems the
rest of this patch-set addresses.

So the plan (again with Wolfram's ack) is for all these patches
including the i2c-designware ones to go upstream through the drm-intel
tree, to avoid an intermediate state were things don't work.

As for the how and why of this patchset, let me copy and paste the
cover letter of v1 of this set which is still valid:

This series fixes an issue where the following messages are shown in dmesg:
[drm:fw_domains_get [i915]] *ERROR* render: timed out waiting for forcewake ack request.
[drm:fw_domains_get [i915]] *MEDIA* render: timed out waiting for forcewake ack request.
i2c_designware 808622C1:06: punit semaphore timed out, resetting
i2c_designware 808622C1:06: PUNIT SEM: 2
i2c_designware 808622C1:06: couldn't acquire bus ownership

Usually followed by a gfx or system freeze, this is happening both on my
cherrytrail tablet as well as being seen by an user commenting on:
https://bugzilla.kernel.org/show_bug.cgi?id=155241

This problem is triggered by my patch series to fix the punit i2c bus
semaphore code in drivers/i2c/busses/designware-baytrail.c .

Which actually allows i2c access to pmic-s wich need bus-access arbritration
for the first time on cherrytrail, combined with patches from me to actually
make the axp288_fuel_gauge driver load, which leads to regular pmic i2c
accesses (when userspace checks the battery level).

The fix / workaround
====================

Testing has shown that just taking the punit i2c bus semaphore on
i2c accesses to a shared pmic i2c bus is not enough to avoid problem,
e.g. besides this series to coordinate i915 access, we also need:
https://patchwork.ozlabs.org/patch/710070/

This series introduces 2 mechanisms to coordinate direct pmic accesses
vs punit requests which lead to pmic accesses. The first mechanism is
a simple mutex, which works well for the direct punit accesses the i915
code does. However testing has shown this is not enough, we also need
to avoid doing a forcewake when a driver is directly accessing the pmic
i2c bus, otherwise we get the timeout errors quoted above, the fact
that after the forcewake errors the designware-baytrail.c code also
fails to acquire the lock, suggests that doing forcewakes during the
access window causes the i2c bus to get stuck (or the punit to get
confused).

The deal with the forcewake issue a notifier is introduced which allows
drivers to get notified before any pmic i2c transfer begins (and after
it ends). This is used in the i915 driver to do a
forcewake_get(FORCEWAKE_ALL) before the access begins avoiding needing to
it while the access is in progress.

Testing has shown that this avoids the problem for both me and the user,
where as before it could be reproduced at will.

The implementation of the fix
=============================

I've decided to put the mutex and the notifier in the iosf_mbi code,
as that is somewhat of a common middle ground between the i915 driver and
the designware-baytrail.c code, with the latter already depending on the
iosf_mbi code. I'm open for moving them if someone has a suggestion for a
better place to put them.

If iosf_mbi support is not enabled then all the functions used will become
static inline NOPs and nothing changes on the i915 side.

If the code is enabled and runs on a system which does not need pmic i2c
bus arbritration, then the mutex lock / unlock calls added to the i915 code
will still be made, but there won't be any contention, so other then the
overhead of the lock / unlock there will not be any delays. These calls are
not made in any hot paths.

The biggest downside IMHO is that taking FORCEWAKE_ALL before each bus
access will cause extra GPU wakeups, and thus powerdrain. I plan to make
the impact of this minimal by modifying any drivers (axp288 code, with
which I'm quite familiar by now) to limit their i2c accesses to a bare
minimum. Specifically I plan to e.g. cache the battery level and wait
at least 15 seconds (max 4 times a minute) before getting a fresh reading
even if userspace asks for it more often, combined with some mechanism
to only do 1 access-begin and 1 access-end notification for i2c accesses
in quick succession.

Alternatives
============

It would be tempting to say that this is not worth the trouble and we
should just disallow direct i2c accesses to the pmic bus. Unfortunately
that will cause some major issues on affected devices:
-No battery monitoring
-No "AC" plugged in monitoring
-If booted with a normal USB-A -> micro-USB cable, or no cable, plugged
 in and then the user replaces the cable with an otg USB-host cable /
 adapter, the id-pin shorting will enable a 5v boost convertor, but we
 need to disable the pmic's USB-Vbus path otherwise it will start drawing
 current from the boost convertor, leading to aprox 300mA of extra
 battery drain, this is done by the axp288_charger driver, which needs
 direct i2c access to the pmic bus

Way forward
===========

I realize the workaround this patch-set adds is not the prettiest code
ever seen, but I see no otherway to get things like battery monitoring
and proper OTG support working on affected devices without breaking
drm/i915 support. So I would like to see this merged in some form, I'm
happy to make any kind of requested changes; and/or to try alternative
fixes.

Regards,

Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v2 1/3] dt: bindings: add documentation for zx2967 family i2c controller
From: Rob Herring @ 2017-01-23 20:52 UTC (permalink / raw)
  To: Baoyou Xie
  Cc: andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
	jun.nie-QSEj5FYQhm4dnm+yROfE0A, wsa-z923LK4zBo2bacvFa/9K2g,
	mark.rutland-5wv7dgnIgG8,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
	chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
	wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
In-Reply-To: <1485147806-17792-1-git-send-email-baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Mon, Jan 23, 2017 at 01:03:24PM +0800, Baoyou Xie wrote:
> This patch adds dt-binding documentation for zx2967 family
> i2c controller.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  .../devicetree/bindings/i2c/i2c-zx2967.txt         | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-zx2967.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 0/3] Init device ids from ACPI of_compatible
From: Andy Shevchenko @ 2017-01-23 17:13 UTC (permalink / raw)
  To: Dan O'Donovan
  Cc: linux-acpi@vger.kernel.org, Rafael J . Wysocki, Jarkko Nikula,
	Mika Westerberg, Mark Brown, Len Brown, linux-i2c, Wolfram Sang,
	linux-spi, linux-kernel@vger.kernel.org
In-Reply-To: <1485187737-22414-1-git-send-email-dan@emutex.com>

On Mon, Jan 23, 2017 at 6:08 PM, Dan O'Donovan <dan@emutex.com> wrote:
> (Re-submitting this patch set originally created by Leonard Crestez,
> possibly abandoned by Leonard due to a change in employment)
>
> When using devicetree, stuff like i2c_client.name or spi_device.modalias
> is initialized to the first DT compatible id with the vendor prefix
> stripped. Since some drivers rely on this in order to differentiate between
> hardware variants try to replicate it when using ACPI with DT ids.
>
> This also makes it so that the i2c_device_id parameter passed to probe is
> non-NULL when matching with ACPI and DT ids.
>
> Tested using ACPI overlays but there is no actual dependency. This series
> just extends the PRP0001 feature to be more useful for I2C/SPI.
>
> The patches only touches the ACPI-specific parts of the i2c and spi core.

The series looks good. Few nitpicks in the comments of corresponding
patches though.
FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
> Here is an example .dsl for an SPI accelerometer connected to minnowboard max:
>
> Device (ACCL)
> {
>     Name (_ADR, Zero)
>     Name (_HID, "PRP0001")
>     Name (_UID, One)
>
>     Method (_CRS, 0, Serialized)
>     {
>         Name (RBUF, ResourceTemplate ()
>         {
>             SPISerialBus(1, PolarityLow, FourWireMode, 16,
>                     ControllerInitiated, 1000000, ClockPolarityLow,
>                     ClockPhaseFirst, "\\_SB.SPI1",)
>             GpioInt (Edge, ActiveHigh, Exclusive, PullDown, 0x0000,
>                      "\\_SB.GPO2", 0x00, ResourceConsumer, , )
>             { // Pin list
>                     1
>             }
>         })
>         Return (RBUF)
>     }
>     Name (_DSD, Package ()
>     {
>         ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>         Package ()
>         {
>             Package () {"compatible", "st,lis3dh"},
>         }
>     })
> }
>
> Link to v2: https://lkml.org/lkml/2016/7/13/392
> Changes:
>  * Use appropriate subject prefix for each subsystem (Mark Brown)
>  * Use ACPI info as before if getting OF info fails (Mark Brown)
>  * Minor cosmetic/readability improvements (Rafael J. Wysocki)
>
> Link to v1: https://www.spinics.net/lists/linux-acpi/msg66469.html
> Changes:
>  * Rebase on after acpi overlays got it.
>  * Change acpi_of_modalias outlen param to size_t
>  * Use {} after else
>
> Crestez Dan Leonard (3):
>   ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
>   i2c: acpi: Initialize info.type from of_compatible
>   spi: acpi: Initialize modalias from of_compatible
>
>  drivers/acpi/bus.c      | 35 +++++++++++++++++++++++++++++++++++
>  drivers/i2c/i2c-core.c  |  8 +++++++-
>  drivers/spi/spi.c       | 10 +++++++++-
>  include/acpi/acpi_bus.h |  1 +
>  4 files changed, 52 insertions(+), 2 deletions(-)
>
> --
> 2.7.4
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible
From: Andy Shevchenko @ 2017-01-23 17:11 UTC (permalink / raw)
  To: Dan O'Donovan
  Cc: linux-acpi@vger.kernel.org, Rafael J . Wysocki, Jarkko Nikula,
	Mika Westerberg, Mark Brown, Len Brown, linux-i2c, Wolfram Sang,
	linux-spi, linux-kernel@vger.kernel.org, Crestez Dan Leonard
In-Reply-To: <1485187737-22414-4-git-send-email-dan@emutex.com>

On Mon, Jan 23, 2017 at 6:08 PM, Dan O'Donovan <dan@emutex.com> wrote:
> From: Crestez Dan Leonard <leonard.crestez@intel.com>
>
> When using devicetree spi_device.modalias is set to the compatible
> string with the vendor prefix removed. For SPI devices described via
> ACPI the spi_device.modalias string is initialized by acpi_device_hid.
> When using ACPI and DT ids this string ends up something like "PRP0001".
>
> Change acpi_register_spi_device to use the of_compatible property if
> present. This makes it easier to instantiate spi drivers through ACPI
> with DT ids.

> +       /*
> +        * Populate modalias from compatible property if available,
> +        * otherwise use native ACPI information
> +        */
> +       if ((!adev->data.of_compatible) ||
> +           acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))

Same comment as in patch 2.

> +               strlcpy(spi->modalias, acpi_device_hid(adev),
> +                       sizeof(spi->modalias));

Could this be one line?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v3 2/3] i2c: acpi: Initialize info.type from of_compatible
From: Andy Shevchenko @ 2017-01-23 17:10 UTC (permalink / raw)
  To: Dan O'Donovan
  Cc: linux-acpi@vger.kernel.org, Rafael J . Wysocki, Jarkko Nikula,
	Mika Westerberg, Mark Brown, Len Brown, linux-i2c, Wolfram Sang,
	linux-spi, linux-kernel@vger.kernel.org, Crestez Dan Leonard
In-Reply-To: <1485187737-22414-3-git-send-email-dan@emutex.com>

On Mon, Jan 23, 2017 at 6:08 PM, Dan O'Donovan <dan@emutex.com> wrote:
> From: Crestez Dan Leonard <leonard.crestez@intel.com>
>
> When using devicetree i2c_board_info.type is set to the compatible
> string with the vendor prefix removed. For I2C devices described via
> ACPI the i2c_board_info.type string is set to the ACPI device name. When
> using ACPI and DT ids this string ends up something like "PRP0001:00".
>
> If the of_compatible property is present try to use that instead. This
> makes it easier to instantiate i2c drivers through ACPI with DT ids.

> -       strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
> +       /*
> +        * Populate modalias from compatible property if available,
> +        * otherwise use native ACPI information
> +        */
> +       if ((!adev->data.of_compatible) ||
> +           acpi_of_modalias(adev, info->type, sizeof(info->type)))

Redundant parens in lvalue. After removing them is it possible to use
one line for it?

> +               strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* [PATCH v3 3/3] spi: acpi: Initialize modalias from of_compatible
From: Dan O'Donovan @ 2017-01-23 16:08 UTC (permalink / raw)
  To: linux-acpi-u79uwXL29TY76Z2rM5mHXA, Rafael J . Wysocki,
	Jarkko Nikula, Mika Westerberg, Mark Brown
  Cc: Len Brown, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Dan O'Donovan,
	Crestez Dan Leonard
In-Reply-To: <1485187737-22414-1-git-send-email-dan-M3NBUjLqch7QT0dZR+AlfA@public.gmane.org>

From: Crestez Dan Leonard <leonard.crestez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

When using devicetree spi_device.modalias is set to the compatible
string with the vendor prefix removed. For SPI devices described via
ACPI the spi_device.modalias string is initialized by acpi_device_hid.
When using ACPI and DT ids this string ends up something like "PRP0001".

Change acpi_register_spi_device to use the of_compatible property if
present. This makes it easier to instantiate spi drivers through ACPI
with DT ids.

Signed-off-by: Crestez Dan Leonard <leonard.crestez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dan O'Donovan <dan-M3NBUjLqch7QT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 656dd3e..4b562e8 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1722,13 +1722,21 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
 		return AE_OK;
 	}
 
+	/*
+	 * Populate modalias from compatible property if available,
+	 * otherwise use native ACPI information
+	 */
+	if ((!adev->data.of_compatible) ||
+	    acpi_of_modalias(adev, spi->modalias, sizeof(spi->modalias)))
+		strlcpy(spi->modalias, acpi_device_hid(adev),
+			sizeof(spi->modalias));
+
 	if (spi->irq < 0)
 		spi->irq = acpi_dev_gpio_irq_get(adev, 0);
 
 	acpi_device_set_enumerated(adev);
 
 	adev->power.flags.ignore_parent = true;
-	strlcpy(spi->modalias, acpi_device_hid(adev), sizeof(spi->modalias));
 	if (spi_add_device(spi)) {
 		adev->power.flags.ignore_parent = false;
 		dev_err(&master->dev, "failed to add SPI device %s from ACPI\n",
-- 
2.7.4

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

^ permalink raw reply related

* [PATCH v3 2/3] i2c: acpi: Initialize info.type from of_compatible
From: Dan O'Donovan @ 2017-01-23 16:08 UTC (permalink / raw)
  To: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
	Mark Brown
  Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel,
	Dan O'Donovan, Crestez Dan Leonard
In-Reply-To: <1485187737-22414-1-git-send-email-dan@emutex.com>

From: Crestez Dan Leonard <leonard.crestez@intel.com>

When using devicetree i2c_board_info.type is set to the compatible
string with the vendor prefix removed. For I2C devices described via
ACPI the i2c_board_info.type string is set to the ACPI device name. When
using ACPI and DT ids this string ends up something like "PRP0001:00".

If the of_compatible property is present try to use that instead. This
makes it easier to instantiate i2c drivers through ACPI with DT ids.

Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
Signed-off-by: Dan O'Donovan <dan@emutex.com>
---
 drivers/i2c/i2c-core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 583e950..a83dbc7 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -221,7 +221,13 @@ static int i2c_acpi_get_info(struct acpi_device *adev,
 
 	acpi_dev_free_resource_list(&resource_list);
 
-	strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+	/*
+	 * Populate modalias from compatible property if available,
+	 * otherwise use native ACPI information
+	 */
+	if ((!adev->data.of_compatible) ||
+	    acpi_of_modalias(adev, info->type, sizeof(info->type)))
+		strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
 
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox