| Message ID | 20210607172402.2938697-2-trix@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fpga: wrappers for fpga_manager_ops | expand |
On Mon, Jun 07, 2021 at 10:23:56AM -0700, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > The board should not be required to provide a Nit: Can you turn these into for whole series: A FPGA Manager should not be ... > write_init() op if there is nothing for it do. > So add a wrapper and move the op checking. > Default to success. > > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/fpga/fpga-mgr.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c > index b85bc47c91a9..24547e36a56d 100644 > --- a/drivers/fpga/fpga-mgr.c > +++ b/drivers/fpga/fpga-mgr.c > @@ -69,6 +69,14 @@ void fpga_image_info_free(struct fpga_image_info *info) > } > EXPORT_SYMBOL_GPL(fpga_image_info_free); > > +static int fpga_mgr_write_init(struct fpga_manager *mgr, > + struct fpga_image_info *info, > + const char *buf, size_t count) > +{ > + if (mgr->mops && mgr->mops->write_init) > + return mgr->mops->write_init(mgr, info, buf, count); > + return 0; > +} > /* > * Call the low level driver's write_init function. This will do the > * device-specific things to get the FPGA into the state where it is ready to > @@ -83,9 +91,9 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr, > > mgr->state = FPGA_MGR_STATE_WRITE_INIT; > if (!mgr->mops->initial_header_size) > - ret = mgr->mops->write_init(mgr, info, NULL, 0); > + ret = fpga_mgr_write_init(mgr, info, NULL, 0); > else > - ret = mgr->mops->write_init( > + ret = fpga_mgr_write_init( > mgr, info, buf, min(mgr->mops->initial_header_size, count)); > > if (ret) { > @@ -569,7 +577,7 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, > int id, ret; > > if (!mops || !mops->write_complete || !mops->state || > - !mops->write_init || (!mops->write && !mops->write_sg) || > + (!mops->write && !mops->write_sg) || > (mops->write && mops->write_sg)) { > dev_err(dev, "Attempt to register without fpga_manager_ops\n"); > return NULL; > -- > 2.26.3 > Can you change the subjects to "fpga: fpga-mgr: ..." Otherwise series looks good. - Moritz
On 08/06/2021 00.36, Moritz Fischer wrote: > On Mon, Jun 07, 2021 at 10:23:56AM -0700, trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> The board should not be required to provide a > Nit: Can you turn these into for whole series: > A FPGA Manager should not be ... Nit nit: should be: An FPGA Manager should not be ... // Martin > >> write_init() op if there is nothing for it do. >> So add a wrapper and move the op checking. >> Default to success. >> >> Signed-off-by: Tom Rix <trix@redhat.com> >> --- >> drivers/fpga/fpga-mgr.c | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c >> index b85bc47c91a9..24547e36a56d 100644 >> --- a/drivers/fpga/fpga-mgr.c >> +++ b/drivers/fpga/fpga-mgr.c >> @@ -69,6 +69,14 @@ void fpga_image_info_free(struct fpga_image_info *info) >> } >> EXPORT_SYMBOL_GPL(fpga_image_info_free); >> >> +static int fpga_mgr_write_init(struct fpga_manager *mgr, >> + struct fpga_image_info *info, >> + const char *buf, size_t count) >> +{ >> + if (mgr->mops && mgr->mops->write_init) >> + return mgr->mops->write_init(mgr, info, buf, count); >> + return 0; >> +} >> /* >> * Call the low level driver's write_init function. This will do the >> * device-specific things to get the FPGA into the state where it is ready to >> @@ -83,9 +91,9 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr, >> >> mgr->state = FPGA_MGR_STATE_WRITE_INIT; >> if (!mgr->mops->initial_header_size) >> - ret = mgr->mops->write_init(mgr, info, NULL, 0); >> + ret = fpga_mgr_write_init(mgr, info, NULL, 0); >> else >> - ret = mgr->mops->write_init( >> + ret = fpga_mgr_write_init( >> mgr, info, buf, min(mgr->mops->initial_header_size, count)); >> >> if (ret) { >> @@ -569,7 +577,7 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, >> int id, ret; >> >> if (!mops || !mops->write_complete || !mops->state || >> - !mops->write_init || (!mops->write && !mops->write_sg) || >> + (!mops->write && !mops->write_sg) || >> (mops->write && mops->write_sg)) { >> dev_err(dev, "Attempt to register without fpga_manager_ops\n"); >> return NULL; >> -- >> 2.26.3 >> > > Can you change the subjects to "fpga: fpga-mgr: ..." > > Otherwise series looks good. > > - Moritz >
On 6/7/21 3:36 PM, Moritz Fischer wrote: > On Mon, Jun 07, 2021 at 10:23:56AM -0700, trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> The board should not be required to provide a > Nit: Can you turn these into for whole series: > A FPGA Manager should not be ... ok > >> write_init() op if there is nothing for it do. >> So add a wrapper and move the op checking. >> Default to success. >> >> Signed-off-by: Tom Rix <trix@redhat.com> >> --- >> drivers/fpga/fpga-mgr.c | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c >> index b85bc47c91a9..24547e36a56d 100644 >> --- a/drivers/fpga/fpga-mgr.c >> +++ b/drivers/fpga/fpga-mgr.c >> @@ -69,6 +69,14 @@ void fpga_image_info_free(struct fpga_image_info *info) >> } >> EXPORT_SYMBOL_GPL(fpga_image_info_free); >> >> +static int fpga_mgr_write_init(struct fpga_manager *mgr, >> + struct fpga_image_info *info, >> + const char *buf, size_t count) >> +{ >> + if (mgr->mops && mgr->mops->write_init) >> + return mgr->mops->write_init(mgr, info, buf, count); >> + return 0; >> +} >> /* >> * Call the low level driver's write_init function. This will do the >> * device-specific things to get the FPGA into the state where it is ready to >> @@ -83,9 +91,9 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr, >> >> mgr->state = FPGA_MGR_STATE_WRITE_INIT; >> if (!mgr->mops->initial_header_size) >> - ret = mgr->mops->write_init(mgr, info, NULL, 0); >> + ret = fpga_mgr_write_init(mgr, info, NULL, 0); >> else >> - ret = mgr->mops->write_init( >> + ret = fpga_mgr_write_init( >> mgr, info, buf, min(mgr->mops->initial_header_size, count)); >> >> if (ret) { >> @@ -569,7 +577,7 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, >> int id, ret; >> >> if (!mops || !mops->write_complete || !mops->state || >> - !mops->write_init || (!mops->write && !mops->write_sg) || >> + (!mops->write && !mops->write_sg) || >> (mops->write && mops->write_sg)) { >> dev_err(dev, "Attempt to register without fpga_manager_ops\n"); >> return NULL; >> -- >> 2.26.3 >> > Can you change the subjects to "fpga: fpga-mgr: ..." ok I know this varies widely, but.. each 'bla:' is a subdir bla/ In the next patchset to reorganize around a subdir structure, there are a few infrastructure files that i think could go into a fpga/fpga-mgr/ fpga-bridge.c fpga-mgr.c fpga-region.c of-fpga-region.c These are the only unmoved files in the patchset. I was not sure about moving them so I left them alone. Tom > > Otherwise series looks good. > > - Moritz >
On 6/7/21 11:23 PM, Martin Hundebøll wrote: > > > On 08/06/2021 00.36, Moritz Fischer wrote: >> On Mon, Jun 07, 2021 at 10:23:56AM -0700, trix@redhat.com wrote: >>> From: Tom Rix <trix@redhat.com> >>> >>> The board should not be required to provide a >> Nit: Can you turn these into for whole series: >> A FPGA Manager should not be ... > > Nit nit: should be: > An FPGA Manager should not be ... > > // Martin ok. I went down a rabbit hole on this one, looks fine. Tom > >> >>> write_init() op if there is nothing for it do. >>> So add a wrapper and move the op checking. >>> Default to success. >>> >>> Signed-off-by: Tom Rix <trix@redhat.com> >>> --- >>> drivers/fpga/fpga-mgr.c | 14 +++++++++++--- >>> 1 file changed, 11 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c >>> index b85bc47c91a9..24547e36a56d 100644 >>> --- a/drivers/fpga/fpga-mgr.c >>> +++ b/drivers/fpga/fpga-mgr.c >>> @@ -69,6 +69,14 @@ void fpga_image_info_free(struct fpga_image_info >>> *info) >>> } >>> EXPORT_SYMBOL_GPL(fpga_image_info_free); >>> +static int fpga_mgr_write_init(struct fpga_manager *mgr, >>> + struct fpga_image_info *info, >>> + const char *buf, size_t count) >>> +{ >>> + if (mgr->mops && mgr->mops->write_init) >>> + return mgr->mops->write_init(mgr, info, buf, count); >>> + return 0; >>> +} >>> /* >>> * Call the low level driver's write_init function. This will do the >>> * device-specific things to get the FPGA into the state where it >>> is ready to >>> @@ -83,9 +91,9 @@ static int fpga_mgr_write_init_buf(struct >>> fpga_manager *mgr, >>> mgr->state = FPGA_MGR_STATE_WRITE_INIT; >>> if (!mgr->mops->initial_header_size) >>> - ret = mgr->mops->write_init(mgr, info, NULL, 0); >>> + ret = fpga_mgr_write_init(mgr, info, NULL, 0); >>> else >>> - ret = mgr->mops->write_init( >>> + ret = fpga_mgr_write_init( >>> mgr, info, buf, min(mgr->mops->initial_header_size, >>> count)); >>> if (ret) { >>> @@ -569,7 +577,7 @@ struct fpga_manager *fpga_mgr_create(struct >>> device *dev, const char *name, >>> int id, ret; >>> if (!mops || !mops->write_complete || !mops->state || >>> - !mops->write_init || (!mops->write && !mops->write_sg) || >>> + (!mops->write && !mops->write_sg) || >>> (mops->write && mops->write_sg)) { >>> dev_err(dev, "Attempt to register without >>> fpga_manager_ops\n"); >>> return NULL; >>> -- >>> 2.26.3 >>> >> >> Can you change the subjects to "fpga: fpga-mgr: ..." >> >> Otherwise series looks good. >> >> - Moritz >> >
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index b85bc47c91a9..24547e36a56d 100644 --- a/drivers/fpga/fpga-mgr.c +++ b/drivers/fpga/fpga-mgr.c @@ -69,6 +69,14 @@ void fpga_image_info_free(struct fpga_image_info *info) } EXPORT_SYMBOL_GPL(fpga_image_info_free); +static int fpga_mgr_write_init(struct fpga_manager *mgr, + struct fpga_image_info *info, + const char *buf, size_t count) +{ + if (mgr->mops && mgr->mops->write_init) + return mgr->mops->write_init(mgr, info, buf, count); + return 0; +} /* * Call the low level driver's write_init function. This will do the * device-specific things to get the FPGA into the state where it is ready to @@ -83,9 +91,9 @@ static int fpga_mgr_write_init_buf(struct fpga_manager *mgr, mgr->state = FPGA_MGR_STATE_WRITE_INIT; if (!mgr->mops->initial_header_size) - ret = mgr->mops->write_init(mgr, info, NULL, 0); + ret = fpga_mgr_write_init(mgr, info, NULL, 0); else - ret = mgr->mops->write_init( + ret = fpga_mgr_write_init( mgr, info, buf, min(mgr->mops->initial_header_size, count)); if (ret) { @@ -569,7 +577,7 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name, int id, ret; if (!mops || !mops->write_complete || !mops->state || - !mops->write_init || (!mops->write && !mops->write_sg) || + (!mops->write && !mops->write_sg) || (mops->write && mops->write_sg)) { dev_err(dev, "Attempt to register without fpga_manager_ops\n"); return NULL;