[PATCH] ARM: SAMSUNG: Fix for S3C2412 EBI memory mapping.

2012-05-09 Thread José Miguel Gonçalves
While upgrading the kernel on a S3C2412 based board I've noted that it was 
impossible to boot the board with a 2.6.32 or upper kernel.
I've tracked down the problem to the EBI virtual memory mapping that is in 
conflict with the IO mapping definition in arch/arm/mach-s3c24xx/s3c2412.c.

Signed-off-by: José Miguel Gonçalves 
---
 arch/arm/plat-samsung/include/plat/map-s3c.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-samsung/include/plat/map-s3c.h 
b/arch/arm/plat-samsung/include/plat/map-s3c.h
index 7d04875..c0c70a8 100644
--- a/arch/arm/plat-samsung/include/plat/map-s3c.h
+++ b/arch/arm/plat-samsung/include/plat/map-s3c.h
@@ -22,7 +22,7 @@
 #define S3C24XX_VA_WATCHDOGS3C_VA_WATCHDOG
 
 #define S3C2412_VA_SSMCS3C_ADDR_CPU(0x)
-#define S3C2412_VA_EBI S3C_ADDR_CPU(0x0001)
+#define S3C2412_VA_EBI S3C_ADDR_CPU(0x0010)
 
 #define S3C2410_PA_UART(0x5000)
 #define S3C24XX_PA_UARTS3C2410_PA_UART
-- 
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] mfd: Add support for MAX77686.

2012-05-09 Thread Chanwoo Choi
Hi Mark,

We have posted following patch on the last week and received
your comment. So, We are implementing that use regmap API for I2C
and modify MFD driver of MAX77686 according to your comment.

[PATCH] MFD : add MAX77686 mfd driver
- https://lkml.org/lkml/2012/4/30/96

Additionally, We are intergrating support irq_domain for MAX77686 irq.

We will post new patch to support MFD driver of MAX77686 including
modification by your comment within this week.

Best Regards,
Chanwoo Choi

On 05/10/2012 03:27 AM, Mark Brown wrote:

> On Wed, May 09, 2012 at 09:54:54PM +0530, Yadwinder Singh wrote:
> 
>> +int max77686_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
>> +{
>> +int ret;
>> +
>> +ret = i2c_smbus_read_byte_data(i2c, reg);
>> +if (ret < 0)
> 
> It would really be better if this used the regmap API - the regulator
> API is starting to build out functionality on top of regmap which this
> won't be able to take advantage of if it doesn't use regmap.
> 
>> +if (of_get_property(dev->of_node, "max77686,wakeup", NULL))
>> +pd->wakeup = true;
> 
> You haven't included any binding documentatiojn for the device tree
> bindings - you should do this.
> 
>> +max77686 = kzalloc(sizeof(struct max77686_dev), GFP_KERNEL);
>> +if (max77686 == NULL) {
>> +dev_err(max77686->dev, "could not allocate memory\n");
>> +return -ENOMEM;
>> +}
> 
> devm_kzalloc().
> 
>> +device_init_wakeup(max77686->dev, max77686->wakeup);
> 
> Why is this not just done unconditionally?  There's sysfs files to allow
> userspace control.
> 
>> +if (max77686_read_reg(i2c, MAX77686_REG_DEVICE_ID, &data) < 0) {
>> +ret = -EIO;
>> +dbg_info("%s : device not found on this channel\n", __func__);
>> +goto err_mfd;
>> +} else
>> +dev_info(max77686->dev, "device found\n");
> 
> You should verify that the device ID you read back is the expected one.
> 
>> +dev_err(max77686->dev, "device probe failed : %d\n", ret);
> 
> Driver core should do this for you.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] regulator: Add support for MAX77686.

2012-05-09 Thread Sylwester Nawrocki

Hi,

just a few nitpicks...

On 05/09/2012 06:24 PM, Yadwinder Singh wrote:

From: Yadwinder Singh Brar

Add support for PMIC/regulator portion of MAX77686 multifunction device.
MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driver
which supports setting and getting the voltage of a regulator with I2C
interface.

Signed-off-by: Yadwinder Singh Brar

...

diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
new file mode 100644
index 000..4aa9722
--- /dev/null
+++ b/drivers/regulator/max77686.c
@@ -0,0 +1,660 @@
+/*
+ * max77686.c - Regulator driver for the Maxim 77686
+ *
+ * Copyright (C) 2012 Samsung Electronics


I believe this should read:

  + * Copyright (C) 2012 Samsung Electronics Co., Ltd.

In patch 1/2 the copyright notice is also not exactly correct.


+ * Chiwoong Byun


s/smasung/samsung

...

+static int max77686_get_enable_register(struct regulator_dev *rdev,
+   int *reg, int *mask, int *pattern)
+{
+   int rid = rdev_get_id(rdev);
+
+   switch (rid) {
+   case MAX77686_LDO1...MAX77686_LDO26:
+   *reg = MAX77686_REG_LDO1CTRL1 + (rid - MAX77686_LDO1);
+   *mask = 0xC0;
+   *pattern = 0xC0;


What about using lower case for all hex numbers ?

...

+static int max77686_get_voltage_register(struct regulator_dev *rdev,
+int *_reg, int *_shift, int *_mask)
+{

...

+   case MAX77686_BUCK2:
+   reg = MAX77686_REG_BUCK2DVS1;
+   mask = 0xff;
+   break;
+   case MAX77686_BUCK3:
+   reg = MAX77686_REG_BUCK3DVS1;
+   mask = 0xff;
+   break;
+   case MAX77686_BUCK4:
+   reg = MAX77686_REG_BUCK4DVS1;
+   mask = 0xff;
+   break;
+   case MAX77686_BUCK5...MAX77686_BUCK9:
+   reg = MAX77686_REG_BUCK5OUT + (rid - MAX77686_BUCK5) * 2;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   *_reg = reg;
+   *_shift = shift;
+   *_mask = mask;
+
+   return 0;
+}


Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] regulator: Add support for MAX77686.

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:

> +/* Voltage maps in mV */
> +static const struct voltage_map_desc ldo_voltage_map_desc = {
> + .min = 800, .max = 3950,.step = 50, .n_bits = 6,
> +};   /* LDO3 ~ 5, 9 ~ 14, 16 ~ 26 */

Hrm, funnily enough I was just thinking about factoring this stuff out
into the core after a conversation with Graeme Gregory the other week.
Let's do that...

> + [MAX77686_EN32KHZ_AP] = NULL,
> + [MAX77686_EN32KHZ_CP] = NULL,

Now that the generic clock API is in mainline these should be moved over
to use it.

