| Message ID | 201306300245.58486.sergei.shtylyov@cogentembedded.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi > From: Max Filippov <max.filippov@cogentembedded.com> > > Use previously empty <mach/dma.h> to declare HPB-DMA slave IDs. > > Signed-off-by: Max Filippov <max.filippov@cogentembedded.com> > [Sergei: changed the guard macro name, fixed comment, extended copyright.] > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > --- > arch/arm/mach-shmobile/include/mach/dma.h | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > Index: renesas/arch/arm/mach-shmobile/include/mach/dma.h > =================================================================== > --- renesas.orig/arch/arm/mach-shmobile/include/mach/dma.h > +++ renesas/arch/arm/mach-shmobile/include/mach/dma.h > @@ -1 +1,24 @@ > -/* empty */ > +/* > + * Copyright (C) 2011-2013 Renesas Electronics Corporation > + * Copyright (C) 2013 Cogent Embedded, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 > + * as published by the Free Software Foundation. > + */ > + > +#ifndef __MACH_DMA_H > +#define __MACH_DMA_H > + > +/* HPB-DMA slave IDs */ > +enum { > + HPBDMA_SLAVE_DUMMY, > + HPBDMA_SLAVE_SDHI0_TX, > + HPBDMA_SLAVE_SDHI0_RX, > + HPBDMA_SLAVE_SSI0_TX_ST, > + HPBDMA_SLAVE_SSI0_TX_MN, > + HPBDMA_SLAVE_SSI1_RX_ST, > + HPBDMA_SLAVE_SSI1_RX_MN, > +}; it should be in r8a7778.h Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 01-07-2013 8:11, Kuninori Morimoto wrote: >> From: Max Filippov <max.filippov@cogentembedded.com> >> Use previously empty <mach/dma.h> to declare HPB-DMA slave IDs. >> Signed-off-by: Max Filippov <max.filippov@cogentembedded.com> >> [Sergei: changed the guard macro name, fixed comment, extended copyright.] >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> --- >> arch/arm/mach-shmobile/include/mach/dma.h | 25 ++++++++++++++++++++++++- >> 1 file changed, 24 insertions(+), 1 deletion(-) >> Index: renesas/arch/arm/mach-shmobile/include/mach/dma.h >> =================================================================== >> --- renesas.orig/arch/arm/mach-shmobile/include/mach/dma.h >> +++ renesas/arch/arm/mach-shmobile/include/mach/dma.h >> @@ -1 +1,24 @@ >> -/* empty */ >> +/* >> + * Copyright (C) 2011-2013 Renesas Electronics Corporation >> + * Copyright (C) 2013 Cogent Embedded, Inc. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 >> + * as published by the Free Software Foundation. >> + */ >> + >> +#ifndef __MACH_DMA_H >> +#define __MACH_DMA_H >> + >> +/* HPB-DMA slave IDs */ >> +enum { >> + HPBDMA_SLAVE_DUMMY, >> + HPBDMA_SLAVE_SDHI0_TX, >> + HPBDMA_SLAVE_SDHI0_RX, >> + HPBDMA_SLAVE_SSI0_TX_ST, >> + HPBDMA_SLAVE_SSI0_TX_MN, >> + HPBDMA_SLAVE_SSI1_RX_ST, >> + HPBDMA_SLAVE_SSI1_RX_MN, >> +}; > it should be in r8a7778.h It will be shared with R8A7779. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sergei, On Sun, Jun 30, 2013 at 7:45 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > From: Max Filippov <max.filippov@cogentembedded.com> > > Use previously empty <mach/dma.h> to declare HPB-DMA slave IDs. > > Signed-off-by: Max Filippov <max.filippov@cogentembedded.com> > [Sergei: changed the guard macro name, fixed comment, extended copyright.] > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > --- > arch/arm/mach-shmobile/include/mach/dma.h | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) Thanks for your patch. Can you please explain the reason why you put this shared information under mach/? From my point of view we should really have as few special things as possible under mach/. Starting to share DMA stuff here looks special. I would like mach-shmobile to be as standard as possible (basically only ARM code in the future), and in the case we have I/O devices or other IOMMUs or DMA controllers then we should use include/linux or that recently added platform data directory if needed. We need to make sure that our headers stay compatible with multiplatform support. I realize you may want to share this header between multiple SoCs, but I really want to avoid creating local common code under arch/arm/ that has nothing to do with ARM. Also, this is IMO a total non-issue, instead of merging code and making back porting more difficult for any particular SoC, then how about spending effort on the ling term instead, like DMA DT? So my recommendation is to keep this information together with each SoC. And also work on DT. In between have a coffee or work on DT, but please do not spend time on merging short term per-SoC information and making back porting more difficult. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 07/01/2013 04:11 PM, Sergei Shtylyov wrote: >>> From: Max Filippov <max.filippov@cogentembedded.com> >>> Use previously empty <mach/dma.h> to declare HPB-DMA slave IDs. >>> Signed-off-by: Max Filippov <max.filippov@cogentembedded.com> >>> [Sergei: changed the guard macro name, fixed comment, extended copyright.] >>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>> --- >>> arch/arm/mach-shmobile/include/mach/dma.h | 25 ++++++++++++++++++++++++- >>> 1 file changed, 24 insertions(+), 1 deletion(-) >>> Index: renesas/arch/arm/mach-shmobile/include/mach/dma.h >>> =================================================================== >>> --- renesas.orig/arch/arm/mach-shmobile/include/mach/dma.h >>> +++ renesas/arch/arm/mach-shmobile/include/mach/dma.h >>> @@ -1 +1,24 @@ >>> -/* empty */ >>> +/* >>> + * Copyright (C) 2011-2013 Renesas Electronics Corporation >>> + * Copyright (C) 2013 Cogent Embedded, Inc. >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 >>> + * as published by the Free Software Foundation. >>> + */ >>> + >>> +#ifndef __MACH_DMA_H >>> +#define __MACH_DMA_H >>> + >>> +/* HPB-DMA slave IDs */ >>> +enum { >>> + HPBDMA_SLAVE_DUMMY, >>> + HPBDMA_SLAVE_SDHI0_TX, >>> + HPBDMA_SLAVE_SDHI0_RX, >>> + HPBDMA_SLAVE_SSI0_TX_ST, >>> + HPBDMA_SLAVE_SSI0_TX_MN, >>> + HPBDMA_SLAVE_SSI1_RX_ST, >>> + HPBDMA_SLAVE_SSI1_RX_MN, >>> +}; >> it should be in r8a7778.h > It will be shared with R8A7779. Apparently not anymore as this file is gone now. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 07/02/2013 08:59 AM, Magnus Damm wrote: >> From: Max Filippov <max.filippov@cogentembedded.com> >> Use previously empty <mach/dma.h> to declare HPB-DMA slave IDs. >> Signed-off-by: Max Filippov <max.filippov@cogentembedded.com> >> [Sergei: changed the guard macro name, fixed comment, extended copyright.] >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> --- >> arch/arm/mach-shmobile/include/mach/dma.h | 25 ++++++++++++++++++++++++- >> 1 file changed, 24 insertions(+), 1 deletion(-) > Thanks for your patch. Can you please explain the reason why you put > this shared information under mach/? I'm afraid that's more a question to the original author, Phil Edworthy. He put this stuff into <mach/hpb-dmae.h>. > From my point of view we should really have as few special things as > possible under mach/. Starting to share DMA stuff here looks special. > I would like mach-shmobile to be as standard as possible (basically > only ARM code in the future), and in the case we have I/O devices or > other IOMMUs or DMA controllers then we should use include/linux or > that recently added platform data directory if needed. We need to make > sure that our headers stay compatible with multiplatform support. > I realize you may want to share this header between multiple SoCs, but > I really want to avoid creating local common code under arch/arm/ that > has nothing to do with ARM. Also, this is IMO a total non-issue, > instead of merging code and making back porting more difficult for any > particular SoC, then how about spending effort on the ling term > instead, like DMA DT? > So my recommendation is to keep this information together with each > SoC. And also work on DT. In between have a coffee or work on DT, but > please do not spend time on merging short term per-SoC information and > making back porting more difficult. OK. DT would be a next step I guess. > Thanks, > / magnus WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi > >> From: Max Filippov <max.filippov@cogentembedded.com> > > >> Use previously empty <mach/dma.h> to declare HPB-DMA slave IDs. > > >> Signed-off-by: Max Filippov <max.filippov@cogentembedded.com> > >> [Sergei: changed the guard macro name, fixed comment, extended copyright.] > >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > >> --- > >> arch/arm/mach-shmobile/include/mach/dma.h | 25 ++++++++++++++++++++++++- > >> 1 file changed, 24 insertions(+), 1 deletion(-) > > > Thanks for your patch. Can you please explain the reason why you put > > this shared information under mach/? > > I'm afraid that's more a question to the original author, Phil Edworthy. > He put this stuff into <mach/hpb-dmae.h>. Then, why you could send this patch instead of original author, even though you can't answer any question about it ? Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 19-07-2013 4:16, Kuninori Morimoto wrote: >>>> From: Max Filippov <max.filippov@cogentembedded.com> >>>> Use previously empty <mach/dma.h> to declare HPB-DMA slave IDs. >>>> Signed-off-by: Max Filippov <max.filippov@cogentembedded.com> >>>> [Sergei: changed the guard macro name, fixed comment, extended copyright.] >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>>> --- >>>> arch/arm/mach-shmobile/include/mach/dma.h | 25 ++++++++++++++++++++++++- >>>> 1 file changed, 24 insertions(+), 1 deletion(-) >>> Thanks for your patch. Can you please explain the reason why you put >>> this shared information under mach/? >> I'm afraid that's more a question to the original author, Phil Edworthy. >> He put this stuff into <mach/hpb-dmae.h>. > Then, why you could send this patch instead of original author, Ask Phil why didn't he upstream his stuff. We, unlike him, do have obligation to upstream it. > even though you can't answer any question about it ? I'm not the author of the patch, let me remind you, Max Filippov is. He was on vacations. > Best regards > --- > Kuninori Morimoto WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 19-07-2013 15:17, Sergei Shtylyov wrote: >>>>> From: Max Filippov <max.filippov@cogentembedded.com> >>>>> Use previously empty <mach/dma.h> to declare HPB-DMA slave IDs. >>>>> Signed-off-by: Max Filippov <max.filippov@cogentembedded.com> >>>>> [Sergei: changed the guard macro name, fixed comment, extended copyright.] >>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>>>> --- >>>>> arch/arm/mach-shmobile/include/mach/dma.h | 25 >>>>> ++++++++++++++++++++++++- >>>>> 1 file changed, 24 insertions(+), 1 deletion(-) >>>> Thanks for your patch. Can you please explain the reason why you put >>>> this shared information under mach/? IIUC, because the only 2 file using it were under arch/arm/mach-shmobile/. >>> I'm afraid that's more a question to the original author, Phil Edworthy. >>> He put this stuff into <mach/hpb-dmae.h>. >> Then, why you could send this patch instead of original author, > Ask Phil why didn't he upstream his stuff. We, unlike him, do have > obligation to upstream it. >> even though you can't answer any question about it ? > I'm not the author of the patch, let me remind you, Max Filippov is. > He was on vacations. Phil's patches were only for R8A7778, IIRC (yet, the platfrom DMA code was placed in a separate file, not r8a7778.c). Max decided to use the <mach/hpb-dmae.h> header for sharing the DMA channel IDs between R8A777[89], I agreed but suggested to use the empty <mach/dma.h> instead. >> Best regards >> --- >> Kuninori Morimoto WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: renesas/arch/arm/mach-shmobile/include/mach/dma.h =================================================================== --- renesas.orig/arch/arm/mach-shmobile/include/mach/dma.h +++ renesas/arch/arm/mach-shmobile/include/mach/dma.h @@ -1 +1,24 @@ -/* empty */ +/* + * Copyright (C) 2011-2013 Renesas Electronics Corporation + * Copyright (C) 2013 Cogent Embedded, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 + * as published by the Free Software Foundation. + */ + +#ifndef __MACH_DMA_H +#define __MACH_DMA_H + +/* HPB-DMA slave IDs */ +enum { + HPBDMA_SLAVE_DUMMY, + HPBDMA_SLAVE_SDHI0_TX, + HPBDMA_SLAVE_SDHI0_RX, + HPBDMA_SLAVE_SSI0_TX_ST, + HPBDMA_SLAVE_SSI0_TX_MN, + HPBDMA_SLAVE_SSI1_RX_ST, + HPBDMA_SLAVE_SSI1_RX_MN, +}; + +#endif /* __MACH_DMA_H */