netdev - Re: [PATCH v2 2/2] drivers: net: phy: mdio-mux: Add support for Generic Mux controls

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: <20190224145733.GD26626@lunn.ch>
Date:   Sun, 24 Feb 2019 15:57:33 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Pankaj Bansal <pankaj.bansal@....com>
Cc:     Leo Li <leoyang.li@....com>, Peter Rosin <peda@...ntia.se>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] drivers: net: phy: mdio-mux: Add support for
 Generic Mux controls

> +static int mdio_mux_multiplexer_switch_fn(int current_child, int desired_child,
> +					  void *data)
> +{
> +	struct platform_device *pdev;
> +	struct mdio_mux_multiplexer_state *s;
> +	int ret = 0;
> +
> +	pdev = (struct platform_device *)data;
> +	s = (struct mdio_mux_multiplexer_state *)platform_get_drvdata(pdev);

platform_get_drvdata() returns a void *. So you don't need the cast.

> +	if (current_child ^ desired_child) {
> +		if (current_child != -1)
> +			ret = mux_control_deselect(s->muxc);
> +		if (ret)
> +			return ret;

Please use {} here, so you check the error when current_child != -1.

> +
> +		ret =  mux_control_select(s->muxc, desired_child);
> +		if (!ret)
> +			dev_dbg(&pdev->dev, "%s %d -> %d\n", __func__,
> +				current_child, desired_child);
> +	}
> +
> +	return ret;
> +}
> +
> +static int mdio_mux_multiplexer_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mdio_mux_multiplexer_state *s;
> +	int ret = 0;
> +
> +	s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
> +	if (!s)
> +		return -ENOMEM;
> +
> +	s->muxc = devm_mux_control_get(dev, NULL);
> +	if (IS_ERR(s->muxc)) {
> +		ret = PTR_ERR(s->muxc);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Failed to get mux: %d\n", ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, s);
> +
> +	ret = mdio_mux_init(&pdev->dev, pdev->dev.of_node,
> +			    mdio_mux_multiplexer_switch_fn, &s->mux_handle,
> +			    pdev, NULL);
> +
> +	return ret;
> +}
> +
> +static int mdio_mux_multiplexer_remove(struct platform_device *pdev)
> +{
> +	struct mdio_mux_multiplexer_state *s = platform_get_drvdata(pdev);
> +
> +	mdio_mux_uninit(s->mux_handle);

I've never used the multiplexer framework before. But i think you need
a call to mux_control_deselect() here in order to unlock the
multiplexer. Without that, it will deadlock when you reload the
module.

	Andrew