* [PATCH 1/3] extcon: Fix indentation coding style to improve readability
2013-09-02 0:20 [PATCH 0/3] extcon: Code clean to fix up coding style and remove Chanwoo Choi
@ 2013-09-02 0:20 ` Chanwoo Choi
2013-09-02 0:20 ` [PATCH 2/3] extcon: Change field type of 'dev' in extcon_dev structure Chanwoo Choi
2013-09-02 0:20 ` [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter Chanwoo Choi
2 siblings, 0 replies; 14+ messages in thread
From: Chanwoo Choi @ 2013-09-02 0:20 UTC (permalink / raw)
To: linux-kernel; +Cc: gregkh, myungjoo.ham, kyungmin.park, Chanwoo Choi
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
drivers/extcon/extcon-adc-jack.c | 20 +++++-----
drivers/extcon/extcon-class.c | 26 ++++++-------
include/linux/extcon.h | 67 ++++++++++++++++++----------------
include/linux/extcon/extcon-adc-jack.h | 42 ++++++++++-----------
include/linux/extcon/extcon-gpio.h | 16 ++++----
5 files changed, 87 insertions(+), 84 deletions(-)
diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index 5985807..5d16428 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -27,16 +27,16 @@
/**
* struct adc_jack_data - internal data for adc_jack device driver
- * @edev - extcon device.
- * @cable_names - list of supported cables.
- * @num_cables - size of cable_names.
- * @adc_conditions - list of adc value conditions.
- * @num_conditions - size of adc_conditions.
- * @irq - irq number of attach/detach event (0 if not exist).
- * @handling_delay - interrupt handler will schedule extcon event
- * handling at handling_delay jiffies.
- * @handler - extcon event handler called by interrupt handler.
- * @chan - iio channel being queried.
+ * @edev: extcon device.
+ * @cable_names: list of supported cables.
+ * @num_cables: size of cable_names.
+ * @adc_conditions: list of adc value conditions.
+ * @num_conditions: size of adc_conditions.
+ * @irq: irq number of attach/detach event (0 if not exist).
+ * @handling_delay: interrupt handler will schedule extcon event
+ * handling at handling_delay jiffies.
+ * @handler: extcon event handler called by interrupt handler.
+ * @chan: iio channel being queried.
*/
struct adc_jack_data {
struct extcon_dev edev;
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 7b6b996..ed456b3 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -74,7 +74,7 @@ static DEFINE_MUTEX(extcon_dev_list_lock);
/**
* check_mutually_exclusive - Check if new_state violates mutually_exclusive
- * condition.
+ * condition.
* @edev: the extcon device
* @new_state: new cable attach status for @edev
*
@@ -186,7 +186,7 @@ static ssize_t cable_state_show(struct device *dev,
/**
* extcon_update_state() - Update the cable attach states of the extcon device
- * only for the masked bits.
+ * only for the masked bits.
* @edev: the extcon device
* @mask: the bit mask to designate updated bits.
* @state: new cable attach status for @edev
@@ -224,7 +224,6 @@ int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
edev->state |= state & mask;
raw_notifier_call_chain(&edev->nh, old_state, edev);
-
/* This could be in interrupt handler */
prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
if (prop_buf) {
@@ -337,8 +336,9 @@ EXPORT_SYMBOL_GPL(extcon_get_cable_state);
/**
* extcon_set_cable_state_() - Set the status of a specific cable.
- * @edev: the extcon device that has the cable.
- * @index: cable index that can be retrieved by extcon_find_cable_index().
+ * @edev: the extcon device that has the cable.
+ * @index: cable index that can be retrieved by
+ * extcon_find_cable_index().
* @cable_state: the new cable status. The default semantics is
* true: attached / false: detached.
*/
@@ -357,8 +357,8 @@ EXPORT_SYMBOL_GPL(extcon_set_cable_state_);
/**
* extcon_set_cable_state() - Set the status of a specific cable.
- * @edev: the extcon device that has the cable.
- * @cable_name: cable name.
+ * @edev: the extcon device that has the cable.
+ * @cable_name: cable name.
* @cable_state: the new cable status. The default semantics is
* true: attached / false: detached.
*
@@ -417,14 +417,14 @@ static int _call_per_cable(struct notifier_block *nb, unsigned long val,
/**
* extcon_register_interest() - Register a notifier for a state change of a
- * specific cable, not an entier set of cables of a
- * extcon device.
- * @obj: an empty extcon_specific_cable_nb object to be returned.
+ * specific cable, not an entier set of cables of a
+ * extcon device.
+ * @obj: an empty extcon_specific_cable_nb object to be returned.
* @extcon_name: the name of extcon device.
* if NULL, extcon_register_interest will register
* every cable with the target cable_name given.
* @cable_name: the target cable name.
- * @nb: the notifier block to get notified.
+ * @nb: the notifier block to get notified.
*
* Provide an empty extcon_specific_cable_nb. extcon_register_interest() sets
* the struct for you.
@@ -487,7 +487,7 @@ EXPORT_SYMBOL_GPL(extcon_register_interest);
/**
* extcon_unregister_interest() - Unregister the notifier registered by
- * extcon_register_interest().
+ * extcon_register_interest().
* @obj: the extcon_specific_cable_nb object returned by
* extcon_register_interest().
*/
@@ -502,7 +502,7 @@ EXPORT_SYMBOL_GPL(extcon_unregister_interest);
/**
* extcon_register_notifier() - Register a notifiee to get notified by
- * any attach status changes from the extcon.
+ * any attach status changes from the extcon.
* @edev: the extcon device.
* @nb: a notifier block to be registered.
*
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index fcb51c8..c2b652dd 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -51,10 +51,10 @@
enum extcon_cable_name {
EXTCON_USB = 0,
EXTCON_USB_HOST,
- EXTCON_TA, /* Travel Adaptor */
+ EXTCON_TA, /* Travel Adaptor */
EXTCON_FAST_CHARGER,
EXTCON_SLOW_CHARGER,
- EXTCON_CHARGE_DOWNSTREAM, /* Charging an external device */
+ EXTCON_CHARGE_DOWNSTREAM, /* Charging an external device */
EXTCON_HDMI,
EXTCON_MHL,
EXTCON_DVI,
@@ -76,8 +76,8 @@ struct extcon_cable;
/**
* struct extcon_dev - An extcon device represents one external connector.
- * @name: The name of this extcon device. Parent device name is used
- * if NULL.
+ * @name: The name of this extcon device. Parent device name is
+ * used if NULL.
* @supported_cable: Array of supported cable names ending with NULL.
* If supported_cable is NULL, cable name related APIs
* are disabled.
@@ -89,21 +89,21 @@ struct extcon_cable;
* be attached simulataneously. {0x7, 0} is equivalent to
* {0x3, 0x6, 0x5, 0}. If it is {0xFFFFFFFF, 0}, there
* can be no simultaneous connections.
- * @print_name: An optional callback to override the method to print the
- * name of the extcon device.
+ * @print_name: An optional callback to override the method to print the
+ * name of the extcon device.
* @print_state: An optional callback to override the method to print the
- * status of the extcon device.
- * @dev: Device of this extcon. Do not provide at register-time.
- * @state: Attach/detach state of this extcon. Do not provide at
- * register-time
- * @nh: Notifier for the state change events from this extcon
- * @entry: To support list of extcon devices so that users can search
- * for extcon devices based on the extcon name.
+ * status of the extcon device.
+ * @dev: Device of this extcon. Do not provide at register-time.
+ * @state: Attach/detach state of this extcon. Do not provide at
+ * register-time.
+ * @nh: Notifier for the state change events from this extcon
+ * @entry: To support list of extcon devices so that users can search
+ * for extcon devices based on the extcon name.
* @lock:
* @max_supported: Internal value to store the number of cables.
* @extcon_dev_type: Device_type struct to provide attribute_groups
* customized for each extcon device.
- * @cables: Sysfs subdirectories. Each represents one cable.
+ * @cables: Sysfs subdirectories. Each represents one cable.
*
* In most cases, users only need to provide "User initializing data" of
* this struct when registering an extcon. In some exceptional cases,
@@ -111,26 +111,27 @@ struct extcon_cable;
* are overwritten by register function.
*/
struct extcon_dev {
- /* --- Optional user initializing data --- */
- const char *name;
+ /* Optional user initializing data */
+ const char *name;
const char **supported_cable;
- const u32 *mutually_exclusive;
+ const u32 *mutually_exclusive;
- /* --- Optional callbacks to override class functions --- */
+ /* Optional callbacks to override class functions */
ssize_t (*print_name)(struct extcon_dev *edev, char *buf);
ssize_t (*print_state)(struct extcon_dev *edev, char *buf);
- /* --- Internal data. Please do not set. --- */
- struct device *dev;
- u32 state;
+ /* Internal data. Please do not set. */
+ struct device *dev;
struct raw_notifier_head nh;
struct list_head entry;
- spinlock_t lock; /* could be called by irq handler */
int max_supported;
+ spinlock_t lock; /* could be called by irq handler */
+ u32 state;
/* /sys/class/extcon/.../cable.n/... */
struct device_type extcon_dev_type;
struct extcon_cable *cables;
+
/* /sys/class/extcon/.../mutually_exclusive/... */
struct attribute_group attr_g_muex;
struct attribute **attrs_muex;
@@ -138,13 +139,13 @@ struct extcon_dev {
};
/**
- * struct extcon_cable - An internal data for each cable of extcon device.
- * @edev: The extcon device
+ * struct extcon_cable - An internal data for each cable of extcon device.
+ * @edev: The extcon device
* @cable_index: Index of this cable in the edev
- * @attr_g: Attribute group for the cable
- * @attr_name: "name" sysfs entry
- * @attr_state: "state" sysfs entry
- * @attrs: Array pointing to attr_name and attr_state for attr_g
+ * @attr_g: Attribute group for the cable
+ * @attr_name: "name" sysfs entry
+ * @attr_state: "state" sysfs entry
+ * @attrs: Array pointing to attr_name and attr_state for attr_g
*/
struct extcon_cable {
struct extcon_dev *edev;
@@ -159,11 +160,13 @@ struct extcon_cable {
/**
* struct extcon_specific_cable_nb - An internal data for
- * extcon_register_interest().
- * @internal_nb: a notifier block bridging extcon notifier and cable notifier.
- * @user_nb: user provided notifier block for events from a specific cable.
+ * extcon_register_interest().
+ * @internal_nb: A notifier block bridging extcon notifier
+ * and cable notifier.
+ * @user_nb: user provided notifier block for events from
+ * a specific cable.
* @cable_index: the target cable.
- * @edev: the target extcon device.
+ * @edev: the target extcon device.
* @previous_value: the saved previous event value.
*/
struct extcon_specific_cable_nb {
diff --git a/include/linux/extcon/extcon-adc-jack.h b/include/linux/extcon/extcon-adc-jack.h
index 20e9eef..9ca958c 100644
--- a/include/linux/extcon/extcon-adc-jack.h
+++ b/include/linux/extcon/extcon-adc-jack.h
@@ -20,10 +20,10 @@
/**
* struct adc_jack_cond - condition to use an extcon state
- * @state - the corresponding extcon state (if 0, this struct denotes
- * the last adc_jack_cond element among the array)
- * @min_adc - min adc value for this condition
- * @max_adc - max adc value for this condition
+ * @state: the corresponding extcon state (if 0, this struct
+ * denotes the last adc_jack_cond element among the array)
+ * @min_adc: min adc value for this condition
+ * @max_adc: max adc value for this condition
*
* For example, if { .state = 0x3, .min_adc = 100, .max_adc = 200}, it means
* that if ADC value is between (inclusive) 100 and 200, than the cable 0 and
@@ -33,34 +33,34 @@
* because when no adc_jack_cond is met, state = 0 is automatically chosen.
*/
struct adc_jack_cond {
- u32 state; /* extcon state value. 0 if invalid */
+ u32 state; /* extcon state value. 0 if invalid */
u32 min_adc;
u32 max_adc;
};
/**
* struct adc_jack_pdata - platform data for adc jack device.
- * @name - name of the extcon device. If null, "adc-jack" is used.
- * @consumer_channel - Unique name to identify the channel on the consumer
- * side. This typically describes the channels used within
- * the consumer. E.g. 'battery_voltage'
- * @cable_names - array of cable names ending with null.
- * @adc_contitions - array of struct adc_jack_cond conditions ending
- * with .state = 0 entry. This describes how to decode
- * adc values into extcon state.
- * @irq_flags - irq flags used for the @irq
- * @handling_delay_ms - in some devices, we need to read ADC value some
- * milli-seconds after the interrupt occurs. You may
- * describe such delays with @handling_delay_ms, which
- * is rounded-off by jiffies.
+ * @name: name of the extcon device. If null, "adc-jack" is used.
+ * @consumer_channel: Unique name to identify the channel on the consumer
+ * side. This typically describes the channels used within
+ * the consumer. E.g. 'battery_voltage'
+ * @cable_names: array of cable names ending with null.
+ * @adc_contitions: array of struct adc_jack_cond conditions ending
+ * with .state = 0 entry. This describes how to decode
+ * adc values into extcon state.
+ * @irq_flags: irq flags used for the @irq
+ * @handling_delay_ms: in some devices, we need to read ADC value some
+ * milli-seconds after the interrupt occurs. You may
+ * describe such delays with @handling_delay_ms, which
+ * is rounded-off by jiffies.
*/
struct adc_jack_pdata {
const char *name;
const char *consumer_channel;
- /*
- * The last entry should be NULL
- */
+
+ /* The last entry should be NULL */
const char **cable_names;
+
/* The last entry's state should be 0 */
struct adc_jack_cond *adc_conditions;
diff --git a/include/linux/extcon/extcon-gpio.h b/include/linux/extcon/extcon-gpio.h
index 2d8307f..4ce1aa7 100644
--- a/include/linux/extcon/extcon-gpio.h
+++ b/include/linux/extcon/extcon-gpio.h
@@ -25,14 +25,14 @@
/**
* struct gpio_extcon_platform_data - A simple GPIO-controlled extcon device.
- * @name The name of this GPIO extcon device.
- * @gpio Corresponding GPIO.
- * @debounce Debounce time for GPIO IRQ in ms.
- * @irq_flags IRQ Flags (e.g., IRQF_TRIGGER_LOW).
- * @state_on print_state is overriden with state_on if attached. If Null,
- * default method of extcon class is used.
- * @state_off print_state is overriden with state_on if detached. If Null,
- * default method of extcon class is used.
+ * @name: The name of this GPIO extcon device.
+ * @gpio: Corresponding GPIO.
+ * @debounce: Debounce time for GPIO IRQ in ms.
+ * @irq_flags: IRQ Flags (e.g., IRQF_TRIGGER_LOW).
+ * @state_on: print_state is overriden with state_on if attached.
+ * If NULL, default method of extcon class is used.
+ * @state_off: print_state is overriden with state_on if detached.
+ * If NUll, default method of extcon class is used.
*
* Note that in order for state_on or state_off to be valid, both state_on
* and state_off should be not NULL. If at least one of them is NULL,
--
1.8.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 2/3] extcon: Change field type of 'dev' in extcon_dev structure
2013-09-02 0:20 [PATCH 0/3] extcon: Code clean to fix up coding style and remove Chanwoo Choi
2013-09-02 0:20 ` [PATCH 1/3] extcon: Fix indentation coding style to improve readability Chanwoo Choi
@ 2013-09-02 0:20 ` Chanwoo Choi
2013-09-02 0:38 ` Greg KH
2013-09-02 0:20 ` [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter Chanwoo Choi
2 siblings, 1 reply; 14+ messages in thread
From: Chanwoo Choi @ 2013-09-02 0:20 UTC (permalink / raw)
To: linux-kernel; +Cc: gregkh, myungjoo.ham, kyungmin.park, Chanwoo Choi
The extcon device must always need 'struct device' so this patch change
field type of 'dev' instead of allocating memory for 'struct device' on
extcon_dev_register() function.
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Myungjoo Ham <cw00.choi@samsung.com>
---
drivers/extcon/extcon-adc-jack.c | 2 +-
drivers/extcon/extcon-class.c | 52 +++++++++++++++++++---------------------
include/linux/extcon.h | 4 ++--
3 files changed, 27 insertions(+), 31 deletions(-)
diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index 5d16428..90346d6 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -64,7 +64,7 @@ static void adc_jack_handler(struct work_struct *work)
ret = iio_read_channel_raw(data->chan, &adc_val);
if (ret < 0) {
- dev_err(data->edev.dev, "read channel() error: %d\n", ret);
+ dev_err(&data->edev.dev, "read channel() error: %d\n", ret);
return;
}
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index ed456b3..8bc43e6 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -160,7 +160,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
return ret;
}
- return sprintf(buf, "%s\n", dev_name(edev->dev));
+ return sprintf(buf, "%s\n", dev_name(&edev->dev));
}
static ssize_t cable_name_show(struct device *dev,
@@ -224,10 +224,11 @@ int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
edev->state |= state & mask;
raw_notifier_call_chain(&edev->nh, old_state, edev);
+
/* This could be in interrupt handler */
prop_buf = (char *)get_zeroed_page(GFP_ATOMIC);
if (prop_buf) {
- length = name_show(edev->dev, NULL, prop_buf);
+ length = name_show(&edev->dev, NULL, prop_buf);
if (length > 0) {
if (prop_buf[length - 1] == '\n')
prop_buf[length - 1] = 0;
@@ -235,7 +236,7 @@ int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
"NAME=%s", prop_buf);
envp[env_offset++] = name_buf;
}
- length = state_show(edev->dev, NULL, prop_buf);
+ length = state_show(&edev->dev, NULL, prop_buf);
if (length > 0) {
if (prop_buf[length - 1] == '\n')
prop_buf[length - 1] = 0;
@@ -247,15 +248,15 @@ int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
/* Unlock early before uevent */
spin_unlock_irqrestore(&edev->lock, flags);
- kobject_uevent_env(&edev->dev->kobj, KOBJ_CHANGE, envp);
+ kobject_uevent_env(&edev->dev.kobj, KOBJ_CHANGE, envp);
free_page((unsigned long)prop_buf);
} else {
/* Unlock early before uevent */
spin_unlock_irqrestore(&edev->lock, flags);
- dev_err(edev->dev,
+ dev_err(&edev->dev,
"out of memory in extcon_set_state\n");
- kobject_uevent(&edev->dev->kobj, KOBJ_CHANGE);
+ kobject_uevent(&edev->dev.kobj, KOBJ_CHANGE);
}
} else {
/* No changes */
@@ -593,20 +594,17 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
}
if (index > SUPPORTED_CABLE_MAX) {
- dev_err(edev->dev,
+ dev_err(&edev->dev,
"maximum number of supported cables exceeded\n");
return -EINVAL;
}
- edev->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
- if (!edev->dev)
- return -ENOMEM;
- edev->dev->parent = dev;
- edev->dev->class = extcon_class;
- edev->dev->release = extcon_dev_release;
+ edev->dev.parent = dev;
+ edev->dev.class = extcon_class;
+ edev->dev.release = extcon_dev_release;
edev->name = edev->name ? edev->name : dev_name(dev);
- dev_set_name(edev->dev, "%s", edev->name);
+ dev_set_name(&edev->dev, "%s", edev->name);
if (edev->max_supported) {
char buf[10];
@@ -714,7 +712,7 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
goto err_alloc_groups;
}
- edev->extcon_dev_type.name = dev_name(edev->dev);
+ edev->extcon_dev_type.name = dev_name(&edev->dev);
edev->extcon_dev_type.release = dummy_sysfs_dev_release;
for (index = 0; index < edev->max_supported; index++)
@@ -724,25 +722,24 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
edev->extcon_dev_type.groups[index] =
&edev->attr_g_muex;
- edev->dev->type = &edev->extcon_dev_type;
+ edev->dev.type = &edev->extcon_dev_type;
}
- ret = device_register(edev->dev);
+ ret = device_register(&edev->dev);
if (ret) {
- put_device(edev->dev);
+ put_device(&edev->dev);
goto err_dev;
}
#if defined(CONFIG_ANDROID)
if (switch_class)
- ret = class_compat_create_link(switch_class, edev->dev,
- NULL);
+ ret = class_compat_create_link(switch_class, &edev->dev, NULL);
#endif /* CONFIG_ANDROID */
spin_lock_init(&edev->lock);
RAW_INIT_NOTIFIER_HEAD(&edev->nh);
- dev_set_drvdata(edev->dev, edev);
+ dev_set_drvdata(&edev->dev, edev);
edev->state = 0;
mutex_lock(&extcon_dev_list_lock);
@@ -768,7 +765,6 @@ err_alloc_cables:
if (edev->max_supported)
kfree(edev->cables);
err_sysfs_alloc:
- kfree(edev->dev);
return ret;
}
EXPORT_SYMBOL_GPL(extcon_dev_register);
@@ -788,9 +784,9 @@ void extcon_dev_unregister(struct extcon_dev *edev)
list_del(&edev->entry);
mutex_unlock(&extcon_dev_list_lock);
- if (IS_ERR_OR_NULL(get_device(edev->dev))) {
- dev_err(edev->dev, "Failed to unregister extcon_dev (%s)\n",
- dev_name(edev->dev));
+ if (IS_ERR_OR_NULL(get_device(&edev->dev))) {
+ dev_err(&edev->dev, "Failed to unregister extcon_dev (%s)\n",
+ dev_name(&edev->dev));
return;
}
@@ -812,10 +808,10 @@ void extcon_dev_unregister(struct extcon_dev *edev)
#if defined(CONFIG_ANDROID)
if (switch_class)
- class_compat_remove_link(switch_class, edev->dev, NULL);
+ class_compat_remove_link(switch_class, &edev->dev, NULL);
#endif
- device_unregister(edev->dev);
- put_device(edev->dev);
+ device_unregister(&edev->dev);
+ put_device(&edev->dev);
}
EXPORT_SYMBOL_GPL(extcon_dev_unregister);
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index c2b652dd..0269baf 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -93,7 +93,7 @@ struct extcon_cable;
* name of the extcon device.
* @print_state: An optional callback to override the method to print the
* status of the extcon device.
- * @dev: Device of this extcon. Do not provide at register-time.
+ * @dev: Device of this extcon.
* @state: Attach/detach state of this extcon. Do not provide at
* register-time.
* @nh: Notifier for the state change events from this extcon
@@ -121,7 +121,7 @@ struct extcon_dev {
ssize_t (*print_state)(struct extcon_dev *edev, char *buf);
/* Internal data. Please do not set. */
- struct device *dev;
+ struct device dev;
struct raw_notifier_head nh;
struct list_head entry;
int max_supported;
--
1.8.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] extcon: Change field type of 'dev' in extcon_dev structure
2013-09-02 0:20 ` [PATCH 2/3] extcon: Change field type of 'dev' in extcon_dev structure Chanwoo Choi
@ 2013-09-02 0:38 ` Greg KH
2013-09-02 0:41 ` Chanwoo Choi
0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2013-09-02 0:38 UTC (permalink / raw)
To: Chanwoo Choi; +Cc: linux-kernel, myungjoo.ham, kyungmin.park
On Mon, Sep 02, 2013 at 09:20:07AM +0900, Chanwoo Choi wrote:
> - edev->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> - if (!edev->dev)
> - return -ENOMEM;
> - edev->dev->parent = dev;
> - edev->dev->class = extcon_class;
> - edev->dev->release = extcon_dev_release;
> + edev->dev.parent = dev;
> + edev->dev.class = extcon_class;
> + edev->dev.release = extcon_dev_release;
You didn't change extcon_dev_release() to properly free all of the
memory you allocated here, otherwise the slab allocator will oops when
you try to call it...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] extcon: Change field type of 'dev' in extcon_dev structure
2013-09-02 0:38 ` Greg KH
@ 2013-09-02 0:41 ` Chanwoo Choi
0 siblings, 0 replies; 14+ messages in thread
From: Chanwoo Choi @ 2013-09-02 0:41 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, myungjoo.ham, kyungmin.park
Hi Greg,
On 09/02/2013 09:38 AM, Greg KH wrote:
> On Mon, Sep 02, 2013 at 09:20:07AM +0900, Chanwoo Choi wrote:
>> - edev->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
>> - if (!edev->dev)
>> - return -ENOMEM;
>> - edev->dev->parent = dev;
>> - edev->dev->class = extcon_class;
>> - edev->dev->release = extcon_dev_release;
>> + edev->dev.parent = dev;
>> + edev->dev.class = extcon_class;
>> + edev->dev.release = extcon_dev_release;
>
> You didn't change extcon_dev_release() to properly free all of the
> memory you allocated here, otherwise the slab allocator will oops when
> you try to call it...
>
I'll fix it. Thanks.
Best Regards,
Chanwoo Choi
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-02 0:20 [PATCH 0/3] extcon: Code clean to fix up coding style and remove Chanwoo Choi
2013-09-02 0:20 ` [PATCH 1/3] extcon: Fix indentation coding style to improve readability Chanwoo Choi
2013-09-02 0:20 ` [PATCH 2/3] extcon: Change field type of 'dev' in extcon_dev structure Chanwoo Choi
@ 2013-09-02 0:20 ` Chanwoo Choi
2013-09-02 0:40 ` Greg KH
2 siblings, 1 reply; 14+ messages in thread
From: Chanwoo Choi @ 2013-09-02 0:20 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, myungjoo.ham, kyungmin.park, Chanwoo Choi, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
This patch remove extcon_dev_register()'s second parameter which means
the pointer of parent device to simplify prototype of this function.
So, if extcon device has the parent device, it should set the pointer of
parent device to edev.dev.parent in extcon device driver instead of in
extcon_dev_register().
Cc: Graeme Gregory <gg@slimlogic.co.uk>
Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
Documentation/extcon/porting-android-switch-class | 6 ++++--
drivers/extcon/extcon-adc-jack.c | 3 ++-
drivers/extcon/extcon-arizona.c | 3 ++-
drivers/extcon/extcon-class.c | 11 +++++++----
drivers/extcon/extcon-gpio.c | 3 ++-
drivers/extcon/extcon-max77693.c | 3 ++-
drivers/extcon/extcon-max8997.c | 3 ++-
drivers/extcon/extcon-palmas.c | 3 ++-
include/linux/extcon.h | 5 ++---
9 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/Documentation/extcon/porting-android-switch-class b/Documentation/extcon/porting-android-switch-class
index eb0fa5f..5377f63 100644
--- a/Documentation/extcon/porting-android-switch-class
+++ b/Documentation/extcon/porting-android-switch-class
@@ -25,8 +25,10 @@ MyungJoo Ham <myungjoo.ham@samsung.com>
@print_state: no change but type change (switch_dev->extcon_dev)
- switch_dev_register(sdev, dev)
- => extcon_dev_register(edev, dev)
- : no change but type change (sdev->edev)
+ => extcon_dev_register(edev)
+ : type change (sdev->edev)
+ : remove second param('dev'). if edev has parent device, should store
+ 'dev' to 'edev.dev.parent' before registering extcon device
- switch_dev_unregister(sdev)
=> extcon_dev_unregister(edev)
: no change but type change (sdev->edev)
diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index 90346d6..7f7f930 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -110,6 +110,7 @@ static int adc_jack_probe(struct platform_device *pdev)
goto out;
}
+ data->edev.dev.parent = &pdev->dev;
data->edev.supported_cable = pdata->cable_names;
/* Check the length of array and set num_cables */
@@ -148,7 +149,7 @@ static int adc_jack_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, data);
- err = extcon_dev_register(&data->edev, &pdev->dev);
+ err = extcon_dev_register(&data->edev);
if (err)
goto out;
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 72fc28e..f639f2a 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1132,9 +1132,10 @@ static int arizona_extcon_probe(struct platform_device *pdev)
}
info->edev.name = "Headset Jack";
+ info->edev.dev.parent = arizona->dev;
info->edev.supported_cable = arizona_cable;
- ret = extcon_dev_register(&info->edev, arizona->dev);
+ ret = extcon_dev_register(&info->edev);
if (ret < 0) {
dev_err(arizona->dev, "extcon_dev_register() failed: %d\n",
ret);
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 8bc43e6..b096db8 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -567,14 +567,13 @@ static void dummy_sysfs_dev_release(struct device *dev)
/**
* extcon_dev_register() - Register a new extcon device
* @edev : the new extcon device (should be allocated before calling)
- * @dev : the parent device for this extcon device.
*
* Among the members of edev struct, please set the "user initializing data"
* in any case and set the "optional callbacks" if required. However, please
* do not set the values of "internal data", which are initialized by
* this function.
*/
-int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
+int extcon_dev_register(struct extcon_dev *edev)
{
int ret, index = 0;
@@ -599,11 +598,15 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
return -EINVAL;
}
- edev->dev.parent = dev;
edev->dev.class = extcon_class;
edev->dev.release = extcon_dev_release;
- edev->name = edev->name ? edev->name : dev_name(dev);
+ edev->name = edev->name ? edev->name : dev_name(edev->dev.parent);
+ if (IS_ERR_OR_NULL(edev->name)) {
+ dev_err(&edev->dev,
+ "extcon device name is null\n");
+ return -EINVAL;
+ }
dev_set_name(&edev->dev, "%s", edev->name);
if (edev->max_supported) {
diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
index f874c30..1a83e33 100644
--- a/drivers/extcon/extcon-gpio.c
+++ b/drivers/extcon/extcon-gpio.c
@@ -95,6 +95,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
return -ENOMEM;
extcon_data->edev.name = pdata->name;
+ extcon_data->edev.dev.parent = &pdev->dev;
extcon_data->gpio = pdata->gpio;
extcon_data->state_on = pdata->state_on;
extcon_data->state_off = pdata->state_off;
@@ -102,7 +103,7 @@ static int gpio_extcon_probe(struct platform_device *pdev)
extcon_data->edev.print_state = extcon_gpio_print_state;
extcon_data->debounce_jiffies = msecs_to_jiffies(pdata->debounce);
- ret = extcon_dev_register(&extcon_data->edev, &pdev->dev);
+ ret = extcon_dev_register(&extcon_data->edev);
if (ret < 0)
return ret;
diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
index 15437d4..da268fb 100644
--- a/drivers/extcon/extcon-max77693.c
+++ b/drivers/extcon/extcon-max77693.c
@@ -1183,8 +1183,9 @@ static int max77693_muic_probe(struct platform_device *pdev)
goto err_irq;
}
info->edev->name = DEV_NAME;
+ info->edev->dev.parent = &pdev->dev;
info->edev->supported_cable = max77693_extcon_cable;
- ret = extcon_dev_register(info->edev, NULL);
+ ret = extcon_dev_register(info->edev);
if (ret) {
dev_err(&pdev->dev, "failed to register extcon device\n");
goto err_irq;
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 83e3a27..6a00464 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -707,8 +707,9 @@ static int max8997_muic_probe(struct platform_device *pdev)
goto err_irq;
}
info->edev->name = DEV_NAME;
+ info->edev->dev.parent = &pdev->dev;
info->edev->supported_cable = max8997_extcon_cable;
- ret = extcon_dev_register(info->edev, NULL);
+ ret = extcon_dev_register(info->edev);
if (ret) {
dev_err(&pdev->dev, "failed to register extcon device\n");
goto err_irq;
diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c
index 89fdd05..ac28281 100644
--- a/drivers/extcon/extcon-palmas.c
+++ b/drivers/extcon/extcon-palmas.c
@@ -178,9 +178,10 @@ static int palmas_usb_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, palmas_usb);
palmas_usb->edev.supported_cable = palmas_extcon_cable;
+ palmas_usb->edev.dev.parent = palmas_usb->dev;
palmas_usb->edev.mutually_exclusive = mutually_exclusive;
- status = extcon_dev_register(&palmas_usb->edev, palmas_usb->dev);
+ status = extcon_dev_register(&palmas_usb->edev);
if (status) {
dev_err(&pdev->dev, "failed to register extcon device\n");
return status;
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 0269baf..21c59af 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -183,7 +183,7 @@ struct extcon_specific_cable_nb {
* Following APIs are for notifiers or configurations.
* Notifiers are the external port and connection devices.
*/
-extern int extcon_dev_register(struct extcon_dev *edev, struct device *dev);
+extern int extcon_dev_register(struct extcon_dev *edev);
extern void extcon_dev_unregister(struct extcon_dev *edev);
extern struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name);
@@ -241,8 +241,7 @@ extern int extcon_register_notifier(struct extcon_dev *edev,
extern int extcon_unregister_notifier(struct extcon_dev *edev,
struct notifier_block *nb);
#else /* CONFIG_EXTCON */
-static inline int extcon_dev_register(struct extcon_dev *edev,
- struct device *dev)
+static inline int extcon_dev_register(struct extcon_dev *edev)
{
return 0;
}
--
1.8.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-02 0:20 ` [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter Chanwoo Choi
@ 2013-09-02 0:40 ` Greg KH
2013-09-02 1:13 ` Chanwoo Choi
0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2013-09-02 0:40 UTC (permalink / raw)
To: Chanwoo Choi
Cc: linux-kernel, myungjoo.ham, kyungmin.park, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
On Mon, Sep 02, 2013 at 09:20:08AM +0900, Chanwoo Choi wrote:
> This patch remove extcon_dev_register()'s second parameter which means
> the pointer of parent device to simplify prototype of this function.
No, please don't. You want the parent to be passed in, as the core
needs it when it is registered with the system, otherwise it will not
show up in sysfs properly (i.e. you can't set it afterwards.)
> So, if extcon device has the parent device, it should set the pointer of
> parent device to edev.dev.parent in extcon device driver instead of in
> extcon_dev_register().
No it will break things if you do that :(
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-02 0:40 ` Greg KH
@ 2013-09-02 1:13 ` Chanwoo Choi
2013-09-03 15:57 ` Greg KH
0 siblings, 1 reply; 14+ messages in thread
From: Chanwoo Choi @ 2013-09-02 1:13 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, myungjoo.ham, kyungmin.park, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
Hi Greg,
On 09/02/2013 09:40 AM, Greg KH wrote:
> On Mon, Sep 02, 2013 at 09:20:08AM +0900, Chanwoo Choi wrote:
>> This patch remove extcon_dev_register()'s second parameter which means
>> the pointer of parent device to simplify prototype of this function.
>
> No, please don't. You want the parent to be passed in, as the core
> needs it when it is registered with the system, otherwise it will not
> show up in sysfs properly (i.e. you can't set it afterwards.)
Currently, each extcon driver have allocated memory for extcon device
by using devm_kzalloc() in each extcon device driver.So,I have plan to
implement "devm_extcon_allocate_device()" which allocate managed extcon device
and connect with parent device(->dev.parent = dev).
(I refer to devm_input_allocate_device() in drivers/input/input.c)
So, This patch is precedence work before implementing devm_extcon_allocate_device()
because the pointer of parent device have to pass devm_extcon_allocate_device()
instead of extcon_dev_register().
I'm going to change registration prcedure for extcon device as following:
Before:
ret = extcon_dev_register(edev, dev);
After:
edev = devm_extcon_allocate_device(dev);
...
ret = extcon_dev_register(edev);
...
If you want me to send this feature including in 'devm_extcon_allocate_device()'
at the same time, I'll complete this feature and then post this patchset again.
>
>> So, if extcon device has the parent device, it should set the pointer of
>> parent device to edev.dev.parent in extcon device driver instead of in
>> extcon_dev_register().
>
> No it will break things if you do that :(
Best Regards,
Chanwoo Choi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-02 1:13 ` Chanwoo Choi
@ 2013-09-03 15:57 ` Greg KH
2013-09-04 0:17 ` Chanwoo Choi
0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2013-09-03 15:57 UTC (permalink / raw)
To: Chanwoo Choi
Cc: linux-kernel, myungjoo.ham, kyungmin.park, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
On Mon, Sep 02, 2013 at 10:13:44AM +0900, Chanwoo Choi wrote:
> Hi Greg,
>
> On 09/02/2013 09:40 AM, Greg KH wrote:
> > On Mon, Sep 02, 2013 at 09:20:08AM +0900, Chanwoo Choi wrote:
> >> This patch remove extcon_dev_register()'s second parameter which means
> >> the pointer of parent device to simplify prototype of this function.
> >
> > No, please don't. You want the parent to be passed in, as the core
> > needs it when it is registered with the system, otherwise it will not
> > show up in sysfs properly (i.e. you can't set it afterwards.)
>
> Currently, each extcon driver have allocated memory for extcon device
> by using devm_kzalloc() in each extcon device driver.
That seems backwards, the extcon core should be the one doing the
allocation, and ownership of the device, like all other subsystem cores
do. That makes the driver logic much simpler, and the lifetime
ownership correct (i.e. what happens when a device is unbound from a
driver by userspace? The driver can't control the device memory
anymore...)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-03 15:57 ` Greg KH
@ 2013-09-04 0:17 ` Chanwoo Choi
2013-09-04 1:16 ` Greg KH
0 siblings, 1 reply; 14+ messages in thread
From: Chanwoo Choi @ 2013-09-04 0:17 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, myungjoo.ham, kyungmin.park, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
On 09/04/2013 12:57 AM, Greg KH wrote:
> On Mon, Sep 02, 2013 at 10:13:44AM +0900, Chanwoo Choi wrote:
>> Hi Greg,
>>
>> On 09/02/2013 09:40 AM, Greg KH wrote:
>>> On Mon, Sep 02, 2013 at 09:20:08AM +0900, Chanwoo Choi wrote:
>>>> This patch remove extcon_dev_register()'s second parameter which means
>>>> the pointer of parent device to simplify prototype of this function.
>>>
>>> No, please don't. You want the parent to be passed in, as the core
>>> needs it when it is registered with the system, otherwise it will not
>>> show up in sysfs properly (i.e. you can't set it afterwards.)
>>
>> Currently, each extcon driver have allocated memory for extcon device
>> by using devm_kzalloc() in each extcon device driver.
>
> That seems backwards, the extcon core should be the one doing the
> allocation, and ownership of the device, like all other subsystem cores
> do. That makes the driver logic much simpler, and the lifetime
> ownership correct (i.e. what happens when a device is unbound from a
> driver by userspace? The driver can't control the device memory
> anymore...)
>
OK,
The extcon core will control memory allocation instead of extcon device driver
as following.
- devm_extcon_allocate_device(struct device *dev)
I'll complete this feature about memory allocation for extcon device
and resend it again. Thanks.
Best Regards,
Chanwoo Choi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-04 0:17 ` Chanwoo Choi
@ 2013-09-04 1:16 ` Greg KH
2013-09-04 5:18 ` Chanwoo Choi
0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2013-09-04 1:16 UTC (permalink / raw)
To: Chanwoo Choi
Cc: linux-kernel, myungjoo.ham, kyungmin.park, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
On Wed, Sep 04, 2013 at 09:17:02AM +0900, Chanwoo Choi wrote:
> On 09/04/2013 12:57 AM, Greg KH wrote:
> > On Mon, Sep 02, 2013 at 10:13:44AM +0900, Chanwoo Choi wrote:
> >> Hi Greg,
> >>
> >> On 09/02/2013 09:40 AM, Greg KH wrote:
> >>> On Mon, Sep 02, 2013 at 09:20:08AM +0900, Chanwoo Choi wrote:
> >>>> This patch remove extcon_dev_register()'s second parameter which means
> >>>> the pointer of parent device to simplify prototype of this function.
> >>>
> >>> No, please don't. You want the parent to be passed in, as the core
> >>> needs it when it is registered with the system, otherwise it will not
> >>> show up in sysfs properly (i.e. you can't set it afterwards.)
> >>
> >> Currently, each extcon driver have allocated memory for extcon device
> >> by using devm_kzalloc() in each extcon device driver.
> >
> > That seems backwards, the extcon core should be the one doing the
> > allocation, and ownership of the device, like all other subsystem cores
> > do. That makes the driver logic much simpler, and the lifetime
> > ownership correct (i.e. what happens when a device is unbound from a
> > driver by userspace? The driver can't control the device memory
> > anymore...)
> >
>
> OK,
> The extcon core will control memory allocation instead of extcon device driver
> as following.
> - devm_extcon_allocate_device(struct device *dev)
Huh? Why do you need a devm allocator? This is a "real" child device,
just create it with a "extcon_create_device()" or some such call, like
all other busses do?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-04 1:16 ` Greg KH
@ 2013-09-04 5:18 ` Chanwoo Choi
2013-09-08 21:51 ` Greg KH
0 siblings, 1 reply; 14+ messages in thread
From: Chanwoo Choi @ 2013-09-04 5:18 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, myungjoo.ham, kyungmin.park, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
On 09/04/2013 10:16 AM, Greg KH wrote:
> On Wed, Sep 04, 2013 at 09:17:02AM +0900, Chanwoo Choi wrote:
>> On 09/04/2013 12:57 AM, Greg KH wrote:
>>> On Mon, Sep 02, 2013 at 10:13:44AM +0900, Chanwoo Choi wrote:
>>>> Hi Greg,
>>>>
>>>> On 09/02/2013 09:40 AM, Greg KH wrote:
>>>>> On Mon, Sep 02, 2013 at 09:20:08AM +0900, Chanwoo Choi wrote:
>>>>>> This patch remove extcon_dev_register()'s second parameter which means
>>>>>> the pointer of parent device to simplify prototype of this function.
>>>>>
>>>>> No, please don't. You want the parent to be passed in, as the core
>>>>> needs it when it is registered with the system, otherwise it will not
>>>>> show up in sysfs properly (i.e. you can't set it afterwards.)
>>>>
>>>> Currently, each extcon driver have allocated memory for extcon device
>>>> by using devm_kzalloc() in each extcon device driver.
>>>
>>> That seems backwards, the extcon core should be the one doing the
>>> allocation, and ownership of the device, like all other subsystem cores
>>> do. That makes the driver logic much simpler, and the lifetime
>>> ownership correct (i.e. what happens when a device is unbound from a
>>> driver by userspace? The driver can't control the device memory
>>> anymore...)
>>>
>>
>> OK,
>> The extcon core will control memory allocation instead of extcon device driver
>> as following.
>> - devm_extcon_allocate_device(struct device *dev)
>
> Huh? Why do you need a devm allocator? This is a "real" child device,
> just create it with a "extcon_create_device()" or some such call, like
> all other busses do?
I refer Input/IIO subsystem to check the process of device registration.
Input subsystem has following functions for memory allocation of input device
and input device registration.
Input subsystem
- struct input_dev *devm_input_allocate_device(struct device *device)
- struct input_dev *input_allocate_device(void)
- int input_register_device(struct input_dev *dev)
drivers/input/input.c
devm_input_allocate_device()/input_allocate_device() can allocate
memory for input device in input core. And then created input device
pass input_register_device() as parameter.
So, input device driver haven't executed kmalloc() or devm_kmalloc()
to allocate memory of input device by using input_allocate_device()/
devm_input_allocate_device().
Also,IIO subsystem has separate iio_device_alloc() function
to allocate memory for iio device. But IIO subsystem hasn't
"devm_" allocator. So, If iio device driver fail initialization
in *_probe, should execute iio_device_free() to free allocated memory.
IIO Subsystem
- iio_device_alloc(int sizeof_priv)
- iio_device_register(struct iio_dev *indio_dev);
drivers/iio/industrialio-core.c
So, I think extcon subsystem need extcon_allocate_device()/devm_extcon_allocate_device()
to allocate for memory extcon device. To implement devm_extcon_allocate_device() function
, extcon subsystem need extcon_allocate_device() because devm_extcon_allocate_device()
must call extcon_allocate_device for memory allocation.
If extcon_allocate_device()/devm_extcon_allocate_device() is implemented to extcon core,
I think extcon core can control the memory operation of extcon device.
EXTCON Subsystem
- devm_extcon_allocate_device(struct device *dev)
- extcon_allocate_device(void)
- extcon_dev_register(struct extcon_dev *edev)
If I'm wrong, please correct me. Thanks.
Best Regards,
Chanwoo Choi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-04 5:18 ` Chanwoo Choi
@ 2013-09-08 21:51 ` Greg KH
2013-09-09 2:44 ` Chanwoo Choi
0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2013-09-08 21:51 UTC (permalink / raw)
To: Chanwoo Choi
Cc: linux-kernel, myungjoo.ham, kyungmin.park, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
On Wed, Sep 04, 2013 at 02:18:30PM +0900, Chanwoo Choi wrote:
> On 09/04/2013 10:16 AM, Greg KH wrote:
> > On Wed, Sep 04, 2013 at 09:17:02AM +0900, Chanwoo Choi wrote:
> >> On 09/04/2013 12:57 AM, Greg KH wrote:
> >>> On Mon, Sep 02, 2013 at 10:13:44AM +0900, Chanwoo Choi wrote:
> >>>> Hi Greg,
> >>>>
> >>>> On 09/02/2013 09:40 AM, Greg KH wrote:
> >>>>> On Mon, Sep 02, 2013 at 09:20:08AM +0900, Chanwoo Choi wrote:
> >>>>>> This patch remove extcon_dev_register()'s second parameter which means
> >>>>>> the pointer of parent device to simplify prototype of this function.
> >>>>>
> >>>>> No, please don't. You want the parent to be passed in, as the core
> >>>>> needs it when it is registered with the system, otherwise it will not
> >>>>> show up in sysfs properly (i.e. you can't set it afterwards.)
> >>>>
> >>>> Currently, each extcon driver have allocated memory for extcon device
> >>>> by using devm_kzalloc() in each extcon device driver.
> >>>
> >>> That seems backwards, the extcon core should be the one doing the
> >>> allocation, and ownership of the device, like all other subsystem cores
> >>> do. That makes the driver logic much simpler, and the lifetime
> >>> ownership correct (i.e. what happens when a device is unbound from a
> >>> driver by userspace? The driver can't control the device memory
> >>> anymore...)
> >>>
> >>
> >> OK,
> >> The extcon core will control memory allocation instead of extcon device driver
> >> as following.
> >> - devm_extcon_allocate_device(struct device *dev)
> >
> > Huh? Why do you need a devm allocator? This is a "real" child device,
> > just create it with a "extcon_create_device()" or some such call, like
> > all other busses do?
>
> I refer Input/IIO subsystem to check the process of device registration.
> Input subsystem has following functions for memory allocation of input device
> and input device registration.
>
> Input subsystem
> - struct input_dev *devm_input_allocate_device(struct device *device)
> - struct input_dev *input_allocate_device(void)
> - int input_register_device(struct input_dev *dev)
> drivers/input/input.c
>
> devm_input_allocate_device()/input_allocate_device() can allocate
> memory for input device in input core. And then created input device
> pass input_register_device() as parameter.
>
> So, input device driver haven't executed kmalloc() or devm_kmalloc()
> to allocate memory of input device by using input_allocate_device()/
> devm_input_allocate_device().
>
>
> Also,IIO subsystem has separate iio_device_alloc() function
> to allocate memory for iio device. But IIO subsystem hasn't
> "devm_" allocator. So, If iio device driver fail initialization
> in *_probe, should execute iio_device_free() to free allocated memory.
>
> IIO Subsystem
> - iio_device_alloc(int sizeof_priv)
> - iio_device_register(struct iio_dev *indio_dev);
> drivers/iio/industrialio-core.c
>
>
> So, I think extcon subsystem need extcon_allocate_device()/devm_extcon_allocate_device()
> to allocate for memory extcon device. To implement devm_extcon_allocate_device() function
> , extcon subsystem need extcon_allocate_device() because devm_extcon_allocate_device()
> must call extcon_allocate_device for memory allocation.
>
> If extcon_allocate_device()/devm_extcon_allocate_device() is implemented to extcon core,
> I think extcon core can control the memory operation of extcon device.
>
> EXTCON Subsystem
> - devm_extcon_allocate_device(struct device *dev)
> - extcon_allocate_device(void)
> - extcon_dev_register(struct extcon_dev *edev)
>
> If I'm wrong, please correct me. Thanks.
No, you are totally correct, I was wrong, nevermind, continue on doing a
great job, my mistake :)
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] extcon: Simplify extcon_dev_register() prototype by removing unnecessary parameter
2013-09-08 21:51 ` Greg KH
@ 2013-09-09 2:44 ` Chanwoo Choi
0 siblings, 0 replies; 14+ messages in thread
From: Chanwoo Choi @ 2013-09-09 2:44 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, myungjoo.ham, kyungmin.park, Graeme Gregory,
Kishon Vijay Abraham I, Charles Keepax, Mark Brown
Hi Greg,
On 09/09/2013 06:51 AM, Greg KH wrote:
> On Wed, Sep 04, 2013 at 02:18:30PM +0900, Chanwoo Choi wrote:
>> On 09/04/2013 10:16 AM, Greg KH wrote:
>>> On Wed, Sep 04, 2013 at 09:17:02AM +0900, Chanwoo Choi wrote:
>>>> On 09/04/2013 12:57 AM, Greg KH wrote:
>>>>> On Mon, Sep 02, 2013 at 10:13:44AM +0900, Chanwoo Choi wrote:
>>>>>> Hi Greg,
>>>>>>
>>>>>> On 09/02/2013 09:40 AM, Greg KH wrote:
>>>>>>> On Mon, Sep 02, 2013 at 09:20:08AM +0900, Chanwoo Choi wrote:
>>>>>>>> This patch remove extcon_dev_register()'s second parameter which means
>>>>>>>> the pointer of parent device to simplify prototype of this function.
>>>>>>>
>>>>>>> No, please don't. You want the parent to be passed in, as the core
>>>>>>> needs it when it is registered with the system, otherwise it will not
>>>>>>> show up in sysfs properly (i.e. you can't set it afterwards.)
>>>>>>
>>>>>> Currently, each extcon driver have allocated memory for extcon device
>>>>>> by using devm_kzalloc() in each extcon device driver.
>>>>>
>>>>> That seems backwards, the extcon core should be the one doing the
>>>>> allocation, and ownership of the device, like all other subsystem cores
>>>>> do. That makes the driver logic much simpler, and the lifetime
>>>>> ownership correct (i.e. what happens when a device is unbound from a
>>>>> driver by userspace? The driver can't control the device memory
>>>>> anymore...)
>>>>>
>>>>
>>>> OK,
>>>> The extcon core will control memory allocation instead of extcon device driver
>>>> as following.
>>>> - devm_extcon_allocate_device(struct device *dev)
>>>
>>> Huh? Why do you need a devm allocator? This is a "real" child device,
>>> just create it with a "extcon_create_device()" or some such call, like
>>> all other busses do?
>>
>> I refer Input/IIO subsystem to check the process of device registration.
>> Input subsystem has following functions for memory allocation of input device
>> and input device registration.
>>
>> Input subsystem
>> - struct input_dev *devm_input_allocate_device(struct device *device)
>> - struct input_dev *input_allocate_device(void)
>> - int input_register_device(struct input_dev *dev)
>> drivers/input/input.c
>>
>> devm_input_allocate_device()/input_allocate_device() can allocate
>> memory for input device in input core. And then created input device
>> pass input_register_device() as parameter.
>>
>> So, input device driver haven't executed kmalloc() or devm_kmalloc()
>> to allocate memory of input device by using input_allocate_device()/
>> devm_input_allocate_device().
>>
>>
>> Also,IIO subsystem has separate iio_device_alloc() function
>> to allocate memory for iio device. But IIO subsystem hasn't
>> "devm_" allocator. So, If iio device driver fail initialization
>> in *_probe, should execute iio_device_free() to free allocated memory.
>>
>> IIO Subsystem
>> - iio_device_alloc(int sizeof_priv)
>> - iio_device_register(struct iio_dev *indio_dev);
>> drivers/iio/industrialio-core.c
>>
>>
>> So, I think extcon subsystem need extcon_allocate_device()/devm_extcon_allocate_device()
>> to allocate for memory extcon device. To implement devm_extcon_allocate_device() function
>> , extcon subsystem need extcon_allocate_device() because devm_extcon_allocate_device()
>> must call extcon_allocate_device for memory allocation.
>>
>> If extcon_allocate_device()/devm_extcon_allocate_device() is implemented to extcon core,
>> I think extcon core can control the memory operation of extcon device.
>>
>> EXTCON Subsystem
>> - devm_extcon_allocate_device(struct device *dev)
>> - extcon_allocate_device(void)
>> - extcon_dev_register(struct extcon_dev *edev)
>>
>> If I'm wrong, please correct me. Thanks.
>
> No, you are totally correct, I was wrong, nevermind, continue on doing a
> great job, my mistake :)
>
> greg k-h
Thanks for your comment always.
Best Regards,
Chanwoo Choi
^ permalink raw reply [flat|nested] 14+ messages in thread