Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 02/10] iio: adc: Add stm32 support
From: Jonathan Cameron @ 2016-11-05 15:44 UTC (permalink / raw)
  To: Fabrice Gasnier, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-I+IVW8TIWO2tmTQ+vhA3Yw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
	knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	Benjamin GAIGNARD
In-Reply-To: <4687bde5-1e1e-83bc-0aca-3224399f0fa2-qxv4g6HH51o@public.gmane.org>

On 03/11/16 08:20, Fabrice Gasnier wrote:
> On 10/30/2016 04:27 PM, Jonathan Cameron wrote:
>> On 25/10/16 17:25, Fabrice Gasnier wrote:
>>> This patch adds support for STMicroelectronics STM32 MCU's analog to
>>> digital converter.
>>>
>>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
>> Hi Fabrice,
>>
>> Sometimes I hate SoC ADCs.  For some reason the hardware designers seem to
>> try and throw everything and the kitchen sink at them.  Discontinuous mode
>> as an example in this device.  Not seen that particular piece of fun before
>> and glad to see you haven't 'yet' tried to support it!
>>
>> Anyhow, the complexity of the hardware leads to an initially complex driver.
>> My first thought it that this would be easier to follow / review if we
>> built it up in smaller steps.   Perhaps ditch the injected channel support
>> entirely in the first instance.  I also wonder if you don't need to support
>> that whole thing (injected sampling) as another iio device entirely using the
>> same channels.  That's kind of what it is from a data flow point of view
>> (we've had arbitary sequencers before with priorities - don't think anyone
>> ever decided the pain was worth supporting the complexity, but right answer
>> has always been multiple IIO devices).
> Hi Jonathan,
> 
> First, many thanks for your review. I agree with you, most reasonable approach is to remove some
> complexity to ease the review. Regarding injected support, basically, bellow approach is to use
> separate IIO devices for regular and injected. But, I'll remove this, at least for now, in next patch set.
> 
>> You also have at least one layer of abstraction in here that serves no
>> current purpose.  Please clear that out for now. It'll make the code
>> shorter and easier to follow.  If/when other parts are introduced then
>> is the time to do that transistion to having the abstraction.
> 
> From your suggestion, this may end-up in a single driver file in drivers/iio.
> I think I'll try to keep simple routines like start, stop, conf_scan and so on, but
> remove indirection routines from stm32-adc.h file (e.g. stm32_adc_ops).
> Is it in line with your suggestions ?
Sure, some of those will want to be in their own functions so it sounds
about right.
> 
>>
>> My first thought on the double / tripple adc handling is that you'd be better
>> off handling them as 3 separate devices then doing some 'unusual' trigger
>> handling to support the weird sequencing.  Guessing you thought about that?
>> If so could you lay out your reasoning for the single driver instance approach.
>> I'm not arguing against it btw, merely want to understand your reasoning!
> 
> I mainly came up with a single driver instance approach because there are basically
> 3 identical ADC instances 'mapped' in a single IP with few common resources.
> I usually see mfd are more heterogeneous and declare cells for various subsystem drivers.
> But I can try to move to mfd as you're suggesting.
> I just hope this will not bring more complexity.
If anything I suspect it'll end up simpler to read (be it a tiny bit longer
in terms of lines of code).
> 
>>
>> It would be tricky given one set of channels are selectable over 3 devices
>> and there are constraints to enforce (not sampling same channel on two ADCs
>> at the same time) but not impossible...  Perhaps what you have here is
>> indeed simpler!
>>
>> Whilst it's been a nasty job to review, I'm guessing writing it was
>> much worse ;)  Pretty good starting point though might take a little while
>> to pin down the remaining questions on how best to handle this particular
>> monster.
> My apologies... I hope you didn't had much of a headache :-) by reading me.
> More questions bellow.
> 
>> Jonathan
>>> ---
>>>   drivers/iio/adc/Kconfig             |   2 +
>>>   drivers/iio/adc/Makefile            |   1 +
>>>   drivers/iio/adc/stm32/Kconfig       |  34 ++
>>>   drivers/iio/adc/stm32/Makefile      |   4 +
>>>   drivers/iio/adc/stm32/stm32-adc.c   | 999 ++++++++++++++++++++++++++++++++++++
>>>   drivers/iio/adc/stm32/stm32-adc.h   | 442 ++++++++++++++++
>>>   drivers/iio/adc/stm32/stm32f4-adc.c | 574 +++++++++++++++++++++
>>>   7 files changed, 2056 insertions(+)
>>>   create mode 100644 drivers/iio/adc/stm32/Kconfig
>>>   create mode 100644 drivers/iio/adc/stm32/Makefile
>>>   create mode 100644 drivers/iio/adc/stm32/stm32-adc.c
>>>   create mode 100644 drivers/iio/adc/stm32/stm32-adc.h
>>>   create mode 100644 drivers/iio/adc/stm32/stm32f4-adc.c
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 7edcf32..5c96a55 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -583,4 +583,6 @@ config XILINX_XADC
>>>         The driver can also be build as a module. If so, the module will be called
>>>         xilinx-xadc.
>>>   +source "drivers/iio/adc/stm32/Kconfig"
>>> +
>>>   endmenu
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index 7a40c04..a9dbf3a 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -13,6 +13,7 @@ obj-$(CONFIG_AD7791) += ad7791.o
>>>   obj-$(CONFIG_AD7793) += ad7793.o
>>>   obj-$(CONFIG_AD7887) += ad7887.o
>>>   obj-$(CONFIG_AD799X) += ad799x.o
>>> +obj-$(CONFIG_ARCH_STM32) += stm32/
>>>   obj-$(CONFIG_AT91_ADC) += at91_adc.o
>>>   obj-$(CONFIG_AT91_SAMA5D2_ADC) += at91-sama5d2_adc.o
>>>   obj-$(CONFIG_AXP288_ADC) += axp288_adc.o
>>> diff --git a/drivers/iio/adc/stm32/Kconfig b/drivers/iio/adc/stm32/Kconfig
>>> new file mode 100644
>>> index 0000000..245d037
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/Kconfig
>>> @@ -0,0 +1,34 @@
>>> +#
>>> +# STM32 familly ADC drivers
>>> +#
>>> +
>>> +config STM32_ADC
>>> +    tristate
>>> +    select REGULATOR
>>> +    select REGULATOR_FIXED_VOLTAGE
>>> +    select IIO_BUFFER
>>> +    select IIO_TRIGGERED_BUFFER
>>> +    help
>>> +      Say yes here to build the driver for the STMicroelectronics
>>> +      STM32 analog-to-digital converter (ADC).
>>> +
>>> +      This driver can also be built as a module.  If so, the module
>>> +      will be called stm32-adc.
>>> +
>>> +config STM32F4_ADC
>>> +    tristate "STMicroelectronics STM32F4 adc"
>>> +    depends on ARCH_STM32 || COMPILE_TEST
>>> +    depends on OF
>>> +    select STM32_ADC
>>> +    help
>>> +      Say yes here to build support for STMicroelectronics stm32f4 Analog
>>> +      to Digital Converter (ADC).
>>> +
>>> +      This driver can also be built as a module.  If so, the module
>>> +      will be called stm32f4-adc.
>>> +
>>> +config STM32_ADC_DEBUG
>>> +    bool "Enable debug for stm32 ADC drivers"
>>> +    depends on STM32_ADC
>>> +    help
>>> +      Say "yes" to enable debug messages, on stm32 ADC drivers.
>>> diff --git a/drivers/iio/adc/stm32/Makefile b/drivers/iio/adc/stm32/Makefile
>>> new file mode 100644
>>> index 0000000..83e8154
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/Makefile
>>> @@ -0,0 +1,4 @@
>>> +# Core
>>> +subdir-ccflags-$(CONFIG_STM32_ADC_DEBUG) := -DDEBUG
>>> +obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>>> +obj-$(CONFIG_STM32F4_ADC) += stm32f4-adc.o
>>> diff --git a/drivers/iio/adc/stm32/stm32-adc.c b/drivers/iio/adc/stm32/stm32-adc.c
>>> new file mode 100644
>>> index 0000000..1e0850d
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/stm32-adc.c
>>> @@ -0,0 +1,999 @@
> [snip]
> 
>>> +
>>> +static int stm32_adc_conf_scan(struct iio_dev *indio_dev,
>>> +                   const unsigned long *scan_mask)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +    int ret;
>>> +
>>> +    ret = stm32_adc_clk_sel(adc);
>>> +    if (ret) {
>>> +        dev_err(&indio_dev->dev, "Clock sel failed\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    ret = stm32_adc_enable(adc);
>>> +    if (ret) {
>>> +        dev_err(&indio_dev->dev, "Failed to enable adc\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    ret = stm32_adc_conf_scan_seq(indio_dev, scan_mask);
>>> +    if (ret) {
>>> +        dev_err(&indio_dev->dev, "Failed to configure sequence\n");
>>> +        goto err_dis;
>>> +    }
>> It's horrible but to end up in the 'obvious' state I'd disable the adc
>> again assuming that doesn't kill the stuff that is configured.
> I'll check this and try to come up with something.
> 
>>> +
>>> +    return 0;
>>> +
>>> +err_dis:
>>> +    stm32_adc_disable(adc);
>>> +
>>> +    return ret;
>>> +}
>>> +
> [snip]
>>> +/**
>>> + * stm32_adc_single_conv() - perform a single conversion
>>> + * @indio_dev: IIO device
>>> + * @chan: IIO channel
>>> + * @result: conversion result
>>> + *
>>> + * The function performs a single conversion on a given channel, by
>>> + * by:
>>> + * - creating scan mask with only one channel
>>> + * - using SW trigger
>>> + * - then start single conv
>>> + */
>>> +static int stm32_adc_single_conv(struct iio_dev *indio_dev,
>>> +                 const struct iio_chan_spec *chan,
>>> +                 int *val)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +    unsigned long *scan_mask;
>>> +    long timeout;
>>> +    u16 result;
>>> +    int ret;
>>> +
>>> +    scan_mask = kcalloc(BITS_TO_LONGS(indio_dev->masklength), sizeof(long),
>>> +                GFP_KERNEL);
>> This is known maximum length... I'd just avoid the complexity of allocating
>> it like this - a comment would do the job to say it is the right length.
> Do you suggest to use a predefined variable (like unsigned long scan_mask) directly ?
> And add a more basic test on 'masklength', to be sure ?
A test would make sense as would prevent any problems if this driver
is extended to support much larger devices in future.
> 
>>> +    if (!scan_mask)
>>> +        return -ENOMEM;
>>> +
>>> +    set_bit(chan->scan_index, scan_mask);
>>> +
>>> +    reinit_completion(&adc->completion);
>>> +
>>> +    adc->bufi = 0;
>>> +    adc->num_conv = 1;
>>> +    adc->buffer = &result;
>>> +
>>> +    ret = stm32_adc_conf_scan(indio_dev, scan_mask);
>>> +    if (ret)
>>> +        goto free;
>>> +
>>> +    /* No HW trigger: conversion can be launched in SW */
>>> +    ret = stm32_adc_set_trig(indio_dev, NULL);
>> Put it back again afterwards?  Otherwise some nasty race conditions look
>> likely to me.. (userspace sets trigger and is about to enable the buffer
>> when along comes this code and changes it underneath).
> I'll fix this.
> 
>>> +    if (ret) {
>>> +        dev_err(&indio_dev->dev, "Can't set SW trigger\n");
>>> +        goto adc_disable;
>>> +    }
>>> +
>>> +    stm32_adc_conv_irq_enable(adc);
>>> +
>>> +    ret = stm32_adc_start_conv(adc);
>>> +    if (ret) {
>>> +        dev_err(&indio_dev->dev, "Failed to start single conv\n");
>>> +        goto irq_disable;
>>> +    }
>>> +
>>> +    timeout = wait_for_completion_interruptible_timeout(
>>> +                    &adc->completion, STM32_ADC_TIMEOUT);
>>> +    if (timeout == 0) {
>>> +        dev_warn(&indio_dev->dev, "Conversion timed out!\n");
>>> +        ret = -ETIMEDOUT;
>>> +    } else if (timeout < 0) {
>>> +        dev_warn(&indio_dev->dev, "Interrupted conversion!\n");
>>> +        ret = -EINTR;
>>> +    } else {
>>> +        *val = result & STM32_RESULT_MASK;
>>> +        ret = IIO_VAL_INT;
>>> +    }
>>> +
>>> +    if (stm32_adc_stop_conv(adc))
>>> +        dev_err(&indio_dev->dev, "stop failed\n");
>>> +
>>> +irq_disable:
>>> +    stm32_adc_conv_irq_disable(adc);
>>> +
>>> +adc_disable:
>>> +    stm32_adc_disable(adc);
>>> +
>>> +free:
>>> +    kfree(scan_mask);
>>> +    adc->buffer = NULL;
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int stm32_adc_read_raw(struct iio_dev *indio_dev,
>>> +                  struct iio_chan_spec const *chan,
>>> +                  int *val, int *val2, long mask)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +    int ret = -EINVAL;
>>> +
>>> +    switch (mask) {
>>> +    case IIO_CHAN_INFO_RAW:
>>> +        ret = iio_device_claim_direct_mode(indio_dev);
>>> +        if (ret)
>>> +            return ret;
>>> +        if (chan->type == IIO_VOLTAGE)
>>> +            ret = stm32_adc_single_conv(indio_dev, chan, val);
>>> +        iio_device_release_direct_mode(indio_dev);
>>> +        break;
>>> +    case IIO_CHAN_INFO_SCALE:
>>> +        *val = adc->common->vref_mv;
>>> +        *val2 = chan->scan_type.realbits;
>>> +        ret = IIO_VAL_FRACTIONAL_LOG2;
>>> +        break;
>>> +    default:
>>> +        break;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +/**
>>> + * stm32_adc_isr() - Treat interrupt for one ADC instance within ADC block
>> As this is kernel doc please document the parameter as well.  Otherwise
>> we'll get a pile of warnings!
> Sure.
>>> + */
>>> +static irqreturn_t stm32_adc_isr(struct stm32_adc *adc)
>>> +{
>>> +    struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>>> +    const struct stm32_adc_reginfo *reginfo =
>>> +        adc->common->data->adc_reginfo;
>>> +    u32 mask, clr_mask, status = stm32_adc_readl(adc, reginfo->isr);
>>> +
>>> +    if (adc->injected) {
>>> +        mask = reginfo->jeoc;
>>> +        clr_mask = mask;
>>> +    } else {
>>> +        mask = reginfo->eoc;
>>> +        /* don't clear 'eoc' as it is cleared when reading 'dr' */
>>> +        clr_mask = 0;
>>> +    }
>>> +
>>> +    /* clear irq */
>>> +    stm32_adc_writel(adc, reginfo->isr, status & ~clr_mask);
>> Want to do this in the non injected case? it's a noop isn't it?
> I'll rework this.
> 
>>
>>> +    status &= mask;
>>> +
>>> +    /* Regular data */
>>> +    if (status & reginfo->eoc) {
>> Hmm.. this is a little bit of 'missuse' of the standard trigger architecture
>> but as long as it's restricted to just this device I don't suppose we need
>> to care.  Only reason we need it is to provide control of 'which' hardware
>> trigger is being used.
>>
>> Guessing the DMA will almost always be turned on and will make this oddity
>> effectively disappear.
> I'm not sure to understand your remark. Above test checks end of conversion status flag.
> Or do you talk about bellow lines ? Can you please clarify ?
Took me  a while to figure this out. (i.e. I wasn't sure what I meant either!)

This setup corresponds (more or less) to having an external trigger fire
off a software based sequencer.  So we'd expect the 'loop' element of this
to run in the buffer handler rather than the trigger handler.  In theory
that would allow other triggers to be used as well as the ones supported
in hardware.

Here that is somewhat of a pain however.  If there weren't multiple triggers
to select between I'd just suggest dropping the trigger interface entirely
(it's optional) but then we'd have to do something custom to select
which of the device supplied triggers to use.

Hence probably best plan is leave it as it is.  Sometimes hardware
just doesn't fit the conceptual model we have for it!
> 
>>> +        adc->buffer[adc->bufi] = stm32_adc_readl(adc, reginfo->dr);
>>> +        if (iio_buffer_enabled(indio_dev)) {
>>> +            adc->bufi++;
>>> +            if (adc->bufi >= adc->num_conv) {
>>> +                stm32_adc_conv_irq_disable(adc);
>>> +                iio_trigger_poll(indio_dev->trig);
>>> +            }
>>> +        } else {
>>> +            complete(&adc->completion);
>>> +        }
>>> +    }
>>> +
>>> +    /* Injected data */
>>> +    if (status & reginfo->jeoc) {
>>> +        int i;
>>> +
>>> +        for (i = 0; i < adc->num_conv; i++) {
>>> +            adc->buffer[i] = stm32_adc_readl(adc, reginfo->jdr[i]);
>>> +            adc->bufi++;
>>> +        }
>>> +
>>> +        if (iio_buffer_enabled(indio_dev)) {
>>> +            stm32_adc_conv_irq_disable(adc);
>>> +            iio_trigger_poll(indio_dev->trig);
>>> +        } else {
>>> +            complete(&adc->completion);
>>> +        }
>>> +    }
>>> +
>>> +    /*
>>> +     * In case end of conversion flags have been handled, this has been
>>> +     * handled for this ADC instance
>>> +     */
>>> +    if (status)
>>> +        return IRQ_HANDLED;
>>> +
>>> +    /* This adc instance didn't trigger this interrupt */
>>> +    return IRQ_NONE;
>>> +}
>>> +
>>> +/**
>>> + * stm32_adc_common_isr() - Common isr for the whole ADC block
>>> + *
>>> + * There is one IRQ for all ADCs in ADC block, check all instances.
>>> + */
>>> +static irqreturn_t stm32_adc_common_isr(int irq, void *data)
>>> +{
>>> +    struct stm32_adc_common *common = data;
>>> +    irqreturn_t ret = IRQ_NONE;
>>> +    struct stm32_adc *adc;
>>> +
>>> +    list_for_each_entry(adc, &common->adc_list, adc_list)
>>> +        ret |= stm32_adc_isr(adc);
>> Hmm.. ret |= is rather fragile.  Preferable to make the handling of NONE
>> vs IRQ_HANDLED explicit.
>>
>> If you were to split the driver up as I suggested might make sense above,
>> then this would be done with an irq chip in a top level device (effectively
>> a very simple mfd).
> I'll look into it.
> 
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +/**
>>> + * stm32_adc_validate_trigger() - validate trigger for stm32 adc
>>> + * @indio_dev: IIO device
>>> + * @trig: new trigger
>>> + *
>>> + * Returns: 0 if trig matches one of the triggers registered by stm32 adc
>>> + * driver, -EINVAL otherwise.
>>> + */
>>> +static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
>>> +                      struct iio_trigger *trig)
>>> +{
>>> +    return stm32_adc_get_trig_index(indio_dev, trig) < 0 ? -EINVAL : 0;
>>> +}
>>> +
>>> +static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
>>> +                      const unsigned long *scan_mask)
>> I'm glad you kept this relatively simple compared to some of the
>> 'fun' the hardware is capable of. Very wise!
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +    int ret;
>>> +    u32 bit;
>>> +
>>> +    adc->num_conv = 0;
>>> +    for_each_set_bit(bit, scan_mask, indio_dev->masklength)
>>> +        adc->num_conv++;
>>> +
>>> +    ret = stm32_adc_conf_scan(indio_dev, scan_mask);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
>>> +                  const struct of_phandle_args *iiospec)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i < indio_dev->num_channels; i++)
>>> +        if (indio_dev->channels[i].channel == iiospec->args[0])
>>> +            return i;
>>> +
>>> +    return -EINVAL;
>>> +}
>>> +
>>> +/**
>>> + * stm32_adc_debugfs_reg_access - read or write register value
>>> + *
>>> + * To read a value from an ADC register:
>>> + *   echo [ADC reg offset] > direct_reg_access
>>> + *   cat direct_reg_access
>>> + *
>>> + * To write a value in a ADC register:
>>> + *   echo [ADC_reg_offset] [value] > direct_reg_access
>>> + */
>>> +static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>>> +                    unsigned reg, unsigned writeval,
>>> +                    unsigned *readval)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +
>>> +    if (!readval)
>>> +        stm32_adc_writel(adc, reg, writeval);
>>> +    else
>>> +        *readval = stm32_adc_readl(adc, reg);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct iio_info stm32_adc_iio_info = {
>>> +    .read_raw = stm32_adc_read_raw,
>>> +    .validate_trigger = stm32_adc_validate_trigger,
>>> +    .update_scan_mode = stm32_adc_update_scan_mode,
>>> +    .debugfs_reg_access = stm32_adc_debugfs_reg_access,
>>> +    .of_xlate = stm32_adc_of_xlate,
>>> +    .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static int stm32_adc_buffer_postdisable(struct iio_dev *indio_dev)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +
>>> +    stm32_adc_disable(adc);
>> This is a surprise as postdisbale should balance preenable...
>> Ah, you have update scan mode enabling the adc.  If you can balance it
>> better by moving that to preenable please do as it is more 'obviously' correct.
> I'll try to rework this.
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
>>> +    .postenable = &iio_triggered_buffer_postenable,
>>> +    .predisable = &iio_triggered_buffer_predisable,
>>> +    .postdisable = &stm32_adc_buffer_postdisable,
>>> +};
>>> +
>>> +static int stm32_adc_validate_device(struct iio_trigger *trig,
>>> +                     struct iio_dev *indio_dev)
>>> +{
>>> +    struct iio_dev *indio = iio_trigger_get_drvdata(trig);
>>> +
>>> +    return indio != indio_dev ? -EINVAL : 0;
>>> +}
>>> +
>>> +static int stm32_adc_set_trigger_state(struct iio_trigger *trig,
>>> +                       bool state)
>>> +{
>>> +    struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +    int ret;
>>> +
>>> +    if (state) {
>>> +        /* Reset adc buffer index */
>>> +        adc->bufi = 0;
>>> +
>>> +        /* Allocate adc buffer */
>>> +        adc->buffer = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
>> I'd be more cynical.  It's not that big a memory allocation at worst.
>> Just put a big enough buffer in your adc structure and don't bother doing
>> it dynamically.
>>
>> If you didn't want to do it, it should be in the preenable callback rather
>> than the trigger state one (for semantic reasons rather than because it's a
>> bug)
> I'll fix this.
>>> +        if (!adc->buffer)
>>> +            return -ENOMEM;
>>> +
>>> +        ret = stm32_adc_set_trig(indio_dev, trig);
>>> +        if (ret) {
>>> +            dev_err(&indio_dev->dev, "Can't set trigger\n");
>>> +            goto err_buffer_free;
>>> +        }
>>> +
>>> +        stm32_adc_conv_irq_enable(adc);
>>> +
>>> +        ret = stm32_adc_start_conv(adc);
>>> +        if (ret) {
>>> +            dev_err(&indio_dev->dev, "Failed to start\n");
>>> +            goto err_irq_trig_disable;
>>> +        }
>>> +    } else {
>>> +        ret = stm32_adc_stop_conv(adc);
>>> +        if (ret < 0) {
>>> +            dev_err(&indio_dev->dev, "Failed to stop\n");
>>> +            return ret;
>>> +        }
>>> +
>>> +        stm32_adc_conv_irq_disable(adc);
>>> +
>>> +        ret = stm32_adc_set_trig(indio_dev, NULL);
>>> +        if (ret)
>>> +            dev_warn(&indio_dev->dev, "Can't clear trigger\n");
>>> +
>>> +        kfree(adc->buffer);
>>> +        adc->buffer = NULL;
>>> +    }
>>> +
>>> +    return 0;
>>> +
>>> +err_irq_trig_disable:
>>> +    stm32_adc_conv_irq_disable(adc);
>>> +    stm32_adc_set_trig(indio_dev, NULL);
>>> +
>>> +err_buffer_free:
>>> +    kfree(adc->buffer);
>>> +    adc->buffer = NULL;
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static const struct iio_trigger_ops stm32_adc_trigger_ops = {
>>> +    .owner = THIS_MODULE,
>>> +    .validate_device = stm32_adc_validate_device,
>>> +    .set_trigger_state = stm32_adc_set_trigger_state,
>>> +};
>>> +
>>> +static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
>>> +{
>>> +    struct iio_poll_func *pf = p;
>>> +    struct iio_dev *indio_dev = pf->indio_dev;
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +
>>> +    dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
>>> +
>>> +    /* reset buffer index */
>>> +    adc->bufi = 0;
>>> +    iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
>>> +                       pf->timestamp);
>>> +
>>> +    iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> +    /* re-enable eoc irq */
>>> +    stm32_adc_conv_irq_enable(adc);
>>> +
>>> +    return IRQ_HANDLED;
>>> +}
>>> +
>>> +static void stm32_adc_trig_unregister(struct iio_dev *indio_dev)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +    struct iio_trigger *trig, *_t;
>>> +
>>> +    list_for_each_entry_safe(trig, _t, &adc->extrig_list, alloc_list) {
>>> +        iio_trigger_unregister(trig);
>>> +        list_del(&trig->alloc_list);
>>> +    }
>>> +}
>>> +
>> I'd like a bit of documentation on this and a few of the other more
>> complex functions.  Here it wasn't immediately obvious to me that it
>> was registering a large set of triggers.  Also, silly question but
>> do you have any means of controlling the various timer setups from userspace?
> Sorry about this, I'll try to comment about trigger list to make it more obvious.
> There is no mean to setup timers via userspace, yet...
> I can remove them from the list for now, until this is supported.
> BTW I have some questions on trigger...
> 
>>
>> There have been numerous discussions over the years on having a generic
>> timer subsystem, but if anything got written it passed me by. I have a couple
>> of boards where it would be handy but never had the time to do more than
>> talk about it ;)
> 
> This is interesting... I'd be glad to hear more about it. Can you
> point some discussions if you have it in mind?
Err. Not sure I'll actually find an email thread on this.  I can find
a reference to earlier discussions on it in the original IIO submission
over 8 years ago...  So it was before we had an IIO list which means
it must have been on lkml.

As far as I know, no one ever took this futher though...
> 
> In this driver, validate_trigger routine enforces that only triggers
> allocated for current indio_dev can be used. What if all timer
> triggers are put in a separate driver ? (e.g. like hrtimer in
> drivers/iio/trigger/) ? Purpose would be to tune 'sampling_frequency'
> and so on, on similar model, and have it configured basically when
> using it (e.g. cat trigger/name>trigger/current_trigger.).
I think we'd be closer to having a timer subsystem offer some 
'services' to the drivers and then have your driver make use of those.
As the timer is hard wired to the actual adc timing here (rather than
a signal passing through kernel space like the high resolution timer
triggers are) I think the ADC driver will need to know about it
directly in some fashion.
> 
> Is it a viable option, not to declare timer triggers in stm32-adc.c,
> but use pre-defined list of triggers, and separate trigger driver ? 
> I'm thinking then, of simple string based list... But maybe you
> already though about this king of things ?
A simple string based identification might be prone to problems as
any driver could define it's own naming.

So I think the triggers need to be supplied by stm32-adc.c as the
driver needs to know about the 'hard wired' nature of which timers
can be used.  The underlying handling of timer configuring etc
might be provided by a separate 'provider' module - similar to we
do for clock sources or regulators for example.  Both of these
have the same characteristic of being separate 'hardware blocks'
that can be connected to lots of things, but are in this case
directly feeding the devices using them.

(we do this with IIO consumers too but that is a different game
and not relevant here + harder to follow than simple regs and
clocks).
> 
> Please kindly share you view on this.

It's a non trivial job, but if you ultimately want to be able
to use those periodic timers for multiple possible purposes then
you'll need to do a fair bit of the work towards a generic
subsystem for timers anyway (callbacks etc).

Whether it is worth supporting the more 'soft' connected
equivalents (blackfin timer which is in staging as an IIO trigger
or the pxa271 periodic timer driver I wrote years ago - which
cheated and created a pile of RTCs to expose the interface)
is unclear.  A lot of those usecases are well handled by
the High Resolution Timer trigger.

Jonathan

> 
> Thanks again for your review.
> Best Regards,
> Fabrice
> 
>>> +static int stm32_adc_trig_register(struct iio_dev *indio_dev)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +    struct stm32_adc_common *common = adc->common;
>>> +    const struct stm32_adc_trig_info *ext = common->data->ext_triggers;
>>> +    struct iio_trigger *trig;
>>> +    int i, ret = 0;
>>> +
>>> +    if (adc->injected)
>>> +        ext = common->data->jext_triggers;
>>> +    else
>>> +        ext = common->data->ext_triggers;
>>> +
>>> +    for (i = 0; ext && ext[i].name; i++) {
>>> +        trig = devm_iio_trigger_alloc(common->dev, "%s_%s%d_%s",
>>> +                          indio_dev->name,
>>> +                          adc->injected ? "jext" : "ext",
>>> +                          ext[i].extsel, ext[i].name);
>>> +        if (!trig) {
>>> +            dev_err(common->dev, "trig %s_%s%d_%s alloc failed\n",
>>> +                indio_dev->name,
>>> +                adc->injected ? "jext" : "ext",
>>> +                ext[i].extsel, ext[i].name);
>>> +            ret = -ENOMEM;
>>> +            goto err;
>>> +        }
>>> +
>>> +        trig->dev.parent = common->dev;
>>> +        trig->ops = &stm32_adc_trigger_ops;
>>> +        iio_trigger_set_drvdata(trig, indio_dev);
>>> +
>>> +        ret = iio_trigger_register(trig);
>>> +        if (ret) {
>>> +            dev_err(common->dev,
>>> +                "trig %s_%s%d_%s register failed\n",
>>> +                indio_dev->name,
>>> +                adc->injected ? "jext" : "ext",
>>> +                ext[i].extsel, ext[i].name);
>>> +            goto err;
>>> +        }
>>> +
>>> +        list_add_tail(&trig->alloc_list, &adc->extrig_list);
>>> +    }
>>> +
>>> +    return 0;
>>> +err:
>>> +    stm32_adc_trig_unregister(indio_dev);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>>> +                    struct iio_chan_spec *chan,
>>> +                    const struct stm32_adc_chan_spec *channel,
>>> +                    int scan_index)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +
>>> +    chan->type = channel->type;
>>> +    chan->channel = channel->channel;
>>> +    chan->datasheet_name = channel->name;
>>> +    chan->extend_name = channel->name;
>>> +    chan->scan_index = scan_index;
>>> +    chan->indexed = 1;
>>> +    chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>>> +    chan->info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE);
>>> +    chan->scan_type.sign = 'u';
>>> +    chan->scan_type.realbits = adc->common->data->highres;
>>> +    chan->scan_type.storagebits = STM32_STORAGEBITS;
>> This is one of those cases where actually I'd argue just having the number
>> here and not under a define would be clearer!  So just put 16 here.
>>> +    chan->scan_type.shift = 0;
>> Should be unneeded.  Shift of 0 is the obvious default so no info provided
>> to readers of the code either really.
> I'll fix this
>>> +}
>>> +
>>> +static int stm32_adc_chan_of_init(struct iio_dev *indio_dev,
>>> +                  const struct stm32_adc_info *adc_info)
>>> +{
>>> +    struct stm32_adc *adc = iio_priv(indio_dev);
>>> +    struct device_node *node = indio_dev->dev.of_node;
>>> +    struct property *prop;
>>> +    const __be32 *cur;
>>> +    struct iio_chan_spec *channels;
>>> +    int scan_index = 0, num_channels = 0;
>>> +    u32 val;
>>> +
>>> +    of_property_for_each_u32(node, "st,adc-channels", prop, cur, val)
>>> +        num_channels++;
>>> +
>>> +    channels = devm_kcalloc(&indio_dev->dev, num_channels,
>>> +                sizeof(struct iio_chan_spec), GFP_KERNEL);
>>> +    if (!channels)
>>> +        return -ENOMEM;
>>> +
>>> +    of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
>>> +        stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
>>> +                    &adc_info->channels[val],
>>> +                    scan_index);
>>> +        scan_index++;
>>> +    }
>>> +
>>> +    adc->max_channels = adc_info->max_channels;
>>> +    indio_dev->num_channels = scan_index;
>>> +    indio_dev->channels = channels;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int stm32_adc_register(struct stm32_adc_common *common,
>>> +                  struct device_node *child)
>>> +{
>>> +    struct iio_dev *indio_dev;
>>> +    struct stm32_adc *adc;
>>> +    int i, ret;
>>> +    u32 reg;
>>> +
>>> +    ret = of_property_read_u32(child, "reg", &reg);
>>> +    if (ret != 0) {
>>> +        dev_err(common->dev, "missing reg property\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    for (i = 0; common->data->adc_info[i].channels; i++)
>>> +        if (common->data->adc_info[i].reg == reg)
>>> +            break;
>>> +
>>> +    if (i >= STM32_ADC_ID_MAX || !common->data->adc_info[i].channels) {
>>> +        dev_err(common->dev, "bad adc reg offset\n");
>>> +        return -ENOENT;
>>> +    }
>>> +
>>> +    indio_dev = devm_iio_device_alloc(common->dev, sizeof(*adc));
>>> +    if (!indio_dev) {
>>> +        dev_err(common->dev, "iio device allocation failed\n");
>>> +        return -ENOMEM;
>>> +    }
>>> +
>>> +    adc = iio_priv(indio_dev);
>>> +    adc->id = i;
>>> +    adc->offset = reg;
>>> +    adc->common = common;
>>> +    INIT_LIST_HEAD(&adc->extrig_list);
>>> +    spin_lock_init(&adc->lock);
>>> +    init_completion(&adc->completion);
>>> +
>>> +    if (child->name)
>>> +        indio_dev->name = child->name;
>>> +    else
>>> +        indio_dev->name = common->data->adc_info[i].name;
>>> +    indio_dev->dev.parent = common->dev;
>>> +    indio_dev->dev.of_node = child;
>>> +    indio_dev->info = &stm32_adc_iio_info;
>>> +    indio_dev->modes = INDIO_DIRECT_MODE;
>>> +
>>> +    if (of_property_read_bool(child, "st,injected")) {
>>> +        dev_dbg(common->dev, "%s Configured to use injected\n",
>>> +            indio_dev->name);
>>> +        adc->injected = true;
>>> +    }
>>> +
>>> +    adc->clk = of_clk_get(child, 0);
>>> +    if (IS_ERR(adc->clk)) {
>>> +        adc->clk = NULL;
>>> +        dev_dbg(common->dev, "No child clk found\n");
>>> +    } else {
>>> +        ret = clk_prepare_enable(adc->clk);
>>> +        if (ret < 0)
>>> +            goto err_clk_put;
>>> +    }
>>> +
>>> +    ret = stm32_adc_chan_of_init(indio_dev, &common->data->adc_info[i]);
>>> +    if (ret < 0) {
>>> +        dev_err(common->dev, "iio channels init failed\n");
>>> +        goto err_clk_disable;
>>> +    }
>>> +
>>> +    ret = stm32_adc_trig_register(indio_dev);
>>> +    if (ret)
>>> +        goto err_clk_disable;
>>> +
>>> +    ret = iio_triggered_buffer_setup(indio_dev,
>>> +                     &iio_pollfunc_store_time,
>>> +                     &stm32_adc_trigger_handler,
>>> +                     &iio_triggered_buffer_setup_ops);
>>> +    if (ret) {
>>> +        dev_err(common->dev, "buffer setup failed\n");
>>> +        goto err_trig_unregister;
>>> +    }
>>> +
>>> +    ret = iio_device_register(indio_dev);
>>> +    if (ret) {
>>> +        dev_err(common->dev, "iio dev register failed\n");
>>> +        goto err_buffer_cleanup;
>>> +    }
>>> +
>>> +    list_add_tail(&adc->adc_list, &common->adc_list);
>>> +
>>> +    return 0;
>>> +
>>> +err_buffer_cleanup:
>>> +    iio_triggered_buffer_cleanup(indio_dev);
>>> +
>>> +err_trig_unregister:
>>> +    stm32_adc_trig_unregister(indio_dev);
>>> +
>>> +err_clk_disable:
>>> +    if (adc->clk)
>>> +        clk_disable_unprepare(adc->clk);
>>> +
>>> +err_clk_put:
>>> +    if (adc->clk)
>>> +        clk_put(adc->clk);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static void stm32_adc_unregister(struct stm32_adc *adc)
>>> +{
>>> +    struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>>> +
>>> +    iio_device_unregister(indio_dev);
>>> +    iio_triggered_buffer_cleanup(indio_dev);
>>> +    stm32_adc_trig_unregister(indio_dev);
>>> +    if (adc->clk) {
>>> +        clk_disable_unprepare(adc->clk);
>>> +        clk_put(adc->clk);
>>> +    }
>>> +}
>>> +
>>> +int stm32_adc_probe(struct platform_device *pdev)
>>> +{
>>> +    struct device_node *np = pdev->dev.of_node, *child;
>>> +    struct device *dev = &pdev->dev;
>>> +    const struct of_device_id *match;
>>> +    struct stm32_adc_common *common;
>>> +    struct stm32_adc *adc;
>>> +    struct resource *res;
>>> +    int ret;
>>> +
>>> +    match = of_match_device(dev->driver->of_match_table, &pdev->dev);
>>> +    if (!match || !match->data) {
>>> +        dev_err(&pdev->dev, "compatible data not provided\n");
>> How would we have instantiated this if there was not a suitable match?
>> As such what does this check give us? (confused!)
> I'll fix this.
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    common = devm_kzalloc(&pdev->dev, sizeof(*common), GFP_KERNEL);
>>> +    if (!common)
>>> +        return -ENOMEM;
>>> +
>>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> +    common->base = devm_ioremap_resource(&pdev->dev, res);
>>> +    if (IS_ERR(common->base))
>>> +        return PTR_ERR(common->base);
>>> +
>>> +    common->data = match->data;
>>> +    common->dev = &pdev->dev;
>>> +    platform_set_drvdata(pdev, common);
>>> +    mutex_init(&common->lock);
>>> +    INIT_LIST_HEAD(&common->adc_list);
>>> +
>>> +    common->vref = devm_regulator_get(&pdev->dev, "vref");
>>> +    if (IS_ERR(common->vref)) {
>>> +        ret = PTR_ERR(common->vref);
>>> +        dev_err(&pdev->dev, "vref get failed, %d\n", ret);
>>> +        return ret;
>>> +    }
>>> +
>>> +    ret = regulator_enable(common->vref);
>>> +    if (ret < 0) {
>>> +        dev_err(&pdev->dev, "vref enable failed\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    ret = regulator_get_voltage(common->vref);
>>> +    if (ret < 0) {
>>> +        dev_err(&pdev->dev, "vref get voltage failed, %d\n", ret);
>>> +        goto err_regulator_disable;
>>> +    }
>>> +    common->vref_mv = ret / 1000;
>>> +    dev_dbg(&pdev->dev, "vref+=%dmV\n", common->vref_mv);
>>> +
>>> +    common->aclk = devm_clk_get(&pdev->dev, "adc");
>>> +    if (IS_ERR(common->aclk)) {
>>> +        ret = PTR_ERR(common->aclk);
>>> +        dev_err(&pdev->dev, "Can't get 'adc' clock\n");
>>> +        goto err_regulator_disable;
>>> +    }
>>> +
>>> +    ret = clk_prepare_enable(common->aclk);
>>> +    if (ret < 0) {
>>> +        dev_err(common->dev, "adc clk enable failed\n");
>>> +        goto err_regulator_disable;
>>> +    }
>>> +
>>> +    common->irq = platform_get_irq(pdev, 0);
>>> +    if (common->irq < 0) {
>>> +        dev_err(&pdev->dev, "failed to get irq\n");
>>> +        ret = common->irq;
>>> +        goto err_clk_disable;
>>> +    }
>>> +
>>> +    ret = devm_request_irq(&pdev->dev, common->irq,    stm32_adc_common_isr,
>>> +                   0, pdev->name, common);
>>> +    if (ret) {
>>> +        dev_err(&pdev->dev, "failed to request irq\n");
>>> +        goto err_clk_disable;
>>> +    }
>>> +
>>> +    /* Parse adc child nodes to retrieve master/slave instances data */
>>> +    for_each_available_child_of_node(np, child) {
>>> +        ret = stm32_adc_register(common, child);
>>> +        if (ret)
>>> +            goto err_unregister;
>>> +    }
>>> +
>>> +    dev_info(&pdev->dev, "registered\n");
>> No benefit in this info being provided (it's obvious, device just turned up
>> in sysfs :) So drop it.
> I'll fix this.
>>> +
>>> +    return 0;
>>> +
>>> +err_unregister:
>>> +    list_for_each_entry(adc, &common->adc_list, adc_list)
>>> +        stm32_adc_unregister(adc);
>>> +
>>> +err_clk_disable:
>>> +    clk_disable_unprepare(common->aclk);
>>> +
>>> +err_regulator_disable:
>>> +    regulator_disable(common->vref);
>>> +
>>> +    return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(stm32_adc_probe);
>>> +
>>> +int stm32_adc_remove(struct platform_device *pdev)
>>> +{
>>> +    struct stm32_adc_common *common = platform_get_drvdata(pdev);
>>> +    struct stm32_adc *adc;
>>> +
>>> +    list_for_each_entry(adc, &common->adc_list, adc_list)
>>> +        stm32_adc_unregister(adc);
>>> +    clk_disable_unprepare(common->aclk);
>>> +    regulator_disable(common->vref);
>>> +
>>> +    return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(stm32_adc_remove);
>>> +
>>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>");
>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 ADC driver");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/iio/adc/stm32/stm32-adc.h b/drivers/iio/adc/stm32/stm32-adc.h
>>> new file mode 100644
>>> index 0000000..0be603c
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/stm32-adc.h
>>> @@ -0,0 +1,442 @@
>>> +/*
>>> + * This file is part of STM32 ADC driver
>>> + *
>>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>>> + * Author: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>.
>>> + *
>>> + * License type: GPLv2
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 as published by
>>> + * the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>>> + * See the GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#ifndef __STM32_ADC_H
>>> +#define __STM32_ADC_H
>>> +
>>> +/*
>>> + * STM32 - ADC global register map
>>> + * ________________________________________________________
>>> + * | Offset |                 Register                    |
>>> + * --------------------------------------------------------
>>> + * | 0x000  |                Master ADC1                  |
>>> + * --------------------------------------------------------
>>> + * | 0x100  |                Slave ADC2                   |
>>> + * --------------------------------------------------------
>>> + * | 0x200  |                Slave ADC3                   |
>>> + * --------------------------------------------------------
>>> + * | 0x300  |         Master & Slave common regs          |
>>> + * --------------------------------------------------------
>>> + */
>>> +#define STM32_ADCX_COMN_OFFSET        0x300
>>> +#define STM32_ADC_ID_MAX        3
>>> +#define STM32_ADC_MAX_SQ        16    /* SQ1..SQ16 */
>>> +#define STM32_ADC_MAX_JSQ        4    /* JSQ1..JSQ4 */
>>> +
>>> +/* STM32 value masks */
>>> +#define STM32_RESULT_MASK    GENMASK(15, 0)
>>> +#define STM32_STORAGEBITS    16
>>> +
>>> +/* External trigger enable for regular or injected channels (exten/jexten) */
>>> +enum stm32_adc_exten {
>>> +    STM32_EXTEN_SWTRIG,
>>> +    STM32_EXTEN_HWTRIG_RISING_EDGE,
>>> +    STM32_EXTEN_HWTRIG_FALLING_EDGE,
>>> +    STM32_EXTEN_HWTRIG_BOTH_EDGES,
>>> +};
>>> +
>>> +enum stm32_adc_extsel {
>>> +    STM32_EXT0,
>>> +    STM32_EXT1,
>>> +    STM32_EXT2,
>>> +    STM32_EXT3,
>>> +    STM32_EXT4,
>>> +    STM32_EXT5,
>>> +    STM32_EXT6,
>>> +    STM32_EXT7,
>>> +    STM32_EXT8,
>>> +    STM32_EXT9,
>>> +    STM32_EXT10,
>>> +    STM32_EXT11,
>>> +    STM32_EXT12,
>>> +    STM32_EXT13,
>>> +    STM32_EXT14,
>>> +    STM32_EXT15,
>>> +    STM32_EXT16,
>>> +    STM32_EXT17,
>>> +    STM32_EXT18,
>>> +    STM32_EXT19,
>>> +    STM32_EXT20,
>>> +    STM32_EXT21,
>>> +    STM32_EXT22,
>>> +    STM32_EXT23,
>>> +    STM32_EXT24,
>>> +    STM32_EXT25,
>>> +    STM32_EXT26,
>>> +    STM32_EXT27,
>>> +    STM32_EXT28,
>>> +    STM32_EXT29,
>>> +    STM32_EXT30,
>>> +    STM32_EXT31,
>>> +};
>>> +
>>> +enum stm32_adc_jextsel {
>>> +    STM32_JEXT0,
>>> +    STM32_JEXT1,
>>> +    STM32_JEXT2,
>>> +    STM32_JEXT3,
>>> +    STM32_JEXT4,
>>> +    STM32_JEXT5,
>>> +    STM32_JEXT6,
>>> +    STM32_JEXT7,
>>> +    STM32_JEXT8,
>>> +    STM32_JEXT9,
>>> +    STM32_JEXT10,
>>> +    STM32_JEXT11,
>>> +    STM32_JEXT12,
>>> +    STM32_JEXT13,
>>> +    STM32_JEXT14,
>>> +    STM32_JEXT15,
>>> +    STM32_JEXT16,
>>> +    STM32_JEXT17,
>>> +    STM32_JEXT18,
>>> +    STM32_JEXT19,
>>> +    STM32_JEXT20,
>>> +    STM32_JEXT21,
>>> +    STM32_JEXT22,
>>> +    STM32_JEXT23,
>>> +    STM32_JEXT24,
>>> +    STM32_JEXT25,
>>> +    STM32_JEXT26,
>>> +    STM32_JEXT27,
>>> +    STM32_JEXT28,
>>> +    STM32_JEXT29,
>>> +    STM32_JEXT30,
>>> +    STM32_JEXT31,
>>> +};
>>> +
>>> +#define    STM32_ADC_TIMEOUT_US    100000
>>> +#define    STM32_ADC_TIMEOUT    (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>>> +
>>> +/**
>>> + * struct stm32_adc_chan_spec - specification of stm32 adc channel
>>> + * @type:    IIO channel type
>>> + * @channel:    channel number (single ended)
>>> + * @name:    channel name (single ended)
>>> + */
>>> +struct stm32_adc_chan_spec {
>>> +    enum iio_chan_type    type;
>>> +    int            channel;
>>> +    const char        *name;
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_trig_info - ADC trigger info
>>> + * @extsel:        trigger selection for regular or injected
>>> + * @name:        name of the trigger, corresponding to its source
>>> + */
>>> +struct stm32_adc_trig_info {
>>> +    u32 extsel;
>>> +    const char *name;
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_info - stm32 ADC, per instance config data
>>> + * @name:        default name for this instance (like "adc1")
>>> + * @reg:        reg offset for this instance (e.g. 0x0 for adc1...)
>>> + * @channels:        Reference to stm32 channels spec
>>> + * @max_channels:    Number of single ended channels
>>> + */
>>> +struct stm32_adc_info {
>>> +    const char *name;
>>> +    u32 reg;
>>> +    const struct stm32_adc_chan_spec *channels;
>>> +    int max_channels;
>>> +};
>>> +
>>> +/**
>>> + * stm32_adc_regs - stm32 ADC misc registers & bitfield desc
>>> + * @reg:        register offset
>>> + * @mask:        bitfield mask
>>> + * @shift:        left shift
>>> + */
>>> +struct stm32_adc_regs {
>>> +    int reg;
>>> +    int mask;
>>> +    int shift;
>>> +};
>>> +
>>> +/**
>>> + * stm32_adc_trig_reginfo - stm32 ADC trigger control registers description
>>> + * @reg:        trigger control register offset (exten/jexten)
>>> + * @exten_mask:        external trigger en/polarity mask in @reg
>>> + * @exten_shift:    external trigger en/polarity shift in @reg
>>> + * @extsel_mask:    external trigger source mask in @reg
>>> + * @extsel_shift:    external trigger source shift in @reg
>>> + */
>>> +struct stm32_adc_trig_reginfo {
>>> +    u32 reg;
>>> +    u32 exten_mask;
>>> +    u32 exten_shift;
>>> +    u32 extsel_mask;
>>> +    u32 extsel_shift;
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_reginfo - stm32 ADC registers description
>>> + * @isr:        interrupt status register offset
>>> + * @eoc:        end of conversion mask in @isr
>>> + * @jeoc:        end of injected conversion sequence mask in @isr
>>> + * @ier:        interrupt enable register offset
>>> + * @eocie:        end of conversion interrupt enable mask in @ier
>>> + * @jeocie:        end of injected conversion sequence interrupt en mask
>>> + * @dr:            data register offset
>>> + * @jdr:        injected data registers offsets
>>> + * @sqr_regs:        Regular sequence registers description
>>> + * @jsqr_reg:        Injected sequence register description
>>> + * @trig_reginfo:    regular trigger control registers description
>>> + * @jtrig_reginfo:    injected trigger control registers description
>>> + */
>>> +struct stm32_adc_reginfo {
>>> +    u32 isr;
>>> +    u32 eoc;
>>> +    u32 jeoc;
>>> +    u32 ier;
>>> +    u32 eocie;
>>> +    u32 jeocie;
>>> +    u32 dr;
>>> +    u32 jdr[4];
>>> +    const struct stm32_adc_regs *sqr_regs;
>>> +    const struct stm32_adc_regs *jsqr_reg;
>>> +    const struct stm32_adc_trig_reginfo *trig_reginfo;
>>> +    const struct stm32_adc_trig_reginfo *jtrig_reginfo;
>>> +};
>>> +
>>> +struct stm32_adc;
>>> +
>>> +/**
>>> + * struct stm32_adc_ops - stm32 ADC, compatible dependent data
>>> + * - stm32 ADC may work as single ADC, or as tightly coupled master/slave ADCs.
>>> + *
>>> + * @adc_info:        Array spec for stm32 adc master/slaves instances
>>> + * @ext_triggers:    Reference to trigger info for regular channels
>>> + * @jext_triggers:    Reference to trigger info for injected channels
>>> + * @adc_reginfo:    stm32 ADC registers description
>>> + * @highres:        Max resolution
>>> + * @max_clock_rate:    Max input clock rate
>>> + * @clk_sel:        routine to select common clock and prescaler
>>> + * @start_conv:        routine to start conversions
>>> + * @stop_conv:        routine to stop conversions
>>> + * @is_started:        routine to get adc 'started' state
>>> + * @regular_started    routine to check regular conversions status
>>> + * @injected_started    routine to check injected conversions status
>>> + * @enable:        optional routine to enable stm32 adc
>>> + * @disable:        optional routine to disable stm32 adc
>>> + * @is_enabled        reports enabled state
>>> + */
>> This is a big chunk of abstraction that seems excessive at the moment.
>> I'd rather see it introduced only just before it's actually used..
>> (I'm guessing it's intended for support of similar parts?)
>>
>> Right now it just makes the driver harder to review.
>>> +struct stm32_adc_ops {
>>> +    const struct stm32_adc_info *adc_info;
>>> +    const struct stm32_adc_trig_info *ext_triggers;
>>> +    const struct stm32_adc_trig_info *jext_triggers;
>>> +    const struct stm32_adc_reginfo *adc_reginfo;
>>> +    int highres;
>>> +    unsigned long max_clock_rate;
>>> +    int (*clk_sel)(struct stm32_adc *adc);
>>> +    int (*start_conv)(struct stm32_adc *adc);
>>> +    int (*stop_conv)(struct stm32_adc *adc);
>>> +    bool (*is_started)(struct stm32_adc *adc);
>>> +    bool (*regular_started)(struct stm32_adc *adc);
>>> +    bool (*injected_started)(struct stm32_adc *adc);
>>> +    int (*enable)(struct stm32_adc *adc);
>>> +    void (*disable)(struct stm32_adc *adc);
>>> +    bool (*is_enabled)(struct stm32_adc *adc);
>>> +};
>>> +
>>> +struct stm32_adc_common;
>>> +
>>> +/**
>>> + * struct stm32_adc - private data of each ADC IIO instance
>>> + * @common:        reference to ADC block common data
>>> + * @adc_list:        current ADC entry in common ADC list
>>> + * @id:            ADC instance number (e.g. adc 1, 2 or 3)
>>> + * @offset:        ADC instance register offset in ADC block
>>> + * @max_channels:    Max channels number for this ADC.
>>> + * @extrig_list:    External trigger list (for regular channel)
>>> + * @completion:        end of single conversion completion
>>> + * @buffer:        data buffer
>>> + * @bufi:        data buffer index
>>> + * @num_conv:        expected number of scan conversions
>>> + * @injected:        use injected channels on this adc
>>> + * @lock:        spinlock
>>> + * @clk:        optional adc clock, for this adc instance
>>> + * @calib:        optional calibration data
>>> + * @en:            emulates enabled state on some stm32 adc
>>> + */
>>> +struct stm32_adc {
>>> +    struct stm32_adc_common    *common;
>>> +    struct list_head    adc_list;
>>> +    int            id;
>>> +    int            offset;
>>> +    int            max_channels;
>>> +    struct list_head    extrig_list;
>>> +    struct completion    completion;
>>> +    u16            *buffer;
>>> +    int            bufi;
>>> +    int            num_conv;
>>> +    bool            injected;
>>> +    spinlock_t        lock;        /* interrupt lock */
>>> +    struct clk        *clk;
>>> +    void            *calib;
>>> +    bool            en;
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_adc_common - private data of ADC driver, common to all
>>> + * ADC instances (ADC block)
>>> + * @dev:        device for this controller
>>> + * @base:        control registers base cpu addr
>>> + * @irq:        Common irq line for all adc instances
>>> + * @data:        STM32 dependent data from compatible
>>> + * @adc_list:        list of all stm32 ADC in this ADC block
>>> + * @aclk:        common clock for the analog circuitry
>>> + * @vref:        regulator reference
>>> + * @vref_mv:        vref voltage (mv)
>>> + * @lock:        mutex
>>> + */
>>> +struct stm32_adc_common {
>>> +    struct device            *dev;
>>> +    void __iomem            *base;
>>> +    int                irq;
>>> +    const struct stm32_adc_ops    *data;
>>> +    struct list_head        adc_list;
>>> +    struct clk            *aclk;
>>> +    struct regulator        *vref;
>>> +    int                vref_mv;
>>> +    struct mutex            lock;    /* read_raw lock */
>>> +};
>>> +
>>> +/* Helper routines */
>>> +static inline int stm32_adc_start_conv(struct stm32_adc *adc)
>>> +{
>>> +    return adc->common->data->start_conv(adc);
>>> +}
>>> +
>>> +static inline int stm32_adc_stop_conv(struct stm32_adc *adc)
>>> +{
>>> +    return adc->common->data->stop_conv(adc);
>>> +}
>>> +
>>> +static inline bool stm32_adc_is_started(struct stm32_adc *adc)
>>> +{
>>> +    return adc->common->data->is_started(adc);
>>> +}
>>> +
>>> +static inline bool stm32_adc_regular_started(struct stm32_adc *adc)
>>> +{
>>> +    return adc->common->data->regular_started(adc);
>>> +}
>>> +
>>> +static inline bool stm32_adc_injected_started(struct stm32_adc *adc)
>>> +{
>>> +    return adc->common->data->injected_started(adc);
>>> +}
>>> +
>>> +static inline bool stm32_adc_clk_sel(struct stm32_adc *adc)
>>> +{
>>> +    return adc->common->data->clk_sel(adc);
>>> +}
>>> +
>>> +static inline int stm32_adc_enable(struct stm32_adc *adc)
>>> +{
>>> +    if (adc->common->data->enable)
>>> +        return adc->common->data->enable(adc);
>>> +
>>> +    adc->en = true;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static inline bool stm32_adc_is_enabled(struct stm32_adc *adc)
>>> +{
>>> +    if (adc->common->data->is_enabled)
>>> +        return adc->common->data->is_enabled(adc);
>>> +    else
>>> +        return adc->en;
>>> +}
>>> +
>>> +static inline void stm32_adc_disable(struct stm32_adc *adc)
>>> +{
>>> +    /* Check there is no regular or injected on-going conversions */
>>> +    if (stm32_adc_is_started(adc))
>>> +        return;
>>> +
>>> +    if (adc->common->data->disable)
>>> +        adc->common->data->disable(adc);
>>> +    else
>>> +        adc->en = false;
>>> +}
>>> +
>>> +/* STM32 ADC registers access routines */
>>> +static inline u32 stm32_adc_common_readl(struct stm32_adc_common *com, u32 reg)
>>> +{
>>> +    u32 val = readl_relaxed(com->base + reg);
>>> +
>>> +    return val;
>>> +}
>>> +
>>> +static inline void stm32_adc_common_writel(struct stm32_adc_common *com,
>>> +                       u32 reg, u32 val)
>>> +{
>>> +    writel_relaxed(val, com->base + reg);
>>> +}
>>> +
>>> +static inline u32 stm32_adc_readl(struct stm32_adc *adc, u32 reg)
>>> +{
>>> +    u32 val = readl_relaxed(adc->common->base + adc->offset + reg);
>>> +
>>> +    return val;
>>> +}
>>> +
>>> +#define stm32_adc_readl_addr(addr)    stm32_adc_readl(adc, addr)
>>> +
>>> +#define stm32_adc_readl_poll_timeout(reg, val, cond, sleep_us, timeout_us) \
>>> +    readx_poll_timeout(stm32_adc_readl_addr, reg, val, \
>>> +               cond, sleep_us, timeout_us)
>>> +
>>> +static inline void stm32_adc_writel(struct stm32_adc *adc, u32 reg, u32 val)
>>> +{
>>> +    writel_relaxed(val, adc->common->base + adc->offset + reg);
>>> +}
>>> +
>>> +static inline void stm32_adc_set_bits(struct stm32_adc *adc, u32 reg, u32 bits)
>>> +{
>>> +    unsigned long flags;
>>> +
>>> +    spin_lock_irqsave(&adc->lock, flags);
>>> +    stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) | bits);
>>> +    spin_unlock_irqrestore(&adc->lock, flags);
>>> +}
>>> +
>>> +static inline void stm32_adc_clr_bits(struct stm32_adc *adc, u32 reg, u32 bits)
>>> +{
>>> +    unsigned long flags;
>>> +
>>> +    spin_lock_irqsave(&adc->lock, flags);
>>> +    stm32_adc_writel(adc, reg, stm32_adc_readl(adc, reg) & ~bits);
>>> +    spin_unlock_irqrestore(&adc->lock, flags);
>>> +}
>>> +
>>> +/* STM32 common extended attributes */
>>> +extern const struct iio_enum stm32_adc_trig_pol;
>>> +int stm32_adc_probe(struct platform_device *pdev);
>>> +int stm32_adc_remove(struct platform_device *pdev);
>>> +
>>> +#endif
>>> diff --git a/drivers/iio/adc/stm32/stm32f4-adc.c b/drivers/iio/adc/stm32/stm32f4-adc.c
>>> new file mode 100644
>>> index 0000000..147fe9c
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32/stm32f4-adc.c
>>> @@ -0,0 +1,574 @@
>>> +/*
>>> + * This file is part of STM32F4 ADC driver
>>> + *
>>> + * Copyright (C) 2016, STMicroelectronics - All Rights Reserved
>>> + * Author: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>.
>>> + *
>>> + * License type: GPLv2
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 as published by
>>> + * the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful, but
>>> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>>> + * or FITNESS FOR A PARTICULAR PURPOSE.
>>> + * See the GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/platform_device.h>
>>> +#include "stm32-adc.h"
>>> +
>>> +/*
>>> + * STM32F4 - ADC global register map
>>> + * ________________________________________________________
>>> + * | Offset |                 Register                    |
>>> + * --------------------------------------------------------
>>> + * | 0x000  |                Master ADC1                  |
>>> + * --------------------------------------------------------
>>> + * | 0x100  |                Slave ADC2                   |
>>> + * --------------------------------------------------------
>>> + * | 0x200  |                Slave ADC3                   |
>>> + * --------------------------------------------------------
>>> + * | 0x300  |         Master & Slave common regs          |
>>> + * --------------------------------------------------------
>>> + */
>>> +
>>> +/* STM32F4 - Registers for each ADC instance */
>>> +#define STM32F4_ADCX_SR            0x00
>>> +#define STM32F4_ADCX_CR1        0x04
>>> +#define STM32F4_ADCX_CR2        0x08
>>> +#define STM32F4_ADCX_SMPR1        0x0C
>>> +#define STM32F4_ADCX_SMPR2        0x10
>>> +#define STM32F4_ADCX_HTR        0x24
>>> +#define STM32F4_ADCX_LTR        0x28
>>> +#define STM32F4_ADCX_SQR1        0x2C
>>> +#define STM32F4_ADCX_SQR2        0x30
>>> +#define STM32F4_ADCX_SQR3        0x34
>>> +#define STM32F4_ADCX_JSQR        0x38
>>> +#define STM32F4_ADCX_JDR1        0x3C
>>> +#define STM32F4_ADCX_JDR2        0x40
>>> +#define STM32F4_ADCX_JDR3        0x44
>>> +#define STM32F4_ADCX_JDR4        0x48
>>> +#define STM32F4_ADCX_DR            0x4C
>>> +
>>> +/* STM32 - Master & slave registers (common for all instances: 1, 2 & 3) */
>>> +#define STM32F4_ADC_CSR            (STM32_ADCX_COMN_OFFSET + 0x00)
>>> +#define STM32F4_ADC_CCR            (STM32_ADCX_COMN_OFFSET + 0x04)
>>> +#define STM32F4_ADC_CDR            (STM32_ADCX_COMN_OFFSET + 0x08)
>>> +
>>> +/* STM32F4_ADCX_SR - bit fields */
>>> +#define STM32F4_OVR            BIT(5)
>>> +#define STM32F4_STRT            BIT(4)
>>> +#define STM32F4_JSTRT            BIT(3)
>>> +#define STM32F4_JEOC            BIT(2)
>>> +#define STM32F4_EOC            BIT(1)
>>> +#define STM32F4_AWD            BIT(0)
>>> +
>>> +/* STM32F4_ADCX_CR1 - bit fields */
>>> +#define STM32F4_OVRIE            BIT(26)
>>> +#define STM32F4_RES_SHIFT        24
>>> +#define STM32F4_RES_MASK        GENMASK(25, 24)
>>> +#define STM32F4_AWDEN            BIT(23)
>>> +#define STM32F4_JAWDEN            BIT(22)
>>> +#define STM32F4_DISCNUM_SHIFT        13
>>> +#define STM32F4_DISCNUM_MASK        GENMASK(15, 13)
>>> +#define STM32F4_JDISCEN            BIT(12)
>>> +#define STM32F4_DISCEN            BIT(11)
>>> +#define STM32F4_JAUTO            BIT(10)
>>> +#define STM32F4_AWDSGL            BIT(9)
>>> +#define STM32F4_SCAN            BIT(8)
>>> +#define STM32F4_JEOCIE            BIT(7)
>>> +#define STM32F4_AWDIE            BIT(6)
>>> +#define STM32F4_EOCIE            BIT(5)
>>> +#define STM32F4_AWDCH_SHIFT        0
>>> +#define STM32F4_AWDCH_MASK        GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADCX_CR2 - bit fields */
>>> +#define STM32F4_SWSTART            BIT(30)
>>> +#define STM32F4_EXTEN_SHIFT        28
>>> +#define STM32F4_EXTEN_MASK        GENMASK(29, 28)
>>> +#define STM32F4_EXTSEL_SHIFT        24
>>> +#define STM32F4_EXTSEL_MASK        GENMASK(27, 24)
>>> +#define STM32F4_JSWSTART        BIT(22)
>>> +#define STM32F4_JEXTEN_SHIFT        20
>>> +#define STM32F4_JEXTEN_MASK        GENMASK(21, 20)
>>> +#define STM32F4_JEXTSEL_SHIFT        16
>>> +#define STM32F4_JEXTSEL_MASK        GENMASK(19, 16)
>>> +#define STM32F4_ALIGN            BIT(11)
>>> +#define STM32F4_EOCS            BIT(10)
>>> +#define STM32F4_DDS            BIT(9)
>>> +#define STM32F4_DMA            BIT(8)
>>> +#define STM32F4_CONT            BIT(1)
>>> +#define STM32F4_ADON            BIT(0)
>>> +
>>> +/* STM32F4_ADCX_SMPR1 - bit fields */
>>> +#define STM32F4_SMP18_SHIFT        24
>>> +#define STM32F4_SMP18_MASK        GENMASK(26, 24)
>>> +#define STM32F4_SMP17_SHIFT        21
>>> +#define STM32F4_SMP17_MASK        GENMASK(23, 21)
>>> +#define STM32F4_SMP16_SHIFT        18
>>> +#define STM32F4_SMP16_MASK        GENMASK(20, 18)
>>> +#define STM32F4_SMP15_SHIFT        15
>>> +#define STM32F4_SMP15_MASK        GENMASK(17, 15)
>>> +#define STM32F4_SMP14_SHIFT        12
>>> +#define STM32F4_SMP14_MASK        GENMASK(14, 12)
>>> +#define STM32F4_SMP13_SHIFT        9
>>> +#define STM32F4_SMP13_MASK        GENMASK(11, 9)
>>> +#define STM32F4_SMP12_SHIFT        6
>>> +#define STM32F4_SMP12_MASK        GENMASK(8, 6)
>>> +#define STM32F4_SMP11_SHIFT        3
>>> +#define STM32F4_SMP11_MASK        GENMASK(5, 3)
>>> +#define STM32F4_SMP10_SHIFT        0
>>> +#define STM32F4_SMP10_MASK        GENMASK(2, 0)
>>> +
>>> +/* STM32F4_ADCX_SMPR2 - bit fields */
>>> +#define STM32F4_SMP9_SHIFT        27
>>> +#define STM32F4_SMP9_MASK        GENMASK(29, 27)
>>> +#define STM32F4_SMP8_SHIFT        24
>>> +#define STM32F4_SMP8_MASK        GENMASK(26, 24)
>>> +#define STM32F4_SMP7_SHIFT        21
>>> +#define STM32F4_SMP7_MASK        GENMASK(23, 21)
>>> +#define STM32F4_SMP6_SHIFT        18
>>> +#define STM32F4_SMP6_MASK        GENMASK(20, 18)
>>> +#define STM32F4_SMP5_SHIFT        15
>>> +#define STM32F4_SMP5_MASK        GENMASK(17, 15)
>>> +#define STM32F4_SMP4_SHIFT        12
>>> +#define STM32F4_SMP4_MASK        GENMASK(14, 12)
>>> +#define STM32F4_SMP3_SHIFT        9
>>> +#define STM32F4_SMP3_MASK        GENMASK(11, 9)
>>> +#define STM32F4_SMP2_SHIFT        6
>>> +#define STM32F4_SMP2_MASK        GENMASK(8, 6)
>>> +#define STM32F4_SMP1_SHIFT        3
>>> +#define STM32F4_SMP1_MASK        GENMASK(5, 3)
>>> +#define STM32F4_SMP0_SHIFT        0
>>> +#define STM32F4_SMP0_MASK        GENMASK(2, 0)
>>> +enum stm32f4_adc_smpr {
>>> +    STM32F4_SMPR_3_CK_CYCLES,
>>> +    STM32F4_SMPR_15_CK_CYCLES,
>>> +    STM32F4_SMPR_28_CK_CYCLES,
>>> +    STM32F4_SMPR_56_CK_CYCLES,
>>> +    STM32F4_SMPR_84_CK_CYCLES,
>>> +    STM32F4_SMPR_112_CK_CYCLES,
>>> +    STM32F4_SMPR_144_CK_CYCLES,
>>> +    STM32F4_SMPR_480_CK_CYCLES,
>>> +};
>>> +
>>> +/* STM32F4_ADCX_SQR1 - bit fields */
>>> +#define STM32F4_L_SHIFT            20
>>> +#define STM32F4_L_MASK            GENMASK(23, 20)
>>> +#define STM32F4_SQ16_SHIFT        15
>>> +#define STM32F4_SQ16_MASK        GENMASK(19, 15)
>>> +#define STM32F4_SQ15_SHIFT        10
>>> +#define STM32F4_SQ15_MASK        GENMASK(14, 10)
>>> +#define STM32F4_SQ14_SHIFT        5
>>> +#define STM32F4_SQ14_MASK        GENMASK(9, 5)
>>> +#define STM32F4_SQ13_SHIFT        0
>>> +#define STM32F4_SQ13_MASK        GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADCX_SQR2 - bit fields */
>>> +#define STM32F4_SQ12_SHIFT        25
>>> +#define STM32F4_SQ12_MASK        GENMASK(29, 25)
>>> +#define STM32F4_SQ11_SHIFT        20
>>> +#define STM32F4_SQ11_MASK        GENMASK(24, 20)
>>> +#define STM32F4_SQ10_SHIFT        15
>>> +#define STM32F4_SQ10_MASK        GENMASK(19, 15)
>>> +#define STM32F4_SQ9_SHIFT        10
>>> +#define STM32F4_SQ9_MASK        GENMASK(14, 10)
>>> +#define STM32F4_SQ8_SHIFT        5
>>> +#define STM32F4_SQ8_MASK        GENMASK(9, 5)
>>> +#define STM32F4_SQ7_SHIFT        0
>>> +#define STM32F4_SQ7_MASK        GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADCX_SQR3 - bit fields */
>>> +#define STM32F4_SQ6_SHIFT        25
>>> +#define STM32F4_SQ6_MASK        GENMASK(29, 25)
>>> +#define STM32F4_SQ5_SHIFT        20
>>> +#define STM32F4_SQ5_MASK        GENMASK(24, 20)
>>> +#define STM32F4_SQ4_SHIFT        15
>>> +#define STM32F4_SQ4_MASK        GENMASK(19, 15)
>>> +#define STM32F4_SQ3_SHIFT        10
>>> +#define STM32F4_SQ3_MASK        GENMASK(14, 10)
>>> +#define STM32F4_SQ2_SHIFT        5
>>> +#define STM32F4_SQ2_MASK        GENMASK(9, 5)
>>> +#define STM32F4_SQ1_SHIFT        0
>>> +#define STM32F4_SQ1_MASK        GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADCX_JSQR - bit fields */
>>> +#define STM32F4_JL_SHIFT        20
>>> +#define STM32F4_JL_MASK            GENMASK(21, 20)
>>> +#define STM32F4_JSQ4_SHIFT        15
>>> +#define STM32F4_JSQ4_MASK        GENMASK(19, 15)
>>> +#define STM32F4_JSQ3_SHIFT        10
>>> +#define STM32F4_JSQ3_MASK        GENMASK(14, 10)
>>> +#define STM32F4_JSQ2_SHIFT        5
>>> +#define STM32F4_JSQ2_MASK        GENMASK(9, 5)
>>> +#define STM32F4_JSQ1_SHIFT        0
>>> +#define STM32F4_JSQ1_MASK        GENMASK(4, 0)
>>> +
>>> +/* STM32F4_ADC_CCR - bit fields */
>>> +#define STM32F4_ADC_ADCPRE_SHIFT    16
>>> +#define STM32F4_ADC_ADCPRE_MASK        GENMASK(17, 16)
>>> +
>>> +/*
>>> + * stm32 ADC1, ADC2 & ADC3 are tightly coupled and may be used in multi mode
>>> + * Define here all inputs for all ADC instances
>>> + */
>>> +static const struct stm32_adc_chan_spec stm32f4_adc1_channels[] = {
>>> +    /* master ADC1 */
>>> +    { IIO_VOLTAGE, 0, "in0" },
>>> +    { IIO_VOLTAGE, 1, "in1" },
>>> +    { IIO_VOLTAGE, 2, "in2" },
>>> +    { IIO_VOLTAGE, 3, "in3" },
>>> +    { IIO_VOLTAGE, 4, "in4" },
>>> +    { IIO_VOLTAGE, 5, "in5" },
>>> +    { IIO_VOLTAGE, 6, "in6" },
>>> +    { IIO_VOLTAGE, 7, "in7" },
>>> +    { IIO_VOLTAGE, 8, "in8" },
>>> +    { IIO_VOLTAGE, 9, "in9" },
>>> +    { IIO_VOLTAGE, 10, "in10" },
>>> +    { IIO_VOLTAGE, 11, "in11" },
>>> +    { IIO_VOLTAGE, 12, "in12" },
>>> +    { IIO_VOLTAGE, 13, "in13" },
>>> +    { IIO_VOLTAGE, 14, "in14" },
>>> +    { IIO_VOLTAGE, 15, "in15" },
>>> +    /* internal analog sources available on input 16 to 18 */
>>> +    { IIO_VOLTAGE, 16, "in16" },
>>> +    { IIO_VOLTAGE, 17, "in17" },
>>> +    { IIO_VOLTAGE, 18, "in18" },
>>> +};
>>> +
>>> +static const struct stm32_adc_chan_spec stm32f4_adc23_channels[] = {
>>> +    /* slave ADC2 /    ADC3 */
>>> +    { IIO_VOLTAGE, 0, "in0" },
>>> +    { IIO_VOLTAGE, 1, "in1" },
>>> +    { IIO_VOLTAGE, 2, "in2" },
>>> +    { IIO_VOLTAGE, 3, "in3" },
>>> +    { IIO_VOLTAGE, 4, "in4" },
>>> +    { IIO_VOLTAGE, 5, "in5" },
>>> +    { IIO_VOLTAGE, 6, "in6" },
>>> +    { IIO_VOLTAGE, 7, "in7" },
>>> +    { IIO_VOLTAGE, 8, "in8" },
>>> +    { IIO_VOLTAGE, 9, "in9" },
>>> +    { IIO_VOLTAGE, 10, "in10" },
>>> +    { IIO_VOLTAGE, 11, "in11" },
>>> +    { IIO_VOLTAGE, 12, "in12" },
>>> +    { IIO_VOLTAGE, 13, "in13" },
>>> +    { IIO_VOLTAGE, 14, "in14" },
>>> +    { IIO_VOLTAGE, 15, "in15" },
>>> +};
>>> +
>>> +/* Triggers for regular channels */
>>> +static const struct stm32_adc_trig_info stm32f4_adc_ext_triggers[] = {
>>> +    { STM32_EXT0, "TIM1_CH1" },
>>> +    { STM32_EXT1, "TIM1_CH2" },
>>> +    { STM32_EXT2, "TIM1_CH3" },
>>> +    { STM32_EXT3, "TIM2_CH2" },
>>> +    { STM32_EXT4, "TIM2_CH3" },
>>> +    { STM32_EXT5, "TIM2_CH4" },
>>> +    { STM32_EXT6, "TIM2_TRGO" },
>>> +    { STM32_EXT7, "TIM3_CH1" },
>>> +    { STM32_EXT8, "TIM3_TRGO" },
>>> +    { STM32_EXT9, "TIM4_CH4" },
>>> +    { STM32_EXT10, "TIM5_CH1" },
>>> +    { STM32_EXT11, "TIM5_CH2" },
>>> +    { STM32_EXT12, "TIM5_CH3" },
>>> +    { STM32_EXT13, "TIM8_CH1" },
>>> +    { STM32_EXT14, "TIM8_TRGO" },
>>> +    { STM32_EXT15, "EXTI_11" },
>>> +    {},
>>> +};
>>> +
>>> +/* Triggers for injected channels */
>>> +static const struct stm32_adc_trig_info  stm32f4_adc_jext_triggers[] = {
>>> +    { STM32_JEXT0, "TIM1_CH4" },
>>> +    { STM32_JEXT1, "TIM1_TRGO" },
>>> +    { STM32_JEXT2, "TIM2_CH1" },
>>> +    { STM32_JEXT3, "TIM2_TRGO" },
>>> +    { STM32_JEXT4, "TIM3_CH2" },
>>> +    { STM32_JEXT5, "TIM3_CH4" },
>>> +    { STM32_JEXT6, "TIM4_CH1" },
>>> +    { STM32_JEXT7, "TIM4_CH2" },
>>> +    { STM32_JEXT8, "TIM4_CH3" },
>>> +    { STM32_JEXT9, "TIM4_TRGO" },
>>> +    { STM32_JEXT10, "TIM5_CH4" },
>>> +    { STM32_JEXT11, "TIM5_TRGO" },
>>> +    { STM32_JEXT12, "TIM8_CH2" },
>>> +    { STM32_JEXT13, "TIM8_CH3" },
>>> +    { STM32_JEXT14, "TIM8_CH4" },
>>> +    { STM32_JEXT15, "EXTI_15" },
>>> +    {},
>>> +};
>>> +
>>> +static const struct stm32_adc_info stm32f4_adc_info[] = {
>>> +    {
>>> +        .name = "adc1-master",
>>> +        .reg = 0x0,
>>> +        .channels = stm32f4_adc1_channels,
>>> +        .max_channels = ARRAY_SIZE(stm32f4_adc1_channels),
>>> +    },
>>> +    {
>>> +        .name = "adc2-slave",
>>> +        .reg = 0x100,
>>> +        .channels = stm32f4_adc23_channels,
>>> +        .max_channels = ARRAY_SIZE(stm32f4_adc23_channels),
>>> +    },
>>> +    {
>>> +        .name = "adc3-slave",
>>> +        .reg = 0x200,
>>> +        .channels = stm32f4_adc23_channels,
>>> +        .max_channels = ARRAY_SIZE(stm32f4_adc23_channels),
>>> +    },
>>> +    {},
>>> +};
>>> +
>>> +/**
>>> + * stm32f4_sqr_regs - describe regular sequence registers
>>> + * - L: sequence len (register & bit field)
>>> + * - SQ1..SQ16: sequence entries (register & bit field)
>>> + */
>>> +static const struct stm32_adc_regs stm32f4_sqr_regs[STM32_ADC_MAX_SQ + 1] = {
>>> +    /* L: len bit field description to be kept as first element */
>>> +    { STM32F4_ADCX_SQR1, STM32F4_L_MASK, STM32F4_L_SHIFT },
>>> +    /* SQ1..SQ16 registers & bit fields */
>>> +    { STM32F4_ADCX_SQR3, STM32F4_SQ1_MASK, STM32F4_SQ1_SHIFT },
>>> +    { STM32F4_ADCX_SQR3, STM32F4_SQ2_MASK, STM32F4_SQ2_SHIFT },
>>> +    { STM32F4_ADCX_SQR3, STM32F4_SQ3_MASK, STM32F4_SQ3_SHIFT },
>>> +    { STM32F4_ADCX_SQR3, STM32F4_SQ4_MASK, STM32F4_SQ4_SHIFT },
>>> +    { STM32F4_ADCX_SQR3, STM32F4_SQ5_MASK, STM32F4_SQ5_SHIFT },
>>> +    { STM32F4_ADCX_SQR3, STM32F4_SQ6_MASK, STM32F4_SQ6_SHIFT },
>>> +    { STM32F4_ADCX_SQR2, STM32F4_SQ7_MASK, STM32F4_SQ7_SHIFT },
>>> +    { STM32F4_ADCX_SQR2, STM32F4_SQ8_MASK, STM32F4_SQ8_SHIFT },
>>> +    { STM32F4_ADCX_SQR2, STM32F4_SQ9_MASK, STM32F4_SQ9_SHIFT },
>>> +    { STM32F4_ADCX_SQR2, STM32F4_SQ10_MASK, STM32F4_SQ10_SHIFT },
>>> +    { STM32F4_ADCX_SQR2, STM32F4_SQ11_MASK, STM32F4_SQ11_SHIFT },
>>> +    { STM32F4_ADCX_SQR2, STM32F4_SQ12_MASK, STM32F4_SQ12_SHIFT },
>>> +    { STM32F4_ADCX_SQR1, STM32F4_SQ13_MASK, STM32F4_SQ13_SHIFT },
>>> +    { STM32F4_ADCX_SQR1, STM32F4_SQ14_MASK, STM32F4_SQ14_SHIFT },
>>> +    { STM32F4_ADCX_SQR1, STM32F4_SQ15_MASK, STM32F4_SQ15_SHIFT },
>>> +    { STM32F4_ADCX_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
>>> +};
>>> +
>>> +/**
>>> + * stm32f4_jsqr_reg - describe injected sequence register:
>>> + * - JL: injected sequence len
>>> + * - JSQ4..SQ1: sequence entries
>>> + * When JL == 3, ADC converts JSQ1, JSQ2, JSQ3, JSQ4
>>> + * When JL == 2, ADC converts JSQ2, JSQ3, JSQ4
>>> + * When JL == 1, ADC converts JSQ3, JSQ4
>>> + * When JL == 0, ADC converts JSQ4
>>> + */
>>> +static const struct stm32_adc_regs stm32f4_jsqr_reg[STM32_ADC_MAX_JSQ + 1] = {
>>> +    /* JL: len bit field description to be kept as first element */
>>> +    {STM32F4_ADCX_JSQR, STM32F4_JL_MASK, STM32F4_JL_SHIFT},
>>> +    /* JSQ4..JSQ1 registers & bit fields */
>>> +    {STM32F4_ADCX_JSQR, STM32F4_JSQ4_MASK, STM32F4_JSQ4_SHIFT},
>>> +    {STM32F4_ADCX_JSQR, STM32F4_JSQ3_MASK, STM32F4_JSQ3_SHIFT},
>>> +    {STM32F4_ADCX_JSQR, STM32F4_JSQ2_MASK, STM32F4_JSQ2_SHIFT},
>>> +    {STM32F4_ADCX_JSQR, STM32F4_JSQ1_MASK, STM32F4_JSQ1_SHIFT},
>>> +};
>>> +
>>> +static const struct stm32_adc_trig_reginfo stm32f4_adc_trig_reginfo = {
>>> +    .reg = STM32F4_ADCX_CR2,
>>> +    .exten_mask = STM32F4_EXTEN_MASK,
>>> +    .exten_shift = STM32F4_EXTEN_SHIFT,
>>> +    .extsel_mask = STM32F4_EXTSEL_MASK,
>>> +    .extsel_shift = STM32F4_EXTSEL_SHIFT,
>>> +};
>>> +
>>> +static const struct stm32_adc_trig_reginfo stm32f4_adc_jtrig_reginfo = {
>>> +    .reg = STM32F4_ADCX_CR2,
>>> +    .exten_mask = STM32F4_JEXTEN_MASK,
>>> +    .exten_shift = STM32F4_JEXTEN_SHIFT,
>>> +    .extsel_mask = STM32F4_JEXTSEL_MASK,
>>> +    .extsel_shift = STM32F4_JEXTSEL_SHIFT,
>>> +};
>>> +
>>> +static const struct stm32_adc_reginfo stm32f4_adc_reginfo = {
>>> +    .isr = STM32F4_ADCX_SR,
>>> +    .eoc = STM32F4_EOC,
>>> +    .jeoc = STM32F4_JEOC,
>>> +    .ier = STM32F4_ADCX_CR1,
>>> +    .eocie = STM32F4_EOCIE,
>>> +    .jeocie = STM32F4_JEOCIE,
>>> +    .dr = STM32F4_ADCX_DR,
>>> +    .jdr = {
>>> +        STM32F4_ADCX_JDR1,
>>> +        STM32F4_ADCX_JDR2,
>>> +        STM32F4_ADCX_JDR3,
>>> +        STM32F4_ADCX_JDR4,
>>> +    },
>>> +    .sqr_regs = stm32f4_sqr_regs,
>>> +    .jsqr_reg = stm32f4_jsqr_reg,
>>> +    .trig_reginfo = &stm32f4_adc_trig_reginfo,
>>> +    .jtrig_reginfo = &stm32f4_adc_jtrig_reginfo,
>>> +};
>>> +
>>> +static bool stm32f4_adc_is_started(struct stm32_adc *adc)
>>> +{
>>> +    u32 val = stm32_adc_readl(adc, STM32F4_ADCX_CR2) & STM32F4_ADON;
>>> +
>>> +    return !!val;
>>> +}
>>> +
>>> +static bool stm32f4_adc_regular_started(struct stm32_adc *adc)
>>> +{
>>> +    u32 val = stm32_adc_readl(adc, STM32F4_ADCX_SR) & STM32F4_STRT;
>>> +
>>> +    return !!val;
>>> +}
>>> +
>>> +static bool stm32f4_adc_injected_started(struct stm32_adc *adc)
>>> +{
>>> +    u32 val = stm32_adc_readl(adc, STM32F4_ADCX_SR) & STM32F4_JSTRT;
>>> +
>>> +    return !!val;
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_adc_start_conv() - Start regular or injected conversions
>>> + * @adc: stm32 adc instance
>>> + *
>>> + * Start single conversions for regular or injected channels.
>>> + */
>>> +static int stm32f4_adc_start_conv(struct stm32_adc *adc)
>>> +{
>>> +    u32 trig_msk, start_msk;
>>> +
>>> +    dev_dbg(adc->common->dev, "%s %s\n", __func__,
>>> +        adc->injected ? "injected" : "regular");
>>> +
>>> +    stm32_adc_set_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
>>> +
>>> +    if (!stm32f4_adc_is_started(adc)) {
>>> +        stm32_adc_set_bits(adc, STM32F4_ADCX_CR2,
>>> +                   STM32F4_EOCS | STM32F4_ADON);
>>> +
>>> +        /* Wait for Power-up time (tSTAB from datasheet) */
>>> +        usleep_range(2, 3);
>>> +    }
>>> +
>>> +    if (adc->injected) {
>>> +        trig_msk = STM32F4_JEXTEN_MASK;
>>> +        start_msk = STM32F4_JSWSTART;
>>> +    } else {
>>> +        trig_msk = STM32F4_EXTEN_MASK;
>>> +        start_msk = STM32F4_SWSTART;
>>> +    }
>>> +
>>> +    /* Software start ? (e.g. trigger detection disabled ?) */
>>> +    if (!(stm32_adc_readl(adc, STM32F4_ADCX_CR2) & trig_msk))
>>> +        stm32_adc_set_bits(adc, STM32F4_ADCX_CR2, start_msk);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int stm32f4_adc_stop_conv(struct stm32_adc *adc)
>>> +{
>>> +    u32 val;
>>> +
>>> +    dev_dbg(adc->common->dev, "%s %s\n", __func__,
>>> +        adc->injected ? "injected" : "regular");
>>> +
>>> +    /* First disable trigger for either regular or injected channels */
>>> +    if (adc->injected) {
>>> +        stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_JEXTEN_MASK);
>>> +        stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_JSTRT);
>>> +    } else {
>>> +        stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_EXTEN_MASK);
>>> +        stm32_adc_clr_bits(adc, STM32F4_ADCX_SR, STM32F4_STRT);
>>> +    }
>>> +
>>> +    /* Disable adc when all triggered conversion have been disabled */
>>> +    val = stm32_adc_readl(adc, STM32F4_ADCX_CR2);
>>> +    val &= STM32F4_EXTEN_MASK | STM32F4_JEXTEN_MASK;
>>> +    if (!val) {
>>> +        stm32_adc_clr_bits(adc, STM32F4_ADCX_CR1, STM32F4_SCAN);
>>> +        stm32_adc_clr_bits(adc, STM32F4_ADCX_CR2, STM32F4_ADON);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +/* ADC internal common clock prescaler division ratios */
>>> +static int stm32f4_pclk_div[] = {2, 4, 6, 8};
>>> +
>>> +/**
>>> + * stm32f4_adc_clk_sel() - Select ADC common clock prescaler
>>> + * @adc: stm32 adc instance
>>> + * Select clock prescaler used for analog conversions.
>>> + */
>>> +static int stm32f4_adc_clk_sel(struct stm32_adc *adc)
>>> +{
>>> +    struct stm32_adc_common *common = adc->common;
>>> +    unsigned long rate;
>>> +    u32 val;
>>> +    int i;
>>> +
>>> +    /* Common prescaler is set only once, when 1st ADC instance starts */
>>> +    list_for_each_entry(adc, &common->adc_list, adc_list)
>>> +        if (stm32f4_adc_is_started(adc))
>>> +            return 0;
>>> +
>>> +    rate = clk_get_rate(common->aclk);
>>> +    for (i = 0; i < ARRAY_SIZE(stm32f4_pclk_div); i++) {
>>> +        if ((rate / stm32f4_pclk_div[i]) <=
>>> +            common->data->max_clock_rate)
>>> +            break;
>>> +    }
>>> +    if (i >= ARRAY_SIZE(stm32f4_pclk_div))
>>> +        return -EINVAL;
>>> +
>>> +    val = stm32_adc_common_readl(common, STM32F4_ADC_CCR);
>>> +    val &= ~STM32F4_ADC_ADCPRE_MASK;
>>> +    val |= i << STM32F4_ADC_ADCPRE_SHIFT;
>>> +    stm32_adc_common_writel(common, STM32F4_ADC_CCR, val);
>>> +
>>> +    dev_dbg(common->dev, "Using analog clock source at %ld kHz\n",
>>> +        rate / (stm32f4_pclk_div[i] * 1000));
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct stm32_adc_ops stm32f4_adc_ops = {
>>> +    .adc_info = stm32f4_adc_info,
>>> +    .ext_triggers = stm32f4_adc_ext_triggers,
>>> +    .jext_triggers = stm32f4_adc_jext_triggers,
>>> +    .adc_reginfo = &stm32f4_adc_reginfo,
>>> +    .highres = 12,
>>> +    .max_clock_rate = 36000000,
>>> +    .clk_sel = stm32f4_adc_clk_sel,
>>> +    .start_conv = stm32f4_adc_start_conv,
>>> +    .stop_conv = stm32f4_adc_stop_conv,
>>> +    .is_started = stm32f4_adc_is_started,
>>> +    .regular_started = stm32f4_adc_regular_started,
>>> +    .injected_started = stm32f4_adc_injected_started,
>>> +};
>>> +
>>> +static const struct of_device_id stm32f4_adc_of_match[] = {
>>> +    { .compatible = "st,stm32f4-adc", .data = (void *)&stm32f4_adc_ops},
>>> +    {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, stm32f4_adc_of_match);
>>> +
>>> +static struct platform_driver stm32f4_adc_driver = {
>>> +    .probe = stm32_adc_probe,
>>> +    .remove = stm32_adc_remove,
>>> +    .driver = {
>>> +        .name = "stm32f4-adc",
>>> +        .of_match_table = stm32f4_adc_of_match,
>>> +    },
>>> +};
>>> +
>>> +module_platform_driver(stm32f4_adc_driver);
>>> +
>>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>");
>>> +MODULE_DESCRIPTION("STMicroelectronics STM32F4 ADC driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
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 2/2] arm64: dts: sunxi: enable EHCI1, OHCI1 and USB PHY nodes in Pine64
From: Icenowy Zheng @ 2016-11-05 14:38 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Hans de Goede,
	Andre Przywara
  Cc: Mark Rutland, devicetree, Catalin Marinas, Will Deacon,
	linux-kernel, Icenowy Zheng, linux-arm-kernel
In-Reply-To: <20161105143830.10099-1-icenowy@aosc.xyz>

Pine64 have two USB Type-A ports, which are wired to the two ports of
A64 USB PHY, and the lower port is the EHCI/OHCI1 port.

Enable the necessary nodes to enable the lower USB port to work.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index 4709590..d836995 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -72,3 +72,15 @@
 &i2c1_pins {
 	bias-pull-up;
 };
+
+&usbphy {
+	status = "okay";
+};
+
+&ehci1 {
+	status = "okay";
+};
+
+&ohci1 {
+	status = "okay";
+};
-- 
2.10.1

^ permalink raw reply related

* [PATCH 1/2] arm64: dts: add USB1-related nodes of Allwinner A64
From: Icenowy Zheng @ 2016-11-05 14:38 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Hans de Goede,
	Andre Przywara
  Cc: Mark Rutland, devicetree, Catalin Marinas, Will Deacon,
	linux-kernel, Icenowy Zheng, linux-arm-kernel

Allwinner A64 have two HCI USB controllers, a OTG controller and a USB
PHY device which have two ports. One of the port is wired to both a HCI
USB controller and the OTG controller, which is currently not supported.
The another one is only wired to a HCI controller, and the device node of
OHCI/EHCI controller of the port can be added now.

Also the A64 USB PHY device node is also added for the HCI controllers to
work.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 50 +++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 3d70be3..c2b6dc8 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -259,5 +259,55 @@
 			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
 		};
+
+		usbphy: phy@01c19400 {
+			compatible = "allwinner,sun50i-a64-usb-phy";
+			reg = <0x01c19400 0x14>,
+			      <0x01c1b800 0x4>;
+			reg-names = "phy_ctrl",
+				    "pmu1";
+			clocks = <&ccu CLK_USB_PHY0>,
+				 <&ccu CLK_USB_PHY1>;
+			clock-names = "usb0_phy",
+				      "usb1_phy";
+			resets = <&ccu RST_USB_PHY0>,
+				 <&ccu RST_USB_PHY1>;
+			reset-names = "usb0_reset",
+				      "usb1_reset";
+			status = "disabled";
+			#phy-cells = <1>;
+		};
+
+		ohci1: usb@01c1a400 {
+			compatible = "allwinner,sun50i-a64-ohci", "generic-ohci";
+			reg = <0x01c1b400 0x100>;
+			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;
+			/*
+			 * According to the user manual, OHCI1 USB clock
+			 * depends on OHCI0 clock.
+			 */
+			clocks = <&ccu CLK_BUS_OHCI1>,
+				 <&ccu CLK_USB_OHCI0>,
+				 <&ccu CLK_USB_OHCI1>;
+			resets = <&ccu RST_BUS_OHCI1>;
+			phys = <&usbphy 1>;
+			phy-names = "usb";
+			status = "disabled";
+		};
+
+		ehci1: usb@01c1a000 {
+			compatible = "allwinner,sun50i-a64-ehci", "generic-ehci";
+			reg = <0x01c1b000 0x100>;
+			interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_OHCI1>,
+				 <&ccu CLK_BUS_EHCI1>,
+				 <&ccu CLK_USB_OHCI0>,
+				 <&ccu CLK_USB_OHCI1>;
+			resets = <&ccu RST_BUS_OHCI1>,
+				 <&ccu RST_BUS_EHCI1>;
+			phys = <&usbphy 1>;
+			phy-names = "usb";
+			status = "disabled";
+		};
 	};
 };
-- 
2.10.1

^ permalink raw reply related

* Re: [PATCH] ARM: dts: imx6: Add imx-weim parameters to dtsi's
From: Shawn Guo @ 2016-11-05 11:47 UTC (permalink / raw)
  To: Joshua Clayton
  Cc: Sascha Hauer, Fabio Estevam, Rob Herring, Mark Rutland,
	Russell King,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <1478044305-5666-1-git-send-email-stillcompiling@gmail.com>

On Tue, Nov 01, 2016 at 04:51:45PM -0700, Joshua Clayton wrote:
> imx-weim should always set address-cells to 2,
> and size_cells to 1.
> On imx6, fsl,weim-cs-gpr will always be &gpr
> 
> Set these common parameters in the dtsi file,
> rather than in a downstream dts.
> 
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [REGRESSION] "console: don't prefer first registered if DT specifies stdout-path" breaks console on video outputs of various ARM boards
From: Paul Burton @ 2016-11-05 11:00 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Linus Torvalds, Andrew Morton, Rob Herring, Frank Rowand,
	Thorsten Leemhuis, Greg Kroah-Hartman, Tejun Heo, devicetree,
	linux-kernel
In-Reply-To: <d3f0f1f4-d6f6-1e75-499c-789b82b5bb1b@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4562 bytes --]

On Friday, 4 November 2016 14:22:17 GMT Hans de Goede wrote:
> Hi,
> 
> On 04-11-16 13:30, Paul Burton wrote:
> > Hi Hans,
> > 
> > On Friday, 4 November 2016 13:11:34 GMT Hans de Goede wrote:
> >> Hi All,
> >> 
> >> While booting 4.9-rc# for the first time on an Allwinner A33 tablet,
> >> I noticed that after u-boot the LCD display stayed black. It turns out
> >> that there was an issue which caused X to never get up, and all kernel
> >> (and other startup) messages prior to that only went to ttyS0 which
> >> consists of 2 tiny testpads on the PCB with this tablet.
> >> 
> >> The same issue will also happen on any ARM boards which have a HDMI or
> >> composite video output and which use a stdout-path pointing to their
> >> serial console. I think this will e.g. also impact the Raspberry Pi,
> >> I know for certain that this will impact the 99 different Allwinnner
> >> boards currently supported by mainline u-boot + the mainline kernel.
> >> 
> >> This is a behavior changes from previous kernels and I consider this
> >> a regression. Thus I propose to revert the commit in question, for more
> >> info here is a partial copy of the commit message of the proposed revert:
> >> 
> >> The reverted commit changes existing behavior on which many ARM boards
> >> rely. Many ARM small-board-computers, like e.g. the Raspberry Pi have
> >> both a video output and a serial console. Depending on whether the user
> >> is using the device as a more regular computer; or as a headless device
> >> we need to have the console on either one or the other.
> >> 
> >> Many users rely on the kernel behavior of the console being present on
> >> both outputs, before the reverted commit the console setup with no
> >> console= kernel arguments on an ARM board which sets stdout-path in dt
> >> would look like this:
> >> 
> >> [root@localhost ~]# cat /proc/consoles
> >> ttyS0                -W- (EC p a)    4:64
> >> tty0                 -WU (E  p  )    4:1
> >> 
> >> Where as after the reverted commit, it looks like this:
> >> 
> >> [root@localhost ~]# cat /proc/consoles
> >> ttyS0                -W- (EC p a)    4:64
> >> 
> >> This commit reverts commit 05fd007e4629 ("console: don't prefer first
> >> registered if DT specifies stdout-path") restoring the original behavior.
> >> 
> >> Regards,
> >> 
> >> Hans
> > 
> > Ugh... so the devices you're talking about rely upon set stdout-path in
> > their device tree but effectively rely upon us ignoring it?
> 
> No they rely on the kernel using stdout-path as an extra console while
> keeping tty0 as console, not ignoring it. This how stdout-path has always
> worked (at least as long as the Allwinner boards have used it, which has
> been 2 - 3 years now).
> 
> If you want only the console specified by stdout-path you can get this by
> specifying it with console=... on the kernel cmdline.
> 
> > If that's the case then I guess reverting is probably the best option, but
> > it does restore us to a position where we honor stdout-path for earlycon
> > & then essentially ignore it for the proper kernel console. That seems
> > pretty bust to me...
> 
> We do not ignore it, we use both the tty pointed to by stdout-path and tty0.
> 
> Regards,
> 
> Hans

Hi Hans,

Could you walk me though how you're getting that behaviour from the current 
code? I don't see how that would happen besides perhaps if drivers are probed 
in a fortunate order. Is that what you're relying upon?

What I see in my systems, and what 05fd007e4629 ("console: don't prefer first 
registered if DT specifies stdout-path") addressed, is that if there are for 
example 2 UARTs uart0 & uart1 that are probed in that order and stdout-path 
indicates that we should use uart1 we wind up essentially ignoring it because 
the ordering of the relevant calls goes:

  - of_console_check() for uart0
  - add_preferred_console() for uart0
  - register_console() for uart0
  - of_console_check() for uart1
  - add_preferred_console() for uart1
  - register_console() for uart1

Since of_check_console() doesn't get called for uart1 until after uart0 has 
been probed, we don't add an entry for it to the console_cmdline array until 
after register_console() has already decided to enable uart0 because 
preferred_console == -1.

I'm not the only one seeing this oddity either, for example see the discussion 
on this patch:

https://patchwork.kernel.org/patch/9263753/

By simply reverting my patch you restore us to a position where so far as I 
can see we simply do not honor stdout-path for the real kernel console.

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [REGRESSION] "console: don't prefer first registered if DT specifies stdout-path" breaks console on video outputs of various ARM boards
From: Paul Burton @ 2016-11-05 10:40 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Linus Torvalds, Andrew Morton, Rob Herring, Frank Rowand,
	Thorsten Leemhuis, Greg Kroah-Hartman, Tejun Heo, devicetree,
	linux-kernel
In-Reply-To: <d3f0f1f4-d6f6-1e75-499c-789b82b5bb1b@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4562 bytes --]

On Friday, 4 November 2016 14:22:17 GMT Hans de Goede wrote:
> Hi,
> 
> On 04-11-16 13:30, Paul Burton wrote:
> > Hi Hans,
> > 
> > On Friday, 4 November 2016 13:11:34 GMT Hans de Goede wrote:
> >> Hi All,
> >> 
> >> While booting 4.9-rc# for the first time on an Allwinner A33 tablet,
> >> I noticed that after u-boot the LCD display stayed black. It turns out
> >> that there was an issue which caused X to never get up, and all kernel
> >> (and other startup) messages prior to that only went to ttyS0 which
> >> consists of 2 tiny testpads on the PCB with this tablet.
> >> 
> >> The same issue will also happen on any ARM boards which have a HDMI or
> >> composite video output and which use a stdout-path pointing to their
> >> serial console. I think this will e.g. also impact the Raspberry Pi,
> >> I know for certain that this will impact the 99 different Allwinnner
> >> boards currently supported by mainline u-boot + the mainline kernel.
> >> 
> >> This is a behavior changes from previous kernels and I consider this
> >> a regression. Thus I propose to revert the commit in question, for more
> >> info here is a partial copy of the commit message of the proposed revert:
> >> 
> >> The reverted commit changes existing behavior on which many ARM boards
> >> rely. Many ARM small-board-computers, like e.g. the Raspberry Pi have
> >> both a video output and a serial console. Depending on whether the user
> >> is using the device as a more regular computer; or as a headless device
> >> we need to have the console on either one or the other.
> >> 
> >> Many users rely on the kernel behavior of the console being present on
> >> both outputs, before the reverted commit the console setup with no
> >> console= kernel arguments on an ARM board which sets stdout-path in dt
> >> would look like this:
> >> 
> >> [root@localhost ~]# cat /proc/consoles
> >> ttyS0                -W- (EC p a)    4:64
> >> tty0                 -WU (E  p  )    4:1
> >> 
> >> Where as after the reverted commit, it looks like this:
> >> 
> >> [root@localhost ~]# cat /proc/consoles
> >> ttyS0                -W- (EC p a)    4:64
> >> 
> >> This commit reverts commit 05fd007e4629 ("console: don't prefer first
> >> registered if DT specifies stdout-path") restoring the original behavior.
> >> 
> >> Regards,
> >> 
> >> Hans
> > 
> > Ugh... so the devices you're talking about rely upon set stdout-path in
> > their device tree but effectively rely upon us ignoring it?
> 
> No they rely on the kernel using stdout-path as an extra console while
> keeping tty0 as console, not ignoring it. This how stdout-path has always
> worked (at least as long as the Allwinner boards have used it, which has
> been 2 - 3 years now).
> 
> If you want only the console specified by stdout-path you can get this by
> specifying it with console=... on the kernel cmdline.
> 
> > If that's the case then I guess reverting is probably the best option, but
> > it does restore us to a position where we honor stdout-path for earlycon
> > & then essentially ignore it for the proper kernel console. That seems
> > pretty bust to me...
> 
> We do not ignore it, we use both the tty pointed to by stdout-path and tty0.
> 
> Regards,
> 
> Hans

Hi Hans,

Could you walk me though how you're getting that behaviour from the current 
code? I don't see how that would happen besides perhaps if drivers are probed 
in a fortunate order. Is that what you're relying upon?

What I see in my systems, and what 05fd007e4629 ("console: don't prefer first 
registered if DT specifies stdout-path") addressed, is that if there are for 
example 2 UARTs uart0 & uart1 that are probed in that order and stdout-path 
indicates that we should use uart1 we wind up essentially ignoring it because 
the ordering of the relevant calls goes:

  - of_console_check() for uart0
  - add_preferred_console() for uart0
  - register_console() for uart0
  - of_console_check() for uart1
  - add_preferred_console() for uart1
  - register_console() for uart1

Since of_check_console() doesn't get called for uart1 until after uart0 has 
been probed, we don't add an entry for it to the console_cmdline array until 
after register_console() has already decided to enable uart0 because 
preferred_console == -1.

I'm not the only one seeing this oddity either, for example see the discussion 
on this patch:

https://patchwork.kernel.org/patch/9263753/

By simply reverting my patch you restore us to a position where so far as I 
can see we simply do not honor stdout-path for the real kernel console.

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v6 7/7] arm64: dts: NS2: add AMAC ethernet support
From: Sergei Shtylyov @ 2016-11-05 10:33 UTC (permalink / raw)
  To: Jon Mason
  Cc: David Miller, Rob Herring, Mark Rutland, Florian Fainelli, rafal,
	bcm-kernel-feedback-list, netdev, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20161104163024.GA6128@broadcom.com>

On 11/4/2016 7:30 PM, Jon Mason wrote:

>>> Add support for the AMAC ethernet to the Broadcom Northstar2 SoC device
>>> tree
>>>
>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>> ---
>>> arch/arm64/boot/dts/broadcom/ns2-svk.dts |  5 +++++
>>> arch/arm64/boot/dts/broadcom/ns2.dtsi    | 12 ++++++++++++
>>> 2 files changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/broadcom/ns2-svk.dts b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
>>> index b09f3bc..c4d5442 100644
>>> --- a/arch/arm64/boot/dts/broadcom/ns2-svk.dts
>>> +++ b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
>>> @@ -56,6 +56,10 @@
>>> 	};
>>> };
>>>
>>> +&enet {
>>> +	status = "ok";
>>
>>    The spec dictates it should be "okay" (although "ok" is also recognized).
>
> The rest of the file uses "ok".  So, the addition above is consistent
> with the other entries.
>
> Perhaps a patch outside this series to convert the entire file from
> "ok" to "okay" would be acceptable to you.

    OK, it would...

>
> Thanks,
> Jon

MBR, Sergei

^ permalink raw reply

* Re: [PATCH 3/3] clk: imx: clk-imx6ul: add clk support for imx6ull
From: Shawn Guo @ 2016-11-05  9:05 UTC (permalink / raw)
  To: Peter Chen
  Cc: sboyd-sgV2jX0FEOL9JmXXK+q4OQ, mturquette-rdvid1DuHRBWk0Htik3J/w,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, fabio.estevam-3arQi8VN3Tc,
	mark.rutland-5wv7dgnIgG8, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	Bai Ping, Peng Fan
In-Reply-To: <1477969343-19887-4-git-send-email-peter.chen-3arQi8VN3Tc@public.gmane.org>

On Tue, Nov 01, 2016 at 11:02:23AM +0800, Peter Chen wrote:
> From: Bai Ping <ping.bai-3arQi8VN3Tc@public.gmane.org>
> 
> imx6ull is the derived SoC from imx6ul
> 
> Cc: Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Peng Fan <peng.fan-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Bai Ping <ping.bai-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
> ---
>  drivers/clk/imx/clk-imx6ul.c             | 74 +++++++++++++++++++++++++++-----
>  include/dt-bindings/clock/imx6ul-clock.h | 15 ++++++-
>  2 files changed, 77 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
> index d1d7787..ceb99a7 100644
> --- a/drivers/clk/imx/clk-imx6ul.c
> +++ b/drivers/clk/imx/clk-imx6ul.c
> @@ -64,6 +64,11 @@ static const char *perclk_sels[] = { "ipg", "osc", };
>  static const char *lcdif_sels[] = { "lcdif_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", };
>  static const char *csi_sels[] = { "osc", "pll2_pfd2_396m", "pll3_120m", "pll3_pfd1_540m", };
>  static const char *sim_sels[] = { "sim_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", };
> +/* epdc_pre_sels, epdc_sels, esai_sels only exists on i.MX6ULL */
> +static const char *epdc_pre_sels[] = { "pll2_bus", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd2_508m", };
> +static const char *esai_sels[] = { "pll4_audio_div", "pll3_pfd2_508m", "pll5_video_div", "pll3_usb_otg", };
> +static const char *epdc_sels[] = { "epdc_podf", "ipp_di0", "ipp_di1", "ldb_di0", "ldb_di1", };
> +

The new line is not needed.  There is already one below.

>  
>  static struct clk *clks[IMX6UL_CLK_END];
>  static struct clk_onecell_data clk_data;

<snip>

> diff --git a/include/dt-bindings/clock/imx6ul-clock.h b/include/dt-bindings/clock/imx6ul-clock.h
> index fd8aee8..563fd5b 100644
> --- a/include/dt-bindings/clock/imx6ul-clock.h
> +++ b/include/dt-bindings/clock/imx6ul-clock.h
> @@ -236,6 +236,19 @@
>  #define IMX6UL_CLK_PLL3_120M		223
>  #define IMX6UL_CLK_KPP			224
>  
> -#define IMX6UL_CLK_END			225
> +/* For i.MX6ULL */
> +#define IMX6UL_CLK_ESAI_PRED		225
> +#define IMX6UL_CLK_ESAI_PODF		226
> +#define IMX6UL_CLK_ESAI_EXTAL		227
> +#define IMX6UL_CLK_ESAI_MEM		228
> +#define IMX6UL_CLK_ESAI_IPG		229
> +#define IMX6UL_CLK_DCP_CLK		230
> +#define IMX6UL_CLK_EPDC_PRE_SEL		231
> +#define IMX6UL_CLK_EPDC_SEL		232
> +#define IMX6UL_CLK_EPDC_PODF		233
> +#define IMX6UL_CLK_EPDC_ACLK		234
> +#define IMX6UL_CLK_EPDC_PIX		235
> +#define IMX6UL_CLK_ESAI_SEL		236

Can we have these imx6ull only clocks named after IMX6ULL_xxx?

Shawn

> +#define IMX6UL_CLK_END			237
>  
>  #endif /* __DT_BINDINGS_CLOCK_IMX6UL_H */
> -- 
> 2.7.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

* Re: [PATCH 1/3] ARM: imx6u: add imx6ull support
From: Shawn Guo @ 2016-11-05  8:59 UTC (permalink / raw)
  To: Peter Chen
  Cc: sboyd-sgV2jX0FEOL9JmXXK+q4OQ, mturquette-rdvid1DuHRBWk0Htik3J/w,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	fabio.estevam-3arQi8VN3Tc, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1477969343-19887-2-git-send-email-peter.chen-3arQi8VN3Tc@public.gmane.org>

On Tue, Nov 01, 2016 at 11:02:21AM +0800, Peter Chen wrote:
> It is the 10th processor in the well-known imx6 series, and derived
> from imx6ul but cost optimized. The more information about imx6ull
> can be found at:
> 
> http://www.nxp.com/products/microcontrollers-and-processors/
> arm-processors/i.mx-applications-processors/i.mx-6-processors
> /i.mx6qp/i.mx-6ull-single-core-processor-with-arm-cortex-a7-core
> :i.MX6ULL
> 
> In this patch, it moves common dts between imx6ul* and imx6ull* as
> new files named imx6u*, and the specific part for imx6ul and imx6ull
> are individual file.

No.  That only helps churn the tree and git history.  Please keep using
imx6ul.dtsi and simply have it be included by imx6ull.dtsi.

> 
> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile              |   3 +-
>  arch/arm/boot/dts/imx6u-14x14-evk.dts   | 475 ++++++++++++++++
>  arch/arm/boot/dts/imx6u-14x14-evk.dtsi  | 487 +++++++++++++++++
>  arch/arm/boot/dts/imx6u.dtsi            | 942 ++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/imx6ul-14x14-evk.dts  | 479 +---------------
>  arch/arm/boot/dts/imx6ul.dtsi           | 936 +------------------------------
>  arch/arm/boot/dts/imx6ull-14x14-evk.dts |  17 +
>  arch/arm/boot/dts/imx6ull-pinfunc.h     |  57 ++
>  arch/arm/boot/dts/imx6ull.dtsi          |  10 +
>  9 files changed, 1993 insertions(+), 1413 deletions(-)
>  create mode 100644 arch/arm/boot/dts/imx6u-14x14-evk.dts
>  create mode 100644 arch/arm/boot/dts/imx6u-14x14-evk.dtsi
>  create mode 100644 arch/arm/boot/dts/imx6u.dtsi
>  create mode 100644 arch/arm/boot/dts/imx6ull-14x14-evk.dts
>  create mode 100644 arch/arm/boot/dts/imx6ull-pinfunc.h
>  create mode 100644 arch/arm/boot/dts/imx6ull.dtsi
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..3d6e199 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -423,7 +423,8 @@ dtb-$(CONFIG_SOC_IMX6UL) += \
>  	imx6ul-pico-hobbit.dtb \
>  	imx6ul-tx6ul-0010.dtb \
>  	imx6ul-tx6ul-0011.dtb \
> -	imx6ul-tx6ul-mainboard.dtb
> +	imx6ul-tx6ul-mainboard.dtb \
> +	imx6ull-14x14-evk.dtb
>  dtb-$(CONFIG_SOC_IMX7D) += \
>  	imx7d-cl-som-imx7.dtb \
>  	imx7d-colibri-eval-v3.dtb \
> diff --git a/arch/arm/boot/dts/imx6u-14x14-evk.dts b/arch/arm/boot/dts/imx6u-14x14-evk.dts
> new file mode 100644
> index 0000000..ba8614c
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6u-14x14-evk.dts
> @@ -0,0 +1,475 @@
> +/*
> + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */

For new DT files, please consider to use GPL/X11 dual license.  You
should be able to find a lot of examples in arch/arm/boot/dts.

<snip>

> diff --git a/arch/arm/boot/dts/imx6ull-pinfunc.h b/arch/arm/boot/dts/imx6ull-pinfunc.h
> new file mode 100644
> index 0000000..fca0036
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull-pinfunc.h
> @@ -0,0 +1,57 @@
> +/*
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __DTS_IMX6ULL_PINFUNC_H
> +#define __DTS_IMX6ULL_PINFUNC_H
> +
> +#include "imx6ul-pinfunc.h"
> +/*
> + * The pin function ID is a tuple of
> + * <mux_reg conf_reg input_reg mux_mode input_val>
> + */
> +#define MX6UL_PAD_ENET2_RX_DATA0__EPDC_SDDO08                    0x00E4 0x0370 0x0000 0x9 0x0

Can we name these imx6ull specific defines MX6ULL_xxx, so that we know
they should only be used in imx6ull specific dts?

> +#define MX6UL_PAD_ENET2_RX_DATA1__EPDC_SDDO09                    0x00E8 0x0374 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_RX_EN__EPDC_SDDO10                       0x00EC 0x0378 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_TX_DATA0__EPDC_SDDO11                    0x00F0 0x037C 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_TX_DATA1__EPDC_SDDO12                    0x00F4 0x0380 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_TX_EN__EPDC_SDDO13                       0x00F8 0x0384 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_TX_CLK__EPDC_SDDO14                      0x00FC 0x0388 0x0000 0x9 0x0
> +#define MX6UL_PAD_ENET2_RX_ER__EPDC_SDDO15                       0x0100 0x038C 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_CLK__EPDC_SDCLK                            0x0104 0x0390 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_ENABLE__EPDC_SDLE                          0x0108 0x0394 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_HSYNC__EPDC_SDOE                           0x010C 0x0398 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_VSYNC__EPDC_SDCE0                          0x0110 0x039C 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_RESET__EPDC_GDOE                           0x0114 0x03A0 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA00__EPDC_SDDO00                        0x0118 0x03A4 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA01__EPDC_SDDO01                        0x011C 0x03A8 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA02__EPDC_SDDO02                        0x0120 0x03AC 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA03__EPDC_SDDO03                        0x0124 0x03B0 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA04__EPDC_SDDO04                        0x0128 0x03B4 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA05__EPDC_SDDO05                        0x012C 0x03B8 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA06__EPDC_SDDO06                        0x0130 0x03BC 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA07__EPDC_SDDO07                        0x0134 0x03C0 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA14__EPDC_SDSHR                         0x0150 0x03DC 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA15__EPDC_GDRL                          0x0154 0x03E0 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA16__EPDC_GDCLK                         0x0158 0x03E4 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA17__EPDC_GDSP                          0x015C 0x03E8 0x0000 0x9 0x0
> +#define MX6UL_PAD_LCD_DATA21__EPDC_SDCE1                         0x016C 0x03F8 0x0000 0x9 0x0
> +

Why this new line?

Shawn

> +#define MX6UL_PAD_CSI_MCLK__ESAI_TX3_RX2                          0x01D4 0x0460 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_PIXCLK__ESAI_TX2_RX3                        0x01D8 0x0464 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_VSYNC__ESAI_TX4_RX1                         0x01DC 0x0468 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_HSYNC__ESAI_TX1                             0x01E0 0x046C 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA00__ESAI_TX_HF_CLK                      0x01E4 0x0470 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA01__ESAI_RX_HF_CLK                      0x01E8 0x0474 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA02__ESAI_RX_FS                          0x01EC 0x0478 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA03__ESAI_RX_CLK                         0x01F0 0x047C 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA04__ESAI_TX_FS                          0x01F4 0x0480 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA05__ESAI_TX_CLK                         0x01F8 0x0484 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA06__ESAI_TX5_RX0                        0x01FC 0x0488 0x0000 0x9 0x0
> +#define MX6UL_PAD_CSI_DATA07__ESAI_T0                             0x0200 0x048C 0x0000 0x9 0x0
> +
> +#endif /* __DTS_IMX6ULL_PINFUNC_H */
> diff --git a/arch/arm/boot/dts/imx6ull.dtsi b/arch/arm/boot/dts/imx6ull.dtsi
> new file mode 100644
> index 0000000..afd9796
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull.dtsi
> @@ -0,0 +1,10 @@
> +/*
> + * Copyright 2016 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include "imx6u.dtsi"
> +#include "imx6ull-pinfunc.h"
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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 1/2] ARM: dts: imx6sx-sdb: update TX D_CAL for USBPHY
From: Shawn Guo @ 2016-11-05  8:31 UTC (permalink / raw)
  To: Peter Chen
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8
In-Reply-To: <1477882709-2518-1-git-send-email-peter.chen-3arQi8VN3Tc@public.gmane.org>

On Mon, Oct 31, 2016 at 10:58:28AM +0800, Peter Chen wrote:
> We need to change trimming value (as a percentage) of the 17.78mA TX
> reference current for better signal quality. With this change, we
> can patch the eye-diagram test on this board.

s/patch/pass?  I just want to confirm this is a typo, and can fix it up
when applying.

Shawn

> 
> Signed-off-by: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6sx-sdb.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6sx-sdb.dtsi b/arch/arm/boot/dts/imx6sx-sdb.dtsi
> index 85f56a5..4933c56 100644
> --- a/arch/arm/boot/dts/imx6sx-sdb.dtsi
> +++ b/arch/arm/boot/dts/imx6sx-sdb.dtsi
> @@ -317,6 +317,14 @@
>  	status = "okay";
>  };
>  
> +&usbphy1 {
> +	fsl,tx-d-cal = <106>;
> +};
> +
> +&usbphy2 {
> +	fsl,tx-d-cal = <106>;
> +};
> +
>  &usdhc2 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_usdhc2>;
> -- 
> 2.7.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

* Re: [PATCH 2/5] drivers: gpio: Add support for multiple IPs
From: Linus Walleij @ 2016-11-05  8:23 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Keerthy, Roger Quadros, Alexandre Courbot, Lokesh Vutla,
	Rob Herring, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-OMAP
In-Reply-To: <b92f729b-5897-c29a-31ec-97510776d85d-l0cyMroinI0@public.gmane.org>

On Fri, Nov 4, 2016 at 8:59 PM, Grygorii Strashko
<grygorii.strashko-l0cyMroinI0@public.gmane.org> wrote:
> On 11/04/2016 09:28 AM, Linus Walleij wrote:

>> The DT model sort of mandates how the interrupts should be mapped
>> at this point, and as far as I can tell from the binding the example looks
>> like so:
>>
>> gpio: gpio@1e26000 {
>>         compatible = "ti,dm6441-gpio";
>>         gpio-controller;
>>         #gpio-cells = <2>;
>>         reg = <0x226000 0x1000>;
>>         interrupt-parent = <&intc>;
>>         interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
>>                 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
>>                 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
>>                 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
>>                 50 IRQ_TYPE_EDGE_BOTH>;
>>         ti,ngpio = <144>;
>>         ti,davinci-gpio-unbanked = <0>;
>>         interrupt-controller;
>>         #interrupt-cells = <2>;
>> };
>
> Above, DT bindings models Davinci GPIO IP as monolithic GPIO controller
> with N gpio pins, but internally separate GPIO chips are created for each
> banksX&Y register set (32 pins, 2 banked irq -or- 32 direct irqs).

Hm it would be good to get away from that and just have one big gpio
chip.

> Translation from linear GPIO numbering to the proper internal GPIO chip is done
> using chip.of_xlate().

Yeah :/ this could be made simpler with a single chip just spanning all
the banks and the common registers I think.

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 v2 1/2] phy: rockchip-inno-usb2: support otg-port for rk3399
From: wlf @ 2016-11-05  2:58 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, heiko-4mtYJXux2i+zQB+pC5nmwQ
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	frank.wang-TNX95d0MmH7DzftRWevZcw,
	huangtao-TNX95d0MmH7DzftRWevZcw, dianders-hpIqsD4AKlfQT0dZR+AlfA,
	briannorris-hpIqsD4AKlfQT0dZR+AlfA, groeck-hpIqsD4AKlfQT0dZR+AlfA
In-Reply-To: <0eeb3dd9-fc1e-f04b-4612-06367929ede6-l0cyMroinI0@public.gmane.org>

Hi Kishon,

在 2016年11月04日 01:17, Kishon Vijay Abraham I 写道:
>
> On Thursday 03 November 2016 07:36 AM, William Wu wrote:
>> The rk3399 SoC USB2 PHY is comprised of one Host port and
>> one OTG port. And OTG port is for USB2.0 part of USB3.0 OTG
>> controller, as a part to construct a fully feature Type-C
>> subsystem.
>>
>> With this patch, we can support OTG port with the following
>> functions:
>> - Support BC1.2 charger detect, and use extcon notifier to
>>    send USB charger types to power driver.
>> - Support PHY suspend for power management.
>> - Support OTG Host only mode.
>>
>> Also, correct 480MHz output clock stable time. We found that
>> the system crashed due to 480MHz output clock of USB2 PHY was
>> unstable after clock had been enabled by gpu module.
> Can you split the clock fix into a separate patch?
OK, I will separate the clock patch in next version.

Best regards,
          wulf
>
> Thanks
> Kishon
>> Theoretically, 1 millisecond is a critical value for 480 output
>> clock stable time, so we try changing the delay time to 1.2
>> millisecond to avoid this issue.
>>
>> Signed-off-by: William Wu <wulf-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> ---
>> Changes in v2:
>> - remove wakelock
>>
>>   drivers/phy/phy-rockchip-inno-usb2.c | 593 +++++++++++++++++++++++++++++++++--
>>   1 file changed, 562 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/phy/phy-rockchip-inno-usb2.c b/drivers/phy/phy-rockchip-inno-usb2.c
>> index ac20310..8f2d2b6 100644
>> --- a/drivers/phy/phy-rockchip-inno-usb2.c
>> +++ b/drivers/phy/phy-rockchip-inno-usb2.c
>> @@ -17,6 +17,7 @@
>>   #include <linux/clk.h>
>>   #include <linux/clk-provider.h>
>>   #include <linux/delay.h>
>> +#include <linux/extcon.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/io.h>
>>   #include <linux/gpio/consumer.h>
>> @@ -30,11 +31,15 @@
>>   #include <linux/of_platform.h>
>>   #include <linux/phy/phy.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/power_supply.h>
>>   #include <linux/regmap.h>
>>   #include <linux/mfd/syscon.h>
>> +#include <linux/usb/of.h>
>> +#include <linux/usb/otg.h>
>>   
>>   #define BIT_WRITEABLE_SHIFT	16
>> -#define SCHEDULE_DELAY	(60 * HZ)
>> +#define SCHEDULE_DELAY		(60 * HZ)
>> +#define OTG_SCHEDULE_DELAY	(2 * HZ)
>>   
>>   enum rockchip_usb2phy_port_id {
>>   	USB2PHY_PORT_OTG,
>> @@ -49,6 +54,37 @@ enum rockchip_usb2phy_host_state {
>>   	PHY_STATE_FS_LS_ONLINE	= 4,
>>   };
>>   
>> +/**
>> + * Different states involved in USB charger detection.
>> + * USB_CHG_STATE_UNDEFINED	USB charger is not connected or detection
>> + *				process is not yet started.
>> + * USB_CHG_STATE_WAIT_FOR_DCD	Waiting for Data pins contact.
>> + * USB_CHG_STATE_DCD_DONE	Data pin contact is detected.
>> + * USB_CHG_STATE_PRIMARY_DONE	Primary detection is completed (Detects
>> + *				between SDP and DCP/CDP).
>> + * USB_CHG_STATE_SECONDARY_DONE	Secondary detection is completed (Detects
>> + *				between DCP and CDP).
>> + * USB_CHG_STATE_DETECTED	USB charger type is determined.
>> + */
>> +enum usb_chg_state {
>> +	USB_CHG_STATE_UNDEFINED = 0,
>> +	USB_CHG_STATE_WAIT_FOR_DCD,
>> +	USB_CHG_STATE_DCD_DONE,
>> +	USB_CHG_STATE_PRIMARY_DONE,
>> +	USB_CHG_STATE_SECONDARY_DONE,
>> +	USB_CHG_STATE_DETECTED,
>> +};
>> +
>> +static const unsigned int rockchip_usb2phy_extcon_cable[] = {
>> +	EXTCON_USB,
>> +	EXTCON_USB_HOST,
>> +	EXTCON_CHG_USB_SDP,
>> +	EXTCON_CHG_USB_CDP,
>> +	EXTCON_CHG_USB_DCP,
>> +	EXTCON_CHG_USB_SLOW,
>> +	EXTCON_NONE,
>> +};
>> +
>>   struct usb2phy_reg {
>>   	unsigned int	offset;
>>   	unsigned int	bitend;
>> @@ -58,19 +94,55 @@ struct usb2phy_reg {
>>   };
>>   
>>   /**
>> + * struct rockchip_chg_det_reg: usb charger detect registers
>> + * @cp_det: charging port detected successfully.
>> + * @dcp_det: dedicated charging port detected successfully.
>> + * @dp_det: assert data pin connect successfully.
>> + * @idm_sink_en: open dm sink curren.
>> + * @idp_sink_en: open dp sink current.
>> + * @idp_src_en: open dm source current.
>> + * @rdm_pdwn_en: open dm pull down resistor.
>> + * @vdm_src_en: open dm voltage source.
>> + * @vdp_src_en: open dp voltage source.
>> + * @opmode: utmi operational mode.
>> + */
>> +struct rockchip_chg_det_reg {
>> +	struct usb2phy_reg	cp_det;
>> +	struct usb2phy_reg	dcp_det;
>> +	struct usb2phy_reg	dp_det;
>> +	struct usb2phy_reg	idm_sink_en;
>> +	struct usb2phy_reg	idp_sink_en;
>> +	struct usb2phy_reg	idp_src_en;
>> +	struct usb2phy_reg	rdm_pdwn_en;
>> +	struct usb2phy_reg	vdm_src_en;
>> +	struct usb2phy_reg	vdp_src_en;
>> +	struct usb2phy_reg	opmode;
>> +};
>> +
>> +/**
>>    * struct rockchip_usb2phy_port_cfg: usb-phy port configuration.
>>    * @phy_sus: phy suspend register.
>> + * @bvalid_det_en: vbus valid rise detection enable register.
>> + * @bvalid_det_st: vbus valid rise detection status register.
>> + * @bvalid_det_clr: vbus valid rise detection clear register.
>>    * @ls_det_en: linestate detection enable register.
>>    * @ls_det_st: linestate detection state register.
>>    * @ls_det_clr: linestate detection clear register.
>> + * @utmi_avalid: utmi vbus avalid status register.
>> + * @utmi_bvalid: utmi vbus bvalid status register.
>>    * @utmi_ls: utmi linestate state register.
>>    * @utmi_hstdet: utmi host disconnect register.
>>    */
>>   struct rockchip_usb2phy_port_cfg {
>>   	struct usb2phy_reg	phy_sus;
>> +	struct usb2phy_reg	bvalid_det_en;
>> +	struct usb2phy_reg	bvalid_det_st;
>> +	struct usb2phy_reg	bvalid_det_clr;
>>   	struct usb2phy_reg	ls_det_en;
>>   	struct usb2phy_reg	ls_det_st;
>>   	struct usb2phy_reg	ls_det_clr;
>> +	struct usb2phy_reg	utmi_avalid;
>> +	struct usb2phy_reg	utmi_bvalid;
>>   	struct usb2phy_reg	utmi_ls;
>>   	struct usb2phy_reg	utmi_hstdet;
>>   };
>> @@ -80,31 +152,51 @@ struct rockchip_usb2phy_port_cfg {
>>    * @reg: the address offset of grf for usb-phy config.
>>    * @num_ports: specify how many ports that the phy has.
>>    * @clkout_ctl: keep on/turn off output clk of phy.
>> + * @chg_det: charger detection registers.
>>    */
>>   struct rockchip_usb2phy_cfg {
>>   	unsigned int	reg;
>>   	unsigned int	num_ports;
>>   	struct usb2phy_reg	clkout_ctl;
>>   	const struct rockchip_usb2phy_port_cfg	port_cfgs[USB2PHY_NUM_PORTS];
>> +	const struct rockchip_chg_det_reg	chg_det;
>>   };
>>   
>>   /**
>>    * struct rockchip_usb2phy_port: usb-phy port data.
>>    * @port_id: flag for otg port or host port.
>>    * @suspended: phy suspended flag.
>> + * @utmi_avalid: utmi avalid status usage flag.
>> + *	true	- use avalid to get vbus status
>> + *	flase	- use bvalid to get vbus status
>> + * @vbus_attached: otg device vbus status.
>> + * @bvalid_irq: IRQ number assigned for vbus valid rise detection.
>>    * @ls_irq: IRQ number assigned for linestate detection.
>>    * @mutex: for register updating in sm_work.
>> - * @sm_work: OTG state machine work.
>> + * @chg_work: charge detect work.
>> + * @otg_sm_work: OTG state machine work.
>> + * @sm_work: HOST state machine work.
>>    * @phy_cfg: port register configuration, assigned by driver data.
>> + * @event_nb: hold event notification callback.
>> + * @state: define OTG enumeration states before device reset.
>> + * @mode: the dr_mode of the controller.
>>    */
>>   struct rockchip_usb2phy_port {
>>   	struct phy	*phy;
>>   	unsigned int	port_id;
>>   	bool		suspended;
>> +	bool		utmi_avalid;
>> +	bool		vbus_attached;
>> +	int		bvalid_irq;
>>   	int		ls_irq;
>>   	struct mutex	mutex;
>> +	struct		delayed_work chg_work;
>> +	struct		delayed_work otg_sm_work;
>>   	struct		delayed_work sm_work;
>>   	const struct	rockchip_usb2phy_port_cfg *port_cfg;
>> +	struct notifier_block	event_nb;
>> +	enum usb_otg_state	state;
>> +	enum usb_dr_mode	mode;
>>   };
>>   
>>   /**
>> @@ -113,6 +205,11 @@ struct rockchip_usb2phy_port {
>>    * @clk: clock struct of phy input clk.
>>    * @clk480m: clock struct of phy output clk.
>>    * @clk_hw: clock struct of phy output clk management.
>> + * @chg_state: states involved in USB charger detection.
>> + * @chg_type: USB charger types.
>> + * @dcd_retries: The retry count used to track Data contact
>> + *		 detection process.
>> + * @edev: extcon device for notification registration
>>    * @phy_cfg: phy register configuration, assigned by driver data.
>>    * @ports: phy port instance.
>>    */
>> @@ -122,6 +219,10 @@ struct rockchip_usb2phy {
>>   	struct clk	*clk;
>>   	struct clk	*clk480m;
>>   	struct clk_hw	clk480m_hw;
>> +	enum usb_chg_state	chg_state;
>> +	enum power_supply_type	chg_type;
>> +	u8			dcd_retries;
>> +	struct extcon_dev	*edev;
>>   	const struct rockchip_usb2phy_cfg	*phy_cfg;
>>   	struct rockchip_usb2phy_port	ports[USB2PHY_NUM_PORTS];
>>   };
>> @@ -166,7 +267,7 @@ static int rockchip_usb2phy_clk480m_enable(struct clk_hw *hw)
>>   			return ret;
>>   
>>   		/* waitting for the clk become stable */
>> -		mdelay(1);
>> +		udelay(1200);
>>   	}
>>   
>>   	return 0;
>> @@ -263,33 +364,84 @@ rockchip_usb2phy_clk480m_register(struct rockchip_usb2phy *rphy)
>>   	return ret;
>>   }
>>   
>> -static int rockchip_usb2phy_init(struct phy *phy)
>> +static int rockchip_usb2phy_extcon_register(struct rockchip_usb2phy *rphy)
>>   {
>> -	struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
>> -	struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
>>   	int ret;
>> +	struct device_node *node = rphy->dev->of_node;
>> +	struct extcon_dev *edev;
>> +
>> +	if (of_property_read_bool(node, "extcon")) {
>> +		edev = extcon_get_edev_by_phandle(rphy->dev, 0);
>> +		if (IS_ERR(edev)) {
>> +			if (PTR_ERR(edev) != -EPROBE_DEFER)
>> +				dev_err(rphy->dev, "Invalid or missing extcon\n");
>> +			return PTR_ERR(edev);
>> +		}
>> +	} else {
>> +		/* Initialize extcon device */
>> +		edev = devm_extcon_dev_allocate(rphy->dev,
>> +						rockchip_usb2phy_extcon_cable);
>>   
>> -	if (rport->port_id == USB2PHY_PORT_HOST) {
>> -		/* clear linestate and enable linestate detect irq */
>> -		mutex_lock(&rport->mutex);
>> +		if (IS_ERR(edev))
>> +			return -ENOMEM;
>>   
>> -		ret = property_enable(rphy, &rport->port_cfg->ls_det_clr, true);
>> +		ret = devm_extcon_dev_register(rphy->dev, edev);
>>   		if (ret) {
>> -			mutex_unlock(&rport->mutex);
>> +			dev_err(rphy->dev, "failed to register extcon device\n");
>>   			return ret;
>>   		}
>> +	}
>>   
>> -		ret = property_enable(rphy, &rport->port_cfg->ls_det_en, true);
>> -		if (ret) {
>> -			mutex_unlock(&rport->mutex);
>> -			return ret;
>> +	rphy->edev = edev;
>> +
>> +	return 0;
>> +}
>> +
>> +static int rockchip_usb2phy_init(struct phy *phy)
>> +{
>> +	struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
>> +	struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
>> +	int ret = 0;
>> +
>> +	mutex_lock(&rport->mutex);
>> +
>> +	if (rport->port_id == USB2PHY_PORT_OTG) {
>> +		if (rport->mode != USB_DR_MODE_HOST) {
>> +			/* clear bvalid status and enable bvalid detect irq */
>> +			ret = property_enable(rphy,
>> +					      &rport->port_cfg->bvalid_det_clr,
>> +					      true);
>> +			if (ret)
>> +				goto out;
>> +
>> +			ret = property_enable(rphy,
>> +					      &rport->port_cfg->bvalid_det_en,
>> +					      true);
>> +			if (ret)
>> +				goto out;
>> +
>> +			schedule_delayed_work(&rport->otg_sm_work,
>> +					      OTG_SCHEDULE_DELAY);
>> +		} else {
>> +			/* If OTG works in host only mode, do nothing. */
>> +			dev_dbg(&rport->phy->dev, "mode %d\n", rport->mode);
>>   		}
>> +	} else if (rport->port_id == USB2PHY_PORT_HOST) {
>> +		/* clear linestate and enable linestate detect irq */
>> +		ret = property_enable(rphy, &rport->port_cfg->ls_det_clr, true);
>> +		if (ret)
>> +			goto out;
>> +
>> +		ret = property_enable(rphy, &rport->port_cfg->ls_det_en, true);
>> +		if (ret)
>> +			goto out;
>>   
>> -		mutex_unlock(&rport->mutex);
>>   		schedule_delayed_work(&rport->sm_work, SCHEDULE_DELAY);
>>   	}
>>   
>> -	return 0;
>> +out:
>> +	mutex_unlock(&rport->mutex);
>> +	return ret;
>>   }
>>   
>>   static int rockchip_usb2phy_power_on(struct phy *phy)
>> @@ -340,7 +492,11 @@ static int rockchip_usb2phy_exit(struct phy *phy)
>>   {
>>   	struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
>>   
>> -	if (rport->port_id == USB2PHY_PORT_HOST)
>> +	if (rport->port_id == USB2PHY_PORT_OTG &&
>> +	    rport->mode != USB_DR_MODE_HOST) {
>> +		cancel_delayed_work_sync(&rport->otg_sm_work);
>> +		cancel_delayed_work_sync(&rport->chg_work);
>> +	} else if (rport->port_id == USB2PHY_PORT_HOST)
>>   		cancel_delayed_work_sync(&rport->sm_work);
>>   
>>   	return 0;
>> @@ -354,6 +510,249 @@ static const struct phy_ops rockchip_usb2phy_ops = {
>>   	.owner		= THIS_MODULE,
>>   };
>>   
>> +static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
>> +{
>> +	struct rockchip_usb2phy_port *rport =
>> +		container_of(work, struct rockchip_usb2phy_port,
>> +			     otg_sm_work.work);
>> +	struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent);
>> +	static unsigned int cable;
>> +	unsigned long delay;
>> +	bool vbus_attach, sch_work, notify_charger;
>> +
>> +	if (rport->utmi_avalid)
>> +		vbus_attach =
>> +			property_enabled(rphy, &rport->port_cfg->utmi_avalid);
>> +	else
>> +		vbus_attach =
>> +			property_enabled(rphy, &rport->port_cfg->utmi_bvalid);
>> +
>> +	sch_work = false;
>> +	notify_charger = false;
>> +	delay = OTG_SCHEDULE_DELAY;
>> +	dev_dbg(&rport->phy->dev, "%s otg sm work\n",
>> +		usb_otg_state_string(rport->state));
>> +
>> +	switch (rport->state) {
>> +	case OTG_STATE_UNDEFINED:
>> +		rport->state = OTG_STATE_B_IDLE;
>> +		if (!vbus_attach)
>> +			rockchip_usb2phy_power_off(rport->phy);
>> +		/* fall through */
>> +	case OTG_STATE_B_IDLE:
>> +		if (extcon_get_cable_state_(rphy->edev, EXTCON_USB_HOST) > 0) {
>> +			dev_dbg(&rport->phy->dev, "usb otg host connect\n");
>> +			rport->state = OTG_STATE_A_HOST;
>> +			rockchip_usb2phy_power_on(rport->phy);
>> +			return;
>> +		} else if (vbus_attach) {
>> +			dev_dbg(&rport->phy->dev, "vbus_attach\n");
>> +			switch (rphy->chg_state) {
>> +			case USB_CHG_STATE_UNDEFINED:
>> +				schedule_delayed_work(&rport->chg_work, 0);
>> +				return;
>> +			case USB_CHG_STATE_DETECTED:
>> +				switch (rphy->chg_type) {
>> +				case POWER_SUPPLY_TYPE_USB:
>> +					dev_dbg(&rport->phy->dev,
>> +						"sdp cable is connecetd\n");
>> +					rockchip_usb2phy_power_on(rport->phy);
>> +					rport->state = OTG_STATE_B_PERIPHERAL;
>> +					notify_charger = true;
>> +					sch_work = true;
>> +					cable = EXTCON_CHG_USB_SDP;
>> +					break;
>> +				case POWER_SUPPLY_TYPE_USB_DCP:
>> +					dev_dbg(&rport->phy->dev,
>> +						"dcp cable is connecetd\n");
>> +					rockchip_usb2phy_power_off(rport->phy);
>> +					notify_charger = true;
>> +					sch_work = true;
>> +					cable = EXTCON_CHG_USB_DCP;
>> +					break;
>> +				case POWER_SUPPLY_TYPE_USB_CDP:
>> +					dev_dbg(&rport->phy->dev,
>> +						"cdp cable is connecetd\n");
>> +					rockchip_usb2phy_power_on(rport->phy);
>> +					rport->state = OTG_STATE_B_PERIPHERAL;
>> +					notify_charger = true;
>> +					sch_work = true;
>> +					cable = EXTCON_CHG_USB_CDP;
>> +					break;
>> +				default:
>> +					break;
>> +				}
>> +				break;
>> +			default:
>> +				break;
>> +			}
>> +		} else {
>> +			notify_charger = true;
>> +			rphy->chg_state = USB_CHG_STATE_UNDEFINED;
>> +			rphy->chg_type = POWER_SUPPLY_TYPE_UNKNOWN;
>> +		}
>> +
>> +		if (rport->vbus_attached != vbus_attach) {
>> +			rport->vbus_attached = vbus_attach;
>> +
>> +			if (notify_charger && rphy->edev)
>> +				extcon_set_cable_state_(rphy->edev,
>> +							cable, vbus_attach);
>> +		}
>> +		break;
>> +	case OTG_STATE_B_PERIPHERAL:
>> +		if (!vbus_attach) {
>> +			dev_dbg(&rport->phy->dev, "usb disconnect\n");
>> +			rphy->chg_state = USB_CHG_STATE_UNDEFINED;
>> +			rphy->chg_type = POWER_SUPPLY_TYPE_UNKNOWN;
>> +			rport->state = OTG_STATE_B_IDLE;
>> +			delay = 0;
>> +			rockchip_usb2phy_power_off(rport->phy);
>> +		}
>> +		sch_work = true;
>> +		break;
>> +	case OTG_STATE_A_HOST:
>> +		if (extcon_get_cable_state_(rphy->edev, EXTCON_USB_HOST) == 0) {
>> +			dev_dbg(&rport->phy->dev, "usb otg host disconnect\n");
>> +			rport->state = OTG_STATE_B_IDLE;
>> +			rockchip_usb2phy_power_off(rport->phy);
>> +		}
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	if (sch_work)
>> +		schedule_delayed_work(&rport->otg_sm_work, delay);
>> +}
>> +
>> +static const char *chg_to_string(enum power_supply_type chg_type)
>> +{
>> +	switch (chg_type) {
>> +	case POWER_SUPPLY_TYPE_USB:
>> +		return "USB_SDP_CHARGER";
>> +	case POWER_SUPPLY_TYPE_USB_DCP:
>> +		return "USB_DCP_CHARGER";
>> +	case POWER_SUPPLY_TYPE_USB_CDP:
>> +		return "USB_CDP_CHARGER";
>> +	default:
>> +		return "INVALID_CHARGER";
>> +	}
>> +}
>> +
>> +static void rockchip_chg_enable_dcd(struct rockchip_usb2phy *rphy,
>> +				    bool en)
>> +{
>> +	property_enable(rphy, &rphy->phy_cfg->chg_det.rdm_pdwn_en, en);
>> +	property_enable(rphy, &rphy->phy_cfg->chg_det.idp_src_en, en);
>> +}
>> +
>> +static void rockchip_chg_enable_primary_det(struct rockchip_usb2phy *rphy,
>> +					    bool en)
>> +{
>> +	property_enable(rphy, &rphy->phy_cfg->chg_det.vdp_src_en, en);
>> +	property_enable(rphy, &rphy->phy_cfg->chg_det.idm_sink_en, en);
>> +}
>> +
>> +static void rockchip_chg_enable_secondary_det(struct rockchip_usb2phy *rphy,
>> +					      bool en)
>> +{
>> +	property_enable(rphy, &rphy->phy_cfg->chg_det.vdm_src_en, en);
>> +	property_enable(rphy, &rphy->phy_cfg->chg_det.idp_sink_en, en);
>> +}
>> +
>> +#define CHG_DCD_POLL_TIME	(100 * HZ / 1000)
>> +#define CHG_DCD_MAX_RETRIES	6
>> +#define CHG_PRIMARY_DET_TIME	(40 * HZ / 1000)
>> +#define CHG_SECONDARY_DET_TIME	(40 * HZ / 1000)
>> +static void rockchip_chg_detect_work(struct work_struct *work)
>> +{
>> +	struct rockchip_usb2phy_port *rport =
>> +		container_of(work, struct rockchip_usb2phy_port, chg_work.work);
>> +	struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent);
>> +	bool is_dcd, tmout, vout;
>> +	unsigned long delay;
>> +
>> +	dev_dbg(&rport->phy->dev, "chg detection work state = %d\n",
>> +		rphy->chg_state);
>> +	switch (rphy->chg_state) {
>> +	case USB_CHG_STATE_UNDEFINED:
>> +		if (!rport->suspended)
>> +			rockchip_usb2phy_power_off(rport->phy);
>> +		/* put the controller in non-driving mode */
>> +		property_enable(rphy, &rphy->phy_cfg->chg_det.opmode, false);
>> +		/* Start DCD processing stage 1 */
>> +		rockchip_chg_enable_dcd(rphy, true);
>> +		rphy->chg_state = USB_CHG_STATE_WAIT_FOR_DCD;
>> +		rphy->dcd_retries = 0;
>> +		delay = CHG_DCD_POLL_TIME;
>> +		break;
>> +	case USB_CHG_STATE_WAIT_FOR_DCD:
>> +		/* get data contact detection status */
>> +		is_dcd = property_enabled(rphy, &rphy->phy_cfg->chg_det.dp_det);
>> +		tmout = ++rphy->dcd_retries == CHG_DCD_MAX_RETRIES;
>> +		/* stage 2 */
>> +		if (is_dcd || tmout) {
>> +			/* stage 4 */
>> +			/* Turn off DCD circuitry */
>> +			rockchip_chg_enable_dcd(rphy, false);
>> +			/* Voltage Source on DP, Probe on DM */
>> +			rockchip_chg_enable_primary_det(rphy, true);
>> +			delay = CHG_PRIMARY_DET_TIME;
>> +			rphy->chg_state = USB_CHG_STATE_DCD_DONE;
>> +		} else {
>> +			/* stage 3 */
>> +			delay = CHG_DCD_POLL_TIME;
>> +		}
>> +		break;
>> +	case USB_CHG_STATE_DCD_DONE:
>> +		vout = property_enabled(rphy, &rphy->phy_cfg->chg_det.cp_det);
>> +		rockchip_chg_enable_primary_det(rphy, false);
>> +		if (vout) {
>> +			/* Voltage Source on DM, Probe on DP  */
>> +			rockchip_chg_enable_secondary_det(rphy, true);
>> +			delay = CHG_SECONDARY_DET_TIME;
>> +			rphy->chg_state = USB_CHG_STATE_PRIMARY_DONE;
>> +		} else {
>> +			if (tmout) {
>> +				/* floating charger found */
>> +				rphy->chg_type = POWER_SUPPLY_TYPE_USB_DCP;
>> +				rphy->chg_state = USB_CHG_STATE_DETECTED;
>> +				delay = 0;
>> +			} else {
>> +				rphy->chg_type = POWER_SUPPLY_TYPE_USB;
>> +				rphy->chg_state = USB_CHG_STATE_DETECTED;
>> +				delay = 0;
>> +			}
>> +		}
>> +		break;
>> +	case USB_CHG_STATE_PRIMARY_DONE:
>> +		vout = property_enabled(rphy, &rphy->phy_cfg->chg_det.dcp_det);
>> +		/* Turn off voltage source */
>> +		rockchip_chg_enable_secondary_det(rphy, false);
>> +		if (vout)
>> +			rphy->chg_type = POWER_SUPPLY_TYPE_USB_DCP;
>> +		else
>> +			rphy->chg_type = POWER_SUPPLY_TYPE_USB_CDP;
>> +		/* fall through */
>> +	case USB_CHG_STATE_SECONDARY_DONE:
>> +		rphy->chg_state = USB_CHG_STATE_DETECTED;
>> +		delay = 0;
>> +		/* fall through */
>> +	case USB_CHG_STATE_DETECTED:
>> +		/* put the controller in normal mode */
>> +		property_enable(rphy, &rphy->phy_cfg->chg_det.opmode, true);
>> +		rockchip_usb2phy_otg_sm_work(&rport->otg_sm_work.work);
>> +		dev_info(&rport->phy->dev, "charger = %s\n",
>> +			 chg_to_string(rphy->chg_type));
>> +		return;
>> +	default:
>> +		return;
>> +	}
>> +
>> +	schedule_delayed_work(&rport->chg_work, delay);
>> +}
>> +
>>   /*
>>    * The function manage host-phy port state and suspend/resume phy port
>>    * to save power.
>> @@ -485,6 +884,26 @@ static irqreturn_t rockchip_usb2phy_linestate_irq(int irq, void *data)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> +static irqreturn_t rockchip_usb2phy_bvalid_irq(int irq, void *data)
>> +{
>> +	struct rockchip_usb2phy_port *rport = data;
>> +	struct rockchip_usb2phy *rphy = dev_get_drvdata(rport->phy->dev.parent);
>> +
>> +	if (!property_enabled(rphy, &rport->port_cfg->bvalid_det_st))
>> +		return IRQ_NONE;
>> +
>> +	mutex_lock(&rport->mutex);
>> +
>> +	/* clear bvalid detect irq pending status */
>> +	property_enable(rphy, &rport->port_cfg->bvalid_det_clr, true);
>> +
>> +	mutex_unlock(&rport->mutex);
>> +
>> +	rockchip_usb2phy_otg_sm_work(&rport->otg_sm_work.work);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>>   static int rockchip_usb2phy_host_port_init(struct rockchip_usb2phy *rphy,
>>   					   struct rockchip_usb2phy_port *rport,
>>   					   struct device_node *child_np)
>> @@ -509,13 +928,86 @@ static int rockchip_usb2phy_host_port_init(struct rockchip_usb2phy *rphy,
>>   					IRQF_ONESHOT,
>>   					"rockchip_usb2phy", rport);
>>   	if (ret) {
>> -		dev_err(rphy->dev, "failed to request irq handle\n");
>> +		dev_err(rphy->dev, "failed to request linestate irq handle\n");
>>   		return ret;
>>   	}
>>   
>>   	return 0;
>>   }
>>   
>> +static int rockchip_otg_event(struct notifier_block *nb,
>> +			      unsigned long event, void *ptr)
>> +{
>> +	struct rockchip_usb2phy_port *rport =
>> +		container_of(nb, struct rockchip_usb2phy_port, event_nb);
>> +
>> +	schedule_delayed_work(&rport->otg_sm_work, OTG_SCHEDULE_DELAY);
>> +
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
>> +					  struct rockchip_usb2phy_port *rport,
>> +					  struct device_node *child_np)
>> +{
>> +	int ret;
>> +
>> +	rport->port_id = USB2PHY_PORT_OTG;
>> +	rport->port_cfg = &rphy->phy_cfg->port_cfgs[USB2PHY_PORT_OTG];
>> +	rport->state = OTG_STATE_UNDEFINED;
>> +
>> +	/*
>> +	 * set suspended flag to true, but actually don't
>> +	 * put phy in suspend mode, it aims to enable usb
>> +	 * phy and clock in power_on() called by usb controller
>> +	 * driver during probe.
>> +	 */
>> +	rport->suspended = true;
>> +	rport->vbus_attached = false;
>> +
>> +	mutex_init(&rport->mutex);
>> +
>> +	rport->mode = of_usb_get_dr_mode_by_phy(child_np, -1);
>> +	if (rport->mode == USB_DR_MODE_HOST) {
>> +		ret = 0;
>> +		goto out;
>> +	}
>> +
>> +	INIT_DELAYED_WORK(&rport->chg_work, rockchip_chg_detect_work);
>> +	INIT_DELAYED_WORK(&rport->otg_sm_work, rockchip_usb2phy_otg_sm_work);
>> +
>> +	rport->utmi_avalid =
>> +		of_property_read_bool(child_np, "rockchip,utmi-avalid");
>> +
>> +	rport->bvalid_irq = of_irq_get_byname(child_np, "otg-bvalid");
>> +	if (rport->bvalid_irq < 0) {
>> +		dev_err(rphy->dev, "no vbus valid irq provided\n");
>> +		ret = rport->bvalid_irq;
>> +		goto out;
>> +	}
>> +
>> +	ret = devm_request_threaded_irq(rphy->dev, rport->bvalid_irq, NULL,
>> +					rockchip_usb2phy_bvalid_irq,
>> +					IRQF_ONESHOT,
>> +					"rockchip_usb2phy_bvalid", rport);
>> +	if (ret) {
>> +		dev_err(rphy->dev, "failed to request otg-bvalid irq handle\n");
>> +		goto out;
>> +	}
>> +
>> +	if (!IS_ERR(rphy->edev)) {
>> +		rport->event_nb.notifier_call = rockchip_otg_event;
>> +
>> +		ret = extcon_register_notifier(rphy->edev, EXTCON_USB_HOST,
>> +					       &rport->event_nb);
>> +		if (ret)
>> +			dev_err(rphy->dev, "register USB HOST notifier failed\n");
>> +	}
>> +
>> +out:
>> +	return ret;
>> +}
>> +
>>   static int rockchip_usb2phy_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>> @@ -553,8 +1045,14 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
>>   
>>   	rphy->dev = dev;
>>   	phy_cfgs = match->data;
>> +	rphy->chg_state = USB_CHG_STATE_UNDEFINED;
>> +	rphy->chg_type = POWER_SUPPLY_TYPE_UNKNOWN;
>>   	platform_set_drvdata(pdev, rphy);
>>   
>> +	ret = rockchip_usb2phy_extcon_register(rphy);
>> +	if (ret)
>> +		return ret;
>> +
>>   	/* find out a proper config which can be matched with dt. */
>>   	index = 0;
>>   	while (phy_cfgs[index].reg) {
>> @@ -591,13 +1089,9 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
>>   		struct rockchip_usb2phy_port *rport = &rphy->ports[index];
>>   		struct phy *phy;
>>   
>> -		/*
>> -		 * This driver aim to support both otg-port and host-port,
>> -		 * but unfortunately, the otg part is not ready in current,
>> -		 * so this comments and below codes are interim, which should
>> -		 * be changed after otg-port is supplied soon.
>> -		 */
>> -		if (of_node_cmp(child_np->name, "host-port"))
>> +		/* This driver aims to support both otg-port and host-port */
>> +		if (of_node_cmp(child_np->name, "host-port") &&
>> +		    of_node_cmp(child_np->name, "otg-port"))
>>   			goto next_child;
>>   
>>   		phy = devm_phy_create(dev, child_np, &rockchip_usb2phy_ops);
>> @@ -610,9 +1104,18 @@ static int rockchip_usb2phy_probe(struct platform_device *pdev)
>>   		rport->phy = phy;
>>   		phy_set_drvdata(rport->phy, rport);
>>   
>> -		ret = rockchip_usb2phy_host_port_init(rphy, rport, child_np);
>> -		if (ret)
>> -			goto put_child;
>> +		/* initialize otg/host port separately */
>> +		if (!of_node_cmp(child_np->name, "host-port")) {
>> +			ret = rockchip_usb2phy_host_port_init(rphy, rport,
>> +							      child_np);
>> +			if (ret)
>> +				goto put_child;
>> +		} else {
>> +			ret = rockchip_usb2phy_otg_port_init(rphy, rport,
>> +							     child_np);
>> +			if (ret)
>> +				goto put_child;
>> +		}
>>   
>>   next_child:
>>   		/* to prevent out of boundary */
>> @@ -654,10 +1157,18 @@ static const struct rockchip_usb2phy_cfg rk3366_phy_cfgs[] = {
>>   
>>   static const struct rockchip_usb2phy_cfg rk3399_phy_cfgs[] = {
>>   	{
>> -		.reg = 0xe450,
>> +		.reg		= 0xe450,
>>   		.num_ports	= 2,
>>   		.clkout_ctl	= { 0xe450, 4, 4, 1, 0 },
>>   		.port_cfgs	= {
>> +			[USB2PHY_PORT_OTG] = {
>> +				.phy_sus	= { 0xe454, 1, 0, 2, 1 },
>> +				.bvalid_det_en	= { 0xe3c0, 3, 3, 0, 1 },
>> +				.bvalid_det_st	= { 0xe3e0, 3, 3, 0, 1 },
>> +				.bvalid_det_clr	= { 0xe3d0, 3, 3, 0, 1 },
>> +				.utmi_avalid	= { 0xe2ac, 7, 7, 0, 1 },
>> +				.utmi_bvalid	= { 0xe2ac, 12, 12, 0, 1 },
>> +			},
>>   			[USB2PHY_PORT_HOST] = {
>>   				.phy_sus	= { 0xe458, 1, 0, 0x2, 0x1 },
>>   				.ls_det_en	= { 0xe3c0, 6, 6, 0, 1 },
>> @@ -667,12 +1178,32 @@ static const struct rockchip_usb2phy_cfg rk3399_phy_cfgs[] = {
>>   				.utmi_hstdet	= { 0xe2ac, 23, 23, 0, 1 }
>>   			}
>>   		},
>> +		.chg_det = {
>> +			.opmode		= { 0xe454, 3, 0, 5, 1 },
>> +			.cp_det		= { 0xe2ac, 2, 2, 0, 1 },
>> +			.dcp_det	= { 0xe2ac, 1, 1, 0, 1 },
>> +			.dp_det		= { 0xe2ac, 0, 0, 0, 1 },
>> +			.idm_sink_en	= { 0xe450, 8, 8, 0, 1 },
>> +			.idp_sink_en	= { 0xe450, 7, 7, 0, 1 },
>> +			.idp_src_en	= { 0xe450, 9, 9, 0, 1 },
>> +			.rdm_pdwn_en	= { 0xe450, 10, 10, 0, 1 },
>> +			.vdm_src_en	= { 0xe450, 12, 12, 0, 1 },
>> +			.vdp_src_en	= { 0xe450, 11, 11, 0, 1 },
>> +		},
>>   	},
>>   	{
>> -		.reg = 0xe460,
>> +		.reg		= 0xe460,
>>   		.num_ports	= 2,
>>   		.clkout_ctl	= { 0xe460, 4, 4, 1, 0 },
>>   		.port_cfgs	= {
>> +			[USB2PHY_PORT_OTG] = {
>> +				.phy_sus        = { 0xe464, 1, 0, 2, 1 },
>> +				.bvalid_det_en  = { 0xe3c0, 8, 8, 0, 1 },
>> +				.bvalid_det_st  = { 0xe3e0, 8, 8, 0, 1 },
>> +				.bvalid_det_clr = { 0xe3d0, 8, 8, 0, 1 },
>> +				.utmi_avalid	= { 0xe2ac, 10, 10, 0, 1 },
>> +				.utmi_bvalid    = { 0xe2ac, 16, 16, 0, 1 },
>> +			},
>>   			[USB2PHY_PORT_HOST] = {
>>   				.phy_sus	= { 0xe468, 1, 0, 0x2, 0x1 },
>>   				.ls_det_en	= { 0xe3c0, 11, 11, 0, 1 },
>>
>
>


--
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 v4 2/2] i2c: aspeed: added documentation for Aspeed I2C driver
From: Brendan Higgins @ 2016-11-05  1:58 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland
  Cc: linux-i2c, devicetree, joel, openbmc, Brendan Higgins
In-Reply-To: <1478311099-6771-1-git-send-email-brendanhiggins@google.com>

Added device tree binding documentation for Aspeed I2C controller and
busses.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Changes for v2:
  - None
Changes for v3:
  - Removed reference to "bus" device tree param
Changes for v4:
  - None
---
 .../devicetree/bindings/i2c/i2c-aspeed.txt         | 61 ++++++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
new file mode 100644
index 0000000..dd11a97
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -0,0 +1,61 @@
+Device tree configuration for the I2C controller and busses on the AST24XX
+and AST25XX SoCs.
+
+Controller:
+
+	Required Properties:
+	- #address-cells	: should be 1
+	- #size-cells 		: should be 1
+	- #interrupt-cells 	: should be 1
+	- compatible 		: should be "aspeed,ast2400-i2c-controller"
+				  or "aspeed,ast2500-i2c-controller"
+	- reg			: address start and range of controller
+	- ranges		: defines address offset and range for busses
+	- interrupts		: interrupt number
+	- clocks		: root clock of bus, should reference the APB
+				  clock
+	- clock-ranges		: specifies that child busses can inherit clocks
+	- interrupt-controller	: denotes that the controller receives and fires
+				  new interrupts for child busses
+
+Bus:
+
+	Required Properties:
+	- #address-cells	: should be 1
+	- #size-cells		: should be 0
+	- reg			: address offset and range of bus
+	- compatible		: should be "aspeed,ast2400-i2c-bus"
+				  or "aspeed,ast2500-i2c-bus"
+	- interrupts		: interrupt number
+
+	Optional Properties:
+	- clock-frequency	: frequency of the bus clock in Hz
+				  defaults to 100 kHz when not specified
+
+Example:
+
+i2c: i2c@1e78a000 {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	#interrupt-cells = <1>;
+
+	compatible = "aspeed,ast2400-i2c-controller";
+	reg = <0x1e78a000 0x40>;
+	ranges = <0 0x1e78a000 0x1000>;
+	interrupts = <12>;
+	clocks = <&clk_apb>;
+	clock-ranges;
+	interrupt-controller;
+
+	i2c0: i2c-bus@40 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x40 0x40>;
+		compatible = "aspeed,ast2400-i2c-bus";
+		clock-frequency = <100000>;
+		status = "disabled";
+		interrupts = <0>;
+		interrupt-parent = <&i2c>;
+	};
+};
+
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related

* [PATCH v4 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2016-11-05  1:58 UTC (permalink / raw)
  To: wsa, robh+dt, mark.rutland
  Cc: linux-i2c, devicetree, joel, openbmc, Brendan Higgins
In-Reply-To: <1478311099-6771-1-git-send-email-brendanhiggins@google.com>

Added initial master and slave support for Aspeed I2C controller.
Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
Aspeed.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Changes for v2:
  - Added single module_init (multiple was breaking some builds).
Changes for v3:
  - Removed "bus" device tree param; now extracted from bus address offset
Changes for v4:
  - I2C adapter number is now generated dynamically unless specified in alias.
---
 drivers/i2c/busses/Kconfig      |  10 +
 drivers/i2c/busses/Makefile     |   1 +
 drivers/i2c/busses/i2c-aspeed.c | 807 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 818 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-aspeed.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d252276..b6caa5d 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1009,6 +1009,16 @@ config I2C_RCAR
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-rcar.
 
+config I2C_ASPEED
+	tristate "Aspeed AST2xxx SoC I2C Controller"
+	depends on ARCH_ASPEED
+	help
+	  If you say yes to this option, support will be included for the
+	  Aspeed AST2xxx SoC I2C controller.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-aspeed.
+
 comment "External I2C/SMBus adapter drivers"
 
 config I2C_DIOLAN_U2C
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 29764cc..826e780 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_I2C_XILINX)	+= i2c-xiic.o
 obj-$(CONFIG_I2C_XLR)		+= i2c-xlr.o
 obj-$(CONFIG_I2C_XLP9XX)	+= i2c-xlp9xx.o
 obj-$(CONFIG_I2C_RCAR)		+= i2c-rcar.o
+obj-$(CONFIG_I2C_ASPEED)	+= i2c-aspeed.o
 
 # External I2C/SMBus adapter drivers
 obj-$(CONFIG_I2C_DIOLAN_U2C)	+= i2c-diolan-u2c.o
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
new file mode 100644
index 0000000..88e078a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -0,0 +1,807 @@
+/*
+ *  I2C adapter for the ASPEED I2C bus.
+ *
+ *  Copyright (C) 2012-2020  ASPEED Technology Inc.
+ *  Copyright 2016 IBM Corporation
+ *  Copyright 2016 Google, Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/slab.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+
+/* I2C Register */
+#define ASPEED_I2C_FUN_CTRL_REG				0x00
+#define ASPEED_I2C_AC_TIMING_REG1			0x04
+#define ASPEED_I2C_AC_TIMING_REG2			0x08
+#define ASPEED_I2C_INTR_CTRL_REG			0x0c
+#define ASPEED_I2C_INTR_STS_REG				0x10
+#define ASPEED_I2C_CMD_REG				0x14
+#define ASPEED_I2C_DEV_ADDR_REG				0x18
+#define ASPEED_I2C_BYTE_BUF_REG				0x20
+#define ASPEED_I2C_OFFSET_START				0x40
+#define ASPEED_I2C_OFFSET_INCREMENT			0x40
+
+#define ASPEED_I2C_NUM_BUS 14
+
+/* Global Register Definition */
+/* 0x00 : I2C Interrupt Status Register  */
+/* 0x08 : I2C Interrupt Target Assignment  */
+
+/* Device Register Definition */
+/* 0x00 : I2CD Function Control Register  */
+#define ASPEED_I2CD_MULTI_MASTER_DIS			BIT(15)
+#define ASPEED_I2CD_SDA_DRIVE_1T_EN			BIT(8)
+#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN			BIT(7)
+#define ASPEED_I2CD_M_HIGH_SPEED_EN			BIT(6)
+#define ASPEED_I2CD_SLAVE_EN				BIT(1)
+#define ASPEED_I2CD_MASTER_EN				BIT(0)
+
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define ASPEED_NO_TIMEOUT_CTRL				0
+
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT			BIT(14)
+#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE		BIT(13)
+#define ASPEED_I2CD_INTR_SLAVE_MATCH			BIT(7)
+#define ASPEED_I2CD_INTR_SCL_TIMEOUT			BIT(6)
+#define ASPEED_I2CD_INTR_ABNORMAL			BIT(5)
+#define ASPEED_I2CD_INTR_NORMAL_STOP			BIT(4)
+#define ASPEED_I2CD_INTR_ARBIT_LOSS			BIT(3)
+#define ASPEED_I2CD_INTR_RX_DONE			BIT(2)
+#define ASPEED_I2CD_INTR_TX_NAK				BIT(1)
+#define ASPEED_I2CD_INTR_TX_ACK				BIT(0)
+
+/* 0x14 : I2CD Command/Status Register   */
+#define ASPEED_I2CD_SCL_LINE_STS			BIT(18)
+#define ASPEED_I2CD_SDA_LINE_STS			BIT(17)
+#define ASPEED_I2CD_BUS_BUSY_STS			BIT(16)
+#define ASPEED_I2CD_BUS_RECOVER_CMD			BIT(11)
+
+/* Command Bit */
+#define ASPEED_I2CD_M_STOP_CMD				BIT(5)
+#define ASPEED_I2CD_M_S_RX_CMD_LAST			BIT(4)
+#define ASPEED_I2CD_M_RX_CMD				BIT(3)
+#define ASPEED_I2CD_S_TX_CMD				BIT(2)
+#define ASPEED_I2CD_M_TX_CMD				BIT(1)
+#define ASPEED_I2CD_M_START_CMD				BIT(0)
+
+/* 0x18 : I2CD Slave Device Address Register   */
+#define ASPEED_I2CD_DEV_ADDR_MASK			GENMASK(6, 0)
+
+enum aspeed_i2c_slave_state {
+	ASPEED_I2C_SLAVE_START,
+	ASPEED_I2C_SLAVE_READ_REQUESTED,
+	ASPEED_I2C_SLAVE_READ_PROCESSED,
+	ASPEED_I2C_SLAVE_WRITE_REQUESTED,
+	ASPEED_I2C_SLAVE_WRITE_RECEIVED,
+	ASPEED_I2C_SLAVE_STOP,
+};
+
+struct aspeed_i2c_bus {
+	struct i2c_adapter		adap;
+	struct device			*dev;
+	void __iomem			*base;
+	spinlock_t			lock;
+	struct completion		cmd_complete;
+	int				irq;
+	/* Transaction state. */
+	struct i2c_msg			*msg;
+	int				msg_pos;
+	u32				cmd_err;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	struct i2c_client		*slave;
+	enum aspeed_i2c_slave_state	slave_state;
+#endif
+};
+
+struct aspeed_i2c_controller {
+	struct device		*dev;
+	void __iomem		*base;
+	int			irq;
+	struct irq_domain	*irq_domain;
+};
+
+static inline void aspeed_i2c_write(struct aspeed_i2c_bus *bus, u32 val,
+				    u32 reg)
+{
+	writel(val, bus->base + reg);
+}
+
+static inline u32 aspeed_i2c_read(struct aspeed_i2c_bus *bus, u32 reg)
+{
+	return readl(bus->base + reg);
+}
+
+static u8 aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
+{
+	u32 command;
+	unsigned long time_left;
+	unsigned long flags;
+	int ret = 0;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
+	/* Bus is idle: no recovery needed. */
+	if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
+	    (command & ASPEED_I2CD_SCL_LINE_STS))
+		goto out;
+
+	dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
+		command);
+
+	/* Bus held: put bus in stop state. */
+	if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
+	    !(command & ASPEED_I2CD_SCL_LINE_STS)) {
+		aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
+				 ASPEED_I2C_CMD_REG);
+		reinit_completion(&bus->cmd_complete);
+		spin_unlock_irqrestore(&bus->lock, flags);
+
+		time_left = wait_for_completion_interruptible_timeout(
+				&bus->cmd_complete, bus->adap.timeout * HZ);
+
+		spin_lock_irqsave(&bus->lock, flags);
+		if (time_left == 0)
+			ret = -ETIMEDOUT;
+		else if (bus->cmd_err)
+			ret = -EIO;
+	/* Bus error. */
+	} else if (!(command & ASPEED_I2CD_SDA_LINE_STS)) {
+		aspeed_i2c_write(bus, ASPEED_I2CD_BUS_RECOVER_CMD,
+				 ASPEED_I2C_CMD_REG);
+		reinit_completion(&bus->cmd_complete);
+		spin_unlock_irqrestore(&bus->lock, flags);
+
+		time_left = wait_for_completion_interruptible_timeout(
+				&bus->cmd_complete, bus->adap.timeout * HZ);
+
+		spin_lock_irqsave(&bus->lock, flags);
+		if (time_left == 0)
+			ret = -ETIMEDOUT;
+		else if (bus->cmd_err)
+			ret = -EIO;
+		/* Recovery failed. */
+		else if (!(aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
+			   ASPEED_I2CD_SDA_LINE_STS))
+			ret = -EIO;
+	}
+
+out:
+	spin_unlock_irqrestore(&bus->lock, flags);
+	return ret;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
+{
+	bool irq_handled = true;
+	u32 command;
+	u32 irq_status;
+	u32 status_ack = 0;
+	u8 value;
+	struct i2c_client *slave = bus->slave;
+
+	spin_lock(&bus->lock);
+	if (!slave) {
+		irq_handled = false;
+		goto out;
+	}
+	command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
+	irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
+
+	/* Slave was requested, restart state machine. */
+	if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
+		status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
+		bus->slave_state = ASPEED_I2C_SLAVE_START;
+	}
+	/* Slave is not currently active, irq was for someone else. */
+	if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+		irq_handled = false;
+		goto out;
+	}
+
+	dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
+		irq_status, command);
+
+	/* Slave was sent something. */
+	if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
+		value = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
+		/* Handle address frame. */
+		if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
+			if (value & 0x1)
+				bus->slave_state =
+						ASPEED_I2C_SLAVE_READ_REQUESTED;
+			else
+				bus->slave_state =
+						ASPEED_I2C_SLAVE_WRITE_REQUESTED;
+		}
+		status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+	}
+
+	/* Slave was asked to stop. */
+	if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+		status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	}
+	if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
+		status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	}
+
+	if (bus->slave_state == ASPEED_I2C_SLAVE_READ_REQUESTED) {
+		if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
+			dev_err(bus->dev, "Unexpected ACK on read request.\n");
+		bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
+
+		i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
+		aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
+		aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
+	} else if (bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
+		status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+		if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
+			dev_err(bus->dev,
+				"Expected ACK after processed read.\n");
+		i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
+		aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
+		aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
+	} else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_REQUESTED) {
+		bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
+		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+	} else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED) {
+		i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+	} else if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+		i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
+	}
+
+	if (status_ack != irq_status)
+		dev_err(bus->dev,
+			"irq handled != irq. expected %x, but was %x\n",
+			irq_status, status_ack);
+	aspeed_i2c_write(bus, status_ack, ASPEED_I2C_INTR_STS_REG);
+
+out:
+	spin_unlock(&bus->lock);
+	return irq_handled;
+}
+#endif
+
+static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
+{
+	const u32 errs = ASPEED_I2CD_INTR_ARBIT_LOSS |
+		ASPEED_I2CD_INTR_ABNORMAL |
+		ASPEED_I2CD_INTR_SCL_TIMEOUT |
+		ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
+		ASPEED_I2CD_INTR_TX_NAK;
+	u32 irq_status;
+
+	spin_lock(&bus->lock);
+	irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
+	bus->cmd_err = irq_status & errs;
+
+	dev_dbg(bus->dev, "master irq status 0x%08x\n", irq_status);
+
+	/* No message to transfer. */
+	if (bus->cmd_err ||
+	    (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) ||
+	    (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE)) {
+		complete(&bus->cmd_complete);
+		goto out;
+	} else if (!bus->msg || bus->msg_pos >= bus->msg->len)
+		goto out;
+
+	if ((bus->msg->flags & I2C_M_RD) &&
+	    (irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
+		bus->msg->buf[bus->msg_pos++] = aspeed_i2c_read(
+				bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
+		if (bus->msg_pos + 1 < bus->msg->len)
+			aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD,
+					 ASPEED_I2C_CMD_REG);
+		else if (bus->msg_pos < bus->msg->len)
+			aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD |
+				      ASPEED_I2CD_M_S_RX_CMD_LAST,
+				      ASPEED_I2C_CMD_REG);
+	} else if (!(bus->msg->flags & I2C_M_RD) &&
+		   (irq_status & ASPEED_I2CD_INTR_TX_ACK)) {
+		aspeed_i2c_write(bus, bus->msg->buf[bus->msg_pos++],
+			      ASPEED_I2C_BYTE_BUF_REG);
+		aspeed_i2c_write(bus, ASPEED_I2CD_M_TX_CMD, ASPEED_I2C_CMD_REG);
+	}
+
+	/* Transmission complete: notify caller. */
+	if (bus->msg_pos >= bus->msg->len)
+		complete(&bus->cmd_complete);
+out:
+	aspeed_i2c_write(bus, irq_status, ASPEED_I2C_INTR_STS_REG);
+	spin_unlock(&bus->lock);
+	return true;
+}
+
+static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
+{
+	struct aspeed_i2c_bus *bus = dev_id;
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	if (aspeed_i2c_slave_irq(bus)) {
+		dev_dbg(bus->dev, "irq handled by slave.\n");
+		return IRQ_HANDLED;
+	}
+#endif
+	if (aspeed_i2c_master_irq(bus)) {
+		dev_dbg(bus->dev, "irq handled by master.\n");
+		return IRQ_HANDLED;
+	}
+	dev_err(bus->dev, "irq not handled properly!\n");
+	return IRQ_HANDLED;
+}
+
+static int aspeed_i2c_master_single_xfer(struct i2c_adapter *adap,
+				      struct i2c_msg *msg)
+{
+	struct aspeed_i2c_bus *bus = adap->algo_data;
+	unsigned long flags;
+	u8 slave_addr;
+	u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
+	int ret = msg->len;
+	unsigned long time_left;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	bus->msg = msg;
+	bus->msg_pos = 0;
+	slave_addr = msg->addr << 1;
+	if (msg->flags & I2C_M_RD) {
+		slave_addr |= 1;
+		command |= ASPEED_I2CD_M_RX_CMD;
+		if (msg->len == 1)
+			command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+	}
+	aspeed_i2c_write(bus, slave_addr, ASPEED_I2C_BYTE_BUF_REG);
+	aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
+	reinit_completion(&bus->cmd_complete);
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	time_left = wait_for_completion_interruptible_timeout(
+			&bus->cmd_complete, bus->adap.timeout * HZ * msg->len);
+	if (time_left == 0)
+		return -ETIMEDOUT;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	if (bus->cmd_err)
+		ret = -EIO;
+	bus->msg = NULL;
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	return ret;
+}
+
+static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
+				  struct i2c_msg *msgs, int num)
+{
+	struct aspeed_i2c_bus *bus = adap->algo_data;
+	int ret;
+	int i;
+	unsigned long flags;
+	unsigned long time_left;
+
+	/* If bus is busy, attempt recovery. We assume a single master
+	 * environment.
+	 */
+	if (aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
+	    ASPEED_I2CD_BUS_BUSY_STS) {
+		ret = aspeed_i2c_recover_bus(bus);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < num; i++) {
+		ret = aspeed_i2c_master_single_xfer(adap, &msgs[i]);
+		if (ret < 0)
+			break;
+		/* TODO: Support other forms of I2C protocol mangling. */
+		if (msgs[i].flags & I2C_M_STOP) {
+			spin_lock_irqsave(&bus->lock, flags);
+			aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
+				      ASPEED_I2C_CMD_REG);
+			reinit_completion(&bus->cmd_complete);
+			spin_unlock_irqrestore(&bus->lock, flags);
+
+			time_left = wait_for_completion_interruptible_timeout(
+					&bus->cmd_complete,
+					bus->adap.timeout * HZ);
+			if (time_left == 0)
+				return -ETIMEDOUT;
+		}
+	}
+
+	spin_lock_irqsave(&bus->lock, flags);
+	aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD, ASPEED_I2C_CMD_REG);
+	reinit_completion(&bus->cmd_complete);
+	spin_unlock_irqrestore(&bus->lock, flags);
+
+	time_left = wait_for_completion_interruptible_timeout(
+			&bus->cmd_complete, bus->adap.timeout * HZ);
+	if (time_left == 0)
+		return -ETIMEDOUT;
+
+	/* If nothing went wrong, return number of messages transferred. */
+	if (ret < 0)
+		return ret;
+	else
+		return i;
+}
+
+static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static int aspeed_i2c_reg_slave(struct i2c_client *client)
+{
+	struct aspeed_i2c_bus *bus;
+	unsigned long flags;
+	u32 addr_reg_val;
+	u32 func_ctrl_reg_val;
+
+	bus = client->adapter->algo_data;
+	spin_lock_irqsave(&bus->lock, flags);
+	if (bus->slave) {
+		spin_unlock_irqrestore(&bus->lock, flags);
+		return -EINVAL;
+	}
+
+	/* Set slave addr. */
+	addr_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_DEV_ADDR_REG);
+	addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
+	addr_reg_val |= client->addr & ASPEED_I2CD_DEV_ADDR_MASK;
+	aspeed_i2c_write(bus, addr_reg_val, ASPEED_I2C_DEV_ADDR_REG);
+
+	/* Switch from master mode to slave mode. */
+	func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
+	func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
+	func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
+	aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
+
+	bus->slave = client;
+	bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+	spin_unlock_irqrestore(&bus->lock, flags);
+	return 0;
+}
+
+static int aspeed_i2c_unreg_slave(struct i2c_client *client)
+{
+	struct aspeed_i2c_bus *bus = client->adapter->algo_data;
+	unsigned long flags;
+	u32 func_ctrl_reg_val;
+
+	spin_lock_irqsave(&bus->lock, flags);
+	if (!bus->slave) {
+		spin_unlock_irqrestore(&bus->lock, flags);
+		return -EINVAL;
+	}
+
+	/* Switch from slave mode to master mode. */
+	func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
+	func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
+	func_ctrl_reg_val |= ASPEED_I2CD_MASTER_EN;
+	aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
+
+	bus->slave = NULL;
+	spin_unlock_irqrestore(&bus->lock, flags);
+	return 0;
+}
+#endif
+
+static const struct i2c_algorithm aspeed_i2c_algo = {
+	.master_xfer	= aspeed_i2c_master_xfer,
+	.functionality	= aspeed_i2c_functionality,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+	.reg_slave	= aspeed_i2c_reg_slave,
+	.unreg_slave	= aspeed_i2c_unreg_slave,
+#endif
+};
+
+static u32 aspeed_i2c_get_clk_reg_val(u32 divider_ratio)
+{
+	unsigned int inc = 0, div;
+	u32 scl_low, scl_high, data;
+
+	for (div = 0; divider_ratio >= 16; div++) {
+		inc |= (divider_ratio & 1);
+		divider_ratio >>= 1;
+	}
+	divider_ratio += inc;
+	scl_low = (divider_ratio >> 1) - 1;
+	scl_high = divider_ratio - scl_low - 2;
+	data = 0x77700300 | (scl_high << 16) | (scl_low << 12) | div;
+	return data;
+}
+
+static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
+			    struct platform_device *pdev)
+{
+	struct clk *pclk;
+	u32 clk_freq;
+	u32 divider_ratio;
+	int ret;
+
+	pclk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(pclk)) {
+		dev_err(&pdev->dev, "clk_get failed\n");
+		return PTR_ERR(pclk);
+	}
+	ret = of_property_read_u32(pdev->dev.of_node,
+			"clock-frequency", &clk_freq);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+				"Could not read clock-frequency property\n");
+		clk_freq = 100000;
+	}
+	divider_ratio = clk_get_rate(pclk) / clk_freq;
+	/* We just need the clock rate, we don't actually use the clk object. */
+	devm_clk_put(&pdev->dev, pclk);
+
+	/* Set AC Timing */
+	if (clk_freq / 1000 > 400) {
+		aspeed_i2c_write(bus, aspeed_i2c_read(bus,
+						      ASPEED_I2C_FUN_CTRL_REG) |
+				ASPEED_I2CD_M_HIGH_SPEED_EN |
+				ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
+				ASPEED_I2CD_SDA_DRIVE_1T_EN,
+				ASPEED_I2C_FUN_CTRL_REG);
+
+		aspeed_i2c_write(bus, 0x3, ASPEED_I2C_AC_TIMING_REG2);
+		aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
+			      ASPEED_I2C_AC_TIMING_REG1);
+	} else {
+		aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
+			      ASPEED_I2C_AC_TIMING_REG1);
+		aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
+				 ASPEED_I2C_AC_TIMING_REG2);
+	}
+
+	return 0;
+}
+
+static void noop(struct irq_data *data) { }
+
+static struct irq_chip aspeed_i2c_irqchip = {
+	.name		= "ast-i2c",
+	.irq_unmask	= noop,
+	.irq_mask	= noop,
+};
+
+static int aspeed_i2c_probe_bus(struct platform_device *pdev)
+{
+	struct aspeed_i2c_bus *bus;
+	struct aspeed_i2c_controller *controller =
+			dev_get_drvdata(pdev->dev.parent);
+	struct resource *res;
+	int ret, irq;
+	u32 hwirq;
+
+	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
+	if (!bus)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	bus->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(bus->base))
+		return PTR_ERR(bus->base);
+
+	bus->irq = platform_get_irq(pdev, 0);
+	if (bus->irq < 0)
+		return -ENXIO;
+	ret = of_property_read_u32(pdev->dev.of_node, "interrupts", &hwirq);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "no I2C 'interrupts' property\n");
+		return -ENXIO;
+	}
+	irq = irq_create_mapping(controller->irq_domain, hwirq);
+	irq_set_chip_data(irq, controller);
+	irq_set_chip_and_handler(irq, &aspeed_i2c_irqchip, handle_simple_irq);
+	ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
+			0, dev_name(&pdev->dev), bus);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request interrupt\n");
+		return -ENXIO;
+	}
+
+	/* Initialize the I2C adapter */
+	spin_lock_init(&bus->lock);
+	init_completion(&bus->cmd_complete);
+	bus->adap.owner = THIS_MODULE;
+	bus->adap.retries = 0;
+	bus->adap.timeout = 5;
+	bus->adap.algo = &aspeed_i2c_algo;
+	bus->adap.algo_data = bus;
+	bus->adap.dev.parent = &pdev->dev;
+	bus->adap.dev.of_node = pdev->dev.of_node;
+	snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c");
+
+	bus->dev = &pdev->dev;
+
+	/* reset device: disable master & slave functions */
+	aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
+
+	ret = aspeed_i2c_init_clk(bus, pdev);
+	if (ret < 0)
+		return ret;
+
+	/* Enable Master Mode */
+	aspeed_i2c_write(bus, aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG) |
+		      ASPEED_I2CD_MASTER_EN |
+		      ASPEED_I2CD_MULTI_MASTER_DIS, ASPEED_I2C_FUN_CTRL_REG);
+
+	/* Set interrupt generation of I2C controller */
+	aspeed_i2c_write(bus, ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
+			ASPEED_I2CD_INTR_BUS_RECOVER_DONE |
+			ASPEED_I2CD_INTR_SCL_TIMEOUT |
+			ASPEED_I2CD_INTR_ABNORMAL |
+			ASPEED_I2CD_INTR_NORMAL_STOP |
+			ASPEED_I2CD_INTR_ARBIT_LOSS |
+			ASPEED_I2CD_INTR_RX_DONE |
+			ASPEED_I2CD_INTR_TX_NAK |
+			ASPEED_I2CD_INTR_TX_ACK,
+			ASPEED_I2C_INTR_CTRL_REG);
+
+	ret = i2c_add_adapter(&bus->adap);
+	if (ret < 0)
+		return -ENXIO;
+
+	platform_set_drvdata(pdev, bus);
+
+	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
+			bus->adap.nr, bus->irq);
+
+	return 0;
+}
+
+static int aspeed_i2c_remove_bus(struct platform_device *pdev)
+{
+	struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
+
+	i2c_del_adapter(&bus->adap);
+	return 0;
+}
+
+static const struct of_device_id aspeed_i2c_bus_of_table[] = {
+	{ .compatible = "aspeed,ast2400-i2c-bus", },
+	{ .compatible = "aspeed,ast2500-i2c-bus", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
+
+static struct platform_driver aspeed_i2c_bus_driver = {
+	.probe		= aspeed_i2c_probe_bus,
+	.remove		= aspeed_i2c_remove_bus,
+	.driver		= {
+		.name		= "ast-i2c-bus",
+		.of_match_table	= aspeed_i2c_bus_of_table,
+	},
+};
+
+static void aspeed_i2c_controller_irq(struct irq_desc *desc)
+{
+	struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
+	unsigned long p, status;
+	unsigned int bus_irq;
+
+	status = readl(c->base);
+	for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
+		bus_irq = irq_find_mapping(c->irq_domain, p);
+		generic_handle_irq(bus_irq);
+	}
+}
+
+static int aspeed_i2c_probe_controller(struct platform_device *pdev)
+{
+	struct aspeed_i2c_controller *controller;
+	struct device_node *np;
+	struct resource *res;
+
+	controller = kzalloc(sizeof(*controller), GFP_KERNEL);
+	if (!controller)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	controller->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(controller->base))
+		return PTR_ERR(controller->base);
+
+	controller->irq = platform_get_irq(pdev, 0);
+	if (controller->irq < 0)
+		return -ENXIO;
+
+	controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
+			ASPEED_I2C_NUM_BUS, &irq_domain_simple_ops, NULL);
+	if (!controller->irq_domain)
+		return -ENXIO;
+	controller->irq_domain->name = "ast-i2c-domain";
+
+	irq_set_chained_handler_and_data(controller->irq,
+			aspeed_i2c_controller_irq, controller);
+
+	controller->dev = &pdev->dev;
+
+	platform_set_drvdata(pdev, controller);
+
+	dev_info(controller->dev, "i2c controller registered, irq %d\n",
+			controller->irq);
+
+	for_each_child_of_node(pdev->dev.of_node, np) {
+		of_platform_device_create(np, NULL, &pdev->dev);
+		of_node_put(np);
+	}
+
+	return 0;
+}
+
+static int aspeed_i2c_remove_controller(struct platform_device *pdev)
+{
+	struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
+
+	irq_domain_remove(controller->irq_domain);
+	return 0;
+}
+
+static const struct of_device_id aspeed_i2c_controller_of_table[] = {
+	{ .compatible = "aspeed,ast2400-i2c-controller", },
+	{ .compatible = "aspeed,ast2500-i2c-controller", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_controller_of_table);
+
+static struct platform_driver aspeed_i2c_controller_driver = {
+	.probe		= aspeed_i2c_probe_controller,
+	.remove		= aspeed_i2c_remove_controller,
+	.driver		= {
+		.name		= "ast-i2c-controller",
+		.of_match_table	= aspeed_i2c_controller_of_table,
+	},
+};
+
+static int __init aspeed_i2c_driver_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&aspeed_i2c_controller_driver);
+	if (ret < 0)
+		return ret;
+	return platform_driver_register(&aspeed_i2c_bus_driver);
+}
+module_init(aspeed_i2c_driver_init);
+
+static void __exit aspeed_i2c_driver_exit(void)
+{
+	platform_driver_unregister(&aspeed_i2c_bus_driver);
+	platform_driver_unregister(&aspeed_i2c_controller_driver);
+}
+module_exit(aspeed_i2c_driver_exit);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
+MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
+MODULE_LICENSE("GPL");
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related

* [PATCH v4 0/2] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2016-11-05  1:58 UTC (permalink / raw)
  To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, joel-U3u1mxZcP9KHXe+LvDLADg,
	openbmc-uLR06cmDAlY/bJ5BZ2RsiQ

Sorry, been a while since I was able to send an update for this.

As a refresher, I added a master mode and slave mode I2C driver for the Aspeed
24XX/25XX SoC as well as device binding documentation.

Changes since first update:
  - I have removed the bus device tree param as requested here:
    http://www.spinics.net/lists/devicetree/msg143850.html
  - I2C adapter number is now generated dynamically unless an alias is provided
  - I changed the Kconfig for the driver to depend only on ARCH_ASPEED
  - Replaced the two module_platform_driver registrations with single
    module_init

Changes have been tested on the Aspeed evaluation board.
--
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 v2 1/3] remoteproc: qcom: Encapsulate pvt data structure for q6v56 hexagon.
From: kbuild test robot @ 2016-11-04 23:51 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A,
	Avaneesh Kumar Dwivedi, Ohad Ben-Cohen, Rob Herring, Mark Rutland,
	open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list
In-Reply-To: <1478268057-11847-2-git-send-email-akdwived-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 7190 bytes --]

Hi Avaneesh,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.9-rc3 next-20161028]
[cannot apply to remoteproc/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Avaneesh-Kumar-Dwivedi/remoteproc-qcom-Encapsulate-pvt-data-structure-for-q6v56-hexagon/20161104-220712
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

Note: the linux-review/Avaneesh-Kumar-Dwivedi/remoteproc-qcom-Encapsulate-pvt-data-structure-for-q6v56-hexagon/20161104-220712 HEAD 1b4c0b8bb3bb8cd30a996282b7a6aa9f352836a2 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/remoteproc/qcom_q6v5_pil.c: In function 'q6_probe':
   drivers/remoteproc/qcom_q6v5_pil.c:848:7: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     desc = of_device_get_match_data(&pdev->dev);
          ^
   drivers/remoteproc/qcom_q6v5_pil.c: At top level:
>> drivers/remoteproc/qcom_q6v5_pil.c:986:19: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     .q6_reset_init = q6v5_init_reset,
                      ^~~~~~~~~~~~~~~
   drivers/remoteproc/qcom_q6v5_pil.c:986:19: note: (near initialization for 'msm_8916_res.q6_reset_init')
   cc1: some warnings being treated as errors

vim +986 drivers/remoteproc/qcom_q6v5_pil.c

   842	{
   843		struct q6v5 *qproc;
   844		struct rproc *rproc;
   845		struct q6_rproc_res *desc;
   846		int ret;
   847	
 > 848		desc = of_device_get_match_data(&pdev->dev);
   849		if (!desc)
   850			return -EINVAL;
   851	
   852		rproc = rproc_alloc(&pdev->dev, pdev->name, &q6_ops,
   853				    desc->q6_mba_image, sizeof(*qproc));
   854		if (!rproc) {
   855			dev_err(&pdev->dev, "failed to allocate rproc\n");
   856			return -ENOMEM;
   857		}
   858	
   859		rproc->fw_ops = &q6_fw_ops;
   860	
   861		qproc = (struct q6v5 *)rproc->priv;
   862		qproc->dev = &pdev->dev;
   863		qproc->rproc = rproc;
   864		platform_set_drvdata(pdev, qproc);
   865	
   866		init_completion(&qproc->start_done);
   867		init_completion(&qproc->stop_done);
   868	
   869		qproc->q6_rproc_res = desc;
   870		ret = q6v5_init_mem(qproc, pdev);
   871		if (ret)
   872			goto free_rproc;
   873	
   874		ret = q6v5_alloc_memory_region(qproc);
   875		if (ret)
   876			goto free_rproc;
   877	
   878		ret = q6v5_init_clocks(qproc);
   879		if (ret)
   880			goto free_rproc;
   881	
   882		ret = q6v5_regulator_init(qproc);
   883		if (ret)
   884			goto free_rproc;
   885	
   886		ret = qproc->q6_rproc_res->q6_reset_init(qproc, pdev);
   887		if (ret)
   888			goto free_rproc;
   889	
   890		ret = q6v5_request_irq(qproc, pdev, "wdog", q6v5_wdog_interrupt);
   891		if (ret < 0)
   892			goto free_rproc;
   893	
   894		ret = q6v5_request_irq(qproc, pdev, "fatal", q6v5_fatal_interrupt);
   895		if (ret < 0)
   896			goto free_rproc;
   897	
   898		ret = q6v5_request_irq(qproc, pdev, "handover", q6v5_handover_interrupt);
   899		if (ret < 0)
   900			goto free_rproc;
   901	
   902		ret = q6v5_request_irq(qproc, pdev, "stop-ack", q6v5_stop_ack_interrupt);
   903		if (ret < 0)
   904			goto free_rproc;
   905	
   906		qproc->state = qcom_smem_state_get(&pdev->dev, "stop", &qproc->stop_bit);
   907		if (IS_ERR(qproc->state))
   908			goto free_rproc;
   909	
   910		ret = rproc_add(rproc);
   911		if (ret)
   912			goto free_rproc;
   913	
   914		return 0;
   915	
   916	free_rproc:
   917		rproc_put(rproc);
   918	
   919		return ret;
   920	}
   921	
   922	static int q6_remove(struct platform_device *pdev)
   923	{
   924		struct q6v5 *qproc = platform_get_drvdata(pdev);
   925	
   926		rproc_del(qproc->rproc);
   927		rproc_put(qproc->rproc);
   928	
   929		return 0;
   930	}
   931	
   932	char *proxy_8x96_reg_str[] = {"mx", "cx", "vdd_pll"};
   933	int  proxy_8x96_reg_action[3][2] = { {0, 1}, {1, 1}, {1, 0} };
   934	int  proxy_8x96_reg_load[] = {0, 100000, 100000};
   935	int  proxy_8x96_reg_min_voltage[] = {1050000, 1250000, 0};
   936	char *proxy_8x96_clk_str[] = {"xo", "pnoc", "qdss"};
   937	char *active_8x96_clk_str[] = {"iface", "bus", "mem", "gpll0_mss_clk",
   938			"snoc_axi_clk", "mnoc_axi_clk"};
   939	
   940	static struct q6_rproc_res msm_8996_res = {
   941		.proxy_clks = proxy_8x96_clk_str,
   942		.proxy_clk_cnt = 3,
   943		.active_clks = active_8x96_clk_str,
   944		.active_clk_cnt = 6,
   945		.proxy_regs = proxy_8x96_reg_str,
   946		.active_regs = NULL,
   947		.proxy_reg_action = (int **)proxy_8x96_reg_action,
   948		.proxy_reg_load = (int *)proxy_8x96_reg_load,
   949		.active_reg_action = NULL,
   950		.active_reg_load = NULL,
   951		.proxy_reg_voltage = (int *)proxy_8x96_reg_min_voltage,
   952		.active_reg_voltage = NULL,
   953		.proxy_reg_cnt = 3,
   954		.active_reg_cnt = 0,
   955		.q6_reset_init = q6v56_init_reset,
   956		.q6_version = "v56",
   957		.q6_mba_image = "mba.mbn",
   958	};
   959	
   960	char *proxy_8x16_reg_str[] = {"mx", "cx", "pll"};
   961	char *active_8x16_reg_str[] = {"mss"};
   962	int  proxy_8x16_reg_action[4][2] = { {0, 1}, {1, 0}, {1, 0} };
   963	int  active_8x16_reg_action[1][2] = { {1, 1} };
   964	int  proxy_8x16_reg_load[] = {100000, 0, 100000, 100000};
   965	int  active_8x16_reg_load[] = {100000};
   966	int  proxy_8x16_reg_min_voltage[] = {1050000, 0, 0};
   967	int  active_8x16_reg_min_voltage[] = {1000000};
   968	char *proxy_8x16_clk_str[] = {"xo"};
   969	char *active_8x16_clk_str[] = {"iface", "bus", "mem"};
   970	
   971	static struct q6_rproc_res msm_8916_res = {
   972		.proxy_clks = proxy_8x16_clk_str,
   973		.proxy_clk_cnt = 1,
   974		.active_clks = active_8x16_clk_str,
   975		.active_clk_cnt = 3,
   976		.proxy_regs = proxy_8x16_reg_str,
   977		.active_regs = active_8x16_reg_str,
   978		.proxy_reg_action = (int **)proxy_8x16_reg_action,
   979		.proxy_reg_load = (int *)proxy_8x16_reg_load,
   980		.active_reg_action = (int **)active_8x16_reg_action,
   981		.active_reg_load = (int *)active_8x16_reg_load,
   982		.proxy_reg_voltage = (int *)proxy_8x16_reg_min_voltage,
   983		.active_reg_voltage = active_8x16_reg_min_voltage,
   984		.proxy_reg_cnt = 3,
   985		.active_reg_cnt = 1,
 > 986		.q6_reset_init = q6v5_init_reset,
   987		.q6_version = "v5",
   988		.q6_mba_image = "mba.b00",
   989	};

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51385 bytes --]

^ permalink raw reply

* Re: [v17 2/2] drm/bridge: Add I2C based driver for ps8640 bridge
From: Daniel Kurtz @ 2016-11-04 23:21 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Enric Balletbo Serra, Jitao Shi, David Airlie, Thierry Reding,
	Mark Rutland, stonea168, dri-devel, Andy Yan, Ajay Kumar,
	Vincent Palatin, cawa cheng,
	Bibby Hsieh (謝濟遠),
	CK Hu (胡俊光), Russell King,
	devicetree@vger.kernel.org, Sascha Hauer, Pawel Moll,
	Ian Campbell
In-Reply-To: <e00519e2-990c-b9b2-71d4-a4c8d36f5e65@gmail.com>

On Tue, Oct 25, 2016 at 6:23 AM, Matthias Brugger
<matthias.bgg@gmail.com> wrote:
>
> On 10/18/2016 04:37 PM, Enric Balletbo Serra wrote:
> [...]
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> [...]
>>>
>>> +
>>> +/* Firmware */
>>> +#define PS_FW_NAME             "ps864x_fw.bin"
>>> +
>>
>> From where I can download this firmware image?
>
> I suppose this FW bits have to be added to linux-firmware repository first, before this patch can be accepted.

All PS8640 devices should already ship with working firmware.
The firmware update procedure is only used in the unlikely event where
one wants to update the bridge to a different firmware provided by
Parade.

Why must the lack of firmware really block landing this driver?

If this is really so, can we just land the functional part of the
driver first, and add the firmware update in a follow-up patch.

>
> Regards,
> Matthias

^ permalink raw reply

* Re: [PATCH] Documentation: bindings: fix twl-rtc documentation
From: Alexandre Belloni @ 2016-11-04 23:09 UTC (permalink / raw)
  To: Nicolae Rosia
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Alessandro Zummo
In-Reply-To: <1477993356-9195-1-git-send-email-Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

On 01/11/2016 at 11:42:36 +0200, Nicolae Rosia wrote :
> Add interrupts documentation
> Fix example and formatting
> 
> Signed-off-by: Nicolae Rosia <Nicolae_Rosia-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/rtc/twl-rtc.txt | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
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 1/7] rtc: rtc-jz4740: Add support for the RTC in the jz4780 SoC
From: Alexandre Belloni @ 2016-11-04 23:08 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Alessandro Zummo, Rob Herring,
	Mark Rutland, Ralf Baechle, Maarten ter Huurne,
	Lars-Peter Clausen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA
In-Reply-To: <20161031203951.5444-1-paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>

On 31/10/2016 at 21:39:45 +0100, Paul Cercueil wrote :
> The RTC unit present in the JZ4780 works mostly the same as the one in
> the JZ4740. The major difference is that register writes need to be
> explicitly enabled, by writing a magic code (0xA55A) to a "write
> enable" register before each access.
> 
> Signed-off-by: Paul Cercueil <paul-icTtO2rgO2OTuSrc4Mpeew@public.gmane.org>
> Acked-by: Maarten ter Huurne <maarten-Ph2Y2OKCxY1M656bX5wj8A@public.gmane.org>
> ---
>  drivers/rtc/Kconfig      |  6 +++---
>  drivers/rtc/rtc-jz4740.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 51 insertions(+), 5 deletions(-)
> 
> v2: No change
> v3: No change
> 

All applied, thanks


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH V5] pinctrl: qcom: Add msm8994 pinctrl driver
From: Linus Walleij @ 2016-11-04 22:58 UTC (permalink / raw)
  To: Michael Scott
  Cc: linux-gpio@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Rob Herring, Mark Rutland, Andy Gross, David Brown,
	Bjorn Andersson, Joonwoo Park, Jeremy McNicoll, Stephen Boyd
In-Reply-To: <20161103211526.6095-1-michael.scott@linaro.org>

On Thu, Nov 3, 2016 at 10:15 PM, Michael Scott <michael.scott@linaro.org> wrote:

> Initial pinctrl driver for QCOM msm8994 platforms.
>
> In order to continue the initial board support for QCOM msm8994/msm8992
> presented in patches from Jeremy McNicoll <jeremymc@redhat.com>, let's put
> a proper pinctrl driver in place.
>
> Currently, the DT for these platforms uses the msm8x74 pinctrl driver to
> enable basic UART.  Beyond the first few pins the rest are different enough
> to justify it's own driver.
>
> Note: This driver is also used by QCOM's msm8992 platform as it's TLM block
> is the same.
>
> - Initial formatting and style was taken from the msm8x74 pinctrl driver
>   added by Björn Andersson <bjorn.andersson@linaro.org>
> - Data was then adjusted per QCOM MSM8994v2 documentation for Top Level
>   Multiplexing
> - Bindings documentation was based on qcom,msm8996-pinctrl.txt by
>   Joonwoo Park <joonwoop@codeaurora.org> and then modified for msm8994
>   content
>
> Signed-off-by: Michael Scott <michael.scott@linaro.org>
> Tested-by: Jeremy McNicoll <jeremymc@redhat.com>
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> V5:

This v5 version applied.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 1/2] pinctrl: tegra: Add DT binding for io pads control
From: Linus Walleij @ 2016-11-04 22:29 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: thierry.reding@gmail.com, Stephen Warren, Rob Herring,
	Mark Rutland, Jon Hunter, Masahiro Yamada,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1478077742-25437-2-git-send-email-ldewangan@nvidia.com>

On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> NVIDIA Tegra124 and later SoCs support the multi-voltage level and
> low power state of some of its IO pads. The IO pads can work in
> the voltage of the 1.8V and 3.3V of IO power rail sources. When IO
> interface are not used then IO pads can be configure in low power
> state to reduce the power from that IO pads.
>
> On Tegra124, the IO power rail source is auto detected by SoC and hence
> it is only require to configure in low power mode if IO pads are not
> used.
>
> On T210 onwards, the auto-detection is removed from SoC and hence SW
> must configure the PMC register explicitly to set proper voltage in
> IO pads based on IO rail power source voltage.
>
> Add DT binding document for detailing the DT properties for
> configuring IO pads voltage levels and its power state.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
(...)

> +-nvidia,power-source-voltage:  Integer. The voltage level of IO pads. The
> +                               valid values are 1.8V and 3.3V. Macros are
> +                               defined for these voltage levels in
> +                               <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
> +                               Use TEGRA_IO_PAD_POWER_SOURCE_1800000UV for 1.8V
> +                               Use TEGRA_IO_PAD_POWER_SOURCE_3300000UV for 3.3V
> +
> +                               All IO pads do not support the 1.8V/3.3V
> +                               configurations. Valid values for "pins" are
> +                               audio-hv, dmic, gpio, sdmmc1, sdmmc3, spi-hv.


