On 21-11-22 17:11:42, Jonathan Cameron wrote:
> On Fri, 19 Nov 2021 16:02:46 -0800
> Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote:
>
> > The component register base address is enumerated and stored in the
> > cxl_device_state structure for use by the endpoint driver.
> >
> > Component register base addresses are obtained through PCI mechanisms.
> > As such it makes most sense for the cxl_pci driver to obtain that
> > address. In order to reuse the port driver for enumerating decoder
> > resources for an endpoint, it is desirable to be able to add the
> > endpoint as a port. The endpoint does know of the cxlds and can pull the
> > component register base from there and pass it along to port creation.
>
> This feels like a lot of explanation in for trivial caching of an address.
> I'm not sure you need to be that detailed, though I guess it does no real
> harm.
It is mostly to articulate that cxl_pci is responsible for PCI stuff, and
cxl_mem is responsible for CXL.mem stuff, and that's why we didn't just do this
work in cxl_mem (which indeed was what I originally did).
>
> Another one where I'm unsure why we are muddling on after an error...
Same motivation as the other - mailbox can still be used even if the media isn't
available.
>
>
> >
> > Signed-off-by: Ben Widawsky <ben.widawsky@xxxxxxxxx>
> > ---
> > Changes since RFCv2:
> > This patch was originally named, "cxl/core: Store component register
> > base for memdevs". It plumbed the component registers through memdev
> > creation. After more work, it became apparent we needed to plumb other
> > stuff from the pci driver, so going forward, cxlds will just be
> > referenced by the cxl_mem driver. This also allows us to ignore the
> > change needed to cxl_test
> >
> > - Rework patch to store the base in cxlds
> > - Remove defunct comment (Dan)
> > ---
> > drivers/cxl/cxlmem.h | 2 ++
> > drivers/cxl/pci.c | 11 +++++++++++
> > 2 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> > index a9424dd4e5c3..b1d753541f4e 100644
> > --- a/drivers/cxl/cxlmem.h
> > +++ b/drivers/cxl/cxlmem.h
> > @@ -134,6 +134,7 @@ struct cxl_endpoint_dvsec_info {
> > * @next_volatile_bytes: volatile capacity change pending device reset
> > * @next_persistent_bytes: persistent capacity change pending device reset
> > * @info: Cached DVSEC information about the device.
> > + * @component_reg_phys: register base of component registers
> > * @mbox_send: @dev specific transport for transmitting mailbox commands
> > *
> > * See section 8.2.9.5.2 Capacity Configuration and Label Storage for
> > @@ -165,6 +166,7 @@ struct cxl_dev_state {
> > u64 next_persistent_bytes;
> >
> > struct cxl_endpoint_dvsec_info *info;
> > + resource_size_t component_reg_phys;
> >
> > int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
> > int (*wait_media_ready)(struct cxl_dev_state *cxlds);
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index f1a68bfe5f77..a8e375950514 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -663,6 +663,17 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > if (rc)
> > return rc;
> >
> > + /*
> > + * If the component registers can't be found, the cxl_pci driver may
> > + * still be useful for management functions so don't return an error.
> > + */
> > + cxlds->component_reg_phys = CXL_RESOURCE_NONE;
> > + rc = cxl_setup_regs(pdev, CXL_REGLOC_RBI_COMPONENT, &map);
> > + if (rc)
> > + dev_warn(&cxlmd->dev, "No component registers (%d)\n", rc);
> > + else
> > + cxlds->component_reg_phys = cxl_reg_block(pdev, &map);
> > +
> > rc = cxl_pci_setup_mailbox(cxlds);
> > if (rc)
> > return rc;
>
