On Fri, 3 Sep 2021 09:26:09 -0700 Dan Williams <[email protected]> wrote:
> On Fri, Sep 3, 2021 at 6:34 AM Jonathan Cameron > <[email protected]> wrote: > > > > On Tue, 24 Aug 2021 09:07:56 -0700 > > Dan Williams <[email protected]> wrote: > > > > > The kbuild robot reports: > > > > > > drivers/cxl/core/bus.c:516:1: warning: stack frame size (1032) exceeds > > > limit (1024) in function 'devm_cxl_add_decoder' > > > > > > It is also the case the devm_cxl_add_decoder() is unwieldy to use for > > > all the different decoder types. Fix the stack usage by splitting the > > > creation into alloc and add steps. This also allows for context > > > specific construction before adding. > > > > > > Reported-by: kernel test robot <[email protected]> > > > Signed-off-by: Dan Williams <[email protected]> > > > > Trivial comment inline - otherwise looks like a nice improvement. > > > > Reviewed-by: Jonathan Cameron <[email protected]> > > > > > > --- > > > drivers/cxl/acpi.c | 74 ++++++++++++++++++++--------- > > > drivers/cxl/core/bus.c | 124 > > > +++++++++++++++--------------------------------- > > > drivers/cxl/cxl.h | 15 ++---- > > > 3 files changed, 95 insertions(+), 118 deletions(-) > > > > > > > > @@ -268,6 +275,7 @@ static int add_host_bridge_uport(struct device > > > *match, void *arg) > > > struct cxl_port *port; > > > struct cxl_dport *dport; > > > struct cxl_decoder *cxld; > > > + int single_port_map[1], rc; > > > struct cxl_walk_context ctx; > > > struct acpi_pci_root *pci_root; > > > struct cxl_port *root_port = arg; > > > @@ -301,22 +309,42 @@ static int add_host_bridge_uport(struct device > > > *match, void *arg) > > > return -ENODEV; > > > if (ctx.error) > > > return ctx.error; > > > + if (ctx.count > 1) > > > + return 0; > > > > > > /* TODO: Scan CHBCR for HDM Decoder resources */ > > > > > > /* > > > - * In the single-port host-bridge case there are no HDM decoders > > > - * in the CHBCR and a 1:1 passthrough decode is implied. > > > + * Per the CXL specification (8.2.5.12 CXL HDM Decoder Capability > > > + * Structure) single ported host-bridges need not publish a decoder > > > + * capability when a passthrough decode can be assumed, i.e. all > > > + * transactions that the uport sees are claimed and passed to the > > > single > > > + * dport. Default the range a 0-base 0-length until the first CXL > > > region > > > + * is activated. > > > */ > > > > Is comment in right place or should it be up with the ctx.count > 1 > > This comment is specifically about the implicit decoder, right beneath > the comment, that is registered in the ctx.count == 1 case. Perhaps > you were reacting to the spec reference which is generic, but later > sentences make it clear this comment is about an exception noted in > that spec reference? More that the conditional is above that leads to us getting here. Probably thrown off by Diff having added the new block that follows this after the removed block rather than before it. I'm fine with how you have it here. J
