From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 01/12] mmc: mmc_spi: Support CD/RO GPIOs Date: Fri, 26 Jul 2013 02:04:34 +0200 Message-ID: <3057612.zXvbbdByqU@avalon> References: <1374794808-21890-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <1374794808-21890-2-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <51F1B896.8020005@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: <51F1B896.8020005@gmail.com> Sender: linux-sh-owner@vger.kernel.org To: Ryan Mallon Cc: Laurent Pinchart , linux-arm-kernel@lists.infradead.org, linux-sh@vger.kernel.org, linux-mmc@vger.kernel.org, linux-spi@vger.kernel.org, Hartley Sweeten , Chris Ball , Guennadi Liakhovetski , Ian Molton List-Id: linux-mmc@vger.kernel.org Hi Ryan, On Friday 26 July 2013 09:45:26 Ryan Mallon wrote: > On 26/07/13 09:26, Laurent Pinchart wrote: > > Add support for passing CD/RO GPIO numbers directly to the mmc_spi > > driver instead of relying solely on board code callbacks to retrieve the > > CD/RO signals values. > > > > Signed-off-by: Laurent Pinchart > > > > --- > > > > drivers/mmc/host/mmc_spi.c | 32 +++++++++++++++++++++--------- > > drivers/mmc/host/of_mmc_spi.c | 46 ++++++++++---------------------------- > > include/linux/spi/mmc_spi.h | 11 +++++++++++ > > 3 files changed, 46 insertions(+), 43 deletions(-) > > > > diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c > > index 74145d1..4e83908 100644 > > --- a/drivers/mmc/host/mmc_spi.c > > +++ b/drivers/mmc/host/mmc_spi.c > > @@ -36,6 +36,7 @@ > > > > #include > > #include /* for R1_SPI_* bit values */ > > +#include > > #include > > #include > > > > @@ -1278,11 +1279,8 @@ static int mmc_spi_get_ro(struct mmc_host *mmc) > > if (host->pdata && host->pdata->get_ro) > > return !!host->pdata->get_ro(mmc->parent); > > > > - /* > > - * Board doesn't support read only detection; let the mmc core > > - * decide what to do. > > - */ > > - return -ENOSYS; > > + else > > + return mmc_gpio_get_ro(mmc); > > Why not just have the board file assign mmc_gpio_get_ro as > host->pdata->get_ro? This would eliminate the need for the > MMC_SPI_USE_RO_GPIO flag. Because the idea is to get rid of board callbacks and rely on proper kernel abstraction layers such as the GPIO subsystem. This will be especially important when adding device tree support to the mmc_spi driver. > Also, if host->pdata->get_ro is not set then this will assume > mmc_gpio_get_ro is valid, even if MMC_SPI_USE_RO_GPIO is not set. I'm > guessing it will end up returning -ENOSYS, but they way the code reads is > that if the host doesn't have get_ro function set, then it is must be a > gpio. mmc_gpio_get_ro() will indeed return -ENOSYS when the MMC_SPI_USE_RO_GPIO flag isn't set, as the mmc_spi driver will not call mmc_gpio_request_ro() in that case. This patch hides the -ENOSYS value from the mmc_spi_get_ro() and mmc_spi_get_cd() functions, perhaps making them slightly confusing, but patch 06/12 then gets rid of those functions, so I don't think it's a bit issue. > Same applies for the other callbacks. -- Regards, Laurent Pinchart