On Tue, 15 Mar 2022 21:14:08 -0700
Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> Update the port driver to use cxl_map_component_registers() so that the
> component register block can be shared between the cxl_pci driver and
> the cxl_port driver. I.e. stop the port driver from reserving the entire
> component register block for itself via request_region() when it only
> needs the HDM Decoder Capability subset.
>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
We go through a dance in the other callers to cxl_probe_component_regs
cxl_map_component_regs that means we don't have block mapped at tim
of calling cxl_map_component_regs().
I'd gotten it into my head that we had that separation for a reason,
and it doesn't exist here as both are called with the block mapped.
Looking again I think that was needed because of use of pci_iomap
for the bar so I guess the same constraint doesn't apply here?
Thanks,
Jonathan
> ---
> drivers/cxl/core/hdm.c | 32 ++++++++++++++++++--------------
> 1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0e89a7a932d4..09221afca309 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -77,18 +77,22 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
> cxlhdm->interleave_mask |= GENMASK(14, 12);
> }
>
> -static void __iomem *map_hdm_decoder_regs(struct cxl_port *port,
> - void __iomem *crb)
> +static int map_hdm_decoder_regs(struct cxl_port *port, void __iomem *crb,
> + struct cxl_component_regs *regs)
> {
> - struct cxl_component_reg_map map;
> + struct cxl_register_map map = {
> + .resource = port->component_reg_phys,
> + .base = crb,
> + .max_size = CXL_COMPONENT_REG_BLOCK_SIZE,
> + };
>
> - cxl_probe_component_regs(&port->dev, crb, &map);
> - if (!map.hdm_decoder.valid) {
> + cxl_probe_component_regs(&port->dev, crb, &map.component_map);
> + if (!map.component_map.hdm_decoder.valid) {
> dev_err(&port->dev, "HDM decoder registers invalid\n");
> - return IOMEM_ERR_PTR(-ENXIO);
> + return -ENXIO;
> }
>
> - return crb + map.hdm_decoder.offset;
> + return cxl_map_component_regs(&port->dev, regs, &map);
> }
>
> /**
> @@ -98,25 +102,25 @@ static void __iomem *map_hdm_decoder_regs(struct cxl_port *port,
> struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port)
> {
> struct device *dev = &port->dev;
> - void __iomem *crb, *hdm;
> struct cxl_hdm *cxlhdm;
> + void __iomem *crb;
> + int rc;
>
> cxlhdm = devm_kzalloc(dev, sizeof(*cxlhdm), GFP_KERNEL);
> if (!cxlhdm)
> return ERR_PTR(-ENOMEM);
>
> cxlhdm->port = port;
> - crb = devm_cxl_iomap_block(dev, port->component_reg_phys,
> - CXL_COMPONENT_REG_BLOCK_SIZE);
> + crb = ioremap(port->component_reg_phys, CXL_COMPONENT_REG_BLOCK_SIZE);
> if (!crb) {
> dev_err(dev, "No component registers mapped\n");
> return ERR_PTR(-ENXIO);
> }
>
> - hdm = map_hdm_decoder_regs(port, crb);
> - if (IS_ERR(hdm))
> - return ERR_CAST(hdm);
> - cxlhdm->regs.hdm_decoder = hdm;
> + rc = map_hdm_decoder_regs(port, crb, &cxlhdm->regs);
> + iounmap(crb);
> + if (rc)
> + return ERR_PTR(rc);
>
> parse_hdm_decoder_caps(cxlhdm);
> if (cxlhdm->decoder_count == 0) {
>
