Re: [PATCH net-next v2] ice: use dev_err_probe() in ice_probe()

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 7/2/26 09:05, weirongguang wrote:
> 
> 
> 在 2026/7/1 22:37, Maciej Fijalkowski 写道:
>> On Wed, Jul 01, 2026 at 09:36:18AM +0800, Rongguang Wei wrote:
>>> From: Rongguang Wei <weirongguang@kylinos.cn>
>>>
>>> dev_err_probe() logs the error and returns the supplied error code, which
>>> allows probe error paths to be written more compactly.
>>>
>>> Use dev_err_probe() in ice_probe() for error paths that currently print an
>>> error message and immediately return the same error code. This keeps the
>>> existing error handling semantics while reducing open-coded logging and
>>> return sequences.
>>>
>>> Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
>>> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>>> ---
>>> v2:
>>>    - Fix commit message per Aleksandr Loktionov's recommendation.
>>> v1: https://lore.kernel.org/netdev/20260630032537.42605-1-clementwei90@163.com/T/#t
>>> ---
>>>   drivers/net/ethernet/intel/ice/ice_main.c | 24 ++++++++---------------
>>>   1 file changed, 8 insertions(+), 16 deletions(-)
>>
>> Could we also address rest of sites within driver at this very same
>> commit?
>>
>> drivers/net/ethernet/intel/ice/ice_dcb_lib.c-873-       dev_err(dev, "DCB init failed\n");
>> drivers/net/ethernet/intel/ice/ice_dcb_lib.c:874:       return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_main.c-4482-         dev_warn(dev, "Failed to initialize hardware after applying Tx scheduling configuration.\n");
>> drivers/net/ethernet/intel/ice/ice_main.c:4483:         return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_main.c-4543-         dev_err(dev, "Fail during requesting FW: %d\n", err);
>> drivers/net/ethernet/intel/ice/ice_main.c:4544:         return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_main.c-4961-         dev_err(dev, "ice_init_pf failed: %d\n", err);
>> drivers/net/ethernet/intel/ice/ice_main.c:4962:         return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_main.c-5192-         dev_err(dev, "BAR0 I/O map error %d\n", err);
>> drivers/net/ethernet/intel/ice/ice_main.c:5193:         return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_main.c-5206-         dev_err(dev, "DMA configuration failed: 0x%x\n", err);
>> drivers/net/ethernet/intel/ice/ice_main.c:5207:         return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_main.c-5244-         dev_err(dev, "ice_init_hw failed: %d\n", err);
>> drivers/net/ethernet/intel/ice/ice_main.c:5245:         return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_main.c-9627-         netdev_err(netdev, "Failed to get link info, error %d\n", err);
>> drivers/net/ethernet/intel/ice/ice_main.c:9628:         return err;
>> --
>> drivers/net/ethernet/intel/ice/devlink/devlink.c-1244-          dev_err(dev, "ice_init_hw failed: %d\n", err);
>> drivers/net/ethernet/intel/ice/devlink/devlink.c:1245:          return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_ptp.c-1935-          dev_err(ice_pf_to_dev(pf), "PTP failed to set time %d\n", err);
>> drivers/net/ethernet/intel/ice/ice_ptp.c:1936:          return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_ptp.c-2000-          dev_err(dev, "PTP failed to adjust time, err %d\n", err);
>> drivers/net/ethernet/intel/ice/ice_ptp.c:2001:          return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_sriov.c-829-         dev_err(dev, "Failed to enable SR-IOV: %d\n", err);
>> drivers/net/ethernet/intel/ice/ice_sriov.c:830:         return err;
>> --
>> drivers/net/ethernet/intel/ice/ice_eswitch_br.c-314-            dev_info(dev, "Bridge port lookup failed (vsi=%u)\n", vsi_idx);
>> drivers/net/ethernet/intel/ice/ice_eswitch_br.c:315:            return ERR_PTR(-EINVAL);
>>
> Hi,
> Per commit a787e5400a1c("driver core: add device probe log helper"), dev_err_probe was
> originally designed for probe functions in device driver to handle -EPROBE_DEFER.
> 
> Using it elsewhere is not the common pattern in the kernel. I'm unsure whether this aligns
> with the intended usage if we also address the rest of the sites within the driver.

Maciej has pointed only the places that are of "probe time" scope, even
if not "directly in .probe()".

It would be nice to have a general handler that just logs and returns
the error, but it will be a rather "too much churn" type of conversion
anyway. But for new code it will be nice.