Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values
From: Viresh Kumar @ 2017-12-27  8:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, Viresh Kumar,
	Nishanth Menon, Stephen Boyd, Rafael Wysocki,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot,
	rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171226202955.32j7gzonrixtwdpt@rob-hp-laptop>

On 26-12-17, 14:29, Rob Herring wrote:
> On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote:

> > +On some platforms the exact frequency or voltage may be hidden from the OS by
> > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain
> > +magic values that represent the frequency or voltage in a firmware dependent
> > +way, for example an index of an array in the firmware.
> 
> I'm still not convinced this is a good idea.

You were kind-of a few days back :)

lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

So here is the deal:

- I proposed "domain-performance-state" property for this stuff
  initially.
- But Kevin didn't like that and proposed reusing "opp-hz" and
  "opp-microvolt", which we all agreed to multiple times..
- And we are back to the same discussion now and its painful and time
  killing for all of us.

TBH, I don't have too strong preferences about any of the suggestions
you guys have and I need you guys to tell me what binding changes to
do here and I will do that.

> If you have firmware 
> partially managing things, then I think we should have platform specific 
> bindings or drivers. 

What about the initial idea then, like "performance-state" for the
power domains ? All platforms will anyway replicate that binding only.

> This is complex enough I'm not taking silence from Stephen as an okay.

Sure, but I am not sure how to make him speak :)

-- 
viresh
--
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] arm64: dts: Hi3660: Fix up psci state id
From: Vincent Guittot @ 2017-12-27  8:29 UTC (permalink / raw)
  To: Leo Yan
  Cc: Wei Xu, Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	LAK, devicetree, linux-kernel, Daniel Lezcano, Sudeep Holla,
	Soby Mathew, John Stultz, Wangtao (Kevin, Kirin)
In-Reply-To: <20171225022202.GB600@leoy-linaro>

Hi Leo

On 25 December 2017 at 03:22, Leo Yan <leo.yan@linaro.org> wrote:
> Hi Vincent,
>
> [ + John, Kevin Wang ]
>
> On Fri, Dec 22, 2017 at 03:22:51PM +0100, Vincent Guittot wrote:
>> Hi Leo,
>>
>> Sorry for jumping late in the discussion but should  we also remove
>> the NAP state from the property cpu-idle-states of the CPUs because
>> this state not supported by the platform at least for now and may be
>> not in a near future ?
>
> Thanks for bringing up this.
>
> I don't want to hide anything for patch discussion :) this patch is to
> resolve the PSCI parameter mismatching issue between kernel and ARM-TF
> and it's not used to resolve the bug for CPU_NAP, so I didn't mention
> the CPU_NAP malfunction issue to avoid complex discussion context.
>
> I want to keep CPU_NAP state and track bug for CPU_NAP fixing; if we
> remove this state, I suspect we might have no chance to enable it
> anymore. Finally this is up to Hisilicon colleague decision and if they
> have time to fix this.
>
> I will offline to check with Daniel and Kevin for this; and if we
> finally decide to remove it we can commit extra patch for this later,
> how about you think?

I would prefer to remove it right now.
Removing NAP from c-state table makes the hikey960 working correctly;
I mean even with current ATF and current state id. So it's the best
solution to the NAP problem IMO and I don't see the benefit of keeping
NAP in the table until this state has been fixed. This will just add
uncertainties in the behavior of the board.
I don't see why you can't re-add it once it has been fixed.

>
>> Then, I have another question regarding the update of the
>> psci-suspend-parameter. These changes implies an update of the psci
>> firmawre which means that we will now have 2 different firmware
>> version compatible with 2 different dt.
>>
>> Is there any way to check that the ATF on the board is the one that
>> compatible with the parameter with something like a version ? I
>> currently use the previous firmware which works fine with current
>> kernel and dt binding once the NAP state is removed from the table.
>> When moving on recent kernel, I will have to take care of updating the
>> firmware and if i need to go back on a previous kernel, i will have to
>> make sure that i have the right ATF version. This make a lot of chance
>> of having the wrong configuration
>
> AFAIK, we cannot distinguish the PSCI parameter by PSCI version or

And that's my main concern because this adds a new possible regression
factor when switching between different kernel version

