| Message ID | 20201012064256.8344-1-pawell@cadence.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: cdns3: Rids of duplicate error message | expand |
Hi Pawel, On 12/10/2020 09:42, Pawel Laszczak wrote: > On failure, the platform_get_irq_byname prints an error message > so, patch removes error message related to this function from > core.c file. > > A change was suggested during reviewing CDNSP driver by Chunfeng Yun. > > Signed-off-by: Pawel Laszczak <pawell@cadence.com> > --- > Changelog: > v2 > - simplified code as sugested by Roger Quadros. > > drivers/usb/cdns3/core.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > index a0f73d4711ae..85ef3025b293 100644 > --- a/drivers/usb/cdns3/core.c > +++ b/drivers/usb/cdns3/core.c > @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev) > if (cdns->dev_irq == -EPROBE_DEFER) Shouldn't this be if (cdns->dev_irq < 0) ? > return cdns->dev_irq; > > - if (cdns->dev_irq < 0) > - dev_err(dev, "couldn't get peripheral irq\n"); > - > regs = devm_platform_ioremap_resource_byname(pdev, "dev"); > if (IS_ERR(regs)) > return PTR_ERR(regs); > cdns->dev_regs = regs; > > cdns->otg_irq = platform_get_irq_byname(pdev, "otg"); > - if (cdns->otg_irq == -EPROBE_DEFER) > - return cdns->otg_irq; > - > - if (cdns->otg_irq < 0) { > - dev_err(dev, "couldn't get otg irq\n"); > + if (cdns->otg_irq < 0) > return cdns->otg_irq; > - } > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg"); > if (!res) { >
Hi Roger, On 12/10/2020 09:42, Pawel Laszczak wrote: > On failure, the platform_get_irq_byname prints an error message > so, patch removes error message related to this function from > core.c file. > > A change was suggested during reviewing CDNSP driver by Chunfeng Yun. > > Signed-off-by: Pawel Laszczak <pawell@cadence.com> > --- > Changelog: > v2 > - simplified code as sugested by Roger Quadros. > > drivers/usb/cdns3/core.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > index a0f73d4711ae..85ef3025b293 100644 > --- a/drivers/usb/cdns3/core.c > +++ b/drivers/usb/cdns3/core.c > @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev) > if (cdns->dev_irq == -EPROBE_DEFER) Shouldn't this be if (cdns->dev_irq < 0) ? No, such line will change the original behavior of driver. Current patch allows to run driver when we support only host. In such case the dev_irq can be < 0 and we still can use host side. Regards, Pawel Laszczak > return cdns->dev_irq; > > - if (cdns->dev_irq < 0) > - dev_err(dev, "couldn't get peripheral irq\n"); > - > regs = devm_platform_ioremap_resource_byname(pdev, "dev"); > if (IS_ERR(regs)) > return PTR_ERR(regs); > cdns->dev_regs = regs; > > cdns->otg_irq = platform_get_irq_byname(pdev, "otg"); > - if (cdns->otg_irq == -EPROBE_DEFER) > - return cdns->otg_irq; > - > - if (cdns->otg_irq < 0) { > - dev_err(dev, "couldn't get otg irq\n"); > + if (cdns->otg_irq < 0) > return cdns->otg_irq; > - } > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg"); > if (!res) { >
Pawel, On 12/10/2020 14:12, Pawel Laszczak wrote: > Hi Roger, > > On 12/10/2020 09:42, Pawel Laszczak wrote: >> On failure, the platform_get_irq_byname prints an error message >> so, patch removes error message related to this function from >> core.c file. >> >> A change was suggested during reviewing CDNSP driver by Chunfeng Yun. >> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> >> --- >> Changelog: >> v2 >> - simplified code as sugested by Roger Quadros. >> >> drivers/usb/cdns3/core.c | 10 +--------- >> 1 file changed, 1 insertion(+), 9 deletions(-) >> >> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c >> index a0f73d4711ae..85ef3025b293 100644 >> --- a/drivers/usb/cdns3/core.c >> +++ b/drivers/usb/cdns3/core.c >> @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev) >> if (cdns->dev_irq == -EPROBE_DEFER) > > Shouldn't this be > if (cdns->dev_irq < 0) > ? > > No, such line will change the original behavior of driver. > > Current patch allows to run driver when we support only host. > In such case the dev_irq can be < 0 and we still can use host side. In that case should we check if we need dev_irq and if so then error out. i.e. if mode is "peripheral" or "otg" Also DT binding document says all 3 IRQs are mandatory. Do we need to update that? cheers, -roger > > Regards, > Pawel Laszczak > >> return cdns->dev_irq; >> >> - if (cdns->dev_irq < 0) >> - dev_err(dev, "couldn't get peripheral irq\n"); >> - >> regs = devm_platform_ioremap_resource_byname(pdev, "dev"); >> if (IS_ERR(regs)) >> return PTR_ERR(regs); >> cdns->dev_regs = regs; >> >> cdns->otg_irq = platform_get_irq_byname(pdev, "otg"); >> - if (cdns->otg_irq == -EPROBE_DEFER) >> - return cdns->otg_irq; >> - >> - if (cdns->otg_irq < 0) { >> - dev_err(dev, "couldn't get otg irq\n"); >> + if (cdns->otg_irq < 0) >> return cdns->otg_irq; >> - } >> >> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg"); >> if (!res) { >> >
On 20-10-12 14:38:16, Roger Quadros wrote: > Pawel, > > On 12/10/2020 14:12, Pawel Laszczak wrote: > > Hi Roger, > > > > On 12/10/2020 09:42, Pawel Laszczak wrote: > > > On failure, the platform_get_irq_byname prints an error message > > > so, patch removes error message related to this function from > > > core.c file. > > > > > > A change was suggested during reviewing CDNSP driver by Chunfeng Yun. > > > > > > Signed-off-by: Pawel Laszczak <pawell@cadence.com> > > > --- > > > Changelog: > > > v2 > > > - simplified code as sugested by Roger Quadros. > > > > > > drivers/usb/cdns3/core.c | 10 +--------- > > > 1 file changed, 1 insertion(+), 9 deletions(-) > > > > > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > > > index a0f73d4711ae..85ef3025b293 100644 > > > --- a/drivers/usb/cdns3/core.c > > > +++ b/drivers/usb/cdns3/core.c > > > @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev) > > > if (cdns->dev_irq == -EPROBE_DEFER) > > > > Shouldn't this be > > if (cdns->dev_irq < 0) > > ? > > > > No, such line will change the original behavior of driver. > > > > Current patch allows to run driver when we support only host. > > In such case the dev_irq can be < 0 and we still can use host side. > > In that case should we check if we need dev_irq and if so then error out. > i.e. if mode is "peripheral" or "otg" > > Also DT binding document says all 3 IRQs are mandatory. Do we need to update that? > I agree with Roger, or you change the driver behaviour that return error if the dev_irq is < 0. Peter > > > > > Regards, > > Pawel Laszczak > > > > > return cdns->dev_irq; > > > - if (cdns->dev_irq < 0) > > > - dev_err(dev, "couldn't get peripheral irq\n"); > > > - > > > regs = devm_platform_ioremap_resource_byname(pdev, "dev"); > > > if (IS_ERR(regs)) > > > return PTR_ERR(regs); > > > cdns->dev_regs = regs; > > > cdns->otg_irq = platform_get_irq_byname(pdev, "otg"); > > > - if (cdns->otg_irq == -EPROBE_DEFER) > > > - return cdns->otg_irq; > > > - > > > - if (cdns->otg_irq < 0) { > > > - dev_err(dev, "couldn't get otg irq\n"); > > > + if (cdns->otg_irq < 0) > > > return cdns->otg_irq; > > > - } > > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg"); > > > if (!res) { > > > > > > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> >On 20-10-12 14:38:16, Roger Quadros wrote: >> Pawel, >> >> On 12/10/2020 14:12, Pawel Laszczak wrote: >> > Hi Roger, >> > >> > On 12/10/2020 09:42, Pawel Laszczak wrote: >> > > On failure, the platform_get_irq_byname prints an error message >> > > so, patch removes error message related to this function from >> > > core.c file. >> > > >> > > A change was suggested during reviewing CDNSP driver by Chunfeng Yun. >> > > >> > > Signed-off-by: Pawel Laszczak <pawell@cadence.com> >> > > --- >> > > Changelog: >> > > v2 >> > > - simplified code as sugested by Roger Quadros. >> > > >> > > drivers/usb/cdns3/core.c | 10 +--------- >> > > 1 file changed, 1 insertion(+), 9 deletions(-) >> > > >> > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c >> > > index a0f73d4711ae..85ef3025b293 100644 >> > > --- a/drivers/usb/cdns3/core.c >> > > +++ b/drivers/usb/cdns3/core.c >> > > @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev) >> > > if (cdns->dev_irq == -EPROBE_DEFER) >> > >> > Shouldn't this be >> > if (cdns->dev_irq < 0) >> > ? >> > >> > No, such line will change the original behavior of driver. >> > >> > Current patch allows to run driver when we support only host. >> > In such case the dev_irq can be < 0 and we still can use host side. >> >> In that case should we check if we need dev_irq and if so then error out. >> i.e. if mode is "peripheral" or "otg" >> >> Also DT binding document says all 3 IRQs are mandatory. Do we need to update that? >> > >I agree with Roger, or you change the driver behaviour that return error >if the dev_irq is < 0. Ok, I will change the condition as suggested by Roger. Pawel > >Peter >> >> > >> > Regards, >> > Pawel Laszczak >> > >> > > return cdns->dev_irq; >> > > - if (cdns->dev_irq < 0) >> > > - dev_err(dev, "couldn't get peripheral irq\n"); >> > > - >> > > regs = devm_platform_ioremap_resource_byname(pdev, "dev"); >> > > if (IS_ERR(regs)) >> > > return PTR_ERR(regs); >> > > cdns->dev_regs = regs; >> > > cdns->otg_irq = platform_get_irq_byname(pdev, "otg"); >> > > - if (cdns->otg_irq == -EPROBE_DEFER) >> > > - return cdns->otg_irq; >> > > - >> > > - if (cdns->otg_irq < 0) { >> > > - dev_err(dev, "couldn't get otg irq\n"); >> > > + if (cdns->otg_irq < 0) >> > > return cdns->otg_irq; >> > > - } >> > > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg"); >> > > if (!res) { >> > > >> > >> >> -- >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > >-- > >Thanks, >Peter Chen
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index a0f73d4711ae..85ef3025b293 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -469,22 +469,14 @@ static int cdns3_probe(struct platform_device *pdev) if (cdns->dev_irq == -EPROBE_DEFER) return cdns->dev_irq; - if (cdns->dev_irq < 0) - dev_err(dev, "couldn't get peripheral irq\n"); - regs = devm_platform_ioremap_resource_byname(pdev, "dev"); if (IS_ERR(regs)) return PTR_ERR(regs); cdns->dev_regs = regs; cdns->otg_irq = platform_get_irq_byname(pdev, "otg"); - if (cdns->otg_irq == -EPROBE_DEFER) - return cdns->otg_irq; - - if (cdns->otg_irq < 0) { - dev_err(dev, "couldn't get otg irq\n"); + if (cdns->otg_irq < 0) return cdns->otg_irq; - } res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "otg"); if (!res) {
On failure, the platform_get_irq_byname prints an error message so, patch removes error message related to this function from core.c file. A change was suggested during reviewing CDNSP driver by Chunfeng Yun. Signed-off-by: Pawel Laszczak <pawell@cadence.com> --- Changelog: v2 - simplified code as sugested by Roger Quadros. drivers/usb/cdns3/core.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)