On 25/10/2022 09:52, Camel Guo wrote: > Add initial framework for Maxlinear's GSW1xx switch and > currently only GSW145 in MDIO managed mode is supported. > > Signed-off-by: Camel Guo <camel.guo@xxxxxxxx> > --- (...) > + priv->ds->dev = dev; > + priv->ds->num_ports = priv->hw_info->max_ports; > + priv->ds->priv = priv; > + priv->ds->ops = &gsw1xx_switch_ops; > + priv->dev = dev; > + version = gsw1xx_switch_r(priv, GSW1XX_IP_VERSION); > + > + np = dev->of_node; > + switch (version) { > + case GSW1XX_IP_VERSION_2_3: > + if (!of_device_is_compatible(np, "mxl,gsw145-mdio")) > + return -EINVAL; > + break; > + default: > + dev_err(dev, "unknown GSW1XX_IP version: 0x%x", version); > + return -ENOENT; > + } > + > + /* bring up the mdio bus */ > + mdio_np = of_get_child_by_name(np, "mdio"); > + if (!mdio_np) { > + dev_err(dev, "missing child mdio node\n"); > + return -EINVAL; > + } > + > + err = gsw1xx_mdio(priv, mdio_np); > + if (err) { > + dev_err(dev, "mdio probe failed\n"); dev_err_probe() > + goto put_mdio_node; > + } > + > + err = dsa_register_switch(priv->ds); > + if (err) { > + dev_err(dev, "dsa switch register failed: %i\n", err); dev_err_probe() > + goto mdio_bus; > + } > + if (!dsa_is_cpu_port(priv->ds, priv->hw_info->cpu_port)) { > + dev_err(dev, > + "wrong CPU port defined, HW only supports port: %i", > + priv->hw_info->cpu_port); > + err = -EINVAL; > + goto disable_switch; > + } > + > + dev_set_drvdata(dev, priv); > + > + return 0; > + > +disable_switch: > + gsw1xx_mdio_mask(priv, GSW1XX_MDIO_GLOB_ENABLE, 0, GSW1XX_MDIO_GLOB); > + dsa_unregister_switch(priv->ds); > +mdio_bus: > + if (mdio_np) { > + mdiobus_unregister(priv->ds->slave_mii_bus); > + mdiobus_free(priv->ds->slave_mii_bus); > + } > +put_mdio_node: > + of_node_put(mdio_np); > + return err; > +} > +EXPORT_SYMBOL(gsw1xx_probe); > + > +void gsw1xx_remove(struct gsw1xx_priv *priv) > +{ > + if (!priv) > + return; > + > + /* disable the switch */ > + gsw1xx_mdio_mask(priv, GSW1XX_MDIO_GLOB_ENABLE, 0, GSW1XX_MDIO_GLOB); > + > + dsa_unregister_switch(priv->ds); > + > + if (priv->ds->slave_mii_bus) { > + mdiobus_unregister(priv->ds->slave_mii_bus); > + of_node_put(priv->ds->slave_mii_bus->dev.of_node); > + mdiobus_free(priv->ds->slave_mii_bus); > + } > + > + dev_set_drvdata(priv->dev, NULL); > +} > +EXPORT_SYMBOL(gsw1xx_remove); > + > +void gsw1xx_shutdown(struct gsw1xx_priv *priv) > +{ > + if (!priv) > + return; > + > + /* disable the switch */ > + gsw1xx_mdio_mask(priv, GSW1XX_MDIO_GLOB_ENABLE, 0, GSW1XX_MDIO_GLOB); > + > + dsa_switch_shutdown(priv->ds); > + > + dev_set_drvdata(priv->dev, NULL); > +} > +EXPORT_SYMBOL(gsw1xx_shutdown); 1. EXPORT_SYMBOL_GPL 2. Why do you do it in the first place? It's one driver, no need for building two modules. Same applies to other places. > + > +static const struct regmap_range gsw1xx_valid_regs[] = { > + /* GSWIP Core Registers */ > + regmap_reg_range(GSW1XX_IP_BASE_ADDR, > + GSW1XX_IP_BASE_ADDR + GSW1XX_IP_REG_LEN), > + /* Top Level PDI Registers, MDIO Master Reigsters */ > + regmap_reg_range(GSW1XX_MDIO_BASE_ADDR, > + GSW1XX_MDIO_BASE_ADDR + GSW1XX_MDIO_REG_LEN), > +}; > + > +const struct regmap_access_table gsw1xx_register_set = { > + .yes_ranges = gsw1xx_valid_regs, > + .n_yes_ranges = ARRAY_SIZE(gsw1xx_valid_regs), > +}; > +EXPORT_SYMBOL(gsw1xx_register_set); > + > +MODULE_AUTHOR("Camel Guo <camel.guo@xxxxxxxx>"); > +MODULE_DESCRIPTION("Core Driver for MaxLinear GSM1XX ethernet switch"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/net/dsa/gsw1xx_mdio.c b/drivers/net/dsa/gsw1xx_mdio.c > new file mode 100644 > index 000000000000..8328001041ed > --- /dev/null > +++ b/drivers/net/dsa/gsw1xx_mdio.c > @@ -0,0 +1,128 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * MaxLinear switch driver for GSW1XX in MDIO managed mode > + */ > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/mdio.h> > +#include <linux/phy.h> > +#include <linux/of.h> > + > +#include "gsw1xx.h" > + > +#define GSW1XX_SMDIO_TARGET_BASE_ADDR_REG 0x1F > + > +static int gsw1xx_mdio_write(void *ctx, uint32_t reg, uint32_t val) > +{ > + struct mdio_device *mdiodev = (struct mdio_device *)ctx; > + int ret = 0; > + > + mutex_lock_nested(&mdiodev->bus->mdio_lock, MDIO_MUTEX_NESTED); > + > + ret = mdiodev->bus->write(mdiodev->bus, mdiodev->addr, > + GSW1XX_SMDIO_TARGET_BASE_ADDR_REG, reg); > + if (ret < 0) > + goto out; > + > + ret = mdiodev->bus->write(mdiodev->bus, mdiodev->addr, 0, val); > + > +out: > + mutex_unlock(&mdiodev->bus->mdio_lock); > + > + return ret; > +} > + > +static int gsw1xx_mdio_read(void *ctx, uint32_t reg, uint32_t *val) > +{ > + struct mdio_device *mdiodev = (struct mdio_device *)ctx; > + int ret = 0; > + > + mutex_lock_nested(&mdiodev->bus->mdio_lock, MDIO_MUTEX_NESTED); > + > + ret = mdiodev->bus->write(mdiodev->bus, mdiodev->addr, > + GSW1XX_SMDIO_TARGET_BASE_ADDR_REG, reg); > + if (ret < 0) > + goto out; > + > + *val = mdiodev->bus->read(mdiodev->bus, mdiodev->addr, 0); > + > +out: > + mutex_unlock(&mdiodev->bus->mdio_lock); > + > + return ret; > +} > + > +static const struct regmap_config gsw1xx_mdio_regmap_config = { > + .reg_bits = 16, > + .val_bits = 16, > + .reg_stride = 1, > + > + .disable_locking = true, > + > + .volatile_table = &gsw1xx_register_set, > + .wr_table = &gsw1xx_register_set, > + .rd_table = &gsw1xx_register_set, > + > + .reg_read = gsw1xx_mdio_read, > + .reg_write = gsw1xx_mdio_write, > + > + .cache_type = REGCACHE_NONE, > +}; > + > +static int gsw1xx_mdio_probe(struct mdio_device *mdiodev) > +{ > + struct gsw1xx_priv *priv; > + struct device *dev = &mdiodev->dev; > + int ret; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->regmap = devm_regmap_init(dev, NULL, mdiodev, > + &gsw1xx_mdio_regmap_config); > + if (IS_ERR(priv->regmap)) { > + ret = PTR_ERR(priv->regmap); > + dev_err(dev, "regmap init failed: %d\n", ret); > + return ret; return dev_err_probe(). > + } > + > + return gsw1xx_probe(priv, dev); > +} > + > +static void gsw1xx_mdio_remove(struct mdio_device *mdiodev) > +{ > + gsw1xx_remove(dev_get_drvdata(&mdiodev->dev)); > +} > + > +static void gsw1xx_mdio_shutdown(struct mdio_device *mdiodev) > +{ > + gsw1xx_shutdown(dev_get_drvdata(&mdiodev->dev)); > +} > + > +static const struct gsw1xx_hw_info gsw145_hw_info = { > + .max_ports = 6, > + .cpu_port = 5, > +}; > + > +static const struct of_device_id gsw1xx_mdio_of_match[] = { > + { .compatible = "mxl,gsw145-mdio", .data = &gsw145_hw_info }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, gsw1xx_mdio_of_match); > + > +static struct mdio_driver gsw1xx_mdio_driver = { > + .probe = gsw1xx_mdio_probe, > + .remove = gsw1xx_mdio_remove, > + .shutdown = gsw1xx_mdio_shutdown, > + .mdiodrv.driver = { > + .name = "GSW1XX_MDIO", > + .of_match_table = of_match_ptr(gsw1xx_mdio_of_match), of_match_ptr requires maybe_unused. Or just drop it. Best regards, Krzysztof