* Re: [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2017-01-11 8:22 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A, patrice.chotard-qxv4g6HH51o,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1483607246-14771-3-git-send-email-cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hello Cedric,
On Thu, Jan 05, 2017 at 10:07:23AM +0100, M'boumba Cedric Madianga wrote:
> +/*
> + * In standard mode:
> + * SCL period = SCL high period = SCL low period = CCR * I2C parent clk period
> + *
> + * In fast mode:
> + * If Duty = 0; SCL high period = 1 * CCR * I2C parent clk period
> + * SCL low period = 2 * CCR * I2C parent clk period
> + * If Duty = 1; SCL high period = 9 * CCR * I2C parent clk period
> + * SCL low period = 16 * CCR * I2C parent clk period
s/ \*/ */ several times
> + * In order to reach 400 kHz with lower I2C parent clk frequencies we always set
> + * Duty = 1
> + *
> + * For both modes, we have CCR = SCL period * I2C parent clk frequency
> + * with scl_period = 5 microseconds in Standard mode and scl_period = 1
s/mode/Mode/
> + * microsecond in Fast Mode in order to satisfy scl_high and scl_low periods
> + * constraints defined by i2c bus specification
I don't understand scl_period = 1 µs for Fast Mode. For a bus freqency
of 400 kHz we need low + high = 2.5 µs. Is there a factor 10 missing
somewhere?
> + */
> +static struct stm32f4_i2c_timings i2c_timings[] = {
> [...]
> +
> +/**
> + * stm32f4_i2c_hw_config() - Prepare I2C block
> + * @i2c_dev: Controller's private data
> + */
> +static int stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
> + int ret = 0;
> +
> + /* Disable I2C */
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
> +
> + ret = stm32f4_i2c_set_periph_clk_freq(i2c_dev);
> + if (ret)
> + return ret;
> +
> + stm32f4_i2c_set_rise_time(i2c_dev);
> +
> + stm32f4_i2c_set_speed_mode(i2c_dev);
> +
> + stm32f4_i2c_set_filter(i2c_dev);
> +
> + /* Enable I2C */
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
This function is called after a hw reset, so there should be no need to
use clr_bits and set_bits because the value read from hw should be
known.
> + return ret;
return 0;
> +}
> +
> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + u32 status;
> + int ret;
> +
> + ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
> + status,
> + !(status & STM32F4_I2C_SR2_BUSY),
> + 10, 1000);
> + if (ret) {
> + dev_dbg(i2c_dev->dev, "bus not free\n");
> + ret = -EBUSY;
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * stm32f4_i2c_write_ byte() - Write a byte in the data register
> + * @i2c_dev: Controller's private data
> + * @byte: Data to write in the register
> + */
> +static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
> +{
> + writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
> +}
> +
> +/**
> + * stm32f4_i2c_write_msg() - Fill the data register in write mode
> + * @i2c_dev: Controller's private data
> + *
> + * This function fills the data register with I2C transfer buffer
> + */
> +static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> +
> + stm32f4_i2c_write_byte(i2c_dev, *msg->buf++);
> + msg->count--;
> +}
> +
> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + u32 rbuf;
> +
> + rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
> + *msg->buf++ = rbuf & 0xff;
This is unnecessary. buf has an 8 bit wide type so
*msg->buf++ = rbuf;
has the same effect. (ISTR this is something I already pointed out
earlier?)
> + msg->count--;
> +}
> +
> +static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> +
> + stm32f4_i2c_disable_irq(i2c_dev);
> +
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + if (msg->stop)
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> + else
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> +
> + complete(&i2c_dev->complete);
> +}
> +
> +/**
> + * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> +
> + if (msg->count) {
> + stm32f4_i2c_write_msg(i2c_dev);
> + if (!msg->count) {
> + /* Disable buffer interrupts for RXNE/TXE events */
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> + }
> + } else {
> + stm32f4_i2c_terminate_xfer(i2c_dev);
Is stm32f4_i2c_terminate_xfer also called when arbitration is lost? If
yes, is it then right to set STM32F4_I2C_CR1_STOP or
STM32F4_I2C_CR1_START?
> + }
> +}
> +
> +/**
> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> +
> + switch (msg->count) {
> + case 1:
> + stm32f4_i2c_disable_irq(i2c_dev);
> + stm32f4_i2c_read_msg(i2c_dev);
> + complete(&i2c_dev->complete);
> + break;
> + /*
> + * For 2 or 3-byte reception, we do not have to read the data register
> + * when RXNE occurs as we have to wait for byte transferred finished
it's hard to understand because if you don't know the hardware the
meaning of RXNE is unknown.
> + * event before reading data. So, here we just disable buffer
> + * interrupt in order to avoid another system preemption due to RXNE
> + * event
> + */
> + case 2:
> + case 3:
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> + break;
> + /* For N byte reception with N > 3 we directly read data register */
> + default:
> + stm32f4_i2c_read_msg(i2c_dev);
> + }
> +}
> +
> +/**
> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
> + * in case of read
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
> +{
btf is a hw-related name. Maybe better use _done which is easier to
understand?
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + void __iomem *reg;
> + u32 mask;
> + int i;
> +
> + switch (msg->count) {
> + case 2:
> + /*
> + * In order to correctly send the Stop or Repeated Start
> + * condition on the I2C bus, the STOP/START bit has to be set
> + * before reading the last two bytes.
> + * After that, we could read the last two bytes, disable
> + * remaining interrupts and notify the end of xfer to the
> + * client
This is surprising. I didn't recheck the manual, but that looks very
uncomfortable. How does this work, when I only want to read a single
byte? Same problem for ACK below.
> + */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + if (msg->stop)
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> + else
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> +
> + for (i = 2; i > 0; i--)
> + stm32f4_i2c_read_msg(i2c_dev);
> +
> + reg = i2c_dev->base + STM32F4_I2C_CR2;
> + mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
> + stm32f4_i2c_clr_bits(reg, mask);
> +
> + complete(&i2c_dev->complete);
> + break;
> + case 3:
> + /*
> + * In order to correctly send the ACK on the I2C bus for the
> + * last two bytes, we have to set ACK bit before reading the
> + * third last data byte
> + */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> + stm32f4_i2c_read_msg(i2c_dev);
> + break;
> + default:
> + stm32f4_i2c_read_msg(i2c_dev);
> + }
> +}
> +
> +/**
> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
> + * master receiver
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> + void __iomem *reg;
> +
> + switch (msg->count) {
> + case 0:
> + stm32f4_i2c_terminate_xfer(i2c_dev);
> + /* Clear ADDR flag */
> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> + break;
> + case 1:
> + /*
> + * Single byte reception:
This also happens for the last byte of a 5 byte transfer, right?
> + * Enable NACK, clear ADDR flag and generate STOP or RepSTART
> + */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> + if (msg->stop)
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> + else
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> + break;
> + case 2:
> + /*
> + * 2-byte reception:
> + * Enable NACK and set POS
What is POS?
> + */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
You could get rid of this, when caching the value of CR1. Would save two
register reads here. This doesn't work for all registers, but it should
be possible to apply for most of them, maybe enough to get rid of the
clr_bits and set_bits function.
> + readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> + break;
> +
> + default:
> + /* N-byte reception: Enable ACK */
> + reg = i2c_dev->base + STM32F4_I2C_CR1;
> + stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
Do you need to set ACK for each byte transferred?
I stopp reviewing here because of -ENOTIME on my side but don't want to
delay discussion, so sent my comments up to here already now.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] i2c: piix4: Avoid race conditions with IMC
From: Andy Shevchenko @ 2017-01-11 1:49 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Jean Delvare, Wolfram Sang, linux-i2c,
linux-kernel@vger.kernel.org
In-Reply-To: <20170110121600.30649-1-ricardo.ribalda@gmail.com>
On Tue, Jan 10, 2017 at 2:16 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> On AMD's SB800 and upwards, the SMBus is shared with the Integrated
> Micro Controller (IMC).
>
> The platform provides a hardware semaphore to avoid race conditions
> among them. (Check page 288 of the SB800-Series Southbridges Register
> Reference Guide http://support.amd.com/TechDocs/45482.pdf)
It would be nice to understand what kind of devices are accessing and to where.
Hans seems discovered one pretty nice issue on Intel
BayTrail/CherryTrail platforms where I2C semaphore is used to prevent
simultaneous access to P-Unit, but we have two paths there which are
not synchronized (yet). It brings a set of interesting (and
unfortunately "famous") bugs.
>
> Without this patch, many access to the SMBus end with an invalid
> transaction or even with the bus stalled.
>
> Credit-to: Alexandre Desnoyers <alex@qtec.com>
Never saw before. Did he suggested the solution or what?
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -585,9 +585,28 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
> u8 command, int size, union i2c_smbus_data *data)
> {
> struct i2c_piix4_adapdata *adapdata = i2c_get_adapdata(adap);
> + unsigned short piix4_smba = adapdata->smba;
> u8 smba_en_lo;
> u8 port;
> int retval;
> + int timeout = 0;
> + int smbslvcnt;
Keep them just after your another added variable.
> + /* Request the SMBUS semaphore, avoid conflicts with the IMC */
> + smbslvcnt = inb_p(SMBSLVCNT);
> + while (++timeout < MAX_TIMEOUT) {
Usual pattern is countdown.
do {
...
} while (--timeout);
> + outb_p(smbslvcnt | 0x10, SMBSLVCNT);
> +
> + /* Check the semaphore status */
> + smbslvcnt = inb_p(SMBSLVCNT);
> + if (smbslvcnt & 0x10)
> + break;
> +
> + usleep_range(1000, 2000);
> + }
> + /* SMBus is still owned by the IMC, we give up */
> + if (timeout == MAX_TIMEOUT)
> + return -EBUSY;
Would caller do it again? Perhaps -EAGAIN?
Since the returned value is not -ETIMEDOUT, I suppose the name of
counter variable is a bit confusing. Basically it's amount of attempts
with some gap between them. Though, it's up to you and maintainer.
> + /* Release the semaphore */
> + outb_p(smbslvcnt | 0x20, SMBSLVCNT);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 05/12] mux: support simplified bindings for single-user gpio mux
From: Jonathan Cameron @ 2017-01-10 21:22 UTC (permalink / raw)
To: Peter Rosin, linux-kernel
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
linux-iio, linux-doc
In-Reply-To: <60cd24a4-dd13-c5e8-c06d-bbd12a8e7e21@axentia.se>
On 08/01/17 21:56, Peter Rosin wrote:
> On 2017-01-08 11:28, Jonathan Cameron wrote:
>> On 05/01/17 16:21, Peter Rosin wrote:
>>> On 2017-01-04 13:16, Peter Rosin wrote:
>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>> ---
>>>> drivers/mux/mux-core.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++--
>>>> drivers/mux/mux-gpio.c | 56 ++--------------------------------
>>>> include/linux/mux.h | 7 +++++
>>>> 3 files changed, 89 insertions(+), 55 deletions(-)
>>>>
>>>> diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
>>>> index 21da15a264ad..d887ae1c0e55 100644
>>>> --- a/drivers/mux/mux-core.c
>>>> +++ b/drivers/mux/mux-core.c
>>>> @@ -15,6 +15,7 @@
>>>> #include <linux/device.h>
>>>> #include <linux/err.h>
>>>> #include <linux/idr.h>
>>>> +#include <linux/gpio/consumer.h>
>>>> #include <linux/module.h>
>>>> #include <linux/mux.h>
>>>> #include <linux/of.h>
>>>> @@ -288,6 +289,63 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
>>>> return dev ? to_mux_chip(dev) : NULL;
>>>> }
>>>>
>>>> +#ifdef CONFIG_MUX_GPIO
>>>> +
>>>> +static int mux_gpio_set(struct mux_control *mux, int state)
>>>> +{
>>>> + struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < mux_gpio->gpios->ndescs; i++)
>>>> + mux_gpio->val[i] = (state >> i) & 1;
>>>> +
>>>> + gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
>>>> + mux_gpio->gpios->desc,
>>>> + mux_gpio->val);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct mux_control_ops mux_gpio_ops = {
>>>> + .set = mux_gpio_set,
>>>> +};
>>>> +
>>>> +struct mux_chip *mux_gpio_alloc(struct device *dev)
>>>> +{
>>>> + struct mux_chip *mux_chip;
>>>> + struct mux_gpio *mux_gpio;
>>>> + int pins;
>>>> + int ret;
>>>> +
>>>> + pins = gpiod_count(dev, "mux");
>>>> + if (pins < 0)
>>>> + return ERR_PTR(pins);
>>>> +
>>>> + mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_gpio) +
>>>> + pins * sizeof(*mux_gpio->val));
>>>> + if (!mux_chip)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + mux_gpio = mux_chip_priv(mux_chip);
>>>> + mux_gpio->val = (int *)(mux_gpio + 1);
>>>> + mux_chip->ops = &mux_gpio_ops;
>>>> +
>>>> + mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
>>>> + if (IS_ERR(mux_gpio->gpios)) {
>>>> + ret = PTR_ERR(mux_gpio->gpios);
>>>> + if (ret != -EPROBE_DEFER)
>>>> + dev_err(dev, "failed to get gpios\n");
>>>> + return ERR_PTR(ret);
>>>> + }
>>>> + WARN_ON(pins != mux_gpio->gpios->ndescs);
>>>> + mux_chip->mux->states = 1 << pins;
>>>> +
>>>> + return mux_chip;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(mux_gpio_alloc);
>>>> +
>>>> +#endif /* CONFIG_MUX_GPIO */
>>>> +
>>>> struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
>>>> {
>>>> struct device_node *np = dev->of_node;
>>>> @@ -307,9 +365,28 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
>>>> ret = of_parse_phandle_with_args(np,
>>>> "mux-controls", "#mux-control-cells",
>>>> index, &args);
>>>> +
>>>> +#ifdef CONFIG_MUX_GPIO
>>>> + if (ret == -ENOENT && !mux_name && gpiod_count(dev, "mux") > 0) {
>>>> + mux_chip = mux_gpio_alloc(dev);
>>>> + if (!IS_ERR(mux_chip)) {
>>>> + ret = devm_mux_chip_register(dev, mux_chip);
>>>> + if (ret < 0) {
>>>> + dev_err(dev, "failed to register mux-chip\n");
>>>> + return ERR_PTR(ret);
>>>> + }
>>>> + get_device(&mux_chip->dev);
>>>> + return mux_chip->mux;
>>>> + }
>>>> +
>>>> + ret = PTR_ERR(mux_chip);
>>>> + }
>>>> +#endif
>>>> +
>>>> if (ret) {
>>>> - dev_err(dev, "%s: failed to get mux-control %s(%i)\n",
>>>> - np->full_name, mux_name ?: "", index);
>>>> + if (ret != -EPROBE_DEFER)
>>>> + dev_err(dev, "%s: failed to get mux-control %s(%i)\n",
>>>> + np->full_name, mux_name ?: "", index);
>>>> return ERR_PTR(ret);
>>>> }
>>>>
>>>> diff --git a/drivers/mux/mux-gpio.c b/drivers/mux/mux-gpio.c
>>>> index 76b52bc63470..8a7bfbc0c4bb 100644
>>>> --- a/drivers/mux/mux-gpio.c
>>>> +++ b/drivers/mux/mux-gpio.c
>>>> @@ -11,37 +11,12 @@
>>>> */
>>>>
>>>> #include <linux/err.h>
>>>> -#include <linux/gpio/consumer.h>
>>>> #include <linux/module.h>
>>>> #include <linux/mux.h>
>>>> #include <linux/of_platform.h>
>>>> #include <linux/platform_device.h>
>>>> #include <linux/property.h>
>>>
>>> Instead of moving the mux-gpio guts from mux-gpio.c to mux-core.c, I
>>> will instead make CONFIG_MUX_GPIO a bool option (no module possible)
>>> and call it from the mux-core. That will be cleaner and less of a
>>> break of abstractions in my opinion.
>> Hmm. I wonder if the balance is right here or whether we should just not have the
>> simplified binding at all as it breaks the assumption that all muxes are of the
>> same level...
>>
>> I like the binding, but it is causing significant complexity in here.
>
> Yes, in this patch it looks pretty bad. But with the suggested
> changes it at least looks fine (apart from the remaining ifdef
> in mux_control_get). But then there's of course the conceptual
> badness of the core depending on a specific driver that you
> mention...
>
> But. I expect that gpio based muxes will be in vast majority, and
> giving them some extra freedoms is probably not wrong considering
> the upside of the simpler binding.
>
Difficult balance. As you are going to be the poor soul who has
to maintain this I'm not that bothered either way ;)
Probably worth gathering a few more opinions on this though if
they are forthcoming.
Jonathan
>>>
>>> Cheers,
>>> Peter
>>>
>>>> -struct mux_gpio {
>>>> - struct gpio_descs *gpios;
>>>> - int *val;
>>>> -};
>>>> -
>>>> -static int mux_gpio_set(struct mux_control *mux, int state)
>>>> -{
>>>> - struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
>>>> - int i;
>>>> -
>>>> - for (i = 0; i < mux_gpio->gpios->ndescs; i++)
>>>> - mux_gpio->val[i] = (state >> i) & 1;
>>>> -
>>>> - gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
>>>> - mux_gpio->gpios->desc,
>>>> - mux_gpio->val);
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> -static const struct mux_control_ops mux_gpio_ops = {
>>>> - .set = mux_gpio_set,
>>>> -};
>>>> -
>>>> static const struct of_device_id mux_gpio_dt_ids[] = {
>>>> { .compatible = "mux-gpio", },
>>>> { /* sentinel */ }
>>>> @@ -51,38 +26,13 @@ MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
>>>> static int mux_gpio_probe(struct platform_device *pdev)
>>>> {
>>>> struct device *dev = &pdev->dev;
>>>> - struct device_node *np = 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 = devm_mux_chip_alloc(dev, 1, sizeof(*mux_gpio) +
>>>> - pins * sizeof(*mux_gpio->val));
>>>> - if (!mux_chip)
>>>> - return -ENOMEM;
>>>> -
>>>> - mux_gpio = mux_chip_priv(mux_chip);
>>>> - mux_gpio->val = (int *)(mux_gpio + 1);
>>>> - mux_chip->ops = &mux_gpio_ops;
>>>> -
>>>> - mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
>>>> - if (IS_ERR(mux_gpio->gpios)) {
>>>> - ret = PTR_ERR(mux_gpio->gpios);
>>>> - if (ret != -EPROBE_DEFER)
>>>> - dev_err(dev, "failed to get gpios\n");
>>>> - return ret;
>>>> - }
>>>> - WARN_ON(pins != mux_gpio->gpios->ndescs);
>>>> - mux_chip->mux->states = 1 << pins;
>>>> + mux_chip = mux_gpio_alloc(dev);
>>>> + if (IS_ERR(mux_chip))
>>>> + return PTR_ERR(mux_chip);
>>>>
>>>> ret = device_property_read_u32(dev, "idle-state", &idle_state);
>>>> if (ret >= 0) {
>>>> diff --git a/include/linux/mux.h b/include/linux/mux.h
>>>> index 3b9439927f11..3bfee23cfb8c 100644
>>>> --- a/include/linux/mux.h
>>>> +++ b/include/linux/mux.h
>>>> @@ -241,4 +241,11 @@ struct mux_control *devm_mux_control_get(struct device *dev,
>>>> */
>>>> void devm_mux_control_put(struct device *dev, struct mux_control *mux);
>>>>
>>>> +struct mux_gpio {
>>>> + struct gpio_descs *gpios;
>>>> + int *val;
>>>> +};
>>>> +
>>>> +struct mux_chip *mux_gpio_alloc(struct device *dev);
>>>> +
>>>> #endif /* _LINUX_MUX_H */
>>>>
>>>
>>
>
^ permalink raw reply
* [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-10 18:48 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
trivial, mranostay, linux-i2c, devicetree
Cc: ak
This is the IIO driver for devantech srf08 ultrasonic ranger which can be
used to measure the distances to an object.
The sensor supports I2C with some registers.
Supported Features include:
- read the distance in ranging mode in centimeters
- output of the driver is directly the read value
- together with the scale the driver delivers the distance in meters
- only the first echo of the nearest object is delivered
- set max gain register; userspace enters analogue value
- set range registers; userspace enters range in millimeters in 43 mm steps
Features not supported by this driver:
- ranging mode in inches or in microseconds
- ANN mode
- change I2C address through this driver
- light sensor
The driver was added in the directory "proximity" of the iio subsystem
in absence of another directory named "distance".
There is also a new submenu "distance"
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
drivers/iio/proximity/Kconfig | 15 ++
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/srf08.c | 362 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 378 insertions(+)
create mode 100644 drivers/iio/proximity/srf08.c
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index ef4c73db5b53..7b10a137702b 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -46,3 +46,18 @@ config SX9500
module will be called sx9500.
endmenu
+
+menu "Distance sensors"
+
+config SRF08
+ tristate "Devantech SRF08 ultrasonic ranger sensor"
+ depends on I2C
+ help
+ Say Y here to build a driver for Devantech SRF08 ultrasonic
+ ranger sensor. This driver can be used to measure the distance
+ of objects.
+
+ To compile this driver as a module, choose M here: the
+ module will be called srf08.
+
+endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 9aadd9a8ee99..e914c2a5dd49 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -5,4 +5,5 @@
# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AS3935) += as3935.o
obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
+obj-$(CONFIG_SRF08) += srf08.o
obj-$(CONFIG_SX9500) += sx9500.o
diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
new file mode 100644
index 000000000000..f38c74ed0933
--- /dev/null
+++ b/drivers/iio/proximity/srf08.c
@@ -0,0 +1,362 @@
+/*
+ * srf08.c - Support for Devantech SRF08 ultrasonic ranger
+ *
+ * Copyright (c) 2016 Andreas Klinger <ak@it-klinger.de>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * For details about the device see:
+ * http://www.robot-electronics.co.uk/htm/srf08tech.html
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/bitops.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/* registers of SRF08 device */
+#define SRF08_WRITE_COMMAND 0x00 /* Command Register */
+#define SRF08_WRITE_MAX_GAIN 0x01 /* Max Gain Register: 0 .. 31 */
+#define SRF08_WRITE_RANGE 0x02 /* Range Register: 0 .. 255 */
+#define SRF08_READ_SW_REVISION 0x00 /* Software Revision */
+#define SRF08_READ_LIGHT 0x01 /* Light Sensor during last echo */
+#define SRF08_READ_ECHO_1_HIGH 0x02 /* Range of first echo received */
+#define SRF08_READ_ECHO_1_LOW 0x03 /* Range of first echo received */
+
+#define SRF08_CMD_RANGING_CM 0x51 /* Ranging Mode - Result in cm */
+
+#define SRF08_DEFAULT_GAIN 1025 /* max. analogue value of Gain */
+#define SRF08_DEFAULT_RANGE 11008 /* max. value of Range in mm */
+
+struct srf08_data {
+ struct i2c_client *client;
+ int gain; /* Max Gain */
+ int range_mm; /* Range in mm */
+ struct mutex lock;
+};
+
+static const int srf08_gain[] = {
+ 94, 97, 100, 103, 107, 110, 114, 118,
+ 123, 128, 133, 139, 145, 152, 159, 168,
+ 177, 187, 199, 212, 227, 245, 265, 288,
+ 317, 352, 395, 450, 524, 626, 777, 1025 };
+
+static int srf08_read_ranging(struct srf08_data *data)
+{
+ struct i2c_client *client = data->client;
+ int ret, i;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ SRF08_WRITE_COMMAND, SRF08_CMD_RANGING_CM);
+ if (ret < 0) {
+ dev_err(&client->dev, "write command - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ /*
+ * normally after 65 ms the device should have the read value
+ * we round it up to 100 ms
+ *
+ * we read here until a correct version number shows up as
+ * suggested by the documentation
+ */
+ for (i = 0; i < 5; i++) {
+ ret = i2c_smbus_read_byte_data(data->client,
+ SRF08_READ_SW_REVISION);
+
+ /* check if a valid version number is read */
+ if (ret < 255 && ret > 0)
+ break;
+ msleep(20);
+ }
+
+ if (ret >= 255 || ret <= 0) {
+ dev_err(&client->dev, "device not ready\n");
+ mutex_unlock(&data->lock);
+ return -EIO;
+ }
+
+ ret = i2c_smbus_read_word_swapped(data->client,
+ SRF08_READ_ECHO_1_HIGH);
+ if (ret < 0) {
+ dev_err(&client->dev, "cannot read distance: ret=%d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ mutex_unlock(&data->lock);
+
+ return ret;
+}
+
+static int srf08_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int *val,
+ int *val2, long mask)
+{
+ struct srf08_data *data = iio_priv(indio_dev);
+ int ret;
+
+ if (channel->type != IIO_DISTANCE)
+ return -EINVAL;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = srf08_read_ranging(data);
+ if (ret < 0)
+ return ret;
+ *val = ret;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ /* 1 LSB is 1 cm */
+ *val = 0;
+ *val2 = 10000;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static ssize_t srf08_show_range_mm_available(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, len = 0;
+
+ for (i = 0; i < 256; i++)
+ len += scnprintf(buf + len, PAGE_SIZE - len,
+ "%d ", (i + 1) * 43);
+
+ buf[len - 1] = '\n';
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(range_mm_available, S_IRUGO,
+ srf08_show_range_mm_available, NULL, 0);
+
+static ssize_t srf08_show_range_mm(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+
+ return sprintf(buf, "%d\n", data->range_mm);
+}
+
+/*
+ * set the range of the sensor to an even multiple of 43 mm
+ * which corresponds to 1 LSB in the register
+ *
+ * register value corresponding range
+ * 0x00 43 mm
+ * 0x01 86 mm
+ * 0x02 129 mm
+ * ...
+ * 0xFF 11008 mm
+ */
+static ssize_t srf08_write_range_mm(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ struct i2c_client *client = data->client;
+ int ret;
+ unsigned int val, mod;
+ u8 regval;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ ret = val / 43 - 1;
+ mod = val % 43;
+
+ if (mod || (ret < 0) || (ret > 255))
+ return -EINVAL;
+
+ regval = ret;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ SRF08_WRITE_RANGE, regval);
+ if (ret < 0) {
+ dev_err(&client->dev, "write_range - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ data->range_mm = val;
+
+ mutex_unlock(&data->lock);
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(range_mm, S_IRUGO | S_IWUSR,
+ srf08_show_range_mm, srf08_write_range_mm, 0);
+
+static ssize_t srf08_show_gain_available(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, len = 0;
+
+ for (i = 0; i < ARRAY_SIZE(srf08_gain); i++)
+ len += sprintf(buf + len, "%d ", srf08_gain[i]);
+
+ len += sprintf(buf + len, "\n");
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(gain_available, S_IRUGO,
+ srf08_show_gain_available, NULL, 0);
+
+static ssize_t srf08_show_gain(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ int len;
+
+ len = sprintf(buf, "%d\n", data->gain);
+
+ return len;
+}
+
+static ssize_t srf08_write_gain(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ struct i2c_client *client = data->client;
+ int ret, i;
+ unsigned int val;
+ u8 regval;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ARRAY_SIZE(srf08_gain); i++)
+ if (val == srf08_gain[i]) {
+ regval = i;
+ break;
+ }
+
+ if (i >= ARRAY_SIZE(srf08_gain))
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ SRF08_WRITE_MAX_GAIN, regval);
+ if (ret < 0) {
+ dev_err(&client->dev, "write_gain - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ data->gain = val;
+
+ mutex_unlock(&data->lock);
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(gain, S_IRUGO | S_IWUSR,
+ srf08_show_gain, srf08_write_gain, 0);
+
+static struct attribute *srf08_attributes[] = {
+ &iio_dev_attr_range_mm.dev_attr.attr,
+ &iio_dev_attr_range_mm_available.dev_attr.attr,
+ &iio_dev_attr_gain.dev_attr.attr,
+ &iio_dev_attr_gain_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group srf08_attribute_group = {
+ .attrs = srf08_attributes,
+};
+
+static const struct iio_chan_spec srf08_channels[] = {
+ {
+ .type = IIO_DISTANCE,
+ .info_mask_separate =
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+static const struct iio_info srf08_info = {
+ .read_raw = srf08_read_raw,
+ .attrs = &srf08_attribute_group,
+ .driver_module = THIS_MODULE,
+};
+
+static int srf08_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct iio_dev *indio_dev;
+ struct srf08_data *data;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
+ I2C_FUNC_SMBUS_READ_WORD_DATA))
+ return -ENODEV;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+
+ /*
+ * set default values of device here
+ * these values are already set on the hardware after power on
+ */
+ data->gain = SRF08_DEFAULT_GAIN;
+ data->range_mm = SRF08_DEFAULT_RANGE;
+
+ indio_dev->name = dev_name(&client->dev);
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &srf08_info;
+ indio_dev->channels = srf08_channels;
+ indio_dev->num_channels = ARRAY_SIZE(srf08_channels);
+
+ mutex_init(&data->lock);
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id srf08_id[] = {
+ { "srf08", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, srf08_id);
+
+static struct i2c_driver srf08_driver = {
+ .driver = {
+ .name = "srf08",
+ },
+ .probe = srf08_probe,
+ .id_table = srf08_id,
+};
+module_i2c_driver(srf08_driver);
+
+MODULE_AUTHOR("Andreas Klinger <ak@it-klinger.de>");
+MODULE_DESCRIPTION("Devantech SRF08 ultrasonic ranger driver");
+MODULE_LICENSE("GPL");
--
2.1.4
^ permalink raw reply related
* [PATCH v2 1/2] iio: distance: srf08: add trivial DT binding
From: Andreas Klinger @ 2017-01-10 18:47 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
trivial, mranostay, linux-i2c, devicetree
Cc: ak
Add DT binding for devantech,srf08
Add vendor devantech to vendor list
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 539874490492..86c6930c3c91 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -36,6 +36,7 @@ dallas,ds1775 Tiny Digital Thermometer and Thermostat
dallas,ds3232 Extremely Accurate I²C RTC with Integrated Crystal and SRAM
dallas,ds4510 CPU Supervisor with Nonvolatile Memory and Programmable I/O
dallas,ds75 Digital Thermometer and Thermostat
+devantech,srf08 Devantech SRF08 ultrasonic ranger
dlg,da9053 DA9053: flexible system level PMIC with multicore support
dlg,da9063 DA9063: system PMIC for quad-core application processors
epson,rx8010 I2C-BUS INTERFACE REAL TIME CLOCK MODULE
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 4696bb5c2198..80325e602403 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -65,6 +65,7 @@ dallas Maxim Integrated Products (formerly Dallas Semiconductor)
davicom DAVICOM Semiconductor, Inc.
delta Delta Electronics, Inc.
denx Denx Software Engineering
+devantech Devantech, Ltd.
digi Digi International Inc.
digilent Diglent, Inc.
dlg Dialog Semiconductor
--
2.1.4
^ permalink raw reply related
* [PATCH v2 0/2] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-10 18:47 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
trivial, mranostay, linux-i2c, devicetree
Cc: ak
This patch series adds IIO driver support for srf08 ultrasonic ranger
devices.
The first patch add a trivial device tree binding for the device together
with a new vendor devantech.
The second patch is the IIO driver which in turn is using I2C to talk to
the device.
Documentation about the sensor can be found here:
http://www.robot-electronics.co.uk/htm/srf08tech.html
Changes in v2:
Lots of updates thanks to Peters really fast review within 30 minutes
after first submission of the driver.
* Patch 2: "iio: distance: srf08: add IIO driver for us ranger"
- alphabetic order in Makefile
- use of u8 while accessing registers
- avoid endianness problems with 16 bit values
- missing return value checks
- some explaining documentation added
Andreas Klinger (2):
iio: distance: srf08: add trivial DT binding
iio: distance: srf08: add IIO driver for us ranger
.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/iio/proximity/Kconfig | 15 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/srf08.c | 362 +++++++++++++++++++++
5 files changed, 380 insertions(+)
create mode 100644 drivers/iio/proximity/srf08.c
--
2.1.4
^ permalink raw reply
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
From: Guenter Roeck @ 2017-01-10 17:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Edward James, andrew, corbet, devicetree, eajames.ibm, jdelvare,
joel, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
mark.rutland, robh+dt, wsa
In-Reply-To: <1484070104.21117.29.camel@kernel.crashing.org>
On Tue, Jan 10, 2017 at 11:41:44AM -0600, Benjamin Herrenschmidt wrote:
> On Sat, 2017-01-07 at 09:15 -0800, Guenter Roeck wrote:
> > > Instead of the "online" attribute, what do you think about using the
> > > "bind"/"unbind" API to probe the device from user space once the system
> > > is powered on? All the hwmon registration would take place in the probe
> > > function, it would just occur some time after boot.
> > >
> >
> > A more common approach would be to have a platform driver. That platform
> > driver would need a means to detect if the OCC is up and running, and
> > instantiate everything else once it is.
> >
> > A trigger from user space is problematic because there is no guarantee
> > that the OCC is really up (or that it even exists).
> >
> > An alternative might be to have the hwmon driver poll for the OCC,
> > but that would be a bit more difficult and might require a kernel thread
> > or maybe asynchronous probing.
>
> Hi Guenter !
>
> I'm not sure I agree with you here ;-)
>
> I'm don't know how much background you got (I missed the beginning of
> the discussion) but basically this driver runs on the BMC (system
> controller) of the POWER9 server, the OCC is inside the POWER9 chip
> itself.
>
> So the presence/power state of the OCC doesn't depend on the BMC
> platform kernel code. The BMC userspace controls power up and down to the POWER9, and thus it's the one to know whether the remote is up.
>
> If we were to create a "platform driver", all it would do is get input
> from userspace exactly like that sysfs file :-)
>
> So if you don't like the sysfs file that registers/de-registers, which
> I sort-of understand, it's a bit of a hack (though a rather efficient
> one), I think the bind/unbind approach makes sense. However, I wonder
> whether the simplest and most efficient (remember this BMC has a really
> slow CPU) is an "online" file sysfs file, though rather than
> registering/de-registering the hwmon it would simply make it stop
> accessing the HW (and return some known "offline" values).
>
I don't like that too much either; it still looks like a hack.
How about using bind/unbind then ?
Guenter
^ permalink raw reply
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
From: Benjamin Herrenschmidt @ 2017-01-10 17:45 UTC (permalink / raw)
To: Andrew Jeffery, Guenter Roeck, Edward James
Cc: corbet, devicetree, eajames.ibm, jdelvare, joel, linux-doc,
linux-hwmon, linux-i2c, linux-kernel, mark.rutland, robh+dt, wsa
In-Reply-To: <1483919532.2950.1.camel@aj.id.au>
On Mon, 2017-01-09 at 10:22 +1030, Andrew Jeffery wrote:
> Alternatively, in the style of your first para, we could push the
> host
> CPU state management into the kernel and expose a boot/reboot/power-
> off
> API to userspace. That would give us a place to hook calls for
> configuring and cleaning up any host-dependent drivers on the BMC.
That's nasty. Each machine has a subtly different way of controller
host power, that would mean a different kernel driver for each of them,
which we are trying to avoid.
This is userspace policy.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
From: Benjamin Herrenschmidt @ 2017-01-10 17:41 UTC (permalink / raw)
To: Guenter Roeck, Edward James
Cc: andrew, corbet, devicetree, eajames.ibm, jdelvare, joel,
linux-doc, linux-hwmon, linux-i2c, linux-kernel, mark.rutland,
robh+dt, wsa
In-Reply-To: <8b182766-32a0-9eb1-7917-14abf811cef5@roeck-us.net>
On Sat, 2017-01-07 at 09:15 -0800, Guenter Roeck wrote:
> > Instead of the "online" attribute, what do you think about using the
> > "bind"/"unbind" API to probe the device from user space once the system
> > is powered on? All the hwmon registration would take place in the probe
> > function, it would just occur some time after boot.
> >
>
> A more common approach would be to have a platform driver. That platform
> driver would need a means to detect if the OCC is up and running, and
> instantiate everything else once it is.
>
> A trigger from user space is problematic because there is no guarantee
> that the OCC is really up (or that it even exists).
>
> An alternative might be to have the hwmon driver poll for the OCC,
> but that would be a bit more difficult and might require a kernel thread
> or maybe asynchronous probing.
Hi Guenter !
I'm not sure I agree with you here ;-)
I'm don't know how much background you got (I missed the beginning of
the discussion) but basically this driver runs on the BMC (system
controller) of the POWER9 server, the OCC is inside the POWER9 chip
itself.
So the presence/power state of the OCC doesn't depend on the BMC
platform kernel code. The BMC userspace controls power up and down to the POWER9, and thus it's the one to know whether the remote is up.
If we were to create a "platform driver", all it would do is get input
from userspace exactly like that sysfs file :-)
So if you don't like the sysfs file that registers/de-registers, which
I sort-of understand, it's a bit of a hack (though a rather efficient
one), I think the bind/unbind approach makes sense. However, I wonder
whether the simplest and most efficient (remember this BMC has a really
slow CPU) is an "online" file sysfs file, though rather than
registering/de-registering the hwmon it would simply make it stop
accessing the HW (and return some known "offline" values).
Cheers,
Ben.
^ permalink raw reply
* [PATCH 1/2] iio: distance: srf08: add trivial DT binding
From: Andreas Klinger @ 2017-01-10 15:27 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
trivial, mranostay, linux-i2c, devicetree
Cc: ak
Add DT binding for devantech,srf08
Add vendor devantech to vendor list
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
2 files changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
index 539874490492..86c6930c3c91 100644
--- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
@@ -36,6 +36,7 @@ dallas,ds1775 Tiny Digital Thermometer and Thermostat
dallas,ds3232 Extremely Accurate I²C RTC with Integrated Crystal and SRAM
dallas,ds4510 CPU Supervisor with Nonvolatile Memory and Programmable I/O
dallas,ds75 Digital Thermometer and Thermostat
+devantech,srf08 Devantech SRF08 ultrasonic ranger
dlg,da9053 DA9053: flexible system level PMIC with multicore support
dlg,da9063 DA9063: system PMIC for quad-core application processors
epson,rx8010 I2C-BUS INTERFACE REAL TIME CLOCK MODULE
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 4696bb5c2198..80325e602403 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -65,6 +65,7 @@ dallas Maxim Integrated Products (formerly Dallas Semiconductor)
davicom DAVICOM Semiconductor, Inc.
delta Delta Electronics, Inc.
denx Denx Software Engineering
+devantech Devantech, Ltd.
digi Digi International Inc.
digilent Diglent, Inc.
dlg Dialog Semiconductor
--
2.1.4
^ permalink raw reply related
* [PATCH 2/2] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-10 15:28 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ktsai-GubuWUlQtMwciDkP5Hr2oA,
wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, trivial-DgEjT+Ai2ygdnm+yROfE0A,
mranostay-Re5JQEeQqe8AvxtiuMwx3w,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: ak-n176/SwNRljddJNmlsFzeA
This is the IIO driver for devantech srf08 ultrasonic ranger which can be
used to measure the distances to an object.
The sensor supports I2C with some registers.
Supported Features include:
- read the distance in ranging mode in centimeters
- output of the driver is directly the read value
- together with the scale the driver delivers the distance in meters
- only the first echo of the nearest object is delivered
- set max gain register; userspace enters analogue value
- set range registers; userspace enters range in millimeters in 43 mm steps
Features not supported by this driver:
- ranging mode in inches or in microseconds
- ANN mode
- change I2C address through this driver
- light sensor
The driver was added in the directory "proximity" of the iio subsystem
in absence of another directory named "distance".
There is also a new submenu "distance"
Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
---
drivers/iio/proximity/Kconfig | 15 ++
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/srf08.c | 345 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 361 insertions(+)
create mode 100644 drivers/iio/proximity/srf08.c
diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index ef4c73db5b53..7b10a137702b 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -46,3 +46,18 @@ config SX9500
module will be called sx9500.
endmenu
+
+menu "Distance sensors"
+
+config SRF08
+ tristate "Devantech SRF08 ultrasonic ranger sensor"
+ depends on I2C
+ help
+ Say Y here to build a driver for Devantech SRF08 ultrasonic
+ ranger sensor. This driver can be used to measure the distance
+ of objects.
+
+ To compile this driver as a module, choose M here: the
+ module will be called srf08.
+
+endmenu
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 9aadd9a8ee99..14d8fe4e5cae 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -6,3 +6,4 @@
obj-$(CONFIG_AS3935) += as3935.o
obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
obj-$(CONFIG_SX9500) += sx9500.o
+obj-$(CONFIG_SRF08) += srf08.o
diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
new file mode 100644
index 000000000000..a888a230b419
--- /dev/null
+++ b/drivers/iio/proximity/srf08.c
@@ -0,0 +1,345 @@
+/*
+ * srf08.c - Support for Devantech SRF08 ultrasonic ranger
+ *
+ * Copyright (c) 2016 Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * For details about the device see:
+ * http://www.robot-electronics.co.uk/htm/srf08tech.html
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/bitops.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+/* registers of SRF08 device */
+#define SRF08_WRITE_COMMAND 0x00 /* Command Register */
+#define SRF08_WRITE_MAX_GAIN 0x01 /* Max Gain Register: 0 .. 31 */
+#define SRF08_WRITE_RANGE 0x02 /* Range Register: 0 .. 255 */
+#define SRF08_READ_SW_REVISION 0x00 /* Software Revision */
+#define SRF08_READ_LIGHT 0x01 /* Light Sensor during last echo */
+#define SRF08_READ_ECHO_1_HIGH 0x02 /* range of first echo received */
+#define SRF08_READ_ECHO_1_LOW 0x03 /* range of first echo received */
+
+#define SRF08_CMD_RANGING_CM 0x51 /* Ranging Mode - Result in cm */
+
+struct srf08_data {
+ struct i2c_client *client;
+ int gain; /* max gain */
+ int range_mm; /* range in mm */
+ struct mutex lock;
+};
+
+static const int srf08_gain[] = {
+ 94, 97, 100, 103, 107, 110, 114, 118,
+ 123, 128, 133, 139, 145, 152, 159, 168,
+ 177, 187, 199, 212, 227, 245, 265, 288,
+ 317, 352, 395, 450, 524, 626, 777, 1025 };
+
+static int srf08_read_ranging(struct srf08_data *data)
+{
+ struct i2c_client *client = data->client;
+ char cmd[2];
+ int ret, i;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ SRF08_WRITE_COMMAND, SRF08_CMD_RANGING_CM);
+ if (ret < 0) {
+ dev_err(&client->dev, "write command - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ /*
+ * normally after 65 ms the device should have the read value
+ * we round it up to 100 ms
+ */
+ for (i = 0; i < 5; i++) {
+
+ ret = i2c_smbus_read_byte_data(data->client,
+ SRF08_READ_SW_REVISION);
+
+ /* check if a valid version number is read */
+ if ((ret < 255) && (ret > 0))
+ break;
+ msleep(20);
+ }
+
+ if ((ret >= 255) || (ret <= 0)) {
+ dev_err(&client->dev, "device not ready\n");
+ mutex_unlock(&data->lock);
+ return -EIO;
+ }
+
+ memset(cmd, 0, sizeof(cmd));
+
+ for (i = 0; i < 2; i++) {
+ ret = i2c_smbus_read_byte_data(data->client,
+ SRF08_READ_ECHO_1_HIGH + i);
+ cmd[i] = ret;
+ }
+
+ mutex_unlock(&data->lock);
+
+ return (cmd[0] << 8 | cmd[1]);
+}
+
+static int srf08_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int *val,
+ int *val2, long mask)
+{
+ struct srf08_data *data = iio_priv(indio_dev);
+ s32 ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (channel->type == IIO_DISTANCE) {
+ ret = srf08_read_ranging(data);
+ *val = ret;
+ } else {
+ break;
+ }
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ if (channel->type == IIO_DISTANCE) {
+ /* 1 LSB is 1 cm */
+ *val = 0;
+ *val2 = 10000;
+ } else {
+ break;
+ }
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ break;
+ }
+
+ return -EINVAL;
+}
+
+static ssize_t srf08_show_range_mm_available(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, len = 0;
+
+ for (i = 0; i < 256; i++)
+ len += sprintf(buf + len, "%d ", (i + 1) * 43);
+
+ len += sprintf(buf + len, "\n");
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(range_mm_available, S_IRUGO,
+ srf08_show_range_mm_available, NULL, 0);
+
+static ssize_t srf08_show_range_mm(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ int len;
+
+ len = sprintf(buf, "%d\n", data->range_mm);
+
+ return len;
+}
+
+static ssize_t srf08_write_range_mm(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ struct i2c_client *client = data->client;
+ int ret, val;
+ int regval = -1;
+ int mod;
+
+ ret = kstrtoint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ regval = val / 43 - 1;
+ mod = val % 43;
+
+ if (mod || (regval < 0) || (regval > 255))
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ SRF08_WRITE_RANGE, regval);
+ if (ret < 0) {
+ dev_err(&client->dev, "write_range - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ data->range_mm = val;
+
+ mutex_unlock(&data->lock);
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(range_mm, S_IRUGO | S_IWUSR,
+ srf08_show_range_mm, srf08_write_range_mm, 0);
+
+static ssize_t srf08_show_gain_available(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, len = 0;
+
+ for (i = 0; i < ARRAY_SIZE(srf08_gain); i++)
+ len += sprintf(buf + len, "%d ", srf08_gain[i]);
+
+ len += sprintf(buf + len, "\n");
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(gain_available, S_IRUGO,
+ srf08_show_gain_available, NULL, 0);
+
+static ssize_t srf08_show_gain(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ int len;
+
+ len = sprintf(buf, "%d\n", data->gain);
+
+ return len;
+}
+
+static ssize_t srf08_write_gain(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct srf08_data *data = iio_priv(indio_dev);
+ struct i2c_client *client = data->client;
+ int ret, val, i;
+ int regval = -1;
+
+ ret = kstrtoint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ARRAY_SIZE(srf08_gain); i++)
+ if (val == srf08_gain[i])
+ regval = i;
+
+ if (regval == -1)
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+
+ ret = i2c_smbus_write_byte_data(data->client,
+ SRF08_WRITE_MAX_GAIN, regval);
+ if (ret < 0) {
+ dev_err(&client->dev, "write_gain - err: %d\n", ret);
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+
+ data->gain = val;
+
+ mutex_unlock(&data->lock);
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR(gain, S_IRUGO | S_IWUSR,
+ srf08_show_gain, srf08_write_gain, 0);
+
+static struct attribute *srf08_attributes[] = {
+ &iio_dev_attr_range_mm.dev_attr.attr,
+ &iio_dev_attr_range_mm_available.dev_attr.attr,
+ &iio_dev_attr_gain.dev_attr.attr,
+ &iio_dev_attr_gain_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group srf08_attribute_group = {
+ .attrs = srf08_attributes,
+};
+
+static const struct iio_chan_spec srf08_channels[] = {
+ {
+ .type = IIO_DISTANCE,
+ .info_mask_separate =
+ BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+static const struct iio_info srf08_info = {
+ .read_raw = srf08_read_raw,
+ .attrs = &srf08_attribute_group,
+ .driver_module = THIS_MODULE,
+};
+
+static int srf08_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct iio_dev *indio_dev;
+ struct srf08_data *data;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA |
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
+ return -ENODEV;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ i2c_set_clientdata(client, indio_dev);
+ data->client = client;
+
+ /* set default values of device */
+ data->gain = 1025;
+ data->range_mm = 11008;
+
+ indio_dev->name = dev_name(&client->dev);
+ indio_dev->dev.parent = &client->dev;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &srf08_info;
+ indio_dev->channels = srf08_channels;
+ indio_dev->num_channels = ARRAY_SIZE(srf08_channels);
+
+ mutex_init(&data->lock);
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id srf08_id[] = {
+ { "srf08", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, srf08_id);
+
+static struct i2c_driver srf08_driver = {
+ .driver = {
+ .name = "srf08",
+ },
+ .probe = srf08_probe,
+ .id_table = srf08_id,
+};
+module_i2c_driver(srf08_driver);
+
+MODULE_AUTHOR("Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>");
+MODULE_DESCRIPTION("Devantech SRF08 ultrasonic ranger driver");
+MODULE_LICENSE("GPL");
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/2] iio: distance: srf08: add IIO driver for us ranger
From: Andreas Klinger @ 2017-01-10 15:26 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, linux-iio, linux-kernel, ktsai,
wsa, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
trivial, mranostay, linux-i2c, devicetree
Cc: ak
This patch series adds IIO driver support for srf08 ultrasonic ranger
devices.
The first patch add a trivial device tree binding for the device together
with a new vendor devantech.
The second patch is the IIO driver which in turn is using I2C to talk to
the device.
Documentation about the sensor can be found here:
http://www.robot-electronics.co.uk/htm/srf08tech.html
Andreas Klinger (2):
iio: distance: srf08: add trivial DT binding
iio: distance: srf08: add IIO driver for srf08
.../devicetree/bindings/i2c/trivial-devices.txt | 1 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/iio/proximity/Kconfig | 15 +
drivers/iio/proximity/Makefile | 1 +
drivers/iio/proximity/srf08.c | 345 +++++++++++++++++++++
5 files changed, 363 insertions(+)
create mode 100644 drivers/iio/proximity/srf08.c
--
2.1.4
^ permalink raw reply
* Re: [PATCH v5 5/6] i2c: designware-baytrail: Fix race when resetting the semaphore
From: Jarkko Nikula @ 2017-01-10 13:16 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede, Wolfram Sang, Len Brown
Cc: Mika Westerberg, Jani Nikula, Ville Syrjälä,
Takashi Iwai, russianneuromancer @ ya . ru, linux-i2c, intel-gfx
In-Reply-To: <1483351617.9552.192.camel@linux.intel.com>
On 01/02/2017 12:06 PM, Andy Shevchenko wrote:
> On Mon, 2017-01-02 at 10:55 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 02-01-17 10:36, Andy Shevchenko wrote:
>>> On Sun, 2017-01-01 at 21:15 +0100, Hans de Goede wrote:
>>>> Use iosf_mbi_modify instead of iosf_mbi_read + iosf_mbi_write so
>>>> that
>>>> we keep the iosf_mbi_lock locked during the read-modify-write done
>>>> to
>>>> reset the semaphore.
>>>>
>>>
>>> While patch itself looks good to me, I think it reduces a
>>> probability to
>>> race and doesn't eliminate an issue completely.
>>
>> All accesses through this register are done through the
>> iosf_mbi_ helpers, and the modify helper holds the
>> iosf_mbi_lock during the entire read-modify-wrtie cycle,
>> so I don't see how there can still be any issue left.
>
> Ah, seems you are right.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
^ permalink raw reply
* [PATCH] i2c: piix4: Avoid race conditions with IMC
From: Ricardo Ribalda Delgado @ 2017-01-10 12:16 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang, linux-i2c, linux-kernel
Cc: Ricardo Ribalda Delgado
On AMD's SB800 and upwards, the SMBus is shared with the Integrated
Micro Controller (IMC).
The platform provides a hardware semaphore to avoid race conditions
among them. (Check page 288 of the SB800-Series Southbridges Register
Reference Guide http://support.amd.com/TechDocs/45482.pdf)
Without this patch, many access to the SMBus end with an invalid
transaction or even with the bus stalled.
Credit-to: Alexandre Desnoyers <alex@qtec.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
drivers/i2c/busses/i2c-piix4.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index f0563f7ce01b..0286cfee6d36 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -585,9 +585,28 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
u8 command, int size, union i2c_smbus_data *data)
{
struct i2c_piix4_adapdata *adapdata = i2c_get_adapdata(adap);
+ unsigned short piix4_smba = adapdata->smba;
u8 smba_en_lo;
u8 port;
int retval;
+ int timeout = 0;
+ int smbslvcnt;
+
+ /* Request the SMBUS semaphore, avoid conflicts with the IMC */
+ smbslvcnt = inb_p(SMBSLVCNT);
+ while (++timeout < MAX_TIMEOUT) {
+ outb_p(smbslvcnt | 0x10, SMBSLVCNT);
+
+ /* Check the semaphore status */
+ smbslvcnt = inb_p(SMBSLVCNT);
+ if (smbslvcnt & 0x10)
+ break;
+
+ usleep_range(1000, 2000);
+ }
+ /* SMBus is still owned by the IMC, we give up */
+ if (timeout == MAX_TIMEOUT)
+ return -EBUSY;
mutex_lock(&piix4_mutex_sb800);
@@ -606,6 +625,9 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
mutex_unlock(&piix4_mutex_sb800);
+ /* Release the semaphore */
+ outb_p(smbslvcnt | 0x20, SMBSLVCNT);
+
return retval;
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v8 0/5] Add support for the STM32F4 I2C
From: Linus Walleij @ 2017-01-10 9:26 UTC (permalink / raw)
To: M'boumba Cedric Madianga
Cc: Wolfram Sang, Rob Herring, Maxime Coquelin, Alexandre TORGUE,
Patrice CHOTARD, Russell King,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Uwe Kleine-König
In-Reply-To: <1483607246-14771-1-git-send-email-cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Thu, Jan 5, 2017 at 10:07 AM, M'boumba Cedric Madianga
<cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This patchset adds support for the I2C controller embedded in STM32F4xx SoC.
> It enables I2C transfer in interrupt mode with Standard-mode and Fast-mode bus
> speed.
The whole series:
Reviewed-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Looks perfect from my point of view now.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for i2c-mux-irq-mask-en
From: Rob Herring @ 2017-01-10 5:36 UTC (permalink / raw)
To: Phil Reid; +Cc: peda, wsa, mark.rutland, linux-i2c, devicetree
In-Reply-To: <1483952576-5308-5-git-send-email-preid@electromag.com.au>
On Mon, Jan 09, 2017 at 05:02:55PM +0800, Phil Reid wrote:
> Unfortunately some hardware device will assert their irq line immediately
> on power on and provide no mechanism to mask the irq. As the i2c muxes
> provide no method to mask irq line this provides a work around by keeping
> the parent irq masked until enough device drivers have loaded to service
> all pending interrupts.
>
> For example the the ltc1760 assert its SMBALERT irq immediately on power
> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
> device is registered irq are enabled and fire continuously as the second
> device driver has not yet loaded. Setting this parameter to 0x3 while
> delay the irq being enabled until both devices are ready.
>
> Acked-by: Peter Rosin <peda@axentia.se>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
> Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 3 +++
> 1 file changed, 3 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2] i2c: do not enable fall back to Host Notify by default
From: Rob Herring @ 2017-01-09 17:56 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Wolfram Sang, Benjamin Tissoires, Pali Rohár,
Michał Kępień, Jean Delvare, Takashi Iwai,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <20170105045722.GA17958@dtor-ws>
On Wed, Jan 04, 2017 at 08:57:22PM -0800, Dmitry Torokhov wrote:
> Falling back unconditionally to HostNotify as primary client's interrupt
> breaks some drivers which alter their functionality depending on whether
> interrupt is present or not, so let's introduce a board flag telling I2C
> core explicitly if we want wired interrupt or HostNotify-based one:
> I2C_CLIENT_HOST_NOTIFY.
>
> For DT-based systems we introduce "host-notify" property that we convert
> to I2C_CLIENT_HOST_NOTIFY board flag.
>
> Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> v1->v2:
>
> - of_read_property_bool -> of_property_read_bool
> - did not change binding wording to avoit mentioning I2C core because we
> use the same wording (mentioning I2C core) for wired interrupts
>
> Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
Acked-by: Rob Herring <robh@kernel.org>
> drivers/i2c/i2c-core.c | 17 ++++++++---------
> include/linux/i2c.h | 1 +
> 3 files changed, 17 insertions(+), 9 deletions(-)
^ permalink raw reply
* [PATCH] drivers/i2c/i2c-dev: Fix kernel memory disclosure
From: Vlad Tsyrklevich @ 2017-01-09 15:53 UTC (permalink / raw)
To: wsa; +Cc: linux-i2c, Vlad Tsyrklevich
i2c_smbus_xfer() does not always fill an entire block, allowing
kernel stack memory disclosure through the temp variable. Clear
it before it's read to.
Signed-off-by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
---
drivers/i2c/i2c-dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 66f323f..6f638bb 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -331,7 +331,7 @@ static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
unsigned long arg)
{
struct i2c_smbus_ioctl_data data_arg;
- union i2c_smbus_data temp;
+ union i2c_smbus_data temp = {};
int datasize, res;
if (copy_from_user(&data_arg,
--
2.7.0
^ permalink raw reply related
* Re: [PATCH] drivers/i2c/i2c-dev: Fix kernel memory disclosure
From: Wolfram Sang @ 2017-01-09 14:48 UTC (permalink / raw)
To: Vlad Tsyrklevich; +Cc: linux-i2c
In-Reply-To: <CAH0z3hP9NRf87WNmtQjXKW7u5ZN=0rcwUWBJvHGFeSywRNvUOQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 544 bytes --]
On Mon, Jan 09, 2017 at 05:19:23PM +0700, Vlad Tsyrklevich wrote:
> I apologize for the long delay, this reply got lost in my inbox. I've
> added an updated patch below:
No problem, thanks for doing it. Looks good. Can you please resend as a
seperate patch (not embedded in the midst of a message?) Thank you!
> i2c_smbus_xfer() does not always fill an entire block, allowing
> kernel stack memory disclosure through the temp variable. Clear
> it before it's read to.
>
> Signed-off-by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Outlook Security Team Micorosof
From: Devaraj Veerasamy, Dr @ 2017-01-09 11:21 UTC (permalink / raw)
To: "\"NO-REPLY@WEBMAIL.NET""
MICROSOFT OUTLOOK anmälan
Din e-rutan konto behöver vara verifiera nu för oegentligheter finns i din e-box-konto eller kommer att blockera. Klicka här<https://mrswangjuan17.wixsite.com/webaccess2017> för att verifiera din e-postkonto och fil i ditt korrekta användarnamn och lösenord omedelbart
Outlook Security Team Micorosof
Tack.
Copyright © 2017 MIcrosoft OUtlook . Inc . All rights reserved.
^ permalink raw reply
* Re: [PATCH] drivers/i2c/i2c-dev: Fix kernel memory disclosure
From: Vlad Tsyrklevich @ 2017-01-09 10:19 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-i2c
In-Reply-To: <20161025094512.GF1597@katana>
I apologize for the long delay, this reply got lost in my inbox. I've
added an updated patch below:
i2c_smbus_xfer() does not always fill an entire block, allowing
kernel stack memory disclosure through the temp variable. Clear
it before it's read to.
Signed-off-by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
---
drivers/i2c/i2c-dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 66f323f..6f638bb 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -331,7 +331,7 @@ static noinline int i2cdev_ioctl_smbus(struct
i2c_client *client,
unsigned long arg)
{
struct i2c_smbus_ioctl_data data_arg;
- union i2c_smbus_data temp;
+ union i2c_smbus_data temp = {};
int datasize, res;
if (copy_from_user(&data_arg,
On Tue, Oct 25, 2016 at 4:45 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Tue, Oct 11, 2016 at 02:52:28PM +0200, Vlad Tsyrklevich wrote:
>> i2c_smbus_xfer() does not always fill an entire block, allowing
>> kernel stack memory disclosure through the temp variable. Clear
>> it before it's read to.
>>
>> Signed-off-by: Vlad Tsyrklevich <vlad@tsyrklevich.net>
>
> Yes, thanks. But what about clearing 'temp' when it is declared? This
> would be rock-solid for all future code paths.
>
>>
>> ---
>> drivers/i2c/i2c-dev.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
>> index 66f323f..62cb111 100644
>> --- a/drivers/i2c/i2c-dev.c
>> +++ b/drivers/i2c/i2c-dev.c
>> @@ -393,6 +393,8 @@ static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
>> (data_arg.read_write == I2C_SMBUS_WRITE)) {
>> if (copy_from_user(&temp, data_arg.data, datasize))
>> return -EFAULT;
>> + } else {
>> + memset(&temp, 0, datasize);
>> }
>> if (data_arg.size == I2C_SMBUS_I2C_BLOCK_BROKEN) {
>> /* Convert old I2C block commands to the new
>> --
>> 2.7.0
>>
^ permalink raw reply related
* Re: [PATCH v4 00/14] ARM: dts: r8a779x: use demuxer for I2C
From: Niklas Söderlund @ 2017-01-09 9:34 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Simon Horman, Wolfram Sang, linux-renesas-soc, linux-i2c
In-Reply-To: <20170104102550.GA1452@katana>
On 2017-01-04 11:25:50 +0100, Wolfram Sang wrote:
> Bump...
>
> > Yes, this is a bug in the rcar-vin driver which is addressed in the Gen3
> > patches. However I'm not sure those patches will make it to v4.10, not
> > much review from the V4L2 side yet (Geert and Sergei have had a few
> > comments so there will at lest be one more iteration).
>
> Seems your predictions were correct. I still get:
>
> * OOPS with renesas/dt-for-v4.11 and these patches on top
> * no OOPS with renesas-drivers/master and these patches on top
>
> So, it seems to me that we still should wait with the HDMI related
> patches of this series until the rcar-vin-gen3 branch is in -next.
> D'accord everyone?
I'm fine with waiting. But as I hinted in my previous email the review
on the rcar-vin-gen3 branch is slow, that is nothing have happened in
that thread since last we spoke of this...
If this is blocking or other create other issue for you please let me
know and I will do my best to extract the fix and post it separately
from Gen3 enablement.
--
Regards,
Niklas Söderlund
^ permalink raw reply
* Re: [PATCH v2 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs
From: Phil Reid @ 2017-01-09 9:04 UTC (permalink / raw)
To: Peter Rosin, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <c5b2eeef-4501-e866-4bb3-f02ada7d4604-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 9/01/2017 15:54, Peter Rosin wrote:
> On 2017-01-05 05:11, Phil Reid wrote:
>> Unfortunately some hardware device will assert their irq line immediately
>> on power on and provide no mechanism to mask the irq. As the i2c muxes
>> provide no method to mask irq line this provides a work around by keeping
>> the parent irq masked until enough device drivers have loaded to service
>> all pending interrupts.
>>
>> For example the the ltc1760 assert its SMBALERT irq immediately on power
>> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>> device is registered irq are enabled and fire continuously as the second
>> device driver has not yet loaded. Setting this parameter to 0x3 while
>> delay the irq being enabled until both devices are ready.
>>
>> Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>
> Ooops, too soon apparently, but the below nitpicks are not that important.
> Maybe Wolfram can fix it up instead of you sending a new version?
G'day Peter,
I've done a v3 with those changes. And added your ack to p1.
Regards
Phil
>
> Cheers,
> peda
>
>> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
>> ---
>> drivers/i2c/muxes/i2c-mux-pca954x.c | 22 +++++++++++++++++++++-
>> 1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> index 84fc767..581a75d 100644
>> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
>> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> @@ -75,6 +75,19 @@ struct chip_desc {
>> } muxtype;
>> };
>>
>> +/*
>> + * irq_mask_enable: Provides a mechanism to work around hardware that asserts
>> + * their irq immediately on power on. It allows the enabling of the irq to be
>
> double space: "the irq"
>
>> + * delayed until the corresponding bits in the the irq_mask are set thru
>> + * irq_unmask.
>> + * For example the the ltc1760 assert its SMBALERT irq immediately on power
>
> "the the"
>
>> + * on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>> + * device is registered irq are enabled and fire continuously as the second
>> + * device driver has not yet loaded. Setting this parameter to 0x3 while
>> + * delay the irq being enabled until both devices are ready.
>> + * This workaround will not work if two devices share an interrupt on the
>> + * same bus segment.
>> + */
>> struct pca954x {
>> const struct chip_desc *chip;
>>
>> @@ -84,6 +97,7 @@ struct pca954x {
>>
>> struct irq_domain *irq;
>> unsigned int irq_mask;
>> + unsigned int irq_mask_enable;
>> };
>>
>> /* Provide specs for the PCA954x types we know about */
>> @@ -270,9 +284,12 @@ static void pca954x_irq_unmask(struct irq_data *idata)
>> struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> unsigned int pos = idata->hwirq;
>>
>> - if (!data->irq_mask)
>> + if (!data->irq_mask_enable && !data->irq_mask)
>> enable_irq(data->client->irq);
>> data->irq_mask |= BIT(pos);
>> + if (data->irq_mask_enable &&
>> + (data->irq_mask & data->irq_mask) == data->irq_mask_enable)
>> + enable_irq(data->client->irq);
>> }
>>
>> static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
>> @@ -395,6 +412,9 @@ static int pca954x_probe(struct i2c_client *client,
>> idle_disconnect_dt = of_node &&
>> of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>>
>> + of_property_read_u32(of_node, "nxp,irq-mask-enable",
>> + &data->irq_mask_enable);
>> +
>> ret = pca954x_irq_setup(muxc);
>> if (ret)
>> goto fail_del_adapters;
>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 3/5] i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-09 9:02 UTC (permalink / raw)
To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1483952576-5308-1-git-send-email-preid@electromag.com.au>
Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
latency can potentially be reduced by reading the status register and
then only calling the registered isr on that bus segment.
As there is no irq masking on the mux irq are disabled until irq_unmask is
called at least once.
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 127 +++++++++++++++++++++++++++++++++++-
1 file changed, 125 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index bbf088e..84fc767 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -41,14 +41,19 @@
#include <linux/i2c.h>
#include <linux/i2c-mux.h>
#include <linux/i2c/pca954x.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#include <linux/pm.h>
#include <linux/slab.h>
#define PCA954X_MAX_NCHANS 8
+#define PCA954X_IRQ_OFFSET 4
+
enum pca_type {
pca_9540,
pca_9542,
@@ -63,6 +68,7 @@ enum pca_type {
struct chip_desc {
u8 nchans;
u8 enable; /* used for muxes only */
+ u8 has_irq;
enum muxtype {
pca954x_ismux = 0,
pca954x_isswi
@@ -75,6 +81,9 @@ struct pca954x {
u8 last_chan; /* last register value */
u8 deselect;
struct i2c_client *client;
+
+ struct irq_domain *irq;
+ unsigned int irq_mask;
};
/* Provide specs for the PCA954x types we know about */
@@ -87,19 +96,23 @@ struct pca954x {
[pca_9542] = {
.nchans = 2,
.enable = 0x4,
+ .has_irq = 1,
.muxtype = pca954x_ismux,
},
[pca_9543] = {
.nchans = 2,
+ .has_irq = 1,
.muxtype = pca954x_isswi,
},
[pca_9544] = {
.nchans = 4,
.enable = 0x4,
+ .has_irq = 1,
.muxtype = pca954x_ismux,
},
[pca_9545] = {
.nchans = 4,
+ .has_irq = 1,
.muxtype = pca954x_isswi,
},
[pca_9547] = {
@@ -222,6 +235,102 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
return pca954x_reg_write(muxc->parent, client, data->last_chan);
}
+static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
+{
+ struct pca954x *data = dev_id;
+ unsigned int child_irq;
+ int ret, i, handled;
+
+ ret = i2c_smbus_read_byte(data->client);
+ if (ret < 0)
+ return IRQ_NONE;
+
+ for (i = 0; i < data->chip->nchans; i++) {
+ if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
+ child_irq = irq_linear_revmap(data->irq, i);
+ handle_nested_irq(child_irq);
+ handled++;
+ }
+ }
+ return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static void pca954x_irq_mask(struct irq_data *idata)
+{
+ struct pca954x *data = irq_data_get_irq_chip_data(idata);
+ unsigned int pos = idata->hwirq;
+
+ data->irq_mask &= ~BIT(pos);
+ if (!data->irq_mask)
+ disable_irq(data->client->irq);
+}
+
+static void pca954x_irq_unmask(struct irq_data *idata)
+{
+ struct pca954x *data = irq_data_get_irq_chip_data(idata);
+ unsigned int pos = idata->hwirq;
+
+ if (!data->irq_mask)
+ enable_irq(data->client->irq);
+ data->irq_mask |= BIT(pos);
+}
+
+static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
+{
+ if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
+ return -EINVAL;
+ return 0;
+}
+
+static struct irq_chip pca954x_irq_chip = {
+ .name = "i2c-mux-pca954x",
+ .irq_mask = pca954x_irq_mask,
+ .irq_unmask = pca954x_irq_unmask,
+ .irq_set_type = pca954x_irq_set_type,
+};
+
+static int pca954x_irq_setup(struct i2c_mux_core *muxc)
+{
+ struct pca954x *data = i2c_mux_priv(muxc);
+ struct i2c_client *client = data->client;
+ int c, err, irq;
+
+ if (!data->chip->has_irq || client->irq <= 0)
+ return 0;
+
+ data->irq = irq_domain_add_linear(client->dev.of_node,
+ data->chip->nchans,
+ &irq_domain_simple_ops, data);
+ if (!data->irq)
+ return -ENODEV;
+
+ for (c = 0; c < data->chip->nchans; c++) {
+ irq = irq_create_mapping(data->irq, c);
+ irq_set_chip_data(irq, data);
+ irq_set_chip_and_handler(irq, &pca954x_irq_chip,
+ handle_simple_irq);
+ }
+
+ err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
+ pca954x_irq_handler,
+ IRQF_ONESHOT | IRQF_SHARED,
+ "pca954x", data);
+ if (err)
+ goto err_req_irq;
+
+ disable_irq(data->client->irq);
+
+ return 0;
+err_req_irq:
+ for (c = 0; c < data->chip->nchans; c++) {
+ irq = irq_find_mapping(data->irq, c);
+ irq_dispose_mapping(irq);
+ }
+ irq_domain_remove(data->irq);
+
+ return err;
+}
+
/*
* I2C init/probing/exit functions
*/
@@ -286,6 +395,10 @@ static int pca954x_probe(struct i2c_client *client,
idle_disconnect_dt = of_node &&
of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
+ ret = pca954x_irq_setup(muxc);
+ if (ret)
+ goto fail_del_adapters;
+
/* Now create an adapter for each channel */
for (num = 0; num < data->chip->nchans; num++) {
bool idle_disconnect_pd = false;
@@ -311,7 +424,7 @@ static int pca954x_probe(struct i2c_client *client,
dev_err(&client->dev,
"failed to register multiplexed adapter"
" %d as bus %d\n", num, force);
- goto virt_reg_failed;
+ goto fail_del_adapters;
}
}
@@ -322,7 +435,7 @@ static int pca954x_probe(struct i2c_client *client,
return 0;
-virt_reg_failed:
+fail_del_adapters:
i2c_mux_del_adapters(muxc);
return ret;
}
@@ -330,6 +443,16 @@ static int pca954x_probe(struct i2c_client *client,
static int pca954x_remove(struct i2c_client *client)
{
struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+ struct pca954x *data = i2c_mux_priv(muxc);
+ int c, irq;
+
+ if (data->irq) {
+ for (c = 0; c < data->chip->nchans; c++) {
+ irq = irq_find_mapping(data->irq, c);
+ irq_dispose_mapping(irq);
+ }
+ irq_domain_remove(data->irq);
+ }
i2c_mux_del_adapters(muxc);
return 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 2/5] dt: bindings: i2c-mux-pca954x: Add documentation for interrupt controller
From: Phil Reid @ 2017-01-09 9:02 UTC (permalink / raw)
To: peda, wsa, robh+dt, mark.rutland, preid, linux-i2c, devicetree
In-Reply-To: <1483952576-5308-1-git-send-email-preid@electromag.com.au>
Various muxes can aggregate multiple irq lines and provide a control
register to determine the active line. Add bindings for interrupt
controller support.
Acked-by: Peter Rosin <peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index cf53d5f..aa09704 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -19,7 +19,14 @@ Optional Properties:
- i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
children in idle state. This is necessary for example, if there are several
multiplexers on the bus and the devices behind them use same I2C addresses.
-
+ - interrupt-parent: Phandle for the interrupt controller that services
+ interrupts for this device.
+ - interrupts: Interrupt mapping for IRQ.
+ - interrupt-controller: Marks the device node as an interrupt controller.
+ - #interrupt-cells : Should be two.
+ - first cell is the pin number
+ - second cell is used to specify flags.
+ See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
Example:
@@ -29,6 +36,11 @@ Example:
#size-cells = <0>;
reg = <0x74>;
+ interrupt-parent = <&ipic>;
+ interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
i2c@2 {
#address-cells = <1>;
#size-cells = <0>;
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox