| Message ID | 20190224141338.29907-2-pankaj.bansal@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,1/2] dt-bindings: net: Add bindings for mdio mux consumers | expand |
> +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
On 2019-02-24 15:57, Andrew Lunn wrote: >> +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. That is correct. Some background; The mux framework was designed for cases where a) two (or more) mux consumers "compete" for the same mux controller, or b) where a single mux chip shares several mux controllers where each controller might be used for some independent purpose. The design is also built around short accesses, such that no single mux consumer starves other consumers of a shared mux controller. I.e. - select the mux and implicitly set it in the desired position - do something short-ish which requires the mux - deselect the mux But it does work for exclusive consumers of a mux controller as in this series. However, if the consumer 1) requires the implementation of a new mux controller, 2) does not need any of the existing mux drivers and 3) the situation is not in one of the a) and b) categories then the mux framework is not really providing any killer features. I.e. it seems like it is certainly an option to just open code the needed regmap accesses and ignore the multiplexer subsystem in this case. However, the changes needed in the mux framework are quite minimal in this case [1], so 1) is arguably not applicable... More details on the design can be found in the original mux series [2]. Cheers, Peter [1] https://lkml.org/lkml/2019/2/24/23 [2] https://lkml.org/lkml/2017/5/14/160
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 3d187cd50eb0..7f66af446ec7 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -87,6 +87,18 @@ config MDIO_BUS_MUX_MMIOREG Currently, only 8/16/32 bits registers are supported. +config MDIO_BUS_MUX_MULTIPLEXER + tristate "MDIO bus multiplexer using kernel multiplexer subsystem" + depends on OF + select MULTIPLEXER + select MDIO_BUS_MUX + help + This module provides a driver for MDIO bus multiplexer + that is controlled via the kernel multiplexer subsystem. The + bus multiplexer connects one of several child MDIO busses to + a parent bus. Child bus selection is under the control of + the kernel multiplexer subsystem. + config MDIO_CAVIUM tristate diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index 5805c0b7d60e..766106237a90 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o obj-$(CONFIG_MDIO_BUS_MUX_BCM_IPROC) += mdio-mux-bcm-iproc.o obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o obj-$(CONFIG_MDIO_BUS_MUX_MMIOREG) += mdio-mux-mmioreg.o +obj-$(CONFIG_MDIO_BUS_MUX_MULTIPLEXER) += mdio-mux-multiplexer.o obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o diff --git a/drivers/net/phy/mdio-mux-multiplexer.c b/drivers/net/phy/mdio-mux-multiplexer.c new file mode 100644 index 000000000000..22adbbb93aa6 --- /dev/null +++ b/drivers/net/phy/mdio-mux-multiplexer.c @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* MDIO bus multiplexer using kernel multiplexer subsystem + * + * Copyright 2019 NXP + */ + +#include <linux/platform_device.h> +#include <linux/mdio-mux.h> +#include <linux/module.h> +#include <linux/mux/consumer.h> + +struct mdio_mux_multiplexer_state { + struct mux_control *muxc; + void *mux_handle; +}; + +/** + * mdio_mux_multiplexer_switch_fn - This function is called by the mdio-mux + * layer when it thinks the mdio bus + * multiplexer needs to switch. + * @current_child: current value of the mux register. + * @desired_child: value of the 'reg' property of the target child MDIO node. + * @data: Private data used by this switch_fn passed to mdio_mux_init function + * via mdio_mux_init(.., .., .., .., data, ..). + * + * The first time this function is called, current_child == -1. + * If current_child == desired_child, then the mux is already set to the + * correct bus. + */ +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); + if (current_child ^ desired_child) { + if (current_child != -1) + ret = mux_control_deselect(s->muxc); + if (ret) + return ret; + + 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); + + return 0; +} + +static const struct of_device_id mdio_mux_multiplexer_match[] = { + { .compatible = "mdio-mux-multiplexer", }, + {}, +}; +MODULE_DEVICE_TABLE(of, mdio_mux_multiplexer_match); + +static struct platform_driver mdio_mux_multiplexer_driver = { + .driver = { + .name = "mdio-mux-multiplexer", + .of_match_table = mdio_mux_multiplexer_match, + }, + .probe = mdio_mux_multiplexer_probe, + .remove = mdio_mux_multiplexer_remove, +}; + +module_platform_driver(mdio_mux_multiplexer_driver); + +MODULE_DESCRIPTION("MDIO bus multiplexer using kernel multiplexer subsystem"); +MODULE_AUTHOR("Pankaj Bansal"); +MODULE_LICENSE("GPL");
Add support for Generic Mux controls, when Mdio mux node is a consumer of mux produced by some other device. Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> --- Notes: V2: - Moved the changes from mdio-mux module to new module mdio-mux-multiplexer - defined new CONFIG - using new compatible "mdio-mux-multiplexer"