[PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role

[PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role

[Xu Yang]

It should not return -EINVAL if the request role is the same with current
role, return non-error and without do anything instead.

Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
cc: <stable@vger.kernel.org>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
changes since v1:
- no change
---
 drivers/usb/chipidea/core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 484b1cd23431..fcb175b22188 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -984,9 +984,12 @@ static ssize_t role_store(struct device *dev,
 			     strlen(ci->roles[role]->name)))
 			break;
 
-	if (role == CI_ROLE_END || role == ci->role)
+	if (role == CI_ROLE_END)
 		return -EINVAL;
 
+	if (role == ci->role)
+		return n;
+
 	pm_runtime_get_sync(dev);
 	disable_irq(ci->irq);
 	ci_role_stop(ci);
-- 
2.34.1

[PATCH v2 2/3] usb: chipidea: core: fix possible concurrent when switch role

[Xu Yang]

The user may call role_store() when driver is handling
ci_handle_id_switch() which is triggerred by otg event or power lost
event. Unfortunately, the controller may go into chaos in this case.
Fix this by protecting it with mutex lock.

Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
cc: <stable@vger.kernel.org>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
changes since v1:
- modify description for mutex member
- wrap role variable in ci_handle_id_switch() too
---
 drivers/usb/chipidea/ci.h   | 2 ++
 drivers/usb/chipidea/core.c | 8 +++++++-
 drivers/usb/chipidea/otg.c  | 5 ++++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 005c67cb3afb..f210b7489fd5 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -208,6 +208,7 @@ struct hw_bank {
  * @in_lpm: if the core in low power mode
  * @wakeup_int: if wakeup interrupt occur
  * @rev: The revision number for controller
+ * @mutex: protect code from concorrent running when doing role switch
  */
 struct ci_hdrc {
 	struct device			*dev;
@@ -260,6 +261,7 @@ struct ci_hdrc {
 	bool				in_lpm;
 	bool				wakeup_int;
 	enum ci_revision		rev;
+	struct mutex                    mutex;
 };
 
 static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index fcb175b22188..d7efde30d59f 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -987,8 +987,12 @@ static ssize_t role_store(struct device *dev,
 	if (role == CI_ROLE_END)
 		return -EINVAL;
 
-	if (role == ci->role)
+	mutex_lock(&ci->mutex);
+
+	if (role == ci->role) {
+		mutex_unlock(&ci->mutex);
 		return n;
+	}
 
 	pm_runtime_get_sync(dev);
 	disable_irq(ci->irq);
@@ -998,6 +1002,7 @@ static ssize_t role_store(struct device *dev,
 		ci_handle_vbus_change(ci);
 	enable_irq(ci->irq);
 	pm_runtime_put_sync(dev);
+	mutex_unlock(&ci->mutex);
 
 	return (ret == 0) ? n : ret;
 }
@@ -1033,6 +1038,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	spin_lock_init(&ci->lock);
+	mutex_init(&ci->mutex);
 	ci->dev = dev;
 	ci->platdata = dev_get_platdata(dev);
 	ci->imx28_write_fix = !!(ci->platdata->flags &
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 622c3b68aa1e..f5490f2a5b6b 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -167,8 +167,10 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
 
 void ci_handle_id_switch(struct ci_hdrc *ci)
 {
-	enum ci_role role = ci_otg_role(ci);
+	enum ci_role role;
 
+	mutex_lock(&ci->mutex);
+	role = ci_otg_role(ci);
 	if (role != ci->role) {
 		dev_dbg(ci->dev, "switching from %s to %s\n",
 			ci_role(ci)->name, ci->roles[role]->name);
@@ -198,6 +200,7 @@ void ci_handle_id_switch(struct ci_hdrc *ci)
 		if (role == CI_ROLE_GADGET)
 			ci_handle_vbus_change(ci);
 	}
+	mutex_unlock(&ci->mutex);
 }
 /**
  * ci_otg_work - perform otg (vbus/id) event handle
-- 
2.34.1

RE: [PATCH v2 2/3] usb: chipidea: core: fix possible concurrent when switch role

[Xu Yang]

> -----Original Message-----
> From: Xu Yang
> Sent: Wednesday, November 30, 2022 4:12 PM
> To: peter.chen@kernel.org
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>;
> Xu Yang <xu.yang_2@nxp.com>
> Subject: [PATCH v2 2/3] usb: chipidea: core: fix possible concurrent when switch role
> 
> The user may call role_store() when driver is handling
> ci_handle_id_switch() which is triggerred by otg event or power lost
> event. Unfortunately, the controller may go into chaos in this case.
> Fix this by protecting it with mutex lock.
> 
> Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
> cc: <stable@vger.kernel.org>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> changes since v1:
> - modify description for mutex member
> - wrap role variable in ci_handle_id_switch() too
> ---
>  drivers/usb/chipidea/ci.h   | 2 ++
>  drivers/usb/chipidea/core.c | 8 +++++++-
>  drivers/usb/chipidea/otg.c  | 5 ++++-
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 005c67cb3afb..f210b7489fd5 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -208,6 +208,7 @@ struct hw_bank {
>   * @in_lpm: if the core in low power mode
>   * @wakeup_int: if wakeup interrupt occur
>   * @rev: The revision number for controller
> + * @mutex: protect code from concorrent running when doing role switch
>   */
>  struct ci_hdrc {
>  	struct device			*dev;
> @@ -260,6 +261,7 @@ struct ci_hdrc {
>  	bool				in_lpm;
>  	bool				wakeup_int;
>  	enum ci_revision		rev;
> +	struct mutex                    mutex;
>  };
> 
>  static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index fcb175b22188..d7efde30d59f 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -987,8 +987,12 @@ static ssize_t role_store(struct device *dev,
>  	if (role == CI_ROLE_END)
>  		return -EINVAL;
> 
> -	if (role == ci->role)
> +	mutex_lock(&ci->mutex);
> +
> +	if (role == ci->role) {
> +		mutex_unlock(&ci->mutex);
>  		return n;
> +	}
> 
>  	pm_runtime_get_sync(dev);
>  	disable_irq(ci->irq);
> @@ -998,6 +1002,7 @@ static ssize_t role_store(struct device *dev,
>  		ci_handle_vbus_change(ci);
>  	enable_irq(ci->irq);
>  	pm_runtime_put_sync(dev);
> +	mutex_unlock(&ci->mutex);
> 
>  	return (ret == 0) ? n : ret;
>  }
> @@ -1033,6 +1038,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  		return -ENOMEM;
> 
>  	spin_lock_init(&ci->lock);
> +	mutex_init(&ci->mutex);
>  	ci->dev = dev;
>  	ci->platdata = dev_get_platdata(dev);
>  	ci->imx28_write_fix = !!(ci->platdata->flags &
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 622c3b68aa1e..f5490f2a5b6b 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -167,8 +167,10 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
> 
>  void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
> -	enum ci_role role = ci_otg_role(ci);
> +	enum ci_role role;
> 
> +	mutex_lock(&ci->mutex);
> +	role = ci_otg_role(ci);
>  	if (role != ci->role) {
>  		dev_dbg(ci->dev, "switching from %s to %s\n",
>  			ci_role(ci)->name, ci->roles[role]->name);
> @@ -198,6 +200,7 @@ void ci_handle_id_switch(struct ci_hdrc *ci)
>  		if (role == CI_ROLE_GADGET)
>  			ci_handle_vbus_change(ci);
>  	}
> +	mutex_unlock(&ci->mutex);
>  }
>  /**
>   * ci_otg_work - perform otg (vbus/id) event handle
> --
> 2.34.1

A gentle ping.

Re: [PATCH v2 2/3] usb: chipidea: core: fix possible concurrent when switch role

[Peter Chen]

On 22-11-30 16:12:30, Xu Yang wrote:
> The user may call role_store() when driver is handling
> ci_handle_id_switch() which is triggerred by otg event or power lost
> event. Unfortunately, the controller may go into chaos in this case.
> Fix this by protecting it with mutex lock.
> 
> Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
> cc: <stable@vger.kernel.org>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> 
> ---
> changes since v1:
> - modify description for mutex member
> - wrap role variable in ci_handle_id_switch() too
> ---
>  drivers/usb/chipidea/ci.h   | 2 ++
>  drivers/usb/chipidea/core.c | 8 +++++++-
>  drivers/usb/chipidea/otg.c  | 5 ++++-
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 005c67cb3afb..f210b7489fd5 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -208,6 +208,7 @@ struct hw_bank {
>   * @in_lpm: if the core in low power mode
>   * @wakeup_int: if wakeup interrupt occur
>   * @rev: The revision number for controller
> + * @mutex: protect code from concorrent running when doing role switch
>   */
>  struct ci_hdrc {
>  	struct device			*dev;
> @@ -260,6 +261,7 @@ struct ci_hdrc {
>  	bool				in_lpm;
>  	bool				wakeup_int;
>  	enum ci_revision		rev;
> +	struct mutex                    mutex;
>  };
>  
>  static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index fcb175b22188..d7efde30d59f 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -987,8 +987,12 @@ static ssize_t role_store(struct device *dev,
>  	if (role == CI_ROLE_END)
>  		return -EINVAL;
>  
> -	if (role == ci->role)
> +	mutex_lock(&ci->mutex);
> +
> +	if (role == ci->role) {
> +		mutex_unlock(&ci->mutex);
>  		return n;
> +	}
>  
>  	pm_runtime_get_sync(dev);
>  	disable_irq(ci->irq);
> @@ -998,6 +1002,7 @@ static ssize_t role_store(struct device *dev,
>  		ci_handle_vbus_change(ci);
>  	enable_irq(ci->irq);
>  	pm_runtime_put_sync(dev);
> +	mutex_unlock(&ci->mutex);
>  
>  	return (ret == 0) ? n : ret;
>  }
> @@ -1033,6 +1038,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	spin_lock_init(&ci->lock);
> +	mutex_init(&ci->mutex);
>  	ci->dev = dev;
>  	ci->platdata = dev_get_platdata(dev);
>  	ci->imx28_write_fix = !!(ci->platdata->flags &
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 622c3b68aa1e..f5490f2a5b6b 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -167,8 +167,10 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
>  
>  void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
> -	enum ci_role role = ci_otg_role(ci);
> +	enum ci_role role;
>  
> +	mutex_lock(&ci->mutex);
> +	role = ci_otg_role(ci);
>  	if (role != ci->role) {
>  		dev_dbg(ci->dev, "switching from %s to %s\n",
>  			ci_role(ci)->name, ci->roles[role]->name);
> @@ -198,6 +200,7 @@ void ci_handle_id_switch(struct ci_hdrc *ci)
>  		if (role == CI_ROLE_GADGET)
>  			ci_handle_vbus_change(ci);
>  	}
> +	mutex_unlock(&ci->mutex);
>  }
>  /**
>   * ci_otg_work - perform otg (vbus/id) event handle
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

[PATCH v2 3/3] usb: chipidea: debug: remove redundant 'role' debug file

[Xu Yang]

Two 'role' file exist in different position but with totally same function.

1. /sys/devices/platform/soc@0/xxxxxxxx.usb/ci_hdrc.0/role
2. /sys/kernel/debug/usb/ci_hdrc.0/role

This will remove the 2rd redundant 'role' debug file (under debugfs) and
keep the one which is more closer to user.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
changes since v1:
- no change
---
 drivers/usb/chipidea/debug.c | 55 ------------------------------------
 1 file changed, 55 deletions(-)

diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
index faf6b078b6c4..37da83de3cba 100644
--- a/drivers/usb/chipidea/debug.c
+++ b/drivers/usb/chipidea/debug.c
@@ -247,60 +247,6 @@ static int ci_otg_show(struct seq_file *s, void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(ci_otg);
 
-static int ci_role_show(struct seq_file *s, void *data)
-{
-	struct ci_hdrc *ci = s->private;
-
-	if (ci->role != CI_ROLE_END)
-		seq_printf(s, "%s\n", ci_role(ci)->name);
-
-	return 0;
-}
-
-static ssize_t ci_role_write(struct file *file, const char __user *ubuf,
-			     size_t count, loff_t *ppos)
-{
-	struct seq_file *s = file->private_data;
-	struct ci_hdrc *ci = s->private;
-	enum ci_role role;
-	char buf[8];
-	int ret;
-
-	if (copy_from_user(buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
-		return -EFAULT;
-
-	for (role = CI_ROLE_HOST; role < CI_ROLE_END; role++)
-		if (ci->roles[role] &&
-		    !strncmp(buf, ci->roles[role]->name,
-			     strlen(ci->roles[role]->name)))
-			break;
-
-	if (role == CI_ROLE_END || role == ci->role)
-		return -EINVAL;
-
-	pm_runtime_get_sync(ci->dev);
-	disable_irq(ci->irq);
-	ci_role_stop(ci);
-	ret = ci_role_start(ci, role);
-	enable_irq(ci->irq);
-	pm_runtime_put_sync(ci->dev);
-
-	return ret ? ret : count;
-}
-
-static int ci_role_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, ci_role_show, inode->i_private);
-}
-
-static const struct file_operations ci_role_fops = {
-	.open		= ci_role_open,
-	.write		= ci_role_write,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
-
 static int ci_registers_show(struct seq_file *s, void *unused)
 {
 	struct ci_hdrc *ci = s->private;
@@ -354,7 +300,6 @@ void dbg_create_files(struct ci_hdrc *ci)
 	if (ci_otg_is_fsm_mode(ci))
 		debugfs_create_file("otg", S_IRUGO, dir, ci, &ci_otg_fops);
 
-	debugfs_create_file("role", S_IRUGO | S_IWUSR, dir, ci, &ci_role_fops);
 	debugfs_create_file("registers", S_IRUGO, dir, ci, &ci_registers_fops);
 }
 
-- 
2.34.1

Re: [PATCH v2 3/3] usb: chipidea: debug: remove redundant 'role' debug file

[Peter Chen]

On 22-11-30 16:12:31, Xu Yang wrote:
> Two 'role' file exist in different position but with totally same function.
> 
> 1. /sys/devices/platform/soc@0/xxxxxxxx.usb/ci_hdrc.0/role
> 2. /sys/kernel/debug/usb/ci_hdrc.0/role
> 
> This will remove the 2rd redundant 'role' debug file (under debugfs) and
> keep the one which is more closer to user.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> 
> ---
> changes since v1:
> - no change
> ---
>  drivers/usb/chipidea/debug.c | 55 ------------------------------------
>  1 file changed, 55 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
> index faf6b078b6c4..37da83de3cba 100644
> --- a/drivers/usb/chipidea/debug.c
> +++ b/drivers/usb/chipidea/debug.c
> @@ -247,60 +247,6 @@ static int ci_otg_show(struct seq_file *s, void *unused)
>  }
>  DEFINE_SHOW_ATTRIBUTE(ci_otg);
>  
> -static int ci_role_show(struct seq_file *s, void *data)
> -{
> -	struct ci_hdrc *ci = s->private;
> -
> -	if (ci->role != CI_ROLE_END)
> -		seq_printf(s, "%s\n", ci_role(ci)->name);
> -
> -	return 0;
> -}
> -
> -static ssize_t ci_role_write(struct file *file, const char __user *ubuf,
> -			     size_t count, loff_t *ppos)
> -{
> -	struct seq_file *s = file->private_data;
> -	struct ci_hdrc *ci = s->private;
> -	enum ci_role role;
> -	char buf[8];
> -	int ret;
> -
> -	if (copy_from_user(buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
> -		return -EFAULT;
> -
> -	for (role = CI_ROLE_HOST; role < CI_ROLE_END; role++)
> -		if (ci->roles[role] &&
> -		    !strncmp(buf, ci->roles[role]->name,
> -			     strlen(ci->roles[role]->name)))
> -			break;
> -
> -	if (role == CI_ROLE_END || role == ci->role)
> -		return -EINVAL;
> -
> -	pm_runtime_get_sync(ci->dev);
> -	disable_irq(ci->irq);
> -	ci_role_stop(ci);
> -	ret = ci_role_start(ci, role);
> -	enable_irq(ci->irq);
> -	pm_runtime_put_sync(ci->dev);
> -
> -	return ret ? ret : count;
> -}
> -
> -static int ci_role_open(struct inode *inode, struct file *file)
> -{
> -	return single_open(file, ci_role_show, inode->i_private);
> -}
> -
> -static const struct file_operations ci_role_fops = {
> -	.open		= ci_role_open,
> -	.write		= ci_role_write,
> -	.read		= seq_read,
> -	.llseek		= seq_lseek,
> -	.release	= single_release,
> -};
> -
>  static int ci_registers_show(struct seq_file *s, void *unused)
>  {
>  	struct ci_hdrc *ci = s->private;
> @@ -354,7 +300,6 @@ void dbg_create_files(struct ci_hdrc *ci)
>  	if (ci_otg_is_fsm_mode(ci))
>  		debugfs_create_file("otg", S_IRUGO, dir, ci, &ci_otg_fops);
>  
> -	debugfs_create_file("role", S_IRUGO | S_IWUSR, dir, ci, &ci_role_fops);
>  	debugfs_create_file("registers", S_IRUGO, dir, ci, &ci_registers_fops);
>  }
>  
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role

[Peter Chen]

On 22-11-30 16:12:29, Xu Yang wrote:
> It should not return -EINVAL if the request role is the same with current
> role, return non-error and without do anything instead.
> 
> Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
> cc: <stable@vger.kernel.org>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> 
> ---
> changes since v1:
> - no change
> ---
>  drivers/usb/chipidea/core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 484b1cd23431..fcb175b22188 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -984,9 +984,12 @@ static ssize_t role_store(struct device *dev,
>  			     strlen(ci->roles[role]->name)))
>  			break;
>  
> -	if (role == CI_ROLE_END || role == ci->role)
> +	if (role == CI_ROLE_END)
>  		return -EINVAL;
>  
> +	if (role == ci->role)
> +		return n;
> +
>  	pm_runtime_get_sync(dev);
>  	disable_irq(ci->irq);
>  	ci_role_stop(ci);
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

RE: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role

[Xu Yang]

Hi Greg,

> -----Original Message-----
> From: Peter Chen <peter.chen@kernel.org>
> Sent: Friday, February 10, 2023 4:55 PM
> To: Xu Yang <xu.yang_2@nxp.com>
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> Subject: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role
> 
> Caution: EXT Email
> 
> On 22-11-30 16:12:29, Xu Yang wrote:
> > It should not return -EINVAL if the request role is the same with current
> > role, return non-error and without do anything instead.
> >
> > Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
> > cc: <stable@vger.kernel.org>
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> Acked-by: Peter Chen <peter.chen@kernel.org>

Could you please add these three patches to your repo?

Thanks,
Xu Yang

> 
> Peter
> >
> > ---
> > changes since v1:
> > - no change
> > ---
> >  drivers/usb/chipidea/core.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 484b1cd23431..fcb175b22188 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -984,9 +984,12 @@ static ssize_t role_store(struct device *dev,
> >                            strlen(ci->roles[role]->name)))
> >                       break;
> >
> > -     if (role == CI_ROLE_END || role == ci->role)
> > +     if (role == CI_ROLE_END)
> >               return -EINVAL;
> >
> > +     if (role == ci->role)
> > +             return n;
> > +
> >       pm_runtime_get_sync(dev);
> >       disable_irq(ci->irq);
> >       ci_role_stop(ci);
> > --
> > 2.34.1
> >
> 
> --
> 
> Thanks,
> Peter Chen

Re: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role

[gregkh]

On Thu, Mar 16, 2023 at 09:57:05AM +0000, Xu Yang wrote:
> Hi Greg,
> 
> > -----Original Message-----
> > From: Peter Chen <peter.chen@kernel.org>
> > Sent: Friday, February 10, 2023 4:55 PM
> > To: Xu Yang <xu.yang_2@nxp.com>
> > Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li <jun.li@nxp.com>
> > Subject: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role
> > 
> > Caution: EXT Email
> > 
> > On 22-11-30 16:12:29, Xu Yang wrote:
> > > It should not return -EINVAL if the request role is the same with current
> > > role, return non-error and without do anything instead.
> > >
> > > Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
> > > cc: <stable@vger.kernel.org>
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > 
> > Acked-by: Peter Chen <peter.chen@kernel.org>
> 
> Could you please add these three patches to your repo?

Can you please resend them, I don't seem to have them anymore.  Also
split this into 2 different series, one for bugfixes-only, that needs to
go to Linus for 6.3-final, and the others that are new features, to go
for 6.4-rc1.

thanks,

greg k-h

RE: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role

[Xu Yang]

Hi Greg,

> -----Original Message-----
> From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> Sent: Thursday, March 16, 2023 7:55 PM
> To: Xu Yang <xu.yang_2@nxp.com>
> Cc: Peter Chen <peter.chen@kernel.org>; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li
> <jun.li@nxp.com>
> Subject: Re: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role
> 
> Caution: EXT Email
> 
> On Thu, Mar 16, 2023 at 09:57:05AM +0000, Xu Yang wrote:
> > Hi Greg,
> >
> > > -----Original Message-----
> > > From: Peter Chen <peter.chen@kernel.org>
> > > Sent: Friday, February 10, 2023 4:55 PM
> > > To: Xu Yang <xu.yang_2@nxp.com>
> > > Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li
> <jun.li@nxp.com>
> > > Subject: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role
> > >
> > > Caution: EXT Email
> > >
> > > On 22-11-30 16:12:29, Xu Yang wrote:
> > > > It should not return -EINVAL if the request role is the same with current
> > > > role, return non-error and without do anything instead.
> > > >
> > > > Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
> > > > cc: <stable@vger.kernel.org>
> > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > >
> > > Acked-by: Peter Chen <peter.chen@kernel.org>
> >
> > Could you please add these three patches to your repo?
> 
> Can you please resend them, I don't seem to have them anymore.  Also
> split this into 2 different series, one for bugfixes-only, that needs to
> go to Linus for 6.3-final, and the others that are new features, to go
> for 6.4-rc1.

Patch 2/3 is a bugfix, but it depends on patch 1/3. I cannot simply
separate them. After I resend them, could you please let them all go
to Linus for 6.4-rc1?

Thanks,
Xu Yang

> 
> thanks,
> 
> greg k-h

Re: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role

[gregkh]

On Fri, Mar 17, 2023 at 02:37:47AM +0000, Xu Yang wrote:
> Hi Greg,
> 
> > -----Original Message-----
> > From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> > Sent: Thursday, March 16, 2023 7:55 PM
> > To: Xu Yang <xu.yang_2@nxp.com>
> > Cc: Peter Chen <peter.chen@kernel.org>; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li
> > <jun.li@nxp.com>
> > Subject: Re: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role
> > 
> > Caution: EXT Email
> > 
> > On Thu, Mar 16, 2023 at 09:57:05AM +0000, Xu Yang wrote:
> > > Hi Greg,
> > >
> > > > -----Original Message-----
> > > > From: Peter Chen <peter.chen@kernel.org>
> > > > Sent: Friday, February 10, 2023 4:55 PM
> > > > To: Xu Yang <xu.yang_2@nxp.com>
> > > > Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li
> > <jun.li@nxp.com>
> > > > Subject: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role
> > > >
> > > > Caution: EXT Email
> > > >
> > > > On 22-11-30 16:12:29, Xu Yang wrote:
> > > > > It should not return -EINVAL if the request role is the same with current
> > > > > role, return non-error and without do anything instead.
> > > > >
> > > > > Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
> > > > > cc: <stable@vger.kernel.org>
> > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > >
> > > > Acked-by: Peter Chen <peter.chen@kernel.org>
> > >
> > > Could you please add these three patches to your repo?
> > 
> > Can you please resend them, I don't seem to have them anymore.  Also
> > split this into 2 different series, one for bugfixes-only, that needs to
> > go to Linus for 6.3-final, and the others that are new features, to go
> > for 6.4-rc1.
> 
> Patch 2/3 is a bugfix, but it depends on patch 1/3. I cannot simply
> separate them. After I resend them, could you please let them all go
> to Linus for 6.4-rc1?

Resend and I will review, but that sounds like you also need to mark
patch 1 as a fix, and then patch 3 is not part of the series and can be
separate, right?

What would you do if you were in my position here?  Make it easy for
maintainers please :)

thanks,

greg k-h

RE: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role

[Xu Yang]

> -----Original Message-----
> From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> Sent: Friday, March 17, 2023 12:51 PM
> To: Xu Yang <xu.yang_2@nxp.com>
> Cc: Peter Chen <peter.chen@kernel.org>; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li
> <jun.li@nxp.com>
> Subject: Re: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current role
> 
> Caution: EXT Email
> 
> On Fri, Mar 17, 2023 at 02:37:47AM +0000, Xu Yang wrote:
> > Hi Greg,
> >
> > > -----Original Message-----
> > > From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> > > Sent: Thursday, March 16, 2023 7:55 PM
> > > To: Xu Yang <xu.yang_2@nxp.com>
> > > Cc: Peter Chen <peter.chen@kernel.org>; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li
> > > <jun.li@nxp.com>
> > > Subject: Re: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current
> role
> > >
> > > Caution: EXT Email
> > >
> > > On Thu, Mar 16, 2023 at 09:57:05AM +0000, Xu Yang wrote:
> > > > Hi Greg,
> > > >
> > > > > -----Original Message-----
> > > > > From: Peter Chen <peter.chen@kernel.org>
> > > > > Sent: Friday, February 10, 2023 4:55 PM
> > > > > To: Xu Yang <xu.yang_2@nxp.com>
> > > > > Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Jun Li
> > > <jun.li@nxp.com>
> > > > > Subject: [EXT] Re: [PATCH v2 1/3] usb: chipdea: core: don't return -EINVAL if request role is the same with current
> role
> > > > >
> > > > > Caution: EXT Email
> > > > >
> > > > > On 22-11-30 16:12:29, Xu Yang wrote:
> > > > > > It should not return -EINVAL if the request role is the same with current
> > > > > > role, return non-error and without do anything instead.
> > > > > >
> > > > > > Fixes: a932a8041ff9 ("usb: chipidea: core: add sysfs group")
> > > > > > cc: <stable@vger.kernel.org>
> > > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > > >
> > > > > Acked-by: Peter Chen <peter.chen@kernel.org>
> > > >
> > > > Could you please add these three patches to your repo?
> > >
> > > Can you please resend them, I don't seem to have them anymore.  Also
> > > split this into 2 different series, one for bugfixes-only, that needs to
> > > go to Linus for 6.3-final, and the others that are new features, to go
> > > for 6.4-rc1.
> >
> > Patch 2/3 is a bugfix, but it depends on patch 1/3. I cannot simply
> > separate them. After I resend them, could you please let them all go
> > to Linus for 6.4-rc1?
> 
> Resend and I will review, but that sounds like you also need to mark
> patch 1 as a fix, and then patch 3 is not part of the series and can be
> separate, right?

Yes, understood.

Thanks

> 
> What would you do if you were in my position here?  Make it easy for
> maintainers please :)
> 
> thanks,
> 
> greg k-h