Re: [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume
On 21.06.2014 01:53, Doug Anderson wrote: > Kevin, > > On Fri, Jun 20, 2014 at 4:13 PM, Kevin Hilman wrote: >> Doug Anderson writes: >> >>> Kevin, >>> >>> On Fri, Jun 20, 2014 at 2:48 PM, Kevin Hilman wrote: Hi Doug, Doug Anderson writes: > On Thu, Jun 19, 2014 at 11:43 AM, Kevin Hilman wrote: >> Doug Anderson writes: >> >>> The original code for the exynos i2c controller registered for the >>> "noirq" variants. However during review feedback it was moved to >>> SIMPLE_DEV_PM_OPS without anyone noticing that it meant we were no >>> longer actually "noirq" (despite functions named >>> exynos5_i2c_suspend_noirq and exynos5_i2c_resume_noirq). >>> >>> i2c controllers that might have wakeup sources on them seem to need to >>> resume at noirq time so that the individual drivers can actually read >>> the i2c bus to handle their wakeup. >> >> I suspect usage of the noirq variants pre-dates the existence of the >> late/early callbacks in the PM core, but based on the description above, >> I suspect what you actually want is the late/early callbacks. > > I think it actually really needs noirq. ;) Yes, it appears it does. Objection withdrawn. I just wanted to be sure because since the introduction of late/early, the need for noirq should be pretty rare, but there certainly are needs. In this case though, the need for it has more to do with the lack of a way for us to describe non parent-child device dependencies than whether or not IRQs are enabled or not. >>> >>> Actually, I'm not sure that's true, but I'll talk through it and you >>> can point to where I'm wrong (I often am!) >>> >>> If you're a wakeup device then you need to be ready to handle >>> interrupts as soon as the "noirq" phase of resume is done, right? >> >> As soon as the noirq phase of your own driver is done, correct. >> >>> Said another way: you need to be ready to handle interrupts _before_ >>> the normal resume code is called and be ready to handle interrupts >>> even _before_ the early resume code is called. >> >> Correct. >> >>> That means if you are implementing a bus that's needed by any devices >>> with wakeup interrupts then it's your responsibility to also be >>> prepared to run this early. >>> >>> In this particular case the max77686 driver doesn't need to do >>> anything at all to be ready to handle interrupts. It's suspend and >>> resume code is just boilerplate "enable wakeups / disable wakeups" and >>> it has no "noirq" code. The max77686 driver doesn't have any "noirq" >>> wake call because it would just be empty. >>> >>> Said another way: the problem isn't that the max77686 wakeup gets >>> called before the i2c wakeup. The problem is that i2c is needed ASAP >>> once IRQs are enabled and thus needs to be run noirq. >>> >>> Does that sound semi-correct? >> >> Yes that's correct. >> >> My point above was (trying to be) that ultimately this is an ordering >> issue. e.g. the bus device needs to be "ready" before wakeup devices on >> that bus can handle wakeup interrupts etc. The way we're handling that >> ordering is by the implied ordering of noirq, late/early and "normal" >> callbacks. That's convenient, but not exactly obvious. >> >> It works because we dont' typically need too many layers here, but it >> would be much more understandable if we could describe this kind of >> dependency in a way that the suspend/resume code would suspend/resume >> things in the right order rather than by tinkering with callback levels >> (since otherwise suspend/resume ordering just depends on probe order.) >> >> This issue then usually gets me headed down my usual rant path about how >> I think runtime PM is much better suited for handling ordering and >> dependencies becuase it automatically handles parent/child dependencies >> and non parent/child dependencies can be handled by taking advantage of >> the get/put APIs which are refcounted, ect etc. but that's another can >> worms. > > Ah, I gotcha. Yes, I'm a fan of having explicit dependency orderings too. > > So I guess in this case the truly correct way to handle it is: > > 1. i2c controller should have Runtime PM even though (as per the code > now) there's nothing you can do to it to save power under normal > circumstances. So the runtime "suspend" code would be a no-op. > > 2. When the i2c controller is told to runtime resume, it should > double-check if a full SoC poweroff has happened since the last time > it checked. In this case it should reinit its hardware. > > 3. If the i2c controller gets a full "resume" callback then it should > also reinit the hardware just so it's not sitting in a half-configured > state until the first peripheral uses it. > > If later someone finds a way to power gate the i2c controller when no > active transfers are going (and we actually save non-trivial power > doing this) then we've got a nice
Re: [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume
Kevin, On Fri, Jun 20, 2014 at 4:13 PM, Kevin Hilman wrote: > Doug Anderson writes: > >> Kevin, >> >> On Fri, Jun 20, 2014 at 2:48 PM, Kevin Hilman wrote: >>> Hi Doug, >>> >>> Doug Anderson writes: >>> On Thu, Jun 19, 2014 at 11:43 AM, Kevin Hilman wrote: > Doug Anderson writes: > >> The original code for the exynos i2c controller registered for the >> "noirq" variants. However during review feedback it was moved to >> SIMPLE_DEV_PM_OPS without anyone noticing that it meant we were no >> longer actually "noirq" (despite functions named >> exynos5_i2c_suspend_noirq and exynos5_i2c_resume_noirq). >> >> i2c controllers that might have wakeup sources on them seem to need to >> resume at noirq time so that the individual drivers can actually read >> the i2c bus to handle their wakeup. > > I suspect usage of the noirq variants pre-dates the existence of the > late/early callbacks in the PM core, but based on the description above, > I suspect what you actually want is the late/early callbacks. I think it actually really needs noirq. ;) >>> >>> Yes, it appears it does. Objection withdrawn. >>> >>> I just wanted to be sure because since the introduction of late/early, >>> the need for noirq should be pretty rare, but there certainly are needs. >>> >>> >>> In this case though, the need for it has more to do with the >>> lack of a way for us to describe non parent-child device dependencies >>> than whether or not IRQs are enabled or not. >>> >> >> Actually, I'm not sure that's true, but I'll talk through it and you >> can point to where I'm wrong (I often am!) >> >> If you're a wakeup device then you need to be ready to handle >> interrupts as soon as the "noirq" phase of resume is done, right? > > As soon as the noirq phase of your own driver is done, correct. > >> Said another way: you need to be ready to handle interrupts _before_ >> the normal resume code is called and be ready to handle interrupts >> even _before_ the early resume code is called. > > Correct. > >> That means if you are implementing a bus that's needed by any devices >> with wakeup interrupts then it's your responsibility to also be >> prepared to run this early. >> >> In this particular case the max77686 driver doesn't need to do >> anything at all to be ready to handle interrupts. It's suspend and >> resume code is just boilerplate "enable wakeups / disable wakeups" and >> it has no "noirq" code. The max77686 driver doesn't have any "noirq" >> wake call because it would just be empty. >> >> Said another way: the problem isn't that the max77686 wakeup gets >> called before the i2c wakeup. The problem is that i2c is needed ASAP >> once IRQs are enabled and thus needs to be run noirq. >> >> Does that sound semi-correct? > > Yes that's correct. > > My point above was (trying to be) that ultimately this is an ordering > issue. e.g. the bus device needs to be "ready" before wakeup devices on > that bus can handle wakeup interrupts etc. The way we're handling that > ordering is by the implied ordering of noirq, late/early and "normal" > callbacks. That's convenient, but not exactly obvious. > > It works because we dont' typically need too many layers here, but it > would be much more understandable if we could describe this kind of > dependency in a way that the suspend/resume code would suspend/resume > things in the right order rather than by tinkering with callback levels > (since otherwise suspend/resume ordering just depends on probe order.) > > This issue then usually gets me headed down my usual rant path about how > I think runtime PM is much better suited for handling ordering and > dependencies becuase it automatically handles parent/child dependencies > and non parent/child dependencies can be handled by taking advantage of > the get/put APIs which are refcounted, ect etc. but that's another can > worms. Ah, I gotcha. Yes, I'm a fan of having explicit dependency orderings too. So I guess in this case the truly correct way to handle it is: 1. i2c controller should have Runtime PM even though (as per the code now) there's nothing you can do to it to save power under normal circumstances. So the runtime "suspend" code would be a no-op. 2. When the i2c controller is told to runtime resume, it should double-check if a full SoC poweroff has happened since the last time it checked. In this case it should reinit its hardware. 3. If the i2c controller gets a full "resume" callback then it should also reinit the hardware just so it's not sitting in a half-configured state until the first peripheral uses it. If later someone finds a way to power gate the i2c controller when no active transfers are going (and we actually save non-trivial power doing this) then we've got a nice place to put that code. NOTE: Unless we can actually save power by power gating the i2c peripheral when there are no active transfers, we would also just have the i2c_xfer() i
Re: [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume
Doug Anderson writes: > Kevin, > > On Fri, Jun 20, 2014 at 2:48 PM, Kevin Hilman wrote: >> Hi Doug, >> >> Doug Anderson writes: >> >>> On Thu, Jun 19, 2014 at 11:43 AM, Kevin Hilman wrote: Doug Anderson writes: > The original code for the exynos i2c controller registered for the > "noirq" variants. However during review feedback it was moved to > SIMPLE_DEV_PM_OPS without anyone noticing that it meant we were no > longer actually "noirq" (despite functions named > exynos5_i2c_suspend_noirq and exynos5_i2c_resume_noirq). > > i2c controllers that might have wakeup sources on them seem to need to > resume at noirq time so that the individual drivers can actually read > the i2c bus to handle their wakeup. I suspect usage of the noirq variants pre-dates the existence of the late/early callbacks in the PM core, but based on the description above, I suspect what you actually want is the late/early callbacks. >>> >>> I think it actually really needs noirq. ;) >> >> Yes, it appears it does. Objection withdrawn. >> >> I just wanted to be sure because since the introduction of late/early, >> the need for noirq should be pretty rare, but there certainly are needs. >> >> >> In this case though, the need for it has more to do with the >> lack of a way for us to describe non parent-child device dependencies >> than whether or not IRQs are enabled or not. >> > > Actually, I'm not sure that's true, but I'll talk through it and you > can point to where I'm wrong (I often am!) > > If you're a wakeup device then you need to be ready to handle > interrupts as soon as the "noirq" phase of resume is done, right? As soon as the noirq phase of your own driver is done, correct. > Said another way: you need to be ready to handle interrupts _before_ > the normal resume code is called and be ready to handle interrupts > even _before_ the early resume code is called. Correct. > That means if you are implementing a bus that's needed by any devices > with wakeup interrupts then it's your responsibility to also be > prepared to run this early. > > In this particular case the max77686 driver doesn't need to do > anything at all to be ready to handle interrupts. It's suspend and > resume code is just boilerplate "enable wakeups / disable wakeups" and > it has no "noirq" code. The max77686 driver doesn't have any "noirq" > wake call because it would just be empty. > > Said another way: the problem isn't that the max77686 wakeup gets > called before the i2c wakeup. The problem is that i2c is needed ASAP > once IRQs are enabled and thus needs to be run noirq. > > Does that sound semi-correct? Yes that's correct. My point above was (trying to be) that ultimately this is an ordering issue. e.g. the bus device needs to be "ready" before wakeup devices on that bus can handle wakeup interrupts etc. The way we're handling that ordering is by the implied ordering of noirq, late/early and "normal" callbacks. That's convenient, but not exactly obvious. It works because we dont' typically need too many layers here, but it would be much more understandable if we could describe this kind of dependency in a way that the suspend/resume code would suspend/resume things in the right order rather than by tinkering with callback levels (since otherwise suspend/resume ordering just depends on probe order.) This issue then usually gets me headed down my usual rant path about how I think runtime PM is much better suited for handling ordering and dependencies becuase it automatically handles parent/child dependencies and non parent/child dependencies can be handled by taking advantage of the get/put APIs which are refcounted, ect etc. but that's another can worms. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume
Kevin, On Fri, Jun 20, 2014 at 2:48 PM, Kevin Hilman wrote: > Hi Doug, > > Doug Anderson writes: > >> On Thu, Jun 19, 2014 at 11:43 AM, Kevin Hilman wrote: >>> Doug Anderson writes: >>> The original code for the exynos i2c controller registered for the "noirq" variants. However during review feedback it was moved to SIMPLE_DEV_PM_OPS without anyone noticing that it meant we were no longer actually "noirq" (despite functions named exynos5_i2c_suspend_noirq and exynos5_i2c_resume_noirq). i2c controllers that might have wakeup sources on them seem to need to resume at noirq time so that the individual drivers can actually read the i2c bus to handle their wakeup. >>> >>> I suspect usage of the noirq variants pre-dates the existence of the >>> late/early callbacks in the PM core, but based on the description above, >>> I suspect what you actually want is the late/early callbacks. >> >> I think it actually really needs noirq. ;) > > Yes, it appears it does. Objection withdrawn. > > I just wanted to be sure because since the introduction of late/early, > the need for noirq should be pretty rare, but there certainly are needs. > > > In this case though, the need for it has more to do with the > lack of a way for us to describe non parent-child device dependencies > than whether or not IRQs are enabled or not. > Actually, I'm not sure that's true, but I'll talk through it and you can point to where I'm wrong (I often am!) If you're a wakeup device then you need to be ready to handle interrupts as soon as the "noirq" phase of resume is done, right? Said another way: you need to be ready to handle interrupts _before_ the normal resume code is called and be ready to handle interrupts even _before_ the early resume code is called. That means if you are implementing a bus that's needed by any devices with wakeup interrupts then it's your responsibility to also be prepared to run this early. In this particular case the max77686 driver doesn't need to do anything at all to be ready to handle interrupts. It's suspend and resume code is just boilerplate "enable wakeups / disable wakeups" and it has no "noirq" code. The max77686 driver doesn't have any "noirq" wake call because it would just be empty. Said another way: the problem isn't that the max77686 wakeup gets called before the i2c wakeup. The problem is that i2c is needed ASAP once IRQs are enabled and thus needs to be run noirq. Does that sound semi-correct? -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume
Hi Doug, Doug Anderson writes: > On Thu, Jun 19, 2014 at 11:43 AM, Kevin Hilman wrote: >> Doug Anderson writes: >> >>> The original code for the exynos i2c controller registered for the >>> "noirq" variants. However during review feedback it was moved to >>> SIMPLE_DEV_PM_OPS without anyone noticing that it meant we were no >>> longer actually "noirq" (despite functions named >>> exynos5_i2c_suspend_noirq and exynos5_i2c_resume_noirq). >>> >>> i2c controllers that might have wakeup sources on them seem to need to >>> resume at noirq time so that the individual drivers can actually read >>> the i2c bus to handle their wakeup. >> >> I suspect usage of the noirq variants pre-dates the existence of the >> late/early callbacks in the PM core, but based on the description above, >> I suspect what you actually want is the late/early callbacks. > > I think it actually really needs noirq. ;) Yes, it appears it does. Objection withdrawn. I just wanted to be sure because since the introduction of late/early, the need for noirq should be pretty rare, but there certainly are needs. In this case though, the need for it has more to do with the lack of a way for us to describe non parent-child device dependencies than whether or not IRQs are enabled or not. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Driver-Support of I2C_M_RECV_LEN
Hello, in the documentation i found at http://ww2.cs.fsu.edu/~rosentha/linux/2.6.26.5/docs/DocBook/kernel-api/re1222.html the following sentence: "Note that using this function requires that the client's adapter support the I2C_FUNC_SMBUS_READ_BLOCK_DATA functionality. Not all adapter drivers support this; its emulation through I2C messaging relies on a specific mechanism (I2C_M_RECV_LEN) which may not be implemented." The second sentence says that the mechanism I2C_M_RECV_LEN is not all over implemented. How i can figure out whether this mechanism will be provided of my driver? What is the alternative of using the function i2c_smbus_read_block_data? Thanks a lot for your support, Michael Post -- 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/9] i2c: Add the ability to match device to compatible string without an of_node
A great deal of I2C devices are currently matched via DT node name, and
as such the compatible naming convention of ',' has gone
somewhat awry - some nodes don't supply one, some supply an arbitrary
string and others the correct device name with an arbitrary vendor prefix.
In an effort to correct this problem we have to supply a mechanism to
match a device by compatible string AND by simple device name. This
function strips off the ',' part of a supplied compatible string
and attempts to match without it.
The plan is to remove this function once all of the compatible strings
for each device have been brought into line.
Acked-by: Grant Likely
Signed-off-by: Lee Jones
---
drivers/i2c/i2c-core.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d3802dc..8a37745 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1090,6 +1090,27 @@ struct i2c_adapter *of_find_i2c_adapter_by_node(struct
device_node *node)
return i2c_verify_adapter(dev);
}
EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
+
+static const struct of_device_id*
+i2c_of_match_device_strip_vendor(const struct of_device_id *matches,
+ struct i2c_client *client)
+{
+ const char *name;
+
+ for (; matches->compatible[0]; matches++) {
+ name = strchr(matches->compatible, ',');
+ if (!name)
+ name = matches->compatible;
+ else
+ name++;
+
+ if (!strnicmp(client->name, name, strlen(client->name)))
+ return matches;
+ }
+
+ return NULL;
+}
+
#else
static void of_i2c_register_devices(struct i2c_adapter *adap) { }
#endif /* CONFIG_OF */
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/7] i2c: Relax mandatory I2C ID table passing
Hi Wolfram, As previously discussed I believe it should be okay for an I2C device driver _not_ supply an I2C ID table to match to. The I2C subsystem should be able to match via other means, such as via OF tables. The blocking factor during our previous conversation was to keep registering via sysfs up and running. This set does that. After thinking more deeply about the problem, it occurred to me that any I2C device driver which uses the sysfs method and issues an of_match_device() would also fail their probe(). Bolted on to this set is a new, more generic way for these devices to match against either of the I2C/OF tables. I hope this ticks all of your boxes. v3: - Insist on passing 'struct i2c_client' instead of 'struct device' - Remove hook from of_match_device() v2: - Removal of ACPI support (this is really an OF issue). - Add a new .probe2( with will seamlessly replace - Supply a warning on devices matching via OF without a suitable compatible - Remove unified match_device() - bad idea as it subverts type-safe behaviour - Provide examples of the kind of clean-up possible after this set. - I already have the full support from the maintainer of these drivers =;-) Kind regards, Lee arch/arm/configs/multi_v7_defconfig | 1 + drivers/i2c/i2c-core.c | 72 +++- drivers/mfd/88pm860x-core.c | 5 ++--- drivers/mfd/as3722.c| 12 ++-- include/linux/i2c.h | 22 +- 5 files changed, 89 insertions(+), 23 deletions(-) -- 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/9] ARM: multi_v7_defconfig: Enable ST's I2C driver
Signed-off-by: Lee Jones --- arch/arm/configs/multi_v7_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig index 91af0a9..57335e1 100644 --- a/arch/arm/configs/multi_v7_defconfig +++ b/arch/arm/configs/multi_v7_defconfig @@ -200,6 +200,7 @@ CONFIG_I2C_EXYNOS5=y CONFIG_I2C_MV64XXX=y CONFIG_I2C_SIRF=y CONFIG_I2C_TEGRA=y +CONFIG_I2C_ST=y CONFIG_SPI=y CONFIG_SPI_OMAP24XX=y CONFIG_SPI_ORION=y -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gpio: crystalcove: Fix implicit declaration of function 'seq_printf' error
drivers/gpio/gpio-crystalcove.c: In function 'crystalcove_gpio_dbg_show': drivers/gpio/gpio-crystalcove.c:286:3: error: implicit declaration of function 'seq_printf' seq_printf(s, " gpio-%-2d %s %s %s %s ctlo=%2x,%s %s %s\n", Reported-by: Stephen Rothwell Signed-off-by: Lee Jones --- drivers/gpio/gpio-crystalcove.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-crystalcove.c index 5a98499..934462f 100644 --- a/drivers/gpio/gpio-crystalcove.c +++ b/drivers/gpio/gpio-crystalcove.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] i2c: Add pointer dereference protection to i2c_match_id()
Here we're providing dereference protection for i2c_match_id(), which
saves us having to do it each time it's called. We're also stripping
out the (now) needless checks in i2c_device_match(). This patch paves
the way for other, similar code trimming.
Acked-by: Grant Likely
Signed-off-by: Lee Jones
---
drivers/i2c/i2c-core.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 7c7f4b8..d3802dc 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -82,6 +82,9 @@ void i2c_transfer_trace_unreg(void)
static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
const struct i2c_client *client)
{
+ if (!(id && client))
+ return NULL;
+
while (id->name[0]) {
if (strcmp(client->name, id->name) == 0)
return id;
@@ -95,8 +98,6 @@ static int i2c_device_match(struct device *dev, struct
device_driver *drv)
struct i2c_client *client = i2c_verify_client(dev);
struct i2c_driver *driver;
- if (!client)
- return 0;
/* Attempt an OF style match */
if (of_driver_match_device(dev, drv))
@@ -107,9 +108,10 @@ static int i2c_device_match(struct device *dev, struct
device_driver *drv)
return 1;
driver = to_i2c_driver(drv);
- /* match on an id table if there is one */
- if (driver->id_table)
- return i2c_match_id(driver->id_table, client) != NULL;
+
+ /* Finally an I2C match */
+ if (i2c_match_id(driver->id_table, client))
+ return 1;
return 0;
}
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] i2c: Make I2C ID tables non-mandatory for DT'ed devices
Currently the I2C framework insists on devices supplying an I2C ID table. Many of the devices which do so unnecessarily adding quite a few wasted lines to kernel code. This patch allows drivers a means to 'not' supply the aforementioned table and match on DT match tables instead. Acked-by: Grant Likely Signed-off-by: Lee Jones --- drivers/i2c/i2c-core.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index a9c97c4..ff2919a 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -100,7 +100,7 @@ static int i2c_device_match(struct device *dev, struct device_driver *drv) /* Attempt an OF style match */ - if (of_driver_match_device(dev, drv)) + if (i2c_of_match_device(drv->of_match_table, client)) return 1; /* Then ACPI style match */ @@ -268,7 +268,15 @@ static int i2c_device_probe(struct device *dev) return 0; driver = to_i2c_driver(dev->driver); - if (!driver->probe || !driver->id_table) + if (!driver->probe) + return -EINVAL; + + /* +* An I2C ID table is not mandatory, if and only if, a suitable Device +* Tree match table entry is supplied for the probing device. +*/ + if (!driver->id_table && + !i2c_of_match_device(dev->driver->of_match_table, client)) return -ENODEV; if (!device_can_wakeup(&client->dev)) -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] i2c: Provide a temporary .probe2() call-back type
This will aid the seamless removal of the current probe()'s, more
commonly unused than used second parameter. Most I2C drivers can
simply switch over to the new interface, others which have DT
support can use its own matching instead and others can call
i2c_match_id() themselves. This brings I2C's device probe method
into line with other similar interfaces in the kernel and prevents
the requirement to pass an i2c_device_id table.
Suggested-by: Grant Likely
Signed-off-by: Lee Jones
---
drivers/i2c/i2c-core.c | 12 +---
include/linux/i2c.h| 8 +++-
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index b8962e4..d8daf24 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -269,8 +269,6 @@ static int i2c_device_probe(struct device *dev)
return 0;
driver = to_i2c_driver(dev->driver);
- if (!driver->probe)
- return -EINVAL;
/*
* An I2C ID table is not mandatory, if and only if, a suitable Device
@@ -286,7 +284,15 @@ static int i2c_device_probe(struct device *dev)
dev_dbg(dev, "probe\n");
acpi_dev_pm_attach(&client->dev, true);
- status = driver->probe(client, i2c_match_id(driver->id_table, client));
+
+ /* When there are no more users of probe(), rename probe2 to probe. */
+ if (driver->probe2)
+ status = driver->probe2(client);
+ else if (driver->probe)
+ status = driver->probe(client,
+ i2c_match_id(driver->id_table, client));
+ else
+ return -EINVAL;
if (status)
acpi_dev_pm_detach(&client->dev, true);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 370905d..9414b16 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -125,7 +125,8 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct
i2c_client *client,
* struct i2c_driver - represent an I2C device driver
* @class: What kind of i2c device we instantiate (for detect)
* @attach_adapter: Callback for bus addition (deprecated)
- * @probe: Callback for device binding
+ * @probe: Callback for device binding - soon to be deprecated
+ * @probe2: New callback for device binding
* @remove: Callback for device unbinding
* @shutdown: Callback for device shutdown
* @suspend: Callback for device suspend
@@ -170,6 +171,11 @@ struct i2c_driver {
int (*probe)(struct i2c_client *, const struct i2c_device_id *);
int (*remove)(struct i2c_client *);
+ /* New driver model interface to aid the seamless removal of the
+* current probe()'s, more commonly unused than used second parameter.
+*/
+ int (*probe2)(struct i2c_client *);
+
/* driver model interfaces that don't relate to enumeration */
void (*shutdown)(struct i2c_client *);
int (*suspend)(struct i2c_client *, pm_message_t mesg);
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] mfd: 88pm860x: Move over to new I2C device .probe() call
As part of an effort to rid the mostly unused second parameter for I2C
related .probe() functions and to conform to other existing frameworks
we're moving over to a temporary replacement .probe() call-back.
Acked-by: Grant Likely
Signed-off-by: Lee Jones
---
drivers/mfd/88pm860x-core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index bcfc9e8..d763f1f 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -1127,8 +1127,7 @@ static int pm860x_dt_init(struct device_node *np,
return 0;
}
-static int pm860x_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int pm860x_probe(struct i2c_client *client)
{
struct pm860x_platform_data *pdata = dev_get_platdata(&client->dev);
struct device_node *node = client->dev.of_node;
@@ -1255,7 +1254,7 @@ static struct i2c_driver pm860x_driver = {
.pm = &pm860x_pm_ops,
.of_match_table = pm860x_dt_ids,
},
- .probe = pm860x_probe,
+ .probe2 = pm860x_probe,
.remove = pm860x_remove,
.id_table = pm860x_id_table,
};
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] mfd: as3722: Rid driver of superfluous I2C device ID structure
Also remove unused second probe() parameter 'i2c_device_id'.
Acked-by: Grant Likely
Signed-off-by: Lee Jones
---
drivers/mfd/as3722.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/mfd/as3722.c b/drivers/mfd/as3722.c
index 39fa554..cb2fcf9 100644
--- a/drivers/mfd/as3722.c
+++ b/drivers/mfd/as3722.c
@@ -354,8 +354,7 @@ static int as3722_i2c_of_probe(struct i2c_client *i2c,
return 0;
}
-static int as3722_i2c_probe(struct i2c_client *i2c,
- const struct i2c_device_id *id)
+static int as3722_i2c_probe(struct i2c_client *i2c)
{
struct as3722 *as3722;
unsigned long irq_flags;
@@ -428,21 +427,14 @@ static const struct of_device_id as3722_of_match[] = {
};
MODULE_DEVICE_TABLE(of, as3722_of_match);
-static const struct i2c_device_id as3722_i2c_id[] = {
- { "as3722", 0 },
- {},
-};
-MODULE_DEVICE_TABLE(i2c, as3722_i2c_id);
-
static struct i2c_driver as3722_i2c_driver = {
.driver = {
.name = "as3722",
.owner = THIS_MODULE,
.of_match_table = as3722_of_match,
},
- .probe = as3722_i2c_probe,
+ .probe2 = as3722_i2c_probe,
.remove = as3722_i2c_remove,
- .id_table = as3722_i2c_id,
};
module_i2c_driver(as3722_i2c_driver);
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] i2c: Export i2c_match_id() for direct use by device drivers
When there was no other way to match a I2C device to driver i2c_match_id()
was exclusively used. However, now there are other types of tables which
are commonly supplied, matching on an i2c_device_id table is used less
frequently. Instead of _always_ calling i2c_match_id() from within the
framework, we only need to do so from drivers which have no other way of
matching. This patch makes i2c_match_id() available to the aforementioned
device drivers.
Acked-by: Grant Likely
Signed-off-by: Lee Jones
---
drivers/i2c/i2c-core.c | 3 ++-
include/linux/i2c.h| 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index ff2919a..b8962e4 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -79,7 +79,7 @@ void i2c_transfer_trace_unreg(void)
/* - */
-static const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
+const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
const struct i2c_client *client)
{
if (!(id && client))
@@ -92,6 +92,7 @@ static const struct i2c_device_id *i2c_match_id(const struct
i2c_device_id *id,
}
return NULL;
}
+EXPORT_SYMBOL_GPL(i2c_match_id);
static int i2c_device_match(struct device *dev, struct device_driver *drv)
{
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 3d0350c..370905d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -229,6 +229,8 @@ struct i2c_client {
extern struct i2c_client *i2c_verify_client(struct device *dev);
extern struct i2c_adapter *i2c_verify_adapter(struct device *dev);
+extern const struct i2c_device_id *i2c_match_id(const struct i2c_device_id *id,
+ const struct i2c_client
*client);
static inline struct i2c_client *kobj_to_i2c_client(struct kobject *kobj)
{
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] i2c: Match using traditional OF methods, then by vendor-less compatible strings
This function provides a single call for all I2C devices which need to
match firstly using traditional OF means i.e by of_node, then if that
fails we attempt to match using the supplied I2C client name with a
list of supplied compatible strings with the ',' string
removed. The latter is required due to the unruly naming conventions
used currently by I2C devices.
Acked-by: Grant Likely
Signed-off-by: Lee Jones
---
drivers/i2c/i2c-core.c | 16
include/linux/i2c.h| 12
2 files changed, 28 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 8a37745..a9c97c4 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -,6 +,22 @@ i2c_of_match_device_strip_vendor(const struct
of_device_id *matches,
return NULL;
}
+const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches,
+struct i2c_client *client)
+{
+ const struct of_device_id *match;
+
+ if (!(client && matches))
+ return NULL;
+
+ match = of_match_device(matches, &client->dev);
+ if (match)
+ return match;
+
+ return i2c_of_match_device_strip_vendor(matches, client);
+}
+EXPORT_SYMBOL_GPL(i2c_of_match_device);
#else
static void of_i2c_register_devices(struct i2c_adapter *adap) { }
#endif /* CONFIG_OF */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b556e0a..3d0350c 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -564,6 +564,10 @@ extern struct i2c_client
*of_find_i2c_device_by_node(struct device_node *node);
/* must call put_device() when done with returned i2c_adapter device */
extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node
*node);
+extern const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches,
+struct i2c_client *client);
+
#else
static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node
*node)
@@ -575,6 +579,14 @@ static inline struct i2c_adapter
*of_find_i2c_adapter_by_node(struct device_node
{
return NULL;
}
+
+const struct of_device_id
+*i2c_of_match_device(const struct of_device_id *matches,
+struct i2c_client *client)
+{
+ return NULL;
+}
+
#endif /* CONFIG_OF */
#endif /* _LINUX_I2C_H */
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
