On Thu, 6 Jun 2019 at 04:50, Samuel Mendoza-Jonas <s...@mendozajonas.com> wrote:
>
> This introduces support for the NC-SI protocol, modelled as a phy driver
> for other ethernet drivers to consume.
>
> NC-SI (Network Controller Sideband Interface) is a protocol to manage a
> sideband connection to a proper network interface, for example a BMC
> (Baseboard Management Controller) sharing the NIC of the host system.
> Probing and configuration occurs by communicating with the "remote" NIC
> via NC-SI control frames (Ethernet header 0x88f8).
>
> This implementation is roughly based on the upstream Linux
> implementation[0], with a reduced feature set and an emphasis on getting
> a link up as fast as possible rather than probing the full possible
> topology of the bus.
> The current phy model relies on the network being "up", sending NC-SI
> command frames via net_send_packet() and receiving them from the
> net_loop() loop (added in a following patch).
>
> The ncsi-pkt.h header[1] is copied from the Linux kernel for consistent
> field definitions.
>
> [0]: https://github.com/torvalds/linux/tree/master/net/ncsi
> [1]: https://github.com/torvalds/linux/blob/master/net/ncsi/ncsi-pkt.h
>
> Signed-off-by: Samuel Mendoza-Jonas <s...@mendozajonas.com>
Looks good. Some comments below.
> +static int ncsi_validate_rsp(struct ncsi_rsp_pkt *pkt, int payload)
> +{
> + struct ncsi_rsp_pkt_hdr *hdr = &pkt->rsp;
> + __be32 pchecksum;
> + u32 checksum;
> + if (ntohs(hdr->common.length) != payload) {
> + printf("NCSI: 0x%02x response has incorrect length %d\n",
> + hdr->common.type, hdr->common.length);
> + return -1;
> + }
> +
> + pchecksum = get_unaligned_be32((void *)(hdr + 1) + payload - 4);
Wheee. So the checksum is the last 4-bytes of the payload. I assume
it's always longer than 4?
A clarifying comment might help, or try to write it in a different way:
endp = (void *)hdr + sizeof(hdr) + payload;
pchecksum = get_unaligned_be32(endp - sizeof(checksum));
or
checksum_offset = sizeof(hdr) + payload - sizeof(checksum);
pchecksum = get_unaligned_be32(payload + checksum_offset);
> + if (pchecksum != 0) {
> + checksum = ncsi_calculate_checksum((unsigned char *)hdr,
> + sizeof(*hdr) + payload -
> 4);
And then this can be:
checksum = ((unsigned char *)hdr, checksum_offset);
> + if (pchecksum != checksum) {
> + printf("NCSI: 0x%02x response has invalid checksum\n",
> + hdr->common.type);
> + return -1;
> + }
> + }
> +static void ncsi_send_sma(unsigned int np, unsigned int nc)
> +{
> + struct ncsi_cmd_sma_pkt cmd;
> + unsigned char *addr;
> +
> + addr = eth_get_ethaddr();
> + if (!addr) {
> + printf("NCSI: no MAC address configured\n");
> + return;
> + }
> +
> + memset(&cmd, 0, sizeof(cmd));
> + memcpy(cmd.mac, addr, 6);
Are there endianness issues with addr here?
> + cmd.index = 1;
> + cmd.at_e = 1;
> +
> + ncsi_send_command(np, nc, NCSI_PKT_CMD_SMA,
> + ((unsigned char *)&cmd)
> + + sizeof(struct ncsi_cmd_pkt_hdr),
> + cmd_payload(NCSI_PKT_CMD_SMA), true);
> +}
> +
> +int ncsi_probe(struct phy_device *phydev)
> +{
> + // TODO Associate per device
Is this required before we can support multiple NICs?
> + if (!ncsi_priv) {
> + ncsi_priv = malloc(sizeof(struct ncsi));
> + if (!ncsi_priv)
> + return -ENOMEM;
> + memset(ncsi_priv, 0, sizeof(struct ncsi));
> + }
> +
> + phydev->priv = ncsi_priv;
> +
> + return 0;
> +}
> +
> +int ncsi_startup(struct phy_device *phydev)
> +{
> + /* Set phydev parameters */
> + phydev->speed = SPEED_100;
> + phydev->duplex = DUPLEX_FULL;
> + /* Normal phy reset is N/A */
> + phydev->flags |= PHY_FLAG_BROKEN_RESET;
> +
> + /* Set initial probe state */
> + ncsi_priv->state = NCSI_PROBE_PACKAGE_SP;
> +
> + /* No active package/channel yet */
> + ncsi_priv->current_package = NCSI_PACKAGE_MAX;
> + ncsi_priv->current_channel = NCSI_CHANNEL_MAX;
> +
> + /* Pretend link works so ftgmac100 sets final bits up */
s/ftgmac100/mac driver/ ?
> + phydev->link = true;
> +
> + return 0;
> +}
> +
> +int ncsi_shutdown(struct phy_device *phydev)
> +{
> + printf("NCSI: Disabling package %d\n", ncsi_priv->current_package);
> + ncsi_send_dp(ncsi_priv->current_package);
> + return 0;
> +}
> +
> +static struct phy_driver ncsi_driver = {
> + .uid = PHY_NCSI_ID,
> + .mask = 0xffffffff,
> + .name = "NC-SI",
> + .features = PHY_100BT_FEATURES | PHY_DEFAULT_FEATURES |
> SUPPORTED_100baseT_Full | SUPPORTED_MII,
> + .probe = ncsi_probe,
> + .startup = ncsi_startup,
> + .shutdown = ncsi_shutdown,
> +};
> +
> +int phy_ncsi_init(void)
> +{
> + phy_register(&ncsi_driver);
> + return 0;
> +}
> --- /dev/null
> +++ b/include/net/ncsi-pkt.h
> @@ -0,0 +1,415 @@
> +/*
> + * Copyright Gavin Shan, IBM Corporation 2016.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
If you grab the version from 5.2-rc3 it has been SPDXified.
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -17,6 +17,7 @@
> #include <phy_interface.h>
>
> #define PHY_FIXED_ID 0xa5a55a5a
> +#define PHY_NCSI_ID 0xbeefcafe
hmmm...
>
> #define PHY_MAX_ADDR 32
>
> --
> 2.21.0
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
