Re: [PATCH] mmc: dt: Add 'broken-cd' DT binding

2012-08-22 Thread Shawn Guo
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

2012-08-22 Thread Kukjin Kim
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

2012-08-22 Thread Kukjin Kim
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

2012-08-22 Thread Chris Ball
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

2012-08-22 Thread Thomas Abraham
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

2012-08-22 Thread Chris Ball
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

2012-08-22 Thread Mark Brown
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

2012-08-22 Thread Julia Lawall
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

2012-08-22 Thread Thomas Abraham
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

2012-08-22 Thread Shawn Guo
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

2012-08-22 Thread Chris Ball
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

2012-08-22 Thread Chris Ball
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

2012-08-22 Thread Stephen Warren
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

2012-08-22 Thread Chris Ball
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

2012-08-22 Thread Thomas Abraham
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

2012-08-22 Thread Mark Brown
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

2012-08-22 Thread Mitch Bradley
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.

2012-08-22 Thread Chris Ball
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

2012-08-22 Thread Kukjin Kim
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