On Mon, Mar 02, 2020 at 11:11:17AM +0100, Linus Walleij wrote:
> This driver for the Intel MID never seems to have been properly
> integrated upstream: the platform data in <linux/spi/ifx_modem.h>
> is not used anywhere in the kernel and haven't been since it was
> merged into the kernel in 2010.
>
> There might be out-of-tree users, so I don't want to delete the
> driver, but I will refactor it to use GPIO descriptors, which
> means that out-of-tree users will need to adapt.
At least if we are going to remove it in the future, we may later on to revive
with already correct approach.
> There are several examples in the kernel of how to provide the
> resources necessary for using GPIO descriptors to pass in the
> GPIO lines, for the MID platform in particular, it will suffice
> to inspect the code in files like:
> arch/x86/platform/intel-mid/device_libs/platform_bt.c
>
> This refactoring transfers all GPIOs in the driver, including
> a hard-coded "PMU reset" in the driver to use GPIO descriptors
> instead.
>
> The following named GPIO descriptors need to be supplied:
> - reset
> - power
> - mrdy
> - srdy
> - rst_out
> - pmu_reset
Acked-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Russ Gorby <russ.gorby@xxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> drivers/tty/serial/ifx6x60.c | 170 ++++++++++++----------------------
> drivers/tty/serial/ifx6x60.h | 13 ++-
> include/linux/spi/ifx_modem.h | 5 -
> 3 files changed, 65 insertions(+), 123 deletions(-)
>
> diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
> index 32a0ccef9339..7d16fe41932f 100644
> --- a/drivers/tty/serial/ifx6x60.c
> +++ b/drivers/tty/serial/ifx6x60.c
> @@ -39,7 +39,7 @@
> #include <linux/fs.h>
> #include <linux/ip.h>
> #include <linux/dmapool.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/sched.h>
> #include <linux/time.h>
> #include <linux/wait.h>
> @@ -61,7 +61,6 @@
> #define IFX_SPI_HEADER_F (-2)
>
> #define PO_POST_DELAY 200
> -#define IFX_MDM_RST_PMU 4
>
> /* forward reference */
> static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev);
> @@ -81,7 +80,7 @@ static struct notifier_block ifx_modem_reboot_notifier_block = {
>
> static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev)
> {
> - gpio_set_value(IFX_MDM_RST_PMU, 1);
> + gpiod_set_value(ifx_dev->gpio.pmu_reset, 1);
> msleep(PO_POST_DELAY);
>
> return 0;
> @@ -107,7 +106,7 @@ static int ifx_modem_reboot_callback(struct notifier_block *nfb,
> */
> static inline void mrdy_set_high(struct ifx_spi_device *ifx)
> {
> - gpio_set_value(ifx->gpio.mrdy, 1);
> + gpiod_set_value(ifx->gpio.mrdy, 1);
> }
>
> /**
> @@ -117,7 +116,7 @@ static inline void mrdy_set_high(struct ifx_spi_device *ifx)
> */
> static inline void mrdy_set_low(struct ifx_spi_device *ifx)
> {
> - gpio_set_value(ifx->gpio.mrdy, 0);
> + gpiod_set_value(ifx->gpio.mrdy, 0);
> }
>
> /**
> @@ -244,7 +243,7 @@ static inline void swap_buf_32(unsigned char *buf, int len, void *end)
> */
> static void mrdy_assert(struct ifx_spi_device *ifx_dev)
> {
> - int val = gpio_get_value(ifx_dev->gpio.srdy);
> + int val = gpiod_get_value(ifx_dev->gpio.srdy);
> if (!val) {
> if (!test_and_set_bit(IFX_SPI_STATE_TIMER_PENDING,
> &ifx_dev->flags)) {
> @@ -691,7 +690,7 @@ static void ifx_spi_complete(void *ctx)
> clear_bit(IFX_SPI_STATE_IO_IN_PROGRESS, &(ifx_dev->flags));
>
> queue_length = kfifo_len(&ifx_dev->tx_fifo);
> - srdy = gpio_get_value(ifx_dev->gpio.srdy);
> + srdy = gpiod_get_value(ifx_dev->gpio.srdy);
> if (!srdy)
> ifx_spi_power_state_clear(ifx_dev, IFX_SPI_POWER_SRDY);
>
> @@ -898,7 +897,7 @@ static irqreturn_t ifx_spi_srdy_interrupt(int irq, void *dev)
> static irqreturn_t ifx_spi_reset_interrupt(int irq, void *dev)
> {
> struct ifx_spi_device *ifx_dev = dev;
> - int val = gpio_get_value(ifx_dev->gpio.reset_out);
> + int val = gpiod_get_value(ifx_dev->gpio.reset_out);
> int solreset = test_bit(MR_START, &ifx_dev->mdm_reset_state);
>
> if (val == 0) {
> @@ -954,14 +953,14 @@ static int ifx_spi_reset(struct ifx_spi_device *ifx_dev)
> * to reset properly
> */
> set_bit(MR_START, &ifx_dev->mdm_reset_state);
> - gpio_set_value(ifx_dev->gpio.po, 0);
> - gpio_set_value(ifx_dev->gpio.reset, 0);
> + gpiod_set_value(ifx_dev->gpio.po, 0);
> + gpiod_set_value(ifx_dev->gpio.reset, 0);
> msleep(25);
> - gpio_set_value(ifx_dev->gpio.reset, 1);
> + gpiod_set_value(ifx_dev->gpio.reset, 1);
> msleep(1);
> - gpio_set_value(ifx_dev->gpio.po, 1);
> + gpiod_set_value(ifx_dev->gpio.po, 1);
> msleep(1);
> - gpio_set_value(ifx_dev->gpio.po, 0);
> + gpiod_set_value(ifx_dev->gpio.po, 0);
> ret = wait_event_timeout(ifx_dev->mdm_reset_wait,
> test_bit(MR_COMPLETE,
> &ifx_dev->mdm_reset_state),
> @@ -1080,107 +1079,68 @@ static int ifx_spi_spi_probe(struct spi_device *spi)
> goto error_ret;
> }
>
> - ifx_dev->gpio.reset = pl_data->rst_pmu;
> - ifx_dev->gpio.po = pl_data->pwr_on;
> - ifx_dev->gpio.mrdy = pl_data->mrdy;
> - ifx_dev->gpio.srdy = pl_data->srdy;
> - ifx_dev->gpio.reset_out = pl_data->rst_out;
> -
> - dev_info(dev, "gpios %d, %d, %d, %d, %d",
> - ifx_dev->gpio.reset, ifx_dev->gpio.po, ifx_dev->gpio.mrdy,
> - ifx_dev->gpio.srdy, ifx_dev->gpio.reset_out);
> -
> - /* Configure gpios */
> - ret = gpio_request(ifx_dev->gpio.reset, "ifxModem");
> - if (ret < 0) {
> - dev_err(dev, "Unable to allocate GPIO%d (RESET)",
> - ifx_dev->gpio.reset);
> + ifx_dev->gpio.reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(ifx_dev->gpio.reset)) {
> + dev_err(dev, "could not obtain reset GPIO\n");
> + ret = PTR_ERR(ifx_dev->gpio.reset);
> goto error_ret;
> }
> - ret += gpio_direction_output(ifx_dev->gpio.reset, 0);
> - ret += gpio_export(ifx_dev->gpio.reset, 1);
> - if (ret) {
> - dev_err(dev, "Unable to configure GPIO%d (RESET)",
> - ifx_dev->gpio.reset);
> - ret = -EBUSY;
> - goto error_ret2;
> - }
> -
> - ret = gpio_request(ifx_dev->gpio.po, "ifxModem");
> - ret += gpio_direction_output(ifx_dev->gpio.po, 0);
> - ret += gpio_export(ifx_dev->gpio.po, 1);
> - if (ret) {
> - dev_err(dev, "Unable to configure GPIO%d (ON)",
> - ifx_dev->gpio.po);
> - ret = -EBUSY;
> - goto error_ret3;
> - }
> -
> - ret = gpio_request(ifx_dev->gpio.mrdy, "ifxModem");
> - if (ret < 0) {
> - dev_err(dev, "Unable to allocate GPIO%d (MRDY)",
> - ifx_dev->gpio.mrdy);
> - goto error_ret3;
> - }
> - ret += gpio_export(ifx_dev->gpio.mrdy, 1);
> - ret += gpio_direction_output(ifx_dev->gpio.mrdy, 0);
> - if (ret) {
> - dev_err(dev, "Unable to configure GPIO%d (MRDY)",
> - ifx_dev->gpio.mrdy);
> - ret = -EBUSY;
> - goto error_ret4;
> + gpiod_set_consumer_name(ifx_dev->gpio.reset, "ifxModem reset");
> + ifx_dev->gpio.po = devm_gpiod_get(dev, "power", GPIOD_OUT_LOW);
> + if (IS_ERR(ifx_dev->gpio.po)) {
> + dev_err(dev, "could not obtain power GPIO\n");
> + ret = PTR_ERR(ifx_dev->gpio.po);
> + goto error_ret;
> }
> -
> - ret = gpio_request(ifx_dev->gpio.srdy, "ifxModem");
> - if (ret < 0) {
> - dev_err(dev, "Unable to allocate GPIO%d (SRDY)",
> - ifx_dev->gpio.srdy);
> - ret = -EBUSY;
> - goto error_ret4;
> + gpiod_set_consumer_name(ifx_dev->gpio.po, "ifxModem power");
> + ifx_dev->gpio.mrdy = devm_gpiod_get(dev, "mrdy", GPIOD_OUT_LOW);
> + if (IS_ERR(ifx_dev->gpio.mrdy)) {
> + dev_err(dev, "could not obtain mrdy GPIO\n");
> + ret = PTR_ERR(ifx_dev->gpio.mrdy);
> + goto error_ret;
> }
> - ret += gpio_export(ifx_dev->gpio.srdy, 1);
> - ret += gpio_direction_input(ifx_dev->gpio.srdy);
> - if (ret) {
> - dev_err(dev, "Unable to configure GPIO%d (SRDY)",
> - ifx_dev->gpio.srdy);
> - ret = -EBUSY;
> - goto error_ret5;
> + gpiod_set_consumer_name(ifx_dev->gpio.mrdy, "ifxModem mrdy");
> + ifx_dev->gpio.srdy = devm_gpiod_get(dev, "srdy", GPIOD_IN);
> + if (IS_ERR(ifx_dev->gpio.srdy)) {
> + dev_err(dev, "could not obtain srdy GPIO\n");
> + ret = PTR_ERR(ifx_dev->gpio.srdy);
> + goto error_ret;
> }
> -
> - ret = gpio_request(ifx_dev->gpio.reset_out, "ifxModem");
> - if (ret < 0) {
> - dev_err(dev, "Unable to allocate GPIO%d (RESET_OUT)",
> - ifx_dev->gpio.reset_out);
> - goto error_ret5;
> + gpiod_set_consumer_name(ifx_dev->gpio.srdy, "ifxModem srdy");
> + ifx_dev->gpio.reset_out = devm_gpiod_get(dev, "rst_out", GPIOD_IN);
> + if (IS_ERR(ifx_dev->gpio.reset_out)) {
> + dev_err(dev, "could not obtain rst_out GPIO\n");
> + ret = PTR_ERR(ifx_dev->gpio.reset_out);
> + goto error_ret;
> }
> - ret += gpio_export(ifx_dev->gpio.reset_out, 1);
> - ret += gpio_direction_input(ifx_dev->gpio.reset_out);
> - if (ret) {
> - dev_err(dev, "Unable to configure GPIO%d (RESET_OUT)",
> - ifx_dev->gpio.reset_out);
> - ret = -EBUSY;
> - goto error_ret6;
> + gpiod_set_consumer_name(ifx_dev->gpio.reset_out, "ifxModem reset out");
> + ifx_dev->gpio.pmu_reset = devm_gpiod_get(dev, "pmu_reset", GPIOD_ASIS);
> + if (IS_ERR(ifx_dev->gpio.pmu_reset)) {
> + dev_err(dev, "could not obtain pmu_reset GPIO\n");
> + ret = PTR_ERR(ifx_dev->gpio.pmu_reset);
> + goto error_ret;
> }
> + gpiod_set_consumer_name(ifx_dev->gpio.pmu_reset, "ifxModem PMU reset");
>
> - ret = request_irq(gpio_to_irq(ifx_dev->gpio.reset_out),
> + ret = request_irq(gpiod_to_irq(ifx_dev->gpio.reset_out),
> ifx_spi_reset_interrupt,
> IRQF_TRIGGER_RISING|IRQF_TRIGGER_FALLING, DRVNAME,
> ifx_dev);
> if (ret) {
> dev_err(dev, "Unable to get irq %x\n",
> - gpio_to_irq(ifx_dev->gpio.reset_out));
> - goto error_ret6;
> + gpiod_to_irq(ifx_dev->gpio.reset_out));
> + goto error_ret;
> }
>
> ret = ifx_spi_reset(ifx_dev);
>
> - ret = request_irq(gpio_to_irq(ifx_dev->gpio.srdy),
> + ret = request_irq(gpiod_to_irq(ifx_dev->gpio.srdy),
> ifx_spi_srdy_interrupt, IRQF_TRIGGER_RISING, DRVNAME,
> ifx_dev);
> if (ret) {
> dev_err(dev, "Unable to get irq %x",
> - gpio_to_irq(ifx_dev->gpio.srdy));
> - goto error_ret7;
> + gpiod_to_irq(ifx_dev->gpio.srdy));
> + goto error_ret2;
> }
>
> /* set pm runtime power state and register with power system */
> @@ -1191,7 +1151,7 @@ static int ifx_spi_spi_probe(struct spi_device *spi)
> /* no outgoing tty open at this point, this just satisfies the
> * modem's read and should reset communication properly
> */
> - srdy = gpio_get_value(ifx_dev->gpio.srdy);
> + srdy = gpiod_get_value(ifx_dev->gpio.srdy);
>
> if (srdy) {
> mrdy_assert(ifx_dev);
> @@ -1200,18 +1160,8 @@ static int ifx_spi_spi_probe(struct spi_device *spi)
> mrdy_set_low(ifx_dev);
> return 0;
>
> -error_ret7:
> - free_irq(gpio_to_irq(ifx_dev->gpio.reset_out), ifx_dev);
> -error_ret6:
> - gpio_free(ifx_dev->gpio.srdy);
> -error_ret5:
> - gpio_free(ifx_dev->gpio.mrdy);
> -error_ret4:
> - gpio_free(ifx_dev->gpio.reset);
> -error_ret3:
> - gpio_free(ifx_dev->gpio.po);
> error_ret2:
> - gpio_free(ifx_dev->gpio.reset_out);
> + free_irq(gpiod_to_irq(ifx_dev->gpio.reset_out), ifx_dev);
> error_ret:
> ifx_spi_free_device(ifx_dev);
> saved_ifx_dev = NULL;
> @@ -1235,14 +1185,8 @@ static int ifx_spi_spi_remove(struct spi_device *spi)
> pm_runtime_disable(&spi->dev);
>
> /* free irq */
> - free_irq(gpio_to_irq(ifx_dev->gpio.reset_out), ifx_dev);
> - free_irq(gpio_to_irq(ifx_dev->gpio.srdy), ifx_dev);
> -
> - gpio_free(ifx_dev->gpio.srdy);
> - gpio_free(ifx_dev->gpio.mrdy);
> - gpio_free(ifx_dev->gpio.reset);
> - gpio_free(ifx_dev->gpio.po);
> - gpio_free(ifx_dev->gpio.reset_out);
> + free_irq(gpiod_to_irq(ifx_dev->gpio.reset_out), ifx_dev);
> + free_irq(gpiod_to_irq(ifx_dev->gpio.srdy), ifx_dev);
>
> /* free allocations */
> ifx_spi_free_device(ifx_dev);
> diff --git a/drivers/tty/serial/ifx6x60.h b/drivers/tty/serial/ifx6x60.h
> index c5a2514212ff..f37e356af553 100644
> --- a/drivers/tty/serial/ifx6x60.h
> +++ b/drivers/tty/serial/ifx6x60.h
> @@ -10,6 +10,8 @@
> #ifndef _IFX6X60_H
> #define _IFX6X60_H
>
> +struct gpio_desc;
> +
> #define DRVNAME "ifx6x60"
> #define TTYNAME "ttyIFX"
>
> @@ -94,11 +96,12 @@ struct ifx_spi_device {
>
> struct {
> /* gpio lines */
> - unsigned short srdy; /* slave-ready gpio */
> - unsigned short mrdy; /* master-ready gpio */
> - unsigned short reset; /* modem-reset gpio */
> - unsigned short po; /* modem-on gpio */
> - unsigned short reset_out; /* modem-in-reset gpio */
> + struct gpio_desc *srdy; /* slave-ready gpio */
> + struct gpio_desc *mrdy; /* master-ready gpio */
> + struct gpio_desc *reset; /* modem-reset gpio */
> + struct gpio_desc *po; /* modem-on gpio */
> + struct gpio_desc *reset_out; /* modem-in-reset gpio */
> + struct gpio_desc *pmu_reset; /* PMU reset gpio */
> /* state/stats */
> int unack_srdy_int_nb;
> } gpio;
> diff --git a/include/linux/spi/ifx_modem.h b/include/linux/spi/ifx_modem.h
> index 694268c78d5d..6d19b09139d0 100644
> --- a/include/linux/spi/ifx_modem.h
> +++ b/include/linux/spi/ifx_modem.h
> @@ -3,12 +3,7 @@
> #define LINUX_IFX_MODEM_H
>
> struct ifx_modem_platform_data {
> - unsigned short rst_out; /* modem reset out */
> - unsigned short pwr_on; /* power on */
> - unsigned short rst_pmu; /* reset modem */
> unsigned short tx_pwr; /* modem power threshold */
> - unsigned short srdy; /* SRDY */
> - unsigned short mrdy; /* MRDY */
> unsigned char modem_type; /* Modem type */
> unsigned long max_hz; /* max SPI frequency */
> unsigned short use_dma:1; /* spi protocol driver supplies
> --
> 2.24.1
>
--
With Best Regards,
Andy Shevchenko
