Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC
From: Eduardo Valentin @ 2017-03-29  4:58 UTC (permalink / raw)
  To: Stefan Wahren
  Cc: Zhang Rui, kernel-TqfNSX0MhmxHKSADF0wUEw, Eric Anholt,
	Rob Herring, Frank Rowand, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1489356665-3175-7-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>

On Sun, Mar 12, 2017 at 10:11:05PM +0000, Stefan Wahren wrote:
> Add basic thermal driver for bcm2835 SoC.
> 
> This driver currently make sure that tsense HW block is set up
> correctly.
> 
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>

I see no issue with this driver at this point.

As I mentioned, the full series needs to go together. You either get an
Ack from OF maintainers, and I take the series, or you can add my

Acked-by: Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

after fixing the small comment from Nobuhiro.

> ---
>  drivers/thermal/Kconfig           |    8 +
>  drivers/thermal/Makefile          |    1 +
>  drivers/thermal/bcm2835_thermal.c |  315 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 324 insertions(+)
>  create mode 100644 drivers/thermal/bcm2835_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 776b343..3bd2406 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -453,4 +453,12 @@ config ZX2967_THERMAL
>  	  the primitive temperature sensor embedded in zx2967 SoCs.
>  	  This sensor generates the real time die temperature.
>  
> +config BCM2835_THERMAL
> +	tristate "Thermal sensors on bcm2835 SoC"
> +	depends on ARCH_BCM2835 || COMPILE_TEST
> +	depends on HAS_IOMEM
> +	depends on THERMAL_OF
> +	help
> +	  Support for thermal sensors on Broadcom bcm2835 SoCs.

small suggestion here:

-If you are sending a new version fixing Nobuhiro comment, you could take
the opportunity to improve the driver Kconfig help description.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 2/3] irqchip: mtk-cirq: Add mediatek mtk-cirq implement
From: Youlin Pei @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Rob Herring, Matthias Brugger, Thomas Gleixner, Jason Cooper,
	Mark Rutland, Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
	hongkun.cao-NuS5LvNUpcJWk0Htik3J/w,
	yong.wu-NuS5LvNUpcJWk0Htik3J/w, erin.lo-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <499f9186-82c1-cb5a-91de-754c89bcdd3d-5wv7dgnIgG8@public.gmane.org>

On Fri, 2017-03-24 at 16:22 +0000, Marc Zyngier wrote:
> On 14/02/17 02:56, Youlin Pei wrote:
> > In Mediatek SOCs, the CIRQ is a low power interrupt controller
> > designed to works outside MCUSYS which comprises with Cortex-Ax
> > cores,CCI and GIC.
> > 
> > The CIRQ controller is integrated in between MCUSYS( include
> > Cortex-Ax, CCI and GIC ) and interrupt sources as the second
> > level interrupt controller. The external interrupts which outside
> > MCUSYS will feed through CIRQ then bypass to GIC. CIRQ can monitors
> > all edge trigger interupts. When an edge interrupt is triggered,
> > CIRQ can record the status and generate a pulse signal to GIC when
> > flush command executed.
> > 
> > When system enters sleep mode, MCUSYS will be turned off to improve
> > power consumption, also GIC is power down. The edge trigger interrupts
> > will be lost in this scenario without CIRQ.
> > 
> > This commit provides the CIRQ irqchip implement.
> > 
> > Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/irqchip/Makefile       |    2 +-
> >  drivers/irqchip/irq-mtk-cirq.c |  288 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 289 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/irqchip/irq-mtk-cirq.c
> > 
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index 0e55d94..db9acd1 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -61,7 +61,7 @@ obj-$(CONFIG_BCM7120_L2_IRQ)		+= irq-bcm7120-l2.o
> >  obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
> >  obj-$(CONFIG_KEYSTONE_IRQ)		+= irq-keystone.o
> >  obj-$(CONFIG_MIPS_GIC)			+= irq-mips-gic.o
> > -obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o
> > +obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o irq-mtk-cirq.o
> >  obj-$(CONFIG_ARCH_DIGICOLOR)		+= irq-digicolor.o
> >  obj-$(CONFIG_RENESAS_H8300H_INTC)	+= irq-renesas-h8300h.o
> >  obj-$(CONFIG_RENESAS_H8S_INTC)		+= irq-renesas-h8s.o
> > diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
> > new file mode 100644
> > index 0000000..b5cf506
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-mtk-cirq.c
> > @@ -0,0 +1,288 @@
> > +/*
> > + * Copyright (c) 2016 MediaTek Inc.
> > + * Author: Youlin.Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/irq.h>
> > +#include <linux/irqchip.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/of.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +#include <linux/syscore_ops.h>
> > +
> > +#define CIRQ_ACK	0x40
> > +#define CIRQ_MASK_SET	0xc0
> > +#define CIRQ_MASK_CLR	0x100
> > +#define CIRQ_SENS_SET	0x180
> > +#define CIRQ_SENS_CLR	0x1c0
> > +#define CIRQ_POL_SET	0x240
> > +#define CIRQ_POL_CLR	0x280
> > +#define CIRQ_CONTROL	0x300
> > +
> > +#define CIRQ_EN	0x1
> > +#define CIRQ_EDGE	0x2
> > +#define CIRQ_FLUSH	0x4
> > +
> > +struct mtk_cirq_chip_data {
> > +	void __iomem *base;
> > +	unsigned int ext_irq_start;
> > +	unsigned int ext_irq_end;
> > +	struct irq_domain *domain;
> > +};
> > +
> > +static struct mtk_cirq_chip_data *cirq_data;
> 
> Question in relation to the sysirq patches that have been queued for
> 4.12: Are we going to see something similar with this driver, where
> you're going to have to support multiple non-contiguous register sets?
> I'd rather have this kind of thing from day one, instead of adding that
> at a later time...
> 
> Specially given that the platform that is using this driver does also
> have a sysirq controller.

Hi Marc,
Thanks for your review.

All of the cirq's registers are in continuous space, So no need to
support multiple non-continuous register sets.

> 
> > +
> > +static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
> > +{
> > +	struct mtk_cirq_chip_data *chip_data = data->chip_data;
> > +	unsigned int cirq_num = data->hwirq;
> > +	u32 mask = 1 << (cirq_num % 32);
> > +
> > +	writel_relaxed(mask, chip_data->base + offset + (cirq_num / 32) * 4);
> > +}
> > +
> > +static void mtk_cirq_mask(struct irq_data *data)
> > +{
> > +	mtk_cirq_write_mask(data, CIRQ_MASK_SET);
> > +	irq_chip_mask_parent(data);
> > +}
> > +
> > +static void mtk_cirq_unmask(struct irq_data *data)
> > +{
> > +	mtk_cirq_write_mask(data, CIRQ_MASK_CLR);
> > +	irq_chip_unmask_parent(data);
> > +}
> > +
> > +static int mtk_cirq_set_type(struct irq_data *data, unsigned int type)
> > +{
> > +	int ret;
> > +
> > +	switch (type & IRQ_TYPE_SENSE_MASK) {
> > +	case IRQ_TYPE_EDGE_FALLING:
> > +		mtk_cirq_write_mask(data, CIRQ_POL_CLR);
> > +		mtk_cirq_write_mask(data, CIRQ_SENS_CLR);
> > +		break;
> > +	case IRQ_TYPE_EDGE_RISING:
> > +		mtk_cirq_write_mask(data, CIRQ_POL_SET);
> > +		mtk_cirq_write_mask(data, CIRQ_SENS_CLR);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_LOW:
> > +		mtk_cirq_write_mask(data, CIRQ_POL_CLR);
> > +		mtk_cirq_write_mask(data, CIRQ_SENS_SET);
> > +		break;
> > +	case IRQ_TYPE_LEVEL_HIGH:
> > +		mtk_cirq_write_mask(data, CIRQ_POL_SET);
> > +		mtk_cirq_write_mask(data, CIRQ_SENS_SET);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	data = data->parent_data;
> > +	ret = data->chip->irq_set_type(data, type);
> > +	return ret;
> > +}
> > +
> > +static struct irq_chip mtk_cirq_chip = {
> > +	.name			= "MT_CIRQ",
> > +	.irq_mask		= mtk_cirq_mask,
> > +	.irq_unmask		= mtk_cirq_unmask,
> > +	.irq_eoi		= irq_chip_eoi_parent,
> > +	.irq_set_type		= mtk_cirq_set_type,
> > +	.irq_retrigger		= irq_chip_retrigger_hierarchy,
> > +#ifdef CONFIG_SMP
> > +	.irq_set_affinity	= irq_chip_set_affinity_parent,
> > +#endif
> > +};
> > +
> > +static int mtk_cirq_domain_translate(struct irq_domain *d,
> > +				     struct irq_fwspec *fwspec,
> > +				     unsigned long *hwirq,
> > +				     unsigned int *type)
> > +{
> > +	if (is_of_node(fwspec->fwnode)) {
> > +		if (fwspec->param_count != 3)
> > +			return -EINVAL;
> > +
> > +		/* No PPI should point to this domain */
> > +		if (fwspec->param[0] != 0)
> > +			return -EINVAL;
> > +
> > +		/* cirq support irq number check */
> > +		if (fwspec->param[1] < cirq_data->ext_irq_start ||
> > +		    fwspec->param[1] > cirq_data->ext_irq_end)
> > +			return -EINVAL;
> > +
> > +		*hwirq = fwspec->param[1] - cirq_data->ext_irq_start;
> > +		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static int mtk_cirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> > +				 unsigned int nr_irqs, void *arg)
> > +{
> > +	int ret;
> > +	irq_hw_number_t hwirq;
> > +	unsigned int type;
> > +	struct irq_fwspec *fwspec = arg;
> > +	struct irq_fwspec parent_fwspec = *fwspec;
> > +
> > +	ret = mtk_cirq_domain_translate(domain, fwspec, &hwirq, &type);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (WARN_ON(nr_irqs != 1))
> > +		return -EINVAL;
> > +
> > +	irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> > +				      &mtk_cirq_chip,
> > +				      domain->host_data);
> > +
> > +	parent_fwspec.fwnode = domain->parent->fwnode;
> > +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> > +					    &parent_fwspec);
> > +}
> > +
> > +static const struct irq_domain_ops cirq_domain_ops = {
> > +	.translate	= mtk_cirq_domain_translate,
> > +	.alloc		= mtk_cirq_domain_alloc,
> > +	.free		= irq_domain_free_irqs_common,
> > +};
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int mtk_cirq_suspend(void)
> > +{
> > +	u32 value, mask;
> > +	unsigned int irq, hwirq_num;
> > +	bool pending, masked;
> > +	int i, pendret, maskret;
> > +
> > +	hwirq_num = cirq_data->ext_irq_end - cirq_data->ext_irq_start + 1;
> > +	for (i = 0; i < hwirq_num; i++) {
> > +		irq = irq_find_mapping(cirq_data->domain, i);
> > +		if (irq) {
> > +			pendret = irq_get_irqchip_state(irq,
> > +							IRQCHIP_STATE_PENDING,
> > +							&pending);
> > +
> > +			maskret = irq_get_irqchip_state(irq,
> > +							IRQCHIP_STATE_MASKED,
> > +							&masked);
> > +
> > +			if (pendret == 0 && maskret == 0 &&
> > +			    (pending && !masked))
> > +				continue;
> > +		}
> > +
> > +		mask = 1 << (i % 32);
> > +		writel_relaxed(mask, cirq_data->base + CIRQ_ACK + (i / 32) * 4);
> > +	}
> 
> It would be worth documenting exactly what this is for. I had to go back
> to our previous discussion in November to remember it.

Okay, i will add some comments to explain what this is for in next
version. Thanks!

> 
> > +
> > +	/* set edge_only mode, record edge-triggerd interrupts */
> > +	/* enable cirq */
> > +	value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> > +	value |= (CIRQ_EDGE | CIRQ_EN);
> > +	writel_relaxed(value, cirq_data->base + CIRQ_CONTROL);
> > +
> > +	return 0;
> > +}
> > +
> > +static void mtk_cirq_resume(void)
> > +{
> > +	u32 value;
> > +
> > +	/* flush recored interrupts, will send signals to parent controller */
> > +	value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> > +	writel_relaxed(value | CIRQ_FLUSH, cirq_data->base + CIRQ_CONTROL);
> > +
> > +	/* disable cirq */
> > +	value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
> > +	value &= ~(CIRQ_EDGE | CIRQ_EN);
> > +	writel_relaxed(value, cirq_data->base + CIRQ_CONTROL);
> > +}
> > +
> > +static struct syscore_ops mtk_cirq_syscore_ops = {
> > +	.suspend	= mtk_cirq_suspend,
> > +	.resume		= mtk_cirq_resume,
> > +};
> > +
> > +static void mtk_cirq_syscore_init(void)
> > +{
> > +	register_syscore_ops(&mtk_cirq_syscore_ops);
> > +}
> > +#else
> > +static inline void mtk_cirq_syscore_init(void) {}
> > +#endif
> > +
> > +static int __init mtk_cirq_of_init(struct device_node *node,
> > +				   struct device_node *parent)
> > +{
> > +	struct irq_domain *domain, *domain_parent;
> > +	unsigned int irq_num;
> > +	int ret;
> > +
> > +	domain_parent = irq_find_host(parent);
> > +	if (!domain_parent) {
> > +		pr_err("mtk_cirq: interrupt-parent not found\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	cirq_data = kzalloc(sizeof(*cirq_data), GFP_KERNEL);
> > +	if (!cirq_data)
> > +		return -ENOMEM;
> > +
> > +	cirq_data->base = of_iomap(node, 0);
> > +	if (!cirq_data->base) {
> > +		pr_err("mtk_cirq: unable to map cirq register\n");
> > +		ret = -ENXIO;
> > +		goto out_free;
> > +	}
> > +
> > +	ret = of_property_read_u32_index(node, "mediatek,ext-irq-range", 0,
> > +					 &cirq_data->ext_irq_start);
> > +	if (ret)
> > +		goto out_unmap;
> > +
> > +	ret = of_property_read_u32_index(node, "mediatek,ext-irq-range", 1,
> > +					 &cirq_data->ext_irq_end);
> > +	if (ret)
> > +		goto out_unmap;
> > +
> > +	irq_num = cirq_data->ext_irq_end - cirq_data->ext_irq_start + 1;
> > +	domain = irq_domain_add_hierarchy(domain_parent, 0,
> > +					  irq_num, node,
> > +					  &cirq_domain_ops, cirq_data);
> > +	if (!domain) {
> > +		ret = -ENOMEM;
> > +		goto out_unmap;
> > +	}
> > +	cirq_data->domain = domain;
> > +
> > +	mtk_cirq_syscore_init();
> > +
> > +	return 0;
> > +
> > +out_unmap:
> > +	iounmap(cirq_data->base);
> > +out_free:
> > +	kfree(cirq_data);
> > +	return ret;
> > +}
> > +
> > +IRQCHIP_DECLARE(mtk_cirq, "mediatek,mtk-cirq", mtk_cirq_of_init);
> > 
> 
> It otherwise looks much better than the previous iteration.
> 
> Thanks,
> 
> 	M.


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v7 0/3] Broadcom FlexRM ring manager support
From: Anup Patel @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Jassi Brar, Rob Herring
  Cc: Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR, Rob Rice,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Anup Patel

The Broadcom FlexRM ring manager provides producer-consumer style
ring interface for offload engines on Broadcom iProc SoCs. We can
have one or more instances of Broadcom FlexRM ring manager in a SoC.

This patchset adds a mailbox driver for Broadcom FlexRM ring manager
which can be used by offload engine drivers as mailbox clients.

The Broadcom FlexRM mailbox driver is feature complete for RAID and
Crypto offload engines. We will have incremental patches in-future
for ring-level statistics using debugfs and minor optimizations.

This patchset is based on Linux-4.11-rc4 and it is also available
at flexrm-v7 branch of https://github.com/Broadcom/arm64-linux.git

Changes since v6:
 - Rebased patches for Linux-4.11-rc4
 - Added a patch to remove depends on COMPILE_TEST for kconfig
   option BCM_FLEXRM_MBOX

Changes since v5:
 - Rebased patches for Linux-4.11-rc2
 - Merged all source files into one source file and removed
   driver directory

Changes since v4:
 - Rebased patches for Linux-4.11-rc1

Changes since v3:
 - Fixed mailbox client example DT node in DT bindings document

Changes since v2:
 - Rebased patches for Linux-4.10-rc2

Changes since v1:
 - Use compatile string as brcm,iproc-flexrm-mbox
 - Rephrase commit message and text in DT bindings patch

Anup Patel (3):
  mailbox: Add driver for Broadcom FlexRM ring manager
  dt-bindings: Add DT bindings info for FlexRM ring manager
  mailbox: Remove depends on COMPILE_TEST for BCM_FLEXRM_MBOX

 .../bindings/mailbox/brcm,iproc-flexrm-mbox.txt    |   59 +
 drivers/mailbox/Kconfig                            |   11 +
 drivers/mailbox/Makefile                           |    2 +
 drivers/mailbox/bcm-flexrm-mailbox.c               | 1595 ++++++++++++++++++++
 include/linux/mailbox/brcm-message.h               |   14 +-
 5 files changed, 1677 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
 create mode 100644 drivers/mailbox/bcm-flexrm-mailbox.c

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v7 1/3] mailbox: Add driver for Broadcom FlexRM ring manager
From: Anup Patel @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Jassi Brar, Rob Herring
  Cc: Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR, Rob Rice,
	devicetree, linux-kernel, linux-arm-kernel,
	bcm-kernel-feedback-list, Anup Patel
In-Reply-To: <1490765455-8556-1-git-send-email-anup.patel@broadcom.com>

Some of the Broadcom iProc SoCs have FlexRM ring manager
which provides a ring-based programming interface to various
offload engines (e.g. RAID, Crypto, etc).

This patch adds a common mailbox driver for Broadcom FlexRM
ring manager which can be shared by various offload engine
drivers (implemented as mailbox clients).

Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Pramod KUMAR <pramod.kumar@broadcom.com>
Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/mailbox/Kconfig              |   11 +
 drivers/mailbox/Makefile             |    2 +
 drivers/mailbox/bcm-flexrm-mailbox.c | 1595 ++++++++++++++++++++++++++++++++++
 include/linux/mailbox/brcm-message.h |   14 +-
 4 files changed, 1618 insertions(+), 4 deletions(-)
 create mode 100644 drivers/mailbox/bcm-flexrm-mailbox.c

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ceff415..305018c 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -152,4 +152,15 @@ config BCM_PDC_MBOX
 	  Mailbox implementation for the Broadcom PDC ring manager,
 	  which provides access to various offload engines on Broadcom
 	  SoCs. Say Y here if you want to use the Broadcom PDC.
+
+config BCM_FLEXRM_MBOX
+	tristate "Broadcom FlexRM Mailbox"
+	depends on ARM64 || COMPILE_TEST
+	depends on HAS_DMA
+	select GENERIC_MSI_IRQ_DOMAIN
+	default ARCH_BCM_IPROC
+	help
+	  Mailbox implementation of the Broadcom FlexRM ring manager,
+	  which provides access to various offload engines on Broadcom
+	  SoCs. Say Y here if you want to use the Broadcom FlexRM.
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 7dde4f6..e2bcb03 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -30,4 +30,6 @@ obj-$(CONFIG_HI6220_MBOX)	+= hi6220-mailbox.o
 
 obj-$(CONFIG_BCM_PDC_MBOX)	+= bcm-pdc-mailbox.o
 
+obj-$(CONFIG_BCM_FLEXRM_MBOX)	+= bcm-flexrm-mailbox.o
+
 obj-$(CONFIG_TEGRA_HSP_MBOX)	+= tegra-hsp.o
