linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
To: jassi brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Ken Mills <ken.k.mills-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	spi mailing list
	<spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Subject: Re: [PATCH] Add support for slave controllers plus sysfs entries for power management
Date: Tue, 16 Feb 2010 15:01:20 -0700	[thread overview]
Message-ID: <fa686aa41002161401y72cd08bfq68135e9e259eca2c@mail.gmail.com> (raw)
In-Reply-To: <1b68c6791002141737l6211c88dy79c762a3761cc93c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sun, Feb 14, 2010 at 6:37 PM, jassi brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Feb 15, 2010 at 8:20 AM, Linus Walleij
> <linus.ml.walleij-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> 2010/1/19 Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>:
>>
>>>  The current model is that each spi_device is registered with an
>>> spi_master.  Many device drivers operate on the assumption that the
>>> ->master pointer is valid.  With this patch, it appears that
>>> spi_devices can be registered either against an spi_master or an
>>> spi_slave; thus invalidating the assumption drivers are already
>>> operating under.  That alone makes me nervous.
>>
>> IIRC in my last review of this patch I proposed that drivers be
>> either master, slave or both. With the current approach that would
>> mean putting  #ifdefs over the ->master as well and making master
>> support optional. (You can even #ifdef out these parts of the struct
>> to be absolutely sure.)
>
> I don't think adding SPI_SLAVE support is just a matter of providing
> additional callbacks and structures, as is pointed out in this thread....
> http://www.mail-archive.com/spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org/msg00368.html
>
> We'd better see if the implementation covers at least some aspects of
> being a SPI slave,
> otherwise it seems more like a 'workaround' than support.

I agree.  I need to see a real usage model before I merge anything.
As it stands right now, spi_master looks like an entirely different
thing from an spi_slave, even if both happen to use spi_messages
(although spi_message only seems to handle a small number of the
possible spi slave use cases.  For example, it doesn't address at all
a command and response within the same SPI transfer).  I don't at all
like the idea of allowing the current spi_device to be registered on
either an spi_master or an spi_slave.

I may let a driver through that happens to turn on an SPI slave mode
in the hardware, but if you're doing that, be aware that it may be an
abuse of the spi_master model, and you'll get little sympathy when
breakage happens.

I haven't seen anything spi_slave related yet that I'll merge in the
common spi code.

g.

------------------------------------------------------------------------------
SOLARIS 10 is the OS for Data Centers - provides features such as DTrace,
Predictive Self Healing and Award Winning ZFS. Get Solaris 10 NOW
http://p.sf.net/sfu/solaris-dev2dev

      parent reply	other threads:[~2010-02-16 22:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-18 21:06 [PATCH] Add support for slave controllers plus sysfs entries for power management Ken Mills
2010-01-19 15:59 ` Grant Likely
     [not found]   ` <fa686aa41001190759j1d916f87o309efe37b3e96bf4-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-02-14 23:20     ` Linus Walleij
     [not found]       ` <63386a3d1002141520p7cf33256vd8d6f7c23f61b0fe-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-02-15  1:37         ` jassi brar
     [not found]           ` <1b68c6791002141737l6211c88dy79c762a3761cc93c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-02-16 19:33             ` Linus Walleij
     [not found]               ` <63386a3d1002161133k501e51f4xf4e94a307cb4fcf5-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-02-16 20:48                 ` Ned Forrester
     [not found]                   ` <4B7B048C.8080205-/d+BM93fTQY@public.gmane.org>
2010-02-17  3:19                     ` Grant Likely
2010-02-16 22:07                 ` Grant Likely
2010-02-17  1:43                 ` jassi brar
2010-02-16 22:01             ` Grant Likely [this message]

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=fa686aa41002161401y72cd08bfq68135e9e259eca2c@mail.gmail.com \
    --to=grant.likely-s3s/wqlpoipyb63q8fvjnq@public.gmane.org \
    --cc=jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=ken.k.mills-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).