[PATCH 14/14] drivers/spi/spi-s3c24xx.c: fix error return code

2012-08-19 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: [RFC/PATCH 09/13] media: s5k6aa: Add support for device tree based instantiation

2012-08-19 Thread Sakari Ailus
Hi Laurent and Sylwester,

My apologies for the late answer.

On Fri, Jul 27, 2012 at 12:50:13AM +0200, Laurent Pinchart wrote:
 Hi Sylwester,
 
 On Thursday 26 July 2012 22:39:31 Sylwester Nawrocki wrote:
  On 07/26/2012 05:21 PM, Laurent Pinchart wrote:
   On Friday 25 May 2012 21:52:48 Sylwester Nawrocki wrote:
   The driver initializes all board related properties except the s_power()
   callback to board code. The platforms that require this callback are not
   supported by this driver yet for CONFIG_OF=y.
   
   Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com
   Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com
   Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com
   ---
   
 .../bindings/camera/samsung-s5k6aafx.txt   |   57 +
 drivers/media/video/s5k6aa.c   |  129 +
 2 files changed, 146 insertions(+), 40 deletions(-)
 create mode 100644
   
   Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
   
   diff --git
   a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
   b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file
   mode 100644
   index 000..6685a9c
   --- /dev/null
   +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
   @@ -0,0 +1,57 @@
   +Samsung S5K6AAFX camera sensor
   +--
   +
   +Required properties:
   +
   +- compatible : samsung,s5k6aafx;
   +- reg : base address of the device on I2C bus;
   +- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601;
   +- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V);
   +- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V);
   +- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to
   1.9V) + or 2.8V (2.6V to 3.0);
   +- vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V)
   + or 2.8V (2.5V to 3.1V);
   +
   +Optional properties:
   +
   +- clock-frequency : the IP's main (system bus) clock frequency in Hz,
   the default
   
   Is that the input clock frequency ? Can't it vary ? Instead of accessing
   the
  Yes, the description is incorrect in this patch, it should read:
  
  +- clock-frequency : the sensor's master clock frequency in Hz;
  
  and be a required property. As in this patch:
  https://github.com/snawrocki/linux/commit/e8a5f890dec0d7414b656bb1d1ac97d5e7
  abe563
  
  It could vary (as this is a PLL input frequency), so probably a range would
  be a better alternative. Given that host device won't always be able to set
  this exact value...
 
 A range sounds good, or perhaps a list of ranges. Sakari, what would you need 
 for the SMIA++ driver ?

Typically the sensor's external clock is derived from another clock using a
divisor. This means there's usually quite limited selection of possible
clock frequencies since the sensors usually have a small range of possible
frequencies such as 6 -- 27 MHz or even less.

Also it's important to choose the frequency in such a way that it doesn't
interfere with other devices in the system. The frequency also must be
picked so that one can achieve the desired highest data transfer rate. The
sensors are also quite flexible in their internal clock tree configuration,
but in the situation where the desired data rate is close to sensor limits
there may be additional constraints. Still it's always possible to come up
with a best value for the board while other values are inferior.

For these reasons I see little point in providing anything else than just a
single value for the external clock frequency. This value is board-specific.

   sensor clock frequency from the FIMC driver you should reference a clock
   in the sensor DT node. That obviously requires generic clock support,
   which might not be available for your platform yet (that's one of the
   reasons the OMAP3 ISP driver doesn't support DT yet).
  
  I agree it might be better, but waiting unknown number of kernel releases
  for the platforms to get converted to common clock API is not a good
  alternative either. I guess we could have some transitional solutions while
  other subsystems are getting adapted.
 
 I agree, we need an interim solution.

The SMIA++ driver allows the platform data to have either the clock name or
a function pointer to set the clock frequency. If the clock name is there
it'll be used instead. The function pointer can be removed once it's no
longer needed.

Kind regards,

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v6 1/6] thermal: add generic cpufreq cooling implementation

2012-08-19 Thread Zhang Rui
On δΊ”, 2012-08-17 at 11:56 +0300, Valentin, Eduardo wrote:
 Hello,

  +
  +
  +1.2 CPU cooling action notifier register/unregister interface
  +1.2.1 int cputherm_register_notifier(struct notifier_block *nb,
  + unsigned int list)
  +
  +This interface registers a driver with cpu cooling layer. The driver 
  will
  +be notified when any cpu cooling action is called.
  +
  +nb: notifier function to register
  +list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
  +
  +1.2.2 int cputherm_unregister_notifier(struct notifier_block *nb,
  + unsigned int list)
  +
  +This interface registers a driver with cpu cooling layer. The driver 
  will
  +be notified when any cpu cooling action is called.
  +
  +nb: notifier function to register
  +list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
 
  what are these two APIs used for?
  I did not see they are used in your patch set, do I miss something?
  No currently they are not used by my patches. I added them on request
  from Eduardo and others
 
 Yeah, this was a suggestion as we didn't really know how the FW part
 would evolve by that time.
 
 The reasoning is to allow any interested user (in kernel) to be
 notified when max frequency changes.

in this case, the cooling device should be updated.
Say all the target of the thermal_instances of a cooling devices is 0,
which means they want the cpu to run at maximum frequency, when the max
frequency changes, we should set the processor to the new max frequency
as well.
Using notification is not a good idea as user can not handle this
without interacting with the thermal framework.

IMO, we should introduce a new API to handle this, rather than just
notifications to users.

  Actually, the use case behind
 this is to allow such users to perform some handshaking or stop their
 activities or even change some paramenters, in case the max frequency
 would change.

It is the cooling device driver that notice this change first, and it
should invoke something like thermal_cooling_device_update/rebind() to
tell the thermal framework this change.

  Ideally it would be possible to nack the cooling
 transition. But that is yet a wider discussion. So far we don't have
 users for this.
 
yep.
I thought about this before, but I'd prefer to do this when there is a
real user. Or else, we are kind of over-designing here.
how about removing this piece of code for now?

thanks,
rui

 
  diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
  index 7dd8c34..996003b 100644
  --- a/drivers/thermal/Kconfig
  +++ b/drivers/thermal/Kconfig
  @@ -19,6 +19,17 @@ config THERMAL_HWMON
depends on HWMON=y || HWMON=THERMAL
default y
 
  +config CPU_THERMAL
  + bool generic cpu cooling support
  + depends on THERMAL  CPU_FREQ
  + help
  +   This implements the generic cpu cooling mechanism through 
  frequency
  +   reduction, cpu hotplug and any other ways of reducing 
  temperature. An
  +   ACPI version of this already 
  exists(drivers/acpi/processor_thermal.c).
  +   This will be useful for platforms using the generic thermal 
  interface
  +   and not the ACPI interface.
  +   If you want this support, you should say Y here.
  +
   config SPEAR_THERMAL
bool SPEAr thermal sensor driver
depends on THERMAL
  diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
  index fd9369a..aae59ad 100644
  --- a/drivers/thermal/Makefile
  +++ b/drivers/thermal/Makefile
  @@ -3,5 +3,6 @@
   #
 
   obj-$(CONFIG_THERMAL)+= thermal_sys.o
  +obj-$(CONFIG_CPU_THERMAL)+= cpu_cooling.o
   obj-$(CONFIG_SPEAR_THERMAL)  += spear_thermal.o
   obj-$(CONFIG_RCAR_THERMAL)   += rcar_thermal.o
  diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
  new file mode 100644
  index 000..c42e557
  --- /dev/null
  +++ b/drivers/thermal/cpu_cooling.c
  @@ -0,0 +1,512 @@
  +/*
  + *  linux/drivers/thermal/cpu_cooling.c
  + *
  + *  Copyright (C) 2012   Samsung Electronics Co., 
  Ltd(http://www.samsung.com)
  + *  Copyright (C) 2012  Amit Daniel amit.kach...@linaro.org
  + *
  + * 
  ~~
  + *  This program is free software; you can redistribute it and/or modify
  + *  it under the terms of the GNU General Public License as published by
  + *  the Free Software Foundation; version 2 of the License.
  + *
  + *  This program is distributed in the hope that it will be useful, but
  + *  WITHOUT ANY WARRANTY; without even the implied warranty of
  + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  + *  General Public License for more details.
  + *
  + *  You should have received a copy of the GNU General Public License 
  along
  + *  with this program; if not, write to the Free Software Foundation, 
  Inc.,
  + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
  + *
  + *