| Message ID | 201310310219.58963.sergei.shtylyov@cogentembedded.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 31, 2013 at 02:19:58AM +0300, Sergei Shtylyov wrote: > Register Ether platform device and pin data on the Koelsch board. > Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Thanks, I have been able to use this to successfully boot a Koelsch board using nfsroot. However, I believe there is a minor build problem which I have commented on below. > --- > arch/arm/mach-shmobile/board-koelsch.c | 61 ++++++++++++++++++++++++++++++++- > 1 file changed, 60 insertions(+), 1 deletion(-) > > Index: renesas/arch/arm/mach-shmobile/board-koelsch.c > =================================================================== > --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c > +++ renesas/arch/arm/mach-shmobile/board-koelsch.c > @@ -24,14 +24,31 @@ > #include <linux/input.h> > #include <linux/kernel.h> > #include <linux/leds.h> > +#include <linux/phy.h> > +#include <linux/pinctrl/machine.h> > #include <linux/platform_data/gpio-rcar.h> > #include <linux/platform_device.h> > +#include <linux/sh_eth.h> > #include <mach/common.h> > +#include <mach/irqs.h> > #include <mach/r8a7791.h> > #include <mach/rcar-gen2.h> > #include <asm/mach-types.h> > #include <asm/mach/arch.h> > > +/* Ether */ > +static const struct sh_eth_plat_data ether_pdata __initconst = { > + .phy = 0x1, > + .edmac_endian = EDMAC_LITTLE_ENDIAN, > + .phy_interface = PHY_INTERFACE_MODE_RMII, > + .ether_link_active_low = 1, > +}; > + > +static const struct resource ether_resources[] __initconst = { > + DEFINE_RES_MEM(0xee700000, 0x400), > + DEFINE_RES_IRQ(gic_spi(162)), > +}; > + > /* LEDS */ > static struct gpio_led koelsch_leds[] = { > { > @@ -70,11 +87,29 @@ static const struct gpio_keys_platform_d > .nbuttons = ARRAY_SIZE(gpio_buttons), > }; > > +static const struct pinctrl_map koelsch_pinctrl_map[] = { > + /* Ether */ > + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", > + "eth_link", "eth"), > + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", > + "eth_mdio", "eth"), > + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", > + "eth_rmii", "eth"), > +}; > + > static void __init koelsch_add_standard_devices(void) > { > r8a7791_clock_init(); > + > + pinctrl_register_mappings(koelsch_pinctrl_map, > + ARRAY_SIZE(koelsch_pinctrl_map)); > r8a7791_pinmux_init(); > + > r8a7791_add_standard_devices(); > + platform_device_register_resndata(&platform_bus, "r8a7790-ether", -1, > + ether_resources, > + ARRAY_SIZE(ether_resources), > + ðer_pdata, sizeof(ether_pdata)); > platform_device_register_data(&platform_bus, "leds-gpio", -1, > &koelsch_leds_pdata, > sizeof(koelsch_leds_pdata)); > @@ -83,6 +118,30 @@ static void __init koelsch_add_standard_ > sizeof(koelsch_keys_pdata)); > } > > +/* > + * Ether LEDs on the Koelsch board are named LINK and ACTIVE which corresponds > + * to non-default 01 setting of the Micrel KSZ8041 PHY control register 1 bits > + * 14-15. We have to set them back to 01 from the default 00 value each time > + * the PHY is reset. It's also important because the PHY's LED0 signal is > + * connected to SoC's ETH_LINK signal and in the PHY's default mode it will > + * bounce on and off after each packet, which we apparently want to avoid. > + */ > +static int koelsch_ksz8041_fixup(struct phy_device *phydev) > +{ > + u16 phyctrl1 = phy_read(phydev, 0x1e); > + > + phyctrl1 &= ~0xc000; > + phyctrl1 |= 0x4000; > + return phy_write(phydev, 0x1e, phyctrl1); > +} > + > +static void __init koelsch_init(void) > +{ > + koelsch_add_standard_devices(); > + > + phy_register_fixup_for_id("r8a7790-ether-ff:01", koelsch_ksz8041_fixup); It seems to me that this code requires CONFIG_PHYLIB in order to compile. So I propose the following enhancement to this patch which replaces the line immediately above. if (IS_ENABLED(CONFIG_PHYLIB)) phy_register_fixup_for_id("r8a7790-ether-ff:01", koelsch_ksz8041_fixup); I believe a similar enhancement is also needed for board-lager.c which I just realised does not compile if CONFIG_PHYLIB is not set. > +} > + > static const char * const koelsch_boards_compat_dt[] __initconst = { > "renesas,koelsch", > NULL, > @@ -91,7 +150,7 @@ static const char * const koelsch_boards > DT_MACHINE_START(KOELSCH_DT, "koelsch") > .smp = smp_ops(r8a7791_smp_ops), > .init_early = r8a7791_init_early, > - .init_machine = koelsch_add_standard_devices, > + .init_machine = koelsch_init, > .init_time = rcar_gen2_timer_init, > .dt_compat = koelsch_boards_compat_dt, > MACHINE_END > -- > To unsubscribe from this list: send the line "unsubscribe linux-sh" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Hi Sergei, On Thu, Oct 31, 2013 at 8:19 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Register Ether platform device and pin data on the Koelsch board. > Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Thanks for your patch. Just like on the Lager board we also need to tie in the PHY IRQ pin. Polling is not good enough when the hardware can do better. Can you please share your PHY IRQ plans with us? Best, / magnus
Hi Sergei, On Thu, Oct 31, 2013 at 8:19 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Register Ether platform device and pin data on the Koelsch board. > Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. > > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > --- > arch/arm/mach-shmobile/board-koelsch.c | 61 ++++++++++++++++++++++++++++++++- > 1 file changed, 60 insertions(+), 1 deletion(-) > > Index: renesas/arch/arm/mach-shmobile/board-koelsch.c > =================================================================== > --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c > +++ renesas/arch/arm/mach-shmobile/board-koelsch.c > @@ -70,11 +87,29 @@ static const struct gpio_keys_platform_d > .nbuttons = ARRAY_SIZE(gpio_buttons), > }; > > +static const struct pinctrl_map koelsch_pinctrl_map[] = { > + /* Ether */ > + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", > + "eth_link", "eth"), > + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", > + "eth_mdio", "eth"), > + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", > + "eth_rmii", "eth"), > +}; Once again we seem to be using r8a7790 when the actual SoC is r8a7791. If you're going to go down the route of using the SoC name then please use the correct one at least. Cheers, / magnus
Hello. On 10/31/2013 11:06 AM, Magnus Damm wrote: >> Register Ether platform device and pin data on the Koelsch board. >> Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > Thanks for your patch. Just like on the Lager board we also need to > tie in the PHY IRQ pin. Not only on the Lager board, BOCK-W also I think. > Polling is not good enough when the hardware > can do better. Can you please share your PHY IRQ plans with us? It's on my agenda now -- will start after I do away with the CAN driver and the current Koelsch Ether support. > Best, > / magnus WBR, Sergei
Hello. On 10/31/2013 11:10 AM, Magnus Damm wrote: >> Register Ether platform device and pin data on the Koelsch board. >> Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> --- >> arch/arm/mach-shmobile/board-koelsch.c | 61 ++++++++++++++++++++++++++++++++- >> 1 file changed, 60 insertions(+), 1 deletion(-) >> >> Index: renesas/arch/arm/mach-shmobile/board-koelsch.c >> =================================================================== >> --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c >> +++ renesas/arch/arm/mach-shmobile/board-koelsch.c >> @@ -70,11 +87,29 @@ static const struct gpio_keys_platform_d >> .nbuttons = ARRAY_SIZE(gpio_buttons), >> }; >> >> +static const struct pinctrl_map koelsch_pinctrl_map[] = { >> + /* Ether */ >> + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", >> + "eth_link", "eth"), >> + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", >> + "eth_mdio", "eth"), >> + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", >> + "eth_rmii", "eth"), >> +}; > Once again we seem to be using r8a7790 when the actual SoC is r8a7791. Note that the last time it was in the DT context which is not quite the same as platfrom device context. > If you're going to go down the route of using the SoC name then please I don't have much choice here. Though we could have used used R-Car gen1/gen2 terminology (at least in the hindsight)... > use the correct one at least. It wasn't me who used "r8a7790-ether" name in the first place, it was Simon. And as these 2 SoCs are indistinguishable at least from the 'sh_eth' driver's point of view, I wouldn't want to introduce another platform device ID, especially as it hasn't been done for R8A777[89] SoCs. What I can offer is renaming "r8a7790-ether" to "r8a779x-ether" if you really want. > Cheers, > / magnus WBR, Sergei
Hello. On 10/31/2013 08:02 AM, Simon Horman wrote: >> Register Ether platform device and pin data on the Koelsch board. >> Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > Thanks, I have been able to use this to successfully boot a Koelsch board > using nfsroot. However, I believe there is a minor build problem which I > have commented on below. Yes, there is. Unfortunately, I haven't tested the build with koelsch_defconfig... >> --- >> arch/arm/mach-shmobile/board-koelsch.c | 61 ++++++++++++++++++++++++++++++++- >> 1 file changed, 60 insertions(+), 1 deletion(-) >> Index: renesas/arch/arm/mach-shmobile/board-koelsch.c >> =================================================================== >> --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c >> +++ renesas/arch/arm/mach-shmobile/board-koelsch.c [...] >> @@ -83,6 +118,30 @@ static void __init koelsch_add_standard_ >> sizeof(koelsch_keys_pdata)); >> } >> >> +/* >> + * Ether LEDs on the Koelsch board are named LINK and ACTIVE which corresponds >> + * to non-default 01 setting of the Micrel KSZ8041 PHY control register 1 bits >> + * 14-15. We have to set them back to 01 from the default 00 value each time >> + * the PHY is reset. It's also important because the PHY's LED0 signal is >> + * connected to SoC's ETH_LINK signal and in the PHY's default mode it will >> + * bounce on and off after each packet, which we apparently want to avoid. >> + */ >> +static int koelsch_ksz8041_fixup(struct phy_device *phydev) >> +{ >> + u16 phyctrl1 = phy_read(phydev, 0x1e); >> + >> + phyctrl1 &= ~0xc000; >> + phyctrl1 |= 0x4000; >> + return phy_write(phydev, 0x1e, phyctrl1); >> +} >> + >> +static void __init koelsch_init(void) >> +{ >> + koelsch_add_standard_devices(); >> + >> + phy_register_fixup_for_id("r8a7790-ether-ff:01", koelsch_ksz8041_fixup); > It seems to me that this code requires CONFIG_PHYLIB in order to compile. Not really -- only in order to link. > So I propose the following enhancement to this patch which replaces > the line immediately above. > if (IS_ENABLED(CONFIG_PHYLIB)) > phy_register_fixup_for_id("r8a7790-ether-ff:01", > koelsch_ksz8041_fixup); Strictly speaking, this is not enough. I'm getting: arch/arm/mach-shmobile/built-in.o: In function `koelsch_ksz8041_fixup': platsmp-apmu.c:(.text+0xb8): undefined reference to `mdiobus_read' platsmp-apmu.c:(.text+0xd0): undefined reference to `mdiobus_write' arch/arm/mach-shmobile/built-in.o: In function `koelsch_init': platsmp-apmu.c:(.init.text+0xa90): undefined reference to `phy_register_fixup_for_id' So I guess the fixup function should be enclosed into #ifdef CONFIG_PHYLIB too. However, in practice this change seems enough (gcc probably drops unused static functions?)... > I believe a similar enhancement is also needed for board-lager.c > which I just realised does not compile if CONFIG_PHYLIB is not set. I believe you meant to say it doesn't link too. WBR, Sergei
Hello. On 10/31/2013 10:03 PM, I wrote: >>> Register Ether platform device and pin data on the Koelsch board. >>> Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> Thanks for your patch. Just like on the Lager board we also need to >> tie in the PHY IRQ pin. > Not only on the Lager board, BOCK-W also I think. >> Polling is not good enough when the hardware >> can do better. Can you please share your PHY IRQ plans with us? > It's on my agenda now -- will start after I do away with the CAN driver > and the current Koelsch Ether support. I have now implemented PHY IRQ support for BOCK-W, Lager, and Koelsch boards (had to extend the Micrel PHY driver to recognize undocumented KSZ8041RNLI PHY ID for that) but I must say that the PHY interrupt doesn't get generated on either of the board and I don't know why yet (I can't rule out the INTC driver issue); that's bad news. Good news is that even this doesn't prevent phylib from working properly as it still polls the PHY once a second. >> Best, >> / magnus WBR, Sergei
On 12/09/2013 10:57 PM, Sergei Shtylyov wrote: >>>> Register Ether platform device and pin data on the Koelsch board. >>>> Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>> Thanks for your patch. Just like on the Lager board we also need to >>> tie in the PHY IRQ pin. >> Not only on the Lager board, BOCK-W also I think. >>> Polling is not good enough when the hardware >>> can do better. Can you please share your PHY IRQ plans with us? >> It's on my agenda now -- will start after I do away with the CAN driver >> and the current Koelsch Ether support. > I have now implemented PHY IRQ support for BOCK-W, Lager, and Koelsch > boards (had to extend the Micrel PHY driver to recognize undocumented > KSZ8041RNLI PHY ID for that) but I must say that the PHY interrupt doesn't get > generated on either of the board and I don't know why yet (I can't rule out > the INTC driver issue); that's bad news. Good news is that even this doesn't > prevent phylib from working properly as it still polls the PHY once a second. Forgot to note that using PHY IRQ on Lager/Koelsch seems quite pointless to me as we're using ETH_LINK signal on these boards and Ether core generates interrupts on the change of this signal using which the driver actually controls packet Rx/Tx; Micrel PHY driver also only enables interrupts on link up/down -- so it seems only useful on BOCK-W where we ignore ETH_LINK and so the driver controls packet Rx/Tx from the phylib's adjust_link() callback. >>> Best, >>> / magnus WBR, Sergei
Hello. On 12/09/2013 10:57 PM, Sergei Shtylyov wrote: >>>> Register Ether platform device and pin data on the Koelsch board. >>>> Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>> Thanks for your patch. Just like on the Lager board we also need to >>> tie in the PHY IRQ pin. >> Not only on the Lager board, BOCK-W also I think. >>> Polling is not good enough when the hardware >>> can do better. Can you please share your PHY IRQ plans with us? >> It's on my agenda now -- will start after I do away with the CAN driver >> and the current Koelsch Ether support. > I have now implemented PHY IRQ support for BOCK-W, Lager, and Koelsch > boards (had to extend the Micrel PHY driver to recognize undocumented > KSZ8041RNLI PHY ID for that) but I must say that the PHY interrupt doesn't get > generated on either of the board and I don't know why yet (I can't rule out > the INTC driver issue); that's bad news. Good news is that even this doesn't > prevent phylib from working properly as it still polls the PHY once a second. Oh yes, it does: phylib fails to recognize link down/up after you disconnect/reconnect the cable since it relies solely on IRQ for the link changes if it's specified. Now for the really good news: I was able to get the PHY IRQs on Koelsch board when I set the trigger mode to falling-edge; that seemed strange as the PHY interrupts are level-sensitive, according to the documentation. And only after we commented out the wild PHY reset done by the 'sh_eth' driver on each open, I was able to get the IRQs in the low-level trigger mode. So, I'm expecting to be able to finally resolve this issue on top of the net-next tree which now contains 2 patchsets that should enable me to finally remove that wild PHY reset for good: http://marc.info/?l=linux-netdev&m=138636384109141 http://marc.info/?l=linux-netdev&m=138692649110301 >>> Best, >>> / magnus WBR, Sergei
Index: renesas/arch/arm/mach-shmobile/board-koelsch.c =================================================================== --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c +++ renesas/arch/arm/mach-shmobile/board-koelsch.c @@ -24,14 +24,31 @@ #include <linux/input.h> #include <linux/kernel.h> #include <linux/leds.h> +#include <linux/phy.h> +#include <linux/pinctrl/machine.h> #include <linux/platform_data/gpio-rcar.h> #include <linux/platform_device.h> +#include <linux/sh_eth.h> #include <mach/common.h> +#include <mach/irqs.h> #include <mach/r8a7791.h> #include <mach/rcar-gen2.h> #include <asm/mach-types.h> #include <asm/mach/arch.h> +/* Ether */ +static const struct sh_eth_plat_data ether_pdata __initconst = { + .phy = 0x1, + .edmac_endian = EDMAC_LITTLE_ENDIAN, + .phy_interface = PHY_INTERFACE_MODE_RMII, + .ether_link_active_low = 1, +}; + +static const struct resource ether_resources[] __initconst = { + DEFINE_RES_MEM(0xee700000, 0x400), + DEFINE_RES_IRQ(gic_spi(162)), +}; + /* LEDS */ static struct gpio_led koelsch_leds[] = { { @@ -70,11 +87,29 @@ static const struct gpio_keys_platform_d .nbuttons = ARRAY_SIZE(gpio_buttons), }; +static const struct pinctrl_map koelsch_pinctrl_map[] = { + /* Ether */ + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", + "eth_link", "eth"), + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", + "eth_mdio", "eth"), + PIN_MAP_MUX_GROUP_DEFAULT("r8a7790-ether", "pfc-r8a7791", + "eth_rmii", "eth"), +}; + static void __init koelsch_add_standard_devices(void) { r8a7791_clock_init(); + + pinctrl_register_mappings(koelsch_pinctrl_map, + ARRAY_SIZE(koelsch_pinctrl_map)); r8a7791_pinmux_init(); + r8a7791_add_standard_devices(); + platform_device_register_resndata(&platform_bus, "r8a7790-ether", -1, + ether_resources, + ARRAY_SIZE(ether_resources), + ðer_pdata, sizeof(ether_pdata)); platform_device_register_data(&platform_bus, "leds-gpio", -1, &koelsch_leds_pdata, sizeof(koelsch_leds_pdata)); @@ -83,6 +118,30 @@ static void __init koelsch_add_standard_ sizeof(koelsch_keys_pdata)); } +/* + * Ether LEDs on the Koelsch board are named LINK and ACTIVE which corresponds + * to non-default 01 setting of the Micrel KSZ8041 PHY control register 1 bits + * 14-15. We have to set them back to 01 from the default 00 value each time + * the PHY is reset. It's also important because the PHY's LED0 signal is + * connected to SoC's ETH_LINK signal and in the PHY's default mode it will + * bounce on and off after each packet, which we apparently want to avoid. + */ +static int koelsch_ksz8041_fixup(struct phy_device *phydev) +{ + u16 phyctrl1 = phy_read(phydev, 0x1e); + + phyctrl1 &= ~0xc000; + phyctrl1 |= 0x4000; + return phy_write(phydev, 0x1e, phyctrl1); +} + +static void __init koelsch_init(void) +{ + koelsch_add_standard_devices(); + + phy_register_fixup_for_id("r8a7790-ether-ff:01", koelsch_ksz8041_fixup); +} + static const char * const koelsch_boards_compat_dt[] __initconst = { "renesas,koelsch", NULL, @@ -91,7 +150,7 @@ static const char * const koelsch_boards DT_MACHINE_START(KOELSCH_DT, "koelsch") .smp = smp_ops(r8a7791_smp_ops), .init_early = r8a7791_init_early, - .init_machine = koelsch_add_standard_devices, + .init_machine = koelsch_init, .init_time = rcar_gen2_timer_init, .dt_compat = koelsch_boards_compat_dt, MACHINE_END
Register Ether platform device and pin data on the Koelsch board. Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board. Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> --- arch/arm/mach-shmobile/board-koelsch.c | 61 ++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)