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. -- Eugeniy Paltsev _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
