On 04/27/2018 08:10 AM, Nisar Sayed wrote: > Add driver for Microchip LAN87XX T1 PHYs > > This patch support driver for Microchp T1 PHYs. > There will be followup patches to this driver to support T1 PHY > features such as cable diagnostics, signal quality indicator(SQI), > sleep and wakeup (TC10) support. > > Signed-off-by: Nisar Sayed <nisar.sa...@microchip.com> > --- > v0 - v1: > * Rename microchipT1phy.c file to microchip_t1.c > * Remove microchipT1phy.h include file > * Add SPDX license identifier > * Remove remove probe and remove functions > * Update LAN87XX_INTERRUPT_MASK write as suggested
This does look better, small comments below. > --- > drivers/net/phy/Kconfig | 5 +++ > drivers/net/phy/Makefile | 1 + > drivers/net/phy/microchip_t1.c | 88 > ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 94 insertions(+) > create mode 100644 drivers/net/phy/microchip_t1.c > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index bdfbabb..7b0b351 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -354,6 +354,11 @@ config MICROCHIP_PHY > help > Supports the LAN88XX PHYs. > > +config MICROCHIP_T1_PHY > + tristate "Microchip T1 PHYs" > + ---help--- > + Supports the LAN87XX PHYs. > + > config MICROSEMI_PHY > tristate "Microsemi PHYs" > ---help--- > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index 01acbcb..3d0550b 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -70,6 +70,7 @@ obj-$(CONFIG_MESON_GXL_PHY) += meson-gxl.o > obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o > obj-$(CONFIG_MICREL_PHY) += micrel.o > obj-$(CONFIG_MICROCHIP_PHY) += microchip.o > +obj-$(CONFIG_MICROCHIP_T1_PHY) += microchip_t1.o > obj-$(CONFIG_MICROSEMI_PHY) += mscc.o > obj-$(CONFIG_NATIONAL_PHY) += national.o > obj-$(CONFIG_QSEMI_PHY) += qsemi.o > 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) > +/* > + * 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 > + */ > +#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. > + } > + > + 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"? -- Florian
