* [PATCH 1/7] fpga: wrap the write_init() op
2021-06-07 17:23 [PATCH 0/7] fpga: wrappers for fpga_manager_ops trix
@ 2021-06-07 17:23 ` trix
2021-06-07 22:36 ` Moritz Fischer
2021-06-07 17:23 ` [PATCH 2/7] fpga: make write_complete() op optional trix
` (6 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: trix @ 2021-06-07 17:23 UTC (permalink / raw)
To: hao.wu, mdf, michal.simek
Cc: linux-fpga, linux-kernel, linux-arm-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
The board 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 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] fpga: wrap the write_init() op
2021-06-07 17:23 ` [PATCH 1/7] fpga: wrap the write_init() op trix
@ 2021-06-07 22:36 ` Moritz Fischer
2021-06-08 6:23 ` Martin Hundebøll
2021-06-08 13:55 ` Tom Rix
0 siblings, 2 replies; 13+ messages in thread
From: Moritz Fischer @ 2021-06-07 22:36 UTC (permalink / raw)
To: trix
Cc: hao.wu, mdf, michal.simek, linux-fpga, linux-kernel, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] fpga: wrap the write_init() op
2021-06-07 22:36 ` Moritz Fischer
@ 2021-06-08 6:23 ` Martin Hundebøll
2021-06-08 19:05 ` Tom Rix
2021-06-08 13:55 ` Tom Rix
1 sibling, 1 reply; 13+ messages in thread
From: Martin Hundebøll @ 2021-06-08 6:23 UTC (permalink / raw)
To: Moritz Fischer, trix
Cc: hao.wu, michal.simek, linux-fpga, linux-kernel, linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] fpga: wrap the write_init() op
2021-06-08 6:23 ` Martin Hundebøll
@ 2021-06-08 19:05 ` Tom Rix
0 siblings, 0 replies; 13+ messages in thread
From: Tom Rix @ 2021-06-08 19:05 UTC (permalink / raw)
To: Martin Hundebøll, Moritz Fischer
Cc: hao.wu, michal.simek, linux-fpga, linux-kernel, linux-arm-kernel
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
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/7] fpga: wrap the write_init() op
2021-06-07 22:36 ` Moritz Fischer
2021-06-08 6:23 ` Martin Hundebøll
@ 2021-06-08 13:55 ` Tom Rix
1 sibling, 0 replies; 13+ messages in thread
From: Tom Rix @ 2021-06-08 13:55 UTC (permalink / raw)
To: Moritz Fischer
Cc: hao.wu, michal.simek, linux-fpga, linux-kernel, linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/7] fpga: make write_complete() op optional
2021-06-07 17:23 [PATCH 0/7] fpga: wrappers for fpga_manager_ops trix
2021-06-07 17:23 ` [PATCH 1/7] fpga: wrap the write_init() op trix
@ 2021-06-07 17:23 ` trix
2021-06-07 17:23 ` [PATCH 3/7] fpga: wrap the write() op trix
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: trix @ 2021-06-07 17:23 UTC (permalink / raw)
To: hao.wu, mdf, michal.simek
Cc: linux-fpga, linux-kernel, linux-arm-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
The board should not be required to provide a
write_complete function if there is nothing. Move
the op check to the existing wrapper.
Default to success.
Remove noop function.
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/fpga/fpga-mgr.c | 7 ++++---
drivers/fpga/zynqmp-fpga.c | 7 -------
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 24547e36a56d..dadad2401502 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -152,10 +152,11 @@ static int fpga_mgr_write_init_sg(struct fpga_manager *mgr,
static int fpga_mgr_write_complete(struct fpga_manager *mgr,
struct fpga_image_info *info)
{
- int ret;
+ int ret = 0;
mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE;
- ret = mgr->mops->write_complete(mgr, info);
+ if (mgr->mops && mgr->mops->write_complete)
+ ret = mgr->mops->write_complete(mgr, info);
if (ret) {
dev_err(&mgr->dev, "Error after writing image data to FPGA\n");
mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE_ERR;
@@ -576,7 +577,7 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
struct fpga_manager *mgr;
int id, ret;
- if (!mops || !mops->write_complete || !mops->state ||
+ if (!mops || !mops->state ||
(!mops->write && !mops->write_sg) ||
(mops->write && mops->write_sg)) {
dev_err(dev, "Attempt to register without fpga_manager_ops\n");
diff --git a/drivers/fpga/zynqmp-fpga.c b/drivers/fpga/zynqmp-fpga.c
index 125743c9797f..9efbd70aa686 100644
--- a/drivers/fpga/zynqmp-fpga.c
+++ b/drivers/fpga/zynqmp-fpga.c
@@ -66,12 +66,6 @@ static int zynqmp_fpga_ops_write(struct fpga_manager *mgr,
return ret;
}
-static int zynqmp_fpga_ops_write_complete(struct fpga_manager *mgr,
- struct fpga_image_info *info)
-{
- return 0;
-}
-
static enum fpga_mgr_states zynqmp_fpga_ops_state(struct fpga_manager *mgr)
{
u32 status = 0;
@@ -87,7 +81,6 @@ static const struct fpga_manager_ops zynqmp_fpga_ops = {
.state = zynqmp_fpga_ops_state,
.write_init = zynqmp_fpga_ops_write_init,
.write = zynqmp_fpga_ops_write,
- .write_complete = zynqmp_fpga_ops_write_complete,
};
static int zynqmp_fpga_probe(struct platform_device *pdev)
--
2.26.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/7] fpga: wrap the write() op
2021-06-07 17:23 [PATCH 0/7] fpga: wrappers for fpga_manager_ops trix
2021-06-07 17:23 ` [PATCH 1/7] fpga: wrap the write_init() op trix
2021-06-07 17:23 ` [PATCH 2/7] fpga: make write_complete() op optional trix
@ 2021-06-07 17:23 ` trix
2021-06-07 17:23 ` [PATCH 4/7] fpga: wrap the status() op trix
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: trix @ 2021-06-07 17:23 UTC (permalink / raw)
To: hao.wu, mdf, michal.simek
Cc: linux-fpga, linux-kernel, linux-arm-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
The board should not be required to provide a
write function. Move the op check to the wrapper.
Default to -EOPNOTSUP so its users will fail
gracefully.
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/fpga/fpga-mgr.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index dadad2401502..c484b4309b2f 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -167,6 +167,13 @@ static int fpga_mgr_write_complete(struct fpga_manager *mgr,
return 0;
}
+static int fpga_mgr_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+ if (mgr->mops && mgr->mops->write)
+ return mgr->mops->write(mgr, buf, count);
+ return -EOPNOTSUPP;
+}
+
/**
* fpga_mgr_buf_load_sg - load fpga from image in buffer from a scatter list
* @mgr: fpga manager
@@ -203,7 +210,7 @@ static int fpga_mgr_buf_load_sg(struct fpga_manager *mgr,
sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
while (sg_miter_next(&miter)) {
- ret = mgr->mops->write(mgr, miter.addr, miter.length);
+ ret = fpga_mgr_write(mgr, miter.addr, miter.length);
if (ret)
break;
}
@@ -233,7 +240,7 @@ static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr,
* Write the FPGA image to the FPGA.
*/
mgr->state = FPGA_MGR_STATE_WRITE;
- ret = mgr->mops->write(mgr, buf, count);
+ ret = fpga_mgr_write(mgr, buf, count);
if (ret) {
dev_err(&mgr->dev, "Error while writing image data to FPGA\n");
mgr->state = FPGA_MGR_STATE_WRITE_ERR;
@@ -577,9 +584,7 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
struct fpga_manager *mgr;
int id, ret;
- if (!mops || !mops->state ||
- (!mops->write && !mops->write_sg) ||
- (mops->write && mops->write_sg)) {
+ if (!mops || !mops->state) {
dev_err(dev, "Attempt to register without fpga_manager_ops\n");
return NULL;
}
--
2.26.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/7] fpga: wrap the status() op
2021-06-07 17:23 [PATCH 0/7] fpga: wrappers for fpga_manager_ops trix
` (2 preceding siblings ...)
2021-06-07 17:23 ` [PATCH 3/7] fpga: wrap the write() op trix
@ 2021-06-07 17:23 ` trix
2021-06-07 17:24 ` [PATCH 5/7] fpga: wrap the state() op trix
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: trix @ 2021-06-07 17:23 UTC (permalink / raw)
To: hao.wu, mdf, michal.simek
Cc: linux-fpga, linux-kernel, linux-arm-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
The board is not required to provide a status() op.
Add a wrapper consistent with the other op wrappers.
Move the op check to the wrapper.
Default to 0, no errors to report.
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/fpga/fpga-mgr.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index c484b4309b2f..cc531f0537ac 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -426,6 +426,14 @@ static ssize_t state_show(struct device *dev,
return sprintf(buf, "%s\n", state_str[mgr->state]);
}
+static u64 fpga_mgr_status(struct fpga_manager *mgr)
+{
+ if (mgr->mops && mgr->mops->status)
+ return mgr->mops->status(mgr);
+
+ return 0;
+}
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -433,10 +441,7 @@ static ssize_t status_show(struct device *dev,
u64 status;
int len = 0;
- if (!mgr->mops->status)
- return -ENOENT;
-
- status = mgr->mops->status(mgr);
+ status = fpga_mgr_status(mgr);
if (status & FPGA_MGR_STATUS_OPERATION_ERR)
len += sprintf(buf + len, "reconfig operation error\n");
--
2.26.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/7] fpga: wrap the state() op
2021-06-07 17:23 [PATCH 0/7] fpga: wrappers for fpga_manager_ops trix
` (3 preceding siblings ...)
2021-06-07 17:23 ` [PATCH 4/7] fpga: wrap the status() op trix
@ 2021-06-07 17:24 ` trix
2021-06-07 17:24 ` [PATCH 6/7] fpga: wrap the fpga_remove() op trix
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: trix @ 2021-06-07 17:24 UTC (permalink / raw)
To: hao.wu, mdf, michal.simek
Cc: linux-fpga, linux-kernel, linux-arm-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
The board should not be required to provide a state() op.
Add a wrapper consistent with the other op wrappers.
Move op check to wrapper.
Default to FPGA_MGR_STATE_UNKNOWN, what noop state() uses.
Remove unneeded noop state() ops
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/fpga/dfl-fme-mgr.c | 6 ------
drivers/fpga/fpga-mgr.c | 11 +++++++++--
drivers/fpga/stratix10-soc.c | 6 ------
drivers/fpga/ts73xx-fpga.c | 6 ------
4 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
index d5861d13b306..313420405d5e 100644
--- a/drivers/fpga/dfl-fme-mgr.c
+++ b/drivers/fpga/dfl-fme-mgr.c
@@ -252,11 +252,6 @@ static int fme_mgr_write_complete(struct fpga_manager *mgr,
return 0;
}
-static enum fpga_mgr_states fme_mgr_state(struct fpga_manager *mgr)
-{
- return FPGA_MGR_STATE_UNKNOWN;
-}
-
static u64 fme_mgr_status(struct fpga_manager *mgr)
{
struct fme_mgr_priv *priv = mgr->priv;
@@ -268,7 +263,6 @@ static const struct fpga_manager_ops fme_mgr_ops = {
.write_init = fme_mgr_write_init,
.write = fme_mgr_write,
.write_complete = fme_mgr_write_complete,
- .state = fme_mgr_state,
.status = fme_mgr_status,
};
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index cc531f0537ac..d06752be9c6e 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -589,7 +589,7 @@ struct fpga_manager *fpga_mgr_create(struct device *dev, const char *name,
struct fpga_manager *mgr;
int id, ret;
- if (!mops || !mops->state) {
+ if (!mops) {
dev_err(dev, "Attempt to register without fpga_manager_ops\n");
return NULL;
}
@@ -692,6 +692,13 @@ struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
}
EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
+static enum fpga_mgr_states fpga_mgr_state(struct fpga_manager *mgr)
+{
+ if (mgr->mops && mgr->mops->state)
+ return mgr->mops->state(mgr);
+ return FPGA_MGR_STATE_UNKNOWN;
+}
+
/**
* fpga_mgr_register - register a FPGA manager
* @mgr: fpga manager struct
@@ -707,7 +714,7 @@ int fpga_mgr_register(struct fpga_manager *mgr)
* from device. FPGA may be in reset mode or may have been programmed
* by bootloader or EEPROM.
*/
- mgr->state = mgr->mops->state(mgr);
+ mgr->state = fpga_mgr_state(mgr);
ret = device_add(&mgr->dev);
if (ret)
diff --git a/drivers/fpga/stratix10-soc.c b/drivers/fpga/stratix10-soc.c
index 657a70c5fc99..5219fa1b555a 100644
--- a/drivers/fpga/stratix10-soc.c
+++ b/drivers/fpga/stratix10-soc.c
@@ -388,13 +388,7 @@ static int s10_ops_write_complete(struct fpga_manager *mgr,
return ret;
}
-static enum fpga_mgr_states s10_ops_state(struct fpga_manager *mgr)
-{
- return FPGA_MGR_STATE_UNKNOWN;
-}
-
static const struct fpga_manager_ops s10_ops = {
- .state = s10_ops_state,
.write_init = s10_ops_write_init,
.write = s10_ops_write,
.write_complete = s10_ops_write_complete,
diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
index 101f016c6ed8..167abb0b08d4 100644
--- a/drivers/fpga/ts73xx-fpga.c
+++ b/drivers/fpga/ts73xx-fpga.c
@@ -32,11 +32,6 @@ struct ts73xx_fpga_priv {
struct device *dev;
};
-static enum fpga_mgr_states ts73xx_fpga_state(struct fpga_manager *mgr)
-{
- return FPGA_MGR_STATE_UNKNOWN;
-}
-
static int ts73xx_fpga_write_init(struct fpga_manager *mgr,
struct fpga_image_info *info,
const char *buf, size_t count)
@@ -98,7 +93,6 @@ static int ts73xx_fpga_write_complete(struct fpga_manager *mgr,
}
static const struct fpga_manager_ops ts73xx_fpga_ops = {
- .state = ts73xx_fpga_state,
.write_init = ts73xx_fpga_write_init,
.write = ts73xx_fpga_write,
.write_complete = ts73xx_fpga_write_complete,
--
2.26.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/7] fpga: wrap the fpga_remove() op
2021-06-07 17:23 [PATCH 0/7] fpga: wrappers for fpga_manager_ops trix
` (4 preceding siblings ...)
2021-06-07 17:24 ` [PATCH 5/7] fpga: wrap the state() op trix
@ 2021-06-07 17:24 ` trix
2021-06-07 17:24 ` [PATCH 7/7] fpga: collect wrappers and change to inline trix
2021-06-07 21:59 ` [PATCH 0/7] fpga: wrappers for fpga_manager_ops Moritz Fischer
7 siblings, 0 replies; 13+ messages in thread
From: trix @ 2021-06-07 17:24 UTC (permalink / raw)
To: hao.wu, mdf, michal.simek
Cc: linux-fpga, linux-kernel, linux-arm-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
The board is not required to provide a fpga_remove() op.
Add a wrapper consistent with the other op wrappers.
Move op check to wrapper.
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/fpga/fpga-mgr.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index d06752be9c6e..84808c7ca440 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -731,6 +731,12 @@ int fpga_mgr_register(struct fpga_manager *mgr)
}
EXPORT_SYMBOL_GPL(fpga_mgr_register);
+static void fpga_mgr_fpga_remove(struct fpga_manager *mgr)
+{
+ if (mgr->mops && mgr->mops->fpga_remove)
+ mgr->mops->fpga_remove(mgr);
+}
+
/**
* fpga_mgr_unregister - unregister a FPGA manager
* @mgr: fpga manager struct
@@ -745,8 +751,7 @@ void fpga_mgr_unregister(struct fpga_manager *mgr)
* If the low level driver provides a method for putting fpga into
* a desired state upon unregister, do it.
*/
- if (mgr->mops->fpga_remove)
- mgr->mops->fpga_remove(mgr);
+ fpga_mgr_fpga_remove(mgr);
device_unregister(&mgr->dev);
}
--
2.26.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 7/7] fpga: collect wrappers and change to inline
2021-06-07 17:23 [PATCH 0/7] fpga: wrappers for fpga_manager_ops trix
` (5 preceding siblings ...)
2021-06-07 17:24 ` [PATCH 6/7] fpga: wrap the fpga_remove() op trix
@ 2021-06-07 17:24 ` trix
2021-06-07 21:59 ` [PATCH 0/7] fpga: wrappers for fpga_manager_ops Moritz Fischer
7 siblings, 0 replies; 13+ messages in thread
From: trix @ 2021-06-07 17:24 UTC (permalink / raw)
To: hao.wu, mdf, michal.simek
Cc: linux-fpga, linux-kernel, linux-arm-kernel, Tom Rix
From: Tom Rix <trix@redhat.com>
Anyone searching for the wrappers should find all of
them together, so move the wrappers.
Since they are all small functions, make them inline.
Signed-off-by: Tom Rix <trix@redhat.com>
---
drivers/fpga/fpga-mgr.c | 117 ++++++++++++++++++++--------------------
1 file changed, 59 insertions(+), 58 deletions(-)
diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
index 84808c7ca440..198a44a62058 100644
--- a/drivers/fpga/fpga-mgr.c
+++ b/drivers/fpga/fpga-mgr.c
@@ -25,6 +25,65 @@ struct fpga_mgr_devres {
struct fpga_manager *mgr;
};
+/* mops wrappers */
+static inline enum fpga_mgr_states fpga_mgr_state(struct fpga_manager *mgr)
+{
+ if (mgr->mops && mgr->mops->state)
+ return mgr->mops->state(mgr);
+ return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static inline u64 fpga_mgr_status(struct fpga_manager *mgr)
+{
+ if (mgr->mops && mgr->mops->status)
+ return mgr->mops->status(mgr);
+ return 0;
+}
+
+static inline 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;
+}
+
+static inline int fpga_mgr_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+ if (mgr->mops && mgr->mops->write)
+ return mgr->mops->write(mgr, buf, count);
+ return -EOPNOTSUPP;
+}
+
+/*
+ * After all the FPGA image has been written, do the device specific steps to
+ * finish and set the FPGA into operating mode.
+ */
+static inline int fpga_mgr_write_complete(struct fpga_manager *mgr,
+ struct fpga_image_info *info)
+{
+ int ret = 0;
+
+ mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE;
+ if (mgr->mops && mgr->mops->write_complete)
+ ret = mgr->mops->write_complete(mgr, info);
+ if (ret) {
+ dev_err(&mgr->dev, "Error after writing image data to FPGA\n");
+ mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE_ERR;
+ return ret;
+ }
+ mgr->state = FPGA_MGR_STATE_OPERATING;
+
+ return 0;
+}
+
+static inline void fpga_mgr_fpga_remove(struct fpga_manager *mgr)
+{
+ if (mgr->mops && mgr->mops->fpga_remove)
+ mgr->mops->fpga_remove(mgr);
+}
+
/**
* fpga_image_info_alloc - Allocate a FPGA image info struct
* @dev: owning device
@@ -69,14 +128,6 @@ 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
@@ -145,35 +196,6 @@ static int fpga_mgr_write_init_sg(struct fpga_manager *mgr,
return ret;
}
-/*
- * After all the FPGA image has been written, do the device specific steps to
- * finish and set the FPGA into operating mode.
- */
-static int fpga_mgr_write_complete(struct fpga_manager *mgr,
- struct fpga_image_info *info)
-{
- int ret = 0;
-
- mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE;
- if (mgr->mops && mgr->mops->write_complete)
- ret = mgr->mops->write_complete(mgr, info);
- if (ret) {
- dev_err(&mgr->dev, "Error after writing image data to FPGA\n");
- mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE_ERR;
- return ret;
- }
- mgr->state = FPGA_MGR_STATE_OPERATING;
-
- return 0;
-}
-
-static int fpga_mgr_write(struct fpga_manager *mgr, const char *buf, size_t count)
-{
- if (mgr->mops && mgr->mops->write)
- return mgr->mops->write(mgr, buf, count);
- return -EOPNOTSUPP;
-}
-
/**
* fpga_mgr_buf_load_sg - load fpga from image in buffer from a scatter list
* @mgr: fpga manager
@@ -426,14 +448,6 @@ static ssize_t state_show(struct device *dev,
return sprintf(buf, "%s\n", state_str[mgr->state]);
}
-static u64 fpga_mgr_status(struct fpga_manager *mgr)
-{
- if (mgr->mops && mgr->mops->status)
- return mgr->mops->status(mgr);
-
- return 0;
-}
-
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -692,13 +706,6 @@ struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name,
}
EXPORT_SYMBOL_GPL(devm_fpga_mgr_create);
-static enum fpga_mgr_states fpga_mgr_state(struct fpga_manager *mgr)
-{
- if (mgr->mops && mgr->mops->state)
- return mgr->mops->state(mgr);
- return FPGA_MGR_STATE_UNKNOWN;
-}
-
/**
* fpga_mgr_register - register a FPGA manager
* @mgr: fpga manager struct
@@ -731,12 +738,6 @@ int fpga_mgr_register(struct fpga_manager *mgr)
}
EXPORT_SYMBOL_GPL(fpga_mgr_register);
-static void fpga_mgr_fpga_remove(struct fpga_manager *mgr)
-{
- if (mgr->mops && mgr->mops->fpga_remove)
- mgr->mops->fpga_remove(mgr);
-}
-
/**
* fpga_mgr_unregister - unregister a FPGA manager
* @mgr: fpga manager struct
--
2.26.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/7] fpga: wrappers for fpga_manager_ops
2021-06-07 17:23 [PATCH 0/7] fpga: wrappers for fpga_manager_ops trix
` (6 preceding siblings ...)
2021-06-07 17:24 ` [PATCH 7/7] fpga: collect wrappers and change to inline trix
@ 2021-06-07 21:59 ` Moritz Fischer
7 siblings, 0 replies; 13+ messages in thread
From: Moritz Fischer @ 2021-06-07 21:59 UTC (permalink / raw)
To: trix
Cc: hao.wu, mdf, michal.simek, linux-fpga, linux-kernel, linux-arm-kernel
Hi Tom,
On Mon, Jun 07, 2021 at 10:23:55AM -0700, trix@redhat.com wrote:
> From: Tom Rix <trix@redhat.com>
>
> As followup from
> https://lore.kernel.org/linux-fpga/06301910-10a1-0e62-45a0-d28ab5a787ed@redhat.com/
>
> Boards should not be required to have noop functions.
> So improve or create fpga-mgr wrappers for the fpga_manager_ops.
> Remove the noop functions.
> Refactor fpga-mgr to use the wrappers.
>
> write_sg op was not wrapped on purpose. Its checking / use in
> fpga_mgr_buf_load_sg() did not warrant a wrapper.
>
> Tom Rix (7):
> fpga: wrap the write_init() op
> fpga: make write_complete() op optional
> fpga: wrap the write() op
> fpga: wrap the status() op
> fpga: wrap the state() op
> fpga: wrap the fpga_remove() op
> fpga: collect wrappers and change to inline
>
> drivers/fpga/dfl-fme-mgr.c | 6 ---
> drivers/fpga/fpga-mgr.c | 102 +++++++++++++++++++++++------------
> drivers/fpga/stratix10-soc.c | 6 ---
> drivers/fpga/ts73xx-fpga.c | 6 ---
> drivers/fpga/zynqmp-fpga.c | 7 ---
> 5 files changed, 67 insertions(+), 60 deletions(-)
>
> --
> 2.26.3
>
Thanks for doing this, will take a look tonight!
- Moritz
^ permalink raw reply [flat|nested] 13+ messages in thread