| 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: Mon, 02 Jan 2017 09:20:21 +0000
From: Jonathan Cameron <jic23@...23.retrosnub.co.uk>
To: Peter Rosin <peda@...ntia.se>, Jonathan Cameron <jic23@...nel.org>,
linux-kernel@...r.kernel.org
CC: Wolfram Sang <wsa@...-dreams.de>, Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Hartmut Knaack <knaack.h@....de>,
Lars-Peter Clausen <lars@...afoo.de>,
Peter Meerwald-Stadler <pmeerw@...erw.net>,
Jonathan Corbet <corbet@....net>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
linux-iio@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v6 2/9] misc: minimal mux subsystem and gpio-based mux controller
On 2 January 2017 09:14:34 GMT+00:00, Peter Rosin <peda@...ntia.se> wrote:
>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@...ntia.se>
>> 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/*
Probably this one...
>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@...ntia.se>
>>> S: Maintained
>>> F: Documentation/devicetree/bindings/misc/mux-*
>>> +F: include/linux/mux.h
>>> +F: drivers/misc/mux-*
>>>
>>> MULTISOUND SOUND DRIVER
>>> M: Andrew Veliath <andrewtv@....net>
>>> 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...
There will be others :) sure do it later.
>
>>> +
>>> + 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@...r.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Powered by blists - more mailing lists