[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190617155834.GK25211@lunn.ch>
Date: Mon, 17 Jun 2019 17:58:34 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Yash Shah <yash.shah@...ive.com>
Cc: davem@...emloft.net, devicetree@...r.kernel.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-riscv@...ts.infradead.org, robh+dt@...nel.org,
mark.rutland@....com, nicolas.ferre@...rochip.com,
palmer@...ive.com, aou@...s.berkeley.edu, paul.walmsley@...ive.com,
ynezz@...e.cz, sachin.ghadi@...ive.com
Subject: Re: [PATCH v2 2/2] macb: Add support for SiFive FU540-C000
On Mon, Jun 17, 2019 at 09:49:27AM +0530, Yash Shah wrote:
> 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@...ive.com>
> ---
> drivers/net/ethernet/cadence/Kconfig | 6 ++
> drivers/net/ethernet/cadence/macb_main.c | 129 +++++++++++++++++++++++++++++++
> 2 files changed, 135 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index b998401..d478fae 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -48,4 +48,10 @@ config MACB_PCI
> To compile this driver as a module, choose M here: the module
> will be called macb_pci.
>
> +config MACB_SIFIVE_FU540
> + bool "Cadence MACB/GEM support for SiFive FU540 SoC"
> + depends on MACB && GPIO_SIFIVE
> + help
> + Enable the Cadence MACB/GEM support for SiFive FU540 SoC.
> +
> endif # NET_VENDOR_CADENCE
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index c049410..275b5e8 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,116 @@ 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);
> + if (rate != 125000000)
> + iowrite32(1, mgmt->reg);
> + else
> + iowrite32(0, 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,
> @@ -3992,6 +4112,9 @@ static int at91ether_init(struct platform_device *pdev)
> { .compatible = "cdns,emac", .data = &emac_config },
> { .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config},
> { .compatible = "cdns,zynq-gem", .data = &zynq_config },
> +#ifdef CONFIG_MACB_SIFIVE_FU540
> + { .compatible = "sifive,fu540-macb", .data = &fu540_c000_config },
> +#endif
This #ifdef should not be needed.
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, macb_dt_ids);
> @@ -4199,6 +4322,9 @@ static int macb_probe(struct platform_device *pdev)
>
> err_disable_clocks:
> clk_disable_unprepare(tx_clk);
> +#ifdef CONFIG_MACB_SIFIVE_FU540
> + clk_unregister(tx_clk);
> +#endif
So long as tx_clk is NULL, you can call clk_unregister(). So please
remove the #ifdef.
> clk_disable_unprepare(hclk);
> clk_disable_unprepare(pclk);
> clk_disable_unprepare(rx_clk);
> @@ -4233,6 +4359,9 @@ static int macb_remove(struct platform_device *pdev)
> pm_runtime_dont_use_autosuspend(&pdev->dev);
> if (!pm_runtime_suspended(&pdev->dev)) {
> clk_disable_unprepare(bp->tx_clk);
> +#ifdef CONFIG_MACB_SIFIVE_FU540
> + clk_unregister(bp->tx_clk);
> +#endif
Same here.
In general try to avoid #ifdef in C code.
Andrew
Powered by blists - more mailing lists