diff --git a/drivers/mailbox/bcm-flexrm-mailbox.c b/drivers/mailbox/bcm-flexrm-mailbox.c
new file mode 100644
index 0000000..da67882
--- /dev/null
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -0,0 +1,1595 @@
+/* Broadcom FlexRM Mailbox Driver
+ *
+ * Copyright (C) 2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Each Broadcom FlexSparx4 offload engine is implemented as an
+ * extension to Broadcom FlexRM ring manager. The FlexRM ring
+ * manager provides a set of rings which can be used to submit
+ * work to a FlexSparx4 offload engine.
+ *
+ * This driver creates a mailbox controller using a set of FlexRM
+ * rings where each mailbox channel represents a separate FlexRM ring.
+ */
+
+#include <asm/barrier.h>
+#include <asm/byteorder.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+#include <linux/err.h>
+#include <linux/idr.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox_client.h>
+#include <linux/mailbox/brcm-message.h>
+#include <linux/module.h>
+#include <linux/msi.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+/* ====== FlexRM register defines ===== */
+
+/* FlexRM configuration */
+#define RING_REGS_SIZE					0x10000
+#define RING_DESC_SIZE					8
+#define RING_DESC_INDEX(offset)				\
+			((offset) / RING_DESC_SIZE)
+#define RING_DESC_OFFSET(index)				\
+			((index) * RING_DESC_SIZE)
+#define RING_MAX_REQ_COUNT				1024
+#define RING_BD_ALIGN_ORDER				12
+#define RING_BD_ALIGN_CHECK(addr)			\
+			(!((addr) & ((0x1 << RING_BD_ALIGN_ORDER) - 1)))
+#define RING_BD_TOGGLE_INVALID(offset)			\
+			(((offset) >> RING_BD_ALIGN_ORDER) & 0x1)
+#define RING_BD_TOGGLE_VALID(offset)			\
+			(!RING_BD_TOGGLE_INVALID(offset))
+#define RING_BD_DESC_PER_REQ				32
+#define RING_BD_DESC_COUNT				\
+			(RING_MAX_REQ_COUNT * RING_BD_DESC_PER_REQ)
+#define RING_BD_SIZE					\
+			(RING_BD_DESC_COUNT * RING_DESC_SIZE)
+#define RING_CMPL_ALIGN_ORDER				13
+#define RING_CMPL_DESC_COUNT				RING_MAX_REQ_COUNT
+#define RING_CMPL_SIZE					\
+			(RING_CMPL_DESC_COUNT * RING_DESC_SIZE)
+#define RING_VER_MAGIC					0x76303031
+
+/* Per-Ring register offsets */
+#define RING_VER					0x000
+#define RING_BD_START_ADDR				0x004
+#define RING_BD_READ_PTR				0x008
+#define RING_BD_WRITE_PTR				0x00c
+#define RING_BD_READ_PTR_DDR_LS				0x010
+#define RING_BD_READ_PTR_DDR_MS				0x014
+#define RING_CMPL_START_ADDR				0x018
+#define RING_CMPL_WRITE_PTR				0x01c
+#define RING_NUM_REQ_RECV_LS				0x020
+#define RING_NUM_REQ_RECV_MS				0x024
+#define RING_NUM_REQ_TRANS_LS				0x028
+#define RING_NUM_REQ_TRANS_MS				0x02c
+#define RING_NUM_REQ_OUTSTAND				0x030
+#define RING_CONTROL					0x034
+#define RING_FLUSH_DONE					0x038
+#define RING_MSI_ADDR_LS				0x03c
+#define RING_MSI_ADDR_MS				0x040
+#define RING_MSI_CONTROL				0x048
+#define RING_BD_READ_PTR_DDR_CONTROL			0x04c
+#define RING_MSI_DATA_VALUE				0x064
+
+/* Register RING_BD_START_ADDR fields */
+#define BD_LAST_UPDATE_HW_SHIFT				28
+#define BD_LAST_UPDATE_HW_MASK				0x1
+#define BD_START_ADDR_VALUE(pa)				\
+	((u32)((((dma_addr_t)(pa)) >> RING_BD_ALIGN_ORDER) & 0x0fffffff))
+#define BD_START_ADDR_DECODE(val)			\
+	((dma_addr_t)((val) & 0x0fffffff) << RING_BD_ALIGN_ORDER)
+
+/* Register RING_CMPL_START_ADDR fields */
+#define CMPL_START_ADDR_VALUE(pa)			\
+	((u32)((((u64)(pa)) >> RING_CMPL_ALIGN_ORDER) & 0x03ffffff))
+
+/* Register RING_CONTROL fields */
+#define CONTROL_MASK_DISABLE_CONTROL			12
+#define CONTROL_FLUSH_SHIFT				5
+#define CONTROL_ACTIVE_SHIFT				4
+#define CONTROL_RATE_ADAPT_MASK				0xf
+#define CONTROL_RATE_DYNAMIC				0x0
+#define CONTROL_RATE_FAST				0x8
+#define CONTROL_RATE_MEDIUM				0x9
+#define CONTROL_RATE_SLOW				0xa
+#define CONTROL_RATE_IDLE				0xb
+
+/* Register RING_FLUSH_DONE fields */
+#define FLUSH_DONE_MASK					0x1
+
+/* Register RING_MSI_CONTROL fields */
+#define MSI_TIMER_VAL_SHIFT				16
+#define MSI_TIMER_VAL_MASK				0xffff
+#define MSI_ENABLE_SHIFT				15
+#define MSI_ENABLE_MASK					0x1
+#define MSI_COUNT_SHIFT					0
+#define MSI_COUNT_MASK					0x3ff
+
+/* Register RING_BD_READ_PTR_DDR_CONTROL fields */
+#define BD_READ_PTR_DDR_TIMER_VAL_SHIFT			16
+#define BD_READ_PTR_DDR_TIMER_VAL_MASK			0xffff
+#define BD_READ_PTR_DDR_ENABLE_SHIFT			15
+#define BD_READ_PTR_DDR_ENABLE_MASK			0x1
+
+/* ====== FlexRM ring descriptor defines ===== */
+
+/* Completion descriptor format */
+#define CMPL_OPAQUE_SHIFT			0
+#define CMPL_OPAQUE_MASK			0xffff
+#define CMPL_ENGINE_STATUS_SHIFT		16
+#define CMPL_ENGINE_STATUS_MASK			0xffff
+#define CMPL_DME_STATUS_SHIFT			32
+#define CMPL_DME_STATUS_MASK			0xffff
+#define CMPL_RM_STATUS_SHIFT			48
+#define CMPL_RM_STATUS_MASK			0xffff
+
+/* Completion DME status code */
+#define DME_STATUS_MEM_COR_ERR			BIT(0)
+#define DME_STATUS_MEM_UCOR_ERR			BIT(1)
+#define DME_STATUS_FIFO_UNDERFLOW		BIT(2)
+#define DME_STATUS_FIFO_OVERFLOW		BIT(3)
+#define DME_STATUS_RRESP_ERR			BIT(4)
+#define DME_STATUS_BRESP_ERR			BIT(5)
+#define DME_STATUS_ERROR_MASK			(DME_STATUS_MEM_COR_ERR | \
+						 DME_STATUS_MEM_UCOR_ERR | \
+						 DME_STATUS_FIFO_UNDERFLOW | \
+						 DME_STATUS_FIFO_OVERFLOW | \
+						 DME_STATUS_RRESP_ERR | \
+						 DME_STATUS_BRESP_ERR)
+
+/* Completion RM status code */
+#define RM_STATUS_CODE_SHIFT			0
+#define RM_STATUS_CODE_MASK			0x3ff
+#define RM_STATUS_CODE_GOOD			0x0
+#define RM_STATUS_CODE_AE_TIMEOUT		0x3ff
+
+/* General descriptor format */
+#define DESC_TYPE_SHIFT				60
+#define DESC_TYPE_MASK				0xf
+#define DESC_PAYLOAD_SHIFT			0
+#define DESC_PAYLOAD_MASK			0x0fffffffffffffff
+
+/* Null descriptor format  */
+#define NULL_TYPE				0
+#define NULL_TOGGLE_SHIFT			58
+#define NULL_TOGGLE_MASK			0x1
+
+/* Header descriptor format */
+#define HEADER_TYPE				1
+#define HEADER_TOGGLE_SHIFT			58
+#define HEADER_TOGGLE_MASK			0x1
+#define HEADER_ENDPKT_SHIFT			57
+#define HEADER_ENDPKT_MASK			0x1
+#define HEADER_STARTPKT_SHIFT			56
+#define HEADER_STARTPKT_MASK			0x1
+#define HEADER_BDCOUNT_SHIFT			36
+#define HEADER_BDCOUNT_MASK			0x1f
+#define HEADER_BDCOUNT_MAX			HEADER_BDCOUNT_MASK
+#define HEADER_FLAGS_SHIFT			16
+#define HEADER_FLAGS_MASK			0xffff
+#define HEADER_OPAQUE_SHIFT			0
+#define HEADER_OPAQUE_MASK			0xffff
+
+/* Source (SRC) descriptor format */
+#define SRC_TYPE				2
+#define SRC_LENGTH_SHIFT			44
+#define SRC_LENGTH_MASK				0xffff
+#define SRC_ADDR_SHIFT				0
+#define SRC_ADDR_MASK				0x00000fffffffffff
+
+/* Destination (DST) descriptor format */
+#define DST_TYPE				3
+#define DST_LENGTH_SHIFT			44
+#define DST_LENGTH_MASK				0xffff
+#define DST_ADDR_SHIFT				0
+#define DST_ADDR_MASK				0x00000fffffffffff
+
+/* Immediate (IMM) descriptor format */
+#define IMM_TYPE				4
+#define IMM_DATA_SHIFT				0
+#define IMM_DATA_MASK				0x0fffffffffffffff
+
+/* Next pointer (NPTR) descriptor format */
+#define NPTR_TYPE				5
+#define NPTR_TOGGLE_SHIFT			58
+#define NPTR_TOGGLE_MASK			0x1
+#define NPTR_ADDR_SHIFT				0
+#define NPTR_ADDR_MASK				0x00000fffffffffff
+
+/* Mega source (MSRC) descriptor format */
+#define MSRC_TYPE				6
+#define MSRC_LENGTH_SHIFT			44
+#define MSRC_LENGTH_MASK			0xffff
+#define MSRC_ADDR_SHIFT				0
+#define MSRC_ADDR_MASK				0x00000fffffffffff
+
+/* Mega destination (MDST) descriptor format */
+#define MDST_TYPE				7
+#define MDST_LENGTH_SHIFT			44
+#define MDST_LENGTH_MASK			0xffff
+#define MDST_ADDR_SHIFT				0
+#define MDST_ADDR_MASK				0x00000fffffffffff
+
+/* Source with tlast (SRCT) descriptor format */
+#define SRCT_TYPE				8
+#define SRCT_LENGTH_SHIFT			44
+#define SRCT_LENGTH_MASK			0xffff
+#define SRCT_ADDR_SHIFT				0
+#define SRCT_ADDR_MASK				0x00000fffffffffff
+
+/* Destination with tlast (DSTT) descriptor format */
+#define DSTT_TYPE				9
+#define DSTT_LENGTH_SHIFT			44
+#define DSTT_LENGTH_MASK			0xffff
+#define DSTT_ADDR_SHIFT				0
+#define DSTT_ADDR_MASK				0x00000fffffffffff
+
+/* Immediate with tlast (IMMT) descriptor format */
+#define IMMT_TYPE				10
+#define IMMT_DATA_SHIFT				0
+#define IMMT_DATA_MASK				0x0fffffffffffffff
+
+/* Descriptor helper macros */
+#define DESC_DEC(_d, _s, _m)			(((_d) >> (_s)) & (_m))
+#define DESC_ENC(_d, _v, _s, _m)		\
+			do { \
+				(_d) &= ~((u64)(_m) << (_s)); \
+				(_d) |= (((u64)(_v) & (_m)) << (_s)); \
+			} while (0)
+
+/* ====== FlexRM data structures ===== */
+
+struct flexrm_ring {
+	/* Unprotected members */
+	int num;
+	struct flexrm_mbox *mbox;
+	void __iomem *regs;
+	bool irq_requested;
+	unsigned int irq;
+	unsigned int msi_timer_val;
+	unsigned int msi_count_threshold;
+	struct ida requests_ida;
+	struct brcm_message *requests[RING_MAX_REQ_COUNT];
+	void *bd_base;
+	dma_addr_t bd_dma_base;
+	u32 bd_write_offset;
+	void *cmpl_base;
+	dma_addr_t cmpl_dma_base;
+	/* Protected members */
+	spinlock_t lock;
+	struct brcm_message *last_pending_msg;
+	u32 cmpl_read_offset;
+};
+
+struct flexrm_mbox {
+	struct device *dev;
+	void __iomem *regs;
+	u32 num_rings;
+	struct flexrm_ring *rings;
+	struct dma_pool *bd_pool;
+	struct dma_pool *cmpl_pool;
+	struct mbox_controller controller;
+};
+
+/* ====== FlexRM ring descriptor helper routines ===== */
+
+static u64 flexrm_read_desc(void *desc_ptr)
+{
+	return le64_to_cpu(*((u64 *)desc_ptr));
+}
+
+static void flexrm_write_desc(void *desc_ptr, u64 desc)
+{
+	*((u64 *)desc_ptr) = cpu_to_le64(desc);
+}
+
+static u32 flexrm_cmpl_desc_to_reqid(u64 cmpl_desc)
+{
+	return (u32)(cmpl_desc & CMPL_OPAQUE_MASK);
+}
+
+static int flexrm_cmpl_desc_to_error(u64 cmpl_desc)
+{
+	u32 status;
+
+	status = DESC_DEC(cmpl_desc, CMPL_DME_STATUS_SHIFT,
+			  CMPL_DME_STATUS_MASK);
+	if (status & DME_STATUS_ERROR_MASK)
+		return -EIO;
+
+	status = DESC_DEC(cmpl_desc, CMPL_RM_STATUS_SHIFT,
+			  CMPL_RM_STATUS_MASK);
+	status &= RM_STATUS_CODE_MASK;
+	if (status == RM_STATUS_CODE_AE_TIMEOUT)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static bool flexrm_is_next_table_desc(void *desc_ptr)
+{
+	u64 desc = flexrm_read_desc(desc_ptr);
+	u32 type = DESC_DEC(desc, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+
+	return (type == NPTR_TYPE) ? true : false;
+}
+
+static u64 flexrm_next_table_desc(u32 toggle, dma_addr_t next_addr)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, NPTR_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, toggle, NPTR_TOGGLE_SHIFT, NPTR_TOGGLE_MASK);
+	DESC_ENC(desc, next_addr, NPTR_ADDR_SHIFT, NPTR_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_null_desc(u32 toggle)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, NULL_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, toggle, NULL_TOGGLE_SHIFT, NULL_TOGGLE_MASK);
+
+	return desc;
+}
+
+static u32 flexrm_estimate_header_desc_count(u32 nhcnt)
+{
+	u32 hcnt = nhcnt / HEADER_BDCOUNT_MAX;
+
+	if (!(nhcnt % HEADER_BDCOUNT_MAX))
+		hcnt += 1;
+
+	return hcnt;
+}
+
+static void flexrm_flip_header_toogle(void *desc_ptr)
+{
+	u64 desc = flexrm_read_desc(desc_ptr);
+
+	if (desc & ((u64)0x1 << HEADER_TOGGLE_SHIFT))
+		desc &= ~((u64)0x1 << HEADER_TOGGLE_SHIFT);
+	else
+		desc |= ((u64)0x1 << HEADER_TOGGLE_SHIFT);
+
+	flexrm_write_desc(desc_ptr, desc);
+}
+
+static u64 flexrm_header_desc(u32 toggle, u32 startpkt, u32 endpkt,
+			       u32 bdcount, u32 flags, u32 opaque)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, HEADER_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, toggle, HEADER_TOGGLE_SHIFT, HEADER_TOGGLE_MASK);
+	DESC_ENC(desc, startpkt, HEADER_STARTPKT_SHIFT, HEADER_STARTPKT_MASK);
+	DESC_ENC(desc, endpkt, HEADER_ENDPKT_SHIFT, HEADER_ENDPKT_MASK);
+	DESC_ENC(desc, bdcount, HEADER_BDCOUNT_SHIFT, HEADER_BDCOUNT_MASK);
+	DESC_ENC(desc, flags, HEADER_FLAGS_SHIFT, HEADER_FLAGS_MASK);
+	DESC_ENC(desc, opaque, HEADER_OPAQUE_SHIFT, HEADER_OPAQUE_MASK);
+
+	return desc;
+}
+
+static void flexrm_enqueue_desc(u32 nhpos, u32 nhcnt, u32 reqid,
+				 u64 desc, void **desc_ptr, u32 *toggle,
+				 void *start_desc, void *end_desc)
+{
+	u64 d;
+	u32 nhavail, _toggle, _startpkt, _endpkt, _bdcount;
+
+	/* Sanity check */
+	if (nhcnt <= nhpos)
+		return;
+
+	/*
+	 * Each request or packet start with a HEADER descriptor followed
+	 * by one or more non-HEADER descriptors (SRC, SRCT, MSRC, DST,
+	 * DSTT, MDST, IMM, and IMMT). The number of non-HEADER descriptors
+	 * following a HEADER descriptor is represented by BDCOUNT field
+	 * of HEADER descriptor. The max value of BDCOUNT field is 31 which
+	 * means we can only have 31 non-HEADER descriptors following one
+	 * HEADER descriptor.
+	 *
+	 * In general use, number of non-HEADER descriptors can easily go
+	 * beyond 31. To tackle this situation, we have packet (or request)
+	 * extenstion bits (STARTPKT and ENDPKT) in the HEADER descriptor.
+	 *
+	 * To use packet extension, the first HEADER descriptor of request
+	 * (or packet) will have STARTPKT=1 and ENDPKT=0. The intermediate
+	 * HEADER descriptors will have STARTPKT=0 and ENDPKT=0. The last
+	 * HEADER descriptor will have STARTPKT=0 and ENDPKT=1. Also, the
+	 * TOGGLE bit of the first HEADER will be set to invalid state to
+	 * ensure that FlexRM does not start fetching descriptors till all
+	 * descriptors are enqueued. The user of this function will flip
+	 * the TOGGLE bit of first HEADER after all descriptors are
+	 * enqueued.
+	 */
+
+	if ((nhpos % HEADER_BDCOUNT_MAX == 0) && (nhcnt - nhpos)) {
+		/* Prepare the header descriptor */
+		nhavail = (nhcnt - nhpos);
+		_toggle = (nhpos == 0) ? !(*toggle) : (*toggle);
+		_startpkt = (nhpos == 0) ? 0x1 : 0x0;
+		_endpkt = (nhavail <= HEADER_BDCOUNT_MAX) ? 0x1 : 0x0;
+		_bdcount = (nhavail <= HEADER_BDCOUNT_MAX) ?
+				nhavail : HEADER_BDCOUNT_MAX;
+		if (nhavail <= HEADER_BDCOUNT_MAX)
+			_bdcount = nhavail;
+		else
+			_bdcount = HEADER_BDCOUNT_MAX;
+		d = flexrm_header_desc(_toggle, _startpkt, _endpkt,
+					_bdcount, 0x0, reqid);
+
+		/* Write header descriptor */
+		flexrm_write_desc(*desc_ptr, d);
+
+		/* Point to next descriptor */
+		*desc_ptr += sizeof(desc);
+		if (*desc_ptr == end_desc)
+			*desc_ptr = start_desc;
+
+		/* Skip next pointer descriptors */
+		while (flexrm_is_next_table_desc(*desc_ptr)) {
+			*toggle = (*toggle) ? 0 : 1;
+			*desc_ptr += sizeof(desc);
+			if (*desc_ptr == end_desc)
+				*desc_ptr = start_desc;
+		}
+	}
+
+	/* Write desired descriptor */
+	flexrm_write_desc(*desc_ptr, desc);
+
+	/* Point to next descriptor */
+	*desc_ptr += sizeof(desc);
+	if (*desc_ptr == end_desc)
+		*desc_ptr = start_desc;
+
+	/* Skip next pointer descriptors */
+	while (flexrm_is_next_table_desc(*desc_ptr)) {
+		*toggle = (*toggle) ? 0 : 1;
+		*desc_ptr += sizeof(desc);
+		if (*desc_ptr == end_desc)
+			*desc_ptr = start_desc;
+	}
+}
+
+static u64 flexrm_src_desc(dma_addr_t addr, unsigned int length)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, SRC_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length, SRC_LENGTH_SHIFT, SRC_LENGTH_MASK);
+	DESC_ENC(desc, addr, SRC_ADDR_SHIFT, SRC_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_msrc_desc(dma_addr_t addr, unsigned int length_div_16)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, MSRC_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length_div_16, MSRC_LENGTH_SHIFT, MSRC_LENGTH_MASK);
+	DESC_ENC(desc, addr, MSRC_ADDR_SHIFT, MSRC_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_dst_desc(dma_addr_t addr, unsigned int length)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, DST_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length, DST_LENGTH_SHIFT, DST_LENGTH_MASK);
+	DESC_ENC(desc, addr, DST_ADDR_SHIFT, DST_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_mdst_desc(dma_addr_t addr, unsigned int length_div_16)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, MDST_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length_div_16, MDST_LENGTH_SHIFT, MDST_LENGTH_MASK);
+	DESC_ENC(desc, addr, MDST_ADDR_SHIFT, MDST_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_imm_desc(u64 data)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, IMM_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, data, IMM_DATA_SHIFT, IMM_DATA_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_srct_desc(dma_addr_t addr, unsigned int length)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, SRCT_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length, SRCT_LENGTH_SHIFT, SRCT_LENGTH_MASK);
+	DESC_ENC(desc, addr, SRCT_ADDR_SHIFT, SRCT_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_dstt_desc(dma_addr_t addr, unsigned int length)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, DSTT_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, length, DSTT_LENGTH_SHIFT, DSTT_LENGTH_MASK);
+	DESC_ENC(desc, addr, DSTT_ADDR_SHIFT, DSTT_ADDR_MASK);
+
+	return desc;
+}
+
+static u64 flexrm_immt_desc(u64 data)
+{
+	u64 desc = 0;
+
+	DESC_ENC(desc, IMMT_TYPE, DESC_TYPE_SHIFT, DESC_TYPE_MASK);
+	DESC_ENC(desc, data, IMMT_DATA_SHIFT, IMMT_DATA_MASK);
+
+	return desc;
+}
+
+static bool flexrm_spu_sanity_check(struct brcm_message *msg)
+{
+	struct scatterlist *sg;
+
+	if (!msg->spu.src || !msg->spu.dst)
+		return false;
+	for (sg = msg->spu.src; sg; sg = sg_next(sg)) {
+		if (sg->length & 0xf) {
+			if (sg->length > SRC_LENGTH_MASK)
+				return false;
+		} else {
+			if (sg->length > (MSRC_LENGTH_MASK * 16))
+				return false;
+		}
+	}
+	for (sg = msg->spu.dst; sg; sg = sg_next(sg)) {
+		if (sg->length & 0xf) {
+			if (sg->length > DST_LENGTH_MASK)
+				return false;
+		} else {
+			if (sg->length > (MDST_LENGTH_MASK * 16))
+				return false;
+		}
+	}
+
+	return true;
+}
+
+static u32 flexrm_spu_estimate_nonheader_desc_count(struct brcm_message *msg)
+{
+	u32 cnt = 0;
+	unsigned int dst_target = 0;
+	struct scatterlist *src_sg = msg->spu.src, *dst_sg = msg->spu.dst;
+
+	while (src_sg || dst_sg) {
+		if (src_sg) {
+			cnt++;
+			dst_target = src_sg->length;
+			src_sg = sg_next(src_sg);
+		} else
+			dst_target = UINT_MAX;
+
+		while (dst_target && dst_sg) {
+			cnt++;
+			if (dst_sg->length < dst_target)
+				dst_target -= dst_sg->length;
+			else
+				dst_target = 0;
+			dst_sg = sg_next(dst_sg);
+		}
+	}
+
+	return cnt;
+}
+
+static int flexrm_spu_dma_map(struct device *dev, struct brcm_message *msg)
+{
+	int rc;
+
+	rc = dma_map_sg(dev, msg->spu.src, sg_nents(msg->spu.src),
+			DMA_TO_DEVICE);
+	if (rc < 0)
+		return rc;
+
+	rc = dma_map_sg(dev, msg->spu.dst, sg_nents(msg->spu.dst),
+			DMA_FROM_DEVICE);
+	if (rc < 0) {
+		dma_unmap_sg(dev, msg->spu.src, sg_nents(msg->spu.src),
+			     DMA_TO_DEVICE);
+		return rc;
+	}
+
+	return 0;
+}
+
+static void flexrm_spu_dma_unmap(struct device *dev, struct brcm_message *msg)
+{
+	dma_unmap_sg(dev, msg->spu.dst, sg_nents(msg->spu.dst),
+		     DMA_FROM_DEVICE);
+	dma_unmap_sg(dev, msg->spu.src, sg_nents(msg->spu.src),
+		     DMA_TO_DEVICE);
+}
+
+static void *flexrm_spu_write_descs(struct brcm_message *msg, u32 nhcnt,
+				     u32 reqid, void *desc_ptr, u32 toggle,
+				     void *start_desc, void *end_desc)
+{
+	u64 d;
+	u32 nhpos = 0;
+	void *orig_desc_ptr = desc_ptr;
+	unsigned int dst_target = 0;
+	struct scatterlist *src_sg = msg->spu.src, *dst_sg = msg->spu.dst;
+
+	while (src_sg || dst_sg) {
+		if (src_sg) {
+			if (sg_dma_len(src_sg) & 0xf)
+				d = flexrm_src_desc(sg_dma_address(src_sg),
+						     sg_dma_len(src_sg));
+			else
+				d = flexrm_msrc_desc(sg_dma_address(src_sg),
+						      sg_dma_len(src_sg)/16);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+			dst_target = sg_dma_len(src_sg);
+			src_sg = sg_next(src_sg);
+		} else
+			dst_target = UINT_MAX;
+
+		while (dst_target && dst_sg) {
+			if (sg_dma_len(dst_sg) & 0xf)
+				d = flexrm_dst_desc(sg_dma_address(dst_sg),
+						     sg_dma_len(dst_sg));
+			else
+				d = flexrm_mdst_desc(sg_dma_address(dst_sg),
+						      sg_dma_len(dst_sg)/16);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+			if (sg_dma_len(dst_sg) < dst_target)
+				dst_target -= sg_dma_len(dst_sg);
+			else
+				dst_target = 0;
+			dst_sg = sg_next(dst_sg);
+		}
+	}
+
+	/* Null descriptor with invalid toggle bit */
+	flexrm_write_desc(desc_ptr, flexrm_null_desc(!toggle));
+
+	/* Ensure that descriptors have been written to memory */
+	wmb();
+
+	/* Flip toggle bit in header */
+	flexrm_flip_header_toogle(orig_desc_ptr);
+
+	return desc_ptr;
+}
+
+static bool flexrm_sba_sanity_check(struct brcm_message *msg)
+{
+	u32 i;
+
+	if (!msg->sba.cmds || !msg->sba.cmds_count)
+		return false;
+
+	for (i = 0; i < msg->sba.cmds_count; i++) {
+		if (((msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_B) ||
+		     (msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_C)) &&
+		    (msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_OUTPUT))
+			return false;
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_B) &&
+		    (msg->sba.cmds[i].data_len > SRCT_LENGTH_MASK))
+			return false;
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_C) &&
+		    (msg->sba.cmds[i].data_len > SRCT_LENGTH_MASK))
+			return false;
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_RESP) &&
+		    (msg->sba.cmds[i].resp_len > DSTT_LENGTH_MASK))
+			return false;
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_OUTPUT) &&
+		    (msg->sba.cmds[i].data_len > DSTT_LENGTH_MASK))
+			return false;
+	}
+
+	return true;
+}
+
+static u32 flexrm_sba_estimate_nonheader_desc_count(struct brcm_message *msg)
+{
+	u32 i, cnt;
+
+	cnt = 0;
+	for (i = 0; i < msg->sba.cmds_count; i++) {
+		cnt++;
+
+		if ((msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_B) ||
+		    (msg->sba.cmds[i].flags & BRCM_SBA_CMD_TYPE_C))
+			cnt++;
+
+		if (msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_RESP)
+			cnt++;
+
+		if (msg->sba.cmds[i].flags & BRCM_SBA_CMD_HAS_OUTPUT)
+			cnt++;
+	}
+
+	return cnt;
+}
+
+static void *flexrm_sba_write_descs(struct brcm_message *msg, u32 nhcnt,
+				     u32 reqid, void *desc_ptr, u32 toggle,
+				     void *start_desc, void *end_desc)
+{
+	u64 d;
+	u32 i, nhpos = 0;
+	struct brcm_sba_command *c;
+	void *orig_desc_ptr = desc_ptr;
+
+	/* Convert SBA commands into descriptors */
+	for (i = 0; i < msg->sba.cmds_count; i++) {
+		c = &msg->sba.cmds[i];
+
+		if ((c->flags & BRCM_SBA_CMD_HAS_RESP) &&
+		    (c->flags & BRCM_SBA_CMD_HAS_OUTPUT)) {
+			/* Destination response descriptor */
+			d = flexrm_dst_desc(c->resp, c->resp_len);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		} else if (c->flags & BRCM_SBA_CMD_HAS_RESP) {
+			/* Destination response with tlast descriptor */
+			d = flexrm_dstt_desc(c->resp, c->resp_len);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		}
+
+		if (c->flags & BRCM_SBA_CMD_HAS_OUTPUT) {
+			/* Destination with tlast descriptor */
+			d = flexrm_dstt_desc(c->data, c->data_len);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		}
+
+		if (c->flags & BRCM_SBA_CMD_TYPE_B) {
+			/* Command as immediate descriptor */
+			d = flexrm_imm_desc(c->cmd);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		} else {
+			/* Command as immediate descriptor with tlast */
+			d = flexrm_immt_desc(c->cmd);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		}
+
+		if ((c->flags & BRCM_SBA_CMD_TYPE_B) ||
+		    (c->flags & BRCM_SBA_CMD_TYPE_C)) {
+			/* Source with tlast descriptor */
+			d = flexrm_srct_desc(c->data, c->data_len);
+			flexrm_enqueue_desc(nhpos, nhcnt, reqid,
+					     d, &desc_ptr, &toggle,
+					     start_desc, end_desc);
+			nhpos++;
+		}
+	}
+
+	/* Null descriptor with invalid toggle bit */
+	flexrm_write_desc(desc_ptr, flexrm_null_desc(!toggle));
+
+	/* Ensure that descriptors have been written to memory */
+	wmb();
+
+	/* Flip toggle bit in header */
+	flexrm_flip_header_toogle(orig_desc_ptr);
+
+	return desc_ptr;
+}
+
+static bool flexrm_sanity_check(struct brcm_message *msg)
+{
+	if (!msg)
+		return false;
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		return flexrm_spu_sanity_check(msg);
+	case BRCM_MESSAGE_SBA:
+		return flexrm_sba_sanity_check(msg);
+	default:
+		return false;
+	};
+}
+
+static u32 flexrm_estimate_nonheader_desc_count(struct brcm_message *msg)
+{
+	if (!msg)
+		return 0;
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		return flexrm_spu_estimate_nonheader_desc_count(msg);
+	case BRCM_MESSAGE_SBA:
+		return flexrm_sba_estimate_nonheader_desc_count(msg);
+	default:
+		return 0;
+	};
+}
+
+static int flexrm_dma_map(struct device *dev, struct brcm_message *msg)
+{
+	if (!dev || !msg)
+		return -EINVAL;
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		return flexrm_spu_dma_map(dev, msg);
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static void flexrm_dma_unmap(struct device *dev, struct brcm_message *msg)
+{
+	if (!dev || !msg)
+		return;
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		flexrm_spu_dma_unmap(dev, msg);
+		break;
+	default:
+		break;
+	}
+}
+
+static void *flexrm_write_descs(struct brcm_message *msg, u32 nhcnt,
+				u32 reqid, void *desc_ptr, u32 toggle,
+				void *start_desc, void *end_desc)
+{
+	if (!msg || !desc_ptr || !start_desc || !end_desc)
+		return ERR_PTR(-ENOTSUPP);
+
+	if ((desc_ptr < start_desc) || (end_desc <= desc_ptr))
+		return ERR_PTR(-ERANGE);
+
+	switch (msg->type) {
+	case BRCM_MESSAGE_SPU:
+		return flexrm_spu_write_descs(msg, nhcnt, reqid,
+					       desc_ptr, toggle,
+					       start_desc, end_desc);
+	case BRCM_MESSAGE_SBA:
+		return flexrm_sba_write_descs(msg, nhcnt, reqid,
+					       desc_ptr, toggle,
+					       start_desc, end_desc);
+	default:
+		return ERR_PTR(-ENOTSUPP);
+	};
+}
+
+/* ====== FlexRM driver helper routines ===== */
+
+static int flexrm_new_request(struct flexrm_ring *ring,
+				struct brcm_message *batch_msg,
+				struct brcm_message *msg)
+{
+	void *next;
+	unsigned long flags;
+	u32 val, count, nhcnt;
+	u32 read_offset, write_offset;
+	bool exit_cleanup = false;
+	int ret = 0, reqid;
+
+	/* Do sanity check on message */
+	if (!flexrm_sanity_check(msg))
+		return -EIO;
+	msg->error = 0;
+
+	/* If no requests possible then save data pointer and goto done. */
+	reqid = ida_simple_get(&ring->requests_ida, 0,
+				RING_MAX_REQ_COUNT, GFP_KERNEL);
+	if (reqid < 0) {
+		spin_lock_irqsave(&ring->lock, flags);
+		if (batch_msg)
+			ring->last_pending_msg = batch_msg;
+		else
+			ring->last_pending_msg = msg;
+		spin_unlock_irqrestore(&ring->lock, flags);
+		return 0;
+	}
+	ring->requests[reqid] = msg;
+
+	/* Do DMA mappings for the message */
+	ret = flexrm_dma_map(ring->mbox->dev, msg);
+	if (ret < 0) {
+		ring->requests[reqid] = NULL;
+		ida_simple_remove(&ring->requests_ida, reqid);
+		return ret;
+	}
+
+	/* If last_pending_msg is already set then goto done with error */
+	spin_lock_irqsave(&ring->lock, flags);
+	if (ring->last_pending_msg)
+		ret = -ENOSPC;
+	spin_unlock_irqrestore(&ring->lock, flags);
+	if (ret < 0) {
+		dev_warn(ring->mbox->dev, "no space in ring %d\n", ring->num);
+		exit_cleanup = true;
+		goto exit;
+	}
+
+	/* Determine current HW BD read offset */
+	read_offset = readl_relaxed(ring->regs + RING_BD_READ_PTR);
+	val = readl_relaxed(ring->regs + RING_BD_START_ADDR);
+	read_offset *= RING_DESC_SIZE;
+	read_offset += (u32)(BD_START_ADDR_DECODE(val) - ring->bd_dma_base);
+
+	/*
+	 * Number required descriptors = number of non-header descriptors +
+	 *				 number of header descriptors +
+	 *				 1x null descriptor
+	 */
+	nhcnt = flexrm_estimate_nonheader_desc_count(msg);
+	count = flexrm_estimate_header_desc_count(nhcnt) + nhcnt + 1;
+
+	/* Check for available descriptor space. */
+	write_offset = ring->bd_write_offset;
+	while (count) {
+		if (!flexrm_is_next_table_desc(ring->bd_base + write_offset))
+			count--;
+		write_offset += RING_DESC_SIZE;
+		if (write_offset == RING_BD_SIZE)
+			write_offset = 0x0;
+		if (write_offset == read_offset)
+			break;
+	}
+	if (count) {
+		spin_lock_irqsave(&ring->lock, flags);
+		if (batch_msg)
+			ring->last_pending_msg = batch_msg;
+		else
+			ring->last_pending_msg = msg;
+		spin_unlock_irqrestore(&ring->lock, flags);
+		ret = 0;
+		exit_cleanup = true;
+		goto exit;
+	}
+
+	/* Write descriptors to ring */
+	next = flexrm_write_descs(msg, nhcnt, reqid,
+			ring->bd_base + ring->bd_write_offset,
+			RING_BD_TOGGLE_VALID(ring->bd_write_offset),
+			ring->bd_base, ring->bd_base + RING_BD_SIZE);
+	if (IS_ERR(next)) {
+		ret = PTR_ERR(next);
+		exit_cleanup = true;
+		goto exit;
+	}
+
+	/* Save ring BD write offset */
+	ring->bd_write_offset = (unsigned long)(next - ring->bd_base);
+
+exit:
+	/* Update error status in message */
+	msg->error = ret;
+
+	/* Cleanup if we failed */
+	if (exit_cleanup) {
+		flexrm_dma_unmap(ring->mbox->dev, msg);
+		ring->requests[reqid] = NULL;
+		ida_simple_remove(&ring->requests_ida, reqid);
+	}
+
+	return ret;
+}
+
+static int flexrm_process_completions(struct flexrm_ring *ring)
+{
+	u64 desc;
+	int err, count = 0;
+	unsigned long flags;
+	struct brcm_message *msg = NULL;
+	u32 reqid, cmpl_read_offset, cmpl_write_offset;
+	struct mbox_chan *chan = &ring->mbox->controller.chans[ring->num];
+
+	spin_lock_irqsave(&ring->lock, flags);
+
+	/* Check last_pending_msg */
+	if (ring->last_pending_msg) {
+		msg = ring->last_pending_msg;
+		ring->last_pending_msg = NULL;
+	}
+
+	/*
+	 * Get current completion read and write offset
+	 *
+	 * Note: We should read completion write pointer atleast once
+	 * after we get a MSI interrupt because HW maintains internal
+	 * MSI status which will allow next MSI interrupt only after
+	 * completion write pointer is read.
+	 */
+	cmpl_write_offset = readl_relaxed(ring->regs + RING_CMPL_WRITE_PTR);
+	cmpl_write_offset *= RING_DESC_SIZE;
+	cmpl_read_offset = ring->cmpl_read_offset;
+	ring->cmpl_read_offset = cmpl_write_offset;
+
+	spin_unlock_irqrestore(&ring->lock, flags);
+
+	/* If last_pending_msg was set then queue it back */
+	if (msg)
+		mbox_send_message(chan, msg);
+
+	/* For each completed request notify mailbox clients */
+	reqid = 0;
+	while (cmpl_read_offset != cmpl_write_offset) {
+		/* Dequeue next completion descriptor */
+		desc = *((u64 *)(ring->cmpl_base + cmpl_read_offset));
+
+		/* Next read offset */
+		cmpl_read_offset += RING_DESC_SIZE;
+		if (cmpl_read_offset == RING_CMPL_SIZE)
+			cmpl_read_offset = 0;
+
+		/* Decode error from completion descriptor */
+		err = flexrm_cmpl_desc_to_error(desc);
+		if (err < 0) {
+			dev_warn(ring->mbox->dev,
+				 "got completion desc=0x%lx with error %d",
+				 (unsigned long)desc, err);
+		}
+
+		/* Determine request id from completion descriptor */
+		reqid = flexrm_cmpl_desc_to_reqid(desc);
+
+		/* Determine message pointer based on reqid */
+		msg = ring->requests[reqid];
+		if (!msg) {
+			dev_warn(ring->mbox->dev,
+				 "null msg pointer for completion desc=0x%lx",
+				 (unsigned long)desc);
+			continue;
+		}
+
+		/* Release reqid for recycling */
+		ring->requests[reqid] = NULL;
+		ida_simple_remove(&ring->requests_ida, reqid);
+
+		/* Unmap DMA mappings */
+		flexrm_dma_unmap(ring->mbox->dev, msg);
+
+		/* Give-back message to mailbox client */
+		msg->error = err;
+		mbox_chan_received_data(chan, msg);
+
+		/* Increment number of completions processed */
+		count++;
+	}
+
+	return count;
+}
+
+/* ====== FlexRM interrupt handler ===== */
+
+static irqreturn_t flexrm_irq_event(int irq, void *dev_id)
+{
+	/* We only have MSI for completions so just wakeup IRQ thread */
+	/* Ring related errors will be informed via completion descriptors */
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t flexrm_irq_thread(int irq, void *dev_id)
+{
+	flexrm_process_completions(dev_id);
+
+	return IRQ_HANDLED;
+}
+
+/* ====== FlexRM mailbox callbacks ===== */
+
+static int flexrm_send_data(struct mbox_chan *chan, void *data)
+{
+	int i, rc;
+	struct flexrm_ring *ring = chan->con_priv;
+	struct brcm_message *msg = data;
+
+	if (msg->type == BRCM_MESSAGE_BATCH) {
+		for (i = msg->batch.msgs_queued;
+		     i < msg->batch.msgs_count; i++) {
+			rc = flexrm_new_request(ring, msg,
+						 &msg->batch.msgs[i]);
+			if (rc) {
+				msg->error = rc;
+				return rc;
+			}
+			msg->batch.msgs_queued++;
+		}
+		return 0;
+	}
+
+	return flexrm_new_request(ring, NULL, data);
+}
+
+static bool flexrm_peek_data(struct mbox_chan *chan)
+{
+	int cnt = flexrm_process_completions(chan->con_priv);
+
+	return (cnt > 0) ? true : false;
+}
+
+static int flexrm_startup(struct mbox_chan *chan)
+{
+	u64 d;
+	u32 val, off;
+	int ret = 0;
+	dma_addr_t next_addr;
+	struct flexrm_ring *ring = chan->con_priv;
+
+	/* Allocate BD memory */
+	ring->bd_base = dma_pool_alloc(ring->mbox->bd_pool,
+				       GFP_KERNEL, &ring->bd_dma_base);
+	if (!ring->bd_base) {
+		dev_err(ring->mbox->dev, "can't allocate BD memory\n");
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	/* Configure next table pointer entries in BD memory */
+	for (off = 0; off < RING_BD_SIZE; off += RING_DESC_SIZE) {
+		next_addr = off + RING_DESC_SIZE;
+		if (next_addr == RING_BD_SIZE)
+			next_addr = 0;
+		next_addr += ring->bd_dma_base;
+		if (RING_BD_ALIGN_CHECK(next_addr))
+			d = flexrm_next_table_desc(RING_BD_TOGGLE_VALID(off),
+						    next_addr);
+		else
+			d = flexrm_null_desc(RING_BD_TOGGLE_INVALID(off));
+		flexrm_write_desc(ring->bd_base + off, d);
+	}
+
+	/* Allocate completion memory */
+	ring->cmpl_base = dma_pool_alloc(ring->mbox->cmpl_pool,
+					 GFP_KERNEL, &ring->cmpl_dma_base);
+	if (!ring->cmpl_base) {
+		dev_err(ring->mbox->dev, "can't allocate completion memory\n");
+		ret = -ENOMEM;
+		goto fail_free_bd_memory;
+	}
+	memset(ring->cmpl_base, 0, RING_CMPL_SIZE);
+
+	/* Request IRQ */
+	if (ring->irq == UINT_MAX) {
+		dev_err(ring->mbox->dev, "ring IRQ not available\n");
+		ret = -ENODEV;
+		goto fail_free_cmpl_memory;
+	}
+	ret = request_threaded_irq(ring->irq,
+				   flexrm_irq_event,
+				   flexrm_irq_thread,
+				   0, dev_name(ring->mbox->dev), ring);
+	if (ret) {
+		dev_err(ring->mbox->dev, "failed to request ring IRQ\n");
+		goto fail_free_cmpl_memory;
+	}
+	ring->irq_requested = true;
+
+	/* Disable/inactivate ring */
+	writel_relaxed(0x0, ring->regs + RING_CONTROL);
+
+	/* Program BD start address */
+	val = BD_START_ADDR_VALUE(ring->bd_dma_base);
+	writel_relaxed(val, ring->regs + RING_BD_START_ADDR);
+
+	/* BD write pointer will be same as HW write pointer */
+	ring->bd_write_offset =
+			readl_relaxed(ring->regs + RING_BD_WRITE_PTR);
+	ring->bd_write_offset *= RING_DESC_SIZE;
+
+	/* Program completion start address */
+	val = CMPL_START_ADDR_VALUE(ring->cmpl_dma_base);
+	writel_relaxed(val, ring->regs + RING_CMPL_START_ADDR);
+
+	/* Ensure last pending message is cleared */
+	ring->last_pending_msg = NULL;
+
+	/* Completion read pointer will be same as HW write pointer */
+	ring->cmpl_read_offset =
+			readl_relaxed(ring->regs + RING_CMPL_WRITE_PTR);
+	ring->cmpl_read_offset *= RING_DESC_SIZE;
+
+	/* Read ring Tx, Rx, and Outstanding counts to clear */
+	readl_relaxed(ring->regs + RING_NUM_REQ_RECV_LS);
+	readl_relaxed(ring->regs + RING_NUM_REQ_RECV_MS);
+	readl_relaxed(ring->regs + RING_NUM_REQ_TRANS_LS);
+	readl_relaxed(ring->regs + RING_NUM_REQ_TRANS_MS);
+	readl_relaxed(ring->regs + RING_NUM_REQ_OUTSTAND);
+
+	/* Configure RING_MSI_CONTROL */
+	val = 0;
+	val |= (ring->msi_timer_val << MSI_TIMER_VAL_SHIFT);
+	val |= BIT(MSI_ENABLE_SHIFT);
+	val |= (ring->msi_count_threshold & MSI_COUNT_MASK) << MSI_COUNT_SHIFT;
+	writel_relaxed(val, ring->regs + RING_MSI_CONTROL);
+
+	/* Enable/activate ring */
+	val = BIT(CONTROL_ACTIVE_SHIFT);
+	writel_relaxed(val, ring->regs + RING_CONTROL);
+
+	return 0;
+
+fail_free_cmpl_memory:
+	dma_pool_free(ring->mbox->cmpl_pool,
+		      ring->cmpl_base, ring->cmpl_dma_base);
+	ring->cmpl_base = NULL;
+fail_free_bd_memory:
+	dma_pool_free(ring->mbox->bd_pool,
+		      ring->bd_base, ring->bd_dma_base);
+	ring->bd_base = NULL;
+fail:
+	return ret;
+}
+
+static void flexrm_shutdown(struct mbox_chan *chan)
+{
+	u32 reqid;
+	unsigned int timeout;
+	struct brcm_message *msg;
+	struct flexrm_ring *ring = chan->con_priv;
+
+	/* Disable/inactivate ring */
+	writel_relaxed(0x0, ring->regs + RING_CONTROL);
+
+	/* Flush ring with timeout of 1s */
+	timeout = 1000;
+	writel_relaxed(BIT(CONTROL_FLUSH_SHIFT),
+			ring->regs + RING_CONTROL);
+	do {
+		if (readl_relaxed(ring->regs + RING_FLUSH_DONE) &
+		    FLUSH_DONE_MASK)
+			break;
+		mdelay(1);
+	} while (timeout--);
+
+	/* Abort all in-flight requests */
+	for (reqid = 0; reqid < RING_MAX_REQ_COUNT; reqid++) {
+		msg = ring->requests[reqid];
+		if (!msg)
+			continue;
+
+		/* Release reqid for recycling */
+		ring->requests[reqid] = NULL;
+		ida_simple_remove(&ring->requests_ida, reqid);
+
+		/* Unmap DMA mappings */
+		flexrm_dma_unmap(ring->mbox->dev, msg);
+
+		/* Give-back message to mailbox client */
+		msg->error = -EIO;
+		mbox_chan_received_data(chan, msg);
+	}
+
+	/* Release IRQ */
+	if (ring->irq_requested) {
+		free_irq(ring->irq, ring);
+		ring->irq_requested = false;
+	}
+
+	/* Free-up completion descriptor ring */
+	if (ring->cmpl_base) {
+		dma_pool_free(ring->mbox->cmpl_pool,
+			      ring->cmpl_base, ring->cmpl_dma_base);
+		ring->cmpl_base = NULL;
+	}
+
+	/* Free-up BD descriptor ring */
+	if (ring->bd_base) {
+		dma_pool_free(ring->mbox->bd_pool,
+			      ring->bd_base, ring->bd_dma_base);
+		ring->bd_base = NULL;
+	}
+}
+
+static bool flexrm_last_tx_done(struct mbox_chan *chan)
+{
+	bool ret;
+	unsigned long flags;
+	struct flexrm_ring *ring = chan->con_priv;
+
+	spin_lock_irqsave(&ring->lock, flags);
+	ret = (ring->last_pending_msg) ? false : true;
+	spin_unlock_irqrestore(&ring->lock, flags);
+
+	return ret;
+}
+
+static const struct mbox_chan_ops flexrm_mbox_chan_ops = {
+	.send_data	= flexrm_send_data,
+	.startup	= flexrm_startup,
+	.shutdown	= flexrm_shutdown,
+	.last_tx_done	= flexrm_last_tx_done,
+	.peek_data	= flexrm_peek_data,
+};
+
+static struct mbox_chan *flexrm_mbox_of_xlate(struct mbox_controller *cntlr,
+					const struct of_phandle_args *pa)
+{
+	struct mbox_chan *chan;
+	struct flexrm_ring *ring;
+
+	if (pa->args_count < 3)
+		return ERR_PTR(-EINVAL);
+
+	if (pa->args[0] >= cntlr->num_chans)
+		return ERR_PTR(-ENOENT);
+
+	if (pa->args[1] > MSI_COUNT_MASK)
+		return ERR_PTR(-EINVAL);
+
+	if (pa->args[2] > MSI_TIMER_VAL_MASK)
+		return ERR_PTR(-EINVAL);
+
+	chan = &cntlr->chans[pa->args[0]];
+	ring = chan->con_priv;
+	ring->msi_count_threshold = pa->args[1];
+	ring->msi_timer_val = pa->args[2];
+
+	return chan;
+}
+
+/* ====== FlexRM platform driver ===== */
+
+static void flexrm_mbox_msi_write(struct msi_desc *desc, struct msi_msg *msg)
+{
+	struct device *dev = msi_desc_to_dev(desc);
+	struct flexrm_mbox *mbox = dev_get_drvdata(dev);
+	struct flexrm_ring *ring = &mbox->rings[desc->platform.msi_index];
+
+	/* Configure per-Ring MSI registers */
+	writel_relaxed(msg->address_lo, ring->regs + RING_MSI_ADDR_LS);
+	writel_relaxed(msg->address_hi, ring->regs + RING_MSI_ADDR_MS);
+	writel_relaxed(msg->data, ring->regs + RING_MSI_DATA_VALUE);
+}
+
+static int flexrm_mbox_probe(struct platform_device *pdev)
+{
+	int index, ret = 0;
+	void __iomem *regs;
+	void __iomem *regs_end;
+	struct msi_desc *desc;
+	struct resource *iomem;
+	struct flexrm_ring *ring;
+	struct flexrm_mbox *mbox;
+	struct device *dev = &pdev->dev;
+
+	/* Allocate driver mailbox struct */
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	mbox->dev = dev;
+	platform_set_drvdata(pdev, mbox);
+
+	/* Get resource for registers */
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!iomem || (resource_size(iomem) < RING_REGS_SIZE)) {
+		ret = -ENODEV;
+		goto fail;
+	}
+
+	/* Map registers of all rings */
+	mbox->regs = devm_ioremap_resource(&pdev->dev, iomem);
+	if (IS_ERR(mbox->regs)) {
+		ret = PTR_ERR(mbox->regs);
+		dev_err(&pdev->dev, "Failed to remap mailbox regs: %d\n", ret);
+		goto fail;
+	}
+	regs_end = mbox->regs + resource_size(iomem);
+
+	/* Scan and count available rings */
+	mbox->num_rings = 0;
+	for (regs = mbox->regs; regs < regs_end; regs += RING_REGS_SIZE) {
+		if (readl_relaxed(regs + RING_VER) == RING_VER_MAGIC)
+			mbox->num_rings++;
+	}
+	if (!mbox->num_rings) {
+		ret = -ENODEV;
+		goto fail;
+	}
+
+	/* Allocate driver ring structs */
+	ring = devm_kcalloc(dev, mbox->num_rings, sizeof(*ring), GFP_KERNEL);
+	if (!ring) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+	mbox->rings = ring;
+
+	/* Initialize members of driver ring structs */
+	regs = mbox->regs;
+	for (index = 0; index < mbox->num_rings; index++) {
+		ring = &mbox->rings[index];
+		ring->num = index;
+		ring->mbox = mbox;
+		while ((regs < regs_end) &&
+		       (readl_relaxed(regs + RING_VER) != RING_VER_MAGIC))
+			regs += RING_REGS_SIZE;
+		if (regs_end <= regs) {
+			ret = -ENODEV;
+			goto fail;
+		}
+		ring->regs = regs;
+		regs += RING_REGS_SIZE;
+		ring->irq = UINT_MAX;
+		ring->irq_requested = false;
+		ring->msi_timer_val = MSI_TIMER_VAL_MASK;
+		ring->msi_count_threshold = 0x1;
+		ida_init(&ring->requests_ida);
+		memset(ring->requests, 0, sizeof(ring->requests));
+		ring->bd_base = NULL;
+		ring->bd_dma_base = 0;
+		ring->cmpl_base = NULL;
+		ring->cmpl_dma_base = 0;
+		spin_lock_init(&ring->lock);
+		ring->last_pending_msg = NULL;
+		ring->cmpl_read_offset = 0;
+	}
+
+	/* FlexRM is capable of 40-bit physical addresses only */
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
+	if (ret) {
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret)
+			goto fail;
+	}
+
+	/* Create DMA pool for ring BD memory */
+	mbox->bd_pool = dma_pool_create("bd", dev, RING_BD_SIZE,
+					1 << RING_BD_ALIGN_ORDER, 0);
+	if (!mbox->bd_pool) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	/* Create DMA pool for ring completion memory */
+	mbox->cmpl_pool = dma_pool_create("cmpl", dev, RING_CMPL_SIZE,
+					  1 << RING_CMPL_ALIGN_ORDER, 0);
+	if (!mbox->cmpl_pool) {
+		ret = -ENOMEM;
+		goto fail_destroy_bd_pool;
+	}
+
+	/* Allocate platform MSIs for each ring */
+	ret = platform_msi_domain_alloc_irqs(dev, mbox->num_rings,
+						flexrm_mbox_msi_write);
+	if (ret)
+		goto fail_destroy_cmpl_pool;
+
+	/* Save alloced IRQ numbers for each ring */
+	for_each_msi_entry(desc, dev) {
+		ring = &mbox->rings[desc->platform.msi_index];
+		ring->irq = desc->irq;
+	}
+
+	/* Initialize mailbox controller */
+	mbox->controller.txdone_irq = false;
+	mbox->controller.txdone_poll = true;
+	mbox->controller.txpoll_period = 1;
+	mbox->controller.ops = &flexrm_mbox_chan_ops;
+	mbox->controller.dev = dev;
+	mbox->controller.num_chans = mbox->num_rings;
+	mbox->controller.of_xlate = flexrm_mbox_of_xlate;
+	mbox->controller.chans = devm_kcalloc(dev, mbox->num_rings,
+				sizeof(*mbox->controller.chans), GFP_KERNEL);
+	if (!mbox->controller.chans) {
+		ret = -ENOMEM;
+		goto fail_free_msis;
+	}
+	for (index = 0; index < mbox->num_rings; index++)
+		mbox->controller.chans[index].con_priv = &mbox->rings[index];
+
+	/* Register mailbox controller */
+	ret = mbox_controller_register(&mbox->controller);
+	if (ret)
+		goto fail_free_msis;
+
+	dev_info(dev, "registered flexrm mailbox with %d channels\n",
+			mbox->controller.num_chans);
+
+	return 0;
+
+fail_free_msis:
+	platform_msi_domain_free_irqs(dev);
+fail_destroy_cmpl_pool:
+	dma_pool_destroy(mbox->cmpl_pool);
+fail_destroy_bd_pool:
+	dma_pool_destroy(mbox->bd_pool);
+fail:
+	return ret;
+}
+
+static int flexrm_mbox_remove(struct platform_device *pdev)
+{
+	int index;
+	struct device *dev = &pdev->dev;
+	struct flexrm_ring *ring;
+	struct flexrm_mbox *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(&mbox->controller);
+
+	platform_msi_domain_free_irqs(dev);
+
+	dma_pool_destroy(mbox->cmpl_pool);
+	dma_pool_destroy(mbox->bd_pool);
+
+	for (index = 0; index < mbox->num_rings; index++) {
+		ring = &mbox->rings[index];
+		ida_destroy(&ring->requests_ida);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id flexrm_mbox_of_match[] = {
+	{ .compatible = "brcm,iproc-flexrm-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, flexrm_mbox_of_match);
+
+static struct platform_driver flexrm_mbox_driver = {
+	.driver = {
+		.name = "brcm-flexrm-mbox",
+		.of_match_table = flexrm_mbox_of_match,
+	},
+	.probe		= flexrm_mbox_probe,
+	.remove		= flexrm_mbox_remove,
+};
+module_platform_driver(flexrm_mbox_driver);
+
+MODULE_AUTHOR("Anup Patel <anup.patel@broadcom.com>");
+MODULE_DESCRIPTION("Broadcom FlexRM mailbox driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mailbox/brcm-message.h b/include/linux/mailbox/brcm-message.h
index 6b55c93..c20b484 100644
--- a/include/linux/mailbox/brcm-message.h
+++ b/include/linux/mailbox/brcm-message.h
@@ -16,6 +16,7 @@
 
 enum brcm_message_type {
 	BRCM_MESSAGE_UNKNOWN = 0,
+	BRCM_MESSAGE_BATCH,
 	BRCM_MESSAGE_SPU,
 	BRCM_MESSAGE_SBA,
 	BRCM_MESSAGE_MAX,
@@ -23,24 +24,29 @@ enum brcm_message_type {
 
 struct brcm_sba_command {
 	u64 cmd;
+	u64 *cmd_dma;
+	dma_addr_t cmd_dma_addr;
 #define BRCM_SBA_CMD_TYPE_A		BIT(0)
 #define BRCM_SBA_CMD_TYPE_B		BIT(1)
 #define BRCM_SBA_CMD_TYPE_C		BIT(2)
 #define BRCM_SBA_CMD_HAS_RESP		BIT(3)
 #define BRCM_SBA_CMD_HAS_OUTPUT		BIT(4)
 	u64 flags;
-	dma_addr_t input;
-	size_t input_len;
 	dma_addr_t resp;
 	size_t resp_len;
-	dma_addr_t output;
-	size_t output_len;
+	dma_addr_t data;
+	size_t data_len;
 };
 
 struct brcm_message {
 	enum brcm_message_type type;
 	union {
 		struct {
+			struct brcm_message *msgs;
+			unsigned int msgs_queued;
+			unsigned int msgs_count;
+		} batch;
+		struct {
 			struct scatterlist *src;
 			struct scatterlist *dst;
 		} spu;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 2/3] dt-bindings: Add DT bindings info for FlexRM ring manager
From: Anup Patel @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Jassi Brar, Rob Herring
  Cc: Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR, Rob Rice,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Anup Patel
In-Reply-To: <1490765455-8556-1-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

This patch adds device tree bindings document for the FlexRM
ring manager found on Broadcom iProc SoCs.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 .../bindings/mailbox/brcm,iproc-flexrm-mbox.txt    | 59 ++++++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
new file mode 100644
index 0000000..752ae6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
@@ -0,0 +1,59 @@
+Broadcom FlexRM Ring Manager
+============================
+The Broadcom FlexRM ring manager provides a set of rings which can be
+used to submit work to offload engines. An SoC may have multiple FlexRM
+hardware blocks. There is one device tree entry per FlexRM block. The
+FlexRM driver will create a mailbox-controller instance for given FlexRM
+hardware block where each mailbox channel is a separate FlexRM ring.
+
+Required properties:
+--------------------
+- compatible:	Should be "brcm,iproc-flexrm-mbox"
+- reg:		Specifies base physical address and size of the FlexRM
+		ring registers
+- msi-parent:	Phandles (and potential Device IDs) to MSI controllers
+		The FlexRM engine will send MSIs (instead of wired
+		interrupts) to CPU. There is one MSI for each FlexRM ring.
+		Refer devicetree/bindings/interrupt-controller/msi.txt
+- #mbox-cells:	Specifies the number of cells needed to encode a mailbox
+		channel. This should be 3.
+
+		The 1st cell is the mailbox channel number.
+
+		The 2nd cell contains MSI completion threshold. This is the
+		number of completion messages for which FlexRM will inject
+		one MSI interrupt to CPU.
+
+		The 3nd cell contains MSI timer value representing time for
+		which FlexRM will wait to accumulate N completion messages
+		where N is the value specified by 2nd cell above. If FlexRM
+		does not get required number of completion messages in time
+		specified by this cell then it will inject one MSI interrupt
+		to CPU provided atleast one completion message is available.
+
+Optional properties:
+--------------------
+- dma-coherent:	Present if DMA operations made by the FlexRM engine (such
+		as DMA descriptor access, access to buffers pointed by DMA
+		descriptors and read/write pointer updates to DDR) are
+		cache coherent with the CPU.
+
+Example:
+--------
+crypto_mbox: mbox@67000000 {
+	compatible = "brcm,iproc-flexrm-mbox";
+	reg = <0x67000000 0x200000>;
+	msi-parent = <&gic_its 0x7f00>;
+	#mbox-cells = <3>;
+};
+
+crypto@672c0000 {
+	compatible = "brcm,spu2-v2-crypto";
+	reg = <0x672c0000 0x1000>;
+	mboxes = <&crypto_mbox 0 0x1 0xffff>,
+		 <&crypto_mbox 1 0x1 0xffff>,
+		 <&crypto_mbox 16 0x1 0xffff>,
+		 <&crypto_mbox 17 0x1 0xffff>,
+		 <&crypto_mbox 30 0x1 0xffff>,
+		 <&crypto_mbox 31 0x1 0xffff>;
+};
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v7 3/3] mailbox: Remove depends on COMPILE_TEST for BCM_FLEXRM_MBOX
From: Anup Patel @ 2017-03-29  5:30 UTC (permalink / raw)
  To: Jassi Brar, Rob Herring
  Cc: Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR, Rob Rice,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Anup Patel
In-Reply-To: <1490765455-8556-1-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

The Broadcom FlexRM mailbox driver uses platform MSI support but
not all ARCHs provide asm/msi.h. Due to this, we get compilation
error in Broadcom FlexRM mailbox driver via linux/msi.h on ARCHs
which lack asm/msi.h.

This patch removes "depends on COMPILE_TEST" for Kconfig option
BCM_FLEXRM_MBOX so that Broadcom FlexRM mailbox driver is only
compiled for ARM64.

Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/mailbox/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 305018c..9c19b9f 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -155,7 +155,7 @@ config BCM_PDC_MBOX
 
 config BCM_FLEXRM_MBOX
 	tristate "Broadcom FlexRM Mailbox"
-	depends on ARM64 || COMPILE_TEST
+	depends on ARM64
 	depends on HAS_DMA
 	select GENERIC_MSI_IRQ_DOMAIN
 	default ARCH_BCM_IPROC
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v7 0/3] Broadcom FlexRM ring manager support
From: Jassi Brar @ 2017-03-29  5:35 UTC (permalink / raw)
  To: Anup Patel
  Cc: Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR,
	Rob Rice, Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	BCM Kernel Feedback
In-Reply-To: <1490765455-8556-1-git-send-email-anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On Wed, Mar 29, 2017 at 11:00 AM, Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> The Broadcom FlexRM ring manager provides producer-consumer style
> ring interface for offload engines on Broadcom iProc SoCs. We can
> have one or more instances of Broadcom FlexRM ring manager in a SoC.
>
> This patchset adds a mailbox driver for Broadcom FlexRM ring manager
> which can be used by offload engine drivers as mailbox clients.
>
> The Broadcom FlexRM mailbox driver is feature complete for RAID and
> Crypto offload engines. We will have incremental patches in-future
> for ring-level statistics using debugfs and minor optimizations.
>
> This patchset is based on Linux-4.11-rc4 and it is also available
> at flexrm-v7 branch of https://github.com/Broadcom/arm64-linux.git
>
> Changes since v6:
>  - Rebased patches for Linux-4.11-rc4
>  - Added a patch to remove depends on COMPILE_TEST for kconfig
>    option BCM_FLEXRM_MBOX
>
You only needed to send the fix to compilation failure reports, not
the whole series.
I believe only the patch-3 is new in the series and I need not even
look at patch-1,2 ?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH] of: Fix DMA configuration for non-DT masters
From: Oza Oza via iommu @ 2017-03-29  5:46 UTC (permalink / raw)
  To: Robin Murphy
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Linux IOMMU,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <CAMSpPPfJKKRkuDKxUgVBdg7vp7Ux5=PiHSxnMLc7-LMcCSatzg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wed, Mar 29, 2017 at 10:23 AM, Oza Oza <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On Wed, Mar 29, 2017 at 12:27 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> For PCI masters not represented in DT, we pass the OF node of their
>> associated host bridge to of_dma_configure(), such that they can inherit
>> the appropriate DMA configuration from whatever is described there.
>> Unfortunately, whilst this has worked for the "dma-coherent" property,
>> it turns out to miss the case where the host bridge node has a non-empty
>> "dma-ranges", since nobody is expecting the 'device' to be a bus itself.
>>
>> It transpires, though, that the de-facto interface since the prototype
>> change in 1f5c69aa51f9 ("of: Move of_dma_configure() to device.c to help
>> re-use") is very clear-cut: either the master_np argument is redundant
>> with dev->of_node, or dev->of_node is NULL and master_np is the relevant
>> parent bus. Let's ratify that behaviour, then teach the whole
>> of_dma_configure() pipeline to cope with both cases properly.
>>
>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>> ---
>>
>> This is what I'd consider the better fix - rather than adding yet more
>> special cases - which will also make it simple to handle multiple
>> "dma-ranges" entries with minimal further disruption. The callers now
>> left passing dev->of_node as 'parent' are harmless, but look a bit silly
>> and clearly want cleaning up - I'd be partial to renaming the existing
>> function and having a single-argument wrapper for the 'normal' case, e.g.:
>>
>> static inline int of_dma_configure(struct device_node *dev)
>> {
>>         return of_dma_configure_parent(dev, NULL);
>> }
>>
>> Thoughts?
>>
>> Robin.
>>
>>  drivers/iommu/of_iommu.c   |  7 ++++---
>>  drivers/of/address.c       | 37 +++++++++++++++++++++++++------------
>>  drivers/of/device.c        | 12 +++++++-----
>>  include/linux/of_address.h |  7 ++++---
>>  include/linux/of_device.h  |  4 ++--
>>  include/linux/of_iommu.h   |  4 ++--
>>  6 files changed, 44 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index 2683e9fc0dcf..35aff07bb5eb 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -138,21 +138,22 @@ static const struct iommu_ops
>>  }
>>
>>  const struct iommu_ops *of_iommu_configure(struct device *dev,
>> -                                          struct device_node *master_np)
>> +                                          struct device_node *parent)
>>  {
>>         struct of_phandle_args iommu_spec;
>> -       struct device_node *np;
>> +       struct device_node *np, *master_np;
>>         const struct iommu_ops *ops = NULL;
>>         int idx = 0;
>>
>>         if (dev_is_pci(dev))
>> -               return of_pci_iommu_configure(to_pci_dev(dev), master_np);
>> +               return of_pci_iommu_configure(to_pci_dev(dev), parent);
>>
>>         /*
>>          * We don't currently walk up the tree looking for a parent IOMMU.
>>          * See the `Notes:' section of
>>          * Documentation/devicetree/bindings/iommu/iommu.txt
>>          */
>> +       master_np = dev->of_node ? dev->of_node : parent;
>>         while (!of_parse_phandle_with_args(master_np, "iommus",
>>                                            "#iommu-cells", idx,
>>                                            &iommu_spec)) {
>> diff --git a/drivers/of/address.c b/drivers/of/address.c
>> index 02b2903fe9d2..833bc17f5e55 100644
>> --- a/drivers/of/address.c
>> +++ b/drivers/of/address.c
>> @@ -808,6 +808,7 @@ EXPORT_SYMBOL(of_io_request_and_map);
>>  /**
>>   * of_dma_get_range - Get DMA range info
>>   * @np:                device node to get DMA range info
>> + * @parent:    node of device's parent bus, if @np is NULL
>>   * @dma_addr:  pointer to store initial DMA address of DMA range
>>   * @paddr:     pointer to store initial CPU address of DMA range
>>   * @size:      pointer to store size of DMA range
>> @@ -822,36 +823,48 @@ EXPORT_SYMBOL(of_io_request_and_map);
>>   * It returns -ENODEV if "dma-ranges" property was not found
>>   * for this device in DT.
>>   */
>> -int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
>> +int of_dma_get_range(struct device_node *np, struct device_node *parent,
>> +                    u64 *dma_addr, u64 *paddr, u64 *size)
>>  {
>> -       struct device_node *node = of_node_get(np);
>> +       struct device_node *node;
>>         const __be32 *ranges = NULL;
>>         int len, naddr, nsize, pna;
>>         int ret = 0;
>>         u64 dmaaddr;
>>
>> -       if (!node)
>> -               return -EINVAL;
>> -
>> -       while (1) {
>> +       if (np) {
>> +               node = of_node_get(np);
>>                 naddr = of_n_addr_cells(node);
>>                 nsize = of_n_size_cells(node);
>>                 node = of_get_next_parent(node);
>> -               if (!node)
>> -                       break;
>> +       } else if (parent) {
>> +               node = of_node_get(parent);
>> +               np = parent;
>> +               if (of_property_read_u32(node, "#address-cells", &naddr))
>> +                       naddr = of_n_addr_cells(node);
>> +               if (of_property_read_u32(node, "#size-cells", &nsize))
>> +                       nsize = of_n_size_cells(node);
>> +       } else {
>> +               return -EINVAL;
>> +       }
>>
>> +       while (node) {
>>                 ranges = of_get_property(node, "dma-ranges", &len);
>>
>> -               /* Ignore empty ranges, they imply no translation required */
>> -               if (ranges && len > 0)
>> -                       break;
>> -
>>                 /*
>>                  * At least empty ranges has to be defined for parent node if
>>                  * DMA is supported
>>                  */
>>                 if (!ranges)
>>                         break;
>> +
>> +               /* Ignore empty ranges, they imply no translation required */
>> +               if (len > 0)
>> +                       break;
>> +
>> +               naddr = of_n_addr_cells(node);
>> +               nsize = of_n_size_cells(node);
>> +               node = of_get_next_parent(node);
>>         }
>>
>>         if (!ranges) {
>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> index 9bb8518b28f2..57ec5324ed6c 100644
>> --- a/drivers/of/device.c
>> +++ b/drivers/of/device.c
>> @@ -73,7 +73,8 @@ int of_device_add(struct platform_device *ofdev)
>>  /**
>>   * of_dma_configure - Setup DMA configuration
>>   * @dev:       Device to apply DMA configuration
>> - * @np:                Pointer to OF node having DMA configuration
>> + * @parent:    OF node of device's parent bus, if @dev is not
>> + *             represented in DT (i.e. @dev->of_node is NULL)
>>   *
>>   * Try to get devices's DMA configuration from DT and update it
>>   * accordingly.
>> @@ -82,13 +83,14 @@ int of_device_add(struct platform_device *ofdev)
>>   * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events
>>   * to fix up DMA configuration.
>>   */
>> -void of_dma_configure(struct device *dev, struct device_node *np)
>> +void of_dma_configure(struct device *dev, struct device_node *parent)
>>  {
>>         u64 dma_addr, paddr, size;
>>         int ret;
>>         bool coherent;
>>         unsigned long offset;
>>         const struct iommu_ops *iommu;
>> +       struct device_node *np = dev->of_node;
>>
>>         /*
>>          * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
>> @@ -104,7 +106,7 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>>         if (!dev->dma_mask)
>>                 dev->dma_mask = &dev->coherent_dma_mask;
>>
>> -       ret = of_dma_get_range(np, &dma_addr, &paddr, &size);
>> +       ret = of_dma_get_range(np, parent, &dma_addr, &paddr, &size);
>>         if (ret < 0) {
>>                 dma_addr = offset = 0;
>>                 size = dev->coherent_dma_mask + 1;
>> @@ -132,11 +134,11 @@ void of_dma_configure(struct device *dev, struct device_node *np)
>>         dev->coherent_dma_mask = DMA_BIT_MASK(ilog2(dma_addr + size));
>>         *dev->dma_mask = dev->coherent_dma_mask;
>>
>> -       coherent = of_dma_is_coherent(np);
>> +       coherent = of_dma_is_coherent(np ? np : parent);
>>         dev_dbg(dev, "device is%sdma coherent\n",
>>                 coherent ? " " : " not ");
>>
>> -       iommu = of_iommu_configure(dev, np);
>> +       iommu = of_iommu_configure(dev, parent);
>>         dev_dbg(dev, "device is%sbehind an iommu\n",
>>                 iommu ? " " : " not ");
>>
>> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
>> index 37864734ca50..f1a507f3ae57 100644
>> --- a/include/linux/of_address.h
>> +++ b/include/linux/of_address.h
>> @@ -52,8 +52,8 @@ extern int of_pci_range_parser_init(struct of_pci_range_parser *parser,
>>  extern struct of_pci_range *of_pci_range_parser_one(
>>                                         struct of_pci_range_parser *parser,
>>                                         struct of_pci_range *range);
>> -extern int of_dma_get_range(struct device_node *np, u64 *dma_addr,
>> -                               u64 *paddr, u64 *size);
>> +extern int of_dma_get_range(struct device_node *np, struct device_node *parent,
>> +                               u64 *dma_addr, u64 *paddr, u64 *size);
>>  extern bool of_dma_is_coherent(struct device_node *np);
>>  #else /* CONFIG_OF_ADDRESS */
>>  static inline void __iomem *of_io_request_and_map(struct device_node *device,
>> @@ -95,7 +95,8 @@ static inline struct of_pci_range *of_pci_range_parser_one(
>>         return NULL;
>>  }
>>
>> -static inline int of_dma_get_range(struct device_node *np, u64 *dma_addr,
>> +static inline int of_dma_get_range(struct device_node *np,
>> +                               struct device_node *parent, u64 *dma_addr,
>>                                 u64 *paddr, u64 *size)
>>  {
>>         return -ENODEV;
>> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
>> index c12dace043f3..bcd2b6fbeef3 100644
>> --- a/include/linux/of_device.h
>> +++ b/include/linux/of_device.h
>> @@ -55,7 +55,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>>         return of_node_get(cpu_dev->of_node);
>>  }
>>
>> -void of_dma_configure(struct device *dev, struct device_node *np);
>> +void of_dma_configure(struct device *dev, struct device_node *parent);
>>  #else /* CONFIG_OF */
>>
>>  static inline int of_driver_match_device(struct device *dev,
>> @@ -103,7 +103,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu)
>>  {
>>         return NULL;
>>  }
>> -static inline void of_dma_configure(struct device *dev, struct device_node *np)
>> +static inline void of_dma_configure(struct device *dev, struct device_node *parent)
>>  {}
>>  #endif /* CONFIG_OF */
>>
>> diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
>> index 13394ac83c66..c02b62e8e6ed 100644
>> --- a/include/linux/of_iommu.h
>> +++ b/include/linux/of_iommu.h
>> @@ -12,7 +12,7 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix,
>>                              size_t *size);
>>
>>  extern const struct iommu_ops *of_iommu_configure(struct device *dev,
>> -                                       struct device_node *master_np);
>> +                                       struct device_node *parent);
>>
>>  #else
>>
>> @@ -24,7 +24,7 @@ static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
>>  }
>>
>>  static inline const struct iommu_ops *of_iommu_configure(struct device *dev,
>> -                                        struct device_node *master_np)
>> +                                        struct device_node *parent)
>>  {
>>         return NULL;
>>  }
>> --
>> 2.11.0.dirty
>>
>
> pci and memory mapped device world is different. of course if you talk
> from iommu perspective, all the master are same for IOMMU
>
> but the original intention of the patch is to try to solve 2 problems.
> please refer to https://lkml.org/lkml/2017/3/29/10
>
> 1) expose generic API for pci world clients to configure their
> dma-ranges. right now there is none.
> 2) same API can be used by IOMMU to derive dma_mask.
>
> while the current patch posted to handle dma-ranges for both memory
> mapped and pci devices, which I think is overdoing.
>
> besides we have different configuration of dma-ranges based on iommu
> is enabled or not enabled.
>  #define PCIE_DMA_RANGES dma-ranges = <0x43000000 0x00 0x80000000 0x00
> 0x80000000 0x00 0x80000000 \
>                                       0x43000000 0x08 0x00000000 0x08
> 0x00000000 0x08 0x00000000 \
>                                       0x43000000 0x80 0x00000000 0x80
> 0x00000000 0x40 0x00000000>;
> Not sure if this patch will consider above dma-ranges.
>
> my suggestion is to leave existing of_dma_get_range as is, and have
> new function for pci world as discussed in
> please refer to https://lkml.org/lkml/2017/3/29/10
>
> Regards,
> Oza.

also I see that, in case of multiple ranges of_dma_get_range doesnt handle that.
and also it is not meant to handle.
so with this patch will return wrong size and hence wrong dma_mask.
having said, I think it is better to separate pci world dma-ranges
function on of_pci.c

for which my discussion with Rob already, (same thread)
https://lkml.org/lkml/2017/3/29/10
Waiting for Rob's viewpoint on this.


Regards,
Oza.

^ permalink raw reply

* Re: [PATCH v7 0/3] Broadcom FlexRM ring manager support
From: Anup Patel @ 2017-03-29  5:52 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR,
	Rob Rice, Devicetree List, Linux Kernel Mailing List,
	linux-arm-kernel@lists.infradead.org, BCM Kernel Feedback
In-Reply-To: <CABb+yY3n2OuvBKfsWj5m5RvdBEZ5_aWpVNUJ9V8oikyJjchZAw@mail.gmail.com>

On Wed, Mar 29, 2017 at 11:05 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Wed, Mar 29, 2017 at 11:00 AM, Anup Patel <anup.patel@broadcom.com> wrote:
>> The Broadcom FlexRM ring manager provides producer-consumer style
>> ring interface for offload engines on Broadcom iProc SoCs. We can
>> have one or more instances of Broadcom FlexRM ring manager in a SoC.
>>
>> This patchset adds a mailbox driver for Broadcom FlexRM ring manager
>> which can be used by offload engine drivers as mailbox clients.
>>
>> The Broadcom FlexRM mailbox driver is feature complete for RAID and
>> Crypto offload engines. We will have incremental patches in-future
>> for ring-level statistics using debugfs and minor optimizations.
>>
>> This patchset is based on Linux-4.11-rc4 and it is also available
>> at flexrm-v7 branch of https://github.com/Broadcom/arm64-linux.git
>>
>> Changes since v6:
>>  - Rebased patches for Linux-4.11-rc4
>>  - Added a patch to remove depends on COMPILE_TEST for kconfig
>>    option BCM_FLEXRM_MBOX
>>
> You only needed to send the fix to compilation failure reports, not
> the whole series.

My bad for the noise.

> I believe only the patch-3 is new in the series and I need not even
> look at patch-1,2 ?

Yes, only patch-3 is required to fix compilation failure.

Regards,
Anup

^ permalink raw reply

* Re: [PATCH v6 0/4] Broadcom SBA RAID support
From: Anup Patel @ 2017-03-29  6:05 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Jassi Brar, Rob Herring, Mark Rutland, Herbert Xu,
	David S . Miller, Dan Williams, Ray Jui, Scott Branden, Jon Mason,
	Rob Rice, BCM Kernel Feedback, dmaengine, Device Tree,
	Linux ARM Kernel, Linux Kernel, linux-crypto, linux-raid
In-Reply-To: <20170321091829.GZ2843@localhost>

On Tue, Mar 21, 2017 at 2:48 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Tue, Mar 21, 2017 at 02:17:21PM +0530, Anup Patel wrote:
>> On Tue, Mar 21, 2017 at 2:00 PM, Vinod Koul <vinod.koul@intel.com> wrote:
>> > On Mon, Mar 06, 2017 at 03:13:24PM +0530, Anup Patel wrote:
>> >> The Broadcom SBA RAID is a stream-based device which provides
>> >> RAID5/6 offload.
>> >>
>> >> It requires a SoC specific ring manager (such as Broadcom FlexRM
>> >> ring manager) to provide ring-based programming interface. Due to
>> >> this, the Broadcom SBA RAID driver (mailbox client) implements
>> >> DMA device having one DMA channel using a set of mailbox channels
>> >> provided by Broadcom SoC specific ring manager driver (mailbox
>> >> controller).
>> >>
>> >> The Broadcom SBA RAID hardware requires PQ disk position instead
>> >> of PQ disk coefficient. To address this, we have added raid_gflog
>> >> table which will help driver to convert PQ disk coefficient to PQ
>> >> disk position.
>> >>
>> >> This patchset is based on Linux-4.11-rc1 and depends on patchset
>> >> "[PATCH v5 0/2] Broadcom FlexRM ring manager support"
>> >
>> > Okay I applied and was about to push when I noticed this :(
>> >
>> > So what is the status of this..?
>>
>> PATCH2 is Acked but PATCH1 is under-review. Currently, its
>> v6 of that patchset.
>>
>> The only dependency on that patchset is the changes in
>> brcm-message.h which are required by this BCM-SBA-RAID
>> driver.
>>
>> @Jassi,
>> Can you please have a look at PATCH v6?
>
> And I would need an immutable branch/tag once merged. I am going to keep
> this series pending till then.

The Broadcom FlexRM patchset is pickedup by Jassi and
can be found in mailbox-for-next branch of
git://git.linaro.org/landing-teams/working/fujitsu/integration

Both patchset (Broadcom FlexRM patchset and this one) are
also available in sba-raid-v7 branch of
https://github.com/Broadcom/arm64-linux.git

Regards,
Anup

^ permalink raw reply

* Re: [PATCH 1/2] extcon: usb-gpio: Add level trigger support
From: Baolin Wang @ 2017-03-29  6:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Chanwoo Choi, MyungJoo Ham, Mark Rutland, Mark Brown,
	Linaro Kernel Mailman List, LKML,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqLOHP5fVcJDf+aMWFmPSHiBnHHfcPm4xu1CZJjWo9GtdQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi,

On 24 March 2017 at 20:41, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Mar 24, 2017 at 6:56 AM, Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> Hi,
>>
>> On 24 March 2017 at 19:08, Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>> Hi,
>>>
>>> On 2017년 03월 20일 16:59, Baolin Wang wrote:
>>>> Now extcon-usb-gpio only supports for GPIO egdge trigger, but VBUS/ID
>>>> gpios' detection can be triggered by the level trigger on some platforms.
>>>> Thus intoduce one property 'extcon-gpio,level-trigger' to identify this
>>>> situation.
>>>>
>>>> Signed-off-by: Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> ---
>>>>  .../devicetree/bindings/extcon/extcon-usb-gpio.txt |    3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>>>> index dfc14f7..191504b 100644
>>>> --- a/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>>>> +++ b/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt
>>>> @@ -9,6 +9,9 @@ Required properties:
>>>>  Either one of id-gpio or vbus-gpio must be present. Both can be present as well.
>>>>  - id-gpio: gpio for USB ID pin. See gpio binding.
>>>>  - vbus-gpio: gpio for USB VBUS pin.
>>>> +- extcon-gpio,level-trigger: Boolean, set this gpio's interrupt flag to
>>>
>>> It is 'extcon-usb-gpio' instead of 'extcon-gpio'.
>>
>> Sure.
>
> "extcon-gpio" is not a vendor, so don't make it a vendor prefix.

OK.

>
>>>> +level trigger. If not specified defaults to false, gpio's interrupt flag
>>>> +defaults to edge trigger.
>>>
>>> I understand why you need the new flag.
>>> But, I'm not sure it is right way to add the new flag for interrupt flag.
>>>
>>> IMHO, I think that we need to find the more proper way to get the interrupt flag
>>> or maybe, we may need to implement the new helper api to get the interrupt
>>> flag for gpio pin when we use the gpio as the interrupt source,
>>
>> Yes, I agree with you. We already have helper functions to get
>> interrupt flag from device tree, but we can not get the irq number of
>> GPIO to configure in device tree, then I introduce one new flag to
>> indicate the trigger type. But I like to change the patch if there are
>> any good suggestion. Thanks.
>
> The extcon binding needs an overhaul in general. I'm not going to take
> extensions to a broken binding. What's needed is a USB connector
> binding like we have for HDMI and other video connectors. And for
> USB-C, those need to be combined.

I am not sure I understand your points correctly, could you elaborate
on how to modify for extcon? Thanks.

-- 
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [alsa-devel] [PATCH v2 6/8] regulator: arizona-micbias: Add regulator driver for Arizona micbiases
From: kbuild test robot @ 2017-03-29  6:18 UTC (permalink / raw)
  To: Charles Keepax
  Cc: kbuild-all-JC7UmRfGjtg, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	patches-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, lee.jones-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1490710484-25277-6-git-send-email-ckeepax-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

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

Hi Charles,

[auto build test WARNING on regulator/for-next]
[also build test WARNING on v4.11-rc4 next-20170328]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Charles-Keepax/regulator-arizona-micsupp-Avoid-potential-memory-leak-reading-init_data/20170329-112224
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
config: i386-randconfig-x0-03291136 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/err.h:4:0,
                    from include/linux/clk.h:15,
                    from drivers//mfd/arizona-core.c:13:
   drivers//mfd/arizona-core.c: In function 'arizona_dev_init':
   drivers//mfd/arizona-core.c:1408:33: error: 'struct arizona_micbias' has no member named 'mV'
      if (!arizona->pdata.micbias[i].mV &&
                                    ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers//mfd/arizona-core.c:1408:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV &&
      ^~
   drivers//mfd/arizona-core.c:1409:33: error: 'struct arizona_micbias' has no member named 'bypass'
          !arizona->pdata.micbias[i].bypass)
                                    ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers//mfd/arizona-core.c:1408:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV &&
      ^~
   drivers//mfd/arizona-core.c:1408:33: error: 'struct arizona_micbias' has no member named 'mV'
      if (!arizona->pdata.micbias[i].mV &&
                                    ^
   include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers//mfd/arizona-core.c:1408:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV &&
      ^~
   drivers//mfd/arizona-core.c:1409:33: error: 'struct arizona_micbias' has no member named 'bypass'
          !arizona->pdata.micbias[i].bypass)
                                    ^
   include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers//mfd/arizona-core.c:1408:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV &&
      ^~
   drivers//mfd/arizona-core.c:1408:33: error: 'struct arizona_micbias' has no member named 'mV'
      if (!arizona->pdata.micbias[i].mV &&
                                    ^
   include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers//mfd/arizona-core.c:1408:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV &&
      ^~
   drivers//mfd/arizona-core.c:1409:33: error: 'struct arizona_micbias' has no member named 'bypass'
          !arizona->pdata.micbias[i].bypass)
                                    ^
   include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers//mfd/arizona-core.c:1408:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV &&
      ^~
   drivers//mfd/arizona-core.c:1413:33: error: 'struct arizona_micbias' has no member named 'mV'
      if (!arizona->pdata.micbias[i].mV)
                                    ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
   drivers//mfd/arizona-core.c:1413:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV)
      ^~
   drivers//mfd/arizona-core.c:1413:33: error: 'struct arizona_micbias' has no member named 'mV'
      if (!arizona->pdata.micbias[i].mV)
                                    ^
   include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
   drivers//mfd/arizona-core.c:1413:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV)
      ^~
   drivers//mfd/arizona-core.c:1413:33: error: 'struct arizona_micbias' has no member named 'mV'
      if (!arizona->pdata.micbias[i].mV)
                                    ^
   include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
   drivers//mfd/arizona-core.c:1413:3: note: in expansion of macro 'if'
      if (!arizona->pdata.micbias[i].mV)
      ^~
   drivers//mfd/arizona-core.c:1414:29: error: 'struct arizona_micbias' has no member named 'mV'
       arizona->pdata.micbias[i].mV = 2800;
                                ^
   drivers//mfd/arizona-core.c:1416:35: error: 'struct arizona_micbias' has no member named 'mV'
      val = (arizona->pdata.micbias[i].mV - 1500) / 100;
                                      ^
   In file included from include/linux/err.h:4:0,
                    from include/linux/clk.h:15,
                    from drivers//mfd/arizona-core.c:13:
   drivers//mfd/arizona-core.c:1423:32: error: 'struct arizona_micbias' has no member named 'discharge'
      if (arizona->pdata.micbias[i].discharge)
                                   ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
   drivers//mfd/arizona-core.c:1423:3: note: in expansion of macro 'if'
      if (arizona->pdata.micbias[i].discharge)
      ^~
   drivers//mfd/arizona-core.c:1423:32: error: 'struct arizona_micbias' has no member named 'discharge'
      if (arizona->pdata.micbias[i].discharge)
                                   ^
   include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
   drivers//mfd/arizona-core.c:1423:3: note: in expansion of macro 'if'
      if (arizona->pdata.micbias[i].discharge)
      ^~
   drivers//mfd/arizona-core.c:1423:32: error: 'struct arizona_micbias' has no member named 'discharge'
      if (arizona->pdata.micbias[i].discharge)
                                   ^
   include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
   drivers//mfd/arizona-core.c:1423:3: note: in expansion of macro 'if'
      if (arizona->pdata.micbias[i].discharge)
      ^~
   drivers//mfd/arizona-core.c:1426:32: error: 'struct arizona_micbias' has no member named 'soft_start'
      if (arizona->pdata.micbias[i].soft_start)
                                   ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
   drivers//mfd/arizona-core.c:1426:3: note: in expansion of macro 'if'
      if (arizona->pdata.micbias[i].soft_start)
      ^~
   drivers//mfd/arizona-core.c:1426:32: error: 'struct arizona_micbias' has no member named 'soft_start'
      if (arizona->pdata.micbias[i].soft_start)
                                   ^
   include/linux/compiler.h:160:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
   drivers//mfd/arizona-core.c:1426:3: note: in expansion of macro 'if'
      if (arizona->pdata.micbias[i].soft_start)
      ^~
   drivers//mfd/arizona-core.c:1426:32: error: 'struct arizona_micbias' has no member named 'soft_start'
      if (arizona->pdata.micbias[i].soft_start)
                                   ^
   include/linux/compiler.h:171:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
   drivers//mfd/arizona-core.c:1426:3: note: in expansion of macro 'if'
      if (arizona->pdata.micbias[i].soft_start)
      ^~
   drivers//mfd/arizona-core.c:1429:32: error: 'struct arizona_micbias' has no member named 'bypass'
      if (arizona->pdata.micbias[i].bypass)
                                   ^
   include/linux/compiler.h:160:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
   drivers//mfd/arizona-core.c:1429:3: note: in expansion of macro 'if'
      if (arizona->pdata.micbias[i].bypass)

vim +/if +1408 drivers//mfd/arizona-core.c

3cc72986 Mark Brown 2012-06-19  1392  				   ARIZONA_CLK_32K_SRC_MASK,
3cc72986 Mark Brown 2012-06-19  1393  				   arizona->pdata.clk32k_src - 1);
767c6dc0 Mark Brown 2013-03-19  1394  		arizona_clk32k_enable(arizona);
3cc72986 Mark Brown 2012-06-19  1395  		break;
3cc72986 Mark Brown 2012-06-19  1396  	case ARIZONA_32KZ_NONE:
3cc72986 Mark Brown 2012-06-19  1397  		regmap_update_bits(arizona->regmap, ARIZONA_CLOCK_32K_1,
3cc72986 Mark Brown 2012-06-19  1398  				   ARIZONA_CLK_32K_SRC_MASK, 2);
3cc72986 Mark Brown 2012-06-19  1399  		break;
3cc72986 Mark Brown 2012-06-19  1400  	default:
3cc72986 Mark Brown 2012-06-19  1401  		dev_err(arizona->dev, "Invalid 32kHz clock source: %d\n",
3cc72986 Mark Brown 2012-06-19  1402  			arizona->pdata.clk32k_src);
3cc72986 Mark Brown 2012-06-19  1403  		ret = -EINVAL;
59db9691 Mark Brown 2012-07-09  1404  		goto err_reset;
3cc72986 Mark Brown 2012-06-19  1405  	}
3cc72986 Mark Brown 2012-06-19  1406  
3d91f828 Mark Brown 2013-01-29  1407  	for (i = 0; i < ARIZONA_MAX_MICBIAS; i++) {
544c7aad Mark Brown 2013-01-29 @1408  		if (!arizona->pdata.micbias[i].mV &&
544c7aad Mark Brown 2013-01-29  1409  		    !arizona->pdata.micbias[i].bypass)
3d91f828 Mark Brown 2013-01-29  1410  			continue;
3d91f828 Mark Brown 2013-01-29  1411  
544c7aad Mark Brown 2013-01-29  1412  		/* Apply default for bypass mode */
544c7aad Mark Brown 2013-01-29  1413  		if (!arizona->pdata.micbias[i].mV)
544c7aad Mark Brown 2013-01-29  1414  			arizona->pdata.micbias[i].mV = 2800;
544c7aad Mark Brown 2013-01-29  1415  
3d91f828 Mark Brown 2013-01-29  1416  		val = (arizona->pdata.micbias[i].mV - 1500) / 100;

:::::: The code at line 1408 was first introduced by commit
:::::: 544c7aadd7d4309ed01fcd787d393db67eb7eaea mfd: arizona: Support configuring MICBIASes into bypass mode

:::::: TO: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
:::::: CC: Samuel Ortiz <sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

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

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

^ permalink raw reply

* Re: [PATCH v2,1/3] meson_uart: expose CLKID_UARTx
From: Jerome Brunet @ 2017-03-29  6:21 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: Helmut Klein, mturquette, sboyd, devicetree, linux-clk,
	linux-kernel, linux-arm-kernel, linux-amlogic
In-Reply-To: <CAFBinCDF29+kQpedsJPqRy6fztWrj91Qi6tCno_2S6UCkhW7XQ@mail.gmail.com>

On Tue, 2017-03-28 at 23:24 +0200, Martin Blumenstingl wrote:
> Hi Jerome,
> 
> On Tue, Mar 28, 2017 at 9:14 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> > On Tue, 2017-03-28 at 20:18 +0200, Helmut Klein wrote:
> > > i know for sure that the bluetooth chip of my system is connected to
> > > uart_A. so this clock must be exposed.
> > > 
> > > i don't know if the other 2 uarts are used on other hardware. so i will
> > > remove them from my patch.
> > 
> > What I meant is device trees "upstream".
> > I would expect such patch as part of a series to add support for your board
> > in
> > device-tree, with its bluetooth chipset.
> 
> are you sure about this? 

Actually no ;) I started questioning this reply as soon as I sent it.
the moto so far as been "we expose what's needed" but there is many ways to
interpret that.

DT bindings is an ABI. Simply because we don't use part of that ABI upstream,
doesn't mean we shouldn't provide it. My previous statement was clearly wrong
about this, apologies.

> Helmut is configuring the core clock of the
> UART controller(s) here. we have the same thing for many other drivers
> as well (MMC, SAR ADC, I2C, SPI, ethernet, you name it...) - these
> clocks are not part of a device specific .dts but rather the SoC
> specific dts (for example meson-gxbb.dtsi - because these clocks are
> not board specific).

Agreed
Since Helmut tested it, this clock should be added to the dtsi.

> I guess most boards are not affected because the bootloader simply
> enables the UART0 core/gate clock and keeps it enabled when booting
> the kernel. additionally our clock gates are marked with
> CLK_IGNORE_UNUSED so if the bootloader keeps the gates enabled then
> we're not disabling it either.
> 
> > I think it would better to drop this patch from the series.
> > You can either keep it for your personal work, or send it again when
> > upstreaming
> > the support for your board and/or add the support for the bluetooth chip.
> 
> if we decide to pass the core/gate clock directly in SoC.dtsi then we
> should think about doing it for all three non-AO UARTs (uart_a, uart_b
> and uart_c).
> we can test uart_b on GPIODV_24 and GPIODV_25 on the Khadas VIM board
> for example (pins 22 and 23 on the header, default mode of these pins
> is i2c_sck_a and i2c_sda_a, but we can re-configure it).
> uart_c unfortunately cannot be tested on the Khadas VIM board since
> it's only routed to GPIOX_8 and GPIOX_9 which are hard-wired to the
> bluetooth module's PCM pins (BTPCM_DOUT and BTPCM_DIN).
> 
> for Helmut this would mean that instead of dropping this patch (or
> dropping CLKID_UART1 and CLKID_UART2 from this patch) he would rather
> have to *add* another patch (for meson-gxbb.dtsi and meson-gxl.dtsi)
> which passes the core clocks to the corresponding UART controllers
> (similar to the CLKID_SD_EMMC_ clocks).

You are right. This could be part of another series.
I guess this patch fine as it is.

Acked-by: Jerome Brunet <jbrunet@baylibre.com>

> 
> 
> Regards,
> Martin
> 
> 
> [0] http://lxr.free-electrons.com/source/drivers/clk/meson/clkc.h?v=4.10#L110

^ permalink raw reply

* [PATCH v3 0/2] add dts files for hi3798cv200-Poplar board
From: Jiancheng Xue @ 2017-03-29  6:30 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
	arnd-r2nGTMty4D4, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, elder-QSEj5FYQhm4dnm+yROfE0A,
	peter.griffin-QSEj5FYQhm4dnm+yROfE0A, afaerber-l3A5Bk7waGM,
	yanhaifeng-C8/M+/jPZTeaMJb+Lgu22Q,
	hermit.wangheming-C8/M+/jPZTeaMJb+Lgu22Q, Jiancheng Xue

This patch set mainly adds dts files for hi3798cv200-Poplar board.

Jiancheng Xue (2):
  dt-bindings: arm: hisilicon: add bindings for hi3798cv200 SoC and
    Poplar board
  arm64: dts: hisilicon: add dts files for hi3798cv200-poplar board

 .../bindings/arm/hisilicon/hisilicon.txt           |   4 +
 arch/arm64/boot/dts/hisilicon/Makefile             |   1 +
 .../boot/dts/hisilicon/hi3798cv200-poplar.dts      | 163 +++++++++
 arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi     | 405 +++++++++++++++++++++
 4 files changed, 573 insertions(+)
 create mode 100644 arch/arm64/boot/dts/hisilicon/hi3798cv200-poplar.dts
 create mode 100644 arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3 1/2] dt-bindings: arm: hisilicon: add bindings for hi3798cv200 SoC and Poplar board
From: Jiancheng Xue @ 2017-03-29  6:30 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
	arnd-r2nGTMty4D4, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, elder-QSEj5FYQhm4dnm+yROfE0A,
	peter.griffin-QSEj5FYQhm4dnm+yROfE0A, afaerber-l3A5Bk7waGM,
	yanhaifeng-C8/M+/jPZTeaMJb+Lgu22Q,
	hermit.wangheming-C8/M+/jPZTeaMJb+Lgu22Q, Jiancheng Xue
In-Reply-To: <1490769009-12552-1-git-send-email-xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>

Add bindings for HiSilicon hi3798cv200 SoC and Poplar Board.

Signed-off-by: Jiancheng Xue <xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
Reviewed-by: Alex Elder <elder-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
index f1c1e21..1fd3dd7 100644
--- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
+++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
@@ -4,6 +4,10 @@ Hi3660 SoC
 Required root node properties:
 	- compatible = "hisilicon,hi3660";
 
+Hi3798cv200 Poplar Board
+Required root node properties:
+	- compatible = "hisilicon,hi3798cv200-poplar", "hisilicon,hi3798cv200";
+
 Hi4511 Board
 Required root node properties:
 	- compatible = "hisilicon,hi3620-hi4511";
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 2/2] arm64: dts: hisilicon: add dts files for hi3798cv200-poplar board
From: Jiancheng Xue @ 2017-03-29  6:30 UTC (permalink / raw)
  To: robh+dt, xuwei5, arnd, catalin.marinas, will.deacon
  Cc: devicetree, linux-arm-kernel, linux-kernel, elder, peter.griffin,
	afaerber, yanhaifeng, hermit.wangheming, Jiancheng Xue
In-Reply-To: <1490769009-12552-1-git-send-email-xuejiancheng@hisilicon.com>

Add basic dts files for hi3798cv200-poplar board. Poplar is the
first development board compliant with the 96Boards Enterprise
Edition TV Platform specification. The board features the
Hi3798CV200 with an integrated quad-core 64-bit ARM Cortex A53
processor and high performance Mali T720 GPU.

Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com>
Reviewed-by: Alex Elder <elder@linaro.org>
Acked-by: Peter Griffin <peter.griffin@linaro.org>
---
Changed Log:
v3:
- Refined the patch according to Andreas's suggestions.
  1. Changed the license.
  2. Added alias for uart2.
  3. Reordered the device nodes. 
v2:
- Fixed issues pointed by Rob Herring.
  1. Moved the led node out of the soc node.
  2. Restrained the ranges property of soc node smaller.
- Refined the patch according to Andreas's suggestions.
- Enabled gmac1 device node instead of gmac0.
- Added a compatible string "syscon" for crg nodes.

 arch/arm64/boot/dts/hisilicon/Makefile             |   1 +
 .../boot/dts/hisilicon/hi3798cv200-poplar.dts      | 163 +++++++++
 arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi     | 405 +++++++++++++++++++++
 3 files changed, 569 insertions(+)
 create mode 100644 arch/arm64/boot/dts/hisilicon/hi3798cv200-poplar.dts
 create mode 100644 arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi

diff --git a/arch/arm64/boot/dts/hisilicon/Makefile b/arch/arm64/boot/dts/hisilicon/Makefile
index c3a6c19..8960eca 100644
--- a/arch/arm64/boot/dts/hisilicon/Makefile
+++ b/arch/arm64/boot/dts/hisilicon/Makefile
@@ -1,4 +1,5 @@
 dtb-$(CONFIG_ARCH_HISI) += hi3660-hikey960.dtb
+dtb-$(CONFIG_ARCH_HISI) += hi3798cv200-poplar.dtb
 dtb-$(CONFIG_ARCH_HISI) += hi6220-hikey.dtb
 dtb-$(CONFIG_ARCH_HISI) += hip05-d02.dtb
 dtb-$(CONFIG_ARCH_HISI) += hip06-d03.dtb
diff --git a/arch/arm64/boot/dts/hisilicon/hi3798cv200-poplar.dts b/arch/arm64/boot/dts/hisilicon/hi3798cv200-poplar.dts
new file mode 100644
index 0000000..fe2f9f1
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hi3798cv200-poplar.dts
@@ -0,0 +1,163 @@
+/*
+ * DTS File for HiSilicon Poplar Development Board
+ *
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include "hi3798cv200.dtsi"
+
+/ {
+	model = "HiSilicon Poplar Development Board";
+	compatible = "hisilicon,hi3798cv200-poplar", "hisilicon,hi3798cv200";
+
+	aliases {
+		serial0 = &uart0;
+		serial2 = &uart2;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x80000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		user-led0 {
+			label = "USER-LED0";
+			gpios = <&gpio6 3 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "heartbeat";
+			default-state = "off";
+		};
+
+		user-led1 {
+			label = "USER-LED1";
+			gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "mmc0";
+			default-state = "off";
+		};
+
+		user-led2 {
+			label = "USER-LED2";
+			gpios = <&gpio5 2 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "none";
+			default-state = "off";
+		};
+
+		user-led3 {
+			label = "USER-LED3";
+			gpios = <&gpio10 6 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "cpu0";
+			default-state = "off";
+		};
+	};
+};
+
+&gmac1 {
+	status = "okay";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	phy-handle = <&eth_phy1>;
+	phy-mode = "rgmii";
+	hisilicon,phy-reset-delays-us = <10000 10000 30000>;
+
+	eth_phy1: phy@3 {
+		reg = <3>;
+	};
+};
+
+&gpio1 {
+	status = "okay";
+	gpio-line-names = "LS-GPIO-E",	"",
+			  "",		"",
+			  "",		"LS-GPIO-F",
+			  "",		"LS-GPIO-J";
+};
+
+&gpio2 {
+	status = "okay";
+	gpio-line-names = "LS-GPIO-H",	"LS-GPIO-I",
+			  "LS-GPIO-L",	"LS-GPIO-G",
+			  "LS-GPIO-K",	"",
+			  "",		"";
+};
+
+&gpio3 {
+	status = "okay";
+	gpio-line-names = "",		"",
+			  "",		"",
+			  "LS-GPIO-C",	"",
+			  "",		"LS-GPIO-B";
+};
+
+&gpio4 {
+	status = "okay";
+	gpio-line-names = "",		"",
+			  "",		"",
+			  "",		"LS-GPIO-D",
+			  "",		"";
+};
+
+&gpio5 {
+	status = "okay";
+	gpio-line-names = "",		"USER-LED-1",
+			  "USER-LED-2",	"",
+			  "",		"LS-GPIO-A",
+			  "",		"";
+};
+
+&gpio6 {
+	status = "okay";
+	gpio-line-names = "",		"",
+			  "",		"USER-LED-0",
+			  "",		"",
+			  "",		"";
+};
+
+&gpio10 {
+	status = "okay";
+	gpio-line-names = "",		"",
+			  "",		"",
+			  "",		"",
+			  "USER-LED-3",	"";
+};
+
+&i2c0 {
+	status = "okay";
+	label = "LS-I2C0";
+};
+
+&i2c2 {
+	status = "okay";
+	label = "LS-I2C1";
+};
+
+&ir {
+	status = "okay";
+};
+
+&spi0 {
+	status = "okay";
+	label = "LS-SPI0";
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+	label = "LS-UART0";
+};
+/* No optional LS-UART1 on Low Speed Expansion Connector. */
+
diff --git a/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi
new file mode 100644
index 0000000..e94d6f6
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi
@@ -0,0 +1,405 @@
+/*
+ * DTS File for HiSilicon Hi3798cv200 SoC.
+ *
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <dt-bindings/clock/histb-clock.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/reset/ti-syscon.h>
+
+/ {
+	compatible = "hisilicon,hi3798cv200";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			compatible = "arm,cortex-a53";
+			device_type = "cpu";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+		};
+
+		cpu@1 {
+			compatible = "arm,cortex-a53";
+			device_type = "cpu";
+			reg = <0x0 0x1>;
+			enable-method = "psci";
+		};
+
+		cpu@2 {
+			compatible = "arm,cortex-a53";
+			device_type = "cpu";
+			reg = <0x0 0x2>;
+			enable-method = "psci";
+		};
+
+		cpu@3 {
+			compatible = "arm,cortex-a53";
+			device_type = "cpu";
+			reg = <0x0 0x3>;
+			enable-method = "psci";
+		};
+	};
+
+	gic: interrupt-controller@f1001000 {
+		compatible = "arm,gic-400";
+		reg = <0x0 0xf1001000 0x0 0x1000>,  /* GICD */
+		      <0x0 0xf1002000 0x0 0x100>;   /* GICC */
+		#address-cells = <0>;
+		#interrupt-cells = <3>;
+		interrupt-controller;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	soc: soc@f0000000 {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x0 0x0 0xf0000000 0x10000000>;
+
+		crg: clock-reset-controller@8a22000 {
+			compatible = "hisilicon,hi3798cv200-crg", "syscon", "simple-mfd";
+			reg = <0x8a22000 0x1000>;
+			#clock-cells = <1>;
+			#reset-cells = <2>;
+
+			gmacphyrst: reset-controller {
+				compatible = "ti,syscon-reset";
+				#reset-cells = <1>;
+				ti,reset-bits =
+					<0xcc 12 0xcc 12 0 0 (ASSERT_CLEAR|DEASSERT_SET|STATUS_NONE)>,  /* 0: gmac0-phy-rst */
+					<0xcc 13 0xcc 13 0 0 (ASSERT_CLEAR|DEASSERT_SET|STATUS_NONE)>;  /* 1: gmac1-phy-rst */
+			};
+		};
+
+		sysctrl: system-controller@8000000 {
+			compatible = "hisilicon,hi3798cv200-sysctrl", "syscon";
+			reg = <0x8000000 0x1000>;
+			#clock-cells = <1>;
+			#reset-cells = <2>;
+		};
+
+		uart0: serial@8b00000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x8b00000 0x1000>;
+			interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sysctrl HISTB_UART0_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		uart2: serial@8b02000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x8b02000 0x1000>;
+			interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HISTB_UART2_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		i2c0: i2c@8b10000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b10000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C0_CLK>;
+			status = "disabled";
+		};
+
+		i2c1: i2c@8b11000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b11000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C1_CLK>;
+			status = "disabled";
+		};
+
+		i2c2: i2c@8b12000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b12000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C2_CLK>;
+			status = "disabled";
+		};
+
+		i2c3: i2c@8b13000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b13000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C3_CLK>;
+			status = "disabled";
+		};
+
+		i2c4: i2c@8b14000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b14000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C4_CLK>;
+			status = "disabled";
+		};
+
+		spi0: spi@8b1a000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x8b1a000 0x1000>;
+			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
+			num-cs = <1>;
+			cs-gpios = <&gpio7 1 0>;
+			clocks = <&crg HISTB_SPI0_CLK>;
+			clock-names = "apb_pclk";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		emmc: mmc@9830000 {
+			compatible = "snps,dw-mshc";
+			reg = <0x9830000 0x10000>;
+			interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HISTB_MMC_CIU_CLK>,
+				 <&crg HISTB_MMC_BIU_CLK>;
+			clock-names = "ciu", "biu";
+		};
+
+		gpio0: gpio@8b20000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b20000 0x1000>;
+			interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio1: gpio@8b21000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b21000 0x1000>;
+			interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio2: gpio@8b22000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b22000 0x1000>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio3: gpio@8b23000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b23000 0x1000>;
+			interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio4: gpio@8b24000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b24000 0x1000>;
+			interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio5: gpio@8004000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8004000 0x1000>;
+			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio6: gpio@8b26000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b26000 0x1000>;
+			interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio7: gpio@8b27000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b27000 0x1000>;
+			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio8: gpio@8b28000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b28000 0x1000>;
+			interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio9: gpio@8b29000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b29000 0x1000>;
+			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio10: gpio@8b2a000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b2a000 0x1000>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio11: gpio@8b2b000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b2b000 0x1000>;
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio12: gpio@8b2c000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b2c000 0x1000>;
+			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gmac0: ethernet@9840000 {
+			compatible = "hisilicon,hi3798cv200-gmac", "hisilicon,hisi-gmac-v2";
+			reg = <0x9840000 0x1000>,
+			      <0x984300c 0x4>;
+			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HISTB_ETH0_MAC_CLK>,
+				 <&crg HISTB_ETH0_MACIF_CLK>;
+			clock-names = "mac_core", "mac_ifc";
+			resets = <&crg 0xcc 8>,
+				 <&crg 0xcc 10>,
+				 <&gmacphyrst 0>;
+			reset-names = "mac_core", "mac_ifc", "phy";
+			status = "disabled";
+		};
+
+		gmac1: ethernet@9841000 {
+			compatible = "hisilicon,hi3798cv200-gmac", "hisilicon,hisi-gmac-v2";
+			reg = <0x9841000 0x1000>,
+			      <0x9843010 0x4>;
+			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HISTB_ETH1_MAC_CLK>,
+				 <&crg HISTB_ETH1_MACIF_CLK>;
+			clock-names = "mac_core", "mac_ifc";
+			resets = <&crg 0xcc 9>,
+				 <&crg 0xcc 11>,
+				 <&gmacphyrst 1>;
+			reset-names = "mac_core", "mac_ifc", "phy";
+			status = "disabled";
+		};
+
+		ir: ir@8001000 {
+			compatible = "hisilicon,hix5hd2-ir";
+			reg = <0x8001000 0x1000>;
+			interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sysctrl HISTB_IR_CLK>;
+			status = "disabled";
+		};
+	};
+};
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v2 2/7] rtc: Add rtc-sh
From: Geert Uytterhoeven @ 2017-03-29  6:49 UTC (permalink / raw)
  To: Rob Herring
  Cc: Chris Brandt, Alessandro Zummo, Alexandre Belloni, Mark Rutland,
	Simon Horman, RTCLINUX,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Renesas
In-Reply-To: <20170329012441.bxnkyqui24mymuvr@rob-hp-laptop>

Hi Rob,

On Wed, Mar 29, 2017 at 3:24 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Mar 22, 2017 at 10:27:49AM -0400, Chris Brandt wrote:
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/rtc/rtc-sh.txt
>> @@ -0,0 +1,29 @@
>> +* Real Time Clock for Renesas SH and ARM SoCs
>> +
>> +Required properties:
>> +- compatible: Should be "renesas,r7s72100-rtc" and "renesas,sh-rtc" as a
>> +  fallback.
>> +- reg: physical base address and length of memory mapped region.
>> +- interrupts: 3 interrupts for alarm, period, and carry.
>> +- interrupt-names: The interrupts should be labeled as "alarm", "period", and
>> +  "carry".
>> +- clocks: The functional clock source for the RTC controller must be listed
>> +  first (if exists). Additionally, potential clock counting sources are to be
>> +  listed.
>> +- clock-names: The functional clock must be labeled as "fck". Other clocks
>> +  may be named in accordance to the SoC hardware manuals.
>> +
>> +
>> +Example:
>> +rtc: rtc@fcff1000 {
>> +     compatible = "renesas,r7s72100-rtc", "renesas,sh-rtc";
>> +     reg = <0xfcff1000 0x2e>;
>> +     interrupts = <GIC_SPI 276 IRQ_TYPE_EDGE_RISING
>> +                   GIC_SPI 277 IRQ_TYPE_EDGE_RISING
>> +                   GIC_SPI 278 IRQ_TYPE_EDGE_RISING>;
>> +     interrupt-names = "alarm", "period", "carry";
>> +     clocks = <&mstp6_clks R7S72100_CLK_RTC>, <&rtc_x1_clk>,
>> +              <&rtc_x3_clk>, <&extal_clk>;
>> +     clock-names = "fck", "rtc_x1", "rtc_x3", "extal";
>> +     power-domains = <&cpg_clocks>;
>
> Not documented.

"power-domains" is a platform property.

All hardware components need power.
All synchronous hardware components need a clock.
Most hardware components have a reset signal.

Whether these are exposed and can be controlled depends on the platform/SoC.
So documenting them in each and every device binding looks overkill to me.
I think this is something to be addressed by devicetree-specification (which
doesn't handle clocks, power-domains, resets yet).

If you prefer, the property can be removed from the example, though.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH 2/3] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
From: Quentin Schulz @ 2017-03-29  6:54 UTC (permalink / raw)
  To: Icenowy Zheng, Lee Jones, Rob Herring, Maxime Ripard,
	Chen-Yu Tsai, Jonathan Cameron, Zhang Rui
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170328173013.16539-3-icenowy-h8G6r0blFSE@public.gmane.org>

Hi,

On 28/03/2017 19:30, Icenowy Zheng wrote:
> This adds support for the Allwinner H3 thermal sensor.
> 
> Allwinner H3 has a thermal sensor like the one in A33, but have its
> registers nearly all re-arranged, sample clock moved to CCU and a pair
> of bus clock and reset added. It's also the base of newer SoCs' thermal
> sensors.
> 
> An option is added to gpadc_data struct, to indicate whether this device
> is a new-generation Allwinner thermal sensor.
> 
> The thermal sensors on A64 and H5 is like the one on H3, but with of
> course different formula factors.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  drivers/iio/adc/sun4i-gpadc-iio.c | 130 ++++++++++++++++++++++++++++++++------
>  include/linux/mfd/sun4i-gpadc.h   |  33 +++++++++-
>  2 files changed, 141 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 74705aa37982..7512b1cae877 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
> @@ -22,6 +22,7 @@
>   * shutdown for not being used.
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -31,6 +32,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
> +#include <linux/reset.h>
>  #include <linux/thermal.h>
>  #include <linux/delay.h>
>  
> @@ -56,6 +58,7 @@ struct gpadc_data {
>  	unsigned int	tp_adc_select;
>  	unsigned int	(*adc_chan_select)(unsigned int chan);
>  	unsigned int	adc_chan_mask;
> +	bool		gen2_ths;
>  };
>  

Instead of a boolean, give the TEMP_DATA register address.

>  static const struct gpadc_data sun4i_gpadc_data = {
> @@ -88,7 +91,20 @@ static const struct gpadc_data sun6i_gpadc_data = {
>  static const struct gpadc_data sun8i_a33_gpadc_data = {
>  	.temp_offset = -1662,
>  	.temp_scale = 162,
> -	.tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN,
> +	.tp_mode_en = SUN8I_A23_GPADC_CTRL1_CHOP_TEMP_EN,
> +};

Separate patch for this?

> +
> +static const struct gpadc_data sun8i_h3_gpadc_data = {
> +	/*
> +	 * The original formula on the datasheet seems to be wrong.
> +	 * These factors are calculated based on the formula in the BSP
> +	 * kernel, which is originally Tem = 217 - (T / 8.253), in which Tem
> +	 * is the temperature in Celsius degree and T is the raw value
> +	 * from the sensor.
> +	 */
> +	.temp_offset = -1791,
> +	.temp_scale = -121,
> +	.gen2_ths = true,
>  };
>  
>  struct sun4i_gpadc_iio {
> @@ -103,6 +119,9 @@ struct sun4i_gpadc_iio {
>  	atomic_t			ignore_temp_data_irq;
>  	const struct gpadc_data		*data;
>  	bool				no_irq;
> +	struct clk			*ths_bus_clk;
> +	struct clk			*ths_clk;
> +	struct reset_control		*reset;
>  	/* prevents concurrent reads of temperature and ADC */
>  	struct mutex			mutex;
>  };
> @@ -274,7 +293,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val)
>  	if (info->no_irq) {
>  		pm_runtime_get_sync(indio_dev->dev.parent);
>  
> -		regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
> +		if (info->data->gen2_ths)
> +			regmap_read(info->regmap, SUN8I_H3_GPADC_TEMP_DATA,
> +				    val);
> +		else
> +			regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val);
>  

Instead of gen2_ths, use the TEMP_DATA register address.

>  		pm_runtime_mark_last_busy(indio_dev->dev.parent);
>  		pm_runtime_put_autosuspend(indio_dev->dev.parent);
> @@ -386,10 +409,15 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev)
>  {
>  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>  
> -	/* Disable the ADC on IP */
> -	regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
> -	/* Disable temperature sensor on IP */
> -	regmap_write(info->regmap, SUN4I_GPADC_TPR, 0);
> +	if (info->data->gen2_ths) {
> +		/* Disable temperature sensor */
> +		regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL2, 0);
> +	} else {
> +		/* Disable the ADC on IP */
> +		regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0);
> +		/* Disable temperature sensor on IP */
> +		regmap_write(info->regmap, SUN4I_GPADC_TPR, 0);
> +	}
>  

Either use another register address or add a suspend function to struct
gpadc_data which will be different for each version of the IP.

>  	return 0;
>  }
> @@ -398,19 +426,36 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
>  {
>  	struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev));
>  
> -	/* clkin = 6MHz */
> -	regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
> -		     SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
> -		     SUN4I_GPADC_CTRL0_FS_DIV(7) |
> -		     SUN4I_GPADC_CTRL0_T_ACQ(63));
> -	regmap_write(info->regmap, SUN4I_GPADC_CTRL1, info->data->tp_mode_en);
> -	regmap_write(info->regmap, SUN4I_GPADC_CTRL3,
> -		     SUN4I_GPADC_CTRL3_FILTER_EN |
> -		     SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
> -	/* period = SUN4I_GPADC_TPR_TEMP_PERIOD * 256 * 16 / clkin; ~0.6s */
> -	regmap_write(info->regmap, SUN4I_GPADC_TPR,
> -		     SUN4I_GPADC_TPR_TEMP_ENABLE |
> -		     SUN4I_GPADC_TPR_TEMP_PERIOD(800));
> +	if (info->data->gen2_ths) {
> +		regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL2,
> +			     SUN8I_H3_GPADC_CTRL2_TEMP_SENSE_EN |
> +			     SUN8I_H3_GPADC_CTRL2_T_ACQ1(31));
> +		regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
> +			     SUN4I_GPADC_CTRL0_T_ACQ(31));
> +		regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL3,
> +			     SUN4I_GPADC_CTRL3_FILTER_EN |
> +			     SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
> +		regmap_write(info->regmap, SUN8I_H3_GPADC_INTC,
> +			     SUN8I_H3_GPADC_INTC_TEMP_PERIOD(800));
> +	} else {
> +		/* clkin = 6MHz */
> +		regmap_write(info->regmap, SUN4I_GPADC_CTRL0,
> +			     SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) |
> +			     SUN4I_GPADC_CTRL0_FS_DIV(7) |
> +			     SUN4I_GPADC_CTRL0_T_ACQ(63));
> +		regmap_write(info->regmap, SUN4I_GPADC_CTRL1,
> +			     info->data->tp_mode_en);
> +		regmap_write(info->regmap, SUN4I_GPADC_CTRL3,
> +			     SUN4I_GPADC_CTRL3_FILTER_EN |
> +			     SUN4I_GPADC_CTRL3_FILTER_TYPE(1));
> +		/*
> +		 * period = SUN4I_GPADC_TPR_TEMP_PERIOD * 256 * 16 / clkin;
> +		 * ~0.6s
> +		 */
> +		regmap_write(info->regmap, SUN4I_GPADC_TPR,
> +			     SUN4I_GPADC_TPR_TEMP_ENABLE |
> +			     SUN4I_GPADC_TPR_TEMP_PERIOD(800));
> +	}
>  

Same here as suspend function?

>  	return 0;
>  }
> @@ -494,6 +539,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = {
>  		.compatible = "allwinner,sun8i-a33-ths",
>  		.data = &sun8i_a33_gpadc_data,
>  	},
> +	{
> +		.compatible = "allwinner,sun8i-h3-ths",
> +		.data = &sun8i_h3_gpadc_data,
> +	},
>  	{ /* sentinel */ }
>  };
>  
> @@ -529,6 +578,43 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev,
>  		return ret;
>  	}
>  
> +	if (info->data->gen2_ths) {
> +		info->reset = devm_reset_control_get(&pdev->dev, NULL);
> +		if (IS_ERR(info->reset)) {
> +			ret = PTR_ERR(info->reset);
> +			return ret;
> +		}
> +
> +		ret = reset_control_deassert(info->reset);
> +		if (ret)
> +			return ret;
> +
> +		info->ths_bus_clk = devm_clk_get(&pdev->dev, "bus");
> +		if (IS_ERR(info->ths_bus_clk)) {
> +			ret = PTR_ERR(info->ths_bus_clk);
> +			return ret;
> +		}
> +
> +		ret = clk_prepare_enable(info->ths_bus_clk);
> +		if (ret)
> +			return ret;
> +
> +		info->ths_clk = devm_clk_get(&pdev->dev, "ths");
> +		if (IS_ERR(info->ths_clk)) {
> +			ret = PTR_ERR(info->ths_clk);
> +			return ret;
> +		}
> +
> +		/* Running at 6MHz */
> +		ret = clk_set_rate(info->ths_clk, 6000000);
> +		if (ret)
> +			return ret;
> +
> +		ret = clk_prepare_enable(info->ths_clk);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	if (!IS_ENABLED(CONFIG_THERMAL_OF))
>  		return 0;
>  
> @@ -691,6 +777,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
>  	if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
>  		iio_map_array_unregister(indio_dev);
>  
> +	if (info->data->gen2_ths) {
> +		clk_disable_unprepare(info->ths_clk);
> +		clk_disable_unprepare(info->ths_bus_clk);
> +		reset_control_deassert(info->reset);
> +	}
> +

I'm not really fond of using this boolean as I don't see it being
possibly reused for any other SoCs that has a GPADC or THS.

Here, you could make use of a list/array of clk which then can be reused
for other SoCs just by changing the list. Add a default rate to the
gpadc_data structure and you're go to go.

>  	return 0;
>  }
>  
> diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h
> index 139872c2e0fe..f794a2988a93 100644
> --- a/include/linux/mfd/sun4i-gpadc.h
> +++ b/include/linux/mfd/sun4i-gpadc.h
> @@ -38,9 +38,12 @@
>  #define SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(x)		(GENMASK(3, 0) & BIT(x))
>  #define SUN6I_GPADC_CTRL1_ADC_CHAN_MASK			GENMASK(3, 0)
>  
> -/* TP_CTRL1 bits for sun8i SoCs */
> -#define SUN8I_GPADC_CTRL1_CHOP_TEMP_EN			BIT(8)
> -#define SUN8I_GPADC_CTRL1_GPADC_CALI_EN			BIT(7)
> +/* TP_CTRL1 bits for sun8i A23/A33 SoCs */
> +#define SUN8I_A23_GPADC_CTRL1_CHOP_TEMP_EN		BIT(8)
> +#define SUN8I_A23_GPADC_CTRL1_GPADC_CALI_EN		BIT(7)
> +

Different patch for these?

Thanks,
Quentin
-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 01/18] dt-bindings: power: battery: add constant-charge-current property
From: Liam Breck @ 2017-03-29  6:55 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Sebastian Reichel, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAKvHMgRKroPFDdNMb=F=bA4bTwg9bYMAR-5Z4DKLzHJtCYz3_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Quentin,

On Thu, Mar 16, 2017 at 12:42 AM, Liam Breck <liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org> wrote:
> On Thu, Mar 16, 2017 at 12:03 AM, Quentin Schulz
> <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> Hi Liam,
>>
>> On 16/03/2017 07:27, Liam Breck wrote:
>>> Hi Rob,
>>>
>>> On Wed, Mar 15, 2017 at 6:10 AM, Quentin Schulz
>>> <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>> Hi Liam,
>>>>
>>>> On 15/03/2017 13:08, Liam Breck wrote:
>>>>> I dropped most of the CCs, pls re-add anyone essential. Use Cc lines
>>>>> in patch description to direct a patch to interested parties and
>>>>> relevant lists. I don't want to see all 19 patches in this series.
>>>>>
>>>>> On Wed, Mar 15, 2017 at 3:55 AM, Quentin Schulz
>>>>> <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>>> This adds the constant-charge-current property to the list of optional
>>>>>> properties of the battery.
>>>>>>
>>>>>> The constant charge current is critical for batteries as they can't
>>>>>> handle all charge currents.
>>>>>>
>>>>>> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>>> ---
>>>>>>
>>>>>> v4:
>>>>>>  - switch from constant-charge-current-microamp to constant-charge-microamp,
>>>
>>> Sebastian is on record supporting name alignment between DT:battery
>>> properties and enum power_supply_property.
>>>
>>> Could you OK a change back to constant-charge-current-microamp in this patch?
>>>
>>> Note that the following could appear in DT:battery in future:
>>>
>>> constant-charge-current-microamp
>>> constant-charge-current-max-microamp
>>
>> OK. I am actually setting max constant charge current and constant
>> current charge with the same DT property so I might need to adapt my
>> patches.
>
> I suspect you should set battery:*-current-max with DT, as -current is
> "programmed by charger" so seems to be a stat, not a fixed
> characteristic.

Did you resolve this issue?

Rob & Sebastian want me to merge all items for
bindings/power/supply/battery.txt. If I do that I'll also roll up
their counterparts in power_supply_core.c and submit a patchset for
those two files.

> You may program your charger's -current with the battery's
> -current-max, of course.
>
> Note that your charger also has a -current-max characteristic, likely
> different than your battery. You may not need that in DT tho.
>
>>> constant-charge-voltage-microvolt
>>> constant-charge-voltage-max-microvolt
>>>
>>> Thanks!
>>>
>>>>> Must be constant-charge-current-microamp for the reasons discussed in
>>>>> battery.txt - consistency with sysfs names.
>>>>>
>>>>
>>>> Hum. Just nitpicking, but I disagree with the use of 'must'. IIRC there
>>>> is nothing in the code that would require the property to be named after
>>>> a property from the enum power_supply_property.
>>>>
>>>> I would say that you _want_ it to be named like that because it
>>>> underlines the relation between the DT property and the actual impacted
>>>> property in the power supply subsystem. I'm fine with this reason but in
>>>> the end, the maintainer's opinion prevails (if (s)he does not want it,
>>>> (s)he will not take it). So, basically, I actually don't mind either
>>>> option and I see arguments on each side.
>>>>
>>>> So, just waiting for maintainer's opinion to make the final version of
>>>> this patch.
>>>>
>>>> Thanks,
>>>> Quentin
>>>>
>>>>> Also applies to power_supply_core.c patch.
>>>>>
>>>>> Rob: enum power_supply_property also lists constant-charge-voltage,
>>>>> hence the -current
>>>>>
>>>>>
>>>>>> added in v3
>>>>>>
>>>>>>  Documentation/devicetree/bindings/power/supply/battery.txt | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>>>>>> index 0278617..9594e1e 100644
>>>>>> --- a/Documentation/devicetree/bindings/power/supply/battery.txt
>>>>>> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>>>>>> @@ -7,6 +7,7 @@ Optional Properties:
>>>>>>   - voltage-min-design-microvolt: drained battery voltage
>>>>>>   - energy-full-design-microwatt-hours: battery design energy
>>>>>>   - charge-full-design-microamp-hours: battery design capacity
>>>>>> + - constant-charge-microamp: battery constant charge current
>>>>>>
>>>>>>  Because drivers surface properties in sysfs using names derived
>>>>>>  from enum power_supply_property, e.g.
>>>>>> @@ -30,6 +31,7 @@ Example:
>>>>>>                 voltage-min-design-microvolt = <3200000>;
>>>>>>                 energy-full-design-microwatt-hours = <5290000>;
>>>>>>                 charge-full-design-microamp-hours = <1430000>;
>>>>>> +               constant-charge-microamp = <300000>;
>>>>>>         };
>>>>>>
>>>>>>         charger: charger@11 {
>>>>>> --
>>>>>> 2.9.3
>>>>>>
>>>>
>>>> --
>>>> Quentin Schulz, Free Electrons
>>>> Embedded Linux and Kernel engineering
>>>> http://free-electrons.com
>>
>> --
>> Quentin Schulz, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC PATCH 2/3] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
From: Icenowy Zheng @ 2017-03-29  6:57 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Zhang Rui, linux-pm, Rob Herring, linux-kernel, linux-iio,
	devicetree, Lee Jones, Jonathan Cameron, Maxime Ripard,
	linux-arm-kernel, Chen-Yu Tsai


