lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKrE-KfS_Q0KJJwB--VTMtaDumfGSdM_2Zg_pYeUb4cSgmrehA@mail.gmail.com>
Date:	Wed, 6 Feb 2013 14:38:50 -0800
From:	Girish KS <girishks2000@...il.com>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	spi-devel-general@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/4] spi: s3c64xx: add gpio quirk for controller

On Wed, Feb 6, 2013 at 2:40 AM, Grant Likely <grant.likely@...retlab.ca> wrote:
> On Tue,  5 Feb 2013 15:09:43 -0800, Girish K S <girishks2000@...il.com> wrote:
>> This patch adds support for spi controllers with
>> dedicated clk/miso/mosi/cs pins. It skips the gpio
>> parsing and initialization for controllers that
>> have dedicated pins.
>>
>> Signed-off-by: Girish K S <ks.giri@...sung.com>
>
> Instead of making this a quirk, you should use the presence/absense of a
> cs-gpios property to determin whether or not a gpio is used to
> manipulate the CS signal.
I can do this for CS signal. Other signals are also dedicated pins.
can i check the "gpios" property for them?
>
> Also, are you certain that you want to make this change? Most SPI
> controllers are actually unfriendly in that they insist on deasserting the
> CS line if the FIFO runs empty. Does this driver correctly keep the CS
> signal asserted when there are multiple transfers in a message?
The CS signal is asserted when the slave select bit is cleared, this
is done in the enable_cs function.
The CS signal is deasserted when the slave select bit is set, this is
done in the disable_cs function.
enable_cs is called at the begining of the message transfer and
disable is called at the end of the transfer.
>
> g.
>
>> ---
>>  drivers/spi/spi-s3c64xx.c |   39 +++++++++++++++++++++++++++++++--------
>>  1 file changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index 90770bd..f06bbee 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -36,6 +36,7 @@
>>
>>  #define MAX_SPI_PORTS                3
>>  #define S3C64XX_SPI_QUIRK_POLL               (1 << 0)
>> +#define S3C64XX_SPI_QUIRK_GPIO               (1 << 1)
>>
>>  /* Registers and bit-fields */
>>
>> @@ -404,14 +405,16 @@ static inline void enable_cs(struct s3c64xx_spi_driver_data *sdd,
>>               if (sdd->tgl_spi != spi) { /* if last mssg on diff device */
>>                       /* Deselect the last toggled device */
>>                       cs = sdd->tgl_spi->controller_data;
>> -                     gpio_set_value(cs->line,
>> -                             spi->mode & SPI_CS_HIGH ? 0 : 1);
>> +                     if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_GPIO))
>> +                             gpio_set_value(cs->line,
>> +                                     spi->mode & SPI_CS_HIGH ? 0 : 1);
>>               }
>>               sdd->tgl_spi = NULL;
>>       }
>>
>>       cs = spi->controller_data;
>> -     gpio_set_value(cs->line, spi->mode & SPI_CS_HIGH ? 1 : 0);
>> +     if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_GPIO))
>> +             gpio_set_value(cs->line, spi->mode & SPI_CS_HIGH ? 1 : 0);
>>
>>       /* Start the signals */
>>       writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
>> @@ -503,7 +506,8 @@ static inline void disable_cs(struct s3c64xx_spi_driver_data *sdd,
>>       if (sdd->tgl_spi == spi)
>>               sdd->tgl_spi = NULL;
>>
>> -     gpio_set_value(cs->line, spi->mode & SPI_CS_HIGH ? 0 : 1);
>> +     if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_GPIO))
>> +             gpio_set_value(cs->line, spi->mode & SPI_CS_HIGH ? 0 : 1);
>>
>>       /* Quiese the signals */
>>       writel(S3C64XX_SPI_SLAVE_SIG_INACT,
>> @@ -842,7 +846,10 @@ static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
>>               return ERR_PTR(-ENOMEM);
>>       }
>>
>> -     cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
>> +     /* In case of dedicated cs pin skip the gpio initialization */
>> +     if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_GPIO))
>> +             cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
>> +
>>       if (!gpio_is_valid(cs->line)) {
>>               dev_err(&spi->dev, "chip select gpio is not specified or "
>>                                       "invalid\n");
>> @@ -883,7 +890,7 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>>               return -ENODEV;
>>       }
>>
>> -     if (!spi_get_ctldata(spi)) {
>> +     if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_GPIO)) {
>>               err = gpio_request_one(cs->line, GPIOF_OUT_INIT_HIGH,
>>                                      dev_name(&spi->dev));
>>               if (err) {
>> @@ -892,9 +899,11 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>>                               cs->line, err);
>>                       goto err_gpio_req;
>>               }
>> -             spi_set_ctldata(spi, cs);
>>       }
>>
>> +     if (!spi_get_ctldata(spi))
>> +             spi_set_ctldata(spi, cs);
>> +
>>       sci = sdd->cntrlr_info;
>>
>>       spin_lock_irqsave(&sdd->lock, flags);
>> @@ -979,8 +988,11 @@ err_gpio_req:
>>  static void s3c64xx_spi_cleanup(struct spi_device *spi)
>>  {
>>       struct s3c64xx_spi_csinfo *cs = spi_get_ctldata(spi);
>> +     struct s3c64xx_spi_driver_data *sdd;
>> +
>> +     sdd = spi_master_get_devdata(spi->master);
>>
>> -     if (cs) {
>> +     if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_GPIO) && cs) {
>>               gpio_free(cs->line);
>>               if (spi->dev.of_node)
>>                       kfree(cs);
>> @@ -1107,6 +1119,13 @@ static int s3c64xx_spi_parse_dt_gpio(struct s3c64xx_spi_driver_data *sdd)
>>       struct device *dev = &sdd->pdev->dev;
>>       int idx, gpio, ret;
>>
>> +     /*
>> +      * If cs is not controlled by gpio, and
>> +      * the SoC uses internal dedicated pins
>> +      */
>> +     if (sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_GPIO)
>> +             return 0;
>> +
>>       /* find gpios for mosi, miso and clock lines */
>>       for (idx = 0; idx < 3; idx++) {
>>               gpio = of_get_gpio(dev->of_node, idx);
>> @@ -1133,6 +1152,10 @@ free_gpio:
>>  static void s3c64xx_spi_dt_gpio_free(struct s3c64xx_spi_driver_data *sdd)
>>  {
>>       unsigned int idx;
>> +
>> +     if (sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_GPIO)
>> +             return;
>> +
>>       for (idx = 0; idx < 3; idx++)
>>               gpio_free(sdd->gpios[idx]);
>>  }
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>
> --
> Grant Likely, B.Sc, P.Eng.
> Secret Lab Technologies, Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