On Mon, Apr 12, 2021 at 02:51:18PM +0000, Saleem, Shiraz wrote: > > Where is ftype initialized? > > Today it is just pf. But the upcoming Intel ethernet VF driver will > set it to true. Then it is dead code, don't send dead code to the kernel. > > This cdev_info should just be a 'struct ice_pf *' and the "struct > > iidc_core_dev_info" should be deleted entirely. You'll notice this > > ends up looking nearly exactly like mlx5 does after this. > > It was intentionally designed to be core device object carving out > only pieces of the PF information required by the rdma driver. The > next near-term PCI driver using IIDC can also this. Why expose the > whole PF? This is a design choice no? Why do we need to follow mlx5? When you get around to building your multi-driver API it should be structured so it doesn't have a de-normalization of the data - don't copy from one authoritative struct to some other struct just to get some weird information hiding. The PF driver should be a subclass if your "generic" driver and directly embed some struct like this as the singular canonical source of information, not be duplicated. > I don't follow what the hackery is. Just because we use cdev_info in > the .ops callbacks as opposed to ice_pf? There are too many signs to ignore: - The obfuscated extensible structs being passed into ops that are only encoding a couple function call parameters - The ops that only have one implementation - The struct that is a complete copy of a different, but "internal", struct You do stuff like this to make stable ABIs. This is forbidden by Linus for in-kernel APIs, and it is not the kernel style in general to code like this. Jason