2017年3月29日 14:54于 Quentin Schulz <quentin.schulz@free-electrons.com>写道:
>
> Hi, 
>
> On 28/03/2017 19:30, Icenowy Zheng wrote: 
> > This adds support for the Allwinner H3 thermal sensor. 
> > 
> > Allwinner H3 has a thermal sensor like the one in A33, but have its 
> > registers nearly all re-arranged, sample clock moved to CCU and a pair 
> > of bus clock and reset added. It's also the base of newer SoCs' thermal 
> > sensors. 
> > 
> > An option is added to gpadc_data struct, to indicate whether this device 
> > is a new-generation Allwinner thermal sensor. 
> > 
> > The thermal sensors on A64 and H5 is like the one on H3, but with of 
> > course different formula factors. 
> > 
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> 
> > --- 
> >  drivers/iio/adc/sun4i-gpadc-iio.c | 130 ++++++++++++++++++++++++++++++++------ 
> >  include/linux/mfd/sun4i-gpadc.h   |  33 +++++++++- 
> >  2 files changed, 141 insertions(+), 22 deletions(-) 
> > 
> > diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c 
> > index 74705aa37982..7512b1cae877 100644 
> > --- a/drivers/iio/adc/sun4i-gpadc-iio.c 
> > +++ b/drivers/iio/adc/sun4i-gpadc-iio.c 
> > @@ -22,6 +22,7 @@ 
> >   * shutdown for not being used. 
> >   */ 
> >  
> > +#include <linux/clk.h> 
> >  #include <linux/completion.h> 
> >  #include <linux/interrupt.h> 
> >  #include <linux/io.h> 
> > @@ -31,6 +32,7 @@ 
> >  #include <linux/platform_device.h> 
> >  #include <linux/pm_runtime.h> 
> >  #include <linux/regmap.h> 
> > +#include <linux/reset.h> 
> >  #include <linux/thermal.h> 
> >  #include <linux/delay.h> 
> >  
> > @@ -56,6 +58,7 @@ struct gpadc_data { 
> >  unsigned int tp_adc_select; 
> >  unsigned int (*adc_chan_select)(unsigned int chan); 
> >  unsigned int adc_chan_mask; 
> > + bool gen2_ths; 
> >  }; 
> >  
>
> Instead of a boolean, give the TEMP_DATA register address. 
>
> >  static const struct gpadc_data sun4i_gpadc_data = { 
> > @@ -88,7 +91,20 @@ static const struct gpadc_data sun6i_gpadc_data = { 
> >  static const struct gpadc_data sun8i_a33_gpadc_data = { 
> >  .temp_offset = -1662, 
> >  .temp_scale = 162, 
> > - .tp_mode_en = SUN8I_GPADC_CTRL1_CHOP_TEMP_EN, 
> > + .tp_mode_en = SUN8I_A23_GPADC_CTRL1_CHOP_TEMP_EN, 
> > +}; 
>
> Separate patch for this? 
>
> > + 
> > +static const struct gpadc_data sun8i_h3_gpadc_data = { 
> > + /* 
> > + * The original formula on the datasheet seems to be wrong. 
> > + * These factors are calculated based on the formula in the BSP 
> > + * kernel, which is originally Tem = 217 - (T / 8.253), in which Tem 
> > + * is the temperature in Celsius degree and T is the raw value 
> > + * from the sensor. 
> > + */ 
> > + .temp_offset = -1791, 
> > + .temp_scale = -121, 
> > + .gen2_ths = true, 
> >  }; 
> >  
> >  struct sun4i_gpadc_iio { 
> > @@ -103,6 +119,9 @@ struct sun4i_gpadc_iio { 
> >  atomic_t ignore_temp_data_irq; 
> >  const struct gpadc_data *data; 
> >  bool no_irq; 
> > + struct clk *ths_bus_clk; 
> > + struct clk *ths_clk; 
> > + struct reset_control *reset; 
> >  /* prevents concurrent reads of temperature and ADC */ 
> >  struct mutex mutex; 
> >  }; 
> > @@ -274,7 +293,11 @@ static int sun4i_gpadc_temp_read(struct iio_dev *indio_dev, int *val) 
> >  if (info->no_irq) { 
> >  pm_runtime_get_sync(indio_dev->dev.parent); 
> >  
> > - regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val); 
> > + if (info->data->gen2_ths) 
> > + regmap_read(info->regmap, SUN8I_H3_GPADC_TEMP_DATA, 
> > +     val); 
> > + else 
> > + regmap_read(info->regmap, SUN4I_GPADC_TEMP_DATA, val); 
> >  
>
> Instead of gen2_ths, use the TEMP_DATA register address. 
>
> >  pm_runtime_mark_last_busy(indio_dev->dev.parent); 
> >  pm_runtime_put_autosuspend(indio_dev->dev.parent); 
> > @@ -386,10 +409,15 @@ static int sun4i_gpadc_runtime_suspend(struct device *dev) 
> >  { 
> >  struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); 
> >  
> > - /* Disable the ADC on IP */ 
> > - regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0); 
> > - /* Disable temperature sensor on IP */ 
> > - regmap_write(info->regmap, SUN4I_GPADC_TPR, 0); 
> > + if (info->data->gen2_ths) { 
> > + /* Disable temperature sensor */ 
> > + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL2, 0); 
> > + } else { 
> > + /* Disable the ADC on IP */ 
> > + regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 0); 
> > + /* Disable temperature sensor on IP */ 
> > + regmap_write(info->regmap, SUN4I_GPADC_TPR, 0); 
> > + } 
> >  
>
> Either use another register address or add a suspend function to struct 
> gpadc_data which will be different for each version of the IP. 
>
> >  return 0; 
> >  } 
> > @@ -398,19 +426,36 @@ static int sun4i_gpadc_runtime_resume(struct device *dev) 
> >  { 
> >  struct sun4i_gpadc_iio *info = iio_priv(dev_get_drvdata(dev)); 
> >  
> > - /* clkin = 6MHz */ 
> > - regmap_write(info->regmap, SUN4I_GPADC_CTRL0, 
> > -      SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) | 
> > -      SUN4I_GPADC_CTRL0_FS_DIV(7) | 
> > -      SUN4I_GPADC_CTRL0_T_ACQ(63)); 
> > - regmap_write(info->regmap, SUN4I_GPADC_CTRL1, info->data->tp_mode_en); 
> > - regmap_write(info->regmap, SUN4I_GPADC_CTRL3, 
> > -      SUN4I_GPADC_CTRL3_FILTER_EN | 
> > -      SUN4I_GPADC_CTRL3_FILTER_TYPE(1)); 
> > - /* period = SUN4I_GPADC_TPR_TEMP_PERIOD * 256 * 16 / clkin; ~0.6s */ 
> > - regmap_write(info->regmap, SUN4I_GPADC_TPR, 
> > -      SUN4I_GPADC_TPR_TEMP_ENABLE | 
> > -      SUN4I_GPADC_TPR_TEMP_PERIOD(800)); 
> > + if (info->data->gen2_ths) { 
> > + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL2, 
> > +      SUN8I_H3_GPADC_CTRL2_TEMP_SENSE_EN | 
> > +      SUN8I_H3_GPADC_CTRL2_T_ACQ1(31)); 
> > + regmap_write(info->regmap, SUN4I_GPADC_CTRL0, 
> > +      SUN4I_GPADC_CTRL0_T_ACQ(31)); 
> > + regmap_write(info->regmap, SUN8I_H3_GPADC_CTRL3, 
> > +      SUN4I_GPADC_CTRL3_FILTER_EN | 
> > +      SUN4I_GPADC_CTRL3_FILTER_TYPE(1)); 
> > + regmap_write(info->regmap, SUN8I_H3_GPADC_INTC, 
> > +      SUN8I_H3_GPADC_INTC_TEMP_PERIOD(800)); 
> > + } else { 
> > + /* clkin = 6MHz */ 
> > + regmap_write(info->regmap, SUN4I_GPADC_CTRL0, 
> > +      SUN4I_GPADC_CTRL0_ADC_CLK_DIVIDER(2) | 
> > +      SUN4I_GPADC_CTRL0_FS_DIV(7) | 
> > +      SUN4I_GPADC_CTRL0_T_ACQ(63)); 
> > + regmap_write(info->regmap, SUN4I_GPADC_CTRL1, 
> > +      info->data->tp_mode_en); 
> > + regmap_write(info->regmap, SUN4I_GPADC_CTRL3, 
> > +      SUN4I_GPADC_CTRL3_FILTER_EN | 
> > +      SUN4I_GPADC_CTRL3_FILTER_TYPE(1)); 
> > + /* 
> > + * period = SUN4I_GPADC_TPR_TEMP_PERIOD * 256 * 16 / clkin; 
> > + * ~0.6s 
> > + */ 
> > + regmap_write(info->regmap, SUN4I_GPADC_TPR, 
> > +      SUN4I_GPADC_TPR_TEMP_ENABLE | 
> > +      SUN4I_GPADC_TPR_TEMP_PERIOD(800)); 
> > + } 
> >  
>
> Same here as suspend function? 
>
> >  return 0; 
> >  } 
> > @@ -494,6 +539,10 @@ static const struct of_device_id sun4i_gpadc_of_id[] = { 
> >  .compatible = "allwinner,sun8i-a33-ths", 
> >  .data = &sun8i_a33_gpadc_data, 
> >  }, 
> > + { 
> > + .compatible = "allwinner,sun8i-h3-ths", 
> > + .data = &sun8i_h3_gpadc_data, 
> > + }, 
> >  { /* sentinel */ } 
> >  }; 
> >  
> > @@ -529,6 +578,43 @@ static int sun4i_gpadc_probe_dt(struct platform_device *pdev, 
> >  return ret; 
> >  } 
> >  
> > + if (info->data->gen2_ths) { 
> > + info->reset = devm_reset_control_get(&pdev->dev, NULL); 
> > + if (IS_ERR(info->reset)) { 
> > + ret = PTR_ERR(info->reset); 
> > + return ret; 
> > + } 
> > + 
> > + ret = reset_control_deassert(info->reset); 
> > + if (ret) 
> > + return ret; 
> > + 
> > + info->ths_bus_clk = devm_clk_get(&pdev->dev, "bus"); 
> > + if (IS_ERR(info->ths_bus_clk)) { 
> > + ret = PTR_ERR(info->ths_bus_clk); 
> > + return ret; 
> > + } 
> > + 
> > + ret = clk_prepare_enable(info->ths_bus_clk); 
> > + if (ret) 
> > + return ret; 
> > + 
> > + info->ths_clk = devm_clk_get(&pdev->dev, "ths"); 
> > + if (IS_ERR(info->ths_clk)) { 
> > + ret = PTR_ERR(info->ths_clk); 
> > + return ret; 
> > + } 
> > + 
> > + /* Running at 6MHz */ 
> > + ret = clk_set_rate(info->ths_clk, 6000000); 
> > + if (ret) 
> > + return ret; 
> > + 
> > + ret = clk_prepare_enable(info->ths_clk); 
> > + if (ret) 
> > + return ret; 
> > + } 
> > + 
> >  if (!IS_ENABLED(CONFIG_THERMAL_OF)) 
> >  return 0; 
> >  
> > @@ -691,6 +777,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev) 
> >  if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF)) 
> >  iio_map_array_unregister(indio_dev); 
> >  
> > + if (info->data->gen2_ths) { 
> > + clk_disable_unprepare(info->ths_clk); 
> > + clk_disable_unprepare(info->ths_bus_clk); 
> > + reset_control_deassert(info->reset); 
> > + } 
> > + 
>
> I'm not really fond of using this boolean as I don't see it being 
> possibly reused for any other SoCs that has a GPADC or THS. 

Because you didn't care new SoCs :-)

All SoCs after H3 (A64, H5, R40) uses the same THS architecture with H3.

>
> Here, you could make use of a list/array of clk which then can be reused 
> for other SoCs just by changing the list. Add a default rate to the 
> gpadc_data structure and you're go to go. 
>
> >  return 0; 
> >  } 
> >  
> > diff --git a/include/linux/mfd/sun4i-gpadc.h b/include/linux/mfd/sun4i-gpadc.h 
> > index 139872c2e0fe..f794a2988a93 100644 
> > --- a/include/linux/mfd/sun4i-gpadc.h 
> > +++ b/include/linux/mfd/sun4i-gpadc.h 
> > @@ -38,9 +38,12 @@ 
> >  #define SUN6I_GPADC_CTRL1_ADC_CHAN_SELECT(x) (GENMASK(3, 0) & BIT(x)) 
> >  #define SUN6I_GPADC_CTRL1_ADC_CHAN_MASK GENMASK(3, 0) 
> >  
> > -/* TP_CTRL1 bits for sun8i SoCs */ 
> > -#define SUN8I_GPADC_CTRL1_CHOP_TEMP_EN BIT(8) 
> > -#define SUN8I_GPADC_CTRL1_GPADC_CALI_EN BIT(7) 
> > +/* TP_CTRL1 bits for sun8i A23/A33 SoCs */ 
> > +#define SUN8I_A23_GPADC_CTRL1_CHOP_TEMP_EN BIT(8) 
> > +#define SUN8I_A23_GPADC_CTRL1_GPADC_CALI_EN BIT(7) 
> > + 
>
> Different patch for these? 
>
> Thanks, 
> Quentin 
> -- 
> Quentin Schulz, Free Electrons 
> Embedded Linux and Kernel engineering 
> http://free-electrons.com 
>
> _______________________________________________ 
> linux-arm-kernel mailing list 
> linux-arm-kernel@lists.infradead.org 
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 

^ permalink raw reply

* Re: [PATCH v4 01/18] dt-bindings: power: battery: add constant-charge-current property
From: Quentin Schulz @ 2017-03-29  7:09 UTC (permalink / raw)
  To: Liam Breck
  Cc: Sebastian Reichel, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAKvHMgRtEnNbokYReu2vEs9UKH6qZnncdyQD=1JMDTZeN+aYCA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Liam,

On 29/03/2017 08:55, Liam Breck wrote:
> Hi Quentin,
> 
> On Thu, Mar 16, 2017 at 12:42 AM, Liam Breck <liam-RYWXG+zxWwBdeoIcmNTgJF6hYfS7NtTn@public.gmane.org> wrote:
>> On Thu, Mar 16, 2017 at 12:03 AM, Quentin Schulz
>> <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>> Hi Liam,
>>>
>>> On 16/03/2017 07:27, Liam Breck wrote:
>>>> Hi Rob,
>>>>
>>>> On Wed, Mar 15, 2017 at 6:10 AM, Quentin Schulz
>>>> <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>> Hi Liam,
>>>>>
>>>>> On 15/03/2017 13:08, Liam Breck wrote:
>>>>>> I dropped most of the CCs, pls re-add anyone essential. Use Cc lines
>>>>>> in patch description to direct a patch to interested parties and
>>>>>> relevant lists. I don't want to see all 19 patches in this series.
>>>>>>
>>>>>> On Wed, Mar 15, 2017 at 3:55 AM, Quentin Schulz
>>>>>> <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>>>> This adds the constant-charge-current property to the list of optional
>>>>>>> properties of the battery.
>>>>>>>
>>>>>>> The constant charge current is critical for batteries as they can't
>>>>>>> handle all charge currents.
>>>>>>>
>>>>>>> Signed-off-by: Quentin Schulz <quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>>>>>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>>>> ---
>>>>>>>
>>>>>>> v4:
>>>>>>>  - switch from constant-charge-current-microamp to constant-charge-microamp,
>>>>
>>>> Sebastian is on record supporting name alignment between DT:battery
>>>> properties and enum power_supply_property.
>>>>
>>>> Could you OK a change back to constant-charge-current-microamp in this patch?
>>>>
>>>> Note that the following could appear in DT:battery in future:
>>>>
>>>> constant-charge-current-microamp
>>>> constant-charge-current-max-microamp
>>>
>>> OK. I am actually setting max constant charge current and constant
>>> current charge with the same DT property so I might need to adapt my
>>> patches.
>>
>> I suspect you should set battery:*-current-max with DT, as -current is
>> "programmed by charger" so seems to be a stat, not a fixed
>> characteristic.
> 
> Did you resolve this issue?
> 

Sorry, completely missed your first mail.

Constant charge current is actually what I set in the PMIC. The maximum
constant charge current is completely PMIC-agnostic in my driver as it
is used to tell the user that (s)he shouldn't really have a constant
charge current over the maximum limit as the battery might not be able
to handle such high current.

What I suggest is to have both properties in DT. I'll set the maximum as
I do today and we can give the user the ability to set a "default"
constant charge current (below maximum constant charge current) in the
DT. Maybe we do not want to recharge your battery with the maximum value
by default?

> Rob & Sebastian want me to merge all items for
> bindings/power/supply/battery.txt. If I do that I'll also roll up
> their counterparts in power_supply_core.c and submit a patchset for
> those two files.
> 

I guess that adding the two properties make sense, don't you think?

Also, could you Cc me on your next version of your patches? So I know
when to rebase and slightly rework my patches once your patch series is
merged.

Thanks,
Quentin

>> You may program your charger's -current with the battery's
>> -current-max, of course.
>>
>> Note that your charger also has a -current-max characteristic, likely
>> different than your battery. You may not need that in DT tho.
>>
>>>> constant-charge-voltage-microvolt
>>>> constant-charge-voltage-max-microvolt
>>>>
>>>> Thanks!
>>>>
>>>>>> Must be constant-charge-current-microamp for the reasons discussed in
>>>>>> battery.txt - consistency with sysfs names.
>>>>>>
>>>>>
>>>>> Hum. Just nitpicking, but I disagree with the use of 'must'. IIRC there
>>>>> is nothing in the code that would require the property to be named after
>>>>> a property from the enum power_supply_property.
>>>>>
>>>>> I would say that you _want_ it to be named like that because it
>>>>> underlines the relation between the DT property and the actual impacted
>>>>> property in the power supply subsystem. I'm fine with this reason but in
>>>>> the end, the maintainer's opinion prevails (if (s)he does not want it,
>>>>> (s)he will not take it). So, basically, I actually don't mind either
>>>>> option and I see arguments on each side.
>>>>>
>>>>> So, just waiting for maintainer's opinion to make the final version of
>>>>> this patch.
>>>>>
>>>>> Thanks,
>>>>> Quentin
>>>>>
>>>>>> Also applies to power_supply_core.c patch.
>>>>>>
>>>>>> Rob: enum power_supply_property also lists constant-charge-voltage,
>>>>>> hence the -current
>>>>>>
>>>>>>
>>>>>>> added in v3
>>>>>>>
>>>>>>>  Documentation/devicetree/bindings/power/supply/battery.txt | 2 ++
>>>>>>>  1 file changed, 2 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>>>>>>> index 0278617..9594e1e 100644
>>>>>>> --- a/Documentation/devicetree/bindings/power/supply/battery.txt
>>>>>>> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>>>>>>> @@ -7,6 +7,7 @@ Optional Properties:
>>>>>>>   - voltage-min-design-microvolt: drained battery voltage
>>>>>>>   - energy-full-design-microwatt-hours: battery design energy
>>>>>>>   - charge-full-design-microamp-hours: battery design capacity
>>>>>>> + - constant-charge-microamp: battery constant charge current
>>>>>>>
>>>>>>>  Because drivers surface properties in sysfs using names derived
>>>>>>>  from enum power_supply_property, e.g.
>>>>>>> @@ -30,6 +31,7 @@ Example:
>>>>>>>                 voltage-min-design-microvolt = <3200000>;
>>>>>>>                 energy-full-design-microwatt-hours = <5290000>;
>>>>>>>                 charge-full-design-microamp-hours = <1430000>;
>>>>>>> +               constant-charge-microamp = <300000>;
>>>>>>>         };
>>>>>>>
>>>>>>>         charger: charger@11 {
>>>>>>> --
>>>>>>> 2.9.3
>>>>>>>
>>>>>
>>>>> --
>>>>> Quentin Schulz, Free Electrons
>>>>> Embedded Linux and Kernel engineering
>>>>> http://free-electrons.com
>>>
>>> --
>>> Quentin Schulz, Free Electrons
>>> Embedded Linux and Kernel engineering
>>> http://free-electrons.com

-- 
Quentin Schulz, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] drm/sun4i: Add display backend interrupt to device tree binding
From: Maxime Ripard @ 2017-03-29  7:09 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: David Airlie, Rob Herring, Mark Rutland,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20170327143847.16259-2-wens-jdAy2FN1RRM@public.gmane.org>

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

On Mon, Mar 27, 2017 at 10:38:46PM +0800, Chen-Yu Tsai wrote:
> The display backend has an interrupt line. Add it to the device tree
> binding.
> 
> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>

The patches prefix for the binding documentation is usually
dt-bindings:

I've changed that and applied that patch, thanks!
Maxime

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] ARM: dts: sun5i: Add interrupt for display backend
From: Maxime Ripard @ 2017-03-29  7:13 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Mark Rutland, devicetree, linux-kernel, dri-devel, linux-sunxi,
	Rob Herring, linux-arm-kernel
In-Reply-To: <20170327143847.16259-3-wens@csie.org>


[-- Attachment #1.1: Type: text/plain, Size: 574 bytes --]

On Mon, Mar 27, 2017 at 10:38:47PM +0800, Chen-Yu Tsai wrote:
> The display backend on sun5i shares the same interrupt line as the
> display frontend. Add it.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

And ideally, patches prefix shouldn't have dts here too. I've dropped
it and applied, thanks!

> This won't directly apply to old releases. We may want to backport
> them though?

Yep, probably. Do you want to send it to stable?

Thanks,
Maxime

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH] input: matrix_keypad: add option to drive inactive columns
From: Dmitry Torokhov @ 2017-03-29  7:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Rivshin, Mark Rutland, linux-input, devicetree,
	linux-kernel
In-Reply-To: <20170329020229.dm3y57dlzjwilifm@rob-hp-laptop>

On Tue, Mar 28, 2017 at 09:02:29PM -0500, Rob Herring wrote:
> On Wed, Mar 22, 2017 at 04:16:50PM -0400, David Rivshin wrote:
> > From: David Rivshin <DRivshin@allworx.com>
> > 
> > The gpio-matrix-keypad driver normally sets inactive columns as inputs
> > while scanning. This does not work for all hardware, which may require
> > the inactive columns to be actively driven in order to overcome any
> > pull-ups/downs on the columns.
> > 
> > Signed-off-by: David Rivshin <drivshin@allworx.com>
> > ---
> >  .../devicetree/bindings/input/gpio-matrix-keypad.txt        |  2 ++
> >  drivers/input/keyboard/matrix_keypad.c                      | 13 +++++++++----
> >  include/linux/input/matrix_keypad.h                         |  3 +++
> >  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> Acked-by: Rob Herring <robh@kernel.org>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/2] ARM: dts: sun5i: Add interrupt for display backend
From: Chen-Yu Tsai @ 2017-03-29  7:26 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, David Airlie, Rob Herring, Mark Rutland, dri-devel,
	devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <20170329071303.ft7oukmhczihy2xb@lukather>

On Wed, Mar 29, 2017 at 3:13 PM, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Mon, Mar 27, 2017 at 10:38:47PM +0800, Chen-Yu Tsai wrote:
>> The display backend on sun5i shares the same interrupt line as the
>> display frontend. Add it.
>>
>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>
> And ideally, patches prefix shouldn't have dts here too. I've dropped
> it and applied, thanks!

Seems like this was a recent policy change? Or almost everyone was getting
it wrong in the past.

>> This won't directly apply to old releases. We may want to backport
>> them though?
>
> Yep, probably. Do you want to send it to stable?

It won't apply, due to the recent sun5i merger. We could wait until
it lands in Linus' tree, then send a backport patch to stable, to
keep the noise to a minimum.

ChenYu

^ permalink raw reply


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