On 2016-12-31 17:19, Jonathan Cameron wrote: > On 30/11/16 08:16, Peter Rosin wrote: >> Add a new minimalistic subsystem that handles multiplexer controllers. >> When multiplexers are used in various places in the kernel, and the >> same multiplexer controller can be used for several independent things, >> there should be one place to implement support for said multiplexer >> controller. >> >> A single multiplexer controller can also be used to control several >> parallel multiplexers, that are in turn used by different subsystems >> in the kernel, leading to a need to coordinate multiplexer accesses. >> The multiplexer subsystem handles this coordination. >> >> This new mux controller subsystem initially comes with a single backend >> driver that controls gpio based multiplexers. Even though not needed by >> this initial driver, the mux controller subsystem is prepared to handle >> chips with multiple (independent) mux controllers. >> >> Signed-off-by: Peter Rosin <peda@xxxxxxxxxx> > Few trivial bits inline + question of whether misc is the right location.. > It's small, but not totally trivial as subsystems go, so perhaps it needs it's > own directory. In [9/9] you come to the conclusion that muxes should have their own directory, but do you think it should be drivers/mux/* or drivers/misc/mux/* ? >> --- >> Documentation/driver-model/devres.txt | 6 +- >> MAINTAINERS | 2 + >> drivers/misc/Kconfig | 30 ++++ >> drivers/misc/Makefile | 2 + >> drivers/misc/mux-core.c | 311 ++++++++++++++++++++++++++++++++++ >> drivers/misc/mux-gpio.c | 138 +++++++++++++++ >> include/linux/mux.h | 197 +++++++++++++++++++++ >> 7 files changed, 685 insertions(+), 1 deletion(-) >> create mode 100644 drivers/misc/mux-core.c >> create mode 100644 drivers/misc/mux-gpio.c >> create mode 100644 include/linux/mux.h >> >> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt >> index ca9d1eb46bc0..d64ede85b61b 100644 >> --- a/Documentation/driver-model/devres.txt >> +++ b/Documentation/driver-model/devres.txt >> @@ -330,7 +330,11 @@ MEM >> devm_kzalloc() >> >> MFD >> - devm_mfd_add_devices() > Technically should be in a separate cleanup patch... >> + devm_mfd_add_devices() >> + >> +MUX >> + devm_mux_control_get() >> + devm_mux_control_put() >> >> PER-CPU MEM >> devm_alloc_percpu() >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 3d4d0efc2b64..dc7498682752 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -8407,6 +8407,8 @@ MULTIPLEXER SUBSYSTEM >> M: Peter Rosin <peda@xxxxxxxxxx> >> S: Maintained >> F: Documentation/devicetree/bindings/misc/mux-* >> +F: include/linux/mux.h >> +F: drivers/misc/mux-* >> >> MULTISOUND SOUND DRIVER >> M: Andrew Veliath <andrewtv@xxxxxxx> >> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig >> index 64971baf11fa..2ce675e410c5 100644 >> --- a/drivers/misc/Kconfig >> +++ b/drivers/misc/Kconfig >> @@ -766,6 +766,36 @@ config PANEL_BOOT_MESSAGE >> An empty message will only clear the display at driver init time. Any other >> printf()-formatted message is valid with newline and escape codes. >> >> +menuconfig MULTIPLEXER >> + bool "Multiplexer subsystem" >> + help >> + Multiplexer controller subsystem. Multiplexers are used in a >> + variety of settings, and this subsystem abstracts their use >> + so that the rest of the kernel sees a common interface. When >> + multiple parallel multiplexers are controlled by one single >> + multiplexer controller, this subsystem also coordinates the >> + multiplexer accesses. >> + >> + If unsure, say no. >> + > Fun question of the day. Is misc the place to put this or should it > have it's own directory. I'd go for own directory... I thought it too small for its own dir and that it could always be moved later. But I don't really care... > On the plus side, whilst it's in misc you get to pester Greg and Arnd > for review. I know :-] >> +if MULTIPLEXER >> + >> +config MUX_GPIO >> + tristate "GPIO-controlled Multiplexer" >> + depends on OF && GPIOLIB >> + help >> + GPIO-controlled Multiplexer controller. >> + >> + The driver builds a single multiplexer controller using a number >> + of gpio pins. For N pins, there will be 2^N possible multiplexer >> + states. The GPIO pins can be connected (by the hardware) to several *snip* >> + >> +void mux_chip_free(struct mux_chip *mux_chip) >> +{ >> + if (!mux_chip) >> + return; > I'd drop a blank line in here. Slightly nicer to read. Right. >> + put_device(&mux_chip->dev); >> +} >> +EXPORT_SYMBOL_GPL(mux_chip_free); *snip* >> + >> +static int mux_gpio_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct device_node *np = pdev->dev.of_node; >> + struct mux_chip *mux_chip; >> + struct mux_gpio *mux_gpio; >> + int pins; >> + u32 idle_state; >> + int ret; >> + >> + if (!np) >> + return -ENODEV; >> + >> + pins = gpiod_count(dev, "mux"); >> + if (pins < 0) >> + return pins; >> + >> + mux_chip = mux_chip_alloc(dev, 1, sizeof(*mux_gpio) + >> + pins * sizeof(*mux_gpio->val)); >> + if (!mux_chip) >> + return -ENOMEM; > Rather feels like mux_chip_alloc is a good candidate for a managed > allocator. Might be worth doing the register as well then the remove > below would go away. I guess perhaps not that worthwhile as not many > mux types are likely to ever exist... To me it seemed like too much extra support code for just two drivers. And it can always be added later if/when more mux drivers show up... >> + >> + mux_gpio = mux_chip_priv(mux_chip); >> + mux_gpio->val = (int *)(mux_gpio + 1); *snip* Cheers, peda -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html