From: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
To: Stefan Roese <sr-ynQEQJNshbs@public.gmane.org>
Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org
Subject: Re: [PATCH v4] spi: Add PPC4xx SPI driver
Date: Thu, 20 Nov 2008 19:40:01 -0800 [thread overview]
Message-ID: <200811201940.02277.david-b@pacbell.net> (raw)
In-Reply-To: <1225451266-23740-1-git-send-email-sr-ynQEQJNshbs@public.gmane.org>
On Friday 31 October 2008, Stefan Roese wrote:
> +static int spi_ppc4xx_setupxfer(struct spi_device *spi, struct spi_transfer *t)
> +{
> ...
>
> + if (in_8(&hw->regs->cdm) != cdm)
> + out_8(&hw->regs->cdm, cdm);
... writes to hardware, updating SPI the clock rate ...
> +static int spi_ppc4xx_setup(struct spi_device *spi)
> +{
> ...
> +
> + ret = spi_ppc4xx_setupxfer(spi, NULL);
... hence this also writes to the hardware.
Except ... the setup() method may be called for one SPI device
in the middle of a transfer for another device. This might for
example cause the clock rate to speed up so it's too fast, thus
corrupting a transfer for that other device. So you should NOT
be calling setupxfer() here.
> + /* write new configration */
> + out_8(&hw->regs->mode, mode);
Or here: changing from a device using MSB-first mode 3 to
one using LSB-first mode 1. This could also corrupt a transfer.
It might be better to have the setup() validate its inputs
and store them in the spi->controller_state, and then have
the setup_transfer() pull values from there ... but not make
setup() call setupxfer(). The best model would be that each
chipselect has its own set of speed/mode registers; if the
hardware doesn't work that way, then it can be emulated.
> +static int __init spi_ppc4xx_init(void)
> +{
> + return of_register_platform_driver(&spi_ppc4xx_of_driver);
> +}
> +
> +static void __exit spi_ppc4xx_exit(void)
> +{
> + of_unregister_platform_driver(&spi_ppc4xx_of_driver);
> +}
> +
> +module_init(spi_ppc4xx_init);
> +module_exit(spi_ppc4xx_exit);
I prefer to see module_{init,exit} annotations snugged up next
to the functions to which they apply ... just like EXPORT_SYMBOL
annotations, and for the same reasons.
- Dave
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
next prev parent reply other threads:[~2008-11-21 3:40 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-31 11:07 [PATCH v4] spi: Add PPC4xx SPI driver Stefan Roese
[not found] ` <1225451266-23740-1-git-send-email-sr-ynQEQJNshbs@public.gmane.org>
2008-11-04 13:10 ` Josh Boyer
2008-11-21 3:40 ` David Brownell [this message]
2008-11-21 3:41 ` David Brownell
2008-11-21 18:01 ` [spi-devel-general] " Stefan Roese
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=200811201940.02277.david-b@pacbell.net \
--to=david-b-ybekhbn/0ldr7s880joybq@public.gmane.org \
--cc=linuxppc-dev-mnsaURCQ41sdnm+yROfE0A@public.gmane.org \
--cc=spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
--cc=sr-ynQEQJNshbs@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).