Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Andy Shevchenko]

On Tue, May 15, 2018 at 7:48 PM, Andy Shevchenko
 wrote:
> On Tue, May 15, 2018 at 12:07 PM, Jan Kiszka  wrote:

>> +   devm_kfree(dev, res);

devm_kfree() makes my eyes hurt.

> res = devm_kmemdump();

devm_kmemdup();

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Andy Shevchenko]

On Tue, May 15, 2018 at 7:48 PM, Andy Shevchenko
 wrote:
> On Tue, May 15, 2018 at 12:07 PM, Jan Kiszka  wrote:

>> +   devm_kfree(dev, res);

devm_kfree() makes my eyes hurt.

> res = devm_kmemdump();

devm_kmemdup();

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Andy Shevchenko]

On Tue, May 15, 2018 at 12:07 PM, Jan Kiszka  wrote:
> From: Jan Kiszka 
>
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.

> -   res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> +   res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> if (!res) {
> err = -ENOMEM;
> -   goto parse_failed;
> +   goto failed;
> }
>
> err = of_pci_range_to_resource(, dev_node, res);
> if (err) {
> -   kfree(res);
> +   devm_kfree(dev, res);
> continue;
> }

Can't you rather make it better, i.e.

struct resource tmp;
...

err = of_pci_range_to_resource(, dev_node, );
if (err)
   continue;

res = devm_kmemdump();
if (!res) {
 ret = -ENOMEM;
 goto failed;
}

?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Andy Shevchenko]

On Tue, May 15, 2018 at 12:07 PM, Jan Kiszka  wrote:
> From: Jan Kiszka 
>
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.

> -   res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> +   res = devm_kzalloc(dev, sizeof(struct resource), GFP_KERNEL);
> if (!res) {
> err = -ENOMEM;
> -   goto parse_failed;
> +   goto failed;
> }
>
> err = of_pci_range_to_resource(, dev_node, res);
> if (err) {
> -   kfree(res);
> +   devm_kfree(dev, res);
> continue;
> }

Can't you rather make it better, i.e.

struct resource tmp;
...

err = of_pci_range_to_resource(, dev_node, );
if (err)
   continue;

res = devm_kmemdump();
if (!res) {
 ret = -ENOMEM;
 goto failed;
}

?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Jingoo Han]

On Tuesday, May 15, 2018 5:07 AM, Jan Kiszka wrote:
> 
> From: Jan Kiszka 
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han 
> CC: Joao Pinto 
> CC: Lorenzo Pieralisi 
> Signed-off-by: Jan Kiszka 

Acked-by: Jingoo Han 

Best regards,
Jingoo Han

> ---
>  drivers/pci/dwc/pcie-designware-host.c |  2 +-
>  drivers/pci/host/pci-aardvark.c|  2 +-
>  drivers/pci/host/pci-ftpci100.c|  2 +-
>  drivers/pci/host/pci-v3-semi.c |  2 +-
>  drivers/pci/host/pci-versatile.c   |  2 +-
>  drivers/pci/host/pci-xgene.c   |  2 +-
>  drivers/pci/host/pcie-altera.c |  2 +-
>  drivers/pci/host/pcie-iproc-platform.c |  2 +-
>  drivers/pci/host/pcie-rcar.c   |  2 +-
>  drivers/pci/host/pcie-rockchip.c   |  2 +-
>  drivers/pci/host/pcie-xilinx-nwl.c |  2 +-
>  drivers/pci/host/pcie-xilinx.c |  2 +-
>  drivers/pci/of.c   | 30
--
>  include/linux/of_pci.h |  4 ++--
>  14 files changed, 26 insertions(+), 32 deletions(-)
> 

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Jingoo Han]

On Tuesday, May 15, 2018 5:07 AM, Jan Kiszka wrote:
> 
> From: Jan Kiszka 
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han 
> CC: Joao Pinto 
> CC: Lorenzo Pieralisi 
> Signed-off-by: Jan Kiszka 

Acked-by: Jingoo Han 

Best regards,
Jingoo Han

> ---
>  drivers/pci/dwc/pcie-designware-host.c |  2 +-
>  drivers/pci/host/pci-aardvark.c|  2 +-
>  drivers/pci/host/pci-ftpci100.c|  2 +-
>  drivers/pci/host/pci-v3-semi.c |  2 +-
>  drivers/pci/host/pci-versatile.c   |  2 +-
>  drivers/pci/host/pci-xgene.c   |  2 +-
>  drivers/pci/host/pcie-altera.c |  2 +-
>  drivers/pci/host/pcie-iproc-platform.c |  2 +-
>  drivers/pci/host/pcie-rcar.c   |  2 +-
>  drivers/pci/host/pcie-rockchip.c   |  2 +-
>  drivers/pci/host/pcie-xilinx-nwl.c |  2 +-
>  drivers/pci/host/pcie-xilinx.c |  2 +-
>  drivers/pci/of.c   | 30
--
>  include/linux/of_pci.h |  4 ++--
>  14 files changed, 26 insertions(+), 32 deletions(-)
> 

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Joao Pinto]

