* [linux-sunxi] GSoC 2014 #1 status report - Improving Allwinner SoC support
From: Julian Calaby @ 2014-07-20 9:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53CAC9ED.9080302@elopez.com.ar>
Hi Emilio,
On Sun, Jul 20, 2014 at 5:41 AM, Emilio L?pez <emilio@elopez.com.ar> wrote:
> Hi everyone,
>
> Here's this week's update on my GSoC project; if you missed the first issue
> or you want a refresher of what this is about, you can read it on the list
> archives[0]
>
> A couple of days after the last report, and with the help of Jon Smirl, I
> got the hardware working on mainline Linux, first on only 16 bit stereo
> mode, then mono as well, and later on, also on 24 bit mode. The first thing
> I had to do was rework the cyclic DMA code to make it perform decently and
> avoid underruns. The rest took a bit of playing with values and reading the
> Allwinner documentation. There is one unresolved issue with 24 bit audio
> still - for some reason, the volume is really low compared to the same track
> played on 16 bit. Unfortunately the SDK driver doesn't support 24 bit, so
> it's hard to compare with anything else.
It might be worth trying to generate some 24 bit audio samples that
only exercise one bit, then check that playing them produces output,
because it sounds to me (and Chen-Yu) that there's something in your
code that's shifting or clipping the samples.
Thanks,
--
Julian Calaby
Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/
^ permalink raw reply
* [PATCH] arm: Fix me in bios32.c
From: Russell King - ARM Linux @ 2014-07-20 9:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405829179-19476-1-git-send-email-xerofoify@gmail.com>
On Sun, Jul 20, 2014 at 12:06:19AM -0400, Nicholas Krause wrote:
> This fixs a fix me in bios32.c for pci_fixup_it8152 as this if
> statement is incorrect needs to be checked against the class bits
> not the whole address for the two or conditions and since they don't
> have define statements outside of their numeratical value.
Unfortunately, this does not address the FIXME at all. The FIXME does
not state that the if statement is incorrect at all.
> - /* fixup for ITE 8152 devices */
> - /* FIXME: add defines for class 0x68000 and 0x80103 */
> if ((dev->class >> 8) == PCI_CLASS_BRIDGE_HOST ||
> - dev->class == 0x68000 ||
> - dev->class == 0x80103) {
> + (dev->class >> 8) == 0x680 ||
> + (dev->class >> 8) == 0x801) {
The FIXME states that there should be defines for these values (0x68000
and 0x80103).
The FIXME statement itself is slightly wrong in that these values are
*not* class IDs. 0x680 and 0x801 are the class ID values.
We have definitions for class IDs 0x680 and 0x801 in
include/linux/pci_ids.h, which are:
#define PCI_CLASS_BRIDGE_OTHER 0x0680
#define PCI_CLASS_SYSTEM_DMA 0x0801
So, to fix the stated issue, without changing the functionality of the
code, this is what I expected you to produce:
/* fixup for ITE 8152 devices */
if ((dev->class >> 8) == PCI_CLASS_BRIDGE_HOST ||
dev->class == PCI_CLASS_BRIDGE_OTHER << 8 ||
def->class == PCI_CLASS_SYSTEM_DMA << 8 | 0x03)
The reason is that "PCI_CLASS_BRIDGE_OTHER << 8" evaluates to 0x68000
and "PCI_CLASS_SYSTEM_DMA << 8 | 0x03" evaluates to 0x80103. Therefore,
the compiled code is the same as the original, because the actual
numerical check is the same as it always was.
As your solution results in a functional change to the code, that would
introduce a new bug - it results in this test matching any PCI device
with any programming interface for class "Bridge other" and "System DMA",
whereas before the code was looking for a specific value there.
Whether this results in the code incorrectly matching devices on the
system(s) which use this code is difficult to know, so my only choice
here is to reject your change.
This is why my fellow kernel developers are asking you to stop trying to
fix this stuff - unless you can produce provably correct patches (or at
least patches that appear to have no functional change) then you are
burdening people with your education, and you will get yourself an
undesirable reputation.
Even with patches which appear to have no functional change, some
maintainers won't take them unless they have actually been tested on
real hardware, or that other people agree that the change is a correct
one. That is because there is a long history of apparantly correct
changes being made which result in subtle bugs.
Bear in mind that a "FIXME" comment indicates that something is not
fully correct in some way, and even though it may not be fully correct,
the resulting code _works_ on the devices that it has been tested with.
Fixing the "FIXME" may result in the code stopping working on those
devices - especially if it changes the functionality of the code like
your patch above.
So, it's often best to leave FIXMEs alone if you don't know what the
right solution should be.
I hope you will pause, and think about the issues I've raised before
continuing with your current project of trying to fix FIXMEs.
--
FTTC broadband for 0.8mile line: currently@9.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Edma.c: Fix Mes
From: Russell King - ARM Linux @ 2014-07-20 9:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDOMVhd-mfNfT+DbHWbzQA8sb3xpqW6a0WEuTTeCGyK8+kRNQ@mail.gmail.com>
On Sun, Jul 20, 2014 at 12:27:37AM -0400, Nick Krause wrote:
> Hey Russell and others,
> Sorry to bug you again but I have a fix questions before I clean up
> the fix mes in this file.
> Is edma_clean_channel needed or is edma_stop good enough? And do we
> CCERR.BIT(16)
> need to write this bit or not?
I have no idea. That's why I'm not trying to fix it; it's best left
alone unless you know for certain how to fix it _and_ that you can
fix it _correctly_.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH v5 1/2] iio: adc: add driver for Rockchip saradc
From: Hartmut Knaack @ 2014-07-20 10:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2386463.CW7L8vIufx@diego>
Heiko St?bner schrieb:
> The ADC is a 3-channel signal-ended 10-bit Successive Approximation
> Register (SAR) A/D Converter. It uses the supply and ground as its reference
> and converts the analog input signal into 10-bit binary digital codes.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
Looks good to me. The fact, that SARADC_DLY_PU_SOC_MASK is defined but never used should be fine under the given circumstances, that no datasheet with register definitions is publicly available.
> changes since v4:
> - address comments from Hartmut Knaack
> - explain the DLY_PU_SOC value
> - determine regulator voltage in IIO_CHAN_INFO_SCALE
> - return ENODEV directly in the !np case
> changes since v3:
> - address comments from Jonathan Cameron
> - explicitly check for negative values in error checks
> - change Kconfig depends to ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
> to prevent issues due to undefined writel_relaxed functions
> The previous check for OF was unecessary, as the only used function
> is of_property_read_u32, for which a stub is defined for the !OF case.
> changes since v2:
> - address more comments from Peter Meerwald
> mainly the missing info_mask_shared_by_type element
> changes since v1:
> - address comments from Peter Meerwald
>
> drivers/iio/adc/Kconfig | 10 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/rockchip_saradc.c | 316 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 327 insertions(+)
> create mode 100644 drivers/iio/adc/rockchip_saradc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index a80d236..01c6ed6 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -187,6 +187,16 @@ config NAU7802
> To compile this driver as a module, choose M here: the
> module will be called nau7802.
>
> +config ROCKCHIP_SARADC
> + tristate "Rockchip SARADC driver"
> + depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
> + help
> + Say yes here to build support for the SARADC found in SoCs from
> + Rockchip.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called rockchip_saradc.
> +
> config TI_ADC081C
> tristate "Texas Instruments ADC081C021/027"
> depends on I2C
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 9d60f2d..8e2932d 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_MCP320X) += mcp320x.o
> obj-$(CONFIG_MCP3422) += mcp3422.o
> obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
> obj-$(CONFIG_NAU7802) += nau7802.o
> +obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> new file mode 100644
> index 0000000..9fa5653
> --- /dev/null
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -0,0 +1,316 @@
> +/*
> + * Rockchip Successive Approximation Register (SAR) A/D Converter
> + * Copyright (C) 2014 ROCKCHIP, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/iio/iio.h>
> +
> +#define SARADC_DATA 0x00
> +#define SARADC_DATA_MASK 0x3ff
> +
> +#define SARADC_STAS 0x04
> +#define SARADC_STAS_BUSY BIT(0)
> +
> +#define SARADC_CTRL 0x08
> +#define SARADC_CTRL_IRQ_STATUS BIT(6)
> +#define SARADC_CTRL_IRQ_ENABLE BIT(5)
> +#define SARADC_CTRL_POWER_CTRL BIT(3)
> +#define SARADC_CTRL_CHN_MASK 0x7
> +
> +#define SARADC_DLY_PU_SOC 0x0c
> +#define SARADC_DLY_PU_SOC_MASK 0x3f
> +
> +#define SARADC_BITS 10
> +#define SARADC_TIMEOUT msecs_to_jiffies(100)
> +
> +struct rockchip_saradc {
> + void __iomem *regs;
> + struct clk *pclk;
> + struct clk *clk;
> + struct completion completion;
> + struct regulator *vref;
> + u16 last_val;
> +};
> +
> +static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct rockchip_saradc *info = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + mutex_lock(&indio_dev->mlock);
> +
> + /* 8 clock periods as delay between power up and start cmd */
> + writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> +
> + /* Select the channel to be used and trigger conversion */
> + writel(SARADC_CTRL_POWER_CTRL
> + | (chan->channel & SARADC_CTRL_CHN_MASK)
> + | SARADC_CTRL_IRQ_ENABLE,
> + info->regs + SARADC_CTRL);
> +
> + if (!wait_for_completion_timeout(&info->completion,
> + SARADC_TIMEOUT)) {
> + writel_relaxed(0, info->regs + SARADC_CTRL);
> + mutex_unlock(&indio_dev->mlock);
> + return -ETIMEDOUT;
> + }
> +
> + *val = info->last_val;
> + mutex_unlock(&indio_dev->mlock);
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + ret = regulator_get_voltage(info->vref);
> + if (ret < 0) {
> + dev_err(&indio_dev->dev, "failed to get voltage\n");
> + return ret;
> + }
> +
> + *val = ret / 1000;
> + *val2 = SARADC_BITS;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
> +{
> + struct rockchip_saradc *info = (struct rockchip_saradc *)dev_id;
> +
> + /* Read value */
> + info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> + info->last_val &= SARADC_DATA_MASK;
> +
> + /* Clear irq & power down adc */
> + writel_relaxed(0, info->regs + SARADC_CTRL);
> +
> + complete(&info->completion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct iio_info rockchip_saradc_iio_info = {
> + .read_raw = rockchip_saradc_read_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +#define ADC_CHANNEL(_index, _id) { \
> + .type = IIO_VOLTAGE, \
> + .indexed = 1, \
> + .channel = _index, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> + .datasheet_name = _id, \
> +}
> +
> +static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
> + ADC_CHANNEL(0, "adc0"),
> + ADC_CHANNEL(1, "adc1"),
> + ADC_CHANNEL(2, "adc2"),
> +};
> +
> +static int rockchip_saradc_probe(struct platform_device *pdev)
> +{
> + struct rockchip_saradc *info = NULL;
> + struct device_node *np = pdev->dev.of_node;
> + struct iio_dev *indio_dev = NULL;
> + struct resource *mem;
> + int ret;
> + int irq;
> + u32 rate;
> +
> + if (!np)
> + return -ENODEV;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "failed allocating iio device\n");
> + return -ENOMEM;
> + }
> + info = iio_priv(indio_dev);
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + info->regs = devm_request_and_ioremap(&pdev->dev, mem);
> + if (!info->regs)
> + return -ENOMEM;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "no irq resource?\n");
> + return irq;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq, rockchip_saradc_isr,
> + 0, dev_name(&pdev->dev), info);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed requesting irq %d\n", irq);
> + return ret;
> + }
> +
> + init_completion(&info->completion);
> +
> + info->pclk = devm_clk_get(&pdev->dev, "apb_pclk");
> + if (IS_ERR(info->pclk)) {
> + dev_err(&pdev->dev, "failed to get pclk\n");
> + return PTR_ERR(info->pclk);
> + }
> +
> + info->clk = devm_clk_get(&pdev->dev, "saradc");
> + if (IS_ERR(info->clk)) {
> + dev_err(&pdev->dev, "failed to get adc clock\n");
> + return PTR_ERR(info->clk);
> + }
> +
> + info->vref = devm_regulator_get(&pdev->dev, "vref");
> + if (IS_ERR(info->vref)) {
> + dev_err(&pdev->dev, "failed to get regulator, %ld\n",
> + PTR_ERR(info->vref));
> + return PTR_ERR(info->vref);
> + }
> +
> + /* use a default of 1MHz for the converter clock */
> + ret = of_property_read_u32(np, "clock-frequency", &rate);
> + if (ret < 0)
> + rate = 1000000;
> +
> + ret = clk_set_rate(info->clk, rate);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
> + return ret;
> + }
> +
> + ret = regulator_enable(info->vref);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to enable vref regulator\n");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(info->pclk);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to enable pclk\n");
> + goto err_reg_voltage;
> + }
> +
> + ret = clk_prepare_enable(info->clk);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to enable converter clock\n");
> + goto err_pclk;
> + }
> +
> + platform_set_drvdata(pdev, indio_dev);
> +
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->info = &rockchip_saradc_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + indio_dev->channels = rockchip_saradc_iio_channels;
> + indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto err_clk;
> +
> + return 0;
> +
> +err_clk:
> + clk_disable_unprepare(info->clk);
> +err_pclk:
> + clk_disable_unprepare(info->pclk);
> +err_reg_voltage:
> + regulator_disable(info->vref);
> + return ret;
> +}
> +
> +static int rockchip_saradc_remove(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct rockchip_saradc *info = iio_priv(indio_dev);
> +
> + iio_device_unregister(indio_dev);
> + clk_disable_unprepare(info->clk);
> + clk_disable_unprepare(info->pclk);
> + regulator_disable(info->vref);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rockchip_saradc_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct rockchip_saradc *info = iio_priv(indio_dev);
> +
> + clk_disable_unprepare(info->clk);
> + clk_disable_unprepare(info->pclk);
> + regulator_disable(info->vref);
> +
> + return 0;
> +}
> +
> +static int rockchip_saradc_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct rockchip_saradc *info = iio_priv(indio_dev);
> + int ret;
> +
> + ret = regulator_enable(info->vref);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(info->pclk);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(info->clk);
> + if (ret)
> + return ret;
> +
> + return ret;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
> + rockchip_saradc_suspend, rockchip_saradc_resume);
> +
> +static const struct of_device_id rockchip_saradc_match[] = {
> + { .compatible = "rockchip,saradc" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> +
> +static struct platform_driver rockchip_saradc_driver = {
> + .probe = rockchip_saradc_probe,
> + .remove = rockchip_saradc_remove,
> + .driver = {
> + .name = "rockchip-saradc",
> + .owner = THIS_MODULE,
> + .of_match_table = rockchip_saradc_match,
> + .pm = &rockchip_saradc_pm_ops,
> + },
> +};
> +
> +module_platform_driver(rockchip_saradc_driver);
^ permalink raw reply
* [PATCH resend v2] arm64: dmi: Add SMBIOS/DMI support
From: Ard Biesheuvel @ 2014-07-20 10:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu_2CYU+Th4xZ3EfjCFqQzBfJ88Hrtx1LwUbG79+1=cqJw@mail.gmail.com>
On 14 July 2014 15:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 14 July 2014 15:36, Will Deacon <will.deacon@arm.com> wrote:
>> On Fri, Jul 11, 2014 at 12:46:50PM +0100, Ard Biesheuvel wrote:
>>> From: Yi Li <yi.li@linaro.org>
>>>
>>> SMbios is important for server hardware vendors. It implements a spec for
>>> providing descriptive information about the platform. Things like serial
>>> numbers, physical layout of the ports, build configuration data, and the like.
>>>
>>> This has been tested by dmidecode and lshw tools.
>>>
>>> Signed-off-by: Yi Li <yi.li@linaro.org>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> Changes since previous version:
>>> - changed double inclusion guard to arm64 flavour
>>> - use kzalloc(GFP_KERNEL) as GFP_ATOMIC is not needed
>>> - do a sanity check on the size of the requested mapping
>>
>> Technically, this patch looks fine to me:
>>
>> Reviewed-by: Will Deacon <will.deacon@arm.com>
>>
Hello Catalin,
This has been tested by Yi on FVP and Juno.
Are you ok to take this?
--
Ard.
>
> Thanks.
>
>> It would be good if somebody with a relevant platform could confirm that
>> it works/is useful though.
>>
>
> The usefulness has been debated here a couple of times before:
> Grant's take on it is here
> http://marc.info/?l=linux-arm-kernel&m=140206407411522&w=2
>
> As to whether it works: I have asked Yi to retest this version of the
> patch with lshw and dmidecode
> @Yi: could you please share your findings here?
>
^ permalink raw reply
* [PATCH v5 1/2] iio: adc: add driver for Rockchip saradc
From: Lars-Peter Clausen @ 2014-07-20 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2386463.CW7L8vIufx@diego>
On 07/14/2014 10:43 PM, Heiko St?bner wrote:
Looks really good overall.
[...]
> +static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct rockchip_saradc *info = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + mutex_lock(&indio_dev->mlock);
> +
> + /* 8 clock periods as delay between power up and start cmd */
> + writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> +
It makes sense to call reinit_completion() here.
> + /* Select the channel to be used and trigger conversion */
> + writel(SARADC_CTRL_POWER_CTRL
> + | (chan->channel & SARADC_CTRL_CHN_MASK)
> + | SARADC_CTRL_IRQ_ENABLE,
> + info->regs + SARADC_CTRL);
> +
> + if (!wait_for_completion_timeout(&info->completion,
> + SARADC_TIMEOUT)) {
> + writel_relaxed(0, info->regs + SARADC_CTRL);
> + mutex_unlock(&indio_dev->mlock);
> + return -ETIMEDOUT;
> + }
> +
> + *val = info->last_val;
> + mutex_unlock(&indio_dev->mlock);
> + return IIO_VAL_INT;
[...]
> +static int rockchip_saradc_probe(struct platform_device *pdev)
> +{
> + struct rockchip_saradc *info = NULL;
> + struct device_node *np = pdev->dev.of_node;
> + struct iio_dev *indio_dev = NULL;
> + struct resource *mem;
> + int ret;
> + int irq;
> + u32 rate;
> +
> + if (!np)
> + return -ENODEV;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "failed allocating iio device\n");
> + return -ENOMEM;
> + }
> + info = iio_priv(indio_dev);
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + info->regs = devm_request_and_ioremap(&pdev->dev, mem);
devm_request_and_ioremap() is deprecated an will be removed in the next
release[1]. Use devm_ioremap_resource(), note that devm_ioremap_resource()
returns a ERR_PTR instead of NULL on error.
[1] https://lkml.org/lkml/2014/6/11/26
> + if (!info->regs)
> + return -ENOMEM;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "no irq resource?\n");
> + return irq;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq, rockchip_saradc_isr,
> + 0, dev_name(&pdev->dev), info);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed requesting irq %d\n", irq);
> + return ret;
> + }
> +
> + init_completion(&info->completion);
Since the completion is used in the interrupt callback it must be
initialized before the interrupt is requested.
> +
[...]
> + /* use a default of 1MHz for the converter clock */
> + ret = of_property_read_u32(np, "clock-frequency", &rate);
> + if (ret < 0)
> + rate = 1000000;
Should this really be in the devicetree or shouldn't this be user
configurable at runtime?
[...]
^ permalink raw reply
* [PATCH 1/2] PCI: spear: Fix Section mismatch compilation warning for probe()
From: Viresh Kumar @ 2014-07-20 11:59 UTC (permalink / raw)
To: linux-arm-kernel
Following compilation warning occurs when compiled with:
CONFIG_DEBUG_SECTION_MISMATCH=y
WARNING: drivers/pci/host/built-in.o(.data+0xc0): Section mismatch in
reference from the variable spear13xx_pcie_driver to the function
.init.text:spear13xx_pcie_probe()
Both .probe() and pcie_init() are marked with __init, but spear13xx_pcie_driver
isn't. And so section mismatch.
Fix it by marking spear13xx_pcie_driver with __initdata.
Fixes: 51b66a6 (PCI: spear: Add PCIe driver for ST Microelectronics SPEAr13xx)
Reported-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Olof/Arnd,
Let me know if a PULL request is required for this, otherwise just apply them
directly.
drivers/pci/host/pcie-spear13xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 99738e4..67315ea 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -382,7 +382,7 @@ static const struct of_device_id spear13xx_pcie_of_match[] = {
};
MODULE_DEVICE_TABLE(of, spear13xx_pcie_of_match);
-static struct platform_driver spear13xx_pcie_driver = {
+static struct platform_driver spear13xx_pcie_driver __initdata = {
.probe = spear13xx_pcie_probe,
.remove = spear13xx_pcie_remove,
.driver = {
--
2.0.0.rc2
^ permalink raw reply related
* [PATCH 2/2] PCI: spear: Remove spear13xx_pcie_remove()
From: Viresh Kumar @ 2014-07-20 11:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <76576b662613369d0c71db30ceed4ed5ae1343d2.1405857442.git.viresh.kumar@linaro.org>
Following compilation warning occurs when compiled with:
CONFIG_DEBUG_SECTION_MISMATCH=y
WARNING: vmlinux.o(.init.data+0x3338): Section mismatch in reference from the
variable spear13xx_pcie_driver to the function
.exit.text:spear13xx_pcie_remove()
This driver isn't allowed to unload, and so doesn't have a *_exit() routine. But
it still has spear13xx_pcie_remove() marked with __exit.
As this driver can't unload, .remove() would never be called, right? So get rid
of it.
Fixes: 51b66a6 (PCI: spear: Add PCIe driver for ST Microelectronics SPEAr13xx)
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/pci/host/pcie-spear13xx.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 67315ea..6dea9e4 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -365,17 +365,6 @@ static int __init spear13xx_pcie_probe(struct platform_device *pdev)
return ret;
}
-static int __exit spear13xx_pcie_remove(struct platform_device *pdev)
-{
- struct spear13xx_pcie *spear13xx_pcie = platform_get_drvdata(pdev);
-
- clk_disable_unprepare(spear13xx_pcie->clk);
-
- phy_exit(spear13xx_pcie->phy);
-
- return 0;
-}
-
static const struct of_device_id spear13xx_pcie_of_match[] = {
{ .compatible = "st,spear1340-pcie", },
{},
@@ -384,7 +373,6 @@ MODULE_DEVICE_TABLE(of, spear13xx_pcie_of_match);
static struct platform_driver spear13xx_pcie_driver __initdata = {
.probe = spear13xx_pcie_probe,
- .remove = spear13xx_pcie_remove,
.driver = {
.name = "spear-pcie",
.owner = THIS_MODULE,
--
2.0.0.rc2
^ permalink raw reply related
* [RFC] cpufreq: Add bindings for CPU clock sharing topology
From: Viresh Kumar @ 2014-07-20 12:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53CA8D95.8010108@ti.com>
On 19 July 2014 20:54, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> Sorry for jumping late
No, you aren't late. Its just 2 days old thread :)
> but one of the point I was raising as part of your
> other series was to extend the CPU topology bindings to cover the voltage
> domain information which is probably what is really needed to let the
> CPUfreq extract the information. Not sure if it was already discussed.
Not it wasn't.
> After all the CPU clocks, cluster, clock-gating, power domains are pretty much
> related. So instead of having new binding for CPUFreq, I was wondering whether
> we can extend the CPU topology binding information to include missing information.
> Scheduler work anyway needs that information.
>
> Ref: Documentation/devicetree/bindings/arm/topology.txt
>
> Does that make sense ?
Yeah it does, but I am not sure what exactly the bindings should look then.
So, the most basic step could be moving the new bindings to topology.txt
and name clock-master to dvfs-master.
What else?
If its going to be much controversial then we *can* go for just dvfs bindings
for now and then update them later.
Doesn't make sense? :)
^ permalink raw reply
* [GIT PULL] Renesas ARM Based SoC Boards Cleanups Updates for v3.17
From: Simon Horman @ 2014-07-20 13:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140719185548.GC4265@quad.lixom.net>
On Sat, Jul 19, 2014 at 11:55:48AM -0700, Olof Johansson wrote:
> On Fri, Jul 18, 2014 at 08:59:25AM +0900, Simon Horman wrote:
> > Hi Olof, Hi Kevin, Hi Arnd,
> >
> > Please consider these Renesas ARM based SoC Boards Cleanups updates for v3.17.
> >
> > This pull request is based on a merge of:
> >
> > * "Fourth Round of Renesas ARM Based SoC Defconfig Updates for v3.17",
> > tagged as renesas-defconfig4-for-v3.17,
> > which I have sent a pull request for.
> >
> > The reason for this dependency is to avoid attempts to use
> > genmai_defconfig after the board code it relies on has been removed.
>
> I'd prefer not to add this dependency. It's not a big deal to have a defconfig
> that won't build momentarily, it doesn't cause any real harm and I'd rather
> avoid keeping this merge/dependency. Can you respin this?
Thanks for the clarification, I'll respin this as you suggest.
> > * "Renesas ARM Based SoC DT Timers Updates for v3.17",
> > tagged as renesas-dt-timers-for-v3.17,
> > which I have sent a pull request for.
>
> Hmm. You're doing cleanups on top of new development instead of the other way
> around. Try to avoid that in future releases if you can.
This relates to the order that the code was developed,
although I agree it is not ideal with the benifit of 20/20 hind-sight.
^ permalink raw reply
* [PATCH 2/2] iio: exynos-adc: add experimental touchscreen support
From: Jonathan Cameron @ 2014-07-20 13:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5456679.XxlLIvc3Q6@wuerfel>
On 18/07/14 20:29, Arnd Bergmann wrote:
> This adds support for the touchscreen on Samsung s3c64xx.
> The driver is completely untested but shows roughly how
> it could be done, following the example of the at91 driver.
>
Hi Arnd,
> Open questions include:
>
> - compared to the old plat-samsung/adc driver, there is
> no support for prioritizing ts over other clients, nor
> for oversampling. From my reading of the code, the
> priorities didn't actually have any effect at all, but
> the oversampling might be needed. Maybe the original
> authors have some insight.
>
> - I simply register the input device from the adc driver
> itself, as the at91 code does. The driver also supports
> sub-nodes, but I don't understand how they are meant
> to be used, so using those might be better.
So, the alternative you are (I think) referring to is to use
the buffered in kernel client interface. That way a separate
touch screen driver can use the output channels provided by IIO
in much the same way you might use a regulator or similar.
Note that whilst this is similar to the simple polled interface
used for things like the iio_hwmon driver, the data flow is
quite different (clearly the polled interfce would be
inappropriate here).
Whilst we've discussed it in the past for touch screen drivers
like this, usually the hardware isn't generic enough to be
of any real use if not being used as a touch screen. As such
it's often simpler to just have the support directly in the
driver (as you've observed the at91 driver does this).
Whilst the interface has been there a while, it's not really had
all that much use. The original target was the simpler case
of 3D accelerometer where we have a generic iio to input
bridge driver. Time constraints meant that I haven't yet actually
formally submitted the input side of this. Whilst there are lots
of other things that can use this interface, right now nothing
actually does so.
>
> - The new exynos_read_s3c64xx_ts() function is intentionally
> very similar to the existing exynos_read_raw() functions.
> It should probably be changed, either by merging the two
> into one, or by simplifying the exynos_read_s3c64xx_ts()
> function. This depends a bit on the answers to the questions
> above.
I'd be tempted to not bother keeping them that similar. It's
not a generic IIO channel so simplify it where possible.
>
> - We probably need to add support for platform_data as well,
> I've skipped this so far.
>
> - Is anybody able to debug this driver on real hardware?
> While it's possible that it actually works, it's more
> likely that I made a few subtle mistakes.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Looks pretty good to me. A few symantic bits and pieces and
one bug spotted. Short and sweet.
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> index e1b74828f413..4329bf3c3326 100644
> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> @@ -41,6 +41,10 @@ Required properties:
> and compatible ADC block)
> - vdd-supply VDD input supply.
>
> +Optional properties:
> +- has-touchscreen: If present, indicates that a touchscreen is
> + connected an usable.
> +
> Note: child nodes can be added for auto probing from device tree.
>
> Example: adding device info in dtsi file
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index 5f95638513d2..cf1d9f3e2492 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -34,6 +34,7 @@
> #include <linux/regulator/consumer.h>
> #include <linux/of_platform.h>
> #include <linux/err.h>
> +#include <linux/input.h>
Might want to make the input side optional at compile time...
I supose the existing parts are unlikely to be used much in headless
devices, but you never know. Maybe we just leave this until someone
shouts they want to be able to avoid compiling it in.
>
> #include <linux/iio/iio.h>
> #include <linux/iio/machine.h>
> @@ -103,6 +104,7 @@
>
> /* Bit definitions common for ADC_V1 and ADC_V2 */
> #define ADC_CON_EN_START (1u << 0)
> +#define ADC_DATX_PRESSED (1u << 15)
> #define ADC_DATX_MASK 0xFFF
>
> #define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
> @@ -110,16 +112,20 @@
> struct exynos_adc {
> struct exynos_adc_data *data;
> struct device *dev;
> + struct input_dev *input;
> void __iomem *regs;
> void __iomem *enable_reg;
> struct clk *clk;
> struct clk *sclk;
> unsigned int irq;
> + unsigned int tsirq;
> struct regulator *vdd;
>
> struct completion completion;
>
> + bool read_ts;
> u32 value;
> + u32 value2;
As I state further down, I'd rather keep a little
clear of the naming used in IIO for bits that aren't
going through IIO (less confusing!). Maybe just
have
u32 x, y;
> unsigned int version;
> };
>
> @@ -390,12 +396,61 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> return ret;
> }
>
> +static int exynos_read_s3c64xx_ts(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val,
> + int *val2,
> + long mask)
> +{
> + struct exynos_adc *info = iio_priv(indio_dev);
> + unsigned long timeout;
> + int ret;
> +
> + if (mask != IIO_CHAN_INFO_RAW)
> + return -EINVAL;
> +
> + mutex_lock(&indio_dev->mlock);
> + info->read_ts = 1;
> +
> + reinit_completion(&info->completion);
> +
> + writel(ADC_S3C2410_TSC_PULL_UP_DISABLE | ADC_TSC_AUTOPST,
> + ADC_V1_TSC(info->regs));
> +
> + /* Select the ts channel to be used and Trigger conversion */
> + info->data->start_conv(info, 0);
0 is a rather magic value. A define perhaps?
> +
> + timeout = wait_for_completion_timeout
> + (&info->completion, EXYNOS_ADC_TIMEOUT);
> + if (timeout == 0) {
> + dev_warn(&indio_dev->dev, "Conversion timed out! Resetting\n");
> + if (info->data->init_hw)
> + info->data->init_hw(info);
> + ret = -ETIMEDOUT;
> + } else {
> + *val = info->value;
> + *val2 = info->value2;
This is definitely abuse as those two values are not intended for
different values. If you want to do this please use different naming
and don't try to fiddle it into the IIO read raw framework.
As you've suggested above, better to simplify this code and drop the
bits cloned from the other handler.
> + ret = IIO_VAL_INT;
> + }
> +
> + info->read_ts = 0;
> + mutex_unlock(&indio_dev->mlock);
> +
> + return ret;
> +}
> +
> static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
> {
> struct exynos_adc *info = (struct exynos_adc *)dev_id;
>
> /* Read value */
> - info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> + if (info->read_ts) {
> + info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> + info->value2 = readl(ADC_V1_DATY(info->regs)) & ADC_DATX_MASK;
ADC_DATY_MASK would be more obviously correct.
> + writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
Perhaps the above is cryptic enough to warrant a comment?
> + } else {
> + info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> + }
>
> /* clear irq */
> if (info->data->clear_irq)
> @@ -406,6 +461,46 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +/*
> + * Here we (ab)use a threaded interrupt handler to stay running
> + * for as long as the touchscreen remains pressed, we report
> + * a new event with the latest data and then sleep until the
> + * next timer tick. This mirrors the behavior of the old
> + * driver, with much less code.
> + */
> +static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
> +{
> + struct exynos_adc *info = dev_id;
> + struct iio_dev *dev = dev_get_drvdata(info->dev);
> + u32 x, y;
> + bool pressed;
> + int ret;
> +
> + do {
> + ret =exynos_read_s3c64xx_ts(dev, NULL, &x, &y, IIO_CHAN_INFO_RAW);
= exynos
> + if (ret == -ETIMEDOUT)
> + break;
> +
> + pressed = x & y & ADC_DATX_PRESSED;
> + if (!pressed)
> + break;
> +
> + input_report_abs(info->input, ABS_X, x & ADC_DATX_MASK);
> + input_report_abs(info->input, ABS_Y, y & ADC_DATX_MASK);
> + input_report_key(info->input, BTN_TOUCH, 1);
> + input_sync(info->input);
> +
> + msleep(1);
> + } while (1);
> +
> + input_report_key(info->input, BTN_TOUCH, 0);
> + input_sync(info->input);
> +
> + writel(0, ADC_V1_CLRINTPNDNUP(info->regs));
> +
> + return IRQ_HANDLED;
> +}
> +
> static int exynos_adc_reg_access(struct iio_dev *indio_dev,
> unsigned reg, unsigned writeval,
> unsigned *readval)
> @@ -457,12 +552,57 @@ static int exynos_adc_remove_devices(struct device *dev, void *c)
> return 0;
> }
>
> +static int exynos_adc_ts_init(struct exynos_adc *info)
> +{
> + int ret;
> +
> + info->input = input_allocate_device();
> + if (!info->input)
> + return -ENOMEM;
> +
> + info->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> + info->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> +
> + input_set_abs_params(info->input, ABS_X, 0, 0x3FF, 0, 0);
> + input_set_abs_params(info->input, ABS_Y, 0, 0x3FF, 0, 0);
> +
> + /* data from s3c2410_ts driver */
> + info->input->name = "S3C24xx TouchScreen";
> + info->input->id.bustype = BUS_HOST;
> + info->input->id.vendor = 0xDEAD;
> + info->input->id.product = 0xBEEF;
> + info->input->id.version = 0x0200;
> +
> + ret = input_register_device(info->input);
> + if (ret) {
> + input_free_device(info->input);
> + goto err;
> + }
> +
> + if (info->tsirq > 0)
> + ret = request_threaded_irq(info->irq, NULL, exynos_ts_isr,
> + 0, "touchscreen", info);
info->tsirq
(that had me really confused for a moment ;)
Also, perhaps a more specific name. touchscreen_updown or similar as the
main interrupt is also used during touchscreen operation.
> + if (ret < 0) {
> + dev_err(info->dev, "failed requesting touchsccreen irq, irq = %d\n",
> + info->irq);
> + goto err_input;
> + }
> +
> + return 0;
> +
> +err_input:
> + input_unregister_device(info->input);
> +err:
> + return ret;
> +}
> +
> static int exynos_adc_probe(struct platform_device *pdev)
> {
> struct exynos_adc *info = NULL;
> struct device_node *np = pdev->dev.of_node;
> struct iio_dev *indio_dev = NULL;
> struct resource *mem;
> + bool has_ts;
> int ret = -ENODEV;
> int irq;
>
> @@ -498,8 +638,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "no irq resource?\n");
> return irq;
> }
> -
> info->irq = irq;
> +
> + irq = platform_get_irq(pdev, 1);
> + if (irq == -EPROBE_DEFER)
> + return irq;
> +
> + info->tsirq = irq;
> +
> info->dev = &pdev->dev;
>
> init_completion(&info->completion);
> @@ -565,6 +711,12 @@ static int exynos_adc_probe(struct platform_device *pdev)
> if (info->data->init_hw)
> info->data->init_hw(info);
>
> + has_ts = of_property_read_bool(pdev->dev.of_node, "has-touchscreen");
> + if (has_ts)
> + ret = exynos_adc_ts_init(info);
> + if (ret)
> + goto err_iio;
> +
> ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
> if (ret < 0) {
> dev_err(&pdev->dev, "failed adding child nodes\n");
> @@ -576,6 +728,11 @@ static int exynos_adc_probe(struct platform_device *pdev)
> err_of_populate:
> device_for_each_child(&indio_dev->dev, NULL,
> exynos_adc_remove_devices);
> + if (has_ts) {
> + input_unregister_device(info->input);
> + free_irq(info->tsirq, info);
> + }
> +err_iio:
> iio_device_unregister(indio_dev);
> err_irq:
> free_irq(info->irq, info);
> @@ -595,9 +752,12 @@ static int exynos_adc_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct exynos_adc *info = iio_priv(indio_dev);
>
> + input_free_device(info->input);
> device_for_each_child(&indio_dev->dev, NULL,
> exynos_adc_remove_devices);
> iio_device_unregister(indio_dev);
> + if (info->tsirq > 0)
> + free_irq(info->tsirq, info);
> free_irq(info->irq, info);
> if (info->data->exit_hw)
> info->data->exit_hw(info);
>
^ permalink raw reply
* [GIT PULL] Renesas ARM Based SoC DT Timers Updates for v3.17
From: Simon Horman @ 2014-07-20 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140719052758.GE20064@quad.lixom.net>
On Fri, Jul 18, 2014 at 10:27:58PM -0700, Olof Johansson wrote:
> On Thu, Jul 17, 2014 at 09:40:20AM +0900, Simon Horman wrote:
> > Hi Olof, Hi Kevin, Hi Arnd,
> >
> > Please consider these Renesas ARM based SoC DT Timers updates for v3.17.
> >
> > This pull request is based on a merge of the following to provide
> > all dependencies and try to eliminate conflicts. It turns out the changes
> > in this pull requests are a nexus for dependencies due to modifying DT,
> > SoC, board and recently moved header files as well as requiring driver
> > changes.
> >
> > * The clockevents/renesas-timers-dt branch of Daniel Lezcano's tree.
> > He has indicated that this branch has stable commit ids and will
> > be included in v3.17. Olof and arm at kernel.org were CCed on the
> > thread where he, Laurent Pinchart and I discussed the use of that branch.
> >
> > The clockevents/renesas-timers-dt's branch is in turn based on v3.16-rc3.
> >
> > * "Third Round of Renesas ARM Based SoC DT Updates for v3.17",
> > tagged as renesas-dt3-for-v3.17, which I have sent a pull request for.
> >
> > * "Renesas ARM Based SoC Clock Updates for v3.17",
> > tagged as renesas-clock-for-v3.17, which you have merged
> > into next/soc
> >
> > * "Second Round of Renesas ARM Based SoC soc-cleanup Updates for v3.17",
> > tagged as renesas-soc-cleanup2-for-v3.17, which you have merged
> > into next/cleanup.
> >
> > * "Third Round of Renesas ARM Based SoC r8a7779 Multiplatform Updates for
> > v3.17", tagged as renesas-r8a7779-multiplatform3-for-v3.17, which
> > you have merged into next/soc
> >
> > * "Renesas ARM Based SoC Boards Updates for v3.17",
> > tagged as renesas-boards-for-v3.17, which you have merged
> > into next/boards
> >
> > * "Third Round of Renesas ARM Based SoC Updates for v3.17",
> > tagged as renesas-soc3-for-v3.17, which you have merged
> > into next/soc
> >
> >
> > The following changes since commit 5c174afd407acc7a90701900b279578151bc007f:
> >
> > Merge branch 'clockevents/renesas-timers-dt' of git://git.linaro.org/people/daniel.lezcano/linux into dt-timers-for-v3.17.base (2014-07-15 16:31:45 +0900)
> >
> > are available in the git repository at:
> >
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-dt-timers-for-v3.17
> >
> > for you to fetch changes up to 9394af4314554d15762585a3464cefaa2e6d0420:
> >
> > ARM: shmobile: genmai-reference: Enable MTU2 in device tree (2014-07-15 21:26:42 +0900)
> >
> > ----------------------------------------------------------------
> > Renesas ARM Based SoC DT Timers Updates for v3.17
> >
> > * Enable timers using DT when booting boards without Legacy-C code
> >
> > ----------------------------------------------------------------
> > Laurent Pinchart (8):
> > ARM: shmobile: r8a7790: Add CMT devices to DT
> > ARM: shmobile: r8a7791: Add CMT devices to DT
> > ARM: shmobile: r8a7779: Add TMU devices to DT
> > ARM: shmobile: lager-reference: Enable CMT0 in device tree
> > ARM: shmobile: koelsch-reference: Enable CMT0 in device tree
> > ARM: shmobile: marzen-reference: Enable TMU0 in device tree
> > ARM: shmobile: r7s72100: Add MTU2 device to DT
> > ARM: shmobile: genmai-reference: Enable MTU2 in device tree
>
> Ok, this branch definitely contains a lot more than this. For dependent
> external branches such as clocksource, we still prefer to see a pull request so
> that we can merge in the dependency and get a clean diffstat when we do the
> merge of your branch, otherwise it gets awkward to compare that what we're
> getting is what you thought you sent (which is one of the things we check on
> merges).
>
> Please regenerate this pull request as appropriate.
Hi Olof,
FWIW, I believe that that the diffstat between
5c174afd407acc7a90701900b279578151bc007f and
9394af4314554d15762585a3464cefaa2e6d0420 is what was included in the
pull-request. But I guess that all the merged-in branches are hampering
your verification process.
Would it help if things were arranged as follows?
1. Use the clocksource branch as a base and then;
2. Merge in each of my branches (the ones listed above) and then;
3. Add the patches on top
^ permalink raw reply
* [PATCH 2/2] iio: exynos-adc: add experimental touchscreen support
From: Jonathan Cameron @ 2014-07-20 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53CBC8D6.6020509@kernel.org>
On 20/07/14 14:49, Jonathan Cameron wrote:
> On 18/07/14 20:29, Arnd Bergmann wrote:
>> This adds support for the touchscreen on Samsung s3c64xx.
>> The driver is completely untested but shows roughly how
>> it could be done, following the example of the at91 driver.
>>
> Hi Arnd,
Cc'd linux-input and Dmitry.
>
>> Open questions include:
>>
>> - compared to the old plat-samsung/adc driver, there is
>> no support for prioritizing ts over other clients, nor
>> for oversampling. From my reading of the code, the
>> priorities didn't actually have any effect at all, but
>> the oversampling might be needed. Maybe the original
>> authors have some insight.
>>
>> - I simply register the input device from the adc driver
>> itself, as the at91 code does. The driver also supports
>> sub-nodes, but I don't understand how they are meant
>> to be used, so using those might be better.
> So, the alternative you are (I think) referring to is to use
> the buffered in kernel client interface. That way a separate
> touch screen driver can use the output channels provided by IIO
> in much the same way you might use a regulator or similar.
> Note that whilst this is similar to the simple polled interface
> used for things like the iio_hwmon driver, the data flow is
> quite different (clearly the polled interfce would be
> inappropriate here).
>
> Whilst we've discussed it in the past for touch screen drivers
> like this, usually the hardware isn't generic enough to be
> of any real use if not being used as a touch screen. As such
> it's often simpler to just have the support directly in the
> driver (as you've observed the at91 driver does this).
>
> Whilst the interface has been there a while, it's not really had
> all that much use. The original target was the simpler case
> of 3D accelerometer where we have a generic iio to input
> bridge driver. Time constraints meant that I haven't yet actually
> formally submitted the input side of this. Whilst there are lots
> of other things that can use this interface, right now nothing
> actually does so.
>
>>
>> - The new exynos_read_s3c64xx_ts() function is intentionally
>> very similar to the existing exynos_read_raw() functions.
>> It should probably be changed, either by merging the two
>> into one, or by simplifying the exynos_read_s3c64xx_ts()
>> function. This depends a bit on the answers to the questions
>> above.
> I'd be tempted to not bother keeping them that similar. It's
> not a generic IIO channel so simplify it where possible.
>>
>> - We probably need to add support for platform_data as well,
>> I've skipped this so far.
>>
>> - Is anybody able to debug this driver on real hardware?
>> While it's possible that it actually works, it's more
>> likely that I made a few subtle mistakes.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Looks pretty good to me. A few symantic bits and pieces and
> one bug spotted. Short and sweet.
>>
>> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> index e1b74828f413..4329bf3c3326 100644
>> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> @@ -41,6 +41,10 @@ Required properties:
>> and compatible ADC block)
>> - vdd-supply VDD input supply.
>>
>> +Optional properties:
>> +- has-touchscreen: If present, indicates that a touchscreen is
>> + connected an usable.
>> +
>> Note: child nodes can be added for auto probing from device tree.
>>
>> Example: adding device info in dtsi file
>> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
>> index 5f95638513d2..cf1d9f3e2492 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -34,6 +34,7 @@
>> #include <linux/regulator/consumer.h>
>> #include <linux/of_platform.h>
>> #include <linux/err.h>
>> +#include <linux/input.h>
> Might want to make the input side optional at compile time...
> I supose the existing parts are unlikely to be used much in headless
> devices, but you never know. Maybe we just leave this until someone
> shouts they want to be able to avoid compiling it in.
>>
>> #include <linux/iio/iio.h>
>> #include <linux/iio/machine.h>
>> @@ -103,6 +104,7 @@
>>
>> /* Bit definitions common for ADC_V1 and ADC_V2 */
>> #define ADC_CON_EN_START (1u << 0)
>> +#define ADC_DATX_PRESSED (1u << 15)
>> #define ADC_DATX_MASK 0xFFF
>>
>> #define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
>> @@ -110,16 +112,20 @@
>> struct exynos_adc {
>> struct exynos_adc_data *data;
>> struct device *dev;
>> + struct input_dev *input;
>> void __iomem *regs;
>> void __iomem *enable_reg;
>> struct clk *clk;
>> struct clk *sclk;
>> unsigned int irq;
>> + unsigned int tsirq;
>> struct regulator *vdd;
>>
>> struct completion completion;
>>
>> + bool read_ts;
>> u32 value;
>> + u32 value2;
> As I state further down, I'd rather keep a little
> clear of the naming used in IIO for bits that aren't
> going through IIO (less confusing!). Maybe just
> have
> u32 x, y;
>> unsigned int version;
>> };
>>
>> @@ -390,12 +396,61 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
>> return ret;
>> }
>>
>> +static int exynos_read_s3c64xx_ts(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int *val,
>> + int *val2,
>> + long mask)
>> +{
>> + struct exynos_adc *info = iio_priv(indio_dev);
>> + unsigned long timeout;
>> + int ret;
>> +
>> + if (mask != IIO_CHAN_INFO_RAW)
>> + return -EINVAL;
>> +
>> + mutex_lock(&indio_dev->mlock);
>> + info->read_ts = 1;
>> +
>> + reinit_completion(&info->completion);
>> +
>> + writel(ADC_S3C2410_TSC_PULL_UP_DISABLE | ADC_TSC_AUTOPST,
>> + ADC_V1_TSC(info->regs));
>> +
>> + /* Select the ts channel to be used and Trigger conversion */
>> + info->data->start_conv(info, 0);
> 0 is a rather magic value. A define perhaps?
>> +
>> + timeout = wait_for_completion_timeout
>> + (&info->completion, EXYNOS_ADC_TIMEOUT);
>> + if (timeout == 0) {
>> + dev_warn(&indio_dev->dev, "Conversion timed out! Resetting\n");
>> + if (info->data->init_hw)
>> + info->data->init_hw(info);
>> + ret = -ETIMEDOUT;
>> + } else {
>> + *val = info->value;
>> + *val2 = info->value2;
> This is definitely abuse as those two values are not intended for
> different values. If you want to do this please use different naming
> and don't try to fiddle it into the IIO read raw framework.
> As you've suggested above, better to simplify this code and drop the
> bits cloned from the other handler.
>> + ret = IIO_VAL_INT;
>> + }
>> +
>> + info->read_ts = 0;
>> + mutex_unlock(&indio_dev->mlock);
>> +
>> + return ret;
>> +}
>> +
>> static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>> {
>> struct exynos_adc *info = (struct exynos_adc *)dev_id;
>>
>> /* Read value */
>> - info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
>> + if (info->read_ts) {
>> + info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
>> + info->value2 = readl(ADC_V1_DATY(info->regs)) & ADC_DATX_MASK;
> ADC_DATY_MASK would be more obviously correct.
>
>> + writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
> Perhaps the above is cryptic enough to warrant a comment?
>> + } else {
>> + info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
>> + }
>>
>> /* clear irq */
>> if (info->data->clear_irq)
>> @@ -406,6 +461,46 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> +/*
>> + * Here we (ab)use a threaded interrupt handler to stay running
>> + * for as long as the touchscreen remains pressed, we report
>> + * a new event with the latest data and then sleep until the
>> + * next timer tick. This mirrors the behavior of the old
>> + * driver, with much less code.
>> + */
>> +static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
>> +{
>> + struct exynos_adc *info = dev_id;
>> + struct iio_dev *dev = dev_get_drvdata(info->dev);
>> + u32 x, y;
>> + bool pressed;
>> + int ret;
>> +
>> + do {
>> + ret =exynos_read_s3c64xx_ts(dev, NULL, &x, &y, IIO_CHAN_INFO_RAW);
> = exynos
>> + if (ret == -ETIMEDOUT)
>> + break;
>> +
>> + pressed = x & y & ADC_DATX_PRESSED;
>> + if (!pressed)
>> + break;
>> +
>> + input_report_abs(info->input, ABS_X, x & ADC_DATX_MASK);
>> + input_report_abs(info->input, ABS_Y, y & ADC_DATX_MASK);
>> + input_report_key(info->input, BTN_TOUCH, 1);
>> + input_sync(info->input);
>> +
>> + msleep(1);
>> + } while (1);
>> +
>> + input_report_key(info->input, BTN_TOUCH, 0);
>> + input_sync(info->input);
>> +
>> + writel(0, ADC_V1_CLRINTPNDNUP(info->regs));
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> static int exynos_adc_reg_access(struct iio_dev *indio_dev,
>> unsigned reg, unsigned writeval,
>> unsigned *readval)
>> @@ -457,12 +552,57 @@ static int exynos_adc_remove_devices(struct device *dev, void *c)
>> return 0;
>> }
>>
>> +static int exynos_adc_ts_init(struct exynos_adc *info)
>> +{
>> + int ret;
>> +
>> + info->input = input_allocate_device();
>> + if (!info->input)
>> + return -ENOMEM;
>> +
>> + info->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
>> + info->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
>> +
>> + input_set_abs_params(info->input, ABS_X, 0, 0x3FF, 0, 0);
>> + input_set_abs_params(info->input, ABS_Y, 0, 0x3FF, 0, 0);
>> +
>> + /* data from s3c2410_ts driver */
>> + info->input->name = "S3C24xx TouchScreen";
>> + info->input->id.bustype = BUS_HOST;
>> + info->input->id.vendor = 0xDEAD;
>> + info->input->id.product = 0xBEEF;
>> + info->input->id.version = 0x0200;
>> +
>> + ret = input_register_device(info->input);
>> + if (ret) {
>> + input_free_device(info->input);
>> + goto err;
>> + }
>> +
>> + if (info->tsirq > 0)
>> + ret = request_threaded_irq(info->irq, NULL, exynos_ts_isr,
>> + 0, "touchscreen", info);
> info->tsirq
> (that had me really confused for a moment ;)
> Also, perhaps a more specific name. touchscreen_updown or similar as the
> main interrupt is also used during touchscreen operation.
>> + if (ret < 0) {
>> + dev_err(info->dev, "failed requesting touchsccreen irq, irq = %d\n",
>> + info->irq);
>> + goto err_input;
>> + }
>> +
>> + return 0;
>> +
>> +err_input:
>> + input_unregister_device(info->input);
>> +err:
>> + return ret;
>> +}
>> +
>> static int exynos_adc_probe(struct platform_device *pdev)
>> {
>> struct exynos_adc *info = NULL;
>> struct device_node *np = pdev->dev.of_node;
>> struct iio_dev *indio_dev = NULL;
>> struct resource *mem;
>> + bool has_ts;
>> int ret = -ENODEV;
>> int irq;
>>
>> @@ -498,8 +638,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
>> dev_err(&pdev->dev, "no irq resource?\n");
>> return irq;
>> }
>> -
>> info->irq = irq;
>> +
>> + irq = platform_get_irq(pdev, 1);
>> + if (irq == -EPROBE_DEFER)
>> + return irq;
>> +
>> + info->tsirq = irq;
>> +
>> info->dev = &pdev->dev;
>>
>> init_completion(&info->completion);
>> @@ -565,6 +711,12 @@ static int exynos_adc_probe(struct platform_device *pdev)
>> if (info->data->init_hw)
>> info->data->init_hw(info);
>>
>> + has_ts = of_property_read_bool(pdev->dev.of_node, "has-touchscreen");
>> + if (has_ts)
>> + ret = exynos_adc_ts_init(info);
>> + if (ret)
>> + goto err_iio;
>> +
>> ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
>> if (ret < 0) {
>> dev_err(&pdev->dev, "failed adding child nodes\n");
>> @@ -576,6 +728,11 @@ static int exynos_adc_probe(struct platform_device *pdev)
>> err_of_populate:
>> device_for_each_child(&indio_dev->dev, NULL,
>> exynos_adc_remove_devices);
>> + if (has_ts) {
>> + input_unregister_device(info->input);
>> + free_irq(info->tsirq, info);
>> + }
>> +err_iio:
>> iio_device_unregister(indio_dev);
>> err_irq:
>> free_irq(info->irq, info);
>> @@ -595,9 +752,12 @@ static int exynos_adc_remove(struct platform_device *pdev)
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct exynos_adc *info = iio_priv(indio_dev);
>>
>> + input_free_device(info->input);
>> device_for_each_child(&indio_dev->dev, NULL,
>> exynos_adc_remove_devices);
>> iio_device_unregister(indio_dev);
>> + if (info->tsirq > 0)
>> + free_irq(info->tsirq, info);
>> free_irq(info->irq, info);
>> if (info->data->exit_hw)
>> info->data->exit_hw(info);
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH -next] spi/rockchip: remove duplicated include from spi-rockchip.c
From: weiyj_lk at 163.com @ 2014-07-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Remove duplicated include.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/spi/spi-rockchip.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 72fb287..d697381 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -26,7 +26,6 @@
#include <linux/of.h>
#include <linux/pm_runtime.h>
#include <linux/io.h>
-#include <linux/scatterlist.h>
#include <linux/dmaengine.h>
#define DRIVER_NAME "rockchip-spi"
^ permalink raw reply related
* [PATCH -next] spi/rockchip: remove redundant dev_err call in rockchip_spi_probe()
From: weiyj_lk at 163.com @ 2014-07-20 14:01 UTC (permalink / raw)
To: linux-arm-kernel
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
There is a error message within devm_ioremap_resource
already, so remove the dev_err call to avoid redundant
error message.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/spi/spi-rockchip.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index 72fb287..171353b 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -585,7 +585,6 @@ static int rockchip_spi_probe(struct platform_device *pdev)
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
rs->regs = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(rs->regs)) {
- dev_err(&pdev->dev, "Failed to map SPI region\n");
ret = PTR_ERR(rs->regs);
goto err_ioremap_resource;
}
^ permalink raw reply related
* [PATCH -next] spi/rockchip: fix error return code in rockchip_spi_probe()
From: weiyj_lk at 163.com @ 2014-07-20 14:02 UTC (permalink / raw)
To: linux-arm-kernel
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Fix to return -EINVAL from the error handling case instead of 0 when
failed to get fifo length.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
drivers/spi/spi-rockchip.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c
index cb8fd6f..8392ff5 100644
--- a/drivers/spi/spi-rockchip.c
+++ b/drivers/spi/spi-rockchip.c
@@ -642,6 +642,7 @@ static int rockchip_spi_probe(struct platform_device *pdev)
rs->fifo_len = get_fifo_len(rs);
if (!rs->fifo_len) {
dev_err(&pdev->dev, "Failed to get fifo length\n");
+ ret = -EINVAL;
goto err_get_fifo_len;
}
^ permalink raw reply related
* [PATCH v3+1 5/5] ARM: DT: STi: STiH416: Add DT node for MiPHY365x
From: Sergei Shtylyov @ 2014-07-20 17:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140714075835.GJ2954@lee--X1>
Hello.
On 07/14/2014 11:58 AM, Lee Jones wrote:
>>> The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
>>> devices. It has 2 ports which it can use for either; both SATA, both
>>> PCIe or one of each in any configuration.
>>> Acked-by: Mark Rutland <mark.rutland@arm.com>
>>> Acked-by: Alexandre Torgue <alexandre.torgue@st.com>
>>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>>> diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts
>>> index 4e2df66..c3c2ac6 100644
>>> --- a/arch/arm/boot/dts/stih416-b2020.dts
>>> +++ b/arch/arm/boot/dts/stih416-b2020.dts
>>> @@ -12,4 +12,16 @@
>>> / {
>>> model = "STiH416 B2020";
>>> compatible = "st,stih416-b2020", "st,stih416";
>>> +
>>> + soc {
>>> + miphy365x_phy: miphy365x at fe382000 {
>>> + phy_port0: port at fe382000 {
>> I don't understand why are you creating the duplicate labels;
>> doesn't 'dtc' complain about them?
> I've never seen dtc complain about this:
> DTC arch/arm/boot/dts/dra72-evm.dtb
> DTC arch/arm/boot/dts/stih407-b2120.dtb
> DTC arch/arm/boot/dts/stih415-b2000.dtb
> DTC arch/arm/boot/dts/stih415-b2020.dtb
> DTC arch/arm/boot/dts/stih416-b2000.dtb
> DTC arch/arm/boot/dts/stih416-b2020.dtb
> DTC arch/arm/boot/dts/stih416-b2020e.dtb
> DTC arch/arm/boot/dts/armada-375-db.dtb
> Probably because they're not actually 'duplicate' per say. Rather
> they are the same node split into different files. I can remove the
> labels if required though.
Yeah, I don't see why you need them if you don't refer to them anywhere.
>> You could instead refer to them
>> as:
>> &miphy365x_phy {
>> };
> I dislike this formatting. I find it convolutes the hierarchical
> structure and makes DTS (and some DTSI) files hard to read i.e hides
> parenthood etc.
Good point...
> [...]
>>> + miphy365x_phy: miphy365x at fe382000 {
>> The ePAPR standard [1] says:
>> The name of a node should be somewhat generic, reflecting the
>> function of the device and not its precise programming model.
> Good point. Will change to 'phy'.
>>> + compatible = "st,miphy365x-phy";
>>> + st,syscfg = <&syscfg_rear>;
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> + ranges;
>>> +
>>> + phy_port0: port at fe382000 {
>>> + #phy-cells = <1>;
>> If these are PHY devices, they should be named "phy", not "port".
> Then what do you call the parent node?
Oh, don't ask me, it wasn't my idea to have PHY subnodes. :-)
WBR, Sergei
^ permalink raw reply
* [GIT PULL RESEND] ARM: mvebu: DT changes for v3.17
From: Olof Johansson @ 2014-07-20 19:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140718222422.GO24496@titan.lakedaemon.net>
On Fri, Jul 18, 2014 at 06:24:22PM -0400, Jason Cooper wrote:
> All,
>
> After discussion with Arnd, this is a resend of my previous pull
> request.
>
> Please pull.
>
> thx,
>
> Jason.
>
>
> The following changes since commit 7171511eaec5bf23fb06078f59784a3a0626b38f:
>
> Linux 3.16-rc1 (2014-06-15 17:45:28 -1000)
>
> are available in the git repository at:
>
> git://git.infradead.org/linux-mvebu.git tags/mvebu-dt-3.17
>
> for you to fetch changes up to d854fa8a1500bec982ed9cb26b82d96bd5ae8dab:
>
> ARM: kirkwood: fix net5big regulator gpio assignments (2014-06-21 19:21:13 +0000)
Merged, thanks.
-Olof
^ permalink raw reply
* [GIT PULL] ARM: mvebu: DT changes for v3.17 (round 2)
From: Olof Johansson @ 2014-07-20 19:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140718222706.GP24496@titan.lakedaemon.net>
On Fri, Jul 18, 2014 at 06:27:06PM -0400, Jason Cooper wrote:
> All,
>
> Here's the accumulated DT changes for v3.17.
>
> Incremental pull request from tags/mvebu-dt-3.17 up to
> tags/mvebu-dt-3.17-2 on the mvebu/dt branch.
>
> Please pull.
>
> thx,
>
> Jason.
>
>
> The following changes since commit d854fa8a1500bec982ed9cb26b82d96bd5ae8dab:
>
> ARM: kirkwood: fix net5big regulator gpio assignments (2014-06-21 19:21:13 +0000)
>
> are available in the git repository at:
>
> git://git.infradead.org/linux-mvebu.git tags/mvebu-dt-3.17-2
Merged, thanks.
-Olof
^ permalink raw reply
* [PATCH 1/2] PCI: spear: Fix Section mismatch compilation warning for probe()
From: Olof Johansson @ 2014-07-20 19:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <76576b662613369d0c71db30ceed4ed5ae1343d2.1405857442.git.viresh.kumar@linaro.org>
On Sun, Jul 20, 2014 at 05:29:47PM +0530, Viresh Kumar wrote:
> Following compilation warning occurs when compiled with:
> CONFIG_DEBUG_SECTION_MISMATCH=y
>
> WARNING: drivers/pci/host/built-in.o(.data+0xc0): Section mismatch in
> reference from the variable spear13xx_pcie_driver to the function
> .init.text:spear13xx_pcie_probe()
>
> Both .probe() and pcie_init() are marked with __init, but spear13xx_pcie_driver
> isn't. And so section mismatch.
>
> Fix it by marking spear13xx_pcie_driver with __initdata.
>
> Fixes: 51b66a6 (PCI: spear: Add PCIe driver for ST Microelectronics SPEAr13xx)
Please use 12 significant digits, since 7 might not be sufficient later
on in time. I've fixed it up here.
> Reported-by: Olof Johansson <olof@lixom.net>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> Olof/Arnd,
>
> Let me know if a PULL request is required for this, otherwise just apply them
> directly.
I've applied both of these on top of next/drivers, where your previous branch
was. No need for a pull request.
-Olof
^ permalink raw reply
* [PATCH 4/4] (RFC) X86: add IPI tracepoints
From: Davidlohr Bueso @ 2014-07-20 20:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1405660735-13408-5-git-send-email-nicolas.pitre@linaro.org>
On Fri, 2014-07-18 at 01:18 -0400, Nicolas Pitre wrote:
> On X86 there are already tracepoints for IRQ vectors through which IPIs
> are handled. However this is highly X86 specific, and the IPI signaling
> is not currently traced.
>
> This is an attempt at adding generic IPI tracepoints to X86.
I welcome this, and fwiw have been trying out this patch. One thing I
would like to see, but due to overhead would probably be better suited
in userspace (trace-cmd, maybe?), is a more packed description of the
IPI. Ie: unifying ipi_init and ipi_exit and show overall cost of the
IPI.
Thanks,
Davidlohr
^ permalink raw reply
* [PATCH 2/2] iio: exynos-adc: add experimental touchscreen support
From: Dmitry Torokhov @ 2014-07-20 20:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53CBC969.4090704@kernel.org>
On Sun, Jul 20, 2014 at 02:51:37PM +0100, Jonathan Cameron wrote:
> On 20/07/14 14:49, Jonathan Cameron wrote:
> >On 18/07/14 20:29, Arnd Bergmann wrote:
> >>This adds support for the touchscreen on Samsung s3c64xx.
> >>The driver is completely untested but shows roughly how
> >>it could be done, following the example of the at91 driver.
> >>
> >Hi Arnd,
> Cc'd linux-input and Dmitry.
> >
> >>Open questions include:
> >>
> >>- compared to the old plat-samsung/adc driver, there is
> >> no support for prioritizing ts over other clients, nor
> >> for oversampling. From my reading of the code, the
> >> priorities didn't actually have any effect at all, but
> >> the oversampling might be needed. Maybe the original
> >> authors have some insight.
> >>
> >>- I simply register the input device from the adc driver
> >> itself, as the at91 code does. The driver also supports
> >> sub-nodes, but I don't understand how they are meant
> >> to be used, so using those might be better.
> >So, the alternative you are (I think) referring to is to use
> >the buffered in kernel client interface. That way a separate
> >touch screen driver can use the output channels provided by IIO
> >in much the same way you might use a regulator or similar.
> >Note that whilst this is similar to the simple polled interface
> >used for things like the iio_hwmon driver, the data flow is
> >quite different (clearly the polled interfce would be
> >inappropriate here).
> >
> >Whilst we've discussed it in the past for touch screen drivers
> >like this, usually the hardware isn't generic enough to be
> >of any real use if not being used as a touch screen. As such
> >it's often simpler to just have the support directly in the
> >driver (as you've observed the at91 driver does this).
> >
> >Whilst the interface has been there a while, it's not really had
> >all that much use. The original target was the simpler case
> >of 3D accelerometer where we have a generic iio to input
> >bridge driver. Time constraints meant that I haven't yet actually
> >formally submitted the input side of this. Whilst there are lots
> >of other things that can use this interface, right now nothing
> >actually does so.
> >
> >>
> >>- The new exynos_read_s3c64xx_ts() function is intentionally
> >> very similar to the existing exynos_read_raw() functions.
> >> It should probably be changed, either by merging the two
> >> into one, or by simplifying the exynos_read_s3c64xx_ts()
> >> function. This depends a bit on the answers to the questions
> >> above.
> >I'd be tempted to not bother keeping them that similar. It's
> >not a generic IIO channel so simplify it where possible.
> >>
> >>- We probably need to add support for platform_data as well,
> >> I've skipped this so far.
> >>
> >>- Is anybody able to debug this driver on real hardware?
> >> While it's possible that it actually works, it's more
> >> likely that I made a few subtle mistakes.
> >>
> >>Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >Looks pretty good to me. A few symantic bits and pieces and
> >one bug spotted. Short and sweet.
> >>
> >>diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> >>index e1b74828f413..4329bf3c3326 100644
> >>--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> >>+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> >>@@ -41,6 +41,10 @@ Required properties:
> >> and compatible ADC block)
> >> - vdd-supply VDD input supply.
> >>
> >>+Optional properties:
> >>+- has-touchscreen: If present, indicates that a touchscreen is
> >>+ connected an usable.
> >>+
> >> Note: child nodes can be added for auto probing from device tree.
> >>
> >> Example: adding device info in dtsi file
> >>diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> >>index 5f95638513d2..cf1d9f3e2492 100644
> >>--- a/drivers/iio/adc/exynos_adc.c
> >>+++ b/drivers/iio/adc/exynos_adc.c
> >>@@ -34,6 +34,7 @@
> >> #include <linux/regulator/consumer.h>
> >> #include <linux/of_platform.h>
> >> #include <linux/err.h>
> >>+#include <linux/input.h>
> >Might want to make the input side optional at compile time...
> >I supose the existing parts are unlikely to be used much in headless
> >devices, but you never know. Maybe we just leave this until someone
> >shouts they want to be able to avoid compiling it in.
> >>
> >> #include <linux/iio/iio.h>
> >> #include <linux/iio/machine.h>
> >>@@ -103,6 +104,7 @@
> >>
> >> /* Bit definitions common for ADC_V1 and ADC_V2 */
> >> #define ADC_CON_EN_START (1u << 0)
> >>+#define ADC_DATX_PRESSED (1u << 15)
> >> #define ADC_DATX_MASK 0xFFF
> >>
> >> #define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
> >>@@ -110,16 +112,20 @@
> >> struct exynos_adc {
> >> struct exynos_adc_data *data;
> >> struct device *dev;
> >>+ struct input_dev *input;
> >> void __iomem *regs;
> >> void __iomem *enable_reg;
> >> struct clk *clk;
> >> struct clk *sclk;
> >> unsigned int irq;
> >>+ unsigned int tsirq;
> >> struct regulator *vdd;
> >>
> >> struct completion completion;
> >>
> >>+ bool read_ts;
> >> u32 value;
> >>+ u32 value2;
> >As I state further down, I'd rather keep a little
> >clear of the naming used in IIO for bits that aren't
> >going through IIO (less confusing!). Maybe just
> >have
> > u32 x, y;
> >> unsigned int version;
> >> };
> >>
> >>@@ -390,12 +396,61 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> >> return ret;
> >> }
> >>
> >>+static int exynos_read_s3c64xx_ts(struct iio_dev *indio_dev,
> >>+ struct iio_chan_spec const *chan,
> >>+ int *val,
> >>+ int *val2,
> >>+ long mask)
> >>+{
> >>+ struct exynos_adc *info = iio_priv(indio_dev);
> >>+ unsigned long timeout;
> >>+ int ret;
> >>+
> >>+ if (mask != IIO_CHAN_INFO_RAW)
> >>+ return -EINVAL;
> >>+
> >>+ mutex_lock(&indio_dev->mlock);
> >>+ info->read_ts = 1;
> >>+
> >>+ reinit_completion(&info->completion);
> >>+
> >>+ writel(ADC_S3C2410_TSC_PULL_UP_DISABLE | ADC_TSC_AUTOPST,
> >>+ ADC_V1_TSC(info->regs));
> >>+
> >>+ /* Select the ts channel to be used and Trigger conversion */
> >>+ info->data->start_conv(info, 0);
> >0 is a rather magic value. A define perhaps?
> >>+
> >>+ timeout = wait_for_completion_timeout
> >>+ (&info->completion, EXYNOS_ADC_TIMEOUT);
> >>+ if (timeout == 0) {
> >>+ dev_warn(&indio_dev->dev, "Conversion timed out! Resetting\n");
> >>+ if (info->data->init_hw)
> >>+ info->data->init_hw(info);
> >>+ ret = -ETIMEDOUT;
> >>+ } else {
> >>+ *val = info->value;
> >>+ *val2 = info->value2;
> >This is definitely abuse as those two values are not intended for
> >different values. If you want to do this please use different naming
> >and don't try to fiddle it into the IIO read raw framework.
> >As you've suggested above, better to simplify this code and drop the
> >bits cloned from the other handler.
> >>+ ret = IIO_VAL_INT;
> >>+ }
> >>+
> >>+ info->read_ts = 0;
> >>+ mutex_unlock(&indio_dev->mlock);
> >>+
> >>+ return ret;
> >>+}
> >>+
> >> static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
> >> {
> >> struct exynos_adc *info = (struct exynos_adc *)dev_id;
> >>
> >> /* Read value */
> >>- info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> >>+ if (info->read_ts) {
> >>+ info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> >>+ info->value2 = readl(ADC_V1_DATY(info->regs)) & ADC_DATX_MASK;
> >ADC_DATY_MASK would be more obviously correct.
> >
> >>+ writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
> >Perhaps the above is cryptic enough to warrant a comment?
> >>+ } else {
> >>+ info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> >>+ }
> >>
> >> /* clear irq */
> >> if (info->data->clear_irq)
> >>@@ -406,6 +461,46 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
> >> return IRQ_HANDLED;
> >> }
> >>
> >>+/*
> >>+ * Here we (ab)use a threaded interrupt handler to stay running
> >>+ * for as long as the touchscreen remains pressed, we report
> >>+ * a new event with the latest data and then sleep until the
> >>+ * next timer tick. This mirrors the behavior of the old
> >>+ * driver, with much less code.
> >>+ */
> >>+static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
> >>+{
> >>+ struct exynos_adc *info = dev_id;
> >>+ struct iio_dev *dev = dev_get_drvdata(info->dev);
> >>+ u32 x, y;
> >>+ bool pressed;
> >>+ int ret;
> >>+
> >>+ do {
> >>+ ret =exynos_read_s3c64xx_ts(dev, NULL, &x, &y, IIO_CHAN_INFO_RAW);
> >= exynos
> >>+ if (ret == -ETIMEDOUT)
> >>+ break;
> >>+
> >>+ pressed = x & y & ADC_DATX_PRESSED;
> >>+ if (!pressed)
> >>+ break;
> >>+
> >>+ input_report_abs(info->input, ABS_X, x & ADC_DATX_MASK);
> >>+ input_report_abs(info->input, ABS_Y, y & ADC_DATX_MASK);
> >>+ input_report_key(info->input, BTN_TOUCH, 1);
> >>+ input_sync(info->input);
> >>+
> >>+ msleep(1);
> >>+ } while (1);
It would be nice to actually close the device even if someone is
touching screen. Please implement open/close methods and have them set a
flag that you would check here.
> >>+
> >>+ input_report_key(info->input, BTN_TOUCH, 0);
> >>+ input_sync(info->input);
> >>+
> >>+ writel(0, ADC_V1_CLRINTPNDNUP(info->regs));
> >>+
> >>+ return IRQ_HANDLED;
> >>+}
> >>+
> >> static int exynos_adc_reg_access(struct iio_dev *indio_dev,
> >> unsigned reg, unsigned writeval,
> >> unsigned *readval)
> >>@@ -457,12 +552,57 @@ static int exynos_adc_remove_devices(struct device *dev, void *c)
> >> return 0;
> >> }
> >>
> >>+static int exynos_adc_ts_init(struct exynos_adc *info)
> >>+{
> >>+ int ret;
> >>+
> >>+ info->input = input_allocate_device();
> >>+ if (!info->input)
> >>+ return -ENOMEM;
> >>+
> >>+ info->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> >>+ info->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
> >>+
> >>+ input_set_abs_params(info->input, ABS_X, 0, 0x3FF, 0, 0);
> >>+ input_set_abs_params(info->input, ABS_Y, 0, 0x3FF, 0, 0);
> >>+
> >>+ /* data from s3c2410_ts driver */
> >>+ info->input->name = "S3C24xx TouchScreen";
> >>+ info->input->id.bustype = BUS_HOST;
> >>+ info->input->id.vendor = 0xDEAD;
> >>+ info->input->id.product = 0xBEEF;
You do not need to fill these entries with fake data.
> >>+ info->input->id.version = 0x0200;
> >>+
> >>+ ret = input_register_device(info->input);
> >>+ if (ret) {
> >>+ input_free_device(info->input);
> >>+ goto err;
> >>+ }
> >>+
> >>+ if (info->tsirq > 0)
> >>+ ret = request_threaded_irq(info->irq, NULL, exynos_ts_isr,
> >>+ 0, "touchscreen", info);
> >info->tsirq
> >(that had me really confused for a moment ;)
> >Also, perhaps a more specific name. touchscreen_updown or similar as the
> >main interrupt is also used during touchscreen operation.
> >>+ if (ret < 0) {
> >>+ dev_err(info->dev, "failed requesting touchsccreen irq, irq = %d\n",
> >>+ info->irq);
> >>+ goto err_input;
> >>+ }
> >>+
> >>+ return 0;
> >>+
> >>+err_input:
> >>+ input_unregister_device(info->input);
> >>+err:
> >>+ return ret;
> >>+}
> >>+
> >> static int exynos_adc_probe(struct platform_device *pdev)
> >> {
> >> struct exynos_adc *info = NULL;
> >> struct device_node *np = pdev->dev.of_node;
> >> struct iio_dev *indio_dev = NULL;
> >> struct resource *mem;
> >>+ bool has_ts;
> >> int ret = -ENODEV;
> >> int irq;
> >>
> >>@@ -498,8 +638,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
> >> dev_err(&pdev->dev, "no irq resource?\n");
> >> return irq;
> >> }
> >>-
> >> info->irq = irq;
> >>+
> >>+ irq = platform_get_irq(pdev, 1);
> >>+ if (irq == -EPROBE_DEFER)
> >>+ return irq;
> >>+
> >>+ info->tsirq = irq;
> >>+
> >> info->dev = &pdev->dev;
> >>
> >> init_completion(&info->completion);
> >>@@ -565,6 +711,12 @@ static int exynos_adc_probe(struct platform_device *pdev)
> >> if (info->data->init_hw)
> >> info->data->init_hw(info);
> >>
> >>+ has_ts = of_property_read_bool(pdev->dev.of_node, "has-touchscreen");
> >>+ if (has_ts)
> >>+ ret = exynos_adc_ts_init(info);
> >>+ if (ret)
> >>+ goto err_iio;
> >>+
> >> ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
> >> if (ret < 0) {
> >> dev_err(&pdev->dev, "failed adding child nodes\n");
> >>@@ -576,6 +728,11 @@ static int exynos_adc_probe(struct platform_device *pdev)
> >> err_of_populate:
> >> device_for_each_child(&indio_dev->dev, NULL,
> >> exynos_adc_remove_devices);
> >>+ if (has_ts) {
> >>+ input_unregister_device(info->input);
> >>+ free_irq(info->tsirq, info);
Are we sure that device is quiesced here and interrupt won't arrive
between input_unregister_device() and free_irq()? I guess if you
properly implement open/close it won't matter.
> >>+ }
> >>+err_iio:
> >> iio_device_unregister(indio_dev);
> >> err_irq:
> >> free_irq(info->irq, info);
> >>@@ -595,9 +752,12 @@ static int exynos_adc_remove(struct platform_device *pdev)
> >> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> >> struct exynos_adc *info = iio_priv(indio_dev);
> >>
> >>+ input_free_device(info->input);
Should be unregister, not free.
> >> device_for_each_child(&indio_dev->dev, NULL,
> >> exynos_adc_remove_devices);
> >> iio_device_unregister(indio_dev);
> >>+ if (info->tsirq > 0)
> >>+ free_irq(info->tsirq, info);
> >> free_irq(info->irq, info);
> >> if (info->data->exit_hw)
> >> info->data->exit_hw(info);
> >>
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] arm: Fix me in bios32.c
From: Nick Krause @ 2014-07-20 21:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140720090935.GD21766@n2100.arm.linux.org.uk>
On Sun, Jul 20, 2014 at 5:09 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Jul 20, 2014 at 12:06:19AM -0400, Nicholas Krause wrote:
>> This fixs a fix me in bios32.c for pci_fixup_it8152 as this if
>> statement is incorrect needs to be checked against the class bits
>> not the whole address for the two or conditions and since they don't
>> have define statements outside of their numeratical value.
>
> Unfortunately, this does not address the FIXME at all. The FIXME does
> not state that the if statement is incorrect at all.
>
>> - /* fixup for ITE 8152 devices */
>> - /* FIXME: add defines for class 0x68000 and 0x80103 */
>> if ((dev->class >> 8) == PCI_CLASS_BRIDGE_HOST ||
>> - dev->class == 0x68000 ||
>> - dev->class == 0x80103) {
>> + (dev->class >> 8) == 0x680 ||
>> + (dev->class >> 8) == 0x801) {
>
> The FIXME states that there should be defines for these values (0x68000
> and 0x80103).
>
> The FIXME statement itself is slightly wrong in that these values are
> *not* class IDs. 0x680 and 0x801 are the class ID values.
>
> We have definitions for class IDs 0x680 and 0x801 in
> include/linux/pci_ids.h, which are:
>
> #define PCI_CLASS_BRIDGE_OTHER 0x0680
> #define PCI_CLASS_SYSTEM_DMA 0x0801
>
> So, to fix the stated issue, without changing the functionality of the
> code, this is what I expected you to produce:
>
> /* fixup for ITE 8152 devices */
> if ((dev->class >> 8) == PCI_CLASS_BRIDGE_HOST ||
> dev->class == PCI_CLASS_BRIDGE_OTHER << 8 ||
> def->class == PCI_CLASS_SYSTEM_DMA << 8 | 0x03)
>
> The reason is that "PCI_CLASS_BRIDGE_OTHER << 8" evaluates to 0x68000
> and "PCI_CLASS_SYSTEM_DMA << 8 | 0x03" evaluates to 0x80103. Therefore,
> the compiled code is the same as the original, because the actual
> numerical check is the same as it always was.
>
> As your solution results in a functional change to the code, that would
> introduce a new bug - it results in this test matching any PCI device
> with any programming interface for class "Bridge other" and "System DMA",
> whereas before the code was looking for a specific value there.
>
> Whether this results in the code incorrectly matching devices on the
> system(s) which use this code is difficult to know, so my only choice
> here is to reject your change.
>
> This is why my fellow kernel developers are asking you to stop trying to
> fix this stuff - unless you can produce provably correct patches (or at
> least patches that appear to have no functional change) then you are
> burdening people with your education, and you will get yourself an
> undesirable reputation.
>
> Even with patches which appear to have no functional change, some
> maintainers won't take them unless they have actually been tested on
> real hardware, or that other people agree that the change is a correct
> one. That is because there is a long history of apparantly correct
> changes being made which result in subtle bugs.
>
> Bear in mind that a "FIXME" comment indicates that something is not
> fully correct in some way, and even though it may not be fully correct,
> the resulting code _works_ on the devices that it has been tested with.
> Fixing the "FIXME" may result in the code stopping working on those
> devices - especially if it changes the functionality of the code like
> your patch above.
>
> So, it's often best to leave FIXMEs alone if you don't know what the
> right solution should be.
>
> I hope you will pause, and think about the issues I've raised before
> continuing with your current project of trying to fix FIXMEs.
>
> --
> FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
> according to speedtest.net.
Hey Russell,
Fair enough. I thought it was wrong. I just guessed ,probably needed to search
for the define statements.
Cheers Nick
^ permalink raw reply
* [PATCH 1/2] iio: exynos-adc: add support for s3c64xx adc
From: Hartmut Knaack @ 2014-07-20 21:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5186890.aLtladpMgD@wuerfel>
Arnd Bergmann schrieb:
> The ADC in s3c64xx is almost the same as exynosv1, but
> has a different 'select' method. Adding this here will be
> helpful to move over the existing s3c64xx platform from the
> legacy plat-samsung/adc driver to the new exynos-adc.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
There are some style issues, see the comments inline.
> [In reply to Exynos3250 ADC support, adding Heiko and Ben]
>
> I spent way too much time this week trying to clean up the
> old plat-samsung/adc.c driver as preparation for s3c64xx multiplatform
> support. Eventually I figured out that all that code is much simpler
> done using the new driver. This adds support for s3c64xx in
> samsung-adc.c, similar code changes can be done to support the
> various s3c24xx variants as well.
>
> This first patch should be fairly straightforward but is not tested
> yet. The second patch is more tricky.
>
> Both are based on the exynos3250 patches sent by Chanwoo Choi.
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> index 26232f98d8c5..f84e9250429b 100644
> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> @@ -11,11 +11,21 @@ New driver handles the following
>
> Required properties:
> - compatible: Must be "samsung,exynos-adc-v1"
> - for exynos4412/5250 controllers.
> + for exynos4412/5250 and s5pv210 controllers.
> Must be "samsung,exynos-adc-v2" for
> future controllers.
> Must be "samsung,exynos3250-adc-v2" for
> controllers compatible with ADC of Exynos3250.
> + Must be "samsung,s3c2410-adc" for
> + the ADC in s3c2410 and compatibles
> + Must be "samsung,s3c2416-adc" for
> + the ADC in s3c2416 and compatibles
> + Must be "samsung,s3c2440-adc" for
> + the ADC in s3c2440 and compatibles
> + Must be "samsung,s3c2440-adc" for
> + the ADC in s3c2440 and compatibles
> + Must be "samsung,s3c2443-adc" for
> + the ADC in s3c2443 and compatibles
> - reg: Contains ADC register address range (base address and
> length) and the address of the phy enable register.
> - interrupts: Contains the interrupt information for the timer. The
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index b63e88247eb2..5f95638513d2 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -39,12 +39,16 @@
> #include <linux/iio/machine.h>
> #include <linux/iio/driver.h>
>
> -/* EXYNOS4412/5250 ADC_V1 registers definitions */
> +/* S3C/EXYNOS4412/5250 ADC_V1 registers definitions */
> #define ADC_V1_CON(x) ((x) + 0x00)
> +#define ADC_V1_TSC(x) ((x) + 0x04)
> #define ADC_V1_DLY(x) ((x) + 0x08)
> #define ADC_V1_DATX(x) ((x) + 0x0C)
> +#define ADC_V1_DATY(x) ((x) + 0x10)
> +#define ADC_V1_UPDN(x) ((x) + 0x14)
> #define ADC_V1_INTCLR(x) ((x) + 0x18)
> #define ADC_V1_MUX(x) ((x) + 0x1c)
> +#define ADC_V1_CLRINTPNDNUP(x) ((x) + 0x20)
>
> /* Future ADC_V2 registers definitions */
> #define ADC_V2_CON1(x) ((x) + 0x00)
> @@ -60,6 +64,30 @@
> #define ADC_V1_CON_PRSCLV(x) (((x) & 0xFF) << 6)
> #define ADC_V1_CON_STANDBY (1u << 2)
>
Leave a whitespace around operators below.
> +#define ADC_S3C2410_CON_SELMUX(x) (((x)&0x7)<<3)
> +
> +/* ADCTSC Register Bits */
> +#define ADC_S3C2443_TSC_UD_SEN (1<<8)
> +#define ADC_S3C2410_TSC_YM_SEN (1<<7)
> +#define ADC_S3C2410_TSC_YP_SEN (1<<6)
> +#define ADC_S3C2410_TSC_XM_SEN (1<<5)
> +#define ADC_S3C2410_TSC_XP_SEN (1<<4)
> +#define ADC_S3C2410_TSC_PULL_UP_DISABLE (1<<3)
> +#define ADC_S3C2410_TSC_AUTO_PST (1<<2)
> +#define ADC_S3C2410_TSC_XY_PST(x) (((x)&0x3)<<0)
> +
> +#define ADC_TSC_WAIT4INT (ADC_S3C2410_TSC_YM_SEN | \
> + ADC_S3C2410_TSC_YP_SEN | \
> + ADC_S3C2410_TSC_XP_SEN | \
> + ADC_S3C2410_TSC_XY_PST(3))
> +
> +#define ADC_TSC_AUTOPST (ADC_S3C2410_TSC_YM_SEN | \
> + ADC_S3C2410_TSC_YP_SEN | \
> + ADC_S3C2410_TSC_XP_SEN | \
> + ADC_S3C2410_TSC_AUTO_PST | \
> + ADC_S3C2410_TSC_XY_PST(0))
> +
> +
> /* Bit definitions for ADC_V2 */
> #define ADC_V2_CON1_SOFT_RESET (1u << 2)
>
> @@ -195,6 +223,26 @@ static void exynos_adc_v1_clear_irq(struct exynos_adc *info)
> writel(1, ADC_V1_INTCLR(info->regs));
> }
>
> +static void exynos_adc_s3c64xx_start_conv(struct exynos_adc *info,
> + unsigned long addr)
This indention could be a bit better.
> +{
> + u32 con1;
> +
> + con1 = readl(ADC_V1_CON(info->regs));
> + con1 &= ~ADC_S3C2410_CON_SELMUX(7);
> + con1 |= ADC_S3C2410_CON_SELMUX(addr);
> + writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
> +}
> +
> +static struct exynos_adc_data const exynos_adc_s3c64xx_data = {
> + .num_channels = MAX_ADC_V1_CHANNELS,
> +
> + .init_hw = exynos_adc_v1_init_hw,
> + .exit_hw = exynos_adc_v1_exit_hw,
> + .clear_irq = exynos_adc_v1_clear_irq,
> + .start_conv = exynos_adc_s3c64xx_start_conv,
> +};
> +
> static void exynos_adc_v1_start_conv(struct exynos_adc *info,
> unsigned long addr)
> {
> @@ -280,6 +328,9 @@ static struct exynos_adc_data const exynos3250_adc_v2_data = {
>
> static const struct of_device_id exynos_adc_match[] = {
> {
> + .compatible = "samsung,s3c64100-adc",
> + .data = &exynos_adc_s3c64xx_data,
> + }, {
> .compatible = "samsung,exynos-adc-v1",
> .data = (void *)&exynos_adc_v1_data,
> }, {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH 2/2] iio: exynos-adc: add experimental touchscreen support
From: Hartmut Knaack @ 2014-07-20 21:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <53CBC8D6.6020509@kernel.org>
Jonathan Cameron schrieb:
> On 18/07/14 20:29, Arnd Bergmann wrote:
>> This adds support for the touchscreen on Samsung s3c64xx.
>> The driver is completely untested but shows roughly how
>> it could be done, following the example of the at91 driver.
>>
> Hi Arnd,
>
>> Open questions include:
>>
>> - compared to the old plat-samsung/adc driver, there is
>> no support for prioritizing ts over other clients, nor
>> for oversampling. From my reading of the code, the
>> priorities didn't actually have any effect at all, but
>> the oversampling might be needed. Maybe the original
>> authors have some insight.
>>
>> - I simply register the input device from the adc driver
>> itself, as the at91 code does. The driver also supports
>> sub-nodes, but I don't understand how they are meant
>> to be used, so using those might be better.
> So, the alternative you are (I think) referring to is to use
> the buffered in kernel client interface. That way a separate
> touch screen driver can use the output channels provided by IIO
> in much the same way you might use a regulator or similar.
> Note that whilst this is similar to the simple polled interface
> used for things like the iio_hwmon driver, the data flow is
> quite different (clearly the polled interfce would be
> inappropriate here).
>
> Whilst we've discussed it in the past for touch screen drivers
> like this, usually the hardware isn't generic enough to be
> of any real use if not being used as a touch screen. As such
> it's often simpler to just have the support directly in the
> driver (as you've observed the at91 driver does this).
>
> Whilst the interface has been there a while, it's not really had
> all that much use. The original target was the simpler case
> of 3D accelerometer where we have a generic iio to input
> bridge driver. Time constraints meant that I haven't yet actually
> formally submitted the input side of this. Whilst there are lots
> of other things that can use this interface, right now nothing
> actually does so.
>
>> - The new exynos_read_s3c64xx_ts() function is intentionally
>> very similar to the existing exynos_read_raw() functions.
>> It should probably be changed, either by merging the two
>> into one, or by simplifying the exynos_read_s3c64xx_ts()
>> function. This depends a bit on the answers to the questions
>> above.
> I'd be tempted to not bother keeping them that similar. It's
> not a generic IIO channel so simplify it where possible.
>> - We probably need to add support for platform_data as well,
>> I've skipped this so far.
>>
>> - Is anybody able to debug this driver on real hardware?
>> While it's possible that it actually works, it's more
>> likely that I made a few subtle mistakes.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Looks pretty good to me. A few symantic bits and pieces and
> one bug spotted. Short and sweet.
>> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> index e1b74828f413..4329bf3c3326 100644
>> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> @@ -41,6 +41,10 @@ Required properties:
>> and compatible ADC block)
>> - vdd-supply VDD input supply.
>>
>> +Optional properties:
>> +- has-touchscreen: If present, indicates that a touchscreen is
>> + connected an usable.
>> +
>> Note: child nodes can be added for auto probing from device tree.
>>
>> Example: adding device info in dtsi file
>> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
>> index 5f95638513d2..cf1d9f3e2492 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -34,6 +34,7 @@
>> #include <linux/regulator/consumer.h>
>> #include <linux/of_platform.h>
>> #include <linux/err.h>
>> +#include <linux/input.h>
> Might want to make the input side optional at compile time...
> I supose the existing parts are unlikely to be used much in headless
> devices, but you never know. Maybe we just leave this until someone
> shouts they want to be able to avoid compiling it in.
>> #include <linux/iio/iio.h>
>> #include <linux/iio/machine.h>
>> @@ -103,6 +104,7 @@
>>
>> /* Bit definitions common for ADC_V1 and ADC_V2 */
>> #define ADC_CON_EN_START (1u << 0)
>> +#define ADC_DATX_PRESSED (1u << 15)
>> #define ADC_DATX_MASK 0xFFF
>>
>> #define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
>> @@ -110,16 +112,20 @@
>> struct exynos_adc {
>> struct exynos_adc_data *data;
>> struct device *dev;
>> + struct input_dev *input;
>> void __iomem *regs;
>> void __iomem *enable_reg;
>> struct clk *clk;
>> struct clk *sclk;
>> unsigned int irq;
>> + unsigned int tsirq;
>> struct regulator *vdd;
>>
>> struct completion completion;
>>
>> + bool read_ts;
>> u32 value;
>> + u32 value2;
> As I state further down, I'd rather keep a little
> clear of the naming used in IIO for bits that aren't
> going through IIO (less confusing!). Maybe just
> have
> u32 x, y;
>> unsigned int version;
>> };
>>
>> @@ -390,12 +396,61 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
>> return ret;
>> }
>>
>> +static int exynos_read_s3c64xx_ts(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int *val,
>> + int *val2,
>> + long mask)
>> +{
>> + struct exynos_adc *info = iio_priv(indio_dev);
>> + unsigned long timeout;
>> + int ret;
>> +
>> + if (mask != IIO_CHAN_INFO_RAW)
>> + return -EINVAL;
>> +
>> + mutex_lock(&indio_dev->mlock);
>> + info->read_ts = 1;
Since info->read_ts is of type bool, use true/false.
>> +
>> + reinit_completion(&info->completion);
>> +
>> + writel(ADC_S3C2410_TSC_PULL_UP_DISABLE | ADC_TSC_AUTOPST,
>> + ADC_V1_TSC(info->regs));
>> +
>> + /* Select the ts channel to be used and Trigger conversion */
>> + info->data->start_conv(info, 0);
> 0 is a rather magic value. A define perhaps?
>> +
>> + timeout = wait_for_completion_timeout
>> + (&info->completion, EXYNOS_ADC_TIMEOUT);
>> + if (timeout == 0) {
>> + dev_warn(&indio_dev->dev, "Conversion timed out! Resetting\n");
>> + if (info->data->init_hw)
>> + info->data->init_hw(info);
>> + ret = -ETIMEDOUT;
>> + } else {
>> + *val = info->value;
>> + *val2 = info->value2;
> This is definitely abuse as those two values are not intended for
> different values. If you want to do this please use different naming
> and don't try to fiddle it into the IIO read raw framework.
> As you've suggested above, better to simplify this code and drop the
> bits cloned from the other handler.
>> + ret = IIO_VAL_INT;
>> + }
>> +
>> + info->read_ts = 0;
>> + mutex_unlock(&indio_dev->mlock);
>> +
>> + return ret;
>> +}
>> +
>> static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>> {
>> struct exynos_adc *info = (struct exynos_adc *)dev_id;
>>
>> /* Read value */
>> - info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
>> + if (info->read_ts) {
>> + info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
>> + info->value2 = readl(ADC_V1_DATY(info->regs)) & ADC_DATX_MASK;
> ADC_DATY_MASK would be more obviously correct.
>
>> + writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN, ADC_V1_TSC(info->regs));
> Perhaps the above is cryptic enough to warrant a comment?
>> + } else {
>> + info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
>> + }
>>
>> /* clear irq */
>> if (info->data->clear_irq)
>> @@ -406,6 +461,46 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> +/*
>> + * Here we (ab)use a threaded interrupt handler to stay running
>> + * for as long as the touchscreen remains pressed, we report
>> + * a new event with the latest data and then sleep until the
>> + * next timer tick. This mirrors the behavior of the old
>> + * driver, with much less code.
>> + */
>> +static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
>> +{
>> + struct exynos_adc *info = dev_id;
>> + struct iio_dev *dev = dev_get_drvdata(info->dev);
>> + u32 x, y;
>> + bool pressed;
>> + int ret;
>> +
>> + do {
>> + ret =exynos_read_s3c64xx_ts(dev, NULL, &x, &y, IIO_CHAN_INFO_RAW);
> = exynos
>> + if (ret == -ETIMEDOUT)
>> + break;
>> +
>> + pressed = x & y & ADC_DATX_PRESSED;
>> + if (!pressed)
>> + break;
>> +
>> + input_report_abs(info->input, ABS_X, x & ADC_DATX_MASK);
>> + input_report_abs(info->input, ABS_Y, y & ADC_DATX_MASK);
>> + input_report_key(info->input, BTN_TOUCH, 1);
>> + input_sync(info->input);
>> +
>> + msleep(1);
>> + } while (1);
>> +
>> + input_report_key(info->input, BTN_TOUCH, 0);
>> + input_sync(info->input);
>> +
>> + writel(0, ADC_V1_CLRINTPNDNUP(info->regs));
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> static int exynos_adc_reg_access(struct iio_dev *indio_dev,
>> unsigned reg, unsigned writeval,
>> unsigned *readval)
>> @@ -457,12 +552,57 @@ static int exynos_adc_remove_devices(struct device *dev, void *c)
>> return 0;
>> }
>>
>> +static int exynos_adc_ts_init(struct exynos_adc *info)
>> +{
>> + int ret;
>> +
>> + info->input = input_allocate_device();
>> + if (!info->input)
>> + return -ENOMEM;
>> +
>> + info->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
>> + info->input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
>> +
>> + input_set_abs_params(info->input, ABS_X, 0, 0x3FF, 0, 0);
>> + input_set_abs_params(info->input, ABS_Y, 0, 0x3FF, 0, 0);
>> +
>> + /* data from s3c2410_ts driver */
>> + info->input->name = "S3C24xx TouchScreen";
>> + info->input->id.bustype = BUS_HOST;
>> + info->input->id.vendor = 0xDEAD;
>> + info->input->id.product = 0xBEEF;
>> + info->input->id.version = 0x0200;
>> +
>> + ret = input_register_device(info->input);
>> + if (ret) {
>> + input_free_device(info->input);
>> + goto err;
Just return ret, without goto (and get rid of label err).
>> + }
>> +
>> + if (info->tsirq > 0)
>> + ret = request_threaded_irq(info->irq, NULL, exynos_ts_isr,
>> + 0, "touchscreen", info);
> info->tsirq
> (that had me really confused for a moment ;)
> Also, perhaps a more specific name. touchscreen_updown or similar as the
> main interrupt is also used during touchscreen operation.
>> + if (ret < 0) {
>> + dev_err(info->dev, "failed requesting touchsccreen irq, irq = %d\n",
>> + info->irq);
>> + goto err_input;
>> + }
>> +
>> + return 0;
>> +
Probably better to get rid of the labels and move the code up, as it is only used once.
>> +err_input:
>> + input_unregister_device(info->input);
>> +err:
>> + return ret;
>> +}
>> +
>> static int exynos_adc_probe(struct platform_device *pdev)
>> {
>> struct exynos_adc *info = NULL;
>> struct device_node *np = pdev->dev.of_node;
>> struct iio_dev *indio_dev = NULL;
>> struct resource *mem;
>> + bool has_ts;
>> int ret = -ENODEV;
>> int irq;
>>
>> @@ -498,8 +638,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
>> dev_err(&pdev->dev, "no irq resource?\n");
>> return irq;
>> }
>> -
>> info->irq = irq;
>> +
>> + irq = platform_get_irq(pdev, 1);
>> + if (irq == -EPROBE_DEFER)
>> + return irq;
What about other possible error codes?
>> +
>> + info->tsirq = irq;
>> +
>> info->dev = &pdev->dev;
>>
>> init_completion(&info->completion);
>> @@ -565,6 +711,12 @@ static int exynos_adc_probe(struct platform_device *pdev)
>> if (info->data->init_hw)
>> info->data->init_hw(info);
>>
>> + has_ts = of_property_read_bool(pdev->dev.of_node, "has-touchscreen");
>> + if (has_ts)
>> + ret = exynos_adc_ts_init(info);
>> + if (ret)
>> + goto err_iio;
>> +
>> ret = of_platform_populate(np, exynos_adc_match, NULL, &indio_dev->dev);
>> if (ret < 0) {
>> dev_err(&pdev->dev, "failed adding child nodes\n");
>> @@ -576,6 +728,11 @@ static int exynos_adc_probe(struct platform_device *pdev)
>> err_of_populate:
>> device_for_each_child(&indio_dev->dev, NULL,
>> exynos_adc_remove_devices);
>> + if (has_ts) {
>> + input_unregister_device(info->input);
>> + free_irq(info->tsirq, info);
>> + }
>> +err_iio:
>> iio_device_unregister(indio_dev);
>> err_irq:
>> free_irq(info->irq, info);
>> @@ -595,9 +752,12 @@ static int exynos_adc_remove(struct platform_device *pdev)
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct exynos_adc *info = iio_priv(indio_dev);
>>
>> + input_free_device(info->input);
>> device_for_each_child(&indio_dev->dev, NULL,
>> exynos_adc_remove_devices);
>> iio_device_unregister(indio_dev);
>> + if (info->tsirq > 0)
>> + free_irq(info->tsirq, info);
>> free_irq(info->irq, info);
>> if (info->data->exit_hw)
>> info->data->exit_hw(info);
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox