| Message ID | 20190805165453.3989-2-alexandru.ardelean@analog.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phy: adin: add support for Analog Devices PHYs | expand |
> +static int adin_config_init(struct phy_device *phydev) > +{ > + int rc; > + > + rc = genphy_config_init(phydev); > + if (rc < 0) > + return rc; > + > + return 0; > +} Why not just return genphy_config_init(phydev); Andrew
> +static struct phy_driver adin_driver[] = { > + { > + .phy_id = PHY_ID_ADIN1200, > + .name = "ADIN1200", > + .phy_id_mask = 0xfffffff0, > + .features = PHY_BASIC_FEATURES, Do you need this? If the device implements the registers correctly, phylib can determine this from the registers. > + .config_init = adin_config_init, > + .config_aneg = genphy_config_aneg, > + .read_status = genphy_read_status, > + }, > + { > + .phy_id = PHY_ID_ADIN1300, > + .name = "ADIN1300", > + .phy_id_mask = 0xfffffff0, > + .features = PHY_GBIT_FEATURES, same here. > + .config_init = adin_config_init, > + .config_aneg = genphy_config_aneg, > + .read_status = genphy_read_status, > + }, > +}; > + > +module_phy_driver(adin_driver); > + > +static struct mdio_device_id __maybe_unused adin_tbl[] = { > + { PHY_ID_ADIN1200, 0xfffffff0 }, > + { PHY_ID_ADIN1300, 0xfffffff0 }, PHY_ID_MATCH_VENDOR(). Andrew
On 05.08.2019 18:54, Alexandru Ardelean wrote: > This change adds support for Analog Devices Industrial Ethernet PHYs. > Particularly the PHYs this driver adds support for: > * ADIN1200 - Robust, Industrial, Low Power 10/100 Ethernet PHY > * ADIN1300 - Robust, Industrial, Low Latency 10/100/1000 Gigabit > Ethernet PHY > > The 2 chips are pin & register compatible with one another. The main > difference being that ADIN1200 doesn't operate in gigabit mode. > > The chips can be operated by the Generic PHY driver as well via the > standard IEEE PHY registers (0x0000 - 0x000F) which are supported by the > kernel as well. This assumes that configuration of the PHY has been done > required. > > Configuration can also be done via registers, which will be implemented by > the driver in the next changes. > > Datasheets: > https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1300.pdf > https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1200.pdf > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > --- > MAINTAINERS | 7 +++++ > drivers/net/phy/Kconfig | 9 ++++++ > drivers/net/phy/Makefile | 1 + > drivers/net/phy/adin.c | 59 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 76 insertions(+) > create mode 100644 drivers/net/phy/adin.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ee663e0e2f2e..faf5723610c8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -938,6 +938,13 @@ S: Supported > F: drivers/mux/adgs1408.c > F: Documentation/devicetree/bindings/mux/adi,adgs1408.txt > > +ANALOG DEVICES INC ADIN DRIVER > +M: Alexandru Ardelean <alexaundru.ardelean@analog.com> > +L: netdev@vger.kernel.org > +W: http://ez.analog.com/community/linux-device-drivers > +S: Supported > +F: drivers/net/phy/adin.c > + > ANALOG DEVICES INC ADIS DRIVER LIBRARY > M: Alexandru Ardelean <alexandru.ardelean@analog.com> > S: Supported > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 206d8650ee7f..5966d3413676 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -257,6 +257,15 @@ config SFP > depends on HWMON || HWMON=n > select MDIO_I2C > > +config ADIN_PHY > + tristate "Analog Devices Industrial Ethernet PHYs" > + help > + Adds support for the Analog Devices Industrial Ethernet PHYs. > + Currently supports the: > + - ADIN1200 - Robust,Industrial, Low Power 10/100 Ethernet PHY > + - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit > + Ethernet PHY > + > config AMD_PHY > tristate "AMD PHYs" > ---help--- > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index ba07c27e4208..a03437e091f3 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -47,6 +47,7 @@ obj-$(CONFIG_SFP) += sfp.o > sfp-obj-$(CONFIG_SFP) += sfp-bus.o > obj-y += $(sfp-obj-y) $(sfp-obj-m) > > +obj-$(CONFIG_ADIN_PHY) += adin.o > obj-$(CONFIG_AMD_PHY) += amd.o > aquantia-objs += aquantia_main.o > ifdef CONFIG_HWMON > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c > new file mode 100644 > index 000000000000..6a610d4563c3 > --- /dev/null > +++ b/drivers/net/phy/adin.c > @@ -0,0 +1,59 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/** > + * Driver for Analog Devices Industrial Ethernet PHYs > + * > + * Copyright 2019 Analog Devices Inc. > + */ > +#include <linux/kernel.h> > +#include <linux/errno.h> > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/mii.h> > +#include <linux/phy.h> > + > +#define PHY_ID_ADIN1200 0x0283bc20 > +#define PHY_ID_ADIN1300 0x0283bc30 > + > +static int adin_config_init(struct phy_device *phydev) > +{ > + int rc; > + > + rc = genphy_config_init(phydev); > + if (rc < 0) > + return rc; > + > + return 0; > +} > + > +static struct phy_driver adin_driver[] = { > + { > + .phy_id = PHY_ID_ADIN1200, You could use PHY_ID_MATCH_MODEL here. > + .name = "ADIN1200", > + .phy_id_mask = 0xfffffff0, > + .features = PHY_BASIC_FEATURES, Setting features is deprecated, instead the get_features callback should be implemented if the default genphy_read_abilities needs to be extended / replaced. You say that the PHY's work with the genphy driver, so I suppose the default feature detection is ok in your case. Then you could simply remove setting "features". > + .config_init = adin_config_init, > + .config_aneg = genphy_config_aneg, > + .read_status = genphy_read_status, > + }, > + { > + .phy_id = PHY_ID_ADIN1300, > + .name = "ADIN1300", > + .phy_id_mask = 0xfffffff0, > + .features = PHY_GBIT_FEATURES, > + .config_init = adin_config_init, > + .config_aneg = genphy_config_aneg, > + .read_status = genphy_read_status, > + }, > +}; > + > +module_phy_driver(adin_driver); > + > +static struct mdio_device_id __maybe_unused adin_tbl[] = { > + { PHY_ID_ADIN1200, 0xfffffff0 }, > + { PHY_ID_ADIN1300, 0xfffffff0 }, PHY_ID_MATCH_MODEL could be used here too. > + { } > +}; > + > +MODULE_DEVICE_TABLE(mdio, adin_tbl); > +MODULE_DESCRIPTION("Analog Devices Industrial Ethernet PHY driver"); > +MODULE_LICENSE("GPL"); >
On Mon, 2019-08-05 at 16:16 +0200, Andrew Lunn wrote: > [External] > > > +static int adin_config_init(struct phy_device *phydev) > > +{ > > + int rc; > > + > > + rc = genphy_config_init(phydev); > > + if (rc < 0) > > + return rc; > > + > > + return 0; > > +} > > Why not just > > return genphy_config_init(phydev); Because stuff will get added after this return statement in the next patches. I thought maybe this would be a good idea to keep the git changes minimal, but I can do a direct return and update it in the next patches when needed. > > Andrew >
On Mon, 2019-08-05 at 17:17 +0200, Andrew Lunn wrote: > [External] > > > +static struct phy_driver adin_driver[] = { > > + { > > + .phy_id = PHY_ID_ADIN1200, > > + .name = "ADIN1200", > > + .phy_id_mask = 0xfffffff0, > > + .features = PHY_BASIC_FEATURES, > > Do you need this? If the device implements the registers correctly, > phylib can determine this from the registers. ack; will take a look; > > > + .config_init = adin_config_init, > > + .config_aneg = genphy_config_aneg, > > + .read_status = genphy_read_status, > > + }, > > + { > > + .phy_id = PHY_ID_ADIN1300, > > + .name = "ADIN1300", > > + .phy_id_mask = 0xfffffff0, > > + .features = PHY_GBIT_FEATURES, > > same here. ack; > > > + .config_init = adin_config_init, > > + .config_aneg = genphy_config_aneg, > > + .read_status = genphy_read_status, > > + }, > > +}; > > + > > +module_phy_driver(adin_driver); > > + > > +static struct mdio_device_id __maybe_unused adin_tbl[] = { > > + { PHY_ID_ADIN1200, 0xfffffff0 }, > > + { PHY_ID_ADIN1300, 0xfffffff0 }, > > PHY_ID_MATCH_VENDOR(). ack; > > Andrew
On Mon, 2019-08-05 at 22:54 +0200, Heiner Kallweit wrote: > [External] > > On 05.08.2019 18:54, Alexandru Ardelean wrote: > > This change adds support for Analog Devices Industrial Ethernet PHYs. > > Particularly the PHYs this driver adds support for: > > * ADIN1200 - Robust, Industrial, Low Power 10/100 Ethernet PHY > > * ADIN1300 - Robust, Industrial, Low Latency 10/100/1000 Gigabit > > Ethernet PHY > > > > The 2 chips are pin & register compatible with one another. The main > > difference being that ADIN1200 doesn't operate in gigabit mode. > > > > The chips can be operated by the Generic PHY driver as well via the > > standard IEEE PHY registers (0x0000 - 0x000F) which are supported by the > > kernel as well. This assumes that configuration of the PHY has been done > > required. > > > > Configuration can also be done via registers, which will be implemented by > > the driver in the next changes. > > > > Datasheets: > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1300.pdf > > https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1200.pdf > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > --- > > MAINTAINERS | 7 +++++ > > drivers/net/phy/Kconfig | 9 ++++++ > > drivers/net/phy/Makefile | 1 + > > drivers/net/phy/adin.c | 59 ++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 76 insertions(+) > > create mode 100644 drivers/net/phy/adin.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index ee663e0e2f2e..faf5723610c8 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -938,6 +938,13 @@ S: Supported > > F: drivers/mux/adgs1408.c > > F: Documentation/devicetree/bindings/mux/adi,adgs1408.txt > > > > +ANALOG DEVICES INC ADIN DRIVER > > +M: Alexandru Ardelean <alexaundru.ardelean@analog.com> > > +L: netdev@vger.kernel.org > > +W: http://ez.analog.com/community/linux-device-drivers > > +S: Supported > > +F: drivers/net/phy/adin.c > > + > > ANALOG DEVICES INC ADIS DRIVER LIBRARY > > M: Alexandru Ardelean <alexandru.ardelean@analog.com> > > S: Supported > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > > index 206d8650ee7f..5966d3413676 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -257,6 +257,15 @@ config SFP > > depends on HWMON || HWMON=n > > select MDIO_I2C > > > > +config ADIN_PHY > > + tristate "Analog Devices Industrial Ethernet PHYs" > > + help > > + Adds support for the Analog Devices Industrial Ethernet PHYs. > > + Currently supports the: > > + - ADIN1200 - Robust,Industrial, Low Power 10/100 Ethernet PHY > > + - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit > > + Ethernet PHY > > + > > config AMD_PHY > > tristate "AMD PHYs" > > ---help--- > > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > > index ba07c27e4208..a03437e091f3 100644 > > --- a/drivers/net/phy/Makefile > > +++ b/drivers/net/phy/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_SFP) += sfp.o > > sfp-obj-$(CONFIG_SFP) += sfp-bus.o > > obj-y += $(sfp-obj-y) $(sfp-obj-m) > > > > +obj-$(CONFIG_ADIN_PHY) += adin.o > > obj-$(CONFIG_AMD_PHY) += amd.o > > aquantia-objs += aquantia_main.o > > ifdef CONFIG_HWMON > > diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c > > new file mode 100644 > > index 000000000000..6a610d4563c3 > > --- /dev/null > > +++ b/drivers/net/phy/adin.c > > @@ -0,0 +1,59 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/** > > + * Driver for Analog Devices Industrial Ethernet PHYs > > + * > > + * Copyright 2019 Analog Devices Inc. > > + */ > > +#include <linux/kernel.h> > > +#include <linux/errno.h> > > +#include <linux/init.h> > > +#include <linux/module.h> > > +#include <linux/mii.h> > > +#include <linux/phy.h> > > + > > +#define PHY_ID_ADIN1200 0x0283bc20 > > +#define PHY_ID_ADIN1300 0x0283bc30 > > + > > +static int adin_config_init(struct phy_device *phydev) > > +{ > > + int rc; > > + > > + rc = genphy_config_init(phydev); > > + if (rc < 0) > > + return rc; > > + > > + return 0; > > +} > > + > > +static struct phy_driver adin_driver[] = { > > + { > > + .phy_id = PHY_ID_ADIN1200, > > You could use PHY_ID_MATCH_MODEL here. > > > + .name = "ADIN1200", > > + .phy_id_mask = 0xfffffff0, > > + .features = PHY_BASIC_FEATURES, > > Setting features is deprecated, instead the get_features callback > should be implemented if the default genphy_read_abilities needs > to be extended / replaced. You say that the PHY's work with the > genphy driver, so I suppose the default feature detection is ok > in your case. Then you could simply remove setting "features". ack; thanks for the info > > > + .config_init = adin_config_init, > > + .config_aneg = genphy_config_aneg, > > + .read_status = genphy_read_status, > > + }, > > + { > > + .phy_id = PHY_ID_ADIN1300, > > + .name = "ADIN1300", > > + .phy_id_mask = 0xfffffff0, > > + .features = PHY_GBIT_FEATURES, > > + .config_init = adin_config_init, > > + .config_aneg = genphy_config_aneg, > > + .read_status = genphy_read_status, > > + }, > > +}; > > + > > +module_phy_driver(adin_driver); > > + > > +static struct mdio_device_id __maybe_unused adin_tbl[] = { > > + { PHY_ID_ADIN1200, 0xfffffff0 }, > > + { PHY_ID_ADIN1300, 0xfffffff0 }, > > PHY_ID_MATCH_MODEL could be used here too. ack; will take a look > > > + { } > > +}; > > + > > +MODULE_DEVICE_TABLE(mdio, adin_tbl); > > +MODULE_DESCRIPTION("Analog Devices Industrial Ethernet PHY driver"); > > +MODULE_LICENSE("GPL"); > >
diff --git a/MAINTAINERS b/MAINTAINERS index ee663e0e2f2e..faf5723610c8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -938,6 +938,13 @@ S: Supported F: drivers/mux/adgs1408.c F: Documentation/devicetree/bindings/mux/adi,adgs1408.txt +ANALOG DEVICES INC ADIN DRIVER +M: Alexandru Ardelean <alexaundru.ardelean@analog.com> +L: netdev@vger.kernel.org +W: http://ez.analog.com/community/linux-device-drivers +S: Supported +F: drivers/net/phy/adin.c + ANALOG DEVICES INC ADIS DRIVER LIBRARY M: Alexandru Ardelean <alexandru.ardelean@analog.com> S: Supported diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 206d8650ee7f..5966d3413676 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -257,6 +257,15 @@ config SFP depends on HWMON || HWMON=n select MDIO_I2C +config ADIN_PHY + tristate "Analog Devices Industrial Ethernet PHYs" + help + Adds support for the Analog Devices Industrial Ethernet PHYs. + Currently supports the: + - ADIN1200 - Robust,Industrial, Low Power 10/100 Ethernet PHY + - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit + Ethernet PHY + config AMD_PHY tristate "AMD PHYs" ---help--- diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index ba07c27e4208..a03437e091f3 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -47,6 +47,7 @@ obj-$(CONFIG_SFP) += sfp.o sfp-obj-$(CONFIG_SFP) += sfp-bus.o obj-y += $(sfp-obj-y) $(sfp-obj-m) +obj-$(CONFIG_ADIN_PHY) += adin.o obj-$(CONFIG_AMD_PHY) += amd.o aquantia-objs += aquantia_main.o ifdef CONFIG_HWMON diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c new file mode 100644 index 000000000000..6a610d4563c3 --- /dev/null +++ b/drivers/net/phy/adin.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0+ +/** + * Driver for Analog Devices Industrial Ethernet PHYs + * + * Copyright 2019 Analog Devices Inc. + */ +#include <linux/kernel.h> +#include <linux/errno.h> +#include <linux/init.h> +#include <linux/module.h> +#include <linux/mii.h> +#include <linux/phy.h> + +#define PHY_ID_ADIN1200 0x0283bc20 +#define PHY_ID_ADIN1300 0x0283bc30 + +static int adin_config_init(struct phy_device *phydev) +{ + int rc; + + rc = genphy_config_init(phydev); + if (rc < 0) + return rc; + + return 0; +} + +static struct phy_driver adin_driver[] = { + { + .phy_id = PHY_ID_ADIN1200, + .name = "ADIN1200", + .phy_id_mask = 0xfffffff0, + .features = PHY_BASIC_FEATURES, + .config_init = adin_config_init, + .config_aneg = genphy_config_aneg, + .read_status = genphy_read_status, + }, + { + .phy_id = PHY_ID_ADIN1300, + .name = "ADIN1300", + .phy_id_mask = 0xfffffff0, + .features = PHY_GBIT_FEATURES, + .config_init = adin_config_init, + .config_aneg = genphy_config_aneg, + .read_status = genphy_read_status, + }, +}; + +module_phy_driver(adin_driver); + +static struct mdio_device_id __maybe_unused adin_tbl[] = { + { PHY_ID_ADIN1200, 0xfffffff0 }, + { PHY_ID_ADIN1300, 0xfffffff0 }, + { } +}; + +MODULE_DEVICE_TABLE(mdio, adin_tbl); +MODULE_DESCRIPTION("Analog Devices Industrial Ethernet PHY driver"); +MODULE_LICENSE("GPL");
This change adds support for Analog Devices Industrial Ethernet PHYs. Particularly the PHYs this driver adds support for: * ADIN1200 - Robust, Industrial, Low Power 10/100 Ethernet PHY * ADIN1300 - Robust, Industrial, Low Latency 10/100/1000 Gigabit Ethernet PHY The 2 chips are pin & register compatible with one another. The main difference being that ADIN1200 doesn't operate in gigabit mode. The chips can be operated by the Generic PHY driver as well via the standard IEEE PHY registers (0x0000 - 0x000F) which are supported by the kernel as well. This assumes that configuration of the PHY has been done required. Configuration can also be done via registers, which will be implemented by the driver in the next changes. Datasheets: https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1300.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1200.pdf Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- MAINTAINERS | 7 +++++ drivers/net/phy/Kconfig | 9 ++++++ drivers/net/phy/Makefile | 1 + drivers/net/phy/adin.c | 59 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 drivers/net/phy/adin.c