mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Christian Hemp <c.hemp@phytec.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 1/2] pcaam1: Add support for the Phytec phyCARD-A-M1 (PCA-A-M1)
Date: Fri, 2 Mar 2012 09:28:34 +0100	[thread overview]
Message-ID: <20120302082834.GL3852@pengutronix.de> (raw)
In-Reply-To: <1330609813-28879-1-git-send-email-c.hemp@phytec.de>

Hi Christian,

On Thu, Mar 01, 2012 at 02:50:12PM +0100, Christian Hemp wrote:
> Add support for the phyCARD-A-M1 (PCA-A-M1).
> 
> +
> +static struct fb_videomode pcaam1_fb_mode[] = {
> +	{
> +		.name		= "Primeview-PD050VL1",
> +		.refresh	= 60,
> +		.xres		= 640,
> +		.yres		= 480,
> +		.pixclock	= 39900, /* in ps (25 MHz) */
> +		.hsync_len	= 32,
> +		.left_margin	= 112,
> +		.right_margin	= 36,
> +		.vsync_len	= 2,
> +		.upper_margin	= 33,
> +		.lower_margin	= 33,
> +		.sync		= FB_SYNC_OE_ACT_HIGH,
> +		.vmode		= FB_VMODE_NONINTERLACED,
> +		.flag		= 0,
> +	}, {
> +		.name		= "Primeview-PD035VL1",
> +		.refresh	= 60,
> +		.xres		= 640,
> +		.yres		= 480,
> +		.pixclock	= 40000, /* in ps (25 MHz) */
> +		.hsync_len	= 32,
> +		.left_margin	= 112,
> +		.right_margin	= 36,
> +		.vsync_len	= 2,
> +		.upper_margin	= 33,
> +		.lower_margin	= 33,
> +		.sync		= FB_SYNC_OE_ACT_HIGH,
> +		.vmode		= FB_VMODE_NONINTERLACED,
> +		.flag		= 0,
> +	}, {
> +		.name		= "Primeview-PD104SLF",
> +		.refresh	= 60,
> +		.xres		= 800,
> +		.yres		= 600,
> +		.pixclock	= 25000, /* in ps (40,0 MHz) */
> +		.hsync_len	= 128,
> +		.left_margin	= 42,
> +		.right_margin	= 42,
> +		.vsync_len	= 4,
> +		.upper_margin	= 23,
> +		.lower_margin	= 1,
> +		.sync		= FB_SYNC_OE_ACT_HIGH,
> +		.vmode		= FB_VMODE_NONINTERLACED,
> +		.flag		= 0,
> +	}, {
> +		.name		= "Primeview-PM070WL4",
> +		.refresh	= 60,
> +		.xres		= 800,
> +		.yres		= 480,
> +		.pixclock       = 50505, /* in ps (19,8 MHz) */
> +		.hsync_len	= 128,
> +		.left_margin	= 42,
> +		.right_margin	= 42,
> +		.vsync_len	= 2,
> +		.upper_margin	= 33,
> +		.lower_margin	= 33,
> +		.sync		= FB_SYNC_OE_ACT_HIGH,
> +		.vmode		= FB_VMODE_NONINTERLACED,
> +		.flag		= 0,
> +	}

This duplicates the entries in
./arch/arm/boards/phycard-i.MX27/pca100.c, though with quite different
timings. It would be nice if we could merge them as these seem to be
your standard displays (which will also be used on i.MX6, right?)

(We would have to replace the embedded struct fb_videomode in struct
imx_fb_videomode with a pointer to struct fb_videomode to do this)

I won't insist on this change now, but I might have a different opinion
when the i.MX6 patches arrive..

> +
> +	reg = readl(IMX_CCM_BASE + CCM_RCSR);
> +	/* some fuses provide us vital information about connected hardware */
> +	if (reg & 0x20000000)
> +		nand_info.width = 2;    /* 16 bit */
> +	else
> +		nand_info.width = 1;    /* 8 bit */
> +
> +	imx35_add_fec(&fec_info);
> +	imx35_add_nand(&nand_info);
> +
> +	/* reset mode: external boot */
> +	if ((reg & 0xc00) == 0x800 && ((reg >> 25) & 0x3) == 0x01) {
> +		devfs_add_partition("nand0", 0x00000, 0x40000, PARTITION_FIXED,
> +				    "self_raw");
> +		dev_add_bb_dev("self_raw", "self0");
> +		devfs_add_partition("nand0", 0x40000, 0x20000, PARTITION_FIXED,
> +				    "env_raw");
> +		dev_add_bb_dev("env_raw", "env0");
> +	}

You end up without a writable environment when you boot for example from
SD card. Is this intended?

> +
> +	imx35_add_fb(&ipu_fb_data);
> +	imx35_add_i2c2(&pcaam1_i2c_3_data);
> +	imx35_add_mmc0(NULL);
> +
> +	armlinux_set_bootparams((void *)0x80000100);
> +	armlinux_set_architecture(MACH_TYPE_PCAAM1);
> +
> +	return 0;
> +}
> +
> +device_initcall(pcaam1_devices_init);
> +
> +static int pcaam1_console_init(void)
> +{
> +	imx35_add_uart0();
> +
> +	return 0;
> +}
> +
> +console_initcall(pcaam1_console_init);
> +
> +#define M3IFCTL_IPU1    0x00000040
> +
> +static int pcaam1_core_setup(void)

Please move this function to arch/arm/mach-imx/imx35.c and rename it
to imx35_init_lowlevel. It is duplicated quite often now and always
the same copy-and-paste.

> +
> +	/*
> +	 * AIPS setup - Only setup MPROTx registers.
> +	 * The PACR default values are good.
> +	 */
> +	/*
> +	 * Set all MPROTx to be non-bufferable, trusted for R/W,
> +	 * not forced to user-mode.
> +	 */
> +	writel(0x77777777, IMX_AIPS1_BASE);
> +	writel(0x77777777, IMX_AIPS1_BASE + 0x4);
> +	writel(0x77777777, IMX_AIPS2_BASE);
> +	writel(0x77777777, IMX_AIPS2_BASE + 0x4);
> +
> +	/*
> +	 * Clear the on and off peripheral modules Supervisor Protect bit
> +	 * for SDMA to access them. Did not change the AIPS control registers
> +	 * (offset 0x20) access type
> +	 */
> +	writel(0x0, IMX_AIPS1_BASE + 0x40);
> +	writel(0x0, IMX_AIPS1_BASE + 0x44);
> +	writel(0x0, IMX_AIPS1_BASE + 0x48);
> +	writel(0x0, IMX_AIPS1_BASE + 0x4C);
> +	tmp = readl(IMX_AIPS1_BASE + 0x50);
> +	tmp &= 0x00FFFFFF;
> +	writel(tmp, IMX_AIPS1_BASE + 0x50);
> +
> +	writel(0x0, IMX_AIPS2_BASE + 0x40);
> +	writel(0x0, IMX_AIPS2_BASE + 0x44);
> +	writel(0x0, IMX_AIPS2_BASE + 0x48);
> +	writel(0x0, IMX_AIPS2_BASE + 0x4C);
> +	tmp = readl(IMX_AIPS2_BASE + 0x50);
> +	tmp &= 0x00FFFFFF;
> +	writel(tmp, IMX_AIPS2_BASE + 0x50);
> +
> +	/* MAX (Multi-Layer AHB Crossbar Switch) setup */
> +
> +	/* MPR - priority is M4 > M2 > M3 > M5 > M0 > M1 */
> +#define MAX_PARAM1 0x00302154
> +	writel(MAX_PARAM1, IMX_MAX_BASE + 0x0);   /* for S0 */
> +	writel(MAX_PARAM1, IMX_MAX_BASE + 0x100); /* for S1 */
> +	writel(MAX_PARAM1, IMX_MAX_BASE + 0x200); /* for S2 */
> +	writel(MAX_PARAM1, IMX_MAX_BASE + 0x300); /* for S3 */
> +	writel(MAX_PARAM1, IMX_MAX_BASE + 0x400); /* for S4 */
> +
> +	/* SGPCR - always park on last master */
> +	writel(0x10, IMX_MAX_BASE + 0x10);	/* for S0 */
> +	writel(0x10, IMX_MAX_BASE + 0x110);	/* for S1 */
> +	writel(0x10, IMX_MAX_BASE + 0x210);	/* for S2 */
> +	writel(0x10, IMX_MAX_BASE + 0x310);	/* for S3 */
> +	writel(0x10, IMX_MAX_BASE + 0x410);	/* for S4 */
> +
> +	/* MGPCR - restore default values */
> +	writel(0x0, IMX_MAX_BASE + 0x800);	/* for M0 */
> +	writel(0x0, IMX_MAX_BASE + 0x900);	/* for M1 */
> +	writel(0x0, IMX_MAX_BASE + 0xa00);	/* for M2 */
> +	writel(0x0, IMX_MAX_BASE + 0xb00);	/* for M3 */
> +	writel(0x0, IMX_MAX_BASE + 0xc00);	/* for M4 */
> +	writel(0x0, IMX_MAX_BASE + 0xd00);	/* for M5 */
> +
> +	writel(M3IFCTL_IPU1, IMX_M3IF_BASE);
> +
> +	return 0;
> +}
> +
> +core_initcall(pcaam1_core_setup);
> +
> +#define CCM_PDR0_PARAM_399	0x00011000
> +#define CCM_PDR0_PARAM_532	0x00001000
> +
> +static int do_cpufreq(struct command *cmdtp, int argc, char *argv[])
> +{
> +	unsigned long freq;
> +
> +	if (argc != 2) {
> +		barebox_cmd_usage(cmdtp);
> +		return 1;
> +	}
> +
> +	freq = simple_strtoul(argv[1], NULL, 0);
> +
> +	switch (freq) {
> +	case 399:
> +		writel(CCM_PDR0_PARAM_399, IMX_CCM_BASE + CCM_PDR0);
> +		break;
> +	case 532:
> +		writel(CCM_PDR0_PARAM_532, IMX_CCM_BASE + CCM_PDR0);
> +		break;
> +	default:
> +		barebox_cmd_usage(cmdtp);
> +		return 1;
> +	}
> +
> +	printf("Switched CPU frequency to %ldMHz\n", freq);
> +
> +	return 0;
> +}

We have this three times now. Please move this to some common place.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

      parent reply	other threads:[~2012-03-02  8:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-01 13:50 Christian Hemp
2012-03-01 13:50 ` [PATCH 2/2] pcaam1: Change the machine id/name to follow the phyCARD naming convention Christian Hemp
2012-03-02  8:11   ` Sascha Hauer
2012-03-02  8:28 ` Sascha Hauer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120302082834.GL3852@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=c.hemp@phytec.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox