from:"Tang Bin"

Re: usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in fsl_udc_probe()

2020-04-09 Thread Tang Bin

On Thu,Apr 9,2020 08:28:28 Markus Elfring  wrote:


> I was unsure if I noticed another programming mistake.

> Do other contributors know the affected software module better than me?

I discovered this problem fews days ago, and doing experiments on the hardware 
to test my idea.

Thanks
Tang Bin

[PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

2020-04-09 Thread Tang Bin

If the function "platform_get_irq()" failed, the negative value
returned will not be detected here, including "-EPROBE_DEFER", which
causes the application to fail to get the correct error message.
Thus it must be fixed.

Signed-off-by: Tang Bin 
Signed-off-by: Shengju Zhang 
---
 drivers/usb/gadget/udc/fsl_udc_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c 
b/drivers/usb/gadget/udc/fsl_udc_core.c
index ec6eda426..60853ad10 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2441,8 +2441,8 @@ static int fsl_udc_probe(struct platform_device *pdev)
udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;
 
udc_controller->irq = platform_get_irq(pdev, 0);
-   if (!udc_controller->irq) {
-   ret = -ENODEV;
+   if (udc_controller->irq <= 0) {
+   ret = udc_controller->irq ? : -ENODEV;
goto err_iounmap;
}
 
-- 
2.20.1.windows.1

Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

2020-04-10 Thread Tang Bin


Hi Markus

On 2020/4/10 15:33, Markus Elfring wrote:

If the function "platform_get_irq()" failed, the negative value
returned will not be detected here, including "-EPROBE_DEFER",

I suggest to adjust this change description.

Wording alternative:
   The negative return value (which could eventually be “-EPROBE_DEFER”)
   will not be detected here from a failed call of the function 
“platform_get_irq”.
Hardware experiments show that the negative return value is not just 
"-EPROBE_DEFER".



which causes the application to fail to get the correct error message.

Will another fine-tuning become relevant also for this wording?

Maybe that's not quite accurate.




Thus it must be fixed.

Wording alternative:
   Thus adjust the error detection and corresponding exception handling.

Got it.




Signed-off-by: Tang Bin 
Signed-off-by: Shengju Zhang 

How do you think about to add the tags “Fixes”, “Link” and “Reported-by”?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=c0cc271173b2e1c2d8d0ceaef14e4dfa79eefc0d#n584

usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in 
fsl_udc_probe()
https://lore.kernel.org/linux-usb/36341bb1-1e00-5eb1-d032-60dcc614d...@web.de/
https://lkml.org/lkml/2020/4/8/442

…

+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2441,8 +2441,8 @@ static int fsl_udc_probe(struct platform_device *pdev)
udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;

udc_controller->irq = platform_get_irq(pdev, 0);
-   if (!udc_controller->irq) {
-   ret = -ENODEV;
+   if (udc_controller->irq <= 0) {

Will such a failure predicate need any more clarification?

How does this check fit to the current software documentation?

Maybe my tags are not suitable.




+   ret = udc_controller->irq ? : -ENODEV;

Will it be clearer to specify values for all cases in such a conditional 
operator
(instead of leaving one case empty)?


I don't know what you mean of "instead of leaving one case empty". But 
by experiment, "ret = udc_controller->irq ? : -ENODEV" or "ret = 
udc_controller->irq < 0 ? udc_controller->irq : -ENODEV" should be 
suitable here.



Thank you for your guidance.

Tang Bin

Re: usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

2020-04-10 Thread Tang Bin


Hi Markus:

On 2020/4/10 16:30, Markus Elfring wrote:

Hardware experiments show that the negative return value is not just 
"-EPROBE_DEFER".

How much will this specific error code influence our understanding
of the discussed software situation?

From my superficial knowledge, I think we should not  think about it 
too complicated. The return value is just zero or negative, and for 
these negative return value, such as 
"-ENODEV"、"-ENXIO"、"-ENOENT"、“EPROBE_DEFER”，may be the same 
effect。But“-EPROBE_DEFER”has another  importment function: Driver 
requested deferred probing，which is used in cases where the dependency 
resource is not ready during the driver initialization process.

+    ret = udc_controller->irq ? : -ENODEV;

Will it be clearer to specify values for all cases in such a conditional 
operator
(instead of leaving one case empty)?

I don't know what you mean of "instead of leaving one case empty".

I suggest to reconsider also the proposed specification “… ? : …”.


What you mean is the way I'm written？

I have provided two ways of patching, both functional can be verified on 
hardware.


Thanks for your patience.

Tang Bin

Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

2020-04-14 Thread Tang Bin


Hi

On 2020/4/14 16:30, Dan Carpenter wrote:

On Fri, Apr 10, 2020 at 04:05:06PM +0800, Tang Bin wrote:



Thus it must be fixed.

Wording alternative:
Thus adjust the error detection and corresponding exception handling.

Got it.

Wow...

No, don't listen to Markus when it comes to writing commit messages.
You couldn't find worse advice anywhere.  :P


I'm actively waiting for a reply from the maintainer. Thank you very much.

Tang Bin

[PATCH] ASoC: fsl_micfil: Omit superfluous error message in fsl_micfil_probe()

2020-04-14 Thread Tang Bin

In the function fsl_micfil_probe(), when get irq failed, the function 
platform_get_irq() logs an error message, so remove redundant message here.

Signed-off-by: Tang Bin 
Signed-off-by: Shengju Zhang 
---
 sound/soc/fsl/fsl_micfil.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index f7f2d29f1..e73bd6570 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -702,10 +702,8 @@ static int fsl_micfil_probe(struct platform_device *pdev)
for (i = 0; i < MICFIL_IRQ_LINES; i++) {
micfil->irq[i] = platform_get_irq(pdev, i);
dev_err(&pdev->dev, "GET IRQ: %d\n", micfil->irq[i]);
-   if (micfil->irq[i] < 0) {
-   dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
+   if (micfil->irq[i] < 0)
return micfil->irq[i];
-   }
}
 
if (of_property_read_bool(np, "fsl,shared-interrupt"))
-- 
2.20.1.windows.1

[PATCH] ASoC: fsl_xcvr: Use devm_platform_ioremap_resource_byname() to simplify code

2021-03-02 Thread Tang Bin

In this function, devm_platform_ioremap_resource_byname() should be 
suitable to simplify code.

Signed-off-by: Tang Bin 
---
 sound/soc/fsl/fsl_xcvr.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index 6dd0a5fcd455..5e8284db857b 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -1131,7 +1131,7 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
 {
struct device *dev = &pdev->dev;
struct fsl_xcvr *xcvr;
-   struct resource *ram_res, *regs_res, *rx_res, *tx_res;
+   struct resource *rx_res, *tx_res;
void __iomem *regs;
int ret, irq;
 
@@ -1166,13 +1166,11 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
return PTR_ERR(xcvr->pll_ipg_clk);
}
 
-   ram_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ram");
-   xcvr->ram_addr = devm_ioremap_resource(dev, ram_res);
+   xcvr->ram_addr = devm_platform_ioremap_resource_byname(pdev, "ram");
if (IS_ERR(xcvr->ram_addr))
return PTR_ERR(xcvr->ram_addr);
 
-   regs_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
-   regs = devm_ioremap_resource(dev, regs_res);
+   regs = devm_platform_ioremap_resource_byname(pdev, "regs");
if (IS_ERR(regs))
return PTR_ERR(regs);
 
-- 
2.18.2

[PATCH] ASoC: fsl_spdif: Fix unnecessary check in fsl_spdif_probe()

2020-08-26 Thread Tang Bin

The function fsl_spdif_probe() is only called with an openfirmware
platform device. Therefore there is no need to check that the passed
in device is NULL.

Signed-off-by: Zhang Shengju 
Signed-off-by: Tang Bin 
---
 sound/soc/fsl/fsl_spdif.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 7858a5499..395c50167 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1221,9 +1221,6 @@ static int fsl_spdif_probe(struct platform_device *pdev)
void __iomem *regs;
int irq, ret, i;
 
-   if (!np)
-   return -ENODEV;
-
spdif_priv = devm_kzalloc(&pdev->dev, sizeof(*spdif_priv), GFP_KERNEL);
if (!spdif_priv)
return -ENOMEM;
-- 
2.20.1.windows.1

Re: [PATCH] ASoC: fsl_spdif: Fix unnecessary check infsl_spdif_probe()

2020-08-26 Thread Tang Bin


Hi Mark

在 2020/8/27 0:53, Mark Brown 写道:

On Wed, Aug 26, 2020 at 11:09:18PM +0800, Tang Bin wrote:

The function fsl_spdif_probe() is only called with an openfirmware
platform device. Therefore there is no need to check that the passed
in device is NULL.

Why is this an issue - the check will make things more robust if someone
manages to load the driver on a non-DT system and otherwise costs us a
couple of instructions?

Thanks for your reply.

In this function,  function fsl_spdif_probe() can be triggered only if 
the platform_device and platform_driver matches,


so I think the judgement at the beginning is redundant.

Thanks

Tang Bin

[PATCH] ASoC: fsl_spdif: Utilize the defined parameter to clear code

2021-01-28 Thread Tang Bin

Utilize the defined parameter 'dev' to make the code cleaner.

Signed-off-by: Tang Bin 
---
 sound/soc/fsl/fsl_spdif.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 455f96908..b6d5563df 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1215,7 +1215,7 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv 
*spdif_priv,
 
for (i = 0; i < STC_TXCLK_SRC_MAX; i++) {
sprintf(tmp, "rxtx%d", i);
-   clk = devm_clk_get(&pdev->dev, tmp);
+   clk = devm_clk_get(dev, tmp);
if (IS_ERR(clk)) {
dev_err(dev, "no rxtx%d clock in devicetree\n", i);
return PTR_ERR(clk);
@@ -1237,14 +1237,14 @@ static int fsl_spdif_probe_txclk(struct fsl_spdif_priv 
*spdif_priv,
break;
}
 
-   dev_dbg(&pdev->dev, "use rxtx%d as tx clock source for %dHz sample 
rate\n",
+   dev_dbg(dev, "use rxtx%d as tx clock source for %dHz sample rate\n",
spdif_priv->txclk_src[index], rate[index]);
-   dev_dbg(&pdev->dev, "use txclk df %d for %dHz sample rate\n",
+   dev_dbg(dev, "use txclk df %d for %dHz sample rate\n",
spdif_priv->txclk_df[index], rate[index]);
if (clk_is_match(spdif_priv->txclk[index], spdif_priv->sysclk))
-   dev_dbg(&pdev->dev, "use sysclk df %d for %dHz sample rate\n",
+   dev_dbg(dev, "use sysclk df %d for %dHz sample rate\n",
spdif_priv->sysclk_df[index], rate[index]);
-   dev_dbg(&pdev->dev, "the best rate for %dHz sample rate is %dHz\n",
+   dev_dbg(dev, "the best rate for %dHz sample rate is %dHz\n",
rate[index], spdif_priv->txrate[index]);
 
return 0;
-- 
2.20.1.windows.1

[PATCH] ASoC: fsl_micfil: Fix format and unused assignment

2020-05-18 Thread Tang Bin

In the function fsl_micfil_startup(), the two lines of dev_err()
can be shortened to one line. And delete unused initialized value
of 'ret', because it will be assigned by the function
fsl_micfil_set_mclk_rate().

Signed-off-by: Tang Bin 
---
 sound/soc/fsl/fsl_micfil.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index f7f2d29f1..a7a6118be 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -217,8 +217,7 @@ static int fsl_micfil_startup(struct snd_pcm_substream 
*substream,
struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
 
if (!micfil) {
-   dev_err(dai->dev,
-   "micfil dai priv_data not set\n");
+   dev_err(dai->dev, "micfil dai priv_data not set\n");
return -EINVAL;
}
 
@@ -296,7 +295,7 @@ static int fsl_set_clock_params(struct device *dev, 
unsigned int rate)
 {
struct fsl_micfil *micfil = dev_get_drvdata(dev);
int clk_div;
-   int ret = 0;
+   int ret;
 
ret = fsl_micfil_set_mclk_rate(micfil, rate);
if (ret < 0)
-- 
2.20.1.windows.1

Re: [PATCH] ASoC: fsl_micfil: Fix format and unused assignment

2020-05-18 Thread Tang Bin




On 2020/5/18 18:25, Mark Brown wrote:

On Mon, May 18, 2020 at 03:44:05PM +0800, Tang Bin wrote:

In the function fsl_micfil_startup(), the two lines of dev_err()
can be shortened to one line. And delete unused initialized value
of 'ret', because it will be assigned by the function
fsl_micfil_set_mclk_rate().

This is two separate changes with no overlap so would have been better
sent as separate patches.


Got it, Thanks

Tang Bin

[PATCH] ASoC: fsl_micfil: Fix indentation to put on one line affected code

2020-05-18 Thread Tang Bin

In the function fsl_micfil_startup(), the two lines of dev_err()
can be shortened to one line.

Signed-off-by: Tang Bin 
---
 sound/soc/fsl/fsl_micfil.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index f7f2d29f1..79cf488fa 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -217,8 +217,7 @@ static int fsl_micfil_startup(struct snd_pcm_substream 
*substream,
struct fsl_micfil *micfil = snd_soc_dai_get_drvdata(dai);
 
if (!micfil) {
-   dev_err(dai->dev,
-   "micfil dai priv_data not set\n");
+   dev_err(dai->dev, "micfil dai priv_data not set\n");
return -EINVAL;
}
 
-- 
2.20.1.windows.1

[PATCH] ASoC: fsl_micfil: Fix unused assignment in fsl_set_clock_params()

2020-05-18 Thread Tang Bin

Delete unused initialized value of 'ret', because it will
be assigned by the function fsl_micfil_set_mclk_rate().

Signed-off-by: Tang Bin 
---
 sound/soc/fsl/fsl_micfil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index 79cf488fa..a7a6118be 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -295,7 +295,7 @@ static int fsl_set_clock_params(struct device *dev, 
unsigned int rate)
 {
struct fsl_micfil *micfil = dev_get_drvdata(dev);
int clk_div;
-   int ret = 0;
+   int ret;
 
ret = fsl_micfil_set_mclk_rate(micfil, rate);
if (ret < 0)
-- 
2.20.1.windows.1

[PATCH] ASoC: fsl_xcvr: Omit superfluous error message in fsl_xcvr_probe()

2021-06-24 Thread Tang Bin

In the function fsl_xcvr__probe(), when get irq failed,
the function platform_get_irq() logs an error message, so remove
redundant message here.

Signed-off-by: Tang Bin 
---
 sound/soc/fsl/fsl_xcvr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index 5e8284db857b..711d738f8de1 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -1190,10 +1190,8 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
 
/* get IRQs */
irq = platform_get_irq(pdev, 0);
-   if (irq < 0) {
-   dev_err(dev, "no irq[0]: %d\n", irq);
+   if (irq < 0)
return irq;
-   }
 
ret = devm_request_irq(dev, irq, irq0_isr, 0, pdev->name, xcvr);
if (ret) {
-- 
2.18.2

Re: usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in fsl_udc_probe()

[PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

Re: usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

[PATCH] ASoC: fsl_micfil: Omit superfluous error message in fsl_micfil_probe()

[PATCH] ASoC: fsl_xcvr: Use devm_platform_ioremap_resource_byname() to simplify code

[PATCH] ASoC: fsl_spdif: Fix unnecessary check in fsl_spdif_probe()

Re: [PATCH] ASoC: fsl_spdif: Fix unnecessary check infsl_spdif_probe()

[PATCH] ASoC: fsl_spdif: Utilize the defined parameter to clear code

[PATCH] ASoC: fsl_micfil: Fix format and unused assignment

Re: [PATCH] ASoC: fsl_micfil: Fix format and unused assignment

[PATCH] ASoC: fsl_micfil: Fix indentation to put on one line affected code

[PATCH] ASoC: fsl_micfil: Fix unused assignment in fsl_set_clock_params()

[PATCH] ASoC: fsl_xcvr: Omit superfluous error message in fsl_xcvr_probe()

15 matches

Site Navigation

Mail list logo

Footer information