Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-12 Thread Jonathan Cameron
David Brownell wrote: > On Thursday 11 September 2008, Ned Forrester wrote: > >> I'm puzzled. What is there about the pxa2xx_spi.c that would be >> simplified by doing anything with respect to SSPFRM? > > The usage model ... callers must understand that option, > evaluate whether it's even usabl

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-11 Thread David Brownell
On Thursday 11 September 2008, Ned Forrester wrote: > I'm puzzled. What is there about the pxa2xx_spi.c that would be > simplified by doing anything with respect to SSPFRM? The usage model ... callers must understand that option, evaluate whether it's even usable on their board, and then have an

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-11 Thread Ned Forrester
David Brownell wrote: > On Tuesday 09 September 2008, Jonathan Cameron wrote: >>> - It's probably worth removing support for the SSFRM >>>mechanism and requiring gpio_cs or (at least as a >>>transitional scheme) the cs_control() thing. >> I disagree strongly with this. >> >> By all means i

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-09 Thread Eric Miao
>> > + if (gpio_is_valid(chip_info->gpio_cs)) { >> > + err = gpio_request(chip_info->gpio_cs, "SPI_CS"); > > I'd actually pass dev_name(&spi->dev) not "SPI_CS", since I > happen to like seeing /sys/kernel/debug/gpio be informative. > Good suggestion, indeed. I'll include this in th

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-09 Thread David Brownell
On Monday 08 September 2008, Eric Miao wrote: > My original patch is going to free the previously requested GPIO > anyway, which I think is acceptable since no existing drivers is > going to call setup() many times during its lifetime. That seems like a bad assumption to make. :) -

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-09 Thread David Brownell
> > + /* NOTE: setup() can be called multiple times, possibly with > > +  * different chip_info, previously requested GPIO shall be > > +  * released, stumped :( > > +  */ No; chip_info should be regarded as immutable between the time the initializing setup() call is invoked and, s

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-09 Thread Jonathan Cameron
Dear David, > Related: > > - It's probably worth removing support for the SSFRM >mechanism and requiring gpio_cs or (at least as a >transitional scheme) the cs_control() thing. I disagree strongly with this. By all means issue a warning that cs_change will be effectively 1 in all cases.

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-08 Thread Eric Miao
On Tue, Sep 9, 2008 at 10:50 AM, Ned Forrester <[EMAIL PROTECTED]> wrote: > Eric Miao wrote: +static int setup_cs(struct spi_device *spi, struct chip_data *chip, + struct pxa2xx_spi_chip *chip_info) >>> My understanding is that it is legal to call setup without a defined >

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-08 Thread Ned Forrester
Eric Miao wrote: > On Tue, Sep 9, 2008 at 9:42 AM, Eric Miao <[EMAIL PROTECTED]> wrote: +static int setup_cs(struct spi_device *spi, struct chip_data *chip, + struct pxa2xx_spi_chip *chip_info) >>> My understanding is that it is legal to call setup without a defined >>> po

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-08 Thread Ned Forrester
Eric Miao wrote: >>> +static int setup_cs(struct spi_device *spi, struct chip_data *chip, >>> + struct pxa2xx_spi_chip *chip_info) >> My understanding is that it is legal to call setup without a defined >> pointer to a struct pxa2xx_spi_chip in spi_dev->controller_data, if the >> ch

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-08 Thread Eric Miao
On Tue, Sep 9, 2008 at 9:42 AM, Eric Miao <[EMAIL PROTECTED]> wrote: >>> +static int setup_cs(struct spi_device *spi, struct chip_data *chip, >>> + struct pxa2xx_spi_chip *chip_info) >> >> My understanding is that it is legal to call setup without a defined >> pointer to a struct px

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-08 Thread Eric Miao
>> +static int setup_cs(struct spi_device *spi, struct chip_data *chip, >> + struct pxa2xx_spi_chip *chip_info) > > My understanding is that it is legal to call setup without a defined > pointer to a struct pxa2xx_spi_chip in spi_dev->controller_data, if the > chip is happy with the

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-08 Thread Ned Forrester
David Brownell wrote: > On Thursday 04 September 2008, Eric Miao wrote: > > From: "Eric Miao" <[EMAIL PROTECTED]> > > Most SPI peripherals use GPIOs as their chip select, introduce .gpio_cs > for this, to avoid > > Signed-off-by: Eric Miao <[EMAIL PROTECTED]> [snip] > +static int

Re: [spi-devel-general] [PATCH] spi: pxa2xx_spi introduce chip select gpio to simplify the common cases

2008-09-08 Thread David Brownell
On Thursday 04 September 2008, Eric Miao wrote: > As Russell suggested, updated with gpio_is_valid(). He beat me to that comment. ;) I like, but that doesn't apply on top of the bugfix patch which I just cc'd you on... Related: - It's probably worth removing support for the SSFRM mechanism