> > +static int lan743x_phy_reset(struct lan743x_adapter *adapter) {
> > + u32 data;
> > +
> > + data = lan743x_csr_read(adapter, PMT_CTL);
> > + data |= PMT_CTL_ETH_PHY_RST_;
> > + lan743x_csr_write(adapter, PMT_CTL, data);
> > +
> > + return readx_poll_timeout(LAN743X_CSR_READ_OP, PMT_CTL,
> data,
> > + (!(data & PMT_CTL_ETH_PHY_RST_) &&
> > + (data & PMT_CTL_READY_)),
> > + 50000, 1000000);
> > +}
>
> Hi Bryan
>
> Could you explain this a bit more. What exactly is it resetting? Do we need to
> tell the phylib that the PHY has been reset and that it needs to re-program
> it?
> Or by phy do you mean a SERDES interface?
Hi Andrew,
This function resets the Ethernet phy. But it is called only in probe and
before mdiobus_register. So I don't believe it is necessary to tell phylib.
[snip]
> > +
> > + /* PHY interrupt enabled here */
> > + phy_start(phydev);
> > + phy_start_aneg(phydev);
> > + return 0;
>
> Are phy interrupts really enabled here? I could of missed it, but i don't see
> anywhere PHY interrupts are configured. This is either done via device tree,
> you set phydev->irq, or mdiobus->irq[X].
Sorry that is an obsolete comment, I will remove it. It is not using phy
interrupts. It's using polling.
>
> > +static int lan743x_tx_open(struct lan743x_tx *tx) {
> > + struct lan743x_adapter *adapter = NULL;
> > + u32 data = 0;
> > + int ret;
> > +
> > + adapter = tx->adapter;
> > + ret = lan743x_tx_ring_init(tx);
> > + if (ret)
> > + goto return_error;
>
> You could just return here. You don't do anything useful at
> return_error:
Yes, I'll fix it.
[snip]
> This is much nicer without all the flags. Thanks.
No problem, thanks for your patients with me.
>
> > +static struct pci_driver lan743x_pcidev_driver = {
> > + .name = DRIVER_NAME,
> > + .id_table = lan743x_pcidev_tbl,
> > + .probe = lan743x_pcidev_probe,
> > + .remove = lan743x_pcidev_remove,
> > + .shutdown = lan743x_pcidev_shutdown, };
> > +
> > +static int __init lan743x_module_init(void) {
> > + int result = -EINVAL;
> > +
> > + pr_info(DRIVER_DESC "\n");
> > + result = pci_register_driver(&lan743x_pcidev_driver);
> > + if (result)
> > + pr_warn("pci_register_driver returned error code, %d\n",
> > + result);
> > + return result;
> > +}
> > +
> > +module_init(lan743x_module_init);
> > +
> > +static void __exit lan743x_module_exit(void) {
> > + pci_unregister_driver(&lan743x_pcidev_driver);
> > +}
> >
> > +module_exit(lan743x_module_exit);
>
> You can replace this boilerplate code with module_pci_driver().
> You don't do anything special here.
OK
Thanks,
Bryan
