Hi Stefano, > On 23 Sep 2021, at 8:12 pm, Stefano Stabellini <sstabell...@kernel.org> wrote: > > On Thu, 23 Sep 2021, Rahul Singh wrote: >>>> + goto err_exit; >>>> + } >>> >>> This is unnecessary at the moment, right? Can we get rid of ops->init ? >> >> No this is required for N1SDP board. Please check below patch. >> https://gitlab.com/rahsingh/xen-integration/-/commit/6379ba5764df33d57547087cff4ffc078dc515d5 > > OK > > >>>> +int pci_host_common_probe(struct dt_device_node *dev, const void *data) >>>> +{ >>>> + struct pci_host_bridge *bridge; >>>> + struct pci_config_window *cfg; >>>> + struct pci_ecam_ops *ops; >>>> + const struct dt_device_match *of_id; >>>> + int err; >>>> + >>>> + if ( dt_device_for_passthrough(dev) ) >>>> + return 0; >>>> + >>>> + of_id = dt_match_node(dev->dev.of_match_table, dev->dev.of_node); >>>> + ops = (struct pci_ecam_ops *) of_id->data; >>> >>> Do we really need dt_match_node and dev->dev.of_match_table to get >>> dt_device_match.data? >>> >> >>> data is passed as a parameter to pci_host_common_probe, isn't it enough >>> to do: >>> >>> ops = (struct pci_ecam_ops *) data; >> >> As of now not required but in future we might need it if we implement other >> ecam supported bridge >> >> static const struct dt_device_match gen_pci_dt_match[] = { >> >> { .compatible = "pci-host-ecam-generic", >> >> .data = &pci_generic_ecam_ops }, >> >> { .compatible = "pci-host-cam-generic", >> .data = &gen_pci_cfg_cam_bus_ops }, >> >> { }, >> >> }; > > Even if we add another ECAM-supported bridge, the following: > > ops = (struct pci_ecam_ops *) data; > > could still work, right? The probe function will directly receive as > parameter the .data pointer. You shouldn't need the indirection via > dt_match_node?
As per my understanding probe function will not get .data pointer.Probe data argument is NULL in most of the cases in XEN Please have a look once dt_pci_init() -> device_init(..) call flow implementation. Regards, Rahul > > If you are worried about gen_pci_cfg_cam_bus_ops not being a struct > pci_ecam_ops: that problem can also be solved by making > gen_pci_cfg_cam_bus_ops a struct containinig a struct pci_ecam_ops.
