date:20140409

Re: [PATCH] arm: exynos: generalize power register address calculation

2014-04-09 Thread Chander Kashyap

Hi Tomasz,

On 9 April 2014 20:15, Tomasz Figa  wrote:
> On 09.04.2014 15:49, Chander Kashyap wrote:
>>
>> Hi Tomasz,
>>
>> On 9 April 2014 17:19, Tomasz Figa  wrote:
>>>
>>> Hi Chander,
>>>
>>>
>>> On 09.04.2014 13:09, Chander Kashyap wrote:


 Currently status/configuration power register values are hard-coded for
 cpu1.

 Make it generic so that it is useful for SoC's with more than two cpus.

 Signed-off-by: Chander Kashyap 
 ---
 changes in v2 : Used existing macros for clusterid and cpuid calculation

arch/arm/mach-exynos/hotplug.c  |   15 ---
arch/arm/mach-exynos/platsmp.c  |   20 +++-
arch/arm/mach-exynos/regs-pmu.h |9 +++--
3 files changed, 34 insertions(+), 10 deletions(-)

 diff --git a/arch/arm/mach-exynos/hotplug.c
 b/arch/arm/mach-exynos/hotplug.c
 index 5eead53..eab6121 100644
 --- a/arch/arm/mach-exynos/hotplug.c
 +++ b/arch/arm/mach-exynos/hotplug.c
 @@ -17,6 +17,7 @@

#include 
#include 
 +#include 
#include 

#include 
 @@ -92,11 +93,19 @@ static inline void cpu_leave_lowpower(void)

static inline void platform_do_lowpower(unsigned int cpu, int
 *spurious)
{
 +   unsigned int mpidr, cpunr, cluster;
 +
 +   mpidr = cpu_logical_map(cpu);
 +   cpunr = MPIDR_AFFINITY_LEVEL(mpidr, 0);
 +   cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
 +
 +   /* Maximum possible cpus in a cluster can be 4 */
 +   cpunr += cluster * 4;
>>>
>>>
>>>
>>> I believe this is rather a weak assumption. First of all, the limit seems
>>> to
>>> be hardcoded only for the few existing SoCs. In addition, the value is
>>> not
>>> used as a maximum, but rather it is assumed that each cluster has always
>>> four cores.
>>
>>
>> The MPIDR register contains 2 bits for cpu id. Hence maximum number of
>> cpus can be 4 only (A15/A9/A7).
>>
>
> This is not what I meant. Exynos5260 contains 2 big cores (not 4) and 4
> little cores. Are you sure that PMU register layout on Exynos5260 matches
> your equation?
>

Yes the equation covers that as the PMU register layout takes care for that:
Address offset are as follows:
2 Big Cores:
cpu0 : 2000
cpu1: 2080

4 Little cores:

cpu0: 2200
cpu1: 2280
cpu2: 2300
cpu3: 2380

>
>>>
>>> Moreover, it is assumed here that the mapping between core ID (calculated
>>> by
>>> the equation below) and PMU core numbers is 1:1, which is not true. On
>>> Exynos4210, the cluster ID is always 0x09 and on Exynos4x12 it is 0x0a,
>>> which will lead to completely wrong register offsets.
>>
>>
>> Exynos4210 and Exynos4x12, cluster ids are not passed from DT as it
>> breaks the gic_init_bases. Hence the Physical CpuID for Exynos4210
>> will be 0,1 and Exynos4x12 will be 0,1,2,3.
>>
>> So it will not break.
>
>
> I already have patches ready fixing GIC driver, just waiting for 3.15-rc1 to
> be released. Anyway, CPU topology in DT is mandatory and Exynos4 device tree
> files need to be fixed to contain them. This needs to be accounted for in
> any changes touching CPU topology related code.
>

That's great.

>
>>
>>
>>>
>>> I believe the proper way to deal with this is to provide per-CPU property
>>> in
>>> DT called "samsung,pmu-offset" that could be used be code like this to
>>> calculate register addresses properly.
>>>
>>> For now, I would recommend doing the above ignoring cluster ID completely
>>> to
>>> not break (and actually fix) single cluster systems and existing multi
>>> cluster ones on which only the first cluster is supported now.
>>>
>>> After that, per-CPU PMU offset should be implemented to support
>>> multi-cluster SoCs with proper support of multiple clusters.
>>
>>
>> As of now the smp-boot (cores > 2) is broken. This is required to fix it.
>
>
> SMP boot works fine on all four cores of Exynos 4412. Obiously hot-(un)plug
> doesn't, but this is another issue.
>

It works as of now as at power on all the cores powered on. Hence the
powerOn in platsmp.c doent make any difference,  It breaks in hotplug
as we always poweron cpu1, not the correct cpu.

> Best regards,
> Tomasz



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

Re: [PATCH 1/2] usb: ehci-exynos: Return immediately from suspend if ehci_suspend fails

2014-04-09 Thread Vivek Gautam

Hi Alan,


On Thu, Apr 10, 2014 at 7:06 AM, Alan Stern  wrote:
> On Thu, 10 Apr 2014, Jingoo Han wrote:
>
>> > > --- a/drivers/usb/host/ehci-exynos.c
>> > > +++ b/drivers/usb/host/ehci-exynos.c
>> > > @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
>> > >   int rc;
>> > >
>> > >   rc = ehci_suspend(hcd, do_wakeup);
>> > > + if (rc)
>> > > + return rc;
>> > >
>> > >   if (exynos_ehci->otg)
>> > >   exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
>> > > @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
>> > >
>> > >   clk_disable_unprepare(exynos_ehci->clk);
>> > >
>> > > - return rc;
>> > > + return 0;
>> > >  }
>> > >
>> > >  static int exynos_ehci_resume(struct device *dev)
>> >
>> > The first hunk of this patch is correct, but the second hunk isn't
>> > needed.  A similar remark is true for the ehci-platform patch.
>>
>> Hi Alan,
>>
>> Do you mean the following?
>>
>> 1st hunk
>>  +if (rc)
>>  +return rc;
>>
>> 2nd hunk
>>  -return rc;
>>  +return 0;
>
> Yes, that's what I mean.
>
>> Currently, the 'rc' will be always 'zero'; however, I don't
>> Have any objection, because the code might be  modified later.
>
> Exactly.  We should add the new "if" statement but leave the "return
> rc" the way it is.

Sure, i will update both the patches.

>
> Alan Stern
>
> --
> 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: [PATCH 05/10] ARM: EXYNOS: Move "regs-pmu" header inclusion in common.h

2014-04-09 Thread Pankaj Dubey


Hi Tomasz,

On 04/09/2014 01:09 AM, Tomasz Figa wrote:

Hi Pankaj,

On 02.04.2014 09:50, Pankaj Dubey wrote:

There are many machine files under "mach-exynos" including "regs-pmu.h"
as well as "common.h", so better we move this header inclusion in common.h.

Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/common.h  |1 +
  arch/arm/mach-exynos/cpuidle.c |1 -
  arch/arm/mach-exynos/exynos.c  |1 -
  arch/arm/mach-exynos/hotplug.c |1 -
  arch/arm/mach-exynos/platsmp.c |1 -
  arch/arm/mach-exynos/pm.c  |1 -
  arch/arm/mach-exynos/pmu.c |1 -
  7 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 277a83e..ff28334 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -14,6 +14,7 @@

  #include 
  #include 
+#include "regs-pmu.h"

  void exynos_init_io(void);
  void exynos_restart(enum reboot_mode mode, const char *cmd);
diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index b530231..b9dd8c3 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -29,7 +29,6 @@
  #include 

  #include "common.h"
-#include "regs-pmu.h"

  #define REG_DIRECTGO_ADDR(samsung_rev() == EXYNOS4210_REV_1_1 ? \
  S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 57bd1cd..a5e1349 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -30,7 +30,6 @@

  #include "common.h"
  #include "mfc.h"
-#include "regs-pmu.h"
  #include "regs-sys.h"

  #define L2_AUX_VAL 0x7C470001
diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index 5eead53..33db6ee 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -22,7 +22,6 @@
  #include 

  #include "common.h"
-#include "regs-pmu.h"

  static inline void cpu_enter_lowpower_a9(void)
  {
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index d8d1555..3ebb03f 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -29,7 +29,6 @@
  #include 

  #include "common.h"
-#include "regs-pmu.h"

  extern void exynos4_secondary_startup(void);

diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 723c988..875151f 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -34,7 +34,6 @@
  #include 

  #include "common.h"
-#include "regs-pmu.h"
  #include "regs-sys.h"

  /**
diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index 05c7ce1..fb44352 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -16,7 +16,6 @@
  #include 

  #include "common.h"
-#include "regs-pmu.h"

  static const struct exynos_pmu_conf *exynos_pmu_config;




I don't think this is a good idea. It adds hidden indirect dependencies 
between source files and thus reduces maintainability and readability. In 
addition it causes the file to be included even by files that don't need it.



Thanks for review.
I did this change because:
1: Currently only these 6 files (under "arm/mach-exynos/") are including 
"regs-pmu.h"
as well as "common.h" so all these source files requires "regs-pmu.h" can get it 
included via "common.h".
2: Next if we change location/rename "regs-pmu.h", we need to update all these 
source files. On the other hand

if it's present in common.h it can be done at single place in "common.h".
3:  More over just checked that even currently common.h has some hidden includes 
such as ("linux/of.h") which is not

required by all source files (I can see only pm.c needs it).
So let me know if still you think it's not good idea, I will consider to drop 
this patch in next version of patch series.



Best regards,
Tomasz




--
Best Regards,
Pankaj Dubey

--
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 01/10] ARM: EXYNOS: Cleanup "mach-exynos/common.h" file

2014-04-09 Thread Pankaj Dubey


Hi Tomasz,

On 04/09/2014 12:59 AM, Tomasz Figa wrote:

Hi Pankaj,

On 02.04.2014 09:50, Pankaj Dubey wrote:

Remove unused declarations from "mach-exynos/common.h"

Signed-off-by: Pankaj Dubey 
---
  arch/arm/mach-exynos/common.h |3 ---
  1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9ef3f83..277a83e 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -15,9 +15,6 @@
  #include 
  #include 

-void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
-
-struct map_desc;
  void exynos_init_io(void);
  void exynos_restart(enum reboot_mode mode, const char *cmd);
  void exynos_cpuidle_init(void);



You could also drop a bit more. From the context above I can see that at least 
exynos_restart() and exynos_init_io() are never used outside of files they are 
defined in, so they too could be dropped from this header. Looking at rest of 
the header, there seems to be a lot of similar definitions. If cleaning up the 
header anyway, could you take care of them as well?




Thanks for review.
Sure, I will take care of other functions which are not required in this header
file in next version.


Best regards,
Tomasz




--
Best Regards,
Pankaj Dubey

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

Re: [PATCH 1/2] usb: ehci-exynos: Return immediately from suspend if ehci_suspend fails

2014-04-09 Thread Alan Stern

On Thu, 10 Apr 2014, Jingoo Han wrote:

> > > --- a/drivers/usb/host/ehci-exynos.c
> > > +++ b/drivers/usb/host/ehci-exynos.c
> > > @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
> > >   int rc;
> > >
> > >   rc = ehci_suspend(hcd, do_wakeup);
> > > + if (rc)
> > > + return rc;
> > >
> > >   if (exynos_ehci->otg)
> > >   exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> > > @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
> > >
> > >   clk_disable_unprepare(exynos_ehci->clk);
> > >
> > > - return rc;
> > > + return 0;
> > >  }
> > >
> > >  static int exynos_ehci_resume(struct device *dev)
> > 
> > The first hunk of this patch is correct, but the second hunk isn't
> > needed.  A similar remark is true for the ehci-platform patch.
> 
> Hi Alan,
> 
> Do you mean the following?
> 
> 1st hunk
>  +if (rc)
>  +return rc;
> 
> 2nd hunk
>  -return rc;
>  +return 0;

Yes, that's what I mean.

> Currently, the 'rc' will be always 'zero'; however, I don't
> Have any objection, because the code might be  modified later.

Exactly.  We should add the new "if" statement but leave the "return 
rc" the way it is.

Alan Stern

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

Re: [PATCH 1/2] usb: ehci-exynos: Return immediately from suspend if ehci_suspend fails

2014-04-09 Thread Jingoo Han

On Thursday, April 10, 2014 3:32 AM, Alan Stern wrote:
> On Wed, 9 Apr 2014, Vivek Gautam wrote:
> 
> > Patch 'b8efdaf USB: EHCI: add check for wakeup/suspend race'
> > adds a check for possible race between suspend and wakeup interrupt,
> > and thereby it returns -EBUSY as error code if there's a wakeup
> > interrupt.
> > So the platform host controller should not proceed further with
> > its suspend callback, rather should return immediately to avoid
> > powering down the essential things, like phy.
> >
> > Signed-off-by: Vivek Gautam 
> > Cc: Alan Stern 
> > Cc: Jingoo Han 
> > ---
> >
> > Based on 'usb-next' branch of Greg's usb tree.
> >
> >  drivers/usb/host/ehci-exynos.c |4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> > index d1d8c47..a4550eb 100644
> > --- a/drivers/usb/host/ehci-exynos.c
> > +++ b/drivers/usb/host/ehci-exynos.c
> > @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
> > int rc;
> >
> > rc = ehci_suspend(hcd, do_wakeup);
> > +   if (rc)
> > +   return rc;
> >
> > if (exynos_ehci->otg)
> > exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> > @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
> >
> > clk_disable_unprepare(exynos_ehci->clk);
> >
> > -   return rc;
> > +   return 0;
> >  }
> >
> >  static int exynos_ehci_resume(struct device *dev)
> 
> The first hunk of this patch is correct, but the second hunk isn't
> needed.  A similar remark is true for the ehci-platform patch.

Hi Alan,

Do you mean the following?

1st hunk
 +  if (rc)
 +  return rc;

2nd hunk
 -  return rc;
 +  return 0;

Currently, the 'rc' will be always 'zero'; however, I don't
Have any objection, because the code might be  modified later.

Best regards,
Jingoo Han
> 
> Alan Stern

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

Re: [PATCH 1/2] usb: ehci-exynos: Return immediately from suspend if ehci_suspend fails

2014-04-09 Thread Alan Stern

On Wed, 9 Apr 2014, Vivek Gautam wrote:

> Patch 'b8efdaf USB: EHCI: add check for wakeup/suspend race'
> adds a check for possible race between suspend and wakeup interrupt,
> and thereby it returns -EBUSY as error code if there's a wakeup
> interrupt.
> So the platform host controller should not proceed further with
> its suspend callback, rather should return immediately to avoid
> powering down the essential things, like phy.
> 
> Signed-off-by: Vivek Gautam 
> Cc: Alan Stern 
> Cc: Jingoo Han 
> ---
> 
> Based on 'usb-next' branch of Greg's usb tree.
> 
>  drivers/usb/host/ehci-exynos.c |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index d1d8c47..a4550eb 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
>   int rc;
>  
>   rc = ehci_suspend(hcd, do_wakeup);
> + if (rc)
> + return rc;
>  
>   if (exynos_ehci->otg)
>   exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
>  
>   clk_disable_unprepare(exynos_ehci->clk);
>  
> - return rc;
> + return 0;
>  }
>  
>  static int exynos_ehci_resume(struct device *dev)

The first hunk of this patch is correct, but the second hunk isn't 
needed.  A similar remark is true for the ehci-platform patch.

Alan Stern

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

Re: [PATCH] arm: exynos: generalize power register address calculation

2014-04-09 Thread Tomasz Figa


On 09.04.2014 15:49, Chander Kashyap wrote:

Hi Tomasz,

On 9 April 2014 17:19, Tomasz Figa  wrote:

Hi Chander,


On 09.04.2014 13:09, Chander Kashyap wrote:


Currently status/configuration power register values are hard-coded for
cpu1.

Make it generic so that it is useful for SoC's with more than two cpus.

Signed-off-by: Chander Kashyap 
---
changes in v2 : Used existing macros for clusterid and cpuid calculation

   arch/arm/mach-exynos/hotplug.c  |   15 ---
   arch/arm/mach-exynos/platsmp.c  |   20 +++-
   arch/arm/mach-exynos/regs-pmu.h |9 +++--
   3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-exynos/hotplug.c
b/arch/arm/mach-exynos/hotplug.c
index 5eead53..eab6121 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -17,6 +17,7 @@

   #include 
   #include 
+#include 
   #include 

   #include 
@@ -92,11 +93,19 @@ static inline void cpu_leave_lowpower(void)

   static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
   {
+   unsigned int mpidr, cpunr, cluster;
+
+   mpidr = cpu_logical_map(cpu);
+   cpunr = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+   cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+   /* Maximum possible cpus in a cluster can be 4 */
+   cpunr += cluster * 4;



I believe this is rather a weak assumption. First of all, the limit seems to
be hardcoded only for the few existing SoCs. In addition, the value is not
used as a maximum, but rather it is assumed that each cluster has always
four cores.


The MPIDR register contains 2 bits for cpu id. Hence maximum number of
cpus can be 4 only (A15/A9/A7).



This is not what I meant. Exynos5260 contains 2 big cores (not 4) and 4 
little cores. Are you sure that PMU register layout on Exynos5260 
matches your equation?




Moreover, it is assumed here that the mapping between core ID (calculated by
the equation below) and PMU core numbers is 1:1, which is not true. On
Exynos4210, the cluster ID is always 0x09 and on Exynos4x12 it is 0x0a,
which will lead to completely wrong register offsets.


Exynos4210 and Exynos4x12, cluster ids are not passed from DT as it
breaks the gic_init_bases. Hence the Physical CpuID for Exynos4210
will be 0,1 and Exynos4x12 will be 0,1,2,3.

So it will not break.


I already have patches ready fixing GIC driver, just waiting for 
3.15-rc1 to be released. Anyway, CPU topology in DT is mandatory and 
Exynos4 device tree files need to be fixed to contain them. This needs 
to be accounted for in any changes touching CPU topology related code.







I believe the proper way to deal with this is to provide per-CPU property in
DT called "samsung,pmu-offset" that could be used be code like this to
calculate register addresses properly.

For now, I would recommend doing the above ignoring cluster ID completely to
not break (and actually fix) single cluster systems and existing multi
cluster ones on which only the first cluster is supported now.

After that, per-CPU PMU offset should be implemented to support
multi-cluster SoCs with proper support of multiple clusters.


As of now the smp-boot (cores > 2) is broken. This is required to fix it.


SMP boot works fine on all four cores of Exynos 4412. Obiously 
hot-(un)plug doesn't, but this is another issue.


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] arm: exynos: generalize power register address calculation

2014-04-09 Thread Chander Kashyap

Hi Tomasz,

On 9 April 2014 17:19, Tomasz Figa  wrote:
> Hi Chander,
>
>
> On 09.04.2014 13:09, Chander Kashyap wrote:
>>
>> Currently status/configuration power register values are hard-coded for
>> cpu1.
>>
>> Make it generic so that it is useful for SoC's with more than two cpus.
>>
>> Signed-off-by: Chander Kashyap 
>> ---
>> changes in v2 : Used existing macros for clusterid and cpuid calculation
>>
>>   arch/arm/mach-exynos/hotplug.c  |   15 ---
>>   arch/arm/mach-exynos/platsmp.c  |   20 +++-
>>   arch/arm/mach-exynos/regs-pmu.h |9 +++--
>>   3 files changed, 34 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/hotplug.c
>> b/arch/arm/mach-exynos/hotplug.c
>> index 5eead53..eab6121 100644
>> --- a/arch/arm/mach-exynos/hotplug.c
>> +++ b/arch/arm/mach-exynos/hotplug.c
>> @@ -17,6 +17,7 @@
>>
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>
>>   #include 
>> @@ -92,11 +93,19 @@ static inline void cpu_leave_lowpower(void)
>>
>>   static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
>>   {
>> +   unsigned int mpidr, cpunr, cluster;
>> +
>> +   mpidr = cpu_logical_map(cpu);
>> +   cpunr = MPIDR_AFFINITY_LEVEL(mpidr, 0);
>> +   cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
>> +
>> +   /* Maximum possible cpus in a cluster can be 4 */
>> +   cpunr += cluster * 4;
>
>
> I believe this is rather a weak assumption. First of all, the limit seems to
> be hardcoded only for the few existing SoCs. In addition, the value is not
> used as a maximum, but rather it is assumed that each cluster has always
> four cores.

The MPIDR register contains 2 bits for cpu id. Hence maximum number of
cpus can be 4 only (A15/A9/A7).

>
> Moreover, it is assumed here that the mapping between core ID (calculated by
> the equation below) and PMU core numbers is 1:1, which is not true. On
> Exynos4210, the cluster ID is always 0x09 and on Exynos4x12 it is 0x0a,
> which will lead to completely wrong register offsets.

Exynos4210 and Exynos4x12, cluster ids are not passed from DT as it
breaks the gic_init_bases. Hence the Physical CpuID for Exynos4210
will be 0,1 and Exynos4x12 will be 0,1,2,3.

So it will not break.


>
> I believe the proper way to deal with this is to provide per-CPU property in
> DT called "samsung,pmu-offset" that could be used be code like this to
> calculate register addresses properly.
>
> For now, I would recommend doing the above ignoring cluster ID completely to
> not break (and actually fix) single cluster systems and existing multi
> cluster ones on which only the first cluster is supported now.
>
> After that, per-CPU PMU offset should be implemented to support
> multi-cluster SoCs with proper support of multiple clusters.

As of now the smp-boot (cores > 2) is broken. This is required to fix it.

>
> Best regards,
> Tomasz



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

Re: [PATCH V3 11/17] ARM: exynos: cpuidle: Move the power sequence call in the cpu_pm notifier

2014-04-09 Thread Daniel Lezcano


On 04/09/2014 02:17 PM, Tomasz Figa wrote:

Hi Daniel,

On 08.04.2014 14:19, Daniel Lezcano wrote:

The code to initiate and exit the powerdown sequence is the same in
pm.c and
cpuidle.c.

Let's split the common part in the pm.c and reuse it from the cpu_pm
notifier.

That is one more step forward to make the cpuidle driver arch
indenpendant.

Signed-off-by: Daniel Lezcano 
Reviewed-by: Viresh Kumar 
Reviewed-by: Bartlomiej Zolnierkiewicz 
---
  arch/arm/mach-exynos/cpuidle.c |   21 -
  arch/arm/mach-exynos/pm.c  |   22 ++
  2 files changed, 18 insertions(+), 25 deletions(-)


[snip]


-static int exynos_pm_suspend(void)
+static void exynos_pm_central_suspend(void)
  {
  unsigned long tmp;

  /* Setting Central Sequence Register for power down mode */
-
  tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
  tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
  __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
+}
+
+static int exynos_pm_suspend(void)
+{
+unsigned long tmp;



Shouldn't exynos_pm_central_suspend() be called here in place of the
code being moved to that function?


Right. Good catch !

Thanks

  -- Daniel



--
  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 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

2014-04-09 Thread Tomasz Figa


On 09.04.2014 13:49, Vivek Gautam wrote:

Hi,


On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa  wrote:

Hi Vivek,

Please see my comments inline.


On 08.04.2014 16:36, Vivek Gautam wrote:


Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
The new driver uses the generic PHY framework and will interact
with DWC3 controller present on Exynos5 series of SoCs.
Thereby, removing old phy-samsung-usb3 driver and related code
used untill now which was based on usb/phy framework.

Signed-off-by: Vivek Gautam 
---
   .../devicetree/bindings/phy/samsung-phy.txt|   42 ++
   drivers/phy/Kconfig|   11 +
   drivers/phy/Makefile   |1 +
   drivers/phy/phy-exynos5-usbdrd.c   |  668

   4 files changed, 722 insertions(+)
   create mode 100644 drivers/phy/phy-exynos5-usbdrd.c



[snip]



+   Additional clock required for Exynos5420:
+   - usb30_sclk_100m: Additional special clock used for PHY operation
+  depicted as 'sclk_usbphy30' in CMU of
Exynos5420.



Are you sure this isn't simply a gate for the ref clock, as it can be found
on another SoC that is not upstream yet? I don't have documentation for
Exynos 5420 so I can't tell, but I'd like to ask you to recheck this.



From what i can see in the manual :

sclk_usbphy30 is derived from OSCCLK.
It is coming from a MUX (default input line to this is OSCCLK)  and
then through a DIV
there's this gate.

   {OSCCLK  + other sources} --->[MUX] ---> [DIV] --> [GATE for
sclk_usbphy30]

the {rate of sclk_usbphy30} == OSCCLK

However the 'ref' clock that we have been using is the actual oscillator clock.
And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30).
So should this mean that ref clock and sclk_usbphy30 are still be controlled by
two different gates ?



Is there maybe a diagram of PHY input clocks in the datasheet, like for 
USB 2.0 PHY in Exynos4210/4412/5250 datasheets in the chapter about 
USB2.0 Device? Something like:


 
||
| ___|
XusbXTI |   Phy_fsel[2:0]|  ___  |
   ___[X]___|| __|_|___|\__|_|
  | |   _v___ |  _   ^ |   |/  | |
_   |  | || | |  | |  ___  | |
 ___|  | || | |  | | |   |_|_|
|___|   |  | X 0 ||_| PLL |__|_|_|CLK|_|_|
_   |  | |  | || |DIV|_|_|
  |___[X]   |  |_| 12   |_|480 | |___| | |
|  MHz MHz |Digital| |
XusbXTO |   USB PHY|___| |
||


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] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

2014-04-09 Thread Sylwester Nawrocki

On 09/04/14 14:24, Vivek Gautam wrote:
> On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
>  wrote:
>> > On 09/04/14 13:54, Vivek Gautam wrote:
>>> >> Adding support to enable/disable VBUS hooked to a gpio
>>> >> to enable vbus supply on the port.
>> >
>> > Does the GPIO control a fixed voltage regulator ? If so, shouldn't
>> > it be modelled by the regulator API instead ?
>
> No, this GPIO controls a 'current limiting power distribution switch',
> which gives the output vbus to usb controller.
> Should i model this as a fixed regulator ?

OK, it's just a power switch then. I suspect using the regulator API
would be more universal, as such a GPIO is somewhat a board design
detail. I'm not going to object to your patch, just might be better
to use the gpiod API instead.

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

Re: [PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

2014-04-09 Thread Tomasz Figa


Hi,

On 09.04.2014 14:24, Vivek Gautam wrote:

Hi Sylwester,


On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
 wrote:

Hi Vivek,

On 09/04/14 13:54, Vivek Gautam wrote:

Adding support to enable/disable VBUS hooked to a gpio
to enable vbus supply on the port.


Does the GPIO control a fixed voltage regulator ? If so, shouldn't
it be modelled by the regulator API instead ?


No, this GPIO controls a 'current limiting power distribution switch',
which gives the output vbus to usb controller.
Should i model this as a fixed regulator ?


If I understand this correctly, this is just a switch that lets you 
control whether vbus is provided to the USB connector or not. If so, 
this doesn't look like an Exynos-specific thing at all and should rather 
be modeled on higher level.


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 V3 00/17] ARM: exynos: cpuidle: Move the driver to drivers/cpuidle

2014-04-09 Thread Tomasz Figa


Hi Daniel,

On 08.04.2014 14:19, Daniel Lezcano wrote:

Changelog:

V3:
* Added patch   : "ARM: exynos: cpuidle: Disable cpuidle for 5440"
* Removed patch : "ARM: exynos: config: Enable cpuidle"
* Removed default ARM_EXYNOS4210_CPUIDLE=y
* Added comment about bug fix side effect 'for_each_possible_cpu'
V2:
* Added comment in changelog for calls order (5/17)
* Call the powerdown only for cpu0 in the pm notifier
* Set the pm notifier for all boards

V1: initial post

This patchset relies on the cpm_pm notifier to initiate the powerdown sequence
operations from pm.c instead cpuidle.c. Thus the cpuidle driver is no longer
dependent from arch specific code as everything is called from the pm.c file.

The patchset applies on top of linux-samsung/for-next.

Tested on exynos4: 4210
Tested on exynos5: 5250 (without AFTR)

Daniel Lezcano (17):
   ARM: exynos: cpuidle: Prevent forward declaration
   ARM: exynos: cpuidle: use cpuidle_register
   ARM: exynos: cpuidle: change function name prefix
   ARM: exynos: cpuidle: encapsulate register access inside a function
   ARM: exynos: cpuidle: Move some code inside the idle_finisher
   ARM: exynos: cpuidle: Fix S5P_WAKEUP_STAT call
   ARM: exynos: cpuidle: Use the cpu_pm notifier
   ARM: exynos: cpuidle: Move scu_enable in the cpu_pm notifier
   ARM: exynos: cpuidle: Remove ifdef for scu_enable
   ARM: exynos: cpuidle: Move exynos_set_wakeupmask in the cpu_pm
 notifier
   ARM: exynos: cpuidle: Move the power sequence call in the cpu_pm
 notifier
   ARM: exynos: cpuidle: Move S5P_CHECK_AFTR in a header
   ARM: exynos: cpuidle: Move clock setup to pm.c
   ARM: exynos: cpuidle: Move the boot vector in pm.c
   ARM: exynos: cpuidle: Disable cpuidle for 5440
   ARM: exynos: cpuidle: Pass the AFTR callback to the platform_data
   ARM: exynos: cpuidle: Move the driver to drivers/cpuidle directory

  arch/arm/mach-exynos/Makefile|1 -
  arch/arm/mach-exynos/common.h|1 +
  arch/arm/mach-exynos/cpuidle.c   |  255 --
  arch/arm/mach-exynos/exynos.c|8 +-
  arch/arm/mach-exynos/pm.c|  177 +-
  arch/arm/mach-exynos/pmu.c   |6 +
  arch/arm/mach-exynos/regs-pmu.h  |   20 +++
  drivers/cpuidle/Kconfig.arm  |6 +
  drivers/cpuidle/Makefile |1 +
  drivers/cpuidle/cpuidle-exynos.c |   97 +++
  10 files changed, 285 insertions(+), 287 deletions(-)
  delete mode 100644 arch/arm/mach-exynos/cpuidle.c
  create mode 100644 drivers/cpuidle/cpuidle-exynos.c



For all patches except 10-12, 14 and 16:

Reviewed-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 V3 16/17] ARM: exynos: cpuidle: Pass the AFTR callback to the platform_data

2014-04-09 Thread Tomasz Figa


Hi Daniel,

On 08.04.2014 14:19, Daniel Lezcano wrote:

No more dependency on the arch code. The platform_data field is used to set the
PM callback as the other cpuidle drivers.

Signed-off-by: Daniel Lezcano 
Reviewed-by: Viresh Kumar 
Reviewed-by: Bartlomiej Zolnierkiewicz 
---
  arch/arm/mach-exynos/common.h  |1 +
  arch/arm/mach-exynos/cpuidle.c |6 --
  arch/arm/mach-exynos/exynos.c  |5 +++--
  arch/arm/mach-exynos/pmu.c |6 ++
  4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9ef3f83..7d9432e 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -62,5 +62,6 @@ struct exynos_pmu_conf {
  };

  extern void exynos_sys_powerdown_conf(enum sys_powerdown mode);
+extern void exynos_sys_powerdown_aftr(void);

  #endif /* __ARCH_ARM_MACH_EXYNOS_COMMON_H */
diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index a1f111e..25d2fad 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -16,12 +16,12 @@
  #include 
  #include 

-#include "common.h"
+static void (*exynos_aftr)(void);

  static int idle_finisher(unsigned long flags)
  {
/* Set value of power down register for aftr mode */
-   exynos_sys_powerdown_conf(SYS_AFTR);
+   exynos_aftr();
cpu_do_idle();
return 1;
  }
@@ -75,6 +75,8 @@ static int exynos_cpuidle_probe(struct platform_device *pdev)
  {
int ret;

+   exynos_aftr = (void *)(pdev->dev.platform_data);
+
ret = cpuidle_register(&exynos_idle_driver, NULL);
if (ret) {
dev_err(&pdev->dev, "failed to register cpuidle driver\n");
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index fe8dac8..a7ab561 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -221,8 +221,9 @@ void exynos_restart(enum reboot_mode mode, const char *cmd)
  }

  static struct platform_device exynos_cpuidle = {
-   .name   = "exynos_cpuidle",
-   .id = -1,
+   .name  = "exynos_cpuidle",
+   .dev.platform_data = exynos_sys_powerdown_aftr,
+   .id= -1,
  };

  void __init exynos_cpuidle_init(void)
diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index 05c7ce1..fd1ae22 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -389,6 +389,12 @@ void exynos_sys_powerdown_conf(enum sys_powerdown mode)
}
  }

+/* Set value of power down register for aftr mode */
+void exynos_sys_powerdown_aftr(void)
+{
+   exynos_sys_powerdown_conf(SYS_AFTR);


I wonder if this callback wouldn't be more appropriate for setting idle 
mode specific things like wake-up mask and boot flag.


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 V3 14/17] ARM: exynos: cpuidle: Move the boot vector in pm.c

2014-04-09 Thread Tomasz Figa


Hi Daniel,

On 08.04.2014 14:19, Daniel Lezcano wrote:

As usual, move the boot vector setting in the pm.c file and use the cpu_pm
notifier to set it up.

Remove the unused headers.

Signed-off-by: Daniel Lezcano 
Reviewed-by: Viresh Kumar 
Reviewed-by: Bartlomiej Zolnierkiewicz 
---
  arch/arm/mach-exynos/cpuidle.c |   23 ---
  arch/arm/mach-exynos/pm.c  |   15 +++
  2 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 44d169b..4b94181 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -8,46 +8,23 @@
   * published by the Free Software Foundation.
  */

-#include 
-#include 
  #include 
  #include 
-#include 
-#include 
-#include 
  #include 

  #include 
  #include 
-#include 
  #include 

  #include 
-#include 
-
-#include 

  #include "common.h"
-#include "regs-pmu.h"
-
-#define REG_DIRECTGO_ADDR  (samsung_rev() == EXYNOS4210_REV_1_1 ? \
-   S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
-   (S5P_VA_SYSRAM + 0x24) : S5P_INFORM0))
-#define REG_DIRECTGO_FLAG  (samsung_rev() == EXYNOS4210_REV_1_1 ? \
-   S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
-   (S5P_VA_SYSRAM + 0x20) : S5P_INFORM1))

  static int idle_finisher(unsigned long flags)
  {
-
-   __raw_writel(virt_to_phys(s3c_cpu_resume), REG_DIRECTGO_ADDR);
-   __raw_writel(S5P_CHECK_AFTR, REG_DIRECTGO_FLAG);
-
/* Set value of power down register for aftr mode */
exynos_sys_powerdown_conf(SYS_AFTR);
-
cpu_do_idle();
-
return 1;
  }

diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 90fb692..e4ecd8c 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -172,6 +172,20 @@ static void __init exynos5_core_down_clk(void)
__raw_writel(tmp, EXYNOS5_PWR_CTRL2);
  }

+#define EXYNOS_BOOT_VECTOR_ADDR (samsung_rev() == EXYNOS4210_REV_1_1 ? \
+   S5P_INFORM7 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
+   (S5P_VA_SYSRAM + 0x24) : S5P_INFORM0))
+
+#define EXYNOS_BOOT_VECTOR_FLAG (samsung_rev() == EXYNOS4210_REV_1_1 ? \
+   S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
+   (S5P_VA_SYSRAM + 0x20) : S5P_INFORM1))
+
+static void exynos_cpu_set_boot_vector(void)
+{
+   __raw_writel(virt_to_phys(exynos_cpu_resume), EXYNOS_BOOT_VECTOR_ADDR);
+   __raw_writel(S5P_CHECK_AFTR, EXYNOS_BOOT_VECTOR_FLAG);


S5P_CHECK_AFTR is a value specific for AFTR state. I wonder if it 
wouldn't be desired to let the cpuidle driver somehow affect the value 
written here.



+}
+
  static int exynos_cpu_suspend(unsigned long arg)
  {
  #ifdef CONFIG_CACHE_L2X0
@@ -387,6 +401,7 @@ static int exynos_cpu_pm_notifier(struct notifier_block 
*self,
exynos_cpu_save_register();
exynos_set_wakeupmask();
}
+   exynos_cpu_set_boot_vector();


Why this is outside of the if above? Should this really be called for 
all CPUs?


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] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

2014-04-09 Thread Vivek Gautam

Hi Sylwester,


On Wed, Apr 9, 2014 at 5:41 PM, Sylwester Nawrocki
 wrote:
> Hi Vivek,
>
> On 09/04/14 13:54, Vivek Gautam wrote:
>> Adding support to enable/disable VBUS hooked to a gpio
>> to enable vbus supply on the port.
>
> Does the GPIO control a fixed voltage regulator ? If so, shouldn't
> it be modelled by the regulator API instead ?

No, this GPIO controls a 'current limiting power distribution switch',
which gives the output vbus to usb controller.
Should i model this as a fixed regulator ?

>
>> Signed-off-by: Vivek Gautam 
> [...]
>> + /* Get required GPIO for vbus */
>> + phy_drd->gpio = of_get_named_gpio(dev->of_node,
>> +   "samsung,vbus-gpio", 0);
>> + if (!gpio_is_valid(phy_drd->gpio))
>> + dev_dbg(dev, "no usbdrd-phy vbus gpio defined\n");
>> +
>> + if (devm_gpio_request(dev, phy_drd->gpio, "phydrd_vbus_gpio"))
>> + dev_dbg(dev, "can't request usbdrd-phy vbus gpio %d\n",
>> + phy_drd->gpio);
>
> --
> Regards,
> Sylwester
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 12/17] ARM: exynos: cpuidle: Move S5P_CHECK_AFTR in a header

2014-04-09 Thread Tomasz Figa


Hi Daniel,

On 08.04.2014 14:19, Daniel Lezcano wrote:

Move the S5P_CHECK_AFTR definition to the header it belongs to.

Signed-off-by: Daniel Lezcano 
Reviewed-by: Viresh Kumar 
Reviewed-by: Bartlomiej Zolnierkiewicz 
---
  arch/arm/mach-exynos/cpuidle.c  |2 --
  arch/arm/mach-exynos/regs-pmu.h |1 +
  2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 81d7197..cd27dbf 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -37,8 +37,6 @@
S5P_INFORM6 : (samsung_rev() == EXYNOS4210_REV_1_0 ? \
(S5P_VA_SYSRAM + 0x20) : S5P_INFORM1))

-#define S5P_CHECK_AFTR 0xFCBA0D10
-
  #define EXYNOS5_PWR_CTRL1 (S5P_VA_CMU + 0x01020)
  #define EXYNOS5_PWR_CTRL2 (S5P_VA_CMU + 0x01024)

diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index 4f6a256..09c43c3 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -120,6 +120,7 @@
  #define S5P_INT_LOCAL_PWR_EN  0x7

  #define S5P_CHECK_SLEEP   0x0BAD
+#define S5P_CHECK_AFTR 0xFCBA0D10


This is not really a PMU-specific value. It is not a part of the 
hardware programming interface, but rather a part of the 
kernel-bootloader ABI used to handle wake-up from low power states. Same 
applies to S5P_CHECK_SLEEP.


I believe the proper place for such definitions would be the source file 
using it, which after patch 14/17 would be pm.c.


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 V3 11/17] ARM: exynos: cpuidle: Move the power sequence call in the cpu_pm notifier

2014-04-09 Thread Tomasz Figa


Hi Daniel,

On 08.04.2014 14:19, Daniel Lezcano wrote:

The code to initiate and exit the powerdown sequence is the same in pm.c and
cpuidle.c.

Let's split the common part in the pm.c and reuse it from the cpu_pm notifier.

That is one more step forward to make the cpuidle driver arch indenpendant.

Signed-off-by: Daniel Lezcano 
Reviewed-by: Viresh Kumar 
Reviewed-by: Bartlomiej Zolnierkiewicz 
---
  arch/arm/mach-exynos/cpuidle.c |   21 -
  arch/arm/mach-exynos/pm.c  |   22 ++
  2 files changed, 18 insertions(+), 25 deletions(-)


[snip]


-static int exynos_pm_suspend(void)
+static void exynos_pm_central_suspend(void)
  {
unsigned long tmp;

/* Setting Central Sequence Register for power down mode */
-
tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
__raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
+}
+
+static int exynos_pm_suspend(void)
+{
+   unsigned long tmp;



Shouldn't exynos_pm_central_suspend() be called here in place of the 
code being moved to that function?


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 V3 10/17] ARM: exynos: cpuidle: Move exynos_set_wakeupmask in the cpu_pm notifier

2014-04-09 Thread Tomasz Figa


Hi Daniel,

On 08.04.2014 14:19, Daniel Lezcano wrote:

Let's encapsulate more the PM code inside the PM file by moving the
'exynos_set_wakeupmask' function inside the pm.c and the call in the cpu_pm
notifier.

Signed-off-by: Daniel Lezcano 
Reviewed-by: Viresh Kumar 
Reviewed-by: Bartlomiej Zolnierkiewicz 
---
  arch/arm/mach-exynos/cpuidle.c |7 ---
  arch/arm/mach-exynos/pm.c  |7 +++
  2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index ce31004..01444ed 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -58,15 +58,8 @@
  #define PWR_CTRL2_CORE2_UP_RATIO  (1 << 4)
  #define PWR_CTRL2_CORE1_UP_RATIO  (1 << 0)

-/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
-static void exynos_set_wakeupmask(void)
-{
-   __raw_writel(0xff3e, S5P_WAKEUP_MASK);
-}
-
  static int idle_finisher(unsigned long flags)
  {
-   exynos_set_wakeupmask();

__raw_writel(virt_to_phys(s3c_cpu_resume), REG_DIRECTGO_ADDR);
__raw_writel(S5P_CHECK_AFTR, REG_DIRECTGO_FLAG);
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 9773a00..c8b3dc4 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -322,6 +322,12 @@ static const struct platform_suspend_ops 
exynos_suspend_ops = {
.valid  = suspend_valid_only_mem,
  };

+/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
+static void exynos_set_wakeupmask(void)
+{
+   __raw_writel(0xff3e, S5P_WAKEUP_MASK);
+}
+
  static int exynos_cpu_pm_notifier(struct notifier_block *self,
  unsigned long cmd, void *v)
  {
@@ -331,6 +337,7 @@ static int exynos_cpu_pm_notifier(struct notifier_block 
*self,
case CPU_PM_ENTER:
if (cpu == 0) {
exynos_cpu_save_register();
+   exynos_set_wakeupmask();


I'm not sure about this change.

Wake-up mask depends heavily on lower power state being entered, so CPU 
idle driver should be able to specify what mask to set. Of course 
currently the only state that requires this mask to be set is AFTR, but 
there are more states, such as LPA, which may need different value.


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] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

2014-04-09 Thread Sylwester Nawrocki

Hi Vivek,

On 09/04/14 13:54, Vivek Gautam wrote:
> Adding support to enable/disable VBUS hooked to a gpio
> to enable vbus supply on the port.

Does the GPIO control a fixed voltage regulator ? If so, shouldn't
it be modelled by the regulator API instead ?

> Signed-off-by: Vivek Gautam 
[...]
> + /* Get required GPIO for vbus */
> + phy_drd->gpio = of_get_named_gpio(dev->of_node,
> +   "samsung,vbus-gpio", 0);
> + if (!gpio_is_valid(phy_drd->gpio))
> + dev_dbg(dev, "no usbdrd-phy vbus gpio defined\n");
> +
> + if (devm_gpio_request(dev, phy_drd->gpio, "phydrd_vbus_gpio"))
> + dev_dbg(dev, "can't request usbdrd-phy vbus gpio %d\n",
> + phy_drd->gpio);

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

[PATCH] phy: exynos5-usbdrd: Add facility to toggle vbus gpio on/off

2014-04-09 Thread Vivek Gautam

Adding support to enable/disable VBUS hooked to a gpio
to enable vbus supply on the port.

Signed-off-by: Vivek Gautam 
---

Based on 'phy-exynos5-usbdrd' patches:
[PATCH V4 0/5] Add Exynos5 USB 3.0 phy driver based on generic PHY framework
http://www.spinics.net/lists/linux-usb/msg105507.html

 drivers/phy/phy-exynos5-usbdrd.c |   18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
index ff54a7c..5ca7485 100644
--- a/drivers/phy/phy-exynos5-usbdrd.c
+++ b/drivers/phy/phy-exynos5-usbdrd.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -176,6 +177,7 @@ struct exynos5_usbdrd_phy {
struct clk *ref_clk;
unsigned long ref_rate;
unsigned int refclk_reg;
+   int gpio;
 };
 
 #define to_usbdrd_phy(inst) \
@@ -460,6 +462,9 @@ static int exynos5_usbdrd_phy_power_on(struct phy *phy)
if (!IS_ERR(phy_drd->usb30_sclk))
clk_prepare_enable(phy_drd->usb30_sclk);
 
+   /* Toggle VBUS gpio - on */
+   gpio_set_value(phy_drd->gpio, 1);
+
/* Power-on PHY*/
inst->phy_cfg->phy_isol(inst, 0);
 
@@ -476,6 +481,9 @@ static int exynos5_usbdrd_phy_power_off(struct phy *phy)
/* Power-off the PHY */
inst->phy_cfg->phy_isol(inst, 1);
 
+   /* Toggle VBUS gpio - off */
+   gpio_set_value(phy_drd->gpio, 0);
+
if (!IS_ERR(phy_drd->usb30_sclk))
clk_disable_unprepare(phy_drd->usb30_sclk);
 
@@ -585,6 +593,16 @@ static int exynos5_usbdrd_phy_probe(struct platform_device 
*pdev)
 
phy_drd->drv_data = drv_data;
 
+   /* Get required GPIO for vbus */
+   phy_drd->gpio = of_get_named_gpio(dev->of_node,
+ "samsung,vbus-gpio", 0);
+   if (!gpio_is_valid(phy_drd->gpio))
+   dev_dbg(dev, "no usbdrd-phy vbus gpio defined\n");
+
+   if (devm_gpio_request(dev, phy_drd->gpio, "phydrd_vbus_gpio"))
+   dev_dbg(dev, "can't request usbdrd-phy vbus gpio %d\n",
+   phy_drd->gpio);
+
if (of_property_read_u32(node, "samsung,pmu-offset", &pmu_offset)) {
dev_err(dev, "Missing pmu-offset for phy isolation\n");
return -EINVAL;
-- 
1.7.10.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] arm: exynos: generalize power register address calculation

2014-04-09 Thread Tomasz Figa


Hi Chander,

On 09.04.2014 13:09, Chander Kashyap wrote:

Currently status/configuration power register values are hard-coded for cpu1.

Make it generic so that it is useful for SoC's with more than two cpus.

Signed-off-by: Chander Kashyap 
---
changes in v2 : Used existing macros for clusterid and cpuid calculation

  arch/arm/mach-exynos/hotplug.c  |   15 ---
  arch/arm/mach-exynos/platsmp.c  |   20 +++-
  arch/arm/mach-exynos/regs-pmu.h |9 +++--
  3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index 5eead53..eab6121 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -17,6 +17,7 @@

  #include 
  #include 
+#include 
  #include 

  #include 
@@ -92,11 +93,19 @@ static inline void cpu_leave_lowpower(void)

  static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
  {
+   unsigned int mpidr, cpunr, cluster;
+
+   mpidr = cpu_logical_map(cpu);
+   cpunr = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+   cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+   /* Maximum possible cpus in a cluster can be 4 */
+   cpunr += cluster * 4;


I believe this is rather a weak assumption. First of all, the limit 
seems to be hardcoded only for the few existing SoCs. In addition, the 
value is not used as a maximum, but rather it is assumed that each 
cluster has always four cores.


Moreover, it is assumed here that the mapping between core ID 
(calculated by the equation below) and PMU core numbers is 1:1, which is 
not true. On Exynos4210, the cluster ID is always 0x09 and on Exynos4x12 
it is 0x0a, which will lead to completely wrong register offsets.


I believe the proper way to deal with this is to provide per-CPU 
property in DT called "samsung,pmu-offset" that could be used be code 
like this to calculate register addresses properly.


For now, I would recommend doing the above ignoring cluster ID 
completely to not break (and actually fix) single cluster systems and 
existing multi cluster ones on which only the first cluster is supported 
now.


After that, per-CPU PMU offset should be implemented to support 
multi-cluster SoCs with proper support of multiple clusters.


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 V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

2014-04-09 Thread Vivek Gautam

Hi,


On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa  wrote:
> Hi Vivek,
>
> Please see my comments inline.
>
>
> On 08.04.2014 16:36, Vivek Gautam wrote:
>>
>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
>> The new driver uses the generic PHY framework and will interact
>> with DWC3 controller present on Exynos5 series of SoCs.
>> Thereby, removing old phy-samsung-usb3 driver and related code
>> used untill now which was based on usb/phy framework.
>>
>> Signed-off-by: Vivek Gautam 
>> ---
>>   .../devicetree/bindings/phy/samsung-phy.txt|   42 ++
>>   drivers/phy/Kconfig|   11 +
>>   drivers/phy/Makefile   |1 +
>>   drivers/phy/phy-exynos5-usbdrd.c   |  668
>> 
>>   4 files changed, 722 insertions(+)
>>   create mode 100644 drivers/phy/phy-exynos5-usbdrd.c
>
>
> [snip]
>
>
>> +   Additional clock required for Exynos5420:
>> +   - usb30_sclk_100m: Additional special clock used for PHY operation
>> +  depicted as 'sclk_usbphy30' in CMU of
>> Exynos5420.
>
>
> Are you sure this isn't simply a gate for the ref clock, as it can be found
> on another SoC that is not upstream yet? I don't have documentation for
> Exynos 5420 so I can't tell, but I'd like to ask you to recheck this.

>From what i can see in the manual :
sclk_usbphy30 is derived from OSCCLK.
It is coming from a MUX (default input line to this is OSCCLK)  and
then through a DIV
there's this gate.

  {OSCCLK  + other sources} --->[MUX] ---> [DIV] --> [GATE for
sclk_usbphy30]

the {rate of sclk_usbphy30} == OSCCLK

However the 'ref' clock that we have been using is the actual oscillator clock.
And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30).
So should this mean that ref clock and sclk_usbphy30 are still be controlled by
two different gates ?

>
>
>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to
>> + control pmu registers for power isolation.
>> +- samsung,pmu-offset: phy power control register offset to
>> pmu-system-controller
>> + base.
>> +- #phy-cells : from the generic PHY bindings, must be 1;
>> +
>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy"
>> +compatible PHYs, the second cell in the PHY specifier identifies the
>> +PHY id, which is interpreted as follows:
>> +  0 - UTMI+ type phy,
>> +  1 - PIPE3 type phy,
>> +
>> +Example:
>> +   usb3_phy: usbphy@1210 {
>> +   compatible = "samsung,exynos5250-usbdrd-phy";
>> +   reg = <0x1210 0x100>;
>> +   clocks = <&clock 286>, <&clock 1>;
>> +   clock-names = "phy", "usb3phy_refclk";
>
>
> Binding description above doesn't mention "usb3phy_refclk" entry.

my bad !! will correct this.

>
>
>> +   samsung,syscon-phandle = <&pmu_syscon>;
>> +   samsung,pmu-offset = <0x704>;
>> +   #phy-cells = <1>;
>> +   };
>
>
> [snip]
>
>
>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c
>> b/drivers/phy/phy-exynos5-usbdrd.c
>> new file mode 100644
>> index 000..ff54a7c
>> --- /dev/null
>> +++ b/drivers/phy/phy-exynos5-usbdrd.c
>
>
> [snip]
>
>
>> +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
>> +{
>> +   struct device *dev = &pdev->dev;
>> +   struct device_node *node = dev->of_node;
>> +   struct exynos5_usbdrd_phy *phy_drd;
>> +   struct phy_provider *phy_provider;
>> +   struct resource *res;
>> +   const struct of_device_id *match;
>> +   const struct exynos5_usbdrd_phy_drvdata *drv_data;
>> +   struct regmap *reg_pmu;
>> +   u32 pmu_offset;
>> +   int i;
>> +
>> +   /*
>> +* Exynos systems are completely DT enabled,
>> +* so lets not have any platform data support for this driver.
>> +*/
>> +   if (!node) {
>> +   dev_err(dev, "no device node found\n");
>
>
> This error message is not very meaningful. I'd rather use something like
> "This driver can be only instantiated using Device Tree".

Sure, will amend this.

>
>
>> +   return -ENODEV;
>> +   }
>> +
>> +   match = of_match_node(exynos5_usbdrd_phy_of_match,
>> pdev->dev.of_node);
>> +   if (!match) {
>> +   dev_err(dev, "of_match_node() failed\n");
>> +   return -EINVAL;
>> +   }
>> +   drv_data = match->data;
>> +
>> +   phy_drd = devm_kzalloc(dev, sizeof(*phy_drd), GFP_KERNEL);
>> +   if (!phy_drd)
>> +   return -ENOMEM;
>> +
>> +   dev_set_drvdata(dev, phy_drd);
>> +   phy_drd->dev = dev;
>> +
>> +   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +   phy_drd->reg_phy = devm_ioremap_resource(dev, res);
>> +   if (IS_ERR(phy_drd->reg_phy)) {
>> +   dev_err(dev, "Failed to map register memory (phy)\n");
>
>
> devm_ioremap_resource() already prints an error message.

Re: [PATCHv2 1/3] phy: Add exynos-simple-phy driver

2014-04-09 Thread Andreas Oberritter

Hello Andrzej,

On 09.04.2014 10:37, Andrzej Hajda wrote:
>> +static int exynos_phy_probe(struct platform_device *pdev)
>> +{
>> +const struct of_device_id *of_id = of_match_device(
>> +of_match_ptr(exynos_phy_of_match), &pdev->dev);
>> +const u32 *offsets = of_id->data;
>> +int count;
>> +struct device *dev = &pdev->dev;
>> +struct phy **phys;
>> +struct resource *res;
>> +void __iomem *regs;
>> +int i;
>> +struct phy_provider *phy_provider;
>> +
>> +/* count number of phys to create */
>> +for (count = 0; offsets[count] != ~0; ++count)
>> +;
> 
> count = ARRAY_SIZE(offsets) - 1;

u32 *offsets is not an array.

Regards,
Andreas
--
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 5/5] usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver

2014-04-09 Thread Vivek Gautam

Hi Tomasz,
'

On Wed, Apr 9, 2014 at 4:43 PM, Tomasz Figa  wrote:
> Hi Vivek,
>
Thanks for reviewing the patch series.

>
> On 08.04.2014 16:36, Vivek Gautam wrote:
>>
>> Removing this older USB 3.0 DRD controller PHY driver, since
>> a new driver based on generic phy framework is now available.
>>
>> Also removing the dt node for older driver from Exynos5250
>> device tree and updating the dt node for DWC3 controller.
>>
>> Signed-off-by: Vivek Gautam 
>> ---
>>
>> NOTE: This patch should be merged only when the new USB 3.0
>> DRD phy controller driver is available in the tree from the
>> patches:
>> phy: Add new Exynos5 USB 3.0 PHY driver; and
>> dt: exynos5250: Enable support for generic USB DRD phy
>>
>>   arch/arm/boot/dts/exynos5250.dtsi  |   17 +-
>>   drivers/usb/phy/phy-samsung-usb.h  |   83 -
>>   drivers/usb/phy/phy-samsung-usb3.c |  350
>> 
>>   3 files changed, 2 insertions(+), 448 deletions(-)
>>   delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c
>>
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
>> b/arch/arm/boot/dts/exynos5250.dtsi
>> index 92c6fcd..1cb1e91 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>
>
> IMHO driver and dts changes should be separated into two patches, first
> updating device tree to use the new driver and second removing the driver.

Sure will separate the patch into driver and dts change.

>
> After fixing this issue,
>
> Reviewed-by: Tomasz Figa 
>
> --
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: [PATCHv2 1/3] phy: Add exynos-simple-phy driver

2014-04-09 Thread Tomasz Stanislawski

Hi Rahul,

On 04/09/2014 11:12 AM, Rahul Sharma wrote:
> Hi Tomasz,
> 
> On 9 April 2014 14:07, Andrzej Hajda  wrote:
>> Hi Tomasz,
>>
>> On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote:
>>> Add exynos-simple-phy driver to support a single register
>>> PHY interfaces present on Exynos4 SoC.
>>>
>>> Signed-off-by: Tomasz Stanislawski 

[snip]

>>> +
>>> + regs = devm_ioremap(dev, res->start, res->end - res->start);
>>> + if (!regs) {
>>> + dev_err(dev, "failed to ioremap registers\n");
>>> + return -EFAULT;
>>> + }
>>
>> Why not devm_ioremap_resource? If not, resource_size function calculates
>> length of resource correctly.
>>
>> Anyway I like the idea of implementing multiple phys in one driver.
>> The only drawback I see is that some phys will be created even there are
>> no consumers for them. To avoid such situation you can try to use
>> lazy approach - create phy only if there is request for it,
>> exynos_phy_xlate callback should allow this.
>>
>> Regards
>> Andrzej
>>
> 
> Idea looks good. How about keeping compatible which is independent
> of SoC, something like "samsung,exynos-simple-phy" and provide Reg
> and Bit through phy provider node. This way we can avoid SoC specific
> hardcoding in phy driver and don't need to look into dt bindings for
> each new SoC.

A very nice idea BUT there is a very strong pressure from DT guys
to avoid adding any bit fields/offsets/masks in DT nodes.

Hopefully, as long as driver name starts with "exynos-" prefix
one can hide SoCs specific tricks deep inside driver code.

The idea behind this driver was not to create a generic phy for 1-bit
devices but rather to hide SoC-specific issues from client drivers
like DRM-HDMI.

> 
> We can use syscon interface to access PMU bits like USB phy.
> PMU is already registered as system controller
> 

Ok. I will try to use it in PATCHv3.

> Regards,
> Rahul Sharma.
> 

Regards,
Tomasz Stanislawski


--
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 5/5] usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver

2014-04-09 Thread Tomasz Figa


Hi Vivek,

On 08.04.2014 16:36, Vivek Gautam wrote:

Removing this older USB 3.0 DRD controller PHY driver, since
a new driver based on generic phy framework is now available.

Also removing the dt node for older driver from Exynos5250
device tree and updating the dt node for DWC3 controller.

Signed-off-by: Vivek Gautam 
---

NOTE: This patch should be merged only when the new USB 3.0
DRD phy controller driver is available in the tree from the
patches:
phy: Add new Exynos5 USB 3.0 PHY driver; and
dt: exynos5250: Enable support for generic USB DRD phy

  arch/arm/boot/dts/exynos5250.dtsi  |   17 +-
  drivers/usb/phy/phy-samsung-usb.h  |   83 -
  drivers/usb/phy/phy-samsung-usb3.c |  350 
  3 files changed, 2 insertions(+), 448 deletions(-)
  delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index 92c6fcd..1cb1e91 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi


IMHO driver and dts changes should be separated into two patches, first 
updating device tree to use the new driver and second removing the driver.


After fixing this issue,

Reviewed-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 V4 4/5] dt: exynos5250: Enable support for generic USB DRD phy

2014-04-09 Thread Tomasz Figa


On 08.04.2014 16:36, Vivek Gautam wrote:

Add device tree node for new usbdrd-phy driver, which
is based on generic phy framework.

Signed-off-by: Vivek Gautam 
---
  arch/arm/boot/dts/exynos5250.dtsi |   10 ++
  1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
b/arch/arm/boot/dts/exynos5250.dtsi
index b7dec41..92c6fcd 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -530,6 +530,16 @@
};
};

+   usbdrd_phy: phy@1210 {
+   compatible = "samsung,exynos5250-usbdrd-phy";
+   reg = <0x1210 0x100>;
+   clocks = <&clock 286>, <&clock 1>;
+   clock-names = "phy", "ref";
+   samsung,syscon-phandle = <&pmu_system_controller>;
+   samsung,pmu-offset = <0x704>;
+   #phy-cells = <1>;
+   };
+
usb@1211 {
compatible = "samsung,exynos4210-ehci";
reg = <0x1211 0x100>;



Reviewed-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 V4 3/5] dt: exynos5420: Enable support for DWC3 controller

2014-04-09 Thread Tomasz Figa


On 08.04.2014 16:36, Vivek Gautam wrote:

Add device tree nodes for DWC3 controller present on
Exynos 5420 SoC, to enable support for USB 3.0.

Signed-off-by: Vivek Gautam 
---
  arch/arm/boot/dts/exynos5420.dtsi |   34 ++
  1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
b/arch/arm/boot/dts/exynos5420.dtsi
index a6efb52..20c2d0b 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -653,6 +653,23 @@
clock-names = "tmu_apbif", "tmu_triminfo_apbif";
};

+   usb@1200 {
+   compatible = "samsung,exynos5250-dwusb3";
+   clocks = <&clock 366>;
+   clock-names = "usbdrd30";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   dwc3 {
+   compatible = "synopsys,dwc3";
+   reg = <0x1200 0x1>;
+   interrupts = <0 72 0>;
+   phys = <&usbdrd_phy0 0>, <&usbdrd_phy0 1>;
+   phy-names = "usb2-phy", "usb3-phy";
+   };
+   };
+
usbdrd_phy0: phy@1210 {
compatible = "samsung,exynos5420-usbdrd-phy";
reg = <0x1210 0x100>;
@@ -663,6 +680,23 @@
#phy-cells = <1>;
};

+   usb@1240 {
+   compatible = "samsung,exynos5250-dwusb3";
+   clocks = <&clock 367>;
+   clock-names = "usbdrd30";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   dwc3 {
+   compatible = "synopsys,dwc3";
+   reg = <0x1240 0x1>;
+   interrupts = <0 73 0>;
+   phys = <&usbdrd_phy1 0>, <&usbdrd_phy1 1>;
+   phy-names = "usb2-phy", "usb3-phy";
+   };
+   };
+
usbdrd_phy1: phy@1250 {
compatible = "samsung,exynos5420-usbdrd-phy";
reg = <0x1250 0x100>;



Reviewed-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 v4 2/5] dt: exynos5420: Enable support for USB 3.0 PHY controller

2014-04-09 Thread Tomasz Figa


On 08.04.2014 16:36, Vivek Gautam wrote:

Add device tree nodes for USB 3.0 PHY present alongwith
USB 3.0 controller Exynos 5420 SoC. This phy driver is
based on generic phy framework.

Signed-off-by: Vivek Gautam 
---
  arch/arm/boot/dts/exynos5420.dtsi |   20 
  1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
b/arch/arm/boot/dts/exynos5420.dtsi
index 8db792b..a6efb52 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -652,4 +652,24 @@
clocks = <&clock 319>, <&clock 318>;
clock-names = "tmu_apbif", "tmu_triminfo_apbif";
};
+
+   usbdrd_phy0: phy@1210 {
+   compatible = "samsung,exynos5420-usbdrd-phy";
+   reg = <0x1210 0x100>;
+   clocks = <&clock 366>, <&clock 1>, <&clock 152>;
+   clock-names = "phy", "ref", "usb30_sclk_100m";


As I mentioned in another reply, please make sure that "usb30_sclk_100m" 
isn't simply a gate clock for "ref" clock.


Otherwise,

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

[PATCH] arm: exynos: generalize power register address calculation

2014-04-09 Thread Chander Kashyap

Currently status/configuration power register values are hard-coded for cpu1.

Make it generic so that it is useful for SoC's with more than two cpus.

Signed-off-by: Chander Kashyap 
---
changes in v2 : Used existing macros for clusterid and cpuid calculation

 arch/arm/mach-exynos/hotplug.c  |   15 ---
 arch/arm/mach-exynos/platsmp.c  |   20 +++-
 arch/arm/mach-exynos/regs-pmu.h |9 +++--
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index 5eead53..eab6121 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -17,6 +17,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -92,11 +93,19 @@ static inline void cpu_leave_lowpower(void)
 
 static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
 {
+   unsigned int mpidr, cpunr, cluster;
+
+   mpidr = cpu_logical_map(cpu);
+   cpunr = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+   cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+   /* Maximum possible cpus in a cluster can be 4 */
+   cpunr += cluster * 4;
for (;;) {
 
-   /* make cpu1 to be turned off at next WFI command */
-   if (cpu == 1)
-   __raw_writel(0, S5P_ARM_CORE1_CONFIGURATION);
+   /* make cpu to be turned off at next WFI command */
+   if (cpu)
+   __raw_writel(0, S5P_ARM_CORE_CONFIGURATION(cpunr));
 
/*
 * here's the WFI
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 8ea02f6..8d06b2c 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -22,6 +22,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -92,6 +93,14 @@ static int exynos_boot_secondary(unsigned int cpu, struct 
task_struct *idle)
 {
unsigned long timeout;
unsigned long phys_cpu = cpu_logical_map(cpu);
+   unsigned int mpidr, cpunr, cluster;
+
+   mpidr = cpu_logical_map(cpu);
+   cpunr = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+   cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+   /* Maximum possible cpus in a cluster can be 4 */
+   cpunr += cluster * 4;
 
/*
 * Set synchronisation state between this boot processor
@@ -109,14 +118,15 @@ static int exynos_boot_secondary(unsigned int cpu, struct 
task_struct *idle)
 */
write_pen_release(phys_cpu);
 
-   if (!(__raw_readl(S5P_ARM_CORE1_STATUS) & S5P_CORE_LOCAL_PWR_EN)) {
+   if (!(__raw_readl(S5P_ARM_CORE_STATUS(cpunr))
+   & S5P_CORE_LOCAL_PWR_EN)) {
__raw_writel(S5P_CORE_LOCAL_PWR_EN,
-S5P_ARM_CORE1_CONFIGURATION);
+S5P_ARM_CORE_CONFIGURATION(cpunr));
 
timeout = 10;
 
-   /* wait max 10 ms until cpu1 is on */
-   while ((__raw_readl(S5P_ARM_CORE1_STATUS)
+   /* wait max 10 ms until secondary cpu is on */
+   while ((__raw_readl(S5P_ARM_CORE_STATUS(cpunr))
& S5P_CORE_LOCAL_PWR_EN) != S5P_CORE_LOCAL_PWR_EN) {
if (timeout-- == 0)
break;
@@ -125,7 +135,7 @@ static int exynos_boot_secondary(unsigned int cpu, struct 
task_struct *idle)
}
 
if (timeout == 0) {
-   printk(KERN_ERR "cpu1 power enable failed");
+   pr_err("cpu%x power enable failed", cpu);
spin_unlock(&boot_lock);
return -ETIMEDOUT;
}
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index 7c029ce..16e17e4 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -104,8 +104,13 @@
 #define S5P_GPS_LOWPWR S5P_PMUREG(0x139C)
 #define S5P_GPS_ALIVE_LOWPWR   S5P_PMUREG(0x13A0)
 
-#define S5P_ARM_CORE1_CONFIGURATIONS5P_PMUREG(0x2080)
-#define S5P_ARM_CORE1_STATUS   S5P_PMUREG(0x2084)
+#define S5P_ARM_CORE0_CONFIGURATIONS5P_PMUREG(0x2000)
+#define S5P_ARM_CORE0_STATUS   S5P_PMUREG(0x2004)
+
+#define S5P_ARM_CORE_CONFIGURATION(_cpunr) \
+   (S5P_ARM_CORE0_CONFIGURATION + 0x80 * _cpunr)
+#define S5P_ARM_CORE_STATUS(_cpunr)\
+   (S5P_ARM_CORE0_STATUS + 0x80 * _cpunr)
 
 #define S5P_PAD_RET_MAUDIO_OPTION  S5P_PMUREG(0x3028)
 #define S5P_PAD_RET_GPIO_OPTIONS5P_PMUREG(0x3108)
-- 
1.7.9.5

--
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 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

2014-04-09 Thread Tomasz Figa


Hi Vivek,

Please see my comments inline.

On 08.04.2014 16:36, Vivek Gautam wrote:

Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
The new driver uses the generic PHY framework and will interact
with DWC3 controller present on Exynos5 series of SoCs.
Thereby, removing old phy-samsung-usb3 driver and related code
used untill now which was based on usb/phy framework.

Signed-off-by: Vivek Gautam 
---
  .../devicetree/bindings/phy/samsung-phy.txt|   42 ++
  drivers/phy/Kconfig|   11 +
  drivers/phy/Makefile   |1 +
  drivers/phy/phy-exynos5-usbdrd.c   |  668 
  4 files changed, 722 insertions(+)
  create mode 100644 drivers/phy/phy-exynos5-usbdrd.c


[snip]


+   Additional clock required for Exynos5420:
+   - usb30_sclk_100m: Additional special clock used for PHY operation
+  depicted as 'sclk_usbphy30' in CMU of Exynos5420.


Are you sure this isn't simply a gate for the ref clock, as it can be 
found on another SoC that is not upstream yet? I don't have 
documentation for Exynos 5420 so I can't tell, but I'd like to ask you 
to recheck this.



+- samsung,syscon-phandle: phandle for syscon interface, which is used to
+ control pmu registers for power isolation.
+- samsung,pmu-offset: phy power control register offset to 
pmu-system-controller
+ base.
+- #phy-cells : from the generic PHY bindings, must be 1;
+
+For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy"
+compatible PHYs, the second cell in the PHY specifier identifies the
+PHY id, which is interpreted as follows:
+  0 - UTMI+ type phy,
+  1 - PIPE3 type phy,
+
+Example:
+   usb3_phy: usbphy@1210 {
+   compatible = "samsung,exynos5250-usbdrd-phy";
+   reg = <0x1210 0x100>;
+   clocks = <&clock 286>, <&clock 1>;
+   clock-names = "phy", "usb3phy_refclk";


Binding description above doesn't mention "usb3phy_refclk" entry.


+   samsung,syscon-phandle = <&pmu_syscon>;
+   samsung,pmu-offset = <0x704>;
+   #phy-cells = <1>;
+   };


[snip]


diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c
new file mode 100644
index 000..ff54a7c
--- /dev/null
+++ b/drivers/phy/phy-exynos5-usbdrd.c


[snip]


+static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
+{
+   struct device *dev = &pdev->dev;
+   struct device_node *node = dev->of_node;
+   struct exynos5_usbdrd_phy *phy_drd;
+   struct phy_provider *phy_provider;
+   struct resource *res;
+   const struct of_device_id *match;
+   const struct exynos5_usbdrd_phy_drvdata *drv_data;
+   struct regmap *reg_pmu;
+   u32 pmu_offset;
+   int i;
+
+   /*
+* Exynos systems are completely DT enabled,
+* so lets not have any platform data support for this driver.
+*/
+   if (!node) {
+   dev_err(dev, "no device node found\n");


This error message is not very meaningful. I'd rather use something like 
"This driver can be only instantiated using Device Tree".



+   return -ENODEV;
+   }
+
+   match = of_match_node(exynos5_usbdrd_phy_of_match, pdev->dev.of_node);
+   if (!match) {
+   dev_err(dev, "of_match_node() failed\n");
+   return -EINVAL;
+   }
+   drv_data = match->data;
+
+   phy_drd = devm_kzalloc(dev, sizeof(*phy_drd), GFP_KERNEL);
+   if (!phy_drd)
+   return -ENOMEM;
+
+   dev_set_drvdata(dev, phy_drd);
+   phy_drd->dev = dev;
+
+   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+   phy_drd->reg_phy = devm_ioremap_resource(dev, res);
+   if (IS_ERR(phy_drd->reg_phy)) {
+   dev_err(dev, "Failed to map register memory (phy)\n");


devm_ioremap_resource() already prints an error message.

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: [PATCHv2 2/3] drm: exynos: hdmi: use hdmiphy as PHY

2014-04-09 Thread Tomasz Stanislawski

Hi Andrzej,
This issue could be solved by exporting a regmap from PMU driver
or Exynos clock provider for the usage by exynos-simple-phy.
The operations on PHYs from exynos-simple-phy provider would
be chained to PMU driver and protected by a spinlock in the regmap.

Luckily, the divider is not used as far as I know.

Regards,
Tomasz Stanislawski

On 04/09/2014 12:30 PM, Andrzej Hajda wrote:
> Hi Tomasz,
> 
> On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote:
>> The HDMIPHY (physical interface) is controlled by a single
>> bit in a power controller's regiter. It was implemented
>> as clock. It was a simple but effective hack.
> 
> This power controller register has also bits to control HDMI clock
> divider ratio. I guess current drivers do not change it, but how do you
> want to implement access to it if some HDMI driver in the future will
> need to change ratio. I guess in case of clk it would be easier.
> 
> Regards
> Andrzej
> 
> 

--
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 v7 3/3] ARM: dts: add dts files for xyref5260 board

2014-04-09 Thread Tomasz Figa


On 07.04.2014 10:40, Sachin Kamat wrote:

On 3 April 2014 15:30, Rahul Sharma  wrote:

The patch adds the dts files for xyref5260 board which
is based on Exynos5260 Evt0 sample.

Signed-off-by: Rahul Sharma 
---



+/ {
+   model = "SAMSUNG XYREF5260 EVT0 board based on EXYNOS5260";
+   compatible = "samsung,xyref5260", "samsung,exynos5260";


"samsung,exynos5260" is OK here for future compatibility (if soc
specific quirk is required).
Please also add "samsung,exynos5" to compatible list.



Right. SoC family compatible string should be added between board and 
SoC model strings.


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: [PATCHv2 2/3] drm: exynos: hdmi: use hdmiphy as PHY

2014-04-09 Thread Rahul Sharma

Hi Andrzej,

On 9 April 2014 16:00, Andrzej Hajda  wrote:
> Hi Tomasz,
>
> On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote:
>> The HDMIPHY (physical interface) is controlled by a single
>> bit in a power controller's regiter. It was implemented
>> as clock. It was a simple but effective hack.
>
> This power controller register has also bits to control HDMI clock
> divider ratio. I guess current drivers do not change it, but how do you
> want to implement access to it if some HDMI driver in the future will
> need to change ratio. I guess in case of clk it would be easier.

If it is really required to change this divider, it should be registered as
a clock provider in clock driver exposing single divider clock.

Regards,
Rahul Sharma

>
> Regards
> Andrzej
>
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
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/3] ARM: dts: add dts files for exynos5260 SoC

2014-04-09 Thread Sachin Kamat

On 7 April 2014 14:14, Sachin Kamat  wrote:
> On 3 April 2014 10:47, Rahul Sharma  wrote:
>> The patch adds the dts files for exynos5260.
>>
>> Signed-off-by: Pankaj Dubey 
>> Signed-off-by: Rahul Sharma 
>> Signed-off-by: Arun Kumar K 
>> Reviewed-by: Tomasz Figa 
>> ---
> 
>
>> +/ {
>> +   compatible = "samsung,exynos5260";
>
> Add "samsung,exynos5260" to the above list.

Sorry for the typo. Should have been "samsung,exynos5" as mentioned by Tomasz.

-- 
With warm regards,
Sachin
--
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: [PATCHv2 2/3] drm: exynos: hdmi: use hdmiphy as PHY

2014-04-09 Thread Andrzej Hajda

Hi Tomasz,

On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote:
> The HDMIPHY (physical interface) is controlled by a single
> bit in a power controller's regiter. It was implemented
> as clock. It was a simple but effective hack.

This power controller register has also bits to control HDMI clock
divider ratio. I guess current drivers do not change it, but how do you
want to implement access to it if some HDMI driver in the future will
need to change ratio. I guess in case of clk it would be easier.

Regards
Andrzej

--
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 v7 2/3] ARM: dts: add dts files for exynos5260 SoC

2014-04-09 Thread Tomasz Figa


Hi Rahul,

On 03.04.2014 12:00, Rahul Sharma wrote:

The patch adds the dts files for exynos5260.

Signed-off-by: Pankaj Dubey 
Signed-off-by: Rahul Sharma 
Signed-off-by: Arun Kumar K 
Reviewed-by: Tomasz Figa 
---
  arch/arm/boot/dts/exynos5260-pinctrl.dtsi |  574 +
  arch/arm/boot/dts/exynos5260.dtsi |  304 +++
  2 files changed, 878 insertions(+)
  create mode 100644 arch/arm/boot/dts/exynos5260-pinctrl.dtsi
  create mode 100644 arch/arm/boot/dts/exynos5260.dtsi


[snip]


diff --git a/arch/arm/boot/dts/exynos5260.dtsi 
b/arch/arm/boot/dts/exynos5260.dtsi
new file mode 100644
index 000..188a2ac
--- /dev/null
+++ b/arch/arm/boot/dts/exynos5260.dtsi
@@ -0,0 +1,304 @@
+/*
+ * SAMSUNG EXYNOS5260 SoC device tree source
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * 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 "skeleton.dtsi"
+
+#include 
+
+/ {
+   compatible = "samsung,exynos5260";


Since Sachin's patches, "samsung,exynos5" compatible should be also 
added, before the more specific ones.


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: [PATCHv2 1/3] phy: Add exynos-simple-phy driver

2014-04-09 Thread Sylwester Nawrocki

Hi,

On 09/04/14 11:12, Rahul Sharma wrote:
> Idea looks good. How about keeping compatible which is independent
> of SoC, something like "samsung,exynos-simple-phy" and provide Reg
> and Bit through phy provider node. This way we can avoid SoC specific
> hardcoding in phy driver and don't need to look into dt bindings for
> each new SoC.

I believe it is a not recommended approach.

> We can use syscon interface to access PMU bits like USB phy.
> PMU is already registered as system controller

Yes, that sounds good. This way we could avoid overlapping memory
mapped register regions specified in 'reg' properties in the device
tree.

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

Re: [PATCH v7 1/3] ARM: EXYNOS: initial board support for exynos5260 SoC

2014-04-09 Thread Rahul Sharma

On 9 April 2014 14:34, Tomasz Figa  wrote:
> On 07.04.2014 10:38, Sachin Kamat wrote:
>>
>> Hi Rahul,
>>
>> On 3 April 2014 15:30, Rahul Sharma  wrote:
>>>
>>> From: Pankaj Dubey 
>>>
>>> This patch add basic arch side support for exynos5260 SoC.
>>
>> 
>>
>>> @@ -374,6 +374,7 @@ static char const *exynos_dt_compat[] __initconst = {
>>>  "samsung,exynos4412",
>>>  "samsung,exynos5",
>>>  "samsung,exynos5250",
>>> +   "samsung,exynos5260",
>>
>>
>> I don't think we should be adding this as we do not yet have any soc
>> specific quirk.
>> Compatibility to generic string, "samsung,exynos5"  should be sufficient.
>>
>
> Yes. Unless a SoC specific quirk that needs to be handled at such low level
> stage emerges, there is no need to add such compatible string to the kernel.
>

Thanks Sachin, Tomasz,

I will change this.

Regards,
Rahul Sharma.

> 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: [PATCHv2 1/3] phy: Add exynos-simple-phy driver

2014-04-09 Thread Rahul Sharma

Hi Tomasz,

On 9 April 2014 14:07, Andrzej Hajda  wrote:
> Hi Tomasz,
>
> On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote:
>> Add exynos-simple-phy driver to support a single register
>> PHY interfaces present on Exynos4 SoC.
>>
>> Signed-off-by: Tomasz Stanislawski 
>> ---
>>  .../devicetree/bindings/phy/samsung-phy.txt|   24 +++
>>  drivers/phy/Kconfig|5 +
>>  drivers/phy/Makefile   |1 +
>>  drivers/phy/exynos-simple-phy.c|  154 
>> 
>>  4 files changed, 184 insertions(+)
>>  create mode 100644 drivers/phy/exynos-simple-phy.c
>>
>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt 
>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> index b422e38..f97c4c3 100644
>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
>> @@ -114,3 +114,27 @@ Example:
>>   compatible = "samsung,exynos-sataphy-i2c";
>>   reg = <0x38>;
>>   };
>> +
>> +Samsung S5P/EXYNOS SoC series SIMPLE PHY
>> +-
>> +
>> +Required properties:
>> +- compatible : should be one of the listed compatibles:
>> + - "samsung,exynos4210-simple-phy"
>> + - "samsung,exynos4412-simple-phy"
>> +- reg : offset and length of the register set;
>> +- #phy-cells : from the generic phy bindings, must be 1;
>> +
>> +For "samsung,exynos4210-simple-phy" compatible PHYs the second cell in
>> +the PHY specifier identifies the PHY and its meaning is as follows:
>> +  0 - HDMI PHY,
>> +  1 - DAC PHY,
>> +  2 - ADC PHY,
>> +  3 - PCIE PHY.
>> +  4 - SATA PHY.
>> +
>> +For "samsung,exynos4412-simple-phy" compatible PHYs the second cell in
>> +the PHY specifier identifies the PHY and its meaning is as follows:
>> +  0 - HDMI PHY,
>> +  1 - ADC PHY,
>
> What about using preprocessor macros?
>
>> +
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 3bb05f1..65ab783 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -166,4 +166,9 @@ config PHY_XGENE
>>   help
>> This option enables support for APM X-Gene SoC multi-purpose PHY.
>>
>> +config EXYNOS_SIMPLE_PHY
>> + tristate "Exynos Simple PHY driver"
>> + help
>> +   Support for 1-bit PHY controllers on SoCs from Exynos family.
>> +
>>  endmenu
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 2faf78e..88c5b60 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2)   += 
>> phy-exynos4210-usb2.o
>>  obj-$(CONFIG_PHY_EXYNOS4X12_USB2)+= phy-exynos4x12-usb2.o
>>  obj-$(CONFIG_PHY_EXYNOS5250_USB2)+= phy-exynos5250-usb2.o
>>  obj-$(CONFIG_PHY_XGENE)  += phy-xgene.o
>> +obj-$(CONFIG_EXYNOS_SIMPLE_PHY)  += exynos-simple-phy.o
>> diff --git a/drivers/phy/exynos-simple-phy.c 
>> b/drivers/phy/exynos-simple-phy.c
>> new file mode 100644
>> index 000..57ad338
>> --- /dev/null
>> +++ b/drivers/phy/exynos-simple-phy.c
>> @@ -0,0 +1,154 @@
>> +/*
>> + * Exynos Simple PHY driver
>> + *
>> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
>> + * Author: Tomasz Stanislawski 
>> + *
>> + * 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 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define EXYNOS_PHY_ENABLE(1 << 0)
>> +
>> +static int exynos_phy_power_on(struct phy *phy)
>> +{
>> + void __iomem *reg = phy_get_drvdata(phy);
>> + u32 val;
>> +
>> + val = readl(reg);
>> + val |= EXYNOS_PHY_ENABLE;
>> + writel(val, reg);
>> +
>> + return 0;
>> +}
>> +
>> +static int exynos_phy_power_off(struct phy *phy)
>> +{
>> + void __iomem *reg = phy_get_drvdata(phy);
>> + u32 val;
>> +
>> + val = readl(reg);
>> + val &= ~EXYNOS_PHY_ENABLE;
>> + writel(val, reg);
>> +
>> + return 0;
>> +}
>> +
>> +static struct phy_ops exynos_phy_ops = {
>> + .power_on   = exynos_phy_power_on,
>> + .power_off  = exynos_phy_power_off,
>> + .owner  = THIS_MODULE,
>> +};
>> +
>> +static const u32 exynos4210_offsets[] = {
>> + 0x0700, /* HDMI_PHY */
>> + 0x070C, /* DAC_PHY */
>> + 0x0718, /* ADC_PHY */
>> + 0x071C, /* PCIE_PHY */
>> + 0x0720, /* SATA_PHY */
>> + ~0, /* end mark */
>> +};
>> +
>> +static const u32 exynos4412_offsets[] = {
>> + 0x0700, /* HDMI_PHY */
>> + 0x0718, /* ADC_PHY */
>> + ~0, /* end mark */
>> +};
>
> Why have you selected only these registers?
> According to specs Exynos 4210 has 9 and 4412 has 7 control registers
> with 'phy-enable' functionality.
> I guess MIPI would require little more work as it has also reset bits,
> but it will be still

Re: [PATCH v7 1/3] ARM: EXYNOS: initial board support for exynos5260 SoC

2014-04-09 Thread Tomasz Figa


On 07.04.2014 10:38, Sachin Kamat wrote:

Hi Rahul,

On 3 April 2014 15:30, Rahul Sharma  wrote:

From: Pankaj Dubey 

This patch add basic arch side support for exynos5260 SoC.




@@ -374,6 +374,7 @@ static char const *exynos_dt_compat[] __initconst = {
 "samsung,exynos4412",
 "samsung,exynos5",
 "samsung,exynos5250",
+   "samsung,exynos5260",


I don't think we should be adding this as we do not yet have any soc
specific quirk.
Compatibility to generic string, "samsung,exynos5"  should be sufficient.



Yes. Unless a SoC specific quirk that needs to be handled at such low 
level stage emerges, there is no need to add such compatible string to 
the kernel.


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: [PATCHv2 1/3] phy: Add exynos-simple-phy driver

2014-04-09 Thread Andrzej Hajda

Hi Tomasz,

On 04/08/2014 04:37 PM, Tomasz Stanislawski wrote:
> Add exynos-simple-phy driver to support a single register
> PHY interfaces present on Exynos4 SoC.
> 
> Signed-off-by: Tomasz Stanislawski 
> ---
>  .../devicetree/bindings/phy/samsung-phy.txt|   24 +++
>  drivers/phy/Kconfig|5 +
>  drivers/phy/Makefile   |1 +
>  drivers/phy/exynos-simple-phy.c|  154 
> 
>  4 files changed, 184 insertions(+)
>  create mode 100644 drivers/phy/exynos-simple-phy.c
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt 
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index b422e38..f97c4c3 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -114,3 +114,27 @@ Example:
>   compatible = "samsung,exynos-sataphy-i2c";
>   reg = <0x38>;
>   };
> +
> +Samsung S5P/EXYNOS SoC series SIMPLE PHY
> +-
> +
> +Required properties:
> +- compatible : should be one of the listed compatibles:
> + - "samsung,exynos4210-simple-phy"
> + - "samsung,exynos4412-simple-phy"
> +- reg : offset and length of the register set;
> +- #phy-cells : from the generic phy bindings, must be 1;
> +
> +For "samsung,exynos4210-simple-phy" compatible PHYs the second cell in
> +the PHY specifier identifies the PHY and its meaning is as follows:
> +  0 - HDMI PHY,
> +  1 - DAC PHY,
> +  2 - ADC PHY,
> +  3 - PCIE PHY.
> +  4 - SATA PHY.
> +
> +For "samsung,exynos4412-simple-phy" compatible PHYs the second cell in
> +the PHY specifier identifies the PHY and its meaning is as follows:
> +  0 - HDMI PHY,
> +  1 - ADC PHY,

What about using preprocessor macros?

> +
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 3bb05f1..65ab783 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -166,4 +166,9 @@ config PHY_XGENE
>   help
> This option enables support for APM X-Gene SoC multi-purpose PHY.
>  
> +config EXYNOS_SIMPLE_PHY
> + tristate "Exynos Simple PHY driver"
> + help
> +   Support for 1-bit PHY controllers on SoCs from Exynos family.
> +
>  endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 2faf78e..88c5b60 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2)   += phy-exynos4210-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS4X12_USB2)+= phy-exynos4x12-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS5250_USB2)+= phy-exynos5250-usb2.o
>  obj-$(CONFIG_PHY_XGENE)  += phy-xgene.o
> +obj-$(CONFIG_EXYNOS_SIMPLE_PHY)  += exynos-simple-phy.o
> diff --git a/drivers/phy/exynos-simple-phy.c b/drivers/phy/exynos-simple-phy.c
> new file mode 100644
> index 000..57ad338
> --- /dev/null
> +++ b/drivers/phy/exynos-simple-phy.c
> @@ -0,0 +1,154 @@
> +/*
> + * Exynos Simple PHY driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Author: Tomasz Stanislawski 
> + *
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define EXYNOS_PHY_ENABLE(1 << 0)
> +
> +static int exynos_phy_power_on(struct phy *phy)
> +{
> + void __iomem *reg = phy_get_drvdata(phy);
> + u32 val;
> +
> + val = readl(reg);
> + val |= EXYNOS_PHY_ENABLE;
> + writel(val, reg);
> +
> + return 0;
> +}
> +
> +static int exynos_phy_power_off(struct phy *phy)
> +{
> + void __iomem *reg = phy_get_drvdata(phy);
> + u32 val;
> +
> + val = readl(reg);
> + val &= ~EXYNOS_PHY_ENABLE;
> + writel(val, reg);
> +
> + return 0;
> +}
> +
> +static struct phy_ops exynos_phy_ops = {
> + .power_on   = exynos_phy_power_on,
> + .power_off  = exynos_phy_power_off,
> + .owner  = THIS_MODULE,
> +};
> +
> +static const u32 exynos4210_offsets[] = {
> + 0x0700, /* HDMI_PHY */
> + 0x070C, /* DAC_PHY */
> + 0x0718, /* ADC_PHY */
> + 0x071C, /* PCIE_PHY */
> + 0x0720, /* SATA_PHY */
> + ~0, /* end mark */
> +};
> +
> +static const u32 exynos4412_offsets[] = {
> + 0x0700, /* HDMI_PHY */
> + 0x0718, /* ADC_PHY */
> + ~0, /* end mark */
> +};

Why have you selected only these registers?
According to specs Exynos 4210 has 9 and 4412 has 7 control registers
with 'phy-enable' functionality.
I guess MIPI would require little more work as it has also reset bits,
but it will be still better than separate driver.

> +
> +static const struct of_device_id exynos_phy_of_match[] = {
> + { .compatible = "samsung,exynos4210-simple-phy",
> +   .data = exynos4210_offsets},
> + { .compatible = "sams

44 matches

Mail list logo