[PATCH linux dev-5.7 v2 6/7] spi: fsi: Check mux status before transfers

[Joel Stanley]

On Thu, 30 Jul 2020 at 22:03, Eddie James <eajames at linux.ibm.com> wrote:
>
> The SPI controllers are not accessible if the mux isn't set. Therefore,
> check the mux status before starting a transfer and fail out if it isn't
> set.
>
> Signed-off-by: Eddie James <eajames at linux.ibm.com>
> ---
>  drivers/spi/spi-fsi.c | 40 +++++++++++++++++++++++++++-------------
>  1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c
> index 53cfa201e187..8949a64ec87d 100644
> --- a/drivers/spi/spi-fsi.c
> +++ b/drivers/spi/spi-fsi.c
> @@ -12,6 +12,7 @@
>
>  #define FSI_ENGID_SPI                  0x23
>  #define FSI_MBOX_ROOT_CTRL_8           0x2860
> +#define  FSI_MBOX_ROOT_CTRL_8_SPI       0xf0000000

Could be called SPI_MUX or something similar.

>
>  #define FSI2SPI_DATA0                  0x00
>  #define FSI2SPI_DATA1                  0x04
> @@ -84,6 +85,26 @@ struct fsi_spi_sequence {
>         u64 data;
>  };
>
> +static int fsi_spi_check_mux(struct fsi_device *fsi, struct device *dev)
> +{
> +       int rc;
> +       u32 root_ctrl_8;
> +       __be32 root_ctrl_8_be;
> +
> +       rc = fsi_slave_read(fsi->slave, FSI_MBOX_ROOT_CTRL_8, &root_ctrl_8_be,
> +                           sizeof(root_ctrl_8_be));
> +       if (rc)
> +               return rc;
> +
> +       root_ctrl_8 = be32_to_cpu(root_ctrl_8_be);
> +       dev_dbg(dev, "Root control register 8: %08x\n", root_ctrl_8);
> +       if ((root_ctrl_8 & FSI_MBOX_ROOT_CTRL_8_SPI) ==
> +            FSI_MBOX_ROOT_CTRL_8_SPI)
> +               return 0;
> +
> +       return -ENOLINK;

We bubble this error up to device drivers. Are they ok with ENOLINK?

> +}
> +
>  static int fsi_spi_check_status(struct fsi_spi *ctx)
>  {
>         int rc;
> @@ -449,11 +470,15 @@ static int fsi_spi_transfer_init(struct fsi_spi *ctx)
>  static int fsi_spi_transfer_one_message(struct spi_controller *ctlr,
>                                         struct spi_message *mesg)
>  {
> -       int rc = 0;
> +       int rc;
>         u8 seq_slave = SPI_FSI_SEQUENCE_SEL_SLAVE(mesg->spi->chip_select + 1);
>         struct spi_transfer *transfer;
>         struct fsi_spi *ctx = spi_controller_get_devdata(ctlr);
>
> +       rc = fsi_spi_check_mux(ctx->fsi, ctx->dev);
> +       if (rc)
> +               return rc;
> +
>         list_for_each_entry(transfer, &mesg->transfers, transfer_list) {
>                 struct fsi_spi_sequence seq;
>                 struct spi_transfer *next = NULL;
> @@ -537,24 +562,13 @@ static size_t fsi_spi_max_transfer_size(struct spi_device *spi)
>  static int fsi_spi_probe(struct device *dev)
>  {
>         int rc;
> -       u32 root_ctrl_8;
>         struct device_node *np;
>         int num_controllers_registered = 0;
>         struct fsi_device *fsi = to_fsi_dev(dev);
>
> -       /*
> -        * Check the SPI mux before attempting to probe. If the mux isn't set
> -        * then the SPI controllers can't access their slave devices.
> -        */
> -       rc = fsi_slave_read(fsi->slave, FSI_MBOX_ROOT_CTRL_8, &root_ctrl_8,
> -                           sizeof(root_ctrl_8));
> +       rc = fsi_spi_check_mux(fsi, dev);
>         if (rc)
> -               return rc;
> -
> -       if (!root_ctrl_8) {
> -               dev_dbg(dev, "SPI mux not set, aborting probe.\n");

Should we make this a dev_warn so it's clear why the driver did not "work"?

>                 return -ENODEV;

> -       }
>
>         for_each_available_child_of_node(dev->of_node, np) {
>                 u32 base;
> --
> 2.24.0
>