* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree @ 2017-10-13 15:18 Eugeniy Paltsev 2017-10-16 17:07 ` Jagan Teki 0 siblings, 1 reply; 21+ messages in thread From: Eugeniy Paltsev @ 2017-10-13 15:18 UTC (permalink / raw) To: u-boot 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 <Eugeniy.Paltsev@synopsys.com> --- 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 <common.h> +#include <clk.h> #include <dm.h> #include <errno.h> #include <malloc.h> @@ -18,7 +19,10 @@ #include <fdtdec.h> #include <linux/compat.h> #include <asm/io.h> +/* Only SOCFPGA_GEN5 and SOCFPGA_ARRIA10 uses their clock_manager functions */ +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) || defined(CONFIG_TARGET_SOCFPGA_ARRIA10) #include <asm/arch/clock_manager.h> +#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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-13 15:18 [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree Eugeniy Paltsev @ 2017-10-16 17:07 ` Jagan Teki 2017-10-17 13:32 ` Eugeniy Paltsev 0 siblings, 1 reply; 21+ messages in thread From: Jagan Teki @ 2017-10-16 17:07 UTC (permalink / raw) To: u-boot On Fri, Oct 13, 2017 at 8:48 PM, Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> 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 <Eugeniy.Paltsev@synopsys.com> > --- > 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 <common.h> > +#include <clk.h> > #include <dm.h> > #include <errno.h> > #include <malloc.h> > @@ -18,7 +19,10 @@ > #include <fdtdec.h> > #include <linux/compat.h> > #include <asm/io.h> > +/* Only SOCFPGA_GEN5 and SOCFPGA_ARRIA10 uses their clock_manager functions */ > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) || defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > #include <asm/arch/clock_manager.h> > +#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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-16 17:07 ` Jagan Teki @ 2017-10-17 13:32 ` Eugeniy Paltsev 2017-10-17 14:57 ` Alexey Brodkin 0 siblings, 1 reply; 21+ messages in thread From: Eugeniy Paltsev @ 2017-10-17 13:32 UTC (permalink / raw) To: u-boot 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 > <Eugeniy.Paltsev@synopsys.com> 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 <Eugeniy.Paltsev@synopsys.com> > > --- > > 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 <common.h> > > +#include <clk.h> > > #include <dm.h> > > #include <errno.h> > > #include <malloc.h> > > @@ -18,7 +19,10 @@ > > #include <fdtdec.h> > > #include <linux/compat.h> > > #include <asm/io.h> > > +/* Only SOCFPGA_GEN5 and SOCFPGA_ARRIA10 uses their clock_manager functions */ > > +#if defined(CONFIG_TARGET_SOCFPGA_GEN5) || defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > > #include <asm/arch/clock_manager.h> > > +#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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-17 13:32 ` Eugeniy Paltsev @ 2017-10-17 14:57 ` Alexey Brodkin 2017-10-17 15:02 ` Jagan Teki 2017-10-17 15:03 ` Marek Vasut 0 siblings, 2 replies; 21+ messages in thread From: Alexey Brodkin @ 2017-10-17 14:57 UTC (permalink / raw) To: u-boot Hi Jagan, > -----Original Message----- > From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] > Sent: Tuesday, October 17, 2017 4:33 PM > To: jagannadh.teki at gmail.com > Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-17 14:57 ` Alexey Brodkin @ 2017-10-17 15:02 ` Jagan Teki 2017-10-19 15:36 ` Eugeniy Paltsev 2017-10-17 15:03 ` Marek Vasut 1 sibling, 1 reply; 21+ messages in thread From: Jagan Teki @ 2017-10-17 15:02 UTC (permalink / raw) To: u-boot On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Hi Jagan, > >> -----Original Message----- >> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >> Sent: Tuesday, October 17, 2017 4:33 PM >> To: jagannadh.teki at gmail.com >> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-17 15:02 ` Jagan Teki @ 2017-10-19 15:36 ` Eugeniy Paltsev 2017-10-19 15:51 ` Marek Vasut 0 siblings, 1 reply; 21+ messages in thread From: Eugeniy Paltsev @ 2017-10-19 15:36 UTC (permalink / raw) To: u-boot On Tue, 2017-10-17 at 20:32 +0530, Jagan Teki wrote: > On Tue, Oct 17, 2017 at 8:27 PM, Alexey Brodkin > <Alexey.Brodkin@synopsys.com> wrote: > > Hi Jagan, > > > > > -----Original Message----- > > > From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] > > > Sent: Tuesday, October 17, 2017 4:33 PM > > > To: jagannadh.teki at gmail.com > > > Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-19 15:36 ` Eugeniy Paltsev @ 2017-10-19 15:51 ` Marek Vasut 2017-10-19 18:20 ` Dinh Nguyen 0 siblings, 1 reply; 21+ messages in thread From: Marek Vasut @ 2017-10-19 15:51 UTC (permalink / raw) To: u-boot 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.Brodkin@synopsys.com> wrote: >>> Hi Jagan, >>> >>>> -----Original Message----- >>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>> To: jagannadh.teki at gmail.com >>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-19 15:51 ` Marek Vasut @ 2017-10-19 18:20 ` Dinh Nguyen 2017-10-23 11:43 ` Eugeniy Paltsev 0 siblings, 1 reply; 21+ messages in thread From: Dinh Nguyen @ 2017-10-19 18:20 UTC (permalink / raw) To: u-boot 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.Brodkin@synopsys.com> wrote: >>>> Hi Jagan, >>>> >>>>> -----Original Message----- >>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>> To: jagannadh.teki at gmail.com >>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-19 18:20 ` Dinh Nguyen @ 2017-10-23 11:43 ` Eugeniy Paltsev 2017-10-24 6:08 ` Marek Vasut 0 siblings, 1 reply; 21+ messages in thread From: Eugeniy Paltsev @ 2017-10-23 11:43 UTC (permalink / raw) To: u-boot 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.Brodkin@synopsys.com> wrote: > > > > > Hi Jagan, > > > > > > > > > > > -----Original Message----- > > > > > > From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] > > > > > > Sent: Tuesday, October 17, 2017 4:33 PM > > > > > > To: jagannadh.teki at gmail.com > > > > > > Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-23 11:43 ` Eugeniy Paltsev @ 2017-10-24 6:08 ` Marek Vasut 2017-10-24 9:52 ` Jagan Teki 0 siblings, 1 reply; 21+ messages in thread From: Marek Vasut @ 2017-10-24 6:08 UTC (permalink / raw) To: u-boot 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.Brodkin@synopsys.com> wrote: >>>>>> Hi Jagan, >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>> To: jagannadh.teki at gmail.com >>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-24 6:08 ` Marek Vasut @ 2017-10-24 9:52 ` Jagan Teki 2017-10-25 6:50 ` Marek Vasut 2017-10-27 13:54 ` Eugeniy Paltsev 0 siblings, 2 replies; 21+ messages in thread From: Jagan Teki @ 2017-10-24 9:52 UTC (permalink / raw) To: u-boot On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut <marex@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.Brodkin@synopsys.com> wrote: >>>>>>> Hi Jagan, >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-24 9:52 ` Jagan Teki @ 2017-10-25 6:50 ` Marek Vasut 2017-10-27 13:54 ` Eugeniy Paltsev 1 sibling, 0 replies; 21+ messages in thread From: Marek Vasut @ 2017-10-25 6:50 UTC (permalink / raw) To: u-boot On 10/24/2017 11:52 AM, Jagan Teki wrote: > On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut <marex@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.Brodkin@synopsys.com> wrote: >>>>>>>> Hi Jagan, >>>>>>>> >>>>>>>>> -----Original Message----- >>>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-24 9:52 ` Jagan Teki 2017-10-25 6:50 ` Marek Vasut @ 2017-10-27 13:54 ` Eugeniy Paltsev 2017-10-28 11:39 ` Marek Vasut 1 sibling, 1 reply; 21+ messages in thread From: Eugeniy Paltsev @ 2017-10-27 13:54 UTC (permalink / raw) To: u-boot On Tue, 2017-10-24 at 15:22 +0530, Jagan Teki wrote: > On Tue, Oct 24, 2017 at 11:38 AM, Marek Vasut <marex@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.Brodkin@synopsys.com> wrote: > > > > > > > > Hi Jagan, > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] > > > > > > > > > Sent: Tuesday, October 17, 2017 4:33 PM > > > > > > > > > To: jagannadh.teki at gmail.com > > > > > > > > > Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-27 13:54 ` Eugeniy Paltsev @ 2017-10-28 11:39 ` Marek Vasut 2017-10-30 6:04 ` Jagan Teki 0 siblings, 1 reply; 21+ messages in thread From: Marek Vasut @ 2017-10-28 11:39 UTC (permalink / raw) To: u-boot 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 <marex@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.Brodkin@synopsys.com> wrote: >>>>>>>>> Hi Jagan, >>>>>>>>> >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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. 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-28 11:39 ` Marek Vasut @ 2017-10-30 6:04 ` Jagan Teki 2017-10-30 10:54 ` Marek Vasut 0 siblings, 1 reply; 21+ messages in thread From: Jagan Teki @ 2017-10-30 6:04 UTC (permalink / raw) To: u-boot On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut <marex@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 <marex@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.Brodkin@synopsys.com> wrote: >>>>>>>>>> Hi Jagan, >>>>>>>>>> >>>>>>>>>>> -----Original Message----- >>>>>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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. --- 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 | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-30 6:04 ` Jagan Teki @ 2017-10-30 10:54 ` Marek Vasut 2017-10-30 11:36 ` Jagan Teki 0 siblings, 1 reply; 21+ messages in thread From: Marek Vasut @ 2017-10-30 10:54 UTC (permalink / raw) To: u-boot On 10/30/2017 07:04 AM, Jagan Teki wrote: > On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut <marex@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 <marex@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.Brodkin@synopsys.com> wrote: >>>>>>>>>>> Hi Jagan, >>>>>>>>>>> >>>>>>>>>>>> -----Original Message----- >>>>>>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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. > --- 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! > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-30 10:54 ` Marek Vasut @ 2017-10-30 11:36 ` Jagan Teki 2017-10-30 11:42 ` Marek Vasut 0 siblings, 1 reply; 21+ messages in thread From: Jagan Teki @ 2017-10-30 11:36 UTC (permalink / raw) To: u-boot On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut <marex@denx.de> wrote: > On 10/30/2017 07:04 AM, Jagan Teki wrote: >> On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut <marex@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 <marex@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.Brodkin@synopsys.com> wrote: >>>>>>>>>>>> Hi Jagan, >>>>>>>>>>>> >>>>>>>>>>>>> -----Original Message----- >>>>>>>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-30 11:36 ` Jagan Teki @ 2017-10-30 11:42 ` Marek Vasut 2017-10-31 8:27 ` Jagan Teki 0 siblings, 1 reply; 21+ messages in thread From: Marek Vasut @ 2017-10-30 11:42 UTC (permalink / raw) To: u-boot On 10/30/2017 12:36 PM, Jagan Teki wrote: > On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut <marex@denx.de> wrote: >> On 10/30/2017 07:04 AM, Jagan Teki wrote: >>> On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut <marex@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 <marex@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.Brodkin@synopsys.com> wrote: >>>>>>>>>>>>> Hi Jagan, >>>>>>>>>>>>> >>>>>>>>>>>>>> -----Original Message----- >>>>>>>>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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) Well, if you do the proposed change, socfpga will break. The __weak is the least possible evil here and I can live with that for now. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-30 11:42 ` Marek Vasut @ 2017-10-31 8:27 ` Jagan Teki 2017-10-31 9:33 ` Marek Vasut 0 siblings, 1 reply; 21+ messages in thread From: Jagan Teki @ 2017-10-31 8:27 UTC (permalink / raw) To: u-boot On Mon, Oct 30, 2017 at 5:12 PM, Marek Vasut <marex@denx.de> wrote: > On 10/30/2017 12:36 PM, Jagan Teki wrote: >> On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut <marex@denx.de> wrote: >>> On 10/30/2017 07:04 AM, Jagan Teki wrote: >>>> On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut <marex@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 <marex@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.Brodkin@synopsys.com> wrote: >>>>>>>>>>>>>> Hi Jagan, >>>>>>>>>>>>>> >>>>>>>>>>>>>>> -----Original Message----- >>>>>>>>>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>>>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>>>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>>>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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) > > Well, if you do the proposed change, socfpga will break. The __weak is > the least possible evil here and I can live with that for now. Yes, __weak can be possible evil here but since I'm planning to apply this in next version, I'm hoping possible change that shouldn't effect the driver. I see some __weak's last from many releases that doesn't resolve properly and they are still __weak functions only. thanks! -- Jagan Teki Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream Maintainer Hyderabad, India. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-31 8:27 ` Jagan Teki @ 2017-10-31 9:33 ` Marek Vasut 0 siblings, 0 replies; 21+ messages in thread From: Marek Vasut @ 2017-10-31 9:33 UTC (permalink / raw) To: u-boot On 10/31/2017 09:27 AM, Jagan Teki wrote: > On Mon, Oct 30, 2017 at 5:12 PM, Marek Vasut <marex@denx.de> wrote: >> On 10/30/2017 12:36 PM, Jagan Teki wrote: >>> On Mon, Oct 30, 2017 at 4:24 PM, Marek Vasut <marex@denx.de> wrote: >>>> On 10/30/2017 07:04 AM, Jagan Teki wrote: >>>>> On Sat, Oct 28, 2017 at 5:09 PM, Marek Vasut <marex@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 <marex@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.Brodkin@synopsys.com> wrote: >>>>>>>>>>>>>>> Hi Jagan, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> -----Original Message----- >>>>>>>>>>>>>>>> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >>>>>>>>>>>>>>>> Sent: Tuesday, October 17, 2017 4:33 PM >>>>>>>>>>>>>>>> To: jagannadh.teki at gmail.com >>>>>>>>>>>>>>>> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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) >> >> Well, if you do the proposed change, socfpga will break. The __weak is >> the least possible evil here and I can live with that for now. > > Yes, __weak can be possible evil here but since I'm planning to apply > this in next version, I'm hoping possible change that shouldn't effect > the driver. I see some __weak's last from many releases that doesn't > resolve properly and they are still __weak functions only. The idea here would be to have the default __weak implementation use clock framework in the driver and override it only for socfpga in the socfpga clock manager. So for this patch, that's a NAK, I'd like a V3 with the __weak. -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 21+ messages in thread
* [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree 2017-10-17 14:57 ` Alexey Brodkin 2017-10-17 15:02 ` Jagan Teki @ 2017-10-17 15:03 ` Marek Vasut 1 sibling, 0 replies; 21+ messages in thread From: Marek Vasut @ 2017-10-17 15:03 UTC (permalink / raw) To: u-boot On 10/17/2017 04:57 PM, Alexey Brodkin wrote: > Hi Jagan, > >> -----Original Message----- >> From: Eugeniy Paltsev [mailto:paltsev at synopsys.com] >> Sent: Tuesday, October 17, 2017 4:33 PM >> To: jagannadh.teki at gmail.com >> Cc: u-boot at lists.denx.de; uboot-snps-arc at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2017-10-31 9:33 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-13 15:18 [U-Boot] [PATCH v2] DW SPI: Get clock value from Device Tree Eugeniy Paltsev 2017-10-16 17:07 ` Jagan Teki 2017-10-17 13:32 ` Eugeniy Paltsev 2017-10-17 14:57 ` Alexey Brodkin 2017-10-17 15:02 ` Jagan Teki 2017-10-19 15:36 ` Eugeniy Paltsev 2017-10-19 15:51 ` Marek Vasut 2017-10-19 18:20 ` Dinh Nguyen 2017-10-23 11:43 ` Eugeniy Paltsev 2017-10-24 6:08 ` Marek Vasut 2017-10-24 9:52 ` Jagan Teki 2017-10-25 6:50 ` Marek Vasut 2017-10-27 13:54 ` Eugeniy Paltsev 2017-10-28 11:39 ` Marek Vasut 2017-10-30 6:04 ` Jagan Teki 2017-10-30 10:54 ` Marek Vasut 2017-10-30 11:36 ` Jagan Teki 2017-10-30 11:42 ` Marek Vasut 2017-10-31 8:27 ` Jagan Teki 2017-10-31 9:33 ` Marek Vasut 2017-10-17 15:03 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.