[PATCH net-next 0/3] ptp: ocp: minor updates

[PATCH net-next 0/3] ptp: ocp: minor updates

[Jonathan Lemon]

Fix errors spotted by automated tools.

Add myself to the MAINTAINERS for the ptp_ocp driver.

Jonathan Lemon (3):
  ptp: ocp: Fix uninitialized variable warning spotted by clang.
  ptp: ocp: Fix error path for pci_ocp_device_init()
  MAINTAINERS: Update for ptp_ocp driver.

 MAINTAINERS           |  6 ++++++
 drivers/ptp/ptp_ocp.c | 11 +++++------
 2 files changed, 11 insertions(+), 6 deletions(-)

-- 
2.31.1

[PATCH net-next 1/3] ptp: ocp: Fix uninitialized variable warning spotted by clang.

[Jonathan Lemon]

If attempting to flash the firmware with a blob of size 0,
the entire write loop is skipped and the uninitialized err
is returned.  Fix by setting to 0 first.

Also remove a now-unused error handling statement.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/ptp/ptp_ocp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 92edf772feed..9b2ba06ebf97 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -763,7 +763,7 @@ ptp_ocp_devlink_flash(struct devlink *devlink, struct device *dev,
 	size_t off, len, resid, wrote;
 	struct erase_info erase;
 	size_t base, blksz;
-	int err;
+	int err = 0;
 
 	off = 0;
 	base = bp->flash_start;
@@ -847,8 +847,6 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
 							       "loader",
 							       buf);
 		}
-		if (err)
-			return err;
 	}
 
 	if (!bp->has_serial)
-- 
2.31.1

Re: [PATCH net-next 1/3] ptp: ocp: Fix uninitialized variable warning spotted by clang.

[Jakub Kicinski]

On Wed, 11 Aug 2021 11:31:31 -0700 Jonathan Lemon wrote:
> If attempting to flash the firmware with a blob of size 0,
> the entire write loop is skipped and the uninitialized err
> is returned.  Fix by setting to 0 first.
> 
> Also remove a now-unused error handling statement.
> 
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> ---
>  drivers/ptp/ptp_ocp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index 92edf772feed..9b2ba06ebf97 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -763,7 +763,7 @@ ptp_ocp_devlink_flash(struct devlink *devlink, struct device *dev,
>  	size_t off, len, resid, wrote;
>  	struct erase_info erase;
>  	size_t base, blksz;
> -	int err;
> +	int err = 0;
>  
>  	off = 0;
>  	base = bp->flash_start;
> @@ -847,8 +847,6 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
>  							       "loader",
>  							       buf);
>  		}
> -		if (err)
> -			return err;

Looks like an accidental change, but it's mentioned in the commit log?

>  	}
>  
>  	if (!bp->has_serial)

Re: [PATCH net-next 1/3] ptp: ocp: Fix uninitialized variable warning spotted by clang.

[Jonathan Lemon]

> On Aug 12, 2021, at 2:58 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> On Wed, 11 Aug 2021 11:31:31 -0700 Jonathan Lemon wrote:
>> If attempting to flash the firmware with a blob of size 0,
>> the entire write loop is skipped and the uninitialized err
>> is returned.  Fix by setting to 0 first.
>> 
>> Also remove a now-unused error handling statement.
>> 
>> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
>> ---
>> drivers/ptp/ptp_ocp.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
>> index 92edf772feed..9b2ba06ebf97 100644
>> --- a/drivers/ptp/ptp_ocp.c
>> +++ b/drivers/ptp/ptp_ocp.c
>> @@ -763,7 +763,7 @@ ptp_ocp_devlink_flash(struct devlink *devlink, struct device *dev,
>>    size_t off, len, resid, wrote;
>>    struct erase_info erase;
>>    size_t base, blksz;
>> -    int err;
>> +    int err = 0;
>> 
>>    off = 0;
>>    base = bp->flash_start;
>> @@ -847,8 +847,6 @@ ptp_ocp_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
>>                                   "loader",
>>                                   buf);
>>        }
>> -        if (err)
>> -            return err;
> 
> Looks like an accidental change, but it's mentioned in the commit log?

Yes, intentional, I spotted this NOP when fixing the compiler warning. 
— 
Jonathan 

[PATCH net-next 2/3] ptp: ocp: Fix error path for pci_ocp_device_init()

[Jonathan Lemon]

If ptp_ocp_device_init() fails, pci_disable_device() is skipped.
Fix the error handling so this case is covered.  Update ptp_ocp_remove()
so the normal exit path is identical.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/ptp/ptp_ocp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 9b2ba06ebf97..a8d390314897 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -1436,7 +1436,7 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	bp = devlink_priv(devlink);
 	err = ptp_ocp_device_init(bp, pdev);
 	if (err)
-		goto out_unregister;
+		goto out_disable;
 
 	/* compat mode.
 	 * Older FPGA firmware only returns 2 irq's.
@@ -1474,8 +1474,9 @@ ptp_ocp_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 out:
 	ptp_ocp_detach(bp);
-	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
+out_disable:
+	pci_disable_device(pdev);
 out_unregister:
 	devlink_unregister(devlink);
 out_free:
@@ -1491,8 +1492,8 @@ ptp_ocp_remove(struct pci_dev *pdev)
 	struct devlink *devlink = priv_to_devlink(bp);
 
 	ptp_ocp_detach(bp);
-	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
+	pci_disable_device(pdev);
 
 	devlink_unregister(devlink);
 	devlink_free(devlink);
-- 
2.31.1

Re: [PATCH net-next 2/3] ptp: ocp: Fix error path for pci_ocp_device_init()

[Jakub Kicinski]

On Wed, 11 Aug 2021 11:31:32 -0700 Jonathan Lemon wrote:
> If ptp_ocp_device_init() fails, pci_disable_device() is skipped.
> Fix the error handling so this case is covered.  Update ptp_ocp_remove()
> so the normal exit path is identical.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>

Fixes tag would be useful on this and previous patch to make it clear
the problems are only present in net-next.

[PATCH net-next 3/3] MAINTAINERS: Update for ptp_ocp driver.

[Jonathan Lemon]

Add maintainer info for the OpenCompute PTP driver.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 41fcfdb24a81..01632d3ca1f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13890,6 +13890,12 @@ F:	Documentation/devicetree/
 F:	arch/*/boot/dts/
 F:	include/dt-bindings/
 
+OPENCOMPUTE PTP CLOCK DRIVER
+M:	Jonathan Lemon <jonathan.lemon@gmail.com>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/ptp/ptp_ocp.c
+
 OPENCORES I2C BUS DRIVER
 M:	Peter Korsgaard <peter@korsgaard.com>
 M:	Andrew Lunn <andrew@lunn.ch>
-- 
2.31.1