* Re: [RFC 4/6] ARM: dts: dra7: add entry for bb2d module
From: Lucas Stach @ 2016-11-18 10:59 UTC (permalink / raw)
To: Robert Nelson
Cc: tony-4v6yS6AI5VpBDgjK7y7TUQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Gowtham Tammana, Tomi Valkeinen
In-Reply-To: <20161118024436.13447-4-robertcnelson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:
> From: Gowtham Tammana <g-tammana-l0cyMroinI0@public.gmane.org>
>
> BB2D entry is added to the dts file. Crossbar index number is used
> for interrupt mapping.
>
> Signed-off-by: Gowtham Tammana <g-tammana-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
> ---
> arch/arm/boot/dts/dra7.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index addb753..43488b6 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -959,6 +959,16 @@
> ti,hwmods = "dmm";
> };
>
> + bb2d: bb2d@59000000 {
> + compatible = "ti,dra7-bb2d";
> + reg = <0x59000000 0x0700>;
> + interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
> + ti,hwmods = "bb2d";
> + clocks = <&dpll_core_h24x2_ck>;
> + clock-names = "fclk";
"fclk" is not an accepted clock name for the etnaviv driver. It supports
up to 3 clocks: "bus", "core" and "shader". If there is only one clock
required in your design it would probably be the "core" clock.
Regards,
Lucas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC 5/6] ARM: dts: dra7: add vivante for bb2d module
From: Lucas Stach @ 2016-11-18 10:55 UTC (permalink / raw)
To: Robert Nelson
Cc: tony-4v6yS6AI5VpBDgjK7y7TUQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Julien,
Nishanth Menon, Tomi Valkeinen
In-Reply-To: <20161118024436.13447-5-robertcnelson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:
> Signed-off-by: Robert Nelson <robertcnelson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: Julien <jboulnois-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> CC: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
> CC: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> ---
> arch/arm/boot/dts/dra7.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 43488b6..22bd0a5 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -960,7 +960,7 @@
> };
>
> bb2d: bb2d@59000000 {
> - compatible = "ti,dra7-bb2d";
> + compatible = "ti,dra7-bb2d","vivante,gc";
This is what the driver expects as a compatible, but it's not consistent
with the DT documentation patch you sent. As the driver can work out the
HW version from the identification registers I would say fix your DT
binding to only require "vivante,gc"
Regards,
Lucas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Sudeep Holla @ 2016-11-18 10:41 UTC (permalink / raw)
To: Frank Rowand, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Sudeep Holla, Rob Herring, Arnd Bergmann,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <582E1A59.7040502-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 17/11/16 21:00, Frank Rowand wrote:
> On 11/17/16 07:32, Sudeep Holla wrote:
>> Currently platforms/drivers needing to get the machine model name are
>> replicating the same snippet of code. In some case, the OF reference
>> counting is either missing or incorrect.
>>
>> This patch adds support to read the machine model name either using
>> the "model" or the "compatible" property in the device tree root node
>> to the core OF/DT code.
>>
>> This can be used to remove all the duplicate code snippets doing exactly
>> same thing later.
>
> I find five instances of reading only property "model":
>
> arch/arm/mach-imx/cpu.c
> arch/arm/mach-mxs/mach-mxs.c
> arch/c6x/kernel/setup.c
> arch/mips/cavium-octeon/setup.c
> arch/sh/boards/of-generic.c
>
Ah sorry you were not Cc-ed in 2/2, but that shows all the instances
that this will be used for.
> I find one instance of reading property "model", then if
> that does not exist, property "compatible":
>
> arch/mips/generic/proc.c
>
Correct as you can check in patch 2/2
> The proposed patch matches the code used in one place, and thus
> current usage does not match the patch description.
>
Yes, but does it matter ? compatibles are somewhat informative about the
model IMO.
> Is my search bad? Are you planning to add additional instances
> of reading "model" then "compatible"?
>
No, just replacing the existing ones as in patch 2/2
--
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] arm64: dts: exynos: add the mshc_2 node for supporting T-Flash
From: Marek Szyprowski @ 2016-11-18 10:38 UTC (permalink / raw)
To: Jaehoon Chung, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, kgene-DgEjT+Ai2ygdnm+yROfE0A,
krzk-DgEjT+Ai2ygdnm+yROfE0A, cw00.choi-Sze3O3UU22JBDgjK7y7TUQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8
In-Reply-To: <20161118071224.12876-1-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Dear Jaehoon,
On 2016-11-18 08:12, Jaehoon Chung wrote:
> Add the mshc_2 node for supporting T-flash.
>
> And it needs to add the "mshc*" aliases. Because dwmmc driver should be
> assigned to "ctrl_id" after parsing to "mshc".
> If there is no aliases for mshc, then it might be set to the wrong
> capabilities.
>
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> index 9ea3f32..75dab01 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
> @@ -42,6 +42,8 @@
> spi2 = &spi_2;
> spi3 = &spi_3;
> spi4 = &spi_4;
> + mshc0 = &mshc_0;
> + mshc2 = &mshc_2;
> };
>
> chosen {
> @@ -661,6 +663,23 @@
> assigned-clock-rates = <800000000>;
> };
>
> +&mshc_2 {
> + status = "okay";
> + num-slots = <1>;
> + cap-sd-highspeed;
> + disable-wp;
> + cd-gpios = <&gpa2 4 0>;
cd-gpios = <&gpa2 4 GPIO_ACTIVE_HIGH>;
> + cd-inverted;
> + card-detect-delay = <200>;
> + samsung,dw-mshc-ciu-div = <3>;
> + samsung,dw-mshc-sdr-timing = <0 4>;
> + samsung,dw-mshc-ddr-timing = <0 2>;
> + fifo-depth = <0x80>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_bus1 &sd2_bus4>;
> + bus-width = <4>;
> +};
> +
> &pinctrl_alive {
> pinctrl-names = "default";
> pinctrl-0 = <&initial_alive>;
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-18 10:17 UTC (permalink / raw)
To: liviu.dudau-5wv7dgnIgG8@public.gmane.org
Cc: Gabriele Paoloni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Yuanzhichang, mark.rutland-5wv7dgnIgG8@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org,
minyard-HInyCGIudOg@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org,
John Garry, will.deacon-5wv7dgnIgG8@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, xuwei (O),
Linuxarm, zourongrong-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
In-Reply-To: <20161114112625.GO10219-2JSQmVVBSi7ZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
On Monday, November 14, 2016 11:26:25 AM CET liviu.dudau-5wv7dgnIgG8@public.gmane.org wrote:
> On Mon, Nov 14, 2016 at 08:26:42AM +0000, Gabriele Paoloni wrote:
> > > Nope, that is not what it means. It means that PCI devices can see I/O
> > > addresses
> > > on the bus that start from 0. There never was any usage for non-PCI
> > > controllers
> >
> > So I am a bit confused...
> > From http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > It seems that ISA buses operate on cpu I/O address range [0, 0xFFF].
> > I thought that was the reason why for most architectures we have
> > PCIBIOS_MIN_IO equal to 0x1000 (so I thought that ISA controllers
> > usually use [0, PCIBIOS_MIN_IO - 1] )
>
> First of all, cpu I/O addresses is an x86-ism. ARM architectures and others
> have no separate address space for I/O, it is all merged into one unified
> address space. So, on arm/arm64 for example, PCIBIOS_MIN_IO = 0 could mean
> that we don't care about ISA I/O because the platform does not support having
> an ISA bus (e.g.).
I think to be more specific, PCIBIOS_MIN_IO=0 would indicate that you cannot
have a PCI-to-ISA or PCI-to-LPC bridge in any PCI domain. This is different
from having an LPC master outside of PCI, as that lives in its own domain
and has a separately addressable I/O space.
> > As said before this series forbid IO tokens to be in [0, PCIBIOS_MIN_IO)
> > to allow special ISA controllers to use that range with special
> > accessors.
> > Having a variable threshold would make life much more difficult
> > as there would be a probe dependency between the PCI controller and
> > the special ISA one (PCI to wait for the special ISA device to be
> > probed and set the right threshold value from DT or ACPI table).
> >
> > Instead using PCIBIOS_MIN_IO is easier and should not impose much
> > constraint as [PCIBIOS_MIN_IO, IO_SPACE_LIMIT] is available to
> > the PCI controller for I/O tokens...
>
> What I am suggesting is to leave PCIBIOS_MIN_IO alone which still reserves
> space for ISA controller and add a PCIBIOS_MIN_DIRECT_IO that will reserve
> space for your direct address I/O on top of PCIBIOS_MIN_IO.
The PCIBIOS_MIN_DIRECT_IO name still suggests having something related to
PCIBIOS_MIN_IO, but it really isn't. We are talking about multiple
concepts here that are not the same but that are somewhat related:
a) keeping PCI devices from allocating low I/O ports on the PCI bus
that would conflict with ISA devices behind a bridge of the
same bus.
b) reserving the low 0x0-0xfff range of the Linux-internal I/O
space abstraction to a particular LPC or PCI domain to make
legacy device drivers work that hardcode a particular port
number.
c) Redirecting inb/outb to call a domain-specific accessor function
rather than doing the normal MMIO window for an LPC master or
more generally any arbitrary LPC or PCI domain that has a
nonstandard I/O space.
[side note: actually if we generalized this, we could avoid
assigning an MMIO range for the I/O space on the pci-mvebu
driver, and that would help free up some other remapping
windows]
I think there is no need to change a) here, we have PCIBIOS_MIN_IO
today and even if we don't need it, there is no obvious downside.
I would also argue that we can ignore b) for the discussion of
the HiSilicon LPC driver, we just need to assign some range
of logical addresses to each domain.
That means solving c) is the important problem here, and it
shouldn't be so hard. We can do this either with a single
special domain as in the v5 patch series, or by generalizing it
so that any I/O space mapping gets looked up through the device
pointer of the bus master.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] ata: xgene: Enable NCQ support for APM X-Gene SATA controller hardware v1.1
From: Rameshwar Prasad Sahu @ 2016-11-18 9:45 UTC (permalink / raw)
To: olof-nZhT3qVonbNeoWH0uzbU5w, tj-DgEjT+Ai2ygdnm+yROfE0A,
arnd-r2nGTMty4D4
Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
mlangsdo-H+wXaHxf7aLQT0dZR+AlfA, jcm-H+wXaHxf7aLQT0dZR+AlfA,
patches-qTEPVZfXA3Y, Rameshwar Prasad Sahu
This patch enables NCQ support for APM X-Gene SATA controller hardware v1.1
that was broken with hardware v1.0. Second thing, here we should not assume
XGENE_AHCI_V2 always in case of having valid _CID in ACPI table. I need to
remove this assumption because V1_1 also has a valid _CID for backward
compatibly with v1.
v2 changes:
1. Changed patch description
Signed-off-by: Rameshwar Prasad Sahu <rsahu-qTEPVZfXA3Y@public.gmane.org>
---
drivers/ata/ahci_xgene.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 73b19b2..8b88be9 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -87,6 +87,7 @@
enum xgene_ahci_version {
XGENE_AHCI_V1 = 1,
+ XGENE_AHCI_V1_1,
XGENE_AHCI_V2,
};
@@ -734,6 +735,7 @@ static struct scsi_host_template ahci_platform_sht = {
#ifdef CONFIG_ACPI
static const struct acpi_device_id xgene_ahci_acpi_match[] = {
{ "APMC0D0D", XGENE_AHCI_V1},
+ { "APMC0D67", XGENE_AHCI_V1_1},
{ "APMC0D32", XGENE_AHCI_V2},
{},
};
@@ -742,6 +744,7 @@ MODULE_DEVICE_TABLE(acpi, xgene_ahci_acpi_match);
static const struct of_device_id xgene_ahci_of_match[] = {
{.compatible = "apm,xgene-ahci", .data = (void *) XGENE_AHCI_V1},
+ {.compatible = "apm,xgene-ahci-v1-1", .data = (void *) XGENE_AHCI_V1_1},
{.compatible = "apm,xgene-ahci-v2", .data = (void *) XGENE_AHCI_V2},
{},
};
@@ -755,8 +758,7 @@ static int xgene_ahci_probe(struct platform_device *pdev)
struct resource *res;
const struct of_device_id *of_devid;
enum xgene_ahci_version version = XGENE_AHCI_V1;
- const struct ata_port_info *ppi[] = { &xgene_ahci_v1_port_info,
- &xgene_ahci_v2_port_info };
+ const struct ata_port_info *ppi;
int rc;
hpriv = ahci_platform_get_resources(pdev);
@@ -821,8 +823,6 @@ static int xgene_ahci_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "%s: Error reading device info. Assume version1\n",
__func__);
version = XGENE_AHCI_V1;
- } else if (info->valid & ACPI_VALID_CID) {
- version = XGENE_AHCI_V2;
}
}
}
@@ -858,18 +858,20 @@ skip_clk_phy:
switch (version) {
case XGENE_AHCI_V1:
+ ppi = &xgene_ahci_v1_port_info;
hpriv->flags = AHCI_HFLAG_NO_NCQ;
break;
case XGENE_AHCI_V2:
+ ppi = &xgene_ahci_v2_port_info;
hpriv->flags |= AHCI_HFLAG_YES_FBS;
hpriv->irq_handler = xgene_ahci_irq_intr;
break;
default:
+ ppi = &xgene_ahci_v1_port_info;
break;
}
- rc = ahci_platform_init_host(pdev, hpriv, ppi[version - 1],
- &ahci_platform_sht);
+ rc = ahci_platform_init_host(pdev, hpriv, ppi, &ahci_platform_sht);
if (rc)
goto disable_resources;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v3 09/13] ARM: dts: armada-375: Fixup soc DT warning
From: Gregory CLEMENT @ 2016-11-18 9:38 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161118101248.784eff2b-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Thomas,
On ven., nov. 18 2016, Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hello,
>
> On Fri, 18 Nov 2016 10:01:32 +0100, Gregory CLEMENT wrote:
>
>> >> + soc@f00100000000 {
>> >
>> > Where is this value coming from? Why does the soc node needs to have a
>>
>> It cames from the dts files.
>
> Where?
--- a/arch/arm/boot/dts/armada-375-db.dts
+++ b/arch/arm/boot/dts/armada-375-db.dts
@@ -63,7 +63,11 @@
reg = <0x00000000 0x40000000>; /* 1 GB */
};
- soc {
+ /* The following unit address is composed of the target
+ * value (bit [40-47]), attributes value (bits [32-39],
+ * and the address value in the window memory: [0-31].
+ */
+ soc@f00100000000 {
ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
just here ---------^
MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000
MBUS_ID(0x09, 0x09) 0 0xf1100000 0x10000
>
>> > unit address? It doesn't have a 'reg' property if I remember
>> > correctly.
>>
>> But it has a range property.
>
> And? There are multiple ranges, and you randomly took the first one for
> the unit address of the soc node?
Not randomly I followed the same rules that for the regs mentioned in
the ePAPR paragraph 2.2.1.1:
"The unit-address should match the first address specified in the reg
property of the node."
>
> You realize that the ranges property is a list of ranges, and they
> could be in any order? Why would you pick the base address of one of
> the ranges rather than any of the others?
It is the same for the regs so as explained I followed the same rules.
>
> I believe there is simply no unit address for the soc {} node. There is
> definitely one for the internal-regs {} node, but not for the soc {}
> node.
It is not the interpretation of the DTC:
"Warning (unit_address_vs_reg): Node /soc has a reg or ranges property,
but no unit name"
Gregory
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] soc/tegra: Implement Tegra186 PMC support
From: Jon Hunter @ 2016-11-18 9:36 UTC (permalink / raw)
To: Thierry Reding
Cc: Rob Herring, Mark Rutland, Stephen Warren, Alexandre Courbot,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161117171636.20580-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 17/11/16 17:16, Thierry Reding wrote:
> From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> The power management controller on Tegra186 has changed in backwards-
> incompatible ways with respect to earlier generations. This implements a
> new driver that supports inversion of the PMU interrupt as well as the
> "recovery", "bootloader" and "forced-recovery" reboot commands.
>
> Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> .../bindings/arm/tegra/nvidia,tegra186-pmc.txt | 34 +++++
> drivers/soc/tegra/Makefile | 2 +-
> drivers/soc/tegra/pmc-tegra186.c | 169 +++++++++++++++++++++
> 3 files changed, 204 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra186-pmc.txt
> create mode 100644 drivers/soc/tegra/pmc-tegra186.c
...
> diff --git a/drivers/soc/tegra/Makefile b/drivers/soc/tegra/Makefile
> index ae857ff7d53d..9976a0de1927 100644
> --- a/drivers/soc/tegra/Makefile
> +++ b/drivers/soc/tegra/Makefile
> @@ -1,4 +1,4 @@
> obj-y += fuse/
>
> obj-y += common.o
> -obj-y += pmc.o
> +obj-y += pmc.o pmc-tegra186.o
Do we want to enable this driver for all Tegra devices? Was not sure if
this should be dependent on ARM64.
Cheers
Jon
--
nvpublic
^ permalink raw reply
* [PATCH 2/2] PM / OPP: Introduce domain-performance-state binding to OPP nodes
From: Viresh Kumar @ 2016-11-18 9:23 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
Kevin Hilman, Ulf Hansson, Vincent Guittot, Lina Iyer,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nayak Rajendra, Viresh Kumar
In-Reply-To: <cover.1479459752.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their nodes directly.
But if the device needs the capability of switching to different domain
performance states, as they may need to support different clock rates,
then the per OPP node can be used to contain that information.
This patch introduces the domain-performance-state (already defined by
Power Domain bindings) to the per OPP node. It can contain a single
positive integer value. An example is also provided.
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 57 +++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index ee91cbdd95ee..9fb7804f784d 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -145,6 +145,14 @@ properties.
- status: Marks the node enabled/disabled.
+- domain-performance-state: A positive integer value representing the minimum
+ performance level (of the parent domain) required by the consumer for the
+ working of respective OPP. The integer value '1' represents the lowest
+ performance level and the highest value represents the highest performance
+ level. The consumer device node (which contains phandle to the OPP table in
+ its "operating-points-v2" property) should have its "power-domains" property
+ set as well.
+
Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
/ {
@@ -517,3 +525,52 @@ Example 5: opp-supported-hw
};
};
};
+
+Example 7: domain-Performance-state:
+(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
+
+/ {
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ compatible = "arm,cortex-a9";
+ reg = <0>;
+ next-level-cache = <&L2>;
+ clocks = <&clk_controller 0>;
+ clock-names = "cpu";
+ cpu-supply = <&cpu_supply0>;
+ operating-points-v2 = <&cpu0_opp_table>;
+ power-domains = <&foo_pd>;
+ };
+ };
+
+ cpu0_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp@1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ opp-microvolt = <970000 975000 985000>;
+ opp-microamp = <70000>;
+ clock-latency-ns = <300000>;
+ opp-suspend;
+ domain-performance-state = <1>;
+ };
+ opp@1100000000 {
+ opp-hz = /bits/ 64 <1100000000>;
+ opp-microvolt = <980000 1000000 1010000>;
+ opp-microamp = <80000>;
+ clock-latency-ns = <310000>;
+ domain-performance-state = <2>;
+ };
+ opp@1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ opp-microvolt = <1025000>;
+ clock-latency-ns = <290000>;
+ turbo-mode;
+ domain-performance-state = <2>;
+ };
+ };
+};
--
2.7.1.410.g6faf27b
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding
From: Viresh Kumar @ 2016-11-18 9:23 UTC (permalink / raw)
To: Rafael Wysocki
Cc: linaro-kernel, linux-pm, linux-kernel, Rob Herring, Mark Rutland,
Kevin Hilman, Ulf Hansson, Vincent Guittot, Lina Iyer, devicetree,
Stephen Boyd, Nayak Rajendra, Viresh Kumar
In-Reply-To: <cover.1479459752.git.viresh.kumar@linaro.org>
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
The power-domains until now were only concentrating on the idle state
management of the device and this needs to change in order to reuse the
infrastructure of power domains for active state management.
This patch introduces a new optional property for the consumers of the
power-domains: domain-performance-state.
If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their node directly. Otherwise the
consumers can define their requirements with help of other
infrastructure, for example the OPP table.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e1650364b296..db42eacf8b5c 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -106,6 +106,12 @@ domain provided by the 'parent' power controller.
- power-domains : A phandle and PM domain specifier as defined by bindings of
the power controller specified by phandle.
+Optional properties:
+- domain-performance-state: A positive integer value representing the minimum
+ performance level (of the parent domain) required by the consumer for its
+ working. The integer value '1' represents the lowest performance level and the
+ highest value represents the highest performance level.
+
Example:
leaky-device@12350000 {
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH 0/2] PM / Domains / OPP: Introduce domain-performance-state binding
From: Viresh Kumar @ 2016-11-18 9:23 UTC (permalink / raw)
To: Rafael Wysocki
Cc: linaro-kernel, linux-pm, linux-kernel, Rob Herring, Mark Rutland,
Kevin Hilman, Ulf Hansson, Vincent Guittot, Lina Iyer, devicetree,
Stephen Boyd, Nayak Rajendra, Viresh Kumar
Hello,
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
We had some discussions about it in the past on the PM list [1], which
is followed by discussions during the LPC. The outcome of all that was
that we should extend Power Domain framework to support active state
power management as well.
The power-domains until now were only concentrating on the idle state
management of the device and this needs to change in order to reuse the
infrastructure of power domains for active state management.
To get a complete picture of the proposed plan, following is what we
need to do:
- Create DT bindings to get domain performance state information for the
platforms.
- Enhance OPP framework to parse these and call into the PM Qos
framework with a performance state request.
- Enhance PM Qos framework to provide the API to be used by consumers
(or OPP framework) and pass it on to the (Generic) Power Domain
framework.
- Enhance Generic Power Domain framework to accept such requests,
accumulate all belonging to a single power domain and call domain
driver specific callback with the performance state we want for the
domain.
- The domain driver shall then, in a platform specific way, set the
requested performance level.
- Note that these features are applicable to the CPU, GPU and other IIO
or non-IIO devices.
- There can be cases where a device can choose between multiple power
domains based on what performance level we want for the device. In
such cases, we should represent the multiplexer with a separate power
domain. In effect, the device (or OPP table) will correspond to a
single power domain, but the backend driver of that domain shall
implement the multiplexing functionality.
This patchset implements the very first part of this chain and
introduces a new optional property for the consumers of the
power-domains: domain-performance-state. This property can be used
directly by the consumer or its OPP table.
--
viresh
[1] https://marc.info/?l=linux-pm&m=147747923708075&w=2
Viresh Kumar (2):
PM / Domains: Introduce domain-performance-state binding
PM / OPP: Introduce domain-performance-state binding to OPP nodes
Documentation/devicetree/bindings/opp/opp.txt | 57 ++++++++++++++++++++++
.../devicetree/bindings/power/power_domain.txt | 6 +++
2 files changed, 63 insertions(+)
--
2.7.1.410.g6faf27b
^ permalink raw reply
* Re: [PATCH V5 2/3] ARM64 LPC: Add missing range exception for special ISA
From: Arnd Bergmann @ 2016-11-18 9:22 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, One Thousand Gnomes, catalin.marinas,
gabriele.paoloni, benh, will.deacon, linuxarm, lorenzo.pieralisi,
xuwei5, linux-serial, linux-pci, devicetree, minyard,
marc.zyngier, liviu.dudau, john.garry, olof, robh+dt, bhelgaas,
kantyzc, zhichang.yuan02, linux-kernel, zhichang.yuan,
zourongrong
In-Reply-To: <20161114111111.1b753dc3@lxorguk.ukuu.org.uk>
On Monday, November 14, 2016 11:11:11 AM CET One Thousand Gnomes wrote:
> > > It's not a safe assumption for x86 at least. There are a few systems with
> > > multiple ISA busses particularly older laptops with a docking station.
> >
> > But do they have multiple ISA domains? There is no real harm in supporting
> > it, the (small) downsides I can think of are:
>
> I don't believe they x86 class ones have multiple ISA domains. But as
> I've said I don't know how the electronics in the older ThinkPad worked
> when it used two PIIX4s with some LPC or ISA stuff on each.
>
> It works in DOS and unmodified Linux so I'm pretty sure there are no
> additional domains. Likewise the various x86 schemes that route some bits
> of ISA bus off into strange places work in DOS and don't have any
> overlaps.
>
> yenta_socket handles PCI/PCMCIA bridging and routes a range of that flat
> ISA space appropriately to the card.
Right, that's what I had expected, so we still don't even
need to handle multiple ISA I/O address spaces for the
only known case of multiple ISA buses, though we may decide
to generalize the code like that anyway.
Arnd
^ permalink raw reply
* Re: [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Arnd Bergmann @ 2016-11-18 9:20 UTC (permalink / raw)
To: linux-arm-kernel
Cc: zhichang.yuan, Benjamin Herrenschmidt, Mark Rutland,
gabriele.paoloni, catalin.marinas, will.deacon, linuxarm,
lorenzo.pieralisi, xuwei5, linux-serial, linux-pci, devicetree,
minyard, marc.zyngier, liviu.dudau, john.garry, zourongrong,
robh+dt, bhelgaas, kantyzc, zhichang.yuan02, linux-kernel, olof
In-Reply-To: <5825984B.3030303@hisilicon.com>
On Friday, November 11, 2016 6:07:07 PM CET zhichang.yuan wrote:
>
> I have similar idea as your PPC MMIO.
>
> We notice the prototype of {in/out()} is something like that:
>
> static inline u8 inb(unsigned long addr)
> static inline void outb(u8 value, unsigned long addr)
>
> The type of parameter 'addr' is unsigned long. For I/O space, it is big enough.
> So, could you divide this 'addr' into several bit segments? The top 8 bits is
> defined as bus index. For normal direct IO, the bus index is 0. For those bus
> device which need indirectIO or some special I/O accessors, when these devices
> are initializing, can request to allocate an unique ID to them, and register
> their own accessors to the entry which is corresponding to the ID.
Ah, have you looked at the IA64 code? It does exactly this.
For ARM64 we decided to use the same basic approach as powerpc with
a single range of virtual memory for mapping it as that somewhat
simplified all cases we knew about at the time.
> In this way, we can support multiple domains, I think.
> But I am not sure whether it is feasible, for example, are there some
> architectures/platforms had populated the top 8 bits? Do we need to request IO
> region from ioport_resource for those devices? etc...
On a 64-bit architecture, the top 32 bits of the port number are
definitely free to use for this, and 8 bits are probably sufficient.
Even on 32 bit architectures, I can't see why we'd ever need more than
16 bits worth of addressing within a domain, so using 8 bit domain
and 16 bit address leaves 8 or 40 unused bits.
Arnd
^ permalink raw reply
* Re: [PATCH] ARM: dts: sunxi: Explicitly enable pull-ups for MMC pins
From: Linus Walleij @ 2016-11-18 9:18 UTC (permalink / raw)
To: klaus.goger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5, Ulf Hansson,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Chen-Yu Tsai, Maxime Ripard,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-sunxi
In-Reply-To: <ea02ec937a12d6ca2a2e0d8bbea5c484-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
[Notice this reply has little to do with the patch in question: I think
it should be applied. I just want to involve some MMC/SD people here]
On Thu, Nov 17, 2016 at 9:57 PM, <klaus.goger-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org> wrote:
> On 2016-11-17 10:34, Chen-Yu Tsai wrote:
>>
>> Given that MMC starts in open-drain mode and the pull-ups are required,
>> it's best to enable it for all the pin settings.
>
> It's even more complicated than that with MMC. It starts in open-drain mode
> for
> CMD during initialization but changes to push-pull afterwards. The card has
> internal pull-ups to prevent bus floating during setup and will disable them
> after switching to 4bit mode (or 8bit for eMMC when available).
> But even after switching to push-pull drivers there are states the bus would
> float and pull ups have to ensure a high level on the bus.
>
> See JESD84-A441 chapter 7.15 ff as reference.
>
> The difference between the P-bit and Z-bit is that a P-bit is actively
> driven
> to HIGH by the card respectively host output driver, while Z-bit is driven
> to
> (respectively kept) HIGH by the pull-up resistors Rcmd respectively Rdat.
>
> Enabling the pull ups on the CPU would be the right choice considering that
> most boards will not have external pull-ups. Even if they would have one,
> adding the internal in parallel would work in almost all cases and the
> increase in power consumption would be negligible.
I guess you are referring to software-controlled pull up on the pad ring of
the SoC. Nominally that should be handled by pin control like in this
patch.
It is not clear from context to me which of the lines need to be handled
like this? Just the data lines? DAT0 would be the critical line to pull up
in that case, since the others will not be used until bus switch anyways
right?
But what about CMD?
I assume CLK should always be push-pull?
Also: if the pins have an explicit Open Drain setting, should that
be used? I guess so?
Overall I think this construction is pretty common.
We essentially have two classes of connections:
- Those connecting the eMMC or even MMC/SD card directly to
the SoC. No pull-ups on the board. Here it makes sense to have:
1. A pin control default state with open drain and pull-ups
enabled for those lines
and it then needs
2. A second "4/8bit mode" that will switch these
pins to push-pull mode and turn off the pull-ups.
If the OD+pull-up state is the default we should probably
standardize a default name for the state when we kick in 4/8bit
mode from the IOS operation.
- Those connection the MMC/SD on an external slot through a
levelshifter/EMI filter. In that case I guess it is pretty much
dependent on the levelshifter or EMI thing how the lines out
of the SoC should be configured, and usually it is static so
you do not need to worry about it after boot configuration
of pins. (Mostly no bias, push-pull I think.)
I highly suspect a whole bunch of SoC drivers are not getting
this business right today. Not even in out-of-tree vendor kernels.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3 09/13] ARM: dts: armada-375: Fixup soc DT warning
From: Thomas Petazzoni @ 2016-11-18 9:12 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87d1htb1qr.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hello,
On Fri, 18 Nov 2016 10:01:32 +0100, Gregory CLEMENT wrote:
> >> + soc@f00100000000 {
> >
> > Where is this value coming from? Why does the soc node needs to have a
>
> It cames from the dts files.
Where?
> > unit address? It doesn't have a 'reg' property if I remember
> > correctly.
>
> But it has a range property.
And? There are multiple ranges, and you randomly took the first one for
the unit address of the soc node?
You realize that the ranges property is a list of ranges, and they
could be in any order? Why would you pick the base address of one of
the ranges rather than any of the others?
I believe there is simply no unit address for the soc {} node. There is
definitely one for the internal-regs {} node, but not for the soc {}
node.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 09/13] ARM: dts: armada-375: Fixup soc DT warning
From: Gregory CLEMENT @ 2016-11-18 9:01 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161118095455.00bfe007-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Thomas,
On ven., nov. 18 2016, Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hello,
>
> On Fri, 18 Nov 2016 00:08:26 +0100, Gregory CLEMENT wrote:
>
>> - soc {
>> + /* The following unit address is composed of the target
>> + * value (bit [40-47]), attributes value (bits [32-39],
>> + * and the address value in the window memory: [0-31].
>> + */
>> + soc@f00100000000 {
>
> Where is this value coming from? Why does the soc node needs to have a
It cames from the dts files.
> unit address? It doesn't have a 'reg' property if I remember
> correctly.
But it has a range property.
Gregory
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 06/13] ARM: dts: armada-375: Fixup sa-ram DT warning
From: Thomas Petazzoni @ 2016-11-18 8:59 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161117230830.31047-7-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hello,
On Fri, 18 Nov 2016 00:08:23 +0100, Gregory CLEMENT wrote:
> - crypto_sram0: sa-sram0 {
> + /* The following unit addresses (for sa-sram) are composed of
> + * the target value (bit [40-47]), attributes value (bits
> + * [32-39], and the address value in the window memory: [0-31].
> + */
The "address value in the window memory" part doesn't make a lot of
sense. Maybe:
"The following unit addresses are composed of the window target ID
(bits 40-47), the window target attributes (bits 32-39) and the offset
inside the window."
Also, the comment formatting is not compliant with the coding style,
should be:
/*
* ...
* ...
*/
But do we really want this comment above each node? Couldn't we instead
add this explanation in the mvebu-mbus.txt DT binding?
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 09/13] ARM: dts: armada-375: Fixup soc DT warning
From: Thomas Petazzoni @ 2016-11-18 8:54 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161117230830.31047-10-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hello,
On Fri, 18 Nov 2016 00:08:26 +0100, Gregory CLEMENT wrote:
> - soc {
> + /* The following unit address is composed of the target
> + * value (bit [40-47]), attributes value (bits [32-39],
> + * and the address value in the window memory: [0-31].
> + */
> + soc@f00100000000 {
Where is this value coming from? Why does the soc node needs to have a
unit address? It doesn't have a 'reg' property if I remember correctly.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V8 2/6] thermal: bcm2835: add thermal driver for bcm2835 soc
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2016-11-18 8:32 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Zhang Rui, Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Lee Jones, Eric Anholt, Russell King, Florian Fainelli,
Catalin Marinas, Will Deacon, linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161117151019.GA3115-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
> On 17.11.2016, at 16:10, Eduardo Valentin <edubezval-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> Hello Martin,
>
> On Thu, Nov 17, 2016 at 10:51:33AM +0100, Martin Sperl wrote:
>>
>>
>> On 17.11.2016 03:11, Eduardo Valentin wrote:
>>> Hey Martin,
>>>
>>> Very sorry for the late feedback. Not so sure if this one got queued
>>> already or not. Anyways, just minor questions as follows:
>>>
>>> On Wed, Nov 02, 2016 at 10:18:22AM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>>>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>>>
>>>> Add basic thermal driver for bcm2835 SOC.
>>>>
>>>> This driver currently relies on the firmware setting up the
>>>> tsense HW block and does not set it up itself.
>>>>
>>>> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>>>> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
>>>> Acked-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>>>
>> ...
>>>> +static int bcm2835_thermal_adc2temp(
>>>> + const struct bcm2835_thermal_info *info, u32 adc)
>>>> +{
>>>> + return info->offset + (adc * info->slope);
>>>
>>> Any specific reason we cannot use thermal_zone_params->slope and
>>> thermal_zone_params->offset?
>>
>> You could - the patch was just rebased to 4.9 and those slope and
>> offset just got merged during this cycle.
>>
>> Do we really need to modify it - the patch has been around since 4.6.
>>
>>>> +
>>>> +static int bcm2835_thermal_get_trip_temp(
>>>> + struct thermal_zone_device *tz, int trip, int *temp)
>>>> +{
>>>> + struct bcm2835_thermal_data *data = tz->devdata;
>>>> + u32 val = readl(data->regs + BCM2835_TS_TSENSCTL);
>>>> +
>>>> + /* get the THOLD bits */
>>>> + val &= BCM2835_TS_TSENSCTL_THOLD_MASK;
>>>> + val >>= BCM2835_TS_TSENSCTL_THOLD_SHIFT;
>>>> +
>>>> + /* if it is zero then use the info value */
>>>> + if (val)
>>>
>>> Is this a read only register or is this driver supposed to program it?
>>> In which scenario it would be 0? Can this be added as comments?
>>
>> It is RW, but the Firmware typically sets up the thermal device with the
>> correct values already - this is just a fallback.
>
> OK, but how do you differentiate from a buggy firmware or misconfigured
> hardware? How do you know if the firmware is supposed to be loaded and
> ready? There is no firmware loading in this driver. Also, there is no
> dependency with a driver that loads firmware, or at least, I missed it.
>
The way that firmware works on the RPI is quite different from most others I guess.
in principle you got 2 different CPUs on the bcm2835:
* ARM, which runs the linux instance
* VideoCore 4, which runs the firmware (loading from SD initially) and
then booting the ARM.
So this Firmware on VC4 is the one that I am talking about.
Without the working firmware linux can not boot on arm.
As of now he firmware is also checking temperature of the SOC itself and
if it finds “critical” temperatures it will clock down ARM and the GPU.
Hope that this explains the “firmware” on bcm2835.
>>
>>>> +static int bcm2835_thermal_get_temp(struct thermal_zone_device *tz,
>>>> + int *temp)
>>>> +{
>>>> + struct bcm2835_thermal_data *data = tz->devdata;
>>>> + u32 val = readl(data->regs + BCM2835_TS_TSENSSTAT);
>>>> +
>>>> + if (!(val & BCM2835_TS_TSENSSTAT_VALID))
>>>
>>> What cases you would get the valid bit not set? Do you need to wait for
>>> the conversion to finish?
>>
>> I guess: if you have just enabled the HW-block (which the FW does much
>> in advance) and start to read the value immediately (before the first sample
>> period has finished), then this will not be valid.
>>
>
> Then again, how does this driver make sure the initialization procedure
> is correct, and that by the time it is using the hardware, the hardware
> is in a sane state?
>
> Back to the original question, does it mean the hardware is in
> some sort of continuous ADC conversion mode or reading the temperature
> requires specific steps to get the conversion done, and therefore you
> are checking the valid bit here?
As far as I understand the conversion is continuous (as soon as the HW is
configured). This case is there primarily to handle the situation where
we initialize the HW ourselves (see line 226 and below), and we immediately
want to read the ADC value before the first conversion is finished.
The above mentioned “configuration if not running” reflect the values that
the FW is currently setting. We should not change those values as long as the
Firmware is also reading the temperature on its own.
>
>> So do you need another version of the patchset that uses that new API?
>
> I think the API usage is change that can be done together with
> clarification for the above questions too: on hardware state,
> firmware loading, maybe a master driver dependency, and the ADC
> conversion sequence, which are not well clear to me on this driver. As long as
> this is clarified and documented in the code (can be simple comments so
> it is clear to whoever reads in the future), then I would be OK with
> this driver.
So how do you want this to get “documented” in the driver?
The setup and Firmware is a generic feature of the SOC, so if we would put
some clarifications in this driver, then we would need to put it in every
bcm283X driver (which seems unreasonable).
Thanks,
Martin--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 REPOST 8/9] arm: add sysfs cpu_capacity attribute
From: Juri Lelli @ 2016-11-18 8:27 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, peterz-wEGCiKHe2LqWVfeAwA7xHQ,
vincent.guittot-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
sudeep.holla-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
morten.rasmussen-5wv7dgnIgG8, dietmar.eggemann-5wv7dgnIgG8,
broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20161103052842.GA16920@e106622-lin>
Hi Russell,
On 03/11/16 05:28, Juri Lelli wrote:
> Hi,
>
> apologies for the delay in replying, but I'm attending Linux Plumbers
> this week.
>
> On 30/10/16 20:45, Russell King - ARM Linux wrote:
> > On Mon, Oct 17, 2016 at 04:46:49PM +0100, Juri Lelli wrote:
[...]
>
> I should have addressed your comments with the updated version below. If
> it looks good to you I'll superseed the old version with this new one.
>
The two updated patches are still listed as incoming in your system.
Do you think we will be able to queue them for 4.10? IMHO, it would be
good to have all pieces in together at once (Catalin and Sudeep already
queued their respective bits).
Thanks a lot.
Best,
- Juri
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/3] hwmon: ltc2990: refactor value conversion
From: Tom Levens @ 2016-11-18 8:18 UTC (permalink / raw)
To: Guenter Roeck
Cc: Tom Levens, jdelvare, robh+dt, mark.rutland, linux-kernel,
linux-hwmon, devicetree
In-Reply-To: <1eff12d5-e0f9-aaa0-010d-1ddcccdd6ec9@roeck-us.net>
Hi Guenter,
On Thu, 17 Nov 2016, Guenter Roeck wrote:
> On 11/17/2016 08:23 AM, Tom Levens wrote:
>> Hi Guenter,
>>
>> Thanks for taking the time to review the patch.
>>
>> On Thu, 17 Nov 2016, Guenter Roeck wrote:
>>
>> > Hi Tom,
>> >
>> > On 11/17/2016 04:10 AM, Tom Levens wrote:
>> > > Conversion from raw values to signed integers has been refactored
>> > > using
>> > > the macros in bitops.h.
>> > >
>> > Please also mention that this fixes a bug in negative temperature
>> > conversions.
>>
>> Yes, of course, I will include the information in v3.
>>
>> >
>> > > Signed-off-by: Tom Levens <tom.levens@cern.ch>
>> > > ---
>> > > drivers/hwmon/ltc2990.c | 27 ++++++++++-----------------
>> > > 1 files changed, 10 insertions(+), 17 deletions(-)
>> > >
>> > > diff --git a/drivers/hwmon/ltc2990.c b/drivers/hwmon/ltc2990.c
>> > > index 8f8fe05..0ec4102 100644
>> > > --- a/drivers/hwmon/ltc2990.c
>> > > +++ b/drivers/hwmon/ltc2990.c
>> > > @@ -9,8 +9,12 @@
>> > > * This driver assumes the chip is wired as a dual current monitor,
>> > > and
>> > > * reports the voltage drop across two series resistors. It also
>> > > reports
>> > > * the chip's internal temperature and Vcc power supply voltage.
>> > > + *
>> > > + * Value conversion refactored
>> > > + * by Tom Levens <tom.levens@cern.ch>
>> >
>> > Kind of unusual to do that for minor changes like this. Imagine if
>> > everyone would do that.
>> > The commit log is what gives you credit.
>>
>> Good point, thanks for the hint. I will remove it from v3.
>>
>> > > */
>> > >
>> > > +#include <linux/bitops.h>
>> > > #include <linux/err.h>
>> > > #include <linux/hwmon.h>
>> > > #include <linux/hwmon-sysfs.h>
>> > > @@ -34,19 +38,10 @@
>> > > #define LTC2990_CONTROL_MODE_CURRENT 0x06
>> > > #define LTC2990_CONTROL_MODE_VOLTAGE 0x07
>> > >
>> > > -/* convert raw register value to sign-extended integer in 16-bit
>> > > range */
>> > > -static int ltc2990_voltage_to_int(int raw)
>> > > -{
>> > > - if (raw & BIT(14))
>> > > - return -(0x4000 - (raw & 0x3FFF)) << 2;
>> > > - else
>> > > - return (raw & 0x3FFF) << 2;
>> > > -}
>> > > -
>> > > /* Return the converted value from the given register in uV or mC */
>> > > -static int ltc2990_get_value(struct i2c_client *i2c, u8 reg, int
>> > > *result)
>> > > +static int ltc2990_get_value(struct i2c_client *i2c, u8 reg, s32
>> > > *result)
>> > > {
>> > > - int val;
>> > > + s32 val;
>> >
>> > Please just leave the variable type alone. it is also used for the
>> > return value
>> > from i2c_smbus_read_word_swapped(), which is an int, and changing it to
>> > s32 doesn't really make the code better.
>>
>> According to i2c.h the return type for i2c_smbus_read_word_swapped() is
>> s32, which is why I modified it here. But it could be changed back if you
>> think it is better to leave it as an int.
>>
> Ah, ok. Good to know. Please leave it anyway, reason being that there is no
> real
> reason to change it. Effectively those are just whitespace changes (unlike
> the rest,
> which is part bug fix, part cleanup).
>
>> > Can you send me a register map for the chip ? I would like to write a
>> > module test.
>>
>> Here is an example register dump:
>
> I meant the output of i2cdump (something like "i2cdump -y -f <bus>
> <i2c-address> w").
>
The register map wraps at 0x0F, so I only sent you the first 16 bytes. But
the fully expanded form is:
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
10: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
20: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
30: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
40: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
50: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
60: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
70: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
80: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
90: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
a0: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
b0: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
c0: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
d0: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
e0: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
f0: 00 00 00 00 01 90 07 d0 2c cd 7d 80 7c 29 20 00
Cheers,
> Thanks,
> Guenter
>
>>
>> 00 00 00 00
>> 01 90 07 d0
>> 2c cd 7d 80
>> 7c 29 20 00
>>
>> The expected values in this case are:
>>
>> in0_input 5000
>> in1_input 610
>> in2_input 3500
>> in3_input -195
>> in4_input -299
>> temp1_input 25000
>> temp2_input 125000
>> temp3_input -40000
>> curr1_input 38840
>> curr2_input -12428
>>
>> Testing with lltc,mode set to <5>, <6> and <7> should give you all
>> measurements.
>>
>> > Thanks,
>> > Guenter
>>
>> Cheers,
>>
>
>
^ permalink raw reply
* [PATCH] arm64: dts: exynos: add the mshc_2 node for supporting T-Flash
From: Jaehoon Chung @ 2016-11-18 7:12 UTC (permalink / raw)
To: linux-samsung-soc
Cc: linux-kernel, devicetree, kgene, krzk, cw00.choi, robh+dt,
mark.rutland, catalin.marinas, will.deacon, Jaehoon Chung
Add the mshc_2 node for supporting T-flash.
And it needs to add the "mshc*" aliases. Because dwmmc driver should be
assigned to "ctrl_id" after parsing to "mshc".
If there is no aliases for mshc, then it might be set to the wrong
capabilities.
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index 9ea3f32..75dab01 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -42,6 +42,8 @@
spi2 = &spi_2;
spi3 = &spi_3;
spi4 = &spi_4;
+ mshc0 = &mshc_0;
+ mshc2 = &mshc_2;
};
chosen {
@@ -661,6 +663,23 @@
assigned-clock-rates = <800000000>;
};
+&mshc_2 {
+ status = "okay";
+ num-slots = <1>;
+ cap-sd-highspeed;
+ disable-wp;
+ cd-gpios = <&gpa2 4 0>;
+ cd-inverted;
+ card-detect-delay = <200>;
+ samsung,dw-mshc-ciu-div = <3>;
+ samsung,dw-mshc-sdr-timing = <0 4>;
+ samsung,dw-mshc-ddr-timing = <0 2>;
+ fifo-depth = <0x80>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_bus1 &sd2_bus4>;
+ bus-width = <4>;
+};
+
&pinctrl_alive {
pinctrl-names = "default";
pinctrl-0 = <&initial_alive>;
--
2.10.1
^ permalink raw reply related
* Re: [PATCH 1/3] arm: hisi: add ARCH_MULTI_V5 support
From: Jiancheng Xue @ 2016-11-18 6:42 UTC (permalink / raw)
To: Marty Plummer
Cc: Wei Xu, wenpan, Arnd Bergmann, mturquette, sboyd, robh+dt,
mark.rutland, linux, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, howell.yang, jalen.hsu, lvkuanliang, suwenping,
raojun, kevin.lixu, qinxiaojun
In-Reply-To: <114569cb-79ab-a46d-8582-6169f182a32c@hisilicon.com>
Hi Marty,
On 2016/11/17 11:03, Jiancheng Xue wrote:
> Hi Wei，
>
> On 2016/11/16 17:31, Wei Xu wrote:
>> Hi Pan,
>>
>> On 2016/11/16 8:56, wenpan wrote:
>>> Hi Marty，
>>> Does this confict with your patch？ If not，I hope this could be merged first. Besides could you tell me the link to your related patch?
>>
>> This is the link: https://patchwork.kernel.org/patch/9334743/
>>
Could you give your comments on this patch?
If you have any objections to it, please let us know.
>
> Thank you for offering this.If I want to give some comments on Marty's patch,
> what should I do?
>
> For Marty's patch, I think there's no need to add specific config item ARCH_HIxxxx
> for every chipset. Some existing chipsets depend on ARCH_HISI directly like Hi3519
> and Hi3798CV200. If some options like ARM_GIC is removed from ARCH_HISI, this kind
> of chipsets will must choose other place to select it. I suggest we should keep selecting
> ARM_GIC under ARCH_HISI as Pan's patch do.
>
> The code may be like this:
>
> config ARCH_HISI
> bool "Hisilicon SoC Support"
> - depends on ARCH_MULTI_V7
> + depends on ARCH_MULTI_V5 || ARCH_MULTI_V6 || ARCH_MULTI_V7
> select ARM_AMBA
> - select ARM_GIC
> + select ARM_GIC if ARCH_MULTI_V7
> + select ARM_VIC if ARCH_MULTI_V5 || depends on ARCH_MULTI_V6
> select ARM_TIMER_SP804
> select POWER_RESET
> select POWER_RESET_HISI
> select POWER_SUPPLY
>
What's your opinion about this?
Best Regards,
Jiancheng
>>> On 2016/10/17 21:48, Arnd Bergmann wrote:
>>>> On Monday, October 17, 2016 8:07:03 PM CEST Pan Wen wrote:
>>>>> Add support for some HiSilicon SoCs which depend on ARCH_MULTI_V5.
>>>>>
>>>>> Signed-off-by: Pan Wen <wenpan@hisilicon.com>
>>>>>
>>>>
>>>> Looks ok. I've added Marty Plummer to Cc, he was recently proposing
>>>> patches for Hi3520, which I think is closely related to this one.
>>>> Please try to work together so the patches don't conflict. It should
>>>> be fairly straightforward since you are basically doing the same
>>>> change here.
>>>>
>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH v5] drm/mediatek: fixed the calc method of data rate per lane
From: CK Hu @ 2016-11-18 5:17 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Mark Rutland, stonea168, dri-devel,
Yingjoe Chen (陳英洲), Ajay Kumar,
Vincent Palatin, cawa cheng, Russell King,
open list:OPEN FIRMWARE AND..., Jitao Shi, Pawel Moll,
Ian Campbell, Rob Herring,
moderated list:ARM/Mediatek SoC support, Andy Yan,
Matthias Brugger, Eddie Huang (黃智傑),
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAGS+omBpdRH5ZnvcApX_pevwSBHwUon77DhhjX0p9aQXuOy4DA@mail.gmail.com>
Hi, Daniel:
On Fri, 2016-11-18 at 11:22 +0800, Daniel Kurtz wrote:
> Hi CK,
>
> On Thu, Nov 17, 2016 at 1:36 PM, CK Hu <ck.hu@mediatek.com> wrote:
> > Hi, Jitao:
> >
> >
> > On Wed, 2016-11-16 at 11:20 +0800, Jitao Shi wrote:
> >> Tune dsi frame rate by pixel clock, dsi add some extra signal (i.e.
> >> Tlpx, Ths-prepare, Ths-zero, Ths-trail,Ths-exit) when enter and exit LP
> >> mode, those signals will cause h-time larger than normal and reduce FPS.
> >> So need to multiply a coefficient to offset the extra signal's effect.
> >> coefficient = ((htotal*bpp/lane_number)+Tlpx+Ths_prep+Ths_zero+
> >> Ths_trail+Ths_exit)/(htotal*bpp/lane_number)
> >>
> >> Signed-off-by: Jitao Shi <jitao.shi@mediatek.com>
> >
> > It looks good to me.
> > But this patch conflict with [1] which is one patch of MT2701 series. I
> > want to apply MT2701 patches first, so please help to refine this patch
> > based on MT2701 patches.
>
> I don't think the MT2701 DSI patches are quite ready yet (I just
> reviewed the one below).
> Can we instead land Jitao's small targeted change first, and then
> rebase the MT2701 series on top.
>
> Thanks,
> -Dan
MT2701 series looks still have some defect to be fixed.
Therefore, I would apply this patch first.
Thanks for your help.
Regards,
CK
> >
> > [1] https://patchwork.kernel.org/patch/9422821/
> >
> > Regards,
> > CK
> >
> >> ---
> >> Change since v4:
> >> - tune the calc comment more clear.
> >> - define the phy timings as constants.
> >>
> >> Chnage since v3:
> >> - wrapp the commit msg.
> >> - fix alignment of some lines.
> >>
> >> Change since v2:
> >> - move phy timing back to dsi_phy_timconfig.
> >>
> >> Change since v1:
> >> - phy_timing2 and phy_timing3 refer clock cycle time.
> >> - define values of LPX HS_PRPR HS_ZERO HS_TRAIL TA_GO TA_SURE TA_GET DA_HS_EXIT.
> >> ---
> >>
> >
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [v3,2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver
From: Christopher Spinrath @ 2016-11-18 5:00 UTC (permalink / raw)
To: Jyri Sarha, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: bcousson-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w,
bgolaszewski-rdvid1DuHRBWk0Htik3J/w, tomi.valkeinen-l0cyMroinI0,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, Spinrath, Christopher
In-Reply-To: <6eaa1b5d467cd354f7d5dc5ff8da1b9bf732ff11.1479388871.git.jsarha-l0cyMroinI0@public.gmane.org>
Hi Jyri,
On 11/17/2016 02:28 PM, Jyri Sarha wrote:
> Add very basic ti-ftp410 DVI transmitter driver. The only feature
s/ftp/tfp ?
> separating this from a completely dummy bridge is the EDID read
> support trough DDC I2C. Even that functionality should be in a
> separate generic connector driver. However, because of missing DRM
> infrastructure support the connector is implemented within the bridge
> driver. Some tfp410 HW specific features may be added later if needed,
> because there is a set of registers behind i2c if it is connected.
>
> This implementation is tested against my new tilcdc bridge support
> and it works with BeagleBone DVI-D Cape Rev A3. A DT binding document
> is also added.
>
> Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
Thanks for working on this. I have tested the driver on my Utilite Pro
which is based on the imx6q SoC and has a tfp410 chip hooked up to its
parallel rgb24 interface. So you can add my
Tested-By: Christopher Spinrath <christopher.spinrath-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
However, I have two more comments below.
> ---
> .../bindings/display/bridge/ti,tfp410.txt | 40 +++
> drivers/gpu/drm/bridge/Kconfig | 7 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/ti-tfp410.c | 287 +++++++++++++++++++++
> 4 files changed, 335 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> new file mode 100644
> index 0000000..6174b18
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
There already is a binding documentation for the tfp410 in
Documentation/devicetree/bindings/display/ti/ti,tfp410.txt .
It is used by the omap drm driver. IMHO you should extend, move or
deprecate that one. Note that it describes an optional powerdown-gpios
property.
> @@ -0,0 +1,40 @@
> +TFP410 DVI bridge bindings
> +
> +Required properties:
> + - compatible: "ti,tfp410"
> +
> +Optional properties
> + - reg: I2C address. If and only if present the device node
> + should be placed into the i2c controller node where the
> + tfp410 i2c is connected to.
> +
> +Required subnodes:
> + - port@0: Video input port node to connect the bridge to a
> + display controller output [1].
> + - port@1: Video output port node to connect the bridge to a
> + connector input [1].
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> + hdmi-bridge {
> + compatible = "ti,tfp410";
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + bridge_in: endpoint {
> + remote-endpoint = <&dc_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + bridge_out: endpoint {
> + remote-endpoint = <&hdmi_in>;
> + };
> + };
> + };
> + };
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index bd6acc8..a424e03 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -81,6 +81,13 @@ config DRM_TOSHIBA_TC358767
> ---help---
> Toshiba TC358767 eDP bridge chip driver.
>
> +config DRM_TI_TFP410
> + tristate "TI TFP410 DVI/HDMI bridge"
> + depends on OF
> + select DRM_KMS_HELPER
> + ---help---
> + Texas Instruments TFP410 DVI/HDMI Transmitter driver
> +
> source "drivers/gpu/drm/bridge/analogix/Kconfig"
>
> source "drivers/gpu/drm/bridge/adv7511/Kconfig"
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 97ed1a5..8b065d9 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_DRM_SII902X) += sii902x.o
> obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> +obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> new file mode 100644
> index 0000000..64f54e4
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> @@ -0,0 +1,287 @@
> +/*
> + * Copyright (C) 2016 Texas Instruments
> + * Author: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +struct tfp410 {
> + struct drm_bridge bridge;
> + struct drm_connector connector;
> +
> + struct i2c_adapter *ddc;
> +
> + struct device *dev;
> +};
> +
> +static inline struct tfp410 *
> +drm_bridge_to_tfp410(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct tfp410, bridge);
> +}
> +
> +static inline struct tfp410 *
> +drm_connector_to_tfp410(struct drm_connector *connector)
> +{
> + return container_of(connector, struct tfp410, connector);
> +}
> +
> +static int tfp410_get_modes(struct drm_connector *connector)
> +{
> + struct tfp410 *dvi = drm_connector_to_tfp410(connector);
> + struct edid *edid;
> + int ret;
> +
> + if (!dvi->ddc)
> + goto fallback;
> +
> + edid = drm_get_edid(connector, dvi->ddc);
> + if (!edid) {
> + DRM_INFO("EDID read failed. Fallback to standard modes\n");
> + goto fallback;
> + }
> +
> + drm_mode_connector_update_edid_property(connector, edid);
> +
> + return drm_add_edid_modes(connector, edid);
> +fallback:
> + /* No EDID, fallback on the XGA standard modes */
> + ret = drm_add_modes_noedid(connector, 1920, 1200);
> +
> + /* And prefer a mode pretty much anything can handle */
> + drm_set_preferred_mode(connector, 1024, 768);
> +
> + return ret;
> +}
> +
> +static const struct drm_connector_helper_funcs tfp410_con_helper_funcs = {
> + .get_modes = tfp410_get_modes,
> +};
> +
> +static enum drm_connector_status
> +tfp410_connector_detect(struct drm_connector *connector, bool force)
> +{
> + struct tfp410 *dvi = drm_connector_to_tfp410(connector);
> +
> + if (dvi->ddc) {
> + if (drm_probe_ddc(dvi->ddc))
> + return connector_status_connected;
> + else
> + return connector_status_disconnected;
> + }
I wonder what happens if a display with none or defect ddc/edid is
attached? The dumb-vga-dac bridge driver reports
connector_status_unknown in the case drm_probe_ddc fails.
Cheers,
Christopher
> +
> + return connector_status_unknown;
> +}
> +
> +static const struct drm_connector_funcs tfp410_con_funcs = {
> + .dpms = drm_atomic_helper_connector_dpms,
> + .detect = tfp410_connector_detect,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .destroy = drm_connector_cleanup,
> + .reset = drm_atomic_helper_connector_reset,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int tfp410_attach(struct drm_bridge *bridge)
> +{
> + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> + int ret;
> +
> + if (!bridge->encoder) {
> + dev_err(dvi->dev, "Missing encoder\n");
> + return -ENODEV;
> + }
> +
> + drm_connector_helper_add(&dvi->connector,
> + &tfp410_con_helper_funcs);
> + ret = drm_connector_init(bridge->dev, &dvi->connector,
> + &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
> + if (ret) {
> + dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
> + return ret;
> + }
> +
> + drm_mode_connector_attach_encoder(&dvi->connector,
> + bridge->encoder);
> +
> + return 0;
> +}
> +
> +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
> + .attach = tfp410_attach,
> +};
> +
> +static int tfp410_get_connector_ddc(struct tfp410 *dvi)
> +{
> + struct device_node *ep = NULL, *connector_node = NULL;
> + struct device_node *ddc_phandle = NULL;
> + int ret = 0;
> +
> + /* port@1 is the connector node */
> + ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 1, -1);
> + if (!ep)
> + goto fail;
> +
> + connector_node = of_graph_get_remote_port_parent(ep);
> + if (!connector_node)
> + goto fail;
> +
> + ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
> + if (!ddc_phandle)
> + goto fail;
> +
> + dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
> + if (dvi->ddc)
> + dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
> + else
> + ret = -EPROBE_DEFER;
> +
> +fail:
> + of_node_put(ep);
> + of_node_put(connector_node);
> + of_node_put(ddc_phandle);
> + return ret;
> +}
> +
> +static int tfp410_init(struct device *dev)
> +{
> + struct tfp410 *dvi;
> + int ret;
> +
> + if (!dev->of_node) {
> + dev_err(dev, "device-tree data is missing\n");
> + return -ENXIO;
> + }
> +
> + dvi = devm_kzalloc(dev, sizeof(*dvi), GFP_KERNEL);
> + if (!dvi)
> + return -ENOMEM;
> + dev_set_drvdata(dev, dvi);
> +
> + dvi->bridge.funcs = &tfp410_bridge_funcs;
> + dvi->bridge.of_node = dev->of_node;
> + dvi->dev = dev;
> +
> + ret = tfp410_get_connector_ddc(dvi);
> + if (ret)
> + goto fail;
> +
> + ret = drm_bridge_add(&dvi->bridge);
> + if (ret) {
> + dev_err(dev, "drm_bridge_add() failed: %d\n", ret);
> + goto fail;
> + }
> +
> + return 0;
> +fail:
> + i2c_put_adapter(dvi->ddc);
> + return ret;
> +}
> +
> +static int tfp410_fini(struct device *dev)
> +{
> + struct tfp410 *dvi = dev_get_drvdata(dev);
> +
> + drm_bridge_remove(&dvi->bridge);
> +
> + if (dvi->ddc)
> + i2c_put_adapter(dvi->ddc);
> +
> + return 0;
> +}
> +
> +static int tfp410_probe(struct platform_device *pdev)
> +{
> + return tfp410_init(&pdev->dev);
> +}
> +
> +static int tfp410_remove(struct platform_device *pdev)
> +{
> + return tfp410_fini(&pdev->dev);
> +}
> +
> +/* There is currently no i2c functionality. */
> +static int tfp410_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int reg;
> +
> + if (!client->dev.of_node ||
> + of_property_read_u32(client->dev.of_node, "reg", ®)) {
> + dev_err(&client->dev,
> + "Can't get i2c reg property from device-tree\n");
> + return -ENXIO;
> + }
> +
> + return tfp410_init(&client->dev);
> +}
> +
> +static int tfp410_i2c_remove(struct i2c_client *client)
> +{
> + return tfp410_fini(&client->dev);
> +}
> +
> +static const struct of_device_id tfp410_match[] = {
> + { .compatible = "ti,tfp410" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, tfp410_match);
> +
> +struct platform_driver tfp410_platform_driver = {
> + .probe = tfp410_probe,
> + .remove = tfp410_remove,
> + .driver = {
> + .name = "tfp410-bridge",
> + .of_match_table = tfp410_match,
> + },
> +};
> +
> +static const struct i2c_device_id tfp410_i2c_ids[] = {
> + { "tfp410", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, tfp410_i2c_ids);
> +
> +static struct i2c_driver tfp410_i2c_driver = {
> + .driver = {
> + .name = "tfp410",
> + .of_match_table = of_match_ptr(tfp410_match),
> + },
> + .id_table = tfp410_i2c_ids,
> + .probe = tfp410_i2c_probe,
> + .remove = tfp410_i2c_remove,
> +};
> +
> +static int __init tfp410_module_init(void)
> +{
> + i2c_add_driver(&tfp410_i2c_driver);
> + platform_driver_register(&tfp410_platform_driver);
> +
> + return 0;
> +}
> +module_init(tfp410_module_init);
> +
> +static void __exit tfp410_module_exit(void)
> +{
> + i2c_del_driver(&tfp410_i2c_driver);
> + platform_driver_unregister(&tfp410_platform_driver);
> +}
> +module_exit(tfp410_module_exit);
> +
> +MODULE_AUTHOR("Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>");
> +MODULE_DESCRIPTION("TI TFP410 DVI bridge driver");
> +MODULE_LICENSE("GPL");
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox