The name field is always set to the parent device name and never
altered. No need to keep it inside the struct extcon_dev as we
always may derive it from the dev_name(edev->dev.parent) call.
Moreover, the parent device pointer won't ever be NULL, otherwise
we may not allocate the extcon device at all.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/extcon/extcon.c | 12 +++---------
drivers/extcon/extcon.h | 3 ---
2 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 47819c5144d5..75a0147703c0 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -387,7 +387,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
{
struct extcon_dev *edev = dev_get_drvdata(dev);
- return sysfs_emit(buf, "%s\n", edev->name);
+ return sysfs_emit(buf, "%s\n", dev_name(edev->dev.parent));
}
static DEVICE_ATTR_RO(name);
@@ -885,7 +885,7 @@ struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
mutex_lock(&extcon_dev_list_lock);
list_for_each_entry(sd, &extcon_dev_list, entry) {
- if (!strcmp(sd->name, extcon_name))
+ if (device_match_name(sd->dev.parent, extcon_name))
goto out;
}
sd = ERR_PTR(-EPROBE_DEFER);
@@ -1269,12 +1269,6 @@ int extcon_dev_register(struct extcon_dev *edev)
edev->dev.class = extcon_class;
edev->dev.release = extcon_dev_release;
- 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, "extcon%lu",
(unsigned long)atomic_inc_return(&edev_no));
@@ -1465,7 +1459,7 @@ EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
*/
const char *extcon_get_edev_name(struct extcon_dev *edev)
{
- return !edev ? NULL : edev->name;
+ return edev ? dev_name(edev->dev.parent) : NULL;
}
EXPORT_SYMBOL_GPL(extcon_get_edev_name);
diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 49e4ed9f6450..9ce7042606d7 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -6,8 +6,6 @@
/**
* struct extcon_dev - An extcon device represents one external connector.
- * @name: The name of this extcon device. Parent device name is
- * used if NULL.
* @supported_cable: Array of supported cable names ending with EXTCON_NONE.
* If supported_cable is NULL, cable name related APIs
* are disabled.
@@ -40,7 +38,6 @@
*/
struct extcon_dev {
/* Optional user initializing data */
- const char *name;
const unsigned int *supported_cable;
const u32 *mutually_exclusive;
--
2.40.0.1.gaa8946217a0b
Hi,
On 23. 4. 6. 00:27, Andy Shevchenko wrote:
> The name field is always set to the parent device name and never
> altered. No need to keep it inside the struct extcon_dev as we
> always may derive it from the dev_name(edev->dev.parent) call.
>
> Moreover, the parent device pointer won't ever be NULL, otherwise
> we may not allocate the extcon device at all.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/extcon/extcon.c | 12 +++---------
> drivers/extcon/extcon.h | 3 ---
> 2 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 47819c5144d5..75a0147703c0 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -387,7 +387,7 @@ static ssize_t name_show(struct device *dev, struct device_attribute *attr,
> {
> struct extcon_dev *edev = dev_get_drvdata(dev);
>
> - return sysfs_emit(buf, "%s\n", edev->name);
> + return sysfs_emit(buf, "%s\n", dev_name(edev->dev.parent));
> }
> static DEVICE_ATTR_RO(name);
>
> @@ -885,7 +885,7 @@ struct extcon_dev *extcon_get_extcon_dev(const char *extcon_name)
>
> mutex_lock(&extcon_dev_list_lock);
> list_for_each_entry(sd, &extcon_dev_list, entry) {
> - if (!strcmp(sd->name, extcon_name))
> + if (device_match_name(sd->dev.parent, extcon_name))
> goto out;
> }
> sd = ERR_PTR(-EPROBE_DEFER);
> @@ -1269,12 +1269,6 @@ int extcon_dev_register(struct extcon_dev *edev)
> edev->dev.class = extcon_class;
> edev->dev.release = extcon_dev_release;
>
> - 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, "extcon%lu",
> (unsigned long)atomic_inc_return(&edev_no));
>
> @@ -1465,7 +1459,7 @@ EXPORT_SYMBOL_GPL(extcon_get_edev_by_phandle);
> */
> const char *extcon_get_edev_name(struct extcon_dev *edev)
> {
> - return !edev ? NULL : edev->name;
> + return edev ? dev_name(edev->dev.parent) : NULL;
> }
> EXPORT_SYMBOL_GPL(extcon_get_edev_name);
>
> diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
> index 49e4ed9f6450..9ce7042606d7 100644
> --- a/drivers/extcon/extcon.h
> +++ b/drivers/extcon/extcon.h
> @@ -6,8 +6,6 @@
>
> /**
> * struct extcon_dev - An extcon device represents one external connector.
> - * @name: The name of this extcon device. Parent device name is
> - * used if NULL.
> * @supported_cable: Array of supported cable names ending with EXTCON_NONE.
> * If supported_cable is NULL, cable name related APIs
> * are disabled.
> @@ -40,7 +38,6 @@
> */
> struct extcon_dev {
> /* Optional user initializing data */
> - const char *name;
No I don't want to remove the name even if the edev->name is equal
with the parent's name. I might reduce the readability and understaning
of the code user and I think that it is not good to use 'dev.parent' directly
at multiple point. I think that it just better to edit the wrong description
as following:
diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 15616446140d..f03d596d148f 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -6,8 +6,7 @@
/**
* 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.
* @supported_cable: Array of supported cable names ending with EXTCON_NONE.
* If supported_cable is NULL, cable name related APIs
* are disabled.
@@ -39,7 +38,6 @@
* are overwritten by register function.
*/
struct extcon_dev {
- /* Optional user initializing data */
const char *name;
const unsigned int *supported_cable;
const u32 *mutually_exclusive;
> const unsigned int *supported_cable;
> const u32 *mutually_exclusive;
--
Best Regards,
Samsung Electronics
Chanwoo Choi
On Fri, Apr 07, 2023 at 04:26:35AM +0900, Chanwoo Choi wrote:
> On 23. 4. 6. 00:27, Andy Shevchenko wrote:
...
> > - const char *name;
>
> No I don't want to remove the name even if the edev->name is equal
> with the parent's name. I might reduce the readability and understaning
> of the code user and I think that it is not good to use 'dev.parent' directly
> at multiple point.
Obviously I think otherwise. But you are the extcon maintainer, so I assume
that I have got NAK for these patches.
--
With Best Regards,
Andy Shevchenko
On Fri, Apr 07, 2023 at 04:26:35AM +0900, Chanwoo Choi wrote:
> On 23. 4. 6. 00:27, Andy Shevchenko wrote:
...
> > - if (IS_ERR_OR_NULL(edev->name)) {
> > - dev_err(&edev->dev,
> > - "extcon device name is null\n");
> > - return -EINVAL;
> > - }
JFYI: this is a dead code in case you want to clean it up.
Since the patch is NAKed, I'm not going to split it. Hence
just for your information.
--
With Best Regards,
Andy Shevchenko