Re: [U-Boot] [U-Boot, 02/10] arm: s3c24xx: Fix incorrect CONFIG_SYS_S3C2410_NAND_HWECC name

2015-05-01 Thread Scott Wood
On Sat, 2015-05-02 at 03:18 +0200, Marek Vasut wrote:
> On Saturday, May 02, 2015 at 03:11:41 AM, Scott Wood wrote:
> > On Sat, 2015-05-02 at 02:46 +0200, Marek Vasut wrote:
> > > On Thursday, November 27, 2014 at 03:03:50 AM, Scott Wood wrote:
> > > > On Sat, Oct 11, 2014 at 06:42:50PM +0200, Marek Vasut wrote:
> > > > > The correct name of this symbol is CONFIG_S3C2410_NAND_HWECC , the
> > > > > _SYS is redundant.
> > > > 
> > > > What makes that the correct name?  The symbol is not documented
> > > > anywhere, and while nothing currently tests for the SYS version,
> > > > nothing currently sets the non-SYS version.
> > > > 
> > > > What is SYS redundant with?
> > > > 
> > > > Is this meant to be a user config knob or something that is fixed for a
> > > > given board?
> > > 
> > > u-boot$ git grep CONFIG_SYS_S3C2410_NAND_HWECC
> > > include/configs/VCMA9.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
> > > include/configs/smdk2410.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
> > > 
> > > u-boot$ git grep CONFIG_S3C2410_NAND_HWECC
> > > drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
> > > drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
> > > 
> > > The driver checks the version without _SYS. This is a clear bugfix,
> > > so please apply.
> > 
> > There's a clear bug.  I asked questions to determine whether this is the
> > proper fix (and encourage a better changelog), and you still haven't
> > answered.  It would also be nice if you'd document the symbol while
> > you're at it.
> 
> Adding new stuff (like documentation) is now mandatory part of bugfix ?

I didn't say mandatory.  I said "would be nice".

> I'd expect bugfixes to be taken in to actually fix bugs and not kept out
> of the tree because the submitter didn't also do another random chore.

In this case the lack of documentation feels related to the cause of the
bug -- no authoritative place to say what the proper name is.  I don't
see it as "another random chore".

> Also, I do not know what else should I say besides that the symbol name
> is incorrect and you can clearly see it. If that is not enough ...

The names don't match.  That's a bug.  I was wondering whether the right
thing was to remove SYS in one place or add it in the other (or have we
given up on maintaining the distinction?).  I didn't expect asking that
question to be a huge burden on the patch's progress.  It was also
bundled up in a patchset with a bunch of non-bugfixes, so it didn't seem
like you were asking for urgent handling of this one.

> > In any case, I don't know why you're asking me to apply a patch with an
> > "arm:" subject line, which only touches ARM board config files, instead
> > of asking an ARM custodian.
> 
> You were the only one who responded, 

And look at what I get for doing so. :-)

> but I'm fine if Tom or whoever picks this.

While I would have liked an answer to the questions I asked, it's not
worth arguing over, so:

Acked-by: Scott Wood 

-Scott


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-Boot, 02/10] arm: s3c24xx: Fix incorrect CONFIG_SYS_S3C2410_NAND_HWECC name

2015-05-01 Thread Marek Vasut
On Saturday, May 02, 2015 at 03:11:41 AM, Scott Wood wrote:
> On Sat, 2015-05-02 at 02:46 +0200, Marek Vasut wrote:
> > On Thursday, November 27, 2014 at 03:03:50 AM, Scott Wood wrote:
> > > On Sat, Oct 11, 2014 at 06:42:50PM +0200, Marek Vasut wrote:
> > > > The correct name of this symbol is CONFIG_S3C2410_NAND_HWECC , the
> > > > _SYS is redundant.
> > > 
> > > What makes that the correct name?  The symbol is not documented
> > > anywhere, and while nothing currently tests for the SYS version,
> > > nothing currently sets the non-SYS version.
> > > 
> > > What is SYS redundant with?
> > > 
> > > Is this meant to be a user config knob or something that is fixed for a
> > > given board?
> > 
> > u-boot$ git grep CONFIG_SYS_S3C2410_NAND_HWECC
> > include/configs/VCMA9.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
> > include/configs/smdk2410.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
> > 
> > u-boot$ git grep CONFIG_S3C2410_NAND_HWECC
> > drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
> > drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
> > 
> > The driver checks the version without _SYS. This is a clear bugfix,
> > so please apply.
> 
> There's a clear bug.  I asked questions to determine whether this is the
> proper fix (and encourage a better changelog), and you still haven't
> answered.  It would also be nice if you'd document the symbol while
> you're at it.

Adding new stuff (like documentation) is now mandatory part of bugfix ?
I'd expect bugfixes to be taken in to actually fix bugs and not kept out
of the tree because the submitter didn't also do another random chore.

Also, I do not know what else should I say besides that the symbol name
is incorrect and you can clearly see it. If that is not enough ...

> In any case, I don't know why you're asking me to apply a patch with an
> "arm:" subject line, which only touches ARM board config files, instead
> of asking an ARM custodian.

You were the only one who responded, but I'm fine if Tom or whoever picks
this.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 09/10] gpio: s3c: Fix the GPIO driver

2015-05-01 Thread Marek Vasut
On Saturday, October 11, 2014 at 06:42:57 PM, Marek Vasut wrote:
> The GPIO driver didn't correctly compute the bank offset
> from the GPIO number and caused random writes into the
> GPIO block address space. Fix the driver so it actually
> does the writes correctly. While at it, make use of the
> clrsetbits_le32() mechanisms.
> 
> Signed-off-by: Marek Vasut 
> Cc: Kyungmin Park 
> Cc: Lukasz Majewski 
> Cc: Minkyu Kang 
> Cc: Vladimir Zapolskiy 

Can this please be applied ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-Boot, 02/10] arm: s3c24xx: Fix incorrect CONFIG_SYS_S3C2410_NAND_HWECC name

2015-05-01 Thread Scott Wood
On Sat, 2015-05-02 at 02:46 +0200, Marek Vasut wrote:
> On Thursday, November 27, 2014 at 03:03:50 AM, Scott Wood wrote:
> > On Sat, Oct 11, 2014 at 06:42:50PM +0200, Marek Vasut wrote:
> > > The correct name of this symbol is CONFIG_S3C2410_NAND_HWECC , the
> > > _SYS is redundant.
> > 
> > What makes that the correct name?  The symbol is not documented anywhere,
> > and while nothing currently tests for the SYS version, nothing currently
> > sets the non-SYS version.
> > 
> > What is SYS redundant with?
> > 
> > Is this meant to be a user config knob or something that is fixed for a
> > given board?
> 
> u-boot$ git grep CONFIG_SYS_S3C2410_NAND_HWECC
> include/configs/VCMA9.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
> include/configs/smdk2410.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
> 
> u-boot$ git grep CONFIG_S3C2410_NAND_HWECC
> drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
> drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
> 
> The driver checks the version without _SYS. This is a clear bugfix,
> so please apply.

There's a clear bug.  I asked questions to determine whether this is the
proper fix (and encourage a better changelog), and you still haven't
answered.  It would also be nice if you'd document the symbol while
you're at it.

In any case, I don't know why you're asking me to apply a patch with an
"arm:" subject line, which only touches ARM board config files, instead
of asking an ARM custodian.

-Scott


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 08/10] i2c: s3c: Implant support for S3C2440

2015-05-01 Thread Marek Vasut
On Saturday, October 11, 2014 at 06:42:56 PM, Marek Vasut wrote:
> This is a matter of simple additional ifdefery to cater
> for the different register layout of the S3C2440 chip.
> 
> Signed-off-by: Marek Vasut 
> Cc: Heiko Schocher 
> Cc: Kyungmin Park 
> Cc: Lukasz Majewski 
> Cc: Minkyu Kang 
> Cc: Vladimir Zapolskiy 

Can this please be applied ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 01/10] video: Add S3C24xx framebuffer support

2015-05-01 Thread Marek Vasut
On Thursday, October 16, 2014 at 11:28:44 AM, Anatolij Gustschin wrote:
> Hi Marek,
> 
> On Sat, 11 Oct 2014 18:42:49 +0200
> 
> Marek Vasut  wrote:
> > Add basic framebuffer driver for the S3C24xx family of CPUs.
> > 
> > Signed-off-by: Marek Vasut 
> > Cc: Anatolij Gustschin 
> > Cc: Kyungmin Park 
> > Cc: Lukasz Majewski 
> > Cc: Minkyu Kang 
> > Cc: Vladimir Zapolskiy 
> > 
> > V2: Keep the Makefile sorted.
> > ---
> > 
> >  drivers/video/Makefile  |   1 +
> >  drivers/video/cfb_console.c |   2 +-
> >  drivers/video/s3c-fb.c  | 172
> >   3 files changed, 174
> >  insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/video/s3c-fb.c
> 
> Acked-by: Anatolij Gustschin 

Can this please be applied ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-Boot,05/10] mtd: nand: s3c: Add S3C2440 specifics

2015-05-01 Thread Scott Wood
On Sat, 2015-05-02 at 02:48 +0200, Marek Vasut wrote:
> On Thursday, November 27, 2014 at 03:20:53 AM, Scott Wood wrote:
> > On Sat, Oct 11, 2014 at 06:42:53PM +0200, Marek Vasut wrote:
> > > +#ifdef CONFIG_S3C2410
> > > + sel_reg = (uint32_t)&nand->nfconf;
> > > + sel_bit = S3C2410_NFCONF_nFCE;
> > > +#else
> > > + sel_reg = (uint32_t)&nand->nfcont;
> > > + sel_bit = S3C2440_NFCONT_nFCE;
> > > +#endif
> > 
> > Why are you casting &nand->nfcon[ft] to an integer...
> 
> Where exactly ?

sel_reg = (uint32_t)&nand->nfconf;

> > >   if (ctrl & NAND_NCE)
> > > 
> > > - writel(readl(&nand->nfconf) & ~S3C2410_NFCONF_nFCE,
> > > -&nand->nfconf);
> > > + clrbits_le32(sel_reg, sel_bit);
> > > 
> > >   else
> > > 
> > > - writel(readl(&nand->nfconf) | S3C2410_NFCONF_nFCE,
> > > -&nand->nfconf);
> > > + setbits_le32(sel_reg, sel_bit);
> > 
> > ...only to pass that integer to something that normally accepts a
> > pointer, which doesn't cause a warning only because of ARM's crappy I/O
> > accessors that don't do type checking?
> > 
> > At least the code that was there before used ulong for inappropriate
> > pointer-to-integer casts rather than uint32_t. :-P
> 
> So what do you suggest ?

Store it in a proper pointer type.

-Scott


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-Boot,05/10] mtd: nand: s3c: Add S3C2440 specifics

2015-05-01 Thread Marek Vasut
On Thursday, November 27, 2014 at 03:20:53 AM, Scott Wood wrote:
> On Sat, Oct 11, 2014 at 06:42:53PM +0200, Marek Vasut wrote:
> > +#ifdef CONFIG_S3C2410
> > 
> >  #define S3C2410_NFCONF_TACLS(x)((x)<<8)
> >  #define S3C2410_NFCONF_TWRPH0(x)   ((x)<<4)
> >  #define S3C2410_NFCONF_TWRPH1(x)   ((x)<<0)
> > 
> > +#else  /* S3C2412, S3C2440 */
> > +#define S3C2410_NFCONF_TACLS(x)((x)<<12)
> > +#define S3C2410_NFCONF_TWRPH0(x)   ((x)<<8)
> > +#define S3C2410_NFCONF_TWRPH1(x)   ((x)<<4)
> > +#endif
> 
> Is there any chance there will be a third variant?  Perhaps the S3C2440
> variant should be explicitly requested with an #error if no variant is
> selected.

No, Samsung has moved on.

> >  #define S3C2410_ADDR_NALE 4
> >  #define S3C2410_ADDR_NCLE 8
> > 
> > @@ -42,25 +51,30 @@ static void s3c24x0_hwcontrol(struct mtd_info *mtd,
> > int cmd, unsigned int ctrl)
> > 
> >  {
> >  
> > struct nand_chip *chip = mtd->priv;
> > struct s3c24x0_nand *nand = s3c24x0_get_base_nand();
> > 
> > +   uint32_t sel_reg, sel_bit;
> > 
> > debug("hwcontrol(): 0x%02x 0x%08x\n", cmd & 0xff, ctrl);
> > 
> > if (ctrl & NAND_CTRL_CHANGE) {
> > 
> > -   ulong IO_ADDR_W = (ulong)nand;
> > +   chip->IO_ADDR_W = &nand->nfconf;
> > 
> > if (!(ctrl & NAND_CLE))
> > 
> > -   IO_ADDR_W |= S3C2410_ADDR_NCLE;
> > +   chip->IO_ADDR_W = &nand->nfaddr;
> > 
> > if (!(ctrl & NAND_ALE))
> > 
> > -   IO_ADDR_W |= S3C2410_ADDR_NALE;
> > +   chip->IO_ADDR_W = &nand->nfcmd;
> > 
> > -   chip->IO_ADDR_W = (void *)IO_ADDR_W;
> > +#ifdef CONFIG_S3C2410
> > +   sel_reg = (uint32_t)&nand->nfconf;
> > +   sel_bit = S3C2410_NFCONF_nFCE;
> > +#else
> > +   sel_reg = (uint32_t)&nand->nfcont;
> > +   sel_bit = S3C2440_NFCONT_nFCE;
> > +#endif
> 
> Why are you casting &nand->nfcon[ft] to an integer...

Where exactly ?

> > if (ctrl & NAND_NCE)
> > 
> > -   writel(readl(&nand->nfconf) & ~S3C2410_NFCONF_nFCE,
> > -  &nand->nfconf);
> > +   clrbits_le32(sel_reg, sel_bit);
> > 
> > else
> > 
> > -   writel(readl(&nand->nfconf) | S3C2410_NFCONF_nFCE,
> > -  &nand->nfconf);
> > +   setbits_le32(sel_reg, sel_bit);
> 
> ...only to pass that integer to something that normally accepts a
> pointer, which doesn't cause a warning only because of ARM's crappy I/O
> accessors that don't do type checking?
> 
> At least the code that was there before used ulong for inappropriate
> pointer-to-integer casts rather than uint32_t. :-P

So what do you suggest ?

> > -   writel(readl(&clk_power->clkcon) | (1 << 4), &clk_power->clkcon);
> > +   setbits_le32(&clk_power->clkcon, 1 << 4);
> 
> This change seems unrelated to adding s3c2440 support.  It'd be clearer
> if the {clr,set,clrset}bits_le32() conversion were done separately,
> before the s3c2440 stuff.  I'm not insisting that you redo this one, but
> next time try to keep cleanup separate.

If that's the case, then please apply.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-Boot, 02/10] arm: s3c24xx: Fix incorrect CONFIG_SYS_S3C2410_NAND_HWECC name

2015-05-01 Thread Marek Vasut
On Thursday, November 27, 2014 at 03:03:50 AM, Scott Wood wrote:
> On Sat, Oct 11, 2014 at 06:42:50PM +0200, Marek Vasut wrote:
> > The correct name of this symbol is CONFIG_S3C2410_NAND_HWECC , the
> > _SYS is redundant.
> 
> What makes that the correct name?  The symbol is not documented anywhere,
> and while nothing currently tests for the SYS version, nothing currently
> sets the non-SYS version.
> 
> What is SYS redundant with?
> 
> Is this meant to be a user config knob or something that is fixed for a
> given board?

u-boot$ git grep CONFIG_SYS_S3C2410_NAND_HWECC
include/configs/VCMA9.h:#define CONFIG_SYS_S3C2410_NAND_HWECC
include/configs/smdk2410.h:#define CONFIG_SYS_S3C2410_NAND_HWECC

u-boot$ git grep CONFIG_S3C2410_NAND_HWECC
drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC
drivers/mtd/nand/s3c2410_nand.c:#ifdef CONFIG_S3C2410_NAND_HWECC

The driver checks the version without _SYS. This is a clear bugfix,
so please apply.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-boot][PATCHv2 2/3] driver/ddr/altera/: Add the sdram calibration portion

2015-05-01 Thread Marek Vasut
On Wednesday, April 29, 2015 at 04:58:15 PM, dingu...@opensource.altera.com 
wrote:
> From: Dinh Nguyen 
> 
> This patch adds the DDR calibration portion of the Altera SDRAM driver.
> 
> Signed-off-by: Dinh Nguyen 

[...]

> +/*
> * +
> **
>  + ** NOTE: Special Rules for Globale Variables
>** + **
>  ** + ** All global variables that are explicitly initialized
> (including  ** + ** explicitly initialized to zero), are only
> initialized once, during   ** + ** configuration time, and not again
> on reset.  This means that they** + ** preserve their current
> contents across resets, which is needed for some  ** + ** special cases
> involving communication with external modules.  In ** + **
> addition, this avoids paying the price to have the memory initialized,  
> ** + ** even for zeroed data, provided it is explicitly set to zero in the
> code, ** + ** and doesn't rely on implicit initialization.
> ** +
> **
>  +
> **
> / +
> +/*
> + * Temporary workaround to place the initial stack pointer at a safe
> + * offset from end
> + */
> +asm(".global __alt_stack_pointer");
> +asm("__alt_stack_pointer = " __stringify(STACK_POINTER));

This is really scary. The idea here is to have some kind of a storage
which is preserved over warm reboots, right ? Tom, Simon, Albert, don't
we have some kind of a generic abstration for this kind of stuff ?

> +/* Just to make the debugging code more uniform */
> +#ifndef RW_MGR_MEM_NUMBER_OF_CS_PER_DIMM
> +#define RW_MGR_MEM_NUMBER_OF_CS_PER_DIMM 0
> +#endif
> +
> +#define SDR_WRITE1
> +#define SDR_READ 0
> +
> +#define DELTA_D  1
> +#define MGR_SELECT_MASK  0xf8000
> +
> +/*
> + * In order to reduce ROM size, most of the selectable calibration steps
> are + * decided at compile time based on the user's calibration mode
> selection, + * as captured by the STATIC_CALIB_STEPS selection below.
> + *
> + * However, to support simulation-time selection of fast simulation mode,
> where + * we skip everything except the bare minimum, we need a few of the
> steps to + * be dynamic.  In those cases, we either use the
> DYNAMIC_CALIB_STEPS for the + * check, which is based on the rtl-supplied
> value, or we dynamically compute + * the value to use based on the
> dynamically-chosen calibration mode + */
> +
> +#define BTFLD_FMT "%u"

Nit: Maybe you should just massage this BTFLD_FMT into the code and drop the 
macro.

> +/* For HPS running on actual hardware */
> +
> +#define DLEVEL 0
> +/*
> + * space around comma is required for varargs macro to remove comma if
> args + * is empty
> + */
> +#define BFM_GBL_SET(field, value)
> +#define BFM_GBL_GET(field)   ((long unsigned int)0)
> +#define BFM_STAGE(stage)
> +#define BFM_INC_VFIFO
> +#define COV(label)

Are the macros above needed ?

> +#define STATIC_IN_RTL_SIM 0
> +
> +#define STATIC_SKIP_DELAY_LOOPS 0
> +
> +#define STATIC_CALIB_STEPS (STATIC_IN_RTL_SIM | CALIB_SKIP_FULL_TEST | \
> + STATIC_SKIP_DELAY_LOOPS)
> +
> +/* calibration steps requested by the rtl */
> +uint16_t dyn_calib_steps;

[...]

> +static inline void scc_mgr_set_dm_in_delay(uint32_t write_group,
> +uint32_t dm, uint32_t delay)
> +{
> + /* Load the setting in the SCC manager */
> + sdr_ops((u32 *)SCC_MGR_IO_IN_DELAY,
> + (RW_MGR_MEM_DQ_PER_WRITE_DQS + 1 + dm) << 2, delay, SDR_WRITE);

Won't it be better to convert this entire sdr_ops() to something like ...

addr = get_reg_address(...args...);
writel(...value..., addr);

? I think that'd make this code a bit more readable. What do you think ?

> +}
> +

[...]

> +/* find a good dqs enable to use */
> +static uint32_t rw_mgr_mem_calibrate_vfifo_find_dqs_en_phase(uint32_t grp)
> +{
> + uint32_t i, d, v, p;
> + uint32_t max_working_cnt;
> + uint32_t fail_cnt;
> + uint32_t bit_chk;
> + uint32_t dtaps_per_ptap;
> + uint32_t found_begin, found_end;
> + uint32_t work_bgn, work_mid, work_end, tmp_delay;
> + uint32_t test_status;
> + uint32_t found_passing_read, found_failing_read, initial_failing_dtap;
> +
> + debug("%s:%d %u\n", __func__, __LINE__, grp);
> + BFM_STAGE("find_dqs_en_phase");
> +
> + reg_file_set_sub_stage(CAL_SUBSTAGE_VFIFO_CENTER);
> +
> + scc_mgr_set_dqs_en_delay_all_ranks(grp, 0);
> + scc_mgr_set_dqs_en_phase_all_ranks(grp, 0);
> +
> + fail_cnt = 0;
> +
> + /* ** */
> + /* * Step 0 : Determine number of delay taps for each phas

[U-Boot] [PATCH v2 1/2] stv0991: fdt: add stv0991 device tree

2015-05-01 Thread Vikas Manocha
This patch adds device tree for the ST Micro stv0991 board & enables
device tree control. Progressively device tree support for the drivers
being used will also be added.

Signed-off-by: Vikas Manocha 
---

Changes in v2:
- added commit message.

 arch/arm/dts/Makefile |1 +
 arch/arm/dts/stv0991.dts  |   23 +++
 configs/stv0991_defconfig |1 +
 include/configs/stv0991.h |3 +++
 4 files changed, 28 insertions(+)
 create mode 100644 arch/arm/dts/stv0991.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 46a6171..86faf58 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -54,6 +54,7 @@ dtb-$(CONFIG_SOCFPGA) +=  \
socfpga_arria5_socdk.dtb\
socfpga_cyclone5_socdk.dtb  \
socfpga_cyclone5_socrates.dtb
+dtb-$(CONFIG_TARGET_STV0991) += stv0991.dtb
 
 dtb-$(CONFIG_LS102XA) += ls1021a-qds.dtb \
ls1021a-twr.dtb
diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
new file mode 100644
index 000..b25c48b
--- /dev/null
+++ b/arch/arm/dts/stv0991.dts
@@ -0,0 +1,23 @@
+/dts-v1/;
+
+/ {
+   model = "ST STV0991 application board";
+   compatible = "st,stv0991";
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   chosen {
+   stdout-path = &uart0;
+   };
+
+   memory {
+   device_type="memory";
+   reg = <0x0 0x198000>;
+   };
+
+   uart0: serial@0x80406000 {
+   compatible = "arm,pl011", "arm,primecell";
+   reg = <0x80406000 0x1000>;
+   clock = <270>;
+   };
+};
diff --git a/configs/stv0991_defconfig b/configs/stv0991_defconfig
index 76ba41b..d9edc06 100644
--- a/configs/stv0991_defconfig
+++ b/configs/stv0991_defconfig
@@ -5,3 +5,4 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_NETDEVICES=y
 CONFIG_NET=y
+CONFIG_DEFAULT_DEVICE_TREE="stv0991"
diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
index 2f65eda..750eebd 100644
--- a/include/configs/stv0991.h
+++ b/include/configs/stv0991.h
@@ -80,4 +80,7 @@
 #define CONFIG_AUTOBOOT_PROMPT \
"Hit SPACE in %d seconds to stop autoboot.\n", bootdelay
 #undef CONFIG_HAS_VBAR
+#define CONFIG_OF_EMBED
+#define CONFIG_OF_CONTROL
+#define CONFIG_OF_LIBFDT
 #endif /* __CONFIG_H */
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 2/2] stv0991: use fdt for serial port platform data

2015-05-01 Thread Vikas Manocha
This patch ignores the serial port static platform data at compilation time
in case of device tree control.

Signed-off-by: Vikas Manocha 
Reviewed-by: Simon Glass 
---

Changes in v2:
- added commit message.

 board/st/stv0991/stv0991.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/board/st/stv0991/stv0991.c b/board/st/stv0991/stv0991.c
index 38f6e1d..09f973f 100644
--- a/board/st/stv0991/stv0991.c
+++ b/board/st/stv0991/stv0991.c
@@ -21,6 +21,7 @@ DECLARE_GLOBAL_DATA_PTR;
 struct gpio_regs *const gpioa_regs =
(struct gpio_regs *) GPIOA_BASE_ADDR;
 
+#ifndef CONFIG_OF_CONTROL
 static const struct pl01x_serial_platdata serial_platdata = {
.base = 0x80406000,
.type = TYPE_PL011,
@@ -31,6 +32,7 @@ U_BOOT_DEVICE(stv09911_serials) = {
.name = "serial_pl01x",
.platdata = &serial_platdata,
 };
+#endif
 
 #ifdef CONFIG_SHOW_BOOT_PROGRESS
 void show_boot_progress(int progress)
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 0/2] stv0991: Add flat device tree support

2015-05-01 Thread Vikas Manocha
This patchset adds device tree support for stv0991 soc.

Changes in v2:
- added commit messages for both patches.

Vikas Manocha (2):
  stv0991: fdt: add stv0991 device tree
  stv0991: use fdt for serial port platform data

 arch/arm/dts/Makefile  |1 +
 arch/arm/dts/stv0991.dts   |   23 +++
 board/st/stv0991/stv0991.c |2 ++
 configs/stv0991_defconfig  |1 +
 include/configs/stv0991.h  |3 +++
 5 files changed, 30 insertions(+)
 create mode 100644 arch/arm/dts/stv0991.dts

-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-boot][PATCHv2 1/3] driver/ddr/altera: Add DDR driver for Altera's SDRAM controller

2015-05-01 Thread Marek Vasut
On Wednesday, April 29, 2015 at 04:58:14 PM, dingu...@opensource.altera.com 
wrote:
> From: Dinh Nguyen 
> 
> This patch enables the SDRAM controller that is used on Altera's SoCFPGA
> family. This patch configures the SDRAM controller based on a configuration
> file that is generated from the Quartus tool, sdram_config.h.
> 
> Signed-off-by: Dinh Nguyen 

Reviewed-by: Marek Vasut 

There's room for improvement, but the quality looks already OKish.
If there's no opposition, I'd like to pick this one patch. I still
have to check the others.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] mx6cuboxi: PHY/FEC detection

2015-05-01 Thread Vagrant Cascadian
On 2015-05-01, Fabio Estevam wrote:
> On Fri, May 1, 2015 at 5:20 PM, Fabio Estevam  wrote:
>
>> Looking at Solid-run's tree they have the following commit that may be
>> related to this PHY issue:
>
> I applied Rabeeh's patch on top of this v2 series and generated the
> two attached patches.
>
> Could you please give them a try and let us know if it fixes the
> Ethernet PHY detect issue?

After some brief testing with these two patches, it seems to be
working. After trying various methods (cold boot, reset from u-boot
prompt several consecutive times, reboot from linux several times), it
seems to consistantly detect the PHY.

Thanks for looking into it!

live well,
  vagrant

> From c0078a4fe907f8c579924f89ee5ac48e40d84736 Mon Sep 17 00:00:00 2001
> From: Fabio Estevam 
> Date: Fri, 1 May 2015 18:09:30 -0300
> Subject: [PATCH 1/2] fec: Allow passing a phy mask in 
> fecmxc_initialize_multi()
>
> Instead of only accepting a fixed PHY address, allow passing a range of PHY
> addresses through a mask address.
>
> Signed-off-by: Rabeeh Khoury 
> Signed-off-by: Fabio Estevam 
> ---
>  board/denx/m28evk/m28evk.c   |  4 ++--
>  board/freescale/mx28evk/mx28evk.c|  4 ++--
>  board/schulercontrol/sc_sps_1/sc_sps_1.c |  4 ++--
>  drivers/net/fec_mxc.c| 10 +-
>  include/netdev.h |  2 +-
>  5 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c
> index 33d38cf..7b4f67d 100644
> --- a/board/denx/m28evk/m28evk.c
> +++ b/board/denx/m28evk/m28evk.c
> @@ -131,13 +131,13 @@ int board_eth_init(bd_t *bis)
>   udelay(1);
>  #endif
>  
> - ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
> + ret = fecmxc_initialize_multi(bis, 0, 1 << 0, MXS_ENET0_BASE);
>   if (ret) {
>   printf("FEC MXS: Unable to init FEC0\n");
>   return ret;
>   }
>  
> - ret = fecmxc_initialize_multi(bis, 1, 3, MXS_ENET1_BASE);
> + ret = fecmxc_initialize_multi(bis, 1, 1 << 3, MXS_ENET1_BASE);
>   if (ret) {
>   printf("FEC MXS: Unable to init FEC1\n");
>   return ret;
> diff --git a/board/freescale/mx28evk/mx28evk.c 
> b/board/freescale/mx28evk/mx28evk.c
> index 5005fe2..4cf9332 100644
> --- a/board/freescale/mx28evk/mx28evk.c
> +++ b/board/freescale/mx28evk/mx28evk.c
> @@ -118,13 +118,13 @@ int board_eth_init(bd_t *bis)
>   udelay(200);
>   gpio_set_value(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 1);
>  
> - ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
> + ret = fecmxc_initialize_multi(bis, 0, 1 << 0, MXS_ENET0_BASE);
>   if (ret) {
>   puts("FEC MXS: Unable to init FEC0\n");
>   return ret;
>   }
>  
> - ret = fecmxc_initialize_multi(bis, 1, 3, MXS_ENET1_BASE);
> + ret = fecmxc_initialize_multi(bis, 1, 1 << 3, MXS_ENET1_BASE);
>   if (ret) {
>   puts("FEC MXS: Unable to init FEC1\n");
>   return ret;
> diff --git a/board/schulercontrol/sc_sps_1/sc_sps_1.c 
> b/board/schulercontrol/sc_sps_1/sc_sps_1.c
> index 7f0b591..fdaa383 100644
> --- a/board/schulercontrol/sc_sps_1/sc_sps_1.c
> +++ b/board/schulercontrol/sc_sps_1/sc_sps_1.c
> @@ -79,13 +79,13 @@ int board_eth_init(bd_t *bis)
>   CLKCTRL_ENET_TIME_SEL_MASK,
>   CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN);
>  
> - ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
> + ret = fecmxc_initialize_multi(bis, 0, 1 << 0, MXS_ENET0_BASE);
>   if (ret) {
>   printf("FEC MXS: Unable to init FEC0\n");
>   return ret;
>   }
>  
> - ret = fecmxc_initialize_multi(bis, 1, 1, MXS_ENET1_BASE);
> + ret = fecmxc_initialize_multi(bis, 1, 1 << 1, MXS_ENET1_BASE);
>   if (ret) {
>   printf("FEC MXS: Unable to init FEC1\n");
>   return ret;
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index b572470..9817df2 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -1077,7 +1077,7 @@ struct mii_dev *fec_get_miibus(uint32_t base_addr, int 
> dev_id)
>   return bus;
>  }
>  
> -int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
> +int fecmxc_initialize_multi(bd_t *bd, int dev_id, uint32_t phy_mask, 
> uint32_t addr)
>  {
>   uint32_t base_mii;
>   struct mii_dev *bus = NULL;
> @@ -1095,19 +1095,19 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int 
> phy_id, uint32_t addr)
>  #else
>   base_mii = addr;
>  #endif
> - debug("eth_init: fec_probe(bd, %i, %i) @ %08x\n", dev_id, phy_id, addr);
> + debug("eth_init: fec_probe(bd, %i, %i) @ %08x\n", dev_id, phy_mask, 
> addr);
>   bus = fec_get_miibus(base_mii, dev_id);
>   if (!bus)
>   return -ENOMEM;
>  #ifdef CONFIG_PHYLIB
> - phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
> + phydev = phy_find_by_mask(bus, phy

Re: [U-Boot] [PATCH 1/2] stv0991: fdt: add stv0991 device tree

2015-05-01 Thread vikasm
Hi Simon,

On 05/01/2015 04:45 PM, Simon Glass wrote:
> Hi Vikas,
>
> On 1 May 2015 at 17:18, vikasm  wrote:
>> Thanks Simon,
>>
>> On 05/01/2015 04:01 PM, Simon Glass wrote:
>>> Hi Vikas,
>>>
>>> On 1 May 2015 at 16:43, Vikas Manocha  wrote:
>>>
>>> 
>> There is not much to add in the commit message apart from info in the title,
>> that is why i didn't add any message in the body.  Please let me know if i am
>> missing something ?
> How about:
>
> "Add a device tree for the St Micro STV0991 board and enable device
> tree control. We will progressively add device tree support to drivers
> used by this board"

Thanks, i will send V2.

Rgds,
Vikas

>
 Signed-off-by: Vikas Manocha 
 ---
  arch/arm/dts/Makefile |1 +
  arch/arm/dts/stv0991.dts  |   23 +++
  configs/stv0991_defconfig |1 +
  include/configs/stv0991.h |3 +++
  4 files changed, 28 insertions(+)
  create mode 100644 arch/arm/dts/stv0991.dts

 diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
 index 46a6171..86faf58 100644
 --- a/arch/arm/dts/Makefile
 +++ b/arch/arm/dts/Makefile
 @@ -54,6 +54,7 @@ dtb-$(CONFIG_SOCFPGA) +=  \
 socfpga_arria5_socdk.dtb\
 socfpga_cyclone5_socdk.dtb  \
 socfpga_cyclone5_socrates.dtb
 +dtb-$(CONFIG_TARGET_STV0991) += stv0991.dtb
>>> Is this the only chip in this soc family? If so fine. If not, could
>>> your CONFIG be a little broader so we can (later) list all the DTBs
>>> here?
>> Yes, it is the only chip :-(, hope there will be more.
> OK.
>
>> Rgds,
>> Vikas
>>
  dtb-$(CONFIG_LS102XA) += ls1021a-qds.dtb \
 ls1021a-twr.dtb
 diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
 new file mode 100644
 index 000..b25c48b
 --- /dev/null
 +++ b/arch/arm/dts/stv0991.dts
 @@ -0,0 +1,23 @@
 +/dts-v1/;
 +
 +/ {
 +   model = "ST STV0991 application board";
 +   compatible = "st,stv0991";
 +   #address-cells = <1>;
 +   #size-cells = <1>;
 +
 +   chosen {
 +   stdout-path = &uart0;
 +   };
 +
 +   memory {
 +   device_type="memory";
 +   reg = <0x0 0x198000>;
 +   };
 +
 +   uart0: serial@0x80406000 {
 +   compatible = "arm,pl011", "arm,primecell";
 +   reg = <0x80406000 0x1000>;
 +   clock = <270>;
 +   };
 +};
 diff --git a/configs/stv0991_defconfig b/configs/stv0991_defconfig
 index 76ba41b..d9edc06 100644
 --- a/configs/stv0991_defconfig
 +++ b/configs/stv0991_defconfig
 @@ -5,3 +5,4 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
  CONFIG_ETH_DESIGNWARE=y
  CONFIG_NETDEVICES=y
  CONFIG_NET=y
 +CONFIG_DEFAULT_DEVICE_TREE="stv0991"
 diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
 index 2f65eda..750eebd 100644
 --- a/include/configs/stv0991.h
 +++ b/include/configs/stv0991.h
 @@ -80,4 +80,7 @@
  #define CONFIG_AUTOBOOT_PROMPT \
 "Hit SPACE in %d seconds to stop autoboot.\n", bootdelay
  #undef CONFIG_HAS_VBAR
 +#define CONFIG_OF_EMBED
 +#define CONFIG_OF_CONTROL
 +#define CONFIG_OF_LIBFDT
  #endif /* __CONFIG_H */
 --
 1.7.9.5

>>> Regards,
>>> Simon
> Regards,
> Simon

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] stv0991: fdt: add stv0991 device tree

2015-05-01 Thread Simon Glass
Hi Vikas,

On 1 May 2015 at 17:18, vikasm  wrote:
> Thanks Simon,
>
> On 05/01/2015 04:01 PM, Simon Glass wrote:
>> Hi Vikas,
>>
>> On 1 May 2015 at 16:43, Vikas Manocha  wrote:
>>
>> 
>
> There is not much to add in the commit message apart from info in the title,
> that is why i didn't add any message in the body.  Please let me know if i am
> missing something ?

How about:

"Add a device tree for the St Micro STV0991 board and enable device
tree control. We will progressively add device tree support to drivers
used by this board"

>
>>
>>> Signed-off-by: Vikas Manocha 
>>> ---
>>>  arch/arm/dts/Makefile |1 +
>>>  arch/arm/dts/stv0991.dts  |   23 +++
>>>  configs/stv0991_defconfig |1 +
>>>  include/configs/stv0991.h |3 +++
>>>  4 files changed, 28 insertions(+)
>>>  create mode 100644 arch/arm/dts/stv0991.dts
>>>
>>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>>> index 46a6171..86faf58 100644
>>> --- a/arch/arm/dts/Makefile
>>> +++ b/arch/arm/dts/Makefile
>>> @@ -54,6 +54,7 @@ dtb-$(CONFIG_SOCFPGA) +=  \
>>> socfpga_arria5_socdk.dtb\
>>> socfpga_cyclone5_socdk.dtb  \
>>> socfpga_cyclone5_socrates.dtb
>>> +dtb-$(CONFIG_TARGET_STV0991) += stv0991.dtb
>> Is this the only chip in this soc family? If so fine. If not, could
>> your CONFIG be a little broader so we can (later) list all the DTBs
>> here?
>
> Yes, it is the only chip :-(, hope there will be more.

OK.

>
> Rgds,
> Vikas
>
>>>  dtb-$(CONFIG_LS102XA) += ls1021a-qds.dtb \
>>> ls1021a-twr.dtb
>>> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
>>> new file mode 100644
>>> index 000..b25c48b
>>> --- /dev/null
>>> +++ b/arch/arm/dts/stv0991.dts
>>> @@ -0,0 +1,23 @@
>>> +/dts-v1/;
>>> +
>>> +/ {
>>> +   model = "ST STV0991 application board";
>>> +   compatible = "st,stv0991";
>>> +   #address-cells = <1>;
>>> +   #size-cells = <1>;
>>> +
>>> +   chosen {
>>> +   stdout-path = &uart0;
>>> +   };
>>> +
>>> +   memory {
>>> +   device_type="memory";
>>> +   reg = <0x0 0x198000>;
>>> +   };
>>> +
>>> +   uart0: serial@0x80406000 {
>>> +   compatible = "arm,pl011", "arm,primecell";
>>> +   reg = <0x80406000 0x1000>;
>>> +   clock = <270>;
>>> +   };
>>> +};
>>> diff --git a/configs/stv0991_defconfig b/configs/stv0991_defconfig
>>> index 76ba41b..d9edc06 100644
>>> --- a/configs/stv0991_defconfig
>>> +++ b/configs/stv0991_defconfig
>>> @@ -5,3 +5,4 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
>>>  CONFIG_ETH_DESIGNWARE=y
>>>  CONFIG_NETDEVICES=y
>>>  CONFIG_NET=y
>>> +CONFIG_DEFAULT_DEVICE_TREE="stv0991"
>>> diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
>>> index 2f65eda..750eebd 100644
>>> --- a/include/configs/stv0991.h
>>> +++ b/include/configs/stv0991.h
>>> @@ -80,4 +80,7 @@
>>>  #define CONFIG_AUTOBOOT_PROMPT \
>>> "Hit SPACE in %d seconds to stop autoboot.\n", bootdelay
>>>  #undef CONFIG_HAS_VBAR
>>> +#define CONFIG_OF_EMBED
>>> +#define CONFIG_OF_CONTROL
>>> +#define CONFIG_OF_LIBFDT
>>>  #endif /* __CONFIG_H */
>>> --
>>> 1.7.9.5
>>>
>> Regards,
>> Simon
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] serial: fdt: add device tree support for pl01x

2015-05-01 Thread vikasm
Thanks Simon,

On 05/01/2015 03:02 PM, Simon Glass wrote:
> +Masahiro, for my of_match_ptr() comment below.
>
> Hi Vikas,
>
> On 1 May 2015 at 15:48, Vikas Manocha  wrote:
>> This patch adds device tree support for arm pl010/pl011 driver.
>>
>> Signed-off-by: Vikas Manocha 
>> ---
>>  doc/device-tree-bindings/serial/pl01x.txt |7 +
>>  drivers/serial/serial_pl01x.c |   41 
>> -
>>  2 files changed, 47 insertions(+), 1 deletion(-)
>>  create mode 100644 doc/device-tree-bindings/serial/pl01x.txt
>>
>> diff --git a/doc/device-tree-bindings/serial/pl01x.txt 
>> b/doc/device-tree-bindings/serial/pl01x.txt
>> new file mode 100644
>> index 000..61c27d1
>> --- /dev/null
>> +++ b/doc/device-tree-bindings/serial/pl01x.txt
>> @@ -0,0 +1,7 @@
>> +* ARM AMBA Primecell PL011 & PL010 serial UART
>> +
>> +Required properties:
>> +- compatible: must be "arm,primecell", "arm,pl011" or "arm,pl010"
>> +- reg: exactly one register range with length 0x1000
>> +- clock: input clock frequency for the UART (used to calculate the baud
>> +  rate divisor)
>> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
>> index 2124161..ea22cfd 100644
>> --- a/drivers/serial/serial_pl01x.c
>> +++ b/drivers/serial/serial_pl01x.c
>> @@ -20,6 +20,9 @@
>>  #include 
>>  #include 
>>  #include "serial_pl01x_internal.h"
>> +#include 
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>>
>>  #ifndef CONFIG_DM_SERIAL
>>
>> @@ -28,7 +31,6 @@ static enum pl01x_type pl01x_type __attribute__ 
>> ((section(".data")));
>>  static struct pl01x_regs *base_regs __attribute__ ((section(".data")));
>>  #define NUM_PORTS (sizeof(port)/sizeof(port[0]))
>>
>> -DECLARE_GLOBAL_DATA_PTR;
>>  #endif
>>
>>  static int pl01x_putc(struct pl01x_regs *regs, char c)
>> @@ -351,9 +353,46 @@ static const struct dm_serial_ops pl01x_serial_ops = {
>> .setbrg = pl01x_serial_setbrg,
>>  };
>>
>> +#ifdef CONFIG_OF_CONTROL
>> +static const struct udevice_id pl01x_serial_id[] ={
>> +   {.compatible = "arm,pl011"},
>> +   {.compatible = "arm,pl010"},
> You can use:
>
>   {.compatible = "arm,pl011", .data = TYPE_PL011},

Thanks for the suggestion.

>> +   {}
>> +};
>> +
>> +static int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +   struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
>> +   fdt_addr_t addr;
>> +   const char* type_pl01x;
>> +
>> +   addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
>> +   if (addr == FDT_ADDR_T_NONE)
>> +   return -EINVAL;
>> +
>> +   plat->base = addr;
>> +   plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock", 
>> 1);
>> +   type_pl01x = fdt_getprop(gd->fdt_blob, dev->of_offset, \
>> +   "compatible", NULL);
> plat->type = dev_get_driver_data(dev);

completely agree, it would make the code much cleaner.

>> +   if(!strcmp(type_pl01x, "arm,pl011"))
>> +   plat->type = TYPE_PL011;
>> +   else if(!strcmp(type_pl01x, "arm,pl010"))
>> +   plat->type = TYPE_PL010;
>> +   else
>> +   return -EINVAL;
> Should be able to drop this line.

yes, all the above block.

>> +
>> +   return 0;
>> +}
>> +#endif
>> +
>>  U_BOOT_DRIVER(serial_pl01x) = {
>> .name   = "serial_pl01x",
>> .id = UCLASS_SERIAL,
>> +#ifdef CONFIG_OF_CONTROL
>> +   .of_match = pl01x_serial_id,
>> +   .ofdata_to_platdata = pl01x_serial_ofdata_to_platdata,
>> +   .platdata_auto_alloc_size = sizeof(struct pl01x_serial_platdata),
>> +#endif
> Would be good to get rid of the #ifdef.
>
> I think you can put the last one outside the #ifdef, since
> device_bind() will do the right thing if there is already platform
> data.

ok.

>
> For the first one you can do   .of_match = of_match_ptr(pl01x_serial_id),

ok, i will check it.

Rgds,
Vikas

>
> But the middle line (ofdata_to_platdata) does need an #ifdef I think.
> Perhaps we could create something similar to of_match_ptr() for this
> case?
>
>> .probe = pl01x_serial_probe,
>> .ops= &pl01x_serial_ops,
>> .flags = DM_FLAG_PRE_RELOC,
>> --
>> 1.7.9.5
>>
> Regards,
> Simon

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] stv0991: fdt: add stv0991 device tree

2015-05-01 Thread vikasm
Thanks Simon,

On 05/01/2015 04:01 PM, Simon Glass wrote:
> Hi Vikas,
>
> On 1 May 2015 at 16:43, Vikas Manocha  wrote:
>
> 

There is not much to add in the commit message apart from info in the title,
that is why i didn't add any message in the body.  Please let me know if i am
missing something ?

>
>> Signed-off-by: Vikas Manocha 
>> ---
>>  arch/arm/dts/Makefile |1 +
>>  arch/arm/dts/stv0991.dts  |   23 +++
>>  configs/stv0991_defconfig |1 +
>>  include/configs/stv0991.h |3 +++
>>  4 files changed, 28 insertions(+)
>>  create mode 100644 arch/arm/dts/stv0991.dts
>>
>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>> index 46a6171..86faf58 100644
>> --- a/arch/arm/dts/Makefile
>> +++ b/arch/arm/dts/Makefile
>> @@ -54,6 +54,7 @@ dtb-$(CONFIG_SOCFPGA) +=  \
>> socfpga_arria5_socdk.dtb\
>> socfpga_cyclone5_socdk.dtb  \
>> socfpga_cyclone5_socrates.dtb
>> +dtb-$(CONFIG_TARGET_STV0991) += stv0991.dtb
> Is this the only chip in this soc family? If so fine. If not, could
> your CONFIG be a little broader so we can (later) list all the DTBs
> here?

Yes, it is the only chip :-(, hope there will be more.

Rgds,
Vikas

>>  dtb-$(CONFIG_LS102XA) += ls1021a-qds.dtb \
>> ls1021a-twr.dtb
>> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
>> new file mode 100644
>> index 000..b25c48b
>> --- /dev/null
>> +++ b/arch/arm/dts/stv0991.dts
>> @@ -0,0 +1,23 @@
>> +/dts-v1/;
>> +
>> +/ {
>> +   model = "ST STV0991 application board";
>> +   compatible = "st,stv0991";
>> +   #address-cells = <1>;
>> +   #size-cells = <1>;
>> +
>> +   chosen {
>> +   stdout-path = &uart0;
>> +   };
>> +
>> +   memory {
>> +   device_type="memory";
>> +   reg = <0x0 0x198000>;
>> +   };
>> +
>> +   uart0: serial@0x80406000 {
>> +   compatible = "arm,pl011", "arm,primecell";
>> +   reg = <0x80406000 0x1000>;
>> +   clock = <270>;
>> +   };
>> +};
>> diff --git a/configs/stv0991_defconfig b/configs/stv0991_defconfig
>> index 76ba41b..d9edc06 100644
>> --- a/configs/stv0991_defconfig
>> +++ b/configs/stv0991_defconfig
>> @@ -5,3 +5,4 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
>>  CONFIG_ETH_DESIGNWARE=y
>>  CONFIG_NETDEVICES=y
>>  CONFIG_NET=y
>> +CONFIG_DEFAULT_DEVICE_TREE="stv0991"
>> diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
>> index 2f65eda..750eebd 100644
>> --- a/include/configs/stv0991.h
>> +++ b/include/configs/stv0991.h
>> @@ -80,4 +80,7 @@
>>  #define CONFIG_AUTOBOOT_PROMPT \
>> "Hit SPACE in %d seconds to stop autoboot.\n", bootdelay
>>  #undef CONFIG_HAS_VBAR
>> +#define CONFIG_OF_EMBED
>> +#define CONFIG_OF_CONTROL
>> +#define CONFIG_OF_LIBFDT
>>  #endif /* __CONFIG_H */
>> --
>> 1.7.9.5
>>
> Regards,
> Simon

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/2] stv0991: use fdt for serial port platform data

2015-05-01 Thread Simon Glass
On 1 May 2015 at 16:43, Vikas Manocha  wrote:
> Signed-off-by: Vikas Manocha 
> ---
>  board/st/stv0991/stv0991.c |2 ++
>  1 file changed, 2 insertions(+)

Needs a commit message. With that:

Reviewed-by: Simon Glass 

>
> diff --git a/board/st/stv0991/stv0991.c b/board/st/stv0991/stv0991.c
> index 38f6e1d..09f973f 100644
> --- a/board/st/stv0991/stv0991.c
> +++ b/board/st/stv0991/stv0991.c
> @@ -21,6 +21,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  struct gpio_regs *const gpioa_regs =
> (struct gpio_regs *) GPIOA_BASE_ADDR;
>
> +#ifndef CONFIG_OF_CONTROL
>  static const struct pl01x_serial_platdata serial_platdata = {
> .base = 0x80406000,
> .type = TYPE_PL011,
> @@ -31,6 +32,7 @@ U_BOOT_DEVICE(stv09911_serials) = {
> .name = "serial_pl01x",
> .platdata = &serial_platdata,
>  };
> +#endif
>
>  #ifdef CONFIG_SHOW_BOOT_PROGRESS
>  void show_boot_progress(int progress)
> --
> 1.7.9.5
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] stv0991: fdt: add stv0991 device tree

2015-05-01 Thread Simon Glass
Hi Vikas,

On 1 May 2015 at 16:43, Vikas Manocha  wrote:



> Signed-off-by: Vikas Manocha 
> ---
>  arch/arm/dts/Makefile |1 +
>  arch/arm/dts/stv0991.dts  |   23 +++
>  configs/stv0991_defconfig |1 +
>  include/configs/stv0991.h |3 +++
>  4 files changed, 28 insertions(+)
>  create mode 100644 arch/arm/dts/stv0991.dts
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 46a6171..86faf58 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -54,6 +54,7 @@ dtb-$(CONFIG_SOCFPGA) +=  \
> socfpga_arria5_socdk.dtb\
> socfpga_cyclone5_socdk.dtb  \
> socfpga_cyclone5_socrates.dtb
> +dtb-$(CONFIG_TARGET_STV0991) += stv0991.dtb

Is this the only chip in this soc family? If so fine. If not, could
your CONFIG be a little broader so we can (later) list all the DTBs
here?

>
>  dtb-$(CONFIG_LS102XA) += ls1021a-qds.dtb \
> ls1021a-twr.dtb
> diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
> new file mode 100644
> index 000..b25c48b
> --- /dev/null
> +++ b/arch/arm/dts/stv0991.dts
> @@ -0,0 +1,23 @@
> +/dts-v1/;
> +
> +/ {
> +   model = "ST STV0991 application board";
> +   compatible = "st,stv0991";
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +
> +   chosen {
> +   stdout-path = &uart0;
> +   };
> +
> +   memory {
> +   device_type="memory";
> +   reg = <0x0 0x198000>;
> +   };
> +
> +   uart0: serial@0x80406000 {
> +   compatible = "arm,pl011", "arm,primecell";
> +   reg = <0x80406000 0x1000>;
> +   clock = <270>;
> +   };
> +};
> diff --git a/configs/stv0991_defconfig b/configs/stv0991_defconfig
> index 76ba41b..d9edc06 100644
> --- a/configs/stv0991_defconfig
> +++ b/configs/stv0991_defconfig
> @@ -5,3 +5,4 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
>  CONFIG_ETH_DESIGNWARE=y
>  CONFIG_NETDEVICES=y
>  CONFIG_NET=y
> +CONFIG_DEFAULT_DEVICE_TREE="stv0991"
> diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
> index 2f65eda..750eebd 100644
> --- a/include/configs/stv0991.h
> +++ b/include/configs/stv0991.h
> @@ -80,4 +80,7 @@
>  #define CONFIG_AUTOBOOT_PROMPT \
> "Hit SPACE in %d seconds to stop autoboot.\n", bootdelay
>  #undef CONFIG_HAS_VBAR
> +#define CONFIG_OF_EMBED
> +#define CONFIG_OF_CONTROL
> +#define CONFIG_OF_LIBFDT
>  #endif /* __CONFIG_H */
> --
> 1.7.9.5
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 2/2] stv0991: use fdt for serial port platform data

2015-05-01 Thread Vikas Manocha
Signed-off-by: Vikas Manocha 
---
 board/st/stv0991/stv0991.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/board/st/stv0991/stv0991.c b/board/st/stv0991/stv0991.c
index 38f6e1d..09f973f 100644
--- a/board/st/stv0991/stv0991.c
+++ b/board/st/stv0991/stv0991.c
@@ -21,6 +21,7 @@ DECLARE_GLOBAL_DATA_PTR;
 struct gpio_regs *const gpioa_regs =
(struct gpio_regs *) GPIOA_BASE_ADDR;
 
+#ifndef CONFIG_OF_CONTROL
 static const struct pl01x_serial_platdata serial_platdata = {
.base = 0x80406000,
.type = TYPE_PL011,
@@ -31,6 +32,7 @@ U_BOOT_DEVICE(stv09911_serials) = {
.name = "serial_pl01x",
.platdata = &serial_platdata,
 };
+#endif
 
 #ifdef CONFIG_SHOW_BOOT_PROGRESS
 void show_boot_progress(int progress)
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 0/2] stv0991: Add flat device tree support

2015-05-01 Thread Vikas Manocha
This patchset adds device tree support for stv0991 soc.

Vikas Manocha (2):
  stv0991: fdt: add stv0991 device tree
  stv0991: use fdt for serial port platform data

 arch/arm/dts/Makefile  |1 +
 arch/arm/dts/stv0991.dts   |   23 +++
 board/st/stv0991/stv0991.c |2 ++
 configs/stv0991_defconfig  |1 +
 include/configs/stv0991.h  |3 +++
 5 files changed, 30 insertions(+)
 create mode 100644 arch/arm/dts/stv0991.dts

-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 1/2] stv0991: fdt: add stv0991 device tree

2015-05-01 Thread Vikas Manocha
Signed-off-by: Vikas Manocha 
---
 arch/arm/dts/Makefile |1 +
 arch/arm/dts/stv0991.dts  |   23 +++
 configs/stv0991_defconfig |1 +
 include/configs/stv0991.h |3 +++
 4 files changed, 28 insertions(+)
 create mode 100644 arch/arm/dts/stv0991.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 46a6171..86faf58 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -54,6 +54,7 @@ dtb-$(CONFIG_SOCFPGA) +=  \
socfpga_arria5_socdk.dtb\
socfpga_cyclone5_socdk.dtb  \
socfpga_cyclone5_socrates.dtb
+dtb-$(CONFIG_TARGET_STV0991) += stv0991.dtb
 
 dtb-$(CONFIG_LS102XA) += ls1021a-qds.dtb \
ls1021a-twr.dtb
diff --git a/arch/arm/dts/stv0991.dts b/arch/arm/dts/stv0991.dts
new file mode 100644
index 000..b25c48b
--- /dev/null
+++ b/arch/arm/dts/stv0991.dts
@@ -0,0 +1,23 @@
+/dts-v1/;
+
+/ {
+   model = "ST STV0991 application board";
+   compatible = "st,stv0991";
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   chosen {
+   stdout-path = &uart0;
+   };
+
+   memory {
+   device_type="memory";
+   reg = <0x0 0x198000>;
+   };
+
+   uart0: serial@0x80406000 {
+   compatible = "arm,pl011", "arm,primecell";
+   reg = <0x80406000 0x1000>;
+   clock = <270>;
+   };
+};
diff --git a/configs/stv0991_defconfig b/configs/stv0991_defconfig
index 76ba41b..d9edc06 100644
--- a/configs/stv0991_defconfig
+++ b/configs/stv0991_defconfig
@@ -5,3 +5,4 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_NETDEVICES=y
 CONFIG_NET=y
+CONFIG_DEFAULT_DEVICE_TREE="stv0991"
diff --git a/include/configs/stv0991.h b/include/configs/stv0991.h
index 2f65eda..750eebd 100644
--- a/include/configs/stv0991.h
+++ b/include/configs/stv0991.h
@@ -80,4 +80,7 @@
 #define CONFIG_AUTOBOOT_PROMPT \
"Hit SPACE in %d seconds to stop autoboot.\n", bootdelay
 #undef CONFIG_HAS_VBAR
+#define CONFIG_OF_EMBED
+#define CONFIG_OF_CONTROL
+#define CONFIG_OF_LIBFDT
 #endif /* __CONFIG_H */
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] serial: fdt: add device tree support for pl01x

2015-05-01 Thread Simon Glass
+Masahiro, for my of_match_ptr() comment below.

Hi Vikas,

On 1 May 2015 at 15:48, Vikas Manocha  wrote:
> This patch adds device tree support for arm pl010/pl011 driver.
>
> Signed-off-by: Vikas Manocha 
> ---
>  doc/device-tree-bindings/serial/pl01x.txt |7 +
>  drivers/serial/serial_pl01x.c |   41 
> -
>  2 files changed, 47 insertions(+), 1 deletion(-)
>  create mode 100644 doc/device-tree-bindings/serial/pl01x.txt
>
> diff --git a/doc/device-tree-bindings/serial/pl01x.txt 
> b/doc/device-tree-bindings/serial/pl01x.txt
> new file mode 100644
> index 000..61c27d1
> --- /dev/null
> +++ b/doc/device-tree-bindings/serial/pl01x.txt
> @@ -0,0 +1,7 @@
> +* ARM AMBA Primecell PL011 & PL010 serial UART
> +
> +Required properties:
> +- compatible: must be "arm,primecell", "arm,pl011" or "arm,pl010"
> +- reg: exactly one register range with length 0x1000
> +- clock: input clock frequency for the UART (used to calculate the baud
> +  rate divisor)
> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
> index 2124161..ea22cfd 100644
> --- a/drivers/serial/serial_pl01x.c
> +++ b/drivers/serial/serial_pl01x.c
> @@ -20,6 +20,9 @@
>  #include 
>  #include 
>  #include "serial_pl01x_internal.h"
> +#include 
> +
> +DECLARE_GLOBAL_DATA_PTR;
>
>  #ifndef CONFIG_DM_SERIAL
>
> @@ -28,7 +31,6 @@ static enum pl01x_type pl01x_type __attribute__ 
> ((section(".data")));
>  static struct pl01x_regs *base_regs __attribute__ ((section(".data")));
>  #define NUM_PORTS (sizeof(port)/sizeof(port[0]))
>
> -DECLARE_GLOBAL_DATA_PTR;
>  #endif
>
>  static int pl01x_putc(struct pl01x_regs *regs, char c)
> @@ -351,9 +353,46 @@ static const struct dm_serial_ops pl01x_serial_ops = {
> .setbrg = pl01x_serial_setbrg,
>  };
>
> +#ifdef CONFIG_OF_CONTROL
> +static const struct udevice_id pl01x_serial_id[] ={
> +   {.compatible = "arm,pl011"},
> +   {.compatible = "arm,pl010"},

You can use:

  {.compatible = "arm,pl011", .data = TYPE_PL011},

> +   {}
> +};
> +
> +static int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
> +{
> +   struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
> +   fdt_addr_t addr;
> +   const char* type_pl01x;
> +
> +   addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
> +   if (addr == FDT_ADDR_T_NONE)
> +   return -EINVAL;
> +
> +   plat->base = addr;
> +   plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock", 
> 1);
> +   type_pl01x = fdt_getprop(gd->fdt_blob, dev->of_offset, \
> +   "compatible", NULL);

plat->type = dev_get_driver_data(dev);

> +   if(!strcmp(type_pl01x, "arm,pl011"))
> +   plat->type = TYPE_PL011;
> +   else if(!strcmp(type_pl01x, "arm,pl010"))
> +   plat->type = TYPE_PL010;
> +   else
> +   return -EINVAL;

Should be able to drop this line.

> +
> +   return 0;
> +}
> +#endif
> +
>  U_BOOT_DRIVER(serial_pl01x) = {
> .name   = "serial_pl01x",
> .id = UCLASS_SERIAL,
> +#ifdef CONFIG_OF_CONTROL
> +   .of_match = pl01x_serial_id,
> +   .ofdata_to_platdata = pl01x_serial_ofdata_to_platdata,
> +   .platdata_auto_alloc_size = sizeof(struct pl01x_serial_platdata),
> +#endif

Would be good to get rid of the #ifdef.

I think you can put the last one outside the #ifdef, since
device_bind() will do the right thing if there is already platform
data.

For the first one you can do   .of_match = of_match_ptr(pl01x_serial_id),

But the middle line (ofdata_to_platdata) does need an #ifdef I think.
Perhaps we could create something similar to of_match_ptr() for this
case?

> .probe = pl01x_serial_probe,
> .ops= &pl01x_serial_ops,
> .flags = DM_FLAG_PRE_RELOC,
> --
> 1.7.9.5
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] serial: fdt: add device tree support for pl01x

2015-05-01 Thread Vikas Manocha
This patch adds device tree support for arm pl010/pl011 driver.

Signed-off-by: Vikas Manocha 
---
 doc/device-tree-bindings/serial/pl01x.txt |7 +
 drivers/serial/serial_pl01x.c |   41 -
 2 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 doc/device-tree-bindings/serial/pl01x.txt

diff --git a/doc/device-tree-bindings/serial/pl01x.txt 
b/doc/device-tree-bindings/serial/pl01x.txt
new file mode 100644
index 000..61c27d1
--- /dev/null
+++ b/doc/device-tree-bindings/serial/pl01x.txt
@@ -0,0 +1,7 @@
+* ARM AMBA Primecell PL011 & PL010 serial UART
+
+Required properties:
+- compatible: must be "arm,primecell", "arm,pl011" or "arm,pl010"
+- reg: exactly one register range with length 0x1000
+- clock: input clock frequency for the UART (used to calculate the baud
+  rate divisor)
diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 2124161..ea22cfd 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -20,6 +20,9 @@
 #include 
 #include 
 #include "serial_pl01x_internal.h"
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
 
 #ifndef CONFIG_DM_SERIAL
 
@@ -28,7 +31,6 @@ static enum pl01x_type pl01x_type __attribute__ 
((section(".data")));
 static struct pl01x_regs *base_regs __attribute__ ((section(".data")));
 #define NUM_PORTS (sizeof(port)/sizeof(port[0]))
 
-DECLARE_GLOBAL_DATA_PTR;
 #endif
 
 static int pl01x_putc(struct pl01x_regs *regs, char c)
@@ -351,9 +353,46 @@ static const struct dm_serial_ops pl01x_serial_ops = {
.setbrg = pl01x_serial_setbrg,
 };
 
+#ifdef CONFIG_OF_CONTROL
+static const struct udevice_id pl01x_serial_id[] ={
+   {.compatible = "arm,pl011"},
+   {.compatible = "arm,pl010"},
+   {}
+};
+
+static int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
+{
+   struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
+   fdt_addr_t addr;
+   const char* type_pl01x;
+
+   addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
+   if (addr == FDT_ADDR_T_NONE)
+   return -EINVAL;
+
+   plat->base = addr;
+   plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "clock", 1);
+   type_pl01x = fdt_getprop(gd->fdt_blob, dev->of_offset, \
+   "compatible", NULL);
+   if(!strcmp(type_pl01x, "arm,pl011"))
+   plat->type = TYPE_PL011;
+   else if(!strcmp(type_pl01x, "arm,pl010"))
+   plat->type = TYPE_PL010;
+   else
+   return -EINVAL;
+
+   return 0;
+}
+#endif
+
 U_BOOT_DRIVER(serial_pl01x) = {
.name   = "serial_pl01x",
.id = UCLASS_SERIAL,
+#ifdef CONFIG_OF_CONTROL
+   .of_match = pl01x_serial_id,
+   .ofdata_to_platdata = pl01x_serial_ofdata_to_platdata,
+   .platdata_auto_alloc_size = sizeof(struct pl01x_serial_platdata),
+#endif
.probe = pl01x_serial_probe,
.ops= &pl01x_serial_ops,
.flags = DM_FLAG_PRE_RELOC,
-- 
1.7.9.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/4] mx6cuboxi: Add USB host support

2015-05-01 Thread Fabio Estevam
Vagrant,

On Fri, May 1, 2015 at 5:20 PM, Fabio Estevam  wrote:

> Looking at Solid-run's tree they have the following commit that may be
> related to this PHY issue:

I applied Rabeeh's patch on top of this v2 series and generated the
two attached patches.

Could you please give them a try and let us know if it fixes the
Ethernet PHY detect issue?

Regards,

Fabio Estevam
From c0078a4fe907f8c579924f89ee5ac48e40d84736 Mon Sep 17 00:00:00 2001
From: Fabio Estevam 
Date: Fri, 1 May 2015 18:09:30 -0300
Subject: [PATCH 1/2] fec: Allow passing a phy mask in fecmxc_initialize_multi()

Instead of only accepting a fixed PHY address, allow passing a range of PHY
addresses through a mask address.

Signed-off-by: Rabeeh Khoury 
Signed-off-by: Fabio Estevam 
---
 board/denx/m28evk/m28evk.c   |  4 ++--
 board/freescale/mx28evk/mx28evk.c|  4 ++--
 board/schulercontrol/sc_sps_1/sc_sps_1.c |  4 ++--
 drivers/net/fec_mxc.c| 10 +-
 include/netdev.h |  2 +-
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c
index 33d38cf..7b4f67d 100644
--- a/board/denx/m28evk/m28evk.c
+++ b/board/denx/m28evk/m28evk.c
@@ -131,13 +131,13 @@ int board_eth_init(bd_t *bis)
udelay(1);
 #endif
 
-   ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
+   ret = fecmxc_initialize_multi(bis, 0, 1 << 0, MXS_ENET0_BASE);
if (ret) {
printf("FEC MXS: Unable to init FEC0\n");
return ret;
}
 
-   ret = fecmxc_initialize_multi(bis, 1, 3, MXS_ENET1_BASE);
+   ret = fecmxc_initialize_multi(bis, 1, 1 << 3, MXS_ENET1_BASE);
if (ret) {
printf("FEC MXS: Unable to init FEC1\n");
return ret;
diff --git a/board/freescale/mx28evk/mx28evk.c 
b/board/freescale/mx28evk/mx28evk.c
index 5005fe2..4cf9332 100644
--- a/board/freescale/mx28evk/mx28evk.c
+++ b/board/freescale/mx28evk/mx28evk.c
@@ -118,13 +118,13 @@ int board_eth_init(bd_t *bis)
udelay(200);
gpio_set_value(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 1);
 
-   ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
+   ret = fecmxc_initialize_multi(bis, 0, 1 << 0, MXS_ENET0_BASE);
if (ret) {
puts("FEC MXS: Unable to init FEC0\n");
return ret;
}
 
-   ret = fecmxc_initialize_multi(bis, 1, 3, MXS_ENET1_BASE);
+   ret = fecmxc_initialize_multi(bis, 1, 1 << 3, MXS_ENET1_BASE);
if (ret) {
puts("FEC MXS: Unable to init FEC1\n");
return ret;
diff --git a/board/schulercontrol/sc_sps_1/sc_sps_1.c 
b/board/schulercontrol/sc_sps_1/sc_sps_1.c
index 7f0b591..fdaa383 100644
--- a/board/schulercontrol/sc_sps_1/sc_sps_1.c
+++ b/board/schulercontrol/sc_sps_1/sc_sps_1.c
@@ -79,13 +79,13 @@ int board_eth_init(bd_t *bis)
CLKCTRL_ENET_TIME_SEL_MASK,
CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN);
 
-   ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
+   ret = fecmxc_initialize_multi(bis, 0, 1 << 0, MXS_ENET0_BASE);
if (ret) {
printf("FEC MXS: Unable to init FEC0\n");
return ret;
}
 
-   ret = fecmxc_initialize_multi(bis, 1, 1, MXS_ENET1_BASE);
+   ret = fecmxc_initialize_multi(bis, 1, 1 << 1, MXS_ENET1_BASE);
if (ret) {
printf("FEC MXS: Unable to init FEC1\n");
return ret;
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index b572470..9817df2 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -1077,7 +1077,7 @@ struct mii_dev *fec_get_miibus(uint32_t base_addr, int 
dev_id)
return bus;
 }
 
-int fecmxc_initialize_multi(bd_t *bd, int dev_id, int phy_id, uint32_t addr)
+int fecmxc_initialize_multi(bd_t *bd, int dev_id, uint32_t phy_mask, uint32_t 
addr)
 {
uint32_t base_mii;
struct mii_dev *bus = NULL;
@@ -1095,19 +1095,19 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int 
phy_id, uint32_t addr)
 #else
base_mii = addr;
 #endif
-   debug("eth_init: fec_probe(bd, %i, %i) @ %08x\n", dev_id, phy_id, addr);
+   debug("eth_init: fec_probe(bd, %i, %i) @ %08x\n", dev_id, phy_mask, 
addr);
bus = fec_get_miibus(base_mii, dev_id);
if (!bus)
return -ENOMEM;
 #ifdef CONFIG_PHYLIB
-   phydev = phy_find_by_mask(bus, 1 << phy_id, PHY_INTERFACE_MODE_RGMII);
+   phydev = phy_find_by_mask(bus, phy_mask, PHY_INTERFACE_MODE_RGMII);
if (!phydev) {
free(bus);
return -ENOMEM;
}
ret = fec_probe(bd, dev_id, addr, bus, phydev);
 #else
-   ret = fec_probe(bd, dev_id, addr, bus, phy_id);
+   ret = fec_probe(bd, dev_id, addr, bus, phy_mask);
 #endif
if (ret) {
 #ifdef CONFIG_PHYLIB
@@ -1121,7 +1121,7 @@ int fecmxc_initialize_multi(bd_t *bd, int dev_id, int 
p

[U-Boot] Please pull u-boot-x86

2015-05-01 Thread Simon Glass
Hi Tom,

This includes PIRQ routing support for a few platforms, the beginnings
of better GPIO/pinmux support and multi-core + SFI support.


The following changes since commit ace97d26176a3ebc9ec07738450de93eea35975c:

  Merge branch 'zynq' of git://www.denx.de/git/u-boot-microblaze
(2015-04-29 06:46:33 -0400)

are available in the git repository at:

  http://git.denx.de/u-boot-x86.git

for you to fetch changes up to 281239ad9dc2a695a53ab34dda44cdbe31c69122:

  x86: Enable multi-core init for Minnowboard MAX (2015-04-30 16:13:52 -0600)


Bin Meng (23):
  x86: minnowmax: Remove CONFIG_VIDEO_X86 in the defconfig
  x86: Remove the old VGA driver
  x86: queensbay: Avoid using PCH prefix
  x86: Move CONFIG_ENV_IS_IN_SPI_FLASH to x86-common.h
  x86: Set serial port IRQ for SMSC LPC47M
  x86: Add alias for SPI node in the board dts
  x86: Clean up arch/x86/include/asm/e820.h
  x86: Install a default e820 table in the __weak install_e820_map()
  x86: Add a function to assign IRQ numbers to PCI device
  x86: Write configuration tables in last_stage_init()
  x86: Support platform PIRQ routing
  x86: queensbay: Implement PIRQ routing
  pci: Option rom class is a 24-bit number
  pci: Remove parameter 'class' of pci_rom_load()
  biosemu: Do not free vga_info->BIOSImage when it is 0xc
  x86: Check PIRQ routing table sanity in the F segment
  x86: quark: Turn on legacy segments decode
  x86: Kconfig: Divide the target selection to vendor/model
  x86: Kconfig: Move platform options forward
  x86: Kconfig: MARK_GRAPHICS_MEM_WRCOMB cosmetics
  x86: Kconfig: Move DM_SPI & DM_SPI_FLASH to arch/Kconfig
  x86: Kconfig: Remove deprecated CONFIG_SYS_EXTRA_OPTIONS
  x86: Correct the typo in write_tables()

Gabriel Huau (3):
  x86: baytrail: fix the GPIOBASE address
  x86: minnowmax: add GPIO banks in the device tree
  x86: minnowmax: use the correct NOR in the configuration

Simon Glass (26):
  x86: Correct Minnowboard instructions to use the right descriptor
  x86: Update chromebook_link instructions for binary blob
  x86: link: Add PCH driver to support SPI Flash
  x86: Implement reset_cpu() correctly for modern CPUs
  x86: ivybridge: Use reset_cpu()
  x86: quark: Use reset_cpu()
  x86: fsp: Use reset_cpu()
  Fix comment nits in board_f.c
  dm: core: Add a function to bind a driver for a device tree node
  x86: Remove unwanted MMC debugging
  x86: Disable -Werror
  Move display_options functions to their own header
  Add print_freq() to display frequencies nicely
  dm: Implement a CPU uclass
  x86: Add support for the Simple Firmware Interface (SFI)
  Add a 'cpu' command to print CPU information
  x86: Add atomic operations
  x86: Add defines for fixed MTRRs
  x86: Add an mfence macro
  x86: Store the GDT pointer in global_data
  x86: Provide access to the IDT
  x86: Add multi-processor init
  x86: Add functions to set and clear bits on MSRs
  x86: Allow CPUs to be set up after relocation
  x86: Add a CPU driver for baytrail
  x86: Enable multi-core init for Minnowboard MAX

 Kconfig   |   2 +-
 README|   6 -
 arch/Kconfig  |   2 +
 arch/x86/Kconfig  | 180 +-
 arch/x86/cpu/Makefile |   2 +
 arch/x86/cpu/baytrail/Makefile|   1 +
 arch/x86/cpu/baytrail/cpu.c   | 205 
 arch/x86/cpu/baytrail/valleyview.c|   1 -
 arch/x86/cpu/config.mk|   2 +-
 arch/x86/cpu/coreboot/pci.c   |  11 ++
 arch/x86/cpu/cpu.c|  71 +--
 arch/x86/cpu/interrupts.c |   5 +
 arch/x86/cpu/ivybridge/car.S  |   1 +
 arch/x86/cpu/ivybridge/cpu.c  |   5 +-
 arch/x86/cpu/ivybridge/early_me.c |  13 +-
 arch/x86/cpu/ivybridge/model_206ax.c  |   4 +-
 arch/x86/cpu/ivybridge/sdram.c|   3 +-
 arch/x86/cpu/mp_init.c| 496

 arch/x86/cpu/pci.c|  21 ++
 arch/x86/cpu/quark/quark.c|  14 +-
 arch/x86/cpu/queensbay/Makefile   |   2 +-
 arch/x86/cpu/queensbay/irq.c  | 242 +++
 arch/x86/cpu/queensbay/tnc.c  |  14 +-
 arch/x86/cpu/sipi_vector.S| 216 +
 arch/x86/dts/chromebook_link.dts  |   2 +-
 arch/x86/dts/crownbay.dts |   6 +-
 arch/x86/dts/galileo.dts   

Re: [U-Boot] fastboot boot base address behaviour

2015-05-01 Thread Maxime Ripard
On Wed, Apr 29, 2015 at 09:11:03AM -0500, Rob Herring wrote:
> On Wed, Apr 29, 2015 at 3:12 AM, Maxime Ripard
>  wrote:
> > Hi Rob,
> >
> > On Tue, Apr 28, 2015 at 05:24:59PM -0500, Rob Herring wrote:
> >> On Wed, Apr 22, 2015 at 8:04 AM, Maxime Ripard
> >>  wrote:
> >> > Hi,
> >> >
> >> > I've been trying to use fastboot (and especially the boot command) on
> >> > sunxi recently, and got it to work pretty fine (apart from PSCI, but
> >> > that's another story).
> >> >
> >> > The only thing that worries me a bit is that by default, both the
> >> > fastboot tool and mkbootimg will generate an image with the kernel
> >> > address set to 0x10008000.
> >> >
> >> > While it might work on some targets, it obviously doesn't on the
> >> > Allwinner SoCs that most of the time have the RAM mapped to 0x400,
> >> > which result in the kernel being relocated to some address that is not
> >> > in RAM, failing badly.
> >> >
> >> > I would expect U-Boot to relocate the kernel to some reasonable
> >> > address, and not try to do something dumb by actually trusting
> >> > completely the boot image.
> >> >
> >> > I guess one way to solve this would be to really treat 0x10008000 as
> >> > the default, and relocate the kernel to whatever value make sense on
> >> > the current platform (even though that needs to be defined).
> >> >
> >> > That way, "fastboot boot zImage" would actually work out of the box,
> >> > without requiring to set the optional "-b" option to set the kernel
> >> > base address to some decent value.
> >> >
> >> > The others implementation I could find seem to just ignore this field
> >> > in the image header, and always load it to the same address, which
> >> > might not really be what we're after here.
> >> >
> >> > What do you think?
> >>
> >> Android boot image is pretty broken in a variety of ways and with
> >> vendors doing their own extensions/hacks. The issues I see are:
> >>
> >> - Addresses are 32-bit
> >
> > I've not really thought about that since it still haven't had my hands
> > on a 64 bit system, but that's true.
> >
> >> - A boot image will only work on 1 platform (because of the kernel and
> >>   ramdisk addresses)
> >
> > Is that really a thing? I mean, the kernel and dtb combination will
> > be different, no matter what kind of image you make.
> 
> You're assuming the dtb is in the boot image. Maybe it is, maybe it
> isn't. Who knows.

Well, there's a good chance it will, at least appended to the zImage.

> > And there's a good chance that the ramdisk image itself will change
> > too from one platform to another, to handle the different hardware,
> > have different packages, etc.
> 
> Agreed, today everything in Android is built to a single platform much
> like the kernel used to be. There is no kernel ABI in Android. There's
> no boot interface ABI either. Maybe all that changes and then this
> will be a problem.

IIRC, in Android, you already have some support already for a
multi-devices image (at least for all that HAL stuff). So the ramdisk
might be the same, but I'm not sure anyone is really building such an
image.

> > So yeah, it will only work on a single platform (even a single board),
> > but I really wouldn't expect it to do more.
> >
> >> - Different kernel Image formats within boot.img: uImage, zImage,
> >>   Image.gz, etc.
> >
> > Can that really happen? I thought that you could only have "real"
> > bootable kernel images in boot.img (ie, not uImage)
> 
> Yes, a vendor's Android I've worked on does just that. The dtb is
> placed at 15MB offset in the boot.img with that offset hardcoded in
> u-boot. It's good thing that a kernel+ramdisk will never be bigger
> than 15MB. ;)

My current kernel is bigger than that ;)

> >> - No standard way to deal with dtb. arm32 is somewhat "standard" with
> >> appended dtb. AOSP adds appended dtb for arm64, but it is never going
> >> upstream.
> >
> > I've also tried to add DTB support in the boot.img file format. I'm
> > struggling for now with the u-boot code to handle this properly, but I
> > don't think I'm that far.
> >
> >> For the kernel address, we should probably just ignore it. For zImage,
> >> it doesn't really need to be moved from where ever the boot.img is
> >> loaded to (assuming it is within the zImage address requirements). It
> >> is going to relocate itself anyway. Putting in a correct kernel
> >> address will just cause a double copy.
> >
> > That's true if we only care about the zImage, which is what happens so
> > far. But if we also care about the embedded initramfs and the embedded
> > dtb, we will have to relocate those to avoid the kernel uncompressing
> > over these two images.
> >
> > I'm not sure what a good address for that would be (ramdisk_addr_r and
> > fdt_addr_r maybe?), but we still need to do it.
> 
> We know the kernel must be within a certain offset of start of RAM(2MB
> on arm64 and 16MB on arm32 IIRC). So the boot.img load address should
> be somewhere above 64 or 128MB offset from start of RAM.


Re: [U-Boot] [PATCH v2 2/4] mx6cuboxi: Add USB host support

2015-05-01 Thread Fabio Estevam
On Fri, May 1, 2015 at 2:10 PM, Fabio Estevam  wrote:
> On Fri, May 1, 2015 at 1:20 PM, Vagrant Cascadian  wrote:
>
>> I did revert back, and at first that seemed to fix it, but after more
>> testing, the issue appears to be intermittent, and unrelated to the USB
>> changes.
>>
>> So my earlier Tested-By stands; I think it's worth applying.
>
> Ok, thanks for the clarification.
>
> I will try to reproduce/investigate this FEC/PHY issue next week.

Looking at Solid-run's tree they have the following commit that may be
related to this PHY issue:

commit efc4835294122212052a8b8b2a23d14fa2b72177
Author: Rabeeh Khoury 
Date:   Sat Jan 25 17:57:51 2014 +0200

fecmxc_initialize_multi change from phy id to phy mask

The fecmxc_initialize_multi now accepts phy address mask instead of a
single phy address.

Modified otherboard that directly uses fecmxc_initialize_multi()

Modified also i.MX6 RXD0/RXD1 pad to pull down (they are also directly
connected to the Atheros 8035/8030 although they should be functional
only in the RMII mode - 8030).

Signed-off-by: Rabeeh Khoury 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

2015-05-01 Thread Marek Vasut
On Friday, May 01, 2015 at 04:49:37 PM, Pavel Machek wrote:
> On Fri 2015-05-01 16:24:45, Marek Vasut wrote:
> > On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote:
> > > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> > > index 201471c..f7cfbd9 100644
> > > --- a/drivers/mtd/spi/sf_probe.c
> > > +++ b/drivers/mtd/spi/sf_probe.c
> > > @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi,
> > > struct spi_flash *flash) }
> > > 
> > >   }
> > > 
> > > +#ifdef CONFIG_SPI_N25Q256A_RESET
> > 
> > Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be
> > used would have similar issue.
> 
> I'm pretty sure some Micron parts use different interface.

Which ones ?

> > It'd be nice if you
> > added diffstat into your patches as it makes things easier during
> > review.
> 
> Yes, it also makes patch harder to create (as it is tricky to
> hand-edit the patches)

git format-patch automatically inserts the diffstat for you.

> , and having diffstat for a patch that fits on a
> screen is just stupid.

It's a 2-line diffstat for this patch. If you get a 1-screen big
diffstat, then your patch is most likely wrong.

> > > +  * { "n25q256a", INFO(0x20ba19, 0, 64 * 1024,  512,
> > > +  *SECT_4K | SHUTDOWN_3BYTE) },
> > > +  *
> > > +  * Add SHUTDOWN_3BYTE here.
> > > +  */
> > > + ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> > > + if (ret) {
> > > + printf("SF: Failed issue reset command\n");
> > 
> > I thought this was just a reset-enable command. If this command
> > fails, user won't be able to tell which of these two failed, so
> > it's a bad idea to use the same error message for both.
> 
> Ok.
>   Pavel

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/4] mx6cuboxi: Add USB host support

2015-05-01 Thread Fabio Estevam
On Fri, May 1, 2015 at 1:20 PM, Vagrant Cascadian  wrote:

> I did revert back, and at first that seemed to fix it, but after more
> testing, the issue appears to be intermittent, and unrelated to the USB
> changes.
>
> So my earlier Tested-By stands; I think it's worth applying.

Ok, thanks for the clarification.

I will try to reproduce/investigate this FEC/PHY issue next week.

Regards,

Fabio Estevam
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/9] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device

2015-05-01 Thread Simon Glass
Hi Hans,

On 1 May 2015 at 04:04, Hans de Goede  wrote:
>
> Currently we copy over a number of usb_device values stored in the on stack
> struct usb_device probed in usb_scan_device() to the final driver-model 
> managed
> struct usb_device in usb_child_pre_probe() through usb_device_platdata, and
> then call usb_select_config() to fill in the rest.
>
> There are 3 problems with this approach:
>
> 1) It does not fill in enough fields before calling usb_select_config(),
> specifically it does not fill in ep0's maxpacketsize causing a div by zero
> exception in the ehci driver.
>
> 2) It unnecessarily redoes a number of usb requests making usb probing slower
>
> 3) Calling usb_select_config() a second time fails on some usb-1 devices
> plugged into usb-2 hubs, causing u-boot to not recognize these devices.
>
> This commit fixes these issues by removing the usb_select_config() call from
> usb_child_pre_probe(), and instead of copying over things field by field
> through usb_device_platdata, store a pointer to the in stack usb_device
> (which is still valid when usb_child_pre_probe() gets called) and copy
> over the entire struct.

OK well you've heard my reservations. Given that we need to fix this
and I don't have the inclination to do the full refactor now (to allow
USB drivers to send requests without a device) I think we should go
along with what you propose.

Let's make sure no one builds on it, by adding comments about the
intended direction (separating out the request from the device that
issues it).

>
> Signed-off-by: Hans de Goede 
> ---
>  drivers/usb/host/usb-uclass.c | 27 ++-
>  include/usb.h |  5 +
>  2 files changed, 7 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 714bc0e..95e371d 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -535,12 +535,7 @@ int usb_scan_device(struct udevice *parent, int port,
> }
> plat = dev_get_parent_platdata(dev);
> debug("%s: Probing '%s', plat=%p\n", __func__, dev->name, plat);
> -   plat->devnum = udev->devnum;
> -   plat->speed = udev->speed;
> -   plat->slot_id = udev->slot_id;
> -   plat->portnr = port;
> -   debug("** device '%s': stashing slot_id=%d\n", dev->name,
> - plat->slot_id);
> +   plat->udev = udev;

This is a local variable. How can it be safe to use this pointer and
then return from this function?

For now, how about putting the whole usb_device in the plat structure.
I think it is a lesser evil. You can add a large TODO to fix it up.

> priv->next_addr++;
> ret = device_probe(dev);
> if (ret) {
> @@ -599,25 +594,15 @@ int usb_get_bus(struct udevice *dev, struct udevice 
> **busp)
>
>  int usb_child_pre_probe(struct udevice *dev)
>  {
> -   struct udevice *bus;
> struct usb_device *udev = dev_get_parentdata(dev);
> struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
> -   int ret;
>
> -   ret = usb_get_bus(dev, &bus);
> -   if (ret)
> -   return ret;
> -   udev->controller_dev = bus;
> +   /*
> +* Copy over all the values set in the on stack struct usb_device in
> +* usb_scan_device() to our final struct usb_device for this dev.
> +*/
> +   *udev = *(plat->udev);
> udev->dev = dev;
> -   udev->devnum = plat->devnum;
> -   udev->slot_id = plat->slot_id;
> -   udev->portnr = plat->portnr;
> -   udev->speed = plat->speed;
> -   debug("** device '%s': getting slot_id=%d\n", dev->name, 
> plat->slot_id);
> -
> -   ret = usb_select_config(udev);
> -   if (ret)
> -   return ret;
>
> return 0;
>  }
> diff --git a/include/usb.h b/include/usb.h
> index 1984e8f..1b667ff 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -581,10 +581,7 @@ struct usb_platdata {
>   */
>  struct usb_dev_platdata {
> struct usb_device_id id;
> -   enum usb_device_speed speed;
> -   int devnum;
> -   int slot_id;
> -   int portnr; /* Hub port number, 1..n */
> +   struct usb_device *udev;

Structure comment needs updating. I think it could do with some big
hairy warnings also.

>  #ifdef CONFIG_SANDBOX
> struct usb_string *strings;
> /* NULL-terminated list of descriptor pointers */
> --
> 2.3.6
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [RFC] Booting Xen from a FIT - Additional discussion about a refactor

2015-05-01 Thread Karl Apsite
Hi Simon,
I Added the email listed in the test script to this conversation

>>>
>>> Please do add tests for the new functionality - see test/image for
>>> some existing tests. Python is preferred if the test is non-trivial.
>>
>> Absolutely, I'll be sure to include some tests in the patch(es)..
>
> Great. This area of U-Boot has had a lot of undocumented or untested
> behaviour. It has got a bit better but your boot function sounds like
> another step in the right direction.

I took a look at the current set of tests, and I'm not sure what's required to
run them.  Is there an undocumented runner in the Makefile?  I can run the
specified make target to prepare for the tests it sounds like:
`make O=sandbox sandbox_config`

However trying to run the script just generated errors about missing images.

Do you know what's needed to run the script?

Log:
$ test/image/test-imagetools.sh

Building multi-file image...
# sandbox/tools/mkimage -A x86 -O linux -T multi -n "v1.0-test" -d
sandbox/boot/vmlinuz:sandbox/boot/initrd.img:sandbox/boot/System.map linux.img
test/image/test-imagetools.sh: line 73: sandbox/tools/mkimage: No such file or
directory
done.

Extracting multi-file image contents...
# sandbox/tools/dumpimage -T multi -i linux.img -p 0 vmlinuz
test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
directory
# sandbox/tools/dumpimage -T multi -i linux.img -p 1 initrd.img
test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
directory
# sandbox/tools/dumpimage -T multi -i linux.img -p 2 System.map
test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
directory
# sandbox/tools/dumpimage -T multi -i linux.img -p 2 System.map -o test_output
test/image/test-imagetools.sh: line 73: sandbox/tools/dumpimage: No such file or
directory
done.
diff: vmlinuz: No such file or directory
Failed.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/4] mx6cuboxi: Add USB host support

2015-05-01 Thread Vagrant Cascadian
On 2015-05-01, Stefano Babic wrote:
> On 01/05/2015 07:50, Vagrant Cascadian wrote:
>
>> Net:   Phy 0 not found
>> PHY reset timed out
>> FEC
>
> ok, the PHY is not found, but I cannot understand how Fabio's changes
> dropping ASIX can be the cause. Have you reverted back U-Boot to V1 set
> of patchset to check again if it works ? It looks a completely unrelated
> cause.

I did revert back, and at first that seemed to fix it, but after more
testing, the issue appears to be intermittent, and unrelated to the USB
changes.

So my earlier Tested-By stands; I think it's worth applying.

live well,
  vagrant


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] u-boot: adjust config_cmd_default.h not to raise ton of warnings

2015-05-01 Thread Stephen Warren

On 05/01/2015 03:14 AM, Pavel Machek wrote:


If there's duplicty between config system and config_cmd_default, a
ton of warnings is raised, because one uses plain defines, and other
defines it to 1. Adjust config_cmd_default.h not to provoke the
warnings.


I believe the correct way to fix this is to fix the board config files 
that cause the defines to be duplicated.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/4] mx6cuboxi: Add USB host support

2015-05-01 Thread Fabio Estevam
On Fri, May 1, 2015 at 4:33 AM, Stefano Babic  wrote:
> Hi Vagrant,
>
> On 01/05/2015 07:50, Vagrant Cascadian wrote:
>
>> Net:   Phy 0 not found
>> PHY reset timed out
>> FEC
>
> ok, the PHY is not found, but I cannot understand how Fabio's changes
> dropping ASIX can be the cause. Have you reverted back U-Boot to V1 set
> of patchset to check again if it works ? It looks a completely unrelated
> cause.

Yes, this is something I did not understand as well.

Vagrant, could you confirm that v1 did not have this FEC issue?

Thanks
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

2015-05-01 Thread Pavel Machek
On Fri 2015-05-01 16:24:45, Marek Vasut wrote:
> On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote:
> > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> > index 201471c..f7cfbd9 100644
> > --- a/drivers/mtd/spi/sf_probe.c
> > +++ b/drivers/mtd/spi/sf_probe.c
> > @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi,
> > struct spi_flash *flash) }
> > }
> > 
> > +#ifdef CONFIG_SPI_N25Q256A_RESET
> 
> Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be
> used would have similar issue.

I'm pretty sure some Micron parts use different interface.

> It'd be nice if you
> added diffstat into your patches as it makes things easier during
> review.

Yes, it also makes patch harder to create (as it is tricky to
hand-edit the patches), and having diffstat for a patch that fits on a
screen is just stupid.

> > +* { "n25q256a", INFO(0x20ba19, 0, 64 * 1024,  512,
> > +*SECT_4K | SHUTDOWN_3BYTE) },
> > +*
> > +* Add SHUTDOWN_3BYTE here.
> > +*/
> > +   ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> > +   if (ret) {
> > +   printf("SF: Failed issue reset command\n");
> 
> I thought this was just a reset-enable command. If this command
> fails, user won't be able to tell which of these two failed, so
> it's a bad idea to use the same error message for both.

Ok.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

2015-05-01 Thread Marek Vasut
On Friday, May 01, 2015 at 11:01:09 AM, Pavel Machek wrote:
> This is needed for the SoCFPGA booting from SPI NOR flash
> e.g. (N25Q256A) as long as u-boot-spl 2013 is used (newer one is not
> available).
> 
> Signed-off-by: Stefan Roese 
> Signed-off-by: Pavel Machek 
> 
> ---
> 
> Ported to today's u-boot version.
> 
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 201471c..f7cfbd9 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi,
> struct spi_flash *flash) }
>   }
> 
> +#ifdef CONFIG_SPI_N25Q256A_RESET

Should be CONFIG_SPI_MICRON_RESET, since other parts which can also be
used would have similar issue.

This should also be documented in README I believe. It'd be nice if you
added diffstat into your patches as it makes things easier during review.

> +#define CMD_RESET_ENABLE 0x66
> +#define CMD_RESET_MEMORY 0x99
> + /*
> +  * This is needed for the SoCFPGA booting from SPI NOR flash
> +  * e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version
> +  * working on that board) sets 4-byt addressing mode.
> +  *
> +  * Additionally it may be good idea to change
> +  * this line in the Linux SPI NOR flash driver:

Please submit a patch for Linux then. But this will be extremely
crappy and unreliable solution, so you should at least make the
kernel spit some warning upon shutdown so people are aware they
are doing something terribly wrong.

> +  * { "n25q256a", INFO(0x20ba19, 0, 64 * 1024,  512,
> +  *SECT_4K | SHUTDOWN_3BYTE) },
> +  *
> +  * Add SHUTDOWN_3BYTE here.
> +  */
> + ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
> + if (ret) {
> + printf("SF: Failed issue reset command\n");

I thought this was just a reset-enable command. If this command
fails, user won't be able to tell which of these two failed, so
it's a bad idea to use the same error message for both.

> + goto err_read_id;
> + }
> +
> + ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
> + if (ret) {
> + printf("SF: Failed issue reset command\n");
> + goto err_read_id;
> + }
> +
> + printf("SF: Device software reset\n");
> +#endif
>  #ifdef CONFIG_OF_CONTROL
>   if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
>   debug("SF: FDT decode error\n");

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] dm: core: Fix regression caused by c1d6f91

2015-05-01 Thread Simon Glass
Hi Tom,

On 1 May 2015 at 07:38, Tom Rini  wrote:
>
> On Thu, Apr 30, 2015 at 07:54:21PM -0600, Simon Glass wrote:
> > Hi Joe,
> >
> > On 29 April 2015 at 10:17, Joe Hershberger  
> > wrote:
> > > Hi Simon,
> > >
> > > On Wed, Apr 29, 2015 at 8:30 AM, Simon Glass  wrote:
> > >> Hi Joe,
> > >>
> > >> On 28 April 2015 at 22:14, Joe Hershberger  
> > >> wrote:
> > >>> The change to refactor these functions created a regression.
> > >>>
> > >>> commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788
> > >>> Author: Przemyslaw Marczak 
> > >>> Date:   Wed Apr 15 13:07:17 2015 +0200
> > >>> dm: core: add internal functions for getting the device without probe
> > >>>
> > >>> With this change, the dm unit tests started failing with a probe error
> > >>> -22 in the dm_test_children test.
> > >>>
> > >>> Test: dm_test_children
> > >>> test/dm/core.c:544, dm_test_children(): 0 == ret: Expected 0, got -22
> > >>>
> > >>> This restores the original behavior which would avoid a probe on invalid
> > >>> device pointers.
> > >>>
> > >>> Signed-off-by: Joe Hershberger 
> > >>> ---
> > >>>
> > >>>  drivers/core/uclass.c | 8 ++--
> > >>>  1 file changed, 6 insertions(+), 2 deletions(-)
> > >>
> > >> Can you please check this - it is very similar to yours:
> > >>
> > >> http://patchwork.ozlabs.org/patch/464459/
> > >
> > > Yes, it looks like it solves the same problem. I don't care which way
> > > it gets solved. Looks like yours is already on the way in. Hopefully
> > > sooner than later.
> > >
> > > At what point will we make the tests be a gating factor for pulling
> > > patches, kinda like checkpatch.pl?
> >
> > Not sure, we don't even check that things build at present
>
> That's not quite true.  For every PR I do:
> ./tools/buildman/buildman -b master --force-build --step 0 -dvel
> 'blackfin|microblaze|m68k|nds32|x86|aarch64|sandbox|mips|avr32|arm|powerpc|sh4'
>
> And check that things haven't changed or at least not changed in a
> breaking way.

Oops I completely got the wrong end of the stick there. I was thinking
of a something like checkpatch that checks the build and tests before
people send patches. It's an idea that has come up before but I'm
don't think anyone has a good idea on how to do it.

Of course PRs are built! I do the same before sending them, although I
actually don't --force-build which might leave me open to problems.
Sorry Tom.

>
> > Now that the tests are running again, I'll resume checking driver
> > model things before pulling things in.
> >
> > But we could use a way to run all tests, and some unification of them
> > (e.g. all run with the same U-Boot build).
>
> Yes, if it was easy to kick off most of the tests (ie anything we can do
> with sandbox) I'd integrate that into my process.

Me too. Something I vaguely think about tidying up one day.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] dm: core: Fix regression caused by c1d6f91

2015-05-01 Thread Tom Rini
On Thu, Apr 30, 2015 at 07:54:21PM -0600, Simon Glass wrote:
> Hi Joe,
> 
> On 29 April 2015 at 10:17, Joe Hershberger  wrote:
> > Hi Simon,
> >
> > On Wed, Apr 29, 2015 at 8:30 AM, Simon Glass  wrote:
> >> Hi Joe,
> >>
> >> On 28 April 2015 at 22:14, Joe Hershberger  wrote:
> >>> The change to refactor these functions created a regression.
> >>>
> >>> commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788
> >>> Author: Przemyslaw Marczak 
> >>> Date:   Wed Apr 15 13:07:17 2015 +0200
> >>> dm: core: add internal functions for getting the device without probe
> >>>
> >>> With this change, the dm unit tests started failing with a probe error
> >>> -22 in the dm_test_children test.
> >>>
> >>> Test: dm_test_children
> >>> test/dm/core.c:544, dm_test_children(): 0 == ret: Expected 0, got -22
> >>>
> >>> This restores the original behavior which would avoid a probe on invalid
> >>> device pointers.
> >>>
> >>> Signed-off-by: Joe Hershberger 
> >>> ---
> >>>
> >>>  drivers/core/uclass.c | 8 ++--
> >>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> Can you please check this - it is very similar to yours:
> >>
> >> http://patchwork.ozlabs.org/patch/464459/
> >
> > Yes, it looks like it solves the same problem. I don't care which way
> > it gets solved. Looks like yours is already on the way in. Hopefully
> > sooner than later.
> >
> > At what point will we make the tests be a gating factor for pulling
> > patches, kinda like checkpatch.pl?
> 
> Not sure, we don't even check that things build at present

That's not quite true.  For every PR I do:
./tools/buildman/buildman -b master --force-build --step 0 -dvel
'blackfin|microblaze|m68k|nds32|x86|aarch64|sandbox|mips|avr32|arm|powerpc|sh4'

And check that things haven't changed or at least not changed in a
breaking way.

> Now that the tests are running again, I'll resume checking driver
> model things before pulling things in.
> 
> But we could use a way to run all tests, and some unification of them
> (e.g. all run with the same U-Boot build).

Yes, if it was easy to kick off most of the tests (ie anything we can do
with sandbox) I'd integrate that into my process.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] CONFIG_MTD_CONCAT issue

2015-05-01 Thread Andrew E. Mileski
I'm having an issue using CONFIG_MTD_CONCAT with a dual-die NOR flash 
part.  Assistance appreciated.


We're using the driver for greater compatibility between single and 
dual-die NOR flash parts.



=> mtdparts

device nor2 , # parts = 5
 #: namesizeoffset  mask_flags
 0: os  0x03f0  0x  0
 1: spare   0x0002  0x03f0  0
 2: redun   0x0002  0x03f2  0
 3: env 0x0002  0x03f4  0
 4: u-boot  0x000a  0x03f6  0

active partition: nor2,0 - (os) 0x03f0 @ 0x

defaults:
mtdids  : nor2=of-flash.2
mtdparts: 
mtdparts=of-flash.0:63m(os),128k(spare),128k(redun),128k(env),640k(u-boot)


=> erase nor2,0
Erase Flash Partition nor2,0, bank 2, 0x - 0x03ef
Error: start and/or end address not on sector boundary


It seems as though CONFIG_MTD_CONCAT is incompatible with
  common/cmd_flash.c : flash_fill_sect_ranges()
called from
  common/cmd_flash.c : do_flerase()
because the MTD_CONCAT offsets passed to it don't match any sectors.

Example:  Two 32 MiB flash banks are located at offsets 0xe000 and 
0xe200, which are always greater than the MTD_CONCAT offsets, 
resulting in a failure to find matching sectors.


Adding a debug message to the conditional at line 230:
  if (addr_last < info->start[sect])
  continue;
resulted in:
  ##DEBUG## addr_last < info->start[sect] (0x03ef < 0xe000)
which demonstrates the passed MTD_CONCAT offsets being compared to the 
actual bank offsets.


I'm also experiencing a secondary issue of a random hang upon erasing 
only the last sector.  The command completes successfully, but never 
returns to the command prompt.  I suspect it is a memory clobber, 
possibly related to the MTD_CONCAT offsets being used when they 
shouldn't.  Still trying to narrow this one down though.


~~
Andrew E. Mileski
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] Add spi nand support in u-boot

2015-05-01 Thread peterpandong
Hi All,

I’d like to add spi nand support in u-boot. But I don’t know where should
the spi nand drivers put in and how to define the architecture.

I find that currently spi_flash in u-boot means spi nor flash. Should I create
a struct spi_nand_flash or make spi_flash compatible?

Thanks
Peter Pan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 7/9] dm: usb: Prefix ehci interrupt-queue functions with _ehci_

2015-05-01 Thread Hans de Goede
This is a preparation patch for adding interrupt-queue support to the
ehci dm code.

Signed-off-by: Hans de Goede 
Acked-by: Simon Glass 
---
 drivers/usb/host/ehci-hcd.c | 31 +--
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 46d01d4..363ce38 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1252,9 +1252,9 @@ disable_periodic(struct ehci_ctrl *ctrl)
return 0;
 }
 
-struct int_queue *
-create_int_queue(struct usb_device *dev, unsigned long pipe, int queuesize,
-int elementsize, void *buffer, int interval)
+static struct int_queue *_ehci_create_int_queue(struct usb_device *dev,
+   unsigned long pipe, int queuesize, int elementsize,
+   void *buffer, int interval)
 {
struct ehci_ctrl *ctrl = ehci_get_ctrl(dev);
struct int_queue *result = NULL;
@@ -1410,7 +1410,8 @@ fail1:
return NULL;
 }
 
-void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
+static void *_ehci_poll_int_queue(struct usb_device *dev,
+ struct int_queue *queue)
 {
struct QH *cur = queue->current;
struct qTD *cur_td;
@@ -1445,8 +1446,8 @@ void *poll_int_queue(struct usb_device *dev, struct 
int_queue *queue)
 }
 
 /* Do not free buffers associated with QHs, they're owned by someone else */
-int
-destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
+static int _ehci_destroy_int_queue(struct usb_device *dev,
+  struct int_queue *queue)
 {
struct ehci_ctrl *ctrl = ehci_get_ctrl(dev);
int result = -1;
@@ -1547,6 +1548,24 @@ int submit_int_msg(struct usb_device *dev, unsigned long 
pipe,
 {
return _ehci_submit_int_msg(dev, pipe, buffer, length, interval);
 }
+
+struct int_queue *create_int_queue(struct usb_device *dev,
+   unsigned long pipe, int queuesize, int elementsize,
+   void *buffer, int interval)
+{
+   return _ehci_create_int_queue(dev, pipe, queuesize, elementsize,
+ buffer, interval);
+}
+
+void *poll_int_queue(struct usb_device *dev, struct int_queue *queue)
+{
+   return _ehci_poll_int_queue(dev, queue);
+}
+
+int destroy_int_queue(struct usb_device *dev, struct int_queue *queue)
+{
+   return _ehci_destroy_int_queue(dev, queue);
+}
 #endif
 
 #ifdef CONFIG_DM_USB
-- 
2.3.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 4/9] dm: usb: Fix finding of first upstream usb-2 hub in the ehci dm code

2015-05-01 Thread Hans de Goede
The ehci driver model code for finding the first upstream usb-2 hub before
this commit has a number of issues:

1) "if (!ttdev->speed != USB_SPEED_HIGH)" does not work because the '!'
   takes presedence over the '!=' this should simply be
   "if (ttdev->speed == USB_SPEED_HIGH)"
2) It makes ttdev point to the first upstream usb-2 hub, but ttdev should
   point to the last usb-1 device before the first usb-2 hub (when going
   upstream from the device), as ttdev is used to find the port of the
   first usb-2 hub to which the the last usb-1 device is connected.
3) parent_devnum however should be set to the devnum of the first usb-2
   hub, so we need to keep pointers around to both usb_device structs.

To complicate things further during enumeration usb_device.dev will point
to the parent udevice, where as during normal use it will point to
the actual udevice, we must handle both cases correctly.

This commit fixes all this making usb-1 devices attached to usb-2 hubs,
including usb-1 devices attached to usb-1 hubs attached to usb-2 hubs, work.

Signed-off-by: Hans de Goede 
---
 drivers/usb/host/ehci-hcd.c | 34 ++
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 85adbf4..9471bcb 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -303,23 +303,33 @@ static void ehci_update_endpt2_dev_n_port(struct 
usb_device *udev,
 * in the tree before that one!
 */
 #ifdef CONFIG_DM_USB
+   /*
+* When called from usb-uclass.c: usb_scan_device() udev->dev points
+* to the parent udevice, not the actual udevice belonging to the
+* udev as the device is not instantiated yet. So when searching
+* for the first usb-2 parent start with udev->dev not
+* udev->dev->parent .
+*/
struct udevice *parent;
+   struct usb_device *uparent;
+
+   ttdev = udev;
+   parent = udev->dev;
+   uparent = dev_get_parentdata(parent);
 
-   for (ttdev = udev; ; ) {
-   struct udevice *dev = ttdev->dev;
+   while (uparent->speed != USB_SPEED_HIGH) {
+   struct udevice *dev = parent;
 
-   if (dev->parent &&
-   device_get_uclass_id(dev->parent) == UCLASS_USB_HUB)
-   parent = dev->parent;
-   else
-   parent = NULL;
-   if (!parent)
+   if (device_get_uclass_id(dev->parent) != UCLASS_USB_HUB) {
+   printf("ehci: Error cannot find high speed parent of 
usb-1 device\n");
return;
-   ttdev = dev_get_parentdata(parent);
-   if (!ttdev->speed != USB_SPEED_HIGH)
-   break;
+   }
+
+   ttdev = dev_get_parentdata(dev);
+   parent = dev->parent;
+   uparent = dev_get_parentdata(parent);
}
-   parent_devnum = ttdev->devnum;
+   parent_devnum = uparent->devnum;
 #else
ttdev = udev;
while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH)
-- 
2.3.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 6/9] dm: usb: Add support for interrupt queues to the dm usb code

2015-05-01 Thread Hans de Goede
Interrupt endpoints typically are polled for a long time by the usb
controller before they return anything, so calls to submit_int_msg() can
take a long time to complete this.

To avoid this the u-boot code has the an interrupt queue mechanism / API,
add support for this to the driver-model usb code.

See the added doc comments for more details.

Signed-off-by: Hans de Goede 
Acked-by: Simon Glass 
---
 drivers/usb/host/usb-uclass.c | 36 
 include/usb.h | 48 ++-
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 9f690dc..a3c3080 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -65,6 +65,42 @@ int submit_bulk_msg(struct usb_device *udev, unsigned long 
pipe, void *buffer,
return ops->bulk(bus, udev, pipe, buffer, length);
 }
 
+struct int_queue *create_int_queue(struct usb_device *udev,
+   unsigned long pipe, int queuesize, int elementsize,
+   void *buffer, int interval)
+{
+   struct udevice *bus = udev->controller_dev;
+   struct dm_usb_ops *ops = usb_get_ops(bus);
+
+   if (!ops->create_int_queue)
+   return NULL;
+
+   return ops->create_int_queue(bus, udev, pipe, queuesize, elementsize,
+buffer, interval);
+}
+
+void *poll_int_queue(struct usb_device *udev, struct int_queue *queue)
+{
+   struct udevice *bus = udev->controller_dev;
+   struct dm_usb_ops *ops = usb_get_ops(bus);
+
+   if (!ops->poll_int_queue)
+   return NULL;
+
+   return ops->poll_int_queue(bus, udev, queue);
+}
+
+int destroy_int_queue(struct usb_device *udev, struct int_queue *queue)
+{
+   struct udevice *bus = udev->controller_dev;
+   struct dm_usb_ops *ops = usb_get_ops(bus);
+
+   if (!ops->destroy_int_queue)
+   return -ENOSYS;
+
+   return ops->destroy_int_queue(bus, udev, queue);
+}
+
 int usb_alloc_device(struct usb_device *udev)
 {
struct udevice *bus = udev->controller_dev;
diff --git a/include/usb.h b/include/usb.h
index d8bde1d..92c5bbd 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -198,7 +198,7 @@ int submit_control_msg(struct usb_device *dev, unsigned 
long pipe, void *buffer,
 int submit_int_msg(struct usb_device *dev, unsigned long pipe, void *buffer,
int transfer_len, int interval);
 
-#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST
+#if defined CONFIG_USB_EHCI || defined CONFIG_MUSB_HOST || 
defined(CONFIG_DM_USB)
 struct int_queue *create_int_queue(struct usb_device *dev, unsigned long pipe,
int queuesize, int elementsize, void *buffer, int interval);
 int destroy_int_queue(struct usb_device *dev, struct int_queue *queue);
@@ -654,6 +654,52 @@ struct dm_usb_ops {
int (*interrupt)(struct udevice *bus, struct usb_device *udev,
 unsigned long pipe, void *buffer, int length,
 int interval);
+
+   /**
+* create_int_queue() - Create and queue interrupt packets
+*
+* Create and queue @queuesize number of interrupt usb packets of
+* @elementsize bytes each. @buffer must be atleast @queuesize *
+* @elementsize bytes.
+*
+* Note some controllers only support a queuesize of 1.
+*
+* @interval: Interrupt interval
+*
+* @return A pointer to the created interrupt queue or NULL on error
+*/
+   struct int_queue *(*create_int_queue)(struct udevice *bus,
+   struct usb_device *udev, unsigned long pipe,
+   int queuesize, int elementsize, void *buffer,
+   int interval);
+
+   /**
+* poll_int_queue() - Poll an interrupt queue for completed packets
+*
+* Poll an interrupt queue for completed packets. The return value
+* points to the part of the buffer passed to create_int_queue()
+* corresponding to the completed packet.
+*
+* @queue: queue to poll
+*
+* @return Pointer to the data of the first completed packet, or
+* NULL if no packets are ready
+*/
+   void *(*poll_int_queue)(struct udevice *bus, struct usb_device *udev,
+   struct int_queue *queue);
+
+   /**
+* destroy_int_queue() - Destroy an interrupt queue
+*
+* Destroy an interrupt queue created by create_int_queue().
+*
+* @queue: queue to poll
+*
+* @return 0 if OK, -ve on error
+*/
+   int (*destroy_int_queue)(struct udevice *bus, struct usb_device *udev,
+struct int_queue *queue);
+
/**
 * alloc_device() - Allocate a new device context (XHCI)
 *
-- 
2.3.6

__

[U-Boot] [PATCH v2 9/9] sunxi: ehci: Convert to the driver-model

2015-05-01 Thread Hans de Goede
Convert sunxi-boards which use the sunxi-ehci code to the driver-model.

Signed-off-by: Hans de Goede 
Acked-by: Simon Glass 
---
 board/sunxi/Kconfig   |  3 ++
 drivers/usb/host/ehci-sunxi.c | 89 +--
 2 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 18e5561..6dcbff3 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -559,4 +559,7 @@ config DM_ETH
 config DM_SERIAL
default y
 
+config DM_USB
+   default y if !USB_MUSB_SUNXI
+
 endif
diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
index 0edb643..91443ea 100644
--- a/drivers/usb/host/ehci-sunxi.c
+++ b/drivers/usb/host/ehci-sunxi.c
@@ -14,53 +14,84 @@
 #include 
 #include 
 #include 
+#include 
 #include "ehci.h"
 
-int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr,
-   struct ehci_hcor **hcor)
+struct ehci_sunxi_priv {
+   struct ehci_ctrl ehci;
+   int ahb_gate_mask; /* Mask of ahb_gate0 clk gate bits for this hcd */
+   int phy_index; /* Index of the usb-phy attached to this hcd */
+};
+
+static int ehci_usb_probe(struct udevice *dev)
 {
struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-   int ahb_gate_offset;
+   struct usb_platdata *plat = dev_get_platdata(dev);
+   struct ehci_sunxi_priv *priv = dev_get_priv(dev);
+   struct ehci_hccr *hccr = (struct ehci_hccr *)dev_get_addr(dev);
+   struct ehci_hcor *hcor;
+
+   if (hccr == (void *)SUNXI_USB1_BASE) {
+   priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;
+   priv->phy_index = 1;
+   } else {
+   priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI1;
+   priv->phy_index = 2;
+   }
 
-   ahb_gate_offset = index ? AHB_GATE_OFFSET_USB_EHCI1 :
- AHB_GATE_OFFSET_USB_EHCI0;
-   setbits_le32(&ccm->ahb_gate0, 1 << ahb_gate_offset);
+   setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
 #ifdef CONFIG_SUNXI_GEN_SUN6I
-   setbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset);
+   setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
 #endif
 
-   sunxi_usb_phy_init(index + 1);
-   sunxi_usb_phy_power_on(index + 1);
-
-   if (index == 0)
-   *hccr = (void *)SUNXI_USB1_BASE;
-   else
-   *hccr = (void *)SUNXI_USB2_BASE;
-
-   *hcor = (struct ehci_hcor *)((uint32_t) *hccr
-   + HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
+   sunxi_usb_phy_init(priv->phy_index);
+   sunxi_usb_phy_power_on(priv->phy_index);
 
-   debug("sunxi-ehci: init hccr %x and hcor %x hc_length %d\n",
- (uint32_t)*hccr, (uint32_t)*hcor,
- (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
+   hcor = (struct ehci_hcor *)((uint32_t)hccr +
+   HC_LENGTH(ehci_readl(&hccr->cr_capbase)));
 
-   return 0;
+   return ehci_register(dev, hccr, hcor, NULL, 0, plat->init_type);
 }
 
-int ehci_hcd_stop(int index)
+static int ehci_usb_remove(struct udevice *dev)
 {
struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-   int ahb_gate_offset;
+   struct ehci_sunxi_priv *priv = dev_get_priv(dev);
+   int ret;
+
+   ret = ehci_deregister(dev);
+   if (ret)
+   return ret;
 
-   sunxi_usb_phy_power_off(index + 1);
-   sunxi_usb_phy_exit(index + 1);
+   sunxi_usb_phy_power_off(priv->phy_index);
+   sunxi_usb_phy_exit(priv->phy_index);
 
-   ahb_gate_offset = index ? AHB_GATE_OFFSET_USB_EHCI1 :
- AHB_GATE_OFFSET_USB_EHCI0;
 #ifdef CONFIG_SUNXI_GEN_SUN6I
-   clrbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset);
+   clrbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
 #endif
-   clrbits_le32(&ccm->ahb_gate0, 1 << ahb_gate_offset);
+   clrbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
 
return 0;
 }
+
+static const struct udevice_id ehci_usb_ids[] = {
+   { .compatible = "allwinner,sun4i-a10-ehci", },
+   { .compatible = "allwinner,sun5i-a13-ehci", },
+   { .compatible = "allwinner,sun6i-a31-ehci", },
+   { .compatible = "allwinner,sun7i-a20-ehci", },
+   { .compatible = "allwinner,sun8i-a23-ehci", },
+   { .compatible = "allwinner,sun9i-a80-ehci", },
+   { }
+};
+
+U_BOOT_DRIVER(usb_ehci) = {
+   .name   = "ehci_sunxi",
+   .id = UCLASS_USB,
+   .of_match = ehci_usb_ids,
+   .probe = ehci_usb_probe,
+   .remove = ehci_usb_remove,
+   .ops= &ehci_usb_ops,
+   .platdata_auto_alloc_size = sizeof(struct usb_platdata),
+   .priv_auto_alloc_size = sizeof(struct ehci_sunxi_priv),
+   .flags  = DM_FLAG_ALLOC_PRIV_DMA,
+};
-- 
2.3.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://li

[U-Boot] [PATCH v2 2/9] dm: usb: Make usb_get_bus easier to use for callers

2015-05-01 Thread Hans de Goede
Make usb_get_bus easier to use for callers, by directly returning the bus
rather then returning it via a pass-by-ref argument.

This also removes the error checking from the single current caller, as
we alreayd have an assert() for bus not being NULL in usb_get_bus().

Signed-off-by: Hans de Goede 
---
 drivers/usb/host/usb-uclass.c | 11 +++
 include/usb.h |  7 +++
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 95e371d..9f690dc 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -477,9 +477,7 @@ int usb_scan_device(struct udevice *parent, int port,
 
*devp = NULL;
memset(udev, '\0', sizeof(*udev));
-   ret = usb_get_bus(parent, &udev->controller_dev);
-   if (ret)
-   return ret;
+   udev->controller_dev = usb_get_bus(parent);
priv = dev_get_uclass_priv(udev->controller_dev);
 
/*
@@ -574,22 +572,19 @@ int usb_child_post_bind(struct udevice *dev)
return 0;
 }
 
-int usb_get_bus(struct udevice *dev, struct udevice **busp)
+struct udevice *usb_get_bus(struct udevice *dev)
 {
struct udevice *bus;
 
-   *busp = NULL;
for (bus = dev; bus && device_get_uclass_id(bus) != UCLASS_USB; )
bus = bus->parent;
if (!bus) {
/* By design this cannot happen */
assert(bus);
debug("USB HUB '%s' does not have a controller\n", dev->name);
-   return -EXDEV;
}
-   *busp = bus;
 
-   return 0;
+   return bus;
 }
 
 int usb_child_pre_probe(struct udevice *dev)
diff --git a/include/usb.h b/include/usb.h
index 1b667ff..d8bde1d 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -739,11 +739,10 @@ int usb_scan_device(struct udevice *parent, int port,
  * will be a device with uclass UCLASS_USB.
  *
  * @dev:   Device to check
- * @busp:  Returns bus, or NULL if not found
- * @return 0 if OK, -EXDEV is somehow this bus does not have a controller (this
- * indicates a critical error in the USB stack
+ * @return The bus, or NULL if not found (this indicates a critical error in
+ * the USB stack
  */
-int usb_get_bus(struct udevice *dev, struct udevice **busp);
+struct udevice *usb_get_bus(struct udevice *dev);
 
 /**
  * usb_select_config() - Set up a device ready for use
-- 
2.3.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 8/9] dm: usb: Add support for interrupt queues to the dm ehci code

2015-05-01 Thread Hans de Goede
Add support for interrupt queues to the dm ehci code.

Signed-off-by: Hans de Goede 
Acked-by: Simon Glass 
---
 drivers/usb/host/ehci-hcd.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 363ce38..0e84265 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1594,6 +1594,29 @@ static int ehci_submit_int_msg(struct udevice *dev, 
struct usb_device *udev,
return _ehci_submit_int_msg(udev, pipe, buffer, length, interval);
 }
 
+static struct int_queue *ehci_create_int_queue(struct udevice *dev,
+   struct usb_device *udev, unsigned long pipe, int queuesize,
+   int elementsize, void *buffer, int interval)
+{
+   debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev);
+   return _ehci_create_int_queue(udev, pipe, queuesize, elementsize,
+ buffer, interval);
+}
+
+static void *ehci_poll_int_queue(struct udevice *dev, struct usb_device *udev,
+struct int_queue *queue)
+{
+   debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev);
+   return _ehci_poll_int_queue(udev, queue);
+}
+
+static int ehci_destroy_int_queue(struct udevice *dev, struct usb_device *udev,
+ struct int_queue *queue)
+{
+   debug("%s: dev='%s', udev=%p\n", __func__, dev->name, udev);
+   return _ehci_destroy_int_queue(udev, queue);
+}
+
 int ehci_register(struct udevice *dev, struct ehci_hccr *hccr,
  struct ehci_hcor *hcor, const struct ehci_ops *ops,
  uint tweaks, enum usb_init_type init)
@@ -1642,6 +1665,9 @@ struct dm_usb_ops ehci_usb_ops = {
.control = ehci_submit_control_msg,
.bulk = ehci_submit_bulk_msg,
.interrupt = ehci_submit_int_msg,
+   .create_int_queue = ehci_create_int_queue,
+   .poll_int_queue = ehci_poll_int_queue,
+   .destroy_int_queue = ehci_destroy_int_queue,
 };
 
 #endif
-- 
2.3.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 5/9] dm: usb: Set desc_before_addr from ehci dm code

2015-05-01 Thread Hans de Goede
Without this usb-1 device descriptors do not get read properly.

Signed-off-by: Hans de Goede 
Acked-by: Simon Glass 
---
 drivers/usb/host/ehci-hcd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 9471bcb..46d01d4 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1579,12 +1579,15 @@ int ehci_register(struct udevice *dev, struct ehci_hccr 
*hccr,
  struct ehci_hcor *hcor, const struct ehci_ops *ops,
  uint tweaks, enum usb_init_type init)
 {
+   struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
struct ehci_ctrl *ctrl = dev_get_priv(dev);
int ret;
 
debug("%s: dev='%s', ctrl=%p, hccr=%p, hcor=%p, init=%d\n", __func__,
  dev->name, ctrl, hccr, hcor, init);
 
+   priv->desc_before_addr = true;
+
ehci_setup_ops(ctrl, ops);
ctrl->hccr = hccr;
ctrl->hcor = hcor;
-- 
2.3.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 0/9] usb: driver-model fixes and dm support sunxi-ehci.c

2015-05-01 Thread Hans de Goede
Hi Simon and Marek,

Here is v2 of my driver-model fixes and dm support for sunxi-ehci.c set.

Changes since v1:
-Improved the commit mesg for "dm: usb: Copy over usb_device values from
 usb_scan_device() to final usb_device" adding that doing select config
 twice does not work for (some) usb-1 devices plugged into a usb-2 hub.
-Added a new patch changing usb_get_bus to make it easier to use for
 callers.
-Replace "dm: usb: Use controller_dev in dm ehci code" with
 "dm: usb: Use usb_get_bus in dm ehci code"
-Replace "dm: usb: Store usb_device parent pointer in usb_device" with
 "dm: usb: Fix finding of first upstream usb-2 hub in the ehci dm code"
-"sunxi: ehci: Convert to the driver-model"
 -Added comments describing the various fields in struct ehci_sunxi_priv
 -Dropped the empty ehci_usb_ofdata_to_platdata() function

As discussed before the plan is for patches 1-8 to go upstream through the
dm tree, and then I'll take patch 9 upstream through the sunxi tree as that
depends on some pending sunxi changes.

Regards,

Hans
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 3/9] dm: usb: Use usb_get_bus in dm ehci code

2015-05-01 Thread Hans de Goede
Use usb_get_bus in dm ehci code rather then re-implementing it.

Signed-off-by: Hans de Goede 
---
 drivers/usb/host/ehci-hcd.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index bd9861d..85adbf4 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -125,14 +125,7 @@ static struct descriptor {
 static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev)
 {
 #ifdef CONFIG_DM_USB
-   struct udevice *dev;
-
-   /* Find the USB controller */
-   for (dev = udev->dev;
-device_get_uclass_id(dev) != UCLASS_USB;
-dev = dev->parent)
-   ;
-   return dev_get_priv(dev);
+   return dev_get_priv(usb_get_bus(udev->dev));
 #else
return udev->controller;
 #endif
-- 
2.3.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 1/9] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device

2015-05-01 Thread Hans de Goede
Currently we copy over a number of usb_device values stored in the on stack
struct usb_device probed in usb_scan_device() to the final driver-model managed
struct usb_device in usb_child_pre_probe() through usb_device_platdata, and
then call usb_select_config() to fill in the rest.

There are 3 problems with this approach:

1) It does not fill in enough fields before calling usb_select_config(),
specifically it does not fill in ep0's maxpacketsize causing a div by zero
exception in the ehci driver.

2) It unnecessarily redoes a number of usb requests making usb probing slower

3) Calling usb_select_config() a second time fails on some usb-1 devices
plugged into usb-2 hubs, causing u-boot to not recognize these devices.

This commit fixes these issues by removing the usb_select_config() call from
usb_child_pre_probe(), and instead of copying over things field by field
through usb_device_platdata, store a pointer to the in stack usb_device
(which is still valid when usb_child_pre_probe() gets called) and copy
over the entire struct.

Signed-off-by: Hans de Goede 
---
 drivers/usb/host/usb-uclass.c | 27 ++-
 include/usb.h |  5 +
 2 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 714bc0e..95e371d 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -535,12 +535,7 @@ int usb_scan_device(struct udevice *parent, int port,
}
plat = dev_get_parent_platdata(dev);
debug("%s: Probing '%s', plat=%p\n", __func__, dev->name, plat);
-   plat->devnum = udev->devnum;
-   plat->speed = udev->speed;
-   plat->slot_id = udev->slot_id;
-   plat->portnr = port;
-   debug("** device '%s': stashing slot_id=%d\n", dev->name,
- plat->slot_id);
+   plat->udev = udev;
priv->next_addr++;
ret = device_probe(dev);
if (ret) {
@@ -599,25 +594,15 @@ int usb_get_bus(struct udevice *dev, struct udevice 
**busp)
 
 int usb_child_pre_probe(struct udevice *dev)
 {
-   struct udevice *bus;
struct usb_device *udev = dev_get_parentdata(dev);
struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
-   int ret;
 
-   ret = usb_get_bus(dev, &bus);
-   if (ret)
-   return ret;
-   udev->controller_dev = bus;
+   /*
+* Copy over all the values set in the on stack struct usb_device in
+* usb_scan_device() to our final struct usb_device for this dev.
+*/
+   *udev = *(plat->udev);
udev->dev = dev;
-   udev->devnum = plat->devnum;
-   udev->slot_id = plat->slot_id;
-   udev->portnr = plat->portnr;
-   udev->speed = plat->speed;
-   debug("** device '%s': getting slot_id=%d\n", dev->name, plat->slot_id);
-
-   ret = usb_select_config(udev);
-   if (ret)
-   return ret;
 
return 0;
 }
diff --git a/include/usb.h b/include/usb.h
index 1984e8f..1b667ff 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -581,10 +581,7 @@ struct usb_platdata {
  */
 struct usb_dev_platdata {
struct usb_device_id id;
-   enum usb_device_speed speed;
-   int devnum;
-   int slot_id;
-   int portnr; /* Hub port number, 1..n */
+   struct usb_device *udev;
 #ifdef CONFIG_SANDBOX
struct usb_string *strings;
/* NULL-terminated list of descriptor pointers */
-- 
2.3.6

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] u-boot: adjust config_cmd_default.h not to raise ton of warnings

2015-05-01 Thread Pavel Machek

If there's duplicty between config system and config_cmd_default, a
ton of warnings is raised, because one uses plain defines, and other
defines it to 1. Adjust config_cmd_default.h not to provoke the
warnings.

Signed-off-by: Pavel Machek 

diff --git a/include/config_cmd_default.h b/include/config_cmd_default.h
index e79a13b..2f60d63 100644
--- a/include/config_cmd_default.h
+++ b/include/config_cmd_default.h
@@ -16,29 +16,29 @@
  * hardware, not fully tested, etc.).
  */
 
-#define CONFIG_CMD_BDI /* bdinfo   */
-#define CONFIG_CMD_BOOTD   /* bootd*/
-#define CONFIG_CMD_CONSOLE /* coninfo  */
-#define CONFIG_CMD_ECHO/* echo arguments   */
-#define CONFIG_CMD_EDITENV /* editenv  */
-#define CONFIG_CMD_ENV_EXISTS  /* query whether env variables exists */
-#define CONFIG_CMD_FPGA/* FPGA configuration Support   */
-#define CONFIG_CMD_IMI /* iminfo   */
-#define CONFIG_CMD_ITEST   /* Integer (and string) test*/
+#define CONFIG_CMD_BDI 1   /* bdinfo   */
+#define CONFIG_CMD_BOOTD 1 /* bootd*/
+#define CONFIG_CMD_CONSOLE 1   /* coninfo  */
+#define CONFIG_CMD_ECHO1   /* echo arguments   */
+#define CONFIG_CMD_EDITENV 1   /* editenv  */
+#define CONFIG_CMD_ENV_EXISTS 1/* query whether env variables exists */
+#define CONFIG_CMD_FPGA1   /* FPGA configuration Support   */
+#define CONFIG_CMD_IMI 1   /* iminfo   */
+#define CONFIG_CMD_ITEST 1 /* Integer (and string) test*/
 #ifndef CONFIG_SYS_NO_FLASH
-#define CONFIG_CMD_FLASH   /* flinfo, erase, protect   */
-#define CONFIG_CMD_IMLS/* List all found images*/
+#define CONFIG_CMD_FLASH 1 /* flinfo, erase, protect   */
+#define CONFIG_CMD_IMLS1   /* List all found images*/
 #endif
-#define CONFIG_CMD_LOADB   /* loadb*/
-#define CONFIG_CMD_LOADS   /* loads*/
-#define CONFIG_CMD_MEMORY  /* md mm nm mw cp cmp crc base loop */
-#define CONFIG_CMD_MISC/* Misc functions like sleep etc*/
-#define CONFIG_CMD_NET /* bootp, tftpboot, rarpboot*/
-#define CONFIG_CMD_NFS /* NFS support  */
-#define CONFIG_CMD_RUN /* run command in env variable  */
-#define CONFIG_CMD_SAVEENV /* saveenv  */
-#define CONFIG_CMD_SETGETDCR   /* DCR support on 4xx   */
-#define CONFIG_CMD_SOURCE  /* "source" command support */
-#define CONFIG_CMD_XIMG/* Load part of Multi Image */
+#define CONFIG_CMD_LOADB 1 /* loadb*/
+#define CONFIG_CMD_LOADS 1 /* loads*/
+#define CONFIG_CMD_MEMORY 1/* md mm nm mw cp cmp crc base loop */
+#define CONFIG_CMD_MISC1   /* Misc functions like sleep etc*/
+#define CONFIG_CMD_NET 1   /* bootp, tftpboot, rarpboot*/
+#define CONFIG_CMD_NFS 1   /* NFS support  */
+#define CONFIG_CMD_RUN 1   /* run command in env variable  */
+#define CONFIG_CMD_SAVEENV 1   /* saveenv  */
+#define CONFIG_CMD_SETGETDCR 1 /* DCR support on 4xx   */
+#define CONFIG_CMD_SOURCE 1/* "source" command support */
+#define CONFIG_CMD_XIMG 1  /* Load part of Multi Image */
 
 #endif /* _CONFIG_CMD_DEFAULT_H */

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] mtd: sf: Add CONFIG_SPI_N25Q256A_RESET for software-reset

2015-05-01 Thread Pavel Machek

This is needed for the SoCFPGA booting from SPI NOR flash
e.g. (N25Q256A) as long as u-boot-spl 2013 is used (newer one is not
available).

Signed-off-by: Stefan Roese 
Signed-off-by: Pavel Machek 

---

Ported to today's u-boot version.

diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
index 201471c..f7cfbd9 100644
--- a/drivers/mtd/spi/sf_probe.c
+++ b/drivers/mtd/spi/sf_probe.c
@@ -347,6 +348,36 @@ int spi_flash_probe_slave(struct spi_slave *spi, struct 
spi_flash *flash)
}
}
 
+#ifdef CONFIG_SPI_N25Q256A_RESET
+#define CMD_RESET_ENABLE 0x66
+#define CMD_RESET_MEMORY 0x99
+   /*
+* This is needed for the SoCFPGA booting from SPI NOR flash
+* e.g. (N25Q256A), as U-Boot SPL 2013-socfpga (only version
+* working on that board) sets 4-byt addressing mode.
+*
+* Additionally it may be good idea to change
+* this line in the Linux SPI NOR flash driver:
+*
+* { "n25q256a", INFO(0x20ba19, 0, 64 * 1024,  512,
+*SECT_4K | SHUTDOWN_3BYTE) },
+*
+* Add SHUTDOWN_3BYTE here.
+*/
+   ret = spi_flash_cmd(spi, CMD_RESET_ENABLE, NULL, 0);
+   if (ret) {
+   printf("SF: Failed issue reset command\n");
+   goto err_read_id;
+   }
+
+   ret = spi_flash_cmd(spi, CMD_RESET_MEMORY, NULL, 0);
+   if (ret) {
+   printf("SF: Failed issue reset command\n");
+   goto err_read_id;
+   }
+
+   printf("SF: Device software reset\n");
+#endif
 #ifdef CONFIG_OF_CONTROL
if (spi_flash_decode_fdt(gd->fdt_blob, flash)) {
debug("SF: FDT decode error\n");

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 8/8] sunxi: ehci: Convert to the driver-model

2015-05-01 Thread Hans de Goede

Hi,

On 01-05-15 06:12, Simon Glass wrote:

Hi Hans,

On 30 April 2015 at 08:35, Hans de Goede  wrote:

Convert sunxi-boards which use the sunxi-ehci code to the driver-model.

Signed-off-by: Hans de Goede 
---
  board/sunxi/Kconfig   |  3 ++
  drivers/usb/host/ehci-sunxi.c | 95 ++-
  2 files changed, 69 insertions(+), 29 deletions(-)


A few nits, but otherwise:

Acked-by: Simon Glass 



diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 18e5561..6dcbff3 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -559,4 +559,7 @@ config DM_ETH
  config DM_SERIAL
 default y

+config DM_USB
+   default y if !USB_MUSB_SUNXI
+
  endif
diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
index 0edb643..9c6703c 100644
--- a/drivers/usb/host/ehci-sunxi.c
+++ b/drivers/usb/host/ehci-sunxi.c
@@ -14,53 +14,90 @@
  #include 
  #include 
  #include 
+#include 
  #include "ehci.h"

-int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr,
-   struct ehci_hcor **hcor)
+struct ehci_sunxi_priv {
+   struct ehci_ctrl ehci;


Comment for these two: ?


+   int ahb_gate_mask;
+   int phy_index;
+};
+
+static int ehci_usb_ofdata_to_platdata(struct udevice *dev)
+{
+   return 0;


Maybe can drop this function if not used? Or do you plan to use it later?


Both are fixed in my sunxi tree now.

Regards,

Hans




+}
+
+static int ehci_usb_probe(struct udevice *dev)
  {
 struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-   int ahb_gate_offset;
+   struct usb_platdata *plat = dev_get_platdata(dev);
+   struct ehci_sunxi_priv *priv = dev_get_priv(dev);
+   struct ehci_hccr *hccr = (struct ehci_hccr *)dev_get_addr(dev);
+   struct ehci_hcor *hcor;
+
+   if (hccr == (void *)SUNXI_USB1_BASE) {
+   priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI0;
+   priv->phy_index = 1;
+   } else {
+   priv->ahb_gate_mask = 1 << AHB_GATE_OFFSET_USB_EHCI1;
+   priv->phy_index = 2;
+   }

-   ahb_gate_offset = index ? AHB_GATE_OFFSET_USB_EHCI1 :
- AHB_GATE_OFFSET_USB_EHCI0;
-   setbits_le32(&ccm->ahb_gate0, 1 << ahb_gate_offset);
+   setbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);
  #ifdef CONFIG_SUNXI_GEN_SUN6I
-   setbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset);
+   setbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
  #endif

-   sunxi_usb_phy_init(index + 1);
-   sunxi_usb_phy_power_on(index + 1);
+   sunxi_usb_phy_init(priv->phy_index);
+   sunxi_usb_phy_power_on(priv->phy_index);

-   if (index == 0)
-   *hccr = (void *)SUNXI_USB1_BASE;
-   else
-   *hccr = (void *)SUNXI_USB2_BASE;
+   hcor = (struct ehci_hcor *)((uint32_t)hccr +
+   HC_LENGTH(ehci_readl(&hccr->cr_capbase)));

-   *hcor = (struct ehci_hcor *)((uint32_t) *hccr
-   + HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
-
-   debug("sunxi-ehci: init hccr %x and hcor %x hc_length %d\n",
- (uint32_t)*hccr, (uint32_t)*hcor,
- (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
-
-   return 0;
+   return ehci_register(dev, hccr, hcor, NULL, 0, plat->init_type);
  }

-int ehci_hcd_stop(int index)
+static int ehci_usb_remove(struct udevice *dev)
  {
 struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-   int ahb_gate_offset;
+   struct ehci_sunxi_priv *priv = dev_get_priv(dev);
+   int ret;

-   sunxi_usb_phy_power_off(index + 1);
-   sunxi_usb_phy_exit(index + 1);
+   ret = ehci_deregister(dev);
+   if (ret)
+   return ret;
+
+   sunxi_usb_phy_power_off(priv->phy_index);
+   sunxi_usb_phy_exit(priv->phy_index);

-   ahb_gate_offset = index ? AHB_GATE_OFFSET_USB_EHCI1 :
- AHB_GATE_OFFSET_USB_EHCI0;
  #ifdef CONFIG_SUNXI_GEN_SUN6I
-   clrbits_le32(&ccm->ahb_reset0_cfg, 1 << ahb_gate_offset);
+   clrbits_le32(&ccm->ahb_reset0_cfg, priv->ahb_gate_mask);
  #endif
-   clrbits_le32(&ccm->ahb_gate0, 1 << ahb_gate_offset);
+   clrbits_le32(&ccm->ahb_gate0, priv->ahb_gate_mask);

 return 0;
  }
+
+static const struct udevice_id ehci_usb_ids[] = {
+   { .compatible = "allwinner,sun4i-a10-ehci", },
+   { .compatible = "allwinner,sun5i-a13-ehci", },
+   { .compatible = "allwinner,sun6i-a31-ehci", },
+   { .compatible = "allwinner,sun7i-a20-ehci", },
+   { .compatible = "allwinner,sun8i-a23-ehci", },
+   { .compatible = "allwinner,sun9i-a80-ehci", },
+   { }
+};
+
+U_BOOT_DRIVER(usb_ehci) = {
+   .name   = "ehci_sunxi",
+   .id = UCLASS_USB,
+   .of_match = ehci_usb_ids,
+   .ofdata_to_platdata = ehci_usb_ofdata_to_platdata,
+   .probe = ehci_usb_prob

Re: [U-Boot] [PATCH 3/8] dm: usb: Store usb_device parent pointer in usb_device

2015-05-01 Thread Hans de Goede

Hi,

On 01-05-15 06:11, Simon Glass wrote:

Hi Hans,

On 30 April 2015 at 08:35, Hans de Goede  wrote:

When calling into the hcd code in usb_scan_device() we do not yet have
the actual udevice for the device we are scanning, so we temporarily set
usb_device.dev to the parent.

This means that we cannot use usb_device.dev to accurately determine our
place in the usb topology when reading descriptors, and this is necessary
to correctly program the usb-2 hub tt settings when a usb-1 device is
connected to a usb-2 hub.

This commit (re)adds a direct parent usb_device pointer to struct usb_device
so that the usb hcd code can get to the usb_device parent without needing to
go through the dm.

This fixes usb-1 devices plugged into usb-2 hubs not working with the driver
model.

Signed-off-by: Hans de Goede 
---
  drivers/usb/host/ehci-hcd.c   | 24 +---
  drivers/usb/host/usb-uclass.c |  5 ++---
  include/usb.h |  2 +-
  3 files changed, 4 insertions(+), 27 deletions(-)



While I agree this is expedient, is there really no other way? How are
we going to move to driver model if we add back in the
non-driver-model pointers, etc.? We'll end up with a hybrid approach
and a real mess.

Is it possible to bug-fix the driver model code (presumably below) instead?



That is what I tried first, and failed todo. But after implementing the fix
as posted to the list I've been thinking about it some more and I think it 
should
be possible so I'll try again.

Regards,

Hans



diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 19f1e29..00c038c 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -292,7 +292,6 @@ static void ehci_update_endpt2_dev_n_port(struct usb_device 
*udev,
   struct QH *qh)
  {
 struct usb_device *ttdev;
-   int parent_devnum;

 if (udev->speed != USB_SPEED_LOW && udev->speed != USB_SPEED_FULL)
 return;
@@ -302,35 +301,14 @@ static void ehci_update_endpt2_dev_n_port(struct 
usb_device *udev,
  * the tt, so of the first upstream usb-2 hub, there may be usb-1 hubs
  * in the tree before that one!
  */
-#ifdef CONFIG_DM_USB
-   struct udevice *parent;
-
-   for (ttdev = udev; ; ) {
-   struct udevice *dev = ttdev->dev;
-
-   if (dev->parent &&
-   device_get_uclass_id(dev->parent) == UCLASS_USB_HUB)
-   parent = dev->parent;
-   else
-   parent = NULL;
-   if (!parent)
-   return;
-   ttdev = dev_get_parentdata(parent);
-   if (!ttdev->speed != USB_SPEED_HIGH)
-   break;
-   }
-   parent_devnum = ttdev->devnum;
-#else
 ttdev = udev;
 while (ttdev->parent && ttdev->parent->speed != USB_SPEED_HIGH)
 ttdev = ttdev->parent;
 if (!ttdev->parent)
 return;
-   parent_devnum = ttdev->parent->devnum;
-#endif

 qh->qh_endpt2 |= cpu_to_hc32(QH_ENDPT2_PORTNUM(ttdev->portnr) |
-QH_ENDPT2_HUBADDR(parent_devnum));
+QH_ENDPT2_HUBADDR(ttdev->parent->devnum));
  }

  static int
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 95e371d..0157bc6 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -470,7 +470,6 @@ int usb_scan_device(struct udevice *parent, int port,
 bool created = false;
 struct usb_dev_platdata *plat;
 struct usb_bus_priv *priv;
-   struct usb_device *parent_udev;
 int ret;
 ALLOC_CACHE_ALIGN_BUFFER(struct usb_device, udev, 1);
 struct usb_interface_descriptor *iface = &udev->config.if_desc[0].desc;
@@ -515,9 +514,9 @@ int usb_scan_device(struct udevice *parent, int port,
 udev->devnum = priv->next_addr + 1;
 udev->portnr = port;
 debug("Calling usb_setup_device(), portnr=%d\n", udev->portnr);
-   parent_udev = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
+   udev->parent = device_get_uclass_id(parent) == UCLASS_USB_HUB ?
 dev_get_parentdata(parent) : NULL;
-   ret = usb_setup_device(udev, priv->desc_before_addr, parent_udev, port);
+   ret = usb_setup_device(udev, priv->desc_before_addr, udev->parent, 
port);
 debug("read_descriptor for '%s': ret=%d\n", parent->name, ret);
 if (ret)
 return ret;
diff --git a/include/usb.h b/include/usb.h
index 1b667ff..7876df4 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -141,9 +141,9 @@ struct usb_device {
 int act_len;/* transfered bytes */
 int maxchild;   /* Number of ports if hub */
 int portnr; /* Port number, 1=first */
-#ifndef CONFIG_DM_USB
 /* parent hub, or NULL if thi

Re: [U-Boot] [PATCH 1/8] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device

2015-05-01 Thread Hans de Goede

Hi,

On 01-05-15 09:21, Hans de Goede wrote:

Hi,

On 01-05-15 06:11, Simon Glass wrote:

Hi Hans,

On 30 April 2015 at 08:35, Hans de Goede  wrote:

Currently we copy over a number of usb_device values stored in the on stack
struct usb_device probed in usb_scan_device() to the final driver-model managed
struct usb_device in usb_child_pre_probe() through usb_device_platdata, and
then call usb_select_config() to fill in the rest.

There are 2 problems with this approach:

1) It does not fill in enough fields before calling usb_select_config(),
specifically it does not fill in ep0's maxpacketsize causing a div by zero
exception in the ehci driver.


Didn't you have another patch that fixes that?


Yes, but as explained in the coverletter of that patch I believe that
this version is better. It would seem we disagree on that though :)


2) It unnecessarily redoes a number of usb requests making usb probing slower,
and potentially upsetting some devices.


Does it actually upset anything?


Not to my knowledge, but I'm afraid that with some devices it may.


The extra requests are in the second call to usb_select_config().


Correct.


Do you think we could put the things we want to copy in a struct, and
copy just those?


We would end up pretty much duplicating usb_device AFAICT, since we need
the descriptors, the max packet sizes per endpoint parsed from them, etc.

Anyways since you clearly dislike this patch I'll drop it for v2, replacing
it with my original fix for the ep0 maxpacket not being set issue, assuming
that doing so does not cause any regressions during my testing.


Ok, so my first test, which is hooking up a sunxi device to my dvi/usb kvm 
switch
fails immediately when I use the maxpacketsize fix instead of my fix to avoid
calling get_config twice. This is actually a pretty though test-case as the
kvm presents itself + the keyboard and mouse as a complex hub hierarchy with
both usb-2 and usb-1 hubs in there, which is what makes it a great test-case.

Now I could spend a couple of hours debugging this and maybe find a different
fix, but this really shows that there is a reason why all usb stacks do the
device probing / descriptor reading all in the exact same sequence, because
usb devices are cheap and there qa consists of plug it into $random windows
version running machine, does it work? Yes -> ship it.

So I really believe that my original fix for this is best. As for trying to
pass all the bits we need through platdata rather then passing the struct
usb_device itself. I can see value in that as part of a patch-set to get rid
of usb_device, iow as part of a larger patchset but until then it just feels
like make work to me.

So I'm going to stick with my original approach for v1 of this patch.

Regards,

Hans
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 2/8] dm: usb: Use controller_dev in dm ehci code

2015-05-01 Thread Hans de Goede

Hi,

On 01-05-15 06:11, Simon Glass wrote:

Hi Hans,

On 30 April 2015 at 08:35, Hans de Goede  wrote:

Use the controller_dev pointer in the ehci hcd code rather then going up
the tree till we find the first UCLASS_USB device.

Signed-off-by: Hans de Goede 
---
  drivers/usb/host/ehci-hcd.c | 9 +
  1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index bd9861d..19f1e29 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -125,14 +125,7 @@ static struct descriptor {
  static struct ehci_ctrl *ehci_get_ctrl(struct usb_device *udev)
  {
  #ifdef CONFIG_DM_USB
-   struct udevice *dev;
-
-   /* Find the USB controller */
-   for (dev = udev->dev;
-device_get_uclass_id(dev) != UCLASS_USB;
-dev = dev->parent)
-   ;
-   return dev_get_priv(dev);
+   return dev_get_priv(udev->controller_dev);
  #else
 return udev->controller;
  #endif


My intent was to remove udev->controller_dev, since I was hoping to
remove all pointers other than those maintained by driver model. Does
this patch actually fix a bug?


I initially wrote this because my plan was to set usb_device.dev to NULL
during the initial probing in usb_scan_device, as setting it to parent
at that point is somewhat bogus.

I dropped the setting of usb_device.dev to NULL later because I was
afraid it may cause regressions for other hcd code. But I kept this
as it seemed like a nice cleanup.

I still think this needs a bit of cleanup, if we keep doing the lookup
this way, at least it should use usb_get_bus rather then looping itself.

I'll put a patch for that in v2 of this set replacing this one.

Regards,

Hans
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/4] mx6cuboxi: Add USB host support

2015-05-01 Thread Stefano Babic
Hi Vagrant,

On 01/05/2015 07:50, Vagrant Cascadian wrote:

> Net:   Phy 0 not found
> PHY reset timed out
> FEC

ok, the PHY is not found, but I cannot understand how Fabio's changes
dropping ASIX can be the cause. Have you reverted back U-Boot to V1 set
of patchset to check again if it works ? It looks a completely unrelated
cause.

Best regards,
Stefano Babic

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 1/8] dm: usb: Copy over usb_device values from usb_scan_device() to final usb_device

2015-05-01 Thread Hans de Goede

Hi,

On 01-05-15 06:11, Simon Glass wrote:

Hi Hans,

On 30 April 2015 at 08:35, Hans de Goede  wrote:

Currently we copy over a number of usb_device values stored in the on stack
struct usb_device probed in usb_scan_device() to the final driver-model managed
struct usb_device in usb_child_pre_probe() through usb_device_platdata, and
then call usb_select_config() to fill in the rest.

There are 2 problems with this approach:

1) It does not fill in enough fields before calling usb_select_config(),
specifically it does not fill in ep0's maxpacketsize causing a div by zero
exception in the ehci driver.


Didn't you have another patch that fixes that?


Yes, but as explained in the coverletter of that patch I believe that
this version is better. It would seem we disagree on that though :)


2) It unnecessarily redoes a number of usb requests making usb probing slower,
and potentially upsetting some devices.


Does it actually upset anything?


Not to my knowledge, but I'm afraid that with some devices it may.


The extra requests are in the second call to usb_select_config().


Correct.


Do you think we could put the things we want to copy in a struct, and
copy just those?


We would end up pretty much duplicating usb_device AFAICT, since we need
the descriptors, the max packet sizes per endpoint parsed from them, etc.

Anyways since you clearly dislike this patch I'll drop it for v2, replacing
it with my original fix for the ep0 maxpacket not being set issue, assuming
that doing so does not cause any regressions during my testing.

Regards,

Hans






This commit fixes both issues by removing the usb_select_config() call from
usb_child_pre_probe(), and instead of copying over things field by field
through usb_device_platdata, store a pointer to the in stack usb_device
(which is still valid when usb_child_pre_probe() gets called) and copy
over the entire struct.

Signed-off-by: Hans de Goede 
---
  drivers/usb/host/usb-uclass.c | 27 ++-
  include/usb.h |  5 +
  2 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 714bc0e..95e371d 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -535,12 +535,7 @@ int usb_scan_device(struct udevice *parent, int port,
 }
 plat = dev_get_parent_platdata(dev);
 debug("%s: Probing '%s', plat=%p\n", __func__, dev->name, plat);
-   plat->devnum = udev->devnum;
-   plat->speed = udev->speed;
-   plat->slot_id = udev->slot_id;
-   plat->portnr = port;
-   debug("** device '%s': stashing slot_id=%d\n", dev->name,
- plat->slot_id);
+   plat->udev = udev;
 priv->next_addr++;
 ret = device_probe(dev);
 if (ret) {
@@ -599,25 +594,15 @@ int usb_get_bus(struct udevice *dev, struct udevice 
**busp)

  int usb_child_pre_probe(struct udevice *dev)
  {
-   struct udevice *bus;
 struct usb_device *udev = dev_get_parentdata(dev);
 struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
-   int ret;

-   ret = usb_get_bus(dev, &bus);
-   if (ret)
-   return ret;
-   udev->controller_dev = bus;
+   /*
+* Copy over all the values set in the on stack struct usb_device in
+* usb_scan_device() to our final struct usb_device for this dev.
+*/
+   *udev = *(plat->udev);
 udev->dev = dev;
-   udev->devnum = plat->devnum;
-   udev->slot_id = plat->slot_id;
-   udev->portnr = plat->portnr;
-   udev->speed = plat->speed;
-   debug("** device '%s': getting slot_id=%d\n", dev->name, plat->slot_id);
-
-   ret = usb_select_config(udev);
-   if (ret)
-   return ret;

 return 0;
  }
diff --git a/include/usb.h b/include/usb.h
index 1984e8f..1b667ff 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -581,10 +581,7 @@ struct usb_platdata {
   */
  struct usb_dev_platdata {
 struct usb_device_id id;
-   enum usb_device_speed speed;
-   int devnum;
-   int slot_id;
-   int portnr; /* Hub port number, 1..n */
+   struct usb_device *udev;
  #ifdef CONFIG_SANDBOX
 struct usb_string *strings;
 /* NULL-terminated list of descriptor pointers */
--
2.3.6



Regards,
Simon


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 0/8] usb: driver-model fixes and dm support sunxi-ehci.c

2015-05-01 Thread Hans de Goede

Hi,

On 01-05-15 00:04, Simon Glass wrote:

Hi Hans,

On 30 April 2015 at 13:38, Hans de Goede  wrote:

Hi,


On 30-04-15 16:48, Simon Glass wrote:


Hi Hans,

On 30 April 2015 at 08:35, Hans de Goede  wrote:


Hi Simon and Marek,

This series completes my work on converting the sunxi usb/ehci code to
the driver model. With this series everything works as it did before,
and all the issues I've been seeing when switching to the driver model
are resolved.

Please review / merge. I think it would be best to merge this through
the dm tree.



Great to see this, thanks for all the work and fixes.

I am not too keen on the passing through of struct usb_device, in fact
I went to some lengths to avoid that.

I'll read through the patches again, but I wonder if we can avoid
heading in that direction? Conceptually in driver model the device
does not exist until we find out what it is and can locate a device
driver.



Right, the problem is that AFAICT the way the driver-model works is
that the driver needs to be known at device creation time, this
probably is a result of the auto alloc mem on behalf of the device
driver feature.


It's really a design decision.


TBH I'm not sure if it is the best decision (in hindsight) we may hit
similar issues in other cases. Anyways this is how things work now,
changing this is going to be a lot of work, so lets just roll with
it.





But for usb this is problematic as we need to read descriptors and
whats not before we can find the right driver, and then we do not
want to re-read those descriptors and the driver needs access to
them too.

What you've been doing sofar is in essence passing through
struct usb_device from usb_scan_device to the pre_child_probe
method, with the difference that you've been doing it field by
field. My patch which just adds the maxpacket size to the fields
you're passing (via platdata) also works, but requires re-reading
all the descriptors which is slow(ish) and may upset some devices,
this can be avoided by stuffing more of usb_device into the
plat_data passed from sb_scan_device to the pre_child_probe, but
I decided it would be easier to just pass all of the usb_device


It might take a few extra milliseconds (I have not timed it) but given
the >1000ms it seems to take to start up USB I don't think it matters.
It would be a strange device that refuses to let you read the
descriptor twice.


I've done a lot of USB work (including redirection of USB devices
into virtual machines for qemu) and I've seen stranger devices :)

But I must admit that I cannot actually name an example, it is just that
I've a feeling that this may become a problem.



While it is of course easier to pass all of usb_device, I am not keen.
It is there not clear which bits are passed through and which are not.
If you like we could put the relevant bits in a struct.



Another slightly cleaner (IMHO) option then copying over the entire
usb_device would be to dynamically alloc usb_device in usb_scan_device,
and keep using the same usb_device all the time, so remove the on
stack copy, and do not auto-alloc the per child data for the uclass
as it is already allocated manually in usb_scan_device.


But then we have a struct usb_device before a struct udevice. That
just cements what I think is wrong with the code.


We already have a struct usb_device before a struct udevice, just
because the struct usb_device is hiding on the stack rather then
sitting in the heap does not mean it is not there.





This effectively gives us the kernel model of allocating the
generic usb_device bits before looking for a driver.


I'd be happier with that approach if we actually could split the
generic bits into some sort of 'struct urb' or similar. It represents
a destination for a request, and the request itself, not the device. I
think it is unfortunate that U-Boot conflates the device and the
request. About all we need to send a request to a device is its pipe
word and packet sizes. The device is a U-Boot concept - we can after
all fire requests all over the USB bus without a 'device'.


We do not just need the usb_device to send usb requests before we
have a udevice (which a struct urb would fix) we also need it to
store the descriptors read before we've a udevice.

Also see my reply to your review of patch 1/8.

Regards,

Hans
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot