On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut <ma...@denx.de> wrote: > On 10/30/2017 07:04 AM, Jagan Teki wrote: >> On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut <ma...@denx.de> 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 <ma...@denx.de> 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 >>>>>>>>>>> <alexey.brod...@synopsys.com> 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 <asm/arch/clock_manager.h> >>>> #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.
I'm trying to avoid __weak or #ifdef on driver instead make changes in clock manager which will remove in future(once all moved with CLK) 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
