Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V5] perf tools: adding support for address filters
From: Masami Hiramatsu @ 2016-09-19 22:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <008718a0-13bf-8245-e2ee-562cffeba3c5@intel.com>

On Mon, 19 Sep 2016 09:15:40 +0300
Adrian Hunter <adrian.hunter@intel.com> wrote:

> On 17/09/16 16:33, Masami Hiramatsu wrote:
> > On Fri, 16 Sep 2016 09:32:43 -0600
> > Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> > 
> >> On 13 September 2016 at 17:25, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >>> On Tue, 13 Sep 2016 08:18:10 -0600
> >>> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> >>>
> >>>> On 13 September 2016 at 04:01, Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>>>> On 12/09/16 20:53, Mathieu Poirier wrote:
> >>>>>> This patch makes it possible to use the current filter
> >>>>>> framework with address filters.  That way address filters for
> >>>>>> HW tracers such as CoreSight and IntelPT can be communicated
> >>>>>> to the kernel drivers.
> >>>>>>
> >>>>>> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> >>>>>>
> >>>>>> ---
> >>>>>> Changes for V5:
> >>>>>>  - Modified perf_evsel__append_filter() to take a string format
> >>>>>>    rather than an operation.
> >>>>>
> >>>>> Hope I'm not being a pain, but aren't there other places calling
> >>>>> perf_evsel__append_filter() that need to be changed.  Might make
> >>>>> sense as a separate patch.
> >>>>
> >>>> No no, you're right - I completely overlooked that.
> >>>>
> >>>> But shouldn't it be in the same patch?  That way a git bisect would
> >>>> stay consistent...
> >>>
> >>> You're right. Caller and callee should be changed in atomic.
> >>>
> >>> BTW, could you add document updates how the perf command line
> >>> will be changed, and also show the result in the patch description?
> >>
> >> This patch does not change anything on the perf command line.  It
> >> simply allows current options to succeed (as they should) rather than
> >> fail.
> > 
> > Yes, and it will make perf acceptable to pass --filter to CoreSight or
> > Intel PT events, or am I misunderstand?
> > If it is correct, could you give us an example how to pass it, since
> > tools/perf/Documentation/perf-record.txt says it is only for tracepoints?
> 
> I am adding support for symbols in the address filter.  I will send the
> patches soon, but the documentation will be:
> 
> --filter=<filter>::
>         Event filter. This option should follow a event selector (-e) which
> 	selects either tracepoint event(s) or a hardware trace PMU
> 	(e.g. Intel PT or CoreSight).
> 
> 	- tracepoint filters
> 
> 	In the case of tracepoints, multiple '--filter' options are combined
> 	using '&&'.
> 
> 	- address filters
> 
> 	A hardware trace PMU advertises its ability to accept a number of
> 	address filters	by specifying a non-zero value in
> 	/sys/bus/event_source/devices/<pmu>/nr_addr_filters.
> 
> 	Address filters have the format:
> 	
> 	filter|start|stop|tracestop <start> [/ <size>] [@<file name>]
> 	
> 	Where:
> 	- 'filter': defines a region that will be traced.
> 	- 'start': defines an address at which tracing will begin.
> 	- 'stop': defines an address at which tracing will stop.
> 	- 'tracestop': defines a region in which tracing will stop.
> 
> 	<file name> is the name of the object file, <start> is the offset to the
> 	code to trace in that file, and <size> is the size of the region to
> 	trace. 'start' and 'stop' filters need not specify a <size>.
> 
> 	If no object file is specified then the kernel is assumed, in which case
> 	the start address must be a current kernel memory address.
> 
> 	<start> can also be specified by providing the name of a symbol. If the
> 	symbol name is not unique, it can be disambiguated by inserting #n where
> 	'n' selects the n'th symbol in address order. Alternately #0, #g or #G
> 	select only a global symbol. <size> can also be specified by providing
> 	the name of a symbol, in which case the size is calculated to the end
> 	of that symbol. For 'filter' and 'tracestop' filters, if <size> is
> 	omitted and <start> is a symbol, then the size is calculated to the end
> 	of that symbol.
> 
> 	If <size> is omitted and <start> is '*', then the start and size will
> 	be calculated from the first and last symbols, i.e. to trace the whole
> 	file.
> 
> 	If symbol names (or "*") are provided, they must be surrounded by white
> 	space.
> 
> 	The filter passed to the kernel is not necessarily the same as entered.
> 	To see the filter that is passed, use the -v option.
> 
> 	The kernel may not be able to configure a trace region if it is not
> 	within a single mapping.  MMAP events (or /proc/<pid>/maps) can be
> 	examined to determine if that is a possibility.
> 
> 	Multiple filters can be separated with space or comma.

Perfect! :)
OK, I'll wait your patch.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply

* [PATCH] ARM: dts: Add power button support for igepv5
From: Tony Lindgren @ 2016-09-19 23:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMVEqEkKrw2eH3L6+te5MZLqxBT2jUiRO-KNuS+9nuNYvihCRA@mail.gmail.com>

* Pau Pajuel <ppajuel@gmail.com> [160919 01:13]:
> Hello Tony,
> 
> 2016-09-14 3:26 GMT+02:00 Javier Martinez Canillas <javier@osg.samsung.com>:
> > Hello Tony,
> >
> > On 09/09/2016 05:07 PM, Tony Lindgren wrote:
> >> Add power button support for igepv5.
> >>
> >> Cc: Agust? Fontquerni i Gorchs <afontquerni@iseebcn.com>
> >> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> >> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> >> Cc: Pau Pajuel <ppajuel@gmail.com>
> >> Signed-off-by: Tony Lindgren <tony@atomide.com>
> >> ---
> >
> > I don't have a schematics for this board, but the patch looks good to me.
> Patch has an errata. Power button is GPIO_ACTIVE_LOW instead GPIO_ACTIVE_HIGH.

Oops sorry about that, here's an incremental fix.

Regards,

Tony

8< -----------------------
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Mon, 19 Sep 2016 16:02:12 -0700
Subject: [PATCH] ARM: dts: Fix igepv5 power button GPIO direction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It should be GPIO_ACTIVE_LOW instead of GPIO_ACTIVE_HIGH as
pointed out by Pau Pajuel <ppajuel@gmail.com>.

Cc: Agust? Fontquerni i Gorchs <afontquerni@iseebcn.com>
Cc: Enric Balletbo Serra <eballetbo@gmail.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Pau Pajuel <ppajuel@gmail.com>
Fixes: b118c6a6ffa1 ("ARM: dts: Add power button support for igepv5")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/boot/dts/omap5-igep0050.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/omap5-igep0050.dts b/arch/arm/boot/dts/omap5-igep0050.dts
--- a/arch/arm/boot/dts/omap5-igep0050.dts
+++ b/arch/arm/boot/dts/omap5-igep0050.dts
@@ -27,7 +27,7 @@
 		power-button {
 			label = "Power Button";
 			linux,code = <KEY_POWER>;
-			gpios = <&gpio4 22 GPIO_ACTIVE_HIGH>;
+			gpios = <&gpio4 22 GPIO_ACTIVE_LOW>;
 		};
 	};
 
-- 
2.9.3

^ permalink raw reply

* [PATCH] clocksource: arm_arch_timer: Don't assume clock runs in suspend
From: Brian Norris @ 2016-09-19 23:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57DBA81F.2060404@arm.com>

On Fri, Sep 16, 2016 at 09:06:55AM +0100, Marc Zyngier wrote:
> Hi Brian,

Hi Marc,

Thanks for the quick response.

