From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 794DAC636D7 for ; Tue, 21 Feb 2023 18:42:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Z2YaDuEHh22/ZSGuqZzXBQnmdKRM2zN9BmUAo4rRhQc=; b=qrvu0Tp6oy2g35 mBnA+MBU6u1ZiUVtGhrIcUg14Q5gMMp1wDY1zGaBt15vtF3MgEK21al3Exb7AlNrfkWYTAVNuZ5G2 6eOdsDGxiOwgzv5VgF04MVup6i6/8KCb34UFhk8g4/Benf+m9LgLTlElxzx74wcIpQuOZxgnHAlsp z5dHnXsrcw38ychawn5EHerMgmAEEIfBMOWUGe2WCI8aVm6fZtreTRqxGNY/WbXsrvrpx7Cf1r82I KwUUTYfMlGEjudtKwMD/s10QdC9uD2B6d5JpwM89+eoXllRALmiRcCGDs6U2kWuD9o8hrt3cfnq1E We7QRksHEhzqeZUrJoKg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pUXa8-009PmD-1T; Tue, 21 Feb 2023 18:41:40 +0000 Received: from mail-pj1-x1032.google.com ([2607:f8b0:4864:20::1032]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pUXa4-009Pky-S4 for linux-arm-kernel@lists.infradead.org; Tue, 21 Feb 2023 18:41:38 +0000 Received: by mail-pj1-x1032.google.com with SMTP id co23-20020a17090afe9700b002341fadc370so5915725pjb.1 for ; Tue, 21 Feb 2023 10:41:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=mVehFQdVGJrMqMTa40u/3esY8x1eGiBhNFRgFgu1gT0=; b=VeCbtMXTBLi3rfPegHSsToMWXrB0ha+ZadIsZuM5awOB0FejJlJg2luB49095xGC7A zaqHdyOAr8mfOBsjSRQ/NV4tIwXEJW8kEDinjeLxJ3vcH470avDlLY0rjnnABEGUiIPQ OVNhWEgbpTS6zUhITxLKFhmuPGKZ//Dx8bHp91yrqXKwUuOcrygOs1GutaJX0Lpzz7nB Y67V4yXk3cS/pZ1effVlCmCg1zsj94aDKIJqKRryVp6kka3s6owDtcKmRJKgMJRXCuPe QPLzud/YkGTRT8arVRUMNYGWAJXoeXjLVfvrVM6uSWCggv85cCrSOuqzdN6p3rUoqpF/ FnqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=mVehFQdVGJrMqMTa40u/3esY8x1eGiBhNFRgFgu1gT0=; b=ssAAeGyCBsX9cad6dpNIFQYJimkK8pF84PbgFmXce7MaOdDrr39g39BuBVtvwvn2vh L9aHtSFFzr+80sw0TpKXaqa+4fwvPtgZfnMsY2qVQoMhFI0rmdYeO6xFWr1/Ls2/vw3M YplAuakAkmnlARhcDtqs/uykctbG8uBeQcAg/9v8c18+DDlf47TqDGQ2gRCTRVjiFIxk Wj8hKJ0QuQY+bj6/F5EhjxapzMRV09K/zZ8SFqu7hPHxBf/KVAKcUYSfrvoWcd6cDv7w c5rr4Chp5BgVPzcGjVfNx/EEEjPw4H+/J4GARhhyyqcIvXiTRB2mCnSuwFzNj2LQaAsj KQsQ== X-Gm-Message-State: AO0yUKW4/jQB5A9e2hD52p8Lju9wKgVAi12xhK9eZ5k96y9zFUBA0Ktz ow5LLMZXWdUZZU7eqbb5WARsjuHGWWU= X-Google-Smtp-Source: AK7set/J+D0m43p+fEQA2RrEHmqQYCaRXTzyT8E1jNvkeze10AzS/rnFLCbb9SfffmoTwlEK20YcBA== X-Received: by 2002:a17:90a:ff15:b0:233:ee50:d291 with SMTP id ce21-20020a17090aff1500b00233ee50d291mr5791003pjb.22.1677004894875; Tue, 21 Feb 2023 10:41:34 -0800 (PST) Received: from google.com ([2620:15c:9d:2:30ba:90e9:a2f2:4b50]) by smtp.gmail.com with ESMTPSA id t5-20020a17090b018500b0023530b1e4a0sm3211354pjs.2.2023.02.21.10.41.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Feb 2023 10:41:34 -0800 (PST) Date: Tue, 21 Feb 2023 10:41:31 -0800 From: Dmitry Torokhov To: Balamanikandan Gunasundar Subject: Re: [PATCH v4 1/2] mmc: atmel-mci: Convert to gpio descriptors Message-ID: References: <20230221094207.102006-1-balamanikandan.gunasundar@microchip.com> <20230221094207.102006-2-balamanikandan.gunasundar@microchip.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230221094207.102006-2-balamanikandan.gunasundar@microchip.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230221_104136_939217_8C15BCB2 X-CRM114-Status: GOOD ( 31.63 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alexandre.belloni@bootlin.com, linus.walleij@linaro.org, linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, ludovic.desroches@microchip.com, hari.prasathge@microchip.com, ulf.hansson@linaro.org, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Balamanikandan, On Tue, Feb 21, 2023 at 03:12:06PM +0530, Balamanikandan Gunasundar wrote: > Replace the legacy GPIO APIs with gpio descriptor consumer interface. > To maintain backward compatibility, we rely on the "cd-inverted" > property to manage the invertion flag instead of GPIO property. > > Signed-off-by: Balamanikandan Gunasundar > --- > drivers/mmc/host/atmel-mci.c | 77 +++++++++++++++++------------------- > include/linux/atmel-mci.h | 4 +- > 2 files changed, 39 insertions(+), 42 deletions(-) > > diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c > index dd18440a90c5..ddad5a929634 100644 > --- a/drivers/mmc/host/atmel-mci.c > +++ b/drivers/mmc/host/atmel-mci.c > @@ -11,7 +11,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -19,7 +18,8 @@ > #include > #include > #include > -#include > +#include > +#include > #include > #include > #include > @@ -388,8 +388,8 @@ struct atmel_mci_slot { > #define ATMCI_CARD_NEED_INIT 1 > #define ATMCI_SHUTDOWN 2 > > - int detect_pin; > - int wp_pin; > + struct gpio_desc *detect_pin; > + struct gpio_desc *wp_pin; > bool detect_is_active_high; > > struct timer_list detect_timer; > @@ -637,7 +637,10 @@ atmci_of_init(struct platform_device *pdev) > pdata->slot[slot_id].bus_width = 1; > > pdata->slot[slot_id].detect_pin = > - of_get_named_gpio(cnp, "cd-gpios", 0); > + devm_fwnode_gpiod_get(&pdev->dev, of_fwnode_handle(cnp), > + "cd", GPIOD_IN, "cd-gpios"); > + if (IS_ERR(pdata->slot[slot_id].detect_pin)) > + pdata->slot[slot_id].detect_pin = NULL; Since we are doing the conversion, may I suggest: error = PTR_ERR_OR_ZERO(pdata->slot[slot_id].detect_pin); if (error) { if (error != -ENOENT) retrun ERR_PTR(error); pdata->slot[slot_id].detect_pin = NULL; } so that we do not ignore valid errors, and handle deferrals. I also wonde if it would not be better to move this (acquiring GPIOs) out of the OF case and into the main probe. > > pdata->slot[slot_id].detect_is_active_high = > of_property_read_bool(cnp, "cd-inverted"); I believe it would be better to have this quirk be handled in gpiolib-of.c where you would set up polarity of gpiod properly, and then in this driver you would not need to bother with using raw API and handle polarity yourself. > @@ -646,7 +649,10 @@ atmci_of_init(struct platform_device *pdev) > of_property_read_bool(cnp, "non-removable"); > > pdata->slot[slot_id].wp_pin = > - of_get_named_gpio(cnp, "wp-gpios", 0); > + devm_fwnode_gpiod_get(&pdev->dev, of_fwnode_handle(cnp), > + "wp", GPIOD_IN, "wp-gpios"); > + if (IS_ERR(pdata->slot[slot_id].wp_pin)) > + pdata->slot[slot_id].wp_pin = NULL; > } > > return pdata; > @@ -1509,8 +1515,8 @@ static int atmci_get_ro(struct mmc_host *mmc) > int read_only = -ENOSYS; > struct atmel_mci_slot *slot = mmc_priv(mmc); > > - if (gpio_is_valid(slot->wp_pin)) { > - read_only = gpio_get_value(slot->wp_pin); > + if (slot->wp_pin) { > + read_only = gpiod_get_value(slot->wp_pin); > dev_dbg(&mmc->class_dev, "card is %s\n", > read_only ? "read-only" : "read-write"); > } > @@ -1523,8 +1529,8 @@ static int atmci_get_cd(struct mmc_host *mmc) > int present = -ENOSYS; > struct atmel_mci_slot *slot = mmc_priv(mmc); > > - if (gpio_is_valid(slot->detect_pin)) { > - present = !(gpio_get_value(slot->detect_pin) ^ > + if (slot->detect_pin) { > + present = !(gpiod_get_raw_value(slot->detect_pin) ^ > slot->detect_is_active_high); > dev_dbg(&mmc->class_dev, "card is %spresent\n", > present ? "" : "not "); > @@ -1637,8 +1643,8 @@ static void atmci_detect_change(struct timer_list *t) > if (test_bit(ATMCI_SHUTDOWN, &slot->flags)) > return; > > - enable_irq(gpio_to_irq(slot->detect_pin)); > - present = !(gpio_get_value(slot->detect_pin) ^ > + enable_irq(gpiod_to_irq(slot->detect_pin)); > + present = !(gpiod_get_raw_value(slot->detect_pin) ^ > slot->detect_is_active_high); > present_old = test_bit(ATMCI_CARD_PRESENT, &slot->flags); > > @@ -2237,9 +2243,9 @@ static int atmci_init_slot(struct atmel_mci *host, > dev_dbg(&mmc->class_dev, > "slot[%u]: bus_width=%u, detect_pin=%d, " > "detect_is_active_high=%s, wp_pin=%d\n", > - id, slot_data->bus_width, slot_data->detect_pin, > + id, slot_data->bus_width, desc_to_gpio(slot_data->detect_pin), > slot_data->detect_is_active_high ? "true" : "false", > - slot_data->wp_pin); > + desc_to_gpio(slot_data->wp_pin)); > > mmc->ops = &atmci_ops; > mmc->f_min = DIV_ROUND_UP(host->bus_hz, 512); > @@ -2275,31 +2281,24 @@ static int atmci_init_slot(struct atmel_mci *host, > > /* Assume card is present initially */ > set_bit(ATMCI_CARD_PRESENT, &slot->flags); > - if (gpio_is_valid(slot->detect_pin)) { > - if (devm_gpio_request(&host->pdev->dev, slot->detect_pin, > - "mmc_detect")) { > - dev_dbg(&mmc->class_dev, "no detect pin available\n"); > - slot->detect_pin = -EBUSY; > - } else if (gpio_get_value(slot->detect_pin) ^ > - slot->detect_is_active_high) { > + if (slot->detect_pin) { > + if (gpiod_get_raw_value(slot->detect_pin) ^ > + slot->detect_is_active_high) { > clear_bit(ATMCI_CARD_PRESENT, &slot->flags); > } > + } else { > + dev_dbg(&mmc->class_dev, "no detect pin available\n"); > } > > - if (!gpio_is_valid(slot->detect_pin)) { > + if (!slot->detect_pin) { > if (slot_data->non_removable) > mmc->caps |= MMC_CAP_NONREMOVABLE; > else > mmc->caps |= MMC_CAP_NEEDS_POLL; > } > > - if (gpio_is_valid(slot->wp_pin)) { > - if (devm_gpio_request(&host->pdev->dev, slot->wp_pin, > - "mmc_wp")) { > - dev_dbg(&mmc->class_dev, "no WP pin available\n"); > - slot->wp_pin = -EBUSY; > - } > - } > + if (!slot->wp_pin) > + dev_dbg(&mmc->class_dev, "no WP pin available\n"); > > host->slot[id] = slot; > mmc_regulator_get_supply(mmc); > @@ -2309,18 +2308,18 @@ static int atmci_init_slot(struct atmel_mci *host, > return ret; > } > > - if (gpio_is_valid(slot->detect_pin)) { > + if (slot->detect_pin) { > timer_setup(&slot->detect_timer, atmci_detect_change, 0); > > - ret = request_irq(gpio_to_irq(slot->detect_pin), > - atmci_detect_interrupt, > - IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, > - "mmc-detect", slot); > + ret = request_irq(gpiod_to_irq(slot->detect_pin), > + atmci_detect_interrupt, > + IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, > + "mmc-detect", slot); > if (ret) { > dev_dbg(&mmc->class_dev, > "could not request IRQ %d for detect pin\n", > - gpio_to_irq(slot->detect_pin)); > - slot->detect_pin = -EBUSY; > + gpiod_to_irq(slot->detect_pin)); > + slot->detect_pin = NULL; > } > } > > @@ -2339,10 +2338,8 @@ static void atmci_cleanup_slot(struct atmel_mci_slot *slot, > > mmc_remove_host(slot->mmc); > > - if (gpio_is_valid(slot->detect_pin)) { > - int pin = slot->detect_pin; > - > - free_irq(gpio_to_irq(pin), slot); > + if (slot->detect_pin) { > + free_irq(gpiod_to_irq(slot->detect_pin), slot); > del_timer_sync(&slot->detect_timer); > } > > diff --git a/include/linux/atmel-mci.h b/include/linux/atmel-mci.h > index 1491af38cc6e..017e7d8f6126 100644 > --- a/include/linux/atmel-mci.h > +++ b/include/linux/atmel-mci.h > @@ -26,8 +26,8 @@ > */ > struct mci_slot_pdata { > unsigned int bus_width; > - int detect_pin; > - int wp_pin; > + struct gpio_desc *detect_pin; > + struct gpio_desc *wp_pin; > bool detect_is_active_high; > bool non_removable; > }; > -- > 2.25.1 > Additionally, I think you can remove handling of the static platform data as I do not believe anyone is using it in the mainline, as well as switch to generic device properties instead of using of_*() APIs. Thanks. -- Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel