[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4632ce69-2a94-0c33-9a76-b97596436e24@gmail.com>
Date: Fri, 7 Apr 2023 04:26:35 +0900
From: Chanwoo Choi <cwchoi00@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Chanwoo Choi <cw00.choi@...sung.com>,
linux-kernel@...r.kernel.org
Cc: MyungJoo Ham <myungjoo.ham@...sung.com>
Subject: Re: [PATCH v2 2/5] extcon: Get rid of not really used name field in
struct extcon_dev
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 <andriy.shevchenko@...ux.intel.com>
> ---
> 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
Powered by blists - more mailing lists