Re: [PATCH v5 06/10] ARM: dts: Clean up exynos5250-smdk5250
Andreas, On Thu, Jul 31, 2014 at 9:54 PM, Andreas Färber afaer...@suse.de wrote: Use the new style for referencing inherited nodes and use symbolic names. Goal is the alignment of all exynos5250 based device trees for comparison. Signed-off-by: Andreas Färber afaer...@suse.de --- v5: New Follow-up after adding dp_hpd pinctrl node new-style. Since I NAKed that one, are you going to drop this patch? It's probably still a useful thing to do. It sounds like you might be spinning? ...if so I'll review it in the next cycle... Note: same feedback as before about splitting changes to exynos5250.dtsi and actual board files... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 03/10] ARM: dts: Prepare node labels for exynos5250
Andreas, On Fri, Aug 1, 2014 at 5:52 PM, Andreas Färber afaer...@suse.de wrote: Allows them to be extended by reference. Signed-off-by: Andreas Färber afaer...@suse.de --- v6: Split off from Snow/SMDK cleanups (Doug Anderson) arch/arm/boot/dts/exynos5250.dtsi | 24 1 file changed, 12 insertions(+), 12 deletions(-) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] ARM: dts: Add exynos5250-spring device tree
Tomasz, On Thu, Jul 31, 2014 at 12:40 PM, Tomasz Figa tomasz.f...@gmail.com wrote: Andreas, On 31.07.2014 21:20, Andreas Färber wrote: Am 31.07.2014 21:05, schrieb Tomasz Figa: On 31.07.2014 18:08, Andreas Färber wrote: Adds initial support for the HP Chromebook 11. [snip] + gpio-keys { + compatible = gpio-keys; + pinctrl-names = default; + pinctrl-0 = power_key_irq, lid_irq; + + power { + label = Power; + gpios = gpx1 3 GPIO_ACTIVE_LOW; + linux,code = KEY_POWER; I assume the key is debounced in hardware, so there is no need for debounce-interval here. Is this correct? You're asking the wrong person... This is copied from -cros-common/-snow. Downstream 3.8 does not have a debounce-interval property. Doug, Vincent? Not something I've looked at, but it's never been a problem... +sd1_clk { + samsung,pin-drv = 0; +}; + +sd1_cmd { + samsung,pin-pud = 3; + samsung,pin-drv = 0; +}; + +sd1_cd { + samsung,pin-drv = 0; +}; + +sd1_bus4 { + samsung,pin-drv = 0; +}; Here generic settings are being overridden, so it might be a good idea to explain why, like with i2c pull-up above. Snow does not have an explanation either, so please suggest what comment you'd like to see. Consider me just a user with no specs. :) Doug, Vincent, someone else? The comment is just in a different place--it's in the dw_mmc node. Probably belongs here, though: /* * Wifi is a SiP, so can keep drive strengths low * to reduce EMI. */ I guess the cmd line isn't documented. There is no external pull on the command line and on most boards there is one. Our hardware guys thought we didn't need it and apparently we don't... -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 10/10] ARM: dts: Add exynos5250-spring device tree
Andreas, On Fri, Aug 1, 2014 at 5:52 PM, Andreas Färber afaer...@suse.de wrote: Adds initial support for the HP Chromebook 11. Cc: Vincent Palatin vpala...@chromium.org Cc: Doug Anderson diand...@chromium.org Cc: Stephan van Schaik step...@synkhronix.com Signed-off-by: Andreas Färber afaer...@suse.de --- v5 - v6: * Updated for mfc node label * Reverted to dp-hpd-gpio node in pinctrl_0 (Doug Anderson) * Fixed alphabetical order of sd1_* nodes (Doug Anderson) v4 - v5: * Dropped bogus USB3 regulator (Vincent Palatin, Tomasz Figa) * Fixed USB3503 reset GPIO (Tomasz Figa) * Introduced labels to use new referencing style consistently (Tomasz Figa) * Don't override dp_hpd, moved to pinctrl_0 instead (Tomasz Figa) * mmc_1: Added comment from Snow's mmc_3 (Tomasz Figa / Doug Anderson) * Override /codec samsung,mfc-{l,r} properties for alignment with Arndale * Use more GPIO_ACTIVE_* constants * Use IRQ_TYPE_* constants * Dropped s5m_ prefix for s5m8767 LDO regulator labels (max77686 is gone) * Labeled also all s5m8767 BUCK regulators v3 - v4: * Fixed samsung,pin-function 1 - 0 for dp-hpd-gpio * Replaced dp-hpd-gpio with existing dp_hpd, overriding it v2 - v3: * Use GPIO_ACTIVE_{LOW,HIGH} (Doug Anderson) * Use symbolic KEY_POWER instead of comment * Moved hsic_reset to new USB3503 node's reset-gpios (Vincent Palatin) * Use dp_hpd_gpio for dp-controller (Doug Anderson, Ajay Kumar) * Override sd1_{clk,cmd,cd,bus4} pinctrl similar to Snow (Doug Anderson) * Added ec_irq pinctrl for cros_ec (Doug Anderson) * Reordered nodes to minimize diff against Snow (Doug Anderson) * Dropped obsolete mmc_2 override (Doug Anderson) * Added lid-switch node (Doug Anderson) * Added gpio-keys pinctrl (Doug Anderson) * Added bootargs to avoid empty /chosen node and to document console setting * Renamed s5m8767_pmic node to avoid underscore * Use new style for overriding inherited pinctrl nodes, too * Enable i2s0 node v1 - v2: * Use label-based overriding/extension of nodes. (Doug Anderson) * Dropped tps65090 for now, until we know where to place it. * Dropped non-Spring nodes from -cros-common.dtsi rather than disabling them. * Enabled a missing MMC node for access to internal storage. * Dropped display-timings from dp-controller node. (Ajay Kumar) arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/exynos5250-spring.dts | 536 2 files changed, 537 insertions(+) create mode 100644 arch/arm/boot/dts/exynos5250-spring.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 80a781f76e88..dec4c292f63d 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -76,6 +76,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \ exynos5250-arndale.dtb \ exynos5250-smdk5250.dtb \ exynos5250-snow.dtb \ + exynos5250-spring.dtb \ exynos5260-xyref5260.dtb \ exynos5410-smdk5410.dtb \ exynos5420-arndale-octa.dtb \ diff --git a/arch/arm/boot/dts/exynos5250-spring.dts b/arch/arm/boot/dts/exynos5250-spring.dts new file mode 100644 index ..f5566f84d885 --- /dev/null +++ b/arch/arm/boot/dts/exynos5250-spring.dts @@ -0,0 +1,536 @@ +/* + * Google Spring board device tree source + * + * Copyright (c) 2013 Google, Inc + * Copyright (c) 2014 SUSE LINUX Products GmbH + * + * 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. + */ + +/dts-v1/; +#include dt-bindings/gpio/gpio.h +#include dt-bindings/interrupt-controller/irq.h +#include dt-bindings/input/input.h +#include exynos5250.dtsi + +/ { + model = Google Spring; + compatible = google,spring, samsung,exynos5250, samsung,exynos5; + + memory { + reg = 0x4000 0x8000; + }; + + chosen { + bootargs = console=tty1; + }; + + gpio-keys { + compatible = gpio-keys; + pinctrl-names = default; + pinctrl-0 = power_key_irq, lid_irq; + + power { + label = Power; + gpios = gpx1 3 GPIO_ACTIVE_LOW; + linux,code = KEY_POWER; + gpio-key,wakeup; + }; + + lid-switch { + label = Lid; + gpios = gpx3 5 GPIO_ACTIVE_LOW; + linux,input-type = 5; /* EV_SW */ + linux,code = 0; /* SW_LID */ + debounce-interval = 1; + gpio-key,wakeup; + }; + }; + + usb-hub { + compatible = smsc,usb3503a; + reset-gpios = gpe1 0 GPIO_ACTIVE_LOW; + }; Last I remember
Re: [PATCH v6 00/10] ARM: dts: exynos: Prepare Spring
Andreas, On Sat, Aug 2, 2014 at 3:25 AM, Andreas Färber afaer...@suse.de wrote: Hi, Am 02.08.2014 06:57, schrieb Doug Anderson: On Fri, Aug 1, 2014 at 7:34 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: On 08/02/2014 02:52 AM, Andreas Färber wrote: Based on the preinstalled 3.8 based ChromeOS kernel and previous 3.15 based attempts by Stephan and me that broke for 3.16, I've prepared a device tree for the HP Chromebook 11 aka Google Spring. v6 renames a node and reverts dp_hpd. Not yet enabled are trackpad, Wifi and sound. I made a comment on patch 05/10 but the rest of the series looks good to me. So for the remaining patches: Reviewed-by: Javier Martinez Canillas javier.marti...@collabora.co.uk NOTE: I thought that Tomasz Figa gave you his Reviewed-by on v5 for the whole series as well but I didn't see his tag on the v6 patches. Yes, I thought that too. I assume he's OK with the small changes you made between v5 and v6. In the very least his Reviewed-by could be present on the patches that didn't change between the last two revs. I did add it to the bootargs, GPIO, USB3503 patches. All other patches were either split off or slightly changed due to dp_hpd[_gpio], so I didn't carry it over. Given Javier's review and Tomasz's review and Vincent's comments, I'll probably skip all the work of reviewing the rest of the series unless someone really wants me to. ;) Could you maybe give an Rb or Ab for the actual Spring patch to have the Cc: updated? :) Done. Note that if there's some problem that can't be resolved by selectively dropping patches, I won't be available next week, so you'll either have to provide fixups for Kukjin to squash or wait till I've returned. One thing I've wondered is whether we should put status = disabled on the dp node with some comment, since it's known not to work as is (but better having the data here than leaving it out, I believe). Don't know about this one. Of course if either of you has input on the discussions on the drm bridge/panel series V6 [1] for how to enable non-simplefb display and iommus, that would be valuable. I've been letting the graphics folks and Samsung hash out the graphics patches, so I don't think I'll be much help here. [1] http://www.spinics.net/lists/linux-samsung-soc/msg35274.html And when one thing is accomplished, I am always quick to look forward: I've taken a quick look at sound nodes: According to 3.8 DT, Spring uses max98089 whereas Snow has 98091, so different codec driver and still lacking DT binding support. I might look into trivially enabling sound/soc/codecs/max98089.c through a maxim,max98089 OF match table once this series lands in linux-next. As for the driver, can we reuse http://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/tree/sound/soc/samsung/snow.c?h=for-next with a google,spring-audio-max98089, or are code changes needed? I don't know this offhand. Both of you mentioned limitations of cros_ec i2c passthrough leading to a forked tps65090 driver downstream - I don't think I can be of help there, as I guess simply copying a driver will not be an option. ;) https://code.google.com/p/chromium/issues/detail?id=391797 Yup, I think this will be real work for someone. I made a quick attempt and failed at it and I haven't had time to work on it since (and don't necessarily expect to have time in the near future)... I think it is possible for anyone versed in i2c to figure this out based on what I already posted and what's in our local tree... For the touchpad it seems DT support has landed in the input tree as atmel,maxtouch. Backporting just that patch does not make it work though. (Tried the rejected pinctrl approach to be on the safe side.) https://code.google.com/p/chromium/issues/detail?id=371114 https://patchwork.kernel.org/patch/3976801/ This is the same work as needed for pit and pi, I believe. Perhaps Javier or Dmitry has this on their todo list? I thought the internal xhci would have the webcam on it, but I don't see it in lsusb. Does that need some pinctrl or tps65090 regulator? Once appearing on a bus, which driver config option will it need? Perhaps tps65090 FET5? That looks like what the device tree says in our local tree. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: dts: Add Peach Pit and Pi dts entry for atmel touchpad
Fabio, On Wed, Aug 6, 2014 at 6:35 PM, Fabio Estevam feste...@gmail.com wrote: On Wed, Aug 6, 2014 at 10:08 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: +hsi2c_8 { + status = okay; + clock-frequency = 333000; Doesn't it work at the more standard 400kHz i2c frequency? I'm pretty sure that they had signaling problems at 400kHz, but perhaps Benson will rememer. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting
Hi, On Fri, Aug 8, 2014 at 11:29 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Hello, On 08/08/2014 06:38 PM, Javier Martinez Canillas wrote: It seems as if the first call to exynos_irq_set_type() that is made by OF is a no-op while the second call is the one that actually setups the hw correctly. Does this make any sense? Maybe is related to the pin not being muxed in the correct function when the interrupts property is parsed by OF? So after a conversation with Tomasz Figa over IRC the problem was after all that the pin was reconfigured. The IRQ trigger type resulted to be just a red herring and not a direct cause. The pinctrl-eyxnos driver does the IRQ pinmux setup in the .irq_set_type function handler. So what happened was that OF parsed the interrupts property and called exynos_irq_set_type() which did the pinmux setup. But after that, due a DTS pinctrl configuration the pin function was changed as a GPIO input and that happened before the atmel driver was probed. So when the driver called request_threaded_irq(), the correct flags were used but the pin was not configured as an IRQ anymore so IRQ were not fired. Setting a trigger type just had the side effect of calling exynos_irq_set_type() which again setup the pin as an IRQ. To fix the issue a variation of patch [0] will be posted that moves the IRQ pinmux setup from .irq_set_type to the .irq_request_resources function handler. That way the pin will be setup as IRQ regardless of the the trigger type [1] when someone calls request_[threaded]_irq(). Only the mentioned patch fixes the issue but Tomasz said that even a call to gpio_direction_{input,output} can change the pin configuration so he will post another patch that will add a bit mask to samsung_pin_bank to prevent any pinmux reconfiguration. Would just making a device tree change fix this? AKA for the pin, do: samsung,pin-function = 0xf; I have some vague recollection that I set interrupts to pin-function 0 by default for some reason (assuming they would go to 0xf when interrupts were enabled). ...but I can't for the life of me remember if it was a good reason or just seemed like the right thing to do. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting
Javier, On Fri, Aug 8, 2014 at 3:26 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: I have some vague recollection that I set interrupts to pin-function 0 by default for some reason (assuming they would go to 0xf when interrupts were enabled). ...but I can't for the life of me remember if it was a good reason or just seemed like the right thing to do. It would be great to know if there is a good reason. I see indeed that all pinctrl setup in the Peach Pit/Pi and Snow DTS that involves interrupts are using 0x0 as the pin function. Since as Tomasz explained Samsung SoC makes a difference between GPIO-IRQ and GPIO input I guess that it's better to change those to 0xf instead. What do you think? I think it's worth trying out. If there are no problems with it then let's do it. My vague recollection is that I was worried that pinctrl would take effect right at driver probe time (maybe this used to happen? or maybe I imagined it?) and that configuring to 0xf at this point in time would cause a spurious interrupt. I can't remember ever testing it so it was probably just something I imagined. Even if it was configured as 0xf I'd imagine that the interrupt would be masked anyway so there should be no spurious interrupt, right? Regardless of this though I think that both the patch to move the IRQ pinmux setup from .irq_set_type to the .irq_request_resources and the patch to to prevent any pinmux reconfiguration are good improvements to avoid future issues like the one we found here. OK. I'll let you, Tomasz, and Linus figure out what's best here since I haven't done extensive thinking on it. ;) -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support
Javier, On Mon, Aug 11, 2014 at 4:06 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Many Exynos5 boards (e.g: Snow, Peach Pit and Pi) have a SBS-compliant gas gauge battery. Enable it as module so the needed support is available for these boards. Suggested-by: Doug Anderson diand...@chromium.org Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Acked-by: Kukjin Kim kgene@samsung.com --- arch/arm/configs/exynos_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig index fc7d168..c390bb9 100644 --- a/arch/arm/configs/exynos_defconfig +++ b/arch/arm/configs/exynos_defconfig @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y CONFIG_I2C_S3C2410=y CONFIG_DEBUG_GPIO=y CONFIG_POWER_SUPPLY=y +CONFIG_BATTERY_SBS=m CONFIG_CHARGER_TPS65090=y # CONFIG_HWMON is not set CONFIG_THERMAL=y I'm good with this, so: Reviewed-by: Doug Anderson diand...@chromium.org To address Bartlomiej: =m is what's in ChromeOS for exynos, though I notice that other platforms in the ChromeOS tree have =y. I'm not sure why there is a difference, but it's probably just carelessness. Normally in ChromeOS we use =m for drivers that can afford to wait until after the UI is up and running. That helps improve boot time since the user can use the device sooner (and it's OK if the battery driver takes a few extra seconds to load). The only things that really should be =y are drivers that are critical to load early or that are needed for the user to interact with the system. In this case you could argue that loading sbs earlier means that if the system is low on battery it will get back to shutdown sooner. I'm not sure getting the shutdown a few seconds earlier is really critical, though... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 1/2] ARM: dts: Improve Peach Pit and Pi power scheme
Javier, On Mon, Aug 11, 2014 at 4:38 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: The DeviceTree files for the Peach Pit and Pi machines have a simplistic model of the connections between the different regulators since not all the tps65090 regulators get their input supply voltage from the VDC. DCDC1-3, LD0-1 and fet7 parent supply is indded VDC but the fet1-6 get their input supply from the DCDC1 and DCDC2 output voltage rails. Update the DeviceTree to better reflect the real connections between tps65090 regulators. Having this information in the DTS is useful since FETs are switches that don't provide an output voltage so the regulator core needs to fetch the FET parent output voltage if the child voltage is queried. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Acked-by: Mark Brown broo...@linaro.org --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 12 ++-- arch/arm/boot/dts/exynos5800-peach-pi.dts | 12 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) Matches my schematics. Kukjin: I think this could be applied to for-next whenever it's convenient. Acked-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support
Bartlomiej, On Mon, Aug 11, 2014 at 8:42 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Hi, On Monday, August 11, 2014 02:52:27 PM Javier Martinez Canillas wrote: Hello Bartlomiej, On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote: index fc7d168..c390bb9 100644 --- a/arch/arm/configs/exynos_defconfig +++ b/arch/arm/configs/exynos_defconfig @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y CONFIG_I2C_S3C2410=y CONFIG_DEBUG_GPIO=y CONFIG_POWER_SUPPLY=y +CONFIG_BATTERY_SBS=m Why not make it =y? Rationale: - currently no hardware related option uses =m in exynos_defconfig - it would match the SBS option usage in multi_v7_defconfig CONFIG_CHARGER_TPS65090=y # CONFIG_HWMON is not set CONFIG_THERMAL=y I know but personally I think this should be changed. The idea of having a multi platform kernel is to build a single kernel image that can be used to boot different platforms. Not all platforms have a SBS-compliant battery so this support shouldn't be built in the kernel image IMHO. This also matches to what real users will do since distributions most likely will have a minimal kernel and every possible hardware support will be enabled as a loadable kernel module. This is what distros do for other platforms too. If someone has a different use case and wants a kernel image that is optimized for a particular platform then she has to create its own defconfig anyways. Distributions usually use their own configs anyway and the current most popular use case for exynos_defconfig (not multi_v7_defconfig) seems to be to build kernel image alone and use it without any modules: $ grep =m arch/arm/configs/exynos_defconfig CONFIG_DM_CRYPT=m $ grep =m .config CONFIG_NET_IP_TUNNEL=m CONFIG_INET_TUNNEL=m CONFIG_IPV6=m CONFIG_INET6_XFRM_MODE_TRANSPORT=m CONFIG_INET6_XFRM_MODE_TUNNEL=m CONFIG_INET6_XFRM_MODE_BEET=m CONFIG_IPV6_SIT=m CONFIG_DM_CRYPT=m CONFIG_CRYPTO_RNG=m CONFIG_CRYPTO_ANSI_CPRNG=m What I'm trying to say is that there is a high probability that people will continue to use just the kernel image for exynos_defconfig and will therefore miss SBS battery support altogether (which is only 3.6 kB of code more in the kernel image so there is no much gain in making it modular currently). I'm not against making it =y for exynos_defconfig. I do pretty strongly agree that the multi_v7 version should be =m eventually, though. We'd need to do everything we can to make that kernel smaller. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 1/1] ARM: exynos_defconfig: Enable SBS battery support
Hi, On Mon, Aug 11, 2014 at 10:28 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Hi, On Monday, August 11, 2014 06:23:01 PM Javier Martinez Canillas wrote: Hello, On 08/11/2014 05:59 PM, Doug Anderson wrote: Bartlomiej, On Mon, Aug 11, 2014 at 8:42 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Hi, On Monday, August 11, 2014 02:52:27 PM Javier Martinez Canillas wrote: Hello Bartlomiej, On 08/11/2014 02:40 PM, Bartlomiej Zolnierkiewicz wrote: index fc7d168..c390bb9 100644 --- a/arch/arm/configs/exynos_defconfig +++ b/arch/arm/configs/exynos_defconfig @@ -77,6 +77,7 @@ CONFIG_SPI_S3C64XX=y CONFIG_I2C_S3C2410=y CONFIG_DEBUG_GPIO=y CONFIG_POWER_SUPPLY=y +CONFIG_BATTERY_SBS=m Why not make it =y? Rationale: - currently no hardware related option uses =m in exynos_defconfig - it would match the SBS option usage in multi_v7_defconfig CONFIG_CHARGER_TPS65090=y # CONFIG_HWMON is not set CONFIG_THERMAL=y I know but personally I think this should be changed. The idea of having a multi platform kernel is to build a single kernel image that can be used to boot different platforms. Not all platforms have a SBS-compliant battery so this support shouldn't be built in the kernel image IMHO. This also matches to what real users will do since distributions most likely will have a minimal kernel and every possible hardware support will be enabled as a loadable kernel module. This is what distros do for other platforms too. If someone has a different use case and wants a kernel image that is optimized for a particular platform then she has to create its own defconfig anyways. Distributions usually use their own configs anyway and the current most popular use case for exynos_defconfig (not multi_v7_defconfig) seems to be to build kernel image alone and use it without any modules: $ grep =m arch/arm/configs/exynos_defconfig CONFIG_DM_CRYPT=m $ grep =m .config CONFIG_NET_IP_TUNNEL=m CONFIG_INET_TUNNEL=m CONFIG_IPV6=m CONFIG_INET6_XFRM_MODE_TRANSPORT=m CONFIG_INET6_XFRM_MODE_TUNNEL=m CONFIG_INET6_XFRM_MODE_BEET=m CONFIG_IPV6_SIT=m CONFIG_DM_CRYPT=m CONFIG_CRYPTO_RNG=m CONFIG_CRYPTO_ANSI_CPRNG=m What I'm trying to say is that there is a high probability that people will continue to use just the kernel image for exynos_defconfig and will therefore miss SBS battery support altogether (which is only 3.6 kB of code more in the kernel image so there is no much gain in making it modular currently). I'm not against making it =y for exynos_defconfig. I do pretty strongly agree that the multi_v7 version should be =m eventually, though. We'd need to do everything we can to make that kernel smaller. Same for me. I don't have such a strong opinion about this so if you think that I should re-spin to change to =m, I will. I do think that we should try to keep the delta between exynos_defconfig and multi_v7_defconfig as small as possible and eventually even get rid of exynos_defconfig. Since building everything as built-in and having a config I completely agree. I proposed exynos_defconfig removal as soon as Exynos gained multiplatform support (and before exynos_defconfig started getting out-of-sync with multi_v7_defconfig). There were arguments that it is still useful in some cases. I think that if it would be possible to go from a modular multi_v7_defconfig to a modular/built-in single platform config (using a script?) all such use cases will be covered. There's one really nice thing about having an exynos_defconfig. It makes it a little easier to tease apart what's actually needed if you just want to build a product kernel that support exynos. Most product kernels shipping in embedded devices only support one or maybe a small number of SoCs. If you need to try to figure out what parts of the multi_v7 config is relevant then it's very hard. It's also nice to help measure multiplatform code bloat. You can keep measuring the output kernel size of the exynos kernel to see how much bloat all of the mutliplatform stuff has added. so I would vote for CONFIG_BATTERY_SBS=y for both configs. Especially since it results in only 3.6 kB bigger kernel image (0.05% kernel size increase for kernel image built with exynos_defconfig, probably a lot less for multi_v7_defconfig one). Death by 1000 cuts is a very painful death indeed, even if each cut is small. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] ARM: dts: Create cros-tps65090 fragment
Javier, On Tue, Aug 12, 2014 at 9:44 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: The tps65090 PMU is a component used in many ChromeOS devices so instead of having the same device tree definitions in many files, create a .dtsi fragment that can be included in DTS. This fragment is based on the DT definitions for Peach boards. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/cros-tps65090.dtsi | 81 1 file changed, 81 insertions(+) create mode 100644 arch/arm/boot/dts/cros-tps65090.dtsi I'd probably skip this patch (just have duplication in pit vs. pi), or make it peach specific (like exynos-peach-tps65090.dtsi?). This is really board-specific info and trying to guess what the various FETs are going to be for for all peach variants doesn't seem great. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: dw_mmc: Make sure we don't get stuck when we get an error
Hi, On Wed, May 21, 2014 at 2:08 AM, Seungwon Jeon tgih@samsung.com wrote: On Wed, May 21, 2014, Doug Anderson wrote: If we happened to get a data error at just the wrong time the dw_mmc driver could get into a state where it would never complete its request. That would leave the caller just hanging there. We fix this two ways and both of the two fixes on their own appear to fix the problems we've seen: 1. Fix a race in the tasklet where the interrupt setting the data error happens _just after_ we check for it, then we get a EVENT_XFER_COMPLETE. We fix this by repeating a bit of code. 2. Fix it so that if we detect that we've got an error in the data busy state and we're not going to do anything else we end the request and unblock anyone waiting. Signed-off-by: Doug Anderson diand...@chromium.org Signed-off-by: Yuvaraj Kumar C D yuvaraj...@gmail.com It will be applied after mmc: dw_mmc: change to use recommended reset procedure Acked-by: Seungwon Jeon tgih@samsung.com Thanks, Seungwon Jeon I saw that Ulf applied mmc: dw_mmc: change to use recommended reset procedure. Could we apply this one now, too? Do you want me to repost? -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REPOST PATCH v2] mmc: dw_mmc: Make sure we don't get stuck when we get an error
If we happened to get a data error at just the wrong time the dw_mmc driver could get into a state where it would never complete its request. That would leave the caller just hanging there. We fix this two ways and both of the two fixes on their own appear to fix the problems we've seen: 1. Fix a race in the tasklet where the interrupt setting the data error happens _just after_ we check for it, then we get a EVENT_XFER_COMPLETE. We fix this by repeating a bit of code. 2. Fix it so that if we detect that we've got an error in the data busy state and we're not going to do anything else we end the request and unblock anyone waiting. Signed-off-by: Doug Anderson diand...@chromium.org Signed-off-by: Yuvaraj Kumar C D yuvaraj...@gmail.com Acked-by: Seungwon Jeon tgih@samsung.com --- Changes in v2: - Removed TODO - Set cmd to NULL before calling dw_mci_request_end() drivers/mmc/host/dw_mmc.c | 46 ++ 1 file changed, 46 insertions(+) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 8f216ed..7f227e9 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1299,6 +1299,14 @@ static void dw_mci_tasklet_func(unsigned long priv) /* fall through */ case STATE_SENDING_DATA: + /* +* We could get a data error and never a transfer +* complete so we'd better check for it here. +* +* Note that we don't really care if we also got a +* transfer complete; stopping the DMA and sending an +* abort won't hurt. +*/ if (test_and_clear_bit(EVENT_DATA_ERROR, host-pending_events)) { dw_mci_stop_dma(host); @@ -1312,7 +1320,29 @@ static void dw_mci_tasklet_func(unsigned long priv) break; set_bit(EVENT_XFER_COMPLETE, host-completed_events); + + /* +* Handle an EVENT_DATA_ERROR that might have shown up +* before the transfer completed. This might not have +* been caught by the check above because the interrupt +* could have gone off between the previous check and +* the check for transfer complete. +* +* Technically this ought not be needed assuming we +* get a DATA_COMPLETE eventually (we'll notice the +* error and end the request), but it shouldn't hurt. +* +* This has the advantage of sending the stop command. +*/ + if (test_and_clear_bit(EVENT_DATA_ERROR, + host-pending_events)) { + dw_mci_stop_dma(host); + send_stop_abort(host, data); + state = STATE_DATA_ERROR; + break; + } prev_state = state = STATE_DATA_BUSY; + /* fall through */ case STATE_DATA_BUSY: @@ -1335,6 +1365,22 @@ static void dw_mci_tasklet_func(unsigned long priv) /* stop command for open-ended transfer*/ if (data-stop) send_stop_abort(host, data); + } else { + /* +* If we don't have a command complete now we'll +* never get one since we just reset everything; +* better end the request. +* +* If we do have a command complete we'll fall +* through to the SENDING_STOP command and +* everything will be peachy keen. +*/ + if (!test_bit(EVENT_CMD_COMPLETE, + host-pending_events)) { + host-cmd = NULL; + dw_mci_request_end(host, mrq); + goto unlock; + } } /* -- 2.1.0.rc2.206.gedb03e5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] ARM: dts: Add vmmc and vqmmc supplies for Peach Pit and Pi boards
Javier, On Tue, Aug 19, 2014 at 8:08 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: The VCC/VDD and VCCQ/VDD_IO power supplies for the MMC are provided by the tps65090 fet4 and max77802 ldo4 regulators respectively. Add the phandle to the regulators tree nodes for the the dw_mmc device device. These DTS snippets were taken from the downstream ChromeOS 3.8 kernel Device Tree for Peach Pit and Pi boards. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 2 ++ arch/arm/boot/dts/exynos5800-peach-pi.dts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts index 8e50042..5b9dbb9 100644 --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts @@ -547,6 +547,8 @@ }; mmc_2 { + vmmc-supply = tps65090_fet4; + vqmmc-supply = vqmmc_sdcard; status = okay; num-slots = 1; supports-highspeed; diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts index 939f91c..dcac443 100644 --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts @@ -545,6 +545,8 @@ }; mmc_2 { + vmmc-supply = tps65090_fet4; + vqmmc-supply = vqmmc_sdcard; status = okay; num-slots = 1; supports-highspeed; While your change is correct, I have a worry that it will break things if it's merged before some patches that Yuvaraj is working on. Specifically the problem on pit and pi (and any exynos5250 / 5420 / 5800 / ... boards using the built-in card detect) is that the card detect line is on the same power rail as vqmmc. That means you can't turn off vqmmc if you still need to be able to detect card insertions. ...but you can't turn off vmmc without turning off vqmmc, otherwise current will leak through the IO lines into the card, which is bad. Right now the SDMMC core will try to turn off power to the card at two times: 1. when the card is ejected 2. when it's trying to reset the card Obviously the first problem is a huge problem on exynos because it means that we won't be able to detect card insertions. ...but we still want to turn the power off from #2. To really fix the problem I think the core needs to be extended to treat the above as two separate cases. Your patch might work at the moment because I think dw_mmc doesn't actually try to turn off these rails with the main SDMMC core asks it to. ...but I still worry about merging them before Yuvaraj's changes are ready. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PATCH 3/7] mfd: cros_ec: stop calling -cmd_xfer() directly
Javier, On Wed, Aug 20, 2014 at 5:13 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: From: Andrew Bresticker abres...@chromium.org Instead of having users of the ChromeOS EC call the interface-specific cmd_xfer() callback directly, introduce a central cros_ec_cmd_xfer() to use instead. This will allow us to put all the locking and retry logic in one place instead of duplicating it across the different drivers. Signed-off-by: Andrew Bresticker abres...@chromium.org Reviewed-by: Simon Glass s...@chromium.org Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +- drivers/input/keyboard/cros_ec_keyb.c | 2 +- drivers/mfd/cros_ec.c | 7 +++ include/linux/mfd/cros_ec.h | 24 ++-- 4 files changed, 27 insertions(+), 8 deletions(-) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart
Guenter, On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck li...@roeck-us.net wrote: machine_restart is supported on non-ARM platforms, and and ultimately calls arm_pm_restart, so dont call arm_pm_restart directly but use the more generic function. Cc: Russell King li...@arm.linux.org.uk Do you need to submit this to his patch tracker to get him to pick it up? How are you envisioning that this series land? It crosses a lot of boundaries so I guess will need a reasonable amount of coordination between maintainers... Signed-off-by: Guenter Roeck li...@roeck-us.net Acked-by: Catalin Marinas catalin.mari...@arm.com Acked-by: Heiko Stuebner he...@sntech.de --- v7: No change. v6: No change. v5: No change. v4: No change. v3: No change. v2: Added patch. drivers/power/reset/restart-poweroff.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Doug Anderson diand...@chromium.org Tested-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 04/11] arm: Support restart through restart handler call chain
Guenter, On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck li...@roeck-us.net wrote: The kernel core now supports a restart handler call chain for system restart functions. With this change, the arm_pm_restart callback is now optional, so drop its initialization and check if it is set before calling it. Only call the kernel restart handler if arm_pm_restart is not set. Signed-off-by: Guenter Roeck li...@roeck-us.net Acked-by: Catalin Marinas catalin.mari...@arm.com Acked-by: Heiko Stuebner he...@sntech.de --- v7: Dropped null_restart and made arm_pm_restart truly optional. v6: No change. v5: Renamed restart function to do_kernel_restart v4: No change. v3: Use wrapper function to execute notifier call chain. v2: Only call notifier call chain if arm_pm_restart is not set. Do not include linux/watchdog.h. arch/arm/kernel/process.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) Reviewed-by: Doug Anderson diand...@chromium.org Tested-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 08/11] arm/arm64: Unexport restart handlers
Guenter, On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck li...@roeck-us.net wrote: Implementing a restart handler in a module don't make sense as there would be no guarantee that the module is loaded when a restart is needed. Unexport arm_pm_restart to ensure that no one gets the idea to do it anyway. Signed-off-by: Guenter Roeck li...@roeck-us.net Acked-by: Catalin Marinas catalin.mari...@arm.com Acked-by: Heiko Stuebner he...@sntech.de --- v7: No change v6: No change v5: No change v4: No change v3: No change v2: No change arch/arm/kernel/process.c | 1 - arch/arm64/kernel/process.c | 1 - 2 files changed, 2 deletions(-) Reviewed-by: Doug Anderson diand...@chromium.org Tested-by: Doug Anderson diand...@chromium.org (FYI: all patches tested by me were tested on rk3288-evb) -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 03/11] arm64: Support restart through restart handler call chain
Guenter, On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck li...@roeck-us.net wrote: The kernel core now supports a restart handler call chain to restart the system. Call it if arm_pm_restart is not set. Signed-off-by: Guenter Roeck li...@roeck-us.net Acked-by: Catalin Marinas catalin.mari...@arm.com Acked-by: Heiko Stuebner he...@sntech.de --- v7: No change. v6: No change. v5: Renamed restart function to do_kernel_restart v4: No change. v3: Use wrapper function to execute notifier call chain. v2: Only call notifier call chain if arm_pm_restart is not set. Do not include linux/watchdog.h. arch/arm64/kernel/process.c | 2 ++ 1 file changed, 2 insertions(+) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 11/11] clk: rockchip: add restart handler
Guenter / Heiko, On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck li...@roeck-us.net wrote: From: Heiko Stübner he...@sntech.de Add infrastructure to write the correct value to the restart register and register the restart notifier for both rk3188 (including rk3066) and rk3288. Signed-off-by: Heiko Stuebner he...@sntech.de Signed-off-by: Guenter Roeck li...@roeck-us.net --- v7: Added patch to series. drivers/clk/rockchip/clk-rk3188.c | 2 ++ drivers/clk/rockchip/clk-rk3288.c | 2 ++ drivers/clk/rockchip/clk.c| 25 + drivers/clk/rockchip/clk.h| 1 + 4 files changed, 30 insertions(+) This patch doesn't apply cleanly with the in-flight (clk: rockchip: protect critical clocks from getting disabled) patch from Heiko. It's trivial to resolve and unclear which will land first, so I think it's fine... Reviewed-by: Doug Anderson diand...@chromium.org Tested-by: Doug Anderson diand...@chromium.org (FYI: all patches tested by me were tested on rk3288-evb) -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart
Guenter, On Wed, Aug 20, 2014 at 9:42 PM, Guenter Roeck li...@roeck-us.net wrote: On Wed, Aug 20, 2014 at 09:10:31PM -0700, Doug Anderson wrote: Guenter, On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck li...@roeck-us.net wrote: machine_restart is supported on non-ARM platforms, and and ultimately calls arm_pm_restart, so dont call arm_pm_restart directly but use the more generic function. Cc: Russell King li...@arm.linux.org.uk Do you need to submit this to his patch tracker to get him to pick it up? How are you envisioning that this series land? It crosses a lot of boundaries so I guess will need a reasonable amount of coordination between maintainers... If I get an Acked-by: from all maintainers, I could send a pull request to Linus directly. How do I send a patch to Russell's patch tracker ? I thought I copied all mailing lists suggested by get_maintainer.pl, but maybe I missed one. Ah, OK. Perhaps it's best to ignore me, then. ;) FYI: Russell's Patch System is at http://www.arm.linux.org.uk/developer/patches/ and is used for getting individual patches into branches maintained by Russel. If you're just going to send a pull request for the series then I don't think you need it. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] ARM: dts: Prepare node labels for exynos5250
Kukjin, On Fri, Aug 22, 2014 at 3:49 AM, Kukjin Kim kgene@samsung.com wrote: Andreas Färber wrote: Allows them to be extended by reference. Reviewed-by: Doug Anderson diand...@chromium.org Signed-off-by: Andreas Färber afaer...@suse.de --- v6 - v7: * Dropped uart* labels (Tomasz Figa) v6: Split off from Snow/SMDK cleanups (Doug Anderson) arch/arm/boot/dts/exynos5250.dtsi | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 492e1eff37bd..42eafd19cfb2 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -218,7 +218,7 @@ clock-names = fimg2d; }; - codec@1100 { + mfc: codec@1100 { compatible = samsung,mfc-v6; reg = 0x1100 0x1; interrupts = 0 96 0; @@ -227,7 +227,7 @@ clock-names = mfc; }; - rtc@101E { + rtc: rtc@101E { clocks = clock CLK_RTC; clock-names = rtc; status = disabled; @@ -261,7 +261,7 @@ clock-names = uart, clk_uart_baud0; }; - sata@122F { + sata: sata@122F { compatible = snps,dwc-ahci; samsung,sata-freq = 66; reg = 0x122F 0x1ff; @@ -573,7 +573,7 @@ #phy-cells = 1; }; - usb@1211 { + ehci: usb@1211 { I'm not sure which one is recommended between above and ehci: ehci@1211 { My understanding is that more generic names should be used for node names. AKA: usb and not ehci. mmc and not dwmmc. In another thread I was pointed at ePAPR http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf: The name of a node should be somewhat generic, reflecting the function of the device and not its precise programming model. - hdmi { + hdmi: hdmi { Should be + hdmi: hdmi@1453 { ? Seems like that should be a followon patch. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 2/3] mmc: dw_mmc: Support voltage changes
Ulf, On Fri, Aug 22, 2014 at 8:35 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: From: Doug Anderson diand...@chromium.org For UHS cards we need the ability to switch voltages from 3.3V to 1.8V. Add support to the dw_mmc driver to handle this. Note that dw_mmc needs a little bit of extra code since the interface needs a special bit programmed to the CMD register while CMD11 is progressing. This means adding a few extra states to the state machine to track. Signed-off-by: Doug Anderson diand...@chromium.org Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com --- changes since v1: 1. Added error message and return error in case of regulator_set_voltage() fail. 2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE) to dw_mci_cmd_interrupt(host,pending). 3. Removed unnecessary comments. drivers/mmc/host/dw_mmc.c | 134 +--- drivers/mmc/host/dw_mmc.h |5 +- include/linux/mmc/dw_mmc.h |2 + 3 files changed, 131 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index aadb0d6..f20b4b8 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -29,6 +29,7 @@ #include linux/irq.h #include linux/mmc/host.h #include linux/mmc/mmc.h +#include linux/mmc/sd.h #include linux/mmc/sdio.h #include linux/mmc/dw_mmc.h #include linux/bitops.h @@ -234,10 +235,13 @@ err: } #endif /* defined(CONFIG_DEBUG_FS) */ +static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg); + static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) { struct mmc_data *data; struct dw_mci_slot *slot = mmc_priv(mmc); + struct dw_mci *host = slot-host; const struct dw_mci_drv_data *drv_data = slot-host-drv_data; u32 cmdr; cmd-error = -EINPROGRESS; @@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) else if (cmd-opcode != MMC_SEND_STATUS cmd-data) cmdr |= SDMMC_CMD_PRV_DAT_WAIT; + if (cmd-opcode == SD_SWITCH_VOLTAGE) { + u32 clk_en_a; + + /* Special bit makes CMD11 not die */ + cmdr |= SDMMC_CMD_VOLT_SWITCH; + + /* Change state to continue to handle CMD11 weirdness */ + WARN_ON(slot-host-state != STATE_SENDING_CMD); + slot-host-state = STATE_SENDING_CMD11; + + /* +* We need to disable clock stop while doing voltage switch +* according to Voltage Switch Normal Scenario. +* It's assumed that by the next time the CLKENA is updated +* (when we set the clock next) that the voltage change will +* be over, so we don't bother setting any bits to synchronize +* with dw_mci_setup_bus(). +*/ I don't know the details about the dw_mmc controller, but normally a host driver is expected to gate the clock from it's -set_ios callback, when the clk frequency are set to 0. Could you elaborate on why dw_mmc can't do that, but need to handle this from here? Let's see, it's been a while since I wrote this... So dw_mmc has a special feature where the controller itself will automatically stop the MMC Card clock when nothing is going on. This is called low power mode. I'm not super familiar with other MMC drivers, I get the impression that this is a special dw_mmc feature and not common to other controllers. I think other drivers have aggressive runtime PM to get similar power savings. The dw_mmc auto clock gating can wreck total havoc on the voltage change procedure, since part of the way that the host and card communicate is that the host is supposed to stop its clock when it gets to a certain phase of the voltage switch sequence. If the controller is stopping the clock for us then it can confuse the card. The dw_mmc manual says that before starting a voltage change you must turn off low power mode. That's what we're doing here. The comment above refers to the fact dw_mci_setup_bus() will unconditionally turn low power mode back on for us when called with a non-zero clock. If dw_mci_setup_bus() might be called with a non-zero clock during the voltage change then this would be disaster (low power mode would be back on!). ...but the clock will always be zero during the voltage change and when it's finally non-zero then it's the last stage of the voltage change and we can go back to low power mode. Possibly the above was not super clear from the comment (even for those familiar with the oddities of dw_mmc). If you want me to try to rewrite the comment, let me know. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body
Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators
Hi, On Fri, Aug 22, 2014 at 3:02 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Hello Mark, On 08/22/2014 08:30 PM, Mark Brown wrote: The problem is that one of these regulators is used as the vqmmc-supply (VCCQ/VDD_IO) so the mmc host controller driver disables it on MMC_POWER_OFF. Now AFAIK (Yuvaraj can correct me what I got wrong) this shouldn't be an issue since on card detection, the vqmmc supply should be enabled again but on Exynos the built-in card detect line is on the same power rail as vqmmc. That means that disabling the regulator prevents card insertions to be detected. If the MMC host controller needs a supply enabling in order to do card detection and it's supposed to be doing card detection I'd expect it to be enabling that supply. Why is it not doing that? Good question. I'm not that familiar with the dw_mmc host controller nor its driver implementation so I'll let Yuvaraj or Doug to answer that. I haven't seen the issue that Yuvaraj is reporting (but I also haven't picked up all of the relevant patches and tried to reproduce), so I'm going to have to leave it to Yuvaraj to answer. As far as I know the dw_mmc driver ought to be enabling vqmmc when it needs it. Perhaps there's a bug in your patch series that adds vqmmc support to dw_mmc? -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators
Jaehoon, On Mon, Aug 25, 2014 at 5:32 AM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/22/2014 10:47 PM, Yuvaraj Kumar C D wrote: This patch makes use of mmc_regulator_get_supply() to handle the vmmc and vqmmc regulators.Also it moves the code handling the these regulators to dw_mci_set_ios().It turned on the vmmc and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off during MMC_POWER_OFF. Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com --- changes from v1: 1.Used mmc_regulator_set_ocr() instead of regulator_enable() for vmmc. 2.Turned on vmmc and vqmmc during MMC_POWER_UP. 3. Removed the flags DW_MMC_CARD_POWERED and DW_MMC_IO_POWERED which added during the initial version of this patch. 4. Added error message, if it failed to turn on regulator's. drivers/mmc/host/dw_mmc.c | 72 +--- include/linux/mmc/dw_mmc.h |2 +- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 7f227e9..aadb0d6 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -936,6 +936,7 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) struct dw_mci_slot *slot = mmc_priv(mmc); const struct dw_mci_drv_data *drv_data = slot-host-drv_data; u32 regs; + int ret; switch (ios-bus_width) { case MMC_BUS_WIDTH_4: @@ -974,12 +975,38 @@ static void dw_mci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) switch (ios-power_mode) { case MMC_POWER_UP: + if (!IS_ERR(mmc-supply.vmmc)) { + ret = mmc_regulator_set_ocr(mmc, mmc-supply.vmmc, + ios-vdd); + if (ret) { + dev_err(slot-host-dev, + failed to enable vmmc regulator\n); + /*return, if failed turn on vmmc*/ + return; + } + } + if (!IS_ERR(mmc-supply.vqmmc) !slot-host-vqmmc_enabled) { Can't use the regulator_is_enabled() instead of slot-host-vqmmc_enabled? I think we mentioned this before, but regulator_is_enabled() won't do what you want with shared regulators since they are refcounted. You need to keep track of whether you've enabled the regulator yourself. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc
Ulf, On Mon, Aug 25, 2014 at 1:13 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 20:27, Sonny Rao sonny...@chromium.org wrote: On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: Exynos 5250 and 5420 based boards uses built-in CD# line for card detection.But unfortunately CD# line is on the same voltage rails as of I/O voltage rails. When we cut off vqmmc,the consequent card detection will break in these boards. I am not sure I follow here. Is the card detect mechanism handled internally by the dw_mmc controller? Yes Just out of curiosity. Do you know how the power to the actual dw_mmc controller is handled? I expect it to be SoC specific and I am guessing power domain regulators may be involved!? You can likely read the dw_mmc registers when vqmmc is off. Is that what you're asking? Certainly if vqmmc is not powered then the lines themselves will be useless, won't they? The vqmmc supply goes to the VDDQ_MMC2 pin on 5420. In my 5420 user manual, I see that clk, cmd, cd, datN, wp and biuvr pins are all in this same voltage (VDDQ_MMC2) domain. Can you really read a pin without powering that part of the SoC? I thought HW engineers long time ago realized that this should be done separately on a GPIO line to be able to save power while waiting for a card to be inserted. But that's not case then? At least in my limited experience, this seems to be common among SoC vendors who are using dw_mmc, as we've seen this elsewhere as well and after seeing it here we know that we need to ignore the CD pin that's routed to dw_mmc and use a separately powered GPIO on the board, but still there are probably many SoCs/boards which are doing it this way. These hosts (obviously) need to keep vqmmc (and thus vmmc) on all the time, even when the mmc core tells them to power off. However, one problem is that these cards won't properly handle mmc_power_cycle(). That's needed to handle error cases when trying to switch voltages (see 0797e5f mmc:core: Fixup signal voltage switch). This patch adds a new MMC_POWER_OFF_HARD mode when it's doing a power cycle. This mode differs from the normal MMC_POWER_OFF mode in that the mmc core will promise to power the slot back on before it expects the host to detect card insertion or removal. This patch is based off of one that Doug wrote (sent privately to Yuvaraj) which just modifies the MMC core, and should be split into two patches. One that modifies the mmc core and one that implements this in dw_mmc. I looked at the mmc core parts, it seems like the wrong approach. I think you shall be able use MMC_CAP_NEEDS_POLL, to handle this broken card detect mechanism. We even have a DT binding for that, broken-cd. I don't think this is possible, but let me explain why I think so and you can correct me. The voltage domain of the card detect pin on the SoC is vqmmc, right? That means that you won't be able to read the pin without turning on vqmmc. Even if you could read the pin without turning on vqmmc, the pullup on this line is connected to vqmmc too. ...so if vqmmc is off then there's no pulup and you can't use card detect. Are you suggesting that we should flip the voltage of vqmmc (and thus vmmc to prevent damaging the card) during polling? That seems ugly. One other thing to mention: we didn't find any power savings by actually turning off vmmc and vqmmc when there was no card inserted. There's no current running through the lines when there is no card inserted and apparently everything is efficient enough that there was no problem. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc
Jaehoon, On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/25/2014 05:13 PM, Ulf Hansson wrote: On 22 August 2014 20:27, Sonny Rao sonny...@chromium.org wrote: On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: Exynos 5250 and 5420 based boards uses built-in CD# line for card detection.But unfortunately CD# line is on the same voltage rails as of I/O voltage rails. When we cut off vqmmc,the consequent card detection will break in these boards. I didn't know that use CD# line for card detect. And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. Which card is used with same voltages? (eMMC? SD? SDIO?) Well, I have checked Exynos5250 and 5420, but it looks like not same rails. I'm not sure I totally understood what you said. In my manual I have a table titled Table 2-1 Exynos 5420 Pin List. Look in this table for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and you'll see the power domain. For both it shows VDDQ_MMC2. If that doesn't mean that the two are in the same voltage domain then I don't know what does. Can you point to any examples where they have different voltage domains? I agree that what exynos does is not sensible, but that's what it is. I am not sure I follow here. Is the card detect mechanism handled internally by the dw_mmc controller? Yes What card detect mechanism? The dw_mmc controller has a way to read the card detect, right? That's internal to the controller. The alternative would be to use a generic GPIO for card detect. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/2] regulator: Add driver for max77802 PMIC PMIC regulators
Javier, On Mon, Aug 25, 2014 at 2:07 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Hello Yuvaraj, On 08/25/2014 10:22 AM, Yuvaraj Cd wrote: Good question. I'm not that familiar with the dw_mmc host controller nor its driver implementation so I'll let Yuvaraj or Doug to answer that. Well,here it goes! 1. Power ON the board LDO4CTRL1[7:6] 11b 2. dw_mmc driver enable the vqmmc. 3. checks for UHS support, complete the voltage switching t0 1.8V 4. Does warm reset by reboot command. 5. mmc core calls mmc_set_ios() with MMC_POWER_OFF. 6. dw_mmc driver cut-off the regulator with LDO4CTRL1[7:6] is 00b 7.dw_mmc driver enable the vqmmc. But after step 7 also, LD4CTRL[7:6] is 00b. Ok, so the dw_mmc driver is enabling vqmmc, that's good. I haven't seen the issue that Yuvaraj is reporting (but I also haven't picked up all of the relevant patches and tried to reproduce), so I'm going to have to leave it to Yuvaraj to answer. static int max77802_enable(struct regulator_dev *rdev) { struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev); int id = rdev_get_id(rdev); int shift = max77802_get_opmode_shift(id); return regmap_update_bits(rdev-regmap, rdev-desc-enable_reg,rdev-desc-enable_mask,max77802-opmode[id] shift); } I think in the above code snippet, the val is what we got it during the probe.We always write that value for enabling this regulator(which is LDO4CTRL1[7:6] 00b after warm reset) which is not correct according the MAX77802 manual. I see, so probably until we have a way to define the operating mode for each regulator using DT we should set the opmode to normal when enabling a regulator independently of the value the hardware register reported on probe. Can you please test the following change [0] so I can post as a proper patch? Doug, Mark do you think that forcing the regulator to opmode normal when enabling is the right solution here? IMHO that makes sense. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 2/3] mmc: dw_mmc: Support voltage changes
Ulf, On Mon, Aug 25, 2014 at 1:31 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 22:38, Doug Anderson diand...@google.com wrote: Ulf, On Fri, Aug 22, 2014 at 8:35 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: From: Doug Anderson diand...@chromium.org For UHS cards we need the ability to switch voltages from 3.3V to 1.8V. Add support to the dw_mmc driver to handle this. Note that dw_mmc needs a little bit of extra code since the interface needs a special bit programmed to the CMD register while CMD11 is progressing. This means adding a few extra states to the state machine to track. Signed-off-by: Doug Anderson diand...@chromium.org Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com --- changes since v1: 1. Added error message and return error in case of regulator_set_voltage() fail. 2. changed dw_mci_cmd_interrupt(host,pending | SDMMC_INT_CMD_DONE) to dw_mci_cmd_interrupt(host,pending). 3. Removed unnecessary comments. drivers/mmc/host/dw_mmc.c | 134 +--- drivers/mmc/host/dw_mmc.h |5 +- include/linux/mmc/dw_mmc.h |2 + 3 files changed, 131 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index aadb0d6..f20b4b8 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -29,6 +29,7 @@ #include linux/irq.h #include linux/mmc/host.h #include linux/mmc/mmc.h +#include linux/mmc/sd.h #include linux/mmc/sdio.h #include linux/mmc/dw_mmc.h #include linux/bitops.h @@ -234,10 +235,13 @@ err: } #endif /* defined(CONFIG_DEBUG_FS) */ +static void mci_send_cmd(struct dw_mci_slot *slot, u32 cmd, u32 arg); + static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) { struct mmc_data *data; struct dw_mci_slot *slot = mmc_priv(mmc); + struct dw_mci *host = slot-host; const struct dw_mci_drv_data *drv_data = slot-host-drv_data; u32 cmdr; cmd-error = -EINPROGRESS; @@ -253,6 +257,31 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) else if (cmd-opcode != MMC_SEND_STATUS cmd-data) cmdr |= SDMMC_CMD_PRV_DAT_WAIT; + if (cmd-opcode == SD_SWITCH_VOLTAGE) { + u32 clk_en_a; + + /* Special bit makes CMD11 not die */ + cmdr |= SDMMC_CMD_VOLT_SWITCH; + + /* Change state to continue to handle CMD11 weirdness */ + WARN_ON(slot-host-state != STATE_SENDING_CMD); + slot-host-state = STATE_SENDING_CMD11; + + /* +* We need to disable clock stop while doing voltage switch +* according to Voltage Switch Normal Scenario. +* It's assumed that by the next time the CLKENA is updated +* (when we set the clock next) that the voltage change will +* be over, so we don't bother setting any bits to synchronize +* with dw_mci_setup_bus(). +*/ I don't know the details about the dw_mmc controller, but normally a host driver is expected to gate the clock from it's -set_ios callback, when the clk frequency are set to 0. Could you elaborate on why dw_mmc can't do that, but need to handle this from here? Let's see, it's been a while since I wrote this... So dw_mmc has a special feature where the controller itself will automatically stop the MMC Card clock when nothing is going on. This is called low power mode. I'm not super familiar with other MMC drivers, I get the impression that this is a special dw_mmc feature and not common to other controllers. I think other drivers have aggressive runtime PM to get similar power savings. I see. I am familiar with such low power mode features, there are certainly other controllers supporting such as well. My experience tells me, it's hard to get things right for all corner cases. The voltage switch behaviour is just one of these, then you have SDIO irq etc. Instead of using the controller HW, yes you may implement clock gating through runtime PM in the host driver. The dw_mmc auto clock gating can wreck total havoc on the voltage change procedure, since part of the way that the host and card communicate is that the host is supposed to stop its clock when it gets to a certain phase of the voltage switch sequence. If the controller is stopping the clock for us then it can confuse the card. The dw_mmc manual says that before starting a voltage change you must turn off low power mode. That's what we're doing here. The comment above refers to the fact dw_mci_setup_bus() will unconditionally turn low power mode back on for us when called with a non-zero clock. If dw_mci_setup_bus() might be called
Re: [PATCH 1/1] regulator: max77802: set opmode to normal if off is read from hw
Javier, On Tue, Aug 26, 2014 at 4:37 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: The max77802 driver reads the default operating mode (opmode) set for regulators when enabled from the hardware registers. But if a regulator is disabled and the system warm restarted, the hardware reports OFF as the opmode so the regulator is not enabled. Default to operating mode NORMAL if OFF is read from the hardware register. Reported-by: Yuvaraj Cd yuvaraj.l...@gmail.com Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- This patch fixes the issue reported in https://lkml.org/lkml/2014/8/25/69 drivers/regulator/max77802.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c index ad1caa9..967e109 100644 --- a/drivers/regulator/max77802.c +++ b/drivers/regulator/max77802.c @@ -540,7 +540,17 @@ static int max77802_pmic_probe(struct platform_device *pdev) config.of_node = pdata-regulators[i].of_node; ret = regmap_read(iodev-regmap, regulators[i].enable_reg, val); - max77802-opmode[id] = val shift MAX77802_OPMODE_MASK; + val = val shift MAX77802_OPMODE_MASK; + + /* +* If the regulator is disabled and the system warm rebooted, +* the hardware reports OFF as the regulator operating mode. +* Default to operating mode NORMAL in that case. +*/ + if (val == MAX77802_OPMODE_OFF) + max77802-opmode[id] = MAX77802_OPMODE_NORMAL; + else + max77802-opmode[id] = val; Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc
Jaehoon, On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi, Doug, On 08/26/2014 12:25 AM, Doug Anderson wrote: Jaehoon, On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/25/2014 05:13 PM, Ulf Hansson wrote: On 22 August 2014 20:27, Sonny Rao sonny...@chromium.org wrote: On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: Exynos 5250 and 5420 based boards uses built-in CD# line for card detection.But unfortunately CD# line is on the same voltage rails as of I/O voltage rails. When we cut off vqmmc,the consequent card detection will break in these boards. I didn't know that use CD# line for card detect. And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. Which card is used with same voltages? (eMMC? SD? SDIO?) Well, I have checked Exynos5250 and 5420, but it looks like not same rails. I'm not sure I totally understood what you said. In my manual I have a table titled Table 2-1 Exynos 5420 Pin List. Look in this table for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and you'll see the power domain. For both it shows VDDQ_MMC2. If that doesn't mean that the two are in the same voltage domain then I don't know what does. Can you point to any examples where they have different voltage domains? I think you're mis-understanding for it. Right, It's described at exynos5420, but it's not connected. It's not connected. What do you mean? If I were to guess I'd say that on some particular board you're looking at they don't happen to use the CD pin for card detect. If this is what you mean, it doesn't help me. exynos5420-peach-pit does use the CD pin for card detect. You can look at the DTS file and confirm it. ...or are you saying that the CD pin somehow changes voltage domains when configured as a GPIO? I find that very hard to believe. What voltage domain does it go to? If it goes to a 1.8V voltage domain then that would be bad when vqmmc was 3.3V. If it goes to a 3.3V voltage domain then that would be bad when vqmmc was 1.8V. Remember that externally we've got a pull up to vqmmc. Even if somehow magically we can read the card detect pin with vqmmc off, we have an external pullup on this line that goes directly to the vqmmc power rail. If the vqmmc power rail is off then this external pull up would not work and would actually act as an external pull down if you could somehow configure the internal line to be a pullup. Exynos4 series are also described, but we used the broken card detection scheme and power used one of always-on powers. Because Card-detection rail need to enable as always-on. We don't need to consider this. I checked the circuit, this patch didn't need. exynos5 also used the gpio-pin for card-detection. And we can use the slot-gpio API. When you say exynos5, what do you mean here? Do you mean the smdk for 5250, or something else? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc
Jaehoon, On Tue, Aug 26, 2014 at 9:47 PM, Jaehoon Chung jh80.ch...@samsung.com wrote: Doug, On 08/27/2014 01:14 PM, Doug Anderson wrote: Jaehoon, On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi, Doug, On 08/26/2014 12:25 AM, Doug Anderson wrote: Jaehoon, On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/25/2014 05:13 PM, Ulf Hansson wrote: On 22 August 2014 20:27, Sonny Rao sonny...@chromium.org wrote: On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: Exynos 5250 and 5420 based boards uses built-in CD# line for card detection.But unfortunately CD# line is on the same voltage rails as of I/O voltage rails. When we cut off vqmmc,the consequent card detection will break in these boards. I didn't know that use CD# line for card detect. And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. Which card is used with same voltages? (eMMC? SD? SDIO?) Well, I have checked Exynos5250 and 5420, but it looks like not same rails. I'm not sure I totally understood what you said. In my manual I have a table titled Table 2-1 Exynos 5420 Pin List. Look in this table for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and you'll see the power domain. For both it shows VDDQ_MMC2. If that doesn't mean that the two are in the same voltage domain then I don't know what does. Can you point to any examples where they have different voltage domains? I think you're mis-understanding for it. Right, It's described at exynos5420, but it's not connected. It's not connected. What do you mean? If I were to guess I'd say that on some particular board you're looking at they don't happen to use the CD pin for card detect. If this is what you mean, it doesn't help me. exynos5420-peach-pit does use the CD pin for card detect. You can look at the DTS file and confirm it. I didn't know how exynos5420-peach-pit's circuit is configured. But i guess that almost all exynos5 boards are configured with the similar circuit. At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain. (CD-pin is external card-detect pin. - like XEINT_# pin) You mentioned CD# and DATA# lines is used the same power domain, right? In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply. Not use the CD# pin, used the XEINT_# pin. So i think we don't need to consider the CD#. If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed. Maybe on your board you have CD connected to a gpx line. ...but not mine. The guys who designed our hardware followed the SMDK5420 reference schematics which connect the SD card slot card detect to gpc2_2, which is the card detect pin. See arch/arm/boot/dts/exynos5420-smdk5420.dts, specifically noting the lack of a GPIO card detect and the inclusion of sd2_cd mmc@1222 { status = okay; card-detect-delay = 200; samsung,dw-mshc-ciu-div = 3; samsung,dw-mshc-sdr-timing = 2 3; samsung,dw-mshc-ddr-timing = 1 2; pinctrl-names = default; pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; bus-width = 4; cap-sd-highspeed; }; See arch/arm/boot/dts/exynos5420-peach-pit.dts too: mmc_2 { status = okay; num-slots = 1; cap-sd-highspeed; card-detect-delay = 200; clock-frequency = 4; samsung,dw-mshc-ciu-div = 3; samsung,dw-mshc-sdr-timing = 2 3; samsung,dw-mshc-ddr-timing = 1 2; pinctrl-names = default; pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; bus-width = 4; }; Here, see this ASCII art that shows how some lines are hooked up on peach-pit. You might need to paste this into something with a fixed-width font. + |Exynos 5420 | | P2.8V_LOUT4 -|- VDDQ_MMC2 (AK7) || || +-+- 10K res -+|- XMMC2CDN (AK6) | || | || | || | Ext CD | | | +-- 10K res-+--+---|- XMMC2CMD (AK8) | | Ext CMD You can see from the above that the external card detect signal (that goes to the connector) named Ext CD is pulled up to the same voltage as the external CMD signal (that also goes to the connector). This is vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act as a pull down, or in the best case it will be floating. Said another way: we can't read card detect when vqmmc is off. Your commit message looks like all exynos5250/5420 board are used CD# line. The commit message should be clearer, agreed. I think I asked Yuvaraj to make sure that the code only invoked this quirk on exynos and only if a GPIO was not used
Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc
Ulf, On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson ulf.hans...@linaro.org wrote: Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be called to check the card detect line, but with vmmc and vqmmc off. It will be unable to return a sensible value without actually turning on vmmc and vqmmc. Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule itself with HZ interval. This to check for card insert/removal. Now, mmc_rescan() will, if present, invoke host's -get_cd() callback to check whether it's worth to continue initialization sequence or if it should re-schedule itself again. If your host driver can distinguish whether a card is inserted, which in this case are when vccq is turned off, your -get_cd() callback need to return 1. That will allow mmc_rescan() to continue it's initialization sequence and do mmc_power_up(). As per my other email, we can't tell whether a card is inserted when vqmmc is off. Does this mean that you have removed your objections to a solution like Yuvaraj has posted? We still need to break it into two patches (the core part and the dwmmc part), but otherwise is it OK? I can post the original patch I sent Yuvaraj if it's helpful (or he can just include my patch as part 1 of his series). -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc
Ulf, On Thu, Aug 28, 2014 at 12:25 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 27 August 2014 17:52, Doug Anderson diand...@google.com wrote: Ulf, On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson ulf.hans...@linaro.org wrote: Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be called to check the card detect line, but with vmmc and vqmmc off. It will be unable to return a sensible value without actually turning on vmmc and vqmmc. Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule itself with HZ interval. This to check for card insert/removal. Now, mmc_rescan() will, if present, invoke host's -get_cd() callback to check whether it's worth to continue initialization sequence or if it should re-schedule itself again. If your host driver can distinguish whether a card is inserted, which in this case are when vccq is turned off, your -get_cd() callback need to return 1. That will allow mmc_rescan() to continue it's initialization sequence and do mmc_power_up(). As per my other email, we can't tell whether a card is inserted when vqmmc is off. I understand. The solution I proposed above, is: 1) Enable MMC_CAP_NEEDS_POLL. 2) Make -get_cd() return 1, when you can't distinguish if the card is inserted. That will solve this issue and without messing up the mmc core. Ah, I see! So every 1 second, we'll do the following: 1. Call get_cd(), which returns 1. 2. MMC core will power everything up (turn on all regulators) for MMC. 3. We'll start to initialize a card. 4. We'll notice that, oops, there's no card there. 5. MMC core will shut things down again. That seems awfully expensive to do every second when the card detect line actually does work (as long as you keep power lines). Is the patch to separate out the concepts of power off because no card is inserted and power off because we're power cycling the card really bad enough to warrant forcing us to use the above? I'm not an EE, but cycling regulators on and off every second doesn't seem super ideal, but maybe they're designed for it? Personally, I'd be tempted to just leave the power on all the time and if a card somehow needs to be power cycled (because UHS negotiation failed?) then that card just isn't supported. This could be done in the dts by saying that the regulator is always on and no need to pollute any source files. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc
Jaehoon, On Thu, Aug 28, 2014 at 1:43 AM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/28/2014 12:49 AM, Doug Anderson wrote: Jaehoon, On Tue, Aug 26, 2014 at 9:47 PM, Jaehoon Chung jh80.ch...@samsung.com wrote: Doug, On 08/27/2014 01:14 PM, Doug Anderson wrote: Jaehoon, On Tue, Aug 26, 2014 at 8:48 PM, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi, Doug, On 08/26/2014 12:25 AM, Doug Anderson wrote: Jaehoon, On Mon, Aug 25, 2014 at 1:50 AM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/25/2014 05:13 PM, Ulf Hansson wrote: On 22 August 2014 20:27, Sonny Rao sonny...@chromium.org wrote: On Fri, Aug 22, 2014 at 8:31 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: Exynos 5250 and 5420 based boards uses built-in CD# line for card detection.But unfortunately CD# line is on the same voltage rails as of I/O voltage rails. When we cut off vqmmc,the consequent card detection will break in these boards. I didn't know that use CD# line for card detect. And if CD# voltage rails and I/O voltage rail are same voltage, it doesn't make sense. Which card is used with same voltages? (eMMC? SD? SDIO?) Well, I have checked Exynos5250 and 5420, but it looks like not same rails. I'm not sure I totally understood what you said. In my manual I have a table titled Table 2-1 Exynos 5420 Pin List. Look in this table for XMMC2CDN and XMMC2DATA_0. Look to the right of the table and you'll see the power domain. For both it shows VDDQ_MMC2. If that doesn't mean that the two are in the same voltage domain then I don't know what does. Can you point to any examples where they have different voltage domains? I think you're mis-understanding for it. Right, It's described at exynos5420, but it's not connected. It's not connected. What do you mean? If I were to guess I'd say that on some particular board you're looking at they don't happen to use the CD pin for card detect. If this is what you mean, it doesn't help me. exynos5420-peach-pit does use the CD pin for card detect. You can look at the DTS file and confirm it. I didn't know how exynos5420-peach-pit's circuit is configured. But i guess that almost all exynos5 boards are configured with the similar circuit. At Almost all Exynos5 board, CD-pin is used, but not included in Same power domain. (CD-pin is external card-detect pin. - like XEINT_# pin) You mentioned CD# and DATA# lines is used the same power domain, right? In Circuit (not exynos5420-peach-pit), DATA# line and CMD/CLK(vqmmc) is same power supply, and vdd is used other power supply. Not use the CD# pin, used the XEINT_# pin. So i think we don't need to consider the CD#. If exynos5420-peach-pit board is used the CD#-pin, then our discussion can be changed. Maybe on your board you have CD connected to a gpx line. ...but not mine. The guys who designed our hardware followed the SMDK5420 reference schematics which connect the SD card slot card detect to gpc2_2, which is the card detect pin. See arch/arm/boot/dts/exynos5420-smdk5420.dts, specifically noting the lack of a GPIO card detect and the inclusion of sd2_cd mmc@1222 { status = okay; card-detect-delay = 200; samsung,dw-mshc-ciu-div = 3; samsung,dw-mshc-sdr-timing = 2 3; samsung,dw-mshc-ddr-timing = 1 2; pinctrl-names = default; pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; bus-width = 4; cap-sd-highspeed; }; See arch/arm/boot/dts/exynos5420-peach-pit.dts too: mmc_2 { status = okay; num-slots = 1; cap-sd-highspeed; card-detect-delay = 200; clock-frequency = 4; samsung,dw-mshc-ciu-div = 3; samsung,dw-mshc-sdr-timing = 2 3; samsung,dw-mshc-ddr-timing = 1 2; pinctrl-names = default; pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; bus-width = 4; }; Here, see this ASCII art that shows how some lines are hooked up on peach-pit. You might need to paste this into something with a fixed-width font. + |Exynos 5420 | | P2.8V_LOUT4 -|- VDDQ_MMC2 (AK7) || || +-+- 10K res -+|- XMMC2CDN (AK6) | || | || | || | Ext CD | | | +-- 10K res-+--+---|- XMMC2CMD (AK8) | | Ext CMD You can see from the above that the external card detect signal (that goes to the connector) named Ext CD is pulled up to the same voltage as the external CMD signal (that also goes to the connector). This is vqmmc. If we turn off vqmmc then the 10K resistor will (I think) act as a pull down, or in the best case it will be floating. Said another way: we can't read card detect when vqmmc is off. If that's the case, it makes sense. But i wonder why
Re: [PATCH V2 3/3] mmc: dw_mmc: Dont cut off vqmmc and vmmc
Hi, On Thu, Aug 28, 2014 at 10:50 AM, Sonny Rao sonny...@chromium.org wrote: On Thu, Aug 28, 2014 at 8:50 AM, Doug Anderson diand...@google.com wrote: Ulf, On Thu, Aug 28, 2014 at 12:25 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 27 August 2014 17:52, Doug Anderson diand...@google.com wrote: Ulf, On Wed, Aug 27, 2014 at 4:17 AM, Ulf Hansson ulf.hans...@linaro.org wrote: Now, we've got MMC_CAP_NEEDS_POLL, so dw_mmc will periodically be called to check the card detect line, but with vmmc and vqmmc off. It will be unable to return a sensible value without actually turning on vmmc and vqmmc. Currently MMC_CAP_NEEDS_POLL mean the mmc rescan work will reschedule itself with HZ interval. This to check for card insert/removal. Now, mmc_rescan() will, if present, invoke host's -get_cd() callback to check whether it's worth to continue initialization sequence or if it should re-schedule itself again. If your host driver can distinguish whether a card is inserted, which in this case are when vccq is turned off, your -get_cd() callback need to return 1. That will allow mmc_rescan() to continue it's initialization sequence and do mmc_power_up(). As per my other email, we can't tell whether a card is inserted when vqmmc is off. I understand. The solution I proposed above, is: 1) Enable MMC_CAP_NEEDS_POLL. 2) Make -get_cd() return 1, when you can't distinguish if the card is inserted. That will solve this issue and without messing up the mmc core. Ah, I see! So every 1 second, we'll do the following: 1. Call get_cd(), which returns 1. 2. MMC core will power everything up (turn on all regulators) for MMC. 3. We'll start to initialize a card. 4. We'll notice that, oops, there's no card there. 5. MMC core will shut things down again. That seems awfully expensive to do every second when the card detect line actually does work (as long as you keep power lines). Is the patch to separate out the concepts of power off because no card is inserted and power off because we're power cycling the card really bad enough to warrant forcing us to use the above? I'm not an EE, but cycling regulators on and off every second doesn't seem super ideal, but maybe they're designed for it? Personally, I'd be tempted to just leave the power on all the time and if a card somehow needs to be power cycled (because UHS negotiation failed?) then that card just isn't supported. This could be done in the dts by saying that the regulator is always on and no need to pollute any source files. Yeah, power cycling the regulators constantly doesn't seem like a great idea, we can ask the EEs what they think. This scheme of leaving them on all the time would prevent us from being able to actually power cycle the card, which I think is required by the spec in case UHS negotiation fails. OK, fair enough. I guess polling is less bad than nor supporting card power cycling. ...it still feels like adding this quirk to the core isn't super ugly, but if everyone is against it... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Will, On Fri, Sep 5, 2014 at 5:22 AM, Will Deacon will.dea...@arm.com wrote: [Looks like it's not just Rutland that can't spell the address of the mailing list today. Fixed here, so please use this post in any replies]. On Fri, Sep 05, 2014 at 12:57:04PM +0100, Will Deacon wrote: Hi all, I'm one of the few, foolish people to try running mainline on my 5250-based Samsung Chromebook (snow). I can live without wireless, usb3 and video acceleration, so actually it makes a reasonable development platform for doing A15-based (micro)-architectural work. However, since 3.15 I've not been able to boot *any* mainline kernels on this board. I did mean to report this earlier, but I have other machines that can run mainline so this has fallen by the wayside. The problems started with 3.16, where simple-fb would fail to initialise and I lost my display. Note that I don't have a serial console on this machine (I looked at the PCB and there's no way I can solder one of those myself :) I bisected the issue at the time, and I could get my display back by removing some of the new regulator and hdmi properties from the DT. At that point, I could boot, but DMA didn't initialise for the MMC controller so I couldn't mount my root filesystem. With 3.17-rc3, it seems a lot worse -- I don't get any output after nv-uboot (i.e. the nv-uboot screen just remains on the display, with the last line reading Stashed 20 records). I'd usually try to debug this a bit further, but without a console it's really painful to get anywhere. I've been working with 3.15, but now I'm having to backport patches when I want to test them, which is more effort than I can be bothered with. Is anybody else running mainline on this device and are these known/fixed problems? I've added Javier, who says he'll try to take a look at the problem on Monday. He's got a snow and I think he's got a serial console hooked up to it (but I don't think he's tried the simplefb workflow). He also added the following thoughts: Have you seen the very long [PATCH 4/4] simplefb: add clock handling code thread [0]?. I wonder if the problem is that the display clocks were not known to the kernel before 3.15 but now are getting disabled and thus the simplefb driver not working? So probably is worth to try passing clk_ignore_unused as a parameter to the kernel command line. [0]: https://www.mail-archive.com/linux-sunxi@googlegroups.com/msg06623.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Hi, On Sun, Sep 7, 2014 at 8:52 AM, Tomasz Figa tomasz.f...@gmail.com wrote: So I believe we've got a process issue here. If you don't have normal support for display hardware, but you want to keep the display operational thanks to bootloader already initializing it, you should not add anything to the kernel which breaks it, until full support comes in. This means that respective regulators should be either always-on or not listed at all (I'd favor the former) and respective clocks either somehow enabled at boot-up or completely ignored, including all their parents capable of being gated. It seems slightly broken to hack the device tree in this way. I'll be the first to admit that I often list regulators as always-on during bringup when not everything is done, and I guess it's not that different. ...but given everything going on upstream (and people working on Suspend/Resume, DRM, etc) it seems like it might be a bit of a pain. ...but if that's what everyone agrees on, I won't disagree too strongly. One (ugly?) solution would be to add a feature to your bootloader to modify the device tree to mark regulators as always-on. Since the booloader gets to touch the device tree and the bootloader is involved in communicating into about SimpleFB, it kinda makes sense. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Hi, On Sun, Sep 7, 2014 at 8:52 AM, Tomasz Figa tomasz.f...@gmail.com wrote: Now with regulators this is pretty straightforward, but with clocks I believe it's an open issue. AFAIR we've discussed this on MLs some time ago (at least I remember Doug commenting on that topic) and kind of concluded that SoC clock drivers could include lists of clocks to be enabled at boot-up (as a HACK to enable things like simplefb until proper support for respective features are added). I think my old problem was with earlyprintk and a core clock getting disabled. See (44ff025 clk: exynos5420: Remove aclk66_peric from the clock tree description). I think I've seen others solve the same problem with the concept of critical clocks. I agree that regulator and clock frameworks allow very different hacks. ;) -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Javier, On Sun, Sep 7, 2014 at 11:09 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Hello Doug, On 09/08/2014 06:36 AM, Doug Anderson wrote: One (ugly?) solution would be to add a feature to your bootloader to modify the device tree to mark regulators as always-on. Since the booloader gets to touch the device tree and the bootloader is involved in communicating into about SimpleFB, it kinda makes sense. I can't say I like to mark the regulators as always-on on the DT and that's why I copied the patch in the response instead of posting it as a proper patch but I think relying in the bootloaders to modify the DT is not better. IMHO U-boot should only modify the strictly necessary like the /chosen branch even though lately I've seen some attempts in the OMAP community to (ab)use U-Boot's fdt command to mangle the DT before passing to the kernel in order to support different Beagle Bone Black capes. So simple-framebuffer is added to the device tree here: https://chromium-review.googlesource.com/#/c/49358/2/board/samsung/smdk5250/smdk5250.c That's one of the two patches to build your own U-Boot for enabling simplefb. You'll notice that's not a super official thing. It's a DO NOT SUBMIT patch sitting in a gerrit code review server, so I wouldn't exactly call it a stable ABI that we can't break. It's not something shipping in real products and it's not even landed in a git tree (I suppose maybe someone somewhere landed it, but...). To me, that means that if someone is using that patch and it works for them, then that's great! If it stops working (possibly because it was making assumptions about the state of the kernel) then it should be fixed up. In this case, that patch really should be adding references to regulators (and possibly clocks) that are needed. Given that this patch is already reaching into the device tree to add the simple-framebuffer node, it doesn't seem unreasonable to say that it should be grabbing the proper references (or mark regulators as always-on). ...as always, though, remember that my opinion doesn't count for much. I also sympathize with the problems people are running into. :( -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Grant, On Mon, Sep 8, 2014 at 5:20 AM, Grant Likely grant.lik...@secretlab.ca wrote: On Mon, Sep 8, 2014 at 12:21 PM, Will Deacon will.dea...@arm.com wrote: On Sun, Sep 07, 2014 at 05:19:03PM +0100, Tomasz Figa wrote: At least for next 3.17-rc I'd suggest fixing this up in respective clock driver and dropping the hack only after Exynos DRM patches are merged and confirmed working. Whilst I'm sympathetic to people working to enable DRM, I think this is the right solution to the problem. The transition from simplefb to DRM shouldn't break display for a bunch of kernel revisions whilst the code is in flux. I would go further. The kernel behaviour has changed, and we have to deal with platforms that assume the old behaviour. That means either defaulting to leaving enabled regulators/clocks alone unless there is a flag in the DT saying they can be power managed, or black listing platforms that are known to depend on the regulator being on. Updating the device tree must not be required to get the kernel to boot, but it is valid to require a DT upgrade to get better performance (battery life) out of the platform. In this case people using SImple FB are not really using an officially sanctioned device tree. The simple-fb fragment is created on the fly via a DO NOT SUBMIT patch sitting on a code review server. It's not something that's shipped with real firmware nor is it something present in the kernel. See https://chromium-review.googlesource.com/#/c/49358/2/board/samsung/smdk5250/smdk5250.c as I mentioned above. Is this really a device tree that we need to guarantee backward compatibility with? -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Will, On Mon, Sep 8, 2014 at 9:07 AM, Will Deacon will.dea...@arm.com wrote: On Mon, Sep 08, 2014 at 04:55:31PM +0100, Doug Anderson wrote: So simple-framebuffer is added to the device tree here: https://chromium-review.googlesource.com/#/c/49358/2/board/samsung/smdk5250/smdk5250.c That's one of the two patches to build your own U-Boot for enabling simplefb. You'll notice that's not a super official thing. It's a DO NOT SUBMIT patch sitting in a gerrit code review server, so I wouldn't exactly call it a stable ABI that we can't break. It's not something shipping in real products and it's not even landed in a git tree (I suppose maybe someone somewhere landed it, but...). I just took the uboot image linked to from the chromium.org page here: http://www.chromium.org/chromium-os/u-boot-porting-guide/using-nv-u-boot-on-the-samsung-arm-chromebook#TOC-Getting-nv-U-Boot Ah, OK. It's still using the DO NOT SUBMIT patchs, but I guess given that there is a binary built up there by a fairly official source... Hrmm. I think Olof is the one that built that. Perhaps he'd be willing to muck with that and see if he can grab the regulators? -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] ARM: dts: Add Peach Pit and Pi dts entry for max77802 PMIC
Javier, On Wed, Aug 20, 2014 at 4:19 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Exynos5420 based Peach Pit and Exynos5800 based Peach Pi boards uses a Maxim 77802 power management IC to drive regulators and its Real Time Clock. This patch adds support for this chip. These are the device nodes and pinctrl configuration that are present on the Peach pit DeviceTree source file in the the Chrome OS kernel 3.8 tree. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Tested-by: Naveen Krishna Chatradhi ch.nav...@samsung.com --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 372 + arch/arm/boot/dts/exynos5800-peach-pi.dts | 372 + 2 files changed, 744 insertions(+) diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts index ab06148..78cea4c 100644 --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts @@ -11,6 +11,7 @@ /dts-v1/; #include dt-bindings/input/input.h #include dt-bindings/gpio/gpio.h +#include dt-bindings/interrupt-controller/irq.h #include exynos5420.dtsi / { @@ -144,6 +145,339 @@ ddc = i2c_2; }; +hsi2c_4 { + status = okay; + clock-frequency = 40; + + max77802-pmic@9 { + compatible = maxim,max77802; + interrupt-parent = gpx3; + interrupts = 1 IRQ_TYPE_NONE; + pinctrl-names = default; + pinctrl-0 = max77802_irq, pmic_selb, + pmic_dvs_1, pmic_dvs_2, pmic_dvs_3; + wakeup-source; + reg = 0x9; + #clock-cells = 1; ...you've actually got the 32k clock specified in a reasonable way now (unlike our tree). We don't have opmode or the DVFS stuff upstream yet, so makes sense that's not here. + inb1-supply = tps65090_dcdc2; + inb2-supply = tps65090_dcdc1; + inb3-supply = tps65090_dcdc2; + inb4-supply = tps65090_dcdc2; + inb5-supply = tps65090_dcdc1; + inb6-supply = tps65090_dcdc2; + inb7-supply = tps65090_dcdc1; + inb8-supply = tps65090_dcdc1; + inb9-supply = tps65090_dcdc1; + inb10-supply = tps65090_dcdc1; + + inl1-supply = buck5_reg; + inl2-supply = buck7_reg; + inl3-supply = buck9_reg; + inl4-supply = buck9_reg; + inl5-supply = buck9_reg; + inl6-supply = tps65090_dcdc2; + inl7-supply = buck9_reg; + inl9-supply = tps65090_dcdc2; + inl10-supply = buck7_reg; I double-checked your input supplies against schematics (since we don't have them right in our tree). They look right. + + regulators { + buck1_reg: BUCK1 { + regulator-name = vdd_mif; + regulator-min-microvolt = 80; + regulator-max-microvolt = 130; + regulator-always-on; + regulator-boot-on; + regulator-ramp-delay = 12500; + }; + + buck2_reg: BUCK2 { + regulator-name = vdd_arm; + regulator-min-microvolt = 80; + regulator-max-microvolt = 150; + regulator-always-on; + regulator-boot-on; + regulator-ramp-delay = 12500; + }; + + buck3_reg: BUCK3 { + regulator-name = vdd_int; + regulator-min-microvolt = 80; + regulator-max-microvolt = 140; + regulator-always-on; + regulator-boot-on; + regulator-ramp-delay = 12500; + }; + + buck4_reg: BUCK4 { + regulator-name = vdd_g3d; + regulator-min-microvolt = 70; + regulator-max-microvolt = 140; + regulator-always-on; + regulator-boot-on; + regulator-ramp-delay = 12500; + }; + + buck5_reg: BUCK5 { + regulator-name = vdd_1v2; + regulator-min-microvolt = 120; + regulator-max-microvolt = 120; + regulator-always-on;
Re: [PATCH 2/6] ARM: dts: add hdmi regulators for exynos5800 based peach-pi board
Javier / Rahul, On Tue, Aug 19, 2014 at 8:08 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: From: Rahul Sharma rahul.sha...@samsung.com Adding regulators for HDMI for Peach-pi board. Signed-off-by: Rahul Sharma rahul.sha...@samsung.com Reviewed-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- v2: 1) Add blank line before hdmi regulators. arch/arm/boot/dts/exynos5800-peach-pi.dts | 5 + 1 file changed, 5 insertions(+) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] ARM: dts: add hdmi regulators for exynos5800 based peach-pi board
Hi, On Tue, Sep 9, 2014 at 9:44 PM, Doug Anderson diand...@chromium.org wrote: Javier / Rahul, On Tue, Aug 19, 2014 at 8:08 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: From: Rahul Sharma rahul.sha...@samsung.com Adding regulators for HDMI for Peach-pi board. Signed-off-by: Rahul Sharma rahul.sha...@samsung.com Reviewed-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- v2: 1) Add blank line before hdmi regulators. arch/arm/boot/dts/exynos5800-peach-pi.dts | 5 + 1 file changed, 5 insertions(+) Reviewed-by: Doug Anderson diand...@chromium.org Meant to add this to the v2. Sorry for the spam... -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/5] ARM: dts: add hdmi regulators for exynos5800 based peach-pi board
Javier / Rahul, On Wed, Aug 20, 2014 at 4:19 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: From: Rahul Sharma rahul.sha...@samsung.com Adding regulators for HDMI for Peach-pi board. Signed-off-by: Rahul Sharma rahul.sha...@samsung.com Reviewed-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5800-peach-pi.dts | 5 + 1 file changed, 5 insertions(+) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/5] ARM: dts: add hdmi regulators for exynos5420 based peach-pit board
Javier / Rahul. On Wed, Aug 20, 2014 at 4:19 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: From: Rahul Sharma rahul.sha...@samsung.com Adding regulators for hdmi for peach-pit board. Signed-off-by: Rahul Sharma rahul.sha...@samsung.com Reviewed-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 5 + 1 file changed, 5 insertions(+) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/5] ARM: dts: Add thermistor dts fragment used by exynos based Peach boards
Javier / Naveen, On Wed, Aug 20, 2014 at 4:19 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: From: Naveen Krishna Chatradhi ch.nav...@samsung.com This patch creates a thermistor fragment carrying the NTC Thermistor nodes as children of the IIO based ADC. This fragment is included in exynos5420-peach-pit.dts and exynos5800-peach-pi.dts. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/cros-adc-thermistors.dtsi | 44 + arch/arm/boot/dts/exynos5420-peach-pit.dts | 6 arch/arm/boot/dts/exynos5800-peach-pi.dts | 6 3 files changed, 56 insertions(+) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/5] ARM: dts: Add Peach Pit and Pi dts entry for ISL29018 sensor
Javier, On Wed, Aug 20, 2014 at 4:19 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: The Exynos5420 based Peach Pit and the Exynos5800 based Peach Pi machines have an i2c ISL29018 light sensor. This patch adds the device nodes needed to support this device. These DTS snippets were taken from the downstream Chrome OS 3.8 kernel Device Tree for Peach Pit and Pi boards. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 6 ++ arch/arm/boot/dts/exynos5800-peach-pi.dts | 6 ++ 2 files changed, 12 insertions(+) I would note that the downstream dts file has this i2c bus at 400kHz. ...but that's not a problem with your patch. Perhaps you could submit that as a separate patch? Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/5] Add max77802 support for Peach boards
Hi, On Mon, Sep 8, 2014 at 8:47 PM, kg...@kernel.org wrote: Javier Martinez Canillas wrote: Hello Kukjin, Hi, On Wed, Aug 20, 2014 at 1:19 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: This is a second version of the series that adds max77802 support for the Peach Pit and Pi boards. The series also have all the pending patches that were posted but depended on this support. I've picked all the patches I found and rebased them to be sure that they apply cleanly on top of linux-next. Also I've taken some DT snippets from the downstream Chrome OS 3.8 kernel DTS for devices that use one of these regulators as their input supply. Any comments on this series? Looks good to me but I just wanted to get ack from chrome guy, Doug? But since Naveen tested, it should be fine I think. I'll take the series. I haven't had crazy amounts of time to review pit/pi patches recently, but luckily Javier has been picking up my slack. I think he's more of a maintainer for these file than I am at this point. ...but I did do a quick skim / compare to our tree and found one small bug (see patch #1) and added my Reviewed-by tags. -Doug -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Will, On Wed, Sep 10, 2014 at 4:17 AM, Will Deacon will.dea...@arm.com wrote: On Mon, Sep 08, 2014 at 03:05:40PM +0100, Javier Martinez Canillas wrote: Hello Will, Hi Javier, Since many folks don't agree that hacking different subsystems is the way forward I'll hold the patches and don't post them. The sunxi thread [0] already shows how different people have strong opposite positions on the correct approach to handle this. For now you can just disable the tps65090 PMIC support by not enabling the CONFIG_REGULATOR_TPS65090 kconfig symbol on your kernel config. That will give you exactly the same behavior that before tps65090 support was added to the Snow DT on commit b16be76 (ARM: dts: add tps65090 power regulator for exynos5250-snow) which AFAIU was good enough for your workflow. Disabling the regulator support gives me my framebuffer back, thanks. Yay! Is this good enough to tide you over until DRM support lands? I then get stuck mounting my root filesystem over sd card because it's reported as read-only. The following hack solved the issue, but I'm not exactly sure where this regression has come from (whether this is a board quirk or pinctrl is really misconfigured). This is a regression that was introduced by: 9795a84 mmc: dw_mmc: remove dw_mci_of_cd_gpio/wp_gpio() In exynos5250-cros-common.dtsi you can see that the wp-gpios = gpc2 1 0; is still listed under the slot node. Nobody is looking there anymore. This is fixed in linux-next by: aaa25a5 ARM: dts: unuse the slot-node and deprecate the supports-highspeed for dw-mmc in exynos I'll let Kukjin decide what he wants to do about the regression in v3.17 (how to deal with conflicts, backporting, etc). -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Grant, On Wed, Sep 10, 2014 at 9:29 AM, Grant Likely grant.lik...@secretlab.ca wrote: On Wed, Sep 10, 2014 at 4:39 PM, Mark Brown broo...@kernel.org wrote: On Wed, Sep 10, 2014 at 03:56:16PM +0100, Grant Likely wrote: On Wed, Sep 10, 2014 at 3:31 PM, Mark Brown broo...@kernel.org wrote: As far as I can tell the problem here is coming from the decision to have simplefb use resources without knowing about them - can we agree that this is a bad idea? No, I don't think we can... there is a certain amount of firmware got things working for us, and we're going to use it for a while that is absolutely reasonable. simplefb is a good example, but there are certainly others. That bit is fine - I definitely think it's reasonable to have things like this where the device is initialized prior to the kernel starting and we use some simplified subset. What I think is a big problem here is that we're not being told what parts of the system state are relevant to this initialization (worse, we're being told things that are actively wrong for some of the resources). This seems inherently fragile. I /do/ think it would be better for the simplefb data to get embedded or linked into the node of the graphics controller so that it can be torn down appropriately, and we need a rule for how long boot-state can be considered valid so that a proper driver can either reserve the resources for a given SoC, or do a full handoff from the simplefb. Even without that though, we need to be able to handle the case of an anonymous simplefb node with no regulator information. If that means the default simplefb behaviour is to inhibit runtime pm on all resources until a real driver show up, then that might just be what we need to do. I think saying that it's a good idea to have an simplefb node without resource management is exactly the problem here - if we start from the assumption that this is a good idea we do get dragged down this path but it seems like we took a wrong turn going that way in the first place. It's not just regulators - we've got exactly the same problem with clocks on this system for example, they're also getting disabled because they seem unused and users have to pass in a kernel command line bodge to avoid that. We'd also have an issue if something decided to change the rates of some of the clocks, and power domains have the same problem (Ulf's patches to genericise their code has the same behaviour with regard to powering off unused domains, some of the existing implementations do that already). Two things should probably be changed from the current setup. 1) simplefb shouldn't be a platform driver. It is a boot thing that handles initial state from the graphics chip. By implementing it as a platform driver, it prevents the real driver from binding to the real device if the simplefb data embedded into it. 2) make sure that an SoC driver can protect the needed resources before they are automatically disabled. Either by putting them in an earlier initcall, or handling it in the subsystem code. I don't know enough about the regulator and clock runtime PM to know what the best way to do this is. Right, I agree with what you're saying here but what I'm saying is that the way to ensure that the resources are protected is for the simplefb node to tell the kernel what resources are being used, otherwise it seems like we're just guessing and will fall over ourselves sooner or later. We can't use initcall hacks as these only work in cases where we will at some point hand over to a real driver and there seems to be a clear use case for using simplefb prior to that driver being written; even where we will hand over to a real driver we can't put a definite timescale on that happening since in the distro case it might be being loaded from disk at some point after userspace is running. What we can do is have an inhibit flag for simplefb/simpleuart/simplewhatever that holds off PM. When a real driver, or a stub that understands parsing the resource dependencies, takes ownership of the device (or userspace tells the kernel to stop caring) it can clear the inhibit. This doesn't seem crazy, though it means that if you're planning on using nothing but simplefb then you're never going to be able to get any power savings anywhere. Right now I know that clock disabling is supposed to be inhibited during the early boot process. I think regulators too? I don't want to build knowledge of resource dependencies into the simple case. We'll simply frequently get it wrong. For example: A future kernel will have better PM and will turn off more devices which isn't accounted for in an older DT. In IRC I made a suggestion that perhaps the simplefb ought to be put in the main in-kernel dts file but with no address information and set to disabled. Then the firmware can do something very simple: set to enabled and fill in the address. Right now the firmware is taking
Re: Unable to boot mainline on snow chromebook since 3.15
Mark, On Wed, Sep 10, 2014 at 12:45 PM, Mark Brown broo...@kernel.org wrote: On Wed, Sep 10, 2014 at 09:45:21AM -0700, Doug Anderson wrote: Right now I know that clock disabling is supposed to be inhibited during the early boot process. I think regulators too? No, for regulators we'll quite happily disable anything a consumer asks us to at any point but we'll only do a sweep for regulators that were enabled on startup then not subsequently referenced and turn them off in late_initcall. Ah, that sounds exactly like the clock framework then. I think I just didn't explain the clock framework properly. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] ARM: dts: Set i2c7 clock at 400kHz for Peach boards
Javier, On Wed, Sep 10, 2014 at 3:19 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: The downstream ChromeOS 3.8 kernel sets the clock frequency for the I2C bus 7 at 400kHz. Do the same change in mainline. Suggested-by: Doug Anderson diand...@chromium.org Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 1 + arch/arm/boot/dts/exynos5800-peach-pi.dts | 1 + 2 files changed, 2 insertions(+) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 7/7] ARM: exynos_defconfig: Enable MAX77802
Javier, On Wed, Sep 10, 2014 at 3:19 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: From: Vivek Gautam gautam.vi...@samsung.com Enabled MAX77802 pmic for exynos systems. One config USB_ANNOUNCE_NEW_DEVICES to display device information on connect. Another config for I2C_CHARDEV to see i2c device nodes. Signed-off-by: Vivek Gautam gautam.vi...@samsung.com Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/configs/exynos_defconfig | 3 +++ 1 file changed, 3 insertions(+) I've no complaints about any of that. Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Unable to boot mainline on snow chromebook since 3.15
Hi, On Thu, Sep 11, 2014 at 11:03 AM, Mark Brown broo...@kernel.org wrote: On Thu, Sep 11, 2014 at 10:22:32AM +0100, Grant Likely wrote: On Wed, 10 Sep 2014 17:57:23 +0100, Mark Brown broo...@kernel.org wrote: It's not quite as simple as just disabling PM - for example in the clocks case we've also got to worry about what happens with rate changes (which is going to get more and more risky as we get smarter about being able to push configuration changes back up the tree), regulators have a similar thing with voltage changes. With simple enables and disables we have to worry about things like handling users who actively want to power things on and and off but may potentially be sharing a resource with an undeclared dependency. I think we can be okay with the above. This is a best-effort situation where we don't want to tear down how firmware has set up the board if it can be reasonably assumed that something depends on it (simplefb). However, if clocks or regulators are shared with other devices and those drivers ask for other settings, then there is simply no recourse. In that situation there must be a driver for the video device that takes care of any constraints. When things break I'm not sure that users are going to understand that something that used to work for them was only provided on a best effort basis, I think they will expect things to carry on working. Right. This is exactly what happened at the start of this thread. SimpleFB was working only on a best effort basis and then it stopped working. I agree that's pretty non-ideal. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND v9 1/5] rtc: max77686: Allow the max77686 rtc to wakeup the system
Hi, On Fri, Sep 12, 2014 at 1:17 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: From: Doug Anderson diand...@chromium.org The max77686 includes an RTC that keeps power during suspend. It's convenient to be able to use it as a wakeup source. NOTE: due to wakeup ordering problems this patch alone doesn't work so well on exynos5250-snow. You also need something that brings the i2c bus up before the max77686 wakeup runs. This note made sense when the patch was first submitted, but no longer does since the i2c change landed. See: b19c195 i2c: s3c2410: resume the I2C controller earlier -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND v9 1/5] rtc: max77686: Allow the max77686 rtc to wakeup the system
Hi, On Fri, Sep 12, 2014 at 8:54 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: You are right, I completely forgot to check if that actually landed and to remove the note in that case... Maybe when the set is applied the note can be removed from this patch or do you think that I should re-spin the series for that? This seems like something a maintainer could handle removing, but I guess it's totally up to the maintainer. If I were the maintainer I'd rather remove the note myself then see a whole spin of a 5-part series just to fix this. ;) -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND] ARM: DTS: correct the capability string for mmc0
Vivek, On Tue, Sep 16, 2014 at 1:50 AM, Vivek Gautam gautam.vi...@samsung.com wrote: From: Naveen Krishna Chatradhi ch.nav...@samsung.com MMC capability for HS200 is parsed in mmc/core/host.c as dts string mmc-hs200-1_8v. This patch corrects the dts string for Exynos5420 based peach-pit and Exynos5800 based peach-pi boards. Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com --- Hi Kukjin, As commented by Doug earlier for this patch, the two patches required -- mmc: dw_mmc: Make sure we don't get stuck when we get an error -- mmc: dw_mmc: change to use recommended reset procedure are now merged in upstream. So you can go ahead and pick this change. arch/arm/boot/dts/exynos5420-peach-pit.dts |2 +- arch/arm/boot/dts/exynos5800-peach-pi.dts |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
Hi, On Tue, Sep 16, 2014 at 8:20 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: Hello Daniel, On Tue, Sep 16, 2014 at 5:03 PM, Daniel Drake dr...@endlessm.com wrote: On Tue, Sep 16, 2014 at 7:48 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: Clock list for s3c-rtc device: - rtc : CLK_RTC of CLK_GATE_IP_PERIR is gate clock for RTC. - rtc_src : XrtcXTI is 32.768.kHz source clock for RTC. Is this RTC source clock needed for all Exynos SoCs? It is at least needed on Exynos4412, which has the XrtcXTI thing exactly as you describe. However the very standard setup there is to hook it up to the CP clock output of the MAX76686 PMIC. This CP clock is on by default, so you can potentially live without that detail being present in the DT. Thanks for confirming for Exynos4412, I just answered my own email saying that I found to be needed on Exynos5420 as well and as you said, it was just working because the Maxim clocks were left on by default. However... one small issue with this setup is that when you enable CONFIG_COMMON_CLK_MAX77686, the CP clock gets exposed in Linux's common clock framework, and Linux then turns it off because it believes it is unused. Then the RTC stops ticking. Indeed, this is an issue about relying on default state. We had a quite long discussion a couple of weeks ago about simplefb relying on clocks and regulators left enabled by the bootloader but once these were know to the kernel, the frameworks disable them because were unused making simplefb to fail. So the rtc_src idea would also be good for Exynos4412. Maybe it would make sense to drop the needs_src_clk flag, and simply require/enable the src clock whenever it is present in the DT. That sounds more sensible to me as well. I wonder what should happen in this case with DT backward compatibility though. But as you said, the external clock is required and the kernel will disable this clock once is know to the CCF since is not used so maybe will be hard to maintain DT backward compatibility in this case. I think you can turn off CONFIG_COMMON_CLK_MAX77686 and then this clock will be left at whatever the bootloader set it to, right? Then there will be no auto-disabling by the CCF and the RTC will work. That's one argument for making the clock optional. NOTE: I don't think that the builtin RTC is terribly important for any exynos-based Chromebooks that I'm aware of. We rely on the RTC that's part of the Maxim PMIC itself and pretty much ignore the one built-in to the exynos. I think there are some cases it was used (as a fallback wakeup source in certain test scripts), but nothing very important. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: dts: Add rtc_src clk for s3c-rtc on exynos Peach boards
Javier, On Wed, Sep 17, 2014 at 4:50 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: commit 546b117fdf17 (rtc: s3c: add support for RTC of Exynos3250 SoC) added an rtc_src DT property for the Samsung's S3C Real Time Clock controller that specifies the 32.768 kHz clock that uses the RTC as its source clock. In the case of the Peach Pit and Pi machines, the Maxim 77802 32kHz AP clock is used as the source clock. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5420-peach-pit.dts | 5 - arch/arm/boot/dts/exynos5800-peach-pi.dts | 5 - 2 files changed, 8 insertions(+), 2 deletions(-) Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: dts: Add rtc_src clk for s3c-rtc on exynos5250-snow
Hi, On Wed, Sep 17, 2014 at 4:50 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: commit 546b117fdf17 (rtc: s3c: add support for RTC of Exynos3250 SoC) added an rtc_src DT property for the Samsung's S3C Real Time Clock controller that specifies the 32.768 kHz clock that uses the RTC as its source clock. In the case of the Exynos5250 based Snow board, the Maxim 77686 32kHz AP clock is used as the source clock. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/boot/dts/exynos5250-snow.dts | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/exynos5250-snow.dts b/arch/arm/boot/dts/exynos5250-snow.dts index e51fcef..56eec56 100644 --- a/arch/arm/boot/dts/exynos5250-snow.dts +++ b/arch/arm/boot/dts/exynos5250-snow.dts @@ -10,6 +10,7 @@ /dts-v1/; #include dt-bindings/gpio/gpio.h +#include dt-bindings/clock/maxim,max77686.h #include exynos5250.dtsi / { @@ -29,6 +30,8 @@ rtc@101E { status = okay; + clocks = clock CLK_RTC, max77686 MAX77686_CLK_AP; + clock-names = rtc, rtc_src; Wait, seriously? Snow is still using the rtc@101E syntax? Whatever happened to the series that Andreas worked so hard on, including https://patchwork.kernel.org/patch/4664801/? Kukjin: Andreas's patch series was Reviewed long ago I think and by now I'd imagine it's got some conflicts due to it not having been applied in a timely fashion. Perhaps you could fix it up for Andreas (since he's already rebased it several times) and land it? If I had to guess, outstanding from Andreas's series (patchwork IDs listed): 4751131 New [v7] ARM: dts: Prepare node labels for exynos5250 4664801 New [v6,04/10] ARM: dts: Clean up exynos5250-snow 4664771 New [v6,05/10] ARM: dts: Fill in bootargs for exynos5250-snow 4664731 New [v6,06/10] ARM: dts: Clean up exynos5250-smdk5250 4664751 New [v6,07/10] ARM: dts: Clean up exynos5250-arndale 4664711 New [v6,08/10] ARM: dts: Fix apparent GPIO typo in exynos5250-arndale 4664681 New [v6,09/10] ARM: dts: Simplify USB3503 on exynos5250-arndale 4664691 New [v6,10/10] ARM: dts: Add exynos5250-spring device tree }; pinctrl@1140 { @@ -350,7 +353,7 @@ samsung,i2c-sda-delay = 100; samsung,i2c-max-bus-freq = 378000; - max77686@09 { + max77686: max77686@09 { compatible = maxim,max77686; interrupt-parent = gpx3; interrupts = 2 0; In any case, there's nothing wrong with Javier's patch other than the fact that it will eventually need to get merged with Andreas's, so: Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 4/5] rtc: s3c: Add support for RTC of Exynos3250 SoC
Daniel, On Wed, Sep 17, 2014 at 9:49 AM, Daniel Drake dr...@endlessm.com wrote: On Tue, Sep 16, 2014 at 4:15 PM, Doug Anderson diand...@chromium.org wrote: NOTE: I don't think that the builtin RTC is terribly important for any exynos-based Chromebooks that I'm aware of. We rely on the RTC that's part of the Maxim PMIC itself and pretty much ignore the one built-in to the exynos. I think there are some cases it was used (as a fallback wakeup source in certain test scripts), but nothing very important. That's not true for all hardware though, at least the board I'm working on now has the SoC RTC as battery-backed and the PMIC one with no battery. So in this case at least, the interesting RTC is the SoC one. Yup, I can totally believe that. My statement was meant only to apply to the boards I knew about firsthand... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 0/6] Add Maxim 77802 RTC support
Javier, On Fri, Sep 19, 2014 at 3:26 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Hello Andrew, This series add support for the Real Time clock present in the Maxim 77802 Power Managment IC. The version number is quite high because it previously was part of a bigger series [0] that aimed to add support for all the devices in the max77802 PMIC. But now that the max77802 dependencies were already merged for 3.17, the series were split but I kept the version numbering. While working on the max77802 rtc support a lot of feedback was given and the issues pointed out also apply to a driver for a similar PMIC RTC (max77686). So patches 01/06 to 05/06 in the series are cleanups for the max77686 driver and patch 06/06 adds the support for the max77802 RTC. This version address the issues pointed out on the previous version [1] and changelog are present on each patch when is applicable. The series were tested on an Exynos5250 Snow (max77686) and Exynos5420 Peach Pit (max77802) machines and applies cleanly to both 3.17-rc1 and today's linux-next (20140919). NOTE: The patches from the previous version [1] were already present in the -mm tree [2] so I didn't know if I should had sent this as a delta or as a new revision. I decided to do the later but please let me know if you expected the former. My records show that Andrew has already accepted most of these. They may not show up in linuxnext yet, but that doesn't mean Andrew hasn't taken them (I think). Doug Anderson (1): rtc: max77686: Allow the max77686 rtc to wakeup the system http://ozlabs.org/~akpm/mmots/broken-out/rtc-max77686-allow-the-max77686-rtc-to-wakeup-the-system.patch Javier Martinez Canillas (5): rtc: max77686: Remove dead code for SMPL and WTSR. http://ozlabs.org/~akpm/mmots/broken-out/rtc-max77686-remove-dead-code-for-smpl-and-wtsr.patch rtc: max77686: Fail to probe if no RTC regmap irqchip is set http://ozlabs.org/~akpm/mmots/broken-out/rtc-max77686-fail-to-probe-if-no-rtc-regmap-irqchip-is-set.patch rtc: max77686: Remove unneded info log http://ozlabs.org/~akpm/mmots/broken-out/rtc-max77686-remove-unneded-info-log.patch rtc: max77686: Use ffs() to calculate tm_wday This one hasn't been accepted... rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock http://ozlabs.org/~akpm/mmots/broken-out/rtc-add-driver-for-maxim-77802-pmic-real-time-clock.patch -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7] mfd: syscon: Decouple syscon interface from platform devices
Pankaj, On Tue, Sep 30, 2014 at 1:35 AM, Pankaj Dubey pankaj.du...@samsung.com wrote: Currently a syscon entity can be only registered directly through a platform device that binds to a dedicated syscon driver. However in certain use cases it is desirable to make a device used with another driver a syscon interface provider. For example, certain SoCs (e.g. Exynos) contain system controller blocks which perform various functions such as power domain control, CPU power management, low power mode control, but in addition contain certain IP integration glue, such as various signal masks, coprocessor power control, etc. In such case, there is a need to have a dedicated driver for such system controller but also share registers with other drivers. The latter is where the syscon interface is helpful. In case of DT based platforms, this patch decouples syscon object from syscon platform driver, and allows to create syscon objects first time when it is required by calling of syscon_regmap_lookup_by APIs and keep a list of such syscon objects along with syscon provider device_nodes and regmap handles. For non-DT based platforms, this patch keeps syscon platform driver structure so that syscon can be probed and such non-DT based drivers can use syscon_regmap_lookup_by_pdev API and access regmap handles. Once all users of syscon_regmap_lookup_by_pdev migrated to DT based, we can completely remove platform driver of syscon, and keep only helper functions to get regmap handles. Suggested-by: Arnd Bergmann a...@arndb.de Suggested-by: Tomasz Figa tomasz.f...@gmail.com Tested-by: Vivek Gautam gautam.vi...@samsung.com Tested-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Signed-off-by: Pankaj Dubey pankaj.du...@samsung.com Reviewed-by: Arnd Bergmann a...@arndb.de Tested-by: Heiko Stuebner he...@sntech.de Reviewed-by: Heiko Stuebner he...@sntech.de --- Patch v6 and related discussions can be found here [1]. Change since v5: - Addressed review comments from Heiko Stuebner. - Updated commit description. - Including Arnd's and Heiko's Reviewed-by. Change since v5: - Dropping creation of dummy platform device in of_syscon_register. - As we are changing syscon to decouple from platform_device, creation of dummy platform_device does not look good option, and as suggested by Arnd, I made another attempt so that regmap_mmio_init API should work with NULL dev pointer itself. Since regmap needs to know about Syscon device node properties so let's parse device node of syscon in syscon itself for any such properties and using regmap_config parameter pass all such information to regmap. Other concern of crashes due to NULL dev pointer in regmap already addressed in separate patches of regmap. Please see [2] and [3]. Changes since v4: - Addressed Tomasz Figa's comments for v4. - Added error handing in of_syscon_register function. - Using devm_regmap_init_mmio instead of regmap_init_mmio. Changes since v3: - Addressed Arnd's comment for v2. - Updated of_syscon_register for adding dev pointer in regmap_init_mmio. - For early users created dummy platform device. Changes since v2: - Added back platform device support from syscon, with one change that syscon will not be probed for DT based platform. - Added back syscon_regmap_lookup_by_pdevname API so that non-DT base users of syscon will not be broken. - Removed unwanted change in syscon.h. - Modified Signed-off-by list, added Suggested-by of Tomasz Figa and Arnd Bergmann. - Added Tested-by of Vivek Gautam for testing on Exynos platform. Changes since v1: - Removed of_syscon_unregister function. - Modified of_syscon_register function and it will be used by syscon.c to create syscon objects whenever required. - Removed platform device support from syscon. - Removed syscon_regmap_lookup_by_pdevname API support. - As there are significant changes w.r.t patchset v1, I am taking over author for this patchset from Tomasz Figa. [1]: https://lkml.org/lkml/2014/9/29/99 [2]: https://lkml.org/lkml/2014/9/18/130 [3]: https://lkml.org/lkml/2014/9/27/2 drivers/mfd/syscon.c | 96 ++ 1 file changed, 74 insertions(+), 22 deletions(-) You probably already have enough tags, but just in case. ;) On an rk3288-based system (this patch backported to 3.14): Tested-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators
Bartlomiej On Mon, Sep 29, 2014 at 5:31 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Hi, On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: This patch makes use of mmc_regulator_get_supply() to handle the vmmc and vqmmc regulators.Also it moves the code handling the these regulators to dw_mci_set_ios().It turned on the vmmc and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off during MMC_POWER_OFF. Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com Thanks! Applied for next. Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC) detection on Exynos5420 Arndale Octa for me: [ 10.797979] dwmmc_exynos 1222.mmc: no support for card's volts [ 10.797998] mmc1: error -22 whilst initialising SD card Without the patch: [ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 5000Hz, actual 5000HZ div = 0) [ 10.866977] mmc1: new high speed SDHC card at address 1234 [ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB [ 10.915054] mmcblk1: p1 p2 p3 The config is attached (exynos_defconfig doesn't work correctly for this board yet). Yup, this is an expected behavior, unfortunately. This was talked about extensively during the review of this patch series. I believe that patch #3 in Yuvaraj's series would fix your problem. Specifically https://patchwork.kernel.org/patch/4763891/. The current summary of this issue is (Ulf, please correct me if I got anything wrong): 1. If nothing else, Yuvaraj's patch should probably be split in two. One half should be the MMC core half that I originally sent Yuvaraj. I just rebased and re-uploaed it at https://chromium-review.googlesource.com/220560 in case you're curious. The second half should be the dw_mmc piece that Yuvaraj wrote. 2. Ulf has indicated that he thinks that the MMC core change (and thus Yuvaraj's patch) is ugly and not necessary. He advocates instead using the MMC_CAP_NEEDS_POLL on all affected platforms. That means we'll turn on the power every second, check for the card, then turn off power. 3. I'm still of the opinion that the MMC core change isn't _that_ ugly. Given that there are a large number of systems affected (across at least two SoC vendors) and that it would be nice if those systems didn't have to poll, I'd still be happy if the MMC core change could go in. ...but I'm a pragmatist and know that the polling isn't _terrible_, so if that's the way we need to go then so be it. -- My understanding was that Yuvaraj was going to spin his patch to use a similar type of scheme to autodetect affected SoCs (looking for SoCs known to have this problem where the platform data shows them using the built-in card detect) and then enable MMC_CAP_NEEDS_POLL. I haven't seen a patch from him, so maybe you could post this up? Note: the reason why exynos5250-snow and some other boards aren't affected yet is that the regulator is simply not specified in the device tree (it's just left always on). --- -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators
Hi, On Wed, Oct 1, 2014 at 6:06 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Hi, On Tuesday, September 30, 2014 10:22:34 AM Doug Anderson wrote: Bartlomiej On Mon, Sep 29, 2014 at 5:31 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Hi, On Friday, August 29, 2014 01:34:44 PM Ulf Hansson wrote: On 22 August 2014 15:47, Yuvaraj Kumar C D yuvaraj...@gmail.com wrote: This patch makes use of mmc_regulator_get_supply() to handle the vmmc and vqmmc regulators.Also it moves the code handling the these regulators to dw_mci_set_ios().It turned on the vmmc and vqmmc during MMC_POWER_UP and MMC_POWER_ON,and turned off during MMC_POWER_OFF. Signed-off-by: Yuvaraj Kumar C D yuvaraj...@samsung.com Thanks! Applied for next. Unfortunately this patch breaks mmc1 card (Kingston 32GB micro SDHC) detection on Exynos5420 Arndale Octa for me: [ 10.797979] dwmmc_exynos 1222.mmc: no support for card's volts [ 10.797998] mmc1: error -22 whilst initialising SD card Without the patch: [ 10.866926] mmc_host mmc1: Bus speed (slot 0) = 5000Hz (slot req 5000Hz, actual 5000HZ div = 0) [ 10.866977] mmc1: new high speed SDHC card at address 1234 [ 10.868730] mmcblk1: mmc1:1234 SA32G 29.3 GiB [ 10.915054] mmcblk1: p1 p2 p3 The config is attached (exynos_defconfig doesn't work correctly for this board yet). Yup, this is an expected behavior, unfortunately. This was talked about extensively during the review of this patch series. Do you mean that a patch with a known regression has been merged into next branch of mmc tree? It would be quite sad if it would be true. ...so looking at your dts file, it looks like this _isn't_ your problem. Your vmmc regulator is listed as always on, so I believe that you're OK. Your regulator probably _shouldn't_ be always on (it prevents power cycling the SD card) but right now it's good that it is... If any board has a vmmc that is not listed as always on then it would regress with the first two patches applied (and without the 3rd), but there are none that I personally know of that do. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/3] mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators
Hi, On Wed, Oct 1, 2014 at 7:00 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Hi, On Wednesday, October 01, 2014 12:47:52 AM YUVARAJ CD wrote: Since I am out of station, i dont have an access to my work set up. Can you send me the dts entries of sd crad and their corresponding regulator entries? From arch/arm/boot/dts/exynos5420-arndale-octa.dts: ... mmc@1220 { status = okay; broken-cd; supports-highspeed; card-detect-delay = 200; samsung,dw-mshc-ciu-div = 3; samsung,dw-mshc-sdr-timing = 0 4; samsung,dw-mshc-ddr-timing = 0 2; pinctrl-names = default; pinctrl-0 = sd0_clk sd0_cmd sd0_bus4 sd0_bus8; vmmc-supply = ldo10_reg; slot@0 { reg = 0; bus-width = 8; }; }; mmc@1222 { status = okay; supports-highspeed; card-detect-delay = 200; samsung,dw-mshc-ciu-div = 3; samsung,dw-mshc-sdr-timing = 2 3; samsung,dw-mshc-ddr-timing = 1 2; pinctrl-names = default; pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; vmmc-supply = ldo10_reg; slot@0 { reg = 0; bus-width = 4; }; }; ... ldo10_reg: LDO10 { regulator-name = PVDD_PRE_1V8; regulator-min-microvolt = 180; regulator-max-microvolt = 180; regulator-always-on; }; I don't have schematics for Arndale Octa, but the above is really fishy. vmmc shouldn't be 1.8V. That's the general power signal to MMC and should be 2.7V - 3.6V. vqmmc could be 1.8V in certain situations, but my understanding is that for maximum compatibility it should at least start out identical to vmmc (and later go down to 1.7V - 1.95V). My first thought would be to just remove the vmmc-supply from your DTS. I think we could land that and pick it back easily. That will get you something working and won't introduce any regressions because: 1. MMC core will give you a dummy regulator 2. The code will default to assuming that vmmc is 3.3V, which is what it used to do anyway. 3. The only referenced regulator is always on anyway. Separately you could specify a proper vmmc and maybe even a vqmmc. On SMDK5420 I see this for the SD card (mmc2): * vmmc should be VDD_SD_2V8. From LDO19. * vqmmc should be VDDQ_MMC2_AP. From LDO13. OK, I dug up the Arndale schematics. For mmc2: * vmmc should be PVDD_TFLASH_2V8. That's LDO19. * vqmmc (hooked up to VDDQ_MMC2): PVDD_APIO_MMCOFF_2V8. LDO13 just like SMDK. ...sadly it looks like Anrdale has a schematics problem that prevents you from doing UHS. I see that the data lines are pulled up to PVDD_TFLASH_2V8 (vmmc), not pulled up to PVDD_APIO_MMCOFF_2V8 (vqmmc). I think that means that if you ever lower vqmmc to 1.8V (as needed for UHS) then you'll still be pulling up to 2.8V. That's not good. You should probably make sure that both LDO13 and LDO19 are listed as being exactly 2.8V. Anyway, the above has (obviously) not been tested and is just based on a casual browsing of schematics. Please let me know how it goes. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board
Bartiomiej, On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it. Without this patch: - mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators patch causes a SD card detection to fail - mmc: dw_mmc: Support voltage changes patch causes a boot hang This patch fixes both above problems. Suggested-by: Doug Anderson diand...@google.com Cc: Yuvaraj Kumar C D yuvaraj...@samsung.com Cc: Ulf Hansson ulf.hans...@linaro.org Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- arch/arm/boot/dts/exynos5420-arndale-octa.dts |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts === --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 @@ -74,7 +74,8 @@ samsung,dw-mshc-ddr-timing = 1 2; pinctrl-names = default; pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; - vmmc-supply = ldo10_reg; + vmmc-supply = ldo19_reg; + vqmmc-supply = ldo13_reg; This looks right to me. ...but I notice that ldo13 and ldo19 are not always-on in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in? ...eventually the always-on won't be needed, but for now I think it is... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board
Bartiomiej On Thu, Oct 2, 2014 at 9:39 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: On Thursday, October 02, 2014 09:19:08 AM Doug Anderson wrote: Bartiomiej, On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it. Without this patch: - mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators patch causes a SD card detection to fail - mmc: dw_mmc: Support voltage changes patch causes a boot hang This patch fixes both above problems. Suggested-by: Doug Anderson diand...@google.com Cc: Yuvaraj Kumar C D yuvaraj...@samsung.com Cc: Ulf Hansson ulf.hans...@linaro.org Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- arch/arm/boot/dts/exynos5420-arndale-octa.dts |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts === --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 @@ -74,7 +74,8 @@ samsung,dw-mshc-ddr-timing = 1 2; pinctrl-names = default; pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; - vmmc-supply = ldo10_reg; + vmmc-supply = ldo19_reg; + vqmmc-supply = ldo13_reg; This looks right to me. ...but I notice that ldo13 and ldo19 are not always-on in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in? ...eventually the always-on won't be needed, but for now I think it is... Card detection works fine without always-on. That's weird. 1. In the schematics I see XMMC2CDN has an external pullup to PVDD_TFLASH_2V8. 2. The internal pullup should (I think) be to VDDQ_MMC2 which is PVDD_APIO_MMCOFF_2V8. 3. In (51da224 mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators) we should be turning off both regulators in MMC_POWER_OFF. 4. If I understand correctly MMC_POWER_OFF is called when the card is ejected, which means that both regulators should be off when the card is ejected. 5. I don't think card detect can work if neither regulator is powered. One of the above points must be wrong. Any idea which one? Can you check to see if MMC_POWER_OFF is called for you when the card is ejected? Can you check to see if these regulators are off? -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board
Bartlomiej, On Thu, Oct 2, 2014 at 10:24 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Hi, On Thursday, October 02, 2014 09:45:41 AM Doug Anderson wrote: Bartiomiej On Thu, Oct 2, 2014 at 9:39 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: On Thursday, October 02, 2014 09:19:08 AM Doug Anderson wrote: Bartiomiej, On Thu, Oct 2, 2014 at 9:10 AM, Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com wrote: Regulators for MMC2 (SD card) are PVDD_TFLASH_2V8 (LDO19) for vmmc and PVDD_APIO_MMCOFF_2V8 (LDO13) for vqmmc. Currently the device tree entry for MMC2 uses PVDD_PRE_1V8 (LDO10) for vmmc and vqmmc is not specified. Fix it. Without this patch: - mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators patch causes a SD card detection to fail - mmc: dw_mmc: Support voltage changes patch causes a boot hang This patch fixes both above problems. Suggested-by: Doug Anderson diand...@google.com Cc: Yuvaraj Kumar C D yuvaraj...@samsung.com Cc: Ulf Hansson ulf.hans...@linaro.org Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com --- arch/arm/boot/dts/exynos5420-arndale-octa.dts |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: b/arch/arm/boot/dts/exynos5420-arndale-octa.dts === --- a/arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 15:44:53.014826886 +0200 +++ b/arch/arm/boot/dts/exynos5420-arndale-octa.dts 2014-10-02 17:35:24.110600398 +0200 @@ -74,7 +74,8 @@ samsung,dw-mshc-ddr-timing = 1 2; pinctrl-names = default; pinctrl-0 = sd2_clk sd2_cmd sd2_cd sd2_bus4; - vmmc-supply = ldo10_reg; + vmmc-supply = ldo19_reg; + vqmmc-supply = ldo13_reg; This looks right to me. ...but I notice that ldo13 and ldo19 are not always-on in the DTS. Are you sure card detect works for you if you eject your card and try to put it back in? ...eventually the always-on won't be needed, but for now I think it is... Card detection works fine without always-on. That's weird. 1. In the schematics I see XMMC2CDN has an external pullup to PVDD_TFLASH_2V8. 2. The internal pullup should (I think) be to VDDQ_MMC2 which is PVDD_APIO_MMCOFF_2V8. 3. In (51da224 mmc: dw_mmc: use mmc_regulator_get_supply to handle regulators) we should be turning off both regulators in MMC_POWER_OFF. 4. If I understand correctly MMC_POWER_OFF is called when the card is ejected, which means that both regulators should be off when the card is ejected. 5. I don't think card detect can work if neither regulator is powered. One of the above points must be wrong. Any idea which one? Can you check to see if MMC_POWER_OFF is called for you when the card is ejected? Can you check to see if these regulators are off? MMC_POWER_OFF is called on card removal and both regulators get disabled (I have verified that they are really off with regulator_is_enabled() which returns 1 before and 0 after disabling regulator). It seems that 5. is wrong? This really doesn't make a lot of sense to me, so I'm still kinda confused. If you want to call it good then that's your (and Ulf's) decision, but it's the kind of thing that would keep me up at night. How can this pin be high if all the regulators pulling it up are off? Is there a current leak somewhere and that's why it's working? How this is supposed to work (as I understand it): 1. When no card is inserted then this pin is supposed to be pulled up to VDDQ_MMC2. That could be either an internal or an external pullup. It should be pulled up to VDDQ_MMC2 (as opposed to any other voltage) since the exynos manual documents that this pin lives in the VDDQ_MMC2 io domain. Note that it could be pulled up externally to a different supply than the one going to VDDQ_MMC2, but for correctness it should be the same voltage. 2. When a card is inserted, the pin will be grounded (AKA this is an open drain pin). With your patch, can you probe the pin and see if card detect is high when all the regulators are off? Any idea how it gets high? If you turn off the internal pullup is it still high? -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: dts: fix MMC2 regulators for Exynos5420 Arndale Octa board
Hi, On Wed, Oct 8, 2014 at 8:20 AM, Arnd Bergmann a...@arndb.de wrote: Why does it cause a regression though? Does this mean you are breaking any boot with an old DT file and a new kernel? That would be very bad, the driver is supposed to keep working with an existing dtb. The old dts file specified completely the wrong regulator. It happened that the old code didn't really care, but when the code was fixed to care then things broke. If we need to keep old dts files working then the most sensible place to do it would be in a fixup old broken device trees stage somewhere in kernel boot. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ARM: EXYNOS: Call regulator core suspend prepare and finish functions
Javier, On Wed, Oct 15, 2014 at 5:01 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: The regulator framework has a set of helpers functions to be used when the system is entering and leaving from suspend but these are not called on Exynos platforms. This means that the .set_suspend_* function handlers defined in regulator drivers are never called when the system is suspended. Suggested-by: Doug Anderson diand...@chromium.org Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Could you also add a patch to your series ripping out the call in drivers/mfd/sec-core.c since it doesn't belong there. If you don't rip that out then it will be called twice on systems with that regulator. --- arch/arm/mach-exynos/suspend.c | 16 1 file changed, 16 insertions(+) diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c index f5d9773..5b9c551 100644 --- a/arch/arm/mach-exynos/suspend.c +++ b/arch/arm/mach-exynos/suspend.c @@ -20,6 +20,7 @@ #include linux/io.h #include linux/irqchip/arm-gic.h #include linux/err.h +#include linux/regulator/machine.h #include asm/cacheflush.h #include asm/hardware/cache-l2x0.h @@ -270,14 +271,29 @@ static int exynos_suspend_enter(suspend_state_t state) static int exynos_suspend_prepare(void) { + int ret; + s3c_pm_check_prepare(); + /* +* REVISIT: It would be better if struct platform_suspend_ops +* .prepare handler get the suspend_state_t as a parameter to +* avoid hard-coding the suspend to mem state. It's safe to do +* it only because the suspend_valid_only_mem function is the +* .valid callback used to check if a given state is supported +* by the platform. +*/ + ret = regulator_suspend_prepare(PM_SUSPEND_MEM); + if (ret) + pr_info(Failed to prepare regulators for system suspend\n); + nit: can you put this before s3c_pm_check_prepare(). pm_check is pretty darn broken and I have a feeling that it will eventually be ripped out (or in the very least ported to not be Samsung-specific and include all of the suspend volatile crud that we have in the chromeos-3.8 kernel), but might as well try not to break it further. Changing the order also has the advantage of making prepare / finish opposite orders (good!) and handling the fact that you would call s3c_pm_check_prepare() but not s3c_pm_check_cleanup() if regulator_suspend_prepare() fails. return 0; } static void exynos_suspend_finish(void) { s3c_pm_check_cleanup(); + regulator_suspend_finish(); } static const struct platform_suspend_ops exynos_suspend_ops = { -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions
Javier, On Thu, Oct 16, 2014 at 3:13 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: The regulator framework has a set of helpers functions to be used when the system is entering and leaving from suspend but these are not called on Exynos platforms. This means that the .set_suspend_* function handlers defined by regulator drivers are not called when the system is suspended. Suggested-by: Doug Anderson diand...@chromium.org Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk --- arch/arm/mach-exynos/suspend.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c index cc8d237..ee9a8e0 100644 --- a/arch/arm/mach-exynos/suspend.c +++ b/arch/arm/mach-exynos/suspend.c @@ -20,6 +20,7 @@ #include linux/io.h #include linux/irqchip/arm-gic.h #include linux/err.h +#include linux/regulator/machine.h #include asm/cacheflush.h #include asm/hardware/cache-l2x0.h @@ -443,6 +444,22 @@ static int exynos_suspend_enter(suspend_state_t state) static int exynos_suspend_prepare(void) { + int ret; + + /* +* REVISIT: It would be better if struct platform_suspend_ops +* .prepare handler get the suspend_state_t as a parameter to +* avoid hard-coding the suspend to mem state. It's safe to do +* it now only because the suspend_valid_only_mem function is +* used as the .valid callback used to check if a given state +* is supported by the platform anyways. +*/ + ret = regulator_suspend_prepare(PM_SUSPEND_MEM); + if (ret) { + pr_err(Failed to prepare regulators for system suspend\n); + return ret; + } + s3c_pm_check_prepare(); return 0; @@ -451,6 +468,7 @@ static int exynos_suspend_prepare(void) static void exynos_suspend_finish(void) { s3c_pm_check_cleanup(); + regulator_suspend_finish(); It turns out that regulator_suspend_finish() actually returns an error code. Could you print a warning if you see it? Other than that, feel free to add my Reviewed-by. Thanks! -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] ARM: EXYNOS: Call regulator core suspend prepare and finish functions
Javier, On Mon, Oct 20, 2014 at 9:58 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: It turns out that regulator_suspend_finish() actually returns an error code. Could you print a warning if you see it? Yes, I noticed this when looking at Chris patch for Rockchip but didn't re-spin because I'm not sure anymore if this is the right solution. I mean, if is correct to add the same calls on every platform or if the regulator suspend prepare and finish functions should be called from the suspend core instead. For example calling regulator_suspend_prepare() from platform_suspend_prepare() [0] will have the advantage of passing the correct suspend_state_t state instead of hard-coding PM_SUSPEND_MEM and will make the regulator suspend states to work on all platforms. Yes. If we can get this added to the core that would be better. I guess I was just trying to follow the suggestion that was in the regulator code: http://lxr.free-electrons.com/source/drivers/regulator/core.c#L3699 that says This will usually be called by machine suspend code prior to supending. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] Revert mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption
Javier, On Mon, Oct 20, 2014 at 2:05 PM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: This reverts commit b7cde7078d2344073c310aa65fc2b0a845d2cb5b (mfd: sec-core: Prepare regulators for suspend state to reduce power-consumption) Commit b7cde7078d23 called regulator_suspend_prepare() to prepare the regulators for a suspend state. But it did from the device pm suspend handler while the regulator suspend prepare function iterates over all regulators and not only the one managed by this device so it doesn't seems to be correct to call it from within a device driver. It is better to call the regulator suspend prepare/finish functions from platform code instead so this patch reverts the mentioned commit. Suggested-by: Doug Anderson diand...@chromium.org Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clocksource: exynos_mct: fix exynos4_mct_write
Tobias, On Tue, Oct 21, 2014 at 6:37 PM, Tobias Jakobi tjak...@math.uni-bielefeld.de wrote: EXYNOS4_MCT_L_MASK is defined as 0xff00, so applying this bitmask produces a number outside the range 0x00 to 0xff, which always results in execution of the default switch statement. Obviously this is wrong and git history shows that the bitmask inversion was incorrectly set during a refactoring of the MCT code. Fix this by putting the inversion at the correct position again. Reported-by: GP Orcullo kinsama...@gmail.com Signed-off-by: Tobias Jakobi tjak...@math.uni-bielefeld.de --- drivers/clocksource/exynos_mct.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Wow, the old code was pretty broken. Nice find! Was this found by code inspection / analysis or were you seeing some actual bug? I think it was broken back in March 2013 in (a1ba7a7 ARM: EXYNOS: add a register base address variable in mct controller driver), so 1.5 years. The broken code has been running for a long time, but that doesn't mean that there isn't a subtle issue... In any case, it seems like we should take this fix, assuming someone has tested it well. Reviewed-by: Doug Anderson diand...@chromium.org -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: dts: Add SPI flash node for Peach boards
Javier, On Mon, Nov 17, 2014 at 9:43 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: +spi_1 { + status = okay; + samsung,spi-src-clk = 0; + num-cs = 1; + cs-gpios = gpa2 5 0; + + spidev@0 { + compatible = spidev; This is common practice in the Chrome OS tree, but we've gotten pushback from upstream questioning about whether spidev is really a physical device. See: http://www.spinics.net/lists/linux-samsung-soc/msg29563.html I don't really have an answer for something better to do here but I figured I'd at least bring up the point. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] ARM: dts: Add SPI flash node for Peach boards
Javier, On Wed, Nov 19, 2014 at 2:07 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Hello Doug, Thanks for your feedback. On 11/18/2014 06:50 PM, Doug Anderson wrote: This is common practice in the Chrome OS tree, but we've gotten pushback from upstream questioning about whether spidev is really a physical device. See: http://www.spinics.net/lists/linux-samsung-soc/msg29563.html I see, I thought that it was a common practice in the mainline kernel too since I saw that many board DTS currently have a spidev node: $ git grep 'compatible = spidev' arch/arm/boot/dts/ | wc -l 19 I don't really have an answer for something better to do here but I figured I'd at least bring up the point. I wonder how the spidev user-space interface is supposed to be used when booting with Device Trees. OK. Please don't take my comments as a NAK on this patch. I should have done the same grep myself before sending--sorry. I just remembered the old conversation and looked for that instead. If the convention is to use spidev like this then I guess we're OK. I do wish it was a little more like i2c myself where you could get a direct access interface no matter what driver was bound underneath (and also if no drivers were bound underneath). ...but I could just be naive. ;) -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pulls and drive strengths in the pinctrl world
Linus, I'm currently working towards adapting exynos5250-snow (the ARM Chromebook) to work well in the new world of pinctrl. We've got a backport of exynos5250 pinctrl in our kernel-3.8 tree and are now fixing all of the bugs that have popped up. Patches will be sent upstream (where applicable) shortly. ...but I'm running into an issue when trying to specify pullups / pulldowns and drive strengths on lines that are just GPIOs or just interrupts. This is important not just for power usage but also for proper functioning (the default internal pulldown was overpowering the weak external pullup in one case). In the old GPIO specifier you could do specify pulls, but the new simpler one doesn't allow it. I've managed to make things work and you can see my progress at https://gerrit.chromium.org/gerrit/#/c/51232/, but it feels a bit awkward. Is there a better way? If so, then I'm all ears! :) If not, then I guess that what I have will have to do for now. ...but I really wish that: * The GPIO specifier could specify initial drive strength and pull values for pins. * The interrupt specifier could specify pull values for pins (drive strength shouldn't be needed since these are inputs). Some examples from the gerrit CL referenced above... Here's how I need to do things when I'm using just an interrupt: pinctrl@1140 { cyapa_irq: cyapa-irq { samsung,pins = gpx1-2; samsung,pin-function = 0xf; samsung,pin-pud = 0; samsung,pin-drv = 0; }; }; trackpad { reg = 0x67; compatible = cypress,cyapa; interrupts = 2 0; interrupt-parent = gpx1; pinctrl-names = default; pinctrl-0 = cyapa_irq; wakeup-source; }; I really wish I could add a 3rd number to the interrupt specifier for pud and skip the pinctrl bit: trackpad { reg = 0x67; compatible = cypress,cyapa; interrupts = 2 0 0; interrupt-parent = gpx1; wakeup-source; }; An example with the GPIO specifier instead of the interrupt one: pinctrl@1140 { ptn3460_gpios: ptn3460-gpios { samsung,pins = gpy2-5, gpx1-5; samsung,pin-function = 1; samsung,pin-pud = 0; samsung,pin-drv = 0; }; }; ptn3460-bridge@20 { compatible = nxp,ptn3460; reg = 0x20; powerdown-gpio = gpy2 5 0; reset-gpio = gpx1 5 0; edid-emulation = 5; pinctrl-names = default; pinctrl-0 = ptn3460_gpios; }; I don't want to specify function/direction (code can handle that), but do wish I could specify the pulls and strength. Perhaps: ptn3460-bridge@20 { compatible = nxp,ptn3460; reg = 0x20; powerdown-gpio = gpy2 5 0 0 0; reset-gpio = gpx1 5 0 0 0; edid-emulation = 5; }; -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
Tomasz, Thanks for your comments. I'm glad I'm not totally off-track. I'll respond to most things in reply to Linus' email, but a few here: On Wed, May 15, 2013 at 10:26 AM, Tomasz Figa tomasz.f...@gmail.com wrote: pinctrl@1140 { cyapa_irq: cyapa-irq { samsung,pins = gpx1-2; samsung,pin-function = 0xf; You can omit samsung,pin-function here. One potential reason for leaving them is the hopes that it might cause a little less line glitching, especially in the case of outputs. There is some delay between the pinmux being configured at the start of device probe and the device actually claiming the GPIO. Things might be worse in the case of deferred probe (?). Can you think of any reason to remove (other than yet more lines of device tree to deal with)? samsung,pin-pud = 0; samsung,pin-drv = 0; For inputs I guess you can omit samsung,pin-drv as well. I will probably leave them even for inputs. They shouldn't matter but I like the idea of initting things to a known state... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
Linus, Thank you for your comments. See below... Stephen: sorry for missing you earlier! :( On Wed, May 15, 2013 at 11:29 AM, Linus Walleij linus.wall...@linaro.org wrote: But please use the preprocessor to provide symbolic names for the configurations. See for example these two patches from J-C: http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg32164.html Ah, that does look nice! This probably needs to be addressed in a separate patch to cleanup all of the samsung pinctrl devicetrees. I don't think the idea with device tree is to write as compact trees as possible, but as expressive and exact yet abstract trees as possible for OS independence. The compactness was one benefit, but also it was about trying to avoid excessive duplication of information. I found it awkward that I needed to list that my interrupt was gpx1-2 in two different ways. I would find it just as good if I could express things like this (for interrupts): pinctrl@1140 { cyapa_irq: cyapa-irq { samsung,pins = gpx1-2; samsung,pin-function = 0xf; samsung,pin-pud = 0; samsung,pin-drv = 0; interrupt-controller; #interrupt-cells = 1; }; }; trackpad { reg = 0x67; compatible = cypress,cyapa; interrupt-parent = cyapa_irq; interrupts = 0; wakeup-source; }; In this case I'm not saying that my interrupt parent is gpx1-2 in two separate places that could diverge. I could come up with a similar example for GPIOs. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
Tomasz, On Wed, May 15, 2013 at 2:36 PM, Tomasz Figa tomasz.f...@gmail.com wrote: One potential reason for leaving them is the hopes that it might cause a little less line glitching, especially in the case of outputs. There is some delay between the pinmux being configured at the start of device probe and the device actually claiming the GPIO. Things might be worse in the case of deferred probe (?). Can you think of any reason to remove (other than yet more lines of device tree to deal with)? Well, actually in case of interrupts the function should not be configured manually, because it is likely to cause a false interrupt to be caught, before appropriate interrupt trigger type is configured. The correct way is to leave setting pin function to EINT to the pin control driver once the trigger gets configured (the pin control driver configures pin function from set_irq_type callback). Ah, OK. I'll set to input for these. I will probably leave them even for inputs. They shouldn't matter but I like the idea of initting things to a known state... Well, the binding you proposed for interrupts doesn't initialize it. This is why I pointed that it can be omitted using current way as well. Agreed. ...though you could say that the actual code in that case would just be setting the drive strength to 0 (for consistency). ;) -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
Tomasz, On Wed, May 15, 2013 at 2:41 PM, Tomasz Figa tomasz.f...@gmail.com wrote: This will be hard, since the phandle in interrupt-parent is represented by an IRQ domain in kernel code. One-interrupt IRQ domains seem a bit awkward to me. Since we are already going to modify the binding, let's think a bit more about this problem and try to figure out a solution that doesn't add any disadvantages (at least any significant) to avoid such situation in future again. I'm definitely not super familiar with the implementation at that level of detail, so don't take my proposed syntax as something I've thought all the way through. ...but hopefully you understand what I'm getting at in terms of eliminating duplication? Thanks! :) -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
Tomasz / Linus, On Wed, May 15, 2013 at 3:06 PM, Tomasz Figa tomasz.f...@gmail.com wrote: Yes. I don't like the current way too much either, duplication being one of the reasons. Do you have any other ideas? It sounds like Linus didn't like my suggestion and makes some good points... I don't have any other great ideas other than having an argument about whether the concept of pulls should be added to the GPIO subsystem (and backed by pinmux). Then we could make an argument that default pull state belonged in the GPIO specifier. OK, maybe we should just pretend that I didn't bring that up. ;) ...but I'm definitely interested in other ideas to eliminate the duplication. Until then I'm planning on submitting what I have locally. I'll probably send some version of it upstream fairly soon. I will probably submit without trying to get all the preprocessor symbols names done and will understand if Linus NAKs because of that. I don't have time to take that on at the moment but can always come back and rework that patch later. ;) -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
Stephen, On Wed, May 15, 2013 at 4:51 PM, Stephen Warren swar...@wwwdotorg.org wrote: I don't really see much disadvantage here; the interrupt bindings specify things related to interrupts and the pinctrl bindings specify thing related to pin configuration. OK. If this is the best way then I can accept that and maybe we should just drop this thread. What do people think? It means less work for me in the short term since I've already got it implemented that way and then I don't need to submit any patches to try to change things! ;) If you want to condense the DT, I'd suggest using a the pinctrl hogging feature, i.e. don't put pinctrl-* properties in the trackpad node, but rather define a system-wide default pinctrl state in the pin controller node itself. That configuration will be applied as soon as the pin controller driver is registered. That'd be the same as the above, with the following added: pinctrl@1140 { pinctrl-names = default; pinctrl-0 = cyapa_irq; }; except that the pinctrl-0 property would probably end up configuring a whole bunch of basic pinctrl state rather than just that one interrupt pin. I prefer to put all the static pinctrl configuration in the pinctrl hog, and only the dynamic stuff in the individual device nodes. I know LinusW won't like this suggestion much though:-) Ah right! I forgot about hogs in this case. That's also reasonable as a solution and is similar to what we've got in the tree for powerdown configuration of pins (I'll try to post this patch soon too, WIP at https://gerrit.chromium.org/gerrit/#/c/51292/ and https://gerrit.chromium.org/gerrit/#/c/51372/. It sounds like there's a bit of disagreement about the best way so I'll probably keep the way I have. ...but I'll keep hogs in my back pocket. I don't like that myself, since it makes the interrupt binding (and I assume you'd want to go back to the similar misuse in the GPIO binding) also configure pinctrl-related stuff. Fair enough. :) -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
Tomasz, On Wed, May 15, 2013 at 5:13 PM, Tomasz Figa tomasz.f...@gmail.com wrote: I don't have anything interesting at the moment. It's a bit late now here (2 AM), so I'm going to get some sleep first. Sorry for keeping you up. Sleep is good! Also after reading Stephen's reply, I'm wondering if hogging wouldn't solve the problem indeed. (It might have to be fixed on pinctrl-samsung first, as last time I tried to use it, it caused some errors from pinctrl core, but haven't time to track them down, as it wasn't anything important at that time). I will give it a shot tomorrow morning and see how it looks. I'll also evaluate Stephen's suggestions then once I've see how it looks with the current bindings... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pulls and drive strengths in the pinctrl world
Tomasz, On Wed, May 15, 2013 at 5:19 PM, Tomasz Figa tomasz.f...@gmail.com wrote: Hmm, last thing before I fell asleep: We already have support for power down configuration in pinctrl-samsung. See samsung,pin-conpdn and samsung,pin-pudpdn. Dang it! OK, we'll work on using those. Also I already have patches for suspend/resume support of pinctrl-samsung and pinctrl-exynos, as well as configuration of wake-up sources. I'm going to post them soon. Huh, I wonder if they look just like the one we've been working on: * https://gerrit.chromium.org/gerrit/#/c/51336/ * https://gerrit.chromium.org/gerrit/#/c/51342/ Those are about ready for upstream, too. I was going to send them this morning when I found out that we were missing a bunch of pinctrl patches and had to rework then. :( Anyway, we can compare our solutions and figure out which is better. ;) I'm happy with anything that works! -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Fix suspend/resume issues created by pinmux on exynos
This set of patches fixes some problems with suspend/resume that were introduced by the switch from the old gpio code to the new pinmux code. Specifically: * It adds saving and restoring of pincontrol registers. * It fixes eint wakeups. This set of two patches was verified on a backport of the current pinmux code onto 3.8 on a Samsung ARM Chromebook. Suspend/resume does not seem functional on the ARM Chromebook on current ToT Linux so I couldn't validate there. This gets us one step closer, though! Since patches applied cleanly I'm fairly certain that they will work on ToT as well as they do in our tree. These patches have only been tested on exynos5250. I have made an effort to support other samsung boards (even those with two CONF registers), but that support is untested. Tomasz Figa has said that he has similar patches in development. I'm posting what we have here but if Tomasz's patches end up being more suitable I have no objections to taking them over these (or of Tomasz wants to merge the two?). If you'd like to see the gerrit reviews of these in the Chrome OS tree, you can see: * https://gerrit.chromium.org/gerrit/#/c/51336/4 * https://gerrit.chromium.org/gerrit/#/c/51342/3 Doug Anderson (1): pinctrl: samsung: fix suspend/resume functionality Prathyush K (1): pinctrl: exynos: fix eint wakeup by using irq_set_wake() drivers/pinctrl/pinctrl-exynos.c | 45 ++--- drivers/pinctrl/pinctrl-exynos.h | 3 +- drivers/pinctrl/pinctrl-samsung.c | 199 ++ drivers/pinctrl/pinctrl-samsung.h | 13 +++ 4 files changed, 247 insertions(+), 13 deletions(-) -- 1.8.2.1 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality
The GPIO states need to be restored after s2r and this is not currently supported in the pinctrl driver. This patch saves the gpio states before suspend and restores them after resume. Logic and commenting for samsung_pinctrl_resume_noirq() is heavily borrowed from the old samsung_gpio_pm_2bit_resume(), which seemed to do this a reasonable way. Patch originally from Prathyush K prathyus...@samsung.com but rewritten by Doug Anderson diand...@chromium.org. Signed-off-by: Prathyush K prathyus...@samsung.com Signed-off-by: Doug Anderson diand...@chromium.org --- drivers/pinctrl/pinctrl-samsung.c | 199 ++ drivers/pinctrl/pinctrl-samsung.h | 11 +++ 2 files changed, 210 insertions(+) diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c index 9763668..0891667 100644 --- a/drivers/pinctrl/pinctrl-samsung.c +++ b/drivers/pinctrl/pinctrl-samsung.c @@ -964,6 +964,204 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) return 0; } +#ifdef CONFIG_PM + +/** + * samsung_pinctrl_resume_noirq - save pinctrl state for suspend + * + * Save data for all banks handled by this device. + */ +static int samsung_pinctrl_suspend_noirq(struct device *dev) +{ + struct samsung_pinctrl_drv_data *drvdata = dev_get_drvdata(dev); + struct samsung_pin_ctrl *ctrl = drvdata-ctrl; + void __iomem * const virt_base = drvdata-virt_base; + int i; + + for (i = 0; i ctrl-nr_banks; i++) { + struct samsung_pin_bank *bank = ctrl-pin_banks[i]; + const struct samsung_pin_bank_type *type = bank-type; + void __iomem * const reg = virt_base + bank-pctl_offset; + + bank-pm_save.con = readl(reg + + type-reg_offset[PINCFG_TYPE_FUNC]); + if (type-fld_width[PINCFG_TYPE_FUNC] 4) + bank-pm_save.con |= (u64)readl(reg + 4 + + type-reg_offset[PINCFG_TYPE_FUNC]) 32; + bank-pm_save.dat = readl(reg + + type-reg_offset[PINCFG_TYPE_DAT]); + bank-pm_save.pud = readl(reg + + type-reg_offset[PINCFG_TYPE_PUD]); + bank-pm_save.drv = readl(reg + + type-reg_offset[PINCFG_TYPE_DRV]); + + if (type-fld_width[PINCFG_TYPE_CON_PDN]) { + bank-pm_save.conpdn = readl(reg + + type-reg_offset[PINCFG_TYPE_CON_PDN]); + bank-pm_save.pudpdn = readl(reg + + type-reg_offset[PINCFG_TYPE_PUD_PDN]); + } + + dev_dbg(dev, Save %s @ %p (con %#010llx)\n, + bank-name, reg, bank-pm_save.con); + } + + return 0; +} + +/** + * is_sfn - test whether a pin config represents special function. + * + * Test whether the given masked+shifted bits of an GPIO configuration + * are one of the SFN (special function) modes. + */ +static inline int is_sfn(u32 con) +{ + return con = 2; +} + +/** + * is_in - test if the given masked+shifted GPIO configuration is an input. + */ +static inline int is_in(u32 con) +{ + return con == 0; +} + +/** + * is_out - test if the given masked+shifted GPIO configuration is an output. + */ +static inline int is_out(u32 con) +{ + return con == 1; +} + +/** + * samsung_pinctrl_resume_noirq - restore pinctrl state from suspend + * + * Restore one of the GPIO banks that was saved during suspend. This is + * not as simple as once thought, due to the possibility of glitches + * from the order that the CON and DAT registers are set in. + * + * The three states the pin can be are {IN,OUT,SFN} which gives us 9 + * combinations of changes to check. Three of these, if the pin stays + * in the same configuration can be discounted. This leaves us with + * the following: + * + * { IN = OUT } Change DAT first + * { IN = SFN } Change CON first + * { OUT = SFN } Change CON first, so new data will not glitch + * { OUT = IN } Change CON first, so new data will not glitch + * { SFN = IN } Change CON first + * { SFN = OUT } Change DAT first, so new data will not glitch [1] + * + * We do not currently deal with the UP registers as these control + * weak resistors, so a small delay in change should not need to bring + * these into the calculations. + * + * [1] this assumes that writing to a pin DAT whilst in SFN will set the + * state for when it is next output. + */ +static int samsung_pinctrl_resume_noirq(struct device *dev) +{ + struct samsung_pinctrl_drv_data *drvdata = dev_get_drvdata(dev); + struct samsung_pin_ctrl *ctrl = drvdata-ctrl; + void __iomem * const virt_base = drvdata-virt_base; + int i; + + for (i = 0; i ctrl-nr_banks; i++) { + const struct samsung_pin_bank *bank = ctrl-pin_banks[i
[PATCH 2/2] pinctrl: exynos: fix eint wakeup by using irq_set_wake()
From: Prathyush K prathyus...@samsung.com Add the irq_set_wake function for exynos pinctrl to configure the external interrupt wakeup mask register. [dianders: minor nit fixes; port to ToT] Signed-off-by: Prathyush K prathyus...@samsung.com Signed-off-by: Doug Anderson diand...@chromium.org --- drivers/pinctrl/pinctrl-exynos.c | 45 --- drivers/pinctrl/pinctrl-exynos.h | 3 ++- drivers/pinctrl/pinctrl-samsung.h | 2 ++ 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644 --- a/drivers/pinctrl/pinctrl-exynos.c +++ b/drivers/pinctrl/pinctrl-exynos.c @@ -30,6 +30,8 @@ #include linux/spinlock.h #include linux/err.h +#include plat/pm.h + #include pinctrl-samsung.h #include pinctrl-exynos.h @@ -326,6 +328,24 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd, unsigned int type) return 0; } +static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int state) +{ + struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd); + const int eint_num = bank-eint_base + irqd-hwirq; + unsigned long bit = 1L eint_num; + + pr_info(wake %s for eint %d / %s[%ld]\n, + state ? enabled : disabled, + eint_num, bank-name, irqd-hwirq); + + if (!state) + s3c_irqwake_eintmask |= bit; + else + s3c_irqwake_eintmask = ~bit; + + return 0; +} + /* * irq_chip for wakeup interrupts */ @@ -335,6 +355,7 @@ static struct irq_chip exynos_wkup_irq_chip = { .irq_mask = exynos_wkup_irq_mask, .irq_ack= exynos_wkup_irq_ack, .irq_set_type = exynos_wkup_irq_set_type, + .irq_set_wake = exynos_wkup_irq_set_wake, }; /* interrupt handler for wakeup interrupts 0..15 */ @@ -543,10 +564,10 @@ static struct samsung_pin_bank exynos4210_pin_banks1[] = { EXYNOS_PIN_BANK_EINTN(8, 0x1A0, gpy4), EXYNOS_PIN_BANK_EINTN(8, 0x1C0, gpy5), EXYNOS_PIN_BANK_EINTN(8, 0x1E0, gpy6), - EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00), - EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04), - EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08), - EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c), + EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0), + EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8), + EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16), + EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24), }; /* pin banks of exynos4210 pin-controller 2 */ @@ -629,10 +650,10 @@ static struct samsung_pin_bank exynos4x12_pin_banks1[] = { EXYNOS_PIN_BANK_EINTN(8, 0x1A0, gpy4), EXYNOS_PIN_BANK_EINTN(8, 0x1C0, gpy5), EXYNOS_PIN_BANK_EINTN(8, 0x1E0, gpy6), - EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00), - EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04), - EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08), - EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c), + EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0), + EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8), + EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16), + EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24), }; /* pin banks of exynos4x12 pin-controller 2 */ @@ -724,10 +745,10 @@ static struct samsung_pin_bank exynos5250_pin_banks0[] = { EXYNOS_PIN_BANK_EINTN(8, 0x220, gpy4), EXYNOS_PIN_BANK_EINTN(8, 0x240, gpy5), EXYNOS_PIN_BANK_EINTN(8, 0x260, gpy6), - EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00), - EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04), - EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08), - EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c), + EXYNOS_PIN_BANK_EINTW(8, 0xC00, gpx0, 0x00, 0), + EXYNOS_PIN_BANK_EINTW(8, 0xC20, gpx1, 0x04, 8), + EXYNOS_PIN_BANK_EINTW(8, 0xC40, gpx2, 0x08, 16), + EXYNOS_PIN_BANK_EINTW(8, 0xC60, gpx3, 0x0c, 24), }; /* pin banks of exynos5250 pin-controller 1 */ diff --git a/drivers/pinctrl/pinctrl-exynos.h b/drivers/pinctrl/pinctrl-exynos.h index 9b1f77a..d98e9ff 100644 --- a/drivers/pinctrl/pinctrl-exynos.h +++ b/drivers/pinctrl/pinctrl-exynos.h @@ -65,13 +65,14 @@ .name = id\ } -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \ +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\ { \ .type = bank_type_alive, \ .pctl_offset= reg, \ .nr_pins= pins, \ .eint_type = EINT_TYPE_WKUP, \ .eint_offset= offs, \ + .eint_base = base, \ .name = id\ } diff --git a/drivers/pinctrl/pinctrl-samsung.h b/drivers/pinctrl/pinctrl-samsung.h
Re: Pulls and drive strengths in the pinctrl world
Tomasz / Stephen, On Wed, May 15, 2013 at 5:55 PM, Doug Anderson diand...@google.com wrote: Also after reading Stephen's reply, I'm wondering if hogging wouldn't solve the problem indeed. (It might have to be fixed on pinctrl-samsung first, as last time I tried to use it, it caused some errors from pinctrl core, but haven't time to track them down, as it wasn't anything important at that time). I will give it a shot tomorrow morning and see how it looks. I'll also evaluate Stephen's suggestions then once I've see how it looks with the current bindings... I wrote this out and it had some nice properties (it was a little more concise), but had the downside that there's no reference from the GPIO usage back to the pinmux. I'd rather have the reference there in the hopes that it will help others get things right when they make changes to the dts file (they'll notice the reference and know that they need to change that too). ...so I think the summary is: I'm OK with keeping what we have. It may be a little awkward in some ways but it's definitely worth it to get all of the benefits of the pinmux / GPIO separation. :) For the curious of what my prototype looked like (feel free to ignore--I'm not planning on keeping this and I didn't actually try testing it), I've included it below. This is just the bit from cros5250-common (the common file shared among several similar boards), so I'd need something similar in exynos5250-snow). pinctrl@1140 { /* Default states for hogs follow */ nopull_inputs_cros5250_a: nopull-inputs-cros5250-a { samsung,pins = gpx1-2, /* trackpad */ gpx1-3, /* gpio-keys - power */ gpx3-2; /* max77686 */ samsung,pin-function = 0; samsung,pin-pud = 0; samsung,pin-drv = 0; }; pulldown_inputs_cros5250-a: pulldown-inputs-cros5250_a { samsung,pins = gpx3-7; /* hdmi */ samsung,pin-function = 0; samsung,pin-pud = 1; samsung,pin-drv = 0; }; simple_outputs_cros5250-a: simple-outputs-cros5250_a { samsung,pins = gpx0-1, /* wifi-en */ gpx0-2, /* wifi-rst */ gpx1-7; /* max98095-en */ samsung,pin-function = 1; samsung,pin-pud = 0; samsung,pin-drv = 0; } pinctrl-names = default; pinctrl-0 = nopull_inputs_cros5250_a pulldown_inputs_cros5250_a simple_outputs_cros5250_a; }; pinctrl@1340 { simple_outputs_cros5250-b: simple-outputs-cros5250_b { samsung,pins = gpe1-0 /* hsic reset */; samsung,pin-function = 1; samsung,pin-pud = 0; samsung,pin-drv = 0; }; pinctrl-names = default; pinctrl-0 = simple_outputs_cros5250_b; }; -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality
Tomasz, Thanks for the review! I'll get a new patch out either today or tomorrow... On Thu, May 16, 2013 at 12:19 PM, Tomasz Figa tomasz.f...@gmail.com wrote: +/** + * samsung_pinctrl_resume_noirq - save pinctrl state for suspend + * + * Save data for all banks handled by this device. + */ +static int samsung_pinctrl_suspend_noirq(struct device *dev) +{ + struct samsung_pinctrl_drv_data *drvdata = dev_get_drvdata(dev); + struct samsung_pin_ctrl *ctrl = drvdata-ctrl; + void __iomem * const virt_base = drvdata-virt_base; Nit: This const is ugly :) . Is it needed for anything? Not anything really. I just got in the habit of adding them for variables that are simple shorthand variables: AKA I'm only creating this variable to avoid typing some long complicated thing below. It's a hint to someone reading the code that they don't need to think about it. I have also occasionally caught bugs by doing this. ...but I can understand the dislike. I'll remove this and other similar (but keep const pointers). + if (type-fld_width[PINCFG_TYPE_FUNC] 4) What is this condition supposed to check? It is supposed to be checking whether there are two CON registers in use. ...oh, but that's probably not the right way to do it now that I think about it. I need to check (bank-nr_pins * type-fld_width[PINCFG_TYPE_FUNC]). I will fix. + bank-pm_save.con |= (u64)readl(reg + 4 + + type-reg_offset[PINCFG_TYPE_FUNC]) 32; This looks ugly. Whatever is going on here, wouldn't it be better to use separate field, like con2 or something? Probably. The resume code seemed cleaner with a 64-bit value, but I think I could make it nearly as clean with two 32-bit ones by using a helper function. I wonder if you couldn't do all the saving here in a single loop over all pin control types, like: unsigned int offsets = bank-type-reg_offsets; unsigned int widths = bank-type-fld_width; for (i = 0; i PINCFG_TYPE_NUM; ++i) if (widths[i]) bank-pm_save[i] = readl(reg + offsets[i]); The only thing not handled by this loop is second CON registers in banks with two of them. I can't think of any better solution for this other than just adding a special case after the loop. Yes, that would work. I think it wasn't possible when I first wrote the code against an older code base that didn't have the arrays. I can give it a shot if it doesn't make restore too bad... Now as I think of it, do CON_PDN and PUD_PDN really need to be saved? I couldn't find in the documentation if they are preserved or lost in sleep mode. Do you have some information on this? I remember it being important. Running a test now. Yes, it's important on exynos5250. As an example: [ 62.22] samsung-pinctrl 1140.pinctrl: Restore gpa2@f0048040 (0x=0x2212 ch 0x) [ 62.22] samsung-pinctrl 1140.pinctrl: Restore gpb0@f0048060 CON_PDN (0x=0x02aa) [ 62.22] samsung-pinctrl 1140.pinctrl: Restore gpb0@f0048060 PUD_PDN (0x=0x0155) I wonder if the whole restoration procedure couldn't be simplified. I don't remember my version being so complicated, but I don't have my patch on my screen at the moment, so I might be wrong on this. I debated about this a bunch. Perhaps I should just delete it. I saw that it was there in the old 2-bit code and it also seemed quite reasonable, so I kept it. Things seem to work OK without it, but most things are pretty tolerant to their lines glitching (or even driving high to low for a short period of time). ...but your question made me check again. From previous experimentation I'm pretty certain that most pins on the exynos are held in the powerdown state even during early bootup of the SoC. The hope is that they are released from powerdown _after_ the GPIO init is called. If not then we're already glitching somewhat as we transition from powerdown state to default state before this function finally gets called. Looking at exynos, that's probably done in exynos_pm_resume(), maybe by mucking with the pad retention options? Oh, that probably means taht no_irq() is too late and that I need to figure out how to get my code called earlier... The exynos_pm_resume() is called by syscore. +#else +#define samsung_pinctrl_suspend_noirqNULL +#define samsung_pinctrl_resume_noirq NULL +#endif + +static const struct dev_pm_ops samsung_pinctrl_dev_pm_ops = { + .suspend_noirq = samsung_pinctrl_suspend_noirq, + .resume_noirq = samsung_pinctrl_resume_noirq, +}; I'm not sure if resume_noirq is really early enough. Some drivers might already need correct pin configuration in their resume_noirq callback. In my patch I have used syscore_ops to register very late suspend and very early resume callbacks for the whole pinctrl-samsung driver and a global list of registered pin
Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality
Tomasz, On Thu, May 16, 2013 at 2:27 PM, Tomasz Figa tomasz.f...@gmail.com wrote: OK. I will be fine to go with your patches, after addressing the comments. In the end it's good that you posted them, as reviewing them allowed me to find even better ways of doing some things than I had in mine ;) . Yes. I often find that the best way to review code is to think about how I would implement it myself. Certainly I think we've ended up with something better / less buggy this way. ;) How all of this works is basically a good question. I couldn't find any mention about pins switching from power down to normal mode in the documentation, but maybe there is a small side note somewhere, which I could miss. On S3C6410, for example, there are two modes. State is switched to power down mode automatically, but can be switched out either automatically on wake-up (exact timing is unknown to me) or by clearing a special bit, depending on value of other special bit. IMHO this is rather important, so we should find out how it work on other SoCs and make the code account for it. Agreed that it's important. ...but it's also good not to have tons of complexity when it's not needed. It sounds like S3C6410 could be handled OK by just using the special bits and waiting to take things out of power down mode. ...thinking about it, all SoCs that have power down modes (which you _must_ have if your pinctrl state is lost across a low power) would be slightly broken if they didn't have a bit to switch out of power down mode. Otherwise you're asking for at least some type of glitch because you'll end up in the default state of pins for a little while during resume. That's not to say that there aren't broken SoCs out there and it's entirely possible that people even designed systems around them (knowing that the default state of each pin after wakeup is not harmful to whatever is connected to that pin). If there are any cases like this then they would need the special code like my V1 patch had. Do you know of any SoCs like this that we need to support on kernel 3.10 and higher? I'm planning on going back to the simpler code for my next patchset unless I can find a problem with it. -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pinctrl: exynos: fix eint wakeup by using irq_set_wake()
Tomasz, On Thu, May 16, 2013 at 12:26 PM, Tomasz Figa tomasz.f...@gmail.com wrote: On Thursday 16 of May 2013 10:12:32 Doug Anderson wrote: From: Prathyush K prathyus...@samsung.com Add the irq_set_wake function for exynos pinctrl to configure the external interrupt wakeup mask register. [dianders: minor nit fixes; port to ToT] Signed-off-by: Prathyush K prathyus...@samsung.com Signed-off-by: Doug Anderson diand...@chromium.org --- drivers/pinctrl/pinctrl-exynos.c | 45 --- drivers/pinctrl/pinctrl-exynos.h | 3 ++- drivers/pinctrl/pinctrl-samsung.h | 2 ++ 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c index ac74281..3ebb2ff 100644 --- a/drivers/pinctrl/pinctrl-exynos.c +++ b/drivers/pinctrl/pinctrl-exynos.c @@ -30,6 +30,8 @@ #include linux/spinlock.h #include linux/err.h +#include plat/pm.h + This is not going to work with CONFIG_MULTIPLATFORM. Hmm, this sounds like it might be a bit of a long path, especially since I haven't been keeping up with what's been going on with MULTIPLATFORM and I'm currently midway through making 3.8 work (which has no MULTIPLATFORM). Perhaps for this patch it makes more sense for you to post your version and I can review it? We may end up just keeping our version of this patch for 3.8 and pick up yours when we do our next rebase. Does that sound OK? -#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs) \ +#define EXYNOS_PIN_BANK_EINTW(pins, reg, id, offs, base)\ { \ .type = bank_type_alive, \ .pctl_offset= reg, \ .nr_pins= pins, \ .eint_type = EINT_TYPE_WKUP, \ .eint_offset= offs, \ + .eint_base = base, \ I can't look at my patch at the moment, but I think I have managed to get EINT index without adding this extra field. It looks like this is always 2 * eint_offset in the code above. Maybe you just multiplied? The multiplication works fine although I think specifying eint_base like this might be more generic and handle future chips better? Ya never know... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] pinctrl: samsung: fix suspend/resume functionality
Tomasz, On Thu, May 16, 2013 at 3:56 PM, Tomasz Figa tomasz.f...@gmail.com wrote: Seems like I need some sleep, as I'm already starting to overlook large blobs of code. Originally, GPIO suspend/resume handlers have been configured in drivers/gpio/gpio-samsung.c, by setting pm field of samsung_gpio_chip struct to point to appropriate samsung_gpio_pm struct, which contains pointers to save and resume callbacks. In result, samsung_gpio_pm_2bit_* or samsung_gpio_pm_4bit_* have been used, depending on bank type, on all SoCs. Now since the documentation states that wake-up reset doesn't reset GPIO registers (at least on S3C2440 and S3C2416), I wonder what is the correct way of handling them. If state of these registers isn't lost on those SoCs then running the save/restore shouldn't _hurt_ though, right? If you can run the old GPIO code on one of those systems and do a suspend/resume you could check... -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] pinctrl: samsung: fix suspend/resume functionality
The GPIO states need to be restored after s2r and this is not currently supported in the pinctrl driver. This patch saves the gpio states before suspend and restores them after resume. Saving and restoring is done very early using syscore_ops and must happen before pins are released from their powerdown state. Patch originally from Prathyush K prathyus...@samsung.com but rewritten by Doug Anderson diand...@chromium.org. Signed-off-by: Prathyush K prathyus...@samsung.com Signed-off-by: Doug Anderson diand...@chromium.org --- Changes in v2: - Now uses sycore_ops to make sure we're early enough. - Try to handle two CON registers better. - Should handle s3c24xx better as per Heiko. - Simpler code; no longer tries to avoid glitching lines since we _think_ all current SoCs should have pins in power down state when the restore is called. - Dropped eint patch for now; Tomasz will post his version. drivers/pinctrl/pinctrl-samsung.c | 140 ++ drivers/pinctrl/pinctrl-samsung.h | 5 ++ 2 files changed, 145 insertions(+) diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c index 9763668..851eada 100644 --- a/drivers/pinctrl/pinctrl-samsung.c +++ b/drivers/pinctrl/pinctrl-samsung.c @@ -28,6 +28,7 @@ #include linux/gpio.h #include linux/irqdomain.h #include linux/spinlock.h +#include linux/syscore_ops.h #include core.h #include pinctrl-samsung.h @@ -48,6 +49,9 @@ static struct pin_config { { samsung,pin-pud-pdn, PINCFG_TYPE_PUD_PDN }, }; +/* Global list of devices (struct samsung_pinctrl_drv_data) */ +LIST_HEAD(drvdata_list); + static unsigned int pin_base; static inline struct samsung_pin_bank *gc_to_pin_bank(struct gpio_chip *gc) @@ -961,9 +965,137 @@ static int samsung_pinctrl_probe(struct platform_device *pdev) ctrl-eint_wkup_init(drvdata); platform_set_drvdata(pdev, drvdata); + + /* Add to the global list */ + list_add_tail(drvdata-node, drvdata_list); + return 0; } +#ifdef CONFIG_PM + +/** + * samsung_pinctrl_suspend_dev - save pinctrl state for suspend for a device + * + * Save data for all banks handled by this device. + */ +static void samsung_pinctrl_suspend_dev( + struct samsung_pinctrl_drv_data *drvdata) +{ + struct samsung_pin_ctrl *ctrl = drvdata-ctrl; + void __iomem *virt_base = drvdata-virt_base; + int i; + + for (i = 0; i ctrl-nr_banks; i++) { + struct samsung_pin_bank *bank = ctrl-pin_banks[i]; + void __iomem *reg = virt_base + bank-pctl_offset; + + u8 *offs = bank-type-reg_offset; + u8 *widths = bank-type-fld_width; + enum pincfg_type type; + + for (type = 0; type PINCFG_TYPE_NUM; type++) + if (widths[type]) + bank-pm_save[type] = readl(reg + offs[type]); + + if (widths[PINCFG_TYPE_FUNC] * bank-nr_pins 32) { + /* Some banks have two config registers */ + bank-pm_save[PINCFG_TYPE_NUM] = + readl(reg + offs[PINCFG_TYPE_FUNC] + 4); + pr_debug(Save %s @ %p (con %#010x %08x)\n, +bank-name, reg, +bank-pm_save[PINCFG_TYPE_FUNC], +bank-pm_save[PINCFG_TYPE_NUM]); + } else { + pr_debug(Save %s @ %p (con %#010x)\n, bank-name, +reg, bank-pm_save[PINCFG_TYPE_FUNC]); + } + } +} + +/** + * samsung_pinctrl_resume_dev - restore pinctrl state from suspend for a device + * + * Restore one of the banks that was saved during suspend. + * + * We don't bother doing anything complicated to avoid glitching lines since + * we're called before pad retention is turned off. + */ +static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data *drvdata) +{ + struct samsung_pin_ctrl *ctrl = drvdata-ctrl; + void __iomem *virt_base = drvdata-virt_base; + int i; + + for (i = 0; i ctrl-nr_banks; i++) { + struct samsung_pin_bank *bank = ctrl-pin_banks[i]; + void __iomem *reg = virt_base + bank-pctl_offset; + + u8 *offs = bank-type-reg_offset; + u8 *widths = bank-type-fld_width; + enum pincfg_type type; + + if (widths[PINCFG_TYPE_FUNC] * bank-nr_pins 32) { + /* Some banks have two config registers */ + pr_debug(%s @ %p (con %#010x %08x = %#010x %08x)\n, +bank-name, reg, +readl(reg + offs[PINCFG_TYPE_FUNC]), +readl(reg + offs[PINCFG_TYPE_FUNC] + 4), +bank-pm_save[PINCFG_TYPE_FUNC], +bank-pm_save[PINCFG_TYPE_NUM
Re: [PATCH 2/2] pinctrl: exynos: fix eint wakeup by using irq_set_wake()
Tomasz, On Thu, May 16, 2013 at 3:37 PM, Tomasz Figa tomasz.f...@gmail.com wrote: Well, to make long story short, including headers from plat/ and mach/ from files outside plat/ or mach/ is no longer valid with CONFIG_MULTIPLATFORM, because more than one plat and/or mach can be enabled at the same time. In addition to this, care must be taken for code to not break platforms other than written for, when compiled into the resulting kernel. Right. That makes sense. That's also why it would be a bit of a longshot for me to get this done right now. I'd imagine that there would be a number of changes to the samsung pm infrastructure that are needed to make this work and I don't have all of those in my tree right now. We've already picked back a lot to 3.8, but multiplatform seems too much. Perhaps for this patch it makes more sense for you to post your version and I can review it? We may end up just keeping our version of this patch for 3.8 and pick up yours when we do our next rebase. Does that sound OK? Fine. I will also send a patch adding save and restore for several EINT registers that need it. OK, sounds good. I was trying to figure out why we didn't seem to have those in our 3.4 stuff and that it seems to work without saving/restoring. I assumed that maybe higher level code was masking/unmasking interrupts but didn't dig. Since EINT handling is highly SoC-specific (i.e. done in pinctrl-exynos, not pinctrl-samsung), such assumption wouldn't be a problem. Let me see how I solved this problem in my version tomorrow at work. Fair enough. :) Looking forward to seeing your patch! -Doug -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html