* Re: [PATCH v3 6/6] mfd: dt: Move syscon bindings to syscon subdirectory
From: Andrew Jeffery @ 2016-12-13 12:05 UTC (permalink / raw)
To: Lee Jones
Cc: Mark Rutland, Corey Minyard, devicetree, Linus Walleij,
linux-kernel, Cédric Le Goater, linux-arm-kernel,
Joel Stanley
In-Reply-To: <20161213110710.GV3625@dell.home>
[-- Attachment #1.1: Type: text/plain, Size: 2716 bytes --]
On Tue, 2016-12-13 at 11:07 +0000, Lee Jones wrote:
> On Tue, 13 Dec 2016, Andrew Jeffery wrote:
>
> > On Mon, 2016-12-12 at 09:39 -0600, Rob Herring wrote:
> > > On Tue, Dec 06, 2016 at 01:53:21PM +1100, Andrew Jeffery wrote:
> > > > The use of syscons is growing, lets collate them in their own part of
> > > > the bindings tree.
> > > >
> > > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > >
> > > > ---
> > > > Documentation/devicetree/bindings/mfd/{ => syscon}/aspeed-scu.txt | 0
> > > > Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-gpbr.txt | 0
> > > > Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-matrix.txt | 0
> > > > Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-smc.txt | 0
> > > > Documentation/devicetree/bindings/mfd/{ => syscon}/qcom,tcsr.txt | 0
> > > > Documentation/devicetree/bindings/mfd/{ => syscon}/syscon.txt | 0
> > > > .../devicetree/bindings/mfd/{ => syscon}/ti-keystone-devctrl.txt | 0
> > > > 7 files changed, 0 insertions(+), 0 deletions(-)
> > > > rename Documentation/devicetree/bindings/mfd/{ => syscon}/aspeed-scu.txt (100%)
> > > > rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-gpbr.txt (100%)
> > > > rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-matrix.txt (100%)
> > > > rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-smc.txt (100%)
> > > > rename Documentation/devicetree/bindings/mfd/{ => syscon}/qcom,tcsr.txt (100%)
> > > > rename Documentation/devicetree/bindings/mfd/{ => syscon}/syscon.txt (100%)
> > > > rename Documentation/devicetree/bindings/mfd/{ => syscon}/ti-keystone-devctrl.txt (100%)
> > >
> > > I'm not so sure this is the right direction. syscon usage is pretty much
> > > spread throughout the tree.
> >
> > This patch was created based on my interpretation of Lee's feedback
> > here:
> >
> > https://lkml.org/lkml/2016/11/18/650
> >
> > Lee's next email in the chain poked Arnd for an opinion, but Arnd
> > didn't reply.
> >
> > I don't mind. I moved these bindings separately so we could just drop
> > the patch if there was push-back. If we drop the whole idea I'll need
> > to apply a small fix to patch 5/6 to avoid creating the syscon
> > subdirectory.
>
> The sub-directory is a good idea for drivers who are *solely* syscon
> based.
>
Yes, I wasn't saying otherwise, just commenting on my motivation and
approach.
As far as I can tell all of the bindings I move here describe solely
syscon-based devices.
Cheers,
Andrew
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 6/6] mfd: dt: Move syscon bindings to syscon subdirectory
From: Arnd Bergmann @ 2016-12-13 12:17 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Andrew Jeffery, Lee Jones, Mark Rutland, Corey Minyard,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Cédric Le Goater,
Joel Stanley
In-Reply-To: <1481630734.3112.40.camel-zrmu5oMJ5Fs@public.gmane.org>
On Tuesday, December 13, 2016 10:35:34 PM CET Andrew Jeffery wrote:
> On Tue, 2016-12-13 at 11:07 +0000, Lee Jones wrote:
> > On Tue, 13 Dec 2016, Andrew Jeffery wrote:
> > > On Mon, 2016-12-12 at 09:39 -0600, Rob Herring wrote:
> > > > On Tue, Dec 06, 2016 at 01:53:21PM +1100, Andrew Jeffery wrote:
> > >
> > > Lee's next email in the chain poked Arnd for an opinion, but Arnd
> > > didn't reply.
> > >
> > > I don't mind. I moved these bindings separately so we could just drop
> > > the patch if there was push-back. If we drop the whole idea I'll need
> > > to apply a small fix to patch 5/6 to avoid creating the syscon
> > > subdirectory.
> >
> > The sub-directory is a good idea for drivers who are *solely* syscon
> > based.
> >
>
> Yes, I wasn't saying otherwise, just commenting on my motivation and
> approach.
>
> As far as I can tell all of the bindings I move here describe solely
> syscon-based devices.
>
But do we know which ones they are?
In principle, any syscon device node can have a specialized driver
exporting an interface, the bindings always allow it to be done
one way or the other, and we may change the driver or run a different
OS that has decided differently.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting
From: Hans Verkuil @ 2016-12-13 12:20 UTC (permalink / raw)
To: Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-mmc, linux-pm, linux-usb, Ulf Hansson,
Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Greg Kroah-Hartman, Mark Brown
Cc: tjakobi, m.szyprowski, Bartlomiej Zolnierkiewicz
In-Reply-To: <1462451666-17945-1-git-send-email-k.kozlowski@samsung.com>
Hi Krzysztof,
This still seems to be broken with the latest 4.9 kernel, right?
Has there been any progress on this? Do you have an updated patch series
for me to use?
Regards,
Hans
On 05/05/16 14:34, Krzysztof Kozlowski wrote:
> Hi,
>
> This is a different, second try to fix usb3503+lan on Odroid U3 board
> if it was initialized by bootloader (e.g. for TFTP boot).
>
> First version:
> http://www.spinics.net/lists/linux-usb/msg140042.html
>
>
> Problem
> =======
> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
> is required, e.g. by suspend to RAM. The actual TFTP boot does
> not have to happen. Just "usb start" from U-Boot is sufficient.
>
^ permalink raw reply
* Re: [PATCH v4 1/4] mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
From: Cédric Le Goater @ 2016-12-13 12:22 UTC (permalink / raw)
To: Marek Vasut, linux-mtd
Cc: Mark Rutland, Boris Brezillon, devicetree, Richard Weinberger,
Rob Herring, Joel Stanley, Cyrille Pitchen, Brian Norris,
David Woodhouse
In-Reply-To: <3f3260d1-e677-b6d9-5571-a08a80344495@gmail.com>
On 12/13/2016 08:50 AM, Marek Vasut wrote:
> On 12/12/2016 04:40 PM, Cédric Le Goater wrote:
>> This driver adds mtd support for the Aspeed AST2500 SoC static memory
>> controllers :
>
> [...]
>
>> +#define DEVICE_NAME "aspeed-smc"
>> +
>> +/*
>> + * The driver only support SPI flash
>> + */
>> +enum aspeed_smc_flash_type {
>> + smc_type_nor = 0,
>> + smc_type_nand = 1,
>> + smc_type_spi = 2,
>> +};
>
> So why is this here ? :)
well, because we do enforce the SPI type in aspeed_smc_chip_set_type()
but I agree, we could be a bit more direct with it and just write
the SPI type. May be in a followup patch then.
>
>> +struct aspeed_smc_chip;
>> +
>> +struct aspeed_smc_info {
>> + u32 maxsize; /* maximum size of chip window */
>> + u8 nce; /* number of chip enables */
>> + bool hastype; /* flash type field exists in config reg */
>> + u8 we0; /* shift for write enable bit for CE0 */
>> + u8 ctl0; /* offset in regs of ctl for CE0 */
>> +
>> + void (*set_4b)(struct aspeed_smc_chip *chip);
>> +};
>
> Otherwise looks good:
> Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
>
Thanks for the review,
C.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting
From: Hans Verkuil @ 2016-12-13 12:34 UTC (permalink / raw)
To: Krzysztof Kozlowski, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-mmc, linux-pm, linux-usb, Ulf Hansson,
Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Greg Kroah-Hartman, Mark Brown
Cc: tjakobi, m.szyprowski, Bartlomiej Zolnierkiewicz
In-Reply-To: <c956e27e-7e01-2a2f-0e35-796c4a6556ae@xs4all.nl>
Try again, this time with Krzysztof's new email address...
On 13/12/16 13:20, Hans Verkuil wrote:
> Hi Krzysztof,
>
> This still seems to be broken with the latest 4.9 kernel, right?
>
> Has there been any progress on this? Do you have an updated patch series
> for me to use?
>
> Regards,
>
> Hans
>
> On 05/05/16 14:34, Krzysztof Kozlowski wrote:
>> Hi,
>>
>> This is a different, second try to fix usb3503+lan on Odroid U3 board
>> if it was initialized by bootloader (e.g. for TFTP boot).
>>
>> First version:
>> http://www.spinics.net/lists/linux-usb/msg140042.html
>>
>>
>> Problem
>> =======
>> When Odroid U3 (usb3503 + smsc95xx + max77686) boots from network (TFTP),
>> the usb3503 and LAN smsc95xx do not show up in "lsusb". Hard-reset
>> is required, e.g. by suspend to RAM. The actual TFTP boot does
>> not have to happen. Just "usb start" from U-Boot is sufficient.
>>
>
>
^ permalink raw reply
* Re: [PATCH v3 6/6] mfd: dt: Move syscon bindings to syscon subdirectory
From: Andrew Jeffery @ 2016-12-13 12:39 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Lee Jones, Mark Rutland, Corey Minyard,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Cédric Le Goater,
Joel Stanley
In-Reply-To: <5094120.Y7DXfRGTEm@wuerfel>
[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]
On Tue, 2016-12-13 at 13:17 +0100, Arnd Bergmann wrote:
> On Tuesday, December 13, 2016 10:35:34 PM CET Andrew Jeffery wrote:
> > On Tue, 2016-12-13 at 11:07 +0000, Lee Jones wrote:
> > > On Tue, 13 Dec 2016, Andrew Jeffery wrote:
> > > > On Mon, 2016-12-12 at 09:39 -0600, Rob Herring wrote:
> > > > > On Tue, Dec 06, 2016 at 01:53:21PM +1100, Andrew Jeffery wrote:
> > > >
> > > > Lee's next email in the chain poked Arnd for an opinion, but Arnd
> > > > didn't reply.
> > > >
> > > > I don't mind. I moved these bindings separately so we could just drop
> > > > the patch if there was push-back. If we drop the whole idea I'll need
> > > > to apply a small fix to patch 5/6 to avoid creating the syscon
> > > > subdirectory.
> > >
> > > The sub-directory is a good idea for drivers who are *solely* syscon
> > > based.
> > >
> >
> > Yes, I wasn't saying otherwise, just commenting on my motivation and
> > approach.
> >
> > As far as I can tell all of the bindings I move here describe solely
> > syscon-based devices.
> >
>
> But do we know which ones they are?
>
> In principle, any syscon device node can have a specialized driver
> exporting an interface, the bindings always allow it to be done
> one way or the other, and we may change the driver or run a different
> OS that has decided differently.
>
Right; for the Linux case there are currently no driver implementations
that match on the compatible strings in the documents I moved (save for
qcom,tcsr, except that it's the qcom,gsbi compatible driver parsing a
phandle to the qcom,tcsr syscon node).
However, I can't guarantee the solely-syscon property for other
operating systems. Given that, it now looks to me like we shouldn't
have such a directory at all.
Cheers,
Andrew
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/9] ARM: dts: dra72: Add separate dtsi for tps65917
From: Roger Quadros @ 2016-12-13 12:40 UTC (permalink / raw)
To: Lokesh Vutla, Tony Lindgren, Linux OMAP Mailing List,
Tomi Valkeinen, KISHON VIJAY ABRAHAM
Cc: Tero Kristo, Sekhar Nori, Nishanth Menon,
Device Tree Mailing List, Rob Herring, Linux ARM Mailing List,
Carlos Hernandez
In-Reply-To: <20161021103841.8044-4-lokeshvutla-l0cyMroinI0@public.gmane.org>
+Tomi, Kishon, Carlos
Hi,
On 21/10/16 13:38, Lokesh Vutla wrote:
> dra72-evm-common.dtsi consolidates dra72-evm.dts and dra72-evm-revc.dts
> which also include tps65917 pmic support as both the evms uses the same
> pmic. But, dra71-evm has mostly similar features with a different pmic.
> In order to exploit dra72-evm-common.dtsi, creating a separate dtsi
> for tps65915 support and including it in respective board files.
>
> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
> ---
> arch/arm/boot/dts/dra72-evm-common.dtsi | 128 ----------------------------
> arch/arm/boot/dts/dra72-evm-revc.dts | 21 +++--
> arch/arm/boot/dts/dra72-evm-tps65917.dtsi | 134 ++++++++++++++++++++++++++++++
> arch/arm/boot/dts/dra72-evm.dts | 14 ++--
> 4 files changed, 154 insertions(+), 143 deletions(-)
> create mode 100644 arch/arm/boot/dts/dra72-evm-tps65917.dtsi
>
This patch breaks USB XHCI and boot on dra72-evm (both revC and non revC)
I'll explain why below.
[ 13.625167] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
[ 13.632557] pgd = ede10000
[ 13.635390] [00000000] *pgd=00000000
[ 13.639145] Internal error: : 1406 [#1] SMP ARM
[ 13.643893] Modules linked in: xhci_plat_hcd(+) xhci_hcd usbcore omapfb dwc3 cfbfillrect snd_soc_davinci_mcasp cfbimgblt cfbcopyarea udc_core connector_hdmi encoder_tpd12s015 snd_soc_edma m25p80 snd_soc_simpe
[ 13.695557] CPU: 0 PID: 440 Comm: modprobe Not tainted 4.9.0-rc1 #1050
[ 13.702399] Hardware name: Generic DRA72X (Flattened Device Tree)
[ 13.708786] task: edb5c040 task.stack: edd10000
[ 13.713540] PC is at _raw_spin_unlock_irqrestore+0x0/0x44
[ 13.719219] LR is at xhci_hub_control+0xc2c/0x15e0 [xhci_hcd]
[ 13.725242] pc : [<c07df718>] lr : [<bf486300>] psr: a0000093
[ 13.725242] sp : edd118c0 ip : c0e306b4 fp : 00000000
[ 13.737278] r10: 00000000 r9 : 60000013 r8 : edf28218
[ 13.742753] r7 : edf28000 r6 : 00000000 r5 : 00000000 r4 : edf2a000
[ 13.749593] r3 : 00000000 r2 : 00000000 r1 : 60000013 r0 : edf28218
> diff --git a/arch/arm/boot/dts/dra72-evm-common.dtsi b/arch/arm/boot/dts/dra72-evm-common.dtsi
> index 8537b6a..9903ac7 100644
> --- a/arch/arm/boot/dts/dra72-evm-common.dtsi
> +++ b/arch/arm/boot/dts/dra72-evm-common.dtsi
> @@ -214,123 +214,6 @@
> status = "okay";
> clock-frequency = <400000>;
>
> - tps65917: tps65917@58 {
> - compatible = "ti,tps65917";
> - reg = <0x58>;
> -
> - interrupts = <GIC_SPI 2 IRQ_TYPE_NONE>; /* IRQ_SYS_1N */
> - interrupt-controller;
> - #interrupt-cells = <2>;
> -
> - ti,system-power-controller;
> -
> - tps65917_pmic {
> - compatible = "ti,tps65917-pmic";
> -
> - smps1-in-supply = <&vsys_3v3>;
> - smps2-in-supply = <&vsys_3v3>;
> - smps3-in-supply = <&vsys_3v3>;
> - smps4-in-supply = <&vsys_3v3>;
> - smps5-in-supply = <&vsys_3v3>;
> - ldo1-in-supply = <&vsys_3v3>;
> - ldo2-in-supply = <&vsys_3v3>;
> - ldo3-in-supply = <&vsys_3v3>;
> - ldo4-in-supply = <&evm_5v0>;
> - ldo5-in-supply = <&vsys_3v3>;
> -
> - tps65917_regulators: regulators {
> - smps1_reg: smps1 {
> - /* VDD_MPU */
> - regulator-name = "smps1";
> - regulator-min-microvolt = <850000>;
> - regulator-max-microvolt = <1250000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> -
> - smps2_reg: smps2 {
> - /* VDD_CORE */
> - regulator-name = "smps2";
> - regulator-min-microvolt = <850000>;
> - regulator-max-microvolt = <1150000>;
> - regulator-boot-on;
> - regulator-always-on;
> - };
> -
> - smps3_reg: smps3 {
> - /* VDD_GPU IVA DSPEVE */
> - regulator-name = "smps3";
> - regulator-min-microvolt = <850000>;
> - regulator-max-microvolt = <1250000>;
> - regulator-boot-on;
> - regulator-always-on;
> - };
> -
> - smps4_reg: smps4 {
> - /* VDDS1V8 */
> - regulator-name = "smps4";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> -
> - smps5_reg: smps5 {
> - /* VDD_DDR */
> - regulator-name = "smps5";
> - regulator-min-microvolt = <1350000>;
> - regulator-max-microvolt = <1350000>;
> - regulator-boot-on;
> - regulator-always-on;
> - };
> -
> - ldo1_reg: ldo1 {
> - /* LDO1_OUT --> SDIO */
> - regulator-name = "ldo1";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-always-on;
> - regulator-boot-on;
> - regulator-allow-bypass;
> - };
> -
> - ldo3_reg: ldo3 {
> - /* VDDA_1V8_PHY */
> - regulator-name = "ldo3";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-boot-on;
> - regulator-always-on;
> - };
> -
> - ldo5_reg: ldo5 {
> - /* VDDA_1V8_PLL */
> - regulator-name = "ldo5";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - regulator-boot-on;
> - };
> -
> - ldo4_reg: ldo4 {
> - /* VDDA_3V_USB: VDDA_USBHS33 */
> - regulator-name = "ldo4";
> - regulator-min-microvolt = <3300000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-boot-on;
> - };
> - };
> - };
> -
> - tps65917_power_button {
> - compatible = "ti,palmas-pwrbutton";
> - interrupt-parent = <&tps65917>;
> - interrupts = <1 IRQ_TYPE_NONE>;
> - wakeup-source;
> - ti,palmas-long-press-seconds = <6>;
> - };
> - };
> -
> pcf_gpio_21: gpio@21 {
> compatible = "ti,pcf8575", "nxp,pcf8575";
> reg = <0x21>;
> @@ -480,14 +363,6 @@
> };
> };
>
> -&usb2_phy1 {
> - phy-supply = <&ldo4_reg>;
> -};
> -
> -&usb2_phy2 {
> - phy-supply = <&ldo4_reg>;
> -};
> -
You remove this here but don't add them in the board dts files.
> &omap_dwc3_1 {
> extcon = <&extcon_usb1>;
> };
> @@ -509,7 +384,6 @@
> pinctrl-names = "default";
> pinctrl-0 = <&mmc1_pins_default>;
> vmmc-supply = <&evm_3v3_sd>;
> - vmmc_aux-supply = <&ldo1_reg>;
What about this?
> bus-width = <4>;
> /*
> * SDCD signal is not being used here - using the fact that GPIO mode
> @@ -606,8 +480,6 @@
>
> &dss {
> status = "ok";
> -
> - vdda_video-supply = <&ldo5_reg>;
and this?
> };
>
> &hdmi {
> diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
> index 064b322..4ea2a0c 100644
> --- a/arch/arm/boot/dts/dra72-evm-revc.dts
> +++ b/arch/arm/boot/dts/dra72-evm-revc.dts
> @@ -17,17 +17,22 @@
> };
> };
>
> -&tps65917_regulators {
> - ldo2_reg: ldo2 {
> - /* LDO2_OUT --> VDDA_1V8_PHY2 */
> - regulator-name = "ldo2";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <1800000>;
> - regulator-always-on;
> - regulator-boot-on;
> +&i2c1 {
> + tps65917: tps65917@58 {
> + reg = <0x58>;
> +
> + interrupts = <GIC_SPI 2 IRQ_TYPE_NONE>; /* IRQ_SYS_1N */
> };
> };
>
> +#include "dra72-evm-tps65917.dtsi"
> +
> +&ldo2_reg {
> + /* LDO2_OUT --> VDDA_1V8_PHY2 */
> + regulator-always-on;
> + regulator-boot-on;
> +};
> +
Here you need to add the usb2_phy1 & 2 supplies.
> &hdmi {
> vdda-supply = <&ldo2_reg>;
> };
> diff --git a/arch/arm/boot/dts/dra72-evm-tps65917.dtsi b/arch/arm/boot/dts/dra72-evm-tps65917.dtsi
> new file mode 100644
> index 0000000..ee6dac4
> --- /dev/null
> +++ b/arch/arm/boot/dts/dra72-evm-tps65917.dtsi
> @@ -0,0 +1,134 @@
> +/*
> + * Copyright (C) 2016 Texas Instruments Incorporated - http://www.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.
> + */
> +
> +/*
> + * Integrated Power Management Chip
> + * http://www.ti.com/lit/ds/symlink/tps65917-q1.pdf
> + */
> +
> +&tps65917 {
> + compatible = "ti,tps65917";
> +
> + interrupt-controller;
> + #interrupt-cells = <2>;
> +
> + ti,system-power-controller;
> +
> + tps65917_pmic {
> + compatible = "ti,tps65917-pmic";
> +
> + smps1-in-supply = <&vsys_3v3>;
> + smps2-in-supply = <&vsys_3v3>;
> + smps3-in-supply = <&vsys_3v3>;
> + smps4-in-supply = <&vsys_3v3>;
> + smps5-in-supply = <&vsys_3v3>;
> + ldo1-in-supply = <&vsys_3v3>;
> + ldo2-in-supply = <&vsys_3v3>;
> + ldo3-in-supply = <&vsys_3v3>;
> + ldo4-in-supply = <&evm_5v0>;
> + ldo5-in-supply = <&vsys_3v3>;
> +
> + tps65917_regulators: regulators {
> + smps1_reg: smps1 {
> + /* VDD_MPU */
> + regulator-name = "smps1";
> + regulator-min-microvolt = <850000>;
> + regulator-max-microvolt = <1250000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + smps2_reg: smps2 {
> + /* VDD_CORE */
> + regulator-name = "smps2";
> + regulator-min-microvolt = <850000>;
> + regulator-max-microvolt = <1150000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + smps3_reg: smps3 {
> + /* VDD_GPU IVA DSPEVE */
> + regulator-name = "smps3";
> + regulator-min-microvolt = <850000>;
> + regulator-max-microvolt = <1250000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + smps4_reg: smps4 {
> + /* VDDS1V8 */
> + regulator-name = "smps4";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + smps5_reg: smps5 {
> + /* VDD_DDR */
> + regulator-name = "smps5";
> + regulator-min-microvolt = <1350000>;
> + regulator-max-microvolt = <1350000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + ldo1_reg: ldo1 {
> + /* LDO1_OUT --> SDIO */
> + regulator-name = "ldo1";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-allow-bypass;
> + };
> +
> + ldo2_reg: ldo2 {
> + regulator-name = "ldo2";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-allow-bypass;
> + };
> +
> + ldo3_reg: ldo3 {
> + /* VDDA_1V8_PHY */
> + regulator-name = "ldo3";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> + ldo5_reg: ldo5 {
> + /* VDDA_1V8_PLL */
> + regulator-name = "ldo5";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + regulator-boot-on;
> + };
> +
> + ldo4_reg: ldo4 {
> + /* VDDA_3V_USB: VDDA_USBHS33 */
> + regulator-name = "ldo4";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-boot-on;
> + };
> + };
> + };
> +
> + tps65917_power_button {
> + compatible = "ti,palmas-pwrbutton";
> + interrupt-parent = <&tps65917>;
> + interrupts = <1 IRQ_TYPE_NONE>;
> + wakeup-source;
> + ti,palmas-long-press-seconds = <6>;
> + };
> +};
> diff --git a/arch/arm/boot/dts/dra72-evm.dts b/arch/arm/boot/dts/dra72-evm.dts
> index e3a9b69..cd9c4ff 100644
> --- a/arch/arm/boot/dts/dra72-evm.dts
> +++ b/arch/arm/boot/dts/dra72-evm.dts
> @@ -15,16 +15,16 @@
> };
> };
>
> -&tps65917_regulators {
> - ldo2_reg: ldo2 {
> - /* LDO2_OUT --> TP1017 (UNUSED) */
> - regulator-name = "ldo2";
> - regulator-min-microvolt = <1800000>;
> - regulator-max-microvolt = <3300000>;
> - regulator-allow-bypass;
> +&i2c1 {
> + tps65917: tps65917@58 {
> + reg = <0x58>;
> +
> + interrupts = <GIC_SPI 2 IRQ_TYPE_NONE>; /* IRQ_SYS_1N */
> };
> };
>
> +#include "dra72-evm-tps65917.dtsi"
> +
> &hdmi {
> vdda-supply = <&ldo3_reg>;
> };
>
Here as well you need to add the usb2_phy1 & 2 supplies.
We probably need to add dss and mmc supplies as well to both the board dts files?
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 0/3] arm64/clk: update Marvell Armada CP110 system controller driver
From: Thomas Petazzoni @ 2016-12-13 12:41 UTC (permalink / raw)
To: devicetree, Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland,
Kumar Gala, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Michael Turquette, Stephen Boyd, linux-clk
Cc: Marcin Wojtas, Nadav Haklai, Hanna Hawa, Yehuda Yitschak,
linux-arm-kernel, Thomas Petazzoni
Hello,
This small set of commits updates the Marvell Armada CP110 system
controller driver, its Device Tree binding, and Device Tree
representation, to take into account two new things:
- The clock driver now handles clock n°9 (GOP) as a child of clock
n°18 (controls SD/MMC and GOP)
- The DT representation is adjusted to name clock n°18 "sd-mmc-gop"
instead of just "sd-mmc".
This set of commits is some preparation work to add networking support
for Marvell Armada 7K/8K.
I would expect patches 1 and 2 to be taken by the clock maintainers,
and patch 3 be taken by the Marvell EBU maintainers.
Thanks,
Thomas
Thomas Petazzoni (3):
dt-bindings: arm: update Armada CP110 system controller binding
clk: mvebu: adjust clock handling for the CP110 system controller
arm64: dts: marvell: adjust name of sd-mmc-gop clock in syscon
.../devicetree/bindings/arm/marvell/cp110-system-controller0.txt | 6 +++---
arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 2 +-
arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 2 +-
drivers/clk/mvebu/cp110-system-controller.c | 6 ++++--
4 files changed, 9 insertions(+), 7 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/3] dt-bindings: arm: update Armada CP110 system controller binding
From: Thomas Petazzoni @ 2016-12-13 12:41 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Gregory Clement, Michael Turquette,
Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA
Cc: Marcin Wojtas, Nadav Haklai, Hanna Hawa, Yehuda Yitschak,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni
In-Reply-To: <1481632880-9198-1-git-send-email-thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
It turns out that in the CP110 HW block present in Marvell Armada
7K/8K SoCs, gatable clock n°18 not only controls SD/MMC, but also the
GOP block. This commit updates the Device Tree binding for this piece
of hardware accordingly.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
.../devicetree/bindings/arm/marvell/cp110-system-controller0.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt
index 30c5469..07dbb35 100644
--- a/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt
+++ b/Documentation/devicetree/bindings/arm/marvell/cp110-system-controller0.txt
@@ -45,7 +45,7 @@ The following clocks are available:
- 1 15 SATA
- 1 16 SATA USB
- 1 17 Main
- - 1 18 SD/MMC
+ - 1 18 SD/MMC/GOP
- 1 21 Slow IO (SPI, NOR, BootROM, I2C, UART)
- 1 22 USB3H0
- 1 23 USB3H1
@@ -65,7 +65,7 @@ Required properties:
"cpm-audio", "cpm-communit", "cpm-nand", "cpm-ppv2", "cpm-sdio",
"cpm-mg-domain", "cpm-mg-core", "cpm-xor1", "cpm-xor0", "cpm-gop-dp", "none",
"cpm-pcie_x10", "cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor", "cpm-sata",
- "cpm-sata-usb", "cpm-main", "cpm-sd-mmc", "none", "none", "cpm-slow-io",
+ "cpm-sata-usb", "cpm-main", "cpm-sd-mmc-gop", "none", "none", "cpm-slow-io",
"cpm-usb3h0", "cpm-usb3h1", "cpm-usb3dev", "cpm-eip150", "cpm-eip197";
Example:
@@ -78,6 +78,6 @@ Example:
gate-clock-output-names = "cpm-audio", "cpm-communit", "cpm-nand", "cpm-ppv2", "cpm-sdio",
"cpm-mg-domain", "cpm-mg-core", "cpm-xor1", "cpm-xor0", "cpm-gop-dp", "none",
"cpm-pcie_x10", "cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor", "cpm-sata",
- "cpm-sata-usb", "cpm-main", "cpm-sd-mmc", "none", "none", "cpm-slow-io",
+ "cpm-sata-usb", "cpm-main", "cpm-sd-mmc-gop", "none", "none", "cpm-slow-io",
"cpm-usb3h0", "cpm-usb3h1", "cpm-usb3dev", "cpm-eip150", "cpm-eip197";
};
--
2.7.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
* [PATCH 2/3] clk: mvebu: adjust clock handling for the CP110 system controller
From: Thomas Petazzoni @ 2016-12-13 12:41 UTC (permalink / raw)
To: devicetree, Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland,
Kumar Gala, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Michael Turquette, Stephen Boyd, linux-clk
Cc: Marcin Wojtas, Nadav Haklai, Hanna Hawa, Yehuda Yitschak,
linux-arm-kernel, Thomas Petazzoni
In-Reply-To: <1481632880-9198-1-git-send-email-thomas.petazzoni@free-electrons.com>
This commit adds support for the GOP_DP gatable clock found in the
CP110 system controller. This clock is controlled by bit 9, and has
clock 18 as its parent. Therefore, clock 18 is in fact not only
controlling SD/MMC, but also GOP, but it is renamed as SD_MMC_GOP
accordingly.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
drivers/clk/mvebu/cp110-system-controller.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/mvebu/cp110-system-controller.c b/drivers/clk/mvebu/cp110-system-controller.c
index f2303da..3c91cab 100644
--- a/drivers/clk/mvebu/cp110-system-controller.c
+++ b/drivers/clk/mvebu/cp110-system-controller.c
@@ -66,6 +66,7 @@ enum {
#define CP110_GATE_SDIO 4
#define CP110_GATE_XOR1 7
#define CP110_GATE_XOR0 8
+#define CP110_GATE_GOP_DP 9
#define CP110_GATE_PCIE_X1_0 11
#define CP110_GATE_PCIE_X1_1 12
#define CP110_GATE_PCIE_X4 13
@@ -73,7 +74,7 @@ enum {
#define CP110_GATE_SATA 15
#define CP110_GATE_SATA_USB 16
#define CP110_GATE_MAIN 17
-#define CP110_GATE_SDMMC 18
+#define CP110_GATE_SDMMC_GOP 18
#define CP110_GATE_SLOW_IO 21
#define CP110_GATE_USB3H0 22
#define CP110_GATE_USB3H1 23
@@ -309,9 +310,10 @@ static int cp110_syscon_clk_probe(struct platform_device *pdev)
parent = ppv2_name;
break;
case CP110_GATE_SDIO:
+ case CP110_GATE_GOP_DP:
of_property_read_string_index(np,
"gate-clock-output-names",
- CP110_GATE_SDMMC, &parent);
+ CP110_GATE_SDMMC_GOP, &parent);
break;
case CP110_GATE_XOR1:
case CP110_GATE_XOR0:
--
2.7.4
^ permalink raw reply related
* [PATCH 3/3] arm64: dts: marvell: adjust name of sd-mmc-gop clock in syscon
From: Thomas Petazzoni @ 2016-12-13 12:41 UTC (permalink / raw)
To: devicetree, Rob Herring, Ian Campbell, Pawel Moll, Mark Rutland,
Kumar Gala, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory Clement, Michael Turquette, Stephen Boyd, linux-clk
Cc: Marcin Wojtas, Nadav Haklai, Hanna Hawa, Yehuda Yitschak,
linux-arm-kernel, Thomas Petazzoni
In-Reply-To: <1481632880-9198-1-git-send-email-thomas.petazzoni@free-electrons.com>
This commit adjusts the names of gatable clock #18 of the Marvell Armada
CP110 system controller. This clock not only controls SD/MMC, but also
the GOP (Group Of Ports) used for networking. So the clock is renamed to
{cpm,cps}-sd-mmc-gop instead of {cpm,cps}-sd-mmc.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi | 2 +-
arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
index 602e2c2..895babc 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
@@ -74,7 +74,7 @@
"cpm-gop-dp", "none", "cpm-pcie_x10",
"cpm-pcie_x11", "cpm-pcie_x4", "cpm-pcie-xor",
"cpm-sata", "cpm-sata-usb", "cpm-main",
- "cpm-sd-mmc", "none", "none",
+ "cpm-sd-mmc-gop", "none", "none",
"cpm-slow-io", "cpm-usb3h0", "cpm-usb3h1",
"cpm-usb3dev", "cpm-eip150", "cpm-eip197";
};
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
index 6bf9e24..db99646 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
@@ -74,7 +74,7 @@
"cps-gop-dp", "none", "cps-pcie_x10",
"cps-pcie_x11", "cps-pcie_x4", "cps-pcie-xor",
"cps-sata", "cps-sata-usb", "cps-main",
- "cps-sd-mmc", "none", "none",
+ "cps-sd-mmc-gop", "none", "none",
"cps-slow-io", "cps-usb3h0", "cps-usb3h1",
"cps-usb3dev", "cps-eip150", "cps-eip197";
};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 3/9] ARM: dts: dra72: Add separate dtsi for tps65917
From: Lokesh Vutla @ 2016-12-13 13:08 UTC (permalink / raw)
To: Roger Quadros, Tony Lindgren, Linux OMAP Mailing List,
Tomi Valkeinen, KISHON VIJAY ABRAHAM
Cc: Tero Kristo, Sekhar Nori, Nishanth Menon,
Device Tree Mailing List, Rob Herring, Linux ARM Mailing List,
Carlos Hernandez
In-Reply-To: <045e8200-69bd-8590-1da4-96235444db4c-l0cyMroinI0@public.gmane.org>
On Tuesday 13 December 2016 06:10 PM, Roger Quadros wrote:
> +Tomi, Kishon, Carlos
>
> Hi,
>
> On 21/10/16 13:38, Lokesh Vutla wrote:
>> dra72-evm-common.dtsi consolidates dra72-evm.dts and dra72-evm-revc.dts
>> which also include tps65917 pmic support as both the evms uses the same
>> pmic. But, dra71-evm has mostly similar features with a different pmic.
>> In order to exploit dra72-evm-common.dtsi, creating a separate dtsi
>> for tps65915 support and including it in respective board files.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
>> ---
>> arch/arm/boot/dts/dra72-evm-common.dtsi | 128 ----------------------------
>> arch/arm/boot/dts/dra72-evm-revc.dts | 21 +++--
>> arch/arm/boot/dts/dra72-evm-tps65917.dtsi | 134 ++++++++++++++++++++++++++++++
>> arch/arm/boot/dts/dra72-evm.dts | 14 ++--
>> 4 files changed, 154 insertions(+), 143 deletions(-)
>> create mode 100644 arch/arm/boot/dts/dra72-evm-tps65917.dtsi
>>
>
> This patch breaks USB XHCI and boot on dra72-evm (both revC and non revC)
>
> I'll explain why below.
>
> [ 13.625167] Unhandled fault: imprecise external abort (0x1406) at 0x00000000
> [ 13.632557] pgd = ede10000
> [ 13.635390] [00000000] *pgd=00000000
> [ 13.639145] Internal error: : 1406 [#1] SMP ARM
> [ 13.643893] Modules linked in: xhci_plat_hcd(+) xhci_hcd usbcore omapfb dwc3 cfbfillrect snd_soc_davinci_mcasp cfbimgblt cfbcopyarea udc_core connector_hdmi encoder_tpd12s015 snd_soc_edma m25p80 snd_soc_simpe
> [ 13.695557] CPU: 0 PID: 440 Comm: modprobe Not tainted 4.9.0-rc1 #1050
> [ 13.702399] Hardware name: Generic DRA72X (Flattened Device Tree)
> [ 13.708786] task: edb5c040 task.stack: edd10000
> [ 13.713540] PC is at _raw_spin_unlock_irqrestore+0x0/0x44
> [ 13.719219] LR is at xhci_hub_control+0xc2c/0x15e0 [xhci_hcd]
> [ 13.725242] pc : [<c07df718>] lr : [<bf486300>] psr: a0000093
> [ 13.725242] sp : edd118c0 ip : c0e306b4 fp : 00000000
> [ 13.737278] r10: 00000000 r9 : 60000013 r8 : edf28218
> [ 13.742753] r7 : edf28000 r6 : 00000000 r5 : 00000000 r4 : edf2a000
> [ 13.749593] r3 : 00000000 r2 : 00000000 r1 : 60000013 r0 : edf28218
Hmm.. Thanks for catching it. I remember it was booting when I tested,
not sure how I missed this :(. usb2_phy1 & 2, mmc and dss supplies needs
to be added in dra72-evm-tps65917.dtsi file.
Tony,
Do you want me to resend this patch or fixup patch on top of this?
Thanks and regards,
Lokesh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 0/9] dmaengine: stm32-dma: Bug fixes and improvements series
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, dan.j.williams, dmaengine, devicetree,
linux-arm-kernel, linux-kernel
Cc: M'boumba Cedric Madianga
This patchset adds bug fixes reported by devices using STM32 DMA and some
improvements mainly linked to dmaengine framework evolution.
M'boumba Cedric Madianga (9):
dmaengine: stm32-dma: Set correct args number for DMA request from DT
dmaengine: stm32-dma: Fix typo in Kconfig
dt-bindings: stm32-dma: Fix typo regarding DMA client binding
dmaengine: stm32-dma: Fix null pointer dereference in stm32_dma_tx_status
dmaengine: stm32-dma: Rework starting transfer management
dmaengine: stm32-dma: Fix residue computation issue in cyclic mode
dmaengine: stm32-dma: Add error messages if xlate fails
dmaengine: stm32-dma: Add synchronization support
dmaengine: stm32-dma: Add max_burst support
.../devicetree/bindings/dma/stm32-dma.txt | 5 +-
drivers/dma/Kconfig | 2 +-
drivers/dma/stm32-dma.c | 103 +++++++++++++--------
3 files changed, 66 insertions(+), 44 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 1/9] dmaengine: stm32-dma: Set correct args number for DMA request from DT
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, dan.j.williams, dmaengine, devicetree,
linux-arm-kernel, linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga@gmail.com>
This patch sets the right number of arguments to be used for DMA clients
which request channels from DT.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Reviewed-by: Ludovic BARRE <ludovic.barre@st.com>
---
drivers/dma/stm32-dma.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index 3688d08..a884b85 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -972,21 +972,18 @@ static struct dma_chan *stm32_dma_of_xlate(struct of_phandle_args *dma_spec,
struct stm32_dma_chan *chan;
struct dma_chan *c;
- if (dma_spec->args_count < 3)
+ if (dma_spec->args_count < 4)
return NULL;
cfg.channel_id = dma_spec->args[0];
cfg.request_line = dma_spec->args[1];
cfg.stream_config = dma_spec->args[2];
- cfg.threshold = 0;
+ cfg.threshold = dma_spec->args[3];
if ((cfg.channel_id >= STM32_DMA_MAX_CHANNELS) || (cfg.request_line >=
STM32_DMA_MAX_REQUEST_ID))
return NULL;
- if (dma_spec->args_count > 3)
- cfg.threshold = dma_spec->args[3];
-
chan = &dmadev->chan[cfg.channel_id];
c = dma_get_slave_channel(&chan->vchan.chan);
--
1.9.1
^ permalink raw reply related
* [PATCH 2/9] dmaengine: stm32-dma: Fix typo in Kconfig
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, dan.j.williams, dmaengine, devicetree,
linux-arm-kernel, linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga@gmail.com>
As STM32 DMA driver is only used as buit-in driver, it couldn't be used as
module.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Reviewed-by: Ludovic BARRE <ludovic.barre@st.com>
---
drivers/dma/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 263495d..96da57a 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -458,7 +458,7 @@ config STM32_DMA
help
Enable support for the on-chip DMA controller on STMicroelectronics
STM32 MCUs.
- If you have a board based on such a MCU and wish to use DMA say Y or M
+ If you have a board based on such a MCU and wish to use DMA say Y
here.
config S3C24XX_DMAC
--
1.9.1
^ permalink raw reply related
* [PATCH 3/9] dt-bindings: stm32-dma: Fix typo regarding DMA client binding
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, dan.j.williams, dmaengine, devicetree,
linux-arm-kernel, linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga@gmail.com>
Only four cells are required for dma client binding not five.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Reviewed-by: Ludovic BARRE <ludovic.barre@st.com>
---
Documentation/devicetree/bindings/dma/stm32-dma.txt | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/stm32-dma.txt b/Documentation/devicetree/bindings/dma/stm32-dma.txt
index 70cd13f..4408af6 100644
--- a/Documentation/devicetree/bindings/dma/stm32-dma.txt
+++ b/Documentation/devicetree/bindings/dma/stm32-dma.txt
@@ -40,8 +40,7 @@ Example:
DMA clients connected to the STM32 DMA controller must use the format
described in the dma.txt file, using a five-cell specifier for each
-channel: a phandle plus four integer cells.
-The four cells in order are:
+channel: a phandle to the DMA controller plus the following four integer cells:
1. The channel id
2. The request line number
@@ -61,7 +60,7 @@ The four cells in order are:
0x1: medium
0x2: high
0x3: very high
-5. A 32bit mask specifying the DMA FIFO threshold configuration which are device
+4. A 32bit mask specifying the DMA FIFO threshold configuration which are device
dependent:
-bit 0-1: Fifo threshold
0x0: 1/4 full FIFO
--
1.9.1
^ permalink raw reply related
* [PATCH 4/9] dmaengine: stm32-dma: Fix null pointer dereference in stm32_dma_tx_status
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, dan.j.williams, dmaengine, devicetree,
linux-arm-kernel, linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga@gmail.com>
chan->desc is always set to NULL when a DMA transfer is complete.
As a DMA transfer could be complete during the call of stm32_dma_tx_status,
we need to be sure that chan->desc is not NULL before using this variable
to avoid a null pointer deference issue.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Reviewed-by: Ludovic BARRE <ludovic.barre@st.com>
---
drivers/dma/stm32-dma.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index a884b85..3056ce7 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -880,7 +880,7 @@ static enum dma_status stm32_dma_tx_status(struct dma_chan *c,
struct virt_dma_desc *vdesc;
enum dma_status status;
unsigned long flags;
- u32 residue;
+ u32 residue = 0;
status = dma_cookie_status(c, cookie, state);
if ((status == DMA_COMPLETE) || (!state))
@@ -888,16 +888,12 @@ static enum dma_status stm32_dma_tx_status(struct dma_chan *c,
spin_lock_irqsave(&chan->vchan.lock, flags);
vdesc = vchan_find_desc(&chan->vchan, cookie);
- if (cookie == chan->desc->vdesc.tx.cookie) {
+ if (chan->desc && cookie == chan->desc->vdesc.tx.cookie)
residue = stm32_dma_desc_residue(chan, chan->desc,
chan->next_sg);
- } else if (vdesc) {
+ else if (vdesc)
residue = stm32_dma_desc_residue(chan,
to_stm32_dma_desc(vdesc), 0);
- } else {
- residue = 0;
- }
-
dma_set_residue(state, residue);
spin_unlock_irqrestore(&chan->vchan.lock, flags);
--
1.9.1
^ permalink raw reply related
* [PATCH 5/9] dmaengine: stm32-dma: Rework starting transfer management
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul-ral2JQCrhuEAvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
dmaengine-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This patch reworks the way to manage transfer starting.
Now, starting DMA is only allowed when the channel is not busy.
Then, stm32_dma_start_transfer is declared as void.
At least, after each transfer completion, we start the next transfer if a
new descriptor as been queued in the issued list during an ongoing
transfer.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Ludovic BARRE <ludovic.barre-qxv4g6HH51o@public.gmane.org>
---
drivers/dma/stm32-dma.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index 3056ce7..adb846c 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -421,7 +421,7 @@ static void stm32_dma_dump_reg(struct stm32_dma_chan *chan)
dev_dbg(chan2dev(chan), "SFCR: 0x%08x\n", sfcr);
}
-static int stm32_dma_start_transfer(struct stm32_dma_chan *chan)
+static void stm32_dma_start_transfer(struct stm32_dma_chan *chan)
{
struct stm32_dma_device *dmadev = stm32_dma_get_dev(chan);
struct virt_dma_desc *vdesc;
@@ -432,12 +432,12 @@ static int stm32_dma_start_transfer(struct stm32_dma_chan *chan)
ret = stm32_dma_disable_chan(chan);
if (ret < 0)
- return ret;
+ return;
if (!chan->desc) {
vdesc = vchan_next_desc(&chan->vchan);
if (!vdesc)
- return -EPERM;
+ return;
chan->desc = to_stm32_dma_desc(vdesc);
chan->next_sg = 0;
@@ -471,7 +471,7 @@ static int stm32_dma_start_transfer(struct stm32_dma_chan *chan)
chan->busy = true;
- return 0;
+ dev_dbg(chan2dev(chan), "vchan %p: started\n", &chan->vchan);
}
static void stm32_dma_configure_next_sg(struct stm32_dma_chan *chan)
@@ -552,15 +552,13 @@ static void stm32_dma_issue_pending(struct dma_chan *c)
{
struct stm32_dma_chan *chan = to_stm32_dma_chan(c);
unsigned long flags;
- int ret;
spin_lock_irqsave(&chan->vchan.lock, flags);
- if (!chan->busy) {
- if (vchan_issue_pending(&chan->vchan) && !chan->desc) {
- ret = stm32_dma_start_transfer(chan);
- if ((!ret) && (chan->desc->cyclic))
- stm32_dma_configure_next_sg(chan);
- }
+ if (vchan_issue_pending(&chan->vchan) && !chan->desc && !chan->busy) {
+ dev_dbg(chan2dev(chan), "vchan %p: issued\n", &chan->vchan);
+ stm32_dma_start_transfer(chan);
+ if (chan->desc->cyclic)
+ stm32_dma_configure_next_sg(chan);
}
spin_unlock_irqrestore(&chan->vchan.lock, flags);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 6/9] dmaengine: stm32-dma: Fix residue computation issue in cyclic mode
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, dan.j.williams, dmaengine, devicetree,
linux-arm-kernel, linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga@gmail.com>
This patch resolves the residue computation issue detected in cyclic mode.
Now, in cyclic mode, we increment next_sg variable as soon as a period is
transferred instead of after pushing a new sg request.
Then, we take into account that after transferring a complete buffer,
the next_sg variable is equal to 0.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Reviewed-by: Ludovic BARRE <ludovic.barre@st.com>
---
drivers/dma/stm32-dma.c | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index adb846c..ba929a9 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -500,8 +500,6 @@ static void stm32_dma_configure_next_sg(struct stm32_dma_chan *chan)
dev_dbg(chan2dev(chan), "CT=0 <=> SM1AR: 0x%08x\n",
stm32_dma_read(dmadev, STM32_DMA_SM1AR(id)));
}
-
- chan->next_sg++;
}
}
@@ -510,6 +508,7 @@ static void stm32_dma_handle_chan_done(struct stm32_dma_chan *chan)
if (chan->desc) {
if (chan->desc->cyclic) {
vchan_cyclic_callback(&chan->desc->vdesc);
+ chan->next_sg++;
stm32_dma_configure_next_sg(chan);
} else {
chan->busy = false;
@@ -846,26 +845,40 @@ static struct dma_async_tx_descriptor *stm32_dma_prep_dma_memcpy(
return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
}
+static u32 stm32_dma_get_remaining_bytes(struct stm32_dma_chan *chan)
+{
+ u32 dma_scr, width, ndtr;
+ struct stm32_dma_device *dmadev = stm32_dma_get_dev(chan);
+
+ dma_scr = stm32_dma_read(dmadev, STM32_DMA_SCR(chan->id));
+ width = STM32_DMA_SCR_PSIZE_GET(dma_scr);
+ ndtr = stm32_dma_read(dmadev, STM32_DMA_SNDTR(chan->id));
+
+ return ndtr << width;
+}
+
static size_t stm32_dma_desc_residue(struct stm32_dma_chan *chan,
struct stm32_dma_desc *desc,
u32 next_sg)
{
- struct stm32_dma_device *dmadev = stm32_dma_get_dev(chan);
- u32 dma_scr, width, residue, count;
+ u32 residue = 0;
int i;
- residue = 0;
+ /*
+ * In cyclic mode, for the last period, residue = remaining bytes from
+ * NDTR
+ */
+ if (chan->desc->cyclic && next_sg == 0)
+ return stm32_dma_get_remaining_bytes(chan);
+ /*
+ * For all other periods in cyclic mode, and in sg mode,
+ * residue = remaining bytes from NDTR + remaining periods/sg to be
+ * transferred
+ */
for (i = next_sg; i < desc->num_sgs; i++)
residue += desc->sg_req[i].len;
-
- if (next_sg != 0) {
- dma_scr = stm32_dma_read(dmadev, STM32_DMA_SCR(chan->id));
- width = STM32_DMA_SCR_PSIZE_GET(dma_scr);
- count = stm32_dma_read(dmadev, STM32_DMA_SNDTR(chan->id));
-
- residue += count << width;
- }
+ residue += stm32_dma_get_remaining_bytes(chan);
return residue;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 7/9] dmaengine: stm32-dma: Add error messages if xlate fails
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, dan.j.williams, dmaengine, devicetree,
linux-arm-kernel, linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga@gmail.com>
This patch adds some error messages when a slave device fails to request a
channel.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Reviewed-by: Ludovic BARRE <ludovic.barre@st.com>
---
drivers/dma/stm32-dma.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index ba929a9..35639f6 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -975,27 +975,36 @@ static struct dma_chan *stm32_dma_of_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma)
{
struct stm32_dma_device *dmadev = ofdma->of_dma_data;
+ struct device *dev = dmadev->ddev.dev;
struct stm32_dma_cfg cfg;
struct stm32_dma_chan *chan;
struct dma_chan *c;
- if (dma_spec->args_count < 4)
+ if (dma_spec->args_count < 4) {
+ dev_err(dev, "Bad number of cells\n");
return NULL;
+ }
cfg.channel_id = dma_spec->args[0];
cfg.request_line = dma_spec->args[1];
cfg.stream_config = dma_spec->args[2];
cfg.threshold = dma_spec->args[3];
- if ((cfg.channel_id >= STM32_DMA_MAX_CHANNELS) || (cfg.request_line >=
- STM32_DMA_MAX_REQUEST_ID))
+ if ((cfg.channel_id >= STM32_DMA_MAX_CHANNELS) ||
+ (cfg.request_line >= STM32_DMA_MAX_REQUEST_ID)) {
+ dev_err(dev, "Bad channel and/or request id\n");
return NULL;
+ }
chan = &dmadev->chan[cfg.channel_id];
c = dma_get_slave_channel(&chan->vchan.chan);
- if (c)
- stm32_dma_set_config(chan, &cfg);
+ if (!c) {
+ dev_err(dev, "No more channel avalaible\n");
+ return NULL;
+ }
+
+ stm32_dma_set_config(chan, &cfg);
return c;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 8/9] dmaengine: stm32-dma: Add synchronization support
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul-ral2JQCrhuEAvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
dmaengine-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Implement the new device_synchronize() callback to allow proper
synchronization when stopping a channel.
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/dma/stm32-dma.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index 35639f6..b7be43a 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -403,6 +403,13 @@ static int stm32_dma_terminate_all(struct dma_chan *c)
return 0;
}
+static void stm32_dma_synchronize(struct dma_chan *c)
+{
+ struct stm32_dma_chan *chan = to_stm32_dma_chan(c);
+
+ vchan_synchronize(&chan->vchan);
+}
+
static void stm32_dma_dump_reg(struct stm32_dma_chan *chan)
{
struct stm32_dma_device *dmadev = stm32_dma_get_dev(chan);
@@ -1068,6 +1075,7 @@ static int stm32_dma_probe(struct platform_device *pdev)
dd->device_prep_dma_cyclic = stm32_dma_prep_dma_cyclic;
dd->device_config = stm32_dma_slave_config;
dd->device_terminate_all = stm32_dma_terminate_all;
+ dd->device_synchronize = stm32_dma_synchronize;
dd->src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 9/9] dmaengine: stm32-dma: Add max_burst support
From: M'boumba Cedric Madianga @ 2016-12-13 13:40 UTC (permalink / raw)
To: vinod.koul, robh+dt, mark.rutland, mcoquelin.stm32,
alexandre.torgue, dan.j.williams, dmaengine, devicetree,
linux-arm-kernel, linux-kernel
Cc: M'boumba Cedric Madianga
In-Reply-To: <1481636451-27863-1-git-send-email-cedric.madianga@gmail.com>
This patch sets the max_burst value supported by the STM32 DMA
Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
---
drivers/dma/stm32-dma.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index b7be43a..49f86ca 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -114,6 +114,7 @@
#define STM32_DMA_MAX_CHANNELS 0x08
#define STM32_DMA_MAX_REQUEST_ID 0x08
#define STM32_DMA_MAX_DATA_PARAM 0x03
+#define STM32_DMA_MAX_BURST 16
enum stm32_dma_width {
STM32_DMA_BYTE,
@@ -1084,6 +1085,7 @@ static int stm32_dma_probe(struct platform_device *pdev)
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
dd->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
dd->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
+ dd->max_burst = STM32_DMA_MAX_BURST;
dd->dev = &pdev->dev;
INIT_LIST_HEAD(&dd->channels);
--
1.9.1
^ permalink raw reply related
* Re: [RFC v2 00/13] usb/mmc/power: Fix USB/LAN when TFTP booting
From: Krzysztof Kozlowski @ 2016-12-13 13:53 UTC (permalink / raw)
To: Hans Verkuil
Cc: devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc,
linux-mmc, linux-pm, linux-usb, Ulf Hansson, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, Greg Kroah-Hartman,
Mark Brown, tjakobi, Marek Szyprowski, Bartlomiej Zolnierkiewicz
In-Reply-To: <b34f49c8-6b37-cabf-2a0f-c8d465c53159@xs4all.nl>
On Tue, Dec 13, 2016 at 2:34 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Try again, this time with Krzysztof's new email address...
>
>
> On 13/12/16 13:20, Hans Verkuil wrote:
>>
>> Hi Krzysztof,
>>
>> This still seems to be broken with the latest 4.9 kernel, right?
>>
>> Has there been any progress on this? Do you have an updated patch series
>> for me to use?
Hi,
I think it is not fixed. Still. I left this work to others. AFAIK,
Peter Chen is working on a new generic approach:
https://lwn.net/Articles/703556/
On top of his patchset, Odroid would need some DTS code as well (and
maybe something in usb3503). However I do not plan to work on this
anymore as I do not have Odroid U3 anymore. Marek and Bartlomiej from
Samsung Poland are in CC-list so maybe they would like to continue the
work?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/2] eeprom: Add IDT 89HPESx driver bindings file
From: Serge Semin @ 2016-12-13 14:08 UTC (permalink / raw)
To: Rob Herring
Cc: Greg Kroah-Hartman, Srinivas Kandagatla, Andrew Lunn,
Mark Rutland, Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+GB85b4p+8JwZPy=ELOpeLGcKryqtwCd9e5PV=jbi_Cw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, Dec 12, 2016 at 05:04:31PM -0600, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Dec 5, 2016 at 1:04 PM, Serge Semin <fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Mon, Dec 05, 2016 at 11:27:07AM -0600, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> On Mon, Dec 5, 2016 at 9:25 AM, Serge Semin <fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> > On Mon, Dec 05, 2016 at 08:46:21AM -0600, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> >> On Tue, Nov 29, 2016 at 01:38:21AM +0300, Serge Semin wrote:
> >> >> > See cover-letter for changelog
> >> >> >
> >> >> > Signed-off-by: Serge Semin <fancer.lancer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> >> >
> >> >> > ---
> >> >> > .../devicetree/bindings/misc/idt_89hpesx.txt | 41 ++++++++++++++++++++++
> >> >>
> >> >> There's not a better location for this? I can't tell because you don't
> >> >> describe what the device is.
> >> >>
> >> >
> >> > The device is PCIe-switch EEPROM driver with additional debug-interface to
> >> > access the switch CSRs. EEPROM is accesses via a separate i2c-slave
> >> > interface of the switch.
> >> >
> >> > There might be another place to put the binding file in. There is a special
> >> > location for EEPROM drivers bindings - Documentation/devicetree/bindings/eeprom/ .
> >> > But as far as I understood from the files put in there, it's intended for
> >> > pure EEPROM drivers only. On the other hand the directory I've chosen:
> >> > Documentation/devicetree/bindings/misc/
> >> > mostly intended for some unusual devices. My device isn't usual, since it
> >> > has CSRs debug-interface as well. Additionally I've found
> >> > eeprom-93xx46.txt binding file there, which describes EEPROM bindings.
> >> >
> >> > Anyway if you find the file should be placed in
> >> > Documentation/devicetree/bindings/eeprom/ instead, I'll move it, it's not
> >> > that a big problem.
> >> >
> >
> > What about this comment? Shall the file be left at the path I placed it?
> >
> >> >> > 1 file changed, 41 insertions(+)
> >> >> > create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> >> >> >
> >> >> > diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> >> >> > index 0000000..469cc93
> >> >> > --- /dev/null
> >> >> > +++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
> >> >> > @@ -0,0 +1,41 @@
> >> >> > +EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices
> >> >> > +
> >> >> > +Required properties:
> >> >> > + - compatible : should be "<manufacturer>,<type>"
> >> >> > + Basically there is only one manufacturer: idt, but some
> >> >> > + compatible devices may be produced in future. Following devices
> >> >> > + are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2,
> >> >> > + 89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2,
> >> >> > + 89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2;
> >> >> > + 89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a;
> >> >> > + 89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2,
> >> >> > + 89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2,
> >> >> > + 89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2,
> >> >> > + 89hpes64h16ag2;
> >> >> > + 89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2,
> >> >> > + 89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5,
> >> >> > + 89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2,
> >> >> > + 89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2,
> >> >> > + 89hpes48t12, 89hpes48t12g2.
> >> >> > + Current implementation of the driver doesn't have any device-
> >> >>
> >> >> Driver capabilties are irrelevant to bindings.
> >> >>
> >> >
> >> > Why? I've told in the comment, that the devices actually differ by the CSRs
> >> > map. Even though it's not reflected in the code at the moment, the CSRs
> >> > read/write restrictions can be added by some concerned programmer in
> >> > future. But If I left something like "compatible : idt,89hpesx" device
> >> > only, it will be problematic to add that functionality.
> >>
> >> Bindings describe the h/w, not what the Linux, FreeBSD, etc. driver
> >> does. You don't want to be changing the binding doc when the driver
> >> changes.
> >>
> >> > Howbeit If you think it's not necessary and "compatible = idt,89hpesx" is
> >> > ok, it's perfectly fine for me to make it this way. The property will be
> >> > even simpler, than current approach.
> >>
> >> NO! That's not at all what I'm suggesting. Specific compatible strings
> >> are the right way to go for the reasons you give. You just don't need
> >> to state why here (because it is true for all bindings).
> >>
> >
> > Oh, I just misunderstood what you said. I'll discard the comment.
> >
> >> >> > + specific functionalities. But since each of them differs
> >> >> > + by registers mapping, CSRs read/write restrictions can be
> >> >> > + added in future.
> >> >> > + - reg : I2C address of the IDT 89HPES device.
> >> >> > +
> >> >> > +Optional properties:
> >> >> > + - read-only : Parameterless property disables writes to the EEPROM
> >> >> > + - idt,eesize : Size of EEPROM device connected to IDT 89HPES i2c-master bus
> >> >> > + (default value is 4096 bytes if option isn't specified)
> >> >> > + - idt,eeaddr : Custom address of EEPROM device
> >> >> > + (If not specified IDT 89HPESx device will try to communicate
> >> >> > + with EEPROM sited by default address - 0x50)
> >> >>
> >> >> Don't we already have standard EEPROM properties that could be used
> >> >> here?
> >> >>
> >> >
> >> > If we do, just tell me which one. There are standard options:
> >>
> >> You can grep thru bindings as easily as I can. I can't do that for
> >> everyone's binding.
> >>
> >
> > It won't be necessary due to the next comment.
> >
> >> > "compatible, reg, pagesize, read-only". There isn't any connected with
> >> > EEPROM actual size.
> >> > Why so? Because standard EEPROM-drivers determine the device size from the
> >> > compatible-string name. Such approach won't work in this case, because
> >> > PCIe-switch and it EEPROM are actually two different devices. Look at the
> >> > chain of the usual platform board design:
> >> > Host <--- i2c ----> i2c-slave iface |PCIe-switch| i2c-master iface <--- i2c ---> EEPROM
> >> >
> >> > As you cas see the Host reaches EEPROM through the set of PCIe-switch
> >> > i2c-interfaces. In order to properly get data from it my driver needs actual
> >> > EEPROM size and it address in the i2c-master bus of the PCIe-switch, in
> >> > addition to the standard reg-field, which is address of PCIe-switch i2c-slave
> >> > interface and read-only parameter if EEPROM-device has got WP pin asserted.
> >>
> >> Ah, this needs to be much different than I thought. You need to model
> >> (i.e. use the same binding) the EEPROM node just like it was directly
> >> attached to the host. So this means you need the 2nd i2c bus modeled
> >> which means you need the PCIe switch modeled. A rough outline of the
> >> nodes would look like this:
> >>
> >> host-i2c: i2c {
> >> compatible ="host-i2c"
> >> };
> >>
> >> pcie {
> >> pcie-switch {
> >> i2c-bus = <&host-i2c>;
> >> i2c-bus {
> >> eeprom@50 {
> >> };
> >> };
> >> };
> >> };
> >>
> >> So this models the PCIe switch as a PCIe device, it has a phandle back
> >> to it's controller since it's not a child of the i2c controller. Then
> >> the devices on switches i2c bus are modeled as children of the switch.
> >>
> >> Alternatively, it could be described all as children of host-i2c node.
> >> It's common for i2c devices to have downstream i2c buses. I2C muxes
> >> are one example and there are bindings defined for all this. There's
> >> also chips like mpu-6050 that have slave buses.
> >>
> >> Rob
> >
> > I think I understand what you says. However let me just bring some details
> > to make things clear.
> >
> > First of all the driver doesn't do any PCI-Express-related work. The device
> > !IDT PCI Express switch! just has two additional i2c interfaces: i2c-slave
> > and i2c-master. As it is obvious from the bus-names i2c-slave is the interface,
> > where IDT PCIe-switch device is actually slave. This interface can be reached
> > from the host by ordinary i2c buses. i2c-master interface is connected to an
> > i2c-bus, where IDT PCIe-switch is single master. This bus can have just one
> > EEPROM device to store some initialization data. Host can send some specific
> > smbus-packets to i2c-slave interface of IDT PCIe-switch in order to
> > preinitialize EEPROM data, connected to i2c-master interface of the device.
> >
> > Additionally IDT PCIe-switch handles some special smbus packets coming to it
> > i2c-slave interface to read/write its internal CSR. This interface can be
> > used to debug the device, when there are problems with it usual PCI Express
> > related functioning.
> >
> > So to speak, it wouldn't be good to have PCIe-switch declared in dts as a
> > PCI-device, since PCI-bus is actually dynamically populated by PCI-core
> > subsystem.
>
> Why not? The DT is just extra data for what is not discoverable. Is
> the device actually hotplugable and in a dynamic location/slot? If
> not, then describing the device in DT is not uncommon. If it is
> hotplugable, you still have same problem of knowing which I2C bus it
> is on. Even if you know for your design, generally speaking you may
> not know.
>
Device isn't hotplugable, it's always placed on the circuit. So to speak it is
always known which i2c bus it's placed on. That's why I placed the device
description in the dts.
> > According to what you said and the device/driver design I described, the
> > following bindings can be suggested:
> >
> > i2c0: i2c@FFFF0000 {
> > compatible = "vendor,i2c-adapter";
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > idt_i2c_iface: idt@60 {
> > compatible = "idt,89hpes32nt8ag2";
> > reg = <0x60>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > eeprom@51 {
> > compatible = "at,24c64";
> > reg = <0x51>;
> > read-only;
> > };
> > };
> > };
> >
> > Suppose there is some host-i2c adapter like "vendor,i2c-adapter" and
> > i2c-slave interface of IDT PCIe-switch is connected to it. In this way
> > i2c-slave interface will be visible like ordinary i2c-device with just
> > one subnode. This subnode explains the actual EEPROM connected to
> > IDT PCIe-switch i2c-master interface.
> >
> > Does it look acceptable? It seems like your last suggestion. Is it?
>
> That is the 2nd option. My concern is this may work for your immediate
> case, but if you started to need to describe the PCIe interface it
> would not work. Similarly, we started out describing USB hubs with I2C
> interfaces this way and it has proven to be in adequate for some
> cases. So we're moving to describing the USB hierarchy in DT. I'm
> concerned that while it may work for you, if the PCIe interface has
> any dependencies like regulators or something, then you would need to
> have a PCIe node.
>
> Rob
Alright then. I'll develop the second option and resend the patchset.
-Sergey
--
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 v4 3/5] i2c: designware: Add slave definitions
From: Rob Herring @ 2016-12-13 14:11 UTC (permalink / raw)
To: Luis de Oliveira
Cc: wsa@the-dreams.de, mark.rutland@arm.com,
jarkko.nikula@linux.intel.com, andriy.shevchenko@linux.intel.com,
mika.westerberg@linux.intel.com, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Ramiro.Oliveira@synopsys.com, Joao.Pinto@synopsys.com,
CARLOS.PALMINHA@synopsys.com
In-Reply-To: <BCF0D4927F9C694C9AEA8827D4A9C7C897CEDB@de02wembxa.internal.synopsys.com>
Again, please don't top post. And your line wrapping is messed up.
IOW, you can't use Outlook.
On Tue, Dec 13, 2016 at 4:50 AM, Luis de Oliveira
<Luis.Oliveira@synopsys.com> wrote:
> The controller for i2c-designware cannot be slave/master at the same time and it has to be enabled knowing beforehand if we want it to be slave or master by something outside of the controller itself.
>
> I as looking and I see the use of this I2C_OWN_SLAVE_ADDRESS with the "linux,slave-24c02" slave interface but I am not seeing how it will help me identify a selected i2c-designware block as a "slave" device before instantiation. I'm sorry if I'm not understanding properly.
> I use the "linux,slave-24c02" to instantiate the i2c-designware as a slave with an address so I can do write/read operations, it is how I tested it.
Something like this:
of_for_each_child_node(child) {
of_property_read_u32(child, "reg", ®);
if (reg & I2C_OWN_SLAVE_ADDRESS))
im_a_slave = true;
}
...rather than testing "mode" is equal to "slave".
Rob
>
> Luis
>
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Monday, December 12, 2016 23:16
> To: Luis de Oliveira <Luis.Oliveira@synopsys.com>
> Cc: wsa@the-dreams.de; mark.rutland@arm.com; jarkko.nikula@linux.intel.com; andriy.shevchenko@linux.intel.com; mika.westerberg@linux.intel.com; linux-i2c@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Ramiro.Oliveira@synopsys.com; Joao.Pinto@synopsys.com; CARLOS.PALMINHA@synopsys.com
> Subject: Re: [PATCH v4 3/5] i2c: designware: Add slave definitions
>
> On Mon, Dec 12, 2016 at 12:35 PM, Luis de Oliveira <Luis.Oliveira@synopsys.com> wrote:
>> Hi all,
>
> Please don't top post.
>
>>
>> The slave address could be set by the I2C slave backend so I can't use it to setup the controller.
>> A boolean property was my initial approach then Andy and Wolfram Sang suggested the use of compatible strings and later It was suggested to use a property to select mode but I can do it again if it's the best way.
>> Can you please tell me where should it be documented?
>
> bindings/i2c/i2c.txt.
>
> Actually, looking at this some more, we already have a way to describe the controller being a slave device with the I2C_OWN_SLAVE_ADDRESS flag in the reg property. We should just need a helper to read reg property of each child and check for the bit set.
>
> Rob
^ 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