Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/31/2017 09:27 AM, Jagan Teki wrote: > On Mon, Oct 30, 2017 at 5:12 PM, Marek Vasut wrote: >> On 10/30/2017 12:36 PM, Jagan Teki wrote: >>> On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut wrote: On 10/30/2017 07:04 AM, Jagan Teki wrote: > On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut wrote: >> On 10/27/2017 03:54 PM, Eugeniy Paltsev wrote: >>> On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote: On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: > On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: >> On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: >>> >>> On 10/19/2017 10:51 AM, Marek Vasut wrote: On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: > On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: >> On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin >> wrote: >>> Hi Jagan, >>> -Original Message- From: Eugeniy Paltsev [mailto:palt...@synopsys.com] Sent: Tuesday, October 17, 2017 4:33 PM To: jagannadh.t...@gmail.com Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from Device Tree > > How hard it is to make others to use clock manager? do you > have any list? clock_manager.h is an old (and non-generic) way to deal with different clocks. For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides cm_get_spi_controller_clk_hz function to deal with spi controller clock. But today we have another, linux-like alternative: to bind clocks via device tree and manipulate with clocks via generic functions provided by clk.h In this patch I added option to get clock via device tree using standard bindings and restrict clock_manager.h functions usage only to targets which still use it, so new targets can simply bind clock via device tree and they do not need to implement/define something in clock_manager.h So we don't need to make others to use clock manager :) >>> >>> Maybe it worth trying the other way around and think about >>> switching SOCFPGA platforms to >>> generic clk framework? >> >> Yes, ie what exactly I thought of, thanks! > > I checked cm_get_spi_controller_clk_hz implementation in > SOCFPGA_GEN5 and > SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" > driver as it > manipulate with real hardware. > The only way to do it is to replace SOCFPGA* clock manager > functions by real > clock driver. > > And given I don't have mentioned hardware so I barely can help > with > those improvements on SOCFPGA. That said if there're no > short-term plans to > switch SOCFPGA to clk framework maybe we'll be OK with my > workaround with #ifdefs? Wait for Dinh's reply ... >>> >>> Honestly, I don't have too much time to work on this right now. So I >>> really don't when it can get done. But it'll go on my to-do list. >>> >>> Dinh >> >> Yep, thanks for your comments. >> >> So, Jagan, >> given Dinh's reply, could you please apply this patch? > > I'd really hate it to start seeing soc-specific ifdefs in drivers, > that's IMO not acceptable. A __weak override might be a temporary > solution I'd be willing to live with though. I would rather like to see some check on clock manager itself whether CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use __weak as well soc #ifdefs in driver. >>> >>> Actually I don't understand what do you mean. >>> Even if I add any #ifdefs to the clock_manager.h I still need to wrap >>> clock_manager.h >>> include with #ifdefs as clock_manager.h is defined only for two >>> targets - SOCFPGA_GEN5 and SOCFPGA_ARRIA10. >>> >>> #if defined(CONFIG_TARGET_SOCFPGA_GEN5) || >>> defined(CONFIG_TARGET_SOCFPGA_ARRIA10) >>> #include >>> #endif >>> >>> And I think it is better to add this #ifdef in driver than create empty >>> clock_manager.h file for every new target which uses this driver. > > Chec
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Mon, Oct 30, 2017 at 5:12 PM, Marek Vasut wrote: > On 10/30/2017 12:36 PM, Jagan Teki wrote: >> On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut wrote: >>> On 10/30/2017 07:04 AM, Jagan Teki wrote: On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut wrote: > On 10/27/2017 03:54 PM, Eugeniy Paltsev wrote: >> On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote: >>> On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: > On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: >> >> On 10/19/2017 10:51 AM, Marek Vasut wrote: >>> On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: > On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin > wrote: >> Hi Jagan, >> >>> -Original Message- >>> From: Eugeniy Paltsev [mailto:palt...@synopsys.com] >>> Sent: Tuesday, October 17, 2017 4:33 PM >>> To: jagannadh.t...@gmail.com >>> Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com >>> Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock >>> value from Device Tree How hard it is to make others to use clock manager? do you have any list? >>> >>> clock_manager.h is an old (and non-generic) way to deal with >>> different clocks. >>> For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h >>> provides >>> cm_get_spi_controller_clk_hz function to deal with spi >>> controller clock. >>> >>> But today we have another, linux-like alternative: to bind >>> clocks via device tree >>> and manipulate with clocks via generic functions provided by >>> clk.h >>> >>> In this patch I added option to get clock via device tree using >>> standard bindings >>> and restrict clock_manager.h functions usage only to targets >>> which still use it, >>> so new targets can simply bind clock via device tree and they >>> do not need to >>> implement/define something in clock_manager.h >>> >>> So we don't need to make others to use clock manager :) >> >> Maybe it worth trying the other way around and think about >> switching SOCFPGA platforms to >> generic clk framework? > > Yes, ie what exactly I thought of, thanks! I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 and SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver as it manipulate with real hardware. The only way to do it is to replace SOCFPGA* clock manager functions by real clock driver. And given I don't have mentioned hardware so I barely can help with those improvements on SOCFPGA. That said if there're no short-term plans to switch SOCFPGA to clk framework maybe we'll be OK with my workaround with #ifdefs? >>> >>> Wait for Dinh's reply ... >>> >> >> Honestly, I don't have too much time to work on this right now. So I >> really don't when it can get done. But it'll go on my to-do list. >> >> Dinh > > Yep, thanks for your comments. > > So, Jagan, > given Dinh's reply, could you please apply this patch? I'd really hate it to start seeing soc-specific ifdefs in drivers, that's IMO not acceptable. A __weak override might be a temporary solution I'd be willing to live with though. >>> >>> I would rather like to see some check on clock manager itself whether >>> CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use >>> __weak as well soc #ifdefs in driver. >>> >> >> Actually I don't understand what do you mean. >> Even if I add any #ifdefs to the clock_manager.h I still need to wrap >> clock_manager.h >> include with #ifdefs as clock_manager.h is defined only for two >> targets - SOCFPGA_GEN5 and SOCFPGA_ARRIA10. >> >> #if defined(CONFIG_TARGET_SOCFPGA_GEN5) || >> defined(CONFIG_TARGET_SOCFPGA_ARRIA10) >> #include >> #endif >> >> And I think it is better to add this #ifdef in driver than create empty >> clock_manager.h file for every new target which uses this driver. Check CLK in clock-manager like below, of-course cm_get_l4_noc_hz used in other IP's as well. we need to carefully adding these check and if require add CLK for
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/30/2017 12:36 PM, Jagan Teki wrote: > On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut wrote: >> On 10/30/2017 07:04 AM, Jagan Teki wrote: >>> On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut wrote: On 10/27/2017 03:54 PM, Eugeniy Paltsev wrote: > On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote: >> On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: >>> On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: > > On 10/19/2017 10:51 AM, Marek Vasut wrote: >> On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: >>> On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin wrote: > Hi Jagan, > >> -Original Message- >> From: Eugeniy Paltsev [mailto:palt...@synopsys.com] >> Sent: Tuesday, October 17, 2017 4:33 PM >> To: jagannadh.t...@gmail.com >> Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com >> Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value >> from Device Tree >>> >>> How hard it is to make others to use clock manager? do you have >>> any list? >> >> clock_manager.h is an old (and non-generic) way to deal with >> different clocks. >> For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h >> provides >> cm_get_spi_controller_clk_hz function to deal with spi >> controller clock. >> >> But today we have another, linux-like alternative: to bind >> clocks via device tree >> and manipulate with clocks via generic functions provided by >> clk.h >> >> In this patch I added option to get clock via device tree using >> standard bindings >> and restrict clock_manager.h functions usage only to targets >> which still use it, >> so new targets can simply bind clock via device tree and they do >> not need to >> implement/define something in clock_manager.h >> >> So we don't need to make others to use clock manager :) > > Maybe it worth trying the other way around and think about > switching SOCFPGA platforms to > generic clk framework? Yes, ie what exactly I thought of, thanks! >>> >>> I checked cm_get_spi_controller_clk_hz implementation in >>> SOCFPGA_GEN5 and >>> SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" >>> driver as it >>> manipulate with real hardware. >>> The only way to do it is to replace SOCFPGA* clock manager >>> functions by real >>> clock driver. >>> >>> And given I don't have mentioned hardware so I barely can help with >>> those improvements on SOCFPGA. That said if there're no short-term >>> plans to >>> switch SOCFPGA to clk framework maybe we'll be OK with my >>> workaround with #ifdefs? >> >> Wait for Dinh's reply ... >> > > Honestly, I don't have too much time to work on this right now. So I > really don't when it can get done. But it'll go on my to-do list. > > Dinh Yep, thanks for your comments. So, Jagan, given Dinh's reply, could you please apply this patch? >>> >>> I'd really hate it to start seeing soc-specific ifdefs in drivers, >>> that's IMO not acceptable. A __weak override might be a temporary >>> solution I'd be willing to live with though. >> >> I would rather like to see some check on clock manager itself whether >> CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use >> __weak as well soc #ifdefs in driver. >> > > Actually I don't understand what do you mean. > Even if I add any #ifdefs to the clock_manager.h I still need to wrap > clock_manager.h > include with #ifdefs as clock_manager.h is defined only for two > targets - SOCFPGA_GEN5 and SOCFPGA_ARRIA10. > > #if defined(CONFIG_TARGET_SOCFPGA_GEN5) || > defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > #include > #endif > > And I think it is better to add this #ifdef in driver than create empty > clock_manager.h file for every new target which uses this driver. >>> >>> Check CLK in clock-manager like below, of-course cm_get_l4_noc_hz used >>> in other IP's as well. we need to carefully adding these check and if >>> require add CLK for remaining drivers as well. the reason for adding >>> checks in clock-manager is it may be removable code if all use CLK. >> >> I do not understand what you're tryin
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut wrote: > On 10/30/2017 07:04 AM, Jagan Teki wrote: >> On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut wrote: >>> On 10/27/2017 03:54 PM, Eugeniy Paltsev wrote: On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote: > On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: >> On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: >>> On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: On 10/19/2017 10:51 AM, Marek Vasut wrote: > On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: >> On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: >>> On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin >>> wrote: Hi Jagan, > -Original Message- > From: Eugeniy Paltsev [mailto:palt...@synopsys.com] > Sent: Tuesday, October 17, 2017 4:33 PM > To: jagannadh.t...@gmail.com > Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com > Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value > from Device Tree >> >> How hard it is to make others to use clock manager? do you have >> any list? > > clock_manager.h is an old (and non-generic) way to deal with > different clocks. > For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h > provides > cm_get_spi_controller_clk_hz function to deal with spi controller > clock. > > But today we have another, linux-like alternative: to bind clocks > via device tree > and manipulate with clocks via generic functions provided by clk.h > > In this patch I added option to get clock via device tree using > standard bindings > and restrict clock_manager.h functions usage only to targets > which still use it, > so new targets can simply bind clock via device tree and they do > not need to > implement/define something in clock_manager.h > > So we don't need to make others to use clock manager :) Maybe it worth trying the other way around and think about switching SOCFPGA platforms to generic clk framework? >>> >>> Yes, ie what exactly I thought of, thanks! >> >> I checked cm_get_spi_controller_clk_hz implementation in >> SOCFPGA_GEN5 and >> SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" >> driver as it >> manipulate with real hardware. >> The only way to do it is to replace SOCFPGA* clock manager functions >> by real >> clock driver. >> >> And given I don't have mentioned hardware so I barely can help with >> those improvements on SOCFPGA. That said if there're no short-term >> plans to >> switch SOCFPGA to clk framework maybe we'll be OK with my workaround >> with #ifdefs? > > Wait for Dinh's reply ... > Honestly, I don't have too much time to work on this right now. So I really don't when it can get done. But it'll go on my to-do list. Dinh >>> >>> Yep, thanks for your comments. >>> >>> So, Jagan, >>> given Dinh's reply, could you please apply this patch? >> >> I'd really hate it to start seeing soc-specific ifdefs in drivers, >> that's IMO not acceptable. A __weak override might be a temporary >> solution I'd be willing to live with though. > > I would rather like to see some check on clock manager itself whether > CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use > __weak as well soc #ifdefs in driver. > Actually I don't understand what do you mean. Even if I add any #ifdefs to the clock_manager.h I still need to wrap clock_manager.h include with #ifdefs as clock_manager.h is defined only for two targets - SOCFPGA_GEN5 and SOCFPGA_ARRIA10. #if defined(CONFIG_TARGET_SOCFPGA_GEN5) || defined(CONFIG_TARGET_SOCFPGA_ARRIA10) #include #endif And I think it is better to add this #ifdef in driver than create empty clock_manager.h file for every new target which uses this driver. >> >> Check CLK in clock-manager like below, of-course cm_get_l4_noc_hz used >> in other IP's as well. we need to carefully adding these check and if >> require add CLK for remaining drivers as well. the reason for adding >> checks in clock-manager is it may be removable code if all use CLK. > > I do not understand what you're trying to tell me, but the patch below > makes no sense. What I would like to see is a weak function in the > driver and that function be overriden in the socfpga clock manage
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/30/2017 07:04 AM, Jagan Teki wrote: > On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut wrote: >> On 10/27/2017 03:54 PM, Eugeniy Paltsev wrote: >>> On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote: On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: > On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: >> On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: >>> >>> On 10/19/2017 10:51 AM, Marek Vasut wrote: On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: > On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: >> On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin >> wrote: >>> Hi Jagan, >>> -Original Message- From: Eugeniy Paltsev [mailto:palt...@synopsys.com] Sent: Tuesday, October 17, 2017 4:33 PM To: jagannadh.t...@gmail.com Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from Device Tree > > How hard it is to make others to use clock manager? do you have > any list? clock_manager.h is an old (and non-generic) way to deal with different clocks. For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides cm_get_spi_controller_clk_hz function to deal with spi controller clock. But today we have another, linux-like alternative: to bind clocks via device tree and manipulate with clocks via generic functions provided by clk.h In this patch I added option to get clock via device tree using standard bindings and restrict clock_manager.h functions usage only to targets which still use it, so new targets can simply bind clock via device tree and they do not need to implement/define something in clock_manager.h So we don't need to make others to use clock manager :) >>> >>> Maybe it worth trying the other way around and think about >>> switching SOCFPGA platforms to >>> generic clk framework? >> >> Yes, ie what exactly I thought of, thanks! > > I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 > and > SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver > as it > manipulate with real hardware. > The only way to do it is to replace SOCFPGA* clock manager functions > by real > clock driver. > > And given I don't have mentioned hardware so I barely can help with > those improvements on SOCFPGA. That said if there're no short-term > plans to > switch SOCFPGA to clk framework maybe we'll be OK with my workaround > with #ifdefs? Wait for Dinh's reply ... >>> >>> Honestly, I don't have too much time to work on this right now. So I >>> really don't when it can get done. But it'll go on my to-do list. >>> >>> Dinh >> >> Yep, thanks for your comments. >> >> So, Jagan, >> given Dinh's reply, could you please apply this patch? > > I'd really hate it to start seeing soc-specific ifdefs in drivers, > that's IMO not acceptable. A __weak override might be a temporary > solution I'd be willing to live with though. I would rather like to see some check on clock manager itself whether CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use __weak as well soc #ifdefs in driver. >>> >>> Actually I don't understand what do you mean. >>> Even if I add any #ifdefs to the clock_manager.h I still need to wrap >>> clock_manager.h >>> include with #ifdefs as clock_manager.h is defined only for two >>> targets - SOCFPGA_GEN5 and SOCFPGA_ARRIA10. >>> >>> #if defined(CONFIG_TARGET_SOCFPGA_GEN5) || >>> defined(CONFIG_TARGET_SOCFPGA_ARRIA10) >>> #include >>> #endif >>> >>> And I think it is better to add this #ifdef in driver than create empty >>> clock_manager.h file for every new target which uses this driver. > > Check CLK in clock-manager like below, of-course cm_get_l4_noc_hz used > in other IP's as well. we need to carefully adding these check and if > require add CLK for remaining drivers as well. the reason for adding > checks in clock-manager is it may be removable code if all use CLK. I do not understand what you're trying to tell me, but the patch below makes no sense. What I would like to see is a weak function in the driver and that function be overriden in the socfpga clock manager. > --- a/arch/arm/mach-socfpga/clock_manager_arria10.c > +++ b/arch/arm/mach-socfpga/clock_manager_arria10.c > @@ -1066,10 +1066,17 @@ unsigned int cm_get_mmc_contro
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut wrote:
> On 10/27/2017 03:54 PM, Eugeniy Paltsev wrote:
>> On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote:
>>> On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote:
On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote:
> On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote:
>>
>> On 10/19/2017 10:51 AM, Marek Vasut wrote:
>>> On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote:
On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote:
> On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin
> wrote:
>> Hi Jagan,
>>
>>> -Original Message-
>>> From: Eugeniy Paltsev [mailto:palt...@synopsys.com]
>>> Sent: Tuesday, October 17, 2017 4:33 PM
>>> To: jagannadh.t...@gmail.com
>>> Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com
>>> Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value
>>> from Device Tree
How hard it is to make others to use clock manager? do you have
any list?
>>>
>>> clock_manager.h is an old (and non-generic) way to deal with
>>> different clocks.
>>> For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h
>>> provides
>>> cm_get_spi_controller_clk_hz function to deal with spi controller
>>> clock.
>>>
>>> But today we have another, linux-like alternative: to bind clocks
>>> via device tree
>>> and manipulate with clocks via generic functions provided by clk.h
>>>
>>> In this patch I added option to get clock via device tree using
>>> standard bindings
>>> and restrict clock_manager.h functions usage only to targets which
>>> still use it,
>>> so new targets can simply bind clock via device tree and they do
>>> not need to
>>> implement/define something in clock_manager.h
>>>
>>> So we don't need to make others to use clock manager :)
>>
>> Maybe it worth trying the other way around and think about switching
>> SOCFPGA platforms to
>> generic clk framework?
>
> Yes, ie what exactly I thought of, thanks!
I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5
and
SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver
as it
manipulate with real hardware.
The only way to do it is to replace SOCFPGA* clock manager functions
by real
clock driver.
And given I don't have mentioned hardware so I barely can help with
those improvements on SOCFPGA. That said if there're no short-term
plans to
switch SOCFPGA to clk framework maybe we'll be OK with my workaround
with #ifdefs?
>>>
>>> Wait for Dinh's reply ...
>>>
>>
>> Honestly, I don't have too much time to work on this right now. So I
>> really don't when it can get done. But it'll go on my to-do list.
>>
>> Dinh
>
> Yep, thanks for your comments.
>
> So, Jagan,
> given Dinh's reply, could you please apply this patch?
I'd really hate it to start seeing soc-specific ifdefs in drivers,
that's IMO not acceptable. A __weak override might be a temporary
solution I'd be willing to live with though.
>>>
>>> I would rather like to see some check on clock manager itself whether
>>> CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use
>>> __weak as well soc #ifdefs in driver.
>>>
>>
>> Actually I don't understand what do you mean.
>> Even if I add any #ifdefs to the clock_manager.h I still need to wrap
>> clock_manager.h
>> include with #ifdefs as clock_manager.h is defined only for two
>> targets - SOCFPGA_GEN5 and SOCFPGA_ARRIA10.
>>
>> #if defined(CONFIG_TARGET_SOCFPGA_GEN5) ||
>> defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
>> #include
>> #endif
>>
>> And I think it is better to add this #ifdef in driver than create empty
>> clock_manager.h file for every new target which uses this driver.
Check CLK in clock-manager like below, of-course cm_get_l4_noc_hz used
in other IP's as well. we need to carefully adding these check and if
require add CLK for remaining drivers as well. the reason for adding
checks in clock-manager is it may be removable code if all use CLK.
--- a/arch/arm/mach-socfpga/clock_manager_arria10.c
+++ b/arch/arm/mach-socfpga/clock_manager_arria10.c
@@ -1066,10 +1066,17 @@ unsigned int cm_get_mmc_controller_clk_hz(void)
return clk_hz / 4;
}
+#if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(CLK)
+unsigned int cm_get_spi_controller_clk_hz(void)
+{
+ return 0;
+}
+#else
unsigned int cm_get_spi_controller_clk_hz(void)
{
return cm_get_l4_noc_hz(CLKMGR_MAINPLL_NOCDIV_L4MPCLK_LSB);
}
+#endif
thanks!
--
Jagan Teki
Free Software Engineer
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/27/2017 03:54 PM, Eugeniy Paltsev wrote: > On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote: >> On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: >>> On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: > > On 10/19/2017 10:51 AM, Marek Vasut wrote: >> On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: >>> On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin wrote: > Hi Jagan, > >> -Original Message- >> From: Eugeniy Paltsev [mailto:palt...@synopsys.com] >> Sent: Tuesday, October 17, 2017 4:33 PM >> To: jagannadh.t...@gmail.com >> Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com >> Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value >> from Device Tree >>> >>> How hard it is to make others to use clock manager? do you have any >>> list? >> >> clock_manager.h is an old (and non-generic) way to deal with >> different clocks. >> For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h >> provides >> cm_get_spi_controller_clk_hz function to deal with spi controller >> clock. >> >> But today we have another, linux-like alternative: to bind clocks >> via device tree >> and manipulate with clocks via generic functions provided by clk.h >> >> In this patch I added option to get clock via device tree using >> standard bindings >> and restrict clock_manager.h functions usage only to targets which >> still use it, >> so new targets can simply bind clock via device tree and they do not >> need to >> implement/define something in clock_manager.h >> >> So we don't need to make others to use clock manager :) > > Maybe it worth trying the other way around and think about switching > SOCFPGA platforms to > generic clk framework? Yes, ie what exactly I thought of, thanks! >>> >>> I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 >>> and >>> SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver >>> as it >>> manipulate with real hardware. >>> The only way to do it is to replace SOCFPGA* clock manager functions by >>> real >>> clock driver. >>> >>> And given I don't have mentioned hardware so I barely can help with >>> those improvements on SOCFPGA. That said if there're no short-term >>> plans to >>> switch SOCFPGA to clk framework maybe we'll be OK with my workaround >>> with #ifdefs? >> >> Wait for Dinh's reply ... >> > > Honestly, I don't have too much time to work on this right now. So I > really don't when it can get done. But it'll go on my to-do list. > > Dinh Yep, thanks for your comments. So, Jagan, given Dinh's reply, could you please apply this patch? >>> >>> I'd really hate it to start seeing soc-specific ifdefs in drivers, >>> that's IMO not acceptable. A __weak override might be a temporary >>> solution I'd be willing to live with though. >> >> I would rather like to see some check on clock manager itself whether >> CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use >> __weak as well soc #ifdefs in driver. >> > > Actually I don't understand what do you mean. > Even if I add any #ifdefs to the clock_manager.h I still need to wrap > clock_manager.h > include with #ifdefs as clock_manager.h is defined only for two > targets - SOCFPGA_GEN5 and SOCFPGA_ARRIA10. > > #if defined(CONFIG_TARGET_SOCFPGA_GEN5) || > defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > #include > #endif > > And I think it is better to add this #ifdef in driver than create empty > clock_manager.h file for every new target which uses this driver. If you have weak default implementation in the DW SPI driver of some function to get the clock rate and override it in the socfpga clock manager, then you don't need to use ifdefs . -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote: > On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: > > On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: > > > On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: > > > > > > > > On 10/19/2017 10:51 AM, Marek Vasut wrote: > > > > > On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: > > > > > > On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: > > > > > > > On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin > > > > > > > wrote: > > > > > > > > Hi Jagan, > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > > > From: Eugeniy Paltsev [mailto:palt...@synopsys.com] > > > > > > > > > Sent: Tuesday, October 17, 2017 4:33 PM > > > > > > > > > To: jagannadh.t...@gmail.com > > > > > > > > > Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com > > > > > > > > > Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock > > > > > > > > > value from Device Tree > > > > > > > > > > > > > > > > > > > > How hard it is to make others to use clock manager? do you > > > > > > > > > > have any list? > > > > > > > > > > > > > > > > > > clock_manager.h is an old (and non-generic) way to deal with > > > > > > > > > different clocks. > > > > > > > > > For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 > > > > > > > > > clock_manager.h provides > > > > > > > > > cm_get_spi_controller_clk_hz function to deal with spi > > > > > > > > > controller clock. > > > > > > > > > > > > > > > > > > But today we have another, linux-like alternative: to bind > > > > > > > > > clocks via device tree > > > > > > > > > and manipulate with clocks via generic functions provided by > > > > > > > > > clk.h > > > > > > > > > > > > > > > > > > In this patch I added option to get clock via device tree > > > > > > > > > using standard bindings > > > > > > > > > and restrict clock_manager.h functions usage only to targets > > > > > > > > > which still use it, > > > > > > > > > so new targets can simply bind clock via device tree and they > > > > > > > > > do not need to > > > > > > > > > implement/define something in clock_manager.h > > > > > > > > > > > > > > > > > > So we don't need to make others to use clock manager :) > > > > > > > > > > > > > > > > Maybe it worth trying the other way around and think about > > > > > > > > switching SOCFPGA platforms to > > > > > > > > generic clk framework? > > > > > > > > > > > > > > Yes, ie what exactly I thought of, thanks! > > > > > > > > > > > > I checked cm_get_spi_controller_clk_hz implementation in > > > > > > SOCFPGA_GEN5 and > > > > > > SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" > > > > > > driver as it > > > > > > manipulate with real hardware. > > > > > > The only way to do it is to replace SOCFPGA* clock manager > > > > > > functions by real > > > > > > clock driver. > > > > > > > > > > > > And given I don't have mentioned hardware so I barely can help with > > > > > > those improvements on SOCFPGA. That said if there're no short-term > > > > > > plans to > > > > > > switch SOCFPGA to clk framework maybe we'll be OK with my > > > > > > workaround with #ifdefs? > > > > > > > > > > Wait for Dinh's reply ... > > > > > > > > > > > > > Honestly, I don't have too much time to work on this right now. So I > > > > really don't when it can get done. But it'll go on my to-do list. > > > > > > > > Dinh > > > > > > Yep, thanks for your comments. > > > > > > So, Jagan, > > > given Dinh's reply, could you please apply this patch? > > > > I'd really hate it to start seeing soc-specific ifdefs in drivers, > > that's IMO not acceptable. A __weak override might be a temporary > > solution I'd be willing to live with though. > > I would rather like to see some check on clock manager itself whether > CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use > __weak as well soc #ifdefs in driver. > Actually I don't understand what do you mean. Even if I add any #ifdefs to the clock_manager.h I still need to wrap clock_manager.h include with #ifdefs as clock_manager.h is defined only for two targets - SOCFPGA_GEN5 and SOCFPGA_ARRIA10. #if defined(CONFIG_TARGET_SOCFPGA_GEN5) || defined(CONFIG_TARGET_SOCFPGA_ARRIA10) #include #endif And I think it is better to add this #ifdef in driver than create empty clock_manager.h file for every new target which uses this driver. -- Eugeniy Paltsev ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/24/2017 11:52 AM, Jagan Teki wrote: > On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: >> On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: >>> On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: On 10/19/2017 10:51 AM, Marek Vasut wrote: > On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: >> On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: >>> On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin >>> wrote: Hi Jagan, > -Original Message- > From: Eugeniy Paltsev [mailto:palt...@synopsys.com] > Sent: Tuesday, October 17, 2017 4:33 PM > To: jagannadh.t...@gmail.com > Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com > Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from > Device Tree >> >> How hard it is to make others to use clock manager? do you have any >> list? > > clock_manager.h is an old (and non-generic) way to deal with > different clocks. > For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h > provides > cm_get_spi_controller_clk_hz function to deal with spi controller > clock. > > But today we have another, linux-like alternative: to bind clocks via > device tree > and manipulate with clocks via generic functions provided by clk.h > > In this patch I added option to get clock via device tree using > standard bindings > and restrict clock_manager.h functions usage only to targets which > still use it, > so new targets can simply bind clock via device tree and they do not > need to > implement/define something in clock_manager.h > > So we don't need to make others to use clock manager :) Maybe it worth trying the other way around and think about switching SOCFPGA platforms to generic clk framework? >>> >>> Yes, ie what exactly I thought of, thanks! >> >> I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 and >> SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver as >> it >> manipulate with real hardware. >> The only way to do it is to replace SOCFPGA* clock manager functions by >> real >> clock driver. >> >> And given I don't have mentioned hardware so I barely can help with >> those improvements on SOCFPGA. That said if there're no short-term plans >> to >> switch SOCFPGA to clk framework maybe we'll be OK with my workaround >> with #ifdefs? > > Wait for Dinh's reply ... > Honestly, I don't have too much time to work on this right now. So I really don't when it can get done. But it'll go on my to-do list. Dinh >>> >>> Yep, thanks for your comments. >>> >>> So, Jagan, >>> given Dinh's reply, could you please apply this patch? >> >> I'd really hate it to start seeing soc-specific ifdefs in drivers, >> that's IMO not acceptable. A __weak override might be a temporary >> solution I'd be willing to live with though. > > I would rather like to see some check on clock manager itself whether > CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use > __weak as well soc #ifdefs in driver. I don't understand what you mean by this , can you elucidate ? -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut wrote: > On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: >> On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: >>> >>> On 10/19/2017 10:51 AM, Marek Vasut wrote: On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: > On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: >> On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin >> wrote: >>> Hi Jagan, >>> -Original Message- From: Eugeniy Paltsev [mailto:palt...@synopsys.com] Sent: Tuesday, October 17, 2017 4:33 PM To: jagannadh.t...@gmail.com Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from Device Tree > > How hard it is to make others to use clock manager? do you have any > list? clock_manager.h is an old (and non-generic) way to deal with different clocks. For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides cm_get_spi_controller_clk_hz function to deal with spi controller clock. But today we have another, linux-like alternative: to bind clocks via device tree and manipulate with clocks via generic functions provided by clk.h In this patch I added option to get clock via device tree using standard bindings and restrict clock_manager.h functions usage only to targets which still use it, so new targets can simply bind clock via device tree and they do not need to implement/define something in clock_manager.h So we don't need to make others to use clock manager :) >>> >>> Maybe it worth trying the other way around and think about switching >>> SOCFPGA platforms to >>> generic clk framework? >> >> Yes, ie what exactly I thought of, thanks! > > I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 and > SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver as > it > manipulate with real hardware. > The only way to do it is to replace SOCFPGA* clock manager functions by > real > clock driver. > > And given I don't have mentioned hardware so I barely can help with > those improvements on SOCFPGA. That said if there're no short-term plans > to > switch SOCFPGA to clk framework maybe we'll be OK with my workaround with > #ifdefs? Wait for Dinh's reply ... >>> >>> Honestly, I don't have too much time to work on this right now. So I >>> really don't when it can get done. But it'll go on my to-do list. >>> >>> Dinh >> >> Yep, thanks for your comments. >> >> So, Jagan, >> given Dinh's reply, could you please apply this patch? > > I'd really hate it to start seeing soc-specific ifdefs in drivers, > that's IMO not acceptable. A __weak override might be a temporary > solution I'd be willing to live with though. I would rather like to see some check on clock manager itself whether CONFIG_IS_ENABLED(CLK) is using or not? this can tends not to use __weak as well soc #ifdefs in driver. thanks! -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/23/2017 01:43 PM, Eugeniy Paltsev wrote: > On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: >> >> On 10/19/2017 10:51 AM, Marek Vasut wrote: >>> On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: > On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin > wrote: >> Hi Jagan, >> >>> -Original Message- >>> From: Eugeniy Paltsev [mailto:palt...@synopsys.com] >>> Sent: Tuesday, October 17, 2017 4:33 PM >>> To: jagannadh.t...@gmail.com >>> Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com >>> Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from >>> Device Tree How hard it is to make others to use clock manager? do you have any list? >>> >>> clock_manager.h is an old (and non-generic) way to deal with different >>> clocks. >>> For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides >>> cm_get_spi_controller_clk_hz function to deal with spi controller clock. >>> >>> But today we have another, linux-like alternative: to bind clocks via >>> device tree >>> and manipulate with clocks via generic functions provided by clk.h >>> >>> In this patch I added option to get clock via device tree using >>> standard bindings >>> and restrict clock_manager.h functions usage only to targets which >>> still use it, >>> so new targets can simply bind clock via device tree and they do not >>> need to >>> implement/define something in clock_manager.h >>> >>> So we don't need to make others to use clock manager :) >> >> Maybe it worth trying the other way around and think about switching >> SOCFPGA platforms to >> generic clk framework? > > Yes, ie what exactly I thought of, thanks! I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 and SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver as it manipulate with real hardware. The only way to do it is to replace SOCFPGA* clock manager functions by real clock driver. And given I don't have mentioned hardware so I barely can help with those improvements on SOCFPGA. That said if there're no short-term plans to switch SOCFPGA to clk framework maybe we'll be OK with my workaround with #ifdefs? >>> >>> Wait for Dinh's reply ... >>> >> >> Honestly, I don't have too much time to work on this right now. So I >> really don't when it can get done. But it'll go on my to-do list. >> >> Dinh > > Yep, thanks for your comments. > > So, Jagan, > given Dinh's reply, could you please apply this patch? I'd really hate it to start seeing soc-specific ifdefs in drivers, that's IMO not acceptable. A __weak override might be a temporary solution I'd be willing to live with though. -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Thu, 2017-10-19 at 13:20 -0500, Dinh Nguyen wrote: > > On 10/19/2017 10:51 AM, Marek Vasut wrote: > > On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: > > > On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: > > > > On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin > > > > wrote: > > > > > Hi Jagan, > > > > > > > > > > > -Original Message- > > > > > > From: Eugeniy Paltsev [mailto:palt...@synopsys.com] > > > > > > Sent: Tuesday, October 17, 2017 4:33 PM > > > > > > To: jagannadh.t...@gmail.com > > > > > > Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com > > > > > > Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value > > > > > > from Device Tree > > > > > > > > > > > > > > How hard it is to make others to use clock manager? do you have > > > > > > > any list? > > > > > > > > > > > > clock_manager.h is an old (and non-generic) way to deal with > > > > > > different clocks. > > > > > > For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h > > > > > > provides > > > > > > cm_get_spi_controller_clk_hz function to deal with spi controller > > > > > > clock. > > > > > > > > > > > > But today we have another, linux-like alternative: to bind clocks > > > > > > via device tree > > > > > > and manipulate with clocks via generic functions provided by clk.h > > > > > > > > > > > > In this patch I added option to get clock via device tree using > > > > > > standard bindings > > > > > > and restrict clock_manager.h functions usage only to targets which > > > > > > still use it, > > > > > > so new targets can simply bind clock via device tree and they do > > > > > > not need to > > > > > > implement/define something in clock_manager.h > > > > > > > > > > > > So we don't need to make others to use clock manager :) > > > > > > > > > > Maybe it worth trying the other way around and think about switching > > > > > SOCFPGA platforms to > > > > > generic clk framework? > > > > > > > > Yes, ie what exactly I thought of, thanks! > > > > > > I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 and > > > SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver as > > > it > > > manipulate with real hardware. > > > The only way to do it is to replace SOCFPGA* clock manager functions by > > > real > > > clock driver. > > > > > > And given I don't have mentioned hardware so I barely can help with > > > those improvements on SOCFPGA. That said if there're no short-term plans > > > to > > > switch SOCFPGA to clk framework maybe we'll be OK with my workaround with > > > #ifdefs? > > > > Wait for Dinh's reply ... > > > > Honestly, I don't have too much time to work on this right now. So I > really don't when it can get done. But it'll go on my to-do list. > > Dinh Yep, thanks for your comments. So, Jagan, given Dinh's reply, could you please apply this patch? Thanks. -- Eugeniy Paltsev ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/19/2017 10:51 AM, Marek Vasut wrote: > On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: >> On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: >>> On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin >>> wrote: Hi Jagan, > -Original Message- > From: Eugeniy Paltsev [mailto:palt...@synopsys.com] > Sent: Tuesday, October 17, 2017 4:33 PM > To: jagannadh.t...@gmail.com > Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com > Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from > Device Tree >> >> How hard it is to make others to use clock manager? do you have any list? > > clock_manager.h is an old (and non-generic) way to deal with different > clocks. > For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides > cm_get_spi_controller_clk_hz function to deal with spi controller clock. > > But today we have another, linux-like alternative: to bind clocks via > device tree > and manipulate with clocks via generic functions provided by clk.h > > In this patch I added option to get clock via device tree using standard > bindings > and restrict clock_manager.h functions usage only to targets which still > use it, > so new targets can simply bind clock via device tree and they do not need > to > implement/define something in clock_manager.h > > So we don't need to make others to use clock manager :) Maybe it worth trying the other way around and think about switching SOCFPGA platforms to generic clk framework? >>> >>> Yes, ie what exactly I thought of, thanks! >> >> I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 and >> SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver as it >> manipulate with real hardware. >> The only way to do it is to replace SOCFPGA* clock manager functions by real >> clock driver. >> >> And given I don't have mentioned hardware so I barely can help with >> those improvements on SOCFPGA. That said if there're no short-term plans to >> switch SOCFPGA to clk framework maybe we'll be OK with my workaround with >> #ifdefs? > > Wait for Dinh's reply ... > Honestly, I don't have too much time to work on this right now. So I really don't when it can get done. But it'll go on my to-do list. Dinh ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/19/2017 05:36 PM, Eugeniy Paltsev wrote: > On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: >> On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin >> wrote: >>> Hi Jagan, >>> -Original Message- From: Eugeniy Paltsev [mailto:palt...@synopsys.com] Sent: Tuesday, October 17, 2017 4:33 PM To: jagannadh.t...@gmail.com Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from Device Tree > > How hard it is to make others to use clock manager? do you have any list? clock_manager.h is an old (and non-generic) way to deal with different clocks. For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides cm_get_spi_controller_clk_hz function to deal with spi controller clock. But today we have another, linux-like alternative: to bind clocks via device tree and manipulate with clocks via generic functions provided by clk.h In this patch I added option to get clock via device tree using standard bindings and restrict clock_manager.h functions usage only to targets which still use it, so new targets can simply bind clock via device tree and they do not need to implement/define something in clock_manager.h So we don't need to make others to use clock manager :) >>> >>> Maybe it worth trying the other way around and think about switching >>> SOCFPGA platforms to >>> generic clk framework? >> >> Yes, ie what exactly I thought of, thanks! > > I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 and > SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver as it > manipulate with real hardware. > The only way to do it is to replace SOCFPGA* clock manager functions by real > clock driver. > > And given I don't have mentioned hardware so I barely can help with > those improvements on SOCFPGA. That said if there're no short-term plans to > switch SOCFPGA to clk framework maybe we'll be OK with my workaround with > #ifdefs? Wait for Dinh's reply ... -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: > On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin > wrote: > > Hi Jagan, > > > > > -Original Message- > > > From: Eugeniy Paltsev [mailto:palt...@synopsys.com] > > > Sent: Tuesday, October 17, 2017 4:33 PM > > > To: jagannadh.t...@gmail.com > > > Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com > > > Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from > > > Device Tree > > > > > > > > How hard it is to make others to use clock manager? do you have any > > > > list? > > > > > > clock_manager.h is an old (and non-generic) way to deal with different > > > clocks. > > > For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides > > > cm_get_spi_controller_clk_hz function to deal with spi controller clock. > > > > > > But today we have another, linux-like alternative: to bind clocks via > > > device tree > > > and manipulate with clocks via generic functions provided by clk.h > > > > > > In this patch I added option to get clock via device tree using standard > > > bindings > > > and restrict clock_manager.h functions usage only to targets which still > > > use it, > > > so new targets can simply bind clock via device tree and they do not need > > > to > > > implement/define something in clock_manager.h > > > > > > So we don't need to make others to use clock manager :) > > > > Maybe it worth trying the other way around and think about switching > > SOCFPGA platforms to > > generic clk framework? > > Yes, ie what exactly I thought of, thanks! I checked cm_get_spi_controller_clk_hz implementation in SOCFPGA_GEN5 and SOCFPGA_ARRIA10: we can't simply replace it with "fixed-clock" driver as it manipulate with real hardware. The only way to do it is to replace SOCFPGA* clock manager functions by real clock driver. And given I don't have mentioned hardware so I barely can help with those improvements on SOCFPGA. That said if there're no short-term plans to switch SOCFPGA to clk framework maybe we'll be OK with my workaround with #ifdefs? -- Eugeniy Paltsev ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin wrote: > Hi Jagan, > >> -Original Message- >> From: Eugeniy Paltsev [mailto:palt...@synopsys.com] >> Sent: Tuesday, October 17, 2017 4:33 PM >> To: jagannadh.t...@gmail.com >> Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com >> Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from Device >> Tree >> > >> > How hard it is to make others to use clock manager? do you have any list? >> >> clock_manager.h is an old (and non-generic) way to deal with different >> clocks. >> For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides >> cm_get_spi_controller_clk_hz function to deal with spi controller clock. >> >> But today we have another, linux-like alternative: to bind clocks via device >> tree >> and manipulate with clocks via generic functions provided by clk.h >> >> In this patch I added option to get clock via device tree using standard >> bindings >> and restrict clock_manager.h functions usage only to targets which still use >> it, >> so new targets can simply bind clock via device tree and they do not need to >> implement/define something in clock_manager.h >> >> So we don't need to make others to use clock manager :) > > Maybe it worth trying the other way around and think about switching SOCFPGA > platforms to > generic clk framework? Yes, ie what exactly I thought of, thanks! -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On 10/17/2017 04:57 PM, Alexey Brodkin wrote: > Hi Jagan, > >> -Original Message- >> From: Eugeniy Paltsev [mailto:palt...@synopsys.com] >> Sent: Tuesday, October 17, 2017 4:33 PM >> To: jagannadh.t...@gmail.com >> Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com >> Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from Device >> Tree >>> >>> How hard it is to make others to use clock manager? do you have any list? >> >> clock_manager.h is an old (and non-generic) way to deal with different >> clocks. >> For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides >> cm_get_spi_controller_clk_hz function to deal with spi controller clock. >> >> But today we have another, linux-like alternative: to bind clocks via device >> tree >> and manipulate with clocks via generic functions provided by clk.h >> >> In this patch I added option to get clock via device tree using standard >> bindings >> and restrict clock_manager.h functions usage only to targets which still use >> it, >> so new targets can simply bind clock via device tree and they do not need to >> implement/define something in clock_manager.h >> >> So we don't need to make others to use clock manager :) > > Maybe it worth trying the other way around and think about switching SOCFPGA > platforms to > generic clk framework? That'd be real neat. > Marek, any plans for that? Patches welcome, +CC Dinh. > -Alexey > -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
Hi Jagan, > -Original Message- > From: Eugeniy Paltsev [mailto:palt...@synopsys.com] > Sent: Tuesday, October 17, 2017 4:33 PM > To: jagannadh.t...@gmail.com > Cc: u-boot@lists.denx.de; uboot-snps-...@synopsys.com > Subject: [uboot-snps-arc] Re: [PATCH v2] DW SPI: Get clock value from Device > Tree > > > > How hard it is to make others to use clock manager? do you have any list? > > clock_manager.h is an old (and non-generic) way to deal with different clocks. > For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides > cm_get_spi_controller_clk_hz function to deal with spi controller clock. > > But today we have another, linux-like alternative: to bind clocks via device > tree > and manipulate with clocks via generic functions provided by clk.h > > In this patch I added option to get clock via device tree using standard > bindings > and restrict clock_manager.h functions usage only to targets which still use > it, > so new targets can simply bind clock via device tree and they do not need to > implement/define something in clock_manager.h > > So we don't need to make others to use clock manager :) Maybe it worth trying the other way around and think about switching SOCFPGA platforms to generic clk framework? Marek, any plans for that? -Alexey ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
Hi Jagan, On Mon, 2017-10-16 at 22:37 +0530, Jagan Teki wrote: > On Fri, Oct 13, 2017 at 8:48 PM, Eugeniy Paltsev > wrote: > > Add option to set spi controller clock frequency via device tree > > using standard clock bindings. > > Old way of setting spi controller clock frequency (via implementation > > of 'cm_get_spi_controller_clk_hz' function in platform specific code) > > remains supported for backward compatibility. > > > > Signed-off-by: Eugeniy Paltsev > > --- > > Changes v1->v2: > > * disable clock if we can't get the rate. > > * get rid of cm_get_spi_controller_clk_hz weak declaration. > > > > drivers/spi/designware_spi.c | 65 > > +++- > > 1 file changed, 64 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c > > index 5aa507b..9eb5b1c 100644 > > --- a/drivers/spi/designware_spi.c > > +++ b/drivers/spi/designware_spi.c > > @@ -11,6 +11,7 @@ > > */ > > > > #include > > +#include > > #include > > #include > > #include > > @@ -18,7 +19,10 @@ > > #include > > #include > > #include > > +/* Only SOCFPGA_GEN5 and SOCFPGA_ARRIA10 uses their clock_manager > > functions */ > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) || > > defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > > #include > > +#endif > > How hard it is to make others to use clock manager? do you have any list? clock_manager.h is an old (and non-generic) way to deal with different clocks. For example in SOCFPGA_GEN5 and SOCFPGA_ARRIA10 clock_manager.h provides cm_get_spi_controller_clk_hz function to deal with spi controller clock. But today we have another, linux-like alternative: to bind clocks via device tree and manipulate with clocks via generic functions provided by clk.h In this patch I added option to get clock via device tree using standard bindings and restrict clock_manager.h functions usage only to targets which still use it, so new targets can simply bind clock via device tree and they do not need to implement/define something in clock_manager.h So we don't need to make others to use clock manager :) > thanks! -- Eugeniy Paltsev ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
On Fri, Oct 13, 2017 at 8:48 PM, Eugeniy Paltsev wrote: > Add option to set spi controller clock frequency via device tree > using standard clock bindings. > Old way of setting spi controller clock frequency (via implementation > of 'cm_get_spi_controller_clk_hz' function in platform specific code) > remains supported for backward compatibility. > > Signed-off-by: Eugeniy Paltsev > --- > Changes v1->v2: > * disable clock if we can't get the rate. > * get rid of cm_get_spi_controller_clk_hz weak declaration. > > drivers/spi/designware_spi.c | 65 > +++- > 1 file changed, 64 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c > index 5aa507b..9eb5b1c 100644 > --- a/drivers/spi/designware_spi.c > +++ b/drivers/spi/designware_spi.c > @@ -11,6 +11,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -18,7 +19,10 @@ > #include > #include > #include > +/* Only SOCFPGA_GEN5 and SOCFPGA_ARRIA10 uses their clock_manager functions > */ > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) || > defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > #include > +#endif How hard it is to make others to use clock manager? do you have any list? thanks! -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree
Add option to set spi controller clock frequency via device tree
using standard clock bindings.
Old way of setting spi controller clock frequency (via implementation
of 'cm_get_spi_controller_clk_hz' function in platform specific code)
remains supported for backward compatibility.
Signed-off-by: Eugeniy Paltsev
---
Changes v1->v2:
* disable clock if we can't get the rate.
* get rid of cm_get_spi_controller_clk_hz weak declaration.
drivers/spi/designware_spi.c | 65 +++-
1 file changed, 64 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 5aa507b..9eb5b1c 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -11,6 +11,7 @@
*/
#include
+#include
#include
#include
#include
@@ -18,7 +19,10 @@
#include
#include
#include
+/* Only SOCFPGA_GEN5 and SOCFPGA_ARRIA10 uses their clock_manager functions */
+#if defined(CONFIG_TARGET_SOCFPGA_GEN5) ||
defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
#include
+#endif
DECLARE_GLOBAL_DATA_PTR;
@@ -94,6 +98,7 @@ struct dw_spi_priv {
void __iomem *regs;
unsigned int freq; /* Default frequency */
unsigned int mode;
+ unsigned long bus_clk_rate;
int bits_per_word;
u8 cs; /* chip select pin */
@@ -176,14 +181,72 @@ static void spi_hw_init(struct dw_spi_priv *priv)
debug("%s: fifo_len=%d\n", __func__, priv->fifo_len);
}
+static int dw_spi_of_get_clk(struct udevice *bus)
+{
+#if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(CLK)
+ struct dw_spi_priv *priv = dev_get_priv(bus);
+ struct clk clk;
+ int ret;
+
+ ret = clk_get_by_index(bus, 0, &clk);
+ if (ret)
+ return -EINVAL;
+
+ ret = clk_enable(&clk);
+ if (ret && ret != -ENOSYS)
+ return ret;
+
+ priv->bus_clk_rate = clk_get_rate(&clk);
+ if (!priv->bus_clk_rate) {
+ clk_disable(&clk);
+ return -EINVAL;
+ }
+
+ clk_free(&clk);
+
+ return 0;
+#else
+ return -ENOSYS;
+#endif
+}
+
+static int dw_spi_get_clk(struct udevice *bus)
+{
+ struct dw_spi_priv *priv = dev_get_priv(bus);
+
+ /* Firstly try to get clock frequency from device tree */
+ if (!dw_spi_of_get_clk(bus))
+ return 0;
+
+ /*
+* SOCFPGA_GEN5 and SOCFPGA_ARRIA10 uses cm_get_spi_controller_clk_hz
+* function (defined in asm/arch/clock_manager.h) to get spi controller
+* clock frequency. So in case of get clock frequency from device
+* tree failure rollback to cm_get_spi_controller_clk_hz
+*/
+#if defined(CONFIG_TARGET_SOCFPGA_GEN5) ||
defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
+ priv->bus_clk_rate = cm_get_spi_controller_clk_hz();
+#endif
+
+ if (!priv->bus_clk_rate)
+ return -EINVAL;
+
+ return 0;
+}
+
static int dw_spi_probe(struct udevice *bus)
{
struct dw_spi_platdata *plat = dev_get_platdata(bus);
struct dw_spi_priv *priv = dev_get_priv(bus);
+ int ret;
priv->regs = plat->regs;
priv->freq = plat->frequency;
+ ret = dw_spi_get_clk(bus);
+ if (ret)
+ return ret;
+
/* Currently only bits_per_word == 8 supported */
priv->bits_per_word = 8;
@@ -369,7 +432,7 @@ static int dw_spi_set_speed(struct udevice *bus, uint speed)
spi_enable_chip(priv, 0);
/* clk_div doesn't support odd number */
- clk_div = cm_get_spi_controller_clk_hz() / speed;
+ clk_div = priv->bus_clk_rate / speed;
clk_div = (clk_div + 1) & 0xfffe;
dw_writel(priv, DW_SPI_BAUDR, clk_div);
--
2.9.3
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
