| Message ID | 1310687525-22486-4-git-send-email-plagnioj@jcrosoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Sorry, I couldn't find a summary message for this patch series, so I picked this one to reply, because this one hurts me most. On 07/15/11 01:52, Jean-Christophe PLAGNIOL-VILLARD wrote: > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com> > Cc: Nicolas Ferre<nicolas.ferre@atmel.com> > --- > arch/arm/mach-at91/include/mach/at91_spi.h | 81 ---------------------------- > 1 files changed, 0 insertions(+), 81 deletions(-) > delete mode 100644 arch/arm/mach-at91/include/mach/at91_spi.h [...] First, I'm actually using mach/at91_spi.h in an SPI slave driver. And I'm using arch/arm/mach-at91/include/mach/at91_spi.h instead of drivers/spi/atmel_spi.h (which still exists in the kernel version we use here), because arch/arm/mach-at91/include/mach/at91_spi.h is accessible from an out-of-tree driver w/o extra effort. And this applies to all all of those header files, so I'm really against all patches in this series. Detlef
On 13:47 Fri 15 Jul , Detlef Vollmann wrote: > Sorry, I couldn't find a summary message for this patch series, > so I picked this one to reply, because this one hurts me most. > > On 07/15/11 01:52, Jean-Christophe PLAGNIOL-VILLARD wrote: > > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com> > > Cc: Nicolas Ferre<nicolas.ferre@atmel.com> > > --- > > arch/arm/mach-at91/include/mach/at91_spi.h | 81 > ---------------------------- > > 1 files changed, 0 insertions(+), 81 deletions(-) > > delete mode 100644 arch/arm/mach-at91/include/mach/at91_spi.h > [...] > > First, I'm actually using mach/at91_spi.h in an SPI slave driver. > And I'm using arch/arm/mach-at91/include/mach/at91_spi.h instead of > drivers/spi/atmel_spi.h (which still exists in the kernel version > we use here), because arch/arm/mach-at91/include/mach/at91_spi.h > is accessible from an out-of-tree driver w/o extra effort. > > And this applies to all all of those header files, so I'm > really against all patches in this series. You need the use SPI framework for this Out of tree driver is not enough good reason to keep it Best Regards, J.
On 07/15/11 17:46, Jean-Christophe PLAGNIOL-VILLARD wrote: > On 13:47 Fri 15 Jul , Detlef Vollmann wrote: >> Sorry, I couldn't find a summary message for this patch series, >> so I picked this one to reply, because this one hurts me most. >> >> On 07/15/11 01:52, Jean-Christophe PLAGNIOL-VILLARD wrote: >>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com> >>> Cc: Nicolas Ferre<nicolas.ferre@atmel.com> >>> --- >>> arch/arm/mach-at91/include/mach/at91_spi.h | 81 >> ---------------------------- >>> 1 files changed, 0 insertions(+), 81 deletions(-) >>> delete mode 100644 arch/arm/mach-at91/include/mach/at91_spi.h >> [...] >> >> First, I'm actually using mach/at91_spi.h in an SPI slave driver. >> And I'm using arch/arm/mach-at91/include/mach/at91_spi.h instead of >> drivers/spi/atmel_spi.h (which still exists in the kernel version >> we use here), because arch/arm/mach-at91/include/mach/at91_spi.h >> is accessible from an out-of-tree driver w/o extra effort. >> >> And this applies to all all of those header files, so I'm >> really against all patches in this series. > You need the use SPI framework for this I think you reply here to the wrong part of my message. This part here is about the whole patch series, as I didn't get a summary message for the patch series. About the SPI driver: From Documentation/spi/spi-summary: "At this writing, Linux has no slave side programming interface." So there's no SPI framwork that covers the slave side. > Out of tree driver is not enough good reason to keep it As there's currently no slave support in-tree, any Linux device that's an SPI slave needs an out-of-tree driver. And again, there's no rationale given for moving all the other headers from an genarally accessible place to a generally inaccessible place, so I'm still against the whole patch series. Regards, Detlef
On 18:44 Fri 15 Jul , Detlef Vollmann wrote: > On 07/15/11 17:46, Jean-Christophe PLAGNIOL-VILLARD wrote: > >On 13:47 Fri 15 Jul , Detlef Vollmann wrote: > >>Sorry, I couldn't find a summary message for this patch series, > >>so I picked this one to reply, because this one hurts me most. > >> > >>On 07/15/11 01:52, Jean-Christophe PLAGNIOL-VILLARD wrote: > >>>Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com> > >>>Cc: Nicolas Ferre<nicolas.ferre@atmel.com> > >>>--- > >>> arch/arm/mach-at91/include/mach/at91_spi.h | 81 > >>---------------------------- > >>> 1 files changed, 0 insertions(+), 81 deletions(-) > >>> delete mode 100644 arch/arm/mach-at91/include/mach/at91_spi.h > >>[...] > >> > >>First, I'm actually using mach/at91_spi.h in an SPI slave driver. > >>And I'm using arch/arm/mach-at91/include/mach/at91_spi.h instead of > >>drivers/spi/atmel_spi.h (which still exists in the kernel version > >>we use here), because arch/arm/mach-at91/include/mach/at91_spi.h > >>is accessible from an out-of-tree driver w/o extra effort. > >> > >>And this applies to all all of those header files, so I'm > >>really against all patches in this series. > >You need the use SPI framework for this > I think you reply here to the wrong part of my message. > This part here is about the whole patch series, as I didn't > get a summary message for the patch series. > > About the SPI driver: > From Documentation/spi/spi-summary: > "At this writing, Linux has no slave side programming interface." > So there's no SPI framwork that covers the slave side. This what I mean use the SPI framework and add the SPI Slave support there > > >Out of tree driver is not enough good reason to keep it > As there's currently no slave support in-tree, any Linux device > that's an SPI slave needs an out-of-tree driver. > > And again, there's no rationale given for moving all the other > headers from an genarally accessible place to a generally > inaccessible place, so I'm still against the whole patch series. those information are driver and IP related keep them in the arch/arm/mach-at91 make those driver difficult to share cross arch for the SPI this IP is shared between avr2 and arm so keep it in arch is not acceptable at all So if you want to add the SPI Slave support you must make is available for both ARCH so NACK to keep this one and the drivers/spi/atmel_spi.h and arch/arm/mach-at91/include/mach/at91_spi.h contain the same information so we drop the one not used Best Regards, J.
> > About the SPI driver: > > From Documentation/spi/spi-summary: > > "At this writing, Linux has no slave side programming interface." > > So there's no SPI framwork that covers the slave side. > This what I mean use the SPI framework and add the SPI Slave support there [CCing spi-devel] If you talk about SPI slaves, be sure to have read Grant's comments in http://thread.gmane.org/gmane.linux.kernel.spi.devel/7401 to see what would be needed for slave support. Short note: Would be seperate subsystem, master subsystem is of limited use. I think a few slave-drivers would be nice to evaluate, so one can get a feeling if a subsystem makes sense and if so, how it could look like. Detlef, have you posted your slave driver so far? Regards, Wolfram
On 07/16/11 12:57, Jean-Christophe PLAGNIOL-VILLARD wrote: > for the SPI this IP is shared between avr2 and arm so keep it in arch is not > acceptable at all Thanks. This was the motivation I missed. In this case I think such include files should go somewhere under include, so they can be accessed by other drivers. And this is really not only true for this at91_spi.h, but also for the other headers in this patch series. Detlef
On 07/16/11 13:56, Wolfram Sang wrote: > >>> About the SPI driver: >>> From Documentation/spi/spi-summary: >>> "At this writing, Linux has no slave side programming interface." >>> So there's no SPI framwork that covers the slave side. >> This what I mean use the SPI framework and add the SPI Slave support there > > [CCing spi-devel] > > If you talk about SPI slaves, be sure to have read Grant's comments in > > http://thread.gmane.org/gmane.linux.kernel.spi.devel/7401 > > to see what would be needed for slave support. I completely agree with Gran't comments here. > Short note: Would be > seperate subsystem, master subsystem is of limited use. I think a few > slave-drivers would be nice to evaluate, so one can get a feeling if a > subsystem makes sense and if so, how it could look like. > > Detlef, have you posted your slave driver so far? No, and I didn't plan to do so. Not because I don't want to publish it, if you want to look at it, I'm happy to send the driver. But I don't think it makes much sense to put my driver into mainline Linux. The reason for this is very simple: the protocol is specific for this hardware, and the driver needs this specific protocol. The Atmel hardware is not able to maintain information about frame ends when running in DMA mode, so we need to put the length of the frame to the beginning of the frame, and also need to put in some extra information to be able to re-sync in case of transmission errors. So this driver is very specific for this hardware, and I wrote SPI slave drivers in the past for other hardware that were also very specific. I really doubt that an SPI slave framework really makes sense. And that's the reason why I'm keen that the hardware description headers are available to out-of-tree drivers. Detlef
On 16:00 Sat 16 Jul , Detlef Vollmann wrote: > On 07/16/11 13:56, Wolfram Sang wrote: > > > >>>About the SPI driver: > >>> From Documentation/spi/spi-summary: > >>>"At this writing, Linux has no slave side programming interface." > >>>So there's no SPI framwork that covers the slave side. > >>This what I mean use the SPI framework and add the SPI Slave support there > > > >[CCing spi-devel] > > > >If you talk about SPI slaves, be sure to have read Grant's comments in > > > >http://thread.gmane.org/gmane.linux.kernel.spi.devel/7401 > > > >to see what would be needed for slave support. > I completely agree with Gran't comments here. > > >Short note: Would be > >seperate subsystem, master subsystem is of limited use. I think a few > >slave-drivers would be nice to evaluate, so one can get a feeling if a > >subsystem makes sense and if so, how it could look like. > > > >Detlef, have you posted your slave driver so far? > No, and I didn't plan to do so. > Not because I don't want to publish it, if you want to look at it, > I'm happy to send the driver. > But I don't think it makes much sense to put my driver into mainline > Linux. > The reason for this is very simple: the protocol is specific for this > hardware, and the driver needs this specific protocol. > The Atmel hardware is not able to maintain information about frame > ends when running in DMA mode, so we need to put the length of the > frame to the beginning of the frame, and also need to put in some > extra information to be able to re-sync in case of transmission errors. > > So this driver is very specific for this hardware, and I wrote SPI > slave drivers in the past for other hardware that were also very > specific. I really doubt that an SPI slave framework really makes > sense. > > And that's the reason why I'm keen that the hardware description > headers are available to out-of-tree drivers. So I'm sorry but this is clear I'm not going to keep the header out-of-tree drivers are not an enough reason to keep it Best Regards, J.
On 15:41 Sat 16 Jul , Detlef Vollmann wrote: > On 07/16/11 12:57, Jean-Christophe PLAGNIOL-VILLARD wrote: > >for the SPI this IP is shared between avr2 and arm so keep it in arch is not > >acceptable at all > Thanks. This was the motivation I missed. > In this case I think such include files should go somewhere under > include, so they can be accessed by other drivers. > And this is really not only true for this at91_spi.h, but also > for the other headers in this patch series. no we already have the header in drivers/spi so this one not used at all in the mainline is drop if you need to atmel_spi.h for mainline for other drivers you need to post your patch. Best Regards, J.
hi, > if you need to atmel_spi.h for mainline for other drivers you need to post > your patch. The at91_spi.h header is also use by the original SPI stack. The atmel_spi and mtd_dataflash don't work properly for AT91RM9200-based boards, therefore out-of-tree drivers need to be used. http://maxim.org.za/at91_26.html (And I have spent way too many hours trying to get atmel_spi to work) Regards, Andrew Victor
diff --git a/arch/arm/mach-at91/include/mach/at91_spi.h b/arch/arm/mach-at91/include/mach/at91_spi.h deleted file mode 100644 index 2f6ba0c..0000000 --- a/arch/arm/mach-at91/include/mach/at91_spi.h +++ /dev/null @@ -1,81 +0,0 @@ -/* - * arch/arm/mach-at91/include/mach/at91_spi.h - * - * Copyright (C) 2005 Ivan Kokshaysky - * Copyright (C) SAN People - * - * Serial Peripheral Interface (SPI) registers. - * Based on AT91RM9200 datasheet revision E. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - */ - -#ifndef AT91_SPI_H -#define AT91_SPI_H - -#define AT91_SPI_CR 0x00 /* Control Register */ -#define AT91_SPI_SPIEN (1 << 0) /* SPI Enable */ -#define AT91_SPI_SPIDIS (1 << 1) /* SPI Disable */ -#define AT91_SPI_SWRST (1 << 7) /* SPI Software Reset */ -#define AT91_SPI_LASTXFER (1 << 24) /* Last Transfer [SAM9261 only] */ - -#define AT91_SPI_MR 0x04 /* Mode Register */ -#define AT91_SPI_MSTR (1 << 0) /* Master/Slave Mode */ -#define AT91_SPI_PS (1 << 1) /* Peripheral Select */ -#define AT91_SPI_PS_FIXED (0 << 1) -#define AT91_SPI_PS_VARIABLE (1 << 1) -#define AT91_SPI_PCSDEC (1 << 2) /* Chip Select Decode */ -#define AT91_SPI_DIV32 (1 << 3) /* Clock Selection [AT91RM9200 only] */ -#define AT91_SPI_MODFDIS (1 << 4) /* Mode Fault Detection */ -#define AT91_SPI_LLB (1 << 7) /* Local Loopback Enable */ -#define AT91_SPI_PCS (0xf << 16) /* Peripheral Chip Select */ -#define AT91_SPI_DLYBCS (0xff << 24) /* Delay Between Chip Selects */ - -#define AT91_SPI_RDR 0x08 /* Receive Data Register */ -#define AT91_SPI_RD (0xffff << 0) /* Receive Data */ -#define AT91_SPI_PCS (0xf << 16) /* Peripheral Chip Select */ - -#define AT91_SPI_TDR 0x0c /* Transmit Data Register */ -#define AT91_SPI_TD (0xffff << 0) /* Transmit Data */ -#define AT91_SPI_PCS (0xf << 16) /* Peripheral Chip Select */ -#define AT91_SPI_LASTXFER (1 << 24) /* Last Transfer [SAM9261 only] */ - -#define AT91_SPI_SR 0x10 /* Status Register */ -#define AT91_SPI_RDRF (1 << 0) /* Receive Data Register Full */ -#define AT91_SPI_TDRE (1 << 1) /* Transmit Data Register Full */ -#define AT91_SPI_MODF (1 << 2) /* Mode Fault Error */ -#define AT91_SPI_OVRES (1 << 3) /* Overrun Error Status */ -#define AT91_SPI_ENDRX (1 << 4) /* End of RX buffer */ -#define AT91_SPI_ENDTX (1 << 5) /* End of TX buffer */ -#define AT91_SPI_RXBUFF (1 << 6) /* RX Buffer Full */ -#define AT91_SPI_TXBUFE (1 << 7) /* TX Buffer Empty */ -#define AT91_SPI_NSSR (1 << 8) /* NSS Rising [SAM9261 only] */ -#define AT91_SPI_TXEMPTY (1 << 9) /* Transmission Register Empty [SAM9261 only] */ -#define AT91_SPI_SPIENS (1 << 16) /* SPI Enable Status */ - -#define AT91_SPI_IER 0x14 /* Interrupt Enable Register */ -#define AT91_SPI_IDR 0x18 /* Interrupt Disable Register */ -#define AT91_SPI_IMR 0x1c /* Interrupt Mask Register */ - -#define AT91_SPI_CSR(n) (0x30 + ((n) * 4)) /* Chip Select Registers 0-3 */ -#define AT91_SPI_CPOL (1 << 0) /* Clock Polarity */ -#define AT91_SPI_NCPHA (1 << 1) /* Clock Phase */ -#define AT91_SPI_CSAAT (1 << 3) /* Chip Select Active After Transfer [SAM9261 only] */ -#define AT91_SPI_BITS (0xf << 4) /* Bits Per Transfer */ -#define AT91_SPI_BITS_8 (0 << 4) -#define AT91_SPI_BITS_9 (1 << 4) -#define AT91_SPI_BITS_10 (2 << 4) -#define AT91_SPI_BITS_11 (3 << 4) -#define AT91_SPI_BITS_12 (4 << 4) -#define AT91_SPI_BITS_13 (5 << 4) -#define AT91_SPI_BITS_14 (6 << 4) -#define AT91_SPI_BITS_15 (7 << 4) -#define AT91_SPI_BITS_16 (8 << 4) -#define AT91_SPI_SCBR (0xff << 8) /* Serial Clock Baud Rate */ -#define AT91_SPI_DLYBS (0xff << 16) /* Delay before SPCK */ -#define AT91_SPI_DLYBCT (0xff << 24) /* Delay between Consecutive Transfers */ - -#endif
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> Cc: Nicolas Ferre <nicolas.ferre@atmel.com> --- arch/arm/mach-at91/include/mach/at91_spi.h | 81 ---------------------------- 1 files changed, 0 insertions(+), 81 deletions(-) delete mode 100644 arch/arm/mach-at91/include/mach/at91_spi.h