* Re: [PATCH v2] of, numa: Return NUMA_NO_NODE from disable of_node_to_nid() if nid not possible.
From: Rob Herring @ 2016-11-10 20:51 UTC (permalink / raw)
To: David Daney
Cc: David Daney, linux-kernel@vger.kernel.org, Frank Rowand,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
Will Deacon, Catalin Marinas, Robert Richter, Hanjun Guo,
Ganapatrao Kulkarni, Gilbert Netzer, David Daney
In-Reply-To: <93b7769a-4907-176e-9f18-0bf6bd72d15d@caviumnetworks.com>
On Thu, Nov 3, 2016 at 10:11 AM, David Daney <ddaney@caviumnetworks.com> wrote:
> On 11/02/2016 08:37 PM, Rob Herring wrote:
>>
>> On Fri, Oct 28, 2016 at 4:15 PM, David Daney <ddaney.cavm@gmail.com>
>> wrote:
>>>
>>> From: David Daney <david.daney@cavium.com>
>>>
>>> On arm64 NUMA kernels we can pass "numa=off" on the command line to
>>> disable NUMA. A side effect of this is that kmalloc_node() calls to
>>> non-zero nodes will crash the system with an OOPS:
>>>
>>> [ 0.000000] ITS@0x0000901000020000: allocated 2097152 Devices
>>> @10002000000 (flat, esz 8, psz 64K, shr 1)
>>> [ 0.000000] Unable to handle kernel NULL pointer dereference at
>>> virtual address 00001680
>>> [ 0.000000] pgd = fffffc0009470000
>>> [ 0.000000] [00001680] *pgd=0000010ffff90003, *pud=0000010ffff90003,
>>> *pmd=0000010ffff90003, *pte=0000000000000000
>>> [ 0.000000] Internal error: Oops: 96000006 [#1] SMP
>>> .
>>> .
>>> .
>>> [ 0.000000] [<fffffc00081c8950>] __alloc_pages_nodemask+0xa4/0xe68
>>> [ 0.000000] [<fffffc000821fa70>] new_slab+0xd0/0x564
>>> [ 0.000000] [<fffffc0008221e24>] ___slab_alloc+0x2e4/0x514
>>> [ 0.000000] [<fffffc0008239498>] __slab_alloc+0x48/0x58
>>> [ 0.000000] [<fffffc0008222c20>] __kmalloc_node+0xd0/0x2dc
>>> [ 0.000000] [<fffffc0008115374>] __irq_domain_add+0x7c/0x164
>>> [ 0.000000] [<fffffc0008b461dc>] its_probe+0x784/0x81c
>>> [ 0.000000] [<fffffc0008b462bc>] its_init+0x48/0x1b0
>>> [ 0.000000] [<fffffc0008b4543c>] gic_init_bases+0x228/0x360
>>> [ 0.000000] [<fffffc0008b456bc>] gic_of_init+0x148/0x1cc
>>> [ 0.000000] [<fffffc0008b5aec8>] of_irq_init+0x184/0x298
>>> [ 0.000000] [<fffffc0008b43f9c>] irqchip_init+0x14/0x38
>>> [ 0.000000] [<fffffc0008b12d60>] init_IRQ+0xc/0x30
>>> [ 0.000000] [<fffffc0008b10a3c>] start_kernel+0x240/0x3b8
>>> [ 0.000000] [<fffffc0008b101c4>] __primary_switched+0x30/0x6c
>>> [ 0.000000] Code: 912ec2a0 b9403809 0a0902fb 37b007db (f9400300)
>>> .
>>> .
>>> .
>>>
>>> This is caused by code like this in kernel/irq/irqdomain.c
>>>
>>> domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) *
>>> size),
>>> GFP_KERNEL, of_node_to_nid(of_node));
>>>
>>> When NUMA is disabled, the concept of a node is really undefined, so
>>> of_node_to_nid() should unconditionally return NUMA_NO_NODE.
>>>
>>> Fix by returning NUMA_NO_NODE when the nid is not in the set of
>>> possible nodes.
>>>
>>> Reported-by: Gilbert Netzer <noname@pdc.kth.se>
>>> Signed-off-by: David Daney <david.daney@cavium.com>
>>
>>
>> Does this need to go in 4.9?
>
>
> That would be my preference.
Given how late this is now, my having nothing else for 4.9 and that
his has never worked, I've applied for 4.10, but I did tag for stable.
Rob
^ permalink raw reply
* Re: [PATCH v2 1/3] remoteproc: qcom: Encapsulate pvt data structure for q6v56 hexagon.
From: Rob Herring @ 2016-11-10 20:30 UTC (permalink / raw)
To: Avaneesh Kumar Dwivedi
Cc: bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A, Ohad Ben-Cohen,
Mark Rutland, open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <1478268057-11847-2-git-send-email-akdwived-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Fri, Nov 04, 2016 at 07:30:54PM +0530, Avaneesh Kumar Dwivedi wrote:
> Encapsulate resources specific to each version of hexagon chip to
> device node to avoid conditional check for manipulation of those
> resources in driver code.
>
> Signed-off-by: Avaneesh Kumar Dwivedi <akdwived-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> .../devicetree/bindings/remoteproc/qcom,q6v5.txt | 1 +
> drivers/remoteproc/qcom_q6v5_pil.c | 137 ++++++++++++++++++---
> 2 files changed, 120 insertions(+), 18 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> index 57cb49e..cbc165c 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> @@ -8,6 +8,7 @@ on the Qualcomm Hexagon core.
> Value type: <string>
> Definition: must be one of:
> "qcom,q6v5-pil"
> + "qcom,q6v56-pil"
Perhaps some explanation in the commit message about what these magic
numbers mean?
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/3] PCI: qcom: add support to msm8996 PCIE controller
From: Rob Herring @ 2016-11-10 20:26 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: svarbanov, Bjorn Helgaas, linux-pci, Mark Rutland, devicetree,
linux-kernel, linux-arm-msm
In-Reply-To: <1478264387-17914-3-git-send-email-srinivas.kandagatla@linaro.org>
On Fri, Nov 04, 2016 at 12:59:46PM +0000, Srinivas Kandagatla wrote:
> This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
> Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
> legacy interrupts and it conforms to PCI Express Base 2.1 specification.
>
> This patch adds post_init callback to qcom_pcie_ops, as this is pcie
> pipe clocks are only setup after the phy is powered on.
> It also adds ltssm_enable callback as it is very much different to other
> supported SOCs in the driver.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> .../devicetree/bindings/pci/qcom,pcie.txt | 68 +++++++-
> drivers/pci/host/pcie-qcom.c | 177 ++++++++++++++++++++-
> 2 files changed, 239 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> index 4059a6f..4a0538d 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt
> @@ -7,6 +7,7 @@
> - "qcom,pcie-ipq8064" for ipq8064
> - "qcom,pcie-apq8064" for apq8064
> - "qcom,pcie-apq8084" for apq8084
> + - "qcom,pcie-msm8996" for msm8996 or apq8096
>
> - reg:
> Usage: required
> @@ -92,6 +93,16 @@
> - "aux" Auxiliary (AUX) clock
> - "bus_master" Master AXI clock
> - "bus_slave" Slave AXI clock
> +
> +- clock-names:
> + Usage: required for msm8996/apq8096
> + Value type: <stringlist>
> + Definition: Should contain the following entries
> + - "aux" Auxiliary (AUX) clock.
> + - "bus_master" Master AXI clock.
> + - "bus_slave" Slave AXI clock.
> + - "pipe" Pipe Clock driving internal logic.
> + - "cfg" Configuration clk.
The order here and the order in the example don't match. The order
should be defined.
> - resets:
> Usage: required
> Value type: <prop-encoded-array>
> @@ -115,7 +126,7 @@
> - "core" Core reset
>
> - power-domains:
> - Usage: required for apq8084
> + Usage: required for apq8084 and msm8996/apq8096
> Value type: <prop-encoded-array>
> Definition: A phandle and power domain specifier pair to the
> power domain which is responsible for collapsing
> @@ -231,3 +242,58 @@
> pinctrl-0 = <&pcie0_pins_default>;
> pinctrl-names = "default";
> };
> +
> +* Example for apq8096:
> +
> + pcie@00608000{
Drop leading 0s.
> + compatible = "qcom,pcie-msm8996", "snps,dw-pcie";
> + power-domains = <&gcc PCIE1_GDSC>;
> + bus-range = <0x00 0xff>;
> + num-lanes = <1>;
> +
> + status = "disabled";
No point to have status in the example.
> +
> + reg = <0x00608000 0x2000>,
> + <0x0d000000 0xf1d>,
> + <0x0d000f20 0xa8>,
> + <0x0d100000 0x100000>;
> +
> + reg-names = "parf", "dbi", "elbi", "config";
> +
> + phys = <&pcie_phy 1>;
> + phy-names = "pciephy";
> +
> + #address-cells = <3>;
> + #size-cells = <2>;
> + ranges = <0x01000000 0x0 0x0d200000 0x0d200000 0x0 0x100000>,
> + <0x02000000 0x0 0x0d300000 0x0d300000 0x0 0xd00000>;
> +
> + interrupts = <GIC_SPI 413 IRQ_TYPE_NONE>;
> + interrupt-names = "msi";
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 0x7>;
> + interrupt-map = <0 0 0 1 &intc 0 272 IRQ_TYPE_LEVEL_HIGH>, /* int_a */
> + <0 0 0 2 &intc 0 273 IRQ_TYPE_LEVEL_HIGH>, /* int_b */
> + <0 0 0 3 &intc 0 274 IRQ_TYPE_LEVEL_HIGH>, /* int_c */
> + <0 0 0 4 &intc 0 275 IRQ_TYPE_LEVEL_HIGH>; /* int_d */
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&pcie1_clkreq_default &pcie1_perst_default &pcie1_wake_default>;
> + pinctrl-1 = <&pcie1_clkreq_sleep &pcie1_perst_default &pcie1_wake_sleep>;
> +
> + vdda-1p8-supply = <&pm8994_l12>;
> + vdda-supply = <&pm8994_l28>;
> + linux,pci-domain = <1>;
> +
> + clocks = <&gcc GCC_PCIE_1_PIPE_CLK>,
> + <&gcc GCC_PCIE_1_AUX_CLK>,
> + <&gcc GCC_PCIE_1_CFG_AHB_CLK>,
> + <&gcc GCC_PCIE_1_MSTR_AXI_CLK>,
> + <&gcc GCC_PCIE_1_SLV_AXI_CLK>;
> +
> + clock-names = "pipe",
> + "aux",
> + "cfg",
> + "bus_master",
> + "bus_slave";
> + };
^ permalink raw reply
* Re: [PATCH v2 11/13] Documentation: devicetree: dwc2: Add host DMA binding
From: Rob Herring @ 2016-11-10 19:59 UTC (permalink / raw)
To: John Youn
Cc: Felipe Balbi, linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland
In-Reply-To: <6eb07c204dcbe7d3d9cb3db593828558ad6b3117.1478220875.git.johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
On Thu, Nov 03, 2016 at 05:56:10PM -0700, John Youn wrote:
> Add the snps,host-dma-disable binding. This controls whether to disable
> DMA in host mode.
>
> Signed-off-by: John Youn <johnyoun-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
From: Rob Herring @ 2016-11-10 19:58 UTC (permalink / raw)
To: Ulf Hansson
Cc: Lina Iyer, Kevin Hilman, Rafael J. Wysocki,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Andy Gross, Stephen Boyd,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Brendan Jackman, Lorenzo Pieralisi, Sudeep Holla, Juri Lelli,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAPDyKFoCjf1qSBDWUY_wNx21_78fRrFqcqrFbsSmabzAZJxQAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > Re-using idle state definition provided by arm,idle-state for domain
> > idle states creates a lot of confusion and limits further evolution of
> > the domain idle definition. To keep things clear and simple, define a
> > idle states for domain using a new compatible "domain-idle-state".
> >
> > Fix existing PM domains code to look for the newly defined compatible.
> >
> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> > .../bindings/power/domain-idle-state.txt | 33 ++++++++++++++++++++++
> > .../devicetree/bindings/power/power_domain.txt | 8 +++---
> > drivers/base/power/domain.c | 2 +-
> > 3 files changed, 38 insertions(+), 5 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > new file mode 100644
> > index 0000000..eefc7ed
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > @@ -0,0 +1,33 @@
> > +PM Domain Idle State Node:
> > +
> > +A domain idle state node represents the state parameters that will be used to
> > +select the state when there are no active components in the domain.
> > +
> > +The state node has the following parameters -
> > +
> > +- compatible:
> > + Usage: Required
> > + Value type: <string>
> > + Definition: Must be "domain-idle-state".
> > +
> > +- entry-latency-us
> > + Usage: Required
> > + Value type: <prop-encoded-array>
> > + Definition: u32 value representing worst case latency in
> > + microseconds required to enter the idle state.
> > + The exit-latency-us duration may be guaranteed
> > + only after entry-latency-us has passed.
>
> As we anyway are going to change this, why not use an u64 and have the
> value in ns instead of us?
I can't imagine that you would need more resolution or range. For times
less than 1us, s/w and register access times are going to dominate the
time.
Unless there is a real need, I'd keep alignment with the existing
binding.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: Add TI SCI PM Domains
From: Dave Gerlach @ 2016-11-10 19:56 UTC (permalink / raw)
To: Rob Herring, Kevin Hilman, Ulf Hansson, Jon Hunter
Cc: Nishanth Menon, devicetree@vger.kernel.org,
linux-pm@vger.kernel.org, Keerthy, Santosh Shilimkar,
Rafael J . Wysocki, linux-kernel@vger.kernel.org, Tero Kristo,
Russell King, Sudeep Holla, linux-arm-kernel@lists.infradead.org
In-Reply-To: <5811FE09.8000006@ti.com>
Rob, Ulf, Jon,
On 10/27/2016 08:15 AM, Dave Gerlach wrote:
> +Jon
> On 10/26/2016 04:59 PM, Rob Herring wrote:
>> On Mon, Oct 24, 2016 at 12:00 PM, Kevin Hilman <khilman@baylibre.com> wrote:
>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>
>>>> Hi,
>>>> On 10/21/2016 01:48 PM, Kevin Hilman wrote:
>>>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>>>
>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>> control device power states.
>>>>>>
>>>>>> Also, provide macros representing each device index as understood
>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>
>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>> ---
>>>>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 54 +++++++++++++
>>>>>> MAINTAINERS | 2 +
>>>>>> include/dt-bindings/genpd/k2g.h | 90 ++++++++++++++++++++++
>>>>>> 3 files changed, 146 insertions(+)
>>>>>> create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..32f38a349656
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>> @@ -0,0 +1,54 @@
>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>> +---------------------------------------------
>>>>>> +
>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>> +Communication between the host processor running an OS and the system
>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm domain
>>>>>> +implementation plugs into the generic pm domain framework and makes use of
>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>> +
>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>> +
>>>>>> +PM Domain Node
>>>>>> +==============
>>>>>> +The PM domain node represents the global PM domain managed by the PMMC,
>>>>>> +which in this case is the single implementation as documented by the generic
>>>>>> +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>> +
>>>>>> +Required Properties:
>>>>>> +--------------------
>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>> +- #power-domain-cells: Must be 0.
>>>>>> +- ti,sci: Phandle to the TI SCI device to use for managing the devices.
>>>>>>
>>>>>> +Example:
>>>>>> +--------------------
>>>>>> +k2g_pds: k2g_pds {
>>>>>
>>>>> should use generic name like "power-contoller", e.g. k2g_pds: power-controller
>>>>
>>>> Ok, that makes more sense.
>>>>
>>>>>
>>>>>> + compatible = "ti,sci-pm-domain";
>>>>>> + #power-domain-cells = <0>;
>>>>>> + ti,sci = <&pmmc>;
>>>>>> +};
>>>>>> +
>>>>>> +PM Domain Consumers
>>>>>> +===================
>>>>>> +Hardware blocks that require SCI control over their state must provide
>>>>>> +a reference to the sci-pm-domain they are part of and a unique device
>>>>>> +specific ID that identifies the device.
>>>>>> +
>>>>>> +Required Properties:
>>>>>> +--------------------
>>>>>> +- power-domains: phandle pointing to the corresponding PM domain node.
>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI to
>>>>>> + be used for device control.
>>>>>
>>>>> This ID doesn't look right.
>>>>>
>>>>> Why not use #power-domain-cells = <1> and pass the index in the DT? ...
>>
>> Exactly. ti,sci-id is a NAK for me.
>
> I was told not to use the onecell during v1 discussion. I agree this would be
> ideal but I cannot due to what the bindings represent, the phandle parameter is
> an index into a list of genpds, whereas we need an actual ID number we can use
> and I do not have the ability to get that from the phandle.
>
> @Ulf/Jon, is there any hope of bringing back custom xlate functions for genpd
> providers? I don't have a good background on why it was even removed. I can
> maintain a single genpd for all devices but I need a way to parse this ID,
> whether it's from a separate property or a phandle. It is locked now to indexing
> into a list of genpds but I need additional per device information for devices
> bound to a genpd and I need either a custom parameter or the ability to parse
> the phandle myself.
>
Any comments here? The meaning of the phandle onecell is fixed in the
genpd framework so I'm not sure how we want to move forward with this, I
need to pass a power domain ID to the genpd driver, and if this
shouldn't be a new property I'm not sure what direction we should take.
Regards,
Dave
>>
>>>>>
>>>>>> +See dt-bindings/genpd/k2g.h for the list of valid identifiers for k2g.
>>>>>> +
>>>>>> +Example:
>>>>>> +--------------------
>>>>>> +uart0: serial@02530c00 {
>>>>>> + compatible = "ns16550a";
>>>>>> + ...
>>>>>> + power-domains = <&k2g_pds>;
>>>>>> + ti,sci-id = <K2G_DEV_UART0>;
>>>>>
>>>>> ... like this:
>>>>>
>>>>> power-domains = <&k2g_pds K2G_DEV_UART0>;
>>>>
>>>> That's how I did it in version one actually. I was able to define my
>>>> own xlate function to parse the phandle and get that index, but Ulf
>>>> pointed me to this series by Jon Hunter [1] that simplified genpd
>>>> providers and dropped the concept of adding your own xlate. This locks
>>>> the onecell approach to using a fixed static array of genpds that get
>>>> indexed into (without passing the index to the provider, just the
>>>> genpd that's looked up), which doesn't fit our usecase, as we don't
>>>> want a 1 to 1 genpd to device mapping based on the comments provided
>>>> in v1. Now we just use the genpd device attach/detach hooks to parse
>>>> the sci-id and then use it in the genpd device start/stop hooks.
>>
>> I have no idea what any of this means. All sounds like driver
>> architecture, not anything to do with bindings.
>
> This was a response to Kevin, not part of binding description.
>
>>
>>>
>>> Ah, right. I remember now. This approach allows you to use a single
>>> genpd as discussed earlier.
>>>
>>> Makes sense now, suggestion retracted.
>>
>> IIRC, the bindings in Jon's case had a node for each domain and didn't
>> need any additional property.
>
> Yes but we only have one domain and index into it, not into a list of domains,
> so the additional property is solving a different problem.
>
> Regards,
> Dave
>
>>
>> Rob
>>
>
^ permalink raw reply
* Re: [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Benjamin Herrenschmidt @ 2016-11-10 19:32 UTC (permalink / raw)
To: Mark Rutland
Cc: zhichang.yuan, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, olof-nZhT3qVonbNeoWH0uzbU5w,
arnd-r2nGTMty4D4,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
lorenzo.pieralisi-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, minyard-HInyCGIudOg,
liviu.dudau-5wv7dgnIgG8, zourongrong-Re5JQEeQqe8AvxtiuMwx3w,
john.garry-hv44wF8Li93QT0dZR+AlfA,
gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
zhichang.yuan02-Re5JQEeQqe8AvxtiuMwx3w, kantyzc-9Onoh4P/yGk,
xuwei5-C8/M+/jPZTeaMJb+Lgu22Q, marc.zyngier-5wv7dgnIgG8
In-Reply-To: <20161110112224.GB4418@leverpostej>
On Thu, 2016-11-10 at 11:22 +0000, Mark Rutland wrote:
> On POWER8, our PCIe doesn't do IO at all, but we have an LPC bus behind
> > firmware calls ;-) We use that infrastructure to plumb in the LPC bus.
>
> Just to check, do you hook that in your inb/outb/etc?
Yes.
> Generally, it would seem nicer if we could have higher-level
> isa_{inb,outb,whatever} accessors that we could hook separately from
> other IO.
Maybe but generally speaking, we don't discriminate accessors per bus,
ie, readl etc... work on all memory mapped busses, inb... works on all
busses with an "IO space", at least that's been the idea. It probably
all comes from the fact that PCI IO and ISA are the same space on
x86 and most other platforms (not all).
> We don't necessarily have to move all ISA drivers over to that if we had
> a separate symbol for that interface.
What I do on ppc today is that I have a chunk of virtual address space
that is reserved for "IO space". The first 64k are "reserved" in that
they route to "the primary" ISA bus (for legacy crap that uses hard
coded addresses, though I use that for my LPC bus too). I "allocate"
space for the PCI IO spaces higher in that space. Was I to support more
LPC busses I could allocate them up there too.
The IO resource of a given device thus becomes the actual IO port plus
the offset of the base of the segment it's in.
For memory mapped IO, inb/outb will just add the virtual address of
the base of all IO space to that. The hooking mechanism will pickup
the stuff that isn't memory mapped.
It's a bit messy but then IO space performance has never been a huge
worry since IO cycles tend to be very slow to begin with.
Note: We also have the ISA memory and ISA FW spaces that we don't have
good accessors for. They somewhat exist (I think the fbdev layer uses
some for vga) but it's messy.
Cheers,
Ben.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] backlight: arcxcnn: devicetree bindings for ArticSand devices
From: Rob Herring @ 2016-11-10 19:25 UTC (permalink / raw)
To: Olimpiu Dejeu; +Cc: lee.jones, linux-kernel, linux-fbdev, devicetree, jg1.han
In-Reply-To: <1478197768-3694-1-git-send-email-olimpiu@arcticsand.com>
On Thu, Nov 03, 2016 at 02:29:28PM -0400, Olimpiu Dejeu wrote:
> Resubmition of arcxcnn backliught driver addressing the naming convention
s/Resubmition/Re-submission/
s/backliught/backlight/
> concerns raised by Rob H. Note that all the device tree properties are
> determined by the board design or IC EPROM settings and are not intended
> to be user adjustable.
>
> Signed-off-by: Olimpiu Dejeu <olimpiu@arcticsand.com>
>
> ---
> .../bindings/leds/backlight/arcxcnn_bl.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/leds/backlight/arcxcnn_bl.txt
With that,
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [RESEND PATCH v1 07/11] perf: hisi: Add support for Hisilicon SoC event counters
From: Mark Rutland @ 2016-11-10 19:10 UTC (permalink / raw)
To: Anurup M
Cc: devicetree, linux-arm-kernel, linux-doc, will.deacon, corbet,
catalin.marinas, robh+dt, arnd, f.fainelli, rmk+kernel, krzk,
anurup.m, zhangshaokun, tanxiaojun, xuwei5, sanil.kumar,
john.garry, gabriele.paoloni, shiju.jose, wangkefeng.wang,
guohanjun, shyju.pv, linuxarm
In-Reply-To: <1478151727-20250-8-git-send-email-anurup.m@huawei.com>
On Thu, Nov 03, 2016 at 01:42:03AM -0400, Anurup M wrote:
> + do {
> + /* Get count from individual L3C banks and sum them up */
> + for (i = 0; i < num_banks; i++) {
> + total_raw_count += hisi_read_l3c_counter(l3c_hwmod_data,
> + idx, i);
> + }
> + prev_raw_count = local64_read(&hwc->prev_count);
> +
> + /*
> + * As prev_raw_count is updated with average value of
> + * L3 cache banks, we multiply it by no of banks and
> + * compute the delta
> + */
> + delta = (total_raw_count - (prev_raw_count * num_banks)) &
> + HISI_MAX_PERIOD;
> +
> + local64_add(delta, &event->count);
> +
> + /*
> + * Divide by num of banks to get average count and
> + * update prev_count with this value
> + */
> + avg_raw_count = total_raw_count / num_banks;
> + } while (local64_cmpxchg(
> + &hwc->prev_count, prev_raw_count, avg_raw_count) !=
> + prev_raw_count);
Please don't aggregate like this; expose separate PMUs instead.
This is racy, and by averaging and multiplying we're making up and/or
throwing away data.
[...]
> + event_value = (val -
> + HISI_HWEVENT_L3C_READ_ALLOCATE);
> +
> + /* Select the appropriate Event select register */
> + if (idx > 3)
> + reg_offset += 4;
> +
> + /* Value to write to event type register */
> + val = event_value << (8 * idx);
> +
Please add helpers for these, and explain *why* the transformations are
necessary.
> + /* Find the djtag Identifier of the Unit */
> + client = l3c_hwmod_data->client;
> +
> + /*
> + * Set the event in L3C_EVENT_TYPEx Register
> + * for all L3C banks
> + */
As above, it seems like you should expose a separate PMU per bank
instead. That applies for all the other instances where you iterate over
banks.
[...]
> + for (i = 0; i < l3c_hwmod_data->l3c_hwcfg.num_banks; i++) {
> + module_id = l3c_hwmod_data->l3c_hwcfg.module_id[i];
> + cfg_en = l3c_hwmod_data->l3c_hwcfg.bank_cfgen[i];
> + ret = hisi_djtag_writereg(module_id,
> + cfg_en,
> + reg_offset,
> + value,
> + client);
> + if (!ret)
> + ret = value;
> + }
This is impossible to read. Please factor this into helpers such that
you don't need this amount of indentation.
Please do similarly elsewhere when you see this indentation pattern.
[...]
> +static int hisi_l3c_get_event_idx(struct hisi_pmu *pl3c_pmu)
> +{
> + struct hisi_l3c_data *l3c_hwmod_data = pl3c_pmu->hwmod_data;
> + int event_idx;
> +
> + event_idx =
> + find_first_zero_bit(
> + l3c_hwmod_data->hisi_l3c_event_used_mask,
> + pl3c_pmu->num_counters);
> +
> + if (event_idx == HISI_MAX_CFG_L3C_CNTR)
> + return -EAGAIN;
> +
> + __set_bit(event_idx,
> + l3c_hwmod_data->hisi_l3c_event_used_mask);
> +
> + return event_idx;
> +}
Please get rid of the weird hungarian notation (i.e. don't use 'p' as a
prefix for pointers), and use temporary variables consistently, e.g.
static int hisi_l3c_get_event_idx(struct hisi_pmu *l3c_pmu)
{
struct hisi_l3c_data *l3c_hwmod_data = l3c_pmu->hwmod_data;
unsigned long *used_mask = l3c_hwmod_data->hisi_l3c_event_used_mask;
int num_counters = pl3c_pmu->num_counters
int idx;
idx = find_first_zero_bit(used_mask, num_counters);
if (idx == num_counters)
return -EAGAIN;
set_bit(idx, used_mask);
return idx;
}
[...]
> + if (of_property_read_u32(node, "counter-reg",
> + &pl3c_hwcfg->counter_reg0_off)) {
> + dev_err(dev, "DT:Couldnot read counter-reg!\n");
> + return -EINVAL;
> + }
Please use spaces in these messages.
Otherwise, my comments on the binding apply here.
[...]
> +static int init_hisi_l3c_data(struct device *dev,
> + struct hisi_pmu *pl3c_pmu,
> + struct hisi_djtag_client *client)
> +{
> + struct hisi_l3c_data *l3c_hwmod_data = NULL;
> + int ret;
> +
> + l3c_hwmod_data = kzalloc(sizeof(struct hisi_l3c_data),
> + GFP_KERNEL);
Use:
l3c_hwmod_data = kzalloc(sizeof(*l3c_hwmod_data, GFP_KERNEL):
[...]
> +static int hisi_pmu_l3c_dev_probe(struct hisi_djtag_client *client)
> +{
> + struct hisi_pmu *pl3c_pmu = NULL;
> + struct device *dev = &client->dev;
> + int ret;
> +
> + pl3c_pmu = hisi_pmu_alloc(dev);
> + if (IS_ERR(pl3c_pmu))
> + return PTR_ERR(pl3c_pmu);
Why use error pointers for this?
hisi_pmu_alloc() only ever returns ERR_PTR(-ENOMEM) if it failed to
allocate.
It's far simpler to have it pass on NULL there, and here do:
pl3c_pmu = hisi_pmu_alloc(dev);
if (!pl3c_pmu)
return -ENOMEM;
Please also s/pl3c_pmu/l3c_pmu/ here, and elsewhere throughout the
driver. The 'p' only serves to make this harder to read.
[...]
> + /* Register with perf PMU */
> + pl3c_pmu->pmu = (struct pmu) {
> + .name = pl3c_pmu->name,
> + .task_ctx_nr = perf_invalid_context,
> + .event_init = hisi_uncore_pmu_event_init,
> + .add = hisi_uncore_pmu_add,
> + .del = hisi_uncore_pmu_del,
> + .start = hisi_uncore_pmu_start,
> + .stop = hisi_uncore_pmu_stop,
> + .read = hisi_uncore_pmu_read,
> + };
Please remove the comment above this.
[...]
> +int hisi_uncore_pmu_event_init(struct perf_event *event)
> +{
> + int err;
> + struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
This is undefined behaviour. This must be done *after* we check the
event->pmu->type.
> +
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + /* we do not support sampling as the counters are all
> + * shared by all CPU cores in a CPU die(SCCL). Also we
> + * donot support attach to a task(per-process mode)
> + */
> + if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
> + return -EOPNOTSUPP;
> +
> + /* counters do not have these bits */
> + if (event->attr.exclude_user ||
> + event->attr.exclude_kernel ||
> + event->attr.exclude_host ||
> + event->attr.exclude_guest ||
> + event->attr.exclude_hv ||
> + event->attr.exclude_idle)
> + return -EINVAL;
> +
> + if (event->cpu < 0)
> + return -EINVAL;
> +
> + event->cpu = cpumask_first(&phisi_pmu->cpu);
You should also check the event grouping.
Take a look at what we do in arch/arm/mm/cache-l2x0-pmu.c.
[...]
> +/*
> + * Enable counter and set the counter to count
> + * the event that we're interested in.
> + */
> +void hisi_uncore_pmu_enable_event(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +
> + /* Disable the hardware event counting */
> + if (phisi_pmu->ops->disable_counter)
> + phisi_pmu->ops->disable_counter(phisi_pmu, GET_CNTR_IDX(hwc));
Why isn't the counter already disabled?
> + /*
> + * Set event (if destined for Hisilicon SoC counters).
> + */
> + if (phisi_pmu->ops->set_evtype)
> + phisi_pmu->ops->set_evtype(phisi_pmu, GET_CNTR_IDX(hwc),
> + hwc->config_base);
Why isn't this done in the pmu::event_add callback?
> +
> + /* Enable the hardware event counting */
> + if (phisi_pmu->ops->enable_counter)
> + phisi_pmu->ops->enable_counter(phisi_pmu, GET_CNTR_IDX(hwc));
This should be the only necessary part of this function.
> +}
> +
> +void hisi_pmu_set_event_period(struct perf_event *event)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +
> + /*
> + * The Hisilicon PMU counters have a period of 2^32. To account for the
> + * possiblity of extreme interrupt latency we program for a period of
> + * half that. Hopefully we can handle the interrupt before another 2^31
> + * events occur and the counter overtakes its previous value.
> + */
> + u64 val = 1ULL << 31;
> +
> + local64_set(&hwc->prev_count, val);
> +
> + /* Write to the hardware event counter */
> + phisi_pmu->ops->write_counter(phisi_pmu, hwc, val);
> +}
> +
> +void hisi_uncore_pmu_start(struct perf_event *event, int flags)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> + struct hisi_pmu_hw_events *hw_events;
> +
> + hw_events = &phisi_pmu->hw_events;
> +
> + if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
> + return;
> +
> + WARN_ON_ONCE(!(hwc->state & PERF_HES_UPTODATE));
> + hwc->state = 0;
> +
> + if (phisi_pmu->ops->set_event_period)
> + phisi_pmu->ops->set_event_period(event);
When will this differ from hisi_pmu_set_event_period() above?
> + if (flags & PERF_EF_RELOAD) {
> + u64 prev_raw_count = local64_read(&hwc->prev_count);
> +
> + phisi_pmu->ops->write_counter(phisi_pmu, hwc,
> + (u32)prev_raw_count);
> + }
If we always go through hisi_pmu_set_event_period(), this looks
redundant.
> +
> + hisi_uncore_pmu_enable_event(event);
There's no matching disable_event() call in this function, so this looks
suspicious.
> + perf_event_update_userpage(event);
> +}
> +
> +void hisi_uncore_pmu_stop(struct perf_event *event, int flags)
> +{
> + struct hw_perf_event *hwc = &event->hw;
> + struct hisi_pmu *phisi_pmu = to_hisi_pmu(event->pmu);
> +
> + if (hwc->state & PERF_HES_UPTODATE)
> + return;
Why?
[...]
> +int hisi_uncore_common_fwprop_read(struct device *dev,
> + struct hisi_pmu *phisi_pmu)
> +{
> + if (device_property_read_u32(dev, "num-events",
> + &phisi_pmu->num_events)) {
> + dev_err(dev, "Cant read num-events from DT!\n");
> + return -EINVAL;
> + }
For consistency with the rest of the driver, and given there is no ACPI
support, please use the of_property_* API here.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Olof Johansson @ 2016-11-10 19:03 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rob Herring, Neil Armstrong, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-amlogic
In-Reply-To: <7ccc12bc-9a05-47e3-8ab8-d1b0ad31159e@arm.com>
On Thu, Nov 10, 2016 at 6:34 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 10/11/16 14:12, Rob Herring wrote:
>>
>> On Thu, Nov 10, 2016 at 4:26 AM, Sudeep Holla <sudeep.holla@arm.com>
>> wrote:
>>>
>>>
>>>
>>> On 10/11/16 01:22, Rob Herring wrote:
>>>>
>>>>
>>>> On Wed, Nov 02, 2016 at 10:52:09PM -0600, Sudeep Holla wrote:
>>>>>
>>>>>
>>>>> This patch adds specific compatible to support legacy SCPI protocol.
>>>>>
>>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>>> ---
>>>>> Documentation/devicetree/bindings/arm/arm,scpi.txt | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>> b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>> index d1882c4540d0..ebd03fc93135 100644
>>>>> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>>>>> @@ -7,7 +7,9 @@ by Linux to initiate various system control and power
>>>>> operations.
>>>>>
>>>>> Required properties:
>>>>>
>>>>> -- compatible : should be "arm,scpi"
>>>>> +- compatible : should be
>>>>> + * "arm,scpi" : For implementations complying to SCPI v1.0 or
>>>>> above
>>>>> + * "arm,legacy-scpi" : For implementations complying pre SCPI
>>>>> v1.0
>>>>
>>>>
>>>>
>>>> I'd prefer that we explicitly enumerate the old versions. Are there
>>>> many?
>>>>
>>>
>>> I understand your concern, but this legacy SCPI protocol was not
>>> officially released. It was just WIP which vendors picked up from very
>>> early releases. Since they are not numbered, it's hard to have specific
>>> compatibles with different versions until v1.0. That's one of the reason
>>> to retain platform specific compatible so that we can add any quirks
>>> based on them if needed.
>>>
>>> I will probably add these information in the commit log so that it's
>>> clear why we can't do version based compatible.
>>
>>
>> This is exactly my point. By enumerate, I meant having platform
>> specific compatibles. Having "arm,legacy-scpi" is pointless because
>> who knows what version they followed and they may all be different.
>>
>
> OK, but IIUC Olof's concern wanted a generic one along with the platform
> specific compatible which kind of makes sense as so far we have seen
> some commonality between Amlogic and Rockchip.
>
> E.g. Amlogic follows most of the legacy protocol though it deviates in
> couple of things which we can handle with platform specific compatible
> (in the following patch in the series). When another user(Rockchip ?)
> make use of this legacy protocol, we can start using those platform
> specific compatible for deviations only.
>
> Is that not acceptable ?
If there's no shared legacy feature set, then it's probably less
useful to have a shared less precise compatible value.
What the main point I was trying to get across was that we shouldn't
expand the generic binding with per-vendor compatible fields, instead
we should have those as extensions on the side.
I'm also a little apprehensive of using "legacy", it goes in the same
bucket as "misc". At some point 1.0 will be legacy too, etc.
-Olof
^ permalink raw reply
* Re: [PATCH 6/6] ARM: dts: rockchip: add rockchip RK1108 Evaluation board
From: Rob Herring @ 2016-11-10 18:57 UTC (permalink / raw)
To: Andy Yan
Cc: heiko-4mtYJXux2i+zQB+pC5nmwQ,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478177039-12257-1-git-send-email-andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On Thu, Nov 03, 2016 at 08:43:59PM +0800, Andy Yan wrote:
> RK1108EVB is designed by Rockchip for CVR field.
> This patch add basic support for it, which can boot with
> initramfs into shell.
>
> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
>
> Documentation/devicetree/bindings/arm/rockchip.txt | 3 +
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/rk1108-evb.dts | 69 ++++++++++++++++++++++
> 3 files changed, 73 insertions(+)
> create mode 100644 arch/arm/boot/dts/rk1108-evb.dts
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: rockchip-dw-mshc: add RK1108 dw-mshc description
From: Rob Herring @ 2016-11-10 18:56 UTC (permalink / raw)
To: Andy Yan
Cc: heiko-4mtYJXux2i+zQB+pC5nmwQ, shawn.lin-TNX95d0MmH7DzftRWevZcw,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8
In-Reply-To: <1478176250-11840-1-git-send-email-andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On Thu, Nov 03, 2016 at 08:30:50PM +0800, Andy Yan wrote:
> From: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> Add "rockchip,rk1108-dw-mshc", "rockchip,rk3288-dw-mshc" for
> dwmmc on rk1108 platform.
>
> Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> Signed-off-by: Andy Yan <andy.yan-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
>
> Documentation/devicetree/bindings/mmc/rockchip-dw-mshc.txt | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] Documentation: DT: Add bmi160 imu binding
From: Rob Herring @ 2016-11-10 18:55 UTC (permalink / raw)
To: Marcin Niestroj
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Daniel Baluta, Gregor Boirie,
Sanchayan Maity, Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161103112527.29623-2-m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
On Thu, Nov 03, 2016 at 12:25:27PM +0100, Marcin Niestroj wrote:
> This adds documentation for Bosch BMI160 Inertial Measurement Unit
> device-tree bindings.
>
> Signed-off-by: Marcin Niestroj <m.niestroj-z3quKL4iOrmQ6ZAhV5LmOA@public.gmane.org>
> ---
> .../devicetree/bindings/iio/imu/bmi160.txt | 34 ++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
> new file mode 100644
> index 0000000..b02ef3e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
> @@ -0,0 +1,34 @@
> +Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
> +and externally connectable Magnetometer
> +
> +https://www.bosch-sensortec.com/bst/products/all_products/bmi160
> +
> +Required properties:
> + - compatible : should be "bosch,bmi160"
> + - reg : the I2C address or SPI chip select number of the sensor
> + - spi-max-frequency : set maximum clock frequency (only for SPI)
> +
> +Optional properties:
> + - interrupt-parent : should be the phandle of the interrupt controller
> + - interrupts : interrupt mapping for GPIO IRQ, must be IRQ_TYPE_LEVEL_LOW
The fact that a GPIO is typically used is outside the scope of this doc.
> + - interrupt-names : set to "INT2" if using INT2 pin
Normally there's no point to have names property when there is a single
interrupt. However, it seems this could be either INT1 or INT2 or both
connected. You need to specify all the options.
Rob
^ permalink raw reply
* Re: [PATCH 13/13] Documentation: synopsys-dw-mshc: remove the unused properties
From: Rob Herring @ 2016-11-10 18:47 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-mmc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, krzk-DgEjT+Ai2ygdnm+yROfE0A,
heiko-4mtYJXux2i+zQB+pC5nmwQ, shawn.lin-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <20161103062135.10697-14-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Thu, Nov 03, 2016 at 03:21:35PM +0900, Jaehoon Chung wrote:
> "support-highspeed" was the obsoleted property.
> And "broken-cd" is not synopsys specific property.
> It can be referred to mmc.txt binding Documentation.
>
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 5 -----
> 1 file changed, 5 deletions(-)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RESEND PATCH v1 05/11] dt-bindings: perf: hisi: Add Devicetree bindings for Hisilicon SoC PMU
From: Mark Rutland @ 2016-11-10 18:30 UTC (permalink / raw)
To: Anurup M
Cc: devicetree, linux-arm-kernel, linux-doc, will.deacon, corbet,
catalin.marinas, robh+dt, arnd, f.fainelli, rmk+kernel, krzk,
anurup.m, zhangshaokun, tanxiaojun, xuwei5, sanil.kumar,
john.garry, gabriele.paoloni, shiju.jose, wangkefeng.wang,
guohanjun, shyju.pv, linuxarm
In-Reply-To: <1478151727-20250-6-git-send-email-anurup.m@huawei.com>
Hi,
On Thu, Nov 03, 2016 at 01:42:01AM -0400, Anurup M wrote:
> 1) Device tree bindings for Hisilicon SoC PMU.
> 2) Add example for Hisilicon L3 cache, MN and DDRC PMU.
>
> Signed-off-by: Anurup M <anurup.m@huawei.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
> .../devicetree/bindings/arm/hisilicon/pmu.txt | 127 +++++++++++++++++++++
> 1 file changed, 127 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
> new file mode 100644
> index 0000000..e7b35e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/hisilicon/pmu.txt
> @@ -0,0 +1,127 @@
> +Hisilicon SoC hip05/06/07 ARMv8 PMU
> +===================================
> +
> +The Hisilicon SoC chips like hip05/06/07 etc. consist of varous independent
> +system device PMU's such as L3 cache (L3C), Miscellaneous Nodes(MN) and DDR
s/PMU's/PMUs/
> +comtroller. These PMU devices are independent and have hardware logic to
s/comtroller/controller/
> +gather statistics and performance information.
> +
> +HiSilicon SoC chip is encapsulated by multiple CPU and IO die's. The CPU die
s/die's/dies/
> +is called as Super CPU cluster (SCCL) which includes 16 cpu-cores. Every SCCL
> +is further grouped as CPU clusters (CCL) which includes 4 cpu-cores each.
> +e.g. In the case of hip05/06/07, each SCCL has 1 L3 cache and 1 MN PMU device.
> +
> +The Hisilicon SoC PMU DT node bindigs for uncore PMU devices are as below.
s/bindigs/bindings/
> +For PMU devices like L3 cache. MN etc. which are accessed using the djtag,
> +the parent node will be the djtag node of the corresponding CPU die(SCCL).
> +
> +For uncore PMU devices there are some common required properties as detailed
> +below.
> +
> +Required properties:
> + - compatible : This field contain two values. The first value is
> + always "hisilicon" and second value is the Module type as shown
> + in below examples:
Just say:
- Compatible: should contain one of:
> + (a) "hisilicon,hisi-pmu-l3c-v1" for Hisilicon SoC L3C PMU
> + device (Version 1)
> + (b) "hisilicon,hisi-pmu-mn-v1" for Hisilicon SoC MN PMU
> + device (Version 1)
> + (c) "hisilicon,hisi-pmu-ddrc-v1" for Hisilicon SoC DDRC PMU
> + device (Version 1)
> + The hip05/06/07 chips have v1 hardware for L3C, MN and DDRC.
> +
> + - scl-id : The Super Cluster ID. This can be the ID of the CPU die
> + or IO die in the chip.
What's this needed for?
> + - num-events : No of events supported by this PMU device.
> +
> + - num-counters : No of hardware counters available for counting.
This isn't probeable or well-known?
> +
> +L3 cache
> +--------
> +The L3 cache is dedicated for each SCCL and hence there are separate DT nodes
> +for L3 cache for each SCCL. For L3 cache PMU the additional required properties
> +are
> + - counter-reg : Counter register offset.
> +
> + - evtype-reg : Event select register offset.
> +
> + - evctrl-reg : Event counting control(LAUCTRL) register offset.
Surely for a given revision of the chip these offsets are known? i.e.
surely the compatible string implies specific offsets?
> + - event-en : Event enable value.
Huh?
> + - module-id : Module ID to input for djtag. This property is an array of
> + module_id for each L3 cache banks.
> +
> + - num-banks : Number of banks or instances of the device.
What's a bank? Surely they have separate instances of the PMU?
What order are these in?
> + - cfgen-map : Config enable array to select the bank.
Huh?
> +Miscellaneous Node
> +-------------------
> +The MN is dedicated for each SCCL and hence there are separate DT nodes for MN
> +for each SCCL. For MN PMU the additional required properties are
> + - counter-reg : Counter register offset.
> +
> + - evtype-reg : Event select register offset.
> +
> + - evctrl-reg : Event counting control register offset.
Likewise, surely this is well-known for a given revision of the chip?
> +
> + - module-id : Module ID to input for djtag. As MN doesnot have multiple banks
> + this property is a single value.
> +
> + - cfgen-map : Config enable to select the bank. For MN it is a single value
> +
> + - event-en : Event enable value.
Same comments as for the L3 cache nodes
[...]
> +DDR controller
> +--------------
> +Each SCCL in Hip05/06/07 chips have 2 DDR channels and hence 2 DDR controllers.
> +There are separate DT nodes for each DDR channel.
> +For DDRC PMU the additional required properties are
> +
> + - ch-id : DDRC Channel ID.
Why is this necessary?
Thanks,
Mark.
> + - reg : Register base address and range for the DDRC channel.
> +
> +Example:
> + /* DDRC for CPU die scl #2 Channel #1 for hip05 */
> + pmu_sccl0_ddrc1: pmu_ddrc1@80358000 {
> + compatible = "hisilicon,hisi-pmu-ddrc-v1";
> + scl-id = <0x02>;
> + ch-id = <0x1>;
> + num-events = <0x0D>;
> + num-counters = <0x04>;
> + reg = <0x80358000 0x10000>; /* TOTEMC DDRC1 */
> + };
> --
> 2.1.4
>
^ permalink raw reply
* Re: [PATCH v2 1/3] ARM: dts: imx6qdl-apalis: Do not rely on DDC I2C bus bitbang for HDMI
From: Vladimir Zapolskiy @ 2016-11-10 18:29 UTC (permalink / raw)
To: Stefan Agner, Philipp Zabel
Cc: maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w, Shawn Guo,
marcel.ziswiler-2KBjVHiyJgBBDgjK7y7TUQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <57268103e8fabf344723eae99b45ec7d-XLVq0VzYD2Y@public.gmane.org>
Hi Stefan, Philipp,
On 11/09/2016 02:50 AM, Stefan Agner wrote:
> On 2016-11-08 09:33, maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> Hello Shawn,
>>
>> On 16-10-22 15:43:04, Vladimir Zapolskiy wrote:
>>> Hi Shawn,
>>>
>>> On 10/22/2016 06:25 AM, Shawn Guo wrote:
>>>> On Mon, Sep 19, 2016 at 10:41:51AM +0530, Sanchayan Maity wrote:
>>>>> Remove the use of DDC I2C bus bitbang to support reading of EDID
>>>>> and rely on support from internal HDMI I2C master controller instead.
>>>>> As a result remove the device tree property ddc-i2c-bus.
>>>>>
>>>>> Signed-off-by: Sanchayan Maity <maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> I think that the dw-hdmi i2c support [1] is a prerequisite of this
>>>> patch. I do not see it lands on v4.9-rc1. Or am I missing something?
>>>>
>>>> Shawn
>>>>
>>>> [1] https://patchwork.kernel.org/patch/9296883/
>>>>
>>>
>>> I'm adding Philipp to Cc, since he is the last one who tested the change
>>> and helped me to push the change to the mainline:
>>>
>>> https://lists.freedesktop.org/archives/dri-devel/2016-September/118569.html
>>>
>>> The problem is that there is no official DW HDMI bridge maintainer, may be
>>> you can review the change, and if you find it satisfactory push it through
>>> ARM/iMX tree.
>>
>> Shawn, is it okay if that patch goes through your ARM/iMX tree?
>
> I don't think it makes sense that the DRM bridge changes go through
> Shawn's tree. Dave should merge Philipps pull request...
>
Philipp, do you mind to submit a rebased pull request one more time?
--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/3] irqchip: mtk-cirq: Add mediatek mtk-cirq implement
From: Marc Zyngier @ 2016-11-10 18:24 UTC (permalink / raw)
To: Youlin Pei
Cc: Rob Herring, Matthias Brugger, Thomas Gleixner, Jason Cooper,
Mark Rutland, Russell King, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
hongkun.cao-NuS5LvNUpcJWk0Htik3J/w,
yong.wu-NuS5LvNUpcJWk0Htik3J/w, erin.lo-NuS5LvNUpcJWk0Htik3J/w,
chieh-jay.liu-NuS5LvNUpcJWk0Htik3J/w
In-Reply-To: <1478573833.32099.11.camel@mtksdaap41>
On 08/11/16 02:57, Youlin Pei wrote:
> On Fri, 2016-11-04 at 22:21 +0000, Marc Zyngier wrote:
>> On Fri, Nov 04 2016 at 04:42:57 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>> On Tue, 2016-11-01 at 20:49 +0000, Marc Zyngier wrote:
>>>> On Tue, Nov 01 2016 at 11:52:01 AM, Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>>>> In Mediatek SOCs, the CIRQ is a low power interrupt controller
>>>>> designed to works outside MCUSYS which comprises with Cortex-Ax
>>>>> cores,CCI and GIC.
>>>>>
>>>>> The CIRQ controller is integrated in between MCUSYS( include
>>>>> Cortex-Ax, CCI and GIC ) and interrupt sources as the second
>>>>> level interrupt controller. The external interrupts which outside
>>>>> MCUSYS will feed through CIRQ then bypass to GIC. CIRQ can monitors
>>>>> all edge trigger interupts. When an edge interrupt is triggered,
>>>>> CIRQ can record the status and generate a pulse signal to GIC when
>>>>> flush command executed.
>>>>>
>>>>> When system enters sleep mode, MCUSYS will be turned off to improve
>>>>> power consumption, also GIC is power down. The edge trigger interrupts
>>>>> will be lost in this scenario without CIRQ.
>>>>>
>>>>> This commit provides the CIRQ irqchip implement.
>>>>>
>>>>> Signed-off-by: Youlin Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>>> ---
>>>>> drivers/irqchip/Makefile | 2 +-
>>>>> drivers/irqchip/irq-mtk-cirq.c | 262 ++++++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 263 insertions(+), 1 deletion(-)
>>>>> create mode 100644 drivers/irqchip/irq-mtk-cirq.c
>>>>>
>>>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>>>> index e4dbfc8..8f33580 100644
>>>>> --- a/drivers/irqchip/Makefile
>>>>> +++ b/drivers/irqchip/Makefile
>>>>> @@ -60,7 +60,7 @@ obj-$(CONFIG_BCM7120_L2_IRQ) += irq-bcm7120-l2.o
>>>>> obj-$(CONFIG_BRCMSTB_L2_IRQ) += irq-brcmstb-l2.o
>>>>> obj-$(CONFIG_KEYSTONE_IRQ) += irq-keystone.o
>>>>> obj-$(CONFIG_MIPS_GIC) += irq-mips-gic.o
>>>>> -obj-$(CONFIG_ARCH_MEDIATEK) += irq-mtk-sysirq.o
>>>>> +obj-$(CONFIG_ARCH_MEDIATEK) += irq-mtk-sysirq.o irq-mtk-cirq.o
>>>>> obj-$(CONFIG_ARCH_DIGICOLOR) += irq-digicolor.o
>>>>> obj-$(CONFIG_RENESAS_H8300H_INTC) += irq-renesas-h8300h.o
>>>>> obj-$(CONFIG_RENESAS_H8S_INTC) += irq-renesas-h8s.o
>>>>> diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
>>>>> new file mode 100644
>>>>> index 0000000..fc43ef3
>>>>> --- /dev/null
>>>>> +++ b/drivers/irqchip/irq-mtk-cirq.c
>>>>> @@ -0,0 +1,262 @@
>>>>> +/*
>>>>> + * Copyright (c) 2016 MediaTek Inc.
>>>>> + * Author: Youlin.Pei <youlin.pei-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or modify
>>>>> + * it under the terms of the GNU General Public License version 2 as
>>>>> + * published by the Free Software Foundation.
>>>>> + *
>>>>> + * This program is distributed in the hope that it will be useful,
>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>>> + * GNU General Public License for more details.
>>>>> + */
>>>>> +
>>>>> +#include <linux/irq.h>
>>>>> +#include <linux/irqchip.h>
>>>>> +#include <linux/irqdomain.h>
>>>>> +#include <linux/of.h>
>>>>> +#include <linux/of_irq.h>
>>>>> +#include <linux/of_address.h>
>>>>> +#include <linux/io.h>
>>>>> +#include <linux/slab.h>
>>>>> +#include <linux/syscore_ops.h>
>>>>> +
>>>>> +#define CIRQ_ACK 0x40
>>>>> +#define CIRQ_MASK_SET 0xc0
>>>>> +#define CIRQ_MASK_CLR 0x100
>>>>> +#define CIRQ_SENS_SET 0x180
>>>>> +#define CIRQ_SENS_CLR 0x1c0
>>>>> +#define CIRQ_POL_SET 0x240
>>>>> +#define CIRQ_POL_CLR 0x280
>>>>> +#define CIRQ_CONTROL 0x300
>>>>> +
>>>>> +#define CIRQ_EN 0x1
>>>>> +#define CIRQ_EDGE 0x2
>>>>> +#define CIRQ_FLUSH 0x4
>>>>> +
>>>>> +#define CIRQ_IRQ_NUM 0x200
>>>>> +
>>>>> +struct mtk_cirq_chip_data {
>>>>> + void __iomem *base;
>>>>> + unsigned int ext_irq_start;
>>>>> +};
>>>>> +
>>>>> +static struct mtk_cirq_chip_data *cirq_data;
>>>>
>>>> Are you guaranteed that you'll only ever have a single CIRQ in any
>>>> system?
>>>
>>> In Mediatek's SOC, only hace a single CIRQ.
>>>
>>>>
>>>>> +
>>>>> +static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
>>>>> +{
>>>>> + struct mtk_cirq_chip_data *chip_data = data->chip_data;
>>>>> + unsigned int cirq_num = data->hwirq;
>>>>> + u32 mask = 1 << (cirq_num % 32);
>>>>> +
>>>>> + writel(mask, chip_data->base + offset + (cirq_num / 32) * 4);
>>>>
>>>> Why can't you use the relaxed accessors?
>>>
>>> It seems that i use wrong function, i will change the writel to
>>> writel_relaxed in next ve
>>>
>>>>
>>>>> +}
>>>>> +
>>>>> +static void mtk_cirq_mask(struct irq_data *data)
>>>>> +{
>>>>> + mtk_cirq_write_mask(data, CIRQ_MASK_SET);
>>>>> + irq_chip_mask_parent(data);
>>>>> +}
>>>>> +
>>>>> +static void mtk_cirq_unmask(struct irq_data *data)
>>>>> +{
>>>>> + mtk_cirq_write_mask(data, CIRQ_MASK_CLR);
>>>>> + irq_chip_unmask_parent(data);
>>>>> +}
>>>>> +
>>>>> +static void mtk_cirq_eoi(struct irq_data *data)
>>>>> +{
>>>>> + mtk_cirq_write_mask(data, CIRQ_ACK);
>>>>
>>>> EOI and ACK have very different semantics. What is this write actually
>>>> doing? Also, you're now doing an additional MMIO write on each interrupt
>>>> EOI, doubling its cost. Do you really need to do actually signal the HW
>>>> that we've EOIed an interrupt? I would have hoped that you'd be able to
>>>> put it in "bypass" mode as long as you're not suspending...
>>>>
>>>
>>> When external interrupt happened, CIRQ status register record the status
>>> even CIRQ is not enabled. when execute the flush command, CIRQ will
>>> resend the signals according to the status. So if don't clear the
>>> status, CIRQ will resend the wrong signals. the ACK write operation will
>>> clear the status.
>>
>> But at this time, we haven't suspended yet, and there is nothing to
>> replay. Also, you only enable the edge capture when suspending. So what
>> are you ACKing here? Can't you simply clear everything right when
>> suspending and not do it at all on the fast path?
>
> I had planned to ACK the status in cirq suspend callback, but
> encountered a problem.
> following is a piece of code from
> http://lxr.free-electrons.com/source/kernel/power/suspend.c#L361
>
> arch_suspend_disable_irqs(); ---step1
> BUG_ON(!irqs_disabled());
>
> error = syscore_suspend();
> |
> ---cirq suspend(); ---step2
>
> if ack the status in cirq suspend, the interrupts will be lost which
> happened during step1 to step2.
>
> So would you mind give me some suggestions about this?
> Thanks a lot!
Right. So maybe there is a way:
- On suspend you can iterate over all the cirq interrupts that have been
recorded
- For each of those, you inspect if it is pending at the GIC level
(using the irq_get_irqchip_state helper)
- if not pending, you Ack it, because it cannot be delivered anymore
- If it is pending, then you know it happened between step 1 and step 2.
- You then have the choice of either going into suspend and waking up
immediately, or failing the suspend.
Thoughts?
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V3 1/9] PM / OPP: Reword binding supporting multiple regulators per device
From: Viresh Kumar @ 2016-11-10 18:09 UTC (permalink / raw)
To: Mark Brown
Cc: Rafael Wysocki, nm, sboyd, Viresh Kumar, linaro-kernel, linux-pm,
linux-kernel, Vincent Guittot, robh, d-gerlach, devicetree
In-Reply-To: <20161110163659.rk67qrmnrovq2ktf@sirena.org.uk>
On 10-11-16, 16:36, Mark Brown wrote:
> On Thu, Nov 10, 2016 at 09:34:40AM +0530, Viresh Kumar wrote:
> > On 09-11-16, 14:58, Mark Brown wrote:
> > > On Wed, Oct 26, 2016 at 12:02:56PM +0530, Viresh Kumar wrote:
>
> > > > + Entries for multiple regulators shall be provided in the same field separated
> > > > + by angular brackets <>. The OPP binding doesn't provide any provisions to
> > > > + relate the values to their power supplies or the order in which the supplies
> > > > + need to be configured.
>
> > > I don't understand how this works. If we have an unordered list of
> > > values to set for regulators how will we make sense of them?
>
> > The platform driver is responsible to identify the order and pass it on to the
> > OPP core. And the platform driver needs to have that hard coded.
>
> That *really* should be in the binding.
Okay, how do you suggest doing that? Will a property like supply-names
in the OPP table be fine? Like this:
@@ -369,13 +378,16 @@ Example 4: Handling multiple regulators
compatible = "arm,cortex-a7";
...
- cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
+ vcc0-supply = <&cpu_supply0>;
+ vcc1-supply = <&cpu_supply1>;
+ vcc2-supply = <&cpu_supply2>;
operating-points-v2 = <&cpu0_opp_table>;
};
};
cpu0_opp_table: opp_table0 {
compatible = "operating-points-v2";
+ supply-names = "vcc0", "vcc1", "vcc2";
opp-shared;
--
viresh
^ permalink raw reply
* Re: [RESEND PATCH] regulator: pwm: fix syntax errors in the examples
From: Mark Brown @ 2016-11-10 18:02 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Rob Herring,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cb8dbca0-1f7b-d336-4347-1e665d57f060-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1258 bytes --]
On Thu, Nov 10, 2016 at 06:05:59PM +0100, Peter Rosin wrote:
> On 2016-11-10 17:18, Mark Brown wrote:
> > On Thu, Nov 10, 2016 at 11:25:27AM +0100, Peter Rosin wrote:
> >> While at it, clean up some other things as well.
> > This isn't a useful changelog, it's just saying that there's some
> > random changes for no reason. Please make proper patches with
> > changelogs that explain them.
> It's a very uninteresting patch, I guess I made a changelog to match.
> But I can't help myself from asking a few qustions...
> Patch*es* and changelog*s*?
> Oh, you want me to split into a series of several patches? One for the
> quotes, one for the "../regultor/" removal, one for the empty
> comment removal and one for the s/pwm_reglator/pwm-regulator/ thing?
> Be careful what you ask for, you might get it...
The above is not a problem for me, indeed it's actually good as it makes
it *much* quicker and simpler for me to read the changes and figure out
if they all make sense and do what was intended. The less the changelog
says about what the change is supposed to do and the more different
things the change does the more effort is required to work out if it's
sensible and the more chance there is of one of the changes blocking the
other ones.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH v1 03/11] drivers: soc: hisi: Add support for Hisilicon Djtag driver
From: Mark Rutland @ 2016-11-10 17:55 UTC (permalink / raw)
To: Anurup M
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-doc-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
corbet-T1hC0tSOHrs, catalin.marinas-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, arnd-r2nGTMty4D4,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ, krzk-DgEjT+Ai2ygdnm+yROfE0A,
anurup.m-hv44wF8Li93QT0dZR+AlfA,
zhangshaokun-C8/M+/jPZTeaMJb+Lgu22Q,
tanxiaojun-hv44wF8Li93QT0dZR+AlfA, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
sanil.kumar-C8/M+/jPZTeaMJb+Lgu22Q,
john.garry-hv44wF8Li93QT0dZR+AlfA,
gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
shiju.jose-hv44wF8Li93QT0dZR+AlfA,
wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA,
guohanjun-hv44wF8Li93QT0dZR+AlfA, shyju.pv-hv44wF8Li93QT0dZR+AlfA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <1478151727-20250-4-git-send-email-anurup.m-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On Thu, Nov 03, 2016 at 01:41:59AM -0400, Anurup M wrote:
> From: Tan Xiaojun <tanxiaojun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> The Hisilicon Djtag is an independent component which connects
> with some other components in the SoC by Debug Bus. This driver
> can be configured to access the registers of connecting components
> (like L3 cache) during real time debugging.
>
Just to check, is this likely to be used in multi-socket hardware, and
if so, are instances always-on?
> Signed-off-by: Tan Xiaojun <tanxiaojun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: John Garry <john.garry-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Anurup M <anurup.m-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
> drivers/soc/Kconfig | 1 +
> drivers/soc/Makefile | 1 +
> drivers/soc/hisilicon/Kconfig | 12 +
> drivers/soc/hisilicon/Makefile | 1 +
> drivers/soc/hisilicon/djtag.c | 639 ++++++++++++++++++++++++++++++++++++
> include/linux/soc/hisilicon/djtag.h | 38 +++
> 6 files changed, 692 insertions(+)
> create mode 100644 drivers/soc/hisilicon/Kconfig
> create mode 100644 drivers/soc/hisilicon/Makefile
> create mode 100644 drivers/soc/hisilicon/djtag.c
> create mode 100644 include/linux/soc/hisilicon/djtag.h
Other than the PMU driver(s), what is going to use this?
If you don't have something in particular, please also place this under
drivers/perf/hisilicon, along with the PMU driver(s).
We can always move it later if necessary.
[...]
> +#define SC_DJTAG_TIMEOUT 100000 /* 100ms */
This would be better as:
#define SC_DJTAG_TIMEOUT_US (100 * USEC_PER_MSEC)
(you'll need to include <linux/time64.h>)
[...]
> +static void djtag_read32_relaxed(void __iomem *regs_base, u32 off, u32 *value)
> +{
> + void __iomem *reg_addr = regs_base + off;
> +
> + *value = readl_relaxed(reg_addr);
> +}
> +
> +static void djtag_write32(void __iomem *regs_base, u32 off, u32 val)
> +{
> + void __iomem *reg_addr = regs_base + off;
> +
> + writel(val, reg_addr);
> +}
I think these make the driver harder to read, especially given the read
function is void and takes an output pointer.
In either case you can call readl/writel directly with base + off for
the __iomem ptr. Please do that.
> +
> +/*
> + * djtag_readwrite_v1/v2: djtag read/write interface
> + * @reg_base: djtag register base address
> + * @offset: register's offset
> + * @mod_sel: module selection
> + * @mod_mask: mask to select specific modules for write
> + * @is_w: write -> true, read -> false
> + * @wval: value to register for write
> + * @chain_id: which sub module for read
> + * @rval: value in register for read
> + *
> + * Return non-zero if error, else return 0.
> + */
> +static int djtag_readwrite_v1(void __iomem *regs_base, u32 offset, u32 mod_sel,
> + u32 mod_mask, bool is_w, u32 wval, int chain_id, u32 *rval)
> +{
> + u32 rd;
> + int timeout = SC_DJTAG_TIMEOUT;
> +
> + if (!(mod_mask & CHAIN_UNIT_CFG_EN)) {
> + pr_warn("djtag: do nothing.\n");
> + return 0;
> + }
> +
> + /* djtag mster enable & accelerate R,W */
> + djtag_write32(regs_base, SC_DJTAG_MSTR_EN,
> + DJTAG_NOR_CFG | DJTAG_MSTR_EN);
> +
> + /* select module */
> + djtag_write32(regs_base, SC_DJTAG_DEBUG_MODULE_SEL, mod_sel);
> + djtag_write32(regs_base, SC_DJTAG_CHAIN_UNIT_CFG_EN,
> + mod_mask & CHAIN_UNIT_CFG_EN);
> +
> + if (is_w) {
> + djtag_write32(regs_base, SC_DJTAG_MSTR_WR, DJTAG_MSTR_W);
> + djtag_write32(regs_base, SC_DJTAG_MSTR_DATA, wval);
> + } else
> + djtag_write32(regs_base, SC_DJTAG_MSTR_WR, DJTAG_MSTR_R);
> +
> + /* address offset */
> + djtag_write32(regs_base, SC_DJTAG_MSTR_ADDR, offset);
> +
> + /* start to write to djtag register */
> + djtag_write32(regs_base, SC_DJTAG_MSTR_START_EN, DJTAG_MSTR_START_EN);
> +
> + /* ensure the djtag operation is done */
> + do {
> + djtag_read32_relaxed(regs_base, SC_DJTAG_MSTR_START_EN, &rd);
> + if (!(rd & DJTAG_MSTR_EN))
> + break;
> +
> + udelay(1);
> + } while (timeout--);
> +
> + if (timeout < 0) {
> + pr_err("djtag: %s timeout!\n", is_w ? "write" : "read");
> + return -EBUSY;
> + }
> +
> + if (!is_w)
> + djtag_read32_relaxed(regs_base,
> + SC_DJTAG_RD_DATA_BASE + chain_id * 0x4, rval);
> +
> + return 0;
> +}
Please factor out the common bits into helpers and have separate
read/write functions. It's incredibly difficult to follow the code with
read/write hidden behind a boolean parameter.
> +static const struct of_device_id djtag_of_match[] = {
> + /* for hip05(D02) cpu die */
> + { .compatible = "hisilicon,hip05-cpu-djtag-v1",
> + .data = (void *)djtag_readwrite_v1 },
> + /* for hip05(D02) io die */
> + { .compatible = "hisilicon,hip05-io-djtag-v1",
> + .data = (void *)djtag_readwrite_v1 },
> + /* for hip06(D03) cpu die */
> + { .compatible = "hisilicon,hip06-cpu-djtag-v1",
> + .data = (void *)djtag_readwrite_v1 },
> + /* for hip06(D03) io die */
> + { .compatible = "hisilicon,hip06-io-djtag-v2",
> + .data = (void *)djtag_readwrite_v2 },
> + /* for hip07(D05) cpu die */
> + { .compatible = "hisilicon,hip07-cpu-djtag-v2",
> + .data = (void *)djtag_readwrite_v2 },
> + /* for hip07(D05) io die */
> + { .compatible = "hisilicon,hip07-io-djtag-v2",
> + .data = (void *)djtag_readwrite_v2 },
> + {},
> +};
Binding documentation for all of these should be added *before* this
patch, per Documentation/devicetree/bindings/submitting-patches.txt.
Please move any relevant binding documentation earlier in the series.
> +MODULE_DEVICE_TABLE(of, djtag_of_match);
> +
> +static ssize_t
> +show_modalias(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct hisi_djtag_client *client = to_hisi_djtag_client(dev);
> +
> + return sprintf(buf, "%s%s\n", MODULE_PREFIX, client->name);
> +}
> +static DEVICE_ATTR(modalias, 0444, show_modalias, NULL);
> +
> +static struct attribute *hisi_djtag_dev_attrs[] = {
> + NULL,
> + /* modalias helps coldplug: modprobe $(cat .../modalias) */
> + &dev_attr_modalias.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(hisi_djtag_dev);
Why do we need to expose this under sysfs?
> +struct bus_type hisi_djtag_bus = {
> + .name = "hisi-djtag",
> + .match = hisi_djtag_device_match,
> + .probe = hisi_djtag_device_probe,
> + .remove = hisi_djtag_device_remove,
> +};
I take it the core bus code handles reference-counting users of the bus?
> +struct hisi_djtag_client *hisi_djtag_new_device(struct hisi_djtag_host *host,
> + struct device_node *node)
> +{
> + struct hisi_djtag_client *client;
> + int status;
> +
> + client = kzalloc(sizeof(*client), GFP_KERNEL);
> + if (!client)
> + return NULL;
> +
> + client->host = host;
> +
> + client->dev.parent = &client->host->dev;
> + client->dev.bus = &hisi_djtag_bus;
> + client->dev.type = &hisi_djtag_client_type;
> + client->dev.of_node = node;
I suspect that we should do:
client->dev.of_node = of_node_get(node);
... so that it's guaranteed we retain a reference.
> + snprintf(client->name, DJTAG_CLIENT_NAME_LEN, "%s%s",
> + DJTAG_PREFIX, node->name);
> + dev_set_name(&client->dev, "%s", client->name);
> +
> + status = device_register(&client->dev);
> + if (status < 0) {
> + pr_err("error adding new device, status=%d\n", status);
> + kfree(client);
> + return NULL;
> + }
> +
> + return client;
> +}
> +
> +static struct hisi_djtag_client *hisi_djtag_of_register_device(
> + struct hisi_djtag_host *host,
> + struct device_node *node)
> +{
> + struct hisi_djtag_client *client;
> +
> + client = hisi_djtag_new_device(host, node);
> + if (client == NULL) {
> + dev_err(&host->dev, "error registering device %s\n",
> + node->full_name);
> + of_node_put(node);
I don't think this should be here, given what djtag_register_devices()
does.
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return client;
> +}
> +
> +static void djtag_register_devices(struct hisi_djtag_host *host)
> +{
> + struct device_node *node;
> + struct hisi_djtag_client *client;
> +
> + if (!host->of_node)
> + return;
> +
> + for_each_available_child_of_node(host->of_node, node) {
> + if (of_node_test_and_set_flag(node, OF_POPULATED))
> + continue;
> + client = hisi_djtag_of_register_device(host, node);
> + list_add(&client->next, &host->client_list);
> + }
> +}
Given hisi_djtag_of_register_device() can return ERR_PTR(-EINVAL), the
list_add is not safe.
> +static int djtag_host_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct hisi_djtag_host *host;
> + const struct of_device_id *of_id;
> + struct resource *res;
> + int rc;
> +
> + of_id = of_match_device(djtag_of_match, dev);
> + if (!of_id)
> + return -EINVAL;
> +
> + host = kzalloc(sizeof(*host), GFP_KERNEL);
> + if (!host)
> + return -ENOMEM;
> +
> + host->of_node = dev->of_node;
host->of_node = of_node_get(dev->of_node);
> + host->djtag_readwrite = of_id->data;
> + spin_lock_init(&host->lock);
> +
> + INIT_LIST_HEAD(&host->client_list);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "No reg resorces!\n");
> + kfree(host);
> + return -EINVAL;
> + }
> +
> + if (!resource_size(res)) {
> + dev_err(&pdev->dev, "Zero reg entry!\n");
> + kfree(host);
> + return -EINVAL;
> + }
> +
> + host->sysctl_reg_map = devm_ioremap_resource(dev, res);
> + if (IS_ERR(host->sysctl_reg_map)) {
> + dev_warn(dev, "Unable to map sysctl registers.\n");
> + kfree(host);
> + return -EINVAL;
> + }
Please have a common error path rather than duplicating this free.
e.g. at the end of the function have:
err_free:
kfree(host);
return err;
... and in cases like this, have:
if (whatever()) {
dev_warn(dev, "this failed xxx\n");
err = -EINVAL;
goto err_free;
}
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/6] mfd: dt: Add bindings for the Aspeed SoC Display Controller (GFX)
From: Rob Herring @ 2016-11-10 17:40 UTC (permalink / raw)
To: Joel Stanley
Cc: Andrew Jeffery, Lee Jones, Linus Walleij, Mark Rutland,
linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <CACPK8XeG2WnpNuHUf9VD+mZenWboq-4wei=LOkN4qVR72QbGXQ@mail.gmail.com>
On Wed, Nov 9, 2016 at 9:19 PM, Joel Stanley <joel@jms.id.au> wrote:
> On Thu, Nov 10, 2016 at 4:56 AM, Rob Herring <robh@kernel.org> wrote:
>> On Thu, Nov 03, 2016 at 01:07:57AM +1030, Andrew Jeffery wrote:
>>> The Aspeed SoC Display Controller is presented as a syscon device to
>>> arbitrate access by display and pinmux drivers. Video pinmux
>>> configuration on fifth generation SoCs depends on bits in both the
>>> System Control Unit and the Display Controller.
>>>
>>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>>> ---
>>> Documentation/devicetree/bindings/mfd/aspeed-gfx.txt | 17 +++++++++++++++++
>>
>> The register space can't be split to 2 nodes?
>
> Do you mean splitting the GFX IP and enable register into two nodes?
>
> We can't. Pinmux needs to check bit 6 and 7 in GFX064, which is in the
> middle the IP block:
>
> GFX060: CRT Control Register I
> GFX064: CRT Control Register II
> GFX068: CRT Status Register
> GFX06C: CRT Misc Setting Register
Okay.
>>> +The Aspeed SoC Display Controller primarily does as its name suggests, but also
>>> +participates in pinmux requests on the g5 SoCs. It is therefore considered a
>>> +syscon device.
>>> +
>>> +Required properties:
>>> +- compatible: "aspeed,ast2500-gfx", "syscon"
>>
>> I think perhaps we should drop the syscon here and the driver should
>> just register as a syscon.
>
> We want the regmap to be present whenever the GFX driver or pinmux is
> loaded. If we register it in pinmux but chose to not build in that
> driver, we lack the regmap. Same for the case where a user builds in
> the GFX driver and not pinmux. I think this means we want the syscon
> compatible string, unless my understanding is wrong?
Right.
Acked-by: Rob Herring <robh@kernel.org>
Rob
^ permalink raw reply
* Re: [PATCH v3 00/10] Add DT support for ohci-da8xx
From: Axel Haslam @ 2016-11-10 17:28 UTC (permalink / raw)
To: Greg KH
Cc: devicetree, David Lechner, Kevin Hilman, linux-usb, Sekhar Nori,
linux-kernel, robh+dt, Alan Stern, linux-arm-kernel
In-Reply-To: <20161110120253.GA5385@kroah.com>
On Thu, Nov 10, 2016 at 1:02 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Nov 08, 2016 at 05:37:41PM +0100, Axel Haslam wrote:
>> Hi,
>>
>> On Mon, Nov 7, 2016 at 9:39 PM, Axel Haslam <ahaslam@baylibre.com> wrote:
>> > The purpose of this patch series is to add DT support for the davinci
>> > ohci driver.
>> >
>>
>> To make it easier to review. I will split the arch/arm and driver
>> patches into separate series.
>
> I don't think it's easier, as now I have no idea what order, or what
> tree it should go through :(
Hi Greg,
i will resend the series one by one and wait for each to be merged
before sending the next, so that there is no confusion on the ordering
or on what tree they should apply.
Regards
Axel.
>
> I'm guessing not mine, so all are now deleted from my patch queue...
>
> greg k-h
^ permalink raw reply
* Re: [RESEND PATCH v1 02/11] dt-bindings: hisi: Add Hisilicon HiP05/06/07 Sysctrl and Djtag dts bindings
From: Mark Rutland @ 2016-11-10 17:23 UTC (permalink / raw)
To: Anurup M
Cc: devicetree, linux-arm-kernel, linux-doc, will.deacon, corbet,
catalin.marinas, robh+dt, arnd, f.fainelli, rmk+kernel, krzk,
anurup.m, zhangshaokun, tanxiaojun, xuwei5, sanil.kumar,
john.garry, gabriele.paoloni, shiju.jose, wangkefeng.wang,
guohanjun, shyju.pv, linuxarm
In-Reply-To: <1478151727-20250-3-git-send-email-anurup.m@huawei.com>
Hi,
On Thu, Nov 03, 2016 at 01:41:58AM -0400, Anurup M wrote:
> From: Tan Xiaojun <tanxiaojun@huawei.com>
>
> 1) Add Hisilicon HiP05/06/07 CPU and ALGSUB system controller dts
> bindings.
> 2) Add Hisilicon Djtag dts binding.
>
> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
> Signed-off-by: Anurup M <anurup.m@huawei.com>
> ---
> .../bindings/arm/hisilicon/hisilicon.txt | 82 ++++++++++++++++++++++
> 1 file changed, 82 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> index 7df79a7..341cbb9 100644
> --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
> @@ -270,3 +270,85 @@ Required Properties:
> [1]: bootwrapper size
> [2]: relocation physical address
> [3]: relocation size
> +-----------------------------------------------------------------------
> +The Hisilicon Djtag in CPU die is an independent component which connects with
> +some other components in the SoC by Debug Bus. This driver can be configured
> +to access the registers of connecting components (like L3 cache, l3 cache PMU
> + etc.) during real time debugging by sysctrl. These components appear as child
> +nodes of djtag.
Please put the djtag binding in a new file.
It's clearly unrelated to many other things in this file, which should
also be split out.
> +The Hip05/06/07 CPU system controller(sysctrl) support to manage some important
> +components (such as clock, reset, soft reset, secure debugger, etc.).
> +The CPU sysctrl registers in hip05/06/07 doesnot use syscon but will be mapped
> +by djtag driver for use by connecting components.
The djtag driver is irrelvant here.
If there's a realtionship between the two, please define that in the
binding rather than implicitly assuming it in the driver.
> +
> +Required properties:
> + - compatible : "hisilicon,hip05-cpu-djtag-v1"
> + - reg : Register address and size
> +
> +Hisilicon HiP06 djtag for CPU sysctrl
> +Required properties:
> +- compatible : "hisilicon,hip06-sysctrl", "syscon", "simple-mfd";
This looks messy. Why is this syscon and a simple-mfd?
We should kill off / deprecate the syscon binding. It's completely
meaningless.
> +- reg : Register address and size
> +- djtag :
> + - compatible : "hisilicon,hip06-cpu-djtag-v1"
> + - reg : Register address and size
> +
> +Hisilicon HiP07 djtag for CPU sysctrl
> +Required properties:
> + - compatible : "hisilicon,hip07-cpu-djtag-v2"
> + - reg : Register address and size
> +
> +Example:
> + /* for Hisilicon HiP05 djtag for CPU sysctrl */
> + djtag0: djtag@80010000 {
> + compatible = "hisilicon,hip05-cpu-djtag-v1";
> + reg = <0x0 0x80010000 0x0 0x10000>;
> +
> + /* For L3 cache PMU */
> + pmul3c0 {
> + compatible = "hisilicon,hisi-pmu-l3c-v1";
> + scl-id = <0x02>;
> + num-events = <0x16>;
> + num-counters = <0x08>;
> + module-id = <0x04>;
> + num-banks = <0x04>;
> + cfgen-map = <0x02 0x04 0x01 0x08>;
> + counter-reg = <0x170>;
> + evctrl-reg = <0x04>;
> + event-en = <0x1000000>;
> + evtype-reg = <0x140>;
> + };
This sub-node needs a binding document.
These properties are completely opaque
> + };
> +
> +-----------------------------------------------------------------------
> +The Hisilicon HiP05/06/07 ALGSUB system controller(sysctrl) is in IO die
> +of SoC. It has a similar function as the Hisilicon HiP05/06/07 CPU system
> +controller in CPU die and it manage different components, like RSA, etc.
> +The Hisilicon Djtag in IO die has a similar function as in CPU die and maps
> +the sysctrl registers for use by connecting components.
> +All connecting components shall appear as child nodes of djtag.
I don't follow the above. This describes an ALGSUB system controllerm
but the documentation below is all about djtag.
Thanks,
Mark.
> +Hisilicon HiP05 djtag for ALGSUB sysctrl
> +Required properties:
> + - compatible : "hisilicon,hip05-io-djtag-v1"
> + - reg : Register address and size
> +
> +Hisilicon HiP06 djtag for ALGSUB sysctrl
> +Required properties:
> + - compatible : "hisilicon,hip06-io-djtag-v2"
> + - reg : Register address and size
> +
> +Hisilicon HiP07 djtag for ALGSUB sysctrl
> +Required properties:
> + - compatible : "hisilicon,hip07-io-djtag-v2"
> + - reg : Register address and size
> +
> +Example:
> + /* for Hisilicon HiP05 djtag for alg sysctrl */
> + djtag0: djtag@d0000000 {
> + compatible = "hisilicon,hip05-io-djtag-v1";
> + reg = <0x0 0xd0000000 0x0 0x10000>;
> + };
> --
> 2.1.4
>
^ permalink raw reply
* [PATCH] lm87: Allow LM87_REG_CHANNEL_MODE to be set via DT
From: Jason Gunthorpe @ 2016-11-10 17:19 UTC (permalink / raw)
To: Guenter Roeck; +Cc: devicetree, linux-kernel, linux-hwmon
This compliments the existing scheme that lets it be set via
platform_data.
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
Documentation/devicetree/bindings/hwmon/lm87.txt | 27 ++++++++++++++++++++++++
drivers/hwmon/lm87.c | 8 ++++++-
2 files changed, 34 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/lm87.txt
diff --git a/Documentation/devicetree/bindings/hwmon/lm87.txt b/Documentation/devicetree/bindings/hwmon/lm87.txt
new file mode 100644
index 00000000000000..4ea2646ce8d2f0
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/lm87.txt
@@ -0,0 +1,27 @@
+* LM87 I2C Temperature and Fan sensor
+
+Required properties:
+- compatible: should contain one of
+ * "national,lm87"
+ * "ti,lm87"
+
+Optional properties:
+- channel-mode: Write the specified value to register 0x16. If not specified
+ the driver will read the value from the chip at startup.
+
+ The value controls how the driver operates certain multi-use
+ pins, eg bit 2 will select between temp_input3 or
+ in0&in5 operation.
+
+See Documentation/devicetree/bindings/i2c/i2c.txt for more required and
+optional properties.
+
+Example:
+
+i2c_master {
+ temperature-sensor@0 {
+ compatible = "ti,lm87";
+ channel-mode = <0x4>;
+ reg = <0>;
+ };
+};
diff --git a/drivers/hwmon/lm87.c b/drivers/hwmon/lm87.c
index 81cb898245a107..ccc0c913b4534e 100644
--- a/drivers/hwmon/lm87.c
+++ b/drivers/hwmon/lm87.c
@@ -853,9 +853,15 @@ static void lm87_restore_config(void *arg)
static int lm87_init_client(struct i2c_client *client)
{
struct lm87_data *data = i2c_get_clientdata(client);
+ u32 channel;
int rc;
- if (dev_get_platdata(&client->dev)) {
+ if (client->dev.of_node &&
+ !of_property_read_u32(client->dev.of_node, "channel-mode",
+ &channel)) {
+ data->channel = channel;
+ lm87_write_value(client, LM87_REG_CHANNEL_MODE, data->channel);
+ } else if (dev_get_platdata(&client->dev)) {
data->channel = *(u8 *)dev_get_platdata(&client->dev);
lm87_write_value(client,
LM87_REG_CHANNEL_MODE, data->channel);
--
2.7.4
^ permalink raw reply related
* Re: [RESEND PATCH] regulator: pwm: fix syntax errors in the examples
From: Peter Rosin @ 2016-11-10 17:05 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Rob Herring,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161110161801.7462udnohbaik5nr-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On 2016-11-10 17:18, Mark Brown wrote:
> On Thu, Nov 10, 2016 at 11:25:27AM +0100, Peter Rosin wrote:
>> While at it, clean up some other things as well.
>
> This isn't a useful changelog, it's just saying that there's some
> random changes for no reason. Please make proper patches with
> changelogs that explain them.
It's a very uninteresting patch, I guess I made a changelog to match.
But I can't help myself from asking a few qustions...
Patch*es* and changelog*s*?
Oh, you want me to split into a series of several patches? One for the
quotes, one for the "../regultor/" removal, one for the empty
comment removal and one for the s/pwm_reglator/pwm-regulator/ thing?
Be careful what you ask for, you might get it...
Or are you saying that patches that I make have poor descriptions
in general?
Anyway, I'm done with that file, if you want crappy docs, by all
means throw the patch away.
Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox