Re: [PATCH] mmc: dt: Add 'broken-cd' DT binding
On Tue, Aug 21, 2012 at 10:48:43AM -0400, Chris Ball wrote: Aside: the bindings do not match the code. The bindings document says to use fsl,cd-internal, and imx51-babbage.dts does so -- but the code doesn't check for fsl,cd-internal, it checks for fsl,cd-controller: if (of_get_property(np, fsl,cd-controller, NULL)) boarddata-cd_type = ESDHC_CD_CONTROLLER; The same confusion is present for fsl,wp-{controller,internal}. Thanks for spotting it. I will submit a patch to fix binding doc and dts. Ignoring that, these bindings are similar, but not the same -- imx-esdhc only allows one of the cd_type cases to be true, so you either have cd-internal or you have cd-gpios: if (of_get_property(np, fsl,cd-controller, NULL)) boarddata-cd_type = ESDHC_CD_CONTROLLER; boarddata-cd_gpio = of_get_named_gpio(np, cd-gpios, 0); if (gpio_is_valid(boarddata-cd_gpio)) boarddata-cd_type = ESDHC_CD_GPIO; This differs from Thomas's binding -- he wants a way to say the cd-gpio mentioned in cd-gpios is [internal/external] to the card's CD pin. Rob Herring said: This makes the most sense to me. However, I prefer broken-cd over cd-internal. The binding should add properties for exceptions, not SDHCI spec compliant implementations. Agreed, I was going to say the same thing. Putting it all together, it sounds like we want: no extra properties: the CD pin on the host just works. broken-cd:the CD pin on the host is broken; use polling. cd-gpios: the GPIO listed is the CD pin on the host being brought out directly to a GPIO. cd-external: when used with cd-gpios, specifies that the GPIO in cd-gpios is external to the CD pin on the host. cd-gpios and cd-external can be present on the same node. if broken-cd is present, it must be the only one of these nodes used. Shawn, I guess I'll leave it up to you on whether/when you'd like to move away from the fsl, properties to the new common ones? Ok, I will move to the common one once it appears on your tree. -- Regards, Shawn -- 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 14/14] drivers/spi/spi-s3c24xx.c: fix error return code
Julia Lawall wrote: From: Julia Lawall julia.law...@lip6.fr Initialize return variable before exiting on an error path. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // smpl ( if@p1 (\(ret 0\|ret != 0\)) { ... return ret; } | ret@p1 = 0 ) ... when != ret = e1 when != ret *if(...) { ... when != ret = e2 when forall return ret; } // /smpl Signed-off-by: Julia Lawall julia.law...@lip6.fr Acked-by: Kukjin Kim kgene@samsung.com (Cc'ed Mark Brown who is handling spi for a moment) --- Perhaps -EINVAL is not the right value in this case. Well, if (pin_cs 0) is true, it is really invalid value. So I think, it makes sense :-) Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. drivers/spi/spi-s3c24xx.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/spi-s3c24xx.c b/drivers/spi/spi-s3c24xx.c index 8ee7d79..a2a080b 100644 --- a/drivers/spi/spi-s3c24xx.c +++ b/drivers/spi/spi-s3c24xx.c @@ -611,6 +611,7 @@ static int __devinit s3c24xx_spi_probe(struct platform_device *pdev) if (!pdata-set_cs) { if (pdata-pin_cs 0) { dev_err(pdev-dev, No chipselect pin\n); + err = -EINVAL; goto err_register; } -- 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: SAMSUNG: use spin_lock_irqsave() in clk_set_parent
Mandeep Singh Baines wrote: From 0cdf3aff, ARM: SAMSUNG: use spin_lock_irqsave() in clk_{enable,disable}: The clk_enable()and clk_disable() can be used process and ISR either. And actually it is used for real product and other platforms use it now. So spin_lock_irqsave() should be used instead. We need to make a similar change in clk_set_parent(). Otherwise, you can potentially get spinlock recursion: BUG: spinlock recursion on CPU#0, kinteractive/68 lock: 807832a8, .magic: dead4ead, .owner: kinteractive/68, .owner_cpu: 0 [80015f54] (unwind_backtrace+0x0/0x128) from [804f2914] (dump_stack+0x20/0x24) [804f2914] (dump_stack+0x20/0x24) from [804f57b8] (spin_dump+0x80/0x94) [804f57b8] (spin_dump+0x80/0x94) from [804f57f8] (spin_bug+0x2c/0x30) [804f57f8] (spin_bug+0x2c/0x30) from [80222730] (do_raw_spin_lock+0x54/0x150) [80222730] (do_raw_spin_lock+0x54/0x150) from [804f96ec] (_raw_spin_lock_irqsave+0x20/0x28) [804f96ec] (_raw_spin_lock_irqsave+0x20/0x28) from [80022ea4] (clk_enable+0x3c/0x84) [80022ea4] (clk_enable+0x3c/0x84) from [8038336c] (s5p_mfc_clock_on+0x60/0x74) [8038336c] (s5p_mfc_clock_on+0x60/0x74) from [8038645c] (s5p_mfc_read_info+0x20/0x38) [8038645c] (s5p_mfc_read_info+0x20/0x38) from [8037ca3c] (s5p_mfc_handle_frame+0x2e4/0x4bc) [8037ca3c] (s5p_mfc_handle_frame+0x2e4/0x4bc) from [8037d420] (s5p_mfc_irq+0x1ec/0x6cc) [8037d420] (s5p_mfc_irq+0x1ec/0x6cc) from [8007fc74] (handle_irq_event_percpu+0x8c/0x244) [8007fc74] (handle_irq_event_percpu+0x8c/0x244) from [8007fe78] (handle_irq_event+0x4c/0x6c) [8007fe78] (handle_irq_event+0x4c/0x6c) from [80082dd8] (handle_fasteoi_irq+0xe4/0x150) [80082dd8] (handle_fasteoi_irq+0xe4/0x150) from [8007f424] (generic_handle_irq+0x3c/0x50) [8007f424] (generic_handle_irq+0x3c/0x50) from [8000f7c4] (handle_IRQ+0x88/0xc8) [8000f7c4] (handle_IRQ+0x88/0xc8) from [80008564] (gic_handle_irq+0x44/0x68) [80008564] (gic_handle_irq+0x44/0x68) from [8000e400] (__irq_svc+0x40/0x60) Exception stack(0xef3cbe68 to 0xef3cbeb0) [8000e400] (__irq_svc+0x40/0x60) from [80022cfc] (clk_set_parent+0x30/0x74) [80022cfc] (clk_set_parent+0x30/0x74) from [803ac7f8] (set_apll.isra.0+0x28/0xb0) [803ac7f8] (set_apll.isra.0+0x28/0xb0) from [803ac8e4] (exynos5250_set_frequency+0x64/0xb8) [803ac8e4] (exynos5250_set_frequency+0x64/0xb8) from [803ac280] (exynos_target+0x1b0/0x220) [803ac280] (exynos_target+0x1b0/0x220) from [803a4a0c] (__cpufreq_driver_target+0xb0/0xd4) [803a4a0c] (__cpufreq_driver_target+0xb0/0xd4) from [803aab80] (cpufreq_interactive_updown_task+0x214/0x264) [803aab80] (cpufreq_interactive_updown_task+0x214/0x264) from [80047d04] (kthread+0x9c/0xa8) [80047d04] (kthread+0x9c/0xa8) from [8000fa48] (kernel_thread_exit+0x0/0x8) Signed-off-by: Mandeep Singh Baines m...@chromium.org Suggested-by: Sunil Mazhavanchery sun...@samsung.com CC: linux-arm-ker...@lists.infradead.org CC: linux-samsung-soc@vger.kernel.org CC: Ben Dooks ben-li...@fluff.org CC: Kukjin Kim kgene@samsung.com CC: Russell King li...@arm.linux.org.uk CC: Minho Ban mh...@samsung.com CC: Jaecheol Lee jc@samsung.com CC: Sunyoung Kang sy0816.k...@samsung.com CC: Kukjin Kim kgene@samsung.com CC: Olof Johansson ol...@chromium.org --- arch/arm/plat-samsung/clock.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/arm/plat-samsung/clock.c b/arch/arm/plat-samsung/clock.c index 65c5eca..80eacca 100644 --- a/arch/arm/plat-samsung/clock.c +++ b/arch/arm/plat-samsung/clock.c @@ -173,17 +173,18 @@ struct clk *clk_get_parent(struct clk *clk) int clk_set_parent(struct clk *clk, struct clk *parent) { + unsigned long flags; int ret = 0; if (IS_ERR(clk)) return -EINVAL; - spin_lock(clocks_lock); + spin_lock_irqsave(clocks_lock, flags); if (clk-ops clk-ops-set_parent) ret = (clk-ops-set_parent)(clk, parent); - spin_unlock(clocks_lock); + spin_unlock_irqrestore(clocks_lock, flags); return ret; } -- 1.7.7.3 Yup, right. Applied. Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- 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] mmc: dt: Add 'broken-cd' DT binding
Hi, On Wed, Aug 22 2012, Shawn Guo wrote: The following is what I have on my mind. broken-cd cd-gpiosimplication --- nono SDHCI CD noyes GPIO CD yes no NO CD / Broken CD yes yes Invalid yes: property presents no: property does not present This matches Mitch's last suggestion exactly -- I think we're all agreed on these properties now. The only remaining question is how to handle the pinctrl for CD in Thomas's case. Thanks! - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- 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] mmc: dt: Add 'broken-cd' DT binding
On 22 August 2012 15:47, Chris Ball c...@laptop.org wrote: Hi, On Wed, Aug 22 2012, Shawn Guo wrote: The following is what I have on my mind. broken-cd cd-gpiosimplication --- nono SDHCI CD noyes GPIO CD yes no NO CD / Broken CD yes yes Invalid yes: property presents no: property does not present This matches Mitch's last suggestion exactly -- I think we're all agreed on these properties now. The only remaining question is how to handle the pinctrl for CD in Thomas's case. Hi Chris, For sdhci-s3c driver, the 'broken-cd' and 'cd-gpios' bindings are sufficient. But, are drivers free to use implementation specific behavior when using these bindings. Or do we strictly adhere to the table which Shawn has listed. For sdhci-s3c driver, I would like to deviate from the implication column listed above. On second thoughts, I now understand that gpio's terminate at the gpio controller, not at the card-detect pad of the mmc host controller. Those that terminate at the mmc controller pads are actually mux functions. I do agree that the long term solution for sdhci-s3c driver is to use the pinctrl driver. If sdhci-s3c has to strictly adhere to the Shawn's table above, then I prefer to postpone the device tree support in sdhci-s3c driver after having proper support for Samsung pinctrl driver. But then, sdhci-s3c driver is used on multiple Samsung SoC's, all of which might not get pinctrl driver support anytime soon. Hence, for sdhci-s3c driver, 'broken-cd' and 'cd-gpios' bindings are sufficient, but sdhci-s3c driver should be free to use these bindings in a implementation specific way. Thanks, Thomas. -- 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] mmc: dt: Add 'broken-cd' DT binding
Hi Thomas, On Wed, Aug 22 2012, Thomas Abraham wrote: This matches Mitch's last suggestion exactly -- I think we're all agreed on these properties now. The only remaining question is how to handle the pinctrl for CD in Thomas's case. Hi Chris, For sdhci-s3c driver, the 'broken-cd' and 'cd-gpios' bindings are sufficient. But, are drivers free to use implementation specific behavior when using these bindings. Or do we strictly adhere to the table which Shawn has listed. For sdhci-s3c driver, I would like to deviate from the implication column listed above. I'd rather you use the implications described above for the names described above; there's not much point in defining named behaviors across the subsystem if they're actually different in each driver. But I think you can still use driver-internal properties that change the interpretation of those names in a documented way. If the meaning of cd-gpios is modified when coupled with your samsung,sdhci-cd-external, that's okay with me. That should cover all the cases you need until you migrate to using pinctrl, right? So, you could immediately support: none - currently samsung,sdhci-cd-internal broken-cd - currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-external - currently samsung,sdhci-cd-external How does that sound? Thanks, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- 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 14/14] drivers/spi/spi-s3c24xx.c: fix error return code
On Wed, Aug 22, 2012 at 06:38:36PM +0900, Kukjin Kim wrote: Julia Lawall wrote: Signed-off-by: Julia Lawall julia.law...@lip6.fr Acked-by: Kukjin Kim kgene@samsung.com (Cc'ed Mark Brown who is handling spi for a moment) Please resend the patch to me so I can apply this. signature.asc Description: Digital signature
Re: [PATCH 14/14] drivers/spi/spi-s3c24xx.c: fix error return code
From: Julia Lawall julia.law...@lip6.fr Initialize return variable before exiting on an error path. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // smpl ( if@p1 (\(ret 0\|ret != 0\)) { ... return ret; } | ret@p1 = 0 ) ... when != ret = e1 when != ret *if(...) { ... when != ret = e2 when forall return ret; } // /smpl Signed-off-by: Julia Lawall julia.law...@lip6.fr --- Perhaps -EINVAL is not the right value in this case. drivers/spi/spi-s3c24xx.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/spi-s3c24xx.c b/drivers/spi/spi-s3c24xx.c index 8ee7d79..a2a080b 100644 --- a/drivers/spi/spi-s3c24xx.c +++ b/drivers/spi/spi-s3c24xx.c @@ -611,6 +611,7 @@ static int __devinit s3c24xx_spi_probe(struct platform_device *pdev) if (!pdata-set_cs) { if (pdata-pin_cs 0) { dev_err(pdev-dev, No chipselect pin\n); + err = -EINVAL; goto err_register; } -- 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] mmc: dt: Add 'broken-cd' DT binding
On 22 August 2012 16:38, Chris Ball c...@laptop.org wrote: Hi Thomas, On Wed, Aug 22 2012, Thomas Abraham wrote: This matches Mitch's last suggestion exactly -- I think we're all agreed on these properties now. The only remaining question is how to handle the pinctrl for CD in Thomas's case. Hi Chris, For sdhci-s3c driver, the 'broken-cd' and 'cd-gpios' bindings are sufficient. But, are drivers free to use implementation specific behavior when using these bindings. Or do we strictly adhere to the table which Shawn has listed. For sdhci-s3c driver, I would like to deviate from the implication column listed above. I'd rather you use the implications described above for the names described above; there's not much point in defining named behaviors across the subsystem if they're actually different in each driver. Ok. But I think you can still use driver-internal properties that change the interpretation of those names in a documented way. If the meaning of cd-gpios is modified when coupled with your samsung,sdhci-cd-external, that's okay with me. That should cover all the cases you need until you migrate to using pinctrl, right? So, you could immediately support: none - currently samsung,sdhci-cd-internal broken-cd - currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-external - currently samsung,sdhci-cd-external How does that sound? Not quite there yet. It is not possible to leave out cd-gpios for samsung,sdhci-cd-internal since the current Samsung pinmux configuration piggybacks on gpio dt binding. Can we use the following instead. none - currently samsung,sdhci-cd-none broken-cd- currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-internal - currently samsung,sdhci-cd-internal Thanks Chris. Regards, Thomas. -- 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] mmc: dt: Add 'broken-cd' DT binding
Hi Chris, On Tue, Aug 21, 2012 at 10:48:43AM -0400, Chris Ball wrote: Aside: the bindings do not match the code. The bindings document says to use fsl,cd-internal, and imx51-babbage.dts does so -- but the code doesn't check for fsl,cd-internal, it checks for fsl,cd-controller: if (of_get_property(np, fsl,cd-controller, NULL)) boarddata-cd_type = ESDHC_CD_CONTROLLER; The same confusion is present for fsl,wp-{controller,internal}. I'm trying to fix the mismatch between dts and code, but seeing that the controller mode has been broken for quite a while. I just ran a bisect and was told as below. 30832ab56c80d96cfaf5a786524f0d8c57fadfa1 is the first bad commit commit 30832ab56c80d96cfaf5a786524f0d8c57fadfa1 Author: Todd Poynor toddpoy...@google.com Date: Tue Dec 27 15:48:46 2011 +0200 mmc: sdhci: Always pass clock request value zero to set_clock host op To allow the set_clock host op to disable the SDCLK source when not needed, always call the host op when the requested clock speed is zero. Do this even if host-clock already equals zero, because the SDHCI driver may set that value (without calling the host op) to force an update at the next (non-zero-speed) call. Signed-off-by: Todd Poynor toddpoy...@google.com Acked-by: Adrian Hunter adrian.hun...@intel.com Signed-off-by: Chris Ball c...@laptop.org Reverting the commit will get controller mode back to work. -- Regards, Shawn -- 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] mmc: dt: Add 'broken-cd' DT binding
Hi Shawn, On Wed, Aug 22 2012, Shawn Guo wrote: mmc: sdhci: Always pass clock request value zero to set_clock host op To allow the set_clock host op to disable the SDCLK source when not needed, always call the host op when the requested clock speed is zero. Do this even if host-clock already equals zero, because the SDHCI driver may set that value (without calling the host op) to force an update at the next (non-zero-speed) call. Signed-off-by: Todd Poynor toddpoy...@google.com Acked-by: Adrian Hunter adrian.hun...@intel.com Signed-off-by: Chris Ball c...@laptop.org Reverting the commit will get controller mode back to work. Weird; I wonder how that's related to card detection. I'm afraid it's ultimately a driver bug -- esdhc should be able to handle having set_clock called with a value of 0. It seems to have a case in esdhc_set_clock() for 0, so I'm surprised it's not working. What's the symptom of the failure? Can you debug some more? Thanks, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- 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] mmc: dt: Add 'broken-cd' DT binding
Hi Thomas, On Wed, Aug 22 2012, Thomas Abraham wrote: none - currently samsung,sdhci-cd-internal broken-cd - currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-external - currently samsung,sdhci-cd-external How does that sound? Not quite there yet. It is not possible to leave out cd-gpios for samsung,sdhci-cd-internal since the current Samsung pinmux configuration piggybacks on gpio dt binding. Can we use the following instead. none - currently samsung,sdhci-cd-none broken-cd- currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-internal - currently samsung,sdhci-cd-internal I see. Okay; unless anyone has better ideas, it sounds like you should go ahead and do that -- with something like this in your binding docs: This binding differs from the core MMC binding by requiring a cd-gpios property to be present to use internal card-detection. If no cd-gpios are present (and non-removable is missing) the driver will poll for card-detection as if a broken-cd property was provided. I'll send out the new core bindings doc shortly. Finishing off another thread -- the samsung-sdhci.txt name is okay. Thanks, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- 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] mmc: dt: Add 'broken-cd' DT binding
On 08/22/2012 04:17 AM, Chris Ball wrote: Hi, On Wed, Aug 22 2012, Shawn Guo wrote: The following is what I have on my mind. broken-cdcd-gpiosimplication --- no no SDHCI CD no yes GPIO CD yes no NO CD / Broken CD yes yes Invalid yes: property presents no: property does not present This matches Mitch's last suggestion exactly -- I think we're all agreed on these properties now. The only remaining question is how to handle the pinctrl for CD in Thomas's case. Sorry if this has already been mentioned, but I assume the standard non-removable property overrides any of those, or is invalid in combination with any of those? -- 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] mmc: dt: Add 'broken-cd' DT binding
Hi Stephen, On Wed, Aug 22 2012, Stephen Warren wrote: On 08/22/2012 04:17 AM, Chris Ball wrote: Hi, On Wed, Aug 22 2012, Shawn Guo wrote: The following is what I have on my mind. broken-cd cd-gpiosimplication --- no no SDHCI CD no yes GPIO CD yes no NO CD / Broken CD yes yes Invalid yes: property presents no: property does not present This matches Mitch's last suggestion exactly -- I think we're all agreed on these properties now. The only remaining question is how to handle the pinctrl for CD in Thomas's case. Sorry if this has already been mentioned, but I assume the standard non-removable property overrides any of those, or is invalid in combination with any of those? Yes, absolutely. I'll make sure to explain that in the new version of the document. (I think the reason it hasn't been mentioned much in this thread is that it's already part of the core bindings.) Thanks, - Chris. -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- 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] mmc: dt: Add 'broken-cd' DT binding
On 22 August 2012 20:24, Chris Ball c...@laptop.org wrote: Hi Thomas, On Wed, Aug 22 2012, Thomas Abraham wrote: none - currently samsung,sdhci-cd-internal broken-cd - currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-external - currently samsung,sdhci-cd-external How does that sound? Not quite there yet. It is not possible to leave out cd-gpios for samsung,sdhci-cd-internal since the current Samsung pinmux configuration piggybacks on gpio dt binding. Can we use the following instead. none - currently samsung,sdhci-cd-none broken-cd- currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-internal - currently samsung,sdhci-cd-internal I see. Okay; unless anyone has better ideas, it sounds like you should go ahead and do that -- with something like this in your binding docs: Ok. This binding differs from the core MMC binding by requiring a cd-gpios property to be present to use internal card-detection. If no cd-gpios are present (and non-removable is missing) the driver will poll for card-detection as if a broken-cd property was provided. I'll send out the new core bindings doc shortly. Finishing off another thread -- the samsung-sdhci.txt name is okay. Thanks Chris for your time on this subject. I will prepare the sdhci-s3c dt support patch on top of your new core bindings patch. Thanks, Thomas. -- 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 14/14] drivers/spi/spi-s3c24xx.c: fix error return code
On Wed, Aug 22, 2012 at 01:42:47PM +0200, Julia Lawall wrote: From: Julia Lawall julia.law...@lip6.fr Initialize return variable before exiting on an error path. Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH] mmc: dt: Add 'broken-cd' DT binding
Hi Thomas, On 8/22/2012 2:04 AM, Thomas Abraham wrote: On 22 August 2012 16:38, Chris Ball c...@laptop.org wrote: Hi Thomas, On Wed, Aug 22 2012, Thomas Abraham wrote: This matches Mitch's last suggestion exactly -- I think we're all agreed on these properties now. The only remaining question is how to handle the pinctrl for CD in Thomas's case. Hi Chris, For sdhci-s3c driver, the 'broken-cd' and 'cd-gpios' bindings are sufficient. But, are drivers free to use implementation specific behavior when using these bindings. Or do we strictly adhere to the table which Shawn has listed. For sdhci-s3c driver, I would like to deviate from the implication column listed above. I'd rather you use the implications described above for the names described above; there's not much point in defining named behaviors across the subsystem if they're actually different in each driver. Ok. But I think you can still use driver-internal properties that change the interpretation of those names in a documented way. If the meaning of cd-gpios is modified when coupled with your samsung,sdhci-cd-external, that's okay with me. That should cover all the cases you need until you migrate to using pinctrl, right? So, you could immediately support: none - currently samsung,sdhci-cd-internal broken-cd - currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-external - currently samsung,sdhci-cd-external How does that sound? Not quite there yet. It is not possible to leave out cd-gpios for samsung,sdhci-cd-internal since the current Samsung pinmux configuration piggybacks on gpio dt binding. Sorry to interject on a topic that seems to have already been decided, but I'm confused by one thing and would like clarification. I understand that you need to use a GPIO-style specifier as a surrogate for a pinmux specification - that much is clear. What is not clear is why it's necessary to (ab)use the name cd-gpios for it. Why not use a different property name, e.g. samsung,cd-pinmux-gpio = gpio-specifier for the cd-gpios + samsung,sdhci-cd-internal case? Then both samsung,sdhci-cdi-internal and samsung,sdhci-cd-external could go away. There would only be one system-dependent property samsung,cd-pinmux-gpio whose name would make it clear that it's conflating pinmuxing and gpios. I think the scheme I propose would be clearer, less likely to confuse other people who try to use the driver as a model, require less hand-waving in the documentation, and easier to change to a proper pinmuxing scheme should that become available later. Can we use the following instead. none - currently samsung,sdhci-cd-none broken-cd- currently samsung,sdhci-cd-none cd-gpios - currently samsung,sdhci-cd-gpios non-removable - currently samsung,sdhci-cd-permanent cd-gpios + samsung,sdhci-cd-internal - currently samsung,sdhci-cd-internal Thanks Chris. Regards, Thomas. ___ devicetree-discuss mailing list devicetree-disc...@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss -- 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] mmc: dt: Add card-detection properties to core binding.
Signed-off-by: Chris Ball c...@laptop.org --- Documentation/devicetree/bindings/mmc/mmc.txt | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt index 8a6811f..8e2e0ba 100644 --- a/Documentation/devicetree/bindings/mmc/mmc.txt +++ b/Documentation/devicetree/bindings/mmc/mmc.txt @@ -9,12 +9,17 @@ Interpreted by the OF core: Required properties: - bus-width: Number of data lines, can be 1, 4, or 8 +Card detection: +If no property below is supplied, standard SDHCI card detect is used. +Only one of the properties in this section should be supplied: + - broken-cd: There is no card detection available; polling must be used. + - cd-gpios: Specify GPIOs for card detection, see gpio binding + - non-removable: non-removable slot (like eMMC); assume always present. + Optional properties: -- cd-gpios: Specify GPIOs for card detection, see gpio binding - wp-gpios: Specify GPIOs for write protection, see gpio binding - cd-inverted: when present, polarity on the cd gpio line is inverted - wp-inverted: when present, polarity on the wp gpio line is inverted -- non-removable: non-removable slot (like eMMC) - max-frequency: maximum operating clock frequency Example: -- Chris Ball c...@laptop.org http://printf.net/ One Laptop Per Child -- 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] spi/s3c64xx: Drop extra calls to spi_master_get in suspend/remove functions
Guenter Roeck wrote: Suspend and resume functions call spi_master_get() without matching spi_master_put(). The extra references are unnecessary and cause subsequent module unload attempts to fail. Drop the calls. Signed-off-by: Guenter Roeck li...@roeck-us.net Acked-by: Kukjin Kim kgene@samsung.com (Cc'ed Mark Brown who is handling spi now) Guenter, maybe you need to re-send this patch to Mark so that he can apply ;-) Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. --- drivers/spi/spi-s3c64xx.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 646a765..d7a87df 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -1409,7 +1409,7 @@ static int s3c64xx_spi_remove(struct platform_device *pdev) #ifdef CONFIG_PM static int s3c64xx_spi_suspend(struct device *dev) { - struct spi_master *master = spi_master_get(dev_get_drvdata(dev)); + struct spi_master *master = dev_get_drvdata(dev); struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master); spi_master_suspend(master); @@ -1428,7 +1428,7 @@ static int s3c64xx_spi_suspend(struct device *dev) static int s3c64xx_spi_resume(struct device *dev) { - struct spi_master *master = spi_master_get(dev_get_drvdata(dev)); + struct spi_master *master = dev_get_drvdata(dev); struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master); struct s3c64xx_spi_info *sci = sdd-cntrlr_info; @@ -1452,7 +1452,7 @@ static int s3c64xx_spi_resume(struct device *dev) #ifdef CONFIG_PM_RUNTIME static int s3c64xx_spi_runtime_suspend(struct device *dev) { - struct spi_master *master = spi_master_get(dev_get_drvdata(dev)); + struct spi_master *master = dev_get_drvdata(dev); struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master); clk_disable(sdd-clk); @@ -1463,7 +1463,7 @@ static int s3c64xx_spi_runtime_suspend(struct device *dev) static int s3c64xx_spi_runtime_resume(struct device *dev) { - struct spi_master *master = spi_master_get(dev_get_drvdata(dev)); + struct spi_master *master = dev_get_drvdata(dev); struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master); clk_enable(sdd-src_clk); -- 1.7.9.7 -- 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