Alexey Kardashevskiy wrote: > > > On 27/8/25 09:54, dan.j.williams@xxxxxxxxx wrote: > > Alexey Kardashevskiy wrote: > > [..] > > [trim multiple pages of uncommented context, please trim your replies] > > > >>> +/** > >>> + * pci_tsm_pf0_initialize() - common 'struct pci_tsm_pf0' initialization > >>> + * @pdev: Physical Function 0 PCI device > >>> + * @tsm: context to initialize > >>> + */ > >>> +int pci_tsm_pf0_initialize(struct pci_dev *pdev, struct pci_tsm_pf0 *tsm) > >> > >> Here it is: struct pci_tsm_pf0 *tsm (it is really a "dsm") > > > > All of the context returned by the TSM driver is a "tsm" context, the > > only time use "dsm" is in referring to the actual pci_dev that runs the > > SPDM session. > > ah ok. Just seems a bit counterintuitive to use a short "tsm" acronym for something else than the actual TSM as defined in the PCI spec and in this case - it is the opposite to the spec's "TSM" - it is a DSM, the other end of trust chain. And if I wanted to reference the actual TSM in the same function - that would be "tsm_dev". And the actual DSM pci_dev is barely used, it is mostly "struct pci_tsm_pf0". The "dsm" pci_dev is used for type-checking assignable subfunctions vs the device that runs the SPDM session, and it is used as the device to lock when running operations on any subfunction. > >> In pci_tsm: struct pci_dev *dsm (alright) > >> > >> May be we need some distinction between PF0's pci_dev and pci_tsm_pf0 but still these are DSMs. > >> > >> In pci_tsm_pf0 it is: struct pci_tsm tsm, imho "base" is less confusing (I keep catching myself thinking it is a pointer to tsm_dev). > > > > Ok, I can change it to base. > > > >> "tsm" would be what you call "tsm_dev" which is ok but seeing short "tsm" used as "dsm" or "TSM data for this pci_dev" is confusing. > >> > >> s/pci_tsm/pci_tsm_ctx/ and s/tsm/tsm_ctx/ ? Thanks, > > > > What is a tsm_ctx? The s/pci_tsm/pci_tsm_ctx/ rename is not adding more > > clarity for me. > > TSM-related attributes of a PCI device. Not the best name, true. > > > If Aneesh or Yilun also find that rename clarifying I > > will switch. For v5 I will stick with 'struct pci_tsm' as the PCI object > > that wraps TSM driver produced objects. > yeah, if it is just me, then never mind, I'll get used to it. > > > The reason I do not think of "pci_tsm" as a "tsm_dev" is because PCI is > > always a consumer of an outside of PCI TSM service provided, PCI does > > not have a TSM concept internal to it. > "struct pci_dev" describes a PCI device, "struct pci_ide" - a PCI IDE stream but "struct pci_tsm" does not describe PCI TSM... Hm. Thanks, How about keep ->tsm as the short name in 'struct pci_dev', but rename the type 'struct pci_tsm_ctl'? It is a core control data structure for TSM operations. It is not a "context" like device drvdata where the owner can do whatever it wants, it is a core control structure that a low-level TSM driver can wrap with its own control attributes.
