Re: [PATCH] ARM: OMAP2+: omap-usb-host: Fix memory leaks
* Roger Quadros rog...@ti.com [130531 00:23]: Hi Tony, On 05/31/2013 12:00 AM, Tony Lindgren wrote: Hi Roger, * Roger Quadros rog...@ti.com [130524 06:12]: Fix memory leaks in the error path. Also, use platform_device_register_full() to allocate the platform devices and set platform data. If you need this for the v3.10-rc, you should describe why this patch is needed and ideally have some oops or regression causing commit listed. Care to update the description for that? There is no oops or regression happening. Just that there will be a small memory leak if any of the memory allocations fail or if the platform device is destroyed. If it doesn't look that serious to you then it can wait. But if we move to device tree only boot then this patch and the file usb-host.c might not be required at all. OK, applying into omap-for-v3.11/fixes-non-critical as we cannot remove it yet. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP2+: omap-usb-host: Fix memory leaks
Hi Tony,
On 05/31/2013 12:00 AM, Tony Lindgren wrote:
Hi Roger,
* Roger Quadros rog...@ti.com [130524 06:12]:
Fix memory leaks in the error path.
Also, use platform_device_register_full() to allocate
the platform devices and set platform data.
If you need this for the v3.10-rc, you should describe why this patch
is needed and ideally have some oops or regression causing commit
listed. Care to update the description for that?
There is no oops or regression happening. Just that there will be a
small memory leak if any of the memory allocations fail or if the
platform device is destroyed.
If it doesn't look that serious to you then it can wait.
But if we move to device tree only boot then this patch and the file
usb-host.c might not be required at all.
cheers,
-roger
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/mach-omap2/usb-host.c | 106
+---
1 files changed, 56 insertions(+), 50 deletions(-)
diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index aa27d7f..609b330 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -28,6 +28,7 @@
#include linux/io.h
#include linux/gpio.h
#include linux/usb/phy.h
+#include linux/usb/nop-usb-xceiv.h
#include soc.h
#include omap_device.h
@@ -560,7 +561,8 @@ static int usbhs_add_regulator(char *name, char *dev_id,
char *dev_supply,
struct regulator_init_data *reg_data;
struct fixed_voltage_config *config;
struct platform_device *pdev;
-int ret;
+struct platform_device_info pdevinfo;
+int ret = -ENOMEM;
supplies = kzalloc(sizeof(*supplies), GFP_KERNEL);
if (!supplies)
@@ -571,7 +573,7 @@ static int usbhs_add_regulator(char *name, char *dev_id,
char *dev_supply,
reg_data = kzalloc(sizeof(*reg_data), GFP_KERNEL);
if (!reg_data)
-return -ENOMEM;
+goto err_data;
reg_data-constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS;
reg_data-consumer_supplies = supplies;
@@ -580,39 +582,53 @@ static int usbhs_add_regulator(char *name, char
*dev_id, char *dev_supply,
config = kmemdup(hsusb_reg_config, sizeof(hsusb_reg_config),
GFP_KERNEL);
if (!config)
-return -ENOMEM;
+goto err_config;
+
+config-supply_name = kstrdup(name, GFP_KERNEL);
+if (!config-supply_name)
+goto err_supplyname;
-config-supply_name = name;
config-gpio = gpio;
config-enable_high = polarity;
config-init_data = reg_data;
/* create a regulator device */
-pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
-if (!pdev)
-return -ENOMEM;
+memset(pdevinfo, 0, sizeof(pdevinfo));
+pdevinfo.name = reg_name;
+pdevinfo.id = PLATFORM_DEVID_AUTO;
+pdevinfo.data = config;
+pdevinfo.size_data = sizeof(*config);
-pdev-id = PLATFORM_DEVID_AUTO;
-pdev-name = reg_name;
-pdev-dev.platform_data = config;
+pdev = platform_device_register_full(pdevinfo);
+if (IS_ERR(pdev)) {
+ret = PTR_ERR(pdev);
+pr_err(%s: Failed registering regulator %s for %s : %d\n,
+__func__, name, dev_id, ret);
+goto err_register;
+}
-ret = platform_device_register(pdev);
-if (ret)
-pr_err(%s: Failed registering regulator %s for %s\n,
-__func__, name, dev_id);
+return 0;
+err_register:
+kfree(config-supply_name);
+err_supplyname:
+kfree(config);
+err_config:
+kfree(reg_data);
+err_data:
+kfree(supplies);
return ret;
}
+#define MAX_STR 20
+
int usbhs_init_phys(struct usbhs_phy_data *phy, int num_phys)
{
-char *rail_name;
-int i, len;
+char rail_name[MAX_STR];
+int i;
struct platform_device *pdev;
char *phy_id;
-
-/* the phy_id will be something like nop_usb_xceiv.1 */
-len = strlen(nop_name) + 3; /* 3 - .1 and NULL terminator */
+struct platform_device_info pdevinfo;
for (i = 0; i num_phys; i++) {
@@ -627,25 +643,26 @@ int usbhs_init_phys(struct usbhs_phy_data *phy, int
num_phys)
!gpio_is_valid(phy-vcc_gpio))
continue;
-/* create a NOP PHY device */
-pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
-if (!pdev)
-return -ENOMEM;
-
-pdev-id = phy-port;
-pdev-name = nop_name;
-pdev-dev.platform_data = phy-platform_data;
-
-phy_id = kmalloc(len, GFP_KERNEL);
-if (!phy_id)
+phy_id = kmalloc(MAX_STR, GFP_KERNEL);
+if (!phy_id) {
+pr_err(%s: kmalloc() failed\n, __func__);
return -ENOMEM;
+}
-scnprintf(phy_id, len, nop_usb_xceiv.%d\n,
-
Re: [PATCH] ARM: OMAP2+: omap-usb-host: Fix memory leaks
Hi Roger,
* Roger Quadros rog...@ti.com [130524 06:12]:
Fix memory leaks in the error path.
Also, use platform_device_register_full() to allocate
the platform devices and set platform data.
If you need this for the v3.10-rc, you should describe why this patch
is needed and ideally have some oops or regression causing commit
listed. Care to update the description for that?
Regards,
Tony
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/mach-omap2/usb-host.c | 106
+---
1 files changed, 56 insertions(+), 50 deletions(-)
diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index aa27d7f..609b330 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -28,6 +28,7 @@
#include linux/io.h
#include linux/gpio.h
#include linux/usb/phy.h
+#include linux/usb/nop-usb-xceiv.h
#include soc.h
#include omap_device.h
@@ -560,7 +561,8 @@ static int usbhs_add_regulator(char *name, char *dev_id,
char *dev_supply,
struct regulator_init_data *reg_data;
struct fixed_voltage_config *config;
struct platform_device *pdev;
- int ret;
+ struct platform_device_info pdevinfo;
+ int ret = -ENOMEM;
supplies = kzalloc(sizeof(*supplies), GFP_KERNEL);
if (!supplies)
@@ -571,7 +573,7 @@ static int usbhs_add_regulator(char *name, char *dev_id,
char *dev_supply,
reg_data = kzalloc(sizeof(*reg_data), GFP_KERNEL);
if (!reg_data)
- return -ENOMEM;
+ goto err_data;
reg_data-constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS;
reg_data-consumer_supplies = supplies;
@@ -580,39 +582,53 @@ static int usbhs_add_regulator(char *name, char
*dev_id, char *dev_supply,
config = kmemdup(hsusb_reg_config, sizeof(hsusb_reg_config),
GFP_KERNEL);
if (!config)
- return -ENOMEM;
+ goto err_config;
+
+ config-supply_name = kstrdup(name, GFP_KERNEL);
+ if (!config-supply_name)
+ goto err_supplyname;
- config-supply_name = name;
config-gpio = gpio;
config-enable_high = polarity;
config-init_data = reg_data;
/* create a regulator device */
- pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
- if (!pdev)
- return -ENOMEM;
+ memset(pdevinfo, 0, sizeof(pdevinfo));
+ pdevinfo.name = reg_name;
+ pdevinfo.id = PLATFORM_DEVID_AUTO;
+ pdevinfo.data = config;
+ pdevinfo.size_data = sizeof(*config);
- pdev-id = PLATFORM_DEVID_AUTO;
- pdev-name = reg_name;
- pdev-dev.platform_data = config;
+ pdev = platform_device_register_full(pdevinfo);
+ if (IS_ERR(pdev)) {
+ ret = PTR_ERR(pdev);
+ pr_err(%s: Failed registering regulator %s for %s : %d\n,
+ __func__, name, dev_id, ret);
+ goto err_register;
+ }
- ret = platform_device_register(pdev);
- if (ret)
- pr_err(%s: Failed registering regulator %s for %s\n,
- __func__, name, dev_id);
+ return 0;
+err_register:
+ kfree(config-supply_name);
+err_supplyname:
+ kfree(config);
+err_config:
+ kfree(reg_data);
+err_data:
+ kfree(supplies);
return ret;
}
+#define MAX_STR 20
+
int usbhs_init_phys(struct usbhs_phy_data *phy, int num_phys)
{
- char *rail_name;
- int i, len;
+ char rail_name[MAX_STR];
+ int i;
struct platform_device *pdev;
char *phy_id;
-
- /* the phy_id will be something like nop_usb_xceiv.1 */
- len = strlen(nop_name) + 3; /* 3 - .1 and NULL terminator */
+ struct platform_device_info pdevinfo;
for (i = 0; i num_phys; i++) {
@@ -627,25 +643,26 @@ int usbhs_init_phys(struct usbhs_phy_data *phy, int
num_phys)
!gpio_is_valid(phy-vcc_gpio))
continue;
- /* create a NOP PHY device */
- pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
- if (!pdev)
- return -ENOMEM;
-
- pdev-id = phy-port;
- pdev-name = nop_name;
- pdev-dev.platform_data = phy-platform_data;
-
- phy_id = kmalloc(len, GFP_KERNEL);
- if (!phy_id)
+ phy_id = kmalloc(MAX_STR, GFP_KERNEL);
+ if (!phy_id) {
+ pr_err(%s: kmalloc() failed\n, __func__);
return -ENOMEM;
+ }
- scnprintf(phy_id, len, nop_usb_xceiv.%d\n,
- pdev-id);
-
- if (platform_device_register(pdev)) {
- pr_err(%s: Failed to register device %s\n,
- __func__, phy_id);
+ /* create a NOP PHY device */
+ memset(pdevinfo, 0,
[PATCH] ARM: OMAP2+: omap-usb-host: Fix memory leaks
Fix memory leaks in the error path.
Also, use platform_device_register_full() to allocate
the platform devices and set platform data.
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/mach-omap2/usb-host.c | 106 +---
1 files changed, 56 insertions(+), 50 deletions(-)
diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index aa27d7f..609b330 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -28,6 +28,7 @@
#include linux/io.h
#include linux/gpio.h
#include linux/usb/phy.h
+#include linux/usb/nop-usb-xceiv.h
#include soc.h
#include omap_device.h
@@ -560,7 +561,8 @@ static int usbhs_add_regulator(char *name, char *dev_id,
char *dev_supply,
struct regulator_init_data *reg_data;
struct fixed_voltage_config *config;
struct platform_device *pdev;
- int ret;
+ struct platform_device_info pdevinfo;
+ int ret = -ENOMEM;
supplies = kzalloc(sizeof(*supplies), GFP_KERNEL);
if (!supplies)
@@ -571,7 +573,7 @@ static int usbhs_add_regulator(char *name, char *dev_id,
char *dev_supply,
reg_data = kzalloc(sizeof(*reg_data), GFP_KERNEL);
if (!reg_data)
- return -ENOMEM;
+ goto err_data;
reg_data-constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS;
reg_data-consumer_supplies = supplies;
@@ -580,39 +582,53 @@ static int usbhs_add_regulator(char *name, char *dev_id,
char *dev_supply,
config = kmemdup(hsusb_reg_config, sizeof(hsusb_reg_config),
GFP_KERNEL);
if (!config)
- return -ENOMEM;
+ goto err_config;
+
+ config-supply_name = kstrdup(name, GFP_KERNEL);
+ if (!config-supply_name)
+ goto err_supplyname;
- config-supply_name = name;
config-gpio = gpio;
config-enable_high = polarity;
config-init_data = reg_data;
/* create a regulator device */
- pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
- if (!pdev)
- return -ENOMEM;
+ memset(pdevinfo, 0, sizeof(pdevinfo));
+ pdevinfo.name = reg_name;
+ pdevinfo.id = PLATFORM_DEVID_AUTO;
+ pdevinfo.data = config;
+ pdevinfo.size_data = sizeof(*config);
- pdev-id = PLATFORM_DEVID_AUTO;
- pdev-name = reg_name;
- pdev-dev.platform_data = config;
+ pdev = platform_device_register_full(pdevinfo);
+ if (IS_ERR(pdev)) {
+ ret = PTR_ERR(pdev);
+ pr_err(%s: Failed registering regulator %s for %s : %d\n,
+ __func__, name, dev_id, ret);
+ goto err_register;
+ }
- ret = platform_device_register(pdev);
- if (ret)
- pr_err(%s: Failed registering regulator %s for %s\n,
- __func__, name, dev_id);
+ return 0;
+err_register:
+ kfree(config-supply_name);
+err_supplyname:
+ kfree(config);
+err_config:
+ kfree(reg_data);
+err_data:
+ kfree(supplies);
return ret;
}
+#define MAX_STR 20
+
int usbhs_init_phys(struct usbhs_phy_data *phy, int num_phys)
{
- char *rail_name;
- int i, len;
+ char rail_name[MAX_STR];
+ int i;
struct platform_device *pdev;
char *phy_id;
-
- /* the phy_id will be something like nop_usb_xceiv.1 */
- len = strlen(nop_name) + 3; /* 3 - .1 and NULL terminator */
+ struct platform_device_info pdevinfo;
for (i = 0; i num_phys; i++) {
@@ -627,25 +643,26 @@ int usbhs_init_phys(struct usbhs_phy_data *phy, int
num_phys)
!gpio_is_valid(phy-vcc_gpio))
continue;
- /* create a NOP PHY device */
- pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
- if (!pdev)
- return -ENOMEM;
-
- pdev-id = phy-port;
- pdev-name = nop_name;
- pdev-dev.platform_data = phy-platform_data;
-
- phy_id = kmalloc(len, GFP_KERNEL);
- if (!phy_id)
+ phy_id = kmalloc(MAX_STR, GFP_KERNEL);
+ if (!phy_id) {
+ pr_err(%s: kmalloc() failed\n, __func__);
return -ENOMEM;
+ }
- scnprintf(phy_id, len, nop_usb_xceiv.%d\n,
- pdev-id);
-
- if (platform_device_register(pdev)) {
- pr_err(%s: Failed to register device %s\n,
- __func__, phy_id);
+ /* create a NOP PHY device */
+ memset(pdevinfo, 0, sizeof(pdevinfo));
+ pdevinfo.name = nop_name;
+ pdevinfo.id = phy-port;
+ pdevinfo.data = phy-platform_data;
+ pdevinfo.size_data = sizeof(struct nop_usb_xceiv_platform_data);
+
+
