Hi Florian > > diff --git a/drivers/net/phy/microchip_t1.c > > b/drivers/net/phy/microchip_t1.c new file mode 100644 index > > 0000000..1f6f299 > > --- /dev/null > > +++ b/drivers/net/phy/microchip_t1.c > > @@ -0,0 +1,88 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > This is not the standard comment for a .c file, it should be: // (C++ style) >
Thanks, will update it. > > +/* > > + * Copyright (C) 2018 Microchip Technology > > + * > > + * 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. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > > That needs to go away now that you used SPDX > Ok fine will remove it. > > + */ > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/mii.h> > > +#include <linux/phy.h> > > + > > +/* Interrupt Source Register */ > > +#define LAN87XX_INTERRUPT_SOURCE (0x18) > > + > > +/* Interrupt Mask Register */ > > +#define LAN87XX_INTERRUPT_MASK (0x19) > > +#define LAN87XX_MASK_LINK_UP (0x0004) > > +#define LAN87XX_MASK_LINK_DOWN (0x0002) > > + > > +#define DRIVER_AUTHOR "Nisar Sayed <nisar.sa...@microchip.com>" > > +#define DRIVER_DESC "Microchip LAN87XX T1 PHY driver" > > + > > +static int lan87xx_phy_config_intr(struct phy_device *phydev) { > > + int rc, val = 0; > > + > > + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { > > + /* unmask all source and clear them before enable */ > > + rc = phy_write(phydev, LAN87XX_INTERRUPT_MASK, > 0x7FFF); > > + rc = phy_read(phydev, LAN87XX_INTERRUPT_SOURCE); > > + val = (LAN87XX_MASK_LINK_UP | > LAN87XX_MASK_LINK_DOWN); > > The parenthesis are not necessary here. Thanks, will remove as suggested. > > > + } > > + > > + rc = phy_write(phydev, LAN87XX_INTERRUPT_MASK, val); > > + > > + return rc < 0 ? rc : 0; > > +} > > + > > +static int lan87xx_phy_ack_interrupt(struct phy_device *phydev) { > > + int rc = phy_read(phydev, LAN87XX_INTERRUPT_SOURCE); > > + > > + return rc < 0 ? rc : 0; > > +} > > + > > +static struct phy_driver microchip_t1_phy_driver[] = { > > + { > > + .phy_id = 0x0007c150, > > + .phy_id_mask = 0xfffffff0, > > + .name = "Microchip LAN87xx", > > Would you want to name this "Microchip LAN87xx T1"? Thanks, yes it's good to mention, will update and send v2 - Nisar
