* [PATCH] extcon: Allow registering a single notifier for all cables on an extcon_dev
@ 2017-03-17 8:46 Hans de Goede
2017-03-19 13:06 ` kbuild test robot
0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2017-03-17 8:46 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi; +Cc: Hans de Goede, linux-kernel
In some cases a driver may want to monitor multiple cables on a single
extcon. For example a charger driver will typically want to monitor all
of EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP to configure
the max. current it can sink while charging.
Due to the signature of the notifier_call function + how extcon passes
state and the extcon_dev as parameters this requires using one
notifier_block + one notifier_call function per cable, otherwise the
notifier_call function cannot get to its driver's data (using container_of
requires it to know which notifier block its first parameter is).
For a driver wanting to monitor the above 3 cables that would result
in something like this:
static const unsigned int vbus_cable_ids[] = {
EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP };
struct driver_data {
struct notifier_block vbus_nb[ARRAY_SIZE(vbus_cable_ids)];
}
/*
* We need 3 copies of this, because there is no way to find out for which
* cable id we are being called from the passed in arguments; and we must
* have a separate nb for each extcon_register_notifier call.
*/
static int vbus_cable0_evt(struct notifier_block *nb, unsigned long e, void *p)
{
struct driver_data *data =
container_of(nb, struct driver_data, vbus_nb[0]);
...
}
static int vbus_cable1_evt(struct notifier_block *nb, unsigned long e, void *p)
{
struct driver_data *data =
container_of(nb, struct driver_data, vbus_nb[1]);
...
}
static int vbus_cable2_evt(struct notifier_block *nb, unsigned long e, void *p)
{
struct driver_data *data =
container_of(nb, struct driver_data, vbus_nb[2]);
...
}
int probe(...)
{
/* Register for vbus notification */
data->vbus_nb[0].notifier_call = vbus_cable0_evt;
data->vbus_nb[1].notifier_call = vbus_cable1_evt;
data->vbus_nb[2].notifier_call = vbus_cable2_evt;
for (i = 0; i < ARRAY_SIZE(vbus_cable_ids); i++) {
ret = devm_extcon_register_notifier(dev, data->vbus_extcon,
vbus_cable_ids[i], &data->vbus_nb[i]);
if (ret)
...
}
}
And then in the event handling the driver often checks the state of
all cables explicitly using extcon_get_state, rather then using the
event argument to the notifier_call.
This commit makes extcon_[un]register_notifier accept -1 as cable-id,
which will cause the notifier to get called for changes on any cable
on the extcon_dev. Compared to the above example code this allows much
simpler code in drivers which want to monitor multiple cable types.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/extcon/extcon.c | 33 +++++++++++++++++++++++++--------
drivers/extcon/extcon.h | 1 +
2 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 09ac5e7..e0254e7 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -449,6 +449,7 @@ int extcon_sync(struct extcon_dev *edev, unsigned int id)
state = !!(edev->state & BIT(index));
raw_notifier_call_chain(&edev->nh[index], state, edev);
+ raw_notifier_call_chain(&edev->nh_all, 0, edev);
/* This could be in interrupt handler */
prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
@@ -900,6 +901,8 @@ EXPORT_SYMBOL_GPL(extcon_get_extcon_dev);
* any attach status changes from the extcon.
* @edev: the extcon device that has the external connecotr.
* @id: the unique id of each external connector in extcon enumeration.
+ * or -1 to get notififications for all cables on edev, in this
+ * case no state info will get passed to the notifier_call.
* @nb: a notifier block to be registered.
*
* Note that the second parameter given to the callback of nb (val) is
@@ -915,12 +918,18 @@ int extcon_register_notifier(struct extcon_dev *edev, unsigned int id,
if (!edev || !nb)
return -EINVAL;
- idx = find_cable_index_by_id(edev, id);
- if (idx < 0)
- return idx;
+ if ((int)id != -1) {
+ idx = find_cable_index_by_id(edev, id);
+ if (idx < 0)
+ return idx;
+ }
spin_lock_irqsave(&edev->lock, flags);
- ret = raw_notifier_chain_register(&edev->nh[idx], nb);
+ if ((int)id != -1)
+ ret = raw_notifier_chain_register(&edev->nh[idx], nb);
+ else
+ ret = raw_notifier_chain_register(&edev->nh_all, nb);
+
spin_unlock_irqrestore(&edev->lock, flags);
return ret;
@@ -942,12 +951,18 @@ int extcon_unregister_notifier(struct extcon_dev *edev, unsigned int id,
if (!edev || !nb)
return -EINVAL;
- idx = find_cable_index_by_id(edev, id);
- if (idx < 0)
- return idx;
+ if ((int)id != -1) {
+ idx = find_cable_index_by_id(edev, id);
+ if (idx < 0)
+ return idx;
+ }
spin_lock_irqsave(&edev->lock, flags);
- ret = raw_notifier_chain_unregister(&edev->nh[idx], nb);
+ if ((int)id != -1)
+ ret = raw_notifier_chain_unregister(&edev->nh[idx], nb);
+ else
+ ret = raw_notifier_chain_unregister(&edev->nh_all, nb);
+
spin_unlock_irqrestore(&edev->lock, flags);
return ret;
@@ -1212,6 +1227,8 @@ int extcon_dev_register(struct extcon_dev *edev)
for (index = 0; index < edev->max_supported; index++)
RAW_INIT_NOTIFIER_HEAD(&edev->nh[index]);
+ RAW_INIT_NOTIFIER_HEAD(&edev->nh_all);
+
dev_set_drvdata(&edev->dev, edev);
edev->state = 0;
diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 993ddcc..2e6c09d 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -44,6 +44,7 @@ struct extcon_dev {
/* Internal data. Please do not set. */
struct device dev;
struct raw_notifier_head *nh;
+ struct raw_notifier_head nh_all;
struct list_head entry;
int max_supported;
spinlock_t lock; /* could be called by irq handler */
--
2.9.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] extcon: Allow registering a single notifier for all cables on an extcon_dev
2017-03-17 8:46 [PATCH] extcon: Allow registering a single notifier for all cables on an extcon_dev Hans de Goede
@ 2017-03-19 13:06 ` kbuild test robot
2017-03-19 18:18 ` Hans de Goede
0 siblings, 1 reply; 3+ messages in thread
From: kbuild test robot @ 2017-03-19 13:06 UTC (permalink / raw)
To: Hans de Goede
Cc: kbuild-all, MyungJoo Ham, Chanwoo Choi, Hans de Goede, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3866 bytes --]
Hi Hans,
[auto build test WARNING on chanwoo-extcon/extcon-next]
[also build test WARNING on v4.11-rc2 next-20170310]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Hans-de-Goede/extcon-Allow-registering-a-single-notifier-for-all-cables-on-an-extcon_dev/20170319-204610
base: https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
config: i386-randconfig-x072-201712 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/extcon/extcon.c: In function 'extcon_unregister_notifier':
>> drivers/extcon/extcon.c:962:48: warning: 'idx' may be used uninitialized in this function [-Wmaybe-uninitialized]
ret = raw_notifier_chain_unregister(&edev->nh[idx], nb);
^
vim +/idx +962 drivers/extcon/extcon.c
74c5d09b drivers/extcon/extcon_class.c Donggeun Kim 2012-04-20 946 struct notifier_block *nb)
74c5d09b drivers/extcon/extcon_class.c Donggeun Kim 2012-04-20 947 {
66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 948 unsigned long flags;
046050f6 drivers/extcon/extcon.c Chanwoo Choi 2015-05-19 949 int ret, idx;
046050f6 drivers/extcon/extcon.c Chanwoo Choi 2015-05-19 950
7eae43ae drivers/extcon/extcon.c Chanwoo Choi 2015-06-21 951 if (!edev || !nb)
7eae43ae drivers/extcon/extcon.c Chanwoo Choi 2015-06-21 952 return -EINVAL;
7eae43ae drivers/extcon/extcon.c Chanwoo Choi 2015-06-21 953
8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 954 if ((int)id != -1) {
046050f6 drivers/extcon/extcon.c Chanwoo Choi 2015-05-19 955 idx = find_cable_index_by_id(edev, id);
a05f44c8 drivers/extcon/extcon.c Stephen Boyd 2016-06-23 956 if (idx < 0)
a05f44c8 drivers/extcon/extcon.c Stephen Boyd 2016-06-23 957 return idx;
8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 958 }
66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 959
66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 960 spin_lock_irqsave(&edev->lock, flags);
8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 961 if ((int)id != -1)
046050f6 drivers/extcon/extcon.c Chanwoo Choi 2015-05-19 @962 ret = raw_notifier_chain_unregister(&edev->nh[idx], nb);
8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 963 else
8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 964 ret = raw_notifier_chain_unregister(&edev->nh_all, nb);
8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 965
66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 966 spin_unlock_irqrestore(&edev->lock, flags);
66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 967
66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 968 return ret;
74c5d09b drivers/extcon/extcon_class.c Donggeun Kim 2012-04-20 969 }
74c5d09b drivers/extcon/extcon_class.c Donggeun Kim 2012-04-20 970 EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
:::::: The code at line 962 was first introduced by commit
:::::: 046050f6e623e442e9c71c525462ebd395dae526 extcon: Update the prototype of extcon_register_notifier() with enum extcon
:::::: TO: Chanwoo Choi <cw00.choi@samsung.com>
:::::: CC: Chanwoo Choi <cw00.choi@samsung.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29190 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] extcon: Allow registering a single notifier for all cables on an extcon_dev
2017-03-19 13:06 ` kbuild test robot
@ 2017-03-19 18:18 ` Hans de Goede
0 siblings, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2017-03-19 18:18 UTC (permalink / raw)
To: kbuild test robot; +Cc: kbuild-all, MyungJoo Ham, Chanwoo Choi, linux-kernel
Hi,
On 19-03-17 14:06, kbuild test robot wrote:
> Hi Hans,
>
> [auto build test WARNING on chanwoo-extcon/extcon-next]
> [also build test WARNING on v4.11-rc2 next-20170310]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Hans-de-Goede/extcon-Allow-registering-a-single-notifier-for-all-cables-on-an-extcon_dev/20170319-204610
> base: https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git extcon-next
> config: i386-randconfig-x072-201712 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
> http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
>
> All warnings (new ones prefixed by >>):
>
> drivers/extcon/extcon.c: In function 'extcon_unregister_notifier':
>>> drivers/extcon/extcon.c:962:48: warning: 'idx' may be used uninitialized in this function [-Wmaybe-uninitialized]
> ret = raw_notifier_chain_unregister(&edev->nh[idx], nb);
I was already afraid this would happen, I was actually pleasantly surprised
when I did not get this warning during my own testing. But it seems other
compiler versions are not as smart as the very recent gcc I'm using.
I'll send a v2 fixing this.
Regards,
Hans
> ^
>
> vim +/idx +962 drivers/extcon/extcon.c
>
> 74c5d09b drivers/extcon/extcon_class.c Donggeun Kim 2012-04-20 946 struct notifier_block *nb)
> 74c5d09b drivers/extcon/extcon_class.c Donggeun Kim 2012-04-20 947 {
> 66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 948 unsigned long flags;
> 046050f6 drivers/extcon/extcon.c Chanwoo Choi 2015-05-19 949 int ret, idx;
> 046050f6 drivers/extcon/extcon.c Chanwoo Choi 2015-05-19 950
> 7eae43ae drivers/extcon/extcon.c Chanwoo Choi 2015-06-21 951 if (!edev || !nb)
> 7eae43ae drivers/extcon/extcon.c Chanwoo Choi 2015-06-21 952 return -EINVAL;
> 7eae43ae drivers/extcon/extcon.c Chanwoo Choi 2015-06-21 953
> 8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 954 if ((int)id != -1) {
> 046050f6 drivers/extcon/extcon.c Chanwoo Choi 2015-05-19 955 idx = find_cable_index_by_id(edev, id);
> a05f44c8 drivers/extcon/extcon.c Stephen Boyd 2016-06-23 956 if (idx < 0)
> a05f44c8 drivers/extcon/extcon.c Stephen Boyd 2016-06-23 957 return idx;
> 8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 958 }
> 66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 959
> 66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 960 spin_lock_irqsave(&edev->lock, flags);
> 8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 961 if ((int)id != -1)
> 046050f6 drivers/extcon/extcon.c Chanwoo Choi 2015-05-19 @962 ret = raw_notifier_chain_unregister(&edev->nh[idx], nb);
> 8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 963 else
> 8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 964 ret = raw_notifier_chain_unregister(&edev->nh_all, nb);
> 8f482488 drivers/extcon/extcon.c Hans de Goede 2017-03-17 965
> 66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 966 spin_unlock_irqrestore(&edev->lock, flags);
> 66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 967
> 66bee35f drivers/extcon/extcon.c Hans de Goede 2015-03-21 968 return ret;
> 74c5d09b drivers/extcon/extcon_class.c Donggeun Kim 2012-04-20 969 }
> 74c5d09b drivers/extcon/extcon_class.c Donggeun Kim 2012-04-20 970 EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
>
> :::::: The code at line 962 was first introduced by commit
> :::::: 046050f6e623e442e9c71c525462ebd395dae526 extcon: Update the prototype of extcon_register_notifier() with enum extcon
>
> :::::: TO: Chanwoo Choi <cw00.choi@samsung.com>
> :::::: CC: Chanwoo Choi <cw00.choi@samsung.com>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-19 18:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-17 8:46 [PATCH] extcon: Allow registering a single notifier for all cables on an extcon_dev Hans de Goede
2017-03-19 13:06 ` kbuild test robot
2017-03-19 18:18 ` Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome