subject:"\[PATCH 06\/11\] usb\: dwc2\/gadget\: ensure that all fifos have correct memory buffers"

Re: [PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

2014-06-23 Thread Marek Szyprowski


Hello,

On 2014-06-23 20:40, Paul Zimmerman wrote:

From: Robert Baldyga [mailto:r.bald...@samsung.com]
Sent: Monday, June 23, 2014 12:51 AM

From: Marek Szyprowski 

Print warning if FIFOs are configured in such a way that they don't fit
into the SPRAM available on the s3c hsotg module.

Signed-off-by: Marek Szyprowski 
Signed-off-by: Robert Baldyga 
---
  drivers/usb/dwc2/core.h   |  1 +
  drivers/usb/dwc2/gadget.c | 15 ++-
  2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 1efd10c..067390e 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -194,6 +194,7 @@ struct s3c_hsotg {
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];

u32 phyif;
+   int fifo_mem;
unsigned intdedicated_fifos:1;
unsigned char   num_of_eps;

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 95b6dcb..21d21de 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
for (ep = 1; ep <= 15; ep++) {
val = addr;
val |= size << FIFOSIZE_DEPTH_SHIFT;
+   WARN_ONCE(addr + size > hsotg->fifo_mem,
+ "insufficient fifo memory");
addr += size;

writel(val, hsotg->regs + DPTXFSIZN(ep));
@@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
   */
  static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
  {
-   u32 cfg2, cfg4;
+   u32 cfg2, cfg3, cfg4;
/* check hardware configuration */

cfg2 = readl(hsotg->regs + 0x48);
hsotg->num_of_eps = (cfg2 >> 10) & 0xF;

-   dev_info(hsotg->dev, "EPs:%d\n", hsotg->num_of_eps);
+   cfg3 = readl(hsotg->regs + 0x4C);
+   hsotg->fifo_mem = (cfg3 >> 16);

cfg4 = readl(hsotg->regs + 0x50);
hsotg->dedicated_fifos = (cfg4 >> 25) & 1;

-   dev_info(hsotg->dev, "%s fifos\n",
-hsotg->dedicated_fifos ? "dedicated" : "shared");
+   dev_info(hsotg->dev, "EPs: %d, %s fifos, %d entries in SPRAM\n",
+hsotg->num_of_eps,
+hsotg->dedicated_fifos ? "dedicated" : "shared",
+hsotg->fifo_mem);
  }

  /**
@@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_phy_enable(hsotg);

s3c_hsotg_corereset(hsotg);
-   s3c_hsotg_init(hsotg);
s3c_hsotg_hw_cfg(hsotg);
+   s3c_hsotg_init(hsotg);

This last hunk looks like it is not related to the rest of the patch? If
so, I think it should be a separate patch.


s3c_hsotg_hw_cfg() only reads hw configuration and some values read there 
(fifo_mem)
are used in s3c_hsotg_init_fifo(), which is called from s3c_hsotg_init(). This 
chunk
really belongs to this patch, although it might not be easy to notice it at the 
first
glance.

Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

2014-06-23 Thread Paul Zimmerman

> From: Robert Baldyga [mailto:r.bald...@samsung.com]
> Sent: Monday, June 23, 2014 12:51 AM
> 
> From: Marek Szyprowski 
> 
> Print warning if FIFOs are configured in such a way that they don't fit
> into the SPRAM available on the s3c hsotg module.
> 
> Signed-off-by: Marek Szyprowski 
> Signed-off-by: Robert Baldyga 
> ---
>  drivers/usb/dwc2/core.h   |  1 +
>  drivers/usb/dwc2/gadget.c | 15 ++-
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 1efd10c..067390e 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -194,6 +194,7 @@ struct s3c_hsotg {
>   struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
> 
>   u32 phyif;
> + int fifo_mem;
>   unsigned intdedicated_fifos:1;
>   unsigned char   num_of_eps;
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 95b6dcb..21d21de 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
>   for (ep = 1; ep <= 15; ep++) {
>   val = addr;
>   val |= size << FIFOSIZE_DEPTH_SHIFT;
> + WARN_ONCE(addr + size > hsotg->fifo_mem,
> +   "insufficient fifo memory");
>   addr += size;
> 
>   writel(val, hsotg->regs + DPTXFSIZN(ep));
> @@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
>   */
>  static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
>  {
> - u32 cfg2, cfg4;
> + u32 cfg2, cfg3, cfg4;
>   /* check hardware configuration */
> 
>   cfg2 = readl(hsotg->regs + 0x48);
>   hsotg->num_of_eps = (cfg2 >> 10) & 0xF;
> 
> - dev_info(hsotg->dev, "EPs:%d\n", hsotg->num_of_eps);
> + cfg3 = readl(hsotg->regs + 0x4C);
> + hsotg->fifo_mem = (cfg3 >> 16);
> 
>   cfg4 = readl(hsotg->regs + 0x50);
>   hsotg->dedicated_fifos = (cfg4 >> 25) & 1;
> 
> - dev_info(hsotg->dev, "%s fifos\n",
> -  hsotg->dedicated_fifos ? "dedicated" : "shared");
> + dev_info(hsotg->dev, "EPs: %d, %s fifos, %d entries in SPRAM\n",
> +  hsotg->num_of_eps,
> +  hsotg->dedicated_fifos ? "dedicated" : "shared",
> +  hsotg->fifo_mem);
>  }
> 
>  /**
> @@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
>   s3c_hsotg_phy_enable(hsotg);
> 
>   s3c_hsotg_corereset(hsotg);
> - s3c_hsotg_init(hsotg);
>   s3c_hsotg_hw_cfg(hsotg);
> + s3c_hsotg_init(hsotg);

This last hunk looks like it is not related to the rest of the patch? If
so, I think it should be a separate patch.

-- 
Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

2014-06-23 Thread Robert Baldyga

From: Marek Szyprowski 

Print warning if FIFOs are configured in such a way that they don't fit
into the SPRAM available on the s3c hsotg module.

Signed-off-by: Marek Szyprowski 
Signed-off-by: Robert Baldyga 
---
 drivers/usb/dwc2/core.h   |  1 +
 drivers/usb/dwc2/gadget.c | 15 ++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 1efd10c..067390e 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -194,6 +194,7 @@ struct s3c_hsotg {
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
 
u32 phyif;
+   int fifo_mem;
unsigned intdedicated_fifos:1;
unsigned char   num_of_eps;
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 95b6dcb..21d21de 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
for (ep = 1; ep <= 15; ep++) {
val = addr;
val |= size << FIFOSIZE_DEPTH_SHIFT;
+   WARN_ONCE(addr + size > hsotg->fifo_mem,
+ "insufficient fifo memory");
addr += size;
 
writel(val, hsotg->regs + DPTXFSIZN(ep));
@@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
  */
 static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
 {
-   u32 cfg2, cfg4;
+   u32 cfg2, cfg3, cfg4;
/* check hardware configuration */
 
cfg2 = readl(hsotg->regs + 0x48);
hsotg->num_of_eps = (cfg2 >> 10) & 0xF;
 
-   dev_info(hsotg->dev, "EPs:%d\n", hsotg->num_of_eps);
+   cfg3 = readl(hsotg->regs + 0x4C);
+   hsotg->fifo_mem = (cfg3 >> 16);
 
cfg4 = readl(hsotg->regs + 0x50);
hsotg->dedicated_fifos = (cfg4 >> 25) & 1;
 
-   dev_info(hsotg->dev, "%s fifos\n",
-hsotg->dedicated_fifos ? "dedicated" : "shared");
+   dev_info(hsotg->dev, "EPs: %d, %s fifos, %d entries in SPRAM\n",
+hsotg->num_of_eps,
+hsotg->dedicated_fifos ? "dedicated" : "shared",
+hsotg->fifo_mem);
 }
 
 /**
@@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_phy_enable(hsotg);
 
s3c_hsotg_corereset(hsotg);
-   s3c_hsotg_init(hsotg);
s3c_hsotg_hw_cfg(hsotg);
+   s3c_hsotg_init(hsotg);
 
/* hsotg->num_of_eps holds number of EPs other than ep0 */
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

2014-06-23 Thread Paul Zimmerman

 From: Robert Baldyga [mailto:r.bald...@samsung.com]
 Sent: Monday, June 23, 2014 12:51 AM
 
 From: Marek Szyprowski m.szyprow...@samsung.com
 
 Print warning if FIFOs are configured in such a way that they don't fit
 into the SPRAM available on the s3c hsotg module.
 
 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
 Signed-off-by: Robert Baldyga r.bald...@samsung.com
 ---
  drivers/usb/dwc2/core.h   |  1 +
  drivers/usb/dwc2/gadget.c | 15 ++-
  2 files changed, 11 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
 index 1efd10c..067390e 100644
 --- a/drivers/usb/dwc2/core.h
 +++ b/drivers/usb/dwc2/core.h
 @@ -194,6 +194,7 @@ struct s3c_hsotg {
   struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
 
   u32 phyif;
 + int fifo_mem;
   unsigned intdedicated_fifos:1;
   unsigned char   num_of_eps;
 
 diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
 index 95b6dcb..21d21de 100644
 --- a/drivers/usb/dwc2/gadget.c
 +++ b/drivers/usb/dwc2/gadget.c
 @@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
   for (ep = 1; ep = 15; ep++) {
   val = addr;
   val |= size  FIFOSIZE_DEPTH_SHIFT;
 + WARN_ONCE(addr + size  hsotg-fifo_mem,
 +   insufficient fifo memory);
   addr += size;
 
   writel(val, hsotg-regs + DPTXFSIZN(ep));
 @@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
   */
  static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
  {
 - u32 cfg2, cfg4;
 + u32 cfg2, cfg3, cfg4;
   /* check hardware configuration */
 
   cfg2 = readl(hsotg-regs + 0x48);
   hsotg-num_of_eps = (cfg2  10)  0xF;
 
 - dev_info(hsotg-dev, EPs:%d\n, hsotg-num_of_eps);
 + cfg3 = readl(hsotg-regs + 0x4C);
 + hsotg-fifo_mem = (cfg3  16);
 
   cfg4 = readl(hsotg-regs + 0x50);
   hsotg-dedicated_fifos = (cfg4  25)  1;
 
 - dev_info(hsotg-dev, %s fifos\n,
 -  hsotg-dedicated_fifos ? dedicated : shared);
 + dev_info(hsotg-dev, EPs: %d, %s fifos, %d entries in SPRAM\n,
 +  hsotg-num_of_eps,
 +  hsotg-dedicated_fifos ? dedicated : shared,
 +  hsotg-fifo_mem);
  }
 
  /**
 @@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
   s3c_hsotg_phy_enable(hsotg);
 
   s3c_hsotg_corereset(hsotg);
 - s3c_hsotg_init(hsotg);
   s3c_hsotg_hw_cfg(hsotg);
 + s3c_hsotg_init(hsotg);

This last hunk looks like it is not related to the rest of the patch? If
so, I think it should be a separate patch.

-- 
Paul

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

2014-06-23 Thread Marek Szyprowski


Hello,

On 2014-06-23 20:40, Paul Zimmerman wrote:

From: Robert Baldyga [mailto:r.bald...@samsung.com]
Sent: Monday, June 23, 2014 12:51 AM

From: Marek Szyprowski m.szyprow...@samsung.com

Print warning if FIFOs are configured in such a way that they don't fit
into the SPRAM available on the s3c hsotg module.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Signed-off-by: Robert Baldyga r.bald...@samsung.com
---
  drivers/usb/dwc2/core.h   |  1 +
  drivers/usb/dwc2/gadget.c | 15 ++-
  2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 1efd10c..067390e 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -194,6 +194,7 @@ struct s3c_hsotg {
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];

u32 phyif;
+   int fifo_mem;
unsigned intdedicated_fifos:1;
unsigned char   num_of_eps;

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 95b6dcb..21d21de 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
for (ep = 1; ep = 15; ep++) {
val = addr;
val |= size  FIFOSIZE_DEPTH_SHIFT;
+   WARN_ONCE(addr + size  hsotg-fifo_mem,
+ insufficient fifo memory);
addr += size;

writel(val, hsotg-regs + DPTXFSIZN(ep));
@@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
   */
  static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
  {
-   u32 cfg2, cfg4;
+   u32 cfg2, cfg3, cfg4;
/* check hardware configuration */

cfg2 = readl(hsotg-regs + 0x48);
hsotg-num_of_eps = (cfg2  10)  0xF;

-   dev_info(hsotg-dev, EPs:%d\n, hsotg-num_of_eps);
+   cfg3 = readl(hsotg-regs + 0x4C);
+   hsotg-fifo_mem = (cfg3  16);

cfg4 = readl(hsotg-regs + 0x50);
hsotg-dedicated_fifos = (cfg4  25)  1;

-   dev_info(hsotg-dev, %s fifos\n,
-hsotg-dedicated_fifos ? dedicated : shared);
+   dev_info(hsotg-dev, EPs: %d, %s fifos, %d entries in SPRAM\n,
+hsotg-num_of_eps,
+hsotg-dedicated_fifos ? dedicated : shared,
+hsotg-fifo_mem);
  }

  /**
@@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_phy_enable(hsotg);

s3c_hsotg_corereset(hsotg);
-   s3c_hsotg_init(hsotg);
s3c_hsotg_hw_cfg(hsotg);
+   s3c_hsotg_init(hsotg);

This last hunk looks like it is not related to the rest of the patch? If
so, I think it should be a separate patch.


s3c_hsotg_hw_cfg() only reads hw configuration and some values read there 
(fifo_mem)
are used in s3c_hsotg_init_fifo(), which is called from s3c_hsotg_init(). This 
chunk
really belongs to this patch, although it might not be easy to notice it at the 
first
glance.

Best regards
--
Marek Szyprowski, PhD
Samsung RD Institute Poland

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

2014-06-23 Thread Robert Baldyga

From: Marek Szyprowski m.szyprow...@samsung.com

Print warning if FIFOs are configured in such a way that they don't fit
into the SPRAM available on the s3c hsotg module.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Signed-off-by: Robert Baldyga r.bald...@samsung.com
---
 drivers/usb/dwc2/core.h   |  1 +
 drivers/usb/dwc2/gadget.c | 15 ++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 1efd10c..067390e 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -194,6 +194,7 @@ struct s3c_hsotg {
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
 
u32 phyif;
+   int fifo_mem;
unsigned intdedicated_fifos:1;
unsigned char   num_of_eps;
 
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 95b6dcb..21d21de 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -194,6 +194,8 @@ static void s3c_hsotg_init_fifo(struct s3c_hsotg *hsotg)
for (ep = 1; ep = 15; ep++) {
val = addr;
val |= size  FIFOSIZE_DEPTH_SHIFT;
+   WARN_ONCE(addr + size  hsotg-fifo_mem,
+ insufficient fifo memory);
addr += size;
 
writel(val, hsotg-regs + DPTXFSIZN(ep));
@@ -3030,19 +3032,22 @@ static void s3c_hsotg_initep(struct s3c_hsotg *hsotg,
  */
 static void s3c_hsotg_hw_cfg(struct s3c_hsotg *hsotg)
 {
-   u32 cfg2, cfg4;
+   u32 cfg2, cfg3, cfg4;
/* check hardware configuration */
 
cfg2 = readl(hsotg-regs + 0x48);
hsotg-num_of_eps = (cfg2  10)  0xF;
 
-   dev_info(hsotg-dev, EPs:%d\n, hsotg-num_of_eps);
+   cfg3 = readl(hsotg-regs + 0x4C);
+   hsotg-fifo_mem = (cfg3  16);
 
cfg4 = readl(hsotg-regs + 0x50);
hsotg-dedicated_fifos = (cfg4  25)  1;
 
-   dev_info(hsotg-dev, %s fifos\n,
-hsotg-dedicated_fifos ? dedicated : shared);
+   dev_info(hsotg-dev, EPs: %d, %s fifos, %d entries in SPRAM\n,
+hsotg-num_of_eps,
+hsotg-dedicated_fifos ? dedicated : shared,
+hsotg-fifo_mem);
 }
 
 /**
@@ -3495,8 +3500,8 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
s3c_hsotg_phy_enable(hsotg);
 
s3c_hsotg_corereset(hsotg);
-   s3c_hsotg_init(hsotg);
s3c_hsotg_hw_cfg(hsotg);
+   s3c_hsotg_init(hsotg);
 
/* hsotg-num_of_eps holds number of EPs other than ep0 */
 
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

RE: [PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

[PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

RE: [PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

Re: [PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

[PATCH 06/11] usb: dwc2/gadget: ensure that all fifos have correct memory buffers

6 matches

Site Navigation

Mail list logo

Footer information