| Message ID | 1439957971-30483-2-git-send-email-imunsie@au.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, 2015-08-19 at 14:19 +1000, Ian Munsie wrote: > From: Michael Neuling <mikey@neuling.org> > > This provides AFU drivers a means to associate private data with a cxl > context. This is particularly intended for make the new callbacks for > driver specific events easier for AFU drivers to use, as they can easily > get back to any private data structures they may use. > > Signed-off-by: Michael Neuling <mikey@neuling.org> > Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> > --- > drivers/misc/cxl/api.c | 21 +++++++++++++++++++++ > drivers/misc/cxl/cxl.h | 3 +++ > include/misc/cxl.h | 7 +++++++ > 3 files changed, 31 insertions(+) > > diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c > index e0f0c78..5f0b22e 100644 > --- a/drivers/misc/cxl/api.c > +++ b/drivers/misc/cxl/api.c > @@ -70,6 +70,27 @@ int cxl_release_context(struct cxl_context *ctx) > } > EXPORT_SYMBOL_GPL(cxl_release_context); > > + > +int cxl_set_priv(struct cxl_context *ctx, void *priv) > +{ > + if (!ctx) > + return -EINVAL; > + > + ctx->priv = priv; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(cxl_set_priv); > + > +void *cxl_get_priv(struct cxl_context *ctx) > +{ > + if (!ctx) > + return ERR_PTR(-EINVAL); > + > + return ctx->priv; > +} > +EXPORT_SYMBOL_GPL(cxl_get_priv); Do we really need the accessors? They don't buy anything I can see over just using ctx->priv directly. cheers
Excerpts from Michael Ellerman's message of 2015-08-19 14:49:30 +1000: > Do we really need the accessors? They don't buy anything I can see over just > using ctx->priv directly. The reasoning there is because we don't currently expose the contents of stuct cxl_context to afu drivers, rather they just treat it as an opaque type. We could potentially change this to expose the details, but there's a lot of junk in there that's just internal details of the cxl driver that isn't of interest to an afu driver that I'd rather not expose. We also already have another accessor function (cxl_process_element) in the api, so it's not out of place. FWIW I'm not opposed to changing how this api works if it ultimately makes things better, but I want to wait until the cxlflash superpipe support is merged so any patches that change the api can change it at the same time. Cheers, -Ian
On Wed, 2015-08-19 at 15:12 +1000, Ian Munsie wrote: > Excerpts from Michael Ellerman's message of 2015-08-19 14:49:30 +1000: > > Do we really need the accessors? They don't buy anything I can see over just > > using ctx->priv directly. > > The reasoning there is because we don't currently expose the contents of > stuct cxl_context to afu drivers, rather they just treat it as an opaque > type. > > We could potentially change this to expose the details, but there's a > lot of junk in there that's just internal details of the cxl driver that > isn't of interest to an afu driver that I'd rather not expose. > > We also already have another accessor function (cxl_process_element) in > the api, so it's not out of place. > > FWIW I'm not opposed to changing how this api works if it ultimately > makes things better, but I want to wait until the cxlflash superpipe > support is merged so any patches that change the api can change it at > the same time. OK. I saw struct cxl_context in cxl.h and figured it was public, but it's in drivers/misc/cxl/cxl.h, so yes other drivers have no business poking in there, even though they *could*. So that's fine. cheers
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c index e0f0c78..5f0b22e 100644 --- a/drivers/misc/cxl/api.c +++ b/drivers/misc/cxl/api.c @@ -70,6 +70,27 @@ int cxl_release_context(struct cxl_context *ctx) } EXPORT_SYMBOL_GPL(cxl_release_context); + +int cxl_set_priv(struct cxl_context *ctx, void *priv) +{ + if (!ctx) + return -EINVAL; + + ctx->priv = priv; + + return 0; +} +EXPORT_SYMBOL_GPL(cxl_set_priv); + +void *cxl_get_priv(struct cxl_context *ctx) +{ + if (!ctx) + return ERR_PTR(-EINVAL); + + return ctx->priv; +} +EXPORT_SYMBOL_GPL(cxl_get_priv); + int cxl_allocate_afu_irqs(struct cxl_context *ctx, int num) { if (num == 0) diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h index 30e44a8..93db76a 100644 --- a/drivers/misc/cxl/cxl.h +++ b/drivers/misc/cxl/cxl.h @@ -431,6 +431,9 @@ struct cxl_context { /* Only used in PR mode */ u64 process_token; + /* driver private data */ + void *priv; + unsigned long *irq_bitmap; /* Accessed from IRQ context */ struct cxl_irq_ranges irqs; struct list_head irq_names; diff --git a/include/misc/cxl.h b/include/misc/cxl.h index 73e03a6..3f5edbe 100644 --- a/include/misc/cxl.h +++ b/include/misc/cxl.h @@ -89,6 +89,13 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev *dev); int cxl_release_context(struct cxl_context *ctx); /* + * Set and get private data associated with a context. Allows drivers to have a + * back pointer to some useful structure. + */ +int cxl_set_priv(struct cxl_context *ctx, void *priv); +void *cxl_get_priv(struct cxl_context *ctx); + +/* * Allocate AFU interrupts for this context. num=0 will allocate the default * for this AFU as given in the AFU descriptor. This number doesn't include the * interrupt 0 (CAIA defines AFU IRQ 0 for page faults). Each interrupt to be