Jonathan Cameron wrote:
[..]
> A few minor things inline.
[..]
> > +/**
> > + * pci_ide_stream_enable() - try to enable a Selective IDE Stream
> > + * @pdev: PCIe device object for either a Root Port or Endpoint Partner Port
> > + * @ide: registered and setup IDE settings descriptor
> > + *
> > + * Activate the stream by writing to the Selective IDE Stream Control
> > + * Register, report whether the state successfully transitioned to
> > + * secure mode.
> and report
ack.
[..]
> > diff --git a/include/linux/pci-ide.h b/include/linux/pci-ide.h
> > new file mode 100644
> > index 000000000000..89c1ef0de841
> > --- /dev/null
> > +++ b/include/linux/pci-ide.h
> > @@ -0,0 +1,70 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> ...
>
> > +/**
> > + * struct pci_ide_partner - Per port pair Selective IDE Stream settings
> > + * @rid_start: Partner Port Requester ID range start
> > + * @rid_start: Partner Port Requester ID range end
> > + * @stream_index: Selective IDE Stream Register Block selection
> > + * @setup: flag to track whether to run pci_ide_stream_teardown for this parnter slot
>
> partner.
>
> > + * @enable: flag whether to run pci_ide_stream_disable for this parnter slot
>
> same again.
yes.
> > +/**
> > + * struct pci_ide - PCIe Selective IDE Stream descriptor
> > + * @pdev: PCIe Endpoint in the pci_ide_partner pair
> > + * @partner: Per-partner settings
> per-partner maybe? Capitalization seems a little random
> as mostly you have used them for spec terms, but Per-partner probably
> isn't one?
true.
> > + * @host_bridge_stream: track platform Stream ID
> > + * @stream_id: unique Stream ID (within Partner Port pairing)
> > + * @name: name of the established Selective IDE Stream in sysfs
> > + *
> > + * Negative @stream_id values indicate "uninitialized" on the
> > + * expectation that with TSM established IDE the TSM owns the stream_id
> > + * allocation.
> > + */
> > +struct pci_ide {
> > + struct pci_dev *pdev;
> > + struct pci_ide_partner partner[PCI_IDE_PARTNER_MAX];
> > + u8 host_bridge_stream;
> > + int stream_id;
> > + const char *name;
> > +};
>
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index a7353df51fea..cc83ae274601 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -538,6 +538,8 @@ struct pci_dev {
> > u16 ide_cap; /* Link Integrity & Data Encryption */
> > u8 nr_ide_mem; /* Address association resources for streams */
> > u8 nr_link_ide; /* Link Stream count (Selective Stream offset) */
> > + u8 nr_sel_ide; /* Selective Stream count (register block allocator) */
> > + DECLARE_BITMAP(ide_stream_map, CONFIG_PCI_IDE_STREAM_MAX);
> > unsigned int ide_cfg:1; /* Config cycles over IDE */
> > unsigned int ide_tee_limit:1; /* Disallow T=0 traffic over IDE */
> > #endif
> > @@ -607,6 +609,10 @@ struct pci_host_bridge {
> > int domain_nr;
> > struct list_head windows; /* resource_entry */
> > struct list_head dma_ranges; /* dma ranges resource list */
> > +#ifdef CONFIG_PCI_IDE
> > + u8 nr_ide_streams; /* Track available vs in-use streams */
>
> Which does it do? Confusing comment.
Oh, true, I was going for a combo comment for nr_ide_streams and
ide_stream_map, but missed on the clarity. Make that relationship
clearer:
- u8 nr_ide_streams; /* Track available vs in-use streams */
+ u8 nr_ide_streams; /* Max streams possibly active in @ide_stream_map */