Hi Jan,

Às 10:07 AM de 5/15/2018, Jan Kiszka escreveu:
> From: Jan Kiszka 
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han 
> CC: Joao Pinto 
> CC: Lorenzo Pieralisi 
> Signed-off-by: Jan Kiszka 
> ---
>  drivers/pci/dwc/pcie-designware-host.c |  2 +-
>  drivers/pci/host/pci-aardvark.c|  2 +-
>  drivers/pci/host/pci-ftpci100.c|  2 +-
>  drivers/pci/host/pci-v3-semi.c |  2 +-
>  drivers/pci/host/pci-versatile.c   |  2 +-
>  drivers/pci/host/pci-xgene.c   |  2 +-
>  drivers/pci/host/pcie-altera.c |  2 +-
>  drivers/pci/host/pcie-iproc-platform.c |  2 +-
>  drivers/pci/host/pcie-rcar.c   |  2 +-
>  drivers/pci/host/pcie-rockchip.c   |  2 +-
>  drivers/pci/host/pcie-xilinx-nwl.c |  2 +-
>  drivers/pci/host/pcie-xilinx.c |  2 +-
>  drivers/pci/of.c   | 30 --
>  include/linux/of_pci.h |  4 ++--
>  14 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c 
> b/drivers/pci/dwc/pcie-designware-host.c
> index 5a535690b7b5..a8f6ab54b4c0 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -342,7 +342,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>   if (!bridge)
>   return -ENOMEM;
>  
> - ret = of_pci_get_host_bridge_resources(dev, 0, 0xff,
> + ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
>   >windows, >io_base);
>   if (ret)
>   return ret;

(snip...)

Thanks for this patch!

Acked-by: Joao Pinto 

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Joao Pinto]

Hi Jan,

Às 10:07 AM de 5/15/2018, Jan Kiszka escreveu:
> From: Jan Kiszka 
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han 
> CC: Joao Pinto 
> CC: Lorenzo Pieralisi 
> Signed-off-by: Jan Kiszka 
> ---
>  drivers/pci/dwc/pcie-designware-host.c |  2 +-
>  drivers/pci/host/pci-aardvark.c|  2 +-
>  drivers/pci/host/pci-ftpci100.c|  2 +-
>  drivers/pci/host/pci-v3-semi.c |  2 +-
>  drivers/pci/host/pci-versatile.c   |  2 +-
>  drivers/pci/host/pci-xgene.c   |  2 +-
>  drivers/pci/host/pcie-altera.c |  2 +-
>  drivers/pci/host/pcie-iproc-platform.c |  2 +-
>  drivers/pci/host/pcie-rcar.c   |  2 +-
>  drivers/pci/host/pcie-rockchip.c   |  2 +-
>  drivers/pci/host/pcie-xilinx-nwl.c |  2 +-
>  drivers/pci/host/pcie-xilinx.c |  2 +-
>  drivers/pci/of.c   | 30 --
>  include/linux/of_pci.h |  4 ++--
>  14 files changed, 26 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c 
> b/drivers/pci/dwc/pcie-designware-host.c
> index 5a535690b7b5..a8f6ab54b4c0 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -342,7 +342,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>   if (!bridge)
>   return -ENOMEM;
>  
> - ret = of_pci_get_host_bridge_resources(dev, 0, 0xff,
> + ret = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
>   >windows, >io_base);
>   if (ret)
>   return ret;

(snip...)

Thanks for this patch!

Acked-by: Joao Pinto 

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Vladimir Zapolskiy]

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han 
> CC: Joao Pinto 
> CC: Lorenzo Pieralisi 
> Signed-off-by: Jan Kiszka 

Now it is correct, thanks.

Tested-by: Vladimir Zapolskiy 
Reviewed-by: Vladimir Zapolskiy 

--
With best wishes,
Vladimir

Re: [PATCH v4 6/8] PCI: Rework of_pci_get_host_bridge_resources() to devm_of_pci_get_host_bridge_resources()

[Vladimir Zapolskiy]

On 05/15/2018 12:07 PM, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> of_pci_get_host_bridge_resources() allocates the resource structures it
> fills dynamically, but none of its callers care to release them so far.
> Rather than requiring everyone to do this explicitly, convert the
> existing function to a managed version.
> 
> CC: Jingoo Han 
> CC: Joao Pinto 
> CC: Lorenzo Pieralisi 
> Signed-off-by: Jan Kiszka 

Now it is correct, thanks.

Tested-by: Vladimir Zapolskiy 
Reviewed-by: Vladimir Zapolskiy 

--
With best wishes,
Vladimir