All of lore.kernel.org
 help / color / mirror / Atom feed
From: Girish KS <girishks2000@gmail.com>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: spi-devel-general@lists.sourceforge.net,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/4] spi: s3c64xx: add gpio quirk for controller
Date: Thu, 7 Feb 2013 10:54:01 -0800	[thread overview]
Message-ID: <CAKrE-KfrWdOxm+DJyWqZn1Ye509E_LjzmDb5FMq55SF9-Sua0A@mail.gmail.com> (raw)
In-Reply-To: <20130207115546.GA3801@sirena.org.uk>

On Thu, Feb 7, 2013 at 3:55 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Feb 05, 2013 at 03:09:43PM -0800, Girish K S 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.
>
>>               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);
>>               }
>
> This isn't going to work with system designs which ignore the /CS line
> the controller has and just use a GPIO instead.  This is very common,
> for example when connecting multiple devices to the same SPI bus.
As per grant's comment i would remove the quirk option and check for
"cs-gpio" property
to handle /CS. When multiple devices are connected to the same spi
bus, "cs-gpio" entry would exist in the dts
file, and works with /CS gpio lines.
>
> It seems like there's really two changes here.  One change is making the
> provision of pinmux information optional, the other is allowing the user
> to use the controller /CS management rather than using GPIO.
As suggested i would make two changes.
1. would parse the "gpios"  property and make decision whether to use
gpios/dedicated pins for miso/miso/clk.
2.would parse "cs-gpio"  property from dts node to handle the chip select
correct me if my understanding is wrong

WARNING: multiple messages have this Message-ID (diff)
From: Girish KS <girishks2000-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Mark Brown
	<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 3/4] spi: s3c64xx: add gpio quirk for controller
Date: Thu, 7 Feb 2013 10:54:01 -0800	[thread overview]
Message-ID: <CAKrE-KfrWdOxm+DJyWqZn1Ye509E_LjzmDb5FMq55SF9-Sua0A@mail.gmail.com> (raw)
In-Reply-To: <20130207115546.GA3801-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

On Thu, Feb 7, 2013 at 3:55 AM, Mark Brown
<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org> wrote:
> On Tue, Feb 05, 2013 at 03:09:43PM -0800, Girish K S 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.
>
>>               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);
>>               }
>
> This isn't going to work with system designs which ignore the /CS line
> the controller has and just use a GPIO instead.  This is very common,
> for example when connecting multiple devices to the same SPI bus.
As per grant's comment i would remove the quirk option and check for
"cs-gpio" property
to handle /CS. When multiple devices are connected to the same spi
bus, "cs-gpio" entry would exist in the dts
file, and works with /CS gpio lines.
>
> It seems like there's really two changes here.  One change is making the
> provision of pinmux information optional, the other is allowing the user
> to use the controller /CS management rather than using GPIO.
As suggested i would make two changes.
1. would parse the "gpios"  property and make decision whether to use
gpios/dedicated pins for miso/miso/clk.
2.would parse "cs-gpio"  property from dts node to handle the chip select
correct me if my understanding is wrong

------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb

WARNING: multiple messages have this Message-ID (diff)
From: girishks2000@gmail.com (Girish KS)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] spi: s3c64xx: add gpio quirk for controller
Date: Thu, 7 Feb 2013 10:54:01 -0800	[thread overview]
Message-ID: <CAKrE-KfrWdOxm+DJyWqZn1Ye509E_LjzmDb5FMq55SF9-Sua0A@mail.gmail.com> (raw)
In-Reply-To: <20130207115546.GA3801@sirena.org.uk>

On Thu, Feb 7, 2013 at 3:55 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Feb 05, 2013 at 03:09:43PM -0800, Girish K S 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.
>
>>               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);
>>               }
>
> This isn't going to work with system designs which ignore the /CS line
> the controller has and just use a GPIO instead.  This is very common,
> for example when connecting multiple devices to the same SPI bus.
As per grant's comment i would remove the quirk option and check for
"cs-gpio" property
to handle /CS. When multiple devices are connected to the same spi
bus, "cs-gpio" entry would exist in the dts
file, and works with /CS gpio lines.
>
> It seems like there's really two changes here.  One change is making the
> provision of pinmux information optional, the other is allowing the user
> to use the controller /CS management rather than using GPIO.
As suggested i would make two changes.
1. would parse the "gpios"  property and make decision whether to use
gpios/dedicated pins for miso/miso/clk.
2.would parse "cs-gpio"  property from dts node to handle the chip select
correct me if my understanding is wrong

  reply	other threads:[~2013-02-07 18:54 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-05 23:09 [PATCH 0/4] Add polling support for 64xx spi controller Girish K S
2013-02-05 23:09 ` Girish K S
2013-02-05 23:09 ` Girish K S
2013-02-05 23:09 ` [PATCH 1/4] spi: s3c64xx: modified error interrupt handling and init Girish K S
2013-02-05 23:09   ` Girish K S
2013-02-05 23:09   ` Girish K S
2013-02-06 10:26   ` Grant Likely
2013-02-06 10:26     ` Grant Likely
2013-02-06 10:26     ` Grant Likely
2013-02-06 20:12     ` Girish KS
2013-02-06 20:12       ` Girish KS
2013-02-06 20:12       ` Girish KS
2013-02-06 23:48       ` Grant Likely
2013-02-06 23:48         ` Grant Likely
2013-02-06 23:48         ` Grant Likely
2013-02-07  0:33         ` Girish KS
2013-02-07  0:33           ` Girish KS
2013-02-07  0:33           ` Girish KS
2013-02-08  1:04         ` Girish KS
2013-02-08  1:04           ` Girish KS
2013-02-08  1:04           ` Girish KS
2013-02-08  8:16           ` Girish KS
2013-02-08  8:16             ` Girish KS
2013-02-08  8:16             ` Girish KS
2013-02-07 11:09       ` Tomasz Figa
2013-02-07 11:09         ` Tomasz Figa
2013-02-07 17:46         ` Girish KS
2013-02-07 17:46           ` Girish KS
2013-02-07 17:46           ` Girish KS
2013-02-08  8:33           ` Tomasz Figa
2013-02-08  8:33             ` Tomasz Figa
2013-02-08  8:33             ` Tomasz Figa
2013-02-08  8:58             ` Girish KS
2013-02-08  8:58               ` Girish KS
2013-02-08  8:58               ` Girish KS
2013-02-08  9:26               ` Girish KS
2013-02-08  9:26                 ` Girish KS
2013-02-08  9:26                 ` Girish KS
2013-02-05 23:09 ` [PATCH 2/4] spi: s3c64xx: added support for polling mode Girish K S
2013-02-05 23:09   ` Girish K S
2013-02-06 10:35   ` Grant Likely
2013-02-06 10:35     ` Grant Likely
2013-02-06 10:35     ` Grant Likely
2013-02-06 22:04     ` Girish KS
2013-02-06 22:04       ` Girish KS
2013-02-06 22:04       ` Girish KS
2013-02-05 23:09 ` [PATCH 3/4] spi: s3c64xx: add gpio quirk for controller Girish K S
2013-02-05 23:09   ` Girish K S
2013-02-05 23:09   ` Girish K S
2013-02-06 10:40   ` Grant Likely
2013-02-06 10:40     ` Grant Likely
2013-02-06 10:40     ` Grant Likely
2013-02-06 22:38     ` Girish KS
2013-02-06 22:38       ` Girish KS
2013-02-06 22:38       ` Girish KS
2013-02-07 11:55   ` Mark Brown
2013-02-07 11:55     ` Mark Brown
2013-02-07 11:55     ` Mark Brown
2013-02-07 18:54     ` Girish KS [this message]
2013-02-07 18:54       ` Girish KS
2013-02-07 18:54       ` Girish KS
2013-02-08 13:17       ` Mark Brown
2013-02-08 13:17         ` Mark Brown
2013-02-05 23:09 ` [PATCH 4/4] spi: s3c64xx: add support for exynos5440 spi Girish K S
2013-02-05 23:09   ` Girish K S
2013-02-05 23:09   ` Girish K S

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKrE-KfrWdOxm+DJyWqZn1Ye509E_LjzmDb5FMq55SF9-Sua0A@mail.gmail.com \
    --to=girishks2000@gmail.com \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=spi-devel-general@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.