From patchwork Tue Jan 20 15:57:09 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: roel kluin X-Patchwork-Id: 19514 X-Patchwork-Delegate: benh@kernel.crashing.org Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from ozlabs.org (localhost [127.0.0.1]) by ozlabs.org (Postfix) with ESMTP id 4DD8CDDF7A for ; Wed, 21 Jan 2009 02:58:31 +1100 (EST) X-Original-To: linuxppc-dev@ozlabs.org Delivered-To: linuxppc-dev@ozlabs.org Received: from ug-out-1314.google.com (ug-out-1314.google.com [66.249.92.174]) by ozlabs.org (Postfix) with ESMTP id 43446DDF0D; Wed, 21 Jan 2009 02:57:12 +1100 (EST) Received: by ug-out-1314.google.com with SMTP id 17so310392ugm.14 for ; Tue, 20 Jan 2009 07:57:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=t4IO0cbgoCE+tPqInL+kpzkOECdJzQQ1SrLf5Ov8ChU=; b=NmRMcGonyIZjs/OLKwM0E8E50gWr0jSLu2NkXxN7mwi1b17YRyFPDoF2IMXkrQLPKc uLClPm9BID64zE8rLq+HL4N61uEgqhdMKfFjY5YKg3was4BsDsdRk6+T+BntHpfgnc4C pxhTi9pIr6KK/Fbj+sz5S8uYW24L5cLiaI7JI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; b=A+Do0HCucf454RIjGtfS4eJi70FXHoPeLE1jo5gn/q2JKdhKIkebzAsmXVtTnLCSfx b0axhjfjH5euC0BmL/0vTOYttdpRP4PCqiAt/nolEDglh8fB6tYKLQFcZ/D4y5xBwWH2 rcrWlidq+TDirwlkeX2tw2Be2cRt9d3D8wgdw= Received: by 10.66.242.15 with SMTP id p15mr37806ugh.54.1232467030034; Tue, 20 Jan 2009 07:57:10 -0800 (PST) Received: from ?192.168.1.115? (d133062.upc-d.chello.nl [213.46.133.62]) by mx.google.com with ESMTPS id s1sm6272961uge.27.2009.01.20.07.57.09 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 20 Jan 2009 07:57:09 -0800 (PST) Message-ID: <4975F455.8020004@gmail.com> Date: Tue, 20 Jan 2009 16:57:09 +0100 From: Roel Kluin User-Agent: Thunderbird 2.0.0.18 (X11/20081105) MIME-Version: 1.0 To: Geert Uytterhoeven Subject: Re: [PATCH] PS3 ps3av_set_video_mode() make id signed References: <4972456C.1050301@gmail.com> In-Reply-To: Cc: linuxppc-dev@ozlabs.org, cbe-oss-dev@ozlabs.org X-BeenThere: linuxppc-dev@ozlabs.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@ozlabs.org Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@ozlabs.org >> make id signed so a negative id will get noticed > > Thanks for noticing! It looks OK, except... > 1. You forgot to update the forward declaration of ps3av_set_video_mode() in > arch/powerpc/include/asm/ps3av.h (did you compile this succesfully?) fixed that (and no, sorry, I didn't compile test it, I have to focus a bit on a biotechnology exam, maybe later). > 2. You forgot to change `u32 id' in ps3av_probe(). ok, changed it. but I am not sure whether the last two hunks are ok. Should we error out like this or just let the negative value be assigned to ps3av->ps3av_mode? If not, is there more cleanup required? > Can you please make these changes, too? Thx again! No problem. --------------------->8-------------------------8<------------------------ Make id signed so a negative id will get noticed. Error out if ps3av_auto_videomode() fails. Signed-off-by: Roel Kluin --- arch/powerpc/include/asm/ps3av.h | 2 +- drivers/ps3/ps3av.c | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/include/asm/ps3av.h b/arch/powerpc/include/asm/ps3av.h index cd24ac1..0427b0b 100644 --- a/arch/powerpc/include/asm/ps3av.h +++ b/arch/powerpc/include/asm/ps3av.h @@ -730,7 +730,7 @@ extern int ps3av_cmd_av_get_hw_conf(struct ps3av_pkt_av_get_hw_conf *); extern int ps3av_cmd_video_get_monitor_info(struct ps3av_pkt_av_get_monitor_info *, u32); -extern int ps3av_set_video_mode(u32); +extern int ps3av_set_video_mode(int); extern int ps3av_set_audio_mode(u32, u32, u32, u32, u32); extern int ps3av_get_auto_mode(void); extern int ps3av_get_mode(void); diff --git a/drivers/ps3/ps3av.c b/drivers/ps3/ps3av.c index 5324978..235e87f 100644 --- a/drivers/ps3/ps3av.c +++ b/drivers/ps3/ps3av.c @@ -838,7 +838,7 @@ static int ps3av_get_hw_conf(struct ps3av *ps3av) } /* set mode using id */ -int ps3av_set_video_mode(u32 id) +int ps3av_set_video_mode(int id) { int size; u32 option; @@ -940,7 +940,7 @@ EXPORT_SYMBOL_GPL(ps3av_audio_mute); static int ps3av_probe(struct ps3_system_bus_device *dev) { int res; - u32 id; + int id; dev_dbg(&dev->core, " -> %s:%d\n", __func__, __LINE__); dev_dbg(&dev->core, " timeout=%d\n", timeout); @@ -962,8 +962,10 @@ static int ps3av_probe(struct ps3_system_bus_device *dev) init_completion(&ps3av->done); complete(&ps3av->done); ps3av->wq = create_singlethread_workqueue("ps3avd"); - if (!ps3av->wq) + if (!ps3av->wq) { + res = -ENOMEM; goto fail; + } switch (ps3_os_area_get_av_multi_out()) { case PS3_PARAM_AV_MULTI_OUT_NTSC: @@ -994,6 +996,12 @@ static int ps3av_probe(struct ps3_system_bus_device *dev) safe_mode = 1; #endif /* CONFIG_FB */ id = ps3av_auto_videomode(&ps3av->av_hw_conf); + if (id < 0) { + printk(KERN_ERR "%s: invalid id :%d\n", __func__, id); + res = -EINVAL; + goto fail; + } + safe_mode = 0; mutex_lock(&ps3av->mutex); @@ -1007,7 +1015,7 @@ static int ps3av_probe(struct ps3_system_bus_device *dev) fail: kfree(ps3av); ps3av = NULL; - return -ENOMEM; + return res; } static int ps3av_remove(struct ps3_system_bus_device *dev)