Re: [PATCH v10 0/17] Add Analogix Core Display Port Driver

[Heiko Stübner]

Hi Yakir,

Am Montag, 7. Dezember 2015, 14:37:19 schrieb Yakir Yang:
>The Samsung Exynos eDP controller and Rockchip RK3288 eDP controller
> share the same IP, so a lot of parts can be re-used. I split the common
> code into bridge directory, then rk3288 and exynos only need to keep
> some platform code. Cause I can't find the exact IP name of exynos dp
> controller, so I decide to name dp core driver with "analogix" which I
> find in rk3288 eDP TRM

so it looks like the hotplug works nicely now. I was able to test it 
sucessfully on both a Jerry and a Minnie device without needing to force 
hotplug :-) .

As I needed to adapt some patches when applying the lastest ones, I think it 
would be good for a full send of the latest version as v11.

When going over the patches before sending, please also fix the indentation 
issues in analogix_dp_core.h - both newly added elements to analogix_dp_device 
use spaces between type and name, where the rest uses tabs.
[This should of course be fixed in the patches adding these lines :-) ]


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

[PATCH 10/11] arm/samsung: Change s3c_pm_run_res() to use System RAM type

[Toshi Kani]

Change s3c_pm_run_res() to check with IORESOURCE_SYSTEM_RAM,
instead of strcmp() with "System RAM", in the resource table.

No functional change is made to the interface.

Cc: Kukjin Kim 
Cc: Krzysztof Kozlowski 
Cc: linux-samsung-soc@vger.kernel.org
Signed-off-by: Toshi Kani 
---
 arch/arm/plat-samsung/pm-check.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-samsung/pm-check.c b/arch/arm/plat-samsung/pm-check.c
index 04aff2c..70f2f69 100644
--- a/arch/arm/plat-samsung/pm-check.c
+++ b/arch/arm/plat-samsung/pm-check.c
@@ -53,8 +53,8 @@ static void s3c_pm_run_res(struct resource *ptr, run_fn_t fn, 
u32 *arg)
if (ptr->child != NULL)
s3c_pm_run_res(ptr->child, fn, arg);
 
-   if ((ptr->flags & IORESOURCE_MEM) &&
-   strcmp(ptr->name, "System RAM") == 0) {
+   if ((ptr->flags & IORESOURCE_SYSTEM_RAM)
+   == IORESOURCE_SYSTEM_RAM) {
S3C_PMDBG("Found system RAM at %08lx..%08lx\n",
  (unsigned long)ptr->start,
  (unsigned long)ptr->end);
--
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 00/11] Support System RAM resource type and EINJ to NVDIMM

[Toshi Kani]

This patch-set introduces a new I/O resource type, IORESOURCE_SYSTEM_RAM,
for System RAM while keeping the current IORESOURCE_MEM type bit set for
all memory-mapped ranges (including System RAM) for backward compatibility.
With the new System RAM type, walking through the iomem resource table
no longer requires to test with strcmp() against "System RAM".  After this
infrastructure update, this patch changes EINJ to support NVDIMM.

Patches 1-2 add a new System RAM type, and make resource interfaces work
with resource flags with modifier bits set.

Patches 3-7 set the System RAM type to System RAM ranges.

Patches 8-10 extend resource interfaces to check System RAM ranges with
the System RAM type.

Patch 11 changes the EINJ driver to allow injecting a memory error to
NVDIMM.

---
v1:
 - Searching for System RAM in the resource table should not require
   strcmp(). (Borislav Petkov)
 - Add a new System RAM type as a modifier to IORESOURCE_MEM.
   (Linus Torvalds)
 - NVDIMM check should remain with strcmp() against "Persistent Memory".
   (Dan Williams)
 - Reset patch version.

prev-v3:
 - Check the param2 value before target memory type. (Tony Luck)
 - Add a blank line before if-statement. Remove an unnecessary brakets.
   (Borislav Petkov)

prev-v2:
 - Change the EINJ driver to call region_intersects_ram() for checking
   RAM with a specified size. (Dan Williams)

---
Toshi Kani (11):
 01/11 resource: Add System RAM resource type
 02/11 resource: make resource flags handled properly
 03/11 x86/e820: Set IORESOURCE_SYSTEM_RAM to System RAM
 04/11 arch: Set IORESOURCE_SYSTEM_RAM to System RAM
 05/11 xen: Set IORESOURCE_SYSTEM_RAM to System RAM
 06/11 kexec: Set IORESOURCE_SYSTEM_RAM to System RAM
 07/11 memory-hotplug: Set IORESOURCE_SYSTEM_RAM to System RAM
 08/11 memremap: Change region_intersects() to use System RAM type
 09/11 resource: Change walk_system_ram to use System RAM type
 10/11 arm/samsung: Change s3c_pm_run_res() to use System RAM type
 11/11 ACPI/EINJ: Allow memory error injection to NVDIMM

---
 arch/arm/kernel/setup.c  |  6 ++---
 arch/arm/plat-samsung/pm-check.c |  4 +--
 arch/arm64/kernel/setup.c|  6 ++---
 arch/avr32/kernel/setup.c|  6 ++---
 arch/ia64/kernel/efi.c   |  6 +++--
 arch/ia64/kernel/setup.c |  6 ++---
 arch/m32r/kernel/setup.c |  4 +--
 arch/mips/kernel/setup.c | 10 +---
 arch/parisc/mm/init.c|  6 ++---
 arch/powerpc/mm/mem.c|  2 +-
 arch/s390/kernel/setup.c |  8 +++---
 arch/score/kernel/setup.c|  2 +-
 arch/sh/kernel/setup.c   |  8 +++---
 arch/sparc/mm/init_64.c  |  8 +++---
 arch/tile/kernel/setup.c | 11 +---
 arch/unicore32/kernel/setup.c|  6 ++---
 arch/x86/kernel/e820.c   | 18 +-
 arch/x86/kernel/setup.c  |  6 ++---
 drivers/acpi/apei/einj.c | 15 ---
 drivers/xen/balloon.c|  2 +-
 include/linux/ioport.h   | 11 
 include/linux/mm.h   |  3 ++-
 kernel/kexec_core.c  |  6 ++---
 kernel/kexec_file.c  |  2 +-
 kernel/memremap.c| 13 +-
 kernel/resource.c| 54 +---
 mm/memory_hotplug.c  |  2 +-
 27 files changed, 140 insertions(+), 91 deletions(-)
--
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/11] arm/samsung: Change s3c_pm_run_res() to use System RAM type

[Krzysztof Kozlowski]

On 15.12.2015 08:37, Toshi Kani wrote:
> Change s3c_pm_run_res() to check with IORESOURCE_SYSTEM_RAM,
> instead of strcmp() with "System RAM", in the resource table.
> 
> No functional change is made to the interface.
> 
> Cc: Kukjin Kim 
> Cc: Krzysztof Kozlowski 
> Cc: linux-samsung-soc@vger.kernel.org
> Signed-off-by: Toshi Kani 
> ---
>  arch/arm/plat-samsung/pm-check.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

--
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 v4 20/20] ARM: dts: Add support of bus frequency for exynos4412-trats/odroidu3

[Krzysztof Kozlowski]

On 14.12.2015 15:38, Chanwoo Choi wrote:
> THis patch adds the bus device tree nodes for both MIF (Memory) and INT
> (Internal) block to enable the bus frequency.
> 
> The DMC bus is parent device in MIF block using VDD_MIF and the LEFTBUS
> bus is parent device in INT block using VDD_INT.
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 
> ---
>  arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 47 
> +
>  arch/arm/boot/dts/exynos4412-trats2.dts | 47 
> +
>  2 files changed, 94 insertions(+)
> 

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

--
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] drm/exynos: mixer: remove non-devicetree based initialization code

[Marek Szyprowski]

Exynos platform has been fully converted to device tree, so old platform
device based init data can be now removed.

Signed-off-by: Marek Szyprowski 
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 28 ++--
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c 
b/drivers/gpu/drm/exynos/exynos_mixer.c
index 4617b873d91f..ce313486d3bd 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1268,18 +1268,6 @@ static struct mixer_drv_data exynos4210_mxr_drv_data = {
.has_sclk = 1,
 };
 
-static const struct platform_device_id mixer_driver_types[] = {
-   {
-   .name   = "s5p-mixer",
-   .driver_data= (unsigned long)_mxr_drv_data,
-   }, {
-   .name   = "exynos5-mixer",
-   .driver_data= (unsigned long)_mxr_drv_data,
-   }, {
-   /* end node */
-   }
-};
-
 static struct of_device_id mixer_match_types[] = {
{
.compatible = "samsung,exynos4210-mixer",
@@ -1358,23 +1346,20 @@ static int mixer_probe(struct platform_device *pdev)
struct device *dev = >dev;
struct mixer_drv_data *drv;
struct mixer_context *ctx;
+   const struct of_device_id *match;
int ret;
 
+   if (!dev->of_node)
+   return -ENODEV;
+
ctx = devm_kzalloc(>dev, sizeof(*ctx), GFP_KERNEL);
if (!ctx) {
DRM_ERROR("failed to alloc mixer context.\n");
return -ENOMEM;
}
 
-   if (dev->of_node) {
-   const struct of_device_id *match;
-
-   match = of_match_node(mixer_match_types, dev->of_node);
-   drv = (struct mixer_drv_data *)match->data;
-   } else {
-   drv = (struct mixer_drv_data *)
-   platform_get_device_id(pdev)->driver_data;
-   }
+   match = of_match_node(mixer_match_types, dev->of_node);
+   drv = (struct mixer_drv_data *)match->data;
 
ctx->pdev = pdev;
ctx->dev = dev;
@@ -1479,5 +1464,4 @@ struct platform_driver mixer_driver = {
},
.probe = mixer_probe,
.remove = mixer_remove,
-   .id_table   = mixer_driver_types,
 };
-- 
1.9.2

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

Re: [PATCH] clk: exynos: use irqsave version of spin_lock to avoid deadlock with irqs

[Tomasz Figa]

Hi Marek,

2015-12-11 23:38 GMT+09:00 Marek Szyprowski :
> It is allowed to enable/disable clocks from interrupts, so common Exynos
> ARM clock management code for CPUfreq should use 'irqsave' version of
> spin_lock calls to avoid potential deadlock caused by spin_lock recursion.
> The same spin_lock is used by gate/mux clocks during enable/disable calls.
>
> This deadlock, can be reproduced by enabling CPUfreq (ondemand or
> userspace) and decoding video with s5p-mfc driver.

Good catch.

Acked-by: Tomasz Figa 

Best regards,
Tomasz
--
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/7] Exynos: MFC driver: reserved memory cleanup and IOMMU support

[Marek Szyprowski]

Hello,

On 2015-12-13 20:52, Laurent Pinchart wrote:

Hi Marek,

Thank you for the patches.

On Wednesday 09 December 2015 14:58:15 Marek Szyprowski wrote:

Hello,

This patchset finally perform cleanup of custom code in s5p-mfc codec
driver. The first part is removal of custom, driver specific code for
intializing and handling of reserved memory. Instead, a generic code for
reserved memory regions is used.

Should you update the reserved memory bindings documentation
(Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt) to
document usage of the memory-region-names property ?


Okay, I will update documentation as well.


Then, once it is done, the proper setup
of DMA parameters (max segment size) is applied for all multimedia
devices found on Exynos SoCs to let them properly handle shared buffers
mapped into contiguous DMA address space. The last patch adds support
for IOMMU to MFC driver. Some additional code is needed because of
specific requirements of MFC device firmware (see patch 7 for more
details). When no IOMMU is available, the code fallbacks to generic
reserved memory regions.

After applying this patchset, MFC device works correctly when IOMMU is
either enabled or disabled.

Patches have been tested on top of linux-next from 20151207. I would
prefer to merge patches 1-2 via Samsung tree and patches 3-7 via media
tree (there are no compile-time dependencies between patches 1-2 and
3-7). Patches have been tested on Odroid U3 (Exynos 4412 based) and
Odroid XU3 (Exynos 5422 based) boards.

Best regards
Marek Szyprowski
Samsung R Institute Poland


Changelog:
v2:
- reworked of_reserved_mem_init* functions on request from Rob Herring,
   added separate index and name based selection of reserved region
- adapted for of_reserved_mem_init* related changes

v1: https://www.mail-archive.com/linux-media@vger.kernel.org/msg94100.html
- initial version of another approach for this problem, rewrote driver code
   for new reserved memory bindings, which finally have been merged some
   time ago

v0:
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/189259.ht
ml - old patchset solving the same problem, abandoned due to other tasks and
long time of merging reserved memory bindings and support code for it

Patch summary:

Marek Szyprowski (7):
   ARM: Exynos: convert MFC device to generic reserved memory bindings
   ARM: dts: exynos4412-odroid*: enable MFC device
   of: reserved_mem: add support for named reserved mem nodes
   media: vb2-dma-contig: add helper for setting dma max seg size
   media: set proper max seg size for devices on Exynos SoCs
   media: s5p-mfc: replace custom reserved memory init code with generic
 one
   media: s5p-mfc: add iommu support

  .../devicetree/bindings/media/s5p-mfc.txt  |  16 +--
  arch/arm/boot/dts/exynos4210-origen.dts|  22 ++-
  arch/arm/boot/dts/exynos4210-smdkv310.dts  |  22 ++-
  arch/arm/boot/dts/exynos4412-odroid-common.dtsi|  24 
  arch/arm/boot/dts/exynos4412-origen.dts|  22 ++-
  arch/arm/boot/dts/exynos4412-smdk4412.dts  |  22 ++-
  arch/arm/boot/dts/exynos5250-arndale.dts   |  22 ++-
  arch/arm/boot/dts/exynos5250-smdk5250.dts  |  22 ++-
  arch/arm/boot/dts/exynos5250-spring.dts|  22 ++-
  arch/arm/boot/dts/exynos5420-arndale-octa.dts  |  22 ++-
  arch/arm/boot/dts/exynos5420-smdk5420.dts  |  22 ++-
  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi |  22 ++-
  arch/arm/mach-exynos/Makefile  |   2 -
  arch/arm/mach-exynos/exynos.c  |  19 ---
  arch/arm/mach-exynos/mfc.h |  16 ---
  arch/arm/mach-exynos/s5p-dev-mfc.c |  94 -
  drivers/media/platform/exynos-gsc/gsc-core.c   |   1 +
  drivers/media/platform/exynos4-is/fimc-core.c  |   1 +
  drivers/media/platform/exynos4-is/fimc-is.c|   1 +
  drivers/media/platform/exynos4-is/fimc-lite.c  |   1 +
  drivers/media/platform/s5p-g2d/g2d.c   |   1 +
  drivers/media/platform/s5p-jpeg/jpeg-core.c|   1 +
  drivers/media/platform/s5p-mfc/s5p_mfc.c   | 153 ++
  drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h |  79 +++
  drivers/media/platform/s5p-tv/mixer_video.c|   1 +
  drivers/media/v4l2-core/videobuf2-dma-contig.c |  15 ++
  drivers/of/of_reserved_mem.c   | 104 +++---
  include/linux/of_reserved_mem.h|  31 -
  include/media/videobuf2-dma-contig.h   |   1 +
  29 files changed, 533 insertions(+), 248 deletions(-)
  delete mode 100644 arch/arm/mach-exynos/mfc.h
  delete mode 100644 arch/arm/mach-exynos/s5p-dev-mfc.c
  create mode 100644 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h


Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a 

Re: [PATCH v4 05/20] PM / devfreq: Add new passive governor

[MyungJoo Ham]

>   
>  This patch adds the new passive governor for DEVFREQ framework. The following
> governors are already present and used for DVFS (Dynamic Voltage and Frequency
> Scaling) drivers. The following governors are independently used for one 
> device
> driver which don't give the influence to other device drviers and also don't
> receive the effect from other device drivers.
> - ondemand / performance / powersave / userspace
> 
> The passive governor depends on operation of parent driver with specific
> governos extremely and is not able to decide the new frequency by oneself.
> According to the decided new frequency of parent driver with governor,
> the passive governor uses it to decide the appropriate frequency for own
> device driver. The passive governor must need the following information
> from device tree:
> - the source clock and OPP tables
> - the instance of parent device
> 
> For exameple,
> there are one more devfreq device drivers which need to change their source
> clock according to their utilization on runtime. But, they share the same
> power line (e.g., regulator). So, specific device driver is operated as parent
> with ondemand governor and then the rest device driver with passive governor
> is influenced by parent device.
> 
> Suggested-by: Myungjoo Ham 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 
> ---
>  drivers/devfreq/Kconfig|   9 
>  drivers/devfreq/Makefile   |   1 +
>  drivers/devfreq/devfreq.c  |  47 
>  drivers/devfreq/governor_passive.c | 108 
> +
>  include/linux/devfreq.h|  15 ++
>  5 files changed, 180 insertions(+)
>  create mode 100644 drivers/devfreq/governor_passive.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 55ec774f794c..d03f635a93e1 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -64,6 +64,15 @@ config DEVFREQ_GOV_USERSPACE
> Otherwise, the governor does not change the frequnecy
> given at the initialization.
>  
> +config DEVFREQ_GOV_PASSIVE
> + tristate "Passive"
> + help
> +   Sets the frequency by other governors (simple_ondemand, performance,
> +   powersave, usersapce) of a parent devfreq device. This governor
> +   always has the dependency on the chosen frequency from paired
> +   governor. This governor does not change the frequency by oneself
> +   through sysfs entry.

Sets the frequency based on the frequency of its parent devfreq
device. This governor does not change the frequency by itself
through sysfs entries.

> +
>  comment "DEVFREQ Drivers"
>  
>  config ARM_EXYNOS_BUS_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 375ebbb4fcfb..f81c313b4b79 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
[]
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 984c5e9e7bdd..15e58779e4c0 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -190,6 +190,31 @@ static struct devfreq_governor 
> *find_devfreq_governor(const char *name)
>  
>  /* Load monitoring helper functions for governors use */
>  
> +static int update_devfreq_passive(struct devfreq *devfreq, unsigned long 
> freq)
> +{
> + struct devfreq *passive;
> + unsigned long rate;
> + int ret;
> +
> + list_for_each_entry(passive, >passive_dev_list, passive_node) {
> + if (!passive->governor)
> + continue;
> + rate = freq;
> +
> + ret = passive->governor->get_target_freq(passive, );
> + if (ret)
> + return ret;
> +
> + ret = passive->profile->target(passive->dev.parent, , 0);
> + if (ret)
> + return ret;
> +
> + passive->previous_freq = rate;
> + }
> +
> + return 0;
> +}
> +
>  /**
>   * update_devfreq() - Reevaluate the device and configure frequency.
>   * @devfreq: the devfreq instance.
> @@ -233,10 +258,18 @@ int update_devfreq(struct devfreq *devfreq)
>   flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */
>   }
>  
> + if (!list_empty(>passive_dev_list)
> + && devfreq->previous_freq > freq)
> + update_devfreq_passive(devfreq, freq);
> +

Could you please comment somewhere appropriate
that the dependent is going to be changed
before its parent if the frequency is going down.
(and after if going up)
And state why as well.

And, is this viable universally?

>   err = devfreq->profile->target(devfreq->dev.parent, , flags);
>   if (err)
>   return err;
>  
> + if (!list_empty(>passive_dev_list)
> + && devfreq->previous_freq < freq)
> + update_devfreq_passive(devfreq, freq);
> +
>   if 

Re: [PATCH v4 06/20] PM / devfreq: Add devfreq_get_devfreq_by_phandle()

[MyungJoo Ham]

>   
>  This patch adds the new devfreq_get_devfreq_by_phandle() OF helper function
> which can find the instance of devfreq device by using phandle ("devfreq").
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 

Signed-off-by: MyungJoo Ham 

> ---
>  drivers/devfreq/devfreq.c | 44 
>  include/linux/devfreq.h   |  9 +
>  2 files changed, 53 insertions(+)
> 

Re: [PATCH v2 4/7] media: vb2-dma-contig: add helper for setting dma max seg size

[Marek Szyprowski]

Hello,

On 2015-12-13 20:57, Laurent Pinchart wrote:

Hi Marek,

Thank you for the patch.

On Wednesday 09 December 2015 14:58:19 Marek Szyprowski wrote:

Add a helper function for device drivers to set DMA's max_seg_size.
Setting it to largest possible value lets DMA-mapping API always create
contiguous mappings in DMA address space. This is essential for all
devices, which use dma-contig videobuf2 memory allocator and shared
buffers.

Signed-off-by: Marek Szyprowski 
---
  drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
  include/media/videobuf2-dma-contig.h   |  1 +
  2 files changed, 16 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
b/drivers/media/v4l2-core/videobuf2-dma-contig.c index c331272..628518d
100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -742,6 +742,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
  }
  EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);

+int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size)
+{
+   if (!dev->dma_parms) {

When can this function be called with dev->dma_parms not NULL ?


When one loads a module with multimedia driver (which calls this 
function), then

unloads and loads it again. It is rather safe to have this check.


+   dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
+ GFP_KERNEL);
+   if (!dev->dma_parms)
+   return -ENOMEM;
+   }
+   if (dma_get_max_seg_size(dev) < size)
+   return dma_set_max_seg_size(dev, size);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);
+
  MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2");
  MODULE_AUTHOR("Pawel Osciak ");
  MODULE_LICENSE("GPL");
diff --git a/include/media/videobuf2-dma-contig.h
b/include/media/videobuf2-dma-contig.h index c33dfa6..0e6ba64 100644
--- a/include/media/videobuf2-dma-contig.h
+++ b/include/media/videobuf2-dma-contig.h
@@ -26,6 +26,7 @@ vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb,
unsigned int plane_no)

  void *vb2_dma_contig_init_ctx(struct device *dev);
  void vb2_dma_contig_cleanup_ctx(void *alloc_ctx);
+int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size);

  extern const struct vb2_mem_ops vb2_dma_contig_memops;


Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

--
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 v4 07/20] PM / devfreq: Show the related information according to governor type

[MyungJoo Ham]

>   
>  This patch modifies the following sysfs entry of DEVFREQ framework
> because the devfreq device using passive governor don't need the same
> information of the devfreq device using rest governor.
> - polling_interval: passive gov don't use the sampling rate.
> - available_governors : passive gov don't be changed on runtime in this 
> version.
> - trans_stat  : passive governor don't support trans_stat in this 
> version.
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 

You have a major update that is not mendtioned in the commit message.
(add governor "type")

> ---
>  drivers/devfreq/devfreq.c | 31 
> +--
>  drivers/devfreq/governor.h|  7 +++
>  drivers/devfreq/governor_passive.c|  1 +
>  drivers/devfreq/governor_performance.c|  1 +
>  drivers/devfreq/governor_powersave.c  |  1 +
>  drivers/devfreq/governor_simpleondemand.c |  1 +
>  drivers/devfreq/governor_userspace.c  |  1 +
>  include/linux/devfreq.h   |  2 ++
>  8 files changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 78ea4cdaa82c..18ad956fec93 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -597,7 +597,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>   goto err_init;
>   }
>  
> - if (!strncmp(devfreq->governor_name, "passive", 7)) {
> + if (devfreq->governor->type == DEVFREQ_GOV_PASSIVE) {
>   struct devfreq *parent_devfreq =
>   ((struct devfreq_passive_data *)data)->parent;

As mentioned in a previous reply, this code may be removed.

>  
> @@ -963,13 +963,23 @@ static ssize_t available_governors_show(struct device 
> *d,
>   struct device_attribute *attr,
>   char *buf)
>  {
> - struct devfreq_governor *tmp_governor;
> + struct devfreq *devfreq = to_devfreq(d);
>   ssize_t count = 0;
>  
>   mutex_lock(_list_lock);
> - list_for_each_entry(tmp_governor, _governor_list, node)
> + if (devfreq->governor->type == DEVFREQ_GOV_PASSIVE) {
>   count += scnprintf([count], (PAGE_SIZE - count - 2),
> -"%s ", tmp_governor->name);
> +"%s ", devfreq->governor->name);
> + } else {
> + struct devfreq_governor *tmp_governor;
> +
> + list_for_each_entry(tmp_governor, _governor_list, node) 
> {
> + if (tmp_governor->type == DEVFREQ_GOV_PASSIVE)
> + continue;
> + count += scnprintf([count], (PAGE_SIZE - count - 2),
> +"%s ", tmp_governor->name);
> + }
> + }

Uh no. As long as we do not have a list of all possible governors
for each device, let's stick with what we've defined in ABI documentation:
show all available governors "in the system".

You MAY have a device that may run both PASSIVE and USERSPACE.

>   mutex_unlock(_list_lock);
>  
>   /* Truncate the trailing space */
> @@ -1006,6 +1016,11 @@ static DEVICE_ATTR_RO(target_freq);
>  static ssize_t polling_interval_show(struct device *dev,
>struct device_attribute *attr, char *buf)
>  {
> + struct devfreq *df = to_devfreq(dev);
> +
> + if (df->governor->type == DEVFREQ_GOV_PASSIVE)
> + return sprintf(buf, "Not Supported.\n");
> +
>   return sprintf(buf, "%d\n", to_devfreq(dev)->profile->polling_ms);
>  }

When polling interval is irrlevent with the governor,
you don't need to print "Not Supported". Instead,
printing "0" is enough, (polling_ms=0 == no polling)
which is what devfreq is doing now.

>  
> @@ -1020,6 +1035,9 @@ static ssize_t polling_interval_store(struct device 
> *dev,
>   if (!df->governor)
>   return -EINVAL;
>  
> + if (df->governor->type == DEVFREQ_GOV_PASSIVE)
> + return -EINVAL;
> +

Please simply return -EINVAL with governor's event_handler with 
DEVFREQ_GOV_INTERVAL
(I see the return value checking is missing at df->governor->event_handler().
You may want to add return value checking there to properly handle what you 
want.)

>   ret = sscanf(buf, "%u", );
>   if (ret != 1)
>   return -EINVAL;
> @@ -1137,11 +1155,12 @@ static ssize_t trans_stat_show(struct device *dev,
>   int i, j;
>   unsigned int max_state = devfreq->profile->max_state;
>  
> + if (max_state == 0 || devfreq->governor->type == DEVFREQ_GOV_PASSIVE)
> + return sprintf(buf, "Not Supported.\n");
> +
>   if (!devfreq->stop_polling &&
>   devfreq_update_status(devfreq, devfreq->previous_freq))
>   return 0;
> - if (max_state == 0)
> -  

Re: [PATCH v2 06/22] drm/exynos: move dma_addr attribute from exynos plane to exynos fb

[Marek Szyprowski]

Hi Inki,

On 2015-12-11 15:52, Inki Dae wrote:

2015-12-11 20:27 GMT+09:00 Marek Szyprowski :

On 2015-12-11 10:57, Inki Dae wrote:

2015년 12월 11일 18:26에 Marek Szyprowski 이(가) 쓴 글:

On 2015-12-11 10:02, Inki Dae wrote:

I found out why NULL point access happened. That was incurred by below
your patch,
[PATCH] drm/exynos: move dma_addr attribute from exynos plane to exynos
fb

When another crtc driver is hotplugged in runtime such as HDMI or VIDI,
the drm frambuffer object of fb_helper is set to the plane state of the
new crtc driver
so the driver should get the drm framebuffer object from the plane's
state or
exynos_plane->pending_fb which is set by exynos_plane_atomic_update
function.

After that, I think the drm framebuffer should be set to drm plane as a
current fb
which would be scanned out.

Anyway, I can fix it like below if you are ok.

Thanks,
Inki Dae

--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -137,7 +137,7 @@ static void vidi_update_plane(struct exynos_drm_crtc
*crtc,
   if (ctx->suspended)
   return;
-   addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
+   addr = exynos_drm_fb_dma_addr(plane->pending_fb, 0);
   DRM_DEBUG_KMS("dma_addr = %pad\n", );

+plane->base.fb = plane->pending_fb;

plane->base.fb should not be modified. I think that the following fix is
more

Could you explain why plane->base.fb shouldn't be modified?


All 'base' classes are modified by DRM core and there should be no need
to modify them from the drivers.

Ok, If so - drm core updates plane->fb - then it's reasonable. But I
couldn't find the exact location where plane->fb is set to a fb to be
scanned out.
So could you let me know the exact location? it's not clear to me yet.


Setting plane->fb is wired somewhere in the atomic update logic, see
__setplane_internal() function in drivers/gpu/drm/drm_crtc.c. Some more 
comments
are also in the drm_atomic_clean_old_fb() function in 
drivers/gpu/drm/drm_atomic.c

However I don't know the exact call path.


I've checked this issue and the proper fix for is the following code:

--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -131,13 +131,14 @@ static void vidi_disable_vblank(struct exynos_drm_crtc
*crtc)
  static void vidi_update_plane(struct exynos_drm_crtc *crtc,
   struct exynos_drm_plane *plane)
  {
+   struct drm_plane_state *state = plane->base.state;
 struct vidi_context *ctx = crtc->ctx;
 dma_addr_t addr;

 if (ctx->suspended)
 return;

-   addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
+   addr = exynos_drm_fb_dma_addr(state->fb, 0);
 DRM_DEBUG_KMS("dma_addr = %pad\n", );

 if (ctx->vblank_on)


plane->base.fb is updated from the core once all crtc/plane atomic update
calls finishes. The drivers should use the fb stored in plane->base.state.
I've missed that VIDI driver doesn't get the fb and incorrectly used
plane->base.fb instad of state->fb while updating the code.


Actually, I used state->fb instead of plane->pending_fb but in
exynos_plane_atomic_update function, plane->pending_fb is set to
state->fb.
That is why I commented like below,
" so the driver should get the drm framebuffer object from the plane's
state or exynos_plane->pending_fb which is set by
exynos_plane_atomic_update function."

Anyway, using state->fb looks like more consistent with other drivers,
fimd, decon and mixer.


Thanks for applying my fix and merging this patch.


In case that userspace requests setplane, plane->base.fb would be updated
after update_plane callback.
However, in other cases, I don't see how plane->base.fb could be updated.
In this case, I think vendor specific drivers would need to update it as a
current fb to be scanned out like other some drivers did.
Of course, it may be possible for drm core part to take care of it
appropriately later.

Thanks,
Inki Dae


appropriate:
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -132,12 +132,13 @@ static void vidi_update_plane(struct
exynos_drm_crtc *crtc,
struct exynos_drm_plane *plane)
   {
  struct vidi_context *ctx = crtc->ctx;
-   dma_addr_t addr;
+   dma_addr_t addr = 0;

  if (ctx->suspended)
  return;

-   addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
+   if (plane->base.fb)
+   addr = exynos_drm_fb_dma_addr(plane->base.fb, 0);
  DRM_DEBUG_KMS("dma_addr = %pad\n", );

  if (ctx->vblank_on)

I will investigate the case of NULL plane->state.fb, because it might be
relevant
to other crtc drivers as well.



 if (ctx->vblank_on)


2015년 12월 10일 22:05에 Inki Dae 이(가) 쓴 글:

2015년 11월 30일 22:53에 Marek Szyprowski 이(가) 쓴 글:

DMA address is a framebuffer attribute and the right place for it is

Re: [PATCH v2 4/7] media: vb2-dma-contig: add helper for setting dma max seg size

[Laurent Pinchart]

Hi Marek,

On Monday 14 December 2015 10:20:22 Marek Szyprowski wrote:
> On 2015-12-13 20:57, Laurent Pinchart wrote:
> > On Wednesday 09 December 2015 14:58:19 Marek Szyprowski wrote:
> >> Add a helper function for device drivers to set DMA's max_seg_size.
> >> Setting it to largest possible value lets DMA-mapping API always create
> >> contiguous mappings in DMA address space. This is essential for all
> >> devices, which use dma-contig videobuf2 memory allocator and shared
> >> buffers.
> >> 
> >> Signed-off-by: Marek Szyprowski 
> >> ---
> >> 
> >>   drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++
> >>   include/media/videobuf2-dma-contig.h   |  1 +
> >>   2 files changed, 16 insertions(+)
> >> 
> >> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> >> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index c331272..628518d
> >> 100644
> >> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> >> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> >> @@ -742,6 +742,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
> >> 
> >>   }
> >>   EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
> >> 
> >> +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int
> >> size)
> >> +{
> >> +  if (!dev->dma_parms) {
> > 
> > When can this function be called with dev->dma_parms not NULL ?
> 
> When one loads a module with multimedia driver (which calls this
> function), then unloads and loads it again. It is rather safe to have this
> check.

Don't you have a much bigger problem in that case ? When you unload the module 
the device will be unbound from the driver, causing the memory allocated by 
devm_kzalloc to be freed. dev->dma_parms will then point to freed memory, 
which will get reused by all subsequent calls to dma_get_max_seg_size(), 
dma_get_max_seg_size() & co (including the ones in this function).

> >> +  dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
> >> +GFP_KERNEL);
> >> +  if (!dev->dma_parms)
> >> +  return -ENOMEM;
> >> +  }
> >> +  if (dma_get_max_seg_size(dev) < size)
> >> +  return dma_set_max_seg_size(dev, size);
> >> +
> >> +  return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);
> >> +
> >>   MODULE_DESCRIPTION("DMA-contig memory handling routines for
> >>   videobuf2");
> >>   MODULE_AUTHOR("Pawel Osciak ");
> >>   MODULE_LICENSE("GPL");
> >> diff --git a/include/media/videobuf2-dma-contig.h
> >> b/include/media/videobuf2-dma-contig.h index c33dfa6..0e6ba64 100644
> >> --- a/include/media/videobuf2-dma-contig.h
> >> +++ b/include/media/videobuf2-dma-contig.h
> >> @@ -26,6 +26,7 @@ vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb,
> >> unsigned int plane_no)
> >>  void *vb2_dma_contig_init_ctx(struct device *dev);
> >>  void vb2_dma_contig_cleanup_ctx(void *alloc_ctx);
> >> +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int
> >> size);
> >>  extern const struct vb2_mem_ops vb2_dma_contig_memops;

-- 
Regards,

Laurent Pinchart

--
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/3] Device tree binding documentation for gpio-switch

[Rob Herring]

On Mon, Dec 14, 2015 at 8:28 AM, Linus Walleij  wrote:
> On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring  wrote:
>> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij  
>> wrote:
>>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch
>>>  wrote:

[...]

>>> Markus Pargmann also did a series that add initial values to
>>> hogs, which is the inverse usecase of this, where you want to
>>> *output* something by default, then maybe also make it available
>>> to userspace.
>>>
>>> So what we need to see here is a patch series that does all of these
>>> things:
>>>
>>> - Name lines
>>>
>>> - Sets them to initial values
>>>
>>> - Mark them as read-only
>>>
>>> - Mark them as "not used by the operating system" so that they
>>>   can be default-exported to userspace.
>>
>> No! This should not be a DT property.
>>
>> Whether I want to control a GPIO in the kernel or userspace is not
>> known and can change over time. It could simply depend on kernel
>> config. There is also the case that a GPIO has no connection or kernel
>> driver until some time later when a DT overlay for an expansion board
>> is applied.
>
> That's correct. So from a DT point of view, what really matters is
> to give things a name, and the hogs and initvals syntax already
> has a structure for this that is in use
> (from Documentation/devicetree/bindings/gpio/gpio.txt):
>
> qe_pio_a: gpio-controller@1400 {
> compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank";
> reg = <0x1400 0x18>;
> gpio-controller;
> #gpio-cells = <2>;
>
> line_b {
> gpio-hog;
> gpios = <6 0>;
> output-low;
> line-name = "foo-bar-gpio";
> };
> };
>
> Markus use this also for initial values. That could easily be used in
> a budget version like this:
>
> line_b {
> /* Just naming */
> gpios = <6 0>;
> line-name = "foo-bar-gpio";
> };
>
> That could grow kind of big though. Or maybe not? How many
> GPIO lines are actually properly named in a typical system?

We should limit it to GPIOs with no connection to another node. For
example, I don't want to see a SD card detect in the list as that
should be in the SD host node. However, that is hard to enforce and
can change over time. Removing it would break userspace potentially.
Of course if the kernel starts own a signal that userspace used, then
that potentially breaks userspace regardless of the DT changing. OTOH,
using GPIOs in userspace is kind of use at your own risk.

The only real differences between this and Martyn's proposal are the
location of the nodes and having a compatible string. A compatible
string may be good to have. It indicates type of signal, but not
specific use. Similar to how gpio-key compatible defines the function,
but not what key code, or gpio-led does not say what color LED. A
compatible here could cover switches, mode/id/revision strapping
signals, jumpers, presence detect, etc.

> The problem is that naming is usually imposed by consumers (they
> are the ones who know how the line is used).
>
> And then I do not mean naming it after the pin on the capsule
> where it comes out as per the datasheet, but
> naming it after the actual use.

Right. We need a way to query "I need the GPIO that does X" which
works across boards.

> Naming it after the hardware specs is something the operating
> system can do, in Linux' case by the array char *names; inside the
> struct gpio_chip and should not be part of the bindings IMO.

Agreed. That just came up with STM32 gpio bindings[1].

> I would rather imagine this is something used in a top-level board
> file naming it: "header-2-22" for the 22nd pin on some second header
> on my BeagleBone Black or something like that. And those may not
> be so vast in numbers so they could be named using this pattern.

Exactly. This is one of the cases I care about. However, this is not
really a function, but it is SOC independent at least.

We also have to consider how to handle add-on boards. We probably need
a connector node which can remap connector signals to host signals in
order to have overlays independent of the host board DT. For GPIOs
this is probably a gpio-map property similar to interrupt-map. The
complicated part will be connectors that require pinmux setup of the
host.

Rob

[1] https://lkml.org/lkml/2015/12/11/600
--
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 v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

[Krzysztof Kozlowski]

On 14.12.2015 15:38, Chanwoo Choi wrote:
> This patch adds the generic exynos bus frequency driver for AMBA AXI bus
> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
> have the common architecture for bus between DRAM and sub-blocks in SoC.
> This driver can support the generic bus frequency driver for Exynos SoCs.
> 
> In devicetree, Each bus block has a bus clock, regulator, operation-point
> and devfreq-event devices which measure the utilization of each bus block.
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 
> ---
>  drivers/devfreq/Kconfig |  15 ++
>  drivers/devfreq/Makefile|   1 +
>  drivers/devfreq/exynos/Makefile |   1 +
>  drivers/devfreq/exynos/exynos-bus.c | 449 
> 
>  4 files changed, 466 insertions(+)
>  create mode 100644 drivers/devfreq/exynos/exynos-bus.c
> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 64281bb2f650..55ec774f794c 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -66,6 +66,21 @@ config DEVFREQ_GOV_USERSPACE
>  
>  comment "DEVFREQ Drivers"
>  
> +config ARM_EXYNOS_BUS_DEVFREQ
> + bool "ARM EXYNOS Generic Memory Bus DEVFREQ Driver"
> + depends on ARCH_EXYNOS
> + select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + select DEVFREQ_EVENT_EXYNOS_PPMU
> + select PM_DEVFREQ_EVENT
> + select PM_OPP
> + help
> +   This adds the common DEVFREQ driver for Exynos Memory bus. Exynos
> +   Memory bus has one more group of memory bus (e.g, MIF and INT block).
> +   Each memory bus group could contain many memoby bus block. It reads
> +   PPMU counters of memory controllers by using DEVFREQ-event device
> +   and adjusts the operating frequencies and voltages with OPP support.
> +   This does not yet operate with optimal voltages.
> +
>  config ARM_EXYNOS4_BUS_DEVFREQ
>   bool "ARM Exynos4210/4212/4412 Memory Bus DEVFREQ Driver"
>   depends on (CPU_EXYNOS4210 || SOC_EXYNOS4212 || SOC_EXYNOS4412) && 
> !ARCH_MULTIPLATFORM
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 5134f9ee983d..375ebbb4fcfb 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)   += governor_powersave.o
>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)  += governor_userspace.o
>  
>  # DEVFREQ Drivers
> +obj-$(CONFIG_ARCH_EXYNOS)+= exynos/

Why limiting it to ARCH_EXYNOS? Are there real dependencies on exynos
mach code? Or on ARM code?

If not, then this probably should be obj-y to allow compile testing.
Particular objects would be selected by ARM_EXYNOS_BUS_DEVFREQ.

Best regards,
Krzysztof

--
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 v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

[Chanwoo Choi]

On 2015년 12월 14일 17:28, MyungJoo Ham wrote:
>>   
>>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>
>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>> and devfreq-event devices which measure the utilization of each bus block.
>>
>> Signed-off-by: Chanwoo Choi 
>> [linux.amoon: Tested on Odroid U3]
>> Tested-by: Anand Moon 
>>
> 
> Chanwoo, could you please show me testing this set of patches in your site?
> Please let me know when is ok to visit you.
> (I do not have exynos machines right now.)
> 
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 5134f9ee983d..375ebbb4fcfb 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)  += governor_powersave.o
>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>  
>>  # DEVFREQ Drivers
>> +obj-$(CONFIG_ARCH_EXYNOS)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)   += exynos/
> 
> CONFIG_ARCH_EXYNOS is true if
>   CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
>   or
>   CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
> Thus, the two lines after you've added have become useless. (dead code)
> 
> Please delete them.

In this series, patch11 deletes all of both exynos4_bus.c and exynos5_bus.c.

> 
> []
>> --- /dev/null
>> +++ b/drivers/devfreq/exynos/exynos-bus.c
> []
>> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
>> flags)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct dev_pm_opp *new_opp;
>> +unsigned long old_freq, new_freq, old_volt, new_volt;
>> +int ret = 0;
>> +
>> +/* Get new opp-bus instance according to new bus clock */
>> +rcu_read_lock();
>> +new_opp = devfreq_recommended_opp(dev, freq, flags);
>> +if (IS_ERR_OR_NULL(new_opp)) {
>> +dev_err(dev, "failed to get recommed opp instance\n");
>> +rcu_read_unlock();
>> +return PTR_ERR(new_opp);
>> +}
>> +
>> +new_freq = dev_pm_opp_get_freq(new_opp);
>> +new_volt = dev_pm_opp_get_voltage(new_opp);
>> +old_freq = dev_pm_opp_get_freq(bus->curr_opp);
>> +old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +if (old_freq == new_freq)
>> +return 0;
>> +
>> +/* Change voltage and frequency according to new OPP level */
>> +mutex_lock(>lock);
>> +
>> +if (old_freq < new_freq) {
>> +ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);
> 
> Setting the maximum volt same as the minimum volt is not recommended.
> Especially for any DVFS mechanisms, I recommend to set values as:
> min_volt = minimum voltage that does not harm the stability
> max_volt = maximum voltage that does not break the circuit
> 
> Please refer to /include/linux/regulator/driver.h
> "@set_voltage" comments.
> 
> For the rest of regulator_set_voltage usages, I'd say the same.

OK.
I'll add the 'voltage-tolerance' property as cpufreq-dt.c driver.
The cpufreq-dt.c get the percentage value by using 'voltage-tolerance'
devicetree property.

For example,
if (of_property_read_u32(np, "exynos,voltage-tolerance",
>voltage_tolerance))
bus->voltage_tolerance = DEFAULT_VOLTAGE_TOLERANCE;

tol = new_volt * bus->voltage_tolerance / 100;
regulator_set_voltage_tol(regulator, new_volt, tol);

> 
> []
>> +static int exynos_bus_get_dev_status(struct device *dev,
>> + struct devfreq_dev_status *stat)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct devfreq_event_data edata;
>> +int ret;
>> +
>> +rcu_read_lock();
>> +stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +ret = exynos_bus_get_event(bus, );
>> +if (ret < 0) {
>> +stat->total_time = stat->busy_time = 0;
>> +goto err;
>> +}
>> +
>> +stat->busy_time = (edata.load_count * 100) / bus->ratio;
>> +stat->total_time = edata.total_count;
>> +
>> +dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
>> +stat->total_time);
> 
> These two values are unsigned long.

OK. I'll modify it (%ld -> %lu)

> 
> []
>> +static int exynos_bus_parse_of(struct device_node *np,
>> +  struct exynos_bus *bus)
>> +{
>> +struct device *dev = bus->dev;
>> +unsigned long rate;
>> +int i, ret, count, size;
>> +
>> +/* Get the clock to provide each bus with source clock */
>> +

Re: [PATCH 1/3] Device tree binding documentation for gpio-switch

[Linus Walleij]

On Fri, Dec 11, 2015 at 3:06 PM, Rob Herring  wrote:
> On Fri, Dec 11, 2015 at 6:39 AM, Linus Walleij  
> wrote:
>> On Fri, Dec 4, 2015 at 6:31 PM, Martyn Welch
>>  wrote:
>>
>>> This patch adds documentation for the gpio-switch binding. This binding
>>> provides a mechanism to bind named links to gpio, with the primary
>>> purpose of enabling standardised access to switches that might be standard
>>> across a group of devices but implemented differently on each device.
>>>
>>> Signed-off-by: Martyn Welch 
>>
>> As mentioned in the comment to the second patch, this solves the
>> following generic problem:
>>
>> Expose a GPIO line to userspace using a specific name
>>
>> That means basically naming GPIO lines and marking them as
>> "not used by the operating system".
>>
>> This is something that has been proposed before, and postponed
>> because the kernel lacks the right infrastructure.
>
> That doesn't necessarily mean we can't define a binding.

Yeah that's true. just saying that the other stuff was not merged
for this reason, but then we can have a look at Markus' bindings
in parallell, Markus can you repost them to this audience? Only
the bindings I mean.

>> Markus Pargmann also did a series that add initial values to
>> hogs, which is the inverse usecase of this, where you want to
>> *output* something by default, then maybe also make it available
>> to userspace.
>>
>> So what we need to see here is a patch series that does all of these
>> things:
>>
>> - Name lines
>>
>> - Sets them to initial values
>>
>> - Mark them as read-only
>>
>> - Mark them as "not used by the operating system" so that they
>>   can be default-exported to userspace.
>
> No! This should not be a DT property.
>
> Whether I want to control a GPIO in the kernel or userspace is not
> known and can change over time. It could simply depend on kernel
> config. There is also the case that a GPIO has no connection or kernel
> driver until some time later when a DT overlay for an expansion board
> is applied.

That's correct. So from a DT point of view, what really matters is
to give things a name, and the hogs and initvals syntax already
has a structure for this that is in use
(from Documentation/devicetree/bindings/gpio/gpio.txt):

qe_pio_a: gpio-controller@1400 {
compatible = "fsl,qe-pario-bank-a", "fsl,qe-pario-bank";
reg = <0x1400 0x18>;
gpio-controller;
#gpio-cells = <2>;

line_b {
gpio-hog;
gpios = <6 0>;
output-low;
line-name = "foo-bar-gpio";
};
};

Markus use this also for initial values. That could easily be used in
a budget version like this:

line_b {
/* Just naming */
gpios = <6 0>;
line-name = "foo-bar-gpio";
};

That could grow kind of big though. Or maybe not? How many
GPIO lines are actually properly named in a typical system?

The problem is that naming is usually imposed by consumers (they
are the ones who know how the line is used).

And then I do not mean naming it after the pin on the capsule
where it comes out as per the datasheet, but
naming it after the actual use.

Naming it after the hardware specs is something the operating
system can do, in Linux' case by the array char *names; inside the
struct gpio_chip and should not be part of the bindings IMO.

I would rather imagine this is something used in a top-level board
file naming it: "header-2-22" for the 22nd pin on some second header
on my BeagleBone Black or something like that. And those may not
be so vast in numbers so they could be named using this pattern.

Yours,
Linus Walleij
--
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] clk: exynos: use irqsave version of spin_lock to avoid deadlock with irqs

[Sylwester Nawrocki]

On 11/12/15 15:38, Marek Szyprowski wrote:
> It is allowed to enable/disable clocks from interrupts, so common Exynos
> ARM clock management code for CPUfreq should use 'irqsave' version of
> spin_lock calls to avoid potential deadlock caused by spin_lock recursion.
> The same spin_lock is used by gate/mux clocks during enable/disable calls.
> 
> This deadlock, can be reproduced by enabling CPUfreq (ondemand or
> userspace) and decoding video with s5p-mfc driver.

> Signed-off-by: Marek Szyprowski 
> CC: sta...@vger.kernel.org  # v4.2+

Acked-by: Sylwester Nawrocki 

Mike, Stephen, could you apply this patch directly?
It would be nice to have it in 4.4 as the bug fixed here causes
some of exynos boards in mainline to fail booting with default
config. I could resend the patch directly to you if needed.

Thanks,
Sylwester

> ---
>  drivers/clk/samsung/clk-cpu.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
> index 2fe37f708dc7..813003d6ce09 100644
> --- a/drivers/clk/samsung/clk-cpu.c
> +++ b/drivers/clk/samsung/clk-cpu.c
> @@ -148,6 +148,7 @@ static int exynos_cpuclk_pre_rate_change(struct 
> clk_notifier_data *ndata,
>   unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent);
>   unsigned long alt_div = 0, alt_div_mask = DIV_MASK;
>   unsigned long div0, div1 = 0, mux_reg;
> + unsigned long flags;
>  
>   /* find out the divider values to use for clock data */
>   while ((cfg_data->prate * 1000) != ndata->new_rate) {
> @@ -156,7 +157,7 @@ static int exynos_cpuclk_pre_rate_change(struct 
> clk_notifier_data *ndata,
>   cfg_data++;
>   }
>  
> - spin_lock(cpuclk->lock);
> + spin_lock_irqsave(cpuclk->lock, flags);
>  
>   /*
>* For the selected PLL clock frequency, get the pre-defined divider
> @@ -212,7 +213,7 @@ static int exynos_cpuclk_pre_rate_change(struct 
> clk_notifier_data *ndata,
>   DIV_MASK_ALL);
>   }
>  
> - spin_unlock(cpuclk->lock);
> + spin_unlock_irqrestore(cpuclk->lock, flags);
>   return 0;
>  }
>  
> @@ -223,6 +224,7 @@ static int exynos_cpuclk_post_rate_change(struct 
> clk_notifier_data *ndata,
>   const struct exynos_cpuclk_cfg_data *cfg_data = cpuclk->cfg;
>   unsigned long div = 0, div_mask = DIV_MASK;
>   unsigned long mux_reg;
> + unsigned long flags;
>  
>   /* find out the divider values to use for clock data */
>   if (cpuclk->flags & CLK_CPU_NEEDS_DEBUG_ALT_DIV) {
> @@ -233,7 +235,7 @@ static int exynos_cpuclk_post_rate_change(struct 
> clk_notifier_data *ndata,
>   }
>   }
>  
> - spin_lock(cpuclk->lock);
> + spin_lock_irqsave(cpuclk->lock, flags);
>  
>   /* select mout_apll as the alternate parent */
>   mux_reg = readl(base + E4210_SRC_CPU);
> @@ -246,7 +248,7 @@ static int exynos_cpuclk_post_rate_change(struct 
> clk_notifier_data *ndata,
>   }
>  
>   exynos_set_safe_div(base, div, div_mask);
> - spin_unlock(cpuclk->lock);
> + spin_unlock_irqrestore(cpuclk->lock, flags);
>   return 0;
>  }
--
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/3] cpuidle: avoid module usage in non-modular code

[Paul Gortmaker]

[Re: [PATCH 0/3] cpuidle: avoid module usage in non-modular code] On 14/12/2015 
(Mon 22:31) Rafael J. Wysocki wrote:

> On Sunday, December 13, 2015 06:57:09 PM Paul Gortmaker wrote:
> > This series of commits is a part of a larger project to ensure
> > people don't reference modular support functions in non-modular
> > code.  Overall there was roughly 5k lines of dead code in the
> > kernel due to this.  So far we've fixed several areas, like tty,
> > x86, net, ... and we continue to work on other areas.

[...]

> 
> If no one objects, I can queue up this series for 4.5 unless you have other
> plans with respect to it.

Please do.

I was hoping to spread as many of these around as possible so I don't
end up with a giant pull request to Linus.  There is code out there
without a clear maintainership path, so eventually I'll have to send
some his way (or via akpm) but the less that end up in that pile, the
better IMHO.

Thanks,
Paul.
--
> 
> Thanks,
> Rafael
> 
--
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/3] cpuidle: avoid module usage in non-modular code

[Rafael J. Wysocki]

On Sunday, December 13, 2015 06:57:09 PM Paul Gortmaker wrote:
> This series of commits is a part of a larger project to ensure
> people don't reference modular support functions in non-modular
> code.  Overall there was roughly 5k lines of dead code in the
> kernel due to this.  So far we've fixed several areas, like tty,
> x86, net, ... and we continue to work on other areas.
> 
> There are several reasons to not use module support for code that
> can never be built as a module, but the big ones are:
> 
>  (1) it is easy to accidentally code up unused module_exit and remove code
>  (2) it can be misleading when reading the source, thinking it can be
>   modular when the Makefile and/or Kconfig prohibit it
>  (3) it requires the include of the module.h header file which in turn
>  includes nearly everything else.
> 
> Fortunately for cpuidle, the changes are largely trivial and change
> zero runtime.  All the changes here just remap the modular functions
> onto the non-modular ones that they would be remapped onto anyway.
> 
> Changes are against linux-next and compile tested on ARM allmodconfig.
> I've Cc'd ARM list because all of these are used on ARM, but I'm
> thinking these probably can go in via the PM tree.

If no one objects, I can queue up this series for 4.5 unless you have other
plans with respect to it.

Thanks,
Rafael

--
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/3] cpuidle: avoid module usage in non-modular code

[Daniel Lezcano]

On 12/14/2015 12:57 AM, Paul Gortmaker wrote:

This series of commits is a part of a larger project to ensure
people don't reference modular support functions in non-modular
code.  Overall there was roughly 5k lines of dead code in the
kernel due to this.  So far we've fixed several areas, like tty,
x86, net, ... and we continue to work on other areas.

There are several reasons to not use module support for code that
can never be built as a module, but the big ones are:

  (1) it is easy to accidentally code up unused module_exit and remove code
  (2) it can be misleading when reading the source, thinking it can be
   modular when the Makefile and/or Kconfig prohibit it
  (3) it requires the include of the module.h header file which in turn
  includes nearly everything else.

Fortunately for cpuidle, the changes are largely trivial and change
zero runtime.  All the changes here just remap the modular functions
onto the non-modular ones that they would be remapped onto anyway.

Changes are against linux-next and compile tested on ARM allmodconfig.
I've Cc'd ARM list because all of these are used on ARM, but I'm
thinking these probably can go in via the PM tree.


Acked-by: Daniel Lezcano 


--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
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 v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

[MyungJoo Ham]

>   
>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
> have the common architecture for bus between DRAM and sub-blocks in SoC.
> This driver can support the generic bus frequency driver for Exynos SoCs.
> 
> In devicetree, Each bus block has a bus clock, regulator, operation-point
> and devfreq-event devices which measure the utilization of each bus block.
> 
> Signed-off-by: Chanwoo Choi 
> [linux.amoon: Tested on Odroid U3]
> Tested-by: Anand Moon 
> 

Chanwoo, could you please show me testing this set of patches in your site?
Please let me know when is ok to visit you.
(I do not have exynos machines right now.)

> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 5134f9ee983d..375ebbb4fcfb 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)   += governor_powersave.o
>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)  += governor_userspace.o
>  
>  # DEVFREQ Drivers
> +obj-$(CONFIG_ARCH_EXYNOS)+= exynos/
>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)+= exynos/
>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)+= exynos/

CONFIG_ARCH_EXYNOS is true if
CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
or
CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
Thus, the two lines after you've added have become useless. (dead code)

Please delete them.

[]
> --- /dev/null
> +++ b/drivers/devfreq/exynos/exynos-bus.c
[]
> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
> flags)
> +{
> + struct exynos_bus *bus = dev_get_drvdata(dev);
> + struct dev_pm_opp *new_opp;
> + unsigned long old_freq, new_freq, old_volt, new_volt;
> + int ret = 0;
> +
> + /* Get new opp-bus instance according to new bus clock */
> + rcu_read_lock();
> + new_opp = devfreq_recommended_opp(dev, freq, flags);
> + if (IS_ERR_OR_NULL(new_opp)) {
> + dev_err(dev, "failed to get recommed opp instance\n");
> + rcu_read_unlock();
> + return PTR_ERR(new_opp);
> + }
> +
> + new_freq = dev_pm_opp_get_freq(new_opp);
> + new_volt = dev_pm_opp_get_voltage(new_opp);
> + old_freq = dev_pm_opp_get_freq(bus->curr_opp);
> + old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
> + rcu_read_unlock();
> +
> + if (old_freq == new_freq)
> + return 0;
> +
> + /* Change voltage and frequency according to new OPP level */
> + mutex_lock(>lock);
> +
> + if (old_freq < new_freq) {
> + ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);

Setting the maximum volt same as the minimum volt is not recommended.
Especially for any DVFS mechanisms, I recommend to set values as:
min_volt = minimum voltage that does not harm the stability
max_volt = maximum voltage that does not break the circuit

Please refer to /include/linux/regulator/driver.h
"@set_voltage" comments.

For the rest of regulator_set_voltage usages, I'd say the same.

[]
> +static int exynos_bus_get_dev_status(struct device *dev,
> +  struct devfreq_dev_status *stat)
> +{
> + struct exynos_bus *bus = dev_get_drvdata(dev);
> + struct devfreq_event_data edata;
> + int ret;
> +
> + rcu_read_lock();
> + stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
> + rcu_read_unlock();
> +
> + ret = exynos_bus_get_event(bus, );
> + if (ret < 0) {
> + stat->total_time = stat->busy_time = 0;
> + goto err;
> + }
> +
> + stat->busy_time = (edata.load_count * 100) / bus->ratio;
> + stat->total_time = edata.total_count;
> +
> + dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
> + stat->total_time);

These two values are unsigned long.

[]
> +static int exynos_bus_parse_of(struct device_node *np,
> +   struct exynos_bus *bus)
> +{
> + struct device *dev = bus->dev;
> + unsigned long rate;
> + int i, ret, count, size;
> +
> + /* Get the clock to provide each bus with source clock */
> + bus->clk = devm_clk_get(dev, "bus");
> + if (IS_ERR(bus->clk)) {
> + dev_err(dev, "failed to get bus clock\n");
> + return PTR_ERR(bus->clk);
> + }
> +
> + ret = clk_prepare_enable(bus->clk);
> + if (ret < 0) {
> + dev_err(dev, "failed to get enable clock\n");
> + return ret;
> + }

[]

> +err_regulator:
> + regulator_disable(bus->regulator);
> +err_opp:
> + dev_pm_opp_of_remove_table(dev);
> +
> + return ret;

No clk_disable_unprepare() somewhere in the error handling routines?

[]

> +#ifdef CONFIG_PM_SLEEP
> +static int exynos_bus_resume(struct device *dev)
> +{
[]
> + ret = regulator_enable(bus->regulator);
[]
> +}
> +

Re: [PATCH v4 02/20] PM / devfreq: exynos: Add documentation for generic exynos bus frequency driver

[MyungJoo Ham]

>   
>  This patch adds the documentation for generic exynos bus frequency
> driver.
> 
> Signed-off-by: Chanwoo Choi 
> Reviewed-by: Krzysztof Kozlowski 

A little changes following:

> ---
>  .../devicetree/bindings/devfreq/exynos-bus.txt | 93 
> ++
>  1 file changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> 
> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt 
> b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> new file mode 100644
> index ..e32daef328da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> @@ -0,0 +1,93 @@
> +* Generic Exynos Bus frequency device
> +
> +The Samsung Exynos SoC have many buses for data transfer between DRAM

+The Samsung Exynos SoC has many buses for data transfer between DRAM

or

+The Samsung Exynos SoCs have many buses for data transfer between DRAM
(because you intend to support mulitple Exynos SoCs)

> +and sub-blocks in SoC. Almost Exynos SoC have the common architecture

+and sub-blocks in SoC. Most Exynos SoCs share the common architecture

> +for buses. Generally, the each bus of Exynos SoC includes the source clock

+for buses. Generally, each bus of Exynos SoC includes a source clock

> +and power line and then is able to change the clock according to the usage

+and a power line, which are able to change the clock frequency 

> +of each buses on runtime. When gathering the usage of each buses on runtime,

+of the bus in runtime. To monitor the usage of each bus in runtime,

> +the driver uses the PPMU (Platform Performance Monitoring Unit) which

+the driver uses the PPMU (Platform Performance Monitoring Unit), which

> +is able to measure the current load of sub-blocks.
> +
> +There are a little different composition among Exynos SoC because each Exynos
> +SoC has the different sub-blocks. So, this difference should be specified

+SoC has different sub-blocks. Therefore, such difference should be specified

> +in devicetree file instead of each device driver. In result, this driver
> +is able to support the bus frequency for all Exynos SoCs.
> +
[]
N�r��yb�X��ǧv�^�)޺{.n�+{�x,�ȧ���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH v4 03/20] ARM: dts: Add DMC bus node for Exynos3250

[MyungJoo Ham]

>   
>  This patch adds the DMC (Dynamic Memory Controller) bus node for Exynos3250 
> SoC.
> The DMC is an AMBA AXI-compliant slave to interface external JEDEC standard
> SDRAM devices. The bus includes the OPP tables and the source clock for DMC
> block.
> 
> Following list specifies the detailed relation between the clock and DMC 
> block:
> - The source clock of DMC block : div_dmc
> 
> Signed-off-by: Chanwoo Choi 
> Reviewed-by: Krzysztof Kozlowski 

Acked-by: MyungJoo Ham 

N�r��yb�X��ǧv�^�)޺{.n�+{�x,�ȧ���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH v4 04/20] ARM: dts: Add DMC bus frequency for exynos3250-rinato/monk

[MyungJoo Ham]

>   
>  This patch adds the DMC (Dynamic Memory Controller) bus frequency node
> which includes the devfreq-events and regulator properties. The bus
> frequency support the DVFS (Dynamic Voltage Frequency Scaling) feature
> with ondemand governor.
> 
> The devfreq-events (ppmu_dmc0*) can monitor the utilization of DMC bus
> on runtime and the buck1_reg (VDD_MIF power line) supplies the power to
> the DMC block.
> 
> Signed-off-by: Chanwoo Choi 
> Reviewed-by: Krzysztof Kozlowski 

Acked-by: MyungJoo Ham 

Re: [PATCH v4 01/20] PM / devfreq: exynos: Add generic exynos bus frequency driver

[Chanwoo Choi]

On 2015년 12월 14일 17:28, MyungJoo Ham wrote:
>>   
>>  This patch adds the generic exynos bus frequency driver for AMBA AXI bus
>> of sub-blocks in exynos SoC with DEVFREQ framework. The Samsung Exynos SoC
>> have the common architecture for bus between DRAM and sub-blocks in SoC.
>> This driver can support the generic bus frequency driver for Exynos SoCs.
>>
>> In devicetree, Each bus block has a bus clock, regulator, operation-point
>> and devfreq-event devices which measure the utilization of each bus block.
>>
>> Signed-off-by: Chanwoo Choi 
>> [linux.amoon: Tested on Odroid U3]
>> Tested-by: Anand Moon 
>>
> 
> Chanwoo, could you please show me testing this set of patches in your site?
> Please let me know when is ok to visit you.
> (I do not have exynos machines right now.)

Sure. I can show it tomorrow whenever you want.
Before visiting to me, just let me know. I'll prepare the demonstartion.

Regards,
Chanwo oChoi

> 
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 5134f9ee983d..375ebbb4fcfb 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)  += governor_powersave.o
>>  obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
>>  
>>  # DEVFREQ Drivers
>> +obj-$(CONFIG_ARCH_EXYNOS)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ)   += exynos/
>>  obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ)   += exynos/
> 
> CONFIG_ARCH_EXYNOS is true if
>   CONFIG_ARM_EXYNOS4_BUS_DEVFREQ is true 
>   or
>   CONFIG_ARM_EXYNOS5_BUS_DEVFREQ is true
> Thus, the two lines after you've added have become useless. (dead code)
> 
> Please delete them.
> 
> []
>> --- /dev/null
>> +++ b/drivers/devfreq/exynos/exynos-bus.c
> []
>> +static int exynos_bus_target(struct device *dev, unsigned long *freq, u32 
>> flags)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct dev_pm_opp *new_opp;
>> +unsigned long old_freq, new_freq, old_volt, new_volt;
>> +int ret = 0;
>> +
>> +/* Get new opp-bus instance according to new bus clock */
>> +rcu_read_lock();
>> +new_opp = devfreq_recommended_opp(dev, freq, flags);
>> +if (IS_ERR_OR_NULL(new_opp)) {
>> +dev_err(dev, "failed to get recommed opp instance\n");
>> +rcu_read_unlock();
>> +return PTR_ERR(new_opp);
>> +}
>> +
>> +new_freq = dev_pm_opp_get_freq(new_opp);
>> +new_volt = dev_pm_opp_get_voltage(new_opp);
>> +old_freq = dev_pm_opp_get_freq(bus->curr_opp);
>> +old_volt = dev_pm_opp_get_voltage(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +if (old_freq == new_freq)
>> +return 0;
>> +
>> +/* Change voltage and frequency according to new OPP level */
>> +mutex_lock(>lock);
>> +
>> +if (old_freq < new_freq) {
>> +ret = regulator_set_voltage(bus->regulator, new_volt, new_volt);
> 
> Setting the maximum volt same as the minimum volt is not recommended.
> Especially for any DVFS mechanisms, I recommend to set values as:
> min_volt = minimum voltage that does not harm the stability
> max_volt = maximum voltage that does not break the circuit
> 
> Please refer to /include/linux/regulator/driver.h
> "@set_voltage" comments.
> 
> For the rest of regulator_set_voltage usages, I'd say the same.
> 
> []
>> +static int exynos_bus_get_dev_status(struct device *dev,
>> + struct devfreq_dev_status *stat)
>> +{
>> +struct exynos_bus *bus = dev_get_drvdata(dev);
>> +struct devfreq_event_data edata;
>> +int ret;
>> +
>> +rcu_read_lock();
>> +stat->current_frequency = dev_pm_opp_get_freq(bus->curr_opp);
>> +rcu_read_unlock();
>> +
>> +ret = exynos_bus_get_event(bus, );
>> +if (ret < 0) {
>> +stat->total_time = stat->busy_time = 0;
>> +goto err;
>> +}
>> +
>> +stat->busy_time = (edata.load_count * 100) / bus->ratio;
>> +stat->total_time = edata.total_count;
>> +
>> +dev_dbg(dev, "Usage of devfreq-event : %ld/%ld\n", stat->busy_time,
>> +stat->total_time);
> 
> These two values are unsigned long.
> 
> []
>> +static int exynos_bus_parse_of(struct device_node *np,
>> +  struct exynos_bus *bus)
>> +{
>> +struct device *dev = bus->dev;
>> +unsigned long rate;
>> +int i, ret, count, size;
>> +
>> +/* Get the clock to provide each bus with source clock */
>> +bus->clk = devm_clk_get(dev, "bus");
>> +if (IS_ERR(bus->clk)) {
>> +dev_err(dev, "failed to get bus clock\n");
>> +return PTR_ERR(bus->clk);
>> +}
>> +
>> +ret = clk_prepare_enable(bus->clk);
>> +if (ret < 0) {
>> +dev_err(dev, "failed to get enable clock\n");
>> +return ret;
>> +}
> 
> []
> 
>> +err_regulator:
>> +regulator_disable(bus->regulator);
>> +err_opp:

Re: [PATCH v4 02/20] PM / devfreq: exynos: Add documentation for generic exynos bus frequency driver

[Chanwoo Choi]

On 2015년 12월 14일 17:40, MyungJoo Ham wrote:
>>   
>>  This patch adds the documentation for generic exynos bus frequency
>> driver.
>>
>> Signed-off-by: Chanwoo Choi 
>> Reviewed-by: Krzysztof Kozlowski 
> 
> A little changes following:
> 
>> ---
>>  .../devicetree/bindings/devfreq/exynos-bus.txt | 93 
>> ++
>>  1 file changed, 93 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>>
>> diff --git a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt 
>> b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>> new file mode 100644
>> index ..e32daef328da
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>> @@ -0,0 +1,93 @@
>> +* Generic Exynos Bus frequency device
>> +
>> +The Samsung Exynos SoC have many buses for data transfer between DRAM
> 
> +The Samsung Exynos SoC has many buses for data transfer between DRAM
> 
> or
> 
> +The Samsung Exynos SoCs have many buses for data transfer between DRAM
> (because you intend to support mulitple Exynos SoCs)
> 
>> +and sub-blocks in SoC. Almost Exynos SoC have the common architecture
> 
> +and sub-blocks in SoC. Most Exynos SoCs share the common architecture
> 
>> +for buses. Generally, the each bus of Exynos SoC includes the source clock
> 
> +for buses. Generally, each bus of Exynos SoC includes a source clock
> 
>> +and power line and then is able to change the clock according to the usage
> 
> +and a power line, which are able to change the clock frequency 
> 
>> +of each buses on runtime. When gathering the usage of each buses on runtime,
> 
> +of the bus in runtime. To monitor the usage of each bus in runtime,
> 
>> +the driver uses the PPMU (Platform Performance Monitoring Unit) which
> 
> +the driver uses the PPMU (Platform Performance Monitoring Unit), which
> 
>> +is able to measure the current load of sub-blocks.
>> +
>> +There are a little different composition among Exynos SoC because each 
>> Exynos
>> +SoC has the different sub-blocks. So, this difference should be specified
> 
> +SoC has different sub-blocks. Therefore, such difference should be specified
> 
>> +in devicetree file instead of each device driver. In result, this driver
>> +is able to support the bus frequency for all Exynos SoCs.
>> +

Okay. I'll modify it.

Regards,
Chanwoo Choi

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