* [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).