Re: [RFC PATCH 0/3] Fix power domains handling on exynos542x

2015-03-10 Thread Javier Martinez Canillas
Hello Kukjin,

On 02/05/2015 03:45 PM, Javier Martinez Canillas wrote:
 Hello Andrzej,
 
 Thanks a lot for finally finding what was causing the HDMI issue.
 
 On 02/05/2015 01:35 PM, Andrzej Hajda wrote:
 Hi,
 
 Exynos chipsets since 542x have asynchronous bridges connecting different 
 IPs.
 These bridges should be operational during power domain switching, ie 
 associated
 clocks cannot be gated.
 This patchset adds binding to provide such clocks per power domain and adds 
 code
 which enables them during domain on/off operation.
 
 This patchset fixes power domain issues with disp1 domain and HDMI (some of 
 them)
 on Odroid XU3:
 - disp1 power domain can be turned off,
 - no more imprecise external abort faults.
 
 The patchset is based on '[PATCH v5 0/9] Enable HDMI support on Exynos 
 platforms' [1].
 
 
 It also depends on '[PATCH 0/2] Add HDMI support for Exynos5420 platform' [2].
 
 It was successfully tested on OdroidXU3.
 
 [1]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/42743
 
 Your patches looks good to me so please feel free to add:
 
 Reviewed-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
 
 I also tested on an Exynos5420 Peach Pit Chromebook and both the Power
 domain power-domain disable failed message and the system crash are gone.
 
 Tested-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
 
 Best regards,
 Javier
 
 [2]: https://lkml.org/lkml/2015/1/20/235
 

Any comments about this series? It fixes system hangs on Exynos
5420/5422/5800 boards and has been in the list for a while.

Please keep in mind that a v2 of patch 3/3 was posted [0].

Best regards,
Javier

[0]: https://lkml.org/lkml/2015/2/6/138
--
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] video: treat signal like timeout as failure

2015-03-10 Thread Russell King - ARM Linux
On Tue, Mar 10, 2015 at 01:51:16PM +0100, Nicholas Mc Guire wrote:
 On Tue, 10 Mar 2015, Tomi Valkeinen wrote:
 
  On 20/01/15 07:23, Nicholas Mc Guire wrote:
   if(!wait_for_completion_interruptible_timeout(...))
   only handles the timeout case - this patch adds handling the
   signal case the same as timeout and cleans up.
   
   Signed-off-by: Nicholas Mc Guire der.h...@hofr.at
   ---
   
   Only the timeout case was being handled, return of 0 in 
   wait_for_completion_interruptible_timeout, the signal case (-ERESTARTSYS)
   was treated just like the case of successful completion, which is most 
   likely not reasonable.
   
   Note that exynos_mipi_dsi_wr_data/exynos_mipi_dsi_rd_data return values
   are not checked at the call sites in s6e8ax0.c (cmd_read/cmd_write)!
   
   This patch simply treats the signal case the same way as the timeout case,
   by releasing locks and returning 0 - which might not be the right thing to
   do - this needs a review by someone knowing the details of this driver.
  
  While I agree that this patch is a bit better than the current state,
  the code still looks wrong as Russell said.
  
  I can merge this, but I'd rather have someone from Samsung look at the
  code and change it to use wait_for_completion_killable_timeout() if
  that's what this code is really supposed to use.
 
 If someone that knows the details takes care of it
 that is of course the best solution. If someone Samsung is 
 going to look into it then it is probably best to completly
 drop this speculative patch so that this does not lead
 to more confusion than it does good.

IMHO, just change it to wait_for_completion_killable_timeout() - that's
a much better change than the change you're proposing.

If we think about it...  The current code uses this:

if (!wait_for_completion_interruptible_timeout(dsim_wr_comp,
MIPI_FIFO_TIMEOUT)) {
dev_warn(dsim-dev, command write timeout.\n);
mutex_unlock(dsim-lock);
return -EAGAIN;
}

which has the effect of treating a signal as success, and doesn't return
an error.  So, if the calling application receives (eg) a SIGPIPE or a
SIGALRM, we proceed as if we received the FIFO empty interrupt and doesn't
cause an error.

Your change results in:

timeout = wait_for_completion_interruptible_timeout(
dsim_wr_comp, MIPI_FIFO_TIMEOUT);
if (timeout = 0) {
dev_warn(dsim-dev,
command write timed-out/interrupted.\n);
mutex_unlock(dsim-lock);
return -EAGAIN;
}

which now means that this call returns -EAGAIN when a signal is raised.

Now, further auditing of this exynos crap (and I really do mean crap)
shows that this function is assigned to a method called cmd_write.
Grepping for that shows that *no caller ever checks the return value*!

So, really, there's a bug here in that we should _never_ complete on a
signal, and we most *definitely can not* error out on a signal either.
The *only* sane change to this code without author/maintainer input is
to change this to wait_for_completion_killable_timeout() - so that
signals do not cause either premature completion nor premature failure
of the wait.

The proper fix is absolutely huge: all call paths need to be augmented
with code to detect this function failing, and back out whatever changes
they've made, and restoring the previous state (if they can) and
propagate the error all the way back to userland, so that syscall
restarting can work correctly.  _Only then_ is it safe to use a call
which causes an interruptible sleep.

Personally, I'd be happier seeing this moved into drivers/staging and
eventually deleted from the kernel unless someone is willing to review
the driver and fix some of these glaring problems.  I wouldn't be
surprised if there was _loads_ of this kind of crap there.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
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] video: treat signal like timeout as failure

2015-03-10 Thread Nicholas Mc Guire
On Tue, 10 Mar 2015, Russell King - ARM Linux wrote:

 On Tue, Mar 10, 2015 at 01:51:16PM +0100, Nicholas Mc Guire wrote:
  On Tue, 10 Mar 2015, Tomi Valkeinen wrote:
  
   On 20/01/15 07:23, Nicholas Mc Guire wrote:
if(!wait_for_completion_interruptible_timeout(...))
only handles the timeout case - this patch adds handling the
signal case the same as timeout and cleans up.

Signed-off-by: Nicholas Mc Guire der.h...@hofr.at
---

Only the timeout case was being handled, return of 0 in 
wait_for_completion_interruptible_timeout, the signal case 
(-ERESTARTSYS)
was treated just like the case of successful completion, which is most 
likely not reasonable.

Note that exynos_mipi_dsi_wr_data/exynos_mipi_dsi_rd_data return values
are not checked at the call sites in s6e8ax0.c (cmd_read/cmd_write)!

This patch simply treats the signal case the same way as the timeout 
case,
by releasing locks and returning 0 - which might not be the right thing 
to
do - this needs a review by someone knowing the details of this driver.
   
   While I agree that this patch is a bit better than the current state,
   the code still looks wrong as Russell said.
   
   I can merge this, but I'd rather have someone from Samsung look at the
   code and change it to use wait_for_completion_killable_timeout() if
   that's what this code is really supposed to use.
  
  If someone that knows the details takes care of it
  that is of course the best solution. If someone Samsung is 
  going to look into it then it is probably best to completly
  drop this speculative patch so that this does not lead
  to more confusion than it does good.
 
 IMHO, just change it to wait_for_completion_killable_timeout() - that's
 a much better change than the change you're proposing.
 
 If we think about it...  The current code uses this:
 
 if (!wait_for_completion_interruptible_timeout(dsim_wr_comp,
 MIPI_FIFO_TIMEOUT)) {
 dev_warn(dsim-dev, command write timeout.\n);
 mutex_unlock(dsim-lock);
 return -EAGAIN;
 }
 
 which has the effect of treating a signal as success, and doesn't return
 an error.  So, if the calling application receives (eg) a SIGPIPE or a
 SIGALRM, we proceed as if we received the FIFO empty interrupt and doesn't
 cause an error.
 
 Your change results in:
 
 timeout = wait_for_completion_interruptible_timeout(
 dsim_wr_comp, MIPI_FIFO_TIMEOUT);
 if (timeout = 0) {
 dev_warn(dsim-dev,
 command write timed-out/interrupted.\n);
 mutex_unlock(dsim-lock);
 return -EAGAIN;
 }
 
 which now means that this call returns -EAGAIN when a signal is raised.

but in case of wait_for_completion_killable_timeout it also would return
-ERESTARTSYS (unless I'm missreading do_wait_for_common - 
signal_pending_state(state, current)) so I still think it would be better to 
have the
dev_warn() in the path and then when the task is killed it atleast leaves
some trace of the of what was going on ?

 
 Now, further auditing of this exynos crap (and I really do mean crap)
 shows that this function is assigned to a method called cmd_write.
 Grepping for that shows that *no caller ever checks the return value*!


yup - as was noted in the patch - and this is also why it was
not really possible to figure out what should really be done
as it runs into a dead end in all cases - the only point of the patch was
to atleast generate a debug message and return some signal
indicating error ... which is then unhandled...
 
 So, really, there's a bug here in that we should _never_ complete on a
 signal, and we most *definitely can not* error out on a signal either.
 The *only* sane change to this code without author/maintainer input is
 to change this to wait_for_completion_killable_timeout() - so that
 signals do not cause either premature completion nor premature failure
 of the wait.
 
 The proper fix is absolutely huge: all call paths need to be augmented
 with code to detect this function failing, and back out whatever changes
 they've made, and restoring the previous state (if they can) and
 propagate the error all the way back to userland, so that syscall
 restarting can work correctly.  _Only then_ is it safe to use a call
 which causes an interruptible sleep.
 
 Personally, I'd be happier seeing this moved into drivers/staging and
 eventually deleted from the kernel unless someone is willing to review
 the driver and fix some of these glaring problems.  I wouldn't be
 surprised if there was _loads_ of this kind of crap there.

there is plenty of this - actually all of the wait_for_completion* related
findings I've been posting in the past 2 

Re: [PATCH] video: treat signal like timeout as failure

2015-03-10 Thread Nicholas Mc Guire
On Tue, 10 Mar 2015, Tomi Valkeinen wrote:

 On 20/01/15 07:23, Nicholas Mc Guire wrote:
  if(!wait_for_completion_interruptible_timeout(...))
  only handles the timeout case - this patch adds handling the
  signal case the same as timeout and cleans up.
  
  Signed-off-by: Nicholas Mc Guire der.h...@hofr.at
  ---
  
  Only the timeout case was being handled, return of 0 in 
  wait_for_completion_interruptible_timeout, the signal case (-ERESTARTSYS)
  was treated just like the case of successful completion, which is most 
  likely not reasonable.
  
  Note that exynos_mipi_dsi_wr_data/exynos_mipi_dsi_rd_data return values
  are not checked at the call sites in s6e8ax0.c (cmd_read/cmd_write)!
  
  This patch simply treats the signal case the same way as the timeout case,
  by releasing locks and returning 0 - which might not be the right thing to
  do - this needs a review by someone knowing the details of this driver.
 
 While I agree that this patch is a bit better than the current state,
 the code still looks wrong as Russell said.
 
 I can merge this, but I'd rather have someone from Samsung look at the
 code and change it to use wait_for_completion_killable_timeout() if
 that's what this code is really supposed to use.

If someone that knows the details takes care of it
that is of course the best solution. If someone Samsung is 
going to look into it then it is probably best to completly
drop this speculative patch so that this does not lead
to more confusion than it does good.

thx!
hofrat
--
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 RESEND] usb: dwc2: rework initialization of host and gadget in dual-role mode

2015-03-10 Thread Marek Szyprowski
If device is configured to work only in HOST or DEVICE mode, there is
no point in initializing both subdrivers. This patch also fixes
resource leakage if host subdriver fails to initialize.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
 drivers/usb/dwc2/core.h |  2 ++
 drivers/usb/dwc2/platform.c | 29 +
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 7a70a1349334..f93b06daef97 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -570,6 +570,8 @@ struct dwc2_hsotg {
struct dwc2_core_params *core_params;
enum usb_otg_state op_state;
enum usb_dr_mode dr_mode;
+   unsigned int hcd_enabled:1;
+   unsigned int gadget_enabled:1;
 
struct phy *phy;
struct usb_phy *uphy;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 6a795aa2ff05..ee0b0b06d0fc 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -121,8 +121,10 @@ static int dwc2_driver_remove(struct platform_device *dev)
 {
struct dwc2_hsotg *hsotg = platform_get_drvdata(dev);
 
-   dwc2_hcd_remove(hsotg);
-   s3c_hsotg_remove(hsotg);
+   if (hsotg-hcd_enabled)
+   dwc2_hcd_remove(hsotg);
+   if (hsotg-gadget_enabled)
+   s3c_hsotg_remove(hsotg);
 
return 0;
 }
@@ -214,12 +216,23 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
spin_lock_init(hsotg-lock);
mutex_init(hsotg-init_mutex);
-   retval = dwc2_gadget_init(hsotg, irq);
-   if (retval)
-   return retval;
-   retval = dwc2_hcd_init(hsotg, irq, params);
-   if (retval)
-   return retval;
+
+   if (hsotg-dr_mode != USB_DR_MODE_HOST) {
+   retval = dwc2_gadget_init(hsotg, irq);
+   if (retval)
+   return retval;
+   hsotg-gadget_enabled = 1;
+   }
+
+   if (hsotg-dr_mode != USB_DR_MODE_PERIPHERAL) {
+   retval = dwc2_hcd_init(hsotg, irq, params);
+   if (retval) {
+   if (hsotg-gadget_enabled)
+   s3c_hsotg_remove(hsotg);
+   return retval;
+   }
+   hsotg-hcd_enabled = 1;
+   }
 
platform_set_drvdata(dev, hsotg);
 
-- 
1.9.2

--
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] video: treat signal like timeout as failure

2015-03-10 Thread Tomi Valkeinen
On 20/01/15 07:23, Nicholas Mc Guire wrote:
 if(!wait_for_completion_interruptible_timeout(...))
 only handles the timeout case - this patch adds handling the
 signal case the same as timeout and cleans up.
 
 Signed-off-by: Nicholas Mc Guire der.h...@hofr.at
 ---
 
 Only the timeout case was being handled, return of 0 in 
 wait_for_completion_interruptible_timeout, the signal case (-ERESTARTSYS)
 was treated just like the case of successful completion, which is most 
 likely not reasonable.
 
 Note that exynos_mipi_dsi_wr_data/exynos_mipi_dsi_rd_data return values
 are not checked at the call sites in s6e8ax0.c (cmd_read/cmd_write)!
 
 This patch simply treats the signal case the same way as the timeout case,
 by releasing locks and returning 0 - which might not be the right thing to
 do - this needs a review by someone knowing the details of this driver.

While I agree that this patch is a bit better than the current state,
the code still looks wrong as Russell said.

I can merge this, but I'd rather have someone from Samsung look at the
code and change it to use wait_for_completion_killable_timeout() if
that's what this code is really supposed to use.

 Tomi




signature.asc
Description: OpenPGP digital signature


Re: ARCH_EXYNOS5433 missing in Kconfig

2015-03-10 Thread Chanwoo Choi
Hi Valentin,

I sent the Exynos5433 clock patch and then separately I'm sending the
Exynos5433 devicetree patch-set[1].
[1] [PATCH v6 0/9] arm64: Add the support for new Exynos5433 SoC
- https://lkml.org/lkml/2015/3/9/1036

But, according to Arnd bergmann's comment[2], latest Exynos5433 dt patch-set[1]
removed the CONFIG_ARCH_EXYNOS5433.
[2] https://lkml.org/lkml/2015/2/24/85


So, I have plan to send following patch.

---
clk: samsung: Use CONFIG_ARCH_EXYNOS instead of Exynos-specific 
configuration

This patch removes the CONFIG_ARCH_EXYNOS{5433|7} and then use only the
CONFIG_ARCH_EXYNOS for ARM-64bit Exynos SoC.

Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Tomasz Figa tomasz.f...@gmail.com
Cc: Arnd Bergmann a...@arndb.de
Signed-off-by: Chanwoo Choi cw00.c...@samsung.com
---
 drivers/clk/samsung/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index 17e9af7..561719d 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -10,11 +10,11 @@ obj-$(CONFIG_SOC_EXYNOS5250)+= clk-exynos5250.o
 obj-$(CONFIG_SOC_EXYNOS5260)   += clk-exynos5260.o
 obj-$(CONFIG_SOC_EXYNOS5410)   += clk-exynos5410.o
 obj-$(CONFIG_SOC_EXYNOS5420)   += clk-exynos5420.o
-obj-$(CONFIG_ARCH_EXYNOS5433)  += clk-exynos5433.o
+obj-$(CONFIG_ARCH_EXYNOS)  += clk-exynos5433.o
 obj-$(CONFIG_SOC_EXYNOS5440)   += clk-exynos5440.o
 obj-$(CONFIG_ARCH_EXYNOS)  += clk-exynos-audss.o
 obj-$(CONFIG_ARCH_EXYNOS)  += clk-exynos-clkout.o


Regards,
Chanwoo Choi


On 03/11/2015 12:32 AM, Valentin Rothberg wrote:
 Hi Chanwoo,
 
 your commit 96bd6224f07b (clk: samsung: exynos5433: Add clocks using
 common clock framework) is included in today's linux-next tree (i.e.,
 next-20150310).
 
 This patch conditionally compiles clk-exynos5433.c depending on the
 Kconfig option ARCH_EXYNOS5433.  However, this option is not defined
 in Kconfig, so that the driver cannot be compiled at the current
 state:
 
 +obj-$(CONFIG_ARCH_EXYNOS5433)  += clk-exynos5433.o
 
 Is there a patch queued somewhere that adds this Kconfig symbol?  I
 detected the issue by running undertaker-checkpatch from the
 Undertaker tool suite (undertaker.cs.fau.de).  There is also a tool in
 the git tree that can detect such issues (i.e.,
 scripts/checkkconfigsymbols.py).
 
 Kind regards,
  Valentin
 

--
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: [v3] libdrm: improvements to userspace exynos component

2015-03-10 Thread Inki Dae
Hi Tobias and Emil

On 2015년 03월 11일 04:36, Emil Velikov wrote:
 On 08/03/15 23:12, Tobias Jakobi wrote:
 Hello Emil,
 Emil Velikov wrote:
 I'm planning to push the above mentioned patches around lunchtime this
 Tuesday. If anyone has objections please speak up.
 I doubt you're going to hear anything from Inki Dae.
 Despite that it might sound a bit rude, I sincerely hope that you're
 wrong on this one. I'm would assume that the lack of reply from Inki was
 due to being him busy, rather than ignoring you.
 

I was busy so I didn't care of it. Sorry for this.

 Anyway, thanks for considering to merge this, even though it lacks the
 review of the exynos-drm maintainer.

 At least this gives me confidence to continue to work on this (I also
 want to expose async g2d operation to userspace).

 Glad that you're still planning to work on this. Ideally the
 Exynos/Samsung guys will take a look at the rest of your patches which
 require someone with greater knowledge of the hardware than me :-)
 

Right. We will review this patch and reply at least until tomorrow.

Thanks,
Inki Dae

 Cheers,
 Emil
 
 

--
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] video: treat signal like timeout as failure

2015-03-10 Thread Russell King - ARM Linux
On Tue, Mar 10, 2015 at 03:39:28PM +0100, Nicholas Mc Guire wrote:
 On Tue, 10 Mar 2015, Russell King - ARM Linux wrote:
  On Tue, Mar 10, 2015 at 01:51:16PM +0100, Nicholas Mc Guire wrote:
   On Tue, 10 Mar 2015, Tomi Valkeinen wrote:
   
On 20/01/15 07:23, Nicholas Mc Guire wrote:
 if(!wait_for_completion_interruptible_timeout(...))
 only handles the timeout case - this patch adds handling the
 signal case the same as timeout and cleans up.
 
 Signed-off-by: Nicholas Mc Guire der.h...@hofr.at
 ---
 
 Only the timeout case was being handled, return of 0 in 
 wait_for_completion_interruptible_timeout, the signal case 
 (-ERESTARTSYS)
 was treated just like the case of successful completion, which is 
 most 
 likely not reasonable.
 
 Note that exynos_mipi_dsi_wr_data/exynos_mipi_dsi_rd_data return 
 values
 are not checked at the call sites in s6e8ax0.c (cmd_read/cmd_write)!
 
 This patch simply treats the signal case the same way as the timeout 
 case,
 by releasing locks and returning 0 - which might not be the right 
 thing to
 do - this needs a review by someone knowing the details of this 
 driver.

While I agree that this patch is a bit better than the current state,
the code still looks wrong as Russell said.

I can merge this, but I'd rather have someone from Samsung look at the
code and change it to use wait_for_completion_killable_timeout() if
that's what this code is really supposed to use.
   
   If someone that knows the details takes care of it
   that is of course the best solution. If someone Samsung is 
   going to look into it then it is probably best to completly
   drop this speculative patch so that this does not lead
   to more confusion than it does good.
  
  IMHO, just change it to wait_for_completion_killable_timeout() - that's
  a much better change than the change you're proposing.
  
  If we think about it...  The current code uses this:
  
  if 
  (!wait_for_completion_interruptible_timeout(dsim_wr_comp,
  MIPI_FIFO_TIMEOUT)) 
  {
  dev_warn(dsim-dev, command write timeout.\n);
  mutex_unlock(dsim-lock);
  return -EAGAIN;
  }
  
  which has the effect of treating a signal as success, and doesn't return
  an error.  So, if the calling application receives (eg) a SIGPIPE or a
  SIGALRM, we proceed as if we received the FIFO empty interrupt and doesn't
  cause an error.
  
  Your change results in:
  
  timeout = wait_for_completion_interruptible_timeout(
  dsim_wr_comp, MIPI_FIFO_TIMEOUT);
  if (timeout = 0) {
  dev_warn(dsim-dev,
  command write timed-out/interrupted.\n);
  mutex_unlock(dsim-lock);
  return -EAGAIN;
  }
  
  which now means that this call returns -EAGAIN when a signal is raised.
 
 but in case of wait_for_completion_killable_timeout it also would return
 -ERESTARTSYS (unless I'm missreading do_wait_for_common - 
 signal_pending_state(state, current)) so I still think it would be better to 
 have the
 dev_warn() in the path and then when the task is killed it atleast leaves
 some trace of the of what was going on ?
 
  
  Now, further auditing of this exynos crap (and I really do mean crap)
  shows that this function is assigned to a method called cmd_write.
  Grepping for that shows that *no caller ever checks the return value*!
 
 
 yup - as was noted in the patch - and this is also why it was
 not really possible to figure out what should really be done
 as it runs into a dead end in all cases - the only point of the patch was
 to atleast generate a debug message and return some signal
 indicating error ... which is then unhandled...
  
  So, really, there's a bug here in that we should _never_ complete on a
  signal, and we most *definitely can not* error out on a signal either.
  The *only* sane change to this code without author/maintainer input is
  to change this to wait_for_completion_killable_timeout() - so that
  signals do not cause either premature completion nor premature failure
  of the wait.
  
  The proper fix is absolutely huge: all call paths need to be augmented
  with code to detect this function failing, and back out whatever changes
  they've made, and restoring the previous state (if they can) and
  propagate the error all the way back to userland, so that syscall
  restarting can work correctly.  _Only then_ is it safe to use a call
  which causes an interruptible sleep.
  
  Personally, I'd be happier seeing this moved into drivers/staging and
  eventually deleted from the kernel unless someone is willing to review
  the driver and fix some of these glaring 

Re: [PATCH] video: treat signal like timeout as failure

2015-03-10 Thread Tomi Valkeinen
On 10/03/15 16:46, Russell King - ARM Linux wrote:

 In which case, let me propose that the exynos fbdev driver needs to be
 moved to drivers/staging, and stay there until this stuff gets fixed.
 drivers/staging is supposed to be for stuff which isn't up to the mark,
 and which is potentially unstable.  And that's what this driver exactly
 is.

There is drivers/gpu/drm/exynos/ which is getting a lot of updates. So...

I'd propose removing the exynos fbdev driver if the exynos drm driver
offers the same functionality. I don't know if that's the case. Does the
drm driver support all the devices the fbdev supports?

Also, I'm not sure if and how we can remove drivers. If exynos fbdev
driver is dropped, that would perhaps break boards that have exynos
fbdev in their .dts file. And if the drm driver doesn't offer the exact
same /dev/fbX interface, it would break the userspace.

So I don't know if that's possible. But that's what I'd like to do,
eventually, for all the fbdev drivers. Implement drm driver, remove the
fbdev one.

 Tomi




signature.asc
Description: OpenPGP digital signature


Re: [PATCH] video: treat signal like timeout as failure

