archive mirror
 help / color / mirror / Atom feed
From: Linus Walleij <>
To: Grant Likely <grant.likely-s3s/>
Cc: spi mailing list
	Ken Mills <>
Subject: Re: [PATCH] Add support for slave controllers plus sysfs entries for power management
Date: Mon, 15 Feb 2010 00:20:19 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

2010/1/19 Grant Likely <grant.likely-s3s/>:

>  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.)

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

> 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

  parent reply	other threads:[~2010-02-14 23:20 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-18 21:06 Ken Mills
2010-01-19 15:59 ` Grant Likely
     [not found]   ` <>
2010-02-14 23:20     ` Linus Walleij [this message]
     [not found]       ` <>
2010-02-15  1:37         ` jassi brar
     [not found]           ` <>
2010-02-16 19:33             ` Linus Walleij
     [not found]               ` <>
2010-02-16 20:48                 ` Ned Forrester
     [not found]                   ` <4B7B048C.8080205-/>
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

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:

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

  git send-email \ \ \
    --cc=grant.likely-s3s/ \ \ \
    --subject='Re: [PATCH] Add support for slave controllers plus sysfs entries for power management' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).