Re: [Intel-wired-lan] [PATCH iwl-next v2] ice: Extend auxbus device naming

[Jiri Pirko <jiri@resnulli.us>]

Tue, Apr 23, 2024 at 11:14:59AM CEST, sergey.temerkhanov@intel.com wrote:
>Include segment/domain number in the device name to distinguish
>between PCI devices located on different root complexes in
>multi-segment configurations. Naming is changed from
>ptp_<bus>_<slot>_clk<clock>  to ptp_<domain>_<bus>_<slot>_clk<clock>

I don't understand why you need to encode pci properties of a parent
device into the auxiliary bus name. Could you please explain the
motivation? Why you need a bus instance per PF?

The rest of the auxbus registrators don't do this. Could you please
align? Just have one bus for ice driver and that's it.


>
>v1->v2
>Rebase on top of the latest changes
>
>Signed-off-by: Sergey Temerkhanov <sergey.temerkhanov@intel.com>
>Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>---
> drivers/net/ethernet/intel/ice/ice_ptp.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
>index 402436b72322..744b102f7636 100644
>--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
>+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
>@@ -2993,8 +2993,9 @@ ice_ptp_auxbus_create_id_table(struct ice_pf *pf, const char *name)
> static int ice_ptp_register_auxbus_driver(struct ice_pf *pf)
> {
> 	struct auxiliary_driver *aux_driver;
>+	struct pci_dev *pdev = pf->pdev;
> 	struct ice_ptp *ptp;
>-	char busdev[8] = {};
>+	char busdev[16] = {};
> 	struct device *dev;
> 	char *name;
> 	int err;
>@@ -3005,8 +3006,10 @@ static int ice_ptp_register_auxbus_driver(struct ice_pf *pf)
> 	INIT_LIST_HEAD(&ptp->ports_owner.ports);
> 	mutex_init(&ptp->ports_owner.lock);
> 	if (ice_is_e810(&pf->hw))
>-		sprintf(busdev, "%u_%u_", pf->pdev->bus->number,
>-			PCI_SLOT(pf->pdev->devfn));
>+		snprintf(busdev, sizeof(busdev), "%u_%u_%u_",
>+			 pci_domain_nr(pdev->bus),
>+			 pdev->bus->number,
>+			 PCI_SLOT(pdev->devfn));
> 	name = devm_kasprintf(dev, GFP_KERNEL, "ptp_%sclk%u", busdev,
> 			      ice_get_ptp_src_clock_index(&pf->hw));
> 	if (!name)
>@@ -3210,8 +3213,9 @@ static void ice_ptp_release_auxbus_device(struct device *dev)
> static int ice_ptp_create_auxbus_device(struct ice_pf *pf)
> {
> 	struct auxiliary_device *aux_dev;
>+	struct pci_dev *pdev = pf->pdev;
> 	struct ice_ptp *ptp;
>-	char busdev[8] = {};
>+	char busdev[16] = {};
> 	struct device *dev;
> 	char *name;
> 	int err;
>@@ -3224,8 +3228,10 @@ static int ice_ptp_create_auxbus_device(struct ice_pf *pf)
> 	aux_dev = &ptp->port.aux_dev;
> 
> 	if (ice_is_e810(&pf->hw))
>-		sprintf(busdev, "%u_%u_", pf->pdev->bus->number,
>-			PCI_SLOT(pf->pdev->devfn));
>+		snprintf(busdev, sizeof(busdev), "%u_%u_%u_",
>+			 pci_domain_nr(pdev->bus),
>+			 pdev->bus->number,
>+			 PCI_SLOT(pdev->devfn));
> 
> 	name = devm_kasprintf(dev, GFP_KERNEL, "ptp_%sclk%u", busdev,
> 			      ice_get_ptp_src_clock_index(&pf->hw));
>-- 
>2.35.3
>
>