On Fri, Sep 3, 2021 at 11:01 AM Jonathan Cameron <[email protected]> wrote: > > 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. >
Thanks, although I am going to leave your reviewed-by off for now because I am going to fold this with this fixup patch that arrived later which makes some significant changes: https://lore.kernel.org/r/163002073312.1700305.17017280228713298614.st...@dwillia2-desk3.amr.corp.intel.com
