* [PATCH] ARM: dts: vfxxx: Enable DMA for DSPI2 and DSPI3
From: Sanchayan Maity @ 2016-11-10 11:45 UTC (permalink / raw)
To: shawnguo
Cc: stefan, linux-arm-kernel, devicetree, linux-kernel,
Sanchayan Maity
Enable DMA for DSPI2 and DSPI3 on Vybrid.
---
arch/arm/boot/dts/vfxxx.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index 000550f..e9d2847 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -573,6 +573,9 @@
clocks = <&clks VF610_CLK_DSPI2>;
clock-names = "dspi";
spi-num-chipselects = <2>;
+ dmas = <&edma1 0 10>,
+ <&edma1 0 11>;
+ dma-names = "rx", "tx";
status = "disabled";
};
@@ -585,6 +588,9 @@
clocks = <&clks VF610_CLK_DSPI3>;
clock-names = "dspi";
spi-num-chipselects = <2>;
+ dmas = <&edma1 0 12>,
+ <&edma1 0 13>;
+ dma-names = "rx", "tx";
status = "disabled";
};
--
2.10.2
^ permalink raw reply related
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Ziji Hu @ 2016-11-10 11:44 UTC (permalink / raw)
To: Rob Herring, Gregory CLEMENT
Cc: Hilbert Zhang, Andrew Lunn, Ulf Hansson, Romain Perier,
Liuliu Zhao, Peng Zhu, linux-kernel, Nadav Haklai, Jack(SH) Zhu,
Victor Gu, Doug Jones, Jisheng Zhang, Yehuda Yitschak,
Marcin Wojtas, Xueping Liu, Shiwu Zhang, Yu Cao,
Sebastian Hesselbarth, devicetree, Jason Cooper, Hanna Hawa,
Kostya Porotchkin, Ryan Gao, Wei(SOCP) Liu, linux-arm-kernel
In-Reply-To: <20161109182426.vfrpb4i2mfatdzz3@rob-hp-laptop>
Hi Rob,
On 2016/11/10 2:24, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 12:09:54PM +0100, Gregory CLEMENT wrote:
>> From: Ziji Hu <huziji@marvell.com>
>>
>> Marvell Xenon SDHC can support eMMC/SD/SDIO.
>> Add Xenon-specific properties.
>> Also add properties for Xenon PHY setting.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt | 161 +++++++-
>> MAINTAINERS | 1 +-
>> 2 files changed, 162 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>> new file mode 100644
>> index 000000000000..0d2d139494d3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>> @@ -0,0 +1,161 @@
>> +Marvell's Xenon SDHCI Controller device tree bindings
>> +This file documents differences between the core mmc properties
>> +described by mmc.txt and the properties used by the Xenon implementation.
>> +
>> +A single Xenon IP can support multiple slots.
>> +Each slot acts as an independent SDHC. It owns independent resources, such
>> +as register sets clock and PHY.
>> +Each slot should have an independent device tree node.
>> +
>> +Required Properties:
>> +- compatible: should be one of the following
>> + - "marvell,armada-3700-sdhci": For controllers on Armada-3700 SOC.
>> + Must provide a second register area and marvell,pad-type.
>> + - "marvell,xenon-sdhci": For controllers on all the SOCs, other than
>> + Armada-3700.
>
> Need SoC specific compatible strings.
>
Xenon SDHC is a common IP for all Marvell SOCs.
It is difficult to use a single SOC specific compatible to represent Xenon SDHC.
There will be so many SOC compatible strings in list if each specific SOC owns a compatible.
Actually only few SOCs require special properties.
Any suggestion please?
>> +
>> +- clocks:
>> + Array of clocks required for SDHCI.
>> + Requires at least one for Xenon IP core.
>> + Some SOCs require additional clock for AXI bus.
>> +
>> +- clock-names:
>> + Array of names corresponding to clocks property.
>> + The input clock for Xenon IP core should be named as "core".
>> + The optional AXI clock should be named as "axi".
>
> When is AXI clock optional? This should be required for ?? compatible
> strings.
>
It is required on some SOCs.
I will double check if a suitable compatible string can be determined for those SOCs.
>> +
>> +- reg:
>> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
>> +
>> + * For "marvell,armada-3700-sdhci", two register areas.
>> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
>> + PHY PAD Voltage Control register.
>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> + in below.
>> + Please also check property marvell,pad-type in below.
>> +
>> +Optional Properties:
>> +- marvell,xenon-slotno:
>
> Multiple slots should be represented as child nodes IMO. I think some
> other bindings already do this.
>
All the slots are entirely independent.
I prefer to consider it as multiple independent SDHCs placed in a single IP, instead of that a IP contains multiple child slots.
It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
In my very own opinion, it is inconvenient and unnecessary.
>> + Indicate the corresponding bit index of current Xenon SDHC slot in
>> + SDHC System Operation Control Register Bit[7:0].
>> + Set/clear the corresponding bit to enable/disable current Xenon SDHC
>> + slot.
>> + If this property is not provided, Xenon IP should contain only one
>> + slot.
>> +
>> +- marvell,xenon-phy-type:
>> + Xenon support mutilple types of PHYs.
>> + To select eMMC 5.1 PHY, set:
>> + marvell,xenon-phy-type = "emmc 5.1 phy"
>> + eMMC 5.1 PHY is the default choice if this property is not provided.
>> + To select eMMC 5.0 PHY, set:
>> + marvell,xenon-phy-type = "emmc 5.0 phy"
>> + To select SDH PHY, set:
>> + marvell,xenon-phy-type = "sdh phy"
>> + Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
>> + eMMC only.
>
> Does this vary per instance on a single SoC? If not, then an SoC
> specific compatible should determine this.
>
> Also, the " phy" part is redundant.
>
Yes. Some SOCs might have multiple Xenon PHY types.
This property is only the name/type of PHY. It doesn't stand for the entire SDHC property.
"emmc 5.1 PHY" doesn't mean that this Xenon SDHC only support eMMC 5.1.
Xenon SDHC with "sdh PHY" can also support eMMC.
>> +
>> +- marvell,xenon-phy-znr:
>> + Set PHY ZNR value.
>> + Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
>> + valid range = [0:0x1F].
>> + ZNR is set as 0xF by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-zpr:
>> + Set PHY ZPR value.
>> + Only available for eMMC PHY 5.1 and eMMC PHY 5.0.
>> + valid range = [0:0x1F].
>> + ZPR is set as 0xF by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-nr-success-tun:
>> + Set the number of required consecutive successful sampling points used to
>> + identify a valid sampling window, in tuning process.
>> + Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-tun-step-divider:
>> + Set the divider for calculating TUN_STEP.
>> + Set as 64 by default if this property is not provided.
>> +
>> +- marvell,xenon-phy-slow-mode:
>> + Force PHY into slow mode.
>> + Only available when bus frequency lower than 50MHz in SDR mde.
>> + Disabled by default. Please do not enable it unless it is necessary.
>> +
>> +- marvell,xenon-mask-conflict-err:
>> + Mask Conflict Error alert on some SOC. Disabled by default.
>> +
>> +- marvell,xenon-tun-count:
>> + Xenon SDHC SOC usually doesn't provide re-tuning counter in
>> + Capabilities Register 3 Bit[11:8].
>> + This property provides the re-tuning counter.
>> + If this property is not set, default re-tuning counter will
>> + be set as 0x9 in driver.
>> +
>> +- marvell,pad-type:
>> + Type of Armada 3700 SOC PHY PAD Voltiage Controller register.
>> + Only valid when "marvell,armada-3700-sdhci" is selected.
>> + Two types: "sd" and "fixed-1-8v".
>> + If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
>> + switched to 1.8V when SD in UHS-I.
>> + If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>> + in below.
>> +
>> +Example:
>> +- For eMMC slot:
>> +
>> + sdhci@aa0000 {
>> + compatible = "marvell,xenon-sdhci";
>> + reg = <0xaa0000 0x1000>;
>> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> + clocks = <&emmc_clk>, <&axi_clock>;
>> + clock-names = "core", "axi";
>> + bus-width = <8>;
>> + marvell,xenon-emmc;
>
> Not documented. If we need to specify the type of slot/card, then we
> need to come up with a standard property. This was either already done
> or attempted IIRC.
Sorry to lost this property in above.
I will add it in above.
Thank you.
Best regards,
Hu Ziji
>
>> + marvell,xenon-slotno = <0>;
>> + marvell,xenon-phy-type = "emmc 5.1 phy";
>> + marvell,xenon-tun-count = <11>;
>> + };
>> +
>> +- For SD/SDIO slot:
>> +
>> + sdhci@ab0000 {
>> + compatible = "marvell,xenon-sdhci";
>> + reg = <0xab0000 0x1000>;
>> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> + vqmmc-supply = <&sd_regulator>;
>> + clocks = <&sdclk>;
>> + clock-names = "core";
>> + bus-width = <4>;
>> + marvell,xenon-tun-count = <9>;
>> + };
>> +
>> +- For eMMC slot with compatible "marvell,armada-3700-sdhci":
>> +
>> + sdhci@aa0000 {
>> + compatible = "marvell,armada-3700-sdhci";
>> + reg = <0xaa0000 0x1000>,
>> + <phy_addr 0x4>;
>> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
>> + clocks = <&emmcclk>;
>> + clock-names = "core";
>> + bus-width = <8>;
>> + marvell,xenon-emmc;
>> +
>> + marvell,pad-type = "fixed-1-8v";
>> + };
>> +
>> +- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
>> +
>> + sdhci@ab0000 {
>> + compatible = "marvell,armada-3700-sdhci";
>> + reg = <0xab0000 0x1000>,
>> + <phy_addr 0x4>;
>> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
>> + vqmmc-supply = <&sd_regulator>;
>> + clocks = <&sdclk>;
>> + clock-names = "core";
>> + bus-width = <4>;
>> +
>> + marvell,pad-type = "sd";
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1a5c4c30ea24..850a0afb0c8d 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
>> M: Ziji Hu <huziji@marvell.com>
>> L: linux-mmc@vger.kernel.org
>> S: Supported
>> +F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>>
>> MATROX FRAMEBUFFER DRIVER
>> L: linux-fbdev@vger.kernel.org
>> --
>> git-series 0.8.10
^ permalink raw reply
* Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus
From: Arnd Bergmann @ 2016-11-10 11:37 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linuxppc-dev@lists.ozlabs.org, Geert Uytterhoeven,
Greg Kroah-Hartman, Yangbo Lu, Simon Horman, Magnus Damm,
Rob Herring, Mark Rutland, devicetree@vger.kernel.org, Dirk Behme,
linux-kernel@vger.kernel.org, Linux-Renesas,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAMuHMdX+T=Y--F6KXZwFmZwWFdfP5mL5Tar-wKwRM2p6Hh2-sg@mail.gmail.com>
On Thursday, November 10, 2016 11:19:20 AM CET Geert Uytterhoeven wrote:
> Hi Arnd,
>
> Thanks for your comments!
>
> On Wed, Nov 9, 2016 at 5:55 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:
> >> v2:
> >> - Drop SoC families and family names; use fixed "Renesas" instead,
> >
> > I think I'd rather have seen the family names left in there, but it's
> > not important, so up to you.
>
> They're not useful for matching, as family names may change anytime, and don't
> always say much about the hardware capabilities.
> E.g. SH-Mobile -> R-Mobile -> R-Car | RZ/A | RZ/G
> Some SH-Mobile (even some R-Car) parts are SuperH only, others have ARM and
> SuperH.
>
> At least the SoC part numbers are stable (hmm, sh73a0 == r8a73a0).
I think the marketing names are much more useful for humans looking
at the sysfs files than the kernel doing matching on, but both use
cases are important.
> >> - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
> >> available, else fall back to hardcoded addresses for compatibility
> >> with existing DTBs,
> >
> > I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
> > binding for these, among other things.
>
> I understand you've received them in the mean time?
Yes, I found them in my inbox later, not sure why I didn't see them
at first.
> > It does seem wrong to have a device node for a specific register though.
> > Shouldn't the node be for the block of registers that these are inside
> > of?
>
> On R-Mobile APE6, R-Car Gen2 and Gen3, PRR is a lone register.
> On R-Car Gen1, it's not even documented (and doesn't exist on all parts).
It just seems odd to have it at address 0xff000044 when all the other
devices are at page-aligned addresses. Do you mean that accessing
0xff000040 or 0xff000048 will result in a bus-level exception for a
missing register and just 0xff000044 is actually valid for access,
or is it just the only thing that is documented?
> On SH-Mobile/R-Mobile, CCCR may be part of the HPB/APB register block, which
> we further don't touch at all.
> On R-Car Gen2, it's not documented, but does exist.
This is where the family names would come in handy ;-) I now have
no idea which chip(s) you are referring to.
If you know the name of the register block, just put it into DT with
that name. The driver can trivially add the right offset.
> >> - Don't register the SoC bus if the chip ID register is missing,
> >
> > Why? My objection was to hardcoding the register, not to registering
> > the device? I think I'd rather see the device registered with an
> > empty revision string.
>
> If there's no chip ID register, there's no reason to use soc_device_match(),
> as we can always look at a compatible value. All SoCs listed in this driver
> have a chip ID register.
But you may still have user space tools looking into sysfs, e.g. to
figure out how to install a kernel that the boot loader can find,
or which hardware specific distro packages to install.
> if you want me to register the soc_bus for those SoCs regardless, I want to
> re-add r7s72100 (RZ/A) and r8a7778 (R-Car M1A), who don't have chip ID
> registers ;-)
Right. Just don't encode too much knowledge about the SoCs into the
driver, so we are prepared for adding new ones: We should still look
for the registers in DT on all chips.
> >> +#define CCCR 0xe600101c /* Common Chip Code Register */
> >> +#define PRR 0xff000044 /* Product Register */
> >> +#define PRR3 0xfff00044 /* Product Register on R-Car Gen3 */
> >> +
> >> +static const struct of_device_id renesas_socs[] __initconst = {
> >> +#ifdef CONFIG_ARCH_R8A73A4
> >> + { .compatible = "renesas,r8a73a4", .data = (void *)PRR, },
> >> +#endif
> >> +#ifdef CONFIG_ARCH_R8A7740
> >> + { .compatible = "renesas,r8a7740", .data = (void *)CCCR, },
> >> +#endif
> >
> > My preference here would be to list the register address only for
> > SoCs that are known to need them, while also having .dtb files that
> > don't have the nodes.
>
> Even if drivers don't have to handle differences, there's been a long
> outstanding request to print SoC revision information during bootup
> (E.g. "Does it still work on ES1.0?"). Hence that covers all SoCs.
Ok, fair enough.
> >> +static int __init renesas_soc_init(void)
> >> +{
> >> + struct soc_device_attribute *soc_dev_attr;
> >> + const struct of_device_id *match;
> >> + void __iomem *chipid = NULL;
> >> + struct soc_device *soc_dev;
> >> + struct device_node *np;
> >> + unsigned int product;
> >> +
> >> + np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
> >> + if (!np)
> >> + return -ENODEV;
> >> +
> >> + of_node_put(np);
> >> +
> >> + /* Try PRR first, then CCCR, then hardcoded fallback */
> >> + np = of_find_compatible_node(NULL, NULL, "renesas,prr");
> >> + if (!np)
> >> + np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
> >> + if (np) {
> >> + chipid = of_iomap(np, 0);
> >> + of_node_put(np);
> >> + } else if (match->data) {
> >> + chipid = ioremap((uintptr_t)match->data, 4);
> >> + }
> >> + if (!chipid)
> >>
> >
> > Here, I'd turn the order around and look for the DT nodes of the
> > devices first. Only if they are not found, look at the compatible
> > string of the root node. No need to search for a node though,
> > you know which one it is when you look for a compatible =
> > "renesas,r8a73a4".
>
> "renesas,r8a73a4" is the root node, not the device, so it does not have the
> "reg" property for reading the chip ID?
I mean replace of_find_matching_node_and_match() with
of_match_node(renesas_socs, of_root).
It does the same thing, just more efficiently.
> There is no SoC part number in the "renesas,prr" and "renesas,cccr" nodes.
> Hence I always need to look at the root nodes.
Not sure what that would protect you from. Could you have a renesas,cccr
Arnd
^ permalink raw reply
* Re: [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Mark Rutland @ 2016-11-10 11:22 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: zhichang.yuan, catalin.marinas, will.deacon, robh+dt, bhelgaas,
olof, arnd, linux-arm-kernel, lorenzo.pieralisi, linux-kernel,
linuxarm, devicetree, linux-pci, linux-serial, minyard,
liviu.dudau, zourongrong, john.garry, gabriele.paoloni,
zhichang.yuan02, kantyzc, xuwei5, marc.zyngier
In-Reply-To: <1478647002.7430.69.camel@kernel.crashing.org>
On Wed, Nov 09, 2016 at 10:16:42AM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2016-11-08 at 12:03 +0000, Mark Rutland wrote:
> > On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> > >
> > > For arm64, there is no I/O space as other architectural platforms, such as
> > > X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
> > > such as Hip06, when accessing some legacy ISA devices connected to LPC, those
> > > known port addresses are used to control the corresponding target devices, for
> > > example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
> > > normal MMIO mode in using.
> >
> > This has nothing to do with arm64. Hardware with this kind of indirect
> > bus access could be integrated with a variety of CPU architectures. It
> > simply hasn't been, yet.
>
> On some ppc's we also use similar indirect access methods for IOs. We
> have a generic infrastructure for re-routing some memory or IO regions
> to hooks.
>
> On POWER8, our PCIe doesn't do IO at all, but we have an LPC bus behind
> firmware calls ;-) We use that infrastructure to plumb in the LPC bus.
Just to check, do you hook that in your inb/outb/etc?
Generally, it would seem nicer if we could have higher-level
isa_{inb,outb,whatever} accessors that we could hook separately from
other IO.
We don't necessarily have to move all ISA drivers over to that if we had
a separate symbol for that interface.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v16 0/5] Mediatek MT8173 CMDQ support
From: Horng-Shyang Liao @ 2016-11-10 11:15 UTC (permalink / raw)
To: Rob Herring, Jassi Brar, Matthias Brugger
Cc: Daniel Kurtz, Sascha Hauer, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Sascha Hauer,
Philipp Zabel, Nicolas Boichat, CK HU, cawa cheng, Bibby Hsieh,
YT Shen, Daoyuan Huang, Damon Chu, Josh-YC Liu, Glory Hung,
Jiaguang Zhang, Dennis-YC Hsieh
In-Reply-To: <1477999698-6288-1-git-send-email-hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
On Tue, 2016-11-01 at 19:28 +0800, HS Liao wrote:
> Hi,
>
> This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used
> to help write registers with critical time limitation, such as
> updating display configuration during the vblank. It controls Global
> Command Engine (GCE) hardware to achieve this requirement.
>
> These patches have a build dependency on top of v4.9-rc1.
>
> Changes since v15:
> - separate "suspend and resume" patch from "save energy" patch
> - don't stop running tasks in cmdq_suspend()
> (i.e. leave no running tasks guarantee to clients)
>
> Best regards,
> HS Liao
>
> HS Liao (5):
> dt-bindings: soc: Add documentation for the MediaTek GCE unit
> CMDQ: Mediatek CMDQ driver
> arm64: dts: mt8173: Add GCE node
> CMDQ: suspend and resume
> CMDQ: save energy
>
> .../devicetree/bindings/mailbox/mtk-gce.txt | 43 ++
> arch/arm64/boot/dts/mediatek/mt8173.dtsi | 10 +
> drivers/mailbox/Kconfig | 10 +
> drivers/mailbox/Makefile | 2 +
> drivers/mailbox/mtk-cmdq-mailbox.c | 632 +++++++++++++++++++++
> drivers/soc/mediatek/Kconfig | 11 +
> drivers/soc/mediatek/Makefile | 1 +
> drivers/soc/mediatek/mtk-cmdq-helper.c | 310 ++++++++++
> include/linux/mailbox/mtk-cmdq-mailbox.h | 67 +++
> include/linux/soc/mediatek/mtk-cmdq.h | 182 ++++++
> 10 files changed, 1268 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c
> create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
> create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h
> create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h
>
Hi Jassi, Matthias,
Sorry to disturb you.
Do you have any further comment on CMDQ v16?
Thanks.
HS
--
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 0/7] soc: renesas: Identify SoC and register with the SoC bus
From: Ulf Hansson @ 2016-11-10 10:59 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linuxppc-dev@lists.ozlabs.org, Mark Rutland,
devicetree@vger.kernel.org, Dirk Behme, Greg Kroah-Hartman,
linux-kernel@vger.kernel.org, Rob Herring, Linux-Renesas,
linux-samsung-soc@vger.kernel.org, Simon Horman, Yangbo Lu,
Pankaj Dubey, linux-arm-kernel@lists.infradead.org, Arnd Bergmann
In-Reply-To: <CAMuHMdWox5ksj1xhJ6_n0wNA9j6veX0AzAE0V9Gs26Kvw5C_sw@mail.gmail.com>
On 10 November 2016 at 10:22, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Ulf,
>
> On Wed, Nov 9, 2016 at 10:12 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Wednesday, November 9, 2016 6:19:06 PM CET Geert Uytterhoeven wrote:
>>> On Wed, Nov 9, 2016 at 5:56 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> > On Wednesday, November 9, 2016 2:34:33 PM CET Geert Uytterhoeven wrote:
>>> >> > And Samsung.
>>> >> > Shall I create the immutable branch now?
>>> >>
>>> >> Arnd: are you happy with the new patches and changes?
>>> >
>>> > I still had some comments for patch 7, but that shouldn't stop
>>> > you from creating a branch for the first three so everyone can
>>> > build on top of that.
>>>
>>> Thanks!
>>>
>>> What about patch [4/7]?
>>> Haven't you received it? Your address was in the To-line for all 7 patches.
>>
>> Ok, I see it now, looks good. That should be included as well then.
>
> Thanks, I've created the branch/tag :
>
> git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
> branch soc-device-match
> signed tag soc-device-match-tag1
>
> In the mean time, Ulf has applied the first two patches to mmc/next, on top
> of lots of MMC work :-(
No worries! :-)
>
> Ulf, as this is not only a dependency for Freescale/NXP (for sdhci-of-esdhc),
> but also for Samsung and Renesas, would it still be possible to replace these
> two commits
>
> 8b82c17a8ae533d6 base: soc: introduce soc_device_match() interface
> 6fa350172b098f0f base: soc: Check for NULL SoC device attributes
>
> by a merge of soc-device-match-tag1?
Yes, I will take care of it during the day.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v2 0/7] soc: renesas: Identify SoC and register with the SoC bus
From: Geert Uytterhoeven @ 2016-11-10 10:56 UTC (permalink / raw)
To: Ulf Hansson
Cc: linuxppc-dev@lists.ozlabs.org, Mark Rutland,
devicetree@vger.kernel.org, Dirk Behme, Greg Kroah-Hartman,
linux-kernel@vger.kernel.org, Rob Herring, Linux-Renesas,
linux-samsung-soc@vger.kernel.org, Simon Horman, Yangbo Lu,
Pankaj Dubey, linux-arm-kernel@lists.infradead.org, Arnd Bergmann
In-Reply-To: <CAMuHMdWox5ksj1xhJ6_n0wNA9j6veX0AzAE0V9Gs26Kvw5C_sw@mail.gmail.com>
On Thu, Nov 10, 2016 at 10:22 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Thanks, I've created the branch/tag :
>
> git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
> branch soc-device-match
> signed tag soc-device-match-tag1
Tested by kbuild test robot:
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
soc-device-match
da65a1589dacc7ec44ea0557a14d70a39d991f32 base: soc: Provide a
dummy implementation of soc_device_match()
elapsed time: 101m
configs tested: 85
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 04/14] ARM: dts: armada-375: Fixup bootrom DT warning
From: Gregory CLEMENT @ 2016-11-10 10:43 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: <20161110111506.02842cbd-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Thomas,
On jeu., nov. 10 2016, Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
[...]
>> > A good example of why I'm worried is the sa-sram case:
>> >
>> > + crypto_sram0: sa-sram0@0 {
>> > compatible = "mmio-sram";
>> > reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
>> >
>> > + crypto_sram1: sa-sram1@0 {
>> > compatible = "mmio-sram";
>> > reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
>> >
>> > The node names should be just "sram" without a number. Indeed for UARTs
>> > for example, you use uart@XYZ, uart@ABC and not uart0@XYZ and
>> > uart1@ABC. But then, if you do that, with your scheme, you end up with
>> > both nodes named sa-sram@0.
>> >
>> > Which clearly shows that the way you set this unit-address is not
>> > correct: those two devices are mapped at completely different
>> > locations, but you end up with an identical unit address.
>> >
>> > I have no idea what is the rule for setting the unit address in this
>> > case, but I'm pretty sure the rule you've chosen is not good.
>>
>> I don't know if there is an existing rules for this case. But I see your
>> concern. What I propose then is to expose the memory windows ID by
>> adding the target and the attributes like this:
>>
>> crypto_sram0: sa-sram@09_09_0 {
>> compatible = "mmio-sram";
>> reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
>>
>>
>> crypto_sram1: sa-sram@09_05_0 {
>> compatible = "mmio-sram";
>> reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
>
> I have no idea if 09_05_0 is considered a valid unit address. Indeed
> the _ character in an address looks a bit weird.
>
> I guess we need guidance from the DT binding maintainers on this, since
> it's really a matter of interpreting what "unit address" means in
> relation to the value of the "reg" property.
So I looked for in the reference: Power_ePAPR_APPROVED_v1.0, and in
paragraph "2.2.1.1 Node Name Requirements" we have:
"The unit-address component of the name is specific to the bus type on
which the node sits. It consists of one or more ASCII characters from
the set of characters in Table 2-1. The fundamental requirement is that
at any level of the device tree the unit-address be unique in order to
differentiate nodes with the same name at the same level in the
tree. The binding for a particular bus may specify additional, more
specific requirements for the format of a unit-address."
In Table 2-1 we have the following characters:
0-9
a-z
A-z
,
.
_
+
-
So the underscore is valid. And by adding information about the memory
windows we match the fundamental requirement :"at any level of the
device tree the unit-address be unique".
Gregory
> --
> 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 v2 3/3] clocksource: Add clockevent support to NPS400 driver
From: Daniel Lezcano @ 2016-11-10 10:34 UTC (permalink / raw)
To: Noam Camus
Cc: robh+dt@kernel.org, mark.rutland@arm.com, tglx@linutronix.de,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Vineet Gupta
In-Reply-To: <DB6PR0501MB25184EFB9A5CFD3AA41CFDC1AAA60@DB6PR0501MB2518.eurprd05.prod.outlook.com>
On 08/11/2016 09:30, Noam Camus wrote:
>> From: Daniel Lezcano [mailto:daniel.lezcano@linaro.org]
>> Sent: Tuesday, November 1, 2016 10:02 PM
> ...
>> Assuming cpu0 and cpu1 are sibling, does
>
>> taskset 0x1 time sleep 2 & taskset 0x2 time sleep 3
>
> I will use 16,17 instead of 0,1
>> give a correct result without a dmesg log ?
> [root@192.168.8.2 /]$ [root@192.168.8.2 /]$ taskset 65536 time sleep 2 & taskset 131072 time sleep 3
Thanks for providing the numbers.
So there is no weird messages in dmesg ?
> real 0m 2.54s
> user 0m 0.04s
> sys 0m 0.14s
> real 0m 3.47s
> user 0m 0.00s
> sys 0m 0.15s
> [1]+ Done taskset 65536 time sleep 2
>
> Seem OK to me.
I'm not sure of that.
2.54 instead of 2
3.47 instead of 3
0.02 time drift could be acceptable but 0.54 or 0.47 is too much.
And 0.15 in sys also a big number AFAICT.
Is the system in NO_HZ ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Sudeep Holla @ 2016-11-10 10:26 UTC (permalink / raw)
To: Rob Herring
Cc: Sudeep Holla, Neil Armstrong, Olof Johansson,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161110012249.ed56ik6kdffoikym@rob-hp-laptop>
On 10/11/16 01:22, Rob Herring wrote:
> On Wed, Nov 02, 2016 at 10:52:09PM -0600, Sudeep Holla wrote:
>> This patch adds specific compatible to support legacy SCPI protocol.
>>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/arm/arm,scpi.txt | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> index d1882c4540d0..ebd03fc93135 100644
>> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> @@ -7,7 +7,9 @@ by Linux to initiate various system control and power operations.
>>
>> Required properties:
>>
>> -- compatible : should be "arm,scpi"
>> +- compatible : should be
>> + * "arm,scpi" : For implementations complying to SCPI v1.0 or above
>> + * "arm,legacy-scpi" : For implementations complying pre SCPI v1.0
>
> I'd prefer that we explicitly enumerate the old versions. Are there
> many?
>
I understand your concern, but this legacy SCPI protocol was not
officially released. It was just WIP which vendors picked up from very
early releases. Since they are not numbered, it's hard to have specific
compatibles with different versions until v1.0. That's one of the reason
to retain platform specific compatible so that we can add any quirks
based on them if needed.
I will probably add these information in the commit log so that it's
clear why we can't do version based compatible.
--
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
* [RESEND PATCH] regulator: pwm: fix syntax errors in the examples
From: Peter Rosin @ 2016-11-10 10:25 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA
While at it, clean up some other things as well.
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
.../devicetree/bindings/regulator/pwm-regulator.txt | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
index 3aeba9f86ed8..6d98fbaeb506 100644
--- a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
@@ -55,11 +55,11 @@ Optional properties:
- enable-gpios: GPIO to use to enable/disable the regulator
Any property defined as part of the core regulator binding can also be used.
-(See: ../regulator/regulator.txt)
+(See: regulator.txt)
Continuous Voltage With Enable GPIO Example:
- pwm_regulator {
- compatible = "pwm-regulator;
+ pwm-regulator {
+ compatible = "pwm-regulator";
pwms = <&pwm1 0 8448 0>;
enable-gpios = <&gpio0 23 GPIO_ACTIVE_HIGH>;
regulator-min-microvolt = <1016000>;
@@ -71,12 +71,12 @@ Continuous Voltage With Enable GPIO Example:
* Inverted PWM logic, and the duty cycle range is limited
* to 30%-70%.
*/
- pwm-dutycycle-range <700 300>; /* */
+ pwm-dutycycle-range <700 300>;
};
Voltage Table Example:
- pwm_regulator {
- compatible = "pwm-regulator;
+ pwm-regulator {
+ compatible = "pwm-regulator";
pwms = <&pwm1 0 8448 0>;
regulator-min-microvolt = <1016000>;
regulator-max-microvolt = <1114000>;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v2 7/7] soc: renesas: Identify SoC and register with the SoC bus
From: Geert Uytterhoeven @ 2016-11-10 10:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linuxppc-dev@lists.ozlabs.org, Geert Uytterhoeven,
Greg Kroah-Hartman, Yangbo Lu, Simon Horman, Magnus Damm,
Rob Herring, Mark Rutland, devicetree@vger.kernel.org, Dirk Behme,
linux-kernel@vger.kernel.org, Linux-Renesas,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <3693445.TSdZGaiT9S@wuerfel>
Hi Arnd,
Thanks for your comments!
On Wed, Nov 9, 2016 at 5:55 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday, October 31, 2016 12:30:55 PM CET Geert Uytterhoeven wrote:
>> v2:
>> - Drop SoC families and family names; use fixed "Renesas" instead,
>
> I think I'd rather have seen the family names left in there, but it's
> not important, so up to you.
They're not useful for matching, as family names may change anytime, and don't
always say much about the hardware capabilities.
E.g. SH-Mobile -> R-Mobile -> R-Car | RZ/A | RZ/G
Some SH-Mobile (even some R-Car) parts are SuperH only, others have ARM and
SuperH.
At least the SoC part numbers are stable (hmm, sh73a0 == r8a73a0).
>> - Use "renesas,prr" and "renesas,cccr" device nodes in DT if
>> available, else fall back to hardcoded addresses for compatibility
>> with existing DTBs,
>
> I only see patches 2, 3, 5, and 7 in my inbox, so I'm lacking the DT
> binding for these, among other things.
I understand you've received them in the mean time?
> It does seem wrong to have a device node for a specific register though.
> Shouldn't the node be for the block of registers that these are inside
> of?
On R-Mobile APE6, R-Car Gen2 and Gen3, PRR is a lone register.
On R-Car Gen1, it's not even documented (and doesn't exist on all parts).
On SH-Mobile/R-Mobile, CCCR may be part of the HPB/APB register block, which
we further don't touch at all.
On R-Car Gen2, it's not documented, but does exist.
BTW, see why I'd prefer not to have it in DT at all, and go for KISS in code
we can change at any time? To avoid mistakes we have to keep on supporting
forever.
>> - Don't register the SoC bus if the chip ID register is missing,
>
> Why? My objection was to hardcoding the register, not to registering
> the device? I think I'd rather see the device registered with an
> empty revision string.
If there's no chip ID register, there's no reason to use soc_device_match(),
as we can always look at a compatible value. All SoCs listed in this driver
have a chip ID register.
if you want me to register the soc_bus for those SoCs regardless, I want to
re-add r7s72100 (RZ/A) and r8a7778 (R-Car M1A), who don't have chip ID
registers ;-)
>> +#define CCCR 0xe600101c /* Common Chip Code Register */
>> +#define PRR 0xff000044 /* Product Register */
>> +#define PRR3 0xfff00044 /* Product Register on R-Car Gen3 */
>> +
>> +static const struct of_device_id renesas_socs[] __initconst = {
>> +#ifdef CONFIG_ARCH_R8A73A4
>> + { .compatible = "renesas,r8a73a4", .data = (void *)PRR, },
>> +#endif
>> +#ifdef CONFIG_ARCH_R8A7740
>> + { .compatible = "renesas,r8a7740", .data = (void *)CCCR, },
>> +#endif
>
> My preference here would be to list the register address only for
> SoCs that are known to need them, while also having .dtb files that
> don't have the nodes.
Even if drivers don't have to handle differences, there's been a long
outstanding request to print SoC revision information during bootup
(E.g. "Does it still work on ES1.0?"). Hence that covers all SoCs.
>> +static int __init renesas_soc_init(void)
>> +{
>> + struct soc_device_attribute *soc_dev_attr;
>> + const struct of_device_id *match;
>> + void __iomem *chipid = NULL;
>> + struct soc_device *soc_dev;
>> + struct device_node *np;
>> + unsigned int product;
>> +
>> + np = of_find_matching_node_and_match(NULL, renesas_socs, &match);
>> + if (!np)
>> + return -ENODEV;
>> +
>> + of_node_put(np);
>> +
>> + /* Try PRR first, then CCCR, then hardcoded fallback */
>> + np = of_find_compatible_node(NULL, NULL, "renesas,prr");
>> + if (!np)
>> + np = of_find_compatible_node(NULL, NULL, "renesas,cccr");
>> + if (np) {
>> + chipid = of_iomap(np, 0);
>> + of_node_put(np);
>> + } else if (match->data) {
>> + chipid = ioremap((uintptr_t)match->data, 4);
>> + }
>> + if (!chipid)
>>
>
> Here, I'd turn the order around and look for the DT nodes of the
> devices first. Only if they are not found, look at the compatible
> string of the root node. No need to search for a node though,
> you know which one it is when you look for a compatible =
> "renesas,r8a73a4".
"renesas,r8a73a4" is the root node, not the device, so it does not have the
"reg" property for reading the chip ID?
There is no SoC part number in the "renesas,prr" and "renesas,cccr" nodes.
Hence I always need to look at the root nodes.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 04/14] ARM: dts: armada-375: Fixup bootrom DT warning
From: Thomas Petazzoni @ 2016-11-10 10:15 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: <87wpgbekwg.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hello,
On Thu, 10 Nov 2016 10:36:47 +0100, Gregory CLEMENT wrote:
> >> - bootrom {
> >> + bootrom@0 {
> >> compatible = "marvell,bootrom";
> >> reg = <MBUS_ID(0x01, 0x1d) 0 0x100000>;
> >
> > I am still not sure whether this "0" unit address is correct compared
> > to the reg property being passed.
>
> I chose to use the adress register inside the window memory.
Which I think is bogus, as my example below highlighted.
> > A good example of why I'm worried is the sa-sram case:
> >
> > + crypto_sram0: sa-sram0@0 {
> > compatible = "mmio-sram";
> > reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
> >
> > + crypto_sram1: sa-sram1@0 {
> > compatible = "mmio-sram";
> > reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
> >
> > The node names should be just "sram" without a number. Indeed for UARTs
> > for example, you use uart@XYZ, uart@ABC and not uart0@XYZ and
> > uart1@ABC. But then, if you do that, with your scheme, you end up with
> > both nodes named sa-sram@0.
> >
> > Which clearly shows that the way you set this unit-address is not
> > correct: those two devices are mapped at completely different
> > locations, but you end up with an identical unit address.
> >
> > I have no idea what is the rule for setting the unit address in this
> > case, but I'm pretty sure the rule you've chosen is not good.
>
> I don't know if there is an existing rules for this case. But I see your
> concern. What I propose then is to expose the memory windows ID by
> adding the target and the attributes like this:
>
> crypto_sram0: sa-sram@09_09_0 {
> compatible = "mmio-sram";
> reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
>
>
> crypto_sram1: sa-sram@09_05_0 {
> compatible = "mmio-sram";
> reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
I have no idea if 09_05_0 is considered a valid unit address. Indeed
the _ character in an address looks a bit weird.
I guess we need guidance from the DT binding maintainers on this, since
it's really a matter of interpreting what "unit address" means in
relation to the value of the "reg" property.
Rob, Mark?
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 5/6] Documentation: bindings: add documentation for ir-spi device driver
From: Andi Shyti @ 2016-11-10 10:02 UTC (permalink / raw)
To: Jacek Anaszewski
Cc: Rob Herring, Mauro Carvalho Chehab, Sean Young, Mark Rutland,
Richard Purdie, linux-media, devicetree, linux-leds, linux-kernel
In-Reply-To: <13210179-ea3f-6106-e3c0-fa30b83e23cc@samsung.com>
Hi Jacek,
> > > > > Only DT bindings of LED class drivers should be placed in
> > > > > Documentation/devicetree/bindings/leds. Please move it to the
> > > > > media bindings.
> > > >
> > > > that's where I placed it first, but Rob asked me to put it in the
> > > > LED directory and Cc the LED mailining list.
> > > >
> > > > That's the discussion of the version 2:
> > > >
> > > > https://lkml.org/lkml/2016/9/12/380
> > > >
> > > > Rob, Jacek, could you please agree where I can put the binding?
> > >
> > > I'm not sure if this is a good approach. I've noticed also that
> > > backlight bindings have been moved to leds, whereas they don't look
> > > similarly.
> > >
> > > We have common.txt LED bindings, that all LED class drivers' bindings
> > > have to follow. Neither backlight bindings nor these ones do that,
> > > which introduces some mess.
> >
> > And there are probably LED bindings that don't follow common.txt either.
> >
> > > Eventually adding a sub-directory, e.g. remote_control could make it
> > > somehow logically justified, but still - shouldn't bindings be
> > > placed in the documentation directory related to the subsystem of the
> > > driver they are predestined to?
> >
> > No. While binding directories often mirror the driver directories, they
> > are not the same. Bindings are grouped by types of h/w and IR LEDs are a
> > type of LED.
> >
> > If you prefer a sub-dir, that is fine with me.
>
> Fine. So how about sub-dir "ir" ?
would we put here all the remote control bindings that currently
are under media?
Thanks,
Andi
^ permalink raw reply
* Re: [PATCH 04/14] ARM: dts: armada-375: Fixup bootrom DT warning
From: Gregory CLEMENT @ 2016-11-10 9:36 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: <20161110092221.0219490b-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Hi Thomas,
On jeu., nov. 10 2016, Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hello,
>
> On Thu, 10 Nov 2016 01:09:50 +0100, Gregory CLEMENT wrote:
>
>> - bootrom {
>> + bootrom@0 {
>> compatible = "marvell,bootrom";
>> reg = <MBUS_ID(0x01, 0x1d) 0 0x100000>;
>
> I am still not sure whether this "0" unit address is correct compared
> to the reg property being passed.
I chose to use the adress register inside the window memory.
>
> A good example of why I'm worried is the sa-sram case:
>
> + crypto_sram0: sa-sram0@0 {
> compatible = "mmio-sram";
> reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
>
> + crypto_sram1: sa-sram1@0 {
> compatible = "mmio-sram";
> reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
>
> The node names should be just "sram" without a number. Indeed for UARTs
> for example, you use uart@XYZ, uart@ABC and not uart0@XYZ and
> uart1@ABC. But then, if you do that, with your scheme, you end up with
> both nodes named sa-sram@0.
>
> Which clearly shows that the way you set this unit-address is not
> correct: those two devices are mapped at completely different
> locations, but you end up with an identical unit address.
>
> I have no idea what is the rule for setting the unit address in this
> case, but I'm pretty sure the rule you've chosen is not good.
I don't know if there is an existing rules for this case. But I see your
concern. What I propose then is to expose the memory windows ID by
adding the target and the attributes like this:
crypto_sram0: sa-sram@09_09_0 {
compatible = "mmio-sram";
reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
crypto_sram1: sa-sram@09_05_0 {
compatible = "mmio-sram";
reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
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: [v16, 0/7] Fix eSDHC host version register bug
From: Geert Uytterhoeven @ 2016-11-10 9:26 UTC (permalink / raw)
To: Ulf Hansson
Cc: Yangbo Lu, linux-mmc, Scott Wood, Arnd Bergmann,
linuxppc-dev@lists.ozlabs.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-clk, iommu,
netdev@vger.kernel.org, Greg Kroah-Hartman, Mark Rutland,
Rob Herring, Russell King, Jochen Friedrich, Joerg Roedel,
Claudiu
In-Reply-To: <CAPDyKFrcAN_pqgtGaUanfB2zh97zGcL23m5VDtJ3g==NJeRrfA@mail.gmail.com>
Hi Ulf,
On Wed, Nov 9, 2016 at 7:27 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 9 November 2016 at 04:14, Yangbo Lu <yangbo.lu@nxp.com> wrote:
>> This patchset is used to fix a host version register bug in the T4240-R1.0-R2.0
>> eSDHC controller. To match the SoC version and revision, 15 previous version
>> patchsets had tried many methods but all of them were rejected by reviewers.
>> Such as
>> - dts compatible method
>> - syscon method
>> - ifdef PPC method
>> - GUTS driver getting SVR method
>> Anrd suggested a soc_device_match method in v10, and this is the only available
>> method left now. This v11 patchset introduces the soc_device_match interface in
>> soc driver.
>>
>> The first four patches of Yangbo are to add the GUTS driver. This is used to
>> register a soc device which contain soc version and revision information.
>> The other three patches introduce the soc_device_match method in soc driver
>> and apply it on esdhc driver to fix this bug.
>>
>> ---
>> Changes for v15:
>> - Dropped patch 'dt: bindings: update Freescale DCFG compatible'
>> since the work had been done by below patch on ShawnGuo's linux tree.
>> 'dt-bindings: fsl: add LS1043A/LS1046A/LS2080A compatible for SCFG
>> and DCFG'
>> - Fixed error code issue in guts driver
>> Changes for v16:
>> - Dropped patch 'powerpc/fsl: move mpc85xx.h to include/linux/fsl'
>> - Added a bug-fix patch from Geert
>> ---
>>
>> Arnd Bergmann (1):
>> base: soc: introduce soc_device_match() interface
>>
>> Geert Uytterhoeven (1):
>> base: soc: Check for NULL SoC device attributes
> Thanks, applied on my mmc tree for next!
Oops, the above two commits (plus two more enhancements) are also a dependency
for Samsung and Renesas. Hence the plan was to use an immutable branch for
that...
Ulf, would it still be possible to replace these two commits in mmc/next:
8b82c17a8ae533d6 base: soc: introduce soc_device_match() interface
6fa350172b098f0f base: soc: Check for NULL SoC device attributes
by a merge of the immutable branch I've just created?
Cfr, the other thread "[PATCH v2 0/7] soc: renesas: Identify SoC and register
with the SoC bus".
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 5/5] media: platform: rcar_drif: Add DRIF support
From: Laurent Pinchart @ 2016-11-10 9:22 UTC (permalink / raw)
To: Ramesh Shanmugasundaram
Cc: robh+dt, mark.rutland, mchehab, hverkuil, sakari.ailus, crope,
chris.paterson2, geert+renesas, linux-media, devicetree,
linux-renesas-soc
In-Reply-To: <1478706284-59134-6-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>
Hi Ramesh,
Thank you for the patch.
On Wednesday 09 Nov 2016 15:44:44 Ramesh Shanmugasundaram wrote:
> This patch adds Digital Radio Interface (DRIF) support to R-Car Gen3 SoCs.
> The driver exposes each instance of DRIF as a V4L2 SDR device. A DRIF
> device represents a channel and each channel can have one or two
> sub-channels respectively depending on the target board.
>
> DRIF supports only Rx functionality. It receives samples from a RF
> frontend tuner chip it is interfaced with. The combination of DRIF and the
> tuner device, which is registered as a sub-device, determines the receive
> sample rate and format.
>
> In order to be compliant as a V4L2 SDR device, DRIF needs to bind with
> the tuner device, which can be provided by a third party vendor. DRIF acts
> as a slave device and the tuner device acts as a master transmitting the
> samples. The driver allows asynchronous binding of a tuner device that
> is registered as a v4l2 sub-device. The driver can learn about the tuner
> it is interfaced with based on port endpoint properties of the device in
> device tree. The V4L2 SDR device inherits the controls exposed by the
> tuner device.
>
> The device can also be configured to use either one or both of the data
> pins at runtime based on the master (tuner) configuration.
>
> Signed-off-by: Ramesh Shanmugasundaram
> <ramesh.shanmugasundaram@bp.renesas.com> ---
> .../devicetree/bindings/media/renesas,drif.txt | 136 ++
> drivers/media/platform/Kconfig | 25 +
> drivers/media/platform/Makefile | 1 +
> drivers/media/platform/rcar_drif.c | 1574
> ++++++++++++++++++++ 4 files changed, 1736 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
> create mode 100644 drivers/media/platform/rcar_drif.c
>
> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt
> b/Documentation/devicetree/bindings/media/renesas,drif.txt new file mode
> 100644
> index 0000000..d65368a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> @@ -0,0 +1,136 @@
> +Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
> +------------------------------------------------------------
> +
> +R-Car Gen3 DRIF is a serial slave device. It interfaces with a master
> +device as shown below
> +
> ++---------------------+ +---------------------+
> +| |-----SCK------->|CLK |
> +| Master |-----SS-------->|SYNC DRIFn (slave) |
> +| |-----SD0------->|D0 |
> +| |-----SD1------->|D1 |
> ++---------------------+ +---------------------+
> +
> +Each DRIF channel (drifn) consists of two sub-channels (drifn0 & drifn1).
> +The sub-channels are like two individual channels in itself that share the
> +common CLK & SYNC. Each sub-channel has it's own dedicated resources like
> +irq, dma channels, address space & clock.
> +
> +The device tree model represents the channel and each of it's sub-channel
> +as a separate node. The parent channel ties the sub-channels together with
> +their phandles.
> +
> +Required properties of a sub-channel:
> +-------------------------------------
> +- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of
> R8A7795 SoC.
> + "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible
> device.
> + When compatible with the generic version, nodes must list the
> + SoC-specific version corresponding to the platform first
> + followed by the generic version.
> +- reg: offset and length of that sub-channel.
> +- interrupts: associated with that sub-channel.
> +- clocks: phandle and clock specifier of that sub-channel.
> +- clock-names: clock input name string: "fck".
> +- dmas: phandles to the DMA channel of that sub-channel.
> +- dma-names: names of the DMA channel: "rx".
> +
> +Optional properties of a sub-channel:
> +-------------------------------------
> +- power-domains: phandle to the respective power domain.
> +
> +Required properties of a channel:
> +---------------------------------
> +- pinctrl-0: pin control group to be used for this channel.
> +- pinctrl-names: must be "default".
> +- sub-channels : phandles to the two sub-channels.
> +
> +Optional properties of a channel:
> +---------------------------------
> +- port: child port node of a channel that defines the local and remote
> + endpoints. The remote endpoint is assumed to be a tuner subdevice
> + endpoint.
> +- renesas,syncmd : sync mode
> + 0 (Frame start sync pulse mode. 1-bit width pulse
> + indicates start of a frame)
> + 1 (L/R sync or I2S mode) (default)
> +- renesas,lsb-first : empty property indicates lsb bit is received
> first.
> + When not defined msb bit is received first (default)
Shouldn't those two properties be instead queried from the tuner at runtime
through the V4L2 subdev API ?
> +- renesas,syncac-pol-high : empty property indicates sync signal polarity.
> + When defined, active high or high->low sync signal.
> + When not defined, active low or low->high sync signal
> + (default)
This could be queried too, except that an inverter could be present on the
board, so it has to be specified in DT. I would however try to standardize it
the same way that hsync-active and vsync-active are standardized in
Documentation/devicetree/bindings/media/video-interfaces.txt.
> +- renesas,dtdl : delay between sync signal and start of reception.
Are this and the next property meant to account for PCB traces delays ?
> + Must contain one of the following values:
> + 0 (no bit delay)
> + 50 (0.5-clock-cycle delay)
> + 100 (1-clock-cycle delay) (default)
> + 150 (1.5-clock-cycle delay)
> + 200 (2-clock-cycle delay)
How about specifying the property in half clock cycle units, from 0 to 4 ?
> +- renesas,syncdl : delay between end of reception and sync signal
> edge.
> + Must contain one of the following values:
> + 0 (no bit delay) (default)
> + 50 (0.5-clock-cycle delay)
> + 100 (1-clock-cycle delay)
> + 150 (1.5-clock-cycle delay)
> + 200 (2-clock-cycle delay)
> + 300 (3-clock-cycle delay)
> +
> +Example
> +--------
> +
> +SoC common dtsi file
> +
> + drif00: rif@e6f40000 {
> + compatible = "renesas,r8a7795-drif",
> + "renesas,rcar-gen3-drif";
> + reg = <0 0xe6f40000 0 0x64>;
> + interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 515>;
> + clock-names = "fck";
> + dmas = <&dmac1 0x20>, <&dmac2 0x20>;
> + dma-names = "rx", "rx";
> + power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> + status = "disabled";
> + };
> +
> + drif01: rif@e6f50000 {
> + compatible = "renesas,r8a7795-drif",
> + "renesas,rcar-gen3-drif";
> + reg = <0 0xe6f50000 0 0x64>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cpg CPG_MOD 514>;
> + clock-names = "fck";
> + dmas = <&dmac1 0x22>, <&dmac2 0x22>;
> + dma-names = "rx", "rx";
> + power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> + status = "disabled";
> + };
> +
> + drif0: rif@0 {
> + compatible = "renesas,r8a7795-drif",
> + "renesas,rcar-gen3-drif";
> + sub-channels = <&drif00>, <&drif01>;
> + status = "disabled";
> + };
I'm afraid this really hurts my eyes, especially using the same compatible
string for both the channel and sub-channel nodes.
We need to decide how to model the hardware in DT. Given that the two channels
are mostly independent having separate DT nodes makes sense to me. However, as
they share the clock and sync signals, somehow grouping them makes sense. I
see three ways to do so, and there might be more.
1. Adding an extra DT node for the channels group, with phandles to the two
channels. This is what you're proposing here.
2. Adding an extra DT node for the channels group, as a parent of the two
channels.
3. Adding phandles to the channels, pointing to each other, or possibly a
phandle from channel 0 pointing to channel 1.
Neither of these options seem perfect to me. I don't like option 1 as the
group DT node really doesn't describe a hardware block. If we want to use a DT
node to convey group information, option 2 seems better to me. However, it
somehow abuses the DT parent-child model that is supposed to describe
relationships from a control bus point of view. Option 3 has the drawback of
not scaling properly, at least with phandles in both channels pointing to the
other one.
Rob, Geert, tell me you have a fourth idea I haven't thought of that would
solve all those problems :-)
> +Board specific dts file
> +
> +&drif00 {
> + status = "okay";
> +};
> +
> +&drif01 {
> + status = "okay";
> +};
> +
> +&drif0 {
> + pinctrl-0 = <&drif0_pins>;
> + pinctrl-names = "default";
> + renesas,syncac-pol-high;
> + status = "okay";
> + port {
> + drif0_ep: endpoint {
> + remote-endpoint = <&tuner_subdev_ep>;
> + };
> + };
> +};
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v2 0/7] soc: renesas: Identify SoC and register with the SoC bus
From: Geert Uytterhoeven @ 2016-11-10 9:22 UTC (permalink / raw)
To: Ulf Hansson
Cc: Mark Rutland, devicetree@vger.kernel.org, Dirk Behme,
Arnd Bergmann, Simon Horman, Greg Kroah-Hartman,
linux-kernel@vger.kernel.org, Linux-Renesas,
linux-samsung-soc@vger.kernel.org, Rob Herring, Yangbo Lu,
linuxppc-dev@lists.ozlabs.org, Pankaj Dubey,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <6723536.97ljqcB8tS@wuerfel>
Hi Ulf,
On Wed, Nov 9, 2016 at 10:12 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday, November 9, 2016 6:19:06 PM CET Geert Uytterhoeven wrote:
>> On Wed, Nov 9, 2016 at 5:56 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Wednesday, November 9, 2016 2:34:33 PM CET Geert Uytterhoeven wrote:
>> >> > And Samsung.
>> >> > Shall I create the immutable branch now?
>> >>
>> >> Arnd: are you happy with the new patches and changes?
>> >
>> > I still had some comments for patch 7, but that shouldn't stop
>> > you from creating a branch for the first three so everyone can
>> > build on top of that.
>>
>> Thanks!
>>
>> What about patch [4/7]?
>> Haven't you received it? Your address was in the To-line for all 7 patches.
>
> Ok, I see it now, looks good. That should be included as well then.
Thanks, I've created the branch/tag :
git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git
branch soc-device-match
signed tag soc-device-match-tag1
In the mean time, Ulf has applied the first two patches to mmc/next, on top
of lots of MMC work :-(
Ulf, as this is not only a dependency for Freescale/NXP (for sdhci-of-esdhc),
but also for Samsung and Renesas, would it still be possible to replace these
two commits
8b82c17a8ae533d6 base: soc: introduce soc_device_match() interface
6fa350172b098f0f base: soc: Check for NULL SoC device attributes
by a merge of soc-device-match-tag1?
You can find more info in the full thread at
https://www.spinics.net/lists/devicetree/msg148558.html
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] phy: rockchip-inno-usb2: correct 480MHz output clock stable time
From: Heiko Stübner @ 2016-11-10 9:21 UTC (permalink / raw)
To: wlf
Cc: Doug Anderson, Kishon Vijay Abraham I,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
open list:ARM/Rockchip SoC..., devicetree@vger.kernel.org,
Rob Herring, Frank Wang, 黄涛, Brian Norris,
Guenter Roeck, Matthias Kaehlcke, linux-clk
In-Reply-To: <9eeb3d84-f850-4b76-ce6a-67cc92d6de3b@rock-chips.com>
Am Donnerstag, 10. November 2016, 10:54:49 schrieb wlf:
> Hi Doug,
>
> 在 2016年11月10日 04:54, Doug Anderson 写道:
> > Hi,
> >
> > On Mon, Nov 7, 2016 at 5:00 AM, William Wu <wulf@rock-chips.com> wrote:
> >> We found that the system crashed due to 480MHz output clock of
> >> USB2 PHY was unstable after clock had been enabled by gpu module.
> >>
> >> Theoretically, 1 millisecond is a critical value for 480MHz
> >> output clock stable time, so we try to change the delay time
> >> to 1.2 millisecond to avoid this issue.
> >>
> >> Signed-off-by: William Wu <wulf@rock-chips.com>
> >> ---
> >>
> >> drivers/phy/phy-rockchip-inno-usb2.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/phy/phy-rockchip-inno-usb2.c
> >> b/drivers/phy/phy-rockchip-inno-usb2.c index ecfd7d1..8f2d2b6 100644
> >> --- a/drivers/phy/phy-rockchip-inno-usb2.c
> >> +++ b/drivers/phy/phy-rockchip-inno-usb2.c
> >> @@ -267,7 +267,7 @@ static int rockchip_usb2phy_clk480m_enable(struct
> >> clk_hw *hw)>>
> >> return ret;
> >>
> >> /* waitting for the clk become stable */
> >>
> >> - mdelay(1);
> >> + udelay(1200);
> >
> > Several people who have seen this patch have expressed concern that a
> > 1.2 ms delay is pretty long for something that's supposed to be
> > "atomic" like a clk_enable(). Consider that someone might call
> > clk_enable() while interrupts are disabled and that a 1.2 ms interrupt
> > latency is not so great.
> >
> > It seems like this clock should be moved to be enabled in "prepare"
> > and the "enable" should be a no-op. This is a functionality change,
> > but I don't think there are any real users for this clock at the
> > moment so it should be fine.
> >
> > (of course, the 1 ms latency that existed before this patch was still
> > pretty bad, but ...)
>
> Thanks a lot for your suggestion.
> I agree with you. clk_enable() will call spin_lock_irqsave() to disable
> interrupt, and we add
> more than 1ms in clk_enable may cause big latency.
>
> And according to clk_prepare() description:
> In a simple case, clk_prepare can be used instead of clk_enable to
> ungate a clk if the
> operation may sleep. One example is a clk which is accessed over I2c.
>
> So maybe we can remove the clock to clk_prepare.
>
> Hi Heiko, Frank,
> What do you think of it?
moving to clk_prepare sounds sensible. That way you can switch from delay to
sleep functions as well.
Heiko
^ permalink raw reply
* Re: [PATCH 2/3] drm/bridge: Add ti-ftp410 HDMI transmitter driver
From: Jyri Sarha @ 2016-11-10 9:16 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree, bcousson, khilman, dri-devel, bgolaszewski,
tomi.valkeinen
In-Reply-To: <12424154.kJqhEf2pOz@avalon>
On 11/03/16 19:46, Laurent Pinchart wrote:
> Hi Jyri,
>
> Thank you for the patch.
>
> On Wednesday 02 Nov 2016 18:32:16 Jyri Sarha wrote:
>> Add very basic ti-ftp410 HDMI transmitter driver. The only feature
>> separating this from a completely dummy bridge is the DDC i2c
>> support. However, other HW specific features may be added later when
>> needed. For instance there is a set of registers behind i2c if it is
>> connected. The implementations is tested against my new tilcdc bridge
>> support and works with BeagleBone DVI-D Cape Rev A3. A DT binding
>> document is also added.
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>> .../bindings/display/bridge/ti,tfp410.txt | 30 ++++
>> drivers/gpu/drm/bridge/Kconfig | 7 +
>> drivers/gpu/drm/bridge/Makefile | 1 +
>> drivers/gpu/drm/bridge/ti-tfp410.c | 199 ++++++++++++++++++
>> 4 files changed, 237 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..dc93713
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
>> @@ -0,0 +1,30 @@
>> +TFP410 HDMI/DVI bridge bindings
>
> I'd name the document "TI TFP410 DVI Transmitter". DVI bridge doesn't tell
> whether the device is a receiver or transmitter.
>
>> +Required properties:
>> + - compatible: "ti,tfp410"
>
> The device is an I2C slave, it should have a reg property. Given that the chip
> can be used without being controlled through I2C, the reg property should be
> optional. You should document this clearly, and explain how the DT node can be
> instantiated as a child of an I2C controller when the I2C interface is used,
> or in other parts of the device tree otherwise.
>
>> +Optional properties:
>> + - ddc-i2c: phandle of an I2C controller used for DDC EDID probing
>
> The TFP410 doesn't handle DDC, this property should be part of the connector
> node.
>
>> +Optional subnodes:
>> + - video input: this subnode can contain a video input port node
>> + to connect the bridge to a display controller output (See this
>> + documentation [1]).
>
> You also need an output port for the DVI output. Those two ports should be
> required, not optional.
>
Ok. So I need another device node. Should I create some specific
compatible string for connectors behind tfp410, or a generic DVI/HDMI
connector with optional ddc-i2c phandle?
The implementation side is not so critical, because it more easily
changed, but should I create an independent generic platform-device
driver for such DVI/HDMI connector or just implement the connector side
within tfp410 driver?
>> +[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>;
>> + };
>> + };
>> + };
>> + };
>
>
>> 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..b0753d2
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
>> @@ -0,0 +1,199 @@
>> +/*
>> + * Copyright (C) 2016 Texas Instruments
>> + * Author: Jyri Sarha <jsarha@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published
>> by + * the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of_graph.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 *hdmi = drm_connector_to_tfp410(connector);
>> + struct edid *edid;
>> + int ret;
>> +
>> + if (!hdmi->ddc)
>> + goto fallback;
>> +
>> + edid = drm_get_edid(connector, hdmi->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 *hdmi = drm_connector_to_tfp410(connector);
>> +
>> + if (hdmi->ddc) {
>> + if (drm_probe_ddc(hdmi->ddc))
>> + return connector_status_connected;
>> + else
>> + return connector_status_disconnected;
>> + }
>> +
>> + 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 *hdmi = drm_bridge_to_tfp410(bridge);
>> + int ret;
>> +
>> + if (!bridge->encoder) {
>> + dev_err(hdmi->dev, "Missing encoder\n");
>> + return -ENODEV;
>> + }
>> +
>> + drm_connector_helper_add(&hdmi->connector,
>> + &tfp410_con_helper_funcs);
>> + ret = drm_connector_init(bridge->dev, &hdmi->connector,
>> + &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
>> + if (ret) {
>> + dev_err(hdmi->dev, "drm_connector_init() failed: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + drm_mode_connector_attach_encoder(&hdmi->connector,
>> + bridge->encoder);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
>> + .attach = tfp410_attach,
>> +};
>> +
>> +static int tfp410_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *ddc_phandle;
>> + struct tfp410 *hdmi;
>> + int ret;
>> +
>> + if (!pdev->dev.of_node) {
>> + dev_err(&pdev->dev, "device-tree data is missing\n");
>> + return -ENXIO;
>> + }
>> +
>> + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
>> + if (!hdmi)
>> + return -ENOMEM;
>> + platform_set_drvdata(pdev, hdmi);
>> +
>> + ddc_phandle = of_parse_phandle(pdev->dev.of_node, "ddc-i2c", 0);
>> + if (ddc_phandle) {
>> + hdmi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
>> + of_node_put(ddc_phandle);
>> + if (!hdmi->ddc)
>> + return -EPROBE_DEFER;
>> + } else {
>> + dev_info(&pdev->dev,
>> + "No ddc i2c bus, disabling EDID readout\n");
>> + }
>> +
>> + hdmi->bridge.funcs = &tfp410_bridge_funcs;
>> + hdmi->bridge.of_node = pdev->dev.of_node;
>> + hdmi->dev = &pdev->dev;
>> +
>> + ret = drm_bridge_add(&hdmi->bridge);
>> + if (ret) {
>> + dev_err(&pdev->dev, "drm_bridge_add() failed: %d\n", ret);
>> + goto fail;
>> + }
>> +
>> + return 0;
>> +fail:
>> + i2c_put_adapter(hdmi->ddc);
>> + return ret;
>> +}
>> +
>> +static int tfp410_remove(struct platform_device *pdev)
>> +{
>> + struct tfp410 *hdmi = platform_get_drvdata(pdev);
>> +
>> + drm_bridge_remove(&hdmi->bridge);
>> +
>> + if (hdmi->ddc)
>> + i2c_put_adapter(hdmi->ddc);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id tfp410_match[] = {
>> + { .compatible = "ti,tfp410" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, tfp410_match);
>> +
>> +struct platform_driver tfp410_driver = {
>> + .probe = tfp410_probe,
>> + .remove = tfp410_remove,
>> + .driver = {
>> + .name = "tfp410-bridge",
>> + .of_match_table = tfp410_match,
>> + },
>> +};
>> +module_platform_driver(tfp410_driver);
>> +
>> +MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>");
>> +MODULE_DESCRIPTION("TI TFP410 HDMI bridge driver");
>> +MODULE_LICENSE("GPL");
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-10 9:12 UTC (permalink / raw)
To: linux-arm-kernel
Cc: zhichang.yuan, mark.rutland@arm.com, devicetree@vger.kernel.org,
lorenzo.pieralisi@arm.com, Gabriele Paoloni, minyard@acm.org,
linux-pci@vger.kernel.org, benh@kernel.crashing.org, John Garry,
will.deacon@arm.com, linux-kernel@vger.kernel.org, xuwei (O),
Linuxarm, zourongrong@gmail.com, robh+dt@kernel.org,
kantyzc@163.com, linux-serial
In-Reply-To: <5824165A.4040303@hisilicon.com>
On Thursday, November 10, 2016 2:40:26 PM CET zhichang.yuan wrote:
> On 2016/11/10 5:34, Arnd Bergmann wrote:
> > On Wednesday, November 9, 2016 12:10:43 PM CET Gabriele Paoloni wrote:
> >>> On Tuesday, November 8, 2016 11:47:09 AM CET zhichang.yuan wrote:
> >>>> + /*
> >>>> + * The first PCIBIOS_MIN_IO is reserved specifically for
> >>> indirectIO.
> >>>> + * It will separate indirectIO range from pci host bridge to
> >>>> + * avoid the possible PIO conflict.
> >>>> + * Set the indirectIO range directly here.
> >>>> + */
> >>>> + lpcdev->io_ops.start = 0;
> >>>> + lpcdev->io_ops.end = PCIBIOS_MIN_IO - 1;
> >>>> + lpcdev->io_ops.devpara = lpcdev;
> >>>> + lpcdev->io_ops.pfin = hisilpc_comm_in;
> >>>> + lpcdev->io_ops.pfout = hisilpc_comm_out;
> >>>> + lpcdev->io_ops.pfins = hisilpc_comm_ins;
> >>>> + lpcdev->io_ops.pfouts = hisilpc_comm_outs;
> >>>
> >>> I have to look at patch 2 in more detail again, after missing a few
> >>> review
> >>> rounds. I'm still a bit skeptical about hardcoding a logical I/O port
> >>> range here, and would hope that we can just go through the same
> >>> assignment of logical port ranges that we have for PCI buses,
> >>> decoupling
> >>> the bus addresses from the linux-internal ones.
> >>
> >> The point here is that we want to avoid any conflict/overlap between
> >> the LPC I/O space and the PCI I/O space. With the assignment above
> >> we make sure that LPC never interfere with PCI I/O space.
> >
> > But we already abstract the PCI I/O space using dynamic registration.
> > There is no need to hardcode the logical address for ISA, though
> > I think we can hardcode the bus address to start at zero here.
>
> Do you means that we can pick up the maximal I/O address from all children's
> device resources??
The driver should not look at the resources of its children, just
register a range of addresses dynamically, as I suggested in an
earlier review.
Your current version has
if (arm64_extio_ops->pfout) \
arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
addr, value, sizeof(type)); \
Instead, just subtract the start of the range from the logical
port number to transform it back into a bus-local port number:
if (arm64_extio_ops->pfout) \
arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
addr - arm64_extio_ops->start, value, sizeof(type)); \
We know that the ISA/LPC bus can only have up to 65536 ports,
so you can register all of those, or possibly limit it further to
1024 or 4096 ports, whichever matches the bus implementation.
Arnd
^ permalink raw reply
* Re: [PATCH 2/2] phy: qcom-qmp: new qmp phy driver for qcom-chipsets
From: Vivek Gautam @ 2016-11-10 9:03 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: robh+dt, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ca4054a8-a813-0f50-3224-1e418eaa7095-l0cyMroinI0@public.gmane.org>
Hi Kishon,
On Thu, Oct 27, 2016 at 1:41 AM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
> Hi,
>
> On Wednesday 19 October 2016 04:13 PM, Vivek Gautam wrote:
>> Qualcomm SOCs have QMP phy controller that provides support
>> to a number of controller, viz. PCIe, UFS, and USB.
>> Add a new driver, based on generic phy framework, for this
>> phy controller.
>>
>> USB3-phy changes: Based on phy-msm-ssusb-qmp driver available on
>> msm-4.4 kernel @codeaurora[1].
>> PCIe-phy changes: Based on msm8996-pcie-phy driver posted by
>> Srinivas [2].
>>
>> [1] https://source.codeaurora.org/quic/la/kernel/msm-4.4/log/?h=caf/3.18/msm-3.18
>> [2] https://patchwork.kernel.org/patch/9318947/
>
> use only lkml links here.
Ok.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Cc: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>> ---
>> .../devicetree/bindings/phy/qcom-qmp-phy.txt | 61 ++
>> drivers/phy/Kconfig | 8 +
>> drivers/phy/Makefile | 1 +
>> drivers/phy/phy-qcom-qmp.c | 1154 ++++++++++++++++++++
>> 4 files changed, 1224 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> create mode 100644 drivers/phy/phy-qcom-qmp.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> new file mode 100644
>> index 0000000..90214aa
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> @@ -0,0 +1,61 @@
>> +Qualcomm QMP PHY
>> +----------------
>> +
>> +QMP phy controller supports physical layer functionality for a number of
>> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.
>> +
>> +Required properties:
>> + - compatible: compatible list, contains:
>> + "qcom,msm8996-qmp-pcie-phy" for 14nm PCIe phy on msm8996,
>> + "qcom,msm8996-qmp-usb3-phy" for 14nm USB3 phy on msm8996.
>> + - reg: offset and length of the PHY register set.
>> + - #phy-cells: must be 1
>> + - Cell after phy phandle should be the port (lane) number.
>> + - clocks: a list of phandles and clock-specifier pairs,
>> + one for each entry in clock-names.
>> + - clock-names: must be "cfg_ahb" for phy config clock,
>> + "aux" for phy aux clock,
>> + "ref_clk" for 19.2 MHz ref clk,
>> + "ref_clk_src" for reference clock source,
>> + "pipe<port-number>" for pipe clock specific to
>> + each port/lane (Optional).
>> + - resets: a list of phandles and reset controller specifier pairs,
>> + one for each entry in reset-names.
>> + - reset-names: must be "phy" for reset of phy block,
>> + "common" for phy common block reset,
>> + "cfg" for phy's ahb cfg block reset (Optional).
>> + "port<port-number>" for reset specific to
>> + each port/lane. (Optional)
>> + - vdda-phy-supply: Phandle to a regulator supply to PHY core block.
>> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
>> +
>> +Optional properties:
>> + - vddp-ref-clk-supply: Phandle to a regulator supply to any specific refclk
>> + pll block.
>> +
>> +Example:
>> + pcie_phy: pciephy@34000 {
>> + compatible = "qcom,qmp-14nm-pcie-phy";
>> + reg = <0x034000 0x3fff>;
>> + #phy-cells = <1>;
>> +
>> + clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>> + <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>> + <&gcc GCC_PCIE_0_PIPE_CLK>,
>> + <&gcc GCC_PCIE_1_PIPE_CLK>,
>> + <&gcc GCC_PCIE_2_PIPE_CLK>;
>> + clock-names = "aux", "cfg_ahb",
>> + "pipe0", "pipe1", "pipe2";
>> +
>> + vdda-phy-supply = <&pm8994_l28>;
>> + vdda-pll-supply = <&pm8994_l12>;
>> +
>> + resets = <&gcc GCC_PCIE_PHY_BCR>,
>> + <&gcc GCC_PCIE_PHY_COM_BCR>,
>> + <&gcc GCC_PCIE_PHY_COM_NOCSR_BCR>,
>> + <&gcc GCC_PCIE_0_PHY_BCR>,
>> + <&gcc GCC_PCIE_1_PHY_BCR>,
>> + <&gcc GCC_PCIE_2_PHY_BCR>;
>> + reset-names = "phy", "common", "cfg",
>> + "lane0", "lane1", "lane2";
>> + };
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 5547984..d5e2b50f 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -446,6 +446,14 @@ config PHY_STIH41X_USB
>> Enable this to support the USB transceiver that is part of
>> STMicroelectronics STiH41x SoC series.
>>
>> +config PHY_QCOM_QMP
>> + tristate "Qualcomm QMP PHY Driver"
>> + depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> + select GENERIC_PHY
>> + help
>> + Enable this to support the QMP PHY transceiver that is used
>> + with controllers such as PCIe, UFS, and USB on Qualcomm chips.
>> +
>> config PHY_QCOM_QUSB2
>> tristate "Qualcomm QUSB2 PHY Driver"
>> depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 848489d..fde9fba 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -51,6 +51,7 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
>> obj-$(CONFIG_PHY_XGENE) += phy-xgene.o
>> obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o
>> obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o
>> +obj-$(CONFIG_PHY_QCOM_QMP) += phy-qcom-qmp.o
>> obj-$(CONFIG_PHY_QCOM_QUSB2) += phy-qcom-qusb2.o
>> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
>> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
>> diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
>> new file mode 100644
>> index 0000000..7e89179
>> --- /dev/null
>> +++ b/drivers/phy/phy-qcom-qmp.c
>> @@ -0,0 +1,1154 @@
>> +/*
>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/reset.h>
>> +#include <linux/slab.h>
>> +
[snip]
>> +
>> +static struct qmp_phy_init_tbl usb3phy_serdes_init_tbl[] = {
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYSCLK_EN_SEL, 0x14),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BIAS_EN_CLKBUFLR_EN, 0x08),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CLK_SELECT, 0x30),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CMN_CONFIG, 0x06),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SVS_MODE_CLK_SEL, 0x01),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_HSCLK_SEL, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TRIM, 0x0f),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_IVCO, 0x0f),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SYS_CLK_CTRL, 0x04),
>> + /* PLL and Loop filter settings */
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DEC_START_MODE0, 0x82),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START1_MODE0, 0x55),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START2_MODE0, 0x55),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_DIV_FRAC_START3_MODE0, 0x03),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CP_CTRL_MODE0, 0x0b),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_RCTRL_MODE0, 0x16),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_PLL_CCTRL_MODE0, 0x28),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_INTEGLOOP_GAIN0_MODE0, 0x80),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_CTRL, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP1_MODE0, 0x15),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP2_MODE0, 0x34),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP3_MODE0, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_CORE_CLK_EN, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_LOCK_CMP_CFG, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_VCO_TUNE_MAP, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_BG_TIMER, 0x0a),
>> + /* SSC settings */
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_EN_CENTER, 0x01),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER1, 0x31),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_PER2, 0x01),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER1, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_ADJ_PER2, 0x00),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE1, 0xde),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_COM_SSC_STEP_SIZE2, 0x07),
>> +};
>> +
>> +static struct qmp_phy_init_tbl usb3phy_tx_init_tbl[] = {
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_HIGHZ_TRANSCEIVEREN_BIAS_DRVR_EN, 0x45),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_RCV_DETECT_LVL_2, 0x12),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_TX_LANE_MODE, 0x06),
>> +};
>> +
>> +static struct qmp_phy_init_tbl usb3phy_rx_init_tbl[] = {
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_FASTLOCK_FO_GAIN, 0x0b),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_UCDR_SO_GAIN, 0x04),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL2, 0x02),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL3, 0x4c),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQU_ADAPTOR_CNTRL4, 0xbb),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x77),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_RX_OFFSET_ADAPTOR_CNTRL2, 0x80),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_CNTRL, 0x03),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_LVL, 0x18),
>> + QCOM_QMP_PHY_INIT_CFG(QSERDES_RX_SIGDET_DEGLITCH_CNTRL, 0x16),
>> +};
>> +
>> +static struct qmp_phy_init_tbl usb3phy_pcs_init_tbl[] = {
>> + /* FLL settings */
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL2, 0x03),
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNTRL1, 0x02),
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_L, 0x09),
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_CNT_VAL_H_TOL, 0x42),
>> + QCOM_QMP_PHY_INIT_CFG_L(QPHY_FLL_MAN_CODE, 0x85),
>> +
>> + /* Lock Det settings */
>> + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG1, 0xd1),
>> + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG2, 0x1f),
>> + QCOM_QMP_PHY_INIT_CFG(QPHY_LOCK_DETECT_CONFIG3, 0x47),
>> + QCOM_QMP_PHY_INIT_CFG(QPHY_POWER_STATE_CONFIG2, 0x08),
>> +};
>
> if you can implement the dt mechanism I mentioned in your other patch, all
> these tables can be got rid of and the code to initialize these can also be
> removed.
These are lot of configurations for PLL, TX, RX and PCS. I think it's
hard to take
out common bindings that can serve purpose of other PHYs as well.
[snip]
>> +unsigned int msm8996_pciephy_tx_offsets[] = { 0x1000, 0x2000, 0x3000 };
>
> you can have a separate reg map for each lane and all these can come from dt.
The idea is to avoid the any child nodes for lanes. So, we have the complete
ioremaped region and these offsets to tx, rx and pcs blocks.
>> +unsigned int msm8996_pciephy_rx_offsets[] = { 0x1200, 0x2200, 0x3200 };
>> +unsigned int msm8996_pciephy_pcs_offsets[] = { 0x1400, 0x2400, 0x3400 };
[snip]
>> +static int qcom_qmp_phy_poweron(struct phy *phy)
>> +{
>> + struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
>> + struct qcom_qmp_phy *qphy = phydesc->qphy;
>> + int ret;
>> +
>> + dev_info(&phy->dev, "Powering on QMP phy\n");
>> +
>> + ret = regulator_enable(qphy->vdda_phy);
>> + if (ret) {
>> + dev_err(qphy->dev, "%s: vdda-phy enable failed, err=%d\n",
>> + __func__, ret);
>> + return ret;
>> + }
>> +
>> + ret = regulator_enable(qphy->vdda_pll);
>> + if (ret) {
>> + dev_err(qphy->dev, "%s: vdda-pll enable failed, err=%d\n",
>> + __func__, ret);
>> + goto err_vdda_pll;
>> + }
>> +
>> + if (qphy->vddp_ref_clk) {
>> + ret = regulator_enable(qphy->vddp_ref_clk);
>> + if (ret) {
>> + dev_err(qphy->dev, "%s: vdda-ref-clk enable failed, err=%d\n",
>> + __func__, ret);
>> + goto err_vddp_refclk;
>> + }
>> + }
>> +
>> + if (!qphy->clk_enabled) {
>
> lot of my comments on the previous PHY driver is applicable here. For example
> the clk_enabled is not required.
Sure, will remove this and take care of other things similar to
qusb2 phy driver.
>> + clk_prepare_enable(qphy->ref_clk_src);
>> + clk_prepare_enable(qphy->ref_clk);
>> + clk_prepare_enable(phydesc->pipe_clk);
>> + qphy->clk_enabled = true;
>> + }
[snip]
>> +static struct phy *qcom_qmp_phy_xlate(struct device *dev,
>> + struct of_phandle_args *args)
>> +{
>> + struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
>> + int i;
>> +
>> + if (WARN_ON(args->args[0] >= qphy->cfg->nlanes))
>> + return ERR_PTR(-ENODEV);
>> +
>> + for (i = 0; i < qphy->cfg->nlanes; i++) {
>> + if (qphy->phys[i]->index == args->args[0])
>> + break;
>
> finding a PHY based on index is not required. Just have a different label for
> each sub-node and using this label in the controller node should be enough.
Like i said in my comment above, the idea is to avoid any kind of sub-nodes
(child nodes) of the PHY device node. So each lane is registered at
separate indices.
Regards
Vivek
--
Qualcomm Innovation Center, Inc. is a member of 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC 1/5] media: i2c: max2175: Add MAX2175 support
From: Laurent Pinchart @ 2016-11-10 8:46 UTC (permalink / raw)
To: Ramesh Shanmugasundaram
Cc: robh+dt@kernel.org, mark.rutland@arm.com, mchehab@kernel.org,
hverkuil@xs4all.nl, sakari.ailus@linux.intel.com, crope@iki.fi,
Chris Paterson, geert@linux-m68k.org, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <SG2PR06MB1038BC62A3011C8EAB20B61FC3D40@SG2PR06MB1038.apcprd06.prod.outlook.com>
Hi Ramesh,
On Friday 21 Oct 2016 14:49:30 Ramesh Shanmugasundaram wrote:
> > On Wednesday 12 Oct 2016 15:10:25 Ramesh Shanmugasundaram wrote:
> >> This patch adds driver support for MAX2175 chip. This is Maxim
> >> Integrated's RF to Bits tuner front end chip designed for
> >> software-defined radio solutions. This driver exposes the tuner as a
> >> sub-device instance with standard and custom controls to configure the
> >> device.
> >>
> >> Signed-off-by: Ramesh Shanmugasundaram
> >> <ramesh.shanmugasundaram@bp.renesas.com> ---
> >>
> >> .../devicetree/bindings/media/i2c/max2175.txt | 60 +
> >> drivers/media/i2c/Kconfig | 4 +
> >> drivers/media/i2c/Makefile | 2 +
> >> drivers/media/i2c/max2175/Kconfig | 8 +
> >> drivers/media/i2c/max2175/Makefile | 4 +
> >> drivers/media/i2c/max2175/max2175.c | 1624 +++++++++++++
> >> drivers/media/i2c/max2175/max2175.h | 124 ++
> >> 7 files changed, 1826 insertions(+)
> >> create mode 100644
> >> Documentation/devicetree/bindings/media/i2c/max2175.txt
> >> create mode 100644 drivers/media/i2c/max2175/Kconfig create mode
> >> 100644 drivers/media/i2c/max2175/Makefile
> >> create mode 100644 drivers/media/i2c/max2175/max2175.c
> >> create mode 100644 drivers/media/i2c/max2175/max2175.h
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/max2175.txt
> >> b/Documentation/devicetree/bindings/media/i2c/max2175.txt new file
> >> mode 100644
> >> index 0000000..2250d5f
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/max2175.txt
> >> @@ -0,0 +1,60 @@
[snip]
> >> +Optional properties:
> >> +--------------------
> >> +- maxim,slave : empty property indicates this is a slave of
> >> another
> >> + master tuner. This is used to define two tuners in
> >> + diversity mode (1 master, 1 slave). By default each
> >> + tuner is an individual master.
> >
> > Would it be useful to make that property a phandle to the master tuner, to
> > give drivers a way to access the master ? I haven't checked whether there
> > could be use cases for that.
>
> As of now, I cannot find any use case for it from the datasheet. In future,
> we could add if such need arise.
My point is that making the maxim,slave property a phandle now would allow
handling such future cases without any change to the DT bindings.
[snip]
> > > diff --git a/drivers/media/i2c/max2175/max2175.c
> > > b/drivers/media/i2c/max2175/max2175.c new file mode 100644 index
> > > 0000000..71b60c2
> > > --- /dev/null
> > > +++ b/drivers/media/i2c/max2175/max2175.c
[snip]
> > > +static int max2175_poll_csm_ready(struct max2175_ctx *ctx) {
> > > + return max2175_poll_timeout(ctx, 69, 1, 1, 0, 50);
> >
> > Please define macros for register addresses and values, this is just
> > unreadable.
>
> The Tuner provider is unwilling to disclose all register details. I agree on
> the readability issue with this restriction but this is somewhat true for
> some sensitive IPs in the media subsystem.
Is it the case that you don't have access to the information, or that you have
been forbidden to disclose them by the tuner manufacturer ?
> > > +}
[snip]
> > > +static int max2175_set_lo_freq(struct max2175_ctx *ctx, u64 lo_freq)
> > > +{
> > > + int ret;
> > > + u32 lo_mult;
> > > + u64 scaled_lo_freq;
> > > + const u64 scale_factor = 1000000ULL;
> > > + u64 scaled_npf, scaled_integer, scaled_fraction;
> > > + u32 frac_desired, int_desired;
> > > + u8 loband_bits, vcodiv_bits;
> >
> > Do you really support frequencies above 4GHz ?
>
> Nope.
>
> If not most of the 64-bit
>
> > values could be stored in 32 bits.
>
> The 64bit variables are needed to extract the fractional part (upto 6 digit
> precision) out of floating point divisions (original user space code).
OK. The code would be more efficient if you made the scaling factor a power of
two though. 1048576 could be a good value.
> >> +
> >> + scaled_lo_freq = lo_freq;
> >> + /* Scale to larger number for precision */
> >> + scaled_lo_freq = scaled_lo_freq * scale_factor * 100;
I just noticed that you could write the two lines as
scaled_lo_freq = lo_freq * scale_factor * 100;
By the way, why do you multiply by 100 here, and...
> >> + mxm_dbg(ctx, "scaled lo_freq %llu lo_freq %llu\n",
> >> + scaled_lo_freq, lo_freq);
> >> +
> >> + if (MAX2175_IS_BAND_AM(ctx)) {
> >> + if (max2175_get_bit(ctx, 5, 7) == 0)
> >> + loband_bits = 0;
> >> + vcodiv_bits = 0;
> >> + lo_mult = 16;
> >> + } else if (max2175_get_bits(ctx, 5, 1, 0) == MAX2175_BAND_FM) {
> >> + if (lo_freq <= 74700000) {
> >> + loband_bits = 0;
> >> + vcodiv_bits = 0;
> >> + lo_mult = 16;
> >> + } else if ((lo_freq > 74700000) && (lo_freq <= 110000000)) {
> >
> > No need for the inner parentheses.
>
> Agreed.
>
> >> + loband_bits = 1;
> >> + vcodiv_bits = 0;
> >> + } else {
> >> + loband_bits = 1;
> >> + vcodiv_bits = 3;
> >> + }
> >> + lo_mult = 8;
> >> + } else if (max2175_get_bits(ctx, 5, 1, 0) == MAX2175_BAND_VHF) {
> >> + if (lo_freq <= 210000000) {
> >> + loband_bits = 2;
> >> + vcodiv_bits = 2;
> >> + } else {
> >> + loband_bits = 2;
> >> + vcodiv_bits = 1;
> >> + }
> >> + lo_mult = 4;
> >> + } else {
> >> + loband_bits = 3;
> >> + vcodiv_bits = 2;
> >> + lo_mult = 2;
> >> + }
> >> +
> >> + if (max2175_get_bits(ctx, 5, 1, 0) == MAX2175_BAND_L)
> >> + scaled_npf = (scaled_lo_freq / ctx->xtal_freq / lo_mult) /
> >> 100;
> >> + else
> >> + scaled_npf = (scaled_lo_freq / ctx->xtal_freq * lo_mult) /
> >> 100;
... divide by 100 here without using the value in-between nor after ?
> >> +
> >> + scaled_integer = scaled_npf / scale_factor * scale_factor;
> >> + int_desired = (u32)(scaled_npf / scale_factor);
> >> + scaled_fraction = scaled_npf - scaled_integer;
> >> + frac_desired = (u32)(scaled_fraction * 1048576 / scale_factor);
> >> +
> >> + /* Check CSM is not busy */
> >> + ret = max2175_poll_csm_ready(ctx);
> >> + if (ret) {
> >> + v4l2_err(ctx->client, "lo_freq: csm busy. freq %llu\n",
> >> + lo_freq);
> >> + return ret;
> >> + }
> >> +
> >> + mxm_dbg(ctx, "loband %u vcodiv %u lo_mult %u scaled_npf %llu\n",
> >> + loband_bits, vcodiv_bits, lo_mult, scaled_npf);
> >> + mxm_dbg(ctx, "scaled int %llu frac %llu desired int %u frac %u\n",
> >> + scaled_integer, scaled_fraction, int_desired, frac_desired);
> >> +
> >> + /* Write the calculated values to the appropriate registers */
> >> + max2175_set_bits(ctx, 5, 3, 2, loband_bits);
> >> + max2175_set_bits(ctx, 6, 7, 6, vcodiv_bits);
> >> + max2175_set_bits(ctx, 1, 7, 0, (u8)(int_desired & 0xff));
> >> + max2175_set_bits(ctx, 2, 3, 0, (u8)((frac_desired >> 16) & 0x1f));
> >> + max2175_set_bits(ctx, 3, 7, 0, (u8)((frac_desired >> 8) & 0xff));
> >> + max2175_set_bits(ctx, 4, 7, 0, (u8)(frac_desired & 0xff));
> >> + /* Flush the above registers to device */
> >> + max2175_flush_regstore(ctx, 1, 6);
> >> + return ret;
> >> +}
[snip]
> >> +static int max2175_get_lna_gain(struct max2175_ctx *ctx) {
> >> + int gain = 0;
> >> + enum max2175_band band = max2175_get_bits(ctx, 5, 1, 0);
> >> +
> >> + switch (band) {
> >> + case MAX2175_BAND_AM:
> >> + gain = max2175_read_bits(ctx, 51, 3, 1);
> >> + break;
> >> + case MAX2175_BAND_FM:
> >> + gain = max2175_read_bits(ctx, 50, 3, 1);
> >> + break;
> >> + case MAX2175_BAND_VHF:
> >> + gain = max2175_read_bits(ctx, 52, 3, 0);
> >> + break;
> >> + default:
> >> + v4l2_err(ctx->client, "invalid band %d to get rf gain\n",
> >> band);
> >
> > Can this happen ?
>
> Yes, there is "L-band". It is a paranoia check as I am testing by comparing
> logs sometimes :-(
OK. By the way, you could get rid of the gain variable by returning directly
in the case statements.
> >> + break;
> >> + }
> >> + return gain;
> >> +}
[snip]
> >> +static const struct v4l2_ctrl_config max2175_i2s_mode = {
> >> + .ops = &max2175_ctrl_ops,
> >> + .id = V4L2_CID_MAX2175_I2S_MODE,
> >> + .name = "I2S_MODE value",
> >> + .type = V4L2_CTRL_TYPE_INTEGER,
> >
> > Should this be a menu control ?
>
> Hmm... the strings would be named "i2s mode x"? Will that be OK?
How about describing the modes instead ? Are they standardized ?
> >> + .min = 0,
> >> + .max = 4,
> >> + .step = 1,
> >> + .def = 0,
> >> +};
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang.yuan @ 2016-11-10 8:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Mark Rutland
Cc: catalin.marinas, will.deacon, robh+dt, bhelgaas, olof, arnd,
linux-arm-kernel, lorenzo.pieralisi, linux-kernel, linuxarm,
devicetree, linux-pci, linux-serial, minyard, liviu.dudau,
zourongrong, john.garry, gabriele.paoloni, zhichang.yuan02,
kantyzc, xuwei5, marc.zyngier
In-Reply-To: <1478647002.7430.69.camel@kernel.crashing.org>
Hi, Ben,
On 2016/11/9 7:16, Benjamin Herrenschmidt wrote:
> On Tue, 2016-11-08 at 12:03 +0000, Mark Rutland wrote:
>> On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
>>>
>>> For arm64, there is no I/O space as other architectural platforms, such as
>>> X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
>>> such as Hip06, when accessing some legacy ISA devices connected to LPC, those
>>> known port addresses are used to control the corresponding target devices, for
>>> example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
>>> normal MMIO mode in using.
>>
>> This has nothing to do with arm64. Hardware with this kind of indirect
>> bus access could be integrated with a variety of CPU architectures. It
>> simply hasn't been, yet.
>
> On some ppc's we also use similar indirect access methods for IOs. We
> have a generic infrastructure for re-routing some memory or IO regions
> to hooks.
>
I am interested on the generic infrastructure on PPC.
Could you point out where those drivers are?
want to take a look..
Thanks,
Zhichang
> On POWER8, our PCIe doesn't do IO at all, but we have an LPC bus behind
> firmware calls ;-) We use that infrastructure to plumb in the LPC bus.
>
>>> To drive these devices, this patch introduces a method named indirect-IO.
>>> In this method the in/out pair in arch/arm64/include/asm/io.h will be
>>> redefined. When upper layer drivers call in/out with those known legacy port
>>> addresses to access the peripherals, the hooking functions corrresponding to
>>> those target peripherals will be called. Through this way, those upper layer
>>> drivers which depend on in/out can run on Hip06 without any changes.
>>
>> As above, this has nothing to do with arm64, and as such, should live in
>> generic code, exactly as we would do if we had higher-level ISA
>> accessor ops.
>>
>> Regardless, given the multi-instance case, I don't think this is
>> sufficient in general (and I think we need higher-level ISA accessors
>> to handle the indirection).
>
> Multi-instance with IO is tricky to do generically because archs already
> have all sort of hacks to deal with the fact that inb/outb don't require
> an explicit ioremap, so an IO resource can take all sort of shape depending
> on the arch.
>
> Overall it boils down to applying some kind of per-instance "offset" to
> the IO port number though.
>
^ permalink raw reply
* Re: [PATCH 04/14] ARM: dts: armada-375: Fixup bootrom DT warning
From: Thomas Petazzoni @ 2016-11-10 8:22 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Andrew Lunn, Jason Cooper, devicetree, Rob Herring,
linux-arm-kernel, Sebastian Hesselbarth
In-Reply-To: <20161110001000.10619-5-gregory.clement@free-electrons.com>
Hello,
On Thu, 10 Nov 2016 01:09:50 +0100, Gregory CLEMENT wrote:
> - bootrom {
> + bootrom@0 {
> compatible = "marvell,bootrom";
> reg = <MBUS_ID(0x01, 0x1d) 0 0x100000>;
I am still not sure whether this "0" unit address is correct compared
to the reg property being passed.
A good example of why I'm worried is the sa-sram case:
+ crypto_sram0: sa-sram0@0 {
compatible = "mmio-sram";
reg = <MBUS_ID(0x09, 0x09) 0 0x800>;
+ crypto_sram1: sa-sram1@0 {
compatible = "mmio-sram";
reg = <MBUS_ID(0x09, 0x05) 0 0x800>;
The node names should be just "sram" without a number. Indeed for UARTs
for example, you use uart@XYZ, uart@ABC and not uart0@XYZ and
uart1@ABC. But then, if you do that, with your scheme, you end up with
both nodes named sa-sram@0.
Which clearly shows that the way you set this unit-address is not
correct: those two devices are mapped at completely different
locations, but you end up with an identical unit address.
I have no idea what is the rule for setting the unit address in this
case, but I'm pretty sure the rule you've chosen is not good.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ 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