[PATCH platform 0/3] platform/x86: Add fixes and amendments for init()/exit() flows

[PATCH platform 0/3] platform/x86: Add fixes and amendments for init()/exit() flows

[Vadim Pasternak]

The patchset:
- Fixes memory leak in error handling flow.
- Add cosmetic changes - misspelling fix, better naming/

Patches #1: Fixes memory leak issue.
Patch #2: Fix misspelling.
Patches #3: Renames few routines for better naming convention.

Vadim Pasternak (3):
  platform: mellanox: Fix a resource leak in an error handling path in
    probing flow
  platform: mellanox: Fix misspelling error in routine name
  platform: mellanox: Rename some init()/exit() functions for consistent
    naming

 drivers/platform/x86/mlx-platform.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

-- 
2.20.1

[PATCH platform 1/3] platform: mellanox: Fix a resource leak in an error handling path in probing flow

[Vadim Pasternak]

Fix missed resource deallocation in rollback flows.

Currently if an error occurs after a successful
mlxplat_i2c_main_init(), mlxplat_i2c_main_exit() call is missed in
rollback flow.
Thus, some resources are not de-allocated.

Move mlxplat_pre_exit() call from mlxplat_remove() into
mlxplat_i2c_main_exit().

Call mlxplat_i2c_main_exit() instead of calling mlxplat_pre_exit() in
mlxplat_probe() error handling flow.

Unregister 'priv->pdev_i2c' device in mlxplat_i2c_main_init() cleanup
flow if this device was successfully registered.

Fixes: 158cd8320776 ("platform: mellanox: Split logic in init and exit flow")
Reported-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Closes: https://lore.kernel.org/lkml/70165032-796e-6f5c-6748-f514e3b9d08c@linux.intel.com/T/
Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
---
 drivers/platform/x86/mlx-platform.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index 3d96dbf79a72..a2ffe4157df1 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -6514,6 +6514,7 @@ static int mlxplat_i2c_main_init(struct mlxplat_priv *priv)
 	return 0;
 
 fail_mlxplat_i2c_mux_topology_init:
+	platform_device_unregister(priv->pdev_i2c);
 fail_platform_i2c_register:
 fail_mlxplat_mlxcpld_verify_bus_topology:
 	return err;
@@ -6521,6 +6522,7 @@ static int mlxplat_i2c_main_init(struct mlxplat_priv *priv)
 
 static void mlxplat_i2c_main_exit(struct mlxplat_priv *priv)
 {
+	mlxplat_pre_exit(priv);
 	mlxplat_i2c_mux_topology_exit(priv);
 	if (priv->pdev_i2c)
 		platform_device_unregister(priv->pdev_i2c);
@@ -6597,7 +6599,7 @@ static int mlxplat_probe(struct platform_device *pdev)
 
 fail_register_reboot_notifier:
 fail_regcache_sync:
-	mlxplat_pre_exit(priv);
+	mlxplat_i2c_main_exit(priv);
 fail_mlxplat_i2c_main_init:
 fail_regmap_write:
 fail_alloc:
@@ -6614,7 +6616,6 @@ static int mlxplat_remove(struct platform_device *pdev)
 		pm_power_off = NULL;
 	if (mlxplat_reboot_nb)
 		unregister_reboot_notifier(mlxplat_reboot_nb);
-	mlxplat_pre_exit(priv);
 	mlxplat_i2c_main_exit(priv);
 	mlxplat_post_exit();
 	return 0;
-- 
2.20.1

[PATCH platform 2/3] platform: mellanox: Fix misspelling error in routine name

[Vadim Pasternak]

Change mlxplat_i2c_main_complition_notify() to
mlxplat_i2c_main_completion_notify().

Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
---
 drivers/platform/x86/mlx-platform.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index a2ffe4157df1..5b4e57c37f2c 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -368,7 +368,7 @@ struct mlxplat_priv {
 };
 
 static struct platform_device *mlxplat_dev;
-static int mlxplat_i2c_main_complition_notify(void *handle, int id);
+static int mlxplat_i2c_main_completion_notify(void *handle, int id);
 static void __iomem *i2c_bridge_addr, *jtag_bridge_addr;
 
 /* Regions for LPC I2C controller and LPC base register space */
@@ -384,7 +384,7 @@ static const struct resource mlxplat_lpc_resources[] = {
 
 /* Platform systems default i2c data */
 static struct mlxreg_core_hotplug_platform_data mlxplat_mlxcpld_i2c_default_data = {
-	.completion_notify = mlxplat_i2c_main_complition_notify,
+	.completion_notify = mlxplat_i2c_main_completion_notify,
 };
 
 /* Platform i2c next generation systems data */
@@ -409,7 +409,7 @@ static struct mlxreg_core_hotplug_platform_data mlxplat_mlxcpld_i2c_ng_data = {
 	.mask = MLXPLAT_CPLD_AGGR_MASK_COMEX,
 	.cell_low = MLXPLAT_CPLD_LPC_REG_AGGRCO_OFFSET,
 	.mask_low = MLXPLAT_CPLD_LOW_AGGR_MASK_I2C,
-	.completion_notify = mlxplat_i2c_main_complition_notify,
+	.completion_notify = mlxplat_i2c_main_completion_notify,
 };
 
 /* Platform default channels */
@@ -6471,7 +6471,7 @@ static void mlxplat_i2c_mux_topology_exit(struct mlxplat_priv *priv)
 	}
 }
 
-static int mlxplat_i2c_main_complition_notify(void *handle, int id)
+static int mlxplat_i2c_main_completion_notify(void *handle, int id)
 {
 	struct mlxplat_priv *priv = handle;
 
-- 
2.20.1

[PATCH platform 3/3] platform: mellanox: Rename some init()/exit() functions for consistent naming

[Vadim Pasternak]

Currently some names of init()/exit() pairing function are not
consistent.

Rename pair mlxplat_pre_init()/mlxplat_post_exit() to respectively
mlxplat_logicdev_init()/mlxplat_logicdev_exit().

Rename pair mlxplat_post_init()/mlxplat_pre_exit() to respectively
mlxplat_platdevs_init()/mlxplat_platdevs_exit().

Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
---
 drivers/platform/x86/mlx-platform.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
index 5b4e57c37f2c..1bad4c64f36c 100644
--- a/drivers/platform/x86/mlx-platform.c
+++ b/drivers/platform/x86/mlx-platform.c
@@ -6291,7 +6291,7 @@ static void mlxplat_pci_fpga_devices_exit(void)
 }
 
 static int
-mlxplat_pre_init(struct resource **hotplug_resources, unsigned int *hotplug_resources_size)
+mlxplat_logicdev_init(struct resource **hotplug_resources, unsigned int *hotplug_resources_size)
 {
 	int err;
 
@@ -6302,7 +6302,7 @@ mlxplat_pre_init(struct resource **hotplug_resources, unsigned int *hotplug_reso
 	return err;
 }
 
-static void mlxplat_post_exit(void)
+static void mlxplat_logicdev_exit(void)
 {
 	if (lpc_bridge)
 		mlxplat_pci_fpga_devices_exit();
@@ -6310,7 +6310,7 @@ static void mlxplat_post_exit(void)
 		mlxplat_lpc_cpld_device_exit();
 }
 
-static int mlxplat_post_init(struct mlxplat_priv *priv)
+static int mlxplat_platdevs_init(struct mlxplat_priv *priv)
 {
 	int i = 0, err;
 
@@ -6407,7 +6407,7 @@ static int mlxplat_post_init(struct mlxplat_priv *priv)
 	return err;
 }
 
-static void mlxplat_pre_exit(struct mlxplat_priv *priv)
+static void mlxplat_platdevs_exit(struct mlxplat_priv *priv)
 {
 	int i;
 
@@ -6429,7 +6429,7 @@ mlxplat_i2c_mux_complition_notify(void *handle, struct i2c_adapter *parent,
 {
 	struct mlxplat_priv *priv = handle;
 
-	return mlxplat_post_init(priv);
+	return mlxplat_platdevs_init(priv);
 }
 
 static int mlxplat_i2c_mux_topology_init(struct mlxplat_priv *priv)
@@ -6522,7 +6522,7 @@ static int mlxplat_i2c_main_init(struct mlxplat_priv *priv)
 
 static void mlxplat_i2c_main_exit(struct mlxplat_priv *priv)
 {
-	mlxplat_pre_exit(priv);
+	mlxplat_platdevs_exit(priv);
 	mlxplat_i2c_mux_topology_exit(priv);
 	if (priv->pdev_i2c)
 		platform_device_unregister(priv->pdev_i2c);
@@ -6544,7 +6544,7 @@ static int mlxplat_probe(struct platform_device *pdev)
 		mlxplat_dev = pdev;
 	}
 
-	err = mlxplat_pre_init(&hotplug_resources, &hotplug_resources_size);
+	err = mlxplat_logicdev_init(&hotplug_resources, &hotplug_resources_size);
 	if (err)
 		return err;
 
@@ -6603,7 +6603,7 @@ static int mlxplat_probe(struct platform_device *pdev)
 fail_mlxplat_i2c_main_init:
 fail_regmap_write:
 fail_alloc:
-	mlxplat_post_exit();
+	mlxplat_logicdev_exit();
 
 	return err;
 }
@@ -6617,7 +6617,7 @@ static int mlxplat_remove(struct platform_device *pdev)
 	if (mlxplat_reboot_nb)
 		unregister_reboot_notifier(mlxplat_reboot_nb);
 	mlxplat_i2c_main_exit(priv);
-	mlxplat_post_exit();
+	mlxplat_logicdev_exit();
 	return 0;
 }
 
-- 
2.20.1

Re: [PATCH platform 1/3] platform: mellanox: Fix a resource leak in an error handling path in probing flow

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2450 bytes --]

On Thu, 5 Oct 2023, Vadim Pasternak wrote:

> Fix missed resource deallocation in rollback flows.
> 
> Currently if an error occurs after a successful
> mlxplat_i2c_main_init(), mlxplat_i2c_main_exit() call is missed in
> rollback flow.
> Thus, some resources are not de-allocated.
> 
> Move mlxplat_pre_exit() call from mlxplat_remove() into
> mlxplat_i2c_main_exit().
> 
> Call mlxplat_i2c_main_exit() instead of calling mlxplat_pre_exit() in
> mlxplat_probe() error handling flow.
> 
> Unregister 'priv->pdev_i2c' device in mlxplat_i2c_main_init() cleanup
> flow if this device was successfully registered.
> 
> Fixes: 158cd8320776 ("platform: mellanox: Split logic in init and exit flow")
> Reported-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Closes: https://lore.kernel.org/lkml/70165032-796e-6f5c-6748-f514e3b9d08c@linux.intel.com/T/
> Signed-off-by: Vadim Pasternak <vadimp@nvidia.com>
> ---
>  drivers/platform/x86/mlx-platform.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/mlx-platform.c b/drivers/platform/x86/mlx-platform.c
> index 3d96dbf79a72..a2ffe4157df1 100644
> --- a/drivers/platform/x86/mlx-platform.c
> +++ b/drivers/platform/x86/mlx-platform.c
> @@ -6514,6 +6514,7 @@ static int mlxplat_i2c_main_init(struct mlxplat_priv *priv)
>  	return 0;
>  
>  fail_mlxplat_i2c_mux_topology_init:
> +	platform_device_unregister(priv->pdev_i2c);
>  fail_platform_i2c_register:
>  fail_mlxplat_mlxcpld_verify_bus_topology:
>  	return err;
> @@ -6521,6 +6522,7 @@ static int mlxplat_i2c_main_init(struct mlxplat_priv *priv)
>  
>  static void mlxplat_i2c_main_exit(struct mlxplat_priv *priv)
>  {
> +	mlxplat_pre_exit(priv);
>  	mlxplat_i2c_mux_topology_exit(priv);
>  	if (priv->pdev_i2c)
>  		platform_device_unregister(priv->pdev_i2c);
> @@ -6597,7 +6599,7 @@ static int mlxplat_probe(struct platform_device *pdev)
>  
>  fail_register_reboot_notifier:
>  fail_regcache_sync:
> -	mlxplat_pre_exit(priv);
> +	mlxplat_i2c_main_exit(priv);
>  fail_mlxplat_i2c_main_init:
>  fail_regmap_write:
>  fail_alloc:
> @@ -6614,7 +6616,6 @@ static int mlxplat_remove(struct platform_device *pdev)
>  		pm_power_off = NULL;
>  	if (mlxplat_reboot_nb)
>  		unregister_reboot_notifier(mlxplat_reboot_nb);
> -	mlxplat_pre_exit(priv);
>  	mlxplat_i2c_main_exit(priv);
>  	mlxplat_post_exit();
>  	return 0;
> 

Thanks,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH platform 0/3] platform/x86: Add fixes and amendments for init()/exit() flows

[Ilpo Järvinen]

On Thu, 5 Oct 2023, Vadim Pasternak wrote:

> The patchset:
> - Fixes memory leak in error handling flow.
> - Add cosmetic changes - misspelling fix, better naming/
> 
> Patches #1: Fixes memory leak issue.
> Patch #2: Fix misspelling.
> Patches #3: Renames few routines for better naming convention.
> 
> Vadim Pasternak (3):
>   platform: mellanox: Fix a resource leak in an error handling path in
>     probing flow
>   platform: mellanox: Fix misspelling error in routine name
>   platform: mellanox: Rename some init()/exit() functions for consistent
>     naming

Thanks for the patches. I've applied these into 
platform-drivers-x86-mellanox-init branch and merged that into 
review-ilpo. (I decided to reorder the patches 2 and 3 though as it felt 
more logical ordering.)


-- 
 i.