Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752203Ab0LVWCO (ORCPT ); Wed, 22 Dec 2010 17:02:14 -0500 Received: from mail-bw0-f45.google.com ([209.85.214.45]:44561 "EHLO mail-bw0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751394Ab0LVWCM convert rfc822-to-8bit (ORCPT ); Wed, 22 Dec 2010 17:02:12 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=sr1hXHyUjZrC9QwR3BVDsBkLo9f4fR1hs8WgPO9fJYOs2XAZ2fSEpKfLEQ+o+j5JmN xPF+/iiyPR9WhIZ/m5xieD89sL86UdKJWclZZbznmaFVPpVW00oPZwpIm0N/fn9AKCkd MSWTEpnwvKiGGoqGC8Um6EiXFrnWWBf/GublQ= MIME-Version: 1.0 In-Reply-To: <4D12730B.2030909@bluewatersys.com> References: <20101220195423.GA14509@alchark-u3s> <4D0FC1AE.2050708@bluewatersys.com> <4D0FD747.9020700@bluewatersys.com> <4D0FE546.7050302@bluewatersys.com> <4D1011CF.9080800@bluewatersys.com> <4D110269.9030805@bluewatersys.com> <20101222211815.GA31054@alchark-u3s> <4D12730B.2030909@bluewatersys.com> Date: Thu, 23 Dec 2010 01:02:11 +0300 Message-ID: Subject: Re: [PATCH 1/6 v10] ARM: Add basic architecture support for VIA/WonderMedia 85xx SoC's From: Alexey Charkov To: Ryan Mallon Cc: Russell King - ARM Linux , linux-arm-kernel@lists.infradead.org, vt8500-wm8505-linux-kernel@googlegroups.com, Eric Miao , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , Albin Tonnerre , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4795 Lines: 134 2010/12/23 Ryan Mallon : > On 12/23/2010 10:18 AM, Alexey Charkov wrote: >> This adds support for the family of Systems-on-Chip produced initially >> by VIA and now its subsidiary WonderMedia that have recently become >> widespread in lower-end Chinese ARM-based tablets and netbooks. >> >> Support is included for both VT8500 and WM8505, selectable by a >> configuration switch at kernel build time. >> >> Included are basic machine initialization files, register and >> interrupt definitions, support for the on-chip interrupt controller, >> high-precision OS timer, GPIO lines, necessary macros for early debug, >> pulse-width-modulated outputs control, as well as platform device >> configurations for the specific drivers implemented elsewhere. >> >> Signed-off-by: Alexey Charkov >> --- >> >> Welcome the jubilee tenth revision of this patch ;-) >> >> I've tried to incorporate the suggestions by Ryan and Arnd, hope that >> there is nothing left out. There was a massive reorganization of code >> to remove less-than-obvious magic with MMIO registers and irqs being >> held in huge structs, now they are again macro definitions. Those >> macros are, however, only included in single isolated files, and >> actual values to use are chosen at runtime by calling the respective >> routines at machine initialization. There are also stylistic changes >> all around, where Ryan suggested. >> >> As a result, i8042 should again be adjusted a bit to reflect the new >> place to find respective register/irq definitions, that one will be >> sent in the respective thread shortly. >> >> Best regards, >> Alexey > > > >> --- /dev/null >> +++ b/arch/arm/mach-vt8500/devices-vt8500.c >> @@ -0,0 +1,117 @@ >> +/* linux/arch/arm/mach-vt8500/devices-vt8500.c >> + * >> + * Copyright (C) 2010 Alexey Charkov >> + * >> + * This software is licensed under the terms of the GNU General Public >> + * License version 2, as published by the Free Software Foundation, and >> + * may be copied, distributed, and modified under those terms. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the >> + * GNU General Public License for more details. >> + * >> + */ >> + >> +#include >> + >> +#include >> +#include >> +#include >> +#include "devices.h" >> + >> +static inline struct resource WMT_MMIO_RES(u32 start, u32 size) >> +{ >> +     struct resource tmp = { >> +             .flags = IORESOURCE_MEM, >> +             .start = start, >> +             .end = start + size - 1, >> +     }; >> + >> +     return tmp; >> +} > > These functions can be marked __init (though I guess they already are if > marked inline?). They should also have lower case names since they are > proper functions. > As inline functions are unrolled into the caller at compile time, and the caller is __init, I would expect their code to be freed after init as well. I could be wrong, though :) > Do these functions generate warnings about returning temporary values > off the stack? If so, they could be rewritten as: > Should those be compile-time or run-time? I did not see any. > static __init void wmt_mmio_res(struct resource *res, >                                u32 start, u32 size) > { >        res->flags = IORESOURCE_MEM; >        res->start = start; >        res->end   = start + size - 1; > } > > You could also make these functions static inline in devices.h to avoid > having to define them for each board. > Agreed. >> + >> +static inline struct resource WMT_IRQ_RES(int irq) >> +{ >> +     struct resource tmp = { >> +             .flags = IORESOURCE_IRQ, >> +             .start = irq, >> +             .end = irq, >> +     }; >> + >> +     return tmp; >> +} >> + >> +#define WMT_RES_ADD(__device, __resource, __num) \ >> +if (platform_device_add_resources(__device, __resource, __num)) \ >> +     pr_err("Failed to assign resources to __device##\n"); > > This could be written as a proper function. The resource add is unlikely > to fail. Maybe keep the warning but don't worry about printing the > device name? > There is memory allocation inside platform_device_add_resources, so probably there is scope for failure. I could add unlikely(), though. Thanks, Alexey -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/