Re: [PATCH] ARM: EXYNOS: Add MFC device tree support

2012-08-20 Thread Karol Lewandowski
On 08/16/2012 08:42 PM, Thomas Abraham wrote:
 On 16 August 2012 18:01, Arun Kumar K arun.kk ... @public.gmane.org wrote:

 +  - interrupts : MFC interupt number to the CPU.
 +
 +  - samsung,mfc-r : Base address of the first memory bank used by MFC
 +   for DMA contiguous memory allocation.
 +
 +  - samsung,mfc-r-size : Size of the first memory bank.
 
 It is not allowed to pass buffer base address and size from device
 tree. Device tree node should describe only the MFC controller
 hardware. Any memory management related information should be handled
 outside of device tree. This helps the bindings to be reusable across
 multiple operating systems.

The question is where elsewhere this should be described as this is strictly
board-dependent option (number and size of RAM banks are important here).

I agree that base addresses are bad, but I'm not aware of any functionality
that would allow driver (actually, its platform dependent part in
exynosN_reserve() function) to enumerate available memory banks and grab
memory chunks from two distinct banks.

My (lack of) knowledge ARM might be to blame here but I simply don't know
how to achieve this. Any suggestions?


On 08/16/2012 09:31 PM, Arun Kumar K wrote:

 +static void s5p_mfc_reserve_mem(phys_addr_t rbase, unsigned int rsize,
 + phys_addr_t lbase, unsigned int lsize) {
 +
 + if (memblock_remove(lbase, lsize)) {
 + pr_err(Failed to reserve bank1 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +
 + if (memblock_remove(rbase, rsize)) {
 + pr_err(Failed to reserve bank2 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +}


non-static function with the same name is already defined in
arch/arm/plat-samsung/s5p-dev-mfc.c. Please don't duplicate it,
especially that you seem to be trying to do that twice!


 diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c 
 b/arch/arm/mach-exynos/mach-exynos5-dt.c

 index ef770bc..898d2de 100644
 --- a/arch/arm/mach-exynos/mach-exynos5-dt.c
 +++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
...
 +static void s5p_mfc_reserve_mem(phys_addr_t rbase, unsigned int rsize,
 + phys_addr_t lbase, unsigned int lsize) {
 +
 + if (memblock_remove(lbase, lsize)) {
 + pr_err(Failed to reserve bank1 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +
 + if (memblock_remove(rbase, rsize)) {
 + pr_err(Failed to reserve bank2 memory for MFC device\n);
 + WARN_ON(1);
 + }
 +}


See above.

 +
 +static void __init exynos5_reserve(void)
 +{
 + s5p_mfc_reserve_mem(0x4300, 8  20, 0x5100, 8  20);


I think it would make sense to make this memory reservation dependent
on mfc* node being present in DTS.  It's to early to use of_* functions
(because tree is not populated at this stage) but fdt_* family of functions
work just fine.

Regards,
-- 
Karol Lewandowski | Samsung Poland RD Center | Linux/Platform
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-08-20 Thread Valentin, Eduardo
Hello Rui,

On Mon, Aug 20, 2012 at 5:16 AM, Zhang Rui rui.zh...@intel.com wrote:
 On δΊ”, 2012-08-17 at 11:56 +0300, Valentin, Eduardo wrote:
 Hello,

  +
  +
  +1.2 CPU cooling action notifier register/unregister interface
  +1.2.1 int cputherm_register_notifier(struct notifier_block *nb,
  + unsigned int list)
  +
  +This interface registers a driver with cpu cooling layer. The 
  driver will
  +be notified when any cpu cooling action is called.
  +
  +nb: notifier function to register
  +list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
  +
  +1.2.2 int cputherm_unregister_notifier(struct notifier_block *nb,
  + unsigned int list)
  +
  +This interface registers a driver with cpu cooling layer. The 
  driver will
  +be notified when any cpu cooling action is called.
  +
  +nb: notifier function to register
  +list: CPUFREQ_COOLING_START or CPUFREQ_COOLING_STOP
 
  what are these two APIs used for?
  I did not see they are used in your patch set, do I miss something?
  No currently they are not used by my patches. I added them on request
  from Eduardo and others

 Yeah, this was a suggestion as we didn't really know how the FW part
 would evolve by that time.

 The reasoning is to allow any interested user (in kernel) to be
 notified when max frequency changes.

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

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

Agreed. The context is actually much wider than the CPU cooling. In
fact, cooling co-processors is actually where things gets a bit
complicated. The idea behind this type of handshaking is to allow the
affected subsystem to change their actual setup when max performance
is not allowed anymore.


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

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


Yeah. Ideally, the framework needs to be aware of cooling device state
change. Today, as we have pretty much a broadcast/unidirectional type
of messaging, the framework/policy always assumes the cooling devices
will be in sync with what it is dictated by the policy.

  Ideally it would be possible to nack the cooling
 transition. But that is yet a wider discussion. So far we don't have
 users for this.

 yep.
 I thought about this before, but I'd prefer to do this when there is a
 real user. Or else, we are kind of over-designing here.
 how about removing this piece of code for now?


Agreed. I second you that this problem is a much wider issue and needs
improvement in the FW itself and how the cooling devices are designed.


 thanks,
 rui

 
  diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
  index 7dd8c34..996003b 100644
  --- a/drivers/thermal/Kconfig
  +++ b/drivers/thermal/Kconfig
  @@ -19,6 +19,17 @@ config THERMAL_HWMON
depends on HWMON=y || HWMON=THERMAL
default y
 
  +config CPU_THERMAL
  + bool generic cpu cooling support
  + depends on THERMAL  CPU_FREQ
  + help
  +   This implements the generic cpu cooling mechanism through 
  frequency
  +   reduction, cpu hotplug and any other ways of reducing 
  temperature. An
  +   ACPI version of this already 
  exists(drivers/acpi/processor_thermal.c).
  +   This will be useful for platforms using the generic thermal 
  interface
  +   and not the ACPI interface.
  +   If you want this support, you should say Y here.
  +
   config SPEAR_THERMAL
bool SPEAr thermal sensor driver
depends on THERMAL
  diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
  index fd9369a..aae59ad 100644
  --- a/drivers/thermal/Makefile
  +++ b/drivers/thermal/Makefile
  @@ -3,5 +3,6 @@
   #
 
   obj-$(CONFIG_THERMAL)+= thermal_sys.o
  +obj-$(CONFIG_CPU_THERMAL)+= cpu_cooling.o
   obj-$(CONFIG_SPEAR_THERMAL)  += spear_thermal.o
   obj-$(CONFIG_RCAR_THERMAL)   += rcar_thermal.o
  diff --git a/drivers/thermal/cpu_cooling.c 
  b/drivers/thermal/cpu_cooling.c
  new file mode 100644
  index 000..c42e557
  --- /dev/null
  +++ b/drivers/thermal/cpu_cooling.c
  @@ -0,0 +1,512 @@
  +/*
  + *  linux/drivers/thermal/cpu_cooling.c
  + *
  + *  Copyright (C) 2012   Samsung Electronics Co., 
  Ltd(http://www.samsung.com)
  + *  Copyright (C) 2012  Amit Daniel amit.kach...@linaro.org
  + *
  + * 
  

Re: [PATCH] mmc: core: skip mmc_power_up call from start host

2012-08-20 Thread Ulf Hansson
Hi Girish,

On 13 July 2012 14:57, Girish K S girish.shivananja...@linaro.org wrote:
 The call to the mmc_power_up during the mmc_start_host breaks the card
 detection in design-ware host controller. This patch removes the call to
 mmc_power_up function during host start.

 This fix works fine with sdhci (sdhci compatilble host controller)
 and dw_mmc (design-ware host controller). and has no side effect due to
 this removal.

 Tested on : origen-board and smdk-5250 board.

 Signed-off-by: Girish K S girish.shivananja...@linaro.org
 Cc: Ulf Hansson ulf.hans...@linaro.org
 ---
  drivers/mmc/core/core.c |1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

 diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
 index 9503cab..503aefc 100644
 --- a/drivers/mmc/core/core.c
 +++ b/drivers/mmc/core/core.c
 @@ -2108,7 +2108,6 @@ void mmc_start_host(struct mmc_host *host)
  {
 host-f_init = max(freqs[0], host-f_min);
 host-rescan_disable = 0;
 -   mmc_power_up(host);

This will introduce a bug (race condition) for host drivers using the
regulator API (from regulator_init_complete) and for eMMC. So please
do not remove this.

 mmc_detect_change(host, 0);
  }

 --
 1.7.4.1



I suggest you find out more details about why this breaks the card
detection mechanism for your stated boards.

Kind regards
Ulf Hansson
--
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