From: Tom Rix <trix@redhat.com> To: Xu Yilun <yilun.xu@intel.com>, Russ Weight <russell.h.weight@intel.com>, Greg KH <gregkh@linuxfoundation.org> Cc: hao.wu@intel.com, mdf@kernel.org, michal.simek@xilinx.com, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 1/7] fpga-mgr: wrap the write_init() op Date: Thu, 24 Jun 2021 07:37:13 -0700 [thread overview] Message-ID: <02bd1acf-6b8d-7ca1-6a9c-c3f6d3a2071c@redhat.com> (raw) In-Reply-To: <20210624075414.GA44700@yilunxu-OptiPlex-7050> On 6/24/21 12:54 AM, Xu Yilun wrote: > On Wed, Jun 23, 2021 at 11:24:04AM -0700, trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> An FPGA manager should not be required to provide a >> 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 ecb4c3c795fa5..87bbb940c9504 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) > Maybe we don't have to check mgr->mops, it is already checked on > creation. The check was on purpose because my earlier patchset responding to problems with sec-mgr. Focusing on Greg's comment that why can't sec-mgr be done with existing code, I think the sec-mgr can be folded into the exiting fpga-mgr via a new set of ops. the 'generalize fpga_mgr_load' patchset set has two sets of ops, one for existing partial reconfiguration and one for reimaging the whole board, what the sec-mgr is doing. Since dfl has the only instance of need, it would have the only reimage ops. The check at creation has been deferred to at use. other targets could have null ops. Having maybe null ops means the wrappers need to check. Here is a ref to the earlier patchset https://lore.kernel.org/linux-fpga/20210524162721.2220782-1-trix@redhat.com/ I'll respin 'generalize fpga_mgr_load' within the context this patchset to give you some more context. It will test is the check is needed and give folks a chance to comment if this a way sec-mgr should go. Tom > > The same concern to all the following patches. > > Thanks, > Yilun > >> + 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 *parent, 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(parent, "Attempt to register without fpga_manager_ops\n"); >> return NULL; >> -- >> 2.26.3
WARNING: multiple messages have this Message-ID (diff)
From: Tom Rix <trix@redhat.com> To: Xu Yilun <yilun.xu@intel.com>, Russ Weight <russell.h.weight@intel.com>, Greg KH <gregkh@linuxfoundation.org> Cc: hao.wu@intel.com, mdf@kernel.org, michal.simek@xilinx.com, linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 1/7] fpga-mgr: wrap the write_init() op Date: Thu, 24 Jun 2021 07:37:13 -0700 [thread overview] Message-ID: <02bd1acf-6b8d-7ca1-6a9c-c3f6d3a2071c@redhat.com> (raw) In-Reply-To: <20210624075414.GA44700@yilunxu-OptiPlex-7050> On 6/24/21 12:54 AM, Xu Yilun wrote: > On Wed, Jun 23, 2021 at 11:24:04AM -0700, trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> An FPGA manager should not be required to provide a >> 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 ecb4c3c795fa5..87bbb940c9504 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) > Maybe we don't have to check mgr->mops, it is already checked on > creation. The check was on purpose because my earlier patchset responding to problems with sec-mgr. Focusing on Greg's comment that why can't sec-mgr be done with existing code, I think the sec-mgr can be folded into the exiting fpga-mgr via a new set of ops. the 'generalize fpga_mgr_load' patchset set has two sets of ops, one for existing partial reconfiguration and one for reimaging the whole board, what the sec-mgr is doing. Since dfl has the only instance of need, it would have the only reimage ops. The check at creation has been deferred to at use. other targets could have null ops. Having maybe null ops means the wrappers need to check. Here is a ref to the earlier patchset https://lore.kernel.org/linux-fpga/20210524162721.2220782-1-trix@redhat.com/ I'll respin 'generalize fpga_mgr_load' within the context this patchset to give you some more context. It will test is the check is needed and give folks a chance to comment if this a way sec-mgr should go. Tom > > The same concern to all the following patches. > > Thanks, > Yilun > >> + 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 *parent, 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(parent, "Attempt to register without fpga_manager_ops\n"); >> return NULL; >> -- >> 2.26.3 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-06-24 14:37 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-23 18:24 [PATCH v3 0/7] wrappers for fpga_manager_ops trix 2021-06-23 18:24 ` trix 2021-06-23 18:24 ` [PATCH v3 1/7] fpga-mgr: wrap the write_init() op trix 2021-06-23 18:24 ` trix 2021-06-24 7:54 ` Xu Yilun 2021-06-24 7:54 ` Xu Yilun 2021-06-24 14:37 ` Tom Rix [this message] 2021-06-24 14:37 ` Tom Rix 2021-06-23 18:24 ` [PATCH v3 2/7] fpga-mgr: make write_complete() op optional trix 2021-06-23 18:24 ` trix 2021-06-23 18:24 ` [PATCH v3 3/7] fpga-mgr: wrap the write() op trix 2021-06-23 18:24 ` trix 2021-06-23 18:24 ` [PATCH v3 4/7] fpga-mgr: wrap the status() op trix 2021-06-23 18:24 ` trix 2021-06-23 18:24 ` [PATCH v3 5/7] fpga-mgr: wrap the state() op trix 2021-06-23 18:24 ` trix 2021-06-23 18:24 ` [PATCH v3 6/7] fpga-mgr: wrap the fpga_remove() op trix 2021-06-23 18:24 ` trix 2021-06-23 18:24 ` [PATCH v3 7/7] fpga-mgr: collect wrappers and change to inline trix 2021-06-23 18:24 ` trix
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=02bd1acf-6b8d-7ca1-6a9c-c3f6d3a2071c@redhat.com \ --to=trix@redhat.com \ --cc=gregkh@linuxfoundation.org \ --cc=hao.wu@intel.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-fpga@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mdf@kernel.org \ --cc=michal.simek@xilinx.com \ --cc=russell.h.weight@intel.com \ --cc=yilun.xu@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.