2015-03-10 Thread Russell King - ARM Linux
On Tue, Mar 10, 2015 at 04:55:56PM +0200, Tomi Valkeinen wrote:
 On 10/03/15 16:46, Russell King - ARM Linux wrote:
 
  In which case, let me propose that the exynos fbdev driver needs to be
  moved to drivers/staging, and stay there until this stuff gets fixed.
  drivers/staging is supposed to be for stuff which isn't up to the mark,
  and which is potentially unstable.  And that's what this driver exactly
  is.
 
 There is drivers/gpu/drm/exynos/ which is getting a lot of updates. So...
 
 I'd propose removing the exynos fbdev driver if the exynos drm driver
 offers the same functionality. I don't know if that's the case. Does the
 drm driver support all the devices the fbdev supports?
 
 Also, I'm not sure if and how we can remove drivers. If exynos fbdev
 driver is dropped, that would perhaps break boards that have exynos
 fbdev in their .dts file. And if the drm driver doesn't offer the exact
 same /dev/fbX interface, it would break the userspace.
 
 So I don't know if that's possible. But that's what I'd like to do,
 eventually, for all the fbdev drivers. Implement drm driver, remove the
 fbdev one.

That's why I suggested moving it to drivers/staging - it's a hint that
the driver needs a serious amount of work, and when built as a module,
it also provides users with the hint that the module they're loading is
of questionable quality (which is definitely the case here.)

Others have done that kind of thing before - we've had drivers which
have fallen by the way side, and at some point the decision has been
made to move them to drivers/staging, and if nothing happens to fix
them up (showing that no one cares about them), they've eventually
been dropped.

Of course, us talking about this might be enough to spur some effort
to get the thing properly fixed. :)

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
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


ARCH_EXYNOS5433 missing in Kconfig

2015-03-10 Thread Valentin Rothberg
Hi Chanwoo,

your commit 96bd6224f07b (clk: samsung: exynos5433: Add clocks using
common clock framework) is included in today's linux-next tree (i.e.,
next-20150310).

This patch conditionally compiles clk-exynos5433.c depending on the
Kconfig option ARCH_EXYNOS5433.  However, this option is not defined
in Kconfig, so that the driver cannot be compiled at the current
state:

+obj-$(CONFIG_ARCH_EXYNOS5433)  += clk-exynos5433.o

Is there a patch queued somewhere that adds this Kconfig symbol?  I
detected the issue by running undertaker-checkpatch from the
Undertaker tool suite (undertaker.cs.fau.de).  There is also a tool in
the git tree that can detect such issues (i.e.,
scripts/checkkconfigsymbols.py).

Kind regards,
 Valentin
--
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: [v3] libdrm: improvements to userspace exynos component

2015-03-10 Thread Emil Velikov
On 08/03/15 23:12, Tobias Jakobi wrote:
 Hello Emil,
 Emil Velikov wrote:
 I'm planning to push the above mentioned patches around lunchtime this
 Tuesday. If anyone has objections please speak up.
 I doubt you're going to hear anything from Inki Dae.
Despite that it might sound a bit rude, I sincerely hope that you're
wrong on this one. I'm would assume that the lack of reply from Inki was
due to being him busy, rather than ignoring you.

 Anyway, thanks for considering to merge this, even though it lacks the
 review of the exynos-drm maintainer.
 
 At least this gives me confidence to continue to work on this (I also
 want to expose async g2d operation to userspace).
 
Glad that you're still planning to work on this. Ideally the
Exynos/Samsung guys will take a look at the rest of your patches which
require someone with greater knowledge of the hardware than me :-)

Cheers,
Emil

--
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] phy: exynos-mipi-video: Fix unbalanced lock on non-regmap path

2015-03-10 Thread Axel Lin
Remove the second lock on non-regmap path.

Fixes: 278270e39efa (phy: exynos-mipi-video: Use spin_lock to protct 
state-regmap rmw operations)
Reported-by: Julia Lawall julia.law...@lip6.fr
Signed-off-by: Axel Lin axel@ingics.com
---
 drivers/phy/phy-exynos-mipi-video.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/phy/phy-exynos-mipi-video.c 
b/drivers/phy/phy-exynos-mipi-video.c
index 8d6d117..df7519a 100644
--- a/drivers/phy/phy-exynos-mipi-video.c
+++ b/drivers/phy/phy-exynos-mipi-video.c
@@ -75,7 +75,6 @@ static int __set_phy_state(struct exynos_mipi_video_phy 
*state,
} else {
addr = state-regs + EXYNOS_MIPI_PHY_CONTROL(id / 2);
 
-   spin_lock(state-slock);
val = readl(addr);
if (on)
val |= reset;
-- 
1.9.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