Jonathan Cameron wrote:
> On Thu, 05 Dec 2024 14:24:19 -0800
> Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>
> > Exercise common setup and teardown flows for a sample platform TSM
> > driver that implements the TSM 'connect' and 'disconnect' flows.
> >
> > This is both a template for platform specific implementations and a test
> > case for the shared infrastructure.
> >
> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > Cc: Lukas Wunner <lukas@xxxxxxxxx>
> > Cc: Samuel Ortiz <sameo@xxxxxxxxxxxx>
> > Cc: Alexey Kardashevskiy <aik@xxxxxxx>
> > Cc: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> Trivial comments inline.
>
> > static int devsec_tsm_connect(struct pci_dev *pdev)
> > {
> > - return -ENXIO;
> > + struct pci_ide *ide;
> > + int rc, stream_id;
> > +
> > + stream_id =
> > + find_first_zero_bit(devsec_stream_ids, DEVSEC_NR_IDE_STREAMS);
> > + if (stream_id == DEVSEC_NR_IDE_STREAMS)
> > + return -EBUSY;
> > + set_bit(stream_id, devsec_stream_ids);
> > + ide = &devsec_streams[stream_id].ide;
> > + pci_ide_stream_probe(pdev, ide);
> > +
> > + ide->stream_id = stream_id;
> > + rc = pci_ide_stream_setup(pdev, ide, PCI_IDE_SETUP_ROOT_PORT);
> > + if (rc)
> > + return rc;
> > + rc = tsm_register_ide_stream(pdev, ide);
> > + if (rc)
> > + goto err;
> > +
> > + devsec_streams[stream_id].pdev = pdev;
> > + pci_ide_enable_stream(pdev, ide);
> > + return 0;
> > +err:
> > + pci_ide_stream_teardown(pdev, ide, PCI_IDE_SETUP_ROOT_PORT);
>
> I'd kind of expect to see more of what we have in disconnect here.
> Like clearing the bit.
Yeah, that was a leak, now fixed and symmetric.
> > + return rc;
> > }
> >
> > static void devsec_tsm_disconnect(struct pci_dev *pdev)
> > {
> > + struct pci_ide *ide;
> > + int i;
> > +
> > + for_each_set_bit(i, devsec_stream_ids, DEVSEC_NR_IDE_STREAMS)
> > + if (devsec_streams[i].pdev == pdev)
> > + break;
> > +
> > + if (i >= DEVSEC_NR_IDE_STREAMS)
> > + return;
> > +
> > + ide = &devsec_streams[i].ide;
> > + pci_ide_disable_stream(pdev, ide);
> > + tsm_unregister_ide_stream(ide);
> > + pci_ide_stream_teardown(pdev, ide, PCI_IDE_SETUP_ROOT_PORT);
> > + devsec_streams[i].pdev = NULL;
> If this setting to NULL needs to be out of order wrt to what happens
> in probe, add a comment. If not move it to after pci_ide_disable_steram()
Also fixed with the same symmetry as the connect unwind case.
