[U-Boot] [PATCH 07/10] mvebu: a37xx: Add init for ESPRESSBin Topaz switch

[Joe Hershberger]

On Wed, Feb 15, 2017 at 9:32 AM, Konstantin Porotchkin
<kostap at marvell.com> wrote:
>
>
> On 02/15/2017 11:07 AM, Konstantin Porotchkin wrote:
>>
>> Hi, Joe,
>>
>> On 02/14/2017 07:17 PM, Joe Hershberger wrote:
>>>
>>> On Tue, Feb 14, 2017 at 6:32 AM, Stefan Roese <sr at denx.de> wrote:
>>> > (added Joe to Cc as network custodian)
>>> >
>>> >
>>> > On 14.02.2017 13:13, Konstantin Porotchkin wrote:
>>> >>
>>> >> Hi, Stefan,
>>> >>
>>> >> On 2/14/2017 13:49, Stefan Roese wrote:
>>> >>>
>>> >>> Hi Kosta,
>>> >>>
>>> >>> On 13.02.2017 14:38, kostap at marvell.com wrote:
>>> >>>>
>>> >>>> From: Konstantin Porotchkin <kostap at marvell.com>
>>> >>>>
>>> >>>> Implement the board-specific network init function for
>>> >>>> ESPRESSOBin community board, setting the on-board Topaz
>>> >>>> switch port to forward mode and allow network connection
>>> >>>> through any of the available Etherenet ports.
>>> >>>>
>>> >>>> Signed-off-by: Konstantin Porotchkin <kostap at marvell.com>
>>> >>>> Cc: Stefan Roese <sr at denx.de>
>>> >>>> Cc: Igal Liberman <igall at marvell.com>
>>> >>>> ---
>>> >>>>  board/Marvell/mvebu_db-88f3720/board.c | 49
>>> >>>> ++++++++++++++++++++++++++++++++++
>>> >>>>  1 file changed, 49 insertions(+)
>>> >>>>
>>> >>>> diff --git a/board/Marvell/mvebu_db-88f3720/board.c
>>> >>>> b/board/Marvell/mvebu_db-88f3720/board.c
>>> >>>> index 3337f3f..45098ce 100644
>>> >>>> --- a/board/Marvell/mvebu_db-88f3720/board.c
>>> >>>> +++ b/board/Marvell/mvebu_db-88f3720/board.c
>>> >>>> @@ -6,6 +6,7 @@
>>> >>>>
>>> >>>>  #include <common.h>
>>> >>>>  #include <i2c.h>
>>> >>>> +#include <phy.h>
>>> >>>>  #include <asm/io.h>
>>> >>>>  #include <asm/arch/cpu.h>
>>> >>>>  #include <asm/arch/soc.h>
>>> >>>> @@ -156,3 +157,51 @@ int board_xhci_enable(void)
>>> >>>>
>>> >>>>      return 0;
>>> >>>>  }
>>> >>>> +
>>> >>>> +static int mii_multi_chip_mode_write(struct mii_dev *bus, int
>>> >>>> dev_smi_addr,
>>> >>>> +                     int smi_addr, int reg, u16 value)
>>> >>>> +{
>>> >>>> +    u16 data = 0;
>>> >>>> +
>>> >>>> +    if (bus->write(bus, dev_smi_addr, 0, 1, value) != 0) {
>>> >>>> +        printf("Error writing to the PHY addr=%02x reg=%02x\n",
>>> >>>> +               smi_addr, reg);
>>> >>>> +        return -EFAULT;
>>> >>>> +    }
>>> >>>> +
>>> >>>> +    data = (1 << 15) | (1 << 12) | (1 << 10) | (smi_addr << 5) |
>>> reg;
>>> >>>> +    if (bus->write(bus, dev_smi_addr, 0, 0, data) != 0) {
>>> >>>> +        printf("Error writing to the PHY addr=%02x reg=%02x\n",
>>> >>>> +               smi_addr, reg);
>>> >>>> +        return -EFAULT;
>>> >>>> +    }
>>> >>>> +
>>> >>>> +    return 0;
>>> >>>> +}
>>> >>>> +
>>> >>>> +
>>> >>>> +int board_network_enable(struct mii_dev *bus)
>>> >>>> +{
>>> >>>> +    if
>>> (!of_machine_is_compatible("marvell,armada-3720-espressobin"))
>>> >>>> +        return 0;
>>> >>>> +
>>> >>>> +    /*
>>> >>>> +     * FIXME: remove this code once Topaz driver gets available
>>> >>>> +     * A3720 Community Board Only
>>> >>>> +     * Configure Topaz switch (88E6341)
>>> >>>> +     * Set port 0,1,2,3 to forwarding Mode
>>> >>>> +     */
>>> >>>
>>> >>>
>>> >>> Just checking: Is this "Topaz switch driver" something thats being
>>> >>> worked on or in the queue to do?
>>> >>
>>> >>
>>> >> I currently do not have it in my queue.
>>> >> I think the driver exists in the kernel (or will exist in 4.10/4.11
>>> >> release), so we may want to port it if required.
>>> >> Which switch operations are needed at u-bot stage?
>>> >
>>> >
>>> > I'm not 100% sure if there is anything really "needed" other than
>>> > to get some ports into operation for the ethernet driver connected
>>> > to this switch. So it might be that such a few register writes
>>> > are acceptable - I'm pretty sure other boards do it this way as
>>> > well.
>>> >
>>> > On the other hand you could take a look at the
>>> > "drivers/net/phy/mv88e61xx.c" switch driver. Might be that this is
>>> > something similar to what you want / need.
>>>
>>> I think the switch driver to model after is drivers/net/vsc9953.c -
>>> there is a command: cmd/ethsw.c / include ethsw.h that implements the
>>> framework (doc/README.t1040-l2switch).
>>
>>
>> I will check this code, thank you for the reference!
>>>
>>>
>>> There is also the drivers/net/cpsw.c that just hard-codes the config.
>>> Eth switches have varying levels of support. What level of support are
>>> you able to implement?
>>
>> I am not really sure about level of support required by the u-boot.
>> The Linux driver configures the 3 output ports of this switch as lan0,
>> lan1
>> and wan interfaces, so they are presented to the kernel as separate NICs.
>> So if I set the NFS server on lan0 and the cable connected to lan1, the
>> connection attempt will fail.
>> The u-boot code however just sets the switch ports to follow all the
>> traffic
>> to the CPU. So when I tfttpload image using default neta0 interface, any
>> switch port will work for that.
>> Anyway, I will check what is supported by the reference code you just
>> pointed
>> and check what I can provide.
>> I personally not an expert in this Topaz switch internals and may need
>> to request help
>> from other Marvell teams for doing something smarter than the code already
>> provided in this patch.
>
> I went trough the Vitesse driver and understood that at this moment I cannot
> implement something similar.
> There are two reasons for this:
> 1. I am not an expert in SOHO switches.
> 2. It looks like a long task and it is not included in my current schedule.
> So I will try to make this code more readable by adding some defines that
> replace the numeric values at least for pointing to the register names. My
> current target is to enable early adopters of the Espressobin board to work
> with the mainline u-boot sources.
> Is it a good reason and will this temporary solution be acceptable?

Yes, that's fine. Thanks!
-Joe