[U-Boot] [PATCH 1/6] usb: ehci: mx6: Add support for i.MX6SL

The i.MX6SL has a different base address for the controller. This patch adapts the driver to support the different base address for this case.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br ---
drivers/usb/host/ehci-mx6.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index c0a557b..5ba1c5e 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -53,6 +53,12 @@ #define UCMD_RUN_STOP (1 << 0) /* controller run/stop */ #define UCMD_RESET (1 << 1) /* controller reset */
+#ifdef CONFIG_MX6SL +#define USB_BASE_ADDR USBO2H_USB_BASE_ADDR +#else +#define USB_BASE_ADDR USBOH3_USB_BASE_ADDR +#endif + static const unsigned phy_bases[] = { USB_PHY0_BASE_ADDR, USB_PHY1_BASE_ADDR, @@ -174,7 +180,7 @@ struct usbnc_regs {
static void usb_oc_config(int index) { - struct usbnc_regs *usbnc = (struct usbnc_regs *)(USBOH3_USB_BASE_ADDR + + struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET); void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]); u32 val; @@ -207,7 +213,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { enum usb_init_type type; - struct usb_ehci *ehci = (struct usb_ehci *)(USBOH3_USB_BASE_ADDR + + struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + (0x200 * index));
if (index > 3)

This adds the DATA[4-7] and RST pin definitions.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br ---
arch/arm/include/asm/arch-mx6/mx6sl_pins.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/arm/include/asm/arch-mx6/mx6sl_pins.h b/arch/arm/include/asm/arch-mx6/mx6sl_pins.h index 045ccc4..3ffd176 100644 --- a/arch/arm/include/asm/arch-mx6/mx6sl_pins.h +++ b/arch/arm/include/asm/arch-mx6/mx6sl_pins.h @@ -20,6 +20,11 @@ enum { MX6_PAD_SD2_DAT1__USDHC2_DAT1 = IOMUX_PAD(0x0568, 0x0260, 0, 0x0000, 0, 0), MX6_PAD_SD2_DAT2__USDHC2_DAT2 = IOMUX_PAD(0x056C, 0x0264, 0, 0x0000, 0, 0), MX6_PAD_SD2_DAT3__USDHC2_DAT3 = IOMUX_PAD(0x0570, 0x0268, 0, 0x0000, 0, 0), + MX6_PAD_SD2_DAT4__USDHC2_DAT4 = IOMUX_PAD(0X0574, 0X026C, 0, 0X0000, 0, 0), + MX6_PAD_SD2_DAT5__USDHC2_DAT5 = IOMUX_PAD(0X0578, 0X0270, 0, 0X0000, 0, 0), + MX6_PAD_SD2_DAT6__USDHC2_DAT6 = IOMUX_PAD(0X057C, 0X0274, 0, 0X0000, 0, 0), + MX6_PAD_SD2_DAT7__USDHC2_DAT7 = IOMUX_PAD(0X0580, 0X0278, 0, 0X0000, 0, 0), + MX6_PAD_SD2_RST__USDHC2_RST = IOMUX_PAD(0x0584, 0x027C, 0, 0x0000, 0, 0), MX6_PAD_UART1_RXD__UART1_RXD = IOMUX_PAD(0x05A0, 0x0298, 0, 0x07FC, 0, 0), MX6_PAD_UART1_TXD__UART1_TXD = IOMUX_PAD(0x05A4, 0x029C, 0, 0x0000, 0, 0),

This adds support to switch to 1.8V in case CMD11 succeeds.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br ---
drivers/mmc/fsl_esdhc.c | 30 +++++++++++++++++++++++------- include/fsl_esdhc.h | 2 ++ include/mmc.h | 1 + 3 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 5541613..c75b38f 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -47,19 +47,21 @@ struct fsl_esdhc { uint fevt; /* Force event register */ uint admaes; /* ADMA error status register */ uint adsaddr; /* ADMA system address register */ - char reserved2[160]; /* reserved */ + char reserved2[100]; /* reserved */ + uint vendorspec; /* Vendor Specific register */ + char reserved3[59]; /* reserved */ uint hostver; /* Host controller version register */ - char reserved3[4]; /* reserved */ - uint dmaerraddr; /* DMA error address register */ char reserved4[4]; /* reserved */ - uint dmaerrattr; /* DMA error attribute register */ + uint dmaerraddr; /* DMA error address register */ char reserved5[4]; /* reserved */ + uint dmaerrattr; /* DMA error attribute register */ + char reserved6[4]; /* reserved */ uint hostcapblt2; /* Host controller capabilities register 2 */ - char reserved6[8]; /* reserved */ + char reserved7[8]; /* reserved */ uint tcr; /* Tuning control register */ - char reserved7[28]; /* reserved */ + char reserved8[28]; /* reserved */ uint sddirctl; /* SD direction control register */ - char reserved8[712]; /* reserved */ + char reserved9[712]; /* reserved */ uint scr; /* eSDHC control register */ };
@@ -334,6 +336,15 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) goto out; }
+ /* Switch voltage to 1.8V if CMD11 succeeded */ + if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V) { + esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT); + + printf("Run CMD11 1.8V switch\n"); + /* Sleep for 5 ms - max time for card to switch to 1.8V */ + udelay(5000); + } + /* Workaround for ESDHC errata ENGcm03648 */ if (!data && (cmd->resp_type & MMC_RSP_BUSY)) { int timeout = 2500; @@ -406,6 +417,11 @@ out: while ((esdhc_read32(®s->sysctl) & SYSCTL_RSTD)) ; } + + /* If this was CMD11, then notify that power cycle is needed */ + if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V) + printf("CMD11 to switch to 1.8V mode failed." + "Card requires power cycle\n"); }
esdhc_write32(®s->irqstat, -1); diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h index 9814964..8876224 100644 --- a/include/fsl_esdhc.h +++ b/include/fsl_esdhc.h @@ -154,6 +154,8 @@ #define ESDHC_HOSTCAPBLT_DMAS 0x00400000 #define ESDHC_HOSTCAPBLT_HSS 0x00200000
+#define ESDHC_VENDORSPEC_VSELECT 0x00000002 /* Use 1.8V */ + struct fsl_esdhc_cfg { u32 esdhc_base; u32 sdhc_clk; diff --git a/include/mmc.h b/include/mmc.h index a3a100b..1b354ff 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -89,6 +89,7 @@ #define SD_CMD_SEND_RELATIVE_ADDR 3 #define SD_CMD_SWITCH_FUNC 6 #define SD_CMD_SEND_IF_COND 8 +#define SD_CMD_SWITCH_UHS18V 11
#define SD_CMD_APP_SET_BUS_WIDTH 6 #define SD_CMD_ERASE_WR_BLK_START 32

On Sun, Jun 15, 2014 at 7:46 PM, Otavio Salvador otavio@ossystems.com.br wrote:
This adds support to switch to 1.8V in case CMD11 succeeds.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/mmc/fsl_esdhc.c | 30 +++++++++++++++++++++++------- include/fsl_esdhc.h | 2 ++ include/mmc.h | 1 + 3 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 5541613..c75b38f 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -47,19 +47,21 @@ struct fsl_esdhc { uint fevt; /* Force event register */ uint admaes; /* ADMA error status register */ uint adsaddr; /* ADMA system address register */
char reserved2[160]; /* reserved */
char reserved2[100]; /* reserved */
uint vendorspec; /* Vendor Specific register */
char reserved3[59]; /* reserved */ uint hostver; /* Host controller version register */
char reserved3[4]; /* reserved */
uint dmaerraddr; /* DMA error address register */ char reserved4[4]; /* reserved */
uint dmaerrattr; /* DMA error attribute register */
uint dmaerraddr; /* DMA error address register */ char reserved5[4]; /* reserved */
uint dmaerrattr; /* DMA error attribute register */
char reserved6[4]; /* reserved */ uint hostcapblt2; /* Host controller capabilities register 2 */
char reserved6[8]; /* reserved */
char reserved7[8]; /* reserved */ uint tcr; /* Tuning control register */
char reserved7[28]; /* reserved */
char reserved8[28]; /* reserved */ uint sddirctl; /* SD direction control register */
char reserved8[712]; /* reserved */
char reserved9[712]; /* reserved */ uint scr; /* eSDHC control register */
};
@@ -334,6 +336,15 @@ esdhc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data) goto out; }
/* Switch voltage to 1.8V if CMD11 succeeded */
if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V) {
esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
printf("Run CMD11 1.8V switch\n");
/* Sleep for 5 ms - max time for card to switch to 1.8V */
udelay(5000);
}
This seems like the wrong place to put this sort of change. send_cmd is meant to deal with sending a command and dealing with any errors. Voltage changes should probably be handled by set_ios. It looks like Linux actually implements a separate op for voltage change, but the operation should be:
send CMD11 deactivate clock change voltage wait 5ms reactivate clock wait 1ms check for errors
Andy

There are board were we cannot do voltage negotiation but want to set the VSELECT bit forcely to ensure it to work at 1.8V.
This commit adds CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT flag for this use.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br ---
drivers/mmc/fsl_esdhc.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index c75b38f..b3870e2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -517,6 +517,10 @@ static int esdhc_init(struct mmc *mmc) /* Set timout to the maximum value */ esdhc_clrsetbits32(®s->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT + esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT); +#endif + return 0; }

On Monday, June 16, 2014 at 02:46:51 AM, Otavio Salvador wrote:
There are board were
Please fix your English and send a patch, thanks :)
we cannot do voltage negotiation but want to set the VSELECT bit forcely to ensure it to work at 1.8V.
This commit adds CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT flag for this use.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/mmc/fsl_esdhc.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index c75b38f..b3870e2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -517,6 +517,10 @@ static int esdhc_init(struct mmc *mmc) /* Set timout to the maximum value */ esdhc_clrsetbits32(®s->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
- esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Documentation is missing.
Best regards, Marek Vasut

On Sun, Jun 15, 2014 at 9:51 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 02:46:51 AM, Otavio Salvador wrote:
There are board were
Please fix your English and send a patch, thanks :)
I fixed the commit log, thanks.
...
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Documentation is missing.
There is no FSL ESDHC README file so that's why I didn't include it anywhere.

On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
[...]
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Documentation is missing.
There is no FSL ESDHC README file so that's why I didn't include it anywhere.
I'm at loss for words here, really...
I think you know what needs to be done (hint: write the documentation), right ?
Best regards, Marek Vasut

On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
[...]
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Documentation is missing.
There is no FSL ESDHC README file so that's why I didn't include it anywhere.
I'm at loss for words here, really...
I think you know what needs to be done (hint: write the documentation), right ?
I won't write the full documentation for it. I am sorry.

On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
[...]
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Documentation is missing.
There is no FSL ESDHC README file so that's why I didn't include it anywhere.
I'm at loss for words here, really...
I think you know what needs to be done (hint: write the documentation), right ?
I won't write the full documentation for it. I am sorry.
Undocumented configuration option is not acceptable, period.
Best regards, Marek Vasut

On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
[...]
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Documentation is missing.
There is no FSL ESDHC README file so that's why I didn't include it anywhere.
I'm at loss for words here, really...
I think you know what needs to be done (hint: write the documentation), right ?
I won't write the full documentation for it. I am sorry.
Undocumented configuration option is not acceptable, period.
Who accepted the driver in the first version, without Doc?
I am not in the position to write the full doc.

Hi Otavio,
On 06/16/14 05:24, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
[...]
> +#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT > + esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT); > +#endif
Documentation is missing.
There is no FSL ESDHC README file so that's why I didn't include it anywhere.
I'm at loss for words here, really...
I think you know what needs to be done (hint: write the documentation), right ?
I won't write the full documentation for it. I am sorry.
Undocumented configuration option is not acceptable, period.
Who accepted the driver in the first version, without Doc?
I am not in the position to write the full doc.
I think there is a misunderstanding here... I think Marek does not want to say that you need to write the full documentation for the driver, but only document the CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT configuration option (what does it do when you define it and why should one define it).

On Mon, Jun 16, 2014 at 4:03 AM, Igor Grinberg grinberg@compulab.co.il wrote:
Hi Otavio,
On 06/16/14 05:24, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
[...]
>> +#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT >> + esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT); >> +#endif > > Documentation is missing.
There is no FSL ESDHC README file so that's why I didn't include it anywhere.
I'm at loss for words here, really...
I think you know what needs to be done (hint: write the documentation), right ?
I won't write the full documentation for it. I am sorry.
Undocumented configuration option is not acceptable, period.
Who accepted the driver in the first version, without Doc?
I am not in the position to write the full doc.
I think there is a misunderstanding here... I think Marek does not want to say that you need to write the full documentation for the driver, but only document the CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT configuration option (what does it do when you define it and why should one define it).
Great but where if it does not exist?
should I make a README.fsl-esdhc and include just it?

On Monday, June 16, 2014 at 01:48:11 PM, Otavio Salvador wrote:
On Mon, Jun 16, 2014 at 4:03 AM, Igor Grinberg grinberg@compulab.co.il
wrote:
Hi Otavio,
On 06/16/14 05:24, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
[...]
>>> +#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT >>> + esdhc_setbits32(®s->vendorspec, >>> ESDHC_VENDORSPEC_VSELECT); +#endif >> >> Documentation is missing. > > There is no FSL ESDHC README file so that's why I didn't include it > anywhere.
I'm at loss for words here, really...
I think you know what needs to be done (hint: write the documentation), right ?
I won't write the full documentation for it. I am sorry.
Undocumented configuration option is not acceptable, period.
Who accepted the driver in the first version, without Doc?
I am not in the position to write the full doc.
I think there is a misunderstanding here... I think Marek does not want to say that you need to write the full documentation for the driver, but only document the CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT configuration option (what does it do when you define it and why should one define it).
Great but where if it does not exist?
should I make a README.fsl-esdhc and include just it?
I briefly looked over the options in the driver, prefixed with hash are the ones which are not driver specific:
CONFIG_ESDHC_DETECT_8_BIT_QUIRK CONFIG_ESDHC_DETECT_QUIRK CONFIG_FSL_ESDHC_PIN_MUX CONFIG_FSL_USDHC #CONFIG_MX53 #CONFIG_OF_LIBFDT CONFIG_SYS_FSL_ERRATUM_ESDHC111 CONFIG_SYS_FSL_ERRATUM_ESDHC135 CONFIG_SYS_FSL_ERRATUM_ESDHC_A001 CONFIG_SYS_FSL_ESDHC_ADDR CONFIG_SYS_FSL_ESDHC_USE_PIO CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33 #CONFIG_SYS_MMC_MAX_BLK_COUNT #CONFIG_SYS_SD_VOLTAGE #CONFIG_T4240QDS
It looks like completely ad-hoc addition of new and new config options as whoever seen fit to me, both from their names and git log of the driver. This makes the driver completely unmaintainable. I would like to see them documented before any new config options are added, so please instead of fighting the mailing list, spend 10 minutes of your time and document them.
Best regards, Marek Vasut

Hi Marek
Il 17/giu/2014 08:03 "Marek Vasut" marex@denx.de ha scritto:
On Monday, June 16, 2014 at 01:48:11 PM, Otavio Salvador wrote:
On Mon, Jun 16, 2014 at 4:03 AM, Igor Grinberg grinberg@compulab.co.il
wrote:
Hi Otavio,
On 06/16/14 05:24, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de
wrote:
> On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote: > > [...] > >>>> +#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT >>>> + esdhc_setbits32(®s->vendorspec, >>>> ESDHC_VENDORSPEC_VSELECT); +#endif >>> >>> Documentation is missing. >> >> There is no FSL ESDHC README file so that's why I didn't include
it
>> anywhere. > > I'm at loss for words here, really... > > I think you know what needs to be done (hint: write the > documentation), right ?
I won't write the full documentation for it. I am sorry.
Undocumented configuration option is not acceptable, period.
Who accepted the driver in the first version, without Doc?
I am not in the position to write the full doc.
I think there is a misunderstanding here... I think Marek does not want to say that you need to write the full documentation for the driver, but only document the CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT configuration option (what does it
do
when you define it and why should one define it).
Great but where if it does not exist?
should I make a README.fsl-esdhc and include just it?
I briefly looked over the options in the driver, prefixed with hash are
the ones
which are not driver specific:
CONFIG_ESDHC_DETECT_8_BIT_QUIRK CONFIG_ESDHC_DETECT_QUIRK CONFIG_FSL_ESDHC_PIN_MUX CONFIG_FSL_USDHC #CONFIG_MX53 #CONFIG_OF_LIBFDT CONFIG_SYS_FSL_ERRATUM_ESDHC111 CONFIG_SYS_FSL_ERRATUM_ESDHC135 CONFIG_SYS_FSL_ERRATUM_ESDHC_A001 CONFIG_SYS_FSL_ESDHC_ADDR CONFIG_SYS_FSL_ESDHC_USE_PIO CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33 #CONFIG_SYS_MMC_MAX_BLK_COUNT #CONFIG_SYS_SD_VOLTAGE #CONFIG_T4240QDS
It looks like completely ad-hoc addition of new and new config options as whoever seen fit to me, both from their names and git log of the driver.
This
makes the driver completely unmaintainable. I would like to see them
documented
Do you think that could be a better option to use flags and runtime detect instead having hundred of define?
Michael
before any new config options are added, so please instead of fighting the mailing list, spend 10 minutes of your time and document them.
Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Tuesday, June 17, 2014 at 08:06:38 AM, Michael Trimarchi wrote:
Hi Marek
Il 17/giu/2014 08:03 "Marek Vasut" marex@denx.de ha scritto:
On Monday, June 16, 2014 at 01:48:11 PM, Otavio Salvador wrote:
On Mon, Jun 16, 2014 at 4:03 AM, Igor Grinberg grinberg@compulab.co.il
wrote:
Hi Otavio,
On 06/16/14 05:24, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote: > On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de
wrote:
>> On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote: >> >> [...] >> >>>>> +#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT >>>>> + esdhc_setbits32(®s->vendorspec, >>>>> ESDHC_VENDORSPEC_VSELECT); +#endif >>>> >>>> Documentation is missing. >>> >>> There is no FSL ESDHC README file so that's why I didn't include
it
>>> anywhere. >> >> I'm at loss for words here, really... >> >> I think you know what needs to be done (hint: write the >> documentation), right ? > > I won't write the full documentation for it. I am sorry.
Undocumented configuration option is not acceptable, period.
Who accepted the driver in the first version, without Doc?
I am not in the position to write the full doc.
I think there is a misunderstanding here... I think Marek does not want to say that you need to write the full documentation for the driver, but only document the CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT configuration option (what does it
do
when you define it and why should one define it).
Great but where if it does not exist?
should I make a README.fsl-esdhc and include just it?
I briefly looked over the options in the driver, prefixed with hash are
the ones
which are not driver specific:
CONFIG_ESDHC_DETECT_8_BIT_QUIRK CONFIG_ESDHC_DETECT_QUIRK CONFIG_FSL_ESDHC_PIN_MUX CONFIG_FSL_USDHC #CONFIG_MX53 #CONFIG_OF_LIBFDT CONFIG_SYS_FSL_ERRATUM_ESDHC111 CONFIG_SYS_FSL_ERRATUM_ESDHC135 CONFIG_SYS_FSL_ERRATUM_ESDHC_A001 CONFIG_SYS_FSL_ESDHC_ADDR CONFIG_SYS_FSL_ESDHC_USE_PIO CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33 #CONFIG_SYS_MMC_MAX_BLK_COUNT #CONFIG_SYS_SD_VOLTAGE #CONFIG_T4240QDS
It looks like completely ad-hoc addition of new and new config options as whoever seen fit to me, both from their names and git log of the driver.
This
makes the driver completely unmaintainable. I would like to see them
documented
Do you think that could be a better option to use flags and runtime detect instead having hundred of define?
Eventually, when we transition to DT, this will be indeed needed anyway. But before that, we need to understand what those flags do (which we don't because previous patches neglected adding proper documentation).

Hi
On Tue, Jun 17, 2014 at 5:49 PM, Marek Vasut marex@denx.de wrote:
On Tuesday, June 17, 2014 at 08:06:38 AM, Michael Trimarchi wrote:
Hi Marek
Il 17/giu/2014 08:03 "Marek Vasut" marex@denx.de ha scritto:
On Monday, June 16, 2014 at 01:48:11 PM, Otavio Salvador wrote:
On Mon, Jun 16, 2014 at 4:03 AM, Igor Grinberg grinberg@compulab.co.il
wrote:
Hi Otavio,
On 06/16/14 05:24, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut marex@denx.de wrote: > On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote: >> On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de
wrote:
>>> On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote: >>> >>> [...] >>> >>>>>> +#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT >>>>>> + esdhc_setbits32(®s->vendorspec, >>>>>> ESDHC_VENDORSPEC_VSELECT); +#endif >>>>> >>>>> Documentation is missing. >>>> >>>> There is no FSL ESDHC README file so that's why I didn't include
it
>>>> anywhere. >>> >>> I'm at loss for words here, really... >>> >>> I think you know what needs to be done (hint: write the >>> documentation), right ? >> >> I won't write the full documentation for it. I am sorry. > > Undocumented configuration option is not acceptable, period.
Who accepted the driver in the first version, without Doc?
I am not in the position to write the full doc.
I think there is a misunderstanding here... I think Marek does not want to say that you need to write the full documentation for the driver, but only document the CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT configuration option (what does it
do
when you define it and why should one define it).
Great but where if it does not exist?
should I make a README.fsl-esdhc and include just it?
I briefly looked over the options in the driver, prefixed with hash are
the ones
which are not driver specific:
CONFIG_ESDHC_DETECT_8_BIT_QUIRK CONFIG_ESDHC_DETECT_QUIRK CONFIG_FSL_ESDHC_PIN_MUX CONFIG_FSL_USDHC #CONFIG_MX53 #CONFIG_OF_LIBFDT CONFIG_SYS_FSL_ERRATUM_ESDHC111 CONFIG_SYS_FSL_ERRATUM_ESDHC135 CONFIG_SYS_FSL_ERRATUM_ESDHC_A001 CONFIG_SYS_FSL_ESDHC_ADDR CONFIG_SYS_FSL_ESDHC_USE_PIO CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33 #CONFIG_SYS_MMC_MAX_BLK_COUNT #CONFIG_SYS_SD_VOLTAGE #CONFIG_T4240QDS
It looks like completely ad-hoc addition of new and new config options as whoever seen fit to me, both from their names and git log of the driver.
This
makes the driver completely unmaintainable. I would like to see them
documented
Do you think that could be a better option to use flags and runtime detect instead having hundred of define?
Eventually, when we transition to DT, this will be indeed needed anyway. But before that, we need to understand what those flags do (which we don't because previous patches neglected adding proper documentation).
I suggest as Stefano said to add this option/flags now in preparation of DT transition and start to document flags as you said.
Michael

Hi Otavio,
On 06/16/14 14:48, Otavio Salvador wrote:
On Mon, Jun 16, 2014 at 4:03 AM, Igor Grinberg grinberg@compulab.co.il wrote:
Hi Otavio,
On 06/16/14 05:24, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
[...]
>>> +#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT >>> + esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT); >>> +#endif >> >> Documentation is missing. > > There is no FSL ESDHC README file so that's why I didn't include it > anywhere.
I'm at loss for words here, really...
I think you know what needs to be done (hint: write the documentation), right ?
I won't write the full documentation for it. I am sorry.
Undocumented configuration option is not acceptable, period.
Who accepted the driver in the first version, without Doc?
I am not in the position to write the full doc.
I think there is a misunderstanding here... I think Marek does not want to say that you need to write the full documentation for the driver, but only document the CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT configuration option (what does it do when you define it and why should one define it).
Great but where if it does not exist?
should I make a README.fsl-esdhc and include just it?
Hmmm... May be. I would make a decision and start something, then send an RFC. Writing some words should not be hard especially for configuration options that you introduce yourself, but if you find some stuff currently hard or time consuming for some reason, a "TODO: ..." might be an acceptable compromise.

Hi Otavio,
On 16/06/2014 02:46, Otavio Salvador wrote:
There are board were we cannot do voltage negotiation but want to set the VSELECT bit forcely to ensure it to work at 1.8V.
This commit adds CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT flag for this use.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/mmc/fsl_esdhc.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index c75b38f..b3870e2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -517,6 +517,10 @@ static int esdhc_init(struct mmc *mmc) /* Set timout to the maximum value */ esdhc_clrsetbits32(®s->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
- esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Instead of adding a new compiler switch that should be documented (I have already read Marek's comments), what do you think to extend struct fsl_esdhc_cfg, putting for exmaple an "options" field with this kind of specialization ?
I see also a CONFIG_SYS_FSL_ESDHC_USE_PIO that slipped into mainline (you are both right : Otavio is not the first to add such kind of configuration switches that are still undocumented, but according to rules should be documented as Marek said).
I would suggest to get rid as much as possible with these CONFIG_ switches. If we have some specialization for this driver before calling esdhc_init (better: fsl_esdhc_initialize), they are self explaining and we do not need further documentation. What do you think ?
Best regards, Stefano

On Tue, Jun 17, 2014 at 12:11 PM, Stefano Babic sbabic@denx.de wrote:
On 16/06/2014 02:46, Otavio Salvador wrote:
There are board were we cannot do voltage negotiation but want to set the VSELECT bit forcely to ensure it to work at 1.8V.
This commit adds CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT flag for this use.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/mmc/fsl_esdhc.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index c75b38f..b3870e2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -517,6 +517,10 @@ static int esdhc_init(struct mmc *mmc) /* Set timout to the maximum value */ esdhc_clrsetbits32(®s->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Instead of adding a new compiler switch that should be documented (I have already read Marek's comments), what do you think to extend struct fsl_esdhc_cfg, putting for exmaple an "options" field with this kind of specialization ?
I will try to cook something in this direction.

Hi Stefano
On Tue, Jun 17, 2014 at 5:12 PM, Otavio Salvador otavio@ossystems.com.br wrote:
On Tue, Jun 17, 2014 at 12:11 PM, Stefano Babic sbabic@denx.de wrote:
On 16/06/2014 02:46, Otavio Salvador wrote:
There are board were we cannot do voltage negotiation but want to set the VSELECT bit forcely to ensure it to work at 1.8V.
This commit adds CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT flag for this use.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/mmc/fsl_esdhc.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index c75b38f..b3870e2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -517,6 +517,10 @@ static int esdhc_init(struct mmc *mmc) /* Set timout to the maximum value */ esdhc_clrsetbits32(®s->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
+#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+#endif
Instead of adding a new compiler switch that should be documented (I have already read Marek's comments), what do you think to extend struct fsl_esdhc_cfg, putting for exmaple an "options" field with this kind of specialization ?
I will try to cook something in this direction.
Well I already said some emails ago ;).
Michael
-- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi Michael,
On 17/06/2014 17:14, Michael Trimarchi wrote:
Instead of adding a new compiler switch that should be documented (I have already read Marek's comments), what do you think to extend struct fsl_esdhc_cfg, putting for exmaple an "options" field with this kind of specialization ?
I will try to cook something in this direction.
Well I already said some emails ago ;).
Nice I am not alone to think it in this way :-). Sorry, I have not read your answer, I was dropped from the CC list and I missed that you have already said the same.
Best regards, Stefano

When debugging initramfs failures it is quite useful to known where it is being loaded from.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br ---
common/image.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/common/image.c b/common/image.c index fa4864d..84e115f 100644 --- a/common/image.c +++ b/common/image.c @@ -1050,12 +1050,13 @@ int boot_ramdisk_high(struct lmb *lmb, ulong rd_data, ulong rd_len, #endif puts("OK\n"); } + + printf(" ramdisk load start = 0x%08lx, ramdisk load end = 0x%08lx\n", + *initrd_start, *initrd_end); } else { *initrd_start = 0; *initrd_end = 0; } - debug(" ramdisk load start = 0x%08lx, ramdisk load end = 0x%08lx\n", - *initrd_start, *initrd_end);
return 0;

Dear Otavio Salvador,
In message 1402879613-21362-5-git-send-email-otavio@ossystems.com.br you wrote:
When debugging initramfs failures it is quite useful to known where it is being loaded from.
Agreed.
printf(" ramdisk load start = 0x%08lx, ramdisk load end = 0x%08lx\n",
} else { *initrd_start = 0; *initrd_end = 0; }*initrd_start, *initrd_end);
- debug(" ramdisk load start = 0x%08lx, ramdisk load end = 0x%08lx\n",
*initrd_start, *initrd_end);
As you write yourself, this is a debug feature. There is no need to pollute normal output with such information. It just makes booting slower. So debug() is appropriate here.
And I don't see why you would NOT want to have the information in the else case.
I don't think this change is a good idea. It should not be applied.
Best regards,
Wolfgang Denk

On Mon, Jun 16, 2014 at 1:38 AM, Wolfgang Denk wd@denx.de wrote:
Dear Otavio Salvador,
In message 1402879613-21362-5-git-send-email-otavio@ossystems.com.br you wrote:
When debugging initramfs failures it is quite useful to known where it is being loaded from.
Agreed.
printf(" ramdisk load start = 0x%08lx, ramdisk load end = 0x%08lx\n",
*initrd_start, *initrd_end); } else { *initrd_start = 0; *initrd_end = 0; }
debug(" ramdisk load start = 0x%08lx, ramdisk load end = 0x%08lx\n",
*initrd_start, *initrd_end);
As you write yourself, this is a debug feature. There is no need to pollute normal output with such information. It just makes booting slower. So debug() is appropriate here.
And I don't see why you would NOT want to have the information in the else case.
I don't think this change is a good idea. It should not be applied.
Ok, no problem with me.
I will drop it for v2.

Signed-off-by: Otavio Salvador otavio@ossystems.com.br ---
board/warp/Makefile | 8 ++ board/warp/warp.c | 111 +++++++++++++++++++++++++++ boards.cfg | 1 + include/configs/warp.h | 198 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 318 insertions(+) create mode 100644 board/warp/Makefile create mode 100644 board/warp/warp.c create mode 100644 include/configs/warp.h
diff --git a/board/warp/Makefile b/board/warp/Makefile new file mode 100644 index 0000000..c555f87 --- /dev/null +++ b/board/warp/Makefile @@ -0,0 +1,8 @@ +# Copyright (C) 2014 O.S. Systems Software LTDA. +# Copyright (C) 2014 Kynetics LLC. +# Copyright (C) 2014 Revolution Robotics, Inc. +# +# SPDX-License-Identifier: GPL-2.0+ +# + +obj-y := warp.o diff --git a/board/warp/warp.c b/board/warp/warp.c new file mode 100644 index 0000000..f08707c --- /dev/null +++ b/board/warp/warp.c @@ -0,0 +1,111 @@ +/* + * Copyright (C) 2014 O.S. Systems Software LTDA. + * Copyright (C) 2014 Kynetics LLC. + * Copyright (C) 2014 Revolution Robotics, Inc. + * + * Author: Otavio Salvador otavio@ossystems.com.br + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <asm/arch/clock.h> +#include <asm/arch/iomux.h> +#include <asm/arch/imx-regs.h> +#include <asm/arch/mx6-pins.h> +#include <asm/arch/sys_proto.h> +#include <asm/gpio.h> +#include <asm/imx-common/iomux-v3.h> +#include <asm/io.h> +#include <linux/sizes.h> +#include <common.h> +#include <fsl_esdhc.h> +#include <mmc.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define UART_PAD_CTRL (PAD_CTL_PUS_100K_UP | \ + PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm | \ + PAD_CTL_SRE_FAST | PAD_CTL_HYS) + +#define USDHC_PAD_CTRL (PAD_CTL_PUS_22K_UP | \ + PAD_CTL_SPEED_LOW | PAD_CTL_DSE_80ohm | \ + PAD_CTL_SRE_FAST | PAD_CTL_HYS | \ + PAD_CTL_LVE) + +int dram_init(void) +{ + gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); + + return 0; +} + +static void setup_iomux_uart(void) +{ + static iomux_v3_cfg_t const uart1_pads[] = { + MX6_PAD_UART1_TXD__UART1_TXD | MUX_PAD_CTRL(UART_PAD_CTRL), + MX6_PAD_UART1_RXD__UART1_RXD | MUX_PAD_CTRL(UART_PAD_CTRL), + }; + + imx_iomux_v3_setup_multiple_pads(uart1_pads, ARRAY_SIZE(uart1_pads)); +} + +static struct fsl_esdhc_cfg usdhc_cfg[1] = { + {USDHC2_BASE_ADDR}, +}; + +int board_mmc_getcd(struct mmc *mmc) +{ + return 1; /* Assume boot SD always present */ +} + +int board_mmc_init(bd_t *bis) +{ + static iomux_v3_cfg_t const usdhc2_pads[] = { + MX6_PAD_SD2_CLK__USDHC2_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_CMD__USDHC2_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_RST__USDHC2_RST | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_DAT0__USDHC2_DAT0 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_DAT1__USDHC2_DAT1 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_DAT2__USDHC2_DAT2 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_DAT3__USDHC2_DAT3 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_DAT4__USDHC2_DAT4 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_DAT5__USDHC2_DAT5 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_DAT6__USDHC2_DAT6 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX6_PAD_SD2_DAT7__USDHC2_DAT7 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + }; + + imx_iomux_v3_setup_multiple_pads(usdhc2_pads, ARRAY_SIZE(usdhc2_pads)); + + usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK); + return fsl_esdhc_initialize(bis, &usdhc_cfg[0]); +} + +int board_early_init_f(void) +{ + setup_iomux_uart(); + return 0; +} + +int board_init(void) +{ + /* address of boot parameters */ + gd->bd->bi_boot_params = PHYS_SDRAM + 0x100; + + return 0; +} + +int board_late_init(void) +{ +#ifdef CONFIG_HW_WATCHDOG + hw_watchdog_init(); +#endif + + return 0; +} + +int checkboard(void) +{ + puts("Board: WaRP Board\n"); + + return 0; +} diff --git a/boards.cfg b/boards.cfg index fd4324d..435edf3 100644 --- a/boards.cfg +++ b/boards.cfg @@ -311,6 +311,7 @@ Active arm armv7 mx6 - udoo Active arm armv7 mx6 - wandboard wandboard_dl wandboard:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6dl.cfg,MX6DL,DDR_MB=1024 Fabio Estevam fabio.estevam@freescale.com Active arm armv7 mx6 - wandboard wandboard_quad wandboard:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6q2g.cfg,MX6Q,DDR_MB=2048 Fabio Estevam fabio.estevam@freescale.com Active arm armv7 mx6 - wandboard wandboard_solo wandboard:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6s.cfg,MX6S,DDR_MB=512 Fabio Estevam fabio.estevam@freescale.com +Active arm armv7 mx6 - warp warp warp:IMX_CONFIG=board/freescale/mx6slevk/imximage.cfg,MX6SL Otavio Salvador otavio@ossystems.com.br Active arm armv7 mx6 barco titanium titanium titanium:IMX_CONFIG=board/barco/titanium/imximage.cfg Stefan Roese sr@denx.de Active arm armv7 mx6 boundary nitrogen6x mx6qsabrelite nitrogen6x:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6q.cfg,MX6Q,DDR_MB=1024,SABRELITE Eric Nelson eric.nelson@boundarydevices.com Active arm armv7 mx6 boundary nitrogen6x nitrogen6dl nitrogen6x:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6dl.cfg,MX6DL,DDR_MB=1024 Eric Nelson eric.nelson@boundarydevices.com diff --git a/include/configs/warp.h b/include/configs/warp.h new file mode 100644 index 0000000..82036e4 --- /dev/null +++ b/include/configs/warp.h @@ -0,0 +1,198 @@ +/* + * Copyright (C) 2014 O.S. Systems Software LTDA. + * Copyright (C) 2014 Kynetics LLC. + * Copyright (C) 2014 Revolution Robotics, Inc. + * + * Author: Otavio Salvador otavio@ossystems.com.br + * + * Configuration settings for the WaRP Board + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __CONFIG_H +#define __CONFIG_H + +#include <asm/arch/imx-regs.h> +#include <linux/sizes.h> +#include "mx6_common.h" + +#define CONFIG_MX6 +#define CONFIG_DISPLAY_CPUINFO +#define CONFIG_DISPLAY_BOARDINFO +#define CONFIG_SYS_GENERIC_BOARD + +#define CONFIG_CMDLINE_TAG +#define CONFIG_SETUP_MEMORY_TAGS +#define CONFIG_INITRD_TAG +#define CONFIG_REVISION_TAG + +/* Size of malloc() pool */ +#define CONFIG_SYS_MALLOC_LEN (3 * SZ_1M) + +#define CONFIG_BOARD_EARLY_INIT_F +#define CONFIG_BOARD_LATE_INIT +#define CONFIG_MXC_GPIO + +#define CONFIG_MXC_UART +#define CONFIG_MXC_UART_BASE UART1_IPS_BASE_ADDR + +/* MMC Configs */ +#define CONFIG_FSL_ESDHC +#define CONFIG_FSL_USDHC +#define CONFIG_SYS_FSL_ESDHC_ADDR 0 +#define CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT + +#define CONFIG_MMC +#define CONFIG_CMD_MMC +#define CONFIG_GENERIC_MMC +#define CONFIG_CMD_FAT +#define CONFIG_DOS_PARTITION + +/* allow to overwrite serial and ethaddr */ +#define CONFIG_ENV_OVERWRITE +#define CONFIG_CONS_INDEX 1 +#define CONFIG_BAUDRATE 115200 + +/* FLASH and environment organization */ +#define CONFIG_SYS_NO_FLASH + +/* Command definition */ +#include <config_cmd_default.h> +#undef CONFIG_CMD_NET +#undef CONFIG_CMD_NFS + +#define CONFIG_BOOTDELAY 3 + +#define CONFIG_LOADADDR 0x82000000 +#define CONFIG_SYS_TEXT_BASE 0x87800000 + +/* Miscellaneous configurable options */ +#define CONFIG_SYS_LONGHELP +#define CONFIG_SYS_HUSH_PARSER +#define CONFIG_AUTO_COMPLETE +#define CONFIG_SYS_CBSIZE 256 + +/* Watchdog */ +#define CONFIG_HW_WATCHDOG +#define CONFIG_IMX_WATCHDOG +#define CONFIG_WATCHDOG_TIMEOUT_MSECS 30000 /* 30s */ + +/* Print Buffer Size */ +#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16) +#define CONFIG_SYS_MAXARGS 16 +#define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE + +#define CONFIG_SYS_MEMTEST_START 0x80000000 +#define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START + SZ_256M) + +#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR + +#define CONFIG_CMDLINE_EDITING +#define CONFIG_STACKSIZE SZ_128K + +/* Physical Memory Map */ +#define CONFIG_NR_DRAM_BANKS 1 +#define PHYS_SDRAM MMDC0_ARB_BASE_ADDR +#define PHYS_SDRAM_SIZE SZ_512M + +#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM +#define CONFIG_SYS_INIT_RAM_ADDR IRAM_BASE_ADDR +#define CONFIG_SYS_INIT_RAM_SIZE IRAM_SIZE + +#define CONFIG_SYS_INIT_SP_OFFSET \ + (CONFIG_SYS_INIT_RAM_SIZE - GENERATED_GBL_DATA_SIZE) +#define CONFIG_SYS_INIT_SP_ADDR \ + (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET) + +#define CONFIG_ENV_OFFSET (6 * SZ_64K) +#define CONFIG_ENV_SIZE SZ_8K +#define CONFIG_ENV_IS_IN_MMC +#define CONFIG_SYS_MMC_ENV_DEV 0 + +/* VDD voltage 1.65 - 1.95 */ +#define CONFIG_SYS_SD_VOLTAGE 0x00000080 + +#define CONFIG_OF_LIBFDT +#define CONFIG_CMD_BOOTZ + +#ifndef CONFIG_SYS_DCACHE_OFF +#define CONFIG_CMD_CACHE +#endif + +#define CONFIG_EXTRA_ENV_SETTINGS \ + "script=boot.scr\0" \ + "image=zImage\0" \ + "console=ttymxc0\0" \ + "fdt_high=0xffffffff\0" \ + "initrd_high=0xffffffff\0" \ + "fdt_file=imx6sl-warp.dtb\0" \ + "fdt_addr=0x88000000\0" \ + "initrd_addr=0x83800000\0" \ + "boot_fdt=try\0" \ + "ip_dyn=yes\0" \ + "mmcdev=0\0" \ + "mmcpart=1\0" \ + "mmcroot=/dev/mmcblk0p2 rootwait rw\0" \ + "mmcargs=setenv bootargs console=${console},${baudrate} " \ + "root=${mmcroot}\0" \ + "loadbootscript=" \ + "fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script};\0" \ + "bootscript=echo Running bootscript from mmc ...; " \ + "source\0" \ + "loadimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${image}\0" \ + "loadfdt=fatload mmc ${mmcdev}:${mmcpart} ${fdt_addr} ${fdt_file}\0" \ + "mmcboot=echo Booting from mmc ...; " \ + "run mmcargs; " \ + "if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \ + "if run loadfdt; then " \ + "bootz ${loadaddr} - ${fdt_addr}; " \ + "else " \ + "if test ${boot_fdt} = try; then " \ + "bootz; " \ + "else " \ + "echo WARN: Cannot load the DT; " \ + "fi; " \ + "fi; " \ + "else " \ + "bootz; " \ + "fi;\0" \ + "netargs=setenv bootargs console=${console},${baudrate} " \ + "root=/dev/nfs " \ + "ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \ + "netboot=echo Booting from net ...; " \ + "run netargs; " \ + "if test ${ip_dyn} = yes; then " \ + "setenv get_cmd dhcp; " \ + "else " \ + "setenv get_cmd tftp; " \ + "fi; " \ + "${get_cmd} ${image}; " \ + "if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \ + "if ${get_cmd} ${fdt_addr} ${fdt_file}; then " \ + "bootz ${loadaddr} - ${fdt_addr}; " \ + "else " \ + "if test ${boot_fdt} = try; then " \ + "bootz; " \ + "else " \ + "echo WARN: Cannot load the DT; " \ + "fi; " \ + "fi; " \ + "else " \ + "bootz; " \ + "fi;\0" + +#define CONFIG_BOOTCOMMAND \ + "mmc dev ${mmcdev};" \ + "mmc dev ${mmcdev}; if mmc rescan; then " \ + "if run loadbootscript; then " \ + "run bootscript; " \ + "else " \ + "if run loadimage; then " \ + "run mmcboot; " \ + "else run netboot; " \ + "fi; " \ + "fi; " \ + "else run netboot; fi" + +#endif /* __CONFIG_H */

On Monday, June 16, 2014 at 02:46:53 AM, Otavio Salvador wrote:
Such commit message, I can't stop reading ...
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
board/warp/Makefile | 8 ++ board/warp/warp.c | 111 +++++++++++++++++++++++++++ boards.cfg | 1 + include/configs/warp.h | 198
[...]
btw. this thing contains like gazilion-th copy of the same env ... that's starting to annoy me.
Best regards, Marek Vasut

Hi Otavio,
On Sun, Jun 15, 2014 at 9:46 PM, Otavio Salvador otavio@ossystems.com.br wrote:
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
board/warp/Makefile | 8 ++ board/warp/warp.c | 111 +++++++++++++++++++++++++++ boards.cfg | 1 + include/configs/warp.h | 198 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 318 insertions(+) create mode 100644 board/warp/Makefile create mode 100644 board/warp/warp.c create mode 100644 include/configs/warp.h
Do you have to submit v2 of this series?

On Mon, Aug 18, 2014 at 2:56 PM, Fabio Estevam festevam@gmail.com wrote:
Hi Otavio,
On Sun, Jun 15, 2014 at 9:46 PM, Otavio Salvador otavio@ossystems.com.br wrote:
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
board/warp/Makefile | 8 ++ board/warp/warp.c | 111 +++++++++++++++++++++++++++ boards.cfg | 1 + include/configs/warp.h | 198 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 318 insertions(+) create mode 100644 board/warp/Makefile create mode 100644 board/warp/warp.c create mode 100644 include/configs/warp.h
Do you have to submit v2 of this series?
I meant "Do you have plans to submit v2 of this series?"

On Mon, Aug 18, 2014 at 2:57 PM, Fabio Estevam festevam@gmail.com wrote:
On Mon, Aug 18, 2014 at 2:56 PM, Fabio Estevam festevam@gmail.com wrote:
Hi Otavio,
On Sun, Jun 15, 2014 at 9:46 PM, Otavio Salvador otavio@ossystems.com.br wrote:
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
board/warp/Makefile | 8 ++ board/warp/warp.c | 111 +++++++++++++++++++++++++++ boards.cfg | 1 + include/configs/warp.h | 198 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 318 insertions(+) create mode 100644 board/warp/Makefile create mode 100644 board/warp/warp.c create mode 100644 include/configs/warp.h
Do you have to submit v2 of this series?
I meant "Do you have plans to submit v2 of this series?"
Yes; working on it.

On Monday, June 16, 2014 at 02:46:48 AM, Otavio Salvador wrote:
The i.MX6SL has a different base address for the controller. This patch adapts the driver to support the different base address for this case.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/usb/host/ehci-mx6.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index c0a557b..5ba1c5e 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -53,6 +53,12 @@ #define UCMD_RUN_STOP (1 << 0) /* controller run/stop */ #define UCMD_RESET (1 << 1) /* controller reset */
+#ifdef CONFIG_MX6SL +#define USB_BASE_ADDR USBO2H_USB_BASE_ADDR +#else +#define USB_BASE_ADDR USBOH3_USB_BASE_ADDR +#endif
Can the CPU type be detected at runtime so we don't need this ugly ifdef ?
Best regards, Marek Vasut

On Sun, Jun 15, 2014 at 9:49 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 02:46:48 AM, Otavio Salvador wrote:
The i.MX6SL has a different base address for the controller. This patch adapts the driver to support the different base address for this case.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
...
Can the CPU type be detected at runtime so we don't need this ugly ifdef ?
The SoloLite is not pin-compatible so it is unable to be used in same binary.

On Monday, June 16, 2014 at 03:11:03 AM, Otavio Salvador wrote:
On Sun, Jun 15, 2014 at 9:49 PM, Marek Vasut marex@denx.de wrote:
On Monday, June 16, 2014 at 02:46:48 AM, Otavio Salvador wrote:
The i.MX6SL has a different base address for the controller. This patch adapts the driver to support the different base address for this case.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
...
Can the CPU type be detected at runtime so we don't need this ugly ifdef ?
The SoloLite is not pin-compatible so it is unable to be used in same binary.
You didn't answer my question ;)
Best regards, Marek Vasut

Hi Otavio,
On 06/16/14 03:46, Otavio Salvador wrote:
The i.MX6SL has a different base address for the controller. This patch adapts the driver to support the different base address for this case.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/usb/host/ehci-mx6.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index c0a557b..5ba1c5e 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -53,6 +53,12 @@ #define UCMD_RUN_STOP (1 << 0) /* controller run/stop */ #define UCMD_RESET (1 << 1) /* controller reset */
+#ifdef CONFIG_MX6SL +#define USB_BASE_ADDR USBO2H_USB_BASE_ADDR +#else +#define USB_BASE_ADDR USBOH3_USB_BASE_ADDR +#endif
Hmmm... Can't this be done dynamically? I mean... you can check the SoC type in runtime, right? And then substitute the correct address in a variable or so... I would really prefer such kind of things done in runtime.
static const unsigned phy_bases[] = { USB_PHY0_BASE_ADDR, USB_PHY1_BASE_ADDR, @@ -174,7 +180,7 @@ struct usbnc_regs {
static void usb_oc_config(int index) {
- struct usbnc_regs *usbnc = (struct usbnc_regs *)(USBOH3_USB_BASE_ADDR +
- struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET); void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]); u32 val;
@@ -207,7 +213,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { enum usb_init_type type;
- struct usb_ehci *ehci = (struct usb_ehci *)(USBOH3_USB_BASE_ADDR +
struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + (0x200 * index));
if (index > 3)

On Mon, Jun 16, 2014 at 4:05 AM, Igor Grinberg grinberg@compulab.co.il wrote:
Hi Otavio,
On 06/16/14 03:46, Otavio Salvador wrote:
The i.MX6SL has a different base address for the controller. This patch adapts the driver to support the different base address for this case.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/usb/host/ehci-mx6.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index c0a557b..5ba1c5e 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -53,6 +53,12 @@ #define UCMD_RUN_STOP (1 << 0) /* controller run/stop */ #define UCMD_RESET (1 << 1) /* controller reset */
+#ifdef CONFIG_MX6SL +#define USB_BASE_ADDR USBO2H_USB_BASE_ADDR +#else +#define USB_BASE_ADDR USBOH3_USB_BASE_ADDR +#endif
Hmmm... Can't this be done dynamically? I mean... you can check the SoC type in runtime, right? And then substitute the correct address in a variable or so... I would really prefer such kind of things done in runtime.
I will check about changing it for v2.

Hi Otavio,
On 16/06/2014 02:46, Otavio Salvador wrote:
The i.MX6SL has a different base address for the controller. This patch adapts the driver to support the different base address for this case.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/usb/host/ehci-mx6.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index c0a557b..5ba1c5e 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -53,6 +53,12 @@ #define UCMD_RUN_STOP (1 << 0) /* controller run/stop */ #define UCMD_RESET (1 << 1) /* controller reset */
+#ifdef CONFIG_MX6SL +#define USB_BASE_ADDR USBO2H_USB_BASE_ADDR +#else +#define USB_BASE_ADDR USBOH3_USB_BASE_ADDR +#endif
What about using the is_cpu_type() function, recently added together with SPL support ? I think we have reached, thank to Tim's patches, the goal to have a single image for different Freescale's mx6 variations. It is pity to go back and decide at compile time which SOC is running.
static const unsigned phy_bases[] = { USB_PHY0_BASE_ADDR, USB_PHY1_BASE_ADDR, @@ -174,7 +180,7 @@ struct usbnc_regs {
static void usb_oc_config(int index) {
- struct usbnc_regs *usbnc = (struct usbnc_regs *)(USBOH3_USB_BASE_ADDR +
- struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET); void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl[index]); u32 val;
@@ -207,7 +213,7 @@ int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr, struct ehci_hcor **hcor) { enum usb_init_type type;
- struct usb_ehci *ehci = (struct usb_ehci *)(USBOH3_USB_BASE_ADDR +
struct usb_ehci *ehci = (struct usb_ehci *)(USB_BASE_ADDR + (0x200 * index));
if (index > 3)
Best regards, Stefano Babic

On Tue, Jun 17, 2014 at 11:56 AM, Stefano Babic sbabic@denx.de wrote:
Hi Otavio,
On 16/06/2014 02:46, Otavio Salvador wrote:
The i.MX6SL has a different base address for the controller. This patch adapts the driver to support the different base address for this case.
Signed-off-by: Otavio Salvador otavio@ossystems.com.br
drivers/usb/host/ehci-mx6.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index c0a557b..5ba1c5e 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -53,6 +53,12 @@ #define UCMD_RUN_STOP (1 << 0) /* controller run/stop */ #define UCMD_RESET (1 << 1) /* controller reset */
+#ifdef CONFIG_MX6SL +#define USB_BASE_ADDR USBO2H_USB_BASE_ADDR +#else +#define USB_BASE_ADDR USBOH3_USB_BASE_ADDR +#endif
What about using the is_cpu_type() function, recently added together with SPL support ? I think we have reached, thank to Tim's patches, the goal to have a single image for different Freescale's mx6 variations. It is pity to go back and decide at compile time which SOC is running.
Yes; I will do it for v2.
participants (8)
-
Andy Fleming
-
Fabio Estevam
-
Igor Grinberg
-
Marek Vasut
-
Michael Trimarchi
-
Otavio Salvador
-
Stefano Babic
-
Wolfgang Denk