| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 25 Oct 2022 10:23:55 -0400
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Camel Guo <camel.guo@...s.com>, Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Florian Fainelli <f.fainelli@...il.com>,
Jakub Kicinski <kuba@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh+dt@...nel.org>,
Russell King <linux@...linux.org.uk>,
Vivien Didelot <vivien.didelot@...il.com>,
Vladimir Oltean <olteanv@...il.com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, Rob Herring <robh@...nel.org>,
kernel@...s.com
Subject: Re: [RFC net-next 2/2] net: dsa: Add driver for Maxlinear GSW1XX
switch
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@...s.com>
> ---
(...)
> + 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@...s.com>");
> +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
Powered by blists - more mailing lists