Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Michal Simek]

On 18.12.2017 15:05, Arnd Bergmann wrote:
> On Mon, Dec 18, 2017 at 2:13 PM, Michal Simek  wrote:
>> On 15.12.2017 14:26, Arnd Bergmann wrote:
>>> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah  wrote:
 Xilinx ZYNQMP logicoreIP Init driver is based on the new
 LogiCoreIP design created. This driver provides the processing system
 and programmable logic isolation. Set the frequency based on the clock
 information get from the logicoreIP register set.

 It is put in drivers/misc as there is no subsystem for this logicoreIP.

 Signed-off-by: Dhaval Shah 
>>>
>>> After giving this some more thought, I'd suggest you move the driver to
>>> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
>>> it merged by Michal Simek as a driver patch that will go through arm-soc.
>>
>> I have not a problem of creating drivers/soc/xilinx/ location for this
>> driver. It is not zynq (arm32) but zynqmp(arm64) device where this
>> driver can be used. As far as I understand it is memory mapped soft IP
>> which could be also accessed by soft core CPU.
> 
> Ok. I wouldn't be worried about having a zynq directory for stuff that
> is common between zynq and zynqmp, but the soft code CPU case
> wouldn't make that ideal.
> 
>> It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
>> We have been discussing that openrisc cases and for sure if someone
>> wants to enable this driver there using misc location would be one
>> option but I also think that using drivers/soc/xilinx location is not a
>> bad option because it is very unlikely that anybody tries it.
>>
>> Arnd: misc or drivers/soc/xilinx?
> 
> drivers/soc/xilinx please, thanks for the clarification.
> 

ok. I have sent patch which prepare structures in drivers/soc/xilinx.

Dhaval: Please rebase your patch based on this and put driver to this
location.

Thanks,
Michal

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Michal Simek]

On 18.12.2017 15:05, Arnd Bergmann wrote:
> On Mon, Dec 18, 2017 at 2:13 PM, Michal Simek  wrote:
>> On 15.12.2017 14:26, Arnd Bergmann wrote:
>>> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah  wrote:
 Xilinx ZYNQMP logicoreIP Init driver is based on the new
 LogiCoreIP design created. This driver provides the processing system
 and programmable logic isolation. Set the frequency based on the clock
 information get from the logicoreIP register set.

 It is put in drivers/misc as there is no subsystem for this logicoreIP.

 Signed-off-by: Dhaval Shah 
>>>
>>> After giving this some more thought, I'd suggest you move the driver to
>>> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
>>> it merged by Michal Simek as a driver patch that will go through arm-soc.
>>
>> I have not a problem of creating drivers/soc/xilinx/ location for this
>> driver. It is not zynq (arm32) but zynqmp(arm64) device where this
>> driver can be used. As far as I understand it is memory mapped soft IP
>> which could be also accessed by soft core CPU.
> 
> Ok. I wouldn't be worried about having a zynq directory for stuff that
> is common between zynq and zynqmp, but the soft code CPU case
> wouldn't make that ideal.
> 
>> It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
>> We have been discussing that openrisc cases and for sure if someone
>> wants to enable this driver there using misc location would be one
>> option but I also think that using drivers/soc/xilinx location is not a
>> bad option because it is very unlikely that anybody tries it.
>>
>> Arnd: misc or drivers/soc/xilinx?
> 
> drivers/soc/xilinx please, thanks for the clarification.
> 

ok. I have sent patch which prepare structures in drivers/soc/xilinx.

Dhaval: Please rebase your patch based on this and put driver to this
location.

Thanks,
Michal

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Arnd Bergmann]

On Mon, Dec 18, 2017 at 2:13 PM, Michal Simek  wrote:
> On 15.12.2017 14:26, Arnd Bergmann wrote:
>> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah  wrote:
>>> Xilinx ZYNQMP logicoreIP Init driver is based on the new
>>> LogiCoreIP design created. This driver provides the processing system
>>> and programmable logic isolation. Set the frequency based on the clock
>>> information get from the logicoreIP register set.
>>>
>>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>>
>>> Signed-off-by: Dhaval Shah 
>>
>> After giving this some more thought, I'd suggest you move the driver to
>> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
>> it merged by Michal Simek as a driver patch that will go through arm-soc.
>
> I have not a problem of creating drivers/soc/xilinx/ location for this
> driver. It is not zynq (arm32) but zynqmp(arm64) device where this
> driver can be used. As far as I understand it is memory mapped soft IP
> which could be also accessed by soft core CPU.

Ok. I wouldn't be worried about having a zynq directory for stuff that
is common between zynq and zynqmp, but the soft code CPU case
wouldn't make that ideal.

> It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
> We have been discussing that openrisc cases and for sure if someone
> wants to enable this driver there using misc location would be one
> option but I also think that using drivers/soc/xilinx location is not a
> bad option because it is very unlikely that anybody tries it.
>
> Arnd: misc or drivers/soc/xilinx?

drivers/soc/xilinx please, thanks for the clarification.

   Arnd

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Arnd Bergmann]

On Mon, Dec 18, 2017 at 2:13 PM, Michal Simek  wrote:
> On 15.12.2017 14:26, Arnd Bergmann wrote:
>> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah  wrote:
>>> Xilinx ZYNQMP logicoreIP Init driver is based on the new
>>> LogiCoreIP design created. This driver provides the processing system
>>> and programmable logic isolation. Set the frequency based on the clock
>>> information get from the logicoreIP register set.
>>>
>>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>>
>>> Signed-off-by: Dhaval Shah 
>>
>> After giving this some more thought, I'd suggest you move the driver to
>> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
>> it merged by Michal Simek as a driver patch that will go through arm-soc.
>
> I have not a problem of creating drivers/soc/xilinx/ location for this
> driver. It is not zynq (arm32) but zynqmp(arm64) device where this
> driver can be used. As far as I understand it is memory mapped soft IP
> which could be also accessed by soft core CPU.

Ok. I wouldn't be worried about having a zynq directory for stuff that
is common between zynq and zynqmp, but the soft code CPU case
wouldn't make that ideal.

> It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
> We have been discussing that openrisc cases and for sure if someone
> wants to enable this driver there using misc location would be one
> option but I also think that using drivers/soc/xilinx location is not a
> bad option because it is very unlikely that anybody tries it.
>
> Arnd: misc or drivers/soc/xilinx?

drivers/soc/xilinx please, thanks for the clarification.

   Arnd

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Michal Simek]

On 15.12.2017 14:26, Arnd Bergmann wrote:
> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah  wrote:
>> Xilinx ZYNQMP logicoreIP Init driver is based on the new
>> LogiCoreIP design created. This driver provides the processing system
>> and programmable logic isolation. Set the frequency based on the clock
>> information get from the logicoreIP register set.
>>
>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>
>> Signed-off-by: Dhaval Shah 
> 
> After giving this some more thought, I'd suggest you move the driver to
> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
> it merged by Michal Simek as a driver patch that will go through arm-soc.

I have not a problem of creating drivers/soc/xilinx/ location for this
driver. It is not zynq (arm32) but zynqmp(arm64) device where this
driver can be used. As far as I understand it is memory mapped soft IP
which could be also accessed by soft core CPU.
It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
We have been discussing that openrisc cases and for sure if someone
wants to enable this driver there using misc location would be one
option but I also think that using drivers/soc/xilinx location is not a
bad option because it is very unlikely that anybody tries it.

Arnd: misc or drivers/soc/xilinx?

Thanks,
Michal

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Michal Simek]

On 15.12.2017 14:26, Arnd Bergmann wrote:
> In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah  wrote:
>> Xilinx ZYNQMP logicoreIP Init driver is based on the new
>> LogiCoreIP design created. This driver provides the processing system
>> and programmable logic isolation. Set the frequency based on the clock
>> information get from the logicoreIP register set.
>>
>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>
>> Signed-off-by: Dhaval Shah 
> 
> After giving this some more thought, I'd suggest you move the driver to
> drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
> it merged by Michal Simek as a driver patch that will go through arm-soc.

I have not a problem of creating drivers/soc/xilinx/ location for this
driver. It is not zynq (arm32) but zynqmp(arm64) device where this
driver can be used. As far as I understand it is memory mapped soft IP
which could be also accessed by soft core CPU.
It means drivers/soc/xilinx could be shared by all xilinx platforms anyway.
We have been discussing that openrisc cases and for sure if someone
wants to enable this driver there using misc location would be one
option but I also think that using drivers/soc/xilinx location is not a
bad option because it is very unlikely that anybody tries it.

Arnd: misc or drivers/soc/xilinx?

Thanks,
Michal

RE: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Dhaval Rajeshbhai Shah]

Hi Randy,

> -Original Message-
> From: Randy Dunlap [mailto:rdun...@infradead.org]
> Sent: Sunday, December 17, 2017 9:40 AM
> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>; a...@arndb.de;
> gre...@linuxfoundation.org; robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> michal.si...@xilinx.com; Hyun Kwon <hy...@xilinx.com>
> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
> init driver
> 
> On 12/16/2017 10:07 PM, Dhaval Rajeshbhai Shah wrote:
> > Hi Randy,
> >
> > Thanks a lot for the review.
> >
> >> -Original Message-
> >> From: Randy Dunlap [mailto:rdun...@infradead.org]
> >> Sent: Saturday, December 16, 2017 2:18 PM
> >> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>; a...@arndb.de;
> >> gre...@linuxfoundation.org; robh...@kernel.org; mark.rutl...@arm.com
> >> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> michal.si...@xilinx.com; Hyun Kwon <hy...@xilinx.com>; Dhaval
> >> Rajeshbhai Shah <ds...@xilinx.com>
> >> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU
> >> logicoreIP init driver
> >>
> >> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> >>> Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
> >>> design created. This driver provides the processing system and
> >>> programmable logic isolation. Set the frequency based on the clock
> >>> information get from the logicoreIP register set.
> >>>
> >>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
> >>>
> >>> Signed-off-by: Dhaval Shah <ds...@xilinx.com>
> >>> ---
> >>>
> >>>  drivers/misc/Kconfig|  15 ++
> >>>  drivers/misc/Makefile   |   1 +
> >>>  drivers/misc/xlnx_vcu.c | 631
> >>> 
> >>>  3 files changed, 647 insertions(+)
> >>>  create mode 100644 drivers/misc/xlnx_vcu.c
> >>
> >>> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> >>> file mode 100644 index 000..f489d34
> >>> --- /dev/null
> >>> +++ b/drivers/misc/xlnx_vcu.c
> >>> @@ -0,0 +1,631 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Xilinx VCU Init
> >>> + *
> >>> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> >>> + *
> >>> + * Contacts   Dhaval Shah <ds...@xilinx.com>
> >>> + */
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>
> >> [snip]
> >>
> >>
> >>> +/**
> >>> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> >>> + * @xvcu:Pointer to the xvcu_device structure
> >>> + *
> >>> + * Programming the VCU PLL based on the user configuration
> >>> + * (ref clock freq, core clock freq, mcu clock freq).
> >>> + * Core clock frequency has higher priority than mcu clock
> >>> +frequency
> >>> + * Errors in following cases
> >>> + *- When mcu or clock clock get from logicoreIP is 0
> >>> + *- When VCU PLL DIV related bits value other than 1
> >>> + *- When proper data not found for given data
> >>> + *- When sis570_1 clocksource related operation failed
> >>> + *
> >>> + * Return:   Returns status, either success or error+reason
> >>> + */
> >>> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
> >>> + u32 refclk, coreclk, mcuclk, inte, deci;
> >>> + u32 divisor_mcu, divisor_core, fvco;
> >>> + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> >>> + u32 cfg_val, mod, ctrl;
> >>> + int ret;
> >>> + unsigned int i;
> >>> + const struct xvcu_pll_cfg *found = NULL;
> >>> +
> >>> + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> >>> + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> >>> + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> >>> + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> >>> + if (!mcuclk || !coreclk) {
> >>> + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> >>> +

RE: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Dhaval Rajeshbhai Shah]

Hi Randy,

> -Original Message-
> From: Randy Dunlap [mailto:rdun...@infradead.org]
> Sent: Sunday, December 17, 2017 9:40 AM
> To: Dhaval Rajeshbhai Shah ; a...@arndb.de;
> gre...@linuxfoundation.org; robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> michal.si...@xilinx.com; Hyun Kwon 
> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
> init driver
> 
> On 12/16/2017 10:07 PM, Dhaval Rajeshbhai Shah wrote:
> > Hi Randy,
> >
> > Thanks a lot for the review.
> >
> >> -Original Message-
> >> From: Randy Dunlap [mailto:rdun...@infradead.org]
> >> Sent: Saturday, December 16, 2017 2:18 PM
> >> To: Dhaval Rajeshbhai Shah ; a...@arndb.de;
> >> gre...@linuxfoundation.org; robh...@kernel.org; mark.rutl...@arm.com
> >> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> michal.si...@xilinx.com; Hyun Kwon ; Dhaval
> >> Rajeshbhai Shah 
> >> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU
> >> logicoreIP init driver
> >>
> >> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> >>> Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
> >>> design created. This driver provides the processing system and
> >>> programmable logic isolation. Set the frequency based on the clock
> >>> information get from the logicoreIP register set.
> >>>
> >>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
> >>>
> >>> Signed-off-by: Dhaval Shah 
> >>> ---
> >>>
> >>>  drivers/misc/Kconfig|  15 ++
> >>>  drivers/misc/Makefile   |   1 +
> >>>  drivers/misc/xlnx_vcu.c | 631
> >>> 
> >>>  3 files changed, 647 insertions(+)
> >>>  create mode 100644 drivers/misc/xlnx_vcu.c
> >>
> >>> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> >>> file mode 100644 index 000..f489d34
> >>> --- /dev/null
> >>> +++ b/drivers/misc/xlnx_vcu.c
> >>> @@ -0,0 +1,631 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Xilinx VCU Init
> >>> + *
> >>> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> >>> + *
> >>> + * Contacts   Dhaval Shah 
> >>> + */
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>> +#include 
> >>
> >> [snip]
> >>
> >>
> >>> +/**
> >>> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> >>> + * @xvcu:Pointer to the xvcu_device structure
> >>> + *
> >>> + * Programming the VCU PLL based on the user configuration
> >>> + * (ref clock freq, core clock freq, mcu clock freq).
> >>> + * Core clock frequency has higher priority than mcu clock
> >>> +frequency
> >>> + * Errors in following cases
> >>> + *- When mcu or clock clock get from logicoreIP is 0
> >>> + *- When VCU PLL DIV related bits value other than 1
> >>> + *- When proper data not found for given data
> >>> + *- When sis570_1 clocksource related operation failed
> >>> + *
> >>> + * Return:   Returns status, either success or error+reason
> >>> + */
> >>> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
> >>> + u32 refclk, coreclk, mcuclk, inte, deci;
> >>> + u32 divisor_mcu, divisor_core, fvco;
> >>> + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> >>> + u32 cfg_val, mod, ctrl;
> >>> + int ret;
> >>> + unsigned int i;
> >>> + const struct xvcu_pll_cfg *found = NULL;
> >>> +
> >>> + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> >>> + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> >>> + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> >>> + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> >>> + if (!mcuclk || !coreclk) {
> >>> + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> >>> + dev_dbg(xvcu-&

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Randy Dunlap]

On 12/16/2017 10:07 PM, Dhaval Rajeshbhai Shah wrote:
> Hi Randy,
> 
> Thanks a lot for the review.
> 
>> -Original Message-
>> From: Randy Dunlap [mailto:rdun...@infradead.org]
>> Sent: Saturday, December 16, 2017 2:18 PM
>> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>; a...@arndb.de;
>> gre...@linuxfoundation.org; robh...@kernel.org; mark.rutl...@arm.com
>> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> michal.si...@xilinx.com; Hyun Kwon <hy...@xilinx.com>; Dhaval Rajeshbhai
>> Shah <ds...@xilinx.com>
>> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
>> init driver
>>
>> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
>>> Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
>>> design created. This driver provides the processing system and
>>> programmable logic isolation. Set the frequency based on the clock
>>> information get from the logicoreIP register set.
>>>
>>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>>
>>> Signed-off-by: Dhaval Shah <ds...@xilinx.com>
>>> ---
>>>
>>>  drivers/misc/Kconfig|  15 ++
>>>  drivers/misc/Makefile   |   1 +
>>>  drivers/misc/xlnx_vcu.c | 631
>>> 
>>>  3 files changed, 647 insertions(+)
>>>  create mode 100644 drivers/misc/xlnx_vcu.c
>>
>>> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
>>> file mode 100644 index 000..f489d34
>>> --- /dev/null
>>> +++ b/drivers/misc/xlnx_vcu.c
>>> @@ -0,0 +1,631 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Xilinx VCU Init
>>> + *
>>> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
>>> + *
>>> + * Contacts   Dhaval Shah <ds...@xilinx.com>
>>> + */
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>
>> [snip]
>>
>>
>>> +/**
>>> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
>>> + * @xvcu:  Pointer to the xvcu_device structure
>>> + *
>>> + * Programming the VCU PLL based on the user configuration
>>> + * (ref clock freq, core clock freq, mcu clock freq).
>>> + * Core clock frequency has higher priority than mcu clock frequency
>>> + * Errors in following cases
>>> + *- When mcu or clock clock get from logicoreIP is 0
>>> + *- When VCU PLL DIV related bits value other than 1
>>> + *- When proper data not found for given data
>>> + *- When sis570_1 clocksource related operation failed
>>> + *
>>> + * Return: Returns status, either success or error+reason
>>> + */
>>> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
>>> +   u32 refclk, coreclk, mcuclk, inte, deci;
>>> +   u32 divisor_mcu, divisor_core, fvco;
>>> +   u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
>>> +   u32 cfg_val, mod, ctrl;
>>> +   int ret;
>>> +   unsigned int i;
>>> +   const struct xvcu_pll_cfg *found = NULL;
>>> +
>>> +   inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
>>> +   deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
>>> +   coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
>>> +   mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
>>> +   if (!mcuclk || !coreclk) {
>>> +   dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
>>> +   dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
>>> +   dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
>>> +   dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
>>> +
>>> +   clk_disable_unprepare(xvcu->pll_ref);
>>> +   ret = clk_set_rate(xvcu->pll_ref, refclk);
>>> +   if (ret)
>>> +   dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
>>> +
>>> +   ret = clk_prepare_enable(xvcu->pll_ref);
>>> +   if (ret) {
>>> +   dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
>>> +   return ret;
>>> +   }
>>> +
>>> +

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Randy Dunlap]

On 12/16/2017 10:07 PM, Dhaval Rajeshbhai Shah wrote:
> Hi Randy,
> 
> Thanks a lot for the review.
> 
>> -Original Message-
>> From: Randy Dunlap [mailto:rdun...@infradead.org]
>> Sent: Saturday, December 16, 2017 2:18 PM
>> To: Dhaval Rajeshbhai Shah ; a...@arndb.de;
>> gre...@linuxfoundation.org; robh...@kernel.org; mark.rutl...@arm.com
>> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> michal.si...@xilinx.com; Hyun Kwon ; Dhaval Rajeshbhai
>> Shah 
>> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
>> init driver
>>
>> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
>>> Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
>>> design created. This driver provides the processing system and
>>> programmable logic isolation. Set the frequency based on the clock
>>> information get from the logicoreIP register set.
>>>
>>> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>>>
>>> Signed-off-by: Dhaval Shah 
>>> ---
>>>
>>>  drivers/misc/Kconfig|  15 ++
>>>  drivers/misc/Makefile   |   1 +
>>>  drivers/misc/xlnx_vcu.c | 631
>>> 
>>>  3 files changed, 647 insertions(+)
>>>  create mode 100644 drivers/misc/xlnx_vcu.c
>>
>>> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
>>> file mode 100644 index 000..f489d34
>>> --- /dev/null
>>> +++ b/drivers/misc/xlnx_vcu.c
>>> @@ -0,0 +1,631 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Xilinx VCU Init
>>> + *
>>> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
>>> + *
>>> + * Contacts   Dhaval Shah 
>>> + */
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>
>> [snip]
>>
>>
>>> +/**
>>> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
>>> + * @xvcu:  Pointer to the xvcu_device structure
>>> + *
>>> + * Programming the VCU PLL based on the user configuration
>>> + * (ref clock freq, core clock freq, mcu clock freq).
>>> + * Core clock frequency has higher priority than mcu clock frequency
>>> + * Errors in following cases
>>> + *- When mcu or clock clock get from logicoreIP is 0
>>> + *- When VCU PLL DIV related bits value other than 1
>>> + *- When proper data not found for given data
>>> + *- When sis570_1 clocksource related operation failed
>>> + *
>>> + * Return: Returns status, either success or error+reason
>>> + */
>>> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
>>> +   u32 refclk, coreclk, mcuclk, inte, deci;
>>> +   u32 divisor_mcu, divisor_core, fvco;
>>> +   u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
>>> +   u32 cfg_val, mod, ctrl;
>>> +   int ret;
>>> +   unsigned int i;
>>> +   const struct xvcu_pll_cfg *found = NULL;
>>> +
>>> +   inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
>>> +   deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
>>> +   coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
>>> +   mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
>>> +   if (!mcuclk || !coreclk) {
>>> +   dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
>>> +   dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
>>> +   dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
>>> +   dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
>>> +
>>> +   clk_disable_unprepare(xvcu->pll_ref);
>>> +   ret = clk_set_rate(xvcu->pll_ref, refclk);
>>> +   if (ret)
>>> +   dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
>>> +
>>> +   ret = clk_prepare_enable(xvcu->pll_ref);
>>> +   if (ret) {
>>> +   dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
>>> +   return ret;
>>> +   }
>>> +
>>> +   refclk = clk_get_rate(xvcu->pll_ref);
>>> +
>>> +   /*
>>> +* The divide-by-2 should b

RE: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Dhaval Rajeshbhai Shah]

Hi Randy,

Thanks a lot for the review.

> -Original Message-
> From: Randy Dunlap [mailto:rdun...@infradead.org]
> Sent: Saturday, December 16, 2017 2:18 PM
> To: Dhaval Rajeshbhai Shah <ds...@xilinx.com>; a...@arndb.de;
> gre...@linuxfoundation.org; robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> michal.si...@xilinx.com; Hyun Kwon <hy...@xilinx.com>; Dhaval Rajeshbhai
> Shah <ds...@xilinx.com>
> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
> init driver
> 
> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> > Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
> > design created. This driver provides the processing system and
> > programmable logic isolation. Set the frequency based on the clock
> > information get from the logicoreIP register set.
> >
> > It is put in drivers/misc as there is no subsystem for this logicoreIP.
> >
> > Signed-off-by: Dhaval Shah <ds...@xilinx.com>
> > ---
> >
> >  drivers/misc/Kconfig|  15 ++
> >  drivers/misc/Makefile   |   1 +
> >  drivers/misc/xlnx_vcu.c | 631
> > 
> >  3 files changed, 647 insertions(+)
> >  create mode 100644 drivers/misc/xlnx_vcu.c
> 
> > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > file mode 100644 index 000..f489d34
> > --- /dev/null
> > +++ b/drivers/misc/xlnx_vcu.c
> > @@ -0,0 +1,631 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Xilinx VCU Init
> > + *
> > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > + *
> > + * Contacts   Dhaval Shah <ds...@xilinx.com>
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> 
> [snip]
> 
> 
> > +/**
> > + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> > + * @xvcu:  Pointer to the xvcu_device structure
> > + *
> > + * Programming the VCU PLL based on the user configuration
> > + * (ref clock freq, core clock freq, mcu clock freq).
> > + * Core clock frequency has higher priority than mcu clock frequency
> > + * Errors in following cases
> > + *- When mcu or clock clock get from logicoreIP is 0
> > + *- When VCU PLL DIV related bits value other than 1
> > + *- When proper data not found for given data
> > + *- When sis570_1 clocksource related operation failed
> > + *
> > + * Return: Returns status, either success or error+reason
> > + */
> > +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
> > +   u32 refclk, coreclk, mcuclk, inte, deci;
> > +   u32 divisor_mcu, divisor_core, fvco;
> > +   u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> > +   u32 cfg_val, mod, ctrl;
> > +   int ret;
> > +   unsigned int i;
> > +   const struct xvcu_pll_cfg *found = NULL;
> > +
> > +   inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> > +   deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> > +   coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> > +   mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> > +   if (!mcuclk || !coreclk) {
> > +   dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> > +   return -EINVAL;
> > +   }
> > +
> > +   refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> > +   dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> > +   dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> > +   dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> > +
> > +   clk_disable_unprepare(xvcu->pll_ref);
> > +   ret = clk_set_rate(xvcu->pll_ref, refclk);
> > +   if (ret)
> > +   dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> > +
> > +   ret = clk_prepare_enable(xvcu->pll_ref);
> > +   if (ret) {
> > +   dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> > +   return ret;
> > +   }
> > +
> > +   refclk = clk_get_rate(xvcu->pll_ref);
> > +
> > +   /*
> > +* The divide-by-2 should be always enabled (==1)
> > +* to meet the timing in the design.
> > +* Otherwise, it's an error
> > +*/
> > +   vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> > +   clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SH

RE: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Dhaval Rajeshbhai Shah]

Hi Randy,

Thanks a lot for the review.

> -Original Message-
> From: Randy Dunlap [mailto:rdun...@infradead.org]
> Sent: Saturday, December 16, 2017 2:18 PM
> To: Dhaval Rajeshbhai Shah ; a...@arndb.de;
> gre...@linuxfoundation.org; robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org;
> michal.si...@xilinx.com; Hyun Kwon ; Dhaval Rajeshbhai
> Shah 
> Subject: Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP
> init driver
> 
> On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> > Xilinx ZYNQMP logicoreIP Init driver is based on the new LogiCoreIP
> > design created. This driver provides the processing system and
> > programmable logic isolation. Set the frequency based on the clock
> > information get from the logicoreIP register set.
> >
> > It is put in drivers/misc as there is no subsystem for this logicoreIP.
> >
> > Signed-off-by: Dhaval Shah 
> > ---
> >
> >  drivers/misc/Kconfig|  15 ++
> >  drivers/misc/Makefile   |   1 +
> >  drivers/misc/xlnx_vcu.c | 631
> > 
> >  3 files changed, 647 insertions(+)
> >  create mode 100644 drivers/misc/xlnx_vcu.c
> 
> > diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c new
> > file mode 100644 index 000..f489d34
> > --- /dev/null
> > +++ b/drivers/misc/xlnx_vcu.c
> > @@ -0,0 +1,631 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Xilinx VCU Init
> > + *
> > + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> > + *
> > + * Contacts   Dhaval Shah 
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> 
> [snip]
> 
> 
> > +/**
> > + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> > + * @xvcu:  Pointer to the xvcu_device structure
> > + *
> > + * Programming the VCU PLL based on the user configuration
> > + * (ref clock freq, core clock freq, mcu clock freq).
> > + * Core clock frequency has higher priority than mcu clock frequency
> > + * Errors in following cases
> > + *- When mcu or clock clock get from logicoreIP is 0
> > + *- When VCU PLL DIV related bits value other than 1
> > + *- When proper data not found for given data
> > + *- When sis570_1 clocksource related operation failed
> > + *
> > + * Return: Returns status, either success or error+reason
> > + */
> > +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu) {
> > +   u32 refclk, coreclk, mcuclk, inte, deci;
> > +   u32 divisor_mcu, divisor_core, fvco;
> > +   u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> > +   u32 cfg_val, mod, ctrl;
> > +   int ret;
> > +   unsigned int i;
> > +   const struct xvcu_pll_cfg *found = NULL;
> > +
> > +   inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> > +   deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> > +   coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> > +   mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> > +   if (!mcuclk || !coreclk) {
> > +   dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> > +   return -EINVAL;
> > +   }
> > +
> > +   refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> > +   dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> > +   dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> > +   dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> > +
> > +   clk_disable_unprepare(xvcu->pll_ref);
> > +   ret = clk_set_rate(xvcu->pll_ref, refclk);
> > +   if (ret)
> > +   dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> > +
> > +   ret = clk_prepare_enable(xvcu->pll_ref);
> > +   if (ret) {
> > +   dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> > +   return ret;
> > +   }
> > +
> > +   refclk = clk_get_rate(xvcu->pll_ref);
> > +
> > +   /*
> > +* The divide-by-2 should be always enabled (==1)
> > +* to meet the timing in the design.
> > +* Otherwise, it's an error
> > +*/
> > +   vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> > +   clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> > +   clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> > +   if (clkoutdiv != 1) {
> 

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Randy Dunlap]

On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> Xilinx ZYNQMP logicoreIP Init driver is based on the new
> LogiCoreIP design created. This driver provides the processing system
> and programmable logic isolation. Set the frequency based on the clock
> information get from the logicoreIP register set.
> 
> It is put in drivers/misc as there is no subsystem for this logicoreIP.
> 
> Signed-off-by: Dhaval Shah 
> ---
> 
>  drivers/misc/Kconfig|  15 ++
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 631 
> 
>  3 files changed, 647 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c

> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
> new file mode 100644
> index 000..f489d34
> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,631 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

[snip]


> +/**
> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> + * @xvcu:Pointer to the xvcu_device structure
> + *
> + * Programming the VCU PLL based on the user configuration
> + * (ref clock freq, core clock freq, mcu clock freq).
> + * Core clock frequency has higher priority than mcu clock frequency
> + * Errors in following cases
> + *- When mcu or clock clock get from logicoreIP is 0
> + *- When VCU PLL DIV related bits value other than 1
> + *- When proper data not found for given data
> + *- When sis570_1 clocksource related operation failed
> + *
> + * Return:   Returns status, either success or error+reason
> + */
> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
> +{
> + u32 refclk, coreclk, mcuclk, inte, deci;
> + u32 divisor_mcu, divisor_core, fvco;
> + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> + u32 cfg_val, mod, ctrl;
> + int ret;
> + unsigned int i;
> + const struct xvcu_pll_cfg *found = NULL;
> +
> + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> + if (!mcuclk || !coreclk) {
> + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> + return -EINVAL;
> + }
> +
> + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> + dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> + dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> + dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> +
> + clk_disable_unprepare(xvcu->pll_ref);
> + ret = clk_set_rate(xvcu->pll_ref, refclk);
> + if (ret)
> + dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> +
> + ret = clk_prepare_enable(xvcu->pll_ref);
> + if (ret) {
> + dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> + return ret;
> + }
> +
> + refclk = clk_get_rate(xvcu->pll_ref);
> +
> + /*
> +  * The divide-by-2 should be always enabled (==1)
> +  * to meet the timing in the design.
> +  * Otherwise, it's an error
> +  */
> + vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> + clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> + clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> + if (clkoutdiv != 1) {
> + dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
> + return -EINVAL;
> + }
> +
> + for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {

When does that for loop terminate?

> + const struct xvcu_pll_cfg *cfg = _pll_cfg[i];
> +
> + fvco = cfg->fbdiv * refclk;
> + if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
> + pll_clk = fvco / VCU_PLL_DIV2;
> + if (fvco % VCU_PLL_DIV2 != 0)
> + pll_clk++;
> + mod = pll_clk % coreclk;
> + if (mod < LIMIT) {
> + divisor_core = pll_clk / coreclk;
> + } else if (coreclk - mod < LIMIT) {
> + divisor_core = pll_clk / coreclk;
> + divisor_core++;
> + } else {
> + continue;
> + }
> + if (divisor_core >= DIVISOR_MIN &&
> + divisor_core <= DIVISOR_MAX) {
> + found = cfg;
> + divisor_mcu = pll_clk / mcuclk;
> + mod = pll_clk % mcuclk;
> + if (mcuclk - mod < LIMIT)
> + 

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Randy Dunlap]

On 12/14/2017 11:24 PM, Dhaval Shah wrote:
> Xilinx ZYNQMP logicoreIP Init driver is based on the new
> LogiCoreIP design created. This driver provides the processing system
> and programmable logic isolation. Set the frequency based on the clock
> information get from the logicoreIP register set.
> 
> It is put in drivers/misc as there is no subsystem for this logicoreIP.
> 
> Signed-off-by: Dhaval Shah 
> ---
> 
>  drivers/misc/Kconfig|  15 ++
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 631 
> 
>  3 files changed, 647 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c

> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
> new file mode 100644
> index 000..f489d34
> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,631 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah 
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

[snip]


> +/**
> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> + * @xvcu:Pointer to the xvcu_device structure
> + *
> + * Programming the VCU PLL based on the user configuration
> + * (ref clock freq, core clock freq, mcu clock freq).
> + * Core clock frequency has higher priority than mcu clock frequency
> + * Errors in following cases
> + *- When mcu or clock clock get from logicoreIP is 0
> + *- When VCU PLL DIV related bits value other than 1
> + *- When proper data not found for given data
> + *- When sis570_1 clocksource related operation failed
> + *
> + * Return:   Returns status, either success or error+reason
> + */
> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
> +{
> + u32 refclk, coreclk, mcuclk, inte, deci;
> + u32 divisor_mcu, divisor_core, fvco;
> + u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> + u32 cfg_val, mod, ctrl;
> + int ret;
> + unsigned int i;
> + const struct xvcu_pll_cfg *found = NULL;
> +
> + inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> + deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> + coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> + mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> + if (!mcuclk || !coreclk) {
> + dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> + return -EINVAL;
> + }
> +
> + refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> + dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> + dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> + dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> +
> + clk_disable_unprepare(xvcu->pll_ref);
> + ret = clk_set_rate(xvcu->pll_ref, refclk);
> + if (ret)
> + dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> +
> + ret = clk_prepare_enable(xvcu->pll_ref);
> + if (ret) {
> + dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> + return ret;
> + }
> +
> + refclk = clk_get_rate(xvcu->pll_ref);
> +
> + /*
> +  * The divide-by-2 should be always enabled (==1)
> +  * to meet the timing in the design.
> +  * Otherwise, it's an error
> +  */
> + vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> + clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> + clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> + if (clkoutdiv != 1) {
> + dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
> + return -EINVAL;
> + }
> +
> + for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {

When does that for loop terminate?

> + const struct xvcu_pll_cfg *cfg = _pll_cfg[i];
> +
> + fvco = cfg->fbdiv * refclk;
> + if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
> + pll_clk = fvco / VCU_PLL_DIV2;
> + if (fvco % VCU_PLL_DIV2 != 0)
> + pll_clk++;
> + mod = pll_clk % coreclk;
> + if (mod < LIMIT) {
> + divisor_core = pll_clk / coreclk;
> + } else if (coreclk - mod < LIMIT) {
> + divisor_core = pll_clk / coreclk;
> + divisor_core++;
> + } else {
> + continue;
> + }
> + if (divisor_core >= DIVISOR_MIN &&
> + divisor_core <= DIVISOR_MAX) {
> + found = cfg;
> + divisor_mcu = pll_clk / mcuclk;
> + mod = pll_clk % mcuclk;
> + if (mcuclk - mod < LIMIT)
> + divisor_mcu++;
> +  

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Arnd Bergmann]

In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah  wrote:
> Xilinx ZYNQMP logicoreIP Init driver is based on the new
> LogiCoreIP design created. This driver provides the processing system
> and programmable logic isolation. Set the frequency based on the clock
> information get from the logicoreIP register set.
>
> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>
> Signed-off-by: Dhaval Shah 

After giving this some more thought, I'd suggest you move the driver to
drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
it merged by Michal Simek as a driver patch that will go through arm-soc.

   Arnd

Re: [PATCH v4 2/2] misc: xlnx_vcu: Add Xilinx ZYNQMP VCU logicoreIP init driver

[Arnd Bergmann]

In Fri, Dec 15, 2017 at 8:24 AM, Dhaval Shah  wrote:
> Xilinx ZYNQMP logicoreIP Init driver is based on the new
> LogiCoreIP design created. This driver provides the processing system
> and programmable logic isolation. Set the frequency based on the clock
> information get from the logicoreIP register set.
>
> It is put in drivers/misc as there is no subsystem for this logicoreIP.
>
> Signed-off-by: Dhaval Shah 

After giving this some more thought, I'd suggest you move the driver to
drivers/soc/xilinx or drivers/soc/zynq instead of drivers/misc/, and have
it merged by Michal Simek as a driver patch that will go through arm-soc.

   Arnd