On 28.10.2019 18:25, Egor Pomozov wrote:
> Hello Dan,
> +Igor
>
> Thank you for pointing the issue. We’ll fixed soon.
>> 1207 clock = ptp_clock_register(&aq_ptp->ptp_info, &aq_nic->ndev->dev);
>> 1208 if (!clock || IS_ERR(clock)) {
>> 1209 netdev_err(aq_nic->ndev, "ptp_clock_register failed\n");
>> 1210 err = PTR_ERR(clock);
>> 1211 goto err_exit;
>>
>> This is a false positive in Smatch but the code is still problematic.
>>
>> The issue is that ptp_clock_register() returns error pointers if there
>> is an error and it returns NULL if the clock is disabled in the config.
>> If "clock" is NULL then this code returns PTR_ERR(NULL) which is
>> success but so that's a bug.
>>
>> The question is, is it really realistic for people to run this hardware
>> without a ptp clock? If so then we should allow it instead of erroring
>> out here when clock is NULL. If not then we should enforce that in the
>> Kconfig instead of waiting until runtime.
Hi Dan, I'd say thats a false positive.
There exist HW configuration where PTP is disabled or not available.
PTR_ERR(NULL) is 0, so eventually driver now functions correctly, allowing to
proceed but marking ptp functionality as disabled.
I agree that's a bit counterintuitive but correct.
Regards,
Igor
