| Message ID | 1558611952-13295-3-git-send-email-yash.shah@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: macb: Add support for SiFive FU540-C000 | expand |
> +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate); > + iowrite32(rate != 125000000, mgmt->reg); That looks odd. Writing the result of a comparison to a register? Andrew
On Thu, May 23, 2019 at 8:24 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate, > > + unsigned long parent_rate) > > +{ > > + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate); > > + iowrite32(rate != 125000000, mgmt->reg); > > That looks odd. Writing the result of a comparison to a register? The idea was to write "1" to the register if the value of rate is anything else than 125000000. To make it easier to read, I will change this to below: - iowrite32(rate != 125000000, mgmt->reg); + if (rate != 125000000) + iowrite32(1, mgmt->reg); + else + iowrite32(0, mgmt->reg); Hope that's fine. Thanks for your comment - Yash > > Andrew > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, May 24, 2019 at 10:22:06AM +0530, Yash Shah wrote: > On Thu, May 23, 2019 at 8:24 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate, > > > + unsigned long parent_rate) > > > +{ > > > + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate); > > > + iowrite32(rate != 125000000, mgmt->reg); > > > > That looks odd. Writing the result of a comparison to a register? > > The idea was to write "1" to the register if the value of rate is > anything else than 125000000. I'm not a language lawyer. Is it guaranteed that an expression like this returns 1? Any value !0 is true, so maybe it actually returns 42? > To make it easier to read, I will change this to below: > - iowrite32(rate != 125000000, mgmt->reg); > + if (rate != 125000000) > + iowrite32(1, mgmt->reg); > + else > + iowrite32(0, mgmt->reg); > > Hope that's fine. Thanks for your comment Yes, that is good. Andrew
On Fri, 24 May 2019 06:48:47 PDT (-0700), andrew@lunn.ch wrote: > On Fri, May 24, 2019 at 10:22:06AM +0530, Yash Shah wrote: >> On Thu, May 23, 2019 at 8:24 PM Andrew Lunn <andrew@lunn.ch> wrote: >> > >> > > +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate, >> > > + unsigned long parent_rate) >> > > +{ >> > > + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate); >> > > + iowrite32(rate != 125000000, mgmt->reg); >> > >> > That looks odd. Writing the result of a comparison to a register? >> >> The idea was to write "1" to the register if the value of rate is >> anything else than 125000000. > > I'm not a language lawyer. Is it guaranteed that an expression like > this returns 1? Any value !0 is true, so maybe it actually returns 42? From Stack Overflow: https://stackoverflow.com/questions/18097922/return-value-of-operator-in-c "C11(ISO/IEC 9899:201x) ยง6.5.8 Relational operators Each of the operators < (less than), > (greater than), <= (less than or equal to), and >= (greater than or equal to) shall yield 1 if the specified relation is true and 0 if it is false. The result has type int." >> To make it easier to read, I will change this to below: >> - iowrite32(rate != 125000000, mgmt->reg); >> + if (rate != 125000000) >> + iowrite32(1, mgmt->reg); >> + else >> + iowrite32(0, mgmt->reg); >> >> Hope that's fine. Thanks for your comment > > Yes, that is good. > > Andrew
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index c049410..a9e5227 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -10,6 +10,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/crc32.h> #include <linux/module.h> #include <linux/moduleparam.h> @@ -40,6 +41,15 @@ #include <linux/pm_runtime.h> #include "macb.h" +/* This structure is only used for MACB on SiFive FU540 devices */ +struct sifive_fu540_macb_mgmt { + void __iomem *reg; + unsigned long rate; + struct clk_hw hw; +}; + +static struct sifive_fu540_macb_mgmt *mgmt; + #define MACB_RX_BUFFER_SIZE 128 #define RX_BUFFER_MULTIPLE 64 /* bytes */ @@ -3903,6 +3913,113 @@ static int at91ether_init(struct platform_device *pdev) return 0; } +static unsigned long fu540_macb_tx_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + return mgmt->rate; +} + +static long fu540_macb_tx_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + if (WARN_ON(rate < 2500000)) + return 2500000; + else if (rate == 2500000) + return 2500000; + else if (WARN_ON(rate < 13750000)) + return 2500000; + else if (WARN_ON(rate < 25000000)) + return 25000000; + else if (rate == 25000000) + return 25000000; + else if (WARN_ON(rate < 75000000)) + return 25000000; + else if (WARN_ON(rate < 125000000)) + return 125000000; + else if (rate == 125000000) + return 125000000; + + WARN_ON(rate > 125000000); + + return 125000000; +} + +static int fu540_macb_tx_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + rate = fu540_macb_tx_round_rate(hw, rate, &parent_rate); + iowrite32(rate != 125000000, mgmt->reg); + mgmt->rate = rate; + + return 0; +} + +static const struct clk_ops fu540_c000_ops = { + .recalc_rate = fu540_macb_tx_recalc_rate, + .round_rate = fu540_macb_tx_round_rate, + .set_rate = fu540_macb_tx_set_rate, +}; + +static int fu540_c000_clk_init(struct platform_device *pdev, struct clk **pclk, + struct clk **hclk, struct clk **tx_clk, + struct clk **rx_clk, struct clk **tsu_clk) +{ + struct clk_init_data init; + int err = 0; + + err = macb_clk_init(pdev, pclk, hclk, tx_clk, rx_clk, tsu_clk); + if (err) + return err; + + mgmt = devm_kzalloc(&pdev->dev, sizeof(*mgmt), GFP_KERNEL); + if (!mgmt) + return -ENOMEM; + + init.name = "sifive-gemgxl-mgmt"; + init.ops = &fu540_c000_ops; + init.flags = 0; + init.num_parents = 0; + + mgmt->rate = 0; + mgmt->hw.init = &init; + + *tx_clk = clk_register(NULL, &mgmt->hw); + if (IS_ERR(*tx_clk)) + return PTR_ERR(*tx_clk); + + err = clk_prepare_enable(*tx_clk); + if (err) + dev_err(&pdev->dev, "failed to enable tx_clk (%u)\n", err); + else + dev_info(&pdev->dev, "Registered clk switch '%s'\n", init.name); + + return 0; +} + +static int fu540_c000_init(struct platform_device *pdev) +{ + struct resource *res; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + if (!res) + return -ENODEV; + + mgmt->reg = ioremap(res->start, resource_size(res)); + if (!mgmt->reg) + return -ENOMEM; + + return macb_init(pdev); +} + +static const struct macb_config fu540_c000_config = { + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO | + MACB_CAPS_GEM_HAS_PTP, + .dma_burst_length = 16, + .clk_init = fu540_c000_clk_init, + .init = fu540_c000_init, + .jumbo_max_len = 10240, +}; + static const struct macb_config at91sam9260_config = { .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII, .clk_init = macb_clk_init, @@ -3980,6 +4097,7 @@ static int at91ether_init(struct platform_device *pdev) { .compatible = "cdns,at32ap7000-macb" }, { .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config }, { .compatible = "cdns,macb" }, + { .compatible = "cdns,fu540-macb", .data = &fu540_c000_config }, { .compatible = "cdns,np4-macb", .data = &np4_config }, { .compatible = "cdns,pc302-gem", .data = &pc302gem_config }, { .compatible = "cdns,gem", .data = &pc302gem_config },
The management IP block is tightly coupled with the Cadence MACB IP block on the FU540, and manages many of the boundary signals from the MACB IP. This patch only controls the tx_clk input signal to the MACB IP. Future patches may add support for monitoring or controlling other IP boundary signals. Signed-off-by: Yash Shah <yash.shah@sifive.com> --- drivers/net/ethernet/cadence/macb_main.c | 118 +++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+)