Re: [PATCH v2 net-next 2/7] ptp: netc: remove unnecessary pcie_flr() call in probe

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Disabling a PEROUT channel that is already disabled incorrectly frees alarm ID 0.
--

--- Patch [2]: [PATCH v2 net-next 2/7] ptp: netc: remove unnecessary pcie_flr() call in probe ---
commit e70feff8e69f873b89699e822c09a88a2517534c
Author: Wei Fang <[email protected]>

ptp: netc: remove unnecessary pcie_flr() call in probe
    
According to the NETC reference manual, function level reset is not
applicable to the timer as a supporting function. Remove the pcie_flr()
call from netc_timer_pci_probe() as it has no effect.

> diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> index 0e0972596d880..d33c49c86ac44 100644
> --- a/drivers/ptp/ptp_netc.c
> +++ b/drivers/ptp/ptp_netc.c
> @@ -774,7 +774,6 @@ static int netc_timer_pci_probe(struct pci_dev *pdev)
>  	if (!priv)
>  		return -ENOMEM;
>  
> -	pcie_flr(pdev);
>  	err = pci_enable_device_mem(pdev);
>  	if (err)
>  		return dev_err_probe(dev, err, "Failed to enable device\n");

[Severity: High]
This is a pre-existing issue, but does disabling a PEROUT channel that is
already disabled incorrectly free alarm ID 0?

drivers/ptp/ptp_netc.c:net_timer_enable_perout() {
...
	} else {
		netc_timer_disable_periodic_pulse(priv, channel);
		priv->fs_alarm_bitmap &= ~BIT(pp->alarm_id);
		memset(pp, 0, sizeof(*pp));
	}
...
}

If a userspace process calls ioctl(PTP_PEROUT_REQUEST) with a channel index but
period = 0 (which implies on = 0), and the channel is already disabled, the code
proceeds to this else block without verifying its state.

Since the channel's struct netc_pp was previously zeroed, pp->alarm_id evaluates
to 0. The driver then executes priv->fs_alarm_bitmap &= ~BIT(0), silently
revoking the allocation of alarm 0 from whichever channel is actively using it.

Could this cause two channels to conflict over the same hardware alarm
configuration and corrupt their output signals?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2