| Message ID | 1381828104-31302-2-git-send-email-cm-hiep@jinso.co.jp (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Oct 15, 2013 at 06:08:23PM +0900, Cao Minh Hiep wrote: > From: Hiep Cao Minh <cm-hiep@jinso.co.jp> > > Add platform device and clock for the r8a7790 QSPI. > > Signed-off-by: Hiep Cao Minh <cm-hiep@jinso.co.jp> Hi Hiep-san, thanks for your nice patches :) Morimoto-san, Magnus, It seems to me that moving all of the device registration into the board-lager.c would be more in keeping with our recent work. Could you confirm this or otherwise? > --- > arch/arm/mach-shmobile/clock-r8a7790.c | 4 ++++ > arch/arm/mach-shmobile/include/mach/r8a7790.h | 2 ++ > arch/arm/mach-shmobile/setup-r8a7790.c | 16 ++++++++++++++++ > 3 files changed, 22 insertions(+) > > diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c > index fc36d3d..baabceb 100644 > --- a/arch/arm/mach-shmobile/clock-r8a7790.c > +++ b/arch/arm/mach-shmobile/clock-r8a7790.c > @@ -52,6 +52,7 @@ > #define SMSTPCR5 0xe6150144 > #define SMSTPCR7 0xe615014c > #define SMSTPCR8 0xe6150990 > +#define SMSTPCR9 0xE6150994 > > #define SDCKCR 0xE6150074 > #define SD2CKCR 0xE6150078 > @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = { > > /* MSTP */ > enum { > + MSTP917, > MSTP813, > MSTP721, MSTP720, > MSTP717, MSTP716, > @@ -192,6 +194,7 @@ enum { > }; > > static struct clk mstp_clks[MSTP_NR] = { > + [MSTP917] = SH_CLK_MSTP32(&qspi_clk, SMSTPCR9, 17, 0), /* QSPI */ > [MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */ > [MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */ > [MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */ > @@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { > CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), > CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), > CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), > + CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), > }; > > #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31) \ > diff --git a/arch/arm/mach-shmobile/include/mach/r8a7790.h b/arch/arm/mach-shmobile/include/mach/r8a7790.h > index 788d559..d590cad 100644 > --- a/arch/arm/mach-shmobile/include/mach/r8a7790.h > +++ b/arch/arm/mach-shmobile/include/mach/r8a7790.h > @@ -1,5 +1,6 @@ > #ifndef __ASM_R8A7790_H__ > #define __ASM_R8A7790_H__ > +#include <linux/spi/rspi.h> > > void r8a7790_add_standard_devices(void); > void r8a7790_add_dt_devices(void); > @@ -7,6 +8,7 @@ void r8a7790_clock_init(void); > void r8a7790_pinmux_init(void); > void r8a7790_init_delay(void); > void r8a7790_timer_init(void); > +void r8a7790_add_qspi_device(struct rspi_plat_data *pdata); > > #define MD(nr) BIT(nr) > u32 r8a7790_read_mode_pins(void); > diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c > index d0f5c9f..30e3362 100644 > --- a/arch/arm/mach-shmobile/setup-r8a7790.c > +++ b/arch/arm/mach-shmobile/setup-r8a7790.c > @@ -30,6 +30,22 @@ > #include <mach/irqs.h> > #include <mach/r8a7790.h> > #include <asm/mach/arch.h> > +#include <linux/spi/spi.h> > +#include <linux/spi/rspi.h> > + > +/* QSPI resource */ > +static struct resource qspi_resources[] __initdata = { > + DEFINE_RES_MEM(0xe6b10000, 0x1000), > + DEFINE_RES_IRQ(gic_spi(184)), > +}; > + > +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) > +{ > + platform_device_register_resndata( > + &platform_bus, "qspi", 0, > + qspi_resources, ARRAY_SIZE(qspi_resources), > + pdata, sizeof(*pdata)); > +} > > static struct resource pfc_resources[] __initdata = { > DEFINE_RES_MEM(0xe6060000, 0x250), > -- > 1.7.9.5 > > -- > 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 > -- 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 Hiep-san, On Tue, Oct 15, 2013 at 6:08 PM, Cao Minh Hiep <cm-hiep@jinso.co.jp> wrote: > From: Hiep Cao Minh <cm-hiep@jinso.co.jp> > > Add platform device and clock for the r8a7790 QSPI. > > Signed-off-by: Hiep Cao Minh <cm-hiep@jinso.co.jp> Thanks for the patch. > --- > arch/arm/mach-shmobile/clock-r8a7790.c | 4 ++++ > arch/arm/mach-shmobile/include/mach/r8a7790.h | 2 ++ > arch/arm/mach-shmobile/setup-r8a7790.c | 16 ++++++++++++++++ > 3 files changed, 22 insertions(+) > > diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c > index fc36d3d..baabceb 100644 > --- a/arch/arm/mach-shmobile/clock-r8a7790.c > +++ b/arch/arm/mach-shmobile/clock-r8a7790.c > @@ -52,6 +52,7 @@ > #define SMSTPCR5 0xe6150144 > #define SMSTPCR7 0xe615014c > #define SMSTPCR8 0xe6150990 > +#define SMSTPCR9 0xE6150994 Please follow same style as other MSTP registers - use "e6" instead of "E6". > #define SDCKCR 0xE6150074 > #define SD2CKCR 0xE6150078 > @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = { > > /* MSTP */ > enum { > + MSTP917, > MSTP813, > MSTP721, MSTP720, > MSTP717, MSTP716, > @@ -192,6 +194,7 @@ enum { > }; > > static struct clk mstp_clks[MSTP_NR] = { > + [MSTP917] = SH_CLK_MSTP32(&qspi_clk, SMSTPCR9, 17, 0), /* QSPI */ > [MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */ > [MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */ > [MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */ > @@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { > CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), > CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), > CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), > + CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), Is "qspi" the name of the driver? If so, where can I find it? > --- a/arch/arm/mach-shmobile/include/mach/r8a7790.h > +++ b/arch/arm/mach-shmobile/include/mach/r8a7790.h > @@ -1,5 +1,6 @@ > #ifndef __ASM_R8A7790_H__ > #define __ASM_R8A7790_H__ > +#include <linux/spi/rspi.h> > > void r8a7790_add_standard_devices(void); > void r8a7790_add_dt_devices(void); > @@ -7,6 +8,7 @@ void r8a7790_clock_init(void); > void r8a7790_pinmux_init(void); > void r8a7790_init_delay(void); > void r8a7790_timer_init(void); > +void r8a7790_add_qspi_device(struct rspi_plat_data *pdata); Please don't add these kind of functions. Do it all inside board-lager.c instead. Simon may want you to split the board code and the SoC code as well. Cheers, / 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
On Wed, Oct 16, 2013 at 09:46:54AM +0900, Magnus Damm wrote: > Hi Hiep-san, > > On Tue, Oct 15, 2013 at 6:08 PM, Cao Minh Hiep <cm-hiep@jinso.co.jp> wrote: > > From: Hiep Cao Minh <cm-hiep@jinso.co.jp> > > > > Add platform device and clock for the r8a7790 QSPI. > > > > Signed-off-by: Hiep Cao Minh <cm-hiep@jinso.co.jp> > > Thanks for the patch. > > > --- > > arch/arm/mach-shmobile/clock-r8a7790.c | 4 ++++ > > arch/arm/mach-shmobile/include/mach/r8a7790.h | 2 ++ > > arch/arm/mach-shmobile/setup-r8a7790.c | 16 ++++++++++++++++ > > 3 files changed, 22 insertions(+) > > > > diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c > > index fc36d3d..baabceb 100644 > > --- a/arch/arm/mach-shmobile/clock-r8a7790.c > > +++ b/arch/arm/mach-shmobile/clock-r8a7790.c > > @@ -52,6 +52,7 @@ > > #define SMSTPCR5 0xe6150144 > > #define SMSTPCR7 0xe615014c > > #define SMSTPCR8 0xe6150990 > > +#define SMSTPCR9 0xE6150994 > > Please follow same style as other MSTP registers - use "e6" instead of "E6". > > > #define SDCKCR 0xE6150074 > > #define SD2CKCR 0xE6150078 > > @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = { > > > > /* MSTP */ > > enum { > > + MSTP917, > > MSTP813, > > MSTP721, MSTP720, > > MSTP717, MSTP716, > > @@ -192,6 +194,7 @@ enum { > > }; > > > > static struct clk mstp_clks[MSTP_NR] = { > > + [MSTP917] = SH_CLK_MSTP32(&qspi_clk, SMSTPCR9, 17, 0), /* QSPI */ > > [MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */ > > [MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */ > > [MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */ > > @@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { > > CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), > > CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), > > CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), > > + CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), > > Is "qspi" the name of the driver? If so, where can I find it? > > > --- a/arch/arm/mach-shmobile/include/mach/r8a7790.h > > +++ b/arch/arm/mach-shmobile/include/mach/r8a7790.h > > @@ -1,5 +1,6 @@ > > #ifndef __ASM_R8A7790_H__ > > #define __ASM_R8A7790_H__ > > +#include <linux/spi/rspi.h> > > > > void r8a7790_add_standard_devices(void); > > void r8a7790_add_dt_devices(void); > > @@ -7,6 +8,7 @@ void r8a7790_clock_init(void); > > void r8a7790_pinmux_init(void); > > void r8a7790_init_delay(void); > > void r8a7790_timer_init(void); > > +void r8a7790_add_qspi_device(struct rspi_plat_data *pdata); > > Please don't add these kind of functions. Do it all inside > board-lager.c instead. > >name Simon may want you to split the board code and the SoC code as well. I think that the best thing would be to move all of the device initialisation code into patch two of this series and just leave the clock portion in the first patch. That way Magnus's request above will be addressed and there will be a board patch and an SoC patch. -- 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 Magnus-san, Simon-san Thanks for your replying >>> --- >>> arch/arm/mach-shmobile/clock-r8a7790.c | 4 ++++ >>> arch/arm/mach-shmobile/include/mach/r8a7790.h | 2 ++ >>> arch/arm/mach-shmobile/setup-r8a7790.c | 16 ++++++++++++++++ >>> 3 files changed, 22 insertions(+) >>> >>> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c >>> index fc36d3d..baabceb 100644 >>> --- a/arch/arm/mach-shmobile/clock-r8a7790.c >>> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c >>> @@ -52,6 +52,7 @@ >>> #define SMSTPCR5 0xe6150144 >>> #define SMSTPCR7 0xe615014c >>> #define SMSTPCR8 0xe6150990 >>> +#define SMSTPCR9 0xE6150994 >> Please follow same style as other MSTP registers - use "e6" instead of "E6". >> I will do it! >>> #define SDCKCR 0xE6150074 >>> #define SD2CKCR 0xE6150078 >>> @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = { >>> >>> /* MSTP */ >>> enum { >>> + MSTP917, >>> MSTP813, >>> MSTP721, MSTP720, >>> MSTP717, MSTP716, >>> @@ -192,6 +194,7 @@ enum { >>> }; >>> >>> static struct clk mstp_clks[MSTP_NR] = { >>> + [MSTP917] = SH_CLK_MSTP32(&qspi_clk, SMSTPCR9, 17, 0), /* QSPI */ >>> [MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */ >>> [MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */ >>> [MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */ >>> @@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { >>> CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), >>> CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), >>> CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), >>> + CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), >> Is "qspi" the name of the driver? If so, where can I find it? I porting SPI driver to SPI-Mailing-list and Mark Brown replied that they are applied, I have tried to find them on https://patchwork.kernel.org/project/spi-devel-general/list/ but not found. I am asking Mark Brown where he put them on. >>> --- a/arch/arm/mach-shmobile/include/mach/r8a7790.h >>> +++ b/arch/arm/mach-shmobile/include/mach/r8a7790.h >>> @@ -1,5 +1,6 @@ >>> #ifndef __ASM_R8A7790_H__ >>> #define __ASM_R8A7790_H__ >>> +#include <linux/spi/rspi.h> >>> >>> void r8a7790_add_standard_devices(void); >>> void r8a7790_add_dt_devices(void); >>> @@ -7,6 +8,7 @@ void r8a7790_clock_init(void); >>> void r8a7790_pinmux_init(void); >>> void r8a7790_init_delay(void); >>> void r8a7790_timer_init(void); >>> +void r8a7790_add_qspi_device(struct rspi_plat_data *pdata); >> Please don't add these kind of functions. Do it all inside >> board-lager.c instead. >> >> name Simon may want you to split the board code and the SoC code as well. > I think that the best thing would be to move all of the device > initialisation code into patch two of this series and just leave the clock > portion in the first patch. That way Magnus's request above will be > addressed and there will be a board patch and an SoC patch. > > OK, I will modify them. thanks, Hiep. -- 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 Hiep I have one comment > From: Hiep Cao Minh <cm-hiep@jinso.co.jp> > > Add platform device and clock for the r8a7790 QSPI. > > Signed-off-by: Hiep Cao Minh <cm-hiep@jinso.co.jp> > --- (snip) > @@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { > CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), > CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), > CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), > + CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), > }; (snip) > +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) > +{ > + platform_device_register_resndata( > + &platform_bus, "qspi", 0, > + qspi_resources, ARRAY_SIZE(qspi_resources), > + pdata, sizeof(*pdata)); > +} Please use -1 ID, and use just "qspi", instead of "qspi.0" 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
On Wed, Oct 16, 2013 at 05:13:51PM -0700, Kuninori Morimoto wrote: > > Hi Hiep > > I have one comment > > > From: Hiep Cao Minh <cm-hiep@jinso.co.jp> > > > > Add platform device and clock for the r8a7790 QSPI. > > > > Signed-off-by: Hiep Cao Minh <cm-hiep@jinso.co.jp> > > --- > (snip) > > @@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { > > CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), > > CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), > > CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), > > + CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), > > }; > (snip) > > +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) > > +{ > > + platform_device_register_resndata( > > + &platform_bus, "qspi", 0, > > + qspi_resources, ARRAY_SIZE(qspi_resources), > > + pdata, sizeof(*pdata)); > > +} > > Please use -1 ID, and use just "qspi", instead of "qspi.0" Thanks Morimoto-san. Hiep-san, could you please update your patches according to the feedback from Morimoto-san, Magnus and myself. Please also rebase them on the latest devel tag of the renesas tree. Then please re-post your patches as v2, noting changes in the change log and including v2 after PATCH in each subject line. Thanks -- 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
On Wed, Oct 16, 2013 at 08:03:25PM +0900, ?? ?? ???? wrote: > Hi Magnus-san, Simon-san > > Thanks for your replying > > > >>>--- > >>> arch/arm/mach-shmobile/clock-r8a7790.c | 4 ++++ > >>> arch/arm/mach-shmobile/include/mach/r8a7790.h | 2 ++ > >>> arch/arm/mach-shmobile/setup-r8a7790.c | 16 ++++++++++++++++ > >>> 3 files changed, 22 insertions(+) > >>> > >>>diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c > >>>index fc36d3d..baabceb 100644 > >>>--- a/arch/arm/mach-shmobile/clock-r8a7790.c > >>>+++ b/arch/arm/mach-shmobile/clock-r8a7790.c > >>>@@ -52,6 +52,7 @@ > >>> #define SMSTPCR5 0xe6150144 > >>> #define SMSTPCR7 0xe615014c > >>> #define SMSTPCR8 0xe6150990 > >>>+#define SMSTPCR9 0xE6150994 > >>Please follow same style as other MSTP registers - use "e6" instead of "E6". > >> > I will do it! > >>> #define SDCKCR 0xE6150074 > >>> #define SD2CKCR 0xE6150078 > >>>@@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = { > >>> > >>> /* MSTP */ > >>> enum { > >>>+ MSTP917, > >>> MSTP813, > >>> MSTP721, MSTP720, > >>> MSTP717, MSTP716, > >>>@@ -192,6 +194,7 @@ enum { > >>> }; > >>> > >>> static struct clk mstp_clks[MSTP_NR] = { > >>>+ [MSTP917] = SH_CLK_MSTP32(&qspi_clk, SMSTPCR9, 17, 0), /* QSPI */ > >>> [MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */ > >>> [MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */ > >>> [MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */ > >>>@@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { > >>> CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), > >>> CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), > >>> CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), > >>>+ CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), > >>Is "qspi" the name of the driver? If so, where can I find it? > I porting SPI driver to SPI-Mailing-list and Mark Brown replied that > they are applied, > I have tried to find them on > https://patchwork.kernel.org/project/spi-devel-general/list/ > but not found. I am asking Mark Brown where he put them on. Thanks, please let us know when you find out. > >>>--- a/arch/arm/mach-shmobile/include/mach/r8a7790.h > >>>+++ b/arch/arm/mach-shmobile/include/mach/r8a7790.h > >>>@@ -1,5 +1,6 @@ > >>> #ifndef __ASM_R8A7790_H__ > >>> #define __ASM_R8A7790_H__ > >>>+#include <linux/spi/rspi.h> > >>> > >>> void r8a7790_add_standard_devices(void); > >>> void r8a7790_add_dt_devices(void); > >>>@@ -7,6 +8,7 @@ void r8a7790_clock_init(void); > >>> void r8a7790_pinmux_init(void); > >>> void r8a7790_init_delay(void); > >>> void r8a7790_timer_init(void); > >>>+void r8a7790_add_qspi_device(struct rspi_plat_data *pdata); > >>Please don't add these kind of functions. Do it all inside > >>board-lager.c instead. > >> > >>name Simon may want you to split the board code and the SoC code as well. > >I think that the best thing would be to move all of the device > >initialisation code into patch two of this series and just leave the clock > >portion in the first patch. That way Magnus's request above will be > >addressed and there will be a board patch and an SoC patch. > > > > > OK, I will modify them. > > thanks, > > Hiep. > > -- > 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 > -- 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 Morimoto-san Thanks for your comment! On 10/17/2013 09:13 AM, Kuninori Morimoto wrote: > Hi Hiep > > I have one comment > >> From: Hiep Cao Minh <cm-hiep@jinso.co.jp> >> >> Add platform device and clock for the r8a7790 QSPI. >> >> Signed-off-by: Hiep Cao Minh <cm-hiep@jinso.co.jp> >> --- > (snip) >> @@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { >> CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), >> CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), >> CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), >> + CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), >> }; > (snip) >> +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) >> +{ >> + platform_device_register_resndata( >> + &platform_bus, "qspi", 0, >> + qspi_resources, ARRAY_SIZE(qspi_resources), >> + pdata, sizeof(*pdata)); >> +} > Please use -1 ID, and use just "qspi", instead of "qspi.0" I have tried to use -1 ID instead of 0, and then I saw that QSPI did not contact to Flash Memory. I found that in QSPI driver, ID is assigned to bus number: master->bus_num = pdev->id. So I wonder how to modify it. Best Regards! Hiep. -- 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 Hiep-san > >> +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) > >> +{ > >> + platform_device_register_resndata( > >> + &platform_bus, "qspi", 0, > >> + qspi_resources, ARRAY_SIZE(qspi_resources), > >> + pdata, sizeof(*pdata)); > >> +} > > Please use -1 ID, and use just "qspi", instead of "qspi.0" > I have tried to use -1 ID instead of 0, and then I saw that QSPI did not > contact to Flash Memory. > I found that in QSPI driver, ID is assigned to bus number: > master->bus_num = pdev->id. > So I wonder how to modify it. According to spi_register_master(), bus_num = -1 seems not problem. Did you exchanged clock name ? 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
Hi Morimoto-san Thanks for your replying! >>>> +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) >>>> +{ >>>> + platform_device_register_resndata( >>>> + &platform_bus, "qspi", 0, >>>> + qspi_resources, ARRAY_SIZE(qspi_resources), >>>> + pdata, sizeof(*pdata)); >>>> +} >>> Please use -1 ID, and use just "qspi", instead of "qspi.0" >> I have tried to use -1 ID instead of 0, and then I saw that QSPI did not >> contact to Flash Memory. >> I found that in QSPI driver, ID is assigned to bus number: >> master->bus_num = pdev->id. >> So I wonder how to modify it. > According to spi_register_master(), bus_num = -1 seems not problem. > Did you exchanged clock name ? I saw that in "struct spi_board_info" has defined bus_num is u16 so I'm afraid if is -1 ID, master->bus_num did not match to bus_num in board_info. Thanks, Hiep. -- 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 > >>>> +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) > >>>> +{ > >>>> + platform_device_register_resndata( > >>>> + &platform_bus, "qspi", 0, > >>>> + qspi_resources, ARRAY_SIZE(qspi_resources), > >>>> + pdata, sizeof(*pdata)); > >>>> +} > >>> Please use -1 ID, and use just "qspi", instead of "qspi.0" > >> I have tried to use -1 ID instead of 0, and then I saw that QSPI did not > >> contact to Flash Memory. > >> I found that in QSPI driver, ID is assigned to bus number: > >> master->bus_num = pdev->id. > >> So I wonder how to modify it. > > According to spi_register_master(), bus_num = -1 seems not problem. > > Did you exchanged clock name ? > I saw that in "struct spi_board_info" has defined bus_num is u16 > so I'm afraid if is -1 ID, master->bus_num did not match to bus_num in > board_info. Hmm... spi master seems use dynamic value as bus_num if ID was -1 (in spi_register_master()) So, using ID = 0 seems reasonable here 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
Good morning Morimoto-san Thanks for your replying! >>>>>> +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) >>>>>> +{ >>>>>> + platform_device_register_resndata( >>>>>> + &platform_bus, "qspi", 0, >>>>>> + qspi_resources, ARRAY_SIZE(qspi_resources), >>>>>> + pdata, sizeof(*pdata)); >>>>>> +} >>>>> Please use -1 ID, and use just "qspi", instead of "qspi.0" >>>> I have tried to use -1 ID instead of 0, and then I saw that QSPI did not >>>> contact to Flash Memory. >>>> I found that in QSPI driver, ID is assigned to bus number: >>>> master->bus_num = pdev->id. >>>> So I wonder how to modify it. >>> According to spi_register_master(), bus_num = -1 seems not problem. >>> Did you exchanged clock name ? >> I saw that in "struct spi_board_info" has defined bus_num is u16 >> so I'm afraid if is -1 ID, master->bus_num did not match to bus_num in >> board_info. > Hmm... > spi master seems use dynamic value > as bus_num if ID was -1 (in spi_register_master()) > So, using ID = 0 seems reasonable here Thanks, so I'll use ID = 0 as you pointed out to me! Hi Simon-san, Magnus-san I will update my patch according to the feedback from Morimoto-san, Magnus-san and you today. Best Regards, Hiep. -- 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
diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c index fc36d3d..baabceb 100644 --- a/arch/arm/mach-shmobile/clock-r8a7790.c +++ b/arch/arm/mach-shmobile/clock-r8a7790.c @@ -52,6 +52,7 @@ #define SMSTPCR5 0xe6150144 #define SMSTPCR7 0xe615014c #define SMSTPCR8 0xe6150990 +#define SMSTPCR9 0xE6150994 #define SDCKCR 0xE6150074 #define SD2CKCR 0xE6150078 @@ -181,6 +182,7 @@ static struct clk div6_clks[DIV6_NR] = { /* MSTP */ enum { + MSTP917, MSTP813, MSTP721, MSTP720, MSTP717, MSTP716, @@ -192,6 +194,7 @@ enum { }; static struct clk mstp_clks[MSTP_NR] = { + [MSTP917] = SH_CLK_MSTP32(&qspi_clk, SMSTPCR9, 17, 0), /* QSPI */ [MSTP813] = SH_CLK_MSTP32(&p_clk, SMSTPCR8, 13, 0), /* Ether */ [MSTP721] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 21, 0), /* SCIF0 */ [MSTP720] = SH_CLK_MSTP32(&p_clk, SMSTPCR7, 20, 0), /* SCIF1 */ @@ -276,6 +279,7 @@ static struct clk_lookup lookups[] = { CLKDEV_DEV_ID("ee220000.mmcif", &mstp_clks[MSTP305]), CLKDEV_DEV_ID("sh_mmcif.1", &mstp_clks[MSTP305]), CLKDEV_DEV_ID("sh_cmt.0", &mstp_clks[MSTP124]), + CLKDEV_DEV_ID("qspi.0", &mstp_clks[MSTP917]), }; #define R8A7790_CLOCK_ROOT(e, m, p0, p1, p30, p31) \ diff --git a/arch/arm/mach-shmobile/include/mach/r8a7790.h b/arch/arm/mach-shmobile/include/mach/r8a7790.h index 788d559..d590cad 100644 --- a/arch/arm/mach-shmobile/include/mach/r8a7790.h +++ b/arch/arm/mach-shmobile/include/mach/r8a7790.h @@ -1,5 +1,6 @@ #ifndef __ASM_R8A7790_H__ #define __ASM_R8A7790_H__ +#include <linux/spi/rspi.h> void r8a7790_add_standard_devices(void); void r8a7790_add_dt_devices(void); @@ -7,6 +8,7 @@ void r8a7790_clock_init(void); void r8a7790_pinmux_init(void); void r8a7790_init_delay(void); void r8a7790_timer_init(void); +void r8a7790_add_qspi_device(struct rspi_plat_data *pdata); #define MD(nr) BIT(nr) u32 r8a7790_read_mode_pins(void); diff --git a/arch/arm/mach-shmobile/setup-r8a7790.c b/arch/arm/mach-shmobile/setup-r8a7790.c index d0f5c9f..30e3362 100644 --- a/arch/arm/mach-shmobile/setup-r8a7790.c +++ b/arch/arm/mach-shmobile/setup-r8a7790.c @@ -30,6 +30,22 @@ #include <mach/irqs.h> #include <mach/r8a7790.h> #include <asm/mach/arch.h> +#include <linux/spi/spi.h> +#include <linux/spi/rspi.h> + +/* QSPI resource */ +static struct resource qspi_resources[] __initdata = { + DEFINE_RES_MEM(0xe6b10000, 0x1000), + DEFINE_RES_IRQ(gic_spi(184)), +}; + +void __init r8a7790_add_qspi_device(struct rspi_plat_data *pdata) +{ + platform_device_register_resndata( + &platform_bus, "qspi", 0, + qspi_resources, ARRAY_SIZE(qspi_resources), + pdata, sizeof(*pdata)); +} static struct resource pfc_resources[] __initdata = { DEFINE_RES_MEM(0xe6060000, 0x250),