[PATCH v1 1/5] usb: fotg210-hcd: use sysfs_emit() to instead of scnprintf()

[PATCH v1 1/5] usb: fotg210-hcd: use sysfs_emit() to instead of scnprintf()

[Andy Shevchenko]

Follow the advice of the Documentation/filesystems/sysfs.rst and show()
should only use sysfs_emit() or sysfs_emit_at() when formatting the
value to be returned to user space.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/fotg210/fotg210-hcd.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/fotg210/fotg210-hcd.c b/drivers/usb/fotg210/fotg210-hcd.c
index 7bd1e8f3080d..46752a75c428 100644
--- a/drivers/usb/fotg210/fotg210-hcd.c
+++ b/drivers/usb/fotg210/fotg210-hcd.c
@@ -4686,14 +4686,11 @@ static ssize_t uframe_periodic_max_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct fotg210_hcd *fotg210;
-	int n;
 
 	fotg210 = hcd_to_fotg210(bus_to_hcd(dev_get_drvdata(dev)));
-	n = scnprintf(buf, PAGE_SIZE, "%d\n", fotg210->uframe_periodic_max);
-	return n;
+	return sysfs_emit(buf, "%d\n", fotg210->uframe_periodic_max);
 }
 
-
 static ssize_t uframe_periodic_max_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
-- 
2.39.0

[PATCH v1 2/5] usb: fotg210-hcd: Don't shadow error codes in store()

[Andy Shevchenko]

kstrtox() along with regmap API can return different error codes based on
circumstances.

Don't shadow them when returning to the caller.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/fotg210/fotg210-hcd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/fotg210/fotg210-hcd.c b/drivers/usb/fotg210/fotg210-hcd.c
index 46752a75c428..5a934f5343a7 100644
--- a/drivers/usb/fotg210/fotg210-hcd.c
+++ b/drivers/usb/fotg210/fotg210-hcd.c
@@ -4702,8 +4702,10 @@ static ssize_t uframe_periodic_max_store(struct device *dev,
 	ssize_t ret;
 
 	fotg210 = hcd_to_fotg210(bus_to_hcd(dev_get_drvdata(dev)));
-	if (kstrtouint(buf, 0, &uframe_periodic_max) < 0)
-		return -EINVAL;
+
+	ret = kstrtouint(buf, 0, &uframe_periodic_max);
+	if (ret)
+		return ret;
 
 	if (uframe_periodic_max < 100 || uframe_periodic_max >= 125) {
 		fotg210_info(fotg210, "rejecting invalid request for uframe_periodic_max=%u\n",
-- 
2.39.0

Re: [PATCH v1 2/5] usb: fotg210-hcd: Don't shadow error codes in store()

[Linus Walleij]

On Fri, Jan 20, 2023 at 4:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> kstrtox() along with regmap API can return different error codes based on
> circumstances.
>
> Don't shadow them when returning to the caller.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 3/5] usb: fotg210-udc: remove redundant error logging

[Andy Shevchenko]

A call to platform_get_irq() already prints an error on failure within
its own implementation. So printing another error based on its return
value in the caller is redundant and should be removed. The clean up
also makes if condition block braces unnecessary. Remove that as well.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/fotg210/fotg210-udc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c
index 4334504fccc8..18d254125186 100644
--- a/drivers/usb/fotg210/fotg210-udc.c
+++ b/drivers/usb/fotg210/fotg210-udc.c
@@ -1169,10 +1169,8 @@ int fotg210_udc_probe(struct platform_device *pdev, struct fotg210 *fotg)
 	int i;
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		pr_err("could not get irq\n");
-		return -ENODEV;
-	}
+	if (irq < 0)
+		return irq;
 
 	/* initialize udc */
 	fotg210 = kzalloc(sizeof(struct fotg210_udc), GFP_KERNEL);
-- 
2.39.0

Re: [PATCH v1 3/5] usb: fotg210-udc: remove redundant error logging

[Linus Walleij]

On Fri, Jan 20, 2023 at 4:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> A call to platform_get_irq() already prints an error on failure within
> its own implementation. So printing another error based on its return
> value in the caller is redundant and should be removed. The clean up
> also makes if condition block braces unnecessary. Remove that as well.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 4/5] usb: fotg210: Switch to use dev_err_probe()

[Andy Shevchenko]

Switch to use dev_err_probe() to simplify the error paths and
unify message template.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/fotg210/fotg210-core.c | 6 ++----
 drivers/usb/fotg210/fotg210-hcd.c  | 8 +++-----
 drivers/usb/fotg210/fotg210-udc.c  | 2 +-
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
index c06f8eb3acbd..ce00d9407ce5 100644
--- a/drivers/usb/fotg210/fotg210-core.c
+++ b/drivers/usb/fotg210/fotg210-core.c
@@ -50,10 +50,8 @@ static int fotg210_gemini_init(struct fotg210 *fotg, struct resource *res,
 	int ret;
 
 	map = syscon_regmap_lookup_by_phandle(np, "syscon");
-	if (IS_ERR(map)) {
-		dev_err(dev, "no syscon\n");
-		return PTR_ERR(map);
-	}
+	if (IS_ERR(map))
+		return dev_err_probe(dev, PTR_ERR(map), "no syscon\n");
 	fotg->map = map;
 	wakeup = of_property_read_bool(np, "wakeup-source");
 
diff --git a/drivers/usb/fotg210/fotg210-hcd.c b/drivers/usb/fotg210/fotg210-hcd.c
index 5a934f5343a7..613d29f04bcb 100644
--- a/drivers/usb/fotg210/fotg210-hcd.c
+++ b/drivers/usb/fotg210/fotg210-hcd.c
@@ -5575,8 +5575,7 @@ int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
 	hcd = usb_create_hcd(&fotg210_fotg210_hc_driver, dev,
 			dev_name(dev));
 	if (!hcd) {
-		dev_err(dev, "failed to create hcd\n");
-		retval = -ENOMEM;
+		retval = dev_err_probe(dev, -ENOMEM, "failed to create hcd\n");
 		goto fail_create_hcd;
 	}
 
@@ -5600,7 +5599,7 @@ int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
 
 	retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (retval) {
-		dev_err(dev, "failed to add hcd with err %d\n", retval);
+		dev_err_probe(dev, retval, "failed to add hcd\n");
 		goto failed_put_hcd;
 	}
 	device_wakeup_enable(hcd->self.controller);
@@ -5611,8 +5610,7 @@ int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
 failed_put_hcd:
 	usb_put_hcd(hcd);
 fail_create_hcd:
-	dev_err(dev, "init %s fail, %d\n", dev_name(dev), retval);
-	return retval;
+	return dev_err_probe(dev, retval, "init %s fail\n", dev_name(dev));
 }
 
 /*
diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c
index 18d254125186..5b515f5cb2d7 100644
--- a/drivers/usb/fotg210/fotg210-udc.c
+++ b/drivers/usb/fotg210/fotg210-udc.c
@@ -1258,7 +1258,7 @@ int fotg210_udc_probe(struct platform_device *pdev, struct fotg210 *fotg)
 	ret = request_irq(irq, fotg210_irq, IRQF_SHARED,
 			  udc_name, fotg210);
 	if (ret < 0) {
-		dev_err(dev, "request_irq error (%d)\n", ret);
+		dev_err_probe(dev, ret, "request_irq error\n");
 		goto err_req;
 	}
 
-- 
2.39.0

Re: [PATCH v1 4/5] usb: fotg210: Switch to use dev_err_probe()

[Linus Walleij]

On Fri, Jan 20, 2023 at 4:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Switch to use dev_err_probe() to simplify the error paths and
> unify message template.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 4/5] usb: fotg210: Switch to use dev_err_probe()

[Christophe JAILLET]

Le 20/01/2023 à 16:44, Andy Shevchenko a écrit :
> Switch to use dev_err_probe() to simplify the error paths and
> unify message template.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>   drivers/usb/fotg210/fotg210-core.c | 6 ++----
>   drivers/usb/fotg210/fotg210-hcd.c  | 8 +++-----
>   drivers/usb/fotg210/fotg210-udc.c  | 2 +-
>   3 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
> index c06f8eb3acbd..ce00d9407ce5 100644
> --- a/drivers/usb/fotg210/fotg210-core.c
> +++ b/drivers/usb/fotg210/fotg210-core.c
> @@ -50,10 +50,8 @@ static int fotg210_gemini_init(struct fotg210 *fotg, struct resource *res,
>   	int ret;
>   
>   	map = syscon_regmap_lookup_by_phandle(np, "syscon");
> -	if (IS_ERR(map)) {
> -		dev_err(dev, "no syscon\n");
> -		return PTR_ERR(map);
> -	}
> +	if (IS_ERR(map))
> +		return dev_err_probe(dev, PTR_ERR(map), "no syscon\n");
>   	fotg->map = map;
>   	wakeup = of_property_read_bool(np, "wakeup-source");
>   
> diff --git a/drivers/usb/fotg210/fotg210-hcd.c b/drivers/usb/fotg210/fotg210-hcd.c
> index 5a934f5343a7..613d29f04bcb 100644
> --- a/drivers/usb/fotg210/fotg210-hcd.c
> +++ b/drivers/usb/fotg210/fotg210-hcd.c
> @@ -5575,8 +5575,7 @@ int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
>   	hcd = usb_create_hcd(&fotg210_fotg210_hc_driver, dev,
>   			dev_name(dev));
>   	if (!hcd) {
> -		dev_err(dev, "failed to create hcd\n");
> -		retval = -ENOMEM;
> +		retval = dev_err_probe(dev, -ENOMEM, "failed to create hcd\n");
>   		goto fail_create_hcd;
>   	}
>   
> @@ -5600,7 +5599,7 @@ int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
>   
>   	retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
>   	if (retval) {
> -		dev_err(dev, "failed to add hcd with err %d\n", retval);
> +		dev_err_probe(dev, retval, "failed to add hcd\n");
>   		goto failed_put_hcd;
>   	}
>   	device_wakeup_enable(hcd->self.controller);
> @@ -5611,8 +5610,7 @@ int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
>   failed_put_hcd:
>   	usb_put_hcd(hcd);
>   fail_create_hcd:
> -	dev_err(dev, "init %s fail, %d\n", dev_name(dev), retval);
> -	return retval;
> +	return dev_err_probe(dev, retval, "init %s fail\n", dev_name(dev));

Hi,
the patch is already applied, but is dev_name(dev) needed here?

CJ

>   }
>   
>   /*
> diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c
> index 18d254125186..5b515f5cb2d7 100644
> --- a/drivers/usb/fotg210/fotg210-udc.c
> +++ b/drivers/usb/fotg210/fotg210-udc.c
> @@ -1258,7 +1258,7 @@ int fotg210_udc_probe(struct platform_device *pdev, struct fotg210 *fotg)
>   	ret = request_irq(irq, fotg210_irq, IRQF_SHARED,
>   			  udc_name, fotg210);
>   	if (ret < 0) {
> -		dev_err(dev, "request_irq error (%d)\n", ret);
> +		dev_err_probe(dev, ret, "request_irq error\n");
>   		goto err_req;
>   	}
>   

Re: [PATCH v1 4/5] usb: fotg210: Switch to use dev_err_probe()

[Linus Walleij]

On Thu, Feb 16, 2023 at 9:07 AM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:

> > -     dev_err(dev, "init %s fail, %d\n", dev_name(dev), retval);
> > -     return retval;
> > +     return dev_err_probe(dev, retval, "init %s fail\n", dev_name(dev));
>
> Hi,
> the patch is already applied, but is dev_name(dev) needed here?

Not really but it's not a big deal either, it's just a string.
Feel free to send an incremental patch dropping it :)

Yours,
Linus Walleij

[PATCH v1 5/5] usb: fotg210: use devm_platform_get_and_ioremap_resource()

[Andy Shevchenko]

Convert platform_get_resource(), devm_ioremap_resource() to a single
call to devm_platform_get_and_ioremap_resource(), as this is exactly
what this function does.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/fotg210/fotg210-core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
index ce00d9407ce5..202d80adca2c 100644
--- a/drivers/usb/fotg210/fotg210-core.c
+++ b/drivers/usb/fotg210/fotg210-core.c
@@ -135,11 +135,7 @@ static int fotg210_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	fotg->dev = dev;
 
-	fotg->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!fotg->res)
-		return -ENODEV;
-
-	fotg->base = devm_ioremap_resource(dev, fotg->res);
+	fotg->base = devm_platform_get_and_ioremap_resource(pdev, 0, &fotg->res);
 	if (!fotg->base)
 		return -ENOMEM;
 
-- 
2.39.0

Re: [PATCH v1 5/5] usb: fotg210: use devm_platform_get_and_ioremap_resource()

[Linus Walleij]

On Fri, Jan 20, 2023 at 4:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Convert platform_get_resource(), devm_ioremap_resource() to a single
> call to devm_platform_get_and_ioremap_resource(), as this is exactly
> what this function does.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 5/5] usb: fotg210: use devm_platform_get_and_ioremap_resource()

[Andy Shevchenko]

On Thu, Jan 26, 2023 at 09:02:30PM +0100, Linus Walleij wrote:
> On Fri, Jan 20, 2023 at 4:44 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:

...

> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks for all your reviews!

But I think you are a bit late with them as Greg applied the series like
a couple of days ago.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 5/5] usb: fotg210: use devm_platform_get_and_ioremap_resource()

[Linus Walleij]

On Fri, Jan 27, 2023 at 9:55 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> On Thu, Jan 26, 2023 at 09:02:30PM +0100, Linus Walleij wrote:
> > On Fri, Jan 20, 2023 at 4:44 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
>
> ...
>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Thanks for all your reviews!
>
> But I think you are a bit late with them as Greg applied the series like
> a couple of days ago.

Yeah he's so quick and I'm so slow...
But these days we record the mail thread link to lore in the
commit so I can still share the blame ;)

Yours,
Linus Walleij

Re: [PATCH v1 5/5] usb: fotg210: use devm_platform_get_and_ioremap_resource()

[Andy Shevchenko]

On Fri, Jan 27, 2023 at 10:10:11AM +0100, Linus Walleij wrote:
> On Fri, Jan 27, 2023 at 9:55 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > On Thu, Jan 26, 2023 at 09:02:30PM +0100, Linus Walleij wrote:
> > > On Fri, Jan 20, 2023 at 4:44 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:

...

> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > Thanks for all your reviews!
> >
> > But I think you are a bit late with them as Greg applied the series like
> > a couple of days ago.
> 
> Yeah he's so quick and I'm so slow...
> But these days we record the mail thread link to lore in the
> commit so I can still share the blame ;)

Right, what I meant is not diminish your efforts, but to say that
they won't make Git history.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/5] usb: fotg210-hcd: use sysfs_emit() to instead of scnprintf()

[Linus Walleij]

On Fri, Jan 20, 2023 at 4:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Follow the advice of the Documentation/filesystems/sysfs.rst and show()
> should only use sysfs_emit() or sysfs_emit_at() when formatting the
> value to be returned to user space.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij