Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <acdf9c04-0816-5995-da90-c53153ffac59@intel.com>
Date:   Thu, 18 Aug 2022 17:38:35 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.11.0
Subject: Re: [PATCH v1] uio: dfl: add IOPLL user-clock feature id
Content-Language: en-US
To:     Xu Yilun <yilun.xu@intel.com>,
        Peter Colberg <peter.colberg@intel.com>
CC:     Wu Hao <hao.wu@intel.com>, Tom Rix <trix@redhat.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        <linux-fpga@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <matthew.gerlach@linux.intel.com>,
        <basheer.ahmed.muddebihal@intel.com>, <tianfei.zhang@intel.com>,
        <marpagan@redhat.com>, <lgoncalv@redhat.com>
References: <20220817213746.4986-1-peter.colberg@intel.com>
 <Yv29ev8OKyEYcaf/@yilunxu-OptiPlex-7050>
From:   Russ Weight <russell.h.weight@intel.com>
In-Reply-To: <Yv29ev8OKyEYcaf/@yilunxu-OptiPlex-7050>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?YnIzSkNHRmZobWVJZFdtL0Y3SnB4OEczcWJxUnMvTmVldEMvWFh4NUI2Yk5R?=
 =?utf-8?B?c2V2U25kYVZ0aUFWOEZuT3ZKYUZnaTBIdzVZajR2ZnJxNXpQLzZKWXU2ZDFN?=
 =?utf-8?B?OXV0dnN0RXpNUCtxZ042RXhKU3d1TExKeDFCUkpXckJhY1QzQlkva2FxTk5u?=
 =?utf-8?B?NkJQVEI4NFJqRC9VV0E2NkU2UTVYbGZoTnRJT0MvKzRtQ2psQlNxbU1zQjdh?=
 =?utf-8?B?U3YyM09nUXJaSzZWMUROQTRnUXV6ZjZBK3prVmUxWE1JV0Y0QWZ2cExsb1hK?=
 =?utf-8?B?NWV5bUdQeEM0OFhxczRJSXROc05pNnhETnRHbS9MYlVjUHkzQkJiU3BrM3Vj?=
 =?utf-8?B?TWdLMCtpejBHQjJWMmdjT2VCZjVWZ0RadVZzUlJiNk1TcUNpcUVGUTZvZFhT?=
 =?utf-8?B?dmxQdEZsby94QmFydGRDbmRyM2ppOWlVb0hTaFZZS0xDc0tJRGdXQmxzclFt?=
 =?utf-8?B?eHZ2ZUFJUEFyMVAzZTZoT3F1VWk2SlBma0dyYTBzQXl5K1QybVB4RGNLdHF1?=
 =?utf-8?B?RFRDbm1oUStZT3NIL0lkeEsraE1iQ0xpeGtvRTFJMC9pQmRkL0hnVVZGVEo3?=
 =?utf-8?B?c051SC9JYXJadHpCK0wyQVRhV1JKT0tsNXhjbllXWGNoK05HM0pMRFJhMUNn?=
 =?utf-8?B?Sk5XRGNmbE9SSmsza1JwVlJ3eEJUWnAxTGJKd2Vab1lqSHBGQkxWQkRXVjU4?=
 =?utf-8?B?TnNrZDk1Wnp4NUZlUjJMcXdPaFlxWUtZc0lLNG0wOFJxQjBlZWNmUDBXOWR4?=
 =?utf-8?B?V3dGeUZ0MWE2cGlJdnQ2Sy84UFRPY3E3bko1NGdmWU1KY2EvTVhSTFRZaHp4?=
 =?utf-8?B?SWVCVXZpM1JkV2VDRHBid2s4Qytndzd1RGlZTWFlRWVrZHFMeG9XWERDaUdu?=
 =?utf-8?B?N1pQaEFMOGoxZG5OS2o2eTZDWDJxYkthbllxWlRtQStjVEZNMmNzR0xaT0lH?=
 =?utf-8?B?d0JmaXcxUWpDU2pPVmxZL3JBRHhESEw3aXJPaXRlTERFSjl3TEQyL0VmMnBV?=
 =?utf-8?B?YXpBSHdubnZWc2VFWS9hWUQ5dDhpZGRNTGwyeURVSVliYkloUXFnK2Mxcmd6?=
 =?utf-8?B?VWZRUnlLTTRhTnkwWFBGUkdRR29hcWVjWlUrdGppakNVOWo5NHpFQ1JtVmJW?=
 =?utf-8?B?cWtNQ2psc0VPUlYzY0orYWFab3RGZkxmZ0xEM2xSMnZhNXJRWFFYVFpnMzM4?=
 =?utf-8?B?WFBNN3J1SHp2QitXVjVxQkJhdUQ5ejM1ekg5ZTJ1d0NmcGhGK1RSWFFqS3RP?=
 =?utf-8?B?N2RBdVdoc3ZBcUlibXV5dzFRNUplTTJueFhZanZhV2d5Sk1pdWVrbmQybDZi?=
 =?utf-8?B?K2dUcDZEd3lzTjZta1hreHh1LzMvclkzWE4wc0tBUVdCY09aS2M5U0ZVTk0x?=
 =?utf-8?B?ejJaSm5lZmxqajhSenFMUE1OMzZGU0NnSWFuVkhSUVVGdVEvb3dSWUg5TlI2?=
 =?utf-8?B?ZkNzc1l0aW4rQmVhdFhpUllORVRhY1h0elBWTjJkeHh3ZkxRUWFQbjNacUVD?=
 =?utf-8?B?cEtzUm04NDFHV3hGL3pERWcxNlhiaTVZMVNGbFRPWkxBZ1pyWFBuOXh0cHBR?=
 =?utf-8?B?MzNsK0RydTliK3lRK2JZYW14T2lmeXNJbHhGODhaL1NkaytPN1NXcEFYbEtt?=
 =?utf-8?B?bEp4YzJJNndTN0F5VWFCVG82RmJNYmhuSURFODRSb3QxalpjZzNoTTh5azE4?=
 =?utf-8?B?UWoxMkUzejcrSUs4N2phbHVvdXVQNFJNTVV3dU1PR0EvYlJva3V6YzQ3UGtn?=
 =?utf-8?B?MmpDOU9mQ1VxM29xaWdjTllLcWRlbmV2UzFYbllMblBrTnpTNUlMM0FtVDlH?=
 =?utf-8?B?SFgweCtBais3WWxZN1BqQlRWdW4weE56VFBrSmxHMHhEVmNhSEljZXEzSHg0?=
 =?utf-8?B?Qm5HVG1QMlR5MDltVCtmcDJWZzg2aEh2WGxFNFVSa2lDa2Z3b2UxSmFIWU1O?=
 =?utf-8?B?VnRLcFo0SUQxZHdaODlPbjZ1WWF0MVo1QmJqVHRvLzlIbzJEb2lrN0tGb1lD?=
 =?utf-8?B?cURTMFFWdmZ1dGYyc0VmQ0pWQ2FFL0hpaEJaNzlPSUlMSVo3Sms1ZUFSUWhJ?=
 =?utf-8?B?QlE3MTBLYThYeDQrKzlnQjNQb3hhWjIyYlJWTUYyZkFOQ2lDOXRoTGx4YjRp?=
 =?utf-8?B?U1pnUmFoT0FQT1EzMnNmbFR4d0JSZGNBYWk5VHRWamZPNDkzWFZlcEdGb3hn?=
 =?utf-8?B?dVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 93cd6d3a-6551-4bfa-d808-08da8172c6ee
X-MS-Exchange-CrossTenant-AuthSource: DM5PR11MB1899.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Aug 2022 23:38:39.1861
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3OzRnGwjw5szO52fPhxglAM8Ytt2uy5Y6viBvMXt8SzAwwadBryYNzX17mHqb2N+4ejpQvx37thmlt+y8SJVzacE4F5rMVjYf1h1a15kuVo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4769
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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
>>