+ Roger, Grygorii On 06/04/22 19:43, Andrew Lunn wrote: >> +static int emac_set_link_ksettings(struct net_device *ndev, >> + const struct ethtool_link_ksettings *ecmd) >> +{ >> + struct prueth_emac *emac = netdev_priv(ndev); >> + >> + if (!emac->phydev || phy_is_pseudo_fixed_link(emac->phydev)) >> + return -EOPNOTSUPP; >> + >> + return phy_ethtool_ksettings_set(emac->phydev, ecmd); >> +} >> + >> +static int emac_get_eee(struct net_device *ndev, struct ethtool_eee *edata) >> +{ >> + struct prueth_emac *emac = netdev_priv(ndev); >> + >> + if (!emac->phydev || phy_is_pseudo_fixed_link(emac->phydev)) >> + return -EOPNOTSUPP; >> + >> + return phy_ethtool_get_eee(emac->phydev, edata); >> +} > > Why do you need the phy_is_pseudo_fixed_link() calls here? > >> +/* called back by PHY layer if there is change in link state of hw port*/ >> +static void emac_adjust_link(struct net_device *ndev) >> +{ > > ... > >> + if (emac->link) { >> + /* link ON */ >> + netif_carrier_on(ndev); >> + /* reactivate the transmit queue */ >> + netif_tx_wake_all_queues(ndev); >> + } else { >> + /* link OFF */ >> + netif_carrier_off(ndev); >> + netif_tx_stop_all_queues(ndev); >> + } > > phylib should of set the carrier for you. > >> + * emac_ndo_open - EMAC device open >> + * @ndev: network adapter device >> + * >> + * Called when system wants to start the interface. >> + * >> + * Returns 0 for a successful open, or appropriate error code >> + */ >> +static int emac_ndo_open(struct net_device *ndev) >> +{ >> + struct prueth_emac *emac = netdev_priv(ndev); >> + int ret, i, num_data_chn = emac->tx_ch_num; >> + struct prueth *prueth = emac->prueth; >> + int slice = prueth_emac_slice(emac); >> + struct device *dev = prueth->dev; >> + int max_rx_flows; >> + int rx_flow; >> + >> + /* clear SMEM and MSMC settings for all slices */ >> + if (!prueth->emacs_initialized) { >> + memset_io(prueth->msmcram.va, 0, prueth->msmcram.size); >> + memset_io(prueth->shram.va, 0, ICSSG_CONFIG_OFFSET_SLICE1 * PRUETH_NUM_MACS); >> + } >> + >> + /* set h/w MAC as user might have re-configured */ >> + ether_addr_copy(emac->mac_addr, ndev->dev_addr); >> + >> + icssg_class_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr); >> + icssg_ft1_set_mac_addr(prueth->miig_rt, slice, emac->mac_addr); >> + >> + icssg_class_default(prueth->miig_rt, slice, 0); >> + >> + netif_carrier_off(ndev); > > phylib should take care of this. > >> + >> + /* Notify the stack of the actual queue counts. */ >> + ret = netif_set_real_num_tx_queues(ndev, num_data_chn); >> + if (ret) { >> + dev_err(dev, "cannot set real number of tx queues\n"); >> + return ret; >> + } >> + >> + init_completion(&emac->cmd_complete); >> + ret = prueth_init_tx_chns(emac); >> + if (ret) { >> + dev_err(dev, "failed to init tx channel: %d\n", ret); >> + return ret; >> + } >> + >> + max_rx_flows = PRUETH_MAX_RX_FLOWS; >> + ret = prueth_init_rx_chns(emac, &emac->rx_chns, "rx", >> + max_rx_flows, PRUETH_MAX_RX_DESC); >> + if (ret) { >> + dev_err(dev, "failed to init rx channel: %d\n", ret); >> + goto cleanup_tx; >> + } >> + >> + ret = prueth_ndev_add_tx_napi(emac); >> + if (ret) >> + goto cleanup_rx; >> + >> + /* we use only the highest priority flow for now i.e. @irq[3] */ >> + rx_flow = PRUETH_RX_FLOW_DATA; >> + ret = request_irq(emac->rx_chns.irq[rx_flow], prueth_rx_irq, >> + IRQF_TRIGGER_HIGH, dev_name(dev), emac); >> + if (ret) { >> + dev_err(dev, "unable to request RX IRQ\n"); >> + goto cleanup_napi; >> + } >> + >> + /* reset and start PRU firmware */ >> + ret = prueth_emac_start(prueth, emac); >> + if (ret) >> + goto free_rx_irq; >> + >> + /* Prepare RX */ >> + ret = prueth_prepare_rx_chan(emac, &emac->rx_chns, PRUETH_MAX_PKT_SIZE); >> + if (ret) >> + goto stop; >> + >> + ret = k3_udma_glue_enable_rx_chn(emac->rx_chns.rx_chn); >> + if (ret) >> + goto reset_rx_chn; >> + >> + for (i = 0; i < emac->tx_ch_num; i++) { >> + ret = k3_udma_glue_enable_tx_chn(emac->tx_chns[i].tx_chn); >> + if (ret) >> + goto reset_tx_chan; >> + } >> + >> + /* Enable NAPI in Tx and Rx direction */ >> + for (i = 0; i < emac->tx_ch_num; i++) >> + napi_enable(&emac->tx_chns[i].napi_tx); >> + napi_enable(&emac->napi_rx); >> + >> + emac_phy_connect(emac); > > Why don't you check the error code? > >> +static int prueth_config_rgmiidelay(struct prueth *prueth, >> + struct device_node *eth_np, >> + phy_interface_t phy_if) >> +{ >> + struct device *dev = prueth->dev; >> + struct regmap *ctrl_mmr; >> + u32 rgmii_tx_id = 0; >> + u32 icssgctrl_reg; >> + >> + if (!phy_interface_mode_is_rgmii(phy_if)) >> + return 0; >> + >> + ctrl_mmr = syscon_regmap_lookup_by_phandle(eth_np, "ti,syscon-rgmii-delay"); >> + if (IS_ERR(ctrl_mmr)) { >> + dev_err(dev, "couldn't get ti,syscon-rgmii-delay\n"); >> + return -ENODEV; >> + } >> + >> + if (of_property_read_u32_index(eth_np, "ti,syscon-rgmii-delay", 1, >> + &icssgctrl_reg)) { >> + dev_err(dev, "couldn't get ti,rgmii-delay reg. offset\n"); >> + return -ENODEV; >> + } >> + >> + if (phy_if == PHY_INTERFACE_MODE_RGMII_ID || >> + phy_if == PHY_INTERFACE_MODE_RGMII_TXID) >> + rgmii_tx_id |= ICSSG_CTRL_RGMII_ID_MODE; >> + >> + regmap_update_bits(ctrl_mmr, icssgctrl_reg, ICSSG_CTRL_RGMII_ID_MODE, rgmii_tx_id); > > Do you need to do a units conversion here, or does the register > already take pico seconds? > > Andrew