[PATCH] ipmi: set si_trydefaults=0 for ARM64

[Tony Camuso]

Port I/O space does not exist in ARM64 and is not mapped. Attempts to
access it on ARM systems cause stack traces and worse.

Signed-off-by: Tony Camuso 
---
 drivers/char/ipmi/ipmi_si_intf.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 615abbf..85dcc86 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3841,6 +3841,11 @@ static int init_ipmi_si(void)
spmi_find_bmc();
 #endif
 
+#ifdef CONFIG_ARM64
+   /* Don't touch port io space */
+   si_trydefaults = 0;
+#endif
+
 #ifdef CONFIG_PARISC
register_parisc_driver(_parisc_driver);
parisc_registered = true;
-- 
1.8.3.1

[PATCH] ipmi: set si_trydefaults=0 for ARM64

[Tony Camuso]

Port I/O space does not exist in ARM64 and is not mapped. Attempts to
access it on ARM systems cause stack traces and worse.

Signed-off-by: Tony Camuso 
---
 drivers/char/ipmi/ipmi_si_intf.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 615abbf..85dcc86 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3841,6 +3841,11 @@ static int init_ipmi_si(void)
spmi_find_bmc();
 #endif
 
+#ifdef CONFIG_ARM64
+   /* Don't touch port io space */
+   si_trydefaults = 0;
+#endif
+
 #ifdef CONFIG_PARISC
register_parisc_driver(_parisc_driver);
parisc_registered = true;
-- 
1.8.3.1

[PATCH v3 09/15] arm64: dts: rockchip: Add soc-ctl-syscon to sdhci for rk3399

[Douglas Anderson]

On rk3399 we'd like to be able to properly set corecfg registers in the
Arasan SDHCI component.  Specify the syscon to enable that.

Signed-off-by: Douglas Anderson 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index a4383f359264..1b57e92e0093 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -220,6 +220,7 @@
compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
reg = <0x0 0xfe33 0x0 0x1>;
interrupts = ;
+   arasan,soc-ctl-syscon = <>;
assigned-clocks = < SCLK_EMMC>;
assigned-clock-rates = <2>;
clocks = < SCLK_EMMC>, < ACLK_EMMC>;
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 09/15] arm64: dts: rockchip: Add soc-ctl-syscon to sdhci for rk3399

[Douglas Anderson]

On rk3399 we'd like to be able to properly set corecfg registers in the
Arasan SDHCI component.  Specify the syscon to enable that.

Signed-off-by: Douglas Anderson 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index a4383f359264..1b57e92e0093 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -220,6 +220,7 @@
compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
reg = <0x0 0xfe33 0x0 0x1>;
interrupts = ;
+   arasan,soc-ctl-syscon = <>;
assigned-clocks = < SCLK_EMMC>;
assigned-clock-rates = <2>;
clocks = < SCLK_EMMC>, < ACLK_EMMC>;
-- 
2.8.0.rc3.226.g39d4020

Re: [v3,1/4] mfd: cros_ec: Add cros_ec_cmd_xfer_status helper

[Javier Martinez Canillas]

Hello Brian,

On 06/20/2016 02:10 PM, Brian Norris wrote:
> Hi,
> 
> On Mon, Jun 20, 2016 at 10:44:55AM -0700, Brian Norris wrote:
>> On Mon, Jun 20, 2016 at 09:46:57AM -0400, Javier Martinez Canillas wrote:
>>> On 06/18/2016 01:09 PM, Guenter Roeck wrote:
 On 06/17/2016 06:08 PM, Brian Norris wrote:
> How do you propose we do that? Do all of the following become EINVAL?
>
>>>
>>> Yes, I would just do that.
>>>
>>> The idea of this helper is to remove duplicated code and AFAICT what most EC
>>> drivers do is something similar to the following:
>>>
>>> ret = cros_ec_cmd_xfer(ec, msg);
>>> if (ret < 0)
>>> return ret;
>>>
>>> if (msg->result != EC_RES_SUCCESS) {
>>> dev_dbg(ec->dev, "EC result %d\n", msg->result);
>>> return -EINVAL;
>>> }
>>>
>>> So in practice what most drivers really care is if the result was successful
>>> or not, I don't see specific EC error handling in the EC drivers.  The real
>>> EC error code is still in the message anyways so drivers that do cares about
>>> the real EC error can look at msg->result instead.
> [...]
>  EC_RES_INVALID_COMMAND

 -EOPNOTSUPP

>  EC_RES_INVALID_PARAM

 -EINVAL or -EBADMSG

>  EC_RES_INVALID_VERSION

 -EPROTO or -EBADR or -EBADE or -EBADRQC or -EPROTOOPT

>  EC_RES_INVALID_HEADER

 -EPROTO or -EBADR or -EBADE

 Doesn't look that bad to me. Also, the raw error could still be logged,
 for example with dev_dbg().

>>>
>>> Yes, I think that adding a dev_dbg() with the real EC error code should
>>> be enough, that's basically what drivers do since they can't propagate
>>> the EC error to higher layers anyways.
>>
>> I'll take a look at adding an error code translation table when I get a
>> chance. Hopefully that doesn't delay the others who are planning to use
>> this API shortly...
> 
> Actually, I had some second thoughts, and others brought similar
> concerns up to me:
> 
> What do we really win by doing the translation? It'll be difficult to do
> a 1:1 translation, and any time the EC adds additional error types,
> we'll have to update the translation. What's more, AFAICT, no one will
> really be looking at the translated error codes now, and will just be
> blindly passing them on. So maybe it makes more sense to just pick a
> single error code and pass that on instead. Possibly just -EPROTO (or
> nominate your favorite) for all EC error results, and if someone cares,


I think I didn't make clear in my previous email but yes, I also think
that a translate table for errors is not a good idea and that just a
single -EINVAL (or another error if is more suitable for all EC errors)
will be enough. I say -EINVAL because that's what you first mentioned
in a previous email and that's what most EC clients drivers are using.

I do believe that EC specific errors shouldn't be returned in a Linux
function though for the reasons I explained before.

> they decode msg->result in their driver, or check the dev_dbg() log?
>

And yes, I also think that a dev_dbg() is a good idea (the more data
we have about an error, the better) and as mentioned the EC error can
always be found in the EC message result field.
 
> Brian
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [v3,1/4] mfd: cros_ec: Add cros_ec_cmd_xfer_status helper

[Javier Martinez Canillas]

Hello Brian,

On 06/20/2016 02:10 PM, Brian Norris wrote:
> Hi,
> 
> On Mon, Jun 20, 2016 at 10:44:55AM -0700, Brian Norris wrote:
>> On Mon, Jun 20, 2016 at 09:46:57AM -0400, Javier Martinez Canillas wrote:
>>> On 06/18/2016 01:09 PM, Guenter Roeck wrote:
 On 06/17/2016 06:08 PM, Brian Norris wrote:
> How do you propose we do that? Do all of the following become EINVAL?
>
>>>
>>> Yes, I would just do that.
>>>
>>> The idea of this helper is to remove duplicated code and AFAICT what most EC
>>> drivers do is something similar to the following:
>>>
>>> ret = cros_ec_cmd_xfer(ec, msg);
>>> if (ret < 0)
>>> return ret;
>>>
>>> if (msg->result != EC_RES_SUCCESS) {
>>> dev_dbg(ec->dev, "EC result %d\n", msg->result);
>>> return -EINVAL;
>>> }
>>>
>>> So in practice what most drivers really care is if the result was successful
>>> or not, I don't see specific EC error handling in the EC drivers.  The real
>>> EC error code is still in the message anyways so drivers that do cares about
>>> the real EC error can look at msg->result instead.
> [...]
>  EC_RES_INVALID_COMMAND

 -EOPNOTSUPP

>  EC_RES_INVALID_PARAM

 -EINVAL or -EBADMSG

>  EC_RES_INVALID_VERSION

 -EPROTO or -EBADR or -EBADE or -EBADRQC or -EPROTOOPT

>  EC_RES_INVALID_HEADER

 -EPROTO or -EBADR or -EBADE

 Doesn't look that bad to me. Also, the raw error could still be logged,
 for example with dev_dbg().

>>>
>>> Yes, I think that adding a dev_dbg() with the real EC error code should
>>> be enough, that's basically what drivers do since they can't propagate
>>> the EC error to higher layers anyways.
>>
>> I'll take a look at adding an error code translation table when I get a
>> chance. Hopefully that doesn't delay the others who are planning to use
>> this API shortly...
> 
> Actually, I had some second thoughts, and others brought similar
> concerns up to me:
> 
> What do we really win by doing the translation? It'll be difficult to do
> a 1:1 translation, and any time the EC adds additional error types,
> we'll have to update the translation. What's more, AFAICT, no one will
> really be looking at the translated error codes now, and will just be
> blindly passing them on. So maybe it makes more sense to just pick a
> single error code and pass that on instead. Possibly just -EPROTO (or
> nominate your favorite) for all EC error results, and if someone cares,


I think I didn't make clear in my previous email but yes, I also think
that a translate table for errors is not a good idea and that just a
single -EINVAL (or another error if is more suitable for all EC errors)
will be enough. I say -EINVAL because that's what you first mentioned
in a previous email and that's what most EC clients drivers are using.

I do believe that EC specific errors shouldn't be returned in a Linux
function though for the reasons I explained before.

> they decode msg->result in their driver, or check the dev_dbg() log?
>

And yes, I also think that a dev_dbg() is a good idea (the more data
we have about an error, the better) and as mentioned the EC error can
always be found in the EC message result field.
 
> Brian
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] torture: use ktime_t consistently

[Paul E. McKenney]

On Mon, Jun 20, 2016 at 05:56:40PM +0200, Arnd Bergmann wrote:
> A recent change accidentally introduced a 64-bit division in torture_shutdown,
> which fails to build on 32-bit architectures:
> 
> kernel/built-in.o: In function `torture_shutdown':
> :(.text+0x4b29a): undefined reference to `__aeabi_uldivmod'
> 
> This converts the function to use ktime_t instead, which also simplifies
> it a little.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: b4aa201e0c7c ("torture: Convert torture_shutdown() to hrtimer")

Thank you, Arnd!

I ended up squashing in the alternative change below before I saw your
email address.  This is currently untested, so I might well end up using
yours instead should breakage ensue.  Given that I haven't done much
with ktime_t, breakage is rather likely.

Thoughts?

Thanx, Paul



commit 303c4c9474e84112ab2474cd383282f5dbfc75c3
Author: Paul E. McKenney 
Date:   Sat Jun 18 07:45:43 2016 -0700

torture: Convert torture_shutdown() to hrtimer

Upcoming changes to the timer wheel introduce significant inaccuracy
and possibly also an ultimate limit on timeout duration.  This is
a problem for the current implementation of torture_shutdown() because
(1) shutdown times are user-specified, and can therefore be quite
long, and (2) the torture scripting will kill a test instance that
runs for more than a few minutes longer than scheduled.  This commit
therefore converts the torture_shutdown() timed waits to an hrtimer.

Signed-off-by: Paul E. McKenney 

diff --git a/kernel/torture.c b/kernel/torture.c
index 75961b3decfe..1c1b5960d27f 100644
--- a/kernel/torture.c
+++ b/kernel/torture.c
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -446,9 +447,9 @@ EXPORT_SYMBOL_GPL(torture_shuffle_cleanup);
  * Variables for auto-shutdown.  This allows "lights out" torture runs
  * to be fully scripted.
  */
-static int shutdown_secs;  /* desired test duration in seconds. */
+static ktime_t shutdown_ms;/* desired test duration in seconds. */
 static struct task_struct *shutdown_task;
-static unsigned long shutdown_time;/* jiffies to system shutdown. */
+static ktime_t shutdown_time;  /* jiffies to system shutdown. */
 static void (*torture_shutdown_hook)(void);
 
 /*
@@ -471,20 +472,21 @@ EXPORT_SYMBOL_GPL(torture_shutdown_absorb);
  */
 static int torture_shutdown(void *arg)
 {
-   long delta;
-   unsigned long jiffies_snap;
+   ktime_t delta;
+   ktime_t ktime_snap;
 
VERBOSE_TOROUT_STRING("torture_shutdown task started");
-   jiffies_snap = jiffies;
-   while (ULONG_CMP_LT(jiffies_snap, shutdown_time) &&
+   ktime_snap = ktime_get();
+   while (ktime_before(ktime_snap, shutdown_time) &&
   !torture_must_stop()) {
-   delta = shutdown_time - jiffies_snap;
+   delta = ktime_sub(shutdown_time, ktime_snap);
if (verbose)
pr_alert("%s" TORTURE_FLAG
-"torture_shutdown task: %lu jiffies 
remaining\n",
-torture_type, delta);
-   schedule_timeout_interruptible(delta);
-   jiffies_snap = jiffies;
+"torture_shutdown task: %llu ms remaining\n",
+torture_type, ktime_to_ms(delta));
+   set_current_state(TASK_INTERRUPTIBLE);
+   schedule_hrtimeout(, HRTIMER_MODE_REL);
+   ktime_snap = ktime_get();
}
if (torture_must_stop()) {
torture_kthread_stopping("torture_shutdown");
@@ -511,10 +513,10 @@ int torture_shutdown_init(int ssecs, void 
(*cleanup)(void))
 {
int ret = 0;
 
-   shutdown_secs = ssecs;
torture_shutdown_hook = cleanup;
-   if (shutdown_secs > 0) {
-   shutdown_time = jiffies + shutdown_secs * HZ;
+   if (ssecs > 0) {
+   shutdown_ms = ms_to_ktime(ssecs * 1000ULL);
+   shutdown_time = ktime_add(ktime_get(), shutdown_ms);
ret = torture_create_kthread(torture_shutdown, NULL,
 shutdown_task);
}

Re: [PATCH 0/5] Input: alps - cleanup

[Pali Rohár]

On Monday 06 June 2016 13:32:31 Hans de Goede wrote:
> Hi,
> 
> On 06-06-16 13:23, Pali Rohár wrote:
> > This patch series cleanup usage of alps_model_data table.
> > 
> > Pali Rohár (5):
> >   Input: alps - move ALPS_PROTO_V6 out of alps_model_data table
> >   Input: alps - move ALPS_PROTO_V4 out of alps_model_data table
> >   Input: alps - move ALPS_PROTO_V1 out of alps_model_data table
> >   Input: alps - warn about unsupported ALPS V9 touchpad
> >   Input: alps - cleanup ALPS_PROTO_V2 detection
> >  
> >  drivers/input/mouse/alps.c |   99
> >  ++--
> >  drivers/input/mouse/alps.h |   10 ++---
> >  2 files changed, 62 insertions(+), 47 deletions(-)
> 
> Series looks good to me:
> 
> Acked-by: Hans de Goede 
> 
> Thanks & Regards,
> 
> Hans

Dmitry, can you take this series? Or is there some issue?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] torture: use ktime_t consistently

[Paul E. McKenney]

On Mon, Jun 20, 2016 at 05:56:40PM +0200, Arnd Bergmann wrote:
> A recent change accidentally introduced a 64-bit division in torture_shutdown,
> which fails to build on 32-bit architectures:
> 
> kernel/built-in.o: In function `torture_shutdown':
> :(.text+0x4b29a): undefined reference to `__aeabi_uldivmod'
> 
> This converts the function to use ktime_t instead, which also simplifies
> it a little.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: b4aa201e0c7c ("torture: Convert torture_shutdown() to hrtimer")

Thank you, Arnd!

I ended up squashing in the alternative change below before I saw your
email address.  This is currently untested, so I might well end up using
yours instead should breakage ensue.  Given that I haven't done much
with ktime_t, breakage is rather likely.

Thoughts?

Thanx, Paul



commit 303c4c9474e84112ab2474cd383282f5dbfc75c3
Author: Paul E. McKenney 
Date:   Sat Jun 18 07:45:43 2016 -0700

torture: Convert torture_shutdown() to hrtimer

Upcoming changes to the timer wheel introduce significant inaccuracy
and possibly also an ultimate limit on timeout duration.  This is
a problem for the current implementation of torture_shutdown() because
(1) shutdown times are user-specified, and can therefore be quite
long, and (2) the torture scripting will kill a test instance that
runs for more than a few minutes longer than scheduled.  This commit
therefore converts the torture_shutdown() timed waits to an hrtimer.

Signed-off-by: Paul E. McKenney 

diff --git a/kernel/torture.c b/kernel/torture.c
index 75961b3decfe..1c1b5960d27f 100644
--- a/kernel/torture.c
+++ b/kernel/torture.c
@@ -43,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -446,9 +447,9 @@ EXPORT_SYMBOL_GPL(torture_shuffle_cleanup);
  * Variables for auto-shutdown.  This allows "lights out" torture runs
  * to be fully scripted.
  */
-static int shutdown_secs;  /* desired test duration in seconds. */
+static ktime_t shutdown_ms;/* desired test duration in seconds. */
 static struct task_struct *shutdown_task;
-static unsigned long shutdown_time;/* jiffies to system shutdown. */
+static ktime_t shutdown_time;  /* jiffies to system shutdown. */
 static void (*torture_shutdown_hook)(void);
 
 /*
@@ -471,20 +472,21 @@ EXPORT_SYMBOL_GPL(torture_shutdown_absorb);
  */
 static int torture_shutdown(void *arg)
 {
-   long delta;
-   unsigned long jiffies_snap;
+   ktime_t delta;
+   ktime_t ktime_snap;
 
VERBOSE_TOROUT_STRING("torture_shutdown task started");
-   jiffies_snap = jiffies;
-   while (ULONG_CMP_LT(jiffies_snap, shutdown_time) &&
+   ktime_snap = ktime_get();
+   while (ktime_before(ktime_snap, shutdown_time) &&
   !torture_must_stop()) {
-   delta = shutdown_time - jiffies_snap;
+   delta = ktime_sub(shutdown_time, ktime_snap);
if (verbose)
pr_alert("%s" TORTURE_FLAG
-"torture_shutdown task: %lu jiffies 
remaining\n",
-torture_type, delta);
-   schedule_timeout_interruptible(delta);
-   jiffies_snap = jiffies;
+"torture_shutdown task: %llu ms remaining\n",
+torture_type, ktime_to_ms(delta));
+   set_current_state(TASK_INTERRUPTIBLE);
+   schedule_hrtimeout(, HRTIMER_MODE_REL);
+   ktime_snap = ktime_get();
}
if (torture_must_stop()) {
torture_kthread_stopping("torture_shutdown");
@@ -511,10 +513,10 @@ int torture_shutdown_init(int ssecs, void 
(*cleanup)(void))
 {
int ret = 0;
 
-   shutdown_secs = ssecs;
torture_shutdown_hook = cleanup;
-   if (shutdown_secs > 0) {
-   shutdown_time = jiffies + shutdown_secs * HZ;
+   if (ssecs > 0) {
+   shutdown_ms = ms_to_ktime(ssecs * 1000ULL);
+   shutdown_time = ktime_add(ktime_get(), shutdown_ms);
ret = torture_create_kthread(torture_shutdown, NULL,
 shutdown_task);
}

Re: [PATCH 0/5] Input: alps - cleanup

[Pali Rohár]

On Monday 06 June 2016 13:32:31 Hans de Goede wrote:
> Hi,
> 
> On 06-06-16 13:23, Pali Rohár wrote:
> > This patch series cleanup usage of alps_model_data table.
> > 
> > Pali Rohár (5):
> >   Input: alps - move ALPS_PROTO_V6 out of alps_model_data table
> >   Input: alps - move ALPS_PROTO_V4 out of alps_model_data table
> >   Input: alps - move ALPS_PROTO_V1 out of alps_model_data table
> >   Input: alps - warn about unsupported ALPS V9 touchpad
> >   Input: alps - cleanup ALPS_PROTO_V2 detection
> >  
> >  drivers/input/mouse/alps.c |   99
> >  ++--
> >  drivers/input/mouse/alps.h |   10 ++---
> >  2 files changed, 62 insertions(+), 47 deletions(-)
> 
> Series looks good to me:
> 
> Acked-by: Hans de Goede 
> 
> Thanks & Regards,
> 
> Hans

Dmitry, can you take this series? Or is there some issue?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH 0/6] Support DAX for device-mapper dm-linear devices

[Mike Snitzer]

On Mon, Jun 13 2016 at  6:57pm -0400,
Mike Snitzer  wrote:

> On Mon, Jun 13 2016 at  6:21pm -0400,
> Toshi Kani  wrote:
> 
> > This patch-set adds DAX support to device-mapper dm-linear devices
> > used by LVM.  It works with LVM commands as follows:
> >  - Creation of a logical volume with all DAX capable devices (such
> >as pmem) sets the logical volume DAX capable as well.
> >  - Once a logical volume is set to DAX capable, the volume may not
> >be extended with non-DAX capable devices.
> > 
> > The direct_access interface is added to dm and dm-linear to map
> > a request to a target device.
> > 
> >  - Patch 1-2 introduce GENHD_FL_DAX flag to indicate DAX capability.
> >  - Patch 3-4 add direct_access functions to dm and dm-linear.
> >  - Patch 5-6 set GENHD_FL_DAX to dm when all targets are DAX capable.
> > 
> > ---
> > Toshi Kani (6):
> >  1/6 genhd: Add GENHD_FL_DAX to gendisk flags
> >  2/6 block: Check GENHD_FL_DAX for DAX capability
> >  3/6 dm: Add dm_blk_direct_access() for mapped device
> >  4/6 dm-linear: Add linear_direct_access()
> >  5/6 dm, dm-linear: Add dax_supported to dm_target
> >  6/6 dm: Enable DAX support for mapper device
> 
> Thanks a lot for doing this.  I recently added it to my TODO so your
> patches come at a great time.
> 
> I'll try to get to reviewing/testing your work by the end of this week.

I rebased your patches on linux-dm.git's 'for-next' (which includes what
I've already staged for the 4.8 merge window).  And I folded/changed
some of the DM patches so that there are only 2 now (1 for DM core and 1
for dm-linear).  Please see the 4 topmost commits in my 'wip' here:

http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

Feel free to pick these patches up to use as the basis for continued
work or re-posting of this set.. either that or I could post them as v2
on your behalf.

As for testing, I've verified that basic IO works to a pmem-based DM
linear device and that mixed table types are rejected as expected.

Mike

Re: [PATCH 0/6] Support DAX for device-mapper dm-linear devices

[Mike Snitzer]

On Mon, Jun 13 2016 at  6:57pm -0400,
Mike Snitzer  wrote:

> On Mon, Jun 13 2016 at  6:21pm -0400,
> Toshi Kani  wrote:
> 
> > This patch-set adds DAX support to device-mapper dm-linear devices
> > used by LVM.  It works with LVM commands as follows:
> >  - Creation of a logical volume with all DAX capable devices (such
> >as pmem) sets the logical volume DAX capable as well.
> >  - Once a logical volume is set to DAX capable, the volume may not
> >be extended with non-DAX capable devices.
> > 
> > The direct_access interface is added to dm and dm-linear to map
> > a request to a target device.
> > 
> >  - Patch 1-2 introduce GENHD_FL_DAX flag to indicate DAX capability.
> >  - Patch 3-4 add direct_access functions to dm and dm-linear.
> >  - Patch 5-6 set GENHD_FL_DAX to dm when all targets are DAX capable.
> > 
> > ---
> > Toshi Kani (6):
> >  1/6 genhd: Add GENHD_FL_DAX to gendisk flags
> >  2/6 block: Check GENHD_FL_DAX for DAX capability
> >  3/6 dm: Add dm_blk_direct_access() for mapped device
> >  4/6 dm-linear: Add linear_direct_access()
> >  5/6 dm, dm-linear: Add dax_supported to dm_target
> >  6/6 dm: Enable DAX support for mapper device
> 
> Thanks a lot for doing this.  I recently added it to my TODO so your
> patches come at a great time.
> 
> I'll try to get to reviewing/testing your work by the end of this week.

I rebased your patches on linux-dm.git's 'for-next' (which includes what
I've already staged for the 4.8 merge window).  And I folded/changed
some of the DM patches so that there are only 2 now (1 for DM core and 1
for dm-linear).  Please see the 4 topmost commits in my 'wip' here:

http://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=wip

Feel free to pick these patches up to use as the basis for continued
work or re-posting of this set.. either that or I could post them as v2
on your behalf.

As for testing, I've verified that basic IO works to a pmem-based DM
linear device and that mixed table types are rejected as expected.

Mike

Re: [PATCH v3 0/15] Changes to support 150 MHz eMMC on rk3399

[Heiko Stübner]

Am Montag, 20. Juni 2016, 10:56:39 schrieb Douglas Anderson:
> The theme of this series of patches is to try to allow running the eMMC
> at 150 MHz on the rk3399 SoC, though the changes should still be correct
> and have merit on their own.  The motivation for running at 150 MHz is
> that doing so improves signal integrity and (with some eMMC devices)
> doesn't affect throughput.
> 
> These patches have been structured to keep things as separate as
> possible, but nevertheless there are still some dependencies between
> patches.  It probably makes the most sense for all of the non-device
> tree patches to go through a single tree.  PHY patches now have Acks
> from Kishon so that means things should be clear for all non-DTS patches
> to go through the MMC tree if Ulf agrees.  Device tree patches should be
> able to be landed separately and the worst what would happen is a
> warning in the kernel log if you have the code without the device tree.
> 
> The code patches are based on Ulf's mmc-next.  In v3 of the series I've
> pulled in the latest version of a previous PHY series posted by Brian
> Norris.
> 
> The device tree patches are based on Heiko's v4.8-armsoc/dts64.

just so it doesn't get lost from v2->v3, I plan on picking up the dts patches 
9 and 15 after the rest landed in the mmc tree - as they will only apply on 
top of my general dts branch.


Heiko

Re: [PATCH v3 0/15] Changes to support 150 MHz eMMC on rk3399

[Heiko Stübner]

Am Montag, 20. Juni 2016, 10:56:39 schrieb Douglas Anderson:
> The theme of this series of patches is to try to allow running the eMMC
> at 150 MHz on the rk3399 SoC, though the changes should still be correct
> and have merit on their own.  The motivation for running at 150 MHz is
> that doing so improves signal integrity and (with some eMMC devices)
> doesn't affect throughput.
> 
> These patches have been structured to keep things as separate as
> possible, but nevertheless there are still some dependencies between
> patches.  It probably makes the most sense for all of the non-device
> tree patches to go through a single tree.  PHY patches now have Acks
> from Kishon so that means things should be clear for all non-DTS patches
> to go through the MMC tree if Ulf agrees.  Device tree patches should be
> able to be landed separately and the worst what would happen is a
> warning in the kernel log if you have the code without the device tree.
> 
> The code patches are based on Ulf's mmc-next.  In v3 of the series I've
> pulled in the latest version of a previous PHY series posted by Brian
> Norris.
> 
> The device tree patches are based on Heiko's v4.8-armsoc/dts64.

just so it doesn't get lost from v2->v3, I plan on picking up the dts patches 
9 and 15 after the rest landed in the mmc tree - as they will only apply on 
top of my general dts branch.


Heiko

Re: [PATCH 2/2] perf record: Add --dry-run option to check cmdline options

[David Ahern]

On 6/20/16 12:13 PM, Arnaldo Carvalho de Melo wrote:

'perf cc' seems sensible, and has the added bonus of being one letter
shorter :-)



perf is now a general front-end to a compiler?

Re: [PATCH 2/2] perf record: Add --dry-run option to check cmdline options

[David Ahern]

On 6/20/16 12:13 PM, Arnaldo Carvalho de Melo wrote:

'perf cc' seems sensible, and has the added bonus of being one letter
shorter :-)



perf is now a general front-end to a compiler?

Re: [PATCH v3 14/15] phy: rockchip-emmc: Set phyctrl_frqsel based on card clock

[Heiko Stübner]

Am Montag, 20. Juni 2016, 10:56:53 schrieb Douglas Anderson:
> The "phyctrl_frqsel" is described in the Arasan datasheet [1] as "the
> frequency range of DLL operation".  Although the Rockchip variant of
> this PHY has different ranges than the reference Arasan PHY it appears
> as if the functionality is similar.  We should set this phyctrl field
> properly.
> 
> Note: as per Rockchip engineers, apparently the "phyctrl_frqsel" is
> actually only useful in HS200 / HS400 modes even though the DLL itself
> it used for some purposes in all modes.  See the discussion in the
> earlier change in this series: ("mmc: sdhci-of-arasan: Always power the
> PHY off/on when clock changes").  In any case, it shouldn't hurt to set
> this always.
> 
> Note that this change should allow boards to run at HS200 / HS400 speed
> modes while running at 100 MHz or 150 MHz.  In fact, running HS400 at
> 150 MHz (giving 300 MB/s) is the main motivation of this series, since
> performance is still good but signal integrity problems are less
> prevelant at 150 MHz.
> 
> [1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf
> 
> Signed-off-by: Douglas Anderson 
> Acked-by: Kishon Vijay Abraham I 

> ---
> Changes in v3:
> - Use phy_init / phy_exit (Heiko)

Reviewed-by: Heiko Stuebner 

Re: [PATCH v3 14/15] phy: rockchip-emmc: Set phyctrl_frqsel based on card clock

[Heiko Stübner]

Am Montag, 20. Juni 2016, 10:56:53 schrieb Douglas Anderson:
> The "phyctrl_frqsel" is described in the Arasan datasheet [1] as "the
> frequency range of DLL operation".  Although the Rockchip variant of
> this PHY has different ranges than the reference Arasan PHY it appears
> as if the functionality is similar.  We should set this phyctrl field
> properly.
> 
> Note: as per Rockchip engineers, apparently the "phyctrl_frqsel" is
> actually only useful in HS200 / HS400 modes even though the DLL itself
> it used for some purposes in all modes.  See the discussion in the
> earlier change in this series: ("mmc: sdhci-of-arasan: Always power the
> PHY off/on when clock changes").  In any case, it shouldn't hurt to set
> this always.
> 
> Note that this change should allow boards to run at HS200 / HS400 speed
> modes while running at 100 MHz or 150 MHz.  In fact, running HS400 at
> 150 MHz (giving 300 MB/s) is the main motivation of this series, since
> performance is still good but signal integrity problems are less
> prevelant at 150 MHz.
> 
> [1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf
> 
> Signed-off-by: Douglas Anderson 
> Acked-by: Kishon Vijay Abraham I 

> ---
> Changes in v3:
> - Use phy_init / phy_exit (Heiko)

Reviewed-by: Heiko Stuebner 

Re: [PATCH 2/2] perf record: Add --dry-run option to check cmdline options

[Arnaldo Carvalho de Melo]

Em Mon, Jun 20, 2016 at 09:22:11AM -0700, Alexei Starovoitov escreveu:
> On Mon, Jun 20, 2016 at 11:38:18AM -0300, Arnaldo Carvalho de Melo wrote:
> > Doing:

> > perf bcc -c foo.c

> > Looks so much simpler and similar to an existing compile source code
> > into object file workflow (gcc's, any C compiler) that I think it would
> > fit in the workflow being discussed really nicely.
 
> I'm hopeful that eventually we'll be able merge iovisor/bcc project
> with perf, so would be good to reserve 'perf bcc' command for that
> future use. Also picking a different name for compiling would be less
> confusing to users who already familiar with bcc. Instead we can use:
> perf bpfcc foo.c -o foo.o
> perf cc foo.c

'perf cc' seems sensible, and has the added bonus of being one letter
shorter :-)

- Arnaldo

> perf compile foo.c

Re: [PATCH 2/2] perf record: Add --dry-run option to check cmdline options

[Arnaldo Carvalho de Melo]

Em Mon, Jun 20, 2016 at 09:22:11AM -0700, Alexei Starovoitov escreveu:
> On Mon, Jun 20, 2016 at 11:38:18AM -0300, Arnaldo Carvalho de Melo wrote:
> > Doing:

> > perf bcc -c foo.c

> > Looks so much simpler and similar to an existing compile source code
> > into object file workflow (gcc's, any C compiler) that I think it would
> > fit in the workflow being discussed really nicely.
 
> I'm hopeful that eventually we'll be able merge iovisor/bcc project
> with perf, so would be good to reserve 'perf bcc' command for that
> future use. Also picking a different name for compiling would be less
> confusing to users who already familiar with bcc. Instead we can use:
> perf bpfcc foo.c -o foo.o
> perf cc foo.c

'perf cc' seems sensible, and has the added bonus of being one letter
shorter :-)

- Arnaldo

> perf compile foo.c

Re: [PATCH v2 1/3] Documentation: DT: Add iproc-static-adc binding

[Rob Herring]

On Sun, Jun 19, 2016 at 03:36:28PM +0530, Raveendra Padasalagi wrote:
> The patch adds devicetree binding document for broadcom's
> iproc-static-adc controller driver.
> 
> Signed-off-by: Raveendra Padasalagi 
> Reviewed-by: Ray Jui 
> Reviewed-by: Scott Branden 
> ---
>  .../bindings/iio/adc/brcm,iproc-static-adc.txt | 43 
> ++
>  1 file changed, 43 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/iio/adc/brcm,iproc-static-adc.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/iio/adc/brcm,iproc-static-adc.txt 
> b/Documentation/devicetree/bindings/iio/adc/brcm,iproc-static-adc.txt
> new file mode 100644
> index 000..1de0dfa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/brcm,iproc-static-adc.txt
> @@ -0,0 +1,43 @@
> +* Broadcom's IPROC Static ADC controller
> +
> +Required properties:
> +
> +- compatible: Must be "brcm,iproc-static-adc"
> +

> +- #address-cells: Specify the number of u32 entries needed in child nodes.
> +  Should set to 1.
> +
> +- #size-cells: Specify number of u32 entries needed to specify child nodes 
> size
> +  in reg property. Should set to 1.

Why are these needed? What child nodes can you have?

> +
> +- adc-syscon: Handler of syscon node defining physical base address of the
> +  controller and length of memory mapped region.
> +
> +- #io-channel-cells = <1>; As ADC has multiple outputs
> +  refer to Documentation/devicetree/bindings/iio/iio-bindings.txt for 
> details.
> +
> +- clocks: Clock used for this block.
> +
> +- clock-names: Clock name should be given as tsc_clk.
> +
> +- interrupts: interrupt line number.
> +
> +For example:
> +
> + ts_adc_syscon: ts_adc_syscon@180a6000 {
> + compatible = "brcm,iproc-ts-adc-syscon","syscon";
> + reg = <0x180a6000 0xc30>;
> + };
> +
> + adc: adc@180a6000 {
> + compatible = "brcm,iproc-static-adc";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + adc-syscon = <_adc_syscon>;
> + #io-channel-cells = <1>;
> + io-channel-ranges;
> + clocks = <_clks BCM_CYGNUS_ASIU_ADC_CLK>;
> + clock-names = "tsc_clk";
> + interrupts = ;
> + status = "disabled";
> + };
> -- 
> 1.9.1
> 

Re: [PATCH v2 1/3] Documentation: DT: Add iproc-static-adc binding

[Rob Herring]

On Sun, Jun 19, 2016 at 03:36:28PM +0530, Raveendra Padasalagi wrote:
> The patch adds devicetree binding document for broadcom's
> iproc-static-adc controller driver.
> 
> Signed-off-by: Raveendra Padasalagi 
> Reviewed-by: Ray Jui 
> Reviewed-by: Scott Branden 
> ---
>  .../bindings/iio/adc/brcm,iproc-static-adc.txt | 43 
> ++
>  1 file changed, 43 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/iio/adc/brcm,iproc-static-adc.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/iio/adc/brcm,iproc-static-adc.txt 
> b/Documentation/devicetree/bindings/iio/adc/brcm,iproc-static-adc.txt
> new file mode 100644
> index 000..1de0dfa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/brcm,iproc-static-adc.txt
> @@ -0,0 +1,43 @@
> +* Broadcom's IPROC Static ADC controller
> +
> +Required properties:
> +
> +- compatible: Must be "brcm,iproc-static-adc"
> +

> +- #address-cells: Specify the number of u32 entries needed in child nodes.
> +  Should set to 1.
> +
> +- #size-cells: Specify number of u32 entries needed to specify child nodes 
> size
> +  in reg property. Should set to 1.

Why are these needed? What child nodes can you have?

> +
> +- adc-syscon: Handler of syscon node defining physical base address of the
> +  controller and length of memory mapped region.
> +
> +- #io-channel-cells = <1>; As ADC has multiple outputs
> +  refer to Documentation/devicetree/bindings/iio/iio-bindings.txt for 
> details.
> +
> +- clocks: Clock used for this block.
> +
> +- clock-names: Clock name should be given as tsc_clk.
> +
> +- interrupts: interrupt line number.
> +
> +For example:
> +
> + ts_adc_syscon: ts_adc_syscon@180a6000 {
> + compatible = "brcm,iproc-ts-adc-syscon","syscon";
> + reg = <0x180a6000 0xc30>;
> + };
> +
> + adc: adc@180a6000 {
> + compatible = "brcm,iproc-static-adc";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + adc-syscon = <_adc_syscon>;
> + #io-channel-cells = <1>;
> + io-channel-ranges;
> + clocks = <_clks BCM_CYGNUS_ASIU_ADC_CLK>;
> + clock-names = "tsc_clk";
> + interrupts = ;
> + status = "disabled";
> + };
> -- 
> 1.9.1
> 

[PATCH v3 14/15] phy: rockchip-emmc: Set phyctrl_frqsel based on card clock

[Douglas Anderson]

The "phyctrl_frqsel" is described in the Arasan datasheet [1] as "the
frequency range of DLL operation".  Although the Rockchip variant of
this PHY has different ranges than the reference Arasan PHY it appears
as if the functionality is similar.  We should set this phyctrl field
properly.

Note: as per Rockchip engineers, apparently the "phyctrl_frqsel" is
actually only useful in HS200 / HS400 modes even though the DLL itself
it used for some purposes in all modes.  See the discussion in the
earlier change in this series: ("mmc: sdhci-of-arasan: Always power the
PHY off/on when clock changes").  In any case, it shouldn't hurt to set
this always.

Note that this change should allow boards to run at HS200 / HS400 speed
modes while running at 100 MHz or 150 MHz.  In fact, running HS400 at
150 MHz (giving 300 MB/s) is the main motivation of this series, since
performance is still good but signal integrity problems are less
prevelant at 150 MHz.

[1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf

Signed-off-by: Douglas Anderson 
Acked-by: Kishon Vijay Abraham I 
---
Changes in v3:
- Use phy_init / phy_exit (Heiko)
- Add Kishon's Ack

Changes in v2:
- Warn if we're more than 15 MHz from ideal rate (Shawn)
- Move code cleanup before set phyctrl_frqsel based on card clock (Shawn)
- Fix typo USB => SDHCI (Shawn)

 drivers/phy/phy-rockchip-emmc.c | 101 +++-
 1 file changed, 89 insertions(+), 12 deletions(-)

diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
index 23fe50864526..9dce958233a0 100644
--- a/drivers/phy/phy-rockchip-emmc.c
+++ b/drivers/phy/phy-rockchip-emmc.c
@@ -14,6 +14,7 @@
  * GNU General Public License for more details.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -78,15 +79,53 @@
 struct rockchip_emmc_phy {
unsigned intreg_offset;
struct regmap   *reg_base;
+   struct clk  *emmcclk;
 };
 
-static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,
-  bool on_off)
+static int rockchip_emmc_phy_power(struct phy *phy, bool on_off)
 {
+   struct rockchip_emmc_phy *rk_phy = phy_get_drvdata(phy);
unsigned int caldone;
unsigned int dllrdy;
+   unsigned int freqsel = PHYCTRL_FREQSEL_200M;
unsigned long timeout;
 
+   if (rk_phy->emmcclk != NULL) {
+   unsigned long rate = clk_get_rate(rk_phy->emmcclk);
+   unsigned long ideal_rate;
+   unsigned long diff;
+
+   switch (rate) {
+   case 0 ... 7499:
+   ideal_rate = 5000;
+   freqsel = PHYCTRL_FREQSEL_50M;
+   break;
+   case 7500 ... 12499:
+   ideal_rate = 1;
+   freqsel = PHYCTRL_FREQSEL_100M;
+   break;
+   case 12500 ... 17499:
+   ideal_rate = 15000;
+   freqsel = PHYCTRL_FREQSEL_150M;
+   break;
+   default:
+   ideal_rate = 2;
+   break;
+   };
+
+   diff = (rate > ideal_rate) ?
+   rate - ideal_rate : ideal_rate - rate;
+
+   /*
+* In order for tuning delays to be accurate we need to be
+* pretty spot on for the DLL range, so warn if we're too
+* far off.  Also warn if we're above the 200 MHz max.  Don't
+* warn for really slow rates since we won't be tuning then.
+*/
+   if ((rate > 5000 && diff > 1500) || (rate > 2))
+   dev_warn(>dev, "Unsupported rate: %lu\n", rate);
+   }
+
/*
 * Keep phyctrl_pdb and phyctrl_endll low to allow
 * initialization of CALIO state M/C DFFs
@@ -132,6 +171,13 @@ static int rockchip_emmc_phy_power(struct 
rockchip_emmc_phy *rk_phy,
return -ETIMEDOUT;
}
 
+   /* Set the frequency of the DLL operation */
+   regmap_write(rk_phy->reg_base,
+rk_phy->reg_offset + GRF_EMMCPHY_CON0,
+HIWORD_UPDATE(freqsel, PHYCTRL_FREQSEL_MASK,
+  PHYCTRL_FREQSEL_SHIFT));
+
+   /* Turn on the DLL */
regmap_write(rk_phy->reg_base,
 rk_phy->reg_offset + GRF_EMMCPHY_CON6,
 HIWORD_UPDATE(PHYCTRL_ENDLL_ENABLE,
@@ -166,25 +212,54 @@ static int rockchip_emmc_phy_power(struct 
rockchip_emmc_phy *rk_phy,
return 0;
 }
 
-static int rockchip_emmc_phy_power_off(struct phy *phy)
+static int rockchip_emmc_phy_init(struct phy *phy)
+{
+   struct rockchip_emmc_phy *rk_phy = phy_get_drvdata(phy);
+   int ret = 0;
+
+   /*
+* We purposely get the clock here and not in probe to 

Re: Stable -rc git trees and email headers

[Kevin Hilman]

Greg KH  writes:

> Hi,
>
> I've finally gotten off my butt and made my quilt trees of patches into
> a "semi-proper" git tree to make it easier for people to test them.
>
> I'm now pushing the patches I accept into the stable queues into the git
> tree here:
>   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
>
> I'll push them out in chunks, as I accept them, and when I do a "real"
> -rc release.  Right now the HEAD of each branch will have a -rc1 tag on
> them, just because it makes it a bit easier on my end.
>
> Note these branches WILL get rebased.  All the time.  So don't count on
> them to contain anything that will stick around, UNTIL a real stable
> release happens.
>
> Guenter, I hope this helps with your testing, is there anything that I
> can do here to make it easier for you?
>
> Kevin, hopefully this tree also helps you.  I will also be adding some
> email headers to my announcements of the -rc releases that you can
> parse.  Here is what they are going to look like for an example release:
>
> X-KernelTest-Patch: 
> http://kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.14-rc1.gz
> X-KernelTest-Tree: 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> X-KernelTest-Branch: linux-4.4.y
> X-KernelTest-Patches: 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git
> X-KernelTest-Version: 4.4.14-rc1
> X-KernelTest-Deadline: 2016-06-20T02:57+00:00
>
> Will those work out?

Yes, those look great.  We'll start swtiching the kernelci.org bot over
to this tree and let you know if there's anything else we need.

> Fengguang, can you add these to the 0-day bot?  The branches to watch
> are all of the currently-active stable trees.  Right now that would be
> "linux-3.14.y", "linux-4.4.y", and "linux-4.6.y".  Do I need to tell you
> about future branches, or can you just pick up new ones when they show
> up?
>
> And can I get announcements if/when the tests pass (or fail) so I know
> all is ok?
>
> Hope this helps people out,

Yes, it does.  Thanks!

Kevin

[PATCH v3 14/15] phy: rockchip-emmc: Set phyctrl_frqsel based on card clock

[Douglas Anderson]

The "phyctrl_frqsel" is described in the Arasan datasheet [1] as "the
frequency range of DLL operation".  Although the Rockchip variant of
this PHY has different ranges than the reference Arasan PHY it appears
as if the functionality is similar.  We should set this phyctrl field
properly.

Note: as per Rockchip engineers, apparently the "phyctrl_frqsel" is
actually only useful in HS200 / HS400 modes even though the DLL itself
it used for some purposes in all modes.  See the discussion in the
earlier change in this series: ("mmc: sdhci-of-arasan: Always power the
PHY off/on when clock changes").  In any case, it shouldn't hurt to set
this always.

Note that this change should allow boards to run at HS200 / HS400 speed
modes while running at 100 MHz or 150 MHz.  In fact, running HS400 at
150 MHz (giving 300 MB/s) is the main motivation of this series, since
performance is still good but signal integrity problems are less
prevelant at 150 MHz.

[1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf

Signed-off-by: Douglas Anderson 
Acked-by: Kishon Vijay Abraham I 
---
Changes in v3:
- Use phy_init / phy_exit (Heiko)
- Add Kishon's Ack

Changes in v2:
- Warn if we're more than 15 MHz from ideal rate (Shawn)
- Move code cleanup before set phyctrl_frqsel based on card clock (Shawn)
- Fix typo USB => SDHCI (Shawn)

 drivers/phy/phy-rockchip-emmc.c | 101 +++-
 1 file changed, 89 insertions(+), 12 deletions(-)

diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
index 23fe50864526..9dce958233a0 100644
--- a/drivers/phy/phy-rockchip-emmc.c
+++ b/drivers/phy/phy-rockchip-emmc.c
@@ -14,6 +14,7 @@
  * GNU General Public License for more details.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -78,15 +79,53 @@
 struct rockchip_emmc_phy {
unsigned intreg_offset;
struct regmap   *reg_base;
+   struct clk  *emmcclk;
 };
 
-static int rockchip_emmc_phy_power(struct rockchip_emmc_phy *rk_phy,
-  bool on_off)
+static int rockchip_emmc_phy_power(struct phy *phy, bool on_off)
 {
+   struct rockchip_emmc_phy *rk_phy = phy_get_drvdata(phy);
unsigned int caldone;
unsigned int dllrdy;
+   unsigned int freqsel = PHYCTRL_FREQSEL_200M;
unsigned long timeout;
 
+   if (rk_phy->emmcclk != NULL) {
+   unsigned long rate = clk_get_rate(rk_phy->emmcclk);
+   unsigned long ideal_rate;
+   unsigned long diff;
+
+   switch (rate) {
+   case 0 ... 7499:
+   ideal_rate = 5000;
+   freqsel = PHYCTRL_FREQSEL_50M;
+   break;
+   case 7500 ... 12499:
+   ideal_rate = 1;
+   freqsel = PHYCTRL_FREQSEL_100M;
+   break;
+   case 12500 ... 17499:
+   ideal_rate = 15000;
+   freqsel = PHYCTRL_FREQSEL_150M;
+   break;
+   default:
+   ideal_rate = 2;
+   break;
+   };
+
+   diff = (rate > ideal_rate) ?
+   rate - ideal_rate : ideal_rate - rate;
+
+   /*
+* In order for tuning delays to be accurate we need to be
+* pretty spot on for the DLL range, so warn if we're too
+* far off.  Also warn if we're above the 200 MHz max.  Don't
+* warn for really slow rates since we won't be tuning then.
+*/
+   if ((rate > 5000 && diff > 1500) || (rate > 2))
+   dev_warn(>dev, "Unsupported rate: %lu\n", rate);
+   }
+
/*
 * Keep phyctrl_pdb and phyctrl_endll low to allow
 * initialization of CALIO state M/C DFFs
@@ -132,6 +171,13 @@ static int rockchip_emmc_phy_power(struct 
rockchip_emmc_phy *rk_phy,
return -ETIMEDOUT;
}
 
+   /* Set the frequency of the DLL operation */
+   regmap_write(rk_phy->reg_base,
+rk_phy->reg_offset + GRF_EMMCPHY_CON0,
+HIWORD_UPDATE(freqsel, PHYCTRL_FREQSEL_MASK,
+  PHYCTRL_FREQSEL_SHIFT));
+
+   /* Turn on the DLL */
regmap_write(rk_phy->reg_base,
 rk_phy->reg_offset + GRF_EMMCPHY_CON6,
 HIWORD_UPDATE(PHYCTRL_ENDLL_ENABLE,
@@ -166,25 +212,54 @@ static int rockchip_emmc_phy_power(struct 
rockchip_emmc_phy *rk_phy,
return 0;
 }
 
-static int rockchip_emmc_phy_power_off(struct phy *phy)
+static int rockchip_emmc_phy_init(struct phy *phy)
+{
+   struct rockchip_emmc_phy *rk_phy = phy_get_drvdata(phy);
+   int ret = 0;
+
+   /*
+* We purposely get the clock here and not in probe to avoid the
+* circular 

Re: Stable -rc git trees and email headers

[Kevin Hilman]

Greg KH  writes:

> Hi,
>
> I've finally gotten off my butt and made my quilt trees of patches into
> a "semi-proper" git tree to make it easier for people to test them.
>
> I'm now pushing the patches I accept into the stable queues into the git
> tree here:
>   git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
>
> I'll push them out in chunks, as I accept them, and when I do a "real"
> -rc release.  Right now the HEAD of each branch will have a -rc1 tag on
> them, just because it makes it a bit easier on my end.
>
> Note these branches WILL get rebased.  All the time.  So don't count on
> them to contain anything that will stick around, UNTIL a real stable
> release happens.
>
> Guenter, I hope this helps with your testing, is there anything that I
> can do here to make it easier for you?
>
> Kevin, hopefully this tree also helps you.  I will also be adding some
> email headers to my announcements of the -rc releases that you can
> parse.  Here is what they are going to look like for an example release:
>
> X-KernelTest-Patch: 
> http://kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.14-rc1.gz
> X-KernelTest-Tree: 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
> X-KernelTest-Branch: linux-4.4.y
> X-KernelTest-Patches: 
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git
> X-KernelTest-Version: 4.4.14-rc1
> X-KernelTest-Deadline: 2016-06-20T02:57+00:00
>
> Will those work out?

Yes, those look great.  We'll start swtiching the kernelci.org bot over
to this tree and let you know if there's anything else we need.

> Fengguang, can you add these to the 0-day bot?  The branches to watch
> are all of the currently-active stable trees.  Right now that would be
> "linux-3.14.y", "linux-4.4.y", and "linux-4.6.y".  Do I need to tell you
> about future branches, or can you just pick up new ones when they show
> up?
>
> And can I get announcements if/when the tests pass (or fail) so I know
> all is ok?
>
> Hope this helps people out,

Yes, it does.  Thanks!

Kevin

[PATCH v3 08/15] mmc: sdhci-of-arasan: Properly set corecfg_baseclkfreq on rk3399

[Douglas Anderson]

In the the earlier change in this series ("Documentation: mmc:
sdhci-of-arasan: Add soc-ctl-syscon for corecfg regs") we can see the
mechansim for specifying a syscon to properly set corecfg registers in
sdhci-of-arasan.  Now let's use this mechanism to properly set
corecfg_baseclkfreq on rk3399.

>From [1] the corecfg_baseclkfreq is supposed to be set to:
  Base Clock Frequency for SD Clock.
  This is the frequency of the xin_clk.

This is a relatively easy thing to do.  Note that we assume that xin_clk
is not dynamic and we can check the clock at probe time.  If any real
devices have a dynamic xin_clk future patches could register for
notifiers for the clock.

At the moment, setting corecfg_baseclkfreq is only supported for rk3399
since we need a specific map for each implementation.  The code is
written in a generic way that should make this easy to extend to other
SoCs.  Note that a specific compatible string for rk3399 is already in
use and so we add that to the table to match rk3399.

[1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf

Signed-off-by: Douglas Anderson 
Reviewed-by: Heiko Stuebner 
Reviewed-by: Shawn Lin 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2:
- Reorder includes (Shawn)

 drivers/mmc/host/sdhci-of-arasan.c | 189 ++---
 1 file changed, 178 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 3ff1711077c2..1286fe8905dc 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -19,9 +19,11 @@
  * your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include "sdhci-pltfm.h"
 
 #define SDHCI_ARASAN_CLK_CTRL_OFFSET   0x2c
@@ -32,18 +34,115 @@
 #define CLK_CTRL_TIMEOUT_MASK  (0xf << CLK_CTRL_TIMEOUT_SHIFT)
 #define CLK_CTRL_TIMEOUT_MIN_EXP   13
 
+/*
+ * On some SoCs the syscon area has a feature where the upper 16-bits of
+ * each 32-bit register act as a write mask for the lower 16-bits.  This allows
+ * atomic updates of the register without locking.  This macro is used on SoCs
+ * that have that feature.
+ */
+#define HIWORD_UPDATE(val, mask, shift) \
+   ((val) << (shift) | (mask) << ((shift) + 16))
+
+/**
+ * struct sdhci_arasan_soc_ctl_field - Field used in sdhci_arasan_soc_ctl_map
+ *
+ * @reg:   Offset within the syscon of the register containing this field
+ * @width: Number of bits for this field
+ * @shift: Bit offset within @reg of this field (or -1 if not avail)
+ */
+struct sdhci_arasan_soc_ctl_field {
+   u32 reg;
+   u16 width;
+   s16 shift;
+};
+
+/**
+ * struct sdhci_arasan_soc_ctl_map - Map in syscon to corecfg registers
+ *
+ * It's up to the licensee of the Arsan IP block to make these available
+ * somewhere if needed.  Presumably these will be scattered somewhere that's
+ * accessible via the syscon API.
+ *
+ * @baseclkfreq:   Where to find corecfg_baseclkfreq
+ * @hiword_update: If true, use HIWORD_UPDATE to access the syscon
+ */
+struct sdhci_arasan_soc_ctl_map {
+   struct sdhci_arasan_soc_ctl_field   baseclkfreq;
+   boolhiword_update;
+};
+
 /**
  * struct sdhci_arasan_data
- * @clk_ahb:   Pointer to the AHB clock
- * @phy:   Pointer to the generic phy
- * @phy_on:True if the PHY is turned on.
+ * @clk_ahb:   Pointer to the AHB clock
+ * @phy:   Pointer to the generic phy
+ * @phy_on:True if the PHY is turned on.
+ * @soc_ctl_base:  Pointer to regmap for syscon for soc_ctl registers.
+ * @soc_ctl_map:   Map to get offsets into soc_ctl registers.
  */
 struct sdhci_arasan_data {
struct clk  *clk_ahb;
struct phy  *phy;
boolphy_on;
+
+   struct regmap   *soc_ctl_base;
+   const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
+};
+
+static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
+   .baseclkfreq = { .reg = 0xf000, .width = 8, .shift = 8 },
+   .hiword_update = true,
 };
 
+/**
+ * sdhci_arasan_syscon_write - Write to a field in soc_ctl registers
+ *
+ * This function allows writing to fields in sdhci_arasan_soc_ctl_map.
+ * Note that if a field is specified as not available (shift < 0) then
+ * this function will silently return an error code.  It will be noisy
+ * and print errors for any other (unexpected) errors.
+ *
+ * @host:  The sdhci_host
+ * @fld:   The field to write to
+ * @val:   The value to write
+ */
+static int sdhci_arasan_syscon_write(struct sdhci_host *host,
+  const struct sdhci_arasan_soc_ctl_field *fld,
+  u32 val)
+{
+   struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+   struct sdhci_arasan_data 

[PATCH v3 08/15] mmc: sdhci-of-arasan: Properly set corecfg_baseclkfreq on rk3399

[Douglas Anderson]

In the the earlier change in this series ("Documentation: mmc:
sdhci-of-arasan: Add soc-ctl-syscon for corecfg regs") we can see the
mechansim for specifying a syscon to properly set corecfg registers in
sdhci-of-arasan.  Now let's use this mechanism to properly set
corecfg_baseclkfreq on rk3399.

>From [1] the corecfg_baseclkfreq is supposed to be set to:
  Base Clock Frequency for SD Clock.
  This is the frequency of the xin_clk.

This is a relatively easy thing to do.  Note that we assume that xin_clk
is not dynamic and we can check the clock at probe time.  If any real
devices have a dynamic xin_clk future patches could register for
notifiers for the clock.

At the moment, setting corecfg_baseclkfreq is only supported for rk3399
since we need a specific map for each implementation.  The code is
written in a generic way that should make this easy to extend to other
SoCs.  Note that a specific compatible string for rk3399 is already in
use and so we add that to the table to match rk3399.

[1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf

Signed-off-by: Douglas Anderson 
Reviewed-by: Heiko Stuebner 
Reviewed-by: Shawn Lin 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2:
- Reorder includes (Shawn)

 drivers/mmc/host/sdhci-of-arasan.c | 189 ++---
 1 file changed, 178 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 3ff1711077c2..1286fe8905dc 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -19,9 +19,11 @@
  * your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include "sdhci-pltfm.h"
 
 #define SDHCI_ARASAN_CLK_CTRL_OFFSET   0x2c
@@ -32,18 +34,115 @@
 #define CLK_CTRL_TIMEOUT_MASK  (0xf << CLK_CTRL_TIMEOUT_SHIFT)
 #define CLK_CTRL_TIMEOUT_MIN_EXP   13
 
+/*
+ * On some SoCs the syscon area has a feature where the upper 16-bits of
+ * each 32-bit register act as a write mask for the lower 16-bits.  This allows
+ * atomic updates of the register without locking.  This macro is used on SoCs
+ * that have that feature.
+ */
+#define HIWORD_UPDATE(val, mask, shift) \
+   ((val) << (shift) | (mask) << ((shift) + 16))
+
+/**
+ * struct sdhci_arasan_soc_ctl_field - Field used in sdhci_arasan_soc_ctl_map
+ *
+ * @reg:   Offset within the syscon of the register containing this field
+ * @width: Number of bits for this field
+ * @shift: Bit offset within @reg of this field (or -1 if not avail)
+ */
+struct sdhci_arasan_soc_ctl_field {
+   u32 reg;
+   u16 width;
+   s16 shift;
+};
+
+/**
+ * struct sdhci_arasan_soc_ctl_map - Map in syscon to corecfg registers
+ *
+ * It's up to the licensee of the Arsan IP block to make these available
+ * somewhere if needed.  Presumably these will be scattered somewhere that's
+ * accessible via the syscon API.
+ *
+ * @baseclkfreq:   Where to find corecfg_baseclkfreq
+ * @hiword_update: If true, use HIWORD_UPDATE to access the syscon
+ */
+struct sdhci_arasan_soc_ctl_map {
+   struct sdhci_arasan_soc_ctl_field   baseclkfreq;
+   boolhiword_update;
+};
+
 /**
  * struct sdhci_arasan_data
- * @clk_ahb:   Pointer to the AHB clock
- * @phy:   Pointer to the generic phy
- * @phy_on:True if the PHY is turned on.
+ * @clk_ahb:   Pointer to the AHB clock
+ * @phy:   Pointer to the generic phy
+ * @phy_on:True if the PHY is turned on.
+ * @soc_ctl_base:  Pointer to regmap for syscon for soc_ctl registers.
+ * @soc_ctl_map:   Map to get offsets into soc_ctl registers.
  */
 struct sdhci_arasan_data {
struct clk  *clk_ahb;
struct phy  *phy;
boolphy_on;
+
+   struct regmap   *soc_ctl_base;
+   const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
+};
+
+static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
+   .baseclkfreq = { .reg = 0xf000, .width = 8, .shift = 8 },
+   .hiword_update = true,
 };
 
+/**
+ * sdhci_arasan_syscon_write - Write to a field in soc_ctl registers
+ *
+ * This function allows writing to fields in sdhci_arasan_soc_ctl_map.
+ * Note that if a field is specified as not available (shift < 0) then
+ * this function will silently return an error code.  It will be noisy
+ * and print errors for any other (unexpected) errors.
+ *
+ * @host:  The sdhci_host
+ * @fld:   The field to write to
+ * @val:   The value to write
+ */
+static int sdhci_arasan_syscon_write(struct sdhci_host *host,
+  const struct sdhci_arasan_soc_ctl_field *fld,
+  u32 val)
+{
+   struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+   struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
+   struct regmap *soc_ctl_base = 

[PATCH v3 12/15] Documentation: phy: Let the rockchip eMMC PHY get an exported card clock

[Douglas Anderson]

As of an earlier change in this series ("Documentation: mmc:
sdhci-of-arasan: Add ability to export card clock") the SDHCI driver
used on Rockchip SoCs can now expose its clock.  Let's now specify that
the PHY can use it.

Letting the PHY get access to this clock means it can adjust
phyctrl_frqsel field appropriately.  Although the Rockchip PHY appears
slightly different than the reference Arasan one, you can see that the
Arasan datasheet [1] had it defined as:
  Select the frequency range of DLL operation:
  3b'000 => 200MHz to 170 MHz
  3b'001 => 170MHz to 140 MHz
  3b'010 => 140MHz to 110 MHz
  3b'011 => 110MHz to 80MHz
  3b'100 => 80MHz to 50 MHz
  3b'101 => 275Mhz to 250MHz
  3b'110 => 250MHz to 225MHz
  3b'111 => 225MHz to 200MHz

On the Rockchip version of the PHY we have less granularity but the idea
is the same.

[1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf

Signed-off-by: Douglas Anderson 
Acked-by: Kishon Vijay Abraham I 
Acked-by: Rob Herring 
Reviewed-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2:
- List out clocks and clock names (Rob)

 Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt 
b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
index 555cb0f40690..e3ea55763b0a 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
@@ -7,6 +7,13 @@ Required properties:
  - reg: PHY register address offset and length in "general
register files"
 
+Optional clocks using the clock bindings (see ../clock/clock-bindings.txt),
+specified by name:
+ - clock-names: Should contain "emmcclk".  Although this is listed as optional
+   (because most boards can get basic functionality without having
+   access to it), it is strongly suggested.
+ - clocks: Should have a phandle to the card clock exported by the SDHCI 
driver.
+
 Example:
 
 
@@ -20,6 +27,8 @@ grf: syscon@ff77 {
emmcphy: phy@f780 {
compatible = "rockchip,rk3399-emmc-phy";
reg = <0xf780 0x20>;
+   clocks = <>;
+   clock-names = "emmcclk";
#phy-cells = <0>;
};
 };
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 15/15] arm64: dts: rockchip: Provide emmcclk to PHY for rk3399

[Douglas Anderson]

Previous changes in this series allowed exposing the card clock from the
rk3399 SDHCI device and allowed consuming the card clock in the rk3399
eMMC PHY.  Hook things up in the main rk3399 dtsi file.

Signed-off-by: Douglas Anderson 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 1b57e92e0093..004b599ca285 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -225,8 +225,10 @@
assigned-clock-rates = <2>;
clocks = < SCLK_EMMC>, < ACLK_EMMC>;
clock-names = "clk_xin", "clk_ahb";
+   clock-output-names = "emmc_cardclock";
phys = <_phy>;
phy-names = "phy_arasan";
+   #clock-cells = <0>;
status = "disabled";
};
 
@@ -621,6 +623,8 @@
emmc_phy: phy@f780 {
compatible = "rockchip,rk3399-emmc-phy";
reg = <0xf780 0x24>;
+   clocks = <>;
+   clock-names = "emmcclk";
#phy-cells = <0>;
status = "disabled";
};
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 12/15] Documentation: phy: Let the rockchip eMMC PHY get an exported card clock

[Douglas Anderson]

As of an earlier change in this series ("Documentation: mmc:
sdhci-of-arasan: Add ability to export card clock") the SDHCI driver
used on Rockchip SoCs can now expose its clock.  Let's now specify that
the PHY can use it.

Letting the PHY get access to this clock means it can adjust
phyctrl_frqsel field appropriately.  Although the Rockchip PHY appears
slightly different than the reference Arasan one, you can see that the
Arasan datasheet [1] had it defined as:
  Select the frequency range of DLL operation:
  3b'000 => 200MHz to 170 MHz
  3b'001 => 170MHz to 140 MHz
  3b'010 => 140MHz to 110 MHz
  3b'011 => 110MHz to 80MHz
  3b'100 => 80MHz to 50 MHz
  3b'101 => 275Mhz to 250MHz
  3b'110 => 250MHz to 225MHz
  3b'111 => 225MHz to 200MHz

On the Rockchip version of the PHY we have less granularity but the idea
is the same.

[1]: https://arasan.com/wp-content/media/eMMC-5-1-Total-Solution_Rev-1-3.pdf

Signed-off-by: Douglas Anderson 
Acked-by: Kishon Vijay Abraham I 
Acked-by: Rob Herring 
Reviewed-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2:
- List out clocks and clock names (Rob)

 Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt | 9 +
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt 
b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
index 555cb0f40690..e3ea55763b0a 100644
--- a/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
+++ b/Documentation/devicetree/bindings/phy/rockchip-emmc-phy.txt
@@ -7,6 +7,13 @@ Required properties:
  - reg: PHY register address offset and length in "general
register files"
 
+Optional clocks using the clock bindings (see ../clock/clock-bindings.txt),
+specified by name:
+ - clock-names: Should contain "emmcclk".  Although this is listed as optional
+   (because most boards can get basic functionality without having
+   access to it), it is strongly suggested.
+ - clocks: Should have a phandle to the card clock exported by the SDHCI 
driver.
+
 Example:
 
 
@@ -20,6 +27,8 @@ grf: syscon@ff77 {
emmcphy: phy@f780 {
compatible = "rockchip,rk3399-emmc-phy";
reg = <0xf780 0x20>;
+   clocks = <>;
+   clock-names = "emmcclk";
#phy-cells = <0>;
};
 };
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 15/15] arm64: dts: rockchip: Provide emmcclk to PHY for rk3399

[Douglas Anderson]

Previous changes in this series allowed exposing the card clock from the
rk3399 SDHCI device and allowed consuming the card clock in the rk3399
eMMC PHY.  Hook things up in the main rk3399 dtsi file.

Signed-off-by: Douglas Anderson 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 1b57e92e0093..004b599ca285 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -225,8 +225,10 @@
assigned-clock-rates = <2>;
clocks = < SCLK_EMMC>, < ACLK_EMMC>;
clock-names = "clk_xin", "clk_ahb";
+   clock-output-names = "emmc_cardclock";
phys = <_phy>;
phy-names = "phy_arasan";
+   #clock-cells = <0>;
status = "disabled";
};
 
@@ -621,6 +623,8 @@
emmc_phy: phy@f780 {
compatible = "rockchip,rk3399-emmc-phy";
reg = <0xf780 0x24>;
+   clocks = <>;
+   clock-names = "emmcclk";
#phy-cells = <0>;
status = "disabled";
};
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 0/15] Changes to support 150 MHz eMMC on rk3399

[Douglas Anderson]

The theme of this series of patches is to try to allow running the eMMC
at 150 MHz on the rk3399 SoC, though the changes should still be correct
and have merit on their own.  The motivation for running at 150 MHz is
that doing so improves signal integrity and (with some eMMC devices)
doesn't affect throughput.

These patches have been structured to keep things as separate as
possible, but nevertheless there are still some dependencies between
patches.  It probably makes the most sense for all of the non-device
tree patches to go through a single tree.  PHY patches now have Acks
from Kishon so that means things should be clear for all non-DTS patches
to go through the MMC tree if Ulf agrees.  Device tree patches should be
able to be landed separately and the worst what would happen is a
warning in the kernel log if you have the code without the device tree.

The code patches are based on Ulf's mmc-next.  In v3 of the series I've
pulled in the latest version of a previous PHY series posted by Brian
Norris.

The device tree patches are based on Heiko's v4.8-armsoc/dts64.

Changes in v3:
- Add Brian's PHY patches into my series
- Add collected tags
- Add dependency on COMMON_CLK (actually in v2.1) (Guenter Roeck)
- Use phy_init / phy_exit (Heiko)
- Add Kishon's Ack

Changes in v2:
- Drop 170 MHz comment (only applicable to a subtly different Arasan PHY)
- Indicate that 5.1 ms is calculated (Shawn).
- Clean up description of rk3399 PHY (Shawn)
- Add Rob Herring's Ack.
- Reorder includes (Shawn)
- Adjust commit message wording (Rob)
- List out clocks and clock names (Rob)
- Move code cleanup before set phyctrl_frqsel based on card clock (Shawn)
- Warn if we're more than 15 MHz from ideal rate (Shawn)
- Fix typo USB => SDHCI (Shawn)

Brian Norris (2):
  phy: rockchip-emmc: configure default output tap delay
  phy: rockchip-emmc: reindent the register definitions

Douglas Anderson (11):
  phy: rockchip-emmc: Increase lock time allowance
  mmc: sdhci-of-arasan: Always power the PHY off/on when clock changes
  Documentation: mmc: sdhci-of-arasan: Add soc-ctl-syscon for corecfg
regs
  mmc: sdhci-of-arasan: Properly set corecfg_baseclkfreq on rk3399
  arm64: dts: rockchip: Add soc-ctl-syscon to sdhci for rk3399
  Documentation: mmc: sdhci-of-arasan: Add ability to export card clock
  mmc: sdhci-of-arasan: Add ability to export card clock
  Documentation: phy: Let the rockchip eMMC PHY get an exported card
clock
  phy: rockchip-emmc: Minor code cleanup in
rockchip_emmc_phy_power_on/off()
  phy: rockchip-emmc: Set phyctrl_frqsel based on card clock
  arm64: dts: rockchip: Provide emmcclk to PHY for rk3399

Shawn Lin (2):
  phy: rockchip-emmc: give DLL some extra time to be ready
  phy: rockchip-emmc: configure frequency range and drive impedance

 .../devicetree/bindings/mmc/arasan,sdhci.txt   |  35 ++-
 .../devicetree/bindings/phy/rockchip-emmc-phy.txt  |   9 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   |   5 +
 drivers/mmc/host/Kconfig   |   1 +
 drivers/mmc/host/sdhci-of-arasan.c | 333 +++--
 drivers/phy/phy-rockchip-emmc.c| 216 ++---
 6 files changed, 525 insertions(+), 74 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 11/15] mmc: sdhci-of-arasan: Add ability to export card clock

[Douglas Anderson]

Some SD/eMMC PHYs (like the PHY from Arasan that is designed to work
with arasan,sdhci-5.1) need to know the card clock in order to function
properly.  Let's add the ability to expose this clock.  Any PHY that
needs to know the clock rate can add a reference and query the clock
rate.

At the moment we register a CLK_GET_RATE_NOCACHE clock that simply
allows querying the clock.  This allows us to be less intrusive with
regards to the main SDHCI driver, which has complex logic for adjusting
the SD clock.  Right now we always fully power cycle the PHY when the
clock changes and that gives the PHY a good chance to query our clock.

Signed-off-by: Douglas Anderson 
Reviewed-by: Heiko Stuebner 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add dependency on COMMON_CLK (actually in v2.1) (Guenter Roeck)
- Add collected tags

Changes in v2: None

 drivers/mmc/host/Kconfig   |   1 +
 drivers/mmc/host/sdhci-of-arasan.c | 125 -
 2 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 0aa484c10c0a..6725dc9e644b 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -122,6 +122,7 @@ config MMC_SDHCI_OF_ARASAN
tristate "SDHCI OF support for the Arasan SDHCI controllers"
depends on MMC_SDHCI_PLTFM
depends on OF
+   depends on COMMON_CLK
help
  This selects the Arasan Secure Digital Host Controller Interface
  (SDHCI). This hardware is found e.g. in Xilinx' Zynq SoC.
diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 1286fe8905dc..678f316702e0 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -19,6 +19,7 @@
  * your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -73,17 +74,24 @@ struct sdhci_arasan_soc_ctl_map {
 
 /**
  * struct sdhci_arasan_data
+ * @host:  Pointer to the main SDHCI host structure.
  * @clk_ahb:   Pointer to the AHB clock
  * @phy:   Pointer to the generic phy
  * @phy_on:True if the PHY is turned on.
+ * @sdcardclk_hw:  Struct for the clock we might provide to a PHY.
+ * @sdcardclk: Pointer to normal 'struct clock' for sdcardclk_hw.
  * @soc_ctl_base:  Pointer to regmap for syscon for soc_ctl registers.
  * @soc_ctl_map:   Map to get offsets into soc_ctl registers.
  */
 struct sdhci_arasan_data {
+   struct sdhci_host *host;
struct clk  *clk_ahb;
struct phy  *phy;
boolphy_on;
 
+   struct clk_hw   sdcardclk_hw;
+   struct clk  *sdcardclk;
+
struct regmap   *soc_ctl_base;
const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
 };
@@ -307,6 +315,31 @@ static const struct of_device_id sdhci_arasan_of_match[] = 
{
 MODULE_DEVICE_TABLE(of, sdhci_arasan_of_match);
 
 /**
+ * sdhci_arasan_sdcardclk_recalc_rate - Return the card clock rate
+ *
+ * Return the current actual rate of the SD card clock.  This can be used
+ * to communicate with out PHY.
+ *
+ * @hw:Pointer to the hardware clock structure.
+ * @parent_rateThe parent rate (should be rate of clk_xin).
+ * Returns the card clock rate.
+ */
+static unsigned long sdhci_arasan_sdcardclk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+
+{
+   struct sdhci_arasan_data *sdhci_arasan =
+   container_of(hw, struct sdhci_arasan_data, sdcardclk_hw);
+   struct sdhci_host *host = sdhci_arasan->host;
+
+   return host->mmc->actual_clock;
+}
+
+static const struct clk_ops arasan_sdcardclk_ops = {
+   .recalc_rate = sdhci_arasan_sdcardclk_recalc_rate,
+};
+
+/**
  * sdhci_arasan_update_baseclkfreq - Set corecfg_baseclkfreq
  *
  * The corecfg_baseclkfreq is supposed to contain the MHz of clk_xin.  This
@@ -345,6 +378,83 @@ static void sdhci_arasan_update_baseclkfreq(struct 
sdhci_host *host)
sdhci_arasan_syscon_write(host, _ctl_map->baseclkfreq, mhz);
 }
 
+/**
+ * sdhci_arasan_register_sdclk - Register the sdclk for a PHY to use
+ *
+ * Some PHY devices need to know what the actual card clock is.  In order for
+ * them to find out, we'll provide a clock through the common clock framework
+ * for them to query.
+ *
+ * Note: without seriously re-architecting SDHCI's clock code and testing on
+ * all platforms, there's no way to create a totally beautiful clock here
+ * with all clock ops implemented.  Instead, we'll just create a clock that can
+ * be queried and set the CLK_GET_RATE_NOCACHE attribute to tell common clock
+ * framework that we're doing things behind its back.  This should be 
sufficient
+ * to create nice clean device tree bindings and later (if needed) we can try
+ * re-architecting SDHCI if we see some benefit to it.
+ *
+ 

[PATCH v3 0/15] Changes to support 150 MHz eMMC on rk3399

[Douglas Anderson]

The theme of this series of patches is to try to allow running the eMMC
at 150 MHz on the rk3399 SoC, though the changes should still be correct
and have merit on their own.  The motivation for running at 150 MHz is
that doing so improves signal integrity and (with some eMMC devices)
doesn't affect throughput.

These patches have been structured to keep things as separate as
possible, but nevertheless there are still some dependencies between
patches.  It probably makes the most sense for all of the non-device
tree patches to go through a single tree.  PHY patches now have Acks
from Kishon so that means things should be clear for all non-DTS patches
to go through the MMC tree if Ulf agrees.  Device tree patches should be
able to be landed separately and the worst what would happen is a
warning in the kernel log if you have the code without the device tree.

The code patches are based on Ulf's mmc-next.  In v3 of the series I've
pulled in the latest version of a previous PHY series posted by Brian
Norris.

The device tree patches are based on Heiko's v4.8-armsoc/dts64.

Changes in v3:
- Add Brian's PHY patches into my series
- Add collected tags
- Add dependency on COMMON_CLK (actually in v2.1) (Guenter Roeck)
- Use phy_init / phy_exit (Heiko)
- Add Kishon's Ack

Changes in v2:
- Drop 170 MHz comment (only applicable to a subtly different Arasan PHY)
- Indicate that 5.1 ms is calculated (Shawn).
- Clean up description of rk3399 PHY (Shawn)
- Add Rob Herring's Ack.
- Reorder includes (Shawn)
- Adjust commit message wording (Rob)
- List out clocks and clock names (Rob)
- Move code cleanup before set phyctrl_frqsel based on card clock (Shawn)
- Warn if we're more than 15 MHz from ideal rate (Shawn)
- Fix typo USB => SDHCI (Shawn)

Brian Norris (2):
  phy: rockchip-emmc: configure default output tap delay
  phy: rockchip-emmc: reindent the register definitions

Douglas Anderson (11):
  phy: rockchip-emmc: Increase lock time allowance
  mmc: sdhci-of-arasan: Always power the PHY off/on when clock changes
  Documentation: mmc: sdhci-of-arasan: Add soc-ctl-syscon for corecfg
regs
  mmc: sdhci-of-arasan: Properly set corecfg_baseclkfreq on rk3399
  arm64: dts: rockchip: Add soc-ctl-syscon to sdhci for rk3399
  Documentation: mmc: sdhci-of-arasan: Add ability to export card clock
  mmc: sdhci-of-arasan: Add ability to export card clock
  Documentation: phy: Let the rockchip eMMC PHY get an exported card
clock
  phy: rockchip-emmc: Minor code cleanup in
rockchip_emmc_phy_power_on/off()
  phy: rockchip-emmc: Set phyctrl_frqsel based on card clock
  arm64: dts: rockchip: Provide emmcclk to PHY for rk3399

Shawn Lin (2):
  phy: rockchip-emmc: give DLL some extra time to be ready
  phy: rockchip-emmc: configure frequency range and drive impedance

 .../devicetree/bindings/mmc/arasan,sdhci.txt   |  35 ++-
 .../devicetree/bindings/phy/rockchip-emmc-phy.txt  |   9 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   |   5 +
 drivers/mmc/host/Kconfig   |   1 +
 drivers/mmc/host/sdhci-of-arasan.c | 333 +++--
 drivers/phy/phy-rockchip-emmc.c| 216 ++---
 6 files changed, 525 insertions(+), 74 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 11/15] mmc: sdhci-of-arasan: Add ability to export card clock

[Douglas Anderson]

Some SD/eMMC PHYs (like the PHY from Arasan that is designed to work
with arasan,sdhci-5.1) need to know the card clock in order to function
properly.  Let's add the ability to expose this clock.  Any PHY that
needs to know the clock rate can add a reference and query the clock
rate.

At the moment we register a CLK_GET_RATE_NOCACHE clock that simply
allows querying the clock.  This allows us to be less intrusive with
regards to the main SDHCI driver, which has complex logic for adjusting
the SD clock.  Right now we always fully power cycle the PHY when the
clock changes and that gives the PHY a good chance to query our clock.

Signed-off-by: Douglas Anderson 
Reviewed-by: Heiko Stuebner 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add dependency on COMMON_CLK (actually in v2.1) (Guenter Roeck)
- Add collected tags

Changes in v2: None

 drivers/mmc/host/Kconfig   |   1 +
 drivers/mmc/host/sdhci-of-arasan.c | 125 -
 2 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 0aa484c10c0a..6725dc9e644b 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -122,6 +122,7 @@ config MMC_SDHCI_OF_ARASAN
tristate "SDHCI OF support for the Arasan SDHCI controllers"
depends on MMC_SDHCI_PLTFM
depends on OF
+   depends on COMMON_CLK
help
  This selects the Arasan Secure Digital Host Controller Interface
  (SDHCI). This hardware is found e.g. in Xilinx' Zynq SoC.
diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 1286fe8905dc..678f316702e0 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -19,6 +19,7 @@
  * your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -73,17 +74,24 @@ struct sdhci_arasan_soc_ctl_map {
 
 /**
  * struct sdhci_arasan_data
+ * @host:  Pointer to the main SDHCI host structure.
  * @clk_ahb:   Pointer to the AHB clock
  * @phy:   Pointer to the generic phy
  * @phy_on:True if the PHY is turned on.
+ * @sdcardclk_hw:  Struct for the clock we might provide to a PHY.
+ * @sdcardclk: Pointer to normal 'struct clock' for sdcardclk_hw.
  * @soc_ctl_base:  Pointer to regmap for syscon for soc_ctl registers.
  * @soc_ctl_map:   Map to get offsets into soc_ctl registers.
  */
 struct sdhci_arasan_data {
+   struct sdhci_host *host;
struct clk  *clk_ahb;
struct phy  *phy;
boolphy_on;
 
+   struct clk_hw   sdcardclk_hw;
+   struct clk  *sdcardclk;
+
struct regmap   *soc_ctl_base;
const struct sdhci_arasan_soc_ctl_map *soc_ctl_map;
 };
@@ -307,6 +315,31 @@ static const struct of_device_id sdhci_arasan_of_match[] = 
{
 MODULE_DEVICE_TABLE(of, sdhci_arasan_of_match);
 
 /**
+ * sdhci_arasan_sdcardclk_recalc_rate - Return the card clock rate
+ *
+ * Return the current actual rate of the SD card clock.  This can be used
+ * to communicate with out PHY.
+ *
+ * @hw:Pointer to the hardware clock structure.
+ * @parent_rateThe parent rate (should be rate of clk_xin).
+ * Returns the card clock rate.
+ */
+static unsigned long sdhci_arasan_sdcardclk_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+
+{
+   struct sdhci_arasan_data *sdhci_arasan =
+   container_of(hw, struct sdhci_arasan_data, sdcardclk_hw);
+   struct sdhci_host *host = sdhci_arasan->host;
+
+   return host->mmc->actual_clock;
+}
+
+static const struct clk_ops arasan_sdcardclk_ops = {
+   .recalc_rate = sdhci_arasan_sdcardclk_recalc_rate,
+};
+
+/**
  * sdhci_arasan_update_baseclkfreq - Set corecfg_baseclkfreq
  *
  * The corecfg_baseclkfreq is supposed to contain the MHz of clk_xin.  This
@@ -345,6 +378,83 @@ static void sdhci_arasan_update_baseclkfreq(struct 
sdhci_host *host)
sdhci_arasan_syscon_write(host, _ctl_map->baseclkfreq, mhz);
 }
 
+/**
+ * sdhci_arasan_register_sdclk - Register the sdclk for a PHY to use
+ *
+ * Some PHY devices need to know what the actual card clock is.  In order for
+ * them to find out, we'll provide a clock through the common clock framework
+ * for them to query.
+ *
+ * Note: without seriously re-architecting SDHCI's clock code and testing on
+ * all platforms, there's no way to create a totally beautiful clock here
+ * with all clock ops implemented.  Instead, we'll just create a clock that can
+ * be queried and set the CLK_GET_RATE_NOCACHE attribute to tell common clock
+ * framework that we're doing things behind its back.  This should be 
sufficient
+ * to create nice clean device tree bindings and later (if needed) we can try
+ * re-architecting SDHCI if we see some benefit to it.
+ *
+ * @sdhci_arasan:  Our private data structure.
+ * 

Re: [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping

[Bin Gao]

On Mon, Jun 20, 2016 at 09:52:00AM +0100, Lee Jones wrote:
> > > > > +static struct trip_config_map str3_trip_config[] = {
> > > > > + {
> > > > > + .irq_reg = BXTWC_THRM2IRQ,
> > > > > + .irq_mask = 0x10,
> > > > > + .irq_en = BXTWC_MTHRM2IRQ,
> > > > > + .irq_en_mask = 0x10,
> > > > > + .evt_stat = BXTWC_STHRM2IRQ,
> > > > > + .evt_mask = 0x10,
> > > > > + .trip_num = 0
> > > > > + },
> > > > > +};
> > > > 
> > > > This looks like a register map to me.
> > > > 
> > > > Can you use the regmap framework instead?
> > > 
> > > These are platform data used by another driver(thermal driver) which
> > > uses regmap framework to access some of the fields of the structure(
> > > irq_reg, irq_en and evt_stat).
> > 
> > I suggest you create the regmap here and pass it to the thermal driver
> > instead.
> 
> Better yet, why don't you just create the regmap in the thermal
> driver?  There is no need (in fact, it's not even allowed) to pass
> register addresses though platform data.

We did implement regmap in the acpi opregion driver(see drivers/acpi/pmic
for details) for the PMIC device. Each individual driver, e.g. gpio,
thermal, etc. will only access registers that belong to its logic unit.
Since the shared regmap implementation is already there, a single individual
driver doesn't need to implement again.

[PATCH v3 06/15] mmc: sdhci-of-arasan: Always power the PHY off/on when clock changes

[Douglas Anderson]

In commit 802ac39a5566 ("mmc: sdhci-of-arasan: fix set_clock when a phy
is supported") we added code to power the PHY off and on whenever the
clock was changed but we avoided doing the power cycle code when the
clock was low speed.  Let's now do it always.

Although there may be other reasons for power cycling the PHY when the
clock changes, one of the main reasons is that we need to give the DLL a
chance to re-lock with the new clock.

One of the things that the DLL is for is tuning the Receive Clock in
HS200 mode and STRB in HS400 mode.  Thus it is clear that we should make
sure we power cycle the PHY (and wait for the DLL to lock) when we know
we'll be in one of these two speed modes.  That's what the original code
did, though it used the clock rate rather than the speed mode.  However,
even in speed modes other than HS200,/HS400 the DLL is used for
something since it can be clearly observed that the PHY doesn't function
properly if you leave the DLL off.

Although it appears less important to power cycle the PHY and wait for
the DLL to lock when not in HS200/HS400 modes (no bugs were reported),
it still seems wise to let the locking always happen nevertheless.

Note: as part of this, we make sure that we never try to turn the PHY on
when the clock is off (when the clock rate is 0).  The PHY cannot work
when the clock is off since its DLL can't lock.

This change requires ("phy: rockchip-emmc: Increase lock time
allowance") and will cause problems if picked without that change.

Signed-off-by: Douglas Anderson 
Reviewed-by: Shawn Lin 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2: None

 drivers/mmc/host/sdhci-of-arasan.c | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 533e2bcb10bc..3ff1711077c2 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -35,11 +35,13 @@
 /**
  * struct sdhci_arasan_data
  * @clk_ahb:   Pointer to the AHB clock
- * @phy: Pointer to the generic phy
+ * @phy:   Pointer to the generic phy
+ * @phy_on:True if the PHY is turned on.
  */
 struct sdhci_arasan_data {
struct clk  *clk_ahb;
struct phy  *phy;
+   boolphy_on;
 };
 
 static unsigned int sdhci_arasan_get_timeout_clock(struct sdhci_host *host)
@@ -61,12 +63,10 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, 
unsigned int clock)
 {
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
-   bool ctrl_phy = false;
 
-   if (clock > MMC_HIGH_52_MAX_DTR && (!IS_ERR(sdhci_arasan->phy)))
-   ctrl_phy = true;
+   if (sdhci_arasan->phy_on && !IS_ERR(sdhci_arasan->phy)) {
+   sdhci_arasan->phy_on = false;
 
-   if (ctrl_phy) {
spin_unlock_irq(>lock);
phy_power_off(sdhci_arasan->phy);
spin_lock_irq(>lock);
@@ -74,7 +74,9 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, 
unsigned int clock)
 
sdhci_set_clock(host, clock);
 
-   if (ctrl_phy) {
+   if (host->mmc->actual_clock && !IS_ERR(sdhci_arasan->phy)) {
+   sdhci_arasan->phy_on = true;
+
spin_unlock_irq(>lock);
phy_power_on(sdhci_arasan->phy);
spin_lock_irq(>lock);
@@ -257,12 +259,6 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
goto clk_disable_all;
}
 
-   ret = phy_power_on(sdhci_arasan->phy);
-   if (ret < 0) {
-   dev_err(>dev, "phy_power_on err.\n");
-   goto err_phy_power;
-   }
-
host->mmc_host_ops.hs400_enhanced_strobe =
sdhci_arasan_hs400_enhanced_strobe;
}
@@ -275,9 +271,6 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 
 err_add_host:
if (!IS_ERR(sdhci_arasan->phy))
-   phy_power_off(sdhci_arasan->phy);
-err_phy_power:
-   if (!IS_ERR(sdhci_arasan->phy))
phy_exit(sdhci_arasan->phy);
 clk_disable_all:
clk_disable_unprepare(clk_xin);
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 05/15] phy: rockchip-emmc: Increase lock time allowance

[Douglas Anderson]

Previous PHY code waited a fixed amount of time for the DLL to lock at
power on time.  Unfortunately, the time for the DLL to lock is actually
a bit more dynamic and can be longer if the card clock is slower.

Instead of waiting a fixed 30 us, let's now dynamically wait until the
lock bit gets set.  We'll wait up to 10 ms which should be OK even if
the card clock is at the super slow 100 kHz.

On its own, this change makes the PHY power on code a little more
robust.  Before this change the PHY was relying on the eMMC code to make
sure the PHY was only powered on when the card clock was set to at least
50 MHz before, though this reliance wasn't documented anywhere.

This change will be even more useful in future changes where we actually
need to be able to wait for a DLL lock at slower clock speeds.

Signed-off-by: Douglas Anderson 
Acked-by: Kishon Vijay Abraham I 
Reviewed-by: Shawn Lin 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2:
- Indicate that 5.1 ms is calculated (Shawn).

 drivers/phy/phy-rockchip-emmc.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
index a69f53630e67..2d059c046978 100644
--- a/drivers/phy/phy-rockchip-emmc.c
+++ b/drivers/phy/phy-rockchip-emmc.c
@@ -85,6 +85,7 @@ static int rockchip_emmc_phy_power(struct rockchip_emmc_phy 
*rk_phy,
 {
unsigned int caldone;
unsigned int dllrdy;
+   unsigned long timeout;
 
/*
 * Keep phyctrl_pdb and phyctrl_endll low to allow
@@ -137,15 +138,26 @@ static int rockchip_emmc_phy_power(struct 
rockchip_emmc_phy *rk_phy,
   PHYCTRL_ENDLL_MASK,
   PHYCTRL_ENDLL_SHIFT));
/*
-* After enable analog DLL circuits, we need an extra 10.2us
-* for dll to be ready for work. But according to testing, we
-* find some chips need more than 25us.
+* After enabling analog DLL circuits docs say that we need 10.2 us if
+* our source clock is at 50 MHz and that lock time scales linearly
+* with clock speed.  If we are powering on the PHY and the card clock
+* is super slow (like 100 kHZ) this could take as long as 5.1 ms as
+* per the math: 10.2 us * (5000 Hz / 10 Hz) => 5.1 ms
+* Hopefully we won't be running at 100 kHz, but we should still make
+* sure we wait long enough.
 */
-   udelay(30);
-   regmap_read(rk_phy->reg_base,
-   rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
-   );
-   dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
+   timeout = jiffies + msecs_to_jiffies(10);
+   do {
+   udelay(1);
+
+   regmap_read(rk_phy->reg_base,
+   rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
+   );
+   dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
+   if (dllrdy == PHYCTRL_DLLRDY_DONE)
+   break;
+   } while (!time_after(jiffies, timeout));
+
if (dllrdy != PHYCTRL_DLLRDY_DONE) {
pr_err("rockchip_emmc_phy_power: dllrdy timeout.\n");
return -ETIMEDOUT;
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH v2] mfd: intel_soc_pmic_bxtwc: Add Intel BXT WhiskeyCove PMIC ADC thermal channel-zone mapping

[Bin Gao]

On Mon, Jun 20, 2016 at 09:52:00AM +0100, Lee Jones wrote:
> > > > > +static struct trip_config_map str3_trip_config[] = {
> > > > > + {
> > > > > + .irq_reg = BXTWC_THRM2IRQ,
> > > > > + .irq_mask = 0x10,
> > > > > + .irq_en = BXTWC_MTHRM2IRQ,
> > > > > + .irq_en_mask = 0x10,
> > > > > + .evt_stat = BXTWC_STHRM2IRQ,
> > > > > + .evt_mask = 0x10,
> > > > > + .trip_num = 0
> > > > > + },
> > > > > +};
> > > > 
> > > > This looks like a register map to me.
> > > > 
> > > > Can you use the regmap framework instead?
> > > 
> > > These are platform data used by another driver(thermal driver) which
> > > uses regmap framework to access some of the fields of the structure(
> > > irq_reg, irq_en and evt_stat).
> > 
> > I suggest you create the regmap here and pass it to the thermal driver
> > instead.
> 
> Better yet, why don't you just create the regmap in the thermal
> driver?  There is no need (in fact, it's not even allowed) to pass
> register addresses though platform data.

We did implement regmap in the acpi opregion driver(see drivers/acpi/pmic
for details) for the PMIC device. Each individual driver, e.g. gpio,
thermal, etc. will only access registers that belong to its logic unit.
Since the shared regmap implementation is already there, a single individual
driver doesn't need to implement again.

[PATCH v3 06/15] mmc: sdhci-of-arasan: Always power the PHY off/on when clock changes

[Douglas Anderson]

In commit 802ac39a5566 ("mmc: sdhci-of-arasan: fix set_clock when a phy
is supported") we added code to power the PHY off and on whenever the
clock was changed but we avoided doing the power cycle code when the
clock was low speed.  Let's now do it always.

Although there may be other reasons for power cycling the PHY when the
clock changes, one of the main reasons is that we need to give the DLL a
chance to re-lock with the new clock.

One of the things that the DLL is for is tuning the Receive Clock in
HS200 mode and STRB in HS400 mode.  Thus it is clear that we should make
sure we power cycle the PHY (and wait for the DLL to lock) when we know
we'll be in one of these two speed modes.  That's what the original code
did, though it used the clock rate rather than the speed mode.  However,
even in speed modes other than HS200,/HS400 the DLL is used for
something since it can be clearly observed that the PHY doesn't function
properly if you leave the DLL off.

Although it appears less important to power cycle the PHY and wait for
the DLL to lock when not in HS200/HS400 modes (no bugs were reported),
it still seems wise to let the locking always happen nevertheless.

Note: as part of this, we make sure that we never try to turn the PHY on
when the clock is off (when the clock rate is 0).  The PHY cannot work
when the clock is off since its DLL can't lock.

This change requires ("phy: rockchip-emmc: Increase lock time
allowance") and will cause problems if picked without that change.

Signed-off-by: Douglas Anderson 
Reviewed-by: Shawn Lin 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2: None

 drivers/mmc/host/sdhci-of-arasan.c | 23 ---
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c 
b/drivers/mmc/host/sdhci-of-arasan.c
index 533e2bcb10bc..3ff1711077c2 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -35,11 +35,13 @@
 /**
  * struct sdhci_arasan_data
  * @clk_ahb:   Pointer to the AHB clock
- * @phy: Pointer to the generic phy
+ * @phy:   Pointer to the generic phy
+ * @phy_on:True if the PHY is turned on.
  */
 struct sdhci_arasan_data {
struct clk  *clk_ahb;
struct phy  *phy;
+   boolphy_on;
 };
 
 static unsigned int sdhci_arasan_get_timeout_clock(struct sdhci_host *host)
@@ -61,12 +63,10 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, 
unsigned int clock)
 {
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
-   bool ctrl_phy = false;
 
-   if (clock > MMC_HIGH_52_MAX_DTR && (!IS_ERR(sdhci_arasan->phy)))
-   ctrl_phy = true;
+   if (sdhci_arasan->phy_on && !IS_ERR(sdhci_arasan->phy)) {
+   sdhci_arasan->phy_on = false;
 
-   if (ctrl_phy) {
spin_unlock_irq(>lock);
phy_power_off(sdhci_arasan->phy);
spin_lock_irq(>lock);
@@ -74,7 +74,9 @@ static void sdhci_arasan_set_clock(struct sdhci_host *host, 
unsigned int clock)
 
sdhci_set_clock(host, clock);
 
-   if (ctrl_phy) {
+   if (host->mmc->actual_clock && !IS_ERR(sdhci_arasan->phy)) {
+   sdhci_arasan->phy_on = true;
+
spin_unlock_irq(>lock);
phy_power_on(sdhci_arasan->phy);
spin_lock_irq(>lock);
@@ -257,12 +259,6 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
goto clk_disable_all;
}
 
-   ret = phy_power_on(sdhci_arasan->phy);
-   if (ret < 0) {
-   dev_err(>dev, "phy_power_on err.\n");
-   goto err_phy_power;
-   }
-
host->mmc_host_ops.hs400_enhanced_strobe =
sdhci_arasan_hs400_enhanced_strobe;
}
@@ -275,9 +271,6 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 
 err_add_host:
if (!IS_ERR(sdhci_arasan->phy))
-   phy_power_off(sdhci_arasan->phy);
-err_phy_power:
-   if (!IS_ERR(sdhci_arasan->phy))
phy_exit(sdhci_arasan->phy);
 clk_disable_all:
clk_disable_unprepare(clk_xin);
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 05/15] phy: rockchip-emmc: Increase lock time allowance

[Douglas Anderson]

Previous PHY code waited a fixed amount of time for the DLL to lock at
power on time.  Unfortunately, the time for the DLL to lock is actually
a bit more dynamic and can be longer if the card clock is slower.

Instead of waiting a fixed 30 us, let's now dynamically wait until the
lock bit gets set.  We'll wait up to 10 ms which should be OK even if
the card clock is at the super slow 100 kHz.

On its own, this change makes the PHY power on code a little more
robust.  Before this change the PHY was relying on the eMMC code to make
sure the PHY was only powered on when the card clock was set to at least
50 MHz before, though this reliance wasn't documented anywhere.

This change will be even more useful in future changes where we actually
need to be able to wait for a DLL lock at slower clock speeds.

Signed-off-by: Douglas Anderson 
Acked-by: Kishon Vijay Abraham I 
Reviewed-by: Shawn Lin 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add collected tags

Changes in v2:
- Indicate that 5.1 ms is calculated (Shawn).

 drivers/phy/phy-rockchip-emmc.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
index a69f53630e67..2d059c046978 100644
--- a/drivers/phy/phy-rockchip-emmc.c
+++ b/drivers/phy/phy-rockchip-emmc.c
@@ -85,6 +85,7 @@ static int rockchip_emmc_phy_power(struct rockchip_emmc_phy 
*rk_phy,
 {
unsigned int caldone;
unsigned int dllrdy;
+   unsigned long timeout;
 
/*
 * Keep phyctrl_pdb and phyctrl_endll low to allow
@@ -137,15 +138,26 @@ static int rockchip_emmc_phy_power(struct 
rockchip_emmc_phy *rk_phy,
   PHYCTRL_ENDLL_MASK,
   PHYCTRL_ENDLL_SHIFT));
/*
-* After enable analog DLL circuits, we need an extra 10.2us
-* for dll to be ready for work. But according to testing, we
-* find some chips need more than 25us.
+* After enabling analog DLL circuits docs say that we need 10.2 us if
+* our source clock is at 50 MHz and that lock time scales linearly
+* with clock speed.  If we are powering on the PHY and the card clock
+* is super slow (like 100 kHZ) this could take as long as 5.1 ms as
+* per the math: 10.2 us * (5000 Hz / 10 Hz) => 5.1 ms
+* Hopefully we won't be running at 100 kHz, but we should still make
+* sure we wait long enough.
 */
-   udelay(30);
-   regmap_read(rk_phy->reg_base,
-   rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
-   );
-   dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
+   timeout = jiffies + msecs_to_jiffies(10);
+   do {
+   udelay(1);
+
+   regmap_read(rk_phy->reg_base,
+   rk_phy->reg_offset + GRF_EMMCPHY_STATUS,
+   );
+   dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
+   if (dllrdy == PHYCTRL_DLLRDY_DONE)
+   break;
+   } while (!time_after(jiffies, timeout));
+
if (dllrdy != PHYCTRL_DLLRDY_DONE) {
pr_err("rockchip_emmc_phy_power: dllrdy timeout.\n");
return -ETIMEDOUT;
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 03/15] phy: rockchip-emmc: configure default output tap delay

[Douglas Anderson]

From: Brian Norris 

The output tap delay controls helps maintain the hold requirements for
eMMC. The exact value is dependent on the SoC and other factors, though
it isn't really an exact science. But the default of 0 is not very good,
as it doesn't give the eMMC much hold time, so let's bump up to 4
(approx 90 degree phase?). If we need to configure this any further
(e.g., based on board or speed factors), we may need to consider a
device tree representation.

Suggested-by: Shawn Lin 
Signed-off-by: Brian Norris 
Signed-off-by: Douglas Anderson 
Acked-by: Kishon Vijay Abraham I 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add Brian's PHY patches into my series

Changes in v2: None

 drivers/phy/phy-rockchip-emmc.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
index f2f75cf69af1..a0b87cc6c818 100644
--- a/drivers/phy/phy-rockchip-emmc.c
+++ b/drivers/phy/phy-rockchip-emmc.c
@@ -69,6 +69,11 @@
 #define PHYCTRL_DR_66OHM   0x2
 #define PHYCTRL_DR_100OHM  0x3
 #define PHYCTRL_DR_40OHM   0x4
+#define PHYCTRL_OTAPDLYENA 0x1
+#define PHYCTRL_OTAPDLYENA_MASK0x1
+#define PHYCTRL_OTAPDLYENA_SHIFT   0xb
+#define PHYCTRL_OTAPDLYSEL_MASK0xf
+#define PHYCTRL_OTAPDLYSEL_SHIFT   0x7
 
 struct rockchip_emmc_phy {
unsigned intreg_offset;
@@ -181,6 +186,20 @@ static int rockchip_emmc_phy_power_on(struct phy *phy)
   PHYCTRL_DR_MASK,
   PHYCTRL_DR_SHIFT));
 
+   /* Output tap delay: enable */
+   regmap_write(rk_phy->reg_base,
+rk_phy->reg_offset + GRF_EMMCPHY_CON0,
+HIWORD_UPDATE(PHYCTRL_OTAPDLYENA,
+  PHYCTRL_OTAPDLYENA_MASK,
+  PHYCTRL_OTAPDLYENA_SHIFT));
+
+   /* Output tap delay */
+   regmap_write(rk_phy->reg_base,
+rk_phy->reg_offset + GRF_EMMCPHY_CON0,
+HIWORD_UPDATE(4,
+  PHYCTRL_OTAPDLYSEL_MASK,
+  PHYCTRL_OTAPDLYSEL_SHIFT));
+
/* Power up emmc phy analog blocks */
ret = rockchip_emmc_phy_power(rk_phy, PHYCTRL_PDB_PWR_ON);
if (ret)
-- 
2.8.0.rc3.226.g39d4020

[PATCH v3 03/15] phy: rockchip-emmc: configure default output tap delay

[Douglas Anderson]

From: Brian Norris 

The output tap delay controls helps maintain the hold requirements for
eMMC. The exact value is dependent on the SoC and other factors, though
it isn't really an exact science. But the default of 0 is not very good,
as it doesn't give the eMMC much hold time, so let's bump up to 4
(approx 90 degree phase?). If we need to configure this any further
(e.g., based on board or speed factors), we may need to consider a
device tree representation.

Suggested-by: Shawn Lin 
Signed-off-by: Brian Norris 
Signed-off-by: Douglas Anderson 
Acked-by: Kishon Vijay Abraham I 
Tested-by: Heiko Stuebner 
---
Changes in v3:
- Add Brian's PHY patches into my series

Changes in v2: None

 drivers/phy/phy-rockchip-emmc.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/phy/phy-rockchip-emmc.c b/drivers/phy/phy-rockchip-emmc.c
index f2f75cf69af1..a0b87cc6c818 100644
--- a/drivers/phy/phy-rockchip-emmc.c
+++ b/drivers/phy/phy-rockchip-emmc.c
@@ -69,6 +69,11 @@
 #define PHYCTRL_DR_66OHM   0x2
 #define PHYCTRL_DR_100OHM  0x3
 #define PHYCTRL_DR_40OHM   0x4
+#define PHYCTRL_OTAPDLYENA 0x1
+#define PHYCTRL_OTAPDLYENA_MASK0x1
+#define PHYCTRL_OTAPDLYENA_SHIFT   0xb
+#define PHYCTRL_OTAPDLYSEL_MASK0xf
+#define PHYCTRL_OTAPDLYSEL_SHIFT   0x7
 
 struct rockchip_emmc_phy {
unsigned intreg_offset;
@@ -181,6 +186,20 @@ static int rockchip_emmc_phy_power_on(struct phy *phy)
   PHYCTRL_DR_MASK,
   PHYCTRL_DR_SHIFT));
 
+   /* Output tap delay: enable */
+   regmap_write(rk_phy->reg_base,
+rk_phy->reg_offset + GRF_EMMCPHY_CON0,
+HIWORD_UPDATE(PHYCTRL_OTAPDLYENA,
+  PHYCTRL_OTAPDLYENA_MASK,
+  PHYCTRL_OTAPDLYENA_SHIFT));
+
+   /* Output tap delay */
+   regmap_write(rk_phy->reg_base,
+rk_phy->reg_offset + GRF_EMMCPHY_CON0,
+HIWORD_UPDATE(4,
+  PHYCTRL_OTAPDLYSEL_MASK,
+  PHYCTRL_OTAPDLYSEL_SHIFT));
+
/* Power up emmc phy analog blocks */
ret = rockchip_emmc_phy_power(rk_phy, PHYCTRL_PDB_PWR_ON);
if (ret)
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH v2 0/3] firmware: scpi: add device power domain support

[Sudeep Holla]

On 20/06/16 18:56, Kevin Hilman wrote:

Sudeep Holla  writes:


This series add support for SCPI based device device power state
management using genpd.

Regards,
Sudeep

v1[1]->v2:
- Fixed the endianness handling in scpi_device_get_power_state
  as spotted by Tixy
- Renamed scpi_pd.c to scpi_pm_domain.c as suggested by Ulf
- Added Mark's ack on DT binding

[1] https://marc.info/?l=linux-kernel=146522850003483=2

Sudeep Holla (3):
   firmware: arm_scpi: add support for device power state management
   Documentation: add DT bindings for ARM SCPI power domains
   firmware: scpi: add device power domain support using genpd


Other than the couple nits on specific patches,

Reviewed-by: Kevin Hilman 

And just a[nother] reminder to be sure to keep things that are Juno
specific well described since we know other vendors will be (and are
being) "creative" in their implementations of SCPI.



Agreed as mentioned in the other thread.

I just sent a PR to arm-soc this afternoon. Do you think it's better to
recall it and incorporate this ?

Extremely sorry for rushing, I thought it would get too late. Ulf
reviewed them so I rushed a bit.

--
Regards,
Sudeep

Re: [PATCH v2 0/3] firmware: scpi: add device power domain support

[Sudeep Holla]

On 20/06/16 18:56, Kevin Hilman wrote:

Sudeep Holla  writes:


This series add support for SCPI based device device power state
management using genpd.

Regards,
Sudeep

v1[1]->v2:
- Fixed the endianness handling in scpi_device_get_power_state
  as spotted by Tixy
- Renamed scpi_pd.c to scpi_pm_domain.c as suggested by Ulf
- Added Mark's ack on DT binding

[1] https://marc.info/?l=linux-kernel=146522850003483=2

Sudeep Holla (3):
   firmware: arm_scpi: add support for device power state management
   Documentation: add DT bindings for ARM SCPI power domains
   firmware: scpi: add device power domain support using genpd


Other than the couple nits on specific patches,

Reviewed-by: Kevin Hilman 

And just a[nother] reminder to be sure to keep things that are Juno
specific well described since we know other vendors will be (and are
being) "creative" in their implementations of SCPI.



Agreed as mentioned in the other thread.

I just sent a PR to arm-soc this afternoon. Do you think it's better to
recall it and incorporate this ?

Extremely sorry for rushing, I thought it would get too late. Ulf
reviewed them so I rushed a bit.

--
Regards,
Sudeep

the usage of __SYSCALL_MASK in entry_SYSCALL_64/do_syscall_64 is not consistent

[Oleg Nesterov]

On 06/19, Andy Lutomirski wrote:
>
> Something's clearly buggy there,

The usage of __X32_SYSCALL_BIT doesn't look right too. Nothing serious
but still.

Damn, initially I thought I have found the serious bug in entry_64.S
and it took me some time to understand why my exploit doesn't work ;)
So I learned that

andl$__SYSCALL_MASK, %eax

in entry_SYSCALL_64_fastpath() zero-extends %rax and thus

cmpl$__NR_syscall_max, %eax
...
call*sys_call_table(, %rax, 8)

is correct (rax <= __NR_syscall_max).

OK, so entry_64.S simply "ignores" the upper bits if CONFIG_X86_X32_ABI.
Fine, but this doesn't match the

if (likely((nr & __SYSCALL_MASK) < NR_syscalls))

check in do_syscall_64(). So this test-case

#include 

int main(void)
{
// __NR_exit == 0x3c
asm volatile ("movq $0x003c, %rax; syscall");

printf("I didn't exit because I am traced\n");

return 0;
}

silently exits if not traced, otherwise it calls printf().

Should we do something or we do not care?

Oleg.

Re: [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros

[Linus Torvalds]

On Sun, Jun 19, 2016 at 5:26 PM, Deepa Dinamani  wrote:
> The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC 
> macros.

This version now looks ok to me.

I do have a comment (or maybe just a RFD) for future work.

It does strike me that once we actually change over the inode times to
use timespec64, the calling conventions are going to be fairly
horrendous on most 32-bit architectures.

Gcc handles 8-byte structure returns (on most architectures) by
returning them as two 32-bit registers (%edx:%eax on x86). But once it
is timespec64, that will no longer be the case, and the calling
convention will end up using a pointer to the local stack instead.

So for 32-bit code generation, we *may* want to introduce a new model of doing

set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);

which basically just does

inode->i_atime = inode->i_mtime = current_time(inode);

but with a much easier calling convention on 32-bit architectures.

But that is entirely orthogonal to this patch-set, and should be seen
as a separate issue.

And maybe it doesn't end up helping anyway, but I do think those
"simple" direct assignments will really generate pretty disgusting
code on 32-bit architectures.

That whole

inode->i_atime = inode->i_mtime = CURRENT_TIME;

model really made a lot more sense back in the ancient days when inode
times were just simply 32-bit seconds (not even timeval structures).

  Linus

the usage of __SYSCALL_MASK in entry_SYSCALL_64/do_syscall_64 is not consistent

[Oleg Nesterov]

On 06/19, Andy Lutomirski wrote:
>
> Something's clearly buggy there,

The usage of __X32_SYSCALL_BIT doesn't look right too. Nothing serious
but still.

Damn, initially I thought I have found the serious bug in entry_64.S
and it took me some time to understand why my exploit doesn't work ;)
So I learned that

andl$__SYSCALL_MASK, %eax

in entry_SYSCALL_64_fastpath() zero-extends %rax and thus

cmpl$__NR_syscall_max, %eax
...
call*sys_call_table(, %rax, 8)

is correct (rax <= __NR_syscall_max).

OK, so entry_64.S simply "ignores" the upper bits if CONFIG_X86_X32_ABI.
Fine, but this doesn't match the

if (likely((nr & __SYSCALL_MASK) < NR_syscalls))

check in do_syscall_64(). So this test-case

#include 

int main(void)
{
// __NR_exit == 0x3c
asm volatile ("movq $0x003c, %rax; syscall");

printf("I didn't exit because I am traced\n");

return 0;
}

silently exits if not traced, otherwise it calls printf().

Should we do something or we do not care?

Oleg.

Re: [PATCH v2 00/24] Delete CURRENT_TIME and CURRENT_TIME_SEC macros

[Linus Torvalds]

On Sun, Jun 19, 2016 at 5:26 PM, Deepa Dinamani  wrote:
> The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC 
> macros.

This version now looks ok to me.

I do have a comment (or maybe just a RFD) for future work.

It does strike me that once we actually change over the inode times to
use timespec64, the calling conventions are going to be fairly
horrendous on most 32-bit architectures.

Gcc handles 8-byte structure returns (on most architectures) by
returning them as two 32-bit registers (%edx:%eax on x86). But once it
is timespec64, that will no longer be the case, and the calling
convention will end up using a pointer to the local stack instead.

So for 32-bit code generation, we *may* want to introduce a new model of doing

set_inode_time(inode, ATTR_ATIME | ATTR_MTIME);

which basically just does

inode->i_atime = inode->i_mtime = current_time(inode);

but with a much easier calling convention on 32-bit architectures.

But that is entirely orthogonal to this patch-set, and should be seen
as a separate issue.

And maybe it doesn't end up helping anyway, but I do think those
"simple" direct assignments will really generate pretty disgusting
code on 32-bit architectures.

That whole

inode->i_atime = inode->i_mtime = CURRENT_TIME;

model really made a lot more sense back in the ancient days when inode
times were just simply 32-bit seconds (not even timeval structures).

  Linus

[PATCH] perf: add 'perf bench syscall'

[Josh Poimboeuf]

On Wed, Feb 03, 2016 at 11:22:47AM +0100, Ingo Molnar wrote:
> 
> * Andy Lutomirski  wrote:
> 
> > On Jan 31, 2016 11:42 PM, "Ingo Molnar"  wrote:
> > >
> > >
> > > * r...@redhat.com  wrote:
> > >
> > > > (v3: address comments raised by Frederic)
> > > >
> > > > Running with nohz_full introduces a fair amount of overhead.
> > > > Specifically, various things that are usually done from the
> > > > timer interrupt are now done at syscall, irq, and guest
> > > > entry and exit times.
> > > >
> > > > However, some of the code that is called every single time
> > > > has only ever worked at jiffy resolution. The code in
> > > > __acct_update_integrals was also doing some unnecessary
> > > > calculations.
> > > >
> > > > Getting rid of the unnecessary calculations, without
> > > > changing any of the functionality in __acct_update_integrals
> > > > gets us about an 11% win.
> > > >
> > > > Not calling the time statistics updating code more than
> > > > once per jiffy, like is done on housekeeping CPUs and on
> > > > all the CPUs of a non-nohz_full system, shaves off a
> > > > further 30%.
> > > >
> > > > I tested this series with a microbenchmark calling
> > > > an invalid syscall number ten million times in a row,
> > > > on a nohz_full cpu.
> > > >
> > > > Run times for the microbenchmark:
> > > >
> > > > 4.4   3.8 seconds
> > > > 4.5-rc1   3.7 seconds
> > > > 4.5-rc1 + first patch 3.3 seconds
> > > > 4.5-rc1 + first 3 patches 3.1 seconds
> > > > 4.5-rc1 + all patches 2.3 seconds
> > >
> > > Another suggestion (beyond fixing the 32-bit build ;-), could you please 
> > > stick
> > > your syscall microbenchmark into 'perf bench', so that we have a 
> > > standardized way
> > > of checking such numbers?
> > >
> > > In fact I'd suggest we introduce an entirely new sub-tool for system call
> > > performance measurement - and this might be the first functionality of it.
> > >
> > > I've attached a quick patch that is basically a copy of 'perf bench numa' 
> > > and
> > > which measures getppid() performance (simple syscall where the result is 
> > > not
> > > cached by glibc).
> > >
> > > I kept the process, threading and memory allocation bits of numa.c, just 
> > > in case
> > > we need them to measure more complex syscalls. Maybe we could keep the 
> > > threading
> > > bits and remove the memory allocation parameters, to simplify the 
> > > benchmark?
> > >
> > > Anyway, this could be a good base to start off on.
> > 
> > So much code...
> 
> Arguably 90% of that should be factored out, as it's now a duplicate between 
> bench/numa.c and bench/syscall.c.
> 
> Technically, for a minimum benchmark, something like this would already be 
> functional for tools/perf/bench/syscall.c:
> 
> #include "../perf.h"
> #include "../util/util.h"
> #include "../builtin.h"
> #include "bench.h"
> 
> static void run_syscall_benchmark(void)
> {
>   [  your benchmark loop as-is ... ]
> }
> 
> int bench_syscall(int argc __maybe_unused, const char **argv __maybe_unused, 
> const char *prefix __maybe_unused)
> {
>   run_syscall_benchmark();
> 
> switch (bench_format) {
> case BENCH_FORMAT_DEFAULT:
> printf("print results in human-readable format\n");
> break;
> case BENCH_FORMAT_SIMPLE:
> printf("print results in machine-parseable format\n");
> break;
> default:
>   BUG_ON(1);
> }
> 
>   return 0;
> }
> 
> Plus the small amount of glue for bench_sycall() I sent in the first patch.
> 
> Completely untested.
> 
> If the loop is long enough then even without any timing measurement this 
> would be 
> usable via:
> 
>   perf stat --null --repeat 10 perf bench syscall
> 
> as the 'perf stat' will do the timing and statistics.

How about this:



From: Josh Poimboeuf 
Subject: [PATCH] perf: add 'perf bench syscall'

Add a basic 'perf bench syscall' benchmark which does a getppid() system
call in a tight loop.

Suggested-by: Ingo Molnar 
Signed-off-by: Josh Poimboeuf 
---
 tools/perf/bench/Build |  1 +
 tools/perf/bench/bench.h   |  1 +
 tools/perf/bench/syscall.c | 80 ++
 tools/perf/builtin-bench.c | 18 ---
 4 files changed, 95 insertions(+), 5 deletions(-)
 create mode 100644 tools/perf/bench/syscall.c

diff --git a/tools/perf/bench/Build b/tools/perf/bench/Build
index 60bf119..0b7395d 100644
--- a/tools/perf/bench/Build
+++ b/tools/perf/bench/Build
@@ -1,5 +1,6 @@
 perf-y += sched-messaging.o
 perf-y += sched-pipe.o
+perf-y += syscall.o
 perf-y += mem-functions.o
 perf-y += futex-hash.o
 perf-y += futex-wake.o
diff --git a/tools/perf/bench/bench.h b/tools/perf/bench/bench.h
index 579a592..bdd6cdc 100644
--- a/tools/perf/bench/bench.h
+++ 

[PATCH] perf: add 'perf bench syscall'

[Josh Poimboeuf]

On Wed, Feb 03, 2016 at 11:22:47AM +0100, Ingo Molnar wrote:
> 
> * Andy Lutomirski  wrote:
> 
> > On Jan 31, 2016 11:42 PM, "Ingo Molnar"  wrote:
> > >
> > >
> > > * r...@redhat.com  wrote:
> > >
> > > > (v3: address comments raised by Frederic)
> > > >
> > > > Running with nohz_full introduces a fair amount of overhead.
> > > > Specifically, various things that are usually done from the
> > > > timer interrupt are now done at syscall, irq, and guest
> > > > entry and exit times.
> > > >
> > > > However, some of the code that is called every single time
> > > > has only ever worked at jiffy resolution. The code in
> > > > __acct_update_integrals was also doing some unnecessary
> > > > calculations.
> > > >
> > > > Getting rid of the unnecessary calculations, without
> > > > changing any of the functionality in __acct_update_integrals
> > > > gets us about an 11% win.
> > > >
> > > > Not calling the time statistics updating code more than
> > > > once per jiffy, like is done on housekeeping CPUs and on
> > > > all the CPUs of a non-nohz_full system, shaves off a
> > > > further 30%.
> > > >
> > > > I tested this series with a microbenchmark calling
> > > > an invalid syscall number ten million times in a row,
> > > > on a nohz_full cpu.
> > > >
> > > > Run times for the microbenchmark:
> > > >
> > > > 4.4   3.8 seconds
> > > > 4.5-rc1   3.7 seconds
> > > > 4.5-rc1 + first patch 3.3 seconds
> > > > 4.5-rc1 + first 3 patches 3.1 seconds
> > > > 4.5-rc1 + all patches 2.3 seconds
> > >
> > > Another suggestion (beyond fixing the 32-bit build ;-), could you please 
> > > stick
> > > your syscall microbenchmark into 'perf bench', so that we have a 
> > > standardized way
> > > of checking such numbers?
> > >
> > > In fact I'd suggest we introduce an entirely new sub-tool for system call
> > > performance measurement - and this might be the first functionality of it.
> > >
> > > I've attached a quick patch that is basically a copy of 'perf bench numa' 
> > > and
> > > which measures getppid() performance (simple syscall where the result is 
> > > not
> > > cached by glibc).
> > >
> > > I kept the process, threading and memory allocation bits of numa.c, just 
> > > in case
> > > we need them to measure more complex syscalls. Maybe we could keep the 
> > > threading
> > > bits and remove the memory allocation parameters, to simplify the 
> > > benchmark?
> > >
> > > Anyway, this could be a good base to start off on.
> > 
> > So much code...
> 
> Arguably 90% of that should be factored out, as it's now a duplicate between 
> bench/numa.c and bench/syscall.c.
> 
> Technically, for a minimum benchmark, something like this would already be 
> functional for tools/perf/bench/syscall.c:
> 
> #include "../perf.h"
> #include "../util/util.h"
> #include "../builtin.h"
> #include "bench.h"
> 
> static void run_syscall_benchmark(void)
> {
>   [  your benchmark loop as-is ... ]
> }
> 
> int bench_syscall(int argc __maybe_unused, const char **argv __maybe_unused, 
> const char *prefix __maybe_unused)
> {
>   run_syscall_benchmark();
> 
> switch (bench_format) {
> case BENCH_FORMAT_DEFAULT:
> printf("print results in human-readable format\n");
> break;
> case BENCH_FORMAT_SIMPLE:
> printf("print results in machine-parseable format\n");
> break;
> default:
>   BUG_ON(1);
> }
> 
>   return 0;
> }
> 
> Plus the small amount of glue for bench_sycall() I sent in the first patch.
> 
> Completely untested.
> 
> If the loop is long enough then even without any timing measurement this 
> would be 
> usable via:
> 
>   perf stat --null --repeat 10 perf bench syscall
> 
> as the 'perf stat' will do the timing and statistics.

How about this:



From: Josh Poimboeuf 
Subject: [PATCH] perf: add 'perf bench syscall'

Add a basic 'perf bench syscall' benchmark which does a getppid() system
call in a tight loop.

Suggested-by: Ingo Molnar 
Signed-off-by: Josh Poimboeuf 
---
 tools/perf/bench/Build |  1 +
 tools/perf/bench/bench.h   |  1 +
 tools/perf/bench/syscall.c | 80 ++
 tools/perf/builtin-bench.c | 18 ---
 4 files changed, 95 insertions(+), 5 deletions(-)
 create mode 100644 tools/perf/bench/syscall.c

diff --git a/tools/perf/bench/Build b/tools/perf/bench/Build
index 60bf119..0b7395d 100644
--- a/tools/perf/bench/Build
+++ b/tools/perf/bench/Build
@@ -1,5 +1,6 @@
 perf-y += sched-messaging.o
 perf-y += sched-pipe.o
+perf-y += syscall.o
 perf-y += mem-functions.o
 perf-y += futex-hash.o
 perf-y += futex-wake.o
diff --git a/tools/perf/bench/bench.h b/tools/perf/bench/bench.h
index 579a592..bdd6cdc 100644
--- a/tools/perf/bench/bench.h
+++ b/tools/perf/bench/bench.h
@@ -28,6 +28,7 @@
 int bench_numa(int argc, const char **argv, const char *prefix);
 int 

[PATCH v3] gpio: add Intel WhiskeyCove GPIO driver

[Bin Gao]

This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
This driver is based on gpio-crystalcove.c.

Signed-off-by: Ajay Thomas 
Signed-off-by: Bin Gao 
---
Changes in v3:
 - Fixed the year in copyright line(2015-->2016).
 - Removed DRV_NAME macro.
 - Added kernel-doc for regmap_irq_chip of the wcove_gpio structure.
 - Line length fix.
Changes in v2:
 - Typo fix (Whsikey --> Whiskey).
 - Included linux/gpio/driver.h instead of linux/gpio.h
 - Implemented .set_single_ended().
 - Added GPIO register description.
 - Replaced container_of() with gpiochip_get_data().
 - Removed unnecessary "if (gpio > WCOVE_VGPIO_NUM" check.
 - Removed the device id table and added MODULE_ALIAS().
 drivers/gpio/Kconfig  |  13 ++
 drivers/gpio/Makefile |   1 +
 drivers/gpio/gpio-wcove.c | 425 ++
 3 files changed, 439 insertions(+)
 create mode 100644 drivers/gpio/gpio-wcove.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index cebcb40..ac74299 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -806,6 +806,19 @@ config GPIO_CRYSTAL_COVE
  This driver can also be built as a module. If so, the module will be
  called gpio-crystalcove.
 
+config GPIO_WHISKEY_COVE
+   tristate "GPIO support for Whiskey Cove PMIC"
+   depends on INTEL_SOC_PMIC
+   select GPIOLIB_IRQCHIP
+   help
+ Support for GPIO pins on Whiskey Cove PMIC.
+
+ Say Yes if you have a Intel SoC based tablet with Whiskey Cove PMIC
+ inside.
+
+ This driver can also be built as a module. If so, the module will be
+ called gpio-wcove.
+
 config GPIO_CS5535
tristate "AMD CS5535/CS5536 GPIO support"
depends on MFD_CS5535
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 991598e..fff6914 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_BT8XX)  += gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CLPS711X)+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_CS5535)  += gpio-cs5535.o
 obj-$(CONFIG_GPIO_CRYSTAL_COVE)+= gpio-crystalcove.o
+obj-$(CONFIG_GPIO_WHISKEY_COVE)+= gpio-wcove.o
 obj-$(CONFIG_GPIO_DA9052)  += gpio-da9052.o
 obj-$(CONFIG_GPIO_DA9055)  += gpio-da9055.o
 obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
new file mode 100644
index 000..61f6d77
--- /dev/null
+++ b/drivers/gpio/gpio-wcove.c
@@ -0,0 +1,425 @@
+/*
+ * gpio-wcove.c - Intel Whiskey Cove GPIO Driver
+ *
+ * This driver is written based on gpio-crystalcove.c
+ *
+ * Copyright (C) 2016 Intel Corporation. All rights reserved.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Whiskey Cove PMIC has 13 physical GPIO pins divided into 3 banks:
+ * Bank 0: Pin 0 - 6
+ * Bank 1: Pin 7 - 10
+ * Bank 2: Pin 11 -12
+ * Each pin has one output control register and one input control register.
+ */
+#define BANK0_NR_PINS  7
+#define BANK1_NR_PINS  4
+#define BANK2_NR_PINS  2
+#define WCOVE_GPIO_NUM (BANK0_NR_PINS + BANK1_NR_PINS + BANK2_NR_PINS)
+#define WCOVE_VGPIO_NUM94
+/* GPIO output control registers(one per pin): 0x4e44 - 0x4e50 */
+#define GPIO_OUT_CTRL_BASE 0x4e44
+/* GPIO input control registers(one per pin): 0x4e51 - 0x4e5d */
+#define GPIO_IN_CTRL_BASE  0x4e51
+
+/*
+ * GPIO interrupts are organized in two groups:
+ * Group 0: Bank 0 pins (Pin 0 - 6)
+ * Group 1: Bank 1 and Bank 2 pins (Pin 7 - 12)
+ * Each group has two registers(one bit per pin): status and mask.
+ */
+#define GROUP0_NR_IRQS 7
+#define GROUP1_NR_IRQS 6
+#define IRQ_MASK_BASE  0x4e19
+#define IRQ_STATUS_BASE0x4e0b
+#define UPDATE_IRQ_TYPEBIT(0)
+#define UPDATE_IRQ_MASKBIT(1)
+
+#define CTLI_INTCNT_DIS(0)
+#define CTLI_INTCNT_NE (1 << 1)
+#define CTLI_INTCNT_PE (2 << 1)
+#define CTLI_INTCNT_BE (3 << 1)
+
+#define CTLO_DIR_IN(0)
+#define CTLO_DIR_OUT   (1 << 5)
+
+#define CTLO_DRV_MASK  (1 << 4)
+#define CTLO_DRV_OD(0)
+#define CTLO_DRV_CMOS  CTLO_DRV_MASK
+
+#define CTLO_DRV_REN   (1 << 3)
+
+#define CTLO_RVAL_2KDW (0)
+#define CTLO_RVAL_2KUP (1 << 1)
+#define CTLO_RVAL_50KDW(2 << 1)
+#define CTLO_RVAL_50KUP(3 << 

[PATCH v3] gpio: add Intel WhiskeyCove GPIO driver

[Bin Gao]

This patch introduces a separate GPIO driver for Intel WhiskeyCove PMIC.
This driver is based on gpio-crystalcove.c.

Signed-off-by: Ajay Thomas 
Signed-off-by: Bin Gao 
---
Changes in v3:
 - Fixed the year in copyright line(2015-->2016).
 - Removed DRV_NAME macro.
 - Added kernel-doc for regmap_irq_chip of the wcove_gpio structure.
 - Line length fix.
Changes in v2:
 - Typo fix (Whsikey --> Whiskey).
 - Included linux/gpio/driver.h instead of linux/gpio.h
 - Implemented .set_single_ended().
 - Added GPIO register description.
 - Replaced container_of() with gpiochip_get_data().
 - Removed unnecessary "if (gpio > WCOVE_VGPIO_NUM" check.
 - Removed the device id table and added MODULE_ALIAS().
 drivers/gpio/Kconfig  |  13 ++
 drivers/gpio/Makefile |   1 +
 drivers/gpio/gpio-wcove.c | 425 ++
 3 files changed, 439 insertions(+)
 create mode 100644 drivers/gpio/gpio-wcove.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index cebcb40..ac74299 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -806,6 +806,19 @@ config GPIO_CRYSTAL_COVE
  This driver can also be built as a module. If so, the module will be
  called gpio-crystalcove.
 
+config GPIO_WHISKEY_COVE
+   tristate "GPIO support for Whiskey Cove PMIC"
+   depends on INTEL_SOC_PMIC
+   select GPIOLIB_IRQCHIP
+   help
+ Support for GPIO pins on Whiskey Cove PMIC.
+
+ Say Yes if you have a Intel SoC based tablet with Whiskey Cove PMIC
+ inside.
+
+ This driver can also be built as a module. If so, the module will be
+ called gpio-wcove.
+
 config GPIO_CS5535
tristate "AMD CS5535/CS5536 GPIO support"
depends on MFD_CS5535
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 991598e..fff6914 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_GPIO_BT8XX)  += gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CLPS711X)+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_CS5535)  += gpio-cs5535.o
 obj-$(CONFIG_GPIO_CRYSTAL_COVE)+= gpio-crystalcove.o
+obj-$(CONFIG_GPIO_WHISKEY_COVE)+= gpio-wcove.o
 obj-$(CONFIG_GPIO_DA9052)  += gpio-da9052.o
 obj-$(CONFIG_GPIO_DA9055)  += gpio-da9055.o
 obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
new file mode 100644
index 000..61f6d77
--- /dev/null
+++ b/drivers/gpio/gpio-wcove.c
@@ -0,0 +1,425 @@
+/*
+ * gpio-wcove.c - Intel Whiskey Cove GPIO Driver
+ *
+ * This driver is written based on gpio-crystalcove.c
+ *
+ * Copyright (C) 2016 Intel Corporation. All rights reserved.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * Whiskey Cove PMIC has 13 physical GPIO pins divided into 3 banks:
+ * Bank 0: Pin 0 - 6
+ * Bank 1: Pin 7 - 10
+ * Bank 2: Pin 11 -12
+ * Each pin has one output control register and one input control register.
+ */
+#define BANK0_NR_PINS  7
+#define BANK1_NR_PINS  4
+#define BANK2_NR_PINS  2
+#define WCOVE_GPIO_NUM (BANK0_NR_PINS + BANK1_NR_PINS + BANK2_NR_PINS)
+#define WCOVE_VGPIO_NUM94
+/* GPIO output control registers(one per pin): 0x4e44 - 0x4e50 */
+#define GPIO_OUT_CTRL_BASE 0x4e44
+/* GPIO input control registers(one per pin): 0x4e51 - 0x4e5d */
+#define GPIO_IN_CTRL_BASE  0x4e51
+
+/*
+ * GPIO interrupts are organized in two groups:
+ * Group 0: Bank 0 pins (Pin 0 - 6)
+ * Group 1: Bank 1 and Bank 2 pins (Pin 7 - 12)
+ * Each group has two registers(one bit per pin): status and mask.
+ */
+#define GROUP0_NR_IRQS 7
+#define GROUP1_NR_IRQS 6
+#define IRQ_MASK_BASE  0x4e19
+#define IRQ_STATUS_BASE0x4e0b
+#define UPDATE_IRQ_TYPEBIT(0)
+#define UPDATE_IRQ_MASKBIT(1)
+
+#define CTLI_INTCNT_DIS(0)
+#define CTLI_INTCNT_NE (1 << 1)
+#define CTLI_INTCNT_PE (2 << 1)
+#define CTLI_INTCNT_BE (3 << 1)
+
+#define CTLO_DIR_IN(0)
+#define CTLO_DIR_OUT   (1 << 5)
+
+#define CTLO_DRV_MASK  (1 << 4)
+#define CTLO_DRV_OD(0)
+#define CTLO_DRV_CMOS  CTLO_DRV_MASK
+
+#define CTLO_DRV_REN   (1 << 3)
+
+#define CTLO_RVAL_2KDW (0)
+#define CTLO_RVAL_2KUP (1 << 1)
+#define CTLO_RVAL_50KDW(2 << 1)
+#define CTLO_RVAL_50KUP(3 << 1)
+
+#define CTLO_INPUT_SET (CTLO_DRV_CMOS | CTLO_DRV_REN | 

Re: [PATCH V3] clocksource/drivers/bcm_kona: Convert init function to return error

[Ray Jui]

Hi Daniel,

On 6/20/2016 10:48 AM, Daniel Lezcano wrote:

The init functions do not return any error. They behave as the following:

  - panic, thus leading to a kernel crash while another timer may work and
   make the system boot up correctly

  or

  - print an error and let the caller unaware if the state of the system

Change that by converting the init functions to return an error conforming
to the CLOCKSOURCE_OF_RET prototype.

Proper error handling (rollback, errno value) will be changed later case
by case, thus this change just return back an error or success in the init
function.

Signed-off-by: Daniel Lezcano 
---
 drivers/clocksource/bcm_kona_timer.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/bcm_kona_timer.c 
b/drivers/clocksource/bcm_kona_timer.c
index e717e87..c251aa6 100644
--- a/drivers/clocksource/bcm_kona_timer.c
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -163,16 +163,11 @@ static struct irqaction kona_timer_irq = {
.handler = kona_timer_interrupt,
 };

-static void __init kona_timer_init(struct device_node *node)
+static int __init kona_timer_init(struct device_node *node)
 {
u32 freq;
struct clk *external_clk;

-   if (!of_device_is_available(node)) {
-   pr_info("Kona Timer v1 marked as disabled in device tree\n");
-   return;
-   }
-
external_clk = of_clk_get_by_name(node, NULL);

if (!IS_ERR(external_clk)) {
@@ -182,7 +177,7 @@ static void __init kona_timer_init(struct device_node *node)
arch_timer_rate = freq;
} else {
pr_err("Kona Timer v1 unable to determine clock-frequency");
-   return;
+   return -EINVAL;
}

/* Setup IRQ numbers */
@@ -196,11 +191,13 @@ static void __init kona_timer_init(struct device_node 
*node)
kona_timer_clockevents_init();
setup_irq(timers.tmr_irq, _timer_irq);
kona_timer_set_next_event((arch_timer_rate / HZ), NULL);
+
+   return 0;
 }

-CLOCKSOURCE_OF_DECLARE(brcm_kona, "brcm,kona-timer", kona_timer_init);
+CLOCKSOURCE_OF_DECLARE_RET(brcm_kona, "brcm,kona-timer", kona_timer_init);
 /*
  * bcm,kona-timer is deprecated by brcm,kona-timer
  * being kept here for driver compatibility
  */
-CLOCKSOURCE_OF_DECLARE(bcm_kona, "bcm,kona-timer", kona_timer_init);
+CLOCKSOURCE_OF_DECLARE_RET(bcm_kona, "bcm,kona-timer", kona_timer_init);


Looks good to me!

Acked-by: Ray Jui 

Thanks,

Ray

Re: [PATCH V3] clocksource/drivers/bcm_kona: Convert init function to return error

[Ray Jui]

Hi Daniel,

On 6/20/2016 10:48 AM, Daniel Lezcano wrote:

The init functions do not return any error. They behave as the following:

  - panic, thus leading to a kernel crash while another timer may work and
   make the system boot up correctly

  or

  - print an error and let the caller unaware if the state of the system

Change that by converting the init functions to return an error conforming
to the CLOCKSOURCE_OF_RET prototype.

Proper error handling (rollback, errno value) will be changed later case
by case, thus this change just return back an error or success in the init
function.

Signed-off-by: Daniel Lezcano 
---
 drivers/clocksource/bcm_kona_timer.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/bcm_kona_timer.c 
b/drivers/clocksource/bcm_kona_timer.c
index e717e87..c251aa6 100644
--- a/drivers/clocksource/bcm_kona_timer.c
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -163,16 +163,11 @@ static struct irqaction kona_timer_irq = {
.handler = kona_timer_interrupt,
 };

-static void __init kona_timer_init(struct device_node *node)
+static int __init kona_timer_init(struct device_node *node)
 {
u32 freq;
struct clk *external_clk;

-   if (!of_device_is_available(node)) {
-   pr_info("Kona Timer v1 marked as disabled in device tree\n");
-   return;
-   }
-
external_clk = of_clk_get_by_name(node, NULL);

if (!IS_ERR(external_clk)) {
@@ -182,7 +177,7 @@ static void __init kona_timer_init(struct device_node *node)
arch_timer_rate = freq;
} else {
pr_err("Kona Timer v1 unable to determine clock-frequency");
-   return;
+   return -EINVAL;
}

/* Setup IRQ numbers */
@@ -196,11 +191,13 @@ static void __init kona_timer_init(struct device_node 
*node)
kona_timer_clockevents_init();
setup_irq(timers.tmr_irq, _timer_irq);
kona_timer_set_next_event((arch_timer_rate / HZ), NULL);
+
+   return 0;
 }

-CLOCKSOURCE_OF_DECLARE(brcm_kona, "brcm,kona-timer", kona_timer_init);
+CLOCKSOURCE_OF_DECLARE_RET(brcm_kona, "brcm,kona-timer", kona_timer_init);
 /*
  * bcm,kona-timer is deprecated by brcm,kona-timer
  * being kept here for driver compatibility
  */
-CLOCKSOURCE_OF_DECLARE(bcm_kona, "bcm,kona-timer", kona_timer_init);
+CLOCKSOURCE_OF_DECLARE_RET(bcm_kona, "bcm,kona-timer", kona_timer_init);


Looks good to me!

Acked-by: Ray Jui 

Thanks,

Ray

Re: adf_ctl_drv.c:(.init.text+0x14915): undefined reference to `adf_init_pf_wq' (was Linux 4.4.13)

[Randy Dunlap]

On 06/20/16 08:43, Greg KH wrote:
> On Sun, Jun 19, 2016 at 09:12:29AM +0200, Hans de Bruin wrote:
>> On 06/08/2016 03:43 AM, Greg KH wrote:
>>> I'm announcing the release of the 4.4.13 kernel.
>>>
>>
>> Hi,
>>
>> I tried to compile 4.4.13 using my 4.4.7 config file and ran in to this:
>>
>>   LD  init/built-in.o
>> drivers/built-in.o: In function `adf_register_ctl_device_driver':
>> adf_ctl_drv.c:(.init.text+0x14915): undefined reference to `adf_init_pf_wq'
>> adf_ctl_drv.c:(.init.text+0x14927): undefined reference to `adf_exit_pf_wq'
>> drivers/built-in.o: In function `adf_unregister_ctl_device_driver':
>> adf_ctl_drv.c:(.exit.text+0xacf): undefined reference to `adf_exit_pf_wq'
>> make[2]: *** [vmlinux] Error 1
>> make[1]: *** [sub-make] Error 2
>> make: *** [__sub-make] Error 2
>>
>>
>> I have got some Google hits on this error referencing to higher kernel
>> versions but not on 4.4:
>>
>>  LKML: Herbert Xu: Crypto Fixes for 4.6
>>
>>  LKML: Greg KH: Linux 4.5.5
> 
> I don't understand, what needs to be done in the 4.4-stable tree to
> resolve this?

Maybe some other DEV_QAT drivers need to add "select PCI_IOV".

See commit 75910d375ecb2079b3418f8b304fd775916025e2 (sorry, I don't
have the 4.4.x stable git tree [yet]).

Hans, please send your failing .config file.


-- 
~Randy

[PATCH v4 net-next v4 04/14] net: dsa: mv88e6xxx: do not increment bus refcount

[Vivien Didelot]

The MDIO device probe and remove functions are respectively incrementing
and decrementing the bus refcount themselves. Since these bus level
actions are out of the device scope, remove them.

Signed-off-by: Vivien Didelot 
Acked-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index b3170ea..4b4bffc 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3712,8 +3712,6 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
ps->sw_addr = mdiodev->addr;
mutex_init(>smi_mutex);
 
-   get_device(>bus->dev);
-
ds->drv = _switch_driver;
 
id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
@@ -3767,7 +3765,6 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
dsa_unregister_switch(ds);
-   put_device(>bus->dev);
 
mv88e6xxx_mdio_unregister(ps);
 }
-- 
2.9.0

Re: adf_ctl_drv.c:(.init.text+0x14915): undefined reference to `adf_init_pf_wq' (was Linux 4.4.13)

[Randy Dunlap]

On 06/20/16 08:43, Greg KH wrote:
> On Sun, Jun 19, 2016 at 09:12:29AM +0200, Hans de Bruin wrote:
>> On 06/08/2016 03:43 AM, Greg KH wrote:
>>> I'm announcing the release of the 4.4.13 kernel.
>>>
>>
>> Hi,
>>
>> I tried to compile 4.4.13 using my 4.4.7 config file and ran in to this:
>>
>>   LD  init/built-in.o
>> drivers/built-in.o: In function `adf_register_ctl_device_driver':
>> adf_ctl_drv.c:(.init.text+0x14915): undefined reference to `adf_init_pf_wq'
>> adf_ctl_drv.c:(.init.text+0x14927): undefined reference to `adf_exit_pf_wq'
>> drivers/built-in.o: In function `adf_unregister_ctl_device_driver':
>> adf_ctl_drv.c:(.exit.text+0xacf): undefined reference to `adf_exit_pf_wq'
>> make[2]: *** [vmlinux] Error 1
>> make[1]: *** [sub-make] Error 2
>> make: *** [__sub-make] Error 2
>>
>>
>> I have got some Google hits on this error referencing to higher kernel
>> versions but not on 4.4:
>>
>>  LKML: Herbert Xu: Crypto Fixes for 4.6
>>
>>  LKML: Greg KH: Linux 4.5.5
> 
> I don't understand, what needs to be done in the 4.4-stable tree to
> resolve this?

Maybe some other DEV_QAT drivers need to add "select PCI_IOV".

See commit 75910d375ecb2079b3418f8b304fd775916025e2 (sorry, I don't
have the 4.4.x stable git tree [yet]).

Hans, please send your failing .config file.


-- 
~Randy

[PATCH v4 net-next v4 04/14] net: dsa: mv88e6xxx: do not increment bus refcount

[Vivien Didelot]

The MDIO device probe and remove functions are respectively incrementing
and decrementing the bus refcount themselves. Since these bus level
actions are out of the device scope, remove them.

Signed-off-by: Vivien Didelot 
Acked-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index b3170ea..4b4bffc 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3712,8 +3712,6 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
ps->sw_addr = mdiodev->addr;
mutex_init(>smi_mutex);
 
-   get_device(>bus->dev);
-
ds->drv = _switch_driver;
 
id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
@@ -3767,7 +3765,6 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
dsa_unregister_switch(ds);
-   put_device(>bus->dev);
 
mv88e6xxx_mdio_unregister(ps);
 }
-- 
2.9.0

[PATCH v4 net-next v4 05/14] net: dsa: mv88e6xxx: add switch register helpers

[Vivien Didelot]

The mixed assignments, allocations and registrations in the probe code
make it hard to follow the logic and figure out what is DSA or chip
specific.

Extract the struct dsa_switch related code in a simple
mv88e6xxx_register_switch helper function.

For symmetry in the code, add a mv88e6xxx_unregister_switch function.

Signed-off-by: Vivien Didelot 
Reviewed-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 41 -
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 4b4bffc..ad7735d 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3690,30 +3690,48 @@ static struct dsa_switch_driver mv88e6xxx_switch_driver 
= {
.port_fdb_dump  = mv88e6xxx_port_fdb_dump,
 };
 
+static int mv88e6xxx_register_switch(struct mv88e6xxx_priv_state *ps,
+struct device_node *np)
+{
+   struct device *dev = ps->dev;
+   struct dsa_switch *ds;
+
+   ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
+   if (!ds)
+   return -ENOMEM;
+
+   ds->dev = dev;
+   ds->priv = ps;
+   ds->drv = _switch_driver;
+
+   dev_set_drvdata(dev, ds);
+
+   return dsa_register_switch(ds, np);
+}
+
+static void mv88e6xxx_unregister_switch(struct mv88e6xxx_priv_state *ps)
+{
+   dsa_unregister_switch(ps->ds);
+}
+
 static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 {
struct device *dev = >dev;
struct device_node *np = dev->of_node;
struct mv88e6xxx_priv_state *ps;
int id, prod_num, rev;
-   struct dsa_switch *ds;
u32 eeprom_len;
int err;
 
-   ds = devm_kzalloc(dev, sizeof(*ds) + sizeof(*ps), GFP_KERNEL);
-   if (!ds)
+   ps = devm_kzalloc(dev, sizeof(*ps), GFP_KERNEL);
+   if (!ps)
return -ENOMEM;
 
-   ps = (struct mv88e6xxx_priv_state *)(ds + 1);
-   ds->priv = ps;
-   ds->dev = dev;
ps->dev = dev;
ps->bus = mdiodev->bus;
ps->sw_addr = mdiodev->addr;
mutex_init(>smi_mutex);
 
-   ds->drv = _switch_driver;
-
id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
if (id < 0)
return id;
@@ -3745,9 +3763,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (err)
return err;
 
-   dev_set_drvdata(dev, ds);
-
-   err = dsa_register_switch(ds, np);
+   err = mv88e6xxx_register_switch(ps, np);
if (err) {
mv88e6xxx_mdio_unregister(ps);
return err;
@@ -3764,8 +3780,7 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
struct dsa_switch *ds = dev_get_drvdata(>dev);
struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-   dsa_unregister_switch(ds);
-
+   mv88e6xxx_unregister_switch(ps);
mv88e6xxx_mdio_unregister(ps);
 }
 
-- 
2.9.0

[PATCH v4 net-next v4 05/14] net: dsa: mv88e6xxx: add switch register helpers

[Vivien Didelot]

The mixed assignments, allocations and registrations in the probe code
make it hard to follow the logic and figure out what is DSA or chip
specific.

Extract the struct dsa_switch related code in a simple
mv88e6xxx_register_switch helper function.

For symmetry in the code, add a mv88e6xxx_unregister_switch function.

Signed-off-by: Vivien Didelot 
Reviewed-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 41 -
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 4b4bffc..ad7735d 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3690,30 +3690,48 @@ static struct dsa_switch_driver mv88e6xxx_switch_driver 
= {
.port_fdb_dump  = mv88e6xxx_port_fdb_dump,
 };
 
+static int mv88e6xxx_register_switch(struct mv88e6xxx_priv_state *ps,
+struct device_node *np)
+{
+   struct device *dev = ps->dev;
+   struct dsa_switch *ds;
+
+   ds = devm_kzalloc(dev, sizeof(*ds), GFP_KERNEL);
+   if (!ds)
+   return -ENOMEM;
+
+   ds->dev = dev;
+   ds->priv = ps;
+   ds->drv = _switch_driver;
+
+   dev_set_drvdata(dev, ds);
+
+   return dsa_register_switch(ds, np);
+}
+
+static void mv88e6xxx_unregister_switch(struct mv88e6xxx_priv_state *ps)
+{
+   dsa_unregister_switch(ps->ds);
+}
+
 static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 {
struct device *dev = >dev;
struct device_node *np = dev->of_node;
struct mv88e6xxx_priv_state *ps;
int id, prod_num, rev;
-   struct dsa_switch *ds;
u32 eeprom_len;
int err;
 
-   ds = devm_kzalloc(dev, sizeof(*ds) + sizeof(*ps), GFP_KERNEL);
-   if (!ds)
+   ps = devm_kzalloc(dev, sizeof(*ps), GFP_KERNEL);
+   if (!ps)
return -ENOMEM;
 
-   ps = (struct mv88e6xxx_priv_state *)(ds + 1);
-   ds->priv = ps;
-   ds->dev = dev;
ps->dev = dev;
ps->bus = mdiodev->bus;
ps->sw_addr = mdiodev->addr;
mutex_init(>smi_mutex);
 
-   ds->drv = _switch_driver;
-
id = mv88e6xxx_reg_read(ps, REG_PORT(0), PORT_SWITCH_ID);
if (id < 0)
return id;
@@ -3745,9 +3763,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (err)
return err;
 
-   dev_set_drvdata(dev, ds);
-
-   err = dsa_register_switch(ds, np);
+   err = mv88e6xxx_register_switch(ps, np);
if (err) {
mv88e6xxx_mdio_unregister(ps);
return err;
@@ -3764,8 +3780,7 @@ static void mv88e6xxx_remove(struct mdio_device *mdiodev)
struct dsa_switch *ds = dev_get_drvdata(>dev);
struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
-   dsa_unregister_switch(ds);
-
+   mv88e6xxx_unregister_switch(ps);
mv88e6xxx_mdio_unregister(ps);
 }
 
-- 
2.9.0

Re: [PATCH v2 3/3] firmware: scpi: add device power domain support using genpd

[Kevin Hilman]

"Jon Medhurst (Tixy)"  writes:

> On Thu, 2016-06-16 at 18:59 +0100, Sudeep Holla wrote:
>> 
>> On 16/06/16 18:47, Jon Medhurst (Tixy) wrote:
>> > On Thu, 2016-06-16 at 11:38 +0100, Sudeep Holla wrote:
>> > [...]
>> >> +enum scpi_power_domain_state {
>> >> + SCPI_PD_STATE_ON = 0,
>> >> + SCPI_PD_STATE_OFF = 3,
>> >> +};
>> >
>> > The SCPI doc defines the meaning of these numbers (0 and 3) in the 'Juno
>> > specifics' chapter. So does these values need to come from device-tree
>> > to allow for other hardware or SCP implementations?
>> >
>> 
>> Ah unfortunately true :(. I had not noticed that. But I would like to
>> check if this can be made as part of the standard protocol. Adding such
>> details to DT seems overkill and defeat of the whole purpose of the
>> standard protocol.
>
> Well. it seems to me the 'standard protocol' is whatever the current
> implementation of ARM's closed source SCP firmware is. It also seems to
> me that people are making things up as they go along, without a clue as
> to how to make things generic, robust and future proof. Basically,
> Status Normal ARM Fucked Up.

Fully agree here.  Just because ARM calls it a "standard" does not make
it so.  As we've already seen[1], vendors are using initial/older/whatever
versions of SCPI, so pushing the Juno version as standard just becuase
it's in ARM's closed firmware is not the right way forward either.

Kevin

[1] http://marc.info/?l=linux-arm-kernel=146425562931515=2

Re: [PATCH v2 3/3] firmware: scpi: add device power domain support using genpd

[Kevin Hilman]

"Jon Medhurst (Tixy)"  writes:

> On Thu, 2016-06-16 at 18:59 +0100, Sudeep Holla wrote:
>> 
>> On 16/06/16 18:47, Jon Medhurst (Tixy) wrote:
>> > On Thu, 2016-06-16 at 11:38 +0100, Sudeep Holla wrote:
>> > [...]
>> >> +enum scpi_power_domain_state {
>> >> + SCPI_PD_STATE_ON = 0,
>> >> + SCPI_PD_STATE_OFF = 3,
>> >> +};
>> >
>> > The SCPI doc defines the meaning of these numbers (0 and 3) in the 'Juno
>> > specifics' chapter. So does these values need to come from device-tree
>> > to allow for other hardware or SCP implementations?
>> >
>> 
>> Ah unfortunately true :(. I had not noticed that. But I would like to
>> check if this can be made as part of the standard protocol. Adding such
>> details to DT seems overkill and defeat of the whole purpose of the
>> standard protocol.
>
> Well. it seems to me the 'standard protocol' is whatever the current
> implementation of ARM's closed source SCP firmware is. It also seems to
> me that people are making things up as they go along, without a clue as
> to how to make things generic, robust and future proof. Basically,
> Status Normal ARM Fucked Up.

Fully agree here.  Just because ARM calls it a "standard" does not make
it so.  As we've already seen[1], vendors are using initial/older/whatever
versions of SCPI, so pushing the Juno version as standard just becuase
it's in ARM's closed firmware is not the right way forward either.

Kevin

[1] http://marc.info/?l=linux-arm-kernel=146425562931515=2

[PATCH v4 net-next v4 03/14] net: dsa: mv88e6xxx: use already declared variables

[Vivien Didelot]

In the MDIO probing function, dev is already assigned to >dev
and np is already assigned to mdiodev->dev.of_node, so use them.

Signed-off-by: Vivien Didelot 
Reviewed-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 673283a..b3170ea 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3728,7 +3728,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (!ps->info)
return -ENODEV;
 
-   ps->reset = devm_gpiod_get(>dev, "reset", GPIOD_ASIS);
+   ps->reset = devm_gpiod_get(dev, "reset", GPIOD_ASIS);
if (IS_ERR(ps->reset)) {
err = PTR_ERR(ps->reset);
if (err == -ENOENT) {
@@ -3743,13 +3743,13 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
!of_property_read_u32(np, "eeprom-length", _len))
ps->eeprom_len = eeprom_len;
 
-   err = mv88e6xxx_mdio_register(ps, mdiodev->dev.of_node);
+   err = mv88e6xxx_mdio_register(ps, np);
if (err)
return err;
 
dev_set_drvdata(dev, ds);
 
-   err = dsa_register_switch(ds, mdiodev->dev.of_node);
+   err = dsa_register_switch(ds, np);
if (err) {
mv88e6xxx_mdio_unregister(ps);
return err;
-- 
2.9.0

Re: [PATCH v4 net-next v4 14/14] net: dsa: mv88e6xxx: abstract switch registers accesses

[Vivien Didelot]

Hi Andrew, David,

Andrew Lunn  writes:

> On Mon, Jun 20, 2016 at 12:03:37PM -0400, Vivien Didelot wrote:
>> When the SMI address of the switch chip is zero, the chip assumes to be
>> the only one on the SMI master bus and thus responds to all its known
>> SMI devices addresses (port registers, Global2, etc.)
>> 
>> When its SMI address is not zero, some chips (e.g. 88E6352) use an
>> indirect access through two SMI Command and Data registers.
>> 
>> Other models (e.g. 88E6060) using less than 16 internal SMI addresses
>> always use a direct access.
>> 
>> Add a capability flag to describe chips supporting the (indirect)
>> Multi-chip Addressing Mode, and a low-level API to access the registers
>> via SMI.
>> 
>> Other accesses (like Ethernet management frames) may be added later.
>> 
>> Signed-off-by: Vivien Didelot 
>
> Reviewed-by: Andrew Lunn 
>
> This series is now ready for merging.

I introduced a warning in that patch by mistake, by printing 'val'
instead of '*val' in a dev_dbg() call...

I respin a v5 with Andrew's tag and the debug printing fixed.

Sorry for the noice...

  Vivien

[PATCH v4 net-next v4 07/14] net: dsa: mv88e6xxx: remove table args in info lookup

[Vivien Didelot]

The mv88e6xxx_table array and the mv88e6xxx_lookup_info function are
static, so remove the table and size arguments from the lookup function.

Signed-off-by: Vivien Didelot 
Reviewed-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ec28465..75e5408 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3590,15 +3590,13 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
},
 };
 
-static const struct mv88e6xxx_info *
-mv88e6xxx_lookup_info(unsigned int prod_num, const struct mv88e6xxx_info 
*table,
- unsigned int num)
+static const struct mv88e6xxx_info *mv88e6xxx_lookup_info(unsigned int 
prod_num)
 {
int i;
 
-   for (i = 0; i < num; ++i)
-   if (table[i].prod_num == prod_num)
-   return [i];
+   for (i = 0; i < ARRAY_SIZE(mv88e6xxx_table); ++i)
+   if (mv88e6xxx_table[i].prod_num == prod_num)
+   return _table[i];
 
return NULL;
 }
@@ -3625,8 +3623,7 @@ static const char *mv88e6xxx_drv_probe(struct device 
*dsa_dev,
prod_num = (id & 0xfff0) >> 4;
rev = id & 0x000f;
 
-   info = mv88e6xxx_lookup_info(prod_num, mv88e6xxx_table,
-ARRAY_SIZE(mv88e6xxx_table));
+   info = mv88e6xxx_lookup_info(prod_num);
if (!info)
return NULL;
 
@@ -3739,8 +3736,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
prod_num = (id & 0xfff0) >> 4;
rev = id & 0x000f;
 
-   ps->info = mv88e6xxx_lookup_info(prod_num, mv88e6xxx_table,
-ARRAY_SIZE(mv88e6xxx_table));
+   ps->info = mv88e6xxx_lookup_info(prod_num);
if (!ps->info)
return -ENODEV;
 
-- 
2.9.0

Re: [PATCH v2 3/3] firmware: scpi: add device power domain support using genpd

[Sudeep Holla]

On 20/06/16 18:50, Kevin Hilman wrote:

"Jon Medhurst (Tixy)"  writes:


On Thu, 2016-06-16 at 18:59 +0100, Sudeep Holla wrote:


On 16/06/16 18:47, Jon Medhurst (Tixy) wrote:

On Thu, 2016-06-16 at 11:38 +0100, Sudeep Holla wrote:
[...]

+enum scpi_power_domain_state {
+   SCPI_PD_STATE_ON = 0,
+   SCPI_PD_STATE_OFF = 3,
+};


The SCPI doc defines the meaning of these numbers (0 and 3) in the 'Juno
specifics' chapter. So does these values need to come from device-tree
to allow for other hardware or SCP implementations?



Ah unfortunately true :(. I had not noticed that. But I would like to
check if this can be made as part of the standard protocol. Adding such
details to DT seems overkill and defeat of the whole purpose of the
standard protocol.


Well. it seems to me the 'standard protocol' is whatever the current
implementation of ARM's closed source SCP firmware is. It also seems to
me that people are making things up as they go along, without a clue as
to how to make things generic, robust and future proof. Basically,
Status Normal ARM Fucked Up.


Fully agree here.  Just because ARM calls it a "standard" does not make
it so.  As we've already seen[1], vendors are using initial/older/whatever
versions of SCPI, so pushing the Juno version as standard just becuase
it's in ARM's closed firmware is not the right way forward either.



I am not disagreeing on that. There's an on going effort to make it as
standard as PSCI. But that's still work in progress. We need to deal
with the existing variety of SCPI until then. No argument on that.

The only argument I had on the other thread is not to make the changes
too flexible that enabled the vendors to make up their own deviation of
SCPI even for their future platforms. All I am asking is to keep is less
flexible just to support existing platforms out there and not to help
the vendors to deviate further.

--
Regards,
Sudeep

[PATCH v4 net-next v4 01/14] net: dsa: mv88e6xxx: fix style issues

[Vivien Didelot]

This patch fixes 5 style problems reported by checkpatch:

WARNING: suspect code indent for conditional statements (8, 24)
#492: FILE: drivers/net/dsa/mv88e6xxx.c:492:
+   if (phydev->link)
+   reg |= PORT_PCS_CTRL_LINK_UP;

CHECK: Logical continuations should be on the previous line
#1318: FILE: drivers/net/dsa/mv88e6xxx.c:1318:
+oldstate == PORT_CONTROL_STATE_FORWARDING)
+   && (state == PORT_CONTROL_STATE_DISABLED ||

CHECK: multiple assignments should be avoided
#1662: FILE: drivers/net/dsa/mv88e6xxx.c:1662:
+   vlan->vid_begin = vlan->vid_end = next.vid;

WARNING: line over 80 characters
#2097: FILE: drivers/net/dsa/mv88e6xxx.c:2097:
+  const struct switchdev_obj_port_vlan 
*vlan,

WARNING: suspect code indent for conditional statements (16, 32)
#2734: FILE: drivers/net/dsa/mv88e6xxx.c:2734:
+   if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
[...]
+   reg |= PORT_CONTROL_EGRESS_ADD_TAG;

total: 0 errors, 3 warnings, 2 checks, 3805 lines checked

It also rebases and integrates changes sent by Ben Dooks [1]:

The driver has a number of functions that are not exported or
declared elsewhere, so make them static to avoid the following
warnings from sparse:

drivers/net/dsa/mv88e6xxx.c:113:5: warning: symbol 'mv88e6xxx_reg_read' was 
not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:167:5: warning: symbol 'mv88e6xxx_reg_write' 
was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:231:5: warning: symbol 'mv88e6xxx_set_addr' was 
not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:367:6: warning: symbol 
'mv88e6xxx_ppu_state_init' was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:3157:5: warning: symbol 
'mv88e6xxx_phy_page_read' was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:3169:5: warning: symbol 
'mv88e6xxx_phy_page_write' was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:3583:26: warning: symbol 
'mv88e6xxx_switch_driver' was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:3621:5: warning: symbol 'mv88e6xxx_probe' was 
not declared. Should it be static?

[1] http://patchwork.ozlabs.org/patch/632708/

Signed-off-by: Vivien Didelot 
Reviewed-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 42 +++---
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ee06055..1972ec5 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -111,7 +111,8 @@ static int _mv88e6xxx_reg_read(struct mv88e6xxx_priv_state 
*ps,
return ret;
 }
 
-int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr, int reg)
+static int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr,
+ int reg)
 {
int ret;
 
@@ -165,8 +166,8 @@ static int _mv88e6xxx_reg_write(struct mv88e6xxx_priv_state 
*ps, int addr,
return __mv88e6xxx_reg_write(ps->bus, ps->sw_addr, addr, reg, val);
 }
 
-int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
-   int reg, u16 val)
+static int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
+  int reg, u16 val)
 {
int ret;
 
@@ -229,7 +230,7 @@ static int mv88e6xxx_set_addr_indirect(struct dsa_switch 
*ds, u8 *addr)
return 0;
 }
 
-int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
+static int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
 {
struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
@@ -370,7 +371,7 @@ static void mv88e6xxx_ppu_access_put(struct 
mv88e6xxx_priv_state *ps)
mutex_unlock(>ppu_mutex);
 }
 
-void mv88e6xxx_ppu_state_init(struct mv88e6xxx_priv_state *ps)
+static void mv88e6xxx_ppu_state_init(struct mv88e6xxx_priv_state *ps)
 {
mutex_init(>ppu_mutex);
INIT_WORK(>ppu_work, mv88e6xxx_ppu_reenable_work);
@@ -490,7 +491,7 @@ static void mv88e6xxx_adjust_link(struct dsa_switch *ds, 
int port,
 
reg |= PORT_PCS_CTRL_FORCE_LINK;
if (phydev->link)
-   reg |= PORT_PCS_CTRL_LINK_UP;
+   reg |= PORT_PCS_CTRL_LINK_UP;
 
if (mv88e6xxx_6065_family(ps) && phydev->speed > SPEED_100)
goto out;
@@ -1314,9 +1315,9 @@ static int _mv88e6xxx_port_state(struct 
mv88e6xxx_priv_state *ps, int port,
 * Blocking or Listening state.
 */
if ((oldstate == PORT_CONTROL_STATE_LEARNING ||
-oldstate == PORT_CONTROL_STATE_FORWARDING)
-   && (state == PORT_CONTROL_STATE_DISABLED ||
-   state == 

[PATCH v4 net-next v4 03/14] net: dsa: mv88e6xxx: use already declared variables

[Vivien Didelot]

In the MDIO probing function, dev is already assigned to >dev
and np is already assigned to mdiodev->dev.of_node, so use them.

Signed-off-by: Vivien Didelot 
Reviewed-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 673283a..b3170ea 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3728,7 +3728,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (!ps->info)
return -ENODEV;
 
-   ps->reset = devm_gpiod_get(>dev, "reset", GPIOD_ASIS);
+   ps->reset = devm_gpiod_get(dev, "reset", GPIOD_ASIS);
if (IS_ERR(ps->reset)) {
err = PTR_ERR(ps->reset);
if (err == -ENOENT) {
@@ -3743,13 +3743,13 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
!of_property_read_u32(np, "eeprom-length", _len))
ps->eeprom_len = eeprom_len;
 
-   err = mv88e6xxx_mdio_register(ps, mdiodev->dev.of_node);
+   err = mv88e6xxx_mdio_register(ps, np);
if (err)
return err;
 
dev_set_drvdata(dev, ds);
 
-   err = dsa_register_switch(ds, mdiodev->dev.of_node);
+   err = dsa_register_switch(ds, np);
if (err) {
mv88e6xxx_mdio_unregister(ps);
return err;
-- 
2.9.0

Re: [PATCH v4 net-next v4 14/14] net: dsa: mv88e6xxx: abstract switch registers accesses

[Vivien Didelot]

Hi Andrew, David,

Andrew Lunn  writes:

> On Mon, Jun 20, 2016 at 12:03:37PM -0400, Vivien Didelot wrote:
>> When the SMI address of the switch chip is zero, the chip assumes to be
>> the only one on the SMI master bus and thus responds to all its known
>> SMI devices addresses (port registers, Global2, etc.)
>> 
>> When its SMI address is not zero, some chips (e.g. 88E6352) use an
>> indirect access through two SMI Command and Data registers.
>> 
>> Other models (e.g. 88E6060) using less than 16 internal SMI addresses
>> always use a direct access.
>> 
>> Add a capability flag to describe chips supporting the (indirect)
>> Multi-chip Addressing Mode, and a low-level API to access the registers
>> via SMI.
>> 
>> Other accesses (like Ethernet management frames) may be added later.
>> 
>> Signed-off-by: Vivien Didelot 
>
> Reviewed-by: Andrew Lunn 
>
> This series is now ready for merging.

I introduced a warning in that patch by mistake, by printing 'val'
instead of '*val' in a dev_dbg() call...

I respin a v5 with Andrew's tag and the debug printing fixed.

Sorry for the noice...

  Vivien

[PATCH v4 net-next v4 07/14] net: dsa: mv88e6xxx: remove table args in info lookup

[Vivien Didelot]

The mv88e6xxx_table array and the mv88e6xxx_lookup_info function are
static, so remove the table and size arguments from the lookup function.

Signed-off-by: Vivien Didelot 
Reviewed-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ec28465..75e5408 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -3590,15 +3590,13 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
},
 };
 
-static const struct mv88e6xxx_info *
-mv88e6xxx_lookup_info(unsigned int prod_num, const struct mv88e6xxx_info 
*table,
- unsigned int num)
+static const struct mv88e6xxx_info *mv88e6xxx_lookup_info(unsigned int 
prod_num)
 {
int i;
 
-   for (i = 0; i < num; ++i)
-   if (table[i].prod_num == prod_num)
-   return [i];
+   for (i = 0; i < ARRAY_SIZE(mv88e6xxx_table); ++i)
+   if (mv88e6xxx_table[i].prod_num == prod_num)
+   return _table[i];
 
return NULL;
 }
@@ -3625,8 +3623,7 @@ static const char *mv88e6xxx_drv_probe(struct device 
*dsa_dev,
prod_num = (id & 0xfff0) >> 4;
rev = id & 0x000f;
 
-   info = mv88e6xxx_lookup_info(prod_num, mv88e6xxx_table,
-ARRAY_SIZE(mv88e6xxx_table));
+   info = mv88e6xxx_lookup_info(prod_num);
if (!info)
return NULL;
 
@@ -3739,8 +3736,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
prod_num = (id & 0xfff0) >> 4;
rev = id & 0x000f;
 
-   ps->info = mv88e6xxx_lookup_info(prod_num, mv88e6xxx_table,
-ARRAY_SIZE(mv88e6xxx_table));
+   ps->info = mv88e6xxx_lookup_info(prod_num);
if (!ps->info)
return -ENODEV;
 
-- 
2.9.0

Re: [PATCH v2 3/3] firmware: scpi: add device power domain support using genpd

[Sudeep Holla]

On 20/06/16 18:50, Kevin Hilman wrote:

"Jon Medhurst (Tixy)"  writes:


On Thu, 2016-06-16 at 18:59 +0100, Sudeep Holla wrote:


On 16/06/16 18:47, Jon Medhurst (Tixy) wrote:

On Thu, 2016-06-16 at 11:38 +0100, Sudeep Holla wrote:
[...]

+enum scpi_power_domain_state {
+   SCPI_PD_STATE_ON = 0,
+   SCPI_PD_STATE_OFF = 3,
+};


The SCPI doc defines the meaning of these numbers (0 and 3) in the 'Juno
specifics' chapter. So does these values need to come from device-tree
to allow for other hardware or SCP implementations?



Ah unfortunately true :(. I had not noticed that. But I would like to
check if this can be made as part of the standard protocol. Adding such
details to DT seems overkill and defeat of the whole purpose of the
standard protocol.


Well. it seems to me the 'standard protocol' is whatever the current
implementation of ARM's closed source SCP firmware is. It also seems to
me that people are making things up as they go along, without a clue as
to how to make things generic, robust and future proof. Basically,
Status Normal ARM Fucked Up.


Fully agree here.  Just because ARM calls it a "standard" does not make
it so.  As we've already seen[1], vendors are using initial/older/whatever
versions of SCPI, so pushing the Juno version as standard just becuase
it's in ARM's closed firmware is not the right way forward either.



I am not disagreeing on that. There's an on going effort to make it as
standard as PSCI. But that's still work in progress. We need to deal
with the existing variety of SCPI until then. No argument on that.

The only argument I had on the other thread is not to make the changes
too flexible that enabled the vendors to make up their own deviation of
SCPI even for their future platforms. All I am asking is to keep is less
flexible just to support existing platforms out there and not to help
the vendors to deviate further.

--
Regards,
Sudeep

[PATCH v4 net-next v4 01/14] net: dsa: mv88e6xxx: fix style issues

[Vivien Didelot]

This patch fixes 5 style problems reported by checkpatch:

WARNING: suspect code indent for conditional statements (8, 24)
#492: FILE: drivers/net/dsa/mv88e6xxx.c:492:
+   if (phydev->link)
+   reg |= PORT_PCS_CTRL_LINK_UP;

CHECK: Logical continuations should be on the previous line
#1318: FILE: drivers/net/dsa/mv88e6xxx.c:1318:
+oldstate == PORT_CONTROL_STATE_FORWARDING)
+   && (state == PORT_CONTROL_STATE_DISABLED ||

CHECK: multiple assignments should be avoided
#1662: FILE: drivers/net/dsa/mv88e6xxx.c:1662:
+   vlan->vid_begin = vlan->vid_end = next.vid;

WARNING: line over 80 characters
#2097: FILE: drivers/net/dsa/mv88e6xxx.c:2097:
+  const struct switchdev_obj_port_vlan 
*vlan,

WARNING: suspect code indent for conditional statements (16, 32)
#2734: FILE: drivers/net/dsa/mv88e6xxx.c:2734:
+   if (mv88e6xxx_6352_family(ps) || mv88e6xxx_6351_family(ps) ||
[...]
+   reg |= PORT_CONTROL_EGRESS_ADD_TAG;

total: 0 errors, 3 warnings, 2 checks, 3805 lines checked

It also rebases and integrates changes sent by Ben Dooks [1]:

The driver has a number of functions that are not exported or
declared elsewhere, so make them static to avoid the following
warnings from sparse:

drivers/net/dsa/mv88e6xxx.c:113:5: warning: symbol 'mv88e6xxx_reg_read' was 
not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:167:5: warning: symbol 'mv88e6xxx_reg_write' 
was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:231:5: warning: symbol 'mv88e6xxx_set_addr' was 
not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:367:6: warning: symbol 
'mv88e6xxx_ppu_state_init' was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:3157:5: warning: symbol 
'mv88e6xxx_phy_page_read' was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:3169:5: warning: symbol 
'mv88e6xxx_phy_page_write' was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:3583:26: warning: symbol 
'mv88e6xxx_switch_driver' was not declared. Should it be static?
drivers/net/dsa/mv88e6xxx.c:3621:5: warning: symbol 'mv88e6xxx_probe' was 
not declared. Should it be static?

[1] http://patchwork.ozlabs.org/patch/632708/

Signed-off-by: Vivien Didelot 
Reviewed-by: Andrew Lunn 
---
 drivers/net/dsa/mv88e6xxx.c | 42 +++---
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index ee06055..1972ec5 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -111,7 +111,8 @@ static int _mv88e6xxx_reg_read(struct mv88e6xxx_priv_state 
*ps,
return ret;
 }
 
-int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr, int reg)
+static int mv88e6xxx_reg_read(struct mv88e6xxx_priv_state *ps, int addr,
+ int reg)
 {
int ret;
 
@@ -165,8 +166,8 @@ static int _mv88e6xxx_reg_write(struct mv88e6xxx_priv_state 
*ps, int addr,
return __mv88e6xxx_reg_write(ps->bus, ps->sw_addr, addr, reg, val);
 }
 
-int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
-   int reg, u16 val)
+static int mv88e6xxx_reg_write(struct mv88e6xxx_priv_state *ps, int addr,
+  int reg, u16 val)
 {
int ret;
 
@@ -229,7 +230,7 @@ static int mv88e6xxx_set_addr_indirect(struct dsa_switch 
*ds, u8 *addr)
return 0;
 }
 
-int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
+static int mv88e6xxx_set_addr(struct dsa_switch *ds, u8 *addr)
 {
struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 
@@ -370,7 +371,7 @@ static void mv88e6xxx_ppu_access_put(struct 
mv88e6xxx_priv_state *ps)
mutex_unlock(>ppu_mutex);
 }
 
-void mv88e6xxx_ppu_state_init(struct mv88e6xxx_priv_state *ps)
+static void mv88e6xxx_ppu_state_init(struct mv88e6xxx_priv_state *ps)
 {
mutex_init(>ppu_mutex);
INIT_WORK(>ppu_work, mv88e6xxx_ppu_reenable_work);
@@ -490,7 +491,7 @@ static void mv88e6xxx_adjust_link(struct dsa_switch *ds, 
int port,
 
reg |= PORT_PCS_CTRL_FORCE_LINK;
if (phydev->link)
-   reg |= PORT_PCS_CTRL_LINK_UP;
+   reg |= PORT_PCS_CTRL_LINK_UP;
 
if (mv88e6xxx_6065_family(ps) && phydev->speed > SPEED_100)
goto out;
@@ -1314,9 +1315,9 @@ static int _mv88e6xxx_port_state(struct 
mv88e6xxx_priv_state *ps, int port,
 * Blocking or Listening state.
 */
if ((oldstate == PORT_CONTROL_STATE_LEARNING ||
-oldstate == PORT_CONTROL_STATE_FORWARDING)
-   && (state == PORT_CONTROL_STATE_DISABLED ||
-   state == PORT_CONTROL_STATE_BLOCKING)) {
+oldstate == 

[PATCH V3] clocksource/drivers/bcm_kona: Convert init function to return error

[Daniel Lezcano]

The init functions do not return any error. They behave as the following:

  - panic, thus leading to a kernel crash while another timer may work and
   make the system boot up correctly

  or

  - print an error and let the caller unaware if the state of the system

Change that by converting the init functions to return an error conforming
to the CLOCKSOURCE_OF_RET prototype.

Proper error handling (rollback, errno value) will be changed later case
by case, thus this change just return back an error or success in the init
function.

Signed-off-by: Daniel Lezcano 
---
 drivers/clocksource/bcm_kona_timer.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/bcm_kona_timer.c 
b/drivers/clocksource/bcm_kona_timer.c
index e717e87..c251aa6 100644
--- a/drivers/clocksource/bcm_kona_timer.c
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -163,16 +163,11 @@ static struct irqaction kona_timer_irq = {
.handler = kona_timer_interrupt,
 };
 
-static void __init kona_timer_init(struct device_node *node)
+static int __init kona_timer_init(struct device_node *node)
 {
u32 freq;
struct clk *external_clk;
 
-   if (!of_device_is_available(node)) {
-   pr_info("Kona Timer v1 marked as disabled in device tree\n");
-   return;
-   }
-
external_clk = of_clk_get_by_name(node, NULL);
 
if (!IS_ERR(external_clk)) {
@@ -182,7 +177,7 @@ static void __init kona_timer_init(struct device_node *node)
arch_timer_rate = freq;
} else {
pr_err("Kona Timer v1 unable to determine clock-frequency");
-   return;
+   return -EINVAL;
}
 
/* Setup IRQ numbers */
@@ -196,11 +191,13 @@ static void __init kona_timer_init(struct device_node 
*node)
kona_timer_clockevents_init();
setup_irq(timers.tmr_irq, _timer_irq);
kona_timer_set_next_event((arch_timer_rate / HZ), NULL);
+
+   return 0;
 }
 
-CLOCKSOURCE_OF_DECLARE(brcm_kona, "brcm,kona-timer", kona_timer_init);
+CLOCKSOURCE_OF_DECLARE_RET(brcm_kona, "brcm,kona-timer", kona_timer_init);
 /*
  * bcm,kona-timer is deprecated by brcm,kona-timer
  * being kept here for driver compatibility
  */
-CLOCKSOURCE_OF_DECLARE(bcm_kona, "bcm,kona-timer", kona_timer_init);
+CLOCKSOURCE_OF_DECLARE_RET(bcm_kona, "bcm,kona-timer", kona_timer_init);
-- 
1.9.1

[PATCH V3] clocksource/drivers/bcm_kona: Convert init function to return error

[Daniel Lezcano]

The init functions do not return any error. They behave as the following:

  - panic, thus leading to a kernel crash while another timer may work and
   make the system boot up correctly

  or

  - print an error and let the caller unaware if the state of the system

Change that by converting the init functions to return an error conforming
to the CLOCKSOURCE_OF_RET prototype.

Proper error handling (rollback, errno value) will be changed later case
by case, thus this change just return back an error or success in the init
function.

Signed-off-by: Daniel Lezcano 
---
 drivers/clocksource/bcm_kona_timer.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/bcm_kona_timer.c 
b/drivers/clocksource/bcm_kona_timer.c
index e717e87..c251aa6 100644
--- a/drivers/clocksource/bcm_kona_timer.c
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -163,16 +163,11 @@ static struct irqaction kona_timer_irq = {
.handler = kona_timer_interrupt,
 };
 
-static void __init kona_timer_init(struct device_node *node)
+static int __init kona_timer_init(struct device_node *node)
 {
u32 freq;
struct clk *external_clk;
 
-   if (!of_device_is_available(node)) {
-   pr_info("Kona Timer v1 marked as disabled in device tree\n");
-   return;
-   }
-
external_clk = of_clk_get_by_name(node, NULL);
 
if (!IS_ERR(external_clk)) {
@@ -182,7 +177,7 @@ static void __init kona_timer_init(struct device_node *node)
arch_timer_rate = freq;
} else {
pr_err("Kona Timer v1 unable to determine clock-frequency");
-   return;
+   return -EINVAL;
}
 
/* Setup IRQ numbers */
@@ -196,11 +191,13 @@ static void __init kona_timer_init(struct device_node 
*node)
kona_timer_clockevents_init();
setup_irq(timers.tmr_irq, _timer_irq);
kona_timer_set_next_event((arch_timer_rate / HZ), NULL);
+
+   return 0;
 }
 
-CLOCKSOURCE_OF_DECLARE(brcm_kona, "brcm,kona-timer", kona_timer_init);
+CLOCKSOURCE_OF_DECLARE_RET(brcm_kona, "brcm,kona-timer", kona_timer_init);
 /*
  * bcm,kona-timer is deprecated by brcm,kona-timer
  * being kept here for driver compatibility
  */
-CLOCKSOURCE_OF_DECLARE(bcm_kona, "bcm,kona-timer", kona_timer_init);
+CLOCKSOURCE_OF_DECLARE_RET(bcm_kona, "bcm,kona-timer", kona_timer_init);
-- 
1.9.1

Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions

[Brian Norris]

Hi Lee,

On Mon, Jun 20, 2016 at 09:00:51AM +0100, Lee Jones wrote:
> On Fri, 17 Jun 2016, Doug Anderson wrote:
> > On Fri, Jun 17, 2016 at 1:06 AM, Lee Jones  wrote:
> > >> Probably the reason for all of these non-kernel-isms is that this
> > >> isn't a kernel file.  From the top of the file:
> > >>
> > >>  * NOTE: This file is copied verbatim from the ChromeOS EC Open Source
> > >>  * project in an attempt to make future updates easy to make.
> > >>
> > >> So the source of truth for this file is
> > >> .
> > >>
> > >> Someone could probably submit a CL to that project to make it a little
> > >> more kernel-ish and then we'd have to see if the EC team would accept
> > >> such changes...
> > >
> > > Hmmm... that kinda puts me in a difficult position.  Do I except
> > > non-kernel code, which does not conform to our stands?
> > 
> > What about if Brian made sure to just fully copy the latest version of
> > "cros_ec_commands.h" from the EC codebase and changed this commit
> > message to say:
> > 
> > Copy the latest version of "cros_ec_commands.h" from the Chrome OS EC
> > code base, which is the source of truth for this file.  See
> > .
> > 
> > From the commit message it would be clear that this is an external
> > file linked into the kernel for convenience.
> > 
> > 
> > > Naturally I'd be happier if you could try to make the code more
> > > 'kernely'.  The practices I mention above are still good ones, even if
> > > you're not writing kernel specific code.
> > 
> > In general requesting that code from outside the kernel conform to
> > "kerneldoc" seems like a bit of a stretch.  In general having some
> > type of parse-able format for comments is nice, but I could see that
> > in the Chrome OS EC codebase it would be a bit overkill.
> 
> It's unfortunate that kerneldoc is named so, since I think it's a nice
> way to write structure/function headers regardless of code base and
> not overkill at all.  It's certainly harder to convince !kernel code
> to use the format, or at least easier for others to push back due to
> the fact that is has 'kernel' in the name.
> 
> > Also: it would be awfully strange if we suddenly started changing the
> > coding convention of this file or we had half the file in one
> > convention and half in another.  The rest of this file is in EC
> > convention and it seems sane to keep it that way...
> 
> Right.  It's also a shame we're only catching this now.  Really we
> should have had this discussion in the first instance.

Well, I see your sign-off on 2 of 4 patches to this file :)

> Taking into consideration that this file is already in the kernel and
> that it's current format is also represented, I'm willing to keep
> adding to it.

Thanks, that seems quite reasonable.

> I would like to see an internal request to adopt
> so-called kerneldoc.  Not because I am wish to blindly push our
> standards to other code-bases, but because I am an advocate of the
> format in general.

As mentioned in another branch of this thread, such a request has
already been made (and it's public, so feel free to follow if you're
so inclined):

https://bugs.chromium.org/p/chromium/issues/detail?id=621123

It seems as if the firmware authors have already agreed that adopting a
more consistent style like kerneldoc would be a net postive. So you
should expect to eventually see a patch to update this file. But in the
meantime, I think it would make things sanest if we update the file
incrementally until that is done.

Regards,
Brian

Re: [PATCH v2 2/4] mfd: cros_ec: add EC_PWM function definitions

[Brian Norris]

Hi Lee,

On Mon, Jun 20, 2016 at 09:00:51AM +0100, Lee Jones wrote:
> On Fri, 17 Jun 2016, Doug Anderson wrote:
> > On Fri, Jun 17, 2016 at 1:06 AM, Lee Jones  wrote:
> > >> Probably the reason for all of these non-kernel-isms is that this
> > >> isn't a kernel file.  From the top of the file:
> > >>
> > >>  * NOTE: This file is copied verbatim from the ChromeOS EC Open Source
> > >>  * project in an attempt to make future updates easy to make.
> > >>
> > >> So the source of truth for this file is
> > >> .
> > >>
> > >> Someone could probably submit a CL to that project to make it a little
> > >> more kernel-ish and then we'd have to see if the EC team would accept
> > >> such changes...
> > >
> > > Hmmm... that kinda puts me in a difficult position.  Do I except
> > > non-kernel code, which does not conform to our stands?
> > 
> > What about if Brian made sure to just fully copy the latest version of
> > "cros_ec_commands.h" from the EC codebase and changed this commit
> > message to say:
> > 
> > Copy the latest version of "cros_ec_commands.h" from the Chrome OS EC
> > code base, which is the source of truth for this file.  See
> > .
> > 
> > From the commit message it would be clear that this is an external
> > file linked into the kernel for convenience.
> > 
> > 
> > > Naturally I'd be happier if you could try to make the code more
> > > 'kernely'.  The practices I mention above are still good ones, even if
> > > you're not writing kernel specific code.
> > 
> > In general requesting that code from outside the kernel conform to
> > "kerneldoc" seems like a bit of a stretch.  In general having some
> > type of parse-able format for comments is nice, but I could see that
> > in the Chrome OS EC codebase it would be a bit overkill.
> 
> It's unfortunate that kerneldoc is named so, since I think it's a nice
> way to write structure/function headers regardless of code base and
> not overkill at all.  It's certainly harder to convince !kernel code
> to use the format, or at least easier for others to push back due to
> the fact that is has 'kernel' in the name.
> 
> > Also: it would be awfully strange if we suddenly started changing the
> > coding convention of this file or we had half the file in one
> > convention and half in another.  The rest of this file is in EC
> > convention and it seems sane to keep it that way...
> 
> Right.  It's also a shame we're only catching this now.  Really we
> should have had this discussion in the first instance.

Well, I see your sign-off on 2 of 4 patches to this file :)

> Taking into consideration that this file is already in the kernel and
> that it's current format is also represented, I'm willing to keep
> adding to it.

Thanks, that seems quite reasonable.

> I would like to see an internal request to adopt
> so-called kerneldoc.  Not because I am wish to blindly push our
> standards to other code-bases, but because I am an advocate of the
> format in general.

As mentioned in another branch of this thread, such a request has
already been made (and it's public, so feel free to follow if you're
so inclined):

https://bugs.chromium.org/p/chromium/issues/detail?id=621123

It seems as if the firmware authors have already agreed that adopting a
more consistent style like kerneldoc would be a net postive. So you
should expect to eventually see a patch to update this file. But in the
meantime, I think it would make things sanest if we update the file
incrementally until that is done.

Regards,
Brian

Re: [PATCH v2 0/3] firmware: scpi: add device power domain support

[Kevin Hilman]

Sudeep Holla  writes:

> This series add support for SCPI based device device power state
> management using genpd.
>
> Regards,
> Sudeep
>
> v1[1]->v2:
>   - Fixed the endianness handling in scpi_device_get_power_state
> as spotted by Tixy
>   - Renamed scpi_pd.c to scpi_pm_domain.c as suggested by Ulf
>   - Added Mark's ack on DT binding
>
> [1] https://marc.info/?l=linux-kernel=146522850003483=2
>
> Sudeep Holla (3):
>   firmware: arm_scpi: add support for device power state management
>   Documentation: add DT bindings for ARM SCPI power domains
>   firmware: scpi: add device power domain support using genpd

Other than the couple nits on specific patches,

Reviewed-by: Kevin Hilman 

And just a[nother] reminder to be sure to keep things that are Juno
specific well described since we know other vendors will be (and are
being) "creative" in their implementations of SCPI.

Kevin

Re: [PATCH v2 0/3] firmware: scpi: add device power domain support

[Kevin Hilman]

Sudeep Holla  writes:

> This series add support for SCPI based device device power state
> management using genpd.
>
> Regards,
> Sudeep
>
> v1[1]->v2:
>   - Fixed the endianness handling in scpi_device_get_power_state
> as spotted by Tixy
>   - Renamed scpi_pd.c to scpi_pm_domain.c as suggested by Ulf
>   - Added Mark's ack on DT binding
>
> [1] https://marc.info/?l=linux-kernel=146522850003483=2
>
> Sudeep Holla (3):
>   firmware: arm_scpi: add support for device power state management
>   Documentation: add DT bindings for ARM SCPI power domains
>   firmware: scpi: add device power domain support using genpd

Other than the couple nits on specific patches,

Reviewed-by: Kevin Hilman 

And just a[nother] reminder to be sure to keep things that are Juno
specific well described since we know other vendors will be (and are
being) "creative" in their implementations of SCPI.

Kevin

Re: Canonical has own Ubuntu driver for ALPS 73 03 28 devices

[Anthony Wong]

On 20 June 2016 at 18:20, Pali Rohár  wrote:
>
> On Monday 20 June 2016 03:16:36 Christoph Hellwig wrote:
> > On Sun, Jun 19, 2016 at 01:43:46AM +0200, Pali Roh??r wrote:
> > > I do not understand it... Why Canonical is hidden and don't communicate
> > > with rest of world? Otherwise touchpads could work out-of-box on non
> > > Ubuntu systems too with mainline kernel.
> >
>
> It must be really frustrating for Ben and other people (me too) who in
> last months working on ALPS patches to support that touchpad as we know
> that Canonical already had some working code for that touchpad...

Hi Pali,

The fix in the DKMS package you referenced was not written by
Canonical but by ALPS, as it came from them I think it is reasonable
they send it to upstream, isn't it? As you can see the patch is non-trivial.

After we got the patch from ALPS, we had follow-up conversation a few
times with our contacts at Taiwan and asked if they would upstream it,
but unfortunately to no avail. I am as desperate as you if the fix
cannot land in mainline, which means many Linux users will not benefit
from it.  We also had opened this bug [1] for this particular issue,
there is nothing to hide. If there is any code written by us, we
happily submit them upstream.

[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1571530

> On Monday 20 June 2016 03:16:36 Christoph Hellwig wrote:
> > Because Canonical doesn't collaborate with the upstream community
> > in any meaninful way.  They've been a bad player since day 1 and will
> > always be.

This reminded me of this old thread [2]. How hard is it to run this
command in your kernel git tree?

$ git log --pretty=oneline --author=canonical

[2] https://www.spinics.net/lists/linux-bluetooth/msg47286.html

Thanks,
Anthony

Re: Canonical has own Ubuntu driver for ALPS 73 03 28 devices

[Anthony Wong]

On 20 June 2016 at 18:20, Pali Rohár  wrote:
>
> On Monday 20 June 2016 03:16:36 Christoph Hellwig wrote:
> > On Sun, Jun 19, 2016 at 01:43:46AM +0200, Pali Roh??r wrote:
> > > I do not understand it... Why Canonical is hidden and don't communicate
> > > with rest of world? Otherwise touchpads could work out-of-box on non
> > > Ubuntu systems too with mainline kernel.
> >
>
> It must be really frustrating for Ben and other people (me too) who in
> last months working on ALPS patches to support that touchpad as we know
> that Canonical already had some working code for that touchpad...

Hi Pali,

The fix in the DKMS package you referenced was not written by
Canonical but by ALPS, as it came from them I think it is reasonable
they send it to upstream, isn't it? As you can see the patch is non-trivial.

After we got the patch from ALPS, we had follow-up conversation a few
times with our contacts at Taiwan and asked if they would upstream it,
but unfortunately to no avail. I am as desperate as you if the fix
cannot land in mainline, which means many Linux users will not benefit
from it.  We also had opened this bug [1] for this particular issue,
there is nothing to hide. If there is any code written by us, we
happily submit them upstream.

[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1571530

> On Monday 20 June 2016 03:16:36 Christoph Hellwig wrote:
> > Because Canonical doesn't collaborate with the upstream community
> > in any meaninful way.  They've been a bad player since day 1 and will
> > always be.

This reminded me of this old thread [2]. How hard is it to run this
command in your kernel git tree?

$ git log --pretty=oneline --author=canonical

[2] https://www.spinics.net/lists/linux-bluetooth/msg47286.html

Thanks,
Anthony

Re: Canonical has own Ubuntu driver for ALPS 73 03 28 devices

[Pali Rohár]

On Monday 20 June 2016 19:37:57 Dmitry Torokhov wrote:
> On Tue, Jun 21, 2016 at 01:29:41AM +0800, Anthony Wong wrote:
> > On 20 June 2016 at 18:20, Pali Rohár  wrote:
> > > On Monday 20 June 2016 03:16:36 Christoph Hellwig wrote:
> > > > On Sun, Jun 19, 2016 at 01:43:46AM +0200, Pali Roh??r wrote:
> > > > > I do not understand it... Why Canonical is hidden and don't
> > > > > communicate with rest of world? Otherwise touchpads could
> > > > > work out-of-box on non Ubuntu systems too with mainline
> > > > > kernel.
> > > 
> > > It must be really frustrating for Ben and other people (me too)
> > > who in last months working on ALPS patches to support that
> > > touchpad as we know that Canonical already had some working code
> > > for that touchpad...
> > 
> > Hi Pali,
> > 
> > The fix in the DKMS package you referenced was not written by
> > Canonical but by ALPS, as it came from them I think it is
> > reasonable they send it to upstream, isn't it? As you can see the
> > patch is non-trivial.
> > 
> > After we got the patch from ALPS, we had follow-up conversation a
> > few times with our contacts at Taiwan and asked if they would
> > upstream it, but unfortunately to no avail. I am as desperate as
> > you if the fix cannot land in mainline, which means many Linux
> > users will not benefit from it.  We also had opened this bug [1]
> > for this particular issue, there is nothing to hide. If there is
> > any code written by us, we happily submit them upstream.
> 
> Why couldn't you guys send it upstream yourselves?

For ALPS 73 03 28 touchpad it is probably too late now, see:
http://thread.gmane.org/gmane.linux.kernel.input/49651

> > [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1571530
> > 
> > > On Monday 20 June 2016 03:16:36 Christoph Hellwig wrote:
> > > > Because Canonical doesn't collaborate with the upstream
> > > > community in any meaninful way.  They've been a bad player
> > > > since day 1 and will always be.
> > 
> > This reminded me of this old thread [2]. How hard is it to run this
> > command in your kernel git tree?
> > 
> > $ git log --pretty=oneline --author=canonical
> > 
> > [2] https://www.spinics.net/lists/linux-bluetooth/msg47286.html
> > 
> > Thanks,
> > Anthony

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: Canonical has own Ubuntu driver for ALPS 73 03 28 devices

[Pali Rohár]

On Monday 20 June 2016 19:37:57 Dmitry Torokhov wrote:
> On Tue, Jun 21, 2016 at 01:29:41AM +0800, Anthony Wong wrote:
> > On 20 June 2016 at 18:20, Pali Rohár  wrote:
> > > On Monday 20 June 2016 03:16:36 Christoph Hellwig wrote:
> > > > On Sun, Jun 19, 2016 at 01:43:46AM +0200, Pali Roh??r wrote:
> > > > > I do not understand it... Why Canonical is hidden and don't
> > > > > communicate with rest of world? Otherwise touchpads could
> > > > > work out-of-box on non Ubuntu systems too with mainline
> > > > > kernel.
> > > 
> > > It must be really frustrating for Ben and other people (me too)
> > > who in last months working on ALPS patches to support that
> > > touchpad as we know that Canonical already had some working code
> > > for that touchpad...
> > 
> > Hi Pali,
> > 
> > The fix in the DKMS package you referenced was not written by
> > Canonical but by ALPS, as it came from them I think it is
> > reasonable they send it to upstream, isn't it? As you can see the
> > patch is non-trivial.
> > 
> > After we got the patch from ALPS, we had follow-up conversation a
> > few times with our contacts at Taiwan and asked if they would
> > upstream it, but unfortunately to no avail. I am as desperate as
> > you if the fix cannot land in mainline, which means many Linux
> > users will not benefit from it.  We also had opened this bug [1]
> > for this particular issue, there is nothing to hide. If there is
> > any code written by us, we happily submit them upstream.
> 
> Why couldn't you guys send it upstream yourselves?

For ALPS 73 03 28 touchpad it is probably too late now, see:
http://thread.gmane.org/gmane.linux.kernel.input/49651

> > [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1571530
> > 
> > > On Monday 20 June 2016 03:16:36 Christoph Hellwig wrote:
> > > > Because Canonical doesn't collaborate with the upstream
> > > > community in any meaninful way.  They've been a bad player
> > > > since day 1 and will always be.
> > 
> > This reminded me of this old thread [2]. How hard is it to run this
> > command in your kernel git tree?
> > 
> > $ git log --pretty=oneline --author=canonical
> > 
> > [2] https://www.spinics.net/lists/linux-bluetooth/msg47286.html
> > 
> > Thanks,
> > Anthony

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH -v2 14/33] locking,m68k: Implement atomic_fetch_{add,sub,and,or,xor}()

[Andreas Schwab]

Peter Zijlstra  writes:

> Could either of you comment on the below patch?
>
> All atomic functions that return a value should imply full memory
> barrier semantics -- this very much includes a compiler barrier / memory
> clobber.

I wonder if it is possible to find a case where this makes a real
difference, ie. where the compiler erroneously reused a value due to
the missing barrier.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH -v2 14/33] locking,m68k: Implement atomic_fetch_{add,sub,and,or,xor}()

[Andreas Schwab]

Peter Zijlstra  writes:

> Could either of you comment on the below patch?
>
> All atomic functions that return a value should imply full memory
> barrier semantics -- this very much includes a compiler barrier / memory
> clobber.

I wonder if it is possible to find a case where this makes a real
difference, ie. where the compiler erroneously reused a value due to
the missing barrier.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v2 2/9] kexec_file: Generalize kexec_add_buffer.

[Thiago Jung Bauermann]

Am Montag, 20 Juni 2016, 10:26:05 schrieb Dave Young:
> kexec_buf should go within #ifdef for kexec file like struct
> purgatory_info
> 
> Other than that it looks good.

Great! Here it is.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


kexec_file: Generalize kexec_add_buffer.

Allow architectures to specify different memory walking functions for
kexec_add_buffer. Intel uses iomem to track reserved memory ranges,
but PowerPC uses the memblock subsystem.

Signed-off-by: Thiago Jung Bauermann 
Cc: Eric Biederman 
Cc: Dave Young 
Cc: ke...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index e8acb2b43dd9..3d91bcfc180d 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -146,7 +146,24 @@ struct kexec_file_ops {
kexec_verify_sig_t *verify_sig;
 #endif
 };
-#endif
+
+/*
+ * Keeps track of buffer parameters as provided by caller for requesting
+ * memory placement of buffer.
+ */
+struct kexec_buf {
+   struct kimage *image;
+   unsigned long mem;
+   unsigned long memsz;
+   unsigned long buf_align;
+   unsigned long buf_min;
+   unsigned long buf_max;
+   bool top_down;  /* allocate from top of memory hole */
+};
+
+int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
+  int (*func)(u64, u64, void *));
+#endif /* CONFIG_KEXEC_FILE */
 
 struct kimage {
kimage_entry_t head;
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b6eec7527e9f..b1f1f6402518 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -428,6 +428,27 @@ static int locate_mem_hole_callback(u64 start, u64 end, 
void *arg)
return locate_mem_hole_bottom_up(start, end, kbuf);
 }
 
+/**
+ * arch_kexec_walk_mem - call func(data) on free memory regions
+ * @kbuf:  Context info for the search. Also passed to @func.
+ * @func:  Function to call for each memory region.
+ *
+ * Return: The memory walk will stop when func returns a non-zero value
+ * and that value will be returned. If all free regions are visited without
+ * func returning non-zero, then zero will be returned.
+ */
+int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
+  int (*func)(u64, u64, void *))
+{
+   if (kbuf->image->type == KEXEC_TYPE_CRASH)
+   return walk_iomem_res_desc(crashk_res.desc,
+  IORESOURCE_SYSTEM_RAM | 
IORESOURCE_BUSY,
+  crashk_res.start, crashk_res.end,
+  kbuf, func);
+   else
+   return walk_system_ram_res(0, ULONG_MAX, kbuf, func);
+}
+
 /*
  * Helper function for placing a buffer in a kexec segment. This assumes
  * that kexec_mutex is held.
@@ -472,14 +493,7 @@ int kexec_add_buffer(struct kimage *image, char *buffer, 
unsigned long bufsz,
kbuf->top_down = top_down;
 
/* Walk the RAM ranges and allocate a suitable range for the buffer */
-   if (image->type == KEXEC_TYPE_CRASH)
-   ret = walk_iomem_res_desc(crashk_res.desc,
-   IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
-   crashk_res.start, crashk_res.end, kbuf,
-   locate_mem_hole_callback);
-   else
-   ret = walk_system_ram_res(0, -1, kbuf,
- locate_mem_hole_callback);
+   ret = arch_kexec_walk_mem(kbuf, locate_mem_hole_callback);
if (ret != 1) {
/* A suitable memory range could not be found for buffer */
return -EADDRNOTAVAIL;
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index eefd5bf960c2..4cef7e4706b0 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -20,20 +20,6 @@ struct kexec_sha_region {
unsigned long len;
 };
 
-/*
- * Keeps track of buffer parameters as provided by caller for requesting
- * memory placement of buffer.
- */
-struct kexec_buf {
-   struct kimage *image;
-   unsigned long mem;
-   unsigned long memsz;
-   unsigned long buf_align;
-   unsigned long buf_min;
-   unsigned long buf_max;
-   bool top_down;  /* allocate from top of memory hole */
-};
-
 void kimage_file_post_load_cleanup(struct kimage *image);
 #else /* CONFIG_KEXEC_FILE */
 static inline void kimage_file_post_load_cleanup(struct kimage *image) { }

Re: [PATCH v2 2/9] kexec_file: Generalize kexec_add_buffer.

[Thiago Jung Bauermann]

Am Montag, 20 Juni 2016, 10:26:05 schrieb Dave Young:
> kexec_buf should go within #ifdef for kexec file like struct
> purgatory_info
> 
> Other than that it looks good.

Great! Here it is.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


kexec_file: Generalize kexec_add_buffer.

Allow architectures to specify different memory walking functions for
kexec_add_buffer. Intel uses iomem to track reserved memory ranges,
but PowerPC uses the memblock subsystem.

Signed-off-by: Thiago Jung Bauermann 
Cc: Eric Biederman 
Cc: Dave Young 
Cc: ke...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index e8acb2b43dd9..3d91bcfc180d 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -146,7 +146,24 @@ struct kexec_file_ops {
kexec_verify_sig_t *verify_sig;
 #endif
 };
-#endif
+
+/*
+ * Keeps track of buffer parameters as provided by caller for requesting
+ * memory placement of buffer.
+ */
+struct kexec_buf {
+   struct kimage *image;
+   unsigned long mem;
+   unsigned long memsz;
+   unsigned long buf_align;
+   unsigned long buf_min;
+   unsigned long buf_max;
+   bool top_down;  /* allocate from top of memory hole */
+};
+
+int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
+  int (*func)(u64, u64, void *));
+#endif /* CONFIG_KEXEC_FILE */
 
 struct kimage {
kimage_entry_t head;
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b6eec7527e9f..b1f1f6402518 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -428,6 +428,27 @@ static int locate_mem_hole_callback(u64 start, u64 end, 
void *arg)
return locate_mem_hole_bottom_up(start, end, kbuf);
 }
 
+/**
+ * arch_kexec_walk_mem - call func(data) on free memory regions
+ * @kbuf:  Context info for the search. Also passed to @func.
+ * @func:  Function to call for each memory region.
+ *
+ * Return: The memory walk will stop when func returns a non-zero value
+ * and that value will be returned. If all free regions are visited without
+ * func returning non-zero, then zero will be returned.
+ */
+int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf,
+  int (*func)(u64, u64, void *))
+{
+   if (kbuf->image->type == KEXEC_TYPE_CRASH)
+   return walk_iomem_res_desc(crashk_res.desc,
+  IORESOURCE_SYSTEM_RAM | 
IORESOURCE_BUSY,
+  crashk_res.start, crashk_res.end,
+  kbuf, func);
+   else
+   return walk_system_ram_res(0, ULONG_MAX, kbuf, func);
+}
+
 /*
  * Helper function for placing a buffer in a kexec segment. This assumes
  * that kexec_mutex is held.
@@ -472,14 +493,7 @@ int kexec_add_buffer(struct kimage *image, char *buffer, 
unsigned long bufsz,
kbuf->top_down = top_down;
 
/* Walk the RAM ranges and allocate a suitable range for the buffer */
-   if (image->type == KEXEC_TYPE_CRASH)
-   ret = walk_iomem_res_desc(crashk_res.desc,
-   IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY,
-   crashk_res.start, crashk_res.end, kbuf,
-   locate_mem_hole_callback);
-   else
-   ret = walk_system_ram_res(0, -1, kbuf,
- locate_mem_hole_callback);
+   ret = arch_kexec_walk_mem(kbuf, locate_mem_hole_callback);
if (ret != 1) {
/* A suitable memory range could not be found for buffer */
return -EADDRNOTAVAIL;
diff --git a/kernel/kexec_internal.h b/kernel/kexec_internal.h
index eefd5bf960c2..4cef7e4706b0 100644
--- a/kernel/kexec_internal.h
+++ b/kernel/kexec_internal.h
@@ -20,20 +20,6 @@ struct kexec_sha_region {
unsigned long len;
 };
 
-/*
- * Keeps track of buffer parameters as provided by caller for requesting
- * memory placement of buffer.
- */
-struct kexec_buf {
-   struct kimage *image;
-   unsigned long mem;
-   unsigned long memsz;
-   unsigned long buf_align;
-   unsigned long buf_min;
-   unsigned long buf_max;
-   bool top_down;  /* allocate from top of memory hole */
-};
-
 void kimage_file_post_load_cleanup(struct kimage *image);
 #else /* CONFIG_KEXEC_FILE */
 static inline void kimage_file_post_load_cleanup(struct kimage *image) { }

Re: [v3,1/4] mfd: cros_ec: Add cros_ec_cmd_xfer_status helper

[Brian Norris]

Hi,

On Mon, Jun 20, 2016 at 09:46:57AM -0400, Javier Martinez Canillas wrote:
> On 06/18/2016 01:09 PM, Guenter Roeck wrote:
> > On 06/17/2016 06:08 PM, Brian Norris wrote:
> >> On Fri, Jun 17, 2016 at 02:41:51PM -0700, Guenter Roeck wrote:
> >>> On Fri, Jun 17, 2016 at 12:58:12PM -0700, Brian Norris wrote:
>  +int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
>  +struct cros_ec_command *msg)
>  +{
>  +int ret;
>  +
>  +ret = cros_ec_cmd_xfer(ec_dev, msg);
>  +if (ret < 0)
>  +dev_err(ec_dev->dev, "Command xfer error (err:%d)\n", ret);
>  +else if (msg->result != EC_RES_SUCCESS)
>  +return -EECRESULT - msg->result;
> >>>
> >>> I have been wondering about the error return codes here, and if they 
> >>> should be
> >>> converted to standard Linux error codes. For example, I just hit error 
> >>> -1003
> >>> with a driver I am working on. This translates to EC_RES_INVALID_PARAM, 
> >>> or,
> >>> in Linux terms, -EINVAL. I think it would be better to use standard error
> >>> codes, especially since some of the errors are logged.
> >>
> 
> Agreed, specially since drivers may (wrongly) propagate whatever is returned
> by this function to higher layers where the ChromeOS EC firmware error codes
> makes no sense. So that will be a bug and can increase the cognitive load of
> getting some weird error codes in core kernel code and developers may wonder
> from where those came from until finally find that a EC driver returned that.

Agreed, I suppose. It's probably best to make it unlikely other that
"client" drivers of cros-ec will blindly propagate nonstandard error
codes.

> >> How do you propose we do that? Do all of the following become EINVAL?
> >>
> 
> Yes, I would just do that.
> 
> The idea of this helper is to remove duplicated code and AFAICT what most EC
> drivers do is something similar to the following:
> 
> ret = cros_ec_cmd_xfer(ec, msg);
> if (ret < 0)
>   return ret;
> 
> if (msg->result != EC_RES_SUCCESS) {
> dev_dbg(ec->dev, "EC result %d\n", msg->result);
> return -EINVAL;
> }
> 
> So in practice what most drivers really care is if the result was successful
> or not, I don't see specific EC error handling in the EC drivers.  The real
> EC error code is still in the message anyways so drivers that do cares about
> the real EC error can look at msg->result instead.

Ah, well that last point is a nice reminder I guess, although that still
doesn't fit the way cros_ec_num_pwms() uses this API. But I can try to
refactor it to fit; cros_ec_num_pwms() will need to get two return
codes from cros_ec_pwm_get_duty() -- uglier, IMO, but doable.

> >>  EC_RES_INVALID_COMMAND
> > 
> > -EOPNOTSUPP
> > 
> >>  EC_RES_INVALID_PARAM
> > 
> > -EINVAL or -EBADMSG
> > 
> >>  EC_RES_INVALID_VERSION
> > 
> > -EPROTO or -EBADR or -EBADE or -EBADRQC or -EPROTOOPT
> > 
> >>  EC_RES_INVALID_HEADER
> > 
> > -EPROTO or -EBADR or -EBADE
> > 
> > Doesn't look that bad to me. Also, the raw error could still be logged,
> > for example with dev_dbg().
> >
> 
> Yes, I think that adding a dev_dbg() with the real EC error code should
> be enough, that's basically what drivers do since they can't propagate
> the EC error to higher layers anyways.

I'll take a look at adding an error code translation table when I get a
chance. Hopefully that doesn't delay the others who are planning to use
this API shortly...

Brian

Re: [v3,1/4] mfd: cros_ec: Add cros_ec_cmd_xfer_status helper

[Brian Norris]

Hi,

On Mon, Jun 20, 2016 at 09:46:57AM -0400, Javier Martinez Canillas wrote:
> On 06/18/2016 01:09 PM, Guenter Roeck wrote:
> > On 06/17/2016 06:08 PM, Brian Norris wrote:
> >> On Fri, Jun 17, 2016 at 02:41:51PM -0700, Guenter Roeck wrote:
> >>> On Fri, Jun 17, 2016 at 12:58:12PM -0700, Brian Norris wrote:
>  +int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
>  +struct cros_ec_command *msg)
>  +{
>  +int ret;
>  +
>  +ret = cros_ec_cmd_xfer(ec_dev, msg);
>  +if (ret < 0)
>  +dev_err(ec_dev->dev, "Command xfer error (err:%d)\n", ret);
>  +else if (msg->result != EC_RES_SUCCESS)
>  +return -EECRESULT - msg->result;
> >>>
> >>> I have been wondering about the error return codes here, and if they 
> >>> should be
> >>> converted to standard Linux error codes. For example, I just hit error 
> >>> -1003
> >>> with a driver I am working on. This translates to EC_RES_INVALID_PARAM, 
> >>> or,
> >>> in Linux terms, -EINVAL. I think it would be better to use standard error
> >>> codes, especially since some of the errors are logged.
> >>
> 
> Agreed, specially since drivers may (wrongly) propagate whatever is returned
> by this function to higher layers where the ChromeOS EC firmware error codes
> makes no sense. So that will be a bug and can increase the cognitive load of
> getting some weird error codes in core kernel code and developers may wonder
> from where those came from until finally find that a EC driver returned that.

Agreed, I suppose. It's probably best to make it unlikely other that
"client" drivers of cros-ec will blindly propagate nonstandard error
codes.

> >> How do you propose we do that? Do all of the following become EINVAL?
> >>
> 
> Yes, I would just do that.
> 
> The idea of this helper is to remove duplicated code and AFAICT what most EC
> drivers do is something similar to the following:
> 
> ret = cros_ec_cmd_xfer(ec, msg);
> if (ret < 0)
>   return ret;
> 
> if (msg->result != EC_RES_SUCCESS) {
> dev_dbg(ec->dev, "EC result %d\n", msg->result);
> return -EINVAL;
> }
> 
> So in practice what most drivers really care is if the result was successful
> or not, I don't see specific EC error handling in the EC drivers.  The real
> EC error code is still in the message anyways so drivers that do cares about
> the real EC error can look at msg->result instead.

Ah, well that last point is a nice reminder I guess, although that still
doesn't fit the way cros_ec_num_pwms() uses this API. But I can try to
refactor it to fit; cros_ec_num_pwms() will need to get two return
codes from cros_ec_pwm_get_duty() -- uglier, IMO, but doable.

> >>  EC_RES_INVALID_COMMAND
> > 
> > -EOPNOTSUPP
> > 
> >>  EC_RES_INVALID_PARAM
> > 
> > -EINVAL or -EBADMSG
> > 
> >>  EC_RES_INVALID_VERSION
> > 
> > -EPROTO or -EBADR or -EBADE or -EBADRQC or -EPROTOOPT
> > 
> >>  EC_RES_INVALID_HEADER
> > 
> > -EPROTO or -EBADR or -EBADE
> > 
> > Doesn't look that bad to me. Also, the raw error could still be logged,
> > for example with dev_dbg().
> >
> 
> Yes, I think that adding a dev_dbg() with the real EC error code should
> be enough, that's basically what drivers do since they can't propagate
> the EC error to higher layers anyways.

I'll take a look at adding an error code translation table when I get a
chance. Hopefully that doesn't delay the others who are planning to use
this API shortly...

Brian

Re: [RFC 00/18] Present useful limits to user

[Austin S. Hemmelgarn]

On 2016-06-18 10:45, Konstantin Khlebnikov wrote:

On Wed, Jun 15, 2016 at 5:47 PM, Austin S. Hemmelgarn
 wrote:

On 2016-06-14 15:03, Konstantin Khlebnikov wrote:


I don't like the idea of this patchset.

All limitations are context dependent and that context changes rapidly.
You'll never dump enough information for predicting future errors or
investigating reson of errors in past. You could try to reproduce all
kernel logic but model always will be aproximate.


It's still better than what we have now, and there is one particular use for
the cgroup stuff that I find intriguing, you can create a cgroup, populate
it, set no limits, and then run a simulated workload against it and see how
it reacts.  This in general will probably provide a better starting point
for what to actually set the limits to than just making an arbitrary guess.
Certain applications in particular come to mind which will just hang when
they can't start a new thread or process (Dropbox is particularly guilty of
this).  In such cases, setting the limit too low doesn't result in a crash,
it results in the program just not appearing to work yet still running
otherwise normally.

In general, I could see the rlimit stuff being in the same situation, it's
not for figuring out why something failed (good software will tell you
somewhere), but figuring out limits so it doesn't fail but still is
reasonably contained.  A lot of things that seem at face value like they
shouldn't need specific exceptions to limits do.  Most normal users probably
wouldn't guess that acpid needs a RLIMIT_NPROC count of at least 4 or more
to work with the default rules.  Similarly, there's probably not many normal
users who know that the Dropbox daemon spawns an insanely large thread pool
and preallocates significant amounts of memory and will just hang if either
of these fail.  By having a way to get running max counts of resource usage,
it makes it easier for people to know what the minimum limit they need to
put on something is.


Rlimits work only if resource usage could be estimated apriori.
They allows app limit itself to prevent failures is something goes wrong.
And yet many apps allow the _user_ to specify rlimits.  Avahi has the 
option for the user to set every single rlimit, ntpd (the reference 
implementation) lets the user configure MEMLOCK, and quite a few other 
daemons I've seen that are very widely used allow similar manual 
configuration of limits.  Most of these are network service daemons, 
which _can't_ reasonably limit themselves, because they can't know what 
type of workload they'll run against.


Rlimits are useless for controlling resource destribition: just use
cgroups for that.
The only rlimit that has a cgroup specifically for managing it is NPROC. 
 There's a bunch of memory ones that can't be individually controlled 
in the memcg.  MEMLOCK is actually pretty widely used from what I've 
seen, but there is no way to control it at all with cgroups right now. 
NOFILE, LOCKS, FSIZE, and CORE all deal with the filesystem and have no 
cgroup that controls such resources (the only two that might be useful 
this way are NOFILE and LOCKS, but I doubt that those will get in, 
because they technically tie in with the kernel memory accounting in 
memcg).  NICE and RTPRIO are nonsensical in a cgroup context, although I 
don't think I've ever talked to anyone who actually uses them.  CPU and 
RTTIME have no equivalent in cgroups, but could in theory be tacked onto 
the cpu controller, but they haven't been and until that happens, people 
still have to use them instead of cgroups.




If you want to track origin of failures in user space applications when it
hits
some limit you should track errors. For example rlimits and other
limitation
subsystems could provide resonable amount of tracepoints which could
tell what exactly happened before error. If you need highwater of some
values you could track it in userspace, or maybe tracing subsystem could
provide postpocessing for tracepoint parameters. Anyway, systemtap and
other monsters can do this right now.


Userspace tracking of some things just isn't practical.  Take RLIMIT_NPROC
for example.  There's not really any reliable way to track this from
userspace without modifying the process which is being tracked, which is not
a user friendly way of doing things, and in some cases is functionally
impossible for an end user to do.


You cannot get reliable upper bound for nr-proc from black box observations.
Highwater mark is very racy - tiny timing shifts can change it drammaticaly.
You can't get a perfectly reliable upper bound for any type of resource 
usage with just black box observations, period.  You also can't do so 
with tracing without some significant secondary work either for _exactly 
the same reason_.  The thing to remember though is that in a majority of 
cases, what most people need is simply a reasonable estimate which is 
guaranteed to not be below the actual usage. 

Re: [RFC 00/18] Present useful limits to user

[Austin S. Hemmelgarn]

On 2016-06-18 10:45, Konstantin Khlebnikov wrote:

On Wed, Jun 15, 2016 at 5:47 PM, Austin S. Hemmelgarn
 wrote:

On 2016-06-14 15:03, Konstantin Khlebnikov wrote:


I don't like the idea of this patchset.

All limitations are context dependent and that context changes rapidly.
You'll never dump enough information for predicting future errors or
investigating reson of errors in past. You could try to reproduce all
kernel logic but model always will be aproximate.


It's still better than what we have now, and there is one particular use for
the cgroup stuff that I find intriguing, you can create a cgroup, populate
it, set no limits, and then run a simulated workload against it and see how
it reacts.  This in general will probably provide a better starting point
for what to actually set the limits to than just making an arbitrary guess.
Certain applications in particular come to mind which will just hang when
they can't start a new thread or process (Dropbox is particularly guilty of
this).  In such cases, setting the limit too low doesn't result in a crash,
it results in the program just not appearing to work yet still running
otherwise normally.

In general, I could see the rlimit stuff being in the same situation, it's
not for figuring out why something failed (good software will tell you
somewhere), but figuring out limits so it doesn't fail but still is
reasonably contained.  A lot of things that seem at face value like they
shouldn't need specific exceptions to limits do.  Most normal users probably
wouldn't guess that acpid needs a RLIMIT_NPROC count of at least 4 or more
to work with the default rules.  Similarly, there's probably not many normal
users who know that the Dropbox daemon spawns an insanely large thread pool
and preallocates significant amounts of memory and will just hang if either
of these fail.  By having a way to get running max counts of resource usage,
it makes it easier for people to know what the minimum limit they need to
put on something is.


Rlimits work only if resource usage could be estimated apriori.
They allows app limit itself to prevent failures is something goes wrong.
And yet many apps allow the _user_ to specify rlimits.  Avahi has the 
option for the user to set every single rlimit, ntpd (the reference 
implementation) lets the user configure MEMLOCK, and quite a few other 
daemons I've seen that are very widely used allow similar manual 
configuration of limits.  Most of these are network service daemons, 
which _can't_ reasonably limit themselves, because they can't know what 
type of workload they'll run against.


Rlimits are useless for controlling resource destribition: just use
cgroups for that.
The only rlimit that has a cgroup specifically for managing it is NPROC. 
 There's a bunch of memory ones that can't be individually controlled 
in the memcg.  MEMLOCK is actually pretty widely used from what I've 
seen, but there is no way to control it at all with cgroups right now. 
NOFILE, LOCKS, FSIZE, and CORE all deal with the filesystem and have no 
cgroup that controls such resources (the only two that might be useful 
this way are NOFILE and LOCKS, but I doubt that those will get in, 
because they technically tie in with the kernel memory accounting in 
memcg).  NICE and RTPRIO are nonsensical in a cgroup context, although I 
don't think I've ever talked to anyone who actually uses them.  CPU and 
RTTIME have no equivalent in cgroups, but could in theory be tacked onto 
the cpu controller, but they haven't been and until that happens, people 
still have to use them instead of cgroups.




If you want to track origin of failures in user space applications when it
hits
some limit you should track errors. For example rlimits and other
limitation
subsystems could provide resonable amount of tracepoints which could
tell what exactly happened before error. If you need highwater of some
values you could track it in userspace, or maybe tracing subsystem could
provide postpocessing for tracepoint parameters. Anyway, systemtap and
other monsters can do this right now.


Userspace tracking of some things just isn't practical.  Take RLIMIT_NPROC
for example.  There's not really any reliable way to track this from
userspace without modifying the process which is being tracked, which is not
a user friendly way of doing things, and in some cases is functionally
impossible for an end user to do.


You cannot get reliable upper bound for nr-proc from black box observations.
Highwater mark is very racy - tiny timing shifts can change it drammaticaly.
You can't get a perfectly reliable upper bound for any type of resource 
usage with just black box observations, period.  You also can't do so 
with tracing without some significant secondary work either for _exactly 
the same reason_.  The thing to remember though is that in a majority of 
cases, what most people need is simply a reasonable estimate which is 
guaranteed to not be below the actual usage.  They don't care 

Re: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS mode

[Dmitry Torokhov]

On Tue, Jun 07, 2016 at 09:34:09PM +0800, Chris Chiu wrote:
> When performing a warm reboot from a system which does not correctly
> support ELAN I2C touchpads, the touchpad will sometimes enter standard
> mouse mode, cursor then never respond to touchpad event, and making the
> driver discard the HID reports and flood dmesg with following error
> messages.
> "elan_i2c i2c-ELAN1000:00: invalid report id data (1)"
> 
> This change is from ELAN's correction. It needs 200ms delay before
> set_mode() so that the mode setting will correctly take effect.

KT, is this feasible?

> 
> Signed-off-by: Chris Chiu 
> ---
>  drivers/input/mouse/elan_i2c_core.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..95080f9 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -210,18 +210,20 @@ static int __elan_initialize(struct elan_tp_data *data)
>   return error;
>   }
>  
> - data->mode |= ETP_ENABLE_ABS;
> - error = data->ops->set_mode(client, data->mode);
> + error = data->ops->sleep_control(client, false);
>   if (error) {
>   dev_err(>dev,
> - "failed to switch to absolute mode: %d\n", error);
> + "failed to wake device up: %d\n", error);
>   return error;
>   }
>  
> - error = data->ops->sleep_control(client, false);
> + msleep(200);
> +
> + data->mode |= ETP_ENABLE_ABS;
> + error = data->ops->set_mode(client, data->mode);
>   if (error) {
>   dev_err(>dev,
> - "failed to wake device up: %d\n", error);
> + "failed to switch to absolute mode: %d\n", error);
>   return error;
>   }
>  
> -- 
> 2.1.4
> 

Thanks.

-- 
Dmitry

Re: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS mode

[Dmitry Torokhov]

On Tue, Jun 07, 2016 at 09:34:09PM +0800, Chris Chiu wrote:
> When performing a warm reboot from a system which does not correctly
> support ELAN I2C touchpads, the touchpad will sometimes enter standard
> mouse mode, cursor then never respond to touchpad event, and making the
> driver discard the HID reports and flood dmesg with following error
> messages.
> "elan_i2c i2c-ELAN1000:00: invalid report id data (1)"
> 
> This change is from ELAN's correction. It needs 200ms delay before
> set_mode() so that the mode setting will correctly take effect.

KT, is this feasible?

> 
> Signed-off-by: Chris Chiu 
> ---
>  drivers/input/mouse/elan_i2c_core.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..95080f9 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -210,18 +210,20 @@ static int __elan_initialize(struct elan_tp_data *data)
>   return error;
>   }
>  
> - data->mode |= ETP_ENABLE_ABS;
> - error = data->ops->set_mode(client, data->mode);
> + error = data->ops->sleep_control(client, false);
>   if (error) {
>   dev_err(>dev,
> - "failed to switch to absolute mode: %d\n", error);
> + "failed to wake device up: %d\n", error);
>   return error;
>   }
>  
> - error = data->ops->sleep_control(client, false);
> + msleep(200);
> +
> + data->mode |= ETP_ENABLE_ABS;
> + error = data->ops->set_mode(client, data->mode);
>   if (error) {
>   dev_err(>dev,
> - "failed to wake device up: %d\n", error);
> + "failed to switch to absolute mode: %d\n", error);
>   return error;
>   }
>  
> -- 
> 2.1.4
> 

Thanks.

-- 
Dmitry

Re: [PATCH] devfreq_cooling: pass a pointer to devfreq in the power model callbacks

[Javi Merino]

Eduardo, Rui,

On Fri, Jun 03, 2016 at 10:25:31AM +0100, Javi Merino wrote:
> When the devfreq cooling device was designed, it was an oversight not to
> pass a pointer to the struct devfreq as the first parameters of the
> callbacks.  The design patterns of the kernel suggest it for a good
> reason.
> 
> By passing a pointer to struct devfreq, the driver can register one
> function that works with multiple devices.  With the current
> implementation, a driver that can work with multiple devices has to
> create multiple copies of the same function with different parameters so
> that each devfreq_cooling_device can use the appropriate one.  By
> passing a pointer to struct devfreq, the driver can identify which
> device it's referring to.
> 
> Cc: Zhang Rui 
> Cc: Eduardo Valentin 
> Reviewed-by: Punit Agrawal 
> Signed-off-by: Javi Merino 
> ---
>  drivers/thermal/devfreq_cooling.c | 5 +++--
>  include/linux/devfreq_cooling.h   | 6 --
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/thermal/devfreq_cooling.c 
> b/drivers/thermal/devfreq_cooling.c
> index 01f0015f80dc..c549d83a0c7d 100644
> --- a/drivers/thermal/devfreq_cooling.c
> +++ b/drivers/thermal/devfreq_cooling.c
> @@ -238,7 +238,7 @@ get_static_power(struct devfreq_cooling_device *dfc, 
> unsigned long freq)
>   return 0;
>   }
>  
> - return dfc->power_ops->get_static_power(voltage);
> + return dfc->power_ops->get_static_power(df, voltage);
>  }
>  
>  /**
> @@ -262,7 +262,8 @@ get_dynamic_power(struct devfreq_cooling_device *dfc, 
> unsigned long freq,
>   struct devfreq_cooling_power *dfc_power = dfc->power_ops;
>  
>   if (dfc_power->get_dynamic_power)
> - return dfc_power->get_dynamic_power(freq, voltage);
> + return dfc_power->get_dynamic_power(dfc->devfreq, freq,
> + voltage);
>  
>   freq_mhz = freq / 100;
>   power = (u64)dfc_power->dyn_power_coeff * freq_mhz * voltage * voltage;
> diff --git a/include/linux/devfreq_cooling.h b/include/linux/devfreq_cooling.h
> index 7adf6cc4b305..959714e93e5b 100644
> --- a/include/linux/devfreq_cooling.h
> +++ b/include/linux/devfreq_cooling.h
> @@ -37,8 +37,10 @@
>   *   @dyn_power_coeff * frequency * voltage^2
>   */
>  struct devfreq_cooling_power {
> - unsigned long (*get_static_power)(unsigned long voltage);
> - unsigned long (*get_dynamic_power)(unsigned long freq,
> + unsigned long (*get_static_power)(struct devfreq *devfreq,
> +   unsigned long voltage);
> + unsigned long (*get_dynamic_power)(struct devfreq *devfreq,
> +unsigned long freq,
>  unsigned long voltage);
>   unsigned long dyn_power_coeff;
>  };

If there are no objections, can you pick this for the next merge
window?

Thanks,
Javi

Re: [PATCH] devfreq_cooling: pass a pointer to devfreq in the power model callbacks

[Javi Merino]

Eduardo, Rui,

On Fri, Jun 03, 2016 at 10:25:31AM +0100, Javi Merino wrote:
> When the devfreq cooling device was designed, it was an oversight not to
> pass a pointer to the struct devfreq as the first parameters of the
> callbacks.  The design patterns of the kernel suggest it for a good
> reason.
> 
> By passing a pointer to struct devfreq, the driver can register one
> function that works with multiple devices.  With the current
> implementation, a driver that can work with multiple devices has to
> create multiple copies of the same function with different parameters so
> that each devfreq_cooling_device can use the appropriate one.  By
> passing a pointer to struct devfreq, the driver can identify which
> device it's referring to.
> 
> Cc: Zhang Rui 
> Cc: Eduardo Valentin 
> Reviewed-by: Punit Agrawal 
> Signed-off-by: Javi Merino 
> ---
>  drivers/thermal/devfreq_cooling.c | 5 +++--
>  include/linux/devfreq_cooling.h   | 6 --
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/thermal/devfreq_cooling.c 
> b/drivers/thermal/devfreq_cooling.c
> index 01f0015f80dc..c549d83a0c7d 100644
> --- a/drivers/thermal/devfreq_cooling.c
> +++ b/drivers/thermal/devfreq_cooling.c
> @@ -238,7 +238,7 @@ get_static_power(struct devfreq_cooling_device *dfc, 
> unsigned long freq)
>   return 0;
>   }
>  
> - return dfc->power_ops->get_static_power(voltage);
> + return dfc->power_ops->get_static_power(df, voltage);
>  }
>  
>  /**
> @@ -262,7 +262,8 @@ get_dynamic_power(struct devfreq_cooling_device *dfc, 
> unsigned long freq,
>   struct devfreq_cooling_power *dfc_power = dfc->power_ops;
>  
>   if (dfc_power->get_dynamic_power)
> - return dfc_power->get_dynamic_power(freq, voltage);
> + return dfc_power->get_dynamic_power(dfc->devfreq, freq,
> + voltage);
>  
>   freq_mhz = freq / 100;
>   power = (u64)dfc_power->dyn_power_coeff * freq_mhz * voltage * voltage;
> diff --git a/include/linux/devfreq_cooling.h b/include/linux/devfreq_cooling.h
> index 7adf6cc4b305..959714e93e5b 100644
> --- a/include/linux/devfreq_cooling.h
> +++ b/include/linux/devfreq_cooling.h
> @@ -37,8 +37,10 @@
>   *   @dyn_power_coeff * frequency * voltage^2
>   */
>  struct devfreq_cooling_power {
> - unsigned long (*get_static_power)(unsigned long voltage);
> - unsigned long (*get_dynamic_power)(unsigned long freq,
> + unsigned long (*get_static_power)(struct devfreq *devfreq,
> +   unsigned long voltage);
> + unsigned long (*get_dynamic_power)(struct devfreq *devfreq,
> +unsigned long freq,
>  unsigned long voltage);
>   unsigned long dyn_power_coeff;
>  };

If there are no objections, can you pick this for the next merge
window?

Thanks,
Javi

Re: [PATCH v2 1/2] remoteproc: qcom: Driver for the self-authenticating Hexagon v5

[Bjorn Andersson]

On Mon 20 Jun 07:48 PDT 2016, Srinivas Kandagatla wrote:

> Thanks Bjorn for this patch,
> 
> I will start playing with patch soon, but here are few comments.
> 
> On 17/06/16 18:17, Bjorn Andersson wrote:
> >From: Bjorn Andersson 
> >
> >This initial hack powers the q6v5, boots and authenticate the mba and
> >use that to load the mdt and subsequent bXX files.
> >
> >Signed-off-by: Bjorn Andersson 
> >Signed-off-by: Bjorn Andersson 
> >---
> >
> >Changes since v1:
> >- Corrected boot address in relocation case
> >- Using rproc_da_to_va() to clean up mdt loader api
> >- Dynamically allocating scratch space for mdt verification
> >
> >  drivers/remoteproc/Kconfig   |  12 +
> >  drivers/remoteproc/Makefile  |   2 +
> >  drivers/remoteproc/qcom_mdt_loader.c | 166 +++
> >  drivers/remoteproc/qcom_mdt_loader.h |  13 +
> >  drivers/remoteproc/qcom_q6v5_pil.c   | 914 
> > +++
> 
> We should probably split this patch into two one for mdt loader and other
> for pil.
> 

I did consider it, as it's currently out for review in two different
patch sets, but I don't want to add dead code so it shouldn't be merged
on its own.

> Also checkpatch reports:
> 
> total: 1 errors, 28 warnings, 1117 lines checked
> 

I'll review that again.

[..]
> >diff --git a/drivers/remoteproc/qcom_mdt_loader.c 
> >b/drivers/remoteproc/qcom_mdt_loader.c
> >new file mode 100644
> >index ..4efeda908d9a
> >--- /dev/null
> >+++ b/drivers/remoteproc/qcom_mdt_loader.c
> >@@ -0,0 +1,166 @@
> >+/*
> >+ * Qualcomm Peripheral Image Loader
> >+ *
> >+ * Copyright (C) 2016 Linaro Ltd
> >+ * Copyright (C) 2015 Sony Mobile Communications Inc
> >+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> >+ *
> >+ * 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.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+ * GNU General Public License for more details.
> >+ */
> >+
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> 
> ??
> 

Leftover from being TZ-only.

> >+#include 
> >+#include 
> ??
> 

For kfree() ?

> >+
> >+#include "remoteproc_internal.h"
> >+#include "qcom_mdt_loader.h"
> >+
> >+/**
> >+ * qcom_mdt_find_rsc_table() - provide dummy resource table for remoteproc
> >+ * @rproc:  remoteproc handle
> >+ * @fw: firmware header
> >+ * @tablesz:outgoing size of the table
> >+ *
> >+ * Returns a dummy table.
> >+ */
> >+struct resource_table *qcom_mdt_find_rsc_table(struct rproc *rproc,
> >+   const struct firmware *fw,
> >+   int *tablesz)
> >+{
> >+static struct resource_table table = { .ver = 1, };
> >+
> >+*tablesz = sizeof(table);
> >+return 
> >+}
> >+EXPORT_SYMBOL_GPL(qcom_mdt_find_rsc_table);
> >+
> >+int qcom_mdt_parse(const struct firmware *fw, phys_addr_t *fw_addr, size_t 
> >*fw_size, bool *fw_relocate)
> Missing doc for this function?
> 

Yes, that should be added.

> >+{
[..]
> >+}
> >+EXPORT_SYMBOL_GPL(qcom_mdt_parse);
> >+
> >+/**
> >+ * qcom_mdt_load() - load the firmware which header is defined in fw
> >+ * @rproc:  rproc handle
> >+ * @pas_id: PAS identifier to load this firmware into
> 
> ??
> 

Sorry, another leftover. Thanks for spotting that.

> >+ * @fw: frimware object for the header
> 
> s/frimware/firmware
> 

Thanks.

> >+ *
> >+ * Returns 0 on success, negative errno otherwise.
> >+ */
> >+int qcom_mdt_load(struct rproc *rproc,
> >+  const struct firmware *fw,
> >+  const char *firmware)
> >+{
[..]
> >+}
> >+EXPORT_SYMBOL_GPL(qcom_mdt_load);
> 
> Module Licence info?
> 

Didn't consider it a module on it's own, but you're right.

[..]
> >diff --git a/drivers/remoteproc/qcom_q6v5_pil.c 
> >b/drivers/remoteproc/qcom_q6v5_pil.c
[..]
> >+
> >+static void q6v5_regulator_disable(struct q6v5 *qproc)
> >+{
> >+regulator_bulk_disable(ARRAY_SIZE(qproc->supply), qproc->supply);
> >+regulator_set_voltage(qproc->supply[Q6V5_SUPPLY_CX].consumer, 0, 
> >INT_MAX);
> >+regulator_set_voltage(qproc->supply[Q6V5_SUPPLY_MX].consumer, 0, 
> >INT_MAX);
> 
> we disable the regulators and then set voltage why?
> I think these should be moved to q6v5_regulator_enable() unless am missing
> something here.
> 

Because during enable we reduce the valid range of voltages on the SMPS,
regardless of it being enabled or not. So I need to broaden that vote
for the application CPU to be allowed to vote for the lower voltages
again.


cx is however supposed to be a corner, so not sure if I should touch

Re: [PATCH v2 1/2] remoteproc: qcom: Driver for the self-authenticating Hexagon v5

[Bjorn Andersson]

On Mon 20 Jun 07:48 PDT 2016, Srinivas Kandagatla wrote:

> Thanks Bjorn for this patch,
> 
> I will start playing with patch soon, but here are few comments.
> 
> On 17/06/16 18:17, Bjorn Andersson wrote:
> >From: Bjorn Andersson 
> >
> >This initial hack powers the q6v5, boots and authenticate the mba and
> >use that to load the mdt and subsequent bXX files.
> >
> >Signed-off-by: Bjorn Andersson 
> >Signed-off-by: Bjorn Andersson 
> >---
> >
> >Changes since v1:
> >- Corrected boot address in relocation case
> >- Using rproc_da_to_va() to clean up mdt loader api
> >- Dynamically allocating scratch space for mdt verification
> >
> >  drivers/remoteproc/Kconfig   |  12 +
> >  drivers/remoteproc/Makefile  |   2 +
> >  drivers/remoteproc/qcom_mdt_loader.c | 166 +++
> >  drivers/remoteproc/qcom_mdt_loader.h |  13 +
> >  drivers/remoteproc/qcom_q6v5_pil.c   | 914 
> > +++
> 
> We should probably split this patch into two one for mdt loader and other
> for pil.
> 

I did consider it, as it's currently out for review in two different
patch sets, but I don't want to add dead code so it shouldn't be merged
on its own.

> Also checkpatch reports:
> 
> total: 1 errors, 28 warnings, 1117 lines checked
> 

I'll review that again.

[..]
> >diff --git a/drivers/remoteproc/qcom_mdt_loader.c 
> >b/drivers/remoteproc/qcom_mdt_loader.c
> >new file mode 100644
> >index ..4efeda908d9a
> >--- /dev/null
> >+++ b/drivers/remoteproc/qcom_mdt_loader.c
> >@@ -0,0 +1,166 @@
> >+/*
> >+ * Qualcomm Peripheral Image Loader
> >+ *
> >+ * Copyright (C) 2016 Linaro Ltd
> >+ * Copyright (C) 2015 Sony Mobile Communications Inc
> >+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> >+ *
> >+ * 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.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+ * GNU General Public License for more details.
> >+ */
> >+
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> >+#include 
> 
> ??
> 

Leftover from being TZ-only.

> >+#include 
> >+#include 
> ??
> 

For kfree() ?

> >+
> >+#include "remoteproc_internal.h"
> >+#include "qcom_mdt_loader.h"
> >+
> >+/**
> >+ * qcom_mdt_find_rsc_table() - provide dummy resource table for remoteproc
> >+ * @rproc:  remoteproc handle
> >+ * @fw: firmware header
> >+ * @tablesz:outgoing size of the table
> >+ *
> >+ * Returns a dummy table.
> >+ */
> >+struct resource_table *qcom_mdt_find_rsc_table(struct rproc *rproc,
> >+   const struct firmware *fw,
> >+   int *tablesz)
> >+{
> >+static struct resource_table table = { .ver = 1, };
> >+
> >+*tablesz = sizeof(table);
> >+return 
> >+}
> >+EXPORT_SYMBOL_GPL(qcom_mdt_find_rsc_table);
> >+
> >+int qcom_mdt_parse(const struct firmware *fw, phys_addr_t *fw_addr, size_t 
> >*fw_size, bool *fw_relocate)
> Missing doc for this function?
> 

Yes, that should be added.

> >+{
[..]
> >+}
> >+EXPORT_SYMBOL_GPL(qcom_mdt_parse);
> >+
> >+/**
> >+ * qcom_mdt_load() - load the firmware which header is defined in fw
> >+ * @rproc:  rproc handle
> >+ * @pas_id: PAS identifier to load this firmware into
> 
> ??
> 

Sorry, another leftover. Thanks for spotting that.

> >+ * @fw: frimware object for the header
> 
> s/frimware/firmware
> 

Thanks.

> >+ *
> >+ * Returns 0 on success, negative errno otherwise.
> >+ */
> >+int qcom_mdt_load(struct rproc *rproc,
> >+  const struct firmware *fw,
> >+  const char *firmware)
> >+{
[..]
> >+}
> >+EXPORT_SYMBOL_GPL(qcom_mdt_load);
> 
> Module Licence info?
> 

Didn't consider it a module on it's own, but you're right.

[..]
> >diff --git a/drivers/remoteproc/qcom_q6v5_pil.c 
> >b/drivers/remoteproc/qcom_q6v5_pil.c
[..]
> >+
> >+static void q6v5_regulator_disable(struct q6v5 *qproc)
> >+{
> >+regulator_bulk_disable(ARRAY_SIZE(qproc->supply), qproc->supply);
> >+regulator_set_voltage(qproc->supply[Q6V5_SUPPLY_CX].consumer, 0, 
> >INT_MAX);
> >+regulator_set_voltage(qproc->supply[Q6V5_SUPPLY_MX].consumer, 0, 
> >INT_MAX);
> 
> we disable the regulators and then set voltage why?
> I think these should be moved to q6v5_regulator_enable() unless am missing
> something here.
> 

Because during enable we reduce the valid range of voltages on the SMPS,
regardless of it being enabled or not. So I need to broaden that vote
for the application CPU to be allowed to vote for the lower voltages
again.


cx is however supposed to be a corner, so not sure if I should touch
that here at all. I'll replace that line with a TODO comment.

> >+

Re: [PATCH] clk: renesas: build clk-rcar-gen2.o for R8A7792

[Sergei Shtylyov]

Hello.

On 06/20/2016 06:43 PM, Arnd Bergmann wrote:


The newly added support for R8A7792 causes build failures
because we try to call rcar_gen2_clocks_init but that is not
built into the kernel:

arch/arm/mach-shmobile/built-in.o: In function `rcar_gen2_timer_init':
:(.init.text+0x3b0): undefined reference to `rcar_gen2_clocks_init'

This changes the clk Makefile to match the other platforms, though
I guess it would be better to find another way to do this, e.g.
by not requiring the external function call by relying on
CLK_OF_DECLARE(), or by making the ARCH_RCAR_GEN2 also control
the compilation of the clk driver.

Signed-off-by: Arnd Bergmann 
Fixes: 3ebee0adbfe7 ("ARM: shmobile: r8a7792: basic SoC support")
---
 drivers/clk/renesas/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index ead8bb843524..3cdb9aaf8717 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_ARCH_R8A7778)  += clk-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779) += clk-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790) += clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7791) += clk-rcar-gen2.o clk-div6.o
+obj-$(CONFIG_ARCH_R8A7792) += clk-rcar-gen2.o
 obj-$(CONFIG_ARCH_R8A7793) += clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7794) += clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7795) += r8a7795-cpg-mssr.o


   It was in the very first patch that I submitted for this SoC:

   http://marc.info/?t=14642126231=1

   It should be queued up by Geert.

MBR, Sergei

Re: [PATCH] clk: renesas: build clk-rcar-gen2.o for R8A7792

[Sergei Shtylyov]

Hello.

On 06/20/2016 06:43 PM, Arnd Bergmann wrote:


The newly added support for R8A7792 causes build failures
because we try to call rcar_gen2_clocks_init but that is not
built into the kernel:

arch/arm/mach-shmobile/built-in.o: In function `rcar_gen2_timer_init':
:(.init.text+0x3b0): undefined reference to `rcar_gen2_clocks_init'

This changes the clk Makefile to match the other platforms, though
I guess it would be better to find another way to do this, e.g.
by not requiring the external function call by relying on
CLK_OF_DECLARE(), or by making the ARCH_RCAR_GEN2 also control
the compilation of the clk driver.

Signed-off-by: Arnd Bergmann 
Fixes: 3ebee0adbfe7 ("ARM: shmobile: r8a7792: basic SoC support")
---
 drivers/clk/renesas/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile
index ead8bb843524..3cdb9aaf8717 100644
--- a/drivers/clk/renesas/Makefile
+++ b/drivers/clk/renesas/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_ARCH_R8A7778)  += clk-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779) += clk-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790) += clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7791) += clk-rcar-gen2.o clk-div6.o
+obj-$(CONFIG_ARCH_R8A7792) += clk-rcar-gen2.o
 obj-$(CONFIG_ARCH_R8A7793) += clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7794) += clk-rcar-gen2.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7795) += r8a7795-cpg-mssr.o


   It was in the very first patch that I submitted for this SoC:

   http://marc.info/?t=14642126231=1

   It should be queued up by Geert.

MBR, Sergei