On Friday 06 December 2013, Sergei Ianovich wrote:
> new file mode 100644
> index 0000000..2612e60
> --- /dev/null
> +++ b/arch/arm/configs/lp8x4x_defconfig
> @@ -0,0 +1,235 @@
> +CONFIG_CROSS_COMPILE="arm-linux-gnueabi-"
This will break some build bots, please remove it here and add it to your
build environment instead.
> +# CONFIG_LBDAF is not set
> +CONFIG_BLK_CMDLINE_PARSER=y
> +CONFIG_PARTITION_ADVANCED=y
> +CONFIG_BSD_DISKLABEL=y
> +CONFIG_MINIX_SUBPARTITION=y
> +CONFIG_SOLARIS_X86_PARTITION=y
> +CONFIG_UNIXWARE_DISKLABEL=y
> +CONFIG_LDM_PARTITION=y
You have some rather unusual options in here. I'd suggest you go through
the reduced defconfig file and remove all options that look like they
are unnecessary for your system.
It's probably based on some distro config that enables lots of modules
you don't actually want.
> +#define LP8X4X_FPGA_PHYS 0x17000000
> +#define LP8X4X_FPGA_VIRT ((void *) 0xf1000000)
> +#define LP8X4X_P2V(x) IOMEM((x) - LP8X4X_FPGA_PHYS +
> LP8X4X_FPGA_VIRT)
> +#define LP8X4X_V2P(x) ((x) - LP8X4X_FPGA_VIRT +
> LP8X4X_FPGA_PHYS)
I think you misunderstood my comment, the point was that you should
move the IOMEM() to the LP8X4X_FPGA_VIRT definition, like
#define LP8X4X_FPGA_VIRT ((void __iomem *) 0xf1000000)
#define LP8X4X_P2V(x) (x) - LP8X4X_FPGA_PHYS + LP8X4X_FPGA_VIRT)
which would result in the correct type because of pointer arithmetics.
> +/* board level registers in the FPGA */
> +
> +#define LP8X4X_EOI LP8X4X_P2V(0x17009006)
> +#define LP8X4X_INSINT LP8X4X_P2V(0x17009008)
> +#define LP8X4X_ENSYSINT LP8X4X_P2V(0x1700900A)
> +#define LP8X4X_PRIMINT LP8X4X_P2V(0x1700900C)
> +#define LP8X4X_SECOINT LP8X4X_P2V(0x1700900E)
> +#define LP8X4X_ENRISEINT LP8X4X_P2V(0x17009010)
> +#define LP8X4X_CLRRISEINT LP8X4X_P2V(0x17009012)
> +#define LP8X4X_ENHILVINT LP8X4X_P2V(0x17009014)
> +#define LP8X4X_CLRHILVINT LP8X4X_P2V(0x17009016)
> +#define LP8X4X_ENFALLINT LP8X4X_P2V(0x17009018)
> +#define LP8X4X_CLRFALLINT LP8X4X_P2V(0x1700901a)
Thinking about this again, it's actually better if you just get rid of
LP8X4X_P2V() entirely and redefine these to
#define LP8X4X_EOI 0x0006
#define LP8X4X_INSINT 0x0008
...
and change the users to do e.g.
readl(LP8X4X_FPGA_VIRT + LP8X4X_INSINT);
This is closer to the normal way of adding the offset to an __iomem
pointer returned from ioremap.
> + platform_device_register_resndata(NULL, "pxa2xx-flash", 0,
> + &lp8x4x_flash_resources[0], 1,
> + &lp8x4x_flash_data[0], sizeof(lp8x4x_flash_data[0]));
> + platform_device_register_resndata(NULL, "pxa2xx-flash", 1,
> + &lp8x4x_flash_resources[1], 1,
> + &lp8x4x_flash_data[1], sizeof(lp8x4x_flash_data[1]));
> + platform_device_register_simple("dm9000", 0,
> + &lp8x4x_dm9000_resources[0], 3);
> + platform_device_register_simple("dm9000", 1,
> + &lp8x4x_dm9000_resources[3], 3);
This looks much better than the first version, a slight improvement IMHO would
be to split the resource arrays into one symbol per device to turn this into
platform_device_register_resndata(NULL, "pxa2xx-flash", 0,
&lp8x4x_flash_resources0, 1,
&lp8x4x_flash_data0, sizeof(lp8x4x_flash_data0));
platform_device_register_resndata(NULL, "pxa2xx-flash", 1,
&lp8x4x_flash_resources1, 1,
&lp8x4x_flash_data1, sizeof(lp8x4x_flash_data1));
It's not important though if you have a strong preference for the way you
did this, it just seems unusual.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