> On 16/09/16 06:49, Brian Norris wrote:
> > Since commit 4fbcdc813fb9 ("clocksource: arm_arch_timer: Use clocksource
> > for suspend timekeeping"), this driver assumes that the ARM architected
> > timer keeps running in suspend. This is not the case for some ARM SoCs,
> > depending on the HW state used for system suspend. Let's not assume that
> > all SoCs support this, and instead only support this if the device tree
> > explicitly tells us it's "always on". In all other cases, just fall back
> > to the RTC. This should be relatively harmless.
> 
> I'm afraid you're confusing two things:
> - the counter, which *must* carry on counting no matter what, as
> (quoting the ARM ARM) "The system counter must be implemented in an
> always-on power domain"
> - the timer, which is allowed to be powered off, and can be tagged with
> the "always-on" property to indicate that it is guaranteed to stay up
> (which in practice only exists in virtual machines and never on real HW).

Indeed, sorry for that confusion, and thanks for the explanations.

> If your counter does stop counting when suspended, then this is starting
> to either feel like a HW bug, or someone is killing the clock that feeds
> this counter when entering suspend.
> 
> If this is the former, then we need a separate quirk to indicate the
> non-standard behaviour. If it is the latter, don't do it! ;-)

It's beginning to seem more like a HW quirk which yields nonstandard
behavior. AIUI, this SoC normally runs the counter off its 24 MHz clock,
but for low power modes, this "always-on" domain switches over to a 32
KHz alternative clock. Unfortunately, the counter doesn't actually tick
when run this way. I'm trying to confirm with the chip designers
(Rockchip, RK3399) about the nature of the quirk, but I think we'll need
a separate DT flag for this behavior.

Brian

^ permalink raw reply

* [PATCH v9 17/19] drm/virtio: kconfig: Fix recursive dependency issue.
From: Emil Velikov @ 2016-09-19 23:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1473081421-16555-18-git-send-email-peter.griffin@linaro.org>

On 5 September 2016 at 14:16, Peter Griffin <peter.griffin@linaro.org> wrote:
> ST_SLIM_REMOTEPROC must select REMOTEPROC, which exposes the following
> recursive dependency.
>
>From a humble skim through remoteproc, drm and a few other subsystems
I think the above is wrong. All the drivers (outside of remoteproc),
that I've seen, depend on the core component, they don't select it.

Furthermore most places explicitly hide the drivers from the menu if
the core component isn't enabled.

Is there something that requires such a different/unusual behaviour in
remoteproc ?

Regards,
Emil

^ permalink raw reply

* linux-next: manual merge of the amlogic tree with the arm-soc tree
From: Stephen Rothwell @ 2016-09-20  0:19 UTC (permalink / raw)
  To: linux-arm-kernel

Hi all,

Today's linux-next merge of the amlogic tree got a conflict in:

  arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi

between commit:

  8735053d7968 ("ARM64: dts: meson-gxbb-p20x: Enable USB Nodes")

from the arm-soc tree and commit:

  d82801a385a1 ("ARM64: dts: meson-gxbb-p20x: Enable USB Nodes")

from the amlogic tree.

This patch was modified before being merged into the arm-soc tree.
Please clean up the amlogic tree.

I fixed it up (I used the arm-soc tree version) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply

* [PATCH v2 3/7] pinctrl: samsung: Add the support the multiple IORESOURCE_MEM for one pin-bank
From: Tomasz Figa @ 2016-09-20  1:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57CD27FC.2020201@samsung.com>

Hi Chanwoo,

Sorry for late reply. Yours was quick compared to mine. ;(

2016-09-05 17:08 GMT+09:00 Chanwoo Choi <cw00.choi@samsung.com>:
> Hi Tomasz,
>
> I'm sorry for late reply.
>
> On 2016? 08? 25? 23:41, Tomasz Figa wrote:
>> 2016-08-25 23:30 GMT+09:00 Tomasz Figa <tomasz.figa@gmail.com>:
>>>> +       }
>>>> +
>>>> +#define EXYNOS_PIN_BANK_EINTN_EXT(pins, reg, id, pctl_idx, eint_idx)   \
>>>> +       {                                               \
>>>> +               .type           = &bank_type_off,       \
>>>> +               .pctl_offset    = reg,                  \
>>>> +               .nr_pins        = pins,                 \
>>>> +               .eint_type      = EINT_TYPE_NONE,       \
>>>> +               .name           = id,                   \
>>>> +               .pctl_res_idx   = pctl_idx,             \
>>>> +               .eint_res_idx   = eint_dix              \
>>>> +       }
>>>
>>> Your patch 4/7 doesn't seem to use this one, so this is dead code for
>>> the time being. Please add when there is real need for it.
>>>
>>> Also it doesn't really make much sense to have index for both pctl and
>>> eint. Please define first entry of regs property as always pointing to
>>> pctl registers and by also eint registers for usual controllers. Then
>>> second regs entry would be eint registers for controllers with
>>> separate register blocks. Then there is only a need to have
>>> eint_res_idx in the driver and no need for pctl_res_idx, because it
>>> would be always 0.
>>
>> Ah, sorry, I got confused again by which registers are where in these
>> GPF banks. Let's make it the other way around and make DT contain eint
>> registers in first regs entry and hardcode eint_res_idx to 0 for the
>> time being.
>
> I got with slight confusion.
> Do you mean that you want to remove the 'eint_res_idx' because
> it is always zero(0) as your comment. And do you agree to add 'pctl_res_idx'?
>
> Also, as you commented, the eint_res_idx for both GPA and GPFx is zero(0).
>
> Example:
>         pinctrl_alive: pinctrl at 10580000 {
>                 compatible = "samsung,exynos5433-pinctrl";
>                       /* ALIVE domain    ,  IMEM domain  */
>                 reg = <0x10580000 0x1a20>, <0x11090000 0x100>;
>
>                 wakeup-interrupt-controller {
>                         compatible = "samsung,exynos7-wakeup-eint";
>                         interrupts = <GIC_SPI 16 0>;
>                 };
>         };
>
>         GPA's eint_res_idx is 0
>         GPA's pctl_res_idx is 0
>
>         GPFx's eint_res_idx is 0
>         GPFx's pctl_res_idx is 1

Yeah, that's right. I just want to avoid adding eint_res_idx until it
is found to be really needed.

>
>
>  However it should be still beneficial to refactor the code
>> and calculate per-bank eint_base to avoid adding the offset every
>> time, similarly to pctl_base/offset, from my suggestion below.
>
> I agree. I'll modify it according to your comment.

Thanks. Looking forward to the patch.

Best regards,
Tomasz

^ permalink raw reply

* [RFC PATCH] PCI/ACPI: xgene: Add ECAM quirk for X-Gene PCIe controller
From: Duc Dang @ 2016-09-20  1:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160919200614.GB21763@localhost>

Hi Bjorn,

Thanks for reviewing my RFC patch.

On Mon, Sep 19, 2016 at 1:06 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> Hi Duc,
>
> On Sat, Sep 17, 2016 at 07:24:38AM -0700, Duc Dang wrote:
>> PCIe controller in X-Gene SoCs is not ECAM compliant: software
>> needs to configure additional concontroller register to address
>> device at bus:dev:function.
>>
>> This patch depends on "ECAM quirks handling for ARM64 platforms"
>> series (http://www.spinics.net/lists/arm-kernel/msg530692.html)
>> to address the limitation above for X-Gene PCIe controller.
>>
>> The quirk will only be applied for X-Gene PCIe MCFG table with
>> OEM revison 1, 2, 3 or 4 (PCIe controller v1 and v2 on X-Gene SoCs).
>>
>> Signed-off-by: Duc Dang <dhdang@apm.com>
>> ---
>>  drivers/acpi/pci_mcfg.c           |  32 +++++
>>  drivers/pci/host/Makefile         |   2 +-
>>  drivers/pci/host/pci-xgene-ecam.c | 280 ++++++++++++++++++++++++++++++++++++++
>>  include/linux/pci-ecam.h          |   5 +
>>  4 files changed, 318 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/pci/host/pci-xgene-ecam.c
>
> This adds a bunch of stuff, but doesn't remove anything.  So I assume
> it's adding new functionality that didn't exist before.  What is it?

This patch only adds the ability for X-Gene PCIe controller to be
probable in ACPI boot mode. All the 'quirk' code added is for ECAM to
work with X-Gene.

>
> I sort of expected this to also remove, for example, the seemingly
> identical xgene_pcie_config_read32() in drivers/pci/host/pci-xgene.c.
> Actually, a bunch of this code seems to be duplicated from there.  It
> doesn't seem like we should end up with all this duplicated code.
>
> I'd really like it better if all this could get folded into
> pci-xgene.c so we don't end up with more files.

I am still debating whether to put this X-Gene ECAM quirk code into
drivers/acpi or keep it here in drivers/pci/host. But given the
direction of other quirk patches for ThunderX and HiSi, seem like the
quirk will stay in drivers/pci/host. I can definitely fold the new
quirk code into pci-xgene.c as you suggested and eliminate the
identical one.

>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index ddf338b..adce35f 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -123,6 +123,38 @@ static struct mcfg_fixup mcfg_quirks[] = {
>>       { "CAVIUM", "THUNDERX", 2, 13, MCFG_BUS_ANY, &pci_thunder_ecam_ops,
>>         MCFG_RES_EMPTY},
>>  #endif
>> +#ifdef CONFIG_PCI_XGENE
>> +     {"APM   ", "XGENE   ", 1, 0, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 1, 1, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 1, 2, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 1, 3, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 1, 4, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 2, 0, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 2, 1, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 2, 2, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 2, 3, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 2, 4, MCFG_BUS_ANY,
>> +             &xgene_v1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 3, 0, MCFG_BUS_ANY,
>> +             &xgene_v2_1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 3, 1, MCFG_BUS_ANY,
>> +             &xgene_v2_1_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 4, 0, MCFG_BUS_ANY,
>> +             &xgene_v2_2_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 4, 1, MCFG_BUS_ANY,
>> +             &xgene_v2_2_pcie_ecam_ops, MCFG_RES_EMPTY},
>> +     {"APM   ", "XGENE   ", 4, 2, MCFG_BUS_ANY,
>> +             &xgene_v2_2_pcie_ecam_ops, MCFG_RES_EMPTY},
>
> Most of these are the same.  Let's add a macro that fills in the
> boilerplate so each entry only contains the variable parts.  I'm going
> to propose the same for the ThunderX quirks.

I will follow up on this.

>
>> +#endif
>>  };
>>
>>  static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 8843410..af4f505 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -15,7 +15,7 @@ obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>>  obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>>  obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
>>  obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
>> -obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
>> +obj-$(CONFIG_PCI_XGENE) += pci-xgene.o pci-xgene-ecam.o
>>  obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
>>  obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>  obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
>> diff --git a/drivers/pci/host/pci-xgene-ecam.c b/drivers/pci/host/pci-xgene-ecam.c
>> new file mode 100644
>> index 0000000..b66a04f
>> --- /dev/null
>> +++ b/drivers/pci/host/pci-xgene-ecam.c
>> @@ -0,0 +1,280 @@
>> +/*
>> + * APM X-Gene PCIe ECAM fixup driver
>> + *
>> + * Copyright (c) 2016, Applied Micro Circuits Corporation
>> + * Author:
>> + *   Duc Dang <dhdang@apm.com>
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/pci-acpi.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pci-ecam.h>
>> +
>> +#ifdef CONFIG_ACPI
>> +#define RTDID                        0x160
>> +#define ROOT_CAP_AND_CTRL    0x5C
>> +
>> +/* PCIe IP version */
>> +#define XGENE_PCIE_IP_VER_UNKN       0
>> +#define XGENE_PCIE_IP_VER_1  1
>> +#define XGENE_PCIE_IP_VER_2  2
>
> We only use XGENE_PCIE_IP_VER_1, so I think the others should be
> removed.  I think it would be nicer to have a "crs_broken:1" bit set
> by the probe path, and get rid of the version field altogether.

We do use XGENE_PCIE_IP_VER_2 for X-Gene v2 PCIe controller, but your
idea about using crs_broken is much better. I will make change
following that. This will affect DT booting too.

>
>> +#define XGENE_CSR_LENGTH     0x10000
>> +
>> +struct xgene_pcie_acpi_root {
>> +     void __iomem *csr_base;
>> +     u32 version;
>> +};
>
> I think this should be folded into struct xgene_pcie_port so we don't
> have to allocate and manage it separately.

I will need to look into this more. When booting with ACPI mode, the
code in pci-xgene.c is not used (except the cfg read/write functions
that are shared with ECAM quirk code), so puting these into
xgene_pcie_port will force ECAM quirk code to allocate this structure
as well.

>
>> +static int xgene_v1_pcie_ecam_init(struct pci_config_window *cfg)
>> +{
>> +     struct xgene_pcie_acpi_root *xgene_root;
>> +     struct device *dev = cfg->parent;
>> +     u32 csr_base;
>> +
>> +     xgene_root = devm_kzalloc(dev, sizeof(*xgene_root), GFP_KERNEL);
>> +     if (!xgene_root)
>> +             return -ENOMEM;
>> +
>> +     switch (cfg->res.start) {
>> +     case 0xE0D0000000ULL:
>> +             csr_base = 0x1F2B0000;
>> +             break;
>> +     case 0xD0D0000000ULL:
>> +             csr_base = 0x1F2C0000;
>> +             break;
>> +     case 0x90D0000000ULL:
>> +             csr_base = 0x1F2D0000;
>> +             break;
>> +     case 0xA0D0000000ULL:
>> +             csr_base = 0x1F500000;
>> +             break;
>> +     case 0xC0D0000000ULL:
>> +             csr_base = 0x1F510000;
>> +             break;
>
> Ugh.  What in the world is going on here?  Apparently we're testing a
> host bridge resource against this hard-coded list of random values,
> and based on that, we know about this *other* list of hard-coded CSR
> ranges?  This is not the way resource discovery normally works ;)

Yes, this is very ugly :(

But discovering CSR regions by using acpi_walk_resource for each root
port is not a preferred solution because of the concern that it may
open a backdoor for continuously abusive usage of the quirk for new
version of SoCs (https://patchwork.kernel.org/patch/9178791/, last 2
comments from Lorenzo and Ard)

>
>> +     default:
>> +             return -ENODEV;
>> +     }
>> +
>> +     xgene_root->csr_base = ioremap(csr_base, XGENE_CSR_LENGTH);
>
> There should be a request_region() somewhere, too.  Ideal would be to
> use devm_ioremap_resource(), but I don't know where this apparent
> resource is coming from.

Yes, I will use request_region/devm_ioremap_resource here.

>
>> +     if (!xgene_root->csr_base) {
>> +             kfree(xgene_root);
>> +             return -ENODEV;
>> +     }
>> +
>> +     xgene_root->version = XGENE_PCIE_IP_VER_1;
>> +
>> +     cfg->priv = xgene_root;
>> +
>> +     return 0;
>> +}
>> +
>> +static int xgene_v2_1_pcie_ecam_init(struct pci_config_window *cfg)
>> +{
>> +     struct xgene_pcie_acpi_root *xgene_root;
>> +     struct device *dev = cfg->parent;
>> +     resource_size_t csr_base;
>> +
>> +     xgene_root = devm_kzalloc(dev, sizeof(*xgene_root), GFP_KERNEL);
>> +     if (!xgene_root)
>> +             return -ENOMEM;
>> +
>> +     switch (cfg->res.start) {
>> +     case 0xC0D0000000ULL:
>> +             csr_base = 0x1F2B0000;
>> +             break;
>> +     case 0xA0D0000000ULL:
>> +             csr_base = 0x1F2C0000;
>> +             break;
>> +     default:
>> +             return -ENODEV;
>> +     }
>> +
>> +     xgene_root->csr_base = ioremap(csr_base, XGENE_CSR_LENGTH);
>> +     if (!xgene_root->csr_base) {
>> +             kfree(xgene_root);
>> +             return -ENODEV;
>> +     }
>> +
>> +     xgene_root->version = XGENE_PCIE_IP_VER_2;
>> +
>> +     cfg->priv = xgene_root;
>> +
>> +     return 0;
>> +}
>> +
>> +static int xgene_v2_2_pcie_ecam_init(struct pci_config_window *cfg)
>> +{
>> +     struct xgene_pcie_acpi_root *xgene_root;
>> +     struct device *dev = cfg->parent;
>> +     resource_size_t csr_base;
>> +
>> +     xgene_root = devm_kzalloc(dev, sizeof(*xgene_root), GFP_KERNEL);
>> +     if (!xgene_root)
>> +             return -ENOMEM;
>> +
>> +     switch (cfg->res.start) {
>> +     case 0xE0D0000000ULL:
>> +             csr_base = 0x1F2B0000;
>> +             break;
>> +     case 0xA0D0000000ULL:
>> +             csr_base = 0x1F500000;
>> +             break;
>> +     case 0x90D0000000ULL:
>> +             csr_base = 0x1F2D0000;
>> +             break;
>> +     default:
>> +             return -ENODEV;
>> +     }
>> +
>> +     xgene_root->csr_base = ioremap(csr_base, XGENE_CSR_LENGTH);
>> +     if (!xgene_root->csr_base) {
>> +             kfree(xgene_root);
>> +             return -ENODEV;
>> +     }
>> +
>> +     xgene_root->version = XGENE_PCIE_IP_VER_2;
>> +
>> +     cfg->priv = xgene_root;
>> +
>> +     return 0;
>> +}
>> +/*
>> + * For Configuration request, RTDID register is used as Bus Number,
>> + * Device Number and Function number of the header fields.
>> + */
>> +static void xgene_pcie_set_rtdid_reg(struct pci_bus *bus, uint devfn)
>> +{
>> +     struct pci_config_window *cfg = bus->sysdata;
>> +     struct xgene_pcie_acpi_root *port = cfg->priv;
>> +     unsigned int b, d, f;
>> +     u32 rtdid_val = 0;
>> +
>> +     b = bus->number;
>> +     d = PCI_SLOT(devfn);
>> +     f = PCI_FUNC(devfn);
>> +
>> +     if (!pci_is_root_bus(bus))
>> +             rtdid_val = (b << 8) | (d << 3) | f;
>> +
>> +     writel(rtdid_val, port->csr_base + RTDID);
>> +     /* read the register back to ensure flush */
>> +     readl(port->csr_base + RTDID);
>> +}
>> +
>> +/*
>> + * X-Gene PCIe port uses BAR0-BAR1 of RC's configuration space as
>> + * the translation from PCI bus to native BUS.  Entire DDR region
>> + * is mapped into PCIe space using these registers, so it can be
>> + * reached by DMA from EP devices.  The BAR0/1 of bridge should be
>> + * hidden during enumeration to avoid the sizing and resource allocation
>> + * by PCIe core.
>> + */
>> +static bool xgene_pcie_hide_rc_bars(struct pci_bus *bus, int offset)
>> +{
>> +     if (pci_is_root_bus(bus) && ((offset == PCI_BASE_ADDRESS_0) ||
>> +                                  (offset == PCI_BASE_ADDRESS_1)))
>> +             return true;
>> +
>> +     return false;
>> +}
>> +
>> +void __iomem *xgene_pcie_ecam_map_bus(struct pci_bus *bus,
>> +                                   unsigned int devfn, int where)
>> +{
>> +     struct pci_config_window *cfg = bus->sysdata;
>> +     unsigned int busn = bus->number;
>> +     void __iomem *base;
>> +
>> +     if (busn < cfg->busr.start || busn > cfg->busr.end)
>> +             return NULL;
>> +
>> +     if ((pci_is_root_bus(bus) && devfn != 0) ||
>> +         xgene_pcie_hide_rc_bars(bus, where))
>> +             return NULL;
>> +
>> +     xgene_pcie_set_rtdid_reg(bus, devfn);
>> +
>> +     if (busn > cfg->busr.start)
>> +             base = cfg->win + (1 << cfg->ops->bus_shift);
>> +     else
>> +             base = cfg->win;
>> +
>> +     return base + where;
>> +}
>> +
>> +static int xgene_pcie_config_read32(struct pci_bus *bus, unsigned int devfn,
>> +                                 int where, int size, u32 *val)
>> +{
>> +     struct pci_config_window *cfg = bus->sysdata;
>> +     struct xgene_pcie_acpi_root *port = cfg->priv;
>> +
>> +     if (pci_generic_config_read32(bus, devfn, where & ~0x3, 4, val) !=
>> +         PCIBIOS_SUCCESSFUL)
>> +             return PCIBIOS_DEVICE_NOT_FOUND;
>> +
>> +     /*
>> +     * The v1 controller has a bug in its Configuration Request
>> +     * Retry Status (CRS) logic: when CRS is enabled and we read the
>> +     * Vendor and Device ID of a non-existent device, the controller
>> +     * fabricates return data of 0xFFFF0001 ("device exists but is not
>> +     * ready") instead of 0xFFFFFFFF ("device does not exist").  This
>> +     * causes the PCI core to retry the read until it times out.
>> +     * Avoid this by not claiming to support CRS.
>> +     */
>> +     if (pci_is_root_bus(bus) && (port->version == XGENE_PCIE_IP_VER_1) &&
>> +         ((where & ~0x3) == ROOT_CAP_AND_CTRL))
>> +             *val &= ~(PCI_EXP_RTCAP_CRSVIS << 16);
>> +
>> +     if (size <= 2)
>> +             *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
>> +
>> +     return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +struct pci_ecam_ops xgene_v1_pcie_ecam_ops = {
>> +     .bus_shift      = 16,
>> +     .init           = xgene_v1_pcie_ecam_init,
>> +     .pci_ops        = {
>> +             .map_bus        = xgene_pcie_ecam_map_bus,
>> +             .read           = xgene_pcie_config_read32,
>> +             .write          = pci_generic_config_write,
>> +     }
>> +};
>> +
>> +struct pci_ecam_ops xgene_v2_1_pcie_ecam_ops = {
>> +     .bus_shift      = 16,
>> +     .init           = xgene_v2_1_pcie_ecam_init,
>> +     .pci_ops        = {
>> +             .map_bus        = xgene_pcie_ecam_map_bus,
>> +             .read           = xgene_pcie_config_read32,
>> +             .write          = pci_generic_config_write,
>> +     }
>> +};
>> +
>> +struct pci_ecam_ops xgene_v2_2_pcie_ecam_ops = {
>> +     .bus_shift      = 16,
>> +     .init           = xgene_v2_2_pcie_ecam_init,
>> +     .pci_ops        = {
>> +             .map_bus        = xgene_pcie_ecam_map_bus,
>> +             .read           = xgene_pcie_config_read32,
>> +             .write          = pci_generic_config_write,
>> +     }
>> +};
>> +#endif
>> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
>> index 35f0e81..40da3e7 100644
>> --- a/include/linux/pci-ecam.h
>> +++ b/include/linux/pci-ecam.h
>> @@ -65,6 +65,11 @@ extern struct pci_ecam_ops pci_thunder_pem_ops;
>>  #ifdef CONFIG_PCI_HOST_THUNDER_ECAM
>>  extern struct pci_ecam_ops pci_thunder_ecam_ops;
>>  #endif
>> +#ifdef CONFIG_PCI_XGENE
>> +extern struct pci_ecam_ops xgene_v1_pcie_ecam_ops;
>> +extern struct pci_ecam_ops xgene_v2_1_pcie_ecam_ops;
>> +extern struct pci_ecam_ops xgene_v2_2_pcie_ecam_ops;
>> +#endif
>>
>>  #ifdef CONFIG_PCI_HOST_GENERIC
>>  /* for DT-based PCI controllers that support ECAM */
>> --
>> 1.9.1
>>
Regards,
Duc Dang.

^ permalink raw reply

* [RFC] Arm64 boot fail with numa enable in BIOS
From: Leizhen (ThunderTown) @ 2016-09-20  1:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160919144542.GK9005@arm.com>



On 2016/9/19 22:45, Will Deacon wrote:
> On Mon, Sep 19, 2016 at 03:07:19PM +0100, Mark Rutland wrote:
>> [adding LAKML, arm64 maintainers]
> 
> I've also looped in Euler ThunderTown, since (a) he's at Huawei and is
> assumedly testing this stuff and (b) he has a fairly big NUMA patch
> series doing the rounds (some of which I've queued).
In my patch series, only one is used to resolve crashed problem, but it's related to device-tree.

> 
>> On Mon, Sep 19, 2016 at 09:05:26PM +0800, Yisheng Xie wrote:
>> In future, please make sure to Cc LAKML along with relevant parties when
>> sending arm64 patches/queries.
>>
>> For everyone newly Cc'd, the original message (with attachments) can be
>> found at:
>>
>> http://lkml.kernel.org/r/7618d76d-bfa8-d8aa-59aa-06f9d90c1a98 at huawei.com
>>
>>> When I enable NUMA in BIOS for arm64, it failed to boot on v4.8-rc4-162-g071e31e.
>>
>> That commit ID doesn't seem to be in mainline (I can't find it in my
>> local tree). Which tree are you using? Do you have local patches
>> applied?
> 
> That commit is in mainline:
> 
>   http://git.kernel.org/linus/071e31e
> 
> It would be nice to know if the problem also exists on the arm64
> for-next/core branch.
> 
> Will
> 
> 
>> I take it that by "enable NUMA in BIOS", you mean exposing SRAT to the
>> OS?
>>
>>> For the crash log, it seems caused by error number of cpumask.
>>> Any ideas about it?
>>
>> Much earlier in your log, there was a (non-fatal) warning, as below. Do
>> you see this without NUMA/SRAT enabled in your FW? I don't see how the
>> SRAT should affect the secondaries we try to bring online.
>>
>> Given your MPIDRs have Aff2 bits set, I wonder if we've conflated a
>> logical ID with a physical ID somewhere, and it just so happens that the
>> NUMA code is more likely to poke something based on that.
>>
>> Can you modify the warning in cpumask.h to dump the bad CPU number? That
>> would make it fairly clear if that's the case.
>>
>> Thanks,
>> Mark.
>>
>>> [    0.297337] Detected PIPT I-cache on CPU1
>>> [    0.297347] GICv3: CPU1: found redistributor 10001 region 1:0x000000004d140000
>>> [    0.297356] CPU1: Booted secondary processor [410fd082]
>>> [    0.297375] ------------[ cut here ]------------
>>> [    0.320390] WARNING: CPU: 1 PID: 0 at ./include/linux/cpumask.h:121 gic_raise_softirq+0x128/0x17c
>>> [    0.329356] Modules linked in:
>>> [    0.332434] 
>>> [    0.333932] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0-rc4-00163-g803ea3a #21
>>> [    0.341581] Hardware name: Hisilicon Hi1616 Evaluation Board (DT)
>>> [    0.347735] task: ffff8013e9dd0000 task.stack: ffff8013e9dcc000
>>> [    0.353714] PC is at gic_raise_softirq+0x128/0x17c
>>> [    0.358550] LR is at gic_raise_softirq+0xa0/0x17c
>>> [    0.363298] pc : [<ffff00000838c124>] lr : [<ffff00000838c09c>] pstate: 200001c5
>>> [    0.370770] sp : ffff8013e9dcfde0
>>> [    0.374112] x29: ffff8013e9dcfde0 x28: 0000000000000000 
>>> [    0.379476] x27: 000000000083207c x26: ffff000008ca5d70 
>>> [    0.384841] x25: 0000000100000001 x24: ffff000008d63ff3 
>>> [    0.390205] x23: 0000000000000000 x22: ffff000008cb0000 
>>> [    0.395569] x21: ffff00000884edb0 x20: 0000000000000001 
>>> [    0.400933] x19: 0000000100000000 x18: 0000000000000000 
>>> [    0.406298] x17: 0000000000000000 x16: 0000000003010066 
>>> [    0.411661] x15: ffff000008ca8000 x14: 0000000000000013 
>>> [    0.417025] x13: 0000000000000000 x12: 0000000000000013 
>>> [    0.422389] x11: 0000000000000013 x10: 0000000002e92aa7 
>>> [    0.427754] x9 : 0000000000000000 x8 : ffff8413eb6ca668 
>>> [    0.433118] x7 : ffff8413eb6ca690 x6 : 0000000000000000 
>>> [    0.438482] x5 : fffffffffffffffe x4 : 0000000000000000 
>>> [    0.443845] x3 : 0000000000000040 x2 : 0000000000000041 
>>> [    0.449209] x1 : 0000000000000000 x0 : 0000000000000001 
>>> [    0.454573] 
>>> [    0.456069] ---[ end trace b58e70f3295a8cd7 ]---
>>> [    0.460730] Call trace:
>>> [    0.463193] Exception stack(0xffff8013e9dcfc10 to 0xffff8013e9dcfd40)
>>> [    0.469699] fc00:                                   0000000100000000 0001000000000000
>>> [    0.477611] fc20: ffff8013e9dcfde0 ffff00000838c124 ffff000008d72228 ffff8013e9dcff70
>>> [    0.485524] fc40: ffff000008d72608 ffff000008ab02a4 0000000000000000 0000000000000000
>>> [    0.493436] fc60: 0000000000000000 3464313430303030 0000000000000000 0000000000000000
>>> [    0.501348] fc80: ffff8013e9dcfc90 ffff00000836e678 ffff8013e9dcfca0 ffff00000836e910
>>> [    0.509259] fca0: ffff8013e9dcfd30 ffff00000836ec10 0000000000000001 0000000000000000
>>> [    0.517171] fcc0: 0000000000000041 0000000000000040 0000000000000000 fffffffffffffffe
>>> [    0.525083] fce0: 0000000000000000 ffff8413eb6ca690 ffff8413eb6ca668 0000000000000000
>>> [    0.532995] fd00: 0000000002e92aa7 0000000000000013 0000000000000013 0000000000000000
>>> [    0.540907] fd20: 0000000000000013 ffff000008ca8000 0000000003010066 0000000000000000
>>> [    0.548819] [<ffff00000838c124>] gic_raise_softirq+0x128/0x17c
>>> [    0.554713] [<ffff00000808e1f4>] smp_send_reschedule+0x34/0x3c
>>> [    0.560605] [<ffff0000080ddf18>] resched_curr+0x40/0x5c
>>> [    0.565881] [<ffff0000080de650>] check_preempt_curr+0x58/0xa0
>>> [    0.571685] [<ffff0000080de6b0>] ttwu_do_wakeup+0x18/0x80
>>> [    0.577136] [<ffff0000080de790>] ttwu_do_activate+0x78/0x88
>>> [    0.582763] [<ffff0000080df5cc>] try_to_wake_up+0x1f8/0x300
>>> [    0.588390] [<ffff0000080df79c>] default_wake_function+0x10/0x18
>>> [    0.594458] [<ffff0000080f3210>] __wake_up_common+0x5c/0x9c
>>> [    0.600085] [<ffff0000080f3264>] __wake_up_locked+0x14/0x1c
>>> [    0.605712] [<ffff0000080f3e10>] complete+0x40/0x5c
>>> [    0.610635] [<ffff00000808dba8>] secondary_start_kernel+0x148/0x1a8
>>> [    0.616965] [<00000000000831a8>] 0x831a8
>>
> 
> .
> 

^ permalink raw reply

* [PATCH 2/8] drm/rockchip: Get rid of some unnecessary code
From: Mark yao @ 2016-09-20  1:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAAFQd5A0M6vAbXHhUDMMXsaWND0hzWcT71HzOsSSCHa--qh0dA@mail.gmail.com>

On 2016?09?18? 12:01, Tomasz Figa wrote:
> Hi Mark,
>
> On Sun, Sep 18, 2016 at 10:50 AM, Mark yao <mark.yao@rock-chips.com> wrote:
>> On 2016?09?14? 20:54, Tomasz Figa wrote:
>>> Current code implements prepare_fb and cleanup_fb callbacks only to
>>> grab/release fb references, which is already done by atomic framework
>>> when creating/destryoing plane state. Let's remove these
>>> unused bits.
>>>
>>> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>>> ---
>>>    drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ------------------
>>>    1 file changed, 18 deletions(-)
>>
>> Hi Tomasz
>>
>> I think we can't get rid of the prepare_fb and cleanup_fb
> I think I have to disagree. Please see below for detailed explanation.
>
>> see the reason:
>> commit 44d0237a26395ac94160cf23f32769013b365590
>> Author: Mark Yao <mark.yao@rock-chips.com>
>> Date:   Fri Apr 29 11:37:20 2016 +0800
>>
>>      drm/rockchip: vop: fix iommu crash with async atomic
>>
>>      After async atomic_commit callback, drm_atomic_clean_old_fb will
>>      clean all old fb, but because async, the old fb may be also on
>>      the vop hardware, dma will access the old fb buffer, clean old
>>      fb will cause iommu page fault.
> I think the above is not quite right. Atomic plane state holds a
> reference to its fb and old state is not supposed to be destroyed
> until the flip completes.
>
> Indeed current rockchip_atomic_commit implementation has following
> order of calls: rockchip_atomic_wait_for_complete(),
> drm_atomic_helper_cleanup_planes(), drm_atomic_state_free(). This
> means that .cleanup_fb() is called from
> drm_atomic_helper_cleanup_planes() just before drm_atomic_state_free()
> will release references by destroying old plane states. Note that both
> are called already after rockchip_atomic_wait_for_complete(), so it
> should be already safe to free the old fbs.
>
> So the above fix doesn't really do anything, possibly just covers the
> race condition of the original wait for vblank function by delaying
> drm_atomic_state_free() a bit.
>
> Moreover, the whole series has been thoroughly tested in Chrome OS 4.4
> kernel, including async commits. (There is still a possibility some
> newer upstream changes slightly modified the semantics, but I couldn't
> find such difference. Actually one of the advantages of atomic helpers
> was to avoid manually refcounting the fbs from the driver.)
>
> Best regards,
> Tomasz
>
Hi Tomasz

You are right, plane_duplicate_state/plane_destroy_state already protect 
the old fbs.
we can get rid of prepare_fb and cleanup_fb.

-- 
?ark Yao

^ permalink raw reply

* [PATCH 1/2] net: ethernet: broadcom: bcm63xx: use phydev from struct net_device
From: David Miller @ 2016-09-20  1:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474210747-19627-1-git-send-email-tremyfr@gmail.com>

From: Philippe Reynes <tremyfr@gmail.com>
Date: Sun, 18 Sep 2016 16:59:06 +0200

> The private structure contain a pointer to phydev, but the structure
> net_device already contain such pointer. So we can remove the pointer
> phydev in the private structure, and update the driver to use the
> one contained in struct net_device.
> 
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>

Applied.

^ permalink raw reply

* [PATCH 2/2] net: ethernet: broadcom: bcm63xx: use new api ethtool_{get|set}_link_ksettings
From: David Miller @ 2016-09-20  1:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474210747-19627-2-git-send-email-tremyfr@gmail.com>

From: Philippe Reynes <tremyfr@gmail.com>
Date: Sun, 18 Sep 2016 16:59:07 +0200

> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> Signed-off-by: Philippe Reynes <tremyfr@gmail.com>

Applied.

^ permalink raw reply

* [PATCH] mtd: s3c2410: add device tree support
From: Sergio Prado @ 2016-09-20  2:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160917193123.762587d0@bbrezillon>

On Sat, Sep 17, 2016 at 07:31:23PM +0200, Boris Brezillon wrote:
> Hi Sergio,
> 
> On Sat, 17 Sep 2016 12:22:40 -0300
> Sergio Prado <sergio.prado@e-labworks.com> wrote:
> 
> > Tested on FriendlyARM Mini2440
> > 
> > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> > ---
> >  .../devicetree/bindings/mtd/samsung-s3c2410.txt    |  70 +++++++++++
> 
> DT maintainers usually ask people to keep the DT bindings doc in a
> separate patch.

Right. I'll send the DT bindings doc in a separate patch.

> 
> >  drivers/mtd/nand/s3c2410.c                         | 129 ++++++++++++++++++++-
> >  include/linux/platform_data/mtd-nand-s3c2410.h     |   1 +
> >  3 files changed, 195 insertions(+), 5 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> > new file mode 100644
> > index 000000000000..1c39f6cf483b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> > @@ -0,0 +1,70 @@
> > +* Samsung S3C2410 and compatible NAND flash controller
> > +
> > +Required properties:
> > +- compatible : The possible values are:
> > +	"samsung,s3c2410-nand"
> > +	"samsung,s3c2412-nand"
> > +	"samsung,s3c2440-nand"
> > +	"samsung,s3c6400-nand"
> > +- reg : register's location and length.
> > +- #address-cells, #size-cells : see nand.txt
> > +- clocks : phandle to the nand controller clock
> > +- clock-names : must contain "nand"
> > +
> > +Optional properties:
> > +- samsung,tacls : time for active CLE/ALE to nWE/nOE
> > +- samsung,twrph0 : active time for nWE/nOE
> > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive
> 
> Can you try to extract these information from the nand_sdr_timings
> struct instead of passing it in your DT?

OK. Looks like it is possible to extract the timings from
nand_sdr_timings. I'll try to do it.

> 
> > +- samsung,ignore_unset_ecc : boolean to ignore error when we have
> > +                             0xff,0xff,0xff read ECC, on the
> > +                             assumption that it is an un-eccd page
> > +
> > +Optional children nodes:
> > +Children nodes representing the available nand chips.
> > +
> > +Optional children properties:
> > +- nand-ecc-mode : see nand.txt
> > +- nand-on-flash-bbt : see nand.txt
> > +
> > +Each children device node may optionally contain a 'partitions' sub-node,
> > +which further contains sub-nodes describing the flash partition mapping.
> > +See partition.txt for more detail.
> > +
> > +Example:
> > +
> > +nand at 4e000000 {
> 
> s/nand/nand-controller/

Ops. My bad.

> 
> > +	compatible = "samsung,s3c2440-nand";
> > +	reg = <0x4e000000 0x40>;
> > +
> > +	#address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +	clocks = <&clocks HCLK_NAND>;
> > +	clock-names = "nand";
> > +
> > +	samsung,tacls = <0>;
> > +	samsung,twrph0 = <25>;
> > +	samsung,twrph1 = <15>;
> 
> As said above, I think these timings can be extracted from the
> nand_sdr_timings struct, which is know automatically attached to
> nand_chip at detection time.

Right.

> 
> > +	samsung,ignore_unset_ecc;
> 
> Just discovered this ->ignore_unset_ecc property, and I don't
> understand why it's here for...
> Either you don't want to have ECC, and in this case you should set
> NAND_ECC_NONE, or you want to have ECC calculated, and in this case,
> the only valid situation where ECC bytes are 0xff is when the page is
> erased.
> 
> If I'm right, please fix the driver instead of adding this DT property.
> If I'm wrong, could you explain in more detail when you have ECC bytes
> set to 0xff?

That's what I initially thought, but I must confess I just focused on
keeping the same interface. I'll try to understand better if this check is
really necessary.

> 
> > +
> > +	nand at 0 {
> 
> @0 implies having a reg property. I don't see any in your example, and
> it's not document in the required property list.
> 
> Is your controller able to connect to different CS?

No, it is not necessary. I'll remove @0.

> 
> > +		nand-ecc-mode = "soft";
> > +		nand-on-flash-bbt;
> > +
> > +		partitions {
> > +			compatible = "fixed-partitions";
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +
> > +			partition at 0 {
> > +				label = "u-boot";
> > +				reg = <0 0x040000>;
> > +			};
> > +
> > +			partition at 40000 {
> > +				label = "kernel";
> > +				reg = <0x040000 0x500000>;
> > +			};
> > +		};
> > +	};
> > +};
> > diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
> > index d9309cf0ce2e..ecbb9c9c1e9a 100644
> > --- a/drivers/mtd/nand/s3c2410.c
> > +++ b/drivers/mtd/nand/s3c2410.c
> > @@ -39,6 +39,8 @@
> >  #include <linux/slab.h>
> >  #include <linux/clk.h>
> >  #include <linux/cpufreq.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> >  
> >  #include <linux/mtd/mtd.h>
> >  #include <linux/mtd/nand.h>
> > @@ -185,6 +187,26 @@ struct s3c2410_nand_info {
> >  #endif
> >  };
> >  
> > +struct s3c24XX_nand_devtype_data {
> > +	enum s3c_cpu_type type;
> > +};
> > +
> > +struct s3c24XX_nand_devtype_data s3c2410_nand_devtype_data = {
> > +	.type = TYPE_S3C2410,
> > +};
> > +
> > +struct s3c24XX_nand_devtype_data s3c2412_nand_devtype_data = {
> > +	.type = TYPE_S3C2412,
> > +};
> > +
> > +struct s3c24XX_nand_devtype_data s3c2440_nand_devtype_data = {
> > +	.type = TYPE_S3C2440,
> > +};
> > +
> > +struct s3c24XX_nand_devtype_data s3c6400_nand_devtype_data = {
> > +	.type = TYPE_S3C2412,
> > +};
> > +
> >  /* conversion functions */
> >  
> >  static struct s3c2410_nand_mtd *s3c2410_nand_mtd_toours(struct mtd_info *mtd)
> > @@ -813,6 +835,8 @@ static void s3c2410_nand_init_chip(struct s3c2410_nand_info *info,
> >  	struct nand_chip *chip = &nmtd->chip;
> >  	void __iomem *regs = info->regs;
> >  
> > +	nand_set_flash_node(chip, set->of_node);
> > +
> >  	chip->write_buf    = s3c2410_nand_write_buf;
> >  	chip->read_buf     = s3c2410_nand_read_buf;
> >  	chip->select_chip  = s3c2410_nand_select_chip;
> > @@ -947,6 +971,96 @@ static void s3c2410_nand_update_chip(struct s3c2410_nand_info *info,
> >  	}
> >  }
> >  
> > +#ifdef CONFIG_OF_MTD
> 
> Hm, I thought this symbol had been dropped, but apparently I forgot to
> remove it. You should make it dependent on CONFIG_OF, not CONFIG_OF_MTD.
> 
> Anyway, I'm not even sure this ifdef is needed. Just test if
> pdev->dev.of_node is NULL in s3c24xx_nand_probe_dt() and return -1 in
> this case.

Right. I'll remove this ifdef.

> 
> > +static const struct of_device_id s3c24xx_nand_dt_ids[] = {
> > +	{
> > +		.compatible = "samsung,s3c2410-nand",
> > +		.data = &s3c2410_nand_devtype_data,
> > +	}, {
> > +		.compatible = "samsung,s3c2412-nand",
> > +		.data = &s3c2412_nand_devtype_data,
> > +	}, {
> > +		.compatible = "samsung,s3c2440-nand",
> > +		.data = &s3c2440_nand_devtype_data,
> > +	}, {
> > +		.compatible = "samsung,s3c6400-nand",
> > +		.data = &s3c6400_nand_devtype_data,
> > +	},
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, s3c24xx_nand_dt_ids);
> > +
> > +static int s3c24xx_nand_probe_dt(struct platform_device *pdev)
> > +{
> > +	const struct s3c24XX_nand_devtype_data *devtype_data;
> > +	struct s3c2410_platform_nand *pdata;
> > +	struct s3c2410_nand_info *info = platform_get_drvdata(pdev);
> > +	struct device_node *np = pdev->dev.of_node, *child;
> > +	const struct of_device_id *of_id;
> > +	struct s3c2410_nand_set *sets;
> > +
> > +	of_id = of_match_device(s3c24xx_nand_dt_ids, &pdev->dev);
> > +	if (!of_id)
> > +		return 1;
> > +
> > +	devtype_data = of_id->data;
> > +	info->cpu_type = devtype_data->type;
> > +
> > +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> > +	if (!pdata)
> > +		return -ENOMEM;
> > +
> > +	pdev->dev.platform_data = pdata;
> > +
> > +	of_property_read_u32(np, "samsung,tacls",  &pdata->tacls);
> > +	of_property_read_u32(np, "samsung,twrph0", &pdata->twrph0);
> > +	of_property_read_u32(np, "samsung,twrph1", &pdata->twrph1);
> > +
> > +	if (of_get_property(np, "samsung,ignore_unset_ecc", NULL))
> > +		pdata->ignore_unset_ecc = 1;
> > +
> > +	pdata->nr_sets = of_get_child_count(np);
> > +	if (!pdata->nr_sets)
> > +		return 0;
> > +
> > +	sets = devm_kzalloc(&pdev->dev, sizeof(*sets) * pdata->nr_sets,
> > +			   GFP_KERNEL);
> > +	if (!sets)
> > +		return -ENOMEM;
> > +
> > +	pdata->sets = sets;
> > +
> > +	for_each_available_child_of_node(np, child) {
> > +
> > +		sets->name = (char *)child->name;
> > +		sets->of_node = child;
> > +		sets->nr_chips = 1;
> > +
> > +		if (!of_property_match_string(child, "nand-ecc-mode", "none"))
> > +			sets->disable_ecc = 1;
> > +
> > +		if (of_get_property(child, "nand-on-flash-bbt", NULL))
> > +			sets->flash_bbt = 1;
> > +
> 
> These properties are automatically extracted in nand_scan_ident(), why
> do you need to parse them twice?

Looks like the driver uses this properties before they are extracted in
nand_scan_ident(). But I'll try to understand better what the driver is
doing and prevent from parsing these properties twice.

> 
> > +		sets++;
> > +	}
> > +
> > +	return 0;
> > +}
> > +#else
> > +static int s3c24xx_nand_probe_dt(struct platform_device *pdev)
> > +{
> > +	return 1;
> > +}
> > +#endif
> > +
> > +static void s3c24xx_nand_probe_pdata(struct platform_device *pdev)
> > +{
> > +	struct s3c2410_nand_info *info = platform_get_drvdata(pdev);
> > +
> > +	info->cpu_type = platform_get_device_id(pdev)->driver_data;
> > +}
> > +
> >  /* s3c24xx_nand_probe
> >   *
> >   * called by device layer when it finds a device matching
> > @@ -956,8 +1070,7 @@ static void s3c2410_nand_update_chip(struct s3c2410_nand_info *info,
> >  */
> >  static int s3c24xx_nand_probe(struct platform_device *pdev)
> >  {
> > -	struct s3c2410_platform_nand *plat = to_nand_plat(pdev);
> > -	enum s3c_cpu_type cpu_type;
> > +	struct s3c2410_platform_nand *plat;
> >  	struct s3c2410_nand_info *info;
> >  	struct s3c2410_nand_mtd *nmtd;
> >  	struct s3c2410_nand_set *sets;
> > @@ -967,8 +1080,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
> >  	int nr_sets;
> >  	int setno;
> >  
> > -	cpu_type = platform_get_device_id(pdev)->driver_data;
> > -
> >  	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> >  	if (info == NULL) {
> >  		err = -ENOMEM;
> > @@ -991,6 +1102,14 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
> >  
> >  	s3c2410_nand_clk_set_state(info, CLOCK_ENABLE);
> >  
> > +	err = s3c24xx_nand_probe_dt(pdev);
> > +	if (err > 0)
> > +		s3c24xx_nand_probe_pdata(pdev);
> > +	else if (err < 0)
> > +		goto exit_error;
> > +
> > +	plat = to_nand_plat(pdev);
> > +
> >  	/* allocate and map the resource */
> >  
> >  	/* currently we assume we have the one resource */
> > @@ -999,7 +1118,6 @@ static int s3c24xx_nand_probe(struct platform_device *pdev)
> >  
> >  	info->device	= &pdev->dev;
> >  	info->platform	= plat;
> > -	info->cpu_type	= cpu_type;
> >  
> >  	info->regs = devm_ioremap_resource(&pdev->dev, res);
> >  	if (IS_ERR(info->regs)) {
> > @@ -1156,6 +1274,7 @@ static struct platform_driver s3c24xx_nand_driver = {
> >  	.id_table	= s3c24xx_driver_ids,
> >  	.driver		= {
> >  		.name	= "s3c24xx-nand",
> > +		.of_match_table = s3c24xx_nand_dt_ids,
> 
> If you keep the #ifdef CONFIG_OF section
> 
> 		.of_match_table = of_match_ptr(s3c24xx_nand_dt_ids),

I'll remove the ifdef.

> 
> >  	},
> >  };
> >  
> > diff --git a/include/linux/platform_data/mtd-nand-s3c2410.h b/include/linux/platform_data/mtd-nand-s3c2410.h
> > index c55e42ee57fa..9d20871e4bbd 100644
> > --- a/include/linux/platform_data/mtd-nand-s3c2410.h
> > +++ b/include/linux/platform_data/mtd-nand-s3c2410.h
> > @@ -40,6 +40,7 @@ struct s3c2410_nand_set {
> >  	char			*name;
> >  	int			*nr_map;
> >  	struct mtd_partition	*partitions;
> > +	struct device_node	*of_node;
> >  };
> >  
> >  struct s3c2410_platform_nand {
> 

-- 
Sergio Prado
Embedded Labworks
Office: +55 11 2628-3461
Mobile: +55 11 97123-3420

^ permalink raw reply

* [PATCH] mtd: s3c2410: add device tree support
From: Sergio Prado @ 2016-09-20  2:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160919091155.GB11118@leverpostej>

On Mon, Sep 19, 2016 at 10:11:55AM +0100, Mark Rutland wrote:
> On Sat, Sep 17, 2016 at 12:22:40PM -0300, Sergio Prado wrote:
> > Tested on FriendlyARM Mini2440
> > 
> > Signed-off-by: Sergio Prado <sergio.prado@e-labworks.com>
> > ---
> >  .../devicetree/bindings/mtd/samsung-s3c2410.txt    |  70 +++++++++++
> >  drivers/mtd/nand/s3c2410.c                         | 129 ++++++++++++++++++++-
> >  include/linux/platform_data/mtd-nand-s3c2410.h     |   1 +
> >  3 files changed, 195 insertions(+), 5 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> > new file mode 100644
> > index 000000000000..1c39f6cf483b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/samsung-s3c2410.txt
> > @@ -0,0 +1,70 @@
> > +* Samsung S3C2410 and compatible NAND flash controller
> > +
> > +Required properties:
> > +- compatible : The possible values are:
> > +	"samsung,s3c2410-nand"
> > +	"samsung,s3c2412-nand"
> > +	"samsung,s3c2440-nand"
> > +	"samsung,s3c6400-nand"
> > +- reg : register's location and length.
> > +- #address-cells, #size-cells : see nand.txt
> > +- clocks : phandle to the nand controller clock
> > +- clock-names : must contain "nand"
> 
> Is there only one clock feeding into the NAND block? Is it actually
> called "nand", or does the datasheet not give the input a nme?

Yes, there is only one clock associated with the NAND controller. The
clock is called "NAND Flash Controller" clock by the datasheet.

> 
> > +
> > +Optional properties:
> > +- samsung,tacls : time for active CLE/ALE to nWE/nOE
> > +- samsung,twrph0 : active time for nWE/nOE
> > +- samsung,twrph1 : time for release CLE/ALE from nWE/nOE inactive
> 
> If these are necessary, what units are these in?

They are probably not necessary and will be removed as suggested by
Boris Brezillon.

> 
> > +- samsung,ignore_unset_ecc : boolean to ignore error when we have
> > +                             0xff,0xff,0xff read ECC, on the
> > +                             assumption that it is an un-eccd page
> 
> s/_/-/ in property names, please.

OK. If I keep this property, I will change it to
samsung,ignore-unset-ecc.

> 
> That said, this name reads like policy rather than a HW description.
> Why/when is this necessary to set?

I'll evaluate if this property is really necessary, as suggested by
Boris Brezillon.

> 
> [...]
> 
> > +	of_property_read_u32(np, "samsung,tacls",  &pdata->tacls);
> > +	of_property_read_u32(np, "samsung,twrph0", &pdata->twrph0);
> > +	of_property_read_u32(np, "samsung,twrph1", &pdata->twrph1);
> 
> Are all values in the range [0, 0xffffffff] valid for these? If not, it
> would be a good idea to sanity-check the values.

OK.

> 
> > +	if (of_get_property(np, "samsung,ignore_unset_ecc", NULL))
> > +		pdata->ignore_unset_ecc = 1;
> 
> Use of_property_read_bool();

Right.

> 
> [...]
> 
> > +	for_each_available_child_of_node(np, child) {
> > +
> > +		sets->name = (char *)child->name;
> > +		sets->of_node = child;
> 
> Strictly speaking, you should increment the child node refcount here, as
> you maintain a reference to it and its name field.

OK.

> 
> > +		sets->nr_chips = 1;
> > +
> > +		if (!of_property_match_string(child, "nand-ecc-mode", "none"))
> > +			sets->disable_ecc = 1;
> > +
> > +		if (of_get_property(child, "nand-on-flash-bbt", NULL))
> > +			sets->flash_bbt = 1;
> 
> Use of_property_read_bool().

OK.

> 
> Thanks,
> Mark.

-- 
Sergio Prado
Embedded Labworks
Office: +55 11 2628-3461
Mobile: +55 11 97123-3420

^ permalink raw reply

* [PATCH] mtd: s3c2410: add device tree support
From: Sergio Prado @ 2016-09-20  2:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f8907616-0d6b-37ef-1419-f6aedf3ddbdc@samsung.com>

On Mon, Sep 19, 2016 at 12:44:07PM +0200, Sylwester Nawrocki wrote:
> On 09/17/2016 05:22 PM, Sergio Prado wrote:
> > +static int s3c24xx_nand_probe_dt(struct platform_device *pdev)
> > +{
> > +	const struct s3c24XX_nand_devtype_data *devtype_data;
> > +	struct s3c2410_platform_nand *pdata;
> > +	struct s3c2410_nand_info *info = platform_get_drvdata(pdev);
> > +	struct device_node *np = pdev->dev.of_node, *child;
> > +	const struct of_device_id *of_id;
> > +	struct s3c2410_nand_set *sets;
> > +
> > +	of_id = of_match_device(s3c24xx_nand_dt_ids, &pdev->dev);
> > +	if (!of_id)
> > +		return 1;
> > +
> > +	devtype_data = of_id->data;
> 
> You could make it a bit simpler with of_device_get_match_data().

I wasn't aware of this interface. I'll use it. Thanks!

> 
> --
> Thanks,
> Sylwester

-- 
Sergio Prado
Embedded Labworks
Office: +55 11 2628-3461
Mobile: +55 11 97123-3420

^ permalink raw reply

* [RFC PATCH v2 11/11] irqchip: mbigen: promote mbigen init
From: Hanjun Guo @ 2016-09-20  2:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <57DFBA0D.7040801@arm.com>

On 2016/9/19 18:12, Marc Zyngier wrote:
> On 19/09/16 10:49, Hanjun Guo wrote:
>> On 2016/9/15 23:24, Marc Zyngier wrote:
>>> On 14/09/16 15:21, Hanjun Guo wrote:
>>>> From: Hanjun Guo <hanjun.guo@linaro.org>
>>>>
>>>> mbigen is an irqchip and it needs to be probed before
>>>> devices, same logic is used for SMMU and etc., let's
>>>> use arch_initcall instead of platform init for mbigen.
>>>>
>>>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>> Cc: Ma Jun <majun258@huawei.com>
>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>>>> ---
>>>>  drivers/irqchip/irq-mbigen.c | 6 +++++-
>>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
>>>> index ca6add1..3a33de6 100644
>>>> --- a/drivers/irqchip/irq-mbigen.c
>>>> +++ b/drivers/irqchip/irq-mbigen.c
>>>> @@ -374,7 +374,11 @@ static struct platform_driver mbigen_platform_driver = {
>>>>  	.probe			= mbigen_device_probe,
>>>>  };
>>>>
>>>> -module_platform_driver(mbigen_platform_driver);
>>>> +static __init int mbigen_init(void)
>>>> +{
>>>> +	return platform_driver_register(&mbigen_platform_driver);
>>>> +}
>>>> +arch_initcall(mbigen_init);
>>>>
>>>>  MODULE_AUTHOR("Jun Ma <majun258@huawei.com>");
>>>>  MODULE_AUTHOR("Yun Wu <wuyun.wu@huawei.com>");
>>>>
>>> I've already NACKed such a patch in the past, and I will carry on
>>> NACKing it until all the other avenues (like properly tracking device
>>> dependencies) have been explored and have been proven not to be fit for
>>> purpose.
>> I'd happy to work on this to make progress.
>>
>>> Rafael had proposed something around this subject last year, and I've
>> Sorry, obviously I missed something, could you give me the link about
>> Rafael's patches? I will pull back and test it on our hardware.
> Please see this discussion from almost a year ago:
>
> https://patchwork.kernel.org/patch/7407401/
>
> in which I give the existing pointers and explain why I'm pushing back
> on things like this patch.

We (Majun and me ) will look into it, thanks!

Hanjun

^ permalink raw reply

* [PATCH 0/5] ARM64: Uprobe support added
From: Pratyush Anand @ 2016-09-20  2:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160824072622.GB24311@localhost.localdomain>

Hi Will/Catalin,

So far there is no comment which would require any modification in
this patch set. However, a rebase would be required, because we have a
fixup "arm64: Improve kprobes test for atomic sequence" in the next
now. Also, since we have "arm64: kprobe: Always clear pstate.D in
breakpoint exception handler" in next, so I can remove __kprobes tag
from uprobe_breakpoint_handler() and uprobe_single_step_handler().

Do you think that these patches are good to be taken for 4.9? If yes,
then I will send a quick V2 with above modifications.

~Pratyush


On Wed, Aug 24, 2016 at 12:56 PM, Pratyush Anand <panand@redhat.com> wrote:
> Hi Will/Catalin,
>
> Do you have any specific comment for this patch set?
>
> ~Pratyush
>
> [1] https://lkml.org/lkml/2016/8/22/69
>
> On 02/08/2016:11:00:04 AM, Pratyush Anand wrote:
>> ARM64 kprobe support is lying in torvalds/linux.git:master now. Therefore
>> sending my uprobe patches which were dependent on decode-insn code of kprobe
>> patches.
>>
>> Unit tests for following have been done so far and they have been found
>> working.
>>      1. Step-able instructions, like sub, ldr, add etc.
>>      2. Simulation-able like ret, cbnz, cbz etc.
>>      3. uretprobe
>>      4. Reject-able instructions like sev, wfe etc.
>>      5. trapped and abort xol path
>>      6. probe at unaligned user address.
>>      7. longjump test cases
>>
>> Currently it does not support aarch32 instruction probing.
>>
>> RFC patches were sent long back, and all review comments for RFCs have been
>> incorporated. RFCs were here: https://lwn.net/Articles/648514/
>>
>> Pratyush Anand (5):
>>   arm64: kprobe: protect/rename few definitions to be reused by uprobe
>>   arm64: kgdb_step_brk_fn: ignore other's exception
>>   arm64: Handle TRAP_HWBRKPT for user mode as well
>>   arm64: Handle TRAP_BRKPT for user mode as well
>>   arm64: Add uprobe support
>>
>>  arch/arm64/Kconfig                      |   3 +
>>  arch/arm64/include/asm/debug-monitors.h |   3 +
>>  arch/arm64/include/asm/probes.h         |  23 ++--
>>  arch/arm64/include/asm/ptrace.h         |   8 ++
>>  arch/arm64/include/asm/thread_info.h    |   5 +-
>>  arch/arm64/include/asm/uprobes.h        |  37 ++++++
>>  arch/arm64/kernel/debug-monitors.c      |  40 +++---
>>  arch/arm64/kernel/entry.S               |   6 +-
>>  arch/arm64/kernel/kgdb.c                |   3 +
>>  arch/arm64/kernel/probes/Makefile       |   2 +
>>  arch/arm64/kernel/probes/decode-insn.c  |  31 ++---
>>  arch/arm64/kernel/probes/decode-insn.h  |   8 +-
>>  arch/arm64/kernel/probes/kprobes.c      |  36 ++---
>>  arch/arm64/kernel/probes/uprobes.c      | 227 ++++++++++++++++++++++++++++++++
>>  arch/arm64/kernel/signal.c              |   4 +-
>>  arch/arm64/mm/flush.c                   |   6 +
>>  16 files changed, 378 insertions(+), 64 deletions(-)
>>  create mode 100644 arch/arm64/include/asm/uprobes.h
>>  create mode 100644 arch/arm64/kernel/probes/uprobes.c
>>
>> --
>> 2.5.5

^ permalink raw reply

* [RFC] Arm64 boot fail with numa enable in BIOS
From: Hanjun Guo @ 2016-09-20  2:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160919140709.GA17464@leverpostej>

On 2016/9/19 22:07, Mark Rutland wrote:
> [adding LAKML, arm64 maintainers]
>
> On Mon, Sep 19, 2016 at 09:05:26PM +0800, Yisheng Xie wrote:
>> hi all,
> Hi,
>
> In future, please make sure to Cc LAKML along with relevant parties when
> sending arm64 patches/queries.
>
> For everyone newly Cc'd, the original message (with attachments) can be
> found at:
>
> http://lkml.kernel.org/r/7618d76d-bfa8-d8aa-59aa-06f9d90c1a98 at huawei.com
>
>> When I enable NUMA in BIOS for arm64, it failed to boot on v4.8-rc4-162-g071e31e.
> That commit ID doesn't seem to be in mainline (I can't find it in my
> local tree). Which tree are you using? Do you have local patches
> applied?

Yes, we have GICv3 ITS and mbigen patches on top which trying to enable PCI msi
and native SAS on the board.

>
> I take it that by "enable NUMA in BIOS", you mean exposing SRAT to the
> OS?

Yes, SRAT and SLIT.

>
>> For the crash log, it seems caused by error number of cpumask.
>> Any ideas about it?
> Much earlier in your log, there was a (non-fatal) warning, as below. Do
> you see this without NUMA/SRAT enabled in your FW? 

Works ok without NUMA/SRAT enabled, we will check the SRAT table.

> I don't see how the
>
> SRAT should affect the secondaries we try to bring online.

Yes, CPU masks and secondaries boot up is related MADT not SRAT.

Thanks
Hanjun

>
> Given your MPIDRs have Aff2 bits set, I wonder if we've conflated a
> logical ID with a physical ID somewhere, and it just so happens that the
> NUMA code is more likely to poke something based on that.
>
> Can you modify the warning in cpumask.h to dump the bad CPU number? That
> would make it fairly clear if that's the case.
>
> Thanks,
> Mark.
>
>> [    0.297337] Detected PIPT I-cache on CPU1
>> [    0.297347] GICv3: CPU1: found redistributor 10001 region 1:0x000000004d140000
>> [    0.297356] CPU1: Booted secondary processor [410fd082]
>> [    0.297375] ------------[ cut here ]------------
>> [    0.320390] WARNING: CPU: 1 PID: 0 at ./include/linux/cpumask.h:121 gic_raise_softirq+0x128/0x17c
>> [    0.329356] Modules linked in:
>> [    0.332434] 
>> [    0.333932] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0-rc4-00163-g803ea3a #21
>> [    0.341581] Hardware name: Hisilicon Hi1616 Evaluation Board (DT)
>> [    0.347735] task: ffff8013e9dd0000 task.stack: ffff8013e9dcc000
>> [    0.353714] PC is at gic_raise_softirq+0x128/0x17c
>> [    0.358550] LR is at gic_raise_softirq+0xa0/0x17c
>> [    0.363298] pc : [<ffff00000838c124>] lr : [<ffff00000838c09c>] pstate: 200001c5
>> [    0.370770] sp : ffff8013e9dcfde0
>> [    0.374112] x29: ffff8013e9dcfde0 x28: 0000000000000000 
>> [    0.379476] x27: 000000000083207c x26: ffff000008ca5d70 
>> [    0.384841] x25: 0000000100000001 x24: ffff000008d63ff3 
>> [    0.390205] x23: 0000000000000000 x22: ffff000008cb0000 
>> [    0.395569] x21: ffff00000884edb0 x20: 0000000000000001 
>> [    0.400933] x19: 0000000100000000 x18: 0000000000000000 
>> [    0.406298] x17: 0000000000000000 x16: 0000000003010066 
>> [    0.411661] x15: ffff000008ca8000 x14: 0000000000000013 
>> [    0.417025] x13: 0000000000000000 x12: 0000000000000013 
>> [    0.422389] x11: 0000000000000013 x10: 0000000002e92aa7 
>> [    0.427754] x9 : 0000000000000000 x8 : ffff8413eb6ca668 
>> [    0.433118] x7 : ffff8413eb6ca690 x6 : 0000000000000000 
>> [    0.438482] x5 : fffffffffffffffe x4 : 0000000000000000 
>> [    0.443845] x3 : 0000000000000040 x2 : 0000000000000041 
>> [    0.449209] x1 : 0000000000000000 x0 : 0000000000000001 
>> [    0.454573] 
>> [    0.456069] ---[ end trace b58e70f3295a8cd7 ]---
>> [    0.460730] Call trace:
>> [    0.463193] Exception stack(0xffff8013e9dcfc10 to 0xffff8013e9dcfd40)
>> [    0.469699] fc00:                                   0000000100000000 0001000000000000
>> [    0.477611] fc20: ffff8013e9dcfde0 ffff00000838c124 ffff000008d72228 ffff8013e9dcff70
>> [    0.485524] fc40: ffff000008d72608 ffff000008ab02a4 0000000000000000 0000000000000000
>> [    0.493436] fc60: 0000000000000000 3464313430303030 0000000000000000 0000000000000000
>> [    0.501348] fc80: ffff8013e9dcfc90 ffff00000836e678 ffff8013e9dcfca0 ffff00000836e910
>> [    0.509259] fca0: ffff8013e9dcfd30 ffff00000836ec10 0000000000000001 0000000000000000
>> [    0.517171] fcc0: 0000000000000041 0000000000000040 0000000000000000 fffffffffffffffe
>> [    0.525083] fce0: 0000000000000000 ffff8413eb6ca690 ffff8413eb6ca668 0000000000000000
>> [    0.532995] fd00: 0000000002e92aa7 0000000000000013 0000000000000013 0000000000000000
>> [    0.540907] fd20: 0000000000000013 ffff000008ca8000 0000000003010066 0000000000000000
>> [    0.548819] [<ffff00000838c124>] gic_raise_softirq+0x128/0x17c
>> [    0.554713] [<ffff00000808e1f4>] smp_send_reschedule+0x34/0x3c
>> [    0.560605] [<ffff0000080ddf18>] resched_curr+0x40/0x5c
>> [    0.565881] [<ffff0000080de650>] check_preempt_curr+0x58/0xa0
>> [    0.571685] [<ffff0000080de6b0>] ttwu_do_wakeup+0x18/0x80
>> [    0.577136] [<ffff0000080de790>] ttwu_do_activate+0x78/0x88
>> [    0.582763] [<ffff0000080df5cc>] try_to_wake_up+0x1f8/0x300
>> [    0.588390] [<ffff0000080df79c>] default_wake_function+0x10/0x18
>> [    0.594458] [<ffff0000080f3210>] __wake_up_common+0x5c/0x9c
>> [    0.600085] [<ffff0000080f3264>] __wake_up_locked+0x14/0x1c
>> [    0.605712] [<ffff0000080f3e10>] complete+0x40/0x5c
>> [    0.610635] [<ffff00000808dba8>] secondary_start_kernel+0x148/0x1a8
>> [    0.616965] [<00000000000831a8>] 0x831a8
> .
>

^ permalink raw reply

* [RFC] Arm64 boot fail with numa enable in BIOS
From: Yisheng Xie @ 2016-09-20  3:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160919140709.GA17464@leverpostej>



On 2016/9/19 22:07, Mark Rutland wrote:
> [adding LAKML, arm64 maintainers]
> 
> On Mon, Sep 19, 2016 at 09:05:26PM +0800, Yisheng Xie wrote:
>> hi all,
> 
> Can you modify the warning in cpumask.h to dump the bad CPU number? That
> would make it fairly clear if that's the case.
> 
hi Mark,
I dump the bad CPU number, it is 64,
And the cpumask get from task is 00000000,00000000.

[    3.873044] select_task_rq: allowed 0, allow_cpumask 00000000,00000000
[    3.879727] cpumask_check: cpu 64, nr_cpumask_bits:64, nr_cpu_ids= 64
[    3.895989] ------------[ cut here ]------------
[    3.900652] WARNING: CPU: 16 PID: 103 at ./include/linux/cpumask.h:122 try_to_wake_up+0x410/0x4ac

Thanks.
Yisheng Xie

> Thanks,
> Mark.
> 
>> [    0.297337] Detected PIPT I-cache on CPU1
>> [    0.297347] GICv3: CPU1: found redistributor 10001 region 1:0x000000004d140000
>> [    0.297356] CPU1: Booted secondary processor [410fd082]
>> [    0.297375] ------------[ cut here ]------------
>> [    0.320390] WARNING: CPU: 1 PID: 0 at ./include/linux/cpumask.h:121 gic_raise_softirq+0x128/0x17c
>> [    0.329356] Modules linked in:
>> [    0.332434] 
>> [    0.333932] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0-rc4-00163-g803ea3a #21
>> [    0.341581] Hardware name: Hisilicon Hi1616 Evaluation Board (DT)
>> [    0.347735] task: ffff8013e9dd0000 task.stack: ffff8013e9dcc000
>> [    0.353714] PC is at gic_raise_softirq+0x128/0x17c
>> [    0.358550] LR is at gic_raise_softirq+0xa0/0x17c
>> [    0.363298] pc : [<ffff00000838c124>] lr : [<ffff00000838c09c>] pstate: 200001c5
>> [    0.370770] sp : ffff8013e9dcfde0
>> [    0.374112] x29: ffff8013e9dcfde0 x28: 0000000000000000 
>> [    0.379476] x27: 000000000083207c x26: ffff000008ca5d70 
>> [    0.384841] x25: 0000000100000001 x24: ffff000008d63ff3 
>> [    0.390205] x23: 0000000000000000 x22: ffff000008cb0000 
>> [    0.395569] x21: ffff00000884edb0 x20: 0000000000000001 
>> [    0.400933] x19: 0000000100000000 x18: 0000000000000000 
>> [    0.406298] x17: 0000000000000000 x16: 0000000003010066 
>> [    0.411661] x15: ffff000008ca8000 x14: 0000000000000013 
>> [    0.417025] x13: 0000000000000000 x12: 0000000000000013 
>> [    0.422389] x11: 0000000000000013 x10: 0000000002e92aa7 
>> [    0.427754] x9 : 0000000000000000 x8 : ffff8413eb6ca668 
>> [    0.433118] x7 : ffff8413eb6ca690 x6 : 0000000000000000 
>> [    0.438482] x5 : fffffffffffffffe x4 : 0000000000000000 
>> [    0.443845] x3 : 0000000000000040 x2 : 0000000000000041 
>> [    0.449209] x1 : 0000000000000000 x0 : 0000000000000001 
>> [    0.454573] 
>> [    0.456069] ---[ end trace b58e70f3295a8cd7 ]---
>> [    0.460730] Call trace:
>> [    0.463193] Exception stack(0xffff8013e9dcfc10 to 0xffff8013e9dcfd40)
>> [    0.469699] fc00:                                   0000000100000000 0001000000000000
>> [    0.477611] fc20: ffff8013e9dcfde0 ffff00000838c124 ffff000008d72228 ffff8013e9dcff70
>> [    0.485524] fc40: ffff000008d72608 ffff000008ab02a4 0000000000000000 0000000000000000
>> [    0.493436] fc60: 0000000000000000 3464313430303030 0000000000000000 0000000000000000
>> [    0.501348] fc80: ffff8013e9dcfc90 ffff00000836e678 ffff8013e9dcfca0 ffff00000836e910
>> [    0.509259] fca0: ffff8013e9dcfd30 ffff00000836ec10 0000000000000001 0000000000000000
>> [    0.517171] fcc0: 0000000000000041 0000000000000040 0000000000000000 fffffffffffffffe
>> [    0.525083] fce0: 0000000000000000 ffff8413eb6ca690 ffff8413eb6ca668 0000000000000000
>> [    0.532995] fd00: 0000000002e92aa7 0000000000000013 0000000000000013 0000000000000000
>> [    0.540907] fd20: 0000000000000013 ffff000008ca8000 0000000003010066 0000000000000000
>> [    0.548819] [<ffff00000838c124>] gic_raise_softirq+0x128/0x17c
>> [    0.554713] [<ffff00000808e1f4>] smp_send_reschedule+0x34/0x3c
>> [    0.560605] [<ffff0000080ddf18>] resched_curr+0x40/0x5c
>> [    0.565881] [<ffff0000080de650>] check_preempt_curr+0x58/0xa0
>> [    0.571685] [<ffff0000080de6b0>] ttwu_do_wakeup+0x18/0x80
>> [    0.577136] [<ffff0000080de790>] ttwu_do_activate+0x78/0x88
>> [    0.582763] [<ffff0000080df5cc>] try_to_wake_up+0x1f8/0x300
>> [    0.588390] [<ffff0000080df79c>] default_wake_function+0x10/0x18
>> [    0.594458] [<ffff0000080f3210>] __wake_up_common+0x5c/0x9c
>> [    0.600085] [<ffff0000080f3264>] __wake_up_locked+0x14/0x1c
>> [    0.605712] [<ffff0000080f3e10>] complete+0x40/0x5c
>> [    0.610635] [<ffff00000808dba8>] secondary_start_kernel+0x148/0x1a8
>> [    0.616965] [<00000000000831a8>] 0x831a8
> 
> .
> 

^ permalink raw reply

* [PATCH v7 0/8] power: add power sequence library
From: Peter Chen @ 2016-09-20  3:36 UTC (permalink / raw)
  To: linux-arm-kernel

Hi all,

This is a follow-up for my last power sequence framework patch set [1].
According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
power sequence instances will be added at postcore_initcall, the match
criteria is compatible string first, if the compatible string is not
matched between dts and library, it will try to use generic power sequence.
	 
The host driver just needs to call of_pwrseq_on/of_pwrseq_off
if only one power sequence instance is needed, for more power sequences
are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).

In future, if there are special power sequence requirements, the special
power sequence library can be created.

This patch set is tested on i.mx6 sabresx evk using a dts change, I use
two hot-plug devices to simulate this use case, the related binding
change is updated at patch [1/6], The udoo board changes were tested
using my last power sequence patch set.[3]

Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
need to power on itself before it can be found by ULPI bus.

[1] http://www.spinics.net/lists/linux-usb/msg142755.html
[2] http://www.spinics.net/lists/linux-usb/msg143106.html
[3] http://www.spinics.net/lists/linux-usb/msg142815.html

Changes for v7:
- Create kinds of power sequence instance at postcore_initcall, and match
  the instance with node using compatible string, the beneit of this is
  the host driver doesn't need to consider which pwrseq instance needs
  to be used, and pwrseq core will match it, however, it eats some memories
  if less power sequence instances are used. [Patch 2/8]
- Add pwrseq_compatible_sample.c to test match pwrseq using device_id. [Patch 2/8]
- Fix the comments Vaibhav Hiremath adds for error path for clock and do not
  use device_node for parameters at pwrseq_on. [Patch 2/8]
- Simplify the caller to use power sequence, follows Alan's commnets [Patch 4/8]
- Tested three pwrseq instances together using both specific compatible string and
  generic libraries.

Changes for v6:
- Add Matthias Kaehlcke's Reviewed-by and Tested-by. (patch [2/6])
- Change chipidea core of_node assignment for coming user. (patch [5/6])
- Applies Joshua Clayton's three dts changes for two boards,
  the USB device's reg has only #address-cells, but without #size-cells.

Changes for v5:
- Delete pwrseq_register/pwrseq_unregister, which is useless currently
- Fix the linker error when the pwrseq user is compiled as module

Changes for v4:
- Create the patch on next-20160722 
- Fix the of_node is not NULL after chipidea driver is unbinded [Patch 5/6]
- Using more friendly wait method for reset gpio [Patch 2/6]
- Support multiple input clocks [Patch 2/6]
- Add Rob Herring's ack for DT changes
- Add Joshua Clayton's Tested-by

Changes for v3:
- Delete "power-sequence" property at binding-doc, and change related code
  at both library and user code.
- Change binding-doc example node name with Rob's comments
- of_get_named_gpio_flags only gets the gpio, but without setting gpio flags,
  add additional code request gpio with proper gpio flags
- Add Philipp Zabel's Ack and MAINTAINER's entry

Changes for v2:
- Delete "pwrseq" prefix and clock-names for properties at dt binding
- Should use structure not but its pointer for kzalloc
- Since chipidea core has no of_node, let core's of_node equals glue
  layer's at core's probe

Joshua Clayton (2):
  ARM: dts: imx6qdl: Enable usb node children with <reg>
  ARM: dts: imx6q-evi: Fix onboard hub reset line

Peter Chen (6):
  binding-doc: power: pwrseq-generic: add binding doc for generic power
    sequence library
  power: add power sequence library
  binding-doc: usb: usb-device: add optional properties for power
    sequence
  usb: core: add power sequence handling for USB devices
  usb: chipidea: let chipidea core device of_node equal's glue layer
    device of_node
  ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property

 .../bindings/power/pwrseq/pwrseq-generic.txt       |  48 ++++++
 .../devicetree/bindings/usb/usb-device.txt         |  10 +-
 MAINTAINERS                                        |   9 +
 arch/arm/boot/dts/imx6q-evi.dts                    |  25 +--
 arch/arm/boot/dts/imx6qdl-udoo.dtsi                |  26 ++-
 arch/arm/boot/dts/imx6qdl.dtsi                     |   6 +
 drivers/power/Kconfig                              |   1 +
 drivers/power/Makefile                             |   1 +
 drivers/power/pwrseq/Kconfig                       |  45 +++++
 drivers/power/pwrseq/Makefile                      |   3 +
 drivers/power/pwrseq/core.c                        | 190 +++++++++++++++++++++
 drivers/power/pwrseq/pwrseq_compatible_sample.c    | 178 +++++++++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c              | 177 +++++++++++++++++++
 drivers/usb/chipidea/core.c                        |  27 ++-
 drivers/usb/core/hub.c                             |  41 ++++-
 drivers/usb/core/hub.h                             |   1 +
 include/linux/power/pwrseq.h                       |  73 ++++++++
 17 files changed, 820 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 create mode 100644 drivers/power/pwrseq/core.c
 create mode 100644 drivers/power/pwrseq/pwrseq_compatible_sample.c
 create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
 create mode 100644 include/linux/power/pwrseq.h

-- 
2.7.4

^ permalink raw reply

* [PATCH v7 1/8] binding-doc: power: pwrseq-generic: add binding doc for generic power sequence library
From: Peter Chen @ 2016-09-20  3:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474342607-27512-1-git-send-email-peter.chen@nxp.com>

Add binding doc for generic power sequence library.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/power/pwrseq/pwrseq-generic.txt       | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt

diff --git a/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
new file mode 100644
index 0000000..ebf0d47
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt
@@ -0,0 +1,48 @@
+The generic power sequence library
+
+Some hard-wired devices (eg USB/MMC) need to do power sequence before
+the device can be enumerated on the bus, the typical power sequence
+like: enable USB PHY clock, toggle reset pin, etc. But current
+Linux device driver lacks of such code to do it, it may cause some
+hard-wired devices works abnormal or can't be recognized by
+controller at all. The power sequence will be done before this device
+can be found at the bus.
+
+The power sequence properties is under the device node.
+
+Optional properties:
+- clocks: the input clocks for device.
+- reset-gpios: Should specify the GPIO for reset.
+- reset-duration-us: the duration in microsecond for assert reset signal.
+
+Below is the example of USB power sequence properties on USB device
+nodes which have two level USB hubs.
+
+&usbotg1 {
+	vbus-supply = <&reg_usb_otg1_vbus>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_usb_otg1_id>;
+	status = "okay";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	genesys: hub at 1 {
+		compatible = "usb5e3,608";
+		reg = <1>;
+
+		clocks = <&clks IMX6SX_CLK_CKO>;
+		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+		reset-duration-us = <10>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+		asix: ethernet at 1 {
+			compatible = "usbb95,1708";
+			reg = <1>;
+
+			clocks = <&clks IMX6SX_CLK_IPG>;
+			reset-gpios = <&gpio4 6 GPIO_ACTIVE_LOW>; /* ethernet_rst */
+			reset-duration-us = <15>;
+		};
+	};
+};
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 2/8] power: add power sequence library
From: Peter Chen @ 2016-09-20  3:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474342607-27512-1-git-send-email-peter.chen@nxp.com>

We have an well-known problem that the device needs to do some power
sequence before it can be recognized by related host, the typical
example like hard-wired mmc devices and usb devices.

This power sequence is hard to be described at device tree and handled by
related host driver, so we have created a common power sequence
library to cover this requirement. The core code has supplied
some common helpers for host driver, and individual power sequence
libraries handle kinds of power sequence for devices.

pwrseq_generic is intended for general purpose of power sequence, which
handles gpios and clocks currently, and can cover regulator and pinctrl
in future. The host driver just needs to call of_pwrseq_on/of_pwrseq_off
if only one power sequence is needed, else call of_pwrseq_on_list
/of_pwrseq_off_list instead (eg, USB hub driver).

Signed-off-by: Peter Chen <peter.chen@nxp.com>
Tested-by Joshua Clayton <stillcompiling@gmail.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 MAINTAINERS                                     |   9 ++
 drivers/power/Kconfig                           |   1 +
 drivers/power/Makefile                          |   1 +
 drivers/power/pwrseq/Kconfig                    |  45 ++++++
 drivers/power/pwrseq/Makefile                   |   3 +
 drivers/power/pwrseq/core.c                     | 190 ++++++++++++++++++++++++
 drivers/power/pwrseq/pwrseq_compatible_sample.c | 178 ++++++++++++++++++++++
 drivers/power/pwrseq/pwrseq_generic.c           | 177 ++++++++++++++++++++++
 include/linux/power/pwrseq.h                    |  73 +++++++++
 9 files changed, 677 insertions(+)
 create mode 100644 drivers/power/pwrseq/Kconfig
 create mode 100644 drivers/power/pwrseq/Makefile
 create mode 100644 drivers/power/pwrseq/core.c
 create mode 100644 drivers/power/pwrseq/pwrseq_compatible_sample.c
 create mode 100644 drivers/power/pwrseq/pwrseq_generic.c
 create mode 100644 include/linux/power/pwrseq.h

diff --git a/MAINTAINERS b/MAINTAINERS
index b3e9395..b353769 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9343,6 +9343,15 @@ F:	include/linux/pm_*
 F:	include/linux/powercap.h
 F:	drivers/powercap/
 
+POWER SEQUENCE LIBRARY
+M:	Peter Chen <Peter.Chen@nxp.com>
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git
+L:	linux-pm at vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/power/pwrseq/
+F:	drivers/power/pwrseq/
+F:	include/linux/power/pwrseq.h/
+
 POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
 M:	Sebastian Reichel <sre@kernel.org>
 M:	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index acd4a15..f6aa4fd 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -515,3 +515,4 @@ endif # POWER_SUPPLY
 
 source "drivers/power/reset/Kconfig"
 source "drivers/power/avs/Kconfig"
+source "drivers/power/pwrseq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e46b75d..4ed2e12 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65217)	+= tps65217_charger.o
 obj-$(CONFIG_POWER_RESET)	+= reset/
 obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
 obj-$(CONFIG_AXP288_CHARGER)	+= axp288_charger.o
+obj-$(CONFIG_POWER_SEQUENCE)	+= pwrseq/
diff --git a/drivers/power/pwrseq/Kconfig b/drivers/power/pwrseq/Kconfig
new file mode 100644
index 0000000..dff5e35
--- /dev/null
+++ b/drivers/power/pwrseq/Kconfig
@@ -0,0 +1,45 @@
+#
+# Power Sequence library
+#
+
+config POWER_SEQUENCE
+	bool
+
+menu "Power Sequence Support"
+
+config PWRSEQ_GENERIC
+	bool "Generic power sequence control"
+	depends on OF
+	select POWER_SEQUENCE
+	help
+	   It is used for drivers which needs to do power sequence
+	   (eg, turn on clock, toggle reset gpio) before the related
+	   devices can be found by hardware. This generic one can be
+	   used for common power sequence control.
+
+config PWRSEQ_GENERIC_INSTANCE_NUMBER
+	int "Number of Generic Power Sequence Instance"
+	depends on PWRSEQ_GENERIC
+	range 1 10
+	default 2
+	help
+	   Usually, there are not so many devices needs power sequence, we set two
+	   as default value.
+
+config PWRSEQ_SAMPLE
+	bool "sample power sequence control using compatible string"
+	depends on OF
+	select POWER_SEQUENCE
+	help
+	   It is a sample library which implements power sequence for device id,
+	   it is an example purpose.
+
+config PWRSEQ_SAMPLE_INSTANCE_NUMBER
+	int "Number of Sample Power Sequence Instance"
+	depends on PWRSEQ_SAMPLE
+	range 1 5
+	default 1
+	help
+	   Usually, this file is special for certain device, so the default for this number
+	   is 1.
+endmenu
diff --git a/drivers/power/pwrseq/Makefile b/drivers/power/pwrseq/Makefile
new file mode 100644
index 0000000..62f3cbf
--- /dev/null
+++ b/drivers/power/pwrseq/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_POWER_SEQUENCE) += core.o
+obj-$(CONFIG_PWRSEQ_GENERIC) += pwrseq_generic.o
+obj-$(CONFIG_PWRSEQ_SAMPLE) += pwrseq_compatible_sample.o
diff --git a/drivers/power/pwrseq/core.c b/drivers/power/pwrseq/core.c
new file mode 100644
index 0000000..6beae20
--- /dev/null
+++ b/drivers/power/pwrseq/core.c
@@ -0,0 +1,190 @@
+/*
+ * core.c	power sequence core file
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/power/pwrseq.h>
+
+static DEFINE_MUTEX(pwrseq_list_mutex);
+static LIST_HEAD(pwrseq_list);
+
+int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	if (p && p->get)
+		return p->get(np, p);
+
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(pwrseq_get);
+
+int pwrseq_on(struct pwrseq *p)
+{
+	if (p && p->on)
+		return p->on(p);
+
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(pwrseq_on);
+
+void pwrseq_off(struct pwrseq *p)
+{
+	if (p && p->off)
+		p->off(p);
+}
+EXPORT_SYMBOL_GPL(pwrseq_off);
+
+void pwrseq_put(struct pwrseq *p)
+{
+	if (p && p->put)
+		p->put(p);
+}
+EXPORT_SYMBOL_GPL(pwrseq_put);
+
+void pwrseq_free(struct pwrseq *p)
+{
+	if (p && p->free)
+		p->free(p);
+}
+EXPORT_SYMBOL_GPL(pwrseq_free);
+
+int pwrseq_suspend(struct pwrseq *p)
+{
+	if (p && p->suspend)
+		return p->suspend(p);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pwrseq_suspend);
+
+int pwrseq_resume(struct pwrseq *p)
+{
+	if (p && p->resume)
+		return p->resume(p);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pwrseq_resume);
+
+void pwrseq_register(struct pwrseq *pwrseq)
+{
+	mutex_lock(&pwrseq_list_mutex);
+	list_add(&pwrseq->node, &pwrseq_list);
+	mutex_unlock(&pwrseq_list_mutex);
+}
+EXPORT_SYMBOL_GPL(pwrseq_register);
+
+static struct pwrseq *pwrseq_find_available_instance(struct device_node *np)
+{
+	struct pwrseq *pwrseq;
+
+	list_for_each_entry(pwrseq, &pwrseq_list, node) {
+		if (pwrseq->used)
+			continue;
+
+		/* compare compatible string for pwrseq node */
+		if (of_match_node(pwrseq->pwrseq_of_match_table, np)) {
+			pwrseq->used = true;
+			return pwrseq;
+		}
+
+		/* return generic pwrseq instance */
+		if (!strcmp(pwrseq->pwrseq_of_match_table->compatible,
+				"generic")) {
+			pr_debug("using generic pwrseq instance for %s\n",
+				np->full_name);
+			pwrseq->used = true;
+			return pwrseq;
+		}
+	}
+	pr_warn("Can't find any pwrseq instances for %s\n", np->full_name);
+
+	return NULL;
+}
+
+struct pwrseq *of_pwrseq_on(struct device_node *np)
+{
+	struct pwrseq *pwrseq;
+	int ret;
+
+	pwrseq = pwrseq_find_available_instance(np);
+	if (!pwrseq)
+		return ERR_PTR(-ENONET);
+
+	ret = pwrseq_get(np, pwrseq);
+	if (ret)
+		goto pwr_free;
+
+	ret = pwrseq_on(pwrseq);
+	if (ret)
+		goto pwr_put;
+
+	return pwrseq;
+
+pwr_put:
+	pwrseq_put(pwrseq);
+pwr_free:
+	pwrseq_free(pwrseq);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_on);
+
+void of_pwrseq_off(struct pwrseq *pwrseq)
+{
+	pwrseq_off(pwrseq);
+	pwrseq_put(pwrseq);
+	pwrseq_free(pwrseq);
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_off);
+
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
+{
+	struct pwrseq *pwrseq;
+	struct pwrseq_list_per_dev *pwrseq_list_node;
+
+	pwrseq = of_pwrseq_on(np);
+	if (IS_ERR(pwrseq))
+		return PTR_ERR(pwrseq);
+
+	pwrseq_list_node = kzalloc(sizeof(*pwrseq_list_node), GFP_KERNEL);
+	if (!pwrseq_list_node) {
+		of_pwrseq_off(pwrseq);
+		return -ENOMEM;
+	}
+	pwrseq_list_node->pwrseq = pwrseq;
+	list_add(&pwrseq_list_node->list, head);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_on_list);
+
+void of_pwrseq_off_list(struct list_head *head)
+{
+	struct pwrseq *pwrseq;
+	struct pwrseq_list_per_dev *pwrseq_list_node, *tmp_node;
+
+	list_for_each_entry_safe(pwrseq_list_node, tmp_node, head, list) {
+		pwrseq = pwrseq_list_node->pwrseq;
+		of_pwrseq_off(pwrseq);
+		list_del(&pwrseq_list_node->list);
+		kfree(pwrseq_list_node);
+	}
+}
+EXPORT_SYMBOL_GPL(of_pwrseq_off_list);
diff --git a/drivers/power/pwrseq/pwrseq_compatible_sample.c b/drivers/power/pwrseq/pwrseq_compatible_sample.c
new file mode 100644
index 0000000..d6bcc6d
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_compatible_sample.c
@@ -0,0 +1,178 @@
+/*
+ * pwrseq_compatible_sample.c	Sample power sequence handling for compatible
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/slab.h>
+
+#include <linux/power/pwrseq.h>
+
+struct pwrseq_sample {
+	struct pwrseq pwrseq;
+	struct gpio_desc *gpiod_reset;
+	struct clk *clks[PWRSEQ_MAX_CLKS];
+	u32 duration_us;
+};
+
+#define to_generic_pwrseq(p) container_of(p, struct pwrseq_sample, pwrseq)
+
+static void pwrseq_sample_free(struct pwrseq *pwrseq)
+{
+	pwrseq->used = false;
+}
+
+static void pwrseq_sample_put(struct pwrseq *pwrseq)
+{
+	struct pwrseq_sample *pwrseq_sam = to_generic_pwrseq(pwrseq);
+	int clk;
+
+	if (pwrseq_sam->gpiod_reset)
+		gpiod_put(pwrseq_sam->gpiod_reset);
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++)
+		clk_put(pwrseq_sam->clks[clk]);
+}
+
+static void pwrseq_sample_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_sample *pwrseq_sam = to_generic_pwrseq(pwrseq);
+	int clk;
+
+	for (clk = PWRSEQ_MAX_CLKS - 1; clk >= 0; clk--)
+		clk_disable_unprepare(pwrseq_sam->clks[clk]);
+}
+
+static int pwrseq_sample_on(struct pwrseq *pwrseq)
+{
+	struct pwrseq_sample *pwrseq_sam = to_generic_pwrseq(pwrseq);
+	int clk, ret = 0;
+	struct gpio_desc *gpiod_reset = pwrseq_sam->gpiod_reset;
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS && pwrseq_sam->clks[clk]; clk++) {
+		ret = clk_prepare_enable(pwrseq_sam->clks[clk]);
+		if (ret) {
+			pr_err("Can't enable clock, ret=%d\n", ret);
+			goto err_disable_clks;
+		}
+	}
+
+	if (gpiod_reset) {
+		u32 duration_us = pwrseq_sam->duration_us;
+
+		if (duration_us <= 10)
+			udelay(10);
+		else
+			usleep_range(duration_us, duration_us + 100);
+		gpiod_set_value(gpiod_reset, 0);
+	}
+
+	return ret;
+
+err_disable_clks:
+	while (--clk >= 0)
+		clk_disable_unprepare(pwrseq_sam->clks[clk]);
+
+	return ret;
+}
+
+static int pwrseq_sample_get(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_sample *pwrseq_sam = to_generic_pwrseq(pwrseq);
+	enum of_gpio_flags flags;
+	int reset_gpio, clk, ret = 0;
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++) {
+		pwrseq_sam->clks[clk] = of_clk_get(np, clk);
+		if (IS_ERR(pwrseq_sam->clks[clk])) {
+			ret = PTR_ERR(pwrseq_sam->clks[clk]);
+			if (ret != -ENOENT)
+				goto err_put_clks;
+			pwrseq_sam->clks[clk] = NULL;
+			break;
+		}
+	}
+
+	reset_gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, &flags);
+	if (gpio_is_valid(reset_gpio)) {
+		unsigned long gpio_flags;
+
+		if (flags & OF_GPIO_ACTIVE_LOW)
+			gpio_flags = GPIOF_ACTIVE_LOW | GPIOF_OUT_INIT_LOW;
+		else
+			gpio_flags = GPIOF_OUT_INIT_HIGH;
+
+		ret = gpio_request_one(reset_gpio, gpio_flags,
+				"pwrseq-reset-gpios");
+		if (ret)
+			goto err_put_clks;
+
+		pwrseq_sam->gpiod_reset = gpio_to_desc(reset_gpio);
+		of_property_read_u32(np, "reset-duration-us",
+				&pwrseq_sam->duration_us);
+	} else {
+		if (reset_gpio == -ENOENT)
+			return 0;
+
+		ret = reset_gpio;
+		pr_err("Failed to get reset gpio on %s, err = %d\n",
+				np->full_name, reset_gpio);
+		goto err_put_clks;
+	}
+
+	return ret;
+
+err_put_clks:
+	while (--clk >= 0)
+		clk_put(pwrseq_sam->clks[clk]);
+	return ret;
+}
+
+static const struct of_device_id sample_id_table[] = {
+	{ .compatible = "usb5e3,608",},
+	{ .compatible = "usbb95,1708",},
+	{ /* sentinel */ }
+};
+
+static int __init pwrseq_compatible_sample_register(void)
+{
+	struct pwrseq_sample *pwrseq_sam;
+	int i;
+
+	for (i = 0; i < CONFIG_PWRSEQ_SAMPLE_INSTANCE_NUMBER; i++) {
+		pwrseq_sam = kzalloc(sizeof(*pwrseq_sam), GFP_KERNEL);
+		if (!pwrseq_sam)
+			return -ENOMEM;
+
+		pwrseq_sam->pwrseq.pwrseq_of_match_table = sample_id_table;
+		pwrseq_sam->pwrseq.get = pwrseq_sample_get;
+		pwrseq_sam->pwrseq.on = pwrseq_sample_on;
+		pwrseq_sam->pwrseq.off = pwrseq_sample_off;
+		pwrseq_sam->pwrseq.put = pwrseq_sample_put;
+		pwrseq_sam->pwrseq.free = pwrseq_sample_free;
+
+		pwrseq_register(&pwrseq_sam->pwrseq);
+	}
+
+	return 0;
+}
+postcore_initcall(pwrseq_compatible_sample_register)
diff --git a/drivers/power/pwrseq/pwrseq_generic.c b/drivers/power/pwrseq/pwrseq_generic.c
new file mode 100644
index 0000000..bcd16c3
--- /dev/null
+++ b/drivers/power/pwrseq/pwrseq_generic.c
@@ -0,0 +1,177 @@
+/*
+ * pwrseq_generic.c	Generic power sequence handling
+ *
+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
+ * Author: Peter Chen <peter.chen@nxp.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/slab.h>
+
+#include <linux/power/pwrseq.h>
+
+struct pwrseq_generic {
+	struct pwrseq pwrseq;
+	struct gpio_desc *gpiod_reset;
+	struct clk *clks[PWRSEQ_MAX_CLKS];
+	u32 duration_us;
+};
+
+#define to_generic_pwrseq(p) container_of(p, struct pwrseq_generic, pwrseq)
+
+static void pwrseq_generic_free(struct pwrseq *pwrseq)
+{
+	pwrseq->used = false;
+}
+
+static void pwrseq_generic_put(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int clk;
+
+	if (pwrseq_gen->gpiod_reset)
+		gpiod_put(pwrseq_gen->gpiod_reset);
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++)
+		clk_put(pwrseq_gen->clks[clk]);
+}
+
+static void pwrseq_generic_off(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int clk;
+
+	for (clk = PWRSEQ_MAX_CLKS - 1; clk >= 0; clk--)
+		clk_disable_unprepare(pwrseq_gen->clks[clk]);
+}
+
+static int pwrseq_generic_on(struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	int clk, ret = 0;
+	struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS && pwrseq_gen->clks[clk]; clk++) {
+		ret = clk_prepare_enable(pwrseq_gen->clks[clk]);
+		if (ret) {
+			pr_err("Can't enable clock, ret=%d\n", ret);
+			goto err_disable_clks;
+		}
+	}
+
+	if (gpiod_reset) {
+		u32 duration_us = pwrseq_gen->duration_us;
+
+		if (duration_us <= 10)
+			udelay(10);
+		else
+			usleep_range(duration_us, duration_us + 100);
+		gpiod_set_value(gpiod_reset, 0);
+	}
+
+	return ret;
+
+err_disable_clks:
+	while (--clk >= 0)
+		clk_disable_unprepare(pwrseq_gen->clks[clk]);
+
+	return ret;
+}
+
+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
+{
+	struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
+	enum of_gpio_flags flags;
+	int reset_gpio, clk, ret = 0;
+
+	for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++) {
+		pwrseq_gen->clks[clk] = of_clk_get(np, clk);
+		if (IS_ERR(pwrseq_gen->clks[clk])) {
+			ret = PTR_ERR(pwrseq_gen->clks[clk]);
+			if (ret != -ENOENT)
+				goto err_put_clks;
+			pwrseq_gen->clks[clk] = NULL;
+			break;
+		}
+	}
+
+	reset_gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, &flags);
+	if (gpio_is_valid(reset_gpio)) {
+		unsigned long gpio_flags;
+
+		if (flags & OF_GPIO_ACTIVE_LOW)
+			gpio_flags = GPIOF_ACTIVE_LOW | GPIOF_OUT_INIT_LOW;
+		else
+			gpio_flags = GPIOF_OUT_INIT_HIGH;
+
+		ret = gpio_request_one(reset_gpio, gpio_flags,
+				"pwrseq-reset-gpios");
+		if (ret)
+			goto err_put_clks;
+
+		pwrseq_gen->gpiod_reset = gpio_to_desc(reset_gpio);
+		of_property_read_u32(np, "reset-duration-us",
+				&pwrseq_gen->duration_us);
+	} else {
+		if (reset_gpio == -ENOENT)
+			return 0;
+
+		ret = reset_gpio;
+		pr_err("Failed to get reset gpio on %s, err = %d\n",
+				np->full_name, reset_gpio);
+		goto err_put_clks;
+	}
+
+	return ret;
+
+err_put_clks:
+	while (--clk >= 0)
+		clk_put(pwrseq_gen->clks[clk]);
+	return ret;
+}
+
+static const struct of_device_id generic_id_table[] = {
+	{ .compatible = "generic",},
+	{ /* sentinel */ }
+};
+
+static int __init pwrseq_generic_register(void)
+{
+	struct pwrseq_generic *pwrseq_gen;
+	int i;
+
+	for (i = 0; i < CONFIG_PWRSEQ_GENERIC_INSTANCE_NUMBER; i++) {
+		pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
+		if (!pwrseq_gen)
+			return -ENOMEM;
+
+		pwrseq_gen->pwrseq.pwrseq_of_match_table = generic_id_table;
+		pwrseq_gen->pwrseq.get = pwrseq_generic_get;
+		pwrseq_gen->pwrseq.on = pwrseq_generic_on;
+		pwrseq_gen->pwrseq.off = pwrseq_generic_off;
+		pwrseq_gen->pwrseq.put = pwrseq_generic_put;
+		pwrseq_gen->pwrseq.free = pwrseq_generic_free;
+
+		pwrseq_register(&pwrseq_gen->pwrseq);
+	}
+
+	return 0;
+}
+postcore_initcall(pwrseq_generic_register)
diff --git a/include/linux/power/pwrseq.h b/include/linux/power/pwrseq.h
new file mode 100644
index 0000000..ae4753f
--- /dev/null
+++ b/include/linux/power/pwrseq.h
@@ -0,0 +1,73 @@
+#ifndef __LINUX_PWRSEQ_H
+#define __LINUX_PWRSEQ_H
+
+#include <linux/of.h>
+
+#define PWRSEQ_MAX_CLKS		3
+
+struct pwrseq {
+	const struct of_device_id *pwrseq_of_match_table;
+	struct list_head node;
+	int (*get)(struct device_node *np, struct pwrseq *p);
+	int (*on)(struct pwrseq *p);
+	void (*off)(struct pwrseq *p);
+	void (*put)(struct pwrseq *p);
+	void (*free)(struct pwrseq *p);
+	int (*suspend)(struct pwrseq *p);
+	int (*resume)(struct pwrseq *p);
+	bool used;
+};
+
+/* used for power sequence instance list in one driver */
+struct pwrseq_list_per_dev {
+	struct pwrseq *pwrseq;
+	struct list_head list;
+};
+
+#if IS_ENABLED(CONFIG_POWER_SEQUENCE)
+int pwrseq_get(struct device_node *np, struct pwrseq *p);
+int pwrseq_on(struct pwrseq *p);
+void pwrseq_off(struct pwrseq *p);
+void pwrseq_put(struct pwrseq *p);
+void pwrseq_free(struct pwrseq *p);
+int pwrseq_suspend(struct pwrseq *p);
+int pwrseq_resume(struct pwrseq *p);
+void pwrseq_register(struct pwrseq *pwrseq);
+struct pwrseq *of_pwrseq_on(struct device_node *np);
+void of_pwrseq_off(struct pwrseq *pwrseq);
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head);
+void of_pwrseq_off_list(struct list_head *head);
+#else
+static inline int pwrseq_get(struct device_node *np, struct pwrseq *p)
+{
+	return 0;
+}
+static inline int pwrseq_on(struct pwrseq *p)
+{
+	return 0;
+}
+static inline void pwrseq_off(struct pwrseq *p) {}
+static inline void pwrseq_put(struct pwrseq *p) {}
+static inline void pwrseq_free(struct pwrseq *p) {}
+static inline int pwrseq_suspend(struct pwrseq *p)
+{
+	return 0;
+}
+static inline int pwrseq_resume(struct pwrseq *p)
+{
+	return 0;
+}
+static inline void pwrseq_register(struct pwrseq *pwrseq) {}
+static inline struct pwrseq *of_pwrseq_on(struct device_node *np)
+{
+	return NULL;
+}
+void of_pwrseq_off(struct pwrseq *pwrseq) {}
+int of_pwrseq_on_list(struct device_node *np, struct list_head *head)
+{
+	return 0;
+}
+void of_pwrseq_off_list(struct list_head *head) {}
+#endif /* CONFIG_POWER_SEQUENCE */
+
+#endif  /* __LINUX_PWRSEQ_H */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 3/8] binding-doc: usb: usb-device: add optional properties for power sequence
From: Peter Chen @ 2016-09-20  3:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474342607-27512-1-git-send-email-peter.chen@nxp.com>

Add optional properties for power sequence.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/usb/usb-device.txt | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-device.txt b/Documentation/devicetree/bindings/usb/usb-device.txt
index 1c35e7b..3661dd2 100644
--- a/Documentation/devicetree/bindings/usb/usb-device.txt
+++ b/Documentation/devicetree/bindings/usb/usb-device.txt
@@ -13,6 +13,10 @@ Required properties:
 - reg: the port number which this device is connecting to, the range
   is 1-31.
 
+Optional properties:
+power sequence properties, see
+Documentation/devicetree/bindings/power/pwrseq/pwrseq-generic.txt for detail
+
 Example:
 
 &usb1 {
@@ -21,8 +25,12 @@ Example:
 	#address-cells = <1>;
 	#size-cells = <0>;
 
-	hub: genesys at 1 {
+	genesys: hub at 1 {
 		compatible = "usb5e3,608";
 		reg = <1>;
+
+		clocks = <&clks IMX6SX_CLK_CKO>;
+		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>; /* hub reset pin */
+		reset-duration-us = <10>;
 	};
 }
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 4/8] usb: core: add power sequence handling for USB devices
From: Peter Chen @ 2016-09-20  3:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474342607-27512-1-git-send-email-peter.chen@nxp.com>

Some hard-wired USB devices need to do power sequence to let the
device work normally, the typical power sequence like: enable USB
PHY clock, toggle reset pin, etc. But current Linux USB driver
lacks of such code to do it, it may cause some hard-wired USB devices
works abnormal or can't be recognized by controller at all.

In this patch, it calls power sequence library APIs to finish
the power sequence events. It will do power on sequence at hub's
probe for all devices under this hub (includes root hub).
At hub_disconnect, it will do power off sequence which is at powered
on list.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
Tested-by Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/usb/core/hub.c | 41 ++++++++++++++++++++++++++++++++++++++---
 drivers/usb/core/hub.h |  1 +
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b48dc76..f3de1de 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -26,6 +26,7 @@
 #include <linux/mutex.h>
 #include <linux/random.h>
 #include <linux/pm_qos.h>
+#include <linux/power/pwrseq.h>
 
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
@@ -1695,6 +1696,7 @@ static void hub_disconnect(struct usb_interface *intf)
 	hub->error = 0;
 	hub_quiesce(hub, HUB_DISCONNECT);
 
+	of_pwrseq_off_list(&hub->pwrseq_on_list);
 	mutex_lock(&usb_port_peer_mutex);
 
 	/* Avoid races with recursively_mark_NOTATTACHED() */
@@ -1722,12 +1724,41 @@ static void hub_disconnect(struct usb_interface *intf)
 	kref_put(&hub->kref, hub_release);
 }
 
+#ifdef CONFIG_OF
+static int hub_of_pwrseq_on(struct usb_hub *hub)
+{
+	struct device *parent;
+	struct usb_device *hdev = hub->hdev;
+	struct device_node *np;
+	int ret;
+
+	if (hdev->parent)
+		parent = &hdev->dev;
+	else
+		parent = bus_to_hcd(hdev->bus)->self.controller;
+
+	for_each_child_of_node(parent->of_node, np) {
+		ret = of_pwrseq_on_list(np, &hub->pwrseq_on_list);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+#else
+static int hub_of_pwrseq_on(struct usb_hub *hub)
+{
+	return 0;
+}
+#endif
+
 static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
 {
 	struct usb_host_interface *desc;
 	struct usb_endpoint_descriptor *endpoint;
 	struct usb_device *hdev;
 	struct usb_hub *hub;
+	int ret = -ENODEV;
 
 	desc = intf->cur_altsetting;
 	hdev = interface_to_usbdev(intf);
@@ -1832,6 +1863,7 @@ descriptor_error:
 	INIT_DELAYED_WORK(&hub->leds, led_work);
 	INIT_DELAYED_WORK(&hub->init_work, NULL);
 	INIT_WORK(&hub->events, hub_event);
+	INIT_LIST_HEAD(&hub->pwrseq_on_list);
 	usb_get_intf(intf);
 	usb_get_dev(hdev);
 
@@ -1845,11 +1877,14 @@ descriptor_error:
 	if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
 		hub->quirk_check_port_auto_suspend = 1;
 
-	if (hub_configure(hub, endpoint) >= 0)
-		return 0;
+	if (hub_configure(hub, endpoint) >= 0) {
+		ret = hub_of_pwrseq_on(hub);
+		if (!ret)
+			return 0;
+	}
 
 	hub_disconnect(intf);
-	return -ENODEV;
+	return ret;
 }
 
 static int
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 34c1a7e..cd86f91 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -78,6 +78,7 @@ struct usb_hub {
 	struct delayed_work	init_work;
 	struct work_struct      events;
 	struct usb_port		**ports;
+	struct list_head	pwrseq_on_list; /* powered pwrseq node list */
 };
 
 /**
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 5/8] usb: chipidea: let chipidea core device of_node equal's glue layer device of_node
From: Peter Chen @ 2016-09-20  3:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474342607-27512-1-git-send-email-peter.chen@nxp.com>

From: Peter Chen <peter.chen@freescale.com>

At device tree, we have no device node for chipidea core,
the glue layer's node is the parent node for host and udc
device. But in related driver, the parent device is chipidea
core. So, in order to let the common driver get parent's node,
we let the core's device node equals glue layer device node.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
Tested-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Tested-by Joshua Clayton <stillcompiling@gmail.com>
---
 drivers/usb/chipidea/core.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 69426e6..6839e19 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -927,6 +927,16 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	/*
+	 * At device tree, we have no device node for chipidea core,
+	 * the glue layer's node is the parent node for host and udc
+	 * device. But in related driver, the parent device is chipidea
+	 * core. So, in order to let the common driver get parent's node,
+	 * we let the core's device node equals glue layer's node.
+	 */
+	if (dev->parent && dev->parent->of_node)
+		dev->of_node = dev->parent->of_node;
+
 	if (ci->platdata->phy) {
 		ci->phy = ci->platdata->phy;
 	} else if (ci->platdata->usb_phy) {
@@ -937,11 +947,15 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 
 		/* if both generic PHY and USB PHY layers aren't enabled */
 		if (PTR_ERR(ci->phy) == -ENOSYS &&
-				PTR_ERR(ci->usb_phy) == -ENXIO)
-			return -ENXIO;
+				PTR_ERR(ci->usb_phy) == -ENXIO) {
+			ret = -ENXIO;
+			goto clear_of_node;
+		}
 
-		if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy))
-			return -EPROBE_DEFER;
+		if (IS_ERR(ci->phy) && IS_ERR(ci->usb_phy)) {
+			ret = -EPROBE_DEFER;
+			goto clear_of_node;
+		}
 
 		if (IS_ERR(ci->phy))
 			ci->phy = NULL;