As mentioned in another patch, what is wrong with the standard
power-source binding?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Linus Walleij @ 2016-11-04 22:24 UTC (permalink / raw)
  To: Laxman Dewangan
  Cc: thierry.reding@gmail.com, Stephen Warren, Rob Herring,
	Mark Rutland, Jon Hunter, Masahiro Yamada,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1478077742-25437-3-git-send-email-ldewangan@nvidia.com>

On Wed, Nov 2, 2016 at 10:09 AM, Laxman Dewangan <ldewangan@nvidia.com> wrote:

> NVIDIA Tegra124 and later SoCs support the multi-voltage level and
> low power state of some of its IO pads. The IO pads can work in
> the voltage of the 1.8V and 3.3V of IO power rail sources. When IO
> interface are not used then IO pads can be configure in low power
> state to reduce the power from that IO pads.
>
> On Tegra124, the IO power rail source is auto detected by SoC and hence
> it is only require to configure in low power mode if IO pads are not
> used.
>
> On T210 onwards, the auto-detection is removed from SoC and hence SW
> must configure the PMC register explicitly to set proper voltage in
> IO pads based on IO rail power source voltage.
>
> This driver adds the IO pad driver to configure the power state and
> IO pad voltage based on the usage and power tree via pincontrol
> framework. The configuration can be static and dynamic.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Looking for an ACK from Stephen &| Thierry.

> ---
> On top of the branch from Thierry's T186 work
>         https://github.com/thierryreding/linux/tree/tegra186

But it's an orthogonal patch right?

The build robot seems to have problems with it so pls fix these.

> +static const struct pinconf_generic_params tegra_io_pads_cfg_params[] = {
> +       {
> +               .property = "nvidia,power-source-voltage",
> +               .param = TEGRA_IO_PAD_POWER_SOURCE_VOLTAGE,
> +       },
> +};

Why can you not use the standard power-source binding
from Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
instead of inventing this nvidia,* variant?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v2 2/3] irqchip: mtk-cirq: Add mediatek mtk-cirq implement
From: Marc Zyngier @ 2016-11-04 22:21 UTC (permalink / raw)
  To: Youlin Pei
  Cc: Rob Herring, Matthias Brugger, Thomas Gleixner, Jason Cooper,
	Mark Rutland, Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
	hongkun.cao-NuS5LvNUpcJWk0Htik3J/w,
	yong.wu-NuS5LvNUpcJWk0Htik3J/w, erin.lo-NuS5LvNUpcJWk0Htik3J/w,
	chieh-jay.liu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <1478234577.7975.29.camel@mtksdaap41>

On Fri, Nov 04 2016 at 04:42:57 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> On Tue, 2016-11-01 at 20:49 +0000, Marc Zyngier wrote:
>> On Tue, Nov 01 2016 at 11:52:01 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> > In Mediatek SOCs, the CIRQ is a low power interrupt controller
>> > designed to works outside MCUSYS which comprises with Cortex-Ax
>> > cores,CCI and GIC.
>> >
>> > The CIRQ controller is integrated in between MCUSYS( include
>> > Cortex-Ax, CCI and GIC ) and interrupt sources as the second
>> > level interrupt controller. The external interrupts which outside
>> > MCUSYS will feed through CIRQ then bypass to GIC. CIRQ can monitors
>> > all edge trigger interupts. When an edge interrupt is triggered,
>> > CIRQ can record the status and generate a pulse signal to GIC when
>> > flush command executed.
>> >
>> > When system enters sleep mode, MCUSYS will be turned off to improve
>> > power consumption, also GIC is power down. The edge trigger interrupts
>> > will be lost in this scenario without CIRQ.
>> >
>> > This commit provides the CIRQ irqchip implement.
>> >
>> > Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> > ---
>> >  drivers/irqchip/Makefile       |    2 +-
>> >  drivers/irqchip/irq-mtk-cirq.c |  262 ++++++++++++++++++++++++++++++++++++++++
>> >  2 files changed, 263 insertions(+), 1 deletion(-)
>> >  create mode 100644 drivers/irqchip/irq-mtk-cirq.c
>> >
>> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> > index e4dbfc8..8f33580 100644
>> > --- a/drivers/irqchip/Makefile
>> > +++ b/drivers/irqchip/Makefile
>> > @@ -60,7 +60,7 @@ obj-$(CONFIG_BCM7120_L2_IRQ)		+= irq-bcm7120-l2.o
>> >  obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
>> >  obj-$(CONFIG_KEYSTONE_IRQ)		+= irq-keystone.o
>> >  obj-$(CONFIG_MIPS_GIC)			+= irq-mips-gic.o
>> > -obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o
>> > +obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o irq-mtk-cirq.o
>> >  obj-$(CONFIG_ARCH_DIGICOLOR)		+= irq-digicolor.o
>> >  obj-$(CONFIG_RENESAS_H8300H_INTC)	+= irq-renesas-h8300h.o
>> >  obj-$(CONFIG_RENESAS_H8S_INTC)		+= irq-renesas-h8s.o
>> > diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
>> > new file mode 100644
>> > index 0000000..fc43ef3
>> > --- /dev/null
>> > +++ b/drivers/irqchip/irq-mtk-cirq.c
>> > @@ -0,0 +1,262 @@
>> > +/*
>> > + * Copyright (c) 2016 MediaTek Inc.
>> > + * Author: Youlin.Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > + * GNU General Public License for more details.
>> > + */
>> > +
>> > +#include <linux/irq.h>
>> > +#include <linux/irqchip.h>
>> > +#include <linux/irqdomain.h>
>> > +#include <linux/of.h>
>> > +#include <linux/of_irq.h>
>> > +#include <linux/of_address.h>
>> > +#include <linux/io.h>
>> > +#include <linux/slab.h>
>> > +#include <linux/syscore_ops.h>
>> > +
>> > +#define CIRQ_ACK	0x40
>> > +#define CIRQ_MASK_SET	0xc0
>> > +#define CIRQ_MASK_CLR	0x100
>> > +#define CIRQ_SENS_SET	0x180
>> > +#define CIRQ_SENS_CLR	0x1c0
>> > +#define CIRQ_POL_SET	0x240
>> > +#define CIRQ_POL_CLR	0x280
>> > +#define CIRQ_CONTROL	0x300
>> > +
>> > +#define CIRQ_EN	0x1
>> > +#define CIRQ_EDGE	0x2
>> > +#define CIRQ_FLUSH	0x4
>> > +
>> > +#define CIRQ_IRQ_NUM    0x200
>> > +
>> > +struct mtk_cirq_chip_data {
>> > +	void __iomem *base;
>> > +	unsigned int ext_irq_start;
>> > +};
>> > +
>> > +static struct mtk_cirq_chip_data *cirq_data;
>> 
>> Are you guaranteed that you'll only ever have a single CIRQ in any
>> system?
>
> In Mediatek's SOC, only hace a single CIRQ.
>
>> 
>> > +
>> > +static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
>> > +{
>> > +	struct mtk_cirq_chip_data *chip_data = data->chip_data;
>> > +	unsigned int cirq_num = data->hwirq;
>> > +	u32 mask = 1 << (cirq_num % 32);
>> > +
>> > +	writel(mask, chip_data->base + offset + (cirq_num / 32) * 4);
>> 
>> Why can't you use the relaxed accessors?
>
> It seems that i use wrong function, i will change the writel to
> writel_relaxed in next version.
>
>> 
>> > +}
>> > +
>> > +static void mtk_cirq_mask(struct irq_data *data)
>> > +{
>> > +	mtk_cirq_write_mask(data, CIRQ_MASK_SET);
>> > +	irq_chip_mask_parent(data);
>> > +}
>> > +
>> > +static void mtk_cirq_unmask(struct irq_data *data)
>> > +{
>> > +	mtk_cirq_write_mask(data, CIRQ_MASK_CLR);
>> > +	irq_chip_unmask_parent(data);
>> > +}
>> > +
>> > +static void mtk_cirq_eoi(struct irq_data *data)
>> > +{
>> > +	mtk_cirq_write_mask(data, CIRQ_ACK);
>> 
>> EOI and ACK have very different semantics. What is this write actually
>> doing? Also, you're now doing an additional MMIO write on each interrupt
>> EOI, doubling its cost. Do you really need to do actually signal the HW
>> that we've EOIed an interrupt? I would have hoped that you'd be able to
>> put it in "bypass" mode as long as you're not suspending...
>> 
>
> When external interrupt happened, CIRQ status register record the status
> even CIRQ is not enabled. when execute the flush command, CIRQ will
> resend the signals according to the status. So if don't clear the
> status, CIRQ will resend the wrong signals. the ACK write operation will
> clear the status.

But at this time, we haven't suspended yet, and there is nothing to
replay. Also, you only enable the edge capture when suspending. So what
are you ACKing here? Can't you simply clear everything right when
suspending and not do it at all on the fast path?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.
--
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] pinctrl: meson: Add GXL pinctrl definitions
From: Linus Walleij @ 2016-11-04 22:07 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Kevin Hilman, Carlo Caione, linux-gpio@vger.kernel.org,
	open list:ARM/Amlogic Meson...,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <1477931531-27120-1-git-send-email-narmstrong@baylibre.com>

On Mon, Oct 31, 2016 at 5:32 PM, Neil Armstrong <narmstrong@baylibre.com> wrote:

> Add support for the Amlogic Meson GXL SoC, this is a partially complete
> definition only based on the Amlogic Vendor tree.
>
> This definition differs a lot from the GXBB and needs a separate entry.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>

Patch applied with Kevin's ACK.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v2 11/11] ARM: dts: sk-rzg1e: add Ether support
From: Sergei Shtylyov @ 2016-11-04 22:05 UTC (permalink / raw)
  To: horms, linux-renesas-soc, robh+dt, mark.rutland, devicetree
  Cc: magnus.damm, linux, linux-arm-kernel
In-Reply-To: <2368353.xfo5beGC5E@wasted.cogentembedded.com>

Define the SK-RZG1E board dependent part of the Ether device node.
Enable DHCP and NFS root  for the kernel booting.

Based on the original (and large) patch by Dmitry Shifrin
<dmitry.shifrin@cogentembedded.com>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

---
Changes in version 2:
- added Geert's tag.

 arch/arm/boot/dts/r8a7745-sk-rzg1e.dts |   15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Index: renesas/arch/arm/boot/dts/r8a7745-sk-rzg1e.dts
===================================================================
--- renesas.orig/arch/arm/boot/dts/r8a7745-sk-rzg1e.dts
+++ renesas/arch/arm/boot/dts/r8a7745-sk-rzg1e.dts
@@ -20,7 +20,7 @@
 	};
 
 	chosen {
-		bootargs = "ignore_loglevel";
+		bootargs = "ignore_loglevel rw root=/dev/nfs ip=dhcp";
 		stdout-path = "serial0:115200n8";
 	};
 
@@ -37,3 +37,16 @@
 &scif2 {
 	status = "okay";
 };
+
+&ether {
+	phy-handle = <&phy1>;
+	renesas,ether-link-active-low;
+	status = "okay";
+
+	phy1: ethernet-phy@1 {
+		reg = <1>;
+		interrupt-parent = <&irqc>;
+		interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
+		micrel,led-mode = <1>;
+	};
+};

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox