netdev - Re: [net-next 1/2] net: dsa: realtek: create realtek-common

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231119121338.dskcbfwlo5txe245@skbuf>
Date: Sun, 19 Nov 2023 14:13:38 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Luiz Angelo Daros de Luca <luizluca@...il.com>
Cc: netdev@...r.kernel.org, linus.walleij@...aro.org, alsi@...g-olufsen.dk,
	andrew@...n.ch, f.fainelli@...il.com, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	arinc.unal@...nc9.com
Subject: Re: [net-next 1/2] net: dsa: realtek: create realtek-common

On Fri, Nov 17, 2023 at 08:50:00PM -0300, Luiz Angelo Daros de Luca wrote:
> +struct realtek_priv *realtek_common_probe(struct device *dev,
> +		struct regmap_config rc, struct regmap_config rc_nolock)

nitpick: A more conventional way of aligning the arguments would be:

struct realtek_priv *
realtek_common_probe(struct device *dev, struct regmap_config rc,
		     struct regmap_config rc_nolock)

> +{
> +	const struct realtek_variant *var;
> +	struct realtek_priv *priv;
> +	struct device_node *np;
> +	int ret;
> +
> +	var = of_device_get_match_data(dev);
> +	if (!var)
> +		return ERR_PTR(-EINVAL);
> +
> +	priv = devm_kzalloc(dev, size_add(sizeof(*priv), var->chip_data_sz),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mutex_init(&priv->map_lock);
> +
> +	rc.lock_arg = priv;
> +	priv->map = devm_regmap_init(dev, NULL, priv, &rc);
> +	if (IS_ERR(priv->map)) {
> +		ret = PTR_ERR(priv->map);
> +		dev_err(dev, "regmap init failed: %d\n", ret);
> +		return ERR_PTR(ret);
> +	}
> +
> +	priv->map_nolock = devm_regmap_init(dev, NULL, priv, &rc_nolock);
> +	if (IS_ERR(priv->map_nolock)) {
> +		ret = PTR_ERR(priv->map_nolock);
> +		dev_err(dev, "regmap init failed: %d\n", ret);
> +		return ERR_PTR(ret);
> +	}
> +
> +	/* Link forward and backward */
> +	priv->dev = dev;
> +	priv->variant = var;
> +	priv->ops = var->ops;
> +	priv->chip_data = (void *)priv + sizeof(*priv);
> +
> +	dev_set_drvdata(dev, priv);
> +	spin_lock_init(&priv->lock);
> +
> +	np = dev->of_node;
> +
> +	priv->leds_disabled = of_property_read_bool(np, "realtek,disable-leds");
> +
> +	/* TODO: if power is software controlled, set up any regulators here */
> +
> +	priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(priv->reset)) {
> +		dev_err(dev, "failed to get RESET GPIO\n");
> +		return ERR_CAST(priv->reset);
> +	}
> +	if (priv->reset) {
> +		gpiod_set_value(priv->reset, 1);
> +		dev_dbg(dev, "asserted RESET\n");
> +		msleep(REALTEK_HW_STOP_DELAY);
> +		gpiod_set_value(priv->reset, 0);
> +		msleep(REALTEK_HW_START_DELAY);
> +		dev_dbg(dev, "deasserted RESET\n");
> +	}
> +
> +	priv->ds = devm_kzalloc(dev, sizeof(*priv->ds), GFP_KERNEL);
> +	if (!priv->ds)
> +		return ERR_PTR(-ENOMEM);
> +
> +	priv->ds->dev = dev;
> +	priv->ds->priv = priv;
> +
> +	return priv;
> +}
> +EXPORT_SYMBOL(realtek_common_probe);
> +
> +void realtek_common_remove(struct realtek_priv *priv)
> +{
> +	if (!priv)
> +		return;
> +
> +	dsa_unregister_switch(priv->ds);
> +	if (priv->user_mii_bus)
> +		of_node_put(priv->user_mii_bus->dev.of_node);
> +
> +	/* leave the device reset asserted */
> +	if (priv->reset)
> +		gpiod_set_value(priv->reset, 1);
> +}
> +EXPORT_SYMBOL(realtek_common_remove);

realtek_common_remove() should not undo things that realtek_common_probe()
never did.