* [PATCH 1/2] ARM: SAMSUNG: Added tx_st_done variable in the platform data of SPI @ 2011-06-30 12:38 Padmavathi Venna 2011-06-30 12:38 ` [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level Padmavathi Venna 0 siblings, 1 reply; 21+ messages in thread From: Padmavathi Venna @ 2011-06-30 12:38 UTC (permalink / raw) To: kgene.kim, jassisinghbrar, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm tx_st_done is required for checking the transmission status of SPI channels with different fifo levels Signed-off-by: Padmavathi Venna <padma.v@samsung.com> --- arch/arm/mach-s3c64xx/mach-smdk6410.c | 2 ++ arch/arm/mach-s5p64x0/mach-smdk6440.c | 2 ++ arch/arm/mach-s5p64x0/mach-smdk6450.c | 2 ++ arch/arm/mach-s5pc100/mach-smdkc100.c | 3 +++ arch/arm/mach-s5pv210/mach-smdkv210.c | 2 ++ arch/arm/plat-samsung/dev-spi0.c | 1 + arch/arm/plat-samsung/dev-spi1.c | 1 + arch/arm/plat-samsung/dev-spi2.c | 1 + arch/arm/plat-samsung/include/plat/s3c64xx-spi.h | 2 ++ 9 files changed, 16 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-s3c64xx/mach-smdk6410.c b/arch/arm/mach-s3c64xx/mach-smdk6410.c index ff31a3d..dce89aa 100644 --- a/arch/arm/mach-s3c64xx/mach-smdk6410.c +++ b/arch/arm/mach-s3c64xx/mach-smdk6410.c @@ -212,11 +212,13 @@ static struct platform_device smdk6410_smsc911x = { static struct s3c64xx_spi_info s3c_spi0_pdata __initdata = { .fifo_lvl_mask = 0x7f, .rx_lvl_offset = 13, + .tx_st_done = 21, }; static struct s3c64xx_spi_info s3c_spi1_pdata __initdata = { .fifo_lvl_mask = 0x7f, .rx_lvl_offset = 13, + .tx_st_done = 21, }; #ifdef CONFIG_REGULATOR diff --git a/arch/arm/mach-s5p64x0/mach-smdk6440.c b/arch/arm/mach-s5p64x0/mach-smdk6440.c index 03469bc..7488feb 100644 --- a/arch/arm/mach-s5p64x0/mach-smdk6440.c +++ b/arch/arm/mach-s5p64x0/mach-smdk6440.c @@ -134,11 +134,13 @@ static struct platform_device smdk6440_backlight_device = { static struct s3c64xx_spi_info s3c_spi0_pdata __initdata = { .fifo_lvl_mask = 0x1ff, .rx_lvl_offset = 15, + .tx_st_done = 25, }; static struct s3c64xx_spi_info s3c_spi1_pdata __initdata = { .fifo_lvl_mask = 0x7f, .rx_lvl_offset = 15, + .tx_st_done = 25, }; static struct platform_device *smdk6440_devices[] __initdata = { diff --git a/arch/arm/mach-s5p64x0/mach-smdk6450.c b/arch/arm/mach-s5p64x0/mach-smdk6450.c index 4048447..e7b2560 100644 --- a/arch/arm/mach-s5p64x0/mach-smdk6450.c +++ b/arch/arm/mach-s5p64x0/mach-smdk6450.c @@ -152,11 +152,13 @@ static struct platform_device smdk6450_backlight_device = { static struct s3c64xx_spi_info s3c_spi0_pdata __initdata = { .fifo_lvl_mask = 0x1ff, .rx_lvl_offset = 15, + .tx_st_done = 25, }; static struct s3c64xx_spi_info s3c_spi1_pdata __initdata = { .fifo_lvl_mask = 0x7f, .rx_lvl_offset = 15, + .tx_st_done = 25, }; static struct platform_device *smdk6450_devices[] __initdata = { diff --git a/arch/arm/mach-s5pc100/mach-smdkc100.c b/arch/arm/mach-s5pc100/mach-smdkc100.c index 127f7b6..4e87530 100644 --- a/arch/arm/mach-s5pc100/mach-smdkc100.c +++ b/arch/arm/mach-s5pc100/mach-smdkc100.c @@ -223,18 +223,21 @@ static struct s3c64xx_spi_info s3c_spi0_pdata __initdata = { .fifo_lvl_mask = 0x7f, .rx_lvl_offset = 13, .high_speed = 1, + .tx_st_done = 21, }; static struct s3c64xx_spi_info s3c_spi1_pdata __initdata = { .fifo_lvl_mask = 0x7f, .rx_lvl_offset = 13, .high_speed = 1, + .tx_st_done = 21, }; static struct s3c64xx_spi_info s3c_spi2_pdata __initdata = { .fifo_lvl_mask = 0x7f, .rx_lvl_offset = 13, .high_speed = 1, + .tx_st_done = 21, }; static struct platform_device *smdkc100_devices[] __initdata = { diff --git a/arch/arm/mach-s5pv210/mach-smdkv210.c b/arch/arm/mach-s5pv210/mach-smdkv210.c index 8978757..71af473 100644 --- a/arch/arm/mach-s5pv210/mach-smdkv210.c +++ b/arch/arm/mach-s5pv210/mach-smdkv210.c @@ -254,12 +254,14 @@ static struct s3c64xx_spi_info s3c_spi0_pdata __initdata = { .fifo_lvl_mask = 0x1ff, .rx_lvl_offset = 15, .high_speed = 1, + .tx_st_done = 25, }; static struct s3c64xx_spi_info s3c_spi1_pdata __initdata = { .fifo_lvl_mask = 0x7f, .rx_lvl_offset = 15, .high_speed = 1, + .tx_st_done = 25, }; static struct platform_device *smdkv210_devices[] __initdata = { &s3c_device_adc, diff --git a/arch/arm/plat-samsung/dev-spi0.c b/arch/arm/plat-samsung/dev-spi0.c index 3d2c8bf..18f379e 100644 --- a/arch/arm/plat-samsung/dev-spi0.c +++ b/arch/arm/plat-samsung/dev-spi0.c @@ -71,6 +71,7 @@ void __init s3c_spi0_set_info(struct s3c64xx_spi_info *pd, } spi_info->fifo_lvl_mask = pd->fifo_lvl_mask; spi_info->rx_lvl_offset = pd->rx_lvl_offset; + spi_info->tx_st_done = pd->tx_st_done; /* Reject invalid configuration */ if (!num_cs || src_clk_nr < 0 || src_clk_nr > ARRAY_SIZE(spi_src_clks)) { diff --git a/arch/arm/plat-samsung/dev-spi1.c b/arch/arm/plat-samsung/dev-spi1.c index 2aaa251..2a80c5d 100644 --- a/arch/arm/plat-samsung/dev-spi1.c +++ b/arch/arm/plat-samsung/dev-spi1.c @@ -71,6 +71,7 @@ void __init s3c_spi1_set_info(struct s3c64xx_spi_info *pd, } spi_info->fifo_lvl_mask = pd->fifo_lvl_mask; spi_info->rx_lvl_offset = pd->rx_lvl_offset; + spi_info->tx_st_done = pd->tx_st_done; /* Reject invalid configuration */ if (!num_cs || src_clk_nr < 0 || src_clk_nr > ARRAY_SIZE(spi_src_clks)) { diff --git a/arch/arm/plat-samsung/dev-spi2.c b/arch/arm/plat-samsung/dev-spi2.c index 15218b5..5430738 100644 --- a/arch/arm/plat-samsung/dev-spi2.c +++ b/arch/arm/plat-samsung/dev-spi2.c @@ -71,6 +71,7 @@ void __init s3c_spi2_set_info(struct s3c64xx_spi_info *pd, } spi_info->fifo_lvl_mask = pd->fifo_lvl_mask; spi_info->rx_lvl_offset = pd->rx_lvl_offset; + spi_info->tx_st_done = pd->tx_st_done; /* Reject invalid configuration */ if (!num_cs || src_clk_nr < 0 || src_clk_nr > ARRAY_SIZE(spi_src_clks)) { diff --git a/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h b/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h index b9f6a4b..551ae27 100644 --- a/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h +++ b/arch/arm/plat-samsung/include/plat/s3c64xx-spi.h @@ -39,6 +39,7 @@ struct s3c64xx_spi_csinfo { * @fifo_lvl_mask: All tx fifo_lvl fields start at offset-6 * @rx_lvl_offset: Depends on tx fifo_lvl field and bus number * @high_speed: If the controller supports HIGH_SPEED_EN bit + * @tx_st_done: Depends on tx fifo_lvl field */ struct s3c64xx_spi_info { int src_clk_nr; @@ -53,6 +54,7 @@ struct s3c64xx_spi_info { int fifo_lvl_mask; int rx_lvl_offset; int high_speed; + int tx_st_done; }; /** -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-06-30 12:38 [PATCH 1/2] ARM: SAMSUNG: Added tx_st_done variable in the platform data of SPI Padmavathi Venna @ 2011-06-30 12:38 ` Padmavathi Venna 2011-06-30 7:08 ` Jassi Brar 0 siblings, 1 reply; 21+ messages in thread From: Padmavathi Venna @ 2011-06-30 12:38 UTC (permalink / raw) To: kgene.kim, jassisinghbrar, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm Fixed the bug in transmission status check for 64 bytes FIFO level. Signed-off-by: Padmavathi Venna <padma.v@samsung.com> --- drivers/spi/spi_s3c64xx.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c index 795828b..8945e20 100644 --- a/drivers/spi/spi_s3c64xx.c +++ b/drivers/spi/spi_s3c64xx.c @@ -116,9 +116,7 @@ (((i)->fifo_lvl_mask + 1))) \ ? 1 : 0) -#define S3C64XX_SPI_ST_TX_DONE(v, i) ((((v) >> (i)->rx_lvl_offset) & \ - (((i)->fifo_lvl_mask + 1) << 1)) \ - ? 1 : 0) +#define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & (1 << (i)->tx_st_done)) ? 1 : 0) #define TX_FIFO_LVL(v, i) (((v) >> 6) & (i)->fifo_lvl_mask) #define RX_FIFO_LVL(v, i) (((v) >> (i)->rx_lvl_offset) & (i)->fifo_lvl_mask) -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-06-30 12:38 ` [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level Padmavathi Venna @ 2011-06-30 7:08 ` Jassi Brar 2011-06-30 9:05 ` padma venkat 0 siblings, 1 reply; 21+ messages in thread From: Jassi Brar @ 2011-06-30 7:08 UTC (permalink / raw) To: Padmavathi Venna Cc: kgene.kim, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm-kernel On Thu, Jun 30, 2011 at 6:08 PM, Padmavathi Venna <padma.v@samsung.com> wrote: > Fixed the bug in transmission status check for 64 bytes FIFO > level. > > Signed-off-by: Padmavathi Venna <padma.v@samsung.com> > --- > drivers/spi/spi_s3c64xx.c | 4 +--- > 1 files changed, 1 insertions(+), 3 deletions(-) > > diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.c > index 795828b..8945e20 100644 > --- a/drivers/spi/spi_s3c64xx.c > +++ b/drivers/spi/spi_s3c64xx.c > @@ -116,9 +116,7 @@ > (((i)->fifo_lvl_mask + 1))) \ > ? 1 : 0) > > -#define S3C64XX_SPI_ST_TX_DONE(v, i) ((((v) >> (i)->rx_lvl_offset) & \ > - (((i)->fifo_lvl_mask + 1) << 1)) \ > - ? 1 : 0) > +#define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & (1 << (i)->tx_st_done)) ? 1 : 0) IIRC the macro is already designed to deduct tx-done levels from other fields. Could you please _explain_ with one example where it fails ? It is difficult to see without numbers. Thanks, -Jassi ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-06-30 7:08 ` Jassi Brar @ 2011-06-30 9:05 ` padma venkat 2011-06-30 9:52 ` Jassi Brar 0 siblings, 1 reply; 21+ messages in thread From: padma venkat @ 2011-06-30 9:05 UTC (permalink / raw) To: Jassi Brar Cc: Padmavathi Venna, kgene.kim, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm-kernel Hi, On Thu, Jun 30, 2011 at 12:38 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > On Thu, Jun 30, 2011 at 6:08 PM, Padmavathi Venna <padma.v@samsung.com> wrote: >> Fixed the bug in transmission status check for 64 bytes FIFO >> level. >> >> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> >> --- >> drivers/spi/spi_s3c64xx.c | 4 +--- >> 1 files changed, 1 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.hc >> index 795828b..8945e20 100644 >> --- a/drivers/spi/spi_s3c64xx.c >> +++ b/drivers/spi/spi_s3c64xx.c >> @@ -116,9 +116,7 @@ >> (((i)->fifo_lvl_mask + 1))) \ >> ? 1 : 0) >> >> -#define S3C64XX_SPI_ST_TX_DONE(v, i) ((((v) >> (i)->rx_lvl_offset) & \ >> - (((i)->fifo_lvl_mask + 1) << 1)) \ >> - ? 1 : 0) >> +#define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & (1 << (i)->tx_st_done)) ? 1 : 0) > > IIRC the macro is already designed to deduct tx-done levels from other fields. > Could you please _explain_ with one example where it fails ? It is > difficult to see without > numbers. The existing macro fails for following scenarios. 1) S5P64X0 channel 1 2) S5PV210 channel 1 3) S5PV310 channel 1 and channel 2 The FIFO data level supported in the above SoCs either 64 or 256 bytes depending on the channel. Because of this the TX_DONE is the 25 bit in the status register. The existing macro works for the following scenarios 1) S3C6410 all channels 2) S5PC100 all channels The FIFO data level supported in the above SoCs 64 bytes on all the channels. Because of this the TX_DONE is the 21 bit in the status register. So when we use the existing macro for the non-working SoCs it is not anding with the TX_DONE bit but it is only anding the bits earlier to TX_DONE bit. > > Thanks, > -Jassi > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-06-30 9:05 ` padma venkat @ 2011-06-30 9:52 ` Jassi Brar [not found] ` <BANLkTi=9c5asfr=Jg6xwjQZW_a369UO64Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 21+ messages in thread From: Jassi Brar @ 2011-06-30 9:52 UTC (permalink / raw) To: padma venkat Cc: Padmavathi Venna, kgene.kim, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm-kernel On Thu, Jun 30, 2011 at 2:35 PM, padma venkat <padma.kvr@gmail.com> wrote: > Hi, > > On Thu, Jun 30, 2011 at 12:38 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >> On Thu, Jun 30, 2011 at 6:08 PM, Padmavathi Venna <padma.v@samsung.com> wrote: >>> Fixed the bug in transmission status check for 64 bytes FIFO >>> level. >>> >>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> >>> --- >>> drivers/spi/spi_s3c64xx.c | 4 +--- >>> 1 files changed, 1 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.hc >>> index 795828b..8945e20 100644 >>> --- a/drivers/spi/spi_s3c64xx.c >>> +++ b/drivers/spi/spi_s3c64xx.c >>> @@ -116,9 +116,7 @@ >>> (((i)->fifo_lvl_mask + 1))) \ >>> ? 1 : 0) >>> >>> -#define S3C64XX_SPI_ST_TX_DONE(v, i) ((((v) >> (i)->rx_lvl_offset) & \ >>> - (((i)->fifo_lvl_mask + 1) << 1)) \ >>> - ? 1 : 0) >>> +#define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & (1 << (i)->tx_st_done)) ? 1 : 0) >> >> IIRC the macro is already designed to deduct tx-done levels from other fields. >> Could you please _explain_ with one example where it fails ? It is >> difficult to see without >> numbers. > The existing macro fails for following scenarios. > 1) S5P64X0 channel 1 > 2) S5PV210 channel 1 > 3) S5PV310 channel 1 and channel 2 > > The FIFO data level supported in the above SoCs either 64 or 256 > bytes depending on the channel. Because of this the TX_DONE > is the 25 bit in the status register. > > The existing macro works for the following scenarios > 1) S3C6410 all channels > 2) S5PC100 all channels > > The FIFO data level supported in the above SoCs 64 bytes > on all the channels. Because of this the TX_DONE is the 21 bit > in the status register. > > So when we use the existing macro for the non-working SoCs > it is not anding with the TX_DONE bit but it is only anding the bits > earlier to TX_DONE bit. > I see. I don't have access to post s3c64xx datasheets. Please confirm if TX_DONE bit at same offset for all channels of an SoC. If so, I am OK with these 2 patches. Thanks, Jassi ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <BANLkTi=9c5asfr=Jg6xwjQZW_a369UO64Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level [not found] ` <BANLkTi=9c5asfr=Jg6xwjQZW_a369UO64Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2011-06-30 11:00 ` Tony Nadackal 2011-07-01 5:46 ` padma venkat 0 siblings, 1 reply; 21+ messages in thread From: Tony Nadackal @ 2011-06-30 11:00 UTC (permalink / raw) To: padma venkat, Padmavathi Venna Cc: kgene.kim-Sze3O3UU22JBDgjK7y7TUQ, sbkim73-Sze3O3UU22JBDgjK7y7TUQ, naushad-Sze3O3UU22JBDgjK7y7TUQ, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, tony.kn-Sze3O3UU22JBDgjK7y7TUQ Hi Padma, With regards to your patch, even though one can check the tx done status using the TX_DONE bit, the present macro itself would work perfectly fine if the 'fifo_lvl_mask' is set properly. For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff (for 9bits, 15:23), while even in your patch, it is wrongly set as 0x7f(only 7bits). Thus, if this fifo_lvl_mask was defined correctly, the existing macro would itself have worked. Thanks, Tony On Thu, Jun 30, 2011 at 3:22 PM, Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>wrote: > On Thu, Jun 30, 2011 at 2:35 PM, padma venkat <padma.kvr-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > > Hi, > > > > On Thu, Jun 30, 2011 at 12:38 PM, Jassi Brar <jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > wrote: > >> On Thu, Jun 30, 2011 at 6:08 PM, Padmavathi Venna <padma.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > wrote: > >>> Fixed the bug in transmission status check for 64 bytes FIFO > >>> level. > >>> > >>> Signed-off-by: Padmavathi Venna <padma.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> > >>> --- > >>> drivers/spi/spi_s3c64xx.c | 4 +--- > >>> 1 files changed, 1 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.hc > >>> index 795828b..8945e20 100644 > >>> --- a/drivers/spi/spi_s3c64xx.c > >>> +++ b/drivers/spi/spi_s3c64xx.c > >>> @@ -116,9 +116,7 @@ > >>> (((i)->fifo_lvl_mask + 1))) \ > >>> ? 1 : 0) > >>> > >>> -#define S3C64XX_SPI_ST_TX_DONE(v, i) ((((v) >> (i)->rx_lvl_offset) & \ > >>> - (((i)->fifo_lvl_mask + 1) << > 1)) \ > >>> - ? 1 : 0) > >>> +#define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & (1 << (i)->tx_st_done)) ? > 1 : 0) > >> > >> IIRC the macro is already designed to deduct tx-done levels from other > fields. > >> Could you please _explain_ with one example where it fails ? It is > >> difficult to see without > >> numbers. > > The existing macro fails for following scenarios. > > 1) S5P64X0 channel 1 > > 2) S5PV210 channel 1 > > 3) S5PV310 channel 1 and channel 2 > > > > The FIFO data level supported in the above SoCs either 64 or 256 > > bytes depending on the channel. Because of this the TX_DONE > > is the 25 bit in the status register. > > > > The existing macro works for the following scenarios > > 1) S3C6410 all channels > > 2) S5PC100 all channels > > > > The FIFO data level supported in the above SoCs 64 bytes > > on all the channels. Because of this the TX_DONE is the 21 bit > > in the status register. > > > > So when we use the existing macro for the non-working SoCs > > it is not anding with the TX_DONE bit but it is only anding the bits > > earlier to TX_DONE bit. > > > > I see. > I don't have access to post s3c64xx datasheets. Please confirm if TX_DONE > bit at same offset for all channels of an SoC. If so, I am OK with > these 2 patches. > > Thanks, > Jassi > -- > To unsubscribe from this list: send the line "unsubscribe > linux-samsung-soc" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ------------------------------------------------------------------------------ All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2d-c2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-06-30 11:00 ` Tony Nadackal @ 2011-07-01 5:46 ` padma venkat 2011-07-01 5:52 ` Jassi Brar 0 siblings, 1 reply; 21+ messages in thread From: padma venkat @ 2011-07-01 5:46 UTC (permalink / raw) To: Tony Nadackal Cc: Padmavathi Venna, Jassi Brar, kgene.kim, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad Hi Tony, On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> wrote: > Hi Padma, > With regards to your patch, even though one can check the tx done status > using the TX_DONE bit, the present macro itself would work perfectly fine if > the 'fifo_lvl_mask' is set properly. > For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff (for 9bits, > 15:23), while even in your patch, it is wrongly set as 0x7f(only 7bits). > > Thus, if this fifo_lvl_mask was defined correctly, the existing macro would > itself have worked. Thanks for your comment. I considered changing to the fifo_lvl_mask to 1ff as you mentioned. But I think that the fifo_lvl_mask reflects the actual FIFO capacity in the SPI driver. For the failing channels the FIFO trigger level is 64 bytes and so i retained that value. In the driver it polls till the FIFO capacity level otherwise it goes for DMA.So if we keep the FIFO level as 1ff when the actual capacity is 7f then it fails. Jassi what do you think about this? Thanks&Regards Padma > Thanks, > Tony > > On Thu, Jun 30, 2011 at 3:22 PM, Jassi Brar <jassisinghbrar@gmail.com> > wrote: >> >> On Thu, Jun 30, 2011 at 2:35 PM, padma venkat <padma.kvr@gmail.com> wrote: >> > Hi, >> > >> > On Thu, Jun 30, 2011 at 12:38 PM, Jassi Brar <jassisinghbrar@gmail.com> >> > wrote: >> >> On Thu, Jun 30, 2011 at 6:08 PM, Padmavathi Venna <padma.v@samsung.com> >> >> wrote: >> >>> Fixed the bug in transmission status check for 64 bytes FIFO >> >>> level. >> >>> >> >>> Signed-off-by: Padmavathi Venna <padma.v@samsung.com> >> >>> --- >> >>> drivers/spi/spi_s3c64xx.c | 4 +--- >> >>> 1 files changed, 1 insertions(+), 3 deletions(-) >> >>> >> >>> diff --git a/drivers/spi/spi_s3c64xx.c b/drivers/spi/spi_s3c64xx.hc >> >>> index 795828b..8945e20 100644 >> >>> --- a/drivers/spi/spi_s3c64xx.c >> >>> +++ b/drivers/spi/spi_s3c64xx.c >> >>> @@ -116,9 +116,7 @@ >> >>> (((i)->fifo_lvl_mask + 1))) \ >> >>> ? 1 : 0) >> >>> >> >>> -#define S3C64XX_SPI_ST_TX_DONE(v, i) ((((v) >> (i)->rx_lvl_offset) & >> >>> \ >> >>> - (((i)->fifo_lvl_mask + 1) << >> >>> 1)) \ >> >>> - ? 1 : 0) >> >>> +#define S3C64XX_SPI_ST_TX_DONE(v, i) (((v) & (1 << (i)->tx_st_done)) >> >>> ? 1 : 0) >> >> >> >> IIRC the macro is already designed to deduct tx-done levels from other >> >> fields. >> >> Could you please _explain_ with one example where it fails ? It is >> >> difficult to see without >> >> numbers. >> > The existing macro fails for following scenarios. >> > 1) S5P64X0 channel 1 >> > 2) S5PV210 channel 1 >> > 3) S5PV310 channel 1 and channel 2 >> > >> > The FIFO data level supported in the above SoCs either 64 or 256 >> > bytes depending on the channel. Because of this the TX_DONE >> > is the 25 bit in the status register. >> > >> > The existing macro works for the following scenarios >> > 1) S3C6410 all channels >> > 2) S5PC100 all channels >> > >> > The FIFO data level supported in the above SoCs 64 bytes >> > on all the channels. Because of this the TX_DONE is the 21 bit >> > in the status register. >> > >> > So when we use the existing macro for the non-working SoCs >> > it is not anding with the TX_DONE bit but it is only anding the bits >> > earlier to TX_DONE bit. >> > >> >> I see. >> I don't have access to post s3c64xx datasheets. Please confirm if TX_DONE >> bit at same offset for all channels of an SoC. If so, I am OK with >> these 2 patches. >> >> Thanks, >> Jassi >> -- >> To unsubscribe from this list: send the line "unsubscribe >> linux-samsung-soc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-01 5:46 ` padma venkat @ 2011-07-01 5:52 ` Jassi Brar 2011-07-01 5:59 ` padma venkat 0 siblings, 1 reply; 21+ messages in thread From: Jassi Brar @ 2011-07-01 5:52 UTC (permalink / raw) To: padma venkat Cc: Tony Nadackal, Padmavathi Venna, kgene.kim, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad On Fri, Jul 1, 2011 at 11:16 AM, padma venkat <padma.kvr@gmail.com> wrote: > Hi Tony, > > On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> wrote: >> Hi Padma, >> With regards to your patch, even though one can check the tx done status >> using the TX_DONE bit, the present macro itself would work perfectly fine if >> the 'fifo_lvl_mask' is set properly. >> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff (for 9bits, >> 15:23), while even in your patch, it is wrongly set as 0x7f(only 7bits). >> >> Thus, if this fifo_lvl_mask was defined correctly, the existing macro would >> itself have worked. > Thanks for your comment. > I considered changing to the fifo_lvl_mask to 1ff as you mentioned. > But I think that the fifo_lvl_mask reflects the actual FIFO capacity > in the SPI driver. > For the failing channels the FIFO trigger level is 64 bytes and so i > retained that value. > In the driver it polls till the FIFO capacity level otherwise it goes > for DMA.So if we keep > the FIFO level as 1ff when the actual capacity is 7f then it fails. > > Jassi what do you think about this? > 'fifo_lvl_mask' is h/w specific and can't be set for convenience. I don't have access to post-s3c64xx datasheets. Please check and reply if TX_DONE bit is at same offset for all channels of an SoC, because I suspect it's otherwise. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-01 5:52 ` Jassi Brar @ 2011-07-01 5:59 ` padma venkat 2011-07-01 6:13 ` Jassi Brar 0 siblings, 1 reply; 21+ messages in thread From: padma venkat @ 2011-07-01 5:59 UTC (permalink / raw) To: Jassi Brar Cc: Tony Nadackal, Padmavathi Venna, kgene.kim, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad Hi Jassi, On Fri, Jul 1, 2011 at 11:22 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > On Fri, Jul 1, 2011 at 11:16 AM, padma venkat <padma.kvr@gmail.com> wrote: >> Hi Tony, >> >> On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> wrote: >>> Hi Padma, >>> With regards to your patch, even though one can check the tx done status >>> using the TX_DONE bit, the present macro itself would work perfectly fine if >>> the 'fifo_lvl_mask' is set properly. >>> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff (for 9bits, >>> 15:23), while even in your patch, it is wrongly set as 0x7f(only 7bits). >>> >>> Thus, if this fifo_lvl_mask was defined correctly, the existing macro would >>> itself have worked. >> Thanks for your comment. >> I considered changing to the fifo_lvl_mask to 1ff as you mentioned. >> But I think that the fifo_lvl_mask reflects the actual FIFO capacity >> in the SPI driver. >> For the failing channels the FIFO trigger level is 64 bytes and so i >> retained that value. >> In the driver it polls till the FIFO capacity level otherwise it goes >> for DMA.So if we keep >> the FIFO level as 1ff when the actual capacity is 7f then it fails. >> >> Jassi what do you think about this? >> > > 'fifo_lvl_mask' is h/w specific and can't be set for convenience. > > I don't have access to post-s3c64xx datasheets. > Please check and reply if TX_DONE bit is at same offset for all > channels of an SoC, because > I suspect it's otherwise. > Yes. The TX_DONE bit is at the same offset for all the channels of an SoC. in S5P64X0,S5PV210 and S5PV310 it is at offset 25. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-01 5:59 ` padma venkat @ 2011-07-01 6:13 ` Jassi Brar 2011-07-04 6:55 ` Grant Likely 0 siblings, 1 reply; 21+ messages in thread From: Jassi Brar @ 2011-07-01 6:13 UTC (permalink / raw) To: padma venkat Cc: Tony Nadackal, Padmavathi Venna, kgene.kim, sbkim73, grant.likely, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad On Fri, Jul 1, 2011 at 11:29 AM, padma venkat <padma.kvr@gmail.com> wrote: > Hi Jassi, > > On Fri, Jul 1, 2011 at 11:22 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >> On Fri, Jul 1, 2011 at 11:16 AM, padma venkat <padma.kvr@gmail.com> wrote: >>> Hi Tony, >>> >>> On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> wrote: >>>> Hi Padma, >>>> With regards to your patch, even though one can check the tx done status >>>> using the TX_DONE bit, the present macro itself would work perfectly fine if >>>> the 'fifo_lvl_mask' is set properly. >>>> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff (for 9bits, >>>> 15:23), while even in your patch, it is wrongly set as 0x7f(only 7bits). >>>> >>>> Thus, if this fifo_lvl_mask was defined correctly, the existing macro would >>>> itself have worked. >>> Thanks for your comment. >>> I considered changing to the fifo_lvl_mask to 1ff as you mentioned. >>> But I think that the fifo_lvl_mask reflects the actual FIFO capacity >>> in the SPI driver. >>> For the failing channels the FIFO trigger level is 64 bytes and so i >>> retained that value. >>> In the driver it polls till the FIFO capacity level otherwise it goes >>> for DMA.So if we keep >>> the FIFO level as 1ff when the actual capacity is 7f then it fails. >>> >>> Jassi what do you think about this? >>> >> >> 'fifo_lvl_mask' is h/w specific and can't be set for convenience. >> >> I don't have access to post-s3c64xx datasheets. >> Please check and reply if TX_DONE bit is at same offset for all >> channels of an SoC, because >> I suspect it's otherwise. >> > Yes. The TX_DONE bit is at the same offset for all the channels of an SoC. > in S5P64X0,S5PV210 and S5PV310 it is at offset 25. > Then, Patches-1,2 Acked-by: Jassi Brar <jassisinghbrar@gmail.com> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-01 6:13 ` Jassi Brar @ 2011-07-04 6:55 ` Grant Likely 2011-07-04 6:58 ` Grant Likely 0 siblings, 1 reply; 21+ messages in thread From: Grant Likely @ 2011-07-04 6:55 UTC (permalink / raw) To: Jassi Brar Cc: padma venkat, Tony Nadackal, Padmavathi Venna, kgene.kim, sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad On Fri, Jul 01, 2011 at 11:43:08AM +0530, Jassi Brar wrote: > On Fri, Jul 1, 2011 at 11:29 AM, padma venkat <padma.kvr@gmail.com> wrote: > > Hi Jassi, > > > > On Fri, Jul 1, 2011 at 11:22 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > >> On Fri, Jul 1, 2011 at 11:16 AM, padma venkat <padma.kvr@gmail.com> wrote: > >>> Hi Tony, > >>> > >>> On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> wrote: > >>>> Hi Padma, > >>>> With regards to your patch, even though one can check the tx done status > >>>> using the TX_DONE bit, the present macro itself would work perfectly fine if > >>>> the 'fifo_lvl_mask' is set properly. > >>>> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff (for 9bits, > >>>> 15:23), while even in your patch, it is wrongly set as 0x7f(only 7bits). > >>>> > >>>> Thus, if this fifo_lvl_mask was defined correctly, the existing macro would > >>>> itself have worked. > >>> Thanks for your comment. > >>> I considered changing to the fifo_lvl_mask to 1ff as you mentioned. > >>> But I think that the fifo_lvl_mask reflects the actual FIFO capacity > >>> in the SPI driver. > >>> For the failing channels the FIFO trigger level is 64 bytes and so i > >>> retained that value. > >>> In the driver it polls till the FIFO capacity level otherwise it goes > >>> for DMA.So if we keep > >>> the FIFO level as 1ff when the actual capacity is 7f then it fails. > >>> > >>> Jassi what do you think about this? > >>> > >> > >> 'fifo_lvl_mask' is h/w specific and can't be set for convenience. > >> > >> I don't have access to post-s3c64xx datasheets. > >> Please check and reply if TX_DONE bit is at same offset for all > >> channels of an SoC, because > >> I suspect it's otherwise. > >> > > Yes. The TX_DONE bit is at the same offset for all the channels of an SoC. > > in S5P64X0,S5PV210 and S5PV310 it is at offset 25. > > > > Then, Patches-1,2 > > Acked-by: Jassi Brar <jassisinghbrar@gmail.com> Are these bug fixes that should be in v3.0, or do I queue them up for v3.1? g. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-04 6:55 ` Grant Likely @ 2011-07-04 6:58 ` Grant Likely 2011-07-04 9:54 ` Kukjin Kim 0 siblings, 1 reply; 21+ messages in thread From: Grant Likely @ 2011-07-04 6:58 UTC (permalink / raw) To: Jassi Brar Cc: padma venkat, Tony Nadackal, Padmavathi Venna, kgene.kim, sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad On Mon, Jul 4, 2011 at 12:55 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > On Fri, Jul 01, 2011 at 11:43:08AM +0530, Jassi Brar wrote: >> On Fri, Jul 1, 2011 at 11:29 AM, padma venkat <padma.kvr@gmail.com> wrote: >> > Hi Jassi, >> > >> > On Fri, Jul 1, 2011 at 11:22 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >> >> On Fri, Jul 1, 2011 at 11:16 AM, padma venkat <padma.kvr@gmail.com> wrote: >> >>> Hi Tony, >> >>> >> >>> On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> wrote: >> >>>> Hi Padma, >> >>>> With regards to your patch, even though one can check the tx done status >> >>>> using the TX_DONE bit, the present macro itself would work perfectly fine if >> >>>> the 'fifo_lvl_mask' is set properly. >> >>>> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff (for 9bits, >> >>>> 15:23), while even in your patch, it is wrongly set as 0x7f(only 7bits). >> >>>> >> >>>> Thus, if this fifo_lvl_mask was defined correctly, the existing macro would >> >>>> itself have worked. >> >>> Thanks for your comment. >> >>> I considered changing to the fifo_lvl_mask to 1ff as you mentioned. >> >>> But I think that the fifo_lvl_mask reflects the actual FIFO capacity >> >>> in the SPI driver. >> >>> For the failing channels the FIFO trigger level is 64 bytes and so i >> >>> retained that value. >> >>> In the driver it polls till the FIFO capacity level otherwise it goes >> >>> for DMA.So if we keep >> >>> the FIFO level as 1ff when the actual capacity is 7f then it fails. >> >>> >> >>> Jassi what do you think about this? >> >>> >> >> >> >> 'fifo_lvl_mask' is h/w specific and can't be set for convenience. >> >> >> >> I don't have access to post-s3c64xx datasheets. >> >> Please check and reply if TX_DONE bit is at same offset for all >> >> channels of an SoC, because >> >> I suspect it's otherwise. >> >> >> > Yes. The TX_DONE bit is at the same offset for all the channels of an SoC. >> > in S5P64X0,S5PV210 and S5PV310 it is at offset 25. >> > >> >> Then, Patches-1,2 >> >> Acked-by: Jassi Brar <jassisinghbrar@gmail.com> > > Are these bug fixes that should be in v3.0, or do I queue them up for v3.1? Regardless, this one touches a lot of arch/arm files, so I'd rather see both patches go through the samsung tree: Acked-by: Grant Likely <grant.likely@secretlab.ca> > > g. > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-04 6:58 ` Grant Likely @ 2011-07-04 9:54 ` Kukjin Kim 2011-07-04 10:04 ` padma venkat 2011-07-04 10:08 ` Kukjin Kim 0 siblings, 2 replies; 21+ messages in thread From: Kukjin Kim @ 2011-07-04 9:54 UTC (permalink / raw) To: 'Grant Likely', 'Jassi Brar' Cc: 'padma venkat', 'Tony Nadackal', 'Padmavathi Venna', sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad Grant Likely wrote: > > On Mon, Jul 4, 2011 at 12:55 AM, Grant Likely <grant.likely@secretlab.ca> wrote: > > On Fri, Jul 01, 2011 at 11:43:08AM +0530, Jassi Brar wrote: > >> On Fri, Jul 1, 2011 at 11:29 AM, padma venkat <padma.kvr@gmail.com> wrote: > >> > Hi Jassi, > >> > > >> > On Fri, Jul 1, 2011 at 11:22 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > >> >> On Fri, Jul 1, 2011 at 11:16 AM, padma venkat <padma.kvr@gmail.com> > wrote: > >> >>> Hi Tony, > >> >>> > >> >>> On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> > wrote: > >> >>>> Hi Padma, > >> >>>> With regards to your patch, even though one can check the tx done > status > >> >>>> using the TX_DONE bit, the present macro itself would work perfectly > fine if > >> >>>> the 'fifo_lvl_mask' is set properly. > >> >>>> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff (for > 9bits, > >> >>>> 15:23), while even in your patch, it is wrongly set as 0x7f(only 7bits). > >> >>>> > >> >>>> Thus, if this fifo_lvl_mask was defined correctly, the existing macro > would > >> >>>> itself have worked. > >> >>> Thanks for your comment. > >> >>> I considered changing to the fifo_lvl_mask to 1ff as you mentioned. > >> >>> But I think that the fifo_lvl_mask reflects the actual FIFO capacity > >> >>> in the SPI driver. > >> >>> For the failing channels the FIFO trigger level is 64 bytes and so i > >> >>> retained that value. > >> >>> In the driver it polls till the FIFO capacity level otherwise it goes > >> >>> for DMA.So if we keep > >> >>> the FIFO level as 1ff when the actual capacity is 7f then it fails. > >> >>> > >> >>> Jassi what do you think about this? > >> >>> > >> >> > >> >> 'fifo_lvl_mask' is h/w specific and can't be set for convenience. > >> >> > >> >> I don't have access to post-s3c64xx datasheets. > >> >> Please check and reply if TX_DONE bit is at same offset for all > >> >> channels of an SoC, because > >> >> I suspect it's otherwise. > >> >> > >> > Yes. The TX_DONE bit is at the same offset for all the channels of an SoC. > >> > in S5P64X0,S5PV210 and S5PV310 it is at offset 25. > >> > > >> > >> Then, Patches-1,2 > >> > >> Acked-by: Jassi Brar <jassisinghbrar@gmail.com> > > > > Are these bug fixes that should be in v3.0, or do I queue them up for v3.1? > > Regardless, this one touches a lot of arch/arm files, so I'd rather > see both patches go through the samsung tree: > > Acked-by: Grant Likely <grant.likely@secretlab.ca> > Thanks Grant, Jassi and all, I will apply these 1 and 2 in my -fix tree for 3.0 with your acks. Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-04 9:54 ` Kukjin Kim @ 2011-07-04 10:04 ` padma venkat 2011-07-04 10:08 ` Kukjin Kim 1 sibling, 0 replies; 21+ messages in thread From: padma venkat @ 2011-07-04 10:04 UTC (permalink / raw) To: Kukjin Kim Cc: Grant Likely, Jassi Brar, Tony Nadackal, Padmavathi Venna, sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad Hi Kukjin, On Mon, Jul 4, 2011 at 3:24 PM, Kukjin Kim <kgene.kim@samsung.com> wrote: > Grant Likely wrote: >> >> On Mon, Jul 4, 2011 at 12:55 AM, Grant Likely <grant.likely@secretlab.ca> > wrote: >> > On Fri, Jul 01, 2011 at 11:43:08AM +0530, Jassi Brar wrote: >> >> On Fri, Jul 1, 2011 at 11:29 AM, padma venkat <padma.kvr@gmail.com> > wrote: >> >> > Hi Jassi, >> >> > >> >> > On Fri, Jul 1, 2011 at 11:22 AM, Jassi Brar > <jassisinghbrar@gmail.com> wrote: >> >> >> On Fri, Jul 1, 2011 at 11:16 AM, padma venkat <padma.kvr@gmail.com> >> wrote: >> >> >>> Hi Tony, >> >> >>> >> >> >>> On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> >> wrote: >> >> >>>> Hi Padma, >> >> >>>> With regards to your patch, even though one can check the tx done >> status >> >> >>>> using the TX_DONE bit, the present macro itself would work > perfectly >> fine if >> >> >>>> the 'fifo_lvl_mask' is set properly. >> >> >>>> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff > (for >> 9bits, >> >> >>>> 15:23), while even in your patch, it is wrongly set as 0x7f(only > 7bits). >> >> >>>> >> >> >>>> Thus, if this fifo_lvl_mask was defined correctly, the existing > macro >> would >> >> >>>> itself have worked. >> >> >>> Thanks for your comment. >> >> >>> I considered changing to the fifo_lvl_mask to 1ff as you mentioned. >> >> >>> But I think that the fifo_lvl_mask reflects the actual FIFO > capacity >> >> >>> in the SPI driver. >> >> >>> For the failing channels the FIFO trigger level is 64 bytes and so > i >> >> >>> retained that value. >> >> >>> In the driver it polls till the FIFO capacity level otherwise it > goes >> >> >>> for DMA.So if we keep >> >> >>> the FIFO level as 1ff when the actual capacity is 7f then it fails. >> >> >>> >> >> >>> Jassi what do you think about this? >> >> >>> >> >> >> >> >> >> 'fifo_lvl_mask' is h/w specific and can't be set for convenience. >> >> >> >> >> >> I don't have access to post-s3c64xx datasheets. >> >> >> Please check and reply if TX_DONE bit is at same offset for all >> >> >> channels of an SoC, because >> >> >> I suspect it's otherwise. >> >> >> >> >> > Yes. The TX_DONE bit is at the same offset for all the channels of an > SoC. >> >> > in S5P64X0,S5PV210 and S5PV310 it is at offset 25. >> >> > >> >> >> >> Then, Patches-1,2 >> >> >> >> Acked-by: Jassi Brar <jassisinghbrar@gmail.com> >> > >> > Are these bug fixes that should be in v3.0, or do I queue them up for > v3.1? >> >> Regardless, this one touches a lot of arch/arm files, so I'd rather >> see both patches go through the samsung tree: >> >> Acked-by: Grant Likely <grant.likely@secretlab.ca> >> > > Thanks Grant, Jassi and all, > > I will apply these 1 and 2 in my -fix tree for 3.0 with your acks. > > Thanks. > > Best regards, > Kgene. > -- > Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, > SW Solution Development Team, Samsung Electronics Co., Ltd. > > These patches are based on new SPI code(on which I tried to make common dev-spi for all SoCs). SO I will remake these two patches based on old code and will send the same. Thanks&Regards Padma ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-04 9:54 ` Kukjin Kim 2011-07-04 10:04 ` padma venkat @ 2011-07-04 10:08 ` Kukjin Kim 2011-07-04 15:06 ` Grant Likely 1 sibling, 1 reply; 21+ messages in thread From: Kukjin Kim @ 2011-07-04 10:08 UTC (permalink / raw) To: 'Kukjin Kim', 'Grant Likely', 'Jassi Brar' Cc: 'padma venkat', 'Tony Nadackal', 'Padmavathi Venna', sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad Kukjin Kim wrote: > > Grant Likely wrote: > > > > On Mon, Jul 4, 2011 at 12:55 AM, Grant Likely <grant.likely@secretlab.ca> > wrote: > > > On Fri, Jul 01, 2011 at 11:43:08AM +0530, Jassi Brar wrote: > > >> On Fri, Jul 1, 2011 at 11:29 AM, padma venkat <padma.kvr@gmail.com> > wrote: > > >> > Hi Jassi, > > >> > > > >> > On Fri, Jul 1, 2011 at 11:22 AM, Jassi Brar > <jassisinghbrar@gmail.com> wrote: > > >> >> On Fri, Jul 1, 2011 at 11:16 AM, padma venkat <padma.kvr@gmail.com> > > wrote: > > >> >>> Hi Tony, > > >> >>> > > >> >>> On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> > > wrote: > > >> >>>> Hi Padma, > > >> >>>> With regards to your patch, even though one can check the tx done > > status > > >> >>>> using the TX_DONE bit, the present macro itself would work > perfectly > > fine if > > >> >>>> the 'fifo_lvl_mask' is set properly. > > >> >>>> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff > (for > > 9bits, > > >> >>>> 15:23), while even in your patch, it is wrongly set as 0x7f(only > 7bits). > > >> >>>> > > >> >>>> Thus, if this fifo_lvl_mask was defined correctly, the existing > macro > > would > > >> >>>> itself have worked. > > >> >>> Thanks for your comment. > > >> >>> I considered changing to the fifo_lvl_mask to 1ff as you mentioned. > > >> >>> But I think that the fifo_lvl_mask reflects the actual FIFO > capacity > > >> >>> in the SPI driver. > > >> >>> For the failing channels the FIFO trigger level is 64 bytes and so > i > > >> >>> retained that value. > > >> >>> In the driver it polls till the FIFO capacity level otherwise it > goes > > >> >>> for DMA.So if we keep > > >> >>> the FIFO level as 1ff when the actual capacity is 7f then it fails. > > >> >>> > > >> >>> Jassi what do you think about this? > > >> >>> > > >> >> > > >> >> 'fifo_lvl_mask' is h/w specific and can't be set for convenience. > > >> >> > > >> >> I don't have access to post-s3c64xx datasheets. > > >> >> Please check and reply if TX_DONE bit is at same offset for all > > >> >> channels of an SoC, because > > >> >> I suspect it's otherwise. > > >> >> > > >> > Yes. The TX_DONE bit is at the same offset for all the channels of an > SoC. > > >> > in S5P64X0,S5PV210 and S5PV310 it is at offset 25. > > >> > > > >> > > >> Then, Patches-1,2 > > >> > > >> Acked-by: Jassi Brar <jassisinghbrar@gmail.com> > > > > > > Are these bug fixes that should be in v3.0, or do I queue them up for > v3.1? > > > > Regardless, this one touches a lot of arch/arm files, so I'd rather > > see both patches go through the samsung tree: > > > > Acked-by: Grant Likely <grant.likely@secretlab.ca> > > > > Thanks Grant, Jassi and all, > > I will apply these 1 and 2 in my -fix tree for 3.0 with your acks. > Oops, this needs previous 'cleanup spi platform specific code' so can't apply -fix tree now. Padmavathi, I will apply these 1 and 2 on -next for 3.1 after your updated 'cleanup spi' patches... If any problems, please let me know. Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-04 10:08 ` Kukjin Kim @ 2011-07-04 15:06 ` Grant Likely 2011-07-05 5:51 ` Kukjin Kim 2011-07-06 6:22 ` Kukjin Kim 0 siblings, 2 replies; 21+ messages in thread From: Grant Likely @ 2011-07-04 15:06 UTC (permalink / raw) To: Kukjin Kim Cc: 'Jassi Brar', 'padma venkat', 'Tony Nadackal', 'Padmavathi Venna', sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad On Mon, Jul 04, 2011 at 07:08:55PM +0900, Kukjin Kim wrote: > Kukjin Kim wrote: > > > > Grant Likely wrote: > > > > > > On Mon, Jul 4, 2011 at 12:55 AM, Grant Likely > <grant.likely@secretlab.ca> > > wrote: > > > > On Fri, Jul 01, 2011 at 11:43:08AM +0530, Jassi Brar wrote: > > > >> On Fri, Jul 1, 2011 at 11:29 AM, padma venkat <padma.kvr@gmail.com> > > wrote: > > > >> > Hi Jassi, > > > >> > > > > >> > On Fri, Jul 1, 2011 at 11:22 AM, Jassi Brar > > <jassisinghbrar@gmail.com> wrote: > > > >> >> On Fri, Jul 1, 2011 at 11:16 AM, padma venkat > <padma.kvr@gmail.com> > > > wrote: > > > >> >>> Hi Tony, > > > >> >>> > > > >> >>> On Thu, Jun 30, 2011 at 4:30 PM, Tony Nadackal <tonykn@gmail.com> > > > wrote: > > > >> >>>> Hi Padma, > > > >> >>>> With regards to your patch, even though one can check the tx > done > > > status > > > >> >>>> using the TX_DONE bit, the present macro itself would work > > perfectly > > > fine if > > > >> >>>> the 'fifo_lvl_mask' is set properly. > > > >> >>>> For example in 6450 channel 1, the fifo_lvl_mask should be 0x1ff > > (for > > > 9bits, > > > >> >>>> 15:23), while even in your patch, it is wrongly set as 0x7f(only > > 7bits). > > > >> >>>> > > > >> >>>> Thus, if this fifo_lvl_mask was defined correctly, the existing > > macro > > > would > > > >> >>>> itself have worked. > > > >> >>> Thanks for your comment. > > > >> >>> I considered changing to the fifo_lvl_mask to 1ff as you > mentioned. > > > >> >>> But I think that the fifo_lvl_mask reflects the actual FIFO > > capacity > > > >> >>> in the SPI driver. > > > >> >>> For the failing channels the FIFO trigger level is 64 bytes and > so > > i > > > >> >>> retained that value. > > > >> >>> In the driver it polls till the FIFO capacity level otherwise it > > goes > > > >> >>> for DMA.So if we keep > > > >> >>> the FIFO level as 1ff when the actual capacity is 7f then it > fails. > > > >> >>> > > > >> >>> Jassi what do you think about this? > > > >> >>> > > > >> >> > > > >> >> 'fifo_lvl_mask' is h/w specific and can't be set for convenience. > > > >> >> > > > >> >> I don't have access to post-s3c64xx datasheets. > > > >> >> Please check and reply if TX_DONE bit is at same offset for all > > > >> >> channels of an SoC, because > > > >> >> I suspect it's otherwise. > > > >> >> > > > >> > Yes. The TX_DONE bit is at the same offset for all the channels of > an > > SoC. > > > >> > in S5P64X0,S5PV210 and S5PV310 it is at offset 25. > > > >> > > > > >> > > > >> Then, Patches-1,2 > > > >> > > > >> Acked-by: Jassi Brar <jassisinghbrar@gmail.com> > > > > > > > > Are these bug fixes that should be in v3.0, or do I queue them up for > > v3.1? > > > > > > Regardless, this one touches a lot of arch/arm files, so I'd rather > > > see both patches go through the samsung tree: > > > > > > Acked-by: Grant Likely <grant.likely@secretlab.ca> > > > > > > > Thanks Grant, Jassi and all, > > > > I will apply these 1 and 2 in my -fix tree for 3.0 with your acks. > > > Oops, this needs previous 'cleanup spi platform specific code' so can't > apply -fix tree now. There may also be conflicts with the heavy rework in the spi tree. Kgene, when you pick up this series, can you put it into a separate topic branch and merge that into your main tree? That way if there are difficult conflicts, then I can also merge that same topic branch into spi/next to resolve them. g. ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-04 15:06 ` Grant Likely @ 2011-07-05 5:51 ` Kukjin Kim 2011-07-06 6:22 ` Kukjin Kim 1 sibling, 0 replies; 21+ messages in thread From: Kukjin Kim @ 2011-07-05 5:51 UTC (permalink / raw) To: 'Grant Likely' Cc: 'Jassi Brar', 'padma venkat', 'Tony Nadackal', 'Padmavathi Venna', sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad Grant Likely wrote: > > On Mon, Jul 04, 2011 at 07:08:55PM +0900, Kukjin Kim wrote: > > Kukjin Kim wrote: > > > > > > Thanks Grant, Jassi and all, > > > > > > I will apply these 1 and 2 in my -fix tree for 3.0 with your acks. > > > > > Oops, this needs previous 'cleanup spi platform specific code' so can't > > apply -fix tree now. > > There may also be conflicts with the heavy rework in the spi tree. > Kgene, when you pick up this series, can you put it into a separate > topic branch and merge that into your main tree? That way if there > are difficult conflicts, then I can also merge that same topic branch > into spi/next to resolve them. > Hi Grant, Sure, no problem. I'm waiting for Padmavathi's new patch which is based on old code not new SPI code. When I merge topic branch for this into my tree, let you know immediately. Then if any problems, let's talk again :) Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-04 15:06 ` Grant Likely 2011-07-05 5:51 ` Kukjin Kim @ 2011-07-06 6:22 ` Kukjin Kim 2011-07-06 6:30 ` Grant Likely 1 sibling, 1 reply; 21+ messages in thread From: Kukjin Kim @ 2011-07-06 6:22 UTC (permalink / raw) To: 'Kukjin Kim', 'Grant Likely' Cc: 'Jassi Brar', 'padma venkat', 'Tony Nadackal', 'Padmavathi Venna', sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad Kukjin Kim wrote: > > Grant Likely wrote: > > > > On Mon, Jul 04, 2011 at 07:08:55PM +0900, Kukjin Kim wrote: > > > Kukjin Kim wrote: > > > > > > > > Thanks Grant, Jassi and all, > > > > > > > > I will apply these 1 and 2 in my -fix tree for 3.0 with your acks. > > > > > > > Oops, this needs previous 'cleanup spi platform specific code' so can't > > > apply -fix tree now. > > > > There may also be conflicts with the heavy rework in the spi tree. > > Kgene, when you pick up this series, can you put it into a separate > > topic branch and merge that into your main tree? That way if there > > are difficult conflicts, then I can also merge that same topic branch > > into spi/next to resolve them. > > > Hi Grant, > > Sure, no problem. I'm waiting for Padmavathi's new patch which is based on old > code not new SPI code. When I merge topic branch for this into my tree, let you > know immediately. Then if any problems, let's talk again :) Hi, I made topic branch for this based on 3.0-rc6. git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git #for-spi If any problems, please let me know :) Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-06 6:22 ` Kukjin Kim @ 2011-07-06 6:30 ` Grant Likely 2011-07-06 7:17 ` Kukjin Kim 0 siblings, 1 reply; 21+ messages in thread From: Grant Likely @ 2011-07-06 6:30 UTC (permalink / raw) To: Kukjin Kim Cc: 'Jassi Brar', 'padma venkat', 'Tony Nadackal', 'Padmavathi Venna', sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad On Wed, Jul 06, 2011 at 03:22:08PM +0900, Kukjin Kim wrote: > Kukjin Kim wrote: > > > > Grant Likely wrote: > > > > > > On Mon, Jul 04, 2011 at 07:08:55PM +0900, Kukjin Kim wrote: > > > > Kukjin Kim wrote: > > > > > > > > > > Thanks Grant, Jassi and all, > > > > > > > > > > I will apply these 1 and 2 in my -fix tree for 3.0 with your acks. > > > > > > > > > Oops, this needs previous 'cleanup spi platform specific code' so > can't > > > > apply -fix tree now. > > > > > > There may also be conflicts with the heavy rework in the spi tree. > > > Kgene, when you pick up this series, can you put it into a separate > > > topic branch and merge that into your main tree? That way if there > > > are difficult conflicts, then I can also merge that same topic branch > > > into spi/next to resolve them. > > > > > Hi Grant, > > > > Sure, no problem. I'm waiting for Padmavathi's new patch which is based on > old > > code not new SPI code. When I merge topic branch for this into my tree, > let you > > know immediately. Then if any problems, let's talk again :) > > Hi, > > I made topic branch for this based on 3.0-rc6. > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > #for-spi Okay. If there are problems, then I'll merge it. Thanks. g. ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-06 6:30 ` Grant Likely @ 2011-07-06 7:17 ` Kukjin Kim 2011-07-06 17:30 ` Grant Likely 0 siblings, 1 reply; 21+ messages in thread From: Kukjin Kim @ 2011-07-06 7:17 UTC (permalink / raw) To: 'Grant Likely' Cc: 'Jassi Brar', 'padma venkat', 'Tony Nadackal', 'Padmavathi Venna', sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad Grant Likely wrote: > > On Wed, Jul 06, 2011 at 03:22:08PM +0900, Kukjin Kim wrote: > > Kukjin Kim wrote: > > > > > > Grant Likely wrote: > > > > > > > > On Mon, Jul 04, 2011 at 07:08:55PM +0900, Kukjin Kim wrote: > > > > > Kukjin Kim wrote: > > > > > > > > > > > > Thanks Grant, Jassi and all, > > > > > > > > > > > > I will apply these 1 and 2 in my -fix tree for 3.0 with your acks. > > > > > > > > > > > Oops, this needs previous 'cleanup spi platform specific code' so > > can't > > > > > apply -fix tree now. > > > > > > > > There may also be conflicts with the heavy rework in the spi tree. > > > > Kgene, when you pick up this series, can you put it into a separate > > > > topic branch and merge that into your main tree? That way if there > > > > are difficult conflicts, then I can also merge that same topic branch > > > > into spi/next to resolve them. > > > > > > > Hi Grant, > > > > > > Sure, no problem. I'm waiting for Padmavathi's new patch which is based on > > old > > > code not new SPI code. When I merge topic branch for this into my tree, > > let you > > > know immediately. Then if any problems, let's talk again :) > > > > Hi, > > > > I made topic branch for this based on 3.0-rc6. > > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > > #for-spi > > Okay. If there are problems, then I'll merge it. Thanks. > Maybe 'If there are not problems'? Anyway, where do you want to merge this -next or -fix? I think, this is required for 3.0. Thanks. Best regards, Kgene. -- Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level 2011-07-06 7:17 ` Kukjin Kim @ 2011-07-06 17:30 ` Grant Likely 0 siblings, 0 replies; 21+ messages in thread From: Grant Likely @ 2011-07-06 17:30 UTC (permalink / raw) To: Kukjin Kim Cc: 'Jassi Brar', 'padma venkat', 'Tony Nadackal', 'Padmavathi Venna', sbkim73, spi-devel-general, linux-samsung-soc, linux-arm-kernel, tony.kn, naushad On Wed, Jul 06, 2011 at 04:17:05PM +0900, Kukjin Kim wrote: > Grant Likely wrote: > > > > On Wed, Jul 06, 2011 at 03:22:08PM +0900, Kukjin Kim wrote: > > > Kukjin Kim wrote: > > > > > > > > Grant Likely wrote: > > > > > > > > > > On Mon, Jul 04, 2011 at 07:08:55PM +0900, Kukjin Kim wrote: > > > > > > Kukjin Kim wrote: > > > > > > > > > > > > > > Thanks Grant, Jassi and all, > > > > > > > > > > > > > > I will apply these 1 and 2 in my -fix tree for 3.0 with your > acks. > > > > > > > > > > > > > Oops, this needs previous 'cleanup spi platform specific code' so > > > can't > > > > > > apply -fix tree now. > > > > > > > > > > There may also be conflicts with the heavy rework in the spi tree. > > > > > Kgene, when you pick up this series, can you put it into a separate > > > > > topic branch and merge that into your main tree? That way if there > > > > > are difficult conflicts, then I can also merge that same topic > branch > > > > > into spi/next to resolve them. > > > > > > > > > Hi Grant, > > > > > > > > Sure, no problem. I'm waiting for Padmavathi's new patch which is > based on > > > old > > > > code not new SPI code. When I merge topic branch for this into my > tree, > > > let you > > > > know immediately. Then if any problems, let's talk again :) > > > > > > Hi, > > > > > > I made topic branch for this based on 3.0-rc6. > > > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > > > #for-spi > > > > Okay. If there are problems, then I'll merge it. Thanks. > > > > Maybe 'If there are not problems'? If there are no problems, then I don't need to merge the topic branch. It is only if merge conflicts show up in linux-next that I'll need to merge the topic branch. (regardless, I'm expecting you to merge the topic branch into one of your trees to actually get it into mainline or linux-next). g. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2011-07-06 17:30 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-06-30 12:38 [PATCH 1/2] ARM: SAMSUNG: Added tx_st_done variable in the platform data of SPI Padmavathi Venna 2011-06-30 12:38 ` [PATCH 2/2] SPI: SAMSUNG: Bug fix for SPI with different FIFO level Padmavathi Venna 2011-06-30 7:08 ` Jassi Brar 2011-06-30 9:05 ` padma venkat 2011-06-30 9:52 ` Jassi Brar [not found] ` <BANLkTi=9c5asfr=Jg6xwjQZW_a369UO64Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2011-06-30 11:00 ` Tony Nadackal 2011-07-01 5:46 ` padma venkat 2011-07-01 5:52 ` Jassi Brar 2011-07-01 5:59 ` padma venkat 2011-07-01 6:13 ` Jassi Brar 2011-07-04 6:55 ` Grant Likely 2011-07-04 6:58 ` Grant Likely 2011-07-04 9:54 ` Kukjin Kim 2011-07-04 10:04 ` padma venkat 2011-07-04 10:08 ` Kukjin Kim 2011-07-04 15:06 ` Grant Likely 2011-07-05 5:51 ` Kukjin Kim 2011-07-06 6:22 ` Kukjin Kim 2011-07-06 6:30 ` Grant Likely 2011-07-06 7:17 ` Kukjin Kim 2011-07-06 17:30 ` Grant Likely
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).