Re: [PATCH 2/9] mm: Provide new get_vaddr_frames() helper
On Fri, 17 Jul 2015 12:23:40 +0200 Hans Verkuil wrote: > On 07/13/2015 04:55 PM, Jan Kara wrote: > > From: Jan Kara > > > > Provide new function get_vaddr_frames(). This function maps virtual > > addresses from given start and fills given array with page frame numbers of > > the corresponding pages. If given start belongs to a normal vma, the > > function > > grabs reference to each of the pages to pin them in memory. If start > > belongs to VM_IO | VM_PFNMAP vma, we don't touch page structures. Caller > > must make sure pfns aren't reused for anything else while he is using > > them. > > > > This function is created for various drivers to simplify handling of > > their buffers. > > > > Acked-by: Mel Gorman > > Acked-by: Vlastimil Babka > > Signed-off-by: Jan Kara > > I'd like to see an Acked-by from Andrew or mm-maintainers before I merge this. I think I already acked this but it got lost. Acked-by: Andrew Morton -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] mm: Provide new get_vaddr_frames() helper
On Tue, 2 Jun 2015 17:23:00 +0200 Jan Kara wrote: > > That's a lump of new code which many kernels won't be needing. Can we > > put all this in a new .c file and select it within drivers/media > > Kconfig? > So the attached patch should do what you had in mind. OK? lgtm. > drivers/gpu/drm/exynos/Kconfig | 1 + > drivers/media/platform/omap/Kconfig | 1 + > drivers/media/v4l2-core/Kconfig | 1 + > mm/Kconfig | 3 + > mm/Makefile | 1 + > mm/frame-vec.c | 233 > But frame_vector.c would be a more pleasing name. For `struct frame_vector'. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] mm: Provide new get_vaddr_frames() helper
On Wed, 13 May 2015 15:08:08 +0200 Jan Kara wrote: > Provide new function get_vaddr_frames(). This function maps virtual > addresses from given start and fills given array with page frame numbers of > the corresponding pages. If given start belongs to a normal vma, the function > grabs reference to each of the pages to pin them in memory. If start > belongs to VM_IO | VM_PFNMAP vma, we don't touch page structures. Caller > must make sure pfns aren't reused for anything else while he is using > them. > > This function is created for various drivers to simplify handling of > their buffers. > > Acked-by: Mel Gorman > Acked-by: Vlastimil Babka > Signed-off-by: Jan Kara > --- > include/linux/mm.h | 44 +++ > mm/gup.c | 226 > + That's a lump of new code which many kernels won't be needing. Can we put all this in a new .c file and select it within drivers/media Kconfig? -- 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] rtc: s3c: Fix the duplicate clock control
On Tue, 10 Feb 2015 12:07:30 +0900 Chanwoo Choi wrote: > This patch fixes the duplicate clock control and split out clock control > function as s3c_rtc_enable_clk()/s3c_rtc_disable_clk() to simplify code. "fixes"? This is a pretty big patch and it is poorly described. Please provide a complete description of what is wrong with the original code and how this patch alters it. Please be sure to describe the user-visible effects of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 00/11] kernel: Add support for restart handler call chain
On Tue, 19 Aug 2014 17:45:27 -0700 Guenter Roeck wrote: > Introduce a system restart handler call chain to solve the described problems. So someone has merged eight of these patches into linux-next but these three: watchdog-s3c2410-add-restart-handler.patch clk-samsung-register-restart-handlers-for-s3c2412-and-s3c2443.patch clk-rockchip-add-restart-handler.patch were omitted. What's up? -- 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 RESEND v9 5/5] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock
On Fri, 12 Sep 2014 10:17:43 +0200 Javier Martinez Canillas
wrote:
> The MAX7802 PMIC has a Real-Time-Clock (RTC) with two alarms.
> This patch adds support for the RTC and is based on a driver
> added by Simon Glass to the Chrome OS kernel 3.8 tree.
>
> Signed-off-by: Javier Martinez Canillas
> Reviewed-by: Krzysztof Kozlowski
Would it be appropriate to gather Simon's signoff?
> +static inline int max77802_rtc_calculate_wday(u8 shifted)
> +{
> + int counter = -1;
> +
> + while (shifted) {
> + shifted >>= 1;
> + counter++;
> + }
Can't use log2() or similar?
> + return counter;
> +}
--
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 RESEND v9 1/5] rtc: max77686: Allow the max77686 rtc to wakeup the system
On Fri, 12 Sep 2014 10:17:39 +0200 Javier Martinez Canillas wrote: > From: Doug Anderson > > The max77686 includes an RTC that keeps power during suspend. It's > convenient to be able to use it as a wakeup source. > > NOTE: due to wakeup ordering problems this patch alone doesn't work so > well on exynos5250-snow. You also need something that brings the i2c > bus up before the max77686 wakeup runs. I removed this paragraph. > Signed-off-by: Doug Anderson > Reviewed-by: Javier Martinez Canillas And I rewrote this to Signed-off-by:, as you were on the patch delivery path. Documentation/SubmittingPatches section 12 has the gory details. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/5] rtc: s3c: Define s3c_rtc structure to remove global variables.
On Mon, 25 Aug 2014 09:57:33 +0900 Chanwoo Choi wrote: > Dear Andrew, > > On 08/23/2014 05:42 AM, Andrew Morton wrote: > > On Tue, 12 Aug 2014 11:01:07 +0900 y...@samsung.com wrote: > > > >> This patch define s3c_rtc structure including necessary variables for S3C > >> RTC > >> device instead of global variables. This patch improves the readability by > >> removing global variables. > > > > Below is the v1->v2 delta. > > > > Why were all those tests of info->base added? Can it really be zero? > > I don't see how. > > If some functions (e.g., s3c_rtc_settime) accesses the rtc register > by using info->base before the initialization of info->base in s3c_rtc_probe, > I thought that null pointer error would happen. probe() should be called before anything else. If we're somehow calling s3c_rtc_settime() before probe() has completed then something very bad is happening - for example, the device may have been registered far too early. But I don't think that's the case here. That being said, it does seem strange that s3c_rtc_probe() calls devm_rtc_device_register() *before* trying to request its IRQs. So if IRQ requesting fails, we go and immediately unregister the device. Some other drivers do it this way, others do not. Wouldn't it be better to defer registration until we know that all the probe() setup operations have succeeded? > But, I missed one point which info->base might have the garbate data instead > of NULL. > I'll add the initialization code for info->base. > info->base = NULL; > > If you don't agree it, I'll drop this code checking the state of info->base > on next patchset(v3). Well, we should have those checks in there unless we know they're needed. And if they *are* needed, we should have a good understanding of why they're needed, and we should be sure that we're not just working around some underlying problem. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/5] rtc: s3c: Define s3c_rtc structure to remove global variables.
On Tue, 12 Aug 2014 11:01:07 +0900 y...@samsung.com wrote:
> This patch define s3c_rtc structure including necessary variables for S3C RTC
> device instead of global variables. This patch improves the readability by
> removing global variables.
Below is the v1->v2 delta.
Why were all those tests of info->base added? Can it really be zero?
I don't see how.
---
a/drivers/rtc/rtc-s3c.c~rtc-s3c-define-s3c_rtc-structure-to-remove-global-variables-v2
+++ a/drivers/rtc/rtc-s3c.c
@@ -121,6 +121,9 @@ static int s3c_rtc_setaie(struct device
struct s3c_rtc *info = dev_get_drvdata(dev);
unsigned int tmp;
+ if (!info->base)
+ return -EINVAL;
+
dev_dbg(info->dev, "%s: aie=%d\n", __func__, enabled);
clk_enable(info->rtc_clk);
@@ -180,6 +183,9 @@ static int s3c_rtc_gettime(struct device
struct s3c_rtc *info = dev_get_drvdata(dev);
unsigned int have_retried = 0;
+ if (!info->base)
+ return -EINVAL;
+
clk_enable(info->rtc_clk);
retry_get_time:
rtc_tm->tm_min = readb(info->base + S3C2410_RTCMIN);
@@ -224,6 +230,9 @@ static int s3c_rtc_settime(struct device
struct s3c_rtc *info = dev_get_drvdata(dev);
int year = tm->tm_year - 100;
+ if (!info->base)
+ return -EINVAL;
+
dev_dbg(dev, "set time %04d.%02d.%02d %02d:%02d:%02d\n",
1900 + tm->tm_year, tm->tm_mon, tm->tm_mday,
tm->tm_hour, tm->tm_min, tm->tm_sec);
@@ -255,6 +264,9 @@ static int s3c_rtc_getalarm(struct devic
struct rtc_time *alm_tm = &alrm->time;
unsigned int alm_en;
+ if (!info->base)
+ return -EINVAL;
+
clk_enable(info->rtc_clk);
alm_tm->tm_sec = readb(info->base + S3C2410_ALMSEC);
alm_tm->tm_min = readb(info->base + S3C2410_ALMMIN);
@@ -317,6 +329,9 @@ static int s3c_rtc_setalarm(struct devic
struct rtc_time *tm = &alrm->time;
unsigned int alrm_en;
+ if (!info->base)
+ return -EINVAL;
+
clk_enable(info->rtc_clk);
dev_dbg(dev, "s3c_rtc_setalarm: %d, %04d.%02d.%02d %02d:%02d:%02d\n",
alrm->enabled,
@@ -357,6 +372,9 @@ static int s3c_rtc_proc(struct device *d
struct s3c_rtc *info = dev_get_drvdata(dev);
unsigned int ticnt;
+ if (!info->base)
+ return -EINVAL;
+
clk_enable(info->rtc_clk);
if (info->cpu_type == TYPE_S3C64XX) {
ticnt = readw(info->base + S3C2410_RTCCON);
@@ -548,7 +566,7 @@ static int s3c_rtc_probe(struct platform
rtc_tm.tm_min = 0;
rtc_tm.tm_sec = 0;
- s3c_rtc_settime(NULL, &rtc_tm);
+ s3c_rtc_settime(&pdev->dev, &rtc_tm);
dev_warn(&pdev->dev, "warning: invalid RTC value so
initializing it\n");
}
_
--
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: [rtc-linux] [PATCH] rtc: max8998: Check for pdata presence before dereferencing
On Wed, 15 May 2013 17:16:08 +0200 Tomasz Figa wrote:
> Currently the driver can crash with a NULL pointer dereference if no pdata
> is provided, despite of successful registration of MFD part. This patch
> fixes the problem by adding a NULL check before dereferencing the pdata
> pointer.
>
> ...
>
> --- a/drivers/rtc/rtc-max8998.c
> +++ b/drivers/rtc/rtc-max8998.c
> @@ -285,7 +285,7 @@ static int max8998_rtc_probe(struct platform_device *pdev)
> info->irq, ret);
>
> dev_info(&pdev->dev, "RTC CHIP NAME: %s\n", pdev->id_entry->name);
> - if (pdata->rtc_delay) {
> + if (pdata && pdata->rtc_delay) {
> info->lp3974_bug_workaround = true;
> dev_warn(&pdev->dev, "LP3974 with RTC REGERR option."
> " RTC updates will be extremely slow.\n");
Looking at your description I'm unable to determine which kernel
versions we should fix. This is because the changelog didn't describe
the circumstances under which the bug triggers.
It's pretty simple: when fixing a bug, include a full description of
the bug! As if you were sending a bug report, not a patch.
--
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: [RESEND PATCH v4 1/5] thermal: add generic cpufreq cooling implementation
On Thu, 12 Jul 2012 19:11:04 +0530 Amit Daniel Kachhap wrote: > [a...@linux-foundation.org: fix comment layout] > Signed-off-by: Amit Daniel Kachhap > Cc: Donggeun Kim > Cc: Guenter Roeck > Cc: SangWook Ju > Cc: Durgadoss > Cc: Len Brown > Cc: Jean Delvare > Signed-off-by: Andrew Morton Something strange appears to have happened here? At a guess it seems that the patches were in my tree, I sent them to someone (Len?), then they were merged into linux-next by "someone" and then they fell out of linux-next again? If so, they will hopefully come back soon. If not, something failed fairly seriously. I took a look at re-merging these patches into my tree, but there are significant conflicts with other work which has gone into linux-next. -- 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: mm: fix faulty initialization in vmalloc_init()
On Thu, 24 May 2012 17:32:56 +0900
KyongHo wrote:
> vmalloc_init() adds 'vmap_area's for early 'vm_struct's.
> This patch fixes vmalloc_init() to correctly initialize
> vmap_area for the given vm_struct.
>
Insufficient information. When fixing a bug please always always
always describe the user-visible effects of the bug. Does the kernel
instantly crash? Is it a comestic cleanliness thing which has no
effect? Something in between? I have simply no idea, and am dependent
upon you to tell me.
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1185,9 +1185,10 @@ void __init vmalloc_init(void)
> /* Import existing vmlist entries. */
> for (tmp = vmlist; tmp; tmp = tmp->next) {
> va = kzalloc(sizeof(struct vmap_area), GFP_NOWAIT);
> - va->flags = tmp->flags | VM_VM_AREA;
> + va->flags = VM_VM_AREA;
This change is a mystery. Why do we no longer transfer ->flags?
> va->va_start = (unsigned long)tmp->addr;
> va->va_end = va->va_start + tmp->size;
> + va->vm = tmp;
OK, I can see how this might be important. But why did you find it
necessary? Why was this change actually needed?
> __insert_vmap_area(va);
> }
--
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 6/6] ARM: exynos: Add thermal sensor driver platform data support
On Tue, 8 May 2012 21:48:18 +0530
Amit Daniel Kachhap wrote:
> This patch adds necessary default platform data support needed for TMU driver.
> This dt/non-dt values are tested for origen exynos4210 and smdk exynos5250
> platforms.
>
>
> ...
>
> --- a/drivers/thermal/exynos_thermal.c
> +++ b/drivers/thermal/exynos_thermal.c
> @@ -646,14 +646,117 @@ static irqreturn_t exynos_tmu_irq(int irq, void *id)
> static struct thermal_sensor_conf exynos_sensor_conf = {
> .name = "exynos-therm",
> .read_temperature = (int (*)(void *))exynos_tmu_read,
> +};
> +
> +#if defined(CONFIG_CPU_EXYNOS4210)
> +static struct exynos_tmu_platform_data exynos4_default_tmu_data = {
Again, make it const if possible.
> + .threshold = 80,
> + .trigger_levels[0] = 5,
> + .trigger_levels[1] = 20,
> + .trigger_levels[2] = 30,
> + .trigger_level0_en = 1,
> + .trigger_level1_en = 1,
> + .trigger_level2_en = 1,
> + .trigger_level3_en = 0,
> + .gain = 15,
> + .reference_voltage = 7,
> + .cal_type = TYPE_ONE_POINT_TRIMMING,
> + .freq_tab[0] = {
> + .freq_clip_max = 800 * 1000,
> + },
> + .freq_tab[1] = {
> + .freq_clip_max = 200 * 1000,
> + },
> + .freq_tab_count = 2,
> + .type = SOC_ARCH_EXYNOS4,
> +};
> +#define EXYNOS4_TMU_DRV_DATA ((kernel_ulong_t)&exynos4_default_tmu_data)
> +#else
> +#define EXYNOS4_TMU_DRV_DATA ((kernel_ulong_t)NULL)
> +#endif
See below.
> +#if defined(CONFIG_SOC_EXYNOS5250)
> +static struct exynos_tmu_platform_data exynos5_default_tmu_data = {
> + .trigger_levels[0] = 85,
> + .trigger_levels[1] = 103,
> + .trigger_levels[2] = 110,
> + .trigger_level0_en = 1,
> + .trigger_level1_en = 1,
> + .trigger_level2_en = 1,
> + .trigger_level3_en = 0,
> + .gain = 8,
> + .reference_voltage = 16,
> + .noise_cancel_mode = 4,
> + .cal_type = TYPE_ONE_POINT_TRIMMING,
> + .efuse_value = 55,
> + .freq_tab[0] = {
> + .freq_clip_max = 800 * 1000,
> + },
> + .freq_tab[1] = {
> + .freq_clip_max = 200 * 1000,
> + },
> + .freq_tab_count = 2,
> + .type = SOC_ARCH_EXYNOS5,
> +};
> +#define EXYNOS5_TMU_DRV_DATA ((kernel_ulong_t)&exynos5_default_tmu_data)
The use of kernel_ulong_t is unexpected.
I suspect you could remove this cast altogether. Or make it void*.
> +#else
> +#define EXYNOS5_TMU_DRV_DATA ((kernel_ulong_t)NULL)
And remove this cast too. Rely upon void* magic.
> +#endif
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id exynos_tmu_match[] = {
> + {
> + .compatible = "samsung,exynos4-tmu",
> + .data = (void *)EXYNOS4_TMU_DRV_DATA,
No cast is needed if EXYNOS4_TMU_DRV_DATA has a pointer type.
> + },
> + {
> + .compatible = "samsung,exynos5-tmu",
> + .data = (void *)EXYNOS5_TMU_DRV_DATA,
No cast is needed if EXYNOS4_TMU_DRV_DATA has a pointer type.
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_tmu_match);
> +#else
> +#define exynos_tmu_match NULL
> +#endif
> +
>
> ...
>
--
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 5/6] thermal: exynos: Register the tmu sensor with the kernel thermal layer
On Tue, 8 May 2012 21:48:17 +0530
Amit Daniel Kachhap wrote:
> This code added creates a link between temperature sensors, linux thermal
> framework and cooling devices for samsung exynos platform. This layer
> monitors the temperature from the sensor and informs the generic thermal
> layer to take the necessary cooling action.
>
>
> ...
>
> +static void exynos_report_trigger(void)
> +{
> + unsigned int i;
> + char data[2];
> + char *envp[] = { data, NULL };
> +
> + if (!th_zone || !th_zone->therm_dev)
> + return;
> +
> + thermal_zone_device_update(th_zone->therm_dev);
> +
> + mutex_lock(&th_zone->therm_dev->lock);
> + /* Find the level for which trip happened */
> + for (i = 0; i < th_zone->sensor_conf->trip_data.trip_count; i++) {
> + if (th_zone->therm_dev->last_temperature <
> + th_zone->sensor_conf->trip_data.trip_val[i] * 1000)
> + break;
> + }
> +
> + if (th_zone->mode == THERMAL_DEVICE_ENABLED) {
> + if (i > 0)
> + th_zone->therm_dev->polling_delay = ACTIVE_INTERVAL;
> + else
> + th_zone->therm_dev->polling_delay = IDLE_INTERVAL;
> + }
> +
> + sprintf(data, "%u", i);
yikes, if `i' exceeds 9, we have a stack scribble. Please review this
and at least use snprintf(... sizeof(data)) to prevent accidents.
> + kobject_uevent_env(&th_zone->therm_dev->device.kobj, KOBJ_CHANGE, envp);
> + mutex_unlock(&th_zone->therm_dev->lock);
> +}
> +
>
> ...
>
> +/* Get trip temperature callback functions for thermal zone */
> +static int exynos_get_trip_temp(struct thermal_zone_device *thermal, int
> trip,
> + unsigned long *temp)
> +{
> + if (trip < 0 || trip > 2)
I don't know what `trip' does and I don't know the meaning of the
values 0, 1 and 2. Documentation, please.
> + return -EINVAL;
> +
> + *temp = th_zone->sensor_conf->trip_data.trip_val[trip];
> + /* convert the temperature into millicelsius */
> + *temp = *temp * 1000;
> +
> + return 0;
> +}
> +
>
> ...
>
> +/* Bind callback functions for thermal zone */
> +static int exynos_bind(struct thermal_zone_device *thermal,
> + struct thermal_cooling_device *cdev)
> +{
> + int ret = 0;
> +
> + /* if the cooling device is the one from exynos4 bind it */
> + if (cdev != th_zone->cool_dev[0])
> + return 0;
> +
> + if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
> + pr_err("error binding cooling dev inst 0\n");
> + return -EINVAL;
> + }
> + if (thermal_zone_bind_cooling_device(thermal, 1, cdev)) {
> + pr_err("error binding cooling dev inst 1\n");
> + ret = -EINVAL;
> + goto error_bind1;
> + }
There can never be more than two instances?
> + return ret;
> +error_bind1:
> + thermal_zone_unbind_cooling_device(thermal, 0, cdev);
> + return ret;
> +}
> +
>
> ...
>
> +/* Get temperature callback functions for thermal zone */
> +static int exynos_get_temp(struct thermal_zone_device *thermal,
> + unsigned long *temp)
> +{
> + void *data;
> +
> + if (!th_zone->sensor_conf) {
> + pr_info("Temperature sensor not initialised\n");
> + return -EINVAL;
> + }
> + data = th_zone->sensor_conf->private_data;
> + *temp = th_zone->sensor_conf->read_temperature(data);
> + /* convert the temperature into millicelsius */
> + *temp = *temp * 1000;
> + return 0;
> +}
> +
> +/* Operation callback functions for thermal zone */
> +static struct thermal_zone_device_ops exynos_dev_ops = {
Can it be const? That sometimes saves space, as the table doesn't need
to be moved into writeable storage at runtime.
> + .bind = exynos_bind,
> + .unbind = exynos_unbind,
> + .get_temp = exynos_get_temp,
> + .get_mode = exynos_get_mode,
> + .set_mode = exynos_set_mode,
> + .get_trip_type = exynos_get_trip_type,
> + .get_trip_temp = exynos_get_trip_temp,
> + .get_crit_temp = exynos_get_crit_temp,
> +};
> +
> +/* Register with the in-kernel thermal management */
> +static int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
> +{
> + int ret, count, tab_size;
> + struct freq_clip_table *tab_ptr;
> +
> + if (!sensor_conf || !sensor_conf->read_temperature) {
> + pr_err("Temperature sensor not initialised\n");
> + return -EINVAL;
> + }
> +
> + th_zone = kzalloc(sizeof(struct exynos_thermal_zone), GFP_KERNEL);
> + if (!th_zone) {
> + ret = -ENOMEM;
> + goto err_unregister;
This seems wrong? If we need to call exynos_unregister_thermal() on
this error path then we needed to call it on the predecing one?
Perhaps?
> + }
> +
> + th_zone->sensor_conf = sensor_conf;
> +
> + tab_ptr = (struct freq_clip_table *)sensor_conf->co
Re: [PATCH v3 3/6] hwmon: exynos4: Move thermal sensor driver to driver/thermal directory
On Tue, 8 May 2012 21:48:15 +0530 Amit Daniel Kachhap wrote: > This movement is needed because the hwmon entries and corresponding > sysfs interface is a duplicate of utilities already provided by > driver/thermal/thermal_sys.c. The goal is to place it in thermal folder > and add necessary functions to use the in-kernel thermal interfaces. > > Signed-off-by: Amit Daniel Kachhap > Signed-off-by: Donggeun Kim > --- > Documentation/hwmon/exynos4_tmu | 81 > Documentation/thermal/exynos_thermal | 52 +++ > drivers/hwmon/Kconfig| 10 - > drivers/hwmon/Makefile |1 - > drivers/hwmon/exynos4_tmu.c | 514 > -- Please cc the hwmon people? Jean Delvare , Guenter Roeck , lm-sens...@lm-sensors.org > drivers/thermal/Kconfig |9 + > drivers/thermal/Makefile |1 + > drivers/thermal/exynos_thermal.c | 409 > include/linux/platform_data/exynos4_tmu.h| 83 > include/linux/platform_data/exynos_thermal.h | 83 -- 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 2/6] thermal: Add generic cpufreq cooling implementation
On Tue, 8 May 2012 21:48:14 +0530
Amit Daniel Kachhap wrote:
> This patch adds support for generic cpu thermal cooling low level
> implementations using frequency scaling up/down based on the registration
> parameters. Different cpu related cooling devices can be registered by the
> user and the binding of these cooling devices to the corresponding
> trip points can be easily done as the registration APIs return the
> cooling device pointer. The user of these APIs are responsible for
> passing clipping frequency . The drivers can also register to recieve
> notification about any cooling action called. Even the driver can effect
> the cooling action by modifying the default data such as freq_clip_max if
> needed.
>
>
> ...
>
> +struct cpufreq_cooling_device {
> + int id;
> + struct thermal_cooling_device *cool_dev;
> + struct freq_clip_table *tab_ptr;
> + unsigned int tab_size;
> + unsigned int cpufreq_state;
> + const struct cpumask *allowed_cpus;
> + struct list_head node;
> +};
It would be nice to document the fields. Especially id, tab_size,
cpufreq_state and node. For `node' we should describe the locking for
the list, and describe which list_head anchors this list.
> +static LIST_HEAD(cooling_cpufreq_list);
> +static DEFINE_MUTEX(cooling_cpufreq_lock);
> +static DEFINE_IDR(cpufreq_idr);
> +static DEFINE_PER_CPU(unsigned int, max_policy_freq);
> +static struct freq_clip_table *notify_table;
> +static int notify_state;
> +static BLOCKING_NOTIFIER_HEAD(cputherm_state_notifier_list);
> +
> +static int get_idr(struct idr *idr, struct mutex *lock, int *id)
> +{
> + int err;
> +again:
> + if (unlikely(idr_pre_get(idr, GFP_KERNEL) == 0))
> + return -ENOMEM;
> +
> + if (lock)
> + mutex_lock(lock);
The test for NULL `lock' is unneeded. In fact the `lock' argument
could be removed altogether - just use cooling_cpufreq_lock directly.
> + err = idr_get_new(idr, NULL, id);
> + if (lock)
> + mutex_unlock(lock);
> + if (unlikely(err == -EAGAIN))
> + goto again;
> + else if (unlikely(err))
> + return err;
> +
> + *id = *id & MAX_ID_MASK;
> + return 0;
> +}
> +
> +static void release_idr(struct idr *idr, struct mutex *lock, int id)
> +{
> + if (lock)
> + mutex_lock(lock);
Ditto.
> + idr_remove(idr, id);
> + if (lock)
> + mutex_unlock(lock);
> +}
> +
>
> ...
>
> +
> +/*Below codes defines functions to be used for cpufreq as cooling device*/
> +static bool is_cpufreq_valid(int cpu)
> +{
> + struct cpufreq_policy policy;
> + return !cpufreq_get_policy(&policy, cpu) ? true : false;
Can use
return !cpufreq_get_policy(&policy, cpu);
> +}
> +
> +static int cpufreq_apply_cooling(struct cpufreq_cooling_device
> *cpufreq_device,
> + unsigned long cooling_state)
> +{
> + unsigned int event, cpuid;
> + struct freq_clip_table *th_table;
> +
> + if (cooling_state > cpufreq_device->tab_size)
> + return -EINVAL;
> +
> + cpufreq_device->cpufreq_state = cooling_state;
> +
> + /*cpufreq thermal notifier uses this cpufreq device pointer*/
This code looks like it was written by two people.
/* One who does this */
/*And one who does this*/
The first one was right. Please go through all the comments in all the
patches and get the layout consistent?
> + notify_state = cooling_state;
> +
> + if (notify_state > 0) {
> + th_table = &(cpufreq_device->tab_ptr[cooling_state - 1]);
> + memcpy(notify_table, th_table, sizeof(struct freq_clip_table));
> + event = CPUFREQ_COOLING_TYPE;
> + blocking_notifier_call_chain(&cputherm_state_notifier_list,
> + event, notify_table);
> + }
> +
> + for_each_cpu(cpuid, cpufreq_device->allowed_cpus) {
> + if (is_cpufreq_valid(cpuid))
> + cpufreq_update_policy(cpuid);
> + }
> +
> + notify_state = -1;
> +
> + return 0;
> +}
> +
> +static int cpufreq_thermal_notifier(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct cpufreq_policy *policy = data;
> + unsigned long max_freq = 0;
> +
> + if ((event != CPUFREQ_ADJUST) || (notify_state == -1))
Please document `notify_state', at its definition site. This reader
doesn't know what "notify_state == -1" *means*.
> + return 0;
> +
> + if (notify_state > 0) {
> + max_freq = notify_table->freq_clip_max;
> +
> + if (per_cpu(max_policy_freq, policy->cpu) == 0)
> + per_cpu(max_policy_freq, policy->cpu) = policy->max;
> + } else {
> + if (per_cpu(max_policy_freq, policy->cpu) != 0) {
> + max_freq = per_cpu(max_policy_freq, policy->cpu);
> + per_cpu(max_policy_freq, polic
Re: [PATCH v3 0/6] thermal: exynos: Add kernel thermal support for exynos platform
On Tue, 8 May 2012 21:48:12 +0530 Amit Daniel Kachhap wrote: > This patchset introduces a new generic cooling device based on cpufreq that > can be used on non-ACPI platforms. As a proof of concept, we have drivers for > the following platforms using this mechanism now: > > * TI OMAP (git://git.linaro.org/people/amitdanielk/linux.git > omap4460_thermal) > * Samsung Exynos (Exynos4 and Exynos5) in the current patchset. > * Freescale i.MX (git://git.linaro.org/people/amitdanielk/linux.git > imx6q_thermal) > > These patches have been reviewed by Rui Zhang > (https://lkml.org/lkml/2012/4/9/448) But we don't have explicit Reviewed-by:s for the changelogs? > who seems to agree with them in principle, but I haven't had any luck getting > them > merged, perhaps a lack of maintainer bandwidth. > > ACPI platforms currently have such a mechanism but it is wrapped in ACPI'isms > that we don't have on ARM platforms. If this is accepted, I'm proposing to > convert over the ACPI thermal driver to use this common code too. -- 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 1/2] backlight: lcd: add driver for raster-type lcd's with gpio controlled panel reset
> Cc: rpur...@rpsys.net, florianschandi...@gmx.de,
> devicetree-disc...@lists.ozlabs.org, linux-fb...@vger.kernel.org,
> linux-samsung-soc@vger.kernel.org, grant.lik...@secretlab.ca,
> rob.herr...@calxeda.com, kgene@samsung.com, jg1@samsung.com,
> broo...@opensource.wolfsonmicro.com, kyungmin.p...@samsung.com,
> augulis.dar...@gmail.com, ben-li...@fluff.org, l...@metafoo.de,
> patc...@linaro.org
Poor me. When someone sends a patch like this, I need to go and hunt
down everyone's real names to nicely add them to the changelog's Cc:
list. I prefer that you do this ;)
You can add Cc:'s to the changelog yourself, of course. Often that
works out better than having me try to work out who might be
interested in the patch.
On Mon, 26 Mar 2012 14:16:39 +0530
Thomas Abraham wrote:
> Add a lcd panel driver for simple raster-type lcd's which uses a gpio
> controlled panel reset. The driver controls the nRESET line of the panel
> using a gpio connected from the host system. The Vcc supply to the panel
> is (optionally) controlled using a voltage regulator. This driver excludes
> support for lcd panels that use a serial command interface or direct
> memory mapped IO interface.
>
>
> ...
>
> +struct lcd_pwrctrl {
> + struct device *dev;
> + struct lcd_device *lcd;
> + struct lcd_pwrctrl_data *pdata;
> + struct regulator*regulator;
> + unsigned intpower;
> + boolsuspended;
> + boolpwr_en;
Generally kernel code will avoid these unpronounceable abbreviations.
So we do
pwr -> power
en -> enable
ctrl -> control
etc. It results in longer identifiers, but the code is more readable
and, more importantly, more *rememberable*.
> +};
> +
> +static int lcd_pwrctrl_get_power(struct lcd_device *lcd)
> +{
> + struct lcd_pwrctrl *lp = lcd_get_data(lcd);
> + return lp->power;
> +}
> +
> +static int lcd_pwrctrl_set_power(struct lcd_device *lcd, int power)
See, shouldn't that have been lcd_pwrctrl_set_pwr?
If we avoid the abbreviations, such issues do not arise.
> +{
> + struct lcd_pwrctrl *lp = lcd_get_data(lcd);
> + struct lcd_pwrctrl_data *pd = lp->pdata;
> + bool lcd_enable;
> + int lcd_reset, ret = 0;
> +
> + lcd_enable = (power == FB_BLANK_POWERDOWN || lp->suspended) ? 0 : 1;
This isn't how to use `bool'. We can use
lcd_enable = (power == FB_BLANK_POWERDOWN) || lp->suspended;
> + lcd_reset = (pd->invert) ? !lcd_enable : lcd_enable;
> +
> + if (lp->pwr_en == lcd_enable)
> + return 0;
> +
> + if (!IS_ERR_OR_NULL(lp->regulator)) {
> + if (lcd_enable) {
> + if (regulator_enable(lp->regulator)) {
> + dev_info(lp->dev, "regulator enable failed\n");
> + ret = -EPERM;
> + }
> + } else {
> + if (regulator_disable(lp->regulator)) {
> + dev_info(lp->dev, "regulator disable failed\n");
> + ret = -EPERM;
> + }
> + }
> + }
> +
> + gpio_direction_output(lp->pdata->gpio, lcd_reset);
> + lp->power = power;
> + lp->pwr_en = lcd_enable;
Again, this could have been any of
lp->[power|pwr] = [power|pwr];
lp->[power|pwr]_[en|enable] = lcd_[en|enable];
zillions of combinations! If we just avoid the abbreviations, there is
only one combination.
> + return ret;
> +}
> +
>
> ...
>
> +static int __devinit lcd_pwrctrl_probe(struct platform_device *pdev)
> +{
> + struct lcd_pwrctrl *lp;
> + struct lcd_pwrctrl_data *pdata = pdev->dev.platform_data;
> + struct device *dev = &pdev->dev;
> + int err;
> +
> +#ifdef CONFIG_OF
> + if (dev->of_node) {
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata) {
> + dev_err(dev, "memory allocation for pdata failed\n");
> + return -ENOMEM;
> + }
> + lcd_pwrctrl_parse_dt(dev, pdata);
> + }
> +#endif
> +
> + if (!pdata) {
> + dev_err(dev, "platform data not available\n");
> + return -EINVAL;
> + }
> +
> + lp = devm_kzalloc(dev, sizeof(struct lcd_pwrctrl), GFP_KERNEL);
Nit: I prefer sizeof(*lp) here, so I don't have to scroll back and
check the type of lp.
> + if (!lp) {
> + dev_err(dev, "memory allocation failed for private data\n");
> + return -ENOMEM;
> + }
> +
> + err = gpio_request(pdata->gpio, "LCD-nRESET");
> + if (err) {
> + dev_err(dev, "gpio [%d] request failed\n", pdata->gpio);
> + return err;
> + }
> +
>
> ...
>
The code looks OK to me, but I do think the naming decisions should be
revisited, please.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vg
Re: [PATCH v5 2/2] sdhci-s3c: Add support no internal clock divider in host controller
On Tue, 12 Oct 2010 03:59:46 +0100 Chris Ball wrote: > Hi Kyungmin, > > On Tue, Oct 12, 2010 at 11:53:35AM +0900, Kyungmin Park wrote: > > Hi Chris, > > > > we make a conclusion use this patch. Can you merge it for 2.6.37? > > > > Acked-by: Kyungmin Park > > I don't feel that I can, because Ben Dooks is listed as the maintainer > for this driver and the patch does not have his ACK. > > I don't know what to do about this; I've already tried asking Ben > whether he intends to continue maintaining sdhci-s3c, with no reply. > People get busy and take vacations and change roles, etc. Sometimes, the best you can do is to make everyone aware of what's going on and then simply proceed on your own. -- 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: [rtc-linux] RE: [PATCH 5/5] rtc: rtc-s3c: Fix debug message format on RTC
On Fri, 08 Oct 2010 08:16:44 +0900 Kukjin Kim wrote: > Updated patch 1/5 has been tested on S3C24XX and its working fine. > Its remained issue is that Ben was wondering its working on S3C24XX. > > And about the patch 4/5, I did address comments from Ben Dooks and Wan > ZongShun. ok... > So could you please handle this series? > If any problems, please kindly let me know. > > They are since commit cb655d0f3d57c23db51b981648e452988c0223f9: > Linux 2.6.36-rc7 (2010-10-06 13:39:52 -0700) > > are available in the git repository at: > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > next-rtc-s3c Can you please email them all out in the old-fashioned way? Because a) I don't manage patches with git and b) that way people will actually look at the code. Thanks. -- 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] sdhci-s3c: fix NULL ptr access in sdhci_s3c_remove
On Thu, 23 Sep 2010 16:22:05 +0200
Marek Szyprowski wrote:
> If not all clocks has been defined in platform data, driver will cause
> a null pointer dereference when it is being removed. This patch fixes
> this issue.
>
> Signed-off-by: Marek Szyprowski
> Signed-off-by: Kyungmin Park
> ---
> drivers/mmc/host/sdhci-s3c.c |6 --
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 71ad416..757d92c 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -481,8 +481,10 @@ static int __devexit sdhci_s3c_remove(struct
> platform_device *pdev)
> sdhci_remove_host(host, 1);
>
> for (ptr = 0; ptr < 3; ptr++) {
> - clk_disable(sc->clk_bus[ptr]);
> - clk_put(sc->clk_bus[ptr]);
> + if (sc->clk_bus[ptr]) {
> + clk_disable(sc->clk_bus[ptr]);
> + clk_put(sc->clk_bus[ptr]);
> + }
> }
> clk_disable(sc->clk_io);
> clk_put(sc->clk_io);
The patch applies OK to 2.6.35. Is the bug present there as well? If
so, should we fix it in earlier kernels? If so then the way in which
we indicate this is by adding
Cc:
to the changelog.
--
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] sdhci-s3c: fix incorrect spinlock usage after merge
On Tue, 21 Sep 2010 02:17:11 +0100 Chris Ball wrote: > On Mon, Sep 20, 2010 at 03:03:42PM +0200, Marek Szyprowski wrote: > > In the commit f522886e202a34a2191dd5d471b3c4d46410a9a0 a merge conflict > > in the sdhci-s3c driver been fixed. However the fix used incorrect > > spinlock operation - it cause a race with sdhci interrupt service. The > > correct way to solve it is to use spin_lock_irqsave/irqrestore() calls. > > Thanks, applied to mmc-next with: > Signed-off-by: Chris Ball > > Andrew, not sure how best to get this upstream -- do you want to send > this up to Linus (for -rc5) via your queue in -mm? Ordinarily I'd expect you to send it to Linus. I tend to merge important-looking bugfixes in my tree in case maintainers lose them (happens regularly). I'll then start spamming the maintainer with the patch. If the maintainer remains dead then I might merge it myself, or I might tag it for -stable backporting and try again in the next kernel. If the maintainer can't be bothered setting up a pull request then he can just send an acked-by and explicitly ask me to merge it up and I'll then add it to my next approximately-weekly Linus patchbombing. -- 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: [rtc-linux] RE: [PATCH 5/5] rtc: rtc-s3c: Fix debug message format on RTC
On Fri, 10 Sep 2010 20:38:39 +0900 Kukjin Kim wrote: > Wan ZongShun wrote: > > > > 2010/9/9 Kukjin Kim : > > > Ben Dooks wrote: > > >> > > >> On 07/09/10 06:29, Kukjin Kim wrote: > > >> > This patch fixes debug message format on rtc-s3c. > > >> > > > >> > Signed-off-by: Kukjin Kim > > >> > Cc: Ben Dooks > > >> > --- > > >> > drivers/rtc/rtc-s3c.c | 18 +- > > >> > 1 files changed, 9 insertions(+), 9 deletions(-) > > >> > > > >> > > >> how long has this lot been broken for? > > >> > > > Haha...we don't know :-) > > > > > >> Acked-by: Ben Dooks > > > > > > Thanks for your ack...so... > > > > > > Hi Alessandro Zummo, > > > Is it possible to send this to upstream through your tree? > > > Or which tree is better to care this. > > > > > > Please get help from Andrew. > > > Ok ;-) > > Hello Andrew, > > Could you please handle following items which have been got Ben's ack? > > http://lists.infradead.org/pipermail/linux-arm-kernel/2010-September/025387.html > http://lists.infradead.org/pipermail/linux-arm-kernel/2010-September/025388.html > http://lists.infradead.org/pipermail/linux-arm-kernel/2010-September/025391.html > > And it's available in the git repository at: >git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > next-rtc-s3c > > If any problems, please let me know. > I'd prefer to handle the whole series rather than cherrypick a subset of it. Patch 1/5 has a question from Ben which hasn't been responded to yet. Patch 4/5 is due to be updated. So please let's fix those things up, resend the entire series and cc me on the emails? -- 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: 'error: 'SDHCI_QUIRK_NO_HISPD_BIT' undeclared' and 'undefined reference to `sdhci_card_detect''
On Fri, 13 Aug 2010 10:13:04 +0200 Marek Szyprowski wrote: > Hello, > > On Friday, August 13, 2010 9:32 AM Kukjin Kim wrote: > > > This is just information about Samsung sdmmc stuff building error now. > > > > I found Marek's 'sdhci-s3c: enable SDHCI_QUIRK_NO_HISPD_BIT quirk' in the > > Linus' tree. > > (commit ID: a1d5646005af1247d6ae78434bb4db15b07a07b2) > > > > But not defined the quirk yet...so following build error happened with > > s3c6400_defconfig in Linus' latest. > > > > drivers/mmc/host/sdhci-s3c.c: In function 'sdhci_s3c_probe': > > drivers/mmc/host/sdhci-s3c.c:400: error: 'SDHCI_QUIRK_NO_HISPD_BIT' > > undeclared (first use in this function) > > drivers/mmc/host/sdhci-s3c.c:400: error: (Each undeclared identifier is > > reported only once > > drivers/mmc/host/sdhci-s3c.c:400: error: for each function it appears in.) > > make[4]: *** [drivers/mmc/host/sdhci-s3c.o] Error 1 > > make[3]: *** [drivers/mmc/host] Error 2 > > make[2]: *** [drivers/mmc] Error 2 > > make[1]: *** [drivers] Error 2 > > > > Kyungmin Park's below patch can solve this and it is in mmotm now. > > (commit ID: 2935b9e7fcc4bea3751b8d039b383b2036a7d36d) > > > > But I think, to update quirk definition should being in Marek's patch for > > avoiding build error. > > Of course, I'm not sure whether the commit order changed. > > Anyway, in this case, will be solved after merging mm tree. > > > > > > And second case is same. > > > > Marek's 'sdhci-s3c: add support for new card detection methods' cause > > following build error. > > (commit ID:17866e14f3a4f219e94f1374ece7226479418ff8) > > > > drivers/built-in.o: In function `sdhci_s3c_notify_change': > > /home/kgene/linux/linux-2.6-mainline-dev/drivers/mmc/host/sdhci-s3c.c:255: > > undefined reference to `sdhci_card_detect' > > make[1]: *** [.tmp_vmlinux1] Error 1 > > make: *** [sub-make] Error 2 > > > > And Andrew's patch(b567e5dd5a34c184e5642100e752cb87e064bb97) can solve this. > > (of course this needs another patches...) > > > > Anyway... > > Marek, in future please make sure your patch has no building problem before > > submitting. > > (or it can help to add some kind of dependency note in your patch) > > My patches submited to Andrew Morton had the description and a note that they > have been prepared especially for his tree, taking into account all patches > that are already there (mainly a tasklets to threaded irq conversion). For me > it was quite obvious that they will be merged after all other sdhci changes > from that tree. I have no idea why the order of the patches has been reversed > and my sdhci patches has been submitted to Linus before the other sdhci > changes. We're talking about these: s5pc110-sdhci-s3c-can-override-host-capabilities.patch s5pc110-sdhci-s3c-support-on-s5pc110.patch sdhci-add-no-hi-speed-bit-quirk-support.patch I have a note that these await an ack from Ben Dooks so I held them back for a closer look after -rc1. I was unaware of this dependency. Ben, are you OK with those patches? http://userweb.kernel.org/~akpm/mmotm/broken-out/s5pc110-sdhci-s3c-can-override-host-capabilities.patch http://userweb.kernel.org/~akpm/mmotm/broken-out/s5pc110-sdhci-s3c-support-on-s5pc110.patch http://userweb.kernel.org/~akpm/mmotm/broken-out/sdhci-add-no-hi-speed-bit-quirk-support.patch Thanks. -- 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] Samsung: sdhci-s3c: add support for new card detection methods
On Thu, 22 Jul 2010 10:25:44 +0200 Marek Szyprowski wrote: > Hello, > > On Thursday, July 22, 2010 1:12 AM > > > On Fri, 16 Jul 2010 08:24:26 +0200 > > Marek Szyprowski wrote: > > > > > > > arch/arm/mach-s3c64xx/setup-sdhci-gpio.c | 14 +--- > > > > > arch/arm/mach-s5pc100/setup-sdhci-gpio.c | 21 ++ > > - > > > > > arch/arm/mach-s5pv210/setup-sdhci-gpio.c | 22 +++--- > > -- > > > > > arch/arm/plat-samsung/dev-hsmmc.c |5 > > > > > arch/arm/plat-samsung/dev-hsmmc1.c |5 > > > > > arch/arm/plat-samsung/dev-hsmmc2.c |5 > > > > > arch/arm/plat-samsung/dev-hsmmc3.c |5 > > > > > arch/arm/plat-samsung/include/plat/sdhci.h | 29 > > > > > > > > > 8 files changed, 90 insertions(+), 16 deletions(-) > > > > > > > > This is quite confusing. You've already sent a patch called > > > > "sdhci-s3c: add support for new card detection methods". It had the > > > > same changelog as this patch and the same title, but the two patches > > > > are utterly different! > > > > > > The real patch has been split into two for easier merging: 1. the driver > > > part and 2. samsung platform related part. The part which patch belongs > > > to is indicated in the last line of the change log. > > > > > > I'm really confused how to submit properly a patch that requires changes > > > to both the driver (which is merged by the proper driver maintainer's > > > sub-tree) and the platform (which should go through platform maintainer's > > > tree). > > > > Make the relationship very very clear in the changelog. Send both > > patches to both maintainers. Ask that one of them merge both patches > > and that the other ack both patches. > > Ok. Thanks for the hint. > > > Anyway, I've forgotten what's happening here. I appear to be sitting on > > > > sdhci-s3c-add-support-for-the-non-standard-minimal-clock-value.patch > > This one is independent from the card-detection patches and can be applied > directly onto linux-next kernel tree. > > > and > > sdhci-s3c-add-support-for-new-card-detection-methods.patch > > > > the latter of which is below. > > > > Am I missing something? Is > > sdhci-s3c-add-support-for-new-card-detection-methods.patch up to date? > > > > Ok, I will repost these 2 patches under new names to avoid further > confusion. Currently there exists two > "sdhci-s3c-add-support-for-new-card-detection-methods.patch", both of them > are required. So the second(?) patch "[PATCH v4] Samsung: add new card detection methods in s3c-sdhci driver (platform part)" has already been applied to linux-next by Kukjin Kim, along with modifications which we have not been shown. But the first patch (sdhci-s3c-add-support-for-the-non-standard-minimal-clock-value.patch) has not been applied to linux-next by anoyne and the third patch "[PATCH v4] sdhci-s3c: add support for new card detection methods (driver part)" has not been applied either. This is a complete mess. What happens if I apply and merge sdhci-s3c-add-support-for-the-non-standard-minimal-clock-value.patch and sdhci-s3c-add-support-for-new-card-detection-methods-driver-part.patch? I have no idea. I think I'll just drop all of it. Please sort all of this out then let me know what you want me to do. -- 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] Samsung: sdhci-s3c: add support for new card detection methods
On Fri, 16 Jul 2010 08:24:26 +0200
Marek Szyprowski wrote:
> > > arch/arm/mach-s3c64xx/setup-sdhci-gpio.c | 14 +---
> > > arch/arm/mach-s5pc100/setup-sdhci-gpio.c | 21 ++-
> > > arch/arm/mach-s5pv210/setup-sdhci-gpio.c | 22 +++-
> > > arch/arm/plat-samsung/dev-hsmmc.c |5
> > > arch/arm/plat-samsung/dev-hsmmc1.c |5
> > > arch/arm/plat-samsung/dev-hsmmc2.c |5
> > > arch/arm/plat-samsung/dev-hsmmc3.c |5
> > > arch/arm/plat-samsung/include/plat/sdhci.h | 29
> >
> > > 8 files changed, 90 insertions(+), 16 deletions(-)
> >
> > This is quite confusing. You've already sent a patch called
> > "sdhci-s3c: add support for new card detection methods". It had the
> > same changelog as this patch and the same title, but the two patches
> > are utterly different!
>
> The real patch has been split into two for easier merging: 1. the driver
> part and 2. samsung platform related part. The part which patch belongs
> to is indicated in the last line of the change log.
>
> I'm really confused how to submit properly a patch that requires changes
> to both the driver (which is merged by the proper driver maintainer's
> sub-tree) and the platform (which should go through platform maintainer's
> tree).
Make the relationship very very clear in the changelog. Send both
patches to both maintainers. Ask that one of them merge both patches
and that the other ack both patches.
Anyway, I've forgotten what's happening here. I appear to be sitting on
sdhci-s3c-add-support-for-the-non-standard-minimal-clock-value.patch
and
sdhci-s3c-add-support-for-new-card-detection-methods.patch
the latter of which is below.
Am I missing something? Is
sdhci-s3c-add-support-for-new-card-detection-methods.patch up to date?
From: Marek Szyprowski
On some Samsung SoCs not all SDHCI controllers have card detect (CD) line.
For some embedded designs it is not even needed, because ususally the
device (like SDIO flash memory or wifi controller) is permanently wired to
the controller. There are also systems which have a card detect line
connected to some of the external interrupt lines or the presence of the
card depends on some other actions (like enabling a power regulator).
This patch adds support for all these cases. The following card detection
methods are possible:
1. internal sdhci host card detect line
2. external event
3. external gpio interrupt
4. no card detect line, controller will poll for the card
5. no card detect line, card is permanently wired to the controller
(once detected host won't poll it any more)
By default, all existing code would use method #1, what is compatible with
the previous version of the driver.
In case of external event, two callbacks must be provided in platdata:
ext_cd_init and ext_cd_cleanup. Both of them get a callback to a function
that notifies the s3c-sdhci host contoller as their argument. That
callback function should be called from the even dispatcher to let host
notice the card insertion/removal.
In case of external gpio interrupt, a gpio pin number must be provided in
platdata (ext_cd_gpio parameter), as well as the information about the
polarity of that gpio pin (ext_cd_gpio_invert). By default
(ext_cd_gpio_invert == 0) gpio value 0 means 'card has been removed', but
this can be changed to 'card has been removed' when ext_cd_gpio_invert ==
1.
This patch adds changes to sdhci-s3c driver.
Signed-off-by: Marek Szyprowski
Signed-off-by: Kyungmin Park
Signed-off-by: Andrew Morton
---
drivers/mmc/host/sdhci-s3c.c | 75 +
1 file changed, 75 insertions(+)
diff -puN
drivers/mmc/host/sdhci-s3c.c~sdhci-s3c-add-support-for-new-card-detection-methods
drivers/mmc/host/sdhci-s3c.c
---
a/drivers/mmc/host/sdhci-s3c.c~sdhci-s3c-add-support-for-new-card-detection-methods
+++ a/drivers/mmc/host/sdhci-s3c.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
@@ -44,6 +45,8 @@ struct sdhci_s3c {
struct resource *ioarea;
struct s3c_sdhci_platdata *pdata;
unsigned intcur_clk;
+ int ext_cd_irq;
+ int ext_cd_gpio;
struct clk *clk_io;
struct clk *clk_bus[MAX_BUS_CLK];
@@ -242,6 +245,39 @@ static struct sdhci_ops sdhci_s3c_ops =
.get_min_clock = sdhci_s3c_get_min_clock,
};
+static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
+{
+ struct sdhci_host *host;
+ unsigned long flags;
+
+ host = platform_get_drvdata(dev);
+ if (host) {
+ spin_lock_irqsave(
Re: [PATCH v3] Samsung: sdhci-s3c: add support for new card detection methods
On Thu, 15 Jul 2010 10:15:21 +0200 Marek Szyprowski wrote: > On some Samsung SoCs not all SDHCI controllers have card detect (CD) > line. For some embedded designs it is not even needed, because ususally > the device (like SDIO flash memory or wifi controller) is permanently > wired to the controller. There are also systems which have a card detect > line connected to some of the external interrupt lines or the presence > of the card depends on some other actions (like enabling a power > regulator). > > This patch adds support for all these cases. The following card > detection methods are possible: > > 1. internal sdhci host card detect line > 2. external event > 3. external gpio interrupt > 4. no card detect line, controller will poll for the card > 5. no card detect line, card is permanently wired to the controller > (once detected host won't poll it any more) > > By default, all existing code would use method #1, what is compatible > with the previous version of the driver. > > In case of external event, two callbacks must be provided in platdata: > ext_cd_init and ext_cd_cleanup. Both of them get a callback to a > function that notifies the s3c-sdhci host contoller as their argument. > That callback function should be called from the even dispatcher to let > host notice the card insertion/removal. > > In case of external gpio interrupt, a gpio pin number must be provided > in platdata (ext_cd_gpio parameter), as well as the information about > the polarity of that gpio pin (ext_cd_gpio_invert). By default > (ext_cd_gpio_invert == 0) gpio value 0 means 'card has been removed', > but this can be changed to 'card has been removed' when > ext_cd_gpio_invert == 1. > > This patch adds all required changes to platform support code. > > Signed-off-by: Marek Szyprowski > Signed-off-by: Kyungmin Park > --- > > This a resend of the previous patch. Relevant patches for sdhci-s3c > driver has been posted in a separate patch series for easier merging, > please refer to the "[PATCH] SDHCI-S3C fixes and enhancements (driver > specific code)" thread. They have been already taken by Andrew Morton > into his patch tracking tree. > > I hope these changes to platform specific code gets merged soon, so the > changes to the driver can be merged as well. > > The patch has been prepared for the following git tree: > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git for-next > > Changes since V2: > - added support for HSMMC3 device > > Changes since V1: > - added support for gpio external interrupt card based detect method > directly to sdhci-s3c driver > - removed s3c64xx compilation fix patch from the series > > Best regards > -- > Marek Szyprowski > Samsung Poland R&D Center > > --- > > arch/arm/mach-s3c64xx/setup-sdhci-gpio.c | 14 +--- > arch/arm/mach-s5pc100/setup-sdhci-gpio.c | 21 ++- > arch/arm/mach-s5pv210/setup-sdhci-gpio.c | 22 +++- > arch/arm/plat-samsung/dev-hsmmc.c |5 > arch/arm/plat-samsung/dev-hsmmc1.c |5 > arch/arm/plat-samsung/dev-hsmmc2.c |5 > arch/arm/plat-samsung/dev-hsmmc3.c |5 > arch/arm/plat-samsung/include/plat/sdhci.h | 29 > > 8 files changed, 90 insertions(+), 16 deletions(-) This is quite confusing. You've already sent a patch called "sdhci-s3c: add support for new card detection methods". It had the same changelog as this patch and the same title, but the two patches are utterly different! I suggest that you resend this patch under an appropriate title and with a more specific changelog, please. -- 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] s3c-fb: Automatically calculate pixel clock when none is given
On Thu, 15 Jul 2010 21:23:15 +0200 Maurus Cuelenaere wrote: > Op 15-07-10 20:52, Andrew Morton schreef: > > On Tue, 13 Jul 2010 02:05:45 +0200 > > Maurus Cuelenaere wrote: > > > >> This patch adds a simple algorithm which calculates the pixel clock based > >> on the > >> video mode parameters. This is only done when no pixel clock is supplied > >> through > >> the platform data. > >> > > So.. we are to assume that there's a platform which doesn't provide > > the pixel clock data? Adding some information about that in the > > changelog would have been useful. > > Not exactly, this allows you to omit the pixel clock data and thus sharing the > "algo" used for calculating it; which is done in another patch (not submitted > to > linux-fbdev, see [1]). > > [1] - http://www.spinics.net/lists/linux-samsung-soc/msg02086.html > That means that there's no reason to merge this patch! So I merged "arm: samsung: remove pixclock from several boards" as well ;) -- 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] s3c-fb: Automatically calculate pixel clock when none is given
On Tue, 13 Jul 2010 02:05:45 +0200 Maurus Cuelenaere wrote: > This patch adds a simple algorithm which calculates the pixel clock based on > the > video mode parameters. This is only done when no pixel clock is supplied > through > the platform data. > So.. we are to assume that there's a platform which doesn't provide the pixel clock data? Adding some information about that in the changelog would have been useful. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] sdhci-s3c: add missing remove function
On Mon, 12 Jul 2010 13:04:09 +0200
Marek Szyprowski wrote:
> > > static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
> > > {
> > > + struct sdhci_host *host = platform_get_drvdata(pdev);
> > > + struct sdhci_s3c *sc = sdhci_priv(host);
> > > + int ptr;
> > > +
> > > + sdhci_remove_host(host, 1);
> > > +
> > > + for (ptr = 0; ptr < 3; ptr++) {
> > > + clk_disable(sc->clk_bus[ptr]);
> > > + clk_put(sc->clk_bus[ptr]);
> > > + }
> > > + clk_disable(sc->clk_io);
> > > + clk_put(sc->clk_io);
> > > +
> > > + iounmap(host->ioaddr);
> > > + release_resource(sc->ioarea);
> > > + kfree(sc->ioarea);
> > > +
> > > + sdhci_free_host(host);
> > > + platform_set_drvdata(pdev, NULL);
> > > +
> > > return 0;
> > > }
> >
> > This looks like it fixes a pretty serious omission. What happens if
> > the user rmmods this driver on a 2.6.34 kernel?
>
> System will crash sooner or later once the memory with the code of the
> s3c-sdhci.ko module is reused for something else. I really have no idea
> how the lack of remove function went unnoticed into the mainline code.
>
> > Because I have a suspicion that this fix should be backported into
> > 2.6.34.x?
>
> Right, this is really a good idea. This patch applies cleanly onto
> v2.6.34 too.
OK, thanks, I added the Cc: to the changelog and
moved the patch into my for-2.6.35 queue.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] sdhci-s3c: add missing remove function
On Wed, 16 Jun 2010 08:49:54 +0200
Marek Szyprowski wrote:
> Signed-off-by: Marek Szyprowski
> Signed-off-by: Kyungmin Park
> ---
> drivers/mmc/host/sdhci-s3c.c | 20
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index af21792..ad30f07 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -365,6 +365,26 @@ static int __devinit sdhci_s3c_probe(struct
> platform_device *pdev)
>
> static int __devexit sdhci_s3c_remove(struct platform_device *pdev)
> {
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct sdhci_s3c *sc = sdhci_priv(host);
> + int ptr;
> +
> + sdhci_remove_host(host, 1);
> +
> + for (ptr = 0; ptr < 3; ptr++) {
> + clk_disable(sc->clk_bus[ptr]);
> + clk_put(sc->clk_bus[ptr]);
> + }
> + clk_disable(sc->clk_io);
> + clk_put(sc->clk_io);
> +
> + iounmap(host->ioaddr);
> + release_resource(sc->ioarea);
> + kfree(sc->ioarea);
> +
> + sdhci_free_host(host);
> + platform_set_drvdata(pdev, NULL);
> +
> return 0;
> }
This looks like it fixes a pretty serious omission. What happens if
the user rmmods this driver on a 2.6.34 kernel?
Because I have a suspicion that this fix should be backported into
2.6.34.x?
--
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/3] sdhci-s3c: add support for new card detection methods
On Wed, 16 Jun 2010 08:49:56 +0200
Marek Szyprowski wrote:
> On some Samsung SoCs not all SDHCI controllers have card detect (CD)
> line. For some embedded designs it is not even needed, because ususally
> the device (like SDIO flash memory or wifi controller) is permanently
> wired to the controller. There are also systems which have a card detect
> line connected to some of the external interrupt lines or the presence
> of the card depends on some other actions (like enabling a power
> regulator).
>
> This patch adds support for all these cases. The following card
> detection methods are possible:
>
> 1. internal sdhci host card detect line
> 2. external event
> 3. external gpio interrupt
> 4. no card detect line, controller will poll for the card
> 5. no card detect line, card is permanently wired to the controller
> (once detected host won't poll it any more)
>
> By default, all existing code would use method #1, what is compatible
> with the previous version of the driver.
>
> In case of external event, two callbacks must be provided in platdata:
> ext_cd_init and ext_cd_cleanup. Both of them get a callback to a
> function that notifies the s3c-sdhci host contoller as their argument.
> That callback function should be called from the even dispatcher to let
> host notice the card insertion/removal.
>
> In case of external gpio interrupt, a gpio pin number must be provided
> in platdata (ext_cd_gpio parameter), as well as the information about
> the polarity of that gpio pin (ext_cd_gpio_invert). By default
> (ext_cd_gpio_invert == 0) gpio value 0 means 'card has been removed',
> but this can be changed to 'card has been removed' when
> ext_cd_gpio_invert == 1.
>
> This patch adds changes to sdhci-s3c driver.
>
> ...
>
> +static void sdhci_s3c_notify_change(struct platform_device *dev, int state)
> +{
> + struct sdhci_host *host;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + host = platform_get_drvdata(dev);
> + if (host) {
> + if (state) {
> + dev_dbg(&dev->dev, "card inserted.\n");
> + host->flags &= ~SDHCI_DEVICE_DEAD;
> + host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> + tasklet_schedule(&host->card_tasklet);
> + } else {
> + dev_dbg(&dev->dev, "card removed.\n");
> + host->flags |= SDHCI_DEVICE_DEAD;
> + host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION;
> + tasklet_schedule(&host->card_tasklet);
> + }
> + }
> + local_irq_restore(flags);
> +}
What's the local_irq_save() there for?
Presumably it is for local-cpu-only protection of some data. But which
data is it there to protect?
It doesn't provide protection on SMP systems and if I'm guessing
correctly about why it was added, it would be much better to use
spin_lock_irq[save]() here. That sets a better example, it means the
code has a hope of working correctly on SMP systems and will devolve to
local_irq_save() on UP kernels anyway.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] sdhci-s3c: add support for the non standard minimal clock value
On Wed, 16 Jun 2010 08:49:55 +0200 Marek Szyprowski wrote: > S3C SDHCI host controller can change the source for generating mmc clock. > By default host bus clock is used, what causes some problems on machines > with 133MHz bus, because the SDHCI divider cannot be as high get proper > clock value for identification mode. This is not a problem for the > controller, because it can generate lower frequencies from other clock > sources. This patch adds a new quirk to SDHCI driver to calculate the > minimal supported clock frequency. > > This fixes the flood of the following warnings on Samsung S5PV210 SoCs: > mmc0: Minimum clock frequency too high for identification mode > > Signed-off-by: Marek Szyprowski > Signed-off-by: Kyungmin Park > --- > drivers/mmc/host/sdhci-of-esdhc.c |1 + > drivers/mmc/host/sdhci-s3c.c | 29 + > drivers/mmc/host/sdhci.c |2 +- > drivers/mmc/host/sdhci.h |2 ++ > 4 files changed, 33 insertions(+), 1 deletions(-) This patch doesn't know about Anton's sdhci-pltfm-add-support-for-cns3xxx-soc-devices.patch. Please check my fixup: --- a/drivers/mmc/host/sdhci-cns3xxx.c~sdhci-s3c-add-support-for-the-non-standard-minimal-clock-value-fix +++ a/drivers/mmc/host/sdhci-cns3xxx.c @@ -93,5 +93,6 @@ struct sdhci_pltfm_data sdhci_cns3xxx_pd SDHCI_QUIRK_INVERTED_WRITE_PROTECT | SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | SDHCI_QUIRK_BROKEN_TIMEOUT_VAL | + SDHCI_QUIRK_NONSTANDARD_MINCLOCK | SDHCI_QUIRK_NONSTANDARD_CLOCK, }; -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] RTC: s3c: Initialize s3c_rtc_cpu_type before using it
On Thu, 27 May 2010 23:28:47 +0200 Maurus Cuelenaere wrote: > Make sure s3c_rtc_cpu_type is initialised _before_ it's used in an if() check. > > This was probably caused due to a merge mistake. Nope, I went back to your original email: @@ -471,7 +509,12 @@ static int __devinit s3c_rtc_probe(struct platform_device *pdev) goto err_nortc; } - rtc->max_user_freq = 128; + if (s3c_rtc_cpu_type == TYPE_S3C64XX) + rtc->max_user_freq = 32768; + else + rtc->max_user_freq = 128; + + s3c_rtc_cpu_type = platform_get_device_id(pdev)->driver_data; -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] RTC: s3c: Initialize driver data before using it
On Thu, 27 May 2010 23:28:45 +0200
Maurus Cuelenaere wrote:
> s3c_rtc_setfreq() uses the platform driver data to derive struct rtc_device,
> so make sure drvdata is set _before_ s3c_rtc_setfreq() is called.
>
> Signed-off-by: Maurus Cuelenaere
> ---
> drivers/rtc/rtc-s3c.c |5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index e5972b2..6adebf3 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -495,8 +495,6 @@ static int __devinit s3c_rtc_probe(struct platform_device
> *pdev)
> pr_debug("s3c2410_rtc: RTCCON=%02x\n",
>readb(s3c_rtc_base + S3C2410_RTCCON));
>
> - s3c_rtc_setfreq(&pdev->dev, 1);
> -
> device_init_wakeup(&pdev->dev, 1);
>
> /* register RTC and exit */
> @@ -518,6 +516,9 @@ static int __devinit s3c_rtc_probe(struct platform_device
> *pdev)
> s3c_rtc_cpu_type = platform_get_device_id(pdev)->driver_data;
>
> platform_set_drvdata(pdev, rtc);
> +
> + s3c_rtc_setfreq(&pdev->dev, 1);
> +
> return 0;
>
> err_nortc:
This one I tagged for a -stable backport.
--
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] sdhci: add adma descriptor set call
On Thu, 28 Jan 2010 05:29:56 + Ben Dooks wrote: > The code to write the ADMA descriptor into memory is repeated several > times throughout sdhci_adma_table_pre, and thus should be moved into a > common function. This will also be useful if the patch to make the write > more efficient is accepted. > > ... > > @@ -499,15 +496,9 @@ static int sdhci_adma_table_pre(struct sdhci_host *host, > /* > * Add a terminating entry. > */ > - desc[7] = 0; > - desc[6] = 0; > - desc[5] = 0; > - desc[4] = 0; > - > - desc[3] = 0; > - desc[2] = 0; > - desc[1] = 0x00; > - desc[0] = 0x03; /* nop, end, valid */ > + > + /* nop, end, valid */ > + sdhci_set_adma_desc(desc, 0, 0, 0x3); > } What kernel are you patching? Current mainline is different from the above. -- 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] gpiolib: add gpio_lookup_chip() to get chip information for gpio
On Fri, 8 Jan 2010 06:32:28 + Ben Dooks wrote:
> Add a call to get the 'struct gpio_chip' pointer for a given gpio, so
> that core implementations which want to use gpiolib gpio numbering for
> things like mux configuration can get back to the gpio_chip that they
> registered without having to have their own list of gpio.
>
> This is especially useful for the Samsung S3C64XX series where the GPIO
> bank sizes can vary from 32 down to 3, making it difficult to store an
> array to convert a number to chip.
>
> ...
>
> +/**
> + * gpio_lookup_chip - return the chip for a given gpio
> + * @gpio: The GPIO number to lookup
> + *
> + * Returns NULL if the GPIO chip is not valid or there is no chip registered
> + * for that GPIO. This call is available for core code to turn a GPIO number
> + * into a chip so that further information can be looked up.
> + *
> + * This call makes no guarantees about the actuall gpio_chip's state, or
> + * whether the @gpio itself is requested.
> + */
> +struct gpio_chip *gpio_lookup_chip(unsigned gpio)
> +{
> + struct gpio_chip *chip;
> + unsigned long flags;
> +
> + if (!gpio_is_valid(gpio))
> + return NULL;
> +
> + spin_lock_irqsave(&gpio_lock, flags);
> + chip = gpio_to_chip(gpio);
> + spin_unlock_irqrestore(&gpio_lock, flags);
> +
> + return chip;
> +}
> +
The locking looks fishy. What's the point in locking the array lookup and
then returning the thing which was looked up after dropping the lock?
Should this function have been exported to modules?
Please cc myself and David Brownell on gpio patches.
--
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