@@ -952,7 +966,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 	ret = ci_usb_phy_init(ci);
 	if (ret) {
 		dev_err(dev, "unable to init phy: %d\n", ret);
-		return ret;
+		goto clear_of_node;
 	}
 
 	ci->hw_bank.phys = res->start;
@@ -1058,6 +1072,8 @@ stop:
 	ci_role_destroy(ci);
 deinit_phy:
 	ci_usb_phy_exit(ci);
+clear_of_node:
+	dev->of_node = NULL;
 
 	return ret;
 }
@@ -1076,6 +1092,7 @@ static int ci_hdrc_remove(struct platform_device *pdev)
 	ci_extcon_unregister(ci);
 	ci_role_destroy(ci);
 	ci_hdrc_enter_lpm(ci, true);
+	ci->dev->of_node = NULL;
 	ci_usb_phy_exit(ci);
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v7 6/8] ARM: dts: imx6qdl: Enable usb node children with <reg>
From: Peter Chen @ 2016-09-20  3:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1474342607-27512-1-git-send-email-peter.chen@nxp.com>

From: Joshua Clayton <stillcompiling@gmail.com>

Give usb nodes #address and #size attributes, so that a child node
representing a permanently connected device such as an onboard hub may
be addressed with a <reg> attribute

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b13b0b2..8fc66e1 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -935,6 +935,8 @@
 
 			usbh1: usb at 02184200 {
 				compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+				#address-cells = <1>;
+				#size-cells = <0>;
 				reg = <0x02184200 0x200>;
 				interrupts = <0 40 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
@@ -949,6 +951,8 @@
 
 			usbh2: usb at 02184400 {
 				compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+				#address-cells = <1>;
+				#size-cells = <0>;
 				reg = <0x02184400 0x200>;
 				interrupts = <0 41 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
@@ -962,6 +966,8 @@
 
 			usbh3: usb at 02184600 {
 				compatible = "fsl,imx6q-usb", "fsl,imx27-usb";
+				#address-cells = <1>;
+				#size-cells = <0>;
 				reg = <0x02184600 0x200>;
 				interrupts = <0 42 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6QDL_CLK_USBOH3>;
-- 
2.7.4

^ permalink raw reply related


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