Re: [PATCH v1 1/2] soc: ti: display firmware file name as part of boot log
On 9/4/15 5:46 PM, Murali Karicheri wrote: To help the user, print the PDSP file name as part of knav_queue_load_pdsp(). This will be useful for users to know what version of the firmware is loaded to PDSP. Also update the document for the location of the QMSS accumulator PDSP firmware. Signed-off-by: Murali Karicheri --- Looks fine. Will pick both the patches. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] arm64: Support Hisilicon Hip05-D02 board
Hip05-D02 Development Board is based on Cortex-A57, this patchset contains initial support for Hip05-D02 Soc and Board. Initial support is minimal and includes just the arch configuration, device tree configuration. PSCI is enabled in device tree and there is no problem to boot all the 16 cores, and the CPU hotplug is also working now, you can download and compile the latest firmware based on the following link to run this patch set. https://github.com/hisilicon/estuary/blob/master/README v1->v2: - 1. Change the compatible name for per CPU. 2. Remove the GIC_CPU_MASK_SIMPLE(xx) which is not used for gicv3 from hip05.dtsi. Ding Tianhong (2): arm64: hip05-d02: Document devicetree bindings for Hisilicon Hip05-D02 Board arm64: dts: add dts files for Hisilicon Hip05-D02 Development Board .../bindings/arm/hisilicon/hisilicon.txt | 4 + arch/arm64/boot/dts/hisilicon/Makefile | 2 +- arch/arm64/boot/dts/hisilicon/hip05-d02.dts| 36 +++ arch/arm64/boot/dts/hisilicon/hip05.dtsi | 271 + 4 files changed, 312 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/boot/dts/hisilicon/hip05-d02.dts create mode 100644 arch/arm64/boot/dts/hisilicon/hip05.dtsi -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] arm64: hip05-d02: Document devicetree bindings for Hisilicon Hip05-D02 Board
This patch adds documentation for the devicetree bindings used by the DT files of Hisilicon Hip05-D02 development board. Signed-off-by: Ding Tianhong --- Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt index c431c67..f98b39d 100644 --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt @@ -20,6 +20,10 @@ HiKey Board Required root node properties: - compatible = "hisilicon,hi6220-hikey", "hisilicon,hi6220"; +HiP05 D02 Board +Required root node properties: + - compatible = "hisilicon,hip05-d02"; + Hisilicon system controller Required properties: -- 1.9.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] arm64: dts: add dts files for Hisilicon Hip05-D02 Development Board
Add initial dtsi file to support Hisilicon Hip05-D02 Board with
support of CPUs in four clusters and each cluster has quard Cortex-A57.
Also add dts file to support Hip05-D02 development board.
Signed-off-by: Ding Tianhong
Signed-off-by: Kefeng Wang
---
arch/arm64/boot/dts/hisilicon/Makefile | 2 +-
arch/arm64/boot/dts/hisilicon/hip05-d02.dts | 36
arch/arm64/boot/dts/hisilicon/hip05.dtsi| 271
3 files changed, 308 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/boot/dts/hisilicon/hip05-d02.dts
create mode 100644 arch/arm64/boot/dts/hisilicon/hip05.dtsi
diff --git a/arch/arm64/boot/dts/hisilicon/Makefile
b/arch/arm64/boot/dts/hisilicon/Makefile
index fa81a6e..cd158b8 100644
--- a/arch/arm64/boot/dts/hisilicon/Makefile
+++ b/arch/arm64/boot/dts/hisilicon/Makefile
@@ -1,4 +1,4 @@
-dtb-$(CONFIG_ARCH_HISI) += hi6220-hikey.dtb
+dtb-$(CONFIG_ARCH_HISI) += hi6220-hikey.dtb hip05-d02.dtb
always := $(dtb-y)
subdir-y := $(dts-dirs)
diff --git a/arch/arm64/boot/dts/hisilicon/hip05-d02.dts
b/arch/arm64/boot/dts/hisilicon/hip05-d02.dts
new file mode 100644
index 000..ae34e25
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hip05-d02.dts
@@ -0,0 +1,36 @@
+/**
+ * dts file for Hisilicon D02 Development Board
+ *
+ * Copyright (C) 2014,2015 Hisilicon Ltd.
+ *
+ * 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
+ * publishhed by the Free Software Foundation.
+ *
+ */
+
+/dts-v1/;
+
+#include "hip05.dtsi"
+
+/ {
+ model = "Hisilicon Hip05 D02 Development Board";
+ compatible = "hisilicon,hip05-d02";
+
+ memory@ {
+ device_type = "memory";
+ reg = <0x0 0x 0x0 0x8000>;
+ };
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+};
+
+&uart0 {
+ status = "ok";
+};
diff --git a/arch/arm64/boot/dts/hisilicon/hip05.dtsi
b/arch/arm64/boot/dts/hisilicon/hip05.dtsi
new file mode 100644
index 000..92d00f4
--- /dev/null
+++ b/arch/arm64/boot/dts/hisilicon/hip05.dtsi
@@ -0,0 +1,271 @@
+/**
+ * dts file for Hisilicon D02 Development Board
+ *
+ * Copyright (C) 2014,2015 Hisilicon Ltd.
+ *
+ * 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
+ * publishhed by the Free Software Foundation.
+ *
+ */
+
+#include
+
+/ {
+ compatible = "hisilicon,hip05-d02";
+ interrupt-parent = <&gic>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ psci {
+ compatible = "arm,psci-0.2";
+ method = "smc";
+ };
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu-map {
+ cluster0 {
+ core0 {
+ cpu = <&cpu0>;
+ };
+ core1 {
+ cpu = <&cpu1>;
+ };
+ core2 {
+ cpu = <&cpu2>;
+ };
+ core3 {
+ cpu = <&cpu3>;
+ };
+ };
+ cluster1 {
+ core0 {
+ cpu = <&cpu4>;
+ };
+ core1 {
+ cpu = <&cpu5>;
+ };
+ core2 {
+ cpu = <&cpu6>;
+ };
+ core3 {
+ cpu = <&cpu7>;
+ };
+ };
+ cluster2 {
+ core0 {
+ cpu = <&cpu8>;
+ };
+ core1 {
+ cpu = <&cpu9>;
+ };
+ core2 {
+ cpu = <&cpu10>;
+ };
+ core3 {
+ cpu = <&cpu11>;
+ };
+ };
+ cluster3 {
+ core0 {
+ cpu = <&cpu12>;
+ };
+ core1 {
+ cpu = <&cpu13>;
+ };
+
Re: [PATCH] of_pci_irq: Silence bogus "of_irq_parse_pci() failed ..." messages.
On 9/4/2015 6:40 PM, David Daney wrote:
> On 09/04/2015 06:14 PM, Frank Rowand wrote:
>> On 9/4/2015 12:12 PM, David Daney wrote:
>>> From: David Daney
>>>
>>> It is perfectly legitimate for a PCI device to have an
>>> PCI_INTERRUPT_PIN value of zero. This happens if the device doesn't
>>> use interrupts, or on PCIe devices, where only MSI/MSI-X are
>>> supported.
>>>
>>> Silence the annoying "of_irq_parse_pci() failed with rc=-19" error
>>> messages by making them conditional on !-ENODEV (which can only be
>>> produced in the PCI_INTERRUPT_PIN == 0 case).
>>>
>>> Signed-off-by: David Daney
>>> ---
>>> drivers/of/of_pci_irq.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
>>> index 1710d9d..33d242a 100644
>>> --- a/drivers/of/of_pci_irq.c
>>> +++ b/drivers/of/of_pci_irq.c
>>> @@ -106,7 +106,9 @@ int of_irq_parse_and_map_pci(const struct pci_dev *dev,
>>> u8 slot, u8 pin)
>>>
>>> ret = of_irq_parse_pci(dev, &oirq);
>>> if (ret) {
>>> -dev_err(&dev->dev, "of_irq_parse_pci() failed with rc=%d\n", ret);
>>> +if (ret != -ENODEV)
>>> +dev_err(&dev->dev,
>>> +"of_irq_parse_pci() failed with rc=%d\n", ret);
>>> return 0; /* Proper return code 0 == NO_IRQ */
>>> }
>>>
>>>
>>
>> It is not safe to assume that the functions that of_irq_parse_pci() calls
>> will never be modified to return -ENODEV, thus resulting in
>> of_irq_parse_pci()
>> returning -ENODEV for a reason other than PCI_INTERRUPT_PIN == 0.
>>
>
> Since the current implementation *only ever* returns -ENODEV for
> PCI_INTERRUPT_PIN == 0, we could just document that behavior, and not hack up
> the APIs by adding a second return channel from the function.
And for each function that of_irq_parse_pci() calls and returns status from the
function call you would need to document that behavior, and so on recursively.
For example, you would need to document of_irq_parse_one(). And that returns
status
from more function calls, so you would need to document of_irq_parse_oldworld(),
of_parse_phandle_with_args(), of_irq_parse_raw(), and then anything they call.
>
> The additional change on top of my patch would be to add a comment describing
> this behavior.
>
> David Daney.
< snip >
-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] of_pci_irq: Silence bogus "of_irq_parse_pci() failed ..." messages.
On 09/04/2015 06:14 PM, Frank Rowand wrote:
On 9/4/2015 12:12 PM, David Daney wrote:
From: David Daney
It is perfectly legitimate for a PCI device to have an
PCI_INTERRUPT_PIN value of zero. This happens if the device doesn't
use interrupts, or on PCIe devices, where only MSI/MSI-X are
supported.
Silence the annoying "of_irq_parse_pci() failed with rc=-19" error
messages by making them conditional on !-ENODEV (which can only be
produced in the PCI_INTERRUPT_PIN == 0 case).
Signed-off-by: David Daney
---
drivers/of/of_pci_irq.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index 1710d9d..33d242a 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -106,7 +106,9 @@ int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8
slot, u8 pin)
ret = of_irq_parse_pci(dev, &oirq);
if (ret) {
- dev_err(&dev->dev, "of_irq_parse_pci() failed with rc=%d\n",
ret);
+ if (ret != -ENODEV)
+ dev_err(&dev->dev,
+ "of_irq_parse_pci() failed with rc=%d\n", ret);
return 0; /* Proper return code 0 == NO_IRQ */
}
It is not safe to assume that the functions that of_irq_parse_pci() calls
will never be modified to return -ENODEV, thus resulting in of_irq_parse_pci()
returning -ENODEV for a reason other than PCI_INTERRUPT_PIN == 0.
Since the current implementation *only ever* returns -ENODEV for
PCI_INTERRUPT_PIN == 0, we could just document that behavior, and not
hack up the APIs by adding a second return channel from the function.
The additional change on top of my patch would be to add a comment
describing this behavior.
David Daney.
A more robust solution would be something like:
(1) Change of_irq_parse_pci() to _of_irq_parse_pci(), adding an argument and
use it to report the case of PCI_INTERRUPT_PIN == 0.
static int _of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args
*out_irq, int *no_pin)
{
...
*no_pin = 0;
...
/* No pin, exit */
if (pin == 0) {
*no_pin = 1;
return -ENODEV;
}
...
int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args
*out_irq)
{
int no_pin;
return _of_irq_parse_pci(pdev, out_irq, &no_pin)
}
(2) Then the fix to of_irq_parse_and_map_pci() would be:
+ int no_pin;
ret = of_irq_parse_pci(dev, &oirq, &no_pin);
if (ret) {
- dev_err(&dev->dev, "of_irq_parse_pci() failed with rc=%d\n",
ret);
+ if (!no_pin)
+ dev_err(&dev->dev,
+ "of_irq_parse_pci() failed with rc=%d\n", ret);
return 0; /* Proper return code 0 == NO_IRQ */
}
I'm not sure I like my solution, there might be a better way.
I also noticed another bug while looking at of_irq_parse_pci(). It returns
the non-zero return value from pci_read_config_byte(). But that value is
one of the PCI function error values from include/linux/pci.h, such as:
#define PCIBIOS_BAD_REGISTER_NUMBER 0x87
instead of a negative errno.
-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] of_pci_irq: Silence bogus "of_irq_parse_pci() failed ..." messages.
On 9/4/2015 12:12 PM, David Daney wrote:
> From: David Daney
>
> It is perfectly legitimate for a PCI device to have an
> PCI_INTERRUPT_PIN value of zero. This happens if the device doesn't
> use interrupts, or on PCIe devices, where only MSI/MSI-X are
> supported.
>
> Silence the annoying "of_irq_parse_pci() failed with rc=-19" error
> messages by making them conditional on !-ENODEV (which can only be
> produced in the PCI_INTERRUPT_PIN == 0 case).
>
> Signed-off-by: David Daney
> ---
> drivers/of/of_pci_irq.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
> index 1710d9d..33d242a 100644
> --- a/drivers/of/of_pci_irq.c
> +++ b/drivers/of/of_pci_irq.c
> @@ -106,7 +106,9 @@ int of_irq_parse_and_map_pci(const struct pci_dev *dev,
> u8 slot, u8 pin)
>
> ret = of_irq_parse_pci(dev, &oirq);
> if (ret) {
> - dev_err(&dev->dev, "of_irq_parse_pci() failed with rc=%d\n",
> ret);
> + if (ret != -ENODEV)
> + dev_err(&dev->dev,
> + "of_irq_parse_pci() failed with rc=%d\n", ret);
> return 0; /* Proper return code 0 == NO_IRQ */
> }
>
>
It is not safe to assume that the functions that of_irq_parse_pci() calls
will never be modified to return -ENODEV, thus resulting in of_irq_parse_pci()
returning -ENODEV for a reason other than PCI_INTERRUPT_PIN == 0.
A more robust solution would be something like:
(1) Change of_irq_parse_pci() to _of_irq_parse_pci(), adding an argument and
use it to report the case of PCI_INTERRUPT_PIN == 0.
static int _of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args
*out_irq, int *no_pin)
{
...
*no_pin = 0;
...
/* No pin, exit */
if (pin == 0) {
*no_pin = 1;
return -ENODEV;
}
...
int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args
*out_irq)
{
int no_pin;
return _of_irq_parse_pci(pdev, out_irq, &no_pin)
}
(2) Then the fix to of_irq_parse_and_map_pci() would be:
+ int no_pin;
> ret = of_irq_parse_pci(dev, &oirq, &no_pin);
> if (ret) {
> - dev_err(&dev->dev, "of_irq_parse_pci() failed with rc=%d\n",
> ret);
> + if (!no_pin)
> + dev_err(&dev->dev,
> + "of_irq_parse_pci() failed with rc=%d\n", ret);
> return 0; /* Proper return code 0 == NO_IRQ */
> }
I'm not sure I like my solution, there might be a better way.
I also noticed another bug while looking at of_irq_parse_pci(). It returns
the non-zero return value from pci_read_config_byte(). But that value is
one of the PCI function error values from include/linux/pci.h, such as:
#define PCIBIOS_BAD_REGISTER_NUMBER 0x87
instead of a negative errno.
-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] spmi-pmic-arb: support configurable number of peripherals
On 09/04, Gilad Avidov wrote:
> On Thu, 3 Sep 2015 17:16:30 -0700
> Stephen Boyd wrote:
>
> > On 09/03, Gilad Avidov wrote:
> > > + supported by HW. Default (minimum
> > > supported) is 128. +
> > > +Example V1 PMIC-Arbiter:
> > >
> > > spmi {
> > > compatible = "qcom,spmi-pmic-arb";
> > > @@ -62,4 +66,32 @@ Example:
> > >
> > > interrupt-controller;
> > > #interrupt-cells = <4>;
> > > +
> > > + qcom,max-peripherals = <256>;
> >
> > If it's v1 isn't it always 128? So having 256 here is just
> > confusing.
>
> Hi Stephen,
>
> Actually some v1 chipsets, such as Dragonboard APQ8074, support 256
> peripherals.
Then the commit text should be updated. It only mentions that v2
HW has sub-versions which support more than 128.
>
> >
> > > + };
> > > +
> > > @@ -129,14 +131,15 @@ struct spmi_pmic_arb_dev {
> > > u8 channel;
> > > int irq;
> > > u8 ee;
> > > - u8 min_apid;
> > > - u8 max_apid;
> > > - u32
> > > mapping_table[SPMI_MAPPING_TABLE_LEN];
> > > + u16 min_irq_apid;
> > > + u16 max_irq_apid;
> > > + u16 max_apid;
> > > + u32 *mapping_table;
> > > struct irq_domain *domain;
> > > struct spmi_controller *spmic;
> > > - u16 apid_to_ppid[256];
> > > + u16 *irq_apid_to_ppid;
> >
> > Please drop all this renaming noise, or at the least, put it in a
> > different patch. More than half the patch is just changing the
> > names of these variables for what seems like no reason.
> >
>
> Regarding the change of max/min_apid to max/min_irq_apid:
> max_apid was already used but the name does not make good sense. Since
> really it is not the max_apid supported. Instead it is the largest apid
> which interrupt is currently requested for. But now we need a value that
> is actually the maximum supported apid. This is why repurposed max_apid
> and corrected the previous naming.
max_apid in this patch is only used in probe, so there's no need
for that variable to be a member of the structure. Meaning we can
leave max_apid alone and call this new variable max_periphs or
something like that and leave it local to the probe function.
>
> > > const struct pmic_arb_ver_ops *ver_ops;
> > > - u8 *ppid_to_chan;
> > > + u16 *ppid_to_chan;
> > > };
> > >
> > > struct spmi_pmic_arb_dev *pa = irq_get_handler_data(irq);
> > > struct irq_chip *chip = irq_get_chip(irq);
> > > void __iomem *intr = pa->intr;
> > > - int first = pa->min_apid >> 5;
> > > - int last = pa->max_apid >> 5;
> > > + int first = pa->min_irq_apid >> 5;
> > > + int last = pa->max_irq_apid >> 5;
> > > u32 status;
> > > int i, id;
> > >
> > > @@ -903,14 +915,30 @@ static int spmi_pmic_arb_probe(struct
> > > platform_device *pdev)
> > > pa->ee = ee;
> > >
> > > - for (i = 0; i < ARRAY_SIZE(pa->mapping_table); ++i)
> > > - pa->mapping_table[i] = readl_relaxed(
> > > - pa->cnfg +
> > > SPMI_MAPPING_TABLE_REG(i));
> > > + pa->irq_apid_to_ppid = devm_kzalloc(&ctrl->dev,
> > > pa->max_apid *
> > > +
> > > sizeof(*pa->irq_apid_to_ppid),
> > > + GFP_KERNEL);
> > > + if (!pa->irq_apid_to_ppid) {
> > > + err = -ENOMEM;
> > > + goto err_put_ctrl;
> > > + }
> > > +
> > > + pa->mapping_table = devm_kzalloc(&ctrl->dev,
> > > + (pa->max_apid - 1) *
> > > sizeof(u32),
> > > + GFP_KERNEL);
> > > + if (!pa->mapping_table) {
> > > + err = -ENOMEM;
> > > + goto err_put_ctrl;
> > > + }
> > > +
> > > + for (i = 0; i < (pa->max_apid - 1); ++i)
> > > + pa->mapping_table[i] = readl_relaxed(pa->cnfg +
> > > +
> > > SPMI_MAPPING_TABLE_REG(i));
>
> > we're searching through the mapping table we can cache the value
> > from the register if the entry isn't 0. This delays the
>
> Greedy approach (reading upto 512 registers from hw) have very small
> upfront penalty since sequential reads take place very fast
> and HW acceleration (prefetching) will improve it. A lazy approach will
> required much more complex algorithm (for example any value
> including zero is valid. So we will need to augment a marker for
> unassigned entries) and prefetching will not help.
That's solvable with a bitmap of 512 entries, not too much of a
complicated algorithm.
We're sort of screwed now that we have to find the channel for a
ppid by reading X number of PMIC_ARB_REG_CHNL registers. We could
lazily do that too though, where we fill in the array linearly
with what we read until we find the ppid we're looking for. We
would need to do a bitmap trick again, but since we know ppid is
at most going to use 12 bits, we can set the highest bit to
indicate the ppid_to_chan entry is valid.
Re: [PATCH 2/2] mtd: spi-nor: Add driver for Cadence Quad SPI Flash Controller.
Hi, On 08/21/2015 02:20 AM, Marek Vasut wrote: > From: Graham Moore > > Add support for the Cadence QSPI controller. This controller is > present in the Altera SoCFPGA SoCs and this driver has been tested > on the Cyclone V SoC. can we add info about the modes supported/not supported like direct mode, indirect etc. > > Signed-off-by: Graham Moore > Signed-off-by: Marek Vasut > Cc: Alan Tull > Cc: Brian Norris > Cc: David Woodhouse > Cc: Dinh Nguyen > Cc: Graham Moore > Cc: Vikas MANOCHA > Cc: Yves Vandervennet > Cc: devicetree@vger.kernel.org > --- > drivers/mtd/spi-nor/Kconfig | 11 + > drivers/mtd/spi-nor/Makefile |1 + > drivers/mtd/spi-nor/cadence-quadspi.c | 1260 > + > 3 files changed, 1272 insertions(+) > create mode 100644 drivers/mtd/spi-nor/cadence-quadspi.c > > V2: use NULL instead of modalias in spi_nor_scan call > V3: Use existing property is-decoded-cs instead of creating duplicate. > V4: Support Micron quad mode by snooping command stream for EVCR command > and subsequently configuring Cadence controller for quad mode. > V5: Clean up sparse and smatch complaints. Remove snooping of Micron > quad mode. Add comment on XIP mode bit and dummy clock cycles. Set > up SRAM partition at 1:1 during init. > V6: Remove dts patch that was included by mistake. Incorporate Vikas's > comments regarding fifo width, SRAM partition setting, and trigger > address. Trigger address was added as an unsigned int, as it is not > an IO resource per se, and does not need to be mapped. Also add > Marek Vasut's workaround for picking up OF properties on subnodes. > V7: - Perform coding-style cleanup and type fixes. Remove ugly QSPI_*() > macros and replace them with functions. Get rid of unused variables. > - Implement support for nor->set_protocol() to handle Quad-command, > this patch now depends on the following patch: > mtd: spi-nor: notify (Q)SPI controller about protocol change > - Replace that cqspi_fifo_read() disaster with plain old readsl() > and cqspi_fifo_write() tentacle horror with pretty writesl(). > - Remove CQSPI_SUPPORT_XIP_CHIPS, which is broken. > - Get rid of cqspi_find_chipselect() mess, instead just place the > struct cqspi_st and chipselect number into struct cqspi_flash_pdata > and set nor->priv to the struct cqspi_flash_pdata of that particular > chip. > - Replace the odd math in calculate_ticks_for_ns() with DIV_ROUND_UP(). > - Make variables const where applicable. > V8: - Implement a function to wait for bit being set/unset for a given > period of time and use it to replace the ad-hoc bits of code. > - Configure the write underflow watermark to be 1/8 if FIFO size. > - Extract out the SPI NOR flash probing code into separate function > to clearly mark what will soon be considered a boilerplate code. > - Repair the handling of mode bits, which caused instability in V7. > - Clean up the interrupt handling > - Fix Kconfig help text and make the patch depend on OF and COMPILE_TEST. > > diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig > index 89bf4c1..ed253a2 100644 > --- a/drivers/mtd/spi-nor/Kconfig > +++ b/drivers/mtd/spi-nor/Kconfig > @@ -40,4 +40,15 @@ config SPI_NXP_SPIFI > Flash. Enable this option if you have a device with a SPIFI > controller and want to access the Flash as a mtd device. > > +config SPI_CADENCE_QUADSPI > + tristate "Cadence Quad SPI controller" > + depends on OF && COMPILE_TEST > + help > + Enable support for the Cadence Quad SPI Flash controller. > + > + Cadence QSPI is a specialized controller for connecting an SPI > + Flash over 1/2/4-bit wide bus. Enable this option if you have a > + device with a Cadence QSPI controller and want to access the > + Flash as an MTD device. > + > endif # MTD_SPI_NOR > diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile > index e5e..446c6b9 100644 > --- a/drivers/mtd/spi-nor/Makefile > +++ b/drivers/mtd/spi-nor/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o > +obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o > obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o > obj-$(CONFIG_SPI_NXP_SPIFI)+= nxp-spifi.o > diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c > b/drivers/mtd/spi-nor/cadence-quadspi.c > new file mode 100644 > index 000..8e024b8 > --- /dev/null > +++ b/drivers/mtd/spi-nor/cadence-quadspi.c > @@ -0,0 +1,1260 @@ > +/* > + * Driver for Cadence QSPI Controller > + * > + * Copyright Altera Corporation (C) 2012-2014. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > +
Re: [PATCH v2] ARM: shmobile: silk: add SDHI1 DT support
Hello. On 09/02/2015 05:29 AM, Simon Horman wrote: Define the SILK board dependent part of the SDHI1 (connected to micro-SD slot) device nodes along with the necessary voltage regulators. Based on the original patch by Vladimir Barinov . Signed-off-by: Sergei Shtylyov --- This patch is against 'renesas-devel-20150810-v4.2-rc6' tag of Simon Horman's 'renesas.git' repo plus the R8A7794/SILK QSPI patches just re-posted. It needs the R8A7794 GPIO patches in order to compile. Changes in version 2: - removed not working SDHI0 stuff, renamed the patch; - replaced SDHI1's "wp-gpios" property with "disable-wp". I am wondering if you could explain the motivation for the "disable-wp" update Please see the comment in mmc_sd_get_ro(). and weather it is appropriate for other r8a779* dts files. In case of micro-SD slots, yes. The MMC binding document says it should only be specified if the controller has WP detection logic. We, so far, seem to have been replying on the GPIOs despite this logic is present (according to the R-Car gen2 SDHI manuals I have). The driver will first call mmc_gpio_get_ro() and when that fails due to "wp-gpios" not being specified, it proceeds to reading the register but that is forbidden by TMIO_MMC_WRPROTECT_DISABLE flag set for the R-Car gen1/2 chips, so 0 is always returned from tmio_mmc_get_ro(). There seems to be no point in going that far (and doing the runtime PM dances) -- Alternatively, the driver could be fixed to check the flag before the RPM call unlike what it does now. and MMC_CAP2_NO_WRITE_PROTECT (set when "disable-wp" is specified) prohibits doing that... That sounds reasonable to me. I'm still wondering why TMIO_MMC_WRPROTECT_DISABLE is set for the R-Car SoCs. Morimoto-san, any comments? Your change logs are too terse. :-) Some more questions from my side: * What is the status of this patch Tested, working. * Can you prepare patches for r8a779* dts files for micro-SD slots? Ugh, I probably can but won't promise anything. MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Docs: dt: Add PCI MSI map bindings
Hi Mark, First of all: Thanks for working on this. I now have a prototype implementation for irq-gic-v3-its.c that is using this binding on Cavium's ThunderX platform. Q: Have you guys had any more thoughts on this that might require changing the binding? If not, I will be sending out my patches for your consideration. Thanks, David Daney On 07/27/2015 01:16 AM, Marc Zyngier wrote: On 23/07/15 17:52, Mark Rutland wrote: Currently msi-parent is used by a few bindings to describe the relationship between a PCI root complex and a single MSI controller, but this property does not have a generic binding document. Additionally, msi-parent is insufficient to describe more complex relationships between MSI controllers and devices under a root complex, where devices may be able to target multiple MSI controllers, or where MSI controllers use (non-probeable) sideband information to distinguish devices. This patch adds a generic binding for mapping PCI devices to MSI controllers. This document covers msi-parent, and a new msi-map property (specific to PCI*) which may be used to map devices (identified by their Requester ID) to sideband data for each MSI controller that they may target. Signed-off-by: Mark Rutland Acked-by: David Daney --- Documentation/devicetree/bindings/pci/pci-msi.txt | 220 ++ 1 file changed, 220 insertions(+) create mode 100644 Documentation/devicetree/bindings/pci/pci-msi.txt diff --git a/Documentation/devicetree/bindings/pci/pci-msi.txt b/Documentation/devicetree/bindings/pci/pci-msi.txt new file mode 100644 index 000..9b3cc81 --- /dev/null +++ b/Documentation/devicetree/bindings/pci/pci-msi.txt @@ -0,0 +1,220 @@ +This document describes the generic device tree binding for describing the +relationship between PCI devices and MSI controllers. + +Each PCI device under a root complex is uniquely identified by its Requester ID +(AKA RID). A Requester ID is a triplet of a Bus number, Device number, and +Function number. + +For the purpose of this document, when treated as a numeric value, a RID is +formatted such that: + +* Bits [15:8] are the Bus number. +* Bits [7:3] are the Device number. +* Bits [2:0] are the Function number. +* Any other bits required for padding must be zero. + +MSIs may be distinguished in part through the use of sideband data accompanying +writes. In the case of PCI devices, this sideband data may be derived from the +Requester ID. A mechanism is required to associate a device with both the MSI +controllers it can address, and the sideband data that will be associated with +its writes to those controllers. + +For generic MSI bindings, see +Documentation/devicetree/bindings/interrupt-controller/msi.txt. + + +PCI root complex + + +Optional properties +--- + +- msi-map: Maps a Requester ID to an MSI controller and associated + msi-specifier data. The property is an arbitrary number of tuples of + (rid-base,msi-controller,msi-base,length), where: + + * rid-base is a single cell describing the first RID matched by the entry. + + * msi-controller is a single phandle to an MSI controller + + * msi-base is an msi-specifier describing the msi-specifier produced for the +first RID matched by the entry. + + * length is a single cell describing how many consecutive RIDs are matched +following the rid-base. + + Any RID r in the interval [rid-base, rid-base + length) is associated with + the listed msi-controller, with the msi-specifier (r - rid-base + msi-base). + +- msi-map-mask: A mask to be applied to each Requester ID prior to being mapped + to an msi-specifier per the msi-map property. + +- msi-parent: Describes the MSI parent of the root complex itself. Where + the root complex and MSI controller do not pass sideband data with MSI + writes, this property may be used to describe the MSI controller(s) + used by PCI devices under the root complex, if defined as such in the + binding for the root complex. Right, this is where I'd expect some details about #msi-cells. Is it meant to be ignored? The lack of symmetry between the PCI and non-PCI use cases feels a bit inelegant (not to mention that it precludes having an unified parser for both cases). This otherwise looks good to me. Thanks, M. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 1/2] soc: ti: display firmware file name as part of boot log
To help the user, print the PDSP file name as part of
knav_queue_load_pdsp(). This will be useful for users to know what
version of the firmware is loaded to PDSP. Also update the
document for the location of the QMSS accumulator PDSP firmware.
Signed-off-by: Murali Karicheri
---
v1 : fixed firmware file names in documentation
.../bindings/soc/ti/keystone-navigator-qmss.txt | 20 +++-
drivers/soc/ti/knav_qmss_queue.c | 3 +++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
b/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
index d8e8cdb..ca0a1a7 100644
--- a/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
+++ b/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
@@ -221,7 +221,7 @@ qmss: qmss@2a4 {
#size-cells = <1>;
ranges;
pdsp0@0x2a1 {
- firmware = "keystone/qmss_pdsp_acc48_k2_le_1_0_0_8.fw";
+ firmware = "k2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin";
reg = <0x2a1 0x1000>,
<0x2a0f000 0x100>,
<0x2a0c000 0x3c8>,
@@ -230,3 +230,21 @@ qmss: qmss@2a4 {
};
};
}; /* qmss */
+
+Accumulator QMSS Channel using PDSP firmware
+
+The QMSS PDSP firmware support accumulator channel that can monitor a single
+queue or multiple contiguous queues. drivers/soc/ti/knav_qmss_acc.c is the
+driver that interface with the accumulator PDSP. This configures
+accumulator channels defined in DTS (example above) to monitor 1 or 32 queues
+per channel. More description on the firmware is available in CPPI/QMSS Low
+Level Driver document (docs/CPPI_QMSS_LLD_SDS.pdf) at
+ git://git.ti.com/keystone-rtos/qmss-lld.git
+
+k2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin firmware supports upto 48 accumulator
+channels. This firmware is available under firmware folder of the above repo
+under the name acc48_le.bin. To use copy the firmware image to lib/firmware
+folder of the initramfs or ubifs file system as
+k2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin and boot up the kernel. User would see
+"firmware file ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin downloaded for PDSP" in
+the boot up log if loading of firmware to PDSP is successful.
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 6d8646d..f26ce99 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -1526,6 +1526,9 @@ static int knav_queue_load_pdsp(struct knav_device *kdev,
pdsp->firmware, pdsp->name);
return ret;
}
+ dev_info(kdev->dev, "firmware file %s downloaded for PDSP\n",
+pdsp->firmware);
+
writel_relaxed(pdsp->id + 1, pdsp->command + 0x18);
/* download the firmware */
fwdata = (u32 *)fw->data;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 2/2] ARM: dts: keystone: enable accumulator channels
Add low priority accumulator channel that can monitor multiple QMSS
queues. User for example could use the accumular queue for Netcp
Rx completion. While at it, also add an extra line end of each top
level node in DTS to make it more readable.
Signed-off-by: Murali Karicheri
---
v1 - No change from initial version
arch/arm/boot/dts/k2e-netcp.dtsi | 24
arch/arm/boot/dts/k2hk-netcp.dtsi | 25 +
arch/arm/boot/dts/k2l-netcp.dtsi | 24
3 files changed, 73 insertions(+)
diff --git a/arch/arm/boot/dts/k2e-netcp.dtsi b/arch/arm/boot/dts/k2e-netcp.dtsi
index b13b3c9..1818929 100644
--- a/arch/arm/boot/dts/k2e-netcp.dtsi
+++ b/arch/arm/boot/dts/k2e-netcp.dtsi
@@ -72,7 +72,17 @@ qmss: qmss@2a4 {
qalloc-by-id;
};
};
+ accumulator {
+ acc-low-0 {
+ qrange = <480 32>;
+ accumulator = <0 47 16 2 50>;
+ interrupts = <0 226 0xf01>;
+ multi-queue;
+ qalloc-by-id;
+ };
+ };
};
+
descriptor-regions {
#address-cells = <1>;
#size-cells = <1>;
@@ -83,6 +93,20 @@ qmss: qmss@2a4 {
link-index = <0x4000>;
};
};
+
+ pdsps {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ pdsp0@0x2a1 {
+ firmware = "ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin";
+ reg = <0x2a1 0x1000/*iram */
+ 0x2a0f000 0x100 /*reg*/
+ 0x2a0c000 0x3c8 /*intd */
+ 0x2a2 0x4000>; /*cmd*/
+ id = <0>;
+ };
+ };
}; /* qmss */
knav_dmas: knav_dmas@0 {
diff --git a/arch/arm/boot/dts/k2hk-netcp.dtsi
b/arch/arm/boot/dts/k2hk-netcp.dtsi
index 77a32c3..b9941b4 100644
--- a/arch/arm/boot/dts/k2hk-netcp.dtsi
+++ b/arch/arm/boot/dts/k2hk-netcp.dtsi
@@ -47,6 +47,7 @@ qmss: qmss@2a4 {
"region", "push", "pop";
};
};
+
queue-pools {
qpend {
qpend-0 {
@@ -88,7 +89,17 @@ qmss: qmss@2a4 {
qalloc-by-id;
};
};
+ accumulator {
+ acc-low-0 {
+ qrange = <480 32>;
+ accumulator = <0 47 16 2 50>;
+ interrupts = <0 226 0xf01>;
+ multi-queue;
+ qalloc-by-id;
+ };
+ };
};
+
descriptor-regions {
#address-cells = <1>;
#size-cells = <1>;
@@ -99,6 +110,20 @@ qmss: qmss@2a4 {
link-index = <0x4000>;
};
};
+
+ pdsps {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ pdsp0@0x2a1 {
+ firmware = "ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin";
+ reg = <0x2a1 0x1000/*iram */
+ 0x2a0f000 0x100 /*reg*/
+ 0x2a0c000 0x3c8 /*intd */
+ 0x2a2 0x4000>; /*cmd*/
+ id = <0>;
+ };
+ };
}; /* qmss */
knav_dmas: knav_dmas@0 {
diff --git a/arch/arm/boot/dts/k2l-netcp.dtsi b/arch/arm/boot/dts/k2l-netcp.dtsi
index 6b95284..8d7ddb3 100644
--- a/arch/arm/boot/dts/k2l-netcp.dtsi
+++ b/arch/arm/boot/dts/k2l-netcp.dtsi
@@ -72,7 +72,16 @@ qmss: qmss@2a4 {
qalloc-by-id;
};
};
+ accumulator {
+ acc-low-0 {
+ qrange = <480 32>;
+ accumulator = <0 47 16 2 50>;
+ interrupts = <0 226 0xf01>;
+ multi-queue;
+ };
+ };
};
+
descriptor-regions {
#address-cells = <1>;
#size-cells = <1>;
@@ -83,6 +92,21 @@ qmss: qmss@2a4 {
link-index = <0x4000>;
};
};
+
+ pdsps {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ pdsp0@0x2a1 {
+ firmware = "ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin";
+ reg = <0x2a1 0x1000/*iram */
+ 0x2a0f000 0x100 /*reg*/
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed
On Wed, Sep 2, 2015 at 11:27 PM, Yakir Yang wrote: > Hi Rob, > > 在 09/03/2015 04:17 AM, Rob Herring 写道: >> >> On Tue, Sep 1, 2015 at 1:14 AM, Yakir Yang wrote: >>> >>> Some edp screen do not have hpd signal, so we can't just return >>> failed when hpd plug in detect failed. >> >> This is a property of the panel (or connector perhaps), so this >> property should be located there. At least, it is a common issue and >> not specific to this chip. We could have an HDMI connector and failed >> to hook up HPD for example. A connector node is also where hpd-gpios >> should be located instead (and are already defined by >> ../bindings/video/hdmi-connector.txt). Perhaps we need a eDP connector >> binding, too. > > > Yep, I agree with your front point, it is a property of panel, not specific > to eDP controller, so this code should handle in connector logic. > > But another question, if we just leave this property to connector, > then we would need to parse this property in analogix_dp-rockchip.c, > and then make an hook in analogix_dp_core.c driver. This is not nice, > and if there are some coming platform which alse want to use analogix_dp > code and meet this "no-hpd" situation, then they would need duplicate > to parse this property and fill the hook in analogix_dp_core.c driver. > So it's little bit conflict :-) Ideally, you would be able to just retrieve this quirk from the connector or panel. Getting this property from your node vs. the port you are attached to should not be much harder. Either the connector struct can have this info or there can be a DT function that can walk the graph and get it. Just don't open code the graph traversal in your driver. > Beside I can not understand your example very well. Do you mean > that there are some HDMI monitor which also do not have HPD > signal (just like some eDP panel do not have hpd too), and then > the "hpd-gpios" ? Hmm... I don't know how the "hpd-gpios" want > to help in this case, would you mind show some sample code :-D I don't know that there is h/w, but it is always possible HPD is not hooked up or hooked up incorrectly some how. If there is no HPD support, then hpd-gpios is not going to help you. I think there are 3 cases to handle: - HPD handled by bridge chip - The bridge driver knows it has this capability. No DT properties are needed and no HPD properties on the connector node imply the bridge chip should handle HPD functions. - HPD handled by GPIO line (or some other block) - Indicated by hpd-gpios present - No or broken HPD - Indicated by "hpd-force" property. > >> Are there any eDP panels which don't have EDID and need panel details in >> DT? > > > Oh, I think you want to collect some info that belong to panel > property but no indicate in panel EDID message, so those can > be collect in eDP connector binding, is it right ? Yes, and as Thierry pointed out we may need to know the exact panel even. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir
Am Freitag, 4. September 2015, 16:06:02 schrieb Rob Herring:
> On Tue, Sep 1, 2015 at 3:46 PM, Heiko Stuebner wrote:
> > Am Dienstag, 1. September 2015, 13:49:58 schrieb Yakir Yang:
> >> Split the dp core driver from exynos directory to bridge
> >> directory, and rename the core driver to analogix_dp_*,
> >> leave the platform code to analogix_dp-exynos.
> >>
> >> Signed-off-by: Yakir Yang
> >
> > [...]
> >
> >> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
> >> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c similarity index 50%
> >> rename from drivers/gpu/drm/exynos/exynos_dp_core.c
> >> rename to drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> index bed0252..7d62f22 100644
> >> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >
> > [...]
> >
> >> connector->polled = DRM_CONNECTOR_POLL_HPD;
> >>
> >> ret = drm_connector_init(dp->drm_dev, connector,
> >>
> >> - &exynos_dp_connector_funcs,
> >> + &analogix_dp_connector_funcs,
> >>
> >>DRM_MODE_CONNECTOR_eDP);
> >>
> >> if (ret) {
> >>
> >> DRM_ERROR("Failed to initialize connector with drm\n");
> >> return ret;
> >>
> >> }
> >>
> >> - drm_connector_helper_add(connector,
> >> &exynos_dp_connector_helper_funcs); +
> >> drm_connector_helper_add(connector,
> >> + &analogix_dp_connector_helper_funcs);
> >>
> >> drm_connector_register(connector);
> >
> > this should only run on exynos, as we're doing all our connector
> > registration in the core driver after all components are bound, so I
> > guess something like>
> > the following is needed:
> >if (dp->plat_data && dp->plat_data->dev_type == EXYNOS_DP)
> >
> >drm_connector_register(connector);
>
> Yuck!
>
> Surely there is a better way. From what I've seen of DRM, I have no
> doubt this is needed, but really a better solution is needed. Surely
> there can be a more generic way for the driver to determine if it
> should handle the connector or not. This seems like a common problem
> including one I have seen. What I'm working on has onchip DSI encoder
> -> ADV7533 -> HDMI. The DSI encoder can also have a direct attached
> panel. So I have to check for a bridge in the encoder driver and only
> register the connector for the panel if a bridge is not attached.
I'm also only a part-time drm meddler, so things may be inaccurate.
This conditional is not meant to prevent the registration of the dp connector,
only delay it for non-exynos drms. The connector registration does publish it
to userspace, so like i.MX we're doing that for all connectors at the same
time after all components are bound - to also make x11 happy. Exynos on the
other hand seems to register its connectors individually and I'm not sure if
they would be willing to change that.
see d3007dabeff4 ("drm/rockchip: register all connectors after bind") or
simply rockchip_drm_drv.c line 178.
and e355e7dd607b ("imx-drm: delay publishing sysfs connector entries")
Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/10] Documentation/dts: Add bindings for QIXIS FPGA controller found on FSL boards
On Fri, Sep 4, 2015 at 3:16 PM, Sharma Bhupesh wrote: >> From: pku@gmail.com [mailto:pku@gmail.com] >> Sent: Friday, September 04, 2015 10:27 PM >> On Fri, Sep 4, 2015 at 1:57 AM, Bhupesh Sharma >> wrote: >> > This patch adds bindings for QIXIS FPGA controller found on FSL boards. >> >> A general comment: when you are updating the device tree bindings. >> You should cc the devicetree@vger.kernel.org mailing list. >> >> > >> > Some Freescale boards like LS2080AQDS/LS2080ARDB have an on-board >> > FPGA/CPLD connected to the IFC controller. The bindings specified in >> > this patch cater to those on-board FPGA/CPLD controllers. >> >> We already have the binding defined in >> Documentation/devicetree/bindings/powerpc/fsl/board.txt. We probably >> should just move it to a more generic location. > > Consolidation of powerpc and ARM bindings is something that needs to be > targeted by a separate patch-series, perhaps in future. > > There are a number of duplications that can be looked into, but I don't > think this patchset should depend on that. There might be some duplication, but we shouldn't be adding more. :) I would rather you directly updating that file than creating a new file even though the original one was in a wrong directory. Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir
On Tue, Sep 1, 2015 at 3:46 PM, Heiko Stuebner wrote:
> Am Dienstag, 1. September 2015, 13:49:58 schrieb Yakir Yang:
>> Split the dp core driver from exynos directory to bridge
>> directory, and rename the core driver to analogix_dp_*,
>> leave the platform code to analogix_dp-exynos.
>>
>> Signed-off-by: Yakir Yang
>
> [...]
>
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c similarity index 50%
>> rename from drivers/gpu/drm/exynos/exynos_dp_core.c
>> rename to drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index bed0252..7d62f22 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>
> [...]
>
>> connector->polled = DRM_CONNECTOR_POLL_HPD;
>>
>> ret = drm_connector_init(dp->drm_dev, connector,
>> - &exynos_dp_connector_funcs,
>> + &analogix_dp_connector_funcs,
>>DRM_MODE_CONNECTOR_eDP);
>> if (ret) {
>> DRM_ERROR("Failed to initialize connector with drm\n");
>> return ret;
>> }
>>
>> - drm_connector_helper_add(connector, &exynos_dp_connector_helper_funcs);
>> + drm_connector_helper_add(connector,
>> + &analogix_dp_connector_helper_funcs);
>> drm_connector_register(connector);
>
> this should only run on exynos, as we're doing all our connector registration
> in the core driver after all components are bound, so I guess something like
> the following is needed:
>
>if (dp->plat_data && dp->plat_data->dev_type == EXYNOS_DP)
>drm_connector_register(connector);
Yuck!
Surely there is a better way. From what I've seen of DRM, I have no
doubt this is needed, but really a better solution is needed. Surely
there can be a more generic way for the driver to determine if it
should handle the connector or not. This seems like a common problem
including one I have seen. What I'm working on has onchip DSI encoder
-> ADV7533 -> HDMI. The DSI encoder can also have a direct attached
panel. So I have to check for a bridge in the encoder driver and only
register the connector for the panel if a bridge is not attached.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/10] dts/ls2085a: Update DTSI to add support of various peripherals
On Fri, Sep 4, 2015 at 2:05 AM, Bhupesh Sharma
wrote:
> This patch updates the LS2085a DTSI (DTS Include) file to add
> support for various peripherals supported by FSL LS2085a SoC, for e.g.:
The title and description here are still using the old LS2085 name.
> - USB 3.0 Host
> - PMU
> - CCN-504
> - Watchdog
> - SATA
> - SPI
> - PCIe
> - etc.
>
> Signed-off-by: Bhupesh Sharma
> Signed-off-by: Jaiprakash Singh
> Signed-off-by: Alison Wang
> Signed-off-by: Liu Gang
> Signed-off-by: Minghuan Lian
> Signed-off-by: Shaohui Xie
> Signed-off-by: Nikhil Badola
> Signed-off-by: Yangbo Lu
> Signed-off-by: Scott Wood
> ---
> arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 469
> +++-
> 1 file changed, 459 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> index 333d942..5fee0a7 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> @@ -20,11 +20,6 @@
> * 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 library; if not, write to the Free
> - * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> - * MA 02110-1301 USA
> - *
> * Or, alternatively,
> *
> * b) Permission is hereby granted, free of charge, to any person
> @@ -71,48 +66,56 @@
> device_type = "cpu";
> compatible = "arm,cortex-a57";
> reg = <0x0 0x0>;
> + clocks = <&clockgen 1 0>;
> };
>
> cpu@1 {
> device_type = "cpu";
> compatible = "arm,cortex-a57";
> reg = <0x0 0x1>;
> + clocks = <&clockgen 1 0>;
> };
>
> cpu@100 {
> device_type = "cpu";
> compatible = "arm,cortex-a57";
> reg = <0x0 0x100>;
> + clocks = <&clockgen 1 1>;
> };
>
> cpu@101 {
> device_type = "cpu";
> compatible = "arm,cortex-a57";
> reg = <0x0 0x101>;
> + clocks = <&clockgen 1 1>;
> };
>
> cpu@200 {
> device_type = "cpu";
> compatible = "arm,cortex-a57";
> reg = <0x0 0x200>;
> + clocks = <&clockgen 1 2>;
> };
>
> cpu@201 {
> device_type = "cpu";
> compatible = "arm,cortex-a57";
> reg = <0x0 0x201>;
> + clocks = <&clockgen 1 2>;
> };
>
> cpu@300 {
> device_type = "cpu";
> compatible = "arm,cortex-a57";
> reg = <0x0 0x300>;
> + clocks = <&clockgen 1 3>;
> };
>
> cpu@301 {
> device_type = "cpu";
> compatible = "arm,cortex-a57";
> reg = <0x0 0x301>;
> + clocks = <&clockgen 1 3>;
> };
> };
>
> @@ -122,13 +125,44 @@
> /* DRAM space - 1, size : 2 GB DRAM */
> };
>
> + pmu {
> + compatible = "arm,armv8-pmuv3";
> + interrupts = <1 7 0x8>; /* PMU PPI, Level low type */
> + };
> +
> gic: interrupt-controller@600 {
> compatible = "arm,gic-v3";
> reg = <0x0 0x0600 0 0x1>, /* GIC Dist */
> - <0x0 0x0610 0 0x10>; /* GICR (RD_base +
> SGI_base) */
> + <0x0 0x0610 0 0x10>, /* GICR (RD_base +
> SGI_base) */
> + <0x0 0x0c0c 0 0x2000>, /* GICC */
> + <0x0 0x0c0d 0 0x1000>, /* GICH */
> + <0x0 0x0c0e 0 0x2>; /* GICV */
> #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> interrupt-controller;
> interrupts = <1 9 0x4>;
> +
> + its: gic-its@602 {
> + compatible = "arm,gic-v3-its";
> + msi-controller;
> + reg = <0x0 0x602 0 0x2>;
> + };
> + };
> +
> + clockgen: clocking@130 {
> + compatible = "fsl,ls2080a-clockgen";
> + reg = <0 0x130 0 0xa>;
[PATCH 2/2] ARM: dts: keystone: enable accumulator channels
Add low priority accumulator channel that can monitor multiple QMSS
queues. User for example could use the accumular queue for Netcp
Rx completion. While at it, also add an extra line end of each top
level node in DTS to make it more readable.
Signed-off-by: Murali Karicheri
---
arch/arm/boot/dts/k2e-netcp.dtsi | 24
arch/arm/boot/dts/k2hk-netcp.dtsi | 25 +
arch/arm/boot/dts/k2l-netcp.dtsi | 24
3 files changed, 73 insertions(+)
diff --git a/arch/arm/boot/dts/k2e-netcp.dtsi b/arch/arm/boot/dts/k2e-netcp.dtsi
index b13b3c9..1818929 100644
--- a/arch/arm/boot/dts/k2e-netcp.dtsi
+++ b/arch/arm/boot/dts/k2e-netcp.dtsi
@@ -72,7 +72,17 @@ qmss: qmss@2a4 {
qalloc-by-id;
};
};
+ accumulator {
+ acc-low-0 {
+ qrange = <480 32>;
+ accumulator = <0 47 16 2 50>;
+ interrupts = <0 226 0xf01>;
+ multi-queue;
+ qalloc-by-id;
+ };
+ };
};
+
descriptor-regions {
#address-cells = <1>;
#size-cells = <1>;
@@ -83,6 +93,20 @@ qmss: qmss@2a4 {
link-index = <0x4000>;
};
};
+
+ pdsps {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ pdsp0@0x2a1 {
+ firmware = "ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin";
+ reg = <0x2a1 0x1000/*iram */
+ 0x2a0f000 0x100 /*reg*/
+ 0x2a0c000 0x3c8 /*intd */
+ 0x2a2 0x4000>; /*cmd*/
+ id = <0>;
+ };
+ };
}; /* qmss */
knav_dmas: knav_dmas@0 {
diff --git a/arch/arm/boot/dts/k2hk-netcp.dtsi
b/arch/arm/boot/dts/k2hk-netcp.dtsi
index 77a32c3..b9941b4 100644
--- a/arch/arm/boot/dts/k2hk-netcp.dtsi
+++ b/arch/arm/boot/dts/k2hk-netcp.dtsi
@@ -47,6 +47,7 @@ qmss: qmss@2a4 {
"region", "push", "pop";
};
};
+
queue-pools {
qpend {
qpend-0 {
@@ -88,7 +89,17 @@ qmss: qmss@2a4 {
qalloc-by-id;
};
};
+ accumulator {
+ acc-low-0 {
+ qrange = <480 32>;
+ accumulator = <0 47 16 2 50>;
+ interrupts = <0 226 0xf01>;
+ multi-queue;
+ qalloc-by-id;
+ };
+ };
};
+
descriptor-regions {
#address-cells = <1>;
#size-cells = <1>;
@@ -99,6 +110,20 @@ qmss: qmss@2a4 {
link-index = <0x4000>;
};
};
+
+ pdsps {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ pdsp0@0x2a1 {
+ firmware = "ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin";
+ reg = <0x2a1 0x1000/*iram */
+ 0x2a0f000 0x100 /*reg*/
+ 0x2a0c000 0x3c8 /*intd */
+ 0x2a2 0x4000>; /*cmd*/
+ id = <0>;
+ };
+ };
}; /* qmss */
knav_dmas: knav_dmas@0 {
diff --git a/arch/arm/boot/dts/k2l-netcp.dtsi b/arch/arm/boot/dts/k2l-netcp.dtsi
index 6b95284..8d7ddb3 100644
--- a/arch/arm/boot/dts/k2l-netcp.dtsi
+++ b/arch/arm/boot/dts/k2l-netcp.dtsi
@@ -72,7 +72,16 @@ qmss: qmss@2a4 {
qalloc-by-id;
};
};
+ accumulator {
+ acc-low-0 {
+ qrange = <480 32>;
+ accumulator = <0 47 16 2 50>;
+ interrupts = <0 226 0xf01>;
+ multi-queue;
+ };
+ };
};
+
descriptor-regions {
#address-cells = <1>;
#size-cells = <1>;
@@ -83,6 +92,21 @@ qmss: qmss@2a4 {
link-index = <0x4000>;
};
};
+
+ pdsps {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ pdsp0@0x2a1 {
+ firmware = "ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin";
+ reg = <0x2a1 0x1000/*iram */
+ 0x2a0f000 0x100 /*reg*/
+ 0x2a0c
[PATCH 1/2] soc: ti: display firmware file name as part of boot log
To help the user, print the PDSP file name as part of
knav_queue_load_pdsp(). This will be useful for users to know what
version of the firmware is loaded to PDSP. Also update the
document for the location of the QMSS accumulator PDSP firmware.
Signed-off-by: Murali Karicheri
---
.../bindings/soc/ti/keystone-navigator-qmss.txt | 20 +++-
drivers/soc/ti/knav_qmss_queue.c | 3 +++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
b/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
index d8e8cdb..a91ee0b 100644
--- a/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
+++ b/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
@@ -221,7 +221,7 @@ qmss: qmss@2a4 {
#size-cells = <1>;
ranges;
pdsp0@0x2a1 {
- firmware = "keystone/qmss_pdsp_acc48_k2_le_1_0_0_8.fw";
+ firmware = "k2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin";
reg = <0x2a1 0x1000>,
<0x2a0f000 0x100>,
<0x2a0c000 0x3c8>,
@@ -230,3 +230,21 @@ qmss: qmss@2a4 {
};
};
}; /* qmss */
+
+Accumulator QMSS Channel using PDSP firmware
+
+The QMSS PDSP firmware support accumulator channel that can monitor a single
+queue or multiple contiguous queues. drivers/soc/ti/knav_qmss_acc.c is the
+driver that interface with the accumulator PDSP. This configures
+accumulator channels defined in DTS (example above) to monitor 1 or 32 queues
+per channel. More description on the firmware is available in CPPI/QMSS Low
+Level Driver document (docs/CPPI_QMSS_LLD_SDS.pdf) at
+ git://git.ti.com/keystone-rtos/qmss-lld.git
+
+k2_qmss_pdsp_acc48_k2_le_1_0_0_9.fw firmware supports upto 48 accumulator
+channels. This firmware is available under firmware folder of the above repo
+under the name acc48_le.bin. To use copy the firmware image to lib/firmware
+folder of the initramfs or ubifs file system as
+k2_qmss_pdsp_acc48_k2_le_1_0_0_9.fw and boot up the kernel. User would see
+"firmware file ks2_qmss_pdsp_acc48_k2_le_1_0_0_9.bin downloaded for PDSP" in
+the boot up log if loading of firmware to PDSP is successful.
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 6d8646d..f26ce99 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -1526,6 +1526,9 @@ static int knav_queue_load_pdsp(struct knav_device *kdev,
pdsp->firmware, pdsp->name);
return ret;
}
+ dev_info(kdev->dev, "firmware file %s downloaded for PDSP\n",
+pdsp->firmware);
+
writel_relaxed(pdsp->id + 1, pdsp->command + 0x18);
/* download the firmware */
fwdata = (u32 *)fw->data;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] firmware: qcom: scm: Convert to platform driver
On Thu, Sep 03, 2015 at 02:33:22PM -0700, Stephen Boyd wrote:
> On 07/20, Andy Gross wrote:
> > This patch creates a platform driver for the SCM so that we can adequately
> > manage resources. This removes clients having to carry the necessary
> > clocks to use the SCM resources.
> >
> > Signed-off-by: Andy Gross
> > ---
>
> It would be nice if we could use this platform device for doing
> the DMAish memory allocations that we do in this driver too. I
> guess one complication there is that we would need to allocate
> memory with the DMA APIs before CPUs are brought up
> (early_initcall level).
Yeah that's one thing we could do but we'd have to defer the memory stuff until
it's used the first time (specific calls require it).
>
> > diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> > index 45c008d..5dd0514 100644
> > --- a/drivers/firmware/qcom_scm.c
> > +++ b/drivers/firmware/qcom_scm.c
> > @@ -15,14 +15,57 @@
> > * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > * 02110-1301, USA.
> > */
> > -
> > +#include
> > +#include
> > +#include
>
> This include is here twice.
>
> > #include
> > #include
> > #include
> > #include
> > +#include
> > +#include
> >
> [...]
> > +
> > +/**
> > + * qcom_scm_is_available() - Checks if SCM is available
> > + */
> > +bool qcom_scm_is_available(void)
> > +{
> > + return !!__scm;
> > +}
> > +EXPORT_SYMBOL(qcom_scm_is_available);
> > +
> > +static int qcom_scm_remove(struct platform_device *pdev)
> > +{
> > + __scm = NULL;
> > +
> > + return 0;
> > +}
>
> Maybe we just shouldn't allow this? The firmware isn't going
> anywhere at runtime, and this driver is currently marked as
> bool in the Kconfig.
Fair enough.
>
> > +
> > +static const struct of_device_id qcom_scm_dt_match[] = {
> > + { .compatible = "qcom,scm",},
> > + {},
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
> > +
> > +static struct platform_driver qcom_scm_driver = {
> > + .driver = {
> > + .name = "scm",
>
> Maybe 'qcom_scm' ?
yeah i should have used that.
>
> > + .of_match_table = qcom_scm_dt_match,
> > + },
> > + .probe = qcom_scm_probe,
> > + .remove = qcom_scm_remove,
> > +};
> > +
> > +module_platform_driver(qcom_scm_driver);
>
> Isn't there some sort of builtin_platform_driver() macro for
> builtin modules?
I'll take a look and convert.
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 03/10] Documentation/dts: Add bindings for QIXIS FPGA controller found on FSL boards
> From: pku@gmail.com [mailto:pku@gmail.com]
> Sent: Friday, September 04, 2015 10:27 PM
> On Fri, Sep 4, 2015 at 1:57 AM, Bhupesh Sharma
> wrote:
> > This patch adds bindings for QIXIS FPGA controller found on FSL boards.
>
> A general comment: when you are updating the device tree bindings.
> You should cc the devicetree@vger.kernel.org mailing list.
>
> >
> > Some Freescale boards like LS2080AQDS/LS2080ARDB have an on-board
> > FPGA/CPLD connected to the IFC controller. The bindings specified in
> > this patch cater to those on-board FPGA/CPLD controllers.
>
> We already have the binding defined in
> Documentation/devicetree/bindings/powerpc/fsl/board.txt. We probably
> should just move it to a more generic location.
Consolidation of powerpc and ARM bindings is something that needs to be
targeted by a separate patch-series, perhaps in future.
There are a number of duplications that can be looked into, but I don't
think this patchset should depend on that.
Regards,
Bhupesh
N�r��yb�X��ǧv�^�){.n�+���z��z��z)w* jg��� �ݢj/���z�ޖ��2�ޙ&�)ߡ�a���� �G���h��j:+v���w��٥
Re: [PATCH v4 1/4] of/pci: Add of_pci_check_probe_only to parse "linux,pci-probe-only"
On Fri, Sep 4, 2015 at 11:50 AM, Marc Zyngier wrote: > Both pci-host-generic and Pseries parse the "linux,pci-probe-only" > property to engage the PCI_PROBE_ONLY mode, and both have a subtle > bug that can be triggered if the property has no parameter. > > Provide a generic, safe implementation that can be used by both. > > Signed-off-by: Marc Zyngier > --- > drivers/of/of_pci.c| 28 > include/linux/of_pci.h | 3 +++ > 2 files changed, 31 insertions(+) > > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c > index 5751dc5..5dd4966 100644 > --- a/drivers/of/of_pci.c > +++ b/drivers/of/of_pci.c > @@ -118,6 +118,34 @@ int of_get_pci_domain_nr(struct device_node *node) > EXPORT_SYMBOL_GPL(of_get_pci_domain_nr); > > /** > + * of_pci_check_probe_only - Setup probe only mode if linux,pci-probe-only > + * is present and valid > + * > + * @node: device tree node that may contain the property (usually "chosen") This line should be removed. Perhaps Bjorn will fix it up. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] of_pci_irq: Silence bogus "of_irq_parse_pci() failed ..." messages.
From: David Daney
It is perfectly legitimate for a PCI device to have an
PCI_INTERRUPT_PIN value of zero. This happens if the device doesn't
use interrupts, or on PCIe devices, where only MSI/MSI-X are
supported.
Silence the annoying "of_irq_parse_pci() failed with rc=-19" error
messages by making them conditional on !-ENODEV (which can only be
produced in the PCI_INTERRUPT_PIN == 0 case).
Signed-off-by: David Daney
---
drivers/of/of_pci_irq.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/of/of_pci_irq.c b/drivers/of/of_pci_irq.c
index 1710d9d..33d242a 100644
--- a/drivers/of/of_pci_irq.c
+++ b/drivers/of/of_pci_irq.c
@@ -106,7 +106,9 @@ int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8
slot, u8 pin)
ret = of_irq_parse_pci(dev, &oirq);
if (ret) {
- dev_err(&dev->dev, "of_irq_parse_pci() failed with rc=%d\n",
ret);
+ if (ret != -ENODEV)
+ dev_err(&dev->dev,
+ "of_irq_parse_pci() failed with rc=%d\n", ret);
return 0; /* Proper return code 0 == NO_IRQ */
}
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: st_fdma: Firmware filename in DT?
On Fri, Sep 4, 2015 at 9:27 AM, Warner Losh wrote: > >> On Sep 4, 2015, at 4:21 AM, Lee Jones wrote: >> >> We could do it by parsing the node name e.g. fdma0-audio, or by adding >> a "instance" DT property to the node? > > Generally we try to avoid caring about node names. Having some index > or numbering also comes up which we also try to avoid. Generally, if > you care about which instance you use for something, then there is > some property you care about and should add. Right, the alternative is a property like the ones already used. However, as these are becoming more prevalent I suggested standardising the property to avoid all these vendor specific firmware properties cluttering up the place. >>> >>> I agree a proliferation of vendor specific firmware properties isn't >>> s good way forward. >>> firmware = "firmwarename.fw"; OR firmware-name = "firmwarename.fw"; ... seems appropriate. >>> >>> Either of those is fine with me. >> >> Just need a DT nod and I'll happily code it up. > > From a FreeBSD perspective, having a filename for the firmware to > load for this node is fine. It doesn’t impose a substantial burden on > the OS so long as the choices of where that file lives is up to the OS > and not encoded in the property. A note saying ‘this firmware should > be loaded’ seems a reasonable description of the hardware since > the DT tells us many things about the device, and those things may > well be dependent on which firmware is loaded. Having an explicit > name is good since it helps insulate from DT and doesn’t force vendors > to do silly renames. It also allows for multiple devices of the same > type to have different firmware loaded. > > FreeBSD would generally put these things in /boot/firmware and > it has a generalized mechanism to load the firmware at run time > that’s based on this. While the path names are flexible, having > the firmware live in a central place is useful from an automation > point of view. Having just a name, and not a full path, enables > this policy, while still allowing others to have other policies. > > Linux distributions would be free to do whatever they wanted > and implement other policies than FreeBSD. > > So a property like this, with the semantics discussed, seems to > meet the OS independent test. Good. I'll await a binding to review... I would suggest "firmware-names" to be consistent with other naming convention and because their can be more than one (either at the same time or as fallback). It also leaves "firmware" available if we want to have phandle's to firmware embedded in the DT at some point later. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/4] PCI: arm64/powerpc: Fix parsing of linux,pci-probe-only
On Fri, Sep 4, 2015 at 11:50 AM, Marc Zyngier wrote: > The pci-host-generic driver parses the linux,pci-probe-only property, > and assumes that it will have a boolean parameter. > > Turns out that the Seattle DTS file has a naked "linux,pci-probe-only" > property, which leads to the driver dereferencing some unsuspecting > memory location. Nothing really bad happens (we end up reading some > other bit of DT, fortunately), but that not a reason to keep it this > way. Turns out that the Pseries code (where this code was lifted from) > may suffer from the same issue. > > The first patch introduces a common (and fixed) version of that check > that can be used by drivers and architectures that require it. The two > following patches change the pci-host-generic driver and the powerpc > code to use it. > > Finally, the bad property is removed from the Seatle DTS, because it > is simply not necessary (it actually prevents me from using SR-IOV, > which otherwise runs fine without the probe-only thing). > > This has been tested on the offending Seattle board. > > * From v3: > - Restrict the property lookup to /chosen (Rob) > - Acked-by on patch #4 from Suravee > - I swear this is the last time I rework these patches! ;-) > > * From v2: > - Use of_property_read_u32 to safely read the property (Rob) > - Add a log message to indicate when we enable probe-only > (probably quite useful for debugging) > > * From v1: > - Consolidate the parsing in of_pci.c (Bjorn) > > Marc Zyngier (4): > of/pci: Add of_pci_check_probe_only to parse "linux,pci-probe-only" > PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property > powerpc: PCI: Fix lookup of linux,pci-probe-only property > arm64: dts: Drop linux,pci-probe-only from the Seattle DTS > > arch/arm64/boot/dts/amd/amd-overdrive.dts | 1 - > arch/powerpc/platforms/pseries/setup.c| 14 ++ > drivers/of/of_pci.c | 28 > drivers/pci/host/pci-host-generic.c | 9 + > include/linux/of_pci.h| 3 +++ > 5 files changed, 34 insertions(+), 21 deletions(-) For the series: Reviewed-by: Rob Herring Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] spmi-pmic-arb: support configurable number of peripherals
On Thu, 3 Sep 2015 17:16:30 -0700
Stephen Boyd wrote:
> On 09/03, Gilad Avidov wrote:
> > +supported by HW. Default (minimum
> > supported) is 128. +
> > +Example V1 PMIC-Arbiter:
> >
> > spmi {
> > compatible = "qcom,spmi-pmic-arb";
> > @@ -62,4 +66,32 @@ Example:
> >
> > interrupt-controller;
> > #interrupt-cells = <4>;
> > +
> > + qcom,max-peripherals = <256>;
>
> If it's v1 isn't it always 128? So having 256 here is just
> confusing.
Hi Stephen,
Actually some v1 chipsets, such as Dragonboard APQ8074, support 256
peripherals.
>
> > + };
> > +
> > @@ -129,14 +131,15 @@ struct spmi_pmic_arb_dev {
> > u8 channel;
> > int irq;
> > u8 ee;
> > - u8 min_apid;
> > - u8 max_apid;
> > - u32
> > mapping_table[SPMI_MAPPING_TABLE_LEN];
> > + u16 min_irq_apid;
> > + u16 max_irq_apid;
> > + u16 max_apid;
> > + u32 *mapping_table;
> > struct irq_domain *domain;
> > struct spmi_controller *spmic;
> > - u16 apid_to_ppid[256];
> > + u16 *irq_apid_to_ppid;
>
> Please drop all this renaming noise, or at the least, put it in a
> different patch. More than half the patch is just changing the
> names of these variables for what seems like no reason.
>
I agree that changing apid_to_ppid to irq_apid_to_ppid is not required
and I'll undo this change on next patch.
Regarding the change of max/min_apid to max/min_irq_apid:
max_apid was already used but the name does not make good sense. Since
really it is not the max_apid supported. Instead it is the largest apid
which interrupt is currently requested for. But now we need a value that
is actually the maximum supported apid. This is why repurposed max_apid
and corrected the previous naming.
> > const struct pmic_arb_ver_ops *ver_ops;
> > - u8 *ppid_to_chan;
> > + u16 *ppid_to_chan;
> > };
> >
> > struct spmi_pmic_arb_dev *pa = irq_get_handler_data(irq);
> > struct irq_chip *chip = irq_get_chip(irq);
> > void __iomem *intr = pa->intr;
> > - int first = pa->min_apid >> 5;
> > - int last = pa->max_apid >> 5;
> > + int first = pa->min_irq_apid >> 5;
> > + int last = pa->max_irq_apid >> 5;
> > u32 status;
> > int i, id;
> >
> > @@ -903,14 +915,30 @@ static int spmi_pmic_arb_probe(struct
> > platform_device *pdev)
> > pa->ee = ee;
> >
> > - for (i = 0; i < ARRAY_SIZE(pa->mapping_table); ++i)
> > - pa->mapping_table[i] = readl_relaxed(
> > - pa->cnfg +
> > SPMI_MAPPING_TABLE_REG(i));
> > + pa->irq_apid_to_ppid = devm_kzalloc(&ctrl->dev,
> > pa->max_apid *
> > +
> > sizeof(*pa->irq_apid_to_ppid),
> > + GFP_KERNEL);
> > + if (!pa->irq_apid_to_ppid) {
> > + err = -ENOMEM;
> > + goto err_put_ctrl;
> > + }
> > +
> > + pa->mapping_table = devm_kzalloc(&ctrl->dev,
> > + (pa->max_apid - 1) *
> > sizeof(u32),
> > + GFP_KERNEL);
> > + if (!pa->mapping_table) {
> > + err = -ENOMEM;
> > + goto err_put_ctrl;
> > + }
> > +
> > + for (i = 0; i < (pa->max_apid - 1); ++i)
> > + pa->mapping_table[i] = readl_relaxed(pa->cnfg +
> > +
> > SPMI_MAPPING_TABLE_REG(i));
>
> Maybe we should stop doing this during probe and always allocate
> an empty cache of size 128 on v1 and 512 on v2 chips? So when
As mentioned above, both v1 and v2 can have different number of
peripherals.
> we're searching through the mapping table we can cache the value
> from the register if the entry isn't 0. This delays the
Greedy approach (reading upto 512 registers from hw) have very small
upfront penalty since sequential reads take place very fast
and HW acceleration (prefetching) will improve it. A lazy approach will
required much more complex algorithm (for example any value
including zero is valid. So we will need to augment a marker for
unassigned entries) and prefetching will not help.
> processing to when we're mapping irqs, hopefully speeding up
> probe for the case where you have a handful of irqs to map.
Please note that we traverse this mapping table every time a PMIC
driver registers for an interrupt. This means that even if we speed up
the probe of pmic-arb driver we will not speed the system boot since
the probing of the PMIC drivers will register for interrupts.
>
> The DT property wouldn't be necessary then. Arguably it's being
> added there to optimize the size of the mapping table and isn't
> really necessary otherwise.
>
We need to know this value for both memory allocation
and for stopping before reading over the bounds of the mapping_table in
HW.
Thank you,
Gilad
Re: [PATCH v2 04/10] doc/bindings: Update PCIe devicetree binding documentation for LS2080A
On Fri, Sep 4, 2015 at 1:57 AM, Bhupesh Sharma wrote: > Add the documentation for compatible string "fsl,ls2080a-pcie" > for Freescale's LS2080A platform. > > Signed-off-by: Minghuan Lian > Signed-off-by: Bhupesh Sharma > --- > .../devicetree/bindings/pci/layerscape-pci.txt |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt > index 6286f04..e72e68f 100644 > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt > @@ -4,7 +4,8 @@ This PCIe host controller is based on the Synopsis Designware > PCIe IP > and thus inherits all the common properties defined in designware-pcie.txt. > > Required properties: > -- compatible: should contain the platform identifier such as > "fsl,ls1021a-pcie" > +- compatible: should contain the platform identifier such as > "fsl,ls1021a-pcie", > + "fsl,ls2080a-pcie". Make it generic like "fsl,-pcie" - Leo -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/10] Documentation/dts: Add bindings for QIXIS FPGA controller found on FSL boards
On Fri, Sep 4, 2015 at 1:57 AM, Bhupesh Sharma wrote: > This patch adds bindings for QIXIS FPGA controller found on FSL boards. A general comment: when you are updating the device tree bindings. You should cc the devicetree@vger.kernel.org mailing list. > > Some Freescale boards like LS2080AQDS/LS2080ARDB have an on-board FPGA/CPLD > connected to the IFC controller. The bindings specified in this patch > cater to those on-board FPGA/CPLD controllers. We already have the binding defined in Documentation/devicetree/bindings/powerpc/fsl/board.txt. We probably should just move it to a more generic location. Regards, Leo -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/4] PCI: arm64/powerpc: Fix parsing of linux,pci-probe-only
The pci-host-generic driver parses the linux,pci-probe-only property, and assumes that it will have a boolean parameter. Turns out that the Seattle DTS file has a naked "linux,pci-probe-only" property, which leads to the driver dereferencing some unsuspecting memory location. Nothing really bad happens (we end up reading some other bit of DT, fortunately), but that not a reason to keep it this way. Turns out that the Pseries code (where this code was lifted from) may suffer from the same issue. The first patch introduces a common (and fixed) version of that check that can be used by drivers and architectures that require it. The two following patches change the pci-host-generic driver and the powerpc code to use it. Finally, the bad property is removed from the Seatle DTS, because it is simply not necessary (it actually prevents me from using SR-IOV, which otherwise runs fine without the probe-only thing). This has been tested on the offending Seattle board. * From v3: - Restrict the property lookup to /chosen (Rob) - Acked-by on patch #4 from Suravee - I swear this is the last time I rework these patches! ;-) * From v2: - Use of_property_read_u32 to safely read the property (Rob) - Add a log message to indicate when we enable probe-only (probably quite useful for debugging) * From v1: - Consolidate the parsing in of_pci.c (Bjorn) Marc Zyngier (4): of/pci: Add of_pci_check_probe_only to parse "linux,pci-probe-only" PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property powerpc: PCI: Fix lookup of linux,pci-probe-only property arm64: dts: Drop linux,pci-probe-only from the Seattle DTS arch/arm64/boot/dts/amd/amd-overdrive.dts | 1 - arch/powerpc/platforms/pseries/setup.c| 14 ++ drivers/of/of_pci.c | 28 drivers/pci/host/pci-host-generic.c | 9 + include/linux/of_pci.h| 3 +++ 5 files changed, 34 insertions(+), 21 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] of/pci: Add of_pci_check_probe_only to parse "linux,pci-probe-only"
Both pci-host-generic and Pseries parse the "linux,pci-probe-only"
property to engage the PCI_PROBE_ONLY mode, and both have a subtle
bug that can be triggered if the property has no parameter.
Provide a generic, safe implementation that can be used by both.
Signed-off-by: Marc Zyngier
---
drivers/of/of_pci.c| 28
include/linux/of_pci.h | 3 +++
2 files changed, 31 insertions(+)
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 5751dc5..5dd4966 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -118,6 +118,34 @@ int of_get_pci_domain_nr(struct device_node *node)
EXPORT_SYMBOL_GPL(of_get_pci_domain_nr);
/**
+ * of_pci_check_probe_only - Setup probe only mode if linux,pci-probe-only
+ * is present and valid
+ *
+ * @node: device tree node that may contain the property (usually "chosen")
+ *
+ */
+void of_pci_check_probe_only(void)
+{
+ u32 val;
+ int ret;
+
+ ret = of_property_read_u32(of_chosen, "linux,pci-probe-only", &val);
+ if (ret) {
+ if (ret == -ENODATA || ret == -EOVERFLOW)
+ pr_warn("linux,pci-probe-only without valid value,
ignoring\n");
+ return;
+ }
+
+ if (val)
+ pci_add_flags(PCI_PROBE_ONLY);
+ else
+ pci_clear_flags(PCI_PROBE_ONLY);
+
+ pr_info("PCI: PROBE_ONLY %sabled\n", val ? "en" : "dis");
+}
+EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
+
+/**
* of_pci_dma_configure - Setup DMA configuration
* @dev: ptr to pci_dev struct of the PCI device
*
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29fd3fe..38c0533 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -17,6 +17,7 @@ int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8
slot, u8 pin);
int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
int of_get_pci_domain_nr(struct device_node *node);
void of_pci_dma_configure(struct pci_dev *pci_dev);
+void of_pci_check_probe_only(void);
#else
static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct
of_phandle_args *out_irq)
{
@@ -53,6 +54,8 @@ of_get_pci_domain_nr(struct device_node *node)
}
static inline void of_pci_dma_configure(struct pci_dev *pci_dev) { }
+
+static inline void of_pci_check_probe_only(void) { }
#endif
#if defined(CONFIG_OF_ADDRESS)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 4/4] arm64: dts: Drop linux,pci-probe-only from the Seattle DTS
The linux,pci-probe-only property mandates an argument to indicate
whether or not to engage the "probe-only" mode, but the Seattle
DTS just provides a naked property, which is illegal.
Also, it turns out that the board is perfectly happy without
probe-only, so let's drop this altogether.
Acked-by: Suravee Suthikulpanit
Signed-off-by: Marc Zyngier
---
arch/arm64/boot/dts/amd/amd-overdrive.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amd/amd-overdrive.dts
b/arch/arm64/boot/dts/amd/amd-overdrive.dts
index 564a3f7..128fa94 100644
--- a/arch/arm64/boot/dts/amd/amd-overdrive.dts
+++ b/arch/arm64/boot/dts/amd/amd-overdrive.dts
@@ -14,7 +14,6 @@
chosen {
stdout-path = &serial0;
- linux,pci-probe-only;
};
};
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/4] powerpc: PCI: Fix lookup of linux,pci-probe-only property
When find_and_init_phbs() looks for the probe-only property, it seems
to trust the firmware to be correctly written, and assumes that there
is a parameter to the property.
It is conceivable that the firmware could not be that perfect, and it
could expose this property naked (at least one arm64 platform seems to
exhibit this exact behaviour). The setup code the ends up making
a decision based on whatever the property pointer points to, which
is likely to be junk.
Instead, switch to the common of_pci.c implementation that doesn't
suffer from this problem and ignore the property if the firmware
couldn't make up its mind.
Signed-off-by: Marc Zyngier
---
arch/powerpc/platforms/pseries/setup.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index 39a74fa..6016709 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include
#include
@@ -495,18 +496,7 @@ static void __init find_and_init_phbs(void)
* PCI_PROBE_ONLY and PCI_REASSIGN_ALL_BUS can be set via properties
* in chosen.
*/
- if (of_chosen) {
- const int *prop;
-
- prop = of_get_property(of_chosen,
- "linux,pci-probe-only", NULL);
- if (prop) {
- if (*prop)
- pci_add_flags(PCI_PROBE_ONLY);
- else
- pci_clear_flags(PCI_PROBE_ONLY);
- }
- }
+ of_pci_check_probe_only();
}
static void __init pSeries_setup_arch(void)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/4] PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property
When pci-host-generic looks for the probe-only property, it seems
to trust the DT to be correctly written, and assumes that there
is a parameter to the property.
Unfortunately, this is not always the case, and some firmware expose
this property naked. The driver ends up making a decision based on
whatever the property pointer points to, which is likely to be junk.
Switch to the common of_pci.c implementation that doesn't suffer
from this problem.
Signed-off-by: Marc Zyngier
---
drivers/pci/host/pci-host-generic.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/pci/host/pci-host-generic.c
b/drivers/pci/host/pci-host-generic.c
index 265dd25..224303d 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -210,7 +210,6 @@ static int gen_pci_probe(struct platform_device *pdev)
int err;
const char *type;
const struct of_device_id *of_id;
- const int *prop;
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
@@ -225,13 +224,7 @@ static int gen_pci_probe(struct platform_device *pdev)
return -EINVAL;
}
- prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
- if (prop) {
- if (*prop)
- pci_add_flags(PCI_PROBE_ONLY);
- else
- pci_clear_flags(PCI_PROBE_ONLY);
- }
+ of_pci_check_probe_only();
of_id = of_match_node(gen_pci_of_match, np);
pci->cfg.ops = of_id->data;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: st_fdma: Firmware filename in DT?
On 04/09/15 11:21, Lee Jones wrote:
Absolutely not. Firmwares have no direct link to DT or platforms that
run DT specifically. They are carried by most platforms these days.
Insisting on firmwares using a DT compatible string format is way off.
If we flip it the other way round, some subsystems derive the firmware
name from the 'node name'. For instance, our zeroth General Purpose
Co-Processor RemoteProc driver has a corresponding node called
'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw'
suffix and et voilà, we load file:
lib/firmware/rproc-st231-gp0-fw
IMO deriving from the node name seems fragile. Also imposing a linux'ism
"rproc" prefix on the firmware name doesn't seem correct as the firmwares
can be shared across OS's. Although this is how remoteproc subsys core
is currently working. It seems a generic DT firmware binding would actually
be most useful for the remoteproc subsystem.
The "rproc-%s-fw", where %s == driver name, is only a fall-back. The
RProc driver is welcome to supply a different firmware name if it
desires. This is where I think a generic 'firmware' property would be
of use.
Hmnnn... That default looks really inappropriate for the ST platforms!
Different generations of the hardware have co-processors with similar
roles (and hence all with names like st231-gp0) but its very rare for
the co-processors to have the same firmware binary.
Thus if the default were applied naively we will end up with a bizarre
situation where the kernel is multi-platform but because the kernel does
not select a unique name for the different firmwares, it becomes
needlessly difficult for the userspace to support multiple platforms.
If there was a generic firmware property for this kind of situation it
is fairly natural to note in the bindings docs the importance of
uniqueness through the generations regarding of the choice of name.
There are other ways a driver can generate a unique name as the
generations of chip develop (compatible strings actually very good for
this) but I don't see it fitting quite so naturally into the binding
docs where it can help future adventurers.
I guess I should also comment about this on the ST remoteproc thread.
I'm curious though as to how the ST remoteproc driver then loads the firmware,
as that name doesn't look like a video or audio firmware filename that I've
seen shipped from ST. IIRC Usually it is named something like
vid_firmware-stih407.elf or audio_firmware-bd-stih407.elf
This driver came direct from ST. I'm using their conventions as a
default, although I would be happy to conduct some investigation into
how this works for releases and suggest a better interface. The only
reason I mentioned it here was because this is how others are using
other SS which are similar in some way. Same for the Firmware SS I
guess.
In my unbiased PoV, I'd much prefer the name was derived from a
predefined propertly.
IMO we should be treating the firmware as a blackbox, and for me that also
includes the filename it is shipped with. Linux drivers making up their own
firmware names based on the name of their subsystem doesn't seem like a
good way forward to me.
I guess the driver author was just using the fall-back convention for
testing. I'm pretty sure RemoteProc isn't being shipped in products
yet.
Both for fdma and co-processors, it is important that the kernel (or
kernel informed by DT properties/compatible strings) is "assertive"
enough to choose names that are sufficiently unique.
That is far more important than honoring vendor filenames.
Vendor filenames could easily be both too simple ("firmware.bin") or too
detailed ("fdma-foo+bar-v1.8_20140811.fw"; the DT should not be encoding
the version number of a firmware loaded from userspace since its not a
fixed property of the platform).
Daniel.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] regulator: pbias: program pbias register offset in pbias driver
On Fri, Sep 04, 2015 at 05:30:24PM +0530, Kishon Vijay Abraham I wrote: > Add separate compatible strings for every platform and populate the > pbias register offset in the driver data. > This helps avoid depending on the dt for pbias register offset. If there are any changes from the already applied patch for this please send an incremental patch. Please don't resend already applied patches. signature.asc Description: Digital signature
Re: [PATCH 02/13] power: bq24257: Add dead battery reporting
On Fri, Sep 04, 2015 at 04:28:05PM +0300, Laurentiu Palcu wrote: > On Tue, Sep 01, 2015 at 04:16:26PM -0500, Andrew F. Davis wrote: > > On 09/01/2015 04:04 PM, Andreas Dannenberg wrote: > > >>> drivers/power/bq24257_charger.c | 4 > > >>> 1 file changed, 4 insertions(+) > > >>> > > >>>diff --git a/drivers/power/bq24257_charger.c > > >>>b/drivers/power/bq24257_charger.c > > >>>index db81356..0b34528 100644 > > >>>--- a/drivers/power/bq24257_charger.c > > >>>+++ b/drivers/power/bq24257_charger.c > > >>>@@ -274,6 +274,10 @@ static int bq24257_power_supply_get_property(struct > > >>>power_supply *psy, > > >>> val->intval = > > >>> POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE; > > >>> break; > > >>> > > >>>+case FAULT_NO_BAT: > > >>>+val->intval = POWER_SUPPLY_HEALTH_DEAD; > > >>>+break; > > >>>+ > > >> > > >>I think the best thing to do would be to return -ENODEV as suggested by > > >>power_supply_sysfs.c:305. Also you should probably add the > > >>POWER_SUPPLY_PROP_PRESENT > > >>property check and set intval to 0 when there is no battery. > > > > > >Good find with -ENODEV. However in this case here the power supply is > > >really a combination of a charger and a battery (which could be split > > >out accordingly but that's a different story). If I were to report > > >-ENODEV I would basically say the entire power supply is gone which is > > >not correct IMHO. Even with a missing battery a system is typically > > >functional as it gets powered through USB and the charger IC is still > > >there and functioning. So I think I would either leave the reporting > > >as *_DEAD or skip the patch. I'm curious if there are additional > > >opinions. > > > > > > > It looks like returning -ENODEV for one property would not mark the whole > > device gone, just POWER_SUPPLY_PROP_HEALTH, but I guess that depends on > > what user-space does with the info. I would think that this is what > > POWER_SUPPLY_PROP_PRESENT is for but different drivers seem to use it for > > different things. Anyway, reporting DEAD for a missing battery is probably > > not the most correct option, maybe drop the patch ( for the cat's sake :) ). > I'm also in favor of dropping this patch for the same reasons. Also, > since the latest phones/tablets do not allow battery removal, it's > highly unlikely we'll even hit this case nowadays. Perhaps, we could use Hi Laurentiu, ok I'll drop it. Understood about the unlikeliness of a battery removal for most devices using LiIon during runtime but a more granular reporting would help in case there is an issue with the HW itself (loose contact, manufacturing defect in the battery, etc.) > DEAD if the battery did not charge and the safety timer expired? > But, if one alters iilimit and sets a limit that's too low for the > battery to charge in time, that doesn't mean the battery is dead... :/ Yeah unless there is a specific register bit or some very specific guidance in a device datasheet as how to detect a truly dead battery I would be a bit hesitant trying to interpret device register values to derive to such a conclusion. Sometimes algorithms which seem simple are only simple because one doesn't consider all the corner cases initially :) > That's the main reason I'm so reluctant on having properties like charge > current, charge voltage, or even input current limit, writable in > sysfs. Agreed from a safety perspective. And if someone really wants to play with fire (pun intended) they can always change the driver in their own copy of the Kernel. -- Andreas Dannenberg Texas Instruments Inc -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] devicetree: Add led-backlight binding
On 09/01/2015 01:12 AM, Rob Herring wrote:
On Wed, Aug 26, 2015 at 4:56 AM, Jacek Anaszewski
wrote:
On 08/26/2015 11:11 AM, Tomi Valkeinen wrote:
On 26/08/15 10:07, Jacek Anaszewski wrote:
On 08/25/2015 05:41 PM, Tomi Valkeinen wrote:
On 25/08/15 16:39, Jacek Anaszewski wrote:
+Example:
+
+backlight {
+compatible = "led-backlight";
+leds = <&backlight_led>;
+
+brightness-levels = <0 4 8 16 32 64 128 255>;
brightness level is not a suitable unit for describing LED brightness
in a Device Tree, as it is not a physical unit. We have
led-max-microamp
property for this, expressed in microamperes, please refer to [0] from
linux-next.
Hmm, ok, but what should the driver do with microamperes? As far as I
see, "enum led_brightness" (which is between 0-255) is used to set the
brightness to LEDs. I don't see any function accepting microamperes.
This is implementation detail. You can convert microamperes to
enum led_brightness in the driver. Please refer to the discussion [1].
The led_set_brightness() takes "enum led_brightness", so I don't
understand what this driver would do with the microampere value. It
could, of course, do an arbitrary conversion, say, direct mapping of the
mA value to brightness, but that would just confuse things further.
OK, I was looking at the problem from LED-centric perspective. Indeed,
backlight subsystem has no other way to pass brightness to the LED
subsystem than in the form of levels. However, the last word belongs
to DT maintainer in this matter.
Cc'ing devicetree@vger.kernel.org.
I don't have a simple answer for you...
There was a similar discussion for pm8941-wled and
"default-brightness-level" units[1]. The conclusion was it should be
units matching the h/w so that there is no conversion between
bootloader and OS units to h/w units. That principle probably applies
here.
If the brightness levels are non-linear, then you need a translation
from percent to h/w level. What's needed here for h/w levels depends
on whether the brightness control is PWM, current control or both. For
PWM, units of the PWM control makes sense. For current control, units
of microamps probably makes sense. I don't know what you do with both,
but I have seen that h/w (FSL PMICs).
This all certainly needs some more work on defining some common
binding. We already have some bindings for backlights with the LED and
PWM bindings (perhaps incomplete?). Do we need another way here? This
also introduces possibility of multiple ways to define GPIO controlled
backlights: gpio -> gpio-leds -> led-backlight or gpio ->
gpio-backlight. We don't want that...
This problem is not really specific at all to backlights, but applies
to all LEDs. Some LEDs you may not have control beyond on/off or
really care about fine-grained control of level, but they are really
no different. The main unique thing about backlights is what display
are they associated with.
Some time ago I was trying to add common LEDs DT properties that would
have defined brightness in levels, but other people insisted on
microamperes [1]. The discussion was focused on flash LEDs, where
currents are significantly greater and there is risk of hardware
damage in case underrated LED is connected [2]. Having the property in
microamperes removes the need for consulting documentation to check
current value for given brightness level.
As it was mentioned earlier in this thread, this approach is not
suitable for PWM driven leds, and they have their own bindings,
which allow for describing brightness in levels. Probably we
should add some note to the common LEDs DT bindings that would
state explicitly that such exceptions are allowed.
In case of backlights which are built upon LED class devices we could
stick to brightness levels, as a LED class driver will assert
brightness level to the leds-max-microamp value anyway.
[1] http://www.spinics.net/lists/linux-leds/msg03416.html
[2] http://patchwork.ozlabs.org/patch/456622/
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/7] staging: board: Add support for devices with complex dependencies
Hi Ulf,
On Fri, 4 Sep 2015, Ulf Hansson wrote:
> On 3 September 2015 at 15:35, Geert Uytterhoeven wrote:
> > On Thu, Sep 3, 2015 at 2:53 PM, Ulf Hansson wrote:
> >> On 17 June 2015 at 10:38, Geert Uytterhoeven
> >> wrote:
> >>> Add support for easy registering of one ore more platform devices that
> >>> may:
> >>> - need clocks that are described in DT,
> >>> - be part of a PM Domain.
> >
> >>> diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c
> >>> index 8712f566b31196e0..29d456e29f38feac 100644
> >>> --- a/drivers/staging/board/board.c
> >>> +++ b/drivers/staging/board/board.c
> >
> >>> +int __init board_staging_register_device(const struct board_staging_dev
> >>> *dev)
> >>> +{
> >>> + struct platform_device *pdev = dev->pdev;
> >>> + unsigned int i;
> >>> + int error;
> >>> +
> >>> + pr_debug("Trying to register device %s\n", pdev->name);
> >>> + if (board_staging_dt_node_available(pdev->resource,
> >>> + pdev->num_resources)) {
> >>> + pr_warn("Skipping %s, already in DT\n", pdev->name);
> >>> + return -EEXIST;
> >>> + }
> >>> +
> >>> + board_staging_gic_fixup_resources(pdev->resource,
> >>> pdev->num_resources);
> >>> +
> >>> + for (i = 0; i < dev->nclocks; i++)
> >>> + board_staging_register_clock(&dev->clocks[i]);
> >>> +
> >>> + error = platform_device_register(pdev);
> >>> + if (error) {
> >>> + pr_err("Failed to register device %s (%d)\n", pdev->name,
> >>> + error);
> >>> + return error;
> >>> + }
> >>> +
> >>> + if (dev->domain)
> >>> + __pm_genpd_name_add_device(dev->domain, &pdev->dev, NULL);
> >>
> >> Urgh, this managed to slip through my filters.
> >>
> >> It seems like we almost managed to remove all users of the
> >> "..._name_add..." APIs for genpd. If hasn't been for $subject patch.
> >> :-)
> >>
> >> Now, I realize this is already too late here, but let's try to fix
> >> this before it turns into a bigger issue.
> >>
> >> Geert, do you think it's possible to convert into using the non-named
> >> bases APIs?
> >
> > That will be difficult. This code is meant to use drivers that are not yet
> > DT-aware on DT-based systems. Hence it uses platform devices with named PM
> > domains, while the PM domains are described in DT.
> > I don't think there's another way to look up a PM domain by name, is there?
>
> As a matter of fact there are, especially for those genpds that has
> been created through DT as in this case. The API to use is
> of_genpd_get_from_provider() to find the struct generic_pm_domain.
Thanks!
> Yes, I do realize that you need to manage the parsing of the domain
> name to make sure it's the one you want, but I would rather keep that
> "hack" in this driver than in the generic API.
OK. It turned out not to be that complex, cfr. the patch below.
For now this supports PM domains with "#power-domain-cells = <0>" only,
but of course it can be extended if the need arises.
I've also tried:
np = of_find_compatible_node(NULL, NULL, "renesas,sysc-rmobile");
np = of_find_node_by_name(np, "a4lc");
(the second step is needed because of the domain hierarchy in DT), but that
would require adding the compatible string to struct board_staging_dev,
and doesn't work if you have multiple identical power providers.
I hope you like it?
>From 5fb11904845eb929a5b3382a9d5153c151cde1cf Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven
Date: Fri, 4 Sep 2015 16:52:33 +0200
Subject: [PATCH/RFC] staging: board: Migrate away from
__pm_genpd_name_add_device()
The named genpd APIs are deprecated. Hence convert the board staging
code from using genpd names to DT node paths.
For now this supports PM domains with "#power-domain-cells = <0>" only.
Signed-off-by: Geert Uytterhoeven
---
drivers/staging/board/armadillo800eva.c | 2 +-
drivers/staging/board/board.c | 36 -
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/board/armadillo800eva.c
b/drivers/staging/board/armadillo800eva.c
index 0165591a52443c46..912c96b0536def7c 100644
--- a/drivers/staging/board/armadillo800eva.c
+++ b/drivers/staging/board/armadillo800eva.c
@@ -91,7 +91,7 @@ static const struct board_staging_dev
armadillo800eva_devices[] __initconst = {
.pdev = &lcdc0_device,
.clocks = lcdc0_clocks,
.nclocks= ARRAY_SIZE(lcdc0_clocks),
- .domain = "a4lc",
+ .domain =
"/system-controller@e618/pm-domains/c5/a4lc@1"
},
};
diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c
index 29d456e29f38feac..3eb5eb8f069c236d 100644
--- a/drivers/staging/board/board.c
+++ b/drivers/staging/board/board.c
@@ -135,6 +135,40 @@ int __init board_staging_regist
Re: st_fdma: Firmware filename in DT?
> On Sep 4, 2015, at 7:54 AM, Lee Jones wrote: > > On Fri, 04 Sep 2015, Rob Herring wrote: > >> On Fri, Sep 4, 2015 at 8:26 AM, Lee Jones wrote: >>> On Fri, 04 Sep 2015, Arnd Bergmann wrote: >>> On Friday 04 September 2015, Lee Jones wrote: >>> If we flip it the other way round, some subsystems derive the firmware >>> name from the 'node name'. For instance, our zeroth General Purpose >>> Co-Processor RemoteProc driver has a corresponding node called >>> 'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw' >>> suffix and et voilà, we load file: >>> >>> lib/firmware/rproc-st231-gp0-fw >> >> IMO deriving from the node name seems fragile. Also imposing a linux'ism >> "rproc" prefix on the firmware name doesn't seem correct as the firmwares >> can be shared across OS's. Although this is how remoteproc subsys core >> is currently working. It seems a generic DT firmware binding would >> actually >> be most useful for the remoteproc subsystem. > > The "rproc-%s-fw", where %s == driver name, is only a fall-back. The > RProc driver is welcome to supply a different firmware name if it > desires. This is where I think a generic 'firmware' property would be > of use. The firmware file name is agreed on between the device driver and the file system, so encoding the linux driver name in it seems appropriate. Generally speaking, I'd say a good policy would be to try basing the firmware name on the "compatible" property strings. That property already contains a hierarchical list of models, which makes it particularly easy to have firmware files for specific models or those that are shared across multiple variations if necessary. Just ask for the most specific compatible string first and try the more specific compatible strings (with an appropriate prefix and/or postfix added by the driver) until a file is found. >>> >>> It depends what you mean by "basing the firmware name on the >>> \"compatible\" property" here. If you mean actually renaming the >>> firmware binary file to match a driver's compatible string, that's >>> absolutely out of the question. Firmwares are not only OS agnostic, >>> but are also independent of any H/W description language a particular >>> OS or platform might be using. Using DT'isums to rename these >>> binaries is not logical. >>> >>> However, if you mean simply match on compatible string and supply the >>> name from within the driver, that's closer to the mark (as then we can >>> at least keep it in-house [kernel]), but it's still not particularly >>> practical for the aforementioned reasons mentioned by Peter earlier. >>> >>> Why not just create a new 'firmware' property? Simples! [0] >> >> Someone give me some evidence that other OS's use or will use the same >> names. Does *BSD use linux-firmware would be enough. With the >> complaints I get that bindings are just Linux driver properties, I'm >> not inclined to take this. Having a filename does imply the OS has a >> filesystem and drivers can access the filesystem which may not always >> be true. > > Peter already provided a real-world example of multiple OSes using the > same Firmware based on his experience at ST. Any vendor using this API > who doesn't _soley_ use Linux is likely to use the same firmware files > across all platforms. The co-processor's jobs don't often change just > because the platform is running a different OS. > > I don't know enough about the inner workings of other Operating > Systems to comment on your final statement, but if they can get > access to the filesystem them they'll need the name too. If they > don't have access to the firmware file, then they don't require the > property and it becomes unused by them. That's not an issue is it? Based on my experience with FreeBSD and different firmware and such, I’d have to agree with Peter. FreeBSD often uses the vendor supplied firmware, just like Linux, because that firmware establishes the ABI for talking to the device. This ABI is typically OS agnostic, and when it isn’t that’s more the exception than the rule. As for ‘not all OSes provide a filesystem’ Sure, but who cares. While the filesystem provides a natural mapping of the property to the binary data to load (e.g. os-firmware-path + “/“ + property-name), that’s not the only way to map names to binary blobs. FreeBSD itself allows one to compile firmware images into the kernel, and those images are addressed / found by name, even though no filesystem is involved based on what the driver desires to load. I would imagine that an OS minimal enough to not support a filesystem would run into this sort of issue often enough to provide a mechanism similar to FreeBSD. Failing that, I’d imagine driver writers would compile images into their drivers on such a system and use the name to pick which one to load. Warner signature.asc De
Re: st_fdma: Firmware filename in DT?
> On Sep 4, 2015, at 7:04 AM, Arnd Bergmann wrote: > > On Friday 04 September 2015, Lee Jones wrote: If we flip it the other way round, some subsystems derive the firmware name from the 'node name'. For instance, our zeroth General Purpose Co-Processor RemoteProc driver has a corresponding node called 'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw' suffix and et voilà, we load file: lib/firmware/rproc-st231-gp0-fw >>> >>> IMO deriving from the node name seems fragile. Also imposing a linux'ism >>> "rproc" prefix on the firmware name doesn't seem correct as the firmwares >>> can be shared across OS's. Although this is how remoteproc subsys core >>> is currently working. It seems a generic DT firmware binding would actually >>> be most useful for the remoteproc subsystem. >> >> The "rproc-%s-fw", where %s == driver name, is only a fall-back. The >> RProc driver is welcome to supply a different firmware name if it >> desires. This is where I think a generic 'firmware' property would be >> of use. > > The firmware file name is agreed on between the device driver and the > file system, so encoding the linux driver name in it seems appropriate. Encoding the driver name is not OS independent. It fosters the impression that DT isn’t OS independent, but rather some silly Linux toy and hurts wider adaptation. > Generally speaking, I'd say a good policy would be to try basing > the firmware name on the "compatible" property strings. That property > already contains a hierarchical list of models, which makes it particularly > easy to have firmware files for specific models or those that are shared > across multiple variations if necessary. Just ask for the most specific > compatible string first and try the more specific compatible strings > (with an appropriate prefix and/or postfix added by the driver) until > a file is found. I think this is a horrible policy. It is an ugly kludge that is fragile to change. While it sounds cool, I don’t think it is really a viable one. It requires different compatibility names for different bits of hardware that might otherwise be the same just to get different firmware. Sometimes this may be OK, but it does seem needlessly limiting for systems that may have firmware “images” that are loaded into one hardware block, but actually control other blocks indirectly (where the different compat strings would be). Warner signature.asc Description: Message signed with OpenPGP using GPGMail
Re: st_fdma: Firmware filename in DT?
> On Sep 4, 2015, at 4:21 AM, Lee Jones wrote: > > We could do it by parsing the node name e.g. fdma0-audio, or by adding > a "instance" DT property to the node? Generally we try to avoid caring about node names. Having some index or numbering also comes up which we also try to avoid. Generally, if you care about which instance you use for something, then there is some property you care about and should add. >>> >>> Right, the alternative is a property like the ones already used. >>> However, as these are becoming more prevalent I suggested >>> standardising the property to avoid all these vendor specific firmware >>> properties cluttering up the place. >> >> I agree a proliferation of vendor specific firmware properties isn't >> s good way forward. >> >>> firmware = "firmwarename.fw"; >>> OR >>> firmware-name = "firmwarename.fw"; >>> >>> ... seems appropriate. >> >> Either of those is fine with me. > > Just need a DT nod and I'll happily code it up. From a FreeBSD perspective, having a filename for the firmware to load for this node is fine. It doesn’t impose a substantial burden on the OS so long as the choices of where that file lives is up to the OS and not encoded in the property. A note saying ‘this firmware should be loaded’ seems a reasonable description of the hardware since the DT tells us many things about the device, and those things may well be dependent on which firmware is loaded. Having an explicit name is good since it helps insulate from DT and doesn’t force vendors to do silly renames. It also allows for multiple devices of the same type to have different firmware loaded. FreeBSD would generally put these things in /boot/firmware and it has a generalized mechanism to load the firmware at run time that’s based on this. While the path names are flexible, having the firmware live in a central place is useful from an automation point of view. Having just a name, and not a full path, enables this policy, while still allowing others to have other policies. Linux distributions would be free to do whatever they wanted and implement other policies than FreeBSD. So a property like this, with the semantics discussed, seems to meet the OS independent test. Warner signature.asc Description: Message signed with OpenPGP using GPGMail
Re: st_fdma: Firmware filename in DT?
On Fri, 04 Sep 2015, Rob Herring wrote: > On Fri, Sep 4, 2015 at 8:26 AM, Lee Jones wrote: > > On Fri, 04 Sep 2015, Arnd Bergmann wrote: > > > >> On Friday 04 September 2015, Lee Jones wrote: > >> > > > If we flip it the other way round, some subsystems derive the > >> > > > firmware > >> > > > name from the 'node name'. For instance, our zeroth General Purpose > >> > > > Co-Processor RemoteProc driver has a corresponding node called > >> > > > 'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw' > >> > > > suffix and et voilà, we load file: > >> > > > > >> > > > lib/firmware/rproc-st231-gp0-fw > >> > > > >> > > IMO deriving from the node name seems fragile. Also imposing a > >> > > linux'ism > >> > > "rproc" prefix on the firmware name doesn't seem correct as the > >> > > firmwares > >> > > can be shared across OS's. Although this is how remoteproc subsys core > >> > > is currently working. It seems a generic DT firmware binding would > >> > > actually > >> > > be most useful for the remoteproc subsystem. > >> > > >> > The "rproc-%s-fw", where %s == driver name, is only a fall-back. The > >> > RProc driver is welcome to supply a different firmware name if it > >> > desires. This is where I think a generic 'firmware' property would be > >> > of use. > >> > >> The firmware file name is agreed on between the device driver and the > >> file system, so encoding the linux driver name in it seems appropriate. > >> > >> Generally speaking, I'd say a good policy would be to try basing > >> the firmware name on the "compatible" property strings. That property > >> already contains a hierarchical list of models, which makes it particularly > >> easy to have firmware files for specific models or those that are shared > >> across multiple variations if necessary. Just ask for the most specific > >> compatible string first and try the more specific compatible strings > >> (with an appropriate prefix and/or postfix added by the driver) until > >> a file is found. > > > > It depends what you mean by "basing the firmware name on the > > \"compatible\" property" here. If you mean actually renaming the > > firmware binary file to match a driver's compatible string, that's > > absolutely out of the question. Firmwares are not only OS agnostic, > > but are also independent of any H/W description language a particular > > OS or platform might be using. Using DT'isums to rename these > > binaries is not logical. > > > > However, if you mean simply match on compatible string and supply the > > name from within the driver, that's closer to the mark (as then we can > > at least keep it in-house [kernel]), but it's still not particularly > > practical for the aforementioned reasons mentioned by Peter earlier. > > > > Why not just create a new 'firmware' property? Simples! [0] > > Someone give me some evidence that other OS's use or will use the same > names. Does *BSD use linux-firmware would be enough. With the > complaints I get that bindings are just Linux driver properties, I'm > not inclined to take this. Having a filename does imply the OS has a > filesystem and drivers can access the filesystem which may not always > be true. Peter already provided a real-world example of multiple OSes using the same Firmware based on his experience at ST. Any vendor using this API who doesn't _soley_ use Linux is likely to use the same firmware files across all platforms. The co-processor's jobs don't often change just because the platform is running a different OS. I don't know enough about the inner workings of other Operating Systems to comment on your final statement, but if they can get access to the filesystem them they'll need the name too. If they don't have access to the firmware file, then they don't require the property and it becomes unused by them. That's not an issue is it? -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: st_fdma: Firmware filename in DT?
On Fri, Sep 4, 2015 at 8:26 AM, Lee Jones wrote: > On Fri, 04 Sep 2015, Arnd Bergmann wrote: > >> On Friday 04 September 2015, Lee Jones wrote: >> > > > If we flip it the other way round, some subsystems derive the firmware >> > > > name from the 'node name'. For instance, our zeroth General Purpose >> > > > Co-Processor RemoteProc driver has a corresponding node called >> > > > 'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw' >> > > > suffix and et voilà, we load file: >> > > > >> > > > lib/firmware/rproc-st231-gp0-fw >> > > >> > > IMO deriving from the node name seems fragile. Also imposing a linux'ism >> > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares >> > > can be shared across OS's. Although this is how remoteproc subsys core >> > > is currently working. It seems a generic DT firmware binding would >> > > actually >> > > be most useful for the remoteproc subsystem. >> > >> > The "rproc-%s-fw", where %s == driver name, is only a fall-back. The >> > RProc driver is welcome to supply a different firmware name if it >> > desires. This is where I think a generic 'firmware' property would be >> > of use. >> >> The firmware file name is agreed on between the device driver and the >> file system, so encoding the linux driver name in it seems appropriate. >> >> Generally speaking, I'd say a good policy would be to try basing >> the firmware name on the "compatible" property strings. That property >> already contains a hierarchical list of models, which makes it particularly >> easy to have firmware files for specific models or those that are shared >> across multiple variations if necessary. Just ask for the most specific >> compatible string first and try the more specific compatible strings >> (with an appropriate prefix and/or postfix added by the driver) until >> a file is found. > > It depends what you mean by "basing the firmware name on the > \"compatible\" property" here. If you mean actually renaming the > firmware binary file to match a driver's compatible string, that's > absolutely out of the question. Firmwares are not only OS agnostic, > but are also independent of any H/W description language a particular > OS or platform might be using. Using DT'isums to rename these > binaries is not logical. > > However, if you mean simply match on compatible string and supply the > name from within the driver, that's closer to the mark (as then we can > at least keep it in-house [kernel]), but it's still not particularly > practical for the aforementioned reasons mentioned by Peter earlier. > > Why not just create a new 'firmware' property? Simples! [0] Someone give me some evidence that other OS's use or will use the same names. Does *BSD use linux-firmware would be enough. With the complaints I get that bindings are just Linux driver properties, I'm not inclined to take this. Having a filename does imply the OS has a filesystem and drivers can access the filesystem which may not always be true. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: st_fdma: Firmware filename in DT?
On Fri, 04 Sep 2015, Arnd Bergmann wrote: > On Friday 04 September 2015, Lee Jones wrote: > > > > If we flip it the other way round, some subsystems derive the firmware > > > > name from the 'node name'. For instance, our zeroth General Purpose > > > > Co-Processor RemoteProc driver has a corresponding node called > > > > 'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw' > > > > suffix and et voilà, we load file: > > > > > > > > lib/firmware/rproc-st231-gp0-fw > > > > > > IMO deriving from the node name seems fragile. Also imposing a linux'ism > > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares > > > can be shared across OS's. Although this is how remoteproc subsys core > > > is currently working. It seems a generic DT firmware binding would > > > actually > > > be most useful for the remoteproc subsystem. > > > > The "rproc-%s-fw", where %s == driver name, is only a fall-back. The > > RProc driver is welcome to supply a different firmware name if it > > desires. This is where I think a generic 'firmware' property would be > > of use. > > The firmware file name is agreed on between the device driver and the > file system, so encoding the linux driver name in it seems appropriate. > > Generally speaking, I'd say a good policy would be to try basing > the firmware name on the "compatible" property strings. That property > already contains a hierarchical list of models, which makes it particularly > easy to have firmware files for specific models or those that are shared > across multiple variations if necessary. Just ask for the most specific > compatible string first and try the more specific compatible strings > (with an appropriate prefix and/or postfix added by the driver) until > a file is found. It depends what you mean by "basing the firmware name on the \"compatible\" property" here. If you mean actually renaming the firmware binary file to match a driver's compatible string, that's absolutely out of the question. Firmwares are not only OS agnostic, but are also independent of any H/W description language a particular OS or platform might be using. Using DT'isums to rename these binaries is not logical. However, if you mean simply match on compatible string and supply the name from within the driver, that's closer to the mark (as then we can at least keep it in-house [kernel]), but it's still not particularly practical for the aforementioned reasons mentioned by Peter earlier. Why not just create a new 'firmware' property? Simples! [0] [0] http://www.oxforddictionaries.com/definition/english/simples -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/13] power: bq24257: Add dead battery reporting
On Tue, Sep 01, 2015 at 04:16:26PM -0500, Andrew F. Davis wrote: > > > On 09/01/2015 04:04 PM, Andreas Dannenberg wrote: > >Andrew- thanks for your feeback. Answers below... > > > >On Tue, Sep 01, 2015 at 02:33:11PM -0500, Andrew F. Davis wrote: > >>On 08/31/2015 09:10 PM, Andreas Dannenberg wrote: > >>>A missing/disconnected battery is now reported as dead rather than an > >>>unspecified failure via the charger's sysfs health property. > >>> > >>>$ cat health > >>>Dead > >> > >>Poor cat :( > >> > > > >I had to laugh pretty hard when I saw that getting printed onto the > >console for the first time so I had to include it here. > > > >>> > >>>Signed-off-by: Andreas Dannenberg > >>>--- > >>> drivers/power/bq24257_charger.c | 4 > >>> 1 file changed, 4 insertions(+) > >>> > >>>diff --git a/drivers/power/bq24257_charger.c > >>>b/drivers/power/bq24257_charger.c > >>>index db81356..0b34528 100644 > >>>--- a/drivers/power/bq24257_charger.c > >>>+++ b/drivers/power/bq24257_charger.c > >>>@@ -274,6 +274,10 @@ static int bq24257_power_supply_get_property(struct > >>>power_supply *psy, > >>> val->intval = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE; > >>> break; > >>> > >>>+ case FAULT_NO_BAT: > >>>+ val->intval = POWER_SUPPLY_HEALTH_DEAD; > >>>+ break; > >>>+ > >> > >>I think the best thing to do would be to return -ENODEV as suggested by > >>power_supply_sysfs.c:305. Also you should probably add the > >>POWER_SUPPLY_PROP_PRESENT > >>property check and set intval to 0 when there is no battery. > > > >Good find with -ENODEV. However in this case here the power supply is > >really a combination of a charger and a battery (which could be split > >out accordingly but that's a different story). If I were to report > >-ENODEV I would basically say the entire power supply is gone which is > >not correct IMHO. Even with a missing battery a system is typically > >functional as it gets powered through USB and the charger IC is still > >there and functioning. So I think I would either leave the reporting > >as *_DEAD or skip the patch. I'm curious if there are additional > >opinions. > > > > It looks like returning -ENODEV for one property would not mark the whole > device gone, just POWER_SUPPLY_PROP_HEALTH, but I guess that depends on > what user-space does with the info. I would think that this is what > POWER_SUPPLY_PROP_PRESENT is for but different drivers seem to use it for > different things. Anyway, reporting DEAD for a missing battery is probably > not the most correct option, maybe drop the patch ( for the cat's sake :) ). I'm also in favor of dropping this patch for the same reasons. Also, since the latest phones/tablets do not allow battery removal, it's highly unlikely we'll even hit this case nowadays. Perhaps, we could use DEAD if the battery did not charge and the safety timer expired? But, if one alters iilimit and sets a limit that's too low for the battery to charge in time, that doesn't mean the battery is dead... :/ That's the main reason I'm so reluctant on having properties like charge current, charge voltage, or even input current limit, writable in sysfs. laurentiu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: st_fdma: Firmware filename in DT?
On Friday 04 September 2015, Lee Jones wrote: > > > If we flip it the other way round, some subsystems derive the firmware > > > name from the 'node name'. For instance, our zeroth General Purpose > > > Co-Processor RemoteProc driver has a corresponding node called > > > 'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw' > > > suffix and et voilà, we load file: > > > > > > lib/firmware/rproc-st231-gp0-fw > > > > IMO deriving from the node name seems fragile. Also imposing a linux'ism > > "rproc" prefix on the firmware name doesn't seem correct as the firmwares > > can be shared across OS's. Although this is how remoteproc subsys core > > is currently working. It seems a generic DT firmware binding would actually > > be most useful for the remoteproc subsystem. > > The "rproc-%s-fw", where %s == driver name, is only a fall-back. The > RProc driver is welcome to supply a different firmware name if it > desires. This is where I think a generic 'firmware' property would be > of use. The firmware file name is agreed on between the device driver and the file system, so encoding the linux driver name in it seems appropriate. Generally speaking, I'd say a good policy would be to try basing the firmware name on the "compatible" property strings. That property already contains a hierarchical list of models, which makes it particularly easy to have firmware files for specific models or those that are shared across multiple variations if necessary. Just ask for the most specific compatible string first and try the more specific compatible strings (with an appropriate prefix and/or postfix added by the driver) until a file is found. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: Fix Makefile target for sun4i-a10-itead-iteaduino-plus
On Thu, Sep 3, 2015 at 5:57 PM, Maxime Ripard wrote: > Hi Josh > > On Wed, Sep 02, 2015 at 04:18:12PM -0400, Josh Boyer wrote: >> linux-ker...@vger.kernel.org >> Bcc: >> Subject: [PATCH] ARM: dts: Fix Makefile target for >> sun4i-a10-itead-iteaduino-plus >> Reply-To: > > Hmmm, something went wrong with your mail header. Well, that's embarrassing. I've resent it. >> ommit 79ae3e66f8d (ARM: dts: sun4i: Add Iteaduino Plus A10) added a new > > Missing opening "C" Fixed in the resend. Thanks for looking it over. josh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND] ARM: dts: Fix Makefile target for sun4i-a10-itead-iteaduino-plus
Commit 79ae3e66f8d (ARM: dts: sun4i: Add Iteaduino Plus A10) added a new make target for the sun4i-a10-itead-iteaduino-plus dts file, but mistakenly used .dts instead of the correct .dtb suffix. This resulted in a build error like: scripts/Makefile.dtbinst:42: target 'sun4i-a10-itead-iteaduino-plus.dts' doesn't match the target pattern when doing a make dtbs_install. Fix it to use the proper file name. Signed-off-by: Josh Boyer --- arch/arm/boot/dts/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 233159d2eaab..bb8fa023d574 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -578,7 +578,7 @@ dtb-$(CONFIG_MACH_SUN4I) += \ sun4i-a10-hackberry.dtb \ sun4i-a10-hyundai-a7hd.dtb \ sun4i-a10-inet97fv2.dtb \ - sun4i-a10-itead-iteaduino-plus.dts \ + sun4i-a10-itead-iteaduino-plus.dtb \ sun4i-a10-jesurun-q5.dtb \ sun4i-a10-marsboard.dtb \ sun4i-a10-mini-xplus.dtb \ -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/2] regulator: Fix pbias regulator enable
On 4 September 2015 at 14:00, Kishon Vijay Abraham I wrote: > vsel_reg and enable_reg of the pbias regulator descriptor should actually > have the offset from syscon. > > However after > "ARM: dts: : add minimal l4 bus layout with control module > support" > vsel_reg and enable_reg started to have the absolute address because > of address translation that happens due to pbias node made as the > child node of syscon. This breaks the pbias regulator enable. > > This series adds the 'offset' to be populated in vsel_reg and enable_reg > in the pbias driver itself. > > Changes from v2: > *) Squashed all the dt patches into a single patch > > Changes from v1: > *) Fixed Tony's review comments on adding a 'comment' for adding offset in >the driver and adding a warning for using platform_get_resource. > *) Added Tony's Acked-by. > > Tested these patches against mmc -next in omap4 panda, omap3 beagle xm, > dra72 and omap5 uevm > > Kishon Vijay Abraham I (2): > regulator: pbias: program pbias register offset in pbias driver > ARM: dts: : use "ti,pbias-" compatible > string for pbias > > .../bindings/regulator/pbias-regulator.txt |7 ++- > arch/arm/boot/dts/dra7.dtsi|2 +- > arch/arm/boot/dts/omap2430.dtsi|2 +- > arch/arm/boot/dts/omap3.dtsi |2 +- > arch/arm/boot/dts/omap4.dtsi |2 +- > arch/arm/boot/dts/omap5.dtsi |2 +- > drivers/regulator/pbias-regulator.c| 56 > +--- > 7 files changed, 61 insertions(+), 12 deletions(-) > > -- > 1.7.9.5 > Okay, just to be clear on the way forward. I spoked with Mark Brown offlist, and he will/has picked up the regulator patch and will send it as fix for the 4.3 rc[n]. Regarding the ARM patch here, I guess Tony might as well handle it and send through arm-soc, especially since the regression won't be fixed within my mmc tree anyway. So, I am going to leave my next branch as is - and thus relying on that the regression for OMAP will be fixed in some the 4.3 rc[n]. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices
On 4 September 2015 at 13:59, Vignesh R wrote:
> In addition to providing direct access to SPI bus, some spi controller
> hardwares (like ti-qspi) provide special memory mapped port
> to accesses SPI flash devices in order to increase read performance.
> This means the controller can automatically send the SPI signals
> required to read data from the SPI flash device.
> For this, spi controller needs to know flash specific information like
> read command to use, dummy bytes and address width. Once these settings
> are populated in hardware registers, any read accesses to flash's memory
> map region(SoC specific) through memcpy or mem-to-mem DMA copy will be
> handled by controller hardware. The hardware will automatically generate
> spi signals required to read data from flash and present it to CPU or
> DMA engine.
>
> Introduce spi_mtd_mmap_read() method to support memory mapped read
> over SPI flash devices. SPI master drivers can implement this method to
> support memory mapped read interfaces. m25p80 flash driver and other
> flash drivers can call this to request memory mapped read. The interface
> should only be used mtd flashes and cannot be used with other spi devices.
>
> Signed-off-by: Vignesh R
> ---
> include/linux/spi/spi.h | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index d673072346f2..b74a3f169fc2 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -293,6 +293,23 @@ static inline void spi_unregister_driver(struct
> spi_driver *sdrv)
> * @handle_err: the subsystem calls the driver to handle an error that occurs
> * in the generic implementation of transfer_one_message().
> * @unprepare_message: undo any work done by prepare_message().
> + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory
> + * mapped interface to communicate with mtd flashes.
> + * For this, spi controller needs to know flash
> + * memory settings like read command to use, dummy
> + * bytes and address width. Once these settings are
> + * populated in hardware registers, any read
> + * accesses to flash's memory map region(as defined
> + * by SoC) through memcpy or mem-to-mem DMA copy
> + * will be handled by controller hardware. The
> + * hardware will automatically generate spi signals
> + * required to read data from flash and present it
> + * to CPU or DMA. SPI master drivers can use this
> + * callback to implement memory mapped read
> + * interface. Flash driver (like m25p80) requests
> + * memory mapped read via this method. The interface
> + * should only be used mtd flashes and cannot be
> + * used with other spi devices.
> * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
> * number. Any individual value may be -ENOENT for CS lines that
> * are not GPIOs (driven by the SPI controller itself).
> @@ -438,6 +455,10 @@ struct spi_master {
>struct spi_message *message);
> int (*unprepare_message)(struct spi_master *master,
> struct spi_message *message);
> + int (*spi_mtd_mmap_read)(struct spi_device *spi,
> +loff_t from, size_t len, size_t *retlen,
> +u_char *buf, u8 read_opcode,
> +u8 addr_width, u8 dummy_bytes);
This looks un-manageable to know spi core or master knows what are the
command or opcode used by spi-nor flash and also I think mmap support
seems to be flash related or any justification this is spi bus
master/controller feature?
thanks!
--
Jagan | openedev.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [[PATCH v2] 2/2] Altera Modular ADC driver support
> +static int alt_modular_adc_probe(struct platform_device *pdev)
> +{
> + struct altera_adc *adc;
> + struct device_node *np = pdev->dev.of_node;
> + struct iio_dev *indio_dev;
> + struct resource *mem;
> + int ret;
> +
> + if (!np)
> + return -ENODEV;
> +
> + indio_dev = iio_device_alloc(sizeof(struct altera_adc));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "failed allocating iio device\n");
> + return -ENOMEM;
> + }
> +
> + adc = iio_priv(indio_dev);
> +
> + mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "sequencer_csr");
> + adc->seq_regs = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(adc->seq_regs)) {
> + ret = PTR_ERR(adc->seq_regs);
> + goto err_iio;
> + }
> +
> + mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "sample_store_csr");
> + adc->sample_regs = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(adc->sample_regs)) {
> + ret = PTR_ERR(adc->sample_regs);
> + goto err_iio;
> + }
> +
> + ret = alt_modular_adc_parse_dt(indio_dev, &pdev->dev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to parse device tree\n");
> + goto err_iio;
> + }
> +
> + ret = alt_modular_adc_channel_init(indio_dev);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed initialize ADC channels\n");
> + goto err_iio;
> + }
> +
> + platform_set_drvdata(pdev, indio_dev);
> +
> + indio_dev->name = dev_name(&pdev->dev);
> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->dev.of_node = pdev->dev.of_node;
> + indio_dev->info = &adc_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->num_channels = adc->slot_count;
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto err_iio;
> +
> + /* Disable Interrupt */
> + writel_relaxed(0, (adc->sample_regs + ADC_IER_REG));
Why disable interrupts?
> +
> + /* Start Continuous Sampling */
> + writel_relaxed((ADC_RUN_MSK), (adc->seq_regs + ADC_CMD_REG));
> +
> + return 0;
> +
> +
> +err_iio:
> + iio_device_free(indio_dev);
> + return ret;
> +}
> +
> +static int alt_modular_adc_remove(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct altera_adc *adc = iio_priv(indio_dev);
> +
> + /* Stop ADC */
> + writel((ADC_STOP_MSK), (adc->seq_regs + ADC_CMD_REG));
> +
> + /* Unregister ADC */
> + iio_device_unregister(indio_dev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver altr_modular_adc_driver = {
> + .probe = alt_modular_adc_probe,
> + .remove = alt_modular_adc_remove,
> + .driver = {
> + .name = "alt-modular-adc",
> + .owner = THIS_MODULE,
> + .of_match_table = alt_modular_adc_match,
> + },
> +};
> +
> +
> +module_platform_driver(altr_modular_adc_driver);
> +
> +MODULE_DESCRIPTION("Altera Modular ADC Driver");
> +MODULE_AUTHOR("Chee Nouk Phoon ");
> +MODULE_LICENSE("GPL v2");
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: ux500: fix typo in regulator names
On 4 September 2015 at 13:48, Ulf Hansson wrote:
> On 5 August 2015 at 14:22, Tomeu Vizoso wrote:
>> Apparently an extra _reg suffix was appended to the end of the names of
>> regulators ab8500_ext2_reg and ab8500_ext3_reg.
>>
>> Signed-off-by: Tomeu Vizoso
>
> You should probably include Linus Walleij as he is the maintainer of ux500.
Thanks, maybe the file should be added to MAINTAINERS?
Regards,
Tomeu
> Acked-by: Ulf Hansson
>
>> ---
>> arch/arm/boot/dts/ste-snowball.dts | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/ste-snowball.dts
>> b/arch/arm/boot/dts/ste-snowball.dts
>> index 32a5ccb14e7e..07c0e346942c 100644
>> --- a/arch/arm/boot/dts/ste-snowball.dts
>> +++ b/arch/arm/boot/dts/ste-snowball.dts
>> @@ -360,11 +360,11 @@
>> regulator-name =
>> "ab8500-ext-supply1";
>> };
>>
>> - ab8500_ext2_reg_reg: ab8500_ext2 {
>> + ab8500_ext2_reg: ab8500_ext2 {
>> regulator-name =
>> "ab8500-ext-supply2";
>> };
>>
>> - ab8500_ext3_reg_reg: ab8500_ext3 {
>> + ab8500_ext3_reg: ab8500_ext3 {
>> regulator-name =
>> "ab8500-ext-supply3";
>> };
>> };
>> --
>> 2.4.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] ARM: dts: >: fix address translation for pbias
"ARM: dts: : add minimal l4 bus
layout with control module support" moved pbias_regulator dt node
from being a child node of ocp to be the child node of
'syscon'. Since 'syscon' doesn't have the 'ranges' property,
address translation fails while trying to convert the address
to resource. Fix it here by populating 'ranges' property in
syscon dt node.
Fixes: 72b10ac00eb1 ("ARM: dts: omap24xx: add minimal l4 bus
layout with control module support")
Fixes: 7415b0b4c645 ("ARM: dts: omap4: add minimal l4 bus layout
with control module support")
Fixes: ed8509edddeb ("ARM: dts: omap5: add minimal l4 bus
layout with control module support")
Fixes: d919501feffa ("ARM: dts: dra7: add minimal l4 bus
layout with control module support")
Signed-off-by: Kishon Vijay Abraham I
---
Changes from v1:
*) squashed [1] to a single patch
[1] -> https://lkml.org/lkml/2015/9/3/166
arch/arm/boot/dts/dra7.dtsi |1 +
arch/arm/boot/dts/omap2430.dtsi |1 +
arch/arm/boot/dts/omap4.dtsi|1 +
arch/arm/boot/dts/omap5.dtsi|1 +
4 files changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index d6bc6db..5896896 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -120,6 +120,7 @@
reg = <0x0 0x1400>;
#address-cells = <1>;
#size-cells = <1>;
+ ranges = <0 0x0 0x1400>;
pbias_regulator: pbias_regulator {
compatible = "ti,pbias-dra7",
"ti,pbias-omap";
diff --git a/arch/arm/boot/dts/omap2430.dtsi b/arch/arm/boot/dts/omap2430.dtsi
index 3961a6f..798dda0 100644
--- a/arch/arm/boot/dts/omap2430.dtsi
+++ b/arch/arm/boot/dts/omap2430.dtsi
@@ -56,6 +56,7 @@
reg = <0x270 0x240>;
#address-cells = <1>;
#size-cells = <1>;
+ ranges = <0 0x270 0x240>;
scm_clocks: clocks {
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 5aad7f3..5a206c1 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -196,6 +196,7 @@
reg = <0x5a0 0x170>;
#address-cells = <1>;
#size-cells = <1>;
+ ranges = <0 0x5a0 0x170>;
pbias_regulator: pbias_regulator {
compatible = "ti,pbias-omap4",
"ti,pbias-omap";
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 76ef595..dd7a0e8 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -185,6 +185,7 @@
reg = <0x5a0 0xec>;
#address-cells = <1>;
#size-cells = <1>;
+ ranges = <0 0x5a0 0xec>;
pbias_regulator: pbias_regulator {
compatible = "ti,pbias-omap5",
"ti,pbias-omap";
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] ARM: dts: : use "ti,pbias-" compatible string for pbias
Use platform specific compatible strings instead of the common
"ti,pbias-omap" compatible string.
Signed-off-by: Kishon Vijay Abraham I
Acked-by: Tony Lindgren
---
arch/arm/boot/dts/dra7.dtsi |2 +-
arch/arm/boot/dts/omap2430.dtsi |2 +-
arch/arm/boot/dts/omap3.dtsi|2 +-
arch/arm/boot/dts/omap4.dtsi|2 +-
arch/arm/boot/dts/omap5.dtsi|2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 1e29ccf..d6bc6db 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -122,7 +122,7 @@
#size-cells = <1>;
pbias_regulator: pbias_regulator {
- compatible = "ti,pbias-omap";
+ compatible = "ti,pbias-dra7",
"ti,pbias-omap";
reg = <0xe00 0x4>;
syscon = <&scm_conf>;
pbias_mmc_reg: pbias_mmc_omap5 {
diff --git a/arch/arm/boot/dts/omap2430.dtsi b/arch/arm/boot/dts/omap2430.dtsi
index 2390f38..3961a6f 100644
--- a/arch/arm/boot/dts/omap2430.dtsi
+++ b/arch/arm/boot/dts/omap2430.dtsi
@@ -63,7 +63,7 @@
};
pbias_regulator: pbias_regulator {
- compatible = "ti,pbias-omap";
+ compatible = "ti,pbias-omap2",
"ti,pbias-omap";
reg = <0x230 0x4>;
syscon = <&scm_conf>;
pbias_mmc_reg:
pbias_mmc_omap2430 {
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 69a40cf..9af9ae1 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -203,7 +203,7 @@
};
pbias_regulator: pbias_regulator {
- compatible = "ti,pbias-omap";
+ compatible = "ti,pbias-omap3", "ti,pbias-omap";
reg = <0x2b0 0x4>;
syscon = <&scm_conf>;
pbias_mmc_reg: pbias_mmc_omap2430 {
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index abc4473..5aad7f3 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -198,7 +198,7 @@
#size-cells = <1>;
pbias_regulator: pbias_regulator {
- compatible = "ti,pbias-omap";
+ compatible = "ti,pbias-omap4",
"ti,pbias-omap";
reg = <0x60 0x4>;
syscon =
<&omap4_padconf_global>;
pbias_mmc_reg: pbias_mmc_omap4 {
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index b1a1263..76ef595 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -187,7 +187,7 @@
#size-cells = <1>;
pbias_regulator: pbias_regulator {
- compatible = "ti,pbias-omap";
+ compatible = "ti,pbias-omap5",
"ti,pbias-omap";
reg = <0x60 0x4>;
syscon =
<&omap5_padconf_global>;
pbias_mmc_reg: pbias_mmc_omap5 {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] regulator: pbias: program pbias register offset in pbias driver
Add separate compatible strings for every platform and populate the
pbias register offset in the driver data.
This helps avoid depending on the dt for pbias register offset.
Also update the dt binding documentation with the new compatible
strings.
Suggested-by: Tony Lindgren
Signed-off-by: Kishon Vijay Abraham I
Acked-by: Tony Lindgren
---
.../bindings/regulator/pbias-regulator.txt |7 ++-
drivers/regulator/pbias-regulator.c| 56 +---
2 files changed, 56 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/pbias-regulator.txt
b/Documentation/devicetree/bindings/regulator/pbias-regulator.txt
index 32aa26f..acbcb45 100644
--- a/Documentation/devicetree/bindings/regulator/pbias-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/pbias-regulator.txt
@@ -2,7 +2,12 @@ PBIAS internal regulator for SD card dual voltage i/o pads on
OMAP SoCs.
Required properties:
- compatible:
- - "ti,pbias-omap" for OMAP2, OMAP3, OMAP4, OMAP5, DRA7.
+ - should be "ti,pbias-dra7" for DRA7
+ - should be "ti,pbias-omap2" for OMAP2
+ - should be "ti,pbias-omap3" for OMAP3
+ - should be "ti,pbias-omap4" for OMAP4
+ - should be "ti,pbias-omap5" for OMAP5
+ - "ti,pbias-omap" is deprecated
- reg: pbias register offset from syscon base and size of pbias register.
- syscon : phandle of the system control module
- regulator-name : should be
diff --git a/drivers/regulator/pbias-regulator.c
b/drivers/regulator/pbias-regulator.c
index bd2b75c..c21cedb 100644
--- a/drivers/regulator/pbias-regulator.c
+++ b/drivers/regulator/pbias-regulator.c
@@ -44,6 +44,10 @@ struct pbias_regulator_data {
int voltage;
};
+struct pbias_of_data {
+ unsigned int offset;
+};
+
static const unsigned int pbias_volt_table[] = {
180,
300
@@ -98,8 +102,35 @@ static struct of_regulator_match pbias_matches[] = {
};
#define PBIAS_NUM_REGS ARRAY_SIZE(pbias_matches)
+/* Offset from SCM general area (and syscon) base */
+
+static const struct pbias_of_data pbias_of_data_omap2 = {
+ .offset = 0x230,
+};
+
+static const struct pbias_of_data pbias_of_data_omap3 = {
+ .offset = 0x2b0,
+};
+
+static const struct pbias_of_data pbias_of_data_omap4 = {
+ .offset = 0x60,
+};
+
+static const struct pbias_of_data pbias_of_data_omap5 = {
+ .offset = 0x60,
+};
+
+static const struct pbias_of_data pbias_of_data_dra7 = {
+ .offset = 0xe00,
+};
+
static const struct of_device_id pbias_of_match[] = {
{ .compatible = "ti,pbias-omap", },
+ { .compatible = "ti,pbias-omap2", .data = &pbias_of_data_omap2, },
+ { .compatible = "ti,pbias-omap3", .data = &pbias_of_data_omap3, },
+ { .compatible = "ti,pbias-omap4", .data = &pbias_of_data_omap4, },
+ { .compatible = "ti,pbias-omap5", .data = &pbias_of_data_omap5, },
+ { .compatible = "ti,pbias-dra7", .data = &pbias_of_data_dra7, },
{},
};
MODULE_DEVICE_TABLE(of, pbias_of_match);
@@ -114,6 +145,9 @@ static int pbias_regulator_probe(struct platform_device
*pdev)
const struct pbias_reg_info *info;
int ret = 0;
int count, idx, data_idx = 0;
+ const struct of_device_id *match;
+ const struct pbias_of_data *data;
+ unsigned int offset;
count = of_regulator_match(&pdev->dev, np, pbias_matches,
PBIAS_NUM_REGS);
@@ -129,6 +163,20 @@ static int pbias_regulator_probe(struct platform_device
*pdev)
if (IS_ERR(syscon))
return PTR_ERR(syscon);
+ match = of_match_device(of_match_ptr(pbias_of_match), &pdev->dev);
+ if (match && match->data) {
+ data = match->data;
+ offset = data->offset;
+ } else {
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
+ offset = res->start;
+ dev_WARN(&pdev->dev,
+"using legacy dt data for pbias offset\n");
+ }
+
cfg.regmap = syscon;
cfg.dev = &pdev->dev;
@@ -141,10 +189,6 @@ static int pbias_regulator_probe(struct platform_device
*pdev)
if (!info)
return -ENODEV;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -EINVAL;
-
drvdata[data_idx].syscon = syscon;
drvdata[data_idx].info = info;
drvdata[data_idx].desc.name = info->name;
@@ -154,9 +198,9 @@ static int pbias_regulator_probe(struct platform_device
*pdev)
drvdata[data_idx].desc.volt_table = pbias_volt_table;
drvdata[data_idx].desc.n_voltages = 2;
drvdata[data_idx].desc.enable_time = info->enable_time;
- drvdata[data_idx].desc.vsel_reg = res->start;
+ drvdata[data_idx].desc.v
[PATCH v3 0/2] regulator: Fix pbias regulator enable
vsel_reg and enable_reg of the pbias regulator descriptor should actually have the offset from syscon. However after "ARM: dts: : add minimal l4 bus layout with control module support" vsel_reg and enable_reg started to have the absolute address because of address translation that happens due to pbias node made as the child node of syscon. This breaks the pbias regulator enable. This series adds the 'offset' to be populated in vsel_reg and enable_reg in the pbias driver itself. Changes from v2: *) Squashed all the dt patches into a single patch Changes from v1: *) Fixed Tony's review comments on adding a 'comment' for adding offset in the driver and adding a warning for using platform_get_resource. *) Added Tony's Acked-by. Tested these patches against mmc -next in omap4 panda, omap3 beagle xm, dra72 and omap5 uevm Kishon Vijay Abraham I (2): regulator: pbias: program pbias register offset in pbias driver ARM: dts: : use "ti,pbias-" compatible string for pbias .../bindings/regulator/pbias-regulator.txt |7 ++- arch/arm/boot/dts/dra7.dtsi|2 +- arch/arm/boot/dts/omap2430.dtsi|2 +- arch/arm/boot/dts/omap3.dtsi |2 +- arch/arm/boot/dts/omap4.dtsi |2 +- arch/arm/boot/dts/omap5.dtsi |2 +- drivers/regulator/pbias-regulator.c| 56 +--- 7 files changed, 61 insertions(+), 12 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: ux500: fix typo in regulator names
On 5 August 2015 at 14:22, Tomeu Vizoso wrote:
> Apparently an extra _reg suffix was appended to the end of the names of
> regulators ab8500_ext2_reg and ab8500_ext3_reg.
>
> Signed-off-by: Tomeu Vizoso
You should probably include Linus Walleij as he is the maintainer of ux500.
Acked-by: Ulf Hansson
> ---
> arch/arm/boot/dts/ste-snowball.dts | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/ste-snowball.dts
> b/arch/arm/boot/dts/ste-snowball.dts
> index 32a5ccb14e7e..07c0e346942c 100644
> --- a/arch/arm/boot/dts/ste-snowball.dts
> +++ b/arch/arm/boot/dts/ste-snowball.dts
> @@ -360,11 +360,11 @@
> regulator-name =
> "ab8500-ext-supply1";
> };
>
> - ab8500_ext2_reg_reg: ab8500_ext2 {
> + ab8500_ext2_reg: ab8500_ext2 {
> regulator-name =
> "ab8500-ext-supply2";
> };
>
> - ab8500_ext3_reg_reg: ab8500_ext3 {
> + ab8500_ext3_reg: ab8500_ext3 {
> regulator-name =
> "ab8500-ext-supply3";
> };
> };
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] omap: Fix broken address translation for pbias
Hi Tony, On Thursday 03 September 2015 08:40 PM, Tony Lindgren wrote: > * Kishon Vijay Abraham I [150903 04:31]: >> pbias device stopped having memory resource after >> "ARM: dts: : add minimal l4 bus layout with control module >> support" got merged. This results in platform_get_resource returning >> -EINVAL in pbias driver. This is because address translation fails >> while trying to convert the address to resource which happens >> during the device creation process during boot up. >> >> Even though after [1], reg property in pbias dt node is un-used, >> it might be an issue if we plan to use it at a later point of time. >> >> Patch series is created on top of [1]. >> >> [1] -> https://lkml.org/lkml/2015/9/3/51 >> >> Verified the address in omap4 panda, omap5 uevm and dra72 boards. > > Your other series is obviously needed for v4.3.. But seems these > are also needed for stable? If so, which versions, v4.2+? Both the series are needed from v4.1+. However since the 'reg' property is not going to be used, I'm not sure if this series has to be marked for stable. > > Can you please just do a single patch with multiple Fixes tags > as they fix regression for multiple commits? Doing tons of one > liner patches for the same thing just creates commit log noise.. Sure. > > Remember to not Cc stable when posting, I'll add that tag when > committing if needed. Sure. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] i2c: davinci: Optimize clock generation on Keystone SoC
On 08/21/2015 12:46 PM, Alexander Sverdlin wrote:
According to "KeyStone Architecture Inter-IC Control Bus User Guide", fixed
additive part of frequency divisors (referred as "d" in the code and datasheet)
always equals to 6, independent of module clock prescaler.
module clock frequency
master clock frequency = --
(ICCL + 6) + (ICCH + 6)
It was not the case with original Davinci IP. Introduce new compatible property
"ti,keystone-i2c", which triggers special handling in the driver.
Without this change Keystone-based systems (having 204.8MHz input clock) choose
prescaler 29 (PSC=28). Using d=5 in this case leads to bus bitrate ~353kHz
instead of requested 400kHz. After correction, assuming d=6 bus rate is ~392kHz.
This gives ~11% transfer rate increase.
Thanks Alexander.
Reviewed-by: Grygorii Strashko
Signed-off-by: Alexander Sverdlin
Tested-by: Hemanth Guruva Reddy
Tested-by: Lukasz Gemborowski
---
.../devicetree/bindings/i2c/i2c-davinci.txt|6 +++---
drivers/i2c/busses/i2c-davinci.c |8
2 files changed, 11 insertions(+), 3 deletions(-)
Changes in v2:
- Introducing new "compatible" property "ti,keystone-i2c" instead of guessing
silicon revision from ID registers
diff --git a/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
b/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
index a4e1cbc..5b123e0 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-davinci.txt
@@ -1,10 +1,10 @@
-* Texas Instruments Davinci I2C
+* Texas Instruments Davinci/Keystone I2C
This file provides information, what the device node for the
-davinci i2c interface contain.
+davinci/keystone i2c interface contains.
Required properties:
-- compatible: "ti,davinci-i2c";
+- compatible: "ti,davinci-i2c" or "ti,keystone-i2c";
- reg : Offset and length of the register set for the device
Recommended properties :
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 3fbb9a0..c5628a4 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -181,6 +181,7 @@ static void i2c_davinci_calc_clk_dividers(struct
davinci_i2c_dev *dev)
u32 clkh;
u32 clkl;
u32 input_clock = clk_get_rate(dev->clk);
+ struct device_node *of_node = dev->dev->of_node;
/* NOTE: I2C Clock divider programming info
* As per I2C specs the following formulas provide prescaler
@@ -196,6 +197,9 @@ static void i2c_davinci_calc_clk_dividers(struct
davinci_i2c_dev *dev)
* where if PSC == 0, d = 7,
* if PSC == 1, d = 6
* if PSC > 1 , d = 5
+*
+* Note:
+* d is always 6 on Keystone I2C controller
*/
/* get minimum of 7 MHz clock, but max of 12 MHz */
@@ -204,6 +208,9 @@ static void i2c_davinci_calc_clk_dividers(struct
davinci_i2c_dev *dev)
psc++; /* better to run under spec than over */
d = (psc >= 2) ? 5 : 7 - psc;
+ if (of_node && of_device_is_compatible(of_node, "ti,keystone-i2c"))
+ d = 6;
+
clk = ((input_clock / (psc + 1)) / (pdata->bus_freq * 1000));
/* Avoid driving the bus too fast because of rounding errors above */
if (input_clock / (psc + 1) / clk > pdata->bus_freq * 1000)
@@ -726,6 +733,7 @@ static struct i2c_algorithm i2c_davinci_algo = {
static const struct of_device_id davinci_i2c_of_match[] = {
{.compatible = "ti,davinci-i2c", },
+ {.compatible = "ti,keystone-i2c", },
{},
};
MODULE_DEVICE_TABLE(of, davinci_i2c_of_match);
--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: st_fdma: Firmware filename in DT?
On Fri, 04 Sep 2015, Peter Griffin wrote: > On Fri, 04 Sep 2015, Lee Jones wrote: > > [...] > > > > > > If yes, is it worth having a generic binding? > > > > > > If we decide it is a good idea, then yes. It doesn't look that hard to > > > arrive at something common. > > > > > > Strictly speaking, the firmware name is just an agreed upon name > > > between the kernel and userspace. So why tie that into DT? Would other > > > OS's use it or want something different? What if we started including > > > paths in the names like /? Now we are imposing a > > > directory structure on the OS filesystem. > > > > In Linux we have a standard place for firmwares, so the path should > > not be required. I certainly agree that DT is no place for absolute > > paths or OS'isums. > > > > [...] > > > > > > Presumably the alternative would be to add a whole bunch of compatibles > > > > in the driver for each SoC, where the only difference from a > > > > functional point of view would be to help build the correct string for > > > > the firmware filename. However I'm also then wondering what the best way > > > > would be to find out the instance name of the IP. > > > > > > If the name/path is Linux specific, then that is probably what we should > > > do. > > > > > > You could perhaps make a policy that firmware files be named by > > > compatible string. So rather than translating from matching compatible > > > to an arbitrary file name, you enforce file name is ", > > block>.fw" or something. I know we don't have policy in the kernel, > > > but we already have it with hardcoded file names and search paths. > > > > Absolutely not. Firmwares have no direct link to DT or platforms that > > run DT specifically. They are carried by most platforms these days. > > Insisting on firmwares using a DT compatible string format is way off. > > > > If we flip it the other way round, some subsystems derive the firmware > > name from the 'node name'. For instance, our zeroth General Purpose > > Co-Processor RemoteProc driver has a corresponding node called > > 'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw' > > suffix and et voilà, we load file: > > > > lib/firmware/rproc-st231-gp0-fw > > IMO deriving from the node name seems fragile. Also imposing a linux'ism > "rproc" prefix on the firmware name doesn't seem correct as the firmwares > can be shared across OS's. Although this is how remoteproc subsys core > is currently working. It seems a generic DT firmware binding would actually > be most useful for the remoteproc subsystem. The "rproc-%s-fw", where %s == driver name, is only a fall-back. The RProc driver is welcome to supply a different firmware name if it desires. This is where I think a generic 'firmware' property would be of use. > I guess I should also comment about this on the ST remoteproc thread. > > I'm curious though as to how the ST remoteproc driver then loads the firmware, > as that name doesn't look like a video or audio firmware filename that I've > seen shipped from ST. IIRC Usually it is named something like > > vid_firmware-stih407.elf or audio_firmware-bd-stih407.elf This driver came direct from ST. I'm using their conventions as a default, although I would be happy to conduct some investigation into how this works for releases and suggest a better interface. The only reason I mentioned it here was because this is how others are using other SS which are similar in some way. Same for the Firmware SS I guess. In my unbiased PoV, I'd much prefer the name was derived from a predefined properly. > IMO we should be treating the firmware as a blackbox, and for me that also > includes the filename it is shipped with. Linux drivers making up their own > firmware names based on the name of their subsystem doesn't seem like a > good way forward to me. I guess the driver author was just using the fall-back convention for testing. I'm pretty sure RemoteProc isn't being shipped in products yet. > Presumably to test this driver you have to rename the firmwares > locally on your machine? The firmware that was provided to conduct testing was already named as expected by the driver, but yes, if we were to test the firmwares you mentioned they would either have to be renamed, or those names would have to be provided to the RemoteProc SS in a different way. > > > > We could do it by parsing the node name e.g. fdma0-audio, or by adding > > > > a "instance" DT property to the node? > > > > > > Generally we try to avoid caring about node names. Having some index > > > or numbering also comes up which we also try to avoid. Generally, if > > > you care about which instance you use for something, then there is > > > some property you care about and should add. > > > > Right, the alternative is a property like the ones already used. > > However, as these are becoming more prevalent I suggested > > standardising the property to avoid all these vendor specific firmware > > properties clutteri
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed
On Thu, Sep 03, 2015 at 11:04:40AM +0200, Thierry Reding wrote: > Conversely, if the panel isn't capable of generating an HPD signal, then > I don't think it would be appropriate to make it a DT property. It would > be better to hard-code it in the driver, lest someone forget to set the > property in DT and get stuck with a device that isn't operational. There is another way to deal with this: DRM supports the idea of connector forcing - where you can force the connector to think that it's connected or disconnected. One of the problems is that not many ARM DRM drivers implement it - maybe it should be a requirement for code to be accepted? :) -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed
On Thu, Sep 03, 2015 at 04:55:59PM -0500, Rob Herring wrote: > On Thu, Sep 3, 2015 at 3:47 AM, Thierry Reding wrote: > > On Wed, Sep 02, 2015 at 03:17:57PM -0500, Rob Herring wrote: > > [...] > >> Are there any eDP panels which don't have EDID and need panel details in > >> DT? > > > > Most panels need information other than EDID. They typically have some > > requirements regarding the power up sequence that aren't to be found > > anywhere in EDID or detectable by some other mechanism. A decision was > > therefore made a long time ago to require panels to be listed in DT with > > a specific compatible string. That way all of these details can be > > stashed away in drivers that know how to deal with these kinds of > > details. > > I guess I was being hopeful that eDP was improving that situation. Unfortunately not. eDP helps with a number of things (DPCD and link training take care of a lot of the issues), but it doesn't provide a solution for everything. To my knowledge in most cases the power up sequence isn't strictly necessary to get the panel to operate. But there have been instances where this is absolutely required if you want to avoid visual artifacts as you set a mode. A lot of panels that I've come across require receiving a couple of frames before they guarantee that something will actually be displayed on the screen. So if your code is too quickly enabling backlight, and your backlight doesn't happen to have just the right amount of ramp-up time you might end up seeing garbage on the screen for a couple of frames. It would be great if somebody came up with, say, an EDID extension to describe these requirements, but I'm not sure if even that would give us panels that could be driven with a generic driver. Often the power supplies or reset/enable signals are very different across panels. So describing it all generically would become messy rather quickly. Thierry signature.asc Description: PGP signature
Re: thermal: mediatek: Add cpu power cooling model
Sorry, forgot to add Rafael and Viresh as reviewer. On Fri, 2015-09-04 at 17:01 +0800, Dawei Chien wrote: > Use Intelligent Power Allocation (IPA) technical to add > static/dynamic power model for binding CPU thermal zone. > The power allocator governor allocates power budget to control > CPU temperature. > > Dawei.Chien (2): > thermal: mediatek: Add cpu power cooling model. > arm64: dts: mt8173: Add thermal zone node for mt8173. > > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 44 ++ > drivers/cpufreq/mt8173-cpufreq.c | 97 > ++ > 2 files changed, 130 insertions(+), 11 deletions(-) > > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/6] ARM: bcm2835: add DT for the bcm2835 auxiliar devices
From: Martin Sperl
Add device tree definitions for auxiliar bcm2835 devices:
* spi1
* spi2
* uart1
This also include a device to get used by the relevant
device-drivers (via a shared register) to enable/disable
the HW-block.
The aux-interrupt-register (0x7e21500) is intentionally left
out of scope, so that in the future it is still possible to
implement a separate interrupt-driver - if deemed necessary.
The spi (and also uart) drivers are able to use shared interrupts,
so there is no real need for an interrupt driver.
Signed-off-by: Martin Sperl
---
arch/arm/boot/dts/bcm2835.dtsi | 37 +
1 file changed, 37 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index 301c73f..4e6fc61 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -158,6 +158,43 @@
arm-pmu {
compatible = "arm,arm1176-pmu";
};
+
+ aux_enable: aux_enable@0x7e215004 {
+ compatible = "brcm,bcm2835-aux";
+ reg = <0x7e215004 0x04>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ uart1: uart@7e215040 {
+ compatible = "brcm,bcm2835-aux-uart";
+ reg = <0x7e215040 0x40>;
+ interrupts = <1 29>;
+ brcm,aux-enable = <&aux_enable 1>;
+ status = "disabled";
+ };
+
+ spi1: spi@7e215080 {
+ compatible = "brcm,bcm2835-aux-spi";
+ reg = <0x7e215080 0x40>;
+ brcm,aux-enable = <&aux_enable 2>;
+ interrupts = <1 29>;
+ clocks = <&clk_spi>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
+ spi2: spi@7e2150c0 {
+ compatible = "brcm,bcm2835-aux-spi";
+ reg = <0x7e2150c0 0x40>;
+ brcm,aux-enable = <&aux_enable 4>;
+ interrupts = <1 29>;
+ clocks = <&clk_spi>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
};
clocks {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] soc: bcm2835: auxiliar devices enable infrastructure
From: Martin Sperl
The bcm2835 SOC contains 3 auxiliar devices (spi1, spi2 and uart1)
that all are enabled via a shared register.
To serialize access to this shared register this soc-driver
is created that implements:
bcm2835aux_enable(struct device *dev, const char *property);
bcm2835aux_disable(struct device *dev, const char *property);
Which will read the property from the device tree of the device
and enable/disable that specific device as per device tree.
First use of this api will be spi-bcm2835aux.
This driver does not implement an interrupt-controller,
so only access to the auxiliar-device-enable register is required.
Signed-off-by: Martin Sperl
---
drivers/soc/Kconfig |1 +
drivers/soc/Makefile|1 +
drivers/soc/bcm/Kconfig | 11 +++
drivers/soc/bcm/Makefile|1 +
drivers/soc/bcm/bcm2835-aux.c | 154 +++
include/linux/soc/bcm/bcm2835-aux.h | 23 ++
6 files changed, 191 insertions(+)
create mode 100644 drivers/soc/bcm/Kconfig
create mode 100644 drivers/soc/bcm/Makefile
create mode 100644 drivers/soc/bcm/bcm2835-aux.c
create mode 100644 include/linux/soc/bcm/bcm2835-aux.h
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 96ddecb..5506e39 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -1,5 +1,6 @@
menu "SOC (System On Chip) specific Drivers"
+source "drivers/soc/bcm/Kconfig"
source "drivers/soc/mediatek/Kconfig"
source "drivers/soc/qcom/Kconfig"
source "drivers/soc/sunxi/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 7dc7c0d..c5744e1 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -2,6 +2,7 @@
# Makefile for the Linux Kernel SOC specific device drivers.
#
+obj-$(CONFIG_ARCH_BCM) += bcm/
obj-$(CONFIG_ARCH_MEDIATEK)+= mediatek/
obj-$(CONFIG_ARCH_QCOM)+= qcom/
obj-$(CONFIG_ARCH_SUNXI) += sunxi/
diff --git a/drivers/soc/bcm/Kconfig b/drivers/soc/bcm/Kconfig
new file mode 100644
index 000..e57e98f
--- /dev/null
+++ b/drivers/soc/bcm/Kconfig
@@ -0,0 +1,11 @@
+#
+# Broadcom SoC drivers
+#
+config SOC_BCM2835_AUX
+ tristate "Broadcom BCM2835 aux"
+ depends on OF
+ depends on ARCH_BCM2835 || COMPILE_TEST
+
+ help
+ Support to enable/disable the BCM2835 auxiliar
+ devices spi1, spi2, uart1
diff --git a/drivers/soc/bcm/Makefile b/drivers/soc/bcm/Makefile
new file mode 100644
index 000..370a872
--- /dev/null
+++ b/drivers/soc/bcm/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SOC_BCM2835_AUX) += bcm2835-aux.o
diff --git a/drivers/soc/bcm/bcm2835-aux.c b/drivers/soc/bcm/bcm2835-aux.c
new file mode 100644
index 000..5980d67
--- /dev/null
+++ b/drivers/soc/bcm/bcm2835-aux.c
@@ -0,0 +1,154 @@
+/*
+ * bcm2835-aux
+ *
+ * Copyright (C) 2015 Martin Sperl
+ *
+ * Author: Martin Sperl
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static DEFINE_SPINLOCK(bcm2835aux_lock);
+
+static struct platform_driver bcm2835aux_driver;
+
+static int bcm2835aux_dev_match(struct device *dev, void *data)
+{
+ struct device_node *dn = data;
+
+ return (dev->of_node == dn) ? 1 : 0;
+}
+
+static void *bcm2835aux_find_base(struct device *dev, const char *property)
+{
+ struct device *found = NULL;
+ struct device_node *np;
+
+ /* get the phandle of the device */
+ np = of_parse_phandle(dev->of_node, property, 0);
+ if (!np) {
+ dev_err(dev, "missing property %s\n", property);
+ return ERR_PTR(-ENODEV);
+ }
+
+ /* now find the device it points to */
+ found = driver_find_device(&bcm2835aux_driver.driver, NULL,
+ np, bcm2835aux_dev_match);
+ if (!found) {
+ dev_err(dev, "device for phandle of %s not found\n",
+ property);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+
+ /* now we got the device, so return the pointer */
+ return dev_get_drvdata(found);
+}
+
+static u32 bcm2835aux_find_mask(struct device *dev, const char *property)
+{
+ int err;
+ u32 mask;
+
+ err = of_property_read_u32_index(dev->of_node, property, 1, &mask);
+ if (err) {
+ dev_err(dev, "missing argument to %s: %d\n",
+ property, err);
+ return 0;
+ }
+
+ return mask;
+}
+
+static int bcm2835aux_bitset(struct device *dev, const char *property,
+bool set)
+{
+ u32 v, mask;
+ unsigned long flags;
+ void __iomem *base;
+
+ /* find the device */
+ base = bcm2
[PATCH v5 3/6] dt/bindings: bcm2835: add binding documentation for bcm2835-aux
From: Martin Sperl
add binding documentation
Signed-off-by: Martin Sperl
---
.../bindings/soc/bcm/brcm,bcm2835-aux.txt | 27
1 file changed, 27 insertions(+)
create mode 100644
Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-aux.txt
diff --git a/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-aux.txt
b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-aux.txt
new file mode 100644
index 000..8b79cf3
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-aux.txt
@@ -0,0 +1,27 @@
+Broadcom BCM2835 auxiliar device enable register
+
+the BCM2835 contains 3 auxiliar devices (spi1/spi2/uart1)
+which need to get enabled via a shared register.
+
+Required properties:
+- compatible: Should be "brcm,bcm2835-aux".
+- reg: Should contain register location and length for the
+ enable register
+
+Example:
+
+aux_enable: aux_enable@0x7e215004 {
+ compatible = "bcrm,bcm2835-aux";
+ reg = <0x7e215004 0x04>;
+};
+
+Typically used in the respective auxiliar device descriptions
+like this:
+
+uart1: uart@7e215040 {
+ ...
+ brcm,aux-enable = <&aux_enable 1>;
+ ...
+};
+
+The name of the property can be device/driver specific.
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 5/6] ARM: bcm2835: enable building of spi-bcm2835aux driver in default config
From: Martin Sperl Added spi-bcm2835aux to default config Signed-off-by: Martin Sperl --- arch/arm/configs/bcm2835_defconfig |1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig index 31cb073..a205c8b 100644 --- a/arch/arm/configs/bcm2835_defconfig +++ b/arch/arm/configs/bcm2835_defconfig @@ -75,6 +75,7 @@ CONFIG_I2C_CHARDEV=y CONFIG_I2C_BCM2835=y CONFIG_SPI=y CONFIG_SPI_BCM2835=y +CONFIG_SPI_BCM2835AUX=y CONFIG_GPIO_SYSFS=y # CONFIG_HWMON is not set CONFIG_FB=y -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 6/6] dt/bindings: bcm2835: Add binding documentation for auxiliar spi devices
From: Martin Sperl
add binding documentation
Signed-off-by: Martin Sperl
---
.../bindings/spi/brcm,bcm2835-aux-spi.txt | 47
1 file changed, 47 insertions(+)
create mode 100644
Documentation/devicetree/bindings/spi/brcm,bcm2835-aux-spi.txt
Changelog:
v4->v5: fixed wording for reg
removed reference to syscon
removed documentation on typical GPIO config
diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm2835-aux-spi.txt
b/Documentation/devicetree/bindings/spi/brcm,bcm2835-aux-spi.txt
new file mode 100644
index 000..3ade499
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/brcm,bcm2835-aux-spi.txt
@@ -0,0 +1,47 @@
+Broadcom BCM2835 auxiliar SPI1/2 controller
+
+The BCM2835 contains two forms of SPI master controller, one known simply as
+SPI0, and the other known as the "Universal SPI Master"; part of the
+auxiliary block. This binding applies to the SPI1/2 controller.
+
+Required properties:
+- compatible: Should be "brcm,bcm2835-aux-spi".
+- reg: Should contain register location and length for the spi block
+- interrupts: Should contain shared interrupt of the aux block
+- clocks: The clock feeding the SPI controller.
+- cs-gpios: the cs-gpios (native cs is NOT supported)
+ see also spi-bus.txt
+- bcrm,aux-enable: the bcrm,bcm2835-aux-enable config entry to handle
+ enabling/disabling of the spi1/spi2/uart1 HW block
+ second "argument" is the mask to apply to the
+ enable register
+
+Example:
+
+spi1@7e215080 {
+ compatible = "brcm,bcm2835-aux-spi";
+ reg = <0x7e215080 0x40>;
+ interrupts = <1 29>;
+ clocks = <&clk_spi>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ cs-gpios = <&gpio 18>, <&gpio 17>, <&gpio 16>;
+ bcrm,aux-enable = <&aux_enable 2>;
+};
+
+spi2@7e2150c0 {
+ compatible = "brcm,bcm2835-aux-spi";
+ reg = <0x7e2150c0 0x40>;
+ interrupts = <1 29>;
+ clocks = <&clk_spi>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ cs-gpios = <&gpio 43>, <&gpio 44>, <&gpio 45>;
+ bcrm,aux-enable = <&aux_enable 4>;
+};
+
+/* the necessary bcm2835 aux-enable referenced above */
+aux_enable: aux_enable@0x7e215004 {
+ compatible = "bcrm,bcm2835-aux";
+ reg = <0x7e215004 0x04>;
+};
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 4/6] spi: bcm2835: new driver implementing auxiliar spi1/spi2 on the bcm2835 soc
From: Martin Sperl Implements spi master driver for the 2 auxiliar spi devices supported by the bcm2835 SOC. The driver does not implement native chip-selects but uses framework provided aribtrary GPIO-chip-selects. Requires soc-bcm2835-aux enable api to enable/disable HW blocks. Signed-off-by: Martin Sperl --- drivers/spi/Kconfig | 12 + drivers/spi/Makefile |1 + drivers/spi/spi-bcm2835aux.c | 514 ++ 3 files changed, 527 insertions(+) create mode 100644 drivers/spi/spi-bcm2835aux.c Changelog: v4->v5: added error-handling and deferred probing support moved change to default-config to a separate patch fixed Kconfig to add the correct dependency diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 4887f31..25242cc 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -88,6 +88,18 @@ config SPI_BCM2835 is for the regular SPI controller. Slave mode operation is not also not supported. +config SPI_BCM2835AUX + tristate "BCM2835 SPI auxiliar controller" + depends on ARCH_BCM2835 || COMPILE_TEST + depends on GPIOLIB + select SOC_BCM2835_AUX + help + This selects a driver for the Broadcom BCM2835 SPI aux master. + + The BCM2835 contains two types of SPI master controller; the + "universal SPI master", and the regular SPI controller. + This driver is for the universal/auxiliar SPI controller. + config SPI_BFIN5XX tristate "SPI controller driver for ADI Blackfin5xx" depends on BLACKFIN && !BF60x diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 6a7f6f9..31fb7fb 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_SPI_ATMEL) += spi-atmel.o obj-$(CONFIG_SPI_ATH79)+= spi-ath79.o obj-$(CONFIG_SPI_AU1550) += spi-au1550.o obj-$(CONFIG_SPI_BCM2835) += spi-bcm2835.o +obj-$(CONFIG_SPI_BCM2835AUX) += spi-bcm2835aux.o obj-$(CONFIG_SPI_BCM53XX) += spi-bcm53xx.o obj-$(CONFIG_SPI_BCM63XX) += spi-bcm63xx.o obj-$(CONFIG_SPI_BCM63XX_HSSPI)+= spi-bcm63xx-hsspi.o diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c new file mode 100644 index 000..d968647 --- /dev/null +++ b/drivers/spi/spi-bcm2835aux.c @@ -0,0 +1,514 @@ +/* + * Driver for Broadcom BCM2835 SPI Controllers + * + * the driver does not rely on the native chipselects at all + * but only uses the gpio type chipselects + * + * Based on: spi-bcm2835.c + * + * Copyright (C) 2015 Martin Sperl + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* + * shared aux registers between spi1/spi2 and uart1 + * + * these defines could go to a separate module if needed + * so that it can also get used with the uart1 implementation + * when it materializes. + */ + +/* the AUX register offsets */ +#define BCM2835_AUX_IRQ0x00 +#define BCM2835_AUX_ENABLE 0x04 + +/* the AUX Bitfield identical for both register */ +#define BCM2835_AUX_BIT_UART 0x0001 +#define BCM2835_AUX_BIT_SPI1 0x0002 +#define BCM2835_AUX_BIT_SPI2 0x0004 + +/* + * spi register defines + * + * note there is garbage in the "official" documentation, + * so somedata taken from the file: + * brcm_usrlib/dag/vmcsx/vcinclude/bcm2708_chip/aux_io.h + * inside of: + * http://www.broadcom.com/docs/support/videocore/Brcm_Android_ICS_Graphics_Stack.tar.gz + */ + +/* SPI register offsets */ +#define BCM2835_AUX_SPI_CNTL0 0x00 +#define BCM2835_AUX_SPI_CNTL1 0x04 +#define BCM2835_AUX_SPI_STAT 0x08 +#define BCM2835_AUX_SPI_PEEK 0x0C +#define BCM2835_AUX_SPI_IO 0x20 +#define BCM2835_AUX_SPI_TXHOLD 0x30 + +/* Bitfields in CNTL0 */ +#define BCM2835_AUX_SPI_CNTL0_SPEED0xFFF0 +#define BCM2835_AUX_SPI_CNTL0_SPEED_MAX0xFFF +#define BCM2835_AUX_SPI_CNTL0_SPEED_SHIFT 20 +#define BCM2835_AUX_SPI_CNTL0_CS 0x000E +#define BCM2835_AUX_SPI_CNTL0_POSTINPUT0x0001 +#define BCM2835_AUX_SPI_CNTL0_VAR_CS 0x8000 +#define BCM2835_AUX_SPI_CNTL0_VAR_WIDTH0x4000 +#define BCM2835_AUX_SPI_CNTL0_DOUTHOLD 0x3000 +#define BCM2835_AUX_SPI_CNTL0_ENABLE 0x0800 +#define BCM283
[PATCH v5 0/6] bcm2835: auxiliar device support for spi
From: Martin Sperl The BCM2835 contains 3 auxiliar devices: * spi1 * spi2 * uart1 All of those 3 devices are enabled/disabled via a shared register, which is set by default to be disabled. Access to this register needs to get serialized. So after several iterations of discussions with the following ideas: * syscon - device tree should describe HW not drivers to use - 'compatiblity = "brcm,bcm2835-aux-enable", "syscon";' is not acceptable * regulator - it is not necessarily a regulator or a power gate that is implemented in HW, so it is not valid to use this framework The recommendation was made to create a new minimal API in soc just for access to this shared enable/disable register. This patch-series implements: * the bcm2835-auxiliar device enable/disable api in soc. * the bcm2835-auxiliar spi device driver The uart1 device driver (ns16550 based) is not implemented so far but would be using the same API. Both spi and uart drivers can run with shared interrupts, so there is no need for an interrupt-controller to get implemented. Martin Sperl (6): soc: bcm2835: auxiliar devices enable infrastructure ARM: bcm2835: add DT for the bcm2835 auxiliar devices dt/bindings: bcm2835: add binding documentation for bcm2835-aux spi: bcm2835: new driver implementing auxiliar spi1/spi2 on the bcm2835 soc ARM: bcm2835: enable building of spi-bcm2835aux driver in default config dt/bindings: bcm2835: Add binding documentation for auxiliar spi devices .../bindings/soc/bcm/brcm,bcm2835-aux.txt | 27 + .../bindings/spi/brcm,bcm2835-aux-spi.txt | 47 ++ arch/arm/boot/dts/bcm2835.dtsi | 37 ++ arch/arm/configs/bcm2835_defconfig |1 + drivers/soc/Kconfig|1 + drivers/soc/Makefile |1 + drivers/soc/bcm/Kconfig| 11 + drivers/soc/bcm/Makefile |1 + drivers/soc/bcm/bcm2835-aux.c | 154 ++ drivers/spi/Kconfig| 12 + drivers/spi/Makefile |1 + drivers/spi/spi-bcm2835aux.c | 514 include/linux/soc/bcm/bcm2835-aux.h| 23 + 13 files changed, 830 insertions(+) create mode 100644 Documentation/devicetree/bindings/soc/bcm/brcm,bcm2835-aux.txt create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm2835-aux-spi.txt create mode 100644 drivers/soc/bcm/Kconfig create mode 100644 drivers/soc/bcm/Makefile create mode 100644 drivers/soc/bcm/bcm2835-aux.c create mode 100644 drivers/spi/spi-bcm2835aux.c create mode 100644 include/linux/soc/bcm/bcm2835-aux.h -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] gpio-pca953x driver and PCA9536 chip
W dniu 2015-08-31 o 16:26, Janusz Użycki pisze:
Hi,
The driver sets register #02 (inversion) to 0x01 instead of 0x00 on my
board.
Reason: device_pca953x_init() does not initialize val buffer because
NBANK() returns
for PCA9536 ngpio=4 value 0 instead of 1.
My proposed code-patch for 3.14.17 is below.
Because I don't work on stable/next I can't deliver complete/ready patch.
Please for comments.
best regards
Janusz
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -75,7 +75,7 @@ MODULE_DEVICE_TABLE(i2c, pca953x_id);
#define MAX_BANK 5
#define BANK_SZ 8
-#define NBANK(chip) (chip->gpio_chip.ngpio / BANK_SZ)
+#define NBANK(chip) DIV_ROUND_UP(chip->gpio_chip.ngpio, BANK_SZ)
struct pca953x_chip {
unsigned gpio_start;
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: st_fdma: Firmware filename in DT?
Hi Lee, On Fri, 04 Sep 2015, Lee Jones wrote: > [...] > > > > If yes, is it worth having a generic binding? > > > > If we decide it is a good idea, then yes. It doesn't look that hard to > > arrive at something common. > > > > Strictly speaking, the firmware name is just an agreed upon name > > between the kernel and userspace. So why tie that into DT? Would other > > OS's use it or want something different? What if we started including > > paths in the names like /? Now we are imposing a > > directory structure on the OS filesystem. > > In Linux we have a standard place for firmwares, so the path should > not be required. I certainly agree that DT is no place for absolute > paths or OS'isums. > > [...] > > > > Presumably the alternative would be to add a whole bunch of compatibles > > > in the driver for each SoC, where the only difference from a > > > functional point of view would be to help build the correct string for > > > the firmware filename. However I'm also then wondering what the best way > > > would be to find out the instance name of the IP. > > > > If the name/path is Linux specific, then that is probably what we should do. > > > > You could perhaps make a policy that firmware files be named by > > compatible string. So rather than translating from matching compatible > > to an arbitrary file name, you enforce file name is ", > block>.fw" or something. I know we don't have policy in the kernel, > > but we already have it with hardcoded file names and search paths. > > Absolutely not. Firmwares have no direct link to DT or platforms that > run DT specifically. They are carried by most platforms these days. > Insisting on firmwares using a DT compatible string format is way off. > > If we flip it the other way round, some subsystems derive the firmware > name from the 'node name'. For instance, our zeroth General Purpose > Co-Processor RemoteProc driver has a corresponding node called > 'st231-gp0@4000'. RemoteProc adds an 'rproc-' prefix and a '-fw' > suffix and et voilà, we load file: > > lib/firmware/rproc-st231-gp0-fw IMO deriving from the node name seems fragile. Also imposing a linux'ism "rproc" prefix on the firmware name doesn't seem correct as the firmwares can be shared across OS's. Although this is how remoteproc subsys core is currently working. It seems a generic DT firmware binding would actually be most useful for the remoteproc subsystem. I guess I should also comment about this on the ST remoteproc thread. I'm curious though as to how the ST remoteproc driver then loads the firmware, as that name doesn't look like a video or audio firmware filename that I've seen shipped from ST. IIRC Usually it is named something like vid_firmware-stih407.elf or audio_firmware-bd-stih407.elf IMO we should be treating the firmware as a blackbox, and for me that also includes the filename it is shipped with. Linux drivers making up their own firmware names based on the name of their subsystem doesn't seem like a good way forward to me. Presumably to test this driver you have to rename the firmwares locally on your machine? > > > > We could do it by parsing the node name e.g. fdma0-audio, or by adding > > > a "instance" DT property to the node? > > > > Generally we try to avoid caring about node names. Having some index > > or numbering also comes up which we also try to avoid. Generally, if > > you care about which instance you use for something, then there is > > some property you care about and should add. > > Right, the alternative is a property like the ones already used. > However, as these are becoming more prevalent I suggested > standardising the property to avoid all these vendor specific firmware > properties cluttering up the place. I agree a proliferation of vendor specific firmware properties isn't s good way forward. > > firmware = "firmwarename.fw"; > OR > firmware-name = "firmwarename.fw"; > > ... seems appropriate. Either of those is fine with me. regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
deps: update in regard to parallel initialization of static linked drivers
Am 26.08.2015 um 14:28 schrieb Alexander Holler: Hello, (...) Some numbers (5 boots on each board, without and with ordering drivers), all times are seconds. (...) imx6q (armv7): unordered: 3.451998 3.418864 3.446952 3.429974 3.440996 (3.4377568) ordered: 3.538312 3.549019 3.538105 3.515916 3.555715 (3.5394134) (...) Further improvements could be to initialize drivers in parallel (using multiple cores) and/or to use this stuff on x86 (ACPI) too (e.g. by using a minimal DT which just includes dependencies). I've now made a quick implementation which uses multiple threads to initialize annotated drivers in parallel. It worked out of the box. Here are the times (dmesg | grep Freeing) I've now measured on the imx6q (4 cores): 3.388273 3.399892 3.411615 3.410523 3.388802 (3.399821) So it's now at least as fast as without ordering the drivers. Even on the one system where ordering didn't help without parallel initialization (likely because the unordered sequence of initcalls is already quiet good on that system, in comparison to the other systems I've tested). And my current implementation for the parallel stuff is far from being perfect and can be optimized much more (enough to not post a patch here). In addition I've added another driver to my config since I measure the old times, so the new times are including one more initialization of a driver (it now calls 30 annotated (static linked) drivers at boot, most stuff is still in modules (and some not annotated static linked drivers) on that system). Maybe this helps raising interest enough that someone else will test and maybe give some (reasonable, not about style) feedback on my patches. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] thermal: mediatek: Add cpu power cooling model
Use Intelligent Power Allocation (IPA) technical to add
static/dynamic power model for binding CPU thermal zone.
The power allocator governor allocates power budget to control
CPU temperature.
Signed-off-by: Dawei Chien
---
This patch is base on
https://patchwork.kernel.org/patch/7034601/
---
drivers/cpufreq/mt8173-cpufreq.c | 97 +-
1 file changed, 86 insertions(+), 11 deletions(-)
diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mt8173-cpufreq.c
index 49caed2..9233ec5 100644
--- a/drivers/cpufreq/mt8173-cpufreq.c
+++ b/drivers/cpufreq/mt8173-cpufreq.c
@@ -28,7 +28,8 @@
#define MAX_VOLT_SHIFT (20)
#define MAX_VOLT_LIMIT (115)
#define VOLT_TOL (1)
-
+#define CAPACITANCE_CA53 (263)
+#define CAPACITANCE_CA57 (530)
/*
* The struct mtk_cpu_dvfs_info holds necessary information for doing CPU DVFS
* on each CPU power/clock domain of Mediatek SoCs. Each CPU cluster in
@@ -51,6 +52,72 @@ struct mtk_cpu_dvfs_info {
bool need_voltage_tracking;
};
+struct mtk_cpu_static_power {
+ unsigned long voltage;
+ unsigned int power;
+};
+
+/* measured by WA program. */
+static const struct mtk_cpu_static_power mtk_ca53_static_power[] = {
+ {859000, 43},
+ {908000, 52},
+ {983000, 86},
+ {1009000, 123},
+ {1028000, 138},
+ {1083000, 172},
+ {1109000, 180},
+ {1125000, 192},
+};
+
+/* measured by WA program. */
+static const struct mtk_cpu_static_power mtk_ca57_static_power[] = {
+ {828000, 72},
+ {867000, 90},
+ {927000, 156},
+ {968000, 181},
+ {1007000, 298},
+ {1049000, 435},
+ {1089000, 533},
+ {1125000, 533},
+};
+
+unsigned int mtk_cpufreq_lookup_power(const struct mtk_cpu_static_power *table,
+ unsigned int count, unsigned long voltage)
+{
+ int i;
+
+ for (i = 0; i < count; i++) {
+ if (voltage <= table[i].voltage)
+ return table[i].power;
+ }
+
+ return table[count - 1].power;
+}
+
+int mtk_cpufreq_get_static(cpumask_t *cpumask, int interval,
+ unsigned long voltage, u32 *power)
+{
+ int nr_cpus = cpumask_weight(cpumask);
+
+ *power = 0;
+
+ if (nr_cpus) {
+
+ if (cpumask_test_cpu(0, cpumask))
+ *power += mtk_cpufreq_lookup_power(
+ mtk_ca53_static_power,
+ ARRAY_SIZE(mtk_ca53_static_power),
+ voltage);
+
+ if (cpumask_test_cpu(2, cpumask))
+ *power += mtk_cpufreq_lookup_power(
+ mtk_ca57_static_power,
+ ARRAY_SIZE(mtk_ca57_static_power),
+ voltage);
+ }
+ return 0;
+}
+
static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info,
int new_vproc)
{
@@ -272,15 +339,21 @@ static void mtk_cpufreq_ready(struct cpufreq_policy
*policy)
return;
if (of_find_property(np, "#cooling-cells", NULL)) {
- info->cdev = of_cpufreq_cooling_register(np,
-policy->related_cpus);
-
- if (IS_ERR(info->cdev)) {
- dev_err(info->cpu_dev,
- "running cpufreq without cooling device: %ld\n",
- PTR_ERR(info->cdev));
-
- info->cdev = NULL;
+ u32 capacitance = cpumask_test_cpu(0, policy->related_cpus) ?
+ CAPACITANCE_CA53 : CAPACITANCE_CA57;
+
+ if (!info->cdev) {
+ info->cdev = of_cpufreq_power_cooling_register(np,
+ policy->related_cpus,
+ capacitance,
+ mtk_cpufreq_get_static);
+
+ if (IS_ERR(info->cdev)) {
+ dev_err(info->cpu_dev,
+ "running cpufreq without cooling
device: %ld\n",
+ PTR_ERR(info->cdev));
+ info->cdev = NULL;
+ }
}
}
@@ -460,7 +533,9 @@ static int mtk_cpufreq_exit(struct cpufreq_policy *policy)
{
struct mtk_cpu_dvfs_info *info = policy->driver_data;
- cpufreq_cooling_unregister(info->cdev);
+ if (info->cdev)
+ cpufreq_cooling_unregister(info->cdev);
+
dev_pm_opp_free_cpufreq_table(info->cpu_dev, &policy->freq_table);
mtk_cpu_dvfs_info_release(info);
kfree(info);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.o
[PATCH 2/2] arm64: dts: mt8173: Add thermal zone node for mt8173.
Add thermal zone node to mt8173.dtsi.
Signed-off-by: Dawei Chien
---
This patch is base on following patches
https://patchwork.kernel.org/patch/6969581/
https://patchwork.kernel.org/patch/6969571/
https://patchwork.kernel.org/patch/6969381/
---
arch/arm64/boot/dts/mediatek/mt8173.dtsi | 44 ++
1 file changed, 44 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 208051a..6493bfd 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -17,6 +17,7 @@
#include
#include
#include "mt8173-pinfunc.h"
+#include
/ {
compatible = "mediatek,mt8173";
@@ -116,6 +117,49 @@
clock-output-names = "clk32k";
};
+ thermal-zones {
+ cpu_thermal: cpu_thermal {
+ polling-delay-passive = <1000>; /* milliseconds */
+ polling-delay = <1000>; /* milliseconds */
+
+ thermal-sensors = <&thermal MT8173_THERMAL_ZONE_CA57>;
+ sustainable-power = <1500>;
+
+ trips {
+ threshold: trip-point@0 {
+ temperature = <68000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ target: trip-point@1 {
+ temperature = <85000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ cpu_crit: cpu_crit@0 {
+ temperature = <115000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+
+ cooling-maps {
+ map@0 {
+ trip = <&target>;
+ cooling-device = <&cpu0 0 0>;
+ contribution = <1024>;
+ };
+ map@1 {
+ trip = <&target>;
+ cooling-device = <&cpu2 0 0>;
+ contribution = <2048>;
+ };
+ };
+ };
+ };
+
timer {
compatible = "arm,armv8-timer";
interrupt-parent = <&gic>;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
thermal: mediatek: Add cpu power cooling model
Use Intelligent Power Allocation (IPA) technical to add static/dynamic power model for binding CPU thermal zone. The power allocator governor allocates power budget to control CPU temperature. Dawei.Chien (2): thermal: mediatek: Add cpu power cooling model. arm64: dts: mt8173: Add thermal zone node for mt8173. arch/arm64/boot/dts/mediatek/mt8173.dtsi | 44 ++ drivers/cpufreq/mt8173-cpufreq.c | 97 ++ 2 files changed, 130 insertions(+), 11 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: st_fdma: Firmware filename in DT?
Hi Rob, Many thanks for your thoughts on the issue. On Thu, 03 Sep 2015, Rob Herring wrote: > +devicetree-spec as a good question to separate from the fire hose. > > On Thu, Sep 3, 2015 at 9:49 AM, Peter Griffin > wrote: > > Hi Rob, Pawel, Mark, Ian and Kumar, > > > > Quick question regarding this series here > > https://lkml.org/lkml/2015/7/8/832. and the proposed > > st,fw-name binding. > > Thanks for looking at the bigger picture. > > > What are the rules with putting firmware names into DT? > > Whatever you can sneak in without DT maintainers noticing... :) I appear to have fallen at the first hurdle! > > > Is it allowed? > > They are already there as you have found, so yes. But should they be > allowed? Possibly. I'm not saying no, but do have some concerns. I was hoping you would have a strong opinion either for or against... > > > If yes, is it worth having a generic binding? > > If we decide it is a good idea, then yes. It doesn't look that hard to > arrive at something common. > > Strictly speaking, the firmware name is just an agreed upon name > between the kernel and userspace. So why tie that into DT? Would other > OS's use it or want something different? What if we started including > paths in the names like /? Now we are imposing a > directory structure on the OS filesystem. To answer a few of your questions in terms of STi chipsets.. For FDMA at least it seems reasonable to tie it into the DT, as like I said the same IP block 'fdma_mpe31' is used on multiple SoC's, where the only difference is location of the IP in the memory map, and the firmware filename which is loaded. I'm not 100% sure if we have different firmware files for various SoC's just because they are statically linked or if there are other differences in the firmware. It could be that some of the IP differences are in a way abstracted away from Linux due to the firmware. In reality it doesn't matter as as have no visibility of the firmware source code, so it is essentially a black box from our perspective and from Linux's PoV the IP is the same, and all the driver code which interacts with the firmware is the same. Regarding other OS's, way back when Windows CE was a thing and ran on STi SH4 chipsets, these same fdma firmwares were used. So they are in theory (at least in the past) used by other OS's. In reality these days AFAIK Linux really is the only primary OS used on STi platforms, with other RTOS's running on the co-processors. Regarding paths we certainly don't require the pathname. I would propose that is up to the primary OS, to figure out the pathnames to check once it has been given the filename. Certainly Linux does this currently, and it seems like a reasonable assumption to make of the primary OS. Like you say I don't think we want to impose directory structure on the OS filesystem, as Android, Windows CE etc will all have different ideas about where these things should live. > > We'd also need to consider firmware that is needed to boot. In that > case, we could include firmware binary directly in the dtb. I think > that has been done before, but not in any standard way. u-boot FIT > images happen to be DTBs containing mostly binary blobs. For STi we don't have any, or any that are required to boot are loaded by primary and secondary bootloaders. Are you proposing there should be a DT property which means the firmware is actually in the DTB and can be pulled out at boot by the kernel? If so it sounds like it could be a useful feature for some platforms, but not one which we require AFAIK. > > > From what I can see TI are using: - > > ti,pm-firmware in wkup_m3_rproc.c > > > > and Freescale are using: - > > fsl,sdma-ram-script-name in imx-sdma.c > > > > So other platforms seem to be putting firmware names into DT, > > there are probably other examples. > > > > Most other STi drivers keep the firmware name in the C code, which is > > usually my first preference. However with fdma it is more complicated > > as there are seperate firmware files for each instance of the IP block. > > > > Currently also the same IP block "fdma_mpe31" is used on a number > > of different SoC's but each firmware is named: - > > > > fdma__.elf > > > > e.g. fdma_STiH407_0.elf > > > > So currently all we need to provide is a different firmware name in DT > > for the new SoC and the driver "just works". > > > > Presumably the alternative would be to add a whole bunch of compatibles > > in the driver for each SoC, where the only difference from a > > functional point of view would be to help build the correct string for > > the firmware filename. However I'm also then wondering what the best way > > would be to find out the instance name of the IP. > > If the name/path is Linux specific, then that is probably what we should do. We don't need a path, and the firmware name isn't Linux specific per se. However I've written some patches which work fine avoiding the need to have the fw name in DT. See e
[PATCH 0/5] Add memory mapped read support for ti-qspi
Hi, This patch series adds support for memory mapped read port of ti-qspi. ti-qspi has a special memory mapped port through which SPI flash memories can be accessed directly via SoC specific memory region. First patch adds a method to pass flash specific information like read opcode, dummy bytes etc and to request mmap read. Second patch implements mmap read method in ti-qspi driver. Patch 3 adapts m25p80 to use mmap read method before trying normal SPI transfer. Patch 4 and 5 add memory map region DT entries for DRA7xx and AM43xx SoCs. This patch series is based on the discussions here: http://www.spinics.net/lists/linux-spi/msg04796.html Tested on DRA74 EVM and AM437x-SK. Read performance increases from ~100kB/s to ~2.5MB/s. Vignesh R (5): spi: introduce mmap read support for spi flash devices spi: spi-ti-qspi: add mmap mode read support mtd: devices: m25p80: add support for mmap read request ARM: dts: DRA7: add entry for qspi mmap region ARM: dts: AM4372: add entry for qspi mmap region Documentation/devicetree/bindings/spi/ti_qspi.txt | 18 +++- arch/arm/boot/dts/am4372.dtsi | 4 +- arch/arm/boot/dts/dra7.dtsi | 6 +- drivers/mtd/devices/m25p80.c | 8 ++ drivers/spi/spi-ti-qspi.c | 106 +- include/linux/spi/spi.h | 21 + 6 files changed, 154 insertions(+), 9 deletions(-) -- 2.5.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/7] staging: board: Add support for devices with complex dependencies
On 3 September 2015 at 15:35, Geert Uytterhoeven wrote:
> Hi Ulf,
>
> On Thu, Sep 3, 2015 at 2:53 PM, Ulf Hansson wrote:
>> On 17 June 2015 at 10:38, Geert Uytterhoeven wrote:
>>> Add support for easy registering of one ore more platform devices that
>>> may:
>>> - need clocks that are described in DT,
>>> - be part of a PM Domain.
>
>>> diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c
>>> index 8712f566b31196e0..29d456e29f38feac 100644
>>> --- a/drivers/staging/board/board.c
>>> +++ b/drivers/staging/board/board.c
>
>>> +int __init board_staging_register_device(const struct board_staging_dev
>>> *dev)
>>> +{
>>> + struct platform_device *pdev = dev->pdev;
>>> + unsigned int i;
>>> + int error;
>>> +
>>> + pr_debug("Trying to register device %s\n", pdev->name);
>>> + if (board_staging_dt_node_available(pdev->resource,
>>> + pdev->num_resources)) {
>>> + pr_warn("Skipping %s, already in DT\n", pdev->name);
>>> + return -EEXIST;
>>> + }
>>> +
>>> + board_staging_gic_fixup_resources(pdev->resource,
>>> pdev->num_resources);
>>> +
>>> + for (i = 0; i < dev->nclocks; i++)
>>> + board_staging_register_clock(&dev->clocks[i]);
>>> +
>>> + error = platform_device_register(pdev);
>>> + if (error) {
>>> + pr_err("Failed to register device %s (%d)\n", pdev->name,
>>> + error);
>>> + return error;
>>> + }
>>> +
>>> + if (dev->domain)
>>> + __pm_genpd_name_add_device(dev->domain, &pdev->dev, NULL);
>>
>> Urgh, this managed to slip through my filters.
>>
>> It seems like we almost managed to remove all users of the
>> "..._name_add..." APIs for genpd. If hasn't been for $subject patch.
>> :-)
>>
>> Now, I realize this is already too late here, but let's try to fix
>> this before it turns into a bigger issue.
>>
>> Geert, do you think it's possible to convert into using the non-named
>> bases APIs?
>
> That will be difficult. This code is meant to use drivers that are not yet
> DT-aware on DT-based systems. Hence it uses platform devices with named PM
> domains, while the PM domains are described in DT.
> I don't think there's another way to look up a PM domain by name, is there?
As a matter of fact there are, especially for those genpds that has
been created through DT as in this case. The API to use is
of_genpd_get_from_provider() to find the struct generic_pm_domain.
Yes, I do realize that you need to manage the parsing of the domain
name to make sure it's the one you want, but I would rather keep that
"hack" in this driver than in the generic API.
>
> This code is meant to go away, once all drivers are converted to DT, or
> considered obsolete.
Well, who knows *when* that is going to happen. :-)
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] spi: spi-ti-qspi: add mmap mode read support
ti-qspi controller provides mmap port to read data from SPI flashes.
mmap port is enabled in QSPI_SPI_SWITCH_REG (ctrl module bits may
also need to be updated for some SoCs). The QSPI_SPI_SETUP_REGx needs to
be populated with flash specific information like read opcode, read
mode(quad, dual, normal), address width and dummy bytes. Once,
controller is in mmap mode, the whole flash memory is available as a
memory region at SoC specific address. This region can be accessed using
normal memcpy() or mem-to-mem dma copy. The ti-qspi controller hardware
will internally communicate with SPI flash over SPI bus and get the
requested data.
Implement spi_mtd_mmap_read() method to support mmap read over SPI
flash devices. With this, the read throughput increases from ~100kB/s to
~2.5 MB/s.
Signed-off-by: Vignesh R
---
drivers/spi/spi-ti-qspi.c | 106 --
1 file changed, 102 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index aa6d284131e0..a07610b84bc9 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -71,11 +71,8 @@ struct ti_qspi {
#define QSPI_SPI_CMD_REG (0x48)
#define QSPI_SPI_STATUS_REG(0x4c)
#define QSPI_SPI_DATA_REG (0x50)
-#define QSPI_SPI_SETUP0_REG(0x54)
+#define QSPI_SPI_SETUP_REG(n) ((0x54 + 4 * n))
#define QSPI_SPI_SWITCH_REG(0x64)
-#define QSPI_SPI_SETUP1_REG(0x58)
-#define QSPI_SPI_SETUP2_REG(0x5c)
-#define QSPI_SPI_SETUP3_REG(0x60)
#define QSPI_SPI_DATA_REG_1(0x68)
#define QSPI_SPI_DATA_REG_2(0x6c)
#define QSPI_SPI_DATA_REG_3(0x70)
@@ -120,6 +117,16 @@ struct ti_qspi {
#define QSPI_AUTOSUSPEND_TIMEOUT 2000
+#define MEM_CS_EN(n) ((n + 1) << 8)
+
+#define MM_SWITCH 0x1
+
+#define QSPI_SETUP_RD_NORMAL (0x0 << 12)
+#define QSPI_SETUP_RD_DUAL (0x1 << 12)
+#define QSPI_SETUP_RD_QUAD (0x3 << 12)
+#define QSPI_SETUP_ADDR_SHIFT 8
+#define QSPI_SETUP_DUMMY_SHIFT 10
+
static inline unsigned long ti_qspi_read(struct ti_qspi *qspi,
unsigned long reg)
{
@@ -361,6 +368,96 @@ static int qspi_transfer_msg(struct ti_qspi *qspi, struct
spi_transfer *t)
return 0;
}
+static void ti_qspi_enable_memory_map(struct spi_device *spi)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(spi->master);
+ u32 val;
+
+ ti_qspi_write(qspi, MM_SWITCH, QSPI_SPI_SWITCH_REG);
+ if (qspi->ctrl_mod) {
+ val = readl(qspi->ctrl_base);
+ val |= MEM_CS_EN(spi->chip_select);
+ writel(val, qspi->ctrl_base);
+ }
+}
+
+static void ti_qspi_disable_memory_map(struct spi_device *spi)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(spi->master);
+ u32 val;
+
+ ti_qspi_write(qspi, 0, QSPI_SPI_SWITCH_REG);
+ if (qspi->ctrl_mod) {
+ val = readl(qspi->ctrl_base);
+ val &= ~MEM_CS_EN(spi->chip_select);
+ writel(val, qspi->ctrl_base);
+ }
+}
+
+static void ti_qspi_setup_mmap_read(struct spi_device *spi,
+ u8 read_opcode, u8 addr_width,
+ u8 dummy_bytes)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(spi->master);
+ u32 mode = spi->mode & (SPI_RX_DUAL | SPI_RX_QUAD);
+ u32 memval = read_opcode;
+
+ switch (mode) {
+ case SPI_RX_QUAD:
+ memval |= QSPI_SETUP_RD_QUAD;
+ break;
+ case SPI_RX_DUAL:
+ memval |= QSPI_SETUP_RD_DUAL;
+ break;
+ default:
+ memval |= QSPI_SETUP_RD_NORMAL;
+ break;
+ }
+ memval |= ((addr_width - 1) << QSPI_SETUP_ADDR_SHIFT |
+ dummy_bytes << QSPI_SETUP_DUMMY_SHIFT);
+ ti_qspi_write(qspi, memval,
+ QSPI_SPI_SETUP_REG(spi->chip_select));
+}
+
+static int ti_qspi_spi_mtd_mmap_read(struct spi_device *spi,
+loff_t from, size_t len,
+size_t *retlen, u_char *buf,
+u8 read_opcode, u8 addr_width,
+u8 dummy_bytes)
+{
+ struct ti_qspi *qspi = spi_master_get_devdata(spi->master);
+ int ret = 0;
+
+ spi_bus_lock(qspi->master);
+ mutex_lock(&qspi->list_lock);
+ ret = pm_runtime_get_sync(qspi->dev);
+ if (ret < 0) {
+ dev_err(qspi->dev, "pm_runtime_get_sync() failed\n");
+ return ret;
+ }
+
+ /* disable WC interrupt during memcpy */
+ ti_qspi_write(qspi, QSPI_WC_INT_DISABLE, QSPI_INTR_ENABLE_CLEAR_REG);
+ ti_qspi_enable_memory_map(spi);
+ ti_qspi_setup_mmap_read(spi, read_opcode, addr_width,
+ dum
[PATCH 1/5] spi: introduce mmap read support for spi flash devices
In addition to providing direct access to SPI bus, some spi controller
hardwares (like ti-qspi) provide special memory mapped port
to accesses SPI flash devices in order to increase read performance.
This means the controller can automatically send the SPI signals
required to read data from the SPI flash device.
For this, spi controller needs to know flash specific information like
read command to use, dummy bytes and address width. Once these settings
are populated in hardware registers, any read accesses to flash's memory
map region(SoC specific) through memcpy or mem-to-mem DMA copy will be
handled by controller hardware. The hardware will automatically generate
spi signals required to read data from flash and present it to CPU or
DMA engine.
Introduce spi_mtd_mmap_read() method to support memory mapped read
over SPI flash devices. SPI master drivers can implement this method to
support memory mapped read interfaces. m25p80 flash driver and other
flash drivers can call this to request memory mapped read. The interface
should only be used mtd flashes and cannot be used with other spi devices.
Signed-off-by: Vignesh R
---
include/linux/spi/spi.h | 21 +
1 file changed, 21 insertions(+)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index d673072346f2..b74a3f169fc2 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -293,6 +293,23 @@ static inline void spi_unregister_driver(struct spi_driver
*sdrv)
* @handle_err: the subsystem calls the driver to handle an error that occurs
* in the generic implementation of transfer_one_message().
* @unprepare_message: undo any work done by prepare_message().
+ * @spi_mtd_mmap_read: some spi-controller hardwares provide memory
+ * mapped interface to communicate with mtd flashes.
+ * For this, spi controller needs to know flash
+ * memory settings like read command to use, dummy
+ * bytes and address width. Once these settings are
+ * populated in hardware registers, any read
+ * accesses to flash's memory map region(as defined
+ * by SoC) through memcpy or mem-to-mem DMA copy
+ * will be handled by controller hardware. The
+ * hardware will automatically generate spi signals
+ * required to read data from flash and present it
+ * to CPU or DMA. SPI master drivers can use this
+ * callback to implement memory mapped read
+ * interface. Flash driver (like m25p80) requests
+ * memory mapped read via this method. The interface
+ * should only be used mtd flashes and cannot be
+ * used with other spi devices.
* @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
* number. Any individual value may be -ENOENT for CS lines that
* are not GPIOs (driven by the SPI controller itself).
@@ -438,6 +455,10 @@ struct spi_master {
struct spi_message *message);
int (*unprepare_message)(struct spi_master *master,
struct spi_message *message);
+ int (*spi_mtd_mmap_read)(struct spi_device *spi,
+loff_t from, size_t len, size_t *retlen,
+u_char *buf, u8 read_opcode,
+u8 addr_width, u8 dummy_bytes);
/*
* These hooks are for drivers that use a generic implementation
--
2.5.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] mtd: devices: m25p80: add support for mmap read request
Certain spi controllers may support memory mapped interface to read from
m25p80 type flash devices. This interface provides better read
performance than regular SPI interface.
Call spi_mtd_mmap_read() function, if available, to make use of
memory-mapped interface.
Signed-off-by: Vignesh R
---
drivers/mtd/devices/m25p80.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index d313f948b96c..b8b391aab331 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -133,6 +133,14 @@ static int m25p80_read(struct spi_nor *nor, loff_t from,
size_t len,
/* convert the dummy cycles to the number of bytes */
dummy /= 8;
+ if (spi->master->spi_mtd_mmap_read) {
+ return spi->master->spi_mtd_mmap_read(spi, from, len,
+ retlen, buf,
+ nor->read_opcode,
+ nor->addr_width,
+ dummy);
+ }
+
spi_message_init(&m);
memset(t, 0, (sizeof t));
--
2.5.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] ARM: dts: AM4372: add entry for qspi mmap region
Add qspi memory mapped region entries for AM43xx based SoCs. Also,
update the binding documents for the controller to document this change.
Signed-off-by: Vignesh R
---
Documentation/devicetree/bindings/spi/ti_qspi.txt | 5 +++--
arch/arm/boot/dts/am4372.dtsi | 4 +++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/ti_qspi.txt
b/Documentation/devicetree/bindings/spi/ti_qspi.txt
index f05dd631bef1..05488970060b 100644
--- a/Documentation/devicetree/bindings/spi/ti_qspi.txt
+++ b/Documentation/devicetree/bindings/spi/ti_qspi.txt
@@ -17,9 +17,10 @@ Recommended properties:
Example:
+For am4372:
qspi: qspi@4b30 {
- compatible = "ti,dra7xxx-qspi";
- reg = <0x4790 0x100>, <0x3000 0x3ff>;
+ compatible = "ti,am4372-qspi";
+ reg = <0x4790 0x100>, <0x3000 0x400>;
reg-names = "qspi_base", "qspi_mmap";
#address-cells = <1>;
#size-cells = <0>;
diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index ade28c790f4b..52cf4846b8e1 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -902,7 +902,9 @@
qspi: qspi@4790 {
compatible = "ti,am4372-qspi";
- reg = <0x4790 0x100>;
+ reg = <0x4790 0x100>,
+ <0x3000 0x400>;
+ reg-names = "qspi_base", "qspi_mmap";
#address-cells = <1>;
#size-cells = <0>;
ti,hwmods = "qspi";
--
2.5.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] ARM: dts: DRA7: add entry for qspi mmap region
Add qspi memory mapped region entries for DRA7xx based SoCs. Also,
update the binding documents for the controller to document this change.
Signed-off-by: Vignesh R
---
Documentation/devicetree/bindings/spi/ti_qspi.txt | 13 +
arch/arm/boot/dts/dra7.dtsi | 6 --
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/ti_qspi.txt
b/Documentation/devicetree/bindings/spi/ti_qspi.txt
index 601a360531a5..f05dd631bef1 100644
--- a/Documentation/devicetree/bindings/spi/ti_qspi.txt
+++ b/Documentation/devicetree/bindings/spi/ti_qspi.txt
@@ -26,3 +26,16 @@ qspi: qspi@4b30 {
spi-max-frequency = <2500>;
ti,hwmods = "qspi";
};
+
+For dra7xx:
+qspi: qspi@4b30 {
+ compatible = "ti,dra7xxx-qspi";
+ reg = <0x4b30 0x100>, <0x4a002558 0x4>,
+ <0x5c00 0x400>;
+ reg-names = "qspi_base", "qspi_ctrlmod",
+ "qspi_mmap";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ spi-max-frequency = <4800>;
+ ti,hwmods = "qspi";
+};
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 1e29ccf77ea2..f6798d6ecd60 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1103,8 +1103,10 @@
qspi: qspi@4b30 {
compatible = "ti,dra7xxx-qspi";
- reg = <0x4b30 0x100>;
- reg-names = "qspi_base";
+ reg = <0x4b30 0x100>, <0x4a002558 0x4>,
+ <0x5c00 0x400>;
+ reg-names = "qspi_base", "qspi_ctrlmod",
+ "qspi_mmap";
#address-cells = <1>;
#size-cells = <0>;
ti,hwmods = "qspi";
--
2.5.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 3/6] pci:host: Add Altera PCIe host controller driver
On Wed, Sep 2, 2015 at 12:33 AM, Lorenzo Pieralisi
wrote:
>
> On Tue, Sep 01, 2015 at 11:30:05AM +0100, Ley Foon Tan wrote:
>
> [...]
>
> > +static void altera_pcie_retrain(struct pci_dev *dev)
> > +{
> > + u16 linkcap, linkstat;
> > +
> > + /*
> > +* Set the retrain bit if the PCIe rootport support > 2.5GB/s, but
> > +* current speed is 2.5 GB/s.
> > +*/
> > + pcie_capability_read_word(dev, PCI_EXP_LNKCAP, &linkcap);
> > +
> > + if ((linkcap & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
> > + return;
> > +
> > + pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &linkstat);
> > + if ((linkstat & PCI_EXP_LNKSTA_CLS) == PCI_EXP_LNKSTA_CLS_2_5GB)
> > + pcie_capability_set_word(dev, PCI_EXP_LNKCTL,
> > +PCI_EXP_LNKCTL_RL);
> > +}
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID,
> > altera_pcie_retrain);
>
> This filtering is still arguable, since it unconditionally applies to
> all Altera PCI devices (I guess there are not any apart from this
> host controller).
This fixup is required for all Altera PCIe controller in all device families.
>
> > +
> > +static void altera_pcie_fixup_res(struct pci_dev *dev)
> > +{
> > + /*
> > +* Prevent enumeration of root port.
> > +*/
> > + if (!dev->bus->parent && dev->devfn == 0) {
> > + int i;
> > +
> > + for (i = 0; i < PCI_NUM_RESOURCES; i++) {
> > + dev->resource[i].start = 0;
> > + dev->resource[i].end = 0;
> > + dev->resource[i].flags = 0;
> > + }
> > + }
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ALTERA, PCI_ANY_ID,
> > +altera_pcie_fixup_res);
>
> Ditto, it is a quirk of your host controller, not a quirk for
> all Altera PCI devices.
Same here.
>
> > +static inline void cra_writel(struct altera_pcie *pcie, u32 value, u32 reg)
> > +{
> > + writel_relaxed(value, pcie->cra_base + reg);
> > +}
> > +
> > +static inline u32 cra_readl(struct altera_pcie *pcie, u32 reg)
> > +{
> > + return readl_relaxed(pcie->cra_base + reg);
> > +}
> > +
> > +static void tlp_read_rx(struct altera_pcie *pcie,
> > + struct tlp_rp_regpair_t *tlp_rp_regdata)
> > +{
> > + tlp_rp_regdata->ctrl = cra_readl(pcie, RP_RXCPL_STATUS);
> > + tlp_rp_regdata->reg0 = cra_readl(pcie, RP_RXCPL_REG0);
> > + tlp_rp_regdata->reg1 = cra_readl(pcie, RP_RXCPL_REG1);
> > +}
> > +
> > +static void tlp_write_tx(struct altera_pcie *pcie,
> > +struct tlp_rp_regpair_t *tlp_rp_regdata)
> > +{
> > + cra_writel(pcie, tlp_rp_regdata->reg0, RP_TX_REG0);
> > + cra_writel(pcie, tlp_rp_regdata->reg1, RP_TX_REG1);
> > + cra_writel(pcie, tlp_rp_regdata->ctrl, RP_TX_CNTRL);
> > +}
> > +
> > +static bool altera_pcie_link_is_up(struct altera_pcie *pcie)
> > +{
> > + return !!(cra_readl(pcie, RP_LTSSM) & LTSSM_L0);
> > +}
> > +
> > +static bool altera_pcie_valid_config(struct altera_pcie *pcie,
> > +struct pci_bus *bus, int dev)
> > +{
> > + /* If there is no link, then there is no device */
> > + if (bus->number != pcie->root_bus_nr) {
> > + if (!altera_pcie_link_is_up(pcie))
> > + return false;
> > + }
>
> Can you explain to pls me why you have to check this for every config
> transaction ? Isn't it something that can prevent probing the
> host controller altogether ?
In our PCIe hardware spec, it stated that software should check the
link status before issuing a configuration request to downstream
ports.
BTW, other pci controllers have similar implementation as well, eg: dw
pci, mvebu pci.
>
> > +
> > + /* access only one slot on each root port */
> > + if (bus->number == pcie->root_bus_nr && dev > 0)
> > + return false;
> > +
> > + /*
> > +* Do not read more than one device on the bus directly attached
> > +* to RC.
>
> You should also explain why.
Okay.
>
> > +*/
> > + if (bus->primary == pcie->root_bus_nr && dev > 0)
> > + return false;
> > +
> > +return true;
> > +}
> > +
> > +static int tlp_read_packet(struct altera_pcie *pcie, u32 *value)
> > +{
> > + u8 loop;
> > + struct tlp_rp_regpair_t tlp_rp_regdata;
> > +
> > + for (loop = 0; loop < TLP_LOOP; loop++) {
> > + tlp_read_rx(pcie, &tlp_rp_regdata);
> > + if (tlp_rp_regdata.ctrl & RP_RXCPL_EOP) {
> > + if (value)
> > + *value = tlp_rp_regdata.reg0;
> > + return PCIBIOS_SUCCESSFUL;
> > + }
> > + udelay(5);
>
> Could you comment please on the chosen udelay/TLP_LOOP values (ie how
> did you come up with them) ?
For udelay value, we just want t
Re: [PATCH v4 3/5] dt/bindings: bcm2835: add binding documentation for bcm2835-aux
> On 26.08.2015, at 03:44, Stephen Warren wrote:
>
> On 08/24/2015 02:40 AM, ker...@martin.sperl.org wrote:
>
>> +Example:
>> +
>> +aux_enable: aux_enable@0x7e215004 {
>> +compatible = "bcrm,bcm2835-aux";
>> +reg = <0x7e215004 0x04>;
>
> I'd expect that to be <0x7e215000 0x8>;
The reason is that we just handle enable with this driver,
which just requires access to the 0x7e215004 register.
The 0x7e215000 register (interrupt mask) could be used by a
cascaded interrupt-controller, but as the spi and uart drivers
can run with shared interrupts this is not a necessity.
Martin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 01/18] platform: delay OF device-driver matches until late_initcall
On 14 August 2015 at 21:09, Grygorii Strashko wrote: > Hi Tomeu, > > On 08/09/2015 04:03 PM, Tomeu Vizoso wrote: >> On 7 August 2015 at 19:06, Grygorii Strashko >> wrote: >>> On 08/07/2015 10:11 AM, Tomeu Vizoso wrote: On 6 August 2015 at 22:19, Rob Herring wrote: > On Thu, Aug 6, 2015 at 9:11 AM, Tomeu Vizoso > wrote: >> Delay matches of platform devices with OF nodes until late_initcall, >> when we are sure that all built-in drivers have been registered already. >> This is needed to prevent deferred probes because of some drivers not >> having registered yet. >> >> The reason why only platform devices are delayed is that some other >> devices are expected to be probed earlier than late_initcall, for >> example, the system PNP driver needs to probe its devices in >> fs_initcall. >> >> Additionally, only platform devices with OF nodes are delayed because >> some machines may depend on oter platform devices being registered at >> specific times. > > How do we know that these probes occur before the unused clocks and > regulators are turned off? Just getting lucky (as is deferred probe)? > Can we do this one level earlier so we have a level left to do things > after probe. Those are already late_initcall_sync so I guess we're fine. >>> >>> I wouldn't be so sure :( >>> FYI: >>> http://git.ti.com/ti-linux-kernel/ti-linux-kernel/commit/763d643bbfc0f445c6685c541fcae3c370e4314a > > I'm very sorry for delayed reply. > > First of all, I'd like to say that proposition to move drivers > probing/matching > one layer early (device_initcall_sync) is reasonable > (and I've already mentioned the same here https://lkml.org/lkml/2015/6/3/979). Yeah, I have submitted this change to kernelci and I'm awaiting the results. Note though that none of the boards in there (currently 91) had any problem when probes were deferred until late_initcall. >> If I understand the situation correctly, this is one more instance of >> starting to do some work at some point and hoping that something else >> that started before has already finished happening. If that's the >> case, how does this series make that worst? > > You going to increase pressure on the late boot stages without taking > into account that there could be platforms which perfectly works > just using init_calls. And the fact that drivers will be not probed at > expected time could be very surprising for them. Well, they work perfectly until a driver in the dependency chain starts deferring its probe, or gets renamed/moved and thus probed later, or async probing gets enabled, etc. Then the boot breaks and someone has to spend an evening adding printks all along the place. I know that things have worked to date, but it's so fragile to have to impose order by manually ordering things in the makefiles and fiddling with initcall levels that we had to introduce the very big hammer that deferred probing is, to assure that we get a working system at the end. But probing devices in a random order and relying on the return value of the probe callback to retry them later means that we can no longer warn about failed probes because the norm now is for them to fail a few times before they succeed. And that also means that if you want for, say, the panel to be up and running as soon as possible during boot, you have to do an absurd amount of fiddling in the DT to have the nodes in dependency order. But if drivers and initcalls explicitly try to acquire the resources they depend on instead of just hoping for them to be there, then of_device_probe should end up being called and will ensure the dependencies are there at the point when they are needed. > Or, there should be an option to disable this functionality. Yeah, Rob suggested it already. I will be adding such an option just in case it's useful to someone. >> During this development I have found many hacks intended to put some >> order, even if not enough care was taken to make sure that the order >> was guaranteed. In general I would recommend for moving code into >> proper drivers and have them to defer the probe of their devices if >> some dependency isn't fulfilled at that moment. > > Unfortunately, not everything can be moved to drivers or it could be > just unreasonable. Hopefully those initcalls can state their dependencies explicitly (which should end up causing the required devices to be probed). >> Once that's done and we have a safe and reliable boot, we can avoid >> those deferred probes by fulfilling the dependency on-demand as this >> series shows. >> >> There was some recent thread about how the disabling of unused clocks >> and regulators isn't really safe because after late_initcall_sync more >> drivers can be registered from modules. Furthermore, there's async >> device probes. > > Clocks and regulators are safe, problems may happen first of all with > platform's specific initcalls. Guess that starting to pr
