Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/3] binding: irqchip: mtk-cirq: Add binding document
From: Youlin Pei @ 2016-11-15  3:09 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marc Zyngier, Matthias Brugger, Thomas Gleixner, Jason Cooper,
	Mark Rutland, Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
	hongkun.cao-NuS5LvNUpcJWk0Htik3J/w,
	yong.wu-NuS5LvNUpcJWk0Htik3J/w, erin.lo-NuS5LvNUpcJWk0Htik3J/w,
	chieh-jay.liu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <20161109182613.7tvzewua6ewxdvjc@rob-hp-laptop>

On Wed, 2016-11-09 at 12:26 -0600, Rob Herring wrote:
> On Tue, Nov 01, 2016 at 07:52:00PM +0800, Youlin Pei wrote:
> > This commit adds the device tree binding document for
> > the mediatek cirq.
> > 
> > Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  .../interrupt-controller/mediatek,cirq.txt         |   30 ++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> > new file mode 100644
> > index 0000000..84e8123
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/mediatek,cirq.txt
> > @@ -0,0 +1,30 @@
> > +* Mediatek 27xx cirq
> > +
> > +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.
> 
> s/works/work/

will fix in next version.

> 
> > +The external interrupts (outside MCUSYS) will feed through CIRQ and connect
> > +to GIC in MCUSYS. When CIRQ is enabled, it will record the edge-sensitive
> > +interrupts and generated a pulse signal to parent interrupt controller when
> 
> s/generated/generate/

will fix in next version.

> 
> > +flush command is executed. With CIRQ, MCUSYS can be completely turned off
> > +to improve the system power consumption without losing interrupts.
> > +
> > +Required properties:
> > +- compatible: should be: "mediatek,mtk-cirq".
> 
> This should be SoC specific. This is fine as a fallback if the same 
> block is in many SoCs, but mediatek and mtk is a bit redundant.
> 

In next version, i will improve as following:
- compatible: Should be one of 
  - "mediatek,mt2701-cirq" for mt2701 CIRQ
  - "mediatek,mt8173-cirq" for mt8173 CIRQ
  - "mediatek,mt8135-cirq" for mt8135 CIRQ
  and "mediatek,cirq" as a fallback.

> > +- interrupt-controller : Identifies the node as an interrupt controller.
> > +- #interrupt-cells : Use the same format as specified by GIC in arm,gic.txt.
> > +- interrupt-parent: phandle of irq parent for cirq. The parent must
> > +  use the same interrupt-cells format as GIC.
> > +- reg: Physical base address of the cirq registers and length of memory
> > +  mapped region.
> > +- mediatek,ext-irq-start: Identifies external irq start number in different
> > +  SOCs.
> 
> Wouldn't this always be 32 if the GIC is the parent? If 32 is the common 
> case, then use the SoC compatible to determine this value.
> 

ext-irq-start is not always 32. In different Socs, this value is
different.
Thanks a lot!

> > +
> > +Example:
> > +	cirq: interrupt-controller@10204000 {
> > +		compatible = "mediatek,mtk-cirq";
> > +		interrupt-controller;
> > +		#interrupt-cells = <3>;
> > +		interrupt-parent = <&sysirq>;
> > +		reg = <0 0x10204000 0 0x4000>;
> > +		mediatek,ext-irq-start = <32>;
> > +	};
> > -- 
> > 1.7.9.5
> > 


--
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: Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply
From: Chen-Yu Tsai @ 2016-11-15  2:59 UTC (permalink / raw)
  To: David Airlie
  Cc: Chen-Yu Tsai, Rob Herring, Maxime Ripard, Mark Rutland, dri-devel,
	linux-arm-kernel, linux-kernel, devicetree, linux-sunxi
In-Reply-To: <CAGb2v64tTacSAG1vvO28h8MN9JC9w+OzSrP+j2eTzqAiQKgaSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi,

On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>> Some dumb VGA DACs are active components which require external power.
>>> Add support for specifying a regulator as its power supply.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>>> ---
>>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>>
>> For the binding,
>>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Any comments on this patch from the DRM people?

ChenYu

>>
>> One code comment below...
>>
>>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>>  2 files changed, 37 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> index 003bc246a270..164cbb15f04c 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>>  - Video port 0 for RGB input
>>>  - Video port 1 for VGA output
>>>
>>> +Optional properties:
>>> +- vdd-supply: Power supply for DAC
>>>
>>>  Example
>>>  -------
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> index afec232185a7..59781e031220 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -12,6 +12,7 @@
>>>
>>>  #include <linux/module.h>
>>>  #include <linux/of_graph.h>
>>> +#include <linux/regulator/consumer.h>
>>>
>>>  #include <drm/drmP.h>
>>>  #include <drm/drm_atomic_helper.h>
>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>       struct drm_connector    connector;
>>>
>>>       struct i2c_adapter      *ddc;
>>> +     struct regulator        *vdd;
>>>  };
>>>
>>>  static inline struct dumb_vga *
>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>>       return 0;
>>>  }
>>>
>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>> +{
>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +     int ret;
>>> +
>>> +     if (!IS_ERR(vga->vdd)) {
>>
>> if (IS_ERR())
>>         return;
>>
>> ...will save some intentation.
>
> I thought about that, though if someone were to add more stuff to
> this, such as an enable GPIO, they might have to rework it. A standalone
> block of code would be easier to work with. I'm OK either way though.
>
> ChenYu
>
>>
>>> +             ret = regulator_enable(vga->vdd);
>>> +
>>> +             if (ret) {
>>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>> +                     return;
>>> +             }
>>> +     }
>>> +}
>>> +
>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>> +{
>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +
>>> +     if (!IS_ERR(vga->vdd))
>>> +             regulator_disable(vga->vdd);
>>> +}
>>> +
>>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>       .attach         = dumb_vga_attach,
>>> +     .enable         = dumb_vga_enable,
>>> +     .disable        = dumb_vga_disable,
>>>  };
>>>
>>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>>               return -ENOMEM;
>>>       platform_set_drvdata(pdev, vga);
>>>
>>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>> +     if (IS_ERR(vga->vdd)) {
>>> +             ret = PTR_ERR(vga->vdd);
>>> +             if (ret == -EPROBE_DEFER)
>>> +                     return -EPROBE_DEFER;
>>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>>> +     }
>>> +
>>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>       if (IS_ERR(vga->ddc)) {
>>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>>> --
>>> 2.9.3
>>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
>> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH v2 2/3] irqchip: mtk-cirq: Add mediatek mtk-cirq implement
From: Youlin Pei @ 2016-11-15  2:49 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,
	chieh-jay.liu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <1f8f7b9f-7788-18c4-19c5-3c9a7623c188-5wv7dgnIgG8@public.gmane.org>

On Thu, 2016-11-10 at 18:24 +0000, Marc Zyngier wrote:
> On 08/11/16 02:57, Youlin Pei wrote:
> > On Fri, 2016-11-04 at 22:21 +0000, Marc Zyngier wrote:
> >> On Fri, Nov 04 2016 at 04:42:57 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >>> On Tue, 2016-11-01 at 20:49 +0000, Marc Zyngier wrote:
> >>>> On Tue, Nov 01 2016 at 11:52:01 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >>>>> In Mediatek SOCs, the CIRQ is a low power interrupt controller
> >>>>> designed to works outside MCUSYS which comprises with Cortex-Ax
> >>>>> cores,CCI and GIC.
> >>>>>
> >>>>> The CIRQ controller is integrated in between MCUSYS( include
> >>>>> Cortex-Ax, CCI and GIC ) and interrupt sources as the second
> >>>>> level interrupt controller. The external interrupts which outside
> >>>>> MCUSYS will feed through CIRQ then bypass to GIC. CIRQ can monitors
> >>>>> all edge trigger interupts. When an edge interrupt is triggered,
> >>>>> CIRQ can record the status and generate a pulse signal to GIC when
> >>>>> flush command executed.
> >>>>>
> >>>>> When system enters sleep mode, MCUSYS will be turned off to improve
> >>>>> power consumption, also GIC is power down. The edge trigger interrupts
> >>>>> will be lost in this scenario without CIRQ.
> >>>>>
> >>>>> This commit provides the CIRQ irqchip implement.
> >>>>>
> >>>>> Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>>>> ---
> >>>>>  drivers/irqchip/Makefile       |    2 +-
> >>>>>  drivers/irqchip/irq-mtk-cirq.c |  262 ++++++++++++++++++++++++++++++++++++++++
> >>>>>  2 files changed, 263 insertions(+), 1 deletion(-)
> >>>>>  create mode 100644 drivers/irqchip/irq-mtk-cirq.c
> >>>>>
> >>>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> >>>>> index e4dbfc8..8f33580 100644
> >>>>> --- a/drivers/irqchip/Makefile
> >>>>> +++ b/drivers/irqchip/Makefile
> >>>>> @@ -60,7 +60,7 @@ obj-$(CONFIG_BCM7120_L2_IRQ)		+= irq-bcm7120-l2.o
> >>>>>  obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
> >>>>>  obj-$(CONFIG_KEYSTONE_IRQ)		+= irq-keystone.o
> >>>>>  obj-$(CONFIG_MIPS_GIC)			+= irq-mips-gic.o
> >>>>> -obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o
> >>>>> +obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mtk-sysirq.o irq-mtk-cirq.o
> >>>>>  obj-$(CONFIG_ARCH_DIGICOLOR)		+= irq-digicolor.o
> >>>>>  obj-$(CONFIG_RENESAS_H8300H_INTC)	+= irq-renesas-h8300h.o
> >>>>>  obj-$(CONFIG_RENESAS_H8S_INTC)		+= irq-renesas-h8s.o
> >>>>> diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
> >>>>> new file mode 100644
> >>>>> index 0000000..fc43ef3
> >>>>> --- /dev/null
> >>>>> +++ b/drivers/irqchip/irq-mtk-cirq.c
> >>>>> @@ -0,0 +1,262 @@
> >>>>> +/*
> >>>>> + * Copyright (c) 2016 MediaTek Inc.
> >>>>> + * Author: Youlin.Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> >>>>> + *
> >>>>> + * This program is free software; you can redistribute it and/or modify
> >>>>> + * it under the terms of the GNU General Public License version 2 as
> >>>>> + * published by the Free Software Foundation.
> >>>>> + *
> >>>>> + * This program is distributed in the hope that it will be useful,
> >>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >>>>> + * GNU General Public License for more details.
> >>>>> + */
> >>>>> +
> >>>>> +#include <linux/irq.h>
> >>>>> +#include <linux/irqchip.h>
> >>>>> +#include <linux/irqdomain.h>
> >>>>> +#include <linux/of.h>
> >>>>> +#include <linux/of_irq.h>
> >>>>> +#include <linux/of_address.h>
> >>>>> +#include <linux/io.h>
> >>>>> +#include <linux/slab.h>
> >>>>> +#include <linux/syscore_ops.h>
> >>>>> +
> >>>>> +#define CIRQ_ACK	0x40
> >>>>> +#define CIRQ_MASK_SET	0xc0
> >>>>> +#define CIRQ_MASK_CLR	0x100
> >>>>> +#define CIRQ_SENS_SET	0x180
> >>>>> +#define CIRQ_SENS_CLR	0x1c0
> >>>>> +#define CIRQ_POL_SET	0x240
> >>>>> +#define CIRQ_POL_CLR	0x280
> >>>>> +#define CIRQ_CONTROL	0x300
> >>>>> +
> >>>>> +#define CIRQ_EN	0x1
> >>>>> +#define CIRQ_EDGE	0x2
> >>>>> +#define CIRQ_FLUSH	0x4
> >>>>> +
> >>>>> +#define CIRQ_IRQ_NUM    0x200
> >>>>> +
> >>>>> +struct mtk_cirq_chip_data {
> >>>>> +	void __iomem *base;
> >>>>> +	unsigned int ext_irq_start;
> >>>>> +};
> >>>>> +
> >>>>> +static struct mtk_cirq_chip_data *cirq_data;
> >>>>
> >>>> Are you guaranteed that you'll only ever have a single CIRQ in any
> >>>> system?
> >>>
> >>> In Mediatek's SOC, only hace a single CIRQ.
> >>>
> >>>>
> >>>>> +
> >>>>> +static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
> >>>>> +{
> >>>>> +	struct mtk_cirq_chip_data *chip_data = data->chip_data;
> >>>>> +	unsigned int cirq_num = data->hwirq;
> >>>>> +	u32 mask = 1 << (cirq_num % 32);
> >>>>> +
> >>>>> +	writel(mask, chip_data->base + offset + (cirq_num / 32) * 4);
> >>>>
> >>>> Why can't you use the relaxed accessors?
> >>>
> >>> It seems that i use wrong function, i will change the writel to
> >>> writel_relaxed in next ve
> >>>
> >>>>
> >>>>> +}
> >>>>> +
> >>>>> +static void mtk_cirq_mask(struct irq_data *data)
> >>>>> +{
> >>>>> +	mtk_cirq_write_mask(data, CIRQ_MASK_SET);
> >>>>> +	irq_chip_mask_parent(data);
> >>>>> +}
> >>>>> +
> >>>>> +static void mtk_cirq_unmask(struct irq_data *data)
> >>>>> +{
> >>>>> +	mtk_cirq_write_mask(data, CIRQ_MASK_CLR);
> >>>>> +	irq_chip_unmask_parent(data);
> >>>>> +}
> >>>>> +
> >>>>> +static void mtk_cirq_eoi(struct irq_data *data)
> >>>>> +{
> >>>>> +	mtk_cirq_write_mask(data, CIRQ_ACK);
> >>>>
> >>>> EOI and ACK have very different semantics. What is this write actually
> >>>> doing? Also, you're now doing an additional MMIO write on each interrupt
> >>>> EOI, doubling its cost. Do you really need to do actually signal the HW
> >>>> that we've EOIed an interrupt? I would have hoped that you'd be able to
> >>>> put it in "bypass" mode as long as you're not suspending...
> >>>>
> >>>
> >>> When external interrupt happened, CIRQ status register record the status
> >>> even CIRQ is not enabled. when execute the flush command, CIRQ will
> >>> resend the signals according to the status. So if don't clear the
> >>> status, CIRQ will resend the wrong signals. the ACK write operation will
> >>> clear the status.
> >>
> >> But at this time, we haven't suspended yet, and there is nothing to
> >> replay. Also, you only enable the edge capture when suspending. So what
> >> are you ACKing here? Can't you simply clear everything right when
> >> suspending and not do it at all on the fast path?
> > 
> > I had planned to ACK the status in cirq suspend callback, but
> > encountered a problem.
> > following is a piece of code from
> > http://lxr.free-electrons.com/source/kernel/power/suspend.c#L361
> > 
> > arch_suspend_disable_irqs(); ---step1
> > BUG_ON(!irqs_disabled());
> > 
> > error = syscore_suspend();
> >            |
> >            ---cirq suspend(); ---step2
> > 
> > if ack the status in cirq suspend, the interrupts will be lost which
> > happened during step1 to step2.
> > 
> > So would you mind give me some suggestions about this?
> > Thanks a lot!
> 
> Right. So maybe there is a way:
> - On suspend you can iterate over all the cirq interrupts that have been
> recorded
> - For each of those, you inspect if it is pending at the GIC level
> (using the irq_get_irqchip_state helper)
> - if not pending, you Ack it, because it cannot be delivered anymore
> - If it is pending, then you know it happened between step 1 and step 2.
> - You then have the choice of either going into suspend and waking up
> immediately, or failing the suspend.
> 
> Thoughts?

Will use this solution in next version.
Thanks a lot!

> 
> 	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 2/3] Revert "Documentation: devicetree: dwc2: Add host DMA binding"
From: John Youn @ 2016-11-15  2:29 UTC (permalink / raw)
  To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <cover.1479176826.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

This reverts commit 751089ecaab0 ("Documentation: devicetree: dwc2: Add
host DMA binding").

Remove this binding as it is not needed by any hardware.

Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 389bb13..ad8f7ff 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -25,7 +25,6 @@ Optional properties:
 Refer to phy/phy-bindings.txt for generic phy consumer properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
-- snps,host-dma-disable: disable host DMA mode.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
-- 
2.10.0

--
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 1/3] Revert "usb: dwc2: Add bindings to disable gadget DMA modes"
From: John Youn @ 2016-11-15  2:29 UTC (permalink / raw)
  To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <cover.1479176826.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

This reverts commit 9acc1ee2b723 ("usb: dwc2: Add bindings to disable
gadget DMA modes").

Don't add bindings and don't read them in. These are not yet needed by
any hardware.

Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 2 --
 drivers/usb/dwc2/params.c                      | 9 ++-------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..389bb13 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -26,8 +26,6 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
 - snps,host-dma-disable: disable host DMA mode.
-- snps,gadget-dma-disable: disable gadget DMA mode.
-- snps,gadget-dma-desc-disable: disable gadget DMA descriptor mode.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..2f18a7b 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1091,22 +1091,17 @@ static void dwc2_set_gadget_dma(struct dwc2_hsotg *hsotg)
 	struct dwc2_hw_params *hw = &hsotg->hw_params;
 	struct dwc2_core_params *p = &hsotg->params;
 	bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
-	bool disable;
 
 	/* Buffer DMA */
-	disable = device_property_read_bool(hsotg->dev,
-					    "snps,gadget-dma-disable");
 	dwc2_set_param_bool(hsotg, &p->g_dma,
 			    false, "gadget-dma",
-			    !disable, false,
+			    true, false,
 			    dma_capable);
 
 	/* DMA Descriptor */
-	disable = device_property_read_bool(hsotg->dev,
-					    "snps,gadget-dma-desc-disable");
 	dwc2_set_param_bool(hsotg, &p->g_dma_desc, false,
 			    "gadget-dma-desc",
-			    p->g_dma && !disable, false,
+			    p->g_dma, false,
 			    !!hw->dma_desc_enable);
 }
 
-- 
2.10.0

--
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 1/3] Revert "usb: dwc2: Add bindings to disable gadget DMA modes"
From: John Youn @ 2016-11-15  2:29 UTC (permalink / raw)
  To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland
In-Reply-To: <cover.1479176826.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

This reverts commit 9acc1ee2b723 ("usb: dwc2: Add bindings to disable
gadget DMA modes").

Don't add bindings and don't read them in. These are not yet needed by
any hardware.

Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 2 --
 drivers/usb/dwc2/params.c                      | 9 ++-------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..389bb13 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -26,8 +26,6 @@ Refer to phy/phy-bindings.txt for generic phy consumer properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
 - snps,host-dma-disable: disable host DMA mode.
-- snps,gadget-dma-disable: disable gadget DMA mode.
-- snps,gadget-dma-desc-disable: disable gadget DMA descriptor mode.
 - g-rx-fifo-size: size of rx fifo size in gadget mode.
 - g-np-tx-fifo-size: size of non-periodic tx fifo size in gadget mode.
 - g-tx-fifo-size: size of periodic tx fifo per endpoint (except ep0) in gadget mode.
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..2f18a7b 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1091,22 +1091,17 @@ static void dwc2_set_gadget_dma(struct dwc2_hsotg *hsotg)
 	struct dwc2_hw_params *hw = &hsotg->hw_params;
 	struct dwc2_core_params *p = &hsotg->params;
 	bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
-	bool disable;
 
 	/* Buffer DMA */
-	disable = device_property_read_bool(hsotg->dev,
-					    "snps,gadget-dma-disable");
 	dwc2_set_param_bool(hsotg, &p->g_dma,
 			    false, "gadget-dma",
-			    !disable, false,
+			    true, false,
 			    dma_capable);
 
 	/* DMA Descriptor */
-	disable = device_property_read_bool(hsotg->dev,
-					    "snps,gadget-dma-desc-disable");
 	dwc2_set_param_bool(hsotg, &p->g_dma_desc, false,
 			    "gadget-dma-desc",
-			    p->g_dma && !disable, false,
+			    p->g_dma, false,
 			    !!hw->dma_desc_enable);
 }
 
-- 
2.10.0

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

^ permalink raw reply related

* [PATCH 0/3] usb: dwc2: params revert and rework
From: John Youn @ 2016-11-15  2:29 UTC (permalink / raw)
  To: John Youn, Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland

Hi Felipe,

This reverts and fixes a few commits that are queued on your
testing/next, removing the previously added DT bindings, and the code
that reads them in.

The feedback was that IP validation is not reason enough to add these.
So we'll leave them out for now.

Regards,
John


John Youn (3):
  Revert "usb: dwc2: Add bindings to disable gadget DMA modes"
  Revert "Documentation: devicetree: dwc2: Add host DMA binding"
  usb: dwc2: Remove reading in of invalid property

 Documentation/devicetree/bindings/usb/dwc2.txt |  3 ---
 drivers/usb/dwc2/params.c                      | 16 +++-------------
 2 files changed, 3 insertions(+), 16 deletions(-)

-- 
2.10.0

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

^ permalink raw reply

* Re: [PATCH V3 1/9] PM / OPP: Reword binding supporting multiple regulators per device
From: Stephen Boyd @ 2016-11-15  2:13 UTC (permalink / raw)
  To: Rob Herring
  Cc: Viresh Kumar, Mark Brown, Rafael Wysocki, nm, Viresh Kumar,
	linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, d-gerlach,
	devicetree
In-Reply-To: <20161115015952.steomn4azdl5iv6z@rob-hp-laptop>

On 11/14, Rob Herring wrote:
> On Fri, Nov 11, 2016 at 08:41:20AM +0530, Viresh Kumar wrote:
> > On 10-11-16, 14:51, Stephen Boyd wrote:
> > > 
> > > No. The supply names (and also clock names/index) should be left
> > > up to the consumer of the OPP table. We don't want to encode any
> > > sort of details like this between the OPP table and the consumer
> > > of it in DT because then it seriously couples the OPP table to
> > > the consumer device. "The binding" in this case that needs to be
> > > updated is the consumer binding, to indicate that it correlated
> > > foo-supply and bar-supply to index 0 and 1 of the OPP table
> > > voltages.
> > 
> > Are you saying that we shall have a property like this then?
> > 
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index ee91cbdd95ee..733946df2fb8 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -389,7 +389,10 @@ Example 4: Handling multiple regulators
> >                         compatible = "arm,cortex-a7";
> >                         ...
> >  
> > -                       cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> > +                       vcc0-supply = <&cpu_supply0>;
> > +                       vcc1-supply = <&cpu_supply1>;
> > +                       vcc2-supply = <&cpu_supply2>;
> > +                       opp-supply-names = "vcc0", "vcc1", "vcc2";
> 
> Uh, no. You already have the names in the *-supply properties. Yes, they 
> are a PIA to retrieve compared to a *-names property, but that is the 
> nature of this style of binding.
> 

I think the problem is that Viresh wants the binding to be "self
describing" so that the OPP can be used without a driver knowing
that a supply corresponds to a particular column in the voltage
table. I don't understand that though. Can't we set the supply
names from C code somewhere based on the consumer of the OPPs?
Similar to how we pick the different tables based on fuses?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
From: Laurent Pinchart @ 2016-11-15  2:11 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree@vger.kernel.org, Laurent Pinchart, dri-devel,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Tomi Valkeinen,
	linux-media@vger.kernel.org
In-Reply-To: <CAL_JsqL_41VGf8ZFEzQyqPU_EeK=Ms+jdt_N9cvRyLFkXF2MTg@mail.gmail.com>

Hi Rob,

On Monday 14 Nov 2016 19:40:26 Rob Herring wrote:
> On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart wrote:
> > On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> >> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >>> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>>> Multiple incompatible data link layers have been used over time to
> >>>>> transmit image data to LVDS panels. This binding supports display
> >>>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>>> specifications.
> >>>>> 
> >>>>> Signed-off-by: Laurent Pinchart
> >>>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>>> ---
> >>>>> 
> >>>>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++
> >>>>>  1 file changed, 119 insertions(+)
> >>>>>  create mode 100644
> >>>>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>>> 
> >>>>> diff --git
> >>>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> new file mode 100644
> >>>>> index 000000000000..250861f2673e
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>>> @@ -0,0 +1,119 @@
> >>>>> +Generic LVDS Panel
> >>>>> +==================
> >>>>> +
> >>>>> +LVDS is a physical layer specification defined in
> >>>>> ANSI/TIA/EIA-644-A. Multiple
> >>>>> +incompatible data link layers have been used over time to transmit
> >>>>> image data
> >>>>> +to LVDS panels. This bindings supports display panels compatible
> >>>>> with the
> >>>>> +following specifications.
> >>>>> +
> >>>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>>> February
> >>>>> +1999 (Version 1.0), Japan Electronic Industry Development
> >>>>> Association (JEIDA)
> >>>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>>> National
> >>>>> +Semiconductor
> >>>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>>> Video
> >>>>> +Electronics Standards Association (VESA)
> >>>>> +
> >>>>> +Device compatible with those specifications have been marketed under
> >>>>> the
> >>>>> +FPD-Link and FlatLink brands.
> >>>>> +
> >>>>> +
> >>>>> +Required properties:
> >>>>> +- compatible: shall contain "panel-lvds"
> >>>> 
> >>>> Maybe as a fallback, but on its own, no way.
> >>> 
> >>> Which brings an interesting question: when designing generic DT
> >>> bindings, what's the rule regarding
> > 
> > Looks like I forgot part of the question. I meant to ask what is the rule
> > regarding usage of more precise compatible strings ?
> 
> When in doubt, always have one. If there's any chance at all that s/w
> will need to know or care, then we should have one.
> 
> > For instance (but perhaps not the best example), the
> > input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> > string, with no other bindings defining more precise compatible strings
> > for the exact rotary encoder model. When it comes to panels I believe it
> > makes sense to define model-specific compatible strings even if they're
> > unused by drivers. I'm however wondering what the rule is there, and
> > where those device-specific compatible strings should be defined. I'd
> > like to avoid using one file per panel model as done today for the
> > simple-panel bindings.
> 
> There's a few exceptions like this where there is not any sort of
> model to base a compatible on. For example, a GPIO connected LED is
> truly generic. The only way to have a more specific compatible would
> be something with the board name in it.
> 
> Your case here is in the middle. It seems like it's generic and
> passive, but perhaps power control is not. Rather than trying to
> decide, we can just cover our ass and put both a generic and specific
> compatible in.

That sounds good to me. I'll mention in the document that a more precise 
compatible is required.

> >> Call it "simple" so I can easily NAK it. :)
> >> 
> >> Define a generic structure, not a single binding trying to serve all.
> >> 
> >>>>> +- width-mm: panel display width in millimeters
> >>>>> +- height-mm: panel display height in millimeters
> >>>> 
> >>>> This is already documented for all panels IIRC.
> >>> 
> >>> Note that this DT binding has nothing to do with the simple-panel
> >>> binding. It is instead similar to the panel-dpi and panel-dsi-cm
> >>> bindings (which currently don't but should specify the panel size in
> >>> DT). The LVDS panel driver will *not* include any panel-specific
> >>> information such as size or timings, these are specified in DT.
> >> 
> >> The panel bindings aren't really different. The biggest difference was
> >> location in the tree, but we now generally allow panels to be either a
> >> child of the LCD controller or connected with OF graph. We probably
> >> need to work on restructuring the panel bindings a bit. We should have
> >> an inheritance with a base panel binding of things like size, label,
> >> graph, backlight, etc, then perhaps an interface specific bindings for
> >> LVDS, DSI, and parallel, then a panel specific binding. With this the
> >> panel specific binding is typically just a compatible string and which
> >> inherited properties apply to it.
> > 
> > That sounds good to me, but we have multiple models for panel bindings.
> > 
> > As you mentioned panels can be referenced through an LCD controller node
> > property containing a phandle to the panel node, or through OF graph.
> > That's a situation we have today, and we need to keep supporting both (at
> > least for existing panels, perhaps not for the new ones).
> > 
> > Another difference is how to express panel data such as size and timings.
> > The simple-panel DT bindings don't contain such data and expects the
> > drivers to contain a table of panel data for all models supported, while
> > the DPI, DSI and now the proposed LVDS panel bindings contain properties
> > for panel data.
> > 
> > How would you like to reconcile all that ?
> 
> Thierry has outlined the position[1] that simple-panel follows many
> times and I generally agree. I could be convinced that perhaps panel
> timings could go into DT. However, you can't really describe
> *everything*, so we're not going to get away from panel specific
> compatible strings and panel specifics in the kernel.

I'm fine with that middle ground, and will include panel timings in DT but not 
an information related to power sequencing or other panel-specific properties 
(beside the standardized properties defined in this binding of course).

> >>>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>>> "jeida-24"
> >>>>> +  or "vesa-24"
> >>>> 
> >>>> Maybe this should be part of the compatible.
> >>> 
> >>> I've thought about it, but given that some panels support selecting
> >>> between multiple modes (through a mode pin that is usually hardwired),
> >>> I believe a separate DT property makes sense.
> >> 
> >> Okay.
> >> 
> >>> Furthermore, LVDS data organization is controlled by the combination of
> >>> both data-mapping and data-mirror. It makes little sense from my point
> >>> of view to handle one as part of the compatible string and the other one
> >>> as a separate property.
> >>> 
> >>>> > +Optional properties:
> >>>> > +- label: a symbolic name for the panel
> >>>> 
> >>>> Could be for any panel or display connector.
> >>> 
> >>> Yes, but I'm not sure to understand how that's relevant :-)
> >> 
> >> Meaning it should be a common property.
> > 
> > Sure. So you expect me to reorganize all the panels and connectors DT
> > bindings in order to get this one merged ? :-)
> 
> No, because I don't think label is widely defined. Just put it in a
> common place and reference it. Any other panels can be fixed later.
> Really, the "simple panel" binding should probably morph into the
> common binding.

The "label" property is actually defined in the "Devicetree Specification, 
Release 0.1" as recently published on devicetree.org. Where would you like me 
to define it in the bindings ?

> >>>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>>> analog supply
> >>>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>>> digital supply
> >>>> 
> >>>> Which one has to be powered on first, what voltage, and with what time
> >>>> in between? This is why "generic" or "simple" bindings don't work.
> >>> 
> >>> The above-mentioned specifications also define connectors, pinouts and
> >>> power supplies, but many LVDS panels compatible with the LVDS physical
> >>> and data layers use a different connector with small differences in
> >>> power supplies.
> >>> 
> >>> I believe the voltage is irrelevant here, it doesn't need to be
> >>> controlled by the operating system. Power supplies order and timing is
> >>> relevant, I'll investigate the level of differences between panels. I'm
> >>> also fine with dropping those properties for now.
> >> 
> >> Whether you have control of the supplies is dependent on the board.
> >> Dropping them is just puts us in the simple binding trap. The simple
> >> bindings start out that way and then people keep adding to them.
> > 
> > Damn, you can't be fooled easily ;-)
> 
> I guess you can count all the simple bindings to see how many times I
> can be fooled. :)
> 
> > On a more serious note, I'd like to design the bindings in a way that
> > wouldn't require adding device-specific code in the driver for each panel
> > model, given that in most cases power supply handling will be generic.
> > What's your opinion about a generic power supply model that would be used
> > in the default case, with the option to override it with device-specific
> > code when needed ?
>
> I don't agree. Read Thierry's post on the subject[1].

I'm not sure you understood me correctly (and writing a clarification at 
4:00am might not help :-)). My point here is that a fair number of panels 
don't care about power sequencing and have no control GPIOs (that's certainly 
the case of the two Mitsubishi panels I use here that have a single power 
supply - good luck trying to convince me that this needs to be sequenced :-) - 
and no enable or reset control pin). This kind of panel should obviously be 
modelled in DT with both a specific and a generic ("panel-lvds") compatible 
string, to ensure that we'll have enough information available on the 
operating system side to control the panel properly. The power supply(ies) 
should be documented in relation to the specific compatible string and not 
mentioned by the generic bindings (whether that should go in one or multiple 
text files is bikeshedding).

This being said, on the driver side, I don't see a reason to list the specific 
compatible strings explicitly for those panels when a generic implementation 
matching the generic compatible string can handle them fine. This has no 
impact on the bindings and is an Linux implementation decision.

> [1]
> http://sietch-tagr.blogspot.com/2016/04/display-panels-are-not-special.html

-- 
Regards,

Laurent Pinchart

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

^ permalink raw reply

* Re: [PATCH V3 1/9] PM / OPP: Reword binding supporting multiple regulators per device
From: Rob Herring @ 2016-11-15  1:59 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Stephen Boyd, Mark Brown, Rafael Wysocki, nm-l0cyMroinI0,
	Viresh Kumar, linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	d-gerlach-l0cyMroinI0, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161111031120.GE11670@vireshk-i7>

On Fri, Nov 11, 2016 at 08:41:20AM +0530, Viresh Kumar wrote:
> On 10-11-16, 14:51, Stephen Boyd wrote:
> > On 11/10, Viresh Kumar wrote:
> > > On 10-11-16, 16:36, Mark Brown wrote:
> > > > On Thu, Nov 10, 2016 at 09:34:40AM +0530, Viresh Kumar wrote:
> > > > > On 09-11-16, 14:58, Mark Brown wrote:
> > > > > > On Wed, Oct 26, 2016 at 12:02:56PM +0530, Viresh Kumar wrote:
> > > > 
> > > > > > > +  Entries for multiple regulators shall be provided in the same field separated
> > > > > > > +  by angular brackets <>. The OPP binding doesn't provide any provisions to
> > > > > > > +  relate the values to their power supplies or the order in which the supplies
> > > > > > > +  need to be configured.
> > > > 
> > > > > > I don't understand how this works.  If we have an unordered list of
> > > > > > values to set for regulators how will we make sense of them?
> > > > 
> > > > > The platform driver is responsible to identify the order and pass it on to the
> > > > > OPP core. And the platform driver needs to have that hard coded.
> > > > 
> > > > That *really* should be in the binding.
> > > 
> > > Okay, how do you suggest doing that? Will a property like supply-names
> > > in the OPP table be fine? Like this:
> > > 
> > > @@ -369,13 +378,16 @@ Example 4: Handling multiple regulators
> > >                         compatible = "arm,cortex-a7";
> > >                         ...
> > >  
> > > -                       cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> > > +                       vcc0-supply = <&cpu_supply0>;
> > > +                       vcc1-supply = <&cpu_supply1>;
> > > +                       vcc2-supply = <&cpu_supply2>;
> > >                         operating-points-v2 = <&cpu0_opp_table>;
> > >                 };
> > >         };
> > >  
> > >         cpu0_opp_table: opp_table0 {
> > >                 compatible = "operating-points-v2";
> > > +               supply-names = "vcc0", "vcc1", "vcc2";
> > >                 opp-shared;
> > > 
> > 
> > No. The supply names (and also clock names/index) should be left
> > up to the consumer of the OPP table. We don't want to encode any
> > sort of details like this between the OPP table and the consumer
> > of it in DT because then it seriously couples the OPP table to
> > the consumer device. "The binding" in this case that needs to be
> > updated is the consumer binding, to indicate that it correlated
> > foo-supply and bar-supply to index 0 and 1 of the OPP table
> > voltages.
> 
> Are you saying that we shall have a property like this then?
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index ee91cbdd95ee..733946df2fb8 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -389,7 +389,10 @@ Example 4: Handling multiple regulators
>                         compatible = "arm,cortex-a7";
>                         ...
>  
> -                       cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> +                       vcc0-supply = <&cpu_supply0>;
> +                       vcc1-supply = <&cpu_supply1>;
> +                       vcc2-supply = <&cpu_supply2>;
> +                       opp-supply-names = "vcc0", "vcc1", "vcc2";

Uh, no. You already have the names in the *-supply properties. Yes, they 
are a PIA to retrieve compared to a *-names property, but that is the 
nature of this style of binding.

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

^ permalink raw reply

* Re: [PATCH 2/2] clk: hisilicon: add CRG driver for Hi3516CV300 SoC
From: Jiancheng Xue @ 2016-11-15  1:58 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: mturquette-rdvid1DuHRBWk0Htik3J/w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	bin.chen-QSEj5FYQhm4dnm+yROfE0A, elder-QSEj5FYQhm4dnm+yROfE0A,
	hermit.wangheming-C8/M+/jPZTeaMJb+Lgu22Q,
	yanhaifeng-C8/M+/jPZTeaMJb+Lgu22Q, wenpan-C8/M+/jPZTeaMJb+Lgu22Q,
	howell.yang-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <20161114200359.GM5177-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>



On 2016/11/15 4:03, Stephen Boyd wrote:
> On 11/14, Jiancheng Xue wrote:
>> Hi Stephen,
>>
>> On 2016/11/12 8:04, Stephen Boyd wrote:
>>> On 10/29, Jiancheng Xue wrote:
>>>
>>> Should be a From: Pan Wen here?
>>>
>>>> Add CRG driver for Hi3516CV300 SoC. CRG(Clock and Reset
>>>> Generator) module generates clock and reset signals used
>>>> by other module blocks on SoC.
>>>>
>>>> Signed-off-by: Pan Wen <wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>>>
>>> And you should have signed it off? Care to resend or state that
>>> this is incorrectly attributed to you instead of Pan Wen?
>>>
>>
>> Pan Wen is the main author of this patch. I just made some small modification
>> with agreement from him. Do I need to resend this patch if it's better to add my
>> signed-off?
>>
> 
> Sure I'll apply the resent one. But you should also make sure to
> retain Pan Wen's authorship. I'll go do that when applying it.
> 

OK. I have resent this patch. Please check it. Thank you, Stephen.

Regards,
Jiancheng



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

^ permalink raw reply

* Re: [PATCH v2 2/2] of: changesets: Introduce changeset helper methods
From: Frank Rowand @ 2016-11-15  1:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: Hans de Goede, Pantelis Antoniou,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+zWiXOtb4hWrpB87z8T4WLfCbLeGNgST4tmAz61dgFHA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 11/14/16 14:16, Rob Herring wrote:
> On Mon, Nov 14, 2016 at 12:44 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 11/14/16 03:04, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 14-11-16 08:34, Frank Rowand wrote:
>>>> Hi Hans, Pantelis,
>>>>
>>>> On 11/12/16 18:15, Frank Rowand wrote:
>>>>> On 11/04/16 07:42, Hans de Goede wrote:
>>>>>> From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>>>
>>>>>> Changesets are very powerful, but the lack of a helper API
>>>>>> makes using them cumbersome. Introduce a simple copy based
>>>>>> API that makes things considerably easier.
>>>>>>
>>>>>> To wit, adding a property using the raw API.
>>>>>>
>>>>>>     struct property *prop;
>>>>>>     prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
>>>>>>     prop->name = kstrdup("compatible");
>>>>>>     prop->value = kstrdup("foo,bar");
>>>>>>     prop->length = strlen(prop->value) + 1;
>>>>>>     of_changeset_add_property(ocs, np, prop);
>>>>>>
>>>>>> while using the helper API
>>>>>>
>>>>>>     of_changeset_add_property_string(ocs, np, "compatible",
>>>>>>             "foo,bar");
>>>>>>
>>>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>>> ---
>>>>>> Changes in v2 (hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
>>>>>> -Address review comments from:
>>>>>>  https://www.spinics.net/lists/kernel/msg2252845.html
>>>>>
>>>>> That points to the May 9 version 1 patches from Pantelis (as expected),
>>>>> but containing 4, not 2, patches.  Patch 1/4 was applied.  Patch 4/4
>>>>> seems to have disappeared?
>>>>>
>>>>> Pantelis then sent a version 2 set of the patches on May 16.
>>>>>
>>>>> Your version is a modification of the May 9 patches (as would be expected
>>>>> of a version 2).  It is confusing to have two different version 2 patch
>>>>> sets.  I don't have any brilliant ideas on how this patch set could have
>>>>> been named differently to avoid that confusion.
>>>>>
>>>>> The point of this little side-track is simply to note the existence of two
>>>>> different version 2 series so I won't be confused when I revisit this
>>>>> thread in the future.
>>>>>
>>>>>>  -Simplify (and fix) __of_changeset_add_update_property_copy OOM handling
>>>>>>  -Remove (by manual inlining) these 2 static helpers:
>>>>>>   __of_changeset_add_update_property_u32
>>>>>>   __of_changeset_add_update_property_bool
>>>>>>  -Remove the following exported helper method:
>>>>>>   of_changeset_node_move_to
>>>>>
>>>>> Not all comments were addressed.
>>>>>
>>>>> There are some other changes made that are not noted in the changelog.
>>>>>
>>>>> I am still reading through the patches. I will reply again either with
>>>>> a reviewed-by or specific comments when I finish.
>>>>
>>>> Replying here for the entire patchset (there was no patch 0 to reply to).
>>>>
>>>> After reading through the patches, my reply is meta instead of specific
>>>> comments about the code.
>>>>
>>>> There are very few users of change sets in tree.  I do not see the need to
>>>> add these helpers until such users are likely to appear.
>>>>
>>>> I would expect change sets to be _mostly_ used internally by the device tree
>>>> overlay framework, not directly by drivers.  If change sets are an attractive
>>>> technology for drivers, I want to approach that usage very carefully to avoid
>>>> inappropriate use, which could be very difficult to reign in after the fact.
>>>>
>>>> Even if helpers should be added, this seems to be an overly complex approach.
>>>> If the need for these helpers becomes apparent I can provide review comments
>>>> with the specifics about how it appears to be overly complex.
>>>>
>>>> Can you please  provide some more insights into the needs driving the desire
>>>> to have change set helpers and the expected use cases of them?  Please put
>>>> your architect's hat on when replying to this question.
>>>
>>> My use case for this is discussed in this thread:
>>> https://www.spinics.net/lists/arm-kernel/msg536111.html
>>>
>>> With the dt-bindings for the hardware-manager I want to add here:
>>> https://www.spinics.net/lists/arm-kernel/msg536109.html
>>>
>>> Note that there is a lot of discussion in this thread whether or
>>> not this belongs in the kernel. I strongly believe though that
>>> some functionality like this will be needed in the kernel for
>>> ARM+dt devices going forward, just like there is plenty of x86
>>> code which adjusts itself to specific hardware, because whether
>>> we like it or not hardware (revisions) will always have quirks.
>>
>> Thanks! That context should have been provided with the patches.
>>
>> The use case discussion is important and I am paying a lot of
>> attention to that discussion and many other discussions about
>> dynamic device trees.  I don't think it makes sense to apply the
>> change set helper patches yet, given the unsettled state of the
>> various dynamic device tree discussions.
> 
> These helpers are useful and easier to use than the existing API
> independent of any issues to sort out with how we use overlays. So I
> plan to take them whether there's a user right away or not.
> 
> Rob

OK, expect a more detailed review from me this week.

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

^ permalink raw reply

* Re: [PATCH v2 1/3] dt/binding: ti-tsc-adc: deprecate ti,charge-delay and add binding doc for ti,charge-delay-ns
From: Rob Herring @ 2016-11-15  1:54 UTC (permalink / raw)
  To: Mugunthan V N
  Cc: linux-input, Dmitry Torokhov, Jonathan Cameron, Mark Rutland,
	Lee Jones, Sekhar Nori, Vignesh R, devicetree, linux-omap,
	linux-kernel
In-Reply-To: <20161110163515.27598-2-mugunthanvnm@ti.com>

On Thu, Nov 10, 2016 at 10:05:13PM +0530, Mugunthan V N wrote:
> ti,charge-delay represents the duration that ADC should wait
> before sampling the ADC line to detect the touch location and pen
> up/downs. Currently the ADC clock is set at 3MHz. The device-tree
> entry for ti,charge-delay is based on assumption of ADC clock at
> 3MHz, but it can be operated up to 24MHz clock. Representing the
> charge delay of touchscreen in terms of ADC clocks is incorrect.
> So change this representation to ti,charge-delay-ns, which driver
> can convert it to number clock cycles based on ref clock
> frequency.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
>  .../bindings/input/touchscreen/ti-tsc-adc.txt      | 32 ++++++++++++++--------
>  1 file changed, 21 insertions(+), 11 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 1/2] devicetree: Add vendor prefix for CZ.NIC
From: Rob Herring @ 2016-11-15  1:49 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Sebastian Hesselbarth,
	Martin Strbačka, Tomas Hlavacek, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161110135721.13098-1-uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>

On Thu, Nov 10, 2016 at 02:57:20PM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
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: [RESEND PATCH] regulator: pwm: fix syntax errors in the examples
From: Rob Herring @ 2016-11-15  1:47 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Liam Girdwood, Mark Brown, Mark Rutland, devicetree
In-Reply-To: <1478773527-10936-1-git-send-email-peda@axentia.se>

qOn Thu, Nov 10, 2016 at 11:25:27AM +0100, Peter Rosin wrote:
> While at it, clean up some other things as well.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  .../devicetree/bindings/regulator/pwm-regulator.txt          | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

As this is just DT docs, I've applied.

Rob

^ permalink raw reply

* Re: [PATCHv3 1/2] gpio: xilinx: dt-binding: Add clock node
From: Rob Herring @ 2016-11-15  1:44 UTC (permalink / raw)
  To: Shubhrajyoti Datta
  Cc: linux-gpio, devicetree, soren.brinkmann, michal.simek,
	shubhrajyoti.datta
In-Reply-To: <1478757365-3311-1-git-send-email-shubhrajyoti.datta@xilinx.com>

On Thu, Nov 10, 2016 at 11:26:04AM +0530, Shubhrajyoti Datta wrote:
> Add the clock node to the dt binding.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
> v2: 
> Make clocks optional
> Add clock name
> v3:
> No change
> 
>  .../devicetree/bindings/gpio/gpio-xilinx.txt       |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v2 1/5] Documentation: pinctrl: palmas: Add ti,palmas-powerhold-override property definition
From: Rob Herring @ 2016-11-15  1:43 UTC (permalink / raw)
  To: Keerthy
  Cc: tony, lee.jones, linux-omap, linux-kernel, devicetree, linux-gpio,
	nm, t-kristo
In-Reply-To: <1478754560-27923-2-git-send-email-j-keerthy@ti.com>

On Thu, Nov 10, 2016 at 10:39:16AM +0530, Keerthy wrote:
> GPIO7 is configured in POWERHOLD mode which has higher priority
> over DEV_ON bit and keeps the PMIC supplies on even after the DEV_ON
> bit is turned off. This property enables driver to over ride the
> POWERHOLD value to GPIO7 so as to turn off the PMIC in power off
> scenarios.
> 
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  Documentation/devicetree/bindings/pinctrl/pinctrl-palmas.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
From: Rob Herring @ 2016-11-15  1:40 UTC (permalink / raw)
  To: Laurent Pinchart, Thierry Reding
  Cc: Laurent Pinchart, dri-devel,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	open list:MEDIA DRIVERS FOR RENESAS - FCP, Tomi Valkeinen,
	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1488637.i0jADhlNmg@avalon>

