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

[Marek Szyprowski]

Hello,

On 2014-06-23 20:40, Paul Zimmerman wrote:
> > From: Robert Baldyga [mailto:r.baldyga@xxxxxxxxxxx]
> > Sent: Monday, June 23, 2014 12:51 AM
> >
> > From: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> >
> > 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.szyprowski@xxxxxxxxxxx>
> > Signed-off-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>
> > ---
> >   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 int            dedicated_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&D Institute Poland

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