From: Shubhrajyoti <shubhrajyoti@ti.com>
To: Govindraj <govindraj.ti@gmail.com>
Cc: linux-serial@vger.kernel.org, linux-omap@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/5] omap-serial: Fix the error handling in the omap_serial probe
Date: Fri, 20 Jan 2012 15:05:06 +0530 [thread overview]
Message-ID: <4F19354A.4040707@ti.com> (raw)
In-Reply-To: <CAAL8m4xVOiGjOHvf=Uy4m9QbqxO21pFeB2HcC_zGijj-+VF_jw@mail.gmail.com>
On Friday 20 January 2012 02:19 PM, Govindraj wrote:
> On Mon, Jan 16, 2012 at 3:52 PM, Shubhrajyoti D <shubhrajyoti@ti.com> wrote:
>> The patch does the following
>>
>> - The pm_runtime_disable is called in the remove not in the error
>> case of probe.The patch calls the pm_runtime_disable in the error
>> case.
>> - The up is not freed in the error path. Fix the memory leak by calling
>> kfree in the error path.
>> - Also the iounmap is not called fix the same by calling iounmap in the
>> error case of probe and remove .
>> - Make the name of the error tags more meaningful.
>>
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>> drivers/tty/serial/omap-serial.c | 27 +++++++++++++++++----------
>> 1 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index 1c24269..8c6f137 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -1369,14 +1369,14 @@ static int serial_omap_probe(struct platform_device *pdev)
>>
>> dma_rx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx");
>> if (!dma_rx) {
>> - ret = -EINVAL;
>> - goto err;
>> + ret = -ENXIO;
>> + goto do_release_region;
>> }
>>
>> dma_tx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx");
>> if (!dma_tx) {
>> - ret = -EINVAL;
>> - goto err;
>> + ret = -ENXIO;
>> + goto do_release_region;
>> }
>>
>> up = kzalloc(sizeof(*up), GFP_KERNEL);
>> @@ -1403,7 +1403,7 @@ static int serial_omap_probe(struct platform_device *pdev)
>> dev_err(&pdev->dev, "failed to get alias/pdev id, errno %d\n",
>> up->port.line);
>> ret = -ENODEV;
>> - goto err;
>> + goto err_port_line;
>> }
>>
>> sprintf(up->name, "OMAP UART%d", up->port.line);
>> @@ -1412,7 +1412,7 @@ static int serial_omap_probe(struct platform_device *pdev)
>> if (!up->port.membase) {
>> dev_err(&pdev->dev, "can't ioremap UART\n");
>> ret = -ENOMEM;
>> - goto err;
>> + goto err_ioremap;
>> }
>>
>> up->port.flags = omap_up_info->flags;
>> @@ -1458,16 +1458,22 @@ static int serial_omap_probe(struct platform_device *pdev)
>>
>> ret = uart_add_one_port(&serial_omap_reg, &up->port);
>> if (ret != 0)
>> - goto do_release_region;
>> + goto err_add_port;
>>
>> pm_runtime_put(&pdev->dev);
>> platform_set_drvdata(pdev, up);
>> return 0;
>> -err:
>> - dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n",
>> - pdev->id, __func__, ret);
>> +
>> +err_add_port:
>> + pm_runtime_disable(&pdev->dev);
>> + iounmap(up->port.membase);
>> +err_ioremap:
>> +err_port_line:
>> + kfree(up);
>> do_release_region:
>> release_mem_region(mem->start, resource_size(mem));
>> + dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n",
>> + pdev->id, __func__, ret);
>> return ret;
>> }
>>
>> @@ -1476,6 +1482,7 @@ static int serial_omap_remove(struct platform_device *dev)
>> struct uart_omap_port *up = platform_get_drvdata(dev);
>>
>> if (up) {
>> + iounmap(up->port.membase);
> you can build omap-serial as module insmod and rmmod
> the module and test this patch.
>
> This can be done on zoom board which uses a non-omap uart
> as console.
Yes will do that and post another version.
> --
> Thanks,
> Govindraj.R
WARNING: multiple messages have this Message-ID (diff)
From: shubhrajyoti@ti.com (Shubhrajyoti)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/5] omap-serial: Fix the error handling in the omap_serial probe
Date: Fri, 20 Jan 2012 15:05:06 +0530 [thread overview]
Message-ID: <4F19354A.4040707@ti.com> (raw)
In-Reply-To: <CAAL8m4xVOiGjOHvf=Uy4m9QbqxO21pFeB2HcC_zGijj-+VF_jw@mail.gmail.com>
On Friday 20 January 2012 02:19 PM, Govindraj wrote:
> On Mon, Jan 16, 2012 at 3:52 PM, Shubhrajyoti D <shubhrajyoti@ti.com> wrote:
>> The patch does the following
>>
>> - The pm_runtime_disable is called in the remove not in the error
>> case of probe.The patch calls the pm_runtime_disable in the error
>> case.
>> - The up is not freed in the error path. Fix the memory leak by calling
>> kfree in the error path.
>> - Also the iounmap is not called fix the same by calling iounmap in the
>> error case of probe and remove .
>> - Make the name of the error tags more meaningful.
>>
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>> drivers/tty/serial/omap-serial.c | 27 +++++++++++++++++----------
>> 1 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index 1c24269..8c6f137 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -1369,14 +1369,14 @@ static int serial_omap_probe(struct platform_device *pdev)
>>
>> dma_rx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx");
>> if (!dma_rx) {
>> - ret = -EINVAL;
>> - goto err;
>> + ret = -ENXIO;
>> + goto do_release_region;
>> }
>>
>> dma_tx = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx");
>> if (!dma_tx) {
>> - ret = -EINVAL;
>> - goto err;
>> + ret = -ENXIO;
>> + goto do_release_region;
>> }
>>
>> up = kzalloc(sizeof(*up), GFP_KERNEL);
>> @@ -1403,7 +1403,7 @@ static int serial_omap_probe(struct platform_device *pdev)
>> dev_err(&pdev->dev, "failed to get alias/pdev id, errno %d\n",
>> up->port.line);
>> ret = -ENODEV;
>> - goto err;
>> + goto err_port_line;
>> }
>>
>> sprintf(up->name, "OMAP UART%d", up->port.line);
>> @@ -1412,7 +1412,7 @@ static int serial_omap_probe(struct platform_device *pdev)
>> if (!up->port.membase) {
>> dev_err(&pdev->dev, "can't ioremap UART\n");
>> ret = -ENOMEM;
>> - goto err;
>> + goto err_ioremap;
>> }
>>
>> up->port.flags = omap_up_info->flags;
>> @@ -1458,16 +1458,22 @@ static int serial_omap_probe(struct platform_device *pdev)
>>
>> ret = uart_add_one_port(&serial_omap_reg, &up->port);
>> if (ret != 0)
>> - goto do_release_region;
>> + goto err_add_port;
>>
>> pm_runtime_put(&pdev->dev);
>> platform_set_drvdata(pdev, up);
>> return 0;
>> -err:
>> - dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n",
>> - pdev->id, __func__, ret);
>> +
>> +err_add_port:
>> + pm_runtime_disable(&pdev->dev);
>> + iounmap(up->port.membase);
>> +err_ioremap:
>> +err_port_line:
>> + kfree(up);
>> do_release_region:
>> release_mem_region(mem->start, resource_size(mem));
>> + dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n",
>> + pdev->id, __func__, ret);
>> return ret;
>> }
>>
>> @@ -1476,6 +1482,7 @@ static int serial_omap_remove(struct platform_device *dev)
>> struct uart_omap_port *up = platform_get_drvdata(dev);
>>
>> if (up) {
>> + iounmap(up->port.membase);
> you can build omap-serial as module insmod and rmmod
> the module and test this patch.
>
> This can be done on zoom board which uses a non-omap uart
> as console.
Yes will do that and post another version.
> --
> Thanks,
> Govindraj.R
next prev parent reply other threads:[~2012-01-20 9:35 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-16 10:22 [PATCH 0/5] uart updates Shubhrajyoti D
2012-01-16 10:22 ` Shubhrajyoti D
2012-01-16 10:22 ` [PATCH 1/5] omap-serial :Make the suspend/resume functions depend on CONFIG_PM_SLEEP Shubhrajyoti D
2012-01-16 10:22 ` Shubhrajyoti D
2012-01-16 10:22 ` [PATCH 2/5] omap-serial: make serial_omap_restore_context depend on CONFIG_PM_RUNTIME Shubhrajyoti D
2012-01-16 10:22 ` Shubhrajyoti D
2012-01-16 10:22 ` [PATCH 3/5] omap-serial: Fix the error handling in the omap_serial probe Shubhrajyoti D
2012-01-16 10:22 ` Shubhrajyoti D
2012-01-20 8:49 ` Govindraj
2012-01-20 8:49 ` Govindraj
2012-01-20 9:35 ` Shubhrajyoti [this message]
2012-01-20 9:35 ` Shubhrajyoti
2012-02-10 6:00 ` Shubhrajyoti
2012-02-10 6:00 ` Shubhrajyoti
2012-01-16 10:22 ` [PATCH 4/5] ARM : OMAP : serial : Make context_loss_cnt signed Shubhrajyoti D
2012-01-16 10:22 ` Shubhrajyoti D
2012-01-20 8:52 ` Govindraj
2012-01-20 8:52 ` Govindraj
2012-01-16 10:22 ` [PATCH 5/5] OMAP : serial : Check for error in get_context_loss_count Shubhrajyoti D
2012-01-16 10:22 ` Shubhrajyoti D
2012-01-20 8:51 ` Govindraj
2012-01-20 8:51 ` Govindraj
2012-01-20 8:45 ` [PATCH 0/5] uart updates Govindraj
2012-01-20 8:45 ` Govindraj
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F19354A.4040707@ti.com \
--to=shubhrajyoti@ti.com \
--cc=govindraj.ti@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.