On Mon, Oct 17, 2016 at 7:42 AM, Laurent Pinchart
<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org> wrote:
> Hi Rob,
>
> On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
>> On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
>> > On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
>> >> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
>> >>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>> Multiple incompatible data link layers have been used over time to
>> >>> transmit image data to LVDS panels. This binding supports display
>> >>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
>> >>> specifications.
>> >>>
>> >>> Signed-off-by: Laurent Pinchart
>> >>> <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>> >>> ---
>> >>>
>> >>>  .../bindings/display/panel/panel-lvds.txt          | 119 ++++++++++++++
>> >>>  1 file changed, 119 insertions(+)
>> >>>  create mode 100644
>> >>>  Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
>> >>>
>> >>> diff --git
>> >>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> new file mode 100644
>> >>> index 000000000000..250861f2673e
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
>> >>> @@ -0,0 +1,119 @@
>> >>> +Generic LVDS Panel
>> >>> +==================
>> >>> +
>> >>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
>> >>> Multiple
>> >>> +incompatible data link layers have been used over time to transmit
>> >>> image data
>> >>> +to LVDS panels. This bindings supports display panels compatible with
>> >>> the
>> >>> +following specifications.
>> >>> +
>> >>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
>> >>> February
>> >>> +1999 (Version 1.0), Japan Electronic Industry Development Association
>> >>> (JEIDA)
>> >>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95), National
>> >>> +Semiconductor
>> >>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
>> >>> Video
>> >>> +Electronics Standards Association (VESA)
>> >>> +
>> >>> +Device compatible with those specifications have been marketed under
>> >>> the
>> >>> +FPD-Link and FlatLink brands.
>> >>> +
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: shall contain "panel-lvds"
>> >>
>> >> Maybe as a fallback, but on its own, no way.
>> >
>> > Which brings an interesting question: when designing generic DT bindings,
>> > what's the rule regarding
>
> Looks like I forgot part of the question. I meant to ask what is the rule
> regarding usage of more precise compatible strings ?

When in doubt, always have one. If there's any chance at all that s/w
will need to know or care, then we should have one.

> For instance (but perhaps not the best example), the input/rotary-encoder.txt
> bindings define a "rotary-encoder" compatible string, with no other bindings
> defining more precise compatible strings for the exact rotary encoder model.
> When it comes to panels I believe it makes sense to define model-specific
> compatible strings even if they're unused by drivers. I'm however wondering
> what the rule is there, and where those device-specific compatible strings
> should be defined. I'd like to avoid using one file per panel model as done
> today for the simple-panel bindings.

There's a few exceptions like this where there is not any sort of
model to base a compatible on. For example, a GPIO connected LED is
truly generic. The only way to have a more specific compatible would
be something with the board name in it.

Your case here is in the middle. It seems like it's generic and
passive, but perhaps power control is not. Rather than trying to
decide, we can just cover our ass and put both a generic and specific
compatible in.

>> Call it "simple" so I can easily NAK it. :)
>>
>> Define a generic structure, not a single binding trying to serve all.
>>
>> >> > +- width-mm: panel display width in millimeters
>> >> > +- height-mm: panel display height in millimeters
>> >>
>> >> This is already documented for all panels IIRC.
>> >
>> > Note that this DT binding has nothing to do with the simple-panel binding.
>> > It is instead similar to the panel-dpi and panel-dsi-cm bindings (which
>> > currently don't but should specify the panel size in DT). The LVDS panel
>> > driver will *not* include any panel-specific information such as size or
>> > timings, these are specified in DT.
>>
>> The panel bindings aren't really different. The biggest difference was
>> location in the tree, but we now generally allow panels to be either a
>> child of the LCD controller or connected with OF graph. We probably
>> need to work on restructuring the panel bindings a bit. We should have
>> an inheritance with a base panel binding of things like size, label,
>> graph, backlight, etc, then perhaps an interface specific bindings for
>> LVDS, DSI, and parallel, then a panel specific binding. With this the
>> panel specific binding is typically just a compatible string and which
>> inherited properties apply to it.
>
> That sounds good to me, but we have multiple models for panel bindings.
>
> As you mentioned panels can be referenced through an LCD controller node
> property containing a phandle to the panel node, or through OF graph. That's a
> situation we have today, and we need to keep supporting both (at least for
> existing panels, perhaps not for the new ones).
>
> Another difference is how to express panel data such as size and timings. The
> simple-panel DT bindings don't contain such data and expects the drivers to
> contain a table of panel data for all models supported, while the DPI, DSI and
> now the proposed LVDS panel bindings contain properties for panel data.
>
> How would you like to reconcile all that ?

Thierry has outlined the position[1] that simple-panel follows many
times and I generally agree. I could be convinced that perhaps panel
timings could go into DT. However, you can't really describe
*everything*, so we're not going to get away from panel specific
compatible strings and panel specifics in the kernel.

>> >>> +- data-mapping: the color signals mapping order, "jeida-18",
>> >>> "jeida-24"
>> >>> +  or "vesa-24"
>> >>
>> >> Maybe this should be part of the compatible.
>> >
>> > I've thought about it, but given that some panels support selecting
>> > between multiple modes (through a mode pin that is usually hardwired), I
>> > believe a separate DT property makes sense.
>>
>> Okay.
>>
>> > Furthermore, LVDS data organization is controlled by the combination of
>> > both data-mapping and data-mirror. It makes little sense from my point of
>> > view to handle one as part of the compatible string and the other one as
>> > a separate property.
>> >
>> >> > +Optional properties:
>> >> > +- label: a symbolic name for the panel
>> >>
>> >> Could be for any panel or display connector.
>> >
>> > Yes, but I'm not sure to understand how that's relevant :-)
>>
>> Meaning it should be a common property.
>
> Sure. So you expect me to reorganize all the panels and connectors DT bindings
> in order to get this one merged ? :-)

No, because I don't think label is widely defined. Just put it in a
common place and reference it. Any other panels can be fixed later.
Really, the "simple panel" binding should probably morph into the
common binding.

>> >>> +- avdd-supply: reference to the regulator that powers the panel
>> >>> analog supply
>> >>> +- dvdd-supply: reference to the regulator that powers the panel
>> >>> digital supply
>> >>
>> >> Which one has to be powered on first, what voltage, and with what time
>> >> in between? This is why "generic" or "simple" bindings don't work.
>> >
>> > The above-mentioned specifications also define connectors, pinouts and
>> > power supplies, but many LVDS panels compatible with the LVDS physical
>> > and data layers use a different connector with small differences in power
>> > supplies.
>> >
>> > I believe the voltage is irrelevant here, it doesn't need to be controlled
>> > by the operating system. Power supplies order and timing is relevant,
>> > I'll investigate the level of differences between panels. I'm also fine
>> > with dropping those properties for now.
>>
>> Whether you have control of the supplies is dependent on the board.
>> Dropping them is just puts us in the simple binding trap. The simple
>> bindings start out that way and then people keep adding to them.
>
> Damn, you can't be fooled easily ;-)

I guess you can count all the simple bindings to see how many times I
can be fooled. :)

> On a more serious note, I'd like to design the bindings in a way that wouldn't
> require adding device-specific code in the driver for each panel model, given
> that in most cases power supply handling will be generic. What's your opinion
> about a generic power supply model that would be used in the default case,
> with the option to override it with device-specific code when needed ?

I don't agree. Read Thierry's post on the subject[1].

Rob

[1] http://sietch-tagr.blogspot.com/2016/04/display-panels-are-not-special.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] [media] mtk-mdp: allocate video_device dynamically
From: Minghsiu Tsai @ 2016-11-15  1:37 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Hans Verkuil, daniel.thompson-QSEj5FYQhm4dnm+yROfE0A, Rob Herring,
	Mauro Carvalho Chehab, Matthias Brugger, Daniel Kurtz,
	Pawel Osciak, srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Eddie Huang,
	Yingjoe Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <d21cd419-e1fc-4d2e-a2a9-74c535865762-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>

On Fri, 2016-11-11 at 11:51 +0100, Hans Verkuil wrote:
> Almost correct:
> 
> On 11/07/2016 01:42 PM, Minghsiu Tsai wrote:
> > It can fix known problems with embedded video_device structs.
> > 
> > Signed-off-by: Minghsiu Tsai <minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/media/platform/mtk-mdp/mtk_mdp_core.h |  2 +-
> >  drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c  | 33 ++++++++++++++++-----------
> >  2 files changed, 21 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> > index 848569d..ad1cff3 100644
> > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> > @@ -167,7 +167,7 @@ struct mtk_mdp_dev {
> >  	struct mtk_mdp_comp		*comp[MTK_MDP_COMP_ID_MAX];
> >  	struct v4l2_m2m_dev		*m2m_dev;
> >  	struct list_head		ctx_list;
> > -	struct video_device		vdev;
> > +	struct video_device		*vdev;
> >  	struct v4l2_device		v4l2_dev;
> >  	struct workqueue_struct		*job_wq;
> >  	struct platform_device		*vpu_dev;
> > diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> > index 9a747e7..b8dee1c 100644
> > --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> > +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> > @@ -1236,16 +1236,22 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
> >  	int ret;
> >  
> >  	mdp->variant = &mtk_mdp_default_variant;
> > -	mdp->vdev.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> > -	mdp->vdev.fops = &mtk_mdp_m2m_fops;
> > -	mdp->vdev.ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> > -	mdp->vdev.release = video_device_release_empty;
> > -	mdp->vdev.lock = &mdp->lock;
> > -	mdp->vdev.vfl_dir = VFL_DIR_M2M;
> > -	mdp->vdev.v4l2_dev = &mdp->v4l2_dev;
> > -	snprintf(mdp->vdev.name, sizeof(mdp->vdev.name), "%s:m2m",
> > +	mdp->vdev = video_device_alloc();
> > +	if (!mdp->vdev) {
> > +		dev_err(dev, "failed to allocate video device\n");
> > +		ret = -ENOMEM;
> > +		goto err_video_alloc;
> > +	}
> > +	mdp->vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> > +	mdp->vdev->fops = &mtk_mdp_m2m_fops;
> > +	mdp->vdev->ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> > +	mdp->vdev->release = video_device_release;
> > +	mdp->vdev->lock = &mdp->lock;
> > +	mdp->vdev->vfl_dir = VFL_DIR_M2M;
> > +	mdp->vdev->v4l2_dev = &mdp->v4l2_dev;
> > +	snprintf(mdp->vdev->name, sizeof(mdp->vdev->name), "%s:m2m",
> >  		 MTK_MDP_MODULE_NAME);
> > -	video_set_drvdata(&mdp->vdev, mdp);
> > +	video_set_drvdata(mdp->vdev, mdp);
> >  
> >  	mdp->m2m_dev = v4l2_m2m_init(&mtk_mdp_m2m_ops);
> >  	if (IS_ERR(mdp->m2m_dev)) {
> > @@ -1254,26 +1260,27 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
> >  		goto err_m2m_init;
> >  	}
> >  
> > -	ret = video_register_device(&mdp->vdev, VFL_TYPE_GRABBER, 2);
> > +	ret = video_register_device(mdp->vdev, VFL_TYPE_GRABBER, 2);
> >  	if (ret) {
> >  		dev_err(dev, "failed to register video device\n");
> >  		goto err_vdev_register;
> >  	}
> >  
> >  	v4l2_info(&mdp->v4l2_dev, "driver registered as /dev/video%d",
> > -		  mdp->vdev.num);
> > +		  mdp->vdev->num);
> >  	return 0;
> >  
> >  err_vdev_register:
> >  	v4l2_m2m_release(mdp->m2m_dev);
> >  err_m2m_init:
> > -	video_device_release(&mdp->vdev);
> > +	video_unregister_device(mdp->vdev);
> 
> This should remain video_device_release: the video_register_device call failed, so
> the device hasn't been registered. In that case just release the device (i.e.
> free the allocated memory).
> 
Hi Hans,

I have uploaded patch v2 for this. Thanks for your review.


Best regards,
Ming Hsiu

> > +err_video_alloc:
> >  
> >  	return ret;
> >  }
> >  
> >  void mtk_mdp_unregister_m2m_device(struct mtk_mdp_dev *mdp)
> >  {
> > -	video_device_release(&mdp->vdev);
> > +	video_unregister_device(mdp->vdev);
> >  	v4l2_m2m_release(mdp->m2m_dev);
> >  }
> > 
> 
> Regards,
> 
> 	Hans


--
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 v2] [media] mtk-mdp: allocate video_device dynamically
From: Minghsiu Tsai @ 2016-11-15  1:34 UTC (permalink / raw)
  To: Hans Verkuil, daniel.thompson, Rob Herring, Mauro Carvalho Chehab,
	Matthias Brugger, Daniel Kurtz, Pawel Osciak
  Cc: srv_heupstream, Eddie Huang, Yingjoe Chen, devicetree,
	linux-kernel, linux-arm-kernel, linux-media, linux-mediatek,
	Minghsiu Tsai

It can fix known problems with embedded video_device structs.

Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>

---
Changes in v2:
. Call video_device_release() instead of video_unregister_device()
  in mtk_mdp_register_m2m_device()


---
 drivers/media/platform/mtk-mdp/mtk_mdp_core.h |  2 +-
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c  | 33 ++++++++++++++++-----------
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
index 848569d..ad1cff3 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
@@ -167,7 +167,7 @@ struct mtk_mdp_dev {
 	struct mtk_mdp_comp		*comp[MTK_MDP_COMP_ID_MAX];
 	struct v4l2_m2m_dev		*m2m_dev;
 	struct list_head		ctx_list;
-	struct video_device		vdev;
+	struct video_device		*vdev;
 	struct v4l2_device		v4l2_dev;
 	struct workqueue_struct		*job_wq;
 	struct platform_device		*vpu_dev;
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
index 9a747e7..13afe48 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
@@ -1236,16 +1236,22 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
 	int ret;
 
 	mdp->variant = &mtk_mdp_default_variant;
-	mdp->vdev.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
-	mdp->vdev.fops = &mtk_mdp_m2m_fops;
-	mdp->vdev.ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
-	mdp->vdev.release = video_device_release_empty;
-	mdp->vdev.lock = &mdp->lock;
-	mdp->vdev.vfl_dir = VFL_DIR_M2M;
-	mdp->vdev.v4l2_dev = &mdp->v4l2_dev;
-	snprintf(mdp->vdev.name, sizeof(mdp->vdev.name), "%s:m2m",
+	mdp->vdev = video_device_alloc();
+	if (!mdp->vdev) {
+		dev_err(dev, "failed to allocate video device\n");
+		ret = -ENOMEM;
+		goto err_video_alloc;
+	}
+	mdp->vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
+	mdp->vdev->fops = &mtk_mdp_m2m_fops;
+	mdp->vdev->ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
+	mdp->vdev->release = video_device_release;
+	mdp->vdev->lock = &mdp->lock;
+	mdp->vdev->vfl_dir = VFL_DIR_M2M;
+	mdp->vdev->v4l2_dev = &mdp->v4l2_dev;
+	snprintf(mdp->vdev->name, sizeof(mdp->vdev->name), "%s:m2m",
 		 MTK_MDP_MODULE_NAME);
-	video_set_drvdata(&mdp->vdev, mdp);
+	video_set_drvdata(mdp->vdev, mdp);
 
 	mdp->m2m_dev = v4l2_m2m_init(&mtk_mdp_m2m_ops);
 	if (IS_ERR(mdp->m2m_dev)) {
@@ -1254,26 +1260,27 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
 		goto err_m2m_init;
 	}
 
-	ret = video_register_device(&mdp->vdev, VFL_TYPE_GRABBER, 2);
+	ret = video_register_device(mdp->vdev, VFL_TYPE_GRABBER, 2);
 	if (ret) {
 		dev_err(dev, "failed to register video device\n");
 		goto err_vdev_register;
 	}
 
 	v4l2_info(&mdp->v4l2_dev, "driver registered as /dev/video%d",
-		  mdp->vdev.num);
+		  mdp->vdev->num);
 	return 0;
 
 err_vdev_register:
 	v4l2_m2m_release(mdp->m2m_dev);
 err_m2m_init:
-	video_device_release(&mdp->vdev);
+	video_device_release(mdp->vdev);
+err_video_alloc:
 
 	return ret;
 }
 
 void mtk_mdp_unregister_m2m_device(struct mtk_mdp_dev *mdp)
 {
-	video_device_release(&mdp->vdev);
+	video_unregister_device(mdp->vdev);
 	v4l2_m2m_release(mdp->m2m_dev);
 }
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v2 05/30] usb: dwc2: Add bindings to disable gadget DMA modes
From: John Youn @ 2016-11-15  1:26 UTC (permalink / raw)
  To: Rob Herring, John Youn
  Cc: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Rutland
In-Reply-To: <20161115011110.b32eljlv42mhbocr@rob-hp-laptop>

On 11/14/2016 5:11 PM, Rob Herring wrote:
> On Wed, Nov 09, 2016 at 07:27:50PM -0800, John Youn wrote:
>> Now that the gadget driver automatically detects DMA modes, we need to
>> provide a way to disable them. Certain platforms may still have issues
>> with DMA and require it to be disabled. It is also needed for IP
>> validation purposes.
> 
> I don't think IP validation is reason enough for these properties. If 
> there is IP that is broken and lies about it, then yes we should have 
> these.
> 
> Rob
> 

Ok. Mark said basically the same thing in a previous thread that lead
to this patch.

Until they are needed by hardware, I'll remove these bindings.

Regards,
John
--
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] arm64: Add DTS support for FSL's LS1012A SoC
From: Shawn Guo @ 2016-11-15  1:18 UTC (permalink / raw)
  To: Harninder Rai
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Bhaskar Upadhaya, oss-fOR+EgIDQEHk1uMJSBkQmQ,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161114071308.GL3310@dragon>

On Mon, Nov 14, 2016 at 03:13:10PM +0800, Shawn Guo wrote:
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index 6602718..39db645 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -1,3 +1,6 @@
> > +dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-frdm.dtb
> > +dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-qds.dtb
> > +dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-rdb.dtb
> >  dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-qds.dtb
> >  dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-rdb.dtb
> >  dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1046a-qds.dtb
> 
> It seems that the patch is not generated against mainline kernel.  We do
> not have fsl-ls1046a-qds.dtb target in mainline kernel.

Sorry, forgot that we just added it in this development cycle.

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

^ permalink raw reply

* Re: [PATCH v2 05/30] usb: dwc2: Add bindings to disable gadget DMA modes
From: Rob Herring @ 2016-11-15  1:11 UTC (permalink / raw)
  To: John Youn
  Cc: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland
In-Reply-To: <16733114b9252b8163c11cc7ca95b2b152b66e0e.1478748145.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

On Wed, Nov 09, 2016 at 07:27:50PM -0800, John Youn wrote:
> Now that the gadget driver automatically detects DMA modes, we need to
> provide a way to disable them. Certain platforms may still have issues
> with DMA and require it to be disabled. It is also needed for IP
> validation purposes.

I don't think IP validation is reason enough for these properties. If 
there is IP that is broken and lies about it, then yes we should have 
these.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: [RESEND PATCH 1/2] PCI: rockchip: cleanup bit definition for PCIE_RC_CONFIG_LCS
From: Shawn Lin @ 2016-11-15  1:09 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Bjorn Helgaas, Rob Herring, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Wenrui Li,
	Brian Norris, Jeffy Chen, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161114222604.GJ9868-1RhO1Y9PlrlHTL0Zs8A6p5iNqAH0jzoTYJqu5kTmcBRl57MIdRCFDg@public.gmane.org>

Hi Bjorn,

On 2016/11/15 6:26, Bjorn Helgaas wrote:
> On Mon, Nov 14, 2016 at 12:11:05PM +0800, Shawn Lin wrote:
>> PCIE_RC_CONFIG_LCS contains control and status bits specific
>> to the PCIe link. The layout for this register looks the same
>> as the existed PCI_EXP_LNKCTL and PCI_EXP_LNKSTA. So let's
>> reuse them.
>>
>> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> Did something change since the version you posted yesterday?
> Resending a patch with no changes or with no hint about what changed
> doesn't speed things up; in fact, it slows things down.
>

Sorry for that, mea culpa. I was finding I had sent a wrong patch which
couldn't be compiled cleanly, so I resent a new one. I should directly
respin a new version for that case.

>> ---
>>
>>  drivers/pci/host/pcie-rockchip.c | 14 ++++----------
>>  1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
>> index 7f238af..1dba698 100644
>> --- a/drivers/pci/host/pcie-rockchip.c
>> +++ b/drivers/pci/host/pcie-rockchip.c
>> @@ -141,12 +141,6 @@
>>  #define   PCIE_RC_CONFIG_DCR_CSPL_LIMIT		0xff
>>  #define   PCIE_RC_CONFIG_DCR_CPLS_SHIFT		26
>>  #define PCIE_RC_CONFIG_LCS		(PCIE_RC_CONFIG_BASE + 0xd0)
>> -#define   PCIE_RC_CONFIG_LCS_RETRAIN_LINK	BIT(5)
>> -#define   PCIE_RC_CONFIG_LCS_CCC		BIT(6)
>> -#define   PCIE_RC_CONFIG_LCS_LBMIE		BIT(10)
>> -#define   PCIE_RC_CONFIG_LCS_LABIE		BIT(11)
>> -#define   PCIE_RC_CONFIG_LCS_LBMS		BIT(30)
>> -#define   PCIE_RC_CONFIG_LCS_LAMS		BIT(31)
>>  #define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
>>  #define PCIE_RC_CONFIG_THP_CAP		(PCIE_RC_CONFIG_BASE + 0x274)
>>  #define   PCIE_RC_CONFIG_THP_CAP_NEXT_MASK	GENMASK(31, 20)
>> @@ -229,7 +223,7 @@ static void rockchip_pcie_enable_bw_int(struct rockchip_pcie *rockchip)
>>  	u32 status;
>>
>>  	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
>> -	status |= (PCIE_RC_CONFIG_LCS_LBMIE | PCIE_RC_CONFIG_LCS_LABIE);
>> +	status |= (PCI_EXP_LNKCTL_LBMIE | PCI_EXP_LNKCTL_LABIE);
>>  	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
>>  }
>>
>> @@ -238,7 +232,7 @@ static void rockchip_pcie_clr_bw_int(struct rockchip_pcie *rockchip)
>>  	u32 status;
>>
>>  	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
>> -	status |= (PCIE_RC_CONFIG_LCS_LBMS | PCIE_RC_CONFIG_LCS_LAMS);
>> +	status |= (PCI_EXP_LNKSTA_LBMS | PCI_EXP_LNKSTA_LABS) << 16;
>>  	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
>>  }
>>
>> @@ -540,7 +534,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>>
>>  	/* Set RC's clock architecture as common clock */
>>  	status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
>> -	status |= PCIE_RC_CONFIG_LCS_CCC;
>> +	status |= PCI_EXP_LNKCTL_CCC;
>>  	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
>>
>>  	/* Enable Gen1 training */
>> @@ -575,7 +569,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
>>  		 * gen1 finished.
>>  		 */
>>  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LCS);
>> -		status |= PCIE_RC_CONFIG_LCS_RETRAIN_LINK;
>> +		status |= PCI_EXP_LNKCTL_RL;
>>  		rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LCS);
>>
>>  		timeout = jiffies + msecs_to_jiffies(500);
>> --
>> 1.9.1
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Best Regards
Shawn Lin

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

^ permalink raw reply

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

On Mon, Nov 14, 2016 at 03:04:20PM +0800, Peter Chen wrote:
> diff --git a/arch/arm/boot/dts/imx6ull-pinfunc.h b/arch/arm/boot/dts/imx6ull-pinfunc.h
> new file mode 100644
> index 0000000..580b5c3
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6ull-pinfunc.h
> @@ -0,0 +1,56 @@
> +/*
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __DTS_IMX6ULL_PINFUNC_H
> +#define __DTS_IMX6ULL_PINFUNC_H
> +
> +#include "imx6ul-pinfunc.h"
> +/*
> + * The pin function ID is a tuple of
> + * <mux_reg conf_reg input_reg mux_mode input_val>
> + */
> +#define MX6ULL_PAD_ENET2_RX_DATA0__EPDC_SDDO08                    0x00E4 0x0370 0x0000 0x9 0x0
> +#define MX6ULL_PAD_ENET2_RX_DATA1__EPDC_SDDO09                    0x00E8 0x0374 0x0000 0x9 0x0
> +#define MX6ULL_PAD_ENET2_RX_EN__EPDC_SDDO10                       0x00EC 0x0378 0x0000 0x9 0x0
> +#define MX6ULL_PAD_ENET2_TX_DATA0__EPDC_SDDO11                    0x00F0 0x037C 0x0000 0x9 0x0
> +#define MX6ULL_PAD_ENET2_TX_DATA1__EPDC_SDDO12                    0x00F4 0x0380 0x0000 0x9 0x0
> +#define MX6ULL_PAD_ENET2_TX_EN__EPDC_SDDO13                       0x00F8 0x0384 0x0000 0x9 0x0
> +#define MX6ULL_PAD_ENET2_TX_CLK__EPDC_SDDO14                      0x00FC 0x0388 0x0000 0x9 0x0
> +#define MX6ULL_PAD_ENET2_RX_ER__EPDC_SDDO15                       0x0100 0x038C 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_CLK__EPDC_SDCLK                            0x0104 0x0390 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_ENABLE__EPDC_SDLE                          0x0108 0x0394 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_HSYNC__EPDC_SDOE                           0x010C 0x0398 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_VSYNC__EPDC_SDCE0                          0x0110 0x039C 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_RESET__EPDC_GDOE                           0x0114 0x03A0 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA00__EPDC_SDDO00                        0x0118 0x03A4 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA01__EPDC_SDDO01                        0x011C 0x03A8 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA02__EPDC_SDDO02                        0x0120 0x03AC 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA03__EPDC_SDDO03                        0x0124 0x03B0 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA04__EPDC_SDDO04                        0x0128 0x03B4 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA05__EPDC_SDDO05                        0x012C 0x03B8 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA06__EPDC_SDDO06                        0x0130 0x03BC 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA07__EPDC_SDDO07                        0x0134 0x03C0 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA14__EPDC_SDSHR                         0x0150 0x03DC 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA15__EPDC_GDRL                          0x0154 0x03E0 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA16__EPDC_GDCLK                         0x0158 0x03E4 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA17__EPDC_GDSP                          0x015C 0x03E8 0x0000 0x9 0x0
> +#define MX6ULL_PAD_LCD_DATA21__EPDC_SDCE1                         0x016C 0x03F8 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_MCLK__ESAI_TX3_RX2                          0x01D4 0x0460 0x0000 0x9 0x0

Not sure why the indentation goes bad from here.  I fixed it up and
applied all 3 patches.

Shawn

> +#define MX6ULL_PAD_CSI_PIXCLK__ESAI_TX2_RX3                        0x01D8 0x0464 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_VSYNC__ESAI_TX4_RX1                         0x01DC 0x0468 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_HSYNC__ESAI_TX1                             0x01E0 0x046C 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_DATA00__ESAI_TX_HF_CLK                      0x01E4 0x0470 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_DATA01__ESAI_RX_HF_CLK                      0x01E8 0x0474 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_DATA02__ESAI_RX_FS                          0x01EC 0x0478 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_DATA03__ESAI_RX_CLK                         0x01F0 0x047C 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_DATA04__ESAI_TX_FS                          0x01F4 0x0480 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_DATA05__ESAI_TX_CLK                         0x01F8 0x0484 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_DATA06__ESAI_TX5_RX0                        0x01FC 0x0488 0x0000 0x9 0x0
> +#define MX6ULL_PAD_CSI_DATA07__ESAI_T0                             0x0200 0x048C 0x0000 0x9 0x0
> +
> +#endif /* __DTS_IMX6ULL_PINFUNC_H */
--
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


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