Re: [PATCH v7 3/9] PCI: Introduce pci_walk_bus_reverse(), for_each_pci_dev_reverse()

[<dan.j.williams@intel.com>]

Jonathan Cameron wrote:
> On Thu, 23 Oct 2025 19:04:12 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > PCI/TSM, the PCI core functionality for the PCIe TEE Device Interface
> > Security Protocol (TDISP), has a need to walk all subordinate functions of
> > a Device Security Manager (DSM) to setup a device security context. A DSM
> > is physical function 0 of multi-function or SR-IOV device endpoint, or it
> > is an upstream switch port.
> > 
> > In error scenarios or when a TEE Security Manager (TSM) device is removed
> > it needs to unwind all established DSM contexts.
> > 
> > Introduce reverse versions of PCI device iteration helpers to mirror the
> > setup path and ensure that dependent children are handled before parents.
> > 
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Bit of archaeology was needed as there are some existing oddities in the
> functions this is based on.
> 
> My suggestions for this are don't use guard() and drop the void * cast that
> we should cleanup in the existing code.

I will keep cleanups that imply touching old functions for a potential
follow-on, but likely will let sleeping dogs lie.

[..]
> > +/**
> > + * pci_walk_bus_reverse - walk devices on/under bus, calling callback.
> > + * @top: bus whose devices should be walked
> > + * @cb: callback to be called for each device found
> > + * @userdata: arbitrary pointer to be passed to callback
> > + *
> > + * Same semantics as pci_walk_bus(), but walks the bus in reverse order.
> > + */
> > +void pci_walk_bus_reverse(struct pci_bus *top,
> > +			  int (*cb)(struct pci_dev *, void *), void *userdata)
> > +{
> > +	guard(rwsem_read)(&pci_bus_sem);
> 
> So this ends up different to pci_walk_bus.  I'd be tempted to just
> not bother bringing a single guard() usage here. Gain is trivial and
> mixing and matching style in a file isn't particularly nice.

Yeah, and violates my earlier claim about being style-bug compatible.

[..]

 3:  f8e6f3d9ba81 !  3:  5ae3e927d3ed PCI: Introduce pci_walk_bus_reverse(), for_each_pci_dev_reverse()
    @@ drivers/pci/bus.c: void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_d
     +void pci_walk_bus_reverse(struct pci_bus *top,
     +                    int (*cb)(struct pci_dev *, void *), void *userdata)
     +{
    -+  guard(rwsem_read)(&pci_bus_sem);
    ++  down_read(&pci_bus_sem);
     +  __pci_walk_bus_reverse(top, cb, userdata);
    ++  up_read(&pci_bus_sem);
     +}
     +EXPORT_SYMBOL_GPL(pci_walk_bus_reverse);
     +