From patchwork Fri May 6 13:06:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Lamparter X-Patchwork-Id: 9033011 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 99C64BF440 for ; Fri, 6 May 2016 13:08:09 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EFAE320389 for ; Fri, 6 May 2016 13:08:03 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1F67F2038F for ; Fri, 6 May 2016 13:08:02 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ayfSw-0000xK-Rs; Fri, 06 May 2016 13:06:46 +0000 Received: from mail-wm0-x243.google.com ([2a00:1450:400c:c09::243]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ayfSr-0000dS-6v for linux-arm-kernel@lists.infradead.org; Fri, 06 May 2016 13:06:42 +0000 Received: by mail-wm0-x243.google.com with SMTP id w143so8769005wmw.3 for ; Fri, 06 May 2016 06:06:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:to:cc:subject:date:message-id:user-agent:in-reply-to :references:mime-version:content-transfer-encoding; bh=yaUPAfBxs0gMNn3uiNKAVAsJIyHu7meEhL/v0fCcSys=; b=Idb/YXh/FGdfv+H3AXmxnTD8oKyKiaDCxQ50ZtCuK878STFCEQ5f6nbLBS/C4iZy7y ChFp5+Jk0NSPsNbiw349CFakN7+TAaBJl7QU63is8ipKvZRkoY1DUJz1XRBXTYBZFhXd bpO/n/40L1fPMp7oDFyFTst04h8aVdg523+HRSiYb7p3eoNLko8Cwiv73yx07xFEgveB 9F8b5xbAM8qPTzX/DRgEvUdZ2KuV5n478xMwNuDgzUZ1WDfu9nx0AWCKGZuYn8gvzNFP cmfBSaesrTH75GcQ9Uev+MSx6mkGyIZphp3EpR26c3WCmvHozRrT+8JRj4rNLkOY4LRu 2W6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:user-agent :in-reply-to:references:mime-version:content-transfer-encoding; bh=yaUPAfBxs0gMNn3uiNKAVAsJIyHu7meEhL/v0fCcSys=; b=IcL2H6t4TquXHXjVIA+DEyoIMjnrDoHlPFtzn1AONmbazcz4XZre6lbwuSOerNHcPW cz33IAqcidwGxCwGf59VDXzCgsWVo70eMQ/RP9mzpX/c/SgpiFXcljmCzWBjpLIt4LeL nTAn+G9ky9BC/oD78NSXmFR6POE2qCcufJ5lpSeljGB1DHpFD3MquEj8ZrQMa/jc8JDh wkE8b99Ma+GvIeEu6ynV+QcbcPZ2l2cKszyjSDDaf28rJOjn3h+9s9dZaXh+WfRlNgB8 WROLj2yPObEzcHTDaa26R0RHeH8I/0h74Ql7NZnOQli/qhXOgCRHy6NnMCvF3am70QpI 9PBA== X-Gm-Message-State: AOPr4FUrTYcjwq8z4/Ov84e0jNUfXB7YEimTpyS5En7m7pzt1lvYxdT2ycF8Cb9hpgTkZg== X-Received: by 10.194.120.135 with SMTP id lc7mr19444772wjb.34.1462539979393; Fri, 06 May 2016 06:06:19 -0700 (PDT) Received: from debian64.daheim (p5B2E6184.dip0.t-ipconnect.de. [91.46.97.132]) by smtp.googlemail.com with ESMTPSA id jp2sm14882644wjc.16.2016.05.06.06.06.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 06 May 2016 06:06:18 -0700 (PDT) Received: from localhost.daheim ([127.0.0.1] helo=debian64.localnet) by debian64.daheim with esmtps (TLS1.0:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.87) (envelope-from ) id 1ayfSO-0007Fg-FW; Fri, 06 May 2016 15:06:15 +0200 From: Christian Lamparter To: Andy Shevchenko Subject: Re: [PATCH v7 3/3] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-mmio Date: Fri, 06 May 2016 15:06:11 +0200 Message-ID: <4995974.1A7SuhXPf2@debian64> User-Agent: KMail/4.14.10 (Linux/4.6.0-rc5-wt; KDE/4.14.14; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160506_060641_707689_3DC08CA9 X-CRM114-Status: GOOD ( 25.90 ) X-Spam-Score: -2.7 (--) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree , =?ISO-8859-1?Q?=C1lvaro_Fern=E1ndez?= Rojas , Pawel Moll , Alexander Shiyan , Ian Campbell , Julien Grossholtz , Linus Walleij , "linux-kernel@vger.kernel.org" , Jonas Jensen , Martyn Welch , "linux-gpio@vger.kernel.org" , Rob Herring , Kumar Gala , Alexandre Courbot , linux-arm Mailing List Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Friday, May 06, 2016 02:53:24 PM Andy Shevchenko wrote: > On Fri, May 6, 2016 at 2:10 PM, Christian Lamparter > wrote: > > This patch integrates these GPIO drivers into gpio-mmio. > > Would be nice to repeat a list here. Ok. > > @@ -285,14 +274,6 @@ config GPIO_MM_LANTIQ > > (EBU) found on Lantiq SoCs. The gpios are output only as they are > > created by attaching a 16bit latch to the bus. > > > > -config GPIO_MOXART > > - bool "MOXART GPIO support" > > - depends on ARCH_MOXART || COMPILE_TEST > > - select GPIO_GENERIC > > - help > > - Select this option to enable GPIO driver for > > - MOXA ART SoC devices. > > - > > Doesn't it change behaviour? So, as a user of old .config I expect to > have driver enabled. How is it achieved now? Ok, I'll restore the configs and let them select GPIO_GENERIC_PLATFORM. > > --- a/drivers/gpio/gpio-mmio.c > > +++ b/drivers/gpio/gpio-mmio.c > > @@ -610,10 +610,200 @@ static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev, > > return 0; > > } > > > > +static int clps711x_parse_dt(struct platform_device *pdev, > > + struct bgpio_pdata *pdata, > > + unsigned long *flags) > > +{ > > + struct device_node *np = pdev->dev.of_node; > > + struct resource *res; > > + const char *dir_reg_name; > > + int id = np ? of_alias_get_id(np, "gpio") : pdev->id; > > + > > + if ((id < 0) || (id > 4)) > > + return -ENODEV; > > + > > > + /* PORTE is 3 lines only */ > > + pdata->ngpio = (id == 4) ? 3 : /* determined by register width */ 0; > > + > > + /* PORTD is inverted logic for direction register */ > > + dir_reg_name = (id == 3) ? "dirin" : "dirout", Actually, the , should be a ; it compiled because , is an operator. But yes, fixed. > Just a nit: possible to use switch case? I think so. I've integrated the id < 0 || id > 4 check as well. I don't know, I liked the old version more, it was shorter > > + > > + pdata->base = id * 8; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) > > + return -EINVAL; > > + if (!res->name || strcmp("dat", res->name)) > > + res->name = devm_kstrdup(&pdev->dev, "dat", GFP_KERNEL); > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > > + if (!res) > > + return -EINVAL; > > + if (!res->name || strcmp(dir_reg_name, res->name)) > > + res->name = devm_kstrdup(&pdev->dev, dir_reg_name, GFP_KERNEL); > > + > > + return 0; > > +} [...] > > +struct compat_gpio_device_data { > > + unsigned int expected_resource_size; > > + unsigned int ngpio; > > + resource_size_t register_width; > > + unsigned long flags; > > + int (*call_back)(struct platform_device *pdev, > > + struct bgpio_pdata *pdata, > > + unsigned long *flags); > > + struct resource_replacement { > > + resource_size_t start_offset; > > + const char *name; > > + } resources[5]; > > I would define magic number with a description what are those 5. Difficult, these are 5 placeholders for the named resources. There's particular order so a enum like { DAT, SET, CLR, DIRIN, DIROUT, __NUM_RES } might look nice but it has no purpose other than maybe confusing people why the entries aren't used. > > +}; > > [...] > > > +static int compat_parse_dt(struct platform_device *pdev, > > + struct bgpio_pdata *pdata, > > + unsigned long *flags) > > +{ > > + const struct device_node *node = pdev->dev.of_node; > > + const struct compat_gpio_device_data *entry; > > + const struct of_device_id *of_id; > > + struct resource *res; > > + int err; > > + > > + of_id = of_match_node(compat_gpio_devices, node); > > + if (!of_id) > > + return -ENODEV; > > + > > + entry = of_id->data; > > + if (!entry || !entry->resources[0].name) > > + return -EINVAL; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) > > + return -EINVAL; > > + > > + if (!res->name || strcmp(entry->resources[0].name, res->name)) { > > + struct resource nres[ARRAY_SIZE(entry->resources)]; > > + int i; > > unsigned int i; ? yes. I think I change it to size_t. ARRAY_SIZE uses sizeof internally. Regards, Christian diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c index f116786..f71021c 100644 --- a/drivers/gpio/gpio-mmio.c +++ b/drivers/gpio/gpio-mmio.c @@ -619,14 +619,25 @@ static int clps711x_parse_dt(struct platform_device *pdev, const char *dir_reg_name; int id = np ? of_alias_get_id(np, "gpio") : pdev->id; - if ((id < 0) || (id > 4)) + switch (id) { + case 0: + case 1: + case 2: + pdata->ngpio = 0; /* determined by register width */ + dir_reg_name = "dirout"; + break; + case 3: + pdata->ngpio = 0; /* determined by register width */ + /* PORTD is inverted logic for direction register */ + dir_reg_name = "dirin"; + break; + case 4: + pdata->ngpio = 3; /* PORTE is 3 lines only */ + dir_reg_name = "dirout"; + break; + default: return -ENODEV; - - /* PORTE is 3 lines only */ - pdata->ngpio = (id == 4) ? 3 : /* determined by register width */ 0; - - /* PORTD is inverted logic for direction register */ - dir_reg_name = (id == 3) ? "dirin" : "dirout", + } pdata->base = id * 8; ---