Re: [PATCH V6 2/2] video: drm: exynos: Add device tree support

2012-09-21 Thread Inki Dae
2012/9/21 Stephen Warren :
> On 09/21/2012 05:22 AM, Leela Krishna Amudala wrote:
>> This patch adds device tree based discovery support for exynos DRM-FIMD
>> driver which includes driver modification to handle platform data in
>> both the cases with DT and non-DT, Also adds the documentation for bindings.
>
>> diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt 
>> b/Documentation/devicetree/bindings/drm/exynos/fimd.txt
> ...
>> + - samsung,fimd-display: This property should specify the phandle of the
>> +   display device node which holds the video interface timing with the
>> +   below mentioned properties.
>> +
>> +   - lcd-htiming: Specifies the horizontal timing for the overlay. The
>> + horizontal timing includes four parameters in the following order.
>> +
>> + - horizontal back porch (in number of lcd clocks)
>> + - horizontal front porch (in number of lcd clocks)
>> + - hsync pulse width (in number of lcd clocks)
>> + - Display panels X resolution.
>> +
>> +   - lcd-vtiming: Specifies the vertical timing for the overlay. The
>> + vertical timing includes four parameters in the following order.
>> +
>> + - vertical back porch (in number of lcd lines)
>> + - vertical front porch (in number of lcd lines)
>> + - vsync pulse width (in number of lcd clocks)
>> + - Display panels Y resolution.
>
> Should this not use the new videomode timings that are under discussion at:
>
> http://lists.freedesktop.org/archives/dri-devel/2012-July/024875.html
>

ok, I agree with you. then the videomode helper is going to be merged
to mainline(3.6)? if so, this patch should be reworked based on the
videomode helper.


> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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/5] ARM: EXYNOS: Add secure firmware support

2012-09-21 Thread Kyungmin Park
On 9/19/12, Tomasz Figa  wrote:
> Hi,
>
> On Thursday 13 of September 2012 10:13:33 Tomasz Figa wrote:
>> Some Exynos-based boards are running with secure firmware running in
>> TrustZone secure world, which changes the way some things have to be
>> initialized.
>>
>> This series adds support for specifying firmware operations, implements
>> some firmware operations for Exynos secure firmware and adds a method of
>> enabling secure firmware operations on Exynos-based boards through board
>> file and device tree.
>>
>> This is a continuation of the patch series by Kyungmin Park:
>> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607
>> [PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
>>
>> Tomasz Figa (5):
>>   ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
>>   ARM: Add interface for registering and calling firmware-specific
>> operations
>>   ARM: EXYNOS: Add support for secure monitor calls
>>   ARM: EXYNOS: Add support for Exynos secure firmware
>>   ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up
>>
>>  .../devicetree/bindings/arm/samsung-boards.txt |  8 
>>  arch/arm/common/Makefile   |  2 +
>>  arch/arm/common/firmware.c | 18 
>>  arch/arm/include/asm/firmware.h| 30 +
>>  arch/arm/mach-exynos/Makefile  |  6 +++
>>  arch/arm/mach-exynos/common.c  | 34 ++
>>  arch/arm/mach-exynos/common.h  |  2 +
>>  arch/arm/mach-exynos/exynos-smc.S  | 22 +
>>  arch/arm/mach-exynos/firmware.c| 52
>> ++ arch/arm/mach-exynos/include/mach/map.h
>>  |  3 ++
>>  arch/arm/mach-exynos/mach-exynos4-dt.c |  1 +
>>  arch/arm/mach-exynos/platsmp.c |  8 
>>  arch/arm/mach-exynos/smc.h | 31 +
>>  arch/arm/plat-samsung/include/plat/map-s5p.h   |  1 +
>>  14 files changed, 218 insertions(+)
>>  create mode 100644 arch/arm/common/firmware.c
>>  create mode 100644 arch/arm/include/asm/firmware.h
>>  create mode 100644 arch/arm/mach-exynos/exynos-smc.S
>>  create mode 100644 arch/arm/mach-exynos/firmware.c
>>  create mode 100644 arch/arm/mach-exynos/smc.h
>
> Any further comments for this series? Maybe we could merge it for 3.7?
>
Hi Arnd, Olof,
Can you merge it for 3.7? If exynos implementation still has issues,
it's okay only merge firmware implementation.

Can you give your opinions?

Thank you,
Kyungmin Park
--
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 1/2] m5mols: Remove unneeded control ops assignments

2012-09-21 Thread Sylwester Nawrocki
Since all host drivers using this subdev are already using
the control framework these compatibility ops can be removed.

Signed-off-by: Sylwester Nawrocki 
Signed-off-by: Kyungmin Park 
---
 drivers/media/video/m5mols/m5mols_core.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/video/m5mols/m5mols_core.c 
b/drivers/media/video/m5mols/m5mols_core.c
index ac7d28b..cbb6381 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -822,13 +822,6 @@ static int m5mols_log_status(struct v4l2_subdev *sd)
 
 static const struct v4l2_subdev_core_ops m5mols_core_ops = {
.s_power= m5mols_s_power,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
-   .queryctrl  = v4l2_subdev_queryctrl,
-   .querymenu  = v4l2_subdev_querymenu,
-   .g_ext_ctrls= v4l2_subdev_g_ext_ctrls,
-   .try_ext_ctrls  = v4l2_subdev_try_ext_ctrls,
-   .s_ext_ctrls= v4l2_subdev_s_ext_ctrls,
.log_status = m5mols_log_status,
 };
 
-- 
1.7.11.3

--
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] m5mols: Protect driver data with a mutex

2012-09-21 Thread Sylwester Nawrocki
Without the locking the driver's data could get corrupted when the subdev
is accessed from user space and from host driver by multiple processes.

Signed-off-by: Sylwester Nawrocki 
Signed-off-by: Kyungmin Park 
---
 drivers/media/video/m5mols/m5mols.h  | 18 +---
 drivers/media/video/m5mols/m5mols_core.c | 77 +---
 2 files changed, 62 insertions(+), 33 deletions(-)

diff --git a/drivers/media/video/m5mols/m5mols.h 
b/drivers/media/video/m5mols/m5mols.h
index bb58991..15d3a4f 100644
--- a/drivers/media/video/m5mols/m5mols.h
+++ b/drivers/media/video/m5mols/m5mols.h
@@ -155,8 +155,6 @@ struct m5mols_version {
  * @pdata: platform data
  * @sd: v4l-subdev instance
  * @pad: media pad
- * @ffmt: current fmt according to resolution type
- * @res_type: current resolution type
  * @irq_waitq: waitqueue for the capture
  * @irq_done: set to 1 in the interrupt handler
  * @handle: control handler
@@ -174,6 +172,10 @@ struct m5mols_version {
  * @wdr: wide dynamic range control
  * @stabilization: image stabilization control
  * @jpeg_quality: JPEG compression quality control
+ * @set_power: optional power callback to the board code
+ * @lock: mutex protecting the structure fields below
+ * @ffmt: current fmt according to resolution type
+ * @res_type: current resolution type
  * @ver: information of the version
  * @cap: the capture mode attributes
  * @isp_ready: 1 when the ISP controller has completed booting
@@ -181,14 +183,11 @@ struct m5mols_version {
  * @ctrl_sync: 1 when the control handler state is restored in H/W
  * @resolution:register value for current resolution
  * @mode: register value for current operation mode
- * @set_power: optional power callback to the board code
  */
 struct m5mols_info {
const struct m5mols_platform_data *pdata;
struct v4l2_subdev sd;
struct media_pad pad;
-   struct v4l2_mbus_framefmt ffmt[M5MOLS_RESTYPE_MAX];
-   int res_type;
 
wait_queue_head_t irq_waitq;
atomic_t irq_done;
@@ -216,6 +215,13 @@ struct m5mols_info {
struct v4l2_ctrl *stabilization;
struct v4l2_ctrl *jpeg_quality;
 
+   int (*set_power)(struct device *dev, int on);
+
+   struct mutex lock;
+
+   struct v4l2_mbus_framefmt ffmt[M5MOLS_RESTYPE_MAX];
+   int res_type;
+
struct m5mols_version ver;
struct m5mols_capture cap;
 
@@ -225,8 +231,6 @@ struct m5mols_info {
 
u8 resolution;
u8 mode;
-
-   int (*set_power)(struct device *dev, int on);
 };
 
 #define is_available_af(__info)(__info->ver.af)
diff --git a/drivers/media/video/m5mols/m5mols_core.c 
b/drivers/media/video/m5mols/m5mols_core.c
index cbb6381..5d108fe 100644
--- a/drivers/media/video/m5mols/m5mols_core.c
+++ b/drivers/media/video/m5mols/m5mols_core.c
@@ -551,13 +551,18 @@ static int m5mols_get_fmt(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh,
 {
struct m5mols_info *info = to_m5mols(sd);
struct v4l2_mbus_framefmt *format;
+   int ret = 0;
+
+   mutex_lock(&info->lock);
 
format = __find_format(info, fh, fmt->which, info->res_type);
if (!format)
-   return -EINVAL;
+   fmt->format = *format;
+   else
+   ret = -EINVAL;
 
-   fmt->format = *format;
-   return 0;
+   mutex_unlock(&info->lock);
+   return ret;
 }
 
 static int m5mols_set_fmt(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
@@ -578,6 +583,7 @@ static int m5mols_set_fmt(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh,
if (!sfmt)
return 0;
 
+   mutex_lock(&info->lock);
 
format->code = m5mols_default_ffmt[type].code;
format->colorspace = V4L2_COLORSPACE_JPEG;
@@ -589,7 +595,8 @@ static int m5mols_set_fmt(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh,
info->res_type = type;
}
 
-   return 0;
+   mutex_unlock(&info->lock);
+   return ret;
 }
 
 static int m5mols_enum_mbus_code(struct v4l2_subdev *sd,
@@ -661,20 +668,25 @@ static int m5mols_start_monitor(struct m5mols_info *info)
 static int m5mols_s_stream(struct v4l2_subdev *sd, int enable)
 {
struct m5mols_info *info = to_m5mols(sd);
-   u32 code = info->ffmt[info->res_type].code;
+   u32 code;
+   int ret;
 
-   if (enable) {
-   int ret = -EINVAL;
+   mutex_lock(&info->lock);
+   code = info->ffmt[info->res_type].code;
 
+   if (enable) {
if (is_code(code, M5MOLS_RESTYPE_MONITOR))
ret = m5mols_start_monitor(info);
if (is_code(code, M5MOLS_RESTYPE_CAPTURE))
ret = m5mols_start_capture(info);
-
-   return ret;
+   else
+   ret = -EINVAL;
+   } else {
+   ret = m5mols_set_mode(info, REG_PARAMETER);
}
 
-   return m5mols_set_mode(info, REG_PARAMETER);
+   mutex_unlock(&info->lock);
+

[PATCH V2] drm/exynos: Add match table for drm platform device

2012-09-21 Thread Leela Krishna Amudala
This patch is a part of moving the driver to support DT style probing
of exynos drm device. The compatible name should match with the
entry in the dtsi file.

Signed-off-by: Leela Krishna Amudala 
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c |   11 +++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index d070719..495be89 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -294,12 +294,23 @@ static int exynos_drm_platform_remove(struct 
platform_device *pdev)
return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id drm_device_dt_match[] = {
+   { .compatible = "samsung,exynos-drm-device"},
+   {},
+};
+MODULE_DEVICE_TABLE(of, drm_device_dt_match);
+#else
+#define drm_device_dt_match NULL
+#endif
+
 static struct platform_driver exynos_drm_platform_driver = {
.probe  = exynos_drm_platform_probe,
.remove = __devexit_p(exynos_drm_platform_remove),
.driver = {
.owner  = THIS_MODULE,
.name   = "exynos-drm",
+   .of_match_table = of_match_ptr(drm_device_dt_match),
},
 };
 
-- 
1.7.0.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 V2] drm/exynos: Add match table for drm platform device

2012-09-21 Thread Srinivas KANDAGATLA
On 21/09/12 19:37, Leela Krishna Amudala wrote:
> This patch is a part of moving the driver to support DT style probing
> of exynos drm device. The compatible name should match with the
> entry in the dtsi file.
>
> Signed-off-by: Leela Krishna Amudala 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c |   11 +++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c 
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index d070719..495be89 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -294,12 +294,23 @@ static int exynos_drm_platform_remove(struct 
> platform_device *pdev)
>   return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id drm_device_dt_match[] = {
> + { .compatible = "samsung,exynos-drm-device"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, drm_device_dt_match);
> +#else
> +#define drm_device_dt_match NULL
> +#endif

No need of else here as you are using of_match_ptr.

> +
>  static struct platform_driver exynos_drm_platform_driver = {
>   .probe  = exynos_drm_platform_probe,
>   .remove = __devexit_p(exynos_drm_platform_remove),
>   .driver = {
>   .owner  = THIS_MODULE,
>   .name   = "exynos-drm",
> + .of_match_table = of_match_ptr(drm_device_dt_match),
>   },
>  };
>  

--
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


[RFC PATCH 0/6] pinctrl: samsung: Convert the driver to use per-bank GPIO and interrupt specifiers

2012-09-21 Thread Tomasz Figa
This is a continuation of work focused on improving the pinctrl-samsung
driver.

First part on which this one depends:
[RFC 0/6] pinctrl: samsung: Remove static platform-specific data
http://article.gmane.org/gmane.linux.kernel.samsung-soc/12759

This series attempts to simplify usage of the driver and fix several
problems of current implementation, in particular:

  - Simplifies GPIO pin specification in device tree by using pin
namespace local to pin bank instead of local to pin controller, e.g.

gpios = <&gpj0 3 0>;

instead of

gpios = <&pinctrl0 115 0>;

  - Simplifies GPIO interrupt specification in device tree by using
namespace local to pin bank (and equal to GPIO namespace), e.g.

interrupt-parent = <&gpj0>;
interrupts = <3 0>;

instead of

interrupt-parent = <&pinctrl0>;
interrupts = <115 0>;

  - Simplifies internal GPIO pin to bank translation thanks to
correspondence of particular GPIO chips to pin banks. This allows
to remove the (costly in case of GPIO bit-banging drivers) lookup
over all banks to find the one that the pin is from.

Any comments are welcome.

To do:
  - Bindings documentation
  - Per-bank interrupt specifiers for wake-up interrupts
  - Configuration of pins used as wake-up interrupts to EINT function

Tomasz Figa (6):
  pinctrl: samsung: Hold OF node of pin bank in bank struct
  pinctrl: samsung: Hold pointer to driver data in bank struct
  pinctrl: exynos: Use one IRQ domain per pin bank
  pinctrl: samsung: Do not pass gpio_chip to pin_to_reg_bank
  pinctrl: samsung: Use one GPIO chip per pin bank
  ARM: dts: exynos4210: Update pin bank nodes to reflect per-bank GPIO
chips and IRQ domains

 arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi |  72 ++
 arch/arm/boot/dts/exynos4210.dtsi   |   4 -
 drivers/pinctrl/pinctrl-exynos.c| 118 +
 drivers/pinctrl/pinctrl-exynos.h|  12 --
 drivers/pinctrl/pinctrl-samsung.c   | 168 ++--
 drivers/pinctrl/pinctrl-samsung.h   |  18 ++-
 6 files changed, 220 insertions(+), 172 deletions(-)

-- 
1.7.12

--
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


[RFC PATCH 3/6] pinctrl: exynos: Use one IRQ domain per pin bank

2012-09-21 Thread Tomasz Figa
Instead of registering one IRQ domain for all pin banks of a pin
controller, this patch implements registration of per-bank domains.

At a cost of a little memory overhead (~2.5KiB for all GPIO interrupts
of Exynos4x12) it simplifies driver code and device tree sources,
because GPIO interrupts can be now specified per banks.

Example:
device {
/* ... */
interrupt-parent = <&gpa1>;
interrupts = <3 0>;
/* ... */
};

Signed-off-by: Tomasz Figa 
---
 drivers/pinctrl/pinctrl-exynos.c  | 118 +++---
 drivers/pinctrl/pinctrl-exynos.h  |  12 
 drivers/pinctrl/pinctrl-samsung.c |   4 +-
 drivers/pinctrl/pinctrl-samsung.h |   5 +-
 4 files changed, 37 insertions(+), 102 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index 340bfc2..2f8427f 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -40,46 +40,46 @@ static const struct of_device_id exynos_wkup_irq_ids[] = {
 
 static void exynos_gpio_irq_unmask(struct irq_data *irqd)
 {
-   struct samsung_pinctrl_drv_data *d = irqd->domain->host_data;
-   struct exynos_geint_data *edata = irq_data_get_irq_handler_data(irqd);
-   unsigned long reg_mask = d->ctrl->geint_mask + edata->eint_offset;
+   struct samsung_pin_bank *bank = irq_data_get_irq_handler_data(irqd);
+   struct samsung_pinctrl_drv_data *d = irq_data_get_irq_chip_data(irqd);
+   unsigned long reg_mask = d->ctrl->geint_mask + bank->eint_offset;
unsigned long mask;
 
mask = readl(d->virt_base + reg_mask);
-   mask &= ~(1 << edata->pin);
+   mask &= ~(1 << irqd->hwirq);
writel(mask, d->virt_base + reg_mask);
 }
 
 static void exynos_gpio_irq_mask(struct irq_data *irqd)
 {
-   struct samsung_pinctrl_drv_data *d = irqd->domain->host_data;
-   struct exynos_geint_data *edata = irq_data_get_irq_handler_data(irqd);
-   unsigned long reg_mask = d->ctrl->geint_mask + edata->eint_offset;
+   struct samsung_pin_bank *bank = irq_data_get_irq_handler_data(irqd);
+   struct samsung_pinctrl_drv_data *d = irq_data_get_irq_chip_data(irqd);
+   unsigned long reg_mask = d->ctrl->geint_mask + bank->eint_offset;
unsigned long mask;
 
mask = readl(d->virt_base + reg_mask);
-   mask |= 1 << edata->pin;
+   mask |= 1 << irqd->hwirq;
writel(mask, d->virt_base + reg_mask);
 }
 
 static void exynos_gpio_irq_ack(struct irq_data *irqd)
 {
-   struct samsung_pinctrl_drv_data *d = irqd->domain->host_data;
-   struct exynos_geint_data *edata = irq_data_get_irq_handler_data(irqd);
-   unsigned long reg_pend = d->ctrl->geint_pend + edata->eint_offset;
+   struct samsung_pin_bank *bank = irq_data_get_irq_handler_data(irqd);
+   struct samsung_pinctrl_drv_data *d = irq_data_get_irq_chip_data(irqd);
+   unsigned long reg_pend = d->ctrl->geint_pend + bank->eint_offset;
 
-   writel(1 << edata->pin, d->virt_base + reg_pend);
+   writel(1 << irqd->hwirq, d->virt_base + reg_pend);
 }
 
 static int exynos_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
 {
-   struct samsung_pinctrl_drv_data *d = irqd->domain->host_data;
+   struct samsung_pin_bank *bank = irq_data_get_irq_handler_data(irqd);
+   struct samsung_pinctrl_drv_data *d = irq_data_get_irq_chip_data(irqd);
struct samsung_pin_ctrl *ctrl = d->ctrl;
-   struct exynos_geint_data *edata = irq_data_get_irq_handler_data(irqd);
-   struct samsung_pin_bank *bank = edata->bank;
-   unsigned int shift = EXYNOS_EINT_CON_LEN * edata->pin;
+   unsigned int pin = irqd->hwirq;
+   unsigned int shift = EXYNOS_EINT_CON_LEN * pin;
unsigned int con, trig_type;
-   unsigned long reg_con = ctrl->geint_con + edata->eint_offset;
+   unsigned long reg_con = ctrl->geint_con + bank->eint_offset;
unsigned int mask;
 
switch (type) {
@@ -114,7 +114,7 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd, 
unsigned int type)
writel(con, d->virt_base + reg_con);
 
reg_con = bank->pctl_offset;
-   shift = edata->pin * bank->func_width;
+   shift = pin * bank->func_width;
mask = (1 << bank->func_width) - 1;
 
con = readl(d->virt_base + reg_con);
@@ -136,81 +136,24 @@ static struct irq_chip exynos_gpio_irq_chip = {
.irq_set_type   = exynos_gpio_irq_set_type,
 };
 
-/*
- * given a controller-local external gpio interrupt number, prepare the handler
- * data for it.
- */
-static struct exynos_geint_data *exynos_get_eint_data(irq_hw_number_t hw,
-   struct samsung_pinctrl_drv_data *d)
-{
-   struct samsung_pin_bank *bank = d->ctrl->pin_banks;
-   struct exynos_geint_data *eint_data;
-   unsigned int nr_banks = d->ctrl->nr_banks, idx;
-   unsigned int irq_base = 0;
-
-   if (hw >= d->ctrl->nr_gint) {
-  

[RFC PATCH 4/6] pinctrl: samsung: Do not pass gpio_chip to pin_to_reg_bank

2012-09-21 Thread Tomasz Figa
The pointer to gpio_chip passed to pin_to_reg_bank utility function is
used only to retrieve a pointer to samsung_pinctrl_drv_data structure.

This patch modifies the function and its users to pass a pointer to
samsung_pinctrl_drv_data directly.

Signed-off-by: Tomasz Figa 
---
 drivers/pinctrl/pinctrl-samsung.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index a6c219a..eadcef7 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -255,14 +255,12 @@ static int samsung_pinmux_get_groups(struct pinctrl_dev 
*pctldev,
  * given a pin number that is local to a pin controller, find out the pin bank
  * and the register base of the pin bank.
  */
-static void pin_to_reg_bank(struct gpio_chip *gc, unsigned pin,
-   void __iomem **reg, u32 *offset,
+static void pin_to_reg_bank(struct samsung_pinctrl_drv_data *drvdata,
+   unsigned pin, void __iomem **reg, u32 *offset,
struct samsung_pin_bank **bank)
 {
-   struct samsung_pinctrl_drv_data *drvdata;
struct samsung_pin_bank *b;
 
-   drvdata = dev_get_drvdata(gc->dev);
b = drvdata->ctrl->pin_banks;
 
while ((pin >= b->pin_base) &&
@@ -297,7 +295,7 @@ static void samsung_pinmux_setup(struct pinctrl_dev 
*pctldev, unsigned selector,
 * pin function number in the config register.
 */
for (cnt = 0; cnt < drvdata->pin_groups[group].num_pins; cnt++) {
-   pin_to_reg_bank(drvdata->gc, pins[cnt] - drvdata->ctrl->base,
+   pin_to_reg_bank(drvdata, pins[cnt] - drvdata->ctrl->base,
®, &pin_offset, &bank);
mask = (1 << bank->func_width) - 1;
shift = pin_offset * bank->func_width;
@@ -334,10 +332,13 @@ static int samsung_pinmux_gpio_set_direction(struct 
pinctrl_dev *pctldev,
struct pinctrl_gpio_range *range, unsigned offset, bool input)
 {
struct samsung_pin_bank *bank;
+   struct samsung_pinctrl_drv_data *drvdata;
void __iomem *reg;
u32 data, pin_offset, mask, shift;
 
-   pin_to_reg_bank(range->gc, offset, ®, &pin_offset, &bank);
+   drvdata = pinctrl_dev_get_drvdata(pctldev);
+
+   pin_to_reg_bank(drvdata, offset, ®, &pin_offset, &bank);
mask = (1 << bank->func_width) - 1;
shift = pin_offset * bank->func_width;
 
@@ -371,7 +372,7 @@ static int samsung_pinconf_rw(struct pinctrl_dev *pctldev, 
unsigned int pin,
u32 cfg_value, cfg_reg;
 
drvdata = pinctrl_dev_get_drvdata(pctldev);
-   pin_to_reg_bank(drvdata->gc, pin - drvdata->ctrl->base, ®_base,
+   pin_to_reg_bank(drvdata, pin - drvdata->ctrl->base, ®_base,
&pin_offset, &bank);
 
switch (cfg_type) {
@@ -470,8 +471,11 @@ static void samsung_gpio_set(struct gpio_chip *gc, 
unsigned offset, int value)
 {
void __iomem *reg;
u32 pin_offset, data;
+   struct samsung_pinctrl_drv_data *drvdata;
 
-   pin_to_reg_bank(gc, offset, ®, &pin_offset, NULL);
+   drvdata = dev_get_drvdata(gc->dev);
+
+   pin_to_reg_bank(drvdata, offset, ®, &pin_offset, NULL);
data = readl(reg + DAT_REG);
data &= ~(1 << pin_offset);
if (value)
@@ -484,8 +488,11 @@ static int samsung_gpio_get(struct gpio_chip *gc, unsigned 
offset)
 {
void __iomem *reg;
u32 pin_offset, data;
+   struct samsung_pinctrl_drv_data *drvdata;
+
+   drvdata = dev_get_drvdata(gc->dev);
 
-   pin_to_reg_bank(gc, offset, ®, &pin_offset, NULL);
+   pin_to_reg_bank(drvdata, offset, ®, &pin_offset, NULL);
data = readl(reg + DAT_REG);
data >>= pin_offset;
data &= 1;
-- 
1.7.12

--
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


[RFC PATCH 5/6] pinctrl: samsung: Use one GPIO chip per pin bank

2012-09-21 Thread Tomasz Figa
This patch modifies the pinctrl-samsung driver to register one GPIO chip
per pin bank, instead of a single chip for all pin banks of the
controller.

It simplifies GPIO accesses a lot (constant time instead of looping
through the list of banks to find the right one) and should have a good
effect on performance of any bit-banging driver.

In addition it allows to reference GPIO pins by a phandle to the bank
node and a local pin offset inside of the bank (similar to previous
gpiolib driver), which is more clear and readable than using indices
relative to the whole pin controller.

Example:
device {
/* ... */
gpios = <&gpk0 4 0>;
/* ... */
};

Signed-off-by: Tomasz Figa 
---
 drivers/pinctrl/pinctrl-samsung.c | 118 +++---
 drivers/pinctrl/pinctrl-samsung.h |   8 ++-
 2 files changed, 78 insertions(+), 48 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index eadcef7..426a13d 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -51,6 +51,11 @@ DEFINE_SPINLOCK(init_lock);
 
 static unsigned int pin_base = 0;
 
+static inline struct samsung_pin_bank *gc_to_pin_bank(struct gpio_chip *gc)
+{
+   return container_of(gc, struct samsung_pin_bank, gpio_chip);
+}
+
 /* check if the selector is a valid pin group selector */
 static int samsung_get_group_count(struct pinctrl_dev *pctldev)
 {
@@ -336,9 +341,12 @@ static int samsung_pinmux_gpio_set_direction(struct 
pinctrl_dev *pctldev,
void __iomem *reg;
u32 data, pin_offset, mask, shift;
 
+   bank = gc_to_pin_bank(range->gc);
drvdata = pinctrl_dev_get_drvdata(pctldev);
 
-   pin_to_reg_bank(drvdata, offset, ®, &pin_offset, &bank);
+   pin_offset = offset - bank->pin_base;
+   reg = drvdata->virt_base + bank->pctl_offset;
+
mask = (1 << bank->func_width) - 1;
shift = pin_offset * bank->func_width;
 
@@ -469,17 +477,16 @@ static struct pinconf_ops samsung_pinconf_ops = {
 /* gpiolib gpio_set callback function */
 static void samsung_gpio_set(struct gpio_chip *gc, unsigned offset, int value)
 {
+   struct samsung_pin_bank *bank = gc_to_pin_bank(gc);
void __iomem *reg;
-   u32 pin_offset, data;
-   struct samsung_pinctrl_drv_data *drvdata;
+   u32 data;
 
-   drvdata = dev_get_drvdata(gc->dev);
+   reg = bank->drvdata->virt_base + bank->pctl_offset;
 
-   pin_to_reg_bank(drvdata, offset, ®, &pin_offset, NULL);
data = readl(reg + DAT_REG);
-   data &= ~(1 << pin_offset);
+   data &= ~(1 << offset);
if (value)
-   data |= 1 << pin_offset;
+   data |= 1 << offset;
writel(data, reg + DAT_REG);
 }
 
@@ -487,14 +494,13 @@ static void samsung_gpio_set(struct gpio_chip *gc, 
unsigned offset, int value)
 static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset)
 {
void __iomem *reg;
-   u32 pin_offset, data;
-   struct samsung_pinctrl_drv_data *drvdata;
+   u32 data;
+   struct samsung_pin_bank *bank = gc_to_pin_bank(gc);
 
-   drvdata = dev_get_drvdata(gc->dev);
+   reg = bank->drvdata->virt_base + bank->pctl_offset;
 
-   pin_to_reg_bank(drvdata, offset, ®, &pin_offset, NULL);
data = readl(reg + DAT_REG);
-   data >>= pin_offset;
+   data >>= offset;
data &= 1;
return data;
 }
@@ -726,12 +732,15 @@ static int __init samsung_pinctrl_register(struct 
platform_device *pdev,
return -EINVAL;
}
 
-   drvdata->grange.name = "samsung-pctrl-gpio-range";
-   drvdata->grange.id = 0;
-   drvdata->grange.base = drvdata->ctrl->base;
-   drvdata->grange.npins = drvdata->ctrl->nr_pins;
-   drvdata->grange.gc = drvdata->gc;
-   pinctrl_add_gpio_range(drvdata->pctl_dev, &drvdata->grange);
+   for (bank = 0; bank < drvdata->ctrl->nr_banks; ++bank) {
+   pin_bank = &drvdata->ctrl->pin_banks[bank];
+   pin_bank->grange.name = pin_bank->name;
+   pin_bank->grange.id = bank;
+   pin_bank->grange.base = pin_bank->gpio_chip.base;
+   pin_bank->grange.npins = pin_bank->gpio_chip.ngpio;
+   pin_bank->grange.gc = &pin_bank->gpio_chip;
+   pinctrl_add_gpio_range(drvdata->pctl_dev, &pin_bank->grange);
+   }
 
ret = samsung_pinctrl_parse_dt(pdev, drvdata);
if (ret) {
@@ -742,49 +751,68 @@ static int __init samsung_pinctrl_register(struct 
platform_device *pdev,
return 0;
 }
 
+static const struct gpio_chip samsung_gpiolib_chip = {
+   .set = samsung_gpio_set,
+   .get = samsung_gpio_get,
+   .direction_input = samsung_gpio_direction_input,
+   .direction_output = samsung_gpio_direction_output,
+   .owner = THIS_MODULE,
+};
+
 /* register the gpiolib interface with the gpiolib subsystem */
 static int __init samsung_gpiolib_reg

[RFC PATCH 6/6] ARM: dts: exynos4210: Update pin bank nodes to reflect per-bank GPIO chips and IRQ domains

2012-09-21 Thread Tomasz Figa
This patch modifies device tree sources of Exynos4210 to match the
changes done to pinctrl-samsung driver.

Signed-off-by: Tomasz Figa 
---
 arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi | 72 +
 arch/arm/boot/dts/exynos4210.dtsi   |  4 --
 2 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi 
b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi
index cac7f71..c3795e4 100644
--- a/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi
+++ b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi
@@ -24,9 +24,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x00>;
+   #interrupt-cells = <2>;
};
 
gpa1: pin-bank@1 {
@@ -39,9 +41,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x04>;
+   #interrupt-cells = <2>;
};
 
gpb: pin-bank@2 {
@@ -54,9 +58,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x08>;
+   #interrupt-cells = <2>;
};
 
gpc0: pin-bank@3 {
@@ -69,9 +75,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x0C>;
+   #interrupt-cells = <2>;
};
 
gpc1: pin-bank@4 {
@@ -84,9 +92,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x10>;
+   #interrupt-cells = <2>;
};
 
gpd0: pin-bank@5 {
@@ -99,9 +109,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x14>;
+   #interrupt-cells = <2>;
};
 
gpd1: pin-bank@6 {
@@ -114,9 +126,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x18>;
+   #interrupt-cells = <2>;
};
 
gpe0: pin-bank@7 {
@@ -129,9 +143,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x1C>;
+   #interrupt-cells = <2>;
};
 
gpe1: pin-bank@8 {
@@ -144,9 +160,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x20>;
+   #interrupt-cells = <2>;
};
 
gpe2: pin-bank@9 {
@@ -159,9 +177,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gpio-cells = <2>;
 
interrupt-controller;
samsung,eint-offset = <0x24>;
+   #interrupt-cells = <2>;
};
 
gpe3: pin-bank@10 {
@@ -174,9 +194,11 @@
samsung,drv-width = <2>;
samsung,conpdn-width = <2>;
samsung,pudpdn-width = <2>;
+   #gp

[RFC PATCH 1/6] pinctrl: samsung: Hold OF node of pin bank in bank struct

2012-09-21 Thread Tomasz Figa
The node pointer will be used in extensions added by patches that will
follow.

Signed-off-by: Tomasz Figa 
---
 drivers/pinctrl/pinctrl-samsung.c | 2 ++
 drivers/pinctrl/pinctrl-samsung.h | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index 03bf743..846b277 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -825,6 +825,8 @@ static int samsung_pinctrl_parse_dt_bank(struct 
samsung_pin_bank *bank,
if (!ret)
bank->pudpdn_width = val;
 
+   bank->of_node = np;
+
if (!of_find_property(np, "interrupt-controller", NULL)) {
bank->eint_type = EINT_TYPE_NONE;
return 0;
diff --git a/drivers/pinctrl/pinctrl-samsung.h 
b/drivers/pinctrl/pinctrl-samsung.h
index 72303f1..b7b74cc 100644
--- a/drivers/pinctrl/pinctrl-samsung.h
+++ b/drivers/pinctrl/pinctrl-samsung.h
@@ -111,6 +111,7 @@ struct samsung_pinctrl_drv_data;
  * @eint_type: type of the external interrupt supported by the bank.
  * @irq_base: starting controller local irq number of the bank.
  * @name: name to be prefixed for each pin in this pin bank.
+ * @of_node: node of pin bank in device tree
  */
 struct samsung_pin_bank {
u32 pctl_offset;
@@ -125,6 +126,8 @@ struct samsung_pin_bank {
u32 eint_offset;
u32 irq_base;
const char  *name;
+
+   struct device_node *of_node;
 };
 
 /**
-- 
1.7.12

--
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


[RFC PATCH 2/6] pinctrl: samsung: Hold pointer to driver data in bank struct

2012-09-21 Thread Tomasz Figa
The pointer will be used by further extensions added to the driver.

Signed-off-by: Tomasz Figa 
---
 drivers/pinctrl/pinctrl-samsung.c | 33 +
 drivers/pinctrl/pinctrl-samsung.h |  2 ++
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index 846b277..3f48d3f 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -844,11 +844,11 @@ static int samsung_pinctrl_parse_dt_bank(struct 
samsung_pin_bank *bank,
 
 /* retrieve the soc specific data */
 static struct samsung_pin_ctrl *samsung_pinctrl_get_soc_data(
-   struct platform_device *pdev)
+   struct samsung_pinctrl_drv_data *d)
 {
int id;
const struct of_device_id *match;
-   const struct device_node *node = pdev->dev.of_node;
+   struct device_node *node = d->dev->of_node;
struct device_node *bank_np;
struct samsung_pin_ctrl *ctrl;
struct samsung_pin_bank *banks, *b;
@@ -858,9 +858,9 @@ static struct samsung_pin_ctrl 
*samsung_pinctrl_get_soc_data(
u32 val;
int ret;
 
-   id = of_alias_get_id(pdev->dev.of_node, "pinctrl");
+   id = of_alias_get_id(node, "pinctrl");
if (id < 0) {
-   dev_err(&pdev->dev, "failed to get alias id\n");
+   dev_err(d->dev, "failed to get alias id\n");
return NULL;
}
match = of_match_node(samsung_pinctrl_dt_match, node);
@@ -873,20 +873,20 @@ static struct samsung_pin_ctrl 
*samsung_pinctrl_get_soc_data(
}
 
if (!bank_cnt) {
-   dev_err(&pdev->dev, "no pin banks specified\n");
+   dev_err(d->dev, "no pin banks specified\n");
return NULL;
}
 
-   ctrl = devm_kzalloc(&pdev->dev, sizeof(*ctrl), GFP_KERNEL);
+   ctrl = devm_kzalloc(d->dev, sizeof(*ctrl), GFP_KERNEL);
if (!ctrl) {
-   dev_err(&pdev->dev, "failed to allocate soc data\n");
+   dev_err(d->dev, "failed to allocate soc data\n");
return NULL;
}
 
-   banks = devm_kzalloc(&pdev->dev,
+   banks = devm_kzalloc(d->dev,
bank_cnt * sizeof(*ctrl->pin_banks), GFP_KERNEL);
if (!banks) {
-   dev_err(&pdev->dev, "failed to allocate pin banks\n");
+   dev_err(d->dev, "failed to allocate pin banks\n");
return NULL;
}
 
@@ -896,6 +896,7 @@ static struct samsung_pin_ctrl 
*samsung_pinctrl_get_soc_data(
continue;
if (samsung_pinctrl_parse_dt_bank(b, bank_np))
return NULL;
+   b->drvdata = d;
b->pin_base = ctrl->nr_pins;
ctrl->nr_pins += b->nr_pins;
if (of_find_property(bank_np, "interrupt-controller", NULL)) {
@@ -956,21 +957,21 @@ static int __devinit samsung_pinctrl_probe(struct 
platform_device *pdev)
return -ENODEV;
}
 
-   ctrl = samsung_pinctrl_get_soc_data(pdev);
-   if (!ctrl) {
-   dev_err(&pdev->dev, "driver data not available\n");
-   return -EINVAL;
-   }
-
drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
if (!drvdata) {
dev_err(dev, "failed to allocate memory for driver's "
"private data\n");
return -ENOMEM;
}
-   drvdata->ctrl = ctrl;
drvdata->dev = dev;
 
+   ctrl = samsung_pinctrl_get_soc_data(drvdata);
+   if (!ctrl) {
+   dev_err(&pdev->dev, "driver data not available\n");
+   return -EINVAL;
+   }
+   drvdata->ctrl = ctrl;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(dev, "cannot find IO resource\n");
diff --git a/drivers/pinctrl/pinctrl-samsung.h 
b/drivers/pinctrl/pinctrl-samsung.h
index b7b74cc..9e30081 100644
--- a/drivers/pinctrl/pinctrl-samsung.h
+++ b/drivers/pinctrl/pinctrl-samsung.h
@@ -112,6 +112,7 @@ struct samsung_pinctrl_drv_data;
  * @irq_base: starting controller local irq number of the bank.
  * @name: name to be prefixed for each pin in this pin bank.
  * @of_node: node of pin bank in device tree
+ * @drvdata: link to controller driver data
  */
 struct samsung_pin_bank {
u32 pctl_offset;
@@ -128,6 +129,7 @@ struct samsung_pin_bank {
const char  *name;
 
struct device_node *of_node;
+   struct samsung_pinctrl_drv_data *drvdata;
 };
 
 /**
-- 
1.7.12

--
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 2/2] video: drm: exynos: Add device tree support

2012-09-21 Thread Stephen Warren
On 09/21/2012 01:22 AM, Inki Dae wrote:
> 2012/9/21 Stephen Warren :
>> On 09/21/2012 05:22 AM, Leela Krishna Amudala wrote:
>>> This patch adds device tree based discovery support for exynos DRM-FIMD
>>> driver which includes driver modification to handle platform data in
>>> both the cases with DT and non-DT, Also adds the documentation for bindings.
>>
>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/fimd.txt 
>>> b/Documentation/devicetree/bindings/drm/exynos/fimd.txt
>> ...
>>> + - samsung,fimd-display: This property should specify the phandle of the
>>> +   display device node which holds the video interface timing with the
>>> +   below mentioned properties.
>>> +
>>> +   - lcd-htiming: Specifies the horizontal timing for the overlay. The
>>> + horizontal timing includes four parameters in the following order.
>>> +
>>> + - horizontal back porch (in number of lcd clocks)
>>> + - horizontal front porch (in number of lcd clocks)
>>> + - hsync pulse width (in number of lcd clocks)
>>> + - Display panels X resolution.
>>> +
>>> +   - lcd-vtiming: Specifies the vertical timing for the overlay. The
>>> + vertical timing includes four parameters in the following order.
>>> +
>>> + - vertical back porch (in number of lcd lines)
>>> + - vertical front porch (in number of lcd lines)
>>> + - vsync pulse width (in number of lcd clocks)
>>> + - Display panels Y resolution.
>>
>> Should this not use the new videomode timings that are under discussion at:
>>
>> http://lists.freedesktop.org/archives/dri-devel/2012-July/024875.html
>>
> 
> ok, I agree with you. then the videomode helper is going to be merged
> to mainline(3.6)? if so, this patch should be reworked based on the
> videomode helper.

I think the videomode helpers would be merged for 3.7 at the very
earliest; 3.6 is cooked already. Given there are still some comments on
the binding, perhaps it won't happen until 3.8, but it'd be best to ask
on that thread so that people more directly involved with the status can
answer.
--
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 0/6] pinctrl: samsung: Remove static platform-specific data

2012-09-21 Thread Stephen Warren
On 09/20/2012 02:53 AM, Tomasz Figa wrote:
> This RFC series is a work on replacing static platform-specific data in
> pinctrl-samsung driver with data dynamically parsed from device tree.

Hmm. I tend to think this is exactly the opposite of the correct
direction; you end up wasting a whole ton of time during the boot
process parsing data out of the device tree only to end up with exactly
the same tables that you would have just put into the kernel anyway. Is
it really likely that future SoCs will change information such as the
width of the pullup/pulldown bitfield, but not change anything else
that's not already in this binding. If that isn't the case, the binding
won't be complete enough to describe any new features on future SoCs anyway.

> It aims at reducing the SoC-specific part of the driver and thus the
> amount of modifications to driver sources when adding support for next
> SoCs (like Exynos4x12).
> 
> Furthermore, moving definitions of pin banks to device tree will allow
> to simplify GPIO and GEINT specification to a format similar to used
> previously by gpiolib-based implementation, using a phandle to the bank
> and pin index inside the bank, e.g.
>   gpios = <&gpa1 4 0>;
>   interrupt-parent = <&gpa1>;
>   interrupts = <4 0>;

I don't think those two are correlated; the GPIO specifier format could
just as easily be  irrespective of whether the pinctrl driver
contains SoC-specific tables or not.

> Any comments are welcome.
> 
> TODO:
>  - bindings documentation

That's unfortunate; it would be the most interesting part to review. I
guess I'll try to work out the binding from the examples in patch 6.
--
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 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers

2012-09-21 Thread Stephen Warren
On 09/20/2012 02:53 AM, Tomasz Figa wrote:
> The patch "pinctrl: samsung: Parse pin banks from DT" introduced
> platform-specific data parsing from DT.
> 
> This patch adds all necessary nodes and properties to exynos4210 device
> tree sources.

> +++ b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi

> +/ {
> + pinctrl@1140 {
> + gpa0: pin-bank@0 {

If you're going to put a unit address ("@0")into the DT node name, the
node should have a reg property containing the same value, and the
parent node should have #address-cells and #size-cells properties.

However, I believe you can actually get unique node names without using
a unit address; instead name the nodes after the object they represent,
so e.g. s/pin-bank@0/gpa0/ above.

> + gpio-controller;

You need a #gpio-cells property too.

> + samsung,pctl-offset = <0x000>;
> + samsung,pin-bank = "gpa0";
> + samsung,pin-count = <8>;
> + samsung,func-width = <4>;
> + samsung,pud-width = <2>;
> + samsung,drv-width = <2>;
> + samsung,conpdn-width = <2>;
> + samsung,pudpdn-width = <2>;

The properties above represent the width of the fields. Must all fields
always be packed together into say the LSB of the registers? What if
there are gaps between the fields in a future SoC variant, or the order
of the fields in the register changes? I think you want to add either a
samsung,func-bit/samsung,func-position property for each of the fields,
or change from samsung,func-width=<4> to samsung,field-mask=<0xf>. IIRC,
the generic pinctrl binding used a mask for this purpose.

What if a future SoC variant adds more fields to the register? At that
point, you'd need to edit the driver anyway in order to define a new DT
property to represent the new field. Perhaps instead of having an
explicit named property per field in the register, you want a simple
list of fields:

samsung,pin-property-names = "func", "pud", "drv", "conpdn", "pudpdn";
samsung,pin-propert-masks = <0xf 0x30 0xc0 0x300 0xc00>;

That would allow a completely arbitrary number of fields and layouts to
be described.

I wonder if for absolute generality you want a samsung,pin-stride
property to represent the difference in register address per pin. I
assume that's hard-coded as 4 right now.

> + interrupt-controller;

You need a #interrupt-cells property too.

> + gpd0: pin-bank@5 {
> + gpio-controller;
> + samsung,pctl-offset = <0x0A0>;

I think hex number are usually lower-case in DT, but I may be
extrapolating a generality from a limited set of examples.

> + gpy5: pin-bank@19{

Missing a space before the { there.

> diff --git a/arch/arm/boot/dts/exynos4210.dtsi 
> b/arch/arm/boot/dts/exynos4210.dtsi
> index ecbc707..0e93717 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -59,6 +59,10 @@
>   reg = <0x1140 0x1000>;
>   interrupts = <0 47 0>;
>   interrupt-controller;
> + samsung,geint-con = <0x700>;
> + samsung,geint-mask = <0x900>;
> + samsung,geint-pend = <0xA00>;
> + samsung,svc = <0xB08>;

I assume those new properties represent register addresses within the
block. If not, I don't understand what they are.

It's unclear to me why those properties aren't all part of
exynos4210-pinctrl-banks.dtsi. Do you really have multiple SoCs where
the register addresses for the pinctrl registers are the same (hence can
be in a shared exynos4210-pinctrl-banks.dtsi), yet the register
addresses for the geint registers are different (hence must be in
non-shared exynos4210.dtsi)?

Do these properties interact with samsung,eint-offset at all? Oh,
perhaps these properties are defining a top-level interrupt controller
that aggregates all the banks together, whereas samsung,eint-offset is
per-bank?
--
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 0/6] pinctrl: samsung: Remove static platform-specific data

2012-09-21 Thread Tomasz Figa
Hi Stephen,

Thanks for your comments.

On Friday 21 of September 2012 12:40:35 Stephen Warren wrote:
> On 09/20/2012 02:53 AM, Tomasz Figa wrote:
> > This RFC series is a work on replacing static platform-specific data in
> > pinctrl-samsung driver with data dynamically parsed from device tree.
> 
> Hmm. I tend to think this is exactly the opposite of the correct
> direction; you end up wasting a whole ton of time during the boot
> process parsing data out of the device tree only to end up with exactly
> the same tables that you would have just put into the kernel anyway.

Yes, I'm aware that parsing all those information from device tree won't be 
free. I have even considering simplifying the binding to something like

samsung,pin-bank = ;

listing widths of fields in fixed order, with some fields allowed to be 
zero, meaning that given bank doesn't support this control. It would 
simplify the parsing to just iterating over a table under a single 
property. What do you think?

I have decided to post the original variant to get some comments earlier, 
as I already had all the rest of patches based on it.

> Is
> it really likely that future SoCs will change information such as the
> width of the pullup/pulldown bitfield, but not change anything else
> that's not already in this binding. If that isn't the case, the binding
> won't be complete enough to describe any new features on future SoCs
> anyway.

Looking at the history of Samsung SoCs and specifics of this subsystem, 
there isn't much likely to change other than the bindings already account 
for (and the binding represents whatever the driver accounts for).

> > It aims at reducing the SoC-specific part of the driver and thus the
> > amount of modifications to driver sources when adding support for next
> > SoCs (like Exynos4x12).
> > 
> > Furthermore, moving definitions of pin banks to device tree will allow
> > to simplify GPIO and GEINT specification to a format similar to used
> > previously by gpiolib-based implementation, using a phandle to the bank
> > and pin index inside the bank, e.g.
> > 
> > gpios = <&gpa1 4 0>;
> > interrupt-parent = <&gpa1>;
> > interrupts = <4 0>;
> 
> I don't think those two are correlated; the GPIO specifier format could
> just as easily be  irrespective of whether the pinctrl driver
> contains SoC-specific tables or not.

Correct me if I'm wrong, but each bank needs to have its own subnode to be 
able to address pins like this. That was the starting point of the whole 
series and the idea that if all the banks (which are SoC-specific) have to 
be defined anyway, maybe it wouldn't be too bad to put all the SoC-specific 
parameters there too.

> > Any comments are welcome.
> > 
> > TODO:
> >  - bindings documentation
> 
> That's unfortunate; it would be the most interesting part to review. I
> guess I'll try to work out the binding from the examples in patch 6.

Sorry about that. I thought the examples would be sufficient. Still, I was 
focused at getting comments about the idea of moving such data to DT in 
general, not the bindings, which are most likely to change, in particular.

Best regards,
Tomasz Figa

--
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 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers

2012-09-21 Thread Tomasz Figa
Hi Stephen,

Thanks for your comments.

On Friday 21 of September 2012 12:56:41 Stephen Warren wrote:
> On 09/20/2012 02:53 AM, Tomasz Figa wrote:
> > The patch "pinctrl: samsung: Parse pin banks from DT" introduced
> > platform-specific data parsing from DT.
> > 
> > This patch adds all necessary nodes and properties to exynos4210 device
> > tree sources.
> > 
> > +++ b/arch/arm/boot/dts/exynos4210-pinctrl-banks.dtsi
> > 
> > +/ {
> > +   pinctrl@1140 {
> > +   gpa0: pin-bank@0 {
> 
> If you're going to put a unit address ("@0")into the DT node name, the
> node should have a reg property containing the same value, and the
> parent node should have #address-cells and #size-cells properties.
> 
> However, I believe you can actually get unique node names without using
> a unit address; instead name the nodes after the object they represent,
> so e.g. s/pin-bank@0/gpa0/ above.

Good point. I wasn't aware of the relation between unit address and reg 
property. I thought it was just for readability.

> 
> > +   gpio-controller;
> 
> You need a #gpio-cells property too.

It will be added in further patches that add per-bank GPIO addressing.

In this RFC, this property is just used to distinguish pin bank nodes from 
pin group nodes.
 
> > +   samsung,pctl-offset = <0x000>;
> > +   samsung,pin-bank = "gpa0";
> > +   samsung,pin-count = <8>;
> > +   samsung,func-width = <4>;
> > +   samsung,pud-width = <2>;
> > +   samsung,drv-width = <2>;
> > +   samsung,conpdn-width = <2>;
> > +   samsung,pudpdn-width = <2>;
> 
> The properties above represent the width of the fields. Must all fields
> always be packed together into say the LSB of the registers? What if
> there are gaps between the fields in a future SoC variant, or the order
> of the fields in the register changes? I think you want to add either a
> samsung,func-bit/samsung,func-position property for each of the fields,
> or change from samsung,func-width=<4> to samsung,field-mask=<0xf>. IIRC,
> the generic pinctrl binding used a mask for this purpose.
> 
> What if a future SoC variant adds more fields to the register? At that
> point, you'd need to edit the driver anyway in order to define a new DT
> property to represent the new field. Perhaps instead of having an
> explicit named property per field in the register, you want a simple
> list of fields:
> 
> samsung,pin-property-names = "func", "pud", "drv", "conpdn", "pudpdn";
> samsung,pin-propert-masks = <0xf 0x30 0xc0 0x300 0xc00>;
> 
> That would allow a completely arbitrary number of fields and layouts to
> be described.
> 
> I wonder if for absolute generality you want a samsung,pin-stride
> property to represent the difference in register address per pin. I
> assume that's hard-coded as 4 right now.

Hmm, considering so many different possible changes, maybe a more 
conservative solution would be better, like reducing the amount of 
information held in DT to bank type, e.g.

samsung,bank-type = "exynos4";

or maybe

compatible = "samsung,exynos4-pin-bank*;

and then define supported bank types in the driver. SoC-specific data would 
remain in DT, i.e. pctl-offset, pin-bank, pin-count, eint-offset, etc.

> > +   interrupt-controller;
> You need a #interrupt-cells property too.

Just as with gpio-controller before, in this RFC this is just a mark to 
distinguish banks with interrupts from banks without interrupts.

Further patches will allow to use it properly (and will add #interrupt-
cells property too).

> > +   gpd0: pin-bank@5 {
> > +   gpio-controller;
> > +   samsung,pctl-offset = <0x0A0>;
> 
> I think hex number are usually lower-case in DT, but I may be
> extrapolating a generality from a limited set of examples.

I have seen both variants, with upper-case being more popular across 
Samsung's dts files and so I have chosen to use it. (Personally I prefer 
lower-case, though, if it does matter.)

> > +   gpy5: pin-bank@19{
> 
> Missing a space before the { there.

Right.

> > diff --git a/arch/arm/boot/dts/exynos4210.dtsi
> > b/arch/arm/boot/dts/exynos4210.dtsi index ecbc707..0e93717 100644
> > --- a/arch/arm/boot/dts/exynos4210.dtsi
> > +++ b/arch/arm/boot/dts/exynos4210.dtsi
> > @@ -59,6 +59,10 @@
> > 
> > reg = <0x1140 0x1000>;
> > interrupts = <0 47 0>;
> > interrupt-controller;
> > 
> > +   samsung,geint-con = <0x700>;
> > +   samsung,geint-mask = <0x900>;
> > +   samsung,geint-pend = <0xA00>;
> > +   samsung,svc = <0xB08>;
> 
> I assume those new properties represent register addresses within the
> block. If not, I don't understand what they are.

Yes, they do.

> It's unclear to me why those properties aren't all part of
> exynos4210-pinctrl-banks.dtsi. Do you really have multiple SoCs wh

Re: [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware

2012-09-21 Thread Olof Johansson
On Wed, Sep 19, 2012 at 3:10 AM, Tomasz Figa  wrote:
> Hi Olof,
>
> On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
>> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
>> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
>> > +{
>> > +   return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>> > +}
>>
>> This communication area in sysram should probably be seen as a part of
>> the firmware interface. It should thus be defined as part of the binding
>> instead, i.e.  through a reg property or similar there. That also would
>> make it easy to convert to using ioremap() instead of iodesc tables,
>> which always a nice thing.
>
> The problem with SYSRAM_NS is that it might be also used in other code, not
> related to firmware only. I don't know exactly all the use cases for it.

If you don't know the use cases, and the use cases are not in the
kernel tree that we care about here (upstream), then there's really
nothing to worry about. It's after all just a define that's moved to
an ioremap, if there's some out of tree code that needs the old legacy
define then it can be added in whatever out-of-tree code that uses it.
Right?

> Is it really a big problem or we could let it be for now, merge the patches
> for firmware and then convert SYSRAM_NS to dynamic mapping when its
> situation clarifies?

What do you expect is required to clarify the situation, and when do
you expect that to happen?


-Olof
--
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/5] ARM: Add interface for registering and calling firmware-specific operations

2012-09-21 Thread Olof Johansson
On Thu, Sep 13, 2012 at 10:13:35AM +0200, Tomasz Figa wrote:
> Some boards are running with secure firmware running in TrustZone secure
> world, which changes the way some things have to be initialized.
> 
> This patch adds an interface for platforms to specify available firmware
> operations and call them.
> 
> A wrapper macro, call_firmware_op(), checks if the operation is provided
> and calls it if so, otherwise returns 0.
> 
> By default no operations are provided.
> 
> This is a follow-up on the patch by Kyungmin Park:
> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607/focus=183988
> 
> Example of use:
> 
> In code using firmware ops:
> 
>   __raw_writel(virt_to_phys(exynos4_secondary_startup),
>   CPU1_BOOT_REG);
> 
>   /* Call Exynos specific smc call */
>   do_firmware_op(cpu_boot, cpu);
> 
>   gic_raise_softirq(cpumask_of(cpu), 1);
> 
> In board-/platform-specific code:
> 
>   static int platformX_do_idle(void)
>   {
>   /* tell platformX firmware to enter idle */
>   return 0;
>   }
> 
>   static void platformX_cpu_boot(int i)
>   {
>   /* tell platformX firmware to boot CPU i */
>   }
> 
>   static const struct firmware_ops platformX_firmware_ops __initdata = {
>   .do_idle= exynos_do_idle,
>   .cpu_boot   = exynos_cpu_boot,
>   /* cpu_boot_reg not available on platformX */
>   };
> 
>   static void __init board_init_early(void)
>   {
>   register_firmware_ops(&platformX_firmware_ops);
>   }
> 
> Signed-off-by: Kyungmin Park 
> Signed-off-by: Tomasz Figa 
> ---
>  arch/arm/common/Makefile|  2 ++
>  arch/arm/common/firmware.c  | 18 ++
>  arch/arm/include/asm/firmware.h | 30 ++
>  3 files changed, 50 insertions(+)
>  create mode 100644 arch/arm/common/firmware.c
>  create mode 100644 arch/arm/include/asm/firmware.h
> 
> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> index e8a4e58..55d4182 100644
> --- a/arch/arm/common/Makefile
> +++ b/arch/arm/common/Makefile
> @@ -2,6 +2,8 @@
>  # Makefile for the linux kernel.
>  #
>  
> +obj-y += firmware.o
> +
>  obj-$(CONFIG_ARM_GIC)+= gic.o
>  obj-$(CONFIG_ARM_VIC)+= vic.o
>  obj-$(CONFIG_ICST)   += icst.o
> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
> new file mode 100644
> index 000..27ddccb
> --- /dev/null
> +++ b/arch/arm/common/firmware.c
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics.
> + * Kyungmin Park 
> + * Tomasz Figa 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +
> +#include 
> +
> +static const struct firmware_ops default_firmware_ops;
> +
> +const struct firmware_ops *firmware_ops = &default_firmware_ops;
> diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
> new file mode 100644
> index 000..ed51b02
> --- /dev/null
> +++ b/arch/arm/include/asm/firmware.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics.
> + * Kyungmin Park 
> + * Tomasz Figa 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __ASM_ARM_FIRMWARE_H
> +#define __ASM_ARM_FIRMWARE_H
> +
> +struct firmware_ops {
> + int (*do_idle)(void);
> + void (*cpu_boot)(int cpu);
> + void __iomem *(*cpu_boot_reg)(int cpu);
> +};
> +
> +extern const struct firmware_ops *firmware_ops;
> +
> +#define call_firmware_op(op, ...)\
> + ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : 0)

I think this will cause sparse warnings for call_firmware_op(cpu_boot_reg)
if there are no ops defined, since the '0' isn't annotated as __iomem. And
you can't annotate it since the other function pointers don't need it.

I think you might be better off with stub functions as fallbacks instead of
allowing and checking for NULL here.


-Olof
--
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: [GIT PULL 4/4] Samsung DT for v3.7

2012-09-21 Thread Kukjin Kim

On 09/21/12 15:26, Kukjin Kim wrote:

Hi Arnd, Olof

This is for updating Samsung DT for v3.7.

But now this makes many conflicts with pinctrl dt because of moving file
so I'm sorting them out to avoid useless conflicts.

I'll re-send this soon again.
This is jut for your information.

I created following branch which is merged on top of Samsung pinctrl and 
board branches already are pulled into arm-soc.


This includes moving common exynos4 dt for supporting other exynos4 
SoCs easily and adding exynos43210-trats board support and some updates.


Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git 
next/dt-samsung-new


If any problems, please kindly let me know.

Thanks.

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


The following changes since commit
5698bd757d55b1bb87edd1a9744ab09c142abfc2:

Linux 3.6-rc6 (2012-09-16 14:58:51 -0700)

are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
next/dt-samsung

Kukjin Kim (1):
ARM: dts: use uart2 for console on smdkv310 and smdk5250

Tomasz Figa (6):
ARM: dts: Move parts common to EXYNOS4 from exynos4210.dtsi to exynos4.dtsi
ARM: EXYNOS: Use exynos4 prefix instead of exynos4210 on exynos4-dt
ARM: dts: Assume status of all optional nodes as disabled for exynos4
ARM: dts: Specify address and size cells for i2c controllers for EXYNOS4
ARM: EXYNOS: Add OF compatibility lookups for EXYNOS4 i2c adapters
ARM: dts: Add basic dts file for Samsung Trats board

Tushar Behera (1):
ARM: dts: Enable on-board keys as wakeup source for exynos4210-origen

arch/arm/boot/dts/exynos4.dtsi | 248 +
arch/arm/boot/dts/exynos4210-origen.dts | 63 +---
arch/arm/boot/dts/exynos4210-smdkv310.dts | 54 +--
arch/arm/boot/dts/exynos4210-trats.dts | 237 +++
arch/arm/boot/dts/exynos4210.dtsi | 181 +-
arch/arm/boot/dts/exynos5250-smdk5250.dts | 2 +-
arch/arm/mach-exynos/Makefile.boot | 2 +-
arch/arm/mach-exynos/mach-exynos4-dt.c | 32 +++-
8 files changed, 523 insertions(+), 296 deletions(-)
create mode 100644 arch/arm/boot/dts/exynos4.dtsi
create mode 100644 arch/arm/boot/dts/exynos4210-trats.dts

--
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/5] ARM: EXYNOS: Add secure firmware support

2012-09-21 Thread Olof Johansson
On Fri, Sep 21, 2012 at 05:39:41PM +0900, Kyungmin Park wrote:
> On 9/19/12, Tomasz Figa  wrote:
> > Hi,
> >
> > On Thursday 13 of September 2012 10:13:33 Tomasz Figa wrote:
> >> Some Exynos-based boards are running with secure firmware running in
> >> TrustZone secure world, which changes the way some things have to be
> >> initialized.
> >>
> >> This series adds support for specifying firmware operations, implements
> >> some firmware operations for Exynos secure firmware and adds a method of
> >> enabling secure firmware operations on Exynos-based boards through board
> >> file and device tree.
> >>
> >> This is a continuation of the patch series by Kyungmin Park:
> >> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
> >> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607
> >> [PATCH v5 2/2] ARM: EXYNOS: SMC instruction (aka firmware) support
> >> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183608/focus=184109
> >>
> >> Tomasz Figa (5):
> >>   ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
> >>   ARM: Add interface for registering and calling firmware-specific
> >> operations
> >>   ARM: EXYNOS: Add support for secure monitor calls
> >>   ARM: EXYNOS: Add support for Exynos secure firmware
> >>   ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up
> >>
> >>  .../devicetree/bindings/arm/samsung-boards.txt |  8 
> >>  arch/arm/common/Makefile   |  2 +
> >>  arch/arm/common/firmware.c | 18 
> >>  arch/arm/include/asm/firmware.h| 30 +
> >>  arch/arm/mach-exynos/Makefile  |  6 +++
> >>  arch/arm/mach-exynos/common.c  | 34 ++
> >>  arch/arm/mach-exynos/common.h  |  2 +
> >>  arch/arm/mach-exynos/exynos-smc.S  | 22 +
> >>  arch/arm/mach-exynos/firmware.c| 52
> >> ++ arch/arm/mach-exynos/include/mach/map.h
> >>  |  3 ++
> >>  arch/arm/mach-exynos/mach-exynos4-dt.c |  1 +
> >>  arch/arm/mach-exynos/platsmp.c |  8 
> >>  arch/arm/mach-exynos/smc.h | 31 +
> >>  arch/arm/plat-samsung/include/plat/map-s5p.h   |  1 +
> >>  14 files changed, 218 insertions(+)
> >>  create mode 100644 arch/arm/common/firmware.c
> >>  create mode 100644 arch/arm/include/asm/firmware.h
> >>  create mode 100644 arch/arm/mach-exynos/exynos-smc.S
> >>  create mode 100644 arch/arm/mach-exynos/firmware.c
> >>  create mode 100644 arch/arm/mach-exynos/smc.h
> >
> > Any further comments for this series? Maybe we could merge it for 3.7?
> >
> Hi Arnd, Olof,
> Can you merge it for 3.7? If exynos implementation still has issues,
> it's okay only merge firmware implementation.
> 
> Can you give your opinions?

Hi,

There are still a few open questions to resolve on this before I think it
can be merged. And it doesn't make much sense to merge the infrastructure
without at least the first user of it.


-Olof
--
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: [GIT PULL 1/4] Samsung fixes for v3.7

2012-09-21 Thread Olof Johansson
On Fri, Sep 21, 2012 at 03:21:37PM +0900, Kukjin Kim wrote:
> Hi Arnd, Olof
> 
> This is non-critical Samsung fixes for v3.7.
> 
> If any problems, please kindly let me know.
> 
> Thanks.
> 
> Best regards,
> Kgene.
> --
> Kukjin Kim , Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
> 
> The following changes since commit 5698bd757d55b1bb87edd1a9744ab09c142abfc2:
> 
>   Linux 3.6-rc6 (2012-09-16 14:58:51 -0700)
> 
> are available in the git repository at:

> git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
> v3.7-samsung-fixes-1

Thanks, pulled into next/fixes-non-critical.


-Olof
--
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 4/5] ARM: EXYNOS: Add support for Exynos secure firmware

2012-09-21 Thread Kyungmin Park
On 9/22/12, Olof Johansson  wrote:
> On Wed, Sep 19, 2012 at 3:10 AM, Tomasz Figa  wrote:
>> Hi Olof,
>>
>> On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
>>> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
>>> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
>>> > +{
>>> > +   return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
>>> > +}
>>>
>>> This communication area in sysram should probably be seen as a part of
>>> the firmware interface. It should thus be defined as part of the binding
>>> instead, i.e.  through a reg property or similar there. That also would
>>> make it easy to convert to using ioremap() instead of iodesc tables,
>>> which always a nice thing.
>>
>> The problem with SYSRAM_NS is that it might be also used in other code,
>> not
>> related to firmware only. I don't know exactly all the use cases for it.
>
> If you don't know the use cases, and the use cases are not in the
> kernel tree that we care about here (upstream), then there's really
> nothing to worry about. It's after all just a define that's moved to
> an ioremap, if there's some out of tree code that needs the old legacy
> define then it can be added in whatever out-of-tree code that uses it.
> Right?
Now this address is used at cpu boot, cpuidle, inform at this time.
As it touched at several places, it's defined at iodesc. if it changed
with ioremap, it has to export remaped address and each codes should
use it.

As I wrote at cover letter, if you want to use ioremap, it can be
modified. however can you merge firmware codes itself? since ioremap
is not related with trustzone  or firmware issues and it's exynos
specific implementation issues. Right?

Thank you,
Kyungmin Park
>
>> Is it really a big problem or we could let it be for now, merge the
>> patches
>> for firmware and then convert SYSRAM_NS to dynamic mapping when its
>> situation clarifies?
>
> What do you expect is required to clarify the situation, and when do
> you expect that to happen?
>
>
> -Olof
> --
> 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
>
--
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: [GIT PULL 2/4] Samsung pinctrl-2 for v3.7

2012-09-21 Thread Olof Johansson
On Fri, Sep 21, 2012 at 03:22:18PM +0900, Kukjin Kim wrote:
> Hi Arnd, Olof
> 
> This is for fix typo and correct fucntion for Samsung pinctrl and
> since this has a dependency with previous pinctrl branch I created
> this based on that.
> 
> If any problems, please kindly let me know.
> 
> Thanks.
> 
> Best regards,
> Kgene.
> --
> Kukjin Kim , Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
> 
> The following changes since commit 011527b45f8e3092e1f3192e5aea94187a6ca269:
> 
>   ARM: EXYNOS: Enable pinctrl driver support for EXYNOS4 device tree
> enabled platform (2012-09-07 06:14:26 +0900)
> 
> are available in the git repository at:

> git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
> next/pinctrl-samsung-2

Thanks, I just pulled this into the existing samsung/pinctrl since it was
a fast-forward there.


-Olof
--
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: [GIT PULL 3/4] Samsung devel for v3.7

2012-09-21 Thread Olof Johansson
On Fri, Sep 21, 2012 at 03:22:43PM +0900, Kukjin Kim wrote:
> Hi Arnd, Olof
> 
> Here is Samsung development for v3.7 and most of them are updating.
> 
> If any problems, please kindly let me know.
> 
> Thanks.
> 
> Best regards,
> Kgene.
> --
> Kukjin Kim , Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
> 
> The following changes since commit 5698bd757d55b1bb87edd1a9744ab09c142abfc2:
> 
>   Linux 3.6-rc6 (2012-09-16 14:58:51 -0700)
> 
> are available in the git repository at:

> git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
> next/devel-samsung
> 

Pulled into next/soc.


-Olof
--
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/5] ARM: Add interface for registering and calling firmware-specific operations

2012-09-21 Thread Kyungmin Park
On 9/22/12, Olof Johansson  wrote:
> On Thu, Sep 13, 2012 at 10:13:35AM +0200, Tomasz Figa wrote:
>> Some boards are running with secure firmware running in TrustZone secure
>> world, which changes the way some things have to be initialized.
>>
>> This patch adds an interface for platforms to specify available firmware
>> operations and call them.
>>
>> A wrapper macro, call_firmware_op(), checks if the operation is provided
>> and calls it if so, otherwise returns 0.
>>
>> By default no operations are provided.
>>
>> This is a follow-up on the patch by Kyungmin Park:
>> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607/focus=183988
>>
>> Example of use:
>>
>> In code using firmware ops:
>>
>>  __raw_writel(virt_to_phys(exynos4_secondary_startup),
>>  CPU1_BOOT_REG);
>>
>>  /* Call Exynos specific smc call */
>>  do_firmware_op(cpu_boot, cpu);
>>
>>  gic_raise_softirq(cpumask_of(cpu), 1);
>>
>> In board-/platform-specific code:
>>
>>  static int platformX_do_idle(void)
>>  {
>>  /* tell platformX firmware to enter idle */
>>  return 0;
>>  }
>>
>>  static void platformX_cpu_boot(int i)
>>  {
>>  /* tell platformX firmware to boot CPU i */
>>  }
>>
>>  static const struct firmware_ops platformX_firmware_ops __initdata = {
>>  .do_idle= exynos_do_idle,
>>  .cpu_boot   = exynos_cpu_boot,
>>  /* cpu_boot_reg not available on platformX */
>>  };
>>
>>  static void __init board_init_early(void)
>>  {
>>  register_firmware_ops(&platformX_firmware_ops);
>>  }
>>
>> Signed-off-by: Kyungmin Park 
>> Signed-off-by: Tomasz Figa 
>> ---
>>  arch/arm/common/Makefile|  2 ++
>>  arch/arm/common/firmware.c  | 18 ++
>>  arch/arm/include/asm/firmware.h | 30 ++
>>  3 files changed, 50 insertions(+)
>>  create mode 100644 arch/arm/common/firmware.c
>>  create mode 100644 arch/arm/include/asm/firmware.h
>>
>> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
>> index e8a4e58..55d4182 100644
>> --- a/arch/arm/common/Makefile
>> +++ b/arch/arm/common/Makefile
>> @@ -2,6 +2,8 @@
>>  # Makefile for the linux kernel.
>>  #
>>
>> +obj-y += firmware.o
>> +
>>  obj-$(CONFIG_ARM_GIC)   += gic.o
>>  obj-$(CONFIG_ARM_VIC)   += vic.o
>>  obj-$(CONFIG_ICST)  += icst.o
>> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
>> new file mode 100644
>> index 000..27ddccb
>> --- /dev/null
>> +++ b/arch/arm/common/firmware.c
>> @@ -0,0 +1,18 @@
>> +/*
>> + * Copyright (C) 2012 Samsung Electronics.
>> + * Kyungmin Park 
>> + * Tomasz Figa 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +static const struct firmware_ops default_firmware_ops;
>> +
>> +const struct firmware_ops *firmware_ops = &default_firmware_ops;
>> diff --git a/arch/arm/include/asm/firmware.h
>> b/arch/arm/include/asm/firmware.h
>> new file mode 100644
>> index 000..ed51b02
>> --- /dev/null
>> +++ b/arch/arm/include/asm/firmware.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Copyright (C) 2012 Samsung Electronics.
>> + * Kyungmin Park 
>> + * Tomasz Figa 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef __ASM_ARM_FIRMWARE_H
>> +#define __ASM_ARM_FIRMWARE_H
>> +
>> +struct firmware_ops {
>> +int (*do_idle)(void);
>> +void (*cpu_boot)(int cpu);
>> +void __iomem *(*cpu_boot_reg)(int cpu);
>> +};
>> +
>> +extern const struct firmware_ops *firmware_ops;
>> +
>> +#define call_firmware_op(op, ...)   \
>> +((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : 0)
>
> I think this will cause sparse warnings for call_firmware_op(cpu_boot_reg)
> if there are no ops defined, since the '0' isn't annotated as __iomem. And
> you can't annotate it since the other function pointers don't need it.
>
> I think you might be better off with stub functions as fallbacks instead of
> allowing and checking for NULL here.
do you mean like this?

#Ifdef CONFIG_ARM_FIRMWARE
#define call_firmware_op(op, ...) ((firmware_ops->op) ?
firmware_ops->op(__VA_ARGS__) : 0)
#else
#define call_firmware_op(op, ...) do { } while (0)
#endif

No problem to modify it.

Thank you,
Kyungmin Park
>
>
> -Olof
> --
> 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
>
--
To unsubsc

[PATCH] serial: samsung: Add poll_get_char & poll_put_char

2012-09-21 Thread Doug Anderson
From: Julien Pichon 

The following patch allows users to use KGDB over serial console on
board based on Samsung SOC. It has been tested on a board using
exynos5.

Signed-off-by: Julien Pichon 
Signed-off-by: Doug Anderson 
(dianders changed poll to return NO_POLL_CHAR, which appears to
fix 'help' in kgdb; also updated commit message)
---
This is pulled from an email on 
from Feb 18, 2012. It's never landed anywhere. I've made a small modification
that make it so that kgdb's "help" doesn't crash.

See: http://comments.gmane.org/gmane.linux.kernel.samsung-soc/9477

 drivers/tty/serial/samsung.c |   47 ++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 8eef114..7f04717 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -876,11 +876,24 @@ s3c24xx_serial_verify_port(struct uart_port *port, struct 
serial_struct *ser)
 
 static struct console s3c24xx_serial_console;
 
+static int __init s3c24xx_serial_console_init(void)
+{
+   register_console(&s3c24xx_serial_console);
+   return 0;
+}
+console_initcall(s3c24xx_serial_console_init);
+
 #define S3C24XX_SERIAL_CONSOLE &s3c24xx_serial_console
 #else
 #define S3C24XX_SERIAL_CONSOLE NULL
 #endif
 
+#ifdef CONFIG_CONSOLE_POLL
+static int s3c24xx_serial_get_poll_char(struct uart_port *port);
+static void s3c24xx_serial_put_poll_char(struct uart_port *port,
+unsigned char c);
+#endif
+
 static struct uart_ops s3c24xx_serial_ops = {
.pm = s3c24xx_serial_pm,
.tx_empty   = s3c24xx_serial_tx_empty,
@@ -899,6 +912,10 @@ static struct uart_ops s3c24xx_serial_ops = {
.request_port   = s3c24xx_serial_request_port,
.config_port= s3c24xx_serial_config_port,
.verify_port= s3c24xx_serial_verify_port,
+#ifdef CONFIG_CONSOLE_POLL
+   .poll_get_char = s3c24xx_serial_get_poll_char,
+   .poll_put_char = s3c24xx_serial_put_poll_char,
+#endif
 };
 
 static struct uart_driver s3c24xx_uart_drv = {
@@ -1316,6 +1333,36 @@ s3c24xx_serial_console_txrdy(struct uart_port *port, 
unsigned int ufcon)
return (utrstat & S3C2410_UTRSTAT_TXE) ? 1 : 0;
 }
 
+#ifdef CONFIG_CONSOLE_POLL
+/*
+ * Console polling routines for writing and reading from the uart while
+ * in an interrupt or debug context.
+ */
+
+static int s3c24xx_serial_get_poll_char(struct uart_port *port)
+{
+   struct s3c24xx_uart_port *ourport = to_ourport(port);
+   unsigned int ufstat;
+
+   ufstat = rd_regl(port, S3C2410_UFSTAT);
+   if (s3c24xx_serial_rx_fifocnt(ourport, ufstat) == 0)
+   return NO_POLL_CHAR;
+
+   return rd_regb(port, S3C2410_URXH);
+}
+
+static void s3c24xx_serial_put_poll_char(struct uart_port *port,
+   unsigned char c)
+{
+   unsigned int ufcon = rd_regl(cons_uart, S3C2410_UFCON);
+
+   while (!s3c24xx_serial_console_txrdy(port, ufcon))
+   cpu_relax();
+   wr_regb(cons_uart, S3C2410_UTXH, c);
+}
+
+#endif /* CONFIG_CONSOLE_POLL */
+
 static void
 s3c24xx_serial_console_putchar(struct uart_port *port, int ch)
 {
-- 
1.7.7.3

--
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/5] ARM: Add interface for registering and calling firmware-specific operations

2012-09-21 Thread Olof Johansson
On Sat, Sep 22, 2012 at 03:01:56PM +0900, Kyungmin Park wrote:
> On 9/22/12, Olof Johansson  wrote:
> > On Thu, Sep 13, 2012 at 10:13:35AM +0200, Tomasz Figa wrote:
> >> Some boards are running with secure firmware running in TrustZone secure
> >> world, which changes the way some things have to be initialized.
> >>
> >> This patch adds an interface for platforms to specify available firmware
> >> operations and call them.
> >>
> >> A wrapper macro, call_firmware_op(), checks if the operation is provided
> >> and calls it if so, otherwise returns 0.
> >>
> >> By default no operations are provided.
> >>
> >> This is a follow-up on the patch by Kyungmin Park:
> >> [PATCH v5 1/2] ARM: Make a compile firmware conditionally
> >> http://thread.gmane.org/gmane.linux.ports.arm.kernel/183607/focus=183988
> >>
> >> Example of use:
> >>
> >> In code using firmware ops:
> >>
> >>__raw_writel(virt_to_phys(exynos4_secondary_startup),
> >>CPU1_BOOT_REG);
> >>
> >>/* Call Exynos specific smc call */
> >>do_firmware_op(cpu_boot, cpu);
> >>
> >>gic_raise_softirq(cpumask_of(cpu), 1);
> >>
> >> In board-/platform-specific code:
> >>
> >>static int platformX_do_idle(void)
> >>{
> >>/* tell platformX firmware to enter idle */
> >>return 0;
> >>}
> >>
> >>static void platformX_cpu_boot(int i)
> >>{
> >>/* tell platformX firmware to boot CPU i */
> >>}
> >>
> >>static const struct firmware_ops platformX_firmware_ops __initdata = {
> >>.do_idle= exynos_do_idle,
> >>.cpu_boot   = exynos_cpu_boot,
> >>/* cpu_boot_reg not available on platformX */
> >>};
> >>
> >>static void __init board_init_early(void)
> >>{
> >>register_firmware_ops(&platformX_firmware_ops);
> >>}
> >>
> >> Signed-off-by: Kyungmin Park 
> >> Signed-off-by: Tomasz Figa 
> >> ---
> >>  arch/arm/common/Makefile|  2 ++
> >>  arch/arm/common/firmware.c  | 18 ++
> >>  arch/arm/include/asm/firmware.h | 30 ++
> >>  3 files changed, 50 insertions(+)
> >>  create mode 100644 arch/arm/common/firmware.c
> >>  create mode 100644 arch/arm/include/asm/firmware.h
> >>
> >> diff --git a/arch/arm/common/Makefile b/arch/arm/common/Makefile
> >> index e8a4e58..55d4182 100644
> >> --- a/arch/arm/common/Makefile
> >> +++ b/arch/arm/common/Makefile
> >> @@ -2,6 +2,8 @@
> >>  # Makefile for the linux kernel.
> >>  #
> >>
> >> +obj-y += firmware.o
> >> +
> >>  obj-$(CONFIG_ARM_GIC) += gic.o
> >>  obj-$(CONFIG_ARM_VIC) += vic.o
> >>  obj-$(CONFIG_ICST)+= icst.o
> >> diff --git a/arch/arm/common/firmware.c b/arch/arm/common/firmware.c
> >> new file mode 100644
> >> index 000..27ddccb
> >> --- /dev/null
> >> +++ b/arch/arm/common/firmware.c
> >> @@ -0,0 +1,18 @@
> >> +/*
> >> + * Copyright (C) 2012 Samsung Electronics.
> >> + * Kyungmin Park 
> >> + * Tomasz Figa 
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +
> >> +#include 
> >> +
> >> +static const struct firmware_ops default_firmware_ops;
> >> +
> >> +const struct firmware_ops *firmware_ops = &default_firmware_ops;
> >> diff --git a/arch/arm/include/asm/firmware.h
> >> b/arch/arm/include/asm/firmware.h
> >> new file mode 100644
> >> index 000..ed51b02
> >> --- /dev/null
> >> +++ b/arch/arm/include/asm/firmware.h
> >> @@ -0,0 +1,30 @@
> >> +/*
> >> + * Copyright (C) 2012 Samsung Electronics.
> >> + * Kyungmin Park 
> >> + * Tomasz Figa 
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + */
> >> +
> >> +#ifndef __ASM_ARM_FIRMWARE_H
> >> +#define __ASM_ARM_FIRMWARE_H
> >> +
> >> +struct firmware_ops {
> >> +  int (*do_idle)(void);
> >> +  void (*cpu_boot)(int cpu);
> >> +  void __iomem *(*cpu_boot_reg)(int cpu);
> >> +};
> >> +
> >> +extern const struct firmware_ops *firmware_ops;
> >> +
> >> +#define call_firmware_op(op, ...) \
> >> +  ((firmware_ops->op) ? firmware_ops->op(__VA_ARGS__) : 0)
> >
> > I think this will cause sparse warnings for call_firmware_op(cpu_boot_reg)
> > if there are no ops defined, since the '0' isn't annotated as __iomem. And
> > you can't annotate it since the other function pointers don't need it.
> >
> > I think you might be better off with stub functions as fallbacks instead of
> > allowing and checking for NULL here.
> do you mean like this?
> 
> #Ifdef CONFIG_ARM_FIRMWARE
> #define call_firmware_op(op, ...) ((firmware_ops->op) ?
> firmware_ops->op(__VA_ARGS__) : 0)
> #else
> #define call_firmware_op(op, ...) do { } while (0)

Re: [PATCH 4/5] ARM: EXYNOS: Add support for Exynos secure firmware

2012-09-21 Thread Olof Johansson
On Fri, Sep 21, 2012 at 10:57 PM, Kyungmin Park
 wrote:
> On 9/22/12, Olof Johansson  wrote:
>> On Wed, Sep 19, 2012 at 3:10 AM, Tomasz Figa  wrote:
>>> Hi Olof,
>>>
>>> On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
 On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
 > +static void __iomem *exynos_cpu_boot_reg(int cpu)
 > +{
 > +   return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
 > +}

 This communication area in sysram should probably be seen as a part of
 the firmware interface. It should thus be defined as part of the binding
 instead, i.e.  through a reg property or similar there. That also would
 make it easy to convert to using ioremap() instead of iodesc tables,
 which always a nice thing.
>>>
>>> The problem with SYSRAM_NS is that it might be also used in other code,
>>> not
>>> related to firmware only. I don't know exactly all the use cases for it.
>>
>> If you don't know the use cases, and the use cases are not in the
>> kernel tree that we care about here (upstream), then there's really
>> nothing to worry about. It's after all just a define that's moved to
>> an ioremap, if there's some out of tree code that needs the old legacy
>> define then it can be added in whatever out-of-tree code that uses it.
>> Right?
> Now this address is used at cpu boot, cpuidle, inform at this time.
> As it touched at several places, it's defined at iodesc. if it changed
> with ioremap, it has to export remaped address and each codes should
> use it.

Won't you have to push down all the references to VA_SYSRAM vs
VA_SYSRAM_NS into the firmware interface anyway, since you will need
to use different addresses for whether you have firmware enabled or
not? I.e. you'll have a "firmware call" at the appropriate level for
the non-trustzone cases that uses the equivalent of VA_SYSRAM, and for
the trustzone firmware op you'll use VA_SYSRAM_NS?


> As I wrote at cover letter, if you want to use ioremap, it can be
> modified. however can you merge firmware codes itself? since ioremap
> is not related with trustzone  or firmware issues and it's exynos
> specific implementation issues. Right?

I'm not quite sure which part you are asking me to merge, if it's the
infrastructure pieces or the exynos-specific pieces.

Either way, one isn't really usable without the other, and it doesn't
make sense to merge code that can't be used. Infrastructure is best
merged together with the first user of it.


-Olof
--
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 4/5] ARM: EXYNOS: Add support for Exynos secure firmware

2012-09-21 Thread Kyungmin Park
On 9/22/12, Olof Johansson  wrote:
> On Fri, Sep 21, 2012 at 10:57 PM, Kyungmin Park
>  wrote:
>> On 9/22/12, Olof Johansson  wrote:
>>> On Wed, Sep 19, 2012 at 3:10 AM, Tomasz Figa  wrote:
 Hi Olof,

 On Saturday 15 of September 2012 17:44:55 Olof Johansson wrote:
> On Thu, Sep 13, 2012 at 10:13:37AM +0200, Tomasz Figa wrote:
> > +static void __iomem *exynos_cpu_boot_reg(int cpu)
> > +{
> > +   return S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
> > +}
>
> This communication area in sysram should probably be seen as a part of
> the firmware interface. It should thus be defined as part of the
> binding
> instead, i.e.  through a reg property or similar there. That also
> would
> make it easy to convert to using ioremap() instead of iodesc tables,
> which always a nice thing.

 The problem with SYSRAM_NS is that it might be also used in other code,
 not
 related to firmware only. I don't know exactly all the use cases for
 it.
>>>
>>> If you don't know the use cases, and the use cases are not in the
>>> kernel tree that we care about here (upstream), then there's really
>>> nothing to worry about. It's after all just a define that's moved to
>>> an ioremap, if there's some out of tree code that needs the old legacy
>>> define then it can be added in whatever out-of-tree code that uses it.
>>> Right?
>> Now this address is used at cpu boot, cpuidle, inform at this time.
>> As it touched at several places, it's defined at iodesc. if it changed
>> with ioremap, it has to export remaped address and each codes should
>> use it.
>
> Won't you have to push down all the references to VA_SYSRAM vs
> VA_SYSRAM_NS into the firmware interface anyway, since you will need
> to use different addresses for whether you have firmware enabled or
> not? I.e. you'll have a "firmware call" at the appropriate level for
> the non-trustzone cases that uses the equivalent of VA_SYSRAM, and for
> the trustzone firmware op you'll use VA_SYSRAM_NS?
Right, in case of no firmware, it uses VA_SYSRAM, but VA_SYSRAM_NS is
used at firmware case.
>
>
>> As I wrote at cover letter, if you want to use ioremap, it can be
>> modified. however can you merge firmware codes itself? since ioremap
>> is not related with trustzone  or firmware issues and it's exynos
>> specific implementation issues. Right?
>
> I'm not quite sure which part you are asking me to merge, if it's the
> infrastructure pieces or the exynos-specific pieces.
>
> Either way, one isn't really usable without the other, and it doesn't
> make sense to merge code that can't be used. Infrastructure is best
> merged together with the first user of it.

Okay, we will fix it.

Thank you,
Kyungmin Park
--
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