* net: macb: Fix compilation on systems without COMMON_CLK @ 2019-06-24 6:16 Palmer Dabbelt 2019-06-24 6:16 ` [PATCH 1/2] " Palmer Dabbelt 2019-06-24 6:16 ` [PATCH 2/2] net: macb: Kconfig: Rename Atmel to Cadence Palmer Dabbelt 0 siblings, 2 replies; 10+ messages in thread From: Palmer Dabbelt @ 2019-06-24 6:16 UTC (permalink / raw) To: davem; +Cc: nicolas.ferre, netdev, linux-kernel Our patch to add support for the FU540-C000 broke compilation on at least powerpc allyesconfig, which was found as part of the linux-next build regression tests. This must have somehow slipped through the cracks, as the patch has been reverted in linux-next for a while now. This patch applies on top of the offending commit, which is the only one I've even tried it on as I'm not sure how this subsystem makes it to Linus. This patch set fixes the issue by adding another Kconfig entry to conditionally enable the FU540-C000 support. It would be less code to just make MACB depend on COMMON_CLK, but I'm not sure if that dependency would cause trouble for anyone so I didn't do it that way. I'm happy to re-spin the patch to add the dependency, but as it's a very small change I'm also happy if you do it yourself :). I've also included a second patch to indicate this is a Cadence driver, not an Atmel driver. As far as I know the controller is from Cadence, but it looks like maybe it showed up first on some Atmel chips. I'm fine either way, so feel free to just drop it if you think the old name is better. The only relation is that I stumbled across it when writing the first patch. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] net: macb: Fix compilation on systems without COMMON_CLK 2019-06-24 6:16 net: macb: Fix compilation on systems without COMMON_CLK Palmer Dabbelt @ 2019-06-24 6:16 ` Palmer Dabbelt 2019-06-24 9:40 ` Nicolas.Ferre 2019-06-24 6:16 ` [PATCH 2/2] net: macb: Kconfig: Rename Atmel to Cadence Palmer Dabbelt 1 sibling, 1 reply; 10+ messages in thread From: Palmer Dabbelt @ 2019-06-24 6:16 UTC (permalink / raw) To: davem; +Cc: nicolas.ferre, netdev, linux-kernel, Palmer Dabbelt The patch to add support for the FU540-C000 added a dependency on COMMON_CLK, but didn't express that via Kconfig. This fixes the build failure by adding CONFIG_MACB_FU540, which depends on COMMON_CLK and conditionally enables the FU540-C000 support. I've built this with a powerpc allyesconfig (which pointed out the bug) and on RISC-V, manually checking to ensure the code was built. I haven't even booted the resulting kernels. Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000") Signed-off-by: Palmer Dabbelt <palmer@sifive.com> --- drivers/net/ethernet/cadence/Kconfig | 11 +++++++++++ drivers/net/ethernet/cadence/macb_main.c | 12 ++++++++++++ 2 files changed, 23 insertions(+) diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig index 1766697c9c5a..74ee2bfd2369 100644 --- a/drivers/net/ethernet/cadence/Kconfig +++ b/drivers/net/ethernet/cadence/Kconfig @@ -40,6 +40,17 @@ config MACB_USE_HWSTAMP ---help--- Enable IEEE 1588 Precision Time Protocol (PTP) support for MACB. +config MACB_FU540 + bool "Enable support for the SiFive FU540 clock controller" + depends on MACB && COMMON_CLK + default y + ---help--- + Enable support for the MACB/GEM clock controller on the SiFive + FU540-C000. This device is necessary for switching between 10/100 + and gigabit modes on the FU540-C000 SoC, without which it is only + possible to bring up the Ethernet link in whatever mode the + bootloader probed. + config MACB_PCI tristate "Cadence PCI MACB/GEM support" depends on MACB && PCI && COMMON_CLK diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index c545c5b435d8..a903dfdd4183 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -41,6 +41,7 @@ #include <linux/pm_runtime.h> #include "macb.h" +#ifdef CONFIG_MACB_FU540 /* This structure is only used for MACB on SiFive FU540 devices */ struct sifive_fu540_macb_mgmt { void __iomem *reg; @@ -49,6 +50,7 @@ struct sifive_fu540_macb_mgmt { }; static struct sifive_fu540_macb_mgmt *mgmt; +#endif #define MACB_RX_BUFFER_SIZE 128 #define RX_BUFFER_MULTIPLE 64 /* bytes */ @@ -3956,6 +3958,7 @@ static int at91ether_init(struct platform_device *pdev) return 0; } +#ifdef CONFIG_MACB_FU540 static unsigned long fu540_macb_tx_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { @@ -4056,7 +4059,9 @@ static int fu540_c000_init(struct platform_device *pdev) return macb_init(pdev); } +#endif +#ifdef CONFIG_MACB_FU540 static const struct macb_config fu540_c000_config = { .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO | MACB_CAPS_GEM_HAS_PTP, @@ -4065,6 +4070,7 @@ static const struct macb_config fu540_c000_config = { .init = fu540_c000_init, .jumbo_max_len = 10240, }; +#endif static const struct macb_config at91sam9260_config = { .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII, @@ -4155,7 +4161,9 @@ static const struct of_device_id macb_dt_ids[] = { { .compatible = "cdns,emac", .data = &emac_config }, { .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config}, { .compatible = "cdns,zynq-gem", .data = &zynq_config }, +#ifdef CONFIG_MACB_FU540 { .compatible = "sifive,fu540-macb", .data = &fu540_c000_config }, +#endif { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, macb_dt_ids); @@ -4363,7 +4371,9 @@ static int macb_probe(struct platform_device *pdev) err_disable_clocks: clk_disable_unprepare(tx_clk); +#ifdef CONFIG_MACB_FU540 clk_unregister(tx_clk); +#endif clk_disable_unprepare(hclk); clk_disable_unprepare(pclk); clk_disable_unprepare(rx_clk); @@ -4398,7 +4408,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_FU540 clk_unregister(bp->tx_clk); +#endif clk_disable_unprepare(bp->hclk); clk_disable_unprepare(bp->pclk); clk_disable_unprepare(bp->rx_clk); -- 2.21.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: macb: Fix compilation on systems without COMMON_CLK 2019-06-24 6:16 ` [PATCH 1/2] " Palmer Dabbelt @ 2019-06-24 9:40 ` Nicolas.Ferre 2019-06-24 9:57 ` Palmer Dabbelt 0 siblings, 1 reply; 10+ messages in thread From: Nicolas.Ferre @ 2019-06-24 9:40 UTC (permalink / raw) To: palmer, davem; +Cc: netdev, linux-kernel On 24/06/2019 at 08:16, Palmer Dabbelt wrote: > External E-Mail > > > The patch to add support for the FU540-C000 added a dependency on > COMMON_CLK, but didn't express that via Kconfig. This fixes the build > failure by adding CONFIG_MACB_FU540, which depends on COMMON_CLK and > conditionally enables the FU540-C000 support. Let's try to limit the use of #ifdef's throughout the code. We are using them in this driver but only for the hot paths and things that have an impact on performance. I don't think it's the case here: so please find another option => NACK. > I've built this with a powerpc allyesconfig (which pointed out the bug) > and on RISC-V, manually checking to ensure the code was built. I > haven't even booted the resulting kernels. > > Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000") > Signed-off-by: Palmer Dabbelt <palmer@sifive.com> > --- > drivers/net/ethernet/cadence/Kconfig | 11 +++++++++++ > drivers/net/ethernet/cadence/macb_main.c | 12 ++++++++++++ > 2 files changed, 23 insertions(+) > > diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig > index 1766697c9c5a..74ee2bfd2369 100644 > --- a/drivers/net/ethernet/cadence/Kconfig > +++ b/drivers/net/ethernet/cadence/Kconfig > @@ -40,6 +40,17 @@ config MACB_USE_HWSTAMP > ---help--- > Enable IEEE 1588 Precision Time Protocol (PTP) support for MACB. > > +config MACB_FU540 > + bool "Enable support for the SiFive FU540 clock controller" > + depends on MACB && COMMON_CLK > + default y > + ---help--- > + Enable support for the MACB/GEM clock controller on the SiFive > + FU540-C000. This device is necessary for switching between 10/100 > + and gigabit modes on the FU540-C000 SoC, without which it is only > + possible to bring up the Ethernet link in whatever mode the > + bootloader probed. > + > config MACB_PCI > tristate "Cadence PCI MACB/GEM support" > depends on MACB && PCI && COMMON_CLK > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c > index c545c5b435d8..a903dfdd4183 100644 > --- a/drivers/net/ethernet/cadence/macb_main.c > +++ b/drivers/net/ethernet/cadence/macb_main.c > @@ -41,6 +41,7 @@ > #include <linux/pm_runtime.h> > #include "macb.h" > > +#ifdef CONFIG_MACB_FU540 > /* This structure is only used for MACB on SiFive FU540 devices */ > struct sifive_fu540_macb_mgmt { > void __iomem *reg; > @@ -49,6 +50,7 @@ struct sifive_fu540_macb_mgmt { > }; > > static struct sifive_fu540_macb_mgmt *mgmt; > +#endif > > #define MACB_RX_BUFFER_SIZE 128 > #define RX_BUFFER_MULTIPLE 64 /* bytes */ > @@ -3956,6 +3958,7 @@ static int at91ether_init(struct platform_device *pdev) > return 0; > } > > +#ifdef CONFIG_MACB_FU540 > static unsigned long fu540_macb_tx_recalc_rate(struct clk_hw *hw, > unsigned long parent_rate) > { > @@ -4056,7 +4059,9 @@ static int fu540_c000_init(struct platform_device *pdev) > > return macb_init(pdev); > } > +#endif > > +#ifdef CONFIG_MACB_FU540 > static const struct macb_config fu540_c000_config = { > .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO | > MACB_CAPS_GEM_HAS_PTP, > @@ -4065,6 +4070,7 @@ static const struct macb_config fu540_c000_config = { > .init = fu540_c000_init, > .jumbo_max_len = 10240, > }; > +#endif > > static const struct macb_config at91sam9260_config = { > .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII, > @@ -4155,7 +4161,9 @@ static const struct of_device_id macb_dt_ids[] = { > { .compatible = "cdns,emac", .data = &emac_config }, > { .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config}, > { .compatible = "cdns,zynq-gem", .data = &zynq_config }, > +#ifdef CONFIG_MACB_FU540 > { .compatible = "sifive,fu540-macb", .data = &fu540_c000_config }, > +#endif > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, macb_dt_ids); > @@ -4363,7 +4371,9 @@ static int macb_probe(struct platform_device *pdev) > > err_disable_clocks: > clk_disable_unprepare(tx_clk); > +#ifdef CONFIG_MACB_FU540 > clk_unregister(tx_clk); > +#endif > clk_disable_unprepare(hclk); > clk_disable_unprepare(pclk); > clk_disable_unprepare(rx_clk); > @@ -4398,7 +4408,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_FU540 > clk_unregister(bp->tx_clk); > +#endif > clk_disable_unprepare(bp->hclk); > clk_disable_unprepare(bp->pclk); > clk_disable_unprepare(bp->rx_clk); > -- Nicolas Ferre ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: macb: Fix compilation on systems without COMMON_CLK 2019-06-24 9:40 ` Nicolas.Ferre @ 2019-06-24 9:57 ` Palmer Dabbelt 2019-06-24 15:22 ` Nicolas.Ferre 0 siblings, 1 reply; 10+ messages in thread From: Palmer Dabbelt @ 2019-06-24 9:57 UTC (permalink / raw) To: Nicolas.Ferre; +Cc: davem, netdev, linux-kernel On Mon, 24 Jun 2019 02:40:21 PDT (-0700), Nicolas.Ferre@microchip.com wrote: > On 24/06/2019 at 08:16, Palmer Dabbelt wrote: >> External E-Mail >> >> >> The patch to add support for the FU540-C000 added a dependency on >> COMMON_CLK, but didn't express that via Kconfig. This fixes the build >> failure by adding CONFIG_MACB_FU540, which depends on COMMON_CLK and >> conditionally enables the FU540-C000 support. > > Let's try to limit the use of #ifdef's throughout the code. We are > using them in this driver but only for the hot paths and things that > have an impact on performance. I don't think it's the case here: so > please find another option => NACK. OK. Would you accept adding a Kconfig dependency of the generic MACB driver on COMMON_CLK, as suggested in the cover letter? > >> I've built this with a powerpc allyesconfig (which pointed out the bug) >> and on RISC-V, manually checking to ensure the code was built. I >> haven't even booted the resulting kernels. >> >> Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000") >> Signed-off-by: Palmer Dabbelt <palmer@sifive.com> >> --- >> drivers/net/ethernet/cadence/Kconfig | 11 +++++++++++ >> drivers/net/ethernet/cadence/macb_main.c | 12 ++++++++++++ >> 2 files changed, 23 insertions(+) >> >> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig >> index 1766697c9c5a..74ee2bfd2369 100644 >> --- a/drivers/net/ethernet/cadence/Kconfig >> +++ b/drivers/net/ethernet/cadence/Kconfig >> @@ -40,6 +40,17 @@ config MACB_USE_HWSTAMP >> ---help--- >> Enable IEEE 1588 Precision Time Protocol (PTP) support for MACB. >> >> +config MACB_FU540 >> + bool "Enable support for the SiFive FU540 clock controller" >> + depends on MACB && COMMON_CLK >> + default y >> + ---help--- >> + Enable support for the MACB/GEM clock controller on the SiFive >> + FU540-C000. This device is necessary for switching between 10/100 >> + and gigabit modes on the FU540-C000 SoC, without which it is only >> + possible to bring up the Ethernet link in whatever mode the >> + bootloader probed. >> + >> config MACB_PCI >> tristate "Cadence PCI MACB/GEM support" >> depends on MACB && PCI && COMMON_CLK >> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c >> index c545c5b435d8..a903dfdd4183 100644 >> --- a/drivers/net/ethernet/cadence/macb_main.c >> +++ b/drivers/net/ethernet/cadence/macb_main.c >> @@ -41,6 +41,7 @@ >> #include <linux/pm_runtime.h> >> #include "macb.h" >> >> +#ifdef CONFIG_MACB_FU540 >> /* This structure is only used for MACB on SiFive FU540 devices */ >> struct sifive_fu540_macb_mgmt { >> void __iomem *reg; >> @@ -49,6 +50,7 @@ struct sifive_fu540_macb_mgmt { >> }; >> >> static struct sifive_fu540_macb_mgmt *mgmt; >> +#endif >> >> #define MACB_RX_BUFFER_SIZE 128 >> #define RX_BUFFER_MULTIPLE 64 /* bytes */ >> @@ -3956,6 +3958,7 @@ static int at91ether_init(struct platform_device *pdev) >> return 0; >> } >> >> +#ifdef CONFIG_MACB_FU540 >> static unsigned long fu540_macb_tx_recalc_rate(struct clk_hw *hw, >> unsigned long parent_rate) >> { >> @@ -4056,7 +4059,9 @@ static int fu540_c000_init(struct platform_device *pdev) >> >> return macb_init(pdev); >> } >> +#endif >> >> +#ifdef CONFIG_MACB_FU540 >> static const struct macb_config fu540_c000_config = { >> .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO | >> MACB_CAPS_GEM_HAS_PTP, >> @@ -4065,6 +4070,7 @@ static const struct macb_config fu540_c000_config = { >> .init = fu540_c000_init, >> .jumbo_max_len = 10240, >> }; >> +#endif >> >> static const struct macb_config at91sam9260_config = { >> .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII, >> @@ -4155,7 +4161,9 @@ static const struct of_device_id macb_dt_ids[] = { >> { .compatible = "cdns,emac", .data = &emac_config }, >> { .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config}, >> { .compatible = "cdns,zynq-gem", .data = &zynq_config }, >> +#ifdef CONFIG_MACB_FU540 >> { .compatible = "sifive,fu540-macb", .data = &fu540_c000_config }, >> +#endif >> { /* sentinel */ } >> }; >> MODULE_DEVICE_TABLE(of, macb_dt_ids); >> @@ -4363,7 +4371,9 @@ static int macb_probe(struct platform_device *pdev) >> >> err_disable_clocks: >> clk_disable_unprepare(tx_clk); >> +#ifdef CONFIG_MACB_FU540 >> clk_unregister(tx_clk); >> +#endif >> clk_disable_unprepare(hclk); >> clk_disable_unprepare(pclk); >> clk_disable_unprepare(rx_clk); >> @@ -4398,7 +4408,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_FU540 >> clk_unregister(bp->tx_clk); >> +#endif >> clk_disable_unprepare(bp->hclk); >> clk_disable_unprepare(bp->pclk); >> clk_disable_unprepare(bp->rx_clk); >> > > > -- > Nicolas Ferre ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: macb: Fix compilation on systems without COMMON_CLK 2019-06-24 9:57 ` Palmer Dabbelt @ 2019-06-24 15:22 ` Nicolas.Ferre 2019-06-25 5:16 ` Harini Katakam 0 siblings, 1 reply; 10+ messages in thread From: Nicolas.Ferre @ 2019-06-24 15:22 UTC (permalink / raw) To: palmer, harini.katakam; +Cc: davem, netdev, linux-kernel, michal.simek On 24/06/2019 at 11:57, Palmer Dabbelt wrote: > External E-Mail > > > On Mon, 24 Jun 2019 02:40:21 PDT (-0700), Nicolas.Ferre@microchip.com wrote: >> On 24/06/2019 at 08:16, Palmer Dabbelt wrote: >>> External E-Mail >>> >>> >>> The patch to add support for the FU540-C000 added a dependency on >>> COMMON_CLK, but didn't express that via Kconfig. This fixes the build >>> failure by adding CONFIG_MACB_FU540, which depends on COMMON_CLK and >>> conditionally enables the FU540-C000 support. >> >> Let's try to limit the use of #ifdef's throughout the code. We are >> using them in this driver but only for the hot paths and things that >> have an impact on performance. I don't think it's the case here: so >> please find another option => NACK. > > OK. Would you accept adding a Kconfig dependency of the generic MACB driver on > COMMON_CLK, as suggested in the cover letter? Yes: all users of this peripheral have COMMON_CLK set. You can remove it from the PCI wrapper then. Best regards, Nicolas >>> I've built this with a powerpc allyesconfig (which pointed out the bug) >>> and on RISC-V, manually checking to ensure the code was built. I >>> haven't even booted the resulting kernels. >>> >>> Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000") >>> Signed-off-by: Palmer Dabbelt <palmer@sifive.com> >>> --- >>> drivers/net/ethernet/cadence/Kconfig | 11 +++++++++++ >>> drivers/net/ethernet/cadence/macb_main.c | 12 ++++++++++++ >>> 2 files changed, 23 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig >>> index 1766697c9c5a..74ee2bfd2369 100644 >>> --- a/drivers/net/ethernet/cadence/Kconfig >>> +++ b/drivers/net/ethernet/cadence/Kconfig >>> @@ -40,6 +40,17 @@ config MACB_USE_HWSTAMP >>> ---help--- >>> Enable IEEE 1588 Precision Time Protocol (PTP) support for MACB. >>> >>> +config MACB_FU540 >>> + bool "Enable support for the SiFive FU540 clock controller" >>> + depends on MACB && COMMON_CLK >>> + default y >>> + ---help--- >>> + Enable support for the MACB/GEM clock controller on the SiFive >>> + FU540-C000. This device is necessary for switching between 10/100 >>> + and gigabit modes on the FU540-C000 SoC, without which it is only >>> + possible to bring up the Ethernet link in whatever mode the >>> + bootloader probed. >>> + >>> config MACB_PCI >>> tristate "Cadence PCI MACB/GEM support" >>> depends on MACB && PCI && COMMON_CLK >>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c >>> index c545c5b435d8..a903dfdd4183 100644 >>> --- a/drivers/net/ethernet/cadence/macb_main.c >>> +++ b/drivers/net/ethernet/cadence/macb_main.c >>> @@ -41,6 +41,7 @@ >>> #include <linux/pm_runtime.h> >>> #include "macb.h" >>> >>> +#ifdef CONFIG_MACB_FU540 >>> /* This structure is only used for MACB on SiFive FU540 devices */ >>> struct sifive_fu540_macb_mgmt { >>> void __iomem *reg; >>> @@ -49,6 +50,7 @@ struct sifive_fu540_macb_mgmt { >>> }; >>> >>> static struct sifive_fu540_macb_mgmt *mgmt; >>> +#endif >>> >>> #define MACB_RX_BUFFER_SIZE 128 >>> #define RX_BUFFER_MULTIPLE 64 /* bytes */ >>> @@ -3956,6 +3958,7 @@ static int at91ether_init(struct platform_device *pdev) >>> return 0; >>> } >>> >>> +#ifdef CONFIG_MACB_FU540 >>> static unsigned long fu540_macb_tx_recalc_rate(struct clk_hw *hw, >>> unsigned long parent_rate) >>> { >>> @@ -4056,7 +4059,9 @@ static int fu540_c000_init(struct platform_device *pdev) >>> >>> return macb_init(pdev); >>> } >>> +#endif >>> >>> +#ifdef CONFIG_MACB_FU540 >>> static const struct macb_config fu540_c000_config = { >>> .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO | >>> MACB_CAPS_GEM_HAS_PTP, >>> @@ -4065,6 +4070,7 @@ static const struct macb_config fu540_c000_config = { >>> .init = fu540_c000_init, >>> .jumbo_max_len = 10240, >>> }; >>> +#endif >>> >>> static const struct macb_config at91sam9260_config = { >>> .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII, >>> @@ -4155,7 +4161,9 @@ static const struct of_device_id macb_dt_ids[] = { >>> { .compatible = "cdns,emac", .data = &emac_config }, >>> { .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config}, >>> { .compatible = "cdns,zynq-gem", .data = &zynq_config }, >>> +#ifdef CONFIG_MACB_FU540 >>> { .compatible = "sifive,fu540-macb", .data = &fu540_c000_config }, >>> +#endif >>> { /* sentinel */ } >>> }; >>> MODULE_DEVICE_TABLE(of, macb_dt_ids); >>> @@ -4363,7 +4371,9 @@ static int macb_probe(struct platform_device *pdev) >>> >>> err_disable_clocks: >>> clk_disable_unprepare(tx_clk); >>> +#ifdef CONFIG_MACB_FU540 >>> clk_unregister(tx_clk); >>> +#endif >>> clk_disable_unprepare(hclk); >>> clk_disable_unprepare(pclk); >>> clk_disable_unprepare(rx_clk); >>> @@ -4398,7 +4408,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_FU540 >>> clk_unregister(bp->tx_clk); >>> +#endif >>> clk_disable_unprepare(bp->hclk); >>> clk_disable_unprepare(bp->pclk); >>> clk_disable_unprepare(bp->rx_clk); >>> >> >> >> -- >> Nicolas Ferre -- Nicolas Ferre ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] net: macb: Fix compilation on systems without COMMON_CLK 2019-06-24 15:22 ` Nicolas.Ferre @ 2019-06-25 5:16 ` Harini Katakam 0 siblings, 0 replies; 10+ messages in thread From: Harini Katakam @ 2019-06-25 5:16 UTC (permalink / raw) To: Nicolas Ferre Cc: palmer, Harini Katakam, David Miller, netdev, linux-kernel, Michal Simek On Tue, Jun 25, 2019 at 4:17 AM <Nicolas.Ferre@microchip.com> wrote: > > On 24/06/2019 at 11:57, Palmer Dabbelt wrote: > > External E-Mail > > > > > > On Mon, 24 Jun 2019 02:40:21 PDT (-0700), Nicolas.Ferre@microchip.com wrote: > >> On 24/06/2019 at 08:16, Palmer Dabbelt wrote: > >>> External E-Mail > >>> > >>> > >>> The patch to add support for the FU540-C000 added a dependency on > >>> COMMON_CLK, but didn't express that via Kconfig. This fixes the build > >>> failure by adding CONFIG_MACB_FU540, which depends on COMMON_CLK and > >>> conditionally enables the FU540-C000 support. > >> > >> Let's try to limit the use of #ifdef's throughout the code. We are > >> using them in this driver but only for the hot paths and things that > >> have an impact on performance. I don't think it's the case here: so > >> please find another option => NACK. > > > > OK. Would you accept adding a Kconfig dependency of the generic MACB driver on > > COMMON_CLK, as suggested in the cover letter? > > Yes: all users of this peripheral have COMMON_CLK set. > You can remove it from the PCI wrapper then. > Yes, +1, both Zynq and ZynqMP have COMMON_CLK set, thanks. Regards, Harini ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] net: macb: Kconfig: Rename Atmel to Cadence 2019-06-24 6:16 net: macb: Fix compilation on systems without COMMON_CLK Palmer Dabbelt 2019-06-24 6:16 ` [PATCH 1/2] " Palmer Dabbelt @ 2019-06-24 6:16 ` Palmer Dabbelt 2019-06-24 9:49 ` Nicolas.Ferre 1 sibling, 1 reply; 10+ messages in thread From: Palmer Dabbelt @ 2019-06-24 6:16 UTC (permalink / raw) To: davem; +Cc: nicolas.ferre, netdev, linux-kernel, Palmer Dabbelt When touching the Kconfig for this driver I noticed that both the Kconfig help text and a comment referred to this being an Atmel driver. As far as I know, this is a Cadence driver. The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig help text as that change caused it to go over 80 characters. Signed-off-by: Palmer Dabbelt <palmer@sifive.com> --- drivers/net/ethernet/cadence/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig index 74ee2bfd2369..29b6132b418e 100644 --- a/drivers/net/ethernet/cadence/Kconfig +++ b/drivers/net/ethernet/cadence/Kconfig @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only # -# Atmel device configuration +# Cadence device configuration # config NET_VENDOR_CADENCE @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE If unsure, say Y. Note that the answer to this question doesn't directly affect the - kernel: saying N will just cause the configurator to skip all - the remaining Atmel network card questions. If you say Y, you will be + kernel: saying N will just cause the configurator to skip all the + remaining Cadence network card questions. If you say Y, you will be asked for your specific card in the following questions. if NET_VENDOR_CADENCE -- 2.21.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] net: macb: Kconfig: Rename Atmel to Cadence 2019-06-24 6:16 ` [PATCH 2/2] net: macb: Kconfig: Rename Atmel to Cadence Palmer Dabbelt @ 2019-06-24 9:49 ` Nicolas.Ferre 2019-06-24 9:57 ` Palmer Dabbelt 0 siblings, 1 reply; 10+ messages in thread From: Nicolas.Ferre @ 2019-06-24 9:49 UTC (permalink / raw) To: palmer, davem; +Cc: netdev, linux-kernel On 24/06/2019 at 08:16, Palmer Dabbelt wrote: > External E-Mail > > > When touching the Kconfig for this driver I noticed that both the > Kconfig help text and a comment referred to this being an Atmel driver. > As far as I know, this is a Cadence driver. The fix is just Indeed: was written and then maintained by Atmel (now Microchip) for years... So I would say that more than a "Cadence driver" it's a driver that applies to a Cadence peripheral. I won't hold the patch just for this as the patch makes perfect sense, but would love that it's been highlighted... > s/Atmel/Cadence/, but I did go and re-wrap the Kconfig help text as that > change caused it to go over 80 characters. > > Signed-off-by: Palmer Dabbelt <palmer@sifive.com> > --- > drivers/net/ethernet/cadence/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig > index 74ee2bfd2369..29b6132b418e 100644 > --- a/drivers/net/ethernet/cadence/Kconfig > +++ b/drivers/net/ethernet/cadence/Kconfig > @@ -1,6 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0-only > # > -# Atmel device configuration > +# Cadence device configuration > # > > config NET_VENDOR_CADENCE > @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE > If unsure, say Y. > > Note that the answer to this question doesn't directly affect the > - kernel: saying N will just cause the configurator to skip all > - the remaining Atmel network card questions. If you say Y, you will be > + kernel: saying N will just cause the configurator to skip all the > + remaining Cadence network card questions. If you say Y, you will be > asked for your specific card in the following questions. > > if NET_VENDOR_CADENCE > -- Nicolas Ferre ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] net: macb: Kconfig: Rename Atmel to Cadence 2019-06-24 9:49 ` Nicolas.Ferre @ 2019-06-24 9:57 ` Palmer Dabbelt 2019-06-24 15:42 ` Nicolas.Ferre 0 siblings, 1 reply; 10+ messages in thread From: Palmer Dabbelt @ 2019-06-24 9:57 UTC (permalink / raw) To: Nicolas.Ferre; +Cc: davem, netdev, linux-kernel On Mon, 24 Jun 2019 02:49:16 PDT (-0700), Nicolas.Ferre@microchip.com wrote: > On 24/06/2019 at 08:16, Palmer Dabbelt wrote: >> External E-Mail >> >> >> When touching the Kconfig for this driver I noticed that both the >> Kconfig help text and a comment referred to this being an Atmel driver. >> As far as I know, this is a Cadence driver. The fix is just > > Indeed: was written and then maintained by Atmel (now Microchip) for > years... So I would say that more than a "Cadence driver" it's a driver > that applies to a Cadence peripheral. > > I won't hold the patch just for this as the patch makes perfect sense, > but would love that it's been highlighted... OK, I don't mind changing it. Does this look OK? I have to submit a v2 anyway for the first patch. Author: Palmer Dabbelt <palmer@sifive.com> Date: Sun Jun 23 23:04:14 2019 -0700 net: macb: Kconfig: Rename Atmel to Cadence The help text makes it look like NET_VENDOR_CADENCE enables support for Atmel devices, when in reality it's a driver written by Atmel that supports Cadence devices. This may confuse users that have this device on a non-Atmel SoC. The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig help text as that change caused it to go over 80 characters. Signed-off-by: Palmer Dabbelt <palmer@sifive.com> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig index 74ee2bfd2369..29b6132b418e 100644 --- a/drivers/net/ethernet/cadence/Kconfig +++ b/drivers/net/ethernet/cadence/Kconfig @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0-only # -# Atmel device configuration +# Cadence device configuration # config NET_VENDOR_CADENCE @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE If unsure, say Y. Note that the answer to this question doesn't directly affect the - kernel: saying N will just cause the configurator to skip all - the remaining Atmel network card questions. If you say Y, you will be + kernel: saying N will just cause the configurator to skip all the + remaining Cadence network card questions. If you say Y, you will be asked for your specific card in the following questions. if NET_VENDOR_CADENCE > >> s/Atmel/Cadence/, but I did go and re-wrap the Kconfig help text as that >> change caused it to go over 80 characters. >> >> Signed-off-by: Palmer Dabbelt <palmer@sifive.com> >> --- >> drivers/net/ethernet/cadence/Kconfig | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig >> index 74ee2bfd2369..29b6132b418e 100644 >> --- a/drivers/net/ethernet/cadence/Kconfig >> +++ b/drivers/net/ethernet/cadence/Kconfig >> @@ -1,6 +1,6 @@ >> # SPDX-License-Identifier: GPL-2.0-only >> # >> -# Atmel device configuration >> +# Cadence device configuration >> # >> >> config NET_VENDOR_CADENCE >> @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE >> If unsure, say Y. >> >> Note that the answer to this question doesn't directly affect the >> - kernel: saying N will just cause the configurator to skip all >> - the remaining Atmel network card questions. If you say Y, you will be >> + kernel: saying N will just cause the configurator to skip all the >> + remaining Cadence network card questions. If you say Y, you will be >> asked for your specific card in the following questions. >> >> if NET_VENDOR_CADENCE >> > > > -- > Nicolas Ferre ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] net: macb: Kconfig: Rename Atmel to Cadence 2019-06-24 9:57 ` Palmer Dabbelt @ 2019-06-24 15:42 ` Nicolas.Ferre 0 siblings, 0 replies; 10+ messages in thread From: Nicolas.Ferre @ 2019-06-24 15:42 UTC (permalink / raw) To: palmer; +Cc: davem, netdev, linux-kernel On 24/06/2019 at 11:57, Palmer Dabbelt wrote: > External E-Mail > > > On Mon, 24 Jun 2019 02:49:16 PDT (-0700), Nicolas.Ferre@microchip.com wrote: >> On 24/06/2019 at 08:16, Palmer Dabbelt wrote: >>> External E-Mail >>> >>> >>> When touching the Kconfig for this driver I noticed that both the >>> Kconfig help text and a comment referred to this being an Atmel driver. >>> As far as I know, this is a Cadence driver. The fix is just >> >> Indeed: was written and then maintained by Atmel (now Microchip) for >> years... So I would say that more than a "Cadence driver" it's a driver >> that applies to a Cadence peripheral. >> >> I won't hold the patch just for this as the patch makes perfect sense, >> but would love that it's been highlighted... > > OK, I don't mind changing it. Does this look OK? I have to submit a v2 anyway > for the first patch. Yep, nice! Thanks, Nicolas > > Author: Palmer Dabbelt <palmer@sifive.com> > Date: Sun Jun 23 23:04:14 2019 -0700 > > net: macb: Kconfig: Rename Atmel to Cadence > > The help text makes it look like NET_VENDOR_CADENCE enables support for > Atmel devices, when in reality it's a driver written by Atmel that > supports Cadence devices. This may confuse users that have this device > on a non-Atmel SoC. > > The fix is just s/Atmel/Cadence/, but I did go and re-wrap the Kconfig > help text as that change caused it to go over 80 characters. > > Signed-off-by: Palmer Dabbelt <palmer@sifive.com> > > diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig > index 74ee2bfd2369..29b6132b418e 100644 > --- a/drivers/net/ethernet/cadence/Kconfig > +++ b/drivers/net/ethernet/cadence/Kconfig > @@ -1,6 +1,6 @@ > # SPDX-License-Identifier: GPL-2.0-only > # > -# Atmel device configuration > +# Cadence device configuration > # > > config NET_VENDOR_CADENCE > @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE > If unsure, say Y. > > Note that the answer to this question doesn't directly affect the > - kernel: saying N will just cause the configurator to skip all > - the remaining Atmel network card questions. If you say Y, you will be > + kernel: saying N will just cause the configurator to skip all the > + remaining Cadence network card questions. If you say Y, you will be > asked for your specific card in the following questions. > > if NET_VENDOR_CADENCE > >> >>> s/Atmel/Cadence/, but I did go and re-wrap the Kconfig help text as that >>> change caused it to go over 80 characters. >>> >>> Signed-off-by: Palmer Dabbelt <palmer@sifive.com> >>> --- >>> drivers/net/ethernet/cadence/Kconfig | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig >>> index 74ee2bfd2369..29b6132b418e 100644 >>> --- a/drivers/net/ethernet/cadence/Kconfig >>> +++ b/drivers/net/ethernet/cadence/Kconfig >>> @@ -1,6 +1,6 @@ >>> # SPDX-License-Identifier: GPL-2.0-only >>> # >>> -# Atmel device configuration >>> +# Cadence device configuration >>> # >>> >>> config NET_VENDOR_CADENCE >>> @@ -13,8 +13,8 @@ config NET_VENDOR_CADENCE >>> If unsure, say Y. >>> >>> Note that the answer to this question doesn't directly affect the >>> - kernel: saying N will just cause the configurator to skip all >>> - the remaining Atmel network card questions. If you say Y, you will be >>> + kernel: saying N will just cause the configurator to skip all the >>> + remaining Cadence network card questions. If you say Y, you will be >>> asked for your specific card in the following questions. >>> >>> if NET_VENDOR_CADENCE >>> >> >> >> -- >> Nicolas Ferre -- Nicolas Ferre ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-06-25 5:16 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-24 6:16 net: macb: Fix compilation on systems without COMMON_CLK Palmer Dabbelt 2019-06-24 6:16 ` [PATCH 1/2] " Palmer Dabbelt 2019-06-24 9:40 ` Nicolas.Ferre 2019-06-24 9:57 ` Palmer Dabbelt 2019-06-24 15:22 ` Nicolas.Ferre 2019-06-25 5:16 ` Harini Katakam 2019-06-24 6:16 ` [PATCH 2/2] net: macb: Kconfig: Rename Atmel to Cadence Palmer Dabbelt 2019-06-24 9:49 ` Nicolas.Ferre 2019-06-24 9:57 ` Palmer Dabbelt 2019-06-24 15:42 ` Nicolas.Ferre
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).