> +static int max77686_get_voltage_unit(int rid)
> +{
> + int unit = 0;
> +
> + switch (rid) {
> + case MAX77686_BUCK2...MAX77686_BUCK4:
> + unit = 1;   /* BUCK2,3,4 is uV */
> + break;
> + default:
> + unit = 1000;

Why not just list everything in uV?

> +static int max77686_get_voltage(struct regulator_dev *rdev)
> +{

Implement get_voltage_sel().

> +static inline int max77686_get_voltage_proper_val(const struct 
> voltage_map_desc
> +   *desc, int min_vol,
> +   int max_vol)
> +{
> + int i = 0;
> +
> + if (desc == NULL)
> + return -EINVAL;
> +
> + if (max_vol < desc->min || min_vol > desc->max)
> + return -EINVAL;
> +
> + while (desc->min + desc->step * i < min_vol &&
> +desc->min + desc->step * i < desc->max)
> + i++;

Why are you iterating here?  Calculate!  Though like I say let's factor
this out anyway.

> + if (rid == MAX77686_BUCK2 || rid == MAX77686_BUCK3 ||
> + rid == MAX77686_BUCK4) {
> + /* If the voltage is increasing */
> + if (org < i)
> + udelay(DIV_ROUND_UP(desc->step * (i - org),
> + ramp[max77686->ramp_delay]));
> + }

Don't do this, implement set_voltage_time_sel().

> + .enable = max77686_reg_enable,
> + .disable = max77686_reg_disable,
> + .set_suspend_enable = max77686_reg_enable,
> + .set_suspend_disable = max77686_reg_disable,

You've got the same ops for suspend and non-suspend cases here, this is
clearly buggy.

> +/* count the number of regulators to be supported in pmic */
> + pdata->num_regulators = 0;

Coding style.

> + if (iodev->dev->of_node) {
> + ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
> + if (ret)
> + return ret;

This ought to use of_regulator_match().

> + }
> +
> + if (!pdata) {
> + dev_err(&pdev->dev, "platform data not found\n");
> + return -ENODEV;
> + }

This should be totally fine.

> + max77686 = kzalloc(sizeof(struct max77686_data), GFP_KERNEL);
> + if (!max77686)
> + return -ENOMEM;

devm_kzalloc().

> + if (pdata->ramp_delay) {
> + max77686->ramp_delay = pdata->ramp_delay;
> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
> + RAMP_VALUE, RAMP_MASK);

This appears not to actually use the value passed in as platform_data.

> +
> + for (i = 0; i < pdata->num_regulators; i++) {
> + const struct voltage_map_desc *desc;
> + int id = pdata->regulators[i].id;
> +
> + desc = reg_voltage_map[id];
> + if (desc)
> + regulators[id].n_voltages =
> + (desc->max - desc->min) / desc->step + 1;
> +
> + rdev[i] = regulator_register(®ulators[id], max77686->dev,
> +  pdata->regulators[i].initdata,
> +  max77686, NULL);

No, you should unconditionally register all regulators the device
physically has.  This is useful for debug and simplifies the code.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] mfd: Add support for MAX77686.

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 09:54:54PM +0530, Yadwinder Singh wrote:

> +int max77686_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
> +{
> + int ret;
> +
> + ret = i2c_smbus_read_byte_data(i2c, reg);
> + if (ret < 0)

It would really be better if this used the regmap API - the regulator
API is starting to build out functionality on top of regmap which this
won't be able to take advantage of if it doesn't use regmap.

> + if (of_get_property(dev->of_node, "max77686,wakeup", NULL))
> + pd->wakeup = true;

You haven't included any binding documentatiojn for the device tree
bindings - you should do this.

> + max77686 = kzalloc(sizeof(struct max77686_dev), GFP_KERNEL);
> + if (max77686 == NULL) {
> + dev_err(max77686->dev, "could not allocate memory\n");
> + return -ENOMEM;
> + }

devm_kzalloc().

> + device_init_wakeup(max77686->dev, max77686->wakeup);

Why is this not just done unconditionally?  There's sysfs files to allow
userspace control.

> + if (max77686_read_reg(i2c, MAX77686_REG_DEVICE_ID, &data) < 0) {
> + ret = -EIO;
> + dbg_info("%s : device not found on this channel\n", __func__);
> + goto err_mfd;
> + } else
> + dev_info(max77686->dev, "device found\n");

You should verify that the device ID you read back is the expected one.

> + dev_err(max77686->dev, "device probe failed : %d\n", ret);

Driver core should do this for you.
--
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 0/2] regulator: add initial suport for max77686

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 09:54:53PM +0530, Yadwinder Singh wrote:
> This patch series adds support for max77686 which is a multifunction device 
> which
> includes regulator (pmic), rtc and charger sub-blocks within it. The support 
> for
> mfd driver and regulator driver are added by this patch series. This patch 
> series
> also includes device tree and irqdomain support for mfd and regulator 
> portions.

Always CC maintainers on patches, please see SubmittingPatches.
--
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 05/10] ARM: Samsung: Update the device names for spi clock lookup

2012-05-09 Thread Thomas Abraham
On 9 May 2012 22:28, Mark Brown  wrote:
> On Wed, May 09, 2012 at 09:40:26PM +0800, Thomas Abraham wrote:
>> On 9 May 2012 16:52, Mark Brown  wrote:
>
>> > This should've been squashed into the patch that updated to use driver
>> > data in order to avoid breaking bisection.
>
>> This patch updates clock devname in the platform code and the
>> modifications to use driver data is in the driver file. So these two
>> changes are kept separate to allow them to be applied to their
>> respective trees.
>
> This means that bisection will be broken - anything with only one tree
> won't be able to load the SPI driver successfully until it's merged
> with the other which isn't ideal.

Yes, we would have that problem until the two trees are merged. But I
still prefer not to squash the two patches which already contain huge
diff.

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 10/10] spi: s3c64xx: add device tree support

2012-05-09 Thread Thomas Abraham
On 10 May 2012 00:47, Mark Brown  wrote:
> On Thu, May 10, 2012 at 12:39:29AM +0800, Thomas Abraham wrote:
>> On 9 May 2012 22:32, Mark Brown  wrote:
>
>> > Yeah, I know.  I'm saying we should try to come up with a binding for
>> > this that can be used by new SPI contollers going forward so things are
>> > consistent.
>
>> Ok. For this patch series, I will continue with the samsung specific
>> binding for the nCS line.
>
> How about just renaming your binding to a generic one and documenting it
> separately?  It looks like a sensible binding which other people should
> be able to use so may as well have something there already.

Ok. I will replace the samsung specific binding with a generic binding
in the next version of this patch series.

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 10/10] spi: s3c64xx: add device tree support

2012-05-09 Thread Mark Brown
On Thu, May 10, 2012 at 12:39:29AM +0800, Thomas Abraham wrote:
> On 9 May 2012 22:32, Mark Brown  wrote:

> > Yeah, I know.  I'm saying we should try to come up with a binding for
> > this that can be used by new SPI contollers going forward so things are
> > consistent.

> Ok. For this patch series, I will continue with the samsung specific
> binding for the nCS line.

How about just renaming your binding to a generic one and documenting it
separately?  It looks like a sensible binding which other people should
be able to use so may as well have something there already.


signature.asc
Description: Digital signature


Re: [PATCH 10/10] spi: s3c64xx: add device tree support

2012-05-09 Thread Thomas Abraham
On 9 May 2012 22:32, Mark Brown  wrote:
> On Wed, May 09, 2012 at 10:13:28PM +0800, Thomas Abraham wrote:
>> On 9 May 2012 17:07, Mark Brown  wrote:
>> > On Wed, May 09, 2012 at 03:34:54AM +0530, Thomas Abraham wrote:
>
>> >> +- gpios: The gpio specifier for clock, mosi and miso interface lines (in 
>> >> no
>> >> +  particular order). The format of the gpio specifier depends on the gpio
>> >> +  controller.
>
>> > This seems odd...  This isn't a bitbanging controller, and surely the
>> > driver will need to know which signal is which?  I suspect this is
>> > actually for pinmux rather than to identify the signals but that should
>> > at least be made clear and really should be being done using the pinmux
>> > API.
>
>> The driver retrieves the list of gpio's that it is allowed to use. The
>> gpio numbers for miso, mosi and clk are mandatory but the order in
>> which they are specified is not important since the driver never needs
>> to which gpio is which interface line. I agree the pinmux api should
>> be used here, but the call to pinmux api would be a incremental change
>> here, not changing the code this patch is adding.
>
> I'd suggest just specifying the order - someone might want to use it
> later for some reason and it's not really a hardship for someone to use
> it.  Avoids any "how does that work?" questions like I had.

Ok. I will add the order of the gpios as you have suggested.

>
>> >> +  - samsung,spi-cs-gpio: A gpio specifier that specifies the gpio line 
>> >> used as
>> >> +    the slave select line by the spi controller. The format of the gpio
>> >> +    specifier depends on the gpio controller.
>
>> > We should really have a binding for this at the SPI level (and ideally
>> > some code to manage setting the GPIO too) - it's pretty common to use a
>> > GPIO as /CS.
>
>> The existing implementations vary in the way the nCS gpio lines are
>> specified. For some controllers, the nCS gpio's are included in the
>> spi device node whereas in this implementation, the nCS gpio is listed
>> in the spi slave device node.
>
> Yeah, I know.  I'm saying we should try to come up with a binding for
> this that can be used by new SPI contollers going forward so things are
> consistent.

Ok. For this patch series, I will continue with the samsung specific
binding for the nCS line.

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


[PATCH 2/2] regulator: Add support for MAX77686.

2012-05-09 Thread Yadwinder Singh
From: Yadwinder Singh Brar 

Add support for PMIC/regulator portion of MAX77686 multifunction device.
MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driver
which supports setting and getting the voltage of a regulator with I2C
interface.

Signed-off-by: Yadwinder Singh Brar 
---
 drivers/regulator/Kconfig|9 +
 drivers/regulator/Makefile   |1 +
 drivers/regulator/max77686.c |  660 ++
 3 files changed, 670 insertions(+), 0 deletions(-)
 create mode 100644 drivers/regulator/max77686.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 36db5a4..67162cc 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -195,6 +195,15 @@ config REGULATOR_MAX8998
  via I2C bus. The provided regulator is suitable for S3C6410
  and S5PC1XX chips to control VCC_CORE and VCC_USIM voltages.
 
+config REGULATOR_MAX77686
+   tristate "Maxim 77686 regulator"
+   depends on MFD_MAX77686
+   help
+ This driver controls a Maxim 77686 voltage regulator via I2C
+ bus. The provided regulator is suitable for Exynos5 chips to
+ control VDD_ARM and VDD_INT voltages.It supports LDOs[1-26]
+ and BUCKs[1-9].
+
 config REGULATOR_PCAP
tristate "Motorola PCAP2 regulator driver"
depends on EZX_PCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 94b5274..008931b 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_REGULATOR_MAX8925) += max8925-regulator.o
 obj-$(CONFIG_REGULATOR_MAX8952) += max8952.o
 obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
 obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
+obj-$(CONFIG_REGULATOR_MAX77686) += max77686.o
 obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
 obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
new file mode 100644
index 000..4aa9722
--- /dev/null
+++ b/drivers/regulator/max77686.c
@@ -0,0 +1,660 @@
+/*
+ * max77686.c - Regulator driver for the Maxim 77686
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Chiwoong Byun 
+ *
+ * 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; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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
+ *
+ * This driver is based on max8997.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct max77686_data {
+   struct device *dev;
+   struct max77686_dev *iodev;
+   int num_regulators;
+   struct regulator_dev **rdev;
+   int ramp_delay; /* index of ramp_delay */
+
+   /*GPIO-DVS feature is not enabled with the
+*current version of MAX77686 driver.*/
+};
+
+struct voltage_map_desc {
+   int min;
+   int max;
+   int step;
+   unsigned int n_bits;
+};
+
+/* Voltage maps in mV */
+static const struct voltage_map_desc ldo_voltage_map_desc = {
+   .min = 800, .max = 3950,.step = 50, .n_bits = 6,
+}; /* LDO3 ~ 5, 9 ~ 14, 16 ~ 26 */
+
+static const struct voltage_map_desc ldo_low_voltage_map_desc = {
+   .min = 800, .max = 2375,.step = 25, .n_bits = 6,
+}; /* LDO1 ~ 2, 6 ~ 8, 15 */
+
+static const struct voltage_map_desc buck_dvs_voltage_map_desc = {
+   .min = 60,  .max = 3787500, .step = 12500,  .n_bits = 8,
+}; /* Buck2, 3, 4 (uV) */
+
+static const struct voltage_map_desc buck_voltage_map_desc = {
+   .min = 750, .max = 3900,.step = 50, .n_bits = 6,
+}; /* Buck1, 5 ~ 9 */
+
+static const struct voltage_map_desc *reg_voltage_map[] = {
+   [MAX77686_LDO1] = &ldo_low_voltage_map_desc,
+   [MAX77686_LDO2] = &ldo_low_voltage_map_desc,
+   [MAX77686_LDO3] = &ldo_voltage_map_desc,
+   [MAX77686_LDO4] = &ldo_voltage_map_desc,
+   [MAX77686_LDO5] = &ldo_voltage_map_desc,
+   [MAX77686_LDO6] = &ldo_low_voltage_map_desc,
+   [MAX77686_LDO7] = &ldo_low_voltage_map_desc,
+   [MAX77686_LDO8] = &ldo_low_voltage_map_desc,
+   [MAX77686_LDO9] = &ldo_voltage_map_desc,
+   [MAX77686_LDO10] = &ldo_voltage_map_desc,
+

[PATCH 1/2] mfd: Add support for MAX77686.

2012-05-09 Thread Yadwinder Singh
From: Yadwinder Singh Brar 

MAX77686 is a mulitifunction device with PMIC, RTC and Charger on chip. This
driver provides common support for accessing the device. This is initial
version of this driver that supports to enable the chip with its primary I2C
bus.It also includes IRQ and device tree support for MAX77686 chip.

Signed-off-by: Yadwinder Singh Brar 
---
 drivers/mfd/Kconfig  |   19 ++
 drivers/mfd/Makefile |1 +
 drivers/mfd/max77686-irq.c   |  258 +
 drivers/mfd/max77686.c   |  299 ++
 include/linux/mfd/max77686-private.h |  279 +++
 include/linux/mfd/max77686.h |  104 
 6 files changed, 960 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/max77686-irq.c
 create mode 100644 drivers/mfd/max77686.c
 create mode 100644 include/linux/mfd/max77686-private.h
 create mode 100644 include/linux/mfd/max77686.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 11e4438..618bc42 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -441,6 +441,25 @@ config MFD_MAX8998
  additional drivers must be enabled in order to use the functionality
  of the device.
 
+config MFD_MAX77686
+   bool "Maxim Semiconductor MAX77686 PMIC Support"
+   depends on I2C=y && GENERIC_HARDIRQS
+   select MFD_CORE
+   help
+ Say yes here to support for Maxim Semiconductor MAX77686.
+ This is a Power Management IC with RTC on chip.
+ This driver provides common support for accessing the device;
+ additional drivers must be enabled in order to use the functionality
+ of the device.
+
+config DEBUG_MAX77686
+   bool "MAX77686 PMIC debugging"
+   depends on MFD_MAX77686
+   help
+ Say yes, if you need enable debug messages in MFD_MAX77686 driver.
+ Further for enabling/disabling particular type of debug messages set
+ max77686_debug_mask accordingly.
+
 config MFD_S5M_CORE
bool "SAMSUNG S5M Series Support"
depends on I2C=y && GENERIC_HARDIRQS
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 05fa538..19ecca9 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -79,6 +79,7 @@ max8925-objs  := max8925-core.o max8925-i2c.o
 obj-$(CONFIG_MFD_MAX8925)  += max8925.o
 obj-$(CONFIG_MFD_MAX8997)  += max8997.o max8997-irq.o
 obj-$(CONFIG_MFD_MAX8998)  += max8998.o max8998-irq.o
+obj-$(CONFIG_MFD_MAX77686) += max77686.o max77686-irq.o
 
 pcf50633-objs  := pcf50633-core.o pcf50633-irq.o
 obj-$(CONFIG_MFD_PCF50633) += pcf50633.o
diff --git a/drivers/mfd/max77686-irq.c b/drivers/mfd/max77686-irq.c
new file mode 100644
index 000..b40a46d
--- /dev/null
+++ b/drivers/mfd/max77686-irq.c
@@ -0,0 +1,258 @@
+/*
+ * max77686-irq.c - Interrupt controller support for MAX77686
+ *
+ * Copyright (C) 2012 Samsung Electronics Co.Ltd
+ * Chiwoong Byun 
+ *
+ * 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; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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
+ *
+ * This driver is based on max8997-irq.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IRQ_GRP (data->hwirq >> 3)
+#define IRQ_MASK (1 << (data->hwirq & 7))
+
+int max77686_debug_mask = MAX77686_DEBUG_INFO; /* enable debug prints */
+
+static const u8 max77686_mask_reg[] = {
+   [PMIC_INT1] = MAX77686_REG_INT1MSK,
+   [PMIC_INT2] = MAX77686_REG_INT2MSK,
+   [RTC_INT]   = MAX77686_RTC_INTM,
+};
+
+static struct i2c_client *max77686_get_i2c(struct max77686_dev *max77686,
+  enum max77686_irq_source src)
+{
+   switch (src) {
+   case PMIC_INT1...PMIC_INT2:
+   return max77686->i2c;
+   case RTC_INT:
+   return max77686->rtc;
+   default:
+   return ERR_PTR(-EINVAL);
+   }
+}
+
+static void max77686_irq_lock(struct irq_data *data)
+{
+   struct max77686_dev *max77686 = irq_get_chip_data(data->irq);
+
+   mutex_lock(&max77686->irqlock);
+}
+
+static void max77686_irq_sync_unlock(struct irq_data *data)
+{
+   struct max77686_dev *max77686 = irq_get_chip_data(data->irq);
+   int i;
+
+   for (i = 0; i < MAX77686_IRQ_GROUP_NR; i++) {
+  

[PATCH 0/2] regulator: add initial suport for max77686

2012-05-09 Thread Yadwinder Singh
This patch series adds support for max77686 which is a multifunction device 
which
includes regulator (pmic), rtc and charger sub-blocks within it. The support for
mfd driver and regulator driver are added by this patch series. This patch 
series
also includes device tree and irqdomain support for mfd and regulator portions.

Thanks,
Yadwinder Singh

--
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 06/10] ARM: Samsung: Modify s3c64xx_spi{0|1|2}_set_platdata function

2012-05-09 Thread Thomas Abraham
On 9 May 2012 22:33, Mark Brown  wrote:
> On Wed, May 09, 2012 at 10:22:26PM +0800, Thomas Abraham wrote:
>> On 9 May 2012 18:55, Mark Brown  wrote:
>
>> > Yes, that's the normal way of handling this and is actually what the
>> > code was originally doing - there's a bunch of ifdefed devices in
>> > plat-samsung/devs.c.  You usually have to do this anyway as the IPs move
>> > about so the resources need changing.
>
>> In addition to the setting the name, the platform data is also
>> assigned at runtime. Adding multiple platform devices means that we
>> add lot more code in setting up the platform data. And since we are
>> moving towards adopting dt, these would anyway go away when we have
>> all the platforms migrated to dt.
>
> With your refactoring the only platform data that's left is the /CS?

The nCS line is not part of the platform data. It is part of the spi
board info. There are three elements in the platform data after the
re-factoring which is used on only non-dt platforms - (a) the clock
source number to be selected as the bus clock in the spi controller's
clock mux, (b) the number of nCS lines emulated by the controller and
(c) the gpio setup callback function. With device tree, all this
information is obtained from the device tree.

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 06/10] ARM: Samsung: Modify s3c64xx_spi{0|1|2}_set_platdata function

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 10:22:26PM +0800, Thomas Abraham wrote:
> On 9 May 2012 18:55, Mark Brown  wrote:

> > Yes, that's the normal way of handling this and is actually what the
> > code was originally doing - there's a bunch of ifdefed devices in
> > plat-samsung/devs.c.  You usually have to do this anyway as the IPs move
> > about so the resources need changing.

> In addition to the setting the name, the platform data is also
> assigned at runtime. Adding multiple platform devices means that we
> add lot more code in setting up the platform data. And since we are
> moving towards adopting dt, these would anyway go away when we have
> all the platforms migrated to dt.

With your refactoring the only platform data that's left is the /CS?


signature.asc
Description: Digital signature


Re: [PATCH 10/10] spi: s3c64xx: add device tree support

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 10:13:28PM +0800, Thomas Abraham wrote:
> On 9 May 2012 17:07, Mark Brown  wrote:
> > On Wed, May 09, 2012 at 03:34:54AM +0530, Thomas Abraham wrote:

> >> +- gpios: The gpio specifier for clock, mosi and miso interface lines (in 
> >> no
> >> +  particular order). The format of the gpio specifier depends on the gpio
> >> +  controller.

> > This seems odd...  This isn't a bitbanging controller, and surely the
> > driver will need to know which signal is which?  I suspect this is
> > actually for pinmux rather than to identify the signals but that should
> > at least be made clear and really should be being done using the pinmux
> > API.

> The driver retrieves the list of gpio's that it is allowed to use. The
> gpio numbers for miso, mosi and clk are mandatory but the order in
> which they are specified is not important since the driver never needs
> to which gpio is which interface line. I agree the pinmux api should
> be used here, but the call to pinmux api would be a incremental change
> here, not changing the code this patch is adding.

I'd suggest just specifying the order - someone might want to use it
later for some reason and it's not really a hardship for someone to use
it.  Avoids any "how does that work?" questions like I had.

> >> +  - samsung,spi-cs-gpio: A gpio specifier that specifies the gpio line 
> >> used as
> >> +    the slave select line by the spi controller. The format of the gpio
> >> +    specifier depends on the gpio controller.

> > We should really have a binding for this at the SPI level (and ideally
> > some code to manage setting the GPIO too) - it's pretty common to use a
> > GPIO as /CS.

> The existing implementations vary in the way the nCS gpio lines are
> specified. For some controllers, the nCS gpio's are included in the
> spi device node whereas in this implementation, the nCS gpio is listed
> in the spi slave device node.

Yeah, I know.  I'm saying we should try to come up with a binding for
this that can be used by new SPI contollers going forward so things are
consistent.


signature.asc
Description: Digital signature


Re: [PATCH 05/10] ARM: Samsung: Update the device names for spi clock lookup

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 09:40:26PM +0800, Thomas Abraham wrote:
> On 9 May 2012 16:52, Mark Brown  wrote:

> > This should've been squashed into the patch that updated to use driver
> > data in order to avoid breaking bisection.

> This patch updates clock devname in the platform code and the
> modifications to use driver data is in the driver file. So these two
> changes are kept separate to allow them to be applied to their
> respective trees.

This means that bisection will be broken - anything with only one tree
won't be able to load the SPI driver successfully until it's merged
with the other which isn't ideal.


signature.asc
Description: Digital signature


Re: [PATCH 06/10] ARM: Samsung: Modify s3c64xx_spi{0|1|2}_set_platdata function

2012-05-09 Thread Thomas Abraham
On 9 May 2012 18:55, Mark Brown  wrote:
> On Wed, May 09, 2012 at 11:10:14AM +0200, Heiko Stübner wrote:
>
>> Similar to the adc and rtc driver, all Samsung platforms reuse a common
>> platform-device definition for the s3c64xx-spi and simply will set the 
>> correct
>> name when the machine type is determined during boot.
>
> Right, that doesn't mean this is a great way of doing things, though -
> it's been a frequent source of errors in the past and is painful to
> debug as the data structures get rewritten during boot which is a bit of
> a surprise.
>
>> The alternative is creating a mulitude of platform devices for each possible
>> machine type using this driver.
>
> Yes, that's the normal way of handling this and is actually what the
> code was originally doing - there's a bunch of ifdefed devices in
> plat-samsung/devs.c.  You usually have to do this anyway as the IPs move
> about so the resources need changing.

In addition to the setting the name, the platform data is also
assigned at runtime. Adding multiple platform devices means that we
add lot more code in setting up the platform data. And since we are
moving towards adopting dt, these would anyway go away when we have
all the platforms migrated to dt.

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 10/10] spi: s3c64xx: add device tree support

2012-05-09 Thread Thomas Abraham
On 9 May 2012 17:07, Mark Brown  wrote:
> On Wed, May 09, 2012 at 03:34:54AM +0530, Thomas Abraham wrote:
>
>> +- gpios: The gpio specifier for clock, mosi and miso interface lines (in no
>> +  particular order). The format of the gpio specifier depends on the gpio
>> +  controller.
>
> This seems odd...  This isn't a bitbanging controller, and surely the
> driver will need to know which signal is which?  I suspect this is
> actually for pinmux rather than to identify the signals but that should
> at least be made clear and really should be being done using the pinmux
> API.

The driver retrieves the list of gpio's that it is allowed to use. The
gpio numbers for miso, mosi and clk are mandatory but the order in
which they are specified is not important since the driver never needs
to which gpio is which interface line. I agree the pinmux api should
be used here, but the call to pinmux api would be a incremental change
here, not changing the code this patch is adding.

>
>> +  - samsung,spi-cs-gpio: A gpio specifier that specifies the gpio line used 
>> as
>> +    the slave select line by the spi controller. The format of the gpio
>> +    specifier depends on the gpio controller.
>
> We should really have a binding for this at the SPI level (and ideally
> some code to manage setting the GPIO too) - it's pretty common to use a
> GPIO as /CS.

The existing implementations vary in the way the nCS gpio lines are
specified. For some controllers, the nCS gpio's are included in the
spi device node whereas in this implementation, the nCS gpio is listed
in the spi slave device node.

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 05/10] ARM: Samsung: Update the device names for spi clock lookup

2012-05-09 Thread Thomas Abraham
On 9 May 2012 16:52, Mark Brown  wrote:
> On Wed, May 09, 2012 at 03:34:49AM +0530, Thomas Abraham wrote:
>> With the addition of platform specific driver data in the spi-s3c64xx
>> driver, the device name of spi controllers are changed. Accordingly,
>> update the device name of spi clocks instances.
>
> This should've been squashed into the patch that updated to use driver
> data in order to avoid breaking bisection.

This patch updates clock devname in the platform code and the
modifications to use driver data is in the driver file. So these two
changes are kept separate to allow them to be applied to their
respective trees.

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 00/33] Use common macro to define resources

2012-05-09 Thread Kukjin Kim
Russell King - ARM Linux wrote:
> 
> On Wed, Apr 25, 2012 at 11:21:47AM +, Arnd Bergmann wrote:
> > Note that the point of the DEFINE_RES_*() macros is really to prevent
> > people from coming up with new silly macros to do the same thing, as
> > we've had in the past.
> 
> One of the other reaons was to stop the stream of resources with wrong
> endings (because people kept thinking it was exclusive rather than
> inclusive) - and the resulting stream of additional patches to fix those
> errors.
> 
> The legacy platforms do have those kinds of errors - when I converted
> SA11x0 as part of a previous patch set, I found a number suffering from
> this error.  (Were any found in this patch set?  I've not read through
> it.)
> 
> I think if this becomes a small %age of the change to Samsung stuff,
> and Kukjin is happy with it, it should go in.

Yes, agree. I will apply this whole series.
Thanks.

Best regards,
Kgene.
--
Kukjin Kim , 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


[PATCH] ARM: EXYNOS: PD: Fix duplicate variable

2012-05-09 Thread Sangwook Lee
struct generic_pm_domain already has a field for name. Use that field
instead of creating another field in struct exynos_pm_domain

Signed-off-by: Sangwook Lee 
---
 arch/arm/mach-exynos/pm_domains.c |7 +++
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-exynos/pm_domains.c 
b/arch/arm/mach-exynos/pm_domains.c
index 13b3068..4d2563c 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -28,7 +28,6 @@
  */
 struct exynos_pm_domain {
void __iomem *base;
-   char const *name;
bool is_off;
struct generic_pm_domain pd;
 };
@@ -75,10 +74,10 @@ static int exynos_pd_power_off(struct generic_pm_domain 
*domain)
 #define EXYNOS_GPD(PD, BASE, NAME) \
 static struct exynos_pm_domain PD = {  \
.base = (void __iomem *)BASE,   \
-   .name = NAME,   \
.pd = { \
.power_off = exynos_pd_power_off,   \
.power_on = exynos_pd_power_on, \
+   .name = NAME,   \
},  \
 }
 
@@ -99,7 +98,7 @@ static __init int exynos_pm_dt_parse_domains(void)
 
if (of_get_property(np, "samsung,exynos4210-pd-off", NULL))
pd->is_off = true;
-   pd->name = np->name;
+   pd->pd.name = (char *)np->name;
pd->base = of_iomap(np, 0);
pd->pd.power_off = exynos_pd_power_off;
pd->pd.power_on = exynos_pd_power_on;
@@ -122,7 +121,7 @@ static __init void exynos_pm_add_dev_to_genpd(struct 
platform_device *pdev,
if (pm_genpd_add_device(&pd->pd, &pdev->dev))
pr_info("%s: error in adding %s device to %s power"
"domain\n", __func__, dev_name(&pdev->dev),
-   pd->name);
+   pd->pd.name);
}
 }
 
-- 
1.7.4.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


Re: [PATCH v3 0/6] thermal: exynos: Add kernel thermal support for exynos platform

2012-05-09 Thread Amit Kachhap
On 9 May 2012 01:36, Zhang, Rui  wrote:
> Hi, Amit,
>
> Sorry for the late response as I'm in a travel recently.
>
> I think the generic cpufreq cooling patches are good.
>
> But about the THERMAL_TRIP_STATE_INSTANCE patch, what I'd like to see is that
> 1. from thermal zone point of view, when the temperature is higher than a 
> trip point, either ACTIVE or PASSIVE, what we should do is to set device 
> cooling state to cur_state+1, right?
>  The only difference is that if we should take passive cooling actions or 
> active cooling actions based on the policy.
>  So my question would be if it is possible to combine these two kind of trip 
> points together. Maybe something like this:
>
>  In thermal_zone_device_update(),
>
>  ...
>  case THERMAL_TRIP_PASSIVE:
>      if (passive cooling not allowed)
>           continue;
>      if (tc1)
>           thermal_zone_device_passive();
>      else
>           thermal_set_cooling_state();
>      break;
>  case THERMAL_TRIP_ACTIVE:
>     if (active cooling not allowed)
>          continue;
>     thermal_set_cooling_state();
>     break;
>  ...
>
> and thermal_set_cooling_state() {
>   list_for_each_entry(instance, &tz->cooling_devices, node) {
>      if (instance->trip != count)
>         continue;
>
>      cdev = instance->cdev;
>
>      if (temp >= trip_temp)
>          cdev->ops->set_cur_state(cdev, 1);
>      else
>          cdev->ops->set_cur_state(cdev, 0);
>   }
> }
>
> 2. use only one cooling_device instance for a thermal_zone, and introduce 
> cdev->trips which shows the trip points this cooling device is bind to.
>  And we may use multiple cooling devices states for one active trip point.
>
> Then, thermal_set_cooling_state() would be look like
>
> list_for_each_entry(instance, &tz->cooling_devices, node) {
>   cdev = instance->cdev;
>   /* check whether this cooling device is bind to the trip point */
>   if (!(cdev->trips & 1<      continue;
>   cdev->ops->get_max_state(cdev, &max_state);
>   cdev->ops->get_cur_state(cdev, &cur_state);
>
>   if (temp >= trip_temp) {
>      if (cur_state++ <= max_state))
>        cdev->ops->set_cur_state(cdev, cur_state);
>   } else if ((temp < trip_temp) && (cur_state-- >= 0))
>      cdev->ops->set_cur_state(cdev, cur_state);
>                        }
> }

Hi Rui,

The above implementation  also cools instance based cooling devices
like passive trip type. I need some changes on top of your
implementation such as,
thermal_set_cooling_state() {
list_for_each_entry(instance, &tz->cooling_devices,
 node) {
cdev = instance->cdev;
if (!cdev->trips & 1ops->get_max_state(cdev, &max_state);
 if ((temp >= trip_temp) && (inst_id <= max_state))
  cdev->ops->set_cur_state(cdev, inst_id);
 else if ((temp < trip_temp) && (--inst_id >= 0))
   cdev->ops->set_cur_state(cdev, inst_id);
 }
}

I agree with you that the instance based trip types violates the
concept like reading the cur_state and do cur_state++/cur_state--
depending upon threshold increase or decrease because it needs the
state_id/inst_id.
I am actually thinking of dropping this new trip type and use the
existing THERMAL_TRIP_ACTIVE because there is so much logic in
calculating the state_id. The only flip side of using TRIP_ACTIVE is
that I need to create so many cooling devices.

Thanks,
Amit D
>
> With these two things, I think the WARN_ZONE AND MONITOR_ZONE can be 
> registered as two PASSIVE trip points in the generic thermal layer, right?
> Actually, this is one thing in my TODO list. And I'd glad to make it high 
> priority if this solves the problem you have.
>
> Thanks,
> rui
>
> -Original Message-
> From: linux-acpi-ow...@vger.kernel.org 
> [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Amit Daniel Kachhap
> Sent: Tuesday, May 08, 2012 9:18 AM
> To: a...@linux-foundation.org; linux...@lists.linux-foundation.org
> Cc: R, Durgadoss; linux-a...@vger.kernel.org; l...@kernel.org; Zhang, Rui; 
> amit.kach...@linaro.org; linaro-...@lists.linaro.org; 
> linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
> linux-samsung-soc@vger.kernel.org; patc...@linaro.org
> Subject: [PATCH v3 0/6] thermal: exynos: Add kernel thermal support for 
> exynos platform
> Importance: High
>
> Hi Andrew,
>
> This patchset introduces a new generic cooling device based on cpufreq that 
> can be used on non-ACPI platforms. As a proof of concept, we have drivers for 
> the following platforms using this mechanism now:
>
>  * TI OMAP (git://git.linaro.org/people/amitdanielk/linux.git 
> omap4460_thermal)
>  * Samsung Exynos (Exynos4 and Exynos5) in the current patchset.
>  * Freescale i.MX (git://git.linaro.org/people/amitdanielk/linux.git 
> imx6q_thermal)
>
> These patches have been reviewed by Rui Zhang 
> (https://lkml.org/lkml/2012/4/9/448)
> who seems to agree with them in principle, but I haven't had a

RE: [PATCH v2 06/33] ARM: S3C24XX: dev-uart: Use common macro to define resources

2012-05-09 Thread Kukjin Kim
Heiko Stübner wrote:
> 
> From: Tushar Behera 
> 
> CC: Ben Dooks 
> CC: Kukjin Kim 
> Signed-off-by: Tushar Behera 
> Signed-off-by: Heiko Stuebner 
> ---
> changes since v1: update the patch to apply against the moved dev-uart
> devices
> As this patch does not touch anything else apart from the uart devices it
> could
> also be applied on top of the movement patches directly
> 
>  arch/arm/mach-s3c24xx/common.c |   56
+++-
> ---
>  1 files changed, 16 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/arm/mach-s3c24xx/common.c b/arch/arm/mach-
> s3c24xx/common.c
> index d42423a..56cdd34 100644
> --- a/arch/arm/mach-s3c24xx/common.c
> +++ b/arch/arm/mach-s3c24xx/common.c
> @@ -241,55 +241,31 @@ void __init s3c24xx_init_io(struct map_desc
> *mach_desc, int size)
>  /* Serial port registrations */
> 
>  static struct resource s3c2410_uart0_resource[] = {
> - [0] = {
> - .start = S3C2410_PA_UART0,
> - .end   = S3C2410_PA_UART0 + 0x3fff,
> - .flags = IORESOURCE_MEM,
> - },
> - [1] = {
> - .start = IRQ_S3CUART_RX0,
> - .end   = IRQ_S3CUART_ERR0,
> - .flags = IORESOURCE_IRQ,
> - }
> + [0] = DEFINE_RES_MEM(S3C2410_PA_UART0, SZ_16K),
> + [1] = DEFINE_RES_NAMED(IRQ_S3CUART_RX0, \
> + IRQ_S3CUART_ERR0 - IRQ_S3CUART_RX0 + 1, \
> + NULL, IORESOURCE_IRQ)
>  };
> 
>  static struct resource s3c2410_uart1_resource[] = {
> - [0] = {
> - .start = S3C2410_PA_UART1,
> - .end   = S3C2410_PA_UART1 + 0x3fff,
> - .flags = IORESOURCE_MEM,
> - },
> - [1] = {
> - .start = IRQ_S3CUART_RX1,
> - .end   = IRQ_S3CUART_ERR1,
> - .flags = IORESOURCE_IRQ,
> - }
> + [0] = DEFINE_RES_MEM(S3C2410_PA_UART1, SZ_16K),
> + [1] = DEFINE_RES_NAMED(IRQ_S3CUART_RX1, \
> + IRQ_S3CUART_ERR1 - IRQ_S3CUART_RX1 + 1, \
> + NULL, IORESOURCE_IRQ)
>  };
> 
>  static struct resource s3c2410_uart2_resource[] = {
> - [0] = {
> - .start = S3C2410_PA_UART2,
> - .end   = S3C2410_PA_UART2 + 0x3fff,
> - .flags = IORESOURCE_MEM,
> - },
> - [1] = {
> - .start = IRQ_S3CUART_RX2,
> - .end   = IRQ_S3CUART_ERR2,
> - .flags = IORESOURCE_IRQ,
> - }
> + [0] = DEFINE_RES_MEM(S3C2410_PA_UART2, SZ_16K),
> + [1] = DEFINE_RES_NAMED(IRQ_S3CUART_RX2, \
> + IRQ_S3CUART_ERR2 - IRQ_S3CUART_RX2 + 1, \
> + NULL, IORESOURCE_IRQ)
>  };
> 
>  static struct resource s3c2410_uart3_resource[] = {
> - [0] = {
> - .start = S3C2443_PA_UART3,
> - .end   = S3C2443_PA_UART3 + 0x3fff,
> - .flags = IORESOURCE_MEM,
> - },
> - [1] = {
> - .start = IRQ_S3CUART_RX3,
> - .end   = IRQ_S3CUART_ERR3,
> - .flags = IORESOURCE_IRQ,
> - },
> + [0] = DEFINE_RES_MEM(S3C2443_PA_UART3, SZ_16K),
> + [1] = DEFINE_RES_NAMED(IRQ_S3CUART_RX3, \
> + IRQ_S3CUART_ERR3 - IRQ_S3CUART_RX3 + 1, \
> + NULL, IORESOURCE_IRQ)
>  };
> 
>  struct s3c24xx_uart_resources s3c2410_uart_resources[] __initdata = {
> --
> 1.7.5.4

OK, will apply.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim , 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 v2 0/4] ARM: S3C24XX: move some code from plat to mach directory

2012-05-09 Thread Kukjin Kim
Heiko Stübner wrote:
> 
> Again the series of small moves of common code from the plat-s3c24xx to
> the
> mach-s3c24xx directory.
> 
> This time without the wrong handled irq.c . Hopefully I'll get time to do
> it
> properly later.
> 
> As the dev-uart.c move breaks the resource patch by Tushar Behera, I
> updated
> the patch to fix the correct code and will submit it as a v2 to the
> resource
> series.
> 
> Heiko Stuebner (4):
>   ARM: S3C24XX: move plat-s3c24xx/cpu.c
>   ARM: S3C24XX: move plat-s3c24xx/dev-uart.c into common.c
>   ARM: S3C24XX: move common power-management code to mach-s3c24xx
>   ARM: S3C24XX: move common clock init into common.c
> 
>  arch/arm/mach-s3c24xx/Makefile |6 +
>  .../{plat-s3c24xx/cpu.c => mach-s3c24xx/common.c}  |   93
> ++-
>  arch/arm/{plat-s3c24xx => mach-s3c24xx}/irq-pm.c   |0
>  arch/arm/{plat-s3c24xx => mach-s3c24xx}/pm.c   |0
>  arch/arm/{plat-s3c24xx => mach-s3c24xx}/sleep.S|0
>  arch/arm/plat-s3c24xx/Makefile |6 -
>  arch/arm/plat-s3c24xx/clock.c  |   59 
>  arch/arm/plat-s3c24xx/dev-uart.c   |  100 
> 
>  8 files changed, 98 insertions(+), 166 deletions(-)
>  rename arch/arm/{plat-s3c24xx/cpu.c => mach-s3c24xx/common.c} (75%)
>  rename arch/arm/{plat-s3c24xx => mach-s3c24xx}/irq-pm.c (100%)
>  rename arch/arm/{plat-s3c24xx => mach-s3c24xx}/pm.c (100%)
>  rename arch/arm/{plat-s3c24xx => mach-s3c24xx}/sleep.S (100%)
>  delete mode 100644 arch/arm/plat-s3c24xx/clock.c
>  delete mode 100644 arch/arm/plat-s3c24xx/dev-uart.c
> 
> --
> 1.7.5.4

Thanks for your re-posting.
Looks better. Will apply this series instead.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim , 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


[PATCH] ARM: EXYNOS: fix the hotplug for Cortex-A15

2012-05-09 Thread Kukjin Kim
From: Changhwan Youn 

The sequence of cpu_enter_lowpower() for Cortex-A15
is different from the sequence for Cortex-A9.
This patch implements cpu_enter_lowpower() for EXYNOS5
SoC which has Cortex-A15 cores.

Signed-off-by: Changhwan Youn 
Cc: Russell King 
Signed-off-by: Kukjin Kim 
---
Note, I didn't see common code for fix of hotplug for Cortex-A15.
If any regarding patch, please kindly let me know, thanks.

 arch/arm/mach-exynos/hotplug.c |   39 +--
 1 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index 9c17a0a..2a444b1 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -21,9 +21,11 @@
 
 #include 
 
+#include 
+
 extern volatile int pen_release;
 
-static inline void cpu_enter_lowpower(void)
+static inline void cpu_enter_lowpower_a9(void)
 {
unsigned int v;
 
@@ -45,6 +47,35 @@ static inline void cpu_enter_lowpower(void)
  : "cc");
 }
 
+static inline void cpu_enter_lowpower_a15(void)
+{
+   unsigned int v;
+
+   asm volatile(
+   "   mrc p15, 0, %0, c1, c0, 0\n"
+   "   bic %0, %0, %1\n"
+   "   mcr p15, 0, %0, c1, c0, 0\n"
+ : "=&r" (v)
+ : "Ir" (CR_C)
+ : "cc");
+
+   flush_cache_all();
+
+   asm volatile(
+   /*
+   * Turn off coherency
+   */
+   "   mrc p15, 0, %0, c1, c0, 1\n"
+   "   bic %0, %0, %1\n"
+   "   mcr p15, 0, %0, c1, c0, 1\n"
+   : "=&r" (v)
+   : "Ir" (0x40)
+   : "cc");
+
+   isb();
+   dsb();
+}
+
 static inline void cpu_leave_lowpower(void)
 {
unsigned int v;
@@ -112,7 +143,11 @@ void platform_cpu_die(unsigned int cpu)
/*
 * we're ready for shutdown now, so do it
 */
-   cpu_enter_lowpower();
+   if (soc_is_exynos5250())
+   cpu_enter_lowpower_a15();
+   else
+   cpu_enter_lowpower_a9();
+
platform_do_lowpower(cpu, &spurious);
 
/*
-- 
1.7.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


RE: [PATCH] ARM: exynos: Adapt to cpuidle core time keeping and irq enable

2012-05-09 Thread Kukjin Kim
Daniel Lezcano wrote:
> 
> On 05/09/2012 01:53 PM, Kukjin Kim wrote:
> > amit kachhap wrote:
> >>
> >> Hi Mr Kukjin,
> >>
> >> Any comment or update about this patch?
> >>
> > I'm not sure we don't need to check the idle_time?
> > Others, looks ok to me.
> 
> Hi,
> 
> may be I misunderstood your question but the behavior is not changed
> here, just the code is refactored and using a common routine where the
> time is computed.
> 
> The idle_time computation is done by the cpuidle core now if the
> en_core_tk_irqen flag is enabled. That allows to remove a lot of
> duplicated code across the cpuidle drivers.
> 
OK, I see and checked. Daniel, thanks.

Amit, this looks ok to me, will apply.
Thanks.

Best regards,
Kgene.
--
Kukjin Kim , 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] ARM: exynos: Adapt to cpuidle core time keeping and irq enable

2012-05-09 Thread Daniel Lezcano

On 05/09/2012 01:53 PM, Kukjin Kim wrote:

amit kachhap wrote:


Hi Mr Kukjin,

Any comment or update about this patch?


I'm not sure we don't need to check the idle_time?
Others, looks ok to me.


Hi,

may be I misunderstood your question but the behavior is not changed 
here, just the code is refactored and using a common routine where the 
time is computed.


The idle_time computation is done by the cpuidle core now if the 
en_core_tk_irqen flag is enabled. That allows to remove a lot of 
duplicated code across the cpuidle drivers.


Thanks
  -- Daniel
--
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: exynos: Adapt to cpuidle core time keeping and irq enable

2012-05-09 Thread Kukjin Kim
amit kachhap wrote:
> 
> Hi Mr Kukjin,
> 
> Any comment or update about this patch?
> 
I'm not sure we don't need to check the idle_time?
Others, looks ok to me.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> Regards,
> Amit Daniel
> 
> On Mon, Apr 30, 2012 at 10:13 PM, Rob Lee  wrote:
> > On Wed, Apr 25, 2012 at 9:44 AM, Daniel Lezcano
> >  wrote:
> >> On 04/25/2012 02:11 PM, Amit Daniel Kachhap wrote:
> >>>
> >>> This patch enables core cpuidle timekeeping and irq enabling and
> >>> remove those redundant parts from the exynos cpuidle drivers
> >>>
> >>> CC: Daniel Lezcano
> >>> CC: Robert Lee
> >>> Signed-off-by: Amit Daniel
> >>> ---
> >>>  arch/arm/mach-exynos/cpuidle.c |   53
> >>> ---
> >>>  1 files changed, 6 insertions(+), 47 deletions(-)
> >>>
> >>> diff --git a/arch/arm/mach-exynos/cpuidle.c
> >>> b/arch/arm/mach-exynos/cpuidle.c
> >>> index 33ab4e7..26dac28 100644
> >>> --- a/arch/arm/mach-exynos/cpuidle.c
> >>> +++ b/arch/arm/mach-exynos/cpuidle.c
> >>> @@ -20,6 +20,7 @@
> >>>  #include
> >>>  #include
> >>>  #include
> >>> +#include
> >>>  #include
> >>>  #include
> >>>
> >>> @@ -34,22 +35,12 @@
> >>>
> >>>  #define S5P_CHECK_AFTR0xFCBA0D10
> >>>
> >>> -static int exynos4_enter_idle(struct cpuidle_device *dev,
> >>> -   struct cpuidle_driver *drv,
> >>> - int index);
> >>>  static int exynos4_enter_lowpower(struct cpuidle_device *dev,
> >>>struct cpuidle_driver *drv,
> >>>int index);
> >>>
> >>>  static struct cpuidle_state exynos4_cpuidle_set[] __initdata = {
> >>> -   [0] = {
> >>> -   .enter  = exynos4_enter_idle,
> >>> -   .exit_latency   = 1,
> >>> -   .target_residency   = 10,
> >>> -   .flags  = CPUIDLE_FLAG_TIME_VALID,
> >>> -   .name   = "C0",
> >>> -   .desc   = "ARM clock gating(WFI)",
> >>> -   },
> >>> +   [0] = ARM_CPUIDLE_WFI_STATE,
> >>>[1] = {
> >>>.enter  = exynos4_enter_lowpower,
> >>>.exit_latency   = 300,
> >>> @@ -63,8 +54,9 @@ static struct cpuidle_state exynos4_cpuidle_set[]
> >>> __initdata = {
> >>>  static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
> >>>
> >>>  static struct cpuidle_driver exynos4_idle_driver = {
> >>> -   .name   = "exynos4_idle",
> >>> -   .owner  = THIS_MODULE,
> >>> +   .name   = "exynos4_idle",
> >>> +   .owner  = THIS_MODULE,
> >>> +   .en_core_tk_irqen   = 1,
> >>>  };
> >>>
> >>>  /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
> >>> @@ -103,13 +95,8 @@ static int exynos4_enter_core0_aftr(struct
> >>> cpuidle_device *dev,
> >>>struct cpuidle_driver *drv,
> >>>int index)
> >>>  {
> >>> -   struct timeval before, after;
> >>> -   int idle_time;
> >>>unsigned long tmp;
> >>>
> >>> -   local_irq_disable();
> >>> -   do_gettimeofday(&before);
> >>> -
> >>>exynos4_set_wakeupmask();
> >>>
> >>>/* Set value of power down register for aftr mode */
> >>> @@ -150,34 +137,6 @@ static int exynos4_enter_core0_aftr(struct
> >>> cpuidle_device *dev,
> >>>/* Clear wakeup state register */
> >>>__raw_writel(0x0, S5P_WAKEUP_STAT);
> >>>
> >>> -   do_gettimeofday(&after);
> >>> -
> >>> -   local_irq_enable();
> >>> -   idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
> >>> -   (after.tv_usec - before.tv_usec);
> >>> -
> >>> -   dev->last_residency = idle_time;
> >>> -   return index;
> >>> -}
> >>> -
> >>> -static int exynos4_enter_idle(struct cpuidle_device *dev,
> >>> -   struct cpuidle_driver *drv,
> >>> -   int index)
> >>> -{
> >>> -   struct timeval before, after;
> >>> -   int idle_time;
> >>> -
> >>> -   local_irq_disable();
> >>> -   do_gettimeofday(&before);
> >>> -
> >>> -   cpu_do_idle();
> >>> -
> >>> -   do_gettimeofday(&after);
> >>> -   local_irq_enable();
> >>> -   idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
> >>> -   (after.tv_usec - before.tv_usec);
> >>> -
> >>> -   dev->last_residency = idle_time;
> >>>return index;
> >>>  }
> >>>
> >>> @@ -192,7 +151,7 @@ static int exynos4_enter_lowpower(struct
> >>> cpuidle_device *dev,
> >>>new_index = drv->safe_state_index;
> >>>
> >>>if (new_index == 0)
> >>> -   return exynos4_enter_idle(dev, drv, new_index);
> >>> +   return arm_cpuidle_simple_enter(dev, drv, new_index);
> >>>else
> >>>  

RE: [PATCH 00/20] ARM: Samsung: Add support for Exynos5250 Rev1.0

2012-05-09 Thread Kukjin Kim
Thomas Abraham wrote:
> 
> This patch series adds support for Samsung's Exynos5250 Rev1.0. It
> includes
> fixes for device tree support, updates for rev1.0 silicon and device tree
> discovery for combiner and wakeup interrupt controller. This patch series
> depricates the existing support for Exynos5250 Rev0.0.
> 
> This patchset is based and tested on top of v3.4-rc5.
> 
> Boojin Kim (1):
>   ARM: EXYNOS: Support DMA for EXYNOS5250 SoC
> 
> Changhwan Youn (2):
>   ARM: EXYNOS: Modify the GIC physical address for static io-mapping
>   ARM: EXYNOS: Redefine IRQ_MCT_L0,1 definition
> 
> Kisoo Yu (1):
>   ARM: EXYNOS: Add pre-divider and fout mux clocks for bpll and mpll
> 
> Kukjin Kim (2):
>   ARM: EXYNOS: fix ctrlbit for exynos5_clk_pdma1
>   ARM: EXYNOS: update irqs for EXYNOS5250 evt1
> 
> Sangsu Park (1):
>   ARM: EXYNOS: add GPC4 bank instance
> 
> Thomas Abraham (13):
>   ARM: EXYNOS: Add watchdog timer clock instance
>   ARM: Exynos: Remove a new bus_type instance for Exynos5
>   of/irq: fix interrupt parent lookup procedure
>   of/irq: add retry support for interrupt controller tree initialization
>   ARM: Exynos: Add irq_domain support for interrupt combiner
>   ARM: Exynos: Add device tree support for interrupt combiner
>   ARM: Exynos: Simplify the wakeup interrupt setup code
>   ARM: Exynos: Add irq_domain support for gpio wakeup interrupts
>   ARM: Exynos: Remove arch_initcall for wakeup interrupt initialization
>   ARM: Exynos: Add device tree support for gpio wakeup interrupt
> controller
>   ARM: dts: Update device tree source files for EXYNOS5250
>   ARM: Exynos5: Add combiner, wakeup interrupt controller and ethernet
> nodes
>   ARM: Exynos5: Add AUXDATA for i2c controllers
> 
>  .../bindings/arm/samsung/interrupt-combiner.txt|   52 +++
>  arch/arm/boot/dts/exynos5250-smdk5250.dts  |   63 
>  arch/arm/boot/dts/exynos5250.dtsi  |   99 --
>  arch/arm/mach-exynos/Kconfig   |7 +-
>  arch/arm/mach-exynos/Makefile  |2 +-
>  arch/arm/mach-exynos/clock-exynos5.c   |   80 +-
>  arch/arm/mach-exynos/common.c  |  338
++--
>  arch/arm/mach-exynos/dma.c |  129 +++--
>  arch/arm/mach-exynos/include/mach/gpio.h   |9 +-
>  arch/arm/mach-exynos/include/mach/irqs.h   |   40 ++-
>  arch/arm/mach-exynos/include/mach/map.h|4 +-
>  arch/arm/mach-exynos/include/mach/regs-clock.h |2 +
>  arch/arm/mach-exynos/include/mach/regs-gpio.h  |4 +-
>  arch/arm/mach-exynos/mach-exynos5-dt.c |4 +
>  arch/arm/mach-exynos/mct.c |   17 +-
>  arch/arm/mach-exynos/pm.c  |2 +-
>  arch/arm/plat-s5p/clock.c  |   11 -
>  arch/arm/plat-samsung/Kconfig  |2 +-
>  arch/arm/plat-samsung/include/plat/cpu.h   |2 +-
>  arch/arm/plat-samsung/include/plat/dma-pl330.h |1 +
>  drivers/gpio/gpio-samsung.c|   11 +-
>  drivers/of/irq.c   |   29 ++-
>  22 files changed, 698 insertions(+), 210 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/arm/samsung/interrupt-combiner.txt
> 
> --
> 1.7.5.4

Hi,

Basically, it's ok to me on 1st~9th patches except #7 as I commented. Will
apply them but need to update #7. And I will look at the others again. If
any comments, let you know.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim , 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 07/20] ARM: EXYNOS: Add pre-divider and fout mux clocks for bpll and mpll

2012-05-09 Thread Kukjin Kim
Thomas Abraham wrote:
> 
> From: Kisoo Yu 
> 
> The fout clock of BPLL and MPLL have a selectable source in rev1 of
> EXYNOS5. The clock options are a fixed divided by 2 clock and the
> output of the PLL itself. Add support for these new clock instances.
> 
> Signed-off-by: Kisoo Yu 
> Signed-off-by: Thomas Abraham 
> [kgene@samsung.com: temporary apply this because I
> don't want to add pll stuff in each clock not commonly]

As I commented, I won't apply this because pll stuff should be added in
common file such as plat-s5p/clock.c even though the plat-s5p files moved
into plat-samsung.

> Temporary-Signed-off-by: Kukjin Kim 
> Signed-off-by: Kukjin Kim 

No. I didn't sign on this in my topic branch.

> ---
>  arch/arm/mach-exynos/clock-exynos5.c   |   73
> +++-
>  arch/arm/mach-exynos/include/mach/regs-clock.h |2 +
>  arch/arm/plat-s5p/clock.c  |   11 
>  3 files changed, 74 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/clock-exynos5.c b/arch/arm/mach-
> exynos/clock-exynos5.c
> index 846741e..7c0f810 100644
> --- a/arch/arm/mach-exynos/clock-exynos5.c
> +++ b/arch/arm/mach-exynos/clock-exynos5.c
> @@ -145,6 +145,39 @@ static struct clksrc_clk exynos5_clk_sclk_apll = {
>   .reg_div = { .reg = EXYNOS5_CLKDIV_CPU0, .shift = 24, .size = 3 },
>  };
> 
> +static struct clk clk_fout_bpll_div2 = {
> + .name   = "fout_bpll_div2",
> +};
> +
> +static struct clk *exynos5_clkset_mout_bpll_fout_list[] = {
> + [0] = &clk_fout_bpll_div2,
> + [1] = &clk_fout_bpll,
> +};
> +
> +static struct clksrc_sources exynos5_clkset_mout_bpll_fout = {
> + .sources= exynos5_clkset_mout_bpll_fout_list,
> + .nr_sources = ARRAY_SIZE(exynos5_clkset_mout_bpll_fout_list),
> +};
> +
> +static struct clksrc_clk exynos5_clk_mout_bpll_fout = {
> + .clk= {
> + .name   = "mout_bpll_fout",
> + },
> + .sources = &exynos5_clkset_mout_bpll_fout,
> + .reg_src = { .reg = EXYNOS5_PLL_DIV2_SEL, .shift = 0, .size = 1 },
> +};
> +
> +/* Possible clock sources for BPLL Mux */
> +static struct clk *clk_src_bpll_list[] = {
> + [0] = &clk_fin_bpll,
> + [1] = &exynos5_clk_mout_bpll_fout.clk,
> +};
> +
> +struct clksrc_sources clk_src_bpll = {
> + .sources= clk_src_bpll_list,
> + .nr_sources = ARRAY_SIZE(clk_src_bpll_list),
> +};
> +
>  static struct clksrc_clk exynos5_clk_mout_bpll = {
>   .clk= {
>   .name   = "mout_bpll",
> @@ -187,11 +220,43 @@ static struct clksrc_clk exynos5_clk_mout_epll = {
>   .reg_src = { .reg = EXYNOS5_CLKSRC_TOP2, .shift = 12, .size = 1 },
>  };
> 
> +static struct clk clk_fout_mpll_div2 = {
> + .name   = "fout_mpll_div2",
> +};
> +
> +static struct clk *exynos5_clkset_mout_mpll_fout_list[] = {
> + [0] = &clk_fout_mpll_div2,
> + [1] = &clk_fout_mpll,
> +};
> +
> +static struct clksrc_sources exynos5_clkset_mout_mpll_fout = {
> + .sources= exynos5_clkset_mout_mpll_fout_list,
> + .nr_sources = ARRAY_SIZE(exynos5_clkset_mout_mpll_fout_list),
> +};
> +
> +static struct clksrc_clk exynos5_clk_mout_mpll_fout = {
> + .clk= {
> + .name   = "mout_mpll_fout",
> + },
> + .sources = &exynos5_clkset_mout_mpll_fout,
> + .reg_src = { .reg = EXYNOS5_PLL_DIV2_SEL, .shift = 4, .size = 1 },
> +};
> +
> +static struct clk *exynos5_clk_src_mpll_list[] = {
> + [0] = &clk_fin_mpll,
> + [1] = &exynos5_clk_mout_mpll_fout.clk,
> +};
> +
> +struct clksrc_sources exynos5_clk_src_mpll = {
> + .sources= exynos5_clk_src_mpll_list,
> + .nr_sources = ARRAY_SIZE(exynos5_clk_src_mpll_list),
> +};
> +
>  struct clksrc_clk exynos5_clk_mout_mpll = {
>   .clk = {
>   .name   = "mout_mpll",
>   },
> - .sources = &clk_src_mpll,
> + .sources = &exynos5_clk_src_mpll,
>   .reg_src = { .reg = EXYNOS5_CLKSRC_CORE1, .shift = 8, .size = 1 },
>  };
> 
> @@ -946,10 +1011,12 @@ static struct clksrc_clk *exynos5_sysclks[] = {
>   &exynos5_clk_mout_apll,
>   &exynos5_clk_sclk_apll,
>   &exynos5_clk_mout_bpll,
> + &exynos5_clk_mout_bpll_fout,
>   &exynos5_clk_mout_bpll_user,
>   &exynos5_clk_mout_cpll,
>   &exynos5_clk_mout_epll,
>   &exynos5_clk_mout_mpll,
> + &exynos5_clk_mout_mpll_fout,
>   &exynos5_clk_mout_mpll_user,
>   &exynos5_clk_vpllsrc,
>   &exynos5_clk_sclk_vpll,
> @@ -1013,6 +1080,8 @@ static struct clk *exynos5_clks[] __initdata = {
>   &exynos5_clk_sclk_hdmi27m,
>   &exynos5_clk_sclk_hdmiphy,
>   &clk_fout_bpll,
> + &clk_fout_bpll_div2,
> + &clk_fout_mpll_div2,
>   &clk_fout_cpll,
>   &exynos5_clk_armclk,
>  };
> @@ -1178,8 +1247,10 @@ void __init_or_cpufreq exynos5_setup_clocks(void)
> 
>   clk_fout_apll.ops = &exynos5_fout_apll_ops;
>   clk_fout_bpll.rate = bpll;
> + clk_fout_bpll_div2.rate = bpll >> 1;
>   clk_fout_cpll.rat

[PATCH] ARM: SAMSUNG: Fix the value of tcnt and tcmp for PWM

2012-05-09 Thread Kukjin Kim

According to PWM hardware spec, the actual period which
has been calculated by period and input clock is same
with (tcnt + 1), so need to down count for tcnt register.
And current PWM HW checks the compare register after
tcmp++ internally in hardware.

Signed-off-by: Kukjin Kim 
---
 arch/arm/plat-samsung/pwm.c |   23 +++
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/arm/plat-samsung/pwm.c b/arch/arm/plat-samsung/pwm.c
index c559d84..88fdb1c 100644
--- a/arch/arm/plat-samsung/pwm.c
+++ b/arch/arm/plat-samsung/pwm.c
@@ -22,6 +22,7 @@
 
 #include 
 
+#include 
 #include 
 
 struct pwm_device {
@@ -215,10 +216,24 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int 
period_ns)
 
tcmp = duty_ns / tin_ns;
tcmp = tcnt - tcmp;
-   /* the pwm hw only checks the compare register after a decrement,
-  so the pin never toggles if tcmp = tcnt */
-   if (tcmp == tcnt)
-   tcmp--;
+
+   if (soc_is_s3c24xx()) {
+   /*
+* The S3C24XX PWM HW only checks the compare register after
+* a decrement, so the pin never toggles if tcmp = tcnt.
+*/
+   if (tcmp == tcnt)
+   tcmp--;
+   } else {
+   /*
+* The other PWM HW checks the compare register after tcmp++
+* internally, so needs -2 for tcmp, and the actual period
+* which has been calculated by period_ns and tin_ns is same
+* with (tcnt + 1), so need to down count for tcnt register.
+*/
+   tcmp = tcmp - 2;
+   tcnt--;
+   }
 
pwm_dbg(pwm, "tin_ns=%lu, tcmp=%ld/%lu\n", tin_ns, tcmp, tcnt);
 
-- 
1.7.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


Re: [PATCH 06/10] ARM: Samsung: Modify s3c64xx_spi{0|1|2}_set_platdata function

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 11:10:14AM +0200, Heiko Stübner wrote:

> Similar to the adc and rtc driver, all Samsung platforms reuse a common 
> platform-device definition for the s3c64xx-spi and simply will set the 
> correct 
> name when the machine type is determined during boot.

Right, that doesn't mean this is a great way of doing things, though -
it's been a frequent source of errors in the past and is painful to
debug as the data structures get rewritten during boot which is a bit of
a surprise.

> The alternative is creating a mulitude of platform devices for each possible 
> machine type using this driver.

Yes, that's the normal way of handling this and is actually what the
code was originally doing - there's a bunch of ifdefed devices in
plat-samsung/devs.c.  You usually have to do this anyway as the IPs move
about so the resources need changing.


signature.asc
Description: Digital signature


Re: [PATCH 07/10] spi: s3c64xx: Remove the 'set_level' callback from controller data

2012-05-09 Thread Jassi Brar
On 9 May 2012 14:50, Heiko Stübner  wrote:
> Am Mittwoch, 9. Mai 2012, 00:04:51 schrieb Thomas Abraham:
>> The set_level callback in the controller data, which is used to configure
>> the slave select line, cannot be supported when migrating the driver to
>> device tree based discovery. Since all the platforms currently use gpio
>> as the slave select line, this callback can be removed from the
>> controller data and replaced with call to gpio_set_value in the driver.
>
> I was quite fond of the set_level callback. On one of the machines I'm working
> on there is some sort of multiplexer (TI-sn74cbtlv3257) sitting between the
> controller and spi devices to provide support for more than one device.
> So I was doing the switch to the correct device in the set_level callback,
> which worked quite well.
>
I suppose you should still be able to do that defining virtual gpios backed
by appropriate physical mux'ing of lines ?

-jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/6] thermal: exynos: Register the tmu sensor with the kernel thermal layer

2012-05-09 Thread Amit Kachhap
On 9 May 2012 01:46, Andrew Morton  wrote:
> On Tue,  8 May 2012 21:48:17 +0530
> Amit Daniel Kachhap  wrote:
>
>> This code added creates a link between temperature sensors, linux thermal
>> framework and cooling devices for samsung exynos platform. This layer
>> monitors the temperature from the sensor and informs the generic thermal
>> layer to take the necessary cooling action.
>>
>>
>> ...
>>
>> +static void exynos_report_trigger(void)
>> +{
>> +     unsigned int i;
>> +     char data[2];
>> +     char *envp[] = { data, NULL };
>> +
>> +     if (!th_zone || !th_zone->therm_dev)
>> +             return;
>> +
>> +     thermal_zone_device_update(th_zone->therm_dev);
>> +
>> +     mutex_lock(&th_zone->therm_dev->lock);
>> +     /* Find the level for which trip happened */
>> +     for (i = 0; i < th_zone->sensor_conf->trip_data.trip_count; i++) {
>> +             if (th_zone->therm_dev->last_temperature <
>> +                     th_zone->sensor_conf->trip_data.trip_val[i] * 1000)
>> +                     break;
>> +     }
>> +
>> +     if (th_zone->mode == THERMAL_DEVICE_ENABLED) {
>> +             if (i > 0)
>> +                     th_zone->therm_dev->polling_delay = ACTIVE_INTERVAL;
>> +             else
>> +                     th_zone->therm_dev->polling_delay = IDLE_INTERVAL;
>> +     }
>> +
>> +     sprintf(data, "%u", i);
>
> yikes, if `i' exceeds 9, we have a stack scribble.  Please review this
> and at least use snprintf(...  sizeof(data)) to prevent accidents.
Ok
>
>
>> +     kobject_uevent_env(&th_zone->therm_dev->device.kobj, KOBJ_CHANGE, 
>> envp);
>> +     mutex_unlock(&th_zone->therm_dev->lock);
>> +}
>> +
>>
>> ...
>>
>> +/* Get trip temperature callback functions for thermal zone */
>> +static int exynos_get_trip_temp(struct thermal_zone_device *thermal, int 
>> trip,
>> +                             unsigned long *temp)
>> +{
>> +     if (trip < 0 || trip > 2)
>
> I don't know what `trip' does and I don't know the meaning of the
> values 0, 1 and 2.  Documentation, please.
Agreed will put their description.
>
>> +             return -EINVAL;
>> +
>> +     *temp = th_zone->sensor_conf->trip_data.trip_val[trip];
>> +     /* convert the temperature into millicelsius */
>> +     *temp = *temp * 1000;
>> +
>> +     return 0;
>> +}
>> +
>>
>> ...
>>
>> +/* Bind callback functions for thermal zone */
>> +static int exynos_bind(struct thermal_zone_device *thermal,
>> +                     struct thermal_cooling_device *cdev)
>> +{
>> +     int ret = 0;
>> +
>> +     /* if the cooling device is the one from exynos4 bind it */
>> +     if (cdev != th_zone->cool_dev[0])
>> +             return 0;
>> +
>> +     if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
>> +             pr_err("error binding cooling dev inst 0\n");
>> +             return -EINVAL;
>> +     }
>> +     if (thermal_zone_bind_cooling_device(thermal, 1, cdev)) {
>> +             pr_err("error binding cooling dev inst 1\n");
>> +             ret = -EINVAL;
>> +             goto error_bind1;
>> +     }
>
> There can never be more than two instances?
As of now it is fixed as we register only 2 zones
>
>> +     return ret;
>> +error_bind1:
>> +     thermal_zone_unbind_cooling_device(thermal, 0, cdev);
>> +     return ret;
>> +}
>> +
>>
>> ...
>>
>> +/* Get temperature callback functions for thermal zone */
>> +static int exynos_get_temp(struct thermal_zone_device *thermal,
>> +                     unsigned long *temp)
>> +{
>> +     void *data;
>> +
>> +     if (!th_zone->sensor_conf) {
>> +             pr_info("Temperature sensor not initialised\n");
>> +             return -EINVAL;
>> +     }
>> +     data = th_zone->sensor_conf->private_data;
>> +     *temp = th_zone->sensor_conf->read_temperature(data);
>> +     /* convert the temperature into millicelsius */
>> +     *temp = *temp * 1000;
>> +     return 0;
>> +}
>> +
>> +/* Operation callback functions for thermal zone */
>> +static struct thermal_zone_device_ops exynos_dev_ops = {
>
> Can it be const?  That sometimes saves space, as the table doesn't need
> to be moved into writeable storage at runtime.
Yes it can be made const
>
>> +     .bind = exynos_bind,
>> +     .unbind = exynos_unbind,
>> +     .get_temp = exynos_get_temp,
>> +     .get_mode = exynos_get_mode,
>> +     .set_mode = exynos_set_mode,
>> +     .get_trip_type = exynos_get_trip_type,
>> +     .get_trip_temp = exynos_get_trip_temp,
>> +     .get_crit_temp = exynos_get_crit_temp,
>> +};
>> +
>> +/* Register with the in-kernel thermal management */
>> +static int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
>> +{
>> +     int ret, count, tab_size;
>> +     struct freq_clip_table *tab_ptr;
>> +
>> +     if (!sensor_conf || !sensor_conf->read_temperature) {
>> +             pr_err("Temperature sensor not initialised\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     th_zone = kzalloc(sizeof(struct exynos_thermal_zone), GFP_KERNEL);
>> +     if (!th_zone) {
>> +             

Re: [PATCH 07/10] spi: s3c64xx: Remove the 'set_level' callback from controller data

2012-05-09 Thread Heiko Stübner
Am Mittwoch, 9. Mai 2012, 00:04:51 schrieb Thomas Abraham:
> The set_level callback in the controller data, which is used to configure
> the slave select line, cannot be supported when migrating the driver to
> device tree based discovery. Since all the platforms currently use gpio
> as the slave select line, this callback can be removed from the
> controller data and replaced with call to gpio_set_value in the driver.

I was quite fond of the set_level callback. On one of the machines I'm working
on there is some sort of multiplexer (TI-sn74cbtlv3257) sitting between the
controller and spi devices to provide support for more than one device.
So I was doing the switch to the correct device in the set_level callback,
which worked quite well.


In the end, I'm still ok with this consolidation, as the device line has
reached it's end of life and the second device (an epaper controller)
provides another form of access to it through the display controller of the
s3c2416.


Heiko

[1] 
https://github.com/mmind/linux-es600/blob/topic/es600-devel/arch/arm/mach-s3c24xx/common-es600.c#L1155

> Cc: Jaswinder Singh 
> Signed-off-by: Thomas Abraham 
> ---
>  arch/arm/plat-samsung/include/plat/s3c64xx-spi.h |2 --
>  drivers/spi/spi-s3c64xx.c|8 
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
> b/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h index a733ce9..48a6495
> 100644
> --- a/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
> +++ b/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h
> @@ -18,7 +18,6 @@ struct platform_device;
>   * @fb_delay: Slave specific feedback delay.
>   *Refer to FB_CLK_SEL register definition in SPI chapter.
>   * @line: Custom 'identity' of the CS line.
> - * @set_level: CS line control.
>   *
>   * This is per SPI-Slave Chipselect information.
>   * Allocate and initialize one in machine init code and make the
> @@ -27,7 +26,6 @@ struct platform_device;
>  struct s3c64xx_spi_csinfo {
>   u8 fb_delay;
>   unsigned line;
> - void (*set_level)(unsigned line_id, int lvl);
>  };
> 
>  /**
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index f6bc0e3..d84ce7f 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -406,14 +406,14 @@ static inline void enable_cs(struct
> s3c64xx_spi_driver_data *sdd, if (sdd->tgl_spi != spi) { /* if last mssg
> on diff device */
>   /* Deselect the last toggled device */
>   cs = sdd->tgl_spi->controller_data;
> - cs->set_level(cs->line,
> - spi->mode & SPI_CS_HIGH ? 0 : 1);
> + gpio_set_value(cs->line,
> + spi->mode & SPI_CS_HIGH ? 0 : 1);
>   }
>   sdd->tgl_spi = NULL;
>   }
> 
>   cs = spi->controller_data;
> - cs->set_level(cs->line, spi->mode & SPI_CS_HIGH ? 1 : 0);
> + gpio_set_value(cs->line, spi->mode & SPI_CS_HIGH ? 1 : 0);
>  }
> 
>  static int wait_for_xfer(struct s3c64xx_spi_driver_data *sdd,
> @@ -499,7 +499,7 @@ static inline void disable_cs(struct
> s3c64xx_spi_driver_data *sdd, if (sdd->tgl_spi == spi)
>   sdd->tgl_spi = NULL;
> 
> - cs->set_level(cs->line, spi->mode & SPI_CS_HIGH ? 0 : 1);
> + gpio_set_value(cs->line, spi->mode & SPI_CS_HIGH ? 0 : 1);
>  }
> 
>  static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd)

--
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 06/10] ARM: Samsung: Modify s3c64xx_spi{0|1|2}_set_platdata function

2012-05-09 Thread Heiko Stübner
Am Mittwoch, 9. Mai 2012, 10:56:17 schrieb Mark Brown:
> On Wed, May 09, 2012 at 03:34:50AM +0530, Thomas Abraham wrote:
> > +   s3c64xx_spi0_set_platdata("s3c6410-spi", NULL, 0, 1);
> 
> Shouldn't we just set the name in the struct platform_device rather than
> requiring the machine to pass it through by hand?

Similar to the adc and rtc driver, all Samsung platforms reuse a common 
platform-device definition for the s3c64xx-spi and simply will set the correct 
name when the machine type is determined during boot.

I.e. I will also have to change my out-of-tree machine stuff to set the 
s3c2443-spi name using this method.

The alternative is creating a mulitude of platform devices for each possible 
machine type using this driver.


Heiko
--
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 10/10] spi: s3c64xx: add device tree support

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 03:34:54AM +0530, Thomas Abraham wrote:

> +- gpios: The gpio specifier for clock, mosi and miso interface lines (in no
> +  particular order). The format of the gpio specifier depends on the gpio
> +  controller.

This seems odd...  This isn't a bitbanging controller, and surely the
driver will need to know which signal is which?  I suspect this is
actually for pinmux rather than to identify the signals but that should
at least be made clear and really should be being done using the pinmux
API.

> +  - samsung,spi-cs-gpio: A gpio specifier that specifies the gpio line used 
> as
> +the slave select line by the spi controller. The format of the gpio
> +specifier depends on the gpio controller.

We should really have a binding for this at the SPI level (and ideally
some code to manage setting the GPIO too) - it's pretty common to use a
GPIO as /CS.
--
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 06/10] ARM: Samsung: Modify s3c64xx_spi{0|1|2}_set_platdata function

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 03:34:50AM +0530, Thomas Abraham wrote:

> + s3c64xx_spi0_set_platdata("s3c6410-spi", NULL, 0, 1);

Shouldn't we just set the name in the struct platform_device rather than
requiring the machine to pass it through by hand?
--
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 05/10] ARM: Samsung: Update the device names for spi clock lookup

2012-05-09 Thread Mark Brown
On Wed, May 09, 2012 at 03:34:49AM +0530, Thomas Abraham wrote:
> With the addition of platform specific driver data in the spi-s3c64xx
> driver, the device name of spi controllers are changed. Accordingly,
> update the device name of spi clocks instances.

This should've been squashed into the patch that updated to use driver
data in order to avoid breaking bisection.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/6] thermal: Add generic cpufreq cooling implementation

2012-05-09 Thread Amit Kachhap
On 9 May 2012 01:46, Andrew Morton  wrote:
> On Tue,  8 May 2012 21:48:14 +0530
> Amit Daniel Kachhap  wrote:
>
>> This patch adds support for generic cpu thermal cooling low level
>> implementations using frequency scaling up/down based on the registration
>> parameters. Different cpu related cooling devices can be registered by the
>> user and the binding of these cooling devices to the corresponding
>> trip points can be easily done as the registration APIs return the
>> cooling device pointer. The user of these APIs are responsible for
>> passing clipping frequency . The drivers can also register to recieve
>> notification about any cooling action called. Even the driver can effect
>> the cooling action by modifying the default data such as freq_clip_max if
>> needed.
>>
>>
>> ...
>>
>> +struct cpufreq_cooling_device {
>> +     int id;
>> +     struct thermal_cooling_device *cool_dev;
>> +     struct freq_clip_table *tab_ptr;
>> +     unsigned int tab_size;
>> +     unsigned int cpufreq_state;
>> +     const struct cpumask *allowed_cpus;
>> +     struct list_head node;
>> +};
>
> It would be nice to document the fields.  Especially id, tab_size,
> cpufreq_state and node.  For `node' we should describe the locking for
> the list, and describe which list_head anchors this list.

Thanks Andrew for the detailed review. I will add more documentation
and post the next version shortly.
>
>> +static LIST_HEAD(cooling_cpufreq_list);
>> +static DEFINE_MUTEX(cooling_cpufreq_lock);
>> +static DEFINE_IDR(cpufreq_idr);
>> +static DEFINE_PER_CPU(unsigned int, max_policy_freq);
>> +static struct freq_clip_table *notify_table;
>> +static int notify_state;
>> +static BLOCKING_NOTIFIER_HEAD(cputherm_state_notifier_list);
>> +
>> +static int get_idr(struct idr *idr, struct mutex *lock, int *id)
>> +{
>> +     int err;
>> +again:
>> +     if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
>> +             return -ENOMEM;
>> +
>> +     if (lock)
>> +             mutex_lock(lock);
>
> The test for NULL `lock' is unneeded.  In fact the `lock' argument
> could be removed altogether - just use cooling_cpufreq_lock directly.
Agreed
>
>> +     err = idr_get_new(idr, NULL, id);
>> +     if (lock)
>> +             mutex_unlock(lock);
>> +     if (unlikely(err == -EAGAIN))
>> +             goto again;
>> +     else if (unlikely(err))
>> +             return err;
>> +
>> +     *id = *id & MAX_ID_MASK;
>> +     return 0;
>> +}
>> +
>> +static void release_idr(struct idr *idr, struct mutex *lock, int id)
>> +{
>> +     if (lock)
>> +             mutex_lock(lock);
>
> Ditto.
>
>> +     idr_remove(idr, id);
>> +     if (lock)
>> +             mutex_unlock(lock);
>> +}
>> +
>>
>> ...
>>
>> +
>> +/*Below codes defines functions to be used for cpufreq as cooling device*/
>> +static bool is_cpufreq_valid(int cpu)
>> +{
>> +     struct cpufreq_policy policy;
>> +     return !cpufreq_get_policy(&policy, cpu) ? true : false;
>
> Can use
Ok
>
>        return !cpufreq_get_policy(&policy, cpu);
>
>> +}
>> +
>> +static int cpufreq_apply_cooling(struct cpufreq_cooling_device 
>> *cpufreq_device,
>> +                             unsigned long cooling_state)
>> +{
>> +     unsigned int event, cpuid;
>> +     struct freq_clip_table *th_table;
>> +
>> +     if (cooling_state > cpufreq_device->tab_size)
>> +             return -EINVAL;
>> +
>> +     cpufreq_device->cpufreq_state = cooling_state;
>> +
>> +     /*cpufreq thermal notifier uses this cpufreq device pointer*/
>
> This code looks like it was written by two people.
>
>        /* One who does this */
>        /*And one who does this*/
>
> The first one was right.  Please go through all the comments in all the
> patches and get the layout consistent?
Sure will add more details.
>
>
>> +     notify_state = cooling_state;
>> +
>> +     if (notify_state > 0) {
>> +             th_table = &(cpufreq_device->tab_ptr[cooling_state - 1]);
>> +             memcpy(notify_table, th_table, sizeof(struct freq_clip_table));
>> +             event = CPUFREQ_COOLING_TYPE;
>> +             blocking_notifier_call_chain(&cputherm_state_notifier_list,
>> +                                             event, notify_table);
>> +     }
>> +
>> +     for_each_cpu(cpuid, cpufreq_device->allowed_cpus) {
>> +             if (is_cpufreq_valid(cpuid))
>> +                     cpufreq_update_policy(cpuid);
>> +     }
>> +
>> +     notify_state = -1;
>> +
>> +     return 0;
>> +}
>> +
>> +static int cpufreq_thermal_notifier(struct notifier_block *nb,
>> +                                     unsigned long event, void *data)
>> +{
>> +     struct cpufreq_policy *policy = data;
>> +     unsigned long max_freq = 0;
>> +
>> +     if ((event != CPUFREQ_ADJUST) || (notify_state == -1))
>
> Please document `notify_state', at its definition site.  This reader
> doesn't know what "notify_state == -1" *means*.
>
>> +             return 0;
>> +
>> +     if (notify_state > 0) {
>> +             max_freq = notify_table->freq_clip_max;
>> +
>> +

Re: [PATCH 00/10] spi: s3c64xx: add support for device tree

2012-05-09 Thread Jassi Brar
On Wed, May 9, 2012 at 3:34 AM, Thomas Abraham
 wrote:
> This patch series adds device tree based discovery support for Samsung's
> s3c64xx compatible spi controller. This is mainly tested for Exynos4210
> and Exynos5250 with onboard spi nor flash device.
>
> This patch series is based on Linux 3.4-rc5 with the following two
> patch series applied.
>
> [1] 
> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg10494.html
>    [PATCH 00/20] ARM: Samsung: Add support for Exynos5250 Rev1.0
>
> [2] 
> http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg09640.html
>    [PATCH 0/6] S3C24XX: Add support for HSSPI on S3C2416/S3C2443
>
> Thomas Abraham (10):
>  spi: s3c64xx: remove unused S3C64XX_SPI_ST_TRLCNTZ macro
>  spi: s3c64xx: move controller information into driver data
>  ARM: Samsung: Remove spi hardware controller information from platform data
>  ARM: Samsung: Remove pdev pointer paremeter from spi gpio setup functions
>  ARM: Samsung: Update the device names for spi clock lookup
>  ARM: Samsung: Modify s3c64xx_spi{0|1|2}_set_platdata function
>  spi: s3c64xx: Remove the 'set_level' callback from controller data
>  ARM: Exynos4: Fix the incorrect hierarchy of spi controller bus clock
>  ARM: Exynos5: Add spi clock support
>  spi: s3c64xx: add device tree support
>
Look ok to me.  FWIW,  Acked-by: Jassi Brar 
--
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 07/10] spi: s3c64xx: Remove the 'set_level' callback from controller data

2012-05-09 Thread Jassi Brar
On Wed, May 9, 2012 at 3:34 AM, Thomas Abraham
 wrote:
>
> The set_level callback in the controller data, which is used to configure
> the slave select line, cannot be supported when migrating the driver to
> device tree based discovery. Since all the platforms currently use gpio
> as the slave select line, this callback can be removed from the
> controller data and replaced with call to gpio_set_value in the driver.
>
This patch is ok.
Separately, you might also want to see if we really need the air-tight
protection of spin_lock_irqsave around enable_cs - IIRC someone
had a CS coming out of an gpio extender connected over I2C
--
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