* [PATCH v9 01/19] remoteproc: st_slim_rproc: add a slimcore rproc driver
From: Lee Jones @ 2016-09-14 8:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913175656.GC21438@tuxbot>
On Tue, 13 Sep 2016, Bjorn Andersson wrote:
> On Mon 05 Sep 06:16 PDT 2016, Peter Griffin wrote:
>
> > slim core is used as a basis for many IPs in the STi
> > chipsets such as fdma and demux. To avoid duplicating
> > the elf loading code in each device driver a slim
> > rproc driver has been created.
> >
> > This driver is designed to be used by other device drivers
> > such as fdma, or demux whose IP is based around a slim core.
> > The device driver can call slim_rproc_alloc() to allocate
> > a slim rproc and slim_rproc_put() when finished.
> >
> > This driver takes care of ioremapping the slim
> > registers (dmem, imem, slimcore, peripherals), whose offsets
> > and sizes can change between IP's. It also obtains and enables
> > any clocks used by the device. This approach avoids having
> > a double mapping of the registers as slim_rproc does not register
> > its own platform device. It also maps well to device tree
> > abstraction as it allows us to have one dt node for the whole
> > device.
> >
> > All of the generic rproc elf loading code can be reused, and
> > we provide start() stop() hooks to start and stop the slim
> > core once the firmware has been loaded. This has been tested
> > successfully with fdma driver.
> >
> > Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
>
> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
What's preventing this from being applied right away?
> > ---
> > drivers/remoteproc/Kconfig | 4 +
> > drivers/remoteproc/Makefile | 1 +
> > drivers/remoteproc/st_slim_rproc.c | 364 +++++++++++++++++++++++++++++++
> > include/linux/remoteproc/st_slim_rproc.h | 58 +++++
> > 4 files changed, 427 insertions(+)
> > create mode 100644 drivers/remoteproc/st_slim_rproc.c
> > create mode 100644 include/linux/remoteproc/st_slim_rproc.h
> >
> > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> > index 1a8bf76a..a7bedc6 100644
> > --- a/drivers/remoteproc/Kconfig
> > +++ b/drivers/remoteproc/Kconfig
> > @@ -100,4 +100,8 @@ config ST_REMOTEPROC
> > processor framework.
> > This can be either built-in or a loadable module.
> >
> > +config ST_SLIM_REMOTEPROC
> > + tristate
> > + select REMOTEPROC
> > +
> > endmenu
> > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> > index 92d3758..db1dae7 100644
> > --- a/drivers/remoteproc/Makefile
> > +++ b/drivers/remoteproc/Makefile
> > @@ -14,3 +14,4 @@ obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> > obj-$(CONFIG_QCOM_MDT_LOADER) += qcom_mdt_loader.o
> > obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
> > obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
> > +obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
> > diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
> > new file mode 100644
> > index 0000000..1484e97
> > --- /dev/null
> > +++ b/drivers/remoteproc/st_slim_rproc.c
> > @@ -0,0 +1,364 @@
> > +/*
> > + * SLIM core rproc driver
> > + *
> > + * Copyright (C) 2016 STMicroelectronics
> > + *
> > + * Author: Peter Griffin <peter.griffin@linaro.org>
> > + *
> > + * 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.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/remoteproc.h>
> > +#include <linux/remoteproc/st_slim_rproc.h>
> > +#include "remoteproc_internal.h"
> > +
> > +/* SLIM core registers */
> > +#define SLIM_ID_OFST 0x0
> > +#define SLIM_VER_OFST 0x4
> > +
> > +#define SLIM_EN_OFST 0x8
> > +#define SLIM_EN_RUN BIT(0)
> > +
> > +#define SLIM_CLK_GATE_OFST 0xC
> > +#define SLIM_CLK_GATE_DIS BIT(0)
> > +#define SLIM_CLK_GATE_RESET BIT(2)
> > +
> > +#define SLIM_SLIM_PC_OFST 0x20
> > +
> > +/* DMEM registers */
> > +#define SLIM_REV_ID_OFST 0x0
> > +#define SLIM_REV_ID_MIN_MASK GENMASK(15, 8)
> > +#define SLIM_REV_ID_MIN(id) ((id & SLIM_REV_ID_MIN_MASK) >> 8)
> > +#define SLIM_REV_ID_MAJ_MASK GENMASK(23, 16)
> > +#define SLIM_REV_ID_MAJ(id) ((id & SLIM_REV_ID_MAJ_MASK) >> 16)
> > +
> > +
> > +/* peripherals registers */
> > +#define SLIM_STBUS_SYNC_OFST 0xF88
> > +#define SLIM_STBUS_SYNC_DIS BIT(0)
> > +
> > +#define SLIM_INT_SET_OFST 0xFD4
> > +#define SLIM_INT_CLR_OFST 0xFD8
> > +#define SLIM_INT_MASK_OFST 0xFDC
> > +
> > +#define SLIM_CMD_CLR_OFST 0xFC8
> > +#define SLIM_CMD_MASK_OFST 0xFCC
> > +
> > +static const char *mem_names[ST_SLIM_MEM_MAX] = {
> > + [ST_SLIM_DMEM] = "dmem",
> > + [ST_SLIM_IMEM] = "imem",
> > +};
> > +
> > +static int slim_clk_get(struct st_slim_rproc *slim_rproc, struct device *dev)
> > +{
> > + int clk, err;
> > +
> > + for (clk = 0; clk < ST_SLIM_MAX_CLK; clk++) {
> > + slim_rproc->clks[clk] = of_clk_get(dev->of_node, clk);
> > + if (IS_ERR(slim_rproc->clks[clk])) {
> > + err = PTR_ERR(slim_rproc->clks[clk]);
> > + if (err == -EPROBE_DEFER)
> > + goto err_put_clks;
> > + slim_rproc->clks[clk] = NULL;
> > + break;
> > + }
> > + }
> > +
> > + return 0;
> > +
> > +err_put_clks:
> > + while (--clk >= 0)
> > + clk_put(slim_rproc->clks[clk]);
> > +
> > + return err;
> > +}
> > +
> > +static void slim_clk_disable(struct st_slim_rproc *slim_rproc)
> > +{
> > + int clk;
> > +
> > + for (clk = 0; clk < ST_SLIM_MAX_CLK && slim_rproc->clks[clk]; clk++)
> > + clk_disable_unprepare(slim_rproc->clks[clk]);
> > +}
> > +
> > +static int slim_clk_enable(struct st_slim_rproc *slim_rproc)
> > +{
> > + int clk, ret;
> > +
> > + for (clk = 0; clk < ST_SLIM_MAX_CLK && slim_rproc->clks[clk]; clk++) {
> > + ret = clk_prepare_enable(slim_rproc->clks[clk]);
> > + if (ret)
> > + goto err_disable_clks;
> > + }
> > +
> > + return 0;
> > +
> > +err_disable_clks:
> > + while (--clk >= 0)
> > + clk_disable_unprepare(slim_rproc->clks[clk]);
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * Remoteproc slim specific device handlers
> > + */
> > +static int slim_rproc_start(struct rproc *rproc)
> > +{
> > + struct device *dev = &rproc->dev;
> > + struct st_slim_rproc *slim_rproc = rproc->priv;
> > + unsigned long hw_id, hw_ver, fw_rev;
> > + u32 val;
> > +
> > + /* disable CPU pipeline clock & reset CPU pipeline */
> > + val = SLIM_CLK_GATE_DIS | SLIM_CLK_GATE_RESET;
> > + writel(val, slim_rproc->slimcore + SLIM_CLK_GATE_OFST);
> > +
> > + /* disable SLIM core STBus sync */
> > + writel(SLIM_STBUS_SYNC_DIS, slim_rproc->peri + SLIM_STBUS_SYNC_OFST);
> > +
> > + /* enable cpu pipeline clock */
> > + writel(!SLIM_CLK_GATE_DIS,
> > + slim_rproc->slimcore + SLIM_CLK_GATE_OFST);
> > +
> > + /* clear int & cmd mailbox */
> > + writel(~0U, slim_rproc->peri + SLIM_INT_CLR_OFST);
> > + writel(~0U, slim_rproc->peri + SLIM_CMD_CLR_OFST);
> > +
> > + /* enable all channels cmd & int */
> > + writel(~0U, slim_rproc->peri + SLIM_INT_MASK_OFST);
> > + writel(~0U, slim_rproc->peri + SLIM_CMD_MASK_OFST);
> > +
> > + /* enable cpu */
> > + writel(SLIM_EN_RUN, slim_rproc->slimcore + SLIM_EN_OFST);
> > +
> > + hw_id = readl_relaxed(slim_rproc->slimcore + SLIM_ID_OFST);
> > + hw_ver = readl_relaxed(slim_rproc->slimcore + SLIM_VER_OFST);
> > +
> > + fw_rev = readl(slim_rproc->mem[ST_SLIM_DMEM].cpu_addr +
> > + SLIM_REV_ID_OFST);
> > +
> > + dev_info(dev, "fw rev:%ld.%ld on SLIM %ld.%ld\n",
> > + SLIM_REV_ID_MAJ(fw_rev), SLIM_REV_ID_MIN(fw_rev),
> > + hw_id, hw_ver);
> > +
> > + return 0;
> > +}
> > +
> > +static int slim_rproc_stop(struct rproc *rproc)
> > +{
> > + struct st_slim_rproc *slim_rproc = rproc->priv;
> > + u32 val;
> > +
> > + /* mask all (cmd & int) channels */
> > + writel(0UL, slim_rproc->peri + SLIM_INT_MASK_OFST);
> > + writel(0UL, slim_rproc->peri + SLIM_CMD_MASK_OFST);
> > +
> > + /* disable cpu pipeline clock */
> > + writel(SLIM_CLK_GATE_DIS, slim_rproc->slimcore + SLIM_CLK_GATE_OFST);
> > +
> > + writel(!SLIM_EN_RUN, slim_rproc->slimcore + SLIM_EN_OFST);
> > +
> > + val = readl(slim_rproc->slimcore + SLIM_EN_OFST);
> > + if (val & SLIM_EN_RUN)
> > + dev_warn(&rproc->dev, "Failed to disable SLIM");
> > +
> > + dev_dbg(&rproc->dev, "slim stopped\n");
> > +
> > + return 0;
> > +}
> > +
> > +static void *slim_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
> > +{
> > + struct st_slim_rproc *slim_rproc = rproc->priv;
> > + void *va = NULL;
> > + int i;
> > +
> > + for (i = 0; i < ST_SLIM_MEM_MAX; i++) {
> > + if (da != slim_rproc->mem[i].bus_addr)
> > + continue;
> > +
> > + if (len <= slim_rproc->mem[i].size) {
> > + /* __force to make sparse happy with type conversion */
> > + va = (__force void *)slim_rproc->mem[i].cpu_addr;
> > + break;
> > + }
> > + }
> > +
> > + dev_dbg(&rproc->dev, "da = 0x%llx len = 0x%x va = 0x%p\n", da, len, va);
> > +
> > + return va;
> > +}
> > +
> > +static struct rproc_ops slim_rproc_ops = {
> > + .start = slim_rproc_start,
> > + .stop = slim_rproc_stop,
> > + .da_to_va = slim_rproc_da_to_va,
> > +};
> > +
> > +/*
> > + * Firmware handler operations: sanity, boot address, load ...
> > + */
> > +
> > +static struct resource_table empty_rsc_tbl = {
> > + .ver = 1,
> > + .num = 0,
> > +};
> > +
> > +static struct resource_table *slim_rproc_find_rsc_table(struct rproc *rproc,
> > + const struct firmware *fw,
> > + int *tablesz)
> > +{
> > + *tablesz = sizeof(empty_rsc_tbl);
> > + return &empty_rsc_tbl;
> > +}
> > +
> > +static struct rproc_fw_ops slim_rproc_fw_ops = {
> > + .find_rsc_table = slim_rproc_find_rsc_table,
> > +};
> > +
> > +/**
> > + * st_slim_rproc_alloc() - allocate and initialise slim rproc
> > + * @pdev: Pointer to the platform_device struct
> > + * @fw_name: Name of firmware for rproc to use
> > + *
> > + * Function for allocating and initialising a slim rproc for use by
> > + * device drivers whose IP is based around the SLIM core. It
> > + * obtains and enables any clocks required by the SLIM core and also
> > + * ioremaps the various IO.
> > + *
> > + * Returns st_slim_rproc pointer or PTR_ERR() on error.
> > + */
> > +
> > +struct st_slim_rproc *st_slim_rproc_alloc(struct platform_device *pdev,
> > + char *fw_name)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct st_slim_rproc *slim_rproc;
> > + struct device_node *np = dev->of_node;
> > + struct rproc *rproc;
> > + struct resource *res;
> > + int err, i;
> > + const struct rproc_fw_ops *elf_ops;
> > +
> > + if (!fw_name)
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (!of_device_is_compatible(np, "st,slim-rproc"))
> > + return ERR_PTR(-EINVAL);
> > +
> > + rproc = rproc_alloc(dev, np->name, &slim_rproc_ops,
> > + fw_name, sizeof(*slim_rproc));
> > + if (!rproc)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + rproc->has_iommu = false;
> > +
> > + slim_rproc = rproc->priv;
> > + slim_rproc->rproc = rproc;
> > +
> > + elf_ops = rproc->fw_ops;
> > + /* Use some generic elf ops */
> > + slim_rproc_fw_ops.load = elf_ops->load;
> > + slim_rproc_fw_ops.sanity_check = elf_ops->sanity_check;
> > +
> > + rproc->fw_ops = &slim_rproc_fw_ops;
> > +
> > + /* get imem and dmem */
> > + for (i = 0; i < ARRAY_SIZE(mem_names); i++) {
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > + mem_names[i]);
> > +
> > + slim_rproc->mem[i].cpu_addr = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(slim_rproc->mem[i].cpu_addr)) {
> > + dev_err(&pdev->dev, "devm_ioremap_resource failed\n");
> > + err = PTR_ERR(slim_rproc->mem[i].cpu_addr);
> > + goto err;
> > + }
> > + slim_rproc->mem[i].bus_addr = res->start;
> > + slim_rproc->mem[i].size = resource_size(res);
> > + }
> > +
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "slimcore");
> > + slim_rproc->slimcore = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(slim_rproc->slimcore)) {
> > + dev_err(&pdev->dev, "failed to ioremap slimcore IO\n");
> > + err = PTR_ERR(slim_rproc->slimcore);
> > + goto err;
> > + }
> > +
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "peripherals");
> > + slim_rproc->peri = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(slim_rproc->peri)) {
> > + dev_err(&pdev->dev, "failed to ioremap peripherals IO\n");
> > + err = PTR_ERR(slim_rproc->peri);
> > + goto err;
> > + }
> > +
> > + err = slim_clk_get(slim_rproc, dev);
> > + if (err)
> > + goto err;
> > +
> > + err = slim_clk_enable(slim_rproc);
> > + if (err) {
> > + dev_err(dev, "Failed to enable clocks\n");
> > + goto err_clk_put;
> > + }
> > +
> > + /* Register as a remoteproc device */
> > + err = rproc_add(rproc);
> > + if (err) {
> > + dev_err(dev, "registration of slim remoteproc failed\n");
> > + goto err_clk_dis;
> > + }
> > +
> > + return slim_rproc;
> > +
> > +err_clk_dis:
> > + slim_clk_disable(slim_rproc);
> > +err_clk_put:
> > + for (i = 0; i < ST_SLIM_MAX_CLK && slim_rproc->clks[i]; i++)
> > + clk_put(slim_rproc->clks[i]);
> > +err:
> > + rproc_put(rproc);
> > + return ERR_PTR(err);
> > +}
> > +EXPORT_SYMBOL(st_slim_rproc_alloc);
> > +
> > +/**
> > + * st_slim_rproc_put() - put slim rproc resources
> > + * @slim_rproc: Pointer to the st_slim_rproc struct
> > + *
> > + * Function for calling respective _put() functions on slim_rproc resources.
> > + *
> > + */
> > +void st_slim_rproc_put(struct st_slim_rproc *slim_rproc)
> > +{
> > + int clk;
> > +
> > + if (!slim_rproc)
> > + return;
> > +
> > + slim_clk_disable(slim_rproc);
> > +
> > + for (clk = 0; clk < ST_SLIM_MAX_CLK && slim_rproc->clks[clk]; clk++)
> > + clk_put(slim_rproc->clks[clk]);
> > +
> > + rproc_del(slim_rproc->rproc);
> > + rproc_put(slim_rproc->rproc);
> > +}
> > +EXPORT_SYMBOL(st_slim_rproc_put);
> > +
> > +MODULE_AUTHOR("Peter Griffin <peter.griffin@linaro.org>");
> > +MODULE_DESCRIPTION("STMicroelectronics SLIM core rproc driver");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/include/linux/remoteproc/st_slim_rproc.h b/include/linux/remoteproc/st_slim_rproc.h
> > new file mode 100644
> > index 0000000..4155556
> > --- /dev/null
> > +++ b/include/linux/remoteproc/st_slim_rproc.h
> > @@ -0,0 +1,58 @@
> > +/*
> > + * SLIM core rproc driver header
> > + *
> > + * Copyright (C) 2016 STMicroelectronics
> > + *
> > + * Author: Peter Griffin <peter.griffin@linaro.org>
> > + *
> > + * 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.
> > + */
> > +#ifndef _ST_REMOTEPROC_SLIM_H
> > +#define _ST_REMOTEPROC_SLIM_H
> > +
> > +#define ST_SLIM_MEM_MAX 2
> > +#define ST_SLIM_MAX_CLK 4
> > +
> > +enum {
> > + ST_SLIM_DMEM,
> > + ST_SLIM_IMEM,
> > +};
> > +
> > +/**
> > + * struct st_slim_mem - slim internal memory structure
> > + * @cpu_addr: MPU virtual address of the memory region
> > + * @bus_addr: Bus address used to access the memory region
> > + * @size: Size of the memory region
> > + */
> > +struct st_slim_mem {
> > + void __iomem *cpu_addr;
> > + phys_addr_t bus_addr;
> > + size_t size;
> > +};
> > +
> > +/**
> > + * struct st_slim_rproc - SLIM slim core
> > + * @rproc: rproc handle
> > + * @mem: slim memory information
> > + * @slimcore: slim slimcore regs
> > + * @peri: slim peripheral regs
> > + * @clks: slim clocks
> > + */
> > +struct st_slim_rproc {
> > + struct rproc *rproc;
> > + struct st_slim_mem mem[ST_SLIM_MEM_MAX];
> > + void __iomem *slimcore;
> > + void __iomem *peri;
> > +
> > + /* st_slim_rproc private */
> > + struct clk *clks[ST_SLIM_MAX_CLK];
> > +};
> > +
> > +struct st_slim_rproc *st_slim_rproc_alloc(struct platform_device *pdev,
> > + char *fw_name);
> > +void st_slim_rproc_put(struct st_slim_rproc *slim_rproc);
> > +
> > +#endif
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* ARM, SoC: About the use DT-defined properties by 3rd-party drivers
From: Sebastian Frias @ 2016-09-14 8:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913145126.GC23336@leverpostej>
Hi Mark,
On 09/13/2016 04:51 PM, Mark Rutland wrote:
> Hi,
>
> On Tue, Sep 13, 2016 at 04:22:09PM +0200, Sebastian Frias wrote:
>> On 09/13/2016 03:12 PM, Mark Rutland wrote:
>
> [context was deleted, TL;DR: binding review is necessary, and takes
> effort, regardless of presence/absence of a driver]
Ok, I see you deleted the part where I was asking about the 'staging' folders.
$ find . -name 'stag*'
./Documentation/devicetree/bindings/staging
./drivers/staging
...
I don't know if that was intended (since you said you did not read it) but
I think it would be interesting for the discussion to give a little overview
on that.
My understanding is that there's a 'staging' area for bindings, couldn't the
nodes/properties from my example be put in there?
>>> Go and take a look at all the effort that went into sorting out generic
>>> IOMMU bindings, when driver support was written after a large amount of
>>> review to sort out fundamental concepts. We had to sort out fundamentals
>>> before prototype driver code could be written, and while we knew drivers
>>> were coming, an awful lot of review effort came first.
>>
>> Again, you are talking about drivers, but it is not the case at hand.
>
> No, I am not. Please do not presume to put words in my mouth.
I'm sorry, that was not my intention.
I just assumed that because your paragraph mentioned the word "driver" and
appears to have been Linux-only effort geared towards writing drivers:
"...We had to sort out fundamentals before prototype driver code could be
written, and while we knew drivers were coming..."
Maybe I miss some more context on that example.
>
> I explicitly described a case where binding review took effort, and the
> presence or absence of drivers was irrelevant. We later had drivers,
> yes, but we had to understand the hardware to get the binding right
> first.
I see.
So if I understood correctly, it was a cleanup, or a way to unify some
bindings, so that drivers could be written easier?
I can see that taking time, but the underlying idea of this discussion is
that the DT could be divided in different sections.
I think this would allow and encourage SoC manufacturers to publish more
details, since they could use them internally as well.
Obviously, this is not strictly necessary, and I'm sure SoC manufacturers
could choose to just avoid having to deal with the open-source community
entirely by using forks.
However, that does not benefit the community and allows a "one-way" street,
with companies "profiting" from open-source software (in this example,
they would profit unwillingly, yet out of the necessity to avoid dealing
with outside 'restrictions' perceived as unnecessary).
It seems it is a similar case to that of allowing or not binary
drivers/blobs.
If they were not allowed, less companies would choose Linux.
I don't want to go into that discussion, but it seems similar enough to
this one for it to be worth mentioning.
> If there's data which has no consumer, it has no value being in the DT.
> Placing data with no consumer in the DT comes with a number of issues,
> e.g.
>
> a) Some DTS authors will ignore it, and not place data according to it
> in DTs. Hence there's no gain in consistency.
Ok, but in this case the DT author (SoC manufacturer) would have interest
in using it.
>
> b) Though some accident (perhaps a typo, perhaps a misunderstanding of
> the binding), a DT will come to have erroneous data, yet this will go
> unnoticed, as there is no consumer. When later a consumer appears, it
> can't trust existing DTs, and has to either ignore the binding
> entirely, or bodge around each and every broken DT.
>
I think it is likely this has already happened, even with the current
constraints on review.
> c) When a consumer eventually appears, it turned out we didn't capture
> details of the hardware sufficiently, and the binding turns out to be
> useless. At worst, this boils down to (b), at best, we require
> additional properties. In this case absolutely nothing is gained.
Case c) could happen even when a driver is provided, for example if the
driver writer got incomplete documentation.
At any rate, since there was no user, even if the bindings need amending,
it should be easy to do, since there's no backwards compatibility to keep.
> In all cases, all we end up doing is enlarging DTBs, and risk causing
> even more work.
>
> If there *is* going to be a consumer, and if information regarding that
> will be provided, then matters are different, and we can consider a
> binding on its own merit. We need a specific example for that.
>
>> If the "user of the binding" is not Linux, under what circumstances
>> could "Linux" have the legitimacy of guaranteeing or enforcing any Linux-
>> specific commitment?
>
> To at least the extent that if someone says they're not going to bother,
> we clearly have no reason to bother supporting them.
I have the impression that this basically means that DT is Linux only.
Did I misunderstood?
What legitimacy does Linux expects to have when dealing with bindings whose
users are not Linux? For example, FreeBSD, U-Boot or others.
In other words, why would Linux usage and policies regarding DT have to affect
other users of DT?
That essentially limits DT usage;
Of course, it can be forked, etc. like you state below, but that is not the
best outcome for everybody, because this basically forces the use of
undocumented binary DT overlays.
> Note that *nothing* stops you from using the DT container format for
> your own purposes, in violation of every binding and rule we have.
> However, for those cases we clearly won't document them as the standard
> mechanism.
Yes, I'm aware of that possibility, but in that case we would not be having
this conversation :-)
Basically, IMHO there is no reason for this thing to be black or white when
it could be gray and have a win-win situation, that's all.
> There are other things build atop of the DTB format, e.g. FIT, which
> aren't quite devicetree in the usual sense.
>
>>> I understand that goal, and I've asked for a specific example, as this
>>> is not clear-cut. e.g. there has been work on describing secure devices
>>> for QEMU, but this isn't necessarily something we want to expose Linux
>>> to in general.
>>
>> Interesting, do you know where in QEMU's code should I look for that?
>
> Documentation/devicetree/bindings/arm/secure.txt for the basics.
>
> Generally, I'd expect that even if the secure OS were using DT in this
> fashion, the non-secure general purpose OS would be handed a DT
> containing only the non-secure world portions.
>
I see that you did not comment on my example, was that intended?
>> IMHO, they apply only when there's a Linux driver, or any other public
>> user of such bindings. But if there's no user, it seems like an unnecessary
>> constraint.
>
> If there's no user, there's no need for the binding.
But you said earlier that bindings for users different from Linux are considered.
Did I miss something?
> If there's some user somewhere, and that user wants the binding
> rubber-stamped as an "official" binding, they need to follow the usual
> rules for bindings.
>
> If there is a user, and they don't want to follow the usual rules,
> there's no point trying to get the binding rubber-stamped.
I understand, but nothing prevents the creation of DT 'sections':
- a section using rubber-stamped "official" bindings
- a section using 'staging' bindings
right?
What technical issues would this have?
Best regards,
Sebastian
^ permalink raw reply
* [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
From: Philipp Zabel @ 2016-09-14 8:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7hzinbmh40.fsf@baylibre.com>
Am Dienstag, den 13.09.2016, 17:59 -0700 schrieb Kevin Hilman:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
> > On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> [...]
>
> >>> I added Philipp and Hans to this thread - maybe they can comment on this.
> >>> To sum it up, our problem is:
> >>> - there are two separate USB PHYs on Meson GXBB
> >>> - both are sharing the same reset line (provided by the reset-meson driver)
> >>> - during initialization of the PHYs we must only call
> >>> reset_control_reset(rstc) once (if we do it for the first *and* second
> >>> PHY then the first PHY gets confused once the second PHY uses the
> >>> reset because the first PHY's state is reset as well)
> >>
> >> If you have an initially asserted reset line and you can enable the
> >> first module by deasserting the reset via reset_control_deassert (and
> >> reset_control_assert to signal when the module may be disabled again
> >> after use), shared resets are for you.
> >>
> >> If you need a reset pulse or have no direct control over the reset line,
> >> (device_reset), the reset framework currently has no solution for this.
> >> The ugly thing about reset_control_once would be that it can't re-reset
> >> modules when unloading and reloading driver modules.
> >
> > The corresponding reset driver in question is reset-meson, which only
> > implements reset (assert/deassert are not implemented). However, I
> > don't know if this is due to hardware design.
> > I think the hardware implements the latter, but maybe Neil can give
> > more information here (I currently don't have access to my board so I
> > cannot test how the hardware actually behaves).
>
> It's implemented that way because the hardware only supports a reset
> pulse.
Would it be possible to bring down both PHYs drivers, pull the reset
line once, and then bring the drivers back up again on this hardware?
regards
Philipp
^ permalink raw reply
* [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
From: Philipp Zabel @ 2016-09-14 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFBinCC+izGS72TZuiiBu=DjtSmoZXRZ6r76M6rC8W7UTpSD6g@mail.gmail.com>
Am Dienstag, den 13.09.2016, 20:38 +0200 schrieb Martin Blumenstingl:
> Hi Philipp,
>
> On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Hi Martin,
> >
> > Am Freitag, den 09.09.2016, 22:36 +0200 schrieb Martin Blumenstingl:
> >> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> >> > Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> >> >
> >> >> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> >> >>> On 08/09/16 21:42, Kevin Hilman wrote:
> >> >>>>
> >> >>>> Ben Dooks <ben.dooks@codethink.co.uk> writes:
> >> >>>>
> >> >>>>> On 08/09/16 20:52, Martin Blumenstingl wrote:
> >> >>>>>>
> >> >>>>>> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman <khilman@baylibre.com>
> >> >>>>>> wrote:
> >> >>>>>>>>
> >> >>>>>>>> + phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
> >> >>>>>>>> + if (IS_ERR(phy)) {
> >> >>>>>>>> + dev_err(&pdev->dev, "failed to create PHY\n");
> >> >>>>>>>> + return PTR_ERR(phy);
> >> >>>>>>>> + }
> >> >>>>>>>> +
> >> >>>>>>>> + if (usb_reset_refcnt++ == 0) {
> >> >>>>>>>> + ret = device_reset(&pdev->dev);
> >> >>>>>>>> + if (ret) {
> >> >>>>>>>> + dev_err(&phy->dev, "Failed to reset USB PHY\n");
> >> >>>>>>>> + return ret;
> >> >>>>>>>> + }
> >> >>>>>>>> + }
> >> >>>>>>>
> >> >>>>>>>
> >> >>>>>>> The ref count + reset here looks like something that could/should be
> >> >>>>>>> handled in a runtime PM callback.
> >> >>>>>>
> >> >>>>>> Unfortunately that doesn't work (as Jerome found out) because both
> >> >>>>>> PHYs are sharing the same reset line.
> >> >>>>>> So if the second PHY would call device_reset then it would also reset
> >> >>>>>> the first PHY!
> >> >>>>>>
> >> >>>>>> There's a comment above the declaration of usb_reset_refcnt which
> >> >>>>>> tries to explain this:
> >> >>>>>> "The PHYs are sharing a common reset line -> we are only allowed to
> >> >>>>>> reset once for all PHYs."
> >> >>>>>> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
> >> >>>>>> {" line to make it easier to see?
> >> >>>>>>
> >> >>>>>
> >> >>>>> pm-runtime has refcounting in it. When one of the nodes turns on,
> >> >>>>> the pm-runtime will call your driver to say there is a user when
> >> >>>>> this first use turns up.
> >> >>>>>
> >> >>>>> If all the sub-phys turn off and drop their refcount then the driver
> >> >>>>> is called to say there are no more users and you can go to sleep.
> >> >>>>
> >> >>>>
> >> >>>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
> >> >>>>
> >> >>>> The reason is because there are physically two PHY devices[1]. Those 2
> >> >>>> devices will be treated independely by runtime PM, and have separate
> >> >>>> use-counting, which means doing what I proposed would cause a reset to
> >> >>>> happen when either device was probed.
> >> >>>>
> >> >>>> So, I think it's OK as it is.
> >> >>>
> >> >>>
> >> >>> Surely you can do pm_runtime_get/put on the phy's parent platform
> >> >>> device and do it that way?
> >> >> could you please be more specific with that (do you mean pdev->dev.parent)?
> >> >> so we would use pm_runtime_{get_sync,put} with the parent, while we
> >> >> would still define the runtime_resume in our driver.
> >> >
> >> > You'd also need to do get/put on the children, but yes, that's what Ben
> >> > is suggesting.
> >> >
> >> > However, the problem with all of the solutions proposed (runtime PM ones
> >> > included) is that we're forcing a board-specific design issue (2 devices
> >> > sharing a reset line) into a driver that should not have any
> >> > board-specific assumptions in it.
> >> >
> >> > For example, if this driver is used on another platform where different
> >> > PHYs have different reset lines, then one of them (the unlucky one who
> >> > is not probed first) will never get reset. So any form of per-device
> >> > ref-counting is not a portable solution.
> >> indeed, so in simple words we would need something like
> >> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
> >> remember internally if any action has already been executed: if not it
> >> does a _reset, _assert or _deassert and otherwise it does nothing.
> >>
> >> > I'm not sure yet how the reset framework is supposed to handle shared
> >> > reset lines, but that needs some investigation. I quick glance and it
> >> > seems that reset controllers can have shared lines, so that should be
> >> > investigated.
> >> I added Philipp and Hans to this thread - maybe they can comment on this.
> >> To sum it up, our problem is:
> >> - there are two separate USB PHYs on Meson GXBB
> >> - both are sharing the same reset line (provided by the reset-meson driver)
> >> - during initialization of the PHYs we must only call
> >> reset_control_reset(rstc) once (if we do it for the first *and* second
> >> PHY then the first PHY gets confused once the second PHY uses the
> >> reset because the first PHY's state is reset as well)
> >
> > If you have an initially asserted reset line and you can enable the
> > first module by deasserting the reset via reset_control_deassert (and
> > reset_control_assert to signal when the module may be disabled again
> > after use), shared resets are for you.
> >
> > If you need a reset pulse or have no direct control over the reset line,
> > (device_reset), the reset framework currently has no solution for this.
> > The ugly thing about reset_control_once would be that it can't re-reset
> > modules when unloading and reloading driver modules.
> The corresponding reset driver in question is reset-meson, which only
> implements reset (assert/deassert are not implemented). However, I
> don't know if this is due to hardware design.
> I think the hardware implements the latter, but maybe Neil can give
> more information here (I currently don't have access to my board so I
> cannot test how the hardware actually behaves).
>
> > A real solution for shared reset lines with reset pulses would have to
> > be some kind of reset request framework where if one module requests a
> > reset, the other module sharing the reset could be notified, and then
> > either veto the reset or, if possible, cease operations, store its
> > state, and prepare to be reset, too, and afterwards restore state. I'd
> > prefer not to think about this too much unless absolutely necessary.
> I'm not sure if this would work in our case: one PHY instance would
> have to know if the other has already triggered the reset or not.
We could add a triggered flag or a counter to struct reset_control, and
have reset_control_reset_once do nothing if it is already set /
incremented. Since the reset_control goes away with the last consumer,
the shared reset line would get triggered again after unbinding both PHY
devices.
regards
Philipp
^ permalink raw reply
* [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
From: Philipp Zabel @ 2016-09-14 8:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7hzinbmh40.fsf@baylibre.com>
Am Dienstag, den 13.09.2016, 17:59 -0700 schrieb Kevin Hilman:
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
>
> > On Tue, Sep 13, 2016 at 5:28 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> [...]
>
> >>> I added Philipp and Hans to this thread - maybe they can comment on this.
> >>> To sum it up, our problem is:
> >>> - there are two separate USB PHYs on Meson GXBB
> >>> - both are sharing the same reset line (provided by the reset-meson driver)
> >>> - during initialization of the PHYs we must only call
> >>> reset_control_reset(rstc) once (if we do it for the first *and* second
> >>> PHY then the first PHY gets confused once the second PHY uses the
> >>> reset because the first PHY's state is reset as well)
> >>
> >> If you have an initially asserted reset line and you can enable the
> >> first module by deasserting the reset via reset_control_deassert (and
> >> reset_control_assert to signal when the module may be disabled again
> >> after use), shared resets are for you.
> >>
> >> If you need a reset pulse or have no direct control over the reset line,
> >> (device_reset), the reset framework currently has no solution for this.
> >> The ugly thing about reset_control_once would be that it can't re-reset
> >> modules when unloading and reloading driver modules.
> >
> > The corresponding reset driver in question is reset-meson, which only
> > implements reset (assert/deassert are not implemented). However, I
> > don't know if this is due to hardware design.
> > I think the hardware implements the latter, but maybe Neil can give
> > more information here (I currently don't have access to my board so I
> > cannot test how the hardware actually behaves).
>
> It's implemented that way because the hardware only supports a reset
> pulse.
Would it be possible to bring down both PHYs drivers, pull the reset
line once, and then bring the drivers back up again?
regards
Philipp
^ permalink raw reply
* [GIT PULL] ARM: at91: drivers for 4.9
From: Alexandre Belloni @ 2016-09-14 8:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd, Olof,
Here are two fixes generated by Coccinnelle for our memory drivers.
The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:
Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git tags/at91-ab-4.9-drivers
for you to fetch changes up to 7922118f8f3e76eb6534e9221a6c9f496c0ae699:
memory: atmel-sdramc: fix a possible NULL dereference (2016-08-31 19:19:16 +0200)
----------------------------------------------------------------
Drivers for 4.9:
- Coccinelle fixes for atmel-ebi and atmel-sdramc
----------------------------------------------------------------
LABBE Corentin (1):
memory: atmel-sdramc: fix a possible NULL dereference
Wei Yongjun (1):
memory: atmel-ebi: use PTR_ERR_OR_ZERO() to simplify the code
drivers/memory/atmel-ebi.c | 10 ++--------
drivers/memory/atmel-sdramc.c | 4 +---
2 files changed, 3 insertions(+), 11 deletions(-)
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
From: Auger Eric @ 2016-09-14 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473695704.git.robin.murphy@arm.com>
Hi,
On 12/09/2016 18:13, Robin Murphy wrote:
> Hi all,
>
> To any more confusing fixups and crazily numbered extra patches, here's
> a quick v7 with everything rebased into the right order. The significant
> change this time is to implement iommu_fwspec properly from the start,
> which ends up being far simpler and more robust than faffing about
> introducing it somewhere 'less intrusive' to move toward core code later.
>
> New branch in the logical place:
>
> git://linux-arm.org/linux-rm iommu/generic-v7
For information, as discussed privately with Robin I experience some
regressions with the former and now deprecated dt description.
on my AMD Overdrive board and my old dt description I now only see a
single group:
/sys/kernel/iommu_groups/
/sys/kernel/iommu_groups/0
/sys/kernel/iommu_groups/0/devices
/sys/kernel/iommu_groups/0/devices/e0700000.xgmac
whereas I formerly see
/sys/kernel/iommu_groups/
/sys/kernel/iommu_groups/3
/sys/kernel/iommu_groups/3/devices
/sys/kernel/iommu_groups/3/devices/0000:00:00.0
/sys/kernel/iommu_groups/1
/sys/kernel/iommu_groups/1/devices
/sys/kernel/iommu_groups/1/devices/e0700000.xgmac
/sys/kernel/iommu_groups/4
/sys/kernel/iommu_groups/4/devices
/sys/kernel/iommu_groups/4/devices/0000:00:02.2
/sys/kernel/iommu_groups/4/devices/0000:01:00.1
/sys/kernel/iommu_groups/4/devices/0000:00:02.0
/sys/kernel/iommu_groups/4/devices/0000:01:00.0
/sys/kernel/iommu_groups/2
/sys/kernel/iommu_groups/2/devices
/sys/kernel/iommu_groups/2/devices/e0900000.xgmac
/sys/kernel/iommu_groups/0
/sys/kernel/iommu_groups/0/devices
/sys/kernel/iommu_groups/0/devices/f0000000.pcie
This is the group topology without ACS override. Applying the non
upstreamed "pci: Enable overrides for missing ACS capabilities" I used
to see separate groups for each PCIe components. Now I don't see any
difference with and without ACS override.
Thanks
Eric
>
> Robin.
>
> Mark Rutland (1):
> Docs: dt: add PCI IOMMU map bindings
>
> Robin Murphy (21):
> of/irq: Break out msi-map lookup (again)
> iommu/of: Handle iommu-map property for PCI
> iommu: Introduce iommu_fwspec
> Docs: dt: document ARM SMMUv3 generic binding usage
> iommu/arm-smmu: Fall back to global bypass
> iommu/arm-smmu: Implement of_xlate() for SMMUv3
> iommu/arm-smmu: Support non-PCI devices with SMMUv3
> iommu/arm-smmu: Set PRIVCFG in stage 1 STEs
> iommu/arm-smmu: Handle stream IDs more dynamically
> iommu/arm-smmu: Consolidate stream map entry state
> iommu/arm-smmu: Keep track of S2CR state
> iommu/arm-smmu: Refactor mmu-masters handling
> iommu/arm-smmu: Streamline SMMU data lookups
> iommu/arm-smmu: Add a stream map entry iterator
> iommu/arm-smmu: Intelligent SMR allocation
> iommu/arm-smmu: Convert to iommu_fwspec
> Docs: dt: document ARM SMMU generic binding usage
> iommu/arm-smmu: Wire up generic configuration support
> iommu/arm-smmu: Set domain geometry
> iommu/dma: Add support for mapping MSIs
> iommu/dma: Avoid PCI host bridge windows
>
> .../devicetree/bindings/iommu/arm,smmu-v3.txt | 8 +-
> .../devicetree/bindings/iommu/arm,smmu.txt | 63 +-
> .../devicetree/bindings/pci/pci-iommu.txt | 171 ++++
> arch/arm64/mm/dma-mapping.c | 2 +-
> drivers/gpu/drm/exynos/exynos_drm_iommu.h | 2 +-
> drivers/iommu/Kconfig | 2 +-
> drivers/iommu/arm-smmu-v3.c | 386 +++++----
> drivers/iommu/arm-smmu.c | 962 ++++++++++-----------
> drivers/iommu/dma-iommu.c | 161 +++-
> drivers/iommu/iommu.c | 56 ++
> drivers/iommu/of_iommu.c | 52 +-
> drivers/irqchip/irq-gic-v2m.c | 3 +
> drivers/irqchip/irq-gic-v3-its.c | 3 +
> drivers/of/irq.c | 78 +-
> drivers/of/of_pci.c | 102 +++
> include/linux/device.h | 3 +
> include/linux/dma-iommu.h | 12 +-
> include/linux/iommu.h | 38 +
> include/linux/of_pci.h | 10 +
> 19 files changed, 1323 insertions(+), 791 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pci/pci-iommu.txt
>
^ permalink raw reply
* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
From: Stephen Boyd @ 2016-09-14 8:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160914080321.GA21674@b29397-desktop>
Quoting Peter Chen (2016-09-14 01:03:21)
> On Tue, Sep 13, 2016 at 10:58:26PM -0700, Stephen Boyd wrote:
> > Quoting Peter Chen (2016-09-13 20:32:00)
> > > On Tue, Sep 13, 2016 at 06:42:46PM -0700, Stephen Boyd wrote:
> > > > On the db410c 96boards platform we have a TC7USB40MU[1] on the
> > > > board to mux the D+/D- lines from the SoC between a micro usb
> > > > "device" port and a USB hub for "host" roles. Upon a role switch,
> > > > we need to change this mux to forward the D+/D- lines to either
> > > > the port or the hub. Therefore, introduce a driver for this
> > > > device that intercepts extcon USB_HOST events and logically
> > > > asserts a gpio to mux the "host" D+/D- lines when a host cable is
> > > > attached. When the cable goes away, it will logically deassert
> > > > the gpio and mux the "device" lines.
> > >
> > > Would you please draw the design? It can also help me review your
> > > chipidea patch well.
> > >
> > > 1. How many ports on the board?
> > > 2. How the lines are connected on the board?
> > >
> >
> > The schematic for the db410c is publically available here[2][3].
> >
> > There's also the 96boards spec[4] which talks about this switch based
> > design a little bit. See the section titled "Single USB port Example".
> >
> > [2] https://github.com/96boards/documentation/blob/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > [3] https://github.com/96boards/documentation/raw/master/ConsumerEdition/DragonBoard-410c/HardwareDocs/Schematics_DragonBoard.pdf
> > [4] https://www.96boards.org/wp-content/uploads/2015/02/96BoardsCESpecificationv1.0-EA1.pdf
>
> Ok, I see several use cases for this role switch
>
> 1. Using the hardware switch (218-4LPST)
> In this case, you can set USB_SW_SEL as input gpio, and use
> extcon-usb-gpio.c like before, just set this gpio as active
> low at dts.
Nice! I didn't think of this case but it's good that we can support
that with some work.
>
> 2. Using USB_HS_ID as vbus-gpio (input), and USB_SW_SEL as id-gpio (output)
This is pretty much what has been implemented. USB_HS_ID is an
extcon-usb-gpio.c device.
> I can't find hardware relationship between each other, maybe I miss
> something.
I believe USB_SW_SEL is the physical switch (218-4LPST) while
USB_SW_SEL_PM is the software controllable part using a GPIO. USB_HS_ID
is just the vbus line from the uB connector and that line goes straight
into the SoC via a GPIO.
> This use case (design) seems strange, usually, we use ID pin controls
> vbus, but seldom use vbus pin control ID.
> How you would like to implement it? When the USB cable is connected
> (between PC), it receives vbus-gpio interrupt, then you set USB_SW_SEL
> as low? If disconnected, you set USB_SW_SEL as high?
Right. The documented behavior is to detect the micro-usb cable and
drive USB_SW_SEL low. When the cable is unplugged we drive USB_SW_SEL
high. Maybe that should be changed though? If we always sampled
USB_SW_SEL as a USB_HOST extcon and the vbus line as a USB extcon then
we could allow the user to decide either with the physical switch or
with some sort of software control to toggle that gpio.
> When the USB controller works at Host mode, what will happen if the user
> connects USB cable at device port?
The devices on the two type-A connectors will be disconnected and we'll
switch from host mode to device mode.
>
> 3. Using sysfs to switch the role
> Set USB_SW_SEL according to "role" at debugfs
sysfs isn't debugfs, but yes I wonder if we need to worry about the
debugfs role switching support here and toggle the gpio manually without
involving extcon. That would mean we need to have a way for the chipidea
controller to toggle this gpio/mux itself.
>
> Which one you would like to implement? Or anything else I miss?
>
Mostly #2, but I'm concerned that the DT binding is going to force that
decision on others who may have the same switch and want to do #1 or #3.
So how to design it in a way that makes it work in all cases? Also, what
to do if the USB_SW_SEL switch is driven high by the physical switch?
That will "override" any software control we might be able to use, so
hopefully we can detect this somehow and prevent the role switch from
happening.
^ permalink raw reply
* [PATCH 05/33] gpio: add generic single-register fixed-direction GPIO driver
From: Linus Walleij @ 2016-09-14 8:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkdZkCHqYYpYKA+Nvj4XY1zecdbZxMuRe=bbdziJL2fHf6w@mail.gmail.com>
On Thu, Sep 8, 2016 at 3:21 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Sep 5, 2016 at 2:26 PM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
>> On Mon, Sep 05, 2016 at 11:06:28AM +0200, Linus Walleij wrote:
>>> I couldn't resist testing on the Compaq iPAQ h3600. It works the
>>> same as before so:
>>> Tested-by: Linus Walleij <linus.walleij@linaro.org> [for Compaq iPAQ H3600]
>>
>> Great news. I've been thinking about digging out my h3600, but it's
>> very old, and hasn't been turned on for many years. I'm not sure what
>> state it's in.
>>
>> I've been hoping to try booting some kernels with qemu-system-arm, but
>> so far I've completely failed to get qemu-system-arm to do anything
>> useful - it just sits there doing apparently nothing, irrespective of
>> which platform I choose or which kernel I give it.
>>
>>> The only news in the bootlog is this:
>>> sa11x0-pcmcia: probe of sa11x0-pcmcia failed with error -2
>>
>> Not so great news - that's -ENOENT. Did that happen before these
>> changes? That could be that the gpiod lookup table isn't found.
>> However, if that were the case, I'd have expected an error message
>> along the lines of:
>>
>> Failed to get GPIO for xxx: -nnn
>>
>> from soc_pcmcia_request_gpiods(). The other possibility is that
>> we're not getting to sa11x0_drv_pcmcia_legacy_probe() but instead
>> trying to initialise it as a generic sa11x0 socket, and
>> sa11x0_pcmcia_hw_init() is failing as a result.
>>
>> We should be using the legacy probe on H3600, so sa11x0_pcmcia_hw_init()
>> should never be reached.
>
> However that is what happens, this is my callstack after
> adding some prints:
>
> sa11x0_pcmcia_init
> sa11x0_drv_pcmcia_probe
> soc_pcmcia_init_one
> soc_pcmcia_add_one
> soc_pcmcia_hw_init
> sa11x0_pcmcia_hw_init
> soc_pcmcia_add_one: pcmcia HW init failed
> sa11x0-pcmcia: probe of sa11x0-pcmcia failed with error -2
Bah I found the cause, just a simple oneliner typo in one of
the patches. I will comment on the patch in question so you can
fix it up on your branch.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v3 3/7] arm64: Introduce uaccess_{disable,enable} functionality based on TTBR0_EL1
From: Mark Rutland @ 2016-09-14 8:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGXu5jJETdRtuonhtkSxMf+sgCdBj2t2_U8yW2cDwcoGY=qvJw@mail.gmail.com>
On Tue, Sep 13, 2016 at 01:45:21PM -0700, Kees Cook wrote:
> On Tue, Sep 13, 2016 at 10:46 AM, Catalin Marinas
> > +static inline bool system_uses_ttbr0_pan(void)
> > +{
> > + return IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
> > + !cpus_have_cap(ARM64_HAS_PAN);
> > +}
> > +
[...]
> > #define __uaccess_enable(alt) \
> > do { \
> > - asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> > - CONFIG_ARM64_PAN)); \
> > + if (system_uses_ttbr0_pan()) \
> > + uaccess_ttbr0_enable(); \
> > + else \
> > + asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), alt, \
> > + CONFIG_ARM64_PAN)); \
> > } while (0)
>
> Does this mean that with CONFIG_ARM64_SW_TTBR0_PAN, even with ARMv8.1,
> a cpu capability bitmask check is done each time we go through
> __uaccess_{en,dis}able?
Catalin reworked cpus_have_cap() to use static keys [1], and that's
queued in the arm64 for-next/core branch [2].
So this should expand to a single branch or nop that we patch when we
detect the presence/absence of PAN. There should be no bitmap check.
Thanks,
Mark.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/454025.html
[2] https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/log/?h=for-next/core
^ permalink raw reply
* [PATCH 18/33] pcmcia: sa1100: provide generic CF support
From: Linus Walleij @ 2016-09-14 8:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <E1beJl3-0000nS-Nw@rmk-PC.armlinux.org.uk>
On Mon, Aug 29, 2016 at 12:25 PM, Russell King
<rmk+kernel@armlinux.org.uk> wrote:
> Provide generic non-voltage sensing socket support for StrongARM
> platforms using the gpiolib and regulator subsystems to obtain the
> resources to control the socket.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
(...)
> +static int sa11x0_drv_pcmcia_probe(struct platform_device *pdev)
> +{
> + struct soc_pcmcia_socket *skt;
> + struct device *dev = &pdev->dev;
> +
> + if (dev->id == -1)
> + return sa11x0_drv_pcmcia_legacy_probe(pdev);
There is a typo there, it should be pdev->id rather than dev->id.
After fixing this, my legacy h3600 PCMCIA started probing again.
(It revealed another bug in fetching GPIOs but it is an orthogobal
problem altogether, looking into it.)
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v3 5/9] ARM: dts: sun8i-h3: add sun8i-emac ethernet driver
From: LABBE Corentin @ 2016-09-14 8:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160912072933.GC9449@lukather>
On Mon, Sep 12, 2016 at 09:29:33AM +0200, Maxime Ripard wrote:
> On Fri, Sep 09, 2016 at 02:45:13PM +0200, Corentin Labbe wrote:
> > The sun8i-emac is an ethernet MAC hardware that support 10/100/1000
> > speed.
> >
> > This patch enable the sun8i-emac on the Allwinner H3 SoC Device-tree.
> > The SoC H3 have an internal PHY, so optionals syscon and ephy are set.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> > arch/arm/boot/dts/sun8i-h3.dtsi | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> > index a39da6f..a3ac476 100644
> > --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> > @@ -50,6 +50,10 @@
> > / {
> > interrupt-parent = <&gic>;
> >
> > + aliases {
> > + ethernet0 = &emac;
> > + };
> > +
>
> This needs to be done at the board level.
>
ok
> > cpus {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > @@ -446,6 +450,21 @@
> > status = "disabled";
> > };
> >
> > + emac: ethernet at 1c30000 {
> > + compatible = "allwinner,sun8i-h3-emac";
> > + syscon = <&syscon>;
> > + reg = <0x01c30000 0x104>;
> > + reg-names = "emac";
>
> You don't need reg-names anymore.
>
ok
> > + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> > + resets = <&ccu RST_BUS_EMAC>, <&ccu RST_BUS_EPHY>;
> > + reset-names = "ahb", "ephy";
> > + clocks = <&ccu CLK_BUS_EMAC>, <&ccu CLK_BUS_EPHY>;
> > + clock-names = "ahb", "ephy";
>
> I still believe that having the same node for both the PHY and the MAC
> is wrong.
>
Ok I have moved clock/reset of ephy in its node.
Thanks
Regards
Corentin Labbe
^ permalink raw reply
* [RFC/PATCH] usb: misc: Add a driver for TC7USB40MU
From: Stephen Boyd @ 2016-09-14 8:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160914014246.31847-1-stephen.boyd@linaro.org>
Quoting Stephen Boyd (2016-09-13 18:42:46)
> On the db410c 96boards platform we have a TC7USB40MU[1] on the
> board to mux the D+/D- lines from the SoC between a micro usb
> "device" port and a USB hub for "host" roles. Upon a role switch,
> we need to change this mux to forward the D+/D- lines to either
> the port or the hub. Therefore, introduce a driver for this
> device that intercepts extcon USB_HOST events and logically
> asserts a gpio to mux the "host" D+/D- lines when a host cable is
> attached. When the cable goes away, it will logically deassert
> the gpio and mux the "device" lines.
>
> [1] https://toshiba.semicon-storage.com/ap-en/product/logic/bus-switch/detail.TC7USB40MU.html
>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>
> Should I make the extcon part optional? I could see a case where there are two
> "OTG" ports connected to the mux (or two hubs), and for some reason the
> software may want to mux between them at runtime. If we mandate an extcon,
> that won't be possible to support. Perhaps it would be better to have
> the node, but connect it to the usb controller with a phandle (maybe of_graph
> endpoints would be useful too) so that when the controller wants to mux over
> a port it can do so.
Here's some dts mock-up on top of the db410c for the of_graph stuff. I
haven't written any code around it, but the idea is to allow the binding
to specify how the mux is connected to upstream and downstream D+/D-
lines. This way, we can do some dt parsing of the endpoints and their
parent nodes to figure out if the mux needs to be set high or low to use
a device connector or a usb hub based on if the id cable is present.
Maybe I'm over thinking things though and we could just have a DT
property for that.
soc {
usb at 78d9000 {
extcon = <&usb_id>, <&usb_id>;
usb-controller; // needed?
ports {
#address-cells = <1>;
#size-cells = <0>;
port at 0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
usb_output: endpoint at 0 { // USB D+/D-
reg = <0>;
remote-endpoint = <&usb_switch_input>;
};
};
};
};
};
usb2513 {
compatible = "smsc,usb3503";
reset-gpios = <&pm8916_gpios 3 GPIO_ACTIVE_LOW>;
initial-mode = <1>;
usb-hub; // indicate this is a hub
ports {
#address-cells = <1>;
#size-cells = <0>;
port at 0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
usb_hub_input: endpoint at 0 { // USB{DP,DM}_UP
reg = <0>;
remote-endpoint = <&usb_switch_hub_ep>;
};
usb_hub_output1: endpoint at 1 { // USB{DP,DM}_DN1
reg = <1>;
remote-endpoint = <&usb_a2_connector>;
};
usb_hub_output2: endpoint at 2 { // USB{DP,DM}_DN2
reg = <2>;
remote-endpoint = <&usb_a1_connector>;
};
usb_hub_output3: endpoint at 3 { // USB{DP,DM}_DN3
reg = <3>;
// goes to expansion connector
};
};
};
};
usb_id: usb-id {
compatible = "linux,extcon-usb-gpio";
id-gpio = <&msmgpio 121 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&usb_id_default>;
};
usb-switch {
compatible = "toshiba,tc7usb40mu";
switch-gpios = <&pm8916_gpios 4 GPIO_ACTIVE_HIGH>;
extcon = <&usb_id>;
pinctrl-names = "default";
pinctrl-0 = <&usb_sw_sel_pm>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port at 0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
usb_switch_input: endpoint at 0 { // D+/D-
reg = <0>;
remote-endpoint = <&usb_output>;
};
usb_switch_device_ep: endpoint at 1 { // D1+/D1-
reg = <1>;
remote-endpoint = <&usb_ub_connector>;
};
usb_switch_hub_ep: endpoint at 2 { // D2+/D2-
reg = <2>;
remote-endpoint = <&usb_hub_input>;
};
};
};
};
uB-connector {
compatible = "usb-ub-connector";
#address-cells = <1>;
#size-cells = <0>;
usb-connector;
port at 0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
usb_ub_connector: endpoint at 0 {
reg = <0>;
remote-endpoint = <&usb_switch_device_ep>;
};
};
};
usb-A-connector1 {
compatible = "usb-A-connector";
#address-cells = <1>;
#size-cells = <0>;
usb-connector;
port at 0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
usb_a1_connector: endpoint at 0 {
reg = <0>;
remote-endpoint = <&usb_hub_output2>;
};
};
};
usb-A-connector2 {
compatible = "usb-A-connector";
#address-cells = <1>;
#size-cells = <0>;
usb-connector;
port at 0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
usb_a2_connector: endpoint at 0 {
reg = <0>;
remote-endpoint = <&usb_hub_output1>;
};
};
};
};
^ permalink raw reply
* [PATCH 18/33] pcmcia: sa1100: provide generic CF support
From: Russell King - ARM Linux @ 2016-09-14 9:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACRpkda7qHx4JVdVv+q3=twTup9rDD2ke8K-tSH9CkhZMQqemg@mail.gmail.com>
On Wed, Sep 14, 2016 at 10:52:39AM +0200, Linus Walleij wrote:
> On Mon, Aug 29, 2016 at 12:25 PM, Russell King
> <rmk+kernel@armlinux.org.uk> wrote:
>
> > Provide generic non-voltage sensing socket support for StrongARM
> > platforms using the gpiolib and regulator subsystems to obtain the
> > resources to control the socket.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> (...)
>
> > +static int sa11x0_drv_pcmcia_probe(struct platform_device *pdev)
> > +{
> > + struct soc_pcmcia_socket *skt;
> > + struct device *dev = &pdev->dev;
> > +
> > + if (dev->id == -1)
> > + return sa11x0_drv_pcmcia_legacy_probe(pdev);
>
> There is a typo there, it should be pdev->id rather than dev->id.
>
> After fixing this, my legacy h3600 PCMCIA started probing again.
Thanks, I've included a change there, should be part of the branch by
the time you get this email - sa1100 head should be 9ad0f8181616. If
not, it's probably still being pushed out over my slow 'net link.
> (It revealed another bug in fetching GPIOs but it is an orthogobal
> problem altogether, looking into it.)
If you're referring to the generic sa1100 pcmcia code, there's a fix
for that already in my branch.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH v5 2/9] drivers: irqchip: Add STM32 external interrupts support
From: Thomas Gleixner @ 2016-09-14 9:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6941f61d-0b33-4108-0135-b11887cd0488@st.com>
On Tue, 13 Sep 2016, Alexandre Torgue wrote:
> On 09/13/2016 05:21 PM, Thomas Gleixner wrote:
> > On Fri, 9 Sep 2016, Alexandre TORGUE wrote:
> > > +static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
> > > + unsigned int nr_irqs)
> > > +{
> > > + struct irq_data *data = irq_get_irq_data(virq);
> > > +
> > > + irq_gc_mask_clr_bit(data->parent_data);
> >
> > I have a hard time to understand this. The irq domain is not hierarchical.
>
> Actually, I wanted to test ".free" callback function of gpio_irq_domain in
> STM32 pinctrl driver. To do that I modified gpio driver: just after getting
> virq through gpio_to_irq, I called "irq_dispose_mapping(virq)".
> I know it is dirty but I thought it was the only way to test.
>
> Doing that, I see that ".free" callback of gpio domain is called but as it is
> hirerchical ".free" callback for parent domain (exti one) is also called. I
> observed that virq was well unmapped, but not masked at exti level. It is for
> this reason than I added "irq_gc_mask_clr_bit(data->parent_data);" which mask
> interrupt at exti level.
Aargh. I really misread the patch, but this is entirely non obvious and you
should do:
struct irq_data *data = irq_domain_get_irq_data(d, virq);
irq_gc_mask_clr_bit(d);
Then it is entirely clear that you mask the interrupt of _this_ (the exti)
domain.
Now what really bugs me is that you do that at all. An interrupt which is
freed must be masked already. Why is it unmasked in the first place?
Thanks,
tglx
^ permalink raw reply
* [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
From: Will Deacon @ 2016-09-14 9:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <11ebd81e-2ea5-5ff3-35b3-be95f03e05bd@redhat.com>
On Wed, Sep 14, 2016 at 10:41:57AM +0200, Auger Eric wrote:
> Hi,
Hi Eric,
> On 12/09/2016 18:13, Robin Murphy wrote:
> > To any more confusing fixups and crazily numbered extra patches, here's
> > a quick v7 with everything rebased into the right order. The significant
> > change this time is to implement iommu_fwspec properly from the start,
> > which ends up being far simpler and more robust than faffing about
> > introducing it somewhere 'less intrusive' to move toward core code later.
> >
> > New branch in the logical place:
> >
> > git://linux-arm.org/linux-rm iommu/generic-v7
>
> For information, as discussed privately with Robin I experience some
> regressions with the former and now deprecated dt description.
Please can you share the DT you're using so we can reproduce this locally?
Will
^ permalink raw reply
* [PATCH v4 22/22] phy: Add support for Qualcomm's USB HS phy
From: Peter Chen @ 2016-09-14 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <147383455256.1541.16582625295760921275@sboyd-linaro>
On Tue, Sep 13, 2016 at 11:29:12PM -0700, Stephen Boyd wrote:
> Quoting Peter Chen (2016-09-13 19:11:33)
> > On Tue, Sep 13, 2016 at 01:41:44PM -0700, Stephen Boyd wrote:
> > > Quoting Peter Chen (2016-09-13 00:03:58)
> > > > On Wed, Sep 07, 2016 at 02:35:19PM -0700, Stephen Boyd wrote:
> > > > > The high-speed phy on qcom SoCs is controlled via the ULPI
> > > > > viewport.
> > > > >
> > > >
> > > > Hi Stephen, I am a little puzzled how this driver co-work with chipidea
> > > > driver. According to nxp IC guys, the ULPI PHY's clock needs to be enabled
> > > > before access portsc.pts (calling hw_phymode_configure), otherwise,
> > > > the system will hang. But I find you call hw_phymode_configure before
> > > > phy->power_on, doesn't your design have this requirement?
> > >
> > > Which clk needs to be enabled? The xcvr_clk? I believe that clk
> > > corresponds to the "core" clk that we enable in the msm glue driver
> > > layer. When that clk is enabled, the ULPI phy is able to respond to
> > > register read/writes via the ULPI viewport.
> > >
> >
> > The input clock for ULPI PHY, maybe it is ref_clk at this PHY driver,
> > so in your platform, even PHY clock is gated, you can still access
> > portsc.pts to configure PHY mode at controller register?
>
> There are a couple input clocks for this phy. I'm not sure which one the
> nxp IC guys think needs to be enabled. Typically, the ref_clk is always
> on so it's hard for me to test a scenario where it isn't enabled. But
> I'm not sure that the ref_clk is what we're talking about anyway. Would
> you know the frequency perhaps?
I think this ref_clk is ULPI PHY vendor specific, at USB3317, it is
26Mhz.
> The ref_clk is usually 19.2MHz on these
> SoCs. That would match up with the "crystal input" pin in the ULPI
> spec[1].
>
> Do you know if this is documented anywhere in the chipidea manual?
> I'll have to look again and see if there's something in there, but I
> didn't see anything like this.
>
> I would guess that we're talking about the xcvr clock though, because
> from what I see in the manual, this is used to clock the interface
> between the ULPI phy and the controller. In the ULPI spec, this matches
> up with the "clock" signal for the ULPI phy and that usually runs at
> something >= 60MHz.
I think you are right, since controller only concerns the output clock.
So, if you have not enabled xcvr, you may meet hang when set portsc.pts too?
At some designs, ULPI input clock (ref_clk) may from Soc internal,
without enabling it, the controller will not get PHY clock (60Mhz).
When visiting portsc.pts, may meet hang.
>
> >
> > > >
> > > > Besides, you read ulpi id before phy->power_on, how can read work before
> > > > phy power on?
> > > >
> > >
> > > I've found that even having the link clk enabled before phy->power_on
> > > doesn't mean it's possible to read the id registers though. That's
> > > because there can be other power supplies, like regulators, which need
> > > to be on for the phy to operate properly.
> > >
> >
> > Then I am puzzled the current initialization for your case, in my mind,
> > it should like below:
> >
> > qcom_usb_hs_phy_probe->qcom_usb_hs_phy_power_on->ci_ulpi_init
> >
> > Like other PHYs, it should get PHY first, then power on it, after that,
> > you can access its register.
> >
>
> Hmm.. maybe the confusion is in which registers we should be able to
> access? Are we talking about the ULPI viewport MMIO register space or
> the ULPI registers that we access through the viewport? I have a
> hw_phymode_configure() inside of of ci_ulpi_init() so that the
> identification registers through the ULPI viewport read properly
> (assuming there aren't other power requirements like regulators). If we
> don't set the portsc.pts before using the viewport, the viewport doesn't
> work and reads timeout. So we really don't touch the ULPI registers
> except for the scratch space and the id registers until after the phy is
> properly powered on with clks and regulators, because the only place we
> touch them after doing the id checking is in this phy driver in
> qcom_usb_hs_phy_power_on(). We've "solved" the chicken-egg problem where
> we don't know which device driver to probe because the phy needs to be
> powered on to read the id registers to know which device driver to use
> by using DT to match up device drivers instead.
>
> [1] https://www.sparkfun.com/datasheets/Components/SMD/ULPI_v1_1.pdf
Ok, ulpi phy works like USB device on USB bus which create device at
runtime. So, like some hard-wired USB devices, it may needs power
sequence too, otherwise, how it knows which driver can loads.
--
Best Regards,
Peter Chen
^ permalink raw reply
* [PATCH v7 00/22] Generic DT bindings for PCI IOMMUs and ARM SMMU
From: Auger Eric @ 2016-09-14 9:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160914092051.GB19622@arm.com>
Hi Will
On 14/09/2016 11:20, Will Deacon wrote:
> On Wed, Sep 14, 2016 at 10:41:57AM +0200, Auger Eric wrote:
>> Hi,
>
> Hi Eric,
>
>> On 12/09/2016 18:13, Robin Murphy wrote:
>>> To any more confusing fixups and crazily numbered extra patches, here's
>>> a quick v7 with everything rebased into the right order. The significant
>>> change this time is to implement iommu_fwspec properly from the start,
>>> which ends up being far simpler and more robust than faffing about
>>> introducing it somewhere 'less intrusive' to move toward core code later.
>>>
>>> New branch in the logical place:
>>>
>>> git://linux-arm.org/linux-rm iommu/generic-v7
>>
>> For information, as discussed privately with Robin I experience some
>> regressions with the former and now deprecated dt description.
>
> Please can you share the DT you're using so we can reproduce this locally?
Done already
Thanks
Eric
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [PATCH V5 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: Eric Anholt @ 2016-09-14 9:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473759262-27031-3-git-send-email-kernel@martin.sperl.org>
kernel at martin.sperl.org writes:
> From: Martin Sperl <kernel@martin.sperl.org>
>
> Add basic thermal driver for bcm2835 SOC.
>
> This driver currently relies on the firmware setting up the
> tsense HW block and does not set it up itself.
>
> Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
>
> ChangeLog:
> V1 -> V2: added specific settings depending on compatiblity
> added trip point based on register
> setting up ctrl-register if HW is not enabled by firmware
> as per recommendation of Eric (untested)
> check that clock frequency is in range
> (1.9 - 5MHz - as per comment in clk-bcm2835.c)
> V2 -> V4: moved back to thermal (not using bcm sub-directory)
> set polling interval to 1second (was 0ms, so interrupt driven)
This is still:
Acked-by: Eric Anholt <eric@anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160914/703cb871/attachment.sig>
^ permalink raw reply
* [PATCH] pintctrl: amlogic: gxbb: add i2c pins
From: Jerome Brunet @ 2016-09-14 9:45 UTC (permalink / raw)
To: linux-arm-kernel
Add EE domains pins for the i2c devices A,B,C
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
index b06cc12f2500..c3928aa3fefa 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
@@ -200,6 +200,15 @@ static const unsigned int uart_rx_c_pins[] = { PIN(GPIOY_14, EE_OFF) };
static const unsigned int uart_cts_c_pins[] = { PIN(GPIOX_11, EE_OFF) };
static const unsigned int uart_rts_c_pins[] = { PIN(GPIOX_12, EE_OFF) };
+static const unsigned int i2c_sck_a_pins[] = { PIN(GPIODV_25, EE_OFF) };
+static const unsigned int i2c_sda_a_pins[] = { PIN(GPIODV_24, EE_OFF) };
+
+static const unsigned int i2c_sck_b_pins[] = { PIN(GPIODV_27, EE_OFF) };
+static const unsigned int i2c_sda_b_pins[] = { PIN(GPIODV_26, EE_OFF) };
+
+static const unsigned int i2c_sck_c_pins[] = { PIN(GPIODV_29, EE_OFF) };
+static const unsigned int i2c_sda_c_pins[] = { PIN(GPIODV_28, EE_OFF) };
+
static const unsigned int eth_mdio_pins[] = { PIN(GPIOZ_0, EE_OFF) };
static const unsigned int eth_mdc_pins[] = { PIN(GPIOZ_1, EE_OFF) };
static const unsigned int eth_clk_rx_clk_pins[] = { PIN(GPIOZ_2, EE_OFF) };
@@ -438,6 +447,12 @@ static struct meson_pmx_group meson_gxbb_periphs_groups[] = {
GROUP(uart_rts_b, 2, 26),
GROUP(pwm_b, 3, 21),
GROUP(pwm_d, 3, 20),
+ GROUP(i2c_sck_a, 7, 27),
+ GROUP(i2c_sda_a, 7, 26),
+ GROUP(i2c_sck_b, 7, 25),
+ GROUP(i2c_sda_b, 7, 24),
+ GROUP(i2c_sck_c, 7, 23),
+ GROUP(i2c_sda_c, 7, 22),
/* Bank BOOT */
GROUP(emmc_nand_d07, 4, 30),
@@ -574,6 +589,18 @@ static const char * const uart_c_groups[] = {
"uart_tx_c", "uart_rx_c", "uart_cts_c", "uart_rts_c",
};
+static const char * const i2c_a_groups[] = {
+ "i2c_sck_a", "i2c_sda_a",
+};
+
+static const char * const i2c_b_groups[] = {
+ "i2c_sck_b", "i2c_sda_b",
+};
+
+static const char * const i2c_c_groups[] = {
+ "i2c_sck_c", "i2c_sda_c",
+};
+
static const char * const eth_groups[] = {
"eth_mdio", "eth_mdc", "eth_clk_rx_clk", "eth_rx_dv",
"eth_rxd0", "eth_rxd1", "eth_rxd2", "eth_rxd3",
@@ -661,6 +688,9 @@ static struct meson_pmx_func meson_gxbb_periphs_functions[] = {
FUNCTION(uart_a),
FUNCTION(uart_b),
FUNCTION(uart_c),
+ FUNCTION(i2c_a),
+ FUNCTION(i2c_b),
+ FUNCTION(i2c_c),
FUNCTION(eth),
FUNCTION(pwm_a_x),
FUNCTION(pwm_a_y),
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] i2c: meson: add gxbb compatible string
From: Jerome Brunet @ 2016-09-14 9:49 UTC (permalink / raw)
To: linux-arm-kernel
This patchset adds a specific compatible string in the Meson I2C driver
for the Amlogic Meson gxbb SoC.
Other patch series (pinctrl and device tree) are being sent to complete
the i2c support on meson gxbb
Neil Armstrong (2):
i2c: meson: add gxbb compatible string
dt-bindings: i2c-meson: add gxbb compatible string
Documentation/devicetree/bindings/i2c/i2c-meson.txt | 2 +-
drivers/i2c/busses/i2c-meson.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/2] i2c: meson: add gxbb compatible string
From: Jerome Brunet @ 2016-09-14 9:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473846557-18123-1-git-send-email-jbrunet@baylibre.com>
From: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/i2c/busses/i2c-meson.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i2c/busses/i2c-meson.c b/drivers/i2c/busses/i2c-meson.c
index 76e28980904f..30977e31cd98 100644
--- a/drivers/i2c/busses/i2c-meson.c
+++ b/drivers/i2c/busses/i2c-meson.c
@@ -473,6 +473,7 @@ static int meson_i2c_remove(struct platform_device *pdev)
static const struct of_device_id meson_i2c_match[] = {
{ .compatible = "amlogic,meson6-i2c" },
+ { .compatible = "amlogic,meson-gxbb-i2c" },
{ },
};
MODULE_DEVICE_TABLE(of, meson_i2c_match);
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2] dt-bindings: i2c-meson: add gxbb compatible string
From: Jerome Brunet @ 2016-09-14 9:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473846557-18123-1-git-send-email-jbrunet@baylibre.com>
From: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Documentation/devicetree/bindings/i2c/i2c-meson.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-meson.txt b/Documentation/devicetree/bindings/i2c/i2c-meson.txt
index 682f9a6f766e..386357d1aab0 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-meson.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-meson.txt
@@ -1,7 +1,7 @@
Amlogic Meson I2C controller
Required properties:
- - compatible: must be "amlogic,meson6-i2c"
+ - compatible: must be "amlogic,meson6-i2c" or "amlogic,meson-gxbb-i2c"
- reg: physical address and length of the device registers
- interrupts: a single interrupt specifier
- clocks: clock for the device
--
2.7.4
^ permalink raw reply related
* [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts.
From: Kishon Vijay Abraham I @ 2016-09-14 9:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913153402.GA4138@localhost>
Hi,
On Tuesday 13 September 2016 09:04 PM, Bjorn Helgaas wrote:
> [+cc Ley Foon (altera), Thomas (aardvark), Kishon (dra7xx), Murali (keystone)]
>
> On Tue, Sep 13, 2016 at 10:05:11AM -0500, Bjorn Helgaas wrote:
>> On Tue, Sep 13, 2016 at 08:41:28AM +0100, Marc Zyngier wrote:
>>> On 12/09/16 23:02, Bjorn Helgaas wrote:
>>>> On Thu, Sep 01, 2016 at 05:19:55AM +0000, Bharat Kumar Gogada wrote:
>>>>>>>>>> Hi Bharat,
>>>>>>>>>>> @@ -561,7 +561,7 @@ static int nwl_pcie_init_irq_domain(struct
>>>>>>>>>>> nwl_pcie
>>>>>>>>>> *pcie)
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> pcie->legacy_irq_domain = irq_domain_add_linear(legacy_intc_node,
>>>>>>>>>>> - INTX_NUM,
>>>>>>>>>>> + INTX_NUM + 1,
>>>>>>>>>>> &legacy_domain_ops,
>>>>>>>>>>> pcie);
>>>>>>>>>>
>>>>>>>>>> This feels like the wrong thing to do. You have INTX_NUM irqs, so
>>>>>>>>>> the domain allocation should reflect this. On the other hand, the
>>>>>>>>>> way the driver currently deals with mappings is quite broken
>>>>>>>>>> (consistently adding 1 to
>>>>>>>> the HW interrupt).
>>>>>>>>>>
>>>>>>>>> Hi Marc,
>>>>>>>>>
>>>>>>>>> Without above change I get following crash in kernel while booting.
>>>>>>>>>
>>>>>>>>> [ 2.441684] error: hwirq 0x4 is too large for dummy
>>>>>>>>>
>>>>>>>>> [ 2.441694] ------------[ cut here ]------------
>>>>>>>>>
>>>>>>>>> [ 2.441698] WARNING: at kernel/irq/irqdomain.c:344
>>>>>>>>>
>>>>>>>>> [ 2.441702] Modules linked in:
>>>>>>>>>
>>>>>>>>> [ 2.441706]
>>>>>>>>>
>>>>>>>>> [ 2.441714] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0 #8
>>>>>>>>>
>>>>>>>>> [ 2.441718] Hardware name: xlnx,zynqmp (DT)
>>>>>>>>>
>>>>>>>>> [ 2.441723] task: ffffffc071886b80 ti: ffffffc071888000 task.ti:
>>>>>>>> ffffffc071888000
>>>>>>>>>
>>>>>>>>> [ 2.441732] PC is at irq_domain_associate+0x138/0x1c0
>>>>>>>>>
>>>>>>>>> [ 2.441738] LR is at irq_domain_associate+0x138/0x1c0
>>>>>>>>>
>>>>>>>>> In kernel/irq/irqdomain.c function irq_domain_associate
>>>>>>>>>
>>>>>>>>> if (WARN(hwirq >= domain->hwirq_max,
>>>>>>>>> "error: hwirq 0x%x is too large for %s\n", (int)hwirq, domain-
>>>>>>> name))
>>>>>>>>> return -EINVAL;
>>>>>>>>>
>>>>>>>>> Here the hwirq and hwirq_max are equal to 4 without the above
>>>>>>>>> condition
>>>>>>>> (INTX_NUM + 1) due to which crash is coming.
>>>>>>>>> This is happening as the legacy interrupts are starting from 1 (INTA).
>>>>>>>>
>>>>>>>> I understood that. I'm still persisting in saying that you have the wrong fix.
>>>>>>>>
>>>>>>>> Your domain should always allocate many interrupts as you have
>>>>>>>> interrupt sources. These interrupts (hwirq) should be numbered from 0 to (n-
>>>>>> 1).
>>>>>>>
>>>>>>> Agreed, but here comes the problem the hwirq for legacy interrupts
>>>>>>> will start at 0x1 to 0x4 (INTA to INTD) and these values are as per
>>>>>>> PCIe specification for legacy interrupts. So these cannot be numbered
>>>>>>> from 0. So when 0x4 (INTD) for a multi-function device comes the crash
>>>>>>> occurs.
>>>>>>
>>>>>> So who provides this hwirq? Who calls irq_domain_associate() with hwirq set to
>>>>>> 4?
>>>>>>
>>>>> PCIe subsystem invokes pcibios_add_device function in arch/arm64/kernel/pci.c for every pci device.
>>>>> The purpose of this function is to assign dev->irq using of_irq_parse_and_map_pci.
>>>>> of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads PCI_INTERRUPT_PIN from configuration space and saves it
>>>>> in parameter of struct of_phandle_args.
>>>>> This structure is passed to irq_create_of_mapping where it invokes irq_create_fwspec_mapping.
>>>>> irq_create_fwspec_mapping invokes irq_domain_translate and gets hwirq, here the above saved PCI_INTERRUPT_PIN value is assigned
>>>>> to hwirq (*hwirq = fwspec->param[0]).
>>>>> And then using this hwirq irq_create_mapping -> irq_domain_associate were invoked and mapping is created for virtual irq with this hwirq.
>>>>> So for any end point PCI_INTERRUPT_PIN value starts from 0x1 to 0x4 and so hwirq starts from 0x1 to 0x4.
>>>>>
>>>>> So the values are more generic w.r.t to protocol, that's why hwirq will range from 0x1 to 0x4.
>>>>> And then if you check pcie-altera.c they are doing this adding one in their handler and while creating legacy domain.
>>>>
>>>> Is this resolved yet? Marc, are you happy, or should we iterate on this
>>>> again?
>>>
>>> Ah, sorry to have dropped the ball on this patch.
>>
>> No problem, I wasn't making forward progress anyway.
>>
>>> I guess that given that the infrastructure imposes the hwirq range on
>>> the host drivers, Bharat's approach is the only way (and a number of
>>> other host drivers are already slightly broken). I'll try and have a
>>> look at solving this at the generic level. In the meantime:
>>>
>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>
>> After looking at this myself, I'm not happy with this either. It feels
>> like there are bugs lurking here and we're just hiding one of them.
>>
>> Here are the callers of irq_domain_add_linear() for legacy INTx in
>> drivers/pci/host:
>>
>> advk_pcie_init_irq_domain LEGACY_IRQ_NUM (4)
>> dra7xx_pcie_init_irq_domain 4
>> ks_dw_pcie_host_init MAX_LEGACY_IRQS (4)
>> altera_pcie_init_irq_domain INTX_NUM + 1 (5)
>> nwl_pcie_init_irq_domain INTX_NUM + 1 (5)
>> xilinx_pcie_init_irq_domain 4
>
> The altera change corresponding to this was 99496bd2971f ("PCI: altera: Fix
> error when INTx is 4"). I should have noticed this inconsistency back
> then.
>
> Are aardvark, dra7xx, keystone, and xilinx (non-NWL) broken because they
> only request 4 IRQs and only INTA, INTB, and INTC work?
yeah.. it's broken in dra7xx. I get [1] when I configure the pci endpoint to
use INTD.
Thanks
Kishon
[1] -> http://pastebin.ubuntu.com/23177268/
>
>> I think all of these use the of_irq_parse_and_map_pci() path you
>> mentioned, so if the problem is in the way that path works, I would
>> think these should *all* be requesting the same number of interrupts
>> in the domain.
>>
>> I agree with Marc that we should request 4 IRQs, because that's what
>> we need. If we can't do that for some reason, we ought to at least
>> make all these callers the same.
>>
>> Bjorn
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" 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] coresight: tmc: fix for trace collection bug in sysFS mode
From: Suzuki K Poulose @ 2016-09-14 9:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CANLsYkzNqMuwcmYJtoORXrON54Wt1oj5uyp2NQpKYM2AN4F26g@mail.gmail.com>
On 13/09/16 16:41, Mathieu Poirier wrote:
> On 13 September 2016 at 06:20, Venkatesh Vivekanandan
> <venkatesh.vivekanandan@broadcom.com> wrote:
>> tmc_etb_dump_hw is never called in sysFS mode to collect trace from
>> hardware, because drvdata->mode is set to CS_MODE_DISABLED at
>> tmc_disable_etf/etr_sink
>>
>> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>> {
>> .
>> .
>> if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
>> tmc_etb_dump_hw(drvdata);
>> .
>> .
>> }
>>
>> static void tmc_disable_etf_sink(struct coresight_device *csdev)
>> {
>> .
>> .
>> val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
>> /* Disable the TMC only if it needs to */
>> if (val != CS_MODE_DISABLED)
>> tmc_etb_disable_hw(drvdata);
>
> You are correct.
>
>> .
>> .
>> }
>>
>> Signed-off-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>
>> ---
>> drivers/hwtracing/coresight/coresight-tmc-etf.c | 9 +++++----
>> drivers/hwtracing/coresight/coresight-tmc-etr.c | 9 +++++----
>> 2 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index 466af86..c7fb7f7 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -61,6 +61,8 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
>>
>> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>> {
>> + long val;
>> +
>> CS_UNLOCK(drvdata->base);
>>
>> tmc_flush_and_stop(drvdata);
>> @@ -68,7 +70,8 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>> * When operating in sysFS mode the content of the buffer needs to be
>> * read before the TMC is disabled.
>> */
>> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
>> + val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
>> + if (val == CS_MODE_SYSFS)
>> tmc_etb_dump_hw(drvdata);
>> tmc_disable_hw(drvdata);
>>
>> @@ -225,7 +228,6 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
>>
>> static void tmc_disable_etf_sink(struct coresight_device *csdev)
>> {
>> - long val;
>> unsigned long flags;
>> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>
>> @@ -235,9 +237,8 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
>> return;
>> }
>>
>> - val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
>> /* Disable the TMC only if it needs to */
>> - if (val != CS_MODE_DISABLED)
>> + if (local_read(&drvdata->mode) != CS_MODE_DISABLED)
>> tmc_etb_disable_hw(drvdata);
>
> This would work but tmc_enable_etf_sink() and tmc_disable_etf_sink()
> are no longer balanced. Another approach would be to add a "mode"
> parameter to tmc_etb_disable_hw() and so something like:
>
> if (val != CS_MODE_DISABLED)
> tmc_etb_disable_hw(drvdata, val);
>
> In tmc_etb_disable_hw(), if mode == CS_MODE_SYSFS then we can move
> ahead with the dump operation. The same apply for ETR
I think we should :
1) First switch the drvdata->mode to a normal type from local_t. Using an
atomic type for mode is completely unnecessary and comes with the overhead
of barriers/synchronisation instructions, while all accesses, including read/write
are performed under the drvdata->spinlock. I have a patch already for this, which
I plan to send it soon.
and
2) Do something like :
void tmc_disable_etX_sink()
{
if (drvdata->mode != CS_MODE_DISABLED) {
tmc_etX_disable_hw(drvdata);
drvdata->mode = CS_MODE_DISABLED;
}
}
Leaving the tmc_etX_disable_hw() untouched.
Suzuki
^ 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