From: Linus Walleij <linus.walleij@linaro.org> To: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com> Cc: ludovic.desroches@microchip.com, nicolas.ferre@microchip.com, alexandre.belloni@bootlin.com, linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, hari.prasathge@microchip.com, dmitry.torokhov@gmail.com, ulf.hansson@linaro.org Subject: Re: [PATCH v3 1/2] mmc: atmel-mci: Convert to gpio descriptors Date: Thu, 29 Dec 2022 01:23:18 +0100 [thread overview] Message-ID: <CACRpkdbORVt9sFCnBFE1U206M92u4fjk9enbDJYZw7HJyAC=ng@mail.gmail.com> (raw) In-Reply-To: <20221226073908.17317-2-balamanikandan.gunasundar@microchip.com> Hi Balamanikandan, thanks for your patch! On Mon, Dec 26, 2022 at 8:39 AM Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com> 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 <balamanikandan.gunasundar@microchip.com> Overall this looks really nice! I thought about converting this driver but was a bit afraid of doing mistakes since it was a bit elaborate. Nice that you use fwnode accessors! Acked-by: Linus Walleij <linus.walleij@linaro.org> Improvement to consider (can also be a separate patch): > + if (slot->detect_pin) { > + present = !(gpiod_get_raw_value(slot->detect_pin) ^ > slot->detect_is_active_high); Normally we contain all this active low/high mess in mmc_of_parse(), but I guess this doesn't work here because the of node is different from the device :P Since the code says *explicitly* "active high" not "inverted" as the core code parses it, could you try just push this to gpiolib like we usually do, by deleting the detect_is_active_high stuff and just apply a patch like this? diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 4fff7258ee41..5979c9a75cf9 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -199,7 +199,9 @@ static void of_gpio_set_polarity_by_property(const struct device_node *np, { "fsl,imx8mm-pcie", "reset-gpio", "reset-gpio-active-high" }, { "fsl,imx8mp-pcie", "reset-gpio", "reset-gpio-active-high" }, #endif - +#if IS_ENABLED(CONFIG_MMC_ATMELMCI) + { "atmel,hsmci", "cd-gpios", "cd-inverted" }, +#endif /* * The regulator GPIO handles are specified such that the * presence or absence of "enable-active-high" solely controls (It's fine to include in your MMC patch if some GPIO maintainer like me ACK it.) Yours, Linus Walleij
WARNING: multiple messages have this Message-ID (diff)
From: Linus Walleij <linus.walleij@linaro.org> To: Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com> Cc: alexandre.belloni@bootlin.com, dmitry.torokhov@gmail.com, 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 Subject: Re: [PATCH v3 1/2] mmc: atmel-mci: Convert to gpio descriptors Date: Thu, 29 Dec 2022 01:23:18 +0100 [thread overview] Message-ID: <CACRpkdbORVt9sFCnBFE1U206M92u4fjk9enbDJYZw7HJyAC=ng@mail.gmail.com> (raw) In-Reply-To: <20221226073908.17317-2-balamanikandan.gunasundar@microchip.com> Hi Balamanikandan, thanks for your patch! On Mon, Dec 26, 2022 at 8:39 AM Balamanikandan Gunasundar <balamanikandan.gunasundar@microchip.com> 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 <balamanikandan.gunasundar@microchip.com> Overall this looks really nice! I thought about converting this driver but was a bit afraid of doing mistakes since it was a bit elaborate. Nice that you use fwnode accessors! Acked-by: Linus Walleij <linus.walleij@linaro.org> Improvement to consider (can also be a separate patch): > + if (slot->detect_pin) { > + present = !(gpiod_get_raw_value(slot->detect_pin) ^ > slot->detect_is_active_high); Normally we contain all this active low/high mess in mmc_of_parse(), but I guess this doesn't work here because the of node is different from the device :P Since the code says *explicitly* "active high" not "inverted" as the core code parses it, could you try just push this to gpiolib like we usually do, by deleting the detect_is_active_high stuff and just apply a patch like this? diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 4fff7258ee41..5979c9a75cf9 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -199,7 +199,9 @@ static void of_gpio_set_polarity_by_property(const struct device_node *np, { "fsl,imx8mm-pcie", "reset-gpio", "reset-gpio-active-high" }, { "fsl,imx8mp-pcie", "reset-gpio", "reset-gpio-active-high" }, #endif - +#if IS_ENABLED(CONFIG_MMC_ATMELMCI) + { "atmel,hsmci", "cd-gpios", "cd-inverted" }, +#endif /* * The regulator GPIO handles are specified such that the * presence or absence of "enable-active-high" solely controls (It's fine to include in your MMC patch if some GPIO maintainer like me ACK it.) Yours, Linus Walleij _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-12-29 0:23 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-12-26 7:39 [PATCH v3 0/2] mmc: atmel-mci: Convert to gpio descriptors Balamanikandan Gunasundar 2022-12-26 7:39 ` Balamanikandan Gunasundar 2022-12-26 7:39 ` [PATCH v3 1/2] " Balamanikandan Gunasundar 2022-12-26 7:39 ` Balamanikandan Gunasundar 2022-12-28 13:34 ` Ludovic.Desroches 2022-12-28 13:34 ` Ludovic.Desroches 2022-12-29 0:23 ` Linus Walleij [this message] 2022-12-29 0:23 ` Linus Walleij 2023-01-02 15:07 ` Ulf Hansson 2023-01-02 15:07 ` Ulf Hansson 2022-12-26 7:39 ` [PATCH v3 2/2] mmc: atmel-mci: move atmel MCI header file Balamanikandan Gunasundar 2022-12-26 7:39 ` Balamanikandan Gunasundar 2022-12-28 13:31 ` Ludovic.Desroches 2022-12-28 13:31 ` Ludovic.Desroches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CACRpkdbORVt9sFCnBFE1U206M92u4fjk9enbDJYZw7HJyAC=ng@mail.gmail.com' \ --to=linus.walleij@linaro.org \ --cc=alexandre.belloni@bootlin.com \ --cc=balamanikandan.gunasundar@microchip.com \ --cc=dmitry.torokhov@gmail.com \ --cc=hari.prasathge@microchip.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mmc@vger.kernel.org \ --cc=ludovic.desroches@microchip.com \ --cc=nicolas.ferre@microchip.com \ --cc=ulf.hansson@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.