Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <1498083022-24907-1-git-send-email-jk@ozlabs.org>
References: <1497864405-26990-4-git-send-email-jk@ozlabs.org> <1498083022-24907-1-git-send-email-jk@ozlabs.org>
From:   Joel Stanley <joel@jms.id.au>
Date:   Tue, 30 Jan 2018 15:15:12 +1030
Message-ID: <CACPK8Xc7DHbNH==MuaFH5CAn94g-h_8bm4O32OvQDrj4hkKFnA@mail.gmail.com>
Subject: Re: [PATCH v2 3/3] fsi/master-gpio: Add external mode
To:     Jeremy Kerr <jk@ozlabs.org>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Christopher Bostic <cbostic@linux.vnet.ibm.com>,
        Greg KH <gregkh@linuxfoundation.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Jeremy,

On Thu, Jun 22, 2017 at 7:40 AM, Jeremy Kerr <jk@ozlabs.org> wrote:
> This change introduces an 'external mode' for GPIO-based FSI masters,
> allowing the clock and data lines to be driven by an external source.
> For example, external mode is selected by a user when an external debug
> device is attached to the FSI pins.
>
> To do this, we need to set specific states for the trans, mux and enable
> gpios, and prevent access to clk & data from the FSI core code (by
> returning EBUSY).
>
> External mode is controlled by a sysfs attribute, so add the relevent
> information to Documentation/ABI/
>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Reviewed-by: Joel Stanley <joel@jms.id.au>

This one never made it into Greg's tree (I assume because we didn't cc him).

Can you resend with Greg on cc please?

Cheers,

Joel

> ---
>  .../ABI/testing/sysfs-driver-fsi-master-gpio       | 10 +++
>  drivers/fsi/fsi-master-gpio.c                      | 78 +++++++++++++++++++++-
>  2 files changed, 86 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
> new file mode 100644
> index 0000000..9667bb4
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-fsi-master-gpio
> @@ -0,0 +1,10 @@
> +What:           /sys/bus/platform/devices/[..]/fsi-master-gpio/external_mode
> +Date:           June 2017
> +KernelVersion:  4.12
> +Contact:        jk@ozlabs.org
> +Description:
> +                Controls access arbitration for GPIO-based FSI master. A
> +               value of 0 (the default) sets normal mode, where the
> +               driver performs FSI bus transactions, 1 sets external mode,
> +               where the FSI bus is driven externally (for example, by
> +               a debug device).
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> index a6d602e..b54c213 100644
> --- a/drivers/fsi/fsi-master-gpio.c
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -59,6 +59,7 @@ struct fsi_master_gpio {
>         struct gpio_desc        *gpio_trans;    /* Voltage translator */
>         struct gpio_desc        *gpio_enable;   /* FSI enable */
>         struct gpio_desc        *gpio_mux;      /* Mux control */
> +       bool                    external_mode;
>  };
>
>  #define CREATE_TRACE_POINTS
> @@ -411,6 +412,12 @@ static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
>         int rc;
>
>         spin_lock_irqsave(&master->cmd_lock, flags);
> +
> +       if (master->external_mode) {
> +               spin_unlock_irqrestore(&master->cmd_lock, flags);
> +               return -EBUSY;
> +       }
> +
>         serial_out(master, cmd);
>         echo_delay(master);
>         rc = poll_for_response(master, slave, resp_len, resp);
> @@ -469,6 +476,10 @@ static int fsi_master_gpio_break(struct fsi_master *_master, int link)
>         trace_fsi_master_gpio_break(master);
>
>         spin_lock_irqsave(&master->cmd_lock, flags);
> +       if (master->external_mode) {
> +               spin_unlock_irqrestore(&master->cmd_lock, flags);
> +               return -EBUSY;
> +       }
>         set_sda_output(master, 1);
>         sda_out(master, 1);
>         clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
> @@ -497,25 +508,84 @@ static void fsi_master_gpio_init(struct fsi_master_gpio *master)
>         clock_zeros(master, FSI_INIT_CLOCKS);
>  }
>
> +static void fsi_master_gpio_init_external(struct fsi_master_gpio *master)
> +{
> +       gpiod_direction_output(master->gpio_mux, 0);
> +       gpiod_direction_output(master->gpio_trans, 0);
> +       gpiod_direction_output(master->gpio_enable, 1);
> +       gpiod_direction_input(master->gpio_clk);
> +       gpiod_direction_input(master->gpio_data);
> +}
> +
>  static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
>  {
>         struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
>         unsigned long flags;
> +       int rc = -EBUSY;
>
>         if (link != 0)
>                 return -ENODEV;
>
>         spin_lock_irqsave(&master->cmd_lock, flags);
> -       gpiod_set_value(master->gpio_enable, 1);
> +       if (!master->external_mode) {
> +               gpiod_set_value(master->gpio_enable, 1);
> +               rc = 0;
> +       }
>         spin_unlock_irqrestore(&master->cmd_lock, flags);
>
> -       return 0;
> +       return rc;
> +}
> +
> +static ssize_t external_mode_show(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +
> +       return snprintf(buf, PAGE_SIZE - 1, "%u\n",
> +                       master->external_mode ? 1 : 0);
> +}
> +
> +static ssize_t external_mode_store(struct device *dev,
> +               struct device_attribute *attr, const char *buf, size_t count)
> +{
> +       struct fsi_master_gpio *master = dev_get_drvdata(dev);
> +       unsigned long flags, val;
> +       bool external_mode;
> +       int err;
> +
> +       err = kstrtoul(buf, 0, &val);
> +       if (err)
> +               return err;
> +
> +       external_mode = !!val;
> +
> +       spin_lock_irqsave(&master->cmd_lock, flags);
> +
> +       if (external_mode == master->external_mode) {
> +               spin_unlock_irqrestore(&master->cmd_lock, flags);
> +               return count;
> +       }
> +
> +       master->external_mode = external_mode;
> +       if (master->external_mode)
> +               fsi_master_gpio_init_external(master);
> +       else
> +               fsi_master_gpio_init(master);
> +       spin_unlock_irqrestore(&master->cmd_lock, flags);
> +
> +       fsi_master_rescan(&master->master);
> +
> +       return count;
>  }
>
> +static DEVICE_ATTR(external_mode, 0664,
> +               external_mode_show, external_mode_store);
> +
>  static int fsi_master_gpio_probe(struct platform_device *pdev)
>  {
>         struct fsi_master_gpio *master;
>         struct gpio_desc *gpio;
> +       int rc;
>
>         master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
>         if (!master)
> @@ -572,6 +642,10 @@ static int fsi_master_gpio_probe(struct platform_device *pdev)
>
>         fsi_master_gpio_init(master);
>
> +       rc = device_create_file(&pdev->dev, &dev_attr_external_mode);
> +       if (rc)
> +               return rc;
> +
>         return fsi_master_register(&master->master);
>  }
>
> --
> 2.7.4
>