* [PATCHv2 0/2] Some omap_device/hwmod/pwrdomain patches @ 2011-05-27 7:38 ` Tomi Valkeinen 0 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 7:38 UTC (permalink / raw) To: Kevin Hilman, Paul Walmsley; +Cc: linux-omap, linux-arm-kernel, Tomi Valkeinen I came up with these patches while implementing pm runtime adaptation for DSS driver. I'm not quite sure on who's turf they belong, or do they even belong together, but here they are anyway. get_context_loss_count patch was discussed on l-o with Kevin. The omap_device_reset patch I made as some parts of DSS currently presume that the HW module is reset when it is enabled, and the reset is better handled in hwmod code. Changes in v2 * Dropped can-ever-lose-context patch * Use INT_MAX instead of ~(1 << 31) when masking the ctx loss count Tomi Tomi Valkeinen (2): OMAP: change get_context_loss_count ret value to int OMAP: add omap_device_reset() arch/arm/mach-omap2/omap_hwmod.c | 2 +- arch/arm/mach-omap2/powerdomain.c | 14 ++++++++++---- arch/arm/mach-omap2/powerdomain.h | 2 +- arch/arm/plat-omap/include/plat/omap-pm.h | 4 ++-- arch/arm/plat-omap/include/plat/omap_device.h | 3 ++- arch/arm/plat-omap/include/plat/omap_hwmod.h | 2 +- arch/arm/plat-omap/omap-pm-noop.c | 24 +++++++++++++++++------- arch/arm/plat-omap/omap_device.c | 25 ++++++++++++++++++++++++- 8 files changed, 58 insertions(+), 18 deletions(-) -- 1.7.4.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 0/2] Some omap_device/hwmod/pwrdomain patches @ 2011-05-27 7:38 ` Tomi Valkeinen 0 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 7:38 UTC (permalink / raw) To: linux-arm-kernel I came up with these patches while implementing pm runtime adaptation for DSS driver. I'm not quite sure on who's turf they belong, or do they even belong together, but here they are anyway. get_context_loss_count patch was discussed on l-o with Kevin. The omap_device_reset patch I made as some parts of DSS currently presume that the HW module is reset when it is enabled, and the reset is better handled in hwmod code. Changes in v2 * Dropped can-ever-lose-context patch * Use INT_MAX instead of ~(1 << 31) when masking the ctx loss count Tomi Tomi Valkeinen (2): OMAP: change get_context_loss_count ret value to int OMAP: add omap_device_reset() arch/arm/mach-omap2/omap_hwmod.c | 2 +- arch/arm/mach-omap2/powerdomain.c | 14 ++++++++++---- arch/arm/mach-omap2/powerdomain.h | 2 +- arch/arm/plat-omap/include/plat/omap-pm.h | 4 ++-- arch/arm/plat-omap/include/plat/omap_device.h | 3 ++- arch/arm/plat-omap/include/plat/omap_hwmod.h | 2 +- arch/arm/plat-omap/omap-pm-noop.c | 24 +++++++++++++++++------- arch/arm/plat-omap/omap_device.c | 25 ++++++++++++++++++++++++- 8 files changed, 58 insertions(+), 18 deletions(-) -- 1.7.4.1 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 1/2] OMAP: change get_context_loss_count ret value to int 2011-05-27 7:38 ` Tomi Valkeinen @ 2011-05-27 7:38 ` Tomi Valkeinen -1 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 7:38 UTC (permalink / raw) To: Kevin Hilman, Paul Walmsley; +Cc: linux-omap, linux-arm-kernel, Tomi Valkeinen get_context_loss_count functions return context loss count as u32, and zero means an error. However, zero is also returned when context has never been lost and could also be returned when the context loss count has wrapped and goes to zero. Change the functions to return an int, with negative value meaning an error. OMAP HSMMC code uses omap_pm_get_dev_context_loss_count(), but as the hsmmc code handles the returned value as an int, with negative value meaning an error, this patch actually fixes hsmmc code also. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Acked-by: Kevin Hilman <khilman@ti.com> --- arch/arm/mach-omap2/omap_hwmod.c | 2 +- arch/arm/mach-omap2/powerdomain.c | 14 ++++++++++---- arch/arm/mach-omap2/powerdomain.h | 2 +- arch/arm/plat-omap/include/plat/omap-pm.h | 4 ++-- arch/arm/plat-omap/include/plat/omap_device.h | 2 +- arch/arm/plat-omap/include/plat/omap_hwmod.h | 2 +- arch/arm/plat-omap/omap-pm-noop.c | 24 +++++++++++++++++------- arch/arm/plat-omap/omap_device.c | 2 +- 8 files changed, 34 insertions(+), 18 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index e034294..4f0d554 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -2332,7 +2332,7 @@ ohsps_unlock: * Returns the context loss count of the powerdomain assocated with @oh * upon success, or zero if no powerdomain exists for @oh. */ -u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh) +int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh) { struct powerdomain *pwrdm; int ret = 0; diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 9af0847..9d53a34 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -935,16 +935,16 @@ int pwrdm_post_transition(void) * @pwrdm: struct powerdomain * to wait for * * Context loss count is the sum of powerdomain off-mode counter, the - * logic off counter and the per-bank memory off counter. Returns 0 + * logic off counter and the per-bank memory off counter. Returns negative * (and WARNs) upon error, otherwise, returns the context loss count. */ -u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm) +int pwrdm_get_context_loss_count(struct powerdomain *pwrdm) { int i, count; if (!pwrdm) { WARN(1, "powerdomain: %s: pwrdm is null\n", __func__); - return 0; + return -ENODEV; } count = pwrdm->state_counter[PWRDM_POWER_OFF]; @@ -953,7 +953,13 @@ u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm) for (i = 0; i < pwrdm->banks; i++) count += pwrdm->ret_mem_off_counter[i]; - pr_debug("powerdomain: %s: context loss count = %u\n", + /* + * Context loss count has to be a non-negative value. Clear the sign + * bit to get a value range from 0 to INT_MAX. + */ + count &= INT_MAX; + + pr_debug("powerdomain: %s: context loss count = %d\n", pwrdm->name, count); return count; diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index d23d979..012827f 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -207,7 +207,7 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm); int pwrdm_pre_transition(void); int pwrdm_post_transition(void); int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm); -u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm); +int pwrdm_get_context_loss_count(struct powerdomain *pwrdm); bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm); extern void omap2xxx_powerdomains_init(void); diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h index c0a7520..68df031 100644 --- a/arch/arm/plat-omap/include/plat/omap-pm.h +++ b/arch/arm/plat-omap/include/plat/omap-pm.h @@ -350,9 +350,9 @@ unsigned long omap_pm_cpu_get_freq(void); * driver must restore device context. If the number of context losses * exceeds the maximum positive integer, the function will wrap to 0 and * continue counting. Returns the number of context losses for this device, - * or zero upon error. + * or negative value upon error. */ -u32 omap_pm_get_dev_context_loss_count(struct device *dev); +int omap_pm_get_dev_context_loss_count(struct device *dev); void omap_pm_enable_off_mode(void); void omap_pm_disable_off_mode(void); diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h index e4c349f..70d31d0 100644 --- a/arch/arm/plat-omap/include/plat/omap_device.h +++ b/arch/arm/plat-omap/include/plat/omap_device.h @@ -107,7 +107,7 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od); int omap_device_align_pm_lat(struct platform_device *pdev, u32 new_wakeup_lat_limit); struct powerdomain *omap_device_get_pwrdm(struct omap_device *od); -u32 omap_device_get_context_loss_count(struct platform_device *pdev); +int omap_device_get_context_loss_count(struct platform_device *pdev); /* Other */ diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h index 1adea9c..8658e2d 100644 --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h @@ -598,7 +598,7 @@ int omap_hwmod_for_each_by_class(const char *classname, void *user); int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state); -u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh); +int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh); int omap_hwmod_no_setup_reset(struct omap_hwmod *oh); diff --git a/arch/arm/plat-omap/omap-pm-noop.c b/arch/arm/plat-omap/omap-pm-noop.c index b0471bb2..3dc3801 100644 --- a/arch/arm/plat-omap/omap-pm-noop.c +++ b/arch/arm/plat-omap/omap-pm-noop.c @@ -27,7 +27,7 @@ #include <plat/omap_device.h> static bool off_mode_enabled; -static u32 dummy_context_loss_counter; +static int dummy_context_loss_counter; /* * Device-driver-originated constraints (via board-*.c files) @@ -311,22 +311,32 @@ void omap_pm_disable_off_mode(void) #ifdef CONFIG_ARCH_OMAP2PLUS -u32 omap_pm_get_dev_context_loss_count(struct device *dev) +int omap_pm_get_dev_context_loss_count(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); - u32 count; + int count; if (WARN_ON(!dev)) - return 0; + return -ENODEV; if (dev->parent == &omap_device_parent) { count = omap_device_get_context_loss_count(pdev); } else { WARN_ONCE(off_mode_enabled, "omap_pm: using dummy context loss counter; device %s should be converted to omap_device", dev_name(dev)); - if (off_mode_enabled) - dummy_context_loss_counter++; + count = dummy_context_loss_counter; + + if (off_mode_enabled) { + count++; + /* + * Context loss count has to be a non-negative value. + * Clear the sign bit to get a value range from 0 to + * INT_MAX. + */ + count &= INT_MAX; + dummy_context_loss_counter = count; + } } pr_debug("OMAP PM: context loss count for dev %s = %d\n", @@ -337,7 +347,7 @@ u32 omap_pm_get_dev_context_loss_count(struct device *dev) #else -u32 omap_pm_get_dev_context_loss_count(struct device *dev) +int omap_pm_get_dev_context_loss_count(struct device *dev) { return dummy_context_loss_counter; } diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c index 9bbda9a..9753f71 100644 --- a/arch/arm/plat-omap/omap_device.c +++ b/arch/arm/plat-omap/omap_device.c @@ -310,7 +310,7 @@ static void _add_optional_clock_clkdev(struct omap_device *od, * return the context loss counter for that hwmod, otherwise return * zero. */ -u32 omap_device_get_context_loss_count(struct platform_device *pdev) +int omap_device_get_context_loss_count(struct platform_device *pdev) { struct omap_device *od; u32 ret = 0; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCHv2 1/2] OMAP: change get_context_loss_count ret value to int @ 2011-05-27 7:38 ` Tomi Valkeinen 0 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 7:38 UTC (permalink / raw) To: linux-arm-kernel get_context_loss_count functions return context loss count as u32, and zero means an error. However, zero is also returned when context has never been lost and could also be returned when the context loss count has wrapped and goes to zero. Change the functions to return an int, with negative value meaning an error. OMAP HSMMC code uses omap_pm_get_dev_context_loss_count(), but as the hsmmc code handles the returned value as an int, with negative value meaning an error, this patch actually fixes hsmmc code also. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> Acked-by: Kevin Hilman <khilman@ti.com> --- arch/arm/mach-omap2/omap_hwmod.c | 2 +- arch/arm/mach-omap2/powerdomain.c | 14 ++++++++++---- arch/arm/mach-omap2/powerdomain.h | 2 +- arch/arm/plat-omap/include/plat/omap-pm.h | 4 ++-- arch/arm/plat-omap/include/plat/omap_device.h | 2 +- arch/arm/plat-omap/include/plat/omap_hwmod.h | 2 +- arch/arm/plat-omap/omap-pm-noop.c | 24 +++++++++++++++++------- arch/arm/plat-omap/omap_device.c | 2 +- 8 files changed, 34 insertions(+), 18 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index e034294..4f0d554 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -2332,7 +2332,7 @@ ohsps_unlock: * Returns the context loss count of the powerdomain assocated with @oh * upon success, or zero if no powerdomain exists for @oh. */ -u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh) +int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh) { struct powerdomain *pwrdm; int ret = 0; diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c index 9af0847..9d53a34 100644 --- a/arch/arm/mach-omap2/powerdomain.c +++ b/arch/arm/mach-omap2/powerdomain.c @@ -935,16 +935,16 @@ int pwrdm_post_transition(void) * @pwrdm: struct powerdomain * to wait for * * Context loss count is the sum of powerdomain off-mode counter, the - * logic off counter and the per-bank memory off counter. Returns 0 + * logic off counter and the per-bank memory off counter. Returns negative * (and WARNs) upon error, otherwise, returns the context loss count. */ -u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm) +int pwrdm_get_context_loss_count(struct powerdomain *pwrdm) { int i, count; if (!pwrdm) { WARN(1, "powerdomain: %s: pwrdm is null\n", __func__); - return 0; + return -ENODEV; } count = pwrdm->state_counter[PWRDM_POWER_OFF]; @@ -953,7 +953,13 @@ u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm) for (i = 0; i < pwrdm->banks; i++) count += pwrdm->ret_mem_off_counter[i]; - pr_debug("powerdomain: %s: context loss count = %u\n", + /* + * Context loss count has to be a non-negative value. Clear the sign + * bit to get a value range from 0 to INT_MAX. + */ + count &= INT_MAX; + + pr_debug("powerdomain: %s: context loss count = %d\n", pwrdm->name, count); return count; diff --git a/arch/arm/mach-omap2/powerdomain.h b/arch/arm/mach-omap2/powerdomain.h index d23d979..012827f 100644 --- a/arch/arm/mach-omap2/powerdomain.h +++ b/arch/arm/mach-omap2/powerdomain.h @@ -207,7 +207,7 @@ int pwrdm_clkdm_state_switch(struct clockdomain *clkdm); int pwrdm_pre_transition(void); int pwrdm_post_transition(void); int pwrdm_set_lowpwrstchange(struct powerdomain *pwrdm); -u32 pwrdm_get_context_loss_count(struct powerdomain *pwrdm); +int pwrdm_get_context_loss_count(struct powerdomain *pwrdm); bool pwrdm_can_ever_lose_context(struct powerdomain *pwrdm); extern void omap2xxx_powerdomains_init(void); diff --git a/arch/arm/plat-omap/include/plat/omap-pm.h b/arch/arm/plat-omap/include/plat/omap-pm.h index c0a7520..68df031 100644 --- a/arch/arm/plat-omap/include/plat/omap-pm.h +++ b/arch/arm/plat-omap/include/plat/omap-pm.h @@ -350,9 +350,9 @@ unsigned long omap_pm_cpu_get_freq(void); * driver must restore device context. If the number of context losses * exceeds the maximum positive integer, the function will wrap to 0 and * continue counting. Returns the number of context losses for this device, - * or zero upon error. + * or negative value upon error. */ -u32 omap_pm_get_dev_context_loss_count(struct device *dev); +int omap_pm_get_dev_context_loss_count(struct device *dev); void omap_pm_enable_off_mode(void); void omap_pm_disable_off_mode(void); diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h index e4c349f..70d31d0 100644 --- a/arch/arm/plat-omap/include/plat/omap_device.h +++ b/arch/arm/plat-omap/include/plat/omap_device.h @@ -107,7 +107,7 @@ void __iomem *omap_device_get_rt_va(struct omap_device *od); int omap_device_align_pm_lat(struct platform_device *pdev, u32 new_wakeup_lat_limit); struct powerdomain *omap_device_get_pwrdm(struct omap_device *od); -u32 omap_device_get_context_loss_count(struct platform_device *pdev); +int omap_device_get_context_loss_count(struct platform_device *pdev); /* Other */ diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h index 1adea9c..8658e2d 100644 --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h @@ -598,7 +598,7 @@ int omap_hwmod_for_each_by_class(const char *classname, void *user); int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state); -u32 omap_hwmod_get_context_loss_count(struct omap_hwmod *oh); +int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh); int omap_hwmod_no_setup_reset(struct omap_hwmod *oh); diff --git a/arch/arm/plat-omap/omap-pm-noop.c b/arch/arm/plat-omap/omap-pm-noop.c index b0471bb2..3dc3801 100644 --- a/arch/arm/plat-omap/omap-pm-noop.c +++ b/arch/arm/plat-omap/omap-pm-noop.c @@ -27,7 +27,7 @@ #include <plat/omap_device.h> static bool off_mode_enabled; -static u32 dummy_context_loss_counter; +static int dummy_context_loss_counter; /* * Device-driver-originated constraints (via board-*.c files) @@ -311,22 +311,32 @@ void omap_pm_disable_off_mode(void) #ifdef CONFIG_ARCH_OMAP2PLUS -u32 omap_pm_get_dev_context_loss_count(struct device *dev) +int omap_pm_get_dev_context_loss_count(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); - u32 count; + int count; if (WARN_ON(!dev)) - return 0; + return -ENODEV; if (dev->parent == &omap_device_parent) { count = omap_device_get_context_loss_count(pdev); } else { WARN_ONCE(off_mode_enabled, "omap_pm: using dummy context loss counter; device %s should be converted to omap_device", dev_name(dev)); - if (off_mode_enabled) - dummy_context_loss_counter++; + count = dummy_context_loss_counter; + + if (off_mode_enabled) { + count++; + /* + * Context loss count has to be a non-negative value. + * Clear the sign bit to get a value range from 0 to + * INT_MAX. + */ + count &= INT_MAX; + dummy_context_loss_counter = count; + } } pr_debug("OMAP PM: context loss count for dev %s = %d\n", @@ -337,7 +347,7 @@ u32 omap_pm_get_dev_context_loss_count(struct device *dev) #else -u32 omap_pm_get_dev_context_loss_count(struct device *dev) +int omap_pm_get_dev_context_loss_count(struct device *dev) { return dummy_context_loss_counter; } diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c index 9bbda9a..9753f71 100644 --- a/arch/arm/plat-omap/omap_device.c +++ b/arch/arm/plat-omap/omap_device.c @@ -310,7 +310,7 @@ static void _add_optional_clock_clkdev(struct omap_device *od, * return the context loss counter for that hwmod, otherwise return * zero. */ -u32 omap_device_get_context_loss_count(struct platform_device *pdev) +int omap_device_get_context_loss_count(struct platform_device *pdev) { struct omap_device *od; u32 ret = 0; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 7:38 ` Tomi Valkeinen @ 2011-05-27 7:38 ` Tomi Valkeinen -1 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 7:38 UTC (permalink / raw) To: Kevin Hilman, Paul Walmsley; +Cc: linux-omap, linux-arm-kernel, Tomi Valkeinen Add omap_device_reset() function which can be used to reset the hwmods associated with the given platform device. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- arch/arm/plat-omap/include/plat/omap_device.h | 1 + arch/arm/plat-omap/omap_device.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 0 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h index 70d31d0..bcf1b35 100644 --- a/arch/arm/plat-omap/include/plat/omap_device.h +++ b/arch/arm/plat-omap/include/plat/omap_device.h @@ -108,6 +108,7 @@ int omap_device_align_pm_lat(struct platform_device *pdev, u32 new_wakeup_lat_limit); struct powerdomain *omap_device_get_pwrdm(struct omap_device *od); int omap_device_get_context_loss_count(struct platform_device *pdev); +int omap_device_reset(struct platform_device *pdev); /* Other */ diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c index 9753f71..4e6fc1b 100644 --- a/arch/arm/plat-omap/omap_device.c +++ b/arch/arm/plat-omap/omap_device.c @@ -324,6 +324,29 @@ int omap_device_get_context_loss_count(struct platform_device *pdev) } /** + * omap_device_reset - reset hwmods of given device + * @pdev: struct platform_device * + * + * Reset all hwmods associated with the given device. If a reset of a hwmod + * fails the rest of the hwmods are skipped and the error is returned. + */ +int omap_device_reset(struct platform_device *pdev) +{ + struct omap_device *od; + int i, r; + + od = _find_by_pdev(pdev); + + for (i = 0; i < od->hwmods_cnt; i++) { + r = omap_hwmod_reset(od->hwmods[i]); + if (r) + return r; + } + + return 0; +} + +/** * omap_device_count_resources - count number of struct resource entries needed * @od: struct omap_device * * -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-27 7:38 ` Tomi Valkeinen 0 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 7:38 UTC (permalink / raw) To: linux-arm-kernel Add omap_device_reset() function which can be used to reset the hwmods associated with the given platform device. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- arch/arm/plat-omap/include/plat/omap_device.h | 1 + arch/arm/plat-omap/omap_device.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 0 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h index 70d31d0..bcf1b35 100644 --- a/arch/arm/plat-omap/include/plat/omap_device.h +++ b/arch/arm/plat-omap/include/plat/omap_device.h @@ -108,6 +108,7 @@ int omap_device_align_pm_lat(struct platform_device *pdev, u32 new_wakeup_lat_limit); struct powerdomain *omap_device_get_pwrdm(struct omap_device *od); int omap_device_get_context_loss_count(struct platform_device *pdev); +int omap_device_reset(struct platform_device *pdev); /* Other */ diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c index 9753f71..4e6fc1b 100644 --- a/arch/arm/plat-omap/omap_device.c +++ b/arch/arm/plat-omap/omap_device.c @@ -324,6 +324,29 @@ int omap_device_get_context_loss_count(struct platform_device *pdev) } /** + * omap_device_reset - reset hwmods of given device + * @pdev: struct platform_device * + * + * Reset all hwmods associated with the given device. If a reset of a hwmod + * fails the rest of the hwmods are skipped and the error is returned. + */ +int omap_device_reset(struct platform_device *pdev) +{ + struct omap_device *od; + int i, r; + + od = _find_by_pdev(pdev); + + for (i = 0; i < od->hwmods_cnt; i++) { + r = omap_hwmod_reset(od->hwmods[i]); + if (r) + return r; + } + + return 0; +} + +/** * omap_device_count_resources - count number of struct resource entries needed * @od: struct omap_device * * -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 7:38 ` Tomi Valkeinen @ 2011-05-27 12:38 ` Cousson, Benoit -1 siblings, 0 replies; 30+ messages in thread From: Cousson, Benoit @ 2011-05-27 12:38 UTC (permalink / raw) To: Valkeinen, Tomi Cc: Hilman, Kevin, Paul Walmsley, linux-omap, linux-arm-kernel Hi Tomi, On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: > Add omap_device_reset() function which can be used to reset the hwmods > associated with the given platform device. We've never exposed it because we are trying to avoid that any driver play with asynchronous HW reset. That can lead to undefined HW behavior :-( Do you have some strong need for that? Regards, Benoit > > Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com> > --- > arch/arm/plat-omap/include/plat/omap_device.h | 1 + > arch/arm/plat-omap/omap_device.c | 23 +++++++++++++++++++++++ > 2 files changed, 24 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h > index 70d31d0..bcf1b35 100644 > --- a/arch/arm/plat-omap/include/plat/omap_device.h > +++ b/arch/arm/plat-omap/include/plat/omap_device.h > @@ -108,6 +108,7 @@ int omap_device_align_pm_lat(struct platform_device *pdev, > u32 new_wakeup_lat_limit); > struct powerdomain *omap_device_get_pwrdm(struct omap_device *od); > int omap_device_get_context_loss_count(struct platform_device *pdev); > +int omap_device_reset(struct platform_device *pdev); > > /* Other */ > > diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c > index 9753f71..4e6fc1b 100644 > --- a/arch/arm/plat-omap/omap_device.c > +++ b/arch/arm/plat-omap/omap_device.c > @@ -324,6 +324,29 @@ int omap_device_get_context_loss_count(struct platform_device *pdev) > } > > /** > + * omap_device_reset - reset hwmods of given device > + * @pdev: struct platform_device * > + * > + * Reset all hwmods associated with the given device. If a reset of a hwmod > + * fails the rest of the hwmods are skipped and the error is returned. > + */ > +int omap_device_reset(struct platform_device *pdev) > +{ > + struct omap_device *od; > + int i, r; > + > + od = _find_by_pdev(pdev); > + > + for (i = 0; i< od->hwmods_cnt; i++) { > + r = omap_hwmod_reset(od->hwmods[i]); > + if (r) > + return r; > + } > + > + return 0; > +} > + > +/** > * omap_device_count_resources - count number of struct resource entries needed > * @od: struct omap_device * > * ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-27 12:38 ` Cousson, Benoit 0 siblings, 0 replies; 30+ messages in thread From: Cousson, Benoit @ 2011-05-27 12:38 UTC (permalink / raw) To: linux-arm-kernel Hi Tomi, On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: > Add omap_device_reset() function which can be used to reset the hwmods > associated with the given platform device. We've never exposed it because we are trying to avoid that any driver play with asynchronous HW reset. That can lead to undefined HW behavior :-( Do you have some strong need for that? Regards, Benoit > > Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com> > --- > arch/arm/plat-omap/include/plat/omap_device.h | 1 + > arch/arm/plat-omap/omap_device.c | 23 +++++++++++++++++++++++ > 2 files changed, 24 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/plat-omap/include/plat/omap_device.h b/arch/arm/plat-omap/include/plat/omap_device.h > index 70d31d0..bcf1b35 100644 > --- a/arch/arm/plat-omap/include/plat/omap_device.h > +++ b/arch/arm/plat-omap/include/plat/omap_device.h > @@ -108,6 +108,7 @@ int omap_device_align_pm_lat(struct platform_device *pdev, > u32 new_wakeup_lat_limit); > struct powerdomain *omap_device_get_pwrdm(struct omap_device *od); > int omap_device_get_context_loss_count(struct platform_device *pdev); > +int omap_device_reset(struct platform_device *pdev); > > /* Other */ > > diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c > index 9753f71..4e6fc1b 100644 > --- a/arch/arm/plat-omap/omap_device.c > +++ b/arch/arm/plat-omap/omap_device.c > @@ -324,6 +324,29 @@ int omap_device_get_context_loss_count(struct platform_device *pdev) > } > > /** > + * omap_device_reset - reset hwmods of given device > + * @pdev: struct platform_device * > + * > + * Reset all hwmods associated with the given device. If a reset of a hwmod > + * fails the rest of the hwmods are skipped and the error is returned. > + */ > +int omap_device_reset(struct platform_device *pdev) > +{ > + struct omap_device *od; > + int i, r; > + > + od = _find_by_pdev(pdev); > + > + for (i = 0; i< od->hwmods_cnt; i++) { > + r = omap_hwmod_reset(od->hwmods[i]); > + if (r) > + return r; > + } > + > + return 0; > +} > + > +/** > * omap_device_count_resources - count number of struct resource entries needed > * @od: struct omap_device * > * ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 12:38 ` Cousson, Benoit @ 2011-05-27 12:46 ` Tomi Valkeinen -1 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 12:46 UTC (permalink / raw) To: Cousson, Benoit Cc: Hilman, Kevin, Paul Walmsley, linux-omap, linux-arm-kernel On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: > Hi Tomi, > > On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: > > Add omap_device_reset() function which can be used to reset the hwmods > > associated with the given platform device. > > We've never exposed it because we are trying to avoid that any driver > play with asynchronous HW reset. That can lead to undefined HW behavior :-( > > Do you have some strong need for that? DSS driver has been designed so that it resets the HW before it begins programming it. That way we get the HW into known state. Otherwise we need to be extra careful to program all possible registers to a sane value. Not impossible, of course, but requires extra work. I noticed the problem with DSI driver, it didn't work anymore if I didn't reset it. Why does it lead to undefined HW behaviour? Isn't it much better to reset the HW before starting to use it to be 100% sure it's in known and valid state? Especially in error situations it may be difficult (even impossible) to recover without reset. DISPC has been known to froze in some sync lost situations, and, if I recall right, if DSI transfer is aborted the only way to recover is to reset the DSI block (on OMAP3). Tomi ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-27 12:46 ` Tomi Valkeinen 0 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 12:46 UTC (permalink / raw) To: linux-arm-kernel On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: > Hi Tomi, > > On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: > > Add omap_device_reset() function which can be used to reset the hwmods > > associated with the given platform device. > > We've never exposed it because we are trying to avoid that any driver > play with asynchronous HW reset. That can lead to undefined HW behavior :-( > > Do you have some strong need for that? DSS driver has been designed so that it resets the HW before it begins programming it. That way we get the HW into known state. Otherwise we need to be extra careful to program all possible registers to a sane value. Not impossible, of course, but requires extra work. I noticed the problem with DSI driver, it didn't work anymore if I didn't reset it. Why does it lead to undefined HW behaviour? Isn't it much better to reset the HW before starting to use it to be 100% sure it's in known and valid state? Especially in error situations it may be difficult (even impossible) to recover without reset. DISPC has been known to froze in some sync lost situations, and, if I recall right, if DSI transfer is aborted the only way to recover is to reset the DSI block (on OMAP3). Tomi ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 12:46 ` Tomi Valkeinen @ 2011-05-27 14:43 ` Cousson, Benoit -1 siblings, 0 replies; 30+ messages in thread From: Cousson, Benoit @ 2011-05-27 14:43 UTC (permalink / raw) To: Valkeinen, Tomi Cc: Hilman, Kevin, Paul Walmsley, linux-omap, linux-arm-kernel On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: > On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: >> Hi Tomi, >> >> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: >>> Add omap_device_reset() function which can be used to reset the hwmods >>> associated with the given platform device. >> >> We've never exposed it because we are trying to avoid that any driver >> play with asynchronous HW reset. That can lead to undefined HW behavior :-( >> >> Do you have some strong need for that? > > DSS driver has been designed so that it resets the HW before it begins > programming it. That way we get the HW into known state. Otherwise we > need to be extra careful to program all possible registers to a sane > value. Not impossible, of course, but requires extra work. > > I noticed the problem with DSI driver, it didn't work anymore if I > didn't reset it. > > Why does it lead to undefined HW behaviour? Isn't it much better to > reset the HW before starting to use it to be 100% sure it's in known and > valid state? In theory, but since your are resetting only the DSS IP, it can leads to side effect at SoC level. Especially wrt to clock management. > Especially in error situations it may be difficult (even impossible) to > recover without reset. DISPC has been known to froze in some sync lost > situations, and, if I recall right, if DSI transfer is aborted the only > way to recover is to reset the DSI block (on OMAP3). In case of recovery error it makes sense. What we did with hardreset is to re-assert the reset upon disable of the module and then the next enable will de-assert it. Softreset does not do that today. My only concern is that people might start abusing that kind of API to use that for funtionnal purpose instead of error recovery. This other issue is that it will add another OMAP specific IP to the driver. Benoit ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-27 14:43 ` Cousson, Benoit 0 siblings, 0 replies; 30+ messages in thread From: Cousson, Benoit @ 2011-05-27 14:43 UTC (permalink / raw) To: linux-arm-kernel On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: > On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: >> Hi Tomi, >> >> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: >>> Add omap_device_reset() function which can be used to reset the hwmods >>> associated with the given platform device. >> >> We've never exposed it because we are trying to avoid that any driver >> play with asynchronous HW reset. That can lead to undefined HW behavior :-( >> >> Do you have some strong need for that? > > DSS driver has been designed so that it resets the HW before it begins > programming it. That way we get the HW into known state. Otherwise we > need to be extra careful to program all possible registers to a sane > value. Not impossible, of course, but requires extra work. > > I noticed the problem with DSI driver, it didn't work anymore if I > didn't reset it. > > Why does it lead to undefined HW behaviour? Isn't it much better to > reset the HW before starting to use it to be 100% sure it's in known and > valid state? In theory, but since your are resetting only the DSS IP, it can leads to side effect at SoC level. Especially wrt to clock management. > Especially in error situations it may be difficult (even impossible) to > recover without reset. DISPC has been known to froze in some sync lost > situations, and, if I recall right, if DSI transfer is aborted the only > way to recover is to reset the DSI block (on OMAP3). In case of recovery error it makes sense. What we did with hardreset is to re-assert the reset upon disable of the module and then the next enable will de-assert it. Softreset does not do that today. My only concern is that people might start abusing that kind of API to use that for funtionnal purpose instead of error recovery. This other issue is that it will add another OMAP specific IP to the driver. Benoit ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 14:43 ` Cousson, Benoit @ 2011-05-27 14:51 ` Santosh Shilimkar -1 siblings, 0 replies; 30+ messages in thread From: Santosh Shilimkar @ 2011-05-27 14:51 UTC (permalink / raw) To: Cousson, Benoit Cc: Valkeinen, Tomi, Hilman, Kevin, Paul Walmsley, linux-omap, linux-arm-kernel On 5/27/2011 8:13 PM, Cousson, Benoit wrote: > On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: >> On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: >>> Hi Tomi, >>> >>> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: >>>> Add omap_device_reset() function which can be used to reset the hwmods >>>> associated with the given platform device. >>> >>> We've never exposed it because we are trying to avoid that any driver >>> play with asynchronous HW reset. That can lead to undefined HW >>> behavior :-( >>> >>> Do you have some strong need for that? >> >> DSS driver has been designed so that it resets the HW before it begins >> programming it. That way we get the HW into known state. Otherwise we >> need to be extra careful to program all possible registers to a sane >> value. Not impossible, of course, but requires extra work. >> >> I noticed the problem with DSI driver, it didn't work anymore if I >> didn't reset it. >> >> Why does it lead to undefined HW behaviour? Isn't it much better to >> reset the HW before starting to use it to be 100% sure it's in known and >> valid state? > > In theory, but since your are resetting only the DSS IP, it can leads to > side effect at SoC level. Especially wrt to clock management. > >> Especially in error situations it may be difficult (even impossible) to >> recover without reset. DISPC has been known to froze in some sync lost >> situations, and, if I recall right, if DSI transfer is aborted the only >> way to recover is to reset the DSI block (on OMAP3). > > In case of recovery error it makes sense. What we did with hardreset is > to re-assert the reset upon disable of the module and then the next > enable will de-assert it. Softreset does not do that today. > I didn't notice this patch but Paul reported an issue on beagle which was making L3 error handling driver hang. Later on after debugging we noticed, that DSS initiator was throwing timeout error. As a temporary fix, we removed the timeout error from the handler since root-cause was not known. [1] I am not sure but may be a proper DSS reset might fix that issue as well. Regards Santosh [1] https://patchwork.kernel.org/patch/769482/ ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-27 14:51 ` Santosh Shilimkar 0 siblings, 0 replies; 30+ messages in thread From: Santosh Shilimkar @ 2011-05-27 14:51 UTC (permalink / raw) To: linux-arm-kernel On 5/27/2011 8:13 PM, Cousson, Benoit wrote: > On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: >> On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: >>> Hi Tomi, >>> >>> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: >>>> Add omap_device_reset() function which can be used to reset the hwmods >>>> associated with the given platform device. >>> >>> We've never exposed it because we are trying to avoid that any driver >>> play with asynchronous HW reset. That can lead to undefined HW >>> behavior :-( >>> >>> Do you have some strong need for that? >> >> DSS driver has been designed so that it resets the HW before it begins >> programming it. That way we get the HW into known state. Otherwise we >> need to be extra careful to program all possible registers to a sane >> value. Not impossible, of course, but requires extra work. >> >> I noticed the problem with DSI driver, it didn't work anymore if I >> didn't reset it. >> >> Why does it lead to undefined HW behaviour? Isn't it much better to >> reset the HW before starting to use it to be 100% sure it's in known and >> valid state? > > In theory, but since your are resetting only the DSS IP, it can leads to > side effect at SoC level. Especially wrt to clock management. > >> Especially in error situations it may be difficult (even impossible) to >> recover without reset. DISPC has been known to froze in some sync lost >> situations, and, if I recall right, if DSI transfer is aborted the only >> way to recover is to reset the DSI block (on OMAP3). > > In case of recovery error it makes sense. What we did with hardreset is > to re-assert the reset upon disable of the module and then the next > enable will de-assert it. Softreset does not do that today. > I didn't notice this patch but Paul reported an issue on beagle which was making L3 error handling driver hang. Later on after debugging we noticed, that DSS initiator was throwing timeout error. As a temporary fix, we removed the timeout error from the handler since root-cause was not known. [1] I am not sure but may be a proper DSS reset might fix that issue as well. Regards Santosh [1] https://patchwork.kernel.org/patch/769482/ ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 14:51 ` Santosh Shilimkar @ 2011-05-27 15:00 ` Cousson, Benoit -1 siblings, 0 replies; 30+ messages in thread From: Cousson, Benoit @ 2011-05-27 15:00 UTC (permalink / raw) To: Shilimkar, Santosh Cc: Valkeinen, Tomi, Hilman, Kevin, Paul Walmsley, linux-omap, linux-arm-kernel On 5/27/2011 4:51 PM, Shilimkar, Santosh wrote: > On 5/27/2011 8:13 PM, Cousson, Benoit wrote: >> On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: >>> On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: >>>> Hi Tomi, >>>> >>>> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: >>>>> Add omap_device_reset() function which can be used to reset the hwmods >>>>> associated with the given platform device. >>>> >>>> We've never exposed it because we are trying to avoid that any driver >>>> play with asynchronous HW reset. That can lead to undefined HW >>>> behavior :-( >>>> >>>> Do you have some strong need for that? >>> >>> DSS driver has been designed so that it resets the HW before it begins >>> programming it. That way we get the HW into known state. Otherwise we >>> need to be extra careful to program all possible registers to a sane >>> value. Not impossible, of course, but requires extra work. >>> >>> I noticed the problem with DSI driver, it didn't work anymore if I >>> didn't reset it. >>> >>> Why does it lead to undefined HW behaviour? Isn't it much better to >>> reset the HW before starting to use it to be 100% sure it's in known and >>> valid state? >> >> In theory, but since your are resetting only the DSS IP, it can leads to >> side effect at SoC level. Especially wrt to clock management. >> >>> Especially in error situations it may be difficult (even impossible) to >>> recover without reset. DISPC has been known to froze in some sync lost >>> situations, and, if I recall right, if DSI transfer is aborted the only >>> way to recover is to reset the DSI block (on OMAP3). >> >> In case of recovery error it makes sense. What we did with hardreset is >> to re-assert the reset upon disable of the module and then the next >> enable will de-assert it. Softreset does not do that today. >> > I didn't notice this patch but Paul reported an issue on beagle > which was making L3 error handling driver hang. > > Later on after debugging we noticed, that DSS initiator > was throwing timeout error. > > As a temporary fix, we removed the timeout error from > the handler since root-cause was not known. [1] > > I am not sure but may be a proper DSS reset might fix > that issue as well. Yeah, but this is the issue... people will start abusing that to fix any kind of problems instead of finding the root cause. That should be use only to fix real HW issue that cannot be solve properly by SW. Regards, Benoit ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-27 15:00 ` Cousson, Benoit 0 siblings, 0 replies; 30+ messages in thread From: Cousson, Benoit @ 2011-05-27 15:00 UTC (permalink / raw) To: linux-arm-kernel On 5/27/2011 4:51 PM, Shilimkar, Santosh wrote: > On 5/27/2011 8:13 PM, Cousson, Benoit wrote: >> On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: >>> On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: >>>> Hi Tomi, >>>> >>>> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: >>>>> Add omap_device_reset() function which can be used to reset the hwmods >>>>> associated with the given platform device. >>>> >>>> We've never exposed it because we are trying to avoid that any driver >>>> play with asynchronous HW reset. That can lead to undefined HW >>>> behavior :-( >>>> >>>> Do you have some strong need for that? >>> >>> DSS driver has been designed so that it resets the HW before it begins >>> programming it. That way we get the HW into known state. Otherwise we >>> need to be extra careful to program all possible registers to a sane >>> value. Not impossible, of course, but requires extra work. >>> >>> I noticed the problem with DSI driver, it didn't work anymore if I >>> didn't reset it. >>> >>> Why does it lead to undefined HW behaviour? Isn't it much better to >>> reset the HW before starting to use it to be 100% sure it's in known and >>> valid state? >> >> In theory, but since your are resetting only the DSS IP, it can leads to >> side effect at SoC level. Especially wrt to clock management. >> >>> Especially in error situations it may be difficult (even impossible) to >>> recover without reset. DISPC has been known to froze in some sync lost >>> situations, and, if I recall right, if DSI transfer is aborted the only >>> way to recover is to reset the DSI block (on OMAP3). >> >> In case of recovery error it makes sense. What we did with hardreset is >> to re-assert the reset upon disable of the module and then the next >> enable will de-assert it. Softreset does not do that today. >> > I didn't notice this patch but Paul reported an issue on beagle > which was making L3 error handling driver hang. > > Later on after debugging we noticed, that DSS initiator > was throwing timeout error. > > As a temporary fix, we removed the timeout error from > the handler since root-cause was not known. [1] > > I am not sure but may be a proper DSS reset might fix > that issue as well. Yeah, but this is the issue... people will start abusing that to fix any kind of problems instead of finding the root cause. That should be use only to fix real HW issue that cannot be solve properly by SW. Regards, Benoit ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 15:00 ` Cousson, Benoit @ 2011-05-27 15:06 ` Santosh Shilimkar -1 siblings, 0 replies; 30+ messages in thread From: Santosh Shilimkar @ 2011-05-27 15:06 UTC (permalink / raw) To: Cousson, Benoit Cc: Valkeinen, Tomi, Hilman, Kevin, Paul Walmsley, linux-omap, linux-arm-kernel On 5/27/2011 8:30 PM, Cousson, Benoit wrote: > On 5/27/2011 4:51 PM, Shilimkar, Santosh wrote: >> On 5/27/2011 8:13 PM, Cousson, Benoit wrote: >>> On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: >>>> On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: >>>>> Hi Tomi, >>>>> >>>>> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: >>>>>> Add omap_device_reset() function which can be used to reset the >>>>>> hwmods >>>>>> associated with the given platform device. >>>>> >>>>> We've never exposed it because we are trying to avoid that any driver >>>>> play with asynchronous HW reset. That can lead to undefined HW >>>>> behavior :-( >>>>> >>>>> Do you have some strong need for that? >>>> >>>> DSS driver has been designed so that it resets the HW before it begins >>>> programming it. That way we get the HW into known state. Otherwise we >>>> need to be extra careful to program all possible registers to a sane >>>> value. Not impossible, of course, but requires extra work. >>>> >>>> I noticed the problem with DSI driver, it didn't work anymore if I >>>> didn't reset it. >>>> >>>> Why does it lead to undefined HW behaviour? Isn't it much better to >>>> reset the HW before starting to use it to be 100% sure it's in known >>>> and >>>> valid state? >>> >>> In theory, but since your are resetting only the DSS IP, it can leads to >>> side effect at SoC level. Especially wrt to clock management. >>> >>>> Especially in error situations it may be difficult (even impossible) to >>>> recover without reset. DISPC has been known to froze in some sync lost >>>> situations, and, if I recall right, if DSI transfer is aborted the only >>>> way to recover is to reset the DSI block (on OMAP3). >>> >>> In case of recovery error it makes sense. What we did with hardreset is >>> to re-assert the reset upon disable of the module and then the next >>> enable will de-assert it. Softreset does not do that today. >>> >> I didn't notice this patch but Paul reported an issue on beagle >> which was making L3 error handling driver hang. >> >> Later on after debugging we noticed, that DSS initiator >> was throwing timeout error. >> >> As a temporary fix, we removed the timeout error from >> the handler since root-cause was not known. [1] >> >> I am not sure but may be a proper DSS reset might fix >> that issue as well. > > Yeah, but this is the issue... people will start abusing that to fix any > kind of problems instead of finding the root cause. > May be you are right. But shouldn't we do a proper reset of the IP. On this same topic, I have a patch for timer IP too. As per TRM, soft reset is not enough to properly reset the timer IP and expecting the additional register to be used for reseting the timer IP. > That should be use only to fix real HW issue that cannot be solve > properly by SW. > I agree but TRM doesn't say the softreset is enough to ensure the certain IP's are properly reseted. Regards Santosh ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-27 15:06 ` Santosh Shilimkar 0 siblings, 0 replies; 30+ messages in thread From: Santosh Shilimkar @ 2011-05-27 15:06 UTC (permalink / raw) To: linux-arm-kernel On 5/27/2011 8:30 PM, Cousson, Benoit wrote: > On 5/27/2011 4:51 PM, Shilimkar, Santosh wrote: >> On 5/27/2011 8:13 PM, Cousson, Benoit wrote: >>> On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: >>>> On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: >>>>> Hi Tomi, >>>>> >>>>> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: >>>>>> Add omap_device_reset() function which can be used to reset the >>>>>> hwmods >>>>>> associated with the given platform device. >>>>> >>>>> We've never exposed it because we are trying to avoid that any driver >>>>> play with asynchronous HW reset. That can lead to undefined HW >>>>> behavior :-( >>>>> >>>>> Do you have some strong need for that? >>>> >>>> DSS driver has been designed so that it resets the HW before it begins >>>> programming it. That way we get the HW into known state. Otherwise we >>>> need to be extra careful to program all possible registers to a sane >>>> value. Not impossible, of course, but requires extra work. >>>> >>>> I noticed the problem with DSI driver, it didn't work anymore if I >>>> didn't reset it. >>>> >>>> Why does it lead to undefined HW behaviour? Isn't it much better to >>>> reset the HW before starting to use it to be 100% sure it's in known >>>> and >>>> valid state? >>> >>> In theory, but since your are resetting only the DSS IP, it can leads to >>> side effect at SoC level. Especially wrt to clock management. >>> >>>> Especially in error situations it may be difficult (even impossible) to >>>> recover without reset. DISPC has been known to froze in some sync lost >>>> situations, and, if I recall right, if DSI transfer is aborted the only >>>> way to recover is to reset the DSI block (on OMAP3). >>> >>> In case of recovery error it makes sense. What we did with hardreset is >>> to re-assert the reset upon disable of the module and then the next >>> enable will de-assert it. Softreset does not do that today. >>> >> I didn't notice this patch but Paul reported an issue on beagle >> which was making L3 error handling driver hang. >> >> Later on after debugging we noticed, that DSS initiator >> was throwing timeout error. >> >> As a temporary fix, we removed the timeout error from >> the handler since root-cause was not known. [1] >> >> I am not sure but may be a proper DSS reset might fix >> that issue as well. > > Yeah, but this is the issue... people will start abusing that to fix any > kind of problems instead of finding the root cause. > May be you are right. But shouldn't we do a proper reset of the IP. On this same topic, I have a patch for timer IP too. As per TRM, soft reset is not enough to properly reset the timer IP and expecting the additional register to be used for reseting the timer IP. > That should be use only to fix real HW issue that cannot be solve > properly by SW. > I agree but TRM doesn't say the softreset is enough to ensure the certain IP's are properly reseted. Regards Santosh ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 14:43 ` Cousson, Benoit @ 2011-05-27 16:40 ` Tomi Valkeinen -1 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 16:40 UTC (permalink / raw) To: Cousson, Benoit Cc: Hilman, Kevin, Paul Walmsley, linux-omap, linux-arm-kernel On Fri, 2011-05-27 at 16:43 +0200, Cousson, Benoit wrote: > On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: > > On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: > >> Hi Tomi, > >> > >> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: > >>> Add omap_device_reset() function which can be used to reset the hwmods > >>> associated with the given platform device. > >> > >> We've never exposed it because we are trying to avoid that any driver > >> play with asynchronous HW reset. That can lead to undefined HW behavior :-( > >> > >> Do you have some strong need for that? > > > > DSS driver has been designed so that it resets the HW before it begins > > programming it. That way we get the HW into known state. Otherwise we > > need to be extra careful to program all possible registers to a sane > > value. Not impossible, of course, but requires extra work. > > > > I noticed the problem with DSI driver, it didn't work anymore if I > > didn't reset it. > > > > Why does it lead to undefined HW behaviour? Isn't it much better to > > reset the HW before starting to use it to be 100% sure it's in known and > > valid state? > > In theory, but since your are resetting only the DSS IP, it can leads to > side effect at SoC level. Especially wrt to clock management. Out of interest, can you tell more what problems it could cause? Can they be solved in the hwmod reset code? Also one thing to note, VENC needs a softreset after changing certain configurations, as per TRM. However, VENC doesn't use the standard syscontrol mechanism, so that cannot be done via omap_device interface anyway. > > Especially in error situations it may be difficult (even impossible) to > > recover without reset. DISPC has been known to froze in some sync lost > > situations, and, if I recall right, if DSI transfer is aborted the only > > way to recover is to reset the DSI block (on OMAP3). > > In case of recovery error it makes sense. What we did with hardreset is > to re-assert the reset upon disable of the module and then the next > enable will de-assert it. Softreset does not do that today. > > My only concern is that people might start abusing that kind of API to > use that for funtionnal purpose instead of error recovery. Why do you see it's abusing? I think it's a good driver design to reset the HW before starting operation. Perhaps it's true that in some cases reset could hide (or fix, in a way) a bug in the driver, but in the end what we want is a driver that works in every situation. And doing a HW reset sounds a good way to make the driver more robust. > This other issue is that it will add another OMAP specific IP to the driver. I presume IP == API. And that is true. But isn't reset functionality present in most HW blocks, and thus would it make a good addition to the generic API? Tomi ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-27 16:40 ` Tomi Valkeinen 0 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-27 16:40 UTC (permalink / raw) To: linux-arm-kernel On Fri, 2011-05-27 at 16:43 +0200, Cousson, Benoit wrote: > On 5/27/2011 2:46 PM, Valkeinen, Tomi wrote: > > On Fri, 2011-05-27 at 14:38 +0200, Cousson, Benoit wrote: > >> Hi Tomi, > >> > >> On 5/27/2011 9:38 AM, Valkeinen, Tomi wrote: > >>> Add omap_device_reset() function which can be used to reset the hwmods > >>> associated with the given platform device. > >> > >> We've never exposed it because we are trying to avoid that any driver > >> play with asynchronous HW reset. That can lead to undefined HW behavior :-( > >> > >> Do you have some strong need for that? > > > > DSS driver has been designed so that it resets the HW before it begins > > programming it. That way we get the HW into known state. Otherwise we > > need to be extra careful to program all possible registers to a sane > > value. Not impossible, of course, but requires extra work. > > > > I noticed the problem with DSI driver, it didn't work anymore if I > > didn't reset it. > > > > Why does it lead to undefined HW behaviour? Isn't it much better to > > reset the HW before starting to use it to be 100% sure it's in known and > > valid state? > > In theory, but since your are resetting only the DSS IP, it can leads to > side effect at SoC level. Especially wrt to clock management. Out of interest, can you tell more what problems it could cause? Can they be solved in the hwmod reset code? Also one thing to note, VENC needs a softreset after changing certain configurations, as per TRM. However, VENC doesn't use the standard syscontrol mechanism, so that cannot be done via omap_device interface anyway. > > Especially in error situations it may be difficult (even impossible) to > > recover without reset. DISPC has been known to froze in some sync lost > > situations, and, if I recall right, if DSI transfer is aborted the only > > way to recover is to reset the DSI block (on OMAP3). > > In case of recovery error it makes sense. What we did with hardreset is > to re-assert the reset upon disable of the module and then the next > enable will de-assert it. Softreset does not do that today. > > My only concern is that people might start abusing that kind of API to > use that for funtionnal purpose instead of error recovery. Why do you see it's abusing? I think it's a good driver design to reset the HW before starting operation. Perhaps it's true that in some cases reset could hide (or fix, in a way) a bug in the driver, but in the end what we want is a driver that works in every situation. And doing a HW reset sounds a good way to make the driver more robust. > This other issue is that it will add another OMAP specific IP to the driver. I presume IP == API. And that is true. But isn't reset functionality present in most HW blocks, and thus would it make a good addition to the generic API? Tomi ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 16:40 ` Tomi Valkeinen @ 2011-05-30 2:15 ` Paul Walmsley -1 siblings, 0 replies; 30+ messages in thread From: Paul Walmsley @ 2011-05-30 2:15 UTC (permalink / raw) To: Tomi Valkeinen Cc: Cousson, Benoit, Hilman, Kevin, linux-omap, linux-arm-kernel On Fri, 27 May 2011, Tomi Valkeinen wrote: > However, VENC doesn't use the standard syscontrol mechanism, so that > cannot be done via omap_device interface anyway. struct omap_hwmod_class has a .reset function pointer for this purpose. It's already been necessary to deal with the I2C IP block: https://patchwork.kernel.org/patch/804072/ If all that's needed are the steps in 4430 TRM Rev. T Section 10.6.5.1.2.3 "Subsequence--Video Encoder Software Reset", then it should be possible to handle this internal reset with hwmod code. - Paul ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-30 2:15 ` Paul Walmsley 0 siblings, 0 replies; 30+ messages in thread From: Paul Walmsley @ 2011-05-30 2:15 UTC (permalink / raw) To: linux-arm-kernel On Fri, 27 May 2011, Tomi Valkeinen wrote: > However, VENC doesn't use the standard syscontrol mechanism, so that > cannot be done via omap_device interface anyway. struct omap_hwmod_class has a .reset function pointer for this purpose. It's already been necessary to deal with the I2C IP block: https://patchwork.kernel.org/patch/804072/ If all that's needed are the steps in 4430 TRM Rev. T Section 10.6.5.1.2.3 "Subsequence--Video Encoder Software Reset", then it should be possible to handle this internal reset with hwmod code. - Paul ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-30 2:15 ` Paul Walmsley @ 2011-05-30 6:00 ` Tomi Valkeinen -1 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-30 6:00 UTC (permalink / raw) To: Paul Walmsley Cc: Cousson, Benoit, Hilman, Kevin, linux-omap, linux-arm-kernel On Sun, 2011-05-29 at 20:15 -0600, Paul Walmsley wrote: > On Fri, 27 May 2011, Tomi Valkeinen wrote: > > > However, VENC doesn't use the standard syscontrol mechanism, so that > > cannot be done via omap_device interface anyway. > > struct omap_hwmod_class has a .reset function pointer for this purpose. > It's already been necessary to deal with the I2C IP block: > > https://patchwork.kernel.org/patch/804072/ > > If all that's needed are the steps in 4430 TRM Rev. T Section 10.6.5.1.2.3 > "Subsequence--Video Encoder Software Reset", then it should be possible to > handle this internal reset with hwmod code. Yes, that should be enough. However, the VENC_F_CONTROL register also contains bits that need to be programmed by the driver. I hope that doesn't cause any problems. Probably not, as hwmod code should use that register only at reset time. Tomi ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-30 6:00 ` Tomi Valkeinen 0 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-30 6:00 UTC (permalink / raw) To: linux-arm-kernel On Sun, 2011-05-29 at 20:15 -0600, Paul Walmsley wrote: > On Fri, 27 May 2011, Tomi Valkeinen wrote: > > > However, VENC doesn't use the standard syscontrol mechanism, so that > > cannot be done via omap_device interface anyway. > > struct omap_hwmod_class has a .reset function pointer for this purpose. > It's already been necessary to deal with the I2C IP block: > > https://patchwork.kernel.org/patch/804072/ > > If all that's needed are the steps in 4430 TRM Rev. T Section 10.6.5.1.2.3 > "Subsequence--Video Encoder Software Reset", then it should be possible to > handle this internal reset with hwmod code. Yes, that should be enough. However, the VENC_F_CONTROL register also contains bits that need to be programmed by the driver. I hope that doesn't cause any problems. Probably not, as hwmod code should use that register only at reset time. Tomi ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-30 2:15 ` Paul Walmsley @ 2011-05-30 8:49 ` Cousson, Benoit -1 siblings, 0 replies; 30+ messages in thread From: Cousson, Benoit @ 2011-05-30 8:49 UTC (permalink / raw) To: Paul Walmsley Cc: Valkeinen, Tomi, Hilman, Kevin, linux-omap, linux-arm-kernel On 5/30/2011 4:15 AM, Paul Walmsley wrote: > On Fri, 27 May 2011, Tomi Valkeinen wrote: > >> However, VENC doesn't use the standard syscontrol mechanism, so that >> cannot be done via omap_device interface anyway. > > struct omap_hwmod_class has a .reset function pointer for this purpose. > It's already been necessary to deal with the I2C IP block: > > https://patchwork.kernel.org/patch/804072/ > > If all that's needed are the steps in 4430 TRM Rev. T Section 10.6.5.1.2.3 > "Subsequence--Video Encoder Software Reset", then it should be possible to > handle this internal reset with hwmod code. Mmm, but in that case, if this is purely handled inside the IP, it is up to the driver to manage that. hwmod should not manage more than the SYSCONFIG ocpreset because this is part of the PRCM/OCP wrapper and not really the IP. In the case of the VENC, the reset is part of the regular init sequence, so the driver will take care of it. Don't you think so? Regards, Benoit ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-30 8:49 ` Cousson, Benoit 0 siblings, 0 replies; 30+ messages in thread From: Cousson, Benoit @ 2011-05-30 8:49 UTC (permalink / raw) To: linux-arm-kernel On 5/30/2011 4:15 AM, Paul Walmsley wrote: > On Fri, 27 May 2011, Tomi Valkeinen wrote: > >> However, VENC doesn't use the standard syscontrol mechanism, so that >> cannot be done via omap_device interface anyway. > > struct omap_hwmod_class has a .reset function pointer for this purpose. > It's already been necessary to deal with the I2C IP block: > > https://patchwork.kernel.org/patch/804072/ > > If all that's needed are the steps in 4430 TRM Rev. T Section 10.6.5.1.2.3 > "Subsequence--Video Encoder Software Reset", then it should be possible to > handle this internal reset with hwmod code. Mmm, but in that case, if this is purely handled inside the IP, it is up to the driver to manage that. hwmod should not manage more than the SYSCONFIG ocpreset because this is part of the PRCM/OCP wrapper and not really the IP. In the case of the VENC, the reset is part of the regular init sequence, so the driver will take care of it. Don't you think so? Regards, Benoit ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-27 12:46 ` Tomi Valkeinen @ 2011-05-30 2:24 ` Paul Walmsley -1 siblings, 0 replies; 30+ messages in thread From: Paul Walmsley @ 2011-05-30 2:24 UTC (permalink / raw) To: Tomi Valkeinen Cc: Cousson, Benoit, Hilman, Kevin, linux-omap, linux-arm-kernel Hi Tomi, On Fri, 27 May 2011, Tomi Valkeinen wrote: > DSS driver has been designed so that it resets the HW before it begins > programming it. That way we get the HW into known state. Otherwise we > need to be extra careful to program all possible registers to a sane > value. Not impossible, of course, but requires extra work. > > I noticed the problem with DSI driver, it didn't work anymore if I > didn't reset it. One thing that I don't quite understand about this thread is that the hwmod code should be resetting the DSS hwmods when the system boots. Is something not working with this process? That is a separate case from the error recovery case. Sounds like we may need to expose a reset function for that purpose as well. Tomi, I don't think your proposed patch is enough for this, though: we'd also need to reset the hwmod internal state for that module, and reprogram the SYSCONFIG bits, etc. as mach-omap2/omap_hwmod.c:_setup() does. - Paul ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-30 2:24 ` Paul Walmsley 0 siblings, 0 replies; 30+ messages in thread From: Paul Walmsley @ 2011-05-30 2:24 UTC (permalink / raw) To: linux-arm-kernel Hi Tomi, On Fri, 27 May 2011, Tomi Valkeinen wrote: > DSS driver has been designed so that it resets the HW before it begins > programming it. That way we get the HW into known state. Otherwise we > need to be extra careful to program all possible registers to a sane > value. Not impossible, of course, but requires extra work. > > I noticed the problem with DSI driver, it didn't work anymore if I > didn't reset it. One thing that I don't quite understand about this thread is that the hwmod code should be resetting the DSS hwmods when the system boots. Is something not working with this process? That is a separate case from the error recovery case. Sounds like we may need to expose a reset function for that purpose as well. Tomi, I don't think your proposed patch is enough for this, though: we'd also need to reset the hwmod internal state for that module, and reprogram the SYSCONFIG bits, etc. as mach-omap2/omap_hwmod.c:_setup() does. - Paul ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCHv2 2/2] OMAP: add omap_device_reset() 2011-05-30 2:24 ` Paul Walmsley @ 2011-05-30 5:55 ` Tomi Valkeinen -1 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-30 5:55 UTC (permalink / raw) To: Paul Walmsley Cc: Cousson, Benoit, Hilman, Kevin, linux-omap, linux-arm-kernel On Sun, 2011-05-29 at 20:24 -0600, Paul Walmsley wrote: > Hi Tomi, > > On Fri, 27 May 2011, Tomi Valkeinen wrote: > > > DSS driver has been designed so that it resets the HW before it begins > > programming it. That way we get the HW into known state. Otherwise we > > need to be extra careful to program all possible registers to a sane > > value. Not impossible, of course, but requires extra work. > > > > I noticed the problem with DSI driver, it didn't work anymore if I > > didn't reset it. > > One thing that I don't quite understand about this thread is that the > hwmod code should be resetting the DSS hwmods when the system boots. Is > something not working with this process? I believe that is working fine. The problem appears when DSS is turned off (i.e. the screen is blanked) and then started again. The driver has been written so that it will reset the DSS HW before starting to use that particular DSS HW block, to get a clean state. Remove that reset, and we've got all the old register values lying around, causing the device to malfunction. This should be fixable in the driver, of course, although my experience with DSI (and DSS in general) shows that it's not always very easy to stop and restart the HW without a reset. However, I'd still argue that a driver is more robust if it does do a reset when starting up the HW. Perhaps the driver should be developed without that reset, but in the production device the reset should give us extra protection for unexpected situations. > That is a separate case from the error recovery case. Sounds like we may > need to expose a reset function for that purpose as well. Tomi, I don't > think your proposed patch is enough for this, though: we'd also need to > reset the hwmod internal state for that module, and reprogram the > SYSCONFIG bits, etc. as mach-omap2/omap_hwmod.c:_setup() does. Ah, ok, sounds right. It worked for me, but I probably wouldn't notice if the PM bits in SYSCONFIG are wrong, as I didn't do any PM testing. Tomi ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 2/2] OMAP: add omap_device_reset() @ 2011-05-30 5:55 ` Tomi Valkeinen 0 siblings, 0 replies; 30+ messages in thread From: Tomi Valkeinen @ 2011-05-30 5:55 UTC (permalink / raw) To: linux-arm-kernel On Sun, 2011-05-29 at 20:24 -0600, Paul Walmsley wrote: > Hi Tomi, > > On Fri, 27 May 2011, Tomi Valkeinen wrote: > > > DSS driver has been designed so that it resets the HW before it begins > > programming it. That way we get the HW into known state. Otherwise we > > need to be extra careful to program all possible registers to a sane > > value. Not impossible, of course, but requires extra work. > > > > I noticed the problem with DSI driver, it didn't work anymore if I > > didn't reset it. > > One thing that I don't quite understand about this thread is that the > hwmod code should be resetting the DSS hwmods when the system boots. Is > something not working with this process? I believe that is working fine. The problem appears when DSS is turned off (i.e. the screen is blanked) and then started again. The driver has been written so that it will reset the DSS HW before starting to use that particular DSS HW block, to get a clean state. Remove that reset, and we've got all the old register values lying around, causing the device to malfunction. This should be fixable in the driver, of course, although my experience with DSI (and DSS in general) shows that it's not always very easy to stop and restart the HW without a reset. However, I'd still argue that a driver is more robust if it does do a reset when starting up the HW. Perhaps the driver should be developed without that reset, but in the production device the reset should give us extra protection for unexpected situations. > That is a separate case from the error recovery case. Sounds like we may > need to expose a reset function for that purpose as well. Tomi, I don't > think your proposed patch is enough for this, though: we'd also need to > reset the hwmod internal state for that module, and reprogram the > SYSCONFIG bits, etc. as mach-omap2/omap_hwmod.c:_setup() does. Ah, ok, sounds right. It worked for me, but I probably wouldn't notice if the PM bits in SYSCONFIG are wrong, as I didn't do any PM testing. Tomi ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2011-05-30 8:49 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-05-27 7:38 [PATCHv2 0/2] Some omap_device/hwmod/pwrdomain patches Tomi Valkeinen 2011-05-27 7:38 ` Tomi Valkeinen 2011-05-27 7:38 ` [PATCHv2 1/2] OMAP: change get_context_loss_count ret value to int Tomi Valkeinen 2011-05-27 7:38 ` Tomi Valkeinen 2011-05-27 7:38 ` [PATCHv2 2/2] OMAP: add omap_device_reset() Tomi Valkeinen 2011-05-27 7:38 ` Tomi Valkeinen 2011-05-27 12:38 ` Cousson, Benoit 2011-05-27 12:38 ` Cousson, Benoit 2011-05-27 12:46 ` Tomi Valkeinen 2011-05-27 12:46 ` Tomi Valkeinen 2011-05-27 14:43 ` Cousson, Benoit 2011-05-27 14:43 ` Cousson, Benoit 2011-05-27 14:51 ` Santosh Shilimkar 2011-05-27 14:51 ` Santosh Shilimkar 2011-05-27 15:00 ` Cousson, Benoit 2011-05-27 15:00 ` Cousson, Benoit 2011-05-27 15:06 ` Santosh Shilimkar 2011-05-27 15:06 ` Santosh Shilimkar 2011-05-27 16:40 ` Tomi Valkeinen 2011-05-27 16:40 ` Tomi Valkeinen 2011-05-30 2:15 ` Paul Walmsley 2011-05-30 2:15 ` Paul Walmsley 2011-05-30 6:00 ` Tomi Valkeinen 2011-05-30 6:00 ` Tomi Valkeinen 2011-05-30 8:49 ` Cousson, Benoit 2011-05-30 8:49 ` Cousson, Benoit 2011-05-30 2:24 ` Paul Walmsley 2011-05-30 2:24 ` Paul Walmsley 2011-05-30 5:55 ` Tomi Valkeinen 2011-05-30 5:55 ` Tomi Valkeinen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.