From patchwork Mon Jul 2 11:42:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Linus Walleij X-Patchwork-Id: 10501199 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 433CE60325 for ; Mon, 2 Jul 2018 11:42:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 31D46287DB for ; Mon, 2 Jul 2018 11:42:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2587E287ED; Mon, 2 Jul 2018 11:42:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 99500287DB for ; Mon, 2 Jul 2018 11:42:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030657AbeGBLml (ORCPT ); Mon, 2 Jul 2018 07:42:41 -0400 Received: from mail-it0-f68.google.com ([209.85.214.68]:52788 "EHLO mail-it0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030706AbeGBLmi (ORCPT ); Mon, 2 Jul 2018 07:42:38 -0400 Received: by mail-it0-f68.google.com with SMTP id p4-v6so11680225itf.2 for ; Mon, 02 Jul 2018 04:42:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=saIqLOPvwRJWVTv55+gmP2/Kn7dzHTktH2Bg8CKQkmQ=; b=U56n9zGSINQfDgMYIA1evHoTL4tKoRhy8znCvZZ+jIogbVNkc7SEpnemQ/Oni7zUW0 Xvjox1snpDMAnCDDaiRk66h/4zx7Zyt9kz5TOQ+MonjB4Y1Z0i/YU/kFOF11C0sq2sck ueo/hMwyoza525t0q8dyrbKYLtk3QnILx1UMA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=saIqLOPvwRJWVTv55+gmP2/Kn7dzHTktH2Bg8CKQkmQ=; b=WAOXz6pyMdlx5Nd4Y/vg/SCc3AlOIjSEKTjdNWhdArztspmST4zyOiaG49LzKFwpJI pRnSu/Dv+Pa7P79HKvLNqY36Jlv/o9v2hnHADx9x927v6JEHfSp9G/yY4b7OBPkGzaSF 2b/XJ42cyt1Z0I6OeNUeJuzVtUlzgOs06GgQ3vqJloPajmFSKUAEeN8X+AwqKIR6Mv4n o6yV4/SiJpT3yLAydfSmJbWH2yiS1bCqRDgxtFpZs6pCN9o3gO4m0AjBQK4z0Ufu/56R j6ZHTuDkLaX45KRkLknHfn39A17A4nlaYArwjszb34ega+uBXU7Mp3Zk+ayRwU9LvjBG tbKw== X-Gm-Message-State: APt69E2d1hOzaRJsAf/73OglM/jkHfMyfk4SlJnZbjE2rqhNz28pTv/j /KUJJrku3DSNSPqO6czcGVYgKfXM8nmNVYa3y3x5Sw== X-Google-Smtp-Source: AAOMgpdh5IZdaWK3qYGicLwXvkDUrJdkAlSdBwE5AjE3QlWw/H1+wCIh9tlCuiEiEY6OHzZsS4BbeI/FnK3ZP1WmeNQ= X-Received: by 2002:a24:ad01:: with SMTP id c1-v6mr5240612itf.70.1530531757933; Mon, 02 Jul 2018 04:42:37 -0700 (PDT) MIME-Version: 1.0 References: <1530494867-15015-1-git-send-email-Anson.Huang@nxp.com> In-Reply-To: <1530494867-15015-1-git-send-email-Anson.Huang@nxp.com> From: Linus Walleij Date: Mon, 2 Jul 2018 13:42:26 +0200 Message-ID: Subject: Re: [PATCH V2] mmc: core: cd_label must be last entry of mmc_gpio struct To: Anson Huang Cc: Ulf Hansson , Masahiro Yamada , Adrian Hunter , evgreen@chromium.org, Shawn Lin , Fabio Estevam , linux-mmc , "linux-kernel@vger.kernel.org" , NXP Linux Team Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Jul 2, 2018 at 3:32 AM Anson Huang wrote: > commit bfd694d5e21c ("mmc: core: Add tunable delay > before detecting card after card is inserted") adds > "u32 cd_debounce_delay_ms" to the last of mmc_gpio > struct and cause "char cd_label[0]" NOT work as string > pointer of card detect label, when "cat /proc/interrupts", > the devname for card detect gpio is incorrect as below: > > 144: 0 gpio-mxc 22 Edge ▒ > 161: 0 gpio-mxc 7 Edge ▒ > > Move the cd_label field down to fix this, and drop the > zero from the array size to prevent future similar bugs, > the result is correct as below: > > 144: 0 gpio-mxc 22 Edge 2198000.mmc cd > 161: 0 gpio-mxc 7 Edge 2190000.mmc cd > > Fixes: bfd694d5e21c ("mmc: core: Add tunable delay before detecting card after card is inserted") > Signed-off-by: Anson Huang > Tested-by: Fabio Estevam Curious. > --- a/drivers/mmc/core/slot-gpio.c > +++ b/drivers/mmc/core/slot-gpio.c > @@ -27,8 +27,8 @@ struct mmc_gpio { > bool override_cd_active_level; > irqreturn_t (*cd_gpio_isr)(int irq, void *dev_id); > char *ro_label; > - char cd_label[0]; > u32 cd_debounce_delay_ms; > + char cd_label[]; Not that I am the smartest in the world when it comes to how the C compilers work this out. But isn't that equivalent to: char *cd_label; ? Look at this from drivers/mmc/core/slot-gpio.c: ctx->ro_label = ctx->cd_label + len; ctx->cd_debounce_delay_ms = 200; snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent)); snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent)); host->slot.handler_priv = ctx; host->slot.cd_irq = -EINVAL; So now that ro_label is char * that points len chars into the struct and that is all fine as the ctx is allocated like that: struct mmc_gpio *ctx = devm_kzalloc(host->parent, sizeof(*ctx) + 2 * len, GFP_KERNEL); And I've seen this being done to allocate a state with some dynamic strings after it. But I just don't like this, it seems fragile and now it broke. What about this: From facb3799f479bfd4dad99c25c9c5d4c77b14c9b0 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Mon, 2 Jul 2018 13:35:01 +0200 Subject: [PATCH] mmc: slot-gpio: Allocate GPIO labels dynamically The use of string pointers in the MMC slot GPIO context is pretty dubious, allocating some 2*len extra bytes for each label of the ro and wp pins. Tidy this up using kasprintf() with dynamic allocation of labels for these strings. Signed-off-by: Linus Walleij --- drivers/mmc/core/slot-gpio.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c index ef05e0039378..c417801b366e 100644 --- a/drivers/mmc/core/slot-gpio.c +++ b/drivers/mmc/core/slot-gpio.c @@ -27,7 +27,7 @@ struct mmc_gpio { bool override_cd_active_level; irqreturn_t (*cd_gpio_isr)(int irq, void *dev_id); char *ro_label; - char cd_label[0]; + char *cd_label; u32 cd_debounce_delay_ms; }; @@ -47,13 +47,18 @@ int mmc_gpio_alloc(struct mmc_host *host) { size_t len = strlen(dev_name(host->parent)) + 4; struct mmc_gpio *ctx = devm_kzalloc(host->parent, - sizeof(*ctx) + 2 * len, GFP_KERNEL); + sizeof(*ctx), GFP_KERNEL); if (ctx) { - ctx->ro_label = ctx->cd_label + len; ctx->cd_debounce_delay_ms = 200; - snprintf(ctx->cd_label, len, "%s cd", dev_name(host->parent)); - snprintf(ctx->ro_label, len, "%s ro", dev_name(host->parent)); + ctx->cd_label = devm_kasprintf(host->parent, GFP_KERNEL, + "%s cd", dev_name(host->parent)); + if (!ctx->cd_label) + return -ENOMEM; + ctx->ro_label = devm_kasprintf(host->parent, GFP_KERNEL, + "%s ro", dev_name(host->parent)); + if (!ctx->ro_label) + return -ENOMEM; host->slot.handler_priv = ctx; host->slot.cd_irq = -EINVAL; }