[PATCH v2 0/5] misc/pvpanic: Fix some errro handling path and simplify code

[PATCH v2 0/5] misc/pvpanic: Fix some errro handling path and simplify code

[Christophe JAILLET]

This serie was previously sent in 2 parts, one for -pci.c and one for
-mmio.c.
Execpt the patch 5/5 which is new, the 4 first patches are the same as the
ones previously posted. Only the description has been slighly updated.

Pacth 5/5 is a proposal to simplify code and turn 'pvpanic_probe()' into a
fully resource managed version.
This way callers don't need to do some clean-up on error in the
probe and on remove.


"Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>" was only
added on patch 1 and 3. I was unsure if his Reviewed-by was also related to
the s/GFP_ATOMIC/GFP_KERNEL/ of patch 2 et 4.

Christophe JAILLET (5):
  misc/pvpanic-pci: Fix error handling in 'pvpanic_pci_probe()'
  misc/pvpanic-pci: Use GFP_KERNEL instead of GFP_ATOMIC
  misc/pvpanic-mmio: Fix error handling in 'pvpanic_mmio_probe()'
  misc/pvpanic-mmio: Use GFP_KERNEL instead of GFP_ATOMIC
  misc/pvpanic: Make 'pvpanic_probe()' resource managed

 drivers/misc/pvpanic/pvpanic-mmio.c | 17 ++--------------
 drivers/misc/pvpanic/pvpanic-pci.c  | 22 ++++-----------------
 drivers/misc/pvpanic/pvpanic.c      | 30 ++++++++++++++---------------
 drivers/misc/pvpanic/pvpanic.h      |  3 +--
 4 files changed, 22 insertions(+), 50 deletions(-)

-- 
2.30.2

[PATCH v2 1/5] misc/pvpanic-pci: Fix error handling in 'pvpanic_pci_probe()'

[Christophe JAILLET]

There is no error handling path in the probe function.
Switch to managed resource so that errors in the probe are handled easily
and simplify the remove function accordingly.

Fixes: db3a4f0abefd ("misc/pvpanic: add PCI driver")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: add -pci in the description of the commit
---
 drivers/misc/pvpanic/pvpanic-pci.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/misc/pvpanic/pvpanic-pci.c b/drivers/misc/pvpanic/pvpanic-pci.c
index 9ecc4e8559d5..046ce4ecc195 100644
--- a/drivers/misc/pvpanic/pvpanic-pci.c
+++ b/drivers/misc/pvpanic/pvpanic-pci.c
@@ -78,15 +78,15 @@ static int pvpanic_pci_probe(struct pci_dev *pdev,
 	void __iomem *base;
 	int ret;
 
-	ret = pci_enable_device(pdev);
+	ret = pcim_enable_device(pdev);
 	if (ret < 0)
 		return ret;
 
-	base = pci_iomap(pdev, 0, 0);
+	base = pcim_iomap(pdev, 0, 0);
 	if (!base)
 		return -ENOMEM;
 
-	pi = kmalloc(sizeof(*pi), GFP_ATOMIC);
+	pi = devm_kmalloc(&pdev->dev, sizeof(*pi), GFP_ATOMIC);
 	if (!pi)
 		return -ENOMEM;
 
@@ -107,9 +107,6 @@ static void pvpanic_pci_remove(struct pci_dev *pdev)
 	struct pvpanic_instance *pi = dev_get_drvdata(&pdev->dev);
 
 	pvpanic_remove(pi);
-	iounmap(pi->base);
-	kfree(pi);
-	pci_disable_device(pdev);
 }
 
 static struct pci_driver pvpanic_pci_driver = {
-- 
2.30.2

[PATCH v2 2/5] misc/pvpanic-pci: Use GFP_KERNEL instead of GFP_ATOMIC

[Christophe JAILLET]

There is no need to use GFP_ATOMIC in a probe function. Use GFP_KERNEL
instead.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
v2: add -pci in the description of the commit
---
 drivers/misc/pvpanic/pvpanic-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/pvpanic/pvpanic-pci.c b/drivers/misc/pvpanic/pvpanic-pci.c
index 046ce4ecc195..3d7f9efb3dd4 100644
--- a/drivers/misc/pvpanic/pvpanic-pci.c
+++ b/drivers/misc/pvpanic/pvpanic-pci.c
@@ -86,7 +86,7 @@ static int pvpanic_pci_probe(struct pci_dev *pdev,
 	if (!base)
 		return -ENOMEM;
 
-	pi = devm_kmalloc(&pdev->dev, sizeof(*pi), GFP_ATOMIC);
+	pi = devm_kmalloc(&pdev->dev, sizeof(*pi), GFP_KERNEL);
 	if (!pi)
 		return -ENOMEM;
 
-- 
2.30.2

[PATCH v2 3/5] misc/pvpanic-mmio: Fix error handling in 'pvpanic_mmio_probe()'

[Christophe JAILLET]

There is no error handling path in the probe function.
Switch to managed resource so that errors in the probe are handled easily
and simplify the remove function accordingly.

Fixes: b3c0f8774668 ("misc/pvpanic: probe multiple instances")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: add -mmio in the description of the commit
---
 drivers/misc/pvpanic/pvpanic-mmio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/misc/pvpanic/pvpanic-mmio.c b/drivers/misc/pvpanic/pvpanic-mmio.c
index 4c0841776087..69b31f7adf4f 100644
--- a/drivers/misc/pvpanic/pvpanic-mmio.c
+++ b/drivers/misc/pvpanic/pvpanic-mmio.c
@@ -93,7 +93,7 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	pi = kmalloc(sizeof(*pi), GFP_ATOMIC);
+	pi = devm_kmalloc(dev, sizeof(*pi), GFP_ATOMIC);
 	if (!pi)
 		return -ENOMEM;
 
@@ -114,7 +114,6 @@ static int pvpanic_mmio_remove(struct platform_device *pdev)
 	struct pvpanic_instance *pi = dev_get_drvdata(&pdev->dev);
 
 	pvpanic_remove(pi);
-	kfree(pi);
 
 	return 0;
 }
-- 
2.30.2

[PATCH v2 4/5] misc/pvpanic-mmio: Use GFP_KERNEL instead of GFP_ATOMIC

[Christophe JAILLET]

There is no need to use GFP_ATOMIC in a probe function. Use GFP_KERNEL
instead.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
v2: add -mmio in the description of the commit
---
 drivers/misc/pvpanic/pvpanic-mmio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/pvpanic/pvpanic-mmio.c b/drivers/misc/pvpanic/pvpanic-mmio.c
index 69b31f7adf4f..d4a407956c07 100644
--- a/drivers/misc/pvpanic/pvpanic-mmio.c
+++ b/drivers/misc/pvpanic/pvpanic-mmio.c
@@ -93,7 +93,7 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	pi = devm_kmalloc(dev, sizeof(*pi), GFP_ATOMIC);
+	pi = devm_kmalloc(dev, sizeof(*pi), GFP_KERNEL);
 	if (!pi)
 		return -ENOMEM;
 
-- 
2.30.2

[PATCH v2 5/5] misc/pvpanic: Make 'pvpanic_probe()' resource managed

[Christophe JAILLET]

Simplify code and turn 'pvpanic_probe()' into a managed resource version.
This simplify callers that don't need to do some clean-up on error in the
probe and on remove.

Update pvpanic-mmio.c and pvpanic-pci.c accordingly.

'pvpanic_remove()' don't need to be exported anymore.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Compile tested only
---
 drivers/misc/pvpanic/pvpanic-mmio.c | 14 +-------------
 drivers/misc/pvpanic/pvpanic-pci.c  | 13 +------------
 drivers/misc/pvpanic/pvpanic.c      | 30 ++++++++++++++---------------
 drivers/misc/pvpanic/pvpanic.h      |  3 +--
 4 files changed, 18 insertions(+), 42 deletions(-)

diff --git a/drivers/misc/pvpanic/pvpanic-mmio.c b/drivers/misc/pvpanic/pvpanic-mmio.c
index d4a407956c07..be4016084979 100644
--- a/drivers/misc/pvpanic/pvpanic-mmio.c
+++ b/drivers/misc/pvpanic/pvpanic-mmio.c
@@ -104,18 +104,7 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
 	pi->capability &= ioread8(base);
 	pi->events = pi->capability;
 
-	dev_set_drvdata(dev, pi);
-
-	return pvpanic_probe(pi);
-}
-
-static int pvpanic_mmio_remove(struct platform_device *pdev)
-{
-	struct pvpanic_instance *pi = dev_get_drvdata(&pdev->dev);
-
-	pvpanic_remove(pi);
-
-	return 0;
+	return devm_pvpanic_probe(dev, pi);
 }
 
 static const struct of_device_id pvpanic_mmio_match[] = {
@@ -138,6 +127,5 @@ static struct platform_driver pvpanic_mmio_driver = {
 		.dev_groups = pvpanic_mmio_dev_groups,
 	},
 	.probe = pvpanic_mmio_probe,
-	.remove = pvpanic_mmio_remove,
 };
 module_platform_driver(pvpanic_mmio_driver);
diff --git a/drivers/misc/pvpanic/pvpanic-pci.c b/drivers/misc/pvpanic/pvpanic-pci.c
index 3d7f9efb3dd4..a43c401017ae 100644
--- a/drivers/misc/pvpanic/pvpanic-pci.c
+++ b/drivers/misc/pvpanic/pvpanic-pci.c
@@ -73,7 +73,6 @@ ATTRIBUTE_GROUPS(pvpanic_pci_dev);
 static int pvpanic_pci_probe(struct pci_dev *pdev,
 			     const struct pci_device_id *ent)
 {
-	struct device *dev = &pdev->dev;
 	struct pvpanic_instance *pi;
 	void __iomem *base;
 	int ret;
@@ -97,23 +96,13 @@ static int pvpanic_pci_probe(struct pci_dev *pdev,
 	pi->capability &= ioread8(base);
 	pi->events = pi->capability;
 
-	dev_set_drvdata(dev, pi);
-
-	return pvpanic_probe(pi);
-}
-
-static void pvpanic_pci_remove(struct pci_dev *pdev)
-{
-	struct pvpanic_instance *pi = dev_get_drvdata(&pdev->dev);
-
-	pvpanic_remove(pi);
+	return devm_pvpanic_probe(&pdev->dev, pi);
 }
 
 static struct pci_driver pvpanic_pci_driver = {
 	.name =         "pvpanic-pci",
 	.id_table =     pvpanic_pci_id_tbl,
 	.probe =        pvpanic_pci_probe,
-	.remove =       pvpanic_pci_remove,
 	.driver = {
 		.dev_groups = pvpanic_pci_dev_groups,
 	},
diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
index 793ea0c01193..82770a088d62 100644
--- a/drivers/misc/pvpanic/pvpanic.c
+++ b/drivers/misc/pvpanic/pvpanic.c
@@ -61,22 +61,10 @@ static struct notifier_block pvpanic_panic_nb = {
 	.priority = 1, /* let this called before broken drm_fb_helper */
 };
 
-int pvpanic_probe(struct pvpanic_instance *pi)
-{
-	if (!pi || !pi->base)
-		return -EINVAL;
-
-	spin_lock(&pvpanic_lock);
-	list_add(&pi->list, &pvpanic_list);
-	spin_unlock(&pvpanic_lock);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(pvpanic_probe);
-
-void pvpanic_remove(struct pvpanic_instance *pi)
+static void pvpanic_remove(void *param)
 {
 	struct pvpanic_instance *pi_cur, *pi_next;
+	struct pvpanic_instance *pi = param;
 
 	if (!pi)
 		return;
@@ -90,7 +78,19 @@ void pvpanic_remove(struct pvpanic_instance *pi)
 	}
 	spin_unlock(&pvpanic_lock);
 }
-EXPORT_SYMBOL_GPL(pvpanic_remove);
+
+int devm_pvpanic_probe(struct device *dev, struct pvpanic_instance *pi)
+{
+	if (!pi || !pi->base)
+		return -EINVAL;
+
+	spin_lock(&pvpanic_lock);
+	list_add(&pi->list, &pvpanic_list);
+	spin_unlock(&pvpanic_lock);
+
+	return devm_add_action_or_reset(dev, pvpanic_remove, pi);
+}
+EXPORT_SYMBOL_GPL(devm_pvpanic_probe);
 
 static int pvpanic_init(void)
 {
diff --git a/drivers/misc/pvpanic/pvpanic.h b/drivers/misc/pvpanic/pvpanic.h
index 1afccc2e9fec..493545951754 100644
--- a/drivers/misc/pvpanic/pvpanic.h
+++ b/drivers/misc/pvpanic/pvpanic.h
@@ -15,7 +15,6 @@ struct pvpanic_instance {
 	struct list_head list;
 };
 
-int pvpanic_probe(struct pvpanic_instance *pi);
-void pvpanic_remove(struct pvpanic_instance *pi);
+int devm_pvpanic_probe(struct device *dev, struct pvpanic_instance *pi);
 
 #endif /* PVPANIC_H_ */
-- 
2.30.2

Re: [PATCH v2 0/5] misc/pvpanic: Fix some errro handling path and simplify code

[Andy Shevchenko]

On Sat, May 22, 2021 at 9:54 AM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> This serie was previously sent in 2 parts, one for -pci.c and one for
> -mmio.c.
> Execpt the patch 5/5 which is new, the 4 first patches are the same as the
> ones previously posted. Only the description has been slighly updated.
>
> Pacth 5/5 is a proposal to simplify code and turn 'pvpanic_probe()' into a
> fully resource managed version.
> This way callers don't need to do some clean-up on error in the
> probe and on remove.
>
>
> "Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>" was only
> added on patch 1 and 3. I was unsure if his Reviewed-by was also related to
> the s/GFP_ATOMIC/GFP_KERNEL/ of patch 2 et 4.

To be sure, always send a series with a cover letter :-)
Yes, 2 and 4 were also included.

> Christophe JAILLET (5):
>   misc/pvpanic-pci: Fix error handling in 'pvpanic_pci_probe()'
>   misc/pvpanic-pci: Use GFP_KERNEL instead of GFP_ATOMIC
>   misc/pvpanic-mmio: Fix error handling in 'pvpanic_mmio_probe()'
>   misc/pvpanic-mmio: Use GFP_KERNEL instead of GFP_ATOMIC
>   misc/pvpanic: Make 'pvpanic_probe()' resource managed
>
>  drivers/misc/pvpanic/pvpanic-mmio.c | 17 ++--------------
>  drivers/misc/pvpanic/pvpanic-pci.c  | 22 ++++-----------------
>  drivers/misc/pvpanic/pvpanic.c      | 30 ++++++++++++++---------------
>  drivers/misc/pvpanic/pvpanic.h      |  3 +--
>  4 files changed, 22 insertions(+), 50 deletions(-)
>
> --
> 2.30.2
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/5] misc/pvpanic: Make 'pvpanic_probe()' resource managed

[Andy Shevchenko]

On Sat, May 22, 2021 at 9:56 AM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> Simplify code and turn 'pvpanic_probe()' into a managed resource version.
> This simplify callers that don't need to do some clean-up on error in the
> probe and on remove.
>
> Update pvpanic-mmio.c and pvpanic-pci.c accordingly.
>
> 'pvpanic_remove()' don't need to be exported anymore.

LGTM, thanks!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> Compile tested only
> ---
>  drivers/misc/pvpanic/pvpanic-mmio.c | 14 +-------------
>  drivers/misc/pvpanic/pvpanic-pci.c  | 13 +------------
>  drivers/misc/pvpanic/pvpanic.c      | 30 ++++++++++++++---------------
>  drivers/misc/pvpanic/pvpanic.h      |  3 +--
>  4 files changed, 18 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/misc/pvpanic/pvpanic-mmio.c b/drivers/misc/pvpanic/pvpanic-mmio.c
> index d4a407956c07..be4016084979 100644
> --- a/drivers/misc/pvpanic/pvpanic-mmio.c
> +++ b/drivers/misc/pvpanic/pvpanic-mmio.c
> @@ -104,18 +104,7 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
>         pi->capability &= ioread8(base);
>         pi->events = pi->capability;
>
> -       dev_set_drvdata(dev, pi);
> -
> -       return pvpanic_probe(pi);
> -}
> -
> -static int pvpanic_mmio_remove(struct platform_device *pdev)
> -{
> -       struct pvpanic_instance *pi = dev_get_drvdata(&pdev->dev);
> -
> -       pvpanic_remove(pi);
> -
> -       return 0;
> +       return devm_pvpanic_probe(dev, pi);
>  }
>
>  static const struct of_device_id pvpanic_mmio_match[] = {
> @@ -138,6 +127,5 @@ static struct platform_driver pvpanic_mmio_driver = {
>                 .dev_groups = pvpanic_mmio_dev_groups,
>         },
>         .probe = pvpanic_mmio_probe,
> -       .remove = pvpanic_mmio_remove,
>  };
>  module_platform_driver(pvpanic_mmio_driver);
> diff --git a/drivers/misc/pvpanic/pvpanic-pci.c b/drivers/misc/pvpanic/pvpanic-pci.c
> index 3d7f9efb3dd4..a43c401017ae 100644
> --- a/drivers/misc/pvpanic/pvpanic-pci.c
> +++ b/drivers/misc/pvpanic/pvpanic-pci.c
> @@ -73,7 +73,6 @@ ATTRIBUTE_GROUPS(pvpanic_pci_dev);
>  static int pvpanic_pci_probe(struct pci_dev *pdev,
>                              const struct pci_device_id *ent)
>  {
> -       struct device *dev = &pdev->dev;
>         struct pvpanic_instance *pi;
>         void __iomem *base;
>         int ret;
> @@ -97,23 +96,13 @@ static int pvpanic_pci_probe(struct pci_dev *pdev,
>         pi->capability &= ioread8(base);
>         pi->events = pi->capability;
>
> -       dev_set_drvdata(dev, pi);
> -
> -       return pvpanic_probe(pi);
> -}
> -
> -static void pvpanic_pci_remove(struct pci_dev *pdev)
> -{
> -       struct pvpanic_instance *pi = dev_get_drvdata(&pdev->dev);
> -
> -       pvpanic_remove(pi);
> +       return devm_pvpanic_probe(&pdev->dev, pi);
>  }
>
>  static struct pci_driver pvpanic_pci_driver = {
>         .name =         "pvpanic-pci",
>         .id_table =     pvpanic_pci_id_tbl,
>         .probe =        pvpanic_pci_probe,
> -       .remove =       pvpanic_pci_remove,
>         .driver = {
>                 .dev_groups = pvpanic_pci_dev_groups,
>         },
> diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
> index 793ea0c01193..82770a088d62 100644
> --- a/drivers/misc/pvpanic/pvpanic.c
> +++ b/drivers/misc/pvpanic/pvpanic.c
> @@ -61,22 +61,10 @@ static struct notifier_block pvpanic_panic_nb = {
>         .priority = 1, /* let this called before broken drm_fb_helper */
>  };
>
> -int pvpanic_probe(struct pvpanic_instance *pi)
> -{
> -       if (!pi || !pi->base)
> -               return -EINVAL;
> -
> -       spin_lock(&pvpanic_lock);
> -       list_add(&pi->list, &pvpanic_list);
> -       spin_unlock(&pvpanic_lock);
> -
> -       return 0;
> -}
> -EXPORT_SYMBOL_GPL(pvpanic_probe);
> -
> -void pvpanic_remove(struct pvpanic_instance *pi)
> +static void pvpanic_remove(void *param)
>  {
>         struct pvpanic_instance *pi_cur, *pi_next;
> +       struct pvpanic_instance *pi = param;
>
>         if (!pi)
>                 return;
> @@ -90,7 +78,19 @@ void pvpanic_remove(struct pvpanic_instance *pi)
>         }
>         spin_unlock(&pvpanic_lock);
>  }
> -EXPORT_SYMBOL_GPL(pvpanic_remove);
> +
> +int devm_pvpanic_probe(struct device *dev, struct pvpanic_instance *pi)
> +{
> +       if (!pi || !pi->base)
> +               return -EINVAL;
> +
> +       spin_lock(&pvpanic_lock);
> +       list_add(&pi->list, &pvpanic_list);
> +       spin_unlock(&pvpanic_lock);
> +
> +       return devm_add_action_or_reset(dev, pvpanic_remove, pi);
> +}
> +EXPORT_SYMBOL_GPL(devm_pvpanic_probe);
>
>  static int pvpanic_init(void)
>  {
> diff --git a/drivers/misc/pvpanic/pvpanic.h b/drivers/misc/pvpanic/pvpanic.h
> index 1afccc2e9fec..493545951754 100644
> --- a/drivers/misc/pvpanic/pvpanic.h
> +++ b/drivers/misc/pvpanic/pvpanic.h
> @@ -15,7 +15,6 @@ struct pvpanic_instance {
>         struct list_head list;
>  };
>
> -int pvpanic_probe(struct pvpanic_instance *pi);
> -void pvpanic_remove(struct pvpanic_instance *pi);
> +int devm_pvpanic_probe(struct device *dev, struct pvpanic_instance *pi);
>
>  #endif /* PVPANIC_H_ */
> --
> 2.30.2
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/5] misc/pvpanic: Make 'pvpanic_probe()' resource managed

[Andy Shevchenko]

On Sat, May 22, 2021 at 1:06 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, May 22, 2021 at 9:56 AM Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:

Hmm... Couple of (minor) comments though.

> > Simplify code and turn 'pvpanic_probe()' into a managed resource version.
> > This simplify callers that don't need to do some clean-up on error in the

simplifies
errors

> > probe and on remove.
> >
> > Update pvpanic-mmio.c and pvpanic-pci.c accordingly.
> >
> > 'pvpanic_remove()' don't need to be exported anymore.
>
> LGTM, thanks!
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> > +static void pvpanic_remove(void *param)
> >  {
> >         struct pvpanic_instance *pi_cur, *pi_next;
> > +       struct pvpanic_instance *pi = param;

> >         if (!pi)
> >                 return;

Looking at this I'm wondering why it's not a dead code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/5] misc/pvpanic: Make 'pvpanic_probe()' resource managed

[Christophe JAILLET]

Le 22/05/2021 à 12:09, Andy Shevchenko a écrit :
> On Sat, May 22, 2021 at 1:06 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Sat, May 22, 2021 at 9:56 AM Christophe JAILLET
>> <christophe.jaillet@wanadoo.fr> wrote:
> 
> Hmm... Couple of (minor) comments though.
> 
>>> Simplify code and turn 'pvpanic_probe()' into a managed resource version.
>>> This simplify callers that don't need to do some clean-up on error in the
> 
> simplifies
> errors
> 

Ok

>>> probe and on remove.
>>>
>>> Update pvpanic-mmio.c and pvpanic-pci.c accordingly.
>>>
>>> 'pvpanic_remove()' don't need to be exported anymore.
>>
>> LGTM, thanks!
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>>> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> ...
> 
>>> +static void pvpanic_remove(void *param)
>>>   {
>>>          struct pvpanic_instance *pi_cur, *pi_next;
>>> +       struct pvpanic_instance *pi = param;
> 
>>>          if (!pi)
>>>                  return;
> 
> Looking at this I'm wondering why it's not a dead code.
> 
Agreed.

I'll send a v3, but my turn to nitpick now:

    Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
    Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Which one, should I use?
I guess the later.

CJ

Re: [PATCH v2 5/5] misc/pvpanic: Make 'pvpanic_probe()' resource managed

[Andy Shevchenko]

On Sat, May 22, 2021 at 1:57 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
> Le 22/05/2021 à 12:09, Andy Shevchenko a écrit :

...

> I'll send a v3, but my turn to nitpick now:
>
>     Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>     Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Which one, should I use?
> I guess the later.

Both. They have different meanings.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/5] misc/pvpanic: Make 'pvpanic_probe()' resource managed

[Christophe JAILLET]

Le 22/05/2021 à 13:06, Andy Shevchenko a écrit :
> On Sat, May 22, 2021 at 1:57 PM Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:
>> Le 22/05/2021 à 12:09, Andy Shevchenko a écrit :
> 
> ...
> 
>> I'll send a v3, but my turn to nitpick now:
>>
>>      Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>      Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>
>> Which one, should I use?
>> I guess the later.
> 
> Both. They have different meanings.
> 

I was meaning gmail.com or intel.com

CJ