Re: [PATCH net v2] net: ethernet: xscale: Check for PTP support properly

[Jacob Keller <jacob.e.keller@intel.com>]

On 2/11/2026 1:03 PM, Linus Walleij wrote:
> On Wed, Feb 11, 2026 at 4:43 PM Andrew Lunn <andrew@lunn.ch> wrote:
>> On Wed, Feb 11, 2026 at 01:58:21PM +0100, Linus Walleij wrote:
>>> In ixp4xx_get_ts_info() ixp46x_ptp_find() is called
>>> unconditionally despite this feature only existing on
>>> ixp46x, leading to the following splat from tcpdump:
>>>
>>> root@OpenWrt:~# tcpdump -vv -X -i eth0
>>> (...)
>>> Unable to handle kernel NULL pointer dereference at virtual address
>>>    00000238 when read
>>> (...)
>>> Call trace:
>>>   ptp_clock_index from ixp46x_ptp_find+0x1c/0x38
>>>   ixp46x_ptp_find from ixp4xx_get_ts_info+0x4c/0x64
>>>   ixp4xx_get_ts_info from __ethtool_get_ts_info+0x90/0x108
>>>   __ethtool_get_ts_info from __dev_ethtool+0xa00/0x2648
>>>   __dev_ethtool from dev_ethtool+0x160/0x234
>>>   dev_ethtool from dev_ioctl+0x2cc/0x460
>>>   dev_ioctl from sock_ioctl+0x1ec/0x524
>>>   sock_ioctl from sys_ioctl+0x51c/0xa94
>>>   sys_ioctl from ret_fast_syscall+0x0/0x44
>>>   (...)
>>> Segmentation fault
>>>
>>> Check for ixp46x in ixp46x_ptp_find() before trying to set up
>>> PTP to avoid this.
>>
>>>        ret = ixp46x_ptp_find(&port->timesync_regs, &port->phc_index);
>>>        if (ret)
>>> -             return ret;
>>> +             return -EOPNOTSUPP;
>>
>> Changing the error code is unusual. Maybe add to the commit message
>> why this is needed, or somebody might ask :-)
> 
> Tricky one, but if you look close changing the error code is what I
> try not to do! :D
> 
> Before this patch ixp4xx_hwtstamp_set() returned -EOPNOTSUPP:
> 
> -       if (!cpu_is_ixp46x())
> -               return -EOPNOTSUPP;
> 
> After moving the check into ixp46x_ptp_find() as requested in the
> review of v1 I have to return some error code from that new check,
> but it is now called from more places, so it returns
> 
> int ixp46x_ptp_find(struct ixp46x_ts_regs *__iomem *regs, int *phc_index)
>   {
> +       if (!cpu_is_ixp46x())
> +               return -ENODEV;
> 
> So this:
> 
>       ret = ixp46x_ptp_find(&port->timesync_regs, &port->phc_index);
>          if (ret)
> -               return ret;
> +               return -EOPNOTSUPP;
> 
> Is put into ixp4xx_hwtstamp_set() so it will return the same error
> code as before if PTP was not found.
> 
> (I can copyedit this reasoning into the commit message if you want.)
> 
> Yours,
> Linus Walleij
> 

That would be helpful to have in the commit message, since it is not 
immediately obvious.