Re: [RFC PATCH v2 08/28] cxl/port: Introduce a port driver

[Ben Widawsky <ben.widawsky@intel.com>]

On 21-10-29 18:37:36, Dan Williams wrote:
> On Fri, Oct 22, 2021 at 11:37 AM Ben Widawsky <ben.widawsky@intel.com> wrote:
> >

[snip]

> 
> +void trigger_decoder_autoremove(void *data)
> +{
> +       struct cxl_decoder *cxld = data;
> +       struct device *host = get_cxl_topology_host();
> +
> +       /* The topology host driver beat us to the punch */
> +       if (!host)
> +               return;
> +
> +       devm_remove_action(host, cxld_unregister, &cxld->dev);
> +       put_cxl_topology_host(host);
> +}
> +
> +/**
> + * cxl_port_decoder_autoremove - remove decoders discovered by the port driver
> + * @cxld: decoder to remove
> + *
> + * CXL.mem requires an intact port / decoder topology from root level
> + * platform decoders to endpoint decoders. Arrange for decoders
> + * enumerated by the port driver to be removed when either the root is
> + * removed (in which the entire hierarchy is removed), or when an
> + * individual port is disabled, in which case only that port's
> + * sub-section of the hierarchy is removed.
> + */
> +int cxl_port_decoder_autoremove(struct cxl_decoder *cxld)
> +{
> +       struct cxl_port *port = to_cxl_port(cxld->dev.parent);
> +       struct device *host = get_cxl_topology_host();
> +       int rc;
> +
> +       if (!host)
> +               return -ENODEV;
> +
> +       if (!port->dev.driver) {
> +               rc = -ENXIO;
> +               goto out;
> +       }
> +
> +       rc = cxl_decoder_autoremove(host, cxld);
> +       if (rc)
> +               goto out;
> +
> +       rc = devm_add_action_or_reset(&port->dev, trigger_decoder_autoremove,
> +                                       cxld);
> +out:
> +       put_cxl_topology_host(host);
> +       return rc;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_port_decoder_autoremove, CXL);

The only port which has decoders that aren't discovered by the port driver would
be the "root port" because it's platform specific. However, that port still is
treated similarly to the other ports. Therefore I don't see why you need
cxl_decoder_autoremove() anymore. Could you please explain? I think the safety
check in trigger_decoder_autoremove() makes this work for all cases.

[snip]