netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v1 0/5] ptp_ocp: set of small cleanups
@ 2022-06-08 12:03 Andy Shevchenko
  2022-06-08 12:03 ` [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe() Andy Shevchenko
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-08 12:03 UTC (permalink / raw)
  To: Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran, Andy Shevchenko

The set of (independent) cleanups against ptp_ocp driver.
Each patch has its own description, no need to repeat it here.

Andy Shevchenko (5):
  ptp_ocp: use dev_err_probe()
  ptp_ocp: use bits.h macros for all masks
  ptp_ocp: drop duplicate NULL check in ptp_ocp_detach()
  ptp_ocp: do not call pci_set_drvdata(pdev, NULL)
  ptp_ocp: replace kzalloc(x*y) by kcalloc(y, x)

 drivers/ptp/ptp_ocp.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe()
  2022-06-08 12:03 [PATCH net-next v1 0/5] ptp_ocp: set of small cleanups Andy Shevchenko
@ 2022-06-08 12:03 ` Andy Shevchenko
  2022-06-08 21:37   ` Vadim Fedorenko
                     ` (2 more replies)
  2022-06-08 12:03 ` [PATCH v1 net-next 2/5] ptp_ocp: use bits.h macros for all masks Andy Shevchenko
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-08 12:03 UTC (permalink / raw)
  To: Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran, Andy Shevchenko

Simplify the error path in ->probe() and unify message format a bit
by using dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ptp/ptp_ocp.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 4519ef42b458..17930762fde9 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -3722,14 +3722,12 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	int err;
 
 	devlink = devlink_alloc(&ptp_ocp_devlink_ops, sizeof(*bp), &pdev->dev);
-	if (!devlink) {
-		dev_err(&pdev->dev, "devlink_alloc failed\n");
-		return -ENOMEM;
-	}
+	if (!devlink)
+		return dev_err_probe(&pdev->dev, -ENOMEM, "devlink_alloc failed\n");
 
 	err = pci_enable_device(pdev);
 	if (err) {
-		dev_err(&pdev->dev, "pci_enable_device\n");
+		dev_err_probe(&pdev->dev, err, "pci_enable_device\n");
 		goto out_free;
 	}
 
@@ -3745,7 +3743,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	 */
 	err = pci_alloc_irq_vectors(pdev, 1, 17, PCI_IRQ_MSI | PCI_IRQ_MSIX);
 	if (err < 0) {
-		dev_err(&pdev->dev, "alloc_irq_vectors err: %d\n", err);
+		dev_err_probe(&pdev->dev, err, "alloc_irq_vectors\n");
 		goto out;
 	}
 	bp->n_irqs = err;
@@ -3757,8 +3755,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	bp->ptp = ptp_clock_register(&bp->ptp_info, &pdev->dev);
 	if (IS_ERR(bp->ptp)) {
-		err = PTR_ERR(bp->ptp);
-		dev_err(&pdev->dev, "ptp_clock_register: %d\n", err);
+		err = dev_err_probe(&pdev->dev, PTR_ERR(bp->ptp), "ptp_clock_register\n");
 		bp->ptp = NULL;
 		goto out;
 	}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 net-next 2/5] ptp_ocp: use bits.h macros for all masks
  2022-06-08 12:03 [PATCH net-next v1 0/5] ptp_ocp: set of small cleanups Andy Shevchenko
  2022-06-08 12:03 ` [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe() Andy Shevchenko
@ 2022-06-08 12:03 ` Andy Shevchenko
  2022-06-08 21:41   ` Vadim Fedorenko
  2022-06-08 12:03 ` [PATCH v1 net-next 3/5] ptp_ocp: drop duplicate NULL check in ptp_ocp_detach() Andy Shevchenko
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-08 12:03 UTC (permalink / raw)
  To: Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran, Andy Shevchenko

Currently we are using BIT(), but GENMASK(). Make use of the latter one
as well (far less error-prone, far more concise).

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ptp/ptp_ocp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 17930762fde9..926add7be9a2 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright (c) 2020 Facebook */
 
+#include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -88,10 +89,10 @@ struct tod_reg {
 #define TOD_CTRL_DISABLE_FMT_A	BIT(17)
 #define TOD_CTRL_DISABLE_FMT_B	BIT(16)
 #define TOD_CTRL_ENABLE		BIT(0)
-#define TOD_CTRL_GNSS_MASK	((1U << 4) - 1)
+#define TOD_CTRL_GNSS_MASK	GENMASK(3, 0)
 #define TOD_CTRL_GNSS_SHIFT	24
 
-#define TOD_STATUS_UTC_MASK		0xff
+#define TOD_STATUS_UTC_MASK		GENMASK(7, 0)
 #define TOD_STATUS_UTC_VALID		BIT(8)
 #define TOD_STATUS_LEAP_ANNOUNCE	BIT(12)
 #define TOD_STATUS_LEAP_VALID		BIT(16)
@@ -205,7 +206,7 @@ struct frequency_reg {
 #define FREQ_STATUS_VALID	BIT(31)
 #define FREQ_STATUS_ERROR	BIT(30)
 #define FREQ_STATUS_OVERRUN	BIT(29)
-#define FREQ_STATUS_MASK	(BIT(24) - 1)
+#define FREQ_STATUS_MASK	GENMASK(23, 0)
 
 struct ptp_ocp_flash_info {
 	const char *name;
@@ -674,9 +675,9 @@ static const struct ocp_selector ptp_ocp_clock[] = {
 	{ }
 };
 
+#define SMA_DISABLE		BIT(16)
 #define SMA_ENABLE		BIT(15)
-#define SMA_SELECT_MASK		((1U << 15) - 1)
-#define SMA_DISABLE		0x10000
+#define SMA_SELECT_MASK		GENMASK(14, 0)
 
 static const struct ocp_selector ptp_ocp_sma_in[] = {
 	{ .name = "10Mhz",	.value = 0x0000 },
@@ -3440,7 +3441,7 @@ ptp_ocp_tod_status_show(struct seq_file *s, void *data)
 
 	val = ioread32(&bp->tod->utc_status);
 	seq_printf(s, "UTC status register: 0x%08X\n", val);
-	seq_printf(s, "UTC offset: %d  valid:%d\n",
+	seq_printf(s, "UTC offset: %ld  valid:%d\n",
 		val & TOD_STATUS_UTC_MASK, val & TOD_STATUS_UTC_VALID ? 1 : 0);
 	seq_printf(s, "Leap second info valid:%d, Leap second announce %d\n",
 		val & TOD_STATUS_LEAP_VALID ? 1 : 0,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 net-next 3/5] ptp_ocp: drop duplicate NULL check in ptp_ocp_detach()
  2022-06-08 12:03 [PATCH net-next v1 0/5] ptp_ocp: set of small cleanups Andy Shevchenko
  2022-06-08 12:03 ` [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe() Andy Shevchenko
  2022-06-08 12:03 ` [PATCH v1 net-next 2/5] ptp_ocp: use bits.h macros for all masks Andy Shevchenko
@ 2022-06-08 12:03 ` Andy Shevchenko
  2022-06-08 21:42   ` Vadim Fedorenko
  2022-06-08 12:03 ` [PATCH v1 net-next 4/5] ptp_ocp: do not call pci_set_drvdata(pdev, NULL) Andy Shevchenko
  2022-06-08 12:03 ` [PATCH v1 net-next 5/5] ptp_ocp: replace kzalloc(x*y) by kcalloc(y, x) Andy Shevchenko
  4 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-08 12:03 UTC (permalink / raw)
  To: Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran, Andy Shevchenko

Since platform_device_unregister() is NULL-aware, we don't need to duplicate
this check. Remove it and fold the rest of the code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ptp/ptp_ocp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 926add7be9a2..4e237f806085 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -3701,10 +3701,8 @@ ptp_ocp_detach(struct ptp_ocp *bp)
 		serial8250_unregister_port(bp->mac_port);
 	if (bp->nmea_port != -1)
 		serial8250_unregister_port(bp->nmea_port);
-	if (bp->spi_flash)
-		platform_device_unregister(bp->spi_flash);
-	if (bp->i2c_ctrl)
-		platform_device_unregister(bp->i2c_ctrl);
+	platform_device_unregister(bp->spi_flash);
+	platform_device_unregister(bp->i2c_ctrl);
 	if (bp->i2c_clk)
 		clk_hw_unregister_fixed_rate(bp->i2c_clk);
 	if (bp->n_irqs)
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 net-next 4/5] ptp_ocp: do not call pci_set_drvdata(pdev, NULL)
  2022-06-08 12:03 [PATCH net-next v1 0/5] ptp_ocp: set of small cleanups Andy Shevchenko
                   ` (2 preceding siblings ...)
  2022-06-08 12:03 ` [PATCH v1 net-next 3/5] ptp_ocp: drop duplicate NULL check in ptp_ocp_detach() Andy Shevchenko
@ 2022-06-08 12:03 ` Andy Shevchenko
  2022-06-08 21:47   ` Vadim Fedorenko
  2022-06-08 12:03 ` [PATCH v1 net-next 5/5] ptp_ocp: replace kzalloc(x*y) by kcalloc(y, x) Andy Shevchenko
  4 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-08 12:03 UTC (permalink / raw)
  To: Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran, Andy Shevchenko

Cleaning up driver data is actually already handled by driver core,
so there is no need to do it manually.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ptp/ptp_ocp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 4e237f806085..857e35c68a04 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -3769,7 +3769,6 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 out:
 	ptp_ocp_detach(bp);
-	pci_set_drvdata(pdev, NULL);
 out_disable:
 	pci_disable_device(pdev);
 out_free:
@@ -3785,7 +3784,6 @@ ptp_ocp_remove(struct pci_dev *pdev)
 
 	devlink_unregister(devlink);
 	ptp_ocp_detach(bp);
-	pci_set_drvdata(pdev, NULL);
 	pci_disable_device(pdev);
 
 	devlink_free(devlink);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v1 net-next 5/5] ptp_ocp: replace kzalloc(x*y) by kcalloc(y, x)
  2022-06-08 12:03 [PATCH net-next v1 0/5] ptp_ocp: set of small cleanups Andy Shevchenko
                   ` (3 preceding siblings ...)
  2022-06-08 12:03 ` [PATCH v1 net-next 4/5] ptp_ocp: do not call pci_set_drvdata(pdev, NULL) Andy Shevchenko
@ 2022-06-08 12:03 ` Andy Shevchenko
  2022-06-08 21:48   ` Vadim Fedorenko
  4 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-08 12:03 UTC (permalink / raw)
  To: Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran, Andy Shevchenko

While here it may be no difference, the kcalloc() has some checks
against overflow and it's logically correct to call it for an array.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/ptp/ptp_ocp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 857e35c68a04..83da36e69361 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -2155,7 +2155,7 @@ ptp_ocp_fb_set_pins(struct ptp_ocp *bp)
 	struct ptp_pin_desc *config;
 	int i;
 
-	config = kzalloc(sizeof(*config) * 4, GFP_KERNEL);
+	config = kcalloc(4, sizeof(*config), GFP_KERNEL);
 	if (!config)
 		return -ENOMEM;
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe()
  2022-06-08 12:03 ` [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe() Andy Shevchenko
@ 2022-06-08 21:37   ` Vadim Fedorenko
  2022-06-08 22:20   ` Jonathan Lemon
  2022-06-10  5:45   ` Jakub Kicinski
  2 siblings, 0 replies; 18+ messages in thread
From: Vadim Fedorenko @ 2022-06-08 21:37 UTC (permalink / raw)
  To: Andy Shevchenko, Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran

On 08.06.2022 13:03, Andy Shevchenko wrote:
> Simplify the error path in ->probe() and unify message format a bit
> by using dev_err_probe().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

LGTM

Acked-by: Vadim Fedorenko <vfedorenko@novek.ru>
> ---
>   drivers/ptp/ptp_ocp.c | 13 +++++--------
>   1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 4519ef42b458..17930762fde9 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -3722,14 +3722,12 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	int err;
>   
>   	devlink = devlink_alloc(&ptp_ocp_devlink_ops, sizeof(*bp), &pdev->dev);
> -	if (!devlink) {
> -		dev_err(&pdev->dev, "devlink_alloc failed\n");
> -		return -ENOMEM;
> -	}
> +	if (!devlink)
> +		return dev_err_probe(&pdev->dev, -ENOMEM, "devlink_alloc failed\n");
>   
>   	err = pci_enable_device(pdev);
>   	if (err) {
> -		dev_err(&pdev->dev, "pci_enable_device\n");
> +		dev_err_probe(&pdev->dev, err, "pci_enable_device\n");
>   		goto out_free;
>   	}
>   
> @@ -3745,7 +3743,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   	 */
>   	err = pci_alloc_irq_vectors(pdev, 1, 17, PCI_IRQ_MSI | PCI_IRQ_MSIX);
>   	if (err < 0) {
> -		dev_err(&pdev->dev, "alloc_irq_vectors err: %d\n", err);
> +		dev_err_probe(&pdev->dev, err, "alloc_irq_vectors\n");
>   		goto out;
>   	}
>   	bp->n_irqs = err;
> @@ -3757,8 +3755,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   
>   	bp->ptp = ptp_clock_register(&bp->ptp_info, &pdev->dev);
>   	if (IS_ERR(bp->ptp)) {
> -		err = PTR_ERR(bp->ptp);
> -		dev_err(&pdev->dev, "ptp_clock_register: %d\n", err);
> +		err = dev_err_probe(&pdev->dev, PTR_ERR(bp->ptp), "ptp_clock_register\n");
>   		bp->ptp = NULL;
>   		goto out;
>   	}


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 net-next 2/5] ptp_ocp: use bits.h macros for all masks
  2022-06-08 12:03 ` [PATCH v1 net-next 2/5] ptp_ocp: use bits.h macros for all masks Andy Shevchenko
@ 2022-06-08 21:41   ` Vadim Fedorenko
  0 siblings, 0 replies; 18+ messages in thread
From: Vadim Fedorenko @ 2022-06-08 21:41 UTC (permalink / raw)
  To: Andy Shevchenko, Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran

On 08.06.2022 13:03, Andy Shevchenko wrote:
> Currently we are using BIT(), but GENMASK(). Make use of the latter one
> as well (far less error-prone, far more concise).
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

LGTM

Acked-by: Vadim Fedorenko <vfedorenko@novek.ru>
> ---
>   drivers/ptp/ptp_ocp.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 17930762fde9..926add7be9a2 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /* Copyright (c) 2020 Facebook */
>   
> +#include <linux/bits.h>
>   #include <linux/err.h>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
> @@ -88,10 +89,10 @@ struct tod_reg {
>   #define TOD_CTRL_DISABLE_FMT_A	BIT(17)
>   #define TOD_CTRL_DISABLE_FMT_B	BIT(16)
>   #define TOD_CTRL_ENABLE		BIT(0)
> -#define TOD_CTRL_GNSS_MASK	((1U << 4) - 1)
> +#define TOD_CTRL_GNSS_MASK	GENMASK(3, 0)
>   #define TOD_CTRL_GNSS_SHIFT	24
>   
> -#define TOD_STATUS_UTC_MASK		0xff
> +#define TOD_STATUS_UTC_MASK		GENMASK(7, 0)
>   #define TOD_STATUS_UTC_VALID		BIT(8)
>   #define TOD_STATUS_LEAP_ANNOUNCE	BIT(12)
>   #define TOD_STATUS_LEAP_VALID		BIT(16)
> @@ -205,7 +206,7 @@ struct frequency_reg {
>   #define FREQ_STATUS_VALID	BIT(31)
>   #define FREQ_STATUS_ERROR	BIT(30)
>   #define FREQ_STATUS_OVERRUN	BIT(29)
> -#define FREQ_STATUS_MASK	(BIT(24) - 1)
> +#define FREQ_STATUS_MASK	GENMASK(23, 0)
>   
>   struct ptp_ocp_flash_info {
>   	const char *name;
> @@ -674,9 +675,9 @@ static const struct ocp_selector ptp_ocp_clock[] = {
>   	{ }
>   };
>   
> +#define SMA_DISABLE		BIT(16)
>   #define SMA_ENABLE		BIT(15)
> -#define SMA_SELECT_MASK		((1U << 15) - 1)
> -#define SMA_DISABLE		0x10000
> +#define SMA_SELECT_MASK		GENMASK(14, 0)
>   
>   static const struct ocp_selector ptp_ocp_sma_in[] = {
>   	{ .name = "10Mhz",	.value = 0x0000 },
> @@ -3440,7 +3441,7 @@ ptp_ocp_tod_status_show(struct seq_file *s, void *data)
>   
>   	val = ioread32(&bp->tod->utc_status);
>   	seq_printf(s, "UTC status register: 0x%08X\n", val);
> -	seq_printf(s, "UTC offset: %d  valid:%d\n",
> +	seq_printf(s, "UTC offset: %ld  valid:%d\n",
>   		val & TOD_STATUS_UTC_MASK, val & TOD_STATUS_UTC_VALID ? 1 : 0);
>   	seq_printf(s, "Leap second info valid:%d, Leap second announce %d\n",
>   		val & TOD_STATUS_LEAP_VALID ? 1 : 0,


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 net-next 3/5] ptp_ocp: drop duplicate NULL check in ptp_ocp_detach()
  2022-06-08 12:03 ` [PATCH v1 net-next 3/5] ptp_ocp: drop duplicate NULL check in ptp_ocp_detach() Andy Shevchenko
@ 2022-06-08 21:42   ` Vadim Fedorenko
  0 siblings, 0 replies; 18+ messages in thread
From: Vadim Fedorenko @ 2022-06-08 21:42 UTC (permalink / raw)
  To: Andy Shevchenko, Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran

On 08.06.2022 13:03, Andy Shevchenko wrote:
> Since platform_device_unregister() is NULL-aware, we don't need to duplicate
> this check. Remove it and fold the rest of the code.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Vadim Fedorenko <vfedorenko@novek.ru>
> ---
>   drivers/ptp/ptp_ocp.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 926add7be9a2..4e237f806085 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -3701,10 +3701,8 @@ ptp_ocp_detach(struct ptp_ocp *bp)
>   		serial8250_unregister_port(bp->mac_port);
>   	if (bp->nmea_port != -1)
>   		serial8250_unregister_port(bp->nmea_port);
> -	if (bp->spi_flash)
> -		platform_device_unregister(bp->spi_flash);
> -	if (bp->i2c_ctrl)
> -		platform_device_unregister(bp->i2c_ctrl);
> +	platform_device_unregister(bp->spi_flash);
> +	platform_device_unregister(bp->i2c_ctrl);
>   	if (bp->i2c_clk)
>   		clk_hw_unregister_fixed_rate(bp->i2c_clk);
>   	if (bp->n_irqs)


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 net-next 4/5] ptp_ocp: do not call pci_set_drvdata(pdev, NULL)
  2022-06-08 12:03 ` [PATCH v1 net-next 4/5] ptp_ocp: do not call pci_set_drvdata(pdev, NULL) Andy Shevchenko
@ 2022-06-08 21:47   ` Vadim Fedorenko
  2022-06-09 10:43     ` Andy Shevchenko
  0 siblings, 1 reply; 18+ messages in thread
From: Vadim Fedorenko @ 2022-06-08 21:47 UTC (permalink / raw)
  To: Andy Shevchenko, Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran

On 08.06.2022 13:03, Andy Shevchenko wrote:
> Cleaning up driver data is actually already handled by driver core,
> so there is no need to do it manually.

I found a couple of places with exactly the same code in error path.
For example Marvell's OcteonX drivers in crypto and net subsystems.
Should we fix them too?

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Overall looks good.

Acked-by: Vadim Fedorenko <vfedorenko@novek.ru>

> ---
>   drivers/ptp/ptp_ocp.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 4e237f806085..857e35c68a04 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -3769,7 +3769,6 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   
>   out:
>   	ptp_ocp_detach(bp);
> -	pci_set_drvdata(pdev, NULL);
>   out_disable:
>   	pci_disable_device(pdev);
>   out_free:
> @@ -3785,7 +3784,6 @@ ptp_ocp_remove(struct pci_dev *pdev)
>   
>   	devlink_unregister(devlink);
>   	ptp_ocp_detach(bp);
> -	pci_set_drvdata(pdev, NULL);
>   	pci_disable_device(pdev);
>   
>   	devlink_free(devlink);


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 net-next 5/5] ptp_ocp: replace kzalloc(x*y) by kcalloc(y, x)
  2022-06-08 12:03 ` [PATCH v1 net-next 5/5] ptp_ocp: replace kzalloc(x*y) by kcalloc(y, x) Andy Shevchenko
@ 2022-06-08 21:48   ` Vadim Fedorenko
  0 siblings, 0 replies; 18+ messages in thread
From: Vadim Fedorenko @ 2022-06-08 21:48 UTC (permalink / raw)
  To: Andy Shevchenko, Jonathan Lemon, netdev, linux-kernel; +Cc: Richard Cochran

On 08.06.2022 13:03, Andy Shevchenko wrote:
> While here it may be no difference, the kcalloc() has some checks
> against overflow and it's logically correct to call it for an array.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Vadim Fedorenko <vfedorenko@novek.ru>

> ---
>   drivers/ptp/ptp_ocp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 857e35c68a04..83da36e69361 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -2155,7 +2155,7 @@ ptp_ocp_fb_set_pins(struct ptp_ocp *bp)
>   	struct ptp_pin_desc *config;
>   	int i;
>   
> -	config = kzalloc(sizeof(*config) * 4, GFP_KERNEL);
> +	config = kcalloc(4, sizeof(*config), GFP_KERNEL);
>   	if (!config)
>   		return -ENOMEM;
>   


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe()
  2022-06-08 12:03 ` [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe() Andy Shevchenko
  2022-06-08 21:37   ` Vadim Fedorenko
@ 2022-06-08 22:20   ` Jonathan Lemon
  2022-06-10  5:45   ` Jakub Kicinski
  2 siblings, 0 replies; 18+ messages in thread
From: Jonathan Lemon @ 2022-06-08 22:20 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: netdev, linux-kernel, Richard Cochran

On Wed, Jun 08, 2022 at 03:03:54PM +0300, Andy Shevchenko wrote:
> Simplify the error path in ->probe() and unify message format a bit
> by using dev_err_probe().

Line length.
-- 
Jonathan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 net-next 4/5] ptp_ocp: do not call pci_set_drvdata(pdev, NULL)
  2022-06-08 21:47   ` Vadim Fedorenko
@ 2022-06-09 10:43     ` Andy Shevchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-09 10:43 UTC (permalink / raw)
  To: Vadim Fedorenko
  Cc: Andy Shevchenko, Jonathan Lemon, netdev,
	Linux Kernel Mailing List, Richard Cochran

On Thu, Jun 9, 2022 at 12:01 AM Vadim Fedorenko <vfedorenko@novek.ru> wrote:
>
> On 08.06.2022 13:03, Andy Shevchenko wrote:
> > Cleaning up driver data is actually already handled by driver core,
> > so there is no need to do it manually.
>
> I found a couple of places with exactly the same code in error path.
> For example Marvell's OcteonX drivers in crypto and net subsystems.
> Should we fix them too?

I believe there are even more, but feel free to fix them, they are not my POI.

...

> Overall looks good.
>
> Acked-by: Vadim Fedorenko <vfedorenko@novek.ru>

Thanks!

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe()
  2022-06-08 12:03 ` [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe() Andy Shevchenko
  2022-06-08 21:37   ` Vadim Fedorenko
  2022-06-08 22:20   ` Jonathan Lemon
@ 2022-06-10  5:45   ` Jakub Kicinski
  2022-06-10 11:09     ` Andy Shevchenko
  2 siblings, 1 reply; 18+ messages in thread
From: Jakub Kicinski @ 2022-06-10  5:45 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Jonathan Lemon, netdev, linux-kernel, Richard Cochran

On Wed,  8 Jun 2022 15:03:54 +0300 Andy Shevchenko wrote:
> Simplify the error path in ->probe() and unify message format a bit
> by using dev_err_probe().

Let's not do this. I get that using dev_err_probe() even without
possibility of -EPROBE_DEFER is acceptable, but converting
existing drivers is too much IMO. Acceptable < best practice.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe()
  2022-06-10  5:45   ` Jakub Kicinski
@ 2022-06-10 11:09     ` Andy Shevchenko
  2022-06-10 15:39       ` Jakub Kicinski
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-10 11:09 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Jonathan Lemon, netdev, linux-kernel, Richard Cochran

On Thu, Jun 09, 2022 at 10:45:23PM -0700, Jakub Kicinski wrote:
> On Wed,  8 Jun 2022 15:03:54 +0300 Andy Shevchenko wrote:
> > Simplify the error path in ->probe() and unify message format a bit
> > by using dev_err_probe().
> 
> Let's not do this. I get that using dev_err_probe() even without
> possibility of -EPROBE_DEFER is acceptable, but converting
> existing drivers is too much IMO. Acceptable < best practice.

Noted.

I have just checked that if you drop this patch the rest will be still
applicable. If you have no objections, can you apply patches 2-5 then?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe()
  2022-06-10 11:09     ` Andy Shevchenko
@ 2022-06-10 15:39       ` Jakub Kicinski
  2022-06-10 15:46         ` Jonathan Lemon
  2022-06-10 16:04         ` Andy Shevchenko
  0 siblings, 2 replies; 18+ messages in thread
From: Jakub Kicinski @ 2022-06-10 15:39 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Jonathan Lemon, netdev, linux-kernel, Richard Cochran

On Fri, 10 Jun 2022 14:09:24 +0300 Andy Shevchenko wrote:
> I have just checked that if you drop this patch the rest will be still
> applicable. If you have no objections, can you apply patches 2-5 then?

It's tradition in netdev to ask people to repost. But looks completely
safe for me to drop patch 1, so applied 2-5. Don't tell anyone I did this.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe()
  2022-06-10 15:39       ` Jakub Kicinski
@ 2022-06-10 15:46         ` Jonathan Lemon
  2022-06-10 16:04         ` Andy Shevchenko
  1 sibling, 0 replies; 18+ messages in thread
From: Jonathan Lemon @ 2022-06-10 15:46 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Andy Shevchenko, netdev, linux-kernel, Richard Cochran

On Fri, Jun 10, 2022 at 08:39:18AM -0700, Jakub Kicinski wrote:
> On Fri, 10 Jun 2022 14:09:24 +0300 Andy Shevchenko wrote:
> > I have just checked that if you drop this patch the rest will be still
> > applicable. If you have no objections, can you apply patches 2-5 then?
> 
> It's tradition in netdev to ask people to repost. But looks completely
> safe for me to drop patch 1, so applied 2-5. Don't tell anyone I did this.

I see what you did there.  :)
-- 
Jonathan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe()
  2022-06-10 15:39       ` Jakub Kicinski
  2022-06-10 15:46         ` Jonathan Lemon
@ 2022-06-10 16:04         ` Andy Shevchenko
  1 sibling, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2022-06-10 16:04 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andy Shevchenko, Jonathan Lemon, netdev,
	Linux Kernel Mailing List, Richard Cochran

On Fri, Jun 10, 2022 at 5:43 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 10 Jun 2022 14:09:24 +0300 Andy Shevchenko wrote:
> > I have just checked that if you drop this patch the rest will be still
> > applicable. If you have no objections, can you apply patches 2-5 then?
>
> It's tradition in netdev to ask people to repost. But looks completely
> safe for me to drop patch 1, so applied 2-5.

Thanks!

> Don't tell anyone I did this.

Tschhh!


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-06-10 16:05 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-08 12:03 [PATCH net-next v1 0/5] ptp_ocp: set of small cleanups Andy Shevchenko
2022-06-08 12:03 ` [PATCH net-next v1 1/5] ptp_ocp: use dev_err_probe() Andy Shevchenko
2022-06-08 21:37   ` Vadim Fedorenko
2022-06-08 22:20   ` Jonathan Lemon
2022-06-10  5:45   ` Jakub Kicinski
2022-06-10 11:09     ` Andy Shevchenko
2022-06-10 15:39       ` Jakub Kicinski
2022-06-10 15:46         ` Jonathan Lemon
2022-06-10 16:04         ` Andy Shevchenko
2022-06-08 12:03 ` [PATCH v1 net-next 2/5] ptp_ocp: use bits.h macros for all masks Andy Shevchenko
2022-06-08 21:41   ` Vadim Fedorenko
2022-06-08 12:03 ` [PATCH v1 net-next 3/5] ptp_ocp: drop duplicate NULL check in ptp_ocp_detach() Andy Shevchenko
2022-06-08 21:42   ` Vadim Fedorenko
2022-06-08 12:03 ` [PATCH v1 net-next 4/5] ptp_ocp: do not call pci_set_drvdata(pdev, NULL) Andy Shevchenko
2022-06-08 21:47   ` Vadim Fedorenko
2022-06-09 10:43     ` Andy Shevchenko
2022-06-08 12:03 ` [PATCH v1 net-next 5/5] ptp_ocp: replace kzalloc(x*y) by kcalloc(y, x) Andy Shevchenko
2022-06-08 21:48   ` Vadim Fedorenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).