> ARM-TF version number; alternatively one simple way for checking ARM-TF
> is we can get commit ID (e.g. 83df7ce) from the ARM-TF log; so any
> ARM-TF commit ID is newer than the patch fdae60b6ba27: "Hikey960:
> Change to use recommended power state id format" should apply this
> kernel patch.
>
> NOTICE:  BL1: Booting BL31
> NOTICE:  BL31: v1.4(debug):v1.4-441-g83df7ce-dirty
> NOTICE:  BL31: Built : 17:31:35, Dec 22 2017
>
> BTW, I hope we can upgrade Linux kernel and ARM-TF to latest code base
> to avoid compatible issue; for Android offical releasing it uses the
> old PSCI parameters with Hisilicon legacy booting images, so they can
> work well, but if someone uses ARM-TF mainline code + Android kernel
> 4.4/4.9, there must have compatible issue.
>
> I am monitoring the integration ARM-TF/UEFI into Android on Hikey960,
> we need backport this patch onto Android kernel 4.4/4.9 ASAP after
> integration ARM-TF/UEFI.
>
> Thanks,
> Leo Yan
>
>> Regards,
>> Vincent
>>
>> On 12 December 2017 at 10:12, Leo Yan <leo.yan@linaro.org> wrote:
>> > Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
>> > idle state.  From ftrace log we can observe CA73 CPUs can be easily
>> > waken up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle
>> > anything and sleep again; so there have tons of trace events for CA73
>> > CPUs entering and exiting idle state.
>> >
>> > On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
>> > set its psci parameter as '0x0000001' and from this parameter it can
>> > calculate state id is 1.  Unfortunately ARM trusted firmware (ARM-TF)
>> > takes 1 as a invalid value for state id, so the CPU cannot enter idle
>> > state and directly bail out to kernel.
>> >
>> > We want to create good practice for psci parameters platform definition,
>> > so review the psci specification. The spec "ARM Power State Coordination
>> > Interface - Platform Design Document (ARM DEN 0022D)" recommends state
>> > ID in chapter "6.5 Recommended StateID Encoding".  The recommended power
>> > state IDs can be presented by below listed values; and it divides into
>> > three fields, every field can use 4 bits to present power states
>> > corresponding to core level, cluster level and system level:
>> >   0: Run
>> >   1: Standby
>> >   2: Retention
>> >   3: Powerdown
>> >
>> > This commit changes psci parameter to compliance with the suggested
>> > state ID in the doc.  Except we change 'CPU_NAP' state psci parameter
>> > to '0x0000002', this commit also changes 'CPU_SLEEP' and 'CLUSTER_SLEEP'
>> > state parameters to '0x0010003' and '0x1010033' respectively.
>> >
>> > Credits to Daniel, Sudeep and Soby for suggestion and consolidation.
>> >
>> > Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> > Cc: Sudeep Holla <sudeep.holla@arm.com>
>> > Cc: Soby Mathew <Soby.Mathew@arm.com>
>> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
>> > ---
>> >  arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 8 ++++----
>> >  1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > index ab0b95b..99d5a46 100644
>> > --- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > +++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
>> > @@ -147,7 +147,7 @@
>> >
>> >                         CPU_NAP: cpu-nap {
>> >                                 compatible = "arm,idle-state";
>> > -                               arm,psci-suspend-param = <0x0000001>;
>> > +                               arm,psci-suspend-param = <0x0000002>;
>> >                                 entry-latency-us = <7>;
>> >                                 exit-latency-us = <2>;
>> >                                 min-residency-us = <15>;
>> > @@ -156,7 +156,7 @@
>> >                         CPU_SLEEP: cpu-sleep {
>> >                                 compatible = "arm,idle-state";
>> >                                 local-timer-stop;
>> > -                               arm,psci-suspend-param = <0x0010000>;
>> > +                               arm,psci-suspend-param = <0x0010003>;
>> >                                 entry-latency-us = <40>;
>> >                                 exit-latency-us = <70>;
>> >                                 min-residency-us = <3000>;
>> > @@ -165,7 +165,7 @@
>> >                         CLUSTER_SLEEP_0: cluster-sleep-0 {
>> >                                 compatible = "arm,idle-state";
>> >                                 local-timer-stop;
>> > -                               arm,psci-suspend-param = <0x1010000>;
>> > +                               arm,psci-suspend-param = <0x1010033>;
>> >                                 entry-latency-us = <500>;
>> >                                 exit-latency-us = <5000>;
>> >                                 min-residency-us = <20000>;
>> > @@ -174,7 +174,7 @@
>> >                         CLUSTER_SLEEP_1: cluster-sleep-1 {
>> >                                 compatible = "arm,idle-state";
>> >                                 local-timer-stop;
>> > -                               arm,psci-suspend-param = <0x1010000>;
>> > +                               arm,psci-suspend-param = <0x1010033>;
>> >                                 entry-latency-us = <1000>;
>> >                                 exit-latency-us = <5000>;
>> >                                 min-residency-us = <20000>;
>> > --
>> > 2.7.4
>> >

^ permalink raw reply

* Re: [PATCHv3 1/3] dt-bindings: net: Add DT bindings for Socionext Netsec
From: Andrew Lunn @ 2017-12-27  8:28 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: jassisinghbrar, netdev, devicetree, davem, arnd.bergmann,
	ard.biesheuvel, robh+dt, mark.rutland, masami.hiramatsu,
	Jassi Brar
In-Reply-To: <f42d29cc-6b3f-20dc-3f42-bc9a9fc7ce4a@gmail.com>

On Tue, Dec 26, 2017 at 11:34:26AM -0800, Florian Fainelli wrote:
> On 12/21/2017 05:55 AM, Andrew Lunn wrote:
> >> +- mdio device tree subnode: When the Netsec has a phy connected to its local
> >> +		mdio, there must be device tree subnode with the following
> >> +		required properties:
> >> +
> >> +	- compatible: Must be "socionext,snq-mdio".
> > 
> > Is there a need for a compatible string? Is there different versions
> > of the MDIO bus hardware? If it was an independent MDIO bus driver,
> > then yes, you need a compatible string. But since it is embedded in
> > the MAC driver, there should not be a need.
> 
> I don't see a problem with doing that though, it may be extra
> information, but if we ever have a standalone MDIO bus controller that
> happens to be supported by the same HW/driver, and the Ethernet driver
> delegates the MDIO management to this MDIO bus driver, then having the
> compatible string is kind of mandatory for proper matching/identification.

Hi Florian

I does not create a problem. It does however make it different to
every other embedded MDIO bus.

My preference is not to have the compatible string, but i can live
with it.

      Andrew

^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: PCI: dra7xx: Add properties to enable x2 lane in dra7
From: Kishon Vijay Abraham I @ 2017-12-27  6:40 UTC (permalink / raw)
  To: Tony Lindgren, Rob Herring
  Cc: Lorenzo Pieralisi, Bjorn Helgaas, Mark Rutland, linux-omap,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sekhar Nori
In-Reply-To: <20171226181009.GE3875-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

Hi,

On Tuesday 26 December 2017 11:40 PM, Tony Lindgren wrote:
> * Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [171226 17:49]:
>> On Fri, Dec 22, 2017 at 12:24 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
>>> * Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> [171222 06:06]:
>>>> Hi Rob,
>>>>
>>>> On Thursday 21 December 2017 12:27 AM, Rob Herring wrote:
>>>>> On Tue, Dec 19, 2017 at 02:28:22PM +0530, Kishon Vijay Abraham I wrote:
>>>>>> Add syscon properties required for configuring PCIe in x2 lane mode.
>>>>>>
>>>>>> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>>>>>> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
>>>>>> ---
>>>>>>  Documentation/devicetree/bindings/pci/ti-pci.txt | 6 ++++++
>>>>>>  1 file changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pci/ti-pci.txt b/Documentation/devicetree/bindings/pci/ti-pci.txt
>>>>>> index 82cb875e4cec..bfbc77ac7355 100644
>>>>>> --- a/Documentation/devicetree/bindings/pci/ti-pci.txt
>>>>>> +++ b/Documentation/devicetree/bindings/pci/ti-pci.txt
>>>>>> @@ -13,6 +13,12 @@ PCIe DesignWare Controller
>>>>>>   - ti,hwmods : Name of the hwmod associated to the pcie, "pcie<X>",
>>>>>>           where <X> is the instance number of the pcie from the HW spec.
>>>>>>   - num-lanes as specified in ../designware-pcie.txt
>>>>>> + - ti,syscon-lane-conf : phandle/offset pair. Phandle to the system control
>>>>>> +                   module and the register offset to specify 1 lane or
>>>>>> +                   2 lane.
>>>>>> + - ti,syscon-lane-sel : phandle/offset pair. Phandle to the system control
>>>>>> +                  module and the register offset to specify lane
>>>>>> +                  selection.
>>>>>
>>>>> Adding a property for every syscon register doesn't really scale and
>>>>> doesn't work if the register layout changes.
>>>>
>>>> The register layout doesn't really change between silicon revisions and for new
>>>> SoCs, the phandle and the register offset for that SoC will have to be
>>>> populated again.
>>
>> And what about SoCs that don't exist yet?
>>
>>>> Having said that, I'm not aware of any other alternative here.
>>
>> What would you do if you had 20 different syscon registers to
>> configure? Add 20 properties? No, you would have per SoC functions in
>> the driver to handle the different cases.
> 
> Ideally these syscon registers would be managed by some Linux
> generic framework such as clock/regulator/mux/phy.
> 
> But yeah, if that does not work, then setting a SoC specific
> configuration function based on the compatible value makes sense
> to me.

All right. I'll add a function for syscon configuration in pci-dra7xx.

Thanks
Kishon
--
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 2/2] pci: dwc: pci-exynos: add the codes to support the exynos5433
From: Jaehoon Chung @ 2017-12-27  5:58 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, krzk-DgEjT+Ai2ygdnm+yROfE0A,
	jingoohan1-Re5JQEeQqe8AvxtiuMwx3w, kgene-DgEjT+Ai2ygdnm+yROfE0A,
	lorenzo.pieralisi-5wv7dgnIgG8
In-Reply-To: <20171226211123.feje5l46y62aft7u@rob-hp-laptop>

On 12/27/2017 06:11 AM, Rob Herring wrote:
> On Thu, Dec 21, 2017 at 09:14:07PM +0900, Jaehoon Chung wrote:
>> Exynos5433 has the PCIe for WiFi.
>> Added the codes relevant to PCIe for supporting the exynos5433.
>> Also changed the binding documentation name to
>> 'samsung,exynos-pcie.txt'.
>> (It's not only exynos5440 anymore.)
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  ...exynos5440-pcie.txt => samsung,exynos-pcie.txt} |   2 +-
>>  drivers/pci/dwc/pci-exynos.c                       | 183 ++++++++++++++++-----
>>  2 files changed, 144 insertions(+), 41 deletions(-)
>>  rename Documentation/devicetree/bindings/pci/{samsung,exynos5440-pcie.txt => samsung,exynos-pcie.txt} (97%)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> similarity index 97%
>> rename from Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
>> rename to Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> index 34a11bfbfb60..958dcc150505 100644
>> --- a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/samsung,exynos-pcie.txt
>> @@ -4,7 +4,7 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
>>  and thus inherits all the common properties defined in designware-pcie.txt.
>>  
>>  Required properties:
>> -- compatible: "samsung,exynos5440-pcie"
>> +- compatible: "samsung,exynos5440-pcie" or "samsung,exynos5433-pcie"
> 
> Quite a lot of driver changes for just a new compatible.

It needs to distinguish between exynos5440 and other exynos variants.
So needs to add the new common compatible likes "samsung,exynos5-pcie" or "samsung,exynos-pcie".

Actually, i hope that exynos5440 can be removed from mainline kernel.

> 
>>  - reg: base addresses and lengths of the PCIe controller,
> 
> For example, you're adding the DBI registers which is not documented 
> here. 
> 
> Perhaps it is time to remove the old phy support before adding a new 
> platform.

Ok, I will send the patches to remove the old phy.

> 
>>  	the PHY controller, additional register for the PHY controller.
>>  	(Registers for the PHY controller are DEPRECATED.
>> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
>> index 5596fdedbb94..8dee2e90347e 100644
>> --- a/drivers/pci/dwc/pci-exynos.c
>> +++ b/drivers/pci/dwc/pci-exynos.c
>> @@ -40,6 +40,8 @@
>>  #define PCIE_IRQ_SPECIAL		0x008
>>  #define PCIE_IRQ_EN_PULSE		0x00c
>>  #define PCIE_IRQ_EN_LEVEL		0x010
>> +#define PCIE_SW_WAKE			0x018
>> +#define PCIE_BUS_EN			BIT(1)
>>  #define IRQ_MSI_ENABLE			BIT(2)
>>  #define PCIE_IRQ_EN_SPECIAL		0x014
>>  #define PCIE_PWR_RESET			0x018
>> @@ -49,7 +51,8 @@
>>  #define PCIE_NONSTICKY_RESET		0x024
>>  #define PCIE_APP_INIT_RESET		0x028
>>  #define PCIE_APP_LTSSM_ENABLE		0x02c
>> -#define PCIE_ELBI_RDLH_LINKUP		0x064
>> +#define PCIE_ELBI_RDLH_LINKUP		0x074
>> +#define PCIE_ELBI_XMLH_LINKUP		BIT(4)
>>  #define PCIE_ELBI_LTSSM_ENABLE		0x1
>>  #define PCIE_ELBI_SLV_AWMISC		0x11c
>>  #define PCIE_ELBI_SLV_ARMISC		0x120
>> @@ -94,6 +97,10 @@
>>  #define PCIE_PHY_TRSV3_PD_TSV		BIT(7)
>>  #define PCIE_PHY_TRSV3_LVCC		0x31c
>>  
>> +/* DBI register */
>> +#define PCIE_MISC_CONTROL_1_OFF		0x8BC
>> +#define DBI_RO_WR_EN			BIT(0)
>> +
>>  struct exynos_pcie_mem_res {
>>  	void __iomem *elbi_base;   /* DT 0th resource: PCIe CTRL */
>>  	void __iomem *phy_base;    /* DT 1st resource: PHY CTRL */
>> @@ -221,6 +228,96 @@ static const struct exynos_pcie_ops exynos5440_pcie_ops = {
>>  	.deinit_clk_resources	= exynos5440_pcie_deinit_clk_resources,
>>  };
>>  
>> +static int exynos5433_pcie_get_mem_resources(struct platform_device *pdev,
>> +					     struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +	struct resource *res;
>> +
>> +	ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL);
>> +	if (!ep->mem_res)
>> +		return -ENOMEM;
>> +
>> +	/* External Local Bus interface(ELBI) Register */
> 
> These are standard DW registers IIRC. So the DW core should handle this.
> 
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "elbi");
>> +	ep->mem_res->elbi_base = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(ep->mem_res->elbi_base))
>> +		return PTR_ERR(ep->mem_res->elbi_base);
>> +
>> +	/* Data Bus Interface(DBI) Register */
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
>> +	pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> 
> This is handled by the DW plat driver. Perhaps the DW core should handle 
> it too. 

Will fix.

> 
> Does the 5440 really not have DBI registers or you just happen to not 
> need to access them?

I didn't have exynso5440 TRM, do you means exynos5433? It doesn't have its own DBI register.

> 
>> +	if (IS_ERR(pci->dbi_base))
>> +		return PTR_ERR(pci->dbi_base);
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos5433_pcie_get_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +
>> +	ep->clk_res = devm_kzalloc(dev, sizeof(*ep->clk_res), GFP_KERNEL);
>> +	if (!ep->clk_res)
>> +		return -ENOMEM;
>> +
>> +	ep->clk_res->clk = devm_clk_get(dev, "pcie");
>> +	if (IS_ERR(ep->clk_res->clk)) {
>> +		dev_err(dev, "Failed to get pcie rc clock\n");
>> +		return PTR_ERR(ep->clk_res->clk);
>> +	}
>> +
>> +	ep->clk_res->bus_clk = devm_clk_get(dev, "pcie_bus");
>> +	if (IS_ERR(ep->clk_res->bus_clk)) {
>> +		dev_err(dev, "Failed to get pcie bus clock\n");
>> +		return PTR_ERR(ep->clk_res->bus_clk);
>> +	}
>> +
>> +	return 0;
>> +}
> 
> Can't you reuse exynos5440_pcie_get_clk_resources? They appear to be the 
> same.

Will reuse.

> 
>> +
>> +static void exynos5433_pcie_deinit_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	clk_disable_unprepare(ep->clk_res->bus_clk);
>> +	clk_disable_unprepare(ep->clk_res->clk);
>> +}
>> +
>> +
>> +static int exynos5433_pcie_init_clk_resources(struct exynos_pcie *ep)
>> +{
>> +	struct dw_pcie *pci = ep->pci;
>> +	struct device *dev = pci->dev;
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(ep->clk_res->clk);
>> +	if (ret) {
>> +		dev_err(dev, "cannot enable pcie rc clock");
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(ep->clk_res->bus_clk);
>> +	if (ret) {
>> +		dev_err(dev, "cannot enable pcie bus clock");
>> +		goto err_bus_clk;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_bus_clk:
>> +	clk_disable_unprepare(ep->clk_res->clk);
>> +
>> +	return ret;
>> +}
> 
> Ditto.

Ok. Will fix.

Best Regards,
Jaehoon Chung

> 
> 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: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values
From: Viresh Kumar @ 2017-12-27  4:45 UTC (permalink / raw)
  To: Rob Herring
  Cc: Stephen Boyd, Ulf Hansson, Kevin Hilman, Viresh Kumar,
	Nishanth Menon, Rafael Wysocki,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Vincent Guittot,
	Rajendra Nayak, Sudeep Holla,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+ntPMiSacpsx7M7VwSXsx+fRbHpX-KFNVpDj+bYdNCiQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 26-12-17, 14:23, Rob Herring wrote:
> >         cpu_opp_table: cpu_opp_table {
> >                 compatible = "operating-points-v2";
> >                 opp-shared;
> >
> >                 opp00 {
> >                         opp-hz = /bits/ 64 <208000000>;
> >                         clock-latency-ns = <500000>;
> >                         power-domain-opp = <&domain_opp_1>;
> 
> What is this? opp00 here is not a device. One OPP should not point to
> another. "power-domain-opp" is only supposed to appear in devices
> alongside power-domains properties.

There are two type of devices:

A.) With fixed performance state requirements and they will have the
new "required-opp" property in the device node itself as you said.

B.) Devices which can do DVFS (CPU, MMC, LCD, etc) and those may need
a different performance state of the domain for their individual OPPs
and so we can't have this property in the device all the time.

Does this make sense ?

-- 
viresh
--
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 00/11] Allwinner H3/H5/A64(DE2) SimpleFB support
From: Chen-Yu Tsai @ 2017-12-27  4:23 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Chen-Yu Tsai, Rob Herring, Maxime Ripard, linux-clk, devicetree,
	linux-arm-kernel, linux-kernel, dri-devel
In-Reply-To: <4610834.pRdGNWpFpX@ice-x220i>

On Wed, Dec 27, 2017 at 12:10 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
> 在 2017年12月27日星期三 CST 下午12:09:41,Chen-Yu Tsai 写道:
>> On Fri, Dec 22, 2017 at 8:22 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
>> > This patchset adds support for the SimpleFB on Allwinner SoCs with
>> > "Display Engine 2.0".
>> >
>> > PATCH 1 to PATCH 3 are DE2 CCU fixes for H3/H5 SoCs.
>> >
>> > PATCH 4 adds the pipeline strings for DE2 SimpleFB.
>> >
>> > PATCH 5 to 7 adds necessary device tree nodes (DE2 CCU and SimpleFB)
>> > for H3/H5 SoCs.
>> >
>> > PATCH 8 to 11 are for Allwinner A64 SoC to enable SimpleFB.
>>
>> Changelog?
>
> They're in seperate patches.

Please add them to all patches in such a case. List "none" if there
were no changes.

ChenYu
--
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] ARM: dts: ls1021a: fix incorrect clock references
From: Shawn Guo @ 2017-12-27  4:14 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jingchang Lu, Rob Herring, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171221214432.3658852-1-arnd-r2nGTMty4D4@public.gmane.org>

On Thu, Dec 21, 2017 at 10:44:20PM +0100, Arnd Bergmann wrote:
> dtc warns about two 'clocks' properties that have an extraneous '1'
> at the end:
> 
> arch/arm/boot/dts/ls1021a-qds.dtb: Warning (clocks_property): arch/arm/boot/dts/ls1021a-twr.dtb: Warning (clocks_property): Property 'clocks', cell 1 is not a phandle reference in /soc/i2c@2180000/mux@77/i2c@4/sgtl5000@2a
> arch/arm/boot/dts/ls1021a-qds.dtb: Warning (clocks_property): Missing property '#clock-cells' in node /soc/interrupt-controller@1400000 or bad phandle (referred from /soc/i2c@2180000/mux@77/i2c@4/sgtl5000@2a:clocks[1])
> Property 'clocks', cell 1 is not a phandle reference in /soc/i2c@2190000/sgtl5000@a
> arch/arm/boot/dts/ls1021a-twr.dtb: Warning (clocks_property): Missing property '#clock-cells' in node /soc/interrupt-controller@1400000 or bad phandle (referred from /soc/i2c@2190000/sgtl5000@a:clocks[1])
> 
> The clocks that get referenced here are fixed-rate, so they do not
> take any argument, and dtc interprets the next cell as a phandle, which
> is invalid.
> 
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

Hi Arnd,

Thanks for the patch.  But I have queued up a couple of patches from
Fabio [1] for that.

Shawn

[1] https://www.spinics.net/lists/arm-kernel/msg624303.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 v3 00/11] Allwinner H3/H5/A64(DE2) SimpleFB support
From: Icenowy Zheng @ 2017-12-27  4:10 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Rob Herring, Maxime Ripard, linux-clk, devicetree,
	linux-arm-kernel, linux-kernel, dri-devel
In-Reply-To: <CAGb2v653n_Q-spOSP9RTXBrL7CtrYiV7dAyRAVYCxg3KpzRvoQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

在 2017年12月27日星期三 CST 下午12:09:41,Chen-Yu Tsai 写道:
> On Fri, Dec 22, 2017 at 8:22 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
> > This patchset adds support for the SimpleFB on Allwinner SoCs with
> > "Display Engine 2.0".
> > 
> > PATCH 1 to PATCH 3 are DE2 CCU fixes for H3/H5 SoCs.
> > 
> > PATCH 4 adds the pipeline strings for DE2 SimpleFB.
> > 
> > PATCH 5 to 7 adds necessary device tree nodes (DE2 CCU and SimpleFB)
> > for H3/H5 SoCs.
> > 
> > PATCH 8 to 11 are for Allwinner A64 SoC to enable SimpleFB.
> 
> Changelog?

They're in seperate patches.

> 
> ChenYu
> 
> > Icenowy Zheng (11):
> >   dt-bindings: fix the binding of Allwinner DE2 CCU of A83T and H3
> >   clk: sunxi-ng: add support for Allwinner H3 DE2 CCU
> >   clk: sunxi-ng: fix the A64/H5 clock description of DE2 CCU
> >   dt-bindings: simplefb-sunxi: add pipelines for DE2
> >   ARM: sun8i: h3/h5: add DE2 CCU device node for H3
> >   arm64: allwinner: h5: add compatible string for DE2 CCU
> >   ARM: sunxi: h3/h5: add simplefb nodes
> >   dt-bindings: add binding for A64 DE2 CCU SRAM
> >   clk: sunxi-ng: add support for Allwinner A64 DE2 CCU
> >   arm64: allwinner: a64: add DE2 CCU for A64 SoC
> >   arm64: allwinner: a64: add simplefb for A64 SoC
> >  
> >  .../devicetree/bindings/clock/sun8i-de2.txt        | 10 ++-
> >  .../bindings/display/simple-framebuffer-sunxi.txt  |  4 +
> >  arch/arm/boot/dts/sun8i-h3.dtsi                    |  4 +
> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi                 | 43 +++++++++++
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      | 65 +++++++++++++++++
> >  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi       |  4 +
> >  drivers/clk/sunxi-ng/ccu-sun8i-de2.c               | 85
> >  +++++++++++++++++++--- 7 files changed, 202 insertions(+), 13
> >  deletions(-)
> > 
> > --
> > 2.14.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: [PATCH v3 00/11] Allwinner H3/H5/A64(DE2) SimpleFB support
From: Chen-Yu Tsai @ 2017-12-27  4:09 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Rob Herring, Maxime Ripard, Chen-Yu Tsai, linux-clk, devicetree,
	linux-arm-kernel, linux-kernel, dri-devel
In-Reply-To: <20171222122243.25735-1-icenowy-h8G6r0blFSE@public.gmane.org>

On Fri, Dec 22, 2017 at 8:22 PM, Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org> wrote:
> This patchset adds support for the SimpleFB on Allwinner SoCs with
> "Display Engine 2.0".
>
> PATCH 1 to PATCH 3 are DE2 CCU fixes for H3/H5 SoCs.
>
> PATCH 4 adds the pipeline strings for DE2 SimpleFB.
>
> PATCH 5 to 7 adds necessary device tree nodes (DE2 CCU and SimpleFB)
> for H3/H5 SoCs.
>
> PATCH 8 to 11 are for Allwinner A64 SoC to enable SimpleFB.

Changelog?

ChenYu

>
> Icenowy Zheng (11):
>   dt-bindings: fix the binding of Allwinner DE2 CCU of A83T and H3
>   clk: sunxi-ng: add support for Allwinner H3 DE2 CCU
>   clk: sunxi-ng: fix the A64/H5 clock description of DE2 CCU
>   dt-bindings: simplefb-sunxi: add pipelines for DE2
>   ARM: sun8i: h3/h5: add DE2 CCU device node for H3
>   arm64: allwinner: h5: add compatible string for DE2 CCU
>   ARM: sunxi: h3/h5: add simplefb nodes
>   dt-bindings: add binding for A64 DE2 CCU SRAM
>   clk: sunxi-ng: add support for Allwinner A64 DE2 CCU
>   arm64: allwinner: a64: add DE2 CCU for A64 SoC
>   arm64: allwinner: a64: add simplefb for A64 SoC
>
>  .../devicetree/bindings/clock/sun8i-de2.txt        | 10 ++-
>  .../bindings/display/simple-framebuffer-sunxi.txt  |  4 +
>  arch/arm/boot/dts/sun8i-h3.dtsi                    |  4 +
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi                 | 43 +++++++++++
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi      | 65 +++++++++++++++++
>  arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi       |  4 +
>  drivers/clk/sunxi-ng/ccu-sun8i-de2.c               | 85 +++++++++++++++++++---
>  7 files changed, 202 insertions(+), 13 deletions(-)
>
> --
> 2.14.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

* [PATCH 2/2] ARM: dts: imx6: RDU2: correct RTC compatible
From: Andrey Smirnov @ 2017-12-27  3:56 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Andrey Smirnov, Sascha Hauer, Fabio Estevam, Rob Herring,
	Mark Rutland, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	cphealy-Re5JQEeQqe8AvxtiuMwx3w, Lucas Stach
In-Reply-To: <20171227035656.4941-1-andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

The RTC is manufactured by Maxim. This is a cosmetic fix, as Linux
doesn't match the vendor string for i2c devices.

Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: cphealy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
index eef737bd52d9..6bef9a98678e 100644
--- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
@@ -581,7 +581,7 @@
 	};
 
 	ds1341: rtc@68 {
-		compatible = "dallas,ds1341";
+		compatible = "maxim,ds1341";
 		reg = <0x68>;
 	};
 };
--
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/2] ARM: dts: imx6: RDU2: disable internal watchdog
From: Andrey Smirnov @ 2017-12-27  3:56 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Andrey Smirnov, Sascha Hauer, Fabio Estevam, Rob Herring,
	Mark Rutland, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	cphealy-Re5JQEeQqe8AvxtiuMwx3w, Lucas Stach

The system has an external watchdog in the environment processor
so the internal watchdog is of no use.

Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: cphealy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
index 6bef9a98678e..818bfc8692a5 100644
--- a/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-zii-rdu2.dtsi
@@ -838,6 +838,10 @@
 	status = "okay";
 };
 
+&wdog1 {
+	status = "disabled";
+};
+
 &audmux {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_audmux>;
--
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 v3 2/6] ARM: dts: imx6: Add initial support for phyBOARD-Mira
From: Shawn Guo @ 2017-12-27  2:29 UTC (permalink / raw)
  To: Stefan Riedmueller
  Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ, fabio.estevam-3arQi8VN3Tc,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Christian Hemp,
	Stefan Christ, Christoph Fritz
In-Reply-To: <1513940353-6145-3-git-send-email-s.riedmueller-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>

On Fri, Dec 22, 2017 at 11:59:09AM +0100, Stefan Riedmueller wrote:
> This patch adds basic support for PHYTEC phyBOARD-Mira as carrier board
> for PHYTEC phyCORE-i.MX 6.
> 
> Signed-off-by: Christian Hemp <c.hemp-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> Signed-off-by: Stefan Christ <s.christ-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Signed-off-by: Stefan Riedmueller <s.riedmueller-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6qdl-phytec-mira.dtsi | 390 +++++++++++++++++++++++++++++
>  1 file changed, 390 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6qdl-phytec-mira.dtsi
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-phytec-mira.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-mira.dtsi
> new file mode 100644
> index 0000000..45d8c0c
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6qdl-phytec-mira.dtsi
> @@ -0,0 +1,390 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2017 PHYTEC Messtechnik GmbH
> + * Author: Christian Hemp <c.hemp-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> + */
> +
> +
> +/ {
> +	aliases {
> +		rtc0 = &i2c_rtc;
> +	};
> +
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		brightness-levels = <0 4 8 16 32 64 128 255>;
> +		default-brightness-level = <7>;
> +		power-supply = <&reg_backlight>;
> +		pwms = <&pwm1 0 5000000>;
> +		status = "okay";
> +	};
> +
> +	gpio_leds: leds {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpioleds>;
> +		status = "disabled";
> +

Generally we do not have newlines in middle of property list.

> +		compatible = "gpio-leds";

Please put 'compatible' at the beginning of property list and always
have 'status' be the end of list.

> +
> +		red {
> +			label = "phyboard-mira:red";
> +			gpios = <&gpio5 22 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		green {
> +			label = "phyboard-mira:green";
> +			gpios = <&gpio5 23 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		blue {
> +			label = "phyboard-mira:blue";
> +			gpios = <&gpio5 24 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "mmc0";
> +		};
> +	};
> +
> +	reg_backlight: regulator-backlight {
> +		compatible = "regulator-fixed";
> +		regulator-name = "backlight_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_en_switch: regulator-en-switch {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_en_switch>;
> +		compatible = "regulator-fixed";

Move the 'compatible' forward.

> +		regulator-name = "Enable Switch";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		enable-active-high;
> +		gpio = <&gpio3 4 GPIO_ACTIVE_HIGH>;
> +		regulator-always-on;
> +	};
> +
> +	reg_flexcan1: regulator-flexcan1 {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_flexcan1_en>;
> +		compatible = "regulator-fixed";
> +		regulator-name = "flexcan1-reg";
> +		regulator-min-microvolt = <1500000>;
> +		regulator-max-microvolt = <1500000>;
> +		gpio = <&gpio2 20 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	reg_panel: regulator-panel {
> +		compatible = "regulator-fixed";
> +		regulator-name = "panel-power-supply";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_pcie: regulator-pcie {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pcie_reg>;
> +		compatible = "regulator-fixed";
> +		regulator-name = "mPCIe_1V5";
> +		regulator-min-microvolt = <1500000>;
> +		regulator-max-microvolt = <1500000>;
> +		gpio = <&gpio3 0 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	reg_usb_h1_vbus: usb-h1-vbus {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_usbh1_vbus>;
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_h1_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpio = <&gpio2 18 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	reg_usbotg_vbus: usbotg-vbus {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_usbotg_vbus>;
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb_otg_vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	panel {
> +		compatible = "auo,g104sn02";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_panel_en>;
> +		power-supply = <&reg_panel>;
> +		enable-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>;
> +

Drop the newline.

Shawn

> +		backlight = <&backlight>;
> +
> +		port {
> +			panel_in: endpoint {
> +				remote-endpoint = <&lvds0_out>;
> +			};
> +		};
> +	};
> +};
> +
> +&can1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_flexcan1>;
> +	xceiver-supply = <&reg_flexcan1>;
> +	status = "disabled";
> +};
> +
> +&hdmi {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_hdmicec>;
> +	ddc-i2c-bus = <&i2c2>;
> +	status = "disabled";
> +};
> +
> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	clock-frequency = <400000>;
> +	status = "disabled";
> +
> +	stmpe: touchctrl@44 {
> +		compatible = "st,stmpe811";
> +		reg = <0x44>;
> +		interrupt-parent = <&gpio7>;
> +		interrupts = <12 IRQ_TYPE_NONE>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_stmpe>;
> +		status = "disabled";
> +
> +		stmpe_touchscreen {
> +			compatible = "st,stmpe-ts";
> +			st,sample-time = <4>;
> +			st,mod-12b = <1>;
> +			st,ref-sel = <0>;
> +			st,adc-freq = <1>;
> +			st,ave-ctrl = <1>;
> +			st,touch-det-delay = <2>;
> +			st,settling = <2>;
> +			st,fraction-z = <7>;
> +			st,i-drive = <1>;
> +		};
> +	};
> +
> +	i2c_rtc: rtc@68 {
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_rtc_int>;
> +		compatible = "microcrystal,rv4162";
> +		reg = <0x68>;
> +		interrupt-parent = <&gpio7>;
> +		interrupts = <8 IRQ_TYPE_LEVEL_HIGH>;
> +		status = "disabled";
> +	};
> +};
> +
> +&i2c2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	clock-frequency = <100000>;
> +	status = "disabled";
> +};
> +
> +&ldb {
> +	status = "okay";
> +
> +	lvds-channel@0 {
> +		fsl,data-mapping = "spwg";
> +		fsl,data-width = <24>;
> +		status = "disabled";
> +
> +		port@4 {
> +			reg = <4>;
> +
> +			lvds0_out: endpoint {
> +				remote-endpoint = <&panel_in>;
> +			};
> +		};
> +	};
> +};
> +
> +&pcie {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pcie>;
> +	reset-gpio = <&gpio2 25 GPIO_ACTIVE_LOW>;
> +	vpcie-supply = <&reg_pcie>;
> +	status = "disabled";
> +};
> +
> +&pwm1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pwm1>;
> +	status = "okay";
> +};
> +
> +&uart2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart2>;
> +	status = "okay";
> +};
> +
> +&uart3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart3>;
> +	uart-has-rtscts;
> +	status = "disabled";
> +};
> +
> +&usbh1 {
> +	vbus-supply = <&reg_usb_h1_vbus>;
> +	disable-over-current;
> +	status = "disabled";
> +};
> +
> +&usbotg {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usbotg>;
> +	vbus-supply = <&reg_usbotg_vbus>;
> +	disable-over-current;
> +	status = "disabled";
> +};
> +
> +&usdhc1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usdhc1>;
> +	cd-gpios = <&gpio6 31 GPIO_ACTIVE_LOW>;
> +	no-1-8-v;
> +	status = "disabled";
> +};
> +
> +&iomuxc {
> +	pinctrl_panel_en: panelen1grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_EB0__GPIO2_IO28		0xb0b1
> +		>;
> +	};
> +
> +	pinctrl_en_switch: enswitchgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_DA4__GPIO3_IO04		0xb0b1
> +		>;
> +	};
> +
> +	pinctrl_flexcan1: flexcan1grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_GPIO_7__FLEXCAN1_TX		0x1b0b0
> +			MX6QDL_PAD_GPIO_8__FLEXCAN1_RX		0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_flexcan1_en: flexcan1engrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_A18__GPIO2_IO20		0xb0b1
> +		>;
> +	};
> +
> +	pinctrl_gpioleds: gpioledsgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_CSI0_DAT4__GPIO5_IO22	0x1b0b0
> +			MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23	0x1b0b0
> +			MX6QDL_PAD_CSI0_DAT6__GPIO5_IO24	0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_hdmicec: hdmicecgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_KEY_ROW2__HDMI_TX_CEC_LINE	0x1f8b0
> +		>;
> +	};
> +
> +	pinctrl_i2c2: i2c2grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_KEY_ROW3__I2C2_SDA		0x4001b8b1
> +			MX6QDL_PAD_KEY_COL3__I2C2_SCL		0x4001b8b1
> +		>;
> +	};
> +
> +	pinctrl_i2c1: i2c1grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_D21__I2C1_SCL		0x4001b8b1
> +			MX6QDL_PAD_EIM_D28__I2C1_SDA		0x4001b8b1
> +		>;
> +	};
> +
> +	pinctrl_pcie: pciegrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_OE__GPIO2_IO25		0xb0b1
> +		>;
> +	};
> +
> +	pinctrl_pcie_reg: pciereggrp {
> +		fsl,pins = <MX6QDL_PAD_EIM_DA0__GPIO3_IO00	0xb0b1>;
> +	};
> +
> +	pinctrl_pwm1: pwm1grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_GPIO_9__PWM1_OUT		0x1b0b1
> +		>;
> +	};
> +
> +	pinctrl_rtc_int: rtcintgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_SD3_RST__GPIO7_IO08		0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_stmpe: stmpegrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_GPIO_17__GPIO7_IO12		0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_uart2: uart2grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_D26__UART2_TX_DATA	0x1b0b1
> +			MX6QDL_PAD_EIM_D27__UART2_RX_DATA	0x1b0b1
> +		>;
> +	};
> +
> +	pinctrl_uart3: uart3grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_EB3__UART3_CTS_B		0x1b0b1
> +			MX6QDL_PAD_EIM_D23__UART3_RTS_B		0x1b0b1
> +			MX6QDL_PAD_EIM_D24__UART3_TX_DATA	0x1b0b1
> +			MX6QDL_PAD_EIM_D25__UART3_RX_DATA	0x1b0b1
> +		>;
> +	};
> +
> +	pinctrl_usbh1_vbus: usbh1vbusgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_A20__GPIO2_IO18		0xb0b1
> +		>;
> +	};
> +
> +	pinctrl_usbotg: usbotggrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_GPIO_1__USB_OTG_ID		0x17059
> +		>;
> +	};
> +
> +	pinctrl_usbotg_vbus: usbotgvbusgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_A19__GPIO2_IO19		0xb0b1
> +		>;
> +	};
> +
> +	pinctrl_usdhc1: usdhc1grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_SD1_CMD__SD1_CMD		0x170f9
> +			MX6QDL_PAD_SD1_CLK__SD1_CLK		0x100f9
> +			MX6QDL_PAD_SD1_DAT0__SD1_DATA0		0x170f9
> +			MX6QDL_PAD_SD1_DAT1__SD1_DATA1		0x170f9
> +			MX6QDL_PAD_SD1_DAT2__SD1_DATA2		0x170f9
> +			MX6QDL_PAD_SD1_DAT3__SD1_DATA3		0x170f9
> +			MX6QDL_PAD_EIM_BCLK__GPIO6_IO31		0xb0b1  /* CD */
> +		>;
> +	};
> +};
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/1] power: reset: remove unused imx-snvs-poweroff driver
From: Fabio Estevam @ 2017-12-27  2:22 UTC (permalink / raw)
  To: Dong Aisheng
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Shawn Guo, Sebastian Reichel, Dong Aisheng, Robin Gong,
	Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <1513938539-20873-1-git-send-email-aisheng.dong-3arQi8VN3Tc@public.gmane.org>

On Fri, Dec 22, 2017 at 8:28 AM, Dong Aisheng <aisheng.dong-3arQi8VN3Tc@public.gmane.org> wrote:
> There's no user of it in kernel now and it basically functions the same
> as the generic syscon-poweroff.c to which we have already switched.
> So let's remove it.
>
> Cc: Robin Gong <yibin.gong-3arQi8VN3Tc@public.gmane.org>
> Cc: Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Dong Aisheng <aisheng.dong-3arQi8VN3Tc@public.gmane.org>

Reviewed-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@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: [PATCH v3 1/6] ARM: dts: imx6: Add initial support for phyCORE-i.MX 6 SOM
From: Shawn Guo @ 2017-12-27  2:18 UTC (permalink / raw)
  To: Stefan Riedmueller
  Cc: kernel-bIcnvbaLZ9MEGnE8C9+IrQ, fabio.estevam-3arQi8VN3Tc,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Christian Hemp,
	Stefan Christ, Christoph Fritz
In-Reply-To: <1513940353-6145-2-git-send-email-s.riedmueller-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>

On Fri, Dec 22, 2017 at 11:59:08AM +0100, Stefan Riedmueller wrote:
> This patch adds basic support for PHYTEC phyCORE-i.MX 6 SOM with i.MX
> 6Quad/Dual or i.MX 6DualLight/Solo.
> 
> Signed-off-by: Christian Hemp <c.hemp-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> Signed-off-by: Stefan Christ <s.christ-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Signed-off-by: Stefan Riedmueller <s.riedmueller-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi | 282 ++++++++++++++++++++++
>  1 file changed, 282 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi
> new file mode 100644
> index 0000000..8501ac6
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx6qdl-phytec-phycore-som.dtsi
> @@ -0,0 +1,282 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2017 PHYTEC Messtechnik GmbH
> + * Author: Christian Hemp <c.hemp-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	model = "PHYTEC phyCORE-i.MX 6";
> +	compatible = "phytec,imx6qdl-pcm058", "fsl,imx6qdl";
> +
> +	aliases {
> +		rtc1 = &da9062_rtc;
> +		rtc2 = &snvs_rtc;
> +	};
> +
> +	/*
> +	 * Set the minimum memory size here and
> +	 * let the bootloader set the real size.
> +	 */
> +	memory@10000000 {
> +		device_type = "memory";
> +		reg = <0x10000000 0x8000000>;
> +	};
> +
> +	gpio_leds_som: somleds {
> +		compatible = "gpio-leds";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gpioleds_som>;
> +
> +		som_green {

We generally use hyphen rather than underscore in node name.  Also I
would suggest to have 'led' in the name to tell what the device is,
maybe 'led-green'?

> +			label = "phycore:green";
> +			gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "heartbeat";
> +		};
> +	};
> +};
> +
> +&ecspi1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_ecspi1>;
> +	cs-gpios = <&gpio3 19 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +
> +	flash: flash@0 {

While it's all good to name the node in a general way, the label can be 
specific, like:

	m25p80: flash@0 {
		...
	}

Even better, if the label is not really needed, just drop it.

> +		compatible = "m25p80";
> +		spi-max-frequency = <20000000>;
> +		reg = <0>;
> +		status = "disabled";
> +	};
> +};
> +
> +&fec {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_enet>;
> +	phy-handle = <&ethphy>;
> +	phy-mode = "rgmii";
> +	phy-supply = <&vdd_eth_io>;
> +	phy-reset-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>;
> +	status = "disabled";
> +
> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy: ethernet-phy@3 {
> +			reg = <3>;
> +			txc-skew-ps = <1680>;
> +			rxc-skew-ps = <1860>;
> +		};
> +	};
> +};
> +
> +&gpmi {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_gpmi_nand>;
> +	nand-on-flash-bbt;
> +	status = "disabled";
> +};
> +
> +&i2c3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c3>;
> +	clock-frequency = <400000>;
> +	status = "okay";
> +
> +	eeprom: eeprom@50 {

Is the label used at all?

> +		compatible = "atmel,24c32";
> +		reg = <0x50>;
> +	};
> +
> +	pmic0: pmic@58 {

Ditto

> +		compatible = "dlg,da9062";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pmic>;
> +		reg = <0x58>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-controller;
> +
> +		da9062_rtc: rtc {
> +			compatible = "dlg,da9062-rtc";
> +		};
> +
> +		da9062_wdt: watchdog {

Ditto

> +			compatible = "dlg,da9062-watchdog";
> +		};
> +
> +		da9062_reg: regulators {

Ditto

Shawn

> +			vdd_arm: buck1 {
> +				regulator-name = "vdd_arm";
> +				regulator-min-microvolt = <730000>;
> +				regulator-max-microvolt = <1380000>;
> +				regulator-always-on;
> +			};
> +
> +			vdd_soc: buck2 {
> +				regulator-name = "vdd_soc";
> +				regulator-min-microvolt = <730000>;
> +				regulator-max-microvolt = <1380000>;
> +				regulator-always-on;
> +			};
> +
> +			vdd_ddr3_1p5: buck3 {
> +				regulator-name = "vdd_ddr3";
> +				regulator-min-microvolt = <1500000>;
> +				regulator-max-microvolt = <1500000>;
> +				regulator-always-on;
> +			};
> +
> +			vdd_eth_1p2: buck4 {
> +				regulator-name = "vdd_eth";
> +				regulator-min-microvolt = <1200000>;
> +				regulator-max-microvolt = <1200000>;
> +				regulator-always-on;
> +			};
> +
> +			vdd_snvs: ldo1 {
> +				regulator-name = "vdd_snvs";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +				regulator-always-on;
> +			};
> +
> +			vdd_high: ldo2 {
> +				regulator-name = "vdd_high";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3000000>;
> +				regulator-always-on;
> +			};
> +
> +			vdd_eth_io: ldo3 {
> +				regulator-name = "vdd_eth_io";
> +				regulator-min-microvolt = <2500000>;
> +				regulator-max-microvolt = <2500000>;
> +			};
> +
> +			vdd_emmc_1p8: ldo4 {
> +				regulator-name = "vdd_emmc";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <1800000>;
> +			};
> +		};
> +	};
> +};
> +
> +&reg_arm {
> +	vin-supply = <&vdd_arm>;
> +};
> +
> +&reg_pu {
> +	vin-supply = <&vdd_soc>;
> +};
> +
> +&reg_soc {
> +	vin-supply = <&vdd_soc>;
> +};
> +
> +&snvs_poweroff {
> +	status = "okay";
> +};
> +
> +&usdhc4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_usdhc4>;
> +	bus-width = <8>;
> +	non-removable;
> +	vmmc-supply = <&vdd_emmc_1p8>;
> +	status = "disabled";
> +};
> +
> +&iomuxc {
> +	pinctrl_enet: enetgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_ENET_MDIO__ENET_MDIO		0x1b0b0
> +			MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
> +			MX6QDL_PAD_RGMII_TXC__RGMII_TXC		0x1b0b0
> +			MX6QDL_PAD_RGMII_TD0__RGMII_TD0		0x1b0b0
> +			MX6QDL_PAD_RGMII_TD1__RGMII_TD1		0x1b0b0
> +			MX6QDL_PAD_RGMII_TD2__RGMII_TD2		0x1b0b0
> +			MX6QDL_PAD_RGMII_TD3__RGMII_TD3		0x1b0b0
> +			MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL	0x1b0b0
> +			MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK	0x1b0b0
> +			MX6QDL_PAD_RGMII_RXC__RGMII_RXC		0x1b0b0
> +			MX6QDL_PAD_RGMII_RD0__RGMII_RD0		0x1b0b0
> +			MX6QDL_PAD_RGMII_RD1__RGMII_RD1		0x1b0b0
> +			MX6QDL_PAD_RGMII_RD2__RGMII_RD2		0x1b0b0
> +			MX6QDL_PAD_RGMII_RD3__RGMII_RD3		0x1b0b0
> +			MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL	0x1b0b0
> +			MX6QDL_PAD_ENET_TX_EN__ENET_TX_EN	0x1b0b0
> +			MX6QDL_PAD_SD2_DAT1__GPIO1_IO14		0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_gpioleds_som: gpioledssomgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_GPIO_4__GPIO1_IO04		0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_gpmi_nand: gpminandgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_NANDF_CLE__NAND_CLE		0xb0b1
> +			MX6QDL_PAD_NANDF_ALE__NAND_ALE		0xb0b1
> +			MX6QDL_PAD_NANDF_WP_B__NAND_WP_B	0xb0b1
> +			MX6QDL_PAD_NANDF_RB0__NAND_READY_B	0xb000
> +			MX6QDL_PAD_NANDF_CS0__NAND_CE0_B	0xb0b1
> +			MX6QDL_PAD_NANDF_CS1__NAND_CE1_B	0xb0b1
> +			MX6QDL_PAD_NANDF_CS2__NAND_CE2_B	0xb0b1
> +			MX6QDL_PAD_NANDF_CS3__NAND_CE3_B	0xb0b1
> +			MX6QDL_PAD_SD4_CMD__NAND_RE_B		0xb0b1
> +			MX6QDL_PAD_SD4_CLK__NAND_WE_B		0xb0b1
> +			MX6QDL_PAD_NANDF_D0__NAND_DATA00	0xb0b1
> +			MX6QDL_PAD_NANDF_D1__NAND_DATA01	0xb0b1
> +			MX6QDL_PAD_NANDF_D2__NAND_DATA02	0xb0b1
> +			MX6QDL_PAD_NANDF_D3__NAND_DATA03	0xb0b1
> +			MX6QDL_PAD_NANDF_D4__NAND_DATA04	0xb0b1
> +			MX6QDL_PAD_NANDF_D5__NAND_DATA05	0xb0b1
> +			MX6QDL_PAD_NANDF_D6__NAND_DATA06	0xb0b1
> +			MX6QDL_PAD_NANDF_D7__NAND_DATA07	0xb0b1
> +			MX6QDL_PAD_SD4_DAT0__NAND_DQS		0x00b1
> +		>;
> +	};
> +
> +	pinctrl_i2c3: i2c3grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_GPIO_6__I2C3_SDA		0x4001b8b1
> +			MX6QDL_PAD_GPIO_5__I2C3_SCL		0x4001b8b1
> +		>;
> +	};
> +
> +	pinctrl_ecspi1: ecspi1grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_EIM_D16__ECSPI1_SCLK		0x100b1
> +			MX6QDL_PAD_EIM_D17__ECSPI1_MISO		0x100b1
> +			MX6QDL_PAD_EIM_D18__ECSPI1_MOSI		0x100b1
> +			MX6QDL_PAD_EIM_D19__GPIO3_IO19		0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_pmic: pmicgrp {
> +		fsl,pins = <
> +			MX6QDL_PAD_GPIO_2__GPIO1_IO02		0x1b0b0
> +		>;
> +	};
> +
> +	pinctrl_usdhc4: usdhc4grp {
> +		fsl,pins = <
> +			MX6QDL_PAD_SD4_CMD__SD4_CMD		0x17059
> +			MX6QDL_PAD_SD4_CLK__SD4_CLK		0x10059
> +			MX6QDL_PAD_SD4_DAT0__SD4_DATA0		0x17059
> +			MX6QDL_PAD_SD4_DAT1__SD4_DATA1		0x17059
> +			MX6QDL_PAD_SD4_DAT2__SD4_DATA2		0x17059
> +			MX6QDL_PAD_SD4_DAT3__SD4_DATA3		0x17059
> +			MX6QDL_PAD_SD4_DAT4__SD4_DATA4		0x17059
> +			MX6QDL_PAD_SD4_DAT5__SD4_DATA5		0x17059
> +			MX6QDL_PAD_SD4_DAT6__SD4_DATA6		0x17059
> +			MX6QDL_PAD_SD4_DAT7__SD4_DATA7		0x17059
> +		>;
> +	};
> +};
> -- 
> 2.7.4
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ARM: make ARCH_S3C24XX select USE_OF and clean-up boot/dts/Makefile
From: Masahiro Yamada @ 2017-12-27  2:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: arm-soc, Olof Johansson, Arnd Bergmann,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	Rob Herring, Mark Rutland, Russell King, linux-arm-kernel
In-Reply-To: <CAJKOXPfd+R0z-RcNLWn=rS6U0uBhDsZS8d-f-b4+jUeQBruc5Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

2017-12-26 2:39 GMT+09:00 Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> On Sun, Dec 24, 2017 at 6:19 PM, Masahiro Yamada
> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
>> 2017-12-22 21:41 GMT+09:00 Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>>> On Mon, Nov 27, 2017 at 3:19 AM, Masahiro Yamada
>>> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
>>>> ARCH_S3C24XX is a very exceptional platform that some DT files in
>>>> arch/arm/boot/dts/, but does not select USE_OF.
>>>
>>> Not entirely. The platform does select USE_OF - when MACH_S3C2416_DT
>>> is chosen. For other boards USE_OF is not necessary because they do
>>> not use DT. Why you need to select it for entire arch?
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>>
>> The reason is simple - to avoid compile error.
>>
>>
>> If I simply drop ifeq ($(CONFIG_OF),y)
>> but do not select ARCH_S3C24XX,
>> s3c2410_defconfig failed to build.
>>
>>
>> $ make -s ARCH=arm mrproper
>> $ make -s ARCH=arm s3c2410_defconfig
>> $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-  dtbs
>>     ...
>>
>>   DTC     arch/arm/boot/dts/s3c2416-smdk2416.dtb
>> /bin/sh: 1: ./scripts/dtc/dtc: not found
>> scripts/Makefile.lib:310: recipe for target
>> 'arch/arm/boot/dts/s3c2416-smdk2416.dtb' failed
>> make[1]: *** [arch/arm/boot/dts/s3c2416-smdk2416.dtb] Error 127
>> arch/arm/Makefile:349: recipe for target 'dtbs' failed
>> make: *** [dtbs] Error 2
>>
>>
>> Another solution would be to enable dtb by CONFIG_MACH_S3C2416_DT.
>>
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index d0381e9..950b5dd 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -800,7 +798,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
>>         rk3288-veyron-pinky.dtb \
>>         rk3288-veyron-speedy.dtb \
>>         rk3288-vyasa.dtb
>> -dtb-$(CONFIG_ARCH_S3C24XX) += \
>> +dtb-$(CONFIG_MACH_S3C2416_DT) += \
>>         s3c2416-smdk2416.dtb
>>  dtb-$(CONFIG_ARCH_S3C64XX) += \
>>         s3c6410-mini6410.dtb \
>>
>>
>>
>> If you migrate S3C24XX platform to DT,
>> per-board CONFIG option will go away.
>>
>> So, I think dtb-$(CONFIG_ARCH_S3C24XX) is OK.
>
> I think this second solution - using CONFIG_MACH_S3C2416_DT - makes
> more sense because:
> 1, S3C24xx will not be converted to DT. This is a legacy platform.
> 2. DT version supports only part of boards and peripherals so most of
> existing platforms will use non-DT boardfiles. Enabling OF on all of
> them is not useful for them.

I do not care this much.
I leave this up to you (and ARM-SOC maintainers).


> 3. The same error and solution probably applies to MACH_S3C64XX.

Maybe no.

Looking at arch/arm/mach-s3c64xx/Kconfig,
ARCH_S3C64XX depends on ARCH_MULTI_V6.

Multiplatform selects USE_OF.




>
> Best regards,
> Krzysztof
> --
> 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



-- 
Best Regards
Masahiro Yamada
--
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 net-next v8 1/2] dt-bindings: net: add DT bindings for Socionext UniPhier AVE
From: Masahiro Yamada @ 2017-12-27  1:49 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: David Miller, netdev, Andrew Lunn, Florian Fainelli, Rob Herring,
	Mark Rutland, linux-arm-kernel, Linux Kernel Mailing List,
	devicetree, Masami Hiramatsu, Jassi Brar
In-Reply-To: <1514164238-28901-2-git-send-email-hayashi.kunihiko@socionext.com>

2017-12-25 10:10 GMT+09:00 Kunihiko Hayashi <hayashi.kunihiko@socionext.com>:
> DT bindings for the AVE ethernet controller found on Socionext's
> UniPhier platforms.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/net/socionext,uniphier-ave4.txt       | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
>
> diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> new file mode 100644
> index 0000000..8b03668
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
> @@ -0,0 +1,47 @@
> +* Socionext AVE ethernet controller
> +
> +This describes the devicetree bindings for AVE ethernet controller
> +implemented on Socionext UniPhier SoCs.
> +
> +Required properties:
> + - compatible: Should be
> +       - "socionext,uniphier-pro4-ave4" : for Pro4 SoC
> +       - "socionext,uniphier-pxs2-ave4" : for PXs2 SoC
> +       - "socionext,uniphier-ld11-ave4" : for LD11 SoC
> +       - "socionext,uniphier-ld20-ave4" : for LD20 SoC
> + - reg: Address where registers are mapped and size of region.
> + - interrupts: Should contain the MAC interrupt.
> + - phy-mode: See ethernet.txt in the same directory. Allow to choose
> +       "rgmii", "rmii", or "mii" according to the PHY.
> + - phy-handle: Should point to the external phy device.
> +       See ethernet.txt file in the same directory.
> + - clocks: A phandle to the clock for the MAC.
> +
> +Optional properties:
> + - resets: A phandle to the reset control for the MAC.
> + - local-mac-address: See ethernet.txt in the same directory.
> +
> +Required subnode:
> + - mdio: A container for child nodes representing phy nodes.
> +         See phy.txt in the same directory.
> +
> +Example:
> +
> +       ether: ethernet@65000000 {
> +               compatible = "socionext,uniphier-ld20-ave4";
> +               reg = <0x65000000 0x8500>;
> +               interrupts = <0 66 4>;
> +               phy-mode = "rgmii";
> +               phy-handle = <&ethphy>;
> +               clocks = <&sys_clk 6>;
> +               resets = <&sys_rst 6>;
> +               local-mac-address = [00 00 00 00 00 00];
> +
> +               mdio {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       ethphy: ethphy@1 {
> +                               reg = <1>;
> +                       };
> +               };

Andrew Lunn suggested to put a blank line before the "mdio" subnode in v7:
https://patchwork.kernel.org/patch/10127461/

Does it apply to the "ethphy" subnode, too?



Looks like you have a chance for v9.  Please consider it.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [RFC PATCH v12 1/5] dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq
From: JeffyChen @ 2017-12-27  1:36 UTC (permalink / raw)
  To: Rafael J. Wysocki, Rob Herring
  Cc: linux-kernel, bhelgaas, linux-pm, tony, shawn.lin, briannorris,
	dianders, devicetree, linux-pci, Mark Rutland
In-Reply-To: <3268448.IvXk7OJtIY@aspire.rjw.lan>

Hi Rafael and Rob,

Thanks for your reply.

On 12/27/2017 08:43 AM, Rafael J. Wysocki wrote:
> On Wednesday, December 27, 2017 12:35:52 AM CET Rob Herring wrote:
>> >On Tue, Dec 26, 2017 at 10:36:42AM +0800, Jeffy Chen wrote:
>>> > >We are going to handle PCIe WAKE# pin for PCI devices in the pci core,
>>> > >so add definitions of the optional PCIe WAKE# pin for PCI devices.
>>> > >
>>> > >Also add an definition of the optional PCI interrupt pin for PCI
>>> > >devices to distinguish it from the PCIe WAKE# pin.
>> >
>> >By v13 you should drop "RFC". RFC implies not ready for merging.
> Which very much still is the case AFAICS.
>
maybe i should split this series, and send dt-binding patch and the pci 
irq parsing patch along without RFC ?

> Thanks,
> Rafael
>
>
>
>

^ permalink raw reply

* Re: [RFC PATCH v12 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF
From: JeffyChen @ 2017-12-27  1:32 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, shawn.lin-TNX95d0MmH7DzftRWevZcw,
	briannorris-F7+t8E8rja9g9hUCZPvPmw, rjw-LthD3rsA81gm4RdzfppkhA,
	dianders-F7+t8E8rja9g9hUCZPvPmw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, Frank Rowand,
	andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA
In-Reply-To: <20171226235607.4rgp4ebrrktt6d56@rob-hp-laptop>

Hi Rob,

Thanks for your reply.

On 12/27/2017 07:56 AM, Rob Herring wrote:
>> >
>> >  drivers/of/of_pci_irq.c  | 49 +++++++++++++++++++++++++++++++
> Please move this to drivers/pci/of.c (or perhaps create pci/of_irq.c).
>
>> >  drivers/pci/Makefile     |  1 +
>> >  drivers/pci/pci-driver.c | 10 +++++++
>> >  drivers/pci/pci-of.c     | 75 ++++++++++++++++++++++++++++++++++++++++++++++++
> We already have drivers/pci/of.c. It's not clear what the difference is
> from the filenames. Either merge with of.c or perhaps of-pm.c.

this new file does something similar to the pci-acpi.c and pci-mid.c..

and i am agree the naming is not clear, maybe we can rename both of 
those files to something like pci-pm-***.c?

Hi Rafael, do you think this would make sense?

>> >  include/linux/of_pci.h   |  9 ++++++
>> >  5 files changed, 144 insertions(+)
>> >  create mode 100644 drivers/pci/pci-of.c
>> >
>> >diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
>> >index d39565d5477b..abec3a44853b 100644
>> >--- a/drivers/of/of_pci_irq.c
>> >+++ b/drivers/of/of_pci_irq.c
>> >@@ -1,8 +1,57 @@
>> >  #include <linux/kernel.h>
>> >  #include <linux/of_pci.h>
>> >  #include <linux/of_irq.h>
>> >+#include <linux/pm_wakeirq.h>
>> >  #include <linux/export.h>
>> >
>> >+int of_pci_setup_wake_irq(struct pci_dev *pdev)
>> >+{
>> >+	struct pci_dev *ppdev;
>> >+	struct device_node *dn;
>> >+	int ret, irq;
>> >+
>> >+	/* Get the pci_dev of our parent. Hopefully it's a port. */
>> >+	ppdev = pdev->bus->self;
>> >+	/* Nope, it's a host bridge. */
>> >+	if (!ppdev)
>> >+		return 0;
>> >+
>> >+	dn = pci_device_to_OF_node(ppdev);
>> >+	if (!dn)
>> >+		return 0;
>> >+
>> >+	irq = of_irq_get_byname(dn, "wakeup");
>> >+	if (irq == -EPROBE_DEFER) {
>> >+		return irq;
>> >+	} else if (irq < 0) {
>> >+		/* Ignore other errors, since a missing wakeup is non-fatal. */
>> >+		dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
>> >+		return 0;
>> >+	}
>> >+
>> >+	device_init_wakeup(&pdev->dev, true);
>> >+
>> >+	ret = dev_pm_set_dedicated_wake_irq(&pdev->dev, irq);
>> >+	if (ret < 0) {
>> >+		dev_err(&pdev->dev, "failed to set wake IRQ: %d\n", ret);
>> >+		device_init_wakeup(&pdev->dev, false);
>> >+		return ret;
>> >+	}
>> >+
>> >+	/* Start out disabled to avoid irq storm */
>> >+	dev_pm_disable_wake_irq(&pdev->dev);
>> >+
>> >+	return 0;
>> >+}
>> >+EXPORT_SYMBOL_GPL(of_pci_setup_wake_irq);
>> >+
>> >+void of_pci_teardown_wake_irq(struct pci_dev *pdev)
>> >+{
>> >+	dev_pm_clear_wake_irq(&pdev->dev);
>> >+	device_init_wakeup(&pdev->dev, false);
>> >+}
>> >+EXPORT_SYMBOL_GPL(of_pci_teardown_wake_irq);
>> >+
>> >  /**
>> >   * of_irq_parse_pci - Resolve the interrupt for a PCI device
>> >   * @pdev:       the device whose interrupt is to be resolved
>> >diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>> >index c7819b973df7..d0182c82162a 100644
>> >--- a/drivers/pci/Makefile
>> >+++ b/drivers/pci/Makefile
>> >@@ -29,6 +29,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>> >  # ACPI _DSM provided firmware instance and string name
>> >  #
>> >  obj-$(CONFIG_ACPI)    += pci-acpi.o
>> >+obj-$(CONFIG_OF)      += pci-of.o
>> >
>> >  # SMBIOS provided firmware instance and labels
>> >  obj-$(CONFIG_PCI_LABEL) += pci-label.o
>> >diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> >index d79dbc377b9c..b4475ff35d97 100644
>> >--- a/drivers/pci/pci-driver.c
>> >+++ b/drivers/pci/pci-driver.c
>> >@@ -17,6 +17,7 @@
>> >  #include <linux/slab.h>
>> >  #include <linux/sched.h>
>> >  #include <linux/cpu.h>
>> >+#include <linux/of_pci.h>
>> >  #include <linux/pm_runtime.h>
>> >  #include <linux/suspend.h>
>> >  #include <linux/kexec.h>
>> >@@ -421,10 +422,17 @@ static int pci_device_probe(struct device *dev)
>> >  	if (error < 0)
>> >  		return error;
>> >
>> >+	error = of_pci_setup_wake_irq(pci_dev);
>> >+	if (error < 0) {
>> >+		pcibios_free_irq(pci_dev);
>> >+		return error;
>> >+	}
>> >+
>> >  	pci_dev_get(pci_dev);
>> >  	if (pci_device_can_probe(pci_dev)) {
>> >  		error = __pci_device_probe(drv, pci_dev);
>> >  		if (error) {
>> >+			of_pci_teardown_wake_irq(pci_dev);
>> >  			pcibios_free_irq(pci_dev);
>> >  			pci_dev_put(pci_dev);
>> >  		}
>> >@@ -438,6 +446,8 @@ static int pci_device_remove(struct device *dev)
>> >  	struct pci_dev *pci_dev = to_pci_dev(dev);
>> >  	struct pci_driver *drv = pci_dev->driver;
>> >
>> >+	of_pci_teardown_wake_irq(pci_dev);
>> >+
>> >  	if (drv) {
>> >  		if (drv->remove) {
>> >  			pm_runtime_get_sync(dev);
>> >diff --git a/drivers/pci/pci-of.c b/drivers/pci/pci-of.c
>> >new file mode 100644
>> >index 000000000000..ad413b2de508
>> >--- /dev/null
>> >+++ b/drivers/pci/pci-of.c
>> >@@ -0,0 +1,75 @@
>> >+/*
>> >+ * File:	pci-of.c
>> >+ * Purpose:	Provide PCI PM/wakeup support in OF systems
>> >+ *
>> >+ * Copyright (C) 2017 Google, Inc.
>> >+ *	Brian Norris<briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> >+ *
>> >+ * Copyright (C) 2017 Rockchip Electronics Co., Ltd.
>> >+ *	Jeffy Chen<jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> >+ *
>> >+ * This program is free software; you can redistribute it and/or modify it
>> >+ * under the terms and conditions of the GNU General Public License,
>> >+ * version 2, as published by the Free Software Foundation.
>> >+ */
>> >+
>> >+#include <linux/acpi.h>
>> >+#include <linux/init.h>
>> >+#include <linux/module.h>
>> >+#include <linux/pci.h>
>> >+#include <linux/pm_wakeirq.h>
>> >+#include "pci.h"
>> >+
>> >+static bool of_pci_power_manageable(struct pci_dev *dev)
>> >+{
>> >+	return false;
>> >+}
>> >+
>> >+static int of_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
>> >+{
>> >+	return -ENOSYS;
>> >+}
>> >+
>> >+static pci_power_t of_pci_get_power_state(struct pci_dev *dev)
>> >+{
>> >+	return PCI_UNKNOWN;
>> >+}
>> >+
>> >+static pci_power_t of_pci_choose_state(struct pci_dev *pdev)
>> >+{
>> >+	return PCI_POWER_ERROR;
>> >+}
>> >+
>> >+static int of_pci_wakeup(struct pci_dev *dev, bool enable)
>> >+{
>> >+	if (enable)
>> >+		dev_pm_enable_wake_irq(&dev->dev);
>> >+	else
>> >+		dev_pm_disable_wake_irq(&dev->dev);
>> >+	return 0;
>> >+}
>> >+
>> >+static bool of_pci_need_resume(struct pci_dev *dev)
>> >+{
>> >+	return false;
>> >+}
>> >+
>> >+static const struct pci_platform_pm_ops of_pci_platform_pm = {
>> >+	.is_manageable = of_pci_power_manageable,
>> >+	.set_state = of_pci_set_power_state,
>> >+	.get_state = of_pci_get_power_state,
>> >+	.choose_state = of_pci_choose_state,
>> >+	.set_wakeup = of_pci_wakeup,
>> >+	.need_resume = of_pci_need_resume,
>> >+};
>> >+
>> >+static int __init of_pci_init(void)
>> >+{
>> >+	if (!acpi_disabled)
>> >+		return 0;
>> >+
>> >+	pci_set_platform_pm(&of_pci_platform_pm);
> I guess no DT based system will override this?

i think the !acpi_disabled means acpi been disabled or CONFIG_ACPI is 
undefined.

and pci-acpi.c would only work when we have CONFIG_ACPI.

but i have no idea about pci-mid.c or would it possible to have more 
platform pm ops in the future...maybe we should add some dependency in 
the Kconfig?

>


--
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 v12 2/5] of/irq: Adjust of_pci_irq parsing for multiple interrupts
From: JeffyChen @ 2017-12-27  1:03 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel@vger.kernel.org, Bjorn Helgaas, linux-pm,
	Tony Lindgren, Shawn Lin, Brian Norris, Rafael J. Wysocki,
	Doug Anderson, Frank Rowand, open, list,
	OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CAL_JsqL8nztgA0RWw5f_H75T4iVEDt23poUXb_RsL+jWmQ-X6w@mail.gmail.com>

Hi Rob,

Thanks for your reply :)

On 12/26/2017 11:48 PM, Rob Herring wrote:
>> >+               /*
>> >+                * Only parse from DT if we have no "interrupt-names",
>> >+                * or if we found an interrupt named "pci".
>> >+                */
>> >+               if (index == 0 || name) {
>> >+                       rc = of_irq_parse_one(dn, index, out_irq);
>> >+                       if (!rc)
>> >+                               return rc;
>> >+               }
> As mentioned before, use of_property_match_string. The following should work:
>
> index = of_property_match_string(dn, "interrupt-names", "pci");
> if (index == -EINVAL)  /* Property doesn't exist */
>    index = 0;
>
> if (index >= 0) {
>    rc = of_irq_parse_one(dn, index, out_irq);
>    if (!rc)
>      return rc;
> }

hmmm, right, that should work. didn't notice of_property_match_string() 
would return different error numbers for string not found and property 
doesn't exist...

will do this in the next version, thanks :)

>
>

^ permalink raw reply

* [PATCH v5 2/2] PCI: mediatek: Set up class type and vendor ID for MT7622
From: honghui.zhang @ 2017-12-27  0:59 UTC (permalink / raw)
  To: bhelgaas, matthias.bgg, linux-arm-kernel, linux-mediatek,
	linux-pci, linux-kernel, devicetree, yingjoe.chen, eddie.huang,
	ryder.lee, lorenzo.pieralisi
  Cc: honghui.zhang, hongkun.cao, youlin.pei, yong.wu, yt.shen,
	sean.wang, xinping.qian
In-Reply-To: <1514336394-17747-1-git-send-email-honghui.zhang@mediatek.com>

From: Honghui Zhang <honghui.zhang@mediatek.com>

The hardware default value of IDs and class type is not correct,
fix that by setup the correct values before start up.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
---
 drivers/pci/host/pcie-mediatek.c | 12 ++++++++++++
 include/linux/pci_ids.h          |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index fc29a9a..62aac0ea 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -74,6 +74,10 @@
 
 /* PCIe V2 per-port registers */
 #define PCIE_MSI_VECTOR		0x0c0
+
+#define PCIE_CONF_ID		0x100
+#define PCIE_CONF_CLASS		0x104
+
 #define PCIE_INT_MASK		0x420
 #define INTX_MASK		GENMASK(19, 16)
 #define INTX_SHIFT		16
@@ -393,6 +397,14 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 		val |= PCIE_CSR_LTSSM_EN(port->slot) |
 		       PCIE_CSR_ASPM_L1_EN(port->slot);
 		writel(val, pcie->base + PCIE_SYS_CFG_V2);
+
+		/* Set up vendor ID and device ID for MT7622*/
+		val = PCI_VENDOR_ID_MEDIATEK;
+		writel(val, port->base + PCIE_CONF_ID);
+
+		/* Set up class code for MT7622 */
+		val = PCI_CLASS_BRIDGE_PCI << 16;
+		writel(val, port->base + PCIE_CONF_CLASS);
 	}
 
 	/* Assert all reset signals */
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index ab20dc5..2480b0e 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2113,6 +2113,8 @@
 
 #define PCI_VENDOR_ID_MYRICOM		0x14c1
 
+#define PCI_VENDOR_ID_MEDIATEK		0x14c3
+
 #define PCI_VENDOR_ID_TITAN		0x14D2
 #define PCI_DEVICE_ID_TITAN_010L	0x8001
 #define PCI_DEVICE_ID_TITAN_100L	0x8010
-- 
2.6.4

^ permalink raw reply related

* [PATCH v5 1/2] PCI: mediatek: Clear IRQ status after IRQ dispatched to avoid reentry
From: honghui.zhang @ 2017-12-27  0:59 UTC (permalink / raw)
  To: bhelgaas, matthias.bgg, linux-arm-kernel, linux-mediatek,
	linux-pci, linux-kernel, devicetree, yingjoe.chen, eddie.huang,
	ryder.lee, lorenzo.pieralisi
  Cc: honghui.zhang, hongkun.cao, youlin.pei, yong.wu, yt.shen,
	sean.wang, xinping.qian
In-Reply-To: <1514336394-17747-1-git-send-email-honghui.zhang@mediatek.com>

From: Honghui Zhang <honghui.zhang@mediatek.com>

There maybe a same IRQ reentry scenario after IRQ received in current
IRQ handle flow:
	EP device		PCIe host driver	EP driver
1. issue an IRQ
			2. received IRQ
			3. clear IRQ status
			4. dispatch IRQ
						5. clear IRQ source
The IRQ status was not successfully cleared at step 2 since the IRQ
source was not cleared yet. So the PCIe host driver may receive the
same IRQ after step 5. Then there's an IRQ reentry occurred.
Even worse, if the reentry IRQ was not an IRQ that EP driver expected,
it may not handle the IRQ. Then we may run into the infinite loop from
step 2 to step 4.
Clear the IRQ status after IRQ have been dispatched to avoid the IRQ
reentry.
This patch also fix another INTx IRQ issue by initialize the iterate
before the loop. If an INTx IRQ re-occurred while we are dispatching
the INTx IRQ, then iterate may start from PCI_NUM_INTX + INTX_SHIFT
instead of INTX_SHIFT for the second time entering the
for_each_set_bit_from() loop.

Signed-off-by: Honghui Zhang <honghui.zhang@mediatek.com>
Acked-by: Ryder Lee <ryder.lee@mediatek.com>
---
 drivers/pci/host/pcie-mediatek.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index db93efd..fc29a9a 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -601,15 +601,16 @@ static irqreturn_t mtk_pcie_intr_handler(int irq, void *data)
 	struct mtk_pcie_port *port = (struct mtk_pcie_port *)data;
 	unsigned long status;
 	u32 virq;
-	u32 bit = INTX_SHIFT;
+	u32 bit;
 
 	while ((status = readl(port->base + PCIE_INT_STATUS)) & INTX_MASK) {
+		bit = INTX_SHIFT;
 		for_each_set_bit_from(bit, &status, PCI_NUM_INTX + INTX_SHIFT) {
-			/* Clear the INTx */
-			writel(1 << bit, port->base + PCIE_INT_STATUS);
 			virq = irq_find_mapping(port->irq_domain,
 						bit - INTX_SHIFT);
 			generic_handle_irq(virq);
+			/* Clear the INTx */
+			writel(1 << bit, port->base + PCIE_INT_STATUS);
 		}
 	}
 
@@ -619,10 +620,10 @@ static irqreturn_t mtk_pcie_intr_handler(int irq, void *data)
 
 			while ((imsi_status = readl(port->base + PCIE_IMSI_STATUS))) {
 				for_each_set_bit(bit, &imsi_status, MTK_MSI_IRQS_NUM) {
-					/* Clear the MSI */
-					writel(1 << bit, port->base + PCIE_IMSI_STATUS);
 					virq = irq_find_mapping(port->msi_domain, bit);
 					generic_handle_irq(virq);
+					/* Clear the MSI */
+					writel(1 << bit, port->base + PCIE_IMSI_STATUS);
 				}
 			}
 			/* Clear MSI interrupt status */
-- 
2.6.4

^ permalink raw reply related

* [PATCH v5 0/2] PCI: mediatek: Fixups for the IRQ handle routine and MT7622's class code
From: honghui.zhang @ 2017-12-27  0:59 UTC (permalink / raw)
  To: bhelgaas, matthias.bgg, linux-arm-kernel, linux-mediatek,
	linux-pci, linux-kernel, devicetree, yingjoe.chen, eddie.huang,
	ryder.lee, lorenzo.pieralisi
  Cc: honghui.zhang, hongkun.cao, youlin.pei, yong.wu, yt.shen,
	sean.wang, xinping.qian

From: Honghui Zhang <honghui.zhang@mediatek.com>

Two fixups for mediatek's host bridge:
The first patch fixup the IRQ handle routine to avoid IRQ reentry which
may exist for both MT2712 and MT7622.
The second patch fixup class type for MT7622.

Change since v4:
 - Only setup vendor ID for MT7622, igorning the device ID since mediatek's
   host bridge driver does not cares about the device ID.

Change since v3:
 - Setup the class type and vendor ID at the beginning of startup instead
   of in a quirk.
 - Add mediatek's vendor ID, it could be found in:
   https://pcisig.com/membership/member-companies?combine=&page=4

Change since v2:
 - Move the initialize of the iterate before the loop to fix an
   INTx IRQ issue in the first patch

Change since v1:
 - Add the second patch.
 - Make the first patch's commit message more standard.

Honghui Zhang (2):
  PCI: mediatek: Clear IRQ status after IRQ dispatched to avoid reentry
  PCI: mediatek: Set up class type and vendor ID for MT7622

 drivers/pci/host/pcie-mediatek.c | 23 ++++++++++++++++++-----
 include/linux/pci_ids.h          |  2 ++
 2 files changed, 20 insertions(+), 5 deletions(-)

-- 
2.6.4

^ permalink raw reply

* Re: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF
From: Rafael J. Wysocki @ 2017-12-27  0:57 UTC (permalink / raw)
  To: JeffyChen, tony
  Cc: linux-kernel, bhelgaas, linux-pm, shawn.lin, briannorris,
	dianders, devicetree, linux-pci, Rob Herring, Frank Rowand
In-Reply-To: <5A41A0A7.5070605@rock-chips.com>

On Tuesday, December 26, 2017 2:06:47 AM CET JeffyChen wrote:
> Hi Rafael,
> 
> Thanks for your reply :)
> 
> On 12/26/2017 08:11 AM, Rafael J. Wysocki wrote:
> >> >+
> >> >+	dn = pci_device_to_OF_node(ppdev);
> >> >+	if (!dn)
> >> >+		return 0;
> >> >+
> >> >+	irq = of_irq_get_byname(dn, "wakeup");
> > Why is this a property of the bridge and not of the device itself?
>
> That is suggested by Brian, because in that way, the wakeup pin would 
> not "tied to what exact device is installed (or no device, if it's a slot)."

But I don't think it works when there are two devices using different WAKE#
interrupt lines under the same bridge.  Or how does it work then?

> >> >+	if (irq == -EPROBE_DEFER)
> > Braces here, please.
> ok, will fix in the next version.
> 
> >
> >> >+		return irq;
> >> >+	/* Ignore other errors, since a missing wakeup is non-fatal. */
> >> >+	else if (irq < 0) {
> >> >+		dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
> >> >+		return 0;
> >> >+	}
> >> >+
> >> >+	device_init_wakeup(&pdev->dev, true);
> > Why do you call this before dev_pm_set_dedicated_wake_irq()?
>
> hmmm, i thought so too, but it turns out the dedicated wake irq 
> framework requires device_init_wakeup(dev, true) before attach the wake irq:
> 
> int device_wakeup_attach_irq(struct device *dev,
>                               struct wake_irq *wakeirq)
> {
>          struct wakeup_source *ws;
> 
>          ws = dev->power.wakeup;
>          if (!ws) {
>                  dev_err(dev, "forgot to call device_init_wakeup?\n");
>                  return -EINVAL;
> 

Well, that's a framework issue, fair enough.

That said, what if user space removes the wakeup source from under you
concurrently via sysfs?  Tony?

Thanks,
Rafael

^ permalink raw reply

* Re: [RFC PATCH v12 0/5] PCI: rockchip: Move PCIe WAKE# handling into pci core
From: Rafael J. Wysocki @ 2017-12-27  0:44 UTC (permalink / raw)
  To: Jeffy Chen
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, shawn.lin-TNX95d0MmH7DzftRWevZcw,
	briannorris-F7+t8E8rja9g9hUCZPvPmw,
	dianders-F7+t8E8rja9g9hUCZPvPmw, Xinming Hu,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Catalin Marinas,
	Kalle Valo, Heiko Stuebner,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Nishant Sarmukadam, Will Deacon, Matthias Kaehlcke,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Ganapathi Bhat, Frank Rowand,
	Amitkumar Karwar,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171226023646.17722-1-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On Tuesday, December 26, 2017 3:36:41 AM CET Jeffy Chen wrote:
> 
> Currently we are handling wake irq in mrvl wifi driver. Move it into
> pci core.
> 
> Tested on my chromebook bob(with cros 4.4 kernel and mrvl wifi).
> 
> 
> Changes in v13:
> Fix compiler error reported by kbuild test robot <fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Changes in v12:
> Only add irq definitions for PCI devices and rewrite the commit message.
> Enable the wake irq in noirq stage to avoid possible irq storm.
> 
> Changes in v11:
> Address Brian's comments.
> Only support 1-per-device PCIe WAKE# pin as suggested.
> Move to pcie port as Brian suggested.
> 
> Changes in v10:
> Use device_set_wakeup_capable() instead of device_set_wakeup_enable(),
> since dedicated wakeirq will be lost in device_set_wakeup_enable(false).
> 
> Changes in v9:
> Add section for PCI devices and rewrite the commit message.
> Fix check error in .cleanup().
> Move dedicated wakeirq setup to setup() callback and use
> device_set_wakeup_enable() to enable/disable.
> Rewrite the commit message.
> 
> Changes in v8:
> Add optional "pci", and rewrite commit message.
> Add pci-of.c and use platform_pm_ops to handle the PCIe WAKE# signal.
> Rewrite the commit message.
> 
> Changes in v7:
> Move PCIE_WAKE handling into pci core.
> 
> Changes in v6:
> Fix device_init_wake error handling, and add some comments.
> 
> Changes in v5:
> Move to pci.txt
> Rebase.
> Use "wakeup" instead of "wake"
> 
> Changes in v3:
> Fix error handling.
> 
> Changes in v2:
> Use dev_pm_set_dedicated_wake_irq.
> 
> Jeffy Chen (5):
>   dt-bindings: PCI: Add definition of PCIe WAKE# irq and PCI irq
>   of/irq: Adjust of_pci_irq parsing for multiple interrupts
>   mwifiex: Disable wakeup irq handling for pcie
>   PCI / PM: Add support for the PCIe WAKE# signal for OF
>   arm64: dts: rockchip: Move PCIe WAKE# irq to pcie port for Gru
> 
>  Documentation/devicetree/bindings/pci/pci.txt | 10 ++++
>  arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 11 ++--
>  drivers/net/wireless/marvell/mwifiex/main.c   |  4 ++
>  drivers/of/of_pci_irq.c                       | 71 +++++++++++++++++++++++--
>  drivers/pci/Makefile                          |  1 +
>  drivers/pci/pci-driver.c                      | 10 ++++
>  drivers/pci/pci-of.c                          | 75 +++++++++++++++++++++++++++
>  include/linux/of_pci.h                        |  9 ++++
>  8 files changed, 183 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/pci/pci-of.c

I'm going to ignore this version till the discussion on the previous one is
over.

Thanks,
Rafael


--
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