Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
On Thu, Nov 6, 2014 at 12:11 AM, Kevin Hilman wrote:
> From: Ulf Hansson
> Date: Thu, 30 Oct 2014 13:02:49 +0100
> Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
> callbacks
>
> Convert the prototypes to return an int in order to support error
> handling in these callbacks.
>
> Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
> inside the callbacks, and so that they match the existing
> power_on/power_off callbacks which currently take the domain pointer.
>
> Acked-by: Dmitry Torokhov
> [khilman: added domain as parameter to callbacks, as suggested by Dmitry]
OK, so back to the signature of the original version...
> Signed-off-by: Ulf Hansson
> Signed-off-by: Kevin Hilman
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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 02/12] drivers: mfd: Add support for Exynos PMU driver
On Thu, Nov 6, 2014 at 11:18 AM, Pankaj Dubey wrote: > On Wednesday, November 05, 2014 7:18 PM, Sylwester Nawrocki wrote: >> On 04/11/14 04:18, Pankaj Dubey wrote: >> > 2: Since PM domain relies on PMU registers and does not have its own >> > DT binding, MFD client and MFD device is most suitable for making >> > this kind of platform drivers. >> >> We have DT binding for the Exynos power domains: >> Documentation/devicetree/bindings/arm/exynos/power_domain.txt >> In fact the IO memory regions used in the power domain bindings fall >> within the >> (sparse) PMU registers region. >> > > Correct. But currently there is no platform driver for those DT bindings so > we have two options: > 1) Register a new platform driver for each of those DT nodes, which are using > a very small subset of PMU register region. And we end up getting platform > driver's probe for each pd nodes. > 2) Register an Exynos power domain driver which will be a client driver of > main Exynos PMU driver, and will get platform data from it. And in this probe > parse all pd device nodes and populate all related information from those > nodes into Exynos_pm_domain struct instances and keep list of these > instances. Same has been proposed in Amit's patch. > > I would prefer second method, as next, we can move pm sleep functionality > (which also uses PMU register sets) also into such small mfd client driver. > And as far as I know for pm sleep currently there is no DT nodes. > >> There is also DT binding for the PMU subsystem: >> Documentation/devicetree/bindings/arm/samsung/pmu.txt > > Yes, and currently I have used same binding to convert PMU implementation > into a platform driver [1]. Now what we trying to address is same PMU > implementation can be reused for ARM64 based SoC if we have to move it into > "drivers/mfd/" or "drivers/power/" or may be "drivers/soc". > > [1]: http://www.spinics.net/lists/linux-samsung-soc/msg37572.html > > Currently we moved into "drivers/mfd" and patches from Amit shows an use case > where power domain implementation has been converted into platform driver, > but its probe will be called by MFD subsystem and will not be bind to DT > nodes of pd as it will be a client device to main PMU device. Please check > this [2], [3]. > > [2]: http://www.spinics.net/lists/linux-samsung-soc/msg38446.html > [3]: http://www.spinics.net/lists/linux-samsung-soc/msg38447.html > >> >> I guess a platform device driver in drivers/power would be a better >> fit than drivers/mfd. I doubt introducing an mfd driver now makes much >> sense, we have already DT bindings for PHYs, the power domains, etc. >> > > The reason behind making it a mfd driver is, to make following design for pm > domain and pm sleep functionality. > -- > | Exynos - PMU | > | MFD device | > | drivers/mfd/ | > > | > --- > | | > --- > > | Exynos -pd | | Exynos - pm-sleep > | > | PMU's client device| | PMU's client device | > | (drivers/soc/samsung/) | | (drivers/soc/samsung/) | > > - I will post this sleep driver shortly. I also suggest to keep everything in drivers/soc/samsung as all these drivers very much SoC specific. > > As Lee mentioned MFD can be used to register the device ( I am assuming he is > referring to use of " mfd_add_devices" from Exynos -PMU driver, to make it a > mfd device) but it should be kept outside of "drivers/mfd". I just looked for > current users of "mfd_add_devices" but only drivers from "drivers/mfd" only > are calling this API, also once we call this API it will become an MFD device > then why not keep into "drivers/mfd" or maybe I have little understanding of > purpose of "drivers/mfd". > > But still if it's OK to register Exynos -PMU as MFD device even if it's not > in "drivers/mfd" folder, then I would suggest better to keep it in > "drivers/soc/samsung/" as it's very much SoC specific. As "drivers/power" > looks like place to keep external power supply drivers such as batteries, AC, > USB etc. > > Thanks, > Pankaj Dubey > >> -- >> Thanks, >> Sylwester > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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
RE: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver
On Wednesday, November 05, 2014 7:18 PM, Sylwester Nawrocki wrote: > On 04/11/14 04:18, Pankaj Dubey wrote: > > 2: Since PM domain relies on PMU registers and does not have its own > > DT binding, MFD client and MFD device is most suitable for making > > this kind of platform drivers. > > We have DT binding for the Exynos power domains: > Documentation/devicetree/bindings/arm/exynos/power_domain.txt > In fact the IO memory regions used in the power domain bindings fall > within the > (sparse) PMU registers region. > Correct. But currently there is no platform driver for those DT bindings so we have two options: 1) Register a new platform driver for each of those DT nodes, which are using a very small subset of PMU register region. And we end up getting platform driver's probe for each pd nodes. 2) Register an Exynos power domain driver which will be a client driver of main Exynos PMU driver, and will get platform data from it. And in this probe parse all pd device nodes and populate all related information from those nodes into Exynos_pm_domain struct instances and keep list of these instances. Same has been proposed in Amit's patch. I would prefer second method, as next, we can move pm sleep functionality (which also uses PMU register sets) also into such small mfd client driver. And as far as I know for pm sleep currently there is no DT nodes. > There is also DT binding for the PMU subsystem: > Documentation/devicetree/bindings/arm/samsung/pmu.txt Yes, and currently I have used same binding to convert PMU implementation into a platform driver [1]. Now what we trying to address is same PMU implementation can be reused for ARM64 based SoC if we have to move it into "drivers/mfd/" or "drivers/power/" or may be "drivers/soc". [1]: http://www.spinics.net/lists/linux-samsung-soc/msg37572.html Currently we moved into "drivers/mfd" and patches from Amit shows an use case where power domain implementation has been converted into platform driver, but its probe will be called by MFD subsystem and will not be bind to DT nodes of pd as it will be a client device to main PMU device. Please check this [2], [3]. [2]: http://www.spinics.net/lists/linux-samsung-soc/msg38446.html [3]: http://www.spinics.net/lists/linux-samsung-soc/msg38447.html > > I guess a platform device driver in drivers/power would be a better > fit than drivers/mfd. I doubt introducing an mfd driver now makes much > sense, we have already DT bindings for PHYs, the power domains, etc. > The reason behind making it a mfd driver is, to make following design for pm domain and pm sleep functionality. -- | Exynos - PMU | | MFD device | | drivers/mfd/ | | --- | | --- | Exynos -pd | | Exynos - pm-sleep | | PMU's client device| | PMU's client device | | (drivers/soc/samsung/) | | (drivers/soc/samsung/) | - As Lee mentioned MFD can be used to register the device ( I am assuming he is referring to use of " mfd_add_devices" from Exynos -PMU driver, to make it a mfd device) but it should be kept outside of "drivers/mfd". I just looked for current users of "mfd_add_devices" but only drivers from "drivers/mfd" only are calling this API, also once we call this API it will become an MFD device then why not keep into "drivers/mfd" or maybe I have little understanding of purpose of "drivers/mfd". But still if it's OK to register Exynos -PMU as MFD device even if it's not in "drivers/mfd" folder, then I would suggest better to keep it in "drivers/soc/samsung/" as it's very much SoC specific. As "drivers/power" looks like place to keep external power supply drivers such as batteries, AC, USB etc. Thanks, Pankaj Dubey > -- > Thanks, > Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
On Wednesday, November 05, 2014 03:11:23 PM Kevin Hilman wrote:
> "Rafael J. Wysocki" writes:
>
> > On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
> >> Dmitry Torokhov writes:
> >>
> >> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
> >> >> "Rafael J. Wysocki" writes:
> >> >>
> >> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> >> >> >> Convert the prototype to return and int. This is just an initial
> >> >> >> step,
> >> >> >> needed to support error handling.
> >> >> >>
> >> >> >> Signed-off-by: Ulf Hansson
> >> >>
> >> >> Acked-by: Kevin Hilman
> >> >>
> >> >> >>
> >> >> >> This patch is intended as fix for 3.18 rc[n]. Why?
> >> >> >>
> >> >> >> There are other SOC specific patches around that adds genpd support
> >> >> >> and which
> >> >> >> implements the ->attach_dev() callback. To prevent having an
> >> >> >> "atomic" patch
> >> >> >> during the next release cycle, let's change the prototype now
> >> >> >> instead.
> >> >> >>
> >> >> >> Further patches will add the actual error handling in genpd and
> >> >> >> these can then
> >> >> >> be reviewed and tested thoroughly.
> >> >> >
> >> >> > So we have no users of ->attach_dev at the moment, right?
> >> >>
> >> >> Not in mainline, but there are a couple getting ready to hit -next, so
> >> >> we wanted to fix this before they arrive so that adding the error
> >> >> handling will be easier.
> >> >
> >> > BTW, while we are at it, can we also pass the domain itself to
> >> > attach_dev() and detach_dev()? If anything it helps with debugging (you
> >> > can print domain name from the callbacks).
> >>
> >> Agreed, and it makes it match the other callbacks (power_off, power_on)
> >> which currently take struct generic_pm_domain *domain.
> >>
> >> Updated version of $SUBJECT patch below.
> >
> > The subject and changelog need to be updated too IMO.
> >
>
> Right. Here you go.
I've replaced the Ulf's original with this one, thanks!
> - >8 --
> From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson
> Date: Thu, 30 Oct 2014 13:02:49 +0100
> Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
> callbacks
>
> Convert the prototypes to return an int in order to support error
> handling in these callbacks.
>
> Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
> inside the callbacks, and so that they match the existing
> power_on/power_off callbacks which currently take the domain pointer.
>
> Acked-by: Dmitry Torokhov
> [khilman: added domain as parameter to callbacks, as suggested by Dmitry]
> Signed-off-by: Ulf Hansson
> Signed-off-by: Kevin Hilman
> ---
> drivers/base/power/domain.c | 4 ++--
> include/linux/pm_domain.h | 6 --
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4072cc..b520687046d4 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain
> *genpd, struct device *dev,
> spin_unlock_irq(&dev->power.lock);
>
> if (genpd->attach_dev)
> - genpd->attach_dev(dev);
> + genpd->attach_dev(genpd, dev);
>
> mutex_lock(&gpd_data->lock);
> gpd_data->base.dev = dev;
> @@ -1499,7 +1499,7 @@ int pm_genpd_remove_device(struct generic_pm_domain
> *genpd,
> genpd->max_off_time_changed = true;
>
> if (genpd->detach_dev)
> - genpd->detach_dev(dev);
> + genpd->detach_dev(genpd, dev);
>
> spin_lock_irq(&dev->power.lock);
>
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 73e938b7e937..b3ed7766a291 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -72,8 +72,10 @@ struct generic_pm_domain {
> bool max_off_time_changed;
> bool cached_power_down_ok;
> struct gpd_cpuidle_data *cpuidle_data;
> - void (*attach_dev)(struct device *dev);
> - void (*detach_dev)(struct device *dev);
> + int (*attach_dev)(struct generic_pm_domain *domain,
> + struct device *dev);
> + void (*detach_dev)(struct generic_pm_domain *domain,
> +struct device *dev);
> };
>
> static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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] PM / Domains: Change prototype for the ->attach_dev() callback
"Rafael J. Wysocki" writes:
> On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
>> Dmitry Torokhov writes:
>>
>> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
>> >> "Rafael J. Wysocki" writes:
>> >>
>> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
>> >> >> Convert the prototype to return and int. This is just an initial step,
>> >> >> needed to support error handling.
>> >> >>
>> >> >> Signed-off-by: Ulf Hansson
>> >>
>> >> Acked-by: Kevin Hilman
>> >>
>> >> >>
>> >> >> This patch is intended as fix for 3.18 rc[n]. Why?
>> >> >>
>> >> >> There are other SOC specific patches around that adds genpd support
>> >> >> and which
>> >> >> implements the ->attach_dev() callback. To prevent having an "atomic"
>> >> >> patch
>> >> >> during the next release cycle, let's change the prototype now instead.
>> >> >>
>> >> >> Further patches will add the actual error handling in genpd and these
>> >> >> can then
>> >> >> be reviewed and tested thoroughly.
>> >> >
>> >> > So we have no users of ->attach_dev at the moment, right?
>> >>
>> >> Not in mainline, but there are a couple getting ready to hit -next, so
>> >> we wanted to fix this before they arrive so that adding the error
>> >> handling will be easier.
>> >
>> > BTW, while we are at it, can we also pass the domain itself to
>> > attach_dev() and detach_dev()? If anything it helps with debugging (you
>> > can print domain name from the callbacks).
>>
>> Agreed, and it makes it match the other callbacks (power_off, power_on)
>> which currently take struct generic_pm_domain *domain.
>>
>> Updated version of $SUBJECT patch below.
>
> The subject and changelog need to be updated too IMO.
>
Right. Here you go.
Kevin
- >8 --
>From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001
From: Ulf Hansson
Date: Thu, 30 Oct 2014 13:02:49 +0100
Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
callbacks
Convert the prototypes to return an int in order to support error
handling in these callbacks.
Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
inside the callbacks, and so that they match the existing
power_on/power_off callbacks which currently take the domain pointer.
Acked-by: Dmitry Torokhov
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
Signed-off-by: Ulf Hansson
Signed-off-by: Kevin Hilman
---
drivers/base/power/domain.c | 4 ++--
include/linux/pm_domain.h | 6 --
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4072cc..b520687046d4 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain
*genpd, struct device *dev,
spin_unlock_irq(&dev->power.lock);
if (genpd->attach_dev)
- genpd->attach_dev(dev);
+ genpd->attach_dev(genpd, dev);
mutex_lock(&gpd_data->lock);
gpd_data->base.dev = dev;
@@ -1499,7 +1499,7 @@ int pm_genpd_remove_device(struct generic_pm_domain
*genpd,
genpd->max_off_time_changed = true;
if (genpd->detach_dev)
- genpd->detach_dev(dev);
+ genpd->detach_dev(genpd, dev);
spin_lock_irq(&dev->power.lock);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 73e938b7e937..b3ed7766a291 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -72,8 +72,10 @@ struct generic_pm_domain {
bool max_off_time_changed;
bool cached_power_down_ok;
struct gpd_cpuidle_data *cpuidle_data;
- void (*attach_dev)(struct device *dev);
- void (*detach_dev)(struct device *dev);
+ int (*attach_dev)(struct generic_pm_domain *domain,
+ struct device *dev);
+ void (*detach_dev)(struct generic_pm_domain *domain,
+ struct device *dev);
};
static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
> Dmitry Torokhov writes:
>
> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
> >> "Rafael J. Wysocki" writes:
> >>
> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> >> >> Convert the prototype to return and int. This is just an initial step,
> >> >> needed to support error handling.
> >> >>
> >> >> Signed-off-by: Ulf Hansson
> >>
> >> Acked-by: Kevin Hilman
> >>
> >> >>
> >> >> This patch is intended as fix for 3.18 rc[n]. Why?
> >> >>
> >> >> There are other SOC specific patches around that adds genpd support and
> >> >> which
> >> >> implements the ->attach_dev() callback. To prevent having an "atomic"
> >> >> patch
> >> >> during the next release cycle, let's change the prototype now instead.
> >> >>
> >> >> Further patches will add the actual error handling in genpd and these
> >> >> can then
> >> >> be reviewed and tested thoroughly.
> >> >
> >> > So we have no users of ->attach_dev at the moment, right?
> >>
> >> Not in mainline, but there are a couple getting ready to hit -next, so
> >> we wanted to fix this before they arrive so that adding the error
> >> handling will be easier.
> >
> > BTW, while we are at it, can we also pass the domain itself to
> > attach_dev() and detach_dev()? If anything it helps with debugging (you
> > can print domain name from the callbacks).
>
> Agreed, and it makes it match the other callbacks (power_off, power_on)
> which currently take struct generic_pm_domain *domain.
>
> Updated version of $SUBJECT patch below.
The subject and changelog need to be updated too IMO.
> - >8 --
> From 353a62ffae2f9228142c8a2093108f9eda8dc6b4 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson
> Date: Thu, 30 Oct 2014 13:02:49 +0100
> Subject: [PATCH] PM / Domains: Change prototype for the ->attach_dev()
> callback
>
> Convert the prototype to return and int. This is just an initial step,
> needed to support error handling.
>
> Cc: Dmitry Torokhov
> [khilman: added domain as parameter to callbacks, as suggested by Dmitry]
> Signed-off-by: Ulf Hansson
> Signed-off-by: Kevin Hilman
> ---
> drivers/base/power/domain.c | 4 ++--
> include/linux/pm_domain.h | 6 --
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4072cc..b520687046d4 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain
> *genpd, struct device *dev,
> spin_unlock_irq(&dev->power.lock);
>
> if (genpd->attach_dev)
> - genpd->attach_dev(dev);
> + genpd->attach_dev(genpd, dev);
>
> mutex_lock(&gpd_data->lock);
> gpd_data->base.dev = dev;
> @@ -1499,7 +1499,7 @@ int pm_genpd_remove_device(struct generic_pm_domain
> *genpd,
> genpd->max_off_time_changed = true;
>
> if (genpd->detach_dev)
> - genpd->detach_dev(dev);
> + genpd->detach_dev(genpd, dev);
>
> spin_lock_irq(&dev->power.lock);
>
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 73e938b7e937..b3ed7766a291 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -72,8 +72,10 @@ struct generic_pm_domain {
> bool max_off_time_changed;
> bool cached_power_down_ok;
> struct gpd_cpuidle_data *cpuidle_data;
> - void (*attach_dev)(struct device *dev);
> - void (*detach_dev)(struct device *dev);
> + int (*attach_dev)(struct generic_pm_domain *domain,
> + struct device *dev);
> + void (*detach_dev)(struct generic_pm_domain *domain,
> +struct device *dev);
> };
>
> static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
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] PM / Domains: Change prototype for the ->attach_dev() callback
On Wed, Nov 05, 2014 at 02:43:31PM -0800, Kevin Hilman wrote: > Dmitry Torokhov writes: > > > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote: > >> "Rafael J. Wysocki" writes: > >> > >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote: > >> >> Convert the prototype to return and int. This is just an initial step, > >> >> needed to support error handling. > >> >> > >> >> Signed-off-by: Ulf Hansson > >> > >> Acked-by: Kevin Hilman > >> > >> >> > >> >> This patch is intended as fix for 3.18 rc[n]. Why? > >> >> > >> >> There are other SOC specific patches around that adds genpd support and > >> >> which > >> >> implements the ->attach_dev() callback. To prevent having an "atomic" > >> >> patch > >> >> during the next release cycle, let's change the prototype now instead. > >> >> > >> >> Further patches will add the actual error handling in genpd and these > >> >> can then > >> >> be reviewed and tested thoroughly. > >> > > >> > So we have no users of ->attach_dev at the moment, right? > >> > >> Not in mainline, but there are a couple getting ready to hit -next, so > >> we wanted to fix this before they arrive so that adding the error > >> handling will be easier. > > > > BTW, while we are at it, can we also pass the domain itself to > > attach_dev() and detach_dev()? If anything it helps with debugging (you > > can print domain name from the callbacks). > > Agreed, and it makes it match the other callbacks (power_off, power_on) > which currently take struct generic_pm_domain *domain. > > Updated version of $SUBJECT patch below. > > Kevin > > > - >8 -- > From 353a62ffae2f9228142c8a2093108f9eda8dc6b4 Mon Sep 17 00:00:00 2001 > From: Ulf Hansson > Date: Thu, 30 Oct 2014 13:02:49 +0100 > Subject: [PATCH] PM / Domains: Change prototype for the ->attach_dev() > callback > > Convert the prototype to return and int. This is just an initial step, > needed to support error handling. > > Cc: Dmitry Torokhov > [khilman: added domain as parameter to callbacks, as suggested by Dmitry] > Signed-off-by: Ulf Hansson > Signed-off-by: Kevin Hilman Acked-by: Dmitry Torokhov > --- > drivers/base/power/domain.c | 4 ++-- > include/linux/pm_domain.h | 6 -- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 40bc2f4072cc..b520687046d4 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain > *genpd, struct device *dev, > spin_unlock_irq(&dev->power.lock); > > if (genpd->attach_dev) > - genpd->attach_dev(dev); > + genpd->attach_dev(genpd, dev); I guess as a followup we need to propagate the error returned by genpd->attach_dev() here. Thanks. -- 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] PM / Domains: Change prototype for the ->attach_dev() callback
Dmitry Torokhov writes:
> On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
>> "Rafael J. Wysocki" writes:
>>
>> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
>> >> Convert the prototype to return and int. This is just an initial step,
>> >> needed to support error handling.
>> >>
>> >> Signed-off-by: Ulf Hansson
>>
>> Acked-by: Kevin Hilman
>>
>> >>
>> >> This patch is intended as fix for 3.18 rc[n]. Why?
>> >>
>> >> There are other SOC specific patches around that adds genpd support and
>> >> which
>> >> implements the ->attach_dev() callback. To prevent having an "atomic"
>> >> patch
>> >> during the next release cycle, let's change the prototype now instead.
>> >>
>> >> Further patches will add the actual error handling in genpd and these can
>> >> then
>> >> be reviewed and tested thoroughly.
>> >
>> > So we have no users of ->attach_dev at the moment, right?
>>
>> Not in mainline, but there are a couple getting ready to hit -next, so
>> we wanted to fix this before they arrive so that adding the error
>> handling will be easier.
>
> BTW, while we are at it, can we also pass the domain itself to
> attach_dev() and detach_dev()? If anything it helps with debugging (you
> can print domain name from the callbacks).
Agreed, and it makes it match the other callbacks (power_off, power_on)
which currently take struct generic_pm_domain *domain.
Updated version of $SUBJECT patch below.
Kevin
- >8 --
>From 353a62ffae2f9228142c8a2093108f9eda8dc6b4 Mon Sep 17 00:00:00 2001
From: Ulf Hansson
Date: Thu, 30 Oct 2014 13:02:49 +0100
Subject: [PATCH] PM / Domains: Change prototype for the ->attach_dev()
callback
Convert the prototype to return and int. This is just an initial step,
needed to support error handling.
Cc: Dmitry Torokhov
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
Signed-off-by: Ulf Hansson
Signed-off-by: Kevin Hilman
---
drivers/base/power/domain.c | 4 ++--
include/linux/pm_domain.h | 6 --
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4072cc..b520687046d4 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain
*genpd, struct device *dev,
spin_unlock_irq(&dev->power.lock);
if (genpd->attach_dev)
- genpd->attach_dev(dev);
+ genpd->attach_dev(genpd, dev);
mutex_lock(&gpd_data->lock);
gpd_data->base.dev = dev;
@@ -1499,7 +1499,7 @@ int pm_genpd_remove_device(struct generic_pm_domain
*genpd,
genpd->max_off_time_changed = true;
if (genpd->detach_dev)
- genpd->detach_dev(dev);
+ genpd->detach_dev(genpd, dev);
spin_lock_irq(&dev->power.lock);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 73e938b7e937..b3ed7766a291 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -72,8 +72,10 @@ struct generic_pm_domain {
bool max_off_time_changed;
bool cached_power_down_ok;
struct gpd_cpuidle_data *cpuidle_data;
- void (*attach_dev)(struct device *dev);
- void (*detach_dev)(struct device *dev);
+ int (*attach_dev)(struct generic_pm_domain *domain,
+ struct device *dev);
+ void (*detach_dev)(struct generic_pm_domain *domain,
+ struct device *dev);
};
static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG 3.18-rc3] kernel BUG at drivers/iommu/exynos-iommu.c:481!
Hi, I'm seeing the following bug running the current linus tree of today, it seems this was introduced by the following commit: commit 6b21a5db36427d54a94091fc26b5890c3d6a8af3 Author: Cho KyongHo Date: Mon May 12 11:45:02 2014 +0530 iommu/exynos: Support for device tree This commit adds device tree support for System MMU. Also, system mmu handling is improved. Previously, an IOMMU domain is bound to a System MMU which is not correct. This patch binds an IOMMU domain with the master device of a System MMU. Signed-off-by: Cho KyongHo Signed-off-by: Shaik Ameer Basha Signed-off-by: Joerg Roedel It is failing on a BUG_ON(!has_sysmmu(dev)); Gustavo [3.188618] [ cut here ] [3.188622] kernel BUG at drivers/iommu/exynos-iommu.c:481! [3.188626] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM [3.188633] Modules linked in: [3.188639] CPU: 0 PID: 18 Comm: kworker/u4:1 Not tainted 3.18.0-rc3-00061-ga1cff6e #3 [3.188650] Workqueue: deferwq deferred_probe_work_func [3.188654] task: ee09bc00 ti: ee11 task.ti: ee11 [3.188661] PC is at __exynos_sysmmu_enable+0x174/0x180 [3.188665] LR is at exynos_iommu_attach_device+0x44/0xb0 [3.188670] pc : []lr : []psr: 6193 [3.188670] sp : ee111d00 ip : fp : 6d444000 [3.188674] r10: ed488080 r9 : a113 r8 : ed4880d0 [3.188678] r7 : ee194e10 r6 : ee194e10 r5 : ed488080 r4 : [3.188683] r3 : 6d444000 r2 : ed488080 r1 : 6d444000 r0 : ee194e10 [3.188687] Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel [3.188691] Control: 10c5387d Table: 4000406a DAC: 0015 [3.188695] Process kworker/u4:1 (pid: 18, stack limit = 0xee110240) [3.188699] Stack: (0xee111d00 to 0xee112000) [3.188705] 1d00: 6d444000 ed4880c0 ed488080 ee194e10 ed4880d0 a113 0001 [3.188710] 1d20: c0380074 ed81d410 6d444000 ed488000 ee194e10 ed81d410 eda54400 [3.188715] 1d40: c0668444 ed81d410 0001 c037e28c ed4885d0 c0016624 ed4885d0 ee194e10 [3.188719] 1d60: ee24a010 ed81d410 eda54400 c026c310 0001 0005 ed81d410 c026e0d0 [3.188724] 1d80: c026dfc0 ed8dc840 ee3fe480 ed48cf80 eda54400 0001 c0692738 0003 [3.188729] 1da0: ee00fc00 c0285678 eda54400 eda54400 eda54400 ed48cf80 ed8dcbd0 [3.188733] 1dc0: ed48c500 c0268908 eda54400 c02551e8 ee24a000 eda54400 [3.188738] 1de0: c0691a30 c0256e68 ed48c580 c028c408 ed8dc840 ed8dcbf0 ed8dcbf0 0004 [3.188743] 1e00: ed8dc840 c0285960 ed8dc840 c06930e0 ed48c500 c048c964 c06922a0 c0285b48 [3.188748] 1e20: ee217a10 ee01f810 ee217a00 c0277cfc 2003 c05aecf4 [3.188753] 1e40: ee01f810 ed9828c0 000c fffe ee217a10 fdfb c06926d4 [3.188757] 1e60: 0001 c028ac84 c028ac3c ee217a10 c06e5774 [3.188762] 1e80: c06926d4 c0289838 ee217a10 c02899e8 ee11 ed93a900 c0287fdc [3.188767] 1ea0: ee005298 ee176ec4 ee217a10 ee217a44 c06932c0 c02896f4 ee217a10 ee217a10 [3.188772] 1ec0: c06932c0 c0288e4c ee217a10 c0693240 c0693218 c0289220 ee0f2880 c0693268 [3.188776] 1ee0: ee00fc00 c0032c38 ee00fc00 c065f5c0 c065f5c0 c065f5c0 0001 ee00fc20 [3.188781] 1f00: ee11 ee11 ee0f2898 ee0f2880 ee00fc00 0088 ee00fc00 c0032e90 [3.188786] 1f20: ee11 ee00fc54 ee051b80 ee0f2880 c0032e54 [3.188791] 1f40: c0037630 ee0f2880 [3.188795] 1f60: dead4ead ee111f74 ee111f74 [3.188800] 1f80: dead4ead ee111f90 ee111f90 ee111fac ee051b80 [3.188805] 1fa0: c0037564 c000e878 [3.188809] 1fc0: [3.188814] 1fe0: 0013 [3.188824] [] (__exynos_sysmmu_enable) from [] (exynos_iommu_attach_device+0x44/0xb0) [3.188830] [] (exynos_iommu_attach_device) from [] (iommu_attach_device+0x18/0x24) [3.188839] [] (iommu_attach_device) from [] (arm_iommu_attach_device+0x18/0xa0) [3.188849] [] (arm_iommu_attach_device) from [] (drm_iommu_attach_device+0x4c/0xc8) [3.188857] [] (drm_iommu_attach_device) from [] (fimd_bind+0x110/0x17c) [3.188866] [] (fimd_bind) from [] (component_bind_all+0xb0/0x1cc) [3.188873] [] (component_bind_all) from [] (exynos_drm_load+0x98/0x148) [3.12] [] (exynos_drm_load) from [] (drm_dev_register+0xa0/0x100) [3.188890] [] (drm_dev_register) from [] (drm_platform_init+0x40/0xd0) [3.188898] [] (drm_platform_init) from [] (try_to_bring_up_master.part.2+0xc8/0x104) [3.188905] [] (try_to_bring_up_master.part.2) from [] (component_add+0x84/0xec) [3.188912] [] (co
Re: [PATCH] thermal: cpu_cooling: Update always cpufreq policy with thermal constraints
Hello Yadwinder,
On Wed, Nov 05, 2014 at 05:46:25PM +0530, Yadwinder Singh Brar wrote:
> Existing code updates cupfreq policy only while executing
> cpufreq_apply_cooling() function (i.e. when notify_device != NOTIFY_INVALID).
Correct. The case you mention is when we receive a notification from
cpufreq. But also, updates the cpufreq policy when the cooling device
changes state, a call from thermal framework.
> It doesn't apply constraints when cpufreq policy update happens from any other
> place but it should update the cpufreq policy with thermal constraints every
> time when there is a cpufreq policy update, to keep state of
> cpufreq_cooling_device and max_feq of cpufreq policy in sync.
I am not sure I follow you here. Can you please elaborate on 'any other
places'? Could you please mention (also in the commit log) what are the
case the current code does not cover? For instance, the
cpufreq_apply_cooling gets called on notification coming from thermal
subsystem, and for changes in cpufreq subsystem,
cpufreq_thermal_notifier is called.
>
> This patch modifies code to maintain a global cpufreq_dev_list and get
> corresponding cpufreq_cooling_device for policy's cpu from cpufreq_dev_list
> when there is any policy update.
OK. Please give real examples of when the current code fails to catch
the event.
BR,
Eduardo Valentin
>
> Signed-off-by: Yadwinder Singh Brar
> ---
> drivers/thermal/cpu_cooling.c | 37 -
> 1 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index 1ab0018..5546278 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -50,15 +50,14 @@ struct cpufreq_cooling_device {
> unsigned int cpufreq_state;
> unsigned int cpufreq_val;
> struct cpumask allowed_cpus;
> + struct list_head node;
> };
> static DEFINE_IDR(cpufreq_idr);
> static DEFINE_MUTEX(cooling_cpufreq_lock);
>
> static unsigned int cpufreq_dev_count;
>
> -/* notify_table passes value to the CPUFREQ_ADJUST callback function. */
> -#define NOTIFY_INVALID NULL
> -static struct cpufreq_cooling_device *notify_device;
> +static LIST_HEAD(cpufreq_dev_list);
>
> /**
> * get_idr - function to get a unique id.
> @@ -287,15 +286,12 @@ static int cpufreq_apply_cooling(struct
> cpufreq_cooling_device *cpufreq_device,
>
> cpufreq_device->cpufreq_state = cooling_state;
> cpufreq_device->cpufreq_val = clip_freq;
> - notify_device = cpufreq_device;
>
> for_each_cpu(cpuid, mask) {
> if (is_cpufreq_valid(cpuid))
> cpufreq_update_policy(cpuid);
> }
>
> - notify_device = NOTIFY_INVALID;
> -
> return 0;
> }
>
> @@ -316,21 +312,27 @@ static int cpufreq_thermal_notifier(struct
> notifier_block *nb,
> {
> struct cpufreq_policy *policy = data;
> unsigned long max_freq = 0;
> + struct cpufreq_cooling_device *cpufreq_dev;
>
> - if (event != CPUFREQ_ADJUST || notify_device == NOTIFY_INVALID)
> + if (event != CPUFREQ_ADJUST)
> return 0;
>
> - if (cpumask_test_cpu(policy->cpu, ¬ify_device->allowed_cpus))
> - max_freq = notify_device->cpufreq_val;
> - else
> - return 0;
> + mutex_lock(&cooling_cpufreq_lock);
> + list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) {
> + if (!cpumask_test_cpu(policy->cpu,
> + &cpufreq_dev->allowed_cpus))
> + continue;
>
> - /* Never exceed user_policy.max */
> - if (max_freq > policy->user_policy.max)
> - max_freq = policy->user_policy.max;
> + max_freq = cpufreq_dev->cpufreq_val;
>
> - if (policy->max != max_freq)
> - cpufreq_verify_within_limits(policy, 0, max_freq);
> + /* Never exceed user_policy.max */
> + if (max_freq > policy->user_policy.max)
> + max_freq = policy->user_policy.max;
> +
> + if (policy->max != max_freq)
> + cpufreq_verify_within_limits(policy, 0, max_freq);
> + }
> + mutex_unlock(&cooling_cpufreq_lock);
So, the problem is when we have several cpu cooling devices and you want
to search for the real max among the existing cpu cooling devices?
>
> return 0;
> }
> @@ -486,7 +488,7 @@ __cpufreq_cooling_register(struct device_node *np,
> cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
> CPUFREQ_POLICY_NOTIFIER);
> cpufreq_dev_count++;
> -
> + list_add(&cpufreq_dev->node, &cpufreq_dev_list);
> mutex_unlock(&cooling_cpufreq_lock);
>
> return cool_dev;
> @@ -549,6 +551,7 @@ void cpufreq_cooling_unregister(struct
> thermal_cooling_device *cdev)
>
> cpufreq_dev = cdev->devdata;
> mutex_lock(&cooling_cpufreq_lock);
> + list_del(&cpufreq_d
Re: [PATCH] ASoC: samsung: Add MODULE_DEVICE_TABLE for Snow
On Wed, Nov 05, 2014 at 07:11:06PM +0100, Andreas Färber wrote: > Am 05.11.2014 um 18:09 schrieb Mark Brown: > > On Wed, Nov 05, 2014 at 05:44:52PM +0100, Andreas Färber wrote: > >> This enables the snd_soc_snow module to be auto-loaded. > > Applied, thanks. > Thanks. I notice you dropped my Fixes: line that SubmittingPatches asks > for. When do I need it and when should I leave it out? I thought it > tells GregKH more precisely which stable branches are affected for > backports than just a CC. It's not really adding anything if it's just identifying the patch that created the file in the first place and what actually happened here is that I applied it without the tag then later went back and decided to add a stable Cc since it was probably worth that. signature.asc Description: Digital signature
Re: [PATCH] ASoC: samsung: Add MODULE_DEVICE_TABLE for Snow
Am 05.11.2014 um 18:09 schrieb Mark Brown: > On Wed, Nov 05, 2014 at 05:44:52PM +0100, Andreas Färber wrote: >> This enables the snd_soc_snow module to be auto-loaded. > > Applied, thanks. Thanks. I notice you dropped my Fixes: line that SubmittingPatches asks for. When do I need it and when should I leave it out? I thought it tells GregKH more precisely which stable branches are affected for backports than just a CC. Regards, Andreas -- SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg signature.asc Description: OpenPGP digital signature
Re: Samsung S3C6410 SoC/devicetree: how to describe RTC interrupts connected to two different interrupt parents?
Myself wrote:
> the Samsung S3C6410 SoC comes with an RTC with two interrupts connected to
> two different interrupt controllers.
>
> The old platform code uses this resource to describe it:
>
> static struct resource s3c_rtc_resource[] = {
> [0] = DEFINE_RES_MEM(S3C24XX_PA_RTC, SZ_256),
> [1] = DEFINE_RES_IRQ(IRQ_RTC), <-- channel 2 at VIC0
> [2] = DEFINE_RES_IRQ(IRQ_TICK), <-- channel 28 at VIC1
> };
>
> In the devicetree I can define multiple interrupt channels with the
> keyword 'interrupts' but it seems all of them must share the same interrupt
> parent defined via 'interrupt-parent'. How to handle the special S3C6410
> case?
> The file "Documentation/devicetree/bindings/rtc/s3c-rtc.txt" isn't really
> helpful.
Ahh, "interrupts-extended" is the keyword... solved.
jbo
--
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] ASoC: samsung: Add MODULE_DEVICE_TABLE for Snow
On Wed, Nov 05, 2014 at 05:44:52PM +0100, Andreas Färber wrote: > This enables the snd_soc_snow module to be auto-loaded. Applied, thanks. signature.asc Description: Digital signature
[PATCH] ASoC: samsung: Add MODULE_DEVICE_TABLE for Snow
This enables the snd_soc_snow module to be auto-loaded.
Fixes: da5993df2374 ("ASoC: samsung: Add sound card driver for Snow board")
Signed-off-by: Andreas Färber
---
Tested using patches for Spring, but should apply to Snow/Peach-pi equally.
Rebased onto sound.git for-next branch.
sound/soc/samsung/snow.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/samsung/snow.c b/sound/soc/samsung/snow.c
index 0acf5d0eed53..72118a77dd5b 100644
--- a/sound/soc/samsung/snow.c
+++ b/sound/soc/samsung/snow.c
@@ -110,6 +110,7 @@ static const struct of_device_id snow_of_match[] = {
{ .compatible = "google,snow-audio-max98095", },
{},
};
+MODULE_DEVICE_TABLE(of, snow_of_match);
static struct platform_driver snow_driver = {
.driver = {
--
2.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: Remove references to non-existent PLAT_S5P symbol
On Wed, Nov 05, 2014 at 11:23:36AM +0100, Sylwester Nawrocki wrote:
> On 04/11/14 20:52, Paul Bolle wrote:
> > On Tue, 2014-11-04 at 11:42 -0800, Greg KH wrote:
> >> > As it's something that no one seemed to ever need before (i.e. it's not
> >> > a regression fix), but it would be a "new feature", I don't think it's
> >> > really a stable fix.
> >> >
> >> > But feel free to convince me otherwise :)
> >
> > Sylwester, was I right in thinking that users of PLAT_S5P, who could set
> > USB_EHCI_EXYNOS or USB_OHCI_EXYNOS pre v3.17, got, well, transferred to
> > ARCH_S5PV210 and lost the ability to set one of those symbols in v3.17?
>
> Yes, after commit d78c16ccde96 ("ARM: SAMSUNG: Remove remaining legacy code")
> we lost the ability to enable USB OHCI and EHCI on S5PV210 SoC.
> Thus for those who use the mainline kernel (might be rare) with S5PV210 SoC
> there is obviously a regression in USB subsystem in v3.17.
Ok, I'll add this to 3.17-stable when it hits Linus's tree, thanks.
greg k-h
--
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: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform
Hello Riku,
I will quickly reply to your message, and reserve any further comments
for the other thread.
>My concern is, that none of us with this problem is a linux network
drivers expert. And no such expert joined the thread to help us. Thus if
we hurry to have proper fix for 3.18, our fix might easily be really
wrong.
>
>Hence, it would seem safer to revert to 3.17 state before 3.18, so we
can propose a proper fix for 3.19. At least from our myopic view, having
no functioning net on arndale is worse than having non-functioning
ethtool (which doesn't >seem to have bothered people for years).
Lets agree to disagree here. Any further comment on this would not be
productive.
Kind regards,
Michel Stam
-Original Message-
From: Riku Voipio [mailto:riku.voi...@iki.fi]
Sent: Wednesday, November 05, 2014 13:40 PM
To: Stam, Michel [FINT]
Cc: Charles Keepax; da...@davemloft.net; linux-...@vger.kernel.org;
net...@vger.kernel.org; linux-ker...@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
onarndale platform
Hi Michel,
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> After looking around I found the reset value for the 8772 chip, which
> seems to be 0x1E1 (ANAR register).
>
> This equates to (according to include/uapi/linux/mii.h) ADVERTISE_ALL
> | ADVERTISE_CSMA.
>
> The register only seems to become 0 if the software reset fails.
>
> Unfortunately, this is exactly what I get when the patch is applied;
> asix 1-2:1.0 eth1: Failed to send software reset: ffb5 asix
> 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2, ASIX
> AX88772 USB 2.0 Ethernet asix 1-2:1.0 eth1: Failed to send software
> reset: ffb5 asix 1-2:1.0 eth1: link reset failed (-75) usbnet
> usb-:00:1d.0-2, ASIX AX88772 USB 2.0 Ethernet
>
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s
> command.
> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
> Can you tell me what device is on the Andale platform, Charles? Same
> vendor/device id?
Bus 003 Device 004: ID 0b95:772a ASIX Electronics Corp. AX88772A Fast
Ethernet
> Another thing that bothers me is that dev->mii.advertising seems to
> contain the same value, so maybe that can be used instead of a call to
> asix_mdio_read(). Can anyone comment on its purpose? Should it be a
> shadow copy of the real register or something?
> Riku, can you test Charles' patch as well?
With that patch + revert to ax88772_reset network works. I'm unable to
get ethtool to work with that patch or with the original 3.17 state of
asix.
Once i disable autoneg network doesn't just work.
> > >I think it would better to revert the change now and with less
> > >hurry
> > introduce a ethtool fix that doesn't break arndale.
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself.
> > Fixing the armdale will then cause breakage in other
> > implementations, such as ours. Blankly reverting breaks other
peoples' implementations.
My concern is, that none of us with this problem is a linux network
drivers expert. And no such expert joined the thread to help us. Thus if
we hurry to have proper fix for 3.18, our fix might easily be really
wrong.
Hence, it would seem safer to revert to 3.17 state before 3.18, so we
can propose a proper fix for 3.19. At least from our myopic view, having
no functioning net on arndale is worse than having non-functioning
ethtool (which doesn't seem to have bothered people for years).
Riku
> > The PHY reset is the thing that breaks ethtool support, so any fix
> > that appeases all would have to take existing PHY state into
account.
> >
> > I'm not an expert on the ASIX driver, nor the MII, but I think this
> > is
>
> > the cause;
> > drivers/net/usb/asix_devices.c:
> >361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> >362 asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> >363 ADVERTISE_ALL | ADVERTISE_CSMA);
> >364 mii_nway_restart(&dev->mii);
> >
> > I would think that the ADVERTISE_ALL is the cause here, as it will
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> >
> > prior to the offending lines, and then;
> >
> >362 asix_mdio_write(dev->net, dev->mii.phy_id,
MII_ADVERTISE,
> >363 reg);
> >
> > solve the problem for you guys?
>
> If I revert the patch in question and add this in:
>
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
> struct asix_data *data = (struct asix_data *)&dev->data;
> int ret, embd_phy;
> + int reg;
> u16 rx_ctl;
>
> ret
RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks netonarndale platform
Hello Charles, First of all, my apologies. I manually applied your patch and made a mistake; I swapped ax88772_link_reset with ax88772_reset for struct driver_info_ax88772_info structure, which caused the software reset failures. Blame it on a lack of coffee... Please disregard my previous mail. After (correctly) applying the patch I still don't get the desired results; see below. Test situation: ifconfig eth1 down ethtool -s advertise 1 ifconfig eth1 up Check dmesg, It will report a link down, followed by the new speed, which when using advertise 1, should be 10 Mbps/half duplex. To make it more interesting, ethtool eth1 reports 10 Mbps/half duplex even though the kernel reports 100 Mbps/full duplex. What does work (but did before this whole situation as well): ifconfig eth1 up ethtool -s eth1 advertise 1 The interface will now be in 10 Mbps/half duplex, that is, until the reset function is called (interface down or otherwise). What I do notice, is that dev->mii.advertising follows whatever I set with ethtool. I tried writing the ADVERTISE register with the value of the dev->mii.advertising variable, but that did not give me the desired result either. Kind regards, Michel Stam -Original Message- From: Charles Keepax [mailto:ckee...@opensource.wolfsonmicro.com] Sent: Wednesday, November 05, 2014 16:03 PM To: Stam, Michel [FINT] Cc: Riku Voipio; da...@davemloft.net; linux-...@vger.kernel.org; net...@vger.kernel.org; linux-ker...@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks netonarndale platform On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote: > Hello Charles, > > After looking around I found the reset value for the 8772 chip, which > seems to be 0x1E1 (ANAR register). > > This equates to (according to include/uapi/linux/mii.h) ADVERTISE_ALL > | ADVERTISE_CSMA. > > The register only seems to become 0 if the software reset fails. Odd it definitely reads back as zero on Arndale. I am guessing that the root of the problem here is that for some reason Arndale POR of the ethernet is pants and it needs a full software reset before it will work and the patch removes the full reset callback. > > Unfortunately, this is exactly what I get when the patch is applied; > asix 1-2:1.0 eth1: Failed to send software reset: ffb5 asix > 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2, ASIX > AX88772 USB 2.0 Ethernet asix 1-2:1.0 eth1: Failed to send software > reset: ffb5 asix 1-2:1.0 eth1: link reset failed (-75) usbnet > usb-:00:1d.0-2, ASIX AX88772 USB 2.0 Ethernet Ok so I am guessing you have a value in the register which is neither the reset value or 0 and this causing problems later in the reset/on the next reset. I do find the naming confusing in the error message there as it says link reset failed but the link_reset callback can't fail in the driver and I modified the reset callback. But I guess that is just oddities of the network stack I am not familiar with. The other thing that feels odd is (and again apologies as I know next to nothing about the networking stack) how come it is unexpected that the reset callback destroys the state of the device. Naively I would have expected that a reset callback would reset the device back to its default state. Here we seem to be trying to avoid that happening. > > A little while after this its 'Failed to enable software MII access'. > The ethernet device fails to see any link or accept any ethtool -s > command. > > My device has vid:devid 0b95:772a (ASIX Elec. Corp.). > > Can you tell me what device is on the Andale platform, Charles? Same > vendor/device id? Yeah mine also idVendor=0b95, idProduct=772a Thanks, Charles > > Another thing that bothers me is that dev->mii.advertising seems to > contain the same value, so maybe that can be used instead of a call to > asix_mdio_read(). Can anyone comment on its purpose? Should it be a > shadow copy of the real register or something? > > Riku, can you test Charles' patch as well? > > Kind regards, > > Michel Stam > > -Original Message- > From: Charles Keepax [mailto:ckee...@opensource.wolfsonmicro.com] > Sent: Tuesday, November 04, 2014 21:09 PM > To: Stam, Michel [FINT] > Cc: Riku Voipio; da...@davemloft.net; linux-...@vger.kernel.org; > net...@vger.kernel.org; linux-ker...@vger.kernel.org; > linux-samsung-soc@vger.kernel.org > Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks > net onarndale platform > > On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote: > > Hello Riku, > > > > >Fixing a bug (ethtool support) must not cause breakage elsewhere > > >(in > > this case on arndale). This is now a regression of functionality > > from 3.17. > > > > > >I think it would better to revert the change now and with less > > >hurry > > introduce a ethtool fix that doesn't break arndale. > > > > I don't fully agree here; > >
Re: [PATCH v2 3/3] ARM: dts: exynos: Add sysreg phandle to ADC node
On 16/09/14 09:58, Naveen Krishna Chatradhi wrote:
> Instead of using the ADC_PHY register base address, use sysreg phandle
> in ADC node to control ADC_PHY configuration register.
>
> This patch adds syscon node for Exynos3250, Exynos4x12, Exynos5250,
> and Exynos5420, Exynos5800.
>
> Signed-off-by: Naveen Krishna Chatradhi
> To: linux-samsung-soc@vger.kernel.org
Applied to the togreg branch of iio.git - initially pushed out as testing.
As this is very closely tied with the driver changes I've applied it through
the IIO tree. If it turns up in the samsung tree as well, we can let git
work it's magic to drop one of the copies!
Jonathan
> ---
> arch/arm/boot/dts/exynos3250.dtsi |3 ++-
> arch/arm/boot/dts/exynos4x12.dtsi |3 ++-
> arch/arm/boot/dts/exynos5250.dtsi |3 ++-
> arch/arm/boot/dts/exynos5420.dtsi |3 ++-
> 4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos3250.dtsi
> b/arch/arm/boot/dts/exynos3250.dtsi
> index 1d52de6..b997a4c 100644
> --- a/arch/arm/boot/dts/exynos3250.dtsi
> +++ b/arch/arm/boot/dts/exynos3250.dtsi
> @@ -272,12 +272,13 @@
> adc: adc@126C {
> compatible = "samsung,exynos3250-adc",
>"samsung,exynos-adc-v2";
> - reg = <0x126C 0x100>, <0x10020718 0x4>;
> + reg = <0x126C 0x100>;
> interrupts = <0 137 0>;
> clock-names = "adc", "sclk";
> clocks = <&cmu CLK_TSADC>, <&cmu CLK_SCLK_TSADC>;
> #io-channel-cells = <1>;
> io-channel-ranges;
> + samsung,syscon-phandle = <&pmu_system_controller>;
> status = "disabled";
> };
>
> diff --git a/arch/arm/boot/dts/exynos4x12.dtsi
> b/arch/arm/boot/dts/exynos4x12.dtsi
> index 861bb91..9ee77d3 100644
> --- a/arch/arm/boot/dts/exynos4x12.dtsi
> +++ b/arch/arm/boot/dts/exynos4x12.dtsi
> @@ -108,13 +108,14 @@
>
> adc: adc@126C {
> compatible = "samsung,exynos-adc-v1";
> - reg = <0x126C 0x100>, <0x10020718 0x4>;
> + reg = <0x126C 0x100>;
> interrupt-parent = <&combiner>;
> interrupts = <10 3>;
> clocks = <&clock CLK_TSADC>;
> clock-names = "adc";
> #io-channel-cells = <1>;
> io-channel-ranges;
> + samsung,syscon-phandle = <&pmu_system_controller>;
> status = "disabled";
> };
>
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
> b/arch/arm/boot/dts/exynos5250.dtsi
> index 492e1ef..108adc5 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -765,12 +765,13 @@
>
> adc: adc@12D1 {
> compatible = "samsung,exynos-adc-v1";
> - reg = <0x12D1 0x100>, <0x10040718 0x4>;
> + reg = <0x12D1 0x100>;
> interrupts = <0 106 0>;
> clocks = <&clock CLK_ADC>;
> clock-names = "adc";
> #io-channel-cells = <1>;
> io-channel-ranges;
> + samsung,syscon-phandle = <&pmu_system_controller>;
> status = "disabled";
> };
>
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi
> b/arch/arm/boot/dts/exynos5420.dtsi
> index bfe056d..5fd587a 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -541,12 +541,13 @@
>
> adc: adc@12D1 {
> compatible = "samsung,exynos-adc-v2";
> - reg = <0x12D1 0x100>, <0x10040720 0x4>;
> + reg = <0x12D1 0x100>;
> interrupts = <0 106 0>;
> clocks = <&clock CLK_TSADC>;
> clock-names = "adc";
> #io-channel-cells = <1>;
> io-channel-ranges;
> + samsung,syscon-phandle = <&pmu_system_controller>;
> status = "disabled";
> };
>
>
--
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 2/3] Documentation: dt-bindings: update exynos-adc.txt with syscon handle
On 16/09/14 09:58, Naveen Krishna Chatradhi wrote:
> This patch updates the DT bindings for ADC in exynos-adc.txt with the
> syscon phandle to the ADC nodes.
>
> Signed-off-by: Naveen Krishna Chatradhi
> To: devicet...@vger.kernel.org
Applied to the togreg branch of iio.git - initially pushed out as testing.
Thanks,
> ---
> .../devicetree/bindings/arm/samsung/exynos-adc.txt |9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> index 709efaa..c368210 100644
> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> @@ -43,13 +43,16 @@ Required properties:
> compatible ADC block)
> - vdd-supply VDD input supply.
>
> +- samsung,syscon-phandle Contains the PMU system controller node
> + (To access the ADC_PHY register on
> Exynos5250/5420/5800/3250)
> +
> Note: child nodes can be added for auto probing from device tree.
>
> Example: adding device info in dtsi file
>
> adc: adc@12D1 {
> compatible = "samsung,exynos-adc-v1";
> - reg = <0x12D1 0x100>, <0x10040718 0x4>;
> + reg = <0x12D1 0x100>;
> interrupts = <0 106 0>;
> #io-channel-cells = <1>;
> io-channel-ranges;
> @@ -58,13 +61,14 @@ adc: adc@12D1 {
> clock-names = "adc";
>
> vdd-supply = <&buck5_reg>;
> + samsung,syscon-phandle = <&pmu_system_controller>;
> };
>
> Example: adding device info in dtsi file for Exynos3250 with additional sclk
>
> adc: adc@126C {
> compatible = "samsung,exynos3250-adc", "samsung,exynos-adc-v2;
> - reg = <0x126C 0x100>, <0x10020718 0x4>;
> + reg = <0x126C 0x100>;
> interrupts = <0 137 0>;
> #io-channel-cells = <1>;
> io-channel-ranges;
> @@ -73,6 +77,7 @@ adc: adc@126C {
> clock-names = "adc", "sclk";
>
> vdd-supply = <&buck5_reg>;
> + samsung,syscon-phandle = <&pmu_system_controller>;
> };
>
> Example: Adding child nodes in dts file
>
--
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 1/3] iio: exyno-adc: use syscon for PMU register access
On 16/09/14 09:58, Naveen Krishna Chatradhi wrote:
> This patch updates the IIO based ADC driver to use syscon and regmap
> APIs to access and use PMU registers instead of remapping the PMU
> registers in the driver.
>
> Signed-off-by: Naveen Krishna Chatradhi
> To: linux-...@vger.kernel.org
Applied to the togreg branch of iio.git - initially pushed out as testing
for the autobuilders to play.
> ---
> Changes since v1:
> Rebased on top of togreg branch of IIO git
>
> drivers/iio/adc/exynos_adc.c | 30 +-
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index 43620fd..fe03177 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -39,6 +39,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> /* S3C/EXYNOS4412/5250 ADC_V1 registers definitions */
> #define ADC_V1_CON(x)((x) + 0x00)
> @@ -90,11 +92,14 @@
>
> #define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
>
> +#define EXYNOS_ADCV1_PHY_OFFSET 0x0718
> +#define EXYNOS_ADCV2_PHY_OFFSET 0x0720
> +
> struct exynos_adc {
> struct exynos_adc_data *data;
> struct device *dev;
> void __iomem*regs;
> - void __iomem*enable_reg;
> + struct regmap *pmu_map;
> struct clk *clk;
> struct clk *sclk;
> unsigned intirq;
> @@ -110,6 +115,7 @@ struct exynos_adc_data {
> int num_channels;
> bool needs_sclk;
> bool needs_adc_phy;
> + int phy_offset;
> u32 mask;
>
> void (*init_hw)(struct exynos_adc *info);
> @@ -183,7 +189,7 @@ static void exynos_adc_v1_init_hw(struct exynos_adc *info)
> u32 con1;
>
> if (info->data->needs_adc_phy)
> - writel(1, info->enable_reg);
> + regmap_write(info->pmu_map, info->data->phy_offset, 1);
>
> /* set default prescaler values and Enable prescaler */
> con1 = ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
> @@ -198,7 +204,7 @@ static void exynos_adc_v1_exit_hw(struct exynos_adc *info)
> u32 con;
>
> if (info->data->needs_adc_phy)
> - writel(0, info->enable_reg);
> + regmap_write(info->pmu_map, info->data->phy_offset, 0);
>
> con = readl(ADC_V1_CON(info->regs));
> con |= ADC_V1_CON_STANDBY;
> @@ -225,6 +231,7 @@ static const struct exynos_adc_data exynos_adc_v1_data = {
> .num_channels = MAX_ADC_V1_CHANNELS,
> .mask = ADC_DATX_MASK,/* 12 bit ADC resolution */
> .needs_adc_phy = true,
> + .phy_offset = EXYNOS_ADCV1_PHY_OFFSET,
>
> .init_hw= exynos_adc_v1_init_hw,
> .exit_hw= exynos_adc_v1_exit_hw,
> @@ -314,7 +321,7 @@ static void exynos_adc_v2_init_hw(struct exynos_adc *info)
> u32 con1, con2;
>
> if (info->data->needs_adc_phy)
> - writel(1, info->enable_reg);
> + regmap_write(info->pmu_map, info->data->phy_offset, 1);
>
> con1 = ADC_V2_CON1_SOFT_RESET;
> writel(con1, ADC_V2_CON1(info->regs));
> @@ -332,7 +339,7 @@ static void exynos_adc_v2_exit_hw(struct exynos_adc *info)
> u32 con;
>
> if (info->data->needs_adc_phy)
> - writel(0, info->enable_reg);
> + regmap_write(info->pmu_map, info->data->phy_offset, 0);
>
> con = readl(ADC_V2_CON1(info->regs));
> con &= ~ADC_CON_EN_START;
> @@ -362,6 +369,7 @@ static const struct exynos_adc_data exynos_adc_v2_data = {
> .num_channels = MAX_ADC_V2_CHANNELS,
> .mask = ADC_DATX_MASK, /* 12 bit ADC resolution */
> .needs_adc_phy = true,
> + .phy_offset = EXYNOS_ADCV2_PHY_OFFSET,
>
> .init_hw= exynos_adc_v2_init_hw,
> .exit_hw= exynos_adc_v2_exit_hw,
> @@ -374,6 +382,7 @@ static const struct exynos_adc_data exynos3250_adc_data =
> {
> .mask = ADC_DATX_MASK, /* 12 bit ADC resolution */
> .needs_sclk = true,
> .needs_adc_phy = true,
> + .phy_offset = EXYNOS_ADCV1_PHY_OFFSET,
>
> .init_hw= exynos_adc_v2_init_hw,
> .exit_hw= exynos_adc_v2_exit_hw,
> @@ -558,10 +567,13 @@ static int exynos_adc_probe(struct platform_device
> *pdev)
>
>
> if (info->data->needs_adc_phy) {
> - mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - info->enable_reg = devm_ioremap_resource(&pdev->dev, mem);
> - if (IS_ERR(info->enable_reg))
> - return PTR_ERR(info->enable_reg);
> + info->pmu_map = syscon_regmap_lookup_by_phandle(
> + pdev->dev.of_node,
> + "samsung,syscon-phandle");
> + if (IS_ERR(info->pmu_map)) {
> + dev_err(&pdev->dev, "syscon regmap lookup failed.\n");
> +
Re: [PATCH v2 1/3] iio: exyno-adc: use syscon for PMU register access
On 28/10/14 12:31, Vivek Gautam wrote:
> Hi all,
>
>
> CC'ing Naveen's gmail id, since the Samsung id is invalid now.
>
>
> On Tue, Sep 16, 2014 at 2:28 PM, Naveen Krishna Chatradhi
> wrote:
>> This patch updates the IIO based ADC driver to use syscon and regmap
>> APIs to access and use PMU registers instead of remapping the PMU
>> registers in the driver.
>>
>> Signed-off-by: Naveen Krishna Chatradhi
>> To: linux-...@vger.kernel.org
>> ---
>> Changes since v1:
>> Rebased on top of togreg branch of IIO git
>
> If the changes look good, then first two patches of this series can be
> picked up ?
> The last dt patch then belongs to samsung tree.
Given the interlinked nature of the patches and Kukjin Kim's ack I'll
take the lot. Obviously if someone wants to pull the 3rd patch into
Samsung's tree as well, then git will deal with it just fine at merge
time!
Thanks,
Jonathan
>
>>
>> drivers/iio/adc/exynos_adc.c | 30 +-
>> 1 file changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
>> index 43620fd..fe03177 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -39,6 +39,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>>
>> /* S3C/EXYNOS4412/5250 ADC_V1 registers definitions */
>> #define ADC_V1_CON(x) ((x) + 0x00)
>> @@ -90,11 +92,14 @@
>>
>> #define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
>>
>> +#define EXYNOS_ADCV1_PHY_OFFSET0x0718
>> +#define EXYNOS_ADCV2_PHY_OFFSET0x0720
>> +
>> struct exynos_adc {
>> struct exynos_adc_data *data;
>> struct device *dev;
>> void __iomem*regs;
>> - void __iomem*enable_reg;
>> + struct regmap *pmu_map;
>> struct clk *clk;
>> struct clk *sclk;
>> unsigned intirq;
>> @@ -110,6 +115,7 @@ struct exynos_adc_data {
>> int num_channels;
>> bool needs_sclk;
>> bool needs_adc_phy;
>> + int phy_offset;
>> u32 mask;
>>
>> void (*init_hw)(struct exynos_adc *info);
>> @@ -183,7 +189,7 @@ static void exynos_adc_v1_init_hw(struct exynos_adc
>> *info)
>> u32 con1;
>>
>> if (info->data->needs_adc_phy)
>> - writel(1, info->enable_reg);
>> + regmap_write(info->pmu_map, info->data->phy_offset, 1);
>>
>> /* set default prescaler values and Enable prescaler */
>> con1 = ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
>> @@ -198,7 +204,7 @@ static void exynos_adc_v1_exit_hw(struct exynos_adc
>> *info)
>> u32 con;
>>
>> if (info->data->needs_adc_phy)
>> - writel(0, info->enable_reg);
>> + regmap_write(info->pmu_map, info->data->phy_offset, 0);
>>
>> con = readl(ADC_V1_CON(info->regs));
>> con |= ADC_V1_CON_STANDBY;
>> @@ -225,6 +231,7 @@ static const struct exynos_adc_data exynos_adc_v1_data =
>> {
>> .num_channels = MAX_ADC_V1_CHANNELS,
>> .mask = ADC_DATX_MASK,/* 12 bit ADC resolution */
>> .needs_adc_phy = true,
>> + .phy_offset = EXYNOS_ADCV1_PHY_OFFSET,
>>
>> .init_hw= exynos_adc_v1_init_hw,
>> .exit_hw= exynos_adc_v1_exit_hw,
>> @@ -314,7 +321,7 @@ static void exynos_adc_v2_init_hw(struct exynos_adc
>> *info)
>> u32 con1, con2;
>>
>> if (info->data->needs_adc_phy)
>> - writel(1, info->enable_reg);
>> + regmap_write(info->pmu_map, info->data->phy_offset, 1);
>>
>> con1 = ADC_V2_CON1_SOFT_RESET;
>> writel(con1, ADC_V2_CON1(info->regs));
>> @@ -332,7 +339,7 @@ static void exynos_adc_v2_exit_hw(struct exynos_adc
>> *info)
>> u32 con;
>>
>> if (info->data->needs_adc_phy)
>> - writel(0, info->enable_reg);
>> + regmap_write(info->pmu_map, info->data->phy_offset, 0);
>>
>> con = readl(ADC_V2_CON1(info->regs));
>> con &= ~ADC_CON_EN_START;
>> @@ -362,6 +369,7 @@ static const struct exynos_adc_data exynos_adc_v2_data =
>> {
>> .num_channels = MAX_ADC_V2_CHANNELS,
>> .mask = ADC_DATX_MASK, /* 12 bit ADC resolution */
>> .needs_adc_phy = true,
>> + .phy_offset = EXYNOS_ADCV2_PHY_OFFSET,
>>
>> .init_hw= exynos_adc_v2_init_hw,
>> .exit_hw= exynos_adc_v2_exit_hw,
>> @@ -374,6 +382,7 @@ static const struct exynos_adc_data exynos3250_adc_data
>> = {
>> .mask = ADC_DATX_MASK, /* 12 bit ADC resolution */
>> .needs_sclk = true,
>> .needs_adc_phy = true,
>> + .phy_offset = EXYNOS_ADCV1_PHY_OFFSET,
>>
>> .init_hw= exynos_adc_v2_init_hw,
>> .exit_hw= exynos_adc_v2_exit_hw,
>> @@ -558,10 +567,13 @@ static int exynos_adc_probe(struct platform_device
Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> Hello Charles,
>
> After looking around I found the reset value for the 8772 chip, which
> seems to be 0x1E1 (ANAR register).
>
> This equates to (according to include/uapi/linux/mii.h)
> ADVERTISE_ALL | ADVERTISE_CSMA.
>
> The register only seems to become 0 if the software reset fails.
Odd it definitely reads back as zero on Arndale. I am guessing
that the root of the problem here is that for some reason Arndale
POR of the ethernet is pants and it needs a full software reset
before it will work and the patch removes the full reset
callback.
>
> Unfortunately, this is exactly what I get when the patch is applied;
> asix 1-2:1.0 eth1: Failed to send software reset: ffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2,
> ASIX AX88772 USB 2.0 Ethernet
> asix 1-2:1.0 eth1: Failed to send software reset: ffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2,
> ASIX AX88772 USB 2.0 Ethernet
Ok so I am guessing you have a value in the register which is
neither the reset value or 0 and this causing problems later in
the reset/on the next reset. I do find the naming confusing in
the error message there as it says link reset failed but the
link_reset callback can't fail in the driver and I modified the
reset callback. But I guess that is just oddities of the network
stack I am not familiar with.
The other thing that feels odd is (and again apologies as I know
next to nothing about the networking stack) how come it is
unexpected that the reset callback destroys the state of the
device. Naively I would have expected that a reset callback would
reset the device back to its default state. Here we seem to be
trying to avoid that happening.
>
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s
> command.
>
> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
>
> Can you tell me what device is on the Andale platform, Charles? Same
> vendor/device id?
Yeah mine also idVendor=0b95, idProduct=772a
Thanks,
Charles
>
> Another thing that bothers me is that dev->mii.advertising seems to
> contain the same value, so maybe that can be used instead of a call to
> asix_mdio_read(). Can anyone comment on its purpose? Should it be a
> shadow copy of the real register or something?
>
> Riku, can you test Charles' patch as well?
>
> Kind regards,
>
> Michel Stam
>
> -Original Message-
> From: Charles Keepax [mailto:ckee...@opensource.wolfsonmicro.com]
> Sent: Tuesday, November 04, 2014 21:09 PM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; da...@davemloft.net; linux-...@vger.kernel.org;
> net...@vger.kernel.org; linux-ker...@vger.kernel.org;
> linux-samsung-soc@vger.kernel.org
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
> onarndale platform
>
> On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> > Hello Riku,
> >
> > >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> > this case on arndale). This is now a regression of functionality from
> > 3.17.
> > >
> > >I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.
> >
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself. Fixing
> > the armdale will then cause breakage in other implementations, such as
>
> > ours. Blankly reverting breaks other peoples' implementations.
> >
> > The PHY reset is the thing that breaks ethtool support, so any fix
> > that appeases all would have to take existing PHY state into account.
> >
> > I'm not an expert on the ASIX driver, nor the MII, but I think this is
>
> > the cause;
> > drivers/net/usb/asix_devices.c:
> >361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> >362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> >363 ADVERTISE_ALL | ADVERTISE_CSMA);
> >364 mii_nway_restart(&dev->mii);
> >
> > I would think that the ADVERTISE_ALL is the cause here, as it will
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> >
> > prior to the offending lines, and then;
> >
> >362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> >363 reg);
> >
> > solve the problem for you guys?
>
> If I revert the patch in question and add this in:
>
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
> struct asix_data *data = (struct asix_data *)&dev->data;
> int ret, embd_phy;
> + int reg;
> u16 rx_ctl;
>
> ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(str
Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver
On 04/11/14 04:18, Pankaj Dubey wrote: > 2: Since PM domain relies on PMU registers and does not have > its own DT binding, MFD client and MFD device > is most suitable for making this kind of platform drivers. We have DT binding for the Exynos power domains: Documentation/devicetree/bindings/arm/exynos/power_domain.txt In fact the IO memory regions used in the power domain bindings fall within the (sparse) PMU registers region. There is also DT binding for the PMU subsystem: Documentation/devicetree/bindings/arm/samsung/pmu.txt I guess a platform device driver in drivers/power would be a better fit than drivers/mfd. I doubt introducing an mfd driver now makes much sense, we have already DT bindings for PHYs, the power domains, etc. -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] arm64: dts: exynos7: add support for cpuidle core power down
Exynos7 has core power down state where cores can be powered off independently.
This patch adds support for this state.
Entry latency for the core power down is calculated as follows:
1. Time difference is measured between cpuidle entry and exit.
2. WFI is skipped for measuring the time.
3. Select the worst case time in the set of 10 cpuidle transactions,
with varying load.
Exit latency and target residency are supplied as per HW team
Signed-off-by: Chander Kashyap
---
This patch has following dependencies:
- [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
http://www.spinics.net/lists/linux-samsung-soc/msg37047.html
Changes in v2:
- Moved the cpu-idle-state property after reg property
- removed the status property.
Changes in v3:
- Added the Entry latency calculation in commit message.
Changes in v4:
- Corrected the commit message.
- Corrected the entry latency value.
arch/arm64/boot/dts/exynos/exynos7.dtsi | 17 +
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi
b/arch/arm64/boot/dts/exynos/exynos7.dtsi
index 50ae936..444dde1 100644
--- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
@@ -37,6 +37,7 @@
compatible = "arm,cortex-a57", "arm,armv8";
enable-method = "psci";
reg = <0x0>;
+ cpu-idle-states = <&CPU_SLEEP>;
};
cpu@1 {
@@ -44,6 +45,7 @@
compatible = "arm,cortex-a57", "arm,armv8";
enable-method = "psci";
reg = <0x1>;
+ cpu-idle-states = <&CPU_SLEEP>;
};
cpu@2 {
@@ -51,6 +53,7 @@
compatible = "arm,cortex-a57", "arm,armv8";
enable-method = "psci";
reg = <0x2>;
+ cpu-idle-states = <&CPU_SLEEP>;
};
cpu@3 {
@@ -58,6 +61,20 @@
compatible = "arm,cortex-a57", "arm,armv8";
enable-method = "psci";
reg = <0x3>;
+ cpu-idle-states = <&CPU_SLEEP>;
+ };
+
+ idle-states {
+ entry-method = "arm,psci";
+
+ CPU_SLEEP: cpu-sleep {
+ compatible = "arm,idle-state";
+ local-timer-stop;
+ arm,psci-suspend-param = <0x001>;
+ entry-latency-us = <34>;
+ exit-latency-us = <150>;
+ min-residency-us = <2100>;
+ };
};
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] arm64: dts: exynos7: add support for cpuidle core power down
On Wed, Nov 5, 2014 at 4:42 PM, Lorenzo Pieralisi
wrote:
> On Wed, Nov 05, 2014 at 10:15:36AM +, Chander Kashyap wrote:
>> Exynos7 has core power down state where cores can be powered off
>> independently.
>> This patch adds support for this state.
>>
>> Entry latency for the core power down is calculated as follows:
>> 1. Time difference is measured between cpuidle entry and exit.
>> 2. WFI is skipped measuring the time.
>> 3. The time is averaged out for 10 cpuidle transactions with varying
>> load.
> - entry-latency-us
> Usage: Required
> "Definition: u32 value representing worst case latency in microseconds
> required to enter the idle state. ..."
>
> Is that an average value in your opinion ? I am being pedantic, I know,
> but we define bindings to be followed, not interpreted.
Sorry i missed the point.
The worst case value in a test set of 10 cpuidle transactions is ~34us
I will update this and resend the patch.
>
> Thanks,
> Lorenzo
>
>> Exit latency and target residency are supplied as per HW team
>>
>> Signed-off-by: Chander Kashyap
>> ---
>> This patch has following dependencies:
>> - [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
>> http://www.spinics.net/lists/linux-samsung-soc/msg37047.html
>> Changes in v2:
>> - Moved the cpu-idle-state property after reg property
>> - removed the status property.
>>
>> Changes in v3:
>> - Added the Entry latency calculation in commit message.
>>
>> arch/arm64/boot/dts/exynos/exynos7.dtsi | 17 +
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> index 50ae936..444dde1 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> @@ -37,6 +37,7 @@
>> compatible = "arm,cortex-a57", "arm,armv8";
>> enable-method = "psci";
>> reg = <0x0>;
>> + cpu-idle-states = <&CPU_SLEEP>;
>> };
>>
>> cpu@1 {
>> @@ -44,6 +45,7 @@
>> compatible = "arm,cortex-a57", "arm,armv8";
>> enable-method = "psci";
>> reg = <0x1>;
>> + cpu-idle-states = <&CPU_SLEEP>;
>> };
>>
>> cpu@2 {
>> @@ -51,6 +53,7 @@
>> compatible = "arm,cortex-a57", "arm,armv8";
>> enable-method = "psci";
>> reg = <0x2>;
>> + cpu-idle-states = <&CPU_SLEEP>;
>> };
>>
>> cpu@3 {
>> @@ -58,6 +61,20 @@
>> compatible = "arm,cortex-a57", "arm,armv8";
>> enable-method = "psci";
>> reg = <0x3>;
>> + cpu-idle-states = <&CPU_SLEEP>;
>> + };
>> +
>> + idle-states {
>> + entry-method = "arm,psci";
>> +
>> + CPU_SLEEP: cpu-sleep {
>> + compatible = "arm,idle-state";
>> + local-timer-stop;
>> + arm,psci-suspend-param = <0x001>;
>> + entry-latency-us = <20>;
>> + exit-latency-us = <150>;
>> + min-residency-us = <2100>;
>> + };
>> };
>> };
>>
>> --
>> 1.7.9.5
>>
>>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform
Hi Michel,
On Wed, Nov 05, 2014 at 01:04:37PM +0100, Stam, Michel [FINT] wrote:
> After looking around I found the reset value for the 8772 chip, which
> seems to be 0x1E1 (ANAR register).
>
> This equates to (according to include/uapi/linux/mii.h)
> ADVERTISE_ALL | ADVERTISE_CSMA.
>
> The register only seems to become 0 if the software reset fails.
>
> Unfortunately, this is exactly what I get when the patch is applied;
> asix 1-2:1.0 eth1: Failed to send software reset: ffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2,
> ASIX AX88772 USB 2.0 Ethernet
> asix 1-2:1.0 eth1: Failed to send software reset: ffb5
> asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2,
> ASIX AX88772 USB 2.0 Ethernet
>
> A little while after this its 'Failed to enable software MII access'.
> The ethernet device fails to see any link or accept any ethtool -s
> command.
> My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
> Can you tell me what device is on the Andale platform, Charles? Same
> vendor/device id?
Bus 003 Device 004: ID 0b95:772a ASIX Electronics Corp. AX88772A Fast Ethernet
> Another thing that bothers me is that dev->mii.advertising seems to
> contain the same value, so maybe that can be used instead of a call to
> asix_mdio_read(). Can anyone comment on its purpose? Should it be a
> shadow copy of the real register or something?
> Riku, can you test Charles' patch as well?
With that patch + revert to ax88772_reset network works. I'm unable to get
ethtool to work with that patch or with the original 3.17 state of asix.
Once i disable autoneg network doesn't just work.
> > >I think it would better to revert the change now and with less hurry
> > introduce a ethtool fix that doesn't break arndale.
> > I don't fully agree here;
> > I would like to point out that this commit is a revert itself. Fixing
> > the armdale will then cause breakage in other implementations, such as
> > ours. Blankly reverting breaks other peoples' implementations.
My concern is, that none of us with this problem is a linux network
drivers expert. And no such expert joined the thread to help us. Thus if
we hurry to have proper fix for 3.18, our fix might easily be really
wrong.
Hence, it would seem safer to revert to 3.17 state before 3.18, so we
can propose a proper fix for 3.19. At least from our myopic view, having
no functioning net on arndale is worse than having non-functioning
ethtool (which doesn't seem to have bothered people for years).
Riku
> > The PHY reset is the thing that breaks ethtool support, so any fix
> > that appeases all would have to take existing PHY state into account.
> >
> > I'm not an expert on the ASIX driver, nor the MII, but I think this is
>
> > the cause;
> > drivers/net/usb/asix_devices.c:
> >361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> > BMCR_RESET);
> >362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> >363 ADVERTISE_ALL | ADVERTISE_CSMA);
> >364 mii_nway_restart(&dev->mii);
> >
> > I would think that the ADVERTISE_ALL is the cause here, as it will
> > reset the MII back to default, thus overriding ethtool settings.
> > Would an:
> > Int reg;
> > reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> >
> > prior to the offending lines, and then;
> >
> >362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> >363 reg);
> >
> > solve the problem for you guys?
>
> If I revert the patch in question and add this in:
>
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
> struct asix_data *data = (struct asix_data *)&dev->data;
> int ret, embd_phy;
> + int reg;
> u16 rx_ctl;
>
> ret = asix_write_gpio(dev,
> @@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
> msleep(150);
>
> asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
> - asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
> - ADVERTISE_ALL | ADVERTISE_CSMA);
> + reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
> + if (!reg)
> + reg = ADVERTISE_ALL | ADVERTISE_CSMA;
> + asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
> mii_nway_restart(&dev->mii);
>
> ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
>
> Then things work on Arndale for me. Does that work for you?
> Whether that is a sensible fix I don't know however.
>
> >
> > Mind, maybe the read function should take into account the reset value
>
> > of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have
>
> > any documention here at the moment.
>
> Yeah I also have no documentation.
>
> Thanks,
> Charles
>
> >
> > Is anyone able to confirm my suspicions?
> >
> > Kind regards,
> >
[PATCH] thermal: cpu_cooling: Update always cpufreq policy with thermal constraints
Existing code updates cupfreq policy only while executing
cpufreq_apply_cooling() function (i.e. when notify_device != NOTIFY_INVALID).
It doesn't apply constraints when cpufreq policy update happens from any other
place but it should update the cpufreq policy with thermal constraints every
time when there is a cpufreq policy update, to keep state of
cpufreq_cooling_device and max_feq of cpufreq policy in sync.
This patch modifies code to maintain a global cpufreq_dev_list and get
corresponding cpufreq_cooling_device for policy's cpu from cpufreq_dev_list
when there is any policy update.
Signed-off-by: Yadwinder Singh Brar
---
drivers/thermal/cpu_cooling.c | 37 -
1 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 1ab0018..5546278 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -50,15 +50,14 @@ struct cpufreq_cooling_device {
unsigned int cpufreq_state;
unsigned int cpufreq_val;
struct cpumask allowed_cpus;
+ struct list_head node;
};
static DEFINE_IDR(cpufreq_idr);
static DEFINE_MUTEX(cooling_cpufreq_lock);
static unsigned int cpufreq_dev_count;
-/* notify_table passes value to the CPUFREQ_ADJUST callback function. */
-#define NOTIFY_INVALID NULL
-static struct cpufreq_cooling_device *notify_device;
+static LIST_HEAD(cpufreq_dev_list);
/**
* get_idr - function to get a unique id.
@@ -287,15 +286,12 @@ static int cpufreq_apply_cooling(struct
cpufreq_cooling_device *cpufreq_device,
cpufreq_device->cpufreq_state = cooling_state;
cpufreq_device->cpufreq_val = clip_freq;
- notify_device = cpufreq_device;
for_each_cpu(cpuid, mask) {
if (is_cpufreq_valid(cpuid))
cpufreq_update_policy(cpuid);
}
- notify_device = NOTIFY_INVALID;
-
return 0;
}
@@ -316,21 +312,27 @@ static int cpufreq_thermal_notifier(struct notifier_block
*nb,
{
struct cpufreq_policy *policy = data;
unsigned long max_freq = 0;
+ struct cpufreq_cooling_device *cpufreq_dev;
- if (event != CPUFREQ_ADJUST || notify_device == NOTIFY_INVALID)
+ if (event != CPUFREQ_ADJUST)
return 0;
- if (cpumask_test_cpu(policy->cpu, ¬ify_device->allowed_cpus))
- max_freq = notify_device->cpufreq_val;
- else
- return 0;
+ mutex_lock(&cooling_cpufreq_lock);
+ list_for_each_entry(cpufreq_dev, &cpufreq_dev_list, node) {
+ if (!cpumask_test_cpu(policy->cpu,
+ &cpufreq_dev->allowed_cpus))
+ continue;
- /* Never exceed user_policy.max */
- if (max_freq > policy->user_policy.max)
- max_freq = policy->user_policy.max;
+ max_freq = cpufreq_dev->cpufreq_val;
- if (policy->max != max_freq)
- cpufreq_verify_within_limits(policy, 0, max_freq);
+ /* Never exceed user_policy.max */
+ if (max_freq > policy->user_policy.max)
+ max_freq = policy->user_policy.max;
+
+ if (policy->max != max_freq)
+ cpufreq_verify_within_limits(policy, 0, max_freq);
+ }
+ mutex_unlock(&cooling_cpufreq_lock);
return 0;
}
@@ -486,7 +488,7 @@ __cpufreq_cooling_register(struct device_node *np,
cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
CPUFREQ_POLICY_NOTIFIER);
cpufreq_dev_count++;
-
+ list_add(&cpufreq_dev->node, &cpufreq_dev_list);
mutex_unlock(&cooling_cpufreq_lock);
return cool_dev;
@@ -549,6 +551,7 @@ void cpufreq_cooling_unregister(struct
thermal_cooling_device *cdev)
cpufreq_dev = cdev->devdata;
mutex_lock(&cooling_cpufreq_lock);
+ list_del(&cpufreq_dev->node);
cpufreq_dev_count--;
/* Unregister the notifier for the last cpufreq cooling device */
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net onarndale platform
Hello Charles,
After looking around I found the reset value for the 8772 chip, which
seems to be 0x1E1 (ANAR register).
This equates to (according to include/uapi/linux/mii.h)
ADVERTISE_ALL | ADVERTISE_CSMA.
The register only seems to become 0 if the software reset fails.
Unfortunately, this is exactly what I get when the patch is applied;
asix 1-2:1.0 eth1: Failed to send software reset: ffb5
asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2,
ASIX AX88772 USB 2.0 Ethernet
asix 1-2:1.0 eth1: Failed to send software reset: ffb5
asix 1-2:1.0 eth1: link reset failed (-75) usbnet usb-:00:1d.0-2,
ASIX AX88772 USB 2.0 Ethernet
A little while after this its 'Failed to enable software MII access'.
The ethernet device fails to see any link or accept any ethtool -s
command.
My device has vid:devid 0b95:772a (ASIX Elec. Corp.).
Can you tell me what device is on the Andale platform, Charles? Same
vendor/device id?
Another thing that bothers me is that dev->mii.advertising seems to
contain the same value, so maybe that can be used instead of a call to
asix_mdio_read(). Can anyone comment on its purpose? Should it be a
shadow copy of the real register or something?
Riku, can you test Charles' patch as well?
Kind regards,
Michel Stam
-Original Message-
From: Charles Keepax [mailto:ckee...@opensource.wolfsonmicro.com]
Sent: Tuesday, November 04, 2014 21:09 PM
To: Stam, Michel [FINT]
Cc: Riku Voipio; da...@davemloft.net; linux-...@vger.kernel.org;
net...@vger.kernel.org; linux-ker...@vger.kernel.org;
linux-samsung-soc@vger.kernel.org
Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
onarndale platform
On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> Hello Riku,
>
> >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> this case on arndale). This is now a regression of functionality from
> 3.17.
> >
> >I think it would better to revert the change now and with less hurry
> introduce a ethtool fix that doesn't break arndale.
>
> I don't fully agree here;
> I would like to point out that this commit is a revert itself. Fixing
> the armdale will then cause breakage in other implementations, such as
> ours. Blankly reverting breaks other peoples' implementations.
>
> The PHY reset is the thing that breaks ethtool support, so any fix
> that appeases all would have to take existing PHY state into account.
>
> I'm not an expert on the ASIX driver, nor the MII, but I think this is
> the cause;
> drivers/net/usb/asix_devices.c:
>361 asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
>362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
>363 ADVERTISE_ALL | ADVERTISE_CSMA);
>364 mii_nway_restart(&dev->mii);
>
> I would think that the ADVERTISE_ALL is the cause here, as it will
> reset the MII back to default, thus overriding ethtool settings.
> Would an:
> Int reg;
> reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
>
> prior to the offending lines, and then;
>
>362 asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
>363 reg);
>
> solve the problem for you guys?
If I revert the patch in question and add this in:
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev) {
struct asix_data *data = (struct asix_data *)&dev->data;
int ret, embd_phy;
+ int reg;
u16 rx_ctl;
ret = asix_write_gpio(dev,
@@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
msleep(150);
asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
BMCR_RESET);
- asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
- ADVERTISE_ALL | ADVERTISE_CSMA);
+ reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
+ if (!reg)
+ reg = ADVERTISE_ALL | ADVERTISE_CSMA;
+ asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
mii_nway_restart(&dev->mii);
ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);
Then things work on Arndale for me. Does that work for you?
Whether that is a sensible fix I don't know however.
>
> Mind, maybe the read function should take into account the reset value
> of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have
> any documention here at the moment.
Yeah I also have no documentation.
Thanks,
Charles
>
> Is anyone able to confirm my suspicions?
>
> Kind regards,
>
> Michel Stam
> -Original Message-
> From: Riku Voipio [mailto:riku.voi...@iki.fi]
> Sent: Tuesday, November 04, 2014 10:44 AM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; da...@davemloft.net; linux-...@vger.kernel.org;
> net...@vger.kernel.org; linux-ker...@vger.kernel.org;
> linux-samsung-soc@vger.kernel.org; ckee...@opensource.wolfsonmicro.com
> Subject: Re:
[PATCH] drm/exynos: add has_vtsel flag
The exynos fimd provides video type selection bits from system register
but exynos3 series don't has it, so needs has_vtsel flag and we can
distinguish whether set video type selection bits.
Signed-off-by: Joonyoung Shim
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 085b066..1b32771 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -96,6 +96,7 @@ struct fimd_driver_data {
unsigned int has_clksel:1;
unsigned int has_limited_fmt:1;
unsigned int has_vidoutcon:1;
+ unsigned int has_vtsel:1;
};
static struct fimd_driver_data s3c64xx_fimd_driver_data = {
@@ -118,6 +119,7 @@ static struct fimd_driver_data exynos4_fimd_driver_data = {
.lcdblk_vt_shift = 10,
.lcdblk_bypass_shift = 1,
.has_shadowcon = 1,
+ .has_vtsel = 1,
};
static struct fimd_driver_data exynos5_fimd_driver_data = {
@@ -127,6 +129,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
.lcdblk_bypass_shift = 15,
.has_shadowcon = 1,
.has_vidoutcon = 1,
+ .has_vtsel = 1,
};
struct fimd_win_data {
@@ -343,7 +346,8 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
writel(0, timing_base + I80IFCONFBx(0));
/* set video type selection to I80 interface */
- if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+ if (driver_data->has_vtsel && ctx->sysreg &&
+ regmap_update_bits(ctx->sysreg,
driver_data->lcdblk_offset,
0x3 << driver_data->lcdblk_vt_shift,
0x1 << driver_data->lcdblk_vt_shift)) {
--
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] drm/exynos: add has_vtsel flag
The exynos fimd provides video type selection bits from system register
but exynos3 series don't has it, so needs has_vtsel flag and we can
distinguish whether set video type selection bits.
Signed-off-by: Joonyoung Shim
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 085b066..1b32771 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -96,6 +96,7 @@ struct fimd_driver_data {
unsigned int has_clksel:1;
unsigned int has_limited_fmt:1;
unsigned int has_vidoutcon:1;
+ unsigned int has_vtsel:1;
};
static struct fimd_driver_data s3c64xx_fimd_driver_data = {
@@ -118,6 +119,7 @@ static struct fimd_driver_data exynos4_fimd_driver_data = {
.lcdblk_vt_shift = 10,
.lcdblk_bypass_shift = 1,
.has_shadowcon = 1,
+ .has_vtsel = 1,
};
static struct fimd_driver_data exynos5_fimd_driver_data = {
@@ -127,6 +129,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
.lcdblk_bypass_shift = 15,
.has_shadowcon = 1,
.has_vidoutcon = 1,
+ .has_vtsel = 1,
};
struct fimd_win_data {
@@ -343,7 +346,8 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
writel(0, timing_base + I80IFCONFBx(0));
/* set video type selection to I80 interface */
- if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+ if (driver_data->has_vtsel && ctx->sysreg &&
+ regmap_update_bits(ctx->sysreg,
driver_data->lcdblk_offset,
0x3 << driver_data->lcdblk_vt_shift,
0x1 << driver_data->lcdblk_vt_shift)) {
--
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] drm/exynos: add has_vtsel flag
The exynos fimd provides video type selection bits from system register
but exynos3 series don't has it, so needs has_vtsel flag and we can
distinguish whether set video type selection bits.
Signed-off-by: Joonyoung Shim
---
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 085b066..1b32771 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -96,6 +96,7 @@ struct fimd_driver_data {
unsigned int has_clksel:1;
unsigned int has_limited_fmt:1;
unsigned int has_vidoutcon:1;
+ unsigned int has_vtsel:1;
};
static struct fimd_driver_data s3c64xx_fimd_driver_data = {
@@ -118,6 +119,7 @@ static struct fimd_driver_data exynos4_fimd_driver_data = {
.lcdblk_vt_shift = 10,
.lcdblk_bypass_shift = 1,
.has_shadowcon = 1,
+ .has_vtsel = 1,
};
static struct fimd_driver_data exynos5_fimd_driver_data = {
@@ -127,6 +129,7 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
.lcdblk_bypass_shift = 15,
.has_shadowcon = 1,
.has_vidoutcon = 1,
+ .has_vtsel = 1,
};
struct fimd_win_data {
@@ -343,7 +346,8 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
writel(0, timing_base + I80IFCONFBx(0));
/* set video type selection to I80 interface */
- if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+ if (driver_data->has_vtsel && ctx->sysreg &&
+ regmap_update_bits(ctx->sysreg,
driver_data->lcdblk_offset,
0x3 << driver_data->lcdblk_vt_shift,
0x1 << driver_data->lcdblk_vt_shift)) {
--
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
Re: [PATCH v3] arm64: dts: exynos7: add support for cpuidle core power down
On Wed, Nov 05, 2014 at 10:15:36AM +, Chander Kashyap wrote:
> Exynos7 has core power down state where cores can be powered off
> independently.
> This patch adds support for this state.
>
> Entry latency for the core power down is calculated as follows:
> 1. Time difference is measured between cpuidle entry and exit.
> 2. WFI is skipped measuring the time.
> 3. The time is averaged out for 10 cpuidle transactions with varying load.
- entry-latency-us
Usage: Required
"Definition: u32 value representing worst case latency in microseconds
required to enter the idle state. ..."
Is that an average value in your opinion ? I am being pedantic, I know,
but we define bindings to be followed, not interpreted.
Thanks,
Lorenzo
> Exit latency and target residency are supplied as per HW team
>
> Signed-off-by: Chander Kashyap
> ---
> This patch has following dependencies:
> - [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
> http://www.spinics.net/lists/linux-samsung-soc/msg37047.html
> Changes in v2:
> - Moved the cpu-idle-state property after reg property
> - removed the status property.
>
> Changes in v3:
> - Added the Entry latency calculation in commit message.
>
> arch/arm64/boot/dts/exynos/exynos7.dtsi | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi
> b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> index 50ae936..444dde1 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
> @@ -37,6 +37,7 @@
> compatible = "arm,cortex-a57", "arm,armv8";
> enable-method = "psci";
> reg = <0x0>;
> + cpu-idle-states = <&CPU_SLEEP>;
> };
>
> cpu@1 {
> @@ -44,6 +45,7 @@
> compatible = "arm,cortex-a57", "arm,armv8";
> enable-method = "psci";
> reg = <0x1>;
> + cpu-idle-states = <&CPU_SLEEP>;
> };
>
> cpu@2 {
> @@ -51,6 +53,7 @@
> compatible = "arm,cortex-a57", "arm,armv8";
> enable-method = "psci";
> reg = <0x2>;
> + cpu-idle-states = <&CPU_SLEEP>;
> };
>
> cpu@3 {
> @@ -58,6 +61,20 @@
> compatible = "arm,cortex-a57", "arm,armv8";
> enable-method = "psci";
> reg = <0x3>;
> + cpu-idle-states = <&CPU_SLEEP>;
> + };
> +
> + idle-states {
> + entry-method = "arm,psci";
> +
> + CPU_SLEEP: cpu-sleep {
> + compatible = "arm,idle-state";
> + local-timer-stop;
> + arm,psci-suspend-param = <0x001>;
> + entry-latency-us = <20>;
> + exit-latency-us = <150>;
> + min-residency-us = <2100>;
> + };
> };
> };
>
> --
> 1.7.9.5
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] usb: Remove references to non-existent PLAT_S5P symbol
On 04/11/14 20:52, Paul Bolle wrote:
> On Tue, 2014-11-04 at 11:42 -0800, Greg KH wrote:
>> > As it's something that no one seemed to ever need before (i.e. it's not
>> > a regression fix), but it would be a "new feature", I don't think it's
>> > really a stable fix.
>> >
>> > But feel free to convince me otherwise :)
>
> Sylwester, was I right in thinking that users of PLAT_S5P, who could set
> USB_EHCI_EXYNOS or USB_OHCI_EXYNOS pre v3.17, got, well, transferred to
> ARCH_S5PV210 and lost the ability to set one of those symbols in v3.17?
Yes, after commit d78c16ccde96 ("ARM: SAMSUNG: Remove remaining legacy code")
we lost the ability to enable USB OHCI and EHCI on S5PV210 SoC.
Thus for those who use the mainline kernel (might be rare) with S5PV210 SoC
there is obviously a regression in USB subsystem in v3.17.
--
Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] arm64: dts: exynos7: add support for cpuidle core power down
Exynos7 has core power down state where cores can be powered off independently.
This patch adds support for this state.
Entry latency for the core power down is calculated as follows:
1. Time difference is measured between cpuidle entry and exit.
2. WFI is skipped measuring the time.
3. The time is averaged out for 10 cpuidle transactions with varying load.
Exit latency and target residency are supplied as per HW team
Signed-off-by: Chander Kashyap
---
This patch has following dependencies:
- [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
http://www.spinics.net/lists/linux-samsung-soc/msg37047.html
Changes in v2:
- Moved the cpu-idle-state property after reg property
- removed the status property.
Changes in v3:
- Added the Entry latency calculation in commit message.
arch/arm64/boot/dts/exynos/exynos7.dtsi | 17 +
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi
b/arch/arm64/boot/dts/exynos/exynos7.dtsi
index 50ae936..444dde1 100644
--- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
@@ -37,6 +37,7 @@
compatible = "arm,cortex-a57", "arm,armv8";
enable-method = "psci";
reg = <0x0>;
+ cpu-idle-states = <&CPU_SLEEP>;
};
cpu@1 {
@@ -44,6 +45,7 @@
compatible = "arm,cortex-a57", "arm,armv8";
enable-method = "psci";
reg = <0x1>;
+ cpu-idle-states = <&CPU_SLEEP>;
};
cpu@2 {
@@ -51,6 +53,7 @@
compatible = "arm,cortex-a57", "arm,armv8";
enable-method = "psci";
reg = <0x2>;
+ cpu-idle-states = <&CPU_SLEEP>;
};
cpu@3 {
@@ -58,6 +61,20 @@
compatible = "arm,cortex-a57", "arm,armv8";
enable-method = "psci";
reg = <0x3>;
+ cpu-idle-states = <&CPU_SLEEP>;
+ };
+
+ idle-states {
+ entry-method = "arm,psci";
+
+ CPU_SLEEP: cpu-sleep {
+ compatible = "arm,idle-state";
+ local-timer-stop;
+ arm,psci-suspend-param = <0x001>;
+ entry-latency-us = <20>;
+ exit-latency-us = <150>;
+ min-residency-us = <2100>;
+ };
};
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: exynos7: add support for cpuidle core power down
Sorry for very late response. As i was on vacation so couldn’t reply.
On Tue, Oct 21, 2014 at 10:03 PM, Lorenzo Pieralisi
wrote:
> On Fri, Oct 17, 2014 at 10:43:59AM +0100, Chander Kashyap wrote:
>> Hi Lorenzo,
>>
>> On Wed, Oct 15, 2014 at 2:30 PM, Lorenzo Pieralisi
>> wrote:
>> > On Wed, Oct 15, 2014 at 07:35:20AM +0100, Chander Kashyap wrote:
>> >> Exynos7 has core power down state where cores can be powered off
>> >> independently.
>> >> This patch adds support for this state.
>> >
>> > Please tell us more about the idle-state values you are adding, in
>> > particular
>> > entry, exit latencies and min-residency values.
>>
>> Entry latency: This value is calculated as follows:
>>
>> On entry to arm64_enter_idle_state:
>> timestamp1 = ktimeget();
>>
>> after returning from cpu_suspend()
>>
>> timestamp2 = ktimeget();
>>
>> latency = timestamp2-timestamp1;
>>
>> Cpu is not allowed to enter core powerdown by skipping wfi instruction at
>> end.
> This may not be the worst case (because the worst case depends on the state
> of the cache in the core unless the latency is power down command dominated,
> so at the cost of being pedantic, please make sure that's what you are
> measuring and document it in the commit log).
>
If i understood correctly you are referring to cache flush time.
The measured entry latency time is averaged time for 10
transactions with varying load.
I will document entry latency calculation in the commit message.
>> Hence calculated time contains entry time + failure exit time.
>>
>>
>> Regarding
>> exit-latency and target-residency time, got these values from HW team.
>>
>> I am using these as initial values and I will be working on optimizing
>> these values with further experiments.
>> If you could suggest any formal method of deriving these values, i can
>> try those methods as well.
>
> Well, you have to set the core/cluster in worst case scenario and
> compute the break-even residency against wfi (since you have two
> states); it certainly has a dependency on PSCI implementation too among
> other things.
>
> exit-latency should come from HW design even though there is a cache
> refill factor to be considered too and should be factored in.
Exit and target residency are provided by HW team.
I will post the V3 with changed commit message.
>
> Lorenzo
>
>>
>> >
>> >> Signed-off-by: Chander Kashyap
>> >> ---
>> >> This patch has following dependencies:
>> >> - [PATCH v5 0/8] arch: arm64: Enable support for Samsung Exynos7 SoC
>> >> http://www.spinics.net/lists/linux-samsung-soc/msg37047.html
>> >> - [PATCH v9 0/8] ARM generic idle states
>> >>
>> >> http://permalink.gmane.org/gmane.linux.power-management.general/49224
>> >
>> > Series above was merged, so dependency is stale.
>>
>> i will remove this
>>
>> >
>> >> arch/arm64/boot/dts/exynos/exynos7.dtsi | 18 ++
>> >> 1 file changed, 18 insertions(+)dont
>> >>
>> >> diff --git a/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> >> b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> >> index ce221ac..8e0a034 100644
>> >> --- a/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> >> +++ b/arch/arm64/boot/dts/exynos/exynos7.dtsi
>> >> @@ -36,6 +36,7 @@
>> >> device_type = "cpu";
>> >> compatible = "arm,cortex-a57", "arm,armv8";
>> >> enable-method = "psci";
>> >> + cpu-idle-states = <&CPU_SLEEP>;
>> >
>> > I would add cpu-idle-states phandle after the reg property, as defined
>> > in the idle states bindings.
>>
>> i will move this after reg property.
>>
>> >
>> >> reg = <0x0>;
>> >> };
>> >>
>> >> @@ -43,6 +44,7 @@
>> >> device_type = "cpu";
>> >> compatible = "arm,cortex-a57", "arm,armv8";
>> >> enable-method = "psci";
>> >> + cpu-idle-states = <&CPU_SLEEP>;
>> >> reg = <0x1>;
>> >> };
>> >>
>> >> @@ -50,6 +52,7 @@
>> >> device_type = "cpu";
>> >> compatible = "arm,cortex-a57", "arm,armv8";
>> >> enable-method = "psci";
>> >> + cpu-idle-states = <&CPU_SLEEP>;
>> >> reg = <0x2>;
>> >> };
>> >>
>> >> @@ -57,8 +60,23 @@
>> >> device_type = "cpu";
>> >> compatible = "arm,cortex-a57", "arm,armv8";
>> >> enable-method = "psci";
>> >> + cpu-idle-states = <&CPU_SLEEP>;
>> >> reg = <0x3>;
>> >> };
>> >> +
>> >> + idle-states {
>> >> + entry-method = "arm,psci";
>> >> +
>> >> + CPU_SLEEP: cpu-sleep {
>> >> + compatible = "arm,idle-state";
>> >> + local-timer-stop;
>> >> + arm,psci-su
Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
On 4 November 2014 22:38, Rafael J. Wysocki wrote: > On Tuesday, November 04, 2014 10:29:20 AM Dmitry Torokhov wrote: >> On Tue, Nov 04, 2014 at 06:01:44PM +0100, Ulf Hansson wrote: >> > >> >> > >> Devices that are created while "discoverable buses" are being probed >> > >> can't be attached to a PM domain before the probing is done, because >> > >> those simply doesn't exist. >> > > >> > > Honestly, I'm not sure what you're talking about. >> > > >> > > Devices on a "discoverable* bus (say PCI) are added when the >> > > *controller* is >> > > probed, not when *they* are probed. >> > >> > Okay, so maybe "discoverable buses" isn't the proper term. >> > >> > > >> > > You very much need to have a struct device registered to be able to call >> > > really_probe() for it. >> > >> > Yes. But my point is that the struct device may be created dynamically >> > at some point in time. >> >> And that is fine. >> >> > >> > This is how mmc/sd/sdio cards are handled. We don't have the >> > information about the card and thus not the struct device of it, until >> > we have detected it. Maybe we could at that point try to add the >> > device to its PM domain? >> >> Well, I think we need to first define what PM domain they will fall >> into. Do they have to be in one? The concept of power domain, as far as I >> understand it, is needed when we need to form relations going outside >> the standard parent/child relationship. Here we have a controller and >> then one or more cards in it. To be able to use card you need to power >> up parent and you can not power down parent until all children are >> powered down. >> >> But in any case, device discovery and binding them to drivers are 2 >> separate steps. You have a PCI bus. You enumerate it - new PCI devices >> are created. Some of them may be put in a certain power domain. You then >> bind drivers to PCI devices (not strictly 'then' but we could implement >> driver core to postpone binding until current round of enumeration is >> complete) - and you discover USB controller and maybe i2c controller. >> Then you bind drivers to them which causes enumeration of the new bus >> and new devices are created. But in all these cases enumeration and >> creation of new 'struct device's, and driver binding are logically >> separate steps. >> >> > >> > > >> > >> Now, I haven't yet seen a demand for such a cases, but it seems wrong >> > >> to not consider them. The current solution cover these. >> > > >> > > Oh dear. Please rethink this. >> > >> > Currently, dev_pm_domain_attach() is being invoked at the point when a >> > SDIO card has been found. From sdio_add_func(). How would that be >> > solved? >> > >> >> It is probably the one place where we currently doing it correctly (form >> logical point of view, not implementation wise - implementation wise in >> sdio we add device to power domain after calling device_add() which >> means that probe() could have been called before we added the new device >> to any power domain). > > I agree. Ideally, devices should be added to power (or generally PM) domains > before registration. That is, before calling device_add() for them. > > That's the first thing to fix here. It's done in patch 7 in this patchset. [PATCH v3 7/9] mmc: core: Attach PM domain prior probing of SDIO func driver Kind regards Uffe -- 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 3/4] PM / Domains: Improve error handling while adding/removing devices
On 5 November 2014 08:47, Geert Uytterhoeven wrote:
> On Tue, Oct 28, 2014 at 3:38 PM, Ulf Hansson wrote:
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -1358,25 +1358,81 @@ 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 int genpd_alloc_dev_data(struct generic_pm_domain *genpd,
>> + struct device *dev, struct gpd_timing_data
>> *td)
>> {
>
> [...]
>
>> + if (genpd->attach_dev)
>> + genpd->attach_dev(dev);
>
> Note that dev->pm_domain is not yet set at this point, so the callee
> can no longer
> know to which domain the device is being attached.
> Should we re-add the parameter, or move the attach_dev() back to
> __pm_genpd_add_device(), like Kevin suggested.
I agree.
I am working on a new version, which adopts to your suggestions.
Kind regards
Uffe
--
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
