* [PATCH net-next v2] ice: use dev_err_probe() in ice_probe() @ 2026-07-01 1:36 Rongguang Wei 2026-07-01 14:37 ` Maciej Fijalkowski 0 siblings, 1 reply; 4+ messages in thread From: Rongguang Wei @ 2026-07-01 1:36 UTC (permalink / raw) To: netdev, intel-wired-lan, aleksandr.loktionov, przemyslaw.kitszel Cc: anthony.l.nguyen, andrew+netdev, Rongguang Wei 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(-) diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index e2fd2dab03e3..31aa42f8e6d3 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -5161,10 +5161,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) struct ice_hw *hw; int err; - if (pdev->is_virtfn) { - dev_err(dev, "can't probe a virtual function\n"); - return -EINVAL; - } + if (pdev->is_virtfn) + return dev_err_probe(dev, -EINVAL, "can't probe a virtual function\n"); /* when under a kdump kernel initiate a reset before enabling the * device in order to clear out any pending DMA transactions. These @@ -5188,10 +5186,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) return err; err = pcim_iomap_regions(pdev, BIT(ICE_BAR0), dev_driver_string(dev)); - if (err) { - dev_err(dev, "BAR0 I/O map error %d\n", err); - return err; - } + if (err) + return dev_err_probe(dev, err, "BAR0 I/O map error %d\n", err); pf = ice_allocate_pf(dev); if (!pf) @@ -5202,10 +5198,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) /* set up for high or low DMA */ err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); - if (err) { - dev_err(dev, "DMA configuration failed: 0x%x\n", err); - return err; - } + if (err) + return dev_err_probe(dev, err, "DMA configuration failed: 0x%x\n", err); pci_set_master(pdev); pf->pdev = pdev; @@ -5240,10 +5234,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) return ice_probe_recovery_mode(pf); err = ice_init_hw(hw); - if (err) { - dev_err(dev, "ice_init_hw failed: %d\n", err); - return err; - } + if (err) + return dev_err_probe(dev, err, "ice_init_hw failed: %d\n", err); ice_init_dev_hw(pf); -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] ice: use dev_err_probe() in ice_probe() 2026-07-01 1:36 [PATCH net-next v2] ice: use dev_err_probe() in ice_probe() Rongguang Wei @ 2026-07-01 14:37 ` Maciej Fijalkowski 2026-07-02 7:05 ` weirongguang 0 siblings, 1 reply; 4+ messages in thread From: Maciej Fijalkowski @ 2026-07-01 14:37 UTC (permalink / raw) To: Rongguang Wei Cc: netdev, intel-wired-lan, aleksandr.loktionov, przemyslaw.kitszel, anthony.l.nguyen, andrew+netdev, Rongguang Wei 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); > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c > index e2fd2dab03e3..31aa42f8e6d3 100644 > --- a/drivers/net/ethernet/intel/ice/ice_main.c > +++ b/drivers/net/ethernet/intel/ice/ice_main.c > @@ -5161,10 +5161,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) > struct ice_hw *hw; > int err; > > - if (pdev->is_virtfn) { > - dev_err(dev, "can't probe a virtual function\n"); > - return -EINVAL; > - } > + if (pdev->is_virtfn) > + return dev_err_probe(dev, -EINVAL, "can't probe a virtual function\n"); > > /* when under a kdump kernel initiate a reset before enabling the > * device in order to clear out any pending DMA transactions. These > @@ -5188,10 +5186,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) > return err; > > err = pcim_iomap_regions(pdev, BIT(ICE_BAR0), dev_driver_string(dev)); > - if (err) { > - dev_err(dev, "BAR0 I/O map error %d\n", err); > - return err; > - } > + if (err) > + return dev_err_probe(dev, err, "BAR0 I/O map error %d\n", err); > > pf = ice_allocate_pf(dev); > if (!pf) > @@ -5202,10 +5198,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) > > /* set up for high or low DMA */ > err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > - if (err) { > - dev_err(dev, "DMA configuration failed: 0x%x\n", err); > - return err; > - } > + if (err) > + return dev_err_probe(dev, err, "DMA configuration failed: 0x%x\n", err); > > pci_set_master(pdev); > pf->pdev = pdev; > @@ -5240,10 +5234,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) > return ice_probe_recovery_mode(pf); > > err = ice_init_hw(hw); > - if (err) { > - dev_err(dev, "ice_init_hw failed: %d\n", err); > - return err; > - } > + if (err) > + return dev_err_probe(dev, err, "ice_init_hw failed: %d\n", err); > > ice_init_dev_hw(pf); > > -- > 2.25.1 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] ice: use dev_err_probe() in ice_probe() 2026-07-01 14:37 ` Maciej Fijalkowski @ 2026-07-02 7:05 ` weirongguang 2026-07-02 12:47 ` Przemek Kitszel 0 siblings, 1 reply; 4+ messages in thread From: weirongguang @ 2026-07-02 7:05 UTC (permalink / raw) To: Maciej Fijalkowski, Rongguang Wei Cc: netdev, intel-wired-lan, aleksandr.loktionov, przemyslaw.kitszel, anthony.l.nguyen, andrew+netdev 在 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. >> >> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c >> index e2fd2dab03e3..31aa42f8e6d3 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_main.c >> +++ b/drivers/net/ethernet/intel/ice/ice_main.c >> @@ -5161,10 +5161,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) >> struct ice_hw *hw; >> int err; >> >> - if (pdev->is_virtfn) { >> - dev_err(dev, "can't probe a virtual function\n"); >> - return -EINVAL; >> - } >> + if (pdev->is_virtfn) >> + return dev_err_probe(dev, -EINVAL, "can't probe a virtual function\n"); >> >> /* when under a kdump kernel initiate a reset before enabling the >> * device in order to clear out any pending DMA transactions. These >> @@ -5188,10 +5186,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) >> return err; >> >> err = pcim_iomap_regions(pdev, BIT(ICE_BAR0), dev_driver_string(dev)); >> - if (err) { >> - dev_err(dev, "BAR0 I/O map error %d\n", err); >> - return err; >> - } >> + if (err) >> + return dev_err_probe(dev, err, "BAR0 I/O map error %d\n", err); >> >> pf = ice_allocate_pf(dev); >> if (!pf) >> @@ -5202,10 +5198,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) >> >> /* set up for high or low DMA */ >> err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); >> - if (err) { >> - dev_err(dev, "DMA configuration failed: 0x%x\n", err); >> - return err; >> - } >> + if (err) >> + return dev_err_probe(dev, err, "DMA configuration failed: 0x%x\n", err); >> >> pci_set_master(pdev); >> pf->pdev = pdev; >> @@ -5240,10 +5234,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) >> return ice_probe_recovery_mode(pf); >> >> err = ice_init_hw(hw); >> - if (err) { >> - dev_err(dev, "ice_init_hw failed: %d\n", err); >> - return err; >> - } >> + if (err) >> + return dev_err_probe(dev, err, "ice_init_hw failed: %d\n", err); >> >> ice_init_dev_hw(pf); >> >> -- >> 2.25.1 >> >> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] ice: use dev_err_probe() in ice_probe() 2026-07-02 7:05 ` weirongguang @ 2026-07-02 12:47 ` Przemek Kitszel 0 siblings, 0 replies; 4+ messages in thread From: Przemek Kitszel @ 2026-07-02 12:47 UTC (permalink / raw) To: weirongguang, Maciej Fijalkowski, Rongguang Wei Cc: netdev, intel-wired-lan, aleksandr.loktionov, anthony.l.nguyen, andrew+netdev 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-02 12:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-01 1:36 [PATCH net-next v2] ice: use dev_err_probe() in ice_probe() Rongguang Wei 2026-07-01 14:37 ` Maciej Fijalkowski 2026-07-02 7:05 ` weirongguang 2026-07-02 12:47 ` Przemek Kitszel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox