Re: [PATCH v10 0/7] devfreq: Add devfreq-event class to provide raw data for devfreq device

[Chanwoo Choi]

Dear Kukjin,

Could you please pick remaining patches (patch4-patch7)?

On 01/27/2015 06:59 PM, MyungJoo Ham wrote:
>>  This patchset add new devfreq_event class to provide raw data to determine
>> current utilization of device  which is used for devfreq governor.
>>
>> The following description explains the feature of two kind of devfreq class:
>> - devfreq class (existing)
>>  : devfreq consumer device use raw data from devfreq_event device for
>>determining proper current system state and change voltage/frequency
>>dynamically using various governors.
>> - devfreq_event class (new)
>>  : Provide measured raw data to devfreq device for governor
> 
> Merged in my local git repo (1/7 to 3/7 as those three are in 
> /drivers/devfreq).
> 
> However, the admin requires me to setup the firewall again, so please wait
> another day for the merged patches to show up in external network
> (git.kernel.org) and to make a pull-request for Rafael.
> (waiting firewall execption approvals from the admin)

Dear Myungjoo,

Thanks for your apply.

Best Regards,
Chanwoo Choi

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

[PATCH 4/8] PM / Domains: Don't check for an existing device when adding a new

[Ulf Hansson]

When adding a device to a genpd, we no longer need to walk genpd's list
of existing devices to verify it hasn't already been added.

Instead we can now rely on the verification of not allowing existing
generic_pm_domain_data for a device, since that has the same meaning.

Signed-off-by: Ulf Hansson 
---
 drivers/base/power/domain.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 76eb0c3..88198ba 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1415,7 +1415,6 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
  struct gpd_timing_data *td)
 {
struct generic_pm_domain_data *gpd_data_new, *gpd_data = NULL;
-   struct pm_domain_data *pdd;
int ret = 0;
 
dev_dbg(dev, "%s()\n", __func__);
@@ -1434,12 +1433,6 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
goto out;
}
 
-   list_for_each_entry(pdd, &genpd->dev_list, list_node)
-   if (pdd->dev == dev) {
-   ret = -EINVAL;
-   goto out;
-   }
-
ret = dev_pm_get_subsys_data(dev);
if (ret)
goto out;
-- 
1.9.1

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

[PATCH 0/8] PM / Domains: Re-structure code for adding/removing devices to genpd

[Ulf Hansson]

While adding error handling of genpd's ->attach_dev() callback, I realized that
we also had a need to re-structure some of the code which deals with
adding/removing devices to genpd. Especially the APIs, __pm_genpd_add_device()
and pm_genpd_remove_device() deserved some attention.

Patch 1 -> 4, can be considered as more simple cleanups and should not impact
the behavior for current clients using the APIs. 

Patch 5, eliminates a mutex for generic_pm_domain_data by re-order the
initialization/cleanup of dev_pm_qos notifiers.

Patch 6, move some code around to fix a potenial memory leakage of a struct
pm_subsys_data.

Patch 7, code restructuring which impacts locking behavior while adding/removing
devices. Should improve code readability and decrease critical regions of
holding locks.

Patch 8, Adds error handling of genpd's ->attach_dev() callback


Ulf Hansson (8):
  PM / Domains: Rename __pm_genpd_alloc|free_dev_data()
  PM / Domains: Remove reference counting for the generic_pm_domain_data
  PM / Domains: Don't allow an existing generic_pm_domain_data
  PM / Domains: Don't check for an existing device when adding a new
  PM / Domains: Eliminate the mutex for the generic_pm_domain_data
  PM / Domains: Free pm_subsys_data in error path in
__pm_genpd_add_device()
  PM / Domains: Re-order initialization of generic_pm_domain_data
  PM / Domains: Handle errors from genpd's ->attach_dev() callback

 drivers/base/power/domain.c | 137 +---
 include/linux/pm_domain.h   |   2 -
 2 files changed, 65 insertions(+), 74 deletions(-)

-- 
1.9.1

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

[PATCH 3/8] PM / Domains: Don't allow an existing generic_pm_domain_data

[Ulf Hansson]

When adding a device to a genpd, a struct generic_pm_domain_data is
allocated per device.

Verify that there are no existing generic_pm_domain_data for the device
we are about to add, since that tells us it has already been added to a
genpd.

When genpd supported PM domain device callbacks, this was a valid
scenario. Now it isn't so let's return an error code.

Signed-off-by: Ulf Hansson 
---
 drivers/base/power/domain.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 351df5b..76eb0c3 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1444,26 +1444,30 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
if (ret)
goto out;
 
-   genpd->device_count++;
-   genpd->max_off_time_changed = true;
-
spin_lock_irq(&dev->power.lock);
 
-   dev->pm_domain = &genpd->domain;
if (dev->power.subsys_data->domain_data) {
-   gpd_data = to_gpd_data(dev->power.subsys_data->domain_data);
-   } else {
-   gpd_data = gpd_data_new;
-   dev->power.subsys_data->domain_data = &gpd_data->base;
+   spin_unlock_irq(&dev->power.lock);
+   ret = -EINVAL;
+   goto out;
}
+
+   gpd_data = gpd_data_new;
+   dev->power.subsys_data->domain_data = &gpd_data->base;
+
if (td)
gpd_data->td = *td;
 
+   dev->pm_domain = &genpd->domain;
+
spin_unlock_irq(&dev->power.lock);
 
if (genpd->attach_dev)
genpd->attach_dev(genpd, dev);
 
+   genpd->device_count++;
+   genpd->max_off_time_changed = true;
+
mutex_lock(&gpd_data->lock);
gpd_data->base.dev = dev;
list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
-- 
1.9.1

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

[PATCH 6/8] PM / Domains: Free pm_subsys_data in error path in __pm_genpd_add_device()

[Ulf Hansson]

The error path in __pm_genpd_add_device() didn't decrease the reference
to the struct pm_subsys_data.

Let's move the calls to dev_pm_get|put_subsys_data() into
genpd_alloc|free_dev_data() to fix this issue and thus prevent a
potential memory leakage.

Signed-off-by: Ulf Hansson 
---
 drivers/base/power/domain.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 1f026c1..3bd342f 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1380,18 +1380,30 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
 static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev)
 {
struct generic_pm_domain_data *gpd_data;
+   int ret;
+
+   ret = dev_pm_get_subsys_data(dev);
+   if (ret)
+   return ERR_PTR(ret);
 
gpd_data = kzalloc(sizeof(*gpd_data), GFP_KERNEL);
-   if (!gpd_data)
-   return NULL;
+   if (!gpd_data) {
+   ret = -ENOMEM;
+   goto err_put;
+   }
 
return gpd_data;
+
+ err_put:
+   dev_pm_put_subsys_data(dev);
+   return ERR_PTR(ret);
 }
 
 static void genpd_free_dev_data(struct device *dev,
struct generic_pm_domain_data *gpd_data)
 {
kfree(gpd_data);
+   dev_pm_put_subsys_data(dev);
 }
 
 /**
@@ -1412,8 +1424,8 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
return -EINVAL;
 
gpd_data = genpd_alloc_dev_data(dev);
-   if (!gpd_data)
-   return -ENOMEM;
+   if (IS_ERR(gpd_data))
+   return PTR_ERR(gpd_data);
 
genpd_acquire_lock(genpd);
 
@@ -1422,10 +1434,6 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
goto out;
}
 
-   ret = dev_pm_get_subsys_data(dev);
-   if (ret)
-   goto out;
-
spin_lock_irq(&dev->power.lock);
 
if (dev->power.subsys_data->domain_data) {
@@ -1528,7 +1536,6 @@ int pm_genpd_remove_device(struct generic_pm_domain 
*genpd,
 
genpd_release_lock(genpd);
 
-   dev_pm_put_subsys_data(dev);
genpd_free_dev_data(dev, gpd_data);
 
return 0;
-- 
1.9.1

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

[PATCH 8/8] PM / Domains: Handle errors from genpd's ->attach_dev() callback

[Ulf Hansson]

The optional genpd's ->attach_dev() callback is invoked from
__pm_genpd_add_device(). Let's add error handling from the response
from this callback and propagate the error code.

When __pm_genpd_add_device() is invoked through the generic OF-based PM
domain look-up path, the device is being probed. Returning an error
will mean the device won't be attached to its PM domain. Errors of
-EPROBE_DEFER get special treatment and is propagated to the driver
core.

Therefore this change also enables the ->attach_dev() callback to
be able to request for a deferred probe sequence.

Signed-off-by: Ulf Hansson 
---
 drivers/base/power/domain.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 0fd5ee1..ba4abbe 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1467,8 +1467,9 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
goto out;
}
 
-   if (genpd->attach_dev)
-   genpd->attach_dev(genpd, dev);
+   ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0;
+   if (ret)
+   goto out;
 
genpd->device_count++;
genpd->max_off_time_changed = true;
-- 
1.9.1

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

[PATCH 2/8] PM / Domains: Remove reference counting for the generic_pm_domain_data

[Ulf Hansson]

The reference counting was needed when genpd supported PM domain device
callbacks. Since this option has been removed, let's also remove the
reference counting of the struct generic_pm_domain_data.

Signed-off-by: Ulf Hansson 
---
 drivers/base/power/domain.c | 10 ++
 include/linux/pm_domain.h   |  1 -
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index f9e7df5..351df5b 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1456,7 +1456,6 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
gpd_data = gpd_data_new;
dev->power.subsys_data->domain_data = &gpd_data->base;
}
-   gpd_data->refcount++;
if (td)
gpd_data->td = *td;
 
@@ -1504,7 +1503,6 @@ int pm_genpd_remove_device(struct generic_pm_domain 
*genpd,
 {
struct generic_pm_domain_data *gpd_data;
struct pm_domain_data *pdd;
-   bool remove = false;
int ret = 0;
 
dev_dbg(dev, "%s()\n", __func__);
@@ -1533,10 +1531,7 @@ int pm_genpd_remove_device(struct generic_pm_domain 
*genpd,
pdd = dev->power.subsys_data->domain_data;
list_del_init(&pdd->list_node);
gpd_data = to_gpd_data(pdd);
-   if (--gpd_data->refcount == 0) {
-   dev->power.subsys_data->domain_data = NULL;
-   remove = true;
-   }
+   dev->power.subsys_data->domain_data = NULL;
 
spin_unlock_irq(&dev->power.lock);
 
@@ -1547,8 +1542,7 @@ int pm_genpd_remove_device(struct generic_pm_domain 
*genpd,
genpd_release_lock(genpd);
 
dev_pm_put_subsys_data(dev);
-   if (remove)
-   genpd_free_dev_data(dev, gpd_data);
+   genpd_free_dev_data(dev, gpd_data);
 
return 0;
 
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index ed60776..e160a0b 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -114,7 +114,6 @@ struct generic_pm_domain_data {
struct gpd_timing_data td;
struct notifier_block nb;
struct mutex lock;
-   unsigned int refcount;
int need_restore;
 };
 
-- 
1.9.1

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

[PATCH 7/8] PM / Domains: Re-order initialization of generic_pm_domain_data

[Ulf Hansson]

Move the initialization of the struct generic_pm_domain_data into
genpd_alloc_dev_data(), including the assignment of the device's
->pm_domain() callback. Make corresponding changes to
genpd_free_dev_data().

These changes will make the related code more readable. It will also
decrease the critical regions for where genpd's mutex is being held and
for where the device's power related spinlock is being held.

Signed-off-by: Ulf Hansson 
---
 drivers/base/power/domain.c | 67 +++--
 1 file changed, 35 insertions(+), 32 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 3bd342f..0fd5ee1 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1377,7 +1377,9 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
 
 #endif /* CONFIG_PM_SLEEP */
 
-static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev)
+static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
+   struct generic_pm_domain *genpd,
+   struct gpd_timing_data *td)
 {
struct generic_pm_domain_data *gpd_data;
int ret;
@@ -1392,8 +1394,32 @@ static struct generic_pm_domain_data 
*genpd_alloc_dev_data(struct device *dev)
goto err_put;
}
 
+   if (td)
+   gpd_data->td = *td;
+
+   gpd_data->base.dev = dev;
+   gpd_data->need_restore = -1;
+   gpd_data->td.constraint_changed = true;
+   gpd_data->td.effective_constraint_ns = -1;
+   gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier;
+
+   spin_lock_irq(&dev->power.lock);
+
+   if (dev->power.subsys_data->domain_data) {
+   ret = -EINVAL;
+   goto err_free;
+   }
+
+   dev->power.subsys_data->domain_data = &gpd_data->base;
+   dev->pm_domain = &genpd->domain;
+
+   spin_unlock_irq(&dev->power.lock);
+
return gpd_data;
 
+ err_free:
+   spin_unlock_irq(&dev->power.lock);
+   kfree(gpd_data);
  err_put:
dev_pm_put_subsys_data(dev);
return ERR_PTR(ret);
@@ -1402,6 +1428,13 @@ static struct generic_pm_domain_data 
*genpd_alloc_dev_data(struct device *dev)
 static void genpd_free_dev_data(struct device *dev,
struct generic_pm_domain_data *gpd_data)
 {
+   spin_lock_irq(&dev->power.lock);
+
+   dev->pm_domain = NULL;
+   dev->power.subsys_data->domain_data = NULL;
+
+   spin_unlock_irq(&dev->power.lock);
+
kfree(gpd_data);
dev_pm_put_subsys_data(dev);
 }
@@ -1423,7 +1456,7 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
return -EINVAL;
 
-   gpd_data = genpd_alloc_dev_data(dev);
+   gpd_data = genpd_alloc_dev_data(dev, genpd, td);
if (IS_ERR(gpd_data))
return PTR_ERR(gpd_data);
 
@@ -1434,35 +1467,13 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
goto out;
}
 
-   spin_lock_irq(&dev->power.lock);
-
-   if (dev->power.subsys_data->domain_data) {
-   spin_unlock_irq(&dev->power.lock);
-   ret = -EINVAL;
-   goto out;
-   }
-
-   dev->power.subsys_data->domain_data = &gpd_data->base;
-
-   if (td)
-   gpd_data->td = *td;
-
-   dev->pm_domain = &genpd->domain;
-
-   spin_unlock_irq(&dev->power.lock);
-
if (genpd->attach_dev)
genpd->attach_dev(genpd, dev);
 
genpd->device_count++;
genpd->max_off_time_changed = true;
 
-   gpd_data->base.dev = dev;
list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
-   gpd_data->need_restore = -1;
-   gpd_data->td.constraint_changed = true;
-   gpd_data->td.effective_constraint_ns = -1;
-   gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier;
 
  out:
genpd_release_lock(genpd);
@@ -1524,15 +1535,7 @@ int pm_genpd_remove_device(struct generic_pm_domain 
*genpd,
if (genpd->detach_dev)
genpd->detach_dev(genpd, dev);
 
-   spin_lock_irq(&dev->power.lock);
-
-   dev->pm_domain = NULL;
list_del_init(&pdd->list_node);
-   dev->power.subsys_data->domain_data = NULL;
-
-   spin_unlock_irq(&dev->power.lock);
-
-   pdd->dev = NULL;
 
genpd_release_lock(genpd);
 
-- 
1.9.1

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

[PATCH 1/8] PM / Domains: Rename __pm_genpd_alloc|free_dev_data()

[Ulf Hansson]

In a step to get consistent names of functions in genpd, rename
the internal __pm_genpd_alloc|free_dev_data() into
gendp_alloc|free_dev_data().

As discussed on the linux-pm list, let's move towards the following
name rules:

Internal functions:
genpd_*
_genpd_*
__genpd_*

External functions:
pm_genpd_*
_pm_genpd_*
__pm_genpd_*

Signed-off-by: Ulf Hansson 
---
 drivers/base/power/domain.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index c5280f2..f9e7df5 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1384,7 +1384,7 @@ EXPORT_SYMBOL_GPL(pm_genpd_syscore_poweron);
 
 #endif /* CONFIG_PM_SLEEP */
 
-static struct generic_pm_domain_data *__pm_genpd_alloc_dev_data(struct device 
*dev)
+static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev)
 {
struct generic_pm_domain_data *gpd_data;
 
@@ -1398,8 +1398,8 @@ static struct generic_pm_domain_data 
*__pm_genpd_alloc_dev_data(struct device *d
return gpd_data;
 }
 
-static void __pm_genpd_free_dev_data(struct device *dev,
-struct generic_pm_domain_data *gpd_data)
+static void genpd_free_dev_data(struct device *dev,
+   struct generic_pm_domain_data *gpd_data)
 {
dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
kfree(gpd_data);
@@ -1423,7 +1423,7 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
return -EINVAL;
 
-   gpd_data_new = __pm_genpd_alloc_dev_data(dev);
+   gpd_data_new = genpd_alloc_dev_data(dev);
if (!gpd_data_new)
return -ENOMEM;
 
@@ -1477,7 +1477,7 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
genpd_release_lock(genpd);
 
if (gpd_data != gpd_data_new)
-   __pm_genpd_free_dev_data(dev, gpd_data_new);
+   genpd_free_dev_data(dev, gpd_data_new);
 
return ret;
 }
@@ -1548,7 +1548,7 @@ int pm_genpd_remove_device(struct generic_pm_domain 
*genpd,
 
dev_pm_put_subsys_data(dev);
if (remove)
-   __pm_genpd_free_dev_data(dev, gpd_data);
+   genpd_free_dev_data(dev, gpd_data);
 
return 0;
 
-- 
1.9.1

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

[PATCH 5/8] PM / Domains: Eliminate the mutex for the generic_pm_domain_data

[Ulf Hansson]

While adding devices to their PM domains, dev_pm_qos_add_notifier() was
invoked while allocating the generic_pm_domain_data for the device.

Since the generic_pm_domain_data's device pointer will be assigned
after allocation, the ->genpd_dev_pm_qos_notifier() callback could be
called prior having a valid pointer to the device. Similar scenario
existed while removing a device from a genpd.

To cope with these scenarios a mutex was used to protect the pointer to
the device.

By re-order the sequence for when dev_pm_qos_add|remove_notifier() are
invoked, we make sure the ->genpd_dev_pm_qos_notifier() callback are
always called with a valid device pointer available.

In this way, we eliminate the need for protecting the pointer and thus
we can remove the mutex from the struct generic_pm_domain_data.

Signed-off-by: Ulf Hansson 
---
 drivers/base/power/domain.c | 37 ++---
 include/linux/pm_domain.h   |  1 -
 2 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 88198ba..1f026c1 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -344,14 +344,7 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block 
*nb,
struct device *dev;
 
gpd_data = container_of(nb, struct generic_pm_domain_data, nb);
-
-   mutex_lock(&gpd_data->lock);
dev = gpd_data->base.dev;
-   if (!dev) {
-   mutex_unlock(&gpd_data->lock);
-   return NOTIFY_DONE;
-   }
-   mutex_unlock(&gpd_data->lock);
 
for (;;) {
struct generic_pm_domain *genpd;
@@ -1392,16 +1385,12 @@ static struct generic_pm_domain_data 
*genpd_alloc_dev_data(struct device *dev)
if (!gpd_data)
return NULL;
 
-   mutex_init(&gpd_data->lock);
-   gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier;
-   dev_pm_qos_add_notifier(dev, &gpd_data->nb);
return gpd_data;
 }
 
 static void genpd_free_dev_data(struct device *dev,
struct generic_pm_domain_data *gpd_data)
 {
-   dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
kfree(gpd_data);
 }
 
@@ -1414,7 +1403,7 @@ static void genpd_free_dev_data(struct device *dev,
 int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
  struct gpd_timing_data *td)
 {
-   struct generic_pm_domain_data *gpd_data_new, *gpd_data = NULL;
+   struct generic_pm_domain_data *gpd_data;
int ret = 0;
 
dev_dbg(dev, "%s()\n", __func__);
@@ -1422,8 +1411,8 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
return -EINVAL;
 
-   gpd_data_new = genpd_alloc_dev_data(dev);
-   if (!gpd_data_new)
+   gpd_data = genpd_alloc_dev_data(dev);
+   if (!gpd_data)
return -ENOMEM;
 
genpd_acquire_lock(genpd);
@@ -1445,7 +1434,6 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
goto out;
}
 
-   gpd_data = gpd_data_new;
dev->power.subsys_data->domain_data = &gpd_data->base;
 
if (td)
@@ -1461,19 +1449,20 @@ int __pm_genpd_add_device(struct generic_pm_domain 
*genpd, struct device *dev,
genpd->device_count++;
genpd->max_off_time_changed = true;
 
-   mutex_lock(&gpd_data->lock);
gpd_data->base.dev = dev;
list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
gpd_data->need_restore = -1;
gpd_data->td.constraint_changed = true;
gpd_data->td.effective_constraint_ns = -1;
-   mutex_unlock(&gpd_data->lock);
+   gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier;
 
  out:
genpd_release_lock(genpd);
 
-   if (gpd_data != gpd_data_new)
-   genpd_free_dev_data(dev, gpd_data_new);
+   if (ret)
+   genpd_free_dev_data(dev, gpd_data);
+   else
+   dev_pm_qos_add_notifier(dev, &gpd_data->nb);
 
return ret;
 }
@@ -1509,6 +1498,11 @@ int pm_genpd_remove_device(struct generic_pm_domain 
*genpd,
||  pd_to_genpd(dev->pm_domain) != genpd)
return -EINVAL;
 
+   /* The above validation also means we have existing domain_data. */
+   pdd = dev->power.subsys_data->domain_data;
+   gpd_data = to_gpd_data(pdd);
+   dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
+
genpd_acquire_lock(genpd);
 
if (genpd->prepared_count > 0) {
@@ -1525,16 +1519,12 @@ int pm_genpd_remove_device(struct generic_pm_domain 
*genpd,
spin_lock_irq(&dev->power.lock);
 
dev->pm_domain = NULL;
-   pdd = dev->power.subsys_data->domain_data;
list_del_init(&pdd->list_node);
-   gpd_data = to_gpd_data(pdd);
dev->power.subsys_data->domain_data = NULL;
 
spin_unlock_irq(&dev->power.lock);
 
-

Re: [alsa-devel] [PATCH v4 1/3] ASoC: samsung: Add machine driver for Trats2

[Mark Brown]

On Fri, Jan 23, 2015 at 02:03:28PM +0900, Inha Song wrote:
> This patch add the sound machine driver for Trats2 board.
> The codec operate in master mode.

This looks like (and mostly should be) a DTified copy of the littlemill
driver.  The major differences are the fact that this lacks jack
detection, the management of the clock and the fact that instead of
picking a fixed clock frequency this tries to adjust based on
hw_params().  Ideally we'd be able to merge these drivers so we're
keeping up with best practice.

The reason we pick a fixed frequency for littlemill is that the device
supports analogue bypass paths which would be broken otherwise which
will apply here also I imagine.

> +config SND_SOC_SAMSUNG_TRATS2_WM1811
> + tristate "SoC I2S Audio support for WM1811 on Tizen Trats2 board"
> + depends on SND_SOC_SAMSUNG
> + select SND_SAMSUNG_I2S
> + select MFD_WM8994

For things outside the audio subsystem it's more common to use a depends
instead of a select - the selects are there because the drivers only
work with a machine driver and if we go outside the subsystem then since
selects aren't recursive we end up breaking elsewhere.  For example here
we'll end up not selecting MFD_CORE.


signature.asc
Description: Digital signature

Re: [alsa-devel] [PATCH v4 2/3] ASoC: samsung: Document Trats2 audio subsystem bindings

[Mark Brown]

On Fri, Jan 23, 2015 at 02:03:29PM +0900, Inha Song wrote:

> + - samsung,audio-routing : A list of the connections between audio
> +   components. each entry is a pair of strings, the first being the
> +   connection's sink, the second being the connection's source

The list of valid components should be specified here - reference the
CODEC bindings for anything on the CODEC but things on the board need to
be enumerated.


signature.asc
Description: Digital signature

Re: [alsa-devel] [PATCH v4 1/3] ASoC: samsung: Add machine driver for Trats2

[Mark Brown]

On Tue, Jan 27, 2015 at 06:09:39PM +0100, Sylwester Nawrocki wrote:
> On 23/01/15 06:03, Inha Song wrote:

> > +   priv->clk_mclk =  of_clk_get_by_name(codec_node, "MCLK1");
> > +   if (IS_ERR(priv->clk_mclk)) {
> > +   dev_err(&pdev->dev, "Failed to get mclk clock\n");
> > +   of_node_put(codec_node);
> > +   return PTR_ERR(priv->clk_mclk);
> > +   }

> Wouldn't it also work if we added clock handling into the wm8994 codec
> driver instead? Not sure if it is correct to retrieve the codec's clock
> in the machine driver like this. Or perhaps the MCLK1 (SoC CLKOUT) clock
> should be added to the sound DT node and handled only by the machine
> driver, together with the other (MCLK2) clock?

That's definitely where we should end up but there are practical issues
with that approach since it involves coordination with all the machine
drivers using the device.


signature.asc
Description: Digital signature

Re: [RFC 2/3] regmap: Use the enhancement of i2c API to address circular dependency problem

[Mark Brown]

On Tue, Jan 20, 2015 at 12:14:31PM +0100, Paul Osmialowski wrote:
> On Mon, 19 Jan 2015, Mark Brown wrote:

> >OK, so that's what should go in the changelog (along with an explanation
> >of why this preparation is required at all) - but I still don't see the
> >async bit of this I'm afraid.

> I don't think preparation stage should be exposed for asynchronous transfer.
> Due to its nature, it shouldn't cause circular lock dependency as we can
> observe for synchronous io. Since i2c does not support async transfer anyway
> (so (map->async && map->bus->async_write) will be always false for i2c
> transfers), let's use spi as an example.

Again I come back to explaining this out of the context of this
particular issue in terms of something that's comprehensible at the
regmap level.

> With spi we have curious situation where both sync and async are handled by
> the same __spi_async() function, though for sync transfer
> wait_for_completion() is called soon after __spi_async() in order to ensure
> that transfer is completed.

That's not actually that strange really, in general synchronous
operation can always be implemented as a simple wrapper around
asynchronous operation.

> Actual transfer is handled by spi_transfer_one_message() called from
> spi_pump_messages(). Note that spi_pump_message() before it calls
> spi_transfer_one_message() also calls prepare_message() callback (if
> provided):

We are *massively* into implementation details here, if we're relying on
these things then they could change at any time (and in fact what you
say above is partly specific and is not true even in the default case
for current code).  Think in terms of abstractions and locking
guarantees rather than the details of the current code.


signature.asc
Description: Digital signature

Re: [alsa-devel] [PATCH v4 1/3] ASoC: samsung: Add machine driver for Trats2

[Sylwester Nawrocki]

On 23/01/15 06:03, Inha Song wrote:
> +static int trats2_aif1_startup(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct trats2_machine_priv *priv = snd_soc_card_get_drvdata(rtd->card);
> + int ret;
> +
> + ret = clk_prepare_enable(priv->clk_mclk);
> + if (ret) {
> + dev_err(rtd->card->dev, "Failed to enable mclk: %d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void trats2_aif1_shutdown(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct trats2_machine_priv *priv = snd_soc_card_get_drvdata(rtd->card);
> +
> + clk_disable_unprepare(priv->clk_mclk);
> +}
> +
> +static const struct snd_soc_ops trats2_aif1_ops = {
> + .startup = trats2_aif1_startup,
> + .shutdown = trats2_aif1_shutdown,
> + .hw_params = trats2_aif1_hw_params,
> +};

> +static int trats2_audio_probe(struct platform_device *pdev)
> +{

> + priv->clk_mclk =  of_clk_get_by_name(codec_node, "MCLK1");
> + if (IS_ERR(priv->clk_mclk)) {
> + dev_err(&pdev->dev, "Failed to get mclk clock\n");
> + of_node_put(codec_node);
> + return PTR_ERR(priv->clk_mclk);
> + }

Wouldn't it also work if we added clock handling into the wm8994 codec
driver instead? Not sure if it is correct to retrieve the codec's clock
in the machine driver like this. Or perhaps the MCLK1 (SoC CLKOUT) clock
should be added to the sound DT node and handled only by the machine
driver, together with the other (MCLK2) clock?

> + of_node_put(codec_node);

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

Re: [PATCH] thermal: Kconfig: Remove config for not used EXYNOS_THERMAL_CORE

[Lukasz Majewski]

Hi Eduardo,

> On Tue, Jan 27, 2015 at 12:13:59PM +0100, Lukasz Majewski wrote:
> > After removing exynos_thermal_common.[c|h] files the
> > CONFIG_EXYNOS_THERMA_CORE is not needed anymore.
> > This patch removes this entry from Kconfig.
> > 
> > Reported-by: Paul Bolle 
> > Signed-off-by: Lukasz Majewski 
> 
> This patch looks fine to me and I am about to queue it in my fixes
> tree. However, you must carry along another patch to remove this
> entry: arch/arm/configs/exynos_defconfig:CONFIG_EXYNOS_THERMAL_CORE=y

OK.

> 
> Can you please post it and ask Kim to queue the above suggested
> removal too?

OK.

> 
> > ---
> >  drivers/thermal/samsung/Kconfig | 9 -
> >  1 file changed, 9 deletions(-)
> > 
> > diff --git a/drivers/thermal/samsung/Kconfig
> > b/drivers/thermal/samsung/Kconfig index c43306e..c8e35c1 100644
> > --- a/drivers/thermal/samsung/Kconfig
> > +++ b/drivers/thermal/samsung/Kconfig
> > @@ -7,12 +7,3 @@ config EXYNOS_THERMAL
> >   the TMU, reports temperature and handles cooling action
> > if defined. This driver uses the Exynos core thermal APIs and TMU
> > configuration data from the supported SoCs.
> > -
> > -config EXYNOS_THERMAL_CORE
> > -   bool "Core thermal framework support for EXYNOS SOCs"
> > -   depends on EXYNOS_THERMAL
> > -   help
> > - If you say yes here you get support for EXYNOS TMU
> > - (Thermal Management Unit) common
> > registration/unregistration
> > - functions to the core thermal layer and also to use the
> > generic
> > - CPU cooling APIs.
> > -- 
> > 2.0.0.rc2
> > 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] thermal: Kconfig: Remove config for not used EXYNOS_THERMAL_CORE

[Eduardo Valentin]

On Tue, Jan 27, 2015 at 12:13:59PM +0100, Lukasz Majewski wrote:
> After removing exynos_thermal_common.[c|h] files the CONFIG_EXYNOS_THERMA_CORE
> is not needed anymore.
> This patch removes this entry from Kconfig.
> 
> Reported-by: Paul Bolle 
> Signed-off-by: Lukasz Majewski 

This patch looks fine to me and I am about to queue it in my fixes tree.
However, you must carry along another patch to remove this entry:
arch/arm/configs/exynos_defconfig:CONFIG_EXYNOS_THERMAL_CORE=y

Can you please post it and ask Kim to queue the above suggested removal
too?

> ---
>  drivers/thermal/samsung/Kconfig | 9 -
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/Kconfig b/drivers/thermal/samsung/Kconfig
> index c43306e..c8e35c1 100644
> --- a/drivers/thermal/samsung/Kconfig
> +++ b/drivers/thermal/samsung/Kconfig
> @@ -7,12 +7,3 @@ config EXYNOS_THERMAL
> the TMU, reports temperature and handles cooling action if defined.
> This driver uses the Exynos core thermal APIs and TMU configuration
> data from the supported SoCs.
> -
> -config EXYNOS_THERMAL_CORE
> - bool "Core thermal framework support for EXYNOS SOCs"
> - depends on EXYNOS_THERMAL
> - help
> -   If you say yes here you get support for EXYNOS TMU
> -   (Thermal Management Unit) common registration/unregistration
> -   functions to the core thermal layer and also to use the generic
> -   CPU cooling APIs.
> -- 
> 2.0.0.rc2
> 


signature.asc
Description: Digital signature

Re: [PATCH] mmc: dw_mmc: exynos: remove incorrect __exit_p()

[Ulf Hansson]

On 24 January 2015 at 01:33, Dmitry Torokhov  wrote:
> dw_mci_pltfm_remove() is not (nor should it be) marked as __exit,
> so we should not be using __exit_p() wrapper with it.
>
> Signed-off-by: Dmitry Torokhov 

Thanks! Applied for next.

Kind regards
Uffe


> ---
>  drivers/mmc/host/dw_mmc-exynos.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
> b/drivers/mmc/host/dw_mmc-exynos.c
> index 12a5eaa..fe32948 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -422,7 +422,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
>
>  static struct platform_driver dw_mci_exynos_pltfm_driver = {
> .probe  = dw_mci_exynos_probe,
> -   .remove = __exit_p(dw_mci_pltfm_remove),
> +   .remove = dw_mci_pltfm_remove,
> .driver = {
> .name   = "dwmmc_exynos",
> .of_match_table = dw_mci_exynos_match,
> --
> 2.2.0.rc0.207.ga3a616c
>
>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Fix reboot issue on Odroid boards with eMMC card

[Marek Szyprowski]

Hello,

On 2015-01-27 09:56, Sjoerd Simons wrote:

On Tue, 2015-01-27 at 09:11 +0100, Marek Szyprowski wrote:

This patchset fixes reboot hang issue on Hardkernel's Odroid boards with
eMMC card. Those boards are designed in such a way, that the eMMC nreset
signal is routed to SoC GPIO line instead of the board reset logic. To
properly restard system, one need to set this line to zero before
performing reboot.

The initial patches consisted of a complete reset/power off driver, but
after further analysis, it turned out that only eMMC nreset line control
logic is specific for those boards. Everything else can be done by
generic Exynos code, so the code has been moved to Exynos DW MMC driver.

This seems a general board design quirk, rather then an exynos specific
one. Potentially better to have this in the mmc core so it can be
re-used by non-exynos boards?

A very quick peek at the schematic for the hardkernel C1 (amlogic
based), shows that for that board the eMMC reset line is hooked up to an
SoC gpio line as well. (Although ofcourse, it may not need to be reset
before a warm restart depending on what the amlogic rom code does).


Random side-note, at least the samsung peach chromebooks are also hooked
up this way. But don't run into this issue as they load their initial
stages from flash rather then the eMMC.


Okay, I will try to move all the code to mmc core. I've also noticed that
sdhci-pci driver already implements hw_reset feature with gpio line, 
although

I cannot find any user of it (I cannot find any platform data provider for
sdhci-pci driver).

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

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

Re: [PATCH 2/6] drm/exynos: track vblank events on a per crtc basis

[Daniel Vetter]

On Tue, Jan 27, 2015 at 12:59:15PM +, Daniel Stone wrote:
> Hi,
> 
> On 23 January 2015 at 12:42, Gustavo Padovan  wrote:
> >  void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe)
> >  {
> > struct exynos_drm_private *dev_priv = dev->dev_private;
> > -   struct drm_pending_vblank_event *e, *t;
> > struct drm_crtc *drm_crtc = dev_priv->crtc[pipe];
> > struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc);
> > -   unsigned long flags;
> >
> > -   spin_lock_irqsave(&dev->event_lock, flags);
> > +   if (exynos_crtc->event) {
> >
> > -   list_for_each_entry_safe(e, t, &dev_priv->pageflip_event_list,
> > -   base.link) {
> > -   /* if event's pipe isn't same as crtc then ignore it. */
> > -   if (pipe != e->pipe)
> > -   continue;
> > -
> > -   list_del(&e->base.link);
> > -   drm_send_vblank_event(dev, -1, e);
> > +   spin_lock_irq(&dev->event_lock);
> > +   drm_send_vblank_event(dev, -1, exynos_crtc->event);
> > drm_vblank_put(dev, pipe);
> > -   atomic_set(&exynos_crtc->pending_flip, 0);
> > wake_up(&exynos_crtc->pending_flip_queue);
> > -   }
> > +   spin_unlock_irq(&dev->event_lock);
> >
> > -   spin_unlock_irqrestore(&dev->event_lock, flags);
> > +   exynos_crtc->event = NULL;
> > +   }
> >  }
> 
> Doesn't this need to hold the CRTC lock to not race with, e.g, the
> page_flip caller? This gets called from IRQ context though, so might
> need conversion to soft-IRQ to do so, or another per-CRTC spinlock
> just to protect the event.

dev->even_lock should be enough, but I suspect that lock also protects
exynos_crtc->event (at least that's the case in i915.ko). So would need to
be moved out of the if block again.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] drm/exynos: track vblank events on a per crtc basis

[Daniel Stone]

Hi,

On 23 January 2015 at 12:42, Gustavo Padovan  wrote:
>  void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe)
>  {
> struct exynos_drm_private *dev_priv = dev->dev_private;
> -   struct drm_pending_vblank_event *e, *t;
> struct drm_crtc *drm_crtc = dev_priv->crtc[pipe];
> struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc);
> -   unsigned long flags;
>
> -   spin_lock_irqsave(&dev->event_lock, flags);
> +   if (exynos_crtc->event) {
>
> -   list_for_each_entry_safe(e, t, &dev_priv->pageflip_event_list,
> -   base.link) {
> -   /* if event's pipe isn't same as crtc then ignore it. */
> -   if (pipe != e->pipe)
> -   continue;
> -
> -   list_del(&e->base.link);
> -   drm_send_vblank_event(dev, -1, e);
> +   spin_lock_irq(&dev->event_lock);
> +   drm_send_vblank_event(dev, -1, exynos_crtc->event);
> drm_vblank_put(dev, pipe);
> -   atomic_set(&exynos_crtc->pending_flip, 0);
> wake_up(&exynos_crtc->pending_flip_queue);
> -   }
> +   spin_unlock_irq(&dev->event_lock);
>
> -   spin_unlock_irqrestore(&dev->event_lock, flags);
> +   exynos_crtc->event = NULL;
> +   }
>  }

Doesn't this need to hold the CRTC lock to not race with, e.g, the
page_flip caller? This gets called from IRQ context though, so might
need conversion to soft-IRQ to do so, or another per-CRTC spinlock
just to protect the event.

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

[PATCH] thermal: Kconfig: Remove config for not used EXYNOS_THERMAL_CORE

[Lukasz Majewski]

After removing exynos_thermal_common.[c|h] files the CONFIG_EXYNOS_THERMA_CORE
is not needed anymore.
This patch removes this entry from Kconfig.

Reported-by: Paul Bolle 
Signed-off-by: Lukasz Majewski 
---
 drivers/thermal/samsung/Kconfig | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/thermal/samsung/Kconfig b/drivers/thermal/samsung/Kconfig
index c43306e..c8e35c1 100644
--- a/drivers/thermal/samsung/Kconfig
+++ b/drivers/thermal/samsung/Kconfig
@@ -7,12 +7,3 @@ config EXYNOS_THERMAL
  the TMU, reports temperature and handles cooling action if defined.
  This driver uses the Exynos core thermal APIs and TMU configuration
  data from the supported SoCs.
-
-config EXYNOS_THERMAL_CORE
-   bool "Core thermal framework support for EXYNOS SOCs"
-   depends on EXYNOS_THERMAL
-   help
- If you say yes here you get support for EXYNOS TMU
- (Thermal Management Unit) common registration/unregistration
- functions to the core thermal layer and also to use the generic
- CPU cooling APIs.
-- 
2.0.0.rc2

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

Re: [PATCH v2 00/16] ASoC: samsung: Add clk provider for I2S internal clocks

[Sylwester Nawrocki]

Hello Tobias,

On 24/01/15 02:53, Tobias Jakobi wrote:
> Hello,
> 
> I've tested the series on my X2 and so far I haven't encountered any
> obvious issues with it.
> 
> I have a small question though. With the move to simple-audio-card the
> old driver (selected by SND_SOC_ODROIDX2) is probably going away after
> some time.
> Currently SND_SOC_ODROIDX2 also selects SND_SOC_MAX98090 and
> SND_SAMSUNG_I2S, which I believe (correct me if I'm wrong) are necessary
> to use the sound subsystem even with simple-audio-card (since the
> max98090 codec has to be built at least). I've found no way of selecting
> these two manually, so at the moment I need both CONFIG_SND_SIMPLE_CARD
> and SND_SOC_ODROIDX2 enabled to use sound. Is this intended behaviour?

Thanks for the feedback.  It seems I finally forgot to update Kconfig,
presumably we should make SND_SOC_ODROIDX2 selecting SND_SIMPLE_CARD,
i.e.

diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig
index 58c797a..7fb728c 100644
--- a/sound/soc/samsung/Kconfig
+++ b/sound/soc/samsung/Kconfig
@@ -247,6 +247,7 @@ config SND_SOC_ODROIDX2
depends on SND_SOC_SAMSUNG
select SND_SOC_MAX98090
select SND_SAMSUNG_I2S
+   select SND_SIMPLE_CARD
help
  Say Y here to enable audio support for the Odroidx2.

Making SND_SOC_SAMSUNG selecting SND_SIMPLE_CARD is probably not a good
idea and still we would be missing the codec part.
I'm going to post a patch as above.

> And while I'm at it. I'm booting the board with an upstream u-boot
> version which sets the MPLL to 800MHz, which AFAIK is the clocking rate
> that makes less 'trouble' (but at the expense of performance).
> I was wondering if anyone can comment on whether the recent rework has
> any influence on the behaviour when booting with the vendor u-boot
> (which sets the MPLL to 880MHz). IIRC then clock rounding issues arose
> there.
> 
> It would be interesting to know if this series gets us closer to drive
> the MPLL with the recommended (?) higher clocking rate.

No, (unfortunately) this series has nothing to do with MPLL.  The whole
clock tree for sound is configured by the kernel, including the EPLL, which
provides reference clock for the audio subsystem.  MPLL and EPLL are normally
used in different clock domains.  So there should be no any clock rounding
issues related to bootloader as far as audio is concerned.  Please let me
know if you find any related problems.

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

Re: [PATCH v10 0/7] devfreq: Add devfreq-event class to provide raw data for devfreq device

[MyungJoo Ham]

>  This patchset add new devfreq_event class to provide raw data to determine
> current utilization of device  which is used for devfreq governor.
> 
> The following description explains the feature of two kind of devfreq class:
> - devfreq class (existing)
>  : devfreq consumer device use raw data from devfreq_event device for
>determining proper current system state and change voltage/frequency
>dynamically using various governors.
> - devfreq_event class (new)
>  : Provide measured raw data to devfreq device for governor

Merged in my local git repo (1/7 to 3/7 as those three are in /drivers/devfreq).

However, the admin requires me to setup the firewall again, so please wait
another day for the merged patches to show up in external network
(git.kernel.org) and to make a pull-request for Rafael.
(waiting firewall execption approvals from the admin)


Cheers,
MyungJoo

Re: [PATCH 0/2] Fix reboot issue on Odroid boards with eMMC card

[Sjoerd Simons]

On Tue, 2015-01-27 at 09:11 +0100, Marek Szyprowski wrote:
> Hello,
> 
> This patchset fixes reboot hang issue on Hardkernel's Odroid boards with
> eMMC card. Those boards are designed in such a way, that the eMMC nreset
> signal is routed to SoC GPIO line instead of the board reset logic. To
> properly restard system, one need to set this line to zero before
> performing reboot.
> 
> The initial patches consisted of a complete reset/power off driver, but
> after further analysis, it turned out that only eMMC nreset line control
> logic is specific for those boards. Everything else can be done by
> generic Exynos code, so the code has been moved to Exynos DW MMC driver.

This seems a general board design quirk, rather then an exynos specific
one. Potentially better to have this in the mmc core so it can be
re-used by non-exynos boards?

A very quick peek at the schematic for the hardkernel C1 (amlogic
based), shows that for that board the eMMC reset line is hooked up to an
SoC gpio line as well. (Although ofcourse, it may not need to be reset
before a warm restart depending on what the amlogic rom code does).


Random side-note, at least the samsung peach chromebooks are also hooked
up this way. But don't run into this issue as they load their initial
stages from flash rather then the eMMC. 


> Best regards
> Marek Szyprowski
> Samsung R&D Institute Poland
> 
> 
> Changelog:
> 
> initial version:
> http://thread.gmane.org/gmane.linux.power-management.general/51855/
> 
> 
> Path summary:
> 
> Marek Szyprowski (2):
>   mmc: dw_mmc-exynos: add support for controlling emmc reset pin
>   ARM: dts: exynos*-odroid*: add eMMC reset line
> 
>  .../devicetree/bindings/mmc/exynos-dw-mshc.txt |  6 +++
>  arch/arm/boot/dts/exynos4412-odroid-common.dtsi|  1 +
>  arch/arm/boot/dts/exynos5422-odroidxu3.dts |  1 +
>  drivers/mmc/host/dw_mmc-exynos.c   | 43 
> +-
>  4 files changed, 50 insertions(+), 1 deletion(-)
> 




smime.p7s
Description: S/MIME cryptographic signature

Re: [PATCH 1/2] mmc: dw_mmc-exynos: add support for controlling emmc reset pin

[Jaehoon Chung]

Hi, Marek.

your patch should be conflicted with 
"https://patchwork.kernel.org/patch/5698421/";

On 01/27/2015 05:11 PM, Marek Szyprowski wrote:
> There are boards (like Hardkernel's Odroid boards) on which eMMC card's
> reset line is connected to SoC GPIO line instead of the hardware reset
> logic. In case of such boards, before performing system reboot,
> additional reset of eMMC card is required to boot again properly.
> This patch adds code for handling such cases.

mmc core supported to hw_reset function.
So i think we can use it. It's called at only initial time to clear the 
previous status.
But i think it can be called at reboot time. (it needs to implement codes.)
how about?

Best Regards,
Jaehoon Chung

> 
> Signed-off-by: Marek Szyprowski 
> ---
>  .../devicetree/bindings/mmc/exynos-dw-mshc.txt |  6 +++
>  drivers/mmc/host/dw_mmc-exynos.c   | 43 
> +-
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt 
> b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
> index ee4fc0576c7d..fc53d335e7db 100644
> --- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
> +++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
> @@ -50,6 +50,12 @@ Required Properties:
>- if CIU clock divider value is 0 (that is divide by 1), both tx and rx
>  phase shift clocks should be 0.
>  
> +Optional properties:
> +
> +* dw-mshc-reset-gpios: optional property specifying gpio for the eMMC nreset
> +  line, it will be triggered on system reboot to properly reset eMMC card for
> +  next system boot.
> +
>  Required properties for a slot (Deprecated - Recommend to use one slot per 
> host):
>  
>  * gpios: specifies a list of gpios used for command, clock and data bus. The
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c 
> b/drivers/mmc/host/dw_mmc-exynos.c
> index 509365cb22c6..2add5a93859d 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -12,12 +12,14 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "dw_mmc.h"
>  #include "dw_mmc-pltfm.h"
> @@ -77,8 +79,23 @@ struct dw_mci_exynos_priv_data {
>   u32 sdr_timing;
>   u32 ddr_timing;
>   u32 cur_speed;
> + struct gpio_desc*reset_gpio;
> + struct notifier_block   reset_nb;
>  };
>  
> +static int dw_mci_restart_handler(struct notifier_block *this,
> +   unsigned long mode, void *cmd)
> +{
> + struct dw_mci_exynos_priv_data *data;
> + data = container_of(this, struct dw_mci_exynos_priv_data, reset_nb);
> +
> + gpiod_direction_output(data->reset_gpio, 0);
> + mdelay(150);
> + gpiod_direction_output(data->reset_gpio, 1);
> +
> + return NOTIFY_DONE;
> +}
> +
>  static struct dw_mci_exynos_compatible {
>   char*compatible;
>   enum dw_mci_exynos_type ctrl_type;
> @@ -295,7 +312,20 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
>   return ret;
>  
>   priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
> +
> + priv->reset_gpio = devm_gpiod_get_optional(host->dev,
> +"samsung,dw-mshc-reset",
> +GPIOD_OUT_HIGH);
> + if (!IS_ERR_OR_NULL(priv->reset_gpio)) {
> + priv->reset_nb.notifier_call = dw_mci_restart_handler;
> + priv->reset_nb.priority = 255;
> + ret = register_restart_handler(&priv->reset_nb);
> + if (ret)
> + dev_err(host->dev, "cannot register restart handler\n");
> + }
> +
>   host->priv = priv;
> +
>   return 0;
>  }
>  
> @@ -490,6 +520,17 @@ static int dw_mci_exynos_probe(struct platform_device 
> *pdev)
>   return dw_mci_pltfm_register(pdev, drv_data);
>  }
>  
> +static int dw_mci_exynos_remove(struct platform_device *pdev)
> +{
> + struct dw_mci *host = platform_get_drvdata(pdev);
> + struct dw_mci_exynos_priv_data *priv = host->priv;
> +
> + if (priv->reset_gpio)
> + unregister_restart_handler(&priv->reset_nb);
> +
> + return dw_mci_pltfm_remove(pdev);
> +}
> +
>  static const struct dev_pm_ops dw_mci_exynos_pmops = {
>   SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
>   .resume_noirq = dw_mci_exynos_resume_noirq,
> @@ -499,7 +540,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
>  
>  static struct platform_driver dw_mci_exynos_pltfm_driver = {
>   .probe  = dw_mci_exynos_probe,
> - .remove = __exit_p(dw_mci_pltfm_remove),
> + .remove = dw_mci_exynos_remove,
>   .driver = {
>   .name   

Re: [PATCH 1/2] power: reset: add driver for Hardkernel's Odroid boards

[Marek Szyprowski]

Hello,

On 2015-01-25 15:32, Sebastian Reichel wrote:

On Fri, Jan 23, 2015 at 12:11:22PM +0100, Marek Szyprowski wrote:

Frankly, I analyzed this case once again and I came to conclusion
that there is no need to make a separate reset driver for Odroid
boards. There is nothing special, specific to whole board about
this gpio. It is rather a property of MMC host controller and eMMC
card that is connected to it. When only gpio toggling code is
moved to reset handler registered from mmc controller, the board
properly performs reboot with a generic exynos4 code.

OK, so I guess this will be fixed independently via mmc subsystem.


I've posted an updated patch.




The poweroff code in above driver is just a generic Exynos4 code,
so again there is no need to duplicate it.

OK. It seems there is a driver for that in arch/arm/mach-exynos.
Otherwise I would have suggest to create something like
syscon-reboot for shutdown.


By moving the code to mmc driver, the same approach can be used for other
Odroid boards (XU/XU3) and maybe even other boards which need manual
resetting of eMMC cards to properly perform reboot procedure.

I suggest to start a new thread for discussing this, which includes
linux-mmc. It might be interesting to provide some more details about
eMMC_nDET, since MMC already contain a reset signal (which seems to
be used according to the odroid schematics I found).


My fault. The documentation for this initial driver was incorrect. The 
reboot
fix has noting to eMMC_nDET signal. It should be eMMC nreset, which is 
routed

directly to SoC GPIO line with external pull-up resistor. I really have no
idea why I wrote eMMC_nDET instead of nreset.

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

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

[PATCH 1/2] mmc: dw_mmc-exynos: add support for controlling emmc reset pin

[Marek Szyprowski]

There are boards (like Hardkernel's Odroid boards) on which eMMC card's
reset line is connected to SoC GPIO line instead of the hardware reset
logic. In case of such boards, before performing system reboot,
additional reset of eMMC card is required to boot again properly.
This patch adds code for handling such cases.

Signed-off-by: Marek Szyprowski 
---
 .../devicetree/bindings/mmc/exynos-dw-mshc.txt |  6 +++
 drivers/mmc/host/dw_mmc-exynos.c   | 43 +-
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt 
b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
index ee4fc0576c7d..fc53d335e7db 100644
--- a/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
+++ b/Documentation/devicetree/bindings/mmc/exynos-dw-mshc.txt
@@ -50,6 +50,12 @@ Required Properties:
   - if CIU clock divider value is 0 (that is divide by 1), both tx and rx
 phase shift clocks should be 0.
 
+Optional properties:
+
+* dw-mshc-reset-gpios: optional property specifying gpio for the eMMC nreset
+  line, it will be triggered on system reboot to properly reset eMMC card for
+  next system boot.
+
 Required properties for a slot (Deprecated - Recommend to use one slot per 
host):
 
 * gpios: specifies a list of gpios used for command, clock and data bus. The
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 509365cb22c6..2add5a93859d 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -12,12 +12,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
@@ -77,8 +79,23 @@ struct dw_mci_exynos_priv_data {
u32 sdr_timing;
u32 ddr_timing;
u32 cur_speed;
+   struct gpio_desc*reset_gpio;
+   struct notifier_block   reset_nb;
 };
 
+static int dw_mci_restart_handler(struct notifier_block *this,
+ unsigned long mode, void *cmd)
+{
+   struct dw_mci_exynos_priv_data *data;
+   data = container_of(this, struct dw_mci_exynos_priv_data, reset_nb);
+
+   gpiod_direction_output(data->reset_gpio, 0);
+   mdelay(150);
+   gpiod_direction_output(data->reset_gpio, 1);
+
+   return NOTIFY_DONE;
+}
+
 static struct dw_mci_exynos_compatible {
char*compatible;
enum dw_mci_exynos_type ctrl_type;
@@ -295,7 +312,20 @@ static int dw_mci_exynos_parse_dt(struct dw_mci *host)
return ret;
 
priv->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0], timing[1], div);
+
+   priv->reset_gpio = devm_gpiod_get_optional(host->dev,
+  "samsung,dw-mshc-reset",
+  GPIOD_OUT_HIGH);
+   if (!IS_ERR_OR_NULL(priv->reset_gpio)) {
+   priv->reset_nb.notifier_call = dw_mci_restart_handler;
+   priv->reset_nb.priority = 255;
+   ret = register_restart_handler(&priv->reset_nb);
+   if (ret)
+   dev_err(host->dev, "cannot register restart handler\n");
+   }
+
host->priv = priv;
+
return 0;
 }
 
@@ -490,6 +520,17 @@ static int dw_mci_exynos_probe(struct platform_device 
*pdev)
return dw_mci_pltfm_register(pdev, drv_data);
 }
 
+static int dw_mci_exynos_remove(struct platform_device *pdev)
+{
+   struct dw_mci *host = platform_get_drvdata(pdev);
+   struct dw_mci_exynos_priv_data *priv = host->priv;
+
+   if (priv->reset_gpio)
+   unregister_restart_handler(&priv->reset_nb);
+
+   return dw_mci_pltfm_remove(pdev);
+}
+
 static const struct dev_pm_ops dw_mci_exynos_pmops = {
SET_SYSTEM_SLEEP_PM_OPS(dw_mci_exynos_suspend, dw_mci_exynos_resume)
.resume_noirq = dw_mci_exynos_resume_noirq,
@@ -499,7 +540,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
 
 static struct platform_driver dw_mci_exynos_pltfm_driver = {
.probe  = dw_mci_exynos_probe,
-   .remove = __exit_p(dw_mci_pltfm_remove),
+   .remove = dw_mci_exynos_remove,
.driver = {
.name   = "dwmmc_exynos",
.of_match_table = dw_mci_exynos_match,
-- 
1.9.2

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

[PATCH 2/2] ARM: dts: exynos*-odroid*: add eMMC reset line

[Marek Szyprowski]

This patch add samsung,dw-mshc-reset-gpios property to the eMMC slot,
so Exynos DW MMC driver is able to properly reset eMMC card on system
restart and thus fixes hang on software reboot.

Signed-off-by: Marek Szyprowski 
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 1 +
 arch/arm/boot/dts/exynos5422-odroidxu3.dts  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi 
b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index 74e89cbfb00c..97df0ef81d1a 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -69,6 +69,7 @@
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <2 3>;
samsung,dw-mshc-ddr-timing = <1 2>;
+   samsung,dw-mshc-reset-gpios = <&gpk1 2 1>;
bus-width = <8>;
cap-mmc-highspeed;
};
diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3.dts 
b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
index d829e220eedb..20064e36a5be 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu3.dts
+++ b/arch/arm/boot/dts/exynos5422-odroidxu3.dts
@@ -336,6 +336,7 @@
samsung,dw-mshc-ciu-div = <3>;
samsung,dw-mshc-sdr-timing = <0 4>;
samsung,dw-mshc-ddr-timing = <0 2>;
+   samsung,dw-mshc-reset-gpios = <&gpd1 0 0>;
pinctrl-names = "default";
pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_bus4 &sd0_bus8>;
bus-width = <8>;
-- 
1.9.2

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

[PATCH 0/2] Fix reboot issue on Odroid boards with eMMC card

[Marek Szyprowski]

Hello,

This patchset fixes reboot hang issue on Hardkernel's Odroid boards with
eMMC card. Those boards are designed in such a way, that the eMMC nreset
signal is routed to SoC GPIO line instead of the board reset logic. To
properly restard system, one need to set this line to zero before
performing reboot.

The initial patches consisted of a complete reset/power off driver, but
after further analysis, it turned out that only eMMC nreset line control
logic is specific for those boards. Everything else can be done by
generic Exynos code, so the code has been moved to Exynos DW MMC driver.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Changelog:

initial version:
http://thread.gmane.org/gmane.linux.power-management.general/51855/


Path summary:

Marek Szyprowski (2):
  mmc: dw_mmc-exynos: add support for controlling emmc reset pin
  ARM: dts: exynos*-odroid*: add eMMC reset line

 .../devicetree/bindings/mmc/exynos-dw-mshc.txt |  6 +++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi|  1 +
 arch/arm/boot/dts/exynos5422-odroidxu3.dts |  1 +
 drivers/mmc/host/dw_mmc-exynos.c   | 43 +-
 4 files changed, 50 insertions(+), 1 deletion(-)

-- 
1.9.2

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