[PATCH v1] uio: dfl: add IOPLL user-clock feature id

[PATCH v1] uio: dfl: add IOPLL user-clock feature id

[Peter Colberg]

Add a Device Feature List (DFL) feature id for the configurable
IOPLL user clock source, which can be used to configure the clock
speeds that are used for RTL logic that is programmed into the
Partial Reconfiguration (PR) region of an FPGA.

The DFL feature id table can be found at:
https://github.com/OPAE/dfl-feature-id

Signed-off-by: Peter Colberg <peter.colberg@intel.com>
---
 drivers/uio/uio_dfl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
index 8f39cc8bb034..69e93f3e7faf 100644
--- a/drivers/uio/uio_dfl.c
+++ b/drivers/uio/uio_dfl.c
@@ -46,10 +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev)
 
 #define FME_FEATURE_ID_ETH_GROUP	0x10
 #define FME_FEATURE_ID_HSSI_SUBSYS	0x15
+#define PORT_FEATURE_ID_IOPLL_USRCLK	0x14
 
 static const struct dfl_device_id uio_dfl_ids[] = {
 	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
 	{ FME_ID, FME_FEATURE_ID_HSSI_SUBSYS },
+	{ PORT_ID, PORT_FEATURE_ID_IOPLL_USRCLK },
 	{ }
 };
 MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
-- 
2.28.0

Re: [PATCH v1] uio: dfl: add IOPLL user-clock feature id

[Xu Yilun]

On 2022-08-17 at 17:37:46 -0400, Peter Colberg wrote:
> Add a Device Feature List (DFL) feature id for the configurable
> IOPLL user clock source, which can be used to configure the clock
> speeds that are used for RTL logic that is programmed into the
> Partial Reconfiguration (PR) region of an FPGA.

Why not use linux clock framework for this IOPLL? And let the PR
driver set it togeter with the RTL logic reporgramming?

Thanks,
Yilun

> 
> The DFL feature id table can be found at:
> https://github.com/OPAE/dfl-feature-id
> 
> Signed-off-by: Peter Colberg <peter.colberg@intel.com>
> ---
>  drivers/uio/uio_dfl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
> index 8f39cc8bb034..69e93f3e7faf 100644
> --- a/drivers/uio/uio_dfl.c
> +++ b/drivers/uio/uio_dfl.c
> @@ -46,10 +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev)
>  
>  #define FME_FEATURE_ID_ETH_GROUP	0x10
>  #define FME_FEATURE_ID_HSSI_SUBSYS	0x15
> +#define PORT_FEATURE_ID_IOPLL_USRCLK	0x14
>  
>  static const struct dfl_device_id uio_dfl_ids[] = {
>  	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
>  	{ FME_ID, FME_FEATURE_ID_HSSI_SUBSYS },
> +	{ PORT_ID, PORT_FEATURE_ID_IOPLL_USRCLK },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
> -- 
> 2.28.0
> 

Re: [PATCH v1] uio: dfl: add IOPLL user-clock feature id

[Russ Weight]

On 8/17/22 21:18, Xu Yilun wrote:
> On 2022-08-17 at 17:37:46 -0400, Peter Colberg wrote:
>> Add a Device Feature List (DFL) feature id for the configurable
>> IOPLL user clock source, which can be used to configure the clock
>> speeds that are used for RTL logic that is programmed into the
>> Partial Reconfiguration (PR) region of an FPGA.
> Why not use linux clock framework for this IOPLL? And let the PR
> driver set it togeter with the RTL logic reporgramming?

Hi Yilun,

We previously explored the possibility of plugging into the linux
clock framework. For this device, setting a desired frequency is
heavily dependent on a table of values that must be programmed in
order to achieve the desired clock speeds.

Here is an example table, indexed by frequency. The first element
in each entry is the frequency in kHz:

https://github.com/OPAE/opae-sdk/blob/master/libraries/plugins/xfpga/usrclk/fpga_user_clk_freq.h

We previously experimented with a kernel-space driver. The
implementation exported a sysfs node into which the table values for
the desired frequency would be written in order to set the desired
frequency. The function of the driver was to execute the logic
required to program the device. We did not think this implementation
should be up-streamed.

It isn't practical to upstream the frequency tables as they are
subject to change for future devices. For example, if the reference
frequency changed in a future device, a whole new table of values would
have to be added for the new device. In a recent transition to a new
device, the range of frequencies was increased which required an
extension to an existing table.

A previous implementation of the user clock was also implemented in
user-space. The kernel driver exported each of the registers, but
all of the logic was implemented in user-space. The kernel portion
can be viewed here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/fpga/dfl-afu-main.c#n380

This is our reasoning in choosing to implement this driver in
user-space. Would you consider a uio based user-space driver to
be acceptable for in this case?

- Russ


>
> Thanks,
> Yilun
>
>> The DFL feature id table can be found at:
>> https://github.com/OPAE/dfl-feature-id
>>
>> Signed-off-by: Peter Colberg <peter.colberg@intel.com>
>> ---
>>  drivers/uio/uio_dfl.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
>> index 8f39cc8bb034..69e93f3e7faf 100644
>> --- a/drivers/uio/uio_dfl.c
>> +++ b/drivers/uio/uio_dfl.c
>> @@ -46,10 +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev)
>>  
>>  #define FME_FEATURE_ID_ETH_GROUP	0x10
>>  #define FME_FEATURE_ID_HSSI_SUBSYS	0x15
>> +#define PORT_FEATURE_ID_IOPLL_USRCLK	0x14
>>  
>>  static const struct dfl_device_id uio_dfl_ids[] = {
>>  	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
>>  	{ FME_ID, FME_FEATURE_ID_HSSI_SUBSYS },
>> +	{ PORT_ID, PORT_FEATURE_ID_IOPLL_USRCLK },
>>  	{ }
>>  };
>>  MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
>> -- 
>> 2.28.0
>>

Re: [PATCH v1] uio: dfl: add IOPLL user-clock feature id

[Xu Yilun]

On 2022-08-18 at 17:38:35 -0600, Russ Weight wrote:
> 
> 
> On 8/17/22 21:18, Xu Yilun wrote:
> > On 2022-08-17 at 17:37:46 -0400, Peter Colberg wrote:
> >> Add a Device Feature List (DFL) feature id for the configurable
> >> IOPLL user clock source, which can be used to configure the clock
> >> speeds that are used for RTL logic that is programmed into the
> >> Partial Reconfiguration (PR) region of an FPGA.
> > Why not use linux clock framework for this IOPLL? And let the PR
> > driver set it togeter with the RTL logic reporgramming?
> 
> Hi Yilun,
> 
> We previously explored the possibility of plugging into the linux
> clock framework. For this device, setting a desired frequency is
> heavily dependent on a table of values that must be programmed in
> order to achieve the desired clock speeds.
> 
> Here is an example table, indexed by frequency. The first element
> in each entry is the frequency in kHz:
> 
> https://github.com/OPAE/opae-sdk/blob/master/libraries/plugins/xfpga/usrclk/fpga_user_clk_freq.h
> 
> We previously experimented with a kernel-space driver. The
> implementation exported a sysfs node into which the table values for
> the desired frequency would be written in order to set the desired
> frequency. The function of the driver was to execute the logic
> required to program the device. We did not think this implementation
> should be up-streamed.
> 
> It isn't practical to upstream the frequency tables as they are
> subject to change for future devices. For example, if the reference
> frequency changed in a future device, a whole new table of values would
> have to be added for the new device. In a recent transition to a new
> device, the range of frequencies was increased which required an
> extension to an existing table.

Making a table for the inputs & outputs is always a easier way to get
things done, but the trade off is, as you said, extension to the table
every time for new outputs.

So do we really need all parameters to be in a table, or these are
actually the outcome of some calculation? Is it possible just
Implementing the calculation.

If I remember correctly, linux clk framework enables a generic clk
caculation mechanism. It encourages people to model the internal
refclk, plls (and deviders?) separately and construct the clk tree.
Then the specified calculation could be simpler for each clk driver.

I'm not sure the clk framework fits all your need, but please
investigate it firstly.

> 
> A previous implementation of the user clock was also implemented in
> user-space. The kernel driver exported each of the registers, but
> all of the logic was implemented in user-space. The kernel portion
> can be viewed here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/fpga/dfl-afu-main.c#n380
> 
> This is our reasoning in choosing to implement this driver in
> user-space. Would you consider a uio based user-space driver to
> be acceptable for in this case?

As usual, we firstly make clear why existing framework cannot fit the
case and should be implemented in userspace, then everything would be
OK.

Thanks,
Yilun

> 
> - Russ
> 
> 
> >
> > Thanks,
> > Yilun
> >
> >> The DFL feature id table can be found at:
> >> https://github.com/OPAE/dfl-feature-id
> >>
> >> Signed-off-by: Peter Colberg <peter.colberg@intel.com>
> >> ---
> >>  drivers/uio/uio_dfl.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
> >> index 8f39cc8bb034..69e93f3e7faf 100644
> >> --- a/drivers/uio/uio_dfl.c
> >> +++ b/drivers/uio/uio_dfl.c
> >> @@ -46,10 +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev)
> >>  
> >>  #define FME_FEATURE_ID_ETH_GROUP	0x10
> >>  #define FME_FEATURE_ID_HSSI_SUBSYS	0x15
> >> +#define PORT_FEATURE_ID_IOPLL_USRCLK	0x14
> >>  
> >>  static const struct dfl_device_id uio_dfl_ids[] = {
> >>  	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
> >>  	{ FME_ID, FME_FEATURE_ID_HSSI_SUBSYS },
> >> +	{ PORT_ID, PORT_FEATURE_ID_IOPLL_USRCLK },
> >>  	{ }
> >>  };
> >>  MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
> >> -- 
> >> 2.28.0
> >>
> 

Re: [PATCH v1] uio: dfl: add IOPLL user-clock feature id

[Russ Weight]

On 8/21/22 21:49, Xu Yilun wrote:
> On 2022-08-18 at 17:38:35 -0600, Russ Weight wrote: >> >> >> On 8/17/22 21:18, Xu Yilun wrote: >>> On 2022-08-17 at 17:37:46 -0400, Peter Colberg wrote: >>>> Add a Device Feature List (DFL) feature id for the >>>> configurable IOPLL user clock source, which can be used to >>>> configure the clock speeds that are used for RTL logic that is >>>> programmed into the Partial Reconfiguration (PR) region of an >>>> FPGA. >>> Why not use linux clock framework for this IOPLL? And let the PR >>> driver set it togeter with the RTL logic reporgramming? >> >> Hi Yilun, >> >> We previously explored the possibility of plugging into the linux >> clock framework. For this device, setting a desired frequency is >> heavily dependent on a table of values that must be programmed in >> order to achieve the desired clock speeds. >> >> Here is an example table, indexed by frequency. The first element >> in each entry is the frequency in kHz: >> >> https://github.com/OPAE/opae-sdk/blob/master/libraries/plugins/xfpga/usrclk/fpga_user_clk_freq.h >> >>
>> >> >> >> >> We previously experimented with a kernel-space driver. The
>> implementation exported a sysfs node into which the table values >> for the desired frequency would be written in order to set the >> desired frequency. The function of the driver was to execute the >> logic required to program the device. We did not think this >> implementation should be up-streamed. >> >> It isn't practical to upstream the frequency tables as they are >> subject to change for future devices. For example, if the >> reference frequency changed in a future device, a whole new table >> of values would have to be added for the new device. In a recent >> transition to a new device, the range of frequencies was increased >> which required an extension to an existing table. > > Making a table for the inputs & outputs is always a easier way to > get things done, but the trade off is, as you said, extension to the > table every time for new outputs. > > So do we really need all parameters to be in a table, or these are > actually the outcome of some calculation? Is it possible just > Implementing the calculation.
For each desired frequency, the table values are produced by calling
the quartus tool, the same tool that generates the IOPLL RTL logic.
The quartus tool allows the RTL designer to select different options
which can affect the table values. For example, the current IOPLL
used in OFS has two frequency outputs and the desired relationship
between the two frequencies is 1x/2x until the 2x frequency reaches
a threshold (about 800MHz) and then the relationship is modified.

To convert this process into an algorithm would require reverse
engineering the quartus algorithm for the set of variables and
clock relationships in a specific implementation. The resulting
algorithm would have a very narrow application; we would have to
upstream additional algorithms for future, modified implementations.
Also, customers have the ability to modify the IOPLL implementation
if they choose. A table driven driver enables customers to easily
adapt the driver to their implementation.

We think a userspace table-driven driver is the best approach for
supporting the user clock.

- Russ

> > > If I remember correctly, linux clk framework enables a generic clk > caculation mechanism. It encourages people to model the internal > refclk, plls (and deviders?) separately and construct the clk tree. > Then the specified calculation could be simpler for each clk driver. > > I'm not sure the clk framework fits all your need, but please > investigate it firstly. > >> >> A previous implementation of the user clock was also implemented >> in user-space. The kernel driver exported each of the registers, >> but all of the logic was implemented in user-space. The kernel >> portion can be viewed here: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/fpga/dfl-afu-main.c#n380 >> >> >> >> >> >> >> This is our reasoning in choosing to implement this driver in
>> user-space. Would you consider a uio based user-space driver to be >> acceptable for in this case? > > As usual, we firstly make clear why existing framework cannot fit > the case and should be implemented in userspace, then everything > would be OK. > > Thanks, Yilun > >> >> - Russ >> >> >>> >>> Thanks, Yilun >>> >>>> The DFL feature id table can be found at: >>>> https://github.com/OPAE/dfl-feature-id >>>> >>>> Signed-off-by: Peter Colberg <peter.colberg@intel.com> --- >>>> drivers/uio/uio_dfl.c | 2 ++ 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c >>>> index 8f39cc8bb034..69e93f3e7faf 100644 --- >>>> a/drivers/uio/uio_dfl.c +++ b/drivers/uio/uio_dfl.c @@ -46,10 >>>> +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev) >>>> >>>> #define FME_FEATURE_ID_ETH_GROUP 0x10 #define >>>> FME_FEATURE_ID_HSSI_SUBSYS 0x15 +#define >>>> PORT_FEATURE_ID_IOPLL_USRCLK 0x14 >>>> >>>> static const struct dfl_device_id uio_dfl_ids[] = { { FME_ID, >>>> FME_FEATURE_ID_ETH_GROUP }, { FME_ID, >>>>
FME_FEATURE_ID_HSSI_SUBSYS }, + { PORT_ID, >>>> PORT_FEATURE_ID_IOPLL_USRCLK }, { } }; MODULE_DEVICE_TABLE(dfl, >>>> uio_dfl_ids); -- 2.28.0 >>>> >>

Re: [PATCH v1] uio: dfl: add IOPLL user-clock feature id

[Tom Rix]

On 8/22/22 10:38 AM, Russ Weight wrote:
>
> On 8/21/22 21:49, Xu Yilun wrote:
>> On 2022-08-18 at 17:38:35 -0600, Russ Weight wrote: >> >> >> On 8/17/22 21:18, Xu Yilun wrote: >>> On 2022-08-17 at 17:37:46 -0400, Peter Colberg wrote: >>>> Add a Device Feature List (DFL) feature id for the >>>> configurable IOPLL user clock source, which can be used to >>>> configure the clock speeds that are used for RTL logic that is >>>> programmed into the Partial Reconfiguration (PR) region of an >>>> FPGA. >>> Why not use linux clock framework for this IOPLL? And let the PR >>> driver set it togeter with the RTL logic reporgramming? >> >> Hi Yilun, >> >> We previously explored the possibility of plugging into the linux >> clock framework. For this device, setting a desired frequency is >> heavily dependent on a table of values that must be programmed in >> order to achieve the desired clock speeds. >> >> Here is an example table, indexed by frequency. The first element >> in each entry is the frequency in kHz: >> >> https://github.com/OPAE/opae-sdk/blob/master/libraries/plugins/xfpga/usrclk/fpga_user_clk_freq.h >> >>
>>>>>>>>>>> We previously experimented with a kernel-space driver. The
>>> implementation exported a sysfs node into which the table values >> for the desired frequency would be written in order to set the >> desired frequency. The function of the driver was to execute the >> logic required to program the device. We did not think this >> implementation should be up-streamed. >> >> It isn't practical to upstream the frequency tables as they are >> subject to change for future devices. For example, if the >> reference frequency changed in a future device, a whole new table >> of values would have to be added for the new device. In a recent >> transition to a new device, the range of frequencies was increased >> which required an extension to an existing table. > > Making a table for the inputs & outputs is always a easier way to > get things done, but the trade off is, as you said, extension to the > table every time for new outputs. > > So do we really need all parameters to be in a table, or these are > actually the outcome of some calculation? Is it possible just > Implementing the calculation.
> For each desired frequency, the table values are produced by calling
> the quartus tool, the same tool that generates the IOPLL RTL logic.
> The quartus tool allows the RTL designer to select different options
> which can affect the table values. For example, the current IOPLL
> used in OFS has two frequency outputs and the desired relationship
> between the two frequencies is 1x/2x until the 2x frequency reaches
> a threshold (about 800MHz) and then the relationship is modified.
>
> To convert this process into an algorithm would require reverse
> engineering the quartus algorithm for the set of variables and
> clock relationships in a specific implementation. The resulting
> algorithm would have a very narrow application; we would have to
> upstream additional algorithms for future, modified implementations.
> Also, customers have the ability to modify the IOPLL implementation
> if they choose. A table driven driver enables customers to easily
> adapt the driver to their implementation.
>
> We think a userspace table-driven driver is the best approach for
> supporting the user clock.
>
> - Russ

Agreeing with Russ, let's move this out to userspace.

Tom

>
>>>> If I remember correctly, linux clk framework enables a generic clk > caculation mechanism. It encourages people to model the internal > refclk, plls (and deviders?) separately and construct the clk tree. > Then the specified calculation could be simpler for each clk driver. > > I'm not sure the clk framework fits all your need, but please > investigate it firstly. > >> >> A previous implementation of the user clock was also implemented >> in user-space. The kernel driver exported each of the registers, >> but all of the logic was implemented in user-space. The kernel >> portion can be viewed here: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/fpga/dfl-afu-main.c#n380 >> >> >> >> >> >> >> This is our reasoning in choosing to implement this driver in
>>> user-space. Would you consider a uio based user-space driver to be >> acceptable for in this case? > > As usual, we firstly make clear why existing framework cannot fit > the case and should be implemented in userspace, then everything > would be OK. > > Thanks, Yilun > >> >> - Russ >> >> >>> >>> Thanks, Yilun >>> >>>> The DFL feature id table can be found at: >>>> https://github.com/OPAE/dfl-feature-id >>>> >>>> Signed-off-by: Peter Colberg <peter.colberg@intel.com> --- >>>> drivers/uio/uio_dfl.c | 2 ++ 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c >>>> index 8f39cc8bb034..69e93f3e7faf 100644 --- >>>> a/drivers/uio/uio_dfl.c +++ b/drivers/uio/uio_dfl.c @@ -46,10 >>>> +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev) >>>> >>>> #define FME_FEATURE_ID_ETH_GROUP 0x10 #define >>>> FME_FEATURE_ID_HSSI_SUBSYS 0x15 +#define >>>> PORT_FEATURE_ID_IOPLL_USRCLK 0x14 >>>> >>>> static const struct dfl_device_id uio_dfl_ids[] = { { FME_ID, >>>> FME_FEATURE_ID_ETH_GROUP }, { FME_ID, >>>>
> FME_FEATURE_ID_HSSI_SUBSYS }, + { PORT_ID, >>>> PORT_FEATURE_ID_IOPLL_USRCLK }, { } }; MODULE_DEVICE_TABLE(dfl, >>>> uio_dfl_ids); -- 2.28.0 >>>> >>
>

Re: [PATCH v1] uio: dfl: add IOPLL user-clock feature id

[Xu Yilun]

On 2022-08-22 at 10:38:51 -0700, Russ Weight wrote:
> 
> 
> On 8/21/22 21:49, Xu Yilun wrote:
> > On 2022-08-18 at 17:38:35 -0600, Russ Weight wrote: >> >> >> On 8/17/22 21:18, Xu Yilun wrote: >>> On 2022-08-17 at 17:37:46 -0400, Peter Colberg wrote: >>>> Add a Device Feature List (DFL) feature id for the >>>> configurable IOPLL user clock source, which can be used to >>>> configure the clock speeds that are used for RTL logic that is >>>> programmed into the Partial Reconfiguration (PR) region of an >>>> FPGA. >>> Why not use linux clock framework for this IOPLL? And let the PR >>> driver set it togeter with the RTL logic reporgramming? >> >> Hi Yilun, >> >> We previously explored the possibility of plugging into the linux >> clock framework. For this device, setting a desired frequency is >> heavily dependent on a table of values that must be programmed in >> order to achieve the desired clock speeds. >> >> Here is an example table, indexed by frequency. The first element >> in each entry is the frequency in kHz: >> >> https://github.com/OPAE/opae-sdk/blob/master/libraries/plugins/xfpga/usrclk/fpga_user_clk_freq.h >> >>
> >> >> >> >> >> We previously experimented with a kernel-space driver. The
> >> implementation exported a sysfs node into which the table values >> for the desired frequency would be written in order to set the >> desired frequency. The function of the driver was to execute the >> logic required to program the device. We did not think this >> implementation should be up-streamed. >> >> It isn't practical to upstream the frequency tables as they are >> subject to change for future devices. For example, if the >> reference frequency changed in a future device, a whole new table >> of values would have to be added for the new device. In a recent >> transition to a new device, the range of frequencies was increased >> which required an extension to an existing table. > > Making a table for the inputs & outputs is always a easier way to > get things done, but the trade off is, as you said, extension to the > table every time for new outputs. > > So do we really need all parameters to be in a table, or these are > actually the outcome of some calculation? Is it possible just > Implementing the calculation.
> For each desired frequency, the table values are produced by calling
> the quartus tool, the same tool that generates the IOPLL RTL logic.

OK, this is an important reason.

> The quartus tool allows the RTL designer to select different options
> which can affect the table values. For example, the current IOPLL
> used in OFS has two frequency outputs and the desired relationship
> between the two frequencies is 1x/2x until the 2x frequency reaches
> a threshold (about 800MHz) and then the relationship is modified.
> 
> To convert this process into an algorithm would require reverse
> engineering the quartus algorithm for the set of variables and
> clock relationships in a specific implementation. The resulting
> algorithm would have a very narrow application; we would have to

This makes sense to me.

> upstream additional algorithms for future, modified implementations.
> Also, customers have the ability to modify the IOPLL implementation
> if they choose. A table driven driver enables customers to easily

That also makes sense to me.

> adapt the driver to their implementation.

So could you please add some brief description in commit message? The
code is good to me.

Thanks,
Yioun

> 
> We think a userspace table-driven driver is the best approach for
> supporting the user clock.
> 
> - Russ
> 
> > > > If I remember correctly, linux clk framework enables a generic clk > caculation mechanism. It encourages people to model the internal > refclk, plls (and deviders?) separately and construct the clk tree. > Then the specified calculation could be simpler for each clk driver. > > I'm not sure the clk framework fits all your need, but please > investigate it firstly. > >> >> A previous implementation of the user clock was also implemented >> in user-space. The kernel driver exported each of the registers, >> but all of the logic was implemented in user-space. The kernel >> portion can be viewed here: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/fpga/dfl-afu-main.c#n380 >> >> >> >> >> >> >> This is our reasoning in choosing to implement this driver in
> >> user-space. Would you consider a uio based user-space driver to be >> acceptable for in this case? > > As usual, we firstly make clear why existing framework cannot fit > the case and should be implemented in userspace, then everything > would be OK. > > Thanks, Yilun > >> >> - Russ >> >> >>> >>> Thanks, Yilun >>> >>>> The DFL feature id table can be found at: >>>> https://github.com/OPAE/dfl-feature-id >>>> >>>> Signed-off-by: Peter Colberg <peter.colberg@intel.com> --- >>>> drivers/uio/uio_dfl.c | 2 ++ 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c >>>> index 8f39cc8bb034..69e93f3e7faf 100644 --- >>>> a/drivers/uio/uio_dfl.c +++ b/drivers/uio/uio_dfl.c @@ -46,10 >>>> +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev) >>>> >>>> #define FME_FEATURE_ID_ETH_GROUP 0x10 #define >>>> FME_FEATURE_ID_HSSI_SUBSYS 0x15 +#define >>>> PORT_FEATURE_ID_IOPLL_USRCLK 0x14 >>>> >>>> static const struct dfl_device_id uio_dfl_ids[] = { { FME_ID, >>>> FME_FEATURE_ID_ETH_GROUP }, { FME_ID, >>>>
> FME_FEATURE_ID_HSSI_SUBSYS }, + { PORT_ID, >>>> PORT_FEATURE_ID_IOPLL_USRCLK }, { } }; MODULE_DEVICE_TABLE(dfl, >>>> uio_dfl_ids); -- 2.28.0 >>>> >>
> 

[PATCH v2] uio: dfl: add IOPLL user-clock feature id

[Peter Colberg]

Add a Device Feature List (DFL) feature id [1] for the configurable
IOPLL user clock source, which can be used to configure the clock
speeds that are used for RTL logic that is programmed into the
Partial Reconfiguration (PR) region of an FPGA.

The IOPLL user-space driver [2] contains frequency tables [3]
with the specific user clock frequencies for an implementation.

For each desired frequency, the table values are produced by calling
the quartus tool, the same tool that generates the IOPLL RTL logic.
The quartus tool allows the RTL designer to select different options
which can affect the table values. The table-driven, user-space
driver allows for supporting future, modified implementations and
provides users the ability to modify the IOPLL implementation.

[1] https://github.com/OPAE/dfl-feature-id
[2] https://github.com/OPAE/opae-sdk/blob/a494f54a9f0356d0425edbff228f0254a4c70303/libraries/plugins/xfpga/usrclk/fpga_user_clk.c
[3] https://github.com/OPAE/opae-sdk/blob/a494f54a9f0356d0425edbff228f0254a4c70303/libraries/plugins/xfpga/usrclk/fpga_user_clk_freq.h

Signed-off-by: Peter Colberg <peter.colberg@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 drivers/uio/uio_dfl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
index 8f39cc8bb034..69e93f3e7faf 100644
--- a/drivers/uio/uio_dfl.c
+++ b/drivers/uio/uio_dfl.c
@@ -46,10 +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev)
 
 #define FME_FEATURE_ID_ETH_GROUP	0x10
 #define FME_FEATURE_ID_HSSI_SUBSYS	0x15
+#define PORT_FEATURE_ID_IOPLL_USRCLK	0x14
 
 static const struct dfl_device_id uio_dfl_ids[] = {
 	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
 	{ FME_ID, FME_FEATURE_ID_HSSI_SUBSYS },
+	{ PORT_ID, PORT_FEATURE_ID_IOPLL_USRCLK },
 	{ }
 };
 MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
-- 
2.28.0

Re: [PATCH v2] uio: dfl: add IOPLL user-clock feature id

[Xu Yilun]

On 2022-08-26 at 11:01:16 -0400, Peter Colberg wrote:
> Add a Device Feature List (DFL) feature id [1] for the configurable
> IOPLL user clock source, which can be used to configure the clock
> speeds that are used for RTL logic that is programmed into the
> Partial Reconfiguration (PR) region of an FPGA.
> 
> The IOPLL user-space driver [2] contains frequency tables [3]
> with the specific user clock frequencies for an implementation.
> 
> For each desired frequency, the table values are produced by calling
> the quartus tool, the same tool that generates the IOPLL RTL logic.
> The quartus tool allows the RTL designer to select different options
> which can affect the table values. The table-driven, user-space
> driver allows for supporting future, modified implementations and
> provides users the ability to modify the IOPLL implementation.
> 
> [1] https://github.com/OPAE/dfl-feature-id
> [2] https://github.com/OPAE/opae-sdk/blob/a494f54a9f0356d0425edbff228f0254a4c70303/libraries/plugins/xfpga/usrclk/fpga_user_clk.c
> [3] https://github.com/OPAE/opae-sdk/blob/a494f54a9f0356d0425edbff228f0254a4c70303/libraries/plugins/xfpga/usrclk/fpga_user_clk_freq.h
> 
> Signed-off-by: Peter Colberg <peter.colberg@intel.com>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>

Acked-by: Xu Yilun <yilun.xu@intel.com>

> ---
>  drivers/uio/uio_dfl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
> index 8f39cc8bb034..69e93f3e7faf 100644
> --- a/drivers/uio/uio_dfl.c
> +++ b/drivers/uio/uio_dfl.c
> @@ -46,10 +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev)
>  
>  #define FME_FEATURE_ID_ETH_GROUP	0x10
>  #define FME_FEATURE_ID_HSSI_SUBSYS	0x15
> +#define PORT_FEATURE_ID_IOPLL_USRCLK	0x14
>  
>  static const struct dfl_device_id uio_dfl_ids[] = {
>  	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
>  	{ FME_ID, FME_FEATURE_ID_HSSI_SUBSYS },
> +	{ PORT_ID, PORT_FEATURE_ID_IOPLL_USRCLK },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
> -- 
> 2.28.0
> 

[PATCH v3] uio: dfl: add IOPLL user-clock feature id

[Peter Colberg]

Add a Device Feature List (DFL) feature id [1] for the configurable
IOPLL user clock source, which can be used to configure the clock
speeds that are used for RTL logic that is programmed into the
Partial Reconfiguration (PR) region of an FPGA.

The IOPLL user-space driver [2] contains frequency tables [3]
with the specific user clock frequencies for an implementation.

For each desired frequency, the table values are produced by calling
the quartus tool, the same tool that generates the IOPLL RTL logic.
The quartus tool allows the RTL designer to select different options
which can affect the table values. The table-driven, user-space
driver allows for supporting future, modified implementations and
provides users the ability to modify the IOPLL implementation.

[1] https://github.com/OPAE/dfl-feature-id
[2] https://github.com/OPAE/opae-sdk/blob/a494f54a9f0356d0425edbff228f0254a4c70303/libraries/plugins/xfpga/usrclk/fpga_user_clk.c
[3] https://github.com/OPAE/opae-sdk/blob/a494f54a9f0356d0425edbff228f0254a4c70303/libraries/plugins/xfpga/usrclk/fpga_user_clk_freq.h

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Peter Colberg <peter.colberg@intel.com>
Acked-by: Xu Yilun <yilun.xu@intel.com>
---
v3:
	Reorder Signed-off-by: such that submitter is last
	Add Acked-by: Xu Yilun

v2:
	Describe IOPLL user-space driver in commit message
	Add Signed-off-by: Russ Weight
---
 drivers/uio/uio_dfl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/uio/uio_dfl.c b/drivers/uio/uio_dfl.c
index 8f39cc8bb034..69e93f3e7faf 100644
--- a/drivers/uio/uio_dfl.c
+++ b/drivers/uio/uio_dfl.c
@@ -46,10 +46,12 @@ static int uio_dfl_probe(struct dfl_device *ddev)
 
 #define FME_FEATURE_ID_ETH_GROUP	0x10
 #define FME_FEATURE_ID_HSSI_SUBSYS	0x15
+#define PORT_FEATURE_ID_IOPLL_USRCLK	0x14
 
 static const struct dfl_device_id uio_dfl_ids[] = {
 	{ FME_ID, FME_FEATURE_ID_ETH_GROUP },
 	{ FME_ID, FME_FEATURE_ID_HSSI_SUBSYS },
+	{ PORT_ID, PORT_FEATURE_ID_IOPLL_USRCLK },
 	{ }
 };
 MODULE_DEVICE_TABLE(dfl, uio_dfl_ids);
-- 
2.28.0