* [PATCH net-next 0/3] net: phy: add Amlogic g12a support @ 2019-03-14 14:01 ` Jerome Brunet 0 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-14 14:01 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman Cc: Jerome Brunet, netdev, linux-amlogic, linux-kernel, devicetree This patchset adds the necessary bits to support network on the Amlogic g12a SoC family. Only the internal PHY and related MDIO mux needed to addressed. The GMAC remains compatible with axg SoC family This series has been tested on the u200 (S905D2) with both the internal and external (Realtek) PHYs. Jerome Brunet (3): dt-bindings: net: phy: add g12a mdio mux documentation net: phy: add amlogic g12a mdio mux support net: phy: meson-gxl: add g12a support .../bindings/net/mdio-mux-meson-g12a.txt | 48 +++ drivers/net/phy/Kconfig | 10 + drivers/net/phy/Makefile | 1 + drivers/net/phy/mdio-mux-meson-g12a.c | 371 ++++++++++++++++++ drivers/net/phy/meson-gxl.c | 14 + 5 files changed, 444 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt create mode 100644 drivers/net/phy/mdio-mux-meson-g12a.c -- 2.20.1 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next 0/3] net: phy: add Amlogic g12a support @ 2019-03-14 14:01 ` Jerome Brunet 0 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-14 14:01 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman Cc: netdev, linux-amlogic, linux-kernel, devicetree, Jerome Brunet This patchset adds the necessary bits to support network on the Amlogic g12a SoC family. Only the internal PHY and related MDIO mux needed to addressed. The GMAC remains compatible with axg SoC family This series has been tested on the u200 (S905D2) with both the internal and external (Realtek) PHYs. Jerome Brunet (3): dt-bindings: net: phy: add g12a mdio mux documentation net: phy: add amlogic g12a mdio mux support net: phy: meson-gxl: add g12a support .../bindings/net/mdio-mux-meson-g12a.txt | 48 +++ drivers/net/phy/Kconfig | 10 + drivers/net/phy/Makefile | 1 + drivers/net/phy/mdio-mux-meson-g12a.c | 371 ++++++++++++++++++ drivers/net/phy/meson-gxl.c | 14 + 5 files changed, 444 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt create mode 100644 drivers/net/phy/mdio-mux-meson-g12a.c -- 2.20.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next 1/3] dt-bindings: net: phy: add g12a mdio mux documentation 2019-03-14 14:01 ` Jerome Brunet @ 2019-03-14 14:01 ` Jerome Brunet -1 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-14 14:01 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman Cc: Jerome Brunet, netdev, linux-amlogic, linux-kernel, devicetree Add documentation for the device tree bindings of the MDIO mux of Amlogic g12a SoC family Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- .../bindings/net/mdio-mux-meson-g12a.txt | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt diff --git a/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt b/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt new file mode 100644 index 000000000000..e738bba6a9de --- /dev/null +++ b/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt @@ -0,0 +1,48 @@ +Properties for the MDIO bus multiplexer/glue of Amlogic G12a SoC family. + +This is a special case of a MDIO bus multiplexer. It allows to choose between +the internal mdio bus leading to the embedded 10/100 PHY or the external +MDIO bus. + +Required properties in addition to the generic multiplexer properties: +- compatible : amlogic,g12a-mdio-mux +- reg: physical address and length of the multiplexer/glue registers +- clocks: list of clock phandle, one for each entry clock-names. +- clock-names: should contain the following: + * "pclk" : peripheral clock. + * "clkin0" : platform crytal + * "clkin1" : SoC 50MHz MPLL + +Example : + +mdio-mux: mdio-multiplexer@4c000 { + compatible = "amlogic,g12a-mdio-mux"; + reg = <0x0 0x4c000 0x0 0xa4>; + clocks = <&clkc CLKID_ETH_PHY>, + <&xtal>, + <&clkc CLKID_MPLL_5OM>; + clock-names = "pclk", "clkin0", "clkin1"; + mdio-parent-bus = <&mdio0>; + #address-cells = <1>; + #size-cells = <0>; + + ext_mdio: mdio@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + }; + + int_mdio: mdio@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + internal_ephy: ethernet_phy@8 { + compatible = "ethernet-phy-id0180.3301", + "ethernet-phy-ieee802.3-c22"; + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>; + reg = <8>; + max-speed = <100>; + }; + }; +}; -- 2.20.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH net-next 1/3] dt-bindings: net: phy: add g12a mdio mux documentation @ 2019-03-14 14:01 ` Jerome Brunet 0 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-14 14:01 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman Cc: netdev, linux-amlogic, linux-kernel, devicetree, Jerome Brunet Add documentation for the device tree bindings of the MDIO mux of Amlogic g12a SoC family Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- .../bindings/net/mdio-mux-meson-g12a.txt | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt diff --git a/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt b/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt new file mode 100644 index 000000000000..e738bba6a9de --- /dev/null +++ b/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt @@ -0,0 +1,48 @@ +Properties for the MDIO bus multiplexer/glue of Amlogic G12a SoC family. + +This is a special case of a MDIO bus multiplexer. It allows to choose between +the internal mdio bus leading to the embedded 10/100 PHY or the external +MDIO bus. + +Required properties in addition to the generic multiplexer properties: +- compatible : amlogic,g12a-mdio-mux +- reg: physical address and length of the multiplexer/glue registers +- clocks: list of clock phandle, one for each entry clock-names. +- clock-names: should contain the following: + * "pclk" : peripheral clock. + * "clkin0" : platform crytal + * "clkin1" : SoC 50MHz MPLL + +Example : + +mdio-mux: mdio-multiplexer@4c000 { + compatible = "amlogic,g12a-mdio-mux"; + reg = <0x0 0x4c000 0x0 0xa4>; + clocks = <&clkc CLKID_ETH_PHY>, + <&xtal>, + <&clkc CLKID_MPLL_5OM>; + clock-names = "pclk", "clkin0", "clkin1"; + mdio-parent-bus = <&mdio0>; + #address-cells = <1>; + #size-cells = <0>; + + ext_mdio: mdio@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + }; + + int_mdio: mdio@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + internal_ephy: ethernet_phy@8 { + compatible = "ethernet-phy-id0180.3301", + "ethernet-phy-ieee802.3-c22"; + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>; + reg = <8>; + max-speed = <100>; + }; + }; +}; -- 2.20.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 1/3] dt-bindings: net: phy: add g12a mdio mux documentation 2019-03-14 14:01 ` Jerome Brunet @ 2019-03-28 15:57 ` Rob Herring -1 siblings, 0 replies; 24+ messages in thread From: Rob Herring @ 2019-03-28 15:57 UTC (permalink / raw) To: Jerome Brunet Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman, netdev, linux-amlogic, linux-kernel, devicetree On Thu, Mar 14, 2019 at 03:01:33PM +0100, Jerome Brunet wrote: > Add documentation for the device tree bindings of the MDIO mux of Amlogic > g12a SoC family > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > .../bindings/net/mdio-mux-meson-g12a.txt | 48 +++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt > > diff --git a/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt b/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt > new file mode 100644 > index 000000000000..e738bba6a9de > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt > @@ -0,0 +1,48 @@ > +Properties for the MDIO bus multiplexer/glue of Amlogic G12a SoC family. > + > +This is a special case of a MDIO bus multiplexer. It allows to choose between > +the internal mdio bus leading to the embedded 10/100 PHY or the external > +MDIO bus. > + > +Required properties in addition to the generic multiplexer properties: > +- compatible : amlogic,g12a-mdio-mux > +- reg: physical address and length of the multiplexer/glue registers > +- clocks: list of clock phandle, one for each entry clock-names. > +- clock-names: should contain the following: > + * "pclk" : peripheral clock. > + * "clkin0" : platform crytal > + * "clkin1" : SoC 50MHz MPLL > + > +Example : > + > +mdio-mux: mdio-multiplexer@4c000 { > + compatible = "amlogic,g12a-mdio-mux"; > + reg = <0x0 0x4c000 0x0 0xa4>; > + clocks = <&clkc CLKID_ETH_PHY>, > + <&xtal>, > + <&clkc CLKID_MPLL_5OM>; > + clock-names = "pclk", "clkin0", "clkin1"; > + mdio-parent-bus = <&mdio0>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + ext_mdio: mdio@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + int_mdio: mdio@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + internal_ephy: ethernet_phy@8 { ethernet-phy@8 Otherwise, Reviewed-by: Rob Herring <robh@kernel.org> > + compatible = "ethernet-phy-id0180.3301", > + "ethernet-phy-ieee802.3-c22"; > + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>; > + reg = <8>; > + max-speed = <100>; > + }; > + }; > +}; > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 1/3] dt-bindings: net: phy: add g12a mdio mux documentation @ 2019-03-28 15:57 ` Rob Herring 0 siblings, 0 replies; 24+ messages in thread From: Rob Herring @ 2019-03-28 15:57 UTC (permalink / raw) To: Jerome Brunet Cc: Andrew Lunn, Florian Fainelli, devicetree, Kevin Hilman, linux-kernel, netdev, linux-amlogic, David S. Miller, Heiner Kallweit On Thu, Mar 14, 2019 at 03:01:33PM +0100, Jerome Brunet wrote: > Add documentation for the device tree bindings of the MDIO mux of Amlogic > g12a SoC family > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > .../bindings/net/mdio-mux-meson-g12a.txt | 48 +++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt > > diff --git a/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt b/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt > new file mode 100644 > index 000000000000..e738bba6a9de > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/mdio-mux-meson-g12a.txt > @@ -0,0 +1,48 @@ > +Properties for the MDIO bus multiplexer/glue of Amlogic G12a SoC family. > + > +This is a special case of a MDIO bus multiplexer. It allows to choose between > +the internal mdio bus leading to the embedded 10/100 PHY or the external > +MDIO bus. > + > +Required properties in addition to the generic multiplexer properties: > +- compatible : amlogic,g12a-mdio-mux > +- reg: physical address and length of the multiplexer/glue registers > +- clocks: list of clock phandle, one for each entry clock-names. > +- clock-names: should contain the following: > + * "pclk" : peripheral clock. > + * "clkin0" : platform crytal > + * "clkin1" : SoC 50MHz MPLL > + > +Example : > + > +mdio-mux: mdio-multiplexer@4c000 { > + compatible = "amlogic,g12a-mdio-mux"; > + reg = <0x0 0x4c000 0x0 0xa4>; > + clocks = <&clkc CLKID_ETH_PHY>, > + <&xtal>, > + <&clkc CLKID_MPLL_5OM>; > + clock-names = "pclk", "clkin0", "clkin1"; > + mdio-parent-bus = <&mdio0>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + ext_mdio: mdio@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + int_mdio: mdio@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + internal_ephy: ethernet_phy@8 { ethernet-phy@8 Otherwise, Reviewed-by: Rob Herring <robh@kernel.org> > + compatible = "ethernet-phy-id0180.3301", > + "ethernet-phy-ieee802.3-c22"; > + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>; > + reg = <8>; > + max-speed = <100>; > + }; > + }; > +}; > -- > 2.20.1 > _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support 2019-03-14 14:01 ` Jerome Brunet @ 2019-03-14 14:01 ` Jerome Brunet -1 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-14 14:01 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman Cc: Jerome Brunet, netdev, linux-amlogic, linux-kernel, devicetree Add support for the mdio mux and internal phy glue of the g12a SoC family Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/net/phy/Kconfig | 10 + drivers/net/phy/Makefile | 1 + drivers/net/phy/mdio-mux-meson-g12a.c | 371 ++++++++++++++++++++++++++ 3 files changed, 382 insertions(+) create mode 100644 drivers/net/phy/mdio-mux-meson-g12a.c diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 071869db44cf..831aa350b1cb 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -74,6 +74,16 @@ config MDIO_BUS_MUX_GPIO several child MDIO busses to a parent bus. Child bus selection is under the control of GPIO lines. +config MDIO_BUS_MUX_MESON_G12A + tristate "Amlogic G12a based MDIO bus multiplexer" + depends on ARCH_MESON || COMPILE_TEST + depends on OF_MDIO && HAS_IOMEM + select MDIO_BUS_MUX + help + This module provides a driver for the MDIO multiplexer/glue of + the amlogic g12a SoC. The multiplexers connects either the external + or the internal MDIO bus to the parent bus. + config MDIO_BUS_MUX_MMIOREG tristate "MMIO device-controlled MDIO bus multiplexers" depends on OF_MDIO && HAS_IOMEM diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index ece5dae67174..27d7f9f3b0de 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o obj-$(CONFIG_MDIO_BUS_MUX_BCM_IPROC) += mdio-mux-bcm-iproc.o obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o +obj-$(CONFIG_MDIO_BUS_MUX_MESON_G12A) += mdio-mux-meson-g12a.o obj-$(CONFIG_MDIO_BUS_MUX_MMIOREG) += mdio-mux-mmioreg.o obj-$(CONFIG_MDIO_BUS_MUX_MULTIPLEXER) += mdio-mux-multiplexer.o obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio-mux-meson-g12a.c new file mode 100644 index 000000000000..9da681cce7cd --- /dev/null +++ b/drivers/net/phy/mdio-mux-meson-g12a.c @@ -0,0 +1,371 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2019 Baylibre, SAS. + * Author: Jerome Brunet <jbrunet@baylibre.com> + */ + +#include <linux/io.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/device.h> +#include <linux/module.h> +#include <linux/phy.h> +#include <linux/mdio-mux.h> +#include <linux/clk-provider.h> +#include <linux/clk.h> +#include <linux/bitfield.h> +#include <linux/iopoll.h> + +#define ETH_PLL_STS 0x40 +#define ETH_PLL_CTL0 0x44 +#define PLL_CTL0_LOCK_DIG BIT(30) +#define PLL_CTL0_RST BIT(29) +#define PLL_CTL0_EN BIT(28) +#define PLL_CTL0_SEL BIT(23) +#define PLL_CTL0_N GENMASK(14, 10) +#define PLL_CTL0_M GENMASK(8, 0) +#define PLL_LOCK_TIMEOUT 1000000 +#define PLL_MUX_NUM_PARENT 2 +#define ETH_PLL_CTL1 0x48 +#define ETH_PLL_CTL2 0x4c +#define ETH_PLL_CTL3 0x50 +#define ETH_PLL_CTL4 0x54 +#define ETH_PLL_CTL5 0x58 +#define ETH_PLL_CTL6 0x5c +#define ETH_PLL_CTL7 0x60 + +#define ETH_PHY_CNTL0 0x80 +#define EPHY_G12A_ID 0x33000180 +#define ETH_PHY_CNTL1 0x84 +#define PHY_CNTL1_ST_MODE GENMASK(2, 0) +#define PHY_CNTL1_ST_PHYADD GENMASK(7, 3) +#define EPHY_DFLT_ADD 8 +#define PHY_CNTL1_MII_MODE GENMASK(15, 14) +#define EPHY_MODE_RMII 0x1 +#define PHY_CNTL1_CLK_EN BIT(16) +#define PHY_CNTL1_CLKFREQ BIT(17) +#define PHY_CNTL1_PHY_ENB BIT(18) +#define ETH_PHY_CNTL2 0x88 +#define PHY_CNTL2_USE_INTERNAL BIT(5) +#define PHY_CNTL2_SMI_SRC_MAC BIT(6) +#define PHY_CNTL2_RX_CLK_EPHY BIT(9) + +#define MESON_G12A_MDIO_EXTERNAL_ID 0 +#define MESON_G12A_MDIO_INTERNAL_ID 1 + +struct g12a_mdio_mux { + void __iomem *regs; + struct clk *pclk; + struct clk *pll; + bool pll_is_enabled; + void *mux_handle; +}; + +struct g12a_ephy_pll { + struct clk_hw hw; + void __iomem *base; +}; + +#define g12a_ephy_pll_to_dev(_hw) \ + container_of(_hw, struct g12a_ephy_pll, hw) \ + +static unsigned long g12a_ephy_pll_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + u32 val, m, n; + + val = readl(pll->base + ETH_PLL_CTL0); + m = FIELD_GET(PLL_CTL0_M, val); + n = FIELD_GET(PLL_CTL0_N, val); + + return parent_rate * m / n; +} + +static int g12a_ephy_pll_enable(struct clk_hw *hw) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + u32 val = readl(pll->base + ETH_PLL_CTL0); + + /* Apply both enable an reset */ + val |= PLL_CTL0_RST | PLL_CTL0_EN; + writel(val, pll->base + ETH_PLL_CTL0); + + /* Clear the reset to let PLL lock */ + val &= ~PLL_CTL0_RST; + writel(val, pll->base + ETH_PLL_CTL0); + + /* Poll on the digital lock instead of the usual analog lock + * This is done because bit 31 is unreliable on some SoC. Bit + * 31 may indicate that the PLL is not lock eventhough the clock + * is actually running + */ + return readl_poll_timeout(pll->base + ETH_PLL_CTL0, val, + val & PLL_CTL0_LOCK_DIG, 0, PLL_LOCK_TIMEOUT); +} + +static void g12a_ephy_pll_disable(struct clk_hw *hw) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + u32 val; + + val = readl(pll->base + ETH_PLL_CTL0); + val &= ~PLL_CTL0_EN; + val |= PLL_CTL0_RST; + writel(val, pll->base + ETH_PLL_CTL0); +} + +static int g12a_ephy_pll_is_enabled(struct clk_hw *hw) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + unsigned int val; + + val = readl(pll->base + ETH_PLL_CTL0); + + return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0; +} + +static void g12a_ephy_pll_init(struct clk_hw *hw) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + + /* Apply PLL HW settings */ + writel(0x29c0040a, pll->base + ETH_PLL_CTL0); + writel(0x927e0000, pll->base + ETH_PLL_CTL1); + writel(0xac5f49e5, pll->base + ETH_PLL_CTL2); + writel(0x00000000, pll->base + ETH_PLL_CTL3); + writel(0x00000000, pll->base + ETH_PLL_CTL4); + writel(0x20200000, pll->base + ETH_PLL_CTL5); + writel(0x0000c002, pll->base + ETH_PLL_CTL6); + writel(0x00000023, pll->base + ETH_PLL_CTL7); +} + +static const struct clk_ops g12a_ephy_pll_ops = { + .recalc_rate = g12a_ephy_pll_recalc_rate, + .is_enabled = g12a_ephy_pll_is_enabled, + .enable = g12a_ephy_pll_enable, + .disable = g12a_ephy_pll_disable, + .init = g12a_ephy_pll_init, +}; + +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) +{ + int ret; + + /* Enable the phy clock */ + if (!priv->pll_is_enabled) { + ret = clk_prepare_enable(priv->pll); + if (ret) + return ret; + } + + priv->pll_is_enabled = true; + + /* Initialize ephy control */ + writel(EPHY_G12A_ID, priv->regs + ETH_PHY_CNTL0); + writel(FIELD_PREP(PHY_CNTL1_ST_MODE, 3) | + FIELD_PREP(PHY_CNTL1_ST_PHYADD, EPHY_DFLT_ADD) | + FIELD_PREP(PHY_CNTL1_MII_MODE, EPHY_MODE_RMII) | + PHY_CNTL1_CLK_EN | + PHY_CNTL1_CLKFREQ | + PHY_CNTL1_PHY_ENB, + priv->regs + ETH_PHY_CNTL1); + writel(PHY_CNTL2_USE_INTERNAL | + PHY_CNTL2_SMI_SRC_MAC | + PHY_CNTL2_RX_CLK_EPHY, + priv->regs + ETH_PHY_CNTL2); + + return 0; +} + +static int _g12a_enable_external_mdio(struct g12a_mdio_mux *priv) +{ + /* Reset the mdio bus mux */ + writel_relaxed(0x0, priv->regs + ETH_PHY_CNTL2); + + /* Disable the phy clock if enabled */ + if (priv->pll_is_enabled) { + clk_disable_unprepare(priv->pll); + priv->pll_is_enabled = false; + } + + return 0; +} + +static int g12a_mdio_switch_fn(int current_child, int desired_child, + void *data) +{ + struct device *dev = data; + struct g12a_mdio_mux *priv = dev_get_drvdata(dev); + + if (current_child == desired_child) + return 0; + + switch (desired_child) { + case MESON_G12A_MDIO_EXTERNAL_ID: + return _g12a_enable_external_mdio(priv); + case MESON_G12A_MDIO_INTERNAL_ID: + return _g12a_enable_internal_mdio(priv); + default: + return -EINVAL; + } +} + +static const struct of_device_id g12a_mdio_mux_match[] = { + { .compatible = "amlogic,g12a-mdio-mux", }, + {}, +}; +MODULE_DEVICE_TABLE(of, g12a_mdio_mux_match); + +static int g12a_ephy_glue_clk_register(struct device *dev) +{ + struct g12a_mdio_mux *priv = dev_get_drvdata(dev); + const char *parent_names[PLL_MUX_NUM_PARENT]; + struct clk_init_data init; + struct clk_mux *mux; + struct g12a_ephy_pll *pll; + struct clk *clk; + char *name; + int i; + + /* get the mux parents */ + for (i = 0; i < PLL_MUX_NUM_PARENT; i++) { + char in_name[8]; + + snprintf(in_name, sizeof(in_name), "clkin%d", i); + clk = devm_clk_get(dev, in_name); + if (IS_ERR(clk)) { + if (PTR_ERR(clk) != -EPROBE_DEFER) + dev_err(dev, "Missing clock %s\n", in_name); + return PTR_ERR(clk); + } + + parent_names[i] = __clk_get_name(clk); + } + + /* create the input mux */ + mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL); + if (!mux) + return -ENOMEM; + + name = kasprintf(GFP_KERNEL, "%s#mux", dev_name(dev)); + if (!name) + return -ENOMEM; + + init.name = name; + init.ops = &clk_mux_ro_ops; + init.flags = 0; + init.parent_names = parent_names; + init.num_parents = PLL_MUX_NUM_PARENT; + + mux->reg = priv->regs + ETH_PLL_CTL0; + mux->shift = __ffs(PLL_CTL0_SEL); + mux->mask = PLL_CTL0_SEL >> mux->shift; + mux->hw.init = &init; + + clk = devm_clk_register(dev, &mux->hw); + kfree(name); + if (IS_ERR(clk)) { + dev_err(dev, "failed to register input mux\n"); + return PTR_ERR(clk); + } + + /* create the pll */ + pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL); + if (!pll) + return -ENOMEM; + + name = kasprintf(GFP_KERNEL, "%s#pll", dev_name(dev)); + if (!name) + return -ENOMEM; + + init.name = name; + init.ops = &g12a_ephy_pll_ops; + init.flags = 0; + parent_names[0] = __clk_get_name(clk); + init.parent_names = parent_names; + init.num_parents = 1; + + pll->base = priv->regs; + pll->hw.init = &init; + + clk = devm_clk_register(dev, &pll->hw); + kfree(name); + if (IS_ERR(clk)) { + dev_err(dev, "failed to register input mux\n"); + return PTR_ERR(clk); + } + + priv->pll = clk; + + return 0; +} + +static int g12a_mdio_mux_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct resource *res; + struct g12a_mdio_mux *priv; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + platform_set_drvdata(pdev, priv); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + priv->regs = devm_ioremap_resource(dev, res); + if (IS_ERR(priv->regs)) + return PTR_ERR(priv->regs); + + priv->pclk = devm_clk_get(dev, "pclk"); + if (IS_ERR(priv->pclk)) { + ret = PTR_ERR(priv->pclk); + if (ret != -EPROBE_DEFER) + dev_err(dev, "failed to get peripheral clock\n"); + return ret; + } + + /* Make sure the device registers are clocked */ + ret = clk_prepare_enable(priv->pclk); + if (ret) { + dev_err(dev, "failed to enable peripheral clock"); + return ret; + } + + /* Register PLL in CCF */ + ret = g12a_ephy_glue_clk_register(dev); + if (ret) + return ret; + + return mdio_mux_init(dev, dev->of_node, g12a_mdio_switch_fn, + &priv->mux_handle, dev, NULL); +} + +static int g12a_mdio_mux_remove(struct platform_device *pdev) +{ + struct g12a_mdio_mux *priv = platform_get_drvdata(pdev); + + mdio_mux_uninit(priv->mux_handle); + + if (priv->pll_is_enabled) + clk_disable_unprepare(priv->pll); + + clk_disable_unprepare(priv->pclk); + + return 0; +} + +static struct platform_driver g12a_mdio_mux_driver = { + .probe = g12a_mdio_mux_probe, + .remove = g12a_mdio_mux_remove, + .driver = { + .name = "g12a-mdio_mux", + .of_match_table = g12a_mdio_mux_match, + }, +}; +module_platform_driver(g12a_mdio_mux_driver); + +MODULE_DESCRIPTION("Amlogic G12a MDIO multiplexer driver"); +MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>"); +MODULE_LICENSE("GPL v2"); -- 2.20.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support @ 2019-03-14 14:01 ` Jerome Brunet 0 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-14 14:01 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman Cc: netdev, linux-amlogic, linux-kernel, devicetree, Jerome Brunet Add support for the mdio mux and internal phy glue of the g12a SoC family Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/net/phy/Kconfig | 10 + drivers/net/phy/Makefile | 1 + drivers/net/phy/mdio-mux-meson-g12a.c | 371 ++++++++++++++++++++++++++ 3 files changed, 382 insertions(+) create mode 100644 drivers/net/phy/mdio-mux-meson-g12a.c diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 071869db44cf..831aa350b1cb 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -74,6 +74,16 @@ config MDIO_BUS_MUX_GPIO several child MDIO busses to a parent bus. Child bus selection is under the control of GPIO lines. +config MDIO_BUS_MUX_MESON_G12A + tristate "Amlogic G12a based MDIO bus multiplexer" + depends on ARCH_MESON || COMPILE_TEST + depends on OF_MDIO && HAS_IOMEM + select MDIO_BUS_MUX + help + This module provides a driver for the MDIO multiplexer/glue of + the amlogic g12a SoC. The multiplexers connects either the external + or the internal MDIO bus to the parent bus. + config MDIO_BUS_MUX_MMIOREG tristate "MMIO device-controlled MDIO bus multiplexers" depends on OF_MDIO && HAS_IOMEM diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index ece5dae67174..27d7f9f3b0de 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o obj-$(CONFIG_MDIO_BUS_MUX_BCM_IPROC) += mdio-mux-bcm-iproc.o obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o +obj-$(CONFIG_MDIO_BUS_MUX_MESON_G12A) += mdio-mux-meson-g12a.o obj-$(CONFIG_MDIO_BUS_MUX_MMIOREG) += mdio-mux-mmioreg.o obj-$(CONFIG_MDIO_BUS_MUX_MULTIPLEXER) += mdio-mux-multiplexer.o obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio-mux-meson-g12a.c new file mode 100644 index 000000000000..9da681cce7cd --- /dev/null +++ b/drivers/net/phy/mdio-mux-meson-g12a.c @@ -0,0 +1,371 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2019 Baylibre, SAS. + * Author: Jerome Brunet <jbrunet@baylibre.com> + */ + +#include <linux/io.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/device.h> +#include <linux/module.h> +#include <linux/phy.h> +#include <linux/mdio-mux.h> +#include <linux/clk-provider.h> +#include <linux/clk.h> +#include <linux/bitfield.h> +#include <linux/iopoll.h> + +#define ETH_PLL_STS 0x40 +#define ETH_PLL_CTL0 0x44 +#define PLL_CTL0_LOCK_DIG BIT(30) +#define PLL_CTL0_RST BIT(29) +#define PLL_CTL0_EN BIT(28) +#define PLL_CTL0_SEL BIT(23) +#define PLL_CTL0_N GENMASK(14, 10) +#define PLL_CTL0_M GENMASK(8, 0) +#define PLL_LOCK_TIMEOUT 1000000 +#define PLL_MUX_NUM_PARENT 2 +#define ETH_PLL_CTL1 0x48 +#define ETH_PLL_CTL2 0x4c +#define ETH_PLL_CTL3 0x50 +#define ETH_PLL_CTL4 0x54 +#define ETH_PLL_CTL5 0x58 +#define ETH_PLL_CTL6 0x5c +#define ETH_PLL_CTL7 0x60 + +#define ETH_PHY_CNTL0 0x80 +#define EPHY_G12A_ID 0x33000180 +#define ETH_PHY_CNTL1 0x84 +#define PHY_CNTL1_ST_MODE GENMASK(2, 0) +#define PHY_CNTL1_ST_PHYADD GENMASK(7, 3) +#define EPHY_DFLT_ADD 8 +#define PHY_CNTL1_MII_MODE GENMASK(15, 14) +#define EPHY_MODE_RMII 0x1 +#define PHY_CNTL1_CLK_EN BIT(16) +#define PHY_CNTL1_CLKFREQ BIT(17) +#define PHY_CNTL1_PHY_ENB BIT(18) +#define ETH_PHY_CNTL2 0x88 +#define PHY_CNTL2_USE_INTERNAL BIT(5) +#define PHY_CNTL2_SMI_SRC_MAC BIT(6) +#define PHY_CNTL2_RX_CLK_EPHY BIT(9) + +#define MESON_G12A_MDIO_EXTERNAL_ID 0 +#define MESON_G12A_MDIO_INTERNAL_ID 1 + +struct g12a_mdio_mux { + void __iomem *regs; + struct clk *pclk; + struct clk *pll; + bool pll_is_enabled; + void *mux_handle; +}; + +struct g12a_ephy_pll { + struct clk_hw hw; + void __iomem *base; +}; + +#define g12a_ephy_pll_to_dev(_hw) \ + container_of(_hw, struct g12a_ephy_pll, hw) \ + +static unsigned long g12a_ephy_pll_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + u32 val, m, n; + + val = readl(pll->base + ETH_PLL_CTL0); + m = FIELD_GET(PLL_CTL0_M, val); + n = FIELD_GET(PLL_CTL0_N, val); + + return parent_rate * m / n; +} + +static int g12a_ephy_pll_enable(struct clk_hw *hw) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + u32 val = readl(pll->base + ETH_PLL_CTL0); + + /* Apply both enable an reset */ + val |= PLL_CTL0_RST | PLL_CTL0_EN; + writel(val, pll->base + ETH_PLL_CTL0); + + /* Clear the reset to let PLL lock */ + val &= ~PLL_CTL0_RST; + writel(val, pll->base + ETH_PLL_CTL0); + + /* Poll on the digital lock instead of the usual analog lock + * This is done because bit 31 is unreliable on some SoC. Bit + * 31 may indicate that the PLL is not lock eventhough the clock + * is actually running + */ + return readl_poll_timeout(pll->base + ETH_PLL_CTL0, val, + val & PLL_CTL0_LOCK_DIG, 0, PLL_LOCK_TIMEOUT); +} + +static void g12a_ephy_pll_disable(struct clk_hw *hw) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + u32 val; + + val = readl(pll->base + ETH_PLL_CTL0); + val &= ~PLL_CTL0_EN; + val |= PLL_CTL0_RST; + writel(val, pll->base + ETH_PLL_CTL0); +} + +static int g12a_ephy_pll_is_enabled(struct clk_hw *hw) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + unsigned int val; + + val = readl(pll->base + ETH_PLL_CTL0); + + return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0; +} + +static void g12a_ephy_pll_init(struct clk_hw *hw) +{ + struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); + + /* Apply PLL HW settings */ + writel(0x29c0040a, pll->base + ETH_PLL_CTL0); + writel(0x927e0000, pll->base + ETH_PLL_CTL1); + writel(0xac5f49e5, pll->base + ETH_PLL_CTL2); + writel(0x00000000, pll->base + ETH_PLL_CTL3); + writel(0x00000000, pll->base + ETH_PLL_CTL4); + writel(0x20200000, pll->base + ETH_PLL_CTL5); + writel(0x0000c002, pll->base + ETH_PLL_CTL6); + writel(0x00000023, pll->base + ETH_PLL_CTL7); +} + +static const struct clk_ops g12a_ephy_pll_ops = { + .recalc_rate = g12a_ephy_pll_recalc_rate, + .is_enabled = g12a_ephy_pll_is_enabled, + .enable = g12a_ephy_pll_enable, + .disable = g12a_ephy_pll_disable, + .init = g12a_ephy_pll_init, +}; + +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) +{ + int ret; + + /* Enable the phy clock */ + if (!priv->pll_is_enabled) { + ret = clk_prepare_enable(priv->pll); + if (ret) + return ret; + } + + priv->pll_is_enabled = true; + + /* Initialize ephy control */ + writel(EPHY_G12A_ID, priv->regs + ETH_PHY_CNTL0); + writel(FIELD_PREP(PHY_CNTL1_ST_MODE, 3) | + FIELD_PREP(PHY_CNTL1_ST_PHYADD, EPHY_DFLT_ADD) | + FIELD_PREP(PHY_CNTL1_MII_MODE, EPHY_MODE_RMII) | + PHY_CNTL1_CLK_EN | + PHY_CNTL1_CLKFREQ | + PHY_CNTL1_PHY_ENB, + priv->regs + ETH_PHY_CNTL1); + writel(PHY_CNTL2_USE_INTERNAL | + PHY_CNTL2_SMI_SRC_MAC | + PHY_CNTL2_RX_CLK_EPHY, + priv->regs + ETH_PHY_CNTL2); + + return 0; +} + +static int _g12a_enable_external_mdio(struct g12a_mdio_mux *priv) +{ + /* Reset the mdio bus mux */ + writel_relaxed(0x0, priv->regs + ETH_PHY_CNTL2); + + /* Disable the phy clock if enabled */ + if (priv->pll_is_enabled) { + clk_disable_unprepare(priv->pll); + priv->pll_is_enabled = false; + } + + return 0; +} + +static int g12a_mdio_switch_fn(int current_child, int desired_child, + void *data) +{ + struct device *dev = data; + struct g12a_mdio_mux *priv = dev_get_drvdata(dev); + + if (current_child == desired_child) + return 0; + + switch (desired_child) { + case MESON_G12A_MDIO_EXTERNAL_ID: + return _g12a_enable_external_mdio(priv); + case MESON_G12A_MDIO_INTERNAL_ID: + return _g12a_enable_internal_mdio(priv); + default: + return -EINVAL; + } +} + +static const struct of_device_id g12a_mdio_mux_match[] = { + { .compatible = "amlogic,g12a-mdio-mux", }, + {}, +}; +MODULE_DEVICE_TABLE(of, g12a_mdio_mux_match); + +static int g12a_ephy_glue_clk_register(struct device *dev) +{ + struct g12a_mdio_mux *priv = dev_get_drvdata(dev); + const char *parent_names[PLL_MUX_NUM_PARENT]; + struct clk_init_data init; + struct clk_mux *mux; + struct g12a_ephy_pll *pll; + struct clk *clk; + char *name; + int i; + + /* get the mux parents */ + for (i = 0; i < PLL_MUX_NUM_PARENT; i++) { + char in_name[8]; + + snprintf(in_name, sizeof(in_name), "clkin%d", i); + clk = devm_clk_get(dev, in_name); + if (IS_ERR(clk)) { + if (PTR_ERR(clk) != -EPROBE_DEFER) + dev_err(dev, "Missing clock %s\n", in_name); + return PTR_ERR(clk); + } + + parent_names[i] = __clk_get_name(clk); + } + + /* create the input mux */ + mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL); + if (!mux) + return -ENOMEM; + + name = kasprintf(GFP_KERNEL, "%s#mux", dev_name(dev)); + if (!name) + return -ENOMEM; + + init.name = name; + init.ops = &clk_mux_ro_ops; + init.flags = 0; + init.parent_names = parent_names; + init.num_parents = PLL_MUX_NUM_PARENT; + + mux->reg = priv->regs + ETH_PLL_CTL0; + mux->shift = __ffs(PLL_CTL0_SEL); + mux->mask = PLL_CTL0_SEL >> mux->shift; + mux->hw.init = &init; + + clk = devm_clk_register(dev, &mux->hw); + kfree(name); + if (IS_ERR(clk)) { + dev_err(dev, "failed to register input mux\n"); + return PTR_ERR(clk); + } + + /* create the pll */ + pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL); + if (!pll) + return -ENOMEM; + + name = kasprintf(GFP_KERNEL, "%s#pll", dev_name(dev)); + if (!name) + return -ENOMEM; + + init.name = name; + init.ops = &g12a_ephy_pll_ops; + init.flags = 0; + parent_names[0] = __clk_get_name(clk); + init.parent_names = parent_names; + init.num_parents = 1; + + pll->base = priv->regs; + pll->hw.init = &init; + + clk = devm_clk_register(dev, &pll->hw); + kfree(name); + if (IS_ERR(clk)) { + dev_err(dev, "failed to register input mux\n"); + return PTR_ERR(clk); + } + + priv->pll = clk; + + return 0; +} + +static int g12a_mdio_mux_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct resource *res; + struct g12a_mdio_mux *priv; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + platform_set_drvdata(pdev, priv); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + priv->regs = devm_ioremap_resource(dev, res); + if (IS_ERR(priv->regs)) + return PTR_ERR(priv->regs); + + priv->pclk = devm_clk_get(dev, "pclk"); + if (IS_ERR(priv->pclk)) { + ret = PTR_ERR(priv->pclk); + if (ret != -EPROBE_DEFER) + dev_err(dev, "failed to get peripheral clock\n"); + return ret; + } + + /* Make sure the device registers are clocked */ + ret = clk_prepare_enable(priv->pclk); + if (ret) { + dev_err(dev, "failed to enable peripheral clock"); + return ret; + } + + /* Register PLL in CCF */ + ret = g12a_ephy_glue_clk_register(dev); + if (ret) + return ret; + + return mdio_mux_init(dev, dev->of_node, g12a_mdio_switch_fn, + &priv->mux_handle, dev, NULL); +} + +static int g12a_mdio_mux_remove(struct platform_device *pdev) +{ + struct g12a_mdio_mux *priv = platform_get_drvdata(pdev); + + mdio_mux_uninit(priv->mux_handle); + + if (priv->pll_is_enabled) + clk_disable_unprepare(priv->pll); + + clk_disable_unprepare(priv->pclk); + + return 0; +} + +static struct platform_driver g12a_mdio_mux_driver = { + .probe = g12a_mdio_mux_probe, + .remove = g12a_mdio_mux_remove, + .driver = { + .name = "g12a-mdio_mux", + .of_match_table = g12a_mdio_mux_match, + }, +}; +module_platform_driver(g12a_mdio_mux_driver); + +MODULE_DESCRIPTION("Amlogic G12a MDIO multiplexer driver"); +MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>"); +MODULE_LICENSE("GPL v2"); -- 2.20.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support 2019-03-14 14:01 ` Jerome Brunet @ 2019-03-16 2:54 ` Andrew Lunn -1 siblings, 0 replies; 24+ messages in thread From: Andrew Lunn @ 2019-03-16 2:54 UTC (permalink / raw) To: Jerome Brunet Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman, netdev, linux-amlogic, linux-kernel, devicetree On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: >> +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) > You would generally use the _ prefix when you have a locked and an unlocked version. I don't see anything like this here. So please drop the _ . Nice to see the generic clock framework being used. I just wonder if this is the correct place to have this clock code. Can it be made part of the SoC clock code? Andrew ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support @ 2019-03-16 2:54 ` Andrew Lunn 0 siblings, 0 replies; 24+ messages in thread From: Andrew Lunn @ 2019-03-16 2:54 UTC (permalink / raw) To: Jerome Brunet Cc: devicetree, Florian Fainelli, Kevin Hilman, linux-kernel, netdev, linux-amlogic, David S. Miller, Heiner Kallweit On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: >> +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) > You would generally use the _ prefix when you have a locked and an unlocked version. I don't see anything like this here. So please drop the _ . Nice to see the generic clock framework being used. I just wonder if this is the correct place to have this clock code. Can it be made part of the SoC clock code? Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support 2019-03-16 2:54 ` Andrew Lunn @ 2019-03-16 17:02 ` Jerome Brunet -1 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-16 17:02 UTC (permalink / raw) To: Andrew Lunn Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman, netdev, linux-amlogic, linux-kernel, devicetree On Sat, 2019-03-16 at 03:54 +0100, Andrew Lunn wrote: > On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > > > +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) > > You would generally use the _ prefix when you have a locked and an > unlocked version. I don't see anything like this here. So please drop > the _ . > will do > Nice to see the generic clock framework being used. I just wonder if > this is the correct place to have this clock code. Can it be made part > of the SoC clock code? the PLL is local to this particular device. In 'Soc clock code' (driver/clk/meson in this case) we usually put clock controllers. Those controllers feeds the different devices of the SoC but we tends some more clock elements in the consumer device Usually, it is just a few mux, dividers and gates (like in drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c) but in this case, we have a PLL. IMO, it belongs here. Do you see a problem with this ? Jerome > > Andrew ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support @ 2019-03-16 17:02 ` Jerome Brunet 0 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-16 17:02 UTC (permalink / raw) To: Andrew Lunn Cc: devicetree, Florian Fainelli, Kevin Hilman, linux-kernel, netdev, linux-amlogic, David S. Miller, Heiner Kallweit On Sat, 2019-03-16 at 03:54 +0100, Andrew Lunn wrote: > On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > > > +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) > > You would generally use the _ prefix when you have a locked and an > unlocked version. I don't see anything like this here. So please drop > the _ . > will do > Nice to see the generic clock framework being used. I just wonder if > this is the correct place to have this clock code. Can it be made part > of the SoC clock code? the PLL is local to this particular device. In 'Soc clock code' (driver/clk/meson in this case) we usually put clock controllers. Those controllers feeds the different devices of the SoC but we tends some more clock elements in the consumer device Usually, it is just a few mux, dividers and gates (like in drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c) but in this case, we have a PLL. IMO, it belongs here. Do you see a problem with this ? Jerome > > Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support 2019-03-16 17:02 ` Jerome Brunet @ 2019-03-17 15:48 ` Andrew Lunn -1 siblings, 0 replies; 24+ messages in thread From: Andrew Lunn @ 2019-03-17 15:48 UTC (permalink / raw) To: Jerome Brunet Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman, netdev, linux-amlogic, linux-kernel, devicetree On Sat, Mar 16, 2019 at 06:02:45PM +0100, Jerome Brunet wrote: > On Sat, 2019-03-16 at 03:54 +0100, Andrew Lunn wrote: > > On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > > > > +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) > > > > You would generally use the _ prefix when you have a locked and an > > unlocked version. I don't see anything like this here. So please drop > > the _ . > > > > will do > > > Nice to see the generic clock framework being used. I just wonder if > > this is the correct place to have this clock code. Can it be made part > > of the SoC clock code? > > the PLL is local to this particular device. > > In 'Soc clock code' (driver/clk/meson in this case) we usually put clock > controllers. Those controllers feeds the different devices of the SoC but we > tends some more clock elements in the consumer device > > Usually, it is just a few mux, dividers and gates (like in > drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c) but in this case, we have > a PLL. IMO, it belongs here. Do you see a problem with this ? Hi Jerome Some maintainers like to have code in their directory structure. It would be good to check that the clock maintainer in happy with clock code under the network driver subtree. Also, the clock maintainer should also review this code. So please at least Cc: the clock maintainer and clock list on the next submission, as well as netdev. I personally don't know the clock code well enough to review such code. Andrew ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support @ 2019-03-17 15:48 ` Andrew Lunn 0 siblings, 0 replies; 24+ messages in thread From: Andrew Lunn @ 2019-03-17 15:48 UTC (permalink / raw) To: Jerome Brunet Cc: devicetree, Florian Fainelli, Kevin Hilman, linux-kernel, netdev, linux-amlogic, David S. Miller, Heiner Kallweit On Sat, Mar 16, 2019 at 06:02:45PM +0100, Jerome Brunet wrote: > On Sat, 2019-03-16 at 03:54 +0100, Andrew Lunn wrote: > > On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > > > > +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) > > > > You would generally use the _ prefix when you have a locked and an > > unlocked version. I don't see anything like this here. So please drop > > the _ . > > > > will do > > > Nice to see the generic clock framework being used. I just wonder if > > this is the correct place to have this clock code. Can it be made part > > of the SoC clock code? > > the PLL is local to this particular device. > > In 'Soc clock code' (driver/clk/meson in this case) we usually put clock > controllers. Those controllers feeds the different devices of the SoC but we > tends some more clock elements in the consumer device > > Usually, it is just a few mux, dividers and gates (like in > drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c) but in this case, we have > a PLL. IMO, it belongs here. Do you see a problem with this ? Hi Jerome Some maintainers like to have code in their directory structure. It would be good to check that the clock maintainer in happy with clock code under the network driver subtree. Also, the clock maintainer should also review this code. So please at least Cc: the clock maintainer and clock list on the next submission, as well as netdev. I personally don't know the clock code well enough to review such code. Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support 2019-03-17 15:48 ` Andrew Lunn @ 2019-03-18 9:36 ` Jerome Brunet -1 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-18 9:36 UTC (permalink / raw) To: Andrew Lunn Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman, netdev, linux-amlogic, linux-kernel, devicetree On Sun, 2019-03-17 at 16:48 +0100, Andrew Lunn wrote: > On Sat, Mar 16, 2019 at 06:02:45PM +0100, Jerome Brunet wrote: > > On Sat, 2019-03-16 at 03:54 +0100, Andrew Lunn wrote: > > > On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > > > > > +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) > > > > > > You would generally use the _ prefix when you have a locked and an > > > unlocked version. I don't see anything like this here. So please drop > > > the _ . > > > > > > > will do > > > > > Nice to see the generic clock framework being used. I just wonder if > > > this is the correct place to have this clock code. Can it be made part > > > of the SoC clock code? > > > > the PLL is local to this particular device. > > > > In 'Soc clock code' (driver/clk/meson in this case) we usually put clock > > controllers. Those controllers feeds the different devices of the SoC but we > > tends some more clock elements in the consumer device > > > > Usually, it is just a few mux, dividers and gates (like in > > drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c) but in this case, we have > > a PLL. IMO, it belongs here. Do you see a problem with this ? > > Hi Jerome > > Some maintainers like to have code in their directory structure. It > would be good to check that the clock maintainer in happy with clock > code under the network driver subtree. Also, the clock maintainer > should also review this code. So please at least Cc: the clock > maintainer and clock list on the next submission, I happen to be maintaining Amlogic clocks ;) but I can Cc Mike and Stephen if you prefer. > as well as netdev. I > personally don't know the clock code well enough to review such code. No problem. FYI, It is not widespread yet but more and more drivers are creating clocks using CCF. Here is a quick grep of the files including <linux/clk-provider.h>, excluding "drivers/clk/*" drivers/acpi/acpi_amba.c drivers/acpi/acpi_apd.c drivers/acpi/acpi_lpss.c drivers/clocksource/arc_timer.c drivers/clocksource/timer-ti-dm.c drivers/cpufreq/loongson1-cpufreq.c drivers/cpufreq/qoriq-cpufreq.c drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c drivers/gpu/drm/imx/imx-tve.c drivers/gpu/drm/mediatek/mtk_mipi_tx.c drivers/gpu/drm/mediatek/mtk_hdmi_phy.h drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_pll.c drivers/gpu/drm/msm/dsi/pll/dsi_pll.h drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c drivers/gpu/drm/pl111/pl111_drm.h drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c drivers/gpu/drm/sun4i/sun4i_dotclock.c drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c drivers/gpu/drm/sun4i/sun8i_tcon_top.h drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c drivers/gpu/drm/sun4i/sun4i_crtc.c drivers/gpu/drm/tegra/sor.c drivers/gpu/drm/vc4/vc4_dsi.c drivers/gpu/ipu-v3/ipu-csi.c drivers/i2c/busses/i2c-designware-platdrv.c drivers/iio/adc/aspeed_adc.c drivers/iio/adc/meson_saradc.c drivers/mailbox/mtk-cmdq-mailbox.c drivers/media/i2c/ov5640.c drivers/media/platform/atmel/atmel-isc.c drivers/media/platform/exynos4-is/media-dev.c drivers/media/platform/exynos4-is/media-dev.h drivers/media/platform/omap3isp/isp.h drivers/memory/tegra/tegra124-emc.c drivers/mfd/intel-lpss.c drivers/mfd/intel_quark_i2c_gpio.c drivers/mmc/host/meson-mx-sdio.c drivers/mmc/host/sdhci-of-arasan.c drivers/mmc/host/meson-gx-mmc.c drivers/net/ethernet/cadence/macb_pci.c drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c drivers/net/ieee802154/ca8210.c drivers/phy/qualcomm/phy-qcom-qmp.c drivers/phy/rockchip/phy-rockchip-inno-hdmi.c drivers/phy/rockchip/phy-rockchip-usb.c drivers/phy/rockchip/phy-rockchip-typec.c drivers/phy/rockchip/phy-rockchip-inno-usb2.c drivers/pinctrl/tegra/pinctrl-tegra20.c drivers/pwm/pwm-meson.c drivers/rtc/rtc-ac100.c drivers/rtc/rtc-pcf8563.c drivers/rtc/rtc-m41t80.c drivers/rtc/rtc-sun6i.c drivers/rtc/rtc-ds1307.c drivers/rtc/rtc-hym8563.c drivers/spi/spi-pxa2xx-pci.c drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c drivers/thermal/st/stm_thermal.c drivers/usb/dwc3/dwc3-qcom.c > > Andrew ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support @ 2019-03-18 9:36 ` Jerome Brunet 0 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-18 9:36 UTC (permalink / raw) To: Andrew Lunn Cc: devicetree, Florian Fainelli, Kevin Hilman, linux-kernel, netdev, linux-amlogic, David S. Miller, Heiner Kallweit On Sun, 2019-03-17 at 16:48 +0100, Andrew Lunn wrote: > On Sat, Mar 16, 2019 at 06:02:45PM +0100, Jerome Brunet wrote: > > On Sat, 2019-03-16 at 03:54 +0100, Andrew Lunn wrote: > > > On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > > > > > +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) > > > > > > You would generally use the _ prefix when you have a locked and an > > > unlocked version. I don't see anything like this here. So please drop > > > the _ . > > > > > > > will do > > > > > Nice to see the generic clock framework being used. I just wonder if > > > this is the correct place to have this clock code. Can it be made part > > > of the SoC clock code? > > > > the PLL is local to this particular device. > > > > In 'Soc clock code' (driver/clk/meson in this case) we usually put clock > > controllers. Those controllers feeds the different devices of the SoC but we > > tends some more clock elements in the consumer device > > > > Usually, it is just a few mux, dividers and gates (like in > > drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c) but in this case, we have > > a PLL. IMO, it belongs here. Do you see a problem with this ? > > Hi Jerome > > Some maintainers like to have code in their directory structure. It > would be good to check that the clock maintainer in happy with clock > code under the network driver subtree. Also, the clock maintainer > should also review this code. So please at least Cc: the clock > maintainer and clock list on the next submission, I happen to be maintaining Amlogic clocks ;) but I can Cc Mike and Stephen if you prefer. > as well as netdev. I > personally don't know the clock code well enough to review such code. No problem. FYI, It is not widespread yet but more and more drivers are creating clocks using CCF. Here is a quick grep of the files including <linux/clk-provider.h>, excluding "drivers/clk/*" drivers/acpi/acpi_amba.c drivers/acpi/acpi_apd.c drivers/acpi/acpi_lpss.c drivers/clocksource/arc_timer.c drivers/clocksource/timer-ti-dm.c drivers/cpufreq/loongson1-cpufreq.c drivers/cpufreq/qoriq-cpufreq.c drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c drivers/gpu/drm/imx/imx-tve.c drivers/gpu/drm/mediatek/mtk_mipi_tx.c drivers/gpu/drm/mediatek/mtk_hdmi_phy.h drivers/gpu/drm/msm/disp/mdp4/mdp4_lvds_pll.c drivers/gpu/drm/msm/dsi/pll/dsi_pll.h drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c drivers/gpu/drm/msm/dsi/pll/dsi_pll_14nm.c drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm.c drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c drivers/gpu/drm/msm/hdmi/hdmi_pll_8960.c drivers/gpu/drm/pl111/pl111_drm.h drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c drivers/gpu/drm/sun4i/sun4i_dotclock.c drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c drivers/gpu/drm/sun4i/sun8i_tcon_top.h drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c drivers/gpu/drm/sun4i/sun4i_crtc.c drivers/gpu/drm/tegra/sor.c drivers/gpu/drm/vc4/vc4_dsi.c drivers/gpu/ipu-v3/ipu-csi.c drivers/i2c/busses/i2c-designware-platdrv.c drivers/iio/adc/aspeed_adc.c drivers/iio/adc/meson_saradc.c drivers/mailbox/mtk-cmdq-mailbox.c drivers/media/i2c/ov5640.c drivers/media/platform/atmel/atmel-isc.c drivers/media/platform/exynos4-is/media-dev.c drivers/media/platform/exynos4-is/media-dev.h drivers/media/platform/omap3isp/isp.h drivers/memory/tegra/tegra124-emc.c drivers/mfd/intel-lpss.c drivers/mfd/intel_quark_i2c_gpio.c drivers/mmc/host/meson-mx-sdio.c drivers/mmc/host/sdhci-of-arasan.c drivers/mmc/host/meson-gx-mmc.c drivers/net/ethernet/cadence/macb_pci.c drivers/net/ethernet/stmicro/stmmac/dwmac-dwc-qos-eth.c drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c drivers/net/ieee802154/ca8210.c drivers/phy/qualcomm/phy-qcom-qmp.c drivers/phy/rockchip/phy-rockchip-inno-hdmi.c drivers/phy/rockchip/phy-rockchip-usb.c drivers/phy/rockchip/phy-rockchip-typec.c drivers/phy/rockchip/phy-rockchip-inno-usb2.c drivers/pinctrl/tegra/pinctrl-tegra20.c drivers/pwm/pwm-meson.c drivers/rtc/rtc-ac100.c drivers/rtc/rtc-pcf8563.c drivers/rtc/rtc-m41t80.c drivers/rtc/rtc-sun6i.c drivers/rtc/rtc-ds1307.c drivers/rtc/rtc-hym8563.c drivers/spi/spi-pxa2xx-pci.c drivers/staging/clocking-wizard/clk-xlnx-clock-wizard.c drivers/thermal/st/stm_thermal.c drivers/usb/dwc3/dwc3-qcom.c > > Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support 2019-03-17 15:48 ` Andrew Lunn @ 2019-03-18 9:45 ` Neil Armstrong -1 siblings, 0 replies; 24+ messages in thread From: Neil Armstrong @ 2019-03-18 9:45 UTC (permalink / raw) To: Andrew Lunn, Jerome Brunet Cc: devicetree, Florian Fainelli, Kevin Hilman, linux-kernel, netdev, linux-amlogic, David S. Miller, Heiner Kallweit Hi Andrew, On 17/03/2019 16:48, Andrew Lunn wrote: > On Sat, Mar 16, 2019 at 06:02:45PM +0100, Jerome Brunet wrote: >> On Sat, 2019-03-16 at 03:54 +0100, Andrew Lunn wrote: >>> On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: >>>>> +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) >>> >>> You would generally use the _ prefix when you have a locked and an >>> unlocked version. I don't see anything like this here. So please drop >>> the _ . >>> >> >> will do >> >>> Nice to see the generic clock framework being used. I just wonder if >>> this is the correct place to have this clock code. Can it be made part >>> of the SoC clock code? >> >> the PLL is local to this particular device. >> >> In 'Soc clock code' (driver/clk/meson in this case) we usually put clock >> controllers. Those controllers feeds the different devices of the SoC but we >> tends some more clock elements in the consumer device >> >> Usually, it is just a few mux, dividers and gates (like in >> drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c) but in this case, we have >> a PLL. IMO, it belongs here. Do you see a problem with this ? > > Hi Jerome > > Some maintainers like to have code in their directory structure. It > would be good to check that the clock maintainer in happy with clock > code under the network driver subtree. Also, the clock maintainer > should also review this code. So please at least Cc: the clock > maintainer and clock list on the next submission, as well as netdev. I > personally don't know the clock code well enough to review such code. If it can help, I also co-maintain the Amlogic Clocks drivers, and I did not interfere with the review process in purpose, but if it can help : For the clock code : Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> Neil > > Andrew > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support @ 2019-03-18 9:45 ` Neil Armstrong 0 siblings, 0 replies; 24+ messages in thread From: Neil Armstrong @ 2019-03-18 9:45 UTC (permalink / raw) To: Andrew Lunn, Jerome Brunet Cc: devicetree, Florian Fainelli, netdev, linux-kernel, Kevin Hilman, linux-amlogic, David S. Miller, Heiner Kallweit Hi Andrew, On 17/03/2019 16:48, Andrew Lunn wrote: > On Sat, Mar 16, 2019 at 06:02:45PM +0100, Jerome Brunet wrote: >> On Sat, 2019-03-16 at 03:54 +0100, Andrew Lunn wrote: >>> On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: >>>>> +static int _g12a_enable_internal_mdio(struct g12a_mdio_mux *priv) >>> >>> You would generally use the _ prefix when you have a locked and an >>> unlocked version. I don't see anything like this here. So please drop >>> the _ . >>> >> >> will do >> >>> Nice to see the generic clock framework being used. I just wonder if >>> this is the correct place to have this clock code. Can it be made part >>> of the SoC clock code? >> >> the PLL is local to this particular device. >> >> In 'Soc clock code' (driver/clk/meson in this case) we usually put clock >> controllers. Those controllers feeds the different devices of the SoC but we >> tends some more clock elements in the consumer device >> >> Usually, it is just a few mux, dividers and gates (like in >> drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c) but in this case, we have >> a PLL. IMO, it belongs here. Do you see a problem with this ? > > Hi Jerome > > Some maintainers like to have code in their directory structure. It > would be good to check that the clock maintainer in happy with clock > code under the network driver subtree. Also, the clock maintainer > should also review this code. So please at least Cc: the clock > maintainer and clock list on the next submission, as well as netdev. I > personally don't know the clock code well enough to review such code. If it can help, I also co-maintain the Amlogic Clocks drivers, and I did not interfere with the review process in purpose, but if it can help : For the clock code : Reviewed-by: Neil Armstrong <narmstrong@baylibre.com> Neil > > Andrew > > _______________________________________________ > linux-amlogic mailing list > linux-amlogic@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-amlogic > _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support 2019-03-14 14:01 ` Jerome Brunet @ 2019-03-17 16:14 ` Andrew Lunn -1 siblings, 0 replies; 24+ messages in thread From: Andrew Lunn @ 2019-03-17 16:14 UTC (permalink / raw) To: Jerome Brunet Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman, netdev, linux-amlogic, linux-kernel, devicetree On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > Add support for the mdio mux and internal phy glue of the g12a SoC family > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/net/phy/Kconfig | 10 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/mdio-mux-meson-g12a.c | 371 ++++++++++++++++++++++++++ > 3 files changed, 382 insertions(+) > create mode 100644 drivers/net/phy/mdio-mux-meson-g12a.c > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 071869db44cf..831aa350b1cb 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -74,6 +74,16 @@ config MDIO_BUS_MUX_GPIO > several child MDIO busses to a parent bus. Child bus > selection is under the control of GPIO lines. > > +config MDIO_BUS_MUX_MESON_G12A > + tristate "Amlogic G12a based MDIO bus multiplexer" > + depends on ARCH_MESON || COMPILE_TEST > + depends on OF_MDIO && HAS_IOMEM > + select MDIO_BUS_MUX Hi Jerome Do you need some clock depends? > +static int g12a_mdio_switch_fn(int current_child, int desired_child, > + void *data) > +{ > + struct device *dev = data; > + struct g12a_mdio_mux *priv = dev_get_drvdata(dev); David won't like that you don't have reverse Christmas tree. You need to do the assignment to priv in the body of the code. Or can you pass data directly to dev_get_drvdata? > +static int g12a_mdio_mux_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct resource *res; > + struct g12a_mdio_mux *priv; > + int ret; Reverse Christmas tree please. > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, priv); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + priv->regs = devm_ioremap_resource(dev, res); > + if (IS_ERR(priv->regs)) > + return PTR_ERR(priv->regs); > + > + priv->pclk = devm_clk_get(dev, "pclk"); > + if (IS_ERR(priv->pclk)) { > + ret = PTR_ERR(priv->pclk); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get peripheral clock\n"); > + return ret; > + } > + > + /* Make sure the device registers are clocked */ > + ret = clk_prepare_enable(priv->pclk); > + if (ret) { > + dev_err(dev, "failed to enable peripheral clock"); > + return ret; > + } > + > + /* Register PLL in CCF */ > + ret = g12a_ephy_glue_clk_register(dev); On error, you are not disabling the peripheral clock. > + if (ret) > + return ret; > + > + return mdio_mux_init(dev, dev->of_node, g12a_mdio_switch_fn, > + &priv->mux_handle, dev, NULL); > +} Andrew ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support @ 2019-03-17 16:14 ` Andrew Lunn 0 siblings, 0 replies; 24+ messages in thread From: Andrew Lunn @ 2019-03-17 16:14 UTC (permalink / raw) To: Jerome Brunet Cc: devicetree, Florian Fainelli, Kevin Hilman, linux-kernel, netdev, linux-amlogic, David S. Miller, Heiner Kallweit On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > Add support for the mdio mux and internal phy glue of the g12a SoC family > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > drivers/net/phy/Kconfig | 10 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/mdio-mux-meson-g12a.c | 371 ++++++++++++++++++++++++++ > 3 files changed, 382 insertions(+) > create mode 100644 drivers/net/phy/mdio-mux-meson-g12a.c > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 071869db44cf..831aa350b1cb 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -74,6 +74,16 @@ config MDIO_BUS_MUX_GPIO > several child MDIO busses to a parent bus. Child bus > selection is under the control of GPIO lines. > > +config MDIO_BUS_MUX_MESON_G12A > + tristate "Amlogic G12a based MDIO bus multiplexer" > + depends on ARCH_MESON || COMPILE_TEST > + depends on OF_MDIO && HAS_IOMEM > + select MDIO_BUS_MUX Hi Jerome Do you need some clock depends? > +static int g12a_mdio_switch_fn(int current_child, int desired_child, > + void *data) > +{ > + struct device *dev = data; > + struct g12a_mdio_mux *priv = dev_get_drvdata(dev); David won't like that you don't have reverse Christmas tree. You need to do the assignment to priv in the body of the code. Or can you pass data directly to dev_get_drvdata? > +static int g12a_mdio_mux_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct resource *res; > + struct g12a_mdio_mux *priv; > + int ret; Reverse Christmas tree please. > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, priv); > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + priv->regs = devm_ioremap_resource(dev, res); > + if (IS_ERR(priv->regs)) > + return PTR_ERR(priv->regs); > + > + priv->pclk = devm_clk_get(dev, "pclk"); > + if (IS_ERR(priv->pclk)) { > + ret = PTR_ERR(priv->pclk); > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "failed to get peripheral clock\n"); > + return ret; > + } > + > + /* Make sure the device registers are clocked */ > + ret = clk_prepare_enable(priv->pclk); > + if (ret) { > + dev_err(dev, "failed to enable peripheral clock"); > + return ret; > + } > + > + /* Register PLL in CCF */ > + ret = g12a_ephy_glue_clk_register(dev); On error, you are not disabling the peripheral clock. > + if (ret) > + return ret; > + > + return mdio_mux_init(dev, dev->of_node, g12a_mdio_switch_fn, > + &priv->mux_handle, dev, NULL); > +} Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support 2019-03-17 16:14 ` Andrew Lunn @ 2019-03-18 9:34 ` Jerome Brunet -1 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-18 9:34 UTC (permalink / raw) To: Andrew Lunn Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman, netdev, linux-amlogic, linux-kernel, devicetree On Sun, 2019-03-17 at 17:14 +0100, Andrew Lunn wrote: > On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > > Add support for the mdio mux and internal phy glue of the g12a SoC family > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > --- > > drivers/net/phy/Kconfig | 10 + > > drivers/net/phy/Makefile | 1 + > > drivers/net/phy/mdio-mux-meson-g12a.c | 371 ++++++++++++++++++++++++++ > > 3 files changed, 382 insertions(+) > > create mode 100644 drivers/net/phy/mdio-mux-meson-g12a.c > > > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > > index 071869db44cf..831aa350b1cb 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -74,6 +74,16 @@ config MDIO_BUS_MUX_GPIO > > several child MDIO busses to a parent bus. Child bus > > selection is under the control of GPIO lines. > > > > +config MDIO_BUS_MUX_MESON_G12A > > + tristate "Amlogic G12a based MDIO bus multiplexer" > > + depends on ARCH_MESON || COMPILE_TEST > > + depends on OF_MDIO && HAS_IOMEM > > + select MDIO_BUS_MUX > > Hi Jerome > > Do you need some clock depends? I don't think we need more. ARCH_MESON already selects the necessary clock parts. As for the COMPILE_TEST, the clock code should be able to compile even if the common clock framework is disabled. > > > +static int g12a_mdio_switch_fn(int current_child, int desired_child, > > + void *data) > > +{ > > + struct device *dev = data; > > + struct g12a_mdio_mux *priv = dev_get_drvdata(dev); > > David won't like that you don't have reverse Christmas tree. You need > to do the assignment to priv in the body of the code. Or can you pass > data directly to dev_get_drvdata? > > > +static int g12a_mdio_mux_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct resource *res; > > + struct g12a_mdio_mux *priv; > > + int ret; > > Reverse Christmas tree please. I'll fix these > > > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + platform_set_drvdata(pdev, priv); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + priv->regs = devm_ioremap_resource(dev, res); > > + if (IS_ERR(priv->regs)) > > + return PTR_ERR(priv->regs); > > + > > + priv->pclk = devm_clk_get(dev, "pclk"); > > + if (IS_ERR(priv->pclk)) { > > + ret = PTR_ERR(priv->pclk); > > + if (ret != -EPROBE_DEFER) > > + dev_err(dev, "failed to get peripheral clock\n"); > > + return ret; > > + } > > + > > + /* Make sure the device registers are clocked */ > > + ret = clk_prepare_enable(priv->pclk); > > + if (ret) { > > + dev_err(dev, "failed to enable peripheral clock"); > > + return ret; > > + } > > + > > + /* Register PLL in CCF */ > > + ret = g12a_ephy_glue_clk_register(dev); > > On error, you are not disabling the peripheral clock. Indeed, good catch. Thx > > > + if (ret) > > + return ret; > > + > > + return mdio_mux_init(dev, dev->of_node, g12a_mdio_switch_fn, > > + &priv->mux_handle, dev, NULL); > > +} > > Andrew ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support @ 2019-03-18 9:34 ` Jerome Brunet 0 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-18 9:34 UTC (permalink / raw) To: Andrew Lunn Cc: devicetree, Florian Fainelli, Kevin Hilman, linux-kernel, netdev, linux-amlogic, David S. Miller, Heiner Kallweit On Sun, 2019-03-17 at 17:14 +0100, Andrew Lunn wrote: > On Thu, Mar 14, 2019 at 03:01:34PM +0100, Jerome Brunet wrote: > > Add support for the mdio mux and internal phy glue of the g12a SoC family > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > > --- > > drivers/net/phy/Kconfig | 10 + > > drivers/net/phy/Makefile | 1 + > > drivers/net/phy/mdio-mux-meson-g12a.c | 371 ++++++++++++++++++++++++++ > > 3 files changed, 382 insertions(+) > > create mode 100644 drivers/net/phy/mdio-mux-meson-g12a.c > > > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > > index 071869db44cf..831aa350b1cb 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -74,6 +74,16 @@ config MDIO_BUS_MUX_GPIO > > several child MDIO busses to a parent bus. Child bus > > selection is under the control of GPIO lines. > > > > +config MDIO_BUS_MUX_MESON_G12A > > + tristate "Amlogic G12a based MDIO bus multiplexer" > > + depends on ARCH_MESON || COMPILE_TEST > > + depends on OF_MDIO && HAS_IOMEM > > + select MDIO_BUS_MUX > > Hi Jerome > > Do you need some clock depends? I don't think we need more. ARCH_MESON already selects the necessary clock parts. As for the COMPILE_TEST, the clock code should be able to compile even if the common clock framework is disabled. > > > +static int g12a_mdio_switch_fn(int current_child, int desired_child, > > + void *data) > > +{ > > + struct device *dev = data; > > + struct g12a_mdio_mux *priv = dev_get_drvdata(dev); > > David won't like that you don't have reverse Christmas tree. You need > to do the assignment to priv in the body of the code. Or can you pass > data directly to dev_get_drvdata? > > > +static int g12a_mdio_mux_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct resource *res; > > + struct g12a_mdio_mux *priv; > > + int ret; > > Reverse Christmas tree please. I'll fix these > > > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + platform_set_drvdata(pdev, priv); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + priv->regs = devm_ioremap_resource(dev, res); > > + if (IS_ERR(priv->regs)) > > + return PTR_ERR(priv->regs); > > + > > + priv->pclk = devm_clk_get(dev, "pclk"); > > + if (IS_ERR(priv->pclk)) { > > + ret = PTR_ERR(priv->pclk); > > + if (ret != -EPROBE_DEFER) > > + dev_err(dev, "failed to get peripheral clock\n"); > > + return ret; > > + } > > + > > + /* Make sure the device registers are clocked */ > > + ret = clk_prepare_enable(priv->pclk); > > + if (ret) { > > + dev_err(dev, "failed to enable peripheral clock"); > > + return ret; > > + } > > + > > + /* Register PLL in CCF */ > > + ret = g12a_ephy_glue_clk_register(dev); > > On error, you are not disabling the peripheral clock. Indeed, good catch. Thx > > > + if (ret) > > + return ret; > > + > > + return mdio_mux_init(dev, dev->of_node, g12a_mdio_switch_fn, > > + &priv->mux_handle, dev, NULL); > > +} > > Andrew _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH net-next 3/3] net: phy: meson-gxl: add g12a support 2019-03-14 14:01 ` Jerome Brunet @ 2019-03-14 14:01 ` Jerome Brunet -1 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-14 14:01 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman Cc: Jerome Brunet, netdev, linux-amlogic, linux-kernel, devicetree The g12a SoC family uses the type of internal PHY that was used on the gxl family. The quirks of gxl family, like the LPA register corruption, appear to have been resolved on this new SoC generation. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/net/phy/meson-gxl.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c index 0eec2913c289..49cad0f4c79b 100644 --- a/drivers/net/phy/meson-gxl.c +++ b/drivers/net/phy/meson-gxl.c @@ -237,11 +237,25 @@ static struct phy_driver meson_gxl_phy[] = { .config_intr = meson_gxl_config_intr, .suspend = genphy_suspend, .resume = genphy_resume, + }, { + .phy_id = 0x01803301, + .phy_id_mask = 0xffffffff, + .name = "Meson G12A Internal PHY", + .features = PHY_BASIC_FEATURES, + .flags = PHY_IS_INTERNAL, + .soft_reset = genphy_soft_reset, + .aneg_done = genphy_aneg_done, + .read_status = genphy_read_status, + .ack_interrupt = meson_gxl_ack_interrupt, + .config_intr = meson_gxl_config_intr, + .suspend = genphy_suspend, + .resume = genphy_resume, }, }; static struct mdio_device_id __maybe_unused meson_gxl_tbl[] = { { 0x01814400, 0xfffffff0 }, + { 0x01803301, 0xffffffff }, { } }; -- 2.20.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH net-next 3/3] net: phy: meson-gxl: add g12a support @ 2019-03-14 14:01 ` Jerome Brunet 0 siblings, 0 replies; 24+ messages in thread From: Jerome Brunet @ 2019-03-14 14:01 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, Kevin Hilman Cc: netdev, linux-amlogic, linux-kernel, devicetree, Jerome Brunet The g12a SoC family uses the type of internal PHY that was used on the gxl family. The quirks of gxl family, like the LPA register corruption, appear to have been resolved on this new SoC generation. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/net/phy/meson-gxl.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c index 0eec2913c289..49cad0f4c79b 100644 --- a/drivers/net/phy/meson-gxl.c +++ b/drivers/net/phy/meson-gxl.c @@ -237,11 +237,25 @@ static struct phy_driver meson_gxl_phy[] = { .config_intr = meson_gxl_config_intr, .suspend = genphy_suspend, .resume = genphy_resume, + }, { + .phy_id = 0x01803301, + .phy_id_mask = 0xffffffff, + .name = "Meson G12A Internal PHY", + .features = PHY_BASIC_FEATURES, + .flags = PHY_IS_INTERNAL, + .soft_reset = genphy_soft_reset, + .aneg_done = genphy_aneg_done, + .read_status = genphy_read_status, + .ack_interrupt = meson_gxl_ack_interrupt, + .config_intr = meson_gxl_config_intr, + .suspend = genphy_suspend, + .resume = genphy_resume, }, }; static struct mdio_device_id __maybe_unused meson_gxl_tbl[] = { { 0x01814400, 0xfffffff0 }, + { 0x01803301, 0xffffffff }, { } }; -- 2.20.1 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-03-28 15:57 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-14 14:01 [PATCH net-next 0/3] net: phy: add Amlogic g12a support Jerome Brunet 2019-03-14 14:01 ` Jerome Brunet 2019-03-14 14:01 ` [PATCH net-next 1/3] dt-bindings: net: phy: add g12a mdio mux documentation Jerome Brunet 2019-03-14 14:01 ` Jerome Brunet 2019-03-28 15:57 ` Rob Herring 2019-03-28 15:57 ` Rob Herring 2019-03-14 14:01 ` [PATCH net-next 2/3] net: phy: add amlogic g12a mdio mux support Jerome Brunet 2019-03-14 14:01 ` Jerome Brunet 2019-03-16 2:54 ` Andrew Lunn 2019-03-16 2:54 ` Andrew Lunn 2019-03-16 17:02 ` Jerome Brunet 2019-03-16 17:02 ` Jerome Brunet 2019-03-17 15:48 ` Andrew Lunn 2019-03-17 15:48 ` Andrew Lunn 2019-03-18 9:36 ` Jerome Brunet 2019-03-18 9:36 ` Jerome Brunet 2019-03-18 9:45 ` Neil Armstrong 2019-03-18 9:45 ` Neil Armstrong 2019-03-17 16:14 ` Andrew Lunn 2019-03-17 16:14 ` Andrew Lunn 2019-03-18 9:34 ` Jerome Brunet 2019-03-18 9:34 ` Jerome Brunet 2019-03-14 14:01 ` [PATCH net-next 3/3] net: phy: meson-gxl: add g12a support Jerome Brunet 2019-03-14 14:01 ` Jerome Brunet
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.