Re: [PATCH] Add support for slave controllers plus sysfs entries for power management

From: jassi brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Linus Walleij <linus.ml.walleij-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: spi mailing list
	<spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>,
	Ken Mills <ken.k.mills-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH] Add support for slave controllers plus sysfs entries for power management
Date: Mon, 15 Feb 2010 10:37:45 +0900	[thread overview]
Message-ID: <1b68c6791002141737l6211c88dy79c762a3761cc93c@mail.gmail.com> (raw)
In-Reply-To: <63386a3d1002141520p7cf33256vd8d6f7c23f61b0fe-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, Feb 15, 2010 at 8:20 AM, Linus Walleij
<linus.ml.walleij@gmail.com> wrote:
> 2010/1/19 Grant Likely <grant.likely@secretlab.ca>:
>
>>  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@lists.sourceforge.net/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.

my 2 paise
-j

>
> The AMBA PL022 can act as both master or slave for example.
> It is possible to support either, master, slave, or both modes on this.
>
> You won't act as both master and slave on a certain bus. So at
> probetime a dual-mode thing like a PL022 would have to
> decide for being either master or slave. But it's not possible to
> do so at compile time: we may have a plethora of PL022:s on
> a SoC, some masters, some slaves.
>
> If spraying #ifdefs all over the place is undesirable, spi.h need
> to be split in spi-master.h and spi-slave.h IMHO, but they may
> share so much code and structure that #ifdefs is less disturbing
> really.
>
>> Do you expect any spi_device to be registered on an spi_slave?
>> Does the behaviour of an spi_device need
>> to change when it is registered against an spi_slave?
>
> Not that I'm an expert on how people construct their SPI systems,
> but only masters have devices, right? If you're a slave, some
> other master is asking you for something, so if one used USB
> terminology (why not) slaves would have functions, not devices,
> and function drivers, not device drivers.
>
> Just my €0.01...
>
> Linus Walleij
>
> ------------------------------------------------------------------------------
> 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
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
>

------------------------------------------------------------------------------
